fix(bench): keep opencode TB permissions in parity#492
Conversation
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — b0d025ec
This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.
tangletools · auto-approval · reason: drewstone_author · 2026-07-08T00:01:08Z
tangletools
left a comment
There was a problem hiding this comment.
🟡 Value Audit — sound-with-nits
| Verdict | sound-with-nits |
| Concerns | 1 (1 weak-concern) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 104.4s (2 bridge agents) |
| Total | 104.4s |
💰 Value — sound
Expands the shared router opencode permission config to allow headless read/write/external-directory access, and removes the supervisor-only permission override so the raw and supervisor TB arms differ only by the MCP coordination block.
- What it does: In bench/tb_agents/opencode_router_agent.py:160-169 it adds "read": "allow", "write": "allow", and "external_directory": "allow" to the base opencode config. In bench/tb_agents/opencode_supervisor_agent.py:60-71 it replaces the previous permission-mutation block with an override that only adds the "mcp" coordination block, inheriting the rest of the config unchanged from OpenCodeRouterAgent.
- Goals it achieves: 1) Let headless Terminal-Bench opencode runs read from and write to external paths without interactive prompts blocking them. 2) Keep the raw (router) and supervisor benchmark arms permission-identical so that the only experimental difference is the coordination MCP, making the A/B comparison cleaner and fairer.
- Assessment: Good on its merits. The change removes an unnecessary divergence between the two arms, centralizes the permission set in one place, and is consistent with the existing design where the supervisor is described as the router agent plus a coordination MCP. It is a minimal, behaviorally appropriate fix.
- Better / existing approach: none — this is the right approach. The permission set is already owned by OpenCodeRouterAgent._build_router_config, and there is no other existing config builder or shared permission helper in the bench/ tree that would be a better home (searched bench/tb_agents and bench/ for permission/config patterns and found only the two changed files).
- Model: opencode/kimi-for-coding/k2p7
- Bridge attempts: 1
🎯 Usefulness — sound-with-nits
Coherent parity fix that moves shared permissions into the base agent and reduces the supervisor to a pure MCP-block delta; reachable, follows the existing inheritance grain.
- Integration: Both agents are TB-registered (OpenCodeRouterAgent.name() at opencode_router_agent.py:101-103 returns 'agent-runtime-opencode-router'; OpenCodeSupervisorAgent.name() at opencode_supervisor_agent.py:53-55 returns 'agent-runtime-opencode-supervisor') and are invoked via
tb run --agent-import-path tb_agents.opencode_<name>:<Class>per the module docstring (opencode_router_agent.py:34-37). The super - Fit with existing patterns: Matches the existing pattern exactly: the supervisor already overrides _build_router_config by calling super() and adding only its delta (the mcp block at opencode_supervisor_agent.py:63-70), and _write_config_to_container already chains via super() (opencode_supervisor_agent.py:73-82). The PR just extends that same inheritance discipline to the permission block — moving the shared baseline into t
- Real-world viability: The permission block is static dict construction with no concurrency or error-path surface, so it holds on the happy path. One real-world caveat: the in-repo reference probe bench/src/coordination-mcp-container-reach.mts:80-107 documents a 10-key opencode permission set for this exact MCP container setup (edit, bash, webfetch, read, write, external_directory, task, plan_enter, plan_exit, question)
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
🎯 Usefulness Audit
🟡 Parity achieved by trimming the supervisor down, leaving a 4-key gap vs. the in-repo opencode MCP reference [robustness] ``
bench/src/coordination-mcp-container-reach.mts:80-107 (the repo's own documented opencode+coordination-MCP config) lists 10 permission keys including task, plan_enter, plan_exit, question. After this PR both production arms emit only 6 keys (opencode_router_agent.py:162-169); the supervisor's previous additions of those 4 keys were removed rather than propagated to the raw arm. Net parity is real, but it is parity at the lower of the two previous sets. If headless opencode ever routes through pl
What this audit checks
It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.
| Pass | What it asks |
|---|---|
| Heuristic | Vague title? Whitespace-only or cruft-bearing diff? (content signals only) |
| Duplication | Do added function/class names already exist elsewhere in the repo? |
| Value Audit | What does it do? What goal does it achieve? Is it good? Better architecture or already-exists? |
| Usefulness Audit | Does it integrate and fit? Will it hold up in real use and actually get used? |
Findings are concerns, not blocks — the human reviewer decides what to do with them.
Summary
Verification
git diff --checkpython3 -m py_compile bench/tb_agents/opencode_router_agent.py bench/tb_agents/opencode_supervisor_agent.pypnpm run typecheckpnpm run lintpnpm test(127 files, 1254 passed, 2 skipped)pnpm run buildpnpm run verify:package