feat(agents): add OpenCode harness with opt-in [opencode] extra - #115
feat(agents): add OpenCode harness with opt-in [opencode] extra#115mohsen-uipath wants to merge 12 commits into
Conversation
New agent kind `opencode`, registered through the existing plugin SPI and selectable from task YAML (agent.type: opencode) or the CLI (-D agent.type=opencode). Drives `opencode run --format json` non-interactively and reduces its nd-JSON event stream into the standardized event protocol; EventCollector builds the TurnRecord, so no telemetry is assembled by hand. Telemetry and cost: - Per-step token buckets with the input convention arbitrated per step from the stream's own `total` (flat vs nested); unverifiable or contradictory shapes warn once per turn instead of silently mis-booking a bucket. - Real per-call cost from step_finish.cost; the rate card fills the gaps (cost omitted, or $0 reported for tokens the card prices above zero) so run totals are never understated. openrouter/ model prefixes normalize to the bare rate-card keys (mirrored in evalboard/lib/pricing.ts). - The reconciliation invariant holds by construction: summing the four buckets across TurnRecord.messages equals token_usage exactly. - Tool names normalize to the canonical vocabulary (bash -> Bash, ...) so one criterion scores identically across harnesses. Failure paths per the Agent contract: AgentCrashError with a crashed=True partial TurnRecord on pending_turn, TurnTimeoutError on deadline, cooperative should_stop honored at event granularity, and a clean exit that recognized no events crashes loudly instead of scoring as an empty success. stderr is drained concurrently and every post-exit read is bounded (the CLI's server child holds the inherited pipes open). Opt-in install: the [opencode] extra is deliberately empty — OpenCode is a Node CLI (npm install -g opencode-ai) and the harness imports no third-party Python package; a missing binary fails at start() with the install command. Validated live end-to-end (SUCCESS backed by real telemetry: turns, tokens, tools, cost, exact reconciliation). Documented at docs/agents/OPENCODE.md and wired into the docs nav and generated index surfaces.
… test every failure path Closes the three deliberately-deferred hardening items: - The post-EOF reap in _settle_turn was unbounded: a CLI that closed its stream but never exited hung the turn past its deadline, the one window where turn_timeout went unenforced. The reap now gets the deadline's remainder (TurnTimeoutError on expiry) or a fixed grace when no deadline is configured (AgentCrashError naming the wedge). - kill()/kill_sync() signaled only the CLI pid, orphaning the server child that opencode run leaves holding the pipes — a slow process leak across a batch. Each invocation now runs in its own session (start_new_session), and teardown sweeps the spawned process groups with SIGKILL. OpenCode persists sessions on disk, so --session continuity survives the sweep. Verified live: zero leftover opencode processes after a real run. - The failure paths were the least-tested code in the file. Eleven new tests cover: deadline expiry mid-stream and post-EOF (TurnTimeoutError, partial parked, single TIMEOUT terminal event, iteration rollback), the no-deadline wedge (AgentCrashError), external CancelledError (partial parked, CRASHED terminal event, cancellation re-raised), kill_sync from the watchdog thread, process-group sweep on stop/cooperative-stop, tool error/permission-denied capture, and the orphan-result branch. Live smoke re-run under the new teardown: SUCCESS with exact reconciliation, normalized tools, heavy cache traffic booked correctly, zero warnings, zero leaked processes.
…F exit grace The constant gained a second consumer in the bounded-reap change (the exit grace in _settle_turn when no turn deadline is configured); the comment still described only the SIGTERM->SIGKILL role. Comment text only.
…rate entry deepseek-v4-flash-0731 is unusable on this OpenRouter account (every serving provider is excluded by the account's data policy), so the checked-in smoke task failed out of the box while all real validation ran on deepseek-v4-pro anyway. Standardize every reference — smoke task, docs examples, config docstring, tests — on v4-pro, and drop the now-orphaned flash-0731 rate-card entry plus its evalboard parity listing (v4-pro was already priced on main). The smoke task now passes as checked in, with no model override. Verified live: SUCCESS 1.000, no data-policy error.
# Conflicts: # README.md # docs/index.md # docs/llms.txt # mkdocs.yml
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
- signal.SIGKILL does not exist on Windows, so pyright failed the Windows Smoke job on kill_sync. Resolve it once as _SIGKILL (SIGTERM fallback) and use it in kill_sync and the group sweep; the sweep itself was already a runtime no-op off POSIX. - The Windows job also runs pytest: install the os.killpg test stub with raising=False (the attribute is absent there) and skip the process-group-teardown test class off POSIX, since the sweep it asserts is POSIX-only by design. - CodeQL py/mixed-returns on communicate(): the final except ends in _crash_turn, whose NoReturn CodeQL cannot see — add an explicit unreachable raise so no path looks like an implicit None return. - CodeQL py/ineffectual-statement on the cancellation test's bare 'await task': bind the (never-produced) value so the statement's effect is explicit.
The E2E job runs --tags smoke-pass on Bedrock runners that have neither the opencode CLI nor OpenRouter credentials, and pins the bucket at exactly 7 tasks; the new task's smoke-pass tag made it an 8th, un-runnable entry. Drop the tag (the task keeps smoke/opencode for local runs) — live opencode coverage needs its own credentialed job, the way Codex has one.
|
Let's wait for merging until we get some results from this code. |
`plugins:` is how a task ships the skills under test, but the OpenCode agent listed it among the fields it silently drops. A skill-injection run therefore looked entirely normal while measuring the bare model: the only loadable skill was OpenCode's built-in `customize-opencode`, and an attempt to load a real one returned an error. Map each local plugin root to OpenCode's `skills.paths`: - Read the `skills` field of `<root>/.claude-plugin/plugin.json` (string or list), the same field Claude Code reads, so one `plugins:` line means the same thing on both harnesses. Fall back to the convention default `<root>/skills`, or to the root itself when it is already a bare skills directory. - Hand the paths over via OPENCODE_CONFIG_CONTENT, which the CLI merges as a final local-scope layer. Chosen over writing `<sandbox>/.opencode/skills/`: it writes nothing into the sandbox that is later preserved as a run artifact and inspected by file criteria, and it does not depend on how the CLI resolves a project root from `--dir`. An inherited value is merged into, not clobbered; with no `plugins:` block the variable is untouched, so runs without one are byte-for-byte unchanged. - Never point at a plugin root that has a skills subdir. `skills.paths` is scanned recursively and a root can hold a self-referential symlink, which resolves skills through an arbitrary path and drops duplicate names. `--pure` skips external *plugins*, not configured skill paths, so the default `pure: true` is unaffected. Also make the engagement observable, without which the injection cannot be told from the old behavior: map OpenCode's lowercase `skill` tool to the canonical `Skill`, and read the skill name from `parameters["name"]` (OpenCode) as well as `parameters["skill"]` (Claude). Every way this can resolve to nothing — unset env var, missing directory, no SKILL.md under the root — is warned at `start()`, and the resolved paths are recorded per task under `environment_info.opencode_skill_paths`. Verified against the real CLI: 1 -> 27 loadable skills, and a live smoke task goes from an invented command at score 0.0 to `Skill` engagement plus the correct invocation at score 1.000.
uipreliga
left a comment
uipreliga
left a comment
There was a problem hiding this comment.
Review: coder_eval — pr:115 (23 files) axis:1,2,3,4,5,6,7,8
Scope: pr:115 (23 files) axis:1,2,3,4,5,6,7,8 · branch feat/opencode-harness · fcb7dad · 2026-08-17T14:02Z · workflow variant
Change class: complex — adds a new ~1250-line agent harness (OpenCodeAgent) with a new stream protocol parser, a new AgentKind enum value, new pricing entries, and a change to the skill_triggered criterion's detection logic; correctness requires reasoning about the Agent lifecycle contract, subprocess/resource management, and token reconciliation
Architecture, security and the merge/config layering remain excellent (9.9/9.9, no critical findings anywhere and clean type-checked, lint-gated code), but the risk is concentrated entirely in the new OpenCode agent's failure paths and the thin test net around them — an unreaped subprocess that lets a retry run a second CLI into the same sandbox, untested mock-CLI PATH shadowing, a zero-telemetry guard that inspects event names instead of captured tokens, an unpinned max_turns boundary, and un-normalized tool parameter keys can each change a task's score or final_status for identical agent output — so the bottom line is: sound to merge only after a focused fix-plus-test pass on opencode_agent.py, since these are measurement-integrity defects, not stylistic ones.
Summary
| Axis | Score | 🔴 | 🟠 | 🟡 | 🔵 | Top Issue |
|---|---|---|---|---|---|---|
| 1. Code Quality & Style | 8.7 / 10 | 0 | 0 | 2 | 3 | OpenCode-specific skill-parameter key hardcoded into the agnostic skill_triggered criterion instead of the agent-layer arg-rename seam |
| 2. Type Safety | 8.8 / 10 | 0 | 0 | 2 | 2 | Blanket # type: ignore[arg-type] at opencode_agent.py:782 masks all diagnostics on its only call site; root cause is a wrongly-shaped (too narrow) parameter type with a clean widening fix |
| 3. Test Health | 7.3 / 10 | 0 | 1 | 3 | 2 | No test for OpenCodeAgent._build_env: the mock-CLI PATH-shadowing and PLUGIN_TOOLS_DIR contract is entirely uncovered (all three sibling agents test it) |
| 4. Security | 9.9 / 10 | 0 | 0 | 0 | 1 | Plugin skill-path resolution has no containment check to the plugin root |
| 5. Architecture & Design | 9.9 / 10 | 0 | 0 | 0 | 1 | New agents/ → isolation/docker_runner import edge for a stream-reading constant |
| 6. Error Handling & Resilience | 7.9 / 10 | 0 | 1 | 2 | 1 | communicate()'s finally never terminates the opencode subprocess, so a crashed turn leaves the CLI running and the retry spawns a second one into the same sandbox |
| 7. API Surface & Maintainability | 8.8 / 10 | 0 | 0 | 2 | 2 | New opencode agent cannot run under the docker sandbox driver and no surface (docs or limitations list) says so |
| 8. Evaluation Harness Quality | 8.9 / 10 | 0 | 0 | 2 | 1 | Cross-harness normalization stops at tool NAMES: OpenCode's tool ARGUMENT keys reach CommandTelemetry.parameters un-renamed, so a non-Bash command_executed criterion regex-matching the serialized parameter blob scores differently per harness |
Overall Score: 8.8 / 10 · Weakest Axis: Test Health at 7.3 / 10
Totals: 🔴 0 · 🟠 2 · 🟡 13 · 🔵 13 across 8 axes.
Blockers
- [Axis 3] No test for OpenCodeAgent._build_env: the mock-CLI PATH-shadowing and PLUGIN_TOOLS_DIR contract is entirely uncovered (all three sibling agents test it) (
src/coder_eval/agents/opencode_agent.py:883) —start(env_path_prepend=..., plugin_tools_dir=...)is the abstractAgent.start()contract (src/coder_eval/agent.py:186-200) and the orchestrator ALWAYS passes both (orchestrator.py:1087-1095:env_path_prepend = [str(p) for p in self.sandbox.resolved_mock_path_dirs]). OpenCode implements it at opencode_agent.py:882-885:
if self._env_path_prepend:
env["PATH"] = os.pathsep.join([*self._env_path_prepend, env.get("PATH", "")])
if self._plugin_tools_dir and "PLUGIN_TOOLS_DIR" not in env:
env["PLUGIN_TOOLS_DIR"] = self._plugin_tools_dir
Both lines are in the coverage MISSING list (883, 885), and grep -n "PATH|env_path|plugin_tools" tests/test_opencode_agent.py returns only a docstring mention at line 554 — zero assertions. Every sibling agent pins exactly this: tests/test_codex_agent.py:166-183 ("This is the mock-shadowing contract: sandbox mock CLIs must resolve before the real ones. An inverted join order (mocks at the back) must fail here."), :208-221 (a non-uppercase 'Path' key is reused, not duplicated), tests/test_agent.py:313 (Claude), tests/test_antigravity_agent.py:1228-1236 (Antigravity). Failure scenario: an inverted join ([env.get("PATH"), *prepend]) or a wrong key leaves sandbox mock CLIs un-shadowed, so a task grading a mocked CLI (cli_called / invocation-log criteria) silently exercises the REAL binary or writes no invocation log and scores 0 for every row — with the full OpenCode suite still green. Note the sibling-tested Windows case is also unhandled here (env["PATH"] hardcoded while codex_agent.py:1140 resolves path_key case-insensitively), which a mirrored test would have surfaced. Add the three codex-shaped tests to tests/test_opencode_agent.py: order-preserving prepend ahead of the parent PATH, PLUGIN_TOOLS_DIR set only when absent from the inherited env, and neither key touched when start() got no kwargs — asserting on captured["kwargs"]["env"], which the existing patch_exec fixture already records.
2. [Axis 6] communicate()'s finally never terminates the opencode subprocess, so a crashed turn leaves the CLI running and the retry spawns a second one into the same sandbox (src/coder_eval/agents/opencode_agent.py:1083) — The turn's only unconditional teardown is:
finally:
if stderr_drain is not None:
stderr_drain.cancel()
self._process = NoneIt cancels the stderr reader and drops the process handle, but never reaps the child. Two reachable exits leave the CLI alive: except asyncio.CancelledError: (1067-1070, which only calls _finalize_external_cancel + _capture_partial_turn) and except Exception as e: (1071-1082, which calls _crash_turn — and _crash_turn at 1168-1185 does state.close_open_tools() / _finalize_and_raise_crash / _capture_partial_turn, no kill). Every OTHER crash site is safe because it follows a completed await proc.wait() (_settle_turn lines 1116, 1136, 1142, 1154) or an explicit await self.kill() (1120, and _timeout_turn line 1199).
Concrete failure: an exception raised from inside the read loop while the CLI is streaming — e.g. int(tokens.get("input") or 0) at line 608 on a non-numeric value (see the separate finding), or read_task.result() at 1027 re-raising a StreamReader ValueError, which the comment at 1073-1075 explicitly anticipates. _crash_turn raises AgentCrashError; categorization.py:164 maps it to ErrorCategory.AGENT_CRASH, whose RETRY_CONFIG entry is max_retries=2 (errors/categories.py:122-126). execute_with_retry (errors/executor.py:68-119) sleeps ~5s and calls operation() again; _on_attempt_failure (orchestrator.py:1366-1381) only drains pending_turn and calls discard_pending_turn() — it never calls agent.kill(). So attempt 2 runs create_subprocess_exec at line 967 with the same --dir <sandbox> and, because self._session_id was captured at line 1233 and is replayed at line 874 (argv += ["--session", self._session_id]), the same session — while attempt 1's CLI is still editing files that the criteria will score.
The in-tree precedent is explicit, in isolation/docker_runner.py:595-599: "If proc is still alive we got cancelled mid-flight. Kill the container and the docker CLI subprocess" — if proc.returncode is None: await self._kill_container(proc, container_name), inside a finally, motivated verbatim by Ctrl-C leaving the child "running and burning LLM budget".
Also note the ordering hazard: self._process = None runs BEFORE anything can use it, so after this finally neither kill() (810: if proc is not None and proc.returncode is None) nor kill_sync() (822-825) can signal the CLI directly — only _sweep_process_groups() can, and that returns immediately when os.name != "posix" (839-840). On POSIX the eventual Orchestrator._cleanup -> agent.stop() -> kill() -> killpg (orchestrator.py:2279) does reap it at end of task, which is why this is High rather than Critical; on Windows nothing does.
Fix: in the finally, before clearing the handle, do if proc is not None and proc.returncode is None: await self.kill() (guarding for the spawn-failure path where proc was never bound), and add a test asserting the process is terminated when communicate() exits via except Exception and via CancelledError — tests/test_opencode_agent.py::TestExternalCancel::test_cancel_parks_partial_and_reraises (line 1024) currently asserts only the parked partial and the terminal event, and TestProcessGroupTeardown::test_stop_sweeps_the_spawned_group (line 1058) asserts captured["killpg"] == [] after the turn, confirming nothing kills mid-flight.
Non-blocking, but please consider before merge
- [Axis 1] OpenCode-specific skill-parameter key hardcoded into the agnostic
skill_triggeredcriterion instead of the agent-layer arg-rename seam (src/coder_eval/criteria/skill_triggered.py:70) — The PR pushes OpenCode-specific knowledge into a shared, agent-agnostic criterion:
# skill_triggered.py:70
skill = cmd.parameters.get("skill") or cmd.parameters.get("name") or ""The tree already has a dedicated seam for exactly this, one layer down, and the new agent already uses half of it. opencode_agent.py:128-147 maps the native tool NAME ("skill": "Skill") at the harness boundary, and antigravity_agent.py:178-181 maps native parameter KEYS at the same boundary for the same stated reason:
# antigravity_agent.py:173-181
# Antigravity per-tool INPUT-arg key -> canonical (Claude-ish) key, so cross-agent
# success criteria (command_executed keys on Bash ``parameters["command"]``; LS on
# ``path``) and reports read the SAME parameter names the Claude/Codex backends emit.
_ANTIGRAVITY_ARG_RENAME: dict[str, dict[str, str]] = {
"Bash": {"command_line": "command"},
"LS": {"directory_path": "path"},
}Fix: add the equivalent per-tool key rename to opencode_agent.py (Skill: {"name": "skill"}), applied in _OpenCodeTurnState.on_tool_use where parameters=params is set (opencode_agent.py:413), and revert skill_triggered.py:70 to cmd.parameters.get("skill", ""). As written, every future harness whose skill tool names the argument something else adds another or cmd.parameters.get(...) alternative to a criterion that is supposed to know nothing about harnesses; parameters is also substring-scanned two lines later (line 73-75), so widening the accepted key set in the criterion is the riskier of the two places to do it.
2. [Axis 1] Complexity cluster in the new agent module: five functions at CC 14-22, with an avoidable dispatcher (src/coder_eval/agents/opencode_agent.py:1206) — Theme-grouped; radon cc -s at PR HEAD (verified locally):
M 1206:4 OpenCodeAgent._handle_line - D (22)
M 385:4 _OpenCodeTurnState.on_tool_use - C (18)
M 1088:4 OpenCodeAgent._settle_turn - C (18)
M 604:4 _OpenCodeTurnState.on_step_finish - C (17)
F 248:0 _plugin_skill_dirs - C (14)
Explicit calibration as requested: OpenCodeAgent.communicate D(26) is NOT part of this ask. Measured against its own class in this tree it is at or below every sibling — ClaudeCodeAgent.communicate D(30), CodexAgent.communicate D(26), AntigravityAgent.communicate F(43) — and the PR branch's pyproject.toml configures no mccabe/C90 gate at all. D(26) for a subprocess turn loop is acceptable-for-this-tree; overall this module (max D26, no E/F) is the cleanest of the four agents.
The one with a cheap, non-cosmetic fix is _handle_line. It is a dispatch table where four of five branches delegate to a handler and the fifth inlines its parsing:
# opencode_agent.py:1235-1250
if event_type == _STEP_START: state.on_step_start(part)
elif event_type == _TEXT: state.on_text(part)
elif event_type == _TOOL_USE: state.on_tool_use(part)
elif event_type == _STEP_FINISH: state.on_step_finish(part)
elif event_type == _ERROR:
error = part.get("error")
if isinstance(error, dict):
data = error.get("data")
message = (data or {}).get("message") if isinstance(data, dict) else None
state.error_message = str(message or error.get("name") or "unknown error")
else:
state.error_message = str(error or "unknown error")Move that body to _OpenCodeTurnState.on_error(part) alongside its four siblings (drops _handle_line back under 20 and makes the dispatch uniform). Optionally: _settle_turn takes both deadline and timeout (opencode_agent.py:1095-1097) — two spellings of one value, reconciled again inside at line 1114-1119; passing only deadline and deriving the message would remove a branch.
3. [Axis 2] Blanket # type: ignore[arg-type] at opencode_agent.py:782 masks all diagnostics on its only call site; root cause is a wrongly-shaped (too narrow) parameter type with a clean widening fix (src/coder_eval/agents/opencode_agent.py:782) — Line 782 reads:
self._skill_dirs = _plugin_skill_dirs(self.config.plugins, log=logger) # type: ignore[arg-type]
Two verified problems.
(1) The suppression is BLANKET, not scoped. Pyright ignores the mypy-style [arg-type] bracket and suppresses every diagnostic on the line. I proved this in the worktree: I rewrote the call as _plugin_skill_dirs(self.config.plugins, log=logger, bogus_kwarg=1) # type: ignore[arg-type] and uv run pyright src/coder_eval/agents/opencode_agent.py reported 0 errors, 4 warnings — a non-existent keyword argument (normally reportCallIssue) was silently accepted. Any future signature change to _plugin_skill_dirs (renaming log=, adding a required parameter) is invisible at this, its only call site.
(2) It carries no justification comment, and it is not needed. Removing it yields exactly one diagnostic:
782:47 - error: Argument of type "list[LocalPluginConfig] | None" cannot be assigned to parameter "plugins" of type "list[dict[str, Any]] | None" in function "_plugin_skill_dirs"
The declared parameter at line 249 is plugins: list[dict[str, Any]] | None, while BaseAgentConfig.plugins (models/agent_config.py:148) is list[LocalPluginConfig] | None — a TypedDict, which is deliberately not assignable to dict[str, Any].
Fix (verified: 0 errors, 4 warnings — i.e. the error is gone with no suppression): change line 249 to plugins: Sequence[Mapping[str, Any]] | None,, add Mapping, Sequence to the from collections.abc import Callable line, and delete the # type: ignore[arg-type]. Annotating list[LocalPluginConfig] | None (already exported from coder_eval.models) is the stronger alternative and additionally makes the plugin.get("type") / plugin.get("path") reads at lines 260-263 statically keyed. If a suppression is ever genuinely required here, use the scoped form # pyright: ignore[reportArgumentType] plus a one-line reason.
4. [Axis 2] OpenCodeAgent.__init__ breaks the SPI factory contract: **_: Any swallows route= and task_id occupies the positional slot siblings use for route (src/coder_eval/agents/opencode_agent.py:744) — Lines 740-745 read:
def __init__(
self,
config: OpenCodeAgentConfig,
task_id: str = "unknown",
**_: Any,
) -> None:
No other in-tree agent does this — every sibling names its kwargs explicitly: ClaudeCodeAgent (config, route=None, *, instance_name, extra_mcp_servers, cost_log_tags), CodexAgent and AntigravityAgent (config, route=None, *, instance_name), NoOpAgent (config, route=None).
This matters because the caller side is already erased: agents/registry.py:163 is
return cast(Any, registration.agent_class)(config, route=route, **kwargs)
so pyright checks nothing about the call. With **_: Any on the callee, nothing checks it at runtime either. Concretely: route=self.route (which every other agent declares and documents — see antigravity_agent.py:246-248, which keeps an unused route "for parity") vanishes into **_ with no declaration and no docstring saying it is intentionally unused; and the TypeError safety net the orchestrator explicitly depends on is defeated. orchestrator.py:1284-1288 states the design: "Gate on AGENT CAPABILITY, not the route ... only agents whose init accepts the kwarg (supports_cost_log_tags) may receive it — otherwise the agent-agnostic factory would forward it into ... constructors that don't declare it and crash with TypeError." That loud crash is the intended signal; on this agent a mis-gated cost_log_tags would instead be silently dropped, yielding runs with no cost correlation and no error. The same sink makes a mistyped task_i= at any call site (e.g. tests/test_opencode_agent.py:194, the only site that passes task_id) silently leave task_id at "unknown".
Fix: replace **_: Any with the explicit parameters the factory actually passes — route: ApiRoute | None = None — and drop the sink, matching the sibling agents. If route is genuinely unused, keep it declared and say so in the docstring as AntigravityAgent does.
5. [Axis 3] max_turns is tested in one direction only — the >→>= mutation at opencode_agent.py:1034 survives all 65 tests in tests/test_opencode_agent.py, and that flag decides FinalStatus (tests/test_opencode_agent.py:918) — The only max_turns test in the file is:
async def test_max_turns_marks_exhausted(self, patch_exec, tmp_path):
patch_exec(_FakeProcess(HAPPY_STREAM))
record = await _run(_agent(), tmp_path, max_turns=1)
assert record.max_turns_exhausted is True
The gate is if max_turns is not None and state.step_count > max_turns: (opencode_agent.py:1034). Flipping > to >= — or counting on step_finish instead of step_start — still passes this test, while a normal 2-step run under max_turns: 2 would now report max_turns_exhausted=True, which orchestrator.py:520-521 turns straight into FinalStatus.MAX_TURNS_EXHAUSTED instead of SUCCESS/FAILURE for identical agent output. Worse, a spurious exhaustion also suppresses two crash guards this PR added: opencode_agent.py:1140 (... and not state.max_turns_exhausted on the non-zero-exit crash) and :1152 (the same conjunct on the zero-telemetry vocabulary-drift crash), so the run would score silently instead of failing loudly. Both siblings pin the full battery and are the template to copy: tests/test_codex_agent.py:2032-2035 and tests/test_antigravity_agent.py:1483-1486 (max_turns=5 over a shorter stream → max_turns_exhausted is False), :1489-1496 (no cap → uncapped), :1469-1473 ("keeps the deciding step whole"). Add: (a) max_turns=2 on HAPPY_STREAM stays False and yields assistant_turn_count == 2; (b) max_turns=1 retains exactly one complete step (assert record.assistant_turn_count == 1 and its token buckets non-zero) rather than only asserting the flag.
6. [Axis 3] The two-event tool lifecycle (pending/running then completed for the same callID) is never exercised — parameters and the start timestamp are frozen from the first event, with no test pinning it (src/coder_eval/agents/opencode_agent.py:401) — on_tool_use documents this path explicitly ("A non-terminal state (pending/running) is still handled: the tool is left open and closed by a later event for the same callID", opencode_agent.py:390-392), but coverage reports the partial branch 401->422, i.e. the telemetry is not None arm — a second event for an already-open callID — is never taken. Every fixture in the test file emits one already-completed event per call; the one "status": "running" event (tests/test_opencode_agent.py:652) is followed by a crash, never by its completion. Because parameters=params if isinstance(params, dict) else {} and tool_name=_TOOL_NAME_MAP.get(...) are assigned ONLY inside the if telemetry is None: block (lines 401-415), a CLI that emits {status: pending} with no state.input and then {status: completed, input: {...}} leaves CommandTelemetry.parameters == {} permanently — criteria/command_executed.py reads parameters["command"] for tool_name: Bash, so that criterion scores 0 on every row while the run looks normal, and no test fails. Add a fixture that emits tool_use twice for one callID (running-without-input, then completed-with-input and output) and assert the final CommandTelemetry carries the mapped tool name, the completion's parameters, result_status == "success", and exactly one ToolStart/ToolEnd pair.
7. [Axis 3] Skill injection ships five documented branches with no test: list-form manifest skills, an unreadable/non-dict manifest, a non-type: local plugin entry, the missing-<name>/SKILL.md warning, and both malformed-inherited-OPENCODE_CONFIG_CONTENT paths (src/coder_eval/agents/opencode_agent.py:241) — Skill injection is the PR's highest-risk new behavior (its own docstring: "before this mapping existed every skill-injection run silently measured the bare model instead — a run that looks entirely normal"), and TestSkillInjection covers only the string-manifest, default-layout, bare-dir, symlink, no-plugins, valid-merge and unresolved-path cases. Uncovered per the coverage report: (1) elif isinstance(value, list): declared = [entry for entry in value if isinstance(entry, str)] (lines 241-242) — the list form the docstring promises ("a string or a list of strings"); (2) the unreadable/non-dict manifest fallback (lines 235-236 and branch 237->243); (3) log.warning("opencode: ignoring non-local plugin entry %r ..."); continue (lines 261-262) — a type: git plugin entry; (4) the "no /SKILL.md directly under" warning (line 288); (5) both invalid-inherited-config branches in _inject_skill_paths — except json.JSONDecodeError (lines 904-905) and the non-JSON-object case (line 913), while only the valid merge is tested (tests/test_opencode_agent.py:540-551). Failure mode for each is identical and silent: the wrong skills.paths (or none) is injected and the run measures the bare model. Add one test per branch, asserting _injected_skill_paths(captured) — the helper already exists at tests/test_opencode_agent.py:493.
8. [Axis 6] finalize() emits no TurnEndEvent for the step left open by a crash/timeout/cancel — or by either clean cut path (should_stop, max_turns) — unlike all three sibling agents (src/coder_eval/agents/opencode_agent.py:705) — _OpenCodeTurnState.on_step_start (361-374) emits a TurnStartEvent per CLI step and on_step_finish (661-674) emits the matching TurnEndEvent(status=TurnEndStatus.COMPLETED). But finalize() (681-729) closes only the tools:
self.finalized = True
self.close_open_tools()
usage = self.usage
...
self.emit(
AgentEndEvent(So when a turn dies between step_start and step_finish, the last TurnStartEvent is never closed. Both siblings close it in exactly this place: claude_code_agent.py:610-613 and codex_agent.py:619-626 both emit TurnEndEvent(..., status=TurnEndStatus(status.value)) from finalize before the AgentEndEvent, and TurnEndStatus (streaming/events.py:57-64) is value-for-value identical to AgentEndStatus (67-74) precisely so that conversion works.
This violates the contract stated on Agent.communicate (agent.py:258-261: "with one TurnStartEvent / TurnEndEvent pair per inner turn") and in CLAUDE.md's "Adding a New Agent" step 6. The PR's own fixture demonstrates it: tests/test_opencode_agent.py:1028 builds _HangingProcess([_evt("step_start", ...)]) and the resulting stream carries a TurnStartEvent with no TurnEndEvent. Visible symptom today is a task.log with >>> Turn start: id=msg_1 and no matching --- Turn end [...] line (streaming/renderers.py:201-203 vs 222-224); EventCollector counts only TurnStartEvents (collector.py:76-79) so the persisted TurnRecord is unaffected, which is why this is Medium rather than High.
Fix: unlike the siblings, OpenCode already emits a TurnEndEvent per completed step, so finalize must emit one only when a step is open. Track that with a flag set in on_step_start and cleared in on_step_finish, and in finalize emit TurnEndEvent(task_id=..., thread_id=..., turn_id=self.turn_id, status=TurnEndStatus(status.value), tokens=None) before the AgentEndEvent when the flag is set. Assert it in TestExternalCancel and TestTimeoutContract.
9. [Axis 6] _handle_line's "Never raises on bad input" docstring (line 1207) is false — on_step_finish's five bare int() casts (608-612) are the module's only unguarded field reads (src/coder_eval/agents/opencode_agent.py:608) — on_step_finish converts the token buckets with no type guard:
raw_in = int(tokens.get("input") or 0)
raw_out = int(tokens.get("output") or 0)
step_reasoning = int(tokens.get("reasoning") or 0)
step_cw = int(cache.get("write") or 0)
step_cr = int(cache.get("read") or 0)(lines 608-612). A non-numeric value raises: int("abc") -> ValueError, int({"a": 1}) -> TypeError, int([5]) -> TypeError. That contradicts the contract the function advertises at line 1207: """Parse one nd-JSON line and dispatch it. Never raises on bad input.""" — a future reader adding a caller outside communicate()'s except Exception net will rely on that sentence.
It is also the module's only unguarded read. Everything else defends: _epoch_ms_to_dt checks isinstance(value, int | float) (213) and catches OverflowError/OSError/ValueError (217); state = state if isinstance(state, dict) else {} (402); params if isinstance(params, dict) else {} (413); cost is gated by isinstance(cost, int | float) (627); total by isinstance(total, int) (563). And the immediately-adjacent _fresh_input_slice (537-602) exists specifically to detect CLI token-schema drift and warn once rather than fail — so a type change in the very same tokens dict crashing the turn is the opposite policy applied to neighbouring fields.
Consequence today: the exception is caught by except Exception as e: at 1071 -> _crash_turn -> AgentCrashError -> categorized AGENT_CRASH (max_retries=2), so a single mistyped bucket burns three full attempts and lands the task as ERROR — and, per the separate finding, leaves the CLI running each time.
Fix: add a _as_int(value: Any) -> int helper mirroring _epoch_ms_to_dt's shape (return 0 and route through state._warn_token_shape on a non-numeric), use it at all five sites, and either keep the "Never raises" docstring honest or delete the claim.
10. [Axis 7] New opencode agent cannot run under the docker sandbox driver and no surface (docs or limitations list) says so (docs/agents/OPENCODE.md:240) — FAILURE: a user follows this page, writes agent: {type: opencode} plus sandbox: {driver: docker} (the mode docs/TASK_DEFINITION_GUIDE.md:188 recommends for untrusted evals); inside the container OpenCodeAgent.start() calls shutil.which("opencode"), finds nothing, and every task dies with "The 'opencode' CLI was not found on PATH. Install it with npm install -g opencode-ai" — advice the user cannot act on, because that PATH is inside an image they did not build. EVIDENCE: docker/Dockerfile:37 installs only npm install -g @anthropic-ai/claude-code@${CLAUDE_CODE_VERSION}, and line 73 is uv export --frozen --extra codex --extra antigravity ${CODER_EVAL_UV_EXTRAS} | uv pip install --system -r /dev/stdin; the Dockerfile's own comment at lines 51-53 claims "Codex and Antigravity are always baked into the image -- peers to the claude-code agent installed above -- so all built-in agents ship in every build", which this PR makes false. Every sibling documents the driver: docs/agents/ANTIGRAVITY.md:196 has ## Running in Docker ("The docker driver works the same as for other agents ... The localharness binary ships inside"). This page has no equivalent and its ## Known limitations block at line 240 never mentions it. FIX: Node 22 is already in the image (docker/Dockerfile:34-37), so either add a pinned npm install -g opencode-ai@<version> beside the claude-code pin (the Dockerfile's stated policy is that an agent binary "travels with the coder_eval release tag"), or add a ## Running in Docker section here stating the driver is unsupported and why. Leaving both undone is the defect.
11. [Axis 7] Public agent roster left stale in README.md, docs/index.md and docs/llms.txt — the PR touched only the three make docs-indexes-generated table rows (9+ hand-written "Claude Code, Codex, Gemini" sentences still omit OpenCode) (README.md:36) — FAILURE: a user — or an LLM reading https://coder-eval.com/docs/llms.txt — asks whether coder-eval supports OpenCode, reads the project's own headline claim, and concludes it does not, even though agent: {type: opencode} is a registered built-in as of this PR. The PR updates only the make docs-indexes-generated table rows; every hand-written roster sentence still names three harnesses. Verified in-scope instances at PR HEAD: README.md:36 - **Agent abstraction** — Claude Code, Codex, and Antigravity (Gemini) today, extensible via a plugin SPI; README.md:14 It runs a real agent (**Claude Code**, **Codex**, or **Google Antigravity / + line 15 Gemini**); README.md:88 uv tool install "coder-eval[codex,antigravity]" # same, with agent extras; docs/index.md:33 (identical sentence to README:36); docs/index.md:6-7 weighted scoring, cost/token telemetry, and CI gates for Claude Code, Codex, / and Gemini. (the page's frontmatter description); docs/index.md:16-17 ...coding agents such as **Claude Code**, **Codex**, and **Google / Antigravity (Gemini)**.; docs/llms.txt:6 (0.0–1.0), runs real agents (Claude Code, Codex, Google Antigravity/Gemini) with. FIX: add OpenCode to each. make docs-indexes will NOT repair them — I ran the generator in the PR worktree and it produced a zero-byte diff, because all seven locations sit outside the <!-- docs-index:start/end --> markers.
12. [Axis 8] Cross-harness normalization stops at tool NAMES: OpenCode's tool ARGUMENT keys reach CommandTelemetry.parameters un-renamed, so a non-Bash command_executed criterion regex-matching the serialized parameter blob scores differently per harness (src/coder_eval/agents/opencode_agent.py:128) — _TOOL_NAME_MAP (opencode_agent.py:128-147) normalizes read->Read, write->Write, edit->Edit etc., and its own comment states the purpose: "without it ... the same task scores differently per harness". But only half the normalization is done. The sibling harness already established the other half — antigravity_agent.py:178-181 declares _ANTIGRAVITY_ARG_RENAME = {"Bash": {"command_line": "command"}, "LS": {"directory_path": "path"}}, with the docstring at :972-973 saying it exists "so command_executed / reports key on the same names (command / path) the Claude/Codex backends emit". OpenCode has no equivalent: on_tool_use stores state.input verbatim (parameters=params if isinstance(params, dict) else {}, opencode_agent.py:413). The PR's own live-captured fixture pins the divergence: tests/test_opencode_agent.py:73 feeds "input": {"filePath": "main.py"} and :259 asserts cmd.parameters == {"filePath": "main.py"} — where Claude emits file_path. Concretely: for any tool other than Bash, criteria/command_executed.py:186 falls through to cmd_text = json.dumps(cmd.parameters), so a criterion {type: command_executed, tool_name: Read, command_pattern: 'file_path.*app\.py'} matches on Claude and scores 0 on OpenCode for identical agent behaviour. Bash is genuinely fine (OpenCode's key is already command, tests/test_opencode_agent.py:451), which is why the docs' claim is scoped to Bash — but the divergence is neither fixed nor listed under docs/agents/OPENCODE.md's "Known limitations", which CLAUDE.md's harness-parity rule requires ("a divergence is either fixed or documented — never silent"). Fix: add an _OPENCODE_ARG_RENAME map mirroring _ANTIGRAVITY_ARG_RENAME (at minimum Read/Write/Edit: {filePath: file_path, oldString: old_string, newString: new_string}) applied where parameters= is built at opencode_agent.py:413, or — if the team prefers to defer — add an explicit bullet to the Known-limitations list. No in-tree task uses a non-Bash tool_name today (grep -rn 'tool_name:' tasks/ yields only Bash and Agent), which is why this is Medium and not High.
13. [Axis 8] The zero-telemetry guard checks the EVENT vocabulary, not whether any tokens were captured: step_finish events carrying no tokens (and no cost) produce a COMPLETED turn with token_usage=None, no warning, and budget gates that cannot fire (src/coder_eval/agents/opencode_agent.py:1152) — _settle_turn's guard is if not stopped_early and not state.max_turns_exhausted and state.recognized_events == 0: (opencode_agent.py:1152) and its comment (:1144-1151) states the harm it prevents: "a clean exit that recognized NO events captured zero telemetry — zero turns, zero tokens, zero cost — while file-based criteria can still pass ... producing a SUCCESS that is silently missing from every aggregate. This already happened once". The guard keys on event NAMES, not on the telemetry actually captured, so the same outcome survives one layer down. Trace a step_finish payload with no tokens key (a provider or auth mode that omits usage): on_step_finish at opencode_agent.py:605-606 does tokens = part.get("tokens"); tokens = tokens if isinstance(tokens, dict) else {} -> raw_in/raw_out/reasoning/cache all 0; _fresh_input_slice (:563-573) sees total non-int and cr or cw false, so it returns 0 WITHOUT calling _warn_token_shape; self.usage stays an empty TokenUsage(); _rate_card_cost (:490) returns None because self.usage.is_empty(); _resolve_cost returns None so finalize (:703) never sets total_cost_usd; and EventCollector.build_turn_record (streaming/collector.py:180-185) then maps that all-zero, costless usage to token_usage = None. Meanwhile state.recognized_events is >= 3 (step_start + text + step_finish), so line 1152 does not fire and the turn returns AgentEndStatus.COMPLETED. Result: a real, billed run scores SUCCESS with no tokens and no cost in any aggregate, run_limits.max_usd / max_total_tokens can never trip, and nothing is logged. Fix: extend the same guard to the telemetry it exists to protect — e.g. crash (or at minimum _warn_token_shape) when a turn completed normally with state.step_count > 0 but state.usage.is_empty() and not state.saw_cost. A regression test alongside TestZeroTelemetryIsLoud (tests/test_opencode_agent.py:694) replaying HAPPY_STREAM with the tokens key removed would pin it.
Nits
Titles only — full text with file:line, evidence and fixes in tmp/code-review-260817-0702/0*-*.md.
- [Axis 1]
_fresh_input_slice: 66 lines of dual-convention auto-detection for a token layout the CLI is not observed to emit —src/coder_eval/agents/opencode_agent.py:537 - [Axis 1]
[opencode]is an empty, no-op install extra in the public packaging surface —pyproject.toml:127 - [Axis 1] Vestigial state and an unnecessary late-binding indirection in
_OpenCodeTurnState—src/coder_eval/agents/opencode_agent.py:316 - [Axis 2] Four new pyright reportOptionalMemberAccess warnings from a non-narrowing ternary the same file already writes correctly elsewhere —
src/coder_eval/agents/opencode_agent.py:404 - [Axis 2] Stringly-typed config-field names read via
getattr(..., None)silently disable the unsupported-knob warning on a rename —src/coder_eval/agents/opencode_agent.py:775 - [Axis 3]
test_terminal_event_is_emitted_exactly_oncedoes not reach the finalize() idempotency guard it names — the double-finalize path is uncovered —src/coder_eval/agents/opencode_agent.py:698 - [Axis 3] kill()'s SIGTERM-then-SIGKILL escalation is untested — the _TERM_GRACE_SECONDS expiry branch never runs —
src/coder_eval/agents/opencode_agent.py:816 - [Axis 4] Plugin skill-path resolution has no containment check to the plugin root —
src/coder_eval/agents/opencode_agent.py:245 - [Axis 5] New
agents/ → isolation/docker_runnerimport edge for a stream-reading constant —src/coder_eval/agents/opencode_agent.py:48 - [Axis 6]
_spawned_pgidsretains pgids of already-reaped invocations, so stop()/kill() SIGKILL potentially recycled process groups —src/coder_eval/agents/opencode_agent.py:984 - [Axis 7]
OpenCodeAgentConfig.variantreuses coder_eval's most overloaded domain term, and is an unvalidated free string —src/coder_eval/models/agent_config.py:296 - [Axis 7] Smoke task placed at
tasks/root and taggedsmoke, contradicting the documented layout for per-agent probes —tasks/opencode_smoke_test.yaml:7 - [Axis 8] skill_triggered's own contract docs were not updated for the third detection signal the PR adds —
src/coder_eval/criteria/skill_triggered.py:70
What's Missing
Parallel paths:
- 🟡
docker/Dockerfilewas not extended: OpenCode is registered as an unconditional built-in (register_builtins, agents/init.py:8/33) but the image installs only@anthropic-ai/claude-code(Dockerfile:37) plus--extra codex --extra antigravity(Dockerfile:73), and the Dockerfile's own comment (lines 51-53) claims "all built-in agents ship in every build" — now false. Node 22 is already present, so a pinnednpm install -g opencode-ai@<version>beside the claude-code pin is the parallel edit. (trigger: src/coder_eval/agents/init.py) (restates: Axis 7: Newopencodeagent cannot run under thedockersandbox driver and no surface says so) - 🟡
models/sandbox.py's dockerenv_passthroughallowlist gained a block per harness (CODEX_API_KEY/CODEX_BASE_URL/CODEX_MODEL,GEMINI_API_KEY/ANTIGRAVITY_MODEL) but none for OpenCode — theOPENROUTER_API_KEY/DEEPSEEK_API_KEYexports OPENCODE.md:61-67 tells users to set are not forwarded, andopencode auth login's credential file is not mounted, so even a custom image with the CLI baked in gets no credentials under--driver docker. (trigger: docs/agents/OPENCODE.md) - 🟡
docs/USER_GUIDE.md:42— the--type, -Tflag reference still enumerates "claude-code,codex,antigravity, or a plugin kind", so the one table that tells a user which values the flag accepts omits the value this PR added. Same root cause as the roster finding, one file further (that finding lists README/index/llms.txt only). (trigger: src/coder_eval/models/enums.py) (restates: Axis 7: Public agent roster left stale in README.md, docs/index.md and docs/llms.txt) - 🔵
tests/lint/doc_env_parity.py::FRAMEWORK_ENV_PREFIXESgainedCODEX_andANTIGRAVITY_when those harnesses landed;OPENCODE_was not added, so CE027 does not check any documentedOPENCODE_*=assignment against a real consumer — and the newOPENCODE_CONFIG_CONTENTis written through a module constant (_CONFIG_CONTENT_ENV), a spelling CE027's_SRC_ENV_READregex cannot see, so adding the prefix needs that read made literal or the rule will misfire. (trigger: src/coder_eval/agents/opencode_agent.py) - 🔵 The parity page now says to run the fixtures with
--type opencode(HARNESS_PARITY.md:115-119), but the fixture's own description was not updated:tasks/run_limits/max_turns_cap.yaml:5-7still reads "Run it with --type claude-code / codex / antigravity", so the file a contributor actually opens contradicts the page that sent them there. (trigger: docs/agents/HARNESS_PARITY.md)
Tests:
- 🟠 No test drives
start(env_path_prepend=…, plugin_tools_dir=…)— the abstractAgent.start()contract the orchestrator always supplies — in the new 1140-line suite; all three sibling agents pin it (test_codex_agent.py:166/:208, test_agent.py:313, test_antigravity_agent.py:1229). (trigger: tests/test_opencode_agent.py) _(restates: Axis 3: No test for OpenCodeAgent.build_env — the mock-CLI PATH-shadowing and PLUGIN_TOOLS_DIR contract is entirely uncovered) - 🟡 The
openrouter/prefix was added to BOTHpricing.py::_normalize_modeland its hand-copied mirrorpricing.ts::_ROUTING_PREFIXES, but only the Python side got a test (test_openrouter_provider_prefix_normalizes_to_bare_key).pricing-parity.test.tscompares onlyModelPricingROWS, never the prefix lists, and there is no vitest exercise ofnormalizeModelat all — so the drift guard CLAUDE.md credits with failing the build "in either direction" does not cover the thing this PR just changed in both directions. (trigger: evalboard/lib/pricing.ts) - 🟡
grep TurnStartEvent|TurnEndEvent tests/test_opencode_agent.pyreturns zero hits — nothing in the new suite asserts the one-TurnStartEvent/one-TurnEndEvent-per-step balance thatAgent.communicate's contract (agent.py:254-262) and CLAUDE.md's "Adding a New Agent" step 6 require, which is why the unbalanced stream shipped. (trigger: tests/test_opencode_agent.py) (restates: Axis 6: finalize() emits no TurnEndEvent for the step left open by a crash/timeout/cancel) - 🟡 No live/e2e exercise of the real CLI anywhere: Codex ships
tests/test_codex_agent_live.pyplus a dedicatedcodex-live-testsCI job (pr-checks.yml:792), while OpenCode is validated only against hand-written nd-JSON fixtures. The module's own warn-once machinery (_fresh_input_slice,_warn_token_shape, the vocabulary-drift crash) exists precisely because the CLI's stream schema is expected to move, and nothing in CI would see it move. (trigger: tests/test_opencode_agent.py) - 🔵 No golden-master snapshot for the new turn loop:
tests/_fixtures/golden_streams/carriesclaude_fixtures.pyandcodex_fixtures.pywith committed expected JSON, so both of those harnesses have a byte-level drift guard on the stream→TurnRecordmapping; the OpenCode mapping is pinned only by hand-written per-field asserts. (trigger: tests/test_opencode_agent.py) - 🔵
get_environment_info()is only partially tested —test_resolved_paths_are_recorded_for_auditassertsopencode_skill_paths, whileopencode_model,opencode_pure,opencode_variantandopencode_session_id(all persisted into run.json'senvironment_info, a report/evalboard-consumed record) have no assertion at all. (trigger: tests/test_opencode_agent.py) - 🔵 The new task file is referenced by nothing executable:
grep -rn opencode tests/ Makefile .github/matches only the two test modules, there is no repo-wide "every task YAML loads throughload_task" test on this branch, and no CI bucket (smoke-pass/smoke-fail/smoke-variants) selects it — so a malformed edit to it would surface as a skipped task in a green run. (trigger: tasks/opencode_smoke_test.yaml) (restates: Axis 7: Smoke task placed at tasks/ root and tagged smoke, contradicting the documented layout for per-agent probes)
Downstream consumers:
- 🟡 Two further contract surfaces still describe exactly two signals and name
parameters['skill']specifically, beyond the three the finding lists:docs/TASK_DEFINITION_GUIDE.md:1232(the criterion's public reference) andplugins/coder-eval/skills/check-skill/SKILL.md:204(shipped to users of the activation-suite skill). (trigger: src/coder_eval/criteria/skill_triggered.py) (restates: Axis 8: skill_triggered's own contract docs were not updated for the third detection signal) - 🔵 The
openrouter/strip in_normalize_modelis GLOBAL, not OpenCode-scoped: any recordedmodel_usedliterally prefixedopenrouter/— e.g. a LiteLLM route configured withLITELLM_MODEL=openrouter/<vendor>/<model>— now resolves to a rate-card entry where it previously resolved to none. Existing run artifacts are re-priced at evalboard render time (pricing.ts got the same prefix), and no test pins the previously-unpriced behaviour or notes the scope change. (trigger: src/coder_eval/pricing.py)
Display & mapping dicts:
- 🟡 The new
AgentKind.OPENCODEis absent from every evalboard harness display map:evalboard/lib/harness.ts(KNOWN_HARNESSESdisplay order,HARNESS_COLORS) andevalboard/app/_components/harness-badge.tsx(HARNESS_LOGO). Both fall back rather than break — an opencode run renders as raw text "opencode" in the runs table and takes the grayHARNESS_COLOR_FALLBACKseries color in the overview charts, sorted after the four known harnesses. (trigger: src/coder_eval/models/enums.py)
Daily/nightly:
- 🟡 Blast radius on the production path is unstated: OpenCode is a built-in on every install, yet it is absent from the Docker image, from every CI job's
uv syncextras, from all three smoke buckets, and from the evalboard'sKNOWN_HARNESSESnightly-rotation list. The PR does not say whether the nightly is expected to rotateopencodein, what image it would run under, or where its provider credentials come from. (trigger: src/coder_eval/agents/init.py) - 🟡 Backend selection is silently inert on this harness, and the preflight is inverted:
Orchestratorstill callssettings.validate_api_keys(agent.type)(orchestrator.py:1018), so under the nightly'sAPI_BACKEND=bedrock/litellman opencode run must present Bedrock/LiteLLM credentials it will never use (hardValueErrorif absent) while nothing pre-checks the provider key it does need — and--backend/routeitself is dropped into**_with no warning, unlike the four agent-config fieldsstart()does warn about. (trigger: src/coder_eval/agents/opencode_agent.py) (restates: Axis 2: OpenCodeAgent.init breaks the SPI factory contract —**_: Anyswallowsroute=) - 🔵 No OpenCode CLI version is captured anywhere in a run artifact:
get_environment_info()records model/pure/variant/session only, andutils.py's runtime-version capture knows justclaude -v. This is the one harness whose telemetry parsing carries explicit CLI-schema-drift warnings (_warn_token_shape, the vocabulary-drift crash, "re-check docs/agents/OPENCODE.md before trusting cost"), so when a nightly's token or cost figures step, nothing in the artifact identifies the CLI build that produced them. (trigger: src/coder_eval/agents/opencode_agent.py)
Harness & Lint Improvements
Proposals abbreviated — full rationale, rule shape and wiring in tmp/code-review-260817-0702/00-summary.md.
Static checks (lint / type):
- [ce-lint] CE055 — canonical tool-parameter keys have one declaration. Prevents: The merged A1/A5 finding at
skill_triggered.py:70(cmd.parameters.get("skill") or cmd.parameters.get("name")— OpenCode-specific knowledge pushed… - [pyright] Set
enableTypeIgnoreComments = falseandreportUnnecessaryTypeIgnoreComment = "error"in[tool.pyright](pyproject.toml), making# pyright: ignore[rule]the only suppression form. Prevents: A2 medium atopencode_agent.py:782(blanket ignore masking a wrongly-narrowlist[dict[str, Any]] | Noneparameter at its only call site — the veri… - [pyright] Make the None-safety warnings gate. Prevents: A2 low — the four new
reportOptionalMemberAccesswarnings atopencode_agent.py:404, 442, 611, 612, all from the non-narrowing `x.get("k") if isins… - [ce-lint] CE056 — no
** _Prevents:_ A2 medium / A5 low atopencode_agent.py:740-745—_: Anyswallows the always-passedroute=` undeclared and undocumented, defeats the deliberate… - [ce-lint] CE057 — layering:
agents/may not import fromisolation/. Prevents: A5 low atopencode_agent.py:48— the tree's firstagents/ → isolation/docker_runneredge, created for a stream-reader constant, which makes `impor… - [ce-lint] CE058 — guarded numeric casts on stream-parsed fields. Prevents: A6 medium at
opencode_agent.py:608-612— the module's only five unguarded field reads, which make_handle_line's advertised `"Never raises on bad… - [ce-lint] CE059 — no fail-open
getattr(obj, <non-literal>, None)insrc/. Prevents: A2 low atopencode_agent.py:775—[f for f in _UNSUPPORTED_CONFIG_FIELDS if getattr(self.config, f, None)], where renaming any ofsystem_prompt… - [ce-lint] CE060 — agent-roster doc parity. Prevents: A7 medium — 9-11 stale "Claude Code, Codex, and Gemini" roster sentences across
README.md(14-15, 20, 36, 43),docs/index.md(6-7, 16-17, 33, 40)… - [ce-lint] CE061 — Docker image / agent-kind parity. Prevents: A7 medium —
opencodeis an unconditional built-in (agents/__init__.py:8) whose CLI is in no image layer (docker/Dockerfile:37installs claude-co… - [ce-lint] CE062 — subprocess lifetime seam. Prevents: A6 high at
opencode_agent.py:1083—communicate()'sfinallycancels the stderr drain and drops the handle without reaping the child, so an `exce… - [ce-lint] CE063 — reserved domain terms may not be user-facing agent-config field names. Prevents: A7 low at
models/agent_config.py:296—OpenCodeAgentConfig.variant, which putsvariant_id: "baseline"andagent_config.variant: "high"in one… - [ce-lint] CE064 — per-agent task placement and tags. Prevents: A7 low at
tasks/opencode_smoke_test.yaml:7— a root-level task taggedsmokethat requires anopencodebinary CI runners do not have, so the umbr… - [bandit-codeql] Containment seam plus CodeQL path coverage. Prevents: A4 low at
opencode_agent.py:245—_manifest_skill_dirsreturns(root / relative).resolve()for values read verbatim from a third-party `plugin.j… - [ruff] Land the
C90mccabe ratchet onmain. Prevents: A1 medium atopencode_agent.py:1206— the five-function cluster at CC 14-22 headed by a dispatcher where four of five branches delegate and the fift…
Harness improvements (not statically reachable):
- A shared agent-conformance suite, parametrized over every registered agent Prevents: A3 high (no
_build_envtest at all —opencode_agent.py:883/885uncovered while all three siblings pin it), A3 medium (the>→>=mutation at `op… - A changed-lines coverage gate in
pr-checks.ymlPrevents: A3 medium (five uncovered skill-injection branches — list-form manifest at 241-242, unreadable/non-dict manifest at 235-236 and 237->243, non-local pl… - A boundary-mutation smoke job Prevents: A3 medium — the
>→>=flip atopencode_agent.py:1034was verified in review to leave all 65 tests green while turning a normal 2-step run under `… - Cross-harness telemetry parity fixtures. Prevents: A8 medium (
_TOOL_NAME_MAPnormalizes names whilestate.inputreachesCommandTelemetry.parametersverbatim, so atool_name: Readcriterion matc… - A per-agent docker-driver smoke matrix in CI Prevents: A7 medium —
opencodeis a registered built-in with no presence in any image layer, and the failure surfaces asshutil.which("opencode")returning… - A child-process leak assertion in the teardown tests Prevents: A6 high at
opencode_agent.py:1083(retry spawns a second CLI into the same sandbox and session) and A6/A4 low atopencode_agent.py:984(pgid regis…
Top 5 Priority Actions
- Reap the child in
communicate()'sfinally(src/coder_eval/agents/opencode_agent.py:1083) — it clearsself._processwithout killing, so an exception in the read loop raisesAgentCrashError, the AGENT_CRASH retry (max_retries=2) respawnsopencode --dir <sandbox> --session <same id>while attempt 1 is still editing the sandbox, and the criteria score whichever writer won; guard on a pre-initialisedproc(the spawn-failure path leaves it unbound) and add crash- and cancel-path teardown tests. - Test
_build_env's mock-CLI contract (src/coder_eval/agents/opencode_agent.py:883) — the PATH prepend and PLUGIN_TOOLS_DIR lines are both in the coverage-missing list and tests/test_opencode_agent.py makes zero assertions on them, so an inverted join order leaves sandbox mock CLIs un-shadowed and everycli_called/invocation-log row scores 0 with the suite still green; mirror the three codex-shaped tests at tests/test_codex_agent.py:166 and :208. - Make the zero-telemetry guard key on captured telemetry, not event vocabulary (src/coder_eval/agents/opencode_agent.py:1152) — a
step_finishpayload without atokenskey yields a COMPLETED SUCCESS withtoken_usage=None(or an all-zero record whencostis present), no warning, andrun_limits.max_usd/max_total_tokensthat can never trip, so extend the condition tostep_count > 0 and state.usage.is_empty()and pin it besideTestZeroTelemetryIsLoud(tests/test_opencode_agent.py:694). - Pin the
max_turnsboundary in both directions (gate at src/coder_eval/agents/opencode_agent.py:1034, sole test at tests/test_opencode_agent.py:918) — a>→>=mutation was measured to survive all 65 tests while flipping a normal 2-step run toFinalStatus.MAX_TURNS_EXHAUSTED(orchestrator.py:520) and suppressing the two crash guards at :1140 and :1152; add the sibling battery'smax_turns=2 → exhausted is Falsecase plus a folded-telemetry assertion (noteassistant_turn_countis 2 either way and discriminates nothing). - Fix tool-parameter handling at its one seam (src/coder_eval/agents/opencode_agent.py:413) — add an
_OPENCODE_ARG_RENAMEmap mirroring antigravity_agent.py:178-181 (filePath→file_path, andSkill: {name: skill}so skill_triggered.py:70 can revert to the agent-agnosticparameters.get("skill")), and refreshparameters/execution_started_aton a second event for an already-opencallID(the untested401->422branch, which otherwise freezesparameters == {}and zeroes everycommand_executedrow); anything left unfixed belongs in docs/agents/OPENCODE.md's Known-limitations list, alongside the missing docker-driver note at :240 and the stale three-harness roster sentences in README.md:36 and docs/index.md:33.
Stats: 0 🔴 · 2 🟠 · 13 🟡 · 13 🔵 across 8 axes reviewed.
Full per-axis breakdown (with the un-abbreviated Nits and Harness & Lint sections): tmp/code-review-260817-0702/01-code-quality.md … 08-harness-quality.md.
Nits and Harness & Lint were condensed to fit GitHub's 65 KB comment limit; nothing was dropped from Blockers, Non-blocking, or What's Missing.
…contract Closes the two review blockers on the OpenCode harness. communicate()'s `finally` cancelled the stderr drain and dropped the process handle without reaping the child. Two exits reach it with the CLI still RUNNING — the `except Exception` crash (a StreamReader ValueError on an over-long line, a malformed-payload TypeError in a handler) and an external cancellation — and neither passes through the graceful `await self.kill()` that the intentional cuts and _settle_turn use. That is not merely a leak: AgentCrashError is categorized AGENT_CRASH (max_retries=2) and the orchestrator's attempt-failure hook only drains pending_turn, so attempt 2 spawned a SECOND `opencode --dir <sandbox> --session <same id>` while attempt 1 was still editing the files the criteria were about to score — whichever writer won decided the task's result. Clearing the handle first also meant neither kill() nor kill_sync() could signal it afterwards, so off POSIX (where the group sweep is a no-op by design) nothing reaped it at all. _reap_orphaned_cli() now runs before the handle is dropped. It is deliberately synchronous: it executes while a CancelledError is propagating, where an await can itself be cut short and leave the child alive after all, so it uses Process.kill() plus the group sweep — no suspension point. Skipping the SIGTERM courtesy is right for a turn that is already lost; the graceful escalation in kill() still owns every path with something left to flush. A clean turn is untouched (the CLI has already exited, so the guard is a no-op and the server child survives for the next turn's --session resume). _build_env's mock-shadowing contract had zero assertions on it, though start(env_path_prepend=..., plugin_tools_dir=...) is the abstract Agent.start() contract and the orchestrator always supplies both. An inverted PATH join would leave sandbox mock CLIs un-shadowed, so a task grading a mocked CLI exercises the real binary, writes no invocation log, and scores 0 on every row with the suite still green. Both lines were in the coverage-missing list; all three sibling agents pin exactly this. Nine new tests, each verified to FAIL against the unfixed code: four teardown assertions (read-loop crash, external cancel, the POSIX group sweep on a crashed turn, and the clean turn that must kill nothing) plus five on the sandbox environment (ordered PATH prepend, PLUGIN_TOOLS_DIR exported, inherited PLUGIN_TOOLS_DIR never clobbered, neither key touched without the kwargs, host credentials inherited whole). Two mutations were confirmed caught: inverting the PATH join order, and letting the sandbox value override an inherited PLUGIN_TOOLS_DIR. _ExplodingRunningProcess is new because _ExplodingProcess could not model the case that matters: it inherits the plain fake's wait(), which reports an exit code the instant it is awaited, so the read loop never died with the CLI still alive. The teardown was extracted to a named method rather than inlined — communicate() was one statement over ruff's PLR0915 cap — which also gives the rationale a better home than a wall of comment inside a finally. _build_env gains a docstring recording why it may hardcode "PATH" where CodexAgent may not: it seeds from os.environ, whose keys CPython upper-cases on Windows, instead of handing the SDK a partial dict merged over the real environment. make verify green: 4,171 tests, coverage gate met (91.76% on the module).
…ax_turns
Three measurement-integrity items from the review, all of which could change a
task's score or final_status for identical agent output.
Zero-telemetry guard keys on the TELEMETRY, not the event vocabulary.
`recognized_events == 0` left the identical outcome reachable one layer down: a
`step_finish` carrying no `tokens` key (a provider or auth mode that omits usage)
recognizes three events, books an all-zero TokenUsage, and EventCollector maps
that to `token_usage=None` — a COMPLETED turn with no tokens, no cost and no
warning, which a file-based criterion can still score SUCCESS, which is absent
from every token aggregate, and whose run_limits.max_total_tokens / max_usd gates
could never trip no matter what the run really billed. The second arm keys on a
step the CLI reported as FINISHED — its own claim that a generation completed —
rather than on `usage.is_empty()` alone, which would also condemn a stream cut
before any step could finish. Intentional cuts stay exempt.
Tool ARGUMENT keys are now normalized alongside tool names. _TOOL_NAME_MAP did
half the job: `command_executed` serializes `parameters` to JSON for every tool
but Bash, so `{tool_name: Read, command_pattern: 'file_path.*app\.py'}` matched
on Claude and scored 0 on OpenCode for identical behaviour. _OPENCODE_ARG_RENAME
mirrors antigravity's _ANTIGRAVITY_ARG_RENAME and is applied at the one seam
where `parameters=` is built.
The review proposed mapping `filePath` -> `file_path`, from the PR's own live
capture. Reading the tool schemas the installed CLI actually registers shows it
now uses `path` for read/write/edit (`{path, oldString, newString, replaceAll}`),
so that map alone would have renamed nothing on a current build. Both spellings
are accepted; the search tools' `path` and Bash's `command` already match
Claude's names and are left alone, which is why the map is keyed per canonical
tool rather than applied globally.
With `Skill: {name -> skill}` at the agent boundary, skill_triggered reverts to
the agent-agnostic `parameters.get("skill")`. Carrying a per-harness alternative
in a criterion that must know nothing about harnesses would make every future
harness edit it, and `parameters` is substring-scanned two lines below, so
widening the key set there was the riskier of the two places.
on_tool_use also stopped freezing the first event's view of a call. The CLI may
emit pending/running before completed for one callID, and the first event
routinely carries no `input` — so `parameters` stayed `{}` permanently and every
command_executed row scored 0 while the run looked normal. Later evidence now
wins; absent evidence never clears what is already held.
max_turns is pinned in both directions. The sole test asserted only that
`max_turns=1` sets the flag, which a `>` -> `>=` mutation survives while turning
a normal 2-step run under `max_turns: 2` into FinalStatus.MAX_TURNS_EXHAUSTED —
and a spurious exhaustion also suppresses the non-zero-exit and zero-telemetry
crash guards, so such a run would score silently instead of failing loudly. Added
the cap-not-reached case, the uncapped case, and a deciding-step-kept-whole case
asserting step 1's exact token buckets survive the cut.
Every fix was mutation-checked rather than trusted green: `>` -> `>=` and
counting finished instead of started steps (both now caught, the first
previously survived the whole suite); reverting the guard to event-vocabulary
only; disabling the arg rename; and applying it tool-agnostically.
docs/agents/OPENCODE.md documents the argument-key table (with the CLI version
drift), the widened guard's two shapes, and the matching troubleshooting entry.
No evalboard mirror is needed: pickArgText already falls through file_path ->
filePath -> path -> skill, so canonical keys hit its preferred entry earlier and
existing run artifacts still render.
make verify green: 4,189 tests, coverage gate met.
Seven medium findings; the docker one takes the documentation route. Factory contract (A2). __init__ declared `**_: Any`, which swallowed the always-passed `route=` undeclared. create_agent calls `agent_class(config, route=route, **kwargs)` through a `cast(Any, ...)`, so pyright checks nothing at the call site — with a sink on this side, nothing checked it at runtime either, and the TypeError the orchestrator deliberately relies on (it gates cost_log_tags on supports_cost_log_tags precisely so an ungated forward crashes loudly) was silently absorbed instead. Every parameter is now declared; `route` is kept and documented as unused, since the CLI owns its own provider configuration. Types (A2). `_plugin_skill_dirs(plugins=...)` was annotated `list[dict[str, Any]] | None` while the config supplies `list[LocalPluginConfig] | None`, and the mismatch was papered over with a `# type: ignore[arg-type]` — which pyright treats as BLANKET, suppressing every diagnostic on its only call site. Widened to `Sequence[Mapping[str, Any]] | None` and the suppression removed: 0 errors, no ignore. Turn events (A6). finalize() emitted no TurnEndEvent for a step left open by a crash, timeout, cancel, or either clean cut, so the last TurnStartEvent was never closed — a task.log with `>>> Turn start` and no matching `--- Turn end`, and a violation of Agent.communicate's one-pair-per-inner-turn contract that all three siblings honor. Unlike them, completed steps here already close themselves in on_step_finish, so a `step_open` flag makes finalize fire for the straggler only. Nothing in the suite asserted this balance before; six tests now do. Token casts (A6). on_step_finish's five bare int() casts were the module's only unguarded field reads, contradicting _handle_line's advertised "Never raises on bad input" — and every neighbouring field already warns-and-continues on drift. Raising here is expensive: AGENT_CRASH retries twice, so ONE mistyped bucket burned three attempts and landed the task as ERROR. `_as_int` warns once and counts 0 instead, keeps numeric strings and floats, and rejects bool (int(True) would book a phantom token). Dispatch (A1). _handle_line inlined the error branch while its four siblings delegated; moved to `_OpenCodeTurnState.on_error`, making the dispatch uniform and the payload-shape handling directly testable (7 shapes pinned). Skill-injection coverage (A3). Six previously-untested branches: list-form manifest (incl. non-string entries), all four unusable-manifest fallbacks, the non-local plugin entry, the missing-SKILL.md warning, and the malformed inherited OPENCODE_CONFIG_CONTENT paths. Two review assumptions did not survive contact: the missing-SKILL.md case still INJECTS (the CLI scans recursively; the warning is advisory), and the non-local branch is unreachable through the typed config (LocalPluginConfig pins `type: Literal["local"]`), so it is driven against _plugin_skill_dirs directly and the test says why. Docker (A7). Documented rather than implemented, as agreed: docs/agents/OPENCODE.md gains a "Running in Docker" section stating the driver is unsupported and naming both gaps — the CLI is absent from the image (adding it needs a pinned version that travels with the release tag, as CLAUDE_CODE_VERSION does) and no OpenCode credentials are in SandboxConfig.env_passthrough, so even a custom image would authenticate against nothing — plus the build-your-own workaround, a Known- limitations bullet, and a correction to the Dockerfile's own comment, which claimed "all built-in agents ship in every build" and this agent made false. Agent roster (A7). Ten hand-written "Claude Code, Codex, and Gemini" sentences across README.md, docs/index.md, docs/llms.txt and docs/USER_GUIDE.md's `--type` table still omitted OpenCode; `make docs-indexes` cannot repair them (all sit outside the generated markers) and was re-run to confirm no drift. README's `coder-eval[codex,antigravity]` install line is deliberately left alone — the `[opencode]` extra is empty by design, so listing it would imply pip installs a Node CLI. Mutation-checked, not trusted green: finalize never closing the open step, accepting bool as a token count, and casting via str() (which caught a genuine gap — no test fed a float, though _as_int admits one) all now fail. make verify green: 4,223 tests, coverage gate met (91.73%).
…zero-token guard The zero-token guard makes a turn that finished steps without booking any tokens a hard crash. That is right by default — such a turn is absent from every token aggregate and its max_total_tokens / max_usd gates can never trip — but it has no override, so a provider or auth mode that genuinely reports no usage would fail EVERY turn and make the harness unusable rather than merely imprecise. (The docs already note OpenCode reports `cost: 0` under subscription-style auth, so a usage- omitting mode is not hypothetical.) `agent.require_token_telemetry: false` downgrades that arm to a warn-and-score. Reachable from YAML, an experiment variant, or `-D agent.require_token_telemetry=false` like any other agent field, with the resolver's did-you-mean on a typo. Deliberately scoped to the missing-token arm only: a stream with NO recognized events still fails even with the hatch open. That arm is event-vocabulary drift, which has silently zeroed a whole run once already, and no provider quirk explains a renamed vocabulary — so one flag must not reopen both holes. Implementation is one field plus one branch at the single existing guard site; the message is unchanged and merely hoisted to a variable so both paths share it. make verify green: 4,225 tests, coverage gate met.
3 participants
Adds a new agent kind
opencode(registered via the existing plugin SPI, selectable withagent.type: opencodeor-D agent.type=opencode) that drivesopencode run --format jsonand reduces its nd-JSON stream into the standard event protocol —EventCollectorbuilds theTurnRecord, no hand-assembled telemetry. Since OpenCode is model-agnostic, this is the cheap path for evaluating open-weight models (DeepSeek, Kimi, GLM, …) through one harness.Highlights
Agentcontract: pending-turn crash/timeout semantics, cooperativeshould_stop, one terminal event per turn;max_turnscounts OpenCode's native steps (documented in the run-limit parity table).total; the reconciliation invariant holds exactly; tool names normalized to the canonical vocabulary.openrouter/model ids normalize to bare rate-card keys (mirrored in evalboard).[opencode]extra is deliberately empty — OpenCode is a Node CLI; a missing binary fails atstart()with the install command.Model: everything is standardized on DeepSeek V4 Pro (
openrouter/deepseek/deepseek-v4-pro) — the smoke task, docs examples, and tests — so the checked-in task runs out of the box. DeepSeek V4 Flash 0731 was dropped: every OpenRouter provider serving it is excluded by the account's data-policy settings (an account-level restriction, not a code issue), so no reference to it ships.Validation (all on DeepSeek V4 Pro via OpenRouter):
make verifygreen (4,149 tests, coverage gate met); live end-to-end runs confirm real telemetry with exact reconciliation (including cache-heavy traffic), zero warnings, zero leaked processes,--sessionresume across turns, and bothtasks/run_limits/parity fixtures behaving per contract (clean cap stop vs. timeout failure).Docs:
docs/agents/OPENCODE.md(wired into nav + generated indexes) + an OpenCode column indocs/agents/HARNESS_PARITY.md.Results — the same 174 tasks on two models
174 tasks from
skills/tests/tasks/across 10 skill directories, run through the OpenCode harness.coder_eval@fcb7dad,skills@1e16eedb,-e tests/experiments/default.yaml --type opencode -j 4— tempdir driver,max_turns: 200,task_timeout: 1200,turn_timeout: 900. Identical task list, experiment, harness and concurrency for both; only-mdiffers.global.profile,/v1/responses)SkillcallsGPT-5.6 Luna wins on every axis: +5.1 pp pass rate, 2.6× faster, and ~17× cheaper — $0.021 vs $0.388 per passing task. It also produced no timeouts at all, where DeepSeek lost 6 tasks to the 1200s task cap.
By skill directory
The two models agree on most of the suite: 133 tasks pass on both, 18 fail on both, 7 pass only on DeepSeek and 16 only on Luna.
uipath-testis the shared floor at 8/18 for both — a suite-side gap rather than a model gap. Luna's gains concentrate inuipath-human-in-the-loop(+19 pp) anduipath-tasks(+33 pp); the singleuipath-platformregression is a one-task directory.Tokens and cost basis
Neither model has a rate-card entry in
src/coder_eval/pricing.py, so both runs report$0.00. Costs above are computed by hand from the models.dev catalog entry for the provider each run actually used, so the two are priced from one source:azure): $1.74/M in, $3.48/M out. Azure publishes no cache-read rate; every provider at this same tier lists ~$0.145/M, used here as an estimate.amazon-bedrock / openai.gpt-5.6-luna): $0.22/M in, $1.32/M out, $0.022/M cache read, $0.275/M cache write (base tier; a higher tier applies above 272k context).Caveat on the cost gap: part of it is billing shape, not just the rate card. The Bedrock route cached the prompt prefix (5.7M cache writes, 48M cache reads, almost no uncached input), while the Azure route billed 27.3M tokens at full input rate — that single bucket is $47.58 of DeepSeek's $54.32.