Skip to content

feat: TurnEmitter and the ports — one turn kernel on every harness (plan 3 of 3) - #184

Open
uipreliga wants to merge 25 commits into
feat/harness-central-enforcementfrom
feat/harness-turn-emitter-and-ports
Open

uipreliga wants to merge 25 commits into
feat/harness-central-enforcementfrom
feat/harness-turn-emitter-and-ports

Conversation

@uipreliga

Copy link
Copy Markdown
Collaborator

Stacked PR. Base is feat/harness-central-enforcement (#183, plan 2 of 3), which is itself stacked on feat/harness-contract (#181, plan 1 of 3). Merge #181, then #183; GitHub then retargets this PR. This diff shows only plan 3.

Implements plan 3 of 3, TurnEmitter and the ports (c/2026-09-18-turn-emitter-and-ports.md, design c/harness-target-architecture-v2.md). Every harness now records its turn through one kernel, so the telemetry means the same thing on every harness. The five per-adapter turn-state accumulators, and the base-class side channel they fed, are gone.

What changes

  1. One cost rule. pricing.price_turn(usage, models): a finite non-zero reported cost wins, otherwise the rate card, otherwise None. The turn monitor and all five adapters use it. CE071 keeps calculate_cost out of agents/ and turn_monitor.py.
  2. TurnEmitter, the sole writer of the event protocol (streaming/emitter.py). It owns open tools, sequence numbers, the transcript, reported usage, the inner turn, the orphan sweep, the one AgentEndEvent, and the record. It is stamped from one clock per HarnessContract.timing_basis (turn_clock for claude-code, pi and antigravity; cli_epoch_ms for codex and opencode). Window, Generation and TurnOutcome are frozen values. Runtime guards reach plugins too: basis TypeError, inner-turn and before-begin() RuntimeError, idempotent finalize / fail, and a WARNING when inner-turn token deltas exceed the published usage.
  3. communicate(..., iteration=) -> TurnOutcome. A crash or a timeout is an outcome with a crashed=True record, not an exception. The orchestrator owns the iteration: it appends a CRASHED/TIMEOUT record and then raises through record_or_raise, so a crash still retries and a timeout still does not. An external cancel ends the turn and re-raises; the orchestrator recovers that record from a per-attempt EventCollector. run_with_watchdog runs the turn body as a child task, so the watchdog never cancels the caller.
  4. The five ports.
    • Pi and OpenCode share SubprocessJsonlAgent (spawn, stderr drain, read loop, settle, kill, process-group sweep, reap).
    • Antigravity, Codex and Claude Code each have a per-turn _<Kind>Decoder.
    • Claude and Codex sub-agent events are nested under the spawning call (parent_thread_id). So max_tool_calls, armed criteria and model_used see the main thread only, while budgets and TurnRecord.commands still include sub-agent work.
    • .claude/notes/agents.md § Transport bases records the decision not to add a second base.
  5. The 300 s stall is fixed. A CLI never inherits stdin: stdin=DEVNULL at every spawn, including authored pre_run / post_run commands and docker run. CE073 enforces it.
  6. coder_eval.testing: replay, assert_identity_closes, assert_stream_balanced and conformance, with no pytest import. The in-tree suites and a plugin's CI use the same checks. Every golden stream is balanced, and the ms-exact identity contract runs on replay for all five harnesses.
  7. Guardrails. CE059, CE060, CE061, CE063 and CE064 are retired: they guarded the accumulators the emitter replaced. CE072 keeps adapters from constructing events, AssistantMessage or EventCollector. The second pyright pass now type-checks tests/*_live.py and the BYOA demo plugin. tests/test_harness_live.py runs one tiny turn per installed harness.

Breaking changes (greenfield, no aliases)

  • SPI 3:
    • communicate(user_input, *, iteration, ...) returns TurnOutcome.
    • HarnessContract.timing_basis is required.
    • Agent.pending_turn, _iteration, discard_pending_turn, _begin_turn, _end_turn_ok and the _finalize_and_raise_* kernels are deleted.
    • coder_eval.spi no longer exports EventCollector, the seven event classes or CompositeStreamCallback. It gains TurnEmitter, TurnOutcome, Generation, Window, TimingBasis, run_with_watchdog, WatchdogFired, SubprocessJsonlAgent, JsonlDecoder, ContentBlock, price_turn and format_timeout_reason.
    • docs/EXTENDING.md has the checklist and a SubprocessJsonlAgent example. The out-of-tree Delegate agent needs a follow-up PR.
  • Reviewed record changes (golden diffs are quoted in each phase's commit):
    • result_summary is filled on every clean turn (status plus final reply) and is null on a failed one.
    • sequence_number is 0-based and assistant_turn_index is derived from the messages.
    • An orphaned tool keeps execution_started_at, with no completion stamp and no duration.
    • A Codex item with no stamps has an unmeasured generation (null window, head and tail).
    • A completed Codex item with no status records success, and a declined command records error (was unknown).
    • A duplicate tool result: the first result is kept.
    • The Antigravity prompt no longer appears in agent_output.

Verification

  • make verify: 6319 passed, coverage 93.57%, both pyright passes clean, make lint 678 passed. make evalboard-verify: 805 passed.
  • Live, on all five harnesses (Haiku; Pi/OpenCode via OpenRouter Haiku; Antigravity gemini-3.5-flash-lite; Codex from .env). Run dirs are local under tmp/p10/runs/<step>_<harness>.
Step claude-code codex pi opencode antigravity
1 smoke SUCCESS SUCCESS SUCCESS SUCCESS SUCCESS
3 max_tool_calls_cap SUCCESS, capped at 4 SUCCESS, capped at 4 SUCCESS, capped at 4 SUCCESS, capped at 4 SUCCESS, capped at 4
5 token budget TOKEN_BUDGET_EXCEEDED TOKEN_BUDGET_EXCEEDED ³ TOKEN_BUDGET_EXCEEDED ² TOKEN_BUDGET_EXCEEDED ² TOKEN_BUDGET_EXCEEDED ²
5 cost budget COST_BUDGET_EXCEEDED COST_BUDGET_EXCEEDED ³ COST_BUDGET_EXCEEDED ² COST_BUDGET_EXCEEDED ² COST_BUDGET_EXCEEDED ²
6 turn_timeout ERROR (timed out at 45 s) ¹ ERROR (timed out at 45 s) ERROR (timed out at 45 s)
7 skill_offered SUCCESS SUCCESS SUCCESS SUCCESS SUCCESS
  • Step 1 details: the timing basis matches each contract; every command's assistant_turn_index points at the message that names it; result_summary.result is the final reply; no agent_output contains the prompt. decompose_run.py --max-residual-pct 5: all 31 gateable turns across every run are within 5% (worst single turn 0.077 ms).
  • Step 2, the stall: Pi 9 s and OpenCode 9 s alone with stdin held open (< <(sleep 900)). Eight concurrent runs with stdin held open: 8/8 SUCCESS in 15 s, no retry. Before the fix this batch stalled 300 s.
  • Step 4, sub-agent scope: tasks/run_limits/subagent_cap.yaml (cap 2) ends SUCCESS: answer.txt = 5050, commands Agent + sub-agent Bash + Write, model_used = Haiku. Before the fix this task ended TOOL_CALLS_EXHAUSTED. codex_subagent_test ends SUCCESS with the Agent calls recorded.
  • Steps 5 and 6: no emitter delta WARNING appeared in any run. Each timeout is one crashed: true partial, logged non-retryable. ERROR is the orchestrator's unchanged mapping for a turn timeout.
  • Step 8: the early stop fired decision_budget_exceeded at tool call 3 (FAILURE by design). The dialog ran 3 turns and stopped on the stop token.
  • Live tests: tests/test_harness_live.py 5/5, tests/test_codex_agent_live.py 5/5. Hard-killed turn: smoke_task_timeout ends TIMEOUT with the killed turn's record and cost recovered.

¹ With the fixture's sleep spelled through python3 (see below).
² With -D agent.permission_mode=bypassPermissions -D agent.allowed_tools=null: the fixture carries Claude-only fields.
³ From a copy of the fixture without those fields, because -D cannot unset permission_mode.

  • Not verified live: tests/test_byoa_plugin_live.py and tests/test_claude_settings_enforcement_live.py (no ANTHROPIC_API_KEY). Every llm_judge criterion (smoke_llm_judge, the simulated-judged dialog) scored 0 for lack of a judge credential.

Defects the live runs caught (fixed here)

  • The current Claude Code CLI refuses a standalone sleep N, so turn_timeout.yaml and smoke_task_timeout.yaml never reached a timeout on Claude. smoke_task_timeout was passing its smoke-fail bucket as a plain FAILURE. Both fixtures now sleep through python3.
  • The Claude case in tests/test_harness_live.py ran under acceptEdits, and Haiku wrote to a mangled tmp path outside the working directory. It now uses bypassPermissions.

Follow-ups

  • Delegate out-of-tree PR for SPI 3: timing_basis, a TurnEmitter in place of its turn state, iteration= and TurnOutcome, imports only from coder_eval.spi, and coder_eval.testing in its CI. Revisit a shared SDK-adapter base then (notes § Transport bases).
  • Deferred harness candidates, in .claude/harness-candidates.md:
    • an unbounded stderr read in the JSONL transport;
    • tool ids keyed across the whole task while adapters mint them per invocation;
    • assert_stream_balanced and interleaved sub-agent turns;
    • CE072 false positives;
    • Claude-only fields in the budget smoke fixtures;
    • an unknown-id Claude tool result counted on the main thread.

🤖 Generated with Claude Code

uipreliga and others added 13 commits September 16, 2026 16:45
…d the turn monitor; CE071

Every adapter and TurnMonitor._price call pricing.price_turn. A reported $0 on a
priced model is repriced from the rate card everywhere (the monitor and Claude
included); an empty turn keeps its reported cost, so it reads token_usage null
on every harness. CE071 keeps calculate_cost out of agents/ and the monitor.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…al.testing; SPI 3

The kernel the ports build on. close_window returns a Window; every contract
declares timing_basis; TurnEmitter is the one writer of the event protocol;
coder_eval.testing gives replay, identity, balance and conformance sensors.
EventCollector records nested tool ends and derives assistant_turn_index from
the messages; TurnMonitor counts main-thread tool calls only. Every golden
stream is checked with assert_stream_balanced (pi_f is a strict xfail).

Reviewed golden diff (assistant_turn_index only):
  pi_b 1,2 -> 0,1; pi_c, pi_d, opencode_b, opencode_c, opencode_d 1 -> 0;
  codex_b, codex_d, codex_e null -> 0; codex_f null,null -> 0,0;
  antigravity_b, antigravity_c null -> 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tor owns the iteration

communicate(..., iteration=) returns a TurnOutcome: a crash or timeout is an
outcome with a crashed record, not an exception. The orchestrator appends a
CRASHED/TIMEOUT record and raises it through record_or_raise (retry policy
unchanged), and recovers a cancelled turn from a per-attempt EventCollector.
run_with_watchdog runs a turn body as a child task so a watchdog timeout never
cancels the caller. NoOpAgent writes through TurnEmitter; the five unported
adapters keep their bodies behind Agent._legacy_outcome. on_attempt_error,
_drain_pending_turn and _on_attempt_failure are deleted. Goldens unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… stdin

_PiTurnState becomes _PiDecoder, a per-turn reducer that calls the emitter;
PiAgent.communicate returns outcomes directly. The spawn passes
stdin=DEVNULL: pi reads a non-TTY stdin to EOF before it emits anything, so an
inherited open stdin stalled every turn to its deadline. The identity case runs
through coder_eval.testing.replay; pi_f_duplicate_turn_end now balances.

Reviewed golden diffs:
  result_summary: pi_a/b/c result null -> the final reply; pi_e summary -> null.
  sequence_number 1-based -> 0-based: pi_b 1,2 -> 0,1; pi_c, pi_d 1 -> 0.
  pi_d orphan error_message "no result observed" -> null.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…Code is cli_epoch_ms; CE073

The nd-JSON CLI transport (spawn with stdin=DEVNULL, stderr drain, read loop,
settle, kill/reap and their constants) lives once in
agents/_transport/subprocess_jsonl.py. Pi moves onto it; OpenCode is ported onto
TurnEmitter with windows bounded by the CLI envelope timestamp and tool spans
from state.time, so TimingBasis.MIXED is deleted. The orchestrator's pre/post-run
shell and the docker run spawn no longer inherit stdin, and CE073 requires every
asyncio subprocess spawn to decide its stdin. A captured real OpenCode stream is
a new golden. Every opencode scenario is now fictional-duration (ms-scripted CLI
stamps); opencode_c_multi_step_tiling measures a window again.

Reviewed golden diffs:
  result_summary: opencode_a/b/c result null -> the final reply; opencode_e -> null.
  sequence_number 1 -> 0: opencode_b, opencode_c, opencode_d.
  opencode_d orphan: execution_completed_at set -> null, error_message -> null.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ompt is no longer agent output

_AntigravityTurnState becomes _AntigravityDecoder; communicate runs the
send/drain/poll body under run_with_watchdog and returns outcomes. One predicate
(_is_reply: TEXT_RESPONSE from MODEL to TARGET_USER) now gates both the streamed
delta and the completed text, so the prompt echo (source=USER) never becomes
assistant text or agent_output; the new f_user_prompt_step golden showed
'do itDONE.' before the port and 'DONE.' after. A final reply is the text after
the last tool call of the last main-thread message (a live Antigravity turn puts
it there). TurnEmitter.fail() takes the payload overrides finalize takes, and
duration_seconds stays monotonic under the wall clock. Live: antigravity hello
world on gemini-3.5-flash-lite SUCCESS, prompt absent from agent_output.

Reviewed golden diffs:
  result_summary null -> a clean summary on antigravity_a..e (result is the
  final reply: a 'All done.', b 'done', c 'read it', d 'backgrounded', e 'third').
  antigravity_d orphan: execution_completed_at and tool_union_ms set -> null.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…vents

Codex now decodes notifications through a per-turn _CodexDecoder that calls
the emitter; _CodexTurnState and the legacy delegation are gone. The turn
body runs under run_with_watchdog, and recovered sub-agent tools and
messages carry parent_tool_id, so they never reach the main-thread cap.

Emitter: close_tool(reported_duration_ms=) keeps a CLI-reported duration
for a call with no stamps; a tool's timestamp is its execution start when
known.

Reviewed golden diffs:
- codex_a..g: result_summary null -> {is_error: false, subtype: "completed",
  stop_reason: null, result: <final reply or null>}
- codex_f: message_id "codex-1-subagent-1" -> "codex-1-subagent-0"
- codex_a (no item stamps, so an unmeasured generation):
  generation_duration_ms, harness_startup_ms, harness_teardown_ms
  "<scrubbed>" -> null
- codex_h (crash): agent_output "" -> "partial"

Reviewed behaviour change: a completed item's result_status follows its
tool end status (no-status item -> success, declined command -> error;
was unknown).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…gent stream; delete the legacy machinery

Claude Code decodes SDK messages through a per-turn _ClaudeDecoder that
calls the emitter. One inner turn per message id; a sub-agent message's
turn, generation, tools and text are nested under its parent_tool_use_id,
so they never reach the main-thread cap or model_used. Tool durations come
from the emitter's clock (tool_use arrival to result). The pump runs under
run_with_watchdog; crashes and timeouts are outcomes.

Deleted: _ClaudeTurnState, _finalize_commands, _resolve_pending_command,
and the base-class side channel (pending_turn, _iteration,
_legacy_outcome, _begin_turn, _end_turn_ok, discard_pending_turn,
_capture_partial_turn, the _finalize_and_raise kernels). Both Claude
identity-contract cases run on coder_eval.testing.replay.

Emitter: fail(model_used=); an explicit num_turns=None is recorded.

New tasks/run_limits/subagent_cap.yaml (live check for the main-thread
cap). Second-base decision recorded in .claude/notes/agents.md
§ Transport bases: no HostAgent.

Reviewed golden diff:
- claude_f_orphaned_tool: commands[0].execution_started_at
  null -> "<scrubbed>" (an orphan keeps the start the emitter stamped at
  tool_use arrival, as on every other harness)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…064; SPI drops the events; live tests type-checked

- Delete CE059, CE060, CE061, CE063 and CE064: they guarded the per-adapter
  turn accumulators that TurnEmitter replaced. Their ids are retired
  forever (runner note).
- CE072 EmitterSoleWriter: an adapter under src/coder_eval/agents/ may not
  construct an event, an AssistantMessage or an EventCollector, under any
  import spelling from coder_eval.streaming or coder_eval.models.
- coder_eval.spi no longer exports EventCollector, the seven event classes
  or CompositeStreamCallback; tests/test_spi.py pins the final list.
- The second pyright pass type-checks tests/*_live.py and the BYOA demo
  plugin, in make typecheck, make verify and CI; the live-test type
  errors are fixed.
- tests/test_harness_live.py: one tiny turn per installed harness through
  communicate, checked with assert_stream_balanced and the bucket sum.
- Docs: CLAUDE.md, README and docs/index promise, EXTENDING
  (SubprocessJsonlAgent example, coder_eval.testing sensors),
  HARNESS_PARITY and timing notes point at the emitter; two harness
  candidates closed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… bypasses permissions

Haiku wrote ok.txt to a mangled tmp path outside the working directory,
which acceptEdits refused; bypassPermissions is a mode the Claude
contract documents and matches the other harnesses' cases.

Live results (all five harnesses): smoke, cap, budgets and skills pass;
the stdin-open batch of eight finished in 15 s; the sub-agent cap task
ends SUCCESS with the main model; turn timeouts end with one crashed
partial and no retry.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- TurnEmitter refuses every write, finalize and fail before begin(): a
  plugin that forgot begin() published an end with no start.
- ClaudeCodeAgent: a turn stopped before any ResultMessage keeps the
  emitter's default result_summary (status and final reply), as on the
  other four harnesses.
- coder_eval.spi exports ContentBlock, price_turn and
  format_timeout_reason, which a plugin port needs.
- EXTENDING: an SDK-raised CancelledError (caller cancelling() == 0) is a
  CRASHED outcome, not a re-raise.
- Codex tool items return a _ToolEnd with only what close_tool uses; the
  discarded CommandTelemetry (and its stale result_status) is gone.
- ThreadedWatchdog: a timer callback that starts after __exit__ does
  nothing.
- Stale comments in the collector, orchestrator, emitter and antigravity
  agent; testing.conformance raises instead of asserting.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e reaches the timeout

The current Claude Code CLI refuses a standalone `sleep N`, so the turn
ended in seconds and smoke_task_timeout passed its smoke-fail bucket
without any timeout firing. Live: smoke_task_timeout now ends TIMEOUT
with the hard-killed turn recovered; turn_timeout times out at 45 s.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@uipreliga

Copy link
Copy Markdown
Collaborator Author

Refactor review: LOC status, pros and cons (stack #181#183#184)

Base: 396c22cc (#176), the fork point from main. Head: 28c057b2. "SLOC" = code lines only (no blank lines, comments or docstrings).

LOC in src/

The target was missed, and nobody recorded it. The 5 in-tree adapters are 5,430 lines (5,787 with the shared transport). That is the top of the 4,500–5,500 target, and the target also had to include Delegate.

Scope Base Head Change
5 adapters (claude_code, codex, antigravity, opencode, pi) 7,664 5,430 −2,234 (−29%)
Adapters + _transport/ (new) + _skills.py (deleted) 7,766 5,787 −1,979 (−25%), −1,431 SLOC
New kernel code (emitter, turn_monitor, testing, plugin_staging, harness_contract ×2, spi) 0 2,112 +2,112
Old kernel code removed (early_stop, agent, orchestrator, utils, errors/executor) −674
All of src/coder_eval 50,044 49,942 −102 lines / +191 SLOC
tests/ 107,898 113,013 +5,115

Per PR, src/ only:

PR src/ + / − agents/ + / −
#181 harness contract + SPI +969 / −308 +314 / −182
#183 central enforcement +1,360 / −1,329 +227 / −532
#184 TurnEmitter + ports +3,149 / −3,943 +1,813 / −3,534

Result: the refactor moved code and did not save it. About 2,000 adapter lines went into a shared kernel of about 2,100 lines. src/ is the same size, and tests grew by about 5,100 lines. The saving starts only when more adapters (for example Delegate) use the kernel. With 5 adapters, we are at about break-even.

Pros

  1. Settings are enforced or rejected, and never ignored.
    • Before: allowed_tools / disallowed_tools on Pi and OpenCode only logged "set but NOT enforced", and the run still got a score. The Codex tool setting did nothing.
    • Now: Pi, OpenCode and Antigravity enforce the restrictions natively, and permission_mode: plan is read-only on all three.
    • A setting that a harness cannot honor stops plan / run / export before the run costs money.
    • This fixes a validity problem for cross-harness A/B comparisons.
  2. The tool-call cap is the same on every harness. One TurnMonitor counts resolved tool calls for the whole task, across retries and dialog turns. Before, each adapter counted on its own, and Pi and OpenCode allowed one extra call. Live: all 5 harnesses stopped at exactly 4.
  3. Budgets stop the run during a turn.
    • Before: token and USD budgets were checked only after a turn completed. If no cost was reported, the max_usd check was skipped with only a log line.
    • Now: a budget stops the turn when the limit is crossed, and an unpriceable max_usd ends the run as ERROR.
  4. Telemetry and cost follow one set of rules.
    • TurnEmitter is the only writer of events. It replaced five copied per-adapter accumulators, which needed five lint rules (CE059–CE064) to stop them from drifting apart.
    • price_turn is the one cost rule.
  5. A crashed or timed-out turn keeps its record and cost (TurnOutcome with crashed=True).
  6. Sub-agent scope is correct. Claude and Codex sub-agent calls no longer use up the parent's tool-call cap. Budgets still include them.
  7. Plugin staging is shared. One staged plugin root replaces per-harness skill scanners. plan rejects a skill_triggered target that no plugin offers.
  8. The 300 s stall is fixed. A CLI no longer inherits stdin (stdin=DEVNULL at every spawn, enforced by CE073). Live: 8 concurrent runs finished in 15 s.
  9. Extension is easier. There is a public coder_eval.spi and a test kit (coder_eval.testing). A JSONL CLI agent is a small SubprocessJsonlAgent subclass. The live smoke test covers every installed harness.
  10. Bugs fixed on the way:
    • Antigravity pulled the step stream again on any RuntimeError, and hid a failed teardown.
    • The early-stop ceiling could divide by zero.
    • A relative plugin root made Claude Code load no plugin.
    • The timeout fixtures never timed out on Claude.

Cons

  1. No net LOC saving. src/ is −102 lines / +191 SLOC. The LOC target was missed, and the miss was not recorded.
  2. Tests grew by about 5,100 lines. That is more code to maintain.
  3. feat: TurnEmitter and the ports — one turn kernel on every harness (plan 3 of 3) #184 has the weakest ratio of value to cost.
    • It is the largest PR (+3,149 / −3,943 in src/, +6,733 / −4,551 in tests/).
    • Its main user-visible gains could have been smaller changes: the stall fix is stdin=DEVNULL, and crash-record recovery could have been separate.
    • Most of its value (one writer, SPI, test kit) pays back only if more harnesses are added.
  4. SPI 3 breaks out-of-tree agents. Delegate (coder_eval_uipath) must declare usage_granularity and timing_basis, and must move to TurnEmitter, iteration= and TurnOutcome. This port is not done.
  5. Open items are in .claude/harness-candidates.md:
    • an unbounded stderr read in the JSONL transport;
    • tool ids keyed across the whole task;
    • interleaved sub-agent turns in assert_stream_balanced;
    • CE072 false positives;
    • Claude-only fields in the budget smoke fixtures.

Verdict

Overall, the stack was worth it. Most of the value is in #181 and #183: settings are enforced or rejected, and caps and budgets have the same meaning on every harness. For an evaluation tool, that is a correctness fix, not a cleanup.

#184 is a bet on more harnesses. It is justified only if new adapters stay small. Recommendations:

  • Record the LOC miss with the numbers above ("moved, not saved").
  • Do the Delegate SPI 3 port as part of this stack, not as a follow-up.
  • Check the next new adapter against a size limit, for example under about 800 lines, to confirm that the kernel pays back.

🤖 Generated with Claude Code

uipreliga and others added 11 commits September 16, 2026 22:07
… resolution gate

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… model turn starts

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…odel-turn count

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e model-turn cap fixture

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…whole plugin

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… turns are counted and is rejected elsewhere

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The plugin copy fallback follows links on a symlink-less host and skips a link loop;
stale canonical-root and cap wording; harness candidates from the final review.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s mock_path_dirs on every harness

The criterion PATH was read back from the agent after each successful turn,
through get_sdk_options(), which only Claude Code implements. On Pi, OpenCode,
Codex and Antigravity a run_command criterion did not see the mock binaries
the agent saw; on Claude Code the same gap occurred after a crash or timeout,
before the first turn, and in a detached grade.

The sandbox now sets the prefix at setup and adopt from resolved_mock_path_dirs,
the same list the orchestrator passes to the agent as env_path_prepend.
_sync_sandbox_command_path_with_agent, _restore_recorded_command_path and
_sanitize_restored_path are deleted: the prefix lives inside the workspace,
which the restore always dropped, so persisting it bought nothing.

Confirmed on tasks/mock_path_dirs_smoke.yaml under --type pi (now with a
run_command criterion): exit 127 before, 3/3 after.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…timing, OpenCode rules and plan

- transport: detect CLI exit by returncode, not Process.wait() alone (3.13 waits for
  every pipe), so a child holding stdout no longer holds a clean turn; SIGTERM grace
  1.0 s, below the orchestrator backstop, so a slow SIGTERM ends TIMEOUT, not
  "turn cancelled"; sweep the process group at the end of every turn
- emitter: a repeated open or close of a tool id already closed in the turn is ignored
- monitor: an empty turn with a NaN reported cost costs 0
- codex: a generation with only one stamp is unmeasured, never host-clocked; a
  minted tool id keeps its CLI start stamp; golden fixtures send started_at_ms on
  item/started, as the SDK does
- opencode: an inherited external_directory / doom_loop rule is placed after "*": "deny"
- plan: resolves each variant through resolve_variant_task, the same path as run

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rceable max_usd, SPI checks, harness version

- retry: an AGENT_CRASH whose attempt made a tool call is not retried; the
  sandbox is not reset, so a retry would grade a changed tree (P1-8)
- emitter: finalize(COMPLETED) on a turn that wrote nothing ends CRASHED, on
  every harness; a requested stop is exempt (P1-5)
- max_usd: HarnessContract.reports_cost; on a harness without it, max_usd needs
  a priced agent.model at resolution, and unpriced in-flight usage latches
  "not enforceable" instead of $0 (P1-13)
- SPI: AgentRegistry.register requires spi_version=; a failing plugin stops the
  load with PluginLoadError; stop_conformance is exported from coder_eval.testing
- Agent.harness_version() is recorded as environment_info.harness_version (P1-11)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Versions 1-3 were bumps inside this unreleased branch; no plugin was built
against them. The first released SPI is version 1.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@uipreliga uipreliga left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Review: coder_eval — branch origin/main...HEAD — PR stack #181#183#184 (342 files) --post-comment → #184

Scope: branch origin/main...HEAD — PR stack #181#183#184 (342 files) --post-comment → #184 · branch feat/harness-turn-emitter-and-ports · 6a86e76 · 2026-09-17T15:25Z · workflow variant

Change class: complex — replaces every agent's turn machinery with a shared TurnEmitter, adds a central TurnMonitor for caps/budgets, a harness contract + plugin SPI, and plugin staging; control flow, public SPI and persisted result fields change

coder_eval is in good shape (8.5/10): security, tests and error handling score well, and every finding was checked against the code. The real risks are three changes that can alter a task's reported outcome or a trend metric for the same agent output (unpriced max_usd turns, a cap in sdk_options.max_turns that no TurnMonitor tracks, and a new default for the final-reply metric). Other risks are a hidden agents-to-orchestration import cycle and branch complexity that is growing in the orchestrator. Bottom line: fix the three outcome-changing defects before release and before the SPI v1 contract is frozen, then pay down the structure debt.

Summary

Axis Score 🔴 🟠 🟡 🔵 Top Issue
1. Code Quality & Style 7.1 / 10 0 1 3 4 High cyclomatic complexity in new or changed functions (orchestrator _setup/_simulation_dialog_loop, OpenCodeAgent._inject_config_content, assert_stream_balanced)
2. Type Safety 8.9 / 10 0 1 0 1 agent.by_type is an untyped dict: a misspelled kind or field is silently ignored
3. Test Health 9.5 / 10 0 0 1 0 Failure and timeout branches in adapters and testing sensors have no tests
4. Security 9.9 / 10 0 0 0 1 The consent text for container grading does not list the new skill-source mounts, so an operator approves an incomplete list of host paths
5. Architecture & Design 7.4 / 10 0 1 3 1 New agents <-> orchestration import cycle, hidden behind function-local imports
6. Error Handling & Resilience 8.8 / 10 0 0 2 2 The new BudgetUnenforceableError has no error category, so task.json records it as 'unknown'
7. API Surface & Maintainability 8.7 / 10 0 0 2 3 run_limits.max_turns / expected_turns reuse their origin/main key names with a new meaning (model turns across the whole task), so stale YAML still loads on Claude Code, OpenCode and Pi
8. Evaluation Harness Quality 7.9 / 10 0 1 2 1 On a Claude Code task with max_usd and a model missing from pricing.py, a crashed or stopped attempt (no ResultMessage) latches unpriced and turns the finished row into ERROR

Overall Score: 8.5 / 10 · Weakest Axis: Code Quality & Style at 7.1 / 10
Totals: 🔴 0 · 🟠 4 · 🟡 13 · 🔵 13 across 8 axes.

Blockers

  1. [Axis 1] High cyclomatic complexity in new or changed functions (orchestrator _setup/_simulation_dialog_loop, OpenCodeAgent._inject_config_content, assert_stream_balanced) (src/coder_eval/orchestrator.py:2301) — Radon at merge-base 396c22c: Orchestrator._setup - C (19) and Orchestrator._simulation_dialog_loop - F (41). At HEAD: M 1416:4 Orchestrator._setup - D (24) and M 2301:4 Orchestrator._simulation_dialog_loop - F (45). The change added these branches to _setup: self._build_monitor(), the recorded skills_offered read-back (if isinstance(recorded_skills, list):), validate_resolved_task(self.task) + registration_for(...).agent_class.contract.counts_model_turns, the if self.task.agent.plugins: staging block, and the try: ... await self.agent.harness_version() except Exception: block. It added these to the dialog loop: self.result.model_turns = self._monitor.model_turns if self._counts_model_turns else None, try: self._monitor.raise_if_over_budget(...) except BudgetExceededError:, if turn_record.tool_calls_exhausted: self.result.tool_calls_exhausted = True and if turn_record.tool_calls_exhausted and stop_decision.reason is not DialogStopReason.CRITERIA_PASSED:. The single-shot loop (orchestrator.py:2066-2078) has the same 'record run facts' sequence: tool_calls_exhausted, then model_turns, then _check_expected_targets. Move it into one helper _record_turn_facts(turn_record, iteration) and call it from both loops. Move the plugin-staging and harness-version blocks out of _setup into _stage_agent_plugins() / _record_harness_version(), so _setup is back at or below 20. The line-2301 # noqa: PLR0915 ... (see plan Phase 5) justification is now out of date, because the loop has grown again.
  2. [Axis 2] agent.by_type is an untyped dict: a misspelled kind or field is silently ignored (src/coder_eval/orchestration/experiment.py:424) — ExperimentDefaults.agent is dict[str, Any] (models/experiment.py:108), and by_type gets only a shape check in _split_by_type (lines 389-395: each key is a str and each entry is a Mapping). The code does not check a key against the registered kinds, or an entry's keys against that kind's config class. An unregistered key only reaches logger.debug("agent.by_type.%s names no registered agent kind; the entry is never applied", kind) (line 424), and debug output is hidden by default. Probe run against HEAD with the test helpers from tests/test_harness_contract.py: by_type: {claude_code: {model: claude-haiku-4-5}} resolves with agent.model == None, so the run silently uses the default model instead of the one the author asked for. by_type: {pi: {modle: x, thinking_levl: 3}} is accepted without error when the resolved kind is claude-code. The typo only fails on a later run whose kind is pi. extra="forbid" is the repo's rule (review criterion 18) for exactly this problem. Fix: in _split_by_type or _log_unregistered_by_type_kinds, validate each entry whose kind IS registered, key by key, against registration.config_class.model_fields, so an unknown field raises no matter which kind is selected. For an unregistered kind, run difflib.get_close_matches against AgentRegistry.list_kinds(). If it finds a match, raise a 'did you mean' error. If not, log at WARNING, not DEBUG. The uninstalled-plugin case in .claude/notes/orchestration.md stays allowed. Add a test for the claude_code typo.
  3. [Axis 5] New agents <-> orchestration import cycle, hidden behind function-local imports (src/coder_eval/orchestration/plugin_staging.py:27) — This change adds a cycle between the adapter layer and the orchestration layer. claude_code_agent.py:66 from coder_eval.orchestration.plugin_staging import staged_plugin_dirs and codex_agent.py:36 from coder_eval.orchestration.plugin_staging import link_or_copy import orchestration at top level. plugin_staging.py:27 from coder_eval.orchestration.harness_contract import TaskResolutionError then pulls in harness_contract, and harness_contract reaches back into agents only through local imports: harness_contract.py:62 from coder_eval.agents.registry import AgentRegistry (again at :158), and the same in turn_monitor.py:66. If those imports moved to the top of the module, importing coder_eval.orchestration.harness_contract first would run coder_eval/agents/__init__ -> claude_code_agent -> plugin_staging -> a half-built harness_contract, which fails with ImportError on TaskResolutionError. That is the anchor case 'circular import worked around with local import'. CodeQL does not flag it because the back edge is lazy. Fix: move the adapter-facing helpers (staged_plugin_dirs, link_or_copy, and the skills-layout constants) into a leaf module with no orchestration imports, for example coder_eval/plugin_layout.py or agents/_plugins.py, that both plugin_staging and the adapters import. Move TaskResolutionError into errors/ so plugin_staging and early_stop stop depending on harness_contract for a base class. Then the dependency runs one way: orchestration -> agents.
  4. [Axis 8] On a Claude Code task with max_usd and a model missing from pricing.py, a crashed or stopped attempt (no ResultMessage) latches unpriced and turns the finished row into ERROR (src/coder_eval/orchestration/turn_monitor.py:329) — Chain: _commit (L341-347) prices each finished attempt with price_turn(usage, (agent.model, start_model, reported_model)). Claude Code reports cost only on a ResultMessage. A crashed attempt (retried when it made 0 tool calls), or a turn ended by should_stop before its ResultMessage (early stop, tool cap, model-turn cap), has token usage but no total_cost_usd. If no candidate model is in pricing.py (a LiteLLM alias, or a new model not yet priced), _commit sets self._unpriced_turn = True (L345). After the criteria have run, raise_if_over_budget (L329-339) reaches if (self._limits is not None and self._limits.max_usd is not None and (self._unpriced_turn or self._unpriced_in_flight)): raise BudgetUnenforceableError(...). Orchestrator.run catches this in its generic except Exception and sets FinalStatus.ERROR, overwriting a SUCCESS or TOOL_CALLS_EXHAUSTED verdict. The resolution check _check_max_usd_priceable returns early when contract.reports_cost is true, so nothing rejects this setup up front. As a result, the same agent trajectory gets a different final_status depending on whether an infrastructure crash happened. That is the scoring-correctness blocker class. It also contradicts docs/REPORT_SCHEMA.md 'Missing cost is never fatal … None of them raises'. Nightly impact: any cross-repo task that sets max_usd while testing a newly released model before pricing.py is updated. Fix: on a reports_cost harness, do not latch unpriced for a turn that reported no cost because it crashed or was stopped. Either skip that turn's cost with a warning (the previous behaviour), or run the priceability check at resolution for every harness that sets max_usd.

Non-blocking, but please consider before merge

  1. [Axis 1] Near-identical watchdog/cancel/timeout turn-failure skeleton is repeated in the three SDK adapters (codex, antigravity, claude_code) (src/coder_eval/agents/codex_agent.py:747) — codex_agent.py:743-758, antigravity_agent.py:639-662 and claude_code_agent.py:880-893 each repeat label=f"Turn timeout ({timeout:g}s)" if timeout else "turn_timeout", except WatchdogFired: return self._fail(decoder, AgentEndStatus.TIMEOUT, format_timeout_reason(timeout or 0)), except asyncio.CancelledError: caller = asyncio.current_task(); if caller is not None and caller.cancelling() == 0: # Not a cancel from outside: the SDK raised it inside the turn body. and self._state = AgentState.ERROR; decoder.end(AgentEndStatus.CRASHED, reason="turn cancelled"); raise. Codex and Antigravity also repeat the same if decoder.timeout_hit: ... if not decoder.ended_cleanly: ... # Already stopped on purpose — do not escalate. arm. Each adapter also has an identical _fail (self._state = AgentState.ERROR; return decoder.end(status, reason=reason): codex 781, claude 916, antigravity 733). The CLI adapters already share SubprocessJsonlAgent. Put the SDK turn body behind one helper on Agent (for example _run_sdk_turn(body, decoder, timeout, crash_label)) so that the cancel and timeout classification exists in one place.
  2. [Axis 1] The harness contract is looked up twice per task (turn_monitor._reports_cost and orchestrator registration_for), and the orchestrator's _counts_model_turns flag re-derives what TurnMonitor could answer (src/coder_eval/orchestration/turn_monitor.py:65) — turn_monitor.py:65-73 def _reports_cost(task) does its own registry lookup (ensure_plugins_loaded(); registration = AgentRegistry.get(str(task.agent.type)); return registration is not None and registration.agent_class.contract.reports_cost). This is a copy of registration_for in orchestration/harness_contract.py:52. The orchestrator then looks up the same contract again at orchestrator.py:1454 (self._counts_model_turns = registration_for(self.task, requirement="model-turn accounting").agent_class.contract.counts_model_turns), only to write the same conditional at two sites: orchestrator.py:2076 and 2446 self.result.model_turns = monitor.model_turns if self._counts_model_turns else None. Resolve the contract once in TurnMonitor.for_task. Let the monitor keep reports_cost and counts_model_turns, and make TurnMonitor.model_turns return int | None. Then delete _reports_cost, the _counts_model_turns orchestrator field and the two conditionals.
  3. [Axis 1] The gated contract-row mapping is kept as string lists, read with getattr, and written twice (harness_contract and testing) (src/coder_eval/testing.py:203) — orchestration/harness_contract.py:40 declares _GATED: dict[str, str] = {"system_prompt": "system_prompt", "plugins": "plugin_skills", "permission_mode": ..., "allowed_tools": ..., "disallowed_tools": ...}. The new public test module declares the same relation again in inverse form: testing.py:203 _FIELDS = ("system_prompt", "plugin_skills", "permission_mode", "allowed_tools", "disallowed_tools") and :204 _CONFIG_FIELD = {"plugin_skills": "plugins"}. If a new gated HarnessContract row is added to only one of them, the conformance sensor and the resolver drift apart without an error. Export one mapping (for example a public GATED_FIELDS from models/harness_contract.py) and derive both from it.
  4. [Axis 3] Failure and timeout branches in adapters and testing sensors have no tests (src/coder_eval/agents/codex_agent.py:756-757) — Coverage reports these lines as never run, and they are changed lines: codex_agent.py 757 (if decoder.timeout_hit: / return self._fail(decoder, AgentEndStatus.TIMEOUT, format_timeout_reason(timeout or 0)) inside except Exception as e:), antigravity_agent.py 645 (the same if decoder.timeout_hit: return self._fail(decoder, AgentEndStatus.TIMEOUT, ...)), and claude_code_agent.py 890 (if self._timed_out(decoder.timeout_hit, deadline): return self._fail(decoder, AgentEndStatus.TIMEOUT, timed_out) on an in-turn CancelledError). This branch decides between TIMEOUT and CRASHED when the SDK raises because the watchdog killed it. The two statuses retry differently (after this stack, an AGENT_CRASH with tool calls is not retried) and are reported differently, so a regression that removes the check would reclassify real timeouts as crashes and no test would fail. For each of the three adapters, add a test that sets decoder.timeout_hit (or fires the watchdog's on_timeout) and then makes the SDK call raise a generic Exception (or CancelledError for Claude Code, with cancelling() == 0). Assert outcome.status is AgentEndStatus.TIMEOUT. Consider adding this case to the shared conformance sensor, so every harness gets it.
  5. [Axis 5] Importing the 'stable' plugin SPI loads every built-in adapter, claude_agent_sdk and the docker driver (src/coder_eval/agents/_transport/subprocess_jsonl.py:27) — spi.py:10 from coder_eval.agents._transport import JsonlDecoder, SubprocessJsonlAgent runs coder_eval/agents/__init__.py, which eagerly imports all six built-ins (lines 4-9, from coder_eval.agents.claude_code_agent import ClaudeCodeAgent ...). The transport base also runs from coder_eval.isolation.docker_runner import STDOUT_LINE_LIMIT_BYTES (line 27), and docker_runner imports orchestration.evaluation and orchestration.plugin_staging. Checked: after import coder_eval.spi, sys.modules holds claude_agent_sdk., all six coder_eval.agents. adapters, coder_eval.isolation.docker_runner and coder_eval.orchestration.{evaluation,harness_contract,plugin_staging}. A third-party agent that follows 'import only from coder_eval.spi' therefore depends on the Claude SDK and on every in-tree harness importing cleanly. A shared constant also makes the SPI depend on a 1500-line sandbox driver. Fix: move STDOUT_LINE_LIMIT_BYTES to a leaf module (for example models/container_paths.py or a small limits constant module) and import it from there in both docker_runner and the transport. Stop coder_eval/agents/__init__.py from importing adapters eagerly: have register_builtins import them lazily, so importing the transport or registry submodules does not pull in the built-ins.
  6. [Axis 5] TurnMonitor is a ~550-line class that mixes criterion gating, structural caps, and token/USD budget pricing (src/coder_eval/orchestration/turn_monitor.py:76) — class TurnMonitor: runs from line 76 to line 629 (the file is 629 lines) and holds three separate state machines: (1) armed-criterion early stop (_ceiling, _floor, _collect_verdicts, _budget_drove, _evaluate_impl at radon D(28), _fire, plus the latch lists _latched, _budget_expired and _prev_verdicts), (2) structural caps (_evaluate_cap, _evaluate_model_turn_cap), and (3) budgets with model-resolution pricing (_commit, _price, _breach, _evaluate_budgets, _evaluate_in_flight_priceable, raise_if_over_budget, cost_usd). Every agent event on every task goes through it. CLAUDE.md asks for one should_stop answer, not one class. Fix: keep TurnMonitor as a thin composer that owns the latch and precedence, and delegate to an ArmedCriteriaGate (moved from the former early_stop watcher logic) and a BudgetLedger (usage, pricing, breach). Each part can then be tested and reasoned about alone. Rated Medium, not High, because the precedence and latch are one real shared concern.
  7. [Axis 5] UiPath-specific plugin_tools_dir frozen into the new public SPI v1 Agent.start contract (src/coder_eval/agent.py:72) — This change publishes coder_eval.spi (spi.py:9 exports Agent) as the stable plugin surface with SPI_VERSION = 1, and 'any signature change to a name exported here bumps SPI_VERSION'. Agent.start keeps plugin_tools_dir: str | None = None (line 73), documented at lines 82-86 as 'Optional canonical node_modules/@uipath to export as PLUGIN_TOOLS_DIR so the agent's UiPath CLI pins plugin discovery'. Every out-of-tree agent must now accept a parameter for an internal vendor's CLI, and removing it later is a breaking SPI bump. The in-tree adapters already disagree: codex_agent and antigravity_agent document it as 'Accepted for the Agent.start signature; unused', while pi/opencode/claude export it. This conflicts with the 'agnostic core, vendor behaviour opt-in' convention for a core that is being made public. Fix, before SPI v1 ships: remove the parameter from the abstract signature and deliver it as a generic extra_env: Mapping[str, str] (the sandbox already owns plugin_tools_dir and can contribute PLUGIN_TOOLS_DIR to it), or move it behind the UiPath extra.
  8. [Axis 6] The new BudgetUnenforceableError has no error category, so task.json records it as 'unknown' (src/coder_eval/errors/budget.py:31) — class BudgetUnenforceableError(Exception): is documented as "An eval-config error: the run finalizes ERROR". turn_monitor.py:334 raises it, and it reaches Orchestrator.run()'s generic except Exception, which calls create_error_context. errors/categorization.py has no typed branch for it. I ran categorize_error on it with component 'orchestrator.iteration_1' and with 'orchestrator.setup': both returned ErrorCategory.UNKNOWN and logged "Could not categorize error: run_limits.max_usd could not be enforced...". As a result, task.json error_details.error_category is 'unknown' (a cross-repo contract field) and no actionable tip is attached. Add a typed check in _categorize_by_exception_type next to BudgetExceededError. Map it to AGENT_CONFIG_ERROR (non-retryable, with a config tip) or to a new pinned category. Also add a categorization test.
  9. [Axis 6] A malformed plugin.json is silently treated as absent, so resolution-time plugin validation can pass a plugin that Claude Code refuses to load (src/coder_eval/orchestration/plugin_staging.py:76) — _read_manifest has except (OSError, UnicodeDecodeError, json.JSONDecodeError): return {}. A plugin.json that is present but has a syntax error, or that the process cannot read, gives no error. _plugin_name then uses the directory name, and _declared_skill_paths ignores the declared skills paths. As a result, validate_plugins (whose stated job is to refuse "before the run is paid for") and the skills_offered positive control come from a reading that differs from what the harness loads. Claude Code rejects a plugin with an invalid manifest, so the run gets no skill and fails only after it has spent money. This is a user-authored, required input, so it must fail loud. manifest.is_file() already covers the absent case: raise PluginStagingError that names the manifest path and the parse error, and do not return {}.
  10. [Axis 7] run_limits.max_turns / expected_turns reuse their origin/main key names with a new meaning (model turns across the whole task), so stale YAML still loads on Claude Code, OpenCode and Pi (src/coder_eval/models/limits.py:51) — On origin/main, max_turns was "Max agent inner-loop turns per iteration. None = SDK default." and expected_turns counted visible entries ("each tool call contributes 1, plus 1 for the final reply"). This stack first renamed these fields to max_tool_calls / expected_tool_calls, then brought back the old names with a different meaning. limits.py:51 max_turns: int | None = Field( is now a cap on model turns across the whole task (all retry attempts and all dialog turns). limits.py:72 expected_turns: int | None = Field( now counts model turns, not tool calls. Because both names still exist, extra="forbid" does not flag a stale YAML. On Claude Code (which counts model turns), an old expected_turns: 20 now compares against a different quantity, and an old per-iteration max_turns in a dialog task now applies to the whole task. REPORT_SCHEMA.md documents the change only for recorded runs. The input YAML gets no error or warning. Per CLAUDE.md "Delete before you guard" and the greenfield rule, use new names (e.g. max_model_turns / expected_model_turns) so that stale keys fail loudly at load time. Then do the old-name grep sweep (tasks/, docs/, plugins/coder-eval/skills/analyze/SKILL.md:58).
  11. [Axis 7] The coder_eval.spi docstring tells plugins to pass the imported SPI_VERSION, which makes the version check always pass; EXTENDING.md says to pass a literal number (src/coder_eval/spi.py:3) — spi.py:3-4 says: "A plugin imports only from this module and passes SPI_VERSION to every AgentRegistry.register call, which rejects a version other than this core's." If a plugin passes spi.SPI_VERSION, it passes the value of the core it runs against, so the check in registry.py (if spi_version != SPI_VERSION:) can never fail. docs/EXTENDING.md:44-47 gives the correct rule: "pass the SPI version the agent was written against, as the literal number" (spi_version=1). The module docstring is the first thing a plugin author reads, and it contradicts the guide. Rewrite the spi.py docstring to require the literal version. Also consider not exporting SPI_VERSION in __all__, or naming it for what it is (the core's provided version), so the check cannot be defeated by accident. A lint or test could reject spi_version=SPI_VERSION outside src/coder_eval/agents/.
  12. [Axis 8] sdk_options.max_turns is user-settable again: a second turn cap whose SDK stop ends COMPLETED with no cap fact (src/coder_eval/models/agent_config.py:88) — This change removed "max_turns" from _FRAMEWORK_OWNED_SDK_FIELDS. Only the comment at L88-89 is left: # (max_turns is the SDK's own agent-loop cap; the framework cap is run_limits.max_tool_calls.). The removal lets agent_judge and the simulator inject it, but a task YAML can now set it too. That creates a second structural cap outside the TurnMonitor, which breaks the rule that 'every structural cap is one TurnMonitor answer'. When the SDK stops with subtype == "error_max_turns", claude_code_agent.py ends the turn COMPLETED (L913, and _max_turns_short_circuit at L1459 on the exception path). So tool_calls_exhausted stays False and a failing run is FAILURE. On main, the same exhaustion gave MAX_TURNS_EXHAUSTED. A user moving an old run_limits.max_turns into sdk_options gets a silently different status. Fix: keep max_turns framework-owned for user YAML and let the judge and simulator set it through a private path, or map error_max_turns to TOOL_CALLS_EXHAUSTED.
  13. [Axis 8] TurnEmitter.finalize's default result_summary now sets result to the final main-thread text on non-Claude harnesses. This silently raises has_final_reply and visible_turns compared with main. (src/coder_eval/streaming/emitter.py:404) — if result_summary is _UNSET: result_summary = ResultSummary(is_error=False, subtype=status.value, stop_reason=stop_reason, result=self._final_reply()) (L404-407) now fills result with the last main-thread text on every harness. On main, Codex wrote result=crash_reason or self.error_message, which is None on a clean turn. result_metrics.has_final_reply counts any non-empty result_summary.result, and visible_turn_count adds 1 for it. So for the same agent behaviour, run.json has_final_reply changes from false to true, visible_turns rises by 1, and expected_tool_calls_overage can now fire on Codex, OpenCode, Pi and Antigravity rows. This is a silent change to a trend metric on the cross-repo contract. docs/REPORT_SCHEMA.md 'Historical spellings' documents the expected_turns change but not this one. Fix: document the definition change beside the other historical notes, or version the metric in the row so trend dashboards can tell old rows from new ones.

Nits

  1. [Axis 1] _open_emitter task_id is filled inconsistently across adapters, and the SubprocessJsonlAgent task_id kwarg is dead (src/coder_eval/agents/_transport/subprocess_jsonl.py:93) — The orchestrator's _create_agent (orchestrator.py:1788) never passes task_id, so task_id: str = "unknown" at subprocess_jsonl.py:93 is always "unknown". Meanwhile codex_agent.py:718, antigravity_agent.py:622, noop_agent.py:94 and claude_code_agent.py:837 pass task_id=str(self.config.type) (the agent kind), with the same copy-pasted comment. TaskScopedCallback overwrites event.task_id downstream, so the only visible effect is emitter log lines such as "[%s] a write after the turn ended was dropped" labelled with the agent kind or "unknown". Drop the task_id parameter from _open_emitter and from the JSONL constructor, and let _open_emitter supply one value itself.
  2. [Axis 1] The unhandled list side channel in _communicate_with_retry guards a closed enum that an import-time assert already covers (src/coder_eval/orchestrator.py:1819) — orchestrator.py:1819 unhandled: list[AgentEndStatus] = [], :1864 unhandled.append(outcome.status); return outcome.record, :1893 if unhandled: raise RuntimeError(f"unhandled end status {unhandled[0]}"). _RETURNED_END_STATUSES together with (CRASHED, TIMEOUT) already covers all 7 AgentEndStatus members, so this path cannot be reached today. The codebase already uses a module-level exhaustiveness assert for this: streaming/events.py:97 assert set(_END_STATUS_FOR_STOP) == set(StopReason). Replace the closure-captured list with assert _RETURNED_END_STATUSES | {CRASHED, TIMEOUT} == set(AgentEndStatus) next to line 103.
  3. [Axis 1] _setup runs validate_early_stop twice on the agent path (src/coder_eval/orchestrator.py:1453) — orchestrator.py:1424 validate_early_stop(self.task) runs first. Then orchestrator.py:1453 validate_resolved_task(self.task) runs, and its body (resolution_checks.py) starts with validate_early_stop(task) again. Keep the early call only for the evaluate-only branch (move it inside if self.sandbox is not None:), or leave a comment that says the repeat is on purpose.
  4. [Axis 1] The token-bucket tuple and the 'turn-end deltas exceed published usage' check are duplicated between emitter.py and testing.py (src/coder_eval/testing.py:151) — testing.py:151 and streaming/emitter.py:54 both declare _BUCKETS = ("uncached_input_tokens", "output_tokens", "cache_creation_input_tokens", "cache_read_input_tokens"). The per-bucket reported > published comparison in assert_stream_balanced (testing.py:193-197) repeats TurnEmitter._warn_on_delta_overshoot (emitter.py:644). Put the bucket list on TokenUsage (for example TokenUsage.BUCKETS or an exceeds(other) -> list[str] method) and use it from both places.
  5. [Axis 2] Public SPI surfaces (TurnEmitter sentinel, testing.replay) are typed as Any (src/coder_eval/streaming/emitter.py:52) — _UNSET: Any = object() is the default for the public SPI keywords started_at: datetime | None = _UNSET (open_tool, line 231), completed_at (close_tool), and num_turns / result_summary (finalize/fail). Typing the sentinel as Any hides the type error in the default. The real contract is enforced only at runtime in _stamp: under TURN_CLOCK the keyword is forbidden, and under CLI_EPOCH_MS it is required. A wrong call raises TypeError in the middle of a turn, although the call type-checks. A third-party plugin adapter gets no static help. Use a typed sentinel, for example class _Unset(Enum): UNSET = auto() with started_at: datetime | None | Literal[_Unset.UNSET] = _Unset.UNSET, so is _UNSET narrows correctly. Optionally add @overloads, or split the method per basis, so a wrong-basis call fails in pyright.
  6. [Axis 4] The consent text for container grading does not list the new skill-source mounts, so an operator approves an incomplete list of host paths (src/coder_eval/isolation/docker_runner.py:1336) — This change made _build_argv mount host paths through the new DockerRunner._plugin_mount_paths() (line 1477: for plugin_path in self._plugin_mount_paths():). That helper adds each resolved skill source that sits outside every plugin root (line 1336: paths += [str(d) for d in skill_dirs if not any(d.is_relative_to(root) for root in roots)]), and each one becomes a -v <path>:<path>:ro bind mount. The consent gate for untrusted run directories, orchestration/regrade.py::_dispatch_host_exposure, says it "Mirrors ... the _auto_mount block of _build_argv" (regrade.py:285). But it still lists only the authored plugin.get("path") values, so a symlinked skill source outside the root is mounted into the grading container without being named in the --allow-recorded-commands prompt. The same drift goes the other way: this change removed the system_prompt_file auto-mount, but the disclosure still lists agent.system_prompt_file (regrade.py:302-303). Exploiting the gap is hard: the symlink must already exist on the grader's host, and the mount is read-only. Fix: have _dispatch_host_exposure call one shared helper (move _plugin_mount_paths logic to a pure function both sides import) instead of re-deriving the mount set by hand, and drop the stale system_prompt_file entry. Add a parity test that the disclosed host paths equal the set of -v sources _build_argv emits for a task whose plugin has a symlinked skill outside its root. CVSS:3.1/AV:L/AC:H/PR:N/UI:R/S:U/C:L/I:N/A:N
  7. [Axis 5] SubprocessJsonlAgent leaves identical start()/env composition duplicated in each subclass (src/coder_eval/agents/_transport/subprocess_jsonl.py:110) — The new transport base declares def env(self) -> dict[str, str]: as abstract (line 110) and has no start(). Both subclasses therefore repeat the same state and env assembly: pi_agent.py:541-544 and opencode_agent.py:643-646 are both 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, and each start() stores _env_path_prepend and _plugin_tools_dir again. The base exists to hold what CLI harnesses share. Fix: store these in a base start() hook and apply the PATH/PLUGIN_TOOLS_DIR overlay in a concrete base env() that calls a subclass base_env().
  8. [Axis 6] The TurnMonitor fail-open handler meant for optional early stop also hides failures in the required cap and budget accounting (src/coder_eval/orchestration/turn_monitor.py:200) — on_event wraps all of _on_event_impl in except Exception: self._disarmed = True; logger.error(..., _DISARMED, ...). The module docstring says "The caps read counters, so they never disarm". But in the ToolEndEvent branch, self._collector.on_event(event) (line 248) runs before self._tool_call_index += 1 / self._resolved_tool_ids.add(...) (lines 251-252). The collector reduction is needed only for armed criteria. If it raises, max_tool_calls stops counting every later call and the log still says "armed criteria disarmed, run degrades to a full run". The same applies to _commit/_evaluate_budgets for max_usd. Update the counters and budgets first, outside the fail-open try. Wrap only the collector reduction and the armed evaluation, so that a required limit fails loud and does not fail open.
  9. [Axis 6] command_version kills a hung --version probe but does not reap it (src/coder_eval/agent.py:249) — The timeout path is except TimeoutError: with contextlib.suppress(ProcessLookupError): proc.kill(); return None. It never awaits proc.wait(), so the killed child stays a zombie and its asyncio transport is not closed until GC ('Event loop is closed' / ResourceWarning on shutdown). A CancelledError that arrives during proc.communicate() (the task-timeout watchdog during setup) leaves the probe running. Use try/finally: if proc.returncode is None, kill and then await asyncio.wait_for(proc.wait(), ...). This matches the reap discipline of SubprocessJsonlAgent._reap.
  10. [Axis 7] The fail-loud hint for the removed DEFAULT_ env knobs suggests a -D path that -D rejects* (src/coder_eval/config.py:44) — config.py:44-45 maps "DEFAULT_AGENT_MODEL": "agent.by_type.claude-code.model" and "DEFAULT_PERMISSION_MODE": "agent.by_type.claude-code.permission_mode". _reject_removed_default_knobs puts these into "override per-run with -D {path}=…". I checked this: apply_overrides(task, {'agent.by_type.claude-code.model': 'x'}) raises OverrideError: unknown field 'by_type' under 'agent'; did you mean 'type'?, because by_type is valid only in experiment defaults. Use the by_type path only for the experiments/default.yaml hint, and give -D agent.model=… / --model for the per-run hint. Also, "DEFAULT_MAX_TURNS": "run_limits.max_tool_calls" (line 46) points at the tool-call cap, but the old knob was the SDK model-turn cap, which is now run_limits.max_turns.
  11. [Axis 7] The sdk_options override error message is ungrammatical when no agent type is set (src/coder_eval/orchestration/overrides.py:111) — overrides.py:111-112: where = "no agent type is set" if kind is None else f"the {kind!r} agent config" / raise OverrideError(f"sdk_options is not a field of {where}; ..."). When kind is None, this produces "sdk_options is not a field of no agent type is set; it is supported by: claude-code." Use a separate sentence for that case, e.g. "sdk_options needs an agent type; it is supported by: …".
  12. [Axis 7] The review command's agent-registration example does not pass the now-required spi_version keyword (.claude/commands/coder-eval-code-review-full.md:371) — Line 371 says: "It registers via registry.register(\"kind\", Config)(Agent) in a register(registry) hook". AgentRegistry.register now has a required keyword-only argument *, spi_version: int (registry.py:92), so the example as written raises TypeError. Change it to registry.register("kind", Config, spi_version=1)(Agent), the form docs/EXTENDING.md:54 uses.
  13. [Axis 8] The task-wide max_tool_calls counter dedupes by tool id, but fallback ids repeat in each communicate() call, so dialog turns and retries undercount (src/coder_eval/orchestration/turn_monitor.py:252) — self._resolved_tool_ids.add(event.tool.tool_id) (L252), with tool_calls returning len(self._resolved_tool_ids) (L292), is cumulative across every communicate() call. Several adapters make fallback ids that restart each turn: Pi uses f"call_{self.tool_count + 1}" (pi_agent.py L223) and Codex uses f"{root_type}_{self._minted}", where _minted restarts per _CodexDecoder (codex_agent.py L285, L349). When the harness omits ids, dialog turn 2's call_1 collides with turn 1's call_1. The cap then fires late, and the monitor's shared collector (keyed by tool_id) overwrites earlier commands that the armed live_verdict reads. Fix: dedupe per communicate() by clearing a per-call id set on AgentStartEvent and adding its size to a running total, or have the emitter namespace fallback ids with the iteration number.

What's Missing

Parallel paths:

  • 🔵 docker_runner._build_argv now mounts skill sources through _plugin_mount_paths() and no longer auto-mounts system_prompt_file. The consent disclosure in orchestration/regrade.py::_dispatch_host_exposure, which says it mirrors that block, was not updated. It still lists only the authored plugin paths and still lists system_prompt_file. (trigger: src/coder_eval/isolation/docker_runner.py) (restates: Axis 4: The consent text for container grading does not list the new skill-source mounts)
  • 🟠 The priceability check at resolution (harness_contract._check_max_usd_priceable) returns early for a harness with reports_cost. But TurnMonitor._commit now latches unpriced for any turn with no reported cost, and raise_if_over_budget turns that latch into ERROR. On Claude Code, a crashed turn or a turn stopped by should_stop has no ResultMessage. The resolution gate and the runtime gate therefore disagree about which setups can be enforced. (trigger: src/coder_eval/orchestration/turn_monitor.py) (restates: Axis 8: On a Claude Code task with max_usd and a model missing from pricing.py, a crashed or stopped attempt latches unpriced and turns the row into ERROR)
  • 🔵 This stack brought back run_limits.max_turns as a model-turn cap. config.py's removed-knob hint still maps DEFAULT_MAX_TURNS to run_limits.max_tool_calls, and its DEFAULT_AGENT_MODEL and DEFAULT_PERMISSION_MODE hints point at an agent.by_type path that -D rejects. (trigger: src/coder_eval/config.py) (restates: Axis 7: The fail-loud hint for the removed DEFAULT* env knobs suggests a -D path that -D rejects)_
  • 🟡 The gated contract rows are declared in orchestration/harness_contract.py (_GATED) and again in testing.py (_FIELDS, _CONFIG_FIELD, _GATED_VALUES), with a third copy in tests/test_harness_contract.py. A new gated row added to the resolver does not reach the conformance sensor. No test checks that the copies agree. (trigger: src/coder_eval/testing.py) (restates: Axis 1: The gated contract-row mapping is kept as string lists, read with getattr, and written twice)
  • 🟡 The three SDK adapters (codex, antigravity, claude_code) each have their own copy of the watchdog, cancel and timeout classification. The CLI adapters share SubprocessJsonlAgent, which still leaves the start() and env() composition copied in pi_agent and opencode_agent. A fix to TIMEOUT-vs-CRASHED classification must be made in up to five places. (trigger: src/coder_eval/agents/codex_agent.py) (restates: Axis 1: Near-identical watchdog/cancel/timeout turn-failure skeleton is repeated in the three SDK adapters)

Tests:

  • 🟠 No test covers the TIMEOUT-vs-CRASHED branch when the SDK raises after the watchdog set decoder.timeout_hit (codex_agent.py:757, antigravity_agent.py:645, and claude_code_agent.py:890 on the in-turn CancelledError path). The shared coder_eval.testing.conformance sensor has no such case either, so plugin agents are not checked for it. (trigger: src/coder_eval/agents/codex_agent.py) (restates: Axis 3: Failure and timeout branches in adapters and testing sensors have no tests)
  • 🟠 The new BudgetUnenforceableError has no categorization test. The only tests check that it is raised (test_turn_monitor.py:459, test_run_limits_orchestrator.py:386). Nothing checks task.json error_details.error_category or the final status for a Claude Code turn that crashed or was stopped early with max_usd set and an unpriced model. (trigger: src/coder_eval/errors/budget.py) (restates: Axis 6: The new BudgetUnenforceableError has no error category, so task.json records it as 'unknown')
  • 🟡 No parity test checks that the host paths regrade discloses equal the -v sources that _build_argv emits, for example for a plugin with a symlinked skill outside its root. tests/test_docker_runner_mounts.py and tests/test_regrade.py each test only their own side. (trigger: src/coder_eval/isolation/docker_runner.py) (restates: Axis 4: The consent text for container grading does not list the new skill-source mounts)
  • 🟡 No test covers agent.by_type with a near-miss kind (claude_code instead of claude-code), or with a misspelled field in the entry for a registered kind that is not selected. Both pass resolution without an error today. (trigger: src/coder_eval/orchestration/experiment.py) (restates: Axis 2: agent.by_type is an untyped dict: a misspelled kind or field is silently ignored)
  • 🟡 tests/test_plugin_staging.py:345 checks that a corrupt plugin.json stages under the directory name. No test checks that validate_plugins or skills_offered agree with Claude Code, which refuses to load a plugin with a corrupt manifest. (trigger: src/coder_eval/orchestration/plugin_staging.py) (restates: Axis 6: A malformed plugin.json is silently treated as absent)
  • 🟡 tests/test_sdk_option_classification.py:112 now checks that a user can set sdk_options.max_turns. No test checks the end status when the SDK stops with error_max_turns on that path: the turn ends COMPLETED with no cap fact, where main gave MAX_TURNS_EXHAUSTED. (trigger: src/coder_eval/models/agent_config.py) (restates: Axis 8: sdk_options.max_turns is user-settable again: a second turn cap whose SDK stop ends COMPLETED with no cap fact)
  • 🔵 No test runs a dialog task or a retried task on a harness that makes its own fallback tool ids (Pi call_N, Codex _minted). Such a test would show that the task-wide max_tool_calls dedupe and the shared collector miscount when ids repeat across communicate() calls. (trigger: src/coder_eval/orchestration/turn_monitor.py) (restates: Axis 8: The task-wide max_tool_calls counter dedupes by tool id, but fallback ids repeat in each communicate() call)

Downstream consumers:

  • 🟡 TurnEmitter.finalize now fills result_summary.result with the final reply on every harness. Everything that reads it changes on Codex, OpenCode, Pi and Antigravity rows: result_metrics.has_final_reply, visible_turn_count, expected_tool_calls_overage, and the evalboard turns and overview charts built on them. The REPORT_SCHEMA 'Historical spellings' note does not mention this, and no row carries a version to tell old rows from new ones. (trigger: src/coder_eval/streaming/emitter.py) (restates: Axis 8: TurnEmitter.finalize's default result_summary now sets result to the final main-thread text on non-Claude harnesses)
  • 🟡 run_limits.expected_turns now counts model turns, not tool calls + 1, and max_turns now applies to the whole task, not to one iteration. run_record.py emits expected_turns only when model_turns is set. The evalboard (lib/turns.ts, turns-stat.tsx) and the analyze skill (plugins/coder-eval/skills/analyze/SKILL.md:58, which reads run_limits.max_turns) compare old and new rows under the same key name, although the key now means something different. (trigger: src/coder_eval/models/limits.py) (restates: Axis 7: run_limits.max_turns / expected_turns reuse their origin/main key names with a new meaning)
  • 🔵 StopReason.MODEL_TURN_CAP maps to AgentEndStatus.TOOL_CALLS_EXHAUSTED (streaming/events.py:93), so run.json tool_calls_exhausted and FinalStatus TOOL_CALLS_EXHAUSTED now count hits of both caps. A rate of tool-call-cap exhaustion computed from these fields cannot tell a model-turn cap from a tool-call cap. REPORT_SCHEMA.md:152 documents the combined meaning, but the status name does not show it. (trigger: src/coder_eval/streaming/events.py)

Display & mapping dicts:

  • 🔵 reports/html.py:777 always labels a turn with tool_calls_exhausted 'tool-call cap reached'. That flag is now also set when run_limits.max_turns (MODEL_TURN_CAP) stops the turn, so a model-turn cap stop gets the wrong label in the HTML report. (trigger: src/coder_eval/reports/html.py)
  • 🔵 The stack added tool_call_cap to _SIMULATION_STOP_REASON_LABELS in reports/html.py. It did not add DialogStopReason.RUN_LIMIT_EXCEEDED, which the dialog loop sets on a budget breach (orchestrator.py:2450). That value falls through to a neutral badge with the raw text 'run_limit_exceeded', where it should show a failure badge. (trigger: src/coder_eval/simulation/termination.py)

Daily/nightly:

  • 🟡 The PR descriptions do not say what the rename of FinalStatus MAX_TURNS_EXHAUSTED to TOOL_CALLS_EXHAUSTED (status icon 'M' to 'C') and the run.json key rename to tool_calls_exhausted do to the nightly trend history. That history includes the evalboard trends and the App Insights dashboards in coder_eval_uipath/infra/dashboards, which filter on status strings. Historical series split at the merge. (trigger: src/coder_eval/models/enums.py)
  • 🟡 The 'Breaking changes' section of PR #183 says run_limits.max_turns and expected_turns are rejected by extra="forbid". At HEAD both keys are accepted again with a new meaning. A shared or nightly YAML that still sets them therefore loads without an error on Claude Code, OpenCode and Pi and applies a different cap. The PR text does not say this. (trigger: src/coder_eval/models/limits.py) (restates: Axis 7: run_limits.max_turns / expected_turns reuse their origin/main key names with a new meaning)
  • 🟡 The PRs list 'an unpriceable max_usd' as 'Not verified live'. They do not say that a nightly task with max_usd, run against a newly released model not yet in pricing.py, can now end ERROR with error_category 'unknown' after a crash retry or an early stop. The same trajectory used to keep its verdict. (trigger: src/coder_eval/orchestration/turn_monitor.py) (restates: Axis 8: On a Claude Code task with max_usd and a model missing from pricing.py, a crashed or stopped attempt latches unpriced and turns the row into ERROR)
  • 🔵 The out-of-tree Delegate agent (coder_eval_uipath) imports coder_eval.spi. Importing it now loads claude_agent_sdk, all six built-in adapters and docker_runner. The PRs do not say that the nightly Delegate pipeline now fails to import if any built-in adapter or its SDK fails to import. (trigger: src/coder_eval/spi.py) (restates: Axis 5: Importing the 'stable' plugin SPI loads every built-in adapter, claude_agent_sdk and the docker driver)

Harness & Lint Improvements

Static checks (lint / type):

  • [ruff] Turn on ruff C901 (mccabe) in pyproject select with [tool.ruff.lint.mccabe] max-complexity = 18, which is below today's worst offenders. Each existing offender gets a visible # noqa: C901 debt marker, the same way PLR0915/PLR0912 are handled: Orchestrator._simulation_dialog_loop F(45), TurnMonitor._evaluate_impl D(28), Orchestrator._setup D(24), OpenCodeAgent._inject_config_content, testing.assert_stream_balanced. With the marker in place, any new branch added to a function that has no marker fails make check. Today PLR0912 (max-branches=25) and CE022 (statement count on one function) both missed _setup going from C(19) to D(24) and the dialog loop going from F(41) to F(45), because neither rule measures cyclomatic complexity. Prevents: A1 high: complexity growth in _setup / _simulation_dialog_loop / _inject_config_content / assert_stream_balanced. It also catches the next increase in A5's TurnMonitor._evaluate_impl D(28).
  • [ce-lint] CE074 NoOrchestrationImportsInAdapters (tests/lint/rules/ce074_no_orchestration_imports_in_adapters.py, wired into tests/lint/runner.py and added to the pyproject CE list). Forbid any import/from of coder_eval.orchestration*, coder_eval.orchestrator or coder_eval.isolation* inside src/coder_eval/agents/**, at module level or inside a function. It also flags the reverse lazy edge: a function-local from coder_eval.agents... import in orchestration/ whose module is also imported at top level from agents/ (the 'cycle hidden by a local import' shape). Adapter-facing helpers (staged_plugin_dirs, link_or_copy, STDOUT_LINE_LIMIT_BYTES) must live in a leaf module, and TaskResolutionError must live in errors/. Prevents: A5 high: the agents <-> orchestration import cycle (claude_code_agent.py:66, codex_agent.py:36 -> plugin_staging -> harness_contract). A5 medium: subprocess_jsonl.py:27 importing docker_runner for one constant.
  • [ce-lint] CE075 SpiImportClosure: a @pytest.mark.lint whole-tree class in tests/test_custom_lint.py. It follows the static import graph from coder_eval/spi.py (AST, following package __init__ files) and fails when the closure reaches claude_agent_sdk, any concrete adapter module in coder_eval/agents/ other than _transport, registry and agent, coder_eval.isolation.* or coder_eval.orchestration.*. Only an explicit allowlist may appear in that closure. Today it would fail on agents/__init__.py, which eagerly imports all six built-ins. Prevents: A5 medium: import coder_eval.spi loads every built-in adapter, claude_agent_sdk and docker_runner.
  • [ce-lint] CE076 ErrorClassCategorized: a @pytest.mark.lint whole-tree class. For every class X(...Exception/Error) defined under src/coder_eval/errors/, it requires that X is referenced by name in errors/categorization.py (an isinstance branch in _categorize_by_exception_type) or is listed in an UNCATEGORIZED_BY_DESIGN set with a reason. This is AST name resolution only. Prevents: A6 medium: BudgetUnenforceableError falls through to ErrorCategory.UNKNOWN in task.json error_details.error_category.
  • [ce-lint] CE077 SpiVersionLiteral: a doc-surface and AST rule in tests/test_custom_lint.py. (a) Outside src/coder_eval/agents/ and registry.py, including tests/fixtures plugins, plugins/, docs/**/*.md, .claude/commands/**/*.md and the docstrings of spi.py and registry.py, a register( call must pass spi_version=<int literal>, and spi_version=SPI_VERSION is flagged. (b) Every registry.register("...", Config) snippet in Markdown must contain the spi_version= keyword, because the keyword is now required. Prevents: A7 medium: the spi.py docstring tells plugins to pass the imported SPI_VERSION, so the version check always passes. A7 low: the review-command example at coder-eval-code-review-full.md:371 has no spi_version and raises TypeError.
  • [ce-lint] CE078 NoDuplicatedStringSetConstants: a @pytest.mark.lint whole-tree class. It collects every module-level _NAME = (tuple|frozenset|set of >=3 str literals) and every dict whose keys or values are all str literals in src/coder_eval/**. It fails when two modules define the same set of strings, compared as sets so order does not matter, and dict keys and values are included so an inverse copy of a mapping still matches. The fix is to import one public constant, for example TokenUsage.BUCKETS or GATED_FIELDS. Prevents: A1 medium: testing.py _FIELDS/_CONFIG_FIELD mirror harness_contract _GATED. A1 low: _BUCKETS is duplicated in emitter.py:54 and testing.py:151.
  • [ce-lint] CE079 NoAnyTypedSentinel: in src/coder_eval/**, flag a module-level NAME: Any = object(), and a parameter whose default is such a name while its annotation does not include that sentinel's type. Require a one-member Enum sentinel so is _UNSET narrows in pyright. Prevents: A2 low: _UNSET: Any = object() hides wrong defaults on the public TurnEmitter SPI (open_tool/close_tool/finalize/fail).
  • [ce-lint] CE080 KilledProcessIsReaped, which extends CE073's subprocess family: in any async def in src/, a <name>.kill() or <name>.terminate() on a value bound from asyncio.create_subprocess_* must be followed in the same function by await <name>.wait(), directly or inside asyncio.wait_for, or the function must call a known reaper (_reap). The rule also flags a create_subprocess_* + communicate() that is not in a try/finally, because a CancelledError skips the kill. Prevents: A6 low: agent.command_version kills a hung --version probe but never reaps it.
  • [pyright] Set reportMatchNotExhaustive = true in [tool.pyright], and make closed-enum dispatch over AgentEndStatus / StopReason use match + assert_never. With that, a missing member is a type error, and module-level assert set(...) == set(Enum) statements and runtime side-channel lists are no longer needed. Prevents: A1 low: the unhandled: list[AgentEndStatus] closure side channel in Orchestrator._communicate_with_retry, which guards an enum that is already closed.
  • [ce-lint] CE081 NoSilentManifestParseFallback, which extends CE005 (no_silent_except): also flag a NARROW handler that catches json.JSONDecodeError, yaml.YAMLError or UnicodeDecodeError when its whole body is return {} / return [] / return None / pass and it does not log or raise. The existing CE005 checks only except Exception: and bare except:, so this narrow form passes today. Known-optional reads use # noqa: CE081. Prevents: A6 medium: plugin_staging._read_manifest returns {} for a corrupt plugin.json, so validate_plugins passes a plugin that Claude Code refuses to load.
  • [ce-lint] CE082 RetiredFieldNamesNotReused: a @pytest.mark.lint class with a RETIRED_FIELD_NAMES registry, where each entry is a (model, name, retired-in commit, reason) tuple, for the CE030-tracked models (RunLimits, TaskDefinition, ...). The test asserts set(Model.model_fields) & retired_names == set(). When a field is renamed, its old name goes into the registry, and a later change that re-adds the name with a new meaning fails, instead of letting stale YAML load silently past extra="forbid". It also requires every path in config._REMOVED_DEFAULT_KNOBS to name a field that exists (AST read of the dict, checked against the RunLimits and agent-config model_fields). That catches DEFAULT_MAX_TURNS -> run_limits.max_tool_calls pointing at the wrong cap. Prevents: A7 medium: run_limits.max_turns / expected_turns reuse their origin/main names with a new meaning. A7 low: the DEFAULT_MAX_TURNS hint maps to the wrong field.

Harness improvements (not statically reachable):

  • Add a timeout-versus-crash classification case to the shared coder_eval.testing.conformance sensor: fire the watchdog's on_timeout (so decoder.timeout_hit is set), then make the SDK call raise a generic Exception, and for Claude Code also an in-turn CancelledError with cancelling()==0. Assert outcome.status is AgentEndStatus.TIMEOUT on every harness. This also gives the three copies of the SDK turn skeleton (codex/antigravity/claude_code) one behavioural contract, so extracting them into a shared Agent._run_sdk_turn is safe. Why not static: It needs a live fake SDK stream and the watchdog firing at runtime. Which status a branch returns is a runtime result, not something AST can see. Prevents: A3 medium: the untested TIMEOUT branches at codex_agent.py:757, antigravity_agent.py:645 and claude_code_agent.py:890. A1 medium: the duplicated watchdog/cancel skeleton.
  • Add a scoring-invariance matrix test: for each registered harness with run_limits.max_usd set and a model that has no rate in pricing.py, replay the same trajectory (a) finishing cleanly, (b) crashed once then retried, and (c) stopped by should_stop (early stop / tool cap / model-turn cap) before the result message. Assert that final_status is the same across (a), (b) and (c), or that resolution rejects the setup up front. Generalize it into a determinism rule: an infrastructure event (retry, crash, cap) must not change the verdict class of the same agent behaviour. Why not static: The outcome depends on runtime event order (a missing ResultMessage) and on the contents of the pricing table. Prevents: A8 high: an unpriced crashed or stopped Claude Code attempt latches BudgetUnenforceableError and turns a SUCCESS row into ERROR.
  • Add a per-harness golden test for run.json metrics: replay a fixed recorded stream for each adapter through TurnEmitter and diff has_final_reply, visible_turns, expected_tool_calls_overage, result_summary and model_turns against checked-in goldens. When a golden changes, the same diff must add a docs/REPORT_SCHEMA.md 'Historical spellings' entry, enforced by a test that the golden's hash is named in that section. Why not static: The metric values come from reducing an event stream at runtime, and the default-filled result_summary only appears after finalize(). Prevents: A8 medium: the default result_summary silently raises has_final_reply / visible_turns on Codex, OpenCode, Pi and Antigravity.
  • Extend assert_stream_balanced / conformance to replay a two-communicate() dialog (and one retry) in which the harness omits tool ids, then assert that TurnMonitor.tool_calls equals the real call count and that the collector keeps both turns' commands. Why not static: Fallback ids are minted at runtime per decoder instance, so a collision appears only across successive communicate() calls. Prevents: A8 low: the task-wide max_tool_calls undercounts because fallback ids (call_N, {root_type}_{minted}) repeat on each turn.
  • Add a TurnMonitor fault-injection test: make the shared EventCollector.on_event raise on a ToolEndEvent and on an AgentEndEvent. Assert that max_tool_calls, max_turns and max_usd still fire, that only the armed criteria disarm, and that the _DISARMED log appears only for the criteria path. Why not static: Fail-open scope is a property of control flow under an exception at runtime. An AST rule cannot know which counters count as 'required'. Prevents: A6 low: the fail-open handler also hides failures in the cap and budget accounting.
  • Add a consent-disclosure parity test: build a task whose plugin has a symlinked skill source outside its root, then assert that the host paths listed by regrade._dispatch_host_exposure equal the set of -v sources in DockerRunner._build_argv. Both sides should call one pure helper. Why not static: The mount set depends on resolving symlinks on the filesystem at runtime. Prevents: A4 low: the --allow-recorded-commands consent text leaves out the skill-source mounts and still lists the removed system_prompt_file mount.
  • Add an executable-hint test: for each entry in config._REMOVED_DEFAULT_KNOBS, feed the suggested -D <path>=x to apply_overrides on a resolved TaskDefinition and assert that it is accepted. Also add a snapshot test of every OverrideError message variant (kind None / set / unsupported), so grammar regressions show up in review. Why not static: Whether -D accepts a path is decided by schema validation of the resolved model at runtime, and message grammar needs judgment on the rendered string. Prevents: A7 low: the DEFAULT_* hint suggests a by_type path that -D rejects. A7 low: the ungrammatical sdk_options error message.
  • Add validation of agent.by_type at experiment load. For a registered kind, check the entry's keys against registration.config_class.model_fields whether or not that kind is selected. For an unregistered kind, run a near-miss check with difflib against AgentRegistry.list_kinds() and raise 'did you mean' on a match, otherwise log WARNING. Add a regression test for claude_code and for {pi: {modle: x}}. Why not static: The set of registered kinds comes from runtime plugin entry points, and the uninstalled-plugin tolerance is deliberate, so it is a load-time check, not a lint. Prevents: A2 high: by_type typos (misspelled kind or field) are silently ignored.
  • Add a structural-cap ownership test: for each SDK option that duplicates a RunLimits cap (sdk_options.max_turns on Claude Code), either assert that it is in _FRAMEWORK_OWNED_SDK_FIELDS for user YAML, or replay an SDK error_max_turns result and assert that the row records a cap fact (tool_calls_exhausted / TOOL_CALLS_EXHAUSTED), not a plain COMPLETED/FAILURE. Judge and simulator injection goes through a private path. Why not static: Whether an SDK option is a second cap is semantic knowledge of the SDK, and the status mapping is only visible on a replayed result message. Prevents: A8 medium: user-settable sdk_options.max_turns adds a second, silent cap outside TurnMonitor.
  • Resolve the harness contract once, in TurnMonitor.for_task, and add a test that registration_for is the only registry lookup reached per task. Patch AgentRegistry.get with a call counter over a full Orchestrator run, including the evaluate-only re-grade path, where the lookup must stay lenient. Why not static: Counting lookups per task needs a runtime call count across the orchestrator flow. The lenient and strict variants have different names, so a grep cannot tell them apart. Prevents: A1 medium: the contract lookup is duplicated (turn_monitor._reports_cost vs orchestrator registration_for / _counts_model_turns).

Top 5 Priority Actions

  1. Stop a Claude Code turn with no ResultMessage (a crash retry, an early stop or a cap stop) from latching unpriced and turning a SUCCESS row into ERROR when max_usd is set and the model has no rate. Either require a priced agent.model at resolution for every harness that sets max_usd, or skip that turn's cost with a warning (src/coder_eval/orchestration/turn_monitor.py:329-345, harness_contract.py _check_max_usd_priceable).
  2. Put max_turns back in _FRAMEWORK_OWNED_SDK_FIELDS for user YAML, and let the judge and simulator set it through a private path. Otherwise, map the SDK stop error_max_turns to TOOL_CALLS_EXHAUSTED, so that a user-set sdk_options.max_turns no longer ends as a plain COMPLETED/FAILURE with no cap fact (src/coder_eval/models/agent_config.py:88, src/coder_eval/agents/claude_code_agent.py:913 and 1459).
  3. Document or version the has_final_reply / visible_turns definition change that TurnEmitter.finalize's default result_summary causes on Codex, OpenCode, Pi and Antigravity rows, so trend dashboards and expected_tool_calls_overage do not shift silently (src/coder_eval/streaming/emitter.py:404-407, docs/REPORT_SCHEMA.md Historical spellings). In the same change, give BudgetUnenforceableError a typed error category (src/coder_eval/errors/categorization.py, src/coder_eval/errors/budget.py:31).
  4. Before SPI v1 ships, remove the agents-to-orchestration import cycle: move staged_plugin_dirs and link_or_copy into a leaf module and TaskResolutionError into errors/, and move STDOUT_LINE_LIMIT_BYTES out of docker_runner. Also fix the spi.py docstring so plugins pass a literal spi_version, and consider replacing the UiPath-specific plugin_tools_dir on Agent.start with a generic env seam (src/coder_eval/orchestration/plugin_staging.py:27, src/coder_eval/agents/_transport/subprocess_jsonl.py:27, src/coder_eval/spi.py:3, src/coder_eval/agent.py:72).
  5. Validate agent.by_type entries: check each registered kind's fields against its config class, and give a 'did you mean' error for a near-miss kind such as claude_code, so a typo cannot silently run the default model (src/coder_eval/orchestration/experiment.py:389-424). Then reduce Orchestrator._setup (D24) and _simulation_dialog_loop (F45) by extracting plugin staging, the harness version and one shared turn-facts helper with a single order of operations (src/coder_eval/orchestrator.py:1416 and 2301).

Stats: 0 🔴 · 4 🟠 · 13 🟡 · 13 🔵 across 8 axes reviewed.

`max_turns` and `expected_turns` were rejected at resolution on Codex and
Antigravity because each opened one inner turn per communicate(), so the
TurnMonitor would have counted calls. Both streams already carry a
per-generation boundary (Codex's thread/tokenUsage/updated, Antigravity's
usage_metadata Step); each now opens one inner turn per generation and closes
it at the cut with that generation's delta as its tokens, and the contract
declares usage_granularity=GENERATION.

The inner turn opens lazily at the first evidence of a generation, never
eagerly after a cut: a tool result landing after the cut (Codex's cross-flush
is_error patch, an Antigravity background job resolving on a later poll) is
not a model turn. The cap therefore latches where it does on Claude Code,
when response N+1 arrives. Token and USD budgets now overshoot by one
generation instead of one whole turn.

Verified live with tasks/run_limits/max_turns_cap.yaml on both harnesses:
model_turns matches the assistant responses in the transcript, and the
per-generation deltas sum to the turn total with no residual.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@uipreliga
uipreliga marked this pull request as ready for review September 18, 2026 00:31
@github-actions

github-actions Bot commented Sep 18, 2026

Copy link
Copy Markdown

Claude finished @uipreliga's task in 2m 3s —— View job


Code Review in Progress

Todo List:

  • Read .github/code_review.md for review guidelines
  • Read CLAUDE.md for project conventions
  • Analyze PR diff (origin/feat/harness-central-enforcement...HEAD)
  • Read key changed files for context
  • Analyze cross-file consistency
  • Perform "what's missing" analysis
  • Complete design-level scrutiny
  • Provide structured review feedback

Starting review now...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant