Skip to content

feat(review): quest review runs opencode reviewers - #603

Merged
kixelated merged 2 commits into
mainfrom
quest/m2/review-harnesses
Nov 10, 2026
Merged

kixelated merged 2 commits into
mainfrom
quest/m2/review-harnesses

Conversation

@kixelated

@kixelated kixelated commented Nov 10, 2026 •

Copy link
Copy Markdown
Collaborator

Finishes quest/m2/review-harnesses.md: quest review --harness opencode reviews a branch as the Claude Code and Codex reviewers do.

Summary

  • quest review --harness opencode --model <provider/model> --effort <variant> runs opencode run --standalone --format json for the review turn and resumes that session (--session) for the schema turn. opencode has no review mode, so the first turn is a review prompt plus the usual scope, REVIEW.md, and responses; opencode run takes no schema, so the findings turn's prompt carries it and the reply is parsed and checked by verdict as before (a Markdown fence around the JSON is stripped).
  • Read-only and running nothing from the head:
    • OPENCODE_DISABLE_PROJECT_CONFIG and OPENCODE_DISABLE_CLAUDE_CODE keep out the checkout's opencode.json, .opencode/ plugins, MCP servers, shell, AGENTS.md, and skills. The base's instructions arrive as an instructions file.
    • Every tool but shell is denied (* deny, then skill deny, then shell allow). These rules come last in OPENCODE_CONFIG_CONTENT, so none of the user's own rules can outrank them. No LSP, formatter, snapshot, or share.
    • The shell is a script that runs each command under sandbox-runtime (srt): no network, reads only the checkout and its Git directories (the rest of /home, /tmp, and the user's home are hidden), writes only a private temp directory, starts from a cleared environment with no provider keys, and uses a PATH with no directory inside the checkout. On Linux, every round first probes that srt refuses Unix sockets, which needs its seccomp helper, and refuses to run otherwise. src/cli/sandbox.ts and nix/sandbox-runtime.nix come back from kix's runner (git show 7fe97931^:src/runner/sandbox.ts), trimmed to a read-only reviewer.
  • Logged out and usage limit are reported separately: if opencode models doesn't list the model, the round fails as logged out (exit 2). A provider.auth error also exits 2. opencode's limit errors (provider.quota, go-limit, free-limit, chatgpt-limit, budget-limit, model-budget-limit, or a LIMIT message) exit 3 through ReviewLimitError, with the reset taken from resetsAt/retryAfterMs. A retry event carrying a limit stops the CLI instead of letting it wait for the reset.
  • run() sets PWD to the checkout for every reviewer CLI, because opencode runs in PWD, not in its working directory.
  • The dev shell gains srt (with the seccomp helper), and on Linux bubblewrap and socat.
  • just review-sandbox covers opencode. Its planted MCP server, two plugins, and shell all fire in the control run and none fire under quest review. A further test runs writes, a home read, and a network request through the sandbox shell directly (all blocked), and has the model report $SANDBOX_RUNTIME to show its commands went through srt. QUEST_LIVE_OPENCODE_MODEL picks the model (default opencode-go/deepseek-v4.1-flash#high).
  • The quest file is deleted, the link to it is removed from council's Required, and opencode is added to the model check's sandbox-test step.

Public API

  • quest review --harness accepts opencode, which no size's list picks, so it needs --model and --effort. The ## Review record names it opencode.
  • Exported: REVIEW_HARNESSES, ReviewHarness, Reviewing, opencodeTurn, opencodeReset. reviewerEnvironment now takes the model. Nothing is on the wire. No breaking change for the existing harnesses.

Choices made without asking

  • I drove opencode run rather than opencode serve plus @opencode/client (which kix used). This fits the existing spawn-per-turn reviewer and adds no dependency. What it gives up: there is no API check that no foreign config loaded. The live test's control run covers that instead.
  • Only the shell is allowed, with no read, grep, or glob tools. kix needed a path-checking plugin to keep opencode's unconfined file tools in the checkout. Here cat and rg run inside the OS sandbox, so the plugin isn't needed.
  • opencode keeps the user's own config (custom providers, gateways) and login, as the other reviewers reach their models the way the user's own CLI does.
  • No opencode entry in REVIEWERS. Which model the council's DeepSeek member runs is council's call (it names opencode-go/deepseek-v4-pro at high).

Validation

  • just check's steps all pass, run directly: tsc, prettier, quest check on both trees, nixfmt, and actionlint. The gated just check/just test waited more than 2 hours for host room (load average 100 to 145) and were stopped, so just test as a whole, including the install scripts and lifecycle builds, did not run here. These suites pass with --testTimeout 60000: review, sandbox, cli, snapshot, and usage (143 tests). At the default 5s, five review tests timed out on the loaded host.
  • just review-sandbox with opencode 2.2.6, codex, and Claude Code: opencode 4/4 and Codex 3/3 pass.
  • Claude: under this agent's host-managed ANTHROPIC_BASE_URL, copied logins can't authenticate. With that unset, 3 of 4 pass. The control fails only because Claude Code's planted apiKeyHelper no longer fires. That failure is unrelated to this change: it fails the same way whether or not the control sets PWD, and this PR doesn't touch Claude's flags. Follow-up below.

Limitations

  • On Linux hosts that restrict unprivileged user namespaces through AppArmor (Ubuntu 23.10+ default), the Nix-store bwrap may be refused. When that happens the socket probe fails and the round stops with that error, rather than running unsandboxed.
  • Models without reasoning variants can't be named, since --effort always becomes #variant.

Follow-ups

  • Plan: the Claude Code live control no longer sees the planted apiKeyHelper fire (Claude Code 2.1.x print mode with a stored login). The model check says to plan a quest for any just review-sandbox failure. Recommendation: plan it under the model check, to either drop that plant from CONTROL or find the release change.

Models

Size: M
Implementer: Claude Code, claude-opus-5-5, medium effort

(Written by Claude Opus 5.5 with medium)

Review

Rewritten by quest review after each round.

  • Reviewer: Codex, gpt-6.1-sol, medium effort
  • Session: 01a1c52d-dc47-7d11-8108-b38af3e7b2da, round 1
  • Head: fec1772
  • Base: origin/main at b6b7197
  • Score: 8/10, The implementation appears sound by inspection with no actionable defects identified, but builds, tests, and live sandbox checks were not run, as instructed.
  • Findings: none

@pullfrog

pullfrog Bot commented Nov 10, 2026

Copy link
Copy Markdown

Pullfrog  | Review this ➔

`quest review --harness opencode --model <provider/model> --effort <variant>`
reviews read-only: opencode loads no project config or Claude Code files,
every tool but its shell is denied, and the shell runs each command under
sandbox-runtime with no network, reading only the checkout and its Git
directories and writing only a private directory. The sandbox and the
seccomp-built srt package are restored from kix's runner.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title quest review runs opencode reviewers feat(review): quest review runs opencode reviewers Nov 10, 2026
@kixelated
kixelated marked this pull request as ready for review November 10, 2026 11:19
@coderabbitai

coderabbitai Bot commented Nov 10, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 42 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: bbd13f75-ec7f-434d-a8fd-9a7c794a201c
Commits

Reviewing files that changed from the base of the PR and between eaa0831 and fec1772.

Files selected for processing: 14

  • README.md
  • flake.nix
  • justfile
  • nix/sandbox-runtime.nix
  • quest/m1/model-check.md
  • quest/m2/council.md
  • quest/m2/review-harnesses.md
  • src/cli/main.ts
  • src/cli/review-record.ts
  • src/cli/review.ts
  • src/cli/sandbox.ts
  • test/live/review-sandbox.test.ts
  • test/review.test.ts
  • test/sandbox.test.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit: fec1772

[P1] src/cli/sandbox.ts:60–65: enforce the advertised checkout-only read boundary. allowRead grants exceptions to the listed denyRead roots; it does not deny reads elsewhere. For a checkout under /workspace, a shell command can still read another project's /workspace/other/.env (likewise user data under /mnt or /srv). The code also explicitly acknowledges that an existing shared /tmp/claude remains readable despite denying /tmp. A reviewed head can induce those reads, and shell output goes back to the provider even though the shell itself has no network. Restrict non-runtime reads to the checkout/Git/private-temp paths, and refuse or isolate the runtime's shared /tmp/claude mount instead of only denying writes. Add direct-shell negative tests for a sibling checkout outside the hidden roots and a planted file in /tmp/claude; the current home-read test misses both cases.

Overall direction: the harness integration and project-config suppression are well structured, but the read-isolation promise needs tightening before relying on this for untrusted changes. Verification limits: GitHub-only static review of the patches and sandbox configuration; no builds, tests or live sandbox probes run independently. The PR itself reports incomplete full-suite validation. Rechecked open/non-draft state, full head SHA and reviews immediately before posting.

@kixelated
kixelated added this pull request to the merge queue Nov 10, 2026
Merged via the queue into main with commit 20c4dce Nov 10, 2026
22 checks passed
@kixelated
kixelated deleted the quest/m2/review-harnesses branch November 10, 2026 16:16
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