From d052db831c0407f9ff9ad20f2829da1484ae716f Mon Sep 17 00:00:00 2001 From: Gal Ankonina Date: Tue, 2 Jun 2026 12:37:45 +0300 Subject: [PATCH 01/10] feat(agents): migrate the finding normalizer to a Pydantic AI app-level agent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IMPL-0022 PR #3b — the second OpenCode consumer off the substrate. The finding normalizer is a single LLM call (raw scanner JSON → normalized findings); it now runs in-process through Pydantic AI instead of the singleton OpenCode process. - add `cliff/agents/runtime/normalizer_agent.py` — `NormalizedFinding` (a deliberately lenient output schema) + `build_normalizer_agent`, with `output_type=list[NormalizedFinding]`, no deps/tools. App-level, so the prompt is passed in by the caller. - rewrite `integrations/normalizer.py`: `normalize_findings(source, raw, *, env, model)` builds the model via the runtime provider factory and calls `agent.run()`. Pydantic AI's structured output + internal retries replace the hand-rolled `_call_llm_with_retry` / `_extract_json_array` / regex cleanup and the `opencode_client` model-override dance. The per-item `FindingCreate` validation + coercion is unchanged, so the partial-success `(valid, errors)` contract is preserved. - `ingest_worker.py`: inject `env_resolver` / `model_resolver` (mirroring the executor's pattern); resolve the provider env per job and fall back to the canonical active model when a job didn't pin one. - `main.py`: wire the worker with the app-level AI resolvers. Tests: - rewrite `test_normalizer.py` against a `FunctionModel` (success, partial failure, source_type injection, null-status default, list-wrapped raw_payload coercion, LLM-error → error string, model-not-configured). - update the `_process_job` call sites + mock signatures for the new resolver params; pass `env`/`model` in the real-LLM eval tests and pick a capable eval model (a nano-tier default under-extracts on batches). The singleton OpenCode process stays (repo-action agents still use the pool; #3c migrates them, #3d deletes the substrate). Full unit suite: 1552 passed; real-LLM normalizer evals pass with a capable model. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../cliff/agents/runtime/normalizer_agent.py | 78 +++++ backend/cliff/integrations/ingest_worker.py | 42 ++- backend/cliff/integrations/normalizer.py | 209 ++++-------- backend/cliff/main.py | 8 +- backend/tests/agents/test_normalizer_agent.py | 38 ++- .../agents/test_plain_description_eval.py | 21 +- .../test_ingest_worker_plain_description.py | 7 +- backend/tests/test_ingest_job.py | 27 +- backend/tests/test_normalizer.py | 316 ++++++++---------- 9 files changed, 415 insertions(+), 331 deletions(-) create mode 100644 backend/cliff/agents/runtime/normalizer_agent.py diff --git a/backend/cliff/agents/runtime/normalizer_agent.py b/backend/cliff/agents/runtime/normalizer_agent.py new file mode 100644 index 00000000..ba544bde --- /dev/null +++ b/backend/cliff/agents/runtime/normalizer_agent.py @@ -0,0 +1,78 @@ +"""App-level finding-normalizer agent (ADR-0022 / ADR-0047, IMPL-0022 PR #3b). + +The normalizer is a single LLM call — raw scanner JSON in, a list of +normalized findings out. Unlike the six pipeline agents it is *app-level*: +it runs outside any workspace, so it has no ``WorkspaceDeps`` and no tools. + +Pydantic AI's structured ``output_type`` + its internal validation/retry +loop replace the hand-rolled ``_call_llm_with_retry`` / ``_extract_json_array`` +machinery the OpenCode-era normalizer carried. + +The output schema (:class:`NormalizedFinding`) is deliberately **lenient** — +every field is optional. The model is coaxed toward the right shape by the +system prompt, but the load-bearing validation stays in +``normalize_findings``, which maps each item onto ``FindingCreate`` and +collects per-item errors. Keeping the PA schema lenient preserves the +normalizer's partial-success contract: one malformed item lands in +``errors`` while the rest succeed, rather than failing the whole batch. + +The domain prompt lives with the caller in +``cliff.integrations.normalizer`` (it is finding-ingest domain content, and +importing it here would be circular) and is passed in via ``system_prompt``. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any + +from pydantic import BaseModel +from pydantic_ai import Agent + +if TYPE_CHECKING: + from pydantic_ai.models import Model + + +class NormalizedFinding(BaseModel): + """One finding as extracted by the normalizer LLM. + + Lenient by design — see the module docstring. Mirrors the JSON object + the system prompt describes; the strict contract is ``FindingCreate``, + enforced downstream in ``normalize_findings``. + """ + + source_type: str | None = None + source_id: str | None = None + title: str | None = None + description: str | None = None + raw_severity: str | None = None + normalized_priority: str | None = None + asset_id: str | None = None + asset_label: str | None = None + status: str | None = None + likely_owner: str | None = None + why_this_matters: str | None = None + plain_description: str | None = None + # Arbitrary passthrough — the model occasionally wraps the original object + # in a single-element list, which ``normalize_findings`` coerces back to a + # dict before the strict ``FindingCreate`` validation. Typed ``Any`` so PA + # doesn't reject the list shape outright. + raw_payload: Any = None + + +def build_normalizer_agent( + model: Model, *, system_prompt: str +) -> Agent[None, list[NormalizedFinding]]: + """Build the app-level normalizer agent. + + No ``deps_type`` and no tools — a pure structured-extraction call. The + union-free ``output_type=list[NormalizedFinding]`` makes Pydantic AI + drive the model to return a JSON array of objects and validate it. + """ + return Agent( + model, + output_type=list[NormalizedFinding], + system_prompt=system_prompt, + ) + + +__all__ = ["NormalizedFinding", "build_normalizer_agent"] diff --git a/backend/cliff/integrations/ingest_worker.py b/backend/cliff/integrations/ingest_worker.py index 0760d1ac..8926a2db 100644 --- a/backend/cliff/integrations/ingest_worker.py +++ b/backend/cliff/integrations/ingest_worker.py @@ -23,8 +23,13 @@ from cliff.integrations.normalizer import normalize_findings if TYPE_CHECKING: + from collections.abc import Awaitable, Callable + import aiosqlite + EnvResolver = Callable[[], Awaitable[dict[str, str]]] + ModelResolver = Callable[[], Awaitable[str | None]] + logger = logging.getLogger(__name__) # Consecutive connection failures before backing off @@ -49,7 +54,13 @@ def estimate_tokens(raw_data: list[dict[str, Any]], chunk_size: int) -> int: return int(raw_chars / 3.5) + (num_chunks * 800) -async def _process_job(db: aiosqlite.Connection, job_id: str) -> None: +async def _process_job( + db: aiosqlite.Connection, + job_id: str, + *, + env_resolver: EnvResolver, + model_resolver: ModelResolver, +) -> None: """Process a single ingest job chunk-by-chunk.""" data = await get_ingest_job_raw_data(db, job_id) if data is None: @@ -58,6 +69,11 @@ async def _process_job(db: aiosqlite.Connection, job_id: str) -> None: return source, raw_data, chunk_size, model = data + # App-level AI state: resolve the provider env once per job; fall back to + # the canonical active model when the job didn't pin one at enqueue time. + env = await env_resolver() + if not model: + model = await model_resolver() await set_job_status(db, job_id, "processing") # For small imports, use per-finding chunks for better progress visibility @@ -79,7 +95,9 @@ async def _process_job(db: aiosqlite.Connection, job_id: str) -> None: chunk = raw_data[start:end] try: - valid, errors = await normalize_findings(source, chunk, model=model) + valid, errors = await normalize_findings( + source, chunk, env=env, model=model + ) consecutive_failures = 0 # reset on success except Exception as exc: consecutive_failures += 1 @@ -111,7 +129,7 @@ async def _process_job(db: aiosqlite.Connection, job_id: str) -> None: for item in chunk: try: sub_valid, sub_errors = await normalize_findings( - source, [item], model=model + source, [item], env=env, model=model ) valid.extend(sub_valid) errors.extend(sub_errors) @@ -159,9 +177,18 @@ async def _process_job(db: aiosqlite.Connection, job_id: str) -> None: await set_job_status(db, job_id, "completed") -async def ingest_worker_loop(db: aiosqlite.Connection) -> None: +async def ingest_worker_loop( + db: aiosqlite.Connection, + *, + env_resolver: EnvResolver, + model_resolver: ModelResolver, +) -> None: """Main worker loop — polls for pending jobs and processes them. + ``env_resolver`` / ``model_resolver`` supply the app-level AI provider + env + active model (ADR-0047 / IMPL-0022 PR #3b) — the normalizer is an + app-level PA agent now, not a singleton-OpenCode call. + This coroutine runs indefinitely until cancelled. """ logger.info("Ingest worker started") @@ -171,7 +198,12 @@ async def ingest_worker_loop(db: aiosqlite.Connection) -> None: job_id = await get_next_pending_job_id(db) if job_id: logger.info("Processing ingest job %s", job_id) - await _process_job(db, job_id) + await _process_job( + db, + job_id, + env_resolver=env_resolver, + model_resolver=model_resolver, + ) else: await asyncio.sleep(_POLL_INTERVAL) except asyncio.CancelledError: diff --git a/backend/cliff/integrations/normalizer.py b/backend/cliff/integrations/normalizer.py index 99739ad1..3dd34246 100644 --- a/backend/cliff/integrations/normalizer.py +++ b/backend/cliff/integrations/normalizer.py @@ -1,7 +1,15 @@ -"""LLM-powered finding normalizer using the singleton OpenCode process. +"""LLM-powered finding normalizer (Pydantic AI, app-level). Accepts raw scanner data from any vendor and normalizes it into FindingCreate -records via a dedicated extraction prompt. See ADR-0022 for design rationale. +records via a dedicated extraction prompt. See ADR-0022 for design rationale +and ADR-0047 / IMPL-0022 PR #3b for the OpenCode → Pydantic AI migration. + +This is an *app-level* agent (no workspace), so its provider key + model are +passed in by the caller (``env`` + ``model``) rather than resolved from a +workspace. Pydantic AI's structured ``output_type`` + internal retry loop +replace the hand-rolled JSON extraction and retry machinery the OpenCode-era +normalizer carried; the per-item ``FindingCreate`` validation below is +unchanged and still drives the partial-success ``(valid, errors)`` contract. Token-cost note (IMPL-0002 C1, 2026-04-16): the prompt grew by roughly ~625 input tokens when ``plain_description`` rules + examples were added. @@ -14,21 +22,23 @@ import json import logging -import re from typing import Any from pydantic import ValidationError - -from cliff.engine.client import opencode_client +from pydantic_ai.exceptions import ( + ModelHTTPError, + UnexpectedModelBehavior, + UsageLimitExceeded, + UserError, +) + +from cliff.agents.runtime.normalizer_agent import build_normalizer_agent +from cliff.agents.runtime.provider import ProviderConfigurationError, build_model from cliff.models import FindingCreate logger = logging.getLogger(__name__) MAX_BATCH_SIZE = 50 -_MAX_RETRIES = 2 # Total attempts: 1 original + 2 retries = 3 - -_RE_FENCED = re.compile(r"```(?:json)?\s*\n?(.*?)```", re.DOTALL) -_RE_TRAILING_COMMA = re.compile(r",\s*([}\]])") # --------------------------------------------------------------------------- # Normalizer prompt — tight extraction, few-shot, no chain-of-thought @@ -228,65 +238,45 @@ """ -def _build_prompt(source: str, raw_json: str) -> str: +def _build_user_message(source: str, raw_json: str) -> str: + """The per-call user message — just the scanner source + raw data. + + The schema, rules, and few-shot examples live in ``NORMALIZER_PROMPT``, + which is the agent's *system* prompt; this is only the variable payload. + """ return ( - f"{NORMALIZER_PROMPT}\n" f"Source: {source}\n" f"Raw data:\n```json\n{raw_json}\n```\n\n" - "JSON array output:" + "Normalize these findings." ) -# --------------------------------------------------------------------------- -# JSON array extractor — handles fenced blocks and bare arrays -# --------------------------------------------------------------------------- - - -def _extract_json_array(text: str) -> list[dict[str, Any]] | None: - """Extract a JSON array from LLM response text. - - Handles: bare JSON arrays, ```json fenced blocks, trailing commas. - Returns None if no valid array is found. - """ - fenced = _RE_FENCED.search(text) - candidate = fenced.group(1).strip() if fenced else text.strip() - - start = candidate.find("[") - if start == -1: - return None - - # Fix trailing commas before attempting parse (common LLM quirk) - cleaned = _RE_TRAILING_COMMA.sub(r"\1", candidate[start:]) - - # Use raw_decode to correctly handle brackets inside JSON strings - try: - parsed, _ = json.JSONDecoder().raw_decode(cleaned) - except json.JSONDecodeError: - return None - - if not isinstance(parsed, list): - return None - return parsed - - # --------------------------------------------------------------------------- # Main normalize function # --------------------------------------------------------------------------- async def normalize_findings( - source: str, raw_data: list[dict[str, Any]], *, model: str | None = None + source: str, + raw_data: list[dict[str, Any]], + *, + env: dict[str, str], + model: str | None = None, ) -> tuple[list[FindingCreate], list[str]]: - """Normalize raw scanner findings via LLM extraction. + """Normalize raw scanner findings via a Pydantic AI extraction call. - Returns (valid_findings, errors) where errors contains human-readable - strings for items that failed validation. + Returns (valid_findings, errors) where errors holds human-readable + strings for items that failed validation. Partial success is normal — + one malformed item lands in ``errors`` while the rest succeed. Args: source: Scanner name (e.g. 'snyk', 'wiz'). raw_data: List of raw finding dicts from the scanner. - model: Optional model override (e.g. 'openai/gpt-4.1-mini'). - If provided, temporarily sets the OpenCode model config. + env: Provider credentials (e.g. ``{"OPENAI_API_KEY": ...}``) used to + build the model — the app-level analogue of the per-workspace + env the pipeline agents receive. + model: Full model id (``'/'``) to run with. Required + in practice; ``build_model`` rejects a missing/blank value. """ if not raw_data: return [], [] @@ -297,106 +287,49 @@ async def normalize_findings( "Use the async ingest endpoint for larger batches." ] - # Build prompt — compact JSON to minimize token cost - raw_json = json.dumps(raw_data, separators=(",", ":")) - prompt = _build_prompt(source, raw_json) + try: + pa_model = build_model(env, model) + except ProviderConfigurationError as exc: + return [], [f"Normalizer model not configured: {exc}"] - # Temporarily override model if requested - original_model: str | None = None - if model: - try: - config = await opencode_client.get_config() - original_model = config.get("model", None) - await opencode_client.update_config({"model": model}) - logger.info("Normalizer using model override: %s", model) - except Exception as exc: - logger.warning("Failed to set model override %s: %s", model, exc) + agent = build_normalizer_agent(pa_model, system_prompt=NORMALIZER_PROMPT) + raw_json = json.dumps(raw_data, separators=(",", ":")) try: - items = await _call_llm_with_retry(prompt) - finally: - # Restore original model if we changed it - if original_model is not None: - try: - await opencode_client.update_config({"model": original_model}) - except Exception: - logger.warning("Failed to restore original model %s", original_model) - - if items is None: - return [], ["Failed to parse JSON array from LLM response after 3 attempts"] - - # Validate each item against FindingCreate schema + result = await agent.run(_build_user_message(source, raw_json)) + except ( + ModelHTTPError, + UnexpectedModelBehavior, + UsageLimitExceeded, + UserError, + ) as exc: + logger.warning("Normalizer LLM call failed: %s", exc) + return [], [f"Normalizer LLM call failed: {type(exc).__name__}: {exc}"] + + # Pydantic AI handed back validated (lenient) NormalizedFinding objects; + # the strict FindingCreate contract — and the per-item partial-success + # accounting — is enforced here, exactly as on the OpenCode-era path. findings: list[FindingCreate] = [] errors: list[str] = [] - for i, item in enumerate(items): - if not isinstance(item, dict): - errors.append(f"Finding {i + 1}: expected object, got {type(item).__name__}") - continue - if "source_type" not in item: + for i, nf in enumerate(result.output): + item = nf.model_dump() + # The model_dump always carries every key; fill the load-bearing + # defaults the LLM may have left null. + if item.get("source_type") is None: item["source_type"] = source - # Coerce raw_payload: LLM sometimes wraps the original object in a list + if item.get("status") is None: + item["status"] = "new" + # Coerce raw_payload: the model sometimes wraps the original object + # in a single-element list. rp = item.get("raw_payload") if isinstance(rp, list): - item["raw_payload"] = rp[0] if len(rp) == 1 and isinstance(rp[0], dict) else None + item["raw_payload"] = ( + rp[0] if len(rp) == 1 and isinstance(rp[0], dict) else None + ) try: findings.append(FindingCreate.model_validate(item)) except ValidationError as exc: errors.append(f"Finding {i + 1}: {exc}") return findings, errors - - -async def _call_llm_with_retry(prompt: str) -> list[dict[str, Any]] | None: - """Send prompt to LLM and extract JSON array, with up to 3 attempts. - - Creates a fresh session for each retry to avoid stuck generation patterns. - Returns the parsed JSON array or None if all attempts fail. - """ - last_response: str | None = None - - for attempt in range(_MAX_RETRIES + 1): - session = await opencode_client.create_session() - try: - full_text = await opencode_client.send_and_get_response( - session.id, prompt - ) - except Exception as exc: - logger.warning( - "Normalizer attempt %d/%d: LLM error: %s", - attempt + 1, _MAX_RETRIES + 1, exc, - ) - last_response = f"[exception] {exc}" - continue - - if not full_text: - logger.warning( - "Normalizer attempt %d/%d: LLM returned empty response", - attempt + 1, _MAX_RETRIES + 1, - ) - last_response = "[empty response]" - continue - - last_response = full_text - items = _extract_json_array(full_text) - if items is not None: - if attempt > 0: - logger.info("Normalizer succeeded on attempt %d", attempt + 1) - return items - - # Log what the LLM actually said for debugging - snippet = full_text[:500].replace("\n", "\\n") - logger.warning( - "Normalizer attempt %d/%d: Failed to parse JSON from response: %s", - attempt + 1, _MAX_RETRIES + 1, snippet, - ) - - # All attempts failed — include a snippet in the error for the user - if last_response and not last_response.startswith("["): - snippet = last_response[:200].replace("\n", " ").strip() - logger.error( - "Normalizer gave up after %d attempts. Last response: %s", - _MAX_RETRIES + 1, snippet, - ) - - return None diff --git a/backend/cliff/main.py b/backend/cliff/main.py index 1d9f4e0d..c1182533 100644 --- a/backend/cliff/main.py +++ b/backend/cliff/main.py @@ -399,7 +399,13 @@ async def _idle_cleanup_loop() -> None: # Background ingest worker (ADR-0023) ingest_task: asyncio.Task[None] | None = None if db_connection._db is not None: - ingest_task = asyncio.create_task(ingest_worker_loop(db_connection._db)) + ingest_task = asyncio.create_task( + ingest_worker_loop( + db_connection._db, + env_resolver=_ai_env_resolver, + model_resolver=_ai_model_resolver, + ) + ) # Assessment watchdog — reaps wedged ``pending``/``running`` rows the # outer asyncio.timeout in ``api/_background.py`` couldn't catch (e.g. diff --git a/backend/tests/agents/test_normalizer_agent.py b/backend/tests/agents/test_normalizer_agent.py index 56c81378..e295519d 100644 --- a/backend/tests/agents/test_normalizer_agent.py +++ b/backend/tests/agents/test_normalizer_agent.py @@ -10,6 +10,7 @@ from __future__ import annotations import json +import os from pathlib import Path import pytest @@ -20,6 +21,27 @@ VALID_PRIORITIES = {"critical", "high", "medium", "low", "info"} FIXTURES_DIR = Path(__file__).resolve().parents[3] / "fixtures" +# Real-LLM provider state for the normalizer (now an app-level PA agent — +# ADR-0047 / IMPL-0022 PR #3b). These tests are skip-gated on an API key +# being present (see conftest), so the env carries a usable provider key. +_LLM_ENV = { + k: v for k, v in os.environ.items() if k.endswith(("_API_KEY", "_BASE_URL")) +} + + +def _eval_model() -> str: + """Pick a capable, cheap model for the real-LLM eval (override with + ``CLIFF_EVAL_MODEL``). Multi-item batch extraction needs a real model — + a nano-tier model under-extracts and fails the batch assertions.""" + if override := os.environ.get("CLIFF_EVAL_MODEL"): + return override + if os.environ.get("ANTHROPIC_API_KEY"): + return "anthropic/claude-haiku-4-5" + return "openai/gpt-4o-mini" + + +_LLM_MODEL = _eval_model() + def _load_fixture(name: str) -> list[dict]: path = FIXTURES_DIR / name @@ -59,7 +81,7 @@ async def test_single_snyk_finding(): } ] - findings, errors = await normalize_findings("snyk", raw) + findings, errors = await normalize_findings("snyk", raw, env=_LLM_ENV, model=_LLM_MODEL) assert len(findings) == 1, f"Expected 1 finding, got {len(findings)}. Errors: {errors}" f = findings[0] @@ -89,7 +111,7 @@ async def test_single_wiz_finding(): } ] - findings, errors = await normalize_findings("wiz", raw) + findings, errors = await normalize_findings("wiz", raw, env=_LLM_ENV, model=_LLM_MODEL) assert len(findings) == 1, f"Expected 1 finding, got {len(findings)}. Errors: {errors}" f = findings[0] @@ -109,7 +131,7 @@ async def test_batch_snyk_5_findings(): raw = _load_fixture("sample-snyk-export.json") assert len(raw) == 5 - findings, errors = await normalize_findings("snyk", raw) + findings, errors = await normalize_findings("snyk", raw, env=_LLM_ENV, model=_LLM_MODEL) assert len(findings) >= 4, ( f"Expected at least 4/5 findings, got {len(findings)}. Errors: {errors}" @@ -128,7 +150,7 @@ async def test_batch_wiz_3_findings(): raw = _load_fixture("sample-wiz-export.json") assert len(raw) == 3 - findings, errors = await normalize_findings("wiz", raw) + findings, errors = await normalize_findings("wiz", raw, env=_LLM_ENV, model=_LLM_MODEL) assert len(findings) >= 2, ( f"Expected at least 2/3 findings, got {len(findings)}. Errors: {errors}" @@ -176,7 +198,7 @@ async def test_severity_mapping(): }, ] - findings, errors = await normalize_findings("snyk", raw) + findings, errors = await normalize_findings("snyk", raw, env=_LLM_ENV, model=_LLM_MODEL) assert len(findings) >= 3, f"Expected at least 3/4, got {len(findings)}. Errors: {errors}" @@ -203,7 +225,7 @@ async def test_minimal_input_fields(): } ] - findings, errors = await normalize_findings("snyk", raw) + findings, errors = await normalize_findings("snyk", raw, env=_LLM_ENV, model=_LLM_MODEL) assert len(findings) == 1, f"Expected 1 finding. Errors: {errors}" f = findings[0] @@ -227,7 +249,7 @@ async def test_unknown_scanner_format(): } ] - findings, errors = await normalize_findings("trivy", raw) + findings, errors = await normalize_findings("trivy", raw, env=_LLM_ENV, model=_LLM_MODEL) assert len(findings) == 1, f"Expected 1 finding. Errors: {errors}" f = findings[0] @@ -248,7 +270,7 @@ async def test_large_batch_20_findings(): assert len(raw) == 20 - findings, errors = await normalize_findings("snyk", raw) + findings, errors = await normalize_findings("snyk", raw, env=_LLM_ENV, model=_LLM_MODEL) # gpt-4.1-nano truncates output for large batches — this is a known limitation. # The chunk fallback in ingest_worker handles this in production by retrying diff --git a/backend/tests/agents/test_plain_description_eval.py b/backend/tests/agents/test_plain_description_eval.py index 8684229c..25da1470 100644 --- a/backend/tests/agents/test_plain_description_eval.py +++ b/backend/tests/agents/test_plain_description_eval.py @@ -19,6 +19,7 @@ from __future__ import annotations import json +import os import re from pathlib import Path @@ -26,6 +27,24 @@ from cliff.integrations.normalizer import normalize_findings +# Real-LLM provider state for the app-level normalizer (IMPL-0022 PR #3b); +# skip-gated on an API key being present (see conftest). +_LLM_ENV = { + k: v for k, v in os.environ.items() if k.endswith(("_API_KEY", "_BASE_URL")) +} + + +def _eval_model() -> str: + """Capable, cheap model for the real-LLM eval (override: ``CLIFF_EVAL_MODEL``).""" + if override := os.environ.get("CLIFF_EVAL_MODEL"): + return override + if os.environ.get("ANTHROPIC_API_KEY"): + return "anthropic/claude-haiku-4-5" + return "openai/gpt-4o-mini" + + +_LLM_MODEL = _eval_model() + FIXTURE_PATH = Path(__file__).parent / "fixtures" / "plain_description_evals.json" @@ -48,7 +67,7 @@ def _count_sentences(text: str) -> int: async def test_plain_description_shape(record: dict) -> None: """Each fixture produces a plain_description that passes shape checks.""" findings, errors = await normalize_findings( - record["source"], [record["raw_finding"]] + record["source"], [record["raw_finding"]], env=_LLM_ENV, model=_LLM_MODEL ) assert not errors, f"Normalizer errors for {record['id']}: {errors}" assert len(findings) == 1, f"Expected 1 finding, got {len(findings)}" diff --git a/backend/tests/integrations/test_ingest_worker_plain_description.py b/backend/tests/integrations/test_ingest_worker_plain_description.py index fec27fd2..1e63c826 100644 --- a/backend/tests/integrations/test_ingest_worker_plain_description.py +++ b/backend/tests/integrations/test_ingest_worker_plain_description.py @@ -39,7 +39,12 @@ async def test_process_job_persists_plain_description(db): ) mocked = AsyncMock(return_value=([fc], [])) with patch("cliff.integrations.ingest_worker.normalize_findings", mocked): - await _process_job(db, job.job_id) + await _process_job( + db, + job.job_id, + env_resolver=AsyncMock(return_value={"OPENAI_API_KEY": "k"}), + model_resolver=AsyncMock(return_value="openai/gpt-4o-mini"), + ) findings = await list_findings(db) assert len(findings) == 1 diff --git a/backend/tests/test_ingest_job.py b/backend/tests/test_ingest_job.py index 849cfa1a..8eea3bf5 100644 --- a/backend/tests/test_ingest_job.py +++ b/backend/tests/test_ingest_job.py @@ -201,7 +201,7 @@ async def test_process_job_success(): # Mock normalize_findings to return valid FindingCreate objects from cliff.models import FindingCreate - async def _mock_normalize(source, data, *, model=None): + async def _mock_normalize(source, data, *, env=None, model=None): return [ FindingCreate(source_type="wiz", source_id=f"wiz-{i}", title=f"Finding {i}") for i in range(len(data)) @@ -219,7 +219,12 @@ async def _mock_normalize(source, data, *, model=None): ): from cliff.integrations.ingest_worker import _process_job - await _process_job(db, job.job_id) + await _process_job( + db, + job.job_id, + env_resolver=AsyncMock(return_value={"OPENAI_API_KEY": "k"}), + model_resolver=AsyncMock(return_value="openai/gpt-4o-mini"), + ) progress = await get_ingest_job(db, job.job_id) assert progress.status == "completed" @@ -243,7 +248,7 @@ async def test_process_job_partial_failure(): call_count = 0 - async def _mock_normalize(source, data, *, model=None): + async def _mock_normalize(source, data, *, env=None, model=None): nonlocal call_count call_count += 1 if call_count == 1: @@ -267,7 +272,12 @@ async def _mock_normalize(source, data, *, model=None): ): from cliff.integrations.ingest_worker import _process_job - await _process_job(db, job.job_id) + await _process_job( + db, + job.job_id, + env_resolver=AsyncMock(return_value={"OPENAI_API_KEY": "k"}), + model_resolver=AsyncMock(return_value="openai/gpt-4o-mini"), + ) progress = await get_ingest_job(db, job.job_id) assert progress.status == "completed" @@ -295,7 +305,7 @@ async def test_process_job_cancellation(): chunk_calls = 0 - async def _mock_normalize(source, data, *, model=None): + async def _mock_normalize(source, data, *, env=None, model=None): nonlocal chunk_calls chunk_calls += 1 # Cancel after first chunk @@ -318,7 +328,12 @@ async def _mock_normalize(source, data, *, model=None): ): from cliff.integrations.ingest_worker import _process_job - await _process_job(db, job.job_id) + await _process_job( + db, + job.job_id, + env_resolver=AsyncMock(return_value={"OPENAI_API_KEY": "k"}), + model_resolver=AsyncMock(return_value="openai/gpt-4o-mini"), + ) progress = await get_ingest_job(db, job.job_id) assert progress.status == "cancelled" diff --git a/backend/tests/test_normalizer.py b/backend/tests/test_normalizer.py index d8141c20..d70f4152 100644 --- a/backend/tests/test_normalizer.py +++ b/backend/tests/test_normalizer.py @@ -1,124 +1,73 @@ -"""Tests for the LLM-powered finding normalizer.""" +"""Unit tests for the Pydantic AI finding normalizer (IMPL-0022 PR #3b). + +These drive ``normalize_findings`` with a ``FunctionModel`` (no live LLM): +``build_model`` is patched to return the fake model, so each test controls +the structured output the agent "returns" and asserts the downstream +``FindingCreate`` validation + partial-success ``(valid, errors)`` contract. +""" from __future__ import annotations -import json -from unittest.mock import AsyncMock, patch +from unittest.mock import patch import pytest +from pydantic_ai.exceptions import ModelHTTPError +from pydantic_ai.messages import ModelResponse, ToolCallPart +from pydantic_ai.models.function import AgentInfo, FunctionModel + +from cliff.integrations.normalizer import MAX_BATCH_SIZE, normalize_findings + +_ENV = {"OPENAI_API_KEY": "test-key"} +_MODEL = "openai/gpt-4o-mini" -from cliff.integrations.normalizer import MAX_BATCH_SIZE, _extract_json_array, normalize_findings - -# --------------------------------------------------------------------------- -# _extract_json_array unit tests -# --------------------------------------------------------------------------- - - -class TestExtractJsonArray: - def test_bare_array(self): - text = '[{"source_type": "wiz", "source_id": "1", "title": "test"}]' - result = _extract_json_array(text) - assert result is not None - assert len(result) == 1 - assert result[0]["source_id"] == "1" - - def test_fenced_json(self): - text = '```json\n[{"source_type": "wiz", "source_id": "1", "title": "t"}]\n```' - result = _extract_json_array(text) - assert result is not None - assert len(result) == 1 - - def test_fenced_no_lang(self): - text = '```\n[{"a": 1}]\n```' - result = _extract_json_array(text) - assert result == [{"a": 1}] - - def test_trailing_comma(self): - text = '[{"a": 1}, {"b": 2},]' - result = _extract_json_array(text) - assert result is not None - assert len(result) == 2 - - def test_surrounding_text(self): - text = 'Here are the results:\n[{"x": 1}]\nDone!' - result = _extract_json_array(text) - assert result == [{"x": 1}] - - def test_no_array(self): - assert _extract_json_array("no json here") is None - - def test_empty_array(self): - assert _extract_json_array("[]") == [] - - def test_not_an_array(self): - assert _extract_json_array('{"key": "value"}') is None - - def test_malformed_json(self): - assert _extract_json_array("[{bad json}]") is None - - def test_nested_brackets(self): - text = '[{"tags": ["a", "b"], "name": "test"}]' - result = _extract_json_array(text) - assert result is not None - assert result[0]["tags"] == ["a", "b"] - - -# --------------------------------------------------------------------------- -# normalize_findings tests (mocked OpenCode client) -# --------------------------------------------------------------------------- - -WIZ_LLM_RESPONSE = json.dumps([ - { - "source_type": "wiz", - "source_id": "wiz-123", - "title": "S3 bucket publicly accessible", - "description": "Public read access via bucket policy.", - "raw_severity": "CRITICAL", - "normalized_priority": "critical", - "asset_id": "arn:aws:s3:::my-bucket", - "asset_label": "my-bucket", - "status": "new", - "likely_owner": None, - "why_this_matters": "Publicly accessible S3 buckets can expose sensitive data.", - } -]) - -PARTIAL_LLM_RESPONSE = json.dumps([ - { - "source_type": "snyk", - "source_id": "SNYK-001", - "title": "Prototype pollution in lodash", - "status": "new", - }, - { - "source_type": "snyk", - # Missing required field: source_id - "title": "Another vuln", - }, -]) - - -@pytest.fixture -def mock_opencode(): - """Patch the opencode_client singleton for normalizer tests. - - The normalizer uses Mode 1 (synchronous RPC) via send_and_get_response, - so we mock that single call. It returns the LLM text or None. + +def _model_returning(items: list[dict]) -> FunctionModel: + """A FunctionModel that emits *items* as the agent's structured output. + + PA wraps a ``list[...]`` output in a synthetic ``final_result`` tool whose + single argument is ``response`` — see the agent's output schema. """ - with patch("cliff.integrations.normalizer.opencode_client") as mock: - mock.create_session = AsyncMock() - mock.create_session.return_value.id = "test-session-id" - mock.send_and_get_response = AsyncMock(return_value=None) - mock.get_config = AsyncMock(return_value={"model": "openai/gpt-4.1-nano"}) - mock.update_config = AsyncMock(return_value={}) - yield mock + def _fn(messages, info: AgentInfo) -> ModelResponse: + tool_name = info.output_tools[0].name + return ModelResponse( + parts=[ToolCallPart(tool_name=tool_name, args={"response": items})] + ) + + return FunctionModel(_fn) + + +def _erroring_model() -> FunctionModel: + def _fn(messages, info: AgentInfo) -> ModelResponse: + raise ModelHTTPError(status_code=429, model_name="x", body="rate limited") + + return FunctionModel(_fn) + + +def _patch_model(model: FunctionModel): + return patch("cliff.integrations.normalizer.build_model", return_value=model) -@pytest.mark.asyncio -async def test_normalize_success(mock_opencode): - mock_opencode.send_and_get_response.return_value = WIZ_LLM_RESPONSE - findings, errors = await normalize_findings("wiz", [{"id": "wiz-123", "name": "test"}]) +_WIZ_ITEM = { + "source_type": "wiz", + "source_id": "wiz-123", + "title": "S3 bucket publicly accessible", + "description": "Public read access via bucket policy.", + "raw_severity": "CRITICAL", + "normalized_priority": "critical", + "asset_id": "arn:aws:s3:::my-bucket", + "asset_label": "my-bucket", + "status": "new", + "why_this_matters": "Publicly accessible S3 buckets can expose data.", +} + + +@pytest.mark.asyncio +async def test_normalize_success(): + with _patch_model(_model_returning([_WIZ_ITEM])): + findings, errors = await normalize_findings( + "wiz", [{"id": "wiz-123", "name": "test"}], env=_ENV, model=_MODEL + ) assert len(findings) == 1 assert findings[0].source_type == "wiz" @@ -127,16 +76,23 @@ async def test_normalize_success(mock_opencode): assert findings[0].normalized_priority == "critical" assert errors == [] - mock_opencode.create_session.assert_called_once() - mock_opencode.send_and_get_response.assert_called_once() - @pytest.mark.asyncio -async def test_normalize_partial_failure(mock_opencode): - """One valid finding, one missing source_id — partial result.""" - mock_opencode.send_and_get_response.return_value = PARTIAL_LLM_RESPONSE - - findings, errors = await normalize_findings("snyk", [{"a": 1}, {"b": 2}]) +async def test_normalize_partial_failure(): + """One valid finding, one missing source_id — partial result preserved.""" + items = [ + { + "source_type": "snyk", + "source_id": "SNYK-001", + "title": "Prototype pollution in lodash", + "status": "new", + }, + {"source_type": "snyk", "title": "Another vuln"}, # missing source_id + ] + with _patch_model(_model_returning(items)): + findings, errors = await normalize_findings( + "snyk", [{"a": 1}, {"b": 2}], env=_ENV, model=_MODEL + ) assert len(findings) == 1 assert findings[0].source_id == "SNYK-001" @@ -145,76 +101,94 @@ async def test_normalize_partial_failure(mock_opencode): @pytest.mark.asyncio -async def test_normalize_empty_input(mock_opencode): - findings, errors = await normalize_findings("wiz", []) - assert findings == [] +async def test_normalize_injects_source_type(): + """If the model omits source_type, it's filled from the request source.""" + items = [{"source_id": "x-1", "title": "Test vuln", "status": "new"}] + with _patch_model(_model_returning(items)): + findings, errors = await normalize_findings( + "trivy", [{"id": "x-1"}], env=_ENV, model=_MODEL + ) + + assert len(findings) == 1 + assert findings[0].source_type == "trivy" assert errors == [] - mock_opencode.create_session.assert_not_called() @pytest.mark.asyncio -async def test_normalize_batch_too_large(mock_opencode): - raw = [{"id": str(i)} for i in range(MAX_BATCH_SIZE + 1)] - findings, errors = await normalize_findings("wiz", raw) - assert findings == [] - assert len(errors) == 1 - assert "Batch too large" in errors[0] - mock_opencode.create_session.assert_not_called() +async def test_normalize_fills_status_when_null(): + """A null status from the model defaults to 'new' (not a validation error).""" + items = [{"source_type": "wiz", "source_id": "w-9", "title": "T", "status": None}] + with _patch_model(_model_returning(items)): + findings, errors = await normalize_findings( + "wiz", [{"id": "w-9"}], env=_ENV, model=_MODEL + ) + assert errors == [] + assert findings[0].status == "new" -@pytest.mark.asyncio -async def test_normalize_llm_error(mock_opencode): - mock_opencode.send_and_get_response.side_effect = RuntimeError("rate limit exceeded") - findings, errors = await normalize_findings("wiz", [{"id": "1"}]) - assert findings == [] - assert len(errors) == 1 - assert "Failed to parse JSON array" in errors[0] - # With retry, it should have been called 3 times (1 original + 2 retries) - assert mock_opencode.create_session.call_count == 3 +@pytest.mark.asyncio +async def test_normalize_coerces_listwrapped_raw_payload(): + """The model sometimes wraps raw_payload in a single-element list.""" + payload = {"id": "w-1", "extra": True} + items = [ + { + "source_type": "wiz", + "source_id": "w-1", + "title": "T", + "status": "new", + "raw_payload": [payload], + } + ] + with _patch_model(_model_returning(items)): + findings, _ = await normalize_findings( + "wiz", [{"id": "w-1"}], env=_ENV, model=_MODEL + ) + + assert findings[0].raw_payload == payload @pytest.mark.asyncio -async def test_normalize_empty_response(mock_opencode): - # send_and_get_response returns None by default (fixture) - findings, errors = await normalize_findings("wiz", [{"id": "1"}]) +async def test_normalize_empty_input(): + # No model call at all for empty input. + with patch("cliff.integrations.normalizer.build_model") as build: + findings, errors = await normalize_findings("wiz", [], env=_ENV, model=_MODEL) assert findings == [] - assert "Failed to parse JSON array" in errors[0] - # Retried 3 times - assert mock_opencode.create_session.call_count == 3 + assert errors == [] + build.assert_not_called() @pytest.mark.asyncio -async def test_normalize_malformed_response(mock_opencode): - mock_opencode.send_and_get_response.return_value = "Sorry, I can't do that." - - findings, errors = await normalize_findings("wiz", [{"id": "1"}]) +async def test_normalize_batch_too_large(): + raw = [{"id": str(i)} for i in range(MAX_BATCH_SIZE + 1)] + with patch("cliff.integrations.normalizer.build_model") as build: + findings, errors = await normalize_findings( + "wiz", raw, env=_ENV, model=_MODEL + ) assert findings == [] - assert "Failed to parse" in errors[0] - # Retried 3 times - assert mock_opencode.create_session.call_count == 3 + assert len(errors) == 1 + assert "Batch too large" in errors[0] + build.assert_not_called() @pytest.mark.asyncio -async def test_normalize_fenced_response(mock_opencode): - """LLM wraps JSON in markdown fences — should still parse.""" - fenced = f"```json\n{WIZ_LLM_RESPONSE}\n```" - mock_opencode.send_and_get_response.return_value = fenced - - findings, errors = await normalize_findings("wiz", [{"id": "wiz-123"}]) - assert len(findings) == 1 - assert errors == [] +async def test_normalize_llm_error_returns_error_not_raises(): + """An LLM transport error is captured as an error string, not raised.""" + with _patch_model(_erroring_model()): + findings, errors = await normalize_findings( + "wiz", [{"id": "1"}], env=_ENV, model=_MODEL + ) + assert findings == [] + assert len(errors) == 1 + assert "Normalizer LLM call failed" in errors[0] @pytest.mark.asyncio -async def test_normalize_injects_source_type(mock_opencode): - """If LLM omits source_type, it's injected from the request.""" - response = json.dumps([ - {"source_id": "x-1", "title": "Test vuln", "status": "new"}, - ]) - mock_opencode.send_and_get_response.return_value = response - - findings, errors = await normalize_findings("trivy", [{"id": "x-1"}]) - assert len(findings) == 1 - assert findings[0].source_type == "trivy" - assert errors == [] +async def test_normalize_model_not_configured(): + """A missing/blank model surfaces as a friendly error, not a crash.""" + findings, errors = await normalize_findings( + "wiz", [{"id": "1"}], env={}, model=None + ) + assert findings == [] + assert len(errors) == 1 + assert "Normalizer model not configured" in errors[0] From 804cb5f0fc718454368134a81b8230b795ecc6e0 Mon Sep 17 00:00:00 2001 From: Gal Ankonina Date: Tue, 2 Jun 2026 13:15:15 +0300 Subject: [PATCH 02/10] feat(agents): migrate repo-action agents to Pydantic AI (#3c) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IMPL-0022 PR #3c — the last per-workspace-pool consumer. The two posture generators (security_md_generator, dependabot_config_generator, ADR-0024) clone → write → commit → push → open a draft PR; they now run in-process through Pydantic AI, reusing the executor's bash/edit/read/gh tools, instead of a per-workspace OpenCode process driven by a Jinja template. - add cliff/agents/runtime/repo_actions.py — RepoActionOutput + build_repo_action_agent(model, kind). Both system prompts are adapted for the PA tool API: self-contained `git -C repo` commands (bash does not persist cwd), the `edit` tool for file writes, and `output_type` in place of the JSON-code-block contract. - WorkspaceDeps.auto_approve + gate_tool_call honour it: repo-action runs pre-approve the `ask` tier (no HITL surface for a one-shot background run); the `deny` tier still hard-denies catastrophic commands. - rewrite repo_workspace_runner.RepoAgentRunner: pool.get_or_start() → agent.run(); drop the SSE collect/stall machinery, template render, and output_parser usage; keep the B16 PR-URL verification. - rewire the _engine_dep spawner: pool → app-level AI env/model resolvers. Also fixes a latent bug in the merged PR #2 executor: the `bash` tool passed `env=env_vars`, which REPLACED the process environment (env_vars carries only GH_TOKEN/CLIFF_REPO_URL) and would strip PATH/HOME so git/gh couldn't run. Now merges over os.environ. The executor's live clone→PR smoke was deferred, so this had never surfaced. Tests: runner (FunctionModel + the B16 verification branches), spawner (autouse no-op for the now-always-launched background run), and auto_approve gating. The deterministic suite stays green. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/cliff/agents/runtime/deps.py | 6 + backend/cliff/agents/runtime/repo_actions.py | 459 ++++++++++++++++++ backend/cliff/agents/runtime/tools/bash.py | 10 +- .../cliff/agents/runtime/tools/permissions.py | 12 +- backend/cliff/api/_engine_dep.py | 49 +- .../cliff/workspace/repo_workspace_runner.py | 262 ++++------ .../tests/agents/tools/test_permissions.py | 27 +- backend/tests/test_repo_workspace_runner.py | 175 ++++--- backend/tests/test_repo_workspace_spawner.py | 29 +- 9 files changed, 750 insertions(+), 279 deletions(-) create mode 100644 backend/cliff/agents/runtime/repo_actions.py diff --git a/backend/cliff/agents/runtime/deps.py b/backend/cliff/agents/runtime/deps.py index 4d92e4bb..7dceac30 100644 --- a/backend/cliff/agents/runtime/deps.py +++ b/backend/cliff/agents/runtime/deps.py @@ -25,3 +25,9 @@ class WorkspaceDeps: prior_context: dict[str, dict[str, Any]] = field(default_factory=dict) env_vars: dict[str, str] = field(default_factory=dict) user_note: str | None = None + # Repo-action workspaces (ADR-0024 security.md / dependabot generators) + # pre-approve their tools: the user already authorised the single action + # by clicking "open a PR", and a one-shot background run has no HITL + # surface to prompt against. When True the ``ask`` tier auto-proceeds; + # the ``deny`` tier (catastrophic commands) still denies. + auto_approve: bool = False diff --git a/backend/cliff/agents/runtime/repo_actions.py b/backend/cliff/agents/runtime/repo_actions.py new file mode 100644 index 00000000..7ff21a3a --- /dev/null +++ b/backend/cliff/agents/runtime/repo_actions.py @@ -0,0 +1,459 @@ +"""Repo-action agents (ADR-0024 / ADR-0047, IMPL-0022 PR #3c). + +The two posture generators — ``security_md_generator`` and +``dependabot_config_generator`` — are single-shot, tool-using agents: they +clone a repo, write one file, and open a draft PR, end-to-end in one run. +They used to run on a per-workspace OpenCode process driven by a Jinja +template; they now run in-process through Pydantic AI, reusing the +executor's runtime tools (``bash`` / ``edit`` / ``read`` / ``gh``). + +Two differences from the remediation_executor: + +* **Pre-approved tools.** ``WorkspaceDeps.auto_approve=True`` — there's no + HITL surface for a background "open a PR" click, and the user already + authorised the action. The catastrophic ``deny`` tier still hard-denies. +* **No DB row.** The runner persists a status file to disk; there's no + ``AgentRun`` / sidebar machinery. + +The per-run repo URL + parameters are passed in the *user* message (see +``build_repo_action_prompt``); the static workflow lives in the system +prompt. The structured result (``status`` / ``pr_url`` / …) is the agent's +``output_type`` — no JSON-code-block contract to parse. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any, Literal + +from pydantic import BaseModel +from pydantic_ai import Agent + +from cliff.agents.runtime.deps import WorkspaceDeps +from cliff.agents.runtime.tools.bash import bash +from cliff.agents.runtime.tools.edit import edit +from cliff.agents.runtime.tools.gh import gh +from cliff.agents.runtime.tools.read import read +from cliff.workspace.workspace_dir_manager import WorkspaceKind + +if TYPE_CHECKING: + from collections.abc import Sequence + + from pydantic_ai.models import Model + from pydantic_ai.toolsets import AbstractToolset + + +class RepoActionOutput(BaseModel): + """Structured result of a repo-action run. + + Mirrors the ``structured_output`` block the OpenCode-era templates + required, so ``RepoAgentRunner`` maps it onto the on-disk status file + unchanged. + """ + + status: Literal["pr_created", "already_present", "failed"] + pr_url: str | None = None + branch_name: str | None = None + file_path: str | None = None + error_details: str | None = None + summary: str | None = None + result_card_markdown: str | None = None + # Dependabot only — the ecosystems it detected and configured. + detected_ecosystems: list[str] | None = None + + +_TOOL_SEMANTICS = """\ +## Tool environment — read before using any tool + +You run in-process with these tools: `bash(command)`, `edit(path, content)`, +`read(path)`, and `gh(args)` (the GitHub CLI, already authenticated with the +workspace token). + +- **Working directory does NOT persist between `bash` calls.** Each command + runs in a fresh shell rooted at the workspace directory. Do not rely on a + prior `cd`. Chain steps in one command with `&&`, or target the clone with + `git -C repo ...`. +- **Write files with the `edit` tool**, not shell redirection. `path` is + relative to the workspace root, e.g. `edit(path="repo/SECURITY.md", + content="...")`. The clone lives at `repo/`. (A heredoc into `bash` is fine + for throwaway files like the PR body under `/tmp`.) +- Do not write to paths outside the workspace. +""" + +SECURITY_MD_SYSTEM_PROMPT = ( + """\ +You are a security-posture automation agent. Your single job is to add a +`SECURITY.md` file to the target repository and open a **draft** pull request. +You do this once, end-to-end, in a single run — clone, write, commit, push, PR. + +""" + + _TOOL_SEMANTICS + + """ +## Workflow + +**Your primary goal is a draft PR that adds or updates `SECURITY.md`. Prioritise +finishing the full workflow (clone → write → commit → push → PR) over polish.** + +### 1. Clone and set up + +Build a token-authenticated clone URL at runtime (the token is in `$GH_TOKEN`), +clone shallowly, set a repo-local commit identity, and create the branch — all +without writing global/system git config. Because the working directory does +not persist, run the post-clone steps with `git -C repo` (or chain with `&&`): + +```bash +REPO_URL="" +CLONE_URL="https://x-access-token:${GH_TOKEN}@${REPO_URL#https://}" +git clone --depth 50 "$CLONE_URL" repo/ \\ + && git -C repo config --local user.email "cliff-bot@users.noreply.github.com" \\ + && git -C repo config --local user.name "Cliff Posture Bot" \\ + && git -C repo checkout -b cliff/posture/security-md +``` + +### 2. Detect whether `SECURITY.md` already exists + +Check the repo root and `.github/` for an existing file (`read` it). If it +already declares a security contact **and** a disclosure policy, return +`status="already_present"` and **do not open a PR** — skip the remaining +steps. If it exists but is a short stub (no contact, no policy), replace it +and continue. + +### 3. Write `SECURITY.md` + +Write the file at `repo/SECURITY.md` with the `edit` tool, using the markdown +below. Substitute the contact email, contact URL, and disclosure window from +the task inputs; fall back to the documented placeholders when a value is +missing — the maintainer edits them before merging. + +**Copy the markdown verbatim, including every blank line between sections.** +Blank lines are load-bearing in markdown. + +```markdown +# Security policy + +Thank you for helping keep this project and its users safe. This document +describes how to report a suspected vulnerability, which versions we +support, and how we coordinate disclosure. + +## Reporting a vulnerability + +Please report suspected security vulnerabilities privately. Do not open +a public GitHub issue for security reports. + +- **Email:** `` +- **Form:** `` + +We acknowledge new reports within 3 business days. We aim to triage and +share a remediation plan within days. + +## Supported versions + +Security fixes are backported to the branches below. Older branches receive +fixes only for high-severity issues on a best-effort basis. + +| Version | Supported | +|-----------------|--------------------| +| `main` (latest) | Yes | +| Older revisions | Best-effort only | + +## Scope + +Security reports are welcome for anything shipped by this repository, including: + +- The application source code committed to `main`. +- Packaging, containers, and deployment manifests under this repo. +- Default configuration and bundled credentials/secrets in repo artifacts. + +Out of scope: + +- Findings about third-party dependencies that do not require a change + here — please file those with the upstream project first. +- Denial-of-service that requires an attacker to already hold + administrator access to the host running this software. + +## Disclosure + +We prefer coordinated disclosure. Once a fix is available we publish a +GitHub Security Advisory and release notes describing the impact and the +upgrade path. Reporters are credited unless they request otherwise. + +## Safe harbour + +We will not pursue legal action against good-faith researchers who: + +- Follow this policy. +- Stop at the proof-of-concept stage — do not exfiltrate data, degrade + services, or pivot beyond the minimum needed to demonstrate the issue. +- Avoid data that is not their own. + +--- + +_Generated by Cliff. Edit this file to match your actual process._ +``` + +### 4. Commit, push, and open a draft PR + +Write the PR body to a file first and pass it with `--body-file` — a PR body +contains backticks and other shell meta-characters, so a single-quoted heredoc +(`<<'MD'`) is the safe way to write it. Then stage, commit, push, and open the +draft PR with `gh`: + +```bash +cat > /tmp/cliff-pr-body.md <<'MD' +## Summary + +Adds a `SECURITY.md` so researchers know how to report a vulnerability +privately, which versions you back-port fixes to, what's in scope, and +how you expect disclosure to work. + +This PR was generated by Cliff as part of the zero-to-secure posture +checks. Everything below is a starting point — edit before merging. + +## Review checklist + +- [ ] Replace the placeholder contact email with a real one you actually + monitor. +- [ ] Confirm the "Supported versions" table matches how you back-port fixes. +- [ ] Tighten or loosen the disclosure timeline. +- [ ] Tighten the "Scope" section if your out-of-scope list is different. +- [ ] Keep or drop the "Safe harbour" paragraph to match your legal posture. + +--- +Generated by Cliff posture generator. +MD +git -C repo add SECURITY.md \\ + && git -C repo commit -m "docs: add SECURITY.md (Cliff posture PR)" \\ + && git -C repo push -u origin HEAD +``` + +Then open the PR (the `gh` tool already carries the token): + +``` +gh -C repo pr create --draft --title "docs: add SECURITY.md" --body-file /tmp/cliff-pr-body.md +``` + +## Rules + +- **Draft PR only.** Never merge, never push to `main`, never force-push. +- **Single commit** on the `cliff/posture/security-md` branch. +- **Minimal changes.** Only add or update `SECURITY.md`. +- **Be fast.** `--depth 50` clone. No unrelated exploration. +- **No secrets in the PR.** Never write a live token, private key, or credential. + +## Result + +Return the structured result. When `gh pr create` printed a real PR URL, set +`status="pr_created"`, `pr_url` to that URL, and `branch_name` to the branch. +If `SECURITY.md` was already adequate, set `status="already_present"`. On any +failure, set `status="failed"` and explain in `error_details`. Set `file_path` +to `SECURITY.md`. +""" +) + +DEPENDABOT_SYSTEM_PROMPT = ( + """\ +You are a security-posture automation agent. Your single job is to add a +`.github/dependabot.yml` to the target repository, configured for the +ecosystems actually used in that repo, and open a **draft** pull request. +You do this once, end-to-end, in a single run — clone, detect, write, commit, +push, PR. + +""" + + _TOOL_SEMANTICS + + """ +## Workflow + +**Your primary goal is a draft PR that adds or updates `.github/dependabot.yml`. +Prioritise finishing the full workflow over polish.** + +### 1. Clone and branch + +```bash +REPO_URL="" +CLONE_URL="https://x-access-token:${GH_TOKEN}@${REPO_URL#https://}" +git clone --depth 50 "$CLONE_URL" repo/ \\ + && git -C repo config --local user.email "cliff-bot@users.noreply.github.com" \\ + && git -C repo config --local user.name "Cliff Posture Bot" \\ + && git -C repo checkout -b cliff/posture/dependabot +``` + +### 2. Detect ecosystems + +Scan the clone for these manifest files and record which are present (check the +repo root and common subdirectory locations such as `apps/*`, `services/*`, +`packages/*`, `frontend/`, `backend/`). Map each manifest to a Dependabot +`package-ecosystem` value: + +| Manifest | `package-ecosystem` | +|--------------------------------------------|---------------------| +| `package-lock.json` / `package.json` | `npm` | +| `yarn.lock` | `npm` | +| `pnpm-lock.yaml` | `npm` | +| `requirements.txt` | `pip` | +| `Pipfile.lock` / `Pipfile` | `pip` | +| `pyproject.toml` (with poetry/pdm) | `pip` | +| `go.mod` | `gomod` | +| `Gemfile.lock` / `Gemfile` | `bundler` | +| `Cargo.toml` | `cargo` | +| `pom.xml` | `maven` | +| `composer.json` | `composer` | +| `Dockerfile` (root or any subdir) | `docker` | +| `.github/workflows/*.yml` | `github-actions` | + +Always include a `github-actions` entry if `.github/workflows/` exists. + +### 3. Handle an existing `.github/dependabot.yml` + +- If it **already exists** with `updates:` entries for every ecosystem you + detected, return `status="already_present"` and **do not open a PR**. +- If it exists but is missing ecosystems you detected, replace it with a + merged version that keeps the existing entries and adds the missing ones, + then continue. + +### 4. Write `.github/dependabot.yml` + +Write the file at `repo/.github/dependabot.yml` with the `edit` tool, one +`updates:` block per detected ecosystem, using this shape: + +```yaml +version: 2 +updates: + - package-ecosystem: "npm" + directory: "/" + schedule: + interval: "weekly" + open-pull-requests-limit: 5 + labels: + - "dependencies" + commit-message: + prefix: "chore(deps)" + include: "scope" + groups: + minor-and-patch: + update-types: + - "minor" + - "patch" +``` + +Rules for the YAML: + +- **One block per ecosystem** you detected; if an ecosystem has manifests in + multiple directories, emit one block per directory. +- Every block sets `schedule.interval: weekly` and + `open-pull-requests-limit: 5`. +- Always add `labels: ["dependencies"]` and the `commit-message` block + (override `prefix` per ecosystem where the convention differs, e.g. + `ci(deps)` for `github-actions`, `build(deps)` for `docker`). +- Always include the `groups.minor-and-patch` block shown above. +- For `github-actions`, set `directory: "/"`. + +### 5. Commit, push, and open a draft PR + +Write the PR body to a file first (single-quoted heredoc), then commit/push: + +```bash +cat > /tmp/cliff-pr-body.md <<'MD' +## Summary + +Adds a `.github/dependabot.yml` so GitHub opens weekly update PRs for the +ecosystems detected in this repo. Minor + patch updates are grouped to keep +the review queue manageable; major updates still come through individually. + +Generated by Cliff as part of the zero-to-secure posture checks. Review the +ecosystem list and tweak the schedule or PR limit before merging. + +## Review checklist + +- [ ] Confirm every detected ecosystem block points at the right directory. +- [ ] Confirm `open-pull-requests-limit: 5` matches your review bandwidth. +- [ ] Confirm the `commit-message.prefix` matches this repo's convention. + +--- +Generated by Cliff posture generator. +MD +git -C repo add .github/dependabot.yml \\ + && git -C repo commit -m "ci: add Dependabot config (Cliff posture PR)" \\ + && git -C repo push -u origin HEAD +``` + +Then open the PR: + +``` +gh -C repo pr create --draft --title "ci: add Dependabot config" --body-file /tmp/cliff-pr-body.md +``` + +## Rules + +- **Draft PR only.** Never merge, never push to `main`, never force-push. +- **Single commit** on the `cliff/posture/dependabot` branch. +- **Minimal changes.** Only add or update `.github/dependabot.yml`. +- **Be fast.** `--depth 50` clone. Only scan for manifest files; don't read + their contents. +- **No secrets in the PR.** + +## Result + +Return the structured result: `status="pr_created"` with the real `pr_url` + +`branch_name` when a PR was opened, `already_present` when the config was +already adequate, or `failed` with `error_details`. Set `file_path` to +`.github/dependabot.yml` and `detected_ecosystems` to the list you configured. +""" +) + +_SYSTEM_PROMPTS: dict[WorkspaceKind, str] = { + WorkspaceKind.repo_action_security_md: SECURITY_MD_SYSTEM_PROMPT, + WorkspaceKind.repo_action_dependabot: DEPENDABOT_SYSTEM_PROMPT, +} + + +def build_repo_action_agent( + model: Model, + kind: WorkspaceKind, + *, + mcp_toolsets: Sequence[AbstractToolset[Any]] = (), +) -> Agent[WorkspaceDeps, RepoActionOutput]: + """Build the PA agent for a repo-action *kind*. + + Registers the same runtime tools as the remediation_executor; the run is + pre-approved via ``WorkspaceDeps.auto_approve`` (set by the runner). + """ + try: + system_prompt = _SYSTEM_PROMPTS[kind] + except KeyError: + raise ValueError(f"not a repo-action kind: {kind!r}") from None + + return Agent( + model=model, + output_type=RepoActionOutput, + deps_type=WorkspaceDeps, + system_prompt=system_prompt, + tools=[bash, edit, read, gh], + toolsets=list(mcp_toolsets), + ) + + +def build_repo_action_prompt( + kind: WorkspaceKind, *, repo_url: str, params: dict[str, Any] +) -> str: + """Render the per-run user message — the variable inputs only.""" + lines = [f"Repository: {repo_url}", ""] + if kind == WorkspaceKind.repo_action_security_md: + if params.get("contact_email"): + lines.append(f"Reported-vuln contact email: {params['contact_email']}") + if params.get("contact_url"): + lines.append(f"Reported-vuln contact URL: {params['contact_url']}") + if params.get("supported_versions"): + lines.append(f"Supported versions note: {params['supported_versions']}") + if params.get("disclosure_window_days"): + lines.append( + f"Disclosure window (days): {params['disclosure_window_days']}" + ) + lines.append("") + lines.append("Run the workflow now and return the structured result.") + return "\n".join(lines) + + +__all__ = [ + "RepoActionOutput", + "build_repo_action_agent", + "build_repo_action_prompt", +] diff --git a/backend/cliff/agents/runtime/tools/bash.py b/backend/cliff/agents/runtime/tools/bash.py index 6dbcdba4..852aa5e9 100644 --- a/backend/cliff/agents/runtime/tools/bash.py +++ b/backend/cliff/agents/runtime/tools/bash.py @@ -10,6 +10,7 @@ from __future__ import annotations import asyncio +import os import subprocess # Imported at runtime (not under TYPE_CHECKING): Pydantic AI introspects a @@ -59,6 +60,13 @@ async def bash(ctx: RunContext[WorkspaceDeps], command: str) -> str: metadata={"tool": "bash", "patterns": [command], "command": command}, ) + # Merge the workspace env *over* the process environment — never replace + # it. ``ctx.deps.env_vars`` carries only a few keys (GH_TOKEN, + # CLIFF_REPO_URL); passing it as the whole environment would strip PATH, + # HOME, etc. and break ``git`` / ``gh`` (the commands the remediation + + # repo-action agents exist to run). + run_env = {**os.environ, **(ctx.deps.env_vars or {})} + def _run() -> subprocess.CompletedProcess[str]: return subprocess.run( command, @@ -69,7 +77,7 @@ def _run() -> subprocess.CompletedProcess[str]: capture_output=True, text=True, timeout=_BASH_TIMEOUT_SECONDS, - env=ctx.deps.env_vars or None, + env=run_env, ) try: diff --git a/backend/cliff/agents/runtime/tools/permissions.py b/backend/cliff/agents/runtime/tools/permissions.py index 3c94ed79..d1d9d31c 100644 --- a/backend/cliff/agents/runtime/tools/permissions.py +++ b/backend/cliff/agents/runtime/tools/permissions.py @@ -175,13 +175,23 @@ def gate_tool_call( # caller somehow flags it approved. ModelRetry (not a raw # exception) so the model sees the denial and pivots. raise ModelRetry(_deny_message(tool, patterns)) - if tier == "ask" and not ctx.tool_call_approved: + if tier == "ask" and not ctx.tool_call_approved and not _auto_approved(ctx): raise ApprovalRequired( metadata=metadata or {"tool": tool, "patterns": list(patterns)} ) return tier +def _auto_approved(ctx: RunContext[WorkspaceDeps]) -> bool: + """Repo-action runs pre-approve the ``ask`` tier (see WorkspaceDeps). + + ``deny`` is checked before this and still hard-denies, so a pre-approved + run can do destructive-but-conceivable things (rm, git reset) but never + catastrophic ones (sudo, mkfs, curl|sh). + """ + return bool(getattr(ctx.deps, "auto_approve", False)) + + __all__ = [ "classify_tool_request", "escapes_workspace", diff --git a/backend/cliff/api/_engine_dep.py b/backend/cliff/api/_engine_dep.py index 8d7657aa..b48ec021 100644 --- a/backend/cliff/api/_engine_dep.py +++ b/backend/cliff/api/_engine_dep.py @@ -26,7 +26,6 @@ if TYPE_CHECKING: import aiosqlite - from cliff.engine.pool import WorkspaceProcessPool from cliff.models import AssessmentResult, AssessmentTool from cliff.workspace.workspace_dir_manager import WorkspaceKind @@ -35,6 +34,8 @@ StepCallback = Callable[[str], Awaitable[None]] ToolCallback = Callable[["AssessmentTool"], Awaitable[None]] +EnvResolver = Callable[[], Awaitable[dict[str, str]]] +ModelResolver = Callable[[], Awaitable[str | None]] class AssessmentEngineProtocol(Protocol): @@ -179,8 +180,17 @@ async def spawn_repo_workspace( class _DefaultRepoWorkspaceSpawner: """Production spawner backed by ``WorkspaceDirManager.create_repo_workspace``.""" - def __init__(self, pool: WorkspaceProcessPool | None) -> None: - self._pool = pool + def __init__( + self, + *, + env_resolver: EnvResolver, + model_resolver: ModelResolver, + ) -> None: + # The repo-action generator runs in-process via Pydantic AI now + # (IMPL-0022 PR #3c); these resolve the app-level AI provider env + + # active model for ``RepoAgentRunner``. + self._env_resolver = env_resolver + self._model_resolver = model_resolver async def spawn_repo_workspace( self, @@ -225,14 +235,10 @@ async def spawn_repo_workspace( shutil.rmtree(workspace_root, ignore_errors=True) raise - if self._pool is None: - logger.warning( - "repo workspace %s created without a pool — agent will not run", - workspace_id, - ) - return workspace_id - - runner = RepoAgentRunner(self._pool) + runner = RepoAgentRunner( + env_resolver=self._env_resolver, + model_resolver=self._model_resolver, + ) async def _run() -> None: try: @@ -254,6 +260,21 @@ async def _run() -> None: def get_repo_workspace_spawner(request: Request) -> RepoWorkspaceSpawnerProtocol: - """Default provider — returns the real spawner wired to the app's pool.""" - pool = getattr(request.app.state, "process_pool", None) - return _DefaultRepoWorkspaceSpawner(pool=pool) + """Default provider — wires the spawner to the app-level AI resolvers. + + Reads the canonical AI env + model from ``app.state`` at run time (the + lifespan refresh keeps these current), so a background repo-agent run + picks up a provider/model change without a restart. + """ + app = request.app + + async def _env_resolver() -> dict[str, str]: + return dict(getattr(app.state, "ai_env_cache", {}) or {}) + + async def _model_resolver() -> str | None: + return getattr(app.state, "ai_model_cache", None) + + return _DefaultRepoWorkspaceSpawner( + env_resolver=_env_resolver, + model_resolver=_model_resolver, + ) diff --git a/backend/cliff/workspace/repo_workspace_runner.py b/backend/cliff/workspace/repo_workspace_runner.py index 2e1054c8..255e4e27 100644 --- a/backend/cliff/workspace/repo_workspace_runner.py +++ b/backend/cliff/workspace/repo_workspace_runner.py @@ -35,18 +35,31 @@ from datetime import UTC, datetime from typing import TYPE_CHECKING, Any, Literal -import httpx from pydantic import BaseModel - -from cliff.agents.output_parser import parse_agent_response -from cliff.agents.template_engine import AgentTemplateEngine +from pydantic_ai.exceptions import ( + ModelHTTPError, + UnexpectedModelBehavior, + UsageLimitExceeded, + UserError, +) + +from cliff.agents.runtime.deps import WorkspaceDeps +from cliff.agents.runtime.provider import ProviderConfigurationError, build_model +from cliff.agents.runtime.repo_actions import ( + RepoActionOutput, + build_repo_action_agent, + build_repo_action_prompt, +) from cliff.services.pr_verifier import verify_pr_url -from cliff.workspace.workspace_dir_manager import WorkspaceKind if TYPE_CHECKING: + from collections.abc import Awaitable, Callable from pathlib import Path - from cliff.engine.pool import WorkspaceProcessPool + from cliff.workspace.workspace_dir_manager import WorkspaceKind + + EnvResolver = Callable[[], Awaitable[dict[str, str]]] + ModelResolver = Callable[[], Awaitable[str | None]] logger = logging.getLogger(__name__) @@ -162,8 +175,17 @@ class RepoAgentRunner: run and discard themselves when it's done. """ - def __init__(self, pool: WorkspaceProcessPool) -> None: - self._pool = pool + def __init__( + self, + *, + env_resolver: EnvResolver, + model_resolver: ModelResolver, + ) -> None: + # Resolve the app-level AI provider env + active model at run time + # (ADR-0047 / IMPL-0022 PR #3c) — the generator is now an in-process + # Pydantic AI agent, not an OpenCode subprocess. + self._env_resolver = env_resolver + self._model_resolver = model_resolver async def run( self, @@ -191,137 +213,122 @@ async def run( _write_status(workspace_root, running) await _sync_workspace_state(workspace_id, "running") + # Build the model from the canonical AI state. try: - prompt = _render_prompt( - kind, repo_url=repo_url, gh_token=gh_token, params=params or {} - ) - except Exception as exc: # noqa: BLE001 — never leak to caller + ai_env = await self._env_resolver() + model_id = await self._model_resolver() + model = build_model(ai_env, model_id) + except ProviderConfigurationError as exc: return await self._finalize( workspace_root, running, status="failed", - error=f"Failed to render agent prompt: {exc}", + error=f"AI provider not configured: {exc}", ) + # Tool subprocess env: the GitHub token + repo URL. ``bash`` merges + # this over ``os.environ`` so git/gh keep PATH etc. env_vars: dict[str, str] = {"CLIFF_REPO_URL": repo_url} if gh_token: env_vars["GH_TOKEN"] = gh_token - # Some gh commands prefer GITHUB_TOKEN; export both. env_vars["GITHUB_TOKEN"] = gh_token - client = None + deps = WorkspaceDeps( + workspace_id=workspace_id, + workspace_dir=str(workspace_root), + finding={}, + env_vars=env_vars, + auto_approve=True, # one-shot, pre-approved — no HITL surface + ) + agent = build_repo_action_agent(model, kind) + user_prompt = build_repo_action_prompt( + kind, repo_url=repo_url, params=params or {} + ) + try: - client = await self._pool.get_or_start( - workspace_id, workspace_root, env_vars=env_vars - ) - session = await client.create_session() - response_text = await _send_and_collect( - client, session.id, prompt, timeout=timeout + result = await asyncio.wait_for( + agent.run(user_prompt, deps=deps), timeout=timeout ) - except (RuntimeError, TimeoutError, httpx.HTTPError) as exc: - logger.exception("repo agent process failed for %s", workspace_id) + except TimeoutError: + logger.warning("repo agent %s timed out after %ss", workspace_id, timeout) return await self._finalize( workspace_root, running, status="failed", - error=f"OpenCode process failure: {exc}", - ) - except Exception as exc: # noqa: BLE001 - logger.exception( - "unexpected error in repo agent runner for %s", workspace_id + error=f"Agent timed out after {timeout:.0f}s", ) + except ( + ModelHTTPError, + UnexpectedModelBehavior, + UsageLimitExceeded, + UserError, + ) as exc: + logger.warning("repo agent %s run failed: %s", workspace_id, exc) return await self._finalize( workspace_root, running, status="failed", - error=f"Unexpected error: {exc}", + error=f"Agent run failed: {type(exc).__name__}: {exc}", ) - finally: - # Repo workspaces are one-shot: release the process + port so we - # don't leak a subprocess per "Generate and open PR" click. - with contextlib.suppress(Exception): - await self._pool.stop(workspace_id) - - # Persist raw text so operators can see exactly what the agent said - # when something goes wrong — especially helpful for "No JSON block - # found" because the UI otherwise can't show the reasoning that - # happened before the stall. - with contextlib.suppress(Exception): - (workspace_root / "history" / "agent-response.txt").write_text( - response_text + except Exception as exc: # noqa: BLE001 — never leak to caller + logger.exception( + "unexpected error in repo agent runner for %s", workspace_id ) - - log_tail = _tail(response_text) - - if not response_text.strip(): return await self._finalize( workspace_root, running, status="failed", - error="Agent returned an empty response", - agent_log_tail=log_tail, + error=f"Unexpected error: {exc}", ) - parsed = parse_agent_response(response_text, agent_type=_agent_type_for(kind)) - structured = parsed.structured_output or {} + output: RepoActionOutput = result.output + structured = output.model_dump() - if not parsed.success: - return await self._finalize( - workspace_root, - running, - status="failed", - error=parsed.error or "Agent output did not match contract", - structured_output=structured or None, - agent_log_tail=log_tail, + # Persist the full message transcript for operator diagnostics — + # the analogue of the old SSE-text dump. + with contextlib.suppress(Exception): + (workspace_root / "history" / "agent-response.txt").write_bytes( + result.all_messages_json() ) + log_tail = _tail(output.result_card_markdown or output.summary or "") - agent_status = (structured.get("status") or "").lower() - pr_url = structured.get("pr_url") - branch_name = structured.get("branch_name") - - if agent_status == "pr_created": + if output.status == "pr_created": # B16: the agent cannot be trusted to emit a real PR URL. Hit - # GitHub's API before we tell the user "PR opened" — a mismatch - # is the symptom we're fixing. - verification = await verify_pr_url(pr_url, token=gh_token) + # GitHub's API before we tell the user "PR opened". + verification = await verify_pr_url(output.pr_url, token=gh_token) if verification.ok: return await self._finalize( workspace_root, running, status="pr_created", - pr_url=verification.html_url or pr_url, - branch_name=branch_name, + pr_url=verification.html_url or output.pr_url, + branch_name=output.branch_name, structured_output=structured, ) - # Verification failed — no fallback. Surface the real reason + - # a tail of the agent's response so the user can tell whether - # a branch was actually pushed or the model invented the URL. return await self._finalize( workspace_root, running, status="failed", error=( - "PR verification failed: " - f"{verification.reason}. Agent claimed pr_url=" - f"{pr_url!r}." + f"PR verification failed: {verification.reason}. " + f"Agent claimed pr_url={output.pr_url!r}." ), structured_output=structured, agent_log_tail=log_tail, ) - if agent_status == "already_present": + if output.status == "already_present": return await self._finalize( workspace_root, running, status="already_present", structured_output=structured, ) - # Fall through: the agent claimed success but didn't give us a PR. + # status == "failed" (or anything else): surface the agent's reason. return await self._finalize( workspace_root, running, status="failed", - error=structured.get("error_details") - or "Agent finished without opening a PR", + error=output.error_details or "Agent finished without opening a PR", structured_output=structured, agent_log_tail=log_tail, ) @@ -385,102 +392,3 @@ def _tail(text: str) -> str | None: if len(text) <= _LOG_TAIL_CHARS: return text return "…(truncated)…\n" + text[-_LOG_TAIL_CHARS:] - - -def _render_prompt( - kind: WorkspaceKind, - *, - repo_url: str, - gh_token: str | None, - params: dict[str, Any], -) -> str: - """Re-render the generator prompt (idempotent with WorkspaceDirManager).""" - engine = AgentTemplateEngine() - rendered = engine.render_repo_action( - kind, repo_url=repo_url, params=params, gh_token=gh_token - ) - return rendered.content - - -def _agent_type_for(kind: WorkspaceKind) -> str: - """Map workspace kind to the agent_type string the parser expects. - - The output parser uses agent_type only for ``validate_structured_output`` - schema lookup; posture generator schemas are registered under these names - in ``cliff.agents.schemas``. Unknown agent_type is allowed — the parser - skips per-agent validation and still extracts ``structured_output``. - """ - return { - WorkspaceKind.repo_action_security_md: "security_md_generator", - WorkspaceKind.repo_action_dependabot: "dependabot_config_generator", - }.get(kind, kind.value) - - -async def _send_and_collect( - client: Any, - session_id: str, - prompt: str, - *, - timeout: float, -) -> str: - """Send the prompt, accumulate text events until done/stall/timeout. - - Simpler than ``AgentExecutor._send_and_collect`` because repo workspaces - pre-approve bash/edit/webfetch — we never see ``permission_request`` - events and therefore don't need an approval queue. - """ - - async def _send() -> None: - # Small delay matches the executor's pattern: give the SSE stream - # time to connect before the message lands. - await asyncio.sleep(0.5) - with contextlib.suppress(httpx.ReadTimeout): - await client.send_message(session_id, prompt) - - send_task = asyncio.create_task(_send()) - try: - return await _collect_response(client, session_id, timeout=timeout) - finally: - if not send_task.done(): - send_task.cancel() - with contextlib.suppress(asyncio.CancelledError): - await send_task - - -async def _collect_response( - client: Any, - session_id: str, - *, - timeout: float, -) -> str: - """Consume the SSE event stream until ``done`` or the overall timeout. - - Text events from OpenCode carry the cumulative response so far — each - successive event replaces the prior content, and the last event before - ``done`` has the full text. We keep the latest and return it either - when we see ``done`` or when the outer timeout wins. - """ - collected = "" - - async def _stream() -> str: - nonlocal collected - async for event in client.stream_events(session_id): - etype = event.get("type") - if etype == "text": - collected = event.get("content", "") or collected - elif etype == "done": - return collected - elif etype == "error": - raise RuntimeError( - f"OpenCode error event: {event.get('message', 'unknown')}" - ) - return collected - - try: - return await asyncio.wait_for(_stream(), timeout=timeout) - except TimeoutError: - # Outer timeout — surface whatever we collected so the caller still - # gets diagnostic text in ``history/agent-response.txt`` instead of - # a blanket "Unexpected error". - logger.warning("repo agent response stream timed out after %ss", timeout) - return collected diff --git a/backend/tests/agents/tools/test_permissions.py b/backend/tests/agents/tools/test_permissions.py index 7dfe4986..8dbe8d8b 100644 --- a/backend/tests/agents/tools/test_permissions.py +++ b/backend/tests/agents/tools/test_permissions.py @@ -79,9 +79,12 @@ def test_empty_bash_patterns_is_ask(self): assert classify_tool_request("bash", []) == "ask" -def _ctx(*, approved: bool = False) -> SimpleNamespace: - """Minimal RunContext stand-in — gate only reads tool_call_approved.""" - return SimpleNamespace(tool_call_approved=approved, deps=None) +def _ctx(*, approved: bool = False, auto_approve: bool = False) -> SimpleNamespace: + """Minimal RunContext stand-in — gate reads tool_call_approved + deps.""" + return SimpleNamespace( + tool_call_approved=approved, + deps=SimpleNamespace(auto_approve=auto_approve), + ) class TestGateToolCall: @@ -117,3 +120,21 @@ def test_custom_metadata_attached_to_approval(self): metadata={"tool": "bash", "command": "rm -rf x"}, ) assert exc_info.value.metadata["command"] == "rm -rf x" + + def test_auto_approve_proceeds_on_ask_tier(self): + """Repo-action runs (auto_approve) skip the approval prompt on the + ask tier — there's no HITL surface for a one-shot background run.""" + assert ( + gate_tool_call( + _ctx(auto_approve=True), tool="bash", patterns=["rm -rf build/"] + ) + == "ask" + ) + + def test_auto_approve_still_denies_catastrophic(self): + """auto_approve never lifts the deny tier — catastrophic commands + stay blocked even for a pre-approved repo-action run.""" + with pytest.raises(ModelRetry, match="Cliff safety policy"): + gate_tool_call( + _ctx(auto_approve=True), tool="bash", patterns=["sudo rm -rf /"] + ) diff --git a/backend/tests/test_repo_workspace_runner.py b/backend/tests/test_repo_workspace_runner.py index f9765f1e..3dda386d 100644 --- a/backend/tests/test_repo_workspace_runner.py +++ b/backend/tests/test_repo_workspace_runner.py @@ -1,88 +1,69 @@ """Unit tests for RepoAgentRunner — focused on the B16 PR-URL guardrail. -The runner is non-raising by contract: every bad outcome (parse failure, GH +The runner is non-raising by contract: every bad outcome (model error, GH 404, hallucinated URL) collapses to a ``RepoAgentStatus(status="failed")`` -row persisted to disk. These tests drive a fake OpenCode client + a fake -``verify_pr_url`` (via monkeypatch) to exercise each branch without spinning -up a real process pool. +row persisted to disk. These tests drive the repo-action agent with a +``FunctionModel`` (no live LLM, no tool calls) and a fake ``verify_pr_url`` +(via monkeypatch) to exercise each branch. """ from __future__ import annotations -import json from typing import TYPE_CHECKING, Any +from unittest.mock import AsyncMock import pytest +from pydantic_ai.messages import ModelResponse, ToolCallPart +from pydantic_ai.models.function import AgentInfo, FunctionModel from cliff.services.pr_verifier import PRVerification from cliff.workspace import repo_workspace_runner -from cliff.workspace.repo_workspace_runner import ( - RepoAgentRunner, - read_status, -) +from cliff.workspace.repo_workspace_runner import RepoAgentRunner, read_status from cliff.workspace.workspace_dir_manager import WorkspaceKind if TYPE_CHECKING: from pathlib import Path -class _FakeClient: - def __init__(self, response_text: str) -> None: - self._response_text = response_text +def _model_returning(output: dict[str, Any]) -> FunctionModel: + """A FunctionModel that returns *output* as the agent's structured result + in one turn (no tool calls).""" - async def create_session(self) -> Any: # noqa: D401 - class _S: - id = "sess-1" + def _fn(messages, info: AgentInfo) -> ModelResponse: + tool_name = info.output_tools[0].name # 'final_result' + return ModelResponse(parts=[ToolCallPart(tool_name=tool_name, args=output)]) - return _S() + return FunctionModel(_fn) - async def send_message(self, session_id: str, prompt: str) -> None: # noqa: ARG002 - return None - async def stream_events(self, session_id: str): # noqa: ARG002 - yield {"type": "text", "content": self._response_text} - yield {"type": "done"} - - -class _FakePool: - def __init__(self, client: _FakeClient) -> None: - self._client = client - self.stopped: list[str] = [] +def _build_runner() -> RepoAgentRunner: + return RepoAgentRunner( + env_resolver=AsyncMock(return_value={"OPENAI_API_KEY": "k"}), + model_resolver=AsyncMock(return_value="openai/gpt-4o-mini"), + ) - async def get_or_start( - self, - workspace_id: str, # noqa: ARG002 - workspace_root: Path, # noqa: ARG002 - env_vars: dict[str, str] | None = None, # noqa: ARG002 - ) -> _FakeClient: - return self._client - async def stop(self, workspace_id: str) -> None: - self.stopped.append(workspace_id) +def _patch_model(monkeypatch: pytest.MonkeyPatch, model: FunctionModel) -> None: + monkeypatch.setattr( + repo_workspace_runner, "build_model", lambda *_a, **_k: model + ) def _scaffold_workspace(tmp_path: Path) -> Path: - """Create the minimal directory layout the runner expects.""" root = tmp_path / "ws-test" (root / "history").mkdir(parents=True) return root -def _agent_response(pr_url: str | None) -> str: - payload = { +def _pr_output(pr_url: str | None) -> dict[str, Any]: + return { + "status": "pr_created", + "pr_url": pr_url, + "branch_name": "cliff/posture/security-md", + "file_path": "SECURITY.md", "summary": "wrote SECURITY.md and opened PR", - "confidence": 0.9, - "result_card_markdown": "## ok", - "evidence_sources": [], - "suggested_next_action": "review_pr", - "structured_output": { - "status": "pr_created", - "pr_url": pr_url, - "branch_name": "cliff/add-security-md", - "file_path": "SECURITY.md", - }, + "result_card_markdown": "## SECURITY.md PR\n\nBranch pushed, PR opened.", } - return "```json\n" + json.dumps(payload) + "\n```" @pytest.mark.asyncio @@ -90,22 +71,18 @@ async def test_run_verifies_pr_url_and_succeeds( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: workspace_root = _scaffold_workspace(tmp_path) - pool = _FakePool( - _FakeClient(_agent_response("https://github.com/acme/repo/pull/12")) + _patch_model( + monkeypatch, + _model_returning(_pr_output("https://github.com/acme/repo/pull/12")), ) async def _fake_verify(url: str | None, **_: Any) -> PRVerification: assert url == "https://github.com/acme/repo/pull/12" - return PRVerification( - ok=True, - reason="verified", - pr_state="open", - html_url=url, - ) + return PRVerification(ok=True, reason="verified", pr_state="open", html_url=url) monkeypatch.setattr(repo_workspace_runner, "verify_pr_url", _fake_verify) - runner = RepoAgentRunner(pool) # type: ignore[arg-type] + runner = _build_runner() result = await runner.run( workspace_id="ws-test", workspace_root=workspace_root, @@ -126,22 +103,20 @@ async def _fake_verify(url: str | None, **_: Any) -> PRVerification: async def test_run_flags_hallucinated_pr_url_as_failed( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: - """B16 regression: verifier says 404 -> status=failed, log tail preserved.""" + """B16 regression: verifier says 404 -> status=failed, diagnostics preserved.""" workspace_root = _scaffold_workspace(tmp_path) - # The agent's claim — a plausibly-shaped URL that doesn't resolve. hallucinated = "https://github.com/acme/repo/pull/999" - pool = _FakePool(_FakeClient(_agent_response(hallucinated))) + _patch_model(monkeypatch, _model_returning(_pr_output(hallucinated))) async def _fake_verify(url: str | None, **_: Any) -> PRVerification: assert url == hallucinated return PRVerification( - ok=False, - reason="not_found: GitHub returned 404 for this pull request", + ok=False, reason="not_found: GitHub returned 404 for this pull request" ) monkeypatch.setattr(repo_workspace_runner, "verify_pr_url", _fake_verify) - runner = RepoAgentRunner(pool) # type: ignore[arg-type] + runner = _build_runner() result = await runner.run( workspace_id="ws-test", workspace_root=workspace_root, @@ -154,37 +129,31 @@ async def _fake_verify(url: str | None, **_: Any) -> PRVerification: assert result.pr_url is None assert "PR verification failed" in (result.error or "") assert "not_found" in (result.error or "") - # B16 also requires surfacing the agent's own log so the user can see - # whether a branch was actually pushed. + # The agent's result card is surfaced as the log tail for the UI. assert result.agent_log_tail is not None - assert "pr_created" in result.agent_log_tail # JSON payload preserved. - # Agent-response file should also be on disk for deeper inspection. + assert "SECURITY.md PR" in result.agent_log_tail + # Full transcript also persisted for deeper inspection. assert (workspace_root / "history" / "agent-response.txt").is_file() @pytest.mark.asyncio -async def test_run_flags_compare_page_without_hitting_network( +async def test_run_flags_compare_page_url_as_failed( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: - """A malformed URL never reaches GitHub — the parser rejects it first.""" + """A compare-page URL is rejected by the verifier (status=failed).""" workspace_root = _scaffold_workspace(tmp_path) - # Compare-page URL the agent has emitted in real dogfooding runs. fake_url = "https://github.com/acme/repo/pull/new/cliff-fix" - pool = _FakePool(_FakeClient(_agent_response(fake_url))) + _patch_model(monkeypatch, _model_returning(_pr_output(fake_url))) calls = {"n": 0} async def _counting_verify(url: str | None, **_: Any) -> PRVerification: calls["n"] += 1 - # The real verifier returns not_a_pull_url without doing I/O for - # this URL shape — we mimic that behaviour here. return PRVerification(ok=False, reason=f"not_a_pull_url: {url!r}") - monkeypatch.setattr( - repo_workspace_runner, "verify_pr_url", _counting_verify - ) + monkeypatch.setattr(repo_workspace_runner, "verify_pr_url", _counting_verify) - runner = RepoAgentRunner(pool) # type: ignore[arg-type] + runner = _build_runner() result = await runner.run( workspace_id="ws-test", workspace_root=workspace_root, @@ -196,3 +165,51 @@ async def _counting_verify(url: str | None, **_: Any) -> PRVerification: assert result.status == "failed" assert "not_a_pull_url" in (result.error or "") assert calls["n"] == 1 + + +@pytest.mark.asyncio +async def test_run_already_present_skips_pr( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """An ``already_present`` result is terminal and opens no PR.""" + workspace_root = _scaffold_workspace(tmp_path) + _patch_model( + monkeypatch, + _model_returning( + {"status": "already_present", "file_path": "SECURITY.md"} + ), + ) + + verify = AsyncMock() + monkeypatch.setattr(repo_workspace_runner, "verify_pr_url", verify) + + runner = _build_runner() + result = await runner.run( + workspace_id="ws-test", + workspace_root=workspace_root, + kind=WorkspaceKind.repo_action_security_md, + repo_url="https://github.com/acme/repo", + gh_token="ghp_x", + ) + + assert result.status == "already_present" + verify.assert_not_called() + + +@pytest.mark.asyncio +async def test_run_unconfigured_model_fails_gracefully(tmp_path: Path) -> None: + """No active model -> status=failed, never raises.""" + workspace_root = _scaffold_workspace(tmp_path) + runner = RepoAgentRunner( + env_resolver=AsyncMock(return_value={}), + model_resolver=AsyncMock(return_value=None), + ) + result = await runner.run( + workspace_id="ws-test", + workspace_root=workspace_root, + kind=WorkspaceKind.repo_action_security_md, + repo_url="https://github.com/acme/repo", + gh_token="ghp_x", + ) + assert result.status == "failed" + assert "AI provider not configured" in (result.error or "") diff --git a/backend/tests/test_repo_workspace_spawner.py b/backend/tests/test_repo_workspace_spawner.py index 5739a5ab..d4bc3621 100644 --- a/backend/tests/test_repo_workspace_spawner.py +++ b/backend/tests/test_repo_workspace_spawner.py @@ -9,6 +9,7 @@ from __future__ import annotations from typing import TYPE_CHECKING +from unittest.mock import AsyncMock import aiosqlite import pytest @@ -22,6 +23,26 @@ from pathlib import Path +def _spawner() -> _DefaultRepoWorkspaceSpawner: + """A spawner with stub AI resolvers — these tests exercise the + filesystem + DB scaffolding, not the (background) agent run, which the + autouse fixture below neutralizes.""" + return _DefaultRepoWorkspaceSpawner( + env_resolver=AsyncMock(return_value={}), + model_resolver=AsyncMock(return_value=None), + ) + + +@pytest.fixture(autouse=True) +def _no_background_agent(monkeypatch): + """Stop the spawner's fire-and-forget ``RepoAgentRunner.run`` from racing + these scaffolding assertions (it would flip ``workspace.state``).""" + monkeypatch.setattr( + "cliff.workspace.repo_workspace_runner.RepoAgentRunner.run", + AsyncMock(return_value=None), + ) + + @pytest.fixture async def db(tmp_path: Path, monkeypatch): conn = await init_db(":memory:") @@ -35,7 +56,7 @@ async def db(tmp_path: Path, monkeypatch): async def test_spawner_inserts_workspace_row(db: aiosqlite.Connection) -> None: - spawner = _DefaultRepoWorkspaceSpawner(pool=None) + spawner = _spawner() workspace_id = await spawner.spawn_repo_workspace( kind=WorkspaceKind.repo_action_security_md, @@ -61,7 +82,7 @@ async def test_spawner_inserts_workspace_row(db: aiosqlite.Connection) -> None: async def test_spawner_row_visible_via_dao(db: aiosqlite.Connection) -> None: - spawner = _DefaultRepoWorkspaceSpawner(pool=None) + spawner = _spawner() workspace_id = await spawner.spawn_repo_workspace( kind=WorkspaceKind.repo_action_dependabot, repo_url="https://github.com/acme/widget", @@ -82,7 +103,7 @@ async def test_spawner_collision_raises_integrity_error( same check. The spawner propagates the IntegrityError so the route can turn it into a 409. """ - spawner = _DefaultRepoWorkspaceSpawner(pool=None) + spawner = _spawner() await spawner.spawn_repo_workspace( kind=WorkspaceKind.repo_action_security_md, @@ -108,7 +129,7 @@ async def test_runner_finalize_releases_partial_index( """ from cliff.db.repo_workspace import set_workspace_state - spawner = _DefaultRepoWorkspaceSpawner(pool=None) + spawner = _spawner() first = await spawner.spawn_repo_workspace( kind=WorkspaceKind.repo_action_security_md, repo_url="https://github.com/acme/widget", From 2aaaff2cd7242bc1da2fc257d88a41ec30e610d6 Mon Sep 17 00:00:00 2001 From: Gal Ankonina Date: Tue, 2 Jun 2026 13:24:43 +0300 Subject: [PATCH 03/10] refactor(#3d): remove the vestigial workspace-scoped chat/sessions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First slice of the substrate removal. The workspace-scoped chat/sessions routes were the last per-workspace-pool consumer, but they're dead weight post-migration: the PA pipeline runs agents in-process (it never used the OpenCode session), the frontend client defined the calls but no component invoked them, and `cliffsec fix` created a session as a leftover no-op. - delete the 4 routes from api/routes/workspaces.py (sessions, chat/send, chat/stream, chat/permission) + the pool-status debug route + _get_pool; drop the pool-stop from delete_workspace - drop the no-op `POST /sessions` step from `cliffsec fix` - remove createWorkspaceSession / sendWorkspaceMessage / streamWorkspaceEvents / respondToChatPermission from the frontend client; tidy the stale IssueSidePanel comment Nothing else drives the pool now — the singleton + pool come out next, once the remaining singleton consumers (health, settings, ai/service, lifespan) are rewired. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/cliff/api/routes/workspaces.py | 191 +----------------- cli/cliff_cli/cli.py | 10 +- frontend/src/api/client.ts | 23 --- .../src/components/issues/IssueSidePanel.tsx | 3 +- 4 files changed, 5 insertions(+), 222 deletions(-) diff --git a/backend/cliff/api/routes/workspaces.py b/backend/cliff/api/routes/workspaces.py index 5f096bf1..ce744060 100644 --- a/backend/cliff/api/routes/workspaces.py +++ b/backend/cliff/api/routes/workspaces.py @@ -8,10 +8,7 @@ from typing import TYPE_CHECKING from fastapi import APIRouter, Depends, HTTPException, Request, Response -from pydantic import BaseModel -from sse_starlette.sse import EventSourceResponse -from cliff.api.tasks import fire_and_forget_send from cliff.db.connection import get_db from cliff.db.repo_finding import ( get_finding, @@ -29,7 +26,6 @@ if TYPE_CHECKING: import aiosqlite - from cliff.engine.pool import WorkspaceProcessPool from cliff.workspace.context_builder import WorkspaceContextBuilder logger = logging.getLogger(__name__) @@ -42,10 +38,6 @@ # --------------------------------------------------------------------------- -def _get_pool(request: Request) -> WorkspaceProcessPool: - return request.app.state.process_pool - - def _get_context_builder(request: Request) -> WorkspaceContextBuilder: return request.app.state.context_builder @@ -240,183 +232,13 @@ async def update_workspace_endpoint( async def delete_workspace_endpoint( workspace_id: str, request: Request, db=Depends(get_db) ): - """Delete workspace: stop process, remove directory, delete DB row.""" - pool = _get_pool(request) - await pool.stop(workspace_id) - + """Delete workspace: remove directory + DB row.""" context_builder = _get_context_builder(request) deleted = await context_builder.delete_workspace(db, workspace_id) if not deleted: raise HTTPException(status_code=404, detail="Workspace not found") -# --------------------------------------------------------------------------- -# Workspace-scoped sessions -# --------------------------------------------------------------------------- - - -@router.post("/workspaces/{workspace_id}/sessions") -async def create_workspace_session( - workspace_id: str, request: Request, db=Depends(get_db) -): - """Create an OpenCode session on this workspace's isolated process.""" - workspace = await _get_workspace_or_404(db, workspace_id) - if not workspace.workspace_dir: - raise HTTPException(status_code=409, detail="Workspace has no directory") - - pool = _get_pool(request) - env_vars = await _resolve_repo_env_vars(request, db, workspace=workspace) - client = await pool.get_or_start( - workspace_id, Path(workspace.workspace_dir), env_vars=env_vars - ) - session = await client.create_session() - return {"session_id": session.id, "workspace_id": workspace_id} - - -# --------------------------------------------------------------------------- -# Workspace-scoped chat -# --------------------------------------------------------------------------- - - -class WorkspaceChatRequest(BaseModel): - session_id: str - content: str - - -@router.post("/workspaces/{workspace_id}/chat/send") -async def workspace_send_message( - workspace_id: str, - body: WorkspaceChatRequest, - request: Request, - db=Depends(get_db), -): - """Send a message to this workspace's OpenCode process.""" - workspace = await _get_workspace_or_404(db, workspace_id) - if not workspace.workspace_dir: - raise HTTPException(status_code=409, detail="Workspace has no directory") - - pool = _get_pool(request) - env_vars = await _resolve_repo_env_vars(request, db, workspace=workspace) - client = await pool.get_or_start( - workspace_id, Path(workspace.workspace_dir), env_vars=env_vars - ) - - fire_and_forget_send(client.send_message(body.session_id, body.content)) - return {"session_id": body.session_id, "status": "sent"} - - -@router.get("/workspaces/{workspace_id}/chat/stream") -async def workspace_stream_events( - workspace_id: str, - session_id: str, - request: Request, - db=Depends(get_db), -): - """Stream SSE events from this workspace's OpenCode process.""" - workspace = await _get_workspace_or_404(db, workspace_id) - if not workspace.workspace_dir: - raise HTTPException(status_code=409, detail="Workspace has no directory") - - pool = _get_pool(request) - env_vars = await _resolve_repo_env_vars(request, db, workspace=workspace) - client = await pool.get_or_start( - workspace_id, Path(workspace.workspace_dir), env_vars=env_vars - ) - - async def event_generator(): - try: - async for event in client.stream_events(session_id): - event_type = event.get("type", "message") - if event_type == "text": - yield {"event": "text", "data": event.get("content", "")} - elif event_type == "error": - yield { - "event": "error", - "data": json.dumps( - {"message": event.get("message", "Unknown error")} - ), - } - elif event_type == "permission_request": - yield { - "event": "permission_request", - "data": json.dumps({ - "id": event.get("id", ""), - "tool": event.get("tool", "unknown"), - "patterns": event.get("patterns", []), - "session_id": session_id, - }), - } - elif event_type == "done": - yield {"event": "done", "data": "{}"} - return - except Exception: - logger.exception( - "Error streaming for workspace %s session %s", - workspace_id, - session_id, - ) - yield { - "event": "error", - "data": json.dumps({"message": "Stream disconnected"}), - } - - return EventSourceResponse(event_generator()) - - -# --------------------------------------------------------------------------- -# Workspace-level permission approval (chat path) -# --------------------------------------------------------------------------- - - -class ChatPermissionDecision(BaseModel): - permission_id: str - session_id: str - approved: bool - - -@router.post("/workspaces/{workspace_id}/chat/permission") -async def respond_to_chat_permission( - workspace_id: str, - body: ChatPermissionDecision, - request: Request, - db=Depends(get_db), -): - """Approve or deny a permission request from the chat path. - - Unlike the agent-execution permission endpoint, this calls - OpenCode's permission API directly (no executor involved). - """ - workspace = await _get_workspace_or_404(db, workspace_id) - if not workspace.workspace_dir: - raise HTTPException(status_code=409, detail="Workspace has no directory") - - pool = _get_pool(request) - env_vars = await _resolve_repo_env_vars(request, db, workspace=workspace) - client = await pool.get_or_start( - workspace_id, Path(workspace.workspace_dir), env_vars=env_vars - ) - - try: - if body.approved: - await client.grant_permission( - body.permission_id, session_id=body.session_id, - ) - else: - await client.deny_permission( - body.permission_id, session_id=body.session_id, - ) - except Exception as exc: - raise HTTPException( - status_code=502, - detail=f"Failed to send permission decision to OpenCode: {exc}", - ) from exc - - return { - "status": "approved" if body.approved else "denied", - "permission_id": body.permission_id, - } - - # --------------------------------------------------------------------------- # Workspace context # --------------------------------------------------------------------------- @@ -434,17 +256,6 @@ async def get_workspace_context(workspace_id: str, request: Request): ) -@router.get("/workspaces/{workspace_id}/pool-status") -async def workspace_pool_status(workspace_id: str, request: Request): - """Debug endpoint: show process pool status for this workspace.""" - pool = _get_pool(request) - full_status = pool.status() - ws_status = full_status["workspaces"].get(workspace_id) - if ws_status is None: - return {"workspace_id": workspace_id, "process_running": False} - return {"workspace_id": workspace_id, "process_running": True, **ws_status} - - @router.get("/workspaces/{workspace_id}/integrations") async def get_workspace_integrations( workspace_id: str, request: Request, db=Depends(get_db) diff --git a/cli/cliff_cli/cli.py b/cli/cliff_cli/cli.py index 2056581b..7def863f 100644 --- a/cli/cliff_cli/cli.py +++ b/cli/cliff_cli/cli.py @@ -314,13 +314,9 @@ def fix(client: Client, issue_id: str, timeout: float) -> None: ) workspace_id = ws["id"] - # Make sure a session exists for the workspace before running the - # pipeline. The session endpoint is idempotent enough for our purposes. - import contextlib - - with contextlib.suppress(HTTPError): - client.post(f"/api/workspaces/{workspace_id}/sessions", json={}) - + # The pipeline runs its agents in-process via Pydantic AI — no OpenCode + # session to pre-create (the old POST /sessions step was a no-op since + # the substrate migration; ADR-0047). client.post(f"/api/workspaces/{workspace_id}/pipeline/run-all") # Poll the sidebar until either: diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 4bf05827..f42e9126 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -499,22 +499,6 @@ export const api = { // Health health: () => request('/health'), - // Workspace-scoped sessions (isolated per-workspace OpenCode process) - createWorkspaceSession: (workspaceId: string) => - request<{ session_id: string; workspace_id: string }>( - `/api/workspaces/${workspaceId}/sessions`, - { method: 'POST' }, - ), - - // Workspace-scoped chat (isolated per-workspace OpenCode process) - sendWorkspaceMessage: (workspaceId: string, sessionId: string, content: string) => - request<{ session_id: string; status: string }>( - `/api/workspaces/${workspaceId}/chat/send`, - { method: 'POST', body: JSON.stringify({ session_id: sessionId, content }) }, - ), - streamWorkspaceEvents: (workspaceId: string, sessionId: string): EventSource => - new EventSource(`/api/workspaces/${workspaceId}/chat/stream?session_id=${sessionId}`), - // Findings listFindings: (params?: { status?: string; @@ -759,13 +743,6 @@ export const api = { { method: 'POST', body: JSON.stringify({ approved }) }, ), - // Permission approval (chat path — calls OpenCode directly) - respondToChatPermission: (workspaceId: string, permissionId: string, sessionId: string, approved: boolean) => - request<{ status: string; permission_id: string }>( - `/api/workspaces/${workspaceId}/chat/permission`, - { method: 'POST', body: JSON.stringify({ permission_id: permissionId, session_id: sessionId, approved }) }, - ), - // Completion share-action recording (EXEC-0002 / IMPL-0002 H5). // Frozen contract: POST /api/completion/{id}/share-action returns HTTP 200 // with { completion_id, share_actions_used }. Frontend treats it as diff --git a/frontend/src/components/issues/IssueSidePanel.tsx b/frontend/src/components/issues/IssueSidePanel.tsx index a1208415..92489902 100644 --- a/frontend/src/components/issues/IssueSidePanel.tsx +++ b/frontend/src/components/issues/IssueSidePanel.tsx @@ -180,8 +180,7 @@ export function IssueSidePanel({ // agent_run rows — both already polled. With the executor now parking a // durable DeferredToolRequests marker on the row, the poll surfaces the // approval prompt and run-status transitions within one interval, so no - // push channel is needed. (The chat SSE — api.streamWorkspaceEvents — is - // unrelated and stays.) + // push channel is needed. return (