fix(opencode): fail closed reviewer inspection - #2221
Merged
Conversation
📝 WalkthroughWalkthroughOpenCode review tasks now reject unsupported immutable inspection before reviewer or native process launch. OpenCode receives denied Bash and edit permissions. KiloCode removes the OpenCode-only plugin. Windows E2E coverage verifies rejection and unchanged review state. ChangesOpenCode unsupported inspection
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ReviewTask
participant ReviewResultPlugin
participant OpenCode
participant NativeGit
ReviewTask->>ReviewResultPlugin: validate immutable review binding
ReviewResultPlugin-->>ReviewTask: return unsupported-capability
ReviewResultPlugin--x OpenCode: do not launch reviewer
ReviewResultPlugin--x NativeGit: do not run inspection process
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Pull request overview
Disables immutable candidate inspection for OpenCode reviewers by failing closed before any reviewer subagent is launched, due to OpenCode 1.18.10’s inability to securely bind provider-injected dynamic values to a child-session Bash permission. This keeps other runtimes (notably Kilocode and native inspect-candidate) unchanged while preventing insecure or misleading “successful” inspection attempts.
Changes:
- Make the OpenCode review-result plugin reject reviewer tasks up front with a typed
unsupported-capabilityoutcome (no reviewer launch). - Generate OpenCode reviewer agents without Bash capability and without inspection allow-rules; update prompt text to explicitly stop and return incomplete.
- Add/adjust tests (including a Windows-only real OpenCode E2E proof) and pin OpenCode to 1.18.10 in both code and CI, with a guard that the E2E test name is present.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
internal/assets/opencode/plugins/review-result-artifacts.ts |
Fail closed in tool.execute.before for reviewer tasks with a typed unsupported-capability error before any launch/capture path. |
internal/components/sdd/inject.go |
Generate OpenCode reviewer agents as unsupported (no Bash, deny-only permissions, updated prompt) while preserving non-OpenCode behavior. |
internal/components/sdd/boundedreview.go |
Add a dedicated unsupported-capability reviewer prompt builder for OpenCode. |
internal/components/sdd/review_ledger_contract_test.go |
Update contract expectations for OpenCode reviewers (unsupported) and add a Kilocode settings baseline guard. |
testdata/golden/sdd-opencode-multi-settings.golden |
Update golden settings to reflect deny-only Bash and unsupported prompt content. |
internal/assets/review_plugin_recovery_test.go |
Update plugin recovery tests to assert “stop before launch/preflight” behavior and preserve existing recovery invariants where applicable. |
internal/assets/assets_test.go |
Update plugin source contract assertions to match the new typed unsupported-capability path. |
e2e/organicruntime/opencode_reviewer_unsupported_e2e_test.go |
Add a Windows-only real OpenCode E2E test proving no Bash side effects / no authority creation occurs. |
internal/versions/versions.go |
Pin OpenCode runtime version to 1.18.10. |
.github/workflows/ci.yml |
Install pinned OpenCode 1.18.10 and add a guard verifying the Windows E2E test is registered by name. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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
🤖 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 @.github/workflows/ci.yml:
- Line 233: Harden the pinned OpenCode installation step by adding integrity
verification for opencode-ai@1.18.10, preferably through a recorded package
integrity hash or a lockfile-based install. Preserve the exact version pin and
ensure the workflow fails if the downloaded package does not match the expected
integrity value.
In `@internal/components/sdd/review_ledger_contract_test.go`:
- Around line 389-390: Document both deterministic baseline changes in
internal/components/sdd/review_ledger_contract_test.go: add a short rationale
comment beside the standard and full-4R wantChars entries at lines 389-390,
noting that OpenCode reviewers now render the short unsupported-capability
prompt; also add a comment for
TestKilocodeReviewSettingsMatchCurrentMainBaseline at lines 231-246 describing
what the SHA-256 hash covers and how to regenerate want after a legitimate
Kilocode settings change.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e460b7c2-aadc-47b7-939b-d06cb6729d3c
⛔ Files ignored due to path filters (1)
testdata/golden/sdd-opencode-multi-settings.goldenis excluded by!testdata/**
📒 Files selected for processing (9)
.github/workflows/ci.ymle2e/organicruntime/opencode_reviewer_unsupported_e2e_test.gointernal/assets/assets_test.gointernal/assets/opencode/plugins/review-result-artifacts.tsinternal/assets/review_plugin_recovery_test.gointernal/components/sdd/boundedreview.gointernal/components/sdd/inject.gointernal/components/sdd/review_ledger_contract_test.gointernal/versions/versions.go
|
|
||
| - name: Install pinned OpenCode runtime | ||
| run: npm install --global opencode-ai@1.18.4 | ||
| run: npm install --global opencode-ai@1.18.10 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial
Consider integrity verification for the pinned OpenCode install.
Static analysis flags npm install --global opencode-ai@1.18.10 as an ad-hoc package install outside a lockfile. The exact version pin limits drift, but the install still has no checksum or integrity verification against a compromised registry or mirror. Consider recording and verifying the package integrity hash, or using a lockfile-based install for this step, as a follow-up hardening item.
🧰 Tools
🪛 zizmor (1.28.0)
[warning] 233-233: ad-hoc installation of packages (adhoc-packages): installs a package outside of a lockfile
(adhoc-packages)
🤖 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 @.github/workflows/ci.yml at line 233, Harden the pinned OpenCode
installation step by adding integrity verification for opencode-ai@1.18.10,
preferably through a recorded package integrity hash or a lockfile-based
install. Preserve the exact version pin and ensure the workflow fails if the
downloaded package does not match the expected integrity value.
Source: Linters/SAST tools
Comment on lines
+389
to
+390
| {name: "standard", agents: []string{"review-reliability"}, beforeChars: 42_301, wantChars: 13_855, maxCharacters: 18_500}, | ||
| {name: "full-4R", agents: []string{"review-risk", "review-resilience", "review-readability", "review-reliability"}, beforeChars: 106_998, wantChars: 21_613, maxCharacters: 36_000}, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Document the rationale for both new baseline changes.
This test file has an established convention: every deterministic baseline change (SHA-256 hash, character-count pin) gets a short comment explaining what changed and why. Both new baseline values in this PR skip that convention.
internal/components/sdd/review_ledger_contract_test.go#L389-L390: add a short comment (following the style of the block ending at line 388) explaining thatwantCharsdropped because OpenCode reviewers now render the short unsupported-capability prompt instead of the full inspection recipe.internal/components/sdd/review_ledger_contract_test.go#L231-L246: add a comment onTestKilocodeReviewSettingsMatchCurrentMainBaselineexplaining what the SHA-256 hash covers and how to regeneratewantwhen a legitimate Kilocode settings change occurs.
📍 Affects 1 file
internal/components/sdd/review_ledger_contract_test.go#L389-L390(this comment)internal/components/sdd/review_ledger_contract_test.go#L231-L246
🤖 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 `@internal/components/sdd/review_ledger_contract_test.go` around lines 389 -
390, Document both deterministic baseline changes in
internal/components/sdd/review_ledger_contract_test.go: add a short rationale
comment beside the standard and full-4R wantChars entries at lines 389-390,
noting that OpenCode reviewers now render the short unsupported-capability
prompt; also add a comment for
TestKilocodeReviewSettingsMatchCurrentMainBaseline at lines 231-246 describing
what the SHA-256 hash covers and how to regenerate want after a legitimate
Kilocode settings change.
Alan-TheGentleman
added
the
size:exception
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 `@e2e/organicruntime/opencode_reviewer_unsupported_e2e_test.go`:
- Around line 130-133: Update the helper containing the tool-use rejection check
to record matching events instead of returning immediately, continue decoding
until io.EOF, and return the recorded rejection only after the complete stream
parses successfully. Ensure malformed trailing JSON returns the decoder error,
and add a test covering a valid rejection followed by "\n{" that expects an
error.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bbb0728c-fcca-403f-8534-d7c7e5a85a84
📒 Files selected for processing (4)
e2e/organicruntime/opencode_reviewer_unsupported_e2e_test.gointernal/assets/assets_test.gointernal/assets/opencode/plugins/review-result-artifacts.tsinternal/assets/review_plugin_recovery_test.go
Comment on lines
+238
to
+241
| $tests = go test -list '^TestRealOpenCodeRejectsUnsupportedImmutableReviewerBash$' ./e2e/organicruntime | ||
| if ($LASTEXITCODE -ne 0 -or $tests -notcontains 'TestRealOpenCodeRejectsUnsupportedImmutableReviewerBash') { | ||
| throw 'Windows OpenCode reviewer rejection E2E is missing' | ||
| } |
20 tasks
This was referenced Aug 2, 2026
8 tasks
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
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.
🔗 Linked Issue
Closes #2076
🏷️ PR Type
type:bug— Bug fix📝 Summary
inspect-candidate, and unrelated SDD behavior unchanged.📂 Changes
🧪 Test Plan
go test ./... -count=1go build ./...go vet ./...go run ./internal/gofmtcheckactionlint .github/workflows/ci.yml✅ Contributor Checklist
type:*labelsize:exception💬 Notes for Reviewers
The 584-line diff has a maintainer-approved
size:exception: the fail-closed behavior, generated configuration, adversarial JSONL proof, armed launch detector, and real Windows runtime gate form one security unit.This is a fail-closed successor to #2109, not a continuation of its wildcard shell-permission approach. The old contributor branch remains untouched. OpenCode stays policy-eligible, but immutable review transport remains disabled until a documented exact child-session binding exists.
Summary by CodeRabbit
Bug Fixes
Tests