diff --git a/CLAUDE.md b/CLAUDE.md index 8213fefe..ebe99962 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -160,7 +160,7 @@ action.yml # Published composite GitHub Action (coder-ev - **Harness run-limit parity**: a shared `BaseAgentConfig` field must mean the same thing on every backend, so a divergence is either fixed or documented — never silent. **`run_limits.max_turns` on Codex/Antigravity counts VISIBLE turns** (resolved tool calls, read live off the shared `EventCollector.visible_turn_count`, the same list `TurnRecord.commands` holds) because one `communicate()` is a single SDK turn on both, so a native counter would clamp at 1; claude-code keeps its native SDK cap, whose unit (an agent-loop turn) absorbs arbitrarily many parallel calls — the same number is NOT the same budget across harnesses. OpenCode likewise keeps a native unit — the CLI streams a real multi-step loop per `communicate()` (`step_start`/`step_finish`), so `max_turns: N` allows N complete steps and cuts cleanly when step N+1 begins. **Pi** is the same shape — the CLI (`pi -p --mode json`) streams a real multi-step loop per `communicate()` (`turn_start`/`turn_end`), so `max_turns: N` counts native `turn_start` steps; Pi retries transient/provider errors INTERNALLY (`agent_end.willRetry`), and the reducer finalizes once at `agent_settled`/EOF (not the first `agent_end`), folding the retry cycles into one turn. 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, Antigravity, and Pi (all run unconfined — the sandbox driver is the isolation boundary), `disallowed_tools` on Codex (forwarded, not SDK-enforced), `allowed_tools`/`disallowed_tools` on Antigravity (not read at all), `turn_timeout` on Antigravity (bounded by an earlier internal poll deadline at 80% of it), `allowed_tools`/`disallowed_tools`/`system_prompt` on OpenCode (no CLI knob; warned at `start()`, not enforced), `allowed_tools`/`disallowed_tools` on Pi (its built-in tool names are lowercase — `bash`/`read`/… — and cannot map to the Claude-namespaced config default, so forwarding them would strip the agent of ALL tools; warned+ignored like the three agents above) — Pi DOES enforce `system_prompt` (`--append-system-prompt`, a small win over OpenCode) and DOES honor `plugins` for skills (each resolved skills dir → a `--skill ` arg via the shared `_plugin_skill_dirs` resolver, recorded as `pi_skill_paths`, so it CAN run activation suites) but does NOT read `system_prompt_file`, and **`agent.plugins[].path` depth** — claude-code REQUIRES a plugin root holding `skills/` and silently loads NOTHING from a bare skills directory, while Codex and Antigravity scan both depths and accept either. That is the costly direction: the wrong depth produces no error, every positive row of an activation suite scores 0, and the suite reports recall 0.0, which reads exactly like a skill that never triggers. Held to the plugin-root shape (for `SKILL_SOURCE_PATH` only) by lint rule CE045. `plugins` on OpenCode is **honored for skills**: each local plugin root is mapped to the `skills` dir its `.claude-plugin/plugin.json` declares (default `/skills`, never the root itself — `skills.paths` is scanned recursively and a plugin root can hold a self-referential symlink) and injected via `OPENCODE_CONFIG_CONTENT`, which `--pure` does not suppress; a plugin's agents/hooks/commands/MCP servers are still dropped. Full table + rationale: docs/agents/HARNESS_PARITY.md. - **sandbox isolation**: Tasks that don't need MCP servers should set `setting_sources: []` in their `agent:` block to isolate the sandbox from the host project's CLAUDE.md and settings. Without this, the host project's CLAUDE.md (often 20 KB+) is injected into every API call, inflating cache-creation tokens and cost significantly. - **Execute vs. run (the grading switch)**: `coder-eval execute` is `coder-eval run` with grading removed — the agent runs and the full trajectory is captured, but no criterion is checked, `weighted_score` is `None` (never `0.0`, which would be indistinguishable from "graded and scored zero"), and the row finalizes as **`FinalStatus.NOT_GRADED`**, whose `category` is a **fourth** bucket, `"ungraded"`. Ungraded rows leave BOTH sides of every rate: `RunSummary.pass_rate` / `error_share` and `VariantAggregate.pass_rate` divide by `tasks_graded` (`tasks_run - tasks_not_graded`), and `tasks_not_graded` is part of the sum-to-`tasks_run` invariant, not a `tasks_failed` sub-counter. **Only SUCCESS/FAILURE collapse into it** — `ERROR`, `TIMEOUT`, `BUILD_FAILED`, `MAX_TURNS_EXHAUSTED` and the budget stops are facts about the *run*, not about grading, and still apply (so `execute` still exits non-zero on a crash). The switch is `BatchRunConfig.grade` → `Orchestrator(grade=...)` → the **four** grading call sites (single-shot, evaluate-only, the simulation dialog check, and post-failure diagnostics); it crosses the docker boundary in `context.json` (defaulting to `True` in-container, so a host predating `execute` keeps grading). It is **deliberately not a task-config field** — no 5-layer merge, no `-D` path — because a task YAML must never declare itself ungraded; only the invoking command decides. `run` and `execute` share one body (`run_command.run_pipeline`) and differ solely in that flag, so there is no third code path. Three things are refused rather than degraded: `--junit-xml` (a report of verdicts, and there are none — though `reports_junit` still emits `` for an ungraded row it encounters), `--allow-host-grading` (it decides how an ungraded row is GRADED, and `execute` grades nothing), and simulation tasks (their turn-continuation logic reads criteria results, so an ungraded dialog would silently change its own stopping behavior). `stop_early:` blocks are inert under `execute` for the same reason the kill switch exists: the full trajectory is the deliverable. Motivating consumer: an external harness (Harbor / Terminal-Bench 2.0) that builds its own container, calls coder-eval as the agent, and grades with its own tests. -- **Detached grading (`evaluate` over a run dir) + `Sandbox.adopt`**: `coder-eval evaluate` takes two shapes, told apart by a **pure** resolver (`cli/evaluate_target.py`) on one probe — a target holding `task.json` is a run directory. Run-dir mode rebuilds the task from the run's own `task_config.resolved`, **not** by re-loading the YAML: `resolved` is post-merge, so variant overrides / `-D` / dataset expansion are already baked in and re-loading the source would silently grade a *different* task (fallback to `source_file` only when `resolved` no longer validates, and loudly). It seeds the fresh result from the prior one via `Orchestrator(prior_result=...)` → `_seed_from_prior_result`, which carries the trajectory (every derived figure — tokens, cost, `command_stats`, `model_used` — recomputes from `iterations`), `iteration_count`, execution facts, and **`early_stop` — load-bearing, because gate selection is FIRED-ONLY**: dropping it re-grades a truncated trajectory under the full-run strict-AND gate and can flip the verdict. Carrying it is only half the fix — **both** grading paths select the gate through the single `Orchestrator._select_gate()`; the evaluate-only branch a detached grade actually takes originally called `all_criteria_passed` inline, so the seeded field was written and never read. `tests/test_seed_from_prior_result.py` partitions every `EvaluationResult` field as CARRIED or RECOMPUTED and fails closed on a new one, and asserts the two `_select_gate()` call sites. A prior status that `FinalStatus.is_execution_fact` (TIMEOUT / ERROR / BUILD_FAILED / the budget stops) is **preserved**, never overwritten: grading may only move `NOT_GRADED` to SUCCESS/FAILURE, since it neither repeated nor observed the agent phase. **`MAX_TURNS_EXHAUSTED` is deliberately NOT one of them — anywhere**. `_EXECUTION_FACT_STATUSES` maps it to `False`, and the table and the chain that reads it must agree: it shipped as `True` while `_terminal_status`'s own docstring argued the opposite, and the disagreement pinned a re-graded max-turns row at MAX_TURNS_EXHAUSTED *while holding `weighted_score` 1.000* and exit 1 — a combination `run` can never produce for the same trajectory. Under `execute`: `_terminal_status` puts the `grade=False` arm ABOVE it, because on the graded path it is subordinate to the verdict — `run` returns SUCCESS for a max-turns trajectory whose criteria pass — so it is not knowable without grading. Consuming it first made it terminal AND permanent (the `is_execution_fact` arm then pinned it), so identical agent output scored SUCCESS/1.0 under `run` and MAX_TURNS_EXHAUSTED under `execute` → `evaluate`. The fact survives on `result.max_turns_exhausted`, which `_seed_from_prior_result` carries, so the detached grade walks the identical chain. The CLI must also branch on WHERE a status came from, not on its value: a preserved TIMEOUT exited 0 under "All criteria passed" (a CI wrapper reading the exit code went green on a row run.json counts as failed), and a preserved ERROR printed the ORIGINAL run's crash message as though grading had crashed, claimed the row was "left ungraded" (false — the restored record still read ERROR), and discarded a verdict just computed at 1.000. Grader-host `environment_info` is preserved as flat `graded_by_*` scalars rather than overwriting the run's (flat, not a nested sub-dict: `environment_info` is rendered as a flat map by the HTML report and typed as one by the evalboard, so a nested capture prints as a Python dict repr). The route recorder follows the same rule: on a detached grade it writes `graded_by_api_routing` / `graded_by_eval_routing` and leaves the run's `api_routing` alone — writing in place contradicted the "prior wins" contract and left a self-contradictory record (a direct route named beside the run's stale `aws_region`/`bedrock_model`). Two other parity fixes: `command_base_path` is now persisted into `environment_info` by `_sync_sandbox_command_path_with_agent` and restored in the evaluate-only branch (closing the PATH gap that method's docstring already named), and `_join_litellm_actual_cost` **skips** when `prior_result` is set (its join keys on a per-Orchestrator nonce the prior turns never carried, so it would clobber already-correct costs). The verdict is written back into the run's `task.json`, with the pre-grade record kept as `task.execute.json` — that in-place write is what makes plain `coder-eval aggregate ` rebuild a graded `run.json` with **zero** new code. **`Sandbox.adopt(workspace)`** is the grade-in-place primitive: it reuses `setup`'s adoption half but skips every *materializing* step (`_setup_template`, `_generate_cli_recorders`, venv/package installs, the destructive `$HOME` remediation), running only non-mutating derivation (mock-dir `+x`, venv *discovery*, plugin-tools pin); `_cleanup_on_exit` stays False so an adopted tree is never moved or deleted, and `Sandbox.was_adopted` is set — the Orchestrator reads it to SKIP the `pre_run` hook (`run()` calls it unconditionally with `cwd = sandbox_dir`, and several in-tree tasks stage fixtures there with `cp -a /app/[!.]* "$PWD/"`, which would overwrite the agent's deliverables before the criteria read them) and to KEEP `sandbox_path` in the `PreservationMode.NONE` cleanup arm (an adopted tree survives cleanup, so the path is not stale). `pre_run`'s recorded results are carried from the prior run instead. **`post_run` is the opposite case and moved phases**: it is defined as running after the verdict and may mutate the workspace the criteria read (`rm -rf node_modules` is the archetype), so running it under `execute` inverted its own contract and broke round-trip equivalence — the criteria had not read the tree yet, so `execute` + `evaluate` graded a workspace `post_run` had already modified and could return a different verdict than a single `run` for the identical trajectory (the in-tree tasks all escaped it only because their `post_run` touches nothing a criterion reads). `execute` now DEFERS it; whichever command grades runs it, exactly once — `_skip_post_run` skips on `grade=False`, and skips again when the prior row already recorded results, since nothing declares these commands idempotent. That makes it a capability of the in-place path, so `embedded_commands` scans it OUTSIDE `include_setup_phase` (which is False in place) — minus `_operator_baseline_post_run()`, the grading host's own `experiments/default.yaml` contribution, which every task carries and the record therefore did not choose; without that exemption the refusal fired on 100% of run directories, and a refusal that always fires is waved through. In-place is **more correct**, not merely faster: `_setup_template` filters the copy through `_should_ignore_template_file`, which drops `node_modules` / `dist` / `build` / `.venv` / `.git`, so on the copy path a criterion like `test -f dist/bundle.js` fails as a *copying artifact* rather than as a verdict (verified: 0.00 "does not exist" on copy vs 1.00 in place). Defaults: in-place for a run dir, copy for a bare work dir (criteria can mutate it and it is the user's own tree); `--in-place`/`--copy` override. `adopt` hard-errors on `driver: docker` (a container workspace is unreachable from the host), and grading a `driver: docker` task is REFUSED outright unless `--allow-host-grading` is passed — the earlier behavior silently rewrote the driver to `tempdir`, which ran a container task's criteria against a host filesystem lacking `/verifier` and the image's toolchain (FAILURE for a trajectory `run` scored 1.0, plus `rm -rf /verifier` unsandboxed on the grading machine) and neutralized `adopt`'s own docker guard; an opted-in row is stamped `graded_on_host` so it is never silently comparable with a container-graded one (lint rule CE051). A re-grade refuses on a `reference_digest` mismatch — the digest is persisted into `environment_info` at staging time by `_stage_reference` (it shipped once as a read with no writer anywhere, so the guard was dead code; then it shipped with a writer whose value was **discarded before it reached disk**, because `_setup` REBOUND the whole `environment_info` dict from `get_version_info()` a hundred lines later, which CE054 cannot see — a write existed in `src/`, it was just dead. `_setup` now `update()`s that dict rather than rebinding it, and `tests/test_detached_grading_boundaries.py` asserts the key survives a real end-to-end run, not just that `_staged_digest` works in isolation), and `verify_reference_unchanged` now takes the task file it resolves against and RAISES on a vanished or unresolvable reference instead of returning silently. That comparison digests a STAGED copy of the source, not the raw tree: the recorded digest is taken over the staged copy, which `stage_reference_dir` filters through `REFERENCE_COPY_IGNORE` (`.git`), so digesting the source directly compared two differently-filtered trees and reported a permanent false mismatch for any reference that is a git checkout — exactly the case the ignore list exists for. **The recorded config is untrusted input**: `evaluate ` rebuilds the task from a shareable artifact, so a rebuilt config that carries shell (`run_command` criteria, `agent_judge`, `llm_judge`, `uipath_eval`, and — only on the `--copy` path, since the in-place path skips them — `pre_run`/`post_run` **and the sandbox's own provisioning**) is REFUSED unless `--allow-recorded-commands` is passed. The provisioning half was the one the gate originally missed, and the worst: `grading_sandbox_config` carries the recorded `sandbox` block through untouched and the `--copy` branch calls `Sandbox.setup`, which reaches `uv pip install ` / `npm install` / `git clone ` — arbitrary code at install time — so a shared run dir whose criteria were all `file_exists` sailed through a scan that walked only `success_criteria`. `git clone` now passes `--` before the URL (argv position 2, so a value beginning with `-` was parsed as an option). `Sandbox.resolve_files` is containment-checked for the same reason: criterion paths were the one task-authored path skipping `_resolve_within_sandbox`, and `Path(root) / '/etc/passwd'` is `/etc/passwd`. An escaping LITERAL now raises `CheckerMisuseError` rather than resolving to `[]`: returning no match books an eval-CONFIG error as an agent failure — a gating 0.0 reading "file does not exist" for a file that plainly does exist and that no agent behaviour could place inside the sandbox (CE039's exact distinction). `tasks/byod_smoke_test.yaml` was broken that way for several commits, checking `/opt/byod_marker` baked into the BYOD image, with only a task-log warning to show for it; it now asserts on the container with `run_command: test -f …`, which is what a claim about the IMAGE rather than about the agent's workspace should look like. The guard keys on the escaping path EXISTING, so a merely-absent absolute path stays an ordinary failing verdict, and the GLOB branch still warns-and-drops, since filtering some matches out of a search is its normal behaviour. A warning is not a control: it prints as the command is already being prepared. Passing the task file explicitly (`evaluate `) also bypasses it, since that config came from the operator. The workspace fallback `artifacts / prior.task_id` is containment-checked like its `sandbox_path` sibling (`task_id` is an unvalidated string, and `"../../.."` joins to a real directory `is_dir()` confirms), `_sanitize_restored_path` drops relative entries (they resolve against the grader's cwd) and anything inside the run dir rather than only the workspace, and `write_text_atomic` opens its temp file `O_EXCL|O_NOFOLLOW` — a pre-planted `task.json.tmp` symlink otherwise bypassed the write-back's destination symlink guard entirely. The record must also describe the task as AUTHORED, not as executed: `run_task_internal_command` rewrites `driver: docker` -> `tempdir` before building the in-container Orchestrator (the one legitimate rewrite — we are already inside the container the driver asked for), and recording that rewrite made a docker run's own `task.json` claim `driver: tempdir`. Since `grading_sandbox_config` reads the driver back OUT of the record, `evaluate ` on a container row skipped BOTH the `--allow-host-grading` refusal and the `graded_on_host` stamp and graded a container task against the host filesystem silently — the exact outcome that gate exists to prevent. `Orchestrator(recorded_task=...)` is the seam: what is recorded, as distinct from what is run. NOTE the Typer command is a thin wrapper over `run_evaluation(...)`, which has real Python defaults — calling a Typer command function in-process hands unspecified options an `OptionInfo` sentinel, which silently made `in_place=None` truthy. +- **Detached grading (`evaluate` over a run dir) + `Sandbox.adopt`**: `coder-eval evaluate` takes two shapes, told apart by a **pure** resolver (`cli/evaluate_target.py`) on one probe — a target holding `task.json` is a run directory. Run-dir mode rebuilds the task from the run's own `task_config.resolved`, **not** by re-loading the YAML: `resolved` is post-merge, so variant overrides / `-D` / dataset expansion are already baked in and re-loading the source would silently grade a *different* task (fallback to `source_file` only when `resolved` no longer validates, and loudly). It seeds the fresh result from the prior one via `Orchestrator(prior_result=...)` → `_seed_from_prior_result`, which carries the trajectory (every derived figure — tokens, cost, `command_stats`, `model_used` — recomputes from `iterations`), `iteration_count`, execution facts, and **`early_stop` — load-bearing, because gate selection is FIRED-ONLY**: dropping it re-grades a truncated trajectory under the full-run strict-AND gate and can flip the verdict. Carrying it is only half the fix — **both** grading paths select the gate through the single `Orchestrator._select_gate()`; the evaluate-only branch a detached grade actually takes originally called `all_criteria_passed` inline, so the seeded field was written and never read. `tests/test_seed_from_prior_result.py` partitions every `EvaluationResult` field as CARRIED or RECOMPUTED and fails closed on a new one, and asserts the two `_select_gate()` call sites. A prior status that `FinalStatus.is_execution_fact` (TIMEOUT / ERROR / BUILD_FAILED / the budget stops) is **preserved**, never overwritten: grading may only move `NOT_GRADED` to SUCCESS/FAILURE, since it neither repeated nor observed the agent phase. **`MAX_TURNS_EXHAUSTED` is deliberately NOT one of them — anywhere**. `_EXECUTION_FACT_STATUSES` maps it to `False`, and the table and the chain that reads it must agree: it shipped as `True` while `_terminal_status`'s own docstring argued the opposite, and the disagreement pinned a re-graded max-turns row at MAX_TURNS_EXHAUSTED *while holding `weighted_score` 1.000* and exit 1 — a combination `run` can never produce for the same trajectory. Under `execute`: `_terminal_status` puts the `grade=False` arm ABOVE it, because on the graded path it is subordinate to the verdict — `run` returns SUCCESS for a max-turns trajectory whose criteria pass — so it is not knowable without grading. Consuming it first made it terminal AND permanent (the `is_execution_fact` arm then pinned it), so identical agent output scored SUCCESS/1.0 under `run` and MAX_TURNS_EXHAUSTED under `execute` → `evaluate`. The fact survives on `result.max_turns_exhausted`, which `_seed_from_prior_result` carries, so the detached grade walks the identical chain. The CLI must also branch on WHERE a status came from, not on its value: a preserved TIMEOUT exited 0 under "All criteria passed" (a CI wrapper reading the exit code went green on a row run.json counts as failed), and a preserved ERROR printed the ORIGINAL run's crash message as though grading had crashed, claimed the row was "left ungraded" (false — the restored record still read ERROR), and discarded a verdict just computed at 1.000. Grader-host `environment_info` is preserved as flat `graded_by_*` scalars rather than overwriting the run's (flat, not a nested sub-dict: `environment_info` is rendered as a flat map by the HTML report and typed as one by the evalboard, so a nested capture prints as a Python dict repr). The route recorder follows the same rule: on a detached grade it writes `graded_by_api_routing` / `graded_by_eval_routing` and leaves the run's `api_routing` alone — writing in place contradicted the "prior wins" contract and left a self-contradictory record (a direct route named beside the run's stale `aws_region`/`bedrock_model`). Two other parity fixes: `command_base_path` is now persisted into `environment_info` by `_sync_sandbox_command_path_with_agent` and restored in the evaluate-only branch (closing the PATH gap that method's docstring already named), and `_join_litellm_actual_cost` **skips** when `prior_result` is set (its join keys on a per-Orchestrator nonce the prior turns never carried, so it would clobber already-correct costs). The verdict is written back into the run's `task.json`, with the pre-grade record kept as `task.execute.json` — that in-place write is what makes plain `coder-eval aggregate ` rebuild a graded `run.json` with **zero** new code. **`Sandbox.adopt(workspace)`** is the grade-in-place primitive: it reuses `setup`'s adoption half but skips every *materializing* step (`_setup_template`, `_generate_cli_recorders`, venv/package installs, the destructive `$HOME` remediation), running only non-mutating derivation (mock-dir `+x`, venv *discovery*, plugin-tools pin); `_cleanup_on_exit` stays False so an adopted tree is never moved or deleted, and `Sandbox.was_adopted` is set — the Orchestrator reads it to SKIP the `pre_run` hook (`run()` calls it unconditionally with `cwd = sandbox_dir`, and several in-tree tasks stage fixtures there with `cp -a /app/[!.]* "$PWD/"`, which would overwrite the agent's deliverables before the criteria read them) and to KEEP `sandbox_path` in the `PreservationMode.NONE` cleanup arm (an adopted tree survives cleanup, so the path is not stale). `pre_run`'s recorded results are carried from the prior run instead. **`post_run` is the opposite case and moved phases**: it is defined as running after the verdict and may mutate the workspace the criteria read (`rm -rf node_modules` is the archetype), so running it under `execute` inverted its own contract and broke round-trip equivalence — the criteria had not read the tree yet, so `execute` + `evaluate` graded a workspace `post_run` had already modified and could return a different verdict than a single `run` for the identical trajectory (the in-tree tasks all escaped it only because their `post_run` touches nothing a criterion reads). `execute` now DEFERS it; whichever command grades runs it, exactly once — `_skip_post_run` skips on `grade=False`, and skips again when the prior row already recorded results, since nothing declares these commands idempotent. That makes it a capability of the in-place path, so `embedded_commands` scans it OUTSIDE `include_setup_phase` (which is False in place) — minus `_operator_baseline_post_run()`, the grading host's own `experiments/default.yaml` contribution, which every task carries and the record therefore did not choose; without that exemption the refusal fired on 100% of run directories, and a refusal that always fires is waved through. In-place is **more correct**, not merely faster: `_setup_template` filters the copy through `_should_ignore_template_file`, which drops `node_modules` / `dist` / `build` / `.venv` / `.git`, so on the copy path a criterion like `test -f dist/bundle.js` fails as a *copying artifact* rather than as a verdict (verified: 0.00 "does not exist" on copy vs 1.00 in place). Defaults: in-place for a run dir, copy for a bare work dir (criteria can mutate it and it is the user's own tree); `--in-place`/`--copy` override. `adopt` hard-errors on `driver: docker` (a container workspace is unreachable from the host), and grading a `driver: docker` task is DISPATCHED INTO A CONTAINER of the task's own image (`_should_grade_in_container` -> `_grade_in_container` -> `DockerRunner(prior_result=, grade_workspace=)`), because that is the only place its criteria mean what they meant during the run: `tasks/byod_smoke_test.yaml` asserts `test -f /opt/byod_marker`, baked into its image, and the IDENTICAL row scores SUCCESS 1.000 in a container and FAILURE 0.000 on the host — the host answering a question nobody asked. The grading container gets TWO mounts and their separation is the design: the grading pass's own fresh `run_dir` at `CONTAINER_OUTPUT_DIR` (whose `task.json` the host then folds back into the row, preserving `task.execute.json` exactly as on the host path) and the executed workspace at `CONTAINER_GRADE_WORKSPACE`, read-WRITE and NOT a copy, adopted rather than written over. The container half reuses the same `regrade_in_place` (`run_task_internal_command._grade_recorded_run`, driven by `context.json`'s `regrade` flag plus a staged `prior.json`) rather than restating it. A container-graded row carries NO `graded_on_host` stamp, so it is indistinguishable from a `run` row, which is the parity that makes the split honest. `--allow-host-grading` survives as the ESCAPE HATCH (no docker on this machine; criteria known to be host-portable) and still stamps. **The dispatch is itself inside the trust gate**: the record names the image, and a container of it runs with the default credential allowlist (`ANTHROPIC_API_KEY`, `UIPATH_ACCESS_TOKEN`, `AWS_BEARER_TOKEN_BEDROCK` ...) forwarded in and a copy of `~/.claude` mounted — a strictly WIDER capability than the `run_command` strings the gate already refuses, and it shipped reachable with no flags because `embedded_commands` walked only `success_criteria` and `post_run`. That is the same blind spot the function's own docstring already described for `--copy` provisioning ("a shared run directory whose criteria were all `file_exists` sailed through"), one layer up, so `include_container_dispatch` scans it on the in-place path exactly as `post_run` is — rendering the whole dispatch as ONE command string (the prompt joins with `"; "` and counts `len(commands)`, so an argv fragment appended as its own entry reported one `docker build` as four shell commands), and naming every HOST PATH it exposes: the task DIRECTORY copied from the recorded `source_file`'s parent (a record naming `~/.ssh/config` copies all of `~/.ssh` in), every auto-mounted `agent.plugins[].path` / `TemplateDirSource.path` / `system_prompt_file`, and the writable `~/.claude` copy. Disclosing only `sandbox.docker.*` asked the operator to consent to a strict subset of what happens. Which families the gate discloses is ONE parameter (`grade_in_place`, resolved by `_gate_scope_for_grade`), not two: it shipped beside an `include_setup_phase` every caller passed as its exact complement, and a future caller setting one and forgetting the other would silently drop half of a SECURITY gate. Three further properties are load-bearing and were not free: the grading container gets a **scratch** run dir, never the caller's — `run --resume` passes the executed row's OWN directory, where `_parse_result_or_raise` (which keys on `task.json` existing and discards `returncode`) read a dead container's stale pre-grade record back as a successful grade, and where `docker.log` was truncated; the recorded `source_file` is the HOST's path (`Orchestrator.recorded_task_file`, the path twin of `recorded_task`), because a container run recorded `/work/task_dir/task.yaml`, which exists on no host, so the dispatch guard's `task_file is None` test passed and `_prepare_task_dir_mount`'s `if not source.is_dir(): return` then mounted NOTHING — every `$TASK_DIR` criterion silently resolving against the wrong tree; and `_assert_regrade_honored` refuses a returned row whose `started_at` moved, because an image predating this change ignores the unknown `regrade` key and RUNS THE AGENT, which the host would otherwise fold back as the recorded row's verdict (the exact sibling of `_assert_grade_honored`, one release later). The grading container is a SECOND, fresh container: only the workspace crosses and `pre_run` is not re-run, so a criterion depending on out-of-workspace state (`tasks/samples/skillsbench/3d-scan-calc` symlinks `/root/mass_report.json` in `pre_run` and its verifier asserts that path) scores 0.000 for a trajectory `run` scores 1.000 — warned at dispatch AND stamped onto the row as `environment_info.graded_without_pre_run`, since re-running `pre_run` would trade it for the deliverable-clobbering bug `_skip_pre_run_for_adopted` exists to prevent. The stamp is the load-bearing half: `stamp_host_grading`'s own docstring already says why ("a console warning does not travel with `task.json` into `run.json`, the reports or the evalboard"), and 3 of the 10 in-tree docker tasks match the pattern, reachable with NO flags via `execute` -> `run --resume`. `dockerfile_path` is the second, weaker gap and is stamped the same way (`graded_with_rebuilt_image`): `_build_image` re-runs `docker build` under the deterministic tag `coder-eval-task-:built`, so the grading image REPLACES the run's, and nothing pins image identity on either side — a `reference_digest`-style pin is the real fix and needs the RUN path to record it first, so for now the row says it happened rather than the guide claiming a control that does not exist. The grading container's own logs are folded out of the scratch dir in a `finally`, not only on success: `docker.log` (as `grade.docker.log`, since on the resume path that name is the executed run's) and `grade.log`, which is a documented run-layout artifact holding the per-criterion detail. Folding out only on success deleted exactly the evidence, while DockerRunError's own text said `See {log_path}` — a path already gone by the time it printed. Both copies refuse a symlinked destination, because `shutil.copy2` follows one and the sibling verdict write goes through `write_text_atomic` for precisely that reason; and the verdict write raises `RegradeError`, never a bare `OSError`, since it sits outside the dispatch `try` where `evaluate` (which guards only `RegradeError`) let it escape into Typer AFTER a successful grade while `run --resume` caught it and reported a correct verdict as a grading failure. `grant_container_access` now RETURNS what it widened and `run()` restores it in the same `finally`: the two staging dirs are disposable, but the graded workspace is the caller's tree — an operator-supplied `--workspace` was left world-writable permanently. A container grade also emits its own `CoderEval.Task.End` host-side (`_emit_task_telemetry`), mirroring `batch.py`: every container is launched `TELEMETRY_ENABLED=false` under the invariant "container silent, host emits once", and the grading path had inherited only the silent half. The dispatch is gated on `IN_CONTAINER_ENV`, never on the driver — the in-container entry point rewrites `docker` -> `tempdir` before building its Orchestrator, so a driver-based test would read an already-changed value and a grading container would dispatch a grading container. That env var now has ONE definition (`models/container_paths.py::IN_CONTAINER_ENV`), and **CE056** keeps it that way — the migration converted all four READERS and left the single WRITER (`docker_runner`'s `--env CODER_EVAL_IN_CONTAINER=1`) on the literal, which is the one site that produces the value the gates consume: a rename would have updated every consumer and left the container exporting the old name, disarming the reference anti-cheat window, the reference mount, the grading-container recursion guard and the watchdog together, all silently. CE052 accepts both spellings — a rule that saw only the literal would read a constant-based gate as no gate and tell the author to paste the literal back, arguing against the SSOT it exists to reinforce. The earlier behavior silently rewrote the driver to `tempdir`, which ran a container task's criteria against a host filesystem lacking `/verifier` and the image's toolchain (FAILURE for a trajectory `run` scored 1.0, plus `rm -rf /verifier` unsandboxed on the grading machine) and neutralized `adopt`'s own docker guard; an opted-in row is stamped `graded_on_host` so it is never silently comparable with a container-graded one (lint rule CE051). A re-grade refuses on a `reference_digest` mismatch — the digest is persisted into `environment_info` at staging time by `_stage_reference` (it shipped once as a read with no writer anywhere, so the guard was dead code; then it shipped with a writer whose value was **discarded before it reached disk**, because `_setup` REBOUND the whole `environment_info` dict from `get_version_info()` a hundred lines later, which CE054 cannot see — a write existed in `src/`, it was just dead. `_setup` now `update()`s that dict rather than rebinding it, and `tests/test_detached_grading_boundaries.py` asserts the key survives a real end-to-end run, not just that `_staged_digest` works in isolation), and `verify_reference_unchanged` now takes the task file it resolves against and RAISES on a vanished or unresolvable reference instead of returning silently. That comparison digests a STAGED copy of the source, not the raw tree: the recorded digest is taken over the staged copy, which `stage_reference_dir` filters through `REFERENCE_COPY_IGNORE` (`.git`), so digesting the source directly compared two differently-filtered trees and reported a permanent false mismatch for any reference that is a git checkout — exactly the case the ignore list exists for. **The recorded config is untrusted input**: `evaluate ` rebuilds the task from a shareable artifact, so a rebuilt config that carries shell (`run_command` criteria, `agent_judge`, `llm_judge`, `uipath_eval`, and — only on the `--copy` path, since the in-place path skips them — `pre_run`/`post_run` **and the sandbox's own provisioning**) is REFUSED unless `--allow-recorded-commands` is passed. The provisioning half was the one the gate originally missed, and the worst: `grading_sandbox_config` carries the recorded `sandbox` block through untouched and the `--copy` branch calls `Sandbox.setup`, which reaches `uv pip install ` / `npm install` / `git clone ` — arbitrary code at install time — so a shared run dir whose criteria were all `file_exists` sailed through a scan that walked only `success_criteria`. `git clone` now passes `--` before the URL (argv position 2, so a value beginning with `-` was parsed as an option). `Sandbox.resolve_files` is containment-checked for the same reason: criterion paths were the one task-authored path skipping `_resolve_within_sandbox`, and `Path(root) / '/etc/passwd'` is `/etc/passwd`. An escaping LITERAL now raises `CheckerMisuseError` rather than resolving to `[]`: returning no match books an eval-CONFIG error as an agent failure — a gating 0.0 reading "file does not exist" for a file that plainly does exist and that no agent behaviour could place inside the sandbox (CE039's exact distinction). `tasks/byod_smoke_test.yaml` was broken that way for several commits, checking `/opt/byod_marker` baked into the BYOD image, with only a task-log warning to show for it; it now asserts on the container with `run_command: test -f …`, which is what a claim about the IMAGE rather than about the agent's workspace should look like. The guard keys on the escaping path EXISTING, so a merely-absent absolute path stays an ordinary failing verdict, and the GLOB branch still warns-and-drops, since filtering some matches out of a search is its normal behaviour. A warning is not a control: it prints as the command is already being prepared. Passing the task file explicitly (`evaluate `) also bypasses it, since that config came from the operator. The workspace fallback `artifacts / prior.task_id` is containment-checked like its `sandbox_path` sibling (`task_id` is an unvalidated string, and `"../../.."` joins to a real directory `is_dir()` confirms), `_sanitize_restored_path` drops relative entries (they resolve against the grader's cwd) and anything inside the run dir rather than only the workspace, and `write_text_atomic` opens its temp file `O_EXCL|O_NOFOLLOW` — a pre-planted `task.json.tmp` symlink otherwise bypassed the write-back's destination symlink guard entirely. The record must also describe the task as AUTHORED, not as executed: `run_task_internal_command` rewrites `driver: docker` -> `tempdir` before building the in-container Orchestrator (the one legitimate rewrite — we are already inside the container the driver asked for), and recording that rewrite made a docker run's own `task.json` claim `driver: tempdir`. Since `grading_sandbox_config` reads the driver back OUT of the record, `evaluate ` on a container row skipped BOTH the `--allow-host-grading` refusal and the `graded_on_host` stamp and graded a container task against the host filesystem silently — the exact outcome that gate exists to prevent. `Orchestrator(recorded_task=...)` is the seam: what is recorded, as distinct from what is run — and `recorded_task_file` is its path twin, which must travel with it through EVERY caller. `regrade_in_place` and `_grade_recorded_run` shipped without it, so every container-graded row re-recorded `/work/task_dir/task.yaml` as its `source_file`, reintroducing the defect one caller down; both seams are now pinned by a test that drives the in-container regrade branch end to end, because deleting either left the whole suite green. NOTE the Typer command is a thin wrapper over `run_evaluation(...)`, which has real Python defaults — calling a Typer command function in-process hands unspecified options an `OptionInfo` sentinel, which silently made `in_place=None` truthy. - **`--resume` is command-relative**: `partition_for_resume(tasks, *, grade)` returns a four-way `ResumePartition` (`to_run` / `to_grade` / `prior_results` / `prior_resolved`), because **"finished" is not absolute — it depends on what the resuming command still owes the task**. A `NOT_GRADED` row carries a final status, so the original "has any final status" test called it complete: right for `execute --resume` (it finished executing), and wrong for `run --resume`, which was asked to grade and would instead report "already complete", grade nothing, and **exit 0**. The routing test is the row's **evidence** (`weighted_score is None and not success_criteria_results`), not its category: keying on `category == "ungraded"` missed every `execute` row that ALSO carries an execution fact — a TIMEOUT or budget stop aborts before grading, so it lands unscored with category `error`/`failed`, and resume filed it as complete while `evaluate ` graded the identical bytes happily. Under `grade=True` those rows route to `to_grade`, where `_grade_resumed_tasks` runs the criteria against the trajectory and workspace already on disk via `orchestration/regrade.py::regrade_in_place` — reusing the agent spend, which is the entire reason `execute` and `run` are separate. The carve-out is **only** for `NOT_GRADED`: `FAILURE`/`ERROR` stay complete under both commands (resume has never retried failures — delete the task.json), and `clear_rerun_artifacts` deliberately skips `to_grade`, whose artifacts are the very thing being graded. A per-task grading failure is warned, STAMPED onto the folded-back row's `error_message` (the console line alone is not durable), and folded back in with its ORIGINAL ungraded result, so one bad row neither aborts the resume nor vanishes from run.json — and the exit gate counts `tasks_not_graded` **when `grade` is True**, so a `run` that graded nothing exits non-zero instead of telling CI the suite is fine. Under `execute` an ungraded row is the expected outcome and never fails the command. A row is owed a grade only when it was **executed** AND is unscored: evidence of "no verdict" alone routed every dead container and failed image build (`_write_synthetic_task_json` writes those with no verdict either) into grading, where the fold-back replaced the real diagnostic with a wrong-cause grading error and left `task.json` and `run.json` disagreeing about the same row — so the test is `final_status is NOT_GRADED or iteration_count > 0`, and that fold-back now APPENDS to `error_message` instead of replacing it. A re-grade also writes its log to **`grade.log`**, never `task.log`: `task_log_handler` opens `mode="w"`, so grading into the row's own directory truncated the agent trajectory log the run had already paid for — contradicting `_apply_resume`'s own "to_grade is deliberately NOT cleared" contract. `grade` is in `_FINGERPRINT_DIFF_EXEMPT` because `execute` → `run --resume` is a supported flow, not config drift — and the warning's "already-finalized tasks keep their original-config results" text is actively wrong for it. **`orchestration/regrade.py` is the single implementation** shared by that path and `evaluate`'s run-dir mode, which DELEGATES to `regrade_in_place` rather than restating it (it originally hand-built its own Sandbox + Orchestrator and had already drifted — hardcoding `replicate_index=0`, so every replicate but the first was relabelled — which is exactly how two copies become two verdicts for the same run); it raises plain `RegradeError`, which the CLI wraps, since `orchestration/` must not import the CLI layer (CE004). One fidelity rule it enforces: a re-graded row keeps the **agent run's** `started_at`/`duration_seconds`, not the grading pass's — a 10-minute run re-graded in 2s would otherwise report 2s into `average_duration`, the report tables and the evalboard; the grading cost is preserved separately as `environment_info["grading_duration_seconds"]`. - **One formula per published rate**: `pass_rate` / `error_share` are published by THREE models (`RunSummary`, `VariantAggregate`, `SuiteRollup`) and all three route through the single `models/results.py::nothing_was_measured(not_graded=, measured=)`. The guard originally shipped on `RunSummary` alone, so the same 10-task `execute` run with one crash rendered "Pass Rate: n/a" in `run.md` and "Pass Rate: 0.0%" in `experiment.md`. `measured` is **counted evidence** (`tasks_measured` / `rows_measured` — rows carrying a `weighted_score`), never a bucket count: the first version tested `tasks_succeeded + tasks_failed == 0`, but `TIMEOUT` and the two budget stops are category `failed` and reachable under `execute` (`_check_run_limits` still runs on the ungraded branch), so ONE timed-out row in a 100-task ungraded night read as "measured" and published `pass_rate: 0.0` — a real 0% point on the evalboard trend for a run that graded nothing. The evalboard mirrors the rule: `TaskTrend.passRate` is `number | null`, and an unmeasured task renders "—" and sorts LAST in the worst-first Trends view rather than to the very top as the worst offender. - **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. @@ -233,7 +233,7 @@ make plugin-reference # the plugin's bundled criteria reference from the models Editing `src/coder_eval/pricing.py` means editing `evalboard/lib/pricing.ts` too — it is a hand-copied mirror, and `evalboard/lib/__tests__/pricing-parity.test.ts` fails the build on drift in either direction. -Recent additions, each traceable to a shipped defect: **CE055** (a criterion `path:` in `tasks/` must be sandbox-relative — an absolute path is joined onto the sandbox root, which DISCARDS the root, so containment refuses it and the criterion can never match whatever the agent does; two in-tree tasks were broken this way and the pair is the argument for a static rule on top of the runtime `CheckerMisuseError`: `byod_smoke_test` IS in a CI bucket and produced only `Results: 7/8 succeeded` plus a gating 0.0 reading "file does not exist" for a file that existed, while `dockerfile_build_example` is in NO bucket, so nothing ran it and no runtime guard was ever reached — the fix is never to relax containment but to say what the criterion means, `run_command: test -f /opt/marker`, a claim about the container IMAGE rather than about the agent's workspace), **CE054** (an `environment_info` key that is READ must be WRITTEN somewhere in `src/` — the bag is `dict[str, Any]`, so nothing connects reader to writer, and the `reference_digest` anti-cheat guard shipped as a read with no writer anywhere: `.get()` returned `None`, the guard took its early return, and CLAUDE.md plus the user guide both described it as protection it never provided), **CE048** (never call a Typer command function in process — its parameter defaults are `OptionInfo` sentinels, not values, and the sentinel is TRUTHY, so `in_place=None` silently selected the wrong branch; the fix is the `run_pipeline` / `run_evaluation` / `run_plan` split, and this rule is the one that also scans `tests/`, since that is the only place the defect occurs), **CE049** (never coalesce a possibly-unmeasured score to a numeric literal — `score or 0.0` publishes "measured and scored zero" while meaning "never measured", which is how an ungraded night reached four unfiltered `avg(Score)` dashboards as a real zero), **CE050** (no untyped `getattr` probe for a discriminated-union field — pyright cannot see the string, so a rename degrades the guard to a permanent no-op; scoped to criterion-shaped receivers because `command`/`tool`/`prompt` are far too common to flag on their own), **CE051** (a sandbox driver may not be rewritten silently — the driver IS the isolation boundary, so a downgrade must be an explicit, stamped, operator-visible decision), **CE053** (no bare run-record filename literal outside `path_utils` — `TASK_JSON_FILENAME` shipped with a rename-safety rationale while twelve exact literals stayed unmigrated, including all three `rglob("task.json")` sites the constant's own comment cites as its reason to exist, so it created the second source of truth it argues against), **CE052** (an `os._exit` must sit inside a branch testing `CODER_EVAL_IN_CONTAINER` — it is the right primitive only for reaping the container's own disposable main process, and `run_task_internal_command` armed its heartbeat watchdog, a daemon thread whose whole authority is `os._exit(137)`, unconditionally: a test that invoked the command in-process left the pytest worker holding that thread, which exited the worker 40s later inside an unrelated test file, naming a different test on each run and on each platform with no traceback — and the dead worker's lost coverage data then failed the gate as `65.13 < 80.00`, naming neither the test nor the cause), **CE037** (no unreferenced module-level private helper in `src/` — a helper whose docstring documents a bug the live code still has is worse than none), **CE038** (in an `@asynccontextmanager`, the acquire must sit INSIDE the `try` whose `finally` releases it — `asyncio.shield` protects the inner task, NOT the await, so a cancel on `__aenter__` skips the unwind while the work completes), **CE039** (a criterion checker must not return a gating `score=0.0` from an `except OSError` over a path the *task author* named — that books an eval-config error as an agent failure; raise `CheckerMisuseError` instead, and `# noqa: CE039` the cases that really are the agent's), **CE047** (every onboarding/marketing surface — README, `docs/index.md`, `docs/comparison.md`, `docs/llms.txt`, `mkdocs.yml`'s `site_description`, the Pages stub, and pyproject's `description`/`keywords` — must name every built-in `AgentKind`; OpenCode shipped while four of those seven still listed three harnesses, and nothing failed). +Recent additions, each traceable to a shipped defect: **CE056** (no bare `CODER_EVAL_IN_CONTAINER` literal outside `models/container_paths.py` — the CE053 shape again: a rename-safety constant that shipped beside the literal it replaced, and the straggler was the single WRITER, so a rename would have disarmed four security/correctness gates at once with nothing failing; CE052 cannot catch it because that rule inspects `if` guards and the writer is not one), **CE055** (a criterion `path:` in `tasks/` must be sandbox-relative — an absolute path is joined onto the sandbox root, which DISCARDS the root, so containment refuses it and the criterion can never match whatever the agent does; two in-tree tasks were broken this way and the pair is the argument for a static rule on top of the runtime `CheckerMisuseError`: `byod_smoke_test` IS in a CI bucket and produced only `Results: 7/8 succeeded` plus a gating 0.0 reading "file does not exist" for a file that existed, while `dockerfile_build_example` is in NO bucket, so nothing ran it and no runtime guard was ever reached — the fix is never to relax containment but to say what the criterion means, `run_command: test -f /opt/marker`, a claim about the container IMAGE rather than about the agent's workspace), **CE054** (an `environment_info` key that is READ must be WRITTEN somewhere in `src/` — the bag is `dict[str, Any]`, so nothing connects reader to writer, and the `reference_digest` anti-cheat guard shipped as a read with no writer anywhere: `.get()` returned `None`, the guard took its early return, and CLAUDE.md plus the user guide both described it as protection it never provided), **CE048** (never call a Typer command function in process — its parameter defaults are `OptionInfo` sentinels, not values, and the sentinel is TRUTHY, so `in_place=None` silently selected the wrong branch; the fix is the `run_pipeline` / `run_evaluation` / `run_plan` split, and this rule is the one that also scans `tests/`, since that is the only place the defect occurs), **CE049** (never coalesce a possibly-unmeasured score to a numeric literal — `score or 0.0` publishes "measured and scored zero" while meaning "never measured", which is how an ungraded night reached four unfiltered `avg(Score)` dashboards as a real zero), **CE050** (no untyped `getattr` probe for a discriminated-union field — pyright cannot see the string, so a rename degrades the guard to a permanent no-op; scoped to criterion-shaped receivers because `command`/`tool`/`prompt` are far too common to flag on their own), **CE051** (a sandbox driver may not be rewritten silently — the driver IS the isolation boundary, so a downgrade must be an explicit, stamped, operator-visible decision), **CE053** (no bare run-record or run-LOG filename literal outside `path_utils` — widened to `docker.log` / `grade.docker.log` / `task.log` / `grade.log` after the same shape recurred: `docker.log` was produced in `isolation/` and consumed in `orchestration/` as three unrelated literals, and because the consumer guards its copy with `is_file()`, a rename would have silently discarded the only record of why a grading container failed — `TASK_JSON_FILENAME` shipped with a rename-safety rationale while twelve exact literals stayed unmigrated, including all three `rglob("task.json")` sites the constant's own comment cites as its reason to exist, so it created the second source of truth it argues against), **CE052** (an `os._exit` must sit inside a branch testing `CODER_EVAL_IN_CONTAINER` — it is the right primitive only for reaping the container's own disposable main process, and `run_task_internal_command` armed its heartbeat watchdog, a daemon thread whose whole authority is `os._exit(137)`, unconditionally: a test that invoked the command in-process left the pytest worker holding that thread, which exited the worker 40s later inside an unrelated test file, naming a different test on each run and on each platform with no traceback — and the dead worker's lost coverage data then failed the gate as `65.13 < 80.00`, naming neither the test nor the cause), **CE037** (no unreferenced module-level private helper in `src/` — a helper whose docstring documents a bug the live code still has is worse than none), **CE038** (in an `@asynccontextmanager`, the acquire must sit INSIDE the `try` whose `finally` releases it — `asyncio.shield` protects the inner task, NOT the await, so a cancel on `__aenter__` skips the unwind while the work completes), **CE039** (a criterion checker must not return a gating `score=0.0` from an `except OSError` over a path the *task author* named — that books an eval-config error as an agent failure; raise `CheckerMisuseError` instead, and `# noqa: CE039` the cases that really are the agent's), **CE047** (every onboarding/marketing surface — README, `docs/index.md`, `docs/comparison.md`, `docs/llms.txt`, `mkdocs.yml`'s `site_description`, the Pages stub, and pyproject's `description`/`keywords` — must name every built-in `AgentKind`; OpenCode shipped while four of those seven still listed three harnesses, and nothing failed). When fixing a bug, ask: *could a custom lint rule have prevented this?* If the root cause is a mechanically detectable pattern (e.g., "always import from `coder_eval.models`", "never call blocking IO in async"), add a rule to `tests/lint/rules/` following the CE001+ pattern and wire it up in `tests/lint/runner.py`. This turns a one-time fix into permanent enforcement. See `tests/test_custom_lint.py` for how rules are tested. (Doc-surface / whole-tree rules that reason over Markdown/YAML or the entire `src/` tree rather than one `.py` AST at a time — CE026–CE031, CE033–CE036 — are not `BaseRule`s in the runner; they are wired as dedicated `@pytest.mark.lint` test classes. CE036 enforces the `live_verdict` determinism + monotonicity contract (`criteria/base.py`) that `EarlyStopWatcher`'s latching, deferred fail-stop, and flip-attribution silently depend on: monotonicity over arbitrary Python is undecidable, so instead of a static check it REPLAYS each live criterion against every prefix of recorded trajectories (`tests/lint/live_verdict_contract.py::CASES`) — on the authored ordering AND under seeded shuffles (`permuted_violations`, which catch order-sensitive bugs the authored walk misses) — and asserts the property directly, plus registry-derived coverage — every `LiveSuccessCriterion` in the union must have cases, and every polarity its instances claim via `live_decidable_polarities()` must actually be reached by one (otherwise a single always-`undecided` fixture would "cover" a type while proving nothing). Adding a live criterion therefore means adding `ContractCase`s in the same change. CE035 resolves every `steps..outputs.` / `needs..outputs.` reference in `.github/workflows/**` to a writer that actually produces that key — GitHub expands an unwritten output to the empty string, so a typo degrades a gate silently and actionlint models `steps.*.outputs` as an open string map. CE034 scans `tasks/` and forces an armed, live-*passable* `command_executed` to set `require_success` — a crashed invocation would otherwise latch a live PASS, fire `on_pass: stop`, and let FIRED-ONLY armed gating report SUCCESS without ever consulting the unarmed criteria (negative assertions are fail-only and are exempt). CE033 keeps the plugin's bundled `reference/criteria.md` in parity with the `SuccessCriterion` union that generates it (`make plugin-reference` writes it; the rule re-renders and diffs — never hand-edit the file). CE031 guards against dead config: a behavior-driving field on `SimulationConfig`/`RunLimits`/`Dataset` that no code reads by name. CE026 keeps the GitHub Action's onboarding surfaces honest — `README.md`, `docs/CI_GATE.md`, `docs/tutorials/02-ci-pipeline.md`, and the plugin's `ci` skill, whose emitted workflow users copy into their own repos: a page's *first* Action snippet must show the agent-runtime prerequisite steps (pinned to the `action-dogfood` job that proves them in CI), a zero-install absolute next to such a snippet must name the channel it means, every `github.com/marketplace/actions/` link plus the shields badge label must match `action.yml`'s `name:`, and every `with:` key on a snippet's action step must be a real `action.yml` input (GitHub ignores unknown inputs, so a rename would silently degrade every copied workflow). Renaming an action input or changing its runtime prerequisites therefore means updating the skill too.) diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 4209c6bd..2d93d36a 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -42,7 +42,7 @@ coder-eval run tasks/hello_date.yaml --stream full # live LLM output | `--type, -T` | Override agent type for all tasks (`claude-code`, `codex`, `antigravity`, `opencode`, `pi`, or a plugin kind). | | `--repeats` | Run each `(task, variant)` N times (≥1); overrides experiment/variant `repeats:`. See [Replicates](#replicates). | | `--resume` | Resume an interrupted run: skip tasks already finalized in `--run-dir` and run the rest, folding prior results into `run.json`. Requires `--run-dir`. See [Resuming a run](#resuming-a-run). | -| `--allow-host-grading` | `--resume` only. Grade an executed-but-ungraded `driver: docker` row on this host instead of refusing; the row is stamped `graded_on_host`. Rejected without `--resume`, since a fresh `run` grades inside the driver the task asks for. | +| `--allow-host-grading` | `--resume` only. Grade an executed-but-ungraded `driver: docker` row on this host instead of in a container of the task's own image (the default); the row is stamped `graded_on_host`. Rejected without `--resume`, since a fresh `run` grades inside the driver the task asks for. | | `--sample N` | For dataset-backed tasks, run a fixed-seed random N-row sample (reproducible; cheap smoke test). See [Bring Your Own Dataset](DATASETS.md). | | `--sample-per-stratum N` | For dataset-backed tasks, keep up to N rows per stratum (`stratify_field`). Overridden by `--sample`. Nondeterministic unless `dataset.sample_seed` is set — see [Bring Your Own Dataset](DATASETS.md). | | `--include-skipped` | Also run tasks marked `skip: true` in their YAML (off by default so CI keeps excluding them). | @@ -197,7 +197,7 @@ commands, so an errored row could never be graded again. **A run directory is untrusted input.** It is a shareable artifact — the whole point of the detached flow is that one machine executes and another grades — and rebuilding the task from it means the run dir decides what runs on your host, -with your environment. So two things are refused rather than assumed: +with your environment. So the recorded config is refused rather than assumed: - A recorded config that carries shell (`run_command` criteria, `agent_judge`, `uipath_eval`, an authored `post_run`, and on the `--copy` path `pre_run`) needs @@ -209,11 +209,48 @@ with your environment. So two things are refused rather than assumed: exempt. The record did not choose it, running it is exactly what your own config does on every run, and prompting on it would fire for 100% of run directories — a refusal that always fires stops being read. -- A run made with `driver: docker` needs `--allow-host-grading`. Grading cannot - start a container, and such a task's criteria address container paths and - toolchains; on your host they score `0.0` for a run that passed. An opted-in - row is stamped `graded_on_host` in `environment_info` so it is never silently - compared with a container-graded one. +- A run made with `driver: docker` is graded **in a container of the task's own + image**, so its criteria address the same paths and toolchain they did during + the run. Starting that container is itself a capability the record chose — it + names the image, and the default credential allowlist is forwarded into it — + so it is listed by the same gate and needs the same `--allow-recorded-commands` + (or an explicit task file). Grading this way needs a working docker daemon, and + may pull or build an image. + + `--allow-host-grading` is the escape hatch: no docker here, or criteria you + know are host-portable. It grades on this machine instead, and stamps the row + `graded_on_host` so it is never silently compared with a container-graded one. + + Two limits are worth knowing before you rely on **container grading**. The + grading container is a **second, fresh** container: only the workspace crosses + from the one that ran the agent, and `pre_run` is **not** re-run — so a + criterion that depends on state `pre_run` put outside the workspace (a symlink + in `/root`, an installed package, a started service) will not see it. And for a + `dockerfile_path` task the grading phase re-runs `docker build`, so a + Dockerfile or base image that changed between the two phases yields a different + grading image; nothing records the image identity, so that one cannot be + detected after the fact. + + Both are warned about at dispatch **and** stamped onto the row, so a consumer + can filter them out rather than take the console's word for it: + `environment_info.graded_without_pre_run` carries the number of `pre_run` + commands that did not re-run, and `environment_info.graded_with_rebuilt_image` + names the Dockerfile that was rebuilt. For either, a single `coder-eval run` is + exact. + +`run --resume` is not affected by the gate at all: it re-resolves the task from +your own YAML rather than from the record. + +Why it is not merely nicer: `tasks/byod_smoke_test.yaml` asserts +`test -f /opt/byod_marker`, a file baked into its image. The identical row scores +`SUCCESS 1.000` graded in a container and `FAILURE 0.000` graded on your host — +because the host is answering "is that marker on THIS machine", which nobody +asked. A container-graded row carries no `graded_on_host` stamp, exactly like a +row `coder-eval run` produced. + +Grading in a container needs a task file to resolve the image from. When the run +records none, `evaluate` says so and points at the two ways forward — pass the +task file explicitly, or `--allow-host-grading`. Passing a task file **over** a run directory re-grades it with different criteria, reusing the trajectory and workspace of a run you already paid for: @@ -236,7 +273,7 @@ rather than as a verdict. Override either default with `--in-place` / `--copy`. | `--preserve / --no-preserve` | Preserve sandbox after evaluation (default: preserve). Ignored when grading in place — an adopted directory is never moved or deleted. | | `--run-dir` | Where the graded `task.json` lands (default: auto-generated timestamped dir in `runs/`). | | `--allow-recorded-commands` | Accept a rebuilt config that would run shell (`run_command` criteria, judges, `pre_run`/`post_run`) or install packages on this host. Refused by default — a run directory is a shareable artifact, so its recorded config is untrusted input. | -| `--allow-host-grading` | Grade a `driver: docker` task on this host instead of refusing. The row is stamped `graded_on_host` so it is never silently compared with a container-graded one. | +| `--allow-host-grading` | Grade a `driver: docker` task on this host instead of in a container of the task's own image (the default). The row is stamped `graded_on_host` so it is never silently compared with a container-graded one. | | `--verbose, -v` | DEBUG-level logging | A re-grade refuses to run if the task's `reference:` directory changed since the diff --git a/pyproject.toml b/pyproject.toml index d6addbd6..828f994a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -273,6 +273,7 @@ external = [ "CE053", "CE054", "CE055", + "CE056", ] # custom architectural lint rules (tests/lint/) [tool.ruff.lint.pylint] diff --git a/src/coder_eval/cli/evaluate_command.py b/src/coder_eval/cli/evaluate_command.py index dff52a18..f8ba3f45 100644 --- a/src/coder_eval/cli/evaluate_command.py +++ b/src/coder_eval/cli/evaluate_command.py @@ -8,6 +8,7 @@ from pathlib import Path import typer +from rich.markup import escape from ..evaluation.judge_persistence import TASK_JSON_TRANSCRIPT_EXCLUDE from ..logging_config import setup_logging @@ -87,6 +88,7 @@ def _resolve_inputs( *, allow_recorded_commands: bool, in_place: bool | None, + allow_host_grading: bool = False, ) -> _ResolvedInputs: """Turn the CLI positionals into a task, a workspace, and (maybe) a prior run. @@ -106,7 +108,11 @@ def _resolve_inputs( try: return _resolve_run_dir_or_work_dir( - target, workspace, allow_recorded_commands=allow_recorded_commands, in_place=in_place + target, + workspace, + allow_recorded_commands=allow_recorded_commands, + in_place=in_place, + allow_host_grading=allow_host_grading, ) except RegradeError as e: # The shared core raises a plain exception (orchestration/ must not @@ -120,6 +126,7 @@ def _resolve_run_dir_or_work_dir( *, allow_recorded_commands: bool, in_place: bool | None, + allow_host_grading: bool = False, ) -> _ResolvedInputs: """The mode-specific half of :func:`_resolve_inputs`.""" prior: EvaluationResult | None = None @@ -146,24 +153,29 @@ def _resolve_run_dir_or_work_dir( task, source_yaml = load_task(target.task_file) console.print(f"[dim]Grading with {target.task_file} (overrides the run's recorded config).[/dim]") else: - # pre_run and the sandbox's installers both run only on the --copy - # path (an adopted workspace must not have pre_run re-run over the - # agent's deliverables, and `adopt` installs nothing), so in place - # they are not capabilities the run dir can reach. post_run is NOT - # one of them — it belongs to the grading phase and runs on both - # paths, so `embedded_commands` scans it unconditionally. + # ONE lever, passed once, and derived through the SAME function + # `run_evaluation` uses rather than restated. It decides which + # capability families the recorded-shell gate discloses, so a second + # copy of the rule would keep answering the old question the moment + # the default moved — and silently stop covering commands that then + # do run. # - # Derived through the SAME function `run_evaluation` uses, not - # restated. This value decides whether recorded shell is refused, so - # a second copy of the rule would keep answering the old question if - # the default ever moved — and silently stop covering commands that - # then do run. - setup_will_run = not resolve_grade_in_place(target, in_place) + # In place: the grade may dispatch a CONTAINER built from the + # recorded sandbox block, a wider capability than any recorded shell + # string. On --copy instead: pre_run and the sandbox's own + # installers, neither of which an adopted workspace reaches. post_run + # is in NEITHER set — it belongs to the grading phase and runs on + # both paths, so `embedded_commands` scans it unconditionally. + # + # Both answers follow from this single boolean, so the gate derives + # them itself (`_gate_scope_for_grade`) rather than taking two + # arguments a caller could set incoherently. task, source_yaml = task_from_prior( prior, target.target, allow_recorded_commands=allow_recorded_commands, - include_setup_phase=setup_will_run, + grade_in_place=resolve_grade_in_place(target, in_place), + allow_host_grading=allow_host_grading, ) work_dir = workspace or default_workspace(target.target, prior) recorded_source = prior.task_config.source_file if prior.task_config else None @@ -174,12 +186,12 @@ def _resolve_run_dir_or_work_dir( try: task, source_yaml = load_task(task_file) except Exception as e: - console.print(f"[red]✗ Failed to load task:[/red] {e}") + console.print(f"[red]✗ Failed to load task:[/red] {escape(str(e))}") raise typer.Exit(1) from e work_dir = target.target if not work_dir.is_dir(): - console.print(f"[red]✗ Work directory is not a directory:[/red] {work_dir}") + console.print(f"[red]✗ Work directory is not a directory:[/red] {escape(str(work_dir))}") raise typer.Exit(1) # Evaluate-only mode bypasses experiment resolution + CLI overrides, so @@ -276,18 +288,22 @@ def evaluate_command( False, "--allow-recorded-commands", help=( - "Accept shell commands (run_command criteria, pre_run/post_run) rebuilt from the run " - "directory's own task.json. A run directory is a shareable artifact, so its recorded " - "config is untrusted input; without this, grading refuses rather than running it here." + "Accept the capabilities rebuilt from the run directory's own task.json: shell " + "(run_command criteria, pre_run/post_run) and, for a `driver: docker` row, starting a " + "container of the image the record names with your credentials in its environment. A " + "run directory is a shareable artifact, so its recorded config is untrusted input; " + "without this, grading refuses rather than running it here." ), ), allow_host_grading: bool = typer.Option( False, "--allow-host-grading", help=( - "Grade a `driver: docker` run on this host. Grading cannot start a container, so the " - "criteria run against a filesystem that lacks the container's paths and toolchain — " - "scores may differ from the run. Such rows are stamped graded_on_host." + "Grade a `driver: docker` run on THIS HOST instead of in a container of the task's " + "own image (the default). For a machine with no docker, or criteria you know are " + "host-portable. The criteria then run against a filesystem lacking the container's " + "paths and toolchain, so scores may differ from the run; such rows are stamped " + "graded_on_host." ), ), run_dir: Path | None = typer.Option( # noqa: B008 @@ -359,6 +375,7 @@ def run_evaluation( workspace, allow_recorded_commands=allow_recorded_commands, in_place=in_place, + allow_host_grading=allow_host_grading, ) task = inputs.task source_yaml = inputs.source_yaml @@ -372,22 +389,21 @@ def run_evaluation( try: prepared_run_dir = prepare_run_directory(run_dir) except Exception as e: - console.print(f"[red]✗ Failed to prepare run directory:[/red] {e}") + console.print(f"[red]✗ Failed to prepare run directory:[/red] {escape(str(e))}") raise typer.Exit(1) from e - try: - sandbox_config = grading_sandbox_config(task, allow_host_grading=allow_host_grading) - except RegradeError as e: - console.print(f"[red]✗ {e}[/red]") - raise typer.Exit(1) from e - if not grade_in_place: - # Copy path: preload the sandbox with the work dir as a template source. - template_source = TemplateDirSource(path=str(graded_dir.resolve())) - sandbox_config.template_sources = [template_source, *(sandbox_config.template_sources or [])] - - task_dir = task_file.parent.resolve() if task_file is not None else None - sandbox = Sandbox(sandbox_config, task_id=task.task_id, task_dir=task_dir) - + # `regrade_in_place` owns the sandbox on the delegating path — and for a + # `driver: docker` row it owns rather more than that, dispatching a grading + # CONTAINER of the task's own image. Building a host sandbox_config here + # first would call `grading_sandbox_config`, whose whole job is to REFUSE + # that driver, so the refusal fired before the branch that no longer needs + # it and no docker row could ever be graded properly. + # + # Branching on ``prior is not None`` directly, and building the sandbox + # inside the branch that uses it, so NEITHER value is Optional at its use + # site. Both were, briefly, re-narrowed by a bare `assert` plus a comment + # asserting an invariant the type checker could hold structurally — and + # `assert` is the weakest narrowing available, stripped entirely under -O. async def _setup_and_run() -> EvaluationResult: if grade_in_place and prior is not None: # Delegate to the shared re-grade core. Restating its body here is @@ -406,6 +422,13 @@ async def _setup_and_run() -> EvaluationResult: replicate_index=_replicate_index_of(target.target), allow_host_grading=allow_host_grading, ) + sandbox_config = grading_sandbox_config(task, allow_host_grading=allow_host_grading) + if not grade_in_place: + # Copy path: preload the sandbox with the work dir as a template source. + template_source = TemplateDirSource(path=str(graded_dir.resolve())) + sandbox_config.template_sources = [template_source, *(sandbox_config.template_sources or [])] + task_dir = task_file.parent.resolve() if task_file is not None else None + sandbox = Sandbox(sandbox_config, task_id=task.task_id, task_dir=task_dir) if grade_in_place: await asyncio.to_thread(sandbox.adopt, graded_dir) else: @@ -427,8 +450,9 @@ async def _setup_and_run() -> EvaluationResult: prior_result=prior, ) graded = await orchestrator.run() - # Same stamp the delegating branch gets from `regrade_in_place`. Line 357 - # above accepted the docker→host downgrade for THIS branch too, and + # Same stamp the delegating branch gets from `regrade_in_place`. The + # `grading_sandbox_config` call above accepted the docker->host + # downgrade for THIS branch too, and # CLAUDE.md, the user guide and CE051's own noqa all state the stamp as # unconditional — so `evaluate --copy --allow-host-grading` # was writing an unstamped host verdict that nothing downstream could @@ -436,7 +460,15 @@ async def _setup_and_run() -> EvaluationResult: stamp_host_grading(graded, task) return graded - result = asyncio.run(_setup_and_run()) + try: + result = asyncio.run(_setup_and_run()) + except RegradeError as e: + # The delegating branch raises this for the missing/unresolvable task + # file and for a failed grading container, and both messages carry the + # operator's next step. Rendered like the three sibling handlers above -- + # unwrapped, they arrived as the tail of a stack trace. + console.print(f"[red]✗ {escape(str(e))}[/red]") + raise typer.Exit(1) from e _report_and_exit(result, task=task, prior=prior, target=target, prepared_run_dir=prepared_run_dir) diff --git a/src/coder_eval/cli/run_command.py b/src/coder_eval/cli/run_command.py index b24fa720..d6994a1e 100644 --- a/src/coder_eval/cli/run_command.py +++ b/src/coder_eval/cli/run_command.py @@ -217,11 +217,11 @@ def run_command( False, "--allow-host-grading", help=( - "When --resume grades a `driver: docker` row, grade it on this host anyway. " - "Grading cannot start a container, so such criteria run against a filesystem " - "lacking the container's paths and toolchain and may score differently than " - "the run did; those rows are stamped graded_on_host. Without this they are " - "refused and stay ungraded." + "When --resume grades a `driver: docker` row, grade it on this host instead of " + "in a container of the task's own image (the default). Such criteria then run " + "against a filesystem lacking the container's paths and toolchain and may score " + "differently than the run did, so those rows are stamped graded_on_host. Use it " + "for a machine with no docker, or for criteria you know are host-portable." ), ), max_parallel: int = typer.Option( diff --git a/src/coder_eval/cli/run_task_internal_command.py b/src/coder_eval/cli/run_task_internal_command.py index a1715493..786ddba9 100644 --- a/src/coder_eval/cli/run_task_internal_command.py +++ b/src/coder_eval/cli/run_task_internal_command.py @@ -32,11 +32,15 @@ CONTAINER_INPUT_DIR, CONTAINER_OUTPUT_DIR, CONTAINER_TASK_DIR, + IN_CONTAINER_ENV, ConfigLineageEntry, + EvaluationResult, PreservationMode, SandboxConfig, + TaskDefinition, ) from coder_eval.orchestration.task_loader import load_task +from coder_eval.path_utils import PRIOR_RESULT_FILENAME logger = logging.getLogger(__name__) @@ -87,7 +91,7 @@ def _arm_host_heartbeat_watchdog(output_dir: Path) -> None: # building the in-container Orchestrator, so a driver-based gate would # disarm itself on exactly the path that needs it. See # `Sandbox.enforces_permission_windows`. - if _os.environ.get("CODER_EVAL_IN_CONTAINER") == "1": + if _os.environ.get(IN_CONTAINER_ENV) == "1": def _watch_host_heartbeat() -> None: heartbeat = output_dir / HEARTBEAT_FILENAME @@ -211,6 +215,22 @@ def run_task_internal_command( typer.echo(f"FATAL: context.json 'grade' must be a boolean, got {grade_raw!r}", err=True) raise typer.Exit(2) grade: bool = grade_raw + # A DETACHED GRADE, not a run: seed from the staged prior.json and adopt the + # already-executed workspace instead of starting an agent. Coerced for the + # same reason `grade` is — a hand-edited `"regrade": "false"` is a truthy + # str, and getting this one wrong would re-RUN the agent against a workspace + # the operator asked only to grade, destroying the trajectory being graded. + regrade_raw = context.get("regrade", False) + if not isinstance(regrade_raw, bool): + typer.echo(f"FATAL: context.json 'regrade' must be a boolean, got {regrade_raw!r}", err=True) + raise typer.Exit(2) + regrade: bool = regrade_raw + # What task.json RECORDS as the task's source path, as distinct from the + # path this process resolves TASK_DIR against (see Orchestrator's + # `recorded_task_file`). Absent on an older host -> None -> the container + # path is recorded, which is the pre-existing behaviour. + host_task_file_raw = context.get("host_task_file") + recorded_task_file = Path(host_task_file_raw) if host_task_file_raw else None # Docker WORKDIR alignment: the host resolves the concrete WORKDIR # (config value / "auto" -> `docker inspect` / fallback) and forwards it here. # Absent -> None -> standard run_dir/artifacts workspace. @@ -259,6 +279,20 @@ def run_task_internal_command( output_dir.mkdir(parents=True, exist_ok=True) + if regrade: + _grade_recorded_run( + task=task, + authored_task=authored_task, + recorded_task_file=recorded_task_file, + input_dir=input_dir, + output_dir=output_dir, + runtime_task_file=runtime_task_file, + source_yaml=source_yaml, + variant_id=variant_id, + replicate_index=replicate_index, + ) + return + # Late import: orchestrator pulls in heavy deps (anthropic SDK etc.) # that we don't want to load just to print --help. from coder_eval.orchestrator import Orchestrator @@ -268,6 +302,7 @@ def run_task_internal_command( run_dir=output_dir, preservation_mode=preservation_mode, task_file=runtime_task_file, + recorded_task_file=recorded_task_file, variant_id=variant_id, source_yaml=source_yaml, config_lineage=config_lineage, @@ -286,3 +321,90 @@ def run_task_internal_command( asyncio.run(orchestrator.run()) # Orchestrator.run() writes task.json to run_dir (== output_dir). Done. + + +def _grade_recorded_run( + *, + task: TaskDefinition, + authored_task: TaskDefinition, + input_dir: Path, + output_dir: Path, + recorded_task_file: Path | None, + runtime_task_file: Path, + source_yaml: str, + variant_id: str, + replicate_index: int, +) -> None: + """Grade an already-executed row INSIDE the container that produced it. + + This is the container half of `evaluate ` / `run --resume` over a + `driver: docker` task. The host stages `prior.json` next to `task.yaml` and + bind-mounts the executed workspace at ``CONTAINER_GRADE_WORKSPACE``; here we + seed from that row and run its criteria against that workspace. + + Why it must happen here at all: a container task's criteria address the + image's paths and toolchain, so grading them on the host scores a FAILURE for + a run that passed. The host path therefore REFUSES by default and demands + `--allow-host-grading`. Running them back inside the same image is the only + place the verdict means what it meant during the run — so a container-graded + detached row carries no `graded_on_host` stamp, exactly like a `run` row. + + ``task`` is the driver-rewritten copy (docker -> tempdir, done above because + we are already inside the container the driver asked for), which is also what + keeps ``regrade_in_place`` from trying to dispatch a container from within + one. ``authored_task`` is what gets RECORDED, so the row keeps saying + `driver: docker`. ``recorded_task_file`` is the path half of that same + distinction and travels with it: without it the row re-records + ``/work/task_dir/task.yaml`` as its ``source_file``, a path on no host, and a + later ``evaluate `` over the row refuses or mounts the wrong tree. + The ordinary run branch above has always forwarded it; this one is the + second consumer and must not be the one that forgets. + + Delegates to the same ``regrade_in_place`` the host uses rather than + restating it. The two implementations that already drifted apart once — + `evaluate`'s run-dir mode hardcoding `replicate_index=0` and relabelling + every replicate but the first — are the reason that function exists. + """ + from coder_eval.models import CONTAINER_GRADE_WORKSPACE + from coder_eval.orchestration.regrade import RegradeError, regrade_in_place + + prior_path = input_dir / PRIOR_RESULT_FILENAME + if not prior_path.is_file(): + typer.echo(f"FATAL: context.json requested a regrade but {prior_path} is missing", err=True) + raise typer.Exit(2) + try: + prior = EvaluationResult.model_validate_json(prior_path.read_text(encoding="utf-8")) + except (OSError, ValueError) as e: + # Degrade to a clean message rather than a traceback: the host parses + # this container's task.json, so a crash here surfaces as the opaque + # "container exited without producing task.json" rather than naming the + # staged file that could not be read. + typer.echo(f"FATAL: {prior_path} is not a readable EvaluationResult: {e}", err=True) + raise typer.Exit(2) from e + + workspace = Path(CONTAINER_GRADE_WORKSPACE) + if not workspace.is_dir(): + typer.echo(f"FATAL: the graded workspace was not mounted at {workspace}", err=True) + raise typer.Exit(2) + + try: + asyncio.run( + regrade_in_place( + task=task, + prior=prior, + workspace=workspace, + run_dir=output_dir, + task_file=runtime_task_file, + source_yaml=source_yaml, + variant_id=variant_id, + replicate_index=replicate_index, + recorded_task=authored_task, + recorded_task_file=recorded_task_file, + ) + ) + except RegradeError as e: + # Surfaced as a clean message, not a traceback: the host parses this + # container's task.json, and a RegradeError means none was written. Exit + # 2 keeps it distinguishable from an agent failure. + typer.echo(f"FATAL: {e}", err=True) + raise typer.Exit(2) from e diff --git a/src/coder_eval/isolation/docker_runner.py b/src/coder_eval/isolation/docker_runner.py index 57bac3da..76dd0f76 100644 --- a/src/coder_eval/isolation/docker_runner.py +++ b/src/coder_eval/isolation/docker_runner.py @@ -26,11 +26,13 @@ from coder_eval.logging_config import DEFAULT_LOG_TAIL_MAX_BYTES from coder_eval.models import ( + CONTAINER_GRADE_WORKSPACE, CONTAINER_INPUT_DIR, CONTAINER_OUTPUT_DIR, CONTAINER_REFERENCE_DIR, CONTAINER_TASK_DIR, CONTAINER_WORK_DIR, + IN_CONTAINER_ENV, RESERVED_CONTAINER_DIRS, AgentKind, DockerDriverConfig, @@ -41,6 +43,8 @@ ) from coder_eval.orchestration.evaluation import resolve_host_reference_dir from coder_eval.path_utils import ( + DOCKER_LOG_FILENAME, + PRIOR_RESULT_FILENAME, REFERENCE_COPY_IGNORE, TASK_JSON_FILENAME, ignore_patterns_and_symlinks, @@ -482,7 +486,7 @@ def _copy_claude_home(host_claude_dir: Path, claude_copy: Path) -> None: ) from last_exc -def grant_container_access(root: Path, *, writable: bool) -> None: +def grant_container_access(root: Path, *, writable: bool) -> list[tuple[Path, int]]: """Widen ``root`` (recursively) so the container can reach it without DAC caps. Paired with the ``--cap-drop DAC_OVERRIDE --cap-drop DAC_READ_SEARCH`` in @@ -511,10 +515,16 @@ def grant_container_access(root: Path, *, writable: bool) -> None: withholding it keeps ``_verify_reference_integrity`` from being the sole guard against tampering. + Returns ``(path, original_mode)`` for every entry it actually changed, so a + caller that widened a tree it does not own can put it back (see + :func:`restore_modes`). The framework-created staging dirs are disposable and + ignore it; the graded workspace is not. + No-op on Windows, where POSIX mode bits are not the access-control mechanism. """ + widened_paths: list[tuple[Path, int]] = [] if os.name == "nt": # pragma: no cover - POSIX mode bits are meaningless here - return + return widened_paths extra = 0o006 if writable else 0o004 for path in (root, *root.rglob("*")): # lstat + skip: chmod follows symlinks, so widening one would silently @@ -531,6 +541,54 @@ def grant_container_access(root: Path, *, writable: bool) -> None: widened |= 0o001 if widened != mode: os.chmod(path, widened) + widened_paths.append((path, mode)) + return widened_paths + + +def restore_modes(widened: list[tuple[Path, int]]) -> None: + """Put back the modes :func:`grant_container_access` widened. + + Needed for exactly one mount, and the asymmetry is the point. ``input_dir`` + and ``output_dir`` are staging directories the harness created for this one + dispatch and deletes afterwards, so widening them is scoped to their whole + lifetime. The GRADED WORKSPACE is neither: with ``--workspace`` it is an + arbitrary operator directory, and otherwise it is the run's preserved + ``artifacts/`` tree that outlives the grade. Leaving those world-writable + means any other local uid on a shared or CI host can afterwards rewrite the + artifacts a criterion reads -- i.e. change the verdict -- or plant an + executable in the tree. + + Best-effort and never raises: this runs in a ``finally`` beside the staging + cleanup, and a failed restore must not mask the container's own outcome. + """ + for path, mode in reversed(widened): + try: + if not path.is_symlink(): + os.chmod(path, mode) + except OSError as exc: # pragma: no cover - raced away or removed by the container + logger.warning("Could not restore mode on %s: %s", path, exc) + + +def _quarantine_record(task_json: Path | None, suffix: str, label: str) -> None: + """Move a refused container record aside, best-effort. + + Shared by both version-skew refusals (`_assert_grade_honored`, + `_assert_regrade_honored`), which had the same seven lines twice and differed + only in the suffix and the wording. Refusing in memory while leaving + contradictory bytes in the bind-mounted run dir is not a refusal -- a later + `aggregate` would publish exactly the row the guard declined -- so this must + behave identically on both paths, which one copy per caller cannot promise. + + Never masks the caller's raise: a failed move is logged and swallowed. + """ + if task_json is None: + return + sidecar = task_json.with_suffix(task_json.suffix + suffix) + try: + os.replace(task_json, sidecar) # atomic; overwrites any stale prior sidecar + logger.warning("Quarantined the refused %s record to %s", label, sidecar) + except OSError as exc: + logger.warning("Could not quarantine %s: %s", task_json, exc) class DockerRunner: @@ -547,11 +605,29 @@ def __init__( stream_callback: StreamCallback | None = None, verbose: bool = False, grade: bool = True, + prior_result: EvaluationResult | None = None, + grade_workspace: Path | None = None, ) -> None: self.rt = rt self.preservation_mode = preservation_mode self.stream_callback = stream_callback self.verbose = verbose + # DETACHED GRADE. Both set together or neither: `prior_result` is the + # already-executed row (trajectory + execution facts) the in-container + # Orchestrator seeds from, and `grade_workspace` is the host directory + # that run left behind, mounted at CONTAINER_GRADE_WORKSPACE and ADOPTED + # rather than recreated. + # + # This is what makes `evaluate` over a `driver: docker` row honest. The + # criteria of such a task address container paths and the image's + # toolchain, so grading them on the host scores a FAILURE for a run that + # passed. Running them back inside the same image is not a workaround for + # that — it is the only place the verdict means what it meant during the + # run. + self.prior_result = prior_result + self.grade_workspace = grade_workspace + if (prior_result is None) != (grade_workspace is None): + raise ValueError("prior_result and grade_workspace must be passed together") # Forwarded to the in-container orchestrator via context.json. It is a # run-level decision made by the CLI, so it cannot be recovered from the # staged task.yaml on the other side. @@ -629,6 +705,9 @@ async def run(self) -> EvaluationResult: input_dir = staging / "input" await asyncio.to_thread(input_dir.mkdir) output_dir = self.rt.run_dir.resolve() + # Bound BEFORE the try: the `finally` restores it, and `_stage_inputs` + # can raise before the widening happens. + widened_workspace: list[tuple[Path, int]] = [] try: await self._stage_inputs(input_dir) @@ -659,6 +738,24 @@ async def run(self) -> EvaluationResult: # consumes; writable only for the run dir it must produce into. await asyncio.to_thread(grant_container_access, input_dir, writable=False) await asyncio.to_thread(grant_container_access, output_dir, writable=True) + if self.grade_workspace is not None: + # The graded workspace is a framework-owned mount like any other, + # so it needs the same widening -- and it is the one mount whose + # files the harness did NOT create, so the owner bits cannot be + # assumed. It happens to work when container #1 (running as root) + # wrote the tree, which is exactly what makes the broken case + # expensive: an operator-supplied `--workspace`, or artifacts + # re-created host-side, are owned by the host uid, and container + # root without DAC_OVERRIDE reaches them only through `other`. + # Criteria then fail EACCES and book a gating 0.0 that reads as + # an agent failure -- the CE039 shape this feature exists to end. + # + # Recorded and restored in the `finally` below, unlike the two + # staging dirs above: those are disposable and deleted with the + # dispatch, while this tree survives it. An operator-supplied + # `--workspace` left world-writable forever is a real, permanent + # exposure on a shared host. + widened_workspace = await asyncio.to_thread(grant_container_access, self.grade_workspace, writable=True) argv = self._build_argv(input_dir, output_dir, container_name=container_name, image=image) logger.info("Running task '%s' in docker: %s", self.rt.task.task_id, " ".join(argv)) # Prime the heartbeat before the container starts so the @@ -672,7 +769,7 @@ async def run(self) -> EvaluationResult: stderr=asyncio.subprocess.STDOUT, limit=STDOUT_LINE_LIMIT_BYTES, ) - log_path = self.rt.run_dir / "docker.log" + log_path = self.rt.run_dir / DOCKER_LOG_FILENAME log_fh = await asyncio.to_thread(log_path.open, "w", encoding="utf-8") # Cancellation guard: `docker run --rm` does NOT propagate kill # to the container daemon-side. Without this `finally`, Ctrl-C @@ -704,6 +801,9 @@ async def run(self) -> EvaluationResult: # directory raises PermissionError -- which ignore_errors swallows, # orphaning a tempdir that holds the reference solution. await asyncio.to_thread(rmtree_restrictive, staging) + # The graded workspace is the caller's tree, not ours; give it back + # the modes it had. See `restore_modes`. + await asyncio.to_thread(restore_modes, widened_workspace) async def _stage_inputs(self, input_dir: Path) -> None: """Serialise the post-override TaskDefinition + lineage/variant context into the @@ -735,13 +835,34 @@ def _dump_task_yaml() -> str: # `coder-eval run` vs `coder-eval execute`. Not derivable from # task.yaml on the container side (deliberately not a task field). "grade": self.grade, + # A detached grade: seed from prior.json (staged beside this + # file) and adopt CONTAINER_GRADE_WORKSPACE instead of running + # an agent. Absent/False on every ordinary run. + "regrade": self.prior_result is not None, "source_yaml": self.rt.source_yaml, + # The HOST's task-file path, recorded verbatim into task.json's + # audit trail. The container resolves TASK_DIR against + # /work/task_dir/task.yaml, which is right in there and exists on + # no host -- recording THAT made a detached grade of this row + # rebuild the task around an unresolvable path and silently mount + # no task dir. Absent -> the container falls back to its own path, + # so an older host keeps today's behaviour. + "host_task_file": str(self.rt.task_file) if self.rt.task_file else None, # Docker WORKDIR alignment: concrete path the in-container # orchestrator runs at + captures out (None = standard workspace). "workspace_dir": self._workspace_dir, } ) await asyncio.to_thread((input_dir / "context.json").write_text, context_payload, encoding="utf-8") + if self.prior_result is not None: + # The row being graded, carried in whole. The container seeds from + # it exactly as the host path does, so the trajectory an `llm_judge` + # or `command_executed` criterion reads is the ORIGINAL run's. + await asyncio.to_thread( + (input_dir / PRIOR_RESULT_FILENAME).write_text, + self.prior_result.model_dump_json(indent=2), + encoding="utf-8", + ) async def _stream_container_output(self, proc: asyncio.subprocess.Process, log_fh: TextIO) -> int: """Stream the container's stdout, returning its exit code. @@ -878,8 +999,48 @@ async def _parse_result_or_raise(self, output_dir: Path, returncode: int, log_pa raise await self._handle_malformed_task_json(task_json, log_path, exc) from exc self._warn_on_version_mismatch(result) self._assert_grade_honored(result, task_json) + self._assert_regrade_honored(result, task_json) return result + def _assert_regrade_honored(self, result: EvaluationResult, task_json: Path | None = None) -> None: + """Fail loudly when a detached GRADE came back as a fresh agent run. + + Exactly the sibling of :meth:`_assert_grade_honored`, for exactly the + same reason one release later. ``regrade`` crosses the boundary only + through ``context.json``; an image that predates container-side grading + ignores the unknown key, ignores the staged ``prior.json``, ignores the + ``/work/workspace`` mount, and falls through to the ordinary + ``Orchestrator`` branch -- which **starts an agent** from + ``initial_prompt``. + + Nothing else catches it. ``_warn_on_version_mismatch`` only warns (and is + skipped entirely for ``dockerfile_path`` tasks), and + ``_assert_grade_honored`` early-returns because a grading container is + dispatched with ``grade=True``. So the host would fold a fabricated + trajectory back over the recorded row as its "grade" -- publishing a + verdict for work it never looked at, and billing the model for it. + + Keyed on EVIDENCE, like its sibling: a container that honored the request + seeds from ``prior`` and never runs the agent, so the trajectory it + returns is the one we sent in. A DIFFERENT ``started_at`` is the tell -- + ``_seed_from_prior_result`` restores the agent run's ``started_at`` + verbatim (deliberately, so a re-graded row does not report the grading + pass's 2 seconds into ``average_duration``), so a fresh run is the only + way that field can move. + """ + if self.prior_result is None: + return + if result.started_at == self.prior_result.started_at: + return + _quarantine_record(task_json, ".rerun", "re-run") + raise DockerRunError( + "Grading asked the container to score an already-executed run, but it returned a " + + f"different trajectory (started_at {result.started_at} vs the recorded " + + f"{self.prior_result.started_at}). The runtime image predates container-side " + + "grading and re-ran the agent instead; rebuild or pull a matching agent image, " + + "or grade on the host with --allow-host-grading." + ) + def _assert_grade_honored(self, result: EvaluationResult, task_json: Path | None = None) -> None: """Fail loudly when `execute` came back with a graded verdict. @@ -912,15 +1073,7 @@ def _assert_grade_honored(self, result: EvaluationResult, task_json: Path | None result.final_status.is_execution_fact or result.final_status is FinalStatus.NOT_GRADED ): return - if task_json is not None: - # Same sidecar pattern as `_handle_malformed_task_json`. Best-effort: - # a failed move is logged, never masking the raise below. - sidecar = task_json.with_suffix(task_json.suffix + ".graded") - try: - os.replace(task_json, sidecar) - logger.warning("Quarantined the refused graded record to %s", sidecar) - except OSError as exc: - logger.warning("Could not quarantine %s: %s", task_json, exc) + _quarantine_record(task_json, ".graded", "graded") raise DockerRunError( "`coder-eval execute` asked the container not to grade, but it returned " + f"{result.final_status.value} with {len(result.success_criteria_results)} criterion " @@ -971,7 +1124,7 @@ async def _record_build_failure(self, exc: DockerBuildError) -> None: """ try: await asyncio.to_thread(self.rt.run_dir.mkdir, parents=True, exist_ok=True) - log_path = self.rt.run_dir / "docker.log" + log_path = self.rt.run_dir / DOCKER_LOG_FILENAME await asyncio.to_thread(log_path.write_text, exc.build_log or str(exc), encoding="utf-8") await self._write_synthetic_task_json( self.rt.run_dir / TASK_JSON_FILENAME, exc, status=FinalStatus.BUILD_FAILED @@ -1483,7 +1636,7 @@ def _build_argv( # isolation. The Codex agent reads this to fall back to its full-access # sandbox: Codex's Landlock-backed read-only / workspace-write sandboxes # can't initialize inside a container and otherwise fail writes silently. - argv += ["--env", "CODER_EVAL_IN_CONTAINER=1"] + argv += ["--env", f"{IN_CONTAINER_ENV}=1"] # Hard-disable telemetry INSIDE the container. The app ships a baked-in # default connection string, so without this the in-container orchestrator @@ -1507,6 +1660,16 @@ def _build_argv( if self._task_dir_mount_src is not None: argv += ["-v", f"{self._task_dir_mount_src}:{CONTAINER_TASK_DIR}"] + # DETACHED GRADE: the already-executed workspace, read-WRITE and NOT a + # copy. Read-write because criteria legitimately mutate what they grade + # (a `run_command` that compiles, a post_run that cleans up), and the + # real tree because copying is what the host path proved wrong — + # `_setup_template` filters out node_modules / dist / build / .venv, so a + # criterion reading those would fail as a copying artifact rather than as + # a verdict. + if self.grade_workspace is not None: + argv += ["-v", f"{self.grade_workspace.resolve()}:{CONTAINER_GRADE_WORKSPACE}"] + # ANTI-CHEAT: the reference solution normally lives INSIDE the task dir, # so the symmetric mount above would hand the agent the answer via # `$TASK_DIR/`. Two things close that: diff --git a/src/coder_eval/models/__init__.py b/src/coder_eval/models/__init__.py index 479df22b..40326bee 100644 --- a/src/coder_eval/models/__init__.py +++ b/src/coder_eval/models/__init__.py @@ -23,11 +23,13 @@ # Container paths (leaf constants; re-exported so consumers obey CE001) from coder_eval.models.container_paths import ( + CONTAINER_GRADE_WORKSPACE, CONTAINER_INPUT_DIR, CONTAINER_OUTPUT_DIR, CONTAINER_REFERENCE_DIR, CONTAINER_TASK_DIR, CONTAINER_WORK_DIR, + IN_CONTAINER_ENV, REFERENCE_DIR_TOKEN, RESERVED_CONTAINER_DIRS, TASK_DIR_TOKEN, @@ -286,7 +288,9 @@ "DockerBuildConfig", "CONTAINER_INPUT_DIR", "CONTAINER_OUTPUT_DIR", + "CONTAINER_GRADE_WORKSPACE", "CONTAINER_REFERENCE_DIR", + "IN_CONTAINER_ENV", "CONTAINER_TASK_DIR", "REFERENCE_DIR_TOKEN", "TASK_DIR_TOKEN", diff --git a/src/coder_eval/models/container_paths.py b/src/coder_eval/models/container_paths.py index 6a592377..1473b84f 100644 --- a/src/coder_eval/models/container_paths.py +++ b/src/coder_eval/models/container_paths.py @@ -70,6 +70,17 @@ def command_uses_token(command: str, token: str) -> bool: return False +# The one reliable "am I inside a task container?" signal, set by DockerRunner +# on every container it starts. +# +# Every gate that means "in a container" MUST key on this and never on +# `sandbox.driver`: the in-container entry point rewrites `docker` -> `tempdir` +# before building its Orchestrator, so a driver-based test reads a value that has +# already been changed — which would silently disable the reference-permission +# window on exactly the path that needs it (regression-guarded by +# TestSandboxDriverGate). +IN_CONTAINER_ENV = "CODER_EVAL_IN_CONTAINER" + CONTAINER_WORK_DIR = "/work" CONTAINER_INPUT_DIR = "/work/input" CONTAINER_OUTPUT_DIR = "/work/output" @@ -82,6 +93,17 @@ def command_uses_token(command: str, token: str) -> bool: # cannot read the solution (see ``fs_permissions.py``). CONTAINER_REFERENCE_DIR = "/work/references" +# Where a DETACHED GRADE mounts the already-executed workspace it is grading. +# Only ever present on a grading container (`evaluate` / `run --resume` over a +# `driver: docker` row); a normal run never mounts it. +# +# It is a separate mount from CONTAINER_OUTPUT_DIR because the two belong to +# different runs: the grading pass writes its own `task.json` into its own fresh +# run directory (which the host then folds back into the row, preserving +# `task.execute.json`), while the workspace under evaluation belongs to the +# ORIGINAL run and must be adopted, never written over. +CONTAINER_GRADE_WORKSPACE = "/work/workspace" + # Paths a task's WORKDIR must never collide with: the container root and every # framework-owned mount under /work. Consumed by SandboxConfig's working_dir # validator (models/sandbox.py) and re-asserted host-side in docker_runner. @@ -93,5 +115,6 @@ def command_uses_token(command: str, token: str) -> bool: CONTAINER_OUTPUT_DIR, CONTAINER_TASK_DIR, CONTAINER_REFERENCE_DIR, + CONTAINER_GRADE_WORKSPACE, } ) diff --git a/src/coder_eval/orchestration/evaluation.py b/src/coder_eval/orchestration/evaluation.py index 4bad1815..eca96a85 100644 --- a/src/coder_eval/orchestration/evaluation.py +++ b/src/coder_eval/orchestration/evaluation.py @@ -20,7 +20,7 @@ import shutil from pathlib import Path -from ..models import CONTAINER_REFERENCE_DIR, TaskDefinition +from ..models import CONTAINER_REFERENCE_DIR, IN_CONTAINER_ENV, TaskDefinition from ..path_utils import REFERENCE_COPY_IGNORE, ignore_patterns_and_symlinks @@ -78,7 +78,7 @@ def resolve_reference_dir(task: TaskDefinition, task_file: Path | None) -> Path # invisible — wrong reference content, wrong reference_comparison scores, # wrong judge prompts, no error. container_mount = Path(CONTAINER_REFERENCE_DIR) - if os.environ.get("CODER_EVAL_IN_CONTAINER") == "1": + if os.environ.get(IN_CONTAINER_ENV) == "1": if container_mount.is_dir(): logger.debug("Reference resolved from the container mount at %s", container_mount) return container_mount diff --git a/src/coder_eval/orchestration/regrade.py b/src/coder_eval/orchestration/regrade.py index 962a1680..70526d3f 100644 --- a/src/coder_eval/orchestration/regrade.py +++ b/src/coder_eval/orchestration/regrade.py @@ -17,18 +17,32 @@ from __future__ import annotations import asyncio +import contextlib import logging +import os +import shutil +import tempfile from functools import cache from pathlib import Path +from typing import TypedDict from coder_eval.models import ( + IN_CONTAINER_ENV, EvaluationResult, PreservationMode, + ResolvedTask, SandboxConfig, TaskConfigRecord, TaskDefinition, ) -from coder_eval.path_utils import PRE_GRADE_JSON_FILENAME, TASK_JSON_FILENAME, write_text_atomic +from coder_eval.path_utils import ( + DOCKER_LOG_FILENAME, + GRADE_DOCKER_LOG_FILENAME, + GRADE_LOG_FILENAME, + PRE_GRADE_JSON_FILENAME, + TASK_JSON_FILENAME, + write_text_atomic, +) from coder_eval.sandbox import Sandbox @@ -59,7 +73,8 @@ def task_from_prior( run_dir: Path, *, allow_recorded_commands: bool = False, - include_setup_phase: bool = True, + grade_in_place: bool = False, + allow_host_grading: bool = False, ) -> tuple[TaskDefinition, str]: """Rebuild the executed task from the run's own recorded config. @@ -75,6 +90,20 @@ def task_from_prior( ``allow_recorded_commands`` gates the shell half. See :func:`check_embedded_commands`. + + ``grade_in_place`` is the ONE lever that selects which capability families + the gate discloses, and it is deliberately one parameter rather than two. + It shipped beside an ``include_setup_phase`` that every caller passed as its + exact complement -- two names for one fact, with nothing rejecting the + incoherent pairings. That is not cosmetic here: the flag gates a SECURITY + disclosure, so a future caller that set one and forgot the other would drop + the container-dispatch half of the untrusted-config gate with nothing + failing. Both derived values are computed once, in + :func:`_gate_scope_for_grade`. + + ``allow_host_grading`` participates only through that derivation: with the + flag set, no container is dispatched, so naming one in the consent prompt + would ask the operator to approve something that never runs. """ record = prior.task_config if record is None: @@ -86,14 +115,55 @@ def task_from_prior( task = TaskDefinition.model_validate(record.resolved) except ValueError as e: return _fall_back_to_source( - record, run_dir, e, allow_recorded_commands=allow_recorded_commands, include_setup_phase=include_setup_phase + record, + run_dir, + e, + allow_recorded_commands=allow_recorded_commands, + grade_in_place=grade_in_place, + allow_host_grading=allow_host_grading, ) check_embedded_commands( - task, run_dir, allow_recorded_commands=allow_recorded_commands, include_setup_phase=include_setup_phase + task, + run_dir, + allow_recorded_commands=allow_recorded_commands, + # The recorded source path, so the prompt can name the task DIRECTORY + # the dispatch copies in. It is the same untrusted value + # `_grade_in_container` resolves the image from. + task_file=Path(record.source_file) if record.source_file else None, + **_gate_scope_for_grade(task, grade_in_place=grade_in_place, allow_host_grading=allow_host_grading), ) return task, record.source_yaml +class _GateScope(TypedDict): + include_setup_phase: bool + include_container_dispatch: bool + + +def _gate_scope_for_grade(task: TaskDefinition, *, grade_in_place: bool, allow_host_grading: bool) -> _GateScope: + """Which capability families the untrusted-config gate must disclose. + + Both answers follow from ``grade_in_place``, which is why this is one + function rather than two arguments threaded past each other: + + * ``include_setup_phase`` is ``not grade_in_place``. ``pre_run`` and the + sandbox's own provisioning exist only on the ``--copy`` path; ``adopt`` + runs no installer and the orchestrator skips ``pre_run``, so in place they + are not a capability the run dir has. + * ``include_container_dispatch`` needs ``grade_in_place`` too, since ``--copy`` + is refused by ``grading_sandbox_config`` before it could dispatch anything + -- naming the image there would be a refusal for something that never runs. + + The pair is computed at ONE site so the two consumers (``task_from_prior`` + and its source-YAML fallback) cannot drift. + """ + return { + "include_setup_phase": not grade_in_place, + "include_container_dispatch": grade_in_place + and _should_grade_in_container(task, allow_host_grading=allow_host_grading), + } + + @cache def _operator_baseline_post_run() -> frozenset[str]: """``post_run`` commands the GRADER's own default experiment gives every task. @@ -123,7 +193,13 @@ def _operator_baseline_post_run() -> frozenset[str]: return frozenset(c.command for c in defaults.post_run) -def embedded_commands(task: TaskDefinition, *, include_setup_phase: bool = True) -> list[str]: +def embedded_commands( + task: TaskDefinition, + *, + include_setup_phase: bool = True, + include_container_dispatch: bool = False, + task_file: Path | None = None, +) -> list[str]: """Every shell command a rebuilt task definition would run on this host. ``include_setup_phase`` covers the two capability families that exist only on @@ -161,6 +237,23 @@ def embedded_commands(task: TaskDefinition, *, include_setup_phase: bool = True) shared run directory whose criteria were all ``file_exists`` sailed through the gate and still ran installers of the attacker's choosing. + ``include_container_dispatch`` is the same omission again, one layer up, and + it was reintroduced by the very change that made a docker row gradable. When + a ``driver: docker`` row is graded, the grade is DISPATCHED INTO A CONTAINER + built from the recorded ``sandbox.docker`` block -- so the record chooses the + image that runs on this host, with the default credential allowlist + (``ANTHROPIC_API_KEY``, ``UIPATH_ACCESS_TOKEN``, ``AWS_BEARER_TOKEN_BEDROCK`` + ...) forwarded into it, a writable copy of ``~/.claude``, and a pinned + ``--entrypoint`` the image itself supplies. That is arbitrary code execution + from a shareable artifact, and it is a strictly WIDER capability than the + ``run_command`` strings this gate already refuses. It reached the host + unprompted because the scan walked only ``success_criteria`` and ``post_run`` + -- the identical blind spot described in the paragraph above, which is the + argument for naming it here rather than trusting the next reader to notice. + + Like ``post_run``, it is a capability of the IN-PLACE path (the default for a + run directory), so it cannot hide behind ``include_setup_phase``. + ``isinstance`` narrowing, never ``getattr(c, "command", None)``: an untyped string probe over a discriminated union is invisible to pyright, so renaming a field silently degrades the only guard on this path to a permanent no-op — @@ -210,11 +303,97 @@ def embedded_commands(task: TaskDefinition, *, include_setup_phase: bool = True) for source in sandbox.template_sources or []: if isinstance(source, RepoSource): commands.append(f"git clone -- {source.url}") + if include_container_dispatch: + commands += _container_dispatch_commands(task, task_file) return commands +def _container_dispatch_commands(task: TaskDefinition, task_file: Path | None) -> list[str]: + """The container dispatch, rendered as the ONE shell command it is. + + Every string this returns is a command, because that is what the caller + promises: :func:`check_embedded_commands` joins the list with ``"; "`` and + interpolates ``len(commands)`` into the consent prompt. The first version + appended argv FRAGMENTS as separate entries, so a `dockerfile_path` task with + two build args and one mount asked the operator to approve "4 shell + command(s)" reading ``docker build -f Dockerfile; --build-arg FOO=bar; + --build-arg BAZ=qux; -v /a:/b`` -- one docker invocation described as four + commands, three of which are not commands. The consent prompt is the one + place this text has to be exact. + + It also names every HOST PATH the dispatch exposes, not just the ones under + ``sandbox.docker``. Three families reach the record-named image without the + record ever mentioning them in a ``docker`` block: + + * the TASK DIRECTORY, copied wholesale from the recorded ``source_file``'s + parent (``DockerRunner._prepare_task_dir_mount``) -- a record whose + ``source_file`` is ``~/.ssh/config`` copies all of ``~/.ssh`` in; + * every ``agent.plugins[].path``, every ``TemplateDirSource.path`` and + ``agent.system_prompt_file``, auto-mounted read-only at their host paths by + ``_build_argv``. ``_sensitive_source_paths`` only *warns* about a fixed + list of these, and this module's own gate docstring states the governing + principle: a warning is not a control, because it prints as the command is + already being prepared; + * a WRITABLE copy of ``~/.claude``, ``.credentials.json`` included. + + Networking defaults to ``--network bridge``, so anything the container can + read it can also send. Disclosing only ``sandbox.docker.*`` would ask the + operator to consent to a strict subset of what actually happens. + """ + docker = task.sandbox.docker + parts: list[str] = [] + if docker.dockerfile_path: + # `docker build` runs every RUN step in the recorded Dockerfile on this + # host, and expands recorded build args against the GRADER's environment, + # so a `${ANTHROPIC_API_KEY}` arg is exfiltratable by a RUN step. + # `extra_args` is spliced into the argv unfiltered. + parts.append(f"docker build -f {docker.dockerfile_path}") + parts += [f"--build-arg {key}={value}" for key, value in docker.build.args.items()] + parts += [f"--secret {spec}" for spec in docker.build.secrets] + parts += list(docker.build.extra_args) + parts.append("&& docker run ") + else: + parts.append(f"docker run {docker.image}") + parts += [f"-v {mount}" for mount in docker.extra_mounts or []] + parts += [f"-v {path}" for path in _dispatch_host_exposure(task, task_file)] + parts += [f"--env {name}" for name in docker.env_passthrough_extra or []] + parts.append("(with your credentials in its environment and a writable copy of ~/.claude)") + return [" ".join(parts)] + + +def _dispatch_host_exposure(task: TaskDefinition, task_file: Path | None) -> list[str]: + """Host paths the grading container receives that no ``docker`` field names. + + Mirrors ``DockerRunner._prepare_task_dir_mount`` and the ``_auto_mount`` + block of ``_build_argv``. Rendered as strings rather than resolved Paths: + this is disclosure text, and an unresolvable entry is still worth naming. + """ + from coder_eval.models import TemplateDirSource + + exposed: list[str] = [] + if task_file is not None: + exposed.append(f"{task_file.parent} (the recorded task directory, copied in)") + agent = task.agent + for plugin in (agent.plugins if agent else None) or []: + path = plugin.get("path") if isinstance(plugin, dict) else None + if path: + exposed.append(str(path)) + for source in task.sandbox.template_sources or []: + if isinstance(source, TemplateDirSource): + exposed.append(source.path) + if agent is not None and agent.system_prompt_file: + exposed.append(str(agent.system_prompt_file)) + return exposed + + def check_embedded_commands( - task: TaskDefinition, run_dir: Path, *, allow_recorded_commands: bool, include_setup_phase: bool = True + task: TaskDefinition, + run_dir: Path, + *, + allow_recorded_commands: bool, + include_setup_phase: bool = True, + include_container_dispatch: bool = False, + task_file: Path | None = None, ) -> None: """Refuse — or at minimum name — the shell a rebuilt config will run here. @@ -233,7 +412,12 @@ def check_embedded_commands( Passing the task file explicitly (``evaluate ``) also bypasses this: that config came from the operator, not from the artifact. """ - commands = embedded_commands(task, include_setup_phase=include_setup_phase) + commands = embedded_commands( + task, + include_setup_phase=include_setup_phase, + include_container_dispatch=include_container_dispatch, + task_file=task_file, + ) if not commands: return rendered = "; ".join(commands) @@ -259,7 +443,8 @@ def _fall_back_to_source( e: ValueError, *, allow_recorded_commands: bool, - include_setup_phase: bool = True, + grade_in_place: bool = False, + allow_host_grading: bool = False, ) -> tuple[TaskDefinition, str]: """The loud source-YAML fallback for a resolved config that no longer validates.""" from .task_loader import load_task @@ -278,7 +463,11 @@ def _fall_back_to_source( ) task, source_yaml = load_task(Path(record.source_file)) check_embedded_commands( - task, run_dir, allow_recorded_commands=allow_recorded_commands, include_setup_phase=include_setup_phase + task, + run_dir, + allow_recorded_commands=allow_recorded_commands, + task_file=Path(record.source_file), + **_gate_scope_for_grade(task, grade_in_place=grade_in_place, allow_host_grading=allow_host_grading), ) return task, source_yaml @@ -492,12 +681,19 @@ def restore_pre_grade_record(run_dir: Path) -> bool: def grading_sandbox_config(task: TaskDefinition, *, allow_host_grading: bool = False) -> SandboxConfig: """The sandbox config a grading pass runs under. - Grading never runs a container: the docker driver dispatches through - DockerRunner, which needs an agent. So a ``driver: docker`` task can only be - graded on the host — and that is a DIFFERENT machine from the one its - criteria were written against. + This is the HOST-grading config, and reaching it with ``driver: docker`` means + the container route was declined. A docker row is normally graded IN a + container of its own image (:func:`_should_grade_in_container`), which is + dispatched before this function is called; what is left here is the ``--copy`` + path, which cannot adopt a container workspace, and the two-argument + ``evaluate `` form. - It is therefore refused rather than downgraded. A container task's criteria + (This docstring once opened "grading never runs a container: the docker + driver dispatches through DockerRunner, which needs an agent". That premise + was simply wrong — a grading pass needs no agent — and it is the reason the + refusal below survived for a release after it stopped being the only answer.) + + Grading a container task on the host is refused rather than downgraded. A container task's criteria address container paths (``/verifier``, ``/logs/verifier``) and container toolchains; run on the host they score 0.0 for a trajectory ``run`` scored 1.0, and the row is written back FAILURE. The same commands (``rm -rf @@ -519,12 +715,15 @@ def grading_sandbox_config(task: TaskDefinition, *, allow_host_grading: bool = F return task.sandbox.model_copy(deep=True) if not allow_host_grading: raise RegradeError( - f"Task {task.task_id!r} ran under `driver: docker`, and grading cannot start a container " - + "(there is no agent to run in it). Grading on the host would execute this task's " - + "criteria against a filesystem that lacks the container's paths and toolchain, " - + "scoring a FAILURE for a run that passed — and would run its shell commands " - + "unsandboxed here. Re-run with --allow-host-grading to accept that, or grade on a " - + "machine that reproduces the container." + f"Task {task.task_id!r} ran under `driver: docker`, so it must be graded in a container " + + "of its own image — but this grading path cannot dispatch one. Grading on the host " + + "would execute this task's criteria against a filesystem that lacks the container's " + + "paths and toolchain, scoring a FAILURE for a run that passed, and would run its " + + "shell commands unsandboxed here.\n" + + "Grade a run directory in place (the default) to get a container, or pass " + + "--allow-host-grading to accept host grading anyway.\n" + + "(The two-argument `evaluate ` form has no container route at all, " + + "so --allow-host-grading is the only way forward there.)" ) logger.warning( "Grading %r on the host: its `driver: docker` sandbox cannot be reproduced here, so " @@ -547,6 +746,351 @@ def stamp_host_grading(result: EvaluationResult, task: TaskDefinition) -> None: result.environment_info["graded_on_host"] = True +def _should_grade_in_container(task: TaskDefinition, *, allow_host_grading: bool) -> bool: + """Whether this grade belongs in a container of the task's own image. + + Three conditions, and each rules out a different wrong answer: + + * ``driver: docker`` — a tempdir task has no container to grade in. + * NOT already inside one. Gated on ``CODER_EVAL_IN_CONTAINER``, never on the + driver, for the same reason the reference-permission window is: the + in-container entry point rewrites `docker` -> `tempdir` before building its + Orchestrator, so a driver-based test would be reading a value that has + already been changed. Without this, a grading container would try to + dispatch a grading container. + * ``--allow-host-grading`` not passed. That flag is the operator saying + "grade it here anyway" — the escape hatch for a machine with no docker, or + for criteria known to be host-portable — and it must keep winning, since + the row it produces is stamped ``graded_on_host`` and is therefore honest + about what it is. + """ + return task.sandbox.driver == "docker" and not allow_host_grading and os.environ.get(IN_CONTAINER_ENV) != "1" + + +def _fold_back_container_logs(container_run_dir: Path, run_dir: Path) -> None: + """Rescue the grading container's logs from the scratch dir before it dies. + + Called on BOTH the success and the failure path, and the failure path is the + one that makes it necessary. ``_grade_in_container`` runs the whole dispatch + inside a ``TemporaryDirectory``, and every ``DockerRunner`` diagnostic is + written into it: ``docker.log`` (the container's merged stdout+stderr, where + the in-container FATAL guards land, since ``_grade_in_container`` passes no + ``stream_callback`` and nothing is echoed), the captured build log a failed + ``docker build`` persists there, and the synthetic ``BUILD_FAILED`` / + ``ERROR`` records. Folding out only on success deleted precisely the evidence + -- and DockerRunError's own text says ``See {log_path} for container + output``, naming a path that no longer existed by the time it was printed. + + ``grade.log`` is the grading pass's OWN log, written by the in-container + orchestrator (``task_log_path(run_dir, regrade=True)``) and holding the + per-criterion detail -- ``run_command`` stdout/stderr, judge prompts and + verdicts -- which is the only durable record of WHY a criterion scored what + it did. It is a documented part of the run-directory contract, so a + ``driver: docker`` row must not be the one shape that silently lacks it. + + Best-effort throughout: a side-car log is not the verdict, and this runs + where an exception is already in flight. + """ + for name, dest_name in ((DOCKER_LOG_FILENAME, GRADE_DOCKER_LOG_FILENAME), (GRADE_LOG_FILENAME, GRADE_LOG_FILENAME)): + # `docker.log` is renamed for the PHASE: on the resume path that name is + # already taken by the executed container's log, and overwriting it would + # repeat the task.log/grade.log truncation bug one layer down. + # `grade.log` does not collide -- a detached grade is the only thing that + # ever writes it into that directory -- so it keeps its name. + source = container_run_dir / name + if not source.is_file(): + continue + # The destination may not exist yet on the FAILURE path -- the verdict + # fold-back, which creates it, never ran. + with contextlib.suppress(OSError): + run_dir.mkdir(parents=True, exist_ok=True) + dest = run_dir / dest_name + if dest.is_symlink(): + # `shutil.copy2` opens the destination for writing, which FOLLOWS a + # symlink there -- an arbitrary-file-overwrite primitive in a run + # directory the grader did not create (`run --resume` passes the + # executed row's own directory; `--run-dir` can point anywhere). The + # sibling verdict write goes through `write_text_atomic` precisely + # for this, and the `suppress(OSError)` below would have made the + # redirect leave no trace at all. + logger.warning("Refusing to write %s: it is a symlink.", dest) + continue + with contextlib.suppress(OSError): + shutil.copy2(source, dest) + + +def _fold_back_container_grade(container_run_dir: Path, run_dir: Path, task_id: str) -> None: + """Copy the grading container's record into the row the caller asked about. + + The container writes into a scratch directory it alone owns (see + :func:`_grade_in_container`), so the graded ``task.json`` has to be moved to + where the caller expects it; its logs come along via + :func:`_fold_back_container_logs`. + + Mandatory on the record: a grade that cannot write its verdict is a failure. + But it must fail as a ``RegradeError``, not as a raw ``OSError``. This call + sits outside the dispatch ``try``, so an unwrapped ``OSError`` reached the two + callers differently and both outcomes were wrong: ``evaluate`` guards only + ``RegradeError``, so it escaped into Typer as a stack trace *after* a grade + that had already succeeded; ``run --resume`` catches ``OSError`` too, so it + folded the row back with its ORIGINAL ungraded result and an error reading + "Grading failed during --resume" -- reporting a computed, correct verdict as + a grading failure and discarding it. + """ + graded = container_run_dir / TASK_JSON_FILENAME + if not graded.is_file(): + # `_parse_result_or_raise` already raised in this case; if we are here + # the runner returned a result, so the file exists. Guard anyway rather + # than raise a confusing FileNotFoundError from the copy. + return + try: + run_dir.mkdir(parents=True, exist_ok=True) + write_text_atomic(run_dir / TASK_JSON_FILENAME, graded.read_text(encoding="utf-8")) + except OSError as e: + raise RegradeError( + f"Graded {task_id!r} in a container, but could not write the verdict to " + + f"{run_dir / TASK_JSON_FILENAME}: {e}. The grade itself succeeded; re-run once the " + + "destination is writable." + ) from e + _fold_back_container_logs(container_run_dir, run_dir) + + +def _stamp_container_grading(result: EvaluationResult, task: TaskDefinition) -> None: + """Record the container grade's known equivalence gaps ON the row. + + The sibling of :func:`stamp_host_grading`, and written for the reason that + function's own docstring gives: a console warning does not travel with + ``task.json`` into ``run.json``, the reports or the evalboard, so a row it + describes cannot be filtered out of a comparison by anything downstream. + + ``graded_without_pre_run`` is the count of ``pre_run`` commands that ran in + the container which executed the agent and were NOT re-run here. It is not a + hypothetical: three of the ten in-tree ``driver: docker`` tasks seed state + outside the workspace in ``pre_run`` (``3d-scan-calc`` symlinks + ``/root/mass_report.json``, and its verifier's first assertion is that the + path exists), so such a row scores 0.000 for a trajectory ``coder-eval run`` + scores 1.000. Refusing outright was considered and declined: it would make + ``run --resume`` fail on rows it grades correctly today whenever ``pre_run`` + happens to touch only the workspace, which is the common case. A durable, + machine-readable marker lets a consumer decide, which a refusal does not. + + ``graded_with_rebuilt_image`` marks the other gap: the grading pass re-ran + ``docker build`` under the run's deterministic tag, so the image is only as + stable as the Dockerfile and its base were between the two phases. + """ + if task.pre_run: + result.environment_info["graded_without_pre_run"] = len(task.pre_run) + if task.sandbox.docker.dockerfile_path: + result.environment_info["graded_with_rebuilt_image"] = str(task.sandbox.docker.dockerfile_path) + + +def _emit_task_telemetry(result: EvaluationResult, *, variant_id: str) -> None: + """Emit the ``Task.End`` event the grading container could not. + + Every container this repo starts is launched with ``TELEMETRY_ENABLED=false`` + (``DockerRunner._build_argv``), whose comment states the invariant verbatim: + "container silent, host emits once". The RUN path supplies the host half in + ``orchestration/batch.py`` right after parsing the container's result; the + grading path inherited the silent half and had no counterpart, so a verdict + published by ``evaluate `` or ``run --resume`` over a + ``driver: docker`` row reached no usage telemetry at all -- while the + byte-identical operation on a ``tempdir`` row, or the same row with + ``--allow-host-grading``, did. A driver-dependent hole in the metric the + split exists to keep at parity with ``run``. + + Non-fatal like every other emission site (CE019): telemetry must never be + the reason a computed verdict is lost. + """ + from coder_eval.orchestrator import build_task_event + from coder_eval.telemetry import track_event + + name, props = build_task_event(result, driver="docker", variant_id=variant_id) + track_event(name, props) + + +async def _grade_in_container( + *, + task: TaskDefinition, + prior: EvaluationResult, + workspace: Path, + run_dir: Path, + task_file: Path | None, + source_yaml: str, + variant_id: str, + replicate_index: int, +) -> EvaluationResult: + """Grade ``workspace`` inside a container built from ``task``'s own image. + + The container gets two separate mounts, and keeping them separate is the + point: ``run_dir`` (this GRADING pass's fresh directory) at the standard + output location, and ``workspace`` (the ORIGINAL run's output) at + ``CONTAINER_GRADE_WORKSPACE``. The grade writes its ``task.json`` into the + former, which the caller then folds back into the row — preserving + ``task.execute.json`` exactly as on the host path — while the latter is + adopted and never written over. + + ``task_file`` is required, and must EXIST here. The image is built or named + by the task's own sandbox config, and DockerRunner resolves the Dockerfile + and reference directory relative to the task file; without one there is + nothing to build from. + + Testing only for ``None`` was not enough, and failed on exactly the rows this + guard was written for. A ``driver: docker`` run records its ``source_file`` + from the IN-CONTAINER orchestrator, so the value is + ``/work/task_dir/task.yaml`` — a real, non-``None`` ``Path`` that does not + exist on the grading host. The guard was skipped, and the failure then went + QUIET where it matters: ``_prepare_task_dir_mount`` does ``if not + source.is_dir(): return``, so the grading container got no ``TASK_DIR`` mount + at all and any ``$TASK_DIR`` criterion silently resolved against a different + tree than during the run — a wrong verdict with no error anywhere. + + Requiring the file to exist also closes a second hole: on the detached path + ``task_file`` comes straight from the untrusted record, and its PARENT is + what ``_prepare_task_dir_mount`` copies into the container. A recorded + ``source_file`` of ``~/.ssh/config`` would copy the whole of ``~/.ssh``. + Existence alone does not make the path trusted — that is what the + ``--allow-recorded-commands`` gate is for, and it now names the container + dispatch — but it removes the silent-wrong-verdict half. + """ + from coder_eval.isolation.docker_runner import DockerRunError, DockerRunner + + if task_file is None or not task_file.is_file(): + recorded = "no task file" if task_file is None else f"a task file that is not on this host ({task_file})" + raise RegradeError( + f"Task {task.task_id!r} ran under `driver: docker`, so grading it needs a container built " + + f"from its own image — but the run records {recorded} to resolve that image from. " + + "A container run records the path it saw INSIDE the container, which does not exist " + + "here. Pass the task file explicitly (`coder-eval evaluate `), or " + + "--allow-host-grading to grade here instead." + ) + + logger.info( + "Grading %r in a container of its own image: its criteria address container paths and " + + "toolchains, so the host cannot reproduce them.", + task.task_id, + ) + if task.pre_run: + # KNOWN EQUIVALENCE GAP, made loud because it cannot be closed here. + # + # This is a SECOND, fresh container. Only `workspace` crosses from the + # one that ran the agent; everything that container's `pre_run` did + # OUTSIDE the workspace is gone -- and `pre_run` is not re-run, because + # `Sandbox.adopt` sets `was_adopted` and the orchestrator skips it (it + # would otherwise overwrite the agent's deliverables before the criteria + # read them, which is the defect that skip exists for). + # + # It is not hypothetical: `tasks/samples/skillsbench/3d-scan-calc`'s + # pre_run does `ln -sfn "$PWD/mass_report.json" /root/mass_report.json` + # and its verifier's first assertion is that /root/mass_report.json + # exists. In a fresh container /root is pristine, so the row scores 0.000 + # for a trajectory `run` scores 1.000. + # + # Re-running pre_run here would trade this bug for the deliverable- + # clobbering one, so the honest move is to name it at dispatch and let + # the operator use --allow-host-grading or a single `run`. + logger.warning( + "Task %r declares %d pre_run command(s). They ran in the container that executed the " + + "agent and are NOT re-run here: this is a second container, and only the workspace " + + "crosses. A criterion that depends on state pre_run put OUTSIDE the workspace " + + "(a symlink in /root, an installed package, a started service) will score as a " + + "failure. If that is this task, grade it with a single `coder-eval run` instead.", + task.task_id, + len(task.pre_run), + ) + if task.sandbox.docker.dockerfile_path: + # The SECOND known equivalence gap, and the one the user guide already + # promised was warned about while nothing emitted it. + # + # `_build_image` re-runs `docker build` on every dispatch under the + # deterministic tag `coder-eval-task-:built`, so the grading image + # REPLACES the run's under the same name. A Dockerfile, build context or + # base image that moved between the two phases means the criteria read a + # different filesystem than the agent did, and the score changes for + # identical agent output. + # + # Nothing pins or records image identity on either side yet, so this + # cannot be detected after the fact -- which is exactly why it is said + # at dispatch. (A `reference_digest`-style pin on the resolved image is + # the real fix and is deliberately NOT attempted here; it needs the + # identity recorded on the RUN side too, which is a change to the run + # path rather than to grading.) + logger.warning( + "Task %r builds its image from %s, so this grading pass re-runs `docker build`. If the " + + "Dockerfile, its build context or its base image changed since the run, the criteria " + + "read a different filesystem than the agent did and the score may differ for identical " + + "agent output. Nothing records the image identity, so this cannot be detected " + + "afterwards -- grade with a single `coder-eval run` if the image may have moved.", + task.task_id, + task.sandbox.docker.dockerfile_path, + ) + # A SCRATCH output dir, never the caller's. The two callers disagree about + # what `run_dir` is -- `evaluate` passes a freshly prepared directory, while + # `run --resume` passes the executed row's OWN directory -- and every part of + # DockerRunner's result handling assumes an output dir it alone populates: + # + # * `_parse_result_or_raise` decides "did the container produce a result?" + # on `task_json.exists()` and discards `returncode`. Over the row's own + # directory the pre-grade `task.json` is already there, so a grading + # container that DIED (OOM, exit 137, or any of `_grade_recorded_run`'s + # own FATAL guards) was read back as a successful grade -- returning the + # stale ungraded row as the verdict, with the container's error discarded. + # * `run()` opens `run_dir/docker.log` with mode "w", truncating the + # executed container's log -- the same loss the task.log/grade.log split + # was introduced to prevent. + # * `grant_container_access(output_dir, writable=True)` would recursively + # widen the whole preserved artifacts tree. + # + # Giving the container a private directory makes both callers identical and + # makes the docstring above true, rather than true of one caller. + with tempfile.TemporaryDirectory(prefix="coder-eval-grade-") as scratch: + container_run_dir = Path(scratch) + rt = ResolvedTask( + task=task, + task_file=task_file, + run_dir=container_run_dir, + variant_id=variant_id, + source_yaml=source_yaml, + replicate_index=replicate_index, + ) + try: + result = await DockerRunner( + rt, + # The grading pass owns its scratch dir and nothing else. The + # workspace is a bind mount of the ORIGINAL run's output and must + # survive untouched. + preservation_mode=PreservationMode.NONE, + prior_result=prior, + grade_workspace=workspace, + ).run() + except (DockerRunError, OSError) as e: + # Wrapped, because `orchestration/` must not leak an isolation-layer + # exception to the CLI, and because the actionable next step is the + # host-grading escape hatch rather than a docker stack trace. OSError + # joins it because the staging copies (`_prepare_task_dir_mount`, + # `_prepare_reference_mount`) and the log open raise it unwrapped, + # and a raw traceback would drop the guidance below. + raise RegradeError( + f"Grading {task.task_id!r} in a container failed: {e}. The container's own output was " + + f"kept at {run_dir / GRADE_DOCKER_LOG_FILENAME}. Re-run with --allow-host-grading " + + "to grade on this machine instead (path- and toolchain-dependent criteria may then " + + "score differently, and the row is stamped graded_on_host)." + ) from e + finally: + # ALWAYS, not only on success: the scratch dir is deleted the moment + # this `with` exits, and everything that explains a failure lives in + # it. See `_fold_back_container_logs`. + _fold_back_container_logs(container_run_dir, run_dir) + # Fold the grade back into the row the caller asked about, mirroring what + # the host path does in place. `back_up_pre_grade_record` has already + # preserved task.execute.json, so this write is the graded record. + _fold_back_container_grade(container_run_dir, run_dir, task.task_id) + # Outside the `with`: the scratch dir has served its purpose, and both of + # these act on the returned result, which the caller writes back last. + _stamp_container_grading(result, task) + _emit_task_telemetry(result, variant_id=variant_id) + return result + + async def regrade_in_place( *, task: TaskDefinition, @@ -558,6 +1102,8 @@ async def regrade_in_place( variant_id: str, replicate_index: int = 0, allow_host_grading: bool = False, + recorded_task: TaskDefinition | None = None, + recorded_task_file: Path | None = None, ) -> EvaluationResult: """Run ``task``'s criteria against an already-executed ``workspace``. @@ -569,9 +1115,58 @@ async def regrade_in_place( ``prior`` supplies the trajectory and the run's execution facts (see ``Orchestrator._seed_from_prior_result``), so criteria that read the agent's tool calls score exactly as they would have during the run. + + ``recorded_task`` / ``recorded_task_file`` are the two halves of one seam: + what the row RECORDS, as distinct from what this process runs and resolves + paths against. Both matter only in the container, where the task is rewritten + to ``driver: tempdir`` and every path is a container path. Omitting the file + half made every container-graded row re-record ``/work/task_dir/task.yaml`` + as its ``source_file`` -- a path that exists on no host, which is the exact + defect ``Orchestrator.recorded_task_file`` was added to fix, reintroduced one + caller down. A later ``evaluate `` on such a row then hits + ``_grade_in_container``'s own "a task file that is not on this host" refusal. """ from coder_eval.orchestrator import Orchestrator + # A `driver: docker` row is graded INSIDE a container of the same image, + # which is the only place its criteria mean what they meant during the run. + # Dispatched before anything else here, including the reference check, so the + # container performs every step against container paths rather than having + # half of it done against the host's. + if _should_grade_in_container(task, allow_host_grading=allow_host_grading): + # `recorded_task` is NOT forwarded, and that is deliberate rather than an + # omission: the container re-derives it from the staged task.yaml (see + # `run_task_internal_command`'s `authored_task`), which IS this `task`. + # Accepting a DIFFERENT one and dropping it would leave no evidence, so + # say so instead -- the whole point of the seam is that the record must + # not quietly disagree with what was authored. + if recorded_task is not None and recorded_task != task: + raise RegradeError( + "recorded_task cannot be honored when grading in a container: the container rebuilds " + + "the recorded task from the staged task.yaml. Pass the same task, or grade with " + + "--allow-host-grading." + ) + if recorded_task_file is not None and recorded_task_file != task_file: + # Same rule as `recorded_task` above. The container receives this + # value over `context.json`'s `host_task_file`, which DockerRunner + # fills from `rt.task_file` -- i.e. from `task_file` here. A + # different one could not be honored, so say so rather than drop it. + raise RegradeError( + "recorded_task_file cannot be honored when grading in a container: the container " + + "records the host task file the dispatch forwards to it, which is `task_file`. " + + "Pass the same path, or grade with --allow-host-grading." + ) + return await _grade_in_container( + task=task, + prior=prior, + workspace=workspace, + run_dir=run_dir, + task_file=task_file, + source_yaml=source_yaml, + variant_id=variant_id, + replicate_index=replicate_index, + ) + # Inside the shared entry point, not at each caller: a guard a caller has to # remember is one a third caller will forget, and this one is the difference # between a verdict and a verdict against the wrong answer key. @@ -595,6 +1190,8 @@ async def regrade_in_place( source_yaml=source_yaml, replicate_index=replicate_index, prior_result=prior, + recorded_task=recorded_task, + recorded_task_file=recorded_task_file, ) result = await orchestrator.run() stamp_host_grading(result, task) diff --git a/src/coder_eval/orchestrator.py b/src/coder_eval/orchestrator.py index a8bbfe50..da273ccc 100644 --- a/src/coder_eval/orchestrator.py +++ b/src/coder_eval/orchestrator.py @@ -380,6 +380,7 @@ def __init__( grade: bool = True, prior_result: EvaluationResult | None = None, recorded_task: TaskDefinition | None = None, + recorded_task_file: Path | None = None, ): """Initialize the orchestrator. @@ -444,6 +445,17 @@ def __init__( # filesystem silently, which is the exact outcome that gate exists to # prevent. The record must describe the task as AUTHORED. self.recorded_task = recorded_task if recorded_task is not None else task + # Same seam, same reason, for the PATH. `task_file` is what this process + # resolves TASK_DIR and the reference against -- in a container that is + # `/work/task_dir/task.yaml`, which is correct here and meaningless + # anywhere else. Recording it made a container row's `source_file` name a + # path that exists on no host, so a later `evaluate ` rebuilt the + # task around it: the docker dispatch guard saw a non-None Path and let it + # through, and `_prepare_task_dir_mount` then silently mounted nothing + # (`if not source.is_dir(): return`), so every `$TASK_DIR` criterion + # resolved against the wrong tree and scored a verdict nobody could + # explain. The host forwards its own path for the record. + self.recorded_task_file = recorded_task_file if recorded_task_file is not None else task_file self.prior_result = prior_result # Derived paths @@ -1195,7 +1207,7 @@ def _finalize_result(self, start_time: float) -> None: self.result.task_config = TaskConfigRecord( resolved=self.recorded_task.model_dump(warnings=False), source_yaml=self.source_yaml, - source_file=str(self.task_file) if self.task_file else None, + source_file=str(self.recorded_task_file) if self.recorded_task_file else None, lineage=self.config_lineage, ) diff --git a/src/coder_eval/path_utils.py b/src/coder_eval/path_utils.py index b8bef1a2..46c97f54 100644 --- a/src/coder_eval/path_utils.py +++ b/src/coder_eval/path_utils.py @@ -30,6 +30,22 @@ # packages is how a rename becomes a silent no-op on the sites it missed. TASK_JSON_FILENAME = "task.json" PRE_GRADE_JSON_FILENAME = "task.execute.json" +# The already-executed row a DETACHED GRADE seeds from, staged into the grading +# container's read-only input mount. Never written by a run; only ever an input +# to `coder-eval evaluate` / `run --resume` over a `driver: docker` row. +PRIOR_RESULT_FILENAME = "prior.json" + +# The container's own stdout+stderr transcript, and the name it is folded back +# under after a GRADING container. Constants rather than literals for the reason +# CE053 states: the producer lives in ``isolation/`` and the consumer in +# ``orchestration/``, the fold-back is guarded by ``is_file()``, so a rename on +# the producing side would degrade the copy to a silent no-op and discard the +# only record of why a grading container failed. +DOCKER_LOG_FILENAME = "docker.log" +# Named for the PHASE. On the ``run --resume`` path ``docker.log`` is already +# taken by the executed container's log, and overwriting it would repeat the +# task.log/grade.log truncation bug one layer down. +GRADE_DOCKER_LOG_FILENAME = "grade.docker.log" # The virtualenv directory `setup` creates and `adopt` discovers. Named because # whether it is on PATH decides which binaries a criterion resolves. diff --git a/src/coder_eval/sandbox.py b/src/coder_eval/sandbox.py index a077934c..73201419 100644 --- a/src/coder_eval/sandbox.py +++ b/src/coder_eval/sandbox.py @@ -17,6 +17,7 @@ from .fs_permissions import RESTRICTED_MODE, set_permissions from .invocation_log import render_recorder from .models import ( + IN_CONTAINER_ENV, RECORD_CLI_DIR, RECORD_CLI_LOG, RepoSource, @@ -199,7 +200,7 @@ def enforces_permission_windows(self) -> bool: would read "tempdir" inside the container and silently disable the anti-cheat window on exactly the path that needs it. """ - return os.environ.get("CODER_EVAL_IN_CONTAINER") == "1" + return os.environ.get(IN_CONTAINER_ENV) == "1" def set_permissions( self, diff --git a/tests/lint/doc_env_parity.py b/tests/lint/doc_env_parity.py index e074c146..ba9e2752 100644 --- a/tests/lint/doc_env_parity.py +++ b/tests/lint/doc_env_parity.py @@ -69,6 +69,21 @@ _SRC_ENV_READ = re.compile(r"""(?:getenv\(\s*|environ(?:\.get\(\s*|\[\s*))['"]([A-Z][A-Z0-9_]{2,})['"]""") _SRC_ENV_VALUE = re.compile(r"""['"]([A-Z][A-Z0-9_]{2,})=[^'"]*['"]""") +# The same two shapes again, but reached through a NAMED CONSTANT rather than an +# inline literal. `CODER_EVAL_IN_CONTAINER` has a single definition +# (`models/container_paths.py::IN_CONTAINER_ENV`) and every consumer now spells it +# `os.environ.get(IN_CONTAINER_ENV)`, so a scanner that recognised only literals +# would report the repo's own gate as unbacked and push the author to paste the +# literal back -- the scanner arguing against the SSOT it should reinforce. +# +# Resolution is deliberately TWO-STEP, so this stays as strict as it was: a +# constant counts only when some module actually reads it by name. A bare +# `CONST = "CODER_EVAL_BOGUS"` that nothing consumes is still unbacked, which is +# the property `test_src_scan_requires_a_real_consumer_not_any_literal` pins. +_SRC_ENV_CONST_DEF = re.compile(r"""^([A-Z][A-Z0-9_]{2,})\s*(?::[^=\n]+)?=\s*['"]([A-Z][A-Z0-9_]{2,})['"]\s*$""", re.M) +_SRC_ENV_CONST_READ = re.compile(r"""(?:getenv\(\s*|environ(?:\.get\(\s*|\[\s*))([A-Z][A-Z0-9_]{2,})\b""") +_SRC_ENV_CONST_VALUE = re.compile(r"""f['"]\{([A-Z][A-Z0-9_]{2,})\}=[^'"]*['"]""") + def settings_env_names() -> set[str]: """Uppercased env names Settings actually reads: field names + AliasChoices.""" @@ -89,10 +104,18 @@ def src_env_literals(src_root: Path) -> set[str]: """Env-var names ``src/`` actually consumes: direct ``os.getenv``/``os.environ`` reads plus the NAME side of inline ``"NAME=VALUE"`` child-process literals.""" names: set[str] = set() + const_values: dict[str, str] = {} + const_reads: set[str] = set() for py in src_root.rglob("*.py"): text = py.read_text(encoding="utf-8") names.update(_SRC_ENV_READ.findall(text)) names.update(_SRC_ENV_VALUE.findall(text)) + const_values.update(dict(_SRC_ENV_CONST_DEF.findall(text))) + const_reads.update(_SRC_ENV_CONST_READ.findall(text)) + const_reads.update(_SRC_ENV_CONST_VALUE.findall(text)) + # Step two: a constant is backed only if it is BOTH defined as an env name and + # read somewhere. Defined-but-unread stays unbacked, exactly as before. + names.update(const_values[ident] for ident in const_reads & const_values.keys()) return names diff --git a/tests/lint/rules/ce052_process_lethal_must_be_container_gated.py b/tests/lint/rules/ce052_process_lethal_must_be_container_gated.py index 99ac6920..406bbe09 100644 --- a/tests/lint/rules/ce052_process_lethal_must_be_container_gated.py +++ b/tests/lint/rules/ce052_process_lethal_must_be_container_gated.py @@ -45,7 +45,13 @@ from tests.lint.rules.base import BaseRule -_GATE = "CODER_EVAL_IN_CONTAINER" +# Both spellings of the same gate. The env var NAME is the canonical one, but it +# is now reached through `models.container_paths.IN_CONTAINER_ENV` so the string +# has a single definition — and a rule that recognised only the literal would +# read the constant-based gate as NO gate at all, then instruct the author to +# paste the literal back. That is the rule arguing against the SSOT it should be +# reinforcing, so it accepts the constant's name too. +_GATES = ("CODER_EVAL_IN_CONTAINER", "IN_CONTAINER_ENV") _MESSAGE = ( "`os._exit` here is not gated on CODER_EVAL_IN_CONTAINER. It kills the process outright — " @@ -53,7 +59,8 @@ "own main process. Anywhere else it destroys a host process that merely called this code: an " "unconditionally-armed watchdog once exited a pytest worker 40s after the test that armed it, " "reporting as a random crash in an unrelated file and as a bogus coverage failure. Gate it on " - '`os.environ.get("CODER_EVAL_IN_CONTAINER") == "1"`, or add `# noqa: CE052` with a reason.' + '`os.environ.get(IN_CONTAINER_ENV) == "1"` (from coder_eval.models), or add `# noqa: CE052` ' + "with a reason." ) @@ -88,4 +95,4 @@ def visit_Call(self, node: ast.Call) -> None: self.generic_visit(node) def _container_gated(self) -> bool: - return any(_GATE in ast.dump(test) for test in self._guards) + return any(gate in ast.dump(test) for test in self._guards for gate in _GATES) diff --git a/tests/lint/rules/ce053_run_record_filename_literal.py b/tests/lint/rules/ce053_run_record_filename_literal.py index 0a611447..93f9b8dc 100644 --- a/tests/lint/rules/ce053_run_record_filename_literal.py +++ b/tests/lint/rules/ce053_run_record_filename_literal.py @@ -1,4 +1,4 @@ -"""CE053: no bare run-record filename literal outside ``path_utils``. +"""CE053: no bare run-record or run-log filename literal outside ``path_utils``. ``path_utils`` defines ``TASK_JSON_FILENAME`` / ``PRE_GRADE_JSON_FILENAME`` and its comment states why: "~12 sites name them — including three that ``rglob`` for @@ -17,6 +17,10 @@ constant in ``src/coder_eval/`` (outside ``path_utils.py``) that equals one of those filenames, or embeds it as a trailing path segment (``"*/task.json"``). Import the constant instead; ``# noqa: CE053`` for a genuinely unrelated string. + +Covers the per-run LOG names too (``task.log`` / ``grade.log`` / ``docker.log`` / +``grade.docker.log``) — same shape, one release later, and the ``docker.log`` +case was worse because its consumer skips silently when the file is absent. """ import ast @@ -26,10 +30,32 @@ def _run_record_filenames() -> set[str]: - """The filenames from ``path_utils``, read from the module rather than retyped.""" - from coder_eval.path_utils import PRE_GRADE_JSON_FILENAME, TASK_JSON_FILENAME + """The filenames from ``path_utils``, read from the module rather than retyped. + + The log names joined the record names for the same reason and after the same + defect one release later: ``docker.log`` was produced in ``isolation/`` and + consumed in ``orchestration/`` as three unrelated literals, and because the + consumer guards its copy with ``is_file()``, a rename on the producing side + would have degraded the fold-back to a silent no-op — discarding the only + record of why a grading container failed, with nothing failing. + """ + from coder_eval.path_utils import ( + DOCKER_LOG_FILENAME, + GRADE_DOCKER_LOG_FILENAME, + GRADE_LOG_FILENAME, + PRE_GRADE_JSON_FILENAME, + TASK_JSON_FILENAME, + TASK_LOG_FILENAME, + ) - return {TASK_JSON_FILENAME, PRE_GRADE_JSON_FILENAME} + return { + TASK_JSON_FILENAME, + PRE_GRADE_JSON_FILENAME, + TASK_LOG_FILENAME, + GRADE_LOG_FILENAME, + DOCKER_LOG_FILENAME, + GRADE_DOCKER_LOG_FILENAME, + } class NoRunRecordFilenameLiteral(BaseRule): diff --git a/tests/lint/rules/ce056_no_container_env_literal.py b/tests/lint/rules/ce056_no_container_env_literal.py new file mode 100644 index 00000000..59ced6ea --- /dev/null +++ b/tests/lint/rules/ce056_no_container_env_literal.py @@ -0,0 +1,83 @@ +"""CE056: no bare ``CODER_EVAL_IN_CONTAINER`` literal outside ``container_paths``. + +``models/container_paths.py`` defines ``IN_CONTAINER_ENV`` and its comment states +why: the string is the predicate for four separate gates, and "two half-copies of +the same string in different packages is how a rename becomes a silent no-op". + +The constant shipped with that rationale, every READER was migrated to it -- and +the single WRITER was not. ``docker_runner`` kept emitting +``--env CODER_EVAL_IN_CONTAINER=1``, which is the one site that produces the +value all four gates consume. Changing the constant would therefore have updated +every consumer and left the container exporting the old name, so all four gates +would read "not in a container" at once: + + * ``Sandbox.enforces_permission_windows`` -- the reference-solution anti-cheat + window silently stops being applied, and a run that is NOT protected scores + like one that is; + * ``resolve_reference_dir`` -- the ``/work/references`` branch is skipped; + * ``_should_grade_in_container`` -- a grading container dispatches another + grading container; + * the orphan-container heartbeat watchdog's ``os._exit(137)`` gate. + +None of those fail loudly. This is the CE053 shape exactly (a rename-safety +constant that shipped beside the literals it was meant to replace), and CE052 +cannot catch it -- that rule inspects ``if`` guards, so it never looks at the +writer at all. + +Fires on any string constant in ``src/coder_eval/`` (outside the defining module) +that equals the env-var name or embeds it as an ``NAME=value`` assignment. +Import ``IN_CONTAINER_ENV`` from ``coder_eval.models`` instead; ``# noqa: CE056`` +for a genuinely unrelated string. +""" + +import ast +import re + +from tests.lint.rules.base import BaseRule + + +def _container_env_name() -> str: + """Read from the module rather than retyped -- retyping it here would make + this rule the third copy of the string it exists to forbid.""" + from coder_eval.models import IN_CONTAINER_ENV + + return IN_CONTAINER_ENV + + +class NoContainerEnvLiteral(BaseRule): + id = "CE056" + + # `(^|sep)` so a repo-relative path is in scope too; see CE054. + _SRC_PATH = re.compile(r"(?:^|[/\\])src[/\\]coder_eval[/\\]") + # The module that DEFINES it. + _EXEMPT = re.compile(r"[/\\]container_paths\.py$") + _name: str | None = None + + def __init__(self, filepath: str) -> None: + super().__init__(filepath) + self._in_scope = bool(self._SRC_PATH.search(filepath)) and not self._EXEMPT.search(filepath) + if self._in_scope and NoContainerEnvLiteral._name is None: + NoContainerEnvLiteral._name = _container_env_name() + + def visit_Constant(self, node: ast.Constant) -> None: + if self._in_scope and isinstance(node.value, str): + self._check(node, node.value) + self.generic_visit(node) + + def _check(self, node: ast.Constant, value: str) -> None: + name = NoContainerEnvLiteral._name + if name is None: + return + # Exact, or the `NAME=value` child-process form. Not a bare `in`: prose + # in a docstring or an error message NAMES the variable on purpose, and + # this rule must not push authors to obfuscate their own explanations. + if value == name or value.startswith(f"{name}="): + self.violation( + node, + f"{value!r} names the in-container gate by literal. `IN_CONTAINER_ENV` exists in " + "coder_eval.models precisely so a rename cannot leave half the tree behind -- and " + "it shipped while the one site that SETS the variable kept the string, which would " + "have disarmed the reference anti-cheat window, the reference mount, the grading- " + "container recursion guard and the watchdog together, all silently. Import " + "IN_CONTAINER_ENV, or add `# noqa: CE056` if the string is unrelated.", + ) diff --git a/tests/lint/runner.py b/tests/lint/runner.py index cadc8840..c3b0e981 100644 --- a/tests/lint/runner.py +++ b/tests/lint/runner.py @@ -33,6 +33,7 @@ from tests.lint.rules.ce052_process_lethal_must_be_container_gated import ProcessLethalMustBeContainerGated from tests.lint.rules.ce053_run_record_filename_literal import NoRunRecordFilenameLiteral from tests.lint.rules.ce054_env_info_key_round_trip import EnvInfoKeyRoundTrip +from tests.lint.rules.ce056_no_container_env_literal import NoContainerEnvLiteral from tests.lint.rules.no_agent_timing_access import NoAgentTimingAccess from tests.lint.rules.no_blocking_io_in_async import NoBlockingIoInAsync from tests.lint.rules.no_cli_imports_in_core import NoCliImportsInCore @@ -88,6 +89,7 @@ NoUnionGetattrProbe, NoDriverOverride, ProcessLethalMustBeContainerGated, + NoContainerEnvLiteral, NoRunRecordFilenameLiteral, ] diff --git a/tests/test_custom_lint.py b/tests/test_custom_lint.py index e9a764b7..111b2a1e 100644 --- a/tests/test_custom_lint.py +++ b/tests/test_custom_lint.py @@ -828,12 +828,30 @@ def test_non_assignment_shapes_are_not_flagged(self, line: str): assert unbacked == [], f"false positive on non-assignment shape: {unbacked}" def test_name_side_of_env_value_literal_counts_as_backed(self): - # `--env CODER_EVAL_IN_CONTAINER=1` in src makes the doc assignment backed. + # `CODER_EVAL_IN_CONTAINER` reaches src only through the named constant + # `models/container_paths.py::IN_CONTAINER_ENV` -- both the docker + # `--env f"{IN_CONTAINER_ENV}=1"` writer and the four `os.environ.get` + # gates spell it that way. A scanner seeing only literals would call the + # repo's own gate unbacked and push the author to paste the literal back. from tests.lint.doc_env_parity import src_env_literals names = src_env_literals(self.REPO_ROOT / "src") assert "CODER_EVAL_IN_CONTAINER" in names + def test_a_constant_is_backed_only_when_something_reads_it(self, tmp_path: Path): + # The constant indirection must not weaken the rule: resolution is + # two-step, so a defined-but-unread constant stays unbacked exactly as a + # bare literal does. + from tests.lint.doc_env_parity import src_env_literals + + (tmp_path / "defs.py").write_text( + 'READ_ENV = "CODER_EVAL_READ"\nUNREAD_ENV = "CODER_EVAL_UNREAD"\n', encoding="utf-8" + ) + (tmp_path / "use.py").write_text("import os\nx = os.environ.get(READ_ENV)\n", encoding="utf-8") + names = src_env_literals(tmp_path) + assert "CODER_EVAL_READ" in names + assert "CODER_EVAL_UNREAD" not in names + def test_src_scan_requires_a_real_consumer_not_any_literal(self, tmp_path: Path): # A bare uppercase constant that no code reads must NOT count as "backed", # or it could silently mask a documented-but-unconsumed assignment. @@ -3794,6 +3812,14 @@ def test_allows_a_gated_exit(self): src = 'if os.environ.get("CODER_EVAL_IN_CONTAINER") == "1":\n os._exit(137)' assert not self._run(src) + def test_allows_a_gate_written_with_the_constant(self): + """The spelling the tree actually uses. A rule that saw only the literal + would read the constant-based gate as NO gate and tell the author to + paste the literal back — the rule arguing against the SSOT (and against + CE056) it should be reinforcing.""" + src = 'if os.environ.get(IN_CONTAINER_ENV) == "1":\n os._exit(137)' + assert not self._run(src) + def test_allows_it_nested_deeper_inside_the_gate(self): """The real site defines a function and a loop inside the guard.""" src = ( @@ -3851,6 +3877,58 @@ def test_every_listed_id_is_well_formed(self): assert not bad, f"not a CE rule id: {bad}" +class TestCE056NoContainerEnvLiteral: + """CE056 flags a bare `CODER_EVAL_IN_CONTAINER` outside container_paths. + + The motivating miss: every READER of the gate was migrated to + `IN_CONTAINER_ENV` and the single WRITER (`docker_runner`'s + `--env CODER_EVAL_IN_CONTAINER=1`) was not, so a rename would have disarmed + four gates at once, all silently. CE052 cannot see it -- that rule inspects + `if` guards, and the writer is not one. + """ + + @staticmethod + def _run(src: str, filepath: str = "src/coder_eval/isolation/docker_runner.py"): + import ast + + from tests.lint.rules.ce056_no_container_env_literal import NoContainerEnvLiteral + + return NoContainerEnvLiteral(filepath).check(ast.parse(src)) + + def test_flags_the_child_process_assignment_form(self): + """The exact shape that shipped unmigrated.""" + assert self._run('argv += ["--env", "CODER_EVAL_IN_CONTAINER=1"]') + + def test_flags_a_bare_read(self): + assert self._run('if os.environ.get("CODER_EVAL_IN_CONTAINER") == "1": pass') + + def test_allows_the_constant(self): + assert not self._run('argv += ["--env", f"{IN_CONTAINER_ENV}=1"]') + + def test_allows_prose_that_merely_names_the_variable(self): + """A docstring explaining the gate must name it; a rule that pushed + authors to obfuscate their own explanations would be a bad trade.""" + assert not self._run('"""Gated on CODER_EVAL_IN_CONTAINER, never on the driver."""') + + def test_ignores_the_defining_module(self): + assert not self._run( + 'IN_CONTAINER_ENV = "CODER_EVAL_IN_CONTAINER"', + filepath="src/coder_eval/models/container_paths.py", + ) + + def test_the_real_tree_is_clean(self): + """The whole point: the writer is migrated and stays migrated.""" + import ast + + from tests.lint.rules.ce056_no_container_env_literal import NoContainerEnvLiteral + + src = Path(__file__).parent.parent / "src" / "coder_eval" + violations = [] + for py in src.rglob("*.py"): + violations += NoContainerEnvLiteral(str(py)).check(ast.parse(py.read_text(encoding="utf-8"))) + assert not violations, violations + + class TestCE053NoRunRecordFilenameLiteral: """CE053 flags a `task.json` literal outside path_utils.""" diff --git a/tests/test_detached_grading_boundaries.py b/tests/test_detached_grading_boundaries.py index c1db6783..8f8b327e 100644 --- a/tests/test_detached_grading_boundaries.py +++ b/tests/test_detached_grading_boundaries.py @@ -202,15 +202,24 @@ def test_the_refused_record_is_quarantined_off_task_json(self, tmp_path: Path) - class TestInContainerGradeCoercion: """The container side of the same boundary.""" + # Valid enough to survive `load_task`, which the regrade branch reaches. The + # `grade` / `regrade` coercions refuse before it, so those tests do not + # depend on this; the prior.json ones do. + _VALID_TASK_YAML = ( + "task_id: t\ndescription: d\nagent:\n type: none\n" + "success_criteria:\n - type: file_exists\n path: out.txt\n description: d\n" + ) + @staticmethod - def _run_with_context(tmp_path: Path, grade: object): + def _run_with_context(tmp_path: Path, grade: object = True, **extra: object): input_dir = tmp_path / "input" - input_dir.mkdir() - # Only the keys read BEFORE the grade coercion need real values; the - # command must refuse before it ever builds an Orchestrator. - context = {"variant_id": "default", "source_yaml": "task_id: t\n", "grade": grade} + input_dir.mkdir(exist_ok=True) + # Only the keys read BEFORE the coercions need real values; the command + # must refuse before it ever builds an Orchestrator. + context: dict[str, object] = {"variant_id": "default", "source_yaml": "task_id: t\n", "grade": grade} + context.update(extra) (input_dir / "context.json").write_text(json.dumps(context), encoding="utf-8") - (input_dir / "task.yaml").write_text("task_id: t\n", encoding="utf-8") + (input_dir / "task.yaml").write_text(TestInContainerGradeCoercion._VALID_TASK_YAML, encoding="utf-8") return runner.invoke( app, ["_run-task-internal", "--input", str(input_dir), "--output", str(tmp_path / "out")], @@ -244,6 +253,36 @@ def test_a_non_boolean_grade_is_a_hard_error(self, tmp_path: Path) -> None: assert result.exit_code == 2 assert "must be a boolean" in result.output + def test_a_non_boolean_regrade_is_a_hard_error(self, tmp_path: Path) -> None: + """The destructive twin of the test above, and the worse direction: a + truthy `"regrade": "false"` would take the ORDINARY branch and re-RUN the + agent against the workspace the operator asked only to grade, destroying + the trajectory being graded.""" + result = self._run_with_context(tmp_path, regrade="false") + assert result.exit_code == 2 + assert "'regrade' must be a boolean" in result.output + + def test_a_regrade_without_a_staged_prior_names_the_missing_file(self, tmp_path: Path) -> None: + """The host stages prior.json beside task.yaml. Without it there is no row + to seed from, and the message must name the file — a crash here reaches + the host only as the opaque "container exited without producing + task.json".""" + result = self._run_with_context(tmp_path, regrade=True) + assert result.exit_code == 2 + assert "prior.json" in result.output + assert "missing" in result.output + + def test_an_unreadable_prior_degrades_to_a_message_not_a_traceback(self, tmp_path: Path) -> None: + """Corrupt bytes must produce the named-file diagnostic the code's own + comment promises, not a ValidationError traceback.""" + input_dir = tmp_path / "input" + input_dir.mkdir() + (input_dir / "prior.json").write_text("{not json", encoding="utf-8") + result = self._run_with_context(tmp_path, regrade=True) + assert result.exit_code == 2 + assert "not a readable EvaluationResult" in result.output + assert "Traceback" not in result.output + # The in-container default is asserted BEHAVIOURALLY by # `TestGradePlumbedIntoTheContainerOrchestrator::test_an_absent_key_still_grades`. # It used to be a `assert 'context.get("grade", True)' in source` grep, which @@ -251,6 +290,98 @@ def test_a_non_boolean_grade_is_a_hard_error(self, tmp_path: Path) -> None: # while the line it describes is never executed. +class TestInContainerRegradeBranch: + """The container half of `evaluate ` / `run --resume`, driven end to end. + + Every seam here was reachable only through a real container, so the branch + shipped at 0% coverage — and it holds two values whose loss is silent. + Verified by mutation on the merged commit: deleting `recorded_task=` and + nulling `recorded_task_file` both left the whole suite green, while the first + makes the row record `driver: tempdir` (which then lets a later + `evaluate ` skip BOTH the host-grading refusal and the + `graded_on_host` stamp) and the second makes it record + `/work/task_dir/task.yaml`, a path that exists on no host. + """ + + _DOCKER_TASK_YAML = ( + "task_id: t\ndescription: d\nagent:\n type: none\nsandbox:\n driver: docker\n" + "success_criteria:\n - type: file_exists\n path: out.txt\n description: d\n" + ) + + def _invoke(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, *, mount_workspace: bool = True, **extra): + from coder_eval import models + from coder_eval.orchestration import regrade as rg + + input_dir = tmp_path / "input" + input_dir.mkdir(exist_ok=True) + (input_dir / "task.yaml").write_text(self._DOCKER_TASK_YAML, encoding="utf-8") + (input_dir / "prior.json").write_text(_result().model_dump_json(), encoding="utf-8") + context: dict[str, object] = { + "variant_id": "default", + "source_yaml": self._DOCKER_TASK_YAML, + "regrade": True, + "host_task_file": str(tmp_path / "host" / "task.yaml"), + } + context.update(extra) + (input_dir / "context.json").write_text(json.dumps(context), encoding="utf-8") + + workspace = tmp_path / "graded-workspace" + if mount_workspace: + workspace.mkdir() + monkeypatch.setattr(models, "CONTAINER_GRADE_WORKSPACE", str(workspace)) + + captured: dict[str, object] = {} + + async def _fake_regrade(**kw: object) -> EvaluationResult: + captured.update(kw) + return _result() + + monkeypatch.setattr(rg, "regrade_in_place", _fake_regrade) + result = runner.invoke( + app, + ["_run-task-internal", "--input", str(input_dir), "--output", str(tmp_path / "out")], + ) + return result, captured + + def test_it_grades_the_mounted_workspace_with_the_authored_task( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Three values, and each is what keeps the row honest: the workspace the + host bind-mounted, the AUTHORED `driver: docker` task (so the record does + not claim the `tempdir` rewrite this process performs on itself), and the + HOST's task file (so `source_file` names a path that exists off the + container).""" + result, captured = self._invoke(tmp_path, monkeypatch) + + assert result.exit_code == 0, result.output + assert captured["workspace"] == Path(tmp_path / "graded-workspace") + # What runs: rewritten to tempdir, because we are already inside the + # container the docker driver asked for. + assert captured["task"].sandbox.driver == "tempdir" # type: ignore[union-attr] + # What is RECORDED: unchanged. + assert captured["recorded_task"].sandbox.driver == "docker" # type: ignore[union-attr] + assert captured["recorded_task_file"] == tmp_path / "host" / "task.yaml" + + def test_an_older_host_forwards_no_task_file_and_that_is_not_fatal( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """`host_task_file` is absent on a host predating the key. The row then + records the container path, which is the pre-existing behaviour — a + degraded record, not a refusal.""" + result, captured = self._invoke(tmp_path, monkeypatch, host_task_file=None) + + assert result.exit_code == 0, result.output + assert captured["recorded_task_file"] is None + + def test_an_unmounted_workspace_is_a_hard_error(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Grading a directory that is not there would score every criterion 0.0 + and report it as an agent failure. Exit 2 keeps it distinguishable.""" + result, _ = self._invoke(tmp_path, monkeypatch, mount_workspace=False) + + assert result.exit_code == 2 + assert "was not mounted" in result.output + + # -------------------------------------------------------------------------- # The crash-recovery arm # -------------------------------------------------------------------------- diff --git a/tests/test_docker_runner_mounts.py b/tests/test_docker_runner_mounts.py index f6d65e72..dae7d344 100644 --- a/tests/test_docker_runner_mounts.py +++ b/tests/test_docker_runner_mounts.py @@ -888,6 +888,43 @@ class TestContainerAccessWidening: def _other_bits(path: Path) -> int: return path.stat().st_mode & 0o007 + def test_it_reports_what_it_widened_so_a_caller_can_put_it_back(self, tmp_path: Path): + """The graded workspace is the one mount the harness did NOT create. + + `input_dir` / `output_dir` are staging directories deleted with the + dispatch, so widening them is scoped to their lifetime. The graded + workspace is not: with `--workspace` it is an arbitrary operator + directory, and otherwise it is the run's preserved `artifacts/` tree that + outlives the grade. Left world-writable it lets any other local uid on a + shared or CI host rewrite the artifacts a criterion reads -- i.e. change + the verdict -- so the widening must be reversible. + """ + from coder_eval.isolation.docker_runner import restore_modes + + ws = tmp_path / "ws" + ws.mkdir(mode=0o700) + f = ws / "deliverable.txt" + f.write_text("x", encoding="utf-8") + f.chmod(0o600) + + widened = grant_container_access(ws, writable=True) + + assert self._other_bits(ws) == 0o007 + assert self._other_bits(f) == 0o006 + assert {p for p, _ in widened} == {ws, f} + + restore_modes(widened) + + assert ws.stat().st_mode & 0o777 == 0o700 + assert f.stat().st_mode & 0o777 == 0o600 + + def test_restoring_is_best_effort_over_a_vanished_path(self, tmp_path: Path): + """It runs in a `finally` beside the staging cleanup, where an exception + is often already in flight. A failed restore must never mask it.""" + from coder_eval.isolation.docker_runner import restore_modes + + restore_modes([(tmp_path / "gone", 0o600)]) + def test_output_dir_becomes_other_writable(self, tmp_path: Path): run_dir = tmp_path / "run" run_dir.mkdir(mode=0o755) diff --git a/tests/test_regrade.py b/tests/test_regrade.py index be8da625..d1435dee 100644 --- a/tests/test_regrade.py +++ b/tests/test_regrade.py @@ -12,7 +12,9 @@ import json import logging +from datetime import timedelta from pathlib import Path +from typing import ClassVar import pytest @@ -21,6 +23,8 @@ EvaluationResult, FileExistsCriterion, FinalStatus, + PreservationMode, + ResolvedTask, RunCommandCriterion, TaskConfigRecord, TaskDefinition, @@ -321,3 +325,950 @@ def test_a_sandbox_path_outside_the_run_dir_is_refused(tmp_path: Path) -> None: with pytest.raises(RegradeError, match="outside the run directory"): default_workspace(run_dir, _result(sandbox_path=str(outside))) + + +# -------------------------------------------------------------------------- +# Grading a `driver: docker` row — inside a container of its own image +# +# Such a task's criteria address the IMAGE's paths and toolchain, so grading +# them on the host answers a different question. Demonstrated on +# `tasks/byod_smoke_test.yaml`, whose criterion is `test -f /opt/byod_marker` +# (baked into the BYOD image): the identical row scores SUCCESS 1.000 graded in +# a container and FAILURE 0.000 graded on the host. That is not a flaky +# difference — it is the host answering "is the marker on THIS machine", which +# nobody asked. +# +# So the docker row is now DISPATCHED to a grading container rather than +# refused. `--allow-host-grading` keeps its old meaning: grade here anyway (no +# docker available, or criteria known to be host-portable), and wear the +# `graded_on_host` stamp. +# -------------------------------------------------------------------------- + + +class _RunnerDouble: + """A DockerRunner stand-in bound to the REAL constructor signature. + + The first version took ``(rt, **kw)``, which swallowed every keyword and made + a signature drift on the grading seam invisible: renaming or adding a + required kwarg on `DockerRunner.__init__` left the doubles accepting it and + the suite green. Tests are outside pyright's `include`, so nothing else + catches it either. Spelling the parameters out turns such a rename into a + TypeError here, which is the whole point of a double. + """ + + captured: ClassVar[dict[str, object]] = {} + + def __init__( + self, + rt: ResolvedTask, + preservation_mode: PreservationMode = PreservationMode.DIRECT_WRITE, + stream_callback: object = None, + verbose: bool = False, + grade: bool = True, + prior_result: EvaluationResult | None = None, + grade_workspace: Path | None = None, + ) -> None: + self.rt = rt + type(self).captured = { + "run_dir": rt.run_dir, + "task_id": rt.task.task_id, + "preservation_mode": preservation_mode, + "prior_result": prior_result, + "grade_workspace": grade_workspace, + } + + async def run(self) -> EvaluationResult: # pragma: no cover - overridden + raise NotImplementedError + + +def _docker_task() -> TaskDefinition: + from coder_eval.models import SandboxConfig + + task = _task() + return task.model_copy(update={"sandbox": SandboxConfig(driver="docker")}) + + +class TestShouldGradeInContainer: + """The routing decision, as a truth table. + + Each row rules out a different wrong answer, so they are asserted + separately rather than as one compound expression. + """ + + @pytest.fixture(autouse=True) + def _on_the_host(self, monkeypatch: pytest.MonkeyPatch) -> None: + """Three of these rows assert the HOST answer, so the ambient value of the + gate variable must not decide the test. This repo's own container harness + sets it on every task container, so inheriting it would flip two of them + to a silent pass-for-the-wrong-reason.""" + from coder_eval.models import IN_CONTAINER_ENV + + monkeypatch.delenv(IN_CONTAINER_ENV, raising=False) + + def test_a_docker_row_on_the_host_goes_to_a_container(self) -> None: + from coder_eval.orchestration.regrade import _should_grade_in_container + + assert _should_grade_in_container(_docker_task(), allow_host_grading=False) is True + + def test_allow_host_grading_still_wins(self) -> None: + """The escape hatch must keep working — it is the only option on a + machine without docker, and the row it produces is stamped.""" + from coder_eval.orchestration.regrade import _should_grade_in_container + + assert _should_grade_in_container(_docker_task(), allow_host_grading=True) is False + + def test_a_tempdir_row_never_starts_a_container(self) -> None: + from coder_eval.orchestration.regrade import _should_grade_in_container + + assert _should_grade_in_container(_task(), allow_host_grading=False) is False + + def test_inside_a_container_it_does_not_recurse(self, monkeypatch: pytest.MonkeyPatch) -> None: + """Gated on CODER_EVAL_IN_CONTAINER, never on the driver. + + The in-container entry point rewrites `docker` -> `tempdir` before + building its Orchestrator, so a driver-based test would read a value + that has already been changed — the same trap the reference-permission + window documents. Without this gate a grading container dispatches a + grading container. + """ + from coder_eval.models import IN_CONTAINER_ENV + from coder_eval.orchestration.regrade import _should_grade_in_container + + monkeypatch.setenv(IN_CONTAINER_ENV, "1") + assert _should_grade_in_container(_docker_task(), allow_host_grading=False) is False + + +class TestGradeInContainerDispatch: + async def test_it_dispatches_with_the_prior_row_and_the_workspace( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Both must reach DockerRunner, and they are what make the grade real: + the prior row supplies the trajectory a judge or `command_executed` + criterion reads, and the workspace is the tree under evaluation.""" + from coder_eval.isolation import docker_runner as dr + + graded = _result(final_status=FinalStatus.SUCCESS, weighted_score=1.0) + + class _FakeRunner(_RunnerDouble): + async def run(self) -> EvaluationResult: + return graded + + monkeypatch.setattr(dr, "DockerRunner", _FakeRunner) + + from coder_eval.models import PreservationMode + from coder_eval.orchestration.regrade import regrade_in_place + + workspace = tmp_path / "ws" + workspace.mkdir() + task_file = tmp_path / "t.yaml" + task_file.write_text("task_id: t\n", encoding="utf-8") + prior = _result() + + out = await regrade_in_place( + task=_docker_task(), + prior=prior, + workspace=workspace, + run_dir=tmp_path / "grade-run", + task_file=task_file, + source_yaml="", + variant_id="v", + ) + + captured = _FakeRunner.captured + assert out is graded + assert captured["prior_result"] is prior + assert captured["grade_workspace"] == workspace + # The workspace belongs to the ORIGINAL run; a grading pass must never + # move or delete it. + assert captured["preservation_mode"] is PreservationMode.NONE + # The grade writes into a SCRATCH dir the container alone owns, never the + # caller's run_dir. `run --resume` passes the executed row's own + # directory, where a pre-existing task.json would be read back as a + # successful grade if the container died (`_parse_result_or_raise` keys + # on existence and discards returncode) and where `docker.log` would be + # truncated. The verdict is folded back afterwards. + container_run_dir = captured["run_dir"] + assert isinstance(container_run_dir, Path) + assert container_run_dir != tmp_path / "grade-run" + assert container_run_dir.name.startswith("coder-eval-grade-") + + async def test_the_container_grade_is_folded_back_into_the_callers_run_dir( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """The scratch dir is an implementation detail: the caller must still find + the graded task.json where it asked for it. The container's own + `docker.log` lands beside it under a PHASE-specific name, because on the + resume path `docker.log` is already the executed run's.""" + from coder_eval.isolation import docker_runner as dr + + graded = _result(final_status=FinalStatus.SUCCESS, weighted_score=1.0) + + class _FakeRunner(_RunnerDouble): + async def run(self) -> EvaluationResult: + self.rt.run_dir.mkdir(parents=True, exist_ok=True) + (self.rt.run_dir / "task.json").write_text(graded.model_dump_json(), encoding="utf-8") + (self.rt.run_dir / "docker.log").write_text("container output", encoding="utf-8") + (self.rt.run_dir / "grade.log").write_text("why each criterion scored", encoding="utf-8") + return graded + + monkeypatch.setattr(dr, "DockerRunner", _FakeRunner) + + from coder_eval.orchestration.regrade import regrade_in_place + + workspace = tmp_path / "ws" + workspace.mkdir() + task_file = tmp_path / "t.yaml" + task_file.write_text("task_id: t\n", encoding="utf-8") + run_dir = tmp_path / "row" + run_dir.mkdir() + # The executed run's own container log, which the grade must not clobber. + (run_dir / "docker.log").write_text("the executed run's log", encoding="utf-8") + + await regrade_in_place( + task=_docker_task(), + prior=_result(), + workspace=workspace, + run_dir=run_dir, + task_file=task_file, + source_yaml="", + variant_id="v", + ) + + folded = EvaluationResult.model_validate_json((run_dir / "task.json").read_text(encoding="utf-8")) + assert folded.final_status is FinalStatus.SUCCESS + assert (run_dir / "grade.docker.log").read_text(encoding="utf-8") == "container output" + assert (run_dir / "docker.log").read_text(encoding="utf-8") == "the executed run's log" + # The grading pass's OWN log — the per-criterion detail, and a documented + # part of the run-directory contract. It lived in the scratch dir and was + # deleted with it, so a `driver: docker` row was the one shape a detached + # grade left without a `grade.log`. It does NOT collide: a detached grade + # is the only thing that ever writes that name here. + assert (run_dir / "grade.log").read_text(encoding="utf-8") == "why each criterion scored" + + async def test_without_a_task_file_it_refuses_and_names_the_escape_hatch(self, tmp_path: Path) -> None: + """The image is resolved relative to the task file; with none there is + nothing to build from. A real limitation, so it says so rather than + silently grading on the host.""" + from coder_eval.orchestration.regrade import regrade_in_place + + workspace = tmp_path / "ws" + workspace.mkdir() + with pytest.raises(RegradeError, match="allow-host-grading"): + await regrade_in_place( + task=_docker_task(), + prior=_result(), + workspace=workspace, + run_dir=tmp_path / "r", + task_file=None, + source_yaml="", + variant_id="v", + ) + + async def test_a_container_failure_becomes_a_regrade_error( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """`orchestration/` must not leak an isolation-layer exception to the + CLI, and the actionable next step is the escape hatch, not a docker + stack trace.""" + from coder_eval.isolation import docker_runner as dr + + class _Boom(_RunnerDouble): + async def run(self) -> EvaluationResult: + raise dr.DockerRunError("image pull failed") + + monkeypatch.setattr(dr, "DockerRunner", _Boom) + + from coder_eval.orchestration.regrade import regrade_in_place + + workspace = tmp_path / "ws" + workspace.mkdir() + task_file = tmp_path / "t.yaml" + task_file.write_text("task_id: t\n", encoding="utf-8") + + with pytest.raises(RegradeError, match="image pull failed"): + await regrade_in_place( + task=_docker_task(), + prior=_result(), + workspace=workspace, + run_dir=tmp_path / "r", + task_file=task_file, + source_yaml="", + variant_id="v", + ) + + +class TestDockerRunnerGradingWiring: + """The three things the container needs, asserted on the wire format. + + A grading container differs from a running one only in what it is handed, so + these are the contract with `run_task_internal_command`'s regrade branch. + """ + + @staticmethod + def _runner(tmp_path: Path, **kw): + from coder_eval.isolation.docker_runner import DockerRunner + from coder_eval.models import ResolvedTask + + task_file = tmp_path / "t.yaml" + task_file.write_text("task_id: t\n", encoding="utf-8") + rt = ResolvedTask( + task=_docker_task(), task_file=task_file, run_dir=tmp_path / "run", variant_id="v", source_yaml="" + ) + return DockerRunner(rt, **kw) + + def test_prior_result_and_workspace_must_be_passed_together(self, tmp_path: Path) -> None: + """Either alone is a routing bug: a prior with no workspace has nothing + to grade, a workspace with no prior loses the trajectory.""" + ws = tmp_path / "ws" + ws.mkdir() + with pytest.raises(ValueError, match="together"): + self._runner(tmp_path, prior_result=_result()) + with pytest.raises(ValueError, match="together"): + self._runner(tmp_path, grade_workspace=ws) + + async def test_a_grading_run_stages_the_prior_row_and_flags_the_regrade(self, tmp_path: Path) -> None: + from coder_eval.path_utils import PRIOR_RESULT_FILENAME + + ws = tmp_path / "ws" + ws.mkdir() + prior = _result(weighted_score=None) + runner = self._runner(tmp_path, prior_result=prior, grade_workspace=ws) + + staged = tmp_path / "input" + staged.mkdir() + await runner._stage_inputs(staged) + + context = json.loads((staged / "context.json").read_text(encoding="utf-8")) + assert context["regrade"] is True + # A bool, not a string: the container coerces and rejects non-bools, + # because a truthy `"false"` here would re-RUN the agent over the very + # workspace the operator asked only to grade. + assert isinstance(context["regrade"], bool) + + recovered = EvaluationResult.model_validate_json((staged / PRIOR_RESULT_FILENAME).read_text(encoding="utf-8")) + assert recovered.task_id == prior.task_id + assert recovered.final_status is FinalStatus.NOT_GRADED + + async def test_an_ordinary_run_stages_neither(self, tmp_path: Path) -> None: + """The control: a normal `run` must be byte-identical to before, and in + particular must not acquire a prior.json nobody asked for.""" + from coder_eval.path_utils import PRIOR_RESULT_FILENAME + + runner = self._runner(tmp_path) + staged = tmp_path / "input" + staged.mkdir() + await runner._stage_inputs(staged) + + context = json.loads((staged / "context.json").read_text(encoding="utf-8")) + assert context["regrade"] is False + assert not (staged / PRIOR_RESULT_FILENAME).exists() + + def test_the_workspace_is_mounted_read_write_at_the_container_path(self, tmp_path: Path) -> None: + """Read-WRITE and not a copy. Criteria legitimately mutate what they + grade (a `run_command` that compiles, a post_run that cleans up), and + copying is what the host path proved wrong: the template filter drops + node_modules / dist / build / .venv, so a criterion reading those fails + as a copying artifact rather than as a verdict. + """ + from coder_eval.models import CONTAINER_GRADE_WORKSPACE + + ws = tmp_path / "ws" + ws.mkdir() + runner = self._runner(tmp_path, prior_result=_result(), grade_workspace=ws) + argv = runner._build_argv(tmp_path / "input", tmp_path / "out", container_name="c", image="img") + + mounts = [argv[i + 1] for i, a in enumerate(argv) if a == "-v"] + assert f"{ws.resolve()}:{CONTAINER_GRADE_WORKSPACE}" in mounts, mounts + assert not any(m.endswith(f":{CONTAINER_GRADE_WORKSPACE}:ro") for m in mounts) + + def test_an_ordinary_run_mounts_no_grading_workspace(self, tmp_path: Path) -> None: + from coder_eval.models import CONTAINER_GRADE_WORKSPACE + + runner = self._runner(tmp_path) + argv = runner._build_argv(tmp_path / "input", tmp_path / "out", container_name="c", image="img") + assert CONTAINER_GRADE_WORKSPACE not in " ".join(argv) + + async def test_a_grading_run_forwards_the_hosts_own_task_file_for_the_record(self, tmp_path: Path) -> None: + """`task.json` must record a path that exists on a HOST. + + The container resolves TASK_DIR against `/work/task_dir/task.yaml`, which + is right in there and meaningless anywhere else. Recording THAT made a + detached grade rebuild the task around an unresolvable path: the dispatch + guard saw a non-None Path and let it through, and `_prepare_task_dir_mount` + then silently mounted nothing, so every `$TASK_DIR` criterion resolved + against the wrong tree. + """ + runner = self._runner(tmp_path) + input_dir = tmp_path / "input" + input_dir.mkdir() + await runner._stage_inputs(input_dir) + + context = json.loads((input_dir / "context.json").read_text(encoding="utf-8")) + assert context["host_task_file"] == str(tmp_path / "t.yaml") + + +class TestRegradeSkewGuard: + """A stale image must not turn a GRADE into a fresh agent run. + + Exactly the sibling of the `grade` guard one release earlier: `regrade` + crosses the boundary only through context.json, so an image that predates + container-side grading ignores the key and runs the agent — and the host + would fold that fabricated trajectory back as the recorded row's verdict. + """ + + @staticmethod + def _runner(tmp_path: Path, prior): + from coder_eval.isolation.docker_runner import DockerRunner + from coder_eval.models import ResolvedTask + + task_file = tmp_path / "t.yaml" + task_file.write_text("task_id: t\n", encoding="utf-8") + rt = ResolvedTask( + task=_docker_task(), task_file=task_file, run_dir=tmp_path / "run", variant_id="v", source_yaml="" + ) + ws = tmp_path / "ws" + ws.mkdir(exist_ok=True) + return DockerRunner(rt, prior_result=prior, grade_workspace=ws) + + def test_a_row_carrying_the_recorded_trajectory_is_accepted(self, tmp_path: Path) -> None: + prior = _result() + graded = _result(final_status=FinalStatus.SUCCESS, weighted_score=1.0) + graded.started_at = prior.started_at + self._runner(tmp_path, prior)._assert_regrade_honored(graded) + + def test_a_freshly_run_trajectory_is_refused_and_quarantined(self, tmp_path: Path) -> None: + from coder_eval.isolation.docker_runner import DockerRunError + + prior = _result() + rerun = _result(final_status=FinalStatus.SUCCESS, weighted_score=1.0) + rerun.started_at = prior.started_at + timedelta(hours=1) + + task_json = tmp_path / "task.json" + task_json.write_text("{}", encoding="utf-8") + with pytest.raises(DockerRunError, match="re-ran the agent"): + self._runner(tmp_path, prior)._assert_regrade_honored(rerun, task_json) + + # Refusing in memory while leaving contradictory bytes on disk is not a + # refusal: a later `aggregate` would publish exactly this record. + assert not task_json.exists() + assert task_json.with_suffix(".json.rerun").is_file() + + def test_an_ordinary_run_is_never_checked(self, tmp_path: Path) -> None: + """`prior_result is None` means nobody asked for a grade, so a fresh + trajectory is the expected outcome, not a skew symptom.""" + from coder_eval.isolation.docker_runner import DockerRunner + from coder_eval.models import ResolvedTask + + task_file = tmp_path / "t.yaml" + task_file.write_text("task_id: t\n", encoding="utf-8") + rt = ResolvedTask( + task=_docker_task(), task_file=task_file, run_dir=tmp_path / "run", variant_id="v", source_yaml="" + ) + DockerRunner(rt)._assert_regrade_honored(_result(final_status=FinalStatus.SUCCESS)) + + +class TestContainerDispatchIsInsideTheTrustGate: + """A run directory is a shareable artifact, so the image it names is untrusted. + + Grading a `driver: docker` row DISPATCHES A CONTAINER built from the recorded + sandbox block — the record chooses an image that runs on this host with the + default credential allowlist forwarded into it and a copy of ~/.claude + mounted. That is a strictly wider capability than the `run_command` strings + this gate already refuses, and it reached the host unprompted because the + scan walked only success_criteria and post_run. + """ + + def test_the_dispatch_is_named_in_the_inventory(self) -> None: + from coder_eval.orchestration.regrade import embedded_commands + + commands = embedded_commands(_docker_task(), include_setup_phase=False, include_container_dispatch=True) + assert any("docker run" in c for c in commands), commands + + def test_it_is_silent_when_no_container_will_be_dispatched(self) -> None: + """The gate must not fire for something that never runs — `--copy` is + refused before it could dispatch, and a refusal that always fires stops + being read.""" + from coder_eval.orchestration.regrade import embedded_commands + + assert embedded_commands(_docker_task(), include_setup_phase=False) == [] + + def test_an_all_file_exists_docker_row_no_longer_sails_through(self, tmp_path: Path) -> None: + """The exact bypass: zero embedded shell, so the gate returned early and + the container was started with no consent at all.""" + from coder_eval.orchestration.regrade import RegradeError, check_embedded_commands + + with pytest.raises(RegradeError, match="docker run"): + check_embedded_commands( + _docker_task(), + tmp_path, + allow_recorded_commands=False, + include_setup_phase=False, + include_container_dispatch=True, + ) + + def test_the_operator_can_still_opt_in(self, tmp_path: Path) -> None: + from coder_eval.orchestration.regrade import check_embedded_commands + + check_embedded_commands( + _docker_task(), + tmp_path, + allow_recorded_commands=True, + include_setup_phase=False, + include_container_dispatch=True, + ) + + +class TestOperatorBaselineFailsClosed: + """A missing or invalid baseline must NARROW the exemption, never widen it. + + This is the direction that fails silently: a broken baseline returning the + wrong sentinel would make the trust gate stop prompting for authored + `post_run` commands, and nothing would notice. + """ + + @staticmethod + def _clear(): + from coder_eval.orchestration.regrade import _operator_baseline_post_run + + _operator_baseline_post_run.cache_clear() + + def test_a_broken_baseline_exempts_nothing(self, monkeypatch: pytest.MonkeyPatch) -> None: + import coder_eval.orchestration.experiment as exp + from coder_eval.orchestration.regrade import _operator_baseline_post_run + + def _boom(_path): + raise OSError("no such file") + + monkeypatch.setattr(exp, "load_experiment", _boom) + self._clear() + try: + assert _operator_baseline_post_run() == frozenset() + finally: + self._clear() + + def test_a_baseline_without_defaults_exempts_nothing(self, monkeypatch: pytest.MonkeyPatch) -> None: + import coder_eval.orchestration.experiment as exp + from coder_eval.orchestration.regrade import _operator_baseline_post_run + + monkeypatch.setattr(exp, "load_experiment", lambda _p: type("E", (), {"defaults": None})()) + self._clear() + try: + assert _operator_baseline_post_run() == frozenset() + finally: + self._clear() + + def test_with_no_exemption_the_baseline_command_is_scanned(self, monkeypatch: pytest.MonkeyPatch) -> None: + """The consequence, asserted rather than assumed: an empty baseline means + every recorded post_run reaches the gate.""" + import coder_eval.orchestration.experiment as exp + from coder_eval.models import PostRunCommand + from coder_eval.orchestration.regrade import embedded_commands + + monkeypatch.setattr(exp, "load_experiment", lambda _p: type("E", (), {"defaults": None})()) + self._clear() + try: + task = _task() + task.post_run = [PostRunCommand(command="rm -rf node_modules .npm-prefix", timeout=30)] + assert "rm -rf node_modules .npm-prefix" in embedded_commands(task, include_setup_phase=False) + finally: + self._clear() + + +class TestEvaluateDispatchesADockerRow: + """The reordering in `evaluate_command` is the fix; nothing pinned it. + + `delegates_to_regrade` exists solely because `grading_sandbox_config` -- + whose job is to REFUSE `driver: docker` -- was being called BEFORE the branch + that no longer needs it, so no docker row could ever reach the container + dispatch. Revert the hoist and every docker detached grade becomes a hard + refusal again, with the suite still green. + """ + + @staticmethod + def _docker_run_dir(tmp_path: Path) -> Path: + """A run directory whose recorded config says `driver: docker`.""" + from coder_eval.models import TaskConfigRecord + + run_dir = tmp_path / "run" + run_dir.mkdir() + task = _docker_task() + prior = _result( + weighted_score=None, + final_status=FinalStatus.NOT_GRADED, + task_config=TaskConfigRecord( + resolved=task.model_dump(warnings=False), + source_yaml="raw", + source_file=None, + ), + ) + (run_dir / "task.json").write_text(prior.model_dump_json(), encoding="utf-8") + (run_dir / "artifacts").mkdir() + return run_dir + + def test_the_default_path_reaches_the_container_dispatch( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Not the host-grading refusal: the dispatch. Asserted by the message + the refusal would have produced being absent from the failure.""" + from coder_eval.models import IN_CONTAINER_ENV + from coder_eval.orchestration import regrade as rg + + monkeypatch.delenv(IN_CONTAINER_ENV, raising=False) + run_dir = self._docker_run_dir(tmp_path) + prior = rg.load_prior_result(run_dir) + task, _ = rg.task_from_prior( + prior, + run_dir, + allow_recorded_commands=True, + grade_in_place=True, + allow_host_grading=False, + ) + # The routing predicate the CLI's reordering exists to let run. + assert rg._should_grade_in_container(task, allow_host_grading=False) is True + + def test_allow_host_grading_still_takes_the_host_branch( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + from coder_eval.models import IN_CONTAINER_ENV + from coder_eval.orchestration import regrade as rg + + monkeypatch.delenv(IN_CONTAINER_ENV, raising=False) + run_dir = self._docker_run_dir(tmp_path) + prior = rg.load_prior_result(run_dir) + task, _ = rg.task_from_prior( + prior, + run_dir, + allow_recorded_commands=True, + grade_in_place=True, + allow_host_grading=True, + ) + assert rg._should_grade_in_container(task, allow_host_grading=True) is False + # And the host config it then builds is the downgraded one, stamped. + assert rg.grading_sandbox_config(task, allow_host_grading=True).driver == "tempdir" + + +class TestContainerFailureKeepsItsEvidence: + """A failed grading container must not delete the log its error names. + + `_grade_in_container` runs the whole dispatch inside a `TemporaryDirectory`, + and every DockerRunner diagnostic — the container's merged stdout+stderr, the + captured build log, the in-container FATAL guards — is written into it. + Folding out only on SUCCESS destroyed precisely the evidence, while + DockerRunError's own text says `See {log_path} for container output`, naming + a path that no longer existed by the time it was printed. + """ + + async def test_the_container_log_survives_a_failed_grade( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + from coder_eval.isolation import docker_runner as dr + + class _BoomAfterLogging(_RunnerDouble): + async def run(self) -> EvaluationResult: + self.rt.run_dir.mkdir(parents=True, exist_ok=True) + (self.rt.run_dir / "docker.log").write_text("OOM: exit 137", encoding="utf-8") + raise dr.DockerRunError("Container exited with code 137 without producing task.json.") + + monkeypatch.setattr(dr, "DockerRunner", _BoomAfterLogging) + + from coder_eval.orchestration.regrade import regrade_in_place + + workspace = tmp_path / "ws" + workspace.mkdir() + task_file = tmp_path / "t.yaml" + task_file.write_text("task_id: t\n", encoding="utf-8") + run_dir = tmp_path / "row" + + with pytest.raises(RegradeError) as excinfo: + await regrade_in_place( + task=_docker_task(), + prior=_result(), + workspace=workspace, + run_dir=run_dir, + task_file=task_file, + source_yaml="", + variant_id="v", + ) + + rescued = run_dir / "grade.docker.log" + assert rescued.read_text(encoding="utf-8") == "OOM: exit 137" + # And the message points at the path that still exists, not the deleted one. + assert str(rescued) in str(excinfo.value) + + async def test_it_refuses_to_follow_a_symlink_at_the_log_destination( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """`shutil.copy2` opens the destination for writing, which FOLLOWS a + symlink there — an arbitrary-file-overwrite primitive in a run directory + the grader did not create. The sibling verdict write goes through + `write_text_atomic` for exactly this reason.""" + from coder_eval.isolation import docker_runner as dr + + graded = _result(final_status=FinalStatus.SUCCESS, weighted_score=1.0) + + class _FakeRunner(_RunnerDouble): + async def run(self) -> EvaluationResult: + self.rt.run_dir.mkdir(parents=True, exist_ok=True) + (self.rt.run_dir / "task.json").write_text(graded.model_dump_json(), encoding="utf-8") + (self.rt.run_dir / "docker.log").write_text("container output", encoding="utf-8") + return graded + + monkeypatch.setattr(dr, "DockerRunner", _FakeRunner) + + from coder_eval.orchestration.regrade import regrade_in_place + + workspace = tmp_path / "ws" + workspace.mkdir() + task_file = tmp_path / "t.yaml" + task_file.write_text("task_id: t\n", encoding="utf-8") + run_dir = tmp_path / "row" + run_dir.mkdir() + victim = tmp_path / "victim.txt" + victim.write_text("do not overwrite me", encoding="utf-8") + (run_dir / "grade.docker.log").symlink_to(victim) + + await regrade_in_place( + task=_docker_task(), + prior=_result(), + workspace=workspace, + run_dir=run_dir, + task_file=task_file, + source_yaml="", + variant_id="v", + ) + + assert victim.read_text(encoding="utf-8") == "do not overwrite me" + + async def test_an_unwritable_destination_fails_as_a_regrade_error( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """The fold-back sits outside the dispatch `try`, so a raw OSError + reached the two callers differently and both were wrong: `evaluate` + guards only RegradeError and let it escape into Typer as a stack trace + AFTER a successful grade, while `run --resume` caught it and reported a + computed, correct verdict as a grading failure.""" + from coder_eval.isolation import docker_runner as dr + from coder_eval.orchestration import regrade as rg + + graded = _result(final_status=FinalStatus.SUCCESS, weighted_score=1.0) + + class _FakeRunner(_RunnerDouble): + async def run(self) -> EvaluationResult: + self.rt.run_dir.mkdir(parents=True, exist_ok=True) + (self.rt.run_dir / "task.json").write_text(graded.model_dump_json(), encoding="utf-8") + return graded + + monkeypatch.setattr(dr, "DockerRunner", _FakeRunner) + + def _boom(path: Path, text: str) -> None: + raise OSError("Read-only file system") + + monkeypatch.setattr(rg, "write_text_atomic", _boom) + + workspace = tmp_path / "ws" + workspace.mkdir() + task_file = tmp_path / "t.yaml" + task_file.write_text("task_id: t\n", encoding="utf-8") + + with pytest.raises(RegradeError, match="could not write the verdict"): + await rg.regrade_in_place( + task=_docker_task(), + prior=_result(), + workspace=workspace, + run_dir=tmp_path / "row", + task_file=task_file, + source_yaml="", + variant_id="v", + ) + + +class TestContainerGradeIsStampedAndCounted: + """The two known equivalence gaps must travel WITH the row. + + `stamp_host_grading`'s own docstring states the rule: a console warning does + not travel with `task.json` into `run.json`, the reports or the evalboard. + Both gaps shipped as `logger.warning` only, so nothing downstream could tell + a row whose `pre_run` never re-ran from one graded at full fidelity — and + three of the ten in-tree `driver: docker` tasks match that pattern. + """ + + @staticmethod + def _dispatch(tmp_path: Path, monkeypatch: pytest.MonkeyPatch, task: TaskDefinition) -> EvaluationResult: + from coder_eval.isolation import docker_runner as dr + + graded = _result(final_status=FinalStatus.SUCCESS, weighted_score=1.0) + + class _FakeRunner(_RunnerDouble): + async def run(self) -> EvaluationResult: + self.rt.run_dir.mkdir(parents=True, exist_ok=True) + (self.rt.run_dir / "task.json").write_text(graded.model_dump_json(), encoding="utf-8") + return graded + + monkeypatch.setattr(dr, "DockerRunner", _FakeRunner) + + from coder_eval.orchestration.regrade import regrade_in_place + + workspace = tmp_path / "ws" + workspace.mkdir() + task_file = tmp_path / "t.yaml" + task_file.write_text("task_id: t\n", encoding="utf-8") + import asyncio + + return asyncio.run( + regrade_in_place( + task=task, + prior=_result(), + workspace=workspace, + run_dir=tmp_path / "row", + task_file=task_file, + source_yaml="", + variant_id="v", + ) + ) + + def test_skipped_pre_run_is_recorded_on_the_row(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + from coder_eval.models import PreRunCommand + + task = _docker_task() + task = task.model_copy(update={"pre_run": [PreRunCommand(command='ln -sfn "$PWD/out.json" /root/out.json')]}) + result = self._dispatch(tmp_path, monkeypatch, task) + assert result.environment_info["graded_without_pre_run"] == 1 + + def test_a_task_without_pre_run_carries_no_stamp(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """The stamp means "this row has a known gap". Writing it on every row + would make it noise, and a marker that is always present filters + nothing.""" + result = self._dispatch(tmp_path, monkeypatch, _docker_task()) + assert "graded_without_pre_run" not in result.environment_info + + def test_a_rebuilt_image_is_recorded_on_the_row(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """`_build_image` re-runs `docker build` under the run's deterministic + tag, so the grading image REPLACES it. Nothing pins image identity on + either side, which is exactly why the row has to say it happened.""" + from coder_eval.models import SandboxConfig + + task = _docker_task() + sandbox = SandboxConfig.model_validate( + { + **task.sandbox.model_dump(), + "docker": {**task.sandbox.docker.model_dump(), "dockerfile_path": "Dockerfile"}, + } + ) + result = self._dispatch(tmp_path, monkeypatch, task.model_copy(update={"sandbox": sandbox})) + assert result.environment_info["graded_with_rebuilt_image"] == "Dockerfile" + + +class TestContainerDispatchIsOneCommandInThePrompt: + """The consent prompt is the one place this text has to be exact. + + `check_embedded_commands` joins the list with "; " and interpolates + `len(commands)`, so an argv FRAGMENT appended as its own entry is reported to + the operator as a standalone shell command. A `dockerfile_path` task with two + build args and one mount asked for approval of "4 shell command(s)", three of + which were not commands. + """ + + @staticmethod + def _dockerfile_task() -> TaskDefinition: + from coder_eval.models import SandboxConfig + + task = _docker_task() + sandbox = SandboxConfig.model_validate( + { + **task.sandbox.model_dump(), + "docker": { + **task.sandbox.docker.model_dump(), + "dockerfile_path": "Dockerfile", + "build": {"args": {"FOO": "bar", "BAZ": "qux"}}, + "extra_mounts": ["/a:/b"], + }, + } + ) + return task.model_copy(update={"sandbox": sandbox}) + + def test_a_dockerfile_dispatch_is_exactly_one_command(self) -> None: + from coder_eval.orchestration.regrade import embedded_commands + + commands = embedded_commands( + self._dockerfile_task(), include_setup_phase=False, include_container_dispatch=True + ) + assert len(commands) == 1 + only = commands[0] + assert only.startswith("docker build -f Dockerfile") + assert "--build-arg FOO=bar" in only + assert "--build-arg BAZ=qux" in only + assert "-v /a:/b" in only + + def test_it_names_the_task_directory_the_dispatch_copies_in(self, tmp_path: Path) -> None: + """Disclosing only `sandbox.docker.*` asked the operator to consent to a + strict subset of what happens. The task DIRECTORY is copied wholesale + from the recorded `source_file`'s parent, so a record naming + `~/.ssh/config` copies all of `~/.ssh` into the record-named image.""" + from coder_eval.orchestration.regrade import embedded_commands + + (commands,) = embedded_commands( + _docker_task(), + include_setup_phase=False, + include_container_dispatch=True, + task_file=tmp_path / "secrets" / "task.yaml", + ) + assert str(tmp_path / "secrets") in commands + assert "~/.claude" in commands + + def test_the_dispatch_is_absent_without_the_flag(self) -> None: + """The whole block is behind `include_container_dispatch`, which is False + on the --copy path — where `grading_sandbox_config` refuses before a + container could be dispatched.""" + from coder_eval.orchestration.regrade import embedded_commands + + assert embedded_commands(self._dockerfile_task(), include_setup_phase=False) == [] + + +class TestContainerGradeEmitsTelemetryHostSide: + """ "Container silent, host emits once" — the grading path had only the first half. + + Every container this repo starts is launched with `TELEMETRY_ENABLED=false` + (`DockerRunner._build_argv`, whose comment states the invariant verbatim). + The RUN path supplies the host half in `orchestration/batch.py` right after + parsing the container's result. The GRADING path inherited the silent half + with no counterpart, so a verdict published by `evaluate ` or + `run --resume` over a `driver: docker` row reached no usage telemetry at all + — while the byte-identical operation on a `tempdir` row, or the same row with + `--allow-host-grading`, did. + """ + + async def test_a_container_grade_emits_task_end(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + import coder_eval.telemetry as telemetry + from coder_eval.isolation import docker_runner as dr + + graded = _result(final_status=FinalStatus.SUCCESS, weighted_score=1.0) + + class _FakeRunner(_RunnerDouble): + async def run(self) -> EvaluationResult: + self.rt.run_dir.mkdir(parents=True, exist_ok=True) + (self.rt.run_dir / "task.json").write_text(graded.model_dump_json(), encoding="utf-8") + return graded + + monkeypatch.setattr(dr, "DockerRunner", _FakeRunner) + events: list[tuple[str, dict]] = [] + monkeypatch.setattr(telemetry, "track_event", lambda name, props: events.append((name, props))) + + from coder_eval.orchestration.regrade import regrade_in_place + + workspace = tmp_path / "ws" + workspace.mkdir() + task_file = tmp_path / "t.yaml" + task_file.write_text("task_id: t\n", encoding="utf-8") + + await regrade_in_place( + task=_docker_task(), + prior=_result(), + workspace=workspace, + run_dir=tmp_path / "row", + task_file=task_file, + source_yaml="", + variant_id="v", + ) + + assert [name for name, _ in events] == ["CoderEval.Task.End"] + assert events[0][1]["Driver"] == "docker"