From 6e1939d5d494dead5565ff9f3d324f2f5c1c79c9 Mon Sep 17 00:00:00 2001 From: Bai Li Date: Wed, 12 Aug 2026 16:01:21 -0700 Subject: [PATCH 01/15] feat(agents)!: make a base-config field mean the same thing on every harness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #68. Closes #108. `system_prompt` is now defined as text APPENDED to each harness's own default agent prompt, and every backend maps it to its additive knob. This is a breaking change on claude-code, which previously mapped it to `--system-prompt` (replacement) and — because the SDK emits `--system-prompt ""` for None — ran with no system prompt at all when the field was unset. `run_limits.max_turns` was accepted and ignored on codex and antigravity. Both now cap on visible turns (resolved tool calls) read off a shared `EventCollector.visible_turn_count`, enforced on the same loop boundary as the cooperative early stop. claude-code keeps its native SDK cap. Antigravity now honors `allowed_tools` / `disallowed_tools` via `CapabilitiesConfig`, so the same task file exposes the same tool surface on all three backends. Structural tools are never stripped, and an allowlist that maps to nothing usable falls back to the harness default with a warning. Where a backend genuinely cannot implement a field it now declares that on its agent class (`Agent.config_support`) rather than dropping it silently: `validate_config_support` hard-errors at resolution on an unhonored field set to a non-default value, mirroring `validate_early_stop`. Today's declarations are all APPROXIMATED, so no existing run changes. Also drops the report's "N turn(s) avoided" claim (derived from `max_turns - sdk_turn_index`, which overstated by the whole budget on the single-SDK-turn backends), and pins the user simulator's model instead of letting BEDROCK_MODEL swap the simulated user underneath an A/B. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 2 + README.md | 1 + docs/TASK_DEFINITION_GUIDE.md | 6 +- docs/agents/ANTIGRAVITY.md | 14 +- docs/agents/CLAUDE_CODE.md | 2 +- docs/agents/CODEX.md | 8 +- docs/agents/HARNESS_PARITY.md | 114 ++++ docs/index.md | 1 + docs/llms.txt | 1 + mkdocs.yml | 2 + src/coder_eval/agent.py | 38 ++ src/coder_eval/agents/antigravity_agent.py | 278 +++++++--- src/coder_eval/agents/claude_code_agent.py | 24 +- src/coder_eval/agents/codex_agent.py | 110 +++- src/coder_eval/cli/plan_command.py | 9 + src/coder_eval/models/__init__.py | 2 + src/coder_eval/models/agent_config.py | 13 +- src/coder_eval/models/results.py | 23 +- src/coder_eval/models/tasks.py | 27 +- .../orchestration/config_support.py | 100 ++++ src/coder_eval/orchestration/experiment.py | 14 +- src/coder_eval/orchestrator.py | 18 +- src/coder_eval/reports.py | 22 +- src/coder_eval/simulation/user_simulator.py | 47 +- src/coder_eval/streaming/collector.py | 18 + tests/test_agent_config_support.py | 140 +++++ tests/test_antigravity_agent.py | 493 +++++++++++++----- tests/test_codex_agent.py | 78 +++ tests/test_early_stop.py | 14 +- tests/test_run_limits_orchestrator.py | 10 + tests/test_system_prompt_parity.py | 194 +++++++ 31 files changed, 1566 insertions(+), 257 deletions(-) create mode 100644 docs/agents/HARNESS_PARITY.md create mode 100644 src/coder_eval/orchestration/config_support.py create mode 100644 tests/test_agent_config_support.py create mode 100644 tests/test_system_prompt_parity.py diff --git a/CLAUDE.md b/CLAUDE.md index a1402582..23c9347c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -81,6 +81,7 @@ coder_eval/ ├── orchestration/ # Batch execution utilities │ ├── batch.py # Parallel task execution (run_batch + run_batch_resolved) │ ├── config.py # Batch run configuration +│ ├── config_support.py # validate_config_support: rejects a task setting a field its agent declares unhonored │ ├── early_stop.py # validate_early_stop guardrails + EarlyStopWatcher (armed live-verdict observer) │ ├── evaluation.py # Evaluation helpers │ ├── experiment.py # ExperimentRunner, resolve_task_for_variant, load_experiment @@ -140,6 +141,7 @@ action.yml # Published composite GitHub Action (coder-ev - **Per-criterion aggregation**: Each `BaseCriterion` subclass exposes `aggregate(criterion, per_row_results) -> CriterionAggregate | None`. Default emits `count / mean / median / std / min / max` so every criterion is suite-thresholdable for free. Classification-style criteria return `ClassificationCriterionResult` (subclass of `CriterionResult`) and layer accuracy / P/R/F1 / confusion via the shared `overlay_classification_metrics` utility. `BaseSuccessCriterion.suite_thresholds` gates the suite on those metrics; CLI exits non-zero on any gate failure. - **Sub-agent token accounting**: There is NO separate per-sub-agent field. Every sub-agent generation is captured as a `parent_tool_use_id`-tagged `AssistantMessage` in the turn transcript, so per-sub-agent usage is derived by grouping those messages on that id (the evalboard's `aggregateSubAgentUsage` does exactly this). Claude bubbles its sub-agent's intermediate generations into the parent stream natively, and the **terminal** generation (delivered as the Agent tool result, never streamed) is synthesized into one via `_synthesize_subagent_terminal_message` from `tool_use_result.usage`. Codex reconstructs all child generations from the child rollout (`_recover_subagent_tool_calls`). The turn total already includes sub-agent cost — Claude via the SDK's cumulative `model_usage`; Codex via `_fold_subagent_tokens`, which folds the child messages (their real per-generation tokens) into the parent total. `CommandTelemetry.result_summary` is stored **untruncated** (no 200-char cap) so sub-agent returns are preserved whole. Set `CODER_EVAL_RAW_SDK_LOG=1` to dump every raw SDK event to the task log for inspection. - **Reconciliation message (stream self-reconciles to the turn total)**: The per-message stream consistently under-reports the authoritative turn total — a fixed prompt slice (~512 input tokens on Claude) is billed on no SDK-emitted message, and sub-agent input/cache only partially bubbles up. So `EventCollector.build_turn_record` appends one synthetic `ReconciliationMessage` (`role="reconciliation"`, in the `TranscriptMessage` union) per turn, carrying the per-bucket residual = `token_usage` − Σ(assistant message buckets). The invariant: **summing the four token buckets across `TurnRecord.messages` (assistant + reconciliation) equals `token_usage` exactly**, for both Claude and Codex (Codex's stream is already complete after `_recover_subagent_tool_calls`, so its residual is usually 0 and no entry is emitted). This is what lets the evalboard SUM the message stream as the source of truth instead of reading a separate aggregate ("agent tokens"): `selectTokenTotals` returns the stream sum whenever a reconciliation entry is present, and the timeline renders it as its own row. It is agent-agnostic (booked at the single `EventCollector` seam), carries no cost (cost stays on `token_usage`), and is excluded from generation/turn counts and the cost simulator. The LiteLLM open-weight actual-cost join (`litellm_cost.apply_actual_cost`) deliberately writes cost at the TURN level only (`token_usage.total_cost_usd` = the real OpenRouter bill) plus the per-call `TurnRecord.provider_call_costs` audit record; it does NOT touch the message token buckets, so `EventCollector` stays the single writer and this invariant holds on every backend. The Python `token_usage`/`total_token_usage` aggregate is unchanged and still authoritative for budget/judges/reports. +- **Harness config parity (`Agent.config_support`)**: a shared `BaseAgentConfig` field must mean the same thing on every backend. Where one cannot implement a field it DECLARES the divergence on its agent class as `{field: ConfigFieldSupport(state, reason)}` — `APPROXIMATED` (acted on with a documented divergence; the agent warns at `start()`) or `UNHONORED` (read by nothing; `orchestration/config_support.py::validate_config_support` hard-errors at resolution when the task sets it to a non-default value, in the style of `validate_early_stop`, and it is wired at the same three seats). An empty map asserts full support, so a silently dropped field is a bug rather than a shortcut. Today's divergences: `permission_mode` on Codex and Antigravity (both run unconfined — the sandbox driver is the isolation boundary) and `disallowed_tools` on Codex (forwarded, not SDK-enforced). Two field semantics are pinned by this contract: **`system_prompt` APPENDS** to each harness's own default agent prompt (`--append-system-prompt` / `developer_instructions` / a `SystemInstructionSection`) — never replaces it, since a task-level guardrail is not a whole agent prompt — and **`run_limits.max_turns` on Codex/Antigravity counts VISIBLE turns** (resolved tool calls, read live off the shared `EventCollector.visible_turn_count`, the same list `TurnRecord.commands` holds) because one `communicate()` is a single SDK turn on both, so a native counter would clamp at 1; claude-code keeps its native SDK cap. The cap is enforced on the same loop boundary as the cooperative early stop and finalizes cleanly as `max_turns_exhausted` (no crash, no retry). Full table + rationale: docs/agents/HARNESS_PARITY.md. - **sandbox isolation**: Tasks that don't need MCP servers should set `setting_sources: []` in their `agent:` block to isolate the sandbox from the host project's CLAUDE.md and settings. Without this, the host project's CLAUDE.md (often 20 KB+) is injected into every API call, inflating cache-creation tokens and cost significantly. - **Run-time caps (non-criterion enforcement)**: `TaskDefinition.run_limits` (`RunLimits` model) is the single namespace for all *task-level* run-time caps — `max_turns` / `task_timeout` / `turn_timeout` (structural) and `max_input_tokens` / `max_output_tokens` / `max_total_tokens` / `max_usd` (cumulative budget). Token/USD breaches abort with `FinalStatus.TOKEN_BUDGET_EXCEEDED` or `COST_BUDGET_EXCEEDED` (both `category == "failed"`). Structural caps are set from the CLI via `-D run_limits.max_turns=…` / `-D run_limits.task_timeout=…` / `-D run_limits.turn_timeout=…` (field-merged into `run_limits`); budget caps via `-D run_limits.max_usd=…` etc. or YAML. Layered config uses field-merge — a variant block overrides individual keys without replacing the task's block. The one *per-criterion* cap, `stop_early.decide_within`, deliberately lives on `LiveSuccessCriterion` instead (see below) — the watcher must attribute a decision-step timeout to a specific criterion, which `RunLimits` (task-scoped, criterion-agnostic) cannot express. - **Early stop on criterion (opt-in, per-criterion arming)**: a `stop_early:` block (`StopEarlyPolicy`) on a criterion ends a single-shot run early once the run's **armed** criteria decide the outcome, so a raised `max_turns` isn't wasted on the smoke flavor. The block's PRESENCE is the arming and alone activates the watcher — there is **no run-level master switch**: `run_limits.stop_early: false` is the run-level KILL SWITCH that force-disarms every block (the one-line experiment-variant/`-D` override for an authoritative full run), and `run_limits.stop_early: true` (the removed master arm) is a hard `EarlyStopConfigError` at resolution. The block exists on `LiveSuccessCriterion` only (currently `skill_triggered`, `command_executed` — so arming an unobservable criterion is unrepresentable, a pydantic extra-forbid error). Arming carries one implicit trigger (a native live-fail may fail-stop the run); its keys refine it: `on_pass: stop` (pass-stop the moment the criterion live-passes; default `continue` just latches) and `decide_within: N` (still undecided after N tool-call steps latches an **effective fail**, fed through the same fail-stop rule, reported as `decision_budget_exceeded` — an ordinary weighted fail, NOT a gate-bypassing force-fail; cumulative across retry attempts of the same turn). A trigger whose polarity the instance can't decide (per the abstract, checker-independent `live_decidable_polarities()`, a pure function of the criterion's own fields, paired with the checker's `live_verdict` override by lint rule CE025, a registry-based whole-tree check) is **inert by design** — one dataset-fanned YAML line serves both positive rows (pass/timeout live) and distractor rows (fail live). Verdicts **latch**: once a criterion decides, its `live_verdict` is never polled again. Stop rule is weighted, not strict-boolean: `run_limits.stop_early_gate_threshold` (default `1.0`, reproducing strict-AND behavior exactly) is the minimum weighted score (`Σ weight·score / Σ weight` over the armed subset) required to pass; a fail-stop fires once the armed set's **ceiling** (best case for everything still undecided) can no longer reach the threshold — so a low-weight fail or timeout that can't doom the gate is absorbed and the run continues — and is **deferred while any pass-capable armed criterion is undecided** (a distractor misfire never truncates a positive row's recall signal); a pass-stop fires once the `on_pass: stop` subset's **floor** (worst case) already meets the threshold, and is symmetrically **deferred while any pass-capable armed criterion outside the `on_pass: stop` subset is undecided** (so an early pass never freezes a sibling `on_pass: continue` criterion's signal out of the trajectory). A fail-stop is therefore verdict-preserving; a pass-stop can miss a *later* distractor misfire, so authoritative P/R/F1 comes from a kill-switched (`stop_early: false`) run. Driven by `orchestration/early_stop.py::EarlyStopWatcher` (built when `early_stop_active(task)`: ≥1 armed criterion, kill switch not thrown) through the agent's cooperative `should_stop` seam (tool-call granularity, no SIGKILL); live verdicts only *trigger* the stop — the standard `check_all_async` on the frozen trajectory is authoritative. Gating is **FIRED-ONLY**: a run the watcher actually cut gates on the **armed subset** via the weighted `EvaluationResult.armed_criteria_passed`; a run that completes naturally — armed or not — gates strict-AND via `all_criteria_passed`, so adding a block never changes the verdict of a run it didn't cut. Note the gate keys on the watcher having FIRED (`result.early_stop is not None`), not on confirmed truncation — an agent that ignores `should_stop`, or a stop firing on the final message, still gates armed-only. Every resolution-time guardrail violation is a hard error at resolution (plan *and* run); the one load-time case — a `stop_early:` block on a non-live criterion — is a pydantic schema error at task load, which the run surface reports as a skipped task like any other malformed task. A runtime verdict bug **fails open** to a full run. Surfaces: `EarlyStopInfo` (incl. `gate_threshold` at stop time), report notes/badges, `stopped_early` run.json rows, `EarlyStopped`/`EarlyStopReason` telemetry dims. Worked rationale: docs/TASK_DEFINITION_GUIDE.md § `stop_early`. No blocks anywhere ⇒ behavior byte-for-byte unchanged. diff --git a/README.md b/README.md index d3b9a2e5..b666289e 100644 --- a/README.md +++ b/README.md @@ -192,6 +192,7 @@ alone. | [Claude Code](docs/agents/CLAUDE_CODE.md) | Configuring and running the default Claude Code agent | | [Codex](docs/agents/CODEX.md) | Running the OpenAI Codex agent | | [Antigravity (Gemini)](docs/agents/ANTIGRAVITY.md) | Running the Google Antigravity / Gemini agent | +| [Harness Config Parity](docs/agents/HARNESS_PARITY.md) | What each agent: field means on every harness, and where they diverge | | [A/B Experiments](docs/AB_EXPERIMENTS.md) | Compare models / tools / prompts across the same tasks | | [Bring Your Own Dataset](docs/DATASETS.md) | Fan a single task out over a dataset | | [Dialog Mode](docs/DIALOG_MODE.md) | Evaluate agents in multi-turn conversation via a simulated user | diff --git a/docs/TASK_DEFINITION_GUIDE.md b/docs/TASK_DEFINITION_GUIDE.md index e96ac153..81bac9d6 100644 --- a/docs/TASK_DEFINITION_GUIDE.md +++ b/docs/TASK_DEFINITION_GUIDE.md @@ -1392,6 +1392,9 @@ simulation: # Sampling (variance analysis). n_trials: 3 # Run N independent dialogs per (task, variant). + # Who plays the simulated user. Pinned, NOT inherited from the run's route. + model: anthropic.claude-sonnet-4-6 + # Criteria timing. check_criteria: every_turn # One of: end_of_dialog | every_turn | both. # Required to be 'every_turn' or 'both' when @@ -1410,8 +1413,9 @@ simulation: | `max_total_tokens` | *unset* | Optional dialog-wide token budget (simulator **plus** agent). Distinct from [`run_limits.max_total_tokens`](#run-limits) — see below. | | `n_trials` | `1` | Independent dialog trajectories per (task, variant). | | `check_criteria` | `end_of_dialog` | `end_of_dialog`, `every_turn`, or `both`. | +| `model` | `anthropic.claude-sonnet-4-6` | Model that plays the simulated user. Auto-translated to the run's backend (Bedrock inference profile / bare Anthropic alias), the same way [`llm_judge`](#llm_judge)'s `model` is. | -The simulator runs as a tools-disabled Claude Code agent sharing the coding agent's `ApiRoute` — model/temperature/sampling are resolved at the route level (same `-b` flag as the coding agent), so they are not configured on this block. +The simulator runs as a tools-disabled Claude Code agent sharing the coding agent's `ApiRoute`, so temperature and sampling are resolved at the route level (same `-b` flag as the coding agent) and are not configured on this block. The **model is not**: it is pinned by `model` above. Inheriting it from the route meant `BEDROCK_MODEL` decided who the simulated user was, so an A/B varying the subject model silently varied its interlocutor too. Hold `model` fixed across variants for the same reason you hold a judge model fixed — the simulator is part of the measuring instrument, not the thing being measured. **Semantics:** diff --git a/docs/agents/ANTIGRAVITY.md b/docs/agents/ANTIGRAVITY.md index 522dc8bf..1b2df5dd 100644 --- a/docs/agents/ANTIGRAVITY.md +++ b/docs/agents/ANTIGRAVITY.md @@ -176,10 +176,16 @@ as every other agent. 3. **`kill_sync()` is best-effort.** The SDK's cancel/disconnect are async-only, so the watchdog's synchronous kill only flips agent state to `ERROR`; real teardown happens on the subsequent async `stop()`. -4. **Process-global spawn lock.** The SDK spawns `localharness` via a subprocess with - no env-injection seam, so the agent transiently mutates `PATH` across the spawn - under a process-wide lock. This serializes harness startup across concurrent - tasks (it does not serialize the turns themselves). +4. **`permission_mode` does not confine the harness.** Every mode runs + `policy.allow_all()`; coder_eval's write boundary is the sandbox driver, and a + headless eval has no human to approve anything. Declared `approximated` in the + agent's `config_support` — see [Harness Config Parity](HARNESS_PARITY.md). +5. **`allowed_tools` entries with no Antigravity builtin are dropped.** `Skill` is the + expected case (skills are discovered through `skills_paths`, not a tool); the drop + is logged. An allowlist that maps to *nothing* usable falls back to the harness + default with a warning, rather than leaving the model only its turn-ender. +6. **`max_turns` counts visible turns.** One `communicate()` is a single SDK turn here, + so the cap counts resolved tool calls instead, enforced on the step loop. ## Running in Docker diff --git a/docs/agents/CLAUDE_CODE.md b/docs/agents/CLAUDE_CODE.md index 66670709..cd540b49 100644 --- a/docs/agents/CLAUDE_CODE.md +++ b/docs/agents/CLAUDE_CODE.md @@ -99,7 +99,7 @@ agent: | `allowed_tools` | `list[str] \| null` | Tool allowlist. Unset ⇒ all tools allowed. | | `disallowed_tools` | `list[str] \| null` | Tool denylist. (`ToolSearch` is always appended for Bedrock parity.) | | `plugins` | `list[{type: local, path}]` | Local plugin/skill directories; `$VAR` in `path` is expanded and resolved to an absolute path. | -| `system_prompt` | `str \| null` | **Replaces** the default system prompt (there is no *append* seam). Mutually exclusive with `system_prompt_file`. | +| `system_prompt` | `str \| null` | Text **appended** to Claude Code's default system prompt (via `--append-system-prompt`), so the same task file means the same thing on every harness — see [Harness Config Parity](HARNESS_PARITY.md). Mutually exclusive with `system_prompt_file`. | | `system_prompt_file` | `str \| null` | Path (relative to the task YAML) loaded into `system_prompt` at resolution. | | `setting_sources` | `list["user"\|"project"\|"local"] \| null` | Which host setting sources the SDK reads. Default resolves to `["project"]`. See [Sandbox isolation](#sandbox-isolation). | | `claude_settings` | `str \| dict \| null` | Passed to the SDK `--settings`. A dict is JSON-serialized; a str is a settings file path. Use `permissions.deny` to block tools/paths. | diff --git a/docs/agents/CODEX.md b/docs/agents/CODEX.md index b020e76b..b7f4f7c5 100644 --- a/docs/agents/CODEX.md +++ b/docs/agents/CODEX.md @@ -214,15 +214,19 @@ The Codex SDK is synchronous. The agent uses `_run_async()` helper to detect and | **Command Tracking** | Full telemetry (tool name, params, duration) | Streamed telemetry: shell → `Bash`, apply_patch → `Write` | | **Model Selection** | Direct via `--model` or config | `agent.model` pinned into `thread_start` | | **Session Resume** | `--resume {session_id}` | Via thread ID | -| **Permissions** | `permission_mode` + `allowed_tools` | `permission_mode` → sandbox/approval + `allowed_tools`/`disallowed_tools` → thread config | +| **Permissions** | `permission_mode` honored | `permission_mode` runs full-access on every mode — the sandbox driver is the boundary | | **Tool Enforcement** | Not enforced by Coder Eval wrapper | `enabled_tools` honored; `disabled_tools` NOT enforced by the SDK | +| **System prompt** | `--append-system-prompt` | `developer_instructions` on `thread_start` (also additive) | +| **`max_turns`** | Native SDK turn cap (assistant messages) | Visible-turn cap (tool calls), enforced on the notification pump | | **Early stop** | Supported (cooperative `should_stop`, polled between messages) | Supported — polled after each streamed notification; the in-flight turn is interrupted best-effort | +Field-by-field, with the reasoning behind each divergence: [Harness Config Parity](HARNESS_PARITY.md). + ## Known Limitations 1. **Tool-name collapse** - Codex reports shell tools (`Read`/`Grep`/`Bash`) all as shell commands, surfaced as `Bash` telemetry; name-keyed criteria that distinguish these tools aren't meaningful across agents. 2. **`skill_triggered` criterion** - Codex has no distinct `Skill` tool (it engages a skill by reading its files via shell), so the criterion detects Codex engagement from that file-read signal (a command referencing `skills//`) instead of a `Skill` tool call. The file-read signal is weaker than Claude's explicit invocation. -3. **`disallowed_tools`** - passed to the SDK but not enforced; not a security boundary. +3. **`disallowed_tools`** - passed to the SDK but not enforced; not a security boundary. Declared `approximated` in the agent's `config_support`. 4. **Authentication** - Requires `CODEX_API_KEY` in the environment (point it at whichever endpoint's key you use — OpenAI, gateway, or Azure); the agent calls `login_api_key` when a key is present. `OPENAI_API_KEY`/`AZURE_OPENAI_API_KEY` are NOT read. 5. **Model field** - `TurnRecord.model_used` reflects the pinned `agent.model`; the Codex `Turn` payload itself doesn't carry the resolved model. 6. **Skills with Windows paths** - Symlink creation may fail on Windows; agent falls back to copying (slower). diff --git a/docs/agents/HARNESS_PARITY.md b/docs/agents/HARNESS_PARITY.md new file mode 100644 index 00000000..f3460183 --- /dev/null +++ b/docs/agents/HARNESS_PARITY.md @@ -0,0 +1,114 @@ +# Harness Config Parity + +One task file, run on three harnesses, must be the same task. This page is the +contract for how each shared `agent:` field is implemented on Claude Code, Codex, +and Antigravity — and, where a backend genuinely cannot implement one, what it +does instead. + +The declarations here are not prose: each agent class carries them as +`config_support`, and resolution rejects a task that sets a field its agent +declares it cannot honor. + +## Support states + +| State | Meaning | What happens | +|-------|---------|--------------| +| **honored** | Implemented faithfully. | Nothing to declare — the default. | +| **approximated** | Acted on, with a documented divergence. | The agent warns at `start()`; resolution allows it. | +| **unhonored** | Read by nothing. | Resolution **hard-errors** if the task sets it to a non-default value. | + +An agent declares only its divergences. An empty `config_support` asserts it +honors every shared field — so a field silently dropped without a declaration is +a bug, not a shortcut. + +## The table + +| Field | claude-code | codex | antigravity | +|---|---|---|---| +| `model` | honored | honored | honored | +| `system_prompt` / `system_prompt_file` | honored (`--append-system-prompt`) | honored (`developer_instructions`) | honored (`system_instructions` section) | +| `allowed_tools` | honored | honored (`enabled_tools`) | honored (`CapabilitiesConfig.enabled_tools`) | +| `disallowed_tools` | honored | **approximated** — forwarded as `disabled_tools`, not enforced by the SDK | honored (subtracted from the allowlist) | +| `permission_mode` | honored | **approximated** — every mode runs full-access | **approximated** — every mode runs `policy.allow_all()` | +| `plugins` | honored | honored (symlinked into `.agents/skills/`) | honored (`skills_paths`) | +| `run_limits.max_turns` | honored (native SDK turn cap) | honored (visible-turn cap) | honored (visible-turn cap) | +| `run_limits.stop_early` | honored | honored | honored | + +## `system_prompt` means *append* + +`agent.system_prompt` is extra text **appended to the harness's own default agent +prompt**. It does not replace it. + +Append is the only semantics all three can express safely. Full replacement is +expressible too, but a task-level guardrail (`"Do not access files in sibling +runs/* directories"`) is not a whole agent prompt — substituting one for Codex's +base instructions or Antigravity's core mandates would gut the harness rather than +constrain it. So the field is defined as the safe one, and each backend maps it to +its own additive knob: + +| Harness | Additive knob | Replacement knob (deliberately unused) | +|---|---|---| +| claude-code | `--append-system-prompt` | `--system-prompt` | +| codex | `developer_instructions` | `base_instructions` | +| antigravity | `system_instructions` (str → `TemplatedSystemInstructions`) | `CustomSystemInstructions` | + +Write task guardrails here, not a persona. + +> **Note on the claude-code change.** This field previously mapped to +> `--system-prompt`, which *replaced* Claude Code's prompt — and because the SDK +> emits `--system-prompt ""` for `None`, a run that set nothing got **no** system +> prompt at all, while Codex and Antigravity kept their full vendor prompts. Both +> cases now route through the preset, so every harness starts from its own default +> prompt and adds the task's text on top. Expect claude-code numbers to move +> against a pre-change baseline. + +## `max_turns` counts visible turns on Codex and Antigravity + +A "visible turn" is one entry in the run's timeline: one resolved tool call. It is +the unit `reports_stats.visible_turn_count` reports and the unit that lands in +`TurnRecord.commands`. Both backends count it live off the shared +`EventCollector.visible_turn_count`, so one `max_turns` value means one thing on +both. + +They need their own counter because a native one would be meaningless: Codex and +Antigravity each deliver exactly **one SDK turn per `communicate()` call**, so an +SDK-level cap would clamp at 1 no matter what the task asked for. Before this, +both simply ignored the field. + +The cap is enforced on the same loop boundary as the cooperative early stop: the +step or notification that reaches the cap is processed whole, and the next one is +never pulled. The in-flight turn is then cancelled server-side (best effort) so +the cap actually stops spend. A run cut this way finalizes cleanly as +`max_turns_exhausted` — it is not a crash, and it is not retried. + +**claude-code keeps its native SDK cap**, which counts assistant messages instead. +That is a real, honored cap, so it is left alone rather than restated in a +different unit; the same `max_turns: 20` therefore bounds slightly different things +on claude-code than on the other two. Documented rather than papered over. + +## `permission_mode` does not confine any harness + +On Codex and Antigravity, every mode runs unconfined, by design: + +- coder_eval's isolation boundary is the **sandbox driver** — a Docker container, + or an ephemeral per-task tempdir it creates and discards. An in-agent approval + policy on top of that is redundant. +- Codex's own OS sandbox actively breaks on the paths we run: Landlock is + unavailable inside the container, the `bwrap` re-exec is denied on constrained CI + agents, and Windows has no OS sandbox at all. In each case writes fail silently + and the task scores 0 with no loud error. +- The modes below `bypassPermissions` differ only in *what they would ask a human + about*, and there is no human on a headless eval path. + +This is declared as **approximated** rather than unhonored — the isolation the +field implies is provided, one layer down — so setting `bypassPermissions` on a +nightly does not fail resolution. + +**For adversarial or untrusted evals, use the Docker driver.** The tempdir/host +driver is a working directory, not a confinement boundary, on any of the three. + +## Related + +- [Claude Code](CLAUDE_CODE.md) · [Codex](CODEX.md) · [Antigravity](ANTIGRAVITY.md) +- [Task Definition Guide](../TASK_DEFINITION_GUIDE.md) — the full `agent:` schema +- [Extending Coder Eval](../EXTENDING.md) — declaring `config_support` on a new agent diff --git a/docs/index.md b/docs/index.md index d928436e..17c0509a 100644 --- a/docs/index.md +++ b/docs/index.md @@ -81,6 +81,7 @@ New here? Start with **[Tutorial 01 — Your First Evaluation](tutorials/01-firs | [Claude Code](agents/CLAUDE_CODE.md) | Configuring and running the default Claude Code agent | | [Codex](agents/CODEX.md) | Running the OpenAI Codex agent | | [Antigravity (Gemini)](agents/ANTIGRAVITY.md) | Running the Google Antigravity / Gemini agent | +| [Harness Config Parity](agents/HARNESS_PARITY.md) | What each agent: field means on every harness, and where they diverge | | [A/B Experiments](AB_EXPERIMENTS.md) | Compare models / tools / prompts across the same tasks | | [Bring Your Own Dataset](DATASETS.md) | Fan a single task out over a dataset | | [Dialog Mode](DIALOG_MODE.md) | Evaluate agents in multi-turn conversation via a simulated user | diff --git a/docs/llms.txt b/docs/llms.txt index 260f6fe0..3a83937a 100644 --- a/docs/llms.txt +++ b/docs/llms.txt @@ -28,6 +28,7 @@ and A/B plumbing. - [Claude Code](https://coder-eval.com/docs/agents/claude-code): Configuring and running the default Claude Code agent - [Codex](https://coder-eval.com/docs/agents/codex): Running the OpenAI Codex agent - [Antigravity (Gemini)](https://coder-eval.com/docs/agents/antigravity): Running the Google Antigravity / Gemini agent +- [Harness Config Parity](https://coder-eval.com/docs/agents/harness-parity): What each agent: field means on every harness, and where they diverge - [A/B Experiments](https://coder-eval.com/docs/ab-experiments): Compare models / tools / prompts across the same tasks - [Bring Your Own Dataset](https://coder-eval.com/docs/datasets): Fan a single task out over a dataset - [Dialog Mode](https://coder-eval.com/docs/dialog-mode): Evaluate agents in multi-turn conversation via a simulated user diff --git a/mkdocs.yml b/mkdocs.yml index d1da7828..eb3af209 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -83,6 +83,7 @@ extra: agents/CLAUDE_CODE.md: "Configuring and running the default Claude Code agent" agents/CODEX.md: "Running the OpenAI Codex agent" agents/ANTIGRAVITY.md: "Running the Google Antigravity / Gemini agent" + agents/HARNESS_PARITY.md: "What each agent: field means on every harness, and where they diverge" AB_EXPERIMENTS.md: "Compare models / tools / prompts across the same tasks" DATASETS.md: "Fan a single task out over a dataset" DIALOG_MODE.md: "Evaluate agents in multi-turn conversation via a simulated user" @@ -109,6 +110,7 @@ nav: - Claude Code: agents/CLAUDE_CODE.md - Codex: agents/CODEX.md - Antigravity (Gemini): agents/ANTIGRAVITY.md + - Harness Config Parity: agents/HARNESS_PARITY.md - Advanced: - A/B Experiments: AB_EXPERIMENTS.md - Bring Your Own Dataset: DATASETS.md diff --git a/src/coder_eval/agent.py b/src/coder_eval/agent.py index e5ebbf9a..c2d5e276 100644 --- a/src/coder_eval/agent.py +++ b/src/coder_eval/agent.py @@ -6,6 +6,8 @@ import logging from abc import ABC, abstractmethod from collections.abc import Callable +from dataclasses import dataclass +from enum import StrEnum from typing import Any, ClassVar, NoReturn, Protocol from .errors import AgentCrashError, TurnTimeoutError @@ -20,6 +22,34 @@ logger = logging.getLogger(__name__) +class ConfigSupport(StrEnum): + """How faithfully one agent backend implements a shared ``BaseAgentConfig`` field. + + A base-config field must mean the same thing on every harness, and where it + cannot, the divergence has to be declared rather than discovered from a run that + quietly did something else. Every field an agent does not fully implement is + listed in its ``config_support`` map with one of these and a reason. + """ + + APPROXIMATED = "approximated" + """Accepted and acted on, but with a documented divergence the operator must know + about (e.g. Codex forwards ``disallowed_tools`` to the SDK, which does not enforce + it). The agent warns at ``start()``; resolution does NOT reject.""" + + UNHONORED = "unhonored" + """Read by nothing — setting it changes no behavior. Resolution HARD-ERRORS when a + task sets the field to anything other than its model default, because a silently + dropped field means two harnesses reading one task file run different tasks.""" + + +@dataclass(frozen=True) +class ConfigFieldSupport: + """One entry in an agent's ``config_support`` map: the state plus why.""" + + support: ConfigSupport + reason: str + + class _FinalizeFn(Protocol): """The per-turn ``finalize`` callback shared by every agent's turn-state. @@ -86,6 +116,14 @@ def __init__(self, config: ClaudeCodeAgentConfig, ...): # crash every agent (NoOp/Codex/Antigravity/plugins) whose ``__init__`` lacks it. supports_cost_log_tags: ClassVar[bool] = False + # Declared divergences from the shared ``BaseAgentConfig`` contract, keyed by field + # name. Empty (the default) asserts the agent honors every field, so a new agent + # opts in to scrutiny only where it must — but a field it silently drops without + # declaring is a bug, not a shortcut. Read by + # ``orchestration/config_support.py::validate_config_support`` at resolution and by + # the parity table in docs/agents/HARNESS_PARITY.md. + config_support: ClassVar[dict[str, ConfigFieldSupport]] = {} + def _begin_turn(self) -> None: """Mark the start of a ``communicate()`` turn: reset the pending slot and bump the iteration counter so a mid-turn failure can be rolled back. diff --git a/src/coder_eval/agents/antigravity_agent.py b/src/coder_eval/agents/antigravity_agent.py index 4e1475be..7725b790 100644 --- a/src/coder_eval/agents/antigravity_agent.py +++ b/src/coder_eval/agents/antigravity_agent.py @@ -23,13 +23,13 @@ import logging import os import time -from collections.abc import AsyncIterator, Callable +from collections.abc import Callable from contextlib import AsyncExitStack from datetime import datetime from pathlib import Path from typing import Any, ClassVar -from coder_eval.agent import Agent, AgentState +from coder_eval.agent import Agent, AgentState, ConfigFieldSupport, ConfigSupport from coder_eval.agents._logging import PrefixedAdapter from coder_eval.agents.registry import AgentRegistry from coder_eval.agents.watchdog import ThreadedWatchdog @@ -71,27 +71,6 @@ logger = logging.getLogger(__name__) -# Serializes the transient ``os.environ['PATH']`` prepend around the localharness -# subprocess spawn (see ``AntigravityAgent._harness_spawn_guard``). The Antigravity -# SDK's ``subprocess.Popen`` inherits the parent process's ``os.environ`` and exposes -# NO env seam, so making mock CLIs shadow real ones forces a global mutation; this -# lock keeps concurrent host-mode starts (``run_batch`` fans them out on one event -# loop) from leaking one task's mock dirs onto another's harness. Lazily created and -# rebound per running loop so pytest's per-test loops don't reuse a stale-loop lock. -_HARNESS_SPAWN_LOCK: asyncio.Lock | None = None -_HARNESS_SPAWN_LOCK_LOOP: asyncio.AbstractEventLoop | None = None - - -def _harness_spawn_lock() -> asyncio.Lock: - """Return the process-wide harness-spawn lock, bound to the running loop.""" - global _HARNESS_SPAWN_LOCK, _HARNESS_SPAWN_LOCK_LOOP - loop = asyncio.get_running_loop() - if _HARNESS_SPAWN_LOCK is None or _HARNESS_SPAWN_LOCK_LOOP is not loop: - _HARNESS_SPAWN_LOCK = asyncio.Lock() - _HARNESS_SPAWN_LOCK_LOOP = loop - return _HARNESS_SPAWN_LOCK - - # Recommended Gemini coding model when a task pins no ``agent.model`` and neither # ``--model`` nor ``ANTIGRAVITY_MODEL`` is set. Gemini 3.5 Flash is Antigravity 2.0's # default coding model (2026-05) — it outperforms the older Gemini 3.1 Pro on coding / @@ -114,9 +93,23 @@ def _harness_spawn_lock() -> asyncio.Lock: "search_web": "WebSearch", "generate_image": "GenerateImage", "ask_question": "AskUser", + "read_url_content": "WebFetch", "finish": "Finish", } +# The inverse, for translating ``agent.allowed_tools`` / ``disallowed_tools`` (written +# in Claude names) into the harness's ``CapabilitiesConfig`` tool lists. Built from the +# forward map so the two can never drift; the forward map is 1:1, so the inversion is +# lossless. A Claude tool with no Antigravity counterpart (``Skill`` — Antigravity +# discovers skills through ``skills_paths``, not a tool; ``TodoWrite``; ...) is absent +# here and is dropped with a log line rather than crashing the enum validation. +_CLAUDE_TO_ANTIGRAVITY_TOOL_MAP: dict[str, str] = {v: k for k, v in _ANTIGRAVITY_TO_CLAUDE_TOOL_MAP.items()} + +# Tools that keep the harness's control flow working and are therefore never removed +# by an allowlist. ``finish`` is how the agent ends its turn — disabling it strands +# every run at the step-loop until the turn timeout fires. +_ANTIGRAVITY_STRUCTURAL_TOOLS: frozenset[str] = frozenset({"finish"}) + # Tool-call arg keys the harness ADDS at completion (the result payload), not # model-supplied inputs — stripped from CommandTelemetry.parameters and mined for # the tool result instead. This is the STATIC backstop; the live mapping ALSO @@ -189,6 +182,20 @@ class AntigravityAgent(Agent[AntigravityAgentConfig]): # ``should_stop`` check runs, so this agent supports early-stop-on-criterion. supports_cooperative_stop: ClassVar[bool] = True + # Declared divergence from the shared BaseAgentConfig contract. APPROXIMATED + # rather than UNHONORED for the same reason as Codex: the run is genuinely + # unconfined under every mode by design, and the isolation the field implies is + # provided one layer down by the sandbox driver — so the mode is not ignored so + # much as satisfied elsewhere. Rejecting it would break every task that sets + # bypassPermissions to mean "this is a headless eval, do not stop to ask". + config_support: ClassVar[dict[str, ConfigFieldSupport]] = { + "permission_mode": ConfigFieldSupport( + ConfigSupport.APPROXIMATED, + "every mode runs the harness with policy.allow_all(); coder_eval's isolation " + + "boundary is the sandbox driver, and a headless eval has no human to approve", + ), + } + def __init__( self, config: AntigravityAgentConfig, @@ -213,7 +220,8 @@ def __init__( self._sdk_agent: Any = None self._exit_stack: AsyncExitStack | None = None # Absolute dirs to prepend to PATH so sandbox mock CLIs shadow real ones - # for the harness's run_command tool — applied at spawn (see start()). + # for the harness's run_command tool — handed to the SDK's per-agent env + # seam at start() (see _harness_env). self._env_path_prepend: list[str] = [] # _state / _iteration / _iteration_was_incremented / pending_turn lifecycle # bookkeeping lives on the Agent base class (shared defaults + helpers). @@ -288,6 +296,91 @@ def _resolve_workspaces(self, skills_paths: list[str]) -> list[str]: """ return [str(self.working_directory), *skills_paths] + def _map_tools(self, tools: list[str], field: str) -> list[str]: + """Translate Claude-named tools to Antigravity builtin names, dropping unmappables. + + ``CapabilitiesConfig`` validates against the ``BuiltinTools`` enum, so an + unmapped name would raise instead of being ignored — hence the explicit drop + plus a log line naming what was dropped and why. + """ + mapped: list[str] = [] + dropped: list[str] = [] + for tool in tools: + antigravity_name = _CLAUDE_TO_ANTIGRAVITY_TOOL_MAP.get(tool) + if antigravity_name is None: + dropped.append(tool) + elif antigravity_name not in mapped: + mapped.append(antigravity_name) + if dropped: + self._log.debug( + "agent.%s entries with no Antigravity builtin were dropped: %s " + + "(Skill is expected here — Antigravity discovers skills via skills_paths, not a tool)", + field, + ", ".join(dropped), + ) + return mapped + + def _build_capabilities(self, types_mod: Any) -> Any: + """Build ``CapabilitiesConfig`` from ``allowed_tools`` / ``disallowed_tools``. + + The two SDK fields are mutually exclusive, so an allowlist wins and any + denylist is subtracted from it rather than passed separately — same resulting + tool set, no SDK validation error. Returns ``None`` when neither field + constrains anything, leaving the harness defaults (all tools) in force. + + The structural tools are always re-added: an allowlist that stripped ``finish`` + would leave the agent unable to end its turn. An allowlist that maps to nothing + usable falls back to the harness defaults with a warning — handing the model a + single ``finish`` tool produces a zero-scoring run with no diagnosable cause, + which is the worse failure for an eval harness. + """ + requested_allow = self.config.allowed_tools or [] + allowed = self._map_tools(requested_allow, "allowed_tools") + disallowed = self._map_tools(self.config.disallowed_tools or [], "disallowed_tools") + + # Branch on what the TASK asked for, not on what survived mapping: an + # allowlist whose every entry is unmappable must reach the warning below, + # not fall through to "no allowlist configured". + if requested_allow: + enabled = [t for t in allowed if t not in disallowed] + enabled += [t for t in sorted(_ANTIGRAVITY_STRUCTURAL_TOOLS) if t not in enabled] + if set(enabled) <= _ANTIGRAVITY_STRUCTURAL_TOOLS: + self._log.warning( + "agent.allowed_tools (%s) maps to no usable Antigravity tool; " + + "falling back to the harness default (all tools enabled).", + ", ".join(self.config.allowed_tools or []), + ) + return None + self._log.debug("Antigravity enabled_tools: %s", ", ".join(enabled)) + return types_mod.CapabilitiesConfig(enabled_tools=enabled) + + if disallowed: + disabled = [t for t in disallowed if t not in _ANTIGRAVITY_STRUCTURAL_TOOLS] + if not disabled: + return None + self._log.debug("Antigravity disabled_tools: %s", ", ".join(disabled)) + return types_mod.CapabilitiesConfig(disabled_tools=disabled) + + return None + + def _harness_env(self) -> dict[str, str] | None: + """Per-agent environment for the localharness subprocess (``LocalAgentConfig.env``). + + Returns the mock-CLI PATH prepend as a one-key overlay, or ``None`` when no + mock dirs are configured (so the SDK spawns with a plain inherited env). The + SDK merges this over ``os.environ`` at spawn (``{**os.environ, **env}``), so + naming only ``PATH`` leaves every other inherited variable untouched. The + same overlay is handed to the harness as its ``run_command`` environment, so + mock CLIs shadow the real ones inside the agent's shell too. + """ + if not self._env_path_prepend: + return None + # Match the parent process's own casing (Windows exports ``Path``) so the + # merge overrides the inherited entry instead of adding a sibling key. + path_key = next((k for k in os.environ if k.upper() == "PATH"), "PATH") + merged = os.pathsep.join([*self._env_path_prepend, os.environ.get(path_key) or ""]) + return {path_key: merged} + async def start( self, working_directory: str, @@ -306,10 +399,9 @@ async def start( env_path_prepend: Absolute directories to prepend to PATH (typically the resolved ``SandboxConfig.mock_path_dirs``) so mock CLIs shadow the real ones for the harness's ``run_command`` tool — same mock-shadowing - contract as the Claude/Codex backends. The Antigravity SDK spawns the - localharness via ``subprocess.Popen`` with no env seam, so the prepend - is applied by transiently mutating ``os.environ['PATH']`` across the - spawn (see ``_harness_spawn_guard``). + contract as the Claude/Codex backends. Delivered through the SDK's + per-agent ``env`` seam (see ``_harness_env``), so concurrent tasks get + genuinely separate environments rather than a time-sliced global one. plugin_tools_dir: A skills/plugin source root. Resolved (together with ``config.plugins``) into the harness's native ``skills_paths`` so the agent can discover and engage UiPath skills — see ``_resolve_skills_paths``. @@ -347,12 +439,35 @@ async def start( workspaces=self._resolve_workspaces(skills_paths), # Autonomous execution: approve every tool call (incl. run_command), # which the default LocalAgentConfig policy would otherwise deny. + # ``permission_mode`` is deliberately NOT mapped onto these policies — + # it does not confine this agent, exactly as on Codex. coder_eval's + # isolation boundary is the driver (a docker container or an ephemeral + # per-task tempdir), so an in-agent approval policy is redundant, and + # the modes below bypassPermissions differ only in what they'd ask a + # human about — there is no human on a headless eval path. Declared as + # such in the parity table so it is visible rather than silent. policies=[policy.allow_all()], + # coder_eval's `system_prompt` is text APPENDED to the harness's own + # default agent prompt (docs/agents/HARNESS_PARITY.md). A plain str + # here is exactly that: the SDK wraps it as a + # TemplatedSystemInstructions section on top of Antigravity's + # defaults. Do NOT switch to types.CustomSystemInstructions — that + # replaces every default instruction, including the core safety + # mandates, which a task-level one-liner cannot stand in for. system_instructions=self.config.system_prompt or None, # Skill discovery: hand the harness the search-path roots that parent # the UiPath skill dirs. Unlike Codex (which symlinks into # .agents/skills/), Antigravity takes skill search paths natively. skills_paths=skills_paths, + # Mock-CLI PATH shadowing, per agent. The SDK merges this over the + # inherited os.environ when it spawns the localharness, so two + # concurrent tasks never see each other's mock dirs. + env=self._harness_env(), + # allowed_tools / disallowed_tools → the harness's tool exposure. + # Stripping a tool from the model's context (rather than denying the + # call via a policy) matches how Claude Code and Codex read the same + # fields, and costs no tokens on rejected attempts. + **({"capabilities": capabilities} if (capabilities := self._build_capabilities(types)) else {}), ) # Attach the configured thinking level (reasoning effort) onto every # resolved model's Gemini endpoint. The SDK validates the model list in @@ -365,47 +480,14 @@ async def start( # Enter the SDK Agent context (boots the localharness subprocess + # opens the conversation). Held open across communicate() calls and - # closed in stop(). The spawn guard prepends the mock dirs onto - # os.environ['PATH'] across the whole context-entry (subprocess spawn - # + session open — the child keeps the env it was spawned with), then - # restores it. + # closed in stop(). self._exit_stack = AsyncExitStack() - async with self._harness_spawn_guard(): - self._sdk_agent = await self._exit_stack.enter_async_context(SdkAgent(cfg)) + self._sdk_agent = await self._exit_stack.enter_async_context(SdkAgent(cfg)) self._log.debug("Antigravity local harness started (model=%s)", self._effective_model()) except Exception as e: await self._teardown() raise RuntimeError(f"Failed to start Antigravity agent: {e}") from e - @contextlib.asynccontextmanager - async def _harness_spawn_guard(self) -> AsyncIterator[None]: - """Prepend ``_env_path_prepend`` onto ``os.environ['PATH']`` across a harness spawn. - - The localharness ``subprocess.Popen`` inherits ``os.environ`` at spawn time and - the SDK exposes no env seam, so mock CLIs can only shadow the real ones by - mutating the process PATH across the harness context-entry (subprocess spawn + - session open). The mutation is serialized (process-wide lock) and restored in - ``finally`` — the spawned child keeps the env it started with, so the restore - never affects the live harness. The lock is taken even when no prepend dirs - were configured: a no-prepend spawn must still wait out any in-flight mutated- - PATH window, or its harness would inherit another task's mock dirs. - """ - async with _harness_spawn_lock(): - if not self._env_path_prepend: - yield - return - path_key = next((k for k in os.environ if k.upper() == "PATH"), "PATH") - original = os.environ.get(path_key) - os.environ[path_key] = os.pathsep.join([*self._env_path_prepend, original or ""]) - self._log.debug("PATH prepend for harness spawn: %s", os.pathsep.join(self._env_path_prepend)) - try: - yield - finally: - if original is None: - os.environ.pop(path_key, None) - else: - os.environ[path_key] = original - async def communicate( self, user_input: str, @@ -422,6 +504,13 @@ async def communicate( conversation is cancelled (best-effort) and the turn finalizes cleanly as ``STOPPED_EARLY`` (``crashed=False``). + ``max_turns`` caps VISIBLE turns — tool calls, the unit + ``reports_stats.visible_turn_count`` counts — enforced in-stream on the same + step-loop boundary as the cooperative stop. Claude Code's native SDK cap + counts assistant messages instead; one ``communicate()`` here is a single SDK + turn, so a native counter would cap at 1 and mean nothing. See + docs/agents/HARNESS_PARITY.md. + Drives one logical turn: ``conversation.send(prompt)`` then iterate ``receive_steps()`` until the turn goes idle, mapping the Gemini step stream onto the standardized event protocol. @@ -454,6 +543,7 @@ async def communicate( iteration=self._iteration, model=model, turn_start_time=turn_start_time, + max_turns=max_turns, ) try: @@ -482,7 +572,15 @@ def _on_turn_timeout() -> None: state.stopped_early_hit = True self._log.debug("Cooperative stop requested; ending step loop at this boundary") break - if state.stopped_early_hit: + # The turn cap shares this boundary: the step that reached the + # cap is kept whole, the next is never pulled. Checked after + # the cooperative stop so an armed early-stop still reports as + # STOPPED_EARLY when both would fire on the same step. + if state.max_turns_reached(): + state.max_turns_hit = True + self._log.debug("max_turns (%s visible turns) reached; ending step loop", max_turns) + break + if state.stopped_early_hit or state.max_turns_hit: # Best-effort server-side cancel, mirrors kill(); a raising # cancel() lands in the guarded handler below. with contextlib.suppress(Exception): @@ -494,13 +592,14 @@ def _on_turn_timeout() -> None: except Exception as e: if state.timeout_hit: self._finalize_and_raise_timeout(state.finalize, timeout or 0, cause=e) - if state.stopped_early_hit: + if state.ended_cleanly: # The turn already stopped cleanly (e.g. the generator's # aclose() raised on the break); escalating to a crash # would trigger the orchestrator's retry with the watcher's # decision still latched → immediate stop-at-turn-0 on the - # retry (wasted spend). Fall through to the clean tail. - self._log.warning("Ignoring post-stop exception; finalizing as STOPPED_EARLY: %s", e) + # retry (wasted spend). A cap-break is the same shape: the + # retry would burn the budget again and re-hit the cap. + self._log.warning("Ignoring post-stop exception; finalizing cleanly: %s", e) else: self._finalize_and_raise_crash( state.finalize, truncate_crash_message(f"Antigravity turn failed: {e!s}"), cause=e @@ -517,10 +616,11 @@ def _on_turn_timeout() -> None: self._finalize_external_cancel(state.finalize) raise except Exception as e: - if state.stopped_early_hit and not state.timeout_hit: - # Same retry-poisoning guard as the inner handler: a cooperative - # stop already happened, so finalize cleanly instead of crashing. - self._log.warning("Ignoring post-stop exception; finalizing as STOPPED_EARLY: %s", e) + if state.ended_cleanly and not state.timeout_hit: + # Same retry-poisoning guard as the inner handler: the turn already + # ended cleanly (cooperative stop or turn cap), so finalize instead + # of crashing. + self._log.warning("Ignoring post-stop exception; finalizing cleanly: %s", e) else: self._finalize_and_raise_crash( state.finalize, truncate_crash_message(f"Antigravity turn failed: {e!s}"), cause=e @@ -528,8 +628,16 @@ def _on_turn_timeout() -> None: self._state = AgentState.WORKING self._end_turn_ok() - # Precedence matches Claude: timeout (raised above) > stopped_early > completed. - status = AgentEndStatus.STOPPED_EARLY if state.stopped_early_hit else AgentEndStatus.COMPLETED + # Precedence matches Claude: timeout (raised above) > stopped_early > + # max_turns_exhausted > completed. stopped_early outranks the cap because an + # armed criterion deciding the outcome is the more specific reason to have + # cut the run, and the step loop checks it first. + if state.stopped_early_hit: + status = AgentEndStatus.STOPPED_EARLY + elif state.max_turns_hit: + status = AgentEndStatus.MAX_TURNS_EXHAUSTED + else: + status = AgentEndStatus.COMPLETED state.finalize(status, crashed=False, crash_reason=None) return collector.build_turn_record() @@ -608,6 +716,7 @@ def __init__( iteration: int, model: str, turn_start_time: float, + max_turns: int | None = None, ) -> None: self._agent = agent self.emit = emit @@ -619,8 +728,10 @@ def __init__( self.model = model self.turn_start_time = turn_start_time + self.max_turns = max_turns self.timeout_hit = False self.stopped_early_hit = False + self.max_turns_hit = False self.finalized = False self.total_usage = TokenUsage() @@ -641,6 +752,26 @@ def __init__( # Content blocks accumulated since the last per-generation flush. self._blocks: list[ContentBlock] = [] + @property + def ended_cleanly(self) -> bool: + """True once the loop broke on purpose (cooperative stop or the turn cap). + + Both are non-crash terminations, so a stray exception raised while unwinding + the step generator afterwards must not be escalated into a retry. + """ + return self.stopped_early_hit or self.max_turns_hit + + def max_turns_reached(self) -> bool: + """True once this turn has produced ``max_turns`` visible turns. + + Delegates the count to the collector (``EventCollector.visible_turn_count``) + — the single agent-agnostic capture path, so one ``max_turns`` value means + the same thing here and on Codex. It counts RESOLVED tool calls (the end + event), which also means the call that reaches the cap keeps its result + instead of being force-closed as unresolved. + """ + return self.max_turns is not None and self.collector.visible_turn_count >= self.max_turns + def process_step(self, step: Any) -> None: """Route one streamed ``Step`` to events + transcript reconstruction.""" stype = _enum_value(step.type) @@ -841,6 +972,7 @@ def finalize(self, status: AgentEndStatus, *, crashed: bool = False, crash_reaso num_turns=self._assistant_turns, crashed=crashed, crash_reason=crash_reason, + max_turns_exhausted=status is AgentEndStatus.MAX_TURNS_EXHAUSTED, duration_seconds=time.monotonic() - self.turn_start_time, ) ) diff --git a/src/coder_eval/agents/claude_code_agent.py b/src/coder_eval/agents/claude_code_agent.py index 71cb2267..0ffe4980 100644 --- a/src/coder_eval/agents/claude_code_agent.py +++ b/src/coder_eval/agents/claude_code_agent.py @@ -27,6 +27,7 @@ # CLI). If this import breaks on an SDK upgrade, the threaded watchdog loses # its kill target and timeouts will no longer be enforced at the agent layer. from claude_agent_sdk._internal.transport.subprocess_cli import SubprocessCLITransport +from claude_agent_sdk.types import SystemPromptPreset from coder_eval.agent import Agent, AgentState from coder_eval.agents._logging import PrefixedAdapter, log_raw_sdk_event @@ -76,6 +77,27 @@ logger = logging.getLogger(__name__) +def _append_system_prompt(system_prompt: str | None) -> SystemPromptPreset: + """Map coder_eval's ``agent.system_prompt`` onto the SDK's preset+append form. + + ``system_prompt`` is defined as text APPENDED to the harness's own default agent + prompt — the one meaning all three backends can express (Codex takes + ``developer_instructions``, Antigravity appends a ``SystemInstructionSection``). + See docs/agents/HARNESS_PARITY.md. + + Passing the raw string would select the SDK's ``--system-prompt``, which REPLACES + Claude Code's prompt outright, and passing ``None`` is worse still: the SDK emits + ``--system-prompt ""``, so an unconfigured run gets NO system prompt at all while + Codex and Antigravity keep their full vendor prompts. Both cases are routed + through the preset here; omitting the ``append`` key leaves the CLI's default + prompt untouched. + """ + preset: SystemPromptPreset = {"type": "preset", "preset": "claude_code"} + if system_prompt: + preset["append"] = system_prompt + return preset + + # Type guards for SDK message types (using duck typing for robustness) def _is_assistant_message(message: Any) -> bool: """Check if message is an AssistantMessage using duck typing.""" @@ -1192,7 +1214,7 @@ def _build_claude_query( # summing per-message values undercounts by 10x+. Without this flag # StreamEvents are suppressed by the SDK. include_partial_messages=True, - system_prompt=self.config.system_prompt, + system_prompt=_append_system_prompt(self.config.system_prompt), setting_sources=self.config.setting_sources if self.config.setting_sources is not None else ["project"], resume=self._session_id, settings=json.dumps(self.config.claude_settings) diff --git a/src/coder_eval/agents/codex_agent.py b/src/coder_eval/agents/codex_agent.py index c6e4b2d3..877971f4 100644 --- a/src/coder_eval/agents/codex_agent.py +++ b/src/coder_eval/agents/codex_agent.py @@ -15,7 +15,7 @@ from typing import Any, ClassVar from urllib.parse import urlparse -from coder_eval.agent import Agent, AgentState +from coder_eval.agent import Agent, AgentState, ConfigFieldSupport, ConfigSupport from coder_eval.agents._logging import PrefixedAdapter, log_raw_sdk_event from coder_eval.agents.registry import AgentRegistry from coder_eval.agents.watchdog import ThreadedWatchdog @@ -268,6 +268,7 @@ def __init__( user_input: str, iteration: int, turn_start_time: float, + max_turns: int | None = None, ) -> None: self._agent = agent self.emit = emit @@ -279,8 +280,10 @@ def __init__( self.user_input = user_input self.iteration = iteration self.turn_start_time = turn_start_time + self.max_turns = max_turns self.timeout_hit = False self.stopped_early_hit = False + self.max_turns_hit = False self.finalized = False # Live pump scratch (set during streaming). @@ -406,6 +409,27 @@ def _flush_message(self, last: Any) -> None: self.open_start_ms = None self.open_end_ms = None + @property + def ended_cleanly(self) -> bool: + """True once the pump broke on purpose (cooperative stop or the turn cap). + + Both are non-crash terminations, so an exception raised while tearing the + stream down afterwards must not be escalated into a retry. + """ + return self.stopped_early_hit or self.max_turns_hit + + def max_turns_reached(self) -> bool: + """True once this turn has produced ``max_turns`` visible turns. + + Delegates the count to the collector (``EventCollector.visible_turn_count``) + so Codex and Antigravity cap on one shared definition rather than each + agent's own scratch list — ``self.commands`` skips items whose telemetry the + SDK does not resolve, while the collector counts every emitted tool end, + which is exactly what lands in ``TurnRecord.commands``. Codex delivers one + SDK turn per ``communicate()``, so the SDK's own turn counter would cap at 1. + """ + return self.max_turns is not None and self.collector.visible_turn_count >= self.max_turns + def dispatch(self, notification: Any) -> bool: """Route a notification to its handler. Returns True on ``turn/completed`` (a valid TurnCompletedNotification) so the pump loop breaks.""" @@ -623,6 +647,7 @@ def finalize(self, status: AgentEndStatus, *, crashed: bool = False, crash_reaso num_turns=1, crashed=crashed, crash_reason=crash_reason, + max_turns_exhausted=status is AgentEndStatus.MAX_TURNS_EXHAUSTED, duration_seconds=time.monotonic() - self.turn_start_time, ) ) @@ -639,6 +664,22 @@ class CodexAgent(Agent[CodexAgentConfig]): # ``should_stop`` check runs, so this agent supports early-stop-on-criterion. supports_cooperative_stop: ClassVar[bool] = True + # Declared divergences from the shared BaseAgentConfig contract. Both are + # APPROXIMATED, not UNHONORED: the values are forwarded to the SDK and the agent + # warns about each at start() (_log_config_enforcement), so an operator reading + # the task log sees exactly what the harness did and did not enforce. + config_support: ClassVar[dict[str, ConfigFieldSupport]] = { + "permission_mode": ConfigFieldSupport( + ConfigSupport.APPROXIMATED, + "every mode resolves to full-access; coder_eval's isolation boundary is the " + + "sandbox driver, and Codex's own OS sandbox is unusable on our container/CI paths", + ), + "disallowed_tools": ConfigFieldSupport( + ConfigSupport.APPROXIMATED, + "forwarded to the SDK as disabled_tools but not enforced by it — not a security boundary", + ), + } + def __init__( self, config: CodexAgentConfig, @@ -744,7 +785,11 @@ async def communicate( user_input: The message/prompt to send stream_callback: Optional callback for real-time event streaming timeout: Hard wall-clock deadline in seconds - max_turns: Hard cap on inner-loop turns (unused for Codex single-turn) + max_turns: Hard cap on VISIBLE turns — tool calls, the unit + ``reports_stats.visible_turn_count`` counts — enforced in-stream on + the same pump boundary as the cooperative stop. Codex delivers one + SDK turn per ``communicate()``, so a native turn counter would cap + at 1; see docs/agents/HARNESS_PARITY.md. should_stop: Cooperative early-stop callback, polled after each dispatched notification. When it returns True the pump breaks, the in-flight turn is interrupted (best-effort) and the turn @@ -791,6 +836,7 @@ async def communicate( user_input=user_input, iteration=self._iteration, turn_start_time=turn_start_time, + max_turns=max_turns, ) try: @@ -836,12 +882,13 @@ def _on_turn_timeout() -> None: except Exception as e: if state.timeout_hit: self._finalize_and_raise_timeout(state.finalize, timeout or 0, cause=e) - if state.stopped_early_hit: + if state.ended_cleanly: # The turn already stopped cleanly; escalating to a crash # would trigger the orchestrator's retry with the watcher's # decision still latched → immediate stop-at-turn-0 on the - # retry (wasted spend). Fall through to the clean tail. - self._log.warning("Ignoring post-stop exception; finalizing as STOPPED_EARLY: %s", e) + # retry (wasted spend). A cap-break is the same shape: the + # retry would burn the budget again and re-hit the cap. + self._log.warning("Ignoring post-stop exception; finalizing cleanly: %s", e) else: self._finalize_and_raise_crash( state.finalize, truncate_crash_message(f"Codex turn failed: {e!s}"), cause=e @@ -871,10 +918,11 @@ def _on_turn_timeout() -> None: # and _format_turn_result. Without this, such errors escape as a bare # exception: the orchestrator never drains pending_turn and _iteration # stays incremented, violating the pending-turn contract. - if state.stopped_early_hit and not state.timeout_hit: - # Same retry-poisoning guard as the inner handler: a cooperative - # stop already happened, so finalize cleanly instead of crashing. - self._log.warning("Ignoring post-stop exception; finalizing as STOPPED_EARLY: %s", e) + if state.ended_cleanly and not state.timeout_hit: + # Same retry-poisoning guard as the inner handler: the turn already + # ended cleanly (cooperative stop or turn cap), so finalize instead + # of crashing. + self._log.warning("Ignoring post-stop exception; finalizing cleanly: %s", e) else: self._finalize_and_raise_crash( state.finalize, truncate_crash_message(f"Codex turn failed: {e!s}"), cause=e @@ -884,8 +932,16 @@ def _on_turn_timeout() -> None: self._end_turn_ok() # The TurnRecord is the EventCollector's reduction of the emitted events. - # Precedence matches Claude: timeout (raised above) > stopped_early > completed. - status = AgentEndStatus.STOPPED_EARLY if state.stopped_early_hit else AgentEndStatus.COMPLETED + # Precedence matches Claude: timeout (raised above) > stopped_early > + # max_turns_exhausted > completed. stopped_early outranks the cap because an + # armed criterion deciding the outcome is the more specific reason to have + # cut the run, and the pump checks it first. + if state.stopped_early_hit: + status = AgentEndStatus.STOPPED_EARLY + elif state.max_turns_hit: + status = AgentEndStatus.MAX_TURNS_EXHAUSTED + else: + status = AgentEndStatus.COMPLETED state.finalize(status, crashed=False, crash_reason=None) return collector.build_turn_record() @@ -1276,6 +1332,19 @@ def _build_thread_options(self) -> dict[str, Any]: options["model"] = effective_model self._log.debug(f"Codex model pinned to {effective_model}") + # coder_eval's `system_prompt` is defined as text APPENDED to whatever the + # harness's own default agent prompt is (the one semantics all three + # backends can express — see docs/agents/HARNESS_PARITY.md). Codex's additive + # knob is `developer_instructions`, a developer-role message carried on the + # thread. Deliberately NOT `base_instructions`, which REPLACES Codex's entire + # built-in agent prompt — a task-level one-liner is not a whole agent prompt, + # and substituting one would silently gut the harness. + if self.config.system_prompt: + options["developer_instructions"] = self.config.system_prompt + self._log.debug( + "Codex developer_instructions set from agent.system_prompt (%d chars)", len(self.config.system_prompt) + ) + permission_mode = self.config.permission_mode.value approval_mode_str = _CODEX_APPROVAL_MODE @@ -1437,6 +1506,15 @@ async def _run_turn_with_streaming( self._log.debug("Cooperative stop requested; ending notification pump at this boundary") self._interrupt_active_turn() # best-effort; stops server-side spend break + # The turn cap shares this boundary: the notification that reached the + # cap is dispatched whole, the next is never pulled. Checked after the + # cooperative stop so an armed early-stop still reports as + # STOPPED_EARLY when both would fire on the same notification. + if state.max_turns_reached(): + state.max_turns_hit = True + self._log.debug("max_turns (%s visible turns) reached; ending notification pump", state.max_turns) + self._interrupt_active_turn() # best-effort; stops server-side spend + break finally: self._active_turn_handle = None # Close any orphan tool (item/started without item/completed), flush any @@ -1447,7 +1525,7 @@ async def _run_turn_with_streaming( with contextlib.suppress(Exception): await self._run_async(stream.close) - if state.turn_result is None and not state.stopped_early_hit: + if state.turn_result is None and not state.ended_cleanly: raise RuntimeError("Turn did not complete (no turn/completed notification received)") # Belt-and-suspenders: if streaming surfaced no assistant transcript, @@ -1459,9 +1537,11 @@ async def _run_turn_with_streaming( # and nest them under the spawning Agent call. The parent stream never # carries the child's commands (Limited persistence drops them), but its # rollout always persists the raw function_call/local_shell_call items. - # Skipped on a cooperative stop: children may have no rollout yet and the - # run is already decided — recovery adds nothing the armed gate uses. - if state.spawned_children and not state.stopped_early_hit: + # Skipped when the pump was cut short (cooperative stop or turn cap): + # children may have no rollout yet and the run is already decided — + # recovery adds nothing the armed gate uses, and its child tool calls + # would push the visible-turn count past the cap that just fired. + if state.spawned_children and not state.ended_cleanly: await self._recover_subagent_tool_calls( state.spawned_children, state.collab_results, diff --git a/src/coder_eval/cli/plan_command.py b/src/coder_eval/cli/plan_command.py index 264d863d..ca7f9c0e 100644 --- a/src/coder_eval/cli/plan_command.py +++ b/src/coder_eval/cli/plan_command.py @@ -59,6 +59,7 @@ def plan_command( check_api_keys() # Lazy import to avoid circular dependency at module level + from ..orchestration.config_support import AgentConfigSupportError, validate_config_support from ..orchestration.early_stop import EarlyStopConfigError, validate_early_stop from ..orchestration.experiment import DEFAULT_EXPERIMENT_PATH, load_experiment, resolve_task_for_variant @@ -136,6 +137,9 @@ def plan_command( resolved, _lineage, _ = resolve_task_for_variant(default_exp, task, exp_def, variant) # Early-stop guardrails (no-op unless a criterion carries a stop_early: block). validate_early_stop(resolved) + # Agent config-support guardrail (no-op unless the task sets a field + # the chosen harness declares it does not implement). + validate_config_support(resolved) agent_type = str(resolved.agent.type) if resolved.agent else "unknown" agent_model = resolved.agent.model if resolved.agent else None model_str = f" ({agent_model})" if agent_model else "" @@ -145,6 +149,11 @@ def plan_command( # failures, which stay soft): flip the plan exit code. console.print(f" [red]Variant '{variant.variant_id}': early-stop config error - {e}[/red]") all_valid = False + except AgentConfigSupportError as e: + # Same hard-error treatment: the task asks this harness for + # something it declares it cannot do. + console.print(f" [red]Variant '{variant.variant_id}': agent config error - {e}[/red]") + all_valid = False except Exception as e: console.print(f" [red]Variant '{variant.variant_id}': resolution failed - {e}[/red]") diff --git a/src/coder_eval/models/__init__.py b/src/coder_eval/models/__init__.py index 51504e92..8619e98f 100644 --- a/src/coder_eval/models/__init__.py +++ b/src/coder_eval/models/__init__.py @@ -170,6 +170,7 @@ # Tasks from coder_eval.models.tasks import ( DEFAULT_SIMULATION_STOP_TOKEN, + DEFAULT_SIMULATOR_MODEL, CriteriaCheckTiming, Dataset, PostRunCommand, @@ -342,6 +343,7 @@ # Tasks "TaskDefinition", "DEFAULT_SIMULATION_STOP_TOKEN", + "DEFAULT_SIMULATOR_MODEL", "CriteriaCheckTiming", "Dataset", "PostRunCommand", diff --git a/src/coder_eval/models/agent_config.py b/src/coder_eval/models/agent_config.py index b4ad98fd..74e0e415 100644 --- a/src/coder_eval/models/agent_config.py +++ b/src/coder_eval/models/agent_config.py @@ -151,17 +151,20 @@ class BaseAgentConfig(BaseModel): system_prompt: str | None = Field( default=None, description=( - "Custom system prompt. Replaces the default system prompt. " - "Supports inline text or multi-line YAML strings. " + "Extra system-prompt text, APPENDED to the harness's own default agent prompt " + "(Claude Code --append-system-prompt, Codex developer_instructions, Antigravity " + "system_instructions section) so one task file means the same thing on every " + "harness. It does not replace the harness prompt — write task guardrails here, " + "not a whole agent persona. Supports inline text or multi-line YAML strings. " "Mutually exclusive with system_prompt_file." ), ) system_prompt_file: str | None = Field( default=None, description=( - "Path to a file containing the system prompt (relative to task YAML). " - "The file contents are loaded at task resolution time and set as system_prompt. " - "Mutually exclusive with system_prompt." + "Path to a file containing the system-prompt text (relative to task YAML). " + "The file contents are loaded at task resolution time and set as system_prompt, " + "with the same append semantics. Mutually exclusive with system_prompt." ), ) diff --git a/src/coder_eval/models/results.py b/src/coder_eval/models/results.py index e0f4f80b..2f03222e 100644 --- a/src/coder_eval/models/results.py +++ b/src/coder_eval/models/results.py @@ -409,6 +409,14 @@ class SimulationTelemetry(BaseModel): simulator_output_tokens: int = Field(default=0, ge=0, description="Sum of simulator completion tokens across turns") simulator_failures: int = Field(default=0, ge=0, description="Number of simulator LLM calls that raised") total_turns: int = Field(description="Number of user↔agent exchanges completed in this dialog", ge=0) + simulator_model: str | None = Field( + default=None, + description=( + "Resolved model that played the simulated user, captured so a persisted " + "task.json is self-describing and its cost prices from a fact rather than " + "from the run's route. None on records written before the model was pinned." + ), + ) class EarlyStopReason(StrEnum): @@ -953,9 +961,10 @@ def judge_cost_usd(result: EvaluationResult) -> float | None: def simulator_cost_usd(result: EvaluationResult) -> float | None: """Price an evaluation's simulator turns. ``None`` outside simulation mode. - Priced at the ROUTE's model, not the subject's: ``UserSimulator`` pins - ``model=None`` so it resolves to ``BEDROCK_MODEL``, which differs from the - subject on any task that pins ``agent.model``. + Priced at the SIMULATOR's own model — ``SimulationConfig.model``, recorded on + the record as ``simulator_model`` — not the subject's and not the route's. The + two older fallbacks remain for records written before the model was pinned, when + the simulator inherited ``BEDROCK_MODEL`` from the route. A floor. ``UserSimulator`` records only ``uncached_input_tokens`` and drops both cache buckets, so a cached prefix is largely absent from the count. @@ -966,9 +975,11 @@ def simulator_cost_usd(result: EvaluationResult) -> float | None: if sim is None or not (sim.simulator_input_tokens or sim.simulator_output_tokens): return None route_model = (result.environment_info or {}).get("bedrock_model") - # Falls back to the subject's model on a non-Bedrock route, where the SDK picks - # its own default and nothing on the record names it. - model = route_model if isinstance(route_model, str) and route_model else result.model_used + model = sim.simulator_model or ( + # Legacy records only: the route's model, else the subject's on a non-Bedrock + # route where the SDK picked its own default and nothing named it. + route_model if isinstance(route_model, str) and route_model else result.model_used + ) if not model: return None return calculate_cost( diff --git a/src/coder_eval/models/tasks.py b/src/coder_eval/models/tasks.py index 77b9169b..47a80d3f 100644 --- a/src/coder_eval/models/tasks.py +++ b/src/coder_eval/models/tasks.py @@ -11,6 +11,7 @@ from coder_eval.models.agent_config import ResolvedAgentConfig from coder_eval.models.criteria import SuccessCriterion from coder_eval.models.enums import AgentKind +from coder_eval.models.judge_defaults import DEFAULT_JUDGE_MODEL from coder_eval.models.limits import RunLimits from coder_eval.models.merge_strategy import MergeField from coder_eval.models.sandbox import SandboxConfig @@ -29,6 +30,16 @@ class UnknownTaskFieldWarning(DeprecationWarning): """Sentinel token the user simulator emits when it considers the task complete.""" +DEFAULT_SIMULATOR_MODEL = DEFAULT_JUDGE_MODEL +"""Default model for the simulated user (``SimulationConfig.model``). + +Aliased to the judge default so both evaluator-side models move together: neither +is the subject under test, and both must stay fixed while the subject varies. +Distinct constants (rather than one shared name at the use sites) so pinning the +simulator to something else later does not drag the judge with it. +""" + + CriteriaCheckTiming = Literal["end_of_dialog", "every_turn", "both"] """When success criteria are evaluated inside a simulated dialog.""" @@ -54,9 +65,19 @@ class SimulationConfig(BaseModel): enabled: bool = Field(default=False, description="Master switch — when false, simulation is skipped entirely.") - # The simulator runs as a tools-disabled Claude Code agent sharing the - # coding agent's ApiRoute, so model/temperature/max_tokens are resolved at - # the route level and are not configured here. + # The simulator runs as a tools-disabled Claude Code agent sharing the coding + # agent's ApiRoute, so temperature/max_tokens are resolved at the route level and + # are not configured here. The MODEL is pinned below rather than inherited. + model: str = Field( + default=DEFAULT_SIMULATOR_MODEL, + description=( + "Model that plays the simulated user. Pinned to a constant by default, NOT " + "inherited from the route: leaving it unset let BEDROCK_MODEL swap the " + "simulated user underneath an A/B, so a run comparing two subject models was " + "silently also comparing two interlocutors. Mirrors llm_judge's `model` field — " + "hold it fixed to keep the dialog partner constant across variants." + ), + ) # Persona / goal. persona: str = Field( diff --git a/src/coder_eval/orchestration/config_support.py b/src/coder_eval/orchestration/config_support.py new file mode 100644 index 00000000..799cef1b --- /dev/null +++ b/src/coder_eval/orchestration/config_support.py @@ -0,0 +1,100 @@ +"""Resolution-time guard on per-agent ``BaseAgentConfig`` support declarations. + +A base-config field must mean the same thing on every harness. Where a backend +cannot implement one, it says so on its agent class (``Agent.config_support``) +instead of dropping the field at runtime, and this module turns the strictest of +those declarations — :attr:`~coder_eval.agent.ConfigSupport.UNHONORED` — into a +hard error at resolution. + +The error fires only when the resolved task actually *sets* the field to +something other than the config model's default. A default-valued field carries +no intent, so rejecting it would break every task on the harness rather than the +ones whose author expected the field to do something. + +:attr:`~coder_eval.agent.ConfigSupport.APPROXIMATED` fields deliberately do NOT +raise here: they are honored, just imperfectly, and each agent already warns +about its own divergence at ``start()`` where the concrete resolved value is in +hand. Silence at resolution, loud in the task log. + +Mirrors ``early_stop.py::validate_early_stop`` in shape and call sites: a +``ValueError`` subclass so the run path's resolve -> ``typer.BadParameter`` +conversion covers it, caught explicitly by ``plan`` to flip its exit code. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any + +from coder_eval.agent import ConfigSupport + + +if TYPE_CHECKING: + from coder_eval.models import BaseAgentConfig, TaskDefinition + + +class AgentConfigSupportError(ValueError): + """Raised when a task sets a config field the chosen agent does not implement.""" + + +def _is_default(config: BaseAgentConfig, field: str) -> bool: + """True when ``field`` still holds the config model's declared default. + + Compares against the field's default rather than checking + ``model_fields_set``, because by the time a task resolves, the five-layer + merge has explicitly set nearly every field — ``model_fields_set`` would + report the whole block as author intent. The default is what "the author did + not ask for anything here" actually looks like on a merged config. + """ + model_field = type(config).model_fields.get(field) + if model_field is None: + # The field does not exist on THIS agent's config subclass, so the task + # cannot have set it. A declaration naming a field its own config lacks is + # a typo, but not a task author's problem — let it pass silently here and + # let the lint rule catch it. + return True + default: Any = model_field.get_default(call_default_factory=True) + return getattr(config, field, default) == default + + +def validate_config_support(task: TaskDefinition) -> None: + """Reject a resolved task that sets a field its agent declares unhonored. + + Called after the config layers have merged — the same seats as + ``validate_early_stop`` (``resolve_all_tasks`` post-CLI overrides, the + ``plan`` per-variant loop, and defensively in ``Orchestrator._setup``). + No-op for an agent that declares nothing (every built-in but Codex and + Antigravity) and for a task that leaves the declared fields at their default. + + Raises: + AgentConfigSupportError: on any set-but-unhonored field. + """ + config = task.agent + if config is None or config.type is None: + return + + # Lazily import the registry + plugin loader so this module stays free of + # runtime coder_eval imports beyond the ABC itself (mirrors early_stop). + from coder_eval.agents.registry import AgentRegistry + from coder_eval.plugins import ensure_plugins_loaded + + ensure_plugins_loaded() + registration = AgentRegistry.get(str(config.type)) + if registration is None: + # Not this guard's failure to report: an unregistered type already raises a + # clear "is the providing plugin installed?" error where the agent is built. + return + + offenders = [ + (field, note) + for field, note in registration.agent_class.config_support.items() + if note.support is ConfigSupport.UNHONORED and not _is_default(config, field) + ] + if not offenders: + return + + details = "; ".join(f"agent.{field} ({note.reason})" for field, note in offenders) + raise AgentConfigSupportError( + f"agent type {str(config.type)!r} does not implement: {details}. " + + "Leaving these set would run a different task than the same file runs on another " + + "harness. Remove them, or pick an agent type that implements them." + ) diff --git a/src/coder_eval/orchestration/experiment.py b/src/coder_eval/orchestration/experiment.py index f75c585e..c02a5f91 100644 --- a/src/coder_eval/orchestration/experiment.py +++ b/src/coder_eval/orchestration/experiment.py @@ -580,6 +580,7 @@ def resolve_all_tasks( Raises: ValueError: If duplicate task IDs are found after resolution. """ + from .config_support import AgentConfigSupportError, validate_config_support from .early_stop import EarlyStopConfigError, validate_early_stop resolved: list[ResolvedTask] = [] @@ -672,6 +673,12 @@ def resolve_all_tasks( # a bad arming raises EarlyStopConfigError (a ValueError). validate_early_stop(resolved_task) + # Agent config-support guardrail: reject a task that sets a field + # the chosen harness declares it does not implement, so one task + # file cannot silently run as two different tasks. No-op unless a + # declared-unhonored field is set to a non-default value. + validate_config_support(resolved_task) + # Fan-out: simulation n_trials takes precedence over experiment repeats # when simulation is active; otherwise use experiment-level repeats. sim = resolved_task.simulation @@ -694,9 +701,10 @@ def resolve_all_tasks( config_lineage=dict(lineage), ) ) - # Early-stop arming errors are a deliberate hard stop: they always - # propagate (never demoted to skipped) so a misarmed run fails loudly. - except EarlyStopConfigError: + # Early-stop arming and agent-config-support errors are a deliberate hard + # stop: they always propagate (never demoted to skipped) so a misconfigured + # run fails loudly instead of quietly shrinking the suite. + except (EarlyStopConfigError, AgentConfigSupportError): raise # Narrow set, matching the load/expand block above: config-resolution # and IO failures are collected (decided after the loop, below); diff --git a/src/coder_eval/orchestrator.py b/src/coder_eval/orchestrator.py index 8640dd7f..4a67185c 100644 --- a/src/coder_eval/orchestrator.py +++ b/src/coder_eval/orchestrator.py @@ -56,6 +56,7 @@ resolve_evaluation_route, resolve_route, ) +from .orchestration.config_support import validate_config_support from .orchestration.early_stop import EarlyStopWatcher, early_stop_active, validate_early_stop from .orchestration.evaluation import load_reference from .path_utils import format_task_log_id, task_log_path @@ -989,6 +990,10 @@ async def _setup(self) -> None: # some criterion carries a stop_early: block. validate_early_stop(self.task) + # Same defensive posture for the agent config-support guardrail: no-op unless + # the task sets a field this harness declares it does not implement. + validate_config_support(self.task) + # Build the early-stop watcher once, up front, when armed (>= 1 criterion # with a stop_early: block and the run_limits.stop_early kill switch not # thrown). This sits BEFORE the evaluate-only early return below, so an @@ -1641,6 +1646,7 @@ def _build_simulation_telemetry( sim_in: int, sim_out: int, sim_failures: int, + sim_model: str | None = None, ) -> SimulationTelemetry: """Single construction point for SimulationTelemetry across the dialog loop's exit paths.""" return SimulationTelemetry( @@ -1651,6 +1657,9 @@ def _build_simulation_telemetry( simulator_output_tokens=sim_out, simulator_failures=sim_failures, total_turns=total_turns, + # The resolved id, not the configured one, so the record names the model + # the backend actually served and simulator cost prices from a fact. + simulator_model=sim_model, ) async def _run_dialog_criteria_check( @@ -1768,6 +1777,7 @@ async def _acquire_opener( sim_in=0, sim_out=0, sim_failures=1, + sim_model=sim_model_id, ) return _OpenerOutcome(short_circuit=True, return_value=False) @@ -1783,6 +1793,7 @@ async def _acquire_opener( sim_in=solicited.sim_in, sim_out=solicited.sim_out, sim_failures=0, + sim_model=sim_model_id, ) return _OpenerOutcome(short_circuit=True, return_value=False) @@ -1857,7 +1868,10 @@ async def _simulation_dialog_loop(self, initial_prompt: str | None, sandbox_dir: # UserMessage captured for the upcoming agent call; prepended to the # next turn_record.messages. None outside simulation paths. pending_user_turn: UserMessage | None = None - sim_model_id = getattr(sim_config, "model", None) + # The RESOLVED simulator model (backend-translated), not the configured id — + # it labels each simulator UserMessage and is persisted on the telemetry so + # simulator cost prices from the model that actually served the call. + sim_model_id = simulator.model # Track whether we entered the agent-call loop — used by the finally # block to decide whether to persist an orphaned pending_user_turn. agent_turn_attempted = False @@ -2035,6 +2049,7 @@ async def _simulation_dialog_loop(self, initial_prompt: str | None, sandbox_dir: sim_in=simulator_input_tokens, sim_out=simulator_output_tokens, sim_failures=simulator_failures, + sim_model=sim_model_id, ) logger.info( "Simulation dialog ended: stop_reason=%s turns=%s criteria_passed=%s", @@ -2070,6 +2085,7 @@ async def _simulation_dialog_loop(self, initial_prompt: str | None, sandbox_dir: sim_in=simulator_input_tokens, sim_out=simulator_output_tokens, sim_failures=simulator_failures, + sim_model=sim_model_id, ) # Always tear down the simulator agent (and its scratch dir) even # when the dialog bails out via exception. diff --git a/src/coder_eval/reports.py b/src/coder_eval/reports.py index 5ff79eab..1c416e40 100644 --- a/src/coder_eval/reports.py +++ b/src/coder_eval/reports.py @@ -87,8 +87,14 @@ def collect_agent_settings_rows(settings_source: dict[str, Any], is_sdk: bool) - betas = settings_source.get("betas") if betas: rows.append(("Betas", ", ".join(betas))) - if settings_source.get("system_prompt") is not None: - prompt_str = str(settings_source["system_prompt"]).replace("\n", " ") + # `system_prompt` reaches the SDK as the preset+append form (the appended + # text is what the task actually configured; the preset itself is constant), + # so report the append payload and render nothing when there is none. + raw_prompt = settings_source.get("system_prompt") + if isinstance(raw_prompt, dict): + raw_prompt = raw_prompt.get("append") + if raw_prompt is not None: + prompt_str = str(raw_prompt).replace("\n", " ") if len(prompt_str) > SYSTEM_PROMPT_PREVIEW_CHARS: prompt_str = prompt_str[:SYSTEM_PROMPT_PREVIEW_CHARS] + "..." rows.append(("System Prompt", prompt_str)) @@ -469,11 +475,13 @@ def _runtime_notes_lines(summary: RunSummary) -> list[str]: ) if t.get("stopped_early"): reason = t.get("early_stop_reason") or "unknown" - turns_remaining = t.get("turns_remaining_at_stop") - avoided = f" <= {turns_remaining} turn(s) avoided —" if isinstance(turns_remaining, int) else "" - notes.append( - f"> **NOTE:** [{task_id}] stopped early ({reason});{avoided} {early_stop_gate_note(reason)}" - ) + # No "N turn(s) avoided" claim here. It derived from + # ``max_turns - sdk_turn_index``, and on Codex and Antigravity one + # ``communicate()`` is a single SDK turn — so an early-stopped row + # advertised dozens of avoided turns when all that was cut was a + # tool-call tail. ``turns_remaining_at_stop`` is still persisted on + # EarlyStopInfo, labelled there as the upper bound it is. + notes.append(f"> **NOTE:** [{task_id}] stopped early ({reason}); {early_stop_gate_note(reason)}") if not notes: return [] return ["## Run-time Notes", "", *notes] diff --git a/src/coder_eval/simulation/user_simulator.py b/src/coder_eval/simulation/user_simulator.py index dca83d57..36ae5320 100644 --- a/src/coder_eval/simulation/user_simulator.py +++ b/src/coder_eval/simulation/user_simulator.py @@ -189,10 +189,13 @@ def __init__( self._agent: Agent[Any] | None = None self._scratch_dir: Path | None = None - # model is intentionally left to the route: ClaudeCodeAgent._build_sdk_env - # maps BedrockRoute.model → ANTHROPIC_MODEL env, so pinning a Gateway-style - # name here (e.g. "anthropic.claude-sonnet-4-6") would break Bedrock runs. - # For direct routes, None lets the SDK pick its default. + # The simulator's model is PINNED from config, not inherited from the route. + # Leaving it None meant BEDROCK_MODEL decided who the simulated user was, so + # an A/B that varied the subject model silently varied the interlocutor too — + # and `_simulator_cost_usd` had to price from environment_info["bedrock_model"] + # to compensate. `_resolve_model` translates the vendor-prefixed id into + # whatever the run's backend accepts, the same way the LLM judge does. + self._model = self._resolve_model(config.model, route) # # allowed_tools=[] is the primary guarantee that the simulator cannot # touch files or run commands. The disallowed_tools list below is @@ -204,7 +207,7 @@ def __init__( agent_config = parse_agent_config( type=AgentKind.CLAUDE_CODE, - model=None, + model=self._model, allowed_tools=[], disallowed_tools=_SIMULATOR_DISALLOWED_TOOLS, plugins=None, @@ -217,9 +220,39 @@ def __init__( self._agent_config = agent_config if route is not None: - logger.info("User simulator: Claude Code agent backend (route=%s)", type(route).__name__) + logger.info( + "User simulator: Claude Code agent backend (route=%s, model=%s)", type(route).__name__, self._model + ) else: - logger.info("User simulator: Claude Code agent backend (default route)") + logger.info("User simulator: Claude Code agent backend (default route, model=%s)", self._model) + + @staticmethod + def _resolve_model(model: str, route: ApiRoute | None) -> str: + """Translate the configured model id into what this run's backend accepts. + + The config holds one vendor-prefixed id (``anthropic.claude-sonnet-4-6``). + Bedrock wants a cross-region inference-profile id and the direct Anthropic + API wants the bare alias, so route through the same translators the LLM judge + uses rather than re-deriving the rules here. An untranslatable id falls back + to the configured string: a wrong-looking model name that the backend rejects + loudly beats silently reverting to a route-chosen interlocutor, which is the + exact ambiguity this pin exists to remove. + """ + from coder_eval.evaluation.judge_models import to_anthropic_alias, to_bedrock_model + from coder_eval.models import BedrockRoute + + try: + if isinstance(route, BedrockRoute): + return to_bedrock_model(model, route.region) + return to_anthropic_alias(model) + except ValueError: + logger.warning("User simulator: could not translate model %r for the route; using it verbatim", model) + return model + + @property + def model(self) -> str: + """The resolved model id the simulated user runs on.""" + return self._model @property def system_prompt(self) -> str: diff --git a/src/coder_eval/streaming/collector.py b/src/coder_eval/streaming/collector.py index c848eb3e..ebac3ee9 100644 --- a/src/coder_eval/streaming/collector.py +++ b/src/coder_eval/streaming/collector.py @@ -82,6 +82,24 @@ def on_event(self, event: StreamEvent) -> None: elif isinstance(event, AgentEndEvent): self._agent_end = event + @property + def visible_turn_count(self) -> int: + """Visible timeline entries observed so far — one per resolved tool call. + + The live, in-stream counterpart of ``reports_stats.visible_turn_count``, + which counts the very same list once the turn is a finished + ``TurnRecord`` (minus its trailing final-reply entry, which cannot exist + while the turn is still running). + + Agents whose SDK has no meaningful native turn counter — Codex and + Antigravity each deliver a single SDK turn per ``communicate()`` — enforce + ``run_limits.max_turns`` against this. Reading it from the collector rather + than from each agent's own scratch list is what makes the cap mean the same + thing on both: the collector is the single agent-agnostic capture path, and + keying on ``tool_id`` means a re-emitted end event cannot double-count. + """ + return len(self._commands) + def _ordered_commands(self) -> list[CommandTelemetry]: return sorted(self._commands.values(), key=lambda c: c.sequence_number) diff --git a/tests/test_agent_config_support.py b/tests/test_agent_config_support.py new file mode 100644 index 00000000..44a05d93 --- /dev/null +++ b/tests/test_agent_config_support.py @@ -0,0 +1,140 @@ +"""Tests for per-agent ``BaseAgentConfig`` support declarations and their guard. + +A base-config field must mean the same thing on every harness. Where a backend +cannot implement one it declares the divergence on its agent class instead of +dropping the field at runtime; ``validate_config_support`` turns the strictest +declaration (UNHONORED) into a resolution-time error. +""" + +from typing import ClassVar + +import pytest + +from coder_eval.agent import Agent, ConfigFieldSupport, ConfigSupport +from coder_eval.agents.antigravity_agent import AntigravityAgent +from coder_eval.agents.claude_code_agent import ClaudeCodeAgent +from coder_eval.agents.codex_agent import CodexAgent +from coder_eval.agents.registry import AgentRegistry +from coder_eval.models import CodexAgentConfig, FileExistsCriterion, TaskDefinition, parse_agent_config +from coder_eval.orchestration.config_support import AgentConfigSupportError, validate_config_support +from coder_eval.plugins import ensure_plugins_loaded + + +def _task(**agent_kwargs) -> TaskDefinition: + return TaskDefinition( + task_id="t", + description="d", + initial_prompt="p", + agent=parse_agent_config(**agent_kwargs), + success_criteria=[FileExistsCriterion(path="x", description="f")], + ) + + +# --- the declarations themselves --------------------------------------------------- + + +def test_claude_code_declares_no_divergence(): + """Claude Code is the reference implementation — it honors every shared field.""" + assert ClaudeCodeAgent.config_support == {} + + +def test_codex_declares_permission_mode_and_disallowed_tools_approximated(): + """Both are forwarded and warned about at start(), so neither is a hard rejection.""" + support = CodexAgent.config_support + assert support["permission_mode"].support is ConfigSupport.APPROXIMATED + assert support["disallowed_tools"].support is ConfigSupport.APPROXIMATED + assert all(note.reason for note in support.values()) + + +def test_antigravity_declares_permission_mode_approximated(): + support = AntigravityAgent.config_support + assert support["permission_mode"].support is ConfigSupport.APPROXIMATED + assert "allowed_tools" not in support # honored since the CapabilitiesConfig wiring + + +def test_every_declared_field_exists_on_that_agents_config(): + """A declaration naming a field the config lacks is dead text that can never fire.""" + ensure_plugins_loaded() + for registration in AgentRegistry.registrations(): + fields = registration.config_class.model_fields + for field in registration.agent_class.config_support: + assert field in fields, f"{registration.agent_class.__name__} declares unknown field {field!r}" + + +# --- the resolution guard ----------------------------------------------------------- + + +def test_approximated_field_does_not_raise(): + """bypassPermissions on Codex is approximated, not rejected — the nightly sets it.""" + validate_config_support(_task(type="codex", permission_mode="bypassPermissions")) + + +def test_no_declarations_is_a_noop(): + validate_config_support(_task(type="claude-code", permission_mode="bypassPermissions")) + + +def test_task_without_agent_type_is_a_noop(): + validate_config_support( + TaskDefinition( + task_id="t", + description="d", + initial_prompt="p", + success_criteria=[FileExistsCriterion(path="x", description="f")], + ) + ) + + +class _StrictAgent(Agent[CodexAgentConfig]): + """A synthetic agent that genuinely drops a field, to drive the UNHONORED path. + + No built-in declares UNHONORED today (the whole point of this PR is that the + known drops were implemented instead), so the reject path needs a stand-in to + stay covered as rot-protection for the next agent that adds one. + """ + + config_support: ClassVar[dict[str, ConfigFieldSupport]] = { + "model": ConfigFieldSupport(ConfigSupport.UNHONORED, "pinned to a fixed model by the vendor"), + } + + async def start(self, working_directory, *, env_path_prepend=None, plugin_tools_dir=None) -> None: ... + + async def communicate( # type: ignore[empty-body] + self, user_input, *, stream_callback=None, timeout=None, max_turns=None, should_stop=None + ): ... + + async def stop(self) -> None: ... + + +@pytest.fixture +def strict_codex(): + """Bind ``_StrictAgent`` to the ``codex`` kind for one test, then restore. + + Rebinding the existing kind (rather than adding a new one) lets the tests build + tasks through the normal ``type: codex`` path — ``_StrictAgent`` deliberately + reuses ``CodexAgentConfig``, so nothing about resolution changes except which + ``config_support`` map the guard reads. + """ + ensure_plugins_loaded() + saved = dict(AgentRegistry._registry) + AgentRegistry._registry["codex"] = type(saved["codex"])(agent_class=_StrictAgent, config_class=CodexAgentConfig) + yield + AgentRegistry._registry.clear() + AgentRegistry._registry.update(saved) + + +def test_unhonored_field_set_to_non_default_raises(strict_codex): + with pytest.raises(AgentConfigSupportError, match="does not implement"): + validate_config_support(_task(type="codex", model="gpt-5.5")) + + +def test_unhonored_field_left_at_default_does_not_raise(strict_codex): + """A field the five-layer merge never touched carries no author intent.""" + validate_config_support(_task(type="codex")) + + +def test_error_names_the_field_and_the_reason(strict_codex): + with pytest.raises(AgentConfigSupportError) as exc: + validate_config_support(_task(type="codex", model="gpt-5.5")) + + assert "agent.model" in str(exc.value) + assert "pinned to a fixed model by the vendor" in str(exc.value) diff --git a/tests/test_antigravity_agent.py b/tests/test_antigravity_agent.py index 99747950..8d743cbb 100644 --- a/tests/test_antigravity_agent.py +++ b/tests/test_antigravity_agent.py @@ -5,8 +5,11 @@ """ import asyncio +import logging import os -from types import SimpleNamespace +import sys +from types import ModuleType, SimpleNamespace +from typing import Any import pytest @@ -503,191 +506,429 @@ async def test_communicate_requires_started_agent(): await agent.communicate("x") -# --- env_path_prepend / harness-spawn PATH shadowing ------------------------------ +def _install_fake_sdk(monkeypatch, sdk_agent_cls) -> None: + """Stub ``google.antigravity`` in sys.modules so ``start()`` runs without the extra. + + ``LocalAgentConfig`` becomes a SimpleNamespace factory, so a test can assert on + exactly the kwargs the agent built (``env``, ``policies``, ``capabilities``, ...). + """ + ag = ModuleType("google.antigravity") + ag.Agent = sdk_agent_cls + ag.LocalAgentConfig = lambda **kwargs: SimpleNamespace(models=[], **kwargs) + ag.types = SimpleNamespace( + ThinkingLevel=lambda level: level, + GeminiAPIEndpoint=type("GeminiAPIEndpoint", (), {}), + GeminiModelOptions=lambda **kw: SimpleNamespace(**kw), + CapabilitiesConfig=lambda **kw: SimpleNamespace(**kw), + ) + hooks = ModuleType("google.antigravity.hooks") + hooks.policy = SimpleNamespace( + allow_all=lambda: SimpleNamespace(kind="allow_all"), + deny=lambda tool, **kw: SimpleNamespace(kind="deny", tool=tool), + allow=lambda tool, **kw: SimpleNamespace(kind="allow", tool=tool), + ) + google_pkg = sys.modules.get("google") or ModuleType("google") + monkeypatch.setitem(sys.modules, "google", google_pkg) + monkeypatch.setitem(sys.modules, "google.antigravity", ag) + monkeypatch.setitem(sys.modules, "google.antigravity.hooks", hooks) + + +# --- env_path_prepend / mock-CLI PATH shadowing ----------------------------------- # -# The localharness subprocess inherits os.environ at Popen time (no SDK env seam), -# so mock CLIs shadow real ones only if the mock dirs sit at the FRONT of PATH for -# the spawn. These drive the guard directly (no SDK needed) — an inverted join -# order (mocks at the back) or a missing restore must fail here. +# The mock dirs reach the localharness through the SDK's per-agent ``env`` seam +# (LocalAgentConfig.env), which the SDK merges over os.environ at Popen time. Mock +# CLIs shadow real ones only if those dirs sit at the FRONT of the merged PATH, so +# an inverted join order (mocks at the back) must fail here. The process env is +# never mutated, which is what lets two tasks start harnesses concurrently. -async def test_harness_spawn_guard_prepends_path_in_order_then_restores(monkeypatch): - """Mock dirs land at the FRONT of PATH in order during the spawn; PATH is restored on exit.""" +async def test_harness_env_prepends_path_in_order(monkeypatch): + """Mock dirs land at the FRONT of the overlay PATH, in order, ahead of the parent's.""" monkeypatch.setenv("PATH", "/parent/bin") agent = AntigravityAgent(parse_agent_config(type="antigravity")) agent._env_path_prepend = ["/sandbox/mocks", "/sandbox/bins"] - async with agent._harness_spawn_guard(): - assert os.environ["PATH"] == f"/sandbox/mocks{os.pathsep}/sandbox/bins{os.pathsep}/parent/bin" - assert os.environ["PATH"] == "/parent/bin" # restored + assert agent._harness_env() == {"PATH": f"/sandbox/mocks{os.pathsep}/sandbox/bins{os.pathsep}/parent/bin"} -async def test_harness_spawn_guard_no_prepend_leaves_path_untouched(monkeypatch): - """Default (no env_path_prepend) never mutates PATH — the guard is a no-op.""" +async def test_harness_env_none_without_prepend(monkeypatch): + """No mock dirs → no overlay at all, so the SDK spawns with a plain inherited env.""" monkeypatch.setenv("PATH", "/parent/bin") agent = AntigravityAgent(parse_agent_config(type="antigravity")) - async with agent._harness_spawn_guard(): - assert os.environ["PATH"] == "/parent/bin" + assert agent._harness_env() is None + + +async def test_harness_env_never_mutates_process_env(monkeypatch): + """Building the overlay leaves os.environ untouched — the whole point of the seam.""" + monkeypatch.setenv("PATH", "/parent/bin") + agent = AntigravityAgent(parse_agent_config(type="antigravity")) + agent._env_path_prepend = ["/sandbox/mocks"] + + agent._harness_env() + assert os.environ["PATH"] == "/parent/bin" -async def test_harness_spawn_guard_resolves_path_key_case_insensitively(monkeypatch): - """A non-uppercase PATH key (e.g. Windows 'Path') is reused, not duplicated.""" +async def test_harness_env_resolves_path_key_case_insensitively(monkeypatch): + """A non-uppercase PATH key (e.g. Windows 'Path') is reused, so the merge overrides it. + + The SDK merges as ``{**os.environ, **env}``; keying the overlay 'PATH' against an + inherited 'Path' would add a sibling entry and leave the real PATH in force. + """ from coder_eval.agents import antigravity_agent monkeypatch.setattr(antigravity_agent.os, "environ", {"Path": "/parent/bin"}) agent = AntigravityAgent(parse_agent_config(type="antigravity")) agent._env_path_prepend = ["/sandbox/mocks"] - async with agent._harness_spawn_guard(): - assert antigravity_agent.os.environ == {"Path": f"/sandbox/mocks{os.pathsep}/parent/bin"} - assert antigravity_agent.os.environ == {"Path": "/parent/bin"} + assert agent._harness_env() == {"Path": f"/sandbox/mocks{os.pathsep}/parent/bin"} -async def test_harness_spawn_guard_restores_absent_path(monkeypatch): - """When PATH was unset, the guard removes the key it added rather than leaving ''.""" +async def test_harness_env_handles_absent_path(monkeypatch): + """When PATH is unset, the overlay is just the mock dirs (no stray separator tail).""" from coder_eval.agents import antigravity_agent monkeypatch.setattr(antigravity_agent.os, "environ", {}) agent = AntigravityAgent(parse_agent_config(type="antigravity")) agent._env_path_prepend = ["/sandbox/mocks"] - async with agent._harness_spawn_guard(): - assert antigravity_agent.os.environ["PATH"] == f"/sandbox/mocks{os.pathsep}" - assert "PATH" not in antigravity_agent.os.environ + assert agent._harness_env() == {"PATH": f"/sandbox/mocks{os.pathsep}"} -async def test_harness_spawn_guard_restores_path_when_body_raises(monkeypatch): - """PATH is restored even when the guarded spawn raises (the failed-boot path). +async def test_concurrent_starts_get_isolated_mock_dirs(monkeypatch, tmp_path): + """Two agents starting concurrently each see ONLY their own mock dirs. - In start() the guard wraps the SDK context-enter, which raises on harness-boot - failure — the restore must live in ``finally`` or a failed spawn leaks the mock - dirs onto the global PATH. + The defect this replaces: with a process-wide PATH mutation, agent B's harness + could spawn inside agent A's mutated-PATH window and resolve run_command against + A's mock CLIs for B's entire session. With the per-agent env seam the two configs + are independent, so overlapping starts cannot contaminate each other. """ monkeypatch.setenv("PATH", "/parent/bin") - agent = AntigravityAgent(parse_agent_config(type="antigravity")) - agent._env_path_prepend = ["/sandbox/mocks"] + configs: list[Any] = [] + a_entered = asyncio.Event() - with pytest.raises(RuntimeError, match="harness boot failed"): - async with agent._harness_spawn_guard(): - assert os.environ["PATH"] == f"/sandbox/mocks{os.pathsep}/parent/bin" - raise RuntimeError("harness boot failed") - assert os.environ["PATH"] == "/parent/bin" + class _FakeSdkAgent: + def __init__(self, cfg): + self._first = not configs + configs.append(cfg) + async def __aenter__(self): + if self._first: + # A parks inside its spawn so B's start() fully overlaps it. + a_entered.set() + await asyncio.sleep(0.05) + return self -async def test_harness_spawn_guard_serializes_concurrent_starts(monkeypatch): - """Two overlapping guards must NOT stack PATHs — the lock serializes the spawn window. + async def __aexit__(self, *exc): + return False + + _install_fake_sdk(monkeypatch, _FakeSdkAgent) - Without the lock, agent B entering while A holds the guard would observe A's mock - dirs on PATH (cross-task mock contamination — the exact defect this fixes). - """ - monkeypatch.setenv("PATH", "/parent/bin") a = AntigravityAgent(parse_agent_config(type="antigravity")) - a._env_path_prepend = ["/a/mocks"] b = AntigravityAgent(parse_agent_config(type="antigravity")) - b._env_path_prepend = ["/b/mocks"] - - b_entered = asyncio.Event() - b_saw_path: list[str] = [] - - async def run_b() -> None: - async with b._harness_spawn_guard(): - b_saw_path.append(os.environ["PATH"]) - b_entered.set() - - async with a._harness_spawn_guard(): - # A holds the guard. Launch B; it must block on the lock and NOT mutate PATH. - task = asyncio.create_task(run_b()) - await asyncio.sleep(0.05) - assert not b_entered.is_set() - assert os.environ["PATH"] == f"/a/mocks{os.pathsep}/parent/bin" # only A's dirs - - await task - # B ran only after A released: it saw the restored parent PATH, not A's mocks. - assert b_saw_path == [f"/b/mocks{os.pathsep}/parent/bin"] - assert os.environ["PATH"] == "/parent/bin" + + task_a = asyncio.create_task(a.start(str(tmp_path), env_path_prepend=["/a/mocks"])) + await a_entered.wait() + await b.start(str(tmp_path), env_path_prepend=["/b/mocks"]) + await task_a + + envs = [c.env for c in configs] + assert envs == [ + {"PATH": f"/a/mocks{os.pathsep}/parent/bin"}, + {"PATH": f"/b/mocks{os.pathsep}/parent/bin"}, + ] + assert os.environ["PATH"] == "/parent/bin" # process env untouched throughout -async def test_harness_spawn_guard_no_prepend_waits_for_active_prepend(monkeypatch): - """A no-prepend spawn must wait out another task's mutated-PATH window. +async def test_start_passes_env_path_prepend_to_sdk_config(monkeypatch, tmp_path): + """start(env_path_prepend=[...]) reaches LocalAgentConfig.env, not the process env. - Without taking the lock on the no-prepend path, agent B would spawn its harness - while A's mock dirs are live on the global PATH — B's run_command tool would - resolve to A's mock CLIs for B's entire session. + The SDK is stubbed via sys.modules so this needs no google-antigravity install. """ monkeypatch.setenv("PATH", "/parent/bin") - a = AntigravityAgent(parse_agent_config(type="antigravity")) - a._env_path_prepend = ["/a/mocks"] - b = AntigravityAgent(parse_agent_config(type="antigravity")) # no mock dirs - - b_entered = asyncio.Event() - b_saw_path: list[str] = [] - - async def run_b() -> None: - async with b._harness_spawn_guard(): - b_saw_path.append(os.environ["PATH"]) - b_entered.set() - - async with a._harness_spawn_guard(): - # A holds the guard with its mock dirs on PATH. B must block, not spawn. - task = asyncio.create_task(run_b()) - await asyncio.sleep(0.05) - assert not b_entered.is_set() - - await task - # B ran only after A restored PATH: it saw the parent PATH, not A's mocks. - assert b_saw_path == ["/parent/bin"] - assert os.environ["PATH"] == "/parent/bin" + configs: list[Any] = [] + + class _FakeSdkAgent: + def __init__(self, cfg): + configs.append(cfg) + + async def __aenter__(self): + return self + + async def __aexit__(self, *exc): + return False + + _install_fake_sdk(monkeypatch, _FakeSdkAgent) + + agent = AntigravityAgent(parse_agent_config(type="antigravity")) + await agent.start(str(tmp_path), env_path_prepend=["/sandbox/mocks", "/sandbox/bins"]) + + assert agent._env_path_prepend == ["/sandbox/mocks", "/sandbox/bins"] + assert configs[0].env == {"PATH": f"/sandbox/mocks{os.pathsep}/sandbox/bins{os.pathsep}/parent/bin"} + assert os.environ["PATH"] == "/parent/bin" # never mutated + + +async def test_start_omits_env_when_no_mock_dirs(monkeypatch, tmp_path): + """Without mock dirs the SDK gets env=None, so the harness inherits os.environ verbatim.""" + + class _FakeSdkAgent: + def __init__(self, cfg): + configs.append(cfg) + + async def __aenter__(self): + return self + + async def __aexit__(self, *exc): + return False + + configs: list[Any] = [] + _install_fake_sdk(monkeypatch, _FakeSdkAgent) + + agent = AntigravityAgent(parse_agent_config(type="antigravity")) + await agent.start(str(tmp_path)) + + assert configs[0].env is None + + +# --- allowed_tools / disallowed_tools / permission_mode ----------------------------- +# +# These fields were ignored entirely before (policies were hardcoded to allow_all and +# no CapabilitiesConfig was built), so two harnesses reading the same task file ran +# different tasks. CapabilitiesConfig validates against the BuiltinTools enum, so the +# Claude→Antigravity name mapping has to be exact and unmappables must be dropped. + + +def _agent(**cfg) -> AntigravityAgent: + return AntigravityAgent(parse_agent_config(type="antigravity", **cfg)) + + +def _fake_types() -> SimpleNamespace: + return SimpleNamespace(CapabilitiesConfig=lambda **kw: SimpleNamespace(**kw)) + + +def test_claude_to_antigravity_tool_map_is_exact_inverse(): + """The inverse map is derived, so a forward-map edit can never leave it stale.""" + from coder_eval.agents.antigravity_agent import _CLAUDE_TO_ANTIGRAVITY_TOOL_MAP + + assert len(_CLAUDE_TO_ANTIGRAVITY_TOOL_MAP) == len(_ANTIGRAVITY_TO_CLAUDE_TOOL_MAP) + for antigravity_name, claude_name in _ANTIGRAVITY_TO_CLAUDE_TOOL_MAP.items(): + assert _CLAUDE_TO_ANTIGRAVITY_TOOL_MAP[claude_name] == antigravity_name + + +def test_allowed_tools_become_enabled_tools(): + """The repo-default allowlist maps onto the matching Antigravity builtins.""" + agent = _agent(allowed_tools=["Bash", "Read", "Write", "Edit", "Glob", "Grep"]) + + caps = agent._build_capabilities(_fake_types()) + + assert caps.enabled_tools == [ + "run_command", + "view_file", + "create_file", + "edit_file", + "find_file", + "search_directory", + "finish", + ] + + +def test_allowed_tools_always_keep_finish(): + """`finish` is how a turn ends — an allowlist must never strip it.""" + agent = _agent(allowed_tools=["Read"]) + + caps = agent._build_capabilities(_fake_types()) + assert "finish" in caps.enabled_tools -async def test_start_stores_env_path_prepend(monkeypatch, tmp_path): - """start(env_path_prepend=[...]) records the dirs on the instance for the spawn guard. - The SDK is stubbed via sys.modules so this needs no google-antigravity install: - the fake SdkAgent captures os.environ['PATH'] at context-enter (mirroring the real - Popen inheriting env), proving the prepend is live exactly at spawn time. +def test_unmappable_allowed_tools_are_dropped_not_raised(): + """`Skill` has no Antigravity builtin (skills come from skills_paths) — drop it.""" + agent = _agent(allowed_tools=["Skill", "Bash"]) + + caps = agent._build_capabilities(_fake_types()) + + assert "Skill" not in caps.enabled_tools + assert "run_command" in caps.enabled_tools + + +def test_allowlist_that_maps_to_nothing_falls_back_to_defaults(caplog): + """An allowlist of only-unmappable names must not hand the model just `finish`. + + Enabling nothing but the turn-ender scores 0 with no diagnosable cause, so the + harness default (all tools) plus a loud warning is the better failure mode. """ - import sys - from types import ModuleType, SimpleNamespace + agent = _agent(allowed_tools=["Skill", "TodoWrite"]) - monkeypatch.setenv("PATH", "/parent/bin") - captured: dict[str, str] = {} + with caplog.at_level(logging.WARNING, logger="coder_eval.agents.antigravity_agent"): + caps = agent._build_capabilities(_fake_types()) + + assert caps is None + assert "maps to no usable Antigravity tool" in caplog.text + + +def test_disallowed_tools_are_subtracted_from_an_allowlist(): + """enabled_tools and disabled_tools are mutually exclusive in the SDK, so subtract.""" + agent = _agent(allowed_tools=["Bash", "Read", "Write"], disallowed_tools=["Write"]) + + caps = agent._build_capabilities(_fake_types()) + + # One field only — passing both would fail the SDK's mutual-exclusion validator. + assert not hasattr(caps, "disabled_tools") + assert caps.enabled_tools == ["run_command", "view_file", "finish"] + + +def test_disallowed_tools_alone_become_disabled_tools(): + agent = _agent(disallowed_tools=["WebSearch"]) + + caps = agent._build_capabilities(_fake_types()) + + assert caps.disabled_tools == ["search_web"] + + +def test_disallowed_tools_cannot_disable_a_structural_tool(): + agent = _agent(disallowed_tools=["Finish"]) + + assert agent._build_capabilities(_fake_types()) is None + + +def test_no_tool_fields_leaves_harness_defaults(): + assert _agent()._build_capabilities(_fake_types()) is None + + +@pytest.mark.parametrize("mode", ["default", "acceptEdits", "plan", "bypassPermissions"]) +async def test_permission_mode_never_confines_the_harness(monkeypatch, tmp_path, mode: str): + """permission_mode is declared unhonored: every mode stays fully autonomous. + + coder_eval's write boundary is the driver (docker container / ephemeral tempdir), + not the agent — same deliberate stance as Codex. A mode that silently switched the + policy list would make an A/B across harnesses incomparable. + """ + configs: list[Any] = [] class _FakeSdkAgent: def __init__(self, cfg): - self._cfg = cfg + configs.append(cfg) async def __aenter__(self): - captured["path"] = os.environ["PATH"] # env the Popen would inherit return self async def __aexit__(self, *exc): return False - def _local_agent_config(**kwargs): - return SimpleNamespace(models=[], **kwargs) + _install_fake_sdk(monkeypatch, _FakeSdkAgent) - ag = ModuleType("google.antigravity") - ag.Agent = _FakeSdkAgent - ag.LocalAgentConfig = _local_agent_config - ag.types = SimpleNamespace( - ThinkingLevel=lambda level: level, - GeminiAPIEndpoint=type("GeminiAPIEndpoint", (), {}), - GeminiModelOptions=lambda **kw: SimpleNamespace(**kw), - ) - hooks = ModuleType("google.antigravity.hooks") - hooks.policy = SimpleNamespace(allow_all=lambda: object()) - google_pkg = sys.modules.get("google") or ModuleType("google") - monkeypatch.setitem(sys.modules, "google", google_pkg) - monkeypatch.setitem(sys.modules, "google.antigravity", ag) - monkeypatch.setitem(sys.modules, "google.antigravity.hooks", hooks) + await _agent(permission_mode=mode).start(str(tmp_path)) - agent = AntigravityAgent(parse_agent_config(type="antigravity")) - await agent.start(str(tmp_path), env_path_prepend=["/sandbox/mocks", "/sandbox/bins"]) + assert [p.kind for p in configs[0].policies] == ["allow_all"] - assert agent._env_path_prepend == ["/sandbox/mocks", "/sandbox/bins"] - # The harness spawn saw the mock dirs at the front of PATH... - assert captured["path"] == f"/sandbox/mocks{os.pathsep}/sandbox/bins{os.pathsep}/parent/bin" - # ...and PATH was restored once the spawn completed. - assert os.environ["PATH"] == "/parent/bin" + +async def test_start_passes_capabilities_to_sdk_config(monkeypatch, tmp_path): + """End-to-end: the allowlist reaches LocalAgentConfig, not just the builder.""" + configs: list[Any] = [] + + class _FakeSdkAgent: + def __init__(self, cfg): + configs.append(cfg) + + async def __aenter__(self): + return self + + async def __aexit__(self, *exc): + return False + + _install_fake_sdk(monkeypatch, _FakeSdkAgent) + + await _agent(allowed_tools=["Bash", "Read"]).start(str(tmp_path)) + + assert configs[0].capabilities.enabled_tools == ["run_command", "view_file", "finish"] + + +async def test_start_omits_capabilities_when_unconstrained(monkeypatch, tmp_path): + """No allowlist → the kwarg is absent entirely, so the SDK default stands.""" + configs: list[Any] = [] + + class _FakeSdkAgent: + def __init__(self, cfg): + configs.append(cfg) + + async def __aenter__(self): + return self + + async def __aexit__(self, *exc): + return False + + _install_fake_sdk(monkeypatch, _FakeSdkAgent) + + await _agent().start(str(tmp_path)) + + assert not hasattr(configs[0], "capabilities") + + +# --- max_turns visible-turn cap ----------------------------------------------------- +# +# max_turns was accepted and never read on this backend, so a task capping turns ran +# uncapped here while the same file capped on Claude Code. The cap counts VISIBLE +# turns (tool calls — reports_stats.visible_turn_count's unit), enforced on the same +# step-loop boundary as the cooperative stop. + + +def _tool_steps(count: int) -> list: + """`count` complete tool calls, each an ACTIVE step followed by its DONE step.""" + steps = [] + for i in range(count): + call = _tc("run_command", f"t{i}", {"command_line": f"echo {i}"}) + steps.append(_step("TOOL_CALL", "ACTIVE", target="TARGET_ENVIRONMENT", tool_calls=[call])) + done = _tc("run_command", f"t{i}", {"command_line": f"echo {i}", "exit_code": 0, "combined_output": str(i)}) + steps.append(_step("TOOL_CALL", "DONE", target="TARGET_ENVIRONMENT", tool_calls=[done])) + return steps + + +async def test_max_turns_caps_visible_turns(): + """The stream offers 5 tool calls; max_turns=2 keeps 2 and never pulls the rest.""" + agent = _agent_with_steps(_tool_steps(5)) + + record = await agent.communicate("go", max_turns=2) + + assert len(record.commands) == 2 + assert record.max_turns_exhausted is True + + +async def test_max_turns_keeps_the_deciding_step_whole(): + """The tool call that reaches the cap is completed, not cut mid-flight.""" + agent = _agent_with_steps(_tool_steps(3)) + + record = await agent.communicate("go", max_turns=1) + + assert len(record.commands) == 1 + assert record.commands[0].result_status == "success" + assert record.commands[0].result_summary == "0" + + +async def test_under_the_cap_completes_normally(): + agent = _agent_with_steps(_tool_steps(2)) + + record = await agent.communicate("go", max_turns=5) + + assert len(record.commands) == 2 + assert record.max_turns_exhausted is False + + +async def test_no_max_turns_is_uncapped(): + """None must preserve the pre-existing behavior exactly.""" + agent = _agent_with_steps(_tool_steps(4)) + + record = await agent.communicate("go") + + assert len(record.commands) == 4 + assert record.max_turns_exhausted is False + + +async def test_cooperative_stop_outranks_the_cap(): + """Both firing on the same step reports STOPPED_EARLY — the more specific reason.""" + agent = _agent_with_steps(_tool_steps(5)) + + record = await agent.communicate("go", max_turns=1, should_stop=lambda: True) + + assert record.max_turns_exhausted is False + assert len(record.commands) == 1 diff --git a/tests/test_codex_agent.py b/tests/test_codex_agent.py index 95ee117f..d60c7bab 100644 --- a/tests/test_codex_agent.py +++ b/tests/test_codex_agent.py @@ -1972,3 +1972,81 @@ def test_zsh_login_shell_restores_mock_prepend_end_to_end(self, monkeypatch, tmp assert path_value.split(":")[0] == str(tmp_path / "mocks") finally: agent._cleanup_login_shell_home() + + +class TestMaxTurnsVisibleTurnCap: + """``max_turns`` was documented as "unused for Codex single-turn" and dropped. + + Codex delivers one SDK turn per ``communicate()``, so a native turn counter would + cap at 1 and mean nothing; the cap therefore counts VISIBLE turns (completed tool + calls — the unit ``reports_stats.visible_turn_count`` sums) and is enforced on the + same pump boundary as the cooperative stop. + """ + + @staticmethod + def _cmd_notifications(count: int) -> list: + """`count` completed shell commands, then the terminal turn/completed.""" + notifications = [] + for i in range(count): + root = SimpleNamespace( + type="commandExecution", + id=f"c{i}", + command=f"echo step-{i}", + exit_code=0, + aggregated_output=f"step-{i}\n", + duration_ms=5, + ) + notifications.append(_item_notification("item/started", root)) + notifications.append(_item_notification("item/completed", root)) + notifications.append(_turn_completed()) + return notifications + + async def test_cap_stops_the_pump_at_the_limit(self): + agent = _started_agent(parse_agent_config(type=AgentKind.CODEX), self._cmd_notifications(5)) + + record = await agent.communicate("go", max_turns=2) + + assert len(record.commands) == 2 + assert record.max_turns_exhausted is True + + async def test_cap_keeps_the_deciding_call_complete(self): + """Counting COMPLETED calls means the one that reaches the cap keeps its result.""" + agent = _started_agent(parse_agent_config(type=AgentKind.CODEX), self._cmd_notifications(3)) + + record = await agent.communicate("go", max_turns=1) + + assert len(record.commands) == 1 + assert record.commands[0].result_status == "success" + + async def test_cap_interrupts_the_in_flight_turn(self): + """Best-effort server-side interrupt, so the cap actually stops spend.""" + agent = _started_agent(parse_agent_config(type=AgentKind.CODEX), self._cmd_notifications(5)) + + await agent.communicate("go", max_turns=1) + + assert agent.thread.last_handle.interrupted is True + + async def test_under_the_cap_completes_normally(self): + agent = _started_agent(parse_agent_config(type=AgentKind.CODEX), self._cmd_notifications(2)) + + record = await agent.communicate("go", max_turns=5) + + assert len(record.commands) == 2 + assert record.max_turns_exhausted is False + + async def test_no_cap_consumes_the_whole_stream(self): + """None must preserve the pre-existing behavior exactly.""" + agent = _started_agent(parse_agent_config(type=AgentKind.CODEX), self._cmd_notifications(4)) + + record = await agent.communicate("go") + + assert len(record.commands) == 4 + assert record.max_turns_exhausted is False + + async def test_cooperative_stop_outranks_the_cap(self): + """Both firing on the same notification reports STOPPED_EARLY.""" + agent = _started_agent(parse_agent_config(type=AgentKind.CODEX), self._cmd_notifications(5)) + + record = await agent.communicate("go", max_turns=1, should_stop=lambda: True) + + assert record.max_turns_exhausted is False diff --git a/tests/test_early_stop.py b/tests/test_early_stop.py index 3f02aa83..1ef55cae 100644 --- a/tests/test_early_stop.py +++ b/tests/test_early_stop.py @@ -2734,12 +2734,22 @@ def test_task_dict_reflects_decision_budget_exceeded(self) -> None: d = eval_result_to_task_dict(_stopped_result(reason=EarlyStopReason.DECISION_BUDGET_EXCEEDED)) assert d["early_stop_reason"] == "decision_budget_exceeded" - def test_runtime_note_rendered_with_turns_avoided(self) -> None: + def test_runtime_note_omits_the_turns_avoided_claim(self) -> None: + """The note states the reason and the gate, and claims no turn saving. + + It used to render ``<= N turn(s) avoided`` from ``max_turns - sdk_turn_index``. + On Codex and Antigravity one ``communicate()`` is a single SDK turn, so that + subtraction advertised the entire max_turns budget as saved when all that was + actually cut was a tool-call tail. ``turns_remaining_at_stop`` is still + persisted on ``EarlyStopInfo``, where its docstring calls it an upper bound. + """ lines = ReportGenerator._runtime_notes_lines(_run_summary([eval_result_to_task_dict(_stopped_result())])) blob = "\n".join(lines) assert "stopped early (criterion_passed)" in blob - assert "<= 14 turn(s) avoided" in blob assert "gated on armed criteria only; other criteria are advisory" in blob + assert "avoided" not in blob + # Still recorded on the row for anyone who wants the bound. + assert eval_result_to_task_dict(_stopped_result())["turns_remaining_at_stop"] == 14 def test_runtime_note_for_decision_budget_exceeded_names_the_timeout(self) -> None: # The budget-exceeded reason is an effective fail gated through the diff --git a/tests/test_run_limits_orchestrator.py b/tests/test_run_limits_orchestrator.py index b586e17e..228168f5 100644 --- a/tests/test_run_limits_orchestrator.py +++ b/tests/test_run_limits_orchestrator.py @@ -11,6 +11,7 @@ from coder_eval.errors import BudgetExceededError from coder_eval.models import ( + DEFAULT_SIMULATOR_MODEL, AgentKind, ClaudeCodeAgentConfig, CriterionResult, @@ -369,6 +370,9 @@ async def test_dialog_aborts_with_run_limit_stop_reason(self, tmp_path): # The UserSimulator must NOT be reached after the budget trip — we # configure it but it should not produce another user message. mock_simulator = MagicMock() + # UserSimulator.model is a real str property (the pinned simulator model); + # an auto-specced MagicMock here fails SimulationTelemetry validation. + mock_simulator.model = DEFAULT_SIMULATOR_MODEL mock_simulator.start = AsyncMock() mock_simulator.stop = AsyncMock() mock_simulator.next_user_message = AsyncMock() @@ -529,6 +533,9 @@ async def test_warning_fires_in_simulation_and_does_not_abort(self, tmp_path, ca # Simulator emits the stop token on the second prompt so the dialog # terminates cleanly after the warning has fired. mock_simulator = MagicMock() + # UserSimulator.model is a real str property (the pinned simulator model); + # an auto-specced MagicMock here fails SimulationTelemetry validation. + mock_simulator.model = DEFAULT_SIMULATOR_MODEL mock_simulator.start = AsyncMock() mock_simulator.stop = AsyncMock() mock_simulator.next_user_message = AsyncMock( @@ -580,6 +587,9 @@ async def test_warning_fires_when_single_simulation_turn_exceeds(self, tmp_path, orch.success_checker = mock_checker mock_simulator = MagicMock() + # UserSimulator.model is a real str property (the pinned simulator model); + # an auto-specced MagicMock here fails SimulationTelemetry validation. + mock_simulator.model = DEFAULT_SIMULATOR_MODEL mock_simulator.start = AsyncMock() mock_simulator.stop = AsyncMock() mock_simulator.next_user_message = AsyncMock( diff --git a/tests/test_system_prompt_parity.py b/tests/test_system_prompt_parity.py new file mode 100644 index 00000000..7565082d --- /dev/null +++ b/tests/test_system_prompt_parity.py @@ -0,0 +1,194 @@ +"""``agent.system_prompt`` must mean the same thing on every harness: APPEND. + +The field is extra text layered on top of whatever the harness's own default agent +prompt is. Full replacement is expressible on all three SDKs but is the wrong +semantics for a task-level guardrail — substituting a one-liner for Codex's base +instructions or Antigravity's core mandates would gut the harness rather than +constrain it. See docs/agents/HARNESS_PARITY.md. +""" + +from types import ModuleType, SimpleNamespace + +import pytest + +from coder_eval.agents.antigravity_agent import AntigravityAgent +from coder_eval.agents.claude_code_agent import _append_system_prompt +from coder_eval.agents.codex_agent import CodexAgent +from coder_eval.models import AgentKind, parse_agent_config +from coder_eval.streaming.collector import EventCollector +from coder_eval.streaming.events import ToolEndEvent, ToolEndStatus + + +PROMPT = "You are a coding agent. Do not access files in sibling runs/* directories." + + +# --- claude-code --------------------------------------------------------------------- + + +def test_claude_appends_rather_than_replacing(): + """The preset+append form selects --append-system-prompt, not --system-prompt.""" + assert _append_system_prompt(PROMPT) == {"type": "preset", "preset": "claude_code", "append": PROMPT} + + +def test_claude_unset_prompt_keeps_the_default_prompt(): + """No append key → the SDK emits no prompt flag at all, so the CLI default stands. + + Passing None straight through would make the SDK emit ``--system-prompt ""``, + leaving an unconfigured run with NO system prompt while Codex and Antigravity + keep their full vendor prompts — the divergence this parity work removes. + """ + preset = _append_system_prompt(None) + + assert preset == {"type": "preset", "preset": "claude_code"} + assert "append" not in preset + + +def test_claude_empty_prompt_is_treated_as_unset(): + assert "append" not in _append_system_prompt("") + + +@pytest.mark.parametrize( + ("prompt", "expected"), + [ + (PROMPT, {"type": "preset", "preset": "claude_code", "append": PROMPT}), + (None, {"type": "preset", "preset": "claude_code"}), + ], +) +def test_claude_options_carry_the_preset_form(prompt, expected): + """End-to-end through the real options builder, not just the helper.""" + agent = _claude_agent(system_prompt=prompt) + + options, _transport, _model = agent._build_claude_query( + user_input="go", timeout=None, max_turns=None, stderr_callback=lambda _line: None + ) + + assert options.system_prompt == expected + + +def _claude_agent(**cfg): + from pathlib import Path + + from coder_eval.agents.claude_code_agent import ClaudeCodeAgent + + agent = ClaudeCodeAgent(parse_agent_config(type=AgentKind.CLAUDE_CODE, **cfg)) + agent.working_directory = Path(".") + return agent + + +# --- codex --------------------------------------------------------------------------- + + +def test_codex_uses_developer_instructions(): + """Codex's additive knob — NOT base_instructions, which replaces its whole prompt.""" + agent = CodexAgent(parse_agent_config(type=AgentKind.CODEX, system_prompt=PROMPT)) + + options = agent._build_thread_options() + + assert options["developer_instructions"] == PROMPT + assert "base_instructions" not in options + + +def test_codex_omits_the_knob_when_unset(): + """The field was dropped entirely before; absent must still mean "SDK default".""" + agent = CodexAgent(parse_agent_config(type=AgentKind.CODEX)) + + assert "developer_instructions" not in agent._build_thread_options() + + +# --- antigravity --------------------------------------------------------------------- + + +async def test_antigravity_passes_a_plain_string(monkeypatch, tmp_path): + """A plain str becomes a TemplatedSystemInstructions SECTION on top of the defaults. + + types.CustomSystemInstructions would replace them wholesale — the SDK's own + docstring flags it as advanced usage that drops the core safety mandates. + """ + configs: list = [] + + class _FakeSdkAgent: + def __init__(self, cfg): + configs.append(cfg) + + async def __aenter__(self): + return self + + async def __aexit__(self, *exc): + return False + + ag = ModuleType("google.antigravity") + ag.Agent = _FakeSdkAgent + ag.LocalAgentConfig = lambda **kwargs: SimpleNamespace(models=[], **kwargs) + ag.types = SimpleNamespace( + ThinkingLevel=lambda level: level, + GeminiAPIEndpoint=type("GeminiAPIEndpoint", (), {}), + GeminiModelOptions=lambda **kw: SimpleNamespace(**kw), + CapabilitiesConfig=lambda **kw: SimpleNamespace(**kw), + ) + hooks = ModuleType("google.antigravity.hooks") + hooks.policy = SimpleNamespace(allow_all=lambda: SimpleNamespace(kind="allow_all")) + import sys + + google_pkg = sys.modules.get("google") or ModuleType("google") + monkeypatch.setitem(sys.modules, "google", google_pkg) + monkeypatch.setitem(sys.modules, "google.antigravity", ag) + monkeypatch.setitem(sys.modules, "google.antigravity.hooks", hooks) + + agent = AntigravityAgent(parse_agent_config(type=AgentKind.ANTIGRAVITY, system_prompt=PROMPT)) + await agent.start(str(tmp_path)) + + assert configs[0].system_instructions == PROMPT + + +# --- the shared visible-turn definition ---------------------------------------------- + + +def _tool_end(collector: EventCollector, tool_id: str) -> None: + from datetime import datetime + + from coder_eval.models import CommandTelemetry + + collector.on_event( + ToolEndEvent( + task_id="t", + turn_id="turn-1", + tool=CommandTelemetry(tool_name="Bash", tool_id=tool_id, timestamp=datetime.now(), sequence_number=0), + status=ToolEndStatus.OK, + ) + ) + + +def test_collector_visible_turn_count_counts_resolved_tool_calls(): + """The single definition Codex and Antigravity both cap against.""" + collector = EventCollector() + assert collector.visible_turn_count == 0 + + _tool_end(collector, "a") + _tool_end(collector, "b") + + assert collector.visible_turn_count == 2 + + +def test_collector_visible_turn_count_does_not_double_count_a_tool_id(): + """Keyed on tool_id, so a re-emitted end event cannot inflate the count past the cap.""" + collector = EventCollector() + + _tool_end(collector, "a") + _tool_end(collector, "a") + + assert collector.visible_turn_count == 1 + + +def test_collector_visible_turn_count_matches_the_built_record(): + """It is the live view of exactly the list ``TurnRecord.commands`` ends up holding.""" + collector = EventCollector() + for tool_id in ("a", "b", "c"): + _tool_end(collector, tool_id) + + assert collector.visible_turn_count == len(collector.build_turn_record().commands) + + +@pytest.mark.parametrize("agent_cls", [CodexAgent, AntigravityAgent]) +def test_both_capped_agents_declare_cooperative_stop(agent_cls): + """The turn cap reuses the cooperative-stop boundary, so both must support it.""" + assert agent_cls.supports_cooperative_stop is True From eb18467b0fd457e047d616cee70716da610a0665 Mon Sep 17 00:00:00 2001 From: Bai Li Date: Wed, 12 Aug 2026 16:13:54 -0700 Subject: [PATCH 02/15] chore(agents): drop the system_prompt changes from this PR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The claude-code append fix, the Codex developer_instructions mapping, and the matching docs are all covered by #92, which is further along in review and also carries the system_prompt_mode escape hatch that judge sub-agents need (their prompt is the entire scoring instrument and must not be prefixed by the claude_code coding-agent preset — SubAgentRunner builds a ClaudeCodeAgent, so an unconditional append would have shifted every llm_judge verdict). Leaves this PR to the rest of #68 plus #108: the visible-turn max_turns cap, Antigravity tool allowlists, the config_support declarations and resolution guard, the SDK env seam, the pinned simulator model, and the turns-avoided note. The system_prompt parity tests are gone; the shared visible-turn-count tests they were sharing a file with move to tests/test_visible_turn_cap.py. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 2 +- docs/agents/CLAUDE_CODE.md | 2 +- docs/agents/CODEX.md | 1 - docs/agents/HARNESS_PARITY.md | 29 --- src/coder_eval/agents/antigravity_agent.py | 7 - src/coder_eval/agents/claude_code_agent.py | 24 +-- src/coder_eval/agents/codex_agent.py | 13 -- src/coder_eval/models/agent_config.py | 13 +- src/coder_eval/reports.py | 10 +- tests/test_system_prompt_parity.py | 194 --------------------- tests/test_visible_turn_cap.py | 68 ++++++++ 11 files changed, 78 insertions(+), 285 deletions(-) delete mode 100644 tests/test_system_prompt_parity.py create mode 100644 tests/test_visible_turn_cap.py diff --git a/CLAUDE.md b/CLAUDE.md index 23c9347c..f7737a7f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -141,7 +141,7 @@ action.yml # Published composite GitHub Action (coder-ev - **Per-criterion aggregation**: Each `BaseCriterion` subclass exposes `aggregate(criterion, per_row_results) -> CriterionAggregate | None`. Default emits `count / mean / median / std / min / max` so every criterion is suite-thresholdable for free. Classification-style criteria return `ClassificationCriterionResult` (subclass of `CriterionResult`) and layer accuracy / P/R/F1 / confusion via the shared `overlay_classification_metrics` utility. `BaseSuccessCriterion.suite_thresholds` gates the suite on those metrics; CLI exits non-zero on any gate failure. - **Sub-agent token accounting**: There is NO separate per-sub-agent field. Every sub-agent generation is captured as a `parent_tool_use_id`-tagged `AssistantMessage` in the turn transcript, so per-sub-agent usage is derived by grouping those messages on that id (the evalboard's `aggregateSubAgentUsage` does exactly this). Claude bubbles its sub-agent's intermediate generations into the parent stream natively, and the **terminal** generation (delivered as the Agent tool result, never streamed) is synthesized into one via `_synthesize_subagent_terminal_message` from `tool_use_result.usage`. Codex reconstructs all child generations from the child rollout (`_recover_subagent_tool_calls`). The turn total already includes sub-agent cost — Claude via the SDK's cumulative `model_usage`; Codex via `_fold_subagent_tokens`, which folds the child messages (their real per-generation tokens) into the parent total. `CommandTelemetry.result_summary` is stored **untruncated** (no 200-char cap) so sub-agent returns are preserved whole. Set `CODER_EVAL_RAW_SDK_LOG=1` to dump every raw SDK event to the task log for inspection. - **Reconciliation message (stream self-reconciles to the turn total)**: The per-message stream consistently under-reports the authoritative turn total — a fixed prompt slice (~512 input tokens on Claude) is billed on no SDK-emitted message, and sub-agent input/cache only partially bubbles up. So `EventCollector.build_turn_record` appends one synthetic `ReconciliationMessage` (`role="reconciliation"`, in the `TranscriptMessage` union) per turn, carrying the per-bucket residual = `token_usage` − Σ(assistant message buckets). The invariant: **summing the four token buckets across `TurnRecord.messages` (assistant + reconciliation) equals `token_usage` exactly**, for both Claude and Codex (Codex's stream is already complete after `_recover_subagent_tool_calls`, so its residual is usually 0 and no entry is emitted). This is what lets the evalboard SUM the message stream as the source of truth instead of reading a separate aggregate ("agent tokens"): `selectTokenTotals` returns the stream sum whenever a reconciliation entry is present, and the timeline renders it as its own row. It is agent-agnostic (booked at the single `EventCollector` seam), carries no cost (cost stays on `token_usage`), and is excluded from generation/turn counts and the cost simulator. The LiteLLM open-weight actual-cost join (`litellm_cost.apply_actual_cost`) deliberately writes cost at the TURN level only (`token_usage.total_cost_usd` = the real OpenRouter bill) plus the per-call `TurnRecord.provider_call_costs` audit record; it does NOT touch the message token buckets, so `EventCollector` stays the single writer and this invariant holds on every backend. The Python `token_usage`/`total_token_usage` aggregate is unchanged and still authoritative for budget/judges/reports. -- **Harness config parity (`Agent.config_support`)**: a shared `BaseAgentConfig` field must mean the same thing on every backend. Where one cannot implement a field it DECLARES the divergence on its agent class as `{field: ConfigFieldSupport(state, reason)}` — `APPROXIMATED` (acted on with a documented divergence; the agent warns at `start()`) or `UNHONORED` (read by nothing; `orchestration/config_support.py::validate_config_support` hard-errors at resolution when the task sets it to a non-default value, in the style of `validate_early_stop`, and it is wired at the same three seats). An empty map asserts full support, so a silently dropped field is a bug rather than a shortcut. Today's divergences: `permission_mode` on Codex and Antigravity (both run unconfined — the sandbox driver is the isolation boundary) and `disallowed_tools` on Codex (forwarded, not SDK-enforced). Two field semantics are pinned by this contract: **`system_prompt` APPENDS** to each harness's own default agent prompt (`--append-system-prompt` / `developer_instructions` / a `SystemInstructionSection`) — never replaces it, since a task-level guardrail is not a whole agent prompt — and **`run_limits.max_turns` on Codex/Antigravity counts VISIBLE turns** (resolved tool calls, read live off the shared `EventCollector.visible_turn_count`, the same list `TurnRecord.commands` holds) because one `communicate()` is a single SDK turn on both, so a native counter would clamp at 1; claude-code keeps its native SDK cap. The cap is enforced on the same loop boundary as the cooperative early stop and finalizes cleanly as `max_turns_exhausted` (no crash, no retry). Full table + rationale: docs/agents/HARNESS_PARITY.md. +- **Harness config parity (`Agent.config_support`)**: a shared `BaseAgentConfig` field must mean the same thing on every backend. Where one cannot implement a field it DECLARES the divergence on its agent class as `{field: ConfigFieldSupport(state, reason)}` — `APPROXIMATED` (acted on with a documented divergence; the agent warns at `start()`) or `UNHONORED` (read by nothing; `orchestration/config_support.py::validate_config_support` hard-errors at resolution when the task sets it to a non-default value, in the style of `validate_early_stop`, and it is wired at the same three seats). An empty map asserts full support, so a silently dropped field is a bug rather than a shortcut. Today's divergences: `permission_mode` on Codex and Antigravity (both run unconfined — the sandbox driver is the isolation boundary) and `disallowed_tools` on Codex (forwarded, not SDK-enforced). One field semantic is pinned by this contract: **`run_limits.max_turns` on Codex/Antigravity counts VISIBLE turns** (resolved tool calls, read live off the shared `EventCollector.visible_turn_count`, the same list `TurnRecord.commands` holds) because one `communicate()` is a single SDK turn on both, so a native counter would clamp at 1; claude-code keeps its native SDK cap. The cap is enforced on the same loop boundary as the cooperative early stop and finalizes cleanly as `max_turns_exhausted` (no crash, no retry). Full table + rationale: docs/agents/HARNESS_PARITY.md. - **sandbox isolation**: Tasks that don't need MCP servers should set `setting_sources: []` in their `agent:` block to isolate the sandbox from the host project's CLAUDE.md and settings. Without this, the host project's CLAUDE.md (often 20 KB+) is injected into every API call, inflating cache-creation tokens and cost significantly. - **Run-time caps (non-criterion enforcement)**: `TaskDefinition.run_limits` (`RunLimits` model) is the single namespace for all *task-level* run-time caps — `max_turns` / `task_timeout` / `turn_timeout` (structural) and `max_input_tokens` / `max_output_tokens` / `max_total_tokens` / `max_usd` (cumulative budget). Token/USD breaches abort with `FinalStatus.TOKEN_BUDGET_EXCEEDED` or `COST_BUDGET_EXCEEDED` (both `category == "failed"`). Structural caps are set from the CLI via `-D run_limits.max_turns=…` / `-D run_limits.task_timeout=…` / `-D run_limits.turn_timeout=…` (field-merged into `run_limits`); budget caps via `-D run_limits.max_usd=…` etc. or YAML. Layered config uses field-merge — a variant block overrides individual keys without replacing the task's block. The one *per-criterion* cap, `stop_early.decide_within`, deliberately lives on `LiveSuccessCriterion` instead (see below) — the watcher must attribute a decision-step timeout to a specific criterion, which `RunLimits` (task-scoped, criterion-agnostic) cannot express. - **Early stop on criterion (opt-in, per-criterion arming)**: a `stop_early:` block (`StopEarlyPolicy`) on a criterion ends a single-shot run early once the run's **armed** criteria decide the outcome, so a raised `max_turns` isn't wasted on the smoke flavor. The block's PRESENCE is the arming and alone activates the watcher — there is **no run-level master switch**: `run_limits.stop_early: false` is the run-level KILL SWITCH that force-disarms every block (the one-line experiment-variant/`-D` override for an authoritative full run), and `run_limits.stop_early: true` (the removed master arm) is a hard `EarlyStopConfigError` at resolution. The block exists on `LiveSuccessCriterion` only (currently `skill_triggered`, `command_executed` — so arming an unobservable criterion is unrepresentable, a pydantic extra-forbid error). Arming carries one implicit trigger (a native live-fail may fail-stop the run); its keys refine it: `on_pass: stop` (pass-stop the moment the criterion live-passes; default `continue` just latches) and `decide_within: N` (still undecided after N tool-call steps latches an **effective fail**, fed through the same fail-stop rule, reported as `decision_budget_exceeded` — an ordinary weighted fail, NOT a gate-bypassing force-fail; cumulative across retry attempts of the same turn). A trigger whose polarity the instance can't decide (per the abstract, checker-independent `live_decidable_polarities()`, a pure function of the criterion's own fields, paired with the checker's `live_verdict` override by lint rule CE025, a registry-based whole-tree check) is **inert by design** — one dataset-fanned YAML line serves both positive rows (pass/timeout live) and distractor rows (fail live). Verdicts **latch**: once a criterion decides, its `live_verdict` is never polled again. Stop rule is weighted, not strict-boolean: `run_limits.stop_early_gate_threshold` (default `1.0`, reproducing strict-AND behavior exactly) is the minimum weighted score (`Σ weight·score / Σ weight` over the armed subset) required to pass; a fail-stop fires once the armed set's **ceiling** (best case for everything still undecided) can no longer reach the threshold — so a low-weight fail or timeout that can't doom the gate is absorbed and the run continues — and is **deferred while any pass-capable armed criterion is undecided** (a distractor misfire never truncates a positive row's recall signal); a pass-stop fires once the `on_pass: stop` subset's **floor** (worst case) already meets the threshold, and is symmetrically **deferred while any pass-capable armed criterion outside the `on_pass: stop` subset is undecided** (so an early pass never freezes a sibling `on_pass: continue` criterion's signal out of the trajectory). A fail-stop is therefore verdict-preserving; a pass-stop can miss a *later* distractor misfire, so authoritative P/R/F1 comes from a kill-switched (`stop_early: false`) run. Driven by `orchestration/early_stop.py::EarlyStopWatcher` (built when `early_stop_active(task)`: ≥1 armed criterion, kill switch not thrown) through the agent's cooperative `should_stop` seam (tool-call granularity, no SIGKILL); live verdicts only *trigger* the stop — the standard `check_all_async` on the frozen trajectory is authoritative. Gating is **FIRED-ONLY**: a run the watcher actually cut gates on the **armed subset** via the weighted `EvaluationResult.armed_criteria_passed`; a run that completes naturally — armed or not — gates strict-AND via `all_criteria_passed`, so adding a block never changes the verdict of a run it didn't cut. Note the gate keys on the watcher having FIRED (`result.early_stop is not None`), not on confirmed truncation — an agent that ignores `should_stop`, or a stop firing on the final message, still gates armed-only. Every resolution-time guardrail violation is a hard error at resolution (plan *and* run); the one load-time case — a `stop_early:` block on a non-live criterion — is a pydantic schema error at task load, which the run surface reports as a skipped task like any other malformed task. A runtime verdict bug **fails open** to a full run. Surfaces: `EarlyStopInfo` (incl. `gate_threshold` at stop time), report notes/badges, `stopped_early` run.json rows, `EarlyStopped`/`EarlyStopReason` telemetry dims. Worked rationale: docs/TASK_DEFINITION_GUIDE.md § `stop_early`. No blocks anywhere ⇒ behavior byte-for-byte unchanged. diff --git a/docs/agents/CLAUDE_CODE.md b/docs/agents/CLAUDE_CODE.md index cd540b49..66670709 100644 --- a/docs/agents/CLAUDE_CODE.md +++ b/docs/agents/CLAUDE_CODE.md @@ -99,7 +99,7 @@ agent: | `allowed_tools` | `list[str] \| null` | Tool allowlist. Unset ⇒ all tools allowed. | | `disallowed_tools` | `list[str] \| null` | Tool denylist. (`ToolSearch` is always appended for Bedrock parity.) | | `plugins` | `list[{type: local, path}]` | Local plugin/skill directories; `$VAR` in `path` is expanded and resolved to an absolute path. | -| `system_prompt` | `str \| null` | Text **appended** to Claude Code's default system prompt (via `--append-system-prompt`), so the same task file means the same thing on every harness — see [Harness Config Parity](HARNESS_PARITY.md). Mutually exclusive with `system_prompt_file`. | +| `system_prompt` | `str \| null` | **Replaces** the default system prompt (there is no *append* seam). Mutually exclusive with `system_prompt_file`. | | `system_prompt_file` | `str \| null` | Path (relative to the task YAML) loaded into `system_prompt` at resolution. | | `setting_sources` | `list["user"\|"project"\|"local"] \| null` | Which host setting sources the SDK reads. Default resolves to `["project"]`. See [Sandbox isolation](#sandbox-isolation). | | `claude_settings` | `str \| dict \| null` | Passed to the SDK `--settings`. A dict is JSON-serialized; a str is a settings file path. Use `permissions.deny` to block tools/paths. | diff --git a/docs/agents/CODEX.md b/docs/agents/CODEX.md index b7f4f7c5..d017991b 100644 --- a/docs/agents/CODEX.md +++ b/docs/agents/CODEX.md @@ -216,7 +216,6 @@ The Codex SDK is synchronous. The agent uses `_run_async()` helper to detect and | **Session Resume** | `--resume {session_id}` | Via thread ID | | **Permissions** | `permission_mode` honored | `permission_mode` runs full-access on every mode — the sandbox driver is the boundary | | **Tool Enforcement** | Not enforced by Coder Eval wrapper | `enabled_tools` honored; `disabled_tools` NOT enforced by the SDK | -| **System prompt** | `--append-system-prompt` | `developer_instructions` on `thread_start` (also additive) | | **`max_turns`** | Native SDK turn cap (assistant messages) | Visible-turn cap (tool calls), enforced on the notification pump | | **Early stop** | Supported (cooperative `should_stop`, polled between messages) | Supported — polled after each streamed notification; the in-flight turn is interrupted best-effort | diff --git a/docs/agents/HARNESS_PARITY.md b/docs/agents/HARNESS_PARITY.md index f3460183..a2624564 100644 --- a/docs/agents/HARNESS_PARITY.md +++ b/docs/agents/HARNESS_PARITY.md @@ -26,7 +26,6 @@ a bug, not a shortcut. | Field | claude-code | codex | antigravity | |---|---|---|---| | `model` | honored | honored | honored | -| `system_prompt` / `system_prompt_file` | honored (`--append-system-prompt`) | honored (`developer_instructions`) | honored (`system_instructions` section) | | `allowed_tools` | honored | honored (`enabled_tools`) | honored (`CapabilitiesConfig.enabled_tools`) | | `disallowed_tools` | honored | **approximated** — forwarded as `disabled_tools`, not enforced by the SDK | honored (subtracted from the allowlist) | | `permission_mode` | honored | **approximated** — every mode runs full-access | **approximated** — every mode runs `policy.allow_all()` | @@ -34,34 +33,6 @@ a bug, not a shortcut. | `run_limits.max_turns` | honored (native SDK turn cap) | honored (visible-turn cap) | honored (visible-turn cap) | | `run_limits.stop_early` | honored | honored | honored | -## `system_prompt` means *append* - -`agent.system_prompt` is extra text **appended to the harness's own default agent -prompt**. It does not replace it. - -Append is the only semantics all three can express safely. Full replacement is -expressible too, but a task-level guardrail (`"Do not access files in sibling -runs/* directories"`) is not a whole agent prompt — substituting one for Codex's -base instructions or Antigravity's core mandates would gut the harness rather than -constrain it. So the field is defined as the safe one, and each backend maps it to -its own additive knob: - -| Harness | Additive knob | Replacement knob (deliberately unused) | -|---|---|---| -| claude-code | `--append-system-prompt` | `--system-prompt` | -| codex | `developer_instructions` | `base_instructions` | -| antigravity | `system_instructions` (str → `TemplatedSystemInstructions`) | `CustomSystemInstructions` | - -Write task guardrails here, not a persona. - -> **Note on the claude-code change.** This field previously mapped to -> `--system-prompt`, which *replaced* Claude Code's prompt — and because the SDK -> emits `--system-prompt ""` for `None`, a run that set nothing got **no** system -> prompt at all, while Codex and Antigravity kept their full vendor prompts. Both -> cases now route through the preset, so every harness starts from its own default -> prompt and adds the task's text on top. Expect claude-code numbers to move -> against a pre-change baseline. - ## `max_turns` counts visible turns on Codex and Antigravity A "visible turn" is one entry in the run's timeline: one resolved tool call. It is diff --git a/src/coder_eval/agents/antigravity_agent.py b/src/coder_eval/agents/antigravity_agent.py index 7725b790..08d4b0c6 100644 --- a/src/coder_eval/agents/antigravity_agent.py +++ b/src/coder_eval/agents/antigravity_agent.py @@ -447,13 +447,6 @@ async def start( # human about — there is no human on a headless eval path. Declared as # such in the parity table so it is visible rather than silent. policies=[policy.allow_all()], - # coder_eval's `system_prompt` is text APPENDED to the harness's own - # default agent prompt (docs/agents/HARNESS_PARITY.md). A plain str - # here is exactly that: the SDK wraps it as a - # TemplatedSystemInstructions section on top of Antigravity's - # defaults. Do NOT switch to types.CustomSystemInstructions — that - # replaces every default instruction, including the core safety - # mandates, which a task-level one-liner cannot stand in for. system_instructions=self.config.system_prompt or None, # Skill discovery: hand the harness the search-path roots that parent # the UiPath skill dirs. Unlike Codex (which symlinks into diff --git a/src/coder_eval/agents/claude_code_agent.py b/src/coder_eval/agents/claude_code_agent.py index 0ffe4980..71cb2267 100644 --- a/src/coder_eval/agents/claude_code_agent.py +++ b/src/coder_eval/agents/claude_code_agent.py @@ -27,7 +27,6 @@ # CLI). If this import breaks on an SDK upgrade, the threaded watchdog loses # its kill target and timeouts will no longer be enforced at the agent layer. from claude_agent_sdk._internal.transport.subprocess_cli import SubprocessCLITransport -from claude_agent_sdk.types import SystemPromptPreset from coder_eval.agent import Agent, AgentState from coder_eval.agents._logging import PrefixedAdapter, log_raw_sdk_event @@ -77,27 +76,6 @@ logger = logging.getLogger(__name__) -def _append_system_prompt(system_prompt: str | None) -> SystemPromptPreset: - """Map coder_eval's ``agent.system_prompt`` onto the SDK's preset+append form. - - ``system_prompt`` is defined as text APPENDED to the harness's own default agent - prompt — the one meaning all three backends can express (Codex takes - ``developer_instructions``, Antigravity appends a ``SystemInstructionSection``). - See docs/agents/HARNESS_PARITY.md. - - Passing the raw string would select the SDK's ``--system-prompt``, which REPLACES - Claude Code's prompt outright, and passing ``None`` is worse still: the SDK emits - ``--system-prompt ""``, so an unconfigured run gets NO system prompt at all while - Codex and Antigravity keep their full vendor prompts. Both cases are routed - through the preset here; omitting the ``append`` key leaves the CLI's default - prompt untouched. - """ - preset: SystemPromptPreset = {"type": "preset", "preset": "claude_code"} - if system_prompt: - preset["append"] = system_prompt - return preset - - # Type guards for SDK message types (using duck typing for robustness) def _is_assistant_message(message: Any) -> bool: """Check if message is an AssistantMessage using duck typing.""" @@ -1214,7 +1192,7 @@ def _build_claude_query( # summing per-message values undercounts by 10x+. Without this flag # StreamEvents are suppressed by the SDK. include_partial_messages=True, - system_prompt=_append_system_prompt(self.config.system_prompt), + system_prompt=self.config.system_prompt, setting_sources=self.config.setting_sources if self.config.setting_sources is not None else ["project"], resume=self._session_id, settings=json.dumps(self.config.claude_settings) diff --git a/src/coder_eval/agents/codex_agent.py b/src/coder_eval/agents/codex_agent.py index 877971f4..26e773fb 100644 --- a/src/coder_eval/agents/codex_agent.py +++ b/src/coder_eval/agents/codex_agent.py @@ -1332,19 +1332,6 @@ def _build_thread_options(self) -> dict[str, Any]: options["model"] = effective_model self._log.debug(f"Codex model pinned to {effective_model}") - # coder_eval's `system_prompt` is defined as text APPENDED to whatever the - # harness's own default agent prompt is (the one semantics all three - # backends can express — see docs/agents/HARNESS_PARITY.md). Codex's additive - # knob is `developer_instructions`, a developer-role message carried on the - # thread. Deliberately NOT `base_instructions`, which REPLACES Codex's entire - # built-in agent prompt — a task-level one-liner is not a whole agent prompt, - # and substituting one would silently gut the harness. - if self.config.system_prompt: - options["developer_instructions"] = self.config.system_prompt - self._log.debug( - "Codex developer_instructions set from agent.system_prompt (%d chars)", len(self.config.system_prompt) - ) - permission_mode = self.config.permission_mode.value approval_mode_str = _CODEX_APPROVAL_MODE diff --git a/src/coder_eval/models/agent_config.py b/src/coder_eval/models/agent_config.py index 74e0e415..b4ad98fd 100644 --- a/src/coder_eval/models/agent_config.py +++ b/src/coder_eval/models/agent_config.py @@ -151,20 +151,17 @@ class BaseAgentConfig(BaseModel): system_prompt: str | None = Field( default=None, description=( - "Extra system-prompt text, APPENDED to the harness's own default agent prompt " - "(Claude Code --append-system-prompt, Codex developer_instructions, Antigravity " - "system_instructions section) so one task file means the same thing on every " - "harness. It does not replace the harness prompt — write task guardrails here, " - "not a whole agent persona. Supports inline text or multi-line YAML strings. " + "Custom system prompt. Replaces the default system prompt. " + "Supports inline text or multi-line YAML strings. " "Mutually exclusive with system_prompt_file." ), ) system_prompt_file: str | None = Field( default=None, description=( - "Path to a file containing the system-prompt text (relative to task YAML). " - "The file contents are loaded at task resolution time and set as system_prompt, " - "with the same append semantics. Mutually exclusive with system_prompt." + "Path to a file containing the system prompt (relative to task YAML). " + "The file contents are loaded at task resolution time and set as system_prompt. " + "Mutually exclusive with system_prompt." ), ) diff --git a/src/coder_eval/reports.py b/src/coder_eval/reports.py index 1c416e40..a11b4440 100644 --- a/src/coder_eval/reports.py +++ b/src/coder_eval/reports.py @@ -87,14 +87,8 @@ def collect_agent_settings_rows(settings_source: dict[str, Any], is_sdk: bool) - betas = settings_source.get("betas") if betas: rows.append(("Betas", ", ".join(betas))) - # `system_prompt` reaches the SDK as the preset+append form (the appended - # text is what the task actually configured; the preset itself is constant), - # so report the append payload and render nothing when there is none. - raw_prompt = settings_source.get("system_prompt") - if isinstance(raw_prompt, dict): - raw_prompt = raw_prompt.get("append") - if raw_prompt is not None: - prompt_str = str(raw_prompt).replace("\n", " ") + if settings_source.get("system_prompt") is not None: + prompt_str = str(settings_source["system_prompt"]).replace("\n", " ") if len(prompt_str) > SYSTEM_PROMPT_PREVIEW_CHARS: prompt_str = prompt_str[:SYSTEM_PROMPT_PREVIEW_CHARS] + "..." rows.append(("System Prompt", prompt_str)) diff --git a/tests/test_system_prompt_parity.py b/tests/test_system_prompt_parity.py deleted file mode 100644 index 7565082d..00000000 --- a/tests/test_system_prompt_parity.py +++ /dev/null @@ -1,194 +0,0 @@ -"""``agent.system_prompt`` must mean the same thing on every harness: APPEND. - -The field is extra text layered on top of whatever the harness's own default agent -prompt is. Full replacement is expressible on all three SDKs but is the wrong -semantics for a task-level guardrail — substituting a one-liner for Codex's base -instructions or Antigravity's core mandates would gut the harness rather than -constrain it. See docs/agents/HARNESS_PARITY.md. -""" - -from types import ModuleType, SimpleNamespace - -import pytest - -from coder_eval.agents.antigravity_agent import AntigravityAgent -from coder_eval.agents.claude_code_agent import _append_system_prompt -from coder_eval.agents.codex_agent import CodexAgent -from coder_eval.models import AgentKind, parse_agent_config -from coder_eval.streaming.collector import EventCollector -from coder_eval.streaming.events import ToolEndEvent, ToolEndStatus - - -PROMPT = "You are a coding agent. Do not access files in sibling runs/* directories." - - -# --- claude-code --------------------------------------------------------------------- - - -def test_claude_appends_rather_than_replacing(): - """The preset+append form selects --append-system-prompt, not --system-prompt.""" - assert _append_system_prompt(PROMPT) == {"type": "preset", "preset": "claude_code", "append": PROMPT} - - -def test_claude_unset_prompt_keeps_the_default_prompt(): - """No append key → the SDK emits no prompt flag at all, so the CLI default stands. - - Passing None straight through would make the SDK emit ``--system-prompt ""``, - leaving an unconfigured run with NO system prompt while Codex and Antigravity - keep their full vendor prompts — the divergence this parity work removes. - """ - preset = _append_system_prompt(None) - - assert preset == {"type": "preset", "preset": "claude_code"} - assert "append" not in preset - - -def test_claude_empty_prompt_is_treated_as_unset(): - assert "append" not in _append_system_prompt("") - - -@pytest.mark.parametrize( - ("prompt", "expected"), - [ - (PROMPT, {"type": "preset", "preset": "claude_code", "append": PROMPT}), - (None, {"type": "preset", "preset": "claude_code"}), - ], -) -def test_claude_options_carry_the_preset_form(prompt, expected): - """End-to-end through the real options builder, not just the helper.""" - agent = _claude_agent(system_prompt=prompt) - - options, _transport, _model = agent._build_claude_query( - user_input="go", timeout=None, max_turns=None, stderr_callback=lambda _line: None - ) - - assert options.system_prompt == expected - - -def _claude_agent(**cfg): - from pathlib import Path - - from coder_eval.agents.claude_code_agent import ClaudeCodeAgent - - agent = ClaudeCodeAgent(parse_agent_config(type=AgentKind.CLAUDE_CODE, **cfg)) - agent.working_directory = Path(".") - return agent - - -# --- codex --------------------------------------------------------------------------- - - -def test_codex_uses_developer_instructions(): - """Codex's additive knob — NOT base_instructions, which replaces its whole prompt.""" - agent = CodexAgent(parse_agent_config(type=AgentKind.CODEX, system_prompt=PROMPT)) - - options = agent._build_thread_options() - - assert options["developer_instructions"] == PROMPT - assert "base_instructions" not in options - - -def test_codex_omits_the_knob_when_unset(): - """The field was dropped entirely before; absent must still mean "SDK default".""" - agent = CodexAgent(parse_agent_config(type=AgentKind.CODEX)) - - assert "developer_instructions" not in agent._build_thread_options() - - -# --- antigravity --------------------------------------------------------------------- - - -async def test_antigravity_passes_a_plain_string(monkeypatch, tmp_path): - """A plain str becomes a TemplatedSystemInstructions SECTION on top of the defaults. - - types.CustomSystemInstructions would replace them wholesale — the SDK's own - docstring flags it as advanced usage that drops the core safety mandates. - """ - configs: list = [] - - class _FakeSdkAgent: - def __init__(self, cfg): - configs.append(cfg) - - async def __aenter__(self): - return self - - async def __aexit__(self, *exc): - return False - - ag = ModuleType("google.antigravity") - ag.Agent = _FakeSdkAgent - ag.LocalAgentConfig = lambda **kwargs: SimpleNamespace(models=[], **kwargs) - ag.types = SimpleNamespace( - ThinkingLevel=lambda level: level, - GeminiAPIEndpoint=type("GeminiAPIEndpoint", (), {}), - GeminiModelOptions=lambda **kw: SimpleNamespace(**kw), - CapabilitiesConfig=lambda **kw: SimpleNamespace(**kw), - ) - hooks = ModuleType("google.antigravity.hooks") - hooks.policy = SimpleNamespace(allow_all=lambda: SimpleNamespace(kind="allow_all")) - import sys - - google_pkg = sys.modules.get("google") or ModuleType("google") - monkeypatch.setitem(sys.modules, "google", google_pkg) - monkeypatch.setitem(sys.modules, "google.antigravity", ag) - monkeypatch.setitem(sys.modules, "google.antigravity.hooks", hooks) - - agent = AntigravityAgent(parse_agent_config(type=AgentKind.ANTIGRAVITY, system_prompt=PROMPT)) - await agent.start(str(tmp_path)) - - assert configs[0].system_instructions == PROMPT - - -# --- the shared visible-turn definition ---------------------------------------------- - - -def _tool_end(collector: EventCollector, tool_id: str) -> None: - from datetime import datetime - - from coder_eval.models import CommandTelemetry - - collector.on_event( - ToolEndEvent( - task_id="t", - turn_id="turn-1", - tool=CommandTelemetry(tool_name="Bash", tool_id=tool_id, timestamp=datetime.now(), sequence_number=0), - status=ToolEndStatus.OK, - ) - ) - - -def test_collector_visible_turn_count_counts_resolved_tool_calls(): - """The single definition Codex and Antigravity both cap against.""" - collector = EventCollector() - assert collector.visible_turn_count == 0 - - _tool_end(collector, "a") - _tool_end(collector, "b") - - assert collector.visible_turn_count == 2 - - -def test_collector_visible_turn_count_does_not_double_count_a_tool_id(): - """Keyed on tool_id, so a re-emitted end event cannot inflate the count past the cap.""" - collector = EventCollector() - - _tool_end(collector, "a") - _tool_end(collector, "a") - - assert collector.visible_turn_count == 1 - - -def test_collector_visible_turn_count_matches_the_built_record(): - """It is the live view of exactly the list ``TurnRecord.commands`` ends up holding.""" - collector = EventCollector() - for tool_id in ("a", "b", "c"): - _tool_end(collector, tool_id) - - assert collector.visible_turn_count == len(collector.build_turn_record().commands) - - -@pytest.mark.parametrize("agent_cls", [CodexAgent, AntigravityAgent]) -def test_both_capped_agents_declare_cooperative_stop(agent_cls): - """The turn cap reuses the cooperative-stop boundary, so both must support it.""" - assert agent_cls.supports_cooperative_stop is True diff --git a/tests/test_visible_turn_cap.py b/tests/test_visible_turn_cap.py new file mode 100644 index 00000000..93cf5f09 --- /dev/null +++ b/tests/test_visible_turn_cap.py @@ -0,0 +1,68 @@ +"""``run_limits.max_turns`` must mean the same thing on Codex and Antigravity. + +Neither SDK can express the cap natively — each delivers exactly one SDK turn per +``communicate()`` call, so a native counter would clamp at 1 no matter what the task +asked for. Both therefore count VISIBLE turns (resolved tool calls) off one shared +definition, ``EventCollector.visible_turn_count``, rather than two per-agent counters +that happen to agree. See docs/agents/HARNESS_PARITY.md. + +Per-agent enforcement (where the cap fires in the loop, and how the run finalizes) +is covered in test_codex_agent.py and test_antigravity_agent.py. +""" + +from datetime import datetime + +import pytest + +from coder_eval.agents.antigravity_agent import AntigravityAgent +from coder_eval.agents.codex_agent import CodexAgent +from coder_eval.models import CommandTelemetry +from coder_eval.streaming.collector import EventCollector +from coder_eval.streaming.events import ToolEndEvent, ToolEndStatus + + +def _tool_end(collector: EventCollector, tool_id: str) -> None: + collector.on_event( + ToolEndEvent( + task_id="t", + turn_id="turn-1", + tool=CommandTelemetry(tool_name="Bash", tool_id=tool_id, timestamp=datetime.now(), sequence_number=0), + status=ToolEndStatus.OK, + ) + ) + + +def test_collector_visible_turn_count_counts_resolved_tool_calls(): + """The single definition Codex and Antigravity both cap against.""" + collector = EventCollector() + assert collector.visible_turn_count == 0 + + _tool_end(collector, "a") + _tool_end(collector, "b") + + assert collector.visible_turn_count == 2 + + +def test_collector_visible_turn_count_does_not_double_count_a_tool_id(): + """Keyed on tool_id, so a re-emitted end event cannot inflate the count past the cap.""" + collector = EventCollector() + + _tool_end(collector, "a") + _tool_end(collector, "a") + + assert collector.visible_turn_count == 1 + + +def test_collector_visible_turn_count_matches_the_built_record(): + """It is the live view of exactly the list ``TurnRecord.commands`` ends up holding.""" + collector = EventCollector() + for tool_id in ("a", "b", "c"): + _tool_end(collector, tool_id) + + assert collector.visible_turn_count == len(collector.build_turn_record().commands) + + +@pytest.mark.parametrize("agent_cls", [CodexAgent, AntigravityAgent]) +def test_both_capped_agents_declare_cooperative_stop(agent_cls): + """The turn cap reuses the cooperative-stop boundary, so both must support it.""" + assert agent_cls.supports_cooperative_stop is True From 8464af501ab7bff590b84de22d3938f00055ae26 Mon Sep 17 00:00:00 2001 From: Bai Li Date: Wed, 12 Aug 2026 17:23:52 -0700 Subject: [PATCH 03/15] chore(agents): drop config_support and the Antigravity tool mapping Both turned out to be their own design problems rather than a side note to the turn cap. config_support (the ConfigSupport/ConfigFieldSupport declaration plus the resolution guard at three seats) had no real user: every declaration was APPROXIMATED, so the guard only ever fired for a synthetic test agent, and the one field that genuinely warranted UNHONORED could not be declared without hard-erroring live nightlies. The Antigravity allowed_tools/disallowed_tools mapping works, but the questions it raises are not small: whether the config field should be spelled in Claude's tool vocabulary at all, and what an allowlist that maps to nothing should do (falling back to every tool hands the model MORE than it asked for, which is the wrong direction for an eval harness). Leaves the PR to the visible-turn max_turns cap, the SDK env seam, the pinned simulator model, and the turns-avoided note. HARNESS_PARITY.md narrows to the run-limit contract it can actually back. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 1 - README.md | 2 +- docs/agents/CODEX.md | 6 +- docs/agents/HARNESS_PARITY.md | 75 ++++------ docs/index.md | 2 +- docs/llms.txt | 2 +- mkdocs.yml | 4 +- src/coder_eval/agent.py | 38 ----- src/coder_eval/agents/antigravity_agent.py | 102 +------------ src/coder_eval/agents/codex_agent.py | 18 +-- src/coder_eval/cli/plan_command.py | 9 -- .../orchestration/config_support.py | 100 ------------- src/coder_eval/orchestration/experiment.py | 15 +- src/coder_eval/orchestrator.py | 5 - tests/test_agent_config_support.py | 140 ------------------ tests/test_antigravity_agent.py | 136 ----------------- 16 files changed, 41 insertions(+), 614 deletions(-) delete mode 100644 src/coder_eval/orchestration/config_support.py delete mode 100644 tests/test_agent_config_support.py diff --git a/CLAUDE.md b/CLAUDE.md index f7737a7f..4fd942f0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -81,7 +81,6 @@ coder_eval/ ├── orchestration/ # Batch execution utilities │ ├── batch.py # Parallel task execution (run_batch + run_batch_resolved) │ ├── config.py # Batch run configuration -│ ├── config_support.py # validate_config_support: rejects a task setting a field its agent declares unhonored │ ├── early_stop.py # validate_early_stop guardrails + EarlyStopWatcher (armed live-verdict observer) │ ├── evaluation.py # Evaluation helpers │ ├── experiment.py # ExperimentRunner, resolve_task_for_variant, load_experiment diff --git a/README.md b/README.md index b666289e..a49d781f 100644 --- a/README.md +++ b/README.md @@ -192,7 +192,7 @@ alone. | [Claude Code](docs/agents/CLAUDE_CODE.md) | Configuring and running the default Claude Code agent | | [Codex](docs/agents/CODEX.md) | Running the OpenAI Codex agent | | [Antigravity (Gemini)](docs/agents/ANTIGRAVITY.md) | Running the Google Antigravity / Gemini agent | -| [Harness Config Parity](docs/agents/HARNESS_PARITY.md) | What each agent: field means on every harness, and where they diverge | +| [Run-Limit Parity](docs/agents/HARNESS_PARITY.md) | What each run_limits field means on every harness | | [A/B Experiments](docs/AB_EXPERIMENTS.md) | Compare models / tools / prompts across the same tasks | | [Bring Your Own Dataset](docs/DATASETS.md) | Fan a single task out over a dataset | | [Dialog Mode](docs/DIALOG_MODE.md) | Evaluate agents in multi-turn conversation via a simulated user | diff --git a/docs/agents/CODEX.md b/docs/agents/CODEX.md index d017991b..c817ecdc 100644 --- a/docs/agents/CODEX.md +++ b/docs/agents/CODEX.md @@ -214,18 +214,18 @@ The Codex SDK is synchronous. The agent uses `_run_async()` helper to detect and | **Command Tracking** | Full telemetry (tool name, params, duration) | Streamed telemetry: shell → `Bash`, apply_patch → `Write` | | **Model Selection** | Direct via `--model` or config | `agent.model` pinned into `thread_start` | | **Session Resume** | `--resume {session_id}` | Via thread ID | -| **Permissions** | `permission_mode` honored | `permission_mode` runs full-access on every mode — the sandbox driver is the boundary | +| **Permissions** | `permission_mode` + `allowed_tools` | `permission_mode` → sandbox/approval + `allowed_tools`/`disallowed_tools` → thread config | | **Tool Enforcement** | Not enforced by Coder Eval wrapper | `enabled_tools` honored; `disabled_tools` NOT enforced by the SDK | | **`max_turns`** | Native SDK turn cap (assistant messages) | Visible-turn cap (tool calls), enforced on the notification pump | | **Early stop** | Supported (cooperative `should_stop`, polled between messages) | Supported — polled after each streamed notification; the in-flight turn is interrupted best-effort | -Field-by-field, with the reasoning behind each divergence: [Harness Config Parity](HARNESS_PARITY.md). +Run-limit semantics per harness: [Run-Limit Parity](HARNESS_PARITY.md). ## Known Limitations 1. **Tool-name collapse** - Codex reports shell tools (`Read`/`Grep`/`Bash`) all as shell commands, surfaced as `Bash` telemetry; name-keyed criteria that distinguish these tools aren't meaningful across agents. 2. **`skill_triggered` criterion** - Codex has no distinct `Skill` tool (it engages a skill by reading its files via shell), so the criterion detects Codex engagement from that file-read signal (a command referencing `skills//`) instead of a `Skill` tool call. The file-read signal is weaker than Claude's explicit invocation. -3. **`disallowed_tools`** - passed to the SDK but not enforced; not a security boundary. Declared `approximated` in the agent's `config_support`. +3. **`disallowed_tools`** - passed to the SDK but not enforced; not a security boundary. 4. **Authentication** - Requires `CODEX_API_KEY` in the environment (point it at whichever endpoint's key you use — OpenAI, gateway, or Azure); the agent calls `login_api_key` when a key is present. `OPENAI_API_KEY`/`AZURE_OPENAI_API_KEY` are NOT read. 5. **Model field** - `TurnRecord.model_used` reflects the pinned `agent.model`; the Codex `Turn` payload itself doesn't carry the resolved model. 6. **Skills with Windows paths** - Symlink creation may fail on Windows; agent falls back to copying (slower). diff --git a/docs/agents/HARNESS_PARITY.md b/docs/agents/HARNESS_PARITY.md index a2624564..70699b57 100644 --- a/docs/agents/HARNESS_PARITY.md +++ b/docs/agents/HARNESS_PARITY.md @@ -1,37 +1,20 @@ -# Harness Config Parity +# Run-Limit Parity -One task file, run on three harnesses, must be the same task. This page is the -contract for how each shared `agent:` field is implemented on Claude Code, Codex, -and Antigravity — and, where a backend genuinely cannot implement one, what it -does instead. +One task file, run on three harnesses, must be the same task. `run_limits.max_turns` +was the field that broke that promise hardest: Claude Code enforced it, and Codex and +Antigravity accepted it and never read it, so `max_turns: 6` ran capped on one +backend and unbounded on the other two. -The declarations here are not prose: each agent class carries them as -`config_support`, and resolution rejects a task that sets a field its agent -declares it cannot honor. - -## Support states - -| State | Meaning | What happens | -|-------|---------|--------------| -| **honored** | Implemented faithfully. | Nothing to declare — the default. | -| **approximated** | Acted on, with a documented divergence. | The agent warns at `start()`; resolution allows it. | -| **unhonored** | Read by nothing. | Resolution **hard-errors** if the task sets it to a non-default value. | - -An agent declares only its divergences. An empty `config_support` asserts it -honors every shared field — so a field silently dropped without a declaration is -a bug, not a shortcut. +This page is the contract for what each run limit means per harness. ## The table -| Field | claude-code | codex | antigravity | +| Limit | claude-code | codex | antigravity | |---|---|---|---| -| `model` | honored | honored | honored | -| `allowed_tools` | honored | honored (`enabled_tools`) | honored (`CapabilitiesConfig.enabled_tools`) | -| `disallowed_tools` | honored | **approximated** — forwarded as `disabled_tools`, not enforced by the SDK | honored (subtracted from the allowlist) | -| `permission_mode` | honored | **approximated** — every mode runs full-access | **approximated** — every mode runs `policy.allow_all()` | -| `plugins` | honored | honored (symlinked into `.agents/skills/`) | honored (`skills_paths`) | -| `run_limits.max_turns` | honored (native SDK turn cap) | honored (visible-turn cap) | honored (visible-turn cap) | -| `run_limits.stop_early` | honored | honored | honored | +| `run_limits.max_turns` | native SDK cap (assistant messages) | visible-turn cap (resolved tool calls) | visible-turn cap (resolved tool calls) | +| `run_limits.turn_timeout` | watchdog, SIGKILL on the CLI subprocess | watchdog + cooperative interrupt | watchdog + cooperative interrupt | +| `run_limits.task_timeout` | orchestrator-level, agent-agnostic | orchestrator-level, agent-agnostic | orchestrator-level, agent-agnostic | +| `run_limits.stop_early` | cooperative `should_stop` | cooperative `should_stop` | cooperative `should_stop` | ## `max_turns` counts visible turns on Codex and Antigravity @@ -43,8 +26,7 @@ both. They need their own counter because a native one would be meaningless: Codex and Antigravity each deliver exactly **one SDK turn per `communicate()` call**, so an -SDK-level cap would clamp at 1 no matter what the task asked for. Before this, -both simply ignored the field. +SDK-level cap would clamp at 1 no matter what the task asked for. The cap is enforced on the same loop boundary as the cooperative early stop: the step or notification that reaches the cap is processed whole, and the next one is @@ -57,29 +39,26 @@ That is a real, honored cap, so it is left alone rather than restated in a different unit; the same `max_turns: 20` therefore bounds slightly different things on claude-code than on the other two. Documented rather than papered over. -## `permission_mode` does not confine any harness +### What a capped run looks like -On Codex and Antigravity, every mode runs unconfined, by design: +The three signals a capped run leaves behind, on every backend: -- coder_eval's isolation boundary is the **sandbox driver** — a Docker container, - or an ephemeral per-task tempdir it creates and discards. An in-agent approval - policy on top of that is redundant. -- Codex's own OS sandbox actively breaks on the paths we run: Landlock is - unavailable inside the container, the `bwrap` re-exec is denied on constrained CI - agents, and Windows has no OS sandbox at all. In each case writes fail silently - and the task scores 0 with no loud error. -- The modes below `bypassPermissions` differ only in *what they would ask a human - about*, and there is no human on a headless eval path. +- `final_status` is a completed status, not `agent_crash`. The cap is an ordinary + end-of-run, so criteria are still checked against whatever the agent produced. +- `max_turns_exhausted: true` on the turn record. +- `visible_turn_count` equals the cap on Codex and Antigravity. On claude-code it + is whatever tool calls fit inside the assistant-message budget, so it is + bounded by the cap rather than equal to it. -This is declared as **approximated** rather than unhonored — the isolation the -field implies is provided, one layer down — so setting `bypassPermissions` on a -nightly does not fail resolution. +## Timeouts are unchanged by this contract -**For adversarial or untrusted evals, use the Docker driver.** The tempdir/host -driver is a working directory, not a confinement boundary, on any of the three. +`turn_timeout` and `task_timeout` already behaved consistently and are listed here +only so the parity table is complete. A timeout is a *failure* (partial turn +captured, `agent_crash` / timeout status); the turn cap is a *clean stop*. Conflating +them is the mistake this page exists to prevent: a task whose cap fires should not +look like a task whose harness hung. ## Related - [Claude Code](CLAUDE_CODE.md) · [Codex](CODEX.md) · [Antigravity](ANTIGRAVITY.md) -- [Task Definition Guide](../TASK_DEFINITION_GUIDE.md) — the full `agent:` schema -- [Extending Coder Eval](../EXTENDING.md) — declaring `config_support` on a new agent +- [Task Definition Guide](../TASK_DEFINITION_GUIDE.md) — the full `run_limits` schema diff --git a/docs/index.md b/docs/index.md index 17c0509a..1eda9606 100644 --- a/docs/index.md +++ b/docs/index.md @@ -81,7 +81,7 @@ New here? Start with **[Tutorial 01 — Your First Evaluation](tutorials/01-firs | [Claude Code](agents/CLAUDE_CODE.md) | Configuring and running the default Claude Code agent | | [Codex](agents/CODEX.md) | Running the OpenAI Codex agent | | [Antigravity (Gemini)](agents/ANTIGRAVITY.md) | Running the Google Antigravity / Gemini agent | -| [Harness Config Parity](agents/HARNESS_PARITY.md) | What each agent: field means on every harness, and where they diverge | +| [Run-Limit Parity](agents/HARNESS_PARITY.md) | What each run_limits field means on every harness | | [A/B Experiments](AB_EXPERIMENTS.md) | Compare models / tools / prompts across the same tasks | | [Bring Your Own Dataset](DATASETS.md) | Fan a single task out over a dataset | | [Dialog Mode](DIALOG_MODE.md) | Evaluate agents in multi-turn conversation via a simulated user | diff --git a/docs/llms.txt b/docs/llms.txt index 3a83937a..04d1adf3 100644 --- a/docs/llms.txt +++ b/docs/llms.txt @@ -28,7 +28,7 @@ and A/B plumbing. - [Claude Code](https://coder-eval.com/docs/agents/claude-code): Configuring and running the default Claude Code agent - [Codex](https://coder-eval.com/docs/agents/codex): Running the OpenAI Codex agent - [Antigravity (Gemini)](https://coder-eval.com/docs/agents/antigravity): Running the Google Antigravity / Gemini agent -- [Harness Config Parity](https://coder-eval.com/docs/agents/harness-parity): What each agent: field means on every harness, and where they diverge +- [Run-Limit Parity](https://coder-eval.com/docs/agents/harness-parity): What each run_limits field means on every harness - [A/B Experiments](https://coder-eval.com/docs/ab-experiments): Compare models / tools / prompts across the same tasks - [Bring Your Own Dataset](https://coder-eval.com/docs/datasets): Fan a single task out over a dataset - [Dialog Mode](https://coder-eval.com/docs/dialog-mode): Evaluate agents in multi-turn conversation via a simulated user diff --git a/mkdocs.yml b/mkdocs.yml index eb3af209..b3370f59 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -83,7 +83,7 @@ extra: agents/CLAUDE_CODE.md: "Configuring and running the default Claude Code agent" agents/CODEX.md: "Running the OpenAI Codex agent" agents/ANTIGRAVITY.md: "Running the Google Antigravity / Gemini agent" - agents/HARNESS_PARITY.md: "What each agent: field means on every harness, and where they diverge" + agents/HARNESS_PARITY.md: "What each run_limits field means on every harness" AB_EXPERIMENTS.md: "Compare models / tools / prompts across the same tasks" DATASETS.md: "Fan a single task out over a dataset" DIALOG_MODE.md: "Evaluate agents in multi-turn conversation via a simulated user" @@ -110,7 +110,7 @@ nav: - Claude Code: agents/CLAUDE_CODE.md - Codex: agents/CODEX.md - Antigravity (Gemini): agents/ANTIGRAVITY.md - - Harness Config Parity: agents/HARNESS_PARITY.md + - Run-Limit Parity: agents/HARNESS_PARITY.md - Advanced: - A/B Experiments: AB_EXPERIMENTS.md - Bring Your Own Dataset: DATASETS.md diff --git a/src/coder_eval/agent.py b/src/coder_eval/agent.py index c2d5e276..e5ebbf9a 100644 --- a/src/coder_eval/agent.py +++ b/src/coder_eval/agent.py @@ -6,8 +6,6 @@ import logging from abc import ABC, abstractmethod from collections.abc import Callable -from dataclasses import dataclass -from enum import StrEnum from typing import Any, ClassVar, NoReturn, Protocol from .errors import AgentCrashError, TurnTimeoutError @@ -22,34 +20,6 @@ logger = logging.getLogger(__name__) -class ConfigSupport(StrEnum): - """How faithfully one agent backend implements a shared ``BaseAgentConfig`` field. - - A base-config field must mean the same thing on every harness, and where it - cannot, the divergence has to be declared rather than discovered from a run that - quietly did something else. Every field an agent does not fully implement is - listed in its ``config_support`` map with one of these and a reason. - """ - - APPROXIMATED = "approximated" - """Accepted and acted on, but with a documented divergence the operator must know - about (e.g. Codex forwards ``disallowed_tools`` to the SDK, which does not enforce - it). The agent warns at ``start()``; resolution does NOT reject.""" - - UNHONORED = "unhonored" - """Read by nothing — setting it changes no behavior. Resolution HARD-ERRORS when a - task sets the field to anything other than its model default, because a silently - dropped field means two harnesses reading one task file run different tasks.""" - - -@dataclass(frozen=True) -class ConfigFieldSupport: - """One entry in an agent's ``config_support`` map: the state plus why.""" - - support: ConfigSupport - reason: str - - class _FinalizeFn(Protocol): """The per-turn ``finalize`` callback shared by every agent's turn-state. @@ -116,14 +86,6 @@ def __init__(self, config: ClaudeCodeAgentConfig, ...): # crash every agent (NoOp/Codex/Antigravity/plugins) whose ``__init__`` lacks it. supports_cost_log_tags: ClassVar[bool] = False - # Declared divergences from the shared ``BaseAgentConfig`` contract, keyed by field - # name. Empty (the default) asserts the agent honors every field, so a new agent - # opts in to scrutiny only where it must — but a field it silently drops without - # declaring is a bug, not a shortcut. Read by - # ``orchestration/config_support.py::validate_config_support`` at resolution and by - # the parity table in docs/agents/HARNESS_PARITY.md. - config_support: ClassVar[dict[str, ConfigFieldSupport]] = {} - def _begin_turn(self) -> None: """Mark the start of a ``communicate()`` turn: reset the pending slot and bump the iteration counter so a mid-turn failure can be rolled back. diff --git a/src/coder_eval/agents/antigravity_agent.py b/src/coder_eval/agents/antigravity_agent.py index 08d4b0c6..515d596a 100644 --- a/src/coder_eval/agents/antigravity_agent.py +++ b/src/coder_eval/agents/antigravity_agent.py @@ -29,7 +29,7 @@ from pathlib import Path from typing import Any, ClassVar -from coder_eval.agent import Agent, AgentState, ConfigFieldSupport, ConfigSupport +from coder_eval.agent import Agent, AgentState from coder_eval.agents._logging import PrefixedAdapter from coder_eval.agents.registry import AgentRegistry from coder_eval.agents.watchdog import ThreadedWatchdog @@ -93,23 +93,9 @@ "search_web": "WebSearch", "generate_image": "GenerateImage", "ask_question": "AskUser", - "read_url_content": "WebFetch", "finish": "Finish", } -# The inverse, for translating ``agent.allowed_tools`` / ``disallowed_tools`` (written -# in Claude names) into the harness's ``CapabilitiesConfig`` tool lists. Built from the -# forward map so the two can never drift; the forward map is 1:1, so the inversion is -# lossless. A Claude tool with no Antigravity counterpart (``Skill`` — Antigravity -# discovers skills through ``skills_paths``, not a tool; ``TodoWrite``; ...) is absent -# here and is dropped with a log line rather than crashing the enum validation. -_CLAUDE_TO_ANTIGRAVITY_TOOL_MAP: dict[str, str] = {v: k for k, v in _ANTIGRAVITY_TO_CLAUDE_TOOL_MAP.items()} - -# Tools that keep the harness's control flow working and are therefore never removed -# by an allowlist. ``finish`` is how the agent ends its turn — disabling it strands -# every run at the step-loop until the turn timeout fires. -_ANTIGRAVITY_STRUCTURAL_TOOLS: frozenset[str] = frozenset({"finish"}) - # Tool-call arg keys the harness ADDS at completion (the result payload), not # model-supplied inputs — stripped from CommandTelemetry.parameters and mined for # the tool result instead. This is the STATIC backstop; the live mapping ALSO @@ -182,20 +168,6 @@ class AntigravityAgent(Agent[AntigravityAgentConfig]): # ``should_stop`` check runs, so this agent supports early-stop-on-criterion. supports_cooperative_stop: ClassVar[bool] = True - # Declared divergence from the shared BaseAgentConfig contract. APPROXIMATED - # rather than UNHONORED for the same reason as Codex: the run is genuinely - # unconfined under every mode by design, and the isolation the field implies is - # provided one layer down by the sandbox driver — so the mode is not ignored so - # much as satisfied elsewhere. Rejecting it would break every task that sets - # bypassPermissions to mean "this is a headless eval, do not stop to ask". - config_support: ClassVar[dict[str, ConfigFieldSupport]] = { - "permission_mode": ConfigFieldSupport( - ConfigSupport.APPROXIMATED, - "every mode runs the harness with policy.allow_all(); coder_eval's isolation " - + "boundary is the sandbox driver, and a headless eval has no human to approve", - ), - } - def __init__( self, config: AntigravityAgentConfig, @@ -296,73 +268,6 @@ def _resolve_workspaces(self, skills_paths: list[str]) -> list[str]: """ return [str(self.working_directory), *skills_paths] - def _map_tools(self, tools: list[str], field: str) -> list[str]: - """Translate Claude-named tools to Antigravity builtin names, dropping unmappables. - - ``CapabilitiesConfig`` validates against the ``BuiltinTools`` enum, so an - unmapped name would raise instead of being ignored — hence the explicit drop - plus a log line naming what was dropped and why. - """ - mapped: list[str] = [] - dropped: list[str] = [] - for tool in tools: - antigravity_name = _CLAUDE_TO_ANTIGRAVITY_TOOL_MAP.get(tool) - if antigravity_name is None: - dropped.append(tool) - elif antigravity_name not in mapped: - mapped.append(antigravity_name) - if dropped: - self._log.debug( - "agent.%s entries with no Antigravity builtin were dropped: %s " - + "(Skill is expected here — Antigravity discovers skills via skills_paths, not a tool)", - field, - ", ".join(dropped), - ) - return mapped - - def _build_capabilities(self, types_mod: Any) -> Any: - """Build ``CapabilitiesConfig`` from ``allowed_tools`` / ``disallowed_tools``. - - The two SDK fields are mutually exclusive, so an allowlist wins and any - denylist is subtracted from it rather than passed separately — same resulting - tool set, no SDK validation error. Returns ``None`` when neither field - constrains anything, leaving the harness defaults (all tools) in force. - - The structural tools are always re-added: an allowlist that stripped ``finish`` - would leave the agent unable to end its turn. An allowlist that maps to nothing - usable falls back to the harness defaults with a warning — handing the model a - single ``finish`` tool produces a zero-scoring run with no diagnosable cause, - which is the worse failure for an eval harness. - """ - requested_allow = self.config.allowed_tools or [] - allowed = self._map_tools(requested_allow, "allowed_tools") - disallowed = self._map_tools(self.config.disallowed_tools or [], "disallowed_tools") - - # Branch on what the TASK asked for, not on what survived mapping: an - # allowlist whose every entry is unmappable must reach the warning below, - # not fall through to "no allowlist configured". - if requested_allow: - enabled = [t for t in allowed if t not in disallowed] - enabled += [t for t in sorted(_ANTIGRAVITY_STRUCTURAL_TOOLS) if t not in enabled] - if set(enabled) <= _ANTIGRAVITY_STRUCTURAL_TOOLS: - self._log.warning( - "agent.allowed_tools (%s) maps to no usable Antigravity tool; " - + "falling back to the harness default (all tools enabled).", - ", ".join(self.config.allowed_tools or []), - ) - return None - self._log.debug("Antigravity enabled_tools: %s", ", ".join(enabled)) - return types_mod.CapabilitiesConfig(enabled_tools=enabled) - - if disallowed: - disabled = [t for t in disallowed if t not in _ANTIGRAVITY_STRUCTURAL_TOOLS] - if not disabled: - return None - self._log.debug("Antigravity disabled_tools: %s", ", ".join(disabled)) - return types_mod.CapabilitiesConfig(disabled_tools=disabled) - - return None - def _harness_env(self) -> dict[str, str] | None: """Per-agent environment for the localharness subprocess (``LocalAgentConfig.env``). @@ -456,11 +361,6 @@ async def start( # inherited os.environ when it spawns the localharness, so two # concurrent tasks never see each other's mock dirs. env=self._harness_env(), - # allowed_tools / disallowed_tools → the harness's tool exposure. - # Stripping a tool from the model's context (rather than denying the - # call via a policy) matches how Claude Code and Codex read the same - # fields, and costs no tokens on rejected attempts. - **({"capabilities": capabilities} if (capabilities := self._build_capabilities(types)) else {}), ) # Attach the configured thinking level (reasoning effort) onto every # resolved model's Gemini endpoint. The SDK validates the model list in diff --git a/src/coder_eval/agents/codex_agent.py b/src/coder_eval/agents/codex_agent.py index 26e773fb..56c7e52c 100644 --- a/src/coder_eval/agents/codex_agent.py +++ b/src/coder_eval/agents/codex_agent.py @@ -15,7 +15,7 @@ from typing import Any, ClassVar from urllib.parse import urlparse -from coder_eval.agent import Agent, AgentState, ConfigFieldSupport, ConfigSupport +from coder_eval.agent import Agent, AgentState from coder_eval.agents._logging import PrefixedAdapter, log_raw_sdk_event from coder_eval.agents.registry import AgentRegistry from coder_eval.agents.watchdog import ThreadedWatchdog @@ -664,22 +664,6 @@ class CodexAgent(Agent[CodexAgentConfig]): # ``should_stop`` check runs, so this agent supports early-stop-on-criterion. supports_cooperative_stop: ClassVar[bool] = True - # Declared divergences from the shared BaseAgentConfig contract. Both are - # APPROXIMATED, not UNHONORED: the values are forwarded to the SDK and the agent - # warns about each at start() (_log_config_enforcement), so an operator reading - # the task log sees exactly what the harness did and did not enforce. - config_support: ClassVar[dict[str, ConfigFieldSupport]] = { - "permission_mode": ConfigFieldSupport( - ConfigSupport.APPROXIMATED, - "every mode resolves to full-access; coder_eval's isolation boundary is the " - + "sandbox driver, and Codex's own OS sandbox is unusable on our container/CI paths", - ), - "disallowed_tools": ConfigFieldSupport( - ConfigSupport.APPROXIMATED, - "forwarded to the SDK as disabled_tools but not enforced by it — not a security boundary", - ), - } - def __init__( self, config: CodexAgentConfig, diff --git a/src/coder_eval/cli/plan_command.py b/src/coder_eval/cli/plan_command.py index ca7f9c0e..264d863d 100644 --- a/src/coder_eval/cli/plan_command.py +++ b/src/coder_eval/cli/plan_command.py @@ -59,7 +59,6 @@ def plan_command( check_api_keys() # Lazy import to avoid circular dependency at module level - from ..orchestration.config_support import AgentConfigSupportError, validate_config_support from ..orchestration.early_stop import EarlyStopConfigError, validate_early_stop from ..orchestration.experiment import DEFAULT_EXPERIMENT_PATH, load_experiment, resolve_task_for_variant @@ -137,9 +136,6 @@ def plan_command( resolved, _lineage, _ = resolve_task_for_variant(default_exp, task, exp_def, variant) # Early-stop guardrails (no-op unless a criterion carries a stop_early: block). validate_early_stop(resolved) - # Agent config-support guardrail (no-op unless the task sets a field - # the chosen harness declares it does not implement). - validate_config_support(resolved) agent_type = str(resolved.agent.type) if resolved.agent else "unknown" agent_model = resolved.agent.model if resolved.agent else None model_str = f" ({agent_model})" if agent_model else "" @@ -149,11 +145,6 @@ def plan_command( # failures, which stay soft): flip the plan exit code. console.print(f" [red]Variant '{variant.variant_id}': early-stop config error - {e}[/red]") all_valid = False - except AgentConfigSupportError as e: - # Same hard-error treatment: the task asks this harness for - # something it declares it cannot do. - console.print(f" [red]Variant '{variant.variant_id}': agent config error - {e}[/red]") - all_valid = False except Exception as e: console.print(f" [red]Variant '{variant.variant_id}': resolution failed - {e}[/red]") diff --git a/src/coder_eval/orchestration/config_support.py b/src/coder_eval/orchestration/config_support.py deleted file mode 100644 index 799cef1b..00000000 --- a/src/coder_eval/orchestration/config_support.py +++ /dev/null @@ -1,100 +0,0 @@ -"""Resolution-time guard on per-agent ``BaseAgentConfig`` support declarations. - -A base-config field must mean the same thing on every harness. Where a backend -cannot implement one, it says so on its agent class (``Agent.config_support``) -instead of dropping the field at runtime, and this module turns the strictest of -those declarations — :attr:`~coder_eval.agent.ConfigSupport.UNHONORED` — into a -hard error at resolution. - -The error fires only when the resolved task actually *sets* the field to -something other than the config model's default. A default-valued field carries -no intent, so rejecting it would break every task on the harness rather than the -ones whose author expected the field to do something. - -:attr:`~coder_eval.agent.ConfigSupport.APPROXIMATED` fields deliberately do NOT -raise here: they are honored, just imperfectly, and each agent already warns -about its own divergence at ``start()`` where the concrete resolved value is in -hand. Silence at resolution, loud in the task log. - -Mirrors ``early_stop.py::validate_early_stop`` in shape and call sites: a -``ValueError`` subclass so the run path's resolve -> ``typer.BadParameter`` -conversion covers it, caught explicitly by ``plan`` to flip its exit code. -""" - -from __future__ import annotations - -from typing import TYPE_CHECKING, Any - -from coder_eval.agent import ConfigSupport - - -if TYPE_CHECKING: - from coder_eval.models import BaseAgentConfig, TaskDefinition - - -class AgentConfigSupportError(ValueError): - """Raised when a task sets a config field the chosen agent does not implement.""" - - -def _is_default(config: BaseAgentConfig, field: str) -> bool: - """True when ``field`` still holds the config model's declared default. - - Compares against the field's default rather than checking - ``model_fields_set``, because by the time a task resolves, the five-layer - merge has explicitly set nearly every field — ``model_fields_set`` would - report the whole block as author intent. The default is what "the author did - not ask for anything here" actually looks like on a merged config. - """ - model_field = type(config).model_fields.get(field) - if model_field is None: - # The field does not exist on THIS agent's config subclass, so the task - # cannot have set it. A declaration naming a field its own config lacks is - # a typo, but not a task author's problem — let it pass silently here and - # let the lint rule catch it. - return True - default: Any = model_field.get_default(call_default_factory=True) - return getattr(config, field, default) == default - - -def validate_config_support(task: TaskDefinition) -> None: - """Reject a resolved task that sets a field its agent declares unhonored. - - Called after the config layers have merged — the same seats as - ``validate_early_stop`` (``resolve_all_tasks`` post-CLI overrides, the - ``plan`` per-variant loop, and defensively in ``Orchestrator._setup``). - No-op for an agent that declares nothing (every built-in but Codex and - Antigravity) and for a task that leaves the declared fields at their default. - - Raises: - AgentConfigSupportError: on any set-but-unhonored field. - """ - config = task.agent - if config is None or config.type is None: - return - - # Lazily import the registry + plugin loader so this module stays free of - # runtime coder_eval imports beyond the ABC itself (mirrors early_stop). - from coder_eval.agents.registry import AgentRegistry - from coder_eval.plugins import ensure_plugins_loaded - - ensure_plugins_loaded() - registration = AgentRegistry.get(str(config.type)) - if registration is None: - # Not this guard's failure to report: an unregistered type already raises a - # clear "is the providing plugin installed?" error where the agent is built. - return - - offenders = [ - (field, note) - for field, note in registration.agent_class.config_support.items() - if note.support is ConfigSupport.UNHONORED and not _is_default(config, field) - ] - if not offenders: - return - - details = "; ".join(f"agent.{field} ({note.reason})" for field, note in offenders) - raise AgentConfigSupportError( - f"agent type {str(config.type)!r} does not implement: {details}. " - + "Leaving these set would run a different task than the same file runs on another " - + "harness. Remove them, or pick an agent type that implements them." - ) diff --git a/src/coder_eval/orchestration/experiment.py b/src/coder_eval/orchestration/experiment.py index c02a5f91..2c75bad9 100644 --- a/src/coder_eval/orchestration/experiment.py +++ b/src/coder_eval/orchestration/experiment.py @@ -580,7 +580,6 @@ def resolve_all_tasks( Raises: ValueError: If duplicate task IDs are found after resolution. """ - from .config_support import AgentConfigSupportError, validate_config_support from .early_stop import EarlyStopConfigError, validate_early_stop resolved: list[ResolvedTask] = [] @@ -673,12 +672,6 @@ def resolve_all_tasks( # a bad arming raises EarlyStopConfigError (a ValueError). validate_early_stop(resolved_task) - # Agent config-support guardrail: reject a task that sets a field - # the chosen harness declares it does not implement, so one task - # file cannot silently run as two different tasks. No-op unless a - # declared-unhonored field is set to a non-default value. - validate_config_support(resolved_task) - # Fan-out: simulation n_trials takes precedence over experiment repeats # when simulation is active; otherwise use experiment-level repeats. sim = resolved_task.simulation @@ -701,10 +694,10 @@ def resolve_all_tasks( config_lineage=dict(lineage), ) ) - # Early-stop arming and agent-config-support errors are a deliberate hard - # stop: they always propagate (never demoted to skipped) so a misconfigured - # run fails loudly instead of quietly shrinking the suite. - except (EarlyStopConfigError, AgentConfigSupportError): + # Early-stop arming errors are a deliberate hard stop: they always + # propagate (never demoted to skipped) so a misconfigured run fails loudly + # instead of quietly shrinking the suite. + except EarlyStopConfigError: raise # Narrow set, matching the load/expand block above: config-resolution # and IO failures are collected (decided after the loop, below); diff --git a/src/coder_eval/orchestrator.py b/src/coder_eval/orchestrator.py index 4a67185c..83f67da3 100644 --- a/src/coder_eval/orchestrator.py +++ b/src/coder_eval/orchestrator.py @@ -56,7 +56,6 @@ resolve_evaluation_route, resolve_route, ) -from .orchestration.config_support import validate_config_support from .orchestration.early_stop import EarlyStopWatcher, early_stop_active, validate_early_stop from .orchestration.evaluation import load_reference from .path_utils import format_task_log_id, task_log_path @@ -990,10 +989,6 @@ async def _setup(self) -> None: # some criterion carries a stop_early: block. validate_early_stop(self.task) - # Same defensive posture for the agent config-support guardrail: no-op unless - # the task sets a field this harness declares it does not implement. - validate_config_support(self.task) - # Build the early-stop watcher once, up front, when armed (>= 1 criterion # with a stop_early: block and the run_limits.stop_early kill switch not # thrown). This sits BEFORE the evaluate-only early return below, so an diff --git a/tests/test_agent_config_support.py b/tests/test_agent_config_support.py deleted file mode 100644 index 44a05d93..00000000 --- a/tests/test_agent_config_support.py +++ /dev/null @@ -1,140 +0,0 @@ -"""Tests for per-agent ``BaseAgentConfig`` support declarations and their guard. - -A base-config field must mean the same thing on every harness. Where a backend -cannot implement one it declares the divergence on its agent class instead of -dropping the field at runtime; ``validate_config_support`` turns the strictest -declaration (UNHONORED) into a resolution-time error. -""" - -from typing import ClassVar - -import pytest - -from coder_eval.agent import Agent, ConfigFieldSupport, ConfigSupport -from coder_eval.agents.antigravity_agent import AntigravityAgent -from coder_eval.agents.claude_code_agent import ClaudeCodeAgent -from coder_eval.agents.codex_agent import CodexAgent -from coder_eval.agents.registry import AgentRegistry -from coder_eval.models import CodexAgentConfig, FileExistsCriterion, TaskDefinition, parse_agent_config -from coder_eval.orchestration.config_support import AgentConfigSupportError, validate_config_support -from coder_eval.plugins import ensure_plugins_loaded - - -def _task(**agent_kwargs) -> TaskDefinition: - return TaskDefinition( - task_id="t", - description="d", - initial_prompt="p", - agent=parse_agent_config(**agent_kwargs), - success_criteria=[FileExistsCriterion(path="x", description="f")], - ) - - -# --- the declarations themselves --------------------------------------------------- - - -def test_claude_code_declares_no_divergence(): - """Claude Code is the reference implementation — it honors every shared field.""" - assert ClaudeCodeAgent.config_support == {} - - -def test_codex_declares_permission_mode_and_disallowed_tools_approximated(): - """Both are forwarded and warned about at start(), so neither is a hard rejection.""" - support = CodexAgent.config_support - assert support["permission_mode"].support is ConfigSupport.APPROXIMATED - assert support["disallowed_tools"].support is ConfigSupport.APPROXIMATED - assert all(note.reason for note in support.values()) - - -def test_antigravity_declares_permission_mode_approximated(): - support = AntigravityAgent.config_support - assert support["permission_mode"].support is ConfigSupport.APPROXIMATED - assert "allowed_tools" not in support # honored since the CapabilitiesConfig wiring - - -def test_every_declared_field_exists_on_that_agents_config(): - """A declaration naming a field the config lacks is dead text that can never fire.""" - ensure_plugins_loaded() - for registration in AgentRegistry.registrations(): - fields = registration.config_class.model_fields - for field in registration.agent_class.config_support: - assert field in fields, f"{registration.agent_class.__name__} declares unknown field {field!r}" - - -# --- the resolution guard ----------------------------------------------------------- - - -def test_approximated_field_does_not_raise(): - """bypassPermissions on Codex is approximated, not rejected — the nightly sets it.""" - validate_config_support(_task(type="codex", permission_mode="bypassPermissions")) - - -def test_no_declarations_is_a_noop(): - validate_config_support(_task(type="claude-code", permission_mode="bypassPermissions")) - - -def test_task_without_agent_type_is_a_noop(): - validate_config_support( - TaskDefinition( - task_id="t", - description="d", - initial_prompt="p", - success_criteria=[FileExistsCriterion(path="x", description="f")], - ) - ) - - -class _StrictAgent(Agent[CodexAgentConfig]): - """A synthetic agent that genuinely drops a field, to drive the UNHONORED path. - - No built-in declares UNHONORED today (the whole point of this PR is that the - known drops were implemented instead), so the reject path needs a stand-in to - stay covered as rot-protection for the next agent that adds one. - """ - - config_support: ClassVar[dict[str, ConfigFieldSupport]] = { - "model": ConfigFieldSupport(ConfigSupport.UNHONORED, "pinned to a fixed model by the vendor"), - } - - async def start(self, working_directory, *, env_path_prepend=None, plugin_tools_dir=None) -> None: ... - - async def communicate( # type: ignore[empty-body] - self, user_input, *, stream_callback=None, timeout=None, max_turns=None, should_stop=None - ): ... - - async def stop(self) -> None: ... - - -@pytest.fixture -def strict_codex(): - """Bind ``_StrictAgent`` to the ``codex`` kind for one test, then restore. - - Rebinding the existing kind (rather than adding a new one) lets the tests build - tasks through the normal ``type: codex`` path — ``_StrictAgent`` deliberately - reuses ``CodexAgentConfig``, so nothing about resolution changes except which - ``config_support`` map the guard reads. - """ - ensure_plugins_loaded() - saved = dict(AgentRegistry._registry) - AgentRegistry._registry["codex"] = type(saved["codex"])(agent_class=_StrictAgent, config_class=CodexAgentConfig) - yield - AgentRegistry._registry.clear() - AgentRegistry._registry.update(saved) - - -def test_unhonored_field_set_to_non_default_raises(strict_codex): - with pytest.raises(AgentConfigSupportError, match="does not implement"): - validate_config_support(_task(type="codex", model="gpt-5.5")) - - -def test_unhonored_field_left_at_default_does_not_raise(strict_codex): - """A field the five-layer merge never touched carries no author intent.""" - validate_config_support(_task(type="codex")) - - -def test_error_names_the_field_and_the_reason(strict_codex): - with pytest.raises(AgentConfigSupportError) as exc: - validate_config_support(_task(type="codex", model="gpt-5.5")) - - assert "agent.model" in str(exc.value) - assert "pinned to a fixed model by the vendor" in str(exc.value) diff --git a/tests/test_antigravity_agent.py b/tests/test_antigravity_agent.py index 8d743cbb..134dec8c 100644 --- a/tests/test_antigravity_agent.py +++ b/tests/test_antigravity_agent.py @@ -5,7 +5,6 @@ """ import asyncio -import logging import os import sys from types import ModuleType, SimpleNamespace @@ -703,99 +702,6 @@ def _agent(**cfg) -> AntigravityAgent: return AntigravityAgent(parse_agent_config(type="antigravity", **cfg)) -def _fake_types() -> SimpleNamespace: - return SimpleNamespace(CapabilitiesConfig=lambda **kw: SimpleNamespace(**kw)) - - -def test_claude_to_antigravity_tool_map_is_exact_inverse(): - """The inverse map is derived, so a forward-map edit can never leave it stale.""" - from coder_eval.agents.antigravity_agent import _CLAUDE_TO_ANTIGRAVITY_TOOL_MAP - - assert len(_CLAUDE_TO_ANTIGRAVITY_TOOL_MAP) == len(_ANTIGRAVITY_TO_CLAUDE_TOOL_MAP) - for antigravity_name, claude_name in _ANTIGRAVITY_TO_CLAUDE_TOOL_MAP.items(): - assert _CLAUDE_TO_ANTIGRAVITY_TOOL_MAP[claude_name] == antigravity_name - - -def test_allowed_tools_become_enabled_tools(): - """The repo-default allowlist maps onto the matching Antigravity builtins.""" - agent = _agent(allowed_tools=["Bash", "Read", "Write", "Edit", "Glob", "Grep"]) - - caps = agent._build_capabilities(_fake_types()) - - assert caps.enabled_tools == [ - "run_command", - "view_file", - "create_file", - "edit_file", - "find_file", - "search_directory", - "finish", - ] - - -def test_allowed_tools_always_keep_finish(): - """`finish` is how a turn ends — an allowlist must never strip it.""" - agent = _agent(allowed_tools=["Read"]) - - caps = agent._build_capabilities(_fake_types()) - - assert "finish" in caps.enabled_tools - - -def test_unmappable_allowed_tools_are_dropped_not_raised(): - """`Skill` has no Antigravity builtin (skills come from skills_paths) — drop it.""" - agent = _agent(allowed_tools=["Skill", "Bash"]) - - caps = agent._build_capabilities(_fake_types()) - - assert "Skill" not in caps.enabled_tools - assert "run_command" in caps.enabled_tools - - -def test_allowlist_that_maps_to_nothing_falls_back_to_defaults(caplog): - """An allowlist of only-unmappable names must not hand the model just `finish`. - - Enabling nothing but the turn-ender scores 0 with no diagnosable cause, so the - harness default (all tools) plus a loud warning is the better failure mode. - """ - agent = _agent(allowed_tools=["Skill", "TodoWrite"]) - - with caplog.at_level(logging.WARNING, logger="coder_eval.agents.antigravity_agent"): - caps = agent._build_capabilities(_fake_types()) - - assert caps is None - assert "maps to no usable Antigravity tool" in caplog.text - - -def test_disallowed_tools_are_subtracted_from_an_allowlist(): - """enabled_tools and disabled_tools are mutually exclusive in the SDK, so subtract.""" - agent = _agent(allowed_tools=["Bash", "Read", "Write"], disallowed_tools=["Write"]) - - caps = agent._build_capabilities(_fake_types()) - - # One field only — passing both would fail the SDK's mutual-exclusion validator. - assert not hasattr(caps, "disabled_tools") - assert caps.enabled_tools == ["run_command", "view_file", "finish"] - - -def test_disallowed_tools_alone_become_disabled_tools(): - agent = _agent(disallowed_tools=["WebSearch"]) - - caps = agent._build_capabilities(_fake_types()) - - assert caps.disabled_tools == ["search_web"] - - -def test_disallowed_tools_cannot_disable_a_structural_tool(): - agent = _agent(disallowed_tools=["Finish"]) - - assert agent._build_capabilities(_fake_types()) is None - - -def test_no_tool_fields_leaves_harness_defaults(): - assert _agent()._build_capabilities(_fake_types()) is None - - @pytest.mark.parametrize("mode", ["default", "acceptEdits", "plan", "bypassPermissions"]) async def test_permission_mode_never_confines_the_harness(monkeypatch, tmp_path, mode: str): """permission_mode is declared unhonored: every mode stays fully autonomous. @@ -823,48 +729,6 @@ async def __aexit__(self, *exc): assert [p.kind for p in configs[0].policies] == ["allow_all"] -async def test_start_passes_capabilities_to_sdk_config(monkeypatch, tmp_path): - """End-to-end: the allowlist reaches LocalAgentConfig, not just the builder.""" - configs: list[Any] = [] - - class _FakeSdkAgent: - def __init__(self, cfg): - configs.append(cfg) - - async def __aenter__(self): - return self - - async def __aexit__(self, *exc): - return False - - _install_fake_sdk(monkeypatch, _FakeSdkAgent) - - await _agent(allowed_tools=["Bash", "Read"]).start(str(tmp_path)) - - assert configs[0].capabilities.enabled_tools == ["run_command", "view_file", "finish"] - - -async def test_start_omits_capabilities_when_unconstrained(monkeypatch, tmp_path): - """No allowlist → the kwarg is absent entirely, so the SDK default stands.""" - configs: list[Any] = [] - - class _FakeSdkAgent: - def __init__(self, cfg): - configs.append(cfg) - - async def __aenter__(self): - return self - - async def __aexit__(self, *exc): - return False - - _install_fake_sdk(monkeypatch, _FakeSdkAgent) - - await _agent().start(str(tmp_path)) - - assert not hasattr(configs[0], "capabilities") - - # --- max_turns visible-turn cap ----------------------------------------------------- # # max_turns was accepted and never read on this backend, so a task capping turns ran From 034584bcef4bd1b742eafb6aae575a477d891bd3 Mon Sep 17 00:00:00 2001 From: Bai Li Date: Wed, 12 Aug 2026 17:24:21 -0700 Subject: [PATCH 04/15] test(run-limits): add cross-harness max_turns / turn_timeout fixtures One task file per limit, run with --type claude-code / codex / antigravity, so the three harnesses can be compared on the same prompt. The max_turns fixture asks for 12 sequential tool calls under a cap of 4; the turn_timeout fixture blocks for 240s under a 45s watchdog. Co-Authored-By: Claude Opus 5 (1M context) --- tasks/run_limits/max_turns_cap.yaml | 33 +++++++++++++++++++++++++++++ tasks/run_limits/turn_timeout.yaml | 24 +++++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 tasks/run_limits/max_turns_cap.yaml create mode 100644 tasks/run_limits/turn_timeout.yaml diff --git a/tasks/run_limits/max_turns_cap.yaml b/tasks/run_limits/max_turns_cap.yaml new file mode 100644 index 00000000..b1cf6d47 --- /dev/null +++ b/tasks/run_limits/max_turns_cap.yaml @@ -0,0 +1,33 @@ +task_id: run-limits-max-turns-cap +description: >- + Parity fixture for run_limits.max_turns. The prompt asks for far more + sequential tool calls than the cap allows, so every harness must stop at the + cap rather than running the prompt to completion. Run it with --type + claude-code / codex / antigravity and compare: the cap must produce a CLEAN + stop (max_turns_exhausted, criteria still checked), never a crash. + +initial_prompt: | + Create 12 files in the current directory named step-01.txt through step-12.txt. + Each file must contain its own name and nothing else. + + Create them ONE AT A TIME. Run a separate shell command for each file. Do not + use a loop, do not combine several files into one command, and do not batch + multiple tool calls together. Work strictly in order, starting at step-01.txt. + +run_limits: + # Far below the 12 the prompt asks for, so the cap always decides the ending. + max_turns: 4 + # Generous: this fixture must fail on the cap, never on the clock. + turn_timeout: 300 + task_timeout: 600 + +agent: + permission_mode: bypassPermissions + +success_criteria: + # The early files prove the agent really was working when the cap cut it off, + # which distinguishes "capped" from "never started". + - type: file_exists + path: "step-01.txt" + description: "First file was created before the cap fired" + weight: 1.0 diff --git a/tasks/run_limits/turn_timeout.yaml b/tasks/run_limits/turn_timeout.yaml new file mode 100644 index 00000000..001ec952 --- /dev/null +++ b/tasks/run_limits/turn_timeout.yaml @@ -0,0 +1,24 @@ +task_id: run-limits-turn-timeout +description: >- + Parity fixture for run_limits.turn_timeout. The prompt blocks far longer than + the timeout allows, so every harness must abort the turn on the watchdog. The + contrast with the max_turns fixture is the point: a timeout is a FAILURE with a + partial turn captured, while the turn cap is a clean stop. + +initial_prompt: | + Run the shell command `sleep 240` and wait for it to finish. When it returns, + report its exit code. Do not run it in the background, and do not shorten the + sleep. + +run_limits: + turn_timeout: 45 + task_timeout: 180 + +agent: + permission_mode: bypassPermissions + +success_criteria: + - type: file_exists + path: "never-created.txt" + description: "Never satisfied — the turn is expected to time out first" + weight: 1.0 From 453904881b51924c15970ef963df1449518a44e4 Mon Sep 17 00:00:00 2001 From: Bai Li Date: Wed, 12 Aug 2026 17:38:08 -0700 Subject: [PATCH 05/15] docs(run-limits): record the measured cross-harness parity results Ran both fixtures on claude-code (Bedrock), codex (gpt-5.4) and antigravity (gemini-3.5-flash) on the eval VM. Three findings worth writing down: The cap works and is now identical on the two backends that ignored it: 12 writes requested under max_turns 4, all three stop at exactly 4 resolved tool calls with max_turns_exhausted and a clean SUCCESS. The claude-code unit difference is bigger than "slightly different". Under a batching prompt with max_turns 2, claude-code permitted all 12 writes across 14 assistant messages, because one SDK agent-loop turn absorbs however many parallel calls the model emits. Codex and Antigravity stop at 2. Same number, very different budget. Antigravity's localharness backgrounds any shell command over ~10s, so turn_timeout never fires for a slow command: sleep 240 under a 45s watchdog ended the turn at 19.3s with the tool force-closed as unresolved, while claude-code and codex both timed out at ~45s with the partial turn captured. That also means any task whose real work is a long install or build is not the same task on Antigravity. Documented in both pages; no workaround attempted here. Co-Authored-By: Claude Opus 5 (1M context) --- docs/agents/ANTIGRAVITY.md | 20 ++++--- docs/agents/HARNESS_PARITY.md | 106 +++++++++++++++++++++++++++------- 2 files changed, 99 insertions(+), 27 deletions(-) diff --git a/docs/agents/ANTIGRAVITY.md b/docs/agents/ANTIGRAVITY.md index 1b2df5dd..e95b71ab 100644 --- a/docs/agents/ANTIGRAVITY.md +++ b/docs/agents/ANTIGRAVITY.md @@ -178,14 +178,20 @@ as every other agent. happens on the subsequent async `stop()`. 4. **`permission_mode` does not confine the harness.** Every mode runs `policy.allow_all()`; coder_eval's write boundary is the sandbox driver, and a - headless eval has no human to approve anything. Declared `approximated` in the - agent's `config_support` — see [Harness Config Parity](HARNESS_PARITY.md). -5. **`allowed_tools` entries with no Antigravity builtin are dropped.** `Skill` is the - expected case (skills are discovered through `skills_paths`, not a tool); the drop - is logged. An allowlist that maps to *nothing* usable falls back to the harness - default with a warning, rather than leaving the model only its turn-ender. + headless eval has no human to approve anything. +5. **`allowed_tools` / `disallowed_tools` are not read.** The harness runs with its + full builtin tool set, so an Antigravity run has tools (web search, subagents, + URL fetch) that the same task file denies on Claude Code and Codex. 6. **`max_turns` counts visible turns.** One `communicate()` is a single SDK turn here, - so the cap counts resolved tool calls instead, enforced on the step loop. + so the cap counts resolved tool calls instead, enforced on the step loop. See + [Run-Limit Parity](HARNESS_PARITY.md). +7. **Shell commands over ~10s are moved to the background.** The localharness has a + 10-second maximum synchronous wait; past it the command becomes a background task + and the model gets a task id, not a result. The turn then usually ends before + `turn_timeout` can fire, and the tool call is force-closed as + `result_status: unknown`. Tasks whose real work is a long `npm install`, build, or + CLI call do not run the same way here as on the other two harnesses. Measured in + [Run-Limit Parity](HARNESS_PARITY.md). ## Running in Docker diff --git a/docs/agents/HARNESS_PARITY.md b/docs/agents/HARNESS_PARITY.md index 70699b57..bb11451d 100644 --- a/docs/agents/HARNESS_PARITY.md +++ b/docs/agents/HARNESS_PARITY.md @@ -11,8 +11,8 @@ This page is the contract for what each run limit means per harness. | Limit | claude-code | codex | antigravity | |---|---|---|---| -| `run_limits.max_turns` | native SDK cap (assistant messages) | visible-turn cap (resolved tool calls) | visible-turn cap (resolved tool calls) | -| `run_limits.turn_timeout` | watchdog, SIGKILL on the CLI subprocess | watchdog + cooperative interrupt | watchdog + cooperative interrupt | +| `run_limits.max_turns` | native SDK cap (agent-loop turns) | visible-turn cap (resolved tool calls) | visible-turn cap (resolved tool calls) | +| `run_limits.turn_timeout` | watchdog, SIGKILL on the CLI subprocess | watchdog + cooperative interrupt | watchdog, but see the 10s note below | | `run_limits.task_timeout` | orchestrator-level, agent-agnostic | orchestrator-level, agent-agnostic | orchestrator-level, agent-agnostic | | `run_limits.stop_early` | cooperative `should_stop` | cooperative `should_stop` | cooperative `should_stop` | @@ -34,29 +34,95 @@ never pulled. The in-flight turn is then cancelled server-side (best effort) so the cap actually stops spend. A run cut this way finalizes cleanly as `max_turns_exhausted` — it is not a crash, and it is not retried. -**claude-code keeps its native SDK cap**, which counts assistant messages instead. -That is a real, honored cap, so it is left alone rather than restated in a -different unit; the same `max_turns: 20` therefore bounds slightly different things -on claude-code than on the other two. Documented rather than papered over. +**claude-code keeps its native SDK cap.** That is a real, honored cap, so it is +left alone rather than reimplemented in a different unit. Its unit is the SDK's own +agent-loop turn, which absorbs an arbitrary number of *parallel* tool calls, so the +same number bounds very different amounts of work: see the measurement below. ### What a capped run looks like -The three signals a capped run leaves behind, on every backend: +The signals a capped run leaves behind, on every backend: -- `final_status` is a completed status, not `agent_crash`. The cap is an ordinary +- `final_status` is a completed status, not an error. The cap is an ordinary end-of-run, so criteria are still checked against whatever the agent produced. -- `max_turns_exhausted: true` on the turn record. -- `visible_turn_count` equals the cap on Codex and Antigravity. On claude-code it - is whatever tool calls fit inside the assistant-message budget, so it is - bounded by the cap rather than equal to it. - -## Timeouts are unchanged by this contract - -`turn_timeout` and `task_timeout` already behaved consistently and are listed here -only so the parity table is complete. A timeout is a *failure* (partial turn -captured, `agent_crash` / timeout status); the turn cap is a *clean stop*. Conflating -them is the mistake this page exists to prevent: a task whose cap fires should not -look like a task whose harness hung. +- `max_turns_exhausted: true` on the task record. +- On Codex and Antigravity, the count of *resolved* tool calls equals the cap. + +## Measured + +Two fixtures under `tasks/run_limits/`, one prompt per limit, run on all three +harnesses (`--type claude-code --backend bedrock` / `--type codex` / +`--type antigravity`). + +**`max_turns_cap.yaml` — 12 sequential file writes requested, `max_turns: 4`:** + +| Harness | resolved tool calls | `max_turns_exhausted` | `final_status` | +|---|---|---|---| +| claude-code | 4 | true | SUCCESS | +| codex | 4 | true | SUCCESS | +| antigravity | 4 | true | SUCCESS | + +All three stop at 4 and finish cleanly, with the first file on disk so the criteria +still grade real work. Before this change, Codex and Antigravity ran the prompt to +completion and wrote all 12. + +**A batching prompt (parallel tool calls encouraged), `max_turns: 2`:** + +| Harness | resolved tool calls | assistant messages | +|---|---|---| +| claude-code | **12** | 14 | +| codex | 2 (+1 in-flight, recorded unresolved) | 1 | +| antigravity | 2 | 1 | + +This is the divergence, quantified: on claude-code a cap of 2 permitted all 12 +writes, because one SDK agent-loop turn carries as many parallel calls as the model +emits. Codex and Antigravity stop at 2. **Hold `max_turns` constant across harnesses +and it is not a constant budget** — if you are A/B-ing across backends and the cap +matters to the result, that is the number to distrust. + +The Codex `+1` is the tool that was already in flight when the cap fired. The cap +stopped the run after 2 completed calls; the third is force-closed and recorded with +`result_status: unknown` rather than being silently dropped, so the trajectory shows +what was interrupted. + +**`turn_timeout.yaml` — `sleep 240` under a 45s watchdog:** + +| Harness | outcome | duration | +|---|---|---| +| claude-code | turn timeout, partial turn captured, `crashed: true` | 45.6s | +| codex | turn timeout, partial turn captured, `crashed: true` | 46.4s | +| antigravity | **no timeout** — turn ended on its own | 19.3s | + +Claude Code and Codex behave identically: the watchdog fires at the deadline, the +partial turn is preserved, and the run ends as an error. Antigravity never reaches +the deadline, for the reason below. + +## Antigravity backgrounds anything over 10 seconds + +The Antigravity localharness has a **10-second maximum synchronous wait** for shell +commands. Past it, the harness moves the command to a background task and hands the +model a task id instead of a result. Measured: `sleep 5` resolves normally in 10.4s +wall-clock; `sleep 240` returns immediately as a background task, the model ends its +turn waiting for a notification that never arrives inside the turn, and the tool call +is force-closed with `result_status: unknown`. + +Consequences worth knowing before reading an Antigravity score: + +- `turn_timeout` is not a meaningful limit for slow commands there. The turn ends + early rather than timing out, so the run looks like a plain failure instead of + a timeout. +- Any task whose real work is a long command (`npm install`, a build, a CLI call + that takes minutes) is not running the same task on Antigravity that it runs on + the other two. + +This is harness behavior, not something coder_eval configures, and it is documented +here rather than worked around. + +## Timeouts are otherwise unchanged by this contract + +A timeout is a *failure* (partial turn captured, error status); the turn cap is a +*clean stop*. Conflating them is the mistake this page exists to prevent: a task +whose cap fires should not look like a task whose harness hung. ## Related From 9060471fa85c61f949b1d122573c6de8e17b5aa7 Mon Sep 17 00:00:00 2001 From: Bai Li Date: Wed, 12 Aug 2026 17:40:59 -0700 Subject: [PATCH 06/15] test(run-limits): tag the parity fixtures CI requires every task YAML to carry at least one tag. Co-Authored-By: Claude Opus 5 (1M context) --- tasks/run_limits/max_turns_cap.yaml | 5 +++++ tasks/run_limits/turn_timeout.yaml | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/tasks/run_limits/max_turns_cap.yaml b/tasks/run_limits/max_turns_cap.yaml index b1cf6d47..201ad370 100644 --- a/tasks/run_limits/max_turns_cap.yaml +++ b/tasks/run_limits/max_turns_cap.yaml @@ -6,6 +6,11 @@ description: >- claude-code / codex / antigravity and compare: the cap must produce a CLEAN stop (max_turns_exhausted, criteria still checked), never a crash. +tags: + - run-limits + - max-turns + - parity + initial_prompt: | Create 12 files in the current directory named step-01.txt through step-12.txt. Each file must contain its own name and nothing else. diff --git a/tasks/run_limits/turn_timeout.yaml b/tasks/run_limits/turn_timeout.yaml index 001ec952..9f9a7863 100644 --- a/tasks/run_limits/turn_timeout.yaml +++ b/tasks/run_limits/turn_timeout.yaml @@ -5,6 +5,11 @@ description: >- contrast with the max_turns fixture is the point: a timeout is a FAILURE with a partial turn captured, while the turn cap is a clean stop. +tags: + - run-limits + - timeout + - parity + initial_prompt: | Run the shell command `sleep 240` and wait for it to finish. When it returns, report its exit code. Do not run it in the background, and do not shorten the From dd02bcc49f4f63390789187bd25ecd77bef2054c Mon Sep 17 00:00:00 2001 From: Bai Li Date: Thu, 13 Aug 2026 11:51:43 -0700 Subject: [PATCH 07/15] docs: use current-generation models in examples MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Examples pinned Sonnet 4-6 / Opus 4-7 / the dated 20250514 and 20250929 ids, so a reader copying one starts on a model two generations back. Refreshed to Sonnet 5 / Opus 5, including the Bedrock inference-profile forms. Untouched on purpose: the `anthropic.claude-sonnet-4-6` values in the llm_judge and simulation sections are the documented code defaults (DEFAULT_JUDGE_MODEL), and experiments/default.yaml's pin is what runs, not an example — bumping either changes results, not docs. Co-Authored-By: Claude Opus 5 (1M context) --- docs/AB_EXPERIMENTS.md | 6 +++--- docs/TASK_DEFINITION_GUIDE.md | 8 ++++---- docs/USER_GUIDE.md | 4 ++-- docs/agents/CLAUDE_CODE.md | 6 +++--- docs/tutorials/04-writing-a-task.md | 2 +- docs/tutorials/05-comparing-models.md | 2 +- experiments/model-comparison.yaml | 4 ++-- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/docs/AB_EXPERIMENTS.md b/docs/AB_EXPERIMENTS.md index 040d9b97..ee2ab84a 100644 --- a/docs/AB_EXPERIMENTS.md +++ b/docs/AB_EXPERIMENTS.md @@ -161,7 +161,7 @@ defaults: agent: type: claude-code permission_mode: bypassPermissions - model: claude-sonnet-4-6 + model: claude-sonnet-5 allowed_tools: ["Skill", "Bash", "Read", "Write", "Edit", "Glob", "Grep"] variants: @@ -205,9 +205,9 @@ description: "Sonnet vs. Opus on the same tasks" variants: - variant_id: sonnet - agent: { model: claude-sonnet-4-6 } + agent: { model: claude-sonnet-5 } - variant_id: opus - agent: { model: claude-opus-4-7 } + agent: { model: claude-opus-5 } ``` ## Recipe: A/B a Prompt diff --git a/docs/TASK_DEFINITION_GUIDE.md b/docs/TASK_DEFINITION_GUIDE.md index 81bac9d6..fc63e0f7 100644 --- a/docs/TASK_DEFINITION_GUIDE.md +++ b/docs/TASK_DEFINITION_GUIDE.md @@ -166,7 +166,7 @@ agent: - "Read" - "Write" - "Bash" - model: "claude-sonnet-4-20250514" # Optional: specific model + model: "claude-sonnet-5" # Optional: specific model sdk_options: # Optional: Claude Code SDK pass-through effort: high # any non-framework-managed ClaudeAgentOptions field ``` @@ -616,11 +616,11 @@ Experiment variants can add `template_sources` that are **appended after** the t variants: - variant_id: baseline agent: - model: "claude-sonnet-4-20250514" + model: "claude-sonnet-5" - variant_id: with-context-hint agent: - model: "claude-sonnet-4-20250514" + model: "claude-sonnet-5" template_sources: - type: "starter_files" files: @@ -1153,7 +1153,7 @@ Spawn a full Claude Code SDK agent as the judge. Unlike `llm_judge` (a single LL max_turns: 5 turn_timeout: 300 agent: # Nested AgentConfig — same shape as task.agent - model: "claude-sonnet-4-6" + model: "claude-sonnet-5" permission_mode: "bypassPermissions" allowed_tools: ["Bash", "Read", "Grep", "Glob"] sdk_options: {effort: low} # Optional SDK pass-through (e.g. effort) diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index e62d4195..f7daba37 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -37,7 +37,7 @@ coder-eval run tasks/hello_date.yaml --stream full # live LLM output | `--preservation-mode` | Sandbox persistence: `NONE` / `MOVE_ON_WRITE` / `DIRECT_WRITE`. Default is driver-derived (docker → `DIRECT_WRITE`, else `MOVE_ON_WRITE`); explicit value always wins. | | `--run-dir` | Custom run directory (default: timestamped in `runs/`) | | `-D path=value` / `--set` | Override any resolved task-config field (`agent`/`run_limits`/`sandbox` roots), e.g. `-D run_limits.max_turns=30 -D agent.permission_mode=plan -D agent.sdk_options.effort=high`. Repeatable; schema-validated. This is the way to set permission mode, turn/timeout limits, token/USD budget caps, tools, plugins, and SDK options. | -| `--model, -m` | Shorthand alias for `-D agent.model=…` (e.g., `claude-sonnet-4-20250514`) | +| `--model, -m` | Shorthand alias for `-D agent.model=…` (e.g., `claude-sonnet-5`) | | `--driver` | Shorthand alias for `-D sandbox.driver=…` (`tempdir` or `docker`) | | `--type, -T` | Override agent type for all tasks (`claude-code`, `codex`, `antigravity`, or a plugin kind). | | `--repeats` | Run each `(task, variant)` N times (≥1); overrides experiment/variant `repeats:`. See [Replicates](#replicates). | @@ -228,7 +228,7 @@ Set these in `.env` (copy from `.env.example`). | `API_BACKEND` | No | API backend: `direct` or `bedrock` (default: `direct`). Overridden by `--backend`. | | `AWS_BEARER_TOKEN_BEDROCK` | For Bedrock | AWS Bedrock bearer token for authentication | | `AWS_REGION` | For Bedrock | AWS region for Bedrock endpoint (e.g., `eu-north-1`) | -| `BEDROCK_MODEL` | No | Cross-region Bedrock model ID (e.g., `eu.anthropic.claude-sonnet-4-5-20250929-v1:0`) | +| `BEDROCK_MODEL` | No | Cross-region Bedrock model ID (e.g., `eu.anthropic.claude-sonnet-5`) | | `BEDROCK_SMALL_MODEL` | No | Cross-region Bedrock small/fast model ID | | `CODEX_API_KEY` / `CODEX_BASE_URL` / `CODEX_MODEL` / `CODEX_API_VERSION` | For Codex | Codex agent auth & endpoint routing — see [Codex Agent Guide](agents/CODEX.md#endpoint-routing). | | `GEMINI_API_KEY` / `ANTIGRAVITY_MODEL` | For Antigravity | Antigravity (Gemini) agent auth & model — see [Antigravity Agent Guide](agents/ANTIGRAVITY.md#setup). | diff --git a/docs/agents/CLAUDE_CODE.md b/docs/agents/CLAUDE_CODE.md index 66670709..4a7e8fae 100644 --- a/docs/agents/CLAUDE_CODE.md +++ b/docs/agents/CLAUDE_CODE.md @@ -56,7 +56,7 @@ direct backend (they call `api.anthropic.com`). | --- | --- | | `AWS_BEARER_TOKEN_BEDROCK` | Bedrock bearer token (required) | | `AWS_REGION` | Bedrock region, e.g. `eu-north-1` (required) | -| `BEDROCK_MODEL` | Cross-region model id, e.g. `eu.anthropic.claude-sonnet-4-5-20250929-v1:0` (required) | +| `BEDROCK_MODEL` | Cross-region model id, e.g. `eu.anthropic.claude-sonnet-5` (required) | | `BEDROCK_SMALL_MODEL` | Small/fast model id (falls back to the main model) | The agent sets `CLAUDE_CODE_USE_BEDROCK=1` and forwards these into the SDK @@ -75,7 +75,7 @@ required; everything else has a default. ```yaml agent: type: claude-code - model: claude-sonnet-4-5-20250929 # optional; omit to use the route default + model: claude-sonnet-5 # optional; omit to use the route default permission_mode: acceptEdits # default | acceptEdits | plan | bypassPermissions allowed_tools: ["Read", "Write", "Bash"] disallowed_tools: ["WebSearch"] @@ -118,7 +118,7 @@ Any of these merge-resolve through `-D` / `--set` (see ```bash coder-eval run tasks/hello_date.yaml \ - -D agent.model=claude-opus-4-8 \ + -D agent.model=claude-opus-5 \ -D agent.permission_mode=plan \ -D agent.sdk_options.effort=high ``` diff --git a/docs/tutorials/04-writing-a-task.md b/docs/tutorials/04-writing-a-task.md index eece8ffa..83d15c4e 100644 --- a/docs/tutorials/04-writing-a-task.md +++ b/docs/tutorials/04-writing-a-task.md @@ -33,7 +33,7 @@ initial_prompt: > agent: type: "claude-code" - model: "claude-sonnet-4-6" + model: "claude-sonnet-5" permission_mode: "acceptEdits" setting_sources: [] # isolate the sandbox from your own CLAUDE.md/settings diff --git a/docs/tutorials/05-comparing-models.md b/docs/tutorials/05-comparing-models.md index a6ff5297..a25170f6 100644 --- a/docs/tutorials/05-comparing-models.md +++ b/docs/tutorials/05-comparing-models.md @@ -39,7 +39,7 @@ variants: model: claude-haiku-4-5-20251001 - variant_id: sonnet agent: - model: claude-sonnet-4-6 + model: claude-sonnet-5 ``` A variant declares only what differs — here just `agent.model`. Everything else diff --git a/experiments/model-comparison.yaml b/experiments/model-comparison.yaml index f0314e9a..f8df1bd1 100644 --- a/experiments/model-comparison.yaml +++ b/experiments/model-comparison.yaml @@ -20,7 +20,7 @@ defaults: variants: - variant_id: sonnet agent: - model: claude-sonnet-4-6 + model: claude-sonnet-5 - variant_id: opus agent: - model: claude-opus-4-6 + model: claude-opus-5 From 3bed0ea9ad26d905595af3f875c40e2c0bb9f071 Mon Sep 17 00:00:00 2001 From: Bai Li Date: Thu, 13 Aug 2026 11:58:35 -0700 Subject: [PATCH 08/15] docs(run-limits): re-measure the antigravity timeout case after the poll loop The measured turn_timeout row was taken before #111 landed and no longer holds: a backgrounded command is now polled for instead of ending the turn on an idle step stream. Re-ran the same fixture on the same box and model, plus an A/B on a job that finishes inside the budget (FAILURE at 19.5s before, SUCCESS at 75.0s after). Restates the residual divergence as what it now is: the terminal signal on a job that outlives the budget, not whether slow work completes at all. Co-Authored-By: Claude Opus 5 (1M context) --- docs/agents/ANTIGRAVITY.md | 10 ++++----- docs/agents/HARNESS_PARITY.md | 41 ++++++++++++++++++++--------------- 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/docs/agents/ANTIGRAVITY.md b/docs/agents/ANTIGRAVITY.md index e95b71ab..b7271c51 100644 --- a/docs/agents/ANTIGRAVITY.md +++ b/docs/agents/ANTIGRAVITY.md @@ -187,11 +187,11 @@ as every other agent. [Run-Limit Parity](HARNESS_PARITY.md). 7. **Shell commands over ~10s are moved to the background.** The localharness has a 10-second maximum synchronous wait; past it the command becomes a background task - and the model gets a task id, not a result. The turn then usually ends before - `turn_timeout` can fire, and the tool call is force-closed as - `result_status: unknown`. Tasks whose real work is a long `npm install`, build, or - CLI call do not run the same way here as on the other two harnesses. Measured in - [Run-Limit Parity](HARNESS_PARITY.md). + and the model gets a task id, not a result. The turn polls for that result instead + of finalizing on an idle step stream, so slow work does complete — but the wait is + bounded by 80% of `turn_timeout`, and a job that outlives it is force-closed as + `result_status: unknown` and graded as an ordinary low score rather than a timeout. + Measured in [Run-Limit Parity](HARNESS_PARITY.md). ## Running in Docker diff --git a/docs/agents/HARNESS_PARITY.md b/docs/agents/HARNESS_PARITY.md index bb11451d..2c54cedb 100644 --- a/docs/agents/HARNESS_PARITY.md +++ b/docs/agents/HARNESS_PARITY.md @@ -12,7 +12,7 @@ This page is the contract for what each run limit means per harness. | Limit | claude-code | codex | antigravity | |---|---|---|---| | `run_limits.max_turns` | native SDK cap (agent-loop turns) | visible-turn cap (resolved tool calls) | visible-turn cap (resolved tool calls) | -| `run_limits.turn_timeout` | watchdog, SIGKILL on the CLI subprocess | watchdog + cooperative interrupt | watchdog, but see the 10s note below | +| `run_limits.turn_timeout` | watchdog, SIGKILL on the CLI subprocess | watchdog + cooperative interrupt | watchdog, plus an earlier internal poll deadline at 80% of it (see below) | | `run_limits.task_timeout` | orchestrator-level, agent-agnostic | orchestrator-level, agent-agnostic | orchestrator-level, agent-agnostic | | `run_limits.stop_early` | cooperative `should_stop` | cooperative `should_stop` | cooperative `should_stop` | @@ -91,32 +91,39 @@ what was interrupted. |---|---|---| | claude-code | turn timeout, partial turn captured, `crashed: true` | 45.6s | | codex | turn timeout, partial turn captured, `crashed: true` | 46.4s | -| antigravity | **no timeout** — turn ended on its own | 19.3s | +| antigravity | poll budget exhausted, tool force-closed, turn graded, `crashed: false` | 40.0s | Claude Code and Codex behave identically: the watchdog fires at the deadline, the -partial turn is preserved, and the run ends as an error. Antigravity never reaches -the deadline, for the reason below. +partial turn is preserved, and the run ends as an error. Antigravity stops earlier +and more gently, for the reason below. ## Antigravity backgrounds anything over 10 seconds The Antigravity localharness has a **10-second maximum synchronous wait** for shell commands. Past it, the harness moves the command to a background task and hands the model a task id instead of a result. Measured: `sleep 5` resolves normally in 10.4s -wall-clock; `sleep 240` returns immediately as a background task, the model ends its -turn waiting for a notification that never arrives inside the turn, and the tool call -is force-closed with `result_status: unknown`. +wall-clock; anything longer comes back immediately as a background task. That is +harness behavior, not something coder_eval configures. -Consequences worth knowing before reading an Antigravity score: +What coder_eval does about it: the turn polls for the backgrounded result rather +than finalizing the moment the step stream goes idle. Measured on a command that +finishes inside the budget (`sleep 60` writing a file, `turn_timeout: 300`), same +task and model on either side: -- `turn_timeout` is not a meaningful limit for slow commands there. The turn ends - early rather than timing out, so the run looks like a plain failure instead of - a timeout. -- Any task whose real work is a long command (`npm install`, a build, a CLI call - that takes minutes) is not running the same task on Antigravity that it runs on - the other two. - -This is harness behavior, not something coder_eval configures, and it is documented -here rather than worked around. +| | outcome | duration | +|---|---|---| +| without the poll loop | FAILURE — file never written, tool left unresolved | 19.5s | +| with it | SUCCESS — file written, exit code reported back | 75.0s | + +The wait is bounded by **80% of `turn_timeout`** (or 120 five-second cycles when the +task sets no timeout), not by `turn_timeout` itself. A job that outlives that bound +is force-closed as unresolved and the turn is graded on everything else, where +Claude Code and Codex instead raise a turn timeout and mark the turn crashed. + +So the residual divergence is the terminal signal, not whether slow work completes: +a long `npm install` or build now runs to completion here the way it does on the +other two, but a command that never finishes reads as an ordinary low score rather +than a timeout. ## Timeouts are otherwise unchanged by this contract From e2b7cd71a679123909edbaef3ebc02b0aeea3bf3 Mon Sep 17 00:00:00 2001 From: Bai Li Date: Thu, 13 Aug 2026 12:02:07 -0700 Subject: [PATCH 09/15] docs(claude): drop the config_support contract from the repo guide The bullet still described `Agent.config_support`, `ConfigFieldSupport`, and `orchestration/config_support.py::validate_config_support` as live machinery after this branch removed all three, pointing an agent reading CLAUDE.md at a module that no longer exists. Rewrites it around what survives: the visible-turn `max_turns` semantic, the claude-code unit divergence, and the current list of known-unfixed field divergences. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 2be0451d..7ca54ef8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -142,7 +142,7 @@ action.yml # Published composite GitHub Action (coder-ev - **Per-criterion aggregation**: Each `BaseCriterion` subclass exposes `aggregate(criterion, per_row_results) -> CriterionAggregate | None`. Default emits `count / mean / median / std / min / max` so every criterion is suite-thresholdable for free. Classification-style criteria return `ClassificationCriterionResult` (subclass of `CriterionResult`) and layer accuracy / P/R/F1 / confusion via the shared `overlay_classification_metrics` utility. `BaseSuccessCriterion.suite_thresholds` gates the suite on those metrics; CLI exits non-zero on any gate failure. - **Sub-agent token accounting**: There is NO separate per-sub-agent field. Every sub-agent generation is captured as a `parent_tool_use_id`-tagged `AssistantMessage` in the turn transcript, so per-sub-agent usage is derived by grouping those messages on that id (the evalboard's `aggregateSubAgentUsage` does exactly this). Claude bubbles its sub-agent's intermediate generations into the parent stream natively, and the **terminal** generation (delivered as the Agent tool result, never streamed) is synthesized into one via `_synthesize_subagent_terminal_message` from `tool_use_result.usage`. Codex reconstructs all child generations from the child rollout (`_recover_subagent_tool_calls`). The turn total already includes sub-agent cost — Claude via the SDK's cumulative `model_usage`; Codex via `_fold_subagent_tokens`, which folds the child messages (their real per-generation tokens) into the parent total. `CommandTelemetry.result_summary` is stored **untruncated** (no 200-char cap) so sub-agent returns are preserved whole. Set `CODER_EVAL_RAW_SDK_LOG=1` to dump every raw SDK event to the task log for inspection. - **Reconciliation message (stream self-reconciles to the turn total)**: The per-message stream consistently under-reports the authoritative turn total — a fixed prompt slice (~512 input tokens on Claude) is billed on no SDK-emitted message, and sub-agent input/cache only partially bubbles up. So `EventCollector.build_turn_record` appends one synthetic `ReconciliationMessage` (`role="reconciliation"`, in the `TranscriptMessage` union) per turn, carrying the per-bucket residual = `token_usage` − Σ(assistant message buckets). The invariant: **summing the four token buckets across `TurnRecord.messages` (assistant + reconciliation) equals `token_usage` exactly**, for both Claude and Codex (Codex's stream is already complete after `_recover_subagent_tool_calls`, so its residual is usually 0 and no entry is emitted). This is what lets the evalboard SUM the message stream as the source of truth instead of reading a separate aggregate ("agent tokens"): `selectTokenTotals` returns the stream sum whenever a reconciliation entry is present, and the timeline renders it as its own row. It is agent-agnostic (booked at the single `EventCollector` seam), carries no cost (cost stays on `token_usage`), and is excluded from generation/turn counts and the cost simulator. The LiteLLM open-weight actual-cost join (`litellm_cost.apply_actual_cost`) deliberately writes cost at the TURN level only (`token_usage.total_cost_usd` = the real OpenRouter bill) plus the per-call `TurnRecord.provider_call_costs` audit record; it does NOT touch the message token buckets, so `EventCollector` stays the single writer and this invariant holds on every backend. The Python `token_usage`/`total_token_usage` aggregate is unchanged and still authoritative for budget/judges/reports. -- **Harness config parity (`Agent.config_support`)**: a shared `BaseAgentConfig` field must mean the same thing on every backend. Where one cannot implement a field it DECLARES the divergence on its agent class as `{field: ConfigFieldSupport(state, reason)}` — `APPROXIMATED` (acted on with a documented divergence; the agent warns at `start()`) or `UNHONORED` (read by nothing; `orchestration/config_support.py::validate_config_support` hard-errors at resolution when the task sets it to a non-default value, in the style of `validate_early_stop`, and it is wired at the same three seats). An empty map asserts full support, so a silently dropped field is a bug rather than a shortcut. Today's divergences: `permission_mode` on Codex and Antigravity (both run unconfined — the sandbox driver is the isolation boundary) and `disallowed_tools` on Codex (forwarded, not SDK-enforced). One field semantic is pinned by this contract: **`run_limits.max_turns` on Codex/Antigravity counts VISIBLE turns** (resolved tool calls, read live off the shared `EventCollector.visible_turn_count`, the same list `TurnRecord.commands` holds) because one `communicate()` is a single SDK turn on both, so a native counter would clamp at 1; claude-code keeps its native SDK cap. The cap is enforced on the same loop boundary as the cooperative early stop and finalizes cleanly as `max_turns_exhausted` (no crash, no retry). Full table + rationale: docs/agents/HARNESS_PARITY.md. +- **Harness run-limit parity**: a shared `BaseAgentConfig` field must mean the same thing on every backend, so a divergence is either fixed or documented — never silent. **`run_limits.max_turns` on Codex/Antigravity counts VISIBLE turns** (resolved tool calls, read live off the shared `EventCollector.visible_turn_count`, the same list `TurnRecord.commands` holds) because one `communicate()` is a single SDK turn on both, so a native counter would clamp at 1; claude-code keeps its native SDK cap, whose unit (an agent-loop turn) absorbs arbitrarily many parallel calls — the same number is NOT the same budget across harnesses. The cap is enforced on the same loop boundary as the cooperative early stop and finalizes cleanly as `max_turns_exhausted` (no crash, no retry); on Antigravity that boundary lives in `_drain()`, so the background-work poll loop honors it too. Known unfixed divergences: `permission_mode` on Codex and Antigravity (both run unconfined — the sandbox driver is the isolation boundary), `disallowed_tools` on Codex (forwarded, not SDK-enforced), `allowed_tools`/`disallowed_tools` on Antigravity (not read at all), and `turn_timeout` on Antigravity (bounded by an earlier internal poll deadline at 80% of it). Full table + rationale: docs/agents/HARNESS_PARITY.md. - **sandbox isolation**: Tasks that don't need MCP servers should set `setting_sources: []` in their `agent:` block to isolate the sandbox from the host project's CLAUDE.md and settings. Without this, the host project's CLAUDE.md (often 20 KB+) is injected into every API call, inflating cache-creation tokens and cost significantly. - **Run-time caps (non-criterion enforcement)**: `TaskDefinition.run_limits` (`RunLimits` model) is the single namespace for all *task-level* run-time caps — `max_turns` / `task_timeout` / `turn_timeout` (structural) and `max_input_tokens` / `max_output_tokens` / `max_total_tokens` / `max_usd` (cumulative budget). Token/USD breaches abort with `FinalStatus.TOKEN_BUDGET_EXCEEDED` or `COST_BUDGET_EXCEEDED` (both `category == "failed"`). Structural caps are set from the CLI via `-D run_limits.max_turns=…` / `-D run_limits.task_timeout=…` / `-D run_limits.turn_timeout=…` (field-merged into `run_limits`); budget caps via `-D run_limits.max_usd=…` etc. or YAML. Layered config uses field-merge — a variant block overrides individual keys without replacing the task's block. The one *per-criterion* cap, `stop_early.decide_within`, deliberately lives on `LiveSuccessCriterion` instead (see below) — the watcher must attribute a decision-step timeout to a specific criterion, which `RunLimits` (task-scoped, criterion-agnostic) cannot express. - **Early stop on criterion (opt-in, per-criterion arming)**: a `stop_early:` block (`StopEarlyPolicy`) on a criterion ends a single-shot run early once the run's **armed** criteria decide the outcome, so a raised `max_turns` isn't wasted on the smoke flavor. The block's PRESENCE is the arming and alone activates the watcher — there is **no run-level master switch**: `run_limits.stop_early: false` is the run-level KILL SWITCH that force-disarms every block (the one-line experiment-variant/`-D` override for an authoritative full run), and `run_limits.stop_early: true` (the removed master arm) is a hard `EarlyStopConfigError` at resolution. The block exists on `LiveSuccessCriterion` only (currently `skill_triggered`, `command_executed` — so arming an unobservable criterion is unrepresentable, a pydantic extra-forbid error). Arming carries one implicit trigger (a native live-fail may fail-stop the run); its keys refine it: `on_pass: stop` (pass-stop the moment the criterion live-passes; default `continue` just latches) and `decide_within: N` (still undecided after N tool-call steps latches an **effective fail**, fed through the same fail-stop rule, reported as `decision_budget_exceeded` — an ordinary weighted fail, NOT a gate-bypassing force-fail; cumulative across retry attempts of the same turn). A trigger whose polarity the instance can't decide (per the abstract, checker-independent `live_decidable_polarities()`, a pure function of the criterion's own fields, paired with the checker's `live_verdict` override by lint rule CE025, a registry-based whole-tree check) is **inert by design** — one dataset-fanned YAML line serves both positive rows (pass/timeout live) and distractor rows (fail live). Verdicts **latch**: once a criterion decides, its `live_verdict` is never polled again. Stop rule is weighted, not strict-boolean: `run_limits.stop_early_gate_threshold` (default `1.0`, reproducing strict-AND behavior exactly) is the minimum weighted score (`Σ weight·score / Σ weight` over the armed subset) required to pass; a fail-stop fires once the armed set's **ceiling** (best case for everything still undecided) can no longer reach the threshold — so a low-weight fail or timeout that can't doom the gate is absorbed and the run continues — and is **deferred while any pass-capable armed criterion is undecided** (a distractor misfire never truncates a positive row's recall signal); a pass-stop fires once the `on_pass: stop` subset's **floor** (worst case) already meets the threshold, and is symmetrically **deferred while any pass-capable armed criterion outside the `on_pass: stop` subset is undecided** (so an early pass never freezes a sibling `on_pass: continue` criterion's signal out of the trajectory). A fail-stop is therefore verdict-preserving; a pass-stop can miss a *later* distractor misfire, so authoritative P/R/F1 comes from a kill-switched (`stop_early: false`) run. Driven by `orchestration/early_stop.py::EarlyStopWatcher` (built when `early_stop_active(task)`: ≥1 armed criterion, kill switch not thrown) through the agent's cooperative `should_stop` seam (tool-call granularity, no SIGKILL); live verdicts only *trigger* the stop — the standard `check_all_async` on the frozen trajectory is authoritative. Gating is **FIRED-ONLY**: a run the watcher actually cut gates on the **armed subset** via the weighted `EvaluationResult.armed_criteria_passed`; a run that completes naturally — armed or not — gates strict-AND via `all_criteria_passed`, so adding a block never changes the verdict of a run it didn't cut. Note the gate keys on the watcher having FIRED (`result.early_stop is not None`), not on confirmed truncation — an agent that ignores `should_stop`, or a stop firing on the final message, still gates armed-only. Every resolution-time guardrail violation is a hard error at resolution (plan *and* run); the one load-time case — a `stop_early:` block on a non-live criterion — is a pydantic schema error at task load, which the run surface reports as a skipped task like any other malformed task. A runtime verdict bug **fails open** to a full run. Surfaces: `EarlyStopInfo` (incl. `gate_threshold` at stop time), report notes/badges, `stopped_early` run.json rows, `EarlyStopped`/`EarlyStopReason` telemetry dims. Worked rationale: docs/TASK_DEFINITION_GUIDE.md § `stop_early`. No blocks anywhere ⇒ behavior byte-for-byte unchanged. From 7e905f8421eda91a267a556ad153ad2274cffc2f Mon Sep 17 00:00:00 2001 From: Bai Li Date: Thu, 13 Aug 2026 12:04:50 -0700 Subject: [PATCH 10/15] docs(run-limits): keep the contract on the page, the measurements in the PR The page mixed a durable per-harness contract with a dated experiment report, and the experiment half went stale inside a day when #111 changed the Antigravity timeout behavior. Durations measured on one box with one model also give an outside reader on coder-eval.com nothing to act on. Keeps every claim the numbers supported, drops the tables, and points at the fixtures under tasks/run_limits/ for anyone who wants to re-measure. Co-Authored-By: Claude Opus 5 (1M context) --- docs/agents/HARNESS_PARITY.md | 95 ++++++++++++----------------------- 1 file changed, 33 insertions(+), 62 deletions(-) diff --git a/docs/agents/HARNESS_PARITY.md b/docs/agents/HARNESS_PARITY.md index 2c54cedb..9bc340d8 100644 --- a/docs/agents/HARNESS_PARITY.md +++ b/docs/agents/HARNESS_PARITY.md @@ -37,7 +37,13 @@ the cap actually stops spend. A run cut this way finalizes cleanly as **claude-code keeps its native SDK cap.** That is a real, honored cap, so it is left alone rather than reimplemented in a different unit. Its unit is the SDK's own agent-loop turn, which absorbs an arbitrary number of *parallel* tool calls, so the -same number bounds very different amounts of work: see the measurement below. +same number bounds very different amounts of work: under a prompt that encourages +batching, a cap of N here permits many more than N tool calls, where it buys exactly +N on the other two. + +**So holding `max_turns` constant across harnesses does not hold the budget +constant.** If you are A/B-ing across backends and the cap is close to binding, that +is the number to distrust. ### What a capped run looks like @@ -47,73 +53,31 @@ The signals a capped run leaves behind, on every backend: end-of-run, so criteria are still checked against whatever the agent produced. - `max_turns_exhausted: true` on the task record. - On Codex and Antigravity, the count of *resolved* tool calls equals the cap. +- A tool call already in flight when the cap fires is force-closed and recorded with + `result_status: unknown` rather than dropped, so the trajectory shows what was + interrupted. That can leave one more *recorded* command than the cap; the resolved + count still matches it. -## Measured - -Two fixtures under `tasks/run_limits/`, one prompt per limit, run on all three -harnesses (`--type claude-code --backend bedrock` / `--type codex` / -`--type antigravity`). - -**`max_turns_cap.yaml` — 12 sequential file writes requested, `max_turns: 4`:** - -| Harness | resolved tool calls | `max_turns_exhausted` | `final_status` | -|---|---|---|---| -| claude-code | 4 | true | SUCCESS | -| codex | 4 | true | SUCCESS | -| antigravity | 4 | true | SUCCESS | - -All three stop at 4 and finish cleanly, with the first file on disk so the criteria -still grade real work. Before this change, Codex and Antigravity ran the prompt to -completion and wrote all 12. - -**A batching prompt (parallel tool calls encouraged), `max_turns: 2`:** +## What a timeout looks like -| Harness | resolved tool calls | assistant messages | -|---|---|---| -| claude-code | **12** | 14 | -| codex | 2 (+1 in-flight, recorded unresolved) | 1 | -| antigravity | 2 | 1 | +On Claude Code and Codex a `turn_timeout` breach is a *failure*: the watchdog fires +at the deadline, the partial turn is preserved on `pending_turn`, and the turn is +marked `crashed`. -This is the divergence, quantified: on claude-code a cap of 2 permitted all 12 -writes, because one SDK agent-loop turn carries as many parallel calls as the model -emits. Codex and Antigravity stop at 2. **Hold `max_turns` constant across harnesses -and it is not a constant budget** — if you are A/B-ing across backends and the cap -matters to the result, that is the number to distrust. - -The Codex `+1` is the tool that was already in flight when the cap fired. The cap -stopped the run after 2 completed calls; the third is force-closed and recorded with -`result_status: unknown` rather than being silently dropped, so the trajectory shows -what was interrupted. - -**`turn_timeout.yaml` — `sleep 240` under a 45s watchdog:** - -| Harness | outcome | duration | -|---|---|---| -| claude-code | turn timeout, partial turn captured, `crashed: true` | 45.6s | -| codex | turn timeout, partial turn captured, `crashed: true` | 46.4s | -| antigravity | poll budget exhausted, tool force-closed, turn graded, `crashed: false` | 40.0s | - -Claude Code and Codex behave identically: the watchdog fires at the deadline, the -partial turn is preserved, and the run ends as an error. Antigravity stops earlier -and more gently, for the reason below. +Antigravity stops earlier and more gently, for the reason in the next section. ## Antigravity backgrounds anything over 10 seconds The Antigravity localharness has a **10-second maximum synchronous wait** for shell commands. Past it, the harness moves the command to a background task and hands the -model a task id instead of a result. Measured: `sleep 5` resolves normally in 10.4s -wall-clock; anything longer comes back immediately as a background task. That is -harness behavior, not something coder_eval configures. +model a task id instead of a result. That is harness behavior, not something +coder_eval configures. What coder_eval does about it: the turn polls for the backgrounded result rather -than finalizing the moment the step stream goes idle. Measured on a command that -finishes inside the budget (`sleep 60` writing a file, `turn_timeout: 300`), same -task and model on either side: - -| | outcome | duration | -|---|---|---| -| without the poll loop | FAILURE — file never written, tool left unresolved | 19.5s | -| with it | SUCCESS — file written, exit code reported back | 75.0s | +than finalizing the moment the step stream goes idle, so slow work does finish and +its real exit code reaches the model. Without that poll, a command over the 10s +boundary left the tool call unresolved and the turn was graded on work that had not +happened yet. The wait is bounded by **80% of `turn_timeout`** (or 120 five-second cycles when the task sets no timeout), not by `turn_timeout` itself. A job that outlives that bound @@ -121,16 +85,23 @@ is force-closed as unresolved and the turn is graded on everything else, where Claude Code and Codex instead raise a turn timeout and mark the turn crashed. So the residual divergence is the terminal signal, not whether slow work completes: -a long `npm install` or build now runs to completion here the way it does on the -other two, but a command that never finishes reads as an ordinary low score rather -than a timeout. +a long `npm install` or build runs to completion here the way it does on the other +two, but a command that never finishes reads as an ordinary low score rather than a +timeout. -## Timeouts are otherwise unchanged by this contract +## Timeouts are not turn caps A timeout is a *failure* (partial turn captured, error status); the turn cap is a *clean stop*. Conflating them is the mistake this page exists to prevent: a task whose cap fires should not look like a task whose harness hung. +## Reproducing + +`tasks/run_limits/` holds one fixture per limit: `max_turns_cap.yaml` asks for more +sequential work than its cap allows, and `turn_timeout.yaml` runs a command that +outlives its watchdog. Run either with `--type claude-code` / `--type codex` / +`--type antigravity` to check a backend against the contract above. + ## Related - [Claude Code](CLAUDE_CODE.md) · [Codex](CODEX.md) · [Antigravity](ANTIGRAVITY.md) From 4e5444ed4bc5fd3e72b86bcddab77910b6b7e35c Mon Sep 17 00:00:00 2001 From: Bai Li Date: Thu, 13 Aug 2026 12:12:54 -0700 Subject: [PATCH 11/15] test(antigravity): clear the CodeQL findings on the fake SDK helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three alerts, all in the merged test helper: - Two `lambda **kw: SimpleNamespace(**kw)` wrappers that are just `SimpleNamespace`. One is dropped outright — `CapabilitiesConfig` was scaffolding for the tool-mapping work this PR no longer carries. - A bare `await task_a` flagged as having no effect. Now bounded by `asyncio.wait_for`, so a regression that re-serializes concurrent starts fails the test instead of hanging the suite. Also refreshes two comments left describing the dropped scope. Co-Authored-By: Claude Opus 5 (1M context) --- tests/test_antigravity_agent.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/tests/test_antigravity_agent.py b/tests/test_antigravity_agent.py index 618e5205..b432ff8d 100644 --- a/tests/test_antigravity_agent.py +++ b/tests/test_antigravity_agent.py @@ -554,8 +554,7 @@ def _install_fake_sdk(monkeypatch, sdk_agent_cls) -> None: ag.types = SimpleNamespace( ThinkingLevel=lambda level: level, GeminiAPIEndpoint=type("GeminiAPIEndpoint", (), {}), - GeminiModelOptions=lambda **kw: SimpleNamespace(**kw), - CapabilitiesConfig=lambda **kw: SimpleNamespace(**kw), + GeminiModelOptions=SimpleNamespace, ) hooks = ModuleType("google.antigravity.hooks") hooks.policy = SimpleNamespace( @@ -1316,7 +1315,9 @@ async def __aexit__(self, *exc): task_a = asyncio.create_task(a.start(str(tmp_path), env_path_prepend=["/a/mocks"])) await a_entered.wait() await b.start(str(tmp_path), env_path_prepend=["/b/mocks"]) - await task_a + # Bounded: if a start ever serializes behind the other again, fail the test + # rather than hang the suite waiting for a task that will never finish. + await asyncio.wait_for(task_a, timeout=10) envs = [c.env for c in configs] assert envs == [ @@ -1376,12 +1377,12 @@ async def __aexit__(self, *exc): assert configs[0].env is None -# --- allowed_tools / disallowed_tools / permission_mode ----------------------------- +# --- permission_mode ---------------------------------------------------------------- # -# These fields were ignored entirely before (policies were hardcoded to allow_all and -# no CapabilitiesConfig was built), so two harnesses reading the same task file ran -# different tasks. CapabilitiesConfig validates against the BuiltinTools enum, so the -# Claude→Antigravity name mapping has to be exact and unmappables must be dropped. +# The local harness has one mode: policies are hardcoded to allow_all, so no +# permission_mode confines it. These pin that as intended behavior rather than an +# oversight — the write boundary is the sandbox driver, and a headless eval has +# nobody to approve anything. def _agent(**cfg) -> AntigravityAgent: @@ -1390,7 +1391,7 @@ def _agent(**cfg) -> AntigravityAgent: @pytest.mark.parametrize("mode", ["default", "acceptEdits", "plan", "bypassPermissions"]) async def test_permission_mode_never_confines_the_harness(monkeypatch, tmp_path, mode: str): - """permission_mode is declared unhonored: every mode stays fully autonomous. + """permission_mode is not honored here: every mode stays fully autonomous. coder_eval's write boundary is the driver (docker container / ephemeral tempdir), not the agent — same deliberate stance as Codex. A mode that silently switched the From 7142fdfcb188f2e4aee82dbab06c977f50c366d6 Mon Sep 17 00:00:00 2001 From: Bai Li Date: Thu, 13 Aug 2026 12:29:40 -0700 Subject: [PATCH 12/15] test(antigravity): pin the SDK half of the env seam contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every other env test stubs LocalAgentConfig, so together they prove only that coder_eval builds the right kwarg. A google-antigravity bump that dropped or renamed `env` would leave all of them green while mock CLIs silently stopped shadowing and the agent called the real tool: the exact silent-wrong-mode the seam exists to prevent. Asserts against the real class instead — the field exists, round-trips, and defaults to None rather than {} (the connection reads `is not None` to decide whether to build a merged env at all). Verified end-to-end on the eval VM alongside this: a record_cli shim for `uip` reached the agent's PATH and returned its sentinel, with the invocation logged. Co-Authored-By: Claude Opus 5 (1M context) --- tests/test_antigravity_agent.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/test_antigravity_agent.py b/tests/test_antigravity_agent.py index b432ff8d..32276b78 100644 --- a/tests/test_antigravity_agent.py +++ b/tests/test_antigravity_agent.py @@ -1254,6 +1254,27 @@ async def test_harness_env_never_mutates_process_env(monkeypatch): assert os.environ["PATH"] == "/parent/bin" +def test_installed_sdk_still_exposes_the_env_seam(): + """Pin the SDK-side half of the contract the rest of this section fakes. + + Every other env test stubs ``LocalAgentConfig``, so they prove only that we + build the right kwarg. If a future ``google-antigravity`` bump dropped or + renamed ``env``, all of them would still pass while mock CLIs silently + stopped shadowing and the agent called the real tool instead — the exact + silent-wrong-mode this seam exists to prevent. So assert against the real + class: the field exists and round-trips. + """ + config_mod = pytest.importorskip("google.antigravity.connections.local.local_connection_config") + + assert "env" in config_mod.LocalAgentConfig.model_fields + cfg = config_mod.LocalAgentConfig(env={"PATH": "/sandbox/mocks:/usr/bin"}) + assert cfg.env == {"PATH": "/sandbox/mocks:/usr/bin"} + # Omitted must stay None, not {} — the connection reads `is not None` to decide + # whether to build a merged env at all, so {} would spawn with a rebuilt env + # for every task instead of plain inheritance. + assert config_mod.LocalAgentConfig().env is None + + async def test_harness_env_resolves_path_key_case_insensitively(monkeypatch): """A non-uppercase PATH key (e.g. Windows 'Path') is reused, so the merge overrides it. From c91a5a7f46a6ef76abf2bb266503f40c442e235b Mon Sep 17 00:00:00 2001 From: Bai Li Date: Fri, 14 Aug 2026 10:12:03 -0700 Subject: [PATCH 13/15] fix(codex): fold sub-agent tokens on a turn-cap stop `_recover_subagent_tool_calls` was skipped whenever the notification pump broke on purpose, which the turn cap had just joined. Recovery is the only writer of the `parent_tool_use_id`-tagged messages `_fold_subagent_tokens` sums, and Codex bills sub-agents on separate threads the parent total never sees, so skipping it silently dropped the child threads' tokens and cost from every capped run that spawned one. Early stop is opt-in and rare; `max_turns` is a standard run limit, so this would have under-reported spend routinely. The cap now recovers as usual. Its recovered child calls land beyond the cap's count, the same way the force-closed orphan already does: the cap bounds what the model was allowed to do, not what the record may explain. A cooperative stop keeps its pre-existing skip. Co-Authored-By: Claude Opus 5 (1M context) --- src/coder_eval/agents/codex_agent.py | 21 ++++++-- tests/test_codex_agent.py | 75 ++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 5 deletions(-) diff --git a/src/coder_eval/agents/codex_agent.py b/src/coder_eval/agents/codex_agent.py index 56c7e52c..cbc6bfdc 100644 --- a/src/coder_eval/agents/codex_agent.py +++ b/src/coder_eval/agents/codex_agent.py @@ -1508,11 +1508,22 @@ async def _run_turn_with_streaming( # and nest them under the spawning Agent call. The parent stream never # carries the child's commands (Limited persistence drops them), but its # rollout always persists the raw function_call/local_shell_call items. - # Skipped when the pump was cut short (cooperative stop or turn cap): - # children may have no rollout yet and the run is already decided — - # recovery adds nothing the armed gate uses, and its child tool calls - # would push the visible-turn count past the cap that just fired. - if state.spawned_children and not state.ended_cleanly: + # + # Runs on a turn-cap stop. Recovery is also what carries the children's + # TOKENS: it is the only writer of the ``parent_tool_use_id``-tagged + # messages that ``_fold_subagent_tokens`` sums into the turn total, so + # skipping it drops the child threads' spend from the run's cost entirely + # (Codex bills children on separate threads the parent total never sees). + # A cap is a routine ending, not an exceptional one, so paying ~2s of + # rollout polling beats under-reporting spend on every capped run that + # spawned a sub-agent. The recovered child calls land in the trajectory + # beyond the cap's count, the same way the force-closed orphan does; + # the cap bounds what the model was allowed to DO, not what the record is + # allowed to explain. + # + # Still skipped on a cooperative stop: an armed gate has already decided + # the run, children may have no rollout yet, and that path predates the cap. + if state.spawned_children and not state.stopped_early_hit: await self._recover_subagent_tool_calls( state.spawned_children, state.collab_results, diff --git a/tests/test_codex_agent.py b/tests/test_codex_agent.py index d60c7bab..9fd53dcd 100644 --- a/tests/test_codex_agent.py +++ b/tests/test_codex_agent.py @@ -2050,3 +2050,78 @@ async def test_cooperative_stop_outranks_the_cap(self): record = await agent.communicate("go", max_turns=1, should_stop=lambda: True) assert record.max_turns_exhausted is False + + async def test_capped_turn_still_folds_sub_agent_tokens(self, monkeypatch, tmp_path): + """A capped turn must not lose the child threads' spend. + + Codex bills sub-agents on separate threads the parent total never sees, and + ``_recover_subagent_tool_calls`` is the ONLY writer of the + ``parent_tool_use_id`` messages ``_fold_subagent_tokens`` sums. So skipping + recovery because the pump was cut short does not just drop telemetry rows — + it silently removes the child's tokens and cost from the run. The cap is a + routine ending, so recovery still runs; only a cooperative stop skips it. + """ + monkeypatch.setenv("CODEX_HOME", str(tmp_path)) + child = "019e0000-eeee-7000-8000-000000000005" + _write_child_rollout( + tmp_path, + child, + [ + {"type": "function_call", "name": "exec_command", "call_id": "c_py", "arguments": '{"cmd":"x"}'}, + {"type": "function_call_output", "call_id": "c_py", "output": "5050"}, + _token_count_event(inp=23859, cached=15104, out=96, tot_in=23859, tot_cached=15104, tot_out=96), + ], + ) + spawn = _collab_call("spawnAgent", call_id="call_spawn", model="gpt-5.5", child_thread=child) + wait = _collab_call("wait", call_id="call_wait", result="5050", child_thread=child) + # The cap fires on the wait, before turn/completed is ever dispatched. + notifications = [ + _item_notification("item/started", spawn), + _item_notification("item/completed", spawn), + _item_notification("item/started", wait), + _item_notification("item/completed", wait), + *self._cmd_notifications(3), + ] + agent = _started_agent(parse_agent_config(type=AgentKind.CODEX), notifications) + + record = await agent.communicate("delegate it", max_turns=2) + + assert record.max_turns_exhausted is True + # The child's inner shell command was recovered despite the cap... + assert [c for c in record.commands if c.tool_name == "Bash"] + # ...and its generation nests under the spawn, carrying its own tokens... + nested = [m for m in record.messages if getattr(m, "parent_tool_use_id", None) == "call_spawn"] + assert sum(m.output_tokens for m in nested) == 96 + # ...which is what makes the turn total (and therefore the run cost) + # include the sub-agent instead of silently under-reporting it. + assert record.token_usage is not None + assert record.token_usage.output_tokens >= 96 + assert record.token_usage.cache_read_input_tokens >= 15104 + + async def test_cooperative_stop_still_skips_sub_agent_recovery(self, monkeypatch, tmp_path): + """The early-stop path keeps its pre-existing skip: an armed gate already decided.""" + monkeypatch.setenv("CODEX_HOME", str(tmp_path)) + child = "019e0000-ffff-7000-8000-000000000006" + _write_child_rollout( + tmp_path, + child, + [ + {"type": "function_call", "name": "exec_command", "call_id": "c_py", "arguments": '{"cmd":"x"}'}, + {"type": "function_call_output", "call_id": "c_py", "output": "5050"}, + _token_count_event(inp=23859, cached=15104, out=96, tot_in=23859, tot_cached=15104, tot_out=96), + ], + ) + spawn = _collab_call("spawnAgent", call_id="call_spawn", model="gpt-5.5", child_thread=child) + wait = _collab_call("wait", call_id="call_wait", result="5050", child_thread=child) + notifications = [ + _item_notification("item/started", spawn), + _item_notification("item/completed", spawn), + _item_notification("item/started", wait), + _item_notification("item/completed", wait), + _turn_completed(), + ] + agent = _started_agent(parse_agent_config(type=AgentKind.CODEX), notifications) + + record = await agent.communicate("delegate it", should_stop=lambda: True) + + assert not [c for c in record.commands if c.tool_name == "Bash"] From 376f95baf3dda0d11eadb5dd53b5b5a253946646 Mon Sep 17 00:00:00 2001 From: Bai Li Date: Fri, 14 Aug 2026 10:12:13 -0700 Subject: [PATCH 14/15] docs(parity): state the real final_status of a capped run "`final_status` is a completed status" was wrong: a capped run that satisfies its criteria finishes as SUCCESS, and one that does not finishes as MAX_TURNS_EXHAUSTED, whose reporting category is `failed` (orchestrator.py's status assignment, `_STATUS_CATEGORIES`). On the page that exists to be the per-harness contract, that is the distinction anyone diffing a nightly needs. Also names the second way a capped Codex run records more commands than the cap: recovered sub-agent calls, alongside the force-closed orphan. Co-Authored-By: Claude Opus 5 (1M context) --- docs/agents/HARNESS_PARITY.md | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/docs/agents/HARNESS_PARITY.md b/docs/agents/HARNESS_PARITY.md index 9bc340d8..296092f6 100644 --- a/docs/agents/HARNESS_PARITY.md +++ b/docs/agents/HARNESS_PARITY.md @@ -49,14 +49,21 @@ is the number to distrust. The signals a capped run leaves behind, on every backend: -- `final_status` is a completed status, not an error. The cap is an ordinary - end-of-run, so criteria are still checked against whatever the agent produced. +- Criteria are still checked against whatever the agent produced, because the cap is + an ordinary end-of-run rather than an error. So a capped run that nonetheless + satisfies its criteria finishes as `SUCCESS`; one that does not finishes as + `MAX_TURNS_EXHAUSTED` (reporting category `failed`, icon `M`). Never `ERROR`, + and never retried. - `max_turns_exhausted: true` on the task record. -- On Codex and Antigravity, the count of *resolved* tool calls equals the cap. -- A tool call already in flight when the cap fires is force-closed and recorded with - `result_status: unknown` rather than dropped, so the trajectory shows what was - interrupted. That can leave one more *recorded* command than the cap; the resolved - count still matches it. +- On Codex and Antigravity, the count of *resolved* tool calls the model itself + issued equals the cap. Two things can add a further *recorded* command, and + neither means the cap leaked: + - A tool call already in flight when the cap fires is force-closed and recorded + with `result_status: unknown` rather than dropped, so the trajectory shows what + was interrupted. + - On Codex, a sub-agent's inner tool calls are recovered from its rollout after + the pump stops, so the child's work and its tokens still reach the record. The + cap bounds what the model was allowed to do, not what the record may explain. ## What a timeout looks like From a2dd57afe661a1bf27f5e2be61a00b1b4a298b8d Mon Sep 17 00:00:00 2001 From: Bai Li Date: Fri, 14 Aug 2026 10:12:13 -0700 Subject: [PATCH 15/15] test(run-limits): make the max_turns fixture assert the cap bound MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fixture's only criterion was `file_exists: step-01.txt`, which passes whether or not the cap fired — including on a harness that ignores max_turns entirely, the exact bug this fixture exists to catch. Adds the other half: step-12.txt must NOT exist. The prompt now chains each file's contents onto the previous one, so a step cannot be written without reading its predecessor and no amount of batching inside a single agent-loop turn reaches step 12 under a cap of 4. A run that produced the last file therefore ran uncapped, on any harness. Co-Authored-By: Claude Opus 5 (1M context) --- tasks/run_limits/max_turns_cap.yaml | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tasks/run_limits/max_turns_cap.yaml b/tasks/run_limits/max_turns_cap.yaml index 201ad370..4d236db1 100644 --- a/tasks/run_limits/max_turns_cap.yaml +++ b/tasks/run_limits/max_turns_cap.yaml @@ -13,7 +13,9 @@ tags: initial_prompt: | Create 12 files in the current directory named step-01.txt through step-12.txt. - Each file must contain its own name and nothing else. + step-01.txt must contain just its own name. Every later file must contain the + contents of the PREVIOUS file, then its own name on a new line — so you have to + read step-N before you can write step-N+1. Create them ONE AT A TIME. Run a separate shell command for each file. Do not use a loop, do not combine several files into one command, and do not batch @@ -36,3 +38,14 @@ success_criteria: path: "step-01.txt" description: "First file was created before the cap fired" weight: 1.0 + + # And this is the half that actually tests the cap. Without it the fixture + # passes on a harness that ignores max_turns entirely — the exact bug it exists + # to catch — because step-01.txt gets written either way. The chained contents + # in the prompt make each step depend on reading the one before it, so no amount + # of batching within a single agent-loop turn can reach step 12 inside a cap of + # 4; a run that produced the last file therefore ran uncapped. + - type: run_command + command: "test ! -f step-12.txt" + description: "The cap bound the run: the agent never reached the last file" + weight: 1.0