From ce380dcd07452d37dd3f5fc78b7afbf2aafb6654 Mon Sep 17 00:00:00 2001 From: mohsen-uipath Date: Fri, 14 Aug 2026 09:46:01 -0700 Subject: [PATCH 01/12] feat(agents): add OpenCode harness with opt-in [opencode] extra MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- README.md | 1 + docs/EXTENDING.md | 4 +- docs/agents/OPENCODE.md | 231 ++++ docs/index.md | 1 + docs/llms.txt | 1 + .../lib/__tests__/pricing-parity.test.ts | 4 + evalboard/lib/pricing.ts | 12 +- mkdocs.yml | 2 + pyproject.toml | 19 + src/coder_eval/agents/__init__.py | 14 +- src/coder_eval/agents/opencode_agent.py | 1018 +++++++++++++++++ src/coder_eval/models/__init__.py | 2 + src/coder_eval/models/agent_config.py | 34 +- src/coder_eval/models/enums.py | 1 + src/coder_eval/pricing.py | 10 +- tasks/opencode_smoke_test.yaml | 34 + tests/test_opencode_agent.py | 796 +++++++++++++ tests/test_pricing_registry.py | 14 +- uv.lock | 2 +- 19 files changed, 2189 insertions(+), 11 deletions(-) create mode 100644 docs/agents/OPENCODE.md create mode 100644 src/coder_eval/agents/opencode_agent.py create mode 100644 tasks/opencode_smoke_test.yaml create mode 100644 tests/test_opencode_agent.py diff --git a/README.md b/README.md index 11cd4420..5af70112 100644 --- a/README.md +++ b/README.md @@ -209,6 +209,7 @@ alone. | [Claude Code](docs/agents/CLAUDE_CODE.md) | Configuring and running the default Claude Code agent | | [Codex](docs/agents/CODEX.md) | Running the OpenAI Codex agent | | [Antigravity (Gemini)](docs/agents/ANTIGRAVITY.md) | Running the Google Antigravity / Gemini agent | +| [OpenCode](docs/agents/OPENCODE.md) | Running the OpenCode agent on open-weight models | | [A/B Experiments](docs/AB_EXPERIMENTS.md) | Compare models / tools / prompts across the same tasks | | [Bring Your Own Dataset](docs/DATASETS.md) | Fan a single task out over a dataset | | [Dialog Mode](docs/DIALOG_MODE.md) | Evaluate agents in multi-turn conversation via a simulated user | diff --git a/docs/EXTENDING.md b/docs/EXTENDING.md index 6c681bcd..1425354d 100644 --- a/docs/EXTENDING.md +++ b/docs/EXTENDING.md @@ -267,8 +267,8 @@ The base package ships **no** plugin rates; only the built-in table. ## See also - [Claude Code](agents/CLAUDE_CODE.md) · [Codex](agents/CODEX.md) · - [Antigravity](agents/ANTIGRAVITY.md) — the built-in agents, each registered via - this same SPI + [Antigravity](agents/ANTIGRAVITY.md) · [OpenCode](agents/OPENCODE.md) — the + built-in agents, each registered via this same SPI - [Task Definition Guide](TASK_DEFINITION_GUIDE.md) — the criterion catalogue - [CLAUDE.md](https://github.com/UiPath/coder_eval/blob/main/CLAUDE.md) — architecture and extension points in depth diff --git a/docs/agents/OPENCODE.md b/docs/agents/OPENCODE.md new file mode 100644 index 00000000..6844ecdd --- /dev/null +++ b/docs/agents/OPENCODE.md @@ -0,0 +1,231 @@ +--- +description: >- + Run OpenCode, the open-source terminal coding agent, as the agent under + evaluation in Coder Eval — installation, provider authentication, model + selection, and how its event stream maps to sandboxed, weighted scoring. +--- + +# Running OpenCode in Coder Eval + +## Overview + +[OpenCode](https://opencode.ai) is an open-source terminal coding agent. Coder Eval +drives it **non-interactively**: + +```bash +opencode run --format json -m --dir --auto --pure "" +``` + +`--format json` streams **newline-delimited JSON events** on stdout, one event per +line. `OpenCodeAgent` reduces that stream into the standardized event protocol +(`AgentStart` / `TurnStart` / `ToolStart` / `ToolEnd` / `TurnEnd` / `AgentEnd`) and +lets `EventCollector` build the `TurnRecord`, exactly like every other harness. + +Because OpenCode is model-agnostic, this is the cheapest way to evaluate a broad +set of open-weight models (DeepSeek, Kimi, GLM, …) through a single agent. + +## Setup + +### 1. Install the OpenCode CLI + +OpenCode is a **Node** CLI, not a Python package: + +```bash +npm install -g opencode-ai +opencode --version +``` + +The `coder-eval[opencode]` extra exists for symmetry with the other harnesses and +carries **no Python dependencies** — the agent shells out to the binary above and +imports no third-party package: + +```bash +uv sync --extra opencode # documents the opt-in; installs no extra packages +``` + +> The `opencode-ai` package on **PyPI** is an HTTP client for `opencode serve`, a +> different integration surface than the CLI this harness drives. You do not need +> it. + +If the binary is missing, the task fails at `start()` with an actionable error +naming the install command, rather than failing obscurely mid-run. + +### 2. Authentication + +OpenCode resolves credentials itself, per provider. Either log in once: + +```bash +opencode auth login +``` + +…or export the provider key, which Coder Eval forwards into the subprocess +environment (it also picks up keys from `.env`, since `config.py` calls +`load_dotenv(override=True)`): + +```bash +export OPENROUTER_API_KEY="sk-or-..." # OpenRouter (any model) +export DEEPSEEK_API_KEY="sk-..." # DeepSeek first-party +``` + +Check what a given model id resolves to with: + +```bash +opencode models | grep +``` + +## Usage + +### Command line + +```bash +uv run coder-eval run tasks/opencode_smoke_test.yaml +uv run coder-eval run tasks/my_task.yaml -D agent.type=opencode -D agent.model=openrouter/deepseek/deepseek-v4-pro +``` + +### Task definition (YAML) + +```yaml +agent: + type: "opencode" + # provider/model, exactly as `opencode models` prints it. + model: "openrouter/deepseek/deepseek-v4-flash-0731" + permission_mode: "acceptEdits" + variant: "high" # optional: provider reasoning effort + pure: true # optional (default): run with --pure, no host plugins +``` + +### Model selection + +`agent.model` is passed through verbatim to `-m`, so it must be OpenCode's +`provider/model` form. The provider prefix decides which credential is used: + +| `agent.model` | Provider | Credential | +|---|---|---| +| `openrouter/deepseek/deepseek-v4-flash-0731` | OpenRouter | `OPENROUTER_API_KEY` | +| `deepseek/deepseek-v4-flash-0731` | DeepSeek direct | `DEEPSEEK_API_KEY` | + +OpenCode speaks OpenRouter natively, so it does **not** need the LiteLLM proxy — +that shim exists to translate Anthropic ↔ OpenAI for the Claude Code SDK. + +### `variant` + +Provider-specific reasoning effort (`minimal` / `high` / `max`, provider +dependent), forwarded as `--variant`. Omit to take the provider default. + +### `pure` + +Defaults to `true`, forwarding `--pure` so the sandbox is isolated from host-level +OpenCode plugin configuration. This mirrors the rationale behind the Claude agent's +`setting_sources: []`. Set to `false` to load host plugins deliberately. + +## Permissions + +Every `permission_mode` except `plan` passes `--auto`, auto-approving tool use. +This is required for unattended evaluation — without it OpenCode blocks on an +interactive approval prompt and the turn runs to its timeout. Use +`permission_mode: plan` when you explicitly want approvals withheld. + +## Telemetry + +Mapping from the CLI's event vocabulary onto `TurnRecord`: + +| OpenCode event | Becomes | +|---|---| +| `step_start` | `TurnStartEvent` (one inner turn) | +| `text` | `TextChunkEvent` + `agent_output` | +| `tool_use` | `ToolStartEvent` + `ToolEndEvent` (one terminal event carries both) | +| `step_finish` | `TurnEndEvent` + per-step tokens/cost, one `AssistantMessage` | +| `error` | `AgentCrashError` with the partial turn preserved | + +Token buckets come from `step_finish.tokens`. Two conventions for `tokens.input` +exist in the wild, and the stream's own `total` arbitrates **per step**: + +- **flat** (what the current CLI emits, verified live — + `7966 = 6796 + 128 + 18 + 1024` exactly): `input` already *is* the fresh slice, + and `total = input + output + reasoning + cache.read + cache.write`; +- **nested** (the OpenAI `prompt_tokens` convention): cached tokens are counted + inside `input`, `total = input + output + reasoning`, so the fresh slice is + `input - cache.read - cache.write`. + +With no cache traffic the two agree. With no usable `total` the flat reading is +taken — logged as a warning when cache traffic is present, since the convention +then cannot be verified. A `total` matching **neither** also warns (once per +turn) that the bucket mapping may no longer match the CLI — so a drifting schema +is visible in `task.log` instead of quietly mis-costing every run. Reasoning +tokens are folded into `output_tokens` (they bill at the output rate) while +remaining visible as `reasoning_tokens` per message. This keeps the +reconciliation invariant exact: summing the four buckets across +`TurnRecord.messages` equals `token_usage`. + +Real per-call cost rides on `step_finish.cost` and lands on +`token_usage.total_cost_usd`, so runs are costed from the provider's own +accounting rather than the static rate card. The rate card +(`calculate_cost` over the captured buckets) fills two gaps so the run total +never books tokens with no money: a stream that reports **no** cost at all (a +provider or auth mode that omits it, or a turn that died before its first +`step_finish`), and a stream that reports **`cost: 0`** for tokens the rate +card prices above zero — OpenCode reports 0 when its own model registry has no +price for the model, or under subscription-style auth, and neither means the +tokens were free (the fallback logs a warning naming the substituted amount). A +*non-zero* cost the CLI reported always wins, and a genuinely free model still +resolves to $0 because its rate entry is absent or all-zero. + +Tool names are normalized to the canonical (Claude) vocabulary on capture — +`bash` → `Bash`, `read` → `Read`, `write`/`edit`/`patch` → `Write`/`Edit`, and so +on; an unmapped tool keeps its own name. This is what lets one +`command_executed` criterion (which filters on `tool_name` and reads a `Bash` +call's `command` parameter) score identically whether the run used Claude, Codex +or OpenCode. + +> These event names are the CLI's own compact vocabulary. They are **not** the +> `session.next.*` names in the OpenAPI schema served by `opencode serve` — that +> describes the HTTP/SSE surface and does not apply here. + +Vocabulary drift is crashed, not scored: a turn whose CLI exits cleanly but whose +stream contained **no recognized events** captured zero telemetry (zero turns, +tokens and cost) while file-based criteria could still pass on whatever the agent +did — a success that silently vanishes from every aggregate. The turn is failed +with an error naming the unrecognized event types it saw instead. + +## Known limitations + +- **`allowed_tools` / `disallowed_tools` / `system_prompt` / `system_prompt_file` / + `plugins` are not enforced.** The CLI exposes no equivalent knob, so these are + dropped — `start()` logs a warning naming each one it saw (`experiments/default.yaml` + sets `allowed_tools` on every task, so expect it on a default run). Do not rely on + them as a boundary here, and note that skill-injection suites, which depend on + `plugins`, cannot run on this harness. +- **No sub-agent attribution.** OpenCode's CLI stream does not expose nested agent + generations, so per-sub-agent token grouping (available for Claude and Codex) is + not derivable. +- **Cooperative stop is at event granularity.** `should_stop` is polled between + events and honored by terminating the CLI, so `stop_early` works, but the cut + lands on an event boundary rather than mid-tool. +- **Pipe teardown.** `opencode run` leaves a local server child holding the + inherited stdout/stderr pipes, so EOF never arrives on its own. The agent races + each read against process exit and bounds the post-exit drain; this is why reads + are never left to block on EOF alone. stderr gets its own concurrent reader from + the moment the CLI starts — draining it only afterwards would let a full stderr + pipe block the child mid-write and stall stdout with it. + +## Troubleshooting + +**`No endpoints available matching your guardrail restrictions and data policy`** +— an OpenRouter *account* setting, not a Coder Eval problem. The model's serving +providers are all excluded by your account's privacy/guardrail configuration. +Verify independently with a direct API call, then adjust at +[openrouter.ai/settings/privacy](https://openrouter.ai/settings/privacy). + +**Task fails with `emitted no recognized events`** — the CLI exited cleanly but +nothing on its stdout matched the event vocabulary above, so the turn would have +scored with zero telemetry; the harness fails it instead. The error names the +event types it did see. Check the raw stream with +`opencode run --format json ... > raw.jsonl` and compare the `type` values +against the table above — an OpenCode upgrade that renames them needs a matching +harness update. + +## References + +- [OpenCode docs](https://opencode.ai/docs/) · [CLI reference](https://opencode.ai/docs/cli/) +- [Extending Coder Eval](../EXTENDING.md) — the agent plugin SPI +- [Task Definition Guide](../TASK_DEFINITION_GUIDE.md) diff --git a/docs/index.md b/docs/index.md index 6c9ad50d..421ac4ab 100644 --- a/docs/index.md +++ b/docs/index.md @@ -81,6 +81,7 @@ New here? Start with **[Tutorial 01 — Your First Evaluation](tutorials/01-firs | [Claude Code](agents/CLAUDE_CODE.md) | Configuring and running the default Claude Code agent | | [Codex](agents/CODEX.md) | Running the OpenAI Codex agent | | [Antigravity (Gemini)](agents/ANTIGRAVITY.md) | Running the Google Antigravity / Gemini agent | +| [OpenCode](agents/OPENCODE.md) | Running the OpenCode agent on open-weight models | | [A/B Experiments](AB_EXPERIMENTS.md) | Compare models / tools / prompts across the same tasks | | [Bring Your Own Dataset](DATASETS.md) | Fan a single task out over a dataset | | [Dialog Mode](DIALOG_MODE.md) | Evaluate agents in multi-turn conversation via a simulated user | diff --git a/docs/llms.txt b/docs/llms.txt index ea733060..38b139f1 100644 --- a/docs/llms.txt +++ b/docs/llms.txt @@ -28,6 +28,7 @@ and A/B plumbing. - [Claude Code](https://coder-eval.com/docs/agents/claude-code): Configuring and running the default Claude Code agent - [Codex](https://coder-eval.com/docs/agents/codex): Running the OpenAI Codex agent - [Antigravity (Gemini)](https://coder-eval.com/docs/agents/antigravity): Running the Google Antigravity / Gemini agent +- [OpenCode](https://coder-eval.com/docs/agents/opencode): Running the OpenCode agent on open-weight models - [A/B Experiments](https://coder-eval.com/docs/ab-experiments): Compare models / tools / prompts across the same tasks - [Bring Your Own Dataset](https://coder-eval.com/docs/datasets): Fan a single task out over a dataset - [Dialog Mode](https://coder-eval.com/docs/dialog-mode): Evaluate agents in multi-turn conversation via a simulated user diff --git a/evalboard/lib/__tests__/pricing-parity.test.ts b/evalboard/lib/__tests__/pricing-parity.test.ts index 0907fd94..dce4d7a9 100644 --- a/evalboard/lib/__tests__/pricing-parity.test.ts +++ b/evalboard/lib/__tests__/pricing-parity.test.ts @@ -104,9 +104,13 @@ describe("pricing.ts ↔ pricing.py parity", () => { // max_usd static fallback. The evalboard deliberately does NOT statically // price them — OpenRouter routes per-request, so it shows the captured // ACTUAL per-call cost instead (see the per-call table, provider_call_costs). + // Same reasoning under the OpenCode harness, which addresses OpenRouter + // natively: it reports the provider's own per-step cost, which the turn + // carries as token_usage.total_cost_usd. "moonshotai/kimi-k3", "z-ai/glm-5.2", "deepseek/deepseek-v4-pro", + "deepseek/deepseek-v4-flash-0731", ]); test("every DELIBERATELY_UNMIRRORED id still exists in pricing.py", () => { diff --git a/evalboard/lib/pricing.ts b/evalboard/lib/pricing.ts index a96f165a..988828c0 100644 --- a/evalboard/lib/pricing.ts +++ b/evalboard/lib/pricing.ts @@ -116,7 +116,17 @@ function p( // pricing key — mirror of src/coder_eval/pricing.py::_normalize_model, since the // recorded model_used arrives qualified (e.g. "converse/zai.glm-5", // "eu.anthropic.claude-sonnet-4-6"). Idempotent on already-bare ids. -const _ROUTING_PREFIXES = ["bedrock/converse/", "bedrock/", "converse/"]; +// "openrouter/" is here for the same reason it is in _normalize_model: a harness +// that addresses OpenRouter natively (OpenCode) records the model WITH its +// provider prefix ("openrouter/deepseek/deepseek-v4-pro"), while the rate keys +// are the bare vendor/model ids the LiteLLM route records. Without the strip the +// same model normalizes differently depending on which harness produced the run. +const _ROUTING_PREFIXES = [ + "bedrock/converse/", + "bedrock/", + "converse/", + "openrouter/", +]; const _REGION_PREFIXES = ["eu.", "us.", "apac.", "global."]; function normalizeModel(model: string): string { let m = model.trim(); diff --git a/mkdocs.yml b/mkdocs.yml index 8fcb00c7..4489a4fa 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -83,6 +83,7 @@ extra: agents/CLAUDE_CODE.md: "Configuring and running the default Claude Code agent" agents/CODEX.md: "Running the OpenAI Codex agent" agents/ANTIGRAVITY.md: "Running the Google Antigravity / Gemini agent" + agents/OPENCODE.md: "Running the OpenCode agent on open-weight models" AB_EXPERIMENTS.md: "Compare models / tools / prompts across the same tasks" DATASETS.md: "Fan a single task out over a dataset" DIALOG_MODE.md: "Evaluate agents in multi-turn conversation via a simulated user" @@ -111,6 +112,7 @@ nav: - Claude Code: agents/CLAUDE_CODE.md - Codex: agents/CODEX.md - Antigravity (Gemini): agents/ANTIGRAVITY.md + - OpenCode: agents/OPENCODE.md - Advanced: - A/B Experiments: AB_EXPERIMENTS.md - Bring Your Own Dataset: DATASETS.md diff --git a/pyproject.toml b/pyproject.toml index 558fa3d2..a017f887 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -106,6 +106,25 @@ codex = [ antigravity = [ "google-antigravity==0.1.7", ] +# Optional extra that enables OpenCode agent support. +# +# Deliberately EMPTY, and that is not an oversight. OpenCode ships as a Node CLI +# (`npm install -g opencode-ai`), and OpenCodeAgent drives that binary over a +# subprocess reading `opencode run --format json` — it imports no third-party +# Python package, so there is nothing for a Python extra to install. The extra +# exists to keep the opt-in surface uniform across harnesses (`--extra opencode` +# is a real, documented step) and to give the prerequisite a single home in the +# packaging metadata. +# +# NOTE: the `opencode-ai` package on PyPI is an HTTP client for `opencode serve`, +# a DIFFERENT integration surface than the CLI this harness drives. Do not add it +# here to make the list non-empty: nothing imports it, and listing it would imply +# a dependency that does not exist. +# +# Without the CLI on PATH the framework still installs and runs; OpenCode tasks +# fail at start() with a clear hint pointing back here. +# See docs/agents/OPENCODE.md. +opencode = [] [project.scripts] coder-eval = "coder_eval.cli:app" diff --git a/src/coder_eval/agents/__init__.py b/src/coder_eval/agents/__init__.py index 5566e386..0bbf0dde 100644 --- a/src/coder_eval/agents/__init__.py +++ b/src/coder_eval/agents/__init__.py @@ -5,12 +5,13 @@ from coder_eval.agents.claude_code_agent import ClaudeCodeAgent from coder_eval.agents.codex_agent import CodexAgent from coder_eval.agents.noop_agent import NoOpAgent +from coder_eval.agents.opencode_agent import OpenCodeAgent from coder_eval.agents.registry import AgentRegistry, create_agent from coder_eval.models import AgentKind def register_builtins(registry: type[AgentRegistry]) -> None: - """Register the built-in agents (Claude/Codex/Antigravity/NoOp) onto ``registry``. + """Register the built-in agents (Claude/Codex/Antigravity/OpenCode/NoOp) onto ``registry``. This is the target of coder-eval's own ``coder_eval.plugins`` entry point, so the built-in agents travel the identical discovery path as any third-party @@ -20,12 +21,18 @@ def register_builtins(registry: type[AgentRegistry]) -> None: """ # Reference the imported classes so the registration side effect is explicit # and a future refactor that drops the top-level imports fails loudly here. - _ = (ClaudeCodeAgent, CodexAgent, AntigravityAgent, NoOpAgent) + _ = (ClaudeCodeAgent, CodexAgent, AntigravityAgent, OpenCodeAgent, NoOpAgent) # Rot-protection: the decorators fire on import, but assert the built-ins are # actually registered so a future lazy-import refactor (which would leave the # import-cached modules' decorators un-run) fails loudly instead of silently # registering nothing. - for kind in (AgentKind.CLAUDE_CODE, AgentKind.CODEX, AgentKind.ANTIGRAVITY, AgentKind.NONE): + for kind in ( + AgentKind.CLAUDE_CODE, + AgentKind.CODEX, + AgentKind.ANTIGRAVITY, + AgentKind.OPENCODE, + AgentKind.NONE, + ): if registry.get(kind) is None: raise RuntimeError(f"register_builtins: built-in agent kind {kind!r} did not register") @@ -36,6 +43,7 @@ def register_builtins(registry: type[AgentRegistry]) -> None: "ClaudeCodeAgent", "CodexAgent", "NoOpAgent", + "OpenCodeAgent", "create_agent", "register_builtins", ] diff --git a/src/coder_eval/agents/opencode_agent.py b/src/coder_eval/agents/opencode_agent.py new file mode 100644 index 00000000..94f653d3 --- /dev/null +++ b/src/coder_eval/agents/opencode_agent.py @@ -0,0 +1,1018 @@ +"""OpenCode agent implementation (the open-source terminal coding agent). + +Drives the ``opencode`` CLI in non-interactive mode:: + + opencode run --format json -m --dir [--auto] [--pure] + +which streams **newline-delimited JSON events** on stdout. Each line is one +event; this module reduces that stream into the standardized coder_eval event +protocol (``AgentStart`` / ``TurnStart`` / ``ToolStart`` / ``ToolEnd`` / +``TurnEnd`` / ``AgentEnd``) and lets :class:`EventCollector` build the +``TurnRecord`` — so no telemetry is assembled by hand here. + +Envelope normalization +---------------------- +The CLI emits two envelope shapes on the same stream: the normal form carries +its payload under ``part`` — ``{"type": "tool_use", "sessionID": …, +"part": {…}}`` — while the CLI's own error path emits a flat object with no +``part`` (``{"type": "error", "sessionID": …, "error": {…}}``). :func:`_unwrap` +normalizes both to ``(event_type, payload)`` so the dispatch table is written +once. (The ``session.next.*``/``properties`` envelopes belong to ``opencode +serve``'s HTTP/SSE surface and never appear here — see the note on the event +constants below.) + +Session continuity +------------------ +The ``sessionID`` observed on the first event is retained and replayed via +``--session`` on the next ``communicate()`` call, which is what makes multi-turn +(dialog-mode) evaluation work against a stateless CLI invocation. +""" + +from __future__ import annotations + +import asyncio +import contextlib +import json +import logging +import os +import shutil +import signal +import time +from collections.abc import Callable +from datetime import datetime +from typing import Any, ClassVar, Literal, NoReturn + +from coder_eval.agent import Agent +from coder_eval.errors import AgentCrashError, TurnTimeoutError +from coder_eval.isolation.docker_runner import STDOUT_LINE_LIMIT_BYTES +from coder_eval.models import ( + AgentKind, + AgentState, + AssistantMessage, + CommandTelemetry, + ContentBlock, + OpenCodeAgentConfig, + PermissionMode, + ResultSummary, + TokenUsage, + TranscriptMessage, + TurnRecord, +) +from coder_eval.pricing import calculate_cost +from coder_eval.streaming.callbacks import StreamCallback, safe_emit +from coder_eval.streaming.collector import EventCollector +from coder_eval.streaming.events import ( + AgentEndEvent, + AgentEndStatus, + AgentStartEvent, + StreamEvent, + TextChunkEvent, + ToolEndEvent, + ToolEndStatus, + ToolStartEvent, + TurnEndEvent, + TurnEndStatus, + TurnStartEvent, +) + +from .registry import AgentRegistry + + +logger = logging.getLogger(__name__) + +# Grace period between SIGTERM and SIGKILL when tearing down the CLI subprocess. +_TERM_GRACE_SECONDS = 5.0 + +# How long to keep draining stdout/stderr after the CLI process has been reaped. +# `opencode run` leaves a local server child holding the inherited pipes open, so +# EOF never arrives on its own and every post-exit read must be bounded. +_DRAIN_SECONDS = 2.0 + +# Event type strings emitted by `opencode run --format json`. These are the CLI's +# OWN compact vocabulary, captured from a live run — NOT the `session.next.*` +# names in the server's OpenAPI schema, which describe the HTTP/SSE surface of +# `opencode serve` instead. The two are not interchangeable. +_STEP_START = "step_start" +_STEP_FINISH = "step_finish" +_TEXT = "text" +_TOOL_USE = "tool_use" +_ERROR = "error" + +# The full recognized vocabulary. A zero-exit turn that recognized NOTHING from +# this set captured zero telemetry, and is crashed rather than reported as a +# clean empty success — an earlier version of this harness parsed the wrong +# vocabulary and scored SUCCESS 1.0 with zero turns and zero tokens, which is +# indistinguishable from a real pass in every aggregate. See _settle_turn. +_RECOGNIZED_EVENTS = frozenset({_STEP_START, _STEP_FINISH, _TEXT, _TOOL_USE, _ERROR}) + +# How many distinct unrecognized event-type strings to retain for the crash +# message when the vocabulary check fails (diagnosis, not an exhaustive list). +_MAX_UNRECOGNIZED_TYPES = 8 + +# OpenCode's native tool names -> the canonical (Claude) vocabulary that every +# criterion is written against. Mirrors codex_agent's _TOOL_ITEM_NAMES: without +# it a `command_executed` criterion with `tool_name: Bash` matches NOTHING on an +# OpenCode run, and the shell-aware `parameters["command"]` extraction in +# criteria/command_executed.py degrades to raw-JSON matching — so the same task +# scores differently per harness. Unknown tools pass through unchanged. +_TOOL_NAME_MAP: dict[str, str] = { + "bash": "Bash", + "read": "Read", + "write": "Write", + "edit": "Edit", + "patch": "Edit", + "multiedit": "Edit", + "glob": "Glob", + "grep": "Grep", + "list": "LS", + "webfetch": "WebFetch", + "todowrite": "TodoWrite", + "todoread": "TodoRead", + "task": "Agent", +} + +# Config fields the OpenCode CLI has no equivalent knob for. `experiments/default.yaml` +# sets `allowed_tools` on every task, so these are silently dropped by default — +# warn once at start() rather than letting a task believe it constrained the agent. +_UNSUPPORTED_CONFIG_FIELDS: tuple[str, ...] = ( + "system_prompt", + "system_prompt_file", + "allowed_tools", + "disallowed_tools", + "plugins", +) + +# ToolEndStatus -> CommandTelemetry.result_status (the persisted tri-state). +_RESULT_STATUS: dict[ToolEndStatus, Literal["success", "error", "unknown"]] = { + ToolEndStatus.OK: "success", + ToolEndStatus.ERROR: "error", + ToolEndStatus.PERMISSION_DENIED: "error", + ToolEndStatus.UNRESOLVED: "unknown", +} + + +def _unwrap(obj: dict[str, Any]) -> tuple[str, dict[str, Any]]: + """Normalize an OpenCode CLI event to ``(event_type, payload)``. + + Every line is ``{type, timestamp, sessionID, part: {...}}`` with the payload + under ``part`` — except the CLI's own error line, which is flat + (``{type: "error", sessionID, error: {...}}``). Returning the top-level dict + for the flat case is safe: the accessors read named keys, never iterate. + """ + event_type = str(obj.get("type") or "") + part = obj.get("part") + if isinstance(part, dict): + return event_type, part + return event_type, obj + + +def _epoch_ms_to_dt(value: Any) -> datetime | None: + """Convert OpenCode's epoch-millisecond timestamps to naive local datetimes. + + Naive-local matches what the rest of the telemetry uses (``datetime.now()``), + so durations computed against these stay consistent. + """ + if not isinstance(value, int | float): + return None + try: + return datetime.fromtimestamp(value / 1000) + except (OverflowError, OSError, ValueError): + return None + + +class _OpenCodeTurnState: + """Per-``communicate()`` accumulator: events in, finalization payload out. + + Owns everything the terminal ``AgentEndEvent`` must carry (transcript + messages, cumulative usage, text output) plus the open-tool bookkeeping + needed to force-close orphans when a turn dies mid-flight. + """ + + def __init__(self, *, task_id: str, iteration: int, user_input: str, model: str | None) -> None: + self.task_id = task_id + self.iteration = iteration + self.user_input = user_input + self.model = model + + self.started_at = time.monotonic() + self.session_id: str | None = None + self.thread_id: str | None = None + + # Cumulative turn totals (summed across every inner step). + self.usage = TokenUsage() + self.cost_usd: float = 0.0 + self.saw_cost = False + + self.messages: list[TranscriptMessage] = [] + self.text_parts: list[str] = [] + self.step_count = 0 + self.turn_id: str = "" + self.step_started_at: datetime | None = None + self.step_text_parts: list[str] = [] + self.step_tool_ids: list[str] = [] + + # callID -> (telemetry, started_at) for tools awaiting a result. + self.open_tools: dict[str, CommandTelemetry] = {} + self.sequence = 0 + self.stop_reason: str | None = None + self.error_message: str | None = None + self.max_turns_exhausted = False + # Guards the one-terminal-event rule; see finalize(). + self.finalized = False + # Guards _warn_token_shape: one report per turn, not one per step. + self.warned_token_shape = False + # Vocabulary drift detection (see _settle_turn): how many events matched + # _RECOGNIZED_EVENTS, and a bounded sample of the types that did not. + self.recognized_events = 0 + self.unrecognized_types: set[str] = set() + + self._emit: Callable[[StreamEvent], None] = lambda _e: None + + def bind(self, emit: Callable[[StreamEvent], None]) -> None: + self._emit = emit + + def emit(self, event: StreamEvent) -> None: + self._emit(event) + + @property + def agent_output(self) -> str: + return "".join(self.text_parts) + + # --- event handlers ---------------------------------------------------- + + def on_step_start(self, part: dict[str, Any]) -> None: + self.step_count += 1 + self.turn_id = str(part.get("messageID") or f"step_{self.step_count}") + self.step_started_at = datetime.now() + self.step_text_parts = [] + self.step_tool_ids = [] + self.emit( + TurnStartEvent( + task_id=self.task_id, + thread_id=self.thread_id, + turn_id=self.turn_id, + model=self.model, + ) + ) + + def on_text(self, part: dict[str, Any]) -> None: + """``text`` carries a COMPLETE assistant message, not a streaming delta.""" + text = part.get("text") + if not isinstance(text, str) or not text: + return + self.text_parts.append(text) + self.step_text_parts.append(text) + self.emit(TextChunkEvent(task_id=self.task_id, thread_id=self.thread_id, turn_id=self.turn_id, text=text)) + + def on_tool_use(self, part: dict[str, Any]) -> None: + """A ``tool_use`` event carries the tool's whole state under ``state``. + + In practice the CLI emits one already-``completed`` event per call rather + than a call/result pair, so the matching ``ToolStart``/``ToolEnd`` are + both synthesized here. A non-terminal state (``pending``/``running``) is + still handled: the tool is left open and closed by a later event for the + same ``callID``, or force-closed as ``unresolved`` if the turn dies first. + Execution timestamps come from ``state.time``, so ``duration_ms`` reflects + the tool's real runtime rather than our parse instant. + """ + state = part.get("state") + state = state if isinstance(state, dict) else {} + call_id = str(part.get("callID") or f"call_{self.sequence + 1}") + + telemetry = self.open_tools.get(call_id) + if telemetry is None: + self.sequence += 1 + times = state.get("time") if isinstance(state.get("time"), dict) else {} + started = _epoch_ms_to_dt(times.get("start")) + params = state.get("input") + raw_tool = str(part.get("tool") or "unknown") + telemetry = CommandTelemetry( + tool_name=_TOOL_NAME_MAP.get(raw_tool.lower(), raw_tool), + tool_id=call_id, + assistant_turn_index=self.step_count, + timestamp=started or datetime.now(), + execution_started_at=started, + parameters=params if isinstance(params, dict) else {}, + sequence_number=self.sequence, + ) + self.open_tools[call_id] = telemetry + self.step_tool_ids.append(call_id) + self.emit( + ToolStartEvent(task_id=self.task_id, thread_id=self.thread_id, turn_id=self.turn_id, tool=telemetry) + ) + + status_text = str(state.get("status") or "").lower() + output = state.get("output") + error_text = state.get("error") + if status_text in ("pending", "running"): + return # still in flight; a later event (or the orphan sweep) closes it + + if status_text == "error" or error_text: + message = str(error_text or output or "tool failed") + denied = "permission" in message.lower() or "denied" in message.lower() + status = ToolEndStatus.PERMISSION_DENIED if denied else ToolEndStatus.ERROR + else: + message = None + status = ToolEndStatus.OK + + times = state.get("time") if isinstance(state.get("time"), dict) else {} + self._close_tool( + call_id, + status=status, + summary=output if isinstance(output, str) else None, + error=message, + completed_at=_epoch_ms_to_dt(times.get("end")), + ) + + def _close_tool( + self, + call_id: str, + *, + status: ToolEndStatus, + summary: str | None, + error: str | None, + completed_at: datetime | None = None, + ) -> None: + telemetry = self.open_tools.pop(call_id, None) + if telemetry is None: + # A result with no matching call (shouldn't happen, but never drop it). + self.sequence += 1 + telemetry = CommandTelemetry( + tool_name="unknown", + tool_id=call_id, + assistant_turn_index=self.step_count, + timestamp=datetime.now(), + sequence_number=self.sequence, + ) + completed = completed_at or datetime.now() + telemetry.execution_completed_at = completed + if telemetry.execution_started_at is not None: + telemetry.duration_ms = (completed - telemetry.execution_started_at).total_seconds() * 1000 + telemetry.result_status = _RESULT_STATUS[status] + # Stored untruncated by design (sub-agent returns must survive whole). + telemetry.result_summary = summary + telemetry.error_message = error + self.emit( + ToolEndEvent( + task_id=self.task_id, + thread_id=self.thread_id, + turn_id=self.turn_id, + tool=telemetry, + status=status, + ) + ) + + def _rate_card_cost(self) -> float | None: + """Price the captured buckets from the static rate card. + + ``None`` when the model is unpinned or unpriced, matching "nothing could + be priced". See :meth:`_resolve_cost` for how this composes with the + stream's own ``cost`` reporting. + """ + if not self.model or self.usage.is_empty(): + return None + return calculate_cost( + self.model, + uncached_input_tokens=self.usage.uncached_input_tokens, + output_tokens=self.usage.output_tokens, + cache_creation_tokens=self.usage.cache_creation_input_tokens, + cache_read_tokens=self.usage.cache_read_input_tokens, + ) + + def _resolve_cost(self) -> float | None: + """Decide the turn's cost: the stream's own accounting vs the rate card. + + A non-zero cost the CLI reported always wins — it is the provider's own + accounting, and (on OpenRouter) per-request routing makes it strictly + better than a static headline rate. The rate card fills two gaps that + would otherwise book tokens with no money and silently understate the + run-level bill: + + - the stream reported no ``cost`` at all (a provider or auth mode that + omits it, or a turn that died before its first ``step_finish``); + - the stream reported ``cost: 0`` for tokens the rate card prices above + zero. OpenCode reports 0 when its own model registry has no price for + the model, or under subscription-style auth — neither means the tokens + were free. A genuinely free model has an all-zero rate entry (or no + entry), so it still resolves to the stream's 0 here. + """ + rate = self._rate_card_cost() + if not self.saw_cost: + return rate + if self.cost_usd == 0.0 and rate: + logger.warning( + "opencode: the stream reported $0 for a turn the rate card prices at $%.6f " + + "(model unpriced in OpenCode's registry, or subscription auth); using the rate card " + + "so the run total is not understated.", + rate, + ) + return rate + return self.cost_usd + + def _warn_token_shape(self, message: str, *args: Any) -> None: + """Report a token-bucket surprise ONCE per turn (a broken stream repeats it).""" + if self.warned_token_shape: + return + self.warned_token_shape = True + logger.warning("opencode: unexpected token accounting — " + message, *args) + + def _fresh_input_slice( + self, tokens: dict[str, Any], raw_in: int, raw_out: int, reasoning: int, cw: int, cr: int + ) -> int: + """Decide what ``tokens.input`` means on this stream — per step, from evidence. + + coder_eval's ``uncached_input_tokens`` is the fresh slice only (cost bills it + at the input rate and the cache buckets separately), and two conventions for + ``input`` exist in the wild: + + - **flat** — ``input`` already IS the fresh slice and + ``total = input + output + reasoning + cache.read + cache.write``. This is + what a live capture on the current CLI shows (observed 2026-08-13: + ``7966 = 6796 + 128 + 18 + 1024`` exactly). + - **nested** — cached tokens are counted inside ``input`` (the OpenAI + ``prompt_tokens`` convention), so ``total = input + output + reasoning`` + and the fresh slice subtracts the cache buckets. + + The stream's own ``total`` arbitrates per step, so a CLI upgrade that flips + the convention re-classifies itself instead of silently mis-booking a bucket. + With no cache traffic the conventions agree. With no usable ``total`` the + flat (live-verified) reading is taken — but if cache traffic is present that + is an UNVERIFIABLE assumption (the original mapping bug was exactly an + unverified assumption of this kind), so it warns once per turn rather than + defaulting in silence. A ``total`` matching NEITHER warns loudly — the + schema moved, and cost should not be trusted blind. + """ + total = tokens.get("total") + if not isinstance(total, int): + if cr or cw: + self._warn_token_shape( + "tokens.total is missing with cache traffic present (cache.read=%d, cache.write=%d); " + + "assuming the flat convention (`input` is the fresh slice) but the mapping cannot be " + + "verified for this stream — re-check docs/agents/OPENCODE.md before trusting cost", + cr, + cw, + ) + return raw_in + nested = raw_in + raw_out + reasoning + flat = nested + cr + cw + # Check flat first: with zero cache traffic the two sums coincide and the + # conventions agree, so `input` is the fresh slice either way. + if total == flat: + return raw_in + if total == nested: # implies cache traffic, since flat was checked first + fresh = raw_in - cr - cw + if fresh < 0: + # The stream contradicts itself: `total` says the cache buckets nest + # inside `input`, but `input` is too small to contain them. + self._warn_token_shape( + "tokens.total says the cache buckets nest inside input, but input(%d) < " + + "cache.read(%d) + cache.write(%d); keeping `input` as the fresh slice", + raw_in, + cr, + cw, + ) + return raw_in + return fresh + self._warn_token_shape( + "tokens.total(%d) matches neither input+output+reasoning(%d) nor that sum plus the cache " + + "buckets(%d); the bucket mapping may no longer match the CLI — re-check " + + "docs/agents/OPENCODE.md before trusting cost", + total, + nested, + flat, + ) + return raw_in + + def on_step_finish(self, part: dict[str, Any]) -> None: + tokens = part.get("tokens") + tokens = tokens if isinstance(tokens, dict) else {} + cache = tokens.get("cache") if isinstance(tokens.get("cache"), dict) else {} + 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) + + step_in = self._fresh_input_slice(tokens, raw_in, raw_out, step_reasoning, step_cw, step_cr) + # Reasoning tokens are billed at the output rate but reported apart from + # `output`, so fold them in for the turn total; the per-message record + # keeps `reasoning_tokens` separately for visibility. + step_out = raw_out + step_reasoning + + self.usage = TokenUsage( + uncached_input_tokens=self.usage.uncached_input_tokens + step_in, + output_tokens=self.usage.output_tokens + step_out, + cache_creation_input_tokens=self.usage.cache_creation_input_tokens + step_cw, + cache_read_input_tokens=self.usage.cache_read_input_tokens + step_cr, + ) + cost = part.get("cost") + if isinstance(cost, int | float): + self.cost_usd += float(cost) + self.saw_cost = True + + finish = part.get("reason") + if isinstance(finish, str) and finish: + self.stop_reason = finish + + started = self.step_started_at or datetime.now() + completed = datetime.now() + blocks: list[ContentBlock] = [] + step_text = "".join(self.step_text_parts) + if step_text: + blocks.append(ContentBlock(block_type="text", sequence=0, text=step_text)) + for i, tool_id in enumerate(self.step_tool_ids, start=len(blocks)): + blocks.append(ContentBlock(block_type="tool_use", sequence=i, tool_use_id=tool_id)) + + self.messages.append( + AssistantMessage( + started_at=started, + completed_at=completed, + generation_duration_ms=(completed - started).total_seconds() * 1000, + content_blocks=blocks, + tool_use_ids=list(self.step_tool_ids), + input_tokens=step_in, + output_tokens=step_out, + cache_creation_tokens=step_cw, + cache_read_tokens=step_cr, + reasoning_tokens=step_reasoning, + stop_reason=finish if isinstance(finish, str) else None, + model=self.model, + message_id=str(part.get("messageID") or "") or None, + ) + ) + self.emit( + TurnEndEvent( + task_id=self.task_id, + thread_id=self.thread_id, + turn_id=self.turn_id, + status=TurnEndStatus.COMPLETED, + tokens=TokenUsage( + uncached_input_tokens=step_in, + output_tokens=step_out, + cache_creation_input_tokens=step_cw, + cache_read_input_tokens=step_cr, + ), + ) + ) + + def close_open_tools(self) -> None: + """Force-close every tool still awaiting a result (crash/timeout orphans).""" + for call_id in list(self.open_tools): + self._close_tool(call_id, status=ToolEndStatus.UNRESOLVED, summary=None, error="no result observed") + + def finalize( + self, + status: AgentEndStatus, + *, + crashed: bool = False, + crash_reason: str | None = None, + ) -> None: + """Close orphaned tools and emit the terminal ``AgentEndEvent``. + + Idempotent: the protocol allows EXACTLY ONE ``AgentEndEvent`` per + ``communicate()``, and the outer ``except Exception`` guard can fire after + a normal finalize (e.g. a failure while building the record). The first + call wins so a late crash cannot emit a second terminal event into the + caller's ``stream_callback``; it still raises, so the failure is not + swallowed. + """ + if self.finalized: + return + self.finalized = True + self.close_open_tools() + usage = self.usage + cost = self._resolve_cost() + if cost is not None: + usage = usage.model_copy(update={"total_cost_usd": cost}) + self.emit( + AgentEndEvent( + task_id=self.task_id, + thread_id=self.thread_id, + status=status, + usage=usage, + iteration=self.iteration, + user_input=self.user_input, + agent_output=self.agent_output, + model_used=self.model, + assistant_turn_count=self.step_count, + messages=list(self.messages), + num_turns=self.step_count, + max_turns_exhausted=self.max_turns_exhausted, + result_summary=ResultSummary( + is_error=crashed, + subtype=status.value, + stop_reason=self.stop_reason, + result=crash_reason or self.error_message, + ), + crashed=crashed, + crash_reason=crash_reason, + duration_seconds=time.monotonic() - self.started_at, + ) + ) + + +@AgentRegistry.register(AgentKind.OPENCODE, OpenCodeAgentConfig) +class OpenCodeAgent(Agent[OpenCodeAgentConfig]): + """Runs the ``opencode`` CLI as a subprocess, one invocation per turn.""" + + # `should_stop` is polled at every event boundary — i.e. tool-call + # granularity — and honored by terminating the CLI subprocess cleanly. + supports_cooperative_stop: ClassVar[bool] = True + + def __init__( + self, + config: OpenCodeAgentConfig, + task_id: str = "unknown", + **_: Any, + ) -> None: + self.config = config + self.task_id = task_id + self.working_directory: str | None = None + self._env_path_prepend: list[str] = [] + self._plugin_tools_dir: str | None = None + self._session_id: str | None = None + self._process: asyncio.subprocess.Process | None = None + self._state = AgentState.WORKING + + # --- lifecycle --------------------------------------------------------- + + async def start( + self, + working_directory: str, + *, + env_path_prepend: list[str] | None = None, + plugin_tools_dir: str | None = None, + ) -> None: + if shutil.which("opencode") is None: + raise RuntimeError( + "The 'opencode' CLI was not found on PATH." + + " Install it with `npm install -g opencode-ai` (or see https://opencode.ai/docs/)." + ) + ignored = [f for f in _UNSUPPORTED_CONFIG_FIELDS if getattr(self.config, f, None)] + if ignored: + logger.warning( + "opencode: %s set but NOT enforced — the CLI has no equivalent knob, so the run is " + + "unconstrained by them; do not rely on them as a boundary (see docs/agents/OPENCODE.md).", + ", ".join(ignored), + ) + self.working_directory = working_directory + self._env_path_prepend = list(env_path_prepend or []) + self._plugin_tools_dir = plugin_tools_dir + self._session_id = None + self._state = AgentState.WORKING + + async def stop(self) -> None: + await self.kill() + self._mark_stopped() + + async def kill(self) -> None: + proc = self._process + if proc is None or proc.returncode is not None: + return + with contextlib.suppress(ProcessLookupError): + proc.terminate() + with contextlib.suppress(TimeoutError, asyncio.TimeoutError): + await asyncio.wait_for(proc.wait(), timeout=_TERM_GRACE_SECONDS) + if proc.returncode is None: + with contextlib.suppress(ProcessLookupError): + proc.kill() + + def kill_sync(self) -> None: + """SIGKILL the in-flight CLI by PID (called from the watchdog thread).""" + proc = self._process + if proc is None or proc.returncode is not None: + return + with contextlib.suppress(ProcessLookupError, PermissionError): + os.kill(proc.pid, signal.SIGKILL) + + def get_environment_info(self) -> dict[str, Any]: + info: dict[str, Any] = {"opencode_model": self.config.model, "opencode_pure": self.config.pure} + if self.config.variant: + info["opencode_variant"] = self.config.variant + if self._session_id: + info["opencode_session_id"] = self._session_id + return info + + # --- command construction --------------------------------------------- + + def _build_argv(self, user_input: str) -> list[str]: + argv = ["opencode", "run", "--format", "json"] + if self.config.model: + argv += ["-m", self.config.model] + if self.working_directory: + argv += ["--dir", self.working_directory] + if self.config.variant: + argv += ["--variant", self.config.variant] + if self.config.pure: + argv.append("--pure") + # PLAN mode is the one mode that must not auto-approve side effects; every + # other mode runs unattended, where an approval prompt would simply hang. + if self.config.permission_mode is not PermissionMode.PLAN: + argv.append("--auto") + if self._session_id: + argv += ["--session", self._session_id] + argv.append("--") + argv.append(user_input) + return argv + + def _build_env(self) -> dict[str, str]: + env = dict(os.environ) + 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 + return env + + # --- the turn ---------------------------------------------------------- + + async def communicate( + self, + user_input: str, + *, + stream_callback: StreamCallback | None = None, + timeout: float | None = None, + max_turns: int | None = None, + should_stop: Callable[[], bool] | None = None, + ) -> TurnRecord: + if self.working_directory is None: + raise RuntimeError("OpenCodeAgent.start() must be called before communicate()") + + self._begin_turn() + collector = EventCollector() + + def emit(event: StreamEvent) -> None: + collector.on_event(event) + if stream_callback is not None: + safe_emit(stream_callback, event) + + state = _OpenCodeTurnState( + task_id=self.task_id, + iteration=self._iteration, + user_input=user_input, + model=self.config.model, + ) + state.bind(emit) + + emit( + AgentStartEvent( + task_id=self.task_id, + prompt=user_input, + iteration=self._iteration, + model=self.config.model, + ) + ) + + deadline = None if timeout is None else time.monotonic() + timeout + stopped_early = False + stderr_drain: asyncio.Future[bytes] | None = None + try: + proc = await asyncio.create_subprocess_exec( + *self._build_argv(user_input), + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + cwd=self.working_directory, + env=self._build_env(), + # A single nd-JSON event can carry a whole tool result (a large file + # read), which blows past StreamReader's default 64 KiB line cap and + # would raise ValueError mid-stream, killing the read loop. + limit=STDOUT_LINE_LIMIT_BYTES, + ) + self._process = proc + assert proc.stdout is not None + + # Drain stderr CONCURRENTLY, from the moment the CLI starts. Reading it + # only after exit (while stdout drives the loop) deadlocks the pair: a + # child that fills the ~64 KiB stderr pipe blocks on write, stops + # emitting stdout, and never exits — so the turn hangs to its deadline. + # docker_runner dodges this by merging stderr into stdout; here that + # would corrupt the nd-JSON, so it gets its own reader instead. + if proc.stderr is not None: + stderr_drain = asyncio.ensure_future(proc.stderr.read()) + + # `opencode run` spawns a local server child that INHERITS this stdout + # pipe, so the pipe is NOT closed when the CLI itself exits — readline() + # would block until the turn deadline waiting for an EOF that never + # comes. So race each read against process exit: whichever lands first + # wins, and once the process is gone a bounded drain collects whatever + # is still buffered before the loop ends. + exit_waiter = asyncio.ensure_future(proc.wait()) + read_task: asyncio.Future[bytes] | None = None + try: + while True: + remaining = None if deadline is None else deadline - time.monotonic() + if remaining is not None and remaining <= 0: + await self._timeout_turn(state, collector, timeout or 0.0) + + if read_task is None: + read_task = asyncio.ensure_future(proc.stdout.readline()) + done, _pending = await asyncio.wait( + {read_task, exit_waiter}, + timeout=remaining, + return_when=asyncio.FIRST_COMPLETED, + ) + if not done: + await self._timeout_turn(state, collector, timeout or 0.0) + if not read_task.done(): + # The process exited with the read still pending. Give the + # buffered tail a bounded window, then stop rather than + # waiting on the grandchild's open write end. + try: + await asyncio.wait_for(asyncio.shield(read_task), _DRAIN_SECONDS) + except TimeoutError: + break + line = read_task.result() + read_task = None + if not line: + break + + self._handle_line(line, state) + + if max_turns is not None and state.step_count > max_turns: + state.max_turns_exhausted = True + await self.kill() + break + if should_stop is not None and should_stop(): + stopped_early = True + await self.kill() + break + finally: + if read_task is not None: + read_task.cancel() + exit_waiter.cancel() + + status = await self._settle_turn(proc, state, collector, stderr_drain, stopped_early=stopped_early) + state.finalize(status) + # Build BEFORE marking the turn clean: a failure in the reduction is a + # failed turn, and `_end_turn_ok` would clear the rollback flag that + # `discard_pending_turn` needs to un-bump `_iteration`. + record = collector.build_turn_record() + self._end_turn_ok() + return record + + except (AgentCrashError, TurnTimeoutError): + # Already funneled through finalize by _crash_turn / _timeout_turn. + raise + except asyncio.CancelledError: + self._finalize_external_cancel(state.finalize) + self._capture_partial_turn(collector) + raise + except Exception as e: + # Everything the turn loop does NOT anticipate: a spawn failure + # (OSError/PermissionError from create_subprocess_exec), a StreamReader + # ValueError on a line past `limit`, a malformed-payload TypeError in a + # handler, a pydantic error assembling telemetry. Without this the + # exception escapes raw and breaks the pending-turn contract three ways: + # no AgentEndEvent (an unbalanced event tree for every renderer), the + # captured telemetry dropped instead of parked on `pending_turn`, and + # `_iteration` left incremented because the orchestrator never reaches + # `discard_pending_turn`. Same guard, same reasons, as CodexAgent. + self._crash_turn(state, collector, f"OpenCode turn failed: {e!s}", cause=e) + finally: + if stderr_drain is not None: + stderr_drain.cancel() + self._process = None + + async def _settle_turn( + self, + proc: asyncio.subprocess.Process, + state: _OpenCodeTurnState, + collector: EventCollector, + stderr_drain: asyncio.Future[bytes] | None, + *, + stopped_early: bool, + ) -> AgentEndStatus: + """Reap the CLI once the read loop is done and decide the turn's end status. + + Raises ``AgentCrashError`` (via :meth:`_crash_turn`) when the stream carried + a structured error, when the process died with neither a structured error + nor an intentional stop, or when a clean exit recognized no events at all + (a zero-telemetry turn must not score — see the guard below). + """ + await proc.wait() + # Collect what the concurrent reader drained. Bounded for the same reason as + # the read loop: the inherited stderr pipe outlives the CLI, so waiting for + # the reader's own EOF would block. Shielded so the timeout doesn't kill it + # before communicate()'s finally can. + stderr_bytes = b"" + if stderr_drain is not None: + with contextlib.suppress(TimeoutError): + stderr_bytes = await asyncio.wait_for(asyncio.shield(stderr_drain), timeout=_DRAIN_SECONDS) + + if state.error_message is not None: + self._crash_turn(state, collector, f"OpenCode error: {state.error_message}") + + # A non-zero exit with no structured error event still means the turn + # died — surface stderr rather than reporting a silent empty success. + if proc.returncode not in (0, None) and not stopped_early and not state.max_turns_exhausted: + detail = stderr_bytes.decode("utf-8", "replace").strip() or f"exit code {proc.returncode}" + self._crash_turn(state, collector, f"OpenCode exited non-zero: {detail}") + + # A clean exit that recognized NO events captured zero telemetry — zero + # turns, zero tokens, zero cost — while file-based criteria can still + # pass on whatever the agent did, producing a SUCCESS that is silently + # missing from every aggregate. This already happened once (the harness + # parsed the `session.next.*` server vocabulary instead of the CLI's), + # so vocabulary drift is crashed loudly instead of scored. Intentional + # cuts (should_stop / max_turns) are exempt: they can land before the + # first event. + if not stopped_early and not state.max_turns_exhausted and state.recognized_events == 0: + seen = ", ".join(sorted(state.unrecognized_types)) or "none (stdout carried no JSON events)" + self._crash_turn( + state, + collector, + "OpenCode exited cleanly but emitted no recognized events, so the turn captured zero " + + f"telemetry. Unrecognized event types seen: {seen}. The CLI's event vocabulary may have " + + "changed — see docs/agents/OPENCODE.md (Telemetry) before trusting any run from this CLI version.", + ) + + if stopped_early: + return AgentEndStatus.STOPPED_EARLY + if state.max_turns_exhausted: + return AgentEndStatus.MAX_TURNS_EXHAUSTED + return AgentEndStatus.COMPLETED + + def _crash_turn( + self, + state: _OpenCodeTurnState, + collector: EventCollector, + message: str, + *, + cause: BaseException | None = None, + ) -> NoReturn: + """Park the crashed partial record and raise ``AgentCrashError``. + + ``cause`` preserves the explicit ``__cause__`` link when called from + inside an ``except ... as e`` block. + """ + state.close_open_tools() + try: + self._finalize_and_raise_crash(state.finalize, message, cause=cause) + finally: + self._capture_partial_turn(collector) + + async def _timeout_turn( + self, + state: _OpenCodeTurnState, + collector: EventCollector, + timeout: float, + ) -> NoReturn: + """Kill the CLI, park the crashed partial record, raise ``TurnTimeoutError``. + + ``_finalize_and_raise_timeout`` emits the terminal event via + ``state.finalize``; the partial record is captured immediately after so + ``pending_turn`` carries everything observed before the deadline. + """ + await self.kill() + state.close_open_tools() + try: + self._finalize_and_raise_timeout(state.finalize, timeout) + finally: + self._capture_partial_turn(collector) + + def _handle_line(self, line: bytes, state: _OpenCodeTurnState) -> None: + """Parse one nd-JSON line and dispatch it. Never raises on bad input.""" + raw = line.decode("utf-8", "replace").strip() + if not raw: + return + try: + obj = json.loads(raw) + except json.JSONDecodeError: + # OpenCode occasionally interleaves non-JSON notices (e.g. the Bun + # AVX warning) on stdout; a malformed line must not kill the turn. + logger.debug("opencode: skipping non-JSON stdout line: %s", raw[:200]) + return + if not isinstance(obj, dict): + return + + event_type, part = _unwrap(obj) + if event_type in _RECOGNIZED_EVENTS: + state.recognized_events += 1 + elif len(state.unrecognized_types) < _MAX_UNRECOGNIZED_TYPES: + state.unrecognized_types.add(event_type or "") + + # sessionID rides on the envelope, not the part. + session_id = obj.get("sessionID") or part.get("sessionID") + if isinstance(session_id, str) and session_id: + if state.session_id is None: + state.session_id = session_id + state.thread_id = session_id + self._session_id = session_id + + 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") + else: + logger.debug("opencode: unhandled event type %r", event_type) diff --git a/src/coder_eval/models/__init__.py b/src/coder_eval/models/__init__.py index e1fe21ba..4c9fe1b7 100644 --- a/src/coder_eval/models/__init__.py +++ b/src/coder_eval/models/__init__.py @@ -13,6 +13,7 @@ CodexAgentConfig, LocalPluginConfig, NoneAgentConfig, + OpenCodeAgentConfig, ResolvedAgentConfig, parse_agent_config, ) @@ -215,6 +216,7 @@ "CodexAgentConfig", "LocalPluginConfig", "NoneAgentConfig", + "OpenCodeAgentConfig", "ResolvedAgentConfig", "parse_agent_config", # Enums diff --git a/src/coder_eval/models/agent_config.py b/src/coder_eval/models/agent_config.py index b4ad98fd..a705d450 100644 --- a/src/coder_eval/models/agent_config.py +++ b/src/coder_eval/models/agent_config.py @@ -278,6 +278,38 @@ class AntigravityAgentConfig(BaseAgentConfig): ) +class OpenCodeAgentConfig(BaseAgentConfig): + """OpenCode agent configuration (the open-source terminal coding agent). + + Drives the ``opencode`` CLI in non-interactive mode + (``opencode run --format json``), which streams newline-delimited JSON events + on stdout. ``model`` is OpenCode's ``provider/model`` form (e.g. + ``deepseek/deepseek-v4-flash-0731``) and is passed through verbatim via ``-m``. + + Permission handling is derived from the inherited ``permission_mode``: every + mode except :attr:`PermissionMode.PLAN` passes ``--auto`` so an unattended + eval run never blocks on an interactive approval prompt. + """ + + type: Literal[AgentKind.OPENCODE] # type: ignore[assignment] + + variant: str | None = Field( + default=None, + description=( + "Provider-specific reasoning effort passed through as OpenCode's --variant " + "(e.g. 'minimal', 'high', 'max'). None leaves the provider default." + ), + ) + pure: bool = Field( + default=True, + description=( + "Run OpenCode with --pure (no external plugins), isolating the sandbox from " + "host-level OpenCode plugin config. Mirrors the isolation rationale behind " + "the Claude agent's `setting_sources: []`. Set False to load host plugins." + ), + ) + + class NoneAgentConfig(BaseAgentConfig): """No-op ("agentless") agent configuration. @@ -302,7 +334,7 @@ class NoneAgentConfig(BaseAgentConfig): # Only includes the concrete subclasses (not BaseAgentConfig) since the discriminator # must be a Literal type. BaseAgentConfig is returned by parse_agent_config when type=None. type AgentConfig = Annotated[ - ClaudeCodeAgentConfig | CodexAgentConfig | AntigravityAgentConfig | NoneAgentConfig, + ClaudeCodeAgentConfig | CodexAgentConfig | AntigravityAgentConfig | OpenCodeAgentConfig | NoneAgentConfig, Field(discriminator="type"), ] diff --git a/src/coder_eval/models/enums.py b/src/coder_eval/models/enums.py index 03afe885..0cba3650 100644 --- a/src/coder_eval/models/enums.py +++ b/src/coder_eval/models/enums.py @@ -108,6 +108,7 @@ class AgentKind(StrEnum): CLAUDE_CODE = "claude-code" CODEX = "codex" ANTIGRAVITY = "antigravity" + OPENCODE = "opencode" NONE = "none" # Agentless / system task — no coding agent runs; success criteria do all the work. UNKNOWN = "unknown" # Used when agent type cannot be determined (e.g., task loading failure) diff --git a/src/coder_eval/pricing.py b/src/coder_eval/pricing.py index c8cf478c..0c74a3e8 100644 --- a/src/coder_eval/pricing.py +++ b/src/coder_eval/pricing.py @@ -117,6 +117,7 @@ class ModelPricing: "moonshotai/kimi-k3": ModelPricing(3.0, 15.0, 3.0, 0.30), "z-ai/glm-5.2": ModelPricing(0.7168, 2.2528, 0.7168, 0.13312), "deepseek/deepseek-v4-pro": ModelPricing(0.435, 0.87, 0.435, 0.003625), + "deepseek/deepseek-v4-flash-0731": ModelPricing(0.14, 0.28, 0.14, 0.0028), } @@ -174,8 +175,13 @@ def _normalize_model(model: str) -> str: """ model = model.strip() # LiteLLM/Bedrock routing prefixes (e.g. "converse/zai.glm-5", - # "bedrock/converse/deepseek.v3.2") → bare model id. - for routing_prefix in ("bedrock/converse/", "bedrock/", "converse/"): + # "bedrock/converse/deepseek.v3.2") → bare model id. ``openrouter/`` is here + # because agents that address OpenRouter natively (OpenCode) report the model + # WITH its provider prefix ("openrouter/deepseek/deepseek-v4-flash-0731"), + # while the OpenRouter rate-card keys are the bare vendor/model ids that the + # LiteLLM route already uses — without this strip the same model prices under + # LiteLLM and silently goes unpriced under OpenCode. + for routing_prefix in ("bedrock/converse/", "bedrock/", "converse/", "openrouter/"): if model.startswith(routing_prefix): model = model[len(routing_prefix) :] break diff --git a/tasks/opencode_smoke_test.yaml b/tasks/opencode_smoke_test.yaml new file mode 100644 index 00000000..bead022b --- /dev/null +++ b/tasks/opencode_smoke_test.yaml @@ -0,0 +1,34 @@ +task_id: "opencode_smoke_test" +description: "Smoke-test the OpenCode agent harness: create and run a small Python script." +initial_prompt: "Create a Python file named app.py in the current working directory that prints 'Hello, OpenCode!' on one line, and today's date in YYYY-MM-DD format on the next line. Use the datetime module. Then run the script with: python app.py" +tags: [smoke, smoke-pass, basic, pure-python, opencode] + +run_limits: + expected_turns: 5 + # The CLI drives a full agent loop per invocation; give it room but keep the + # smoke bounded so a hung provider fails fast instead of stalling a suite. + task_timeout: 600 + +agent: + type: "opencode" + # OpenCode addresses models as provider/model and speaks OpenRouter natively + # (authenticated by OPENROUTER_API_KEY), so it does NOT need the LiteLLM proxy — + # that shim exists to translate Anthropic <-> OpenAI for the Claude Code SDK. + # Real per-call cost still lands on the turn: OpenCode reports `cost` on every + # step_finish event and the harness folds it into token_usage.total_cost_usd + # (falling back to the rate card when the stream omits or zeroes it). + model: "openrouter/deepseek/deepseek-v4-flash-0731" + permission_mode: "acceptEdits" + +success_criteria: + - type: "file_exists" + path: "app.py" + description: "The file app.py must be created." + - type: "file_contains" + path: "app.py" + includes: ["Hello, OpenCode!", "datetime"] + description: "The script must contain the required string and import." + - type: "run_command" + command: "python app.py" + timeout: 10 + description: "The script must execute successfully." diff --git a/tests/test_opencode_agent.py b/tests/test_opencode_agent.py new file mode 100644 index 00000000..c931a7f7 --- /dev/null +++ b/tests/test_opencode_agent.py @@ -0,0 +1,796 @@ +"""Tests for the OpenCode agent harness. + +The CLI is never invoked: ``asyncio.create_subprocess_exec`` is patched with a +fake process that replays a newline-delimited JSON event stream, so the whole +reduction path (nd-JSON -> standardized events -> ``TurnRecord``) is exercised +offline and without credentials. + +The fixtures below mirror event lines CAPTURED FROM A LIVE ``opencode run +--format json`` — the CLI's own compact vocabulary (``step_start`` / +``step_finish`` / ``text`` / ``tool_use``, payload under ``part``). Do NOT +"correct" them toward the ``session.next.*`` names in the server's OpenAPI +schema: those describe `opencode serve`'s SSE surface, and an earlier version of +this harness parsed them and silently captured zero telemetry on a real run. +""" + +from __future__ import annotations + +import asyncio +import json +from typing import Any + +import pytest + +from coder_eval.agents.opencode_agent import OpenCodeAgent, _unwrap +from coder_eval.errors import AgentCrashError +from coder_eval.models import AssistantMessage, OpenCodeAgentConfig, PermissionMode +from coder_eval.pricing import calculate_cost +from coder_eval.streaming.events import AgentEndEvent, AgentEndStatus, AgentStartEvent + + +SESSION = "ses_test123" + + +def _evt(event_type: str, part: dict[str, Any]) -> str: + """One CLI event line: payload under ``part``, sessionID on the envelope.""" + return json.dumps( + {"type": event_type, "timestamp": 1786663016802, "sessionID": SESSION, "part": {"sessionID": SESSION, **part}} + ) + + +def _tokens(inp: int, out: int, *, write: int = 0, read: int = 0, reasoning: int = 0) -> dict[str, Any]: + """Token payload in the NESTED convention (total = input+output+reasoning, cache + counted inside `input`); see TestTokenShapeIsObservable for the flat one.""" + return { + "total": inp + out + reasoning, + "input": inp, + "output": out, + "reasoning": reasoning, + "cache": {"write": write, "read": read}, + } + + +HAPPY_STREAM = [ + _evt("step_start", {"id": "prt_1", "messageID": "msg_1", "type": "step-start"}), + _evt( + "tool_use", + { + "id": "prt_2", + "messageID": "msg_1", + "type": "tool", + "tool": "read", + "callID": "call_1", + "state": { + "status": "completed", + "input": {"filePath": "main.py"}, + "output": "print('hi')", + "time": {"start": 1786663018214, "end": 1786663018231}, + }, + }, + ), + _evt( + "step_finish", + { + "id": "prt_3", + "messageID": "msg_1", + "reason": "tool-calls", + "cost": 0.001, + "tokens": _tokens(100, 20, write=5, read=10), + }, + ), + _evt("step_start", {"id": "prt_4", "messageID": "msg_2", "type": "step-start"}), + _evt("text", {"id": "prt_5", "messageID": "msg_2", "type": "text", "text": "Created the file."}), + _evt( + "step_finish", + { + "id": "prt_6", + "messageID": "msg_2", + "reason": "stop", + "cost": 0.002, + "tokens": _tokens(50, 30, read=40, reasoning=7), + }, + ), +] + + +class _FakeProcess: + def __init__(self, lines: list[str], returncode: int = 0, stderr: bytes = b"") -> None: + self._lines = [f"{line}\n".encode() for line in lines] + self.returncode: int | None = None + self._final_returncode = returncode + self._stderr = stderr + self.pid = 4242 + self.terminated = False + self.stdout = self + + async def readline(self) -> bytes: + if self._lines: + return self._lines.pop(0) + self.returncode = self._final_returncode + return b"" + + async def read(self) -> bytes: + return self._stderr + + async def wait(self) -> int: + self.returncode = self._final_returncode + return self.returncode + + def terminate(self) -> None: + self.terminated = True + self.returncode = self._final_returncode + + def kill(self) -> None: + self.returncode = self._final_returncode + + +class _RunningProcess(_FakeProcess): + """A process that stays alive until it is explicitly terminated or killed. + + Needed for teardown assertions: the plain fake reports an exit code as soon + as ``wait()`` is awaited, so ``kill()`` would (correctly) skip ``terminate()`` + on an already-dead process and the test would prove nothing. + """ + + def __init__(self, lines: list[str], **kwargs: Any) -> None: + super().__init__(lines, **kwargs) + self._exited = asyncio.Event() + + async def wait(self) -> int: + await self._exited.wait() + self.returncode = self._final_returncode + return self.returncode + + def terminate(self) -> None: + self.terminated = True + self._exited.set() + + def kill(self) -> None: + self._exited.set() + + +@pytest.fixture +def patch_exec(monkeypatch: pytest.MonkeyPatch): + """Patch subprocess spawn; return a dict capturing the argv used.""" + captured: dict[str, Any] = {} + + def _install(proc: _FakeProcess) -> dict[str, Any]: + async def fake_exec(*argv: str, **kwargs: Any) -> _FakeProcess: + captured["argv"] = list(argv) + captured["kwargs"] = kwargs + proc.stderr = proc # type: ignore[assignment] + return proc + + monkeypatch.setattr(asyncio, "create_subprocess_exec", fake_exec) + monkeypatch.setattr("shutil.which", lambda _name: "/usr/local/bin/opencode") + return captured + + return _install + + +async def _run(agent: OpenCodeAgent, tmp_path: Any, prompt: str = "do the thing", **kwargs: Any): + await agent.start(str(tmp_path)) + return await agent.communicate(prompt, **kwargs) + + +def _agent(**overrides: Any) -> OpenCodeAgent: + config = OpenCodeAgentConfig(type="opencode", **{"model": "deepseek/deepseek-v4-flash-0731", **overrides}) + return OpenCodeAgent(config, task_id="t1") + + +class TestEnvelopeNormalization: + def test_part_envelope(self): + """Normal events carry their payload under `part`.""" + t, part = _unwrap({"type": "step_finish", "sessionID": "s", "part": {"reason": "stop"}}) + assert t == "step_finish" + assert part["reason"] == "stop" + + def test_flat_envelope(self): + """The CLI's own error path emits a flat object with no `part`.""" + t, props = _unwrap({"type": "error", "sessionID": "s", "error": {"name": "UnknownError"}}) + assert t == "error" + assert props["error"]["name"] == "UnknownError" + + +class TestHappyPath: + async def test_builds_turn_record(self, patch_exec, tmp_path): + patch_exec(_FakeProcess(HAPPY_STREAM)) + record = await _run(_agent(), tmp_path) + + assert record.crashed is False + assert record.agent_output == "Created the file." + assert record.assistant_turn_count == 2 + assert record.model_used == "deepseek/deepseek-v4-flash-0731" + + async def test_token_buckets_accumulate_across_steps(self, patch_exec, tmp_path): + patch_exec(_FakeProcess(HAPPY_STREAM)) + record = await _run(_agent(), tmp_path) + + usage = record.token_usage + assert usage is not None + # The fixture encodes the nested convention (`input` includes the cache + # buckets), so the fresh slice subtracts them: + # step1 100-10-5=85, step2 50-40=10 -> 95 + assert usage.uncached_input_tokens == 95 + # reasoning bills at the output rate: step1 20+0=20, step2 30+7=37 -> 57 + assert usage.output_tokens == 57 + assert usage.cache_creation_input_tokens == 5 + assert usage.cache_read_input_tokens == 50 # 10 + 40 + assert usage.total_cost_usd == pytest.approx(0.003) + + async def test_reconciliation_invariant(self, patch_exec, tmp_path): + """Summing the four buckets across messages must equal token_usage exactly.""" + patch_exec(_FakeProcess(HAPPY_STREAM)) + record = await _run(_agent(), tmp_path) + + usage = record.token_usage + assert usage is not None + assert sum(m.input_tokens for m in record.messages) == usage.uncached_input_tokens + assert sum(m.output_tokens for m in record.messages) == usage.output_tokens + assert sum(m.cache_creation_tokens for m in record.messages) == usage.cache_creation_input_tokens + assert sum(m.cache_read_tokens for m in record.messages) == usage.cache_read_input_tokens + + async def test_tool_call_captured(self, patch_exec, tmp_path): + patch_exec(_FakeProcess(HAPPY_STREAM)) + record = await _run(_agent(), tmp_path) + + assert len(record.commands) == 1 + cmd = record.commands[0] + # Normalized to the canonical vocabulary criteria are written against. + assert cmd.tool_name == "Read" + assert cmd.tool_id == "call_1" + assert cmd.result_status == "success" + assert cmd.parameters == {"filePath": "main.py"} + assert cmd.result_summary == "print('hi')" + # Duration comes from state.time, not our parse instant (17ms in fixture). + assert cmd.duration_ms == pytest.approx(17, abs=1) + + async def test_messages_attributed_to_steps(self, patch_exec, tmp_path): + patch_exec(_FakeProcess(HAPPY_STREAM)) + record = await _run(_agent(), tmp_path) + + assistants = [m for m in record.messages if isinstance(m, AssistantMessage)] + assert len(assistants) == 2 + assert assistants[0].tool_use_ids == ["call_1"] + assert assistants[0].stop_reason == "tool-calls" + assert assistants[1].tool_use_ids == [] + + +class TestTokenShapeIsObservable: + """Two conventions for `tokens.input` exist in the wild — flat (`input` IS the + fresh slice; `total` adds the cache buckets on top) and nested (cached tokens + counted inside `input`, the OpenAI convention). The stream's own `total` + arbitrates per step; a `total` matching neither must be loud, because a silent + mis-mapping under- or over-books a bucket on every cached run. + """ + + @staticmethod + def _step(tokens: dict[str, Any]) -> str: + return _evt("step_finish", {"id": "p", "messageID": "m", "reason": "stop", "tokens": tokens}) + + async def test_flat_convention_keeps_input_verbatim(self, patch_exec, tmp_path, caplog): + """The exact numbers of a live capture (2026-08-13): 7966 = 6796+128+18+1024, + so `input` excludes the cache buckets and must NOT have them subtracted.""" + step = self._step( + {"total": 7966, "input": 6796, "output": 128, "reasoning": 18, "cache": {"read": 1024, "write": 0}} + ) + patch_exec(_FakeProcess([step])) + with caplog.at_level("WARNING"): + record = await _run(_agent(), tmp_path) + + usage = record.token_usage + assert usage is not None + assert usage.uncached_input_tokens == 6796 + assert usage.cache_read_input_tokens == 1024 + assert usage.output_tokens == 146 # 128 + 18 reasoning + assert "unexpected token accounting" not in caplog.text + + async def test_nested_convention_subtracts_the_cache_buckets(self, patch_exec, tmp_path, caplog): + """total = input+output+reasoning ⇒ cached tokens nest inside `input`; the + fresh slice must come back out or the cached portion is billed twice.""" + patch_exec(_FakeProcess(HAPPY_STREAM)) # _tokens() builds nested totals + with caplog.at_level("WARNING"): + record = await _run(_agent(), tmp_path) + + usage = record.token_usage + assert usage is not None + assert usage.uncached_input_tokens == 95 # (100-10-5) + (50-40) + assert "unexpected token accounting" not in caplog.text + + async def test_total_matching_neither_convention_warns(self, patch_exec, tmp_path, caplog): + """nested=350, flat=8030, reported 8000 — the schema moved; keep `input`.""" + patch_exec(_FakeProcess([self._step({"total": 8000, "input": 300, "output": 50, "cache": {"read": 7680}})])) + with caplog.at_level("WARNING"): + record = await _run(_agent(), tmp_path) + + usage = record.token_usage + assert usage is not None + assert usage.uncached_input_tokens == 300 + assert usage.cache_read_input_tokens == 7680 + assert "matches neither" in caplog.text + + async def test_total_disagreeing_with_zero_cache_buckets_warns(self, patch_exec, tmp_path, caplog): + """With no cache traffic the conventions coincide; a mismatch is still drift.""" + patch_exec(_FakeProcess([self._step({"total": 999, "input": 100, "output": 20, "reasoning": 5})])) + with caplog.at_level("WARNING"): + await _run(_agent(), tmp_path) + assert "tokens.total" in caplog.text + + async def test_missing_total_with_cache_traffic_defaults_flat_but_warns(self, patch_exec, tmp_path, caplog): + """No arbiter + cache traffic ⇒ the flat reading is an UNVERIFIABLE assumption + (the original mapping bug was exactly such an assumption), so it must not be + silent — but `input` is still taken verbatim, the live-verified convention.""" + patch_exec(_FakeProcess([self._step({"input": 500, "output": 20, "cache": {"read": 200}})])) + with caplog.at_level("WARNING"): + record = await _run(_agent(), tmp_path) + + usage = record.token_usage + assert usage is not None + assert usage.uncached_input_tokens == 500 + assert usage.cache_read_input_tokens == 200 + assert "tokens.total is missing" in caplog.text + + async def test_missing_total_without_cache_traffic_is_silent(self, patch_exec, tmp_path, caplog): + """No arbiter but no cache either ⇒ the conventions agree; nothing to verify.""" + patch_exec(_FakeProcess([self._step({"input": 500, "output": 20})])) + with caplog.at_level("WARNING"): + record = await _run(_agent(), tmp_path) + + usage = record.token_usage + assert usage is not None + assert usage.uncached_input_tokens == 500 + assert "unexpected token accounting" not in caplog.text + + async def test_nested_total_contradicted_by_small_input_warns(self, patch_exec, tmp_path, caplog): + """`total` says nested but input < cache: self-contradictory; keep `input`.""" + patch_exec(_FakeProcess([self._step({"total": 350, "input": 300, "output": 50, "cache": {"read": 7680}})])) + with caplog.at_level("WARNING"): + record = await _run(_agent(), tmp_path) + + usage = record.token_usage + assert usage is not None + assert usage.uncached_input_tokens == 300 + assert "nest inside input" in caplog.text + + +class TestCostFallsBackToTheRateCard: + async def test_stream_cost_wins_when_reported(self, patch_exec, tmp_path): + """The provider's own accounting beats a static headline rate.""" + patch_exec(_FakeProcess(HAPPY_STREAM)) + record = await _run(_agent(), tmp_path) + assert record.token_usage is not None + assert record.token_usage.total_cost_usd == pytest.approx(0.003) # 0.001 + 0.002 + + async def test_missing_cost_is_priced_from_the_rate_card(self, patch_exec, tmp_path): + """Without this the turn books tokens with no money and the run total understates.""" + stream = [ + _evt("step_start", {"id": "prt_1", "messageID": "msg_1"}), + # No `cost` key — the provider/auth mode did not report one. + _evt("step_finish", {"id": "prt_2", "messageID": "msg_1", "reason": "stop", "tokens": _tokens(1000, 500)}), + ] + patch_exec(_FakeProcess(stream)) + record = await _run(_agent(), tmp_path) + + assert record.token_usage is not None + expected = calculate_cost("deepseek/deepseek-v4-flash-0731", uncached_input_tokens=1000, output_tokens=500) + assert expected is not None and expected > 0 + assert record.token_usage.total_cost_usd == pytest.approx(expected) + + async def test_unpriced_model_reports_no_cost(self, patch_exec, tmp_path): + """`None` (not 0.0) so "unpriceable" stays distinct from "ran for free".""" + patch_exec(_FakeProcess([_evt("step_finish", {"id": "p", "reason": "stop", "tokens": _tokens(10, 5)})])) + record = await _run(_agent(model="nowhere/not-a-real-model"), tmp_path) + assert record.token_usage is not None + assert record.token_usage.total_cost_usd is None + + async def test_zero_reported_cost_on_a_priced_model_uses_the_rate_card(self, patch_exec, tmp_path, caplog): + """OpenCode reports `cost: 0` when its own registry lacks a price for the + model, or under subscription-style auth — neither means the tokens were + free. Latching on the reported 0 would book real tokens with no money.""" + stream = [ + _evt("step_start", {"id": "prt_1", "messageID": "msg_1"}), + _evt( + "step_finish", + {"id": "prt_2", "messageID": "msg_1", "reason": "stop", "cost": 0, "tokens": _tokens(1000, 500)}, + ), + ] + patch_exec(_FakeProcess(stream)) + with caplog.at_level("WARNING"): + record = await _run(_agent(), tmp_path) + + expected = calculate_cost("deepseek/deepseek-v4-flash-0731", uncached_input_tokens=1000, output_tokens=500) + assert expected is not None and expected > 0 + assert record.token_usage is not None + assert record.token_usage.total_cost_usd == pytest.approx(expected) + assert "not understated" in caplog.text + + async def test_zero_reported_cost_on_an_unpriced_model_stays_zero(self, patch_exec, tmp_path): + """With no rate to fall back to, the stream's 0 is the best information we have.""" + stream = [ + _evt("step_start", {"id": "prt_1", "messageID": "msg_1"}), + _evt( + "step_finish", + {"id": "prt_2", "messageID": "msg_1", "reason": "stop", "cost": 0, "tokens": _tokens(10, 5)}, + ), + ] + patch_exec(_FakeProcess(stream)) + record = await _run(_agent(model="nowhere/not-a-real-model"), tmp_path) + assert record.token_usage is not None + assert record.token_usage.total_cost_usd == 0.0 + + +class TestCrossHarnessNormalization: + """A criterion written once must score identically on every harness.""" + + @staticmethod + def _tool_event(tool: str) -> str: + return _evt( + "tool_use", + { + "id": "prt_2", + "messageID": "msg_1", + "type": "tool", + "tool": tool, + "callID": f"call_{tool}", + "state": {"status": "completed", "input": {"command": "pytest -q"}, "output": "ok"}, + }, + ) + + async def test_native_names_map_to_canonical(self, patch_exec, tmp_path): + """`command_executed` filters on `tool_name == "Bash"` and pulls + `parameters["command"]` only for that name — OpenCode's `bash` would match + nothing and fall back to raw-JSON matching.""" + patch_exec(_FakeProcess([self._tool_event("bash"), self._tool_event("write")])) + record = await _run(_agent(), tmp_path) + assert [c.tool_name for c in record.commands] == ["Bash", "Write"] + + async def test_unknown_tool_passes_through(self, patch_exec, tmp_path): + """An unmapped tool still surfaces under its own name rather than vanishing.""" + patch_exec(_FakeProcess([self._tool_event("some_new_tool")])) + record = await _run(_agent(), tmp_path) + assert [c.tool_name for c in record.commands] == ["some_new_tool"] + + +class TestUnsupportedConfigIsAnnounced: + async def test_start_warns_about_unenforced_fields(self, patch_exec, tmp_path, caplog): + """`experiments/default.yaml` sets allowed_tools on every task; the CLI has no + equivalent knob, so silence would let a task believe it was constrained.""" + patch_exec(_FakeProcess(HAPPY_STREAM)) + with caplog.at_level("WARNING"): + await _agent(allowed_tools=["Bash"], system_prompt="be terse").start(str(tmp_path)) + assert "allowed_tools" in caplog.text + assert "system_prompt" in caplog.text + + async def test_no_warning_when_nothing_is_dropped(self, patch_exec, tmp_path, caplog): + patch_exec(_FakeProcess(HAPPY_STREAM)) + with caplog.at_level("WARNING"): + await _agent().start(str(tmp_path)) + assert "NOT enforced" not in caplog.text + + +class TestArgvConstruction: + async def test_defaults_include_auto_and_pure(self, patch_exec, tmp_path): + captured = patch_exec(_FakeProcess(HAPPY_STREAM)) + await _run(_agent(), tmp_path) + + argv = captured["argv"] + assert argv[:4] == ["opencode", "run", "--format", "json"] + assert "--auto" in argv + assert "--pure" in argv + assert argv[argv.index("-m") + 1] == "deepseek/deepseek-v4-flash-0731" + assert argv[-1] == "do the thing" + + async def test_plan_mode_withholds_auto(self, patch_exec, tmp_path): + captured = patch_exec(_FakeProcess(HAPPY_STREAM)) + await _run(_agent(permission_mode=PermissionMode.PLAN), tmp_path) + assert "--auto" not in captured["argv"] + + async def test_variant_and_pure_off(self, patch_exec, tmp_path): + captured = patch_exec(_FakeProcess(HAPPY_STREAM)) + await _run(_agent(variant="high", pure=False), tmp_path) + + argv = captured["argv"] + assert argv[argv.index("--variant") + 1] == "high" + assert "--pure" not in argv + + async def test_explicit_line_limit_is_passed(self, patch_exec, tmp_path): + """A large tool result must not blow StreamReader's default 64 KiB cap.""" + captured = patch_exec(_FakeProcess(HAPPY_STREAM)) + await _run(_agent(), tmp_path) + assert captured["kwargs"]["limit"] > 64 * 1024 + + +class TestSessionContinuity: + async def test_second_turn_resumes_session(self, patch_exec, tmp_path): + patch_exec(_FakeProcess(HAPPY_STREAM)) + agent = _agent() + await _run(agent, tmp_path) + assert agent._session_id == SESSION + + captured2 = patch_exec(_FakeProcess(HAPPY_STREAM)) + await agent.communicate("follow up") + argv = captured2["argv"] + assert argv[argv.index("--session") + 1] == SESSION + + +class TestFailurePaths: + async def test_error_event_raises_and_parks_partial(self, patch_exec, tmp_path): + stream = [ + _evt("step_start", {"id": "prt_1", "messageID": "msg_1", "type": "step-start"}), + _evt( + "tool_use", + { + "id": "prt_2", + "messageID": "msg_1", + "type": "tool", + "tool": "bash", + "callID": "call_1", + "state": {"status": "running", "input": {"command": "ls"}}, + }, + ), + json.dumps( + { + "type": "error", + "sessionID": SESSION, + "error": {"name": "UnknownError", "data": {"message": "provider exploded"}}, + } + ), + ] + patch_exec(_FakeProcess(stream)) + agent = _agent() + + with pytest.raises(AgentCrashError, match="provider exploded"): + await _run(agent, tmp_path) + + partial = agent.pending_turn + assert partial is not None + assert partial.crashed is True + # The in-flight tool was force-closed rather than dropped. + assert [c.result_status for c in partial.commands] == ["unknown"] + + async def test_nonzero_exit_without_error_event_crashes(self, patch_exec, tmp_path): + patch_exec(_FakeProcess([], returncode=1, stderr=b"boom: bad model")) + with pytest.raises(AgentCrashError, match="boom: bad model"): + await _run(_agent(), tmp_path) + + async def test_malformed_line_is_skipped(self, patch_exec, tmp_path): + """Non-JSON noise on stdout must not kill the turn.""" + stream = ["warn: CPU lacks AVX support", *HAPPY_STREAM] + patch_exec(_FakeProcess(stream)) + record = await _run(_agent(), tmp_path) + assert record.crashed is False + assert record.assistant_turn_count == 2 + + async def test_missing_cli_is_actionable(self, monkeypatch, tmp_path): + monkeypatch.setattr("shutil.which", lambda _name: None) + with pytest.raises(RuntimeError, match="npm install -g opencode-ai"): + await _agent().start(str(tmp_path)) + + +class TestZeroTelemetryIsLoud: + """A clean exit that recognized no events must crash, not score. + + An earlier version of this harness parsed the `session.next.*` server + vocabulary instead of the CLI's and reported SUCCESS 1.0 with zero turns, + zero tokens and zero cost — indistinguishable from a real pass in every + aggregate. Vocabulary drift must be an ERROR, not a quiet empty success. + """ + + async def test_unrecognized_vocabulary_crashes_and_names_the_types(self, patch_exec, tmp_path): + stream = [ + json.dumps( + { + "id": "evt_1", + "type": "session.next.step.ended", + "properties": {"sessionID": SESSION, "tokens": {"input": 100, "output": 20}}, + } + ), + json.dumps({"id": "evt_2", "type": "session.next.idle", "properties": {"sessionID": SESSION}}), + ] + patch_exec(_FakeProcess(stream)) + agent = _agent() + + with pytest.raises(AgentCrashError, match="no recognized events") as exc: + await _run(agent, tmp_path) + # The crash names what it DID see, for diagnosis. + assert "session.next.step.ended" in str(exc.value) + + partial = agent.pending_turn + assert partial is not None + assert partial.crashed is True + + async def test_empty_stdout_with_clean_exit_crashes(self, patch_exec, tmp_path): + """Zero events at all is the same zero-telemetry hole as wrong vocabulary.""" + patch_exec(_FakeProcess([], returncode=0)) + with pytest.raises(AgentCrashError, match="no recognized events"): + await _run(_agent(), tmp_path) + + async def test_intentional_cuts_are_exempt(self, patch_exec, tmp_path): + """A cooperative stop can land before the first recognized event; that is + an intentional cut, not vocabulary drift.""" + stream = [json.dumps({"id": "evt_1", "type": "session.next.idle", "properties": {"sessionID": SESSION}})] + proc = _RunningProcess(stream) + patch_exec(proc) + record = await _run(_agent(), tmp_path, should_stop=lambda: True) + assert record.crashed is False + + +class _ExplodingProcess(_FakeProcess): + """Replays events, then raises from ``readline`` mid-stream. + + Stands in for everything the turn loop does not anticipate — most concretely + ``StreamReader.readline`` raising ``ValueError`` on a line past ``limit``. + """ + + async def readline(self) -> bytes: + if self._lines: + return self._lines.pop(0) + raise ValueError("Separator is not found, and chunk exceed the limit") + + +class TestUnexpectedErrorContract: + """An unanticipated exception must still honor the pending-turn contract. + + Escaping raw would break it three ways: no terminal ``AgentEndEvent`` (an + unbalanced event tree for every renderer), captured telemetry dropped instead + of parked on ``pending_turn``, and ``_iteration`` left incremented because the + orchestrator never reaches ``discard_pending_turn``. + """ + + class _Recorder: + """Minimal ``StreamCallback``: records every event the agent emits.""" + + def __init__(self) -> None: + self.events: list[Any] = [] + + def on_event(self, event: Any) -> None: + self.events.append(event) + + async def test_stream_error_becomes_a_crash_with_partial_parked(self, patch_exec, tmp_path): + stream = [ + _evt("step_start", {"id": "prt_1", "messageID": "msg_1", "type": "step-start"}), + _evt( + "tool_use", + { + "id": "prt_2", + "messageID": "msg_1", + "type": "tool", + "tool": "bash", + "callID": "call_1", + "state": {"status": "running", "input": {"command": "ls"}}, + }, + ), + ] + patch_exec(_ExplodingProcess(stream)) + agent = _agent() + + with pytest.raises(AgentCrashError, match="OpenCode turn failed"): + await _run(agent, tmp_path) + + partial = agent.pending_turn + assert partial is not None + assert partial.crashed is True + # Telemetry captured before the failure survives, orphan tool force-closed. + assert [c.result_status for c in partial.commands] == ["unknown"] + + async def test_spawn_failure_becomes_a_crash(self, monkeypatch, tmp_path): + """A failure before the first byte (OSError from the spawn) is still a crash.""" + + async def boom(*_argv: str, **_kwargs: Any): + raise OSError("no fork for you") + + monkeypatch.setattr(asyncio, "create_subprocess_exec", boom) + monkeypatch.setattr("shutil.which", lambda _name: "/usr/local/bin/opencode") + + with pytest.raises(AgentCrashError, match="no fork for you"): + await _run(_agent(), tmp_path) + + async def test_terminal_event_is_emitted_exactly_once(self, patch_exec, tmp_path): + """The protocol allows exactly one AgentEnd per communicate(), crash included.""" + patch_exec(_ExplodingProcess([_evt("step_start", {"id": "prt_1", "messageID": "msg_1"})])) + recorder = self._Recorder() + + with pytest.raises(AgentCrashError): + await _run(_agent(), tmp_path, stream_callback=recorder) + + seen = recorder.events + assert len([e for e in seen if isinstance(e, AgentStartEvent)]) == 1 + ends = [e for e in seen if isinstance(e, AgentEndEvent)] + assert len(ends) == 1 + assert ends[0].crashed is True + assert ends[0].status is AgentEndStatus.CRASHED + + async def test_iteration_rolls_back_after_the_crash(self, patch_exec, tmp_path): + """`discard_pending_turn` must find the bump it needs to undo.""" + patch_exec(_ExplodingProcess([])) + agent = _agent() + + with pytest.raises(AgentCrashError): + await _run(agent, tmp_path) + assert agent._iteration == 1 + await agent.discard_pending_turn() + assert agent._iteration == 0 + assert agent.pending_turn is None + + +class _LeakyPipeProcess(_FakeProcess): + """Replays events, then never signals EOF — the real CLI's behavior. + + ``opencode run`` leaves a local server child holding the inherited stdout + pipe open, so after the CLI exits ``readline()`` blocks forever instead of + returning b"". The agent must fall back to a bounded drain rather than hang + until the turn deadline. + """ + + async def readline(self) -> bytes: + if self._lines: + return self._lines.pop(0) + self.returncode = self._final_returncode # process reaped... + await asyncio.sleep(3600) # ...but the pipe stays open + return b"" + + async def read(self) -> bytes: + await asyncio.sleep(3600) + return b"" + + +class TestLeakedPipeDrain: + async def test_completes_without_eof(self, patch_exec, tmp_path): + """A stdout pipe that never closes must not stall the turn.""" + patch_exec(_LeakyPipeProcess(HAPPY_STREAM)) + record = await asyncio.wait_for(_run(_agent(), tmp_path, timeout=300), timeout=30) + + assert record.crashed is False + assert record.assistant_turn_count == 2 + assert record.agent_output == "Created the file." + + +class _StderrBackpressureProcess(_FakeProcess): + """Models the two-pipe deadlock: the child makes no progress until stderr is read. + + A real CLI that fills the ~64 KiB stderr pipe blocks on write, so it emits no + further stdout and never exits. Reading stderr only after the stdout loop ends + therefore hangs the turn to its deadline. + """ + + def __init__(self, lines: list[str], **kwargs: Any) -> None: + super().__init__(lines, **kwargs) + self._stderr_read = asyncio.Event() + + async def readline(self) -> bytes: + await self._stderr_read.wait() + return await super().readline() + + async def read(self) -> bytes: + self._stderr_read.set() + return self._stderr + + +class TestStderrIsDrainedConcurrently: + async def test_turn_completes_under_stderr_backpressure(self, patch_exec, tmp_path): + patch_exec(_StderrBackpressureProcess(HAPPY_STREAM, stderr=b"noisy")) + # Bounded so a regression fails here instead of hanging the suite. + record = await asyncio.wait_for(_run(_agent(), tmp_path, timeout=300), timeout=10) + assert record.assistant_turn_count == 2 + assert record.crashed is False + + +class TestCooperativeStop: + def test_capability_flag_is_declared(self): + assert OpenCodeAgent.supports_cooperative_stop is True + + async def test_should_stop_ends_turn_cleanly(self, patch_exec, tmp_path): + """A live subprocess must be torn down, and the turn must not be a crash.""" + proc = _RunningProcess(HAPPY_STREAM) + patch_exec(proc) + record = await _run(_agent(), tmp_path, should_stop=lambda: True) + + assert record.crashed is False + assert proc.terminated is True + # Stopped at the first event boundary rather than draining the stream. + assert record.assistant_turn_count < 2 + + 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 diff --git a/tests/test_pricing_registry.py b/tests/test_pricing_registry.py index ba647b7f..df29d001 100644 --- a/tests/test_pricing_registry.py +++ b/tests/test_pricing_registry.py @@ -3,7 +3,7 @@ import pytest from coder_eval import pricing -from coder_eval.pricing import ModelPricing, calculate_cost, register_pricing +from coder_eval.pricing import ModelPricing, calculate_cost, is_priced, register_pricing @pytest.fixture(autouse=True) @@ -29,6 +29,18 @@ def test_registered_bare_key_resolves_through_prefixed_lookup(): assert calculate_cost("eu.acme-1", 1_000_000, 0) == 1.0 +def test_openrouter_provider_prefix_normalizes_to_bare_key(): + """An agent that addresses OpenRouter natively (OpenCode) reports the model + WITH its provider prefix, while the rate-card keys are the bare vendor/model + ids the LiteLLM route uses. Both spellings must price identically, else the + same model silently goes unpriced depending on which agent ran it.""" + assert is_priced("deepseek/deepseek-v4-flash-0731") + assert is_priced("openrouter/deepseek/deepseek-v4-flash-0731") + bare = calculate_cost("deepseek/deepseek-v4-flash-0731", 1_000_000, 1_000_000) + prefixed = calculate_cost("openrouter/deepseek/deepseek-v4-flash-0731", 1_000_000, 1_000_000) + assert bare == prefixed > 0 + + def test_lookup_rate_overlay_precedes_builtins(): """_lookup_rate returns the registered overlay before the built-in table. The anti-shadow rule forbids a registered value *differing* from a built-in, diff --git a/uv.lock b/uv.lock index bbe0e461..84c51130 100644 --- a/uv.lock +++ b/uv.lock @@ -531,7 +531,7 @@ requires-dist = [ { name = "typer", specifier = ">=0.24.1" }, { name = "uipath", marker = "extra == 'uipath'", specifier = ">=2.10.31" }, ] -provides-extras = ["dev", "uipath", "codex", "antigravity"] +provides-extras = ["dev", "uipath", "codex", "antigravity", "opencode"] [[package]] name = "colorama" From 5d9a73e3e838e5c66fbfce74b652c43300d4bca8 Mon Sep 17 00:00:00 2001 From: mohsen-uipath Date: Fri, 14 Aug 2026 10:21:56 -0700 Subject: [PATCH 02/12] fix(opencode): bound the post-EOF reap, reap the whole process group, test every failure path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- docs/agents/OPENCODE.md | 17 +- src/coder_eval/agents/opencode_agent.py | 88 ++++++-- tests/test_opencode_agent.py | 263 ++++++++++++++++++++++-- 3 files changed, 332 insertions(+), 36 deletions(-) diff --git a/docs/agents/OPENCODE.md b/docs/agents/OPENCODE.md index 6844ecdd..421922f9 100644 --- a/docs/agents/OPENCODE.md +++ b/docs/agents/OPENCODE.md @@ -201,12 +201,17 @@ with an error naming the unrecognized event types it saw instead. - **Cooperative stop is at event granularity.** `should_stop` is polled between events and honored by terminating the CLI, so `stop_early` works, but the cut lands on an event boundary rather than mid-tool. -- **Pipe teardown.** `opencode run` leaves a local server child holding the - inherited stdout/stderr pipes, so EOF never arrives on its own. The agent races - each read against process exit and bounds the post-exit drain; this is why reads - are never left to block on EOF alone. stderr gets its own concurrent reader from - the moment the CLI starts — draining it only afterwards would let a full stderr - pipe block the child mid-write and stall stdout with it. +- **Pipe and process teardown.** `opencode run` leaves a local server child + holding the inherited stdout/stderr pipes, so EOF never arrives on its own. The + agent races each read against process exit, bounds the post-exit drain, and + bounds the final reap by the turn deadline (a CLI that closes its stream but + never exits is cut as a timeout/crash, not waited out). stderr gets its own + concurrent reader from the moment the CLI starts — draining it only afterwards + would let a full stderr pipe block the child mid-write and stall stdout with it. + Each invocation runs in its own process group (`start_new_session`), and + `kill()` / `kill_sync()` / `stop()` sweep that group with SIGKILL, so the server + child is reaped rather than leaked across a batch; OpenCode persists sessions on + disk, so `--session` continuity survives the sweep. ## Troubleshooting diff --git a/src/coder_eval/agents/opencode_agent.py b/src/coder_eval/agents/opencode_agent.py index 94f653d3..2c8ecd78 100644 --- a/src/coder_eval/agents/opencode_agent.py +++ b/src/coder_eval/agents/opencode_agent.py @@ -632,6 +632,11 @@ def __init__( self._plugin_tools_dir: str | None = None self._session_id: str | None = None self._process: asyncio.subprocess.Process | None = None + # Process-group ids (== the CLI's pid under start_new_session) of every + # invocation this agent spawned, swept on kill()/kill_sync()/stop() — + # `opencode run` leaves a server child alive after the CLI exits, and + # signaling only the CLI pid would orphan it (a slow leak across a batch). + self._spawned_pgids: list[int] = [] self._state = AgentState.WORKING # --- lifecycle --------------------------------------------------------- @@ -667,23 +672,41 @@ async def stop(self) -> None: async def kill(self) -> None: proc = self._process - if proc is None or proc.returncode is not None: - return - with contextlib.suppress(ProcessLookupError): - proc.terminate() - with contextlib.suppress(TimeoutError, asyncio.TimeoutError): - await asyncio.wait_for(proc.wait(), timeout=_TERM_GRACE_SECONDS) - if proc.returncode is None: + if proc is not None and proc.returncode is None: with contextlib.suppress(ProcessLookupError): - proc.kill() + proc.terminate() + with contextlib.suppress(TimeoutError, asyncio.TimeoutError): + await asyncio.wait_for(proc.wait(), timeout=_TERM_GRACE_SECONDS) + if proc.returncode is None: + with contextlib.suppress(ProcessLookupError): + proc.kill() + self._sweep_process_groups() def kill_sync(self) -> None: - """SIGKILL the in-flight CLI by PID (called from the watchdog thread).""" + """SIGKILL the in-flight CLI and its process group (watchdog thread; must not await).""" proc = self._process - if proc is None or proc.returncode is not None: + if proc is not None and proc.returncode is None: + with contextlib.suppress(ProcessLookupError, PermissionError): + os.kill(proc.pid, signal.SIGKILL) + self._sweep_process_groups() + + def _sweep_process_groups(self) -> None: + """SIGKILL every process group this agent spawned (POSIX only). + + Each invocation runs in its own session (``start_new_session``), so its + pgid is the CLI's pid and the group contains ONLY what that invocation + spawned — the lingering server child included, a shared daemon we did not + start excluded. The CLI itself gets SIGTERM-then-SIGKILL first (see + ``kill``); this reaps whatever survives it. Sessions are persisted on + disk by OpenCode, so killing a turn's server does not lose ``--session`` + continuity. + """ + if os.name != "posix": return - with contextlib.suppress(ProcessLookupError, PermissionError): - os.kill(proc.pid, signal.SIGKILL) + for pgid in self._spawned_pgids: + with contextlib.suppress(ProcessLookupError, PermissionError, OSError): + os.killpg(pgid, signal.SIGKILL) + self._spawned_pgids.clear() def get_environment_info(self) -> dict[str, Any]: info: dict[str, Any] = {"opencode_model": self.config.model, "opencode_pure": self.config.pure} @@ -776,8 +799,14 @@ def emit(event: StreamEvent) -> None: # read), which blows past StreamReader's default 64 KiB line cap and # would raise ValueError mid-stream, killing the read loop. limit=STDOUT_LINE_LIMIT_BYTES, + # Own session/process group, so teardown can killpg the lingering + # server child without touching anything this invocation didn't + # spawn. POSIX-only knob; harmless False elsewhere. + start_new_session=os.name == "posix", ) self._process = proc + if os.name == "posix": + self._spawned_pgids.append(proc.pid) assert proc.stdout is not None # Drain stderr CONCURRENTLY, from the moment the CLI starts. Reading it @@ -840,7 +869,15 @@ def emit(event: StreamEvent) -> None: read_task.cancel() exit_waiter.cancel() - status = await self._settle_turn(proc, state, collector, stderr_drain, stopped_early=stopped_early) + status = await self._settle_turn( + proc, + state, + collector, + stderr_drain, + stopped_early=stopped_early, + deadline=deadline, + timeout=timeout, + ) state.finalize(status) # Build BEFORE marking the turn clean: a failure in the reduction is a # failed turn, and `_end_turn_ok` would clear the rollback flag that @@ -880,15 +917,36 @@ async def _settle_turn( stderr_drain: asyncio.Future[bytes] | None, *, stopped_early: bool, + deadline: float | None, + timeout: float | None, ) -> AgentEndStatus: """Reap the CLI once the read loop is done and decide the turn's end status. Raises ``AgentCrashError`` (via :meth:`_crash_turn`) when the stream carried a structured error, when the process died with neither a structured error nor an intentional stop, or when a clean exit recognized no events at all - (a zero-telemetry turn must not score — see the guard below). + (a zero-telemetry turn must not score — see the guard below). Raises + ``TurnTimeoutError`` when the turn deadline elapses while waiting for the + exit. """ - await proc.wait() + # Bound the reap: the read loop can end at EOF with the CLI still alive + # (it closed its stream but never exited), and an unbounded wait here + # would outlive the turn deadline — the one window where `timeout` was + # previously unenforced. Give the exit the deadline's remainder, or a + # short fixed grace when no deadline is configured (post-EOF, a healthy + # CLI exits almost immediately). + remaining = None if deadline is None else max(0.0, deadline - time.monotonic()) + try: + await asyncio.wait_for(proc.wait(), timeout=_TERM_GRACE_SECONDS if remaining is None else remaining) + except TimeoutError: + if remaining is not None: + await self._timeout_turn(state, collector, timeout or 0.0) + await self.kill() + self._crash_turn( + state, + collector, + f"OpenCode closed its event stream but did not exit within {_TERM_GRACE_SECONDS:.0f}s", + ) # Collect what the concurrent reader drained. Bounded for the same reason as # the read loop: the inherited stderr pipe outlives the CLI, so waiting for # the reader's own EOF would block. Shielded so the timeout doesn't kill it diff --git a/tests/test_opencode_agent.py b/tests/test_opencode_agent.py index c931a7f7..582b5985 100644 --- a/tests/test_opencode_agent.py +++ b/tests/test_opencode_agent.py @@ -17,15 +17,23 @@ import asyncio import json +import os +import signal from typing import Any import pytest -from coder_eval.agents.opencode_agent import OpenCodeAgent, _unwrap -from coder_eval.errors import AgentCrashError +from coder_eval.agents.opencode_agent import OpenCodeAgent, _OpenCodeTurnState, _unwrap +from coder_eval.errors import AgentCrashError, TurnTimeoutError from coder_eval.models import AssistantMessage, OpenCodeAgentConfig, PermissionMode from coder_eval.pricing import calculate_cost -from coder_eval.streaming.events import AgentEndEvent, AgentEndStatus, AgentStartEvent +from coder_eval.streaming.events import ( + AgentEndEvent, + AgentEndStatus, + AgentStartEvent, + ToolEndEvent, + ToolEndStatus, +) SESSION = "ses_test123" @@ -151,8 +159,13 @@ def kill(self) -> None: @pytest.fixture def patch_exec(monkeypatch: pytest.MonkeyPatch): - """Patch subprocess spawn; return a dict capturing the argv used.""" - captured: dict[str, Any] = {} + """Patch subprocess spawn; return a dict capturing the argv used. + + Also stubs ``os.killpg`` (recording each call under ``captured["killpg"]``) so + the agent's process-group sweep can never signal a real group whose id happens + to collide with the fake pid. + """ + captured: dict[str, Any] = {"killpg": []} def _install(proc: _FakeProcess) -> dict[str, Any]: async def fake_exec(*argv: str, **kwargs: Any) -> _FakeProcess: @@ -163,6 +176,7 @@ async def fake_exec(*argv: str, **kwargs: Any) -> _FakeProcess: monkeypatch.setattr(asyncio, "create_subprocess_exec", fake_exec) monkeypatch.setattr("shutil.which", lambda _name: "/usr/local/bin/opencode") + monkeypatch.setattr(os, "killpg", lambda pgid, sig: captured["killpg"].append((pgid, sig))) return captured return _install @@ -628,6 +642,16 @@ async def readline(self) -> bytes: raise ValueError("Separator is not found, and chunk exceed the limit") +class _EventRecorder: + """Minimal ``StreamCallback``: records every event the agent emits.""" + + def __init__(self) -> None: + self.events: list[Any] = [] + + def on_event(self, event: Any) -> None: + self.events.append(event) + + class TestUnexpectedErrorContract: """An unanticipated exception must still honor the pending-turn contract. @@ -637,15 +661,6 @@ class TestUnexpectedErrorContract: orchestrator never reaches ``discard_pending_turn``. """ - class _Recorder: - """Minimal ``StreamCallback``: records every event the agent emits.""" - - def __init__(self) -> None: - self.events: list[Any] = [] - - def on_event(self, event: Any) -> None: - self.events.append(event) - async def test_stream_error_becomes_a_crash_with_partial_parked(self, patch_exec, tmp_path): stream = [ _evt("step_start", {"id": "prt_1", "messageID": "msg_1", "type": "step-start"}), @@ -688,7 +703,7 @@ async def boom(*_argv: str, **_kwargs: Any): async def test_terminal_event_is_emitted_exactly_once(self, patch_exec, tmp_path): """The protocol allows exactly one AgentEnd per communicate(), crash included.""" patch_exec(_ExplodingProcess([_evt("step_start", {"id": "prt_1", "messageID": "msg_1"})])) - recorder = self._Recorder() + recorder = _EventRecorder() with pytest.raises(AgentCrashError): await _run(_agent(), tmp_path, stream_callback=recorder) @@ -794,3 +809,221 @@ 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 + + +class _HangingProcess(_FakeProcess): + """Emits nothing and never exits until it is signaled. + + Models a CLI stuck mid-turn (a wedged provider call): no stdout, no exit — + the shape that must be cut by the turn deadline, not waited out. + """ + + def __init__(self, lines: list[str], **kwargs: Any) -> None: + super().__init__(lines, **kwargs) + self._exited = asyncio.Event() + + async def readline(self) -> bytes: + if self._lines: + return self._lines.pop(0) + await self._exited.wait() + return b"" + + async def read(self) -> bytes: + await self._exited.wait() + return self._stderr + + async def wait(self) -> int: + await self._exited.wait() + self.returncode = self._final_returncode + return self.returncode + + def terminate(self) -> None: + self.terminated = True + self._exited.set() + + def kill(self) -> None: + self._exited.set() + + +class _EofNoExitProcess(_HangingProcess): + """Replays its lines, signals EOF — but never exits until killed. + + Models a CLI that closed its stream during shutdown and then wedged: the one + window where the read loop is already done, so only a bounded reap in + ``_settle_turn`` stands between the turn and an unbounded hang. + """ + + async def readline(self) -> bytes: + if self._lines: + return self._lines.pop(0) + return b"" # EOF — but the process is still alive + + +class TestTimeoutContract: + async def test_deadline_raises_turn_timeout_with_partial_parked(self, patch_exec, tmp_path): + """A wedged CLI must yield TurnTimeoutError + a crashed partial record, + with exactly one terminal AgentEndEvent (status TIMEOUT) emitted.""" + proc = _HangingProcess([_evt("step_start", {"id": "prt_1", "messageID": "msg_1"})]) + patch_exec(proc) + agent = _agent() + recorder = _EventRecorder() + + with pytest.raises(TurnTimeoutError): + await _run(agent, tmp_path, timeout=0.2, stream_callback=recorder) + + partial = agent.pending_turn + assert partial is not None + assert partial.crashed is True + assert proc.terminated is True # the CLI was torn down, not abandoned + ends = [e for e in recorder.events if isinstance(e, AgentEndEvent)] + assert len(ends) == 1 + assert ends[0].status is AgentEndStatus.TIMEOUT + + await agent.discard_pending_turn() + assert agent._iteration == 0 # the failed turn's bump was rolled back + + async def test_eof_without_exit_hits_the_deadline(self, patch_exec, tmp_path): + """Stream closed, process wedged: the post-EOF reap must be bounded by the + turn deadline instead of waiting for an exit that never comes.""" + proc = _EofNoExitProcess(HAPPY_STREAM) + patch_exec(proc) + agent = _agent() + + with pytest.raises(TurnTimeoutError): + await asyncio.wait_for(_run(agent, tmp_path, timeout=0.3), timeout=10) + + # Everything parsed before the wedge survives on the partial record. + partial = agent.pending_turn + assert partial is not None + assert partial.crashed is True + assert partial.token_usage is not None + assert partial.token_usage.output_tokens > 0 + + async def test_eof_without_exit_and_no_deadline_crashes(self, patch_exec, monkeypatch, tmp_path): + """With no turn deadline configured, the reap still gets a fixed grace — + a stream-closed-but-wedged CLI is a crash, not an indefinite hang.""" + monkeypatch.setattr("coder_eval.agents.opencode_agent._TERM_GRACE_SECONDS", 0.1) + proc = _EofNoExitProcess(HAPPY_STREAM) + patch_exec(proc) + + with pytest.raises(AgentCrashError, match="did not exit"): + await asyncio.wait_for(_run(_agent(), tmp_path), timeout=10) + + +class TestExternalCancel: + async def test_cancel_parks_partial_and_reraises(self, patch_exec, tmp_path): + """The watchdog's CancelledError must not swallow captured telemetry: the + partial record is parked, the terminal event says CRASHED, and the + cancellation still propagates.""" + proc = _HangingProcess([_evt("step_start", {"id": "prt_1", "messageID": "msg_1"})]) + patch_exec(proc) + agent = _agent() + await agent.start(str(tmp_path)) + recorder = _EventRecorder() + + task = asyncio.ensure_future(agent.communicate("do the thing", stream_callback=recorder)) + await asyncio.sleep(0.05) # let it spawn and read the first event + task.cancel() + with pytest.raises(asyncio.CancelledError): + await task + + partial = agent.pending_turn + assert partial is not None + assert partial.crashed is True + ends = [e for e in recorder.events if isinstance(e, AgentEndEvent)] + assert len(ends) == 1 + assert ends[0].status is AgentEndStatus.CRASHED + assert ends[0].crash_reason == "turn cancelled" + + +class TestProcessGroupTeardown: + async def test_spawn_uses_its_own_session(self, patch_exec, tmp_path): + """Each invocation must be its own process group, so killpg can reap the + server child without touching anything this invocation didn't spawn.""" + captured = patch_exec(_FakeProcess(HAPPY_STREAM)) + await _run(_agent(), tmp_path) + assert captured["kwargs"]["start_new_session"] is (os.name == "posix") + + async def test_stop_sweeps_the_spawned_group(self, patch_exec, tmp_path): + """`opencode run` leaves a server child holding the pipes; stop() must + SIGKILL the whole group or every task in a batch leaks one.""" + captured = patch_exec(_FakeProcess(HAPPY_STREAM)) + agent = _agent() + await _run(agent, tmp_path) + assert captured["killpg"] == [] # a clean turn does not kill mid-run state + + await agent.stop() + assert (4242, signal.SIGKILL) in captured["killpg"] + + async def test_cooperative_stop_sweeps_the_group_too(self, patch_exec, tmp_path): + captured = patch_exec(_RunningProcess(HAPPY_STREAM)) + await _run(_agent(), tmp_path, should_stop=lambda: True) + assert (4242, signal.SIGKILL) in captured["killpg"] + + async def test_kill_sync_signals_pid_and_group(self, patch_exec, monkeypatch, tmp_path): + """kill_sync runs on the watchdog's non-asyncio thread: plain os.kill on + the CLI plus a group sweep, no awaits.""" + killed: list[tuple[int, int]] = [] + monkeypatch.setattr(os, "kill", lambda pid, sig: killed.append((pid, sig))) + captured = patch_exec(_HangingProcess([])) + agent = _agent() + await agent.start(str(tmp_path)) + proc = _HangingProcess([]) + agent._process = proc # type: ignore[assignment] + agent._spawned_pgids = [proc.pid] + + agent.kill_sync() + + assert (4242, signal.SIGKILL) in killed + assert (4242, signal.SIGKILL) in captured["killpg"] + + +class TestToolFailureCapture: + @staticmethod + def _failing_tool(error: str) -> str: + return _evt( + "tool_use", + { + "id": "prt_2", + "messageID": "msg_1", + "type": "tool", + "tool": "bash", + "callID": "call_1", + "state": {"status": "error", "input": {"command": "ls /root"}, "error": error}, + }, + ) + + async def test_tool_error_is_captured_not_dropped(self, patch_exec, tmp_path): + recorder = _EventRecorder() + patch_exec(_FakeProcess([self._failing_tool("boom: command exploded")])) + record = await _run(_agent(), tmp_path, stream_callback=recorder) + + [cmd] = record.commands + assert cmd.result_status == "error" + assert cmd.error_message == "boom: command exploded" + [end] = [e for e in recorder.events if isinstance(e, ToolEndEvent)] + assert end.status is ToolEndStatus.ERROR + + async def test_permission_denial_gets_its_own_status(self, patch_exec, tmp_path): + recorder = _EventRecorder() + patch_exec(_FakeProcess([self._failing_tool("Permission denied by policy")])) + record = await _run(_agent(), tmp_path, stream_callback=recorder) + + [cmd] = record.commands + assert cmd.result_status == "error" # the persisted tri-state folds both + [end] = [e for e in recorder.events if isinstance(e, ToolEndEvent)] + assert end.status is ToolEndStatus.PERMISSION_DENIED + + def test_orphan_result_is_never_dropped(self): + """A result with no matching call still surfaces as an `unknown` tool.""" + state = _OpenCodeTurnState(task_id="t", iteration=1, user_input="x", model=None) + events: list[Any] = [] + state.bind(events.append) + + state._close_tool("ghost", status=ToolEndStatus.UNRESOLVED, summary=None, error="no result observed") + + [event] = events + assert isinstance(event, ToolEndEvent) + assert event.tool.tool_name == "unknown" + assert event.tool.result_status == "unknown" + assert event.tool.error_message == "no result observed" From 8d4f12c0ac9594b6fa6fa7c3551b14c1c613ecb7 Mon Sep 17 00:00:00 2001 From: mohsen-uipath Date: Fri, 14 Aug 2026 10:25:20 -0700 Subject: [PATCH 03/12] docs(opencode): note _TERM_GRACE_SECONDS's second role as the post-EOF 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. --- src/coder_eval/agents/opencode_agent.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/coder_eval/agents/opencode_agent.py b/src/coder_eval/agents/opencode_agent.py index 2c8ecd78..bef7d7ce 100644 --- a/src/coder_eval/agents/opencode_agent.py +++ b/src/coder_eval/agents/opencode_agent.py @@ -81,6 +81,9 @@ logger = logging.getLogger(__name__) # Grace period between SIGTERM and SIGKILL when tearing down the CLI subprocess. +# Doubles as the post-EOF exit grace in _settle_turn when no turn deadline is +# configured (a CLI that closed its stream but won't exit gets this long to die +# before the turn is crashed). _TERM_GRACE_SECONDS = 5.0 # How long to keep draining stdout/stderr after the CLI process has been reaped. From 6705a10314bfc798bd2d71845a7b93e373f1ef8f Mon Sep 17 00:00:00 2001 From: mohsen-uipath Date: Fri, 14 Aug 2026 10:37:36 -0700 Subject: [PATCH 04/12] chore(opencode): standardize on deepseek-v4-pro, drop the flash-0731 rate entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- docs/agents/OPENCODE.md | 6 +++--- evalboard/lib/__tests__/pricing-parity.test.ts | 1 - src/coder_eval/models/agent_config.py | 2 +- src/coder_eval/pricing.py | 3 +-- tasks/opencode_smoke_test.yaml | 2 +- tests/test_opencode_agent.py | 10 +++++----- tests/test_pricing_registry.py | 8 ++++---- 7 files changed, 15 insertions(+), 17 deletions(-) diff --git a/docs/agents/OPENCODE.md b/docs/agents/OPENCODE.md index 421922f9..a3f819d0 100644 --- a/docs/agents/OPENCODE.md +++ b/docs/agents/OPENCODE.md @@ -88,7 +88,7 @@ uv run coder-eval run tasks/my_task.yaml -D agent.type=opencode -D agent.model=o agent: type: "opencode" # provider/model, exactly as `opencode models` prints it. - model: "openrouter/deepseek/deepseek-v4-flash-0731" + model: "openrouter/deepseek/deepseek-v4-pro" permission_mode: "acceptEdits" variant: "high" # optional: provider reasoning effort pure: true # optional (default): run with --pure, no host plugins @@ -101,8 +101,8 @@ agent: | `agent.model` | Provider | Credential | |---|---|---| -| `openrouter/deepseek/deepseek-v4-flash-0731` | OpenRouter | `OPENROUTER_API_KEY` | -| `deepseek/deepseek-v4-flash-0731` | DeepSeek direct | `DEEPSEEK_API_KEY` | +| `openrouter/deepseek/deepseek-v4-pro` | OpenRouter | `OPENROUTER_API_KEY` | +| `deepseek/deepseek-v4-pro` | DeepSeek direct | `DEEPSEEK_API_KEY` | OpenCode speaks OpenRouter natively, so it does **not** need the LiteLLM proxy — that shim exists to translate Anthropic ↔ OpenAI for the Claude Code SDK. diff --git a/evalboard/lib/__tests__/pricing-parity.test.ts b/evalboard/lib/__tests__/pricing-parity.test.ts index dce4d7a9..531657ca 100644 --- a/evalboard/lib/__tests__/pricing-parity.test.ts +++ b/evalboard/lib/__tests__/pricing-parity.test.ts @@ -110,7 +110,6 @@ describe("pricing.ts ↔ pricing.py parity", () => { "moonshotai/kimi-k3", "z-ai/glm-5.2", "deepseek/deepseek-v4-pro", - "deepseek/deepseek-v4-flash-0731", ]); test("every DELIBERATELY_UNMIRRORED id still exists in pricing.py", () => { diff --git a/src/coder_eval/models/agent_config.py b/src/coder_eval/models/agent_config.py index a705d450..91cb8c03 100644 --- a/src/coder_eval/models/agent_config.py +++ b/src/coder_eval/models/agent_config.py @@ -284,7 +284,7 @@ class OpenCodeAgentConfig(BaseAgentConfig): Drives the ``opencode`` CLI in non-interactive mode (``opencode run --format json``), which streams newline-delimited JSON events on stdout. ``model`` is OpenCode's ``provider/model`` form (e.g. - ``deepseek/deepseek-v4-flash-0731``) and is passed through verbatim via ``-m``. + ``deepseek/deepseek-v4-pro``) and is passed through verbatim via ``-m``. Permission handling is derived from the inherited ``permission_mode``: every mode except :attr:`PermissionMode.PLAN` passes ``--auto`` so an unattended diff --git a/src/coder_eval/pricing.py b/src/coder_eval/pricing.py index 0c74a3e8..88af853a 100644 --- a/src/coder_eval/pricing.py +++ b/src/coder_eval/pricing.py @@ -117,7 +117,6 @@ class ModelPricing: "moonshotai/kimi-k3": ModelPricing(3.0, 15.0, 3.0, 0.30), "z-ai/glm-5.2": ModelPricing(0.7168, 2.2528, 0.7168, 0.13312), "deepseek/deepseek-v4-pro": ModelPricing(0.435, 0.87, 0.435, 0.003625), - "deepseek/deepseek-v4-flash-0731": ModelPricing(0.14, 0.28, 0.14, 0.0028), } @@ -177,7 +176,7 @@ def _normalize_model(model: str) -> str: # LiteLLM/Bedrock routing prefixes (e.g. "converse/zai.glm-5", # "bedrock/converse/deepseek.v3.2") → bare model id. ``openrouter/`` is here # because agents that address OpenRouter natively (OpenCode) report the model - # WITH its provider prefix ("openrouter/deepseek/deepseek-v4-flash-0731"), + # WITH its provider prefix ("openrouter/deepseek/deepseek-v4-pro"), # while the OpenRouter rate-card keys are the bare vendor/model ids that the # LiteLLM route already uses — without this strip the same model prices under # LiteLLM and silently goes unpriced under OpenCode. diff --git a/tasks/opencode_smoke_test.yaml b/tasks/opencode_smoke_test.yaml index bead022b..2bd7717a 100644 --- a/tasks/opencode_smoke_test.yaml +++ b/tasks/opencode_smoke_test.yaml @@ -17,7 +17,7 @@ agent: # Real per-call cost still lands on the turn: OpenCode reports `cost` on every # step_finish event and the harness folds it into token_usage.total_cost_usd # (falling back to the rate card when the stream omits or zeroes it). - model: "openrouter/deepseek/deepseek-v4-flash-0731" + model: "openrouter/deepseek/deepseek-v4-pro" permission_mode: "acceptEdits" success_criteria: diff --git a/tests/test_opencode_agent.py b/tests/test_opencode_agent.py index 582b5985..e630ac45 100644 --- a/tests/test_opencode_agent.py +++ b/tests/test_opencode_agent.py @@ -188,7 +188,7 @@ async def _run(agent: OpenCodeAgent, tmp_path: Any, prompt: str = "do the thing" def _agent(**overrides: Any) -> OpenCodeAgent: - config = OpenCodeAgentConfig(type="opencode", **{"model": "deepseek/deepseek-v4-flash-0731", **overrides}) + config = OpenCodeAgentConfig(type="opencode", **{"model": "deepseek/deepseek-v4-pro", **overrides}) return OpenCodeAgent(config, task_id="t1") @@ -214,7 +214,7 @@ async def test_builds_turn_record(self, patch_exec, tmp_path): assert record.crashed is False assert record.agent_output == "Created the file." assert record.assistant_turn_count == 2 - assert record.model_used == "deepseek/deepseek-v4-flash-0731" + assert record.model_used == "deepseek/deepseek-v4-pro" async def test_token_buckets_accumulate_across_steps(self, patch_exec, tmp_path): patch_exec(_FakeProcess(HAPPY_STREAM)) @@ -386,7 +386,7 @@ async def test_missing_cost_is_priced_from_the_rate_card(self, patch_exec, tmp_p record = await _run(_agent(), tmp_path) assert record.token_usage is not None - expected = calculate_cost("deepseek/deepseek-v4-flash-0731", uncached_input_tokens=1000, output_tokens=500) + expected = calculate_cost("deepseek/deepseek-v4-pro", uncached_input_tokens=1000, output_tokens=500) assert expected is not None and expected > 0 assert record.token_usage.total_cost_usd == pytest.approx(expected) @@ -412,7 +412,7 @@ async def test_zero_reported_cost_on_a_priced_model_uses_the_rate_card(self, pat with caplog.at_level("WARNING"): record = await _run(_agent(), tmp_path) - expected = calculate_cost("deepseek/deepseek-v4-flash-0731", uncached_input_tokens=1000, output_tokens=500) + expected = calculate_cost("deepseek/deepseek-v4-pro", uncached_input_tokens=1000, output_tokens=500) assert expected is not None and expected > 0 assert record.token_usage is not None assert record.token_usage.total_cost_usd == pytest.approx(expected) @@ -491,7 +491,7 @@ async def test_defaults_include_auto_and_pure(self, patch_exec, tmp_path): assert argv[:4] == ["opencode", "run", "--format", "json"] assert "--auto" in argv assert "--pure" in argv - assert argv[argv.index("-m") + 1] == "deepseek/deepseek-v4-flash-0731" + assert argv[argv.index("-m") + 1] == "deepseek/deepseek-v4-pro" assert argv[-1] == "do the thing" async def test_plan_mode_withholds_auto(self, patch_exec, tmp_path): diff --git a/tests/test_pricing_registry.py b/tests/test_pricing_registry.py index df29d001..928e29f1 100644 --- a/tests/test_pricing_registry.py +++ b/tests/test_pricing_registry.py @@ -34,10 +34,10 @@ def test_openrouter_provider_prefix_normalizes_to_bare_key(): WITH its provider prefix, while the rate-card keys are the bare vendor/model ids the LiteLLM route uses. Both spellings must price identically, else the same model silently goes unpriced depending on which agent ran it.""" - assert is_priced("deepseek/deepseek-v4-flash-0731") - assert is_priced("openrouter/deepseek/deepseek-v4-flash-0731") - bare = calculate_cost("deepseek/deepseek-v4-flash-0731", 1_000_000, 1_000_000) - prefixed = calculate_cost("openrouter/deepseek/deepseek-v4-flash-0731", 1_000_000, 1_000_000) + assert is_priced("deepseek/deepseek-v4-pro") + assert is_priced("openrouter/deepseek/deepseek-v4-pro") + bare = calculate_cost("deepseek/deepseek-v4-pro", 1_000_000, 1_000_000) + prefixed = calculate_cost("openrouter/deepseek/deepseek-v4-pro", 1_000_000, 1_000_000) assert bare == prefixed > 0 From ec6531de5e98d32bf87b21ded471ddf4272e782b Mon Sep 17 00:00:00 2001 From: mohsen-uipath Date: Fri, 14 Aug 2026 10:57:35 -0700 Subject: [PATCH 05/12] fix(opencode): typecheck on Windows, satisfy both CodeQL findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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. --- src/coder_eval/agents/opencode_agent.py | 10 ++++++++-- tests/test_opencode_agent.py | 7 +++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/coder_eval/agents/opencode_agent.py b/src/coder_eval/agents/opencode_agent.py index bef7d7ce..a6b4ca6c 100644 --- a/src/coder_eval/agents/opencode_agent.py +++ b/src/coder_eval/agents/opencode_agent.py @@ -86,6 +86,11 @@ # before the turn is crashed). _TERM_GRACE_SECONDS = 5.0 +# SIGKILL does not exist on Windows (where the process-group sweep is a no-op +# anyway); resolve it dynamically so the module imports and typechecks on every +# platform, falling back to SIGTERM for the direct-pid kill_sync path. +_SIGKILL: signal.Signals = getattr(signal, "SIGKILL", signal.SIGTERM) + # How long to keep draining stdout/stderr after the CLI process has been reaped. # `opencode run` leaves a local server child holding the inherited pipes open, so # EOF never arrives on its own and every post-exit read must be bounded. @@ -690,7 +695,7 @@ def kill_sync(self) -> None: proc = self._process if proc is not None and proc.returncode is None: with contextlib.suppress(ProcessLookupError, PermissionError): - os.kill(proc.pid, signal.SIGKILL) + os.kill(proc.pid, _SIGKILL) self._sweep_process_groups() def _sweep_process_groups(self) -> None: @@ -708,7 +713,7 @@ def _sweep_process_groups(self) -> None: return for pgid in self._spawned_pgids: with contextlib.suppress(ProcessLookupError, PermissionError, OSError): - os.killpg(pgid, signal.SIGKILL) + os.killpg(pgid, _SIGKILL) self._spawned_pgids.clear() def get_environment_info(self) -> dict[str, Any]: @@ -907,6 +912,7 @@ def emit(event: StreamEvent) -> None: # `_iteration` left incremented because the orchestrator never reaches # `discard_pending_turn`. Same guard, same reasons, as CodexAgent. self._crash_turn(state, collector, f"OpenCode turn failed: {e!s}", cause=e) + raise # unreachable (_crash_turn is NoReturn) — makes the no-fall-through explicit finally: if stderr_drain is not None: stderr_drain.cancel() diff --git a/tests/test_opencode_agent.py b/tests/test_opencode_agent.py index e630ac45..a6daa868 100644 --- a/tests/test_opencode_agent.py +++ b/tests/test_opencode_agent.py @@ -176,7 +176,9 @@ async def fake_exec(*argv: str, **kwargs: Any) -> _FakeProcess: monkeypatch.setattr(asyncio, "create_subprocess_exec", fake_exec) monkeypatch.setattr("shutil.which", lambda _name: "/usr/local/bin/opencode") - monkeypatch.setattr(os, "killpg", lambda pgid, sig: captured["killpg"].append((pgid, sig))) + # raising=False: os.killpg does not exist on Windows, where the sweep is a + # no-op — the stub must still install so the fixture works on every platform. + monkeypatch.setattr(os, "killpg", lambda pgid, sig: captured["killpg"].append((pgid, sig)), raising=False) return captured return _install @@ -925,7 +927,7 @@ async def test_cancel_parks_partial_and_reraises(self, patch_exec, tmp_path): await asyncio.sleep(0.05) # let it spawn and read the first event task.cancel() with pytest.raises(asyncio.CancelledError): - await task + _ = await task # the await re-raises the cancellation; no value ever exists partial = agent.pending_turn assert partial is not None @@ -936,6 +938,7 @@ async def test_cancel_parks_partial_and_reraises(self, patch_exec, tmp_path): assert ends[0].crash_reason == "turn cancelled" +@pytest.mark.skipif(os.name != "posix", reason="process-group teardown (killpg/SIGKILL) is POSIX-only by design") class TestProcessGroupTeardown: async def test_spawn_uses_its_own_session(self, patch_exec, tmp_path): """Each invocation must be its own process group, so killpg can reap the From 0c45010dfb6689fd32e9333d2ae5f0741f1adba3 Mon Sep 17 00:00:00 2001 From: mohsen-uipath Date: Fri, 14 Aug 2026 11:23:34 -0700 Subject: [PATCH 06/12] fix(opencode): keep the smoke task out of the CI smoke-pass bucket MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- tasks/opencode_smoke_test.yaml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tasks/opencode_smoke_test.yaml b/tasks/opencode_smoke_test.yaml index 2bd7717a..d5e9f20a 100644 --- a/tasks/opencode_smoke_test.yaml +++ b/tasks/opencode_smoke_test.yaml @@ -1,7 +1,10 @@ task_id: "opencode_smoke_test" description: "Smoke-test the OpenCode agent harness: create and run a small Python script." initial_prompt: "Create a Python file named app.py in the current working directory that prints 'Hello, OpenCode!' on one line, and today's date in YYYY-MM-DD format on the next line. Use the datetime module. Then run the script with: python app.py" -tags: [smoke, smoke-pass, basic, pure-python, opencode] +# No `smoke-pass`: that tag routes a task into the CI E2E bucket, which runs on +# Bedrock runners with no `opencode` CLI and no OpenRouter credentials. Run this +# task locally (or in a job that installs both) instead. +tags: [smoke, basic, pure-python, opencode] run_limits: expected_turns: 5 From fcb7dadf531354432df56f4e64789c224417c437 Mon Sep 17 00:00:00 2001 From: mohsen-uipath Date: Sat, 15 Aug 2026 22:09:48 -0700 Subject: [PATCH 07/12] fix(opencode): inject plugin skills so skill suites measure the skills MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 `/.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 `/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 `/.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. --- CLAUDE.md | 2 +- docs/agents/OPENCODE.md | 60 +++++++- src/coder_eval/agents/opencode_agent.py | 169 ++++++++++++++++++++- src/coder_eval/criteria/skill_triggered.py | 6 +- tests/test_opencode_agent.py | 108 +++++++++++++ tests/test_skill_triggered.py | 13 ++ 6 files changed, 351 insertions(+), 7 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index c044eff2..76d693d0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -142,7 +142,7 @@ action.yml # Published composite GitHub Action (coder-ev - **Per-criterion aggregation**: Each `BaseCriterion` subclass exposes `aggregate(criterion, per_row_results) -> CriterionAggregate | None`. Default emits `count / mean / median / std / min / max` so every criterion is suite-thresholdable for free. Classification-style criteria return `ClassificationCriterionResult` (subclass of `CriterionResult`) and layer accuracy / P/R/F1 / confusion via the shared `overlay_classification_metrics` utility. `BaseSuccessCriterion.suite_thresholds` gates the suite on those metrics; CLI exits non-zero on any gate failure. - **Sub-agent token accounting**: There is NO separate per-sub-agent field. Every sub-agent generation is captured as a `parent_tool_use_id`-tagged `AssistantMessage` in the turn transcript, so per-sub-agent usage is derived by grouping those messages on that id (the evalboard's `aggregateSubAgentUsage` does exactly this). Claude bubbles its sub-agent's intermediate generations into the parent stream natively, and the **terminal** generation (delivered as the Agent tool result, never streamed) is synthesized into one via `_synthesize_subagent_terminal_message` from `tool_use_result.usage`. Codex reconstructs all child generations from the child rollout (`_recover_subagent_tool_calls`). The turn total already includes sub-agent cost — Claude via the SDK's cumulative `model_usage`; Codex via `_fold_subagent_tokens`, which folds the child messages (their real per-generation tokens) into the parent total. `CommandTelemetry.result_summary` is stored **untruncated** (no 200-char cap) so sub-agent returns are preserved whole. Set `CODER_EVAL_RAW_SDK_LOG=1` to dump every raw SDK event to the task log for inspection. - **Reconciliation message (stream self-reconciles to the turn total)**: The per-message stream consistently under-reports the authoritative turn total — a fixed prompt slice (~512 input tokens on Claude) is billed on no SDK-emitted message, and sub-agent input/cache only partially bubbles up. So `EventCollector.build_turn_record` appends one synthetic `ReconciliationMessage` (`role="reconciliation"`, in the `TranscriptMessage` union) per turn, carrying the per-bucket residual = `token_usage` − Σ(assistant message buckets). The invariant: **summing the four token buckets across `TurnRecord.messages` (assistant + reconciliation) equals `token_usage` exactly**, for both Claude and Codex (Codex's stream is already complete after `_recover_subagent_tool_calls`, so its residual is usually 0 and no entry is emitted). This is what lets the evalboard SUM the message stream as the source of truth instead of reading a separate aggregate ("agent tokens"): `selectTokenTotals` returns the stream sum whenever a reconciliation entry is present, and the timeline renders it as its own row. It is agent-agnostic (booked at the single `EventCollector` seam), carries no cost (cost stays on `token_usage`), and is excluded from generation/turn counts and the cost simulator. The LiteLLM open-weight actual-cost join (`litellm_cost.apply_actual_cost`) deliberately writes cost at the TURN level only (`token_usage.total_cost_usd` = the real OpenRouter bill) plus the per-call `TurnRecord.provider_call_costs` audit record; it does NOT touch the message token buckets, so `EventCollector` stays the single writer and this invariant holds on every backend. The Python `token_usage`/`total_token_usage` aggregate is unchanged and still authoritative for budget/judges/reports. -- **Harness run-limit parity**: a shared `BaseAgentConfig` field must mean the same thing on every backend, so a divergence is either fixed or documented — never silent. **`run_limits.max_turns` on Codex/Antigravity counts VISIBLE turns** (resolved tool calls, read live off the shared `EventCollector.visible_turn_count`, the same list `TurnRecord.commands` holds) because one `communicate()` is a single SDK turn on both, so a native counter would clamp at 1; claude-code keeps its native SDK cap, whose unit (an agent-loop turn) absorbs arbitrarily many parallel calls — the same number is NOT the same budget across harnesses. OpenCode likewise keeps a native unit — the CLI streams a real multi-step loop per `communicate()` (`step_start`/`step_finish`), so `max_turns: N` allows N complete steps and cuts cleanly when step N+1 begins. The cap is enforced on the same loop boundary as the cooperative early stop and finalizes cleanly as `max_turns_exhausted` (no crash, no retry); on Antigravity that boundary lives in `_drain()`, so the background-work poll loop honors it too. Known unfixed divergences: `permission_mode` on Codex and Antigravity (both run unconfined — the sandbox driver is the isolation boundary), `disallowed_tools` on Codex (forwarded, not SDK-enforced), `allowed_tools`/`disallowed_tools` on Antigravity (not read at all), `turn_timeout` on Antigravity (bounded by an earlier internal poll deadline at 80% of it), and `allowed_tools`/`disallowed_tools`/`system_prompt`/`plugins` on OpenCode (no CLI knob; warned at `start()`, not enforced). Full table + rationale: docs/agents/HARNESS_PARITY.md. +- **Harness run-limit parity**: a shared `BaseAgentConfig` field must mean the same thing on every backend, so a divergence is either fixed or documented — never silent. **`run_limits.max_turns` on Codex/Antigravity counts VISIBLE turns** (resolved tool calls, read live off the shared `EventCollector.visible_turn_count`, the same list `TurnRecord.commands` holds) because one `communicate()` is a single SDK turn on both, so a native counter would clamp at 1; claude-code keeps its native SDK cap, whose unit (an agent-loop turn) absorbs arbitrarily many parallel calls — the same number is NOT the same budget across harnesses. OpenCode likewise keeps a native unit — the CLI streams a real multi-step loop per `communicate()` (`step_start`/`step_finish`), so `max_turns: N` allows N complete steps and cuts cleanly when step N+1 begins. The cap is enforced on the same loop boundary as the cooperative early stop and finalizes cleanly as `max_turns_exhausted` (no crash, no retry); on Antigravity that boundary lives in `_drain()`, so the background-work poll loop honors it too. Known unfixed divergences: `permission_mode` on Codex and Antigravity (both run unconfined — the sandbox driver is the isolation boundary), `disallowed_tools` on Codex (forwarded, not SDK-enforced), `allowed_tools`/`disallowed_tools` on Antigravity (not read at all), `turn_timeout` on Antigravity (bounded by an earlier internal poll deadline at 80% of it), and `allowed_tools`/`disallowed_tools`/`system_prompt` on OpenCode (no CLI knob; warned at `start()`, not enforced). `plugins` on OpenCode is **honored for skills**: each local plugin root is mapped to the `skills` dir its `.claude-plugin/plugin.json` declares (default `/skills`, never the root itself — `skills.paths` is scanned recursively and a plugin root can hold a self-referential symlink) and injected via `OPENCODE_CONFIG_CONTENT`, which `--pure` does not suppress; a plugin's agents/hooks/commands/MCP servers are still dropped. Full table + rationale: docs/agents/HARNESS_PARITY.md. - **sandbox isolation**: Tasks that don't need MCP servers should set `setting_sources: []` in their `agent:` block to isolate the sandbox from the host project's CLAUDE.md and settings. Without this, the host project's CLAUDE.md (often 20 KB+) is injected into every API call, inflating cache-creation tokens and cost significantly. - **Run-time caps (non-criterion enforcement)**: `TaskDefinition.run_limits` (`RunLimits` model) is the single namespace for all *task-level* run-time caps — `max_turns` / `task_timeout` / `turn_timeout` (structural) and `max_input_tokens` / `max_output_tokens` / `max_total_tokens` / `max_usd` (cumulative budget). Token/USD breaches abort with `FinalStatus.TOKEN_BUDGET_EXCEEDED` or `COST_BUDGET_EXCEEDED` (both `category == "failed"`). Structural caps are set from the CLI via `-D run_limits.max_turns=…` / `-D run_limits.task_timeout=…` / `-D run_limits.turn_timeout=…` (field-merged into `run_limits`); budget caps via `-D run_limits.max_usd=…` etc. or YAML. Layered config uses field-merge — a variant block overrides individual keys without replacing the task's block. The one *per-criterion* cap, `stop_early.decide_within`, deliberately lives on `LiveSuccessCriterion` instead (see below) — the watcher must attribute a decision-step timeout to a specific criterion, which `RunLimits` (task-scoped, criterion-agnostic) cannot express. - **Early stop on criterion (opt-in, per-criterion arming)**: a `stop_early:` block (`StopEarlyPolicy`) on a criterion ends a single-shot run early once the run's **armed** criteria decide the outcome, so a raised `max_turns` isn't wasted on the smoke flavor. The block's PRESENCE is the arming and alone activates the watcher — there is **no run-level master switch**: `run_limits.stop_early: false` is the run-level KILL SWITCH that force-disarms every block (the one-line experiment-variant/`-D` override for an authoritative full run), and `run_limits.stop_early: true` (the removed master arm) is a hard `EarlyStopConfigError` at resolution. The block exists on `LiveSuccessCriterion` only (currently `skill_triggered`, `command_executed` — so arming an unobservable criterion is unrepresentable, a pydantic extra-forbid error). Arming carries one implicit trigger (a native live-fail may fail-stop the run); its keys refine it: `on_pass: stop` (pass-stop the moment the criterion live-passes; default `continue` just latches) and `decide_within: N` (still undecided after N tool-call steps latches an **effective fail**, fed through the same fail-stop rule, reported as `decision_budget_exceeded` — an ordinary weighted fail, NOT a gate-bypassing force-fail; cumulative across retry attempts of the same turn). A trigger whose polarity the instance can't decide (per the abstract, checker-independent `live_decidable_polarities()`, a pure function of the criterion's own fields, paired with the checker's `live_verdict` override by lint rule CE025, a registry-based whole-tree check) is **inert by design** — one dataset-fanned YAML line serves both positive rows (pass/timeout live) and distractor rows (fail live). Verdicts **latch**: once a criterion decides, its `live_verdict` is never polled again. Stop rule is weighted, not strict-boolean: `run_limits.stop_early_gate_threshold` (default `1.0`, reproducing strict-AND behavior exactly) is the minimum weighted score (`Σ weight·score / Σ weight` over the armed subset) required to pass; a fail-stop fires once the armed set's **ceiling** (best case for everything still undecided) can no longer reach the threshold — so a low-weight fail or timeout that can't doom the gate is absorbed and the run continues — and is **deferred while any pass-capable armed criterion is undecided** (a distractor misfire never truncates a positive row's recall signal); a pass-stop fires once the `on_pass: stop` subset's **floor** (worst case) already meets the threshold, and is symmetrically **deferred while any pass-capable armed criterion outside the `on_pass: stop` subset is undecided** (so an early pass never freezes a sibling `on_pass: continue` criterion's signal out of the trajectory). A fail-stop is therefore verdict-preserving; a pass-stop can miss a *later* distractor misfire, so authoritative P/R/F1 comes from a kill-switched (`stop_early: false`) run. Driven by `orchestration/early_stop.py::EarlyStopWatcher` (built when `early_stop_active(task)`: ≥1 armed criterion, kill switch not thrown) through the agent's cooperative `should_stop` seam (tool-call granularity, no SIGKILL); live verdicts only *trigger* the stop — the standard `check_all_async` on the frozen trajectory is authoritative. Gating is **FIRED-ONLY**: a run the watcher actually cut gates on the **armed subset** via the weighted `EvaluationResult.armed_criteria_passed`; a run that completes naturally — armed or not — gates strict-AND via `all_criteria_passed`, so adding a block never changes the verdict of a run it didn't cut. Note the gate keys on the watcher having FIRED (`result.early_stop is not None`), not on confirmed truncation — an agent that ignores `should_stop`, or a stop firing on the final message, still gates armed-only. Every resolution-time guardrail violation is a hard error at resolution (plan *and* run); the one load-time case — a `stop_early:` block on a non-live criterion — is a pydantic schema error at task load, which the run surface reports as a skipped task like any other malformed task. A runtime verdict bug **fails open** to a full run. Surfaces: `EarlyStopInfo` (incl. `gate_threshold` at stop time), report notes/badges, `stopped_early` run.json rows, `EarlyStopped`/`EarlyStopReason` telemetry dims. Worked rationale: docs/TASK_DEFINITION_GUIDE.md § `stop_early`. No blocks anywhere ⇒ behavior byte-for-byte unchanged. diff --git a/docs/agents/OPENCODE.md b/docs/agents/OPENCODE.md index 74e74c07..b098a613 100644 --- a/docs/agents/OPENCODE.md +++ b/docs/agents/OPENCODE.md @@ -118,6 +118,56 @@ Defaults to `true`, forwarding `--pure` so the sandbox is isolated from host-lev OpenCode plugin configuration. This mirrors the rationale behind the Claude agent's `setting_sources: []`. Set to `false` to load host plugins deliberately. +`--pure` skips external *plugins*; it does **not** skip configured skill paths, so +skill injection (below) works under the default `pure: true`. + +### `plugins` — skill injection + +A `plugins:` entry is a Claude-plugin root, which is how a task ships the skills +under test: + +```yaml +agent: + plugins: + - type: "local" + path: "$SKILLS_REPO_PATH" +``` + +OpenCode has no plugin knob, but it does load skills from `skills.paths` in its +config, so each local plugin root is mapped to that: + +1. `/.claude-plugin/plugin.json` is read for its `skills` field (a string or + a list, each relative to the root); Claude Code reads the same field, so one + `plugins:` line means the same thing on both harnesses. +2. Absent a manifest, the convention default `/skills` is used. +3. A path that is already a bare skills directory (`//SKILL.md`, no + `skills/` subdir) is used as-is. + +The resulting directories are passed through `OPENCODE_CONFIG_CONTENT`, which the +CLI merges as a final local-scope config layer. That seam was chosen over writing +`/.opencode/skills/` because it writes nothing into the sandbox that is +later preserved as a run artifact and inspected by file criteria, and because it +does not depend on how the CLI resolves a project root from `--dir`. An inherited +`OPENCODE_CONFIG_CONTENT` is merged into, not clobbered. With no `plugins:` entry +the variable is left exactly as inherited. + +> A plugin root is mapped to its *skills subdirectory*, never to the root itself +> when one exists. `skills.paths` is scanned **recursively**, and a plugin root can +> contain a self-referential symlink (`UiPath/skills` has `plugins/uipath -> ..`), +> which resolves skills through an arbitrary path and silently drops duplicate names. + +Verify what the agent will actually see, using the same environment it builds: + +```bash +opencode debug skill --pure # lists every skill the CLI can load +``` + +Every way this can resolve to nothing — an unset `$SKILLS_REPO_PATH`, a missing +directory, a root with no `SKILL.md` under it — is logged as a warning at `start()`, +and the resolved paths are recorded per task under `environment_info` +(`opencode_skill_paths`). A run that quietly measures the bare model instead of the +skills under test otherwise looks entirely normal. + ## Permissions Every `permission_mode` except `plan` passes `--auto`, auto-approving tool use. @@ -189,12 +239,14 @@ with an error naming the unrecognized event types it saw instead. ## Known limitations -- **`allowed_tools` / `disallowed_tools` / `system_prompt` / `system_prompt_file` / - `plugins` are not enforced.** The CLI exposes no equivalent knob, so these are +- **`allowed_tools` / `disallowed_tools` / `system_prompt` / `system_prompt_file` + are not enforced.** The CLI exposes no equivalent knob, so these are dropped — `start()` logs a warning naming each one it saw (`experiments/default.yaml` sets `allowed_tools` on every task, so expect it on a default run). Do not rely on - them as a boundary here, and note that skill-injection suites, which depend on - `plugins`, cannot run on this harness. + them as a boundary here. +- **Only the *skills* half of a `plugins:` entry is honored** (see below). A Claude + plugin's agents, hooks, commands and MCP servers have no OpenCode equivalent and + are still dropped. - **`max_turns` counts OpenCode's native steps.** One step = one assistant generation (`step_start`/`step_finish`) and may carry several tool calls; `max_turns: N` allows N complete steps, then the run finalizes cleanly as diff --git a/src/coder_eval/agents/opencode_agent.py b/src/coder_eval/agents/opencode_agent.py index a6b4ca6c..04dd2363 100644 --- a/src/coder_eval/agents/opencode_agent.py +++ b/src/coder_eval/agents/opencode_agent.py @@ -40,6 +40,7 @@ import time from collections.abc import Callable from datetime import datetime +from pathlib import Path from typing import Any, ClassVar, Literal, NoReturn from coder_eval.agent import Agent @@ -74,6 +75,7 @@ TurnEndStatus, TurnStartEvent, ) +from coder_eval.utils import expand_env_vars from .registry import AgentRegistry @@ -137,19 +139,47 @@ "todowrite": "TodoWrite", "todoread": "TodoRead", "task": "Agent", + # OpenCode's native skill loader. Without this entry `skill_triggered` (which + # keys on the canonical `Skill`) and any `command_executed` written against + # `tool_name: Skill` read false on every OpenCode run — the engagement happened + # but no criterion could see it. + "skill": "Skill", } # Config fields the OpenCode CLI has no equivalent knob for. `experiments/default.yaml` # sets `allowed_tools` on every task, so these are silently dropped by default — # warn once at start() rather than letting a task believe it constrained the agent. +# `plugins` is NOT here: its skills half is honored via _plugin_skill_dirs below. _UNSUPPORTED_CONFIG_FIELDS: tuple[str, ...] = ( "system_prompt", "system_prompt_file", "allowed_tools", "disallowed_tools", - "plugins", ) +# --- skill injection ------------------------------------------------------ +# +# A `plugins:` entry is a Claude-plugin root. Claude Code reads its skills from +# the `skills` field of `/.claude-plugin/plugin.json` (conventionally +# `./skills/`). OpenCode has no plugin knob, but it does load skills from +# `skills.paths` in its config — so mapping the plugin root to that directory is +# what makes one `plugins:` line mean the same thing on both harnesses. +# +# The config is handed over through OPENCODE_CONFIG_CONTENT, which OpenCode +# merges as a final local-scope layer. That was chosen over writing +# `/.opencode/skills/` because it (a) writes nothing into the sandbox +# that is later preserved as run artifacts and inspected by file criteria, and +# (b) does not depend on how the CLI resolves a project root from `--dir`. +# Verified orthogonal to `--pure`, which skips external *plugins*, not +# configured skill paths. +# +# Only the skills half of a plugin is honored. A Claude plugin's agents, hooks, +# commands and MCP servers have no OpenCode equivalent and are still dropped. +_CONFIG_CONTENT_ENV = "OPENCODE_CONFIG_CONTENT" +_PLUGIN_MANIFEST_RELPATH = (".claude-plugin", "plugin.json") +_DEFAULT_PLUGIN_SKILLS_SUBDIR = "skills" +_SKILL_FILE = "SKILL.md" + # ToolEndStatus -> CommandTelemetry.result_status (the persisted tri-state). _RESULT_STATUS: dict[ToolEndStatus, Literal["success", "error", "unknown"]] = { ToolEndStatus.OK: "success", @@ -188,6 +218,86 @@ def _epoch_ms_to_dt(value: Any) -> datetime | None: return None +def _manifest_skill_dirs(root: Path) -> list[Path]: + """Skill directories a Claude-plugin root declares, in manifest order. + + Reads the ``skills`` field of ``/.claude-plugin/plugin.json`` (a string + or a list of strings, each relative to the root) and falls back to the + convention default ``/skills`` when the manifest is absent, unreadable, + or declares none. Honoring the manifest rather than hardcoding ``skills/`` + keeps a plugin that relocates its skills working on both harnesses. + """ + manifest = root.joinpath(*_PLUGIN_MANIFEST_RELPATH) + declared: list[str] = [] + if manifest.is_file(): + try: + data: Any = json.loads(manifest.read_text(encoding="utf-8")) + except (OSError, UnicodeDecodeError, json.JSONDecodeError): + data = None + if isinstance(data, dict): + value = data.get("skills") + if isinstance(value, str): + declared = [value] + elif isinstance(value, list): + declared = [entry for entry in value if isinstance(entry, str)] + if not declared: + declared = [_DEFAULT_PLUGIN_SKILLS_SUBDIR] + return [(root / relative).resolve() for relative in declared] + + +def _plugin_skill_dirs( + plugins: list[dict[str, Any]] | None, + log: logging.Logger | logging.LoggerAdapter[Any] = logger, +) -> list[str]: + """Resolve ``plugins:`` entries to OpenCode ``skills.paths`` directories. + + Every way this can come up empty is logged rather than passed over: a plugin + whose skills never reach the agent still *looks* like a normal run, which is + precisely the failure this function exists to close. + """ + resolved: list[str] = [] + for plugin in plugins or []: + if not isinstance(plugin, dict) or plugin.get("type") != "local": + log.warning("opencode: ignoring non-local plugin entry %r — only `type: local` maps to skills.", plugin) + continue + path_str = plugin.get("path") + if not path_str: + continue + expanded = expand_env_vars(str(path_str)) + root = Path(expanded).resolve() + if not root.is_dir(): + hint = "env var likely unset" if "$" in expanded else "path does not exist" + log.warning( + "opencode: plugin skills path did not resolve: %r -> %r (%s); no skills injected from it", + path_str, + expanded, + hint, + ) + continue + candidates = [directory for directory in _manifest_skill_dirs(root) if directory.is_dir()] + # A path that is ALREADY a bare skills directory (//SKILL.md) + # has no `skills/` subdir, so use it as-is. Deliberately not a fallback for + # a root that HAS one: `skills.paths` is scanned recursively and a repo + # root can contain self-referential symlinks (UiPath/skills has + # `plugins/uipath -> ..`), which resolves skills through an arbitrary path + # and silently drops duplicate names. + if not candidates: + candidates = [root] + for directory in candidates: + if next(directory.glob(f"*/{_SKILL_FILE}"), None) is None: + log.warning( + "opencode: no /%s directly under %s (from plugin %r) — the CLI still scans it " + + "recursively, but check the plugin path points at a skills root", + _SKILL_FILE, + directory, + path_str, + ) + as_text = str(directory) + if as_text not in resolved: + resolved.append(as_text) + return resolved + + class _OpenCodeTurnState: """Per-``communicate()`` accumulator: events in, finalization payload out. @@ -638,6 +748,7 @@ def __init__( self.working_directory: str | None = None self._env_path_prepend: list[str] = [] self._plugin_tools_dir: str | None = None + self._skill_dirs: list[str] = [] self._session_id: str | None = None self._process: asyncio.subprocess.Process | None = None # Process-group ids (== the CLI's pid under start_new_session) of every @@ -668,6 +779,22 @@ async def start( + "unconstrained by them; do not rely on them as a boundary (see docs/agents/OPENCODE.md).", ", ".join(ignored), ) + self._skill_dirs = _plugin_skill_dirs(self.config.plugins, log=logger) # type: ignore[arg-type] + if self._skill_dirs: + logger.info( + "opencode: injecting %d skill path(s) via %s: %s", + len(self._skill_dirs), + _CONFIG_CONTENT_ENV, + self._skill_dirs, + ) + elif self.config.plugins: + # Plugins were declared but produced nothing — the run is about to + # measure the model without the skills under test. Say so loudly. + logger.warning( + "opencode: %d plugin(s) declared but 0 skill path(s) resolved — the agent will run " + + "WITHOUT them (see docs/agents/OPENCODE.md).", + len(self.config.plugins), + ) self.working_directory = working_directory self._env_path_prepend = list(env_path_prepend or []) self._plugin_tools_dir = plugin_tools_dir @@ -718,6 +845,10 @@ def _sweep_process_groups(self) -> None: def get_environment_info(self) -> dict[str, Any]: info: dict[str, Any] = {"opencode_model": self.config.model, "opencode_pure": self.config.pure} + if self._skill_dirs: + # Recorded per task so a run's report can be checked for whether the + # skills under test actually reached the agent. + info["opencode_skill_paths"] = list(self._skill_dirs) if self.config.variant: info["opencode_variant"] = self.config.variant if self._session_id: @@ -752,8 +883,44 @@ def _build_env(self) -> dict[str, str]: 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 + self._inject_skill_paths(env) return env + def _inject_skill_paths(self, env: dict[str, str]) -> None: + """Merge the resolved skill directories into ``OPENCODE_CONFIG_CONTENT``. + + No plugins means the variable is left exactly as inherited, so a run + without a ``plugins:`` block behaves byte-for-byte as before. An inherited + value is preserved and appended to rather than clobbered, since the host + may legitimately configure OpenCode through the same seam. + """ + if not self._skill_dirs: + return + config: dict[str, Any] = {} + inherited = env.get(_CONFIG_CONTENT_ENV) + if inherited: + try: + parsed = json.loads(inherited) + except json.JSONDecodeError: + logger.warning( + "opencode: inherited %s is not valid JSON; replacing it with the injected skill paths.", + _CONFIG_CONTENT_ENV, + ) + else: + if isinstance(parsed, dict): + config = parsed + else: + logger.warning( + "opencode: inherited %s is not a JSON object; replacing it with the injected skill paths.", + _CONFIG_CONTENT_ENV, + ) + skills = config.get("skills") + skills = dict(skills) if isinstance(skills, dict) else {} + existing = [path for path in skills.get("paths", []) if isinstance(path, str)] + skills["paths"] = existing + [path for path in self._skill_dirs if path not in existing] + config["skills"] = skills + env[_CONFIG_CONTENT_ENV] = json.dumps(config) + # --- the turn ---------------------------------------------------------- async def communicate( diff --git a/src/coder_eval/criteria/skill_triggered.py b/src/coder_eval/criteria/skill_triggered.py index 2b1ded9c..5a309756 100644 --- a/src/coder_eval/criteria/skill_triggered.py +++ b/src/coder_eval/criteria/skill_triggered.py @@ -63,7 +63,11 @@ def _engaged_skill_names(cmd: CommandTelemetry) -> set[str]: """ names: set[str] = set() if cmd.tool_name == "Skill": - skill = cmd.parameters.get("skill", "") + # Claude names the parameter `skill`; OpenCode's native skill tool names + # the same value `name` (its raw tool name is lowercase `skill`, mapped to + # the canonical `Skill` by the OpenCode agent). Read both so one criterion + # scores a skill engagement identically on either harness. + skill = cmd.parameters.get("skill") or cmd.parameters.get("name") or "" if isinstance(skill, str) and skill: names.add(skill.split(":")[-1]) for value in cmd.parameters.values(): diff --git a/tests/test_opencode_agent.py b/tests/test_opencode_agent.py index a6daa868..d2b40393 100644 --- a/tests/test_opencode_agent.py +++ b/tests/test_opencode_agent.py @@ -466,6 +466,114 @@ async def test_unknown_tool_passes_through(self, patch_exec, tmp_path): record = await _run(_agent(), tmp_path) assert [c.tool_name for c in record.commands] == ["some_new_tool"] + async def test_native_skill_tool_maps_to_canonical_skill(self, patch_exec, tmp_path): + """`skill_triggered` keys on the canonical `Skill`; OpenCode emits lowercase + `skill`, so without the mapping a real engagement scores as a miss.""" + patch_exec(_FakeProcess([self._tool_event("skill")])) + record = await _run(_agent(), tmp_path) + assert [c.tool_name for c in record.commands] == ["Skill"] + + +def _skill_repo(root, names=("uipath-admin",), *, manifest: str | None = None, nested: bool = True): + """Build a plugin root on disk; returns it. + + ``nested`` mirrors the Claude-plugin layout (``/skills//SKILL.md``); + False makes ``root`` itself a bare skills directory. + """ + base = root / "skills" if nested else root + for name in names: + (base / name).mkdir(parents=True, exist_ok=True) + (base / name / "SKILL.md").write_text(f"---\nname: {name}\ndescription: d\n---\n", encoding="utf-8") + if manifest is not None: + (root / ".claude-plugin").mkdir(parents=True, exist_ok=True) + (root / ".claude-plugin" / "plugin.json").write_text(manifest, encoding="utf-8") + return root + + +def _injected_skill_paths(captured) -> list[str]: + raw = captured["kwargs"]["env"].get("OPENCODE_CONFIG_CONTENT") + return [] if raw is None else json.loads(raw)["skills"]["paths"] + + +class TestSkillInjection: + """`plugins:` is how a task ships the skills under test. + + OpenCode has no plugin knob, so before this mapping existed every skill-injection + run silently measured the bare model instead — a run that looks entirely normal. + """ + + async def test_manifest_declared_skills_dir_is_used(self, patch_exec, tmp_path): + root = _skill_repo(tmp_path / "plug", manifest='{"name": "uipath", "skills": "./skills/"}') + captured = patch_exec(_FakeProcess(HAPPY_STREAM)) + await _run(_agent(plugins=[{"type": "local", "path": str(root)}]), tmp_path / "sandbox") + assert _injected_skill_paths(captured) == [str(root / "skills")] + + async def test_default_layout_without_a_manifest(self, patch_exec, tmp_path): + root = _skill_repo(tmp_path / "plug") + captured = patch_exec(_FakeProcess(HAPPY_STREAM)) + await _run(_agent(plugins=[{"type": "local", "path": str(root)}]), tmp_path / "sandbox") + assert _injected_skill_paths(captured) == [str(root / "skills")] + + async def test_bare_skills_directory_is_used_as_is(self, patch_exec, tmp_path): + root = _skill_repo(tmp_path / "bare", nested=False) + captured = patch_exec(_FakeProcess(HAPPY_STREAM)) + await _run(_agent(plugins=[{"type": "local", "path": str(root)}]), tmp_path / "sandbox") + assert _injected_skill_paths(captured) == [str(root)] + + async def test_plugin_root_is_never_added_alongside_its_skills_dir(self, patch_exec, tmp_path): + """`skills.paths` is scanned RECURSIVELY. A plugin root can contain a + self-referential symlink (UiPath/skills has `plugins/uipath -> ..`), which + resolves skills through an arbitrary path and drops duplicate names.""" + root = _skill_repo(tmp_path / "plug") + (root / "plugins").mkdir() + (root / "plugins" / "self").symlink_to(root, target_is_directory=True) + captured = patch_exec(_FakeProcess(HAPPY_STREAM)) + await _run(_agent(plugins=[{"type": "local", "path": str(root)}]), tmp_path / "sandbox") + assert _injected_skill_paths(captured) == [str(root / "skills")] + + async def test_env_untouched_when_no_plugins_declared(self, patch_exec, tmp_path): + """A run without `plugins:` must behave byte-for-byte as before.""" + captured = patch_exec(_FakeProcess(HAPPY_STREAM)) + await _run(_agent(), tmp_path) + assert "OPENCODE_CONFIG_CONTENT" not in captured["kwargs"]["env"] + + async def test_inherited_config_content_is_merged_not_clobbered(self, patch_exec, tmp_path, monkeypatch): + root = _skill_repo(tmp_path / "plug") + monkeypatch.setenv( + "OPENCODE_CONFIG_CONTENT", + json.dumps({"username": "host", "skills": {"paths": ["/host/skills"]}}), + ) + captured = patch_exec(_FakeProcess(HAPPY_STREAM)) + await _run(_agent(plugins=[{"type": "local", "path": str(root)}]), tmp_path / "sandbox") + + config = json.loads(captured["kwargs"]["env"]["OPENCODE_CONFIG_CONTENT"]) + assert config["username"] == "host" + assert config["skills"]["paths"] == ["/host/skills", str(root / "skills")] + + async def test_unresolved_path_warns_and_injects_nothing(self, patch_exec, tmp_path, caplog): + """An unset `$SKILLS_REPO_PATH` is the exact shape of the original defect.""" + patch_exec(_FakeProcess(HAPPY_STREAM)) + agent = _agent(plugins=[{"type": "local", "path": "$DEFINITELY_UNSET_REPO/skills"}]) + with caplog.at_level("WARNING"): + await agent.start(str(tmp_path)) + assert agent._skill_dirs == [] + assert "env var likely unset" in caplog.text + assert "0 skill path(s) resolved" in caplog.text + + async def test_plugins_are_no_longer_announced_as_unenforced(self, patch_exec, tmp_path, caplog): + root = _skill_repo(tmp_path / "plug") + patch_exec(_FakeProcess(HAPPY_STREAM)) + with caplog.at_level("WARNING"): + await _agent(plugins=[{"type": "local", "path": str(root)}]).start(str(tmp_path / "sandbox")) + assert "NOT enforced" not in caplog.text + + async def test_resolved_paths_are_recorded_for_audit(self, patch_exec, tmp_path): + root = _skill_repo(tmp_path / "plug") + patch_exec(_FakeProcess(HAPPY_STREAM)) + agent = _agent(plugins=[{"type": "local", "path": str(root)}]) + await agent.start(str(tmp_path / "sandbox")) + assert agent.get_environment_info()["opencode_skill_paths"] == [str(root / "skills")] + class TestUnsupportedConfigIsAnnounced: async def test_start_warns_about_unenforced_fields(self, patch_exec, tmp_path, caplog): diff --git a/tests/test_skill_triggered.py b/tests/test_skill_triggered.py index a856cde5..de6ea57e 100644 --- a/tests/test_skill_triggered.py +++ b/tests/test_skill_triggered.py @@ -50,6 +50,19 @@ def test_skill_invoked_tp(self) -> None: ) assert result.score == 1.0 and result.observed_label == "yes" and result.expected_label == "yes" + def test_opencode_name_parameter_counts_as_engagement(self) -> None: + """OpenCode's native skill tool carries the skill under `name`, not `skill`. + Reading only `skill` scored every OpenCode engagement as a miss.""" + result = _check( + expected_skill="uipath-flow", skill_name="uipath-flow", commands=[_cmd("Skill", {"name": "uipath-flow"})] + ) + assert result.score == 1.0 and result.observed_label == "yes" + + def test_opencode_name_parameter_is_scoped_to_the_skill_tool(self) -> None: + """`name` is a generic parameter; only a `Skill` call may be read that way.""" + result = _check(expected_skill="", skill_name="uipath-flow", commands=[_cmd("Write", {"name": "uipath-flow"})]) + assert result.score == 1.0 and result.observed_label == "no" + def test_no_skill_tn(self) -> None: result = _check(expected_skill="", skill_name="uipath-flow", commands=[_cmd("Read", {"file_path": "x"})]) assert result.score == 1.0 and result.observed_label == "no" and result.expected_label == "no" From 7b5cf679224bef684831aacd20181983cf3bc812 Mon Sep 17 00:00:00 2001 From: mohsen-uipath Date: Mon, 17 Aug 2026 10:03:37 -0700 Subject: [PATCH 08/12] fix(opencode): reap the CLI on every turn exit, test the sandbox env contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --session ` 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). --- src/coder_eval/agents/opencode_agent.py | 51 ++++++++ tests/test_opencode_agent.py | 154 ++++++++++++++++++++++++ 2 files changed, 205 insertions(+) diff --git a/src/coder_eval/agents/opencode_agent.py b/src/coder_eval/agents/opencode_agent.py index 04dd2363..48d7ce4a 100644 --- a/src/coder_eval/agents/opencode_agent.py +++ b/src/coder_eval/agents/opencode_agent.py @@ -878,6 +878,21 @@ def _build_argv(self, user_input: str) -> list[str]: return argv def _build_env(self) -> dict[str, str]: + """The CLI's full environment: the host's, plus the sandbox's contributions. + + The PATH prepend is the mock-shadowing contract (``Agent.start``): the + sandbox's mock CLI directories must resolve BEFORE the real binaries, in + the order given, or a task grading a mocked CLI silently exercises the + real one. ``PLUGIN_TOOLS_DIR`` is advisory and never overrides an + inherited value. + + Unlike ``CodexAgent._build_codex_env`` — which hands the SDK a partial + dict merged over the real environment, and so must resolve the PATH key + case-insensitively — this returns the WHOLE environment, seeded from + ``os.environ``, whose keys CPython upper-cases on Windows (``os.py``'s + ``encodekey``). ``"PATH"`` is therefore the inherited key on every + platform and cannot duplicate a differently-cased one. + """ env = dict(os.environ) if self._env_path_prepend: env["PATH"] = os.pathsep.join([*self._env_path_prepend, env.get("PATH", "")]) @@ -963,6 +978,10 @@ def emit(event: StreamEvent) -> None: deadline = None if timeout is None else time.monotonic() + timeout stopped_early = False stderr_drain: asyncio.Future[bytes] | None = None + # Bound OUTSIDE the try so the teardown in `finally` can tell "never + # spawned" (a create_subprocess_exec failure) from "spawned and possibly + # still running". + proc: asyncio.subprocess.Process | None = None try: proc = await asyncio.create_subprocess_exec( *self._build_argv(user_input), @@ -1083,8 +1102,40 @@ def emit(event: StreamEvent) -> None: finally: if stderr_drain is not None: stderr_drain.cancel() + self._reap_orphaned_cli(proc) self._process = None + def _reap_orphaned_cli(self, proc: asyncio.subprocess.Process | None) -> None: + """Kill a CLI that is still running as the turn unwinds. No-op otherwise. + + Two exits from :meth:`communicate` reach its ``finally`` with the child + ALIVE: the ``except Exception`` crash (a ``StreamReader`` ``ValueError`` + on an over-long line, a malformed-payload ``TypeError`` in a handler) and + an external cancellation — neither passes through the graceful + ``await self.kill()`` that the intentional cuts and ``_settle_turn`` use. + + Abandoning it 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 would spawn a SECOND + ``opencode --dir --session `` while attempt 1 is still + editing the files the criteria are about to score — and whichever writer + won would decide the task's result. ``docker_runner`` kills its container + from ``finally`` for the same reason. + + Deliberately synchronous. This runs while a ``CancelledError`` is + propagating, where any await can itself be cut short and leave the child + alive after all; ``Process.kill()`` and the group sweep deliver their + signals with no suspension point. Skipping the SIGTERM courtesy is right + for a turn that is already lost — the graceful escalation in :meth:`kill` + still owns every path that has something left to flush. ``proc`` is + ``None`` when the spawn itself failed, i.e. there is nothing to reap. + """ + if proc is None or proc.returncode is not None: + return + with contextlib.suppress(ProcessLookupError, PermissionError): + proc.kill() + self._sweep_process_groups() + async def _settle_turn( self, proc: asyncio.subprocess.Process, diff --git a/tests/test_opencode_agent.py b/tests/test_opencode_agent.py index d2b40393..5806943b 100644 --- a/tests/test_opencode_agent.py +++ b/tests/test_opencode_agent.py @@ -109,6 +109,7 @@ def __init__(self, lines: list[str], returncode: int = 0, stderr: bytes = b"") - self._stderr = stderr self.pid = 4242 self.terminated = False + self.killed = False self.stdout = self async def readline(self) -> bytes: @@ -129,6 +130,7 @@ def terminate(self) -> None: self.returncode = self._final_returncode def kill(self) -> None: + self.killed = True self.returncode = self._final_returncode @@ -154,6 +156,7 @@ def terminate(self) -> None: self._exited.set() def kill(self) -> None: + self.killed = True self._exited.set() @@ -474,6 +477,68 @@ async def test_native_skill_tool_maps_to_canonical_skill(self, patch_exec, tmp_p assert [c.tool_name for c in record.commands] == ["Skill"] +class TestSandboxEnvironment: + """`start(env_path_prepend=..., plugin_tools_dir=...)` is the abstract + `Agent.start()` contract, and the orchestrator ALWAYS supplies both. + + The PATH prepend is the mock-shadowing contract: a task grading a mocked CLI + (`cli_called`, invocation-log criteria) only works if the sandbox's mock + directories resolve BEFORE the real binaries. An inverted join order leaves + the real binary in front, so the mock writes no invocation log and every row + scores 0 — with the whole suite still green. It must fail here instead. + """ + + async def test_prepends_mock_dirs_ahead_of_the_inherited_path(self, patch_exec, tmp_path, monkeypatch): + """The dirs land at the FRONT of PATH, in order, with the parent appended.""" + monkeypatch.setenv("PATH", "/parent/bin") + captured = patch_exec(_FakeProcess(HAPPY_STREAM)) + agent = _agent() + await agent.start(str(tmp_path), env_path_prepend=["/sandbox/mocks", "/sandbox/bins"]) + await agent.communicate("do the thing") + + expected = os.pathsep.join(["/sandbox/mocks", "/sandbox/bins", "/parent/bin"]) + assert captured["kwargs"]["env"]["PATH"] == expected + + async def test_plugin_tools_dir_is_exported(self, patch_exec, tmp_path, monkeypatch): + monkeypatch.delenv("PLUGIN_TOOLS_DIR", raising=False) + captured = patch_exec(_FakeProcess(HAPPY_STREAM)) + agent = _agent() + await agent.start(str(tmp_path), plugin_tools_dir="/sandbox/tools") + await agent.communicate("do the thing") + + assert captured["kwargs"]["env"]["PLUGIN_TOOLS_DIR"] == "/sandbox/tools" + + async def test_inherited_plugin_tools_dir_wins(self, patch_exec, tmp_path, monkeypatch): + """The export is advisory: a host that already set it is never overridden.""" + monkeypatch.setenv("PLUGIN_TOOLS_DIR", "/host/tools") + captured = patch_exec(_FakeProcess(HAPPY_STREAM)) + agent = _agent() + await agent.start(str(tmp_path), plugin_tools_dir="/sandbox/tools") + await agent.communicate("do the thing") + + assert captured["kwargs"]["env"]["PLUGIN_TOOLS_DIR"] == "/host/tools" + + async def test_neither_key_is_touched_without_the_kwargs(self, patch_exec, tmp_path, monkeypatch): + """A start() with no sandbox contributions passes the environment through.""" + monkeypatch.setenv("PATH", "/parent/bin") + monkeypatch.delenv("PLUGIN_TOOLS_DIR", raising=False) + captured = patch_exec(_FakeProcess(HAPPY_STREAM)) + await _run(_agent(), tmp_path) + + env = captured["kwargs"]["env"] + assert env["PATH"] == "/parent/bin" + assert "PLUGIN_TOOLS_DIR" not in env + + async def test_the_host_environment_is_inherited_whole(self, patch_exec, tmp_path, monkeypatch): + """The CLI needs the host's provider credentials (OPENROUTER_API_KEY, ...); + this builds the full env rather than a merge dict, so nothing is dropped.""" + monkeypatch.setenv("OPENROUTER_API_KEY", "sk-test") + captured = patch_exec(_FakeProcess(HAPPY_STREAM)) + await _run(_agent(), tmp_path) + + assert captured["kwargs"]["env"]["OPENROUTER_API_KEY"] == "sk-test" + + def _skill_repo(root, names=("uipath-admin",), *, manifest: str | None = None, nested: bool = True): """Build a plugin root on disk; returns it. @@ -952,9 +1017,24 @@ def terminate(self) -> None: self._exited.set() def kill(self) -> None: + self.killed = True self._exited.set() +class _ExplodingRunningProcess(_HangingProcess): + """Raises from ``readline`` mid-stream AND stays alive, like the real CLI. + + ``_ExplodingProcess`` inherits the plain fake's ``wait()``, which reports an + exit code the instant it is awaited — so it can never model the case that + matters for teardown: the read loop dying while the CLI is still streaming. + """ + + async def readline(self) -> bytes: + if self._lines: + return self._lines.pop(0) + raise ValueError("Separator is not found, and chunk exceed the limit") + + class _EofNoExitProcess(_HangingProcess): """Replays its lines, signals EOF — but never exits until killed. @@ -1044,6 +1124,70 @@ async def test_cancel_parks_partial_and_reraises(self, patch_exec, tmp_path): assert len(ends) == 1 assert ends[0].status is AgentEndStatus.CRASHED assert ends[0].crash_reason == "turn cancelled" + assert proc.killed is True # not abandoned mid-stream — see TestTurnAlwaysReapsTheCli + + +class TestTurnAlwaysReapsTheCli: + """No exit from `communicate()` may leave the CLI running. + + `AgentCrashError` is categorized AGENT_CRASH (max_retries=2) and the + orchestrator's attempt-failure hook only drains `pending_turn` — it never + kills the agent. An abandoned CLI therefore means attempt 2 spawns a SECOND + `opencode --dir --session ` while attempt 1 is still + editing the very files the criteria are about to score, and whichever writer + wins decides the task's result. + + The graceful `await self.kill()` already covers the intentional cuts and the + timeout; these pin the two paths that reach `finally` with a live child. + """ + + async def test_read_loop_crash_kills_the_cli(self, patch_exec, tmp_path): + """`_crash_turn` is synchronous and raises — nothing below it reaps.""" + proc = _ExplodingRunningProcess([_evt("step_start", {"id": "prt_1", "messageID": "msg_1"})]) + patch_exec(proc) + + with pytest.raises(AgentCrashError, match="OpenCode turn failed"): + await _run(_agent(), tmp_path) + + assert proc.killed is True + + async def test_external_cancel_kills_the_cli(self, patch_exec, tmp_path): + """The teardown must survive a CancelledError in flight, so it takes no + await — an interrupted one would leave the child alive after all.""" + proc = _HangingProcess([_evt("step_start", {"id": "prt_1", "messageID": "msg_1"})]) + patch_exec(proc) + agent = _agent() + await agent.start(str(tmp_path)) + + task = asyncio.ensure_future(agent.communicate("do the thing")) + await asyncio.sleep(0.05) # let it spawn and read the first event + task.cancel() + with pytest.raises(asyncio.CancelledError): + _ = await task + + assert proc.killed is True + + async def test_a_clean_turn_kills_nothing(self, patch_exec, tmp_path): + """The happy path is unchanged: the CLI exited, so the guard is a no-op + and the server child survives for the next turn's `--session` resume.""" + proc = _FakeProcess(HAPPY_STREAM) + captured = patch_exec(proc) + await _run(_agent(), tmp_path) + + assert proc.killed is False + assert captured["killpg"] == [] + + async def test_a_spawn_failure_has_no_process_to_reap(self, monkeypatch, tmp_path): + """`proc` is unbound on this path; the guard must not raise NameError over it.""" + + async def boom(*_argv: str, **_kwargs: Any): + raise OSError("no fork for you") + + monkeypatch.setattr(asyncio, "create_subprocess_exec", boom) + monkeypatch.setattr("shutil.which", lambda _name: "/usr/local/bin/opencode") + + with pytest.raises(AgentCrashError, match="no fork for you"): + await _run(_agent(), tmp_path) @pytest.mark.skipif(os.name != "posix", reason="process-group teardown (killpg/SIGKILL) is POSIX-only by design") @@ -1066,6 +1210,16 @@ async def test_stop_sweeps_the_spawned_group(self, patch_exec, tmp_path): await agent.stop() assert (4242, signal.SIGKILL) in captured["killpg"] + async def test_a_crashed_turn_sweeps_the_group_too(self, patch_exec, tmp_path): + """Killing the CLI pid alone would orphan the server child it left holding + the pipes — across a retried batch, that is the leak that compounds.""" + captured = patch_exec(_ExplodingRunningProcess([_evt("step_start", {"id": "prt_1", "messageID": "msg_1"})])) + + with pytest.raises(AgentCrashError): + await _run(_agent(), tmp_path) + + assert (4242, signal.SIGKILL) in captured["killpg"] + async def test_cooperative_stop_sweeps_the_group_too(self, patch_exec, tmp_path): captured = patch_exec(_RunningProcess(HAPPY_STREAM)) await _run(_agent(), tmp_path, should_stop=lambda: True) From 839e818ae3eb7a72067ee6a0d4732b88b8da1a47 Mon Sep 17 00:00:00 2001 From: mohsen-uipath Date: Mon, 17 Aug 2026 10:18:48 -0700 Subject: [PATCH 09/12] fix(opencode): gate on captured tokens, canonicalize tool args, pin max_turns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- docs/agents/OPENCODE.md | 54 ++++- src/coder_eval/agents/opencode_agent.py | 127 +++++++++-- src/coder_eval/criteria/skill_triggered.py | 10 +- tests/test_opencode_agent.py | 248 ++++++++++++++++++++- tests/test_skill_triggered.py | 22 +- 5 files changed, 411 insertions(+), 50 deletions(-) diff --git a/docs/agents/OPENCODE.md b/docs/agents/OPENCODE.md index b098a613..dc57872c 100644 --- a/docs/agents/OPENCODE.md +++ b/docs/agents/OPENCODE.md @@ -227,15 +227,41 @@ on; an unmapped tool keeps its own name. This is what lets one call's `command` parameter) score identically whether the run used Claude, Codex or OpenCode. +Tool **argument keys** are normalized the same way, because `command_executed` +serializes `parameters` to JSON for every tool but `Bash` — so a criterion like +`{type: command_executed, tool_name: Read, command_pattern: 'file_path.*app\.py'}` +would otherwise match on Claude and score 0 on OpenCode for identical behaviour: + +| Canonical tool | OpenCode key | Recorded as | +|---|---|---| +| `Read` / `Write` / `Edit` | `path` (or `filePath`) | `file_path` | +| `Edit` | `oldString` / `newString` / `replaceAll` | `old_string` / `new_string` / `replace_all` | +| `Skill` | `name` | `skill` | + +Both file-path spellings are accepted because the CLI has moved between versions +(a 2026-08-13 capture emitted `filePath`; current builds register `path`). +`Bash`'s `command` and the search tools' `path` already match Claude's names and +pass through untouched, as does every unlisted key. + > These event names are the CLI's own compact vocabulary. They are **not** the > `session.next.*` names in the OpenAPI schema served by `opencode serve` — that > describes the HTTP/SSE surface and does not apply here. -Vocabulary drift is crashed, not scored: a turn whose CLI exits cleanly but whose -stream contained **no recognized events** captured zero telemetry (zero turns, -tokens and cost) while file-based criteria could still pass on whatever the agent -did — a success that silently vanishes from every aggregate. The turn is failed -with an error naming the unrecognized event types it saw instead. +Drift is crashed, not scored: a turn whose CLI exits cleanly but which captured +**no token telemetry** is failed rather than reported as a clean empty success. +File-based criteria could otherwise still pass on whatever the agent did, giving +a SUCCESS that silently vanishes from every token aggregate — and whose +`run_limits.max_total_tokens` / `max_usd` gates could never trip no matter what +the run really billed. Two shapes reach it: + +- the stream contained **no recognized events** (an upgrade renamed the + vocabulary) — the error names the unrecognized event types it saw; +- events were recognized but **every finished step carried no usable token + counts** (a provider or auth mode that omits `tokens`) — the error reports the + finished-step count and whether cost was present. + +Intentional cuts (`should_stop`, `max_turns`) are exempt: both can land before +the first event, or between a step's start and its `step_finish`. ## Known limitations @@ -280,13 +306,17 @@ providers are all excluded by your account's privacy/guardrail configuration. Verify independently with a direct API call, then adjust at [openrouter.ai/settings/privacy](https://openrouter.ai/settings/privacy). -**Task fails with `emitted no recognized events`** — the CLI exited cleanly but -nothing on its stdout matched the event vocabulary above, so the turn would have -scored with zero telemetry; the harness fails it instead. The error names the -event types it did see. Check the raw stream with -`opencode run --format json ... > raw.jsonl` and compare the `type` values -against the table above — an OpenCode upgrade that renames them needs a matching -harness update. +**Task fails with `captured zero token telemetry`** — the CLI exited cleanly but +the turn booked no tokens, so it would have scored with nothing in any aggregate; +the harness fails it instead. Check the raw stream with +`opencode run --format json ... > raw.jsonl`: + +- if the error says *no recognized events*, compare the `type` values against the + table above — an OpenCode upgrade that renames them needs a matching harness + update; +- if it reports finished steps with no usable token counts, inspect a + `step_finish` payload's `tokens` object — a provider or auth mode that omits + usage, or a renamed bucket, produces this. ## References diff --git a/src/coder_eval/agents/opencode_agent.py b/src/coder_eval/agents/opencode_agent.py index 48d7ce4a..fe9d14c5 100644 --- a/src/coder_eval/agents/opencode_agent.py +++ b/src/coder_eval/agents/opencode_agent.py @@ -146,6 +146,41 @@ "skill": "Skill", } +# OpenCode per-tool INPUT-arg key -> canonical (Claude) key. Mirrors +# antigravity_agent's _ANTIGRAVITY_ARG_RENAME and completes what _TOOL_NAME_MAP +# starts: normalizing the tool NAME alone still leaves a `command_executed` with +# a non-Bash `tool_name` matching against a differently-keyed JSON blob (see +# criteria/command_executed.py, which falls back to `json.dumps(parameters)` for +# every tool but Bash), so the same task scores differently per harness. Keyed by +# the canonical tool name (post _TOOL_NAME_MAP); unlisted keys pass through. +# +# `bash` needs no entry: OpenCode already names it `command`, which is why the +# Bash-only shell-aware extraction in command_executed.py was correct as-is. +# `glob`/`grep`/`list` also need none — their `path` already matches Claude's. +# +# BOTH file-path spellings are mapped because the CLI has MOVED: a live capture +# on 2026-08-13 emitted `filePath` (see the fixture in tests/test_opencode_agent.py), +# while the tool schemas registered by the CLI installed at the time of writing +# read `path` (`read`/`write`/`edit` all take `{path, ...}`). Accepting both keeps +# telemetry canonical across the CLI versions a run might use, and neither +# spelling collides with a legitimate parameter of these three tools. +_OPENCODE_ARG_RENAME: dict[str, dict[str, str]] = { + "Read": {"path": "file_path", "filePath": "file_path"}, + "Write": {"path": "file_path", "filePath": "file_path"}, + "Edit": { + "path": "file_path", + "filePath": "file_path", + "oldString": "old_string", + "newString": "new_string", + "replaceAll": "replace_all", + }, + # The skill loader's argument. With this rename, `skill_triggered` reads the + # agent-agnostic `parameters["skill"]` on every harness instead of carrying a + # per-harness alternative list in a criterion that must know nothing about + # harnesses. + "Skill": {"name": "skill"}, +} + # Config fields the OpenCode CLI has no equivalent knob for. `experiments/default.yaml` # sets `allowed_tools` on every task, so these are silently dropped by default — # warn once at start() rather than letting a task believe it constrained the agent. @@ -218,6 +253,18 @@ def _epoch_ms_to_dt(value: Any) -> datetime | None: return None +def _canonical_params(tool_name: str, params: dict[str, Any]) -> dict[str, Any]: + """Rename a tool call's argument keys to the canonical cross-agent vocabulary. + + Order is preserved and unlisted keys pass through untouched, so this only ever + re-labels what ``_OPENCODE_ARG_RENAME`` names for this tool. + """ + rename = _OPENCODE_ARG_RENAME.get(tool_name) + if not rename: + return params + return {rename.get(key, key): value for key, value in params.items()} + + def _manifest_skill_dirs(root: Path) -> list[Path]: """Skill directories a Claude-plugin root declares, in manifest order. @@ -324,6 +371,11 @@ def __init__(self, *, task_id: str, iteration: int, user_input: str, model: str self.messages: list[TranscriptMessage] = [] self.text_parts: list[str] = [] self.step_count = 0 + # Steps the CLI reported as FINISHED (`step_finish`), as opposed to + # `step_count`, which counts the ones it started. `_settle_turn` needs the + # distinction: a finished step is the CLI's own claim that a generation + # completed, so one that booked no tokens means the token schema moved. + self.steps_finished = 0 self.turn_id: str = "" self.step_started_at: datetime | None = None self.step_text_parts: list[str] = [] @@ -396,21 +448,23 @@ def on_tool_use(self, part: dict[str, Any]) -> None: state = part.get("state") state = state if isinstance(state, dict) else {} call_id = str(part.get("callID") or f"call_{self.sequence + 1}") + times = state.get("time") if isinstance(state.get("time"), dict) else {} + started = _epoch_ms_to_dt(times.get("start")) + params = state.get("input") + params = params if isinstance(params, dict) else {} telemetry = self.open_tools.get(call_id) if telemetry is None: self.sequence += 1 - times = state.get("time") if isinstance(state.get("time"), dict) else {} - started = _epoch_ms_to_dt(times.get("start")) - params = state.get("input") raw_tool = str(part.get("tool") or "unknown") + tool_name = _TOOL_NAME_MAP.get(raw_tool.lower(), raw_tool) telemetry = CommandTelemetry( - tool_name=_TOOL_NAME_MAP.get(raw_tool.lower(), raw_tool), + tool_name=tool_name, tool_id=call_id, assistant_turn_index=self.step_count, timestamp=started or datetime.now(), execution_started_at=started, - parameters=params if isinstance(params, dict) else {}, + parameters=_canonical_params(tool_name, params), sequence_number=self.sequence, ) self.open_tools[call_id] = telemetry @@ -418,6 +472,17 @@ def on_tool_use(self, part: dict[str, Any]) -> None: self.emit( ToolStartEvent(task_id=self.task_id, thread_id=self.thread_id, turn_id=self.turn_id, tool=telemetry) ) + else: + # A SECOND event for a call already open — the pending/running-then- + # completed lifecycle. The first event routinely carries no `input` + # (the CLI has not finished assembling the call), so freezing the + # first event's view would leave `parameters` permanently `{}` and + # zero every `command_executed` row while the run looked normal. + # Later evidence wins; absent evidence never clears what we have. + if params: + telemetry.parameters = _canonical_params(telemetry.tool_name, params) + if started is not None: + telemetry.execution_started_at = started status_text = str(state.get("status") or "").lower() output = state.get("output") @@ -602,6 +667,7 @@ def _fresh_input_slice( return raw_in def on_step_finish(self, part: dict[str, Any]) -> None: + self.steps_finished += 1 tokens = part.get("tokens") tokens = tokens if isinstance(tokens, dict) else {} cache = tokens.get("cache") if isinstance(tokens.get("cache"), dict) else {} @@ -1151,7 +1217,7 @@ async def _settle_turn( Raises ``AgentCrashError`` (via :meth:`_crash_turn`) when the stream carried a structured error, when the process died with neither a structured error - nor an intentional stop, or when a clean exit recognized no events at all + nor an intentional stop, or when a clean exit captured no token telemetry (a zero-telemetry turn must not score — see the guard below). Raises ``TurnTimeoutError`` when the turn deadline elapses while waiting for the exit. @@ -1192,22 +1258,45 @@ async def _settle_turn( detail = stderr_bytes.decode("utf-8", "replace").strip() or f"exit code {proc.returncode}" self._crash_turn(state, collector, f"OpenCode exited non-zero: {detail}") - # A clean exit that recognized NO events captured zero telemetry — zero - # turns, zero tokens, zero cost — while file-based criteria can still - # pass on whatever the agent did, producing a SUCCESS that is silently - # missing from every aggregate. This already happened once (the harness - # parsed the `session.next.*` server vocabulary instead of the CLI's), - # so vocabulary drift is crashed loudly instead of scored. Intentional - # cuts (should_stop / max_turns) are exempt: they can land before the - # first event. - if not stopped_early and not state.max_turns_exhausted and state.recognized_events == 0: - seen = ", ".join(sorted(state.unrecognized_types)) or "none (stdout carried no JSON events)" + # A clean exit that captured NO token telemetry must not score. File-based + # criteria can still pass on whatever the agent did, producing a SUCCESS + # that is silently missing from every aggregate — and, worse, one whose + # `run_limits.max_total_tokens` / `max_usd` gates could never have tripped + # no matter how much the run actually billed. This already happened once + # (the harness parsed the `session.next.*` server vocabulary instead of + # the CLI's), so drift is crashed loudly instead of scored. + # + # The condition is the TELEMETRY, not the event vocabulary. Keying on + # `recognized_events == 0` alone 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` then maps that to `token_usage=None` + # — a COMPLETED turn with no tokens, no cost and no warning. + # + # Intentional cuts (should_stop / max_turns) are exempt: both can land + # before the first event, or between a step's start and its `step_finish`. + # + # 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 that was cut before any step + # could finish. + nothing_recognized = state.recognized_events == 0 + finished_without_tokens = state.steps_finished > 0 and state.usage.is_empty() + if not stopped_early and not state.max_turns_exhausted and (nothing_recognized or finished_without_tokens): + if nothing_recognized: + seen = ", ".join(sorted(state.unrecognized_types)) or "none (stdout carried no JSON events)" + detail = f"It emitted no recognized events at all. Unrecognized event types seen: {seen}." + else: + detail = ( + f"It reported {state.steps_finished} finished step(s), none of which carried usable " + + f"token counts (cost reported: {'yes' if state.saw_cost else 'no'})." + ) self._crash_turn( state, collector, - "OpenCode exited cleanly but emitted no recognized events, so the turn captured zero " - + f"telemetry. Unrecognized event types seen: {seen}. The CLI's event vocabulary may have " - + "changed — see docs/agents/OPENCODE.md (Telemetry) before trusting any run from this CLI version.", + f"OpenCode exited cleanly but the turn captured zero token telemetry. {detail} The CLI's " + + "event or token schema may have changed — see docs/agents/OPENCODE.md (Telemetry) before " + + "trusting any run from this CLI version.", ) if stopped_early: diff --git a/src/coder_eval/criteria/skill_triggered.py b/src/coder_eval/criteria/skill_triggered.py index 5a309756..18bfc761 100644 --- a/src/coder_eval/criteria/skill_triggered.py +++ b/src/coder_eval/criteria/skill_triggered.py @@ -50,6 +50,10 @@ def _engaged_skill_names(cmd: CommandTelemetry) -> set[str]: - Claude: an explicit ``Skill`` tool call carries the skill in ``parameters['skill']``, optionally namespaced (e.g. ``plugin:uipath-agents``); the namespace is stripped via ``.split(":")[-1]``. + A harness whose skill tool names that argument something else renames it at + the AGENT boundary (OpenCode's ``name`` -> ``skill``, via + ``_OPENCODE_ARG_RENAME``), so this stays keyed on one canonical name rather + than growing an alternative per harness. - Codex (and any non-Claude agent): no ``Skill`` tool exists, so a skill is engaged by reading its files off disk via shell. Both the repo layout (``.../skills//...``) and the sandbox symlink @@ -63,11 +67,7 @@ def _engaged_skill_names(cmd: CommandTelemetry) -> set[str]: """ names: set[str] = set() if cmd.tool_name == "Skill": - # Claude names the parameter `skill`; OpenCode's native skill tool names - # the same value `name` (its raw tool name is lowercase `skill`, mapped to - # the canonical `Skill` by the OpenCode agent). Read both so one criterion - # scores a skill engagement identically on either harness. - skill = cmd.parameters.get("skill") or cmd.parameters.get("name") or "" + skill = cmd.parameters.get("skill") or "" if isinstance(skill, str) and skill: names.add(skill.split(":")[-1]) for value in cmd.parameters.values(): diff --git a/tests/test_opencode_agent.py b/tests/test_opencode_agent.py index 5806943b..417d8977 100644 --- a/tests/test_opencode_agent.py +++ b/tests/test_opencode_agent.py @@ -33,6 +33,7 @@ AgentStartEvent, ToolEndEvent, ToolEndStatus, + ToolStartEvent, ) @@ -259,7 +260,9 @@ async def test_tool_call_captured(self, patch_exec, tmp_path): assert cmd.tool_name == "Read" assert cmd.tool_id == "call_1" assert cmd.result_status == "success" - assert cmd.parameters == {"filePath": "main.py"} + # ...including the ARGUMENT keys: the fixture's native `filePath` is + # recorded under Claude's `file_path` (see TestCrossHarnessNormalization). + assert cmd.parameters == {"file_path": "main.py"} assert cmd.result_summary == "print('hi')" # Duration comes from state.time, not our parse instant (17ms in fixture). assert cmd.duration_ms == pytest.approx(17, abs=1) @@ -476,6 +479,145 @@ async def test_native_skill_tool_maps_to_canonical_skill(self, patch_exec, tmp_p record = await _run(_agent(), tmp_path) assert [c.tool_name for c in record.commands] == ["Skill"] + @staticmethod + def _tool_with_input(tool: str, params: dict[str, Any]) -> str: + return _evt( + "tool_use", + { + "id": "prt_2", + "messageID": "msg_1", + "type": "tool", + "tool": tool, + "callID": f"call_{tool}", + "state": {"status": "completed", "input": params, "output": "ok"}, + }, + ) + + @pytest.mark.parametrize( + ("tool", "native", "expected"), + [ + # The CLI installed at the time of writing registers `{path, ...}` for + # all three file tools; a 2026-08-13 capture emitted `filePath`. Both + # spellings must land on Claude's `file_path`. + ("read", {"path": "main.py"}, {"file_path": "main.py"}), + ("read", {"filePath": "main.py"}, {"file_path": "main.py"}), + ("write", {"path": "a.py", "content": "x"}, {"file_path": "a.py", "content": "x"}), + ( + "edit", + {"path": "a.py", "oldString": "a", "newString": "b", "replaceAll": True}, + {"file_path": "a.py", "old_string": "a", "new_string": "b", "replace_all": True}, + ), + ("skill", {"name": "uipath-flow"}, {"skill": "uipath-flow"}), + ], + ) + async def test_argument_keys_map_to_canonical(self, patch_exec, tmp_path, tool, native, expected): + """Normalizing the tool NAME is only half the job. + + `command_executed` serializes `parameters` to JSON for every tool but Bash + (criteria/command_executed.py), so a criterion like + `{tool_name: Read, command_pattern: 'file_path.*app\\.py'}` matches on Claude + and scores 0 on OpenCode for identical agent behaviour. + """ + patch_exec(_FakeProcess([self._tool_with_input(tool, native)])) + record = await _run(_agent(), tmp_path) + assert record.commands[0].parameters == expected + + @pytest.mark.parametrize( + ("tool", "native"), + [ + ("bash", {"command": "pytest -q"}), # already canonical + ("grep", {"pattern": "x", "path": "src"}), # `path` is Claude's key here too + ("list", {"path": "src"}), + ("some_new_tool", {"whatever": 1}), # unmapped tool: untouched + ], + ) + async def test_already_canonical_keys_are_left_alone(self, patch_exec, tmp_path, tool, native): + """The rename is per-tool: `path` means `file_path` on Read/Write/Edit and + stays `path` on the search tools, which is exactly Claude's split.""" + patch_exec(_FakeProcess([self._tool_with_input(tool, native)])) + record = await _run(_agent(), tmp_path) + assert record.commands[0].parameters == native + + +class TestTwoEventToolLifecycle: + """The CLI may emit `pending`/`running` before `completed` for one callID. + + The first event routinely carries no `input` — the call is not assembled yet — + so freezing the first event's view leaves `parameters` permanently `{}`. + `command_executed` reads `parameters["command"]` for `tool_name: Bash`, so that + criterion would score 0 on every row while the run looked entirely normal. + """ + + @staticmethod + def _event(status: str, state_extra: dict[str, Any]) -> str: + return _evt( + "tool_use", + { + "id": "prt_2", + "messageID": "msg_1", + "type": "tool", + "tool": "bash", + "callID": "call_1", + "state": {"status": status, **state_extra}, + }, + ) + + async def test_the_completion_supplies_the_parameters(self, patch_exec, tmp_path): + patch_exec( + _FakeProcess( + [ + _evt("step_start", {"id": "prt_1", "messageID": "msg_1", "type": "step-start"}), + self._event("running", {}), + self._event( + "completed", + { + "input": {"command": "pytest -q"}, + "output": "ok", + "time": {"start": 1786663018214, "end": 1786663018231}, + }, + ), + ] + ) + ) + record = await _run(_agent(), tmp_path) + + assert len(record.commands) == 1 # one tool, not two + cmd = record.commands[0] + assert cmd.tool_name == "Bash" + assert cmd.parameters == {"command": "pytest -q"} + assert cmd.result_status == "success" + assert cmd.execution_started_at is not None + + async def test_one_tool_start_end_pair_is_emitted(self, patch_exec, tmp_path): + patch_exec( + _FakeProcess( + [ + _evt("step_start", {"id": "prt_1", "messageID": "msg_1", "type": "step-start"}), + self._event("pending", {}), + self._event("completed", {"input": {"command": "ls"}, "output": "ok"}), + ] + ) + ) + recorder = _EventRecorder() + await _run(_agent(), tmp_path, stream_callback=recorder) + + assert len([e for e in recorder.events if isinstance(e, ToolStartEvent)]) == 1 + assert len([e for e in recorder.events if isinstance(e, ToolEndEvent)]) == 1 + + async def test_a_later_event_without_input_never_clears_what_we_have(self, patch_exec, tmp_path): + """Absent evidence is not evidence of absence — the first event's args stay.""" + patch_exec( + _FakeProcess( + [ + _evt("step_start", {"id": "prt_1", "messageID": "msg_1", "type": "step-start"}), + self._event("running", {"input": {"command": "ls"}}), + self._event("completed", {"output": "ok"}), + ] + ) + ) + record = await _run(_agent(), tmp_path) + assert record.commands[0].parameters == {"command": "ls"} + class TestSandboxEnvironment: """`start(env_path_prepend=..., plugin_tools_dir=...)` is the abstract @@ -757,12 +899,17 @@ async def test_missing_cli_is_actionable(self, monkeypatch, tmp_path): class TestZeroTelemetryIsLoud: - """A clean exit that recognized no events must crash, not score. + """A clean exit that captured no token telemetry must crash, not score. An earlier version of this harness parsed the `session.next.*` server vocabulary instead of the CLI's and reported SUCCESS 1.0 with zero turns, zero tokens and zero cost — indistinguishable from a real pass in every - aggregate. Vocabulary drift must be an ERROR, not a quiet empty success. + aggregate. Drift must be an ERROR, not a quiet empty success. + + The guard keys on the TELEMETRY, not the event vocabulary: recognizing the + event names is not the property worth protecting, and checking them alone + left the identical outcome reachable one layer down (see + `test_finished_step_without_tokens_crashes`). """ async def test_unrecognized_vocabulary_crashes_and_names_the_types(self, patch_exec, tmp_path): @@ -803,6 +950,55 @@ async def test_intentional_cuts_are_exempt(self, patch_exec, tmp_path): record = await _run(_agent(), tmp_path, should_stop=lambda: True) assert record.crashed is False + @staticmethod + def _stream_without_tokens(**finish_extra: Any) -> list[str]: + """HAPPY_STREAM's shape with the `tokens` key absent from every step.""" + return [ + _evt("step_start", {"id": "prt_1", "messageID": "msg_1", "type": "step-start"}), + _evt("text", {"id": "prt_2", "messageID": "msg_1", "type": "text", "text": "Done."}), + _evt("step_finish", {"id": "prt_3", "messageID": "msg_1", "reason": "stop", **finish_extra}), + ] + + async def test_finished_step_without_tokens_crashes(self, patch_exec, tmp_path): + """The event vocabulary is fine and three events are recognized — but the + turn still captured nothing. + + `EventCollector` maps an all-zero, costless `TokenUsage` to + `token_usage=None`, so this is a COMPLETED turn a file-based criterion can + score SUCCESS on, absent from every token aggregate, whose + `run_limits.max_total_tokens` / `max_usd` gates could never trip no matter + what the run really billed. + """ + patch_exec(_FakeProcess(self._stream_without_tokens())) + agent = _agent() + + with pytest.raises(AgentCrashError, match="zero token telemetry") as exc: + await _run(agent, tmp_path) + assert "1 finished step(s)" in str(exc.value) + assert agent.pending_turn is not None # telemetry captured so far still parked + + async def test_cost_without_tokens_still_crashes(self, patch_exec, tmp_path): + """Reported cost does not excuse missing tokens: the USD gate might trip, + but every token gate and aggregate is still silently blind.""" + patch_exec(_FakeProcess(self._stream_without_tokens(cost=0.004))) + with pytest.raises(AgentCrashError, match="cost reported: yes"): + await _run(_agent(), tmp_path) + + async def test_a_cut_before_any_step_finished_is_exempt(self, patch_exec, tmp_path): + """The arm keys on a step the CLI reported FINISHED. A stop landing between + a step's start and its `step_finish` is an intentional cut, not drift.""" + proc = _RunningProcess([_evt("step_start", {"id": "prt_1", "messageID": "msg_1", "type": "step-start"})]) + patch_exec(proc) + record = await _run(_agent(), tmp_path, should_stop=lambda: True) + assert record.crashed is False + + async def test_real_tokens_are_never_condemned(self, patch_exec, tmp_path): + """The guard must not fire on the ordinary path it lives beside.""" + patch_exec(_FakeProcess(HAPPY_STREAM)) + record = await _run(_agent(), tmp_path) + assert record.crashed is False + assert record.token_usage is not None + class _ExplodingProcess(_FakeProcess): """Replays events, then raises from ``readline`` mid-stream. @@ -985,6 +1181,52 @@ async def test_max_turns_marks_exhausted(self, patch_exec, tmp_path): record = await _run(_agent(), tmp_path, max_turns=1) assert record.max_turns_exhausted is True + async def test_a_cap_the_run_stays_under_is_not_exhausted(self, patch_exec, tmp_path): + """The OTHER direction, which decides `FinalStatus`. + + HAPPY_STREAM is exactly 2 steps, so `max_turns=2` is the boundary: an + off-by-one here (`>` becoming `>=`, or counting finished steps instead of + started ones) reports MAX_TURNS_EXHAUSTED — orchestrator.py turns the flag + straight into `FinalStatus.MAX_TURNS_EXHAUSTED` — for a run that finished + well inside its budget. A spurious exhaustion also suppresses the non-zero- + exit and zero-telemetry crash guards, which are both conditioned on it, so + the run would score silently instead of failing loudly. + """ + patch_exec(_FakeProcess(HAPPY_STREAM)) + record = await _run(_agent(), tmp_path, max_turns=2) + + assert record.max_turns_exhausted is False + assert record.assistant_turn_count == 2 + # Both steps' telemetry is present — the cap did not truncate the stream. + assert record.token_usage is not None + assert record.token_usage.output_tokens == 57 + + async def test_no_cap_is_uncapped(self, patch_exec, tmp_path): + patch_exec(_FakeProcess(HAPPY_STREAM)) + record = await _run(_agent(), tmp_path) + assert record.max_turns_exhausted is False + assert record.assistant_turn_count == 2 + + async def test_the_deciding_step_is_kept_whole(self, patch_exec, tmp_path): + """`max_turns=1` cuts at the START of step 2, so step 1 survives complete. + + Asserting only the flag would let a cut that discards the step that earned + the budget pass — the run would report exhaustion with none of the + telemetry that reached it. + """ + patch_exec(_FakeProcess(HAPPY_STREAM)) + record = await _run(_agent(), tmp_path, max_turns=1) + + assert record.max_turns_exhausted is True + assert len(record.commands) == 1 # step 1's tool call + usage = record.token_usage + assert usage is not None + # Step 1's buckets exactly (nested convention: 100-10-5=85 fresh input). + assert usage.uncached_input_tokens == 85 + assert usage.output_tokens == 20 + assert usage.cache_creation_input_tokens == 5 + assert usage.cache_read_input_tokens == 10 + class _HangingProcess(_FakeProcess): """Emits nothing and never exits until it is signaled. diff --git a/tests/test_skill_triggered.py b/tests/test_skill_triggered.py index de6ea57e..03a335fd 100644 --- a/tests/test_skill_triggered.py +++ b/tests/test_skill_triggered.py @@ -50,17 +50,17 @@ def test_skill_invoked_tp(self) -> None: ) assert result.score == 1.0 and result.observed_label == "yes" and result.expected_label == "yes" - def test_opencode_name_parameter_counts_as_engagement(self) -> None: - """OpenCode's native skill tool carries the skill under `name`, not `skill`. - Reading only `skill` scored every OpenCode engagement as a miss.""" - result = _check( - expected_skill="uipath-flow", skill_name="uipath-flow", commands=[_cmd("Skill", {"name": "uipath-flow"})] - ) - assert result.score == 1.0 and result.observed_label == "yes" - - def test_opencode_name_parameter_is_scoped_to_the_skill_tool(self) -> None: - """`name` is a generic parameter; only a `Skill` call may be read that way.""" - result = _check(expected_skill="", skill_name="uipath-flow", commands=[_cmd("Write", {"name": "uipath-flow"})]) + def test_only_the_canonical_skill_parameter_is_read(self) -> None: + """The criterion is agent-agnostic: it knows ONE key. + + A harness whose skill tool names the argument differently renames it at the + agent boundary (OpenCode's `name` -> `skill`, `_OPENCODE_ARG_RENAME`) — see + tests/test_opencode_agent.py::TestCrossHarnessNormalization. Accepting + alternatives here instead would make every future harness edit a criterion + that must know nothing about harnesses, and `parameters` is substring-scanned + just below, so widening the key set here is the riskier of the two places. + """ + result = _check(expected_skill="", skill_name="uipath-flow", commands=[_cmd("Skill", {"name": "uipath-flow"})]) assert result.score == 1.0 and result.observed_label == "no" def test_no_skill_tn(self) -> None: From ad0a559223899d8464b4780233cc47805b772aaa Mon Sep 17 00:00:00 2001 From: mohsen-uipath Date: Mon, 17 Aug 2026 10:43:49 -0700 Subject: [PATCH 10/12] fix(opencode): close the remaining review findings on the harness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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%). --- README.md | 12 +- docker/Dockerfile | 16 +- docs/USER_GUIDE.md | 2 +- docs/agents/OPENCODE.md | 33 +++ docs/index.md | 10 +- docs/llms.txt | 3 +- src/coder_eval/agents/opencode_agent.py | 125 ++++++++-- tests/test_opencode_agent.py | 300 +++++++++++++++++++++++- 8 files changed, 465 insertions(+), 36 deletions(-) diff --git a/README.md b/README.md index 03eaaa21..fe7adb51 100644 --- a/README.md +++ b/README.md @@ -11,14 +11,14 @@ **Coder Eval** (`pip install coder-eval` / `uv tool install coder-eval`) is an open-source framework for **evaluating and benchmarking AI coding agents and their skills** — built for CLI and skill builders — with sandboxing, reproducibility, and data-driven analysis. -It runs a real agent (**Claude Code**, **Codex**, or **Google Antigravity / -Gemini**) in a sandbox against declarative YAML tasks, then scores the files and +It runs a real agent (**Claude Code**, **Codex**, **Google Antigravity / +Gemini**, or **OpenCode**) in a sandbox against declarative YAML tasks, then scores the files and commands it actually produced. Not an "agentic coding" benchmark: it measures how effective your CLI and skills are when used by coding agents. Reach for it when you want to **test whether a Claude Code skill triggers**, -**A/B-test Claude Code vs. Codex vs. Gemini** (or model vs. model, prompt vs. -prompt), or **gate CI on coding-agent quality**. Unlike fixed datasets (SWE-bench, +**A/B-test Claude Code vs. Codex vs. Gemini vs. OpenCode** (or model vs. model, +prompt vs. prompt), or **gate CI on coding-agent quality**. Unlike fixed datasets (SWE-bench, SkillsBench) that rank models on a shared leaderboard, Coder Eval evaluates the tasks, skills, and workflows *you* ship — with weighted 0.0–1.0 criteria, a `skill_triggered` activation check, an A/B experiment layer, and per-tool cost @@ -33,14 +33,14 @@ telemetry. See [How it compares](https://coder-eval.com/docs/comparison). - **Sandboxed execution** in isolated environments with resource limits - **Weighted, continuous scoring** (0.0–1.0) with fractional credit and thresholds - **Many criterion types** — from file checks to code similarity and LLM-graded rubrics -- **Agent abstraction** — Claude Code, Codex, and Antigravity (Gemini) today, extensible via a plugin SPI +- **Agent abstraction** — Claude Code, Codex, Antigravity (Gemini), and OpenCode today, extensible via a plugin SPI - **Experiment layer** — A/B agent configs (models, tools, prompts) side-by-side - **Full telemetry** — every tool call, token counts, and cost, with real-time streaming ## What you can do with it - **Benchmark coding agents** — score an agent across a suite of tasks with weighted scoring and pass/fail thresholds -- **Compare models & configs** — A/B-test Claude vs. Codex vs. Gemini, model vs. model, tool-on vs. tool-off, prompt vs. prompt +- **Compare models & configs** — A/B-test Claude vs. Codex vs. Gemini vs. OpenCode, model vs. model, tool-on vs. tool-off, prompt vs. prompt - **Evaluate skills** — verify an agent actually engages a target skill (`skill_triggered`) and score skill-driven suites (SkillsBench-style) - **Keep skills up to date in CI** — re-validate your skills on every change or on a schedule; catch silent regressions when models, prompts, or the skills themselves drift - **Gate CI on agent quality** — run the suite in GitHub Actions and fail the build on regressions diff --git a/docker/Dockerfile b/docker/Dockerfile index 89717613..af585524 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -49,11 +49,17 @@ COPY src/ ./src/ COPY experiments/default.yaml ./experiments/default.yaml # 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. `openai-codex` (+ its pinned cli-bin) and `google-antigravity` (which -# bundles its `localharness` binary as a manylinux wheel) come from public PyPI, -# so this needs no private-index credentials. The RUN below always passes -# `--extra codex --extra antigravity`. +# claude-code agent installed above. `openai-codex` (+ its pinned cli-bin) and +# `google-antigravity` (which bundles its `localharness` binary as a manylinux +# wheel) come from public PyPI, so this needs no private-index credentials. The +# RUN below always passes `--extra codex --extra antigravity`. +# +# NOT every built-in agent ships here: `opencode` is registered unconditionally +# but its CLI is a Node package (`npm install -g opencode-ai`), absent from this +# image, and no OpenCode credentials are in SandboxConfig.env_passthrough -- so +# `--driver docker` does not support it. Adding it means a pinned version that +# travels with the release tag (as CLAUDE_CODE_VERSION does) plus an +# env_passthrough block; see docs/agents/OPENCODE.md "Running in Docker". # # CODER_EVAL_UV_EXTRAS carries ADDITIONAL opt-in extras on top of those; it # defaults to none. `make docker-image-full` passes `--extra uipath`, which diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index f7daba37..1676690d 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -39,7 +39,7 @@ coder-eval run tasks/hello_date.yaml --stream full # live LLM output | `-D path=value` / `--set` | Override any resolved task-config field (`agent`/`run_limits`/`sandbox` roots), e.g. `-D run_limits.max_turns=30 -D agent.permission_mode=plan -D agent.sdk_options.effort=high`. Repeatable; schema-validated. This is the way to set permission mode, turn/timeout limits, token/USD budget caps, tools, plugins, and SDK options. | | `--model, -m` | Shorthand alias for `-D agent.model=…` (e.g., `claude-sonnet-5`) | | `--driver` | Shorthand alias for `-D sandbox.driver=…` (`tempdir` or `docker`) | -| `--type, -T` | Override agent type for all tasks (`claude-code`, `codex`, `antigravity`, or a plugin kind). | +| `--type, -T` | Override agent type for all tasks (`claude-code`, `codex`, `antigravity`, `opencode`, or a plugin kind). | | `--repeats` | Run each `(task, variant)` N times (≥1); overrides experiment/variant `repeats:`. See [Replicates](#replicates). | | `--resume` | Resume an interrupted run: skip tasks already finalized in `--run-dir` and run the rest, folding prior results into `run.json`. Requires `--run-dir`. A task with *any* final status (incl. FAILED/ERROR) counts as finalized, so resume does **not** retry failures — delete a task's `task.json` to force a re-run. A config mismatch is warned, not refused. | | `--sample N` | For dataset-backed tasks, run a fixed-seed random N-row sample (reproducible; cheap smoke test). See [Bring Your Own Dataset](DATASETS.md). | diff --git a/docs/agents/OPENCODE.md b/docs/agents/OPENCODE.md index dc57872c..3b00d1d9 100644 --- a/docs/agents/OPENCODE.md +++ b/docs/agents/OPENCODE.md @@ -263,6 +263,35 @@ the run really billed. Two shapes reach it: Intentional cuts (`should_stop`, `max_turns`) are exempt: both can land before the first event, or between a step's start and its `step_finish`. +## Running in Docker + +**Not supported yet — use the default `tempdir` driver.** Unlike the other +built-in agents, `sandbox: {driver: docker}` does not work with +`agent: {type: opencode}`, and the failure is loud rather than silent: +`start()` finds no `opencode` on PATH inside the container and every task dies +with the install hint, which you cannot act on because that PATH lives in an +image you did not build. + +Two things are missing, both deliberate rather than overlooked: + +- **The CLI is not in the image.** `docker/Dockerfile` bakes in `claude-code` + (pinned) plus the `codex` and `antigravity` extras. OpenCode is a Node CLI + installed with `npm install -g opencode-ai`, so shipping it means adding a + pinned version that travels with the coder_eval release tag the way + `CLAUDE_CODE_VERSION` does — a release-process decision, not a one-line edit. + (Node 22 is already present in the image, so the change itself is small.) +- **No credentials would reach it.** The docker driver forwards host environment + variables through an explicit allowlist (`SandboxConfig.env_passthrough`), + which carries per-harness blocks for Codex and Antigravity but none for + OpenCode — so `OPENROUTER_API_KEY` and friends are not passed through, and + `opencode auth login`'s credential file is not mounted. Even a custom image + with the CLI baked in would authenticate against nothing. + +Until both land, run OpenCode tasks under `tempdir` (the default) on a host that +has the CLI and its provider credentials. If you need container isolation now, +build your own image from `docker/Dockerfile` with the `npm install -g` line +added and pass the credentials via `sandbox.env_passthrough_extra`. + ## Known limitations - **`allowed_tools` / `disallowed_tools` / `system_prompt` / `system_prompt_file` @@ -280,6 +309,10 @@ the first event, or between a step's start and its `step_finish`. visible-turn unit Codex/Antigravity use — see [Run-Limit Parity](HARNESS_PARITY.md) before holding `max_turns` constant across harnesses. +- **The `docker` sandbox driver is unsupported.** The CLI is not in the image and + no OpenCode credentials are in the `env_passthrough` allowlist — see + [Running in Docker](#running-in-docker) for the workaround and what it would + take to close. - **No sub-agent attribution.** OpenCode's CLI stream does not expose nested agent generations, so per-sub-agent token grouping (available for Claude and Codex) is not derivable. diff --git a/docs/index.md b/docs/index.md index f988cd74..6960f0e3 100644 --- a/docs/index.md +++ b/docs/index.md @@ -4,7 +4,7 @@ description: >- Coder Eval is an open-source framework to evaluate, benchmark, and A/B-test AI coding agents and Claude Code skills in a sandbox — declarative YAML tasks, weighted scoring, cost/token telemetry, and CI gates for Claude Code, Codex, - and Gemini. + Gemini, and OpenCode. --- # Evaluate AI coding agents & Claude Code skills — Coder Eval @@ -13,8 +13,8 @@ description: >- **evaluating AI coding agents and their skills** — built for CLI and skill builders — with sandboxing, reproducibility, and data-driven analysis. It is not an "agentic coding" benchmark: it measures how effective *your* CLI and skills -are when used by coding agents such as **Claude Code**, **Codex**, and **Google -Antigravity (Gemini)**. +are when used by coding agents such as **Claude Code**, **Codex**, **Google +Antigravity (Gemini)**, and **OpenCode**. If you have ever asked *"how do I test whether my Claude Code skill actually triggers?"*, *"how do I benchmark Claude Code vs. Codex on my own tasks?"*, or @@ -30,14 +30,14 @@ triggers?"*, *"how do I benchmark Claude Code vs. Codex on my own tasks?"*, or - **Sandboxed execution** in isolated environments with resource limits - **Weighted, continuous scoring** (0.0–1.0) with fractional credit and thresholds - **Many criterion types** — from file checks to code similarity and LLM-graded rubrics -- **Agent abstraction** — Claude Code, Codex, and Antigravity (Gemini) today, extensible via a plugin SPI +- **Agent abstraction** — Claude Code, Codex, Antigravity (Gemini), and OpenCode today, extensible via a plugin SPI - **Experiment layer** — A/B agent configs (models, tools, prompts) side-by-side - **Full telemetry** — every tool call, token counts, and cost, with real-time streaming ## Use cases - **Benchmark coding agents** — score an agent across a suite of tasks with weighted pass/fail thresholds -- **Compare models & configs** — A/B-test Claude Code vs. Codex vs. Gemini, model vs. model, tool-on vs. tool-off, prompt vs. prompt +- **Compare models & configs** — A/B-test Claude Code vs. Codex vs. Gemini vs. OpenCode, model vs. model, tool-on vs. tool-off, prompt vs. prompt - **Test whether a Claude Code skill triggers** — verify an agent actually engages a target skill (`skill_triggered`) and score skill-driven suites (SkillsBench-style) - **Keep skills fresh in CI** — re-validate skills on every change or on a schedule; catch silent regressions when models, prompts, or the skills themselves drift diff --git a/docs/llms.txt b/docs/llms.txt index 04042806..8e30b514 100644 --- a/docs/llms.txt +++ b/docs/llms.txt @@ -3,7 +3,8 @@ > Coder Eval (`pip install coder-eval`) is an open-source framework for evaluating, > benchmarking, and A/B-testing AI coding agents and their Claude Code skills in a > sandbox. It uses declarative YAML tasks with weighted, continuous scoring -> (0.0–1.0), runs real agents (Claude Code, Codex, Google Antigravity/Gemini) with +> (0.0–1.0), runs real agents (Claude Code, Codex, Google Antigravity/Gemini, +> OpenCode) with > full tool use, captures per-tool cost/token telemetry, and provides CI-ready > pass/fail gates. It is not a fixed benchmark or leaderboard — it scores your own > tasks, and can verify whether a Claude Code skill actually triggers. diff --git a/src/coder_eval/agents/opencode_agent.py b/src/coder_eval/agents/opencode_agent.py index fe9d14c5..dd570881 100644 --- a/src/coder_eval/agents/opencode_agent.py +++ b/src/coder_eval/agents/opencode_agent.py @@ -38,7 +38,7 @@ import shutil import signal import time -from collections.abc import Callable +from collections.abc import Callable, Mapping, Sequence from datetime import datetime from pathlib import Path from typing import Any, ClassVar, Literal, NoReturn @@ -49,6 +49,7 @@ from coder_eval.models import ( AgentKind, AgentState, + ApiRoute, AssistantMessage, CommandTelemetry, ContentBlock, @@ -293,7 +294,7 @@ def _manifest_skill_dirs(root: Path) -> list[Path]: def _plugin_skill_dirs( - plugins: list[dict[str, Any]] | None, + plugins: Sequence[Mapping[str, Any]] | None, log: logging.Logger | logging.LoggerAdapter[Any] = logger, ) -> list[str]: """Resolve ``plugins:`` entries to OpenCode ``skills.paths`` directories. @@ -304,7 +305,7 @@ def _plugin_skill_dirs( """ resolved: list[str] = [] for plugin in plugins or []: - if not isinstance(plugin, dict) or plugin.get("type") != "local": + if not isinstance(plugin, Mapping) or plugin.get("type") != "local": log.warning("opencode: ignoring non-local plugin entry %r — only `type: local` maps to skills.", plugin) continue path_str = plugin.get("path") @@ -377,6 +378,9 @@ def __init__(self, *, task_id: str, iteration: int, user_input: str, model: str # completed, so one that booked no tokens means the token schema moved. self.steps_finished = 0 self.turn_id: str = "" + # True between a step's `step_start` and its `step_finish`. `finalize` + # needs it to close a TurnStartEvent the stream never got to close. + self.step_open = False self.step_started_at: datetime | None = None self.step_text_parts: list[str] = [] self.step_tool_ids: list[str] = [] @@ -412,6 +416,7 @@ def agent_output(self) -> str: def on_step_start(self, part: dict[str, Any]) -> None: self.step_count += 1 + self.step_open = True self.turn_id = str(part.get("messageID") or f"step_{self.step_count}") self.step_started_at = datetime.now() self.step_text_parts = [] @@ -599,6 +604,44 @@ def _warn_token_shape(self, message: str, *args: Any) -> None: self.warned_token_shape = True logger.warning("opencode: unexpected token accounting — " + message, *args) + def _as_int(self, bucket: str, value: Any) -> int: + """Coerce one stream-supplied token count, warning instead of raising. + + A bare ``int()`` raises on anything non-numeric (``int("abc")`` -> + ``ValueError``; ``int({...})``/``int([...])`` -> ``TypeError``), which + ``communicate``'s ``except Exception`` turns into an ``AgentCrashError`` + — categorized ``AGENT_CRASH`` with ``max_retries=2``, so ONE mistyped + bucket burns three full attempts and lands the task as ERROR. + + That is the opposite of the policy every neighbouring field follows: + ``_epoch_ms_to_dt`` type-checks, ``state``/``input``/``cost``/``total`` are + all ``isinstance``-gated, and ``_fresh_input_slice`` exists specifically to + warn-once on token-schema drift rather than fail. A changed type in the + very same ``tokens`` dict is drift too, so it is reported the same way and + the turn survives on the buckets it could read. It is also what makes + ``_handle_line``'s advertised "Never raises on bad input" true. + """ + if isinstance(value, bool) or not isinstance(value, int | float | str): + if value is not None: + self._warn_token_shape( + "tokens.%s is %r (%s), not a number; counting it as 0 — the CLI's token schema " + + "may have changed, so re-check docs/agents/OPENCODE.md before trusting cost", + bucket, + value, + type(value).__name__, + ) + return 0 + try: + return int(value) + except (TypeError, ValueError): + self._warn_token_shape( + "tokens.%s is %r, which is not convertible to a number; counting it as 0 — the CLI's " + + "token schema may have changed, so re-check docs/agents/OPENCODE.md before trusting cost", + bucket, + value, + ) + return 0 + def _fresh_input_slice( self, tokens: dict[str, Any], raw_in: int, raw_out: int, reasoning: int, cw: int, cr: int ) -> int: @@ -668,14 +711,15 @@ def _fresh_input_slice( def on_step_finish(self, part: dict[str, Any]) -> None: self.steps_finished += 1 + self.step_open = False tokens = part.get("tokens") tokens = tokens if isinstance(tokens, dict) else {} cache = tokens.get("cache") if isinstance(tokens.get("cache"), dict) else {} - 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) + raw_in = self._as_int("input", tokens.get("input") or 0) + raw_out = self._as_int("output", tokens.get("output") or 0) + step_reasoning = self._as_int("reasoning", tokens.get("reasoning") or 0) + step_cw = self._as_int("cache.write", cache.get("write") or 0) + step_cr = self._as_int("cache.read", cache.get("read") or 0) step_in = self._fresh_input_slice(tokens, raw_in, raw_out, step_reasoning, step_cw, step_cr) # Reasoning tokens are billed at the output rate but reported apart from @@ -739,6 +783,21 @@ def on_step_finish(self, part: dict[str, Any]) -> None: ) ) + def on_error(self, part: dict[str, Any]) -> None: + """Record the CLI's own structured error, which ``_settle_turn`` crashes on. + + The payload is the flat envelope (no ``part``), and its shape varies: a + nested ``error.data.message`` when the CLI has one, otherwise the error's + ``name``. Anything else degrades to its string form rather than raising. + """ + error = part.get("error") + if isinstance(error, dict): + data = error.get("data") + message = (data or {}).get("message") if isinstance(data, dict) else None + self.error_message = str(message or error.get("name") or "unknown error") + else: + self.error_message = str(error or "unknown error") + def close_open_tools(self) -> None: """Force-close every tool still awaiting a result (crash/timeout orphans).""" for call_id in list(self.open_tools): @@ -768,6 +827,26 @@ def finalize( cost = self._resolve_cost() if cost is not None: usage = usage.model_copy(update={"total_cost_usd": cost}) + # A step still open here never received its `step_finish` — the turn + # died between the two (crash, timeout, cancel) or was cut cleanly + # (should_stop, max_turns). Either way its TurnStartEvent must be + # closed, or the protocol's one-pair-per-inner-turn contract + # (Agent.communicate) is broken and every renderer shows a turn that + # opens and never ends. Unlike the siblings, the completed steps have + # already closed themselves in `on_step_finish`, so this fires ONLY for + # the straggler. TurnEndStatus mirrors AgentEndStatus value-for-value + # precisely so this conversion is total. + if self.step_open: + self.step_open = False + self.emit( + TurnEndEvent( + task_id=self.task_id, + thread_id=self.thread_id, + turn_id=self.turn_id, + status=TurnEndStatus(status.value), + tokens=None, + ) + ) self.emit( AgentEndEvent( task_id=self.task_id, @@ -806,10 +885,28 @@ class OpenCodeAgent(Agent[OpenCodeAgentConfig]): def __init__( self, config: OpenCodeAgentConfig, + route: ApiRoute | None = None, + *, task_id: str = "unknown", - **_: Any, ) -> None: + """Every parameter the agent factory can pass is DECLARED, not absorbed. + + ``create_agent`` calls ``agent_class(config, route=route, **kwargs)`` through + a ``cast(Any, ...)``, so pyright checks nothing at the call site; a ``**_`` + sink on this side would mean nothing checks it at runtime either. The + orchestrator depends on that TypeError as a signal — it gates + ``cost_log_tags`` on ``supports_cost_log_tags`` precisely "otherwise the + agent-agnostic factory would forward it into ... constructors that don't + declare it and crash with TypeError" — so a mis-gated kwarg must be loud + here rather than silently dropped. + + ``route`` is accepted for factory parity and deliberately unused: the CLI + owns its own provider configuration (see ``docs/agents/OPENCODE.md``), so + the run's Bedrock/Anthropic routing does not apply to it. ``task_id`` only + labels the emitted event stream. + """ self.config = config + self.route = route self.task_id = task_id self.working_directory: str | None = None self._env_path_prepend: list[str] = [] @@ -845,7 +942,7 @@ async def start( + "unconstrained by them; do not rely on them as a boundary (see docs/agents/OPENCODE.md).", ", ".join(ignored), ) - self._skill_dirs = _plugin_skill_dirs(self.config.plugins, log=logger) # type: ignore[arg-type] + self._skill_dirs = _plugin_skill_dirs(self.config.plugins, log=logger) if self._skill_dirs: logger.info( "opencode: injecting %d skill path(s) via %s: %s", @@ -1381,12 +1478,6 @@ def _handle_line(self, line: bytes, state: _OpenCodeTurnState) -> None: 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") + state.on_error(part) else: logger.debug("opencode: unhandled event type %r", event_type) diff --git a/tests/test_opencode_agent.py b/tests/test_opencode_agent.py index 417d8977..9723b03d 100644 --- a/tests/test_opencode_agent.py +++ b/tests/test_opencode_agent.py @@ -23,7 +23,12 @@ import pytest -from coder_eval.agents.opencode_agent import OpenCodeAgent, _OpenCodeTurnState, _unwrap +from coder_eval.agents.opencode_agent import ( + OpenCodeAgent, + _OpenCodeTurnState, + _plugin_skill_dirs, + _unwrap, +) from coder_eval.errors import AgentCrashError, TurnTimeoutError from coder_eval.models import AssistantMessage, OpenCodeAgentConfig, PermissionMode from coder_eval.pricing import calculate_cost @@ -34,6 +39,9 @@ ToolEndEvent, ToolEndStatus, ToolStartEvent, + TurnEndEvent, + TurnEndStatus, + TurnStartEvent, ) @@ -619,6 +627,34 @@ async def test_a_later_event_without_input_never_clears_what_we_have(self, patch assert record.commands[0].parameters == {"command": "ls"} +class TestFactoryContract: + """`create_agent` calls `agent_class(config, route=route, **kwargs)` through a + `cast(Any, ...)`, so pyright checks nothing at the call site. Every parameter + must therefore be DECLARED here, or nothing checks it at runtime either. + """ + + def test_route_is_accepted_positionally_and_by_keyword(self): + config = OpenCodeAgentConfig(type="opencode", model="deepseek/deepseek-v4-pro") + assert OpenCodeAgent(config, route=None).route is None + assert OpenCodeAgent(config, None).route is None + + def test_an_undeclared_kwarg_raises_instead_of_vanishing(self): + """The orchestrator gates `cost_log_tags` on `supports_cost_log_tags` + precisely because an ungated forward must crash with TypeError. A `**_` + sink defeated that: a mis-gated kwarg would be silently dropped, yielding + runs with no cost correlation and no error. + """ + config = OpenCodeAgentConfig(type="opencode", model="deepseek/deepseek-v4-pro") + assert OpenCodeAgent.supports_cost_log_tags is False + with pytest.raises(TypeError): + OpenCodeAgent(config, cost_log_tags={"x-ce-run-id": "r1"}) # type: ignore[call-arg] + + def test_a_mistyped_task_id_raises_instead_of_defaulting(self): + config = OpenCodeAgentConfig(type="opencode", model="deepseek/deepseek-v4-pro") + with pytest.raises(TypeError): + OpenCodeAgent(config, task_i="t1") # type: ignore[call-arg] + + class TestSandboxEnvironment: """`start(env_path_prepend=..., plugin_tools_dir=...)` is the abstract `Agent.start()` contract, and the orchestrator ALWAYS supplies both. @@ -781,6 +817,89 @@ async def test_resolved_paths_are_recorded_for_audit(self, patch_exec, tmp_path) await agent.start(str(tmp_path / "sandbox")) assert agent.get_environment_info()["opencode_skill_paths"] == [str(root / "skills")] + async def test_list_form_manifest_declares_several_dirs(self, patch_exec, tmp_path): + """The docstring promises "a string or a list of strings"; only the string + form was exercised, so the list form could have been broken on arrival.""" + root = tmp_path / "plug" + for sub in ("skills", "extra"): + (root / sub / "s1").mkdir(parents=True) + (root / sub / "s1" / "SKILL.md").write_text("---\nname: s1\n---\n", encoding="utf-8") + (root / ".claude-plugin").mkdir(parents=True) + (root / ".claude-plugin" / "plugin.json").write_text( + json.dumps({"name": "p", "skills": ["./skills", "./extra"]}), encoding="utf-8" + ) + captured = patch_exec(_FakeProcess(HAPPY_STREAM)) + await _run(_agent(plugins=[{"type": "local", "path": str(root)}]), tmp_path / "sandbox") + assert _injected_skill_paths(captured) == [str(root / "skills"), str(root / "extra")] + + async def test_list_form_manifest_ignores_non_string_entries(self, patch_exec, tmp_path): + root = _skill_repo(tmp_path / "plug", manifest=json.dumps({"skills": [123, "./skills", None]})) + captured = patch_exec(_FakeProcess(HAPPY_STREAM)) + await _run(_agent(plugins=[{"type": "local", "path": str(root)}]), tmp_path / "sandbox") + assert _injected_skill_paths(captured) == [str(root / "skills")] + + @pytest.mark.parametrize( + ("manifest", "case"), + [ + ("{ not json at all", "unparseable"), + ('["a", "list"]', "not a JSON object"), + ('{"name": "p"}', "no skills field"), + ('{"name": "p", "skills": 7}', "skills is not a string or list"), + ], + ) + async def test_unusable_manifest_falls_back_to_the_convention(self, patch_exec, tmp_path, manifest, case): + """A manifest we cannot read must not lose the skills — `/skills` is + the convention default, and silently injecting nothing is the exact failure + this whole mapping exists to close.""" + root = _skill_repo(tmp_path / "plug", manifest=manifest) + captured = patch_exec(_FakeProcess(HAPPY_STREAM)) + await _run(_agent(plugins=[{"type": "local", "path": str(root)}]), tmp_path / "sandbox") + assert _injected_skill_paths(captured) == [str(root / "skills")], case + + def test_a_non_local_plugin_entry_is_skipped_with_a_warning(self, tmp_path, caplog): + """Only `type: local` maps to a directory; anything else has no path to + hand OpenCode and must say so rather than vanish. + + Driven through `_plugin_skill_dirs` directly, not the agent: this branch is + defensive only — `LocalPluginConfig` pins `type: Literal["local"]`, so + pydantic rejects any other value before the agent ever sees it. + """ + root = _skill_repo(tmp_path / "plug") + with caplog.at_level("WARNING"): + resolved = _plugin_skill_dirs([{"type": "git", "path": str(root)}, {"type": "local", "path": str(root)}]) + + assert "ignoring non-local plugin entry" in caplog.text + assert resolved == [str(root / "skills")] + + async def test_a_root_with_no_skill_md_warns_but_still_injects(self, patch_exec, tmp_path, caplog): + """A directory holding no `/SKILL.md` is suspicious, not fatal — the + CLI scans `skills.paths` recursively, so the path is still injected and the + warning tells the author to check that the plugin path is a skills root.""" + root = tmp_path / "plug" + (root / "skills").mkdir(parents=True) + patch_exec(_FakeProcess(HAPPY_STREAM)) + agent = _agent(plugins=[{"type": "local", "path": str(root)}]) + with caplog.at_level("WARNING"): + await agent.start(str(tmp_path / "sandbox")) + + assert "no /SKILL.md directly under" in caplog.text + assert agent._skill_dirs == [str(root / "skills")] + + @pytest.mark.parametrize("inherited", ["{ not json", '["a", "list"]', '"a string"']) + async def test_unusable_inherited_config_is_replaced_with_a_warning( + self, patch_exec, tmp_path, monkeypatch, caplog, inherited + ): + """An inherited value we cannot merge into must not cost us the skills; + replacing it is announced so the host knows its config was dropped.""" + root = _skill_repo(tmp_path / "plug") + monkeypatch.setenv("OPENCODE_CONFIG_CONTENT", inherited) + captured = patch_exec(_FakeProcess(HAPPY_STREAM)) + with caplog.at_level("WARNING"): + await _run(_agent(plugins=[{"type": "local", "path": str(root)}]), tmp_path / "sandbox") + + assert _injected_skill_paths(captured) == [str(root / "skills")] + assert "replacing it with the injected skill paths" in caplog.text + class TestUnsupportedConfigIsAnnounced: async def test_start_warns_about_unenforced_fields(self, patch_exec, tmp_path, caplog): @@ -844,6 +963,100 @@ async def test_second_turn_resumes_session(self, patch_exec, tmp_path): assert argv[argv.index("--session") + 1] == SESSION +class TestErrorEventShapes: + """`error` is the CLI's own flat envelope, and its payload shape varies.""" + + @staticmethod + def _state() -> _OpenCodeTurnState: + return _OpenCodeTurnState(task_id="t1", iteration=1, user_input="p", model="m") + + @pytest.mark.parametrize( + ("payload", "expected"), + [ + ({"error": {"data": {"message": "provider refused"}, "name": "ProviderError"}}, "provider refused"), + ({"error": {"name": "UnknownError"}}, "UnknownError"), # no data.message -> the name + ({"error": {"data": None, "name": "UnknownError"}}, "UnknownError"), + ({"error": {"data": "not-a-dict", "name": "UnknownError"}}, "UnknownError"), + ({"error": {}}, "unknown error"), + ({"error": "plain string"}, "plain string"), + ({}, "unknown error"), + ], + ) + def test_message_extraction(self, payload, expected): + state = self._state() + state.on_error(payload) + assert state.error_message == expected + + +class TestTokenCastsNeverRaise: + """`_handle_line` advertises "Never raises on bad input"; these five casts were + the module's only unguarded field reads, and every neighbouring field already + warns-and-continues on drift rather than failing. + + Raising here is expensive: `communicate`'s `except Exception` turns it into an + AgentCrashError, categorized AGENT_CRASH with max_retries=2, so ONE mistyped + bucket burns three full attempts and lands the task as ERROR. + """ + + @staticmethod + def _stream(tokens: dict[str, Any]) -> list[str]: + return [ + _evt("step_start", {"id": "prt_1", "messageID": "msg_1", "type": "step-start"}), + _evt("step_finish", {"id": "prt_2", "messageID": "msg_1", "reason": "stop", "tokens": tokens}), + ] + + @pytest.mark.parametrize( + "tokens", + [ + {"input": "abc", "output": 20, "total": 120}, # ValueError on int() + {"input": {"nested": 1}, "output": 20}, # TypeError on int() + {"input": [5], "output": 20}, # TypeError on int() + {"input": 100, "output": 20, "cache": {"read": "lots", "write": None}}, + ], + ) + async def test_a_non_numeric_bucket_warns_instead_of_crashing(self, patch_exec, tmp_path, tokens, caplog): + patch_exec(_FakeProcess(self._stream(tokens))) + with caplog.at_level("WARNING"): + record = await _run(_agent(), tmp_path) + + assert record.crashed is False + assert "unexpected token accounting" in caplog.text + # The buckets that WERE readable still land. + assert record.token_usage is not None + + async def test_numeric_strings_are_still_accepted(self, patch_exec, tmp_path, caplog): + """A stringly-typed but numeric count is a serialization detail, not drift.""" + patch_exec(_FakeProcess(self._stream({"input": "100", "output": "20", "total": 120}))) + with caplog.at_level("WARNING"): + record = await _run(_agent(), tmp_path) + + assert record.token_usage is not None + assert record.token_usage.uncached_input_tokens == 100 + assert record.token_usage.output_tokens == 20 + assert "unexpected token accounting" not in caplog.text + + async def test_a_float_count_truncates(self, patch_exec, tmp_path, caplog): + """JSON has one number type, so a provider may serialize a count as 100.0.""" + patch_exec(_FakeProcess(self._stream({"input": 100.0, "output": 20.7, "total": 120}))) + with caplog.at_level("WARNING"): + record = await _run(_agent(), tmp_path) + + assert record.token_usage is not None + assert record.token_usage.uncached_input_tokens == 100 + assert record.token_usage.output_tokens == 20 + assert "unexpected token accounting" not in caplog.text + + async def test_a_bool_is_not_a_token_count(self, patch_exec, tmp_path, caplog): + """`int(True) == 1` would book a phantom token.""" + patch_exec(_FakeProcess(self._stream({"input": True, "output": 20}))) + with caplog.at_level("WARNING"): + record = await _run(_agent(), tmp_path) + + assert record.token_usage is not None + assert record.token_usage.uncached_input_tokens == 0 + assert "unexpected token accounting" in caplog.text + + class TestFailurePaths: async def test_error_event_raises_and_parks_partial(self, patch_exec, tmp_path): stream = [ @@ -1369,6 +1582,91 @@ async def test_cancel_parks_partial_and_reraises(self, patch_exec, tmp_path): assert proc.killed is True # not abandoned mid-stream — see TestTurnAlwaysReapsTheCli +class TestTurnEventsAreBalanced: + """`Agent.communicate`'s contract is one TurnStart/TurnEnd pair per inner turn. + + `on_step_start` opens one per CLI step and `on_step_finish` closes it, but a + turn that dies (or is cut) between the two left the last TurnStartEvent open + forever — a task.log with `>>> Turn start` and no matching `--- Turn end`. + All three sibling agents close it from `finalize`. + """ + + @staticmethod + def _pairs(recorder: _EventRecorder) -> tuple[int, int]: + starts = len([e for e in recorder.events if isinstance(e, TurnStartEvent)]) + ends = len([e for e in recorder.events if isinstance(e, TurnEndEvent)]) + return starts, ends + + async def test_a_clean_turn_is_balanced(self, patch_exec, tmp_path): + patch_exec(_FakeProcess(HAPPY_STREAM)) + recorder = _EventRecorder() + await _run(_agent(), tmp_path, stream_callback=recorder) + assert self._pairs(recorder) == (2, 2) # HAPPY_STREAM is two steps + + async def test_a_timeout_closes_the_open_step(self, patch_exec, tmp_path): + proc = _HangingProcess([_evt("step_start", {"id": "prt_1", "messageID": "msg_1"})]) + patch_exec(proc) + recorder = _EventRecorder() + + with pytest.raises(TurnTimeoutError): + await _run(_agent(), tmp_path, timeout=0.2, stream_callback=recorder) + + assert self._pairs(recorder) == (1, 1) + end = next(e for e in recorder.events if isinstance(e, TurnEndEvent)) + assert end.status is TurnEndStatus.TIMEOUT + assert end.turn_id == "msg_1" + + async def test_a_cancel_closes_the_open_step(self, patch_exec, tmp_path): + proc = _HangingProcess([_evt("step_start", {"id": "prt_1", "messageID": "msg_1"})]) + patch_exec(proc) + agent = _agent() + await agent.start(str(tmp_path)) + recorder = _EventRecorder() + + task = asyncio.ensure_future(agent.communicate("do the thing", stream_callback=recorder)) + await asyncio.sleep(0.05) + task.cancel() + with pytest.raises(asyncio.CancelledError): + _ = await task + + assert self._pairs(recorder) == (1, 1) + end = next(e for e in recorder.events if isinstance(e, TurnEndEvent)) + assert end.status is TurnEndStatus.CRASHED + + async def test_a_crash_closes_the_open_step(self, patch_exec, tmp_path): + patch_exec(_ExplodingProcess([_evt("step_start", {"id": "prt_1", "messageID": "msg_1"})])) + recorder = _EventRecorder() + + with pytest.raises(AgentCrashError): + await _run(_agent(), tmp_path, stream_callback=recorder) + + assert self._pairs(recorder) == (1, 1) + + async def test_a_clean_cut_closes_the_open_step(self, patch_exec, tmp_path): + """should_stop and max_turns cut between a step's start and its finish too.""" + proc = _RunningProcess([_evt("step_start", {"id": "prt_1", "messageID": "msg_1"})]) + patch_exec(proc) + recorder = _EventRecorder() + + await _run(_agent(), tmp_path, should_stop=lambda: True, stream_callback=recorder) + + assert self._pairs(recorder) == (1, 1) + end = next(e for e in recorder.events if isinstance(e, TurnEndEvent)) + assert end.status is TurnEndStatus.STOPPED_EARLY + + async def test_a_completed_step_is_never_closed_twice(self, patch_exec, tmp_path): + """Unlike the siblings, completed steps close themselves in `on_step_finish`, + so `finalize` must fire ONLY for a straggler.""" + patch_exec(_FakeProcess(HAPPY_STREAM)) + recorder = _EventRecorder() + record = await _run(_agent(), tmp_path, stream_callback=recorder) + + assert record.crashed is False + starts, ends = self._pairs(recorder) + assert starts == ends == 2 + assert all(e.status is TurnEndStatus.COMPLETED for e in recorder.events if isinstance(e, TurnEndEvent)) + + class TestTurnAlwaysReapsTheCli: """No exit from `communicate()` may leave the CLI running. From 5d492ab62f79c5da84bde8b466b591d13b3bcc41 Mon Sep 17 00:00:00 2001 From: mohsen-uipath Date: Mon, 17 Aug 2026 11:07:58 -0700 Subject: [PATCH 11/12] feat(opencode): add require_token_telemetry, an escape hatch for the zero-token guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- docs/agents/OPENCODE.md | 7 +++++++ src/coder_eval/agents/opencode_agent.py | 15 +++++++++++---- src/coder_eval/models/agent_config.py | 12 ++++++++++++ tests/test_opencode_agent.py | 17 +++++++++++++++++ 4 files changed, 47 insertions(+), 4 deletions(-) diff --git a/docs/agents/OPENCODE.md b/docs/agents/OPENCODE.md index 3b00d1d9..83bd73a2 100644 --- a/docs/agents/OPENCODE.md +++ b/docs/agents/OPENCODE.md @@ -263,6 +263,13 @@ the run really billed. Two shapes reach it: Intentional cuts (`should_stop`, `max_turns`) are exempt: both can land before the first event, or between a step's start and its `step_finish`. +For a provider or auth mode that genuinely reports no usage — where failing every +turn would make the harness unusable rather than merely imprecise — set +`require_token_telemetry: false` (or `-D agent.require_token_telemetry=false`). +The turn is then warned about and scored. This relaxes only the missing-token +arm: a stream with **no recognized events** still fails, since no provider quirk +explains a renamed event vocabulary. + ## Running in Docker **Not supported yet — use the default `tempdir` driver.** Unlike the other diff --git a/src/coder_eval/agents/opencode_agent.py b/src/coder_eval/agents/opencode_agent.py index dd570881..a429889e 100644 --- a/src/coder_eval/agents/opencode_agent.py +++ b/src/coder_eval/agents/opencode_agent.py @@ -1388,13 +1388,20 @@ async def _settle_turn( f"It reported {state.steps_finished} finished step(s), none of which carried usable " + f"token counts (cost reported: {'yes' if state.saw_cost else 'no'})." ) - self._crash_turn( - state, - collector, + message = ( f"OpenCode exited cleanly but the turn captured zero token telemetry. {detail} The CLI's " + "event or token schema may have changed — see docs/agents/OPENCODE.md (Telemetry) before " - + "trusting any run from this CLI version.", + + "trusting any run from this CLI version." ) + # Escape hatch for a provider/auth mode that reports no usage at all, + # where crashing every turn would make the harness unusable rather than + # merely imprecise. Deliberately does NOT cover `nothing_recognized`: + # that arm is vocabulary drift, which has silently zeroed a whole run + # before, and no provider quirk can explain it. + if not self.config.require_token_telemetry and not nothing_recognized: + logger.warning("opencode: %s Scored anyway — require_token_telemetry is off.", message) + else: + self._crash_turn(state, collector, message) if stopped_early: return AgentEndStatus.STOPPED_EARLY diff --git a/src/coder_eval/models/agent_config.py b/src/coder_eval/models/agent_config.py index 91cb8c03..0a56137b 100644 --- a/src/coder_eval/models/agent_config.py +++ b/src/coder_eval/models/agent_config.py @@ -308,6 +308,18 @@ class OpenCodeAgentConfig(BaseAgentConfig): "the Claude agent's `setting_sources: []`. Set False to load host plugins." ), ) + require_token_telemetry: bool = Field( + default=True, + description=( + "Fail a turn that finished steps but captured no token counts, instead of scoring " + "it. On by default: such a turn is missing from every token aggregate and its " + "max_total_tokens / max_usd budget gates can never trip. Set False only for a " + "provider or auth mode that genuinely reports no usage, where crashing every turn " + "would make the harness unusable — the turn is then warned about and scored. This " + "never relaxes the separate event-vocabulary check (a stream with no recognized " + "events still fails), since no provider quirk explains that." + ), + ) class NoneAgentConfig(BaseAgentConfig): diff --git a/tests/test_opencode_agent.py b/tests/test_opencode_agent.py index 9723b03d..1b852588 100644 --- a/tests/test_opencode_agent.py +++ b/tests/test_opencode_agent.py @@ -1197,6 +1197,23 @@ async def test_cost_without_tokens_still_crashes(self, patch_exec, tmp_path): with pytest.raises(AgentCrashError, match="cost reported: yes"): await _run(_agent(), tmp_path) + async def test_require_token_telemetry_false_warns_and_scores(self, patch_exec, tmp_path, caplog): + """The escape hatch, for a provider that genuinely reports no usage: crashing + every turn there would make the harness unusable, not merely imprecise.""" + patch_exec(_FakeProcess(self._stream_without_tokens())) + with caplog.at_level("WARNING"): + record = await _run(_agent(require_token_telemetry=False), tmp_path) + + assert record.crashed is False + assert "require_token_telemetry is off" in caplog.text + + async def test_the_hatch_never_relaxes_the_vocabulary_check(self, patch_exec, tmp_path): + """Vocabulary drift has silently zeroed a whole run before, and no provider + quirk explains it — so this arm stays fatal even with the hatch open.""" + patch_exec(_FakeProcess([json.dumps({"type": "session.next.idle", "properties": {"sessionID": SESSION}})])) + with pytest.raises(AgentCrashError, match="no recognized events"): + await _run(_agent(require_token_telemetry=False), tmp_path) + async def test_a_cut_before_any_step_finished_is_exempt(self, patch_exec, tmp_path): """The arm keys on a step the CLI reported FINISHED. A stop landing between a step's start and its `step_finish` is an intentional cut, not drift.""" From e0487de014e8d9c471f9b59e7b07db76af9ad623 Mon Sep 17 00:00:00 2001 From: mohsen-uipath Date: Mon, 17 Aug 2026 12:31:15 -0700 Subject: [PATCH 12/12] fix(opencode): map apply_patch to Write so GPT-family edits are seen by criteria MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OpenCode exposes a provider-specific tool set, so the tool vocabulary varies by MODEL within this one harness — not just across harnesses, which is what _TOOL_NAME_MAP was built for. A live 174-task run makes it concrete: DeepSeek V4 Pro : write=144 edit=55 apply_patch=0 GPT-5.6 Luna : write=0 edit=0 apply_patch=120 `apply_patch` was unmapped, so it reached CommandTelemetry under its raw name. Every `command_executed` criterion keyed on `tool_name: Write` or `tool_name: Edit` therefore scores 0 on a GPT-family model that edited the file correctly — and that suite carries 33 Write and 30 Edit such criteria. Mapped to `Write`, matching codex_agent's `_TOOL_ITEM_NAMES["apply_patch"]`, so one criterion reads the same on either harness. Measured blast radius on the run that surfaced it: zero. All 76 Luna tasks that used apply_patch had their failures elsewhere (llm_judge 2, run_command 4) and not one command_executed criterion among them — so the published comparison was not distorted. This is a latent scoring gap being closed, not a correction to those numbers. Its argument is a patch envelope (`patchText`), not a file path, so no arg rename applies and a `command_pattern` written against `file_path` still will not match it. Documented in OPENCODE.md alongside the guidance to assert on `tool_name` alone or on the resulting file. Mutation-checked: removing the entry fails the new test. make verify green: 4,226 tests, coverage gate met. --- docs/agents/OPENCODE.md | 8 ++++++++ src/coder_eval/agents/opencode_agent.py | 9 +++++++++ tests/test_opencode_agent.py | 15 +++++++++++++++ 3 files changed, 32 insertions(+) diff --git a/docs/agents/OPENCODE.md b/docs/agents/OPENCODE.md index 83bd73a2..31921b3d 100644 --- a/docs/agents/OPENCODE.md +++ b/docs/agents/OPENCODE.md @@ -243,6 +243,14 @@ Both file-path spellings are accepted because the CLI has moved between versions `Bash`'s `command` and the search tools' `path` already match Claude's names and pass through untouched, as does every unlisted key. +> **The tool vocabulary varies by MODEL, not just by harness.** OpenCode exposes a +> provider-specific tool set: a GPT-family model edits via `apply_patch` where +> DeepSeek uses `write`/`edit`. `apply_patch` is therefore mapped to `Write` (as +> `codex_agent` does), so one `tool_name: Write` criterion reads the same on +> either. Its argument is a patch envelope (`patchText`), not a file path, so a +> `command_pattern` written against `file_path` will not match it — assert on +> `tool_name` alone, or on the resulting file with `file_exists`/`file_contains`. + > These event names are the CLI's own compact vocabulary. They are **not** the > `session.next.*` names in the OpenAPI schema served by `opencode serve` — that > describes the HTTP/SSE surface and does not apply here. diff --git a/src/coder_eval/agents/opencode_agent.py b/src/coder_eval/agents/opencode_agent.py index a429889e..dec1da2a 100644 --- a/src/coder_eval/agents/opencode_agent.py +++ b/src/coder_eval/agents/opencode_agent.py @@ -140,6 +140,15 @@ "todowrite": "TodoWrite", "todoread": "TodoRead", "task": "Agent", + # The GPT-family edit tool. OpenCode exposes a provider-specific tool set, so + # the vocabulary varies by MODEL within this one harness: a live 174-task run + # showed DeepSeek using write/edit 199 times and apply_patch 0, while GPT-5.6 + # used apply_patch 120 times and write/edit 0. Unmapped, every + # `tool_name: Write` / `tool_name: Edit` criterion scores 0 on a GPT-family + # model that edited the file correctly. Maps to `Write` to match codex_agent's + # `_TOOL_ITEM_NAMES["apply_patch"] = "Write"`, so one criterion reads the same + # on both harnesses. + "apply_patch": "Write", # OpenCode's native skill loader. Without this entry `skill_triggered` (which # keys on the canonical `Skill`) and any `command_executed` written against # `tool_name: Skill` read false on every OpenCode run — the engagement happened diff --git a/tests/test_opencode_agent.py b/tests/test_opencode_agent.py index 1b852588..329f0d56 100644 --- a/tests/test_opencode_agent.py +++ b/tests/test_opencode_agent.py @@ -474,6 +474,21 @@ async def test_native_names_map_to_canonical(self, patch_exec, tmp_path): record = await _run(_agent(), tmp_path) assert [c.tool_name for c in record.commands] == ["Bash", "Write"] + async def test_gpt_family_apply_patch_maps_to_write(self, patch_exec, tmp_path): + """OpenCode's tool set is provider-specific, so the vocabulary varies by + MODEL within this one harness. + + A live 174-task run showed DeepSeek using write/edit 199 times and + apply_patch 0, while GPT-5.6 used apply_patch 120 times and write/edit 0. + Unmapped, every `tool_name: Write` / `tool_name: Edit` criterion scores 0 + on a GPT-family model that edited the file correctly — the suite in that + run carries 33 Write and 30 Edit criteria. Mirrors codex_agent's + `_TOOL_ITEM_NAMES["apply_patch"] = "Write"`. + """ + patch_exec(_FakeProcess([self._tool_with_input("apply_patch", {"patchText": "*** Begin Patch\n"})])) + record = await _run(_agent(), tmp_path) + assert [c.tool_name for c in record.commands] == ["Write"] + async def test_unknown_tool_passes_through(self, patch_exec, tmp_path): """An unmapped tool still surfaces under its own name rather than vanishing.""" patch_exec(_FakeProcess([self._tool_event("some_new_tool")]))