Skip to content

fix(review): replace built-in plan agent with custom review agent#49

Merged
JohnnyVicious merged 1 commit intomainfrom
fix/custom-review-agent
Apr 12, 2026
Merged

fix(review): replace built-in plan agent with custom review agent#49
JohnnyVicious merged 1 commit intomainfrom
fix/custom-review-agent

Conversation

@JohnnyVicious
Copy link
Copy Markdown
Owner

Summary

  • Ships a dedicated review OpenCode agent inside the plugin (at plugins/opencode/opencode-config/agent/review.md) and threads OPENCODE_CONFIG_DIR through ensureServer so OpenCode discovers it when the companion spawns the server.
  • Both /opencode:review and /opencode:adversarial-review (and the stop-review-gate hook) now route through the new agent via a shared resolveReviewAgent helper, with a fallback to build + per-call tools restrictions when the custom agent isn't available on a pre-running server.
  • Also fixes a latent bug surfaced during end-to-end testing: --model was passed as a plain string but OpenCode's HTTP API expects {providerID, modelID}, so every --model invocation was silently erroring out with HTTP 400. A new parseModelString helper converts at the CLI boundary.

Why

OpenCode's built-in plan agent isn't just read-only — it injects a synthetic user-message directive ("Plan mode ACTIVE — STRICTLY FORBIDDEN… produce an implementation plan") on every turn, which dominates whatever system prompt the plugin sends. The result: users running /opencode:review or /opencode:adversarial-review got back an implementation plan instead of the requested review. This has been broken since the plugin was first ported from codex-plugin-cc, where the equivalent call used `sandbox: "read-only"` — a syscall-level filesystem sandbox with no prompt-layer effects. The opencode port translated that to `agent: "plan"` under the assumption that plan was the read-only counterpart to build, but missed the prompt-layer side effect.

A custom review agent sidesteps the injection entirely (insertReminders() in OpenCode only fires on agent.name === \"plan\"), while still enforcing read-only behavior at the permission layer.

Test plan

  • 10 unit tests for `resolveReviewAgent` (tests/review-agent.test.mjs) covering array/object `listAgents` shapes, missing-agent fallback, throw-on-list fallback, and tool-mutation safety
  • 9 unit tests for `parseModelString` (tests/model.test.mjs) covering provider splitting, nested model ids, trimming, empty/invalid input, non-string rejection
  • Full test suite: 102/102 passing (was 83 pre-fix)
  • CI syntax-check list updated to cover new files
  • End-to-end verified: ran the companion script directly from this branch with `CLAUDE_PLUGIN_ROOT` override and `--model openrouter/anthropic/claude-haiku-4.5`. The server spawned with our bundled config dir, `resolveReviewAgent` selected the custom agent (job log shows `agent: review`), and the review came back as a structured `needs-attention` verdict with two findings — review-shaped, not plan-shaped.
  • Manually inspected `GET /agent` on a test server started with `OPENCODE_CONFIG_DIR` pointing at the bundled config — `review` appears in the agent list with the expected read-only permissions (`*: deny` base, `edit/write/patch/bash/webfetch/task` explicit deny, `read/grep/glob/list` allow) and the neutral prompt body from `review.md`.

Known limitation

If a user already has `opencode serve` running in another terminal when the plugin runs, `ensureServer` reuses the existing process and `OPENCODE_CONFIG_DIR` is never set, so the custom agent isn't loaded. The fallback catches this via `listAgents()`, uses `build` with per-call tool overrides, and logs a warning telling the user to stop the pre-existing server. Correctness is preserved; only the prompt layer is slightly less clean than the preferred path.

OpenCode's built-in `plan` agent isn't just read-only — it injects a
synthetic user-message directive on every turn ("Plan mode ACTIVE —
STRICTLY FORBIDDEN... produce an implementation plan") which overrides
whatever system prompt the caller sends and makes the model return an
implementation plan instead of the requested review. This has been the
root cause of `/opencode:review` and `/opencode:adversarial-review`
returning implementation plans since the plugin was first ported from
codex-plugin-cc, where the equivalent was `sandbox: "read-only"` — a
syscall-level filesystem sandbox that doesn't touch the system prompt.

This commit ships a dedicated `review` agent inside the plugin and
threads OPENCODE_CONFIG_DIR through `ensureServer` so OpenCode discovers
it when the companion spawns the server. The agent is read-only at the
permission layer (deny `*`, allow only read/grep/glob/list) and has a
neutral prompt body that tells the model to follow the caller's brief
exactly without inventing implementation steps.

Both review handlers and the stop-review-gate hook now resolve the
agent through a shared `resolveReviewAgent` helper. When the custom
agent isn't available (user already had `opencode serve` running
without our config dir), the helper falls back to `build` + per-call
`tools: {write:false, edit:false, ...}` overrides with a warning log,
so reviews still work correctly even if the preferred path isn't taken.

Also fixes a latent bug surfaced during end-to-end testing: the CLI
passed `--model` as a plain string through to OpenCode's HTTP API,
which expects `{providerID, modelID}` and rejected every invocation
with HTTP 400. A new `parseModelString` helper splits on the first
slash (preserving nested model ids like `openrouter/anthropic/...`)
and callers convert at the boundary.

Tests:
- 10 unit tests for `resolveReviewAgent` covering array/object list
  shapes, missing agent fallback, listAgents errors, and tool mutation
  safety
- 9 unit tests for `parseModelString` covering provider splitting,
  trimming, empty/invalid input, and non-string rejection
- End-to-end verified by running the companion script against this
  branch's own working tree with `--model openrouter/anthropic/claude-haiku-4.5`:
  server spawned with OPENCODE_CONFIG_DIR, custom agent was resolved
  (job log shows `agent: review`), adversarial review returned with a
  structured needs-attention verdict + two findings (review-shaped, not
  plan-shaped)
@JohnnyVicious JohnnyVicious merged commit 866451c into main Apr 12, 2026
1 check passed
@JohnnyVicious JohnnyVicious deleted the fix/custom-review-agent branch April 12, 2026 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant