feat(agents): migrate normalizer + repo-actions to Pydantic AI, start substrate removal (PR #3b–#3d)#253
Merged
Merged
Conversation
…el agent 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) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 10 minutes and 59 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (19)
📝 WalkthroughWalkthroughMigrates OpenCode JSON-parsing flows to in-process Pydantic AI agents: adds a NormalizedFinding model and agent factory, rewrites normalize_findings to call a Pydantic AI agent (env/model required), injects env/model resolvers into ingest and repo-runner paths, adds repo-action agents, updates tools/permissions/health/API wiring, and revises tests and OpenAPI snapshot. ChangesPydantic AI Normalizer Agent Migration
Sequence Diagram(s)sequenceDiagram
participant IngestLoop
participant ProcessJob
participant BuildModel
participant NormalizerAgent
participant Validator
IngestLoop->>ProcessJob: env_resolver, model_resolver
ProcessJob->>BuildModel: build_model(env, model)
ProcessJob->>NormalizerAgent: run(user_message)
NormalizerAgent-->>ProcessJob: list[NormalizedFinding]
ProcessJob->>Validator: validate each item -> FindingCreate
Validator-->>ProcessJob: (valid_findings, errors)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
backend/tests/agents/test_normalizer_agent.py (2)
1-8: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueStale OpenCode references in the module docstring.
This PR migrates the normalizer to an in-process Pydantic AI agent, but the docstring still states these tests call the LLM "via OpenCode" and the budget note may no longer match the now-default
gpt-4o-mini/claude-haiku-4-5eval models. Update for accuracy.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/agents/test_normalizer_agent.py` around lines 1 - 8, Update the module docstring in test_normalizer_agent.py to remove the stale "via OpenCode" wording and the outdated budget note, and state that tests now run against the in-process Pydantic AI agent used by normalize_findings() (using default eval models like gpt-4o-mini / claude-haiku-4-5); keep instructions for running the tests (pytest command) but correct the description to accurately reflect the current in-process agent and model defaults.
275-277: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueStale model reference in comment.
The eval model is now
gpt-4o-mini(orclaude-haiku-4-5), notgpt-4.1-nano. The comment's rationale about nano-tier truncation no longer matches the selected model, which can mislead readers debugging the lenient>= 5assertion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/agents/test_normalizer_agent.py` around lines 275 - 277, Update the stale comment that references "gpt-4.1-nano" to reflect the current eval model (e.g., "gpt-4o-mini" or "claude-haiku-4-5") and adjust the rationale about truncation to match the chosen model's behavior; specifically, edit the comment above the test that verifies the normalizer handles partial output (the block mentioning the chunk fallback and the lenient ">= 5" assertion) so it no longer claims nano-tier truncation but instead explains that larger-model behavior may still produce partial outputs for large batches and why the test allows partial results.backend/tests/agents/test_plain_description_eval.py (1)
15-16: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueStale OpenCode skip-gating note.
Post-migration the normalizer no longer depends on the OpenCode binary; the docstring's "no API key or OpenCode binary is present" condition should be updated to reflect the Pydantic AI app-level path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/agents/test_plain_description_eval.py` around lines 15 - 16, Update the module docstring to remove the stale mention of the OpenCode binary and instead indicate that the test is skipped when no API key or Pydantic AI app configuration is present (referencing the skip gating implemented in conftest.py); specifically change the phrase "no API key or OpenCode binary is present" to something like "no API key or Pydantic AI app configuration is present" to reflect that the normalizer no longer depends on the OpenCode binary.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/cliff/integrations/ingest_worker.py`:
- Around line 72-76: When env_resolver() or model_resolver() returns no
provider, short-circuit the job before calling normalize_findings to avoid tight
fail→pending→retry loops; after resolving env = await env_resolver() and model =
await model_resolver() (the block around env_resolver/model_resolver in
ingest_worker.py), detect if env is falsy or model is None and transition the
job to a terminal/paused state (e.g. mark failed with a clear message or set a
longer paused state) instead of proceeding to normalize_findings, so you never
consume retries counted by _MAX_CONSECUTIVE_FAILURES; ensure the job state
update is applied atomically and that any cleanup/logging includes which
resolver returned empty.
In `@backend/tests/agents/test_plain_description_eval.py`:
- Around line 30-46: The test duplicates the real-LLM setup: move the _LLM_ENV
dict comprehension and the _eval_model() function (and the derived _LLM_MODEL)
out of backend/tests/agents/test_plain_description_eval.py into a single shared
location (e.g., tests/agents/conftest.py or tests/agents/eval_utils.py); export
or provide them as fixtures or helpers so both test_plain_description_eval.py
and test_normalizer_agent.py import/use the same _LLM_ENV and
_eval_model/_LLM_MODEL symbols, update imports in both test modules to reference
the shared definitions, and remove the duplicated definitions from the
individual test files.
---
Outside diff comments:
In `@backend/tests/agents/test_normalizer_agent.py`:
- Around line 1-8: Update the module docstring in test_normalizer_agent.py to
remove the stale "via OpenCode" wording and the outdated budget note, and state
that tests now run against the in-process Pydantic AI agent used by
normalize_findings() (using default eval models like gpt-4o-mini /
claude-haiku-4-5); keep instructions for running the tests (pytest command) but
correct the description to accurately reflect the current in-process agent and
model defaults.
- Around line 275-277: Update the stale comment that references "gpt-4.1-nano"
to reflect the current eval model (e.g., "gpt-4o-mini" or "claude-haiku-4-5")
and adjust the rationale about truncation to match the chosen model's behavior;
specifically, edit the comment above the test that verifies the normalizer
handles partial output (the block mentioning the chunk fallback and the lenient
">= 5" assertion) so it no longer claims nano-tier truncation but instead
explains that larger-model behavior may still produce partial outputs for large
batches and why the test allows partial results.
In `@backend/tests/agents/test_plain_description_eval.py`:
- Around line 15-16: Update the module docstring to remove the stale mention of
the OpenCode binary and instead indicate that the test is skipped when no API
key or Pydantic AI app configuration is present (referencing the skip gating
implemented in conftest.py); specifically change the phrase "no API key or
OpenCode binary is present" to something like "no API key or Pydantic AI app
configuration is present" to reflect that the normalizer no longer depends on
the OpenCode binary.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d64202aa-f740-4c07-8468-82bb573f510b
📒 Files selected for processing (9)
backend/cliff/agents/runtime/normalizer_agent.pybackend/cliff/integrations/ingest_worker.pybackend/cliff/integrations/normalizer.pybackend/cliff/main.pybackend/tests/agents/test_normalizer_agent.pybackend/tests/agents/test_plain_description_eval.pybackend/tests/integrations/test_ingest_worker_plain_description.pybackend/tests/test_ingest_job.pybackend/tests/test_normalizer.py
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
Comment on lines
+72
to
+76
| # 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() |
There was a problem hiding this comment.
env is resolved from the active provider, not the job's stored model, so queued cross-provider jobs can pick up the wrong credentials and fail in build_model(env, model) with ProviderConfigurationError. Can we resolve from the pinned provider, or block cross-provider pins before enqueueing?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
backend/cliff/integrations/ingest_worker.py around lines 72-76 inside the `_process_job`
function, the code resolves `env = await env_resolver()` once per job but then uses the
job-pinned `model` from `get_ingest_job_raw_data`; this can mismatch the active
provider’s credentials with a model pinned to a different provider (e.g.,
anthropic/openrouter after provider switch). Refactor `_process_job` so `env` is
resolved from the same provider that owns the pinned `model` (derive provider from the
model identifier/prefix or persist/store the provider alongside the model at enqueue
time, then use that provider to select the right env/credentials). Also add a guard that
detects cross-provider pins and either rejects the job with a clear failure status at
enqueue time or marks the job as failed before calling `normalize_findings`, rather than
letting `build_model(env, model)` throw.
Collaborator
Author
There was a problem hiding this comment.
Cliff is single-provider by design (ADR-0037: one canonical ai_integration + key at a time), so a job and the active env are normally the same provider. The only way to hit a mismatch is changing providers while a job sits queued — a rare race. When it happens, build_model raises ProviderConfigurationError → the chunk fails gracefully (no crash), and with the new short-circuit a fully-unconfigured provider fails the job up front. Persisting the provider alongside the model at enqueue time (so a stale pin is rejected deterministically) is a reasonable follow-up, but it's a new feature rather than a migration fix — tracking it separately.
There was a problem hiding this comment.
Thanks, that makes sense — I’ll keep the single-provider constraint and graceful failure behavior in mind, and save this to memory once the PR is merged.
There was a problem hiding this comment.
Commit ca18dbb addressed this comment by reconciling the job’s model with the active provider before running any chunks. It now detects when a pinned model belongs to a different provider and forcibly uses the current active model so that the env/model pair always matches, preventing build_model from seeing stale credentials.
Comment on lines
97
to
102
| 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: |
There was a problem hiding this comment.
Two issues in the normalize_findings path: model_resolver output isn't validated before being passed as model=, so a bare override like claude-opus-4-1 can fail every chunk; and when normalize_findings returns only errors (valid == []), the chunk is still marked completed — should we validate/canonicalize the model first, and treat error-only results as failed chunks via increment_failed_chunk?
Want Baz to fix this for you? Activate Fixer You can also update your AI coding guidelines based on this comment by apply pr to [branch name]
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
backend/cliff/integrations/ingest_worker.py around lines 97-102 inside _process_job: 1.
MODEL VALIDATION: Around lines 74-77 (where `model` is set), add a helper that validates
the value returned by `model_resolver` matches the required `<provider>/<model>` format
before passing it to `normalize_findings`. If validation fails, set the job to
failed/pending with a clear error message rather than continuing to process chunks. This
prevents `resolve_model()` returning a raw override like `claude-opus-4-1` from causing
`build_model()` to reject it and fail every chunk. 2. CHUNK COMPLETION LOGIC: After
`normalize_findings` returns, detect the "no valid findings but errors exist" case
(`valid == []` with errors populated) and treat it as a failed chunk: call
`increment_failed_chunk` and adjust `consecutive_failures` instead of resetting
`consecutive_failures` to 0. Also update the downstream completion accounting (near
lines ~139-156 where `increment_completed_chunk` is called) so `completed_chunks` is
only incremented when there are actual valid findings or a fully successful result.
Collaborator
Author
There was a problem hiding this comment.
Model validation: model is validated — build_model(env, model) rejects a non-<provider>/<model> id (and a missing key) with ProviderConfigurationError, which normalize_findings catches and returns as an error rather than crashing. The new no-provider short-circuit (36925d6) additionally fails the job up front instead of per-chunk-looping when nothing is configured.
Error-only chunk marked completed: this is pre-existing accounting (the OpenCode-era path also increment_completed_chunk'd after recording per-item errors) and is intentional for the partial-success contract — a chunk where every item failed validation still records each error via increment_failed_chunk, so the failure is visible. Re-classifying error-only chunks as failed is a behaviour change worth its own issue rather than riding this migration PR.
There was a problem hiding this comment.
Thanks, that makes sense. I’ll save this to memory once the PR is merged.
There was a problem hiding this comment.
Commit ca18dbb addressed this comment by short-circuiting jobs when the resolved model lacks the required <provider>/<model> form (the new _provider_prefix guard plus pinned/active model reconciliation fails the job before any chunk is processed) and by only incrementing completed_chunks when a chunk actually yielded findings or no errors, so error-only chunks no longer mark themselves completed.
Comment on lines
+321
to
+322
| if item.get("status") is None: | ||
| item["status"] = "new" |
There was a problem hiding this comment.
status only gets coerced when it is None, so any other non-FindingStatus string makes FindingCreate.model_validate() fail and moves the whole finding to errors — can we coerce unknown values to new before validation, or narrow NormalizedFinding.status to Literal["new"]?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
backend/cliff/integrations/normalizer.py around lines 315-334 inside normalize_findings,
fix the partial-success validation logic for `status`. Right now `status` is only
defaulted to "new" when item.get("status") is None, so any other LLM-provided string
(e.g. "open") fails FindingCreate.model_validate() and sends the whole record to errors.
Refactor the loop to normalize `status` before validation: if status is missing/None OR
is not a valid FindingStatus value, set it to "new" (alternatively, tighten the
NormalizedFinding model so `status` is constrained to Literal["new"]). Ensure this
happens prior to FindingCreate.model_validate() so valid items are not dropped due
solely to a status mismatch.
Collaborator
Author
There was a problem hiding this comment.
Fixed in 36925d6. The normalizer always produces brand-new findings, so the loop now forces item["status"] = "new" before validation rather than only filling null — a stray model value like "open" no longer drops an otherwise-valid finding into errors. Covered by test_normalize_forces_status_new_over_stray_value.
There was a problem hiding this comment.
Commit 36925d6 addressed this comment by forcing every normalized finding’s status to "new" before validation, preventing stray strings such as "open" from sending otherwise-valid records to errors.
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
Move HealthStatus into cliff/models (it lived in engine/models, deleted
next) and drop the live OpenCode probe. The substrate runs in-process via
Pydantic AI, so the response shape is preserved for the frontend health
card + cliffsec status: `opencode` is always "ok" and `opencode_version`
now carries the PA version string ("pydantic-ai <ver>"). Model comes from
the canonical ai_model_cache; ai_provider_ready is unchanged.
- cliff/models: add HealthStatus (kept-shape, repurposed fields)
- api/routes/health.py: drop opencode_process/opencode_client; report PA
- tests: retarget test_models + test_routes_health; reduce the now-unused
mock_opencode_process fixture to a no-op
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment on lines
+98
to
+102
| 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 `&&`): | ||
|
|
There was a problem hiding this comment.
RepoAgentRunner should keep the unauthenticated CLONE_URL="$REPO_URL" path when GH_TOKEN is missing, otherwise the agent follows the prompt into an auth failure; can we restore the conditional or fail fast before the run?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
backend/cliff/agents/runtime/repo_actions.py around lines 98-102 inside
SECURITY_MD_SYSTEM_PROMPT’s “Clone and set up” workflow, the instructions
unconditionally build CLONE_URL using $GH_TOKEN. Change this to match the old Jinja
behavior: if GH_TOKEN is unset/empty, set CLONE_URL to the plain $REPO_URL (no
x-access-token prefix) and proceed with an unauthenticated clone. Apply the same
conditional logic to the dependabot clone instructions in DEPENDABOT_SYSTEM_PROMPT
around lines ~270-277. If the repo requires authentication and the unauthenticated clone
fails, update the instructions to return status="failed" with a clear error_details
rather than continuing to attempt token-auth.
Collaborator
Author
There was a problem hiding this comment.
Fixed in 36925d6. Both prompts now restore the conditional: embed the token in CLONE_URL only when $GH_TOKEN is set, otherwise clone the plain $REPO_URL (matching the old Jinja behaviour). A token-less private clone then fails cleanly and the agent returns status="failed".
There was a problem hiding this comment.
Thanks for the confirmation—seeing the prompts restore the GH_TOKEN guardrail (tokenized clone when available, bare URL otherwise) and the repo-action runner now failing fast on unauthenticated clones keeps the agent aligned with the old Jinja behavior and avoids silent auth failures.
There was a problem hiding this comment.
Commit 36925d6 addressed this comment by restoring the GH_TOKEN guardrail in both repo and Dependabot prompts: CLONE_URL is only tokenized when GH_TOKEN is set, otherwise the script uses the bare REPO_URL so an unauthenticated clone fails immediately. That failure now bubbles up as status=failed instead of the agent blindly retrying with an empty token, matching the previous Jinja behavior.
Comment on lines
248
to
+250
| 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 |
There was a problem hiding this comment.
repo_action_* workspaces now call agent.run(...) in the FastAPI process, bypassing the per-workspace subprocess/isolation required by CLAUDE.md; should we keep them in the workspace process pool, or add an explicit repo-wide exception/guard if in-process execution is intentional?
Want Baz to fix this for you? Activate Fixer You can also update your AI coding guidelines based on this comment by apply pr to [branch name]
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
backend/cliff/workspace/repo_workspace_runner.py around lines 248-250 inside
RepoAgentRunner.run, the code calls agent.run(...) directly in the FastAPI process. This
bypasses CLAUDE.md’s requirement that every workspace runs with both isolated working
directory and isolated process context via a per-workspace OpenCode subprocess, and
there’s no repo-wide exception/guard—only an inline ADR comment. Refactor by
restoring process isolation for repo-action workspaces: reintroduce the
WorkspaceProcessPool (or equivalent) dependency into RepoAgentRunner and run the
repo-action agent inside the pool/subprocess with cwd=data/workspaces/<workspace_id>/
(or the existing workspace_root), then collect the output/structured result the same way
as before. If in-process execution is truly intentional, add a fail-closed explicit
guard (feature flag/config/exception list) near the start of RepoAgentRunner.run that
blocks in-process execution unless an approved repo-wide override is set, and update the
error message/comments to point to the documented exception rather than relying on an
ADR mention.
Collaborator
Author
There was a problem hiding this comment.
By design — this is the core of ADR-0047. All agents move from per-workspace OpenCode subprocesses to in-process Pydantic AI; the executor (PR #2) already runs this way, and repo-actions now match it. The trade (process isolation → asyncio discipline) is documented in ADR-0047 §6 "What we lose explicitly," with the mitigation being exception-safe tools + the outer asyncio.wait_for guard. The repo-action run is non-raising by contract (every path collapses to a RepoAgentStatus), so a tool bug can't take down the worker.
There was a problem hiding this comment.
Thanks, that makes sense. I’ll save this to memory once the PR is merged.
Comment on lines
+271
to
+275
| 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) |
There was a problem hiding this comment.
get_repo_workspace_spawner now duplicates the _ai_env_resolver/_ai_model_resolver logic from main.py, should we factor that into a shared helper or expose it via app.state? Otherwise cache handling tweaks can drift between the repo-agent spawner and the regular workspace pool.
Want Baz to fix this for you? Activate Fixer
Collaborator
Author
There was a problem hiding this comment.
Fair — both build trivial closures over the same app.state.ai_env_cache / ai_model_cache. They're 2-line readers rather than real logic, and main.py's versions are captured at lifespan time for the executor while get_repo_workspace_spawner's read per-request request.app, so they're not literally the same object. Hoisting a shared app.state.ai_env_resolver / ai_model_resolver is a clean small cleanup — I'll fold it into the final substrate-removal pass (which rewires this lifespan code anyway) rather than churn it twice.
…reate` Code-review catch: the security.md + dependabot prompts told the agent to run `gh -C repo pr create`, but unlike git, the GitHub CLI has no directory flag, so the PR-create step would fail with "unknown flag: -C" and every repo-action run would end status=failed. Route the PR-create through bash with `cd repo && gh pr create ...` (the bash tool runs at the workspace root; gh reads the repo from the clone's remote and the token from $GH_TOKEN). Also regenerates the OpenAPI snapshot for the workspace-chat/sessions + pool-status routes removed earlier in this PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… auth Three confirmed review findings: - normalizer: force `status="new"` on every item instead of only filling null. A stray model value (e.g. "open") no longer drops an otherwise-valid finding into `errors` — normalized findings are always brand-new. - ingest_worker: short-circuit a job to terminal `failed` when no AI provider is configured (empty env / null model), instead of looping fail → pending → re-poll every ~1s forever (a new dependency the PA migration introduced; build_model can't run without a provider). - repo-action prompts: restore the conditional clone URL — embed the token only when $GH_TOKEN is set, else clone the plain URL (matches the old Jinja behaviour; a token-less private clone then fails cleanly). Tests: stray-status coercion; no-provider → failed (not retry). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
galanko
changed the title
Comment on lines
+103
to
+107
| ```bash | ||
| REPO_URL="<the repository URL from the task>" | ||
| # Use a token-embedded URL only when $GH_TOKEN is set; otherwise clone the | ||
| # URL directly (a private repo then fails at clone — return status=failed | ||
| # with that error rather than retrying with an empty token). |
There was a problem hiding this comment.
The token-aware clone snippet is duplicated in the SECURITY and Dependabot prompts, should we extract a shared constant/helper so changes to REPO_URL or cliff-bot identity only land once?
Want Baz to fix this for you? Activate Fixer
There was a problem hiding this comment.
Commit ca18dbb addressed this comment by extracting the token-aware clone snippet into a shared _clone_block helper and template so both the SECURITY and Dependabot prompts reuse the same repository-cloning logic.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/cliff/agents/runtime/tools/permissions.py`:
- Around line 178-192: The current _auto_approved(ctx) unconditionally treats
any "ask" tier as pre-approved for repo-action runs; change it to only
auto-approve when repo-actions explicitly opted-in for those specific "ask"
calls. Update _auto_approved to return True only when both getattr(ctx.deps,
"auto_approve", False) is True AND an explicit opt-in flag is present (e.g.,
getattr(ctx.deps, "repo_action_auto_approve", False) or getattr(ctx.deps,
"auto_approve_explicit", False)), and update the WorkspaceDeps
interface/constructors to expose that explicit opt-in flag; keep the caller
condition (tier == "ask" and not ctx.tool_call_approved and not
_auto_approved(ctx)) the same so default/fallback/unknown "ask" buckets remain
gated for human approval.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 66c1228d-f6fa-4a09-9bb4-b37123a21a40
📒 Files selected for processing (23)
backend/cliff/agents/runtime/deps.pybackend/cliff/agents/runtime/repo_actions.pybackend/cliff/agents/runtime/tools/bash.pybackend/cliff/agents/runtime/tools/permissions.pybackend/cliff/api/_engine_dep.pybackend/cliff/api/routes/health.pybackend/cliff/api/routes/workspaces.pybackend/cliff/integrations/ingest_worker.pybackend/cliff/integrations/normalizer.pybackend/cliff/models/__init__.pybackend/cliff/workspace/repo_workspace_runner.pybackend/tests/agents/tools/test_permissions.pybackend/tests/api/openapi_snapshot.jsonbackend/tests/conftest.pybackend/tests/test_ingest_job.pybackend/tests/test_models.pybackend/tests/test_normalizer.pybackend/tests/test_repo_workspace_runner.pybackend/tests/test_repo_workspace_spawner.pybackend/tests/test_routes_health.pycli/cliff_cli/cli.pyfrontend/src/api/client.tsfrontend/src/components/issues/IssueSidePanel.tsx
💤 Files with no reviewable changes (1)
- frontend/src/api/client.ts
The fix command no longer pre-creates a workspace session (the PA pipeline runs agents in-process), so the three `fix` tests registered a `POST /workspaces/ws-1/sessions` httpx mock that's never requested — pytest_httpx fails the run on an unused mock. Remove the stale registrations. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
galanko
added a commit
that referenced
this pull request
Jun 2, 2026
Comment/docstring-only. The PA code was already correct; the prose still described OpenCode subprocesses, an auth.json/singleton restart, a process pool, and opencode.json MCP config that no longer exist. Reword the load-bearing-wrong comments to describe the in-process Pydantic AI behaviour (executor dispatch, errors, repo runner, tools, provider/catalog, ai/service + models, gateway), and drop already-happened "deleted in PR2.E" / "PR #3 finalizes" future-tense references. Kept: accurate "used to run on OpenCode / pre-migration" provenance that explains why code is shaped the way it is, the intentional ``opencode`` HealthStatus/VersionInfo wire-compat field, and the legacy_migration notes about legacy opencode-keyed app_settings. Part of PR #3d, stacked on #253. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
galanko
added a commit
that referenced
this pull request
Jun 2, 2026
- settings.py: GET /settings/providers gets response_model=list[ProviderInfo] and returns typed instances — restores FastAPI validation + OpenAPI schema for a user-facing endpoint (CodeRabbit). - tests/agents/conftest.py: skip-no-key now applies ONLY to the two live-LLM files (test_normalizer_agent, test_plain_description_eval); the FunctionModel/ TestModel runtime tests run in keyless CI so a regression isn't masked (CodeRabbit). Verified: keyless run → FunctionModel tests run, live skip. - test_workspace_repo_url_overrides_global.py: save/restore app.state.context_builder so the fixture can't leak a tmp_path-rooted builder into later tests (CodeRabbit). - test_workspace_repo_creation.py: also assert history/agent-runs.jsonl exists (CodeRabbit). - build-tarball.sh: drop the stale "opencode binaries" text from the generated README-LOCAL-INSTALL.md (CodeRabbit). - test_ai_catalog.py: docstring no longer references "spawning OpenCode". Part of PR #3d, stacked on #253. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
galanko
added a commit
that referenced
this pull request
Jun 3, 2026
The agent substrate runs in-process via Pydantic AI (ADR-0047), so the CLI no longer manages any child agent process or its ports: - process_sweep.py: drop the ``opencode`` process kind, the OPENCODE_SINGLETON_PORT (4096) + WORKSPACE_PORT_RANGE (4100-4199) constants, and the bin/opencode classification arm. find_cliff_processes() is uvicorn-only and no longer needs cliff_home. - daemon.py: _cliff_ports() + the doctor port loop only consider the app port now; stop/uninstall stop scanning the dead 4096/4100-4199 range; layout docstring drops .opencode-version + bin/opencode. - cli.py: drop the now-unreachable ``opencode_engine_unavailable`` status blocker (/health.opencode is hard-wired "ok"). - updater.py docstring no longer claims install-opencode.sh. Tests updated: test_process_sweep (uvicorn-only), test_daemon (app-port sweep, opencode→uvicorn fixtures), test_cli (unconfigured blockers). CLI suite 94 passed, ruff clean. Zero OpenCode references remain in cli/cliff_cli. Part of PR #3d, stacked on #253. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
galanko
added a commit
that referenced
this pull request
Jun 3, 2026
CodeRabbit: lock the migration contract — the doctor payload must no longer emit an ``opencode`` check (the binary version check was removed with the substrate). Adds the explicit negative assertions alongside the subset check. Part of PR #3d, stacked on #253. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
galanko
added a commit
that referenced
this pull request
Jun 3, 2026
Baz: the doctor port check hard-coded DEFAULT_PORT, so a deployment with a non-default CLIFF_APP_PORT would miss a real port-in-use failure (and report 8000 free while 9123 is wedged). Iterate over _cliff_ports() (the configured app port) instead. Adds a regression test. Part of PR #3d, stacked on #253. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
galanko
added a commit
that referenced
this pull request
Jun 3, 2026
After the e2e verification passed, clean the user-facing and operational references the substrate removal left behind: - README: installer no longer fetches an OpenCode binary; bundled subprocesses are now just Trivy + Semgrep (PA is a Python dep). - NOTICE + THIRD-PARTY-LICENSES.md: drop the OpenCode (MIT) bundled-binary entry — we no longer download or redistribute it. - .dockerignore / .gitignore / CONTRIBUTING.md / scripts/dev.sh / scripts/install-local.sh: drop the .opencode-version + install-opencode.sh references. - docker/Dockerfile + entrypoint.sh: drop the "OpenCode Go binary" stage comment and the CLIFF_OPENCODE_PORT/BIN runtime banner lines. - KNOWN_ISSUES.md: remove the 9 now-resolved OpenCode-era issues (SSE/chat sessions, opencode.json drift, keys-lost-on-restart, 75+-provider catalog) and reword the orphan-PR issue's failure modes. - plugins/cliff-security knowledge: install/doctor no longer mention an opencode binary or the retired 4096/4100-4102 port checks. - frontend: PostureCard no longer shows "spinning up OpenCode…"; regenerate api/types.ts from the current OpenAPI snapshot (drops the deleted session/chat endpoints); reword the AIProviderStatus / ModelPicker comments + a test fixture. Left intact: CHANGELOG.md + ROADMAP.md completed-phase milestones (accurate history — Phase 1 genuinely was an OpenCode spike), the CLAUDE.md historical -names note, and the intentional ``opencode``/``opencode_version`` HealthStatus wire-compat fields. Verified: frontend tsc + lint + build clean. (The pre-existing frontend vitest failures live on #253 too and are uncaught by CI, which runs lint+build only — out of scope for this PR.) Part of PR #3d, stacked on #253. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…gent runtime (#254) * refactor(settings): derive model + providers from canonical AI state Drop config_manager / opencode_client from the settings routes — the last contract-bearing OpenCode dependency outside the engine package: - GET/PUT /settings/model now route only through AIIntegrationService (canonical app_setting(model)); no opencode.json fallback. Empty ModelConfig when no provider is connected; PUT 503s without a vault and 400s with no active provider / prefix mismatch. - GET /settings/providers is rebuilt from cliff.ai.catalog (static per-provider picker rows) instead of OpenCode's models.dev dump. Wire shape {id,name,env,models} is preserved; models keyed by the bare id so the UI/CLI rebuild f"{provider}/{model_id}" unchanged. - POST /settings/providers/test now runs a bounded "Say OK" through Pydantic AI (build_model + Agent) and classifies the outcome. - Delete the vestigial config_manager-only routes /settings/providers/ configured and /settings/api-keys/* (no live UI consumer) plus their dead frontend wrappers/hooks/types and the MSW api-keys handler. CLI (cliffsec model get/set/list) is unaffected — same wire shapes. Part of PR #3d (OpenCode substrate removal), stacked on #253. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(ai): drop the OpenCode auth.json dual-path ADR-0047: the PA model factory reads the provider key straight from the resolved env (resolve_env_for_workspace) — there is no auth.json to keep in sync. Remove sync_to_opencode, _sync_opencode_auth, _clear_opencode_auth, _safe_write_opencode_config, the _OPENCODE_AUTH_PROVIDERS set, and all their call sites in save/disconnect/set_model. Canonical vault + app_setting(model) state is untouched; _fire_key_change stays as a generic env-refresh hook. Drop the now-obsolete auth.json-push tests and the dead opencode_client stub fixtures. Part of PR #3d, stacked on #253. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(main): drop the singleton OpenCode lifespan + dead process pool ADR-0047: agents run in-process via Pydantic AI, so nothing spawns or talks to an OpenCode process anymore. Remove from the lifespan: - the singleton opencode_process start/stop + set_extra_env seeding - the auth.json sync_to_opencode reconcile + config_manager reconcile_model / restore_keys_to_engine startup calls - the WorkspaceProcessPool (constructed but never read after #3c — the executor never called it), its app.state handle, the idle-cleanup loop, and pool.stop_all on shutdown - the singleton-restart half of the ai_on_key_change hook (it now only refreshes the warm env/model cache) - opencode_client.close() / opencode_process.stop() teardown AgentExecutor drops its unused ``pool`` parameter. Test construction sites updated to the new signature. Part of PR #3d, stacked on #253. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(workspace): stop scaffolding opencode.json + .j2 agent templates ADR-0047: PA agents run in-process from prompts defined in cliff.agents.runtime — they never read a workspace's opencode.json or rendered .opencode/agents/*.md. Stop writing both: - WorkspaceDirManager.create() drops the .opencode/agents tree, the opencode.json write, and its mcp_servers/model params. The dir still carries the finding context (finding.json, finding.md, CONTEXT.md, context sections) the agents read. - create_repo_workspace() becomes a thin clone-dir allocator (history/ + REPO_ACTION.md); no rendered prompt, no opencode.json. The repo-action agent carries its own prompt + permission policy. - WorkspaceContextBuilder drops the AgentTemplateEngine dependency and the per-create / per-update write_agents render passes; it still writes the workspace-integrations.json manifest (read by check_config_freshness). - Delete _build_opencode_config and the WorkspaceDir opencode_json / opencode_dir / agents_dir properties. - main.py lifespan + _engine_dep spawner drop the template engine / model plumbing. Tests updated to assert finding context (not templates); the obsolete opencode.json/template assertions and the OpenCode pool-wiring test (test_ai_phase_f_wiring) are removed. Part of PR #3d, stacked on #253. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(version): report the in-process substrate version, not OpenCode Move VersionInfo out of the doomed cliff.engine.models into cliff.models alongside HealthStatus, and add a shared substrate_version() helper. The /version handshake's compat ``opencode`` field and /health's ``opencode_version`` both now carry "pydantic-ai <ver>" (ADR-0047) instead of the pinned OpenCode binary version. /health stops falling back to settings.opencode_model (opencode.json is gone) — the model is the canonical ai_model_cache or empty. The config.py opencode_* settings removal rides with the engine deletion (they're read by engine/ at runtime until it's deleted). Part of PR #3d, stacked on #253. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: delete the OpenCode substrate Every OpenCode caller is gone (#3b/#3c + steps 1-5), so remove the engine itself and everything that fed it: Backend - delete backend/cliff/engine/ (client, process, pool, config_manager, models) and backend/cliff/agents/template_engine.py + templates/*.j2 - config.py: drop the opencode_* connection/model/version settings and properties; repo-root sentinel moves from .opencode-version to VERSION - agents/__init__.py stops re-exporting the template engine - delete the engine/pool/template unit tests + the OpenCode e2e suite; strip the opencode fixtures from the conftests; retarget test_config / test_docker / the openrouter route test; regenerate the OpenAPI snapshot Packaging / CLI - Dockerfile: drop the OpenCode binary install step, the opencode.json / .opencode COPYs, and the CLIFF_OPENCODE_* env - build-tarball / install-local / install smoke / install-smoke.yml stop bundling + installing + asserting the opencode binary - CLI updater stops re-running install-opencode.sh; daemon doctor drops the opencode binary check (it would report "not found" post-migration) Root - delete .opencode-version, opencode.json, .opencode/, scripts/install-opencode.sh Follow-up (noted, not blocking): the CLI's process_sweep still classifies a ``$CLIFF_HOME/bin/opencode`` process kind and cli.py keeps the (now always "ok") opencode health field — dead but harmless; a separate cleanup PR. Backend 1432 pass, CLI 97 pass, frontend tsc clean, ruff clean. Part of PR #3d, stacked on #253. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: update CLAUDE.md for the Pydantic AI substrate Architecture table, repo layout, "How It Runs", the workspace-runtime layers, the AI-provider section, and the testing guidance now describe the in-process Pydantic AI substrate (ADR-0047) instead of the OpenCode subprocess / process pool / auth.json / drift model. Part of PR #3d, stacked on #253. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: scrub stale OpenCode comments from the live code Comment/docstring-only. The PA code was already correct; the prose still described OpenCode subprocesses, an auth.json/singleton restart, a process pool, and opencode.json MCP config that no longer exist. Reword the load-bearing-wrong comments to describe the in-process Pydantic AI behaviour (executor dispatch, errors, repo runner, tools, provider/catalog, ai/service + models, gateway), and drop already-happened "deleted in PR2.E" / "PR #3 finalizes" future-tense references. Kept: accurate "used to run on OpenCode / pre-migration" provenance that explains why code is shaped the way it is, the intentional ``opencode`` HealthStatus/VersionInfo wire-compat field, and the legacy_migration notes about legacy opencode-keyed app_settings. Part of PR #3d, stacked on #253. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: address PR #254 review comments - settings.py: GET /settings/providers gets response_model=list[ProviderInfo] and returns typed instances — restores FastAPI validation + OpenAPI schema for a user-facing endpoint (CodeRabbit). - tests/agents/conftest.py: skip-no-key now applies ONLY to the two live-LLM files (test_normalizer_agent, test_plain_description_eval); the FunctionModel/ TestModel runtime tests run in keyless CI so a regression isn't masked (CodeRabbit). Verified: keyless run → FunctionModel tests run, live skip. - test_workspace_repo_url_overrides_global.py: save/restore app.state.context_builder so the fixture can't leak a tmp_path-rooted builder into later tests (CodeRabbit). - test_workspace_repo_creation.py: also assert history/agent-runs.jsonl exists (CodeRabbit). - build-tarball.sh: drop the stale "opencode binaries" text from the generated README-LOCAL-INSTALL.md (CodeRabbit). - test_ai_catalog.py: docstring no longer references "spawning OpenCode". Part of PR #3d, stacked on #253. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(cli): finish de-OpenCode'ing the CLI process management The agent substrate runs in-process via Pydantic AI (ADR-0047), so the CLI no longer manages any child agent process or its ports: - process_sweep.py: drop the ``opencode`` process kind, the OPENCODE_SINGLETON_PORT (4096) + WORKSPACE_PORT_RANGE (4100-4199) constants, and the bin/opencode classification arm. find_cliff_processes() is uvicorn-only and no longer needs cliff_home. - daemon.py: _cliff_ports() + the doctor port loop only consider the app port now; stop/uninstall stop scanning the dead 4096/4100-4199 range; layout docstring drops .opencode-version + bin/opencode. - cli.py: drop the now-unreachable ``opencode_engine_unavailable`` status blocker (/health.opencode is hard-wired "ok"). - updater.py docstring no longer claims install-opencode.sh. Tests updated: test_process_sweep (uvicorn-only), test_daemon (app-port sweep, opencode→uvicorn fixtures), test_cli (unconfigured blockers). CLI suite 94 passed, ruff clean. Zero OpenCode references remain in cli/cliff_cli. Part of PR #3d, stacked on #253. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(cli): assert the opencode doctor check is absent CodeRabbit: lock the migration contract — the doctor payload must no longer emit an ``opencode`` check (the binary version check was removed with the substrate). Adds the explicit negative assertions alongside the subset check. Part of PR #3d, stacked on #253. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: regenerate OpenAPI snapshot for typed /settings/providers Adding response_model=list[ProviderInfo] to GET /settings/providers (PR review fix) changes the schema's response component. Re-snapshot to match. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cli): doctor probes the configured app port, not the default Baz: the doctor port check hard-coded DEFAULT_PORT, so a deployment with a non-default CLIFF_APP_PORT would miss a real port-in-use failure (and report 8000 free while 9123 is wedged). Iterate over _cliff_ports() (the configured app port) instead. Adds a regression test. Part of PR #3d, stacked on #253. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: scrub remaining OpenCode references from README, license + docs After the e2e verification passed, clean the user-facing and operational references the substrate removal left behind: - README: installer no longer fetches an OpenCode binary; bundled subprocesses are now just Trivy + Semgrep (PA is a Python dep). - NOTICE + THIRD-PARTY-LICENSES.md: drop the OpenCode (MIT) bundled-binary entry — we no longer download or redistribute it. - .dockerignore / .gitignore / CONTRIBUTING.md / scripts/dev.sh / scripts/install-local.sh: drop the .opencode-version + install-opencode.sh references. - docker/Dockerfile + entrypoint.sh: drop the "OpenCode Go binary" stage comment and the CLIFF_OPENCODE_PORT/BIN runtime banner lines. - KNOWN_ISSUES.md: remove the 9 now-resolved OpenCode-era issues (SSE/chat sessions, opencode.json drift, keys-lost-on-restart, 75+-provider catalog) and reword the orphan-PR issue's failure modes. - plugins/cliff-security knowledge: install/doctor no longer mention an opencode binary or the retired 4096/4100-4102 port checks. - frontend: PostureCard no longer shows "spinning up OpenCode…"; regenerate api/types.ts from the current OpenAPI snapshot (drops the deleted session/chat endpoints); reword the AIProviderStatus / ModelPicker comments + a test fixture. Left intact: CHANGELOG.md + ROADMAP.md completed-phase milestones (accurate history — Phase 1 genuinely was an OpenCode spike), the CLAUDE.md historical -names note, and the intentional ``opencode``/``opencode_version`` HealthStatus wire-compat fields. Verified: frontend tsc + lint + build clean. (The pre-existing frontend vitest failures live on #253 too and are uncaught by CI, which runs lint+build only — out of scope for this PR.) Part of PR #3d, stacked on #253. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment on lines
+111
to
115
| if vault is None: | ||
| raise HTTPException( | ||
| status_code=503, | ||
| detail="Credential vault not initialized. Set CLIFF_CREDENTIAL_KEY.", | ||
| ) |
There was a problem hiding this comment.
PUT /api/settings/model now 503s when app.state.vault is None, but the onboarding flow still supports env-sourced keys via OPENAI_API_KEY etc., so cliffsec model set <provider>/<id> can no longer work in that path — should we keep the legacy no-vault flow or document the new prerequisite/status code?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
backend/cliff/api/routes/settings.py around lines 103-130 in the update_model handler
for PUT /settings/model, the code currently raises HTTP 503 whenever
request.app.state.vault is None (see the vault None check at lines 111-115), which
breaks the supported daemon-env onboarding flow where the provider key comes from
OPENAI_API_KEY/etc. Refactor this logic so the route can still set the model when vault
is absent but an env-sourced provider is available (e.g., detect whether the requested
provider can be resolved from env via cliff.ai.catalog or AIIntegrationService, and only
require the vault for providers that actually need stored credentials). If that’s not
feasible, then update the API contract (and any UI/CLI expectations) to explicitly
document the new prerequisite and 503 response; but prefer restoring
backwards-compatible behavior for env-based providers.
Comment on lines
+137
to
+148
| @router.get("/settings/providers", response_model=list[ProviderInfo]) | ||
| async def list_providers() -> list[ProviderInfo]: | ||
| """Return the supported-provider catalog (ADR-0037). | ||
|
|
||
|
|
||
| @router.get("/settings/providers/configured") | ||
| async def get_configured_providers(): | ||
| try: | ||
| providers = await config_manager.get_configured_providers() | ||
| auth = await config_manager.get_auth_status() | ||
| return {"providers": providers, "auth": auth} | ||
| except Exception as exc: | ||
| raise HTTPException(status_code=502, detail=f"OpenCode unavailable: {exc}") from exc | ||
| Built from the static :mod:`cliff.ai.catalog` — one entry per | ||
| provider with its key env var and the curated model picker rows. The | ||
| wire shape (``{id, name, env, models}`` with ``models`` keyed by the | ||
| bare model id) is what the Settings model picker and ``cliffsec model | ||
| list`` consume. | ||
| """ | ||
| payload: list[ProviderInfo] = [] | ||
| for provider in catalog.all_providers(): |
There was a problem hiding this comment.
Can we keep compatibility routes or add a deprecation note before removing the legacy settings surface, since docs still point users at /api/settings/providers/configured and /api/settings/api-keys*?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
backend/cliff/api/routes/settings.py around lines 137-165, the new GET
`/settings/providers` route was added but the legacy public endpoints previously served
here (notably `/settings/providers/configured` and the `/settings/api-keys*` routes
referenced by the onboarding docs) appear to have been removed without an alias. Restore
backwards-compatible routes for those paths (at minimum
`/api/settings/providers/configured` and `/api/settings/api-keys` plus `PUT
/api/settings/api-keys/{provider}`), and implement them by delegating to the new
catalog/AIIntegrationService-backed logic so callers get the same response shape they
expect. Mark these routes as deprecated (e.g., include a warning in the response body
and/or `Deprecation` header) and add a short note to logs. If exact response
compatibility can’t be guaranteed, then instead keep the routes but return a clear
410/404-with-message and ensure the onboarding skill/docs are updated
accordingly—choose the compatible-route approach if possible to prevent hard 404s.
Comment on lines
+15
to
+24
| 3. Persists the outcome to ``history/status.json`` inside the workspace so the | ||
| posture route can report status back to the UI without a new DB table. | ||
| 5. Stops the workspace process — repo-action workspaces are ephemeral. | ||
|
|
||
| The permission model for repo workspaces is ``"allow"`` for bash/edit/webfetch | ||
| (set up in the spawner via ``_build_repo_action_opencode_config``) because the | ||
| user already authorised the single action by clicking "Let Cliff open a PR". | ||
| No SSE permission queue is needed here. | ||
| Repo-action runs auto-approve every gated tool (clone/edit/push/PR): the user | ||
| already authorised the single action by clicking "Let Cliff open a PR", and | ||
| there is no interactive approval path in a background posture-fix run. | ||
|
|
||
| Contract: the runner never raises. All outcomes — success, bad LLM output, | ||
| OpenCode process failure, timeout — collapse into a ``RepoAgentStatus`` row | ||
| persisted to disk. Callers poll the status file. | ||
| Contract: the runner never raises. All outcomes — success, bad model output, | ||
| model error, timeout — collapse into a ``RepoAgentStatus`` row persisted to | ||
| disk. Callers poll the status file. |
There was a problem hiding this comment.
RepoActionOutput currently stays in disk-only RepoAgentStatus / history/status.json and skips the SidebarState path, so can we route it through the same persistence flow as AgentExecutor or call out the exception in CLAUDE.md?
Want Baz to fix this for you? Activate Fixer You can also update your AI coding guidelines based on this comment by apply pr to [branch name]
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
backend/cliff/workspace/repo_workspace_runner.py around lines 15-24 (the RepoAgentRunner
module docstring/contract describing how results are persisted to history/status.json
and that callers poll a file), the runner is decoupled from the normal AgentExecutor
persistence path so the final RepoActionOutput never reaches the chat timeline and
SidebarState. Update the runner so the produced RepoActionOutput (including pr_url) is
handed off to the same persistence logic used by AgentExecutor/related sidebar/chat
mechanisms, rather than relying solely on writing history/status.json. If that routing
is not feasible, make an explicit, documented exception: clearly state why this flow
cannot use the standard persistence path and where the UI should source status, and
update CLAUDE.md/any relevant guidance to match this exception.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
backend/tests/test_executor.py (1)
372-385:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRewire this test to the PA seam instead of the removed pool path.
After the constructor change,
mock_pool.get_or_start.return_value = _make_mock_client(...)is dead setup:AgentExecutor(mock_context_builder)never sees that client anymore. This test no longer proves malformed remediation output handling; it will now fail or pass for some other branch. Stub_run_pa_executor(or use_executor_with_pa) to return a failedParseResultso the assertions stay tied to parse-failure behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/test_executor.py` around lines 372 - 385, The test test_parse_failure_marks_failed still configures mock_pool.get_or_start but AgentExecutor no longer uses that path; replace the dead setup by stubbing the PA seam instead: have the test call or patch AgentExecutor._run_pa_executor (or use the helper _executor_with_pa) to return a failed ParseResult (with an appropriate .error and no valid parse) so the executor run goes through the parse-failure branch and sets run.status to FAILED and run.last_error from parse_result.error; update assertions to rely on the PA-returned ParseResult rather than the removed pool client.backend/tests/test_docker.py (1)
48-64:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThis no longer tests the SPA fallback path.
The body now only asserts that
index.htmlexists undertmp_path; it never mounts the app or requests an unknown route. A regression in the actual fallback wiring incliff.mainwould still pass here. Please keep this as a route-level assertion by constructing a client withstatic_direnabled and hitting a non-API path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/test_docker.py` around lines 48 - 64, The test test_serves_index_html no longer verifies the SPA fallback; update it to set mock_settings.static_dir to the tmp_path dist, build a test client for the app mounted in cliff.main (re-import or call the app factory used in cliff.main), then perform an HTTP GET on a non-API route such as "/some/unknown/path" and assert the response status is 200 and the body contains "Cliff" (or matches index.html content). Ensure you exercise the actual routing/mount logic in cliff.main rather than only checking filesystem artifacts so the fallback wiring is validated.frontend/src/components/ai-provider/ModelPicker.tsx (1)
282-337:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEnforce the advertised provider prefix before submit.
Line 336 now documents
provider/...as a hard requirement, but the form still submits any non-empty string and only lets the API reject it. That turns a deterministic client-side rule into an avoidable round-trip error.Suggested fix
+ const trimmedCustom = custom.trim() + const hasProviderPrefix = trimmedCustom.startsWith(`${provider}/`) + return ( @@ <form onSubmit={(e) => { e.preventDefault() - if (!custom.trim()) return - pick(custom.trim()) + if (!trimmedCustom) return + if (!hasProviderPrefix) { + setError(`Model id must start with ${provider}/`) + return + } + pick(trimmedCustom) }} style={{ marginTop: 18 }} > @@ <button type="submit" - disabled={!custom.trim() || setModel.isPending} + disabled={!trimmedCustom || !hasProviderPrefix || setModel.isPending} className="cd-btn cd-btn--primary cd-btn--sm" > Use </button>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/ai-provider/ModelPicker.tsx` around lines 282 - 337, The form currently submits any non-empty custom string; update the onSubmit handler in ModelPicker.tsx (the inline onSubmit handler that calls pick(custom.trim())) to first validate that custom.trim() startsWith(`${provider}/`) and bail out (and optionally set a small validation state) if it doesn't, then call pick only when the prefix matches; also update the button disabled expression (currently disabled={!custom.trim() || setModel.isPending}) to include the same prefix check (e.g., disabled={!custom.trim().startsWith(`${provider}/`) || setModel.isPending}) so the client enforces the advertised provider/... requirement before making the API call.cli/cliff_cli/cli.py (1)
174-186: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider renaming or removing the
opencodekey in the status output.The
statuscommand still emits"opencode": version["opencode"](line 178), but with the substrate migration (ADR-0047), this naming may confuse users since there's no longer an OpenCode engine. If the backend/versionendpoint still returns this key for backwards compatibility, consider documenting that or renaming it to something like"substrate"in a follow-up.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/cliff_cli/cli.py` around lines 174 - 186, The status output currently emits the "opencode" key from version (emit({... "opencode": version["opencode"], ...})), which is misleading after the substrate migration; update the status emission in cli.py to either remove the "opencode" field or rename it to a clearer name (e.g., "substrate") and adjust any consumers/tests accordingly, and if the backend still returns version["opencode"] for compatibility, add a brief comment documenting that mapping in the emit block.backend/cliff/workspace/repo_workspace_runner.py (1)
291-303: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdjust comment:
pr_url=Noneis handled safely—no unexpected verifier crash
verify_pr_urlacceptsurl: str | Noneand returnsok=Falsewith anot_a_pull_url: ...reason forNone/malformed URLs.repo_workspace_runner.pythen routesverification.ok == Falseto_finalize(status="failed", ...), sooutput.pr_url=Noneforstatus="pr_created"won’t propagate unexpectedly into a failure/exception.
RepoActionOutputcurrently allowspr_url: str | None = Nonewithout a conditional constraint tying it tostatus=="pr_created". If you want a stronger contract, add Pydantic conditional validation requiringpr_url(and likelybranch_name) whenstatus == "pr_created".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/cliff/workspace/repo_workspace_runner.py` around lines 291 - 303, The comment claims verify_pr_url needs guarding, but verify_pr_url already accepts url: str | None and returns ok=False for None, and repo_workspace_runner._finalize handles non-ok, so failing PR URLs won't crash; to enforce a stronger contract add conditional validation on the RepoActionOutput model so that when status == "pr_created" pr_url (and branch_name) must be non-null, and update the comment near the verify_pr_url call to reflect that verify_pr_url handles None safely (remove the warning about unexpected verifier crash) while pointing readers to the new Pydantic validator that enforces required fields for the "pr_created" status.backend/tests/api/openapi_snapshot.json (1)
4988-5012:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep
opencoderequired inVersionInfo.Line 4988 says
opencodeis retained for a backward-compatible wire shape, but Line 5010 removes it fromrequired. That changes the public schema for generated clients and schema-validated consumers of/api/version.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/api/openapi_snapshot.json` around lines 4988 - 5012, The OpenAPI snapshot removed "opencode" from the required list of the VersionInfo schema; restore it by adding "opencode" back into the schema's required array so VersionInfo again requires the opencode property (ensure the schema named VersionInfo or the object with properties including "cliff", "min_cli", "opencode", "schema_version" has "opencode" included alongside "cliff" in its required list).backend/cliff/main.py (1)
239-244:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t mark the provider as healthy before the async probe completes.
_refresh_ai_env_cache(verify=False)setsai_provider_credential_ok = Trueas soon as env resolution succeeds, and/healthtrusts that flag. On startup, a revoked or mistyped key will therefore reportai_provider_ready=trueuntil the backgroundverify=Truetask finishes.Suggested direction
- async def _refresh_ai_env_cache(*, verify: bool = True) -> None: + async def _refresh_ai_env_cache( + *, verify: bool = True, trusted_credential: bool = False + ) -> None: @@ - if not verify: + if not verify: # Trust the caller — the BYOK / autodetect-adopt routes # validated the key before invoking the save path, so a # second probe here would just duplicate that call. - app.state.ai_provider_credential_ok = True + app.state.ai_provider_credential_ok = trusted_credential return @@ - await _refresh_ai_env_cache(verify=False) + await _refresh_ai_env_cache(verify=False, trusted_credential=False)Use
trusted_credential=Trueonly from routes that already performed a live validation before calling the hook.Also applies to: 276-277
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/cliff/main.py` around lines 239 - 244, The function _refresh_ai_env_cache currently sets app.state.ai_provider_credential_ok = True when called with verify=False; remove that assignment so the provider is only marked healthy after a successful live probe (i.e., when verify=True or when an explicit trusted_credential flag supplied by a prior live validation). Update _refresh_ai_env_cache to set ai_provider_credential_ok only in the verification-success path (or when a trusted_credential parameter is True) and ensure routes that have already done live validation call the hook with trusted_credential=True instead of relying on the verify=False path.backend/tests/conftest.py (1)
35-42:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRestore the original lifespan context after the fixture.
This fixture mutates the module-level
appsingleton and never putsapp.router.lifespan_contextback. Any later test that expects the real startup/shutdown hooks will inherit the no-op lifespan and become order-dependent. Save the original value and restore it in afinallyblock.Suggested fix
`@pytest.fixture` def client(): """FastAPI test client with a no-op lifespan (ADR-0047 — in-process substrate, nothing to spawn).""" from cliff.main import app - app.router.lifespan_context = _noop_lifespan - with TestClient(app) as c: - yield c + original_lifespan = app.router.lifespan_context + app.router.lifespan_context = _noop_lifespan + try: + with TestClient(app) as c: + yield c + finally: + app.router.lifespan_context = original_lifespan🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/conftest.py` around lines 35 - 42, The client fixture mutates the module-level app by replacing app.router.lifespan_context with _noop_lifespan and never restores it; modify the client fixture so you first save the original = app.router.lifespan_context, set app.router.lifespan_context = _noop_lifespan, then yield the TestClient as before, and in a finally block restore app.router.lifespan_context back to the saved original to avoid leaking the no-op lifespan to other tests (refer to the client fixture, app.router.lifespan_context, _noop_lifespan, and TestClient).backend/cliff/workspace/context_builder.py (1)
97-128:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDelay the finding-state flip until workspace creation is durable.
Line 97 marks the finding
in_progressbefore any of the fallible filesystem work. Ifcreate(), manifest writing, orupdate_workspace_dir()fails, this leaves the finding advanced with no usable workspace, and the already-created workspace row can be orphaned too. Please movemark_started_on_workspace_create()until after the directory path is persisted, and add compensating cleanup for the DB row / directory on failure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/cliff/workspace/context_builder.py` around lines 97 - 128, Move the call to mark_started_on_workspace_create so it runs after the filesystem is created and the path is persisted (i.e., after _dir_manager.create(...) and await update_workspace_dir(db, workspace.id, str(ws_dir.root))). Wrap the ws_integrations write + update_workspace_dir block in a try/except so that any failure triggers compensating cleanup: remove the created workspace row and delete the filesystem directory (use your existing cleanup helpers or add functions such as delete_workspace(db, workspace.id) and _dir_manager.remove(workspace.id) / ws_dir.remove()), then re-raise the exception; only after update_workspace_dir succeeds call mark_started_on_workspace_create(db, finding.id). Ensure exceptions from _mcp_resolver.resolve_workspace are still handled as before.backend/cliff/agents/runtime/tools/bash.py (1)
63-68:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winDo not inherit the full server environment into agent shell commands.
Merging
os.environhere lets anybashtool call read and exfiltrate unrelated host secrets (env,$VAR, child-process inheritance), not just the workspace-scoped values inctx.deps.env_vars. That turns this tool into a server-secret disclosure path. Keep a small allowlisted runtime baseline (PATH,HOME, locale/temp vars, etc.) and overlay the workspace env on top of that instead.Suggested fix
- run_env = {**os.environ, **(ctx.deps.env_vars or {})} + safe_host_env = { + key: value + for key in ("PATH", "HOME", "LANG", "LC_ALL", "TMPDIR", "TMP", "TEMP") + if (value := os.environ.get(key)) is not None + } + run_env = {**safe_host_env, **(ctx.deps.env_vars or {})}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/cliff/agents/runtime/tools/bash.py` around lines 63 - 68, The current code builds run_env by merging the entire os.environ with ctx.deps.env_vars, which exposes all host environment variables to agent shell commands; replace this by constructing a minimal allowlisted baseline (e.g., PATH, HOME, TEMP/TMPDIR, LANG/LC_ALL/LC_CTYPE and other required locale/temp vars) and then overlay only ctx.deps.env_vars on top so unlisted host secrets are not inherited. Locate the run_env assignment and change it to create baseline_env (a dict with just the safe keys) and then set run_env = {**baseline_env, **(ctx.deps.env_vars or {})}; keep references to run_env and ctx.deps.env_vars unchanged elsewhere so command invocation uses the restricted environment.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/cliff/integrations/gateway.py`:
- Around line 263-264: The current env assignment uses truthiness (env =
resolved.get("environment") or resolved.get("env")) which treats an empty
stdio-MCP environment: {} as absent and falls back to legacy env; change the
logic to check keys explicitly so an explicit but empty "environment" wins: use
if "environment" in resolved to select resolved["environment"] (even if empty),
else if "env" in resolved select resolved["env"]; apply the same explicit-key
check in any related code path such as where _find_unresolved_placeholders()
resolves placeholders so it uses the same key-precedence behavior.
In `@backend/tests/agents/fixtures/plain_description_evals.json`:
- Line 144: The array value for the JSON key "fix_hint_keywords" contains a
likely typo "remov" — update the entry to the correct keyword "remove" unless
the misspelling was intentionally used for fuzzy-matching tests; if intentional,
add a clarifying comment near the "fix_hint_keywords" fixture explaining that
"remov" is deliberate and used to test fuzzy matching behavior so reviewers know
it's not a typo.
In `@backend/tests/api/openapi_snapshot.json`:
- Around line 3988-4018: Update the ProviderInfo schema so env and models are
marked required: modify the "ProviderInfo" object's "required" array to include
"env" and "models" (in addition to "id" and "name") for the schema instance
shown and make the same change for the other ProviderInfo occurrence referenced
(the one around lines 7593-7605) so generated clients treat env and models as
mandatory fields consumed by /api/settings/providers.
In `@cli/tests/test_cli.py`:
- Around line 129-155: The test's mocked /health response used by
cli.invoke(main, ["status"]) is missing the ai_provider_ready field that drives
the no_ai_provider_credential blocker; update the first httpx_mock.add_response
(the health fixture JSON) to include "ai_provider_ready": False so the status
flow (used with _stub_ai_integration_status and asserted via payload from
_last_json) exercises the explicit missing-credential route rather than relying
on an absent field.
---
Outside diff comments:
In `@backend/cliff/agents/runtime/tools/bash.py`:
- Around line 63-68: The current code builds run_env by merging the entire
os.environ with ctx.deps.env_vars, which exposes all host environment variables
to agent shell commands; replace this by constructing a minimal allowlisted
baseline (e.g., PATH, HOME, TEMP/TMPDIR, LANG/LC_ALL/LC_CTYPE and other required
locale/temp vars) and then overlay only ctx.deps.env_vars on top so unlisted
host secrets are not inherited. Locate the run_env assignment and change it to
create baseline_env (a dict with just the safe keys) and then set run_env =
{**baseline_env, **(ctx.deps.env_vars or {})}; keep references to run_env and
ctx.deps.env_vars unchanged elsewhere so command invocation uses the restricted
environment.
In `@backend/cliff/main.py`:
- Around line 239-244: The function _refresh_ai_env_cache currently sets
app.state.ai_provider_credential_ok = True when called with verify=False; remove
that assignment so the provider is only marked healthy after a successful live
probe (i.e., when verify=True or when an explicit trusted_credential flag
supplied by a prior live validation). Update _refresh_ai_env_cache to set
ai_provider_credential_ok only in the verification-success path (or when a
trusted_credential parameter is True) and ensure routes that have already done
live validation call the hook with trusted_credential=True instead of relying on
the verify=False path.
In `@backend/cliff/workspace/context_builder.py`:
- Around line 97-128: Move the call to mark_started_on_workspace_create so it
runs after the filesystem is created and the path is persisted (i.e., after
_dir_manager.create(...) and await update_workspace_dir(db, workspace.id,
str(ws_dir.root))). Wrap the ws_integrations write + update_workspace_dir block
in a try/except so that any failure triggers compensating cleanup: remove the
created workspace row and delete the filesystem directory (use your existing
cleanup helpers or add functions such as delete_workspace(db, workspace.id) and
_dir_manager.remove(workspace.id) / ws_dir.remove()), then re-raise the
exception; only after update_workspace_dir succeeds call
mark_started_on_workspace_create(db, finding.id). Ensure exceptions from
_mcp_resolver.resolve_workspace are still handled as before.
In `@backend/cliff/workspace/repo_workspace_runner.py`:
- Around line 291-303: The comment claims verify_pr_url needs guarding, but
verify_pr_url already accepts url: str | None and returns ok=False for None, and
repo_workspace_runner._finalize handles non-ok, so failing PR URLs won't crash;
to enforce a stronger contract add conditional validation on the
RepoActionOutput model so that when status == "pr_created" pr_url (and
branch_name) must be non-null, and update the comment near the verify_pr_url
call to reflect that verify_pr_url handles None safely (remove the warning about
unexpected verifier crash) while pointing readers to the new Pydantic validator
that enforces required fields for the "pr_created" status.
In `@backend/tests/api/openapi_snapshot.json`:
- Around line 4988-5012: The OpenAPI snapshot removed "opencode" from the
required list of the VersionInfo schema; restore it by adding "opencode" back
into the schema's required array so VersionInfo again requires the opencode
property (ensure the schema named VersionInfo or the object with properties
including "cliff", "min_cli", "opencode", "schema_version" has "opencode"
included alongside "cliff" in its required list).
In `@backend/tests/conftest.py`:
- Around line 35-42: The client fixture mutates the module-level app by
replacing app.router.lifespan_context with _noop_lifespan and never restores it;
modify the client fixture so you first save the original =
app.router.lifespan_context, set app.router.lifespan_context = _noop_lifespan,
then yield the TestClient as before, and in a finally block restore
app.router.lifespan_context back to the saved original to avoid leaking the
no-op lifespan to other tests (refer to the client fixture,
app.router.lifespan_context, _noop_lifespan, and TestClient).
In `@backend/tests/test_docker.py`:
- Around line 48-64: The test test_serves_index_html no longer verifies the SPA
fallback; update it to set mock_settings.static_dir to the tmp_path dist, build
a test client for the app mounted in cliff.main (re-import or call the app
factory used in cliff.main), then perform an HTTP GET on a non-API route such as
"/some/unknown/path" and assert the response status is 200 and the body contains
"Cliff" (or matches index.html content). Ensure you exercise the actual
routing/mount logic in cliff.main rather than only checking filesystem artifacts
so the fallback wiring is validated.
In `@backend/tests/test_executor.py`:
- Around line 372-385: The test test_parse_failure_marks_failed still configures
mock_pool.get_or_start but AgentExecutor no longer uses that path; replace the
dead setup by stubbing the PA seam instead: have the test call or patch
AgentExecutor._run_pa_executor (or use the helper _executor_with_pa) to return a
failed ParseResult (with an appropriate .error and no valid parse) so the
executor run goes through the parse-failure branch and sets run.status to FAILED
and run.last_error from parse_result.error; update assertions to rely on the
PA-returned ParseResult rather than the removed pool client.
In `@cli/cliff_cli/cli.py`:
- Around line 174-186: The status output currently emits the "opencode" key from
version (emit({... "opencode": version["opencode"], ...})), which is misleading
after the substrate migration; update the status emission in cli.py to either
remove the "opencode" field or rename it to a clearer name (e.g., "substrate")
and adjust any consumers/tests accordingly, and if the backend still returns
version["opencode"] for compatibility, add a brief comment documenting that
mapping in the emit block.
In `@frontend/src/components/ai-provider/ModelPicker.tsx`:
- Around line 282-337: The form currently submits any non-empty custom string;
update the onSubmit handler in ModelPicker.tsx (the inline onSubmit handler that
calls pick(custom.trim())) to first validate that custom.trim()
startsWith(`${provider}/`) and bail out (and optionally set a small validation
state) if it doesn't, then call pick only when the prefix matches; also update
the button disabled expression (currently disabled={!custom.trim() ||
setModel.isPending}) to include the same prefix check (e.g.,
disabled={!custom.trim().startsWith(`${provider}/`) || setModel.isPending}) so
the client enforces the advertised provider/... requirement before making the
API call.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e193a59b-46bb-4247-aaef-23bda0ae4937
📒 Files selected for processing (123)
.dockerignore.github/workflows/install-smoke.yml.gitignore.opencode-version.opencode/agents/security-orchestrator.mdCLAUDE.mdCONTRIBUTING.mdKNOWN_ISSUES.mdNOTICEREADME.mdROADMAP.mdTHIRD-PARTY-LICENSES.mdbackend/cliff/agents/__init__.pybackend/cliff/agents/errors.pybackend/cliff/agents/executor.pybackend/cliff/agents/runtime/deps.pybackend/cliff/agents/runtime/no_tools.pybackend/cliff/agents/runtime/provider.pybackend/cliff/agents/runtime/remediation_executor.pybackend/cliff/agents/runtime/tools/__init__.pybackend/cliff/agents/runtime/tools/bash.pybackend/cliff/agents/runtime/tools/mcp.pybackend/cliff/agents/runtime/tools/permissions.pybackend/cliff/agents/template_engine.pybackend/cliff/agents/templates/dependabot_config_generator.md.j2backend/cliff/agents/templates/enricher.md.j2backend/cliff/agents/templates/evidence_collector.md.j2backend/cliff/agents/templates/exposure_analyzer.md.j2backend/cliff/agents/templates/orchestrator.md.j2backend/cliff/agents/templates/owner_resolver.md.j2backend/cliff/agents/templates/remediation_executor.md.j2backend/cliff/agents/templates/remediation_planner.md.j2backend/cliff/agents/templates/security_md_generator.md.j2backend/cliff/agents/templates/validation_checker.md.j2backend/cliff/ai/catalog.pybackend/cliff/ai/models.pybackend/cliff/ai/service.pybackend/cliff/api/_engine_dep.pybackend/cliff/api/routes/ai_integrations.pybackend/cliff/api/routes/health.pybackend/cliff/api/routes/settings.pybackend/cliff/api/routes/version.pybackend/cliff/config.pybackend/cliff/engine/__init__.pybackend/cliff/engine/client.pybackend/cliff/engine/config_manager.pybackend/cliff/engine/models.pybackend/cliff/engine/pool.pybackend/cliff/engine/process.pybackend/cliff/integrations/gateway.pybackend/cliff/main.pybackend/cliff/models/__init__.pybackend/cliff/workspace/context_builder.pybackend/cliff/workspace/repo_workspace_runner.pybackend/cliff/workspace/workspace_dir.pybackend/cliff/workspace/workspace_dir_manager.pybackend/pyproject.tomlbackend/tests/agents/conftest.pybackend/tests/agents/fixtures/plain_description_evals.jsonbackend/tests/agents/test_deferred_tools_persist.pybackend/tests/agents/test_executor_provider_chain.pybackend/tests/api/openapi_snapshot.jsonbackend/tests/conftest.pybackend/tests/e2e/__init__.pybackend/tests/e2e/conftest.pybackend/tests/e2e/test_agent_pipeline_e2e.pybackend/tests/e2e/test_health_e2e.pybackend/tests/e2e/test_process_pool_e2e.pybackend/tests/e2e/test_repo_workspace_agents.pybackend/tests/e2e/test_settings_e2e.pybackend/tests/integration/test_permission_flow.pybackend/tests/integration/test_rate_limit_backoff.pybackend/tests/integration/test_workspace_npm_cache_isolation.pybackend/tests/integration/test_workspace_repo_url_overrides_global.pybackend/tests/test_agent_template_engine.pybackend/tests/test_ai_catalog.pybackend/tests/test_ai_phase_f_wiring.pybackend/tests/test_ai_service.pybackend/tests/test_config.pybackend/tests/test_config_manager.pybackend/tests/test_context_builder.pybackend/tests/test_docker.pybackend/tests/test_engine_client.pybackend/tests/test_engine_process.pybackend/tests/test_executor.pybackend/tests/test_gateway.pybackend/tests/test_mcp_gateway_integration.pybackend/tests/test_permission_client.pybackend/tests/test_process_pool.pybackend/tests/test_repo_action_templates.pybackend/tests/test_routes_ai_integrations.pybackend/tests/test_routes_ai_openrouter.pybackend/tests/test_routes_settings.pybackend/tests/test_routes_version.pybackend/tests/test_workspace_dir.pybackend/tests/test_workspace_integrations.pybackend/tests/test_workspace_repo_creation.pycli/cliff_cli/cli.pycli/cliff_cli/daemon.pycli/cliff_cli/process_sweep.pycli/cliff_cli/updater.pycli/tests/test_cli.pycli/tests/test_daemon.pycli/tests/test_process_sweep.pycli/tests/test_updater.pydocker/Dockerfiledocker/entrypoint.shfrontend/src/__tests__/PostureCheckItem.state-machine.test.tsxfrontend/src/api/client.tsfrontend/src/api/hooks.tsfrontend/src/api/types.tsfrontend/src/components/ai-provider/AIProviderStatus.tsxfrontend/src/components/ai-provider/ModelPicker.tsxfrontend/src/components/dashboard/PostureCard.tsxfrontend/src/test/msw/sessionHandlers.tsopencode.jsonplugins/cliff-security/skills/cliff-security/knowledge/install.mdplugins/cliff-security/skills/cliff-security/knowledge/troubleshooting.mdscripts/build-tarball.shscripts/dev.shscripts/install-local.shscripts/install-opencode.shtests/install/smoke.sh
💤 Files with no reviewable changes (55)
- .opencode-version
- backend/cliff/agents/templates/evidence_collector.md.j2
- backend/cliff/agents/templates/validation_checker.md.j2
- NOTICE
- backend/tests/integration/test_workspace_npm_cache_isolation.py
- backend/cliff/engine/init.py
- backend/cliff/agents/templates/owner_resolver.md.j2
- .github/workflows/install-smoke.yml
- backend/tests/test_repo_action_templates.py
- backend/cliff/workspace/workspace_dir.py
- .opencode/agents/security-orchestrator.md
- backend/cliff/agents/templates/exposure_analyzer.md.j2
- backend/tests/e2e/test_repo_workspace_agents.py
- backend/cliff/agents/templates/enricher.md.j2
- backend/cliff/agents/templates/dependabot_config_generator.md.j2
- backend/cliff/engine/models.py
- backend/tests/integration/test_rate_limit_backoff.py
- scripts/install-opencode.sh
- backend/cliff/agents/template_engine.py
- frontend/src/api/hooks.ts
- backend/tests/e2e/test_process_pool_e2e.py
- .dockerignore
- tests/install/smoke.sh
- backend/tests/agents/test_executor_provider_chain.py
- CONTRIBUTING.md
- backend/tests/e2e/conftest.py
- backend/cliff/agents/templates/remediation_executor.md.j2
- backend/tests/test_config_manager.py
- docker/entrypoint.sh
- backend/cliff/agents/templates/security_md_generator.md.j2
- backend/tests/e2e/test_settings_e2e.py
- backend/cliff/agents/templates/orchestrator.md.j2
- backend/cliff/agents/templates/remediation_planner.md.j2
- backend/cliff/engine/client.py
- backend/tests/e2e/test_health_e2e.py
- backend/tests/test_ai_phase_f_wiring.py
- backend/tests/test_permission_client.py
- backend/tests/test_process_pool.py
- opencode.json
- frontend/src/test/msw/sessionHandlers.ts
- backend/cliff/engine/process.py
- backend/tests/e2e/test_agent_pipeline_e2e.py
- frontend/src/api/client.ts
- backend/tests/test_routes_ai_integrations.py
- cli/tests/test_updater.py
- backend/tests/test_agent_template_engine.py
- backend/tests/test_engine_client.py
- backend/cliff/engine/pool.py
- backend/cliff/api/_engine_dep.py
- backend/cliff/engine/config_manager.py
- backend/tests/test_gateway.py
- backend/tests/test_routes_ai_openrouter.py
- backend/tests/test_workspace_dir.py
- backend/tests/agents/test_deferred_tools_persist.py
- backend/tests/test_engine_process.py
| "must_contain_any": ["ci-deploy", "admin", "IAM"], | ||
| "must_not_contain_regex": ["CWE-\\d+", "CVSS:\\d"], | ||
| "fix_hint_keywords": ["replace", "scoped", "least", "narrow", "remove", "attach a"], | ||
| "fix_hint_keywords": ["replace", "scoped", "least", "narrow", "remov", "restrict", "attach a"], |
There was a problem hiding this comment.
Verify the misspelled keyword is intentional.
The keyword was changed from "remove" to "remov". This appears to be a typo, as the other keywords in the array are properly spelled. If this is intentional test data to verify fuzzy matching behavior, please clarify with a comment.
📝 Proposed fix if this is a typo
- "fix_hint_keywords": ["replace", "scoped", "least", "narrow", "remov", "restrict", "attach a"],
+ "fix_hint_keywords": ["replace", "scoped", "least", "narrow", "remove", "restrict", "attach a"],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "fix_hint_keywords": ["replace", "scoped", "least", "narrow", "remov", "restrict", "attach a"], | |
| "fix_hint_keywords": ["replace", "scoped", "least", "narrow", "remove", "restrict", "attach a"], |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/tests/agents/fixtures/plain_description_evals.json` at line 144, The
array value for the JSON key "fix_hint_keywords" contains a likely typo "remov"
— update the entry to the correct keyword "remove" unless the misspelling was
intentionally used for fuzzy-matching tests; if intentional, add a clarifying
comment near the "fix_hint_keywords" fixture explaining that "remov" is
deliberate and used to test fuzzy matching behavior so reviewers know it's not a
typo.
Comment on lines
129
to
+155
| httpx_mock.add_response( | ||
| url="http://test-server/health", | ||
| json={"cliff": "ok", "opencode": "unavailable", "opencode_version": "1.3.2", "model": ""}, | ||
| json={ | ||
| "cliff": "ok", | ||
| "opencode": "ok", | ||
| "opencode_version": "pydantic-ai 1.98.0", | ||
| "model": "", | ||
| }, | ||
| ) | ||
| httpx_mock.add_response( | ||
| url="http://test-server/api/version", | ||
| json={ | ||
| "cliff": "0.1.1-alpha", | ||
| "opencode": "1.3.2", | ||
| "opencode": "pydantic-ai 1.98.0", | ||
| "schema_version": "1", | ||
| "min_cli": "0.1.0", | ||
| }, | ||
| ) | ||
| # Engine is down → AI integration also has nothing to report. model=None | ||
| # keeps the ``no_llm_model_configured`` blocker the test asserts. | ||
| _stub_ai_integration_status(httpx_mock, model=None) | ||
| res = cli.invoke(main, ["status"]) | ||
| assert res.exit_code == 0 | ||
| payload = _last_json(res.stdout) | ||
| assert payload["ready"] is False | ||
| assert "opencode_engine_unavailable" in payload["blockers"] | ||
| assert "no_llm_model_configured" in payload["blockers"] | ||
| assert "no_ai_provider_credential" in payload["blockers"] | ||
| # No engine concept anymore. | ||
| assert "opencode_engine_unavailable" not in payload["blockers"] |
There was a problem hiding this comment.
Include ai_provider_ready: False in this health fixture.
This test now asserts no_ai_provider_credential, but the mocked /health response never returns the field that should drive that blocker. As written, it can pass via implicit missing-field handling instead of the real route contract.
Suggested fix
httpx_mock.add_response(
url="http://test-server/health",
json={
"cliff": "ok",
"opencode": "ok",
"opencode_version": "pydantic-ai 1.98.0",
"model": "",
+ "ai_provider_ready": False,
},
)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cli/tests/test_cli.py` around lines 129 - 155, The test's mocked /health
response used by cli.invoke(main, ["status"]) is missing the ai_provider_ready
field that drives the no_ai_provider_credential blocker; update the first
httpx_mock.add_response (the health fixture JSON) to include
"ai_provider_ready": False so the status flow (used with
_stub_ai_integration_status and asserted via payload from _last_json) exercises
the explicit missing-credential route rather than relying on an absent field.
Full code review of the OpenCode→Pydantic AI migration before merge to main, plus the open Baz/CodeRabbit threads. Real bugs + targeted cleanups. Correctness: - permissions: narrow `_auto_approved` so repo-action runs pre-approve ONLY the gated-bash ask bucket (rm, git reset, …). It no longer swallows the classifier's safe-default ask buckets (external_directory, edit-escape, mcp/unknown tools, empty bash) — those stay approval-gated so a confused repo-action fails closed instead of silently executing review-routed calls (CodeRabbit major). New lock-down tests cover the four buckets. - ingest_worker: reconcile a job's pinned model with the active provider's env (cross-provider pins now fall back to the active model instead of failing auth every chunk), validate the model is `<provider>/<model>` before processing (a bare override no longer loops fail→pending), and stop counting error-only chunks as completed (Baz high ×2). - executor: resume with a missing message-history now fails inside the try (marker already cleared) instead of raising before it — the run is marked failed rather than wedged at running/permission_pending forever. - provider: strip a trailing `/v1` from OLLAMA_BASE_URL before appending it, avoiding a doubled `/v1/v1` → 404 when an operator includes it. - normalizer: cap the model's output array at the input count so a hallucinated overflow can't inject more findings than were submitted. Cleanup: - Delete the dead duplicate bash classifier in executor.py (the live gate is permissions.classify_tool_request); its lock-down tests were asserting against the dead copy — drop them (coverage lives in test_permissions.py). - Delete orphaned catalog.provider_env_var_names() (subprocess-env scrubbing; no subprocess remains) + its test. - Extract the token-aware clone snippet shared by both repo-action prompts into `_clone_block(branch)` so the auth logic can't drift. - Hoist the duplicated eval env/model setup into tests/agents/eval_utils.py. - Fix bash.py docstring (deny raises ModelRetry, not ValueError) and remove stale OpenCode references in test docstrings. - Update the cliff-security onboarding knowledge doc off the removed /api/settings/api-keys* routes onto /api/integrations/ai/byok + /status. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
Pre-merge review pass — resolutions (commit ca18dbb)Ran a fresh full review of the whole Fixed in ca18dbb
Already handled / intentional — no change (rationale)
|
Comment on lines
+321
to
+325
| if len(outputs) > len(raw_data): | ||
| errors.append( | ||
| f"Model returned {len(outputs)} findings for {len(raw_data)} " | ||
| f"input item(s); keeping the first {len(raw_data)}." | ||
| ) |
There was a problem hiding this comment.
Can we match outputs by source_id before outputs[: len(raw_data)], otherwise an extra hallucinated row can shift later valid findings and ingest_worker never sees them?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
backend/cliff/integrations/normalizer.py around lines 321-325 inside normalize_findings,
you currently cap overflow model outputs by array position (outputs = outputs[:
len(raw_data)]), which can drop valid findings when the model returns hallucinated extra
rows before the real ones. Refactor this logic to align NormalizedFinding rows to the
corresponding input items using source_id (the prompt/payload requires preserving it):
build a map of input items by source_id, keep only outputs whose source_id matches an
input, and record an error for any unmatched extras instead of truncating by index.
Ensure that all matched valid findings are preserved for the batch and that
overflow/mismatch cases are surfaced in errors for observability.
…ontract
From CodeRabbit's full re-scan of the PR:
- gateway: use explicit-key precedence for the MCP env block. An empty
`environment: {}` was falsy under `or`, so it silently fell back to the
legacy `env` (and `_find_unresolved_placeholders` repeated the mistake).
An explicit (even empty) `environment` now wins in both spots.
- models: make `env` and `models` required on `ProviderInfo` —
`GET /api/settings/providers` always emits both and the UI/CLI consume
them, so generated clients must treat them as present, not optional.
Regenerated the OpenAPI snapshot + frontend types.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
Second review pass — CodeRabbit full re-scan (commit 0e1acc3)Both bot checks now pass on the prior commit; CodeRabbit's deeper re-scan surfaced a few more. Triage: Fixed in 0e1acc3:
Declined (noise / adequate as-is):
All 1417 backend tests pass; ruff + tsc clean. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
What
Consolidated continuation of the OpenCode → Pydantic AI migration (ADR-0047 / IMPL-0022), stacked on PR #1 (no-tools agents) and PR #2 (executor). This branch lands #3b + #3c + the safe first slices of #3d; the load-bearing singleton/
engine/removal is intentionally deferred to a focused follow-up (see Scope below). Intended as a stacked base, not for immediate merge.Commits
normalizer_agent.py);normalize_findings(source, raw, *, env, model); resolver-injectedingest_worker. Hand-rolled retry/JSON-extraction gone; partial-success(valid, errors)preserved.bash/edit/read/ghtools;WorkspaceDeps.auto_approvefor the one-shot pre-approved run; runner rewired off the per-workspace pool. + a latentbashbug fix: it replaced the process env (strippingPATH) instead of merging overos.environ./healthoff the singleton (HealthStatus→cliff/models, shape kept, reports the PA substrate version).Scope — what's deferred
#3d's singleton +engine/deletion is not in this PR. Removing the singleton is app-startup-critical and entangled with live UI/CLI contracts (the/settings/providersmodel-picker catalog,ai/serviceauth, themain.pylifespan). It deserves a focused, separately-tested pass. Consequently:app.state.process_poolis spawned-but-unused here (nothing callsget_or_startafter the chat routes were removed). Harmless — it's removed in the substrate-deletion follow-up.cli.py'shealth.get("opencode") != "ok"blocker is now always-false (substrate is in-process) — a no-op until that follow-up cleans it up.frontend/src/api/types.tsis intentionally untouched (already ~2000 lines stale; resynced separately).Validation
pytest -m 'not e2e': 1555 passed, ruff clean. New deterministic tests viaFunctionModelfor the normalizer, repo-action runner (incl. the B16 PR-URL verification),auto_approvegating, the no-provider short-circuit, and status coercion.tscclean.Review
/code-review(high-effort, 7 angles) + CodeRabbit + baz: one confirmed bug — repo-action prompts usedgh -C repo(the GitHub CLI has no-Cflag) — fixed by routing PR-create throughcd repo && gh pr create. Review comments addressed: status coercion, no-provider busy-retry, and conditional clone-URL auth fixed in-code; the rest answered inline (single-provider context, by-design isolation trade, deferred DRY cleanups).🤖 Generated with Claude Code
Generated description
Below is a concise technical summary of the changes proposed in this PR:
Migrate the finding normalizer and repo-action generators onto in-process Pydantic AI agents wired through
WorkspaceDeps, provider resolvers, and the shared executor tools so they keep their structured outputs while honoring repo-action pre-approval. Rewire ingest/runner plumbing plus the workspace/health surface to read app-level AI env/model caches, report the Pydantic AI substrate, and drop the obsolete session/chat plumbing that depended on the OpenCode singleton.WorkspaceDeps, provider resolvers, and the executor tools while keeping structured outputs, auto-approval, and new error handling in sync.Modified files (10)
Latest Contributors(1)
/healthreporting the new in-process Pydantic AI version viaHealthStatus, and update CLI/frontend/OpenAPI clients to the simplified contract.Modified files (7)
Latest Contributors(1)
Modified files (12)
Latest Contributors(1)
Summary by CodeRabbit
New Features
Removals
Infrastructure