diff --git a/CLAUDE.md b/CLAUDE.md index e7fc33e9..7ca54ef8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -142,6 +142,7 @@ action.yml # Published composite GitHub Action (coder-ev - **Per-criterion aggregation**: Each `BaseCriterion` subclass exposes `aggregate(criterion, per_row_results) -> CriterionAggregate | None`. Default emits `count / mean / median / std / min / max` so every criterion is suite-thresholdable for free. Classification-style criteria return `ClassificationCriterionResult` (subclass of `CriterionResult`) and layer accuracy / P/R/F1 / confusion via the shared `overlay_classification_metrics` utility. `BaseSuccessCriterion.suite_thresholds` gates the suite on those metrics; CLI exits non-zero on any gate failure. - **Sub-agent token accounting**: There is NO separate per-sub-agent field. Every sub-agent generation is captured as a `parent_tool_use_id`-tagged `AssistantMessage` in the turn transcript, so per-sub-agent usage is derived by grouping those messages on that id (the evalboard's `aggregateSubAgentUsage` does exactly this). Claude bubbles its sub-agent's intermediate generations into the parent stream natively, and the **terminal** generation (delivered as the Agent tool result, never streamed) is synthesized into one via `_synthesize_subagent_terminal_message` from `tool_use_result.usage`. Codex reconstructs all child generations from the child rollout (`_recover_subagent_tool_calls`). The turn total already includes sub-agent cost — Claude via the SDK's cumulative `model_usage`; Codex via `_fold_subagent_tokens`, which folds the child messages (their real per-generation tokens) into the parent total. `CommandTelemetry.result_summary` is stored **untruncated** (no 200-char cap) so sub-agent returns are preserved whole. Set `CODER_EVAL_RAW_SDK_LOG=1` to dump every raw SDK event to the task log for inspection. - **Reconciliation message (stream self-reconciles to the turn total)**: The per-message stream consistently under-reports the authoritative turn total — a fixed prompt slice (~512 input tokens on Claude) is billed on no SDK-emitted message, and sub-agent input/cache only partially bubbles up. So `EventCollector.build_turn_record` appends one synthetic `ReconciliationMessage` (`role="reconciliation"`, in the `TranscriptMessage` union) per turn, carrying the per-bucket residual = `token_usage` − Σ(assistant message buckets). The invariant: **summing the four token buckets across `TurnRecord.messages` (assistant + reconciliation) equals `token_usage` exactly**, for both Claude and Codex (Codex's stream is already complete after `_recover_subagent_tool_calls`, so its residual is usually 0 and no entry is emitted). This is what lets the evalboard SUM the message stream as the source of truth instead of reading a separate aggregate ("agent tokens"): `selectTokenTotals` returns the stream sum whenever a reconciliation entry is present, and the timeline renders it as its own row. It is agent-agnostic (booked at the single `EventCollector` seam), carries no cost (cost stays on `token_usage`), and is excluded from generation/turn counts and the cost simulator. The LiteLLM open-weight actual-cost join (`litellm_cost.apply_actual_cost`) deliberately writes cost at the TURN level only (`token_usage.total_cost_usd` = the real OpenRouter bill) plus the per-call `TurnRecord.provider_call_costs` audit record; it does NOT touch the message token buckets, so `EventCollector` stays the single writer and this invariant holds on every backend. The Python `token_usage`/`total_token_usage` aggregate is unchanged and still authoritative for budget/judges/reports. +- **Harness run-limit parity**: a shared `BaseAgentConfig` field must mean the same thing on every backend, so a divergence is either fixed or documented — never silent. **`run_limits.max_turns` on Codex/Antigravity counts VISIBLE turns** (resolved tool calls, read live off the shared `EventCollector.visible_turn_count`, the same list `TurnRecord.commands` holds) because one `communicate()` is a single SDK turn on both, so a native counter would clamp at 1; claude-code keeps its native SDK cap, whose unit (an agent-loop turn) absorbs arbitrarily many parallel calls — the same number is NOT the same budget across harnesses. 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. diff --git a/README.md b/README.md index 11cd4420..104e3f7d 100644 --- a/README.md +++ b/README.md @@ -209,6 +209,7 @@ alone. | [Claude Code](docs/agents/CLAUDE_CODE.md) | Configuring and running the default Claude Code agent | | [Codex](docs/agents/CODEX.md) | Running the OpenAI Codex agent | | [Antigravity (Gemini)](docs/agents/ANTIGRAVITY.md) | Running the Google Antigravity / Gemini agent | +| [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/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 e96ac153..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) @@ -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/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/ANTIGRAVITY.md b/docs/agents/ANTIGRAVITY.md index 522dc8bf..b7271c51 100644 --- a/docs/agents/ANTIGRAVITY.md +++ b/docs/agents/ANTIGRAVITY.md @@ -176,10 +176,22 @@ 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. +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. 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 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/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/agents/CODEX.md b/docs/agents/CODEX.md index b020e76b..c817ecdc 100644 --- a/docs/agents/CODEX.md +++ b/docs/agents/CODEX.md @@ -216,8 +216,11 @@ 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` + `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 | +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. diff --git a/docs/agents/HARNESS_PARITY.md b/docs/agents/HARNESS_PARITY.md new file mode 100644 index 00000000..296092f6 --- /dev/null +++ b/docs/agents/HARNESS_PARITY.md @@ -0,0 +1,115 @@ +# Run-Limit Parity + +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. + +This page is the contract for what each run limit means per harness. + +## The table + +| 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, 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` | + +## `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. + +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.** 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: 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 + +The signals a capped run leaves behind, on every backend: + +- 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 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 + +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`. + +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. 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, 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 +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 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 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) +- [Task Definition Guide](../TASK_DEFINITION_GUIDE.md) — the full `run_limits` schema diff --git a/docs/index.md b/docs/index.md index 6c9ad50d..90df7069 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 | +| [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 ea733060..db96ce1f 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 +- [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/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 diff --git a/mkdocs.yml b/mkdocs.yml index 8fcb00c7..5b42c6cb 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 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" @@ -111,6 +112,7 @@ nav: - Claude Code: agents/CLAUDE_CODE.md - Codex: agents/CODEX.md - Antigravity (Gemini): agents/ANTIGRAVITY.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/agents/antigravity_agent.py b/src/coder_eval/agents/antigravity_agent.py index 79718ebe..72de13f3 100644 --- a/src/coder_eval/agents/antigravity_agent.py +++ b/src/coder_eval/agents/antigravity_agent.py @@ -23,7 +23,7 @@ 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 @@ -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 / @@ -277,7 +256,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). @@ -352,6 +332,24 @@ def _resolve_workspaces(self, skills_paths: list[str]) -> list[str]: """ return [str(self.working_directory), *skills_paths] + 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, @@ -370,10 +368,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``. @@ -411,12 +408,23 @@ 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()], 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(), ) # Attach the configured thinking level (reasoning effort) onto every # resolved model's Gemini endpoint. The SDK validates the model list in @@ -429,47 +437,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 _drain( self, conversation: Any, @@ -513,6 +488,17 @@ async def _drain( state.stopped_early_hit = True self._log.debug("Cooperative stop requested; ending step loop at this boundary") break + # 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", + state.max_turns, + ) + break return except RuntimeError: if attempt == _RECEIVE_STEPS_REENTRY_RETRIES - 1: @@ -539,6 +525,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. @@ -571,6 +564,7 @@ async def communicate( iteration=self._iteration, model=model, turn_start_time=turn_start_time, + max_turns=max_turns, ) try: @@ -618,6 +612,7 @@ def _on_turn_timeout() -> None: # as fast as today. while ( not state.stopped_early_hit + and not state.max_turns_hit and not state.timeout_hit and state.has_orphaned_tool_call() and ( @@ -639,9 +634,18 @@ def _on_turn_timeout() -> None: if should_stop is not None and should_stop(): state.stopped_early_hit = True break + # A re-drain honors the turn cap the same way the initial one + # does (the check lives in _drain), so a poll cycle can also + # be the cycle that reaches it; the loop head above then + # stops polling instead of waiting out the background work. await self._drain(conversation, state, should_stop) - if state.has_orphaned_tool_call() and not state.stopped_early_hit and not state.timeout_hit: + if ( + state.has_orphaned_tool_call() + and not state.stopped_early_hit + and not state.max_turns_hit + and not state.timeout_hit + ): # Exited via this loop's own bound (poll_deadline or the # cycle cap), not an external stop/timeout -- the tool call # is force-closed as unresolved in finalize() below and the @@ -654,7 +658,7 @@ def _on_turn_timeout() -> None: msg = "Poll budget exhausted (%s, poll_count=%d) with a tool call still ACTIVE." self._log.warning(msg, bound, poll_count) - if state.stopped_early_hit: + 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. Single check # point covers a stop from either the initial drain or any @@ -668,13 +672,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 @@ -691,10 +696,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 @@ -702,8 +708,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() @@ -782,6 +796,7 @@ def __init__( iteration: int, model: str, turn_start_time: float, + max_turns: int | None = None, ) -> None: self._agent = agent self.emit = emit @@ -793,8 +808,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() @@ -819,6 +836,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) @@ -1055,6 +1092,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/codex_agent.py b/src/coder_eval/agents/codex_agent.py index c6e4b2d3..cbc6bfdc 100644 --- a/src/coder_eval/agents/codex_agent.py +++ b/src/coder_eval/agents/codex_agent.py @@ -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, ) ) @@ -744,7 +769,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 +820,7 @@ async def communicate( user_input=user_input, iteration=self._iteration, turn_start_time=turn_start_time, + max_turns=max_turns, ) try: @@ -836,12 +866,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 +902,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 +916,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() @@ -1437,6 +1477,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 +1496,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,8 +1508,21 @@ 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. + # + # 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, diff --git a/src/coder_eval/models/__init__.py b/src/coder_eval/models/__init__.py index e1fe21ba..ca029798 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, NORMALIZED_CRITERION_ALIASES, REMOVED_CRITERION_TYPES, CriteriaCheckTiming, @@ -344,6 +345,7 @@ # Tasks "TaskDefinition", "DEFAULT_SIMULATION_STOP_TOKEN", + "DEFAULT_SIMULATOR_MODEL", "NORMALIZED_CRITERION_ALIASES", "REMOVED_CRITERION_TYPES", "CriteriaCheckTiming", 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 9614d675..dcf1d5fe 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.""" @@ -81,9 +92,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/experiment.py b/src/coder_eval/orchestration/experiment.py index f75c585e..2c75bad9 100644 --- a/src/coder_eval/orchestration/experiment.py +++ b/src/coder_eval/orchestration/experiment.py @@ -695,7 +695,8 @@ def resolve_all_tasks( ) ) # Early-stop arming errors are a deliberate hard stop: they always - # propagate (never demoted to skipped) so a misarmed run fails loudly. + # 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 diff --git a/src/coder_eval/orchestrator.py b/src/coder_eval/orchestrator.py index 8640dd7f..83f67da3 100644 --- a/src/coder_eval/orchestrator.py +++ b/src/coder_eval/orchestrator.py @@ -1641,6 +1641,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 +1652,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 +1772,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 +1788,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 +1863,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 +2044,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 +2080,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..a11b4440 100644 --- a/src/coder_eval/reports.py +++ b/src/coder_eval/reports.py @@ -469,11 +469,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/tasks/run_limits/max_turns_cap.yaml b/tasks/run_limits/max_turns_cap.yaml new file mode 100644 index 00000000..4d236db1 --- /dev/null +++ b/tasks/run_limits/max_turns_cap.yaml @@ -0,0 +1,51 @@ +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. + +tags: + - run-limits + - max-turns + - parity + +initial_prompt: | + Create 12 files in the current directory named step-01.txt through step-12.txt. + 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 + 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 + + # 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 diff --git a/tasks/run_limits/turn_timeout.yaml b/tasks/run_limits/turn_timeout.yaml new file mode 100644 index 00000000..9f9a7863 --- /dev/null +++ b/tasks/run_limits/turn_timeout.yaml @@ -0,0 +1,29 @@ +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. + +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 + 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 diff --git a/tests/test_antigravity_agent.py b/tests/test_antigravity_agent.py index e7a15df7..32276b78 100644 --- a/tests/test_antigravity_agent.py +++ b/tests/test_antigravity_agent.py @@ -6,8 +6,10 @@ import asyncio import os +import sys from collections.abc import Callable -from types import SimpleNamespace +from types import ModuleType, SimpleNamespace +from typing import Any import pytest @@ -540,6 +542,32 @@ async def test_communicate_requires_started_agent(): await agent.communicate("x") +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=SimpleNamespace, + ) + 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) + + # --- background-task poll loop (wait_for_wakeup is a dead stub on the Local ------ # harness; see antigravity_agent.py's communicate() comment + the plan for the # full evidence trail. The model leaves a tool call open (never DONE) when it @@ -1189,191 +1217,341 @@ async def cancel(self): assert agent.pending_turn is None -# --- env_path_prepend / harness-spawn PATH shadowing ------------------------------ +# --- 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_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")) + + assert agent._harness_env() is None -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_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() - async with agent._harness_spawn_guard(): - assert os.environ["PATH"] == "/parent/bin" 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.""" +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. + + 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"]) + # 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 == [ + {"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 test_start_stores_env_path_prepend(monkeypatch, tmp_path): - """start(env_path_prepend=[...]) records the dirs on the instance for the spawn guard. + async def __aenter__(self): + return self - 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. - """ - import sys - from types import ModuleType, SimpleNamespace + async def __aexit__(self, *exc): + return False - monkeypatch.setenv("PATH", "/parent/bin") - captured: dict[str, str] = {} + _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): - 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) - - 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) + configs: list[Any] = [] + _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"]) + await agent.start(str(tmp_path)) - 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" + assert configs[0].env is None + + +# --- permission_mode ---------------------------------------------------------------- +# +# 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: + return AntigravityAgent(parse_agent_config(type="antigravity", **cfg)) + + +@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 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 + policy list would make an A/B across harnesses incomparable. + """ + 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(permission_mode=mode).start(str(tmp_path)) + + assert [p.kind for p in configs[0].policies] == ["allow_all"] + + +# --- 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 + + +async def test_cap_reached_on_a_poll_redrain_stops_polling(monkeypatch): + """The cap and the background-poll loop share a boundary. + + A turn that backgrounds work drains, polls, and re-drains — so a poll cycle can + be the cycle that reaches the cap. The re-drain honors it (the check lives in + ``_drain``, which both paths call), and the loop must then stop polling rather + than keep waiting out the background job on a run that is already over. + """ + from coder_eval.agents import antigravity_agent + + monkeypatch.setattr(antigravity_agent.asyncio, "sleep", _no_sleep) + + bg = _tc("run_command", "bg1", {"command_line": "sleep 999"}) + batch1 = [_step("TOOL_CALL", "ACTIVE", target="TARGET_ENVIRONMENT", tool_calls=[bg])] + # The re-drain kicks off a SECOND background job, then closes the first and runs + # one more call — reaching the cap (2) with an orphan still ACTIVE. Both exit + # conditions are live at once, and the cap has to win: otherwise the loop keeps + # polling out a background job on a run that is already over. + batch2 = [ + _step( + "TOOL_CALL", + "ACTIVE", + target="TARGET_ENVIRONMENT", + tool_calls=[_tc("run_command", "bg2", {"command_line": "sleep 999"})], + ), + _step( + "TOOL_CALL", + "DONE", + target="TARGET_ENVIRONMENT", + tool_calls=[ + _tc("run_command", "bg1", {"command_line": "sleep 999", "exit_code": 0, "combined_output": "x"}) + ], + ), + *_tool_steps(1), + ] + batch3 = _tool_steps(2) # must never be drained + agent = _agent_with_steps([batch1, batch2, batch3]) + conv = agent._sdk_agent.conversation + + record = await agent.communicate("go", max_turns=2) + + assert record.max_turns_exhausted is True + # The cap counts RESOLVED calls. The still-open bg2 is force-closed and recorded + # as unresolved rather than dropped, so the trajectory shows what was interrupted. + resolved = [c for c in record.commands if c.result_status != "unknown"] + assert [c.tool_id for c in resolved] == ["bg1", "t0"] + assert [c.tool_id for c in record.commands if c.result_status == "unknown"] == ["bg2"] + assert conv.receive_steps_call_count == 2 # initial drain + one poll re-drain, then stop + assert conv.cancel_call_count == 1 diff --git a/tests/test_codex_agent.py b/tests/test_codex_agent.py index 95ee117f..9fd53dcd 100644 --- a/tests/test_codex_agent.py +++ b/tests/test_codex_agent.py @@ -1972,3 +1972,156 @@ 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 + + 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"] 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_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