diff --git a/.claude/harness-candidates.md b/.claude/harness-candidates.md index 03bd25a3..abd49ea1 100644 --- a/.claude/harness-candidates.md +++ b/.claude/harness-candidates.md @@ -992,3 +992,20 @@ re-derive from scratch. such pointer exists today. Deferred because restricting the target is a design decision (a `docs/` guide heading is a plausible SSOT target) rather than a mechanical guard. Caught in: tests prose slimming, final review (gpt-5.6-sol). + +## From container-contract-and-command-surface (2026-09-15) + +- [ ] **A field-name assertion against CLI output can pass vacuously through `tmp_path`.** + `assert "variant_id" in result.output` matched the echoed `context.json` path, because + pytest names each `tmp_path` after the test (`test_a_non_string_variant_id_i…`). Nothing + guards it today; the fix was to assert the pydantic `loc` line (`"\nvariant_id\n"`). + Deferred because telling a vacuous substring from a real one needs to know the test's + own name and what the command echoes — a convention for reviewers, not an AST pattern. + Caught in: Phase 1 quality review. +- [ ] **A path interpolated into Rich markup without `escape()`.** A run directory name is + untrusted, and `[/y]` in it raised `rich.errors.MarkupError` (not an `OSError`) inside + `evaluate`'s best-effort refresh, after the verdict printed. Nothing guards it; the fix + escaped every new console line. Deferred because the rule needs type information (which + f-string placeholders are `Path`s) that an AST-only rule does not have, and the existing + CLI has many pre-existing unescaped lines a literal rule would flag at once. Caught in: + Phase 5 quality review. diff --git a/.claude/notes/isolation.md b/.claude/notes/isolation.md index 70f4b377..56b13447 100644 --- a/.claude/notes/isolation.md +++ b/.claude/notes/isolation.md @@ -58,43 +58,44 @@ (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 + write is what lets `evaluate` refresh the owning run's `run.json` by re-reading its + rows, with no grading-specific aggregation 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 @@ -137,9 +138,9 @@ 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 the dispatch is guarded against an - image that ignores the `regrade` key (see § The two honored-request guards). 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 + image that ignores the `regrade` key (see § The contract echo). 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 @@ -169,32 +170,31 @@ `CoderEval.Task.End` host-side (`_emit_task_telemetry`), mirroring `batch.py`: the grading path had inherited only the silent half of the container-silent invariant (§ Environment forwarding). 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 + driver — the host stages a container's task with `driver: tempdir`, so a driver-based + test would read an already-resolved 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 @@ -223,18 +223,18 @@ 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 (see .claude/notes/orchestration.md § The in-container - driver rewrite), 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 + entirely. The record must also describe the task as AUTHORED, not as executed: the + host stages a container's task with `driver: tempdir` and forwards the authored + sandbox beside it (see .claude/notes/orchestration.md § The host-side driver rewrite), + 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 @@ -342,6 +342,25 @@ host — and it is atomic, matching the orchestrator's own writer, because a tor the row parse as malformed, which a later `--resume` reads as "not complete" and re-pays for the agent. +After the write-back, `evaluate` rebuilds the `run.json` of the run that owns the row — the +nearest ancestor holding one — so a detached grade needs no second command. The rebuild +re-reads every row on disk, so replicate siblings are summarized at their current state and a +quarantined `task.json.unhonored` is never folded in. It is best-effort like the write-back: +the verdict is already printed, so a failed refresh warns and never changes the exit code. It +refuses to write through a symlinked `run.json` or `run.md` for the same reason the write-back +refuses a symlinked `task.json`, and the refusal sits in `rebuild_run_summary` itself — the +one write path `report --rebuild` shares — so neither caller can skip it. A row with no +`run.json` above it (copied out of its run) gets no summary: creating one would invent a run. + +The walk-up accepts only a `run.json` that is a JSON object with `run_id` and `task_results`. +The name is generic, and a row copied into a project or home directory that holds another +tool's `run.json` would otherwise have that file silently overwritten by a plain `evaluate`. +The refresh is also skipped when the grading pass's own `--run-dir` sits inside the owning +run: the orchestrator writes that pass's `task.json` there, so the rebuild would count the +same row twice. `run.json` and `run.md` are written through `write_text_atomic`, because +the refresh now runs on every detached grade, and a torn `run.json` makes the next rebuild +silently drop the tags, paths and window it carries forward. + The pre-grade snapshot is taken BEFORE anything grades. Taking it inside `_write_back` captured an ALREADY-GRADED record whenever `--run-dir` pointed at the target run dir (the orchestrator writes there first), destroying the evidence the copy exists to preserve. @@ -458,9 +477,9 @@ one task. On the host (`driver: tempdir`) it is a deliberate no-op: parallel tas batch share the checked-out `tasks//` tree, so chmod-ing it is a cross-task side effect on the user's own working copy for no isolation benefit — there is no boundary to enforce when the agent is just another process with the same uid. The predicate is the -`CODER_EVAL_IN_CONTAINER` env var, NOT `config.driver`, because the in-container entry -point rewrites `driver: docker` to `tempdir` before constructing the orchestrator, so a -driver-keyed predicate would read "tempdir" inside the container and disable the window on +`CODER_EVAL_IN_CONTAINER` env var, NOT `config.driver`, because the host stages a +container's task with `driver: tempdir`, so a driver-keyed predicate would read +"tempdir" inside the container and disable the window on exactly the path that needs it. ### grant_container_access @@ -654,22 +673,33 @@ routinely be lost, making a genuine stale-heartbeat exit indistinguishable from SIGKILL in the archived logs. Flush best-effort first; never let a flush failure stop the exit. -### The context payload is untrusted input - -`context.json` is the host→container boundary, and every value crossing it is COERCED, not -merely annotated. `json.loads` returns `Any`, so pyright accepts `variant_id: str = -context["variant_id"]` for a value that may be anything at all — the annotation reads like a -guarantee and enforces nothing, and a `"replicate_index": "00"` reached `build_task_run_dir` -typed as `int`. `grade` was once the only value coerced: a hand-edited or older-format -`"grade": "false"` arrives as a truthy `str` typed as `bool` and silently grades a run that -asked not to be graded. `regrade` is coerced for the same reason, and getting that one wrong -re-RUNS the agent against a workspace the operator asked only to grade, destroying the -trajectory being graded. - -Keys absent on an older host fall back to the pre-existing behaviour rather than failing: -`grade` defaults to True, `preservation_mode` to the docker default (a deliberate default, -not back-compat — this command only ever runs under the docker driver), `host_task_file` and -`workspace_dir` to None, and `source_yaml` to the staged post-override YAML. +### The container contract + +`context.json` is the host→container boundary, and it is parsed as one `ContainerContext` +rather than read key by key. `json.loads` returns `Any`, so pyright accepts `variant_id: str += context["variant_id"]` for a value that may be anything at all — the annotation reads like +a guarantee and enforces nothing, and a `"replicate_index": "00"` reached +`build_task_run_dir` typed as `int`. `grade` and `regrade` are `StrictBool` because lax +coercion is itself the defect: a hand-edited `"grade": "false"` is a truthy string that +silently grades a run that asked not to be graded, and the same mistake on `regrade` re-RUNS +the agent against a workspace the operator asked only to grade, destroying the trajectory +being graded. `replicate_index` is `StrictInt` because a bool is an int, and `True` files the +row under `01/`. + +Every field is required and unknown top-level keys are refused. A default on any key is +reachable only +when host and image DISAGREE — `grade: True` for a host that predates `execute`, +`host_task_file: None` for one that predates the record seam — so a default does not +preserve behaviour, it hides a skew. With `extra="forbid"` and no defaults, that disagreement +is a parse failure naming the field, in both directions: an older host omits a key, a newer +host sends a key the image does not know. `host_task_file` and `workspace_dir` are required +keys with nullable values, because `null` is a real answer (no task file; the standard +workspace) and absence is not. `tests/test_container_context.py` derives its checks from +`model_fields`, so a field added with a default fails there. + +No explicit contract-version field exists. It would answer the question the image's +`org.coder-eval.version` label and this parse already answer, with no bump policy: an image +older than the field ignores it, and one newer always agrees. The host always serialises the POST-override `TaskDefinition` into the staged `task.yaml`, never `source_yaml`, because the raw on-disk text predates `--model` and `-D` mutations the @@ -724,36 +754,80 @@ Suppression is narrowed to `CancelledError` throughout, so a genuine `KeyboardIn the container was already gone (a race with `--rm`) or the daemon refused, so stderr is surfaced to keep the ambiguity debuggable. -### The two honored-request guards - -`grade` and `regrade` cross the boundary only through `context.json`, and an image that -predates either key ignores it and falls through to its old behaviour. The image-version -preflight only warns, so version skew would change what a command MEANS. - -For `grade`: a stale image grades anyway, so `execute --driver docker` would silently -produce SUCCESS/FAILURE rows indistinguishable from a normal graded run. For `regrade`: a -stale image ignores the staged `prior.json` and the workspace mount and falls through to the -ordinary orchestrator branch, which **starts an agent** from `initial_prompt` — 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. Nothing else catches that: -`_assert_grade_honored` early-returns because a grading container is dispatched with -`grade=True`. - -Both are keyed on EVIDENCE, not on the label. For `grade`, "did it grade" is -`success_criteria_results` or a non-None `weighted_score`: exempting every execution-fact -status let a stale image return a fully graded MAX_TURNS_EXHAUSTED row — criteria vector, -weighted score and all — unchallenged, because that exemption exists for statuses a *fresh* -image also produces, and a fresh one produces them with neither. For `regrade`, a container -that honored the request seeds from `prior` and never runs the agent, so a DIFFERENT -`started_at` is the tell: `_seed_from_prior_result` restores the agent run's `started_at` -verbatim, so a fresh run is the only way that field can move. - -The refusal quarantines the on-disk record before raising. Refusing in memory only left the -graded `task.json` sitting in the bind-mounted host run dir, where a later `execute --resume` -read it back as a completed row (its category is `succeeded`, so the resume partition files -it under prior results) and plain `aggregate` folded it straight into `run.json` — publishing -exactly the row the guard declined to publish. Refusing in memory while leaving contradictory -bytes on disk is not a refusal. +### The image version preflight + +Before a billed container starts, the host reads the image's `org.coder-eval.version` label +once. A missing label refuses: the image cannot run the in-container orchestrator, and a bare +`FROM ubuntu` would otherwise build fine and then die at `docker run` with a cryptic +missing-entrypoint error. A label that differs from the host's installed coder-eval also +refuses. `dockerfile_path` images are checked the same way, because a task Dockerfile must +start `FROM coder-eval-agent` and so inherits the label. + +A mismatch is a legitimate operator choice in one known case — testing an unreleased image +against a released host wheel, which a downstream CI workflow does on purpose — so +`ALLOW_IMAGE_SKEW=1` downgrades it to a warning, and the refusal message names the variable +so the operator can recover from the error text alone. It is a `Settings` field, not a task +field: image freshness is a property of the operator's machine, not of the evaluation being +defined. It never excuses a missing label. A label reading `unknown` — what `docker/Dockerfile` +stamps when built without `make`, which passes no `CODER_EVAL_VERSION` — is a version that +differs, not a missing label, so the escape hatch applies to it: such an image does carry the +runtime. A source checkout has no packaged version, so skew +is not computable there; the preflight warns and continues. An inspect or daemon failure is +left for `docker run` to report canonically. + +The label is ADVISORY. It is a build-time claim every derived image inherits, and an overlay +that reinstalls or patches coder-eval in the container keeps the base's label while changing +what runs; a stale image rebuilt under the same version string is the same case. The preflight +buys an early, cheap failure before a paid run. The contract echo is the authoritative check. + +### The contract echo + +The container writes the `ContainerContext` it actually parsed back into +`environment_info["container_contract"]`, and the host refuses a result whose echo is absent +or differs from what it staged, naming each differing field. One comparison covers every +field, so a field added to the contract is guarded with no new code. An absent echo means the +image predates the contract; a different one means the container ran code that read the +contract differently. + +Both directions that matter most are expensive. An image that ignores `grade` makes `execute +--driver docker` publish SUCCESS/FAILURE rows indistinguishable from a graded run. An image +that ignores `regrade` falls through to the ordinary orchestrator branch, which **starts an +agent** from `initial_prompt` — 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. Guards keyed on indirect evidence for each flag (criteria results for `grade`, +a moved `started_at` for `regrade`) had to be written per field and each had its own blind +spot; the echo asks the direct question once. + +The echo is written LATE, after `_finalize_regrade_timing`. `_seed_from_prior_result` merges +the prior row's `environment_info` over ours with the prior winning, so an echo written at +setup is erased on the regrade path, which is the path that most needs it — and a +previously graded row already carries a stale echo that would win. +`tests/test_container_context.py` drives that path end to end. Both sides compare +`model_dump(mode="json")`, since enum and path objects do not equal their JSON forms. +`ALLOW_IMAGE_SKEW` never reaches this check: it tolerates a version difference, never a +container that did something other than what it was asked. + +The echo detects a skewed image; it is NOT a security boundary. The run dir is bind-mounted +writable into the agent's own container, so an agent can forge `container_contract` exactly as +it can forge the verdict beside it in the same `task.json`. It proves the harness code honored +the contract, not that the agent was honest. A host grade (`--allow-host-grading`) removes a +prior row's echo, since that echo describes a container that did not produce the new verdict. +The key is excluded from the rendered Environment table: it is a nested object, and +`environment_info` is rendered as a flat map. + +The refusal quarantines the on-disk record to `task.json.unhonored` before raising, and a +synthetic ERROR `task.json` takes its place, as it does for a container that wrote no record: +a refused row that simply vanished would drop out of every later rebuild of `run.json` while +the batch's in-memory summary still counted it. A detached grade runs its container in a +scratch directory, so the refused record is folded back beside the graded row with the +grading logs. The staged `prior.json` carries no echo: an image that predates the echo keeps +the prior row's `environment_info`, and a matching echo from an earlier identical dispatch +would otherwise pass as its own. The run +dir is bind-mounted, so a refused `task.json` left in place is read straight back by a later +`execute --resume` (its category is `succeeded`, so the resume partition files it under prior +results) and folded into `run.json` by a run-level rebuild — publishing exactly the row the +refusal declined. A build failure's synthetic `BUILD_FAILED` record and a container that +wrote no `task.json` never reach the check: both raise earlier. ## The sandbox the criteria run in diff --git a/.claude/notes/lint-rules.md b/.claude/notes/lint-rules.md index 7d73e8bc..02491bb4 100644 --- a/.claude/notes/lint-rules.md +++ b/.claude/notes/lint-rules.md @@ -70,7 +70,7 @@ boundary is mechanically detectable, so the rule guards it. crosses the container boundary, and the per-task record every dashboard and timeline reads. A bare `EvaluationResult.model_validate_json(text)` turns a present-but-malformed file into an uncaught exception that crashes the run. Two causes produce such a file: schema -skew between a stale `:latest` image and the host (the docker version checks only warn), +skew between a stale `:latest` image and the host that the image preflight did not catch, and a truncated or torn write. The incident was at `docker_runner.py`: the parse re-bucketed the task to a non-persisted in-memory ERROR with no per-task report. The fix degrades: catch `ValueError` and persist a synthetic ERROR record (`batch.py::_load_completed_result`, @@ -517,8 +517,8 @@ host. The motivating bug: `regrade.grading_sandbox_config` rewrote the driver unconditionally on BOTH new grading entry points, which also neutralized the `driver: docker` refusal in `Sandbox.adopt` — a guard added in the same change specifically to catch this. The -legitimate suppressions are the in-container rewrite in `run_task_internal_command` and -the opt-in host-grading branch, which refuses by default and stamps `graded_on_host` on +legitimate suppressions are the host-side staging rewrite in `docker_runner._stage_inputs` +and the opt-in host-grading branch, which refuses by default and stamps `graded_on_host` on the row. ## CE052 diff --git a/.claude/notes/orchestration.md b/.claude/notes/orchestration.md index a3042f95..a1b10c4c 100644 --- a/.claude/notes/orchestration.md +++ b/.claude/notes/orchestration.md @@ -42,20 +42,20 @@ 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.py` 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. + (a required contract field with no default, so a host and image that disagree about it + fail at parse time). 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.py` 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. ### The terminal-status chain @@ -176,8 +176,8 @@ on both the success and the raised path, without catching the `typer.Exit` decid Per-suite rollups are skipped entirely under `execute`: a rollup aggregates per-criterion results and there are none, so running it would gate a suite on an empty aggregate and report a threshold failure for a run that was never measured. The ungraded bucket is named -explicitly in the aggregate line for the same reason — `coder-eval aggregate ` is the -step right after `coder-eval execute`, so an ungraded run is the FIRST thing it renders, and +explicitly in the rebuild line for the same reason — `coder-eval report --rebuild` is +the step right after `coder-eval execute`, so an ungraded run is the FIRST thing it renders, and without the term it reads "Aggregated 12 task(s) (0 ok / 0 fail / 0 err)": four numbers that no longer sum to `tasks_run`, with nothing on screen to say where the rest went. The end-of-run summary likewise reports what happened instead of "0/N succeeded", which for a @@ -478,9 +478,9 @@ rather than errors live in the run-limits validator for the same post-merge visi `task_config.resolved` and `source_file` describe the task as AUTHORED, which is NOT always what this process runs. -`run_task_internal_command` rewrites `driver: docker` → `tempdir` before building the -in-container orchestrator, because it is already inside the container the driver asked -for. Recording that rewrite made the run's own record deny it ever used docker — and a +The host stages a `driver: docker` task for its container with `driver: tempdir`, because +the container is the isolation the driver asked for. Recording that execution copy made +the run's own record deny it ever used docker — and a later `evaluate ` reads the driver back out of the record, so the host-grading refusal never fired and the `graded_on_host` stamp was never applied. A container task's criteria ran against the host filesystem silently, which is the exact outcome that gate @@ -494,17 +494,29 @@ around it: the docker dispatch guard saw a non-None `Path` and let it through, a task-dir mount then silently mounted nothing, so every `$TASK_DIR` criterion resolved against the wrong tree and scored a verdict nobody could explain. -### The in-container driver rewrite - -CE051 forbids rewriting `sandbox.driver`, and this is its single exemption: the process is -already inside the container the docker driver asked for, so the isolation the driver names -is present rather than bypassed, and a nested docker would be both wrong and impossible (no -docker CLI in the image). The rewrite goes through `model_validate` rather than -`model_copy(update=...)`, matching its sibling in `regrade.grading_sandbox_config`: `update` -skips BOTH pydantic and pyright, so a typo produces a `SandboxConfig` violating its own -`Literal` and only surfaces far downstream. Two driver-rewrite sites landing in one change -with two different levels of type safety is how the weaker one becomes the pattern people -copy. +### The host-side driver rewrite + +CE051 forbids rewriting `sandbox.driver` silently, and `DockerRunner._stage_inputs` is one of +its two exemptions: the host resolves the driver for the container it is itself about to +start, so the isolation the driver names is present rather than bypassed, and a nested docker +inside the image would be both wrong and impossible (no docker CLI in it). The rewrite happens +where both values are in hand — the staged `task.yaml` carries the execution copy and +`ContainerContext.authored_sandbox` carries the block as authored, which the container +records. Doing it inside the container instead took a rewrite plus a "captured BEFORE the +rewrite" local in the consumer, and a lint exemption for code on the far side of the boundary. + +The rewrite goes through `model_validate` rather than `model_copy(update=...)`, matching its +sibling in `regrade.grading_sandbox_config`: `update` skips BOTH pydantic and pyright, so a +typo produces a `SandboxConfig` violating its own `Literal` and only surfaces far downstream. +Two driver-rewrite sites with two different levels of type safety is how the weaker one +becomes the pattern people copy. + +The two sites are deliberately NOT collapsed into a `SandboxConfig.as_tempdir()` helper. +CE051 exempts `models/sandbox.py` outright ("the model's own construction"), so moving the +rewrite there would take both call sites out of the rule's view and turn a guarded operation +into an unguarded one-liner any future caller could reach. Each site carries a different +reason in its `noqa`, and that reason text is the operator-visible control the rule exists to +force. ## Three routes, resolved separately diff --git a/.claude/shared/run-layout.md b/.claude/shared/run-layout.md index c00a82c6..2fd724a1 100644 --- a/.claude/shared/run-layout.md +++ b/.claude/shared/run-layout.md @@ -13,7 +13,7 @@ runs/////{task.json, task.log, artifacts/} - `task.json` — the persisted per-replicate result (the consumer contract; carries the large `iterations` array — still accepted under its former name `turns` when reading, but not what current runs write). - `task.json.malformed` — present only on the docker degrade path: when an existing `task.json` fails to parse (schema skew from a stale `:latest` image, or a truncated/torn write), the docker runner moves the unparseable original aside to this sidecar and writes a synthetic `final_status=ERROR` `task.json` in its place. Diagnostic-only; `rglob("task.json")` consumers do not match it. - `task.execute.json` — present only after a DETACHED grade (`coder-eval evaluate ` or `coder-eval run --resume` over a `NOT_GRADED` row). The pre-grade snapshot of `task.json`, written once and never overwritten by a later grade, so "this run was executed separately from grading" stays auditable. Diagnostic-only; `rglob("task.json")` consumers do not match it. -- `task.json.graded` — present only after `coder-eval execute --driver docker` refused a container's verdict: the runtime image predated `execute` and graded anyway, so the runner quarantines the graded record here rather than leaving it readable as `task.json`, where a later `--resume` / `aggregate` would fold in exactly the row it declined to publish. Diagnostic-only; `rglob("task.json")` consumers do not match it. +- `task.json.unhonored` — present only after the docker runner refused a container's result because it did not echo the contract the host staged (`environment_info.container_contract` absent or different — for example an image that predates `execute` and graded anyway, or one that re-ran the agent instead of grading). The runner quarantines the record here rather than leaving it readable as `task.json`, where a later `--resume` or run-level rebuild would fold in exactly the row it refused, and writes a synthetic `final_status=ERROR` `task.json` in its place so the row stays visible. On a detached grade the refused record is folded back beside the graded row. Diagnostic-only; `rglob("task.json")` consumers do not match it. - `grade.log` — present only after a DETACHED grade over this directory (`coder-eval run --resume`). The grading pass's own log. It is a separate file because the log handler truncates whatever file it opens, so writing to `task.log` would destroy the agent trajectory log the run already paid for. - `task.log` — the human-readable task log; `artifacts/` — files the agent produced. diff --git a/docker/Dockerfile.runtime b/docker/Dockerfile.runtime index aa8a01dd..fd79c373 100644 --- a/docker/Dockerfile.runtime +++ b/docker/Dockerfile.runtime @@ -14,7 +14,7 @@ # /usr/local/bin/coder_eval_entrypoint.sh entrypoint (host pins --entrypoint here) # LABEL org.coder-eval.version= for `docker inspect` / parity # -# NOTE: the host's _assert_runtime_image gate inspects the *injected task image*, +# NOTE: the host's _preflight_image_contract gate inspects the *injected task image*, # not this kit image — labels don't survive `COPY --from`. The converter re-stamps # org.coder-eval.version on the injected image (that's the label the host asserts). # This kit carries it too purely for `docker inspect`/parity. diff --git a/docs/DOCKER_ISOLATION.md b/docs/DOCKER_ISOLATION.md index 14850092..f898e3b5 100644 --- a/docs/DOCKER_ISOLATION.md +++ b/docs/DOCKER_ISOLATION.md @@ -105,8 +105,9 @@ coder-eval run tasks/byod_smoke_test.yaml | Symptom | Cause and fix | | --- | --- | | `docker: Error response from daemon: pull access denied` | The image isn't built locally and isn't pullable. Check `docker images`, then rebuild it. Docker treats an unknown local tag as a remote reference, which is why the error mentions a pull. | -| `Image coder_eval != host ` | The custom image carries an `org.coder-eval.version` label inherited from a stale framework base. Rebuild the base with `make docker-image`, then rebuild your derived image with `docker build --no-cache`. | -| `Image has no org.coder-eval.version label` | The image doesn't descend from `coder-eval-agent` (or predates the label). Rebase it on the framework image, or use the runtime kit. | +| `Image runs coder_eval but the host runs ` | The run is refused before the container starts: the image carries an `org.coder-eval.version` label from a different coder-eval than the one installed on the host, usually inherited from a stale framework base. Rebuild the base with `make docker-image`, then rebuild your derived image with `docker build --no-cache`. To run a deliberately different image anyway (for example an unreleased build under test), set `ALLOW_IMAGE_SKEW=1` in the environment or `.env`; the mismatch then only warns, and the run gives up the reproducibility guarantee. | +| `Image is not a coder-eval runtime image (missing the org.coder-eval.version label)` | The image doesn't descend from `coder-eval-agent` (or predates the label). Rebase it on the framework image, or use the runtime kit. `ALLOW_IMAGE_SKEW` does not bypass this. | +| `The container returned a result with no container_contract echo` or `The container did not honor the contract it was sent` | The code inside the image does not match the host's, even if its label agrees — for example an overlay image that reinstalled coder-eval, or a stale image rebuilt under the same tag. The result is refused: the record is moved to `task.json.unhonored` and a synthetic ERROR `task.json` takes its place. Rebuild or pull a matching image. `ALLOW_IMAGE_SKEW` does not bypass this. | ## Building the image from a task Dockerfile @@ -183,9 +184,10 @@ Behavior: `coder-eval-task-:built`, so repeat runs of the same task reuse Docker's layer cache. Edit the Dockerfile and the next run rebuilds the changed layers only. -- **Version-label check skipped** — the `org.coder-eval.version` preflight only - applies to the framework image; task-built images don't carry it and won't - warn. +- **Version-checked too** — the built image inherits `org.coder-eval.version` + from its `FROM coder-eval-agent` base, so the same preflight applies: a missing + label, or a version that differs from the host's, refuses the run (see + [Troubleshooting custom images](#troubleshooting-custom-images)). A build failure aborts the task with a `DockerBuildError` (a `DockerRunError` subclass) carrying `docker build`'s output. Because the build runs before the diff --git a/docs/REPORT_SCHEMA.md b/docs/REPORT_SCHEMA.md index b232a3d5..4d740cfb 100644 --- a/docs/REPORT_SCHEMA.md +++ b/docs/REPORT_SCHEMA.md @@ -11,7 +11,7 @@ Coder Eval writes machine-readable JSON alongside every markdown/HTML report. Th page is the field-level reference for consumers (dashboards, CI parsers, evalboard forks). For the on-disk directory tree see [User Guide → Output Structure](USER_GUIDE.md#output-structure); for how to -re-generate these files see [`coder-eval report` / `aggregate`](USER_GUIDE.md#cli-commands). +re-generate these files see [`coder-eval report`](USER_GUIDE.md#cli-commands) (`--rebuild` for `run.json`). All JSON is Pydantic `model_dump_json` output — keys are the model field names verbatim (no aliases, except `iterations` also accepts the legacy key `turns` on @@ -21,7 +21,7 @@ read). Times are ISO-8601. | File | Model | When | | --- | --- | --- | -| `run.json` / `run.md` | `RunSummary` | Every run (and rebuildable via `coder-eval aggregate`) | +| `run.json` / `run.md` | `RunSummary` | Every run; refreshed by `coder-eval evaluate ` and rebuildable via `coder-eval report --rebuild` | | `///task.json` | `EvaluationResult` | One per replicate | | `///task.execute.json` | `EvaluationResult` | Pre-grade snapshot, written once by a detached grade (`evaluate ` / `run --resume`). Deliberately **not** matched by `rglob("task.json")`, so it never enters an aggregation. | | `//suite.json` / `.md` | `SuiteRollup` | Dataset-backed suites only | @@ -361,6 +361,6 @@ respectively), checked after each completed agent turn — see ## See also - [User Guide → Output Structure](USER_GUIDE.md#output-structure) and the - [`aggregate`](USER_GUIDE.md#cli-commands) command + [`report --rebuild`](USER_GUIDE.md#cli-commands) command - [A/B Experiments → Reading the Report](AB_EXPERIMENTS.md#reading-the-report) - [Task Definition Guide](TASK_DEFINITION_GUIDE.md) diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 2d93d36a..7c3b333a 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -176,8 +176,7 @@ coder-eval evaluate tasks/hello_date.yaml ./my_solution # 2. Re-grade a finished run — including one left NOT_GRADED by `execute` coder-eval execute tasks/hello_date.yaml --run-dir ./r -coder-eval evaluate ./r/default/hello_date/00 -coder-eval aggregate ./r # run.json now reports the verdict +coder-eval evaluate ./r/default/hello_date/00 # grades the row and refreshes ./r/run.json ``` **Run-directory mode** rebuilds the task from the run's own recorded @@ -189,8 +188,11 @@ criteria that read the agent's tool calls (`command_executed`, `skill_triggered` judges with trajectory) score exactly as they would have during the run. It writes the verdict back into the run's `task.json` and keeps the pre-grade -record beside it as `task.execute.json`. Writing back in place is what makes -`aggregate` free — no new flag, no second copy of the results. If grading itself +record beside it as `task.execute.json`. It then rebuilds the run's +`run.json` from the rows on disk, so `run.json` reports the verdict with no second command +and no second copy of the results. A row that is not inside a run directory gets no +`run.json`, a symlinked `run.json` is refused, and a failed refresh only warns — the exit +code is always the verdict's. If grading itself crashes, the ungraded record is put back: `ERROR` counts as complete for both commands, so an errored row could never be graded again. @@ -286,6 +288,7 @@ new answer key. coder-eval report runs/latest # view latest run (markdown to stdout) coder-eval report runs/latest -o summary.md # export markdown to a file coder-eval report runs/latest --format html # (re)render every task.json as task.html +coder-eval report runs/2026-06-22_14-32-27 --rebuild # rebuild run.json + run.md in place ``` The `run` command already writes reports during execution; `report` re-displays or @@ -296,26 +299,23 @@ reads, see [Output Structure](#output-structure) and the | Flag | Description | | --- | --- | | `--output, -o` | Write to a file instead of stdout (markdown). | -| `--format, -f` | `md` (default) or `html`. `html` re-renders each `task.json` under the run dir to a `task.html` beside it (or to `-o` when exactly one task is found). | - -### `coder-eval aggregate` — rebuild `run.json` from task results - -```bash -coder-eval aggregate runs/2026-06-22_14-32-27 # rebuild the summary in place -coder-eval aggregate runs/combined -o runs/combined # aggregate a merged dir -``` - -Re-derives the run-level `run.json` + `run.md` from the finalized `task.json` files -already on disk, using the same builder a live run uses. Use it when a run dir's -top-level summary is missing or stale — e.g. after recovering an interrupted run or -combining several run directories. It rebuilds the **run-level summary only**; -per-suite rollups (`suite.json`/`suite.md`) and experiment reports -(`experiment.json`/`experiment.md`) are *not* rebuilt, because the per-row -suite/variant grouping they need is not recoverable from `task.json` alone. - -| Flag | Description | -| --- | --- | -| `--output, -o` | Write `run.json`/`run.md` into this directory instead of the run dir (e.g. a merged output dir). | +| `--format, -f` | `md` (default), `html` or `junit`. `html` re-renders each `task.json` under the run dir to a `task.html` beside it (or to `-o` when exactly one task is found); `junit` writes JUnit XML from `run.json`. | +| `--rebuild` | Rebuild the run-level `run.json` + `run.md` in place from the finalized `task.json` files under the run dir. Cannot be combined with `--format` or `--output`. | + +**Rebuilding `run.json`.** `--rebuild` re-derives the run-level `run.json` + `run.md` +from the finalized `task.json` files already on disk, using the same builder a live run +uses. Use it when a run dir's top-level summary is missing or stale — e.g. after +recovering an interrupted run or combining several run directories. Point it at the run +root (the directory that holds `run.json`); a task directory, or any directory inside a +run, is refused. It writes in place: to summarize a combined directory, gather the task +directories into it first, then rebuild that directory. Copy the task directories, not +whole run directories — a copied run keeps its own `run.json`, so its rows belong to it and +are left out. It rebuilds the **run-level summary only**; per-suite rollups +(`suite.json`/`suite.md`) and experiment reports (`experiment.json`/`experiment.md`) are +*not* rebuilt, because the per-row suite/variant grouping they need is not recoverable +from `task.json` alone. It exits 1 when no finalized `task.json` is found. A rebuild — +including the one `coder-eval evaluate ` runs — records the rebuilding host's +coder-eval version and environment in `run.json`. ### Claude Code slash commands diff --git a/plugins/coder-eval/reference/run-layout.md b/plugins/coder-eval/reference/run-layout.md index ee94d736..6c80f355 100644 --- a/plugins/coder-eval/reference/run-layout.md +++ b/plugins/coder-eval/reference/run-layout.md @@ -12,7 +12,7 @@ runs/////{task.json, task.log, artifacts/} - `task.json` — the persisted per-replicate result (the consumer contract; carries the large `iterations` array — still accepted under its former name `turns` when reading, but not what current runs write). - `task.json.malformed` — present only on the docker degrade path: when an existing `task.json` fails to parse (schema skew from a stale `:latest` image, or a truncated/torn write), the docker runner moves the unparseable original aside to this sidecar and writes a synthetic `final_status=ERROR` `task.json` in its place. Diagnostic-only; `rglob("task.json")` consumers do not match it. - `task.execute.json` — present only after a DETACHED grade (`coder-eval evaluate ` or `coder-eval run --resume` over a `NOT_GRADED` row). The pre-grade snapshot of `task.json`, written once and never overwritten by a later grade, so "this run was executed separately from grading" stays auditable. Diagnostic-only; `rglob("task.json")` consumers do not match it. -- `task.json.graded` — present only after `coder-eval execute --driver docker` refused a container's verdict: the runtime image predated `execute` and graded anyway, so the runner quarantines the graded record here rather than leaving it readable as `task.json`, where a later `--resume` / `aggregate` would fold in exactly the row it declined to publish. Diagnostic-only; `rglob("task.json")` consumers do not match it. +- `task.json.unhonored` — present only after the docker runner refused a container's result because it did not echo the contract the host staged (`environment_info.container_contract` absent or different — for example an image that predates `execute` and graded anyway, or one that re-ran the agent instead of grading). The runner quarantines the record here rather than leaving it readable as `task.json`, where a later `--resume` or run-level rebuild would fold in exactly the row it refused, and writes a synthetic `final_status=ERROR` `task.json` in its place so the row stays visible. On a detached grade the refused record is folded back beside the graded row. Diagnostic-only; `rglob("task.json")` consumers do not match it. - `grade.log` — present only after a DETACHED grade over this directory (`coder-eval run --resume`). The grading pass's own log. It is a separate file because the log handler truncates whatever file it opens, so writing to `task.log` would destroy the agent trajectory log the run already paid for. - `task.log` — the human-readable task log; `artifacts/` — files the agent produced. diff --git a/src/coder_eval/cli/__init__.py b/src/coder_eval/cli/__init__.py index 6b95d108..f1b3a670 100644 --- a/src/coder_eval/cli/__init__.py +++ b/src/coder_eval/cli/__init__.py @@ -4,7 +4,6 @@ from coder_eval.telemetry import track_command -from .aggregate_command import aggregate_command from .console import console from .evaluate_command import evaluate_command from .execute_command import execute_command @@ -54,7 +53,6 @@ def main( - plan: Validate task files (dry-run) - evaluate: Grade a directory against a task, or re-grade a finished run - report: Display or export evaluation reports - - aggregate: Rebuild run.json/run.md from finalized task.json files """ # Discover and register agents (built-in + third-party plugins) before any # subcommand resolves a task or builds an agent. @@ -83,7 +81,6 @@ def main( app.command(name="plan")(track_command("plan")(plan_command)) app.command(name="evaluate")(track_command("evaluate")(evaluate_command)) app.command(name="report")(track_command("report")(report_command)) -app.command(name="aggregate")(track_command("aggregate")(aggregate_command)) app.command(name="export")(track_command("export")(export_command)) harbor_app.command(name="reward")(track_command("harbor-reward")(reward_command)) app.add_typer(harbor_app, name="harbor") diff --git a/src/coder_eval/cli/aggregate_command.py b/src/coder_eval/cli/aggregate_command.py deleted file mode 100644 index 511cbc3e..00000000 --- a/src/coder_eval/cli/aggregate_command.py +++ /dev/null @@ -1,153 +0,0 @@ -"""Aggregate command — (re)build run.json + run.md from finalized task.json files. - -The standalone aggregation step: where `coder-eval run` folds the live batch into -run.json/run.md at the end of execution, this re-aggregates the same artifacts -afterwards from the finalized task.json files already on disk — for a run dir -whose top-level run.json is missing or stale (e.g. after recovering or combining -run dirs). It reuses the exact builder a live run uses (`build_run_summary`), so -the counts and version chip are identical. -""" - -import json -from datetime import datetime, timedelta -from pathlib import Path -from typing import Any - -import typer -from pydantic import ValidationError - -from ..models import SkippedTask, TaskResult -from .console import console - - -def aggregate_command( - run_dir: Path = typer.Argument( # noqa: B008 - ..., - help="Run directory holding finalized task.json files (e.g. runs/2026-06-22_14-32-27).", - exists=True, - file_okay=False, - ), - output_dir: Path | None = typer.Option( # noqa: B008 - None, - "--output", - "-o", - help="Directory to write run.json/run.md into instead of run_dir (e.g. a merged output dir).", - file_okay=False, - ), -) -> None: - """(Re)build run.json + run.md by aggregating the finalized task.json files under a run dir. - - Rebuilds the run-level summary only. Per-suite rollups (suite.json/suite.md) and - experiment reports (experiment.json/experiment.md) that a live ``run`` produces are - NOT rebuilt — the per-row suite/variant grouping they need is not recoverable from - task.json alone. - - Examples: - # Rebuild a run's summary in place - coder-eval aggregate runs/2026-06-22_14-32-27 - - # Aggregate a combined dir's task results into a fresh summary - coder-eval aggregate runs/combined -o runs/combined - """ - from ..orchestration.batch import build_run_summary, recover_task_results, write_run_summary - - results = recover_task_results(run_dir) - if not results: - console.print(f"[red]Error: no finalized task.json files found under {run_dir}[/red]") - console.print("\n[dim]Hint: this aggregates a finished run — use 'coder-eval run' to create one.[/dim]") - raise typer.Exit(1) - - out_dir = output_dir or run_dir - - # Inputs (static task metadata), not results, so carrying them from an existing - # run.json is safe even when that summary is stale. - task_tags, task_paths, prior = _read_prior_metadata(run_dir) - start_time, end_time = _resolve_window(results, prior) - skipped = _recover_skipped_tasks(prior) - - summary = build_run_summary( - out_dir.name, - results, - start_time, - end_time, - task_tags, - task_paths=task_paths, - max_parallel=int(prior.get("max_parallel", 1) or 1), - skipped_tasks=skipped, - ) - write_run_summary(summary, out_dir) - # The fourth bucket is named here too: `aggregate` is the step right after - # `execute`, so an ungraded run is the FIRST thing this line renders. - # Rationale: .claude/notes/orchestration.md § What the exit code counts - counts = f"{summary.tasks_succeeded} ok / {summary.tasks_failed} fail / {summary.tasks_error} err" - if summary.tasks_not_graded: - counts += f" / {summary.tasks_not_graded} not graded" - console.print(f"[green][OK][/green] Aggregated {summary.tasks_run} task(s) ({counts}) → {out_dir / 'run.json'}") - console.print( - "[dim]Note: run-level summary only — per-suite (suite.json/suite.md) and " - + "experiment (experiment.json/experiment.md) rollups are not rebuilt.[/dim]" - ) - - -def _read_prior_metadata(run_dir: Path) -> tuple[dict[str, list[str]], dict[str, str], dict[str, Any]]: - """Pull per-task tags/source-paths + run-level fields from an existing run.json. - - Returns ``(task_tags, task_paths, run_meta)`` — empty maps and ``{}`` when there - is no readable run.json (a fresh dir, or one assembled purely from task.json). - """ - try: - prior = json.loads((run_dir / "run.json").read_text(encoding="utf-8")) - except (OSError, ValueError): - return {}, {}, {} - if not isinstance(prior, dict): - return {}, {}, {} - task_tags: dict[str, list[str]] = {} - task_paths: dict[str, str] = {} - for row in prior.get("task_results", []): - if not isinstance(row, dict): - continue - task_id = row.get("task_id") - if not task_id: - continue - if isinstance(row.get("tags"), list): - task_tags[task_id] = row["tags"] - if isinstance(row.get("task_path"), str): - task_paths[task_id] = row["task_path"] - return task_tags, task_paths, prior - - -def _recover_skipped_tasks(prior: dict[str, Any]) -> list[SkippedTask]: - """Reconstruct the skipped-task carry-over from a prior run.json, per entry. - - The prior summary is untrusted (possibly stale / hand-edited / older-schema), so a - malformed entry must drop rather than abort the rebuild — mirroring the rest of - ``_read_prior_metadata``'s degrade-to-empty stance. - """ - recovered: list[SkippedTask] = [] - for entry in prior.get("skipped_tasks", []): - if not isinstance(entry, dict): - continue - try: - recovered.append(SkippedTask.model_validate(entry)) - except ValidationError: - console.print(f"[yellow]Dropping malformed skipped_tasks entry from prior run.json: {entry}[/yellow]") - return recovered - - -def _resolve_window(results: list[TaskResult], prior: dict[str, Any]) -> tuple[datetime, datetime]: - """Determine the run's (start, end) for total_duration_seconds. - - Prefer the prior run.json's timestamps (the real wall-clock); otherwise derive a - best-effort window from the recovered results' start times + durations. ``results`` - is always non-empty here (the command errors out earlier on an empty recovery) and - ``EvaluationResult.started_at`` is a required field, so a start time always exists. - """ - start_raw, end_raw = prior.get("start_time"), prior.get("end_time") - if isinstance(start_raw, str) and isinstance(end_raw, str): - try: - return datetime.fromisoformat(start_raw), datetime.fromisoformat(end_raw) - except ValueError: - pass - start = min(r.result.started_at for r in results) - end = max(r.result.started_at + timedelta(seconds=r.duration) for r in results) - return start, max(end, start) diff --git a/src/coder_eval/cli/evaluate_command.py b/src/coder_eval/cli/evaluate_command.py index d024f905..163eeb31 100644 --- a/src/coder_eval/cli/evaluate_command.py +++ b/src/coder_eval/cli/evaluate_command.py @@ -23,6 +23,7 @@ TemplateDirSource, parse_agent_config, ) +from ..orchestration import run_summary_rebuild from ..orchestration.regrade import ( RegradeError, back_up_pre_grade_record, @@ -583,7 +584,7 @@ def _report_and_exit( f"[dim]Re-graded {prior.final_status.value} → {result.final_status.value} " + f"over {len(result.iterations)} recorded turn(s).[/dim]" ) - _write_back(target.target, result) + _write_back(target.target, result, prepared_run_dir) if result.final_status.is_execution_fact: # The criteria tally is real -- it is why the table above still renders -- @@ -602,12 +603,12 @@ def _report_and_exit( raise typer.Exit(1) -def _write_back(run_dir: Path, result: EvaluationResult) -> None: +def _write_back(run_dir: Path, result: EvaluationResult, grading_run_dir: Path) -> None: """Replace the graded run's ``task.json`` with the verdict, keeping a copy of the original. - Updating in place is what makes the rest of the toolchain free: plain - ``coder-eval aggregate `` then rebuilds ``run.json`` from these rows - with no new code, and every report and evalboard view reads the graded row. + Updating in place is what makes the rest of the toolchain free: the owning run's + ``run.json`` is then rebuilt from these rows (``_refresh_run_summary``), and every + report and evalboard view reads the graded row. The pre-grade original is kept alongside as ``task.execute.json`` so the ungraded record is auditable — the write is not a silent overwrite of the @@ -629,7 +630,41 @@ def _write_back(run_dir: Path, result: EvaluationResult) -> None: # already printed, and the fresh run dir holds its own task.json. console.print(f"[yellow]⚠[/] Could not update {target}: {e}") return - console.print( - f"[dim]Updated {target} (original kept as {backup.name}); " - + "run `coder-eval aggregate` to refresh run.json.[/dim]" - ) + console.print(f"[dim]Updated {escape(str(target))} (original kept as {backup.name}).[/dim]") + _refresh_run_summary(run_dir, grading_run_dir) + + +def _refresh_run_summary(row_dir: Path, grading_run_dir: Path) -> None: + """Rebuild the run-level ``run.json`` of the run that owns ``row_dir``, best-effort. + + Never raises and never changes the exit code: the verdict is already computed and + printed. A row with no ``run.json`` above it gets none. Skipped when this grade wrote + its own ``task.json`` inside the owning run (``grading_run_dir`` under it and not the + row itself), because that record would be counted as a second row. + + Rationale: .claude/notes/isolation.md § Detached grading from the CLI + """ + try: + root = run_summary_rebuild.find_run_root(row_dir) + if root is None: + console.print( + f"[dim]{escape(str(row_dir))} is not inside a run directory; " + + "no run-level run.json was refreshed.[/dim]" + ) + return + grading = grading_run_dir.resolve() + if grading != row_dir.resolve() and grading.is_relative_to(root): + console.print( + f"[yellow]⚠[/] The grading run dir {escape(str(grading_run_dir))} is inside the run at " + + f"{escape(str(root))}; its task.json would count as a second row, so run.json was not " + + "refreshed. Grade with a --run-dir outside the run." + ) + return + summary = run_summary_rebuild.rebuild_run_summary(root) + except Exception as e: + console.print(f"[yellow]⚠[/] Could not refresh the run-level run.json: {escape(str(e))}") + return + if summary is None: + console.print(f"[yellow]⚠[/] No finalized task.json under {escape(str(root))}; its run.json was not refreshed.") + return + console.print(f"[dim]Refreshed {escape(str(root / 'run.json'))}[/dim]") diff --git a/src/coder_eval/cli/report_command.py b/src/coder_eval/cli/report_command.py index c864db57..b09d624d 100644 --- a/src/coder_eval/cli/report_command.py +++ b/src/coder_eval/cli/report_command.py @@ -4,8 +4,10 @@ import typer from rich.markdown import Markdown +from rich.markup import escape from ..models import EvaluationResult +from ..orchestration.run_summary_rebuild import find_run_root, rebuild_run_summary from ..path_utils import TASK_JSON_FILENAME from ..reports import ReportGenerator, write_task_html from .console import console @@ -23,8 +25,8 @@ def report_command( "-o", help="Output file (default: display markdown to stdout).", ), - report_format: str = typer.Option( - "md", + report_format: str | None = typer.Option( + None, "--format", "-f", help=( @@ -32,6 +34,14 @@ def report_command( "'junit' (JUnit XML from run.json)." ), ), + rebuild: bool = typer.Option( + False, + "--rebuild", + help=( + "Rebuild the run-level run.json + run.md in place from the finalized task.json files under " + "RUN_DIR, which must be a run root. Cannot be combined with --format or --output." + ), + ), ) -> None: """Display or export a run report. @@ -50,8 +60,19 @@ def report_command( # Write a JUnit XML report (defaults to /junit.xml) coder-eval report runs/latest --format junit + + # Rebuild a run's run.json + run.md in place from its task.json files + coder-eval report runs/2026-06-22_14-32-27 --rebuild """ - fmt = report_format.lower() + if rebuild: + if report_format is not None or output_file is not None: + raise typer.BadParameter( + "--rebuild writes run.json and run.md in place; it cannot be combined with --format or --output." + ) + _rebuild_run_summary(run_dir) + return + + fmt = (report_format or "md").lower() if fmt not in ("md", "html", "junit"): console.print(f"[red]Error: unknown --format '{report_format}' (expected 'md', 'html', or 'junit')[/red]") raise typer.Exit(1) @@ -89,6 +110,35 @@ def report_command( console.print(Markdown(report_md)) +def _rebuild_run_summary(run_dir: Path) -> None: + """Rebuild ``run_dir``'s run.json + run.md and print the counts; exit 1 when there is nothing to aggregate.""" + # A run.json written below the real root makes every later rebuild of that root drop these rows. + if (run_dir / TASK_JSON_FILENAME).is_file(): + raise typer.BadParameter(f"{run_dir} is a task directory, not a run root; pass the run directory above it.") + enclosing = find_run_root(run_dir.resolve().parent) + if enclosing is not None: + raise typer.BadParameter(f"{run_dir} is inside the run at {enclosing}; rebuild that directory instead.") + + try: + summary = rebuild_run_summary(run_dir) + except (OSError, ValueError) as e: + console.print(f"[red]Error: {escape(str(e))}[/red]") + raise typer.Exit(1) from e + if summary is None: + console.print(f"[red]Error: no finalized task.json files found under {escape(str(run_dir))}[/red]") + console.print("\n[dim]Hint: --rebuild aggregates a finished run — use 'coder-eval run' to create one.[/dim]") + raise typer.Exit(1) + counts = f"{summary.tasks_succeeded} ok / {summary.tasks_failed} fail / {summary.tasks_error} err" + if summary.tasks_not_graded: + counts += f" / {summary.tasks_not_graded} not graded" + run_json = escape(str(run_dir / "run.json")) + console.print(f"[green][OK][/green] Aggregated {summary.tasks_run} task(s) ({counts}) → {run_json}") + console.print( + "[dim]Note: run-level summary only — per-suite (suite.json/suite.md) and " + + "experiment (experiment.json/experiment.md) rollups are not rebuilt.[/dim]" + ) + + def _regenerate_html_reports(run_dir: Path, output_file: Path | None) -> None: """Regenerate task-level HTML reports from every task.json under run_dir. diff --git a/src/coder_eval/cli/run_task_internal_command.py b/src/coder_eval/cli/run_task_internal_command.py index 85e938d8..7f0fe1b3 100644 --- a/src/coder_eval/cli/run_task_internal_command.py +++ b/src/coder_eval/cli/run_task_internal_command.py @@ -16,11 +16,12 @@ import asyncio import contextlib -import json import logging from pathlib import Path +from typing import Any import typer +from pydantic import ValidationError from coder_eval.config import settings from coder_eval.isolation.docker_runner import ( @@ -33,10 +34,8 @@ CONTAINER_OUTPUT_DIR, CONTAINER_TASK_DIR, IN_CONTAINER_ENV, - ConfigLineageEntry, + ContainerContext, EvaluationResult, - PreservationMode, - SandboxConfig, TaskDefinition, ) from coder_eval.orchestration.task_loader import load_task @@ -72,8 +71,8 @@ def _arm_host_heartbeat_watchdog(output_dir: Path) -> None: # `os._exit(137)` on the process it runs in, so anywhere else it can only harm # -- it once killed a pytest worker mid-test-file and took its coverage with it. # Gated on CODER_EVAL_IN_CONTAINER, NOT on `driver`, for the same reason - # `Sandbox.enforces_permission_windows` is: this command rewrites - # `driver: docker` -> `tempdir` before building the Orchestrator. + # `Sandbox.enforces_permission_windows` is: the host stages the task this + # command runs with `driver: tempdir`. # Rationale: .claude/notes/isolation.md § The heartbeat watchdog is armed only inside a container if _os.environ.get(IN_CONTAINER_ENV) == "1": @@ -162,76 +161,31 @@ def run_task_internal_command( typer.echo(f"FATAL: missing {context_json}", err=True) raise typer.Exit(2) - context = json.loads(context_json.read_text(encoding="utf-8")) - # CHECKED, not just annotated: `json.loads` returns `Any`, so an annotation - # here reads like a guarantee and enforces nothing. - # Rationale: .claude/notes/isolation.md § The context payload is untrusted input - variant_id = context["variant_id"] - if not isinstance(variant_id, str): - typer.echo(f"FATAL: context.json 'variant_id' must be a string, got {variant_id!r}", err=True) - raise typer.Exit(2) - replicate_index = context.get("replicate_index", 0) - if not isinstance(replicate_index, int) or isinstance(replicate_index, bool): - typer.echo(f"FATAL: context.json 'replicate_index' must be an integer, got {replicate_index!r}", err=True) - raise typer.Exit(2) - # The host resolves the driver-derived default; the container obeys it. A - # missing key falls back to the docker default -- deliberate, not back-compat. - preservation_mode = PreservationMode(context.get("preservation_mode", PreservationMode.DIRECT_WRITE.value)) - # `run` vs `execute`, decided host-side. Defaults to True so a host predating - # `execute` keeps its behaviour. COERCED, like every value crossing this - # boundary: a `"grade": "false"` is a truthy str typed as bool. - grade_raw = context.get("grade", True) - if not isinstance(grade_raw, bool): - 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. Getting this one wrong re-RUNS the agent against - # the workspace it was asked only to grade. - 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 distinct from the path this process resolves - # TASK_DIR against. Absent on an older host -> the container path is recorded. + try: + ctx = ContainerContext.model_validate_json(context_json.read_text(encoding="utf-8")) + except ValidationError as e: + # Exit 2, a setup failure: a host/image skew must not read as a task failure. + # Rationale: .claude/notes/isolation.md § The container contract + typer.echo(f"FATAL: {context_json} is not a valid container contract: {e}", err=True) + raise typer.Exit(2) from e + # What task.json RECORDS, as distinct from the path this process resolves TASK_DIR against. # Rationale: .claude/notes/orchestration.md § Recording the task as authored - 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, resolved host-side. Absent -> the standard - # run_dir/artifacts workspace. - workspace_dir_raw = context.get("workspace_dir") - workspace_dir = Path(workspace_dir_raw) if workspace_dir_raw else None - config_lineage = {k: ConfigLineageEntry.model_validate(v) for k, v in (context.get("config_lineage") or {}).items()} - # The host's RAW source_yaml, so task.json's audit trail matches the in-process - # driver. Falls back to the staged post-override YAML on an older host. - host_source_yaml: str | None = context.get("source_yaml") + recorded_task_file = Path(ctx.host_task_file) if ctx.host_task_file else None + workspace_dir = Path(ctx.workspace_dir) if ctx.workspace_dir else None # `task_file` is then pointed under the task_dir mount, so the `TASK_DIR` the # Orchestrator exposes to `run_command` criteria resolves there, not /work/input. - task, source_yaml = load_task(task_yaml) - if host_source_yaml is not None: - source_yaml = host_source_yaml + task, _ = load_task(task_yaml) # The path below is never re-read; it only seeds Orchestrator's TASK_DIR. runtime_task_file = task_dir / "task.yaml" if task_dir.is_dir() else task_yaml - # Captured BEFORE the rewrite below: this is what `task.json` records. + # The staged task is the host's execution copy (driver: tempdir); task.json records the sandbox as authored. # Rationale: .claude/notes/orchestration.md § Recording the task as authored - authored_task = task - - # Force driver back to tempdir for the actual in-container run. - if task.sandbox.driver == "docker": - # noqa: CE051 — the ONE legitimate rewrite. We are already inside the - # container the docker driver asked for, so the isolation it names is - # present, not bypassed. Re-validated rather than `model_copy(update=...)`, - # which skips both pydantic and pyright. - # Rationale: .claude/notes/orchestration.md § The in-container driver rewrite - rewritten = SandboxConfig.model_validate({**task.sandbox.model_dump(), "driver": "tempdir"}) # noqa: CE051 - task = task.model_copy(update={"sandbox": rewritten}) + authored_task = task.model_copy(update={"sandbox": ctx.authored_sandbox}) output_dir.mkdir(parents=True, exist_ok=True) - if regrade: + if ctx.regrade: _grade_recorded_run( task=task, authored_task=authored_task, @@ -239,9 +193,10 @@ def run_task_internal_command( 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, + source_yaml=ctx.source_yaml, + variant_id=ctx.variant_id, + replicate_index=ctx.replicate_index, + container_contract=ctx.model_dump(mode="json"), ) return @@ -252,16 +207,17 @@ def run_task_internal_command( orchestrator = Orchestrator( task=task, run_dir=output_dir, - preservation_mode=preservation_mode, + preservation_mode=ctx.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, - replicate_index=replicate_index, + variant_id=ctx.variant_id, + source_yaml=ctx.source_yaml, + config_lineage=ctx.config_lineage, + replicate_index=ctx.replicate_index, workspace_dir=workspace_dir, - grade=grade, + grade=ctx.grade, recorded_task=authored_task, + container_contract=ctx.model_dump(mode="json"), ) # Late import keeps the streaming module out of the default --help path. @@ -284,6 +240,7 @@ def _grade_recorded_run( source_yaml: str, variant_id: str, replicate_index: int, + container_contract: dict[str, Any], ) -> None: """Grade an already-executed row INSIDE the container that produced it. @@ -291,7 +248,7 @@ def _grade_recorded_run( `driver: docker` task: the host stages `prior.json` next to `task.yaml` and bind-mounts the executed workspace at ``CONTAINER_GRADE_WORKSPACE``. - ``task`` is the driver-rewritten copy (docker -> tempdir), which is also what + ``task`` is the execution copy the host staged with ``driver: tempdir``, which is also what keeps ``regrade_in_place`` from dispatching a container from within one. ``authored_task`` is what gets RECORDED, and ``recorded_task_file`` is the path half of that same distinction and travels with it. @@ -334,6 +291,7 @@ def _grade_recorded_run( replicate_index=replicate_index, recorded_task=authored_task, recorded_task_file=recorded_task_file, + container_contract=container_contract, ) ) except RegradeError as e: diff --git a/src/coder_eval/config.py b/src/coder_eval/config.py index 65178814..53f33830 100644 --- a/src/coder_eval/config.py +++ b/src/coder_eval/config.py @@ -110,6 +110,7 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: log_level: str = "INFO" # Default log level (DEBUG, INFO, WARNING, ERROR, CRITICAL) log_to_file: bool = False # Whether to enable file logging + allow_image_skew: bool = False # ALLOW_IMAGE_SKEW=1: local image iteration; drops docker reproducibility # On by default via the baked-in connection string, which any set value (env # or .env) overrides. TELEMETRY_ENABLED is the single canonical disable gate. diff --git a/src/coder_eval/isolation/docker_runner.py b/src/coder_eval/isolation/docker_runner.py index fff43caf..7f8b19d7 100644 --- a/src/coder_eval/isolation/docker_runner.py +++ b/src/coder_eval/isolation/docker_runner.py @@ -10,10 +10,10 @@ import asyncio import contextlib -import json import logging import os import re +import reprlib import shutil import subprocess import tempfile @@ -24,6 +24,7 @@ import yaml +from coder_eval.config import settings from coder_eval.logging_config import DEFAULT_LOG_TAIL_MAX_BYTES from coder_eval.models import ( CONTAINER_GRADE_WORKSPACE, @@ -35,11 +36,13 @@ IN_CONTAINER_ENV, RESERVED_CONTAINER_DIRS, AgentKind, + ContainerContext, DockerDriverConfig, EvaluationResult, FinalStatus, PreservationMode, ResourceLimits, + SandboxConfig, ) from coder_eval.orchestration.evaluation import resolve_host_reference_dir from coder_eval.path_utils import ( @@ -187,26 +190,24 @@ def _preflight() -> None: raise DockerRunError("docker daemon is not responding. Start Docker Desktop or check `docker info`.") from exc -def _preflight_image_version(image: str) -> None: - """Assert the image's ``coder_eval`` label matches the host BEFORE running. +def _preflight_image_contract(image: str, dockerfile: Path | None) -> None: + """Refuse, before a billed container starts, an image that cannot honor this host's contract. - The PR's original mismatch warning ran *after* ``task.json`` was parsed - — i.e. after the billed LLM run. The whole point of ``--driver docker`` - is reproducibility; warning post-hoc is the wrong order. Here we inspect - the image label and warn *before* spawning the container, so a stale - ``:latest`` doesn't quietly waste a paid run. + The one reader of the ``org.coder-eval.version`` label. A missing label always refuses. + A label that differs from the host's installed version refuses unless + ``settings.allow_image_skew`` is set, which downgrades it to a warning. A source checkout + has no packaged version, so skew is not computable there: warn and continue. An inspect + or daemon failure is debug-logged and left for ``docker run`` to report. - Missing image / missing label / no-host-version are all soft-fail: log - and continue (image may have been built before the label was added, or - coder-eval may be running from a source checkout without a packaged - version). + Advisory only: ``DockerRunner._assert_contract_echoed`` is the authoritative check. + + Raises: + DockerRunError: The label is absent, or it differs and the escape hatch is off. + + Rationale: .claude/notes/isolation.md § The image version preflight """ from importlib.metadata import PackageNotFoundError, version - try: - host_version = version("coder-eval") - except PackageNotFoundError: - return try: result = subprocess.run( [ @@ -229,20 +230,38 @@ def _preflight_image_version(image: str) -> None: # even when the image is fine, and would then double-fail. logger.debug("Pre-flight image inspect failed for %s: %s", image, exc) return + # `docker inspect` renders a missing label as "" (the Go template zero value); older clients print "". image_version = result.stdout.strip() - if not image_version or image_version == "unknown": - logger.warning( - "Image %s has no org.coder-eval.version label; rebuild with `make docker-image` for pre-flight checks.", - image, + if not image_version or image_version == "": + base = get_default_docker_image_tag() + subject = f"Image built from {dockerfile}" if dockerfile is not None else f"Image {image}" + raise DockerRunError( + f"{subject} is not a coder-eval runtime image (missing the org.coder-eval.version label). " + + "The container must run the in-container orchestrator, so the image must be the framework " + + f"image (built via `make docker-image`) or start `FROM {base}` and only add task-specific " + + "layers on top. See docs/DOCKER_ISOLATION.md." ) - return - if image_version != host_version: + try: + host_version = version("coder-eval") + except PackageNotFoundError: logger.warning( - "Image %s coder_eval %s != host %s. Rebuild with `make docker-image` to keep reproducibility.", + "coder-eval has no installed package version (a source checkout?), so image %s (coder_eval %s) " + + "cannot be checked against the host. Continuing; the contract echo still applies.", image, image_version, - host_version, ) + return + if image_version == host_version: + return + message = ( + f"Image {image} runs coder_eval {image_version} but the host runs {host_version}. Rebuild it with " + + "`make docker-image` (then rebuild any image derived from it), or set ALLOW_IMAGE_SKEW=1 to run a " + + "deliberately different image without the reproducibility guarantee." + ) + if settings.allow_image_skew: + logger.warning("%s Continuing because ALLOW_IMAGE_SKEW is set.", message) + return + raise DockerRunError(message) _CONTAINER_NAME_INVALID = re.compile(r"[^a-zA-Z0-9_.-]") @@ -373,7 +392,7 @@ def _resolve_workspace_dir(cfg_working_dir: str | None, image: str) -> str | Non ``None`` -> ``None`` (feature off). A concrete path -> re-asserted + returned. ``"auto"`` -> the image's WORKDIR via ``docker image inspect`` (falling back to ``/root`` on an empty / ``"/"`` WORKDIR or any inspect failure -- never crash - the run over WORKDIR detection, mirroring ``_preflight_image_version``). + the run over WORKDIR detection, mirroring ``_preflight_image_contract``'s inspect handling). """ if cfg_working_dir is None: return None @@ -508,20 +527,16 @@ def restore_modes(widened: list[tuple[Path, int]]) -> None: logger.warning("Could not restore mode on %s: %s", path, exc) -def _quarantine_record(task_json: Path | None, suffix: str, label: str) -> None: +def _quarantine_record(task_json: Path, 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. + Call it before raising a refusal. The run dir is bind-mounted, so a record left + readable as ``task.json`` is read straight off disk by a later ``--resume`` or + run-level rebuild, publishing exactly the row that was refused. The sidecar name + is not matched by ``rglob("task.json")``. 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 @@ -578,6 +593,7 @@ def __init__( # Resolved in run() (needs the built image for "auto"). Concrete WORKDIR the # agent runs at + copies out from; None = standard artifacts workspace. self._workspace_dir: str | None = None + self._staged_context: ContainerContext | None = None @property def _docker_config(self) -> DockerDriverConfig: @@ -591,9 +607,10 @@ async def run(self) -> EvaluationResult: """Run the task in a container and return the parsed EvaluationResult. The container is responsible for producing ``task.json`` in - ``CONTAINER_OUTPUT_DIR``. On any path where the container exits - without producing it, this raises ``DockerRunError`` and the batch - dispatcher converts that to an ERROR-status EvaluationResult. + ``CONTAINER_OUTPUT_DIR``. This raises ``DockerRunError`` when the image is + refused before the container starts, when the container produces no usable + ``task.json``, or when its result does not echo the staged contract; the + batch dispatcher converts that to an ERROR-status EvaluationResult. """ _preflight() # Side-effecting, so it runs in a worker thread like the other docker calls. @@ -605,10 +622,8 @@ async def run(self) -> EvaluationResult: # Rationale: .claude/notes/isolation.md § A container that produced no task.json await self._record_build_failure(exc) raise - # The version-label preflight only makes sense for the framework image; - # a task-supplied Dockerfile won't carry the org.coder-eval.version label. - if not self._docker_config.dockerfile_path: - await asyncio.to_thread(_preflight_image_version, image) + dockerfile = Path(self._docker_config.dockerfile_path) if self._docker_config.dockerfile_path else None + await asyncio.to_thread(_preflight_image_contract, image, dockerfile) await asyncio.to_thread(self.rt.run_dir.mkdir, parents=True, exist_ok=True) # Docker WORKDIR alignment: config value / "auto" -> inspect / fallback /root. @@ -691,51 +706,50 @@ async def run(self) -> EvaluationResult: 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 - staging ``input_dir`` (``task.yaml`` + ``context.json``). Pure I/O off the event - loop; no control-flow change. + """Serialise the post-override TaskDefinition and the ``ContainerContext`` into the + staging ``input_dir`` (``task.yaml`` + ``context.json``), keeping the contract on + ``self._staged_context``. Pure I/O off the event loop. """ # POST-override, not rt.source_yaml: _apply_cli_overrides has since mutated # rt.task in-memory and the container must see those mutations. - # Rationale: .claude/notes/isolation.md § The context payload is untrusted input + # Rationale: .claude/notes/isolation.md § The container contract task_yaml_in = input_dir / "task.yaml" + # noqa: CE051 — the host resolves the driver for its own container; the authored block rides in the contract. + # Rationale: .claude/notes/orchestration.md § The host-side driver rewrite + execution_sandbox = SandboxConfig.model_validate({**self.rt.task.sandbox.model_dump(), "driver": "tempdir"}) # noqa: CE051 + execution_task = self.rt.task.model_copy(update={"sandbox": execution_sandbox}) def _dump_task_yaml() -> str: - return yaml.safe_dump(self.rt.task.model_dump(mode="json"), sort_keys=False) + return yaml.safe_dump(execution_task.model_dump(mode="json"), sort_keys=False) task_yaml_text = await asyncio.to_thread(_dump_task_yaml) await asyncio.to_thread(task_yaml_in.write_text, task_yaml_text, encoding="utf-8") - # Lineage + variant metadata so the in-container Orchestrator reconstructs - # the same context (variant_id is load-bearing for report grouping). - context_payload = json.dumps( - { - "variant_id": self.rt.variant_id, - "replicate_index": self.rt.replicate_index, - "config_lineage": {k: v.model_dump(mode="json") for k, v in self.rt.config_lineage.items()}, - "preservation_mode": self.preservation_mode.value, - # `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 and adopt - # CONTAINER_GRADE_WORKSPACE instead of running an agent. - "regrade": self.prior_result is not None, - "source_yaml": self.rt.source_yaml, - # The HOST's path, recorded verbatim into task.json's audit trail -- - # distinct from the container path TASK_DIR resolves against. - # Rationale: .claude/notes/orchestration.md § Recording the task as authored - "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, - } + self._staged_context = ContainerContext( + variant_id=self.rt.variant_id, + replicate_index=self.rt.replicate_index, + config_lineage=self.rt.config_lineage, + preservation_mode=self.preservation_mode, + grade=self.grade, + regrade=self.prior_result is not None, + source_yaml=self.rt.source_yaml, + host_task_file=str(self.rt.task_file) if self.rt.task_file else None, + workspace_dir=self._workspace_dir, + authored_sandbox=self.rt.task.sandbox.model_copy(deep=True), + ) + await asyncio.to_thread( + (input_dir / "context.json").write_text, + self._staged_context.model_dump_json(indent=2), + encoding="utf-8", ) - await asyncio.to_thread((input_dir / "context.json").write_text, context_payload, encoding="utf-8") if self.prior_result is not None: - # Carried in whole, so the trajectory an `llm_judge` or - # `command_executed` criterion reads is the ORIGINAL run's. + # Carried whole (the trajectory criteria read is the ORIGINAL run's) minus its echo, + # which an image that predates the echo would otherwise hand back as its own. + staged_prior = self.prior_result.model_copy(deep=True) + if staged_prior.environment_info: + staged_prior.environment_info.pop("container_contract", None) await asyncio.to_thread( (input_dir / PRIOR_RESULT_FILENAME).write_text, - self.prior_result.model_dump_json(indent=2), + staged_prior.model_dump_json(indent=2), encoding="utf-8", ) @@ -831,8 +845,9 @@ async def _kill_container(self, proc: asyncio.subprocess.Process, container_name async def _parse_result_or_raise(self, output_dir: Path, returncode: int, log_path: Path) -> EvaluationResult: """Read back ``task.json`` (the only artifact crossing the boundary) and parse it. - If the container exited without producing it, persist a synthetic ERROR - task.json and raise ``DockerRunError`` so the batch dispatcher records the + If the container produced no ``task.json``, an unparseable one, or a result + that fails the contract echo, persist a synthetic ERROR ``task.json`` and raise + ``DockerRunError`` so the row stays visible and the batch dispatcher records the failure as an ERROR-status result. """ task_json = output_dir / TASK_JSON_FILENAME @@ -855,74 +870,57 @@ async def _parse_result_or_raise(self, output_dir: Path, returncode: int, log_pa # Present but unparseable (schema skew from a stale image, a torn # write): degrade like the missing-file branch. 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) + try: + self._assert_contract_echoed(result, task_json) + except DockerRunError as refusal: + await self._write_synthetic_task_json(task_json, refusal) + raise 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. - - ``regrade`` crosses the boundary only through ``context.json``; an image - that predates container-side grading ignores it and falls through to the - ordinary ``Orchestrator`` branch -- which **starts an agent**. Nothing else - catches it: ``_assert_grade_honored`` early-returns because a grading - container is dispatched with ``grade=True``. - - Keyed on EVIDENCE: a container that honored the request seeds from - ``prior`` and never runs the agent, so a DIFFERENT ``started_at`` is the - tell. + def _assert_contract_echoed(self, result: EvaluationResult, task_json: Path) -> None: + """Refuse a result whose container did not do what the staged contract asked. - Rationale: .claude/notes/isolation.md § The two honored-request guards - """ - 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. + Compares ``environment_info["container_contract"]`` with what ``_stage_inputs`` + sent, in JSON mode on both sides. ``settings.allow_image_skew`` never reaches + this check. ``task_json`` is quarantined to ``task.json.unhonored`` before every raise; + the caller writes a synthetic ERROR record in its place. - ``grade`` crosses the boundary only through ``context.json``. An image that - predates ``execute`` ignores the unknown key and grades anyway, and the - image-version preflight only warns -- so version skew would change what a - command MEANS. - - ``task_json`` is the on-disk record, quarantined before the raise: refusing - in memory while leaving contradictory bytes on disk is not a refusal. + Raises: + DockerRunError: The echo is absent, or any field differs from what was sent. - Rationale: .claude/notes/isolation.md § The two honored-request guards + Rationale: .claude/notes/isolation.md § The contract echo """ - if self.grade: + sent = self._staged_context.model_dump(mode="json") if self._staged_context is not None else None + echo = (result.environment_info or {}).get("container_contract") + if sent is not None and echo == sent: return - # Keyed on EVIDENCE, not on the label: the question is not "what status is this" but "did it grade". - graded_anyway = bool(result.success_criteria_results) or result.weighted_score is not None - if not graded_anyway and ( - result.final_status.is_execution_fact or result.final_status is FinalStatus.NOT_GRADED - ): - return - _quarantine_record(task_json, ".graded", "graded") + _quarantine_record(task_json, ".unhonored", "unhonored") + if sent is None: + raise DockerRunError("A container result was parsed for a dispatch that staged no contract.") + if echo is None: + raise DockerRunError( + "The container returned a result with no container_contract echo: the runtime image predates " + + "the host→container contract and may have ignored what it was asked to do (grade, regrade). " + + "Rebuild with `make docker-image` or pull the image matching this host's coder-eval version." + ) + used = echo if isinstance(echo, dict) else {} + differing = [ + f"{key}: sent {reprlib.repr(sent.get(key))}, container used " + + (reprlib.repr(used[key]) if key in used else "") + for key in sorted(sent.keys() | used.keys()) + if (key in sent, sent.get(key)) != (key in used, used.get(key)) + ] 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 " - + "result(s). The runtime image predates `execute` and ignored the request; " - + "rebuild or pull a matching agent image." + f"The container did not honor the contract it was sent ({'; '.join(differing)}). The runtime image " + + "runs different coder-eval code than this host; rebuild or pull a matching image." ) async def _handle_malformed_task_json(self, task_json: Path, log_path: Path, exc: ValueError) -> DockerRunError: """Degrade a present-but-malformed task.json; return the DockerRunError to raise. Triggered by a present-but-unparseable task.json -- most realistically a - schema skew between a stale ``:latest`` image and the host (the version - checks only warn), or a truncated/torn write. Mirrors the missing-file + schema skew between a stale ``:latest`` image and the host that the + image preflight did not catch, or a truncated/torn write. Mirrors the missing-file branch and the batch.py recovery paths: log naming the path, move the original aside to ``task.json.malformed`` (so its possibly-recoverable content isn't masked AND so the synthetic write lands -- @@ -1004,37 +1002,6 @@ def _write() -> None: except OSError as exc: logger.warning("Failed to write synthetic task.json to %s: %s", target, exc) - def _warn_on_version_mismatch(self, result: EvaluationResult) -> None: - """Warn loudly if the in-container coder_eval version != the host's. - - Reproducibility is one of two reasons users pick driver:docker. - Without this check, an outdated image silently runs stale code - against a refreshed host -- a class of "works on my machine" - regression that's near-impossible to debug. The host already - embeds its own version in environment_info before this point. - """ - from importlib.metadata import PackageNotFoundError, version - - try: - host_version = version("coder-eval") - except PackageNotFoundError: - return - env_info = result.environment_info or {} - if "coder_eval" not in env_info: - # Surface the silent-disable. Future refactor removing this key - # would otherwise stop the version check without anyone noticing. - logger.warning( - "Cannot verify container coder_eval version: result.environment_info missing 'coder_eval' key." - ) - return - container_version = env_info["coder_eval"] - if container_version and container_version != host_version: - logger.warning( - "coder_eval version mismatch -- host %s, container %s. Rebuild image with `make docker-image`.", - host_version, - container_version, - ) - @staticmethod def _sensitive_source_paths() -> list[Path]: """Host paths whose auto-mount should emit a loud warning. @@ -1156,9 +1123,8 @@ def _build_image(self) -> str: The image reference to pass to ``docker run``. Raises: - DockerRunError: If ``docker build`` exits non-zero, or the built image - is not a coder-eval runtime image (missing the - ``org.coder-eval.version`` label). + DockerBuildError: If ``docker build`` exits non-zero. Whether the built + image is a coder-eval runtime is checked by ``_preflight_image_contract``. """ cfg = self._docker_config if not cfg.dockerfile_path: @@ -1204,59 +1170,8 @@ def _build_image(self) -> str: raise DockerBuildError( f"Failed to build Docker image from {dockerfile}: {exc.stderr}", build_log=build_log ) from exc - self._assert_runtime_image(image, dockerfile) return image - def _assert_runtime_image(self, image: str, dockerfile: Path) -> None: - """Fail fast unless the built image carries the coder-eval runtime. - - The host pins ``--entrypoint`` at run time, so we no longer inspect the - baked ``ENTRYPOINT``; instead we verify the image is a coder-eval runtime - image by checking for the ``org.coder-eval.version`` label, which - docker/Dockerfile stamps and any ``FROM coder-eval-agent`` task inherits. - This is the only pre-run validation for a ``dockerfile_path`` task - (``run()`` skips :func:`_preflight_image_version` for that case), so - without it a bare ``FROM ubuntu`` image would build, then die at - ``docker run`` with a cryptic ``exec ...coder_eval_entrypoint.sh: no - such file``. A docker/inspect failure is soft (debug-logged, no raise): - the subsequent ``docker run`` surfaces any real problem. - - Raises: - DockerRunError: If the image carries no ``org.coder-eval.version`` - label (i.e. it is not built ``FROM coder-eval-agent``). - """ - try: - result = subprocess.run( - [ - "docker", - "image", - "inspect", - "--format", - '{{ index .Config.Labels "org.coder-eval.version" }}', - image, - ], - check=True, - capture_output=True, - text=True, - encoding="utf-8", - timeout=10, - ) - except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError) as exc: - logger.debug("Could not inspect labels of built image %s: %s", image, exc) - return - # `docker inspect` renders a missing label as the empty string (the Go - # template's zero value); "" can occur on older clients. - label = result.stdout.strip() - if not label or label == "": - base = get_default_docker_image_tag() - raise DockerRunError( - f"Image built from {dockerfile} is not a coder-eval runtime image " - + "(missing the org.coder-eval.version label). The container must run the " - + f"in-container orchestrator, so a task Dockerfile must start `FROM {base}` " - + "(the framework image, built via `make docker-image`) and only add " - + "task-specific layers on top. See docs/DOCKER_ISOLATION.md." - ) - def _resolve_host_reference_dir(self) -> Path | None: """Host path of ``task.reference.directory``, or None when unset/missing. diff --git a/src/coder_eval/models/__init__.py b/src/coder_eval/models/__init__.py index a3aa4cf8..02648225 100644 --- a/src/coder_eval/models/__init__.py +++ b/src/coder_eval/models/__init__.py @@ -27,6 +27,9 @@ FlagMatch, ) +# Host→container contract (staged as context.json under driver: docker) +from coder_eval.models.container_context import ContainerContext + # Container paths (leaf constants; re-exported so consumers obey CE001) from coder_eval.models.container_paths import ( CONTAINER_GRADE_WORKSPACE, @@ -296,6 +299,7 @@ "TemplateSource", # Sandbox "DockerBuildConfig", + "ContainerContext", "CONTAINER_INPUT_DIR", "CONTAINER_OUTPUT_DIR", "CONTAINER_GRADE_WORKSPACE", diff --git a/src/coder_eval/models/container_context.py b/src/coder_eval/models/container_context.py new file mode 100644 index 00000000..7b91d646 --- /dev/null +++ b/src/coder_eval/models/container_context.py @@ -0,0 +1,65 @@ +"""The host→container contract a ``driver: docker`` dispatch stages as ``context.json``.""" + +from __future__ import annotations + +from pydantic import BaseModel, ConfigDict, Field, StrictBool, StrictInt + +from coder_eval.models.enums import PreservationMode +from coder_eval.models.results import ConfigLineageEntry +from coder_eval.models.sandbox import SandboxConfig + + +class ContainerContext(BaseModel): + """What the host asks one container to do. + + Every field is required and unknown keys are refused: a host and an image that + disagree fail at parse time, never by falling back to a default. + + Rationale: .claude/notes/isolation.md § The container contract + """ + + model_config = ConfigDict(extra="forbid") + + variant_id: str = Field(description="Experiment variant id; load-bearing for report grouping.") + replicate_index: StrictInt = Field( + description="Zero-indexed trial number. Strict, because a bool is an int and True would land in 01/." + ) + config_lineage: dict[str, ConfigLineageEntry] = Field( + description="Dotted-path -> the config layer that supplied it. May be empty." + ) + preservation_mode: PreservationMode = Field( + description="Resolved host-side from the driver-derived default; the container obeys it." + ) + grade: StrictBool = Field( + description=( + "False is `coder-eval execute`. A run-level CLI decision, deliberately not a task field, " + "so the container cannot derive it from task.yaml." + ) + ) + regrade: StrictBool = Field( + description=( + "A detached grade: seed from the staged prior.json and adopt CONTAINER_GRADE_WORKSPACE. " + "Getting it wrong re-RUNS the agent against the workspace it was asked only to grade." + ) + ) + source_yaml: str = Field( + description="The host's raw task YAML, so task.json's audit trail matches the in-process driver." + ) + host_task_file: str | None = Field( + description=( + "The HOST's task file path, recorded verbatim into task.json -- distinct from the " + "container path TASK_DIR resolves against. Null when the task has no file." + ) + ) + workspace_dir: str | None = Field( + description=( + "Docker WORKDIR alignment: the concrete path the agent runs at and is captured from. " + "Null is the standard run_dir/artifacts workspace." + ) + ) + authored_sandbox: SandboxConfig = Field( + description=( + "The sandbox block as AUTHORED (driver: docker), which task.json records. The staged " + "task.yaml carries the execution copy the host resolved to driver: tempdir." + ) + ) diff --git a/src/coder_eval/models/container_paths.py b/src/coder_eval/models/container_paths.py index 8bc72a66..ceb07779 100644 --- a/src/coder_eval/models/container_paths.py +++ b/src/coder_eval/models/container_paths.py @@ -70,8 +70,9 @@ def command_uses_token(command: str, token: str) -> bool: # HAZARD: the one reliable "am I inside a task container?" signal. Every gate that -# means "in a container" MUST key on this and never on `sandbox.driver`, which the -# in-container entry point has already rewritten to `tempdir`. +# means "in a container" MUST key on this and never on `sandbox.driver`: the host +# stages a container's task with `driver: tempdir`, so a driver-based test reads a +# value already resolved host-side. # Rationale: .claude/notes/isolation.md § Capability drops and the anti-cheat window IN_CONTAINER_ENV = "CODER_EVAL_IN_CONTAINER" diff --git a/src/coder_eval/orchestration/batch.py b/src/coder_eval/orchestration/batch.py index 3b343e8a..d00a63f2 100644 --- a/src/coder_eval/orchestration/batch.py +++ b/src/coder_eval/orchestration/batch.py @@ -28,7 +28,7 @@ TaskDefinition, TaskResult, ) -from ..path_utils import TASK_JSON_FILENAME, format_task_log_id +from ..path_utils import TASK_JSON_FILENAME, format_task_log_id, write_text_atomic from ..pricing import unpriced_models from ..run_record import eval_result_to_task_dict from ..streaming.callbacks import StreamCallback @@ -748,11 +748,13 @@ def write_run_summary(summary: RunSummary, run_dir: Path) -> None: run_dir.mkdir(parents=True, exist_ok=True) # run.json — run-level summary (distinct from experiment.json from ExperimentReportGenerator) - (run_dir / "run.json").write_text(summary.model_dump_json(indent=2), encoding="utf-8") + # Atomic: an interrupted write would leave a torn run.json, and the next rebuild + # silently loses the tags, paths and window it carries forward from it. + write_text_atomic(run_dir / "run.json", summary.model_dump_json(indent=2)) # run.md — command statistics report_md = ReportGenerator.generate_markdown(summary, run_dir=run_dir) - (run_dir / "run.md").write_text(report_md, encoding="utf-8") + write_text_atomic(run_dir / "run.md", report_md) def filter_tasks_by_tags( diff --git a/src/coder_eval/orchestration/regrade.py b/src/coder_eval/orchestration/regrade.py index ee0a1323..28e601bf 100644 --- a/src/coder_eval/orchestration/regrade.py +++ b/src/coder_eval/orchestration/regrade.py @@ -24,7 +24,7 @@ import tempfile from functools import cache from pathlib import Path -from typing import TypedDict +from typing import Any, TypedDict from coder_eval.models import ( IN_CONTAINER_ENV, @@ -641,11 +641,10 @@ def _should_grade_in_container(task: TaskDefinition, *, allow_host_grading: bool * ``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. + driver, for the same reason the reference-permission window is: the host + stages a container's task with `driver: tempdir`, so a driver-based test + would be reading a value that has already been resolved. 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 @@ -664,14 +663,21 @@ def _fold_back_container_logs(container_run_dir: Path, run_dir: Path) -> None: ``grade.log`` is the grading pass's OWN log, holding the per-criterion detail that is the only durable record of WHY a criterion scored what it did, and a - documented part of the run-directory contract. + documented part of the run-directory contract. A ``task.json.unhonored`` record + the contract echo refused is rescued the same way, beside the row it was grading. Best-effort throughout: a side-car log is not the verdict, and this runs where an exception is already in flight. Rationale: .claude/notes/isolation.md § Grading a docker row inside a container """ - for name, dest_name in ((DOCKER_LOG_FILENAME, GRADE_DOCKER_LOG_FILENAME), (GRADE_LOG_FILENAME, GRADE_LOG_FILENAME)): + unhonored = f"{TASK_JSON_FILENAME}.unhonored" + rescued = ( + (DOCKER_LOG_FILENAME, GRADE_DOCKER_LOG_FILENAME), + (GRADE_LOG_FILENAME, GRADE_LOG_FILENAME), + (unhonored, unhonored), + ) + for name, dest_name in rescued: # Renamed for the PHASE: on the resume path that name is already taken by # the executed container's log. `grade.log` does not collide. source = container_run_dir / name @@ -872,9 +878,10 @@ async def _grade_in_container( # OSError joins it because the staging copies raise it unwrapped. 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)." + + f"kept at {run_dir / GRADE_DOCKER_LOG_FILENAME}. If the image itself was refused (its " + + "version or its contract echo), rebuild or pull a matching image. Otherwise, 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 dies with this `with`, @@ -903,6 +910,7 @@ async def regrade_in_place( allow_host_grading: bool = False, recorded_task: TaskDefinition | None = None, recorded_task_file: Path | None = None, + container_contract: dict[str, Any] | None = None, ) -> EvaluationResult: """Run ``task``'s criteria against an already-executed ``workspace``. @@ -916,7 +924,10 @@ async def regrade_in_place( ``recorded_task`` / ``recorded_task_file`` are two halves of one seam — what the row RECORDS, as distinct from what this process runs. Both matter only in - the container, where the task is rewritten to ``driver: tempdir``. + the container, whose task the host stages with ``driver: tempdir``. + + ``container_contract`` is the echo the in-container caller forwards to the + Orchestrator; the host never passes it. Rationale: .claude/notes/orchestration.md § Recording the task as authored """ @@ -991,6 +1002,7 @@ async def regrade_in_place( prior_result=prior, recorded_task=recorded_task, recorded_task_file=recorded_task_file, + container_contract=container_contract, ) result = await orchestrator.run() stamp_host_grading(result, task) diff --git a/src/coder_eval/orchestration/run_summary_rebuild.py b/src/coder_eval/orchestration/run_summary_rebuild.py new file mode 100644 index 00000000..36467b23 --- /dev/null +++ b/src/coder_eval/orchestration/run_summary_rebuild.py @@ -0,0 +1,136 @@ +"""Rebuild a run's ``run.json`` + ``run.md`` from the finalized ``task.json`` files on disk.""" + +from __future__ import annotations + +import json +import logging +from datetime import datetime, timedelta +from pathlib import Path +from typing import Any + +from pydantic import ValidationError + +from ..models import RunSummary, SkippedTask, TaskResult +from .batch import build_run_summary, recover_task_results, write_run_summary + + +logger = logging.getLogger(__name__) + + +def rebuild_run_summary(run_dir: Path) -> RunSummary | None: + """(Re)build and write ``run_dir``'s run-level ``run.json`` + ``run.md`` in place. Prints nothing. + + Aggregates the rows ``recover_task_results`` assigns to ``run_dir`` with the builder a + live run uses. Static metadata (tags, source paths, skipped tasks, the run window, + ``max_parallel``) is carried from an existing ``run.json``, which is untrusted: a + malformed entry is dropped with a warning. Per-suite and experiment rollups are not + rebuilt, because the grouping they need is not recoverable from ``task.json`` alone. + + Returns: + The written summary, or ``None`` when ``run_dir`` holds no finalized ``task.json``. + + Raises: + ValueError: ``run.json`` or ``run.md`` in ``run_dir`` is a symlink. A run directory is + a shareable artifact, so writing through a link would overwrite an arbitrary file. + """ + for name in ("run.json", "run.md"): + if (run_dir / name).is_symlink(): + raise ValueError(f"{run_dir / name} is a symlink; refusing to write through it.") + results = recover_task_results(run_dir) + if not results: + return None + task_tags, task_paths, prior = _read_prior_metadata(run_dir) + start_time, end_time = _resolve_window(results, prior) + summary = build_run_summary( + run_dir.resolve().name, + results, + start_time, + end_time, + task_tags, + task_paths=task_paths, + max_parallel=int(prior.get("max_parallel", 1) or 1), + skipped_tasks=_recover_skipped_tasks(prior), + ) + write_run_summary(summary, run_dir) + return summary + + +def find_run_root(path: Path) -> Path | None: + """The nearest directory at or above ``path`` holding coder-eval's ``run.json``, or ``None``. + + ``path`` is resolved first, so a relative path walks past the working directory. A + ``run.json`` that is not a JSON object with ``run_id`` and ``task_results`` belongs to + another tool and is walked past, so a row copied into an unrelated tree never + overwrites that file. A symlinked ``run.json`` is returned as found, for + ``rebuild_run_summary`` to refuse. The inverse of ``recover_task_results``' rule that + the nearest ``run.json`` owns a row. + """ + resolved = path.resolve() + for candidate in (resolved, *resolved.parents): + run_json = candidate / "run.json" + if run_json.is_symlink() or _is_run_summary_file(run_json): + return candidate + return None + + +def _is_run_summary_file(run_json: Path) -> bool: + try: + data = json.loads(run_json.read_text(encoding="utf-8")) + except (OSError, ValueError): + return False + return isinstance(data, dict) and "run_id" in data and "task_results" in data + + +def _read_prior_metadata(run_dir: Path) -> tuple[dict[str, list[str]], dict[str, str], dict[str, Any]]: + """Per-task tags and source paths plus the run-level fields of an existing ``run.json``. + + Returns ``({}, {}, {})`` when there is no readable ``run.json``. + """ + try: + prior = json.loads((run_dir / "run.json").read_text(encoding="utf-8")) + except (OSError, ValueError): + return {}, {}, {} + if not isinstance(prior, dict): + return {}, {}, {} + task_tags: dict[str, list[str]] = {} + task_paths: dict[str, str] = {} + for row in prior.get("task_results", []): + if not isinstance(row, dict): + continue + task_id = row.get("task_id") + if not task_id: + continue + if isinstance(row.get("tags"), list): + task_tags[task_id] = row["tags"] + if isinstance(row.get("task_path"), str): + task_paths[task_id] = row["task_path"] + return task_tags, task_paths, prior + + +def _recover_skipped_tasks(prior: dict[str, Any]) -> list[SkippedTask]: + """The prior ``run.json``'s skipped tasks; a malformed entry is dropped, never fatal.""" + recovered: list[SkippedTask] = [] + for entry in prior.get("skipped_tasks", []): + if not isinstance(entry, dict): + continue + try: + recovered.append(SkippedTask.model_validate(entry)) + except ValidationError: + logger.warning("Dropping malformed skipped_tasks entry from prior run.json: %s", entry) + return recovered + + +def _resolve_window(results: list[TaskResult], prior: dict[str, Any]) -> tuple[datetime, datetime]: + """The run's ``(start, end)``: the prior ``run.json``'s timestamps, else derived from ``results``. + + ``results`` must be non-empty. + """ + start_raw, end_raw = prior.get("start_time"), prior.get("end_time") + if isinstance(start_raw, str) and isinstance(end_raw, str): + try: + return datetime.fromisoformat(start_raw), datetime.fromisoformat(end_raw) + except ValueError: + pass + start = min(r.result.started_at for r in results) + end = max(r.result.started_at + timedelta(seconds=r.duration) for r in results) + return start, max(end, start) diff --git a/src/coder_eval/orchestrator.py b/src/coder_eval/orchestrator.py index 819d3c94..b96bba42 100644 --- a/src/coder_eval/orchestrator.py +++ b/src/coder_eval/orchestrator.py @@ -372,6 +372,7 @@ def __init__( prior_result: EvaluationResult | None = None, recorded_task: TaskDefinition | None = None, recorded_task_file: Path | None = None, + container_contract: dict[str, Any] | None = None, ): """Initialize the orchestrator. @@ -395,6 +396,8 @@ def __init__( prior_result: A completed run's result to re-grade. recorded_task / recorded_task_file: What the run RECORDS, which is not always what this process runs. + container_contract: The parsed ``ContainerContext`` (JSON mode), echoed + into ``environment_info["container_contract"]``. In-container only. Rationale: .claude/notes/orchestration.md § Recording the task as authored """ @@ -416,9 +419,8 @@ def __init__( self.replicate_index = replicate_index self.grade = grade # What `task_config.resolved` records, which is NOT always what we RUN: - # the in-container path rewrites `driver: docker` -> `tempdir` before - # building its Orchestrator, and recording that made the run's own record - # deny it ever used docker. + # a container runs the task the host staged with `driver: tempdir`, and + # recording that made the run's own record deny it ever used docker. # Rationale: .claude/notes/orchestration.md § Recording the task as authored self.recorded_task = recorded_task if recorded_task is not None else task # Same seam, same reason, for the PATH: in a container `task_file` is @@ -426,6 +428,7 @@ def __init__( # 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 + self.container_contract = container_contract # Derived paths self.report_path = self.run_dir / TASK_JSON_FILENAME @@ -1051,6 +1054,15 @@ def _finalize_result(self, start_time: float) -> None: # preserved alongside, so a slow judge is still visible. self._finalize_regrade_timing() + # HAZARD: after _seed_from_prior_result, whose merge lets the PRIOR row's environment_info win. + # Written any earlier, the echo is erased on the regrade path -- the one it must cover. + # Rationale: .claude/notes/isolation.md § The contract echo + if self.container_contract is not None: + self.result.environment_info["container_contract"] = self.container_contract + elif self.prior_result is not None: + # A host grade: the prior row's echo describes a container that did not produce this verdict. + self.result.environment_info.pop("container_contract", None) + # Wrapped because _finalize_result runs inside run()'s finally, where an # unguarded raise would skip persistence and lose task.json. The # simulation-path calls run inside run()'s try, whose broad handler already diff --git a/src/coder_eval/reports/helpers.py b/src/coder_eval/reports/helpers.py index 5183a25c..bdb7a98f 100644 --- a/src/coder_eval/reports/helpers.py +++ b/src/coder_eval/reports/helpers.py @@ -78,7 +78,7 @@ class VariantSeries(NamedTuple): # bookkeeping the reader did not ask for — `command_base_path` is a full PATH # string on every row, and the graded_by_* provenance keys only appear on a # re-graded row where they would read as facts about the run itself. -ENV_TABLE_EXCLUDE = frozenset({"installed_tools", "command_base_path", "reference_digest"}) +ENV_TABLE_EXCLUDE = frozenset({"installed_tools", "command_base_path", "reference_digest", "container_contract"}) def is_env_table_key(key: str) -> bool: diff --git a/src/coder_eval/sandbox.py b/src/coder_eval/sandbox.py index ce62c526..e1752733 100644 --- a/src/coder_eval/sandbox.py +++ b/src/coder_eval/sandbox.py @@ -184,9 +184,9 @@ def enforces_permission_windows(self) -> bool: user's own working copy across tasks. NOTE the predicate is the ``CODER_EVAL_IN_CONTAINER`` env var, NOT - ``config.driver``: the in-container entry point rewrites ``driver: docker`` - to ``tempdir`` before constructing the Orchestrator, so keying on the - driver would silently disable the window on the path that needs it. + ``config.driver``: the host stages a container's task with ``driver: tempdir``, + so keying on the driver would silently disable the window on the path that + needs it. Rationale: .claude/notes/isolation.md § Capability drops and the anti-cheat window """ diff --git a/tests/_container_contract.py b/tests/_container_contract.py new file mode 100644 index 00000000..52ef5bf2 --- /dev/null +++ b/tests/_container_contract.py @@ -0,0 +1,25 @@ +"""A complete, valid ``context.json`` payload for tests that drive the container side.""" + +from __future__ import annotations + +from typing import Any + + +def contract_payload(*, omit: tuple[str, ...] = (), **overrides: Any) -> dict[str, Any]: + """Every ``ContainerContext`` field with a valid value, ``overrides`` applied and ``omit`` removed.""" + payload: dict[str, Any] = { + "variant_id": "default", + "replicate_index": 0, + "config_lineage": {}, + "preservation_mode": "DIRECT_WRITE", + "grade": True, + "regrade": False, + "source_yaml": "task_id: t\n", + "host_task_file": None, + "workspace_dir": None, + "authored_sandbox": {"driver": "docker"}, + } + payload.update(overrides) + for key in omit: + del payload[key] + return payload diff --git a/tests/lint/prose_budget.py b/tests/lint/prose_budget.py index 1226699f..780b53da 100644 --- a/tests/lint/prose_budget.py +++ b/tests/lint/prose_budget.py @@ -70,7 +70,6 @@ ("src/coder_eval/cli/plan_command.py", "plan_command"), ("src/coder_eval/cli/evaluate_command.py", "evaluate_command"), ("src/coder_eval/cli/report_command.py", "report_command"), - ("src/coder_eval/cli/aggregate_command.py", "aggregate_command"), ("src/coder_eval/cli/export_command.py", "export_command"), ("src/coder_eval/cli/harbor_command.py", "reward_command"), ("src/coder_eval/cli/run_task_internal_command.py", "run_task_internal_command"), diff --git a/tests/lint/rules/ce051_no_driver_override.py b/tests/lint/rules/ce051_no_driver_override.py index 029e7232..91fba9dc 100644 --- a/tests/lint/rules/ce051_no_driver_override.py +++ b/tests/lint/rules/ce051_no_driver_override.py @@ -10,8 +10,10 @@ A driver downgrade must be an explicit, logged, operator-visible decision. Exempt: ``models/sandbox.py`` (the model's own construction). Elsewhere, -``# noqa: CE051`` must name the reason, for example an opt-in that refuses by -default and stamps the row. +``# noqa: CE051`` must name the reason. The two legitimate sites are the host-side +staging rewrite in ``docker_runner._stage_inputs`` (the authored sandbox crosses +beside it in the contract) and the opt-in host-grading branch in +``regrade.grading_sandbox_config``, which refuses by default and stamps the row. Rationale: .claude/notes/lint-rules.md § CE051 """ 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 e7b6365a..c68c0b1f 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 @@ -6,9 +6,9 @@ ``os._exit`` skips ``atexit``, ``finally`` and every handler, so it is correct only for reaping the container's own disposable main process. -HAZARD: do not gate on ``sandbox.driver``. ``run_task_internal_command`` rewrites the -driver to ``tempdir`` before it builds the in-container Orchestrator, so a driver gate -disables itself on the one path that needs it. +HAZARD: do not gate on ``sandbox.driver``. ``DockerRunner._stage_inputs`` stages the +in-container task with ``driver: tempdir``, so a driver gate disables itself on the one +path that needs it. The check is lexical, not a data-flow proof: it forces the guard to be written at the site. Add ``# noqa: CE052`` with a reason for an intentional exception. diff --git a/tests/test_container_context.py b/tests/test_container_context.py new file mode 100644 index 00000000..78975e88 --- /dev/null +++ b/tests/test_container_context.py @@ -0,0 +1,333 @@ +"""`ContainerContext`: the host→container contract refuses every shape a skewed host/image pair produces.""" + +from __future__ import annotations + +import ast +import inspect +import json +from datetime import datetime +from pathlib import Path +from typing import Any + +import pytest +import yaml +from pydantic import ValidationError +from typer.testing import CliRunner + +from coder_eval import models +from coder_eval.cli import app, run_task_internal_command +from coder_eval.isolation.docker_runner import DockerRunner +from coder_eval.models import ( + AgentKind, + ConfigLineageEntry, + ContainerContext, + DockerDriverConfig, + EvaluationResult, + FileExistsCriterion, + FinalStatus, + PreservationMode, + ResolvedTask, + ResourceLimits, + SandboxConfig, + TaskDefinition, +) +from coder_eval.orchestration.regrade import regrade_in_place +from coder_eval.orchestration.task_loader import load_task +from coder_eval.path_utils import PRIOR_RESULT_FILENAME, TASK_JSON_FILENAME +from coder_eval.reports import is_env_table_key +from tests._container_contract import contract_payload + + +def test_the_fixture_names_every_field() -> None: + """Otherwise the missing-key parametrization below silently skips a new field.""" + assert set(contract_payload()) == set(ContainerContext.model_fields) + + +def test_every_field_is_required() -> None: + """A default on any field lets a mismatched host/image pair fall back silently instead of failing.""" + defaulted = [name for name, field in ContainerContext.model_fields.items() if not field.is_required()] + assert not defaulted, f"ContainerContext fields must not carry defaults: {defaulted}" + + +@pytest.mark.parametrize("field", sorted(ContainerContext.model_fields)) +def test_a_missing_key_is_refused(field: str) -> None: + with pytest.raises(ValidationError, match=rf"(?m)^{field}$"): + ContainerContext.model_validate(contract_payload(omit=(field,))) + + +def test_an_unknown_key_is_refused() -> None: + """A host newer than the image sends a key the image does not know. That must be a + failure naming the key, never a silent ignore.""" + with pytest.raises(ValidationError, match="sent_by_a_newer_host"): + ContainerContext.model_validate(contract_payload(sent_by_a_newer_host=1)) + + +@pytest.mark.parametrize("field", ["grade", "regrade"]) +@pytest.mark.parametrize("value", ["false", "False", "0", 0, 1]) +def test_a_string_grade_is_refused(field: str, value: object) -> None: + """Lax coercion must refuse: `"false"` is a truthy string, and a `regrade` read + that way re-RUNS the agent over the workspace it was asked only to grade.""" + with pytest.raises(ValidationError, match=rf"(?m)^{field}$"): + ContainerContext.model_validate(contract_payload(**{field: value})) + + +@pytest.mark.parametrize("value", [True, False, "00", 1.0]) +def test_a_boolean_replicate_index_is_refused(value: object) -> None: + """A bool is an int, so a lax field turns `True` into 1 and files the row under 01/.""" + with pytest.raises(ValidationError, match="replicate_index"): + ContainerContext.model_validate(contract_payload(replicate_index=value)) + + +def test_an_invalid_lineage_entry_names_config_lineage() -> None: + with pytest.raises(ValidationError, match="config_lineage"): + ContainerContext.model_validate(contract_payload(config_lineage={"agent.model": {"value": "m", "source": "?"}})) + + +def test_round_trip_through_json() -> None: + ctx = ContainerContext( + variant_id="v1", + replicate_index=2, + config_lineage={"agent.model": ConfigLineageEntry(value="claude-haiku-4-5", source="variant")}, + preservation_mode=PreservationMode.DIRECT_WRITE, + grade=False, + regrade=True, + source_yaml="task_id: t\n", + host_task_file="/host/tasks/t.yaml", + workspace_dir="/root", + authored_sandbox=SandboxConfig(driver="docker"), + ) + parsed = ContainerContext.model_validate_json(ctx.model_dump_json()) + assert parsed == ctx + assert parsed.model_dump(mode="json") == ctx.model_dump(mode="json") + + +def test_the_echo_round_trips_a_maximal_authored_sandbox(tmp_path: Path) -> None: + """The host compares the container's echo with its own dump, so any validator under + `authored_sandbox` that is not idempotent across a JSON round trip refuses EVERY + docker run. Exercised with every sandbox field a validator normalizes.""" + template_dir = tmp_path / "template" + template_dir.mkdir() + authored = SandboxConfig.model_validate( + { + "driver": "docker", + "python": {"env_packages": ["requests"]}, + "node": {"env_packages": ["left-pad"]}, + "limits": {"max_memory_mb": 2048, "max_cpus": 2.0, "max_pids": 128}, + "template_sources": [ + {"type": "repo", "url": "https://example.com/repo.git"}, + {"type": "template_dir", "path": str(template_dir)}, + ], + "mock_path_dirs": ["mocks"], + "record_cli": [ + {"tool": "uip", "responses": [{"when": {"verb": "ixp dummy1"}, "stdout": "ok"}]}, + ], + "additional_ignore_patterns": ["!dist", "*.log"], + "docker": { + "image": "img:1", + "network": "none", + "working_dir": "/srv/app", + "env_passthrough_extra": ["FOO"], + "build": {"args": {"A": "1"}, "secrets": ["id=npm,env=NPM_TOKEN"]}, + }, + } + ) + host_dump = ContainerContext.model_validate(contract_payload(authored_sandbox=authored)).model_dump(mode="json") + + container_parse = ContainerContext.model_validate_json(json.dumps(host_dump)) + echo = json.loads(json.dumps(container_parse.model_dump(mode="json"))) + + assert echo == host_dump + + +def test_host_task_file_null_is_accepted() -> None: + """Required key, nullable value: `null` is a real answer (the task has no file); absence is not.""" + assert ContainerContext.model_validate(contract_payload(host_task_file=None)).host_task_file is None + + +def test_an_empty_config_lineage_is_accepted() -> None: + assert ContainerContext.model_validate(contract_payload(config_lineage={})).config_lineage == {} + + +# -------------------------------------------------------------------------- +# The echo, driven end to end through the real container entry point +# -------------------------------------------------------------------------- + +_AGENTLESS_TASK_YAML = ( + "task_id: echo\ndescription: d\nagent:\n type: none\n" + "success_criteria:\n - type: file_exists\n path: out.txt\n description: d\n" +) + + +def _run_container_entry_point(tmp_path: Path, **overrides: object) -> tuple[dict[str, Any], EvaluationResult]: + """Invoke `_run-task-internal` in process; return (the staged payload, the task.json it wrote).""" + input_dir = tmp_path / "input" + input_dir.mkdir(exist_ok=True) + (input_dir / "task.yaml").write_text(_AGENTLESS_TASK_YAML, encoding="utf-8") + payload = contract_payload(source_yaml=_AGENTLESS_TASK_YAML, **overrides) + (input_dir / "context.json").write_text(json.dumps(payload), encoding="utf-8") + output_dir = tmp_path / "out" + + invoked = CliRunner().invoke(app, ["_run-task-internal", "--input", str(input_dir), "--output", str(output_dir)]) + + record = output_dir / TASK_JSON_FILENAME + assert record.is_file(), invoked.output + return payload, EvaluationResult.model_validate_json(record.read_text(encoding="utf-8")) + + +def test_every_contract_field_is_echoed(tmp_path: Path) -> None: + """Derived from `model_fields`, so a field added to the contract is checked with no new test.""" + payload, written = _run_container_entry_point(tmp_path, grade=False) + echo = written.environment_info["container_contract"] + + assert set(echo) == set(ContainerContext.model_fields) + assert echo == ContainerContext.model_validate(payload).model_dump(mode="json") + + +def test_the_echo_survives_a_regrade(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """`_seed_from_prior_result` lets the PRIOR row's environment_info win. The prior here + carries a stale echo from an earlier pass, so an echo written before the seed would be + replaced by it and the host would refuse a correct grade.""" + + workspace = tmp_path / "workspace" + workspace.mkdir() + (workspace / "out.txt").write_text("done", encoding="utf-8") + monkeypatch.setattr(models, "CONTAINER_GRADE_WORKSPACE", str(workspace)) + + input_dir = tmp_path / "input" + input_dir.mkdir() + prior = EvaluationResult( + task_id="echo", + task_description="d", + variant_id="default", + agent_type=AgentKind.CLAUDE_CODE, + started_at=datetime(2026, 1, 1), + final_status=FinalStatus.NOT_GRADED, + iteration_count=0, + environment_info={"container_contract": {"stale": "from an earlier pass"}}, + ) + (input_dir / PRIOR_RESULT_FILENAME).write_text(prior.model_dump_json(), encoding="utf-8") + + payload, written = _run_container_entry_point(tmp_path, regrade=True) + + assert written.environment_info["container_contract"] == ContainerContext.model_validate(payload).model_dump( + mode="json" + ) + + +async def test_a_host_grade_drops_the_prior_echo(tmp_path: Path) -> None: + """A host grade is not a container's verdict. Keeping the prior echo would record + `grade: false` / `regrade: false` on a row that this pass just graded.""" + + task_yaml = tmp_path / "task.yaml" + task_yaml.write_text(_AGENTLESS_TASK_YAML, encoding="utf-8") + task, source_yaml = load_task(task_yaml) + workspace = tmp_path / "workspace" + workspace.mkdir() + (workspace / "out.txt").write_text("done", encoding="utf-8") + prior = EvaluationResult( + task_id="echo", + task_description="d", + variant_id="default", + agent_type=AgentKind.CLAUDE_CODE, + started_at=datetime(2026, 1, 1), + final_status=FinalStatus.NOT_GRADED, + iteration_count=0, + environment_info={ + "container_contract": ContainerContext.model_validate(contract_payload(grade=False)).model_dump(mode="json") + }, + ) + + graded = await regrade_in_place( + task=task, + prior=prior, + workspace=workspace, + run_dir=tmp_path / "grade", + task_file=task_yaml, + source_yaml=source_yaml, + variant_id="default", + ) + + assert graded.final_status is FinalStatus.SUCCESS + assert "container_contract" not in graded.environment_info + + +# -------------------------------------------------------------------------- +# The driver rewrite happens host-side, at staging +# -------------------------------------------------------------------------- + + +def _authored_docker_task() -> TaskDefinition: + return TaskDefinition( + task_id="staged", + description="d", + agent={"type": "none"}, # type: ignore[arg-type] + sandbox=SandboxConfig( + driver="docker", + limits=ResourceLimits(max_memory_mb=2048, max_pids=128), + docker=DockerDriverConfig(image="img:1", network="none", working_dir="/srv/app"), + ), + success_criteria=[FileExistsCriterion(path="x.txt", description="x")], + ) + + +async def _stage(tmp_path: Path) -> tuple[ResolvedTask, Path, ContainerContext]: + + rt = ResolvedTask( + task=_authored_docker_task(), + task_file=tmp_path / "t.yaml", + run_dir=tmp_path / "run", + variant_id="default", + original_task_id="staged", + ) + input_dir = tmp_path / "input" + input_dir.mkdir() + await DockerRunner(rt)._stage_inputs(input_dir) + return rt, input_dir, ContainerContext.model_validate_json((input_dir / "context.json").read_text(encoding="utf-8")) + + +async def test_the_staged_task_yaml_says_tempdir(tmp_path: Path) -> None: + + _, input_dir, _ = await _stage(tmp_path) + + staged, _ = load_task(input_dir / "task.yaml") + assert staged.sandbox.driver == "tempdir" + + +async def test_the_contract_carries_the_authored_sandbox(tmp_path: Path) -> None: + rt, _, ctx = await _stage(tmp_path) + + assert ctx.authored_sandbox.driver == "docker" + assert ctx.authored_sandbox == rt.task.sandbox + + +async def test_the_execution_sandbox_preserves_every_other_field(tmp_path: Path) -> None: + rt, input_dir, _ = await _stage(tmp_path) + + staged = yaml.safe_load((input_dir / "task.yaml").read_text(encoding="utf-8"))["sandbox"] + authored = rt.task.sandbox.model_dump(mode="json") + assert {key for key in authored if staged.get(key) != authored[key]} == {"driver"} + + +def test_the_container_records_the_authored_driver(tmp_path: Path) -> None: + """The staged task says tempdir; the record must still say docker, or a later + `evaluate ` reads the driver back out and skips the host-grading refusal.""" + _, written = _run_container_entry_point(tmp_path) + + assert written.task_config is not None + assert written.task_config.resolved["sandbox"]["driver"] == "docker" + + +def test_run_task_internal_contains_no_driver_rewrite() -> None: + """CE051 covers the pattern tree-wide; this pins the one module that must hold no exemption.""" + source = inspect.getsource(run_task_internal_command) + driver_keys = [ + node.lineno + for node in ast.walk(ast.parse(source)) + if isinstance(node, ast.Dict) and any(isinstance(k, ast.Constant) and k.value == "driver" for k in node.keys) + ] + assert not driver_keys, f"a `driver` key is built at line(s) {driver_keys}" + + +def test_the_echo_is_not_rendered_in_the_environment_table() -> None: + """A nested object in a flat key/value table renders as a Python dict repr.""" + assert not is_env_table_key("container_contract") diff --git a/tests/test_detached_grading_boundaries.py b/tests/test_detached_grading_boundaries.py index 8f060833..1dd5eace 100644 --- a/tests/test_detached_grading_boundaries.py +++ b/tests/test_detached_grading_boundaries.py @@ -2,9 +2,8 @@ Three boundaries, each shipped without a behavioural test: -* the docker ``grade`` boundary — the only thing standing between - ``execute --driver docker`` against a stale image and a run that silently - publishes real verdicts; +* the docker contract echo — the only thing standing between a skewed image and + a result that silently ignored ``grade`` or ``regrade``; * ``grading_sandbox_config`` — which decides whether a container task's criteria may run on the grading host at all; * the crash-recovery arm — the one a REAL grading failure takes, which is not the @@ -27,7 +26,6 @@ from coder_eval.errors.checker_misuse import CheckerMisuseError from coder_eval.models import ( AgentKind, - CriterionResult, EvaluationResult, FileExistsCriterion, FinalStatus, @@ -43,6 +41,7 @@ stamp_host_grading, ) from coder_eval.path_utils import PRE_GRADE_JSON_FILENAME, TASK_JSON_FILENAME +from tests._container_contract import contract_payload runner = CliRunner() @@ -113,97 +112,135 @@ def test_a_normal_row_carries_no_stamp(self) -> None: # -------------------------------------------------------------------------- -# The docker `grade` boundary +# The docker contract echo # -------------------------------------------------------------------------- -def _docker_runner(*, grade: bool, tmp_path: Path): - from coder_eval.isolation.docker_runner import DockerRunner +class TestContractEcho: + """The host refuses a container result that does not echo the contract it staged. - rt = ResolvedTask( - task=_task("docker"), - task_file=tmp_path / "t.yaml", - run_dir=tmp_path / "run", - variant_id="default", - original_task_id="t", - ) - return DockerRunner(rt, grade=grade) + Every refusal must also quarantine `task.json`: a later `--resume` or run-level + rebuild reads it straight off the bind-mounted run dir. + """ + + _RECORD = '{"final_status": "SUCCESS"}' + + @staticmethod + async def _staged(tmp_path: Path, *, grade: bool = True, regrade: bool = False): + from coder_eval.isolation.docker_runner import DockerRunner + + rt = ResolvedTask( + task=_task("docker"), + task_file=tmp_path / "t.yaml", + run_dir=tmp_path / "run", + variant_id="default", + original_task_id="t", + ) + workspace = None + if regrade: + workspace = tmp_path / "ws" + workspace.mkdir() + runner_ = DockerRunner(rt, grade=grade, prior_result=_result() if regrade else None, grade_workspace=workspace) + input_dir = tmp_path / "input" + input_dir.mkdir() + await runner_._stage_inputs(input_dir) + return runner_ + + @staticmethod + def _echoing(runner_, **flips: object) -> EvaluationResult: + """A result carrying the staged contract with ``flips`` applied, round-tripped through JSON as on disk.""" + result = _result(FinalStatus.SUCCESS) + result.environment_info["container_contract"] = runner_._staged_context.model_dump(mode="json") | flips + return EvaluationResult.model_validate_json(result.model_dump_json()) + def _task_json(self, tmp_path: Path) -> Path: + task_json = tmp_path / TASK_JSON_FILENAME + task_json.write_text(self._RECORD, encoding="utf-8") + return task_json -class TestDockerGradeBoundary: - """`grade` crosses the container boundary only through context.json, so an - image that predates `execute` ignores the key and grades anyway.""" + def _assert_quarantined(self, task_json: Path) -> None: + assert not task_json.exists(), "the refused record must not stay readable as task.json" + assert task_json.with_suffix(".json.unhonored").read_text(encoding="utf-8") == self._RECORD - def test_a_graded_verdict_from_an_execute_run_is_refused(self, tmp_path: Path) -> None: + async def test_a_missing_echo_is_refused_and_quarantined(self, tmp_path: Path) -> None: from coder_eval.isolation.docker_runner import DockerRunError - runner_ = _docker_runner(grade=False, tmp_path=tmp_path) - with pytest.raises(DockerRunError, match="predates `execute`"): - runner_._assert_grade_honored(_result(FinalStatus.SUCCESS)) - - def test_an_ungraded_row_is_accepted(self, tmp_path: Path) -> None: - _docker_runner(grade=False, tmp_path=tmp_path)._assert_grade_honored(_result()) - - def test_an_execution_fact_is_exempt(self, tmp_path: Path) -> None: - """TIMEOUT / ERROR describe the agent phase, not grading. `execute` - reports them exactly as `run` does, so they are not evidence the image - graded anything.""" - for status in (FinalStatus.TIMEOUT, FinalStatus.ERROR, FinalStatus.BUILD_FAILED): - _docker_runner(grade=False, tmp_path=tmp_path)._assert_grade_honored(_result(status)) - - def test_a_graded_run_short_circuits(self, tmp_path: Path) -> None: - _docker_runner(grade=True, tmp_path=tmp_path)._assert_grade_honored(_result(FinalStatus.SUCCESS)) - - def test_an_execution_fact_carrying_a_verdict_is_still_refused(self, tmp_path: Path) -> None: - """The guard keys on EVIDENCE, and until now nothing proved it. - - Every fixture above builds a result with neither a criteria vector nor a - score, so `graded_anyway` was `False` in all four tests — replacing that - whole expression with a literal `False` left the suite fully green, i.e. - the defect it exists for could be reintroduced silently. A stale image - returning a fully graded MAX_TURNS_EXHAUSTED row is exactly the case the - exemption must NOT cover: a fresh image reports that status with no - verdict attached. - """ + runner_ = await self._staged(tmp_path) + task_json = self._task_json(tmp_path) + with pytest.raises(DockerRunError, match="no container_contract echo"): + runner_._assert_contract_echoed(_result(FinalStatus.SUCCESS), task_json) + self._assert_quarantined(task_json) + + async def test_an_echo_that_flipped_grade_is_refused(self, tmp_path: Path) -> None: + """`execute` asked for no grade; a stale image graded anyway.""" from coder_eval.isolation.docker_runner import DockerRunError - graded = _result(FinalStatus.MAX_TURNS_EXHAUSTED) - graded.weighted_score = 1.0 - graded.success_criteria_results = [ - CriterionResult(criterion_type="file_exists", description="x", score=1.0, weight=1.0) - ] + runner_ = await self._staged(tmp_path, grade=False) + task_json = self._task_json(tmp_path) + with pytest.raises(DockerRunError, match="grade: sent False, container used True"): + runner_._assert_contract_echoed(self._echoing(runner_, grade=True), task_json) + self._assert_quarantined(task_json) - runner_ = _docker_runner(grade=False, tmp_path=tmp_path) - with pytest.raises(DockerRunError, match="predates `execute`"): - runner_._assert_grade_honored(graded) + async def test_an_echo_that_flipped_regrade_is_refused(self, tmp_path: Path) -> None: + """A grade came back as a fresh agent run over the recorded row.""" + from coder_eval.isolation.docker_runner import DockerRunError - def test_the_refused_record_is_quarantined_off_task_json(self, tmp_path: Path) -> None: - """Refusing in memory is not enough while the graded bytes stay on disk. + runner_ = await self._staged(tmp_path, regrade=True) + task_json = self._task_json(tmp_path) + with pytest.raises(DockerRunError, match="regrade: sent True, container used False"): + runner_._assert_contract_echoed(self._echoing(runner_, regrade=False), task_json) + self._assert_quarantined(task_json) - A later `execute --resume` / `aggregate` reads task.json straight off the - filesystem, so leaving it in place folds in exactly the row this guard - declined to publish. The rename was shipped with 0% coverage — all four - tests left `task_json` at its `None` default, so the block never ran. - """ + async def test_an_echo_missing_one_field_names_it(self, tmp_path: Path) -> None: from coder_eval.isolation.docker_runner import DockerRunError - task_json = tmp_path / TASK_JSON_FILENAME - task_json.write_text('{"final_status": "SUCCESS"}', encoding="utf-8") + runner_ = await self._staged(tmp_path) + result = self._echoing(runner_) + del result.environment_info["container_contract"]["workspace_dir"] + with pytest.raises(DockerRunError, match="workspace_dir: sent None, container used "): + runner_._assert_contract_echoed(result, self._task_json(tmp_path)) - runner_ = _docker_runner(grade=False, tmp_path=tmp_path) + async def test_a_matching_echo_is_accepted(self, tmp_path: Path) -> None: + runner_ = await self._staged(tmp_path, grade=False) + task_json = self._task_json(tmp_path) + runner_._assert_contract_echoed(self._echoing(runner_), task_json) + assert task_json.exists() + + async def test_the_escape_hatch_does_not_excuse_a_bad_echo( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """ALLOW_IMAGE_SKEW tolerates a version difference, never a container that did something else.""" + from coder_eval.config import settings + from coder_eval.isolation.docker_runner import DockerRunError + + monkeypatch.setattr(settings, "allow_image_skew", True) + runner_ = await self._staged(tmp_path, grade=False) with pytest.raises(DockerRunError): - runner_._assert_grade_honored(_result(FinalStatus.SUCCESS), task_json=task_json) + runner_._assert_contract_echoed(self._echoing(runner_, grade=True), self._task_json(tmp_path)) - assert not task_json.exists(), "the refused record must not stay readable as task.json" - sidecar = task_json.with_suffix(task_json.suffix + ".graded") - assert sidecar.read_text(encoding="utf-8") == '{"final_status": "SUCCESS"}' + async def test_the_parsed_result_path_applies_the_check(self, tmp_path: Path) -> None: + """The guard is wired into `_parse_result_or_raise`, not merely defined.""" + from coder_eval.isolation.docker_runner import DockerRunError + + runner_ = await self._staged(tmp_path) + output_dir = tmp_path / "out" + output_dir.mkdir() + task_json = output_dir / TASK_JSON_FILENAME + task_json.write_text(_result(FinalStatus.SUCCESS).model_dump_json(), encoding="utf-8") + with pytest.raises(DockerRunError, match="no container_contract echo"): + await runner_._parse_result_or_raise(output_dir, returncode=0, log_path=output_dir / "docker.log") + assert task_json.with_suffix(".json.unhonored").is_file() + # Quarantined, not vanished: a synthetic ERROR row keeps it in every later rebuild of run.json. + replacement = EvaluationResult.model_validate_json(task_json.read_text(encoding="utf-8")) + assert replacement.final_status is FinalStatus.ERROR + assert "no container_contract echo" in (replacement.error_message or "") 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 + # contract parse refuses before it, so the `grade` / `regrade` tests do not # depend on this; the prior.json ones do. _VALID_TASK_YAML = ( "task_id: t\ndescription: d\nagent:\n type: none\n" @@ -214,10 +251,7 @@ class TestInContainerGradeCoercion: def _run_with_context(tmp_path: Path, grade: object = True, **extra: object): input_dir = tmp_path / "input" 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) + context = contract_payload(grade=grade, **extra) (input_dir / "context.json").write_text(json.dumps(context), encoding="utf-8") (input_dir / "task.yaml").write_text(TestInContainerGradeCoercion._VALID_TASK_YAML, encoding="utf-8") return runner.invoke( @@ -247,7 +281,8 @@ def test_a_non_boolean_grade_is_a_hard_error(self, tmp_path: Path) -> None: as bool, which would silently grade a run that asked not to be.""" result = self._run_with_context(tmp_path, "false") assert result.exit_code == 2 - assert "must be a boolean" in result.output + assert "is not a valid container contract" in result.output + assert "\ngrade\n" 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 @@ -256,7 +291,8 @@ def test_a_non_boolean_regrade_is_a_hard_error(self, tmp_path: Path) -> None: 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 + assert "is not a valid container contract" in result.output + assert "\nregrade\n" 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 @@ -279,11 +315,6 @@ def test_an_unreadable_prior_degrades_to_a_message_not_a_traceback(self, tmp_pat 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`. - # A `assert 'context.get("grade", True)' in source` grep is no substitute: it - # passes while the line it describes is never executed. - class TestInContainerRegradeBranch: """The container half of `evaluate ` / `run --resume`, driven end to end. @@ -298,26 +329,34 @@ class TestInContainerRegradeBranch: `/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" + # What the host stages: the execution copy. The authored `driver: docker` crosses in the contract. + _STAGED_TASK_YAML = ( + "task_id: t\ndescription: d\nagent:\n type: none\nsandbox:\n driver: tempdir\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): + def _invoke( + self, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + *, + mount_workspace: bool = True, + omit: tuple[str, ...] = (), + **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 / "task.yaml").write_text(self._STAGED_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, + defaults = { + "source_yaml": self._STAGED_TASK_YAML, "regrade": True, "host_task_file": str(tmp_path / "host" / "task.yaml"), } - context.update(extra) + context = contract_payload(omit=omit, **(defaults | extra)) (input_dir / "context.json").write_text(json.dumps(context), encoding="utf-8") workspace = tmp_path / "graded-workspace" @@ -350,19 +389,25 @@ def test_it_grades_the_mounted_workspace_with_the_authored_task( 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. + # What runs: the execution copy the host staged. 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( + def test_an_older_host_that_omits_host_task_file_is_refused( 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.""" + """An absent key is host/image skew. Accepting it would record the container + path, `/work/task_dir/task.yaml`, which exists on no host.""" + result, captured = self._invoke(tmp_path, monkeypatch, omit=("host_task_file",)) + + assert result.exit_code == 2 + assert "host_task_file" in result.output + assert not captured, "a refused contract must never reach the grade" + + def test_a_null_host_task_file_is_a_real_answer(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Required key, nullable value: `null` means the task has no file.""" result, captured = self._invoke(tmp_path, monkeypatch, host_task_file=None) assert result.exit_code == 0, result.output @@ -787,7 +832,7 @@ class TestGradePlumbedIntoTheContainerOrchestrator: describes is never executed — deleting `grade=grade` left every test green.""" @staticmethod - def _invoke(tmp_path: Path, context: dict) -> object: + def _invoke(tmp_path: Path, *, omit: tuple[str, ...] = (), **overrides: object): captured: dict[str, object] = {} class _FakeOrchestrator: @@ -805,22 +850,30 @@ async def run(self): ) (input_dir / "task.yaml").write_text(task_yaml, encoding="utf-8") (input_dir / "context.json").write_text( - json.dumps({"variant_id": "default", "source_yaml": task_yaml, **context}), encoding="utf-8" + json.dumps(contract_payload(omit=omit, source_yaml=task_yaml, **overrides)), encoding="utf-8" ) with patch("coder_eval.orchestrator.Orchestrator", _FakeOrchestrator): - runner.invoke( + result = runner.invoke( app, ["_run-task-internal", "--input", str(input_dir), "--output", str(tmp_path / "out")], ) - return captured.get("grade") + return result, captured def test_execute_forwards_grade_false(self, tmp_path: Path) -> None: - assert self._invoke(tmp_path, {"grade": False}) is False + _, captured = self._invoke(tmp_path, grade=False) + assert captured.get("grade") is False - def test_an_absent_key_still_grades(self, tmp_path: Path) -> None: - """A host predating `execute` writes no key; the container must keep its - original behaviour rather than silently withholding verdicts.""" - assert self._invoke(tmp_path, {}) is True + def test_run_forwards_grade_true(self, tmp_path: Path) -> None: + _, captured = self._invoke(tmp_path, grade=True) + assert captured.get("grade") is True + + def test_an_absent_key_is_refused(self, tmp_path: Path) -> None: + """A host that writes no `grade` key is host/image skew. Defaulting it to + True would publish verdicts a skewed `execute` asked to withhold.""" + result, captured = self._invoke(tmp_path, omit=("grade",)) + assert result.exit_code == 2 + assert "\ngrade\n" in result.output + assert not captured, "a refused contract must never build an Orchestrator" class TestContainerContextIsValidated: @@ -828,23 +881,36 @@ class TestContainerContextIsValidated: guarantee and enforces nothing.""" @staticmethod - def _run(tmp_path: Path, context: dict): + def _run(tmp_path: Path, **overrides: object): input_dir = tmp_path / "input" input_dir.mkdir() (input_dir / "task.yaml").write_text("task_id: t\n", encoding="utf-8") - (input_dir / "context.json").write_text(json.dumps(context), encoding="utf-8") + (input_dir / "context.json").write_text(json.dumps(contract_payload(**overrides)), encoding="utf-8") return runner.invoke(app, ["_run-task-internal", "--input", str(input_dir), "--output", str(tmp_path / "out")]) def test_a_non_string_variant_id_is_refused(self, tmp_path: Path) -> None: - result = self._run(tmp_path, {"variant_id": 7, "source_yaml": "task_id: t\n"}) + result = self._run(tmp_path, variant_id=7) assert result.exit_code == 2 - assert "variant_id" in result.output + assert "\nvariant_id\n" in result.output def test_a_string_replicate_index_is_refused(self, tmp_path: Path) -> None: """`"00"` would reach build_task_run_dir typed as int.""" - result = self._run(tmp_path, {"variant_id": "default", "replicate_index": "00", "source_yaml": "task_id: t\n"}) + result = self._run(tmp_path, replicate_index="00") assert result.exit_code == 2 - assert "replicate_index" in result.output + assert "\nreplicate_index\n" in result.output + + def test_an_unknown_key_is_refused_naming_it(self, tmp_path: Path) -> None: + """A host newer than the image: a named refusal, never a silent ignore.""" + result = self._run(tmp_path, sent_by_a_newer_host=True) + assert result.exit_code == 2 + assert "sent_by_a_newer_host" in result.output + + def test_an_invalid_lineage_entry_is_a_clean_refusal(self, tmp_path: Path) -> None: + """A nested field that fails validation exits 2 naming the field, not with a traceback.""" + result = self._run(tmp_path, config_lineage={"agent.model": {"value": "m", "source": "nowhere"}}) + assert result.exit_code == 2 + assert "config_lineage" in result.output + assert "Traceback" not in result.output class TestCriterionPathsCannotEscapeTheSandbox: diff --git a/tests/test_detached_grading_guards.py b/tests/test_detached_grading_guards.py index e07cfad3..0d13d4b8 100644 --- a/tests/test_detached_grading_guards.py +++ b/tests/test_detached_grading_guards.py @@ -329,7 +329,7 @@ async def test_a_container_run_records_the_driver_it_was_authored_with(tmp_path: """`task_config.resolved` must describe the task as AUTHORED, not as rewritten. Pins: an Orchestrator built with `recorded_task` records that task's - `driver: docker`, not the in-container `tempdir` rewrite it runs. A recorded + `driver: docker`, not the `tempdir` copy the host stages for the container. A recorded `tempdir` lets `evaluate ` skip the host-grading refusal and the `graded_on_host` stamp. `recorded_task` is the seam, exercised without docker. diff --git a/tests/test_docker_runner_container_death.py b/tests/test_docker_runner_container_death.py index 0a76ca40..c866150d 100644 --- a/tests/test_docker_runner_container_death.py +++ b/tests/test_docker_runner_container_death.py @@ -22,7 +22,15 @@ import pytest from coder_eval.isolation.docker_runner import DockerRunError, DockerRunner, build_error_result -from coder_eval.models import EvaluationResult, FileExistsCriterion, FinalStatus, SandboxConfig, TaskDefinition +from coder_eval.models import ( + ContainerContext, + EvaluationResult, + FileExistsCriterion, + FinalStatus, + SandboxConfig, + TaskDefinition, +) +from tests._container_contract import contract_payload # DockerRunner targets Linux containers from POSIX hosts; see @@ -105,7 +113,7 @@ class TestMalformedTaskJson: """A present-but-malformed task.json degrades to a synthetic ERROR record. Mirrors the missing-file branch: a stale ``:latest`` image producing a - schema-skewed task.json (the version checks only warn), or a truncated/torn + schema-skewed task.json the image preflight did not catch, or a truncated/torn write, must not surface as an uncaught ``ValidationError``/``JSONDecodeError``. Instead the runner preserves the original aside (``task.json.malformed``), persists a parseable synthetic ERROR task.json, and returns a ``DockerRunError`` @@ -246,9 +254,11 @@ def test_missing_task_json_writes_synthetic_and_raises(self, run_dir): def test_present_task_json_parsed_and_returned(self, run_dir): """A real task.json -> parsed EvaluationResult returned, nothing raised.""" runner = _make_runner(run_dir) + runner._staged_context = ContainerContext.model_validate(contract_payload()) task_json = run_dir / "task.json" # A valid (non-synthetic) result the in-container orchestrator would have written. expected = build_error_result(runner.rt, DockerRunError("in-container failure")) + expected.environment_info["container_contract"] = runner._staged_context.model_dump(mode="json") task_json.write_text(expected.model_dump_json(indent=2), encoding="utf-8") result = asyncio.run(runner._parse_result_or_raise(run_dir, returncode=0, log_path=run_dir / "docker.log")) diff --git a/tests/test_docker_runner_mounts.py b/tests/test_docker_runner_mounts.py index ff166279..609681ea 100644 --- a/tests/test_docker_runner_mounts.py +++ b/tests/test_docker_runner_mounts.py @@ -1011,6 +1011,8 @@ class TestOutputMountWidenedBeforeLaunch: async def test_run_widens_output_dir_before_container_starts(self, tmp_path: Path, monkeypatch): monkeypatch.setenv("CODER_EVAL_NO_CLAUDE_MOUNT", "1") + # Whatever image this machine happens to have locally must not decide this test. + monkeypatch.setattr("coder_eval.isolation.docker_runner._preflight_image_contract", lambda *_args: None) run_dir = tmp_path / "run" run_dir.mkdir(mode=0o755) task = TaskDefinition( diff --git a/tests/test_evaluate_command.py b/tests/test_evaluate_command.py index 06d26f3a..b824fe00 100644 --- a/tests/test_evaluate_command.py +++ b/tests/test_evaluate_command.py @@ -1,13 +1,26 @@ """Tests for evaluate CLI command.""" +import json +import re +import shutil +import sys +from collections import Counter from pathlib import Path from unittest.mock import patch import pytest import typer +from typer.testing import CliRunner + +from coder_eval.cli import app FIXTURES_DIR = Path(__file__).parent / "fixtures" +AGENTLESS_TASK = Path(__file__).resolve().parents[1] / "tasks" / "agentless_smoke_test.yaml" +_needs_agentless = pytest.mark.skipif( + not AGENTLESS_TASK.is_file(), reason="needs a source checkout (tasks/ is not in the wheel)" +) +_ANSI_RE = re.compile(r"\x1b\[[0-9;]*m") def test_evaluate_command_success(tmp_path): @@ -216,3 +229,191 @@ def test_evaluate_command_multiple_criteria(tmp_path): # All 3 criteria should pass assert exc_info.value.exit_code == 0 + + +# -------------------------------------------------------------------------- +# `evaluate ` refreshes the run-level run.json itself +# -------------------------------------------------------------------------- + + +def _shows(output: str, text: str) -> bool: + """Whether console ``output`` shows ``text``; Rich wraps paths at any character, so compare without whitespace.""" + + def squash(s: str) -> str: + return "".join(_ANSI_RE.sub("", s).split()) + + return squash(text) in squash(output) + + +def _row_dirs(run_dir: Path) -> list[Path]: + return sorted(p.parent for p in run_dir.rglob("task.json")) + + +def _read_json(path: Path) -> dict: + return json.loads(path.read_text(encoding="utf-8")) + + +def _plant_ungraded_row(run_dir: Path, source_row: Path, task_id: str) -> Path: + """A second executed row beside ``source_row``, under its own task id.""" + from coder_eval.models import EvaluationResult + from coder_eval.path_utils import build_task_run_dir + + record = EvaluationResult.model_validate_json((source_row / "task.json").read_text(encoding="utf-8")) + record.task_id = task_id + target = build_task_run_dir(run_dir, record.variant_id, task_id, 0) + target.mkdir(parents=True) + (target / "task.json").write_text(record.model_dump_json(indent=2), encoding="utf-8") + return target + + +def _evaluate(target: Path): + return CliRunner().invoke(app, ["evaluate", str(target)]) + + +@pytest.fixture +def executed_run(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + """A real `execute` run dir: one NOT_GRADED row and its run.json. Grading passes stay inside tmp_path.""" + from coder_eval.cli import run_helpers + + monkeypatch.setattr(run_helpers.settings, "runs_dir", tmp_path / "default-runs") + run_dir = tmp_path / "r" + result = CliRunner().invoke(app, ["execute", str(AGENTLESS_TASK), "--run-dir", str(run_dir)]) + assert result.exit_code == 0, result.output + return run_dir + + +@_needs_agentless +def test_evaluate_refreshes_the_run_level_run_json(executed_run: Path) -> None: + """No second command: after grading one row, run.json agrees with every row on disk.""" + from coder_eval.orchestration.batch import recover_task_results + + (graded_row,) = _row_dirs(executed_run) + _plant_ungraded_row(executed_run, graded_row, "a_second_row") + stale = _read_json(executed_run / "run.json") + + result = _evaluate(graded_row) + + assert result.exit_code == 0, result.output + buckets = Counter(r.result.final_status.category for r in recover_task_results(executed_run)) + refreshed = _read_json(executed_run / "run.json") + assert refreshed["tasks_run"] == sum(buckets.values()) == stale["tasks_run"] + 1 + assert refreshed["tasks_succeeded"] == buckets["succeeded"] + assert refreshed["tasks_not_graded"] == buckets["ungraded"] + assert _shows(result.output, f"Refreshed {executed_run / 'run.json'}") + + +@_needs_agentless +def test_evaluate_without_a_run_root_says_so(executed_run: Path, tmp_path: Path) -> None: + """A row copied out of its run has no run.json above it: say so, and create none.""" + (row,) = _row_dirs(executed_run) + copied_root = tmp_path / "copied-out" + copied_row = copied_root / "00" + shutil.copytree(row, copied_row, symlinks=True) + # The recorded workspace still points into the original run, so name the copy explicitly. + (workspace,) = (copied_row / "artifacts").iterdir() + + result = CliRunner().invoke(app, ["evaluate", str(copied_row), "--workspace", str(workspace)]) + + assert result.exit_code == 0, result.output + assert _shows(result.output, "not inside a run directory") + assert not list(copied_root.rglob("run.json")) + + +@_needs_agentless +@pytest.mark.skipif(sys.platform == "win32", reason="creating a symlink needs a privilege on Windows") +def test_a_symlinked_run_json_is_refused(executed_run: Path, tmp_path: Path) -> None: + """A run dir is a shareable artifact; following its run.json link would overwrite any file the grader can write.""" + (row,) = _row_dirs(executed_run) + victim = tmp_path / "victim.json" + victim.write_text("keep me", encoding="utf-8") + (executed_run / "run.json").unlink() + (executed_run / "run.json").symlink_to(victim) + + result = _evaluate(row) + + assert result.exit_code == 0, result.output + assert victim.read_text(encoding="utf-8") == "keep me" + assert (executed_run / "run.json").is_symlink() + assert _shows(result.output, "is a symlink"), result.output + + +@_needs_agentless +def test_a_quarantined_row_is_not_folded_in(executed_run: Path) -> None: + """`task.json.unhonored` is a refused container record; `rglob("task.json")` must not see it.""" + (row,) = _row_dirs(executed_run) + graded_task_id = _read_json(row / "task.json")["task_id"] + refused = _plant_ungraded_row(executed_run, row, "refused_by_the_contract_echo") + (refused / "task.json").rename(refused / "task.json.unhonored") + + result = _evaluate(row) + + assert result.exit_code == 0, result.output + assert _shows(result.output, f"Refreshed {executed_run / 'run.json'}"), result.output + assert {r["task_id"] for r in _read_json(executed_run / "run.json")["task_results"]} == {graded_task_id} + + +@_needs_agentless +def test_a_rebuild_failure_does_not_change_the_exit_code(executed_run: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Best-effort, like the write-back: the verdict is computed and printed before the refresh.""" + from coder_eval.orchestration import run_summary_rebuild + + def _boom(_run_dir: Path) -> None: + raise OSError("disk full") + + monkeypatch.setattr(run_summary_rebuild, "rebuild_run_summary", _boom) + (row,) = _row_dirs(executed_run) + + result = _evaluate(row) + + assert result.exit_code == 0, result.output + assert _shows(result.output, "disk full") + assert _read_json(row / "task.json")["final_status"] == "SUCCESS" + + +@_needs_agentless +def test_a_grading_run_dir_inside_the_run_does_not_refresh(executed_run: Path) -> None: + """`--run-dir` under the owning run writes a second task.json there; folding it in would count the row twice.""" + (row,) = _row_dirs(executed_run) + before = (executed_run / "run.json").read_text(encoding="utf-8") + + result = CliRunner().invoke(app, ["evaluate", str(row), "--run-dir", str(executed_run / "regrade")]) + + assert result.exit_code == 0, result.output + assert _shows(result.output, "would count as a second row"), result.output + assert (executed_run / "run.json").read_text(encoding="utf-8") == before + + +def test_the_refresh_lines_survive_rich_markup_in_a_path( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + """A run directory name is untrusted; `[/y]` in it must print, not raise a MarkupError after the verdict.""" + from coder_eval.cli.evaluate_command import _refresh_run_summary + from coder_eval.orchestration import run_summary_rebuild + + odd_root = tmp_path / "odd[/y]run" + monkeypatch.setattr(run_summary_rebuild, "find_run_root", lambda _path: odd_root) + monkeypatch.setattr(run_summary_rebuild, "rebuild_run_summary", lambda _root: object()) + + _refresh_run_summary(tmp_path / "row", tmp_path / "elsewhere") + + assert _shows(capsys.readouterr().out, f"Refreshed {odd_root / 'run.json'}") + + +@_needs_agentless +def test_work_dir_mode_refreshes_no_run_json(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """`evaluate ` grades a directory, not a run row, so nothing run-level is touched.""" + from coder_eval.cli import run_helpers + + monkeypatch.setattr(run_helpers.settings, "runs_dir", tmp_path / "default-runs") + work = tmp_path / "work" + work.mkdir() + (work / "proof.txt").write_text("coder-eval-ran-without-a-coder", encoding="utf-8") + ancestor_run_json = '{"run_id": "not-this-one", "task_results": []}' + (tmp_path / "run.json").write_text(ancestor_run_json, encoding="utf-8") + + result = CliRunner().invoke(app, ["evaluate", str(AGENTLESS_TASK), str(work)]) + + assert result.exit_code == 0, result.output + assert (tmp_path / "run.json").read_text(encoding="utf-8") == ancestor_run_json + assert not _shows(result.output, "Refreshed") + assert not _shows(result.output, "not inside a run directory") diff --git a/tests/test_execute_command.py b/tests/test_execute_command.py index d90545c9..c8152f85 100644 --- a/tests/test_execute_command.py +++ b/tests/test_execute_command.py @@ -25,7 +25,7 @@ from typer.testing import CliRunner from coder_eval.cli import app -from coder_eval.models import FinalStatus, RunSummary +from coder_eval.models import ContainerContext, FinalStatus, RunSummary runner = CliRunner() @@ -221,10 +221,9 @@ def _option_names(command: str) -> set[str]: return {opt for param in cmd.params for opt in getattr(param, "opts", [])} -# `execute` restates `run`'s Typer signature because Typer builds its parser from -# the signature and there is no way to share one. That duplication is the drift -# risk this test exists to close: a flag added to `run` must be added here too, -# or consciously listed below as a deliberate omission. +# `execute` restates `run`'s Typer signature. This test pins that every `run` flag +# reaches `execute` unless it is listed below as a deliberate omission; the next one +# pins that each shared flag is declared identically on both commands. _DELIBERATELY_ABSENT_FROM_EXECUTE = { "--junit-xml", # a report of verdicts, and there are none "--allow-host-grading", # decides how a GRADE runs; execute never grades @@ -246,13 +245,84 @@ def test_execute_exposes_run_flags_minus_the_refused_one() -> None: assert not _DELIBERATELY_ABSENT_FROM_EXECUTE & execute_opts +# Declared per command because the help text is deliberately command-specific. +_COMMAND_SPECIFIC_HELP = {"resume", "format"} + + +def _shared_parameter_names() -> set[str]: + import inspect + + from coder_eval.cli.execute_command import execute_command + from coder_eval.cli.run_command import run_command + + run_params = set(inspect.signature(run_command).parameters) + execute_params = set(inspect.signature(execute_command).parameters) + return (run_params & execute_params) - _COMMAND_SPECIFIC_HELP + + +def _declaration(param: Any) -> dict[str, Any]: + """Everything a click parameter shows or validates, with type objects reduced to comparable values.""" + fields = ( + "opts", + "secondary_opts", + "help", + "default", + "show_default", + "multiple", + "is_flag", + "flag_value", + "count", + "nargs", + "required", + "metavar", + "hidden", + "envvar", + "show_envvar", + ) + rendered: dict[str, Any] = {field: getattr(param, field, None) for field in fields} + kind = param.type + type_fields = ( + "choices", + "case_sensitive", + "min", + "max", + "min_open", + "max_open", + "clamp", + "exists", + "file_okay", + "dir_okay", + "readable", + "writable", + "resolve_path", + ) + rendered["type"] = (type(kind).__name__, *(getattr(kind, field, None) for field in type_fields)) + return rendered + + +def test_run_and_execute_help_are_identical_for_shared_flags() -> None: + """A shared flag must look and validate the same on both commands; only `--resume` and `--format` differ.""" + import typer.main + + click_app = typer.main.get_command(app) + run_params = {p.name: p for p in click_app.commands["run"].params} # type: ignore[attr-defined] + execute_params = {p.name: p for p in click_app.commands["execute"].params} # type: ignore[attr-defined] + + shared = _shared_parameter_names() + assert shared, "no shared parameters found" + differing = sorted(n for n in shared if _declaration(run_params[n]) != _declaration(execute_params[n])) + assert not differing, f"shared flag(s) {differing} differ between `run` and `execute`" + for name in _COMMAND_SPECIFIC_HELP: + assert run_params[name].help != execute_params[name].help, f"--{name} is no longer command-specific" + + # -------------------------------------------------------------------------- # The docker boundary # -------------------------------------------------------------------------- -async def _staged_context(tmp_path: Path, *, grade: bool) -> dict[str, Any]: - """Stage a docker task's inputs and read back the context.json the container sees.""" +async def _staged_context(tmp_path: Path, *, grade: bool) -> ContainerContext: + """Stage a docker task's inputs and parse back the contract the container sees.""" from coder_eval.isolation.docker_runner import DockerRunner from coder_eval.models import ResolvedTask, TaskDefinition @@ -274,7 +344,7 @@ async def _staged_context(tmp_path: Path, *, grade: bool) -> dict[str, Any]: staged = tmp_path / "input" staged.mkdir() await DockerRunner(rt, grade=grade)._stage_inputs(staged) - return json.loads((staged / "context.json").read_text(encoding="utf-8")) + return ContainerContext.model_validate_json((staged / "context.json").read_text(encoding="utf-8")) @pytest.mark.parametrize("grade", [True, False]) @@ -282,7 +352,7 @@ async def test_docker_forwards_grade_to_the_container(tmp_path: Path, grade: boo """`grade` is a run-level CLI decision, so it is NOT recoverable from the staged task.yaml on the container side — it has to cross the boundary in context.json. Without this, `execute --driver docker` would silently grade after all.""" - assert (await _staged_context(tmp_path, grade=grade))["grade"] is grade + assert (await _staged_context(tmp_path, grade=grade)).grade is grade # The in-container grading default is asserted behaviourally in diff --git a/tests/test_execute_evaluate_loop.py b/tests/test_execute_evaluate_loop.py index 5ed16398..4e8e8060 100644 --- a/tests/test_execute_evaluate_loop.py +++ b/tests/test_execute_evaluate_loop.py @@ -1,4 +1,4 @@ -"""The `execute` -> `evaluate` -> `aggregate` loop. +"""The `execute` -> `evaluate` -> `report --rebuild` loop. `coder-eval execute` withholds the verdict; `coder-eval evaluate ` supplies it later. The pair only earns its keep if it ends up where a single @@ -111,15 +111,15 @@ def test_evaluate_upgrades_the_row_in_place_and_keeps_the_original(tmp_path: Pat assert _row(task_dir, "task.execute.json")["final_status"] == FinalStatus.NOT_GRADED.value -def test_aggregate_rebuilds_a_graded_run_json_with_no_extra_step(tmp_path: Path) -> None: +def test_report_rebuild_sees_a_graded_run_with_no_extra_step(tmp_path: Path) -> None: """Grading in place is what makes the rest of the toolchain free: the - existing `aggregate` command sees the upgraded rows with no new code.""" + run-summary rebuild sees the upgraded rows with no grading-specific code.""" run_dir = tmp_path / "r" _invoke(["execute", str(AGENTLESS_TASK), "--run-dir", str(run_dir)]) assert json.loads((run_dir / "run.json").read_text(encoding="utf-8"))["tasks_not_graded"] == 1 _invoke(["evaluate", str(_task_dir(run_dir))]) - _invoke(["aggregate", str(run_dir)]) + _invoke(["report", str(run_dir), "--rebuild"]) summary = json.loads((run_dir / "run.json").read_text(encoding="utf-8")) assert summary["tasks_not_graded"] == 0 diff --git a/tests/test_image_from_dockerfiles.py b/tests/test_image_from_dockerfiles.py index 0e197fdd..85e46839 100644 --- a/tests/test_image_from_dockerfiles.py +++ b/tests/test_image_from_dockerfiles.py @@ -224,40 +224,16 @@ def test_build_failure_raises_dockerbuilderror_with_log(self, tmp_path: Path, mo assert isinstance(ei.value, DockerRunError) assert "boom: bad layer" in ei.value.build_log - def test_accepts_runtime_image_with_version_label(self, tmp_path: Path, mocker) -> None: - """An image carrying org.coder-eval.version (FROM coder-eval-agent) passes.""" - dockerfile = tmp_path / "Dockerfile" - dockerfile.write_text("FROM coder-eval-agent:latest\n") - mocker.patch.object(dr.subprocess, "run", side_effect=_docker_side_effect(version_label="0.3.0")) - runner = _make_runner(task_id="ok", dockerfile_path=str(dockerfile)) - assert runner._build_image() == "coder-eval-task-ok:built" - - def test_rejects_image_without_version_label(self, tmp_path: Path, mocker) -> None: - """A non-framework image (no org.coder-eval.version label) -> actionable DockerRunError. - - The host pins --entrypoint, so the build is not gated on the baked - ENTRYPOINT; the runtime-image check uses the version label. - """ + def test_the_build_leaves_the_label_check_to_the_preflight(self, tmp_path: Path, mocker) -> None: + """`_build_image` only builds. Whether the result is a coder-eval runtime image is + `_preflight_image_contract`'s job, which `run()` applies to every image; its missing-label, + FROM-hint and inspect-failure cases live in tests/test_image_skew_refusal.py.""" dockerfile = tmp_path / "Dockerfile" dockerfile.write_text("FROM ubuntu:24.04\n") - mocker.patch.object(dr.subprocess, "run", side_effect=_docker_side_effect(version_label="")) - runner = _make_runner(dockerfile_path=str(dockerfile)) - with pytest.raises(DockerRunError, match=r"FROM coder-eval-agent"): - runner._build_image() - - def test_label_inspect_failure_is_soft(self, tmp_path: Path, mocker) -> None: - """If `docker image inspect` itself fails, don't block -- the run surfaces real issues.""" - dockerfile = tmp_path / "Dockerfile" - dockerfile.write_text("FROM coder-eval-agent:latest\n") - - def _run(argv, *a, **k): - if "build" in argv: - return subprocess.CompletedProcess(argv, 0, "", "") - raise subprocess.CalledProcessError(1, argv, stderr="inspect boom") - - mocker.patch.object(dr.subprocess, "run", side_effect=_run) + run = mocker.patch.object(dr.subprocess, "run", side_effect=_docker_side_effect(version_label="")) runner = _make_runner(task_id="ok", dockerfile_path=str(dockerfile)) - assert runner._build_image() == "coder-eval-task-ok:built" # no raise + assert runner._build_image() == "coder-eval-task-ok:built" + assert [call.args[0][1] for call in run.call_args_list] == ["build"] # --------------------------------------------------------------------------- # @@ -445,7 +421,7 @@ def _runtime_dockerfile() -> Path | None: def test_runtime_kit_stamps_version_label() -> None: """The kit Dockerfile must stamp `org.coder-eval.version`. - The host's :meth:`_assert_runtime_image` rejects an image lacking that label; + The host's ``_preflight_image_contract`` rejects an image lacking that label; the converter re-declares it on the *injected* image (guarded converter-side), and the kit carries it too for `docker inspect`/parity. Static guard so dropping the LABEL fails here, not only against a freshly-built image. diff --git a/tests/test_image_skew_refusal.py b/tests/test_image_skew_refusal.py new file mode 100644 index 00000000..fa192d9b --- /dev/null +++ b/tests/test_image_skew_refusal.py @@ -0,0 +1,180 @@ +"""The image version preflight: refuse a skewed image before a billed container starts.""" + +from __future__ import annotations + +import logging +import subprocess +import sys +from importlib.metadata import PackageNotFoundError +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +from coder_eval.config import settings +from coder_eval.isolation import docker_runner as dr +from coder_eval.isolation.docker_runner import DockerRunError, DockerRunner +from coder_eval.models import DockerDriverConfig, FileExistsCriterion, SandboxConfig, TaskDefinition + + +pytestmark = pytest.mark.skipif(sys.platform == "win32", reason="docker driver is POSIX-only") + +HOST_VERSION = "9.9.9" + + +@pytest.fixture(autouse=True) +def _host_version(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr("importlib.metadata.version", lambda _name: HOST_VERSION) + monkeypatch.setattr(settings, "allow_image_skew", False) + + +def _inspect_prints(monkeypatch: pytest.MonkeyPatch, label: str | None) -> None: + """Fake `docker image inspect`: print ``label``, or fail when it is None.""" + + def _run(argv, *args, **kwargs): + if label is None: + raise subprocess.CalledProcessError(1, argv, stderr="No such image") + return subprocess.CompletedProcess(argv, 0, f"{label}\n", "") + + monkeypatch.setattr(dr.subprocess, "run", _run) + + +def _runner(tmp_path: Path, *, dockerfile_path: str | None = None) -> DockerRunner: + task = TaskDefinition( + task_id="skew", + description="d", + initial_prompt="p", + sandbox=SandboxConfig(driver="docker", docker=DockerDriverConfig(dockerfile_path=dockerfile_path)), + success_criteria=[FileExistsCriterion(description="c", path="t.txt")], + ) + rt = MagicMock() + rt.task = task + rt.run_dir = tmp_path / "run" + rt.replicate_index = 0 + rt.variant_id = "default" + rt.config_lineage = {} + rt.source_yaml = "# task" + rt.task_file = None + return DockerRunner(rt) + + +_LAUNCH_SENTINEL = "docker run is not under test" + + +async def _run_until_launch( + runner: DockerRunner, monkeypatch: pytest.MonkeyPatch +) -> tuple[list[tuple[object, ...]], BaseException]: + """Drive ``run()`` with the daemon checks faked; return the launches attempted and the error raised.""" + monkeypatch.setenv("CODER_EVAL_NO_CLAUDE_MOUNT", "1") + monkeypatch.setattr(dr, "_preflight", lambda: None) + launches: list[tuple[object, ...]] = [] + + async def _fake_exec(*argv, **kwargs): + launches.append(argv) + raise FileNotFoundError(_LAUNCH_SENTINEL) + + monkeypatch.setattr("asyncio.create_subprocess_exec", _fake_exec) + with pytest.raises((DockerRunError, FileNotFoundError)) as exc: + await runner.run() + return launches, exc.value + + +async def test_a_version_mismatch_refuses_before_the_container_starts( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + _inspect_prints(monkeypatch, "0.0.1") + + with pytest.raises(DockerRunError) as exc: + dr._preflight_image_contract("img", None) + message = str(exc.value) + assert "0.0.1" in message and HOST_VERSION in message + assert "make docker-image" in message + assert "ALLOW_IMAGE_SKEW=1" in message, "the operator must be able to recover from the error text alone" + + launches, error = await _run_until_launch(_runner(tmp_path), monkeypatch) + assert launches == [], "no container may start for a skewed image" + assert isinstance(error, DockerRunError) and "but the host runs" in str(error) + + +def test_the_escape_hatch_downgrades_the_mismatch_to_a_warning( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + _inspect_prints(monkeypatch, "0.0.1") + monkeypatch.setattr(settings, "allow_image_skew", True) + + with caplog.at_level(logging.WARNING, logger=dr.logger.name): + dr._preflight_image_contract("img", None) + + assert any("0.0.1" in r.getMessage() and "ALLOW_IMAGE_SKEW" in r.getMessage() for r in caplog.records) + + +@pytest.mark.parametrize("label", ["", ""]) +def test_a_missing_label_is_refused_for_a_configured_image(monkeypatch: pytest.MonkeyPatch, label: str) -> None: + _inspect_prints(monkeypatch, label) + with pytest.raises(DockerRunError, match=r"Image img is not a coder-eval runtime image"): + dr._preflight_image_contract("img", None) + + +def test_a_missing_label_is_refused_for_a_dockerfile_image_with_the_from_hint( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The BYOD author's only guidance: name the base to build FROM and the doc.""" + _inspect_prints(monkeypatch, "") + dockerfile = tmp_path / "Dockerfile" + with pytest.raises(DockerRunError) as exc: + dr._preflight_image_contract("coder-eval-task-x:built", dockerfile) + message = str(exc.value) + assert f"Image built from {dockerfile}" in message + assert "FROM coder-eval-agent" in message + assert "docs/DOCKER_ISOLATION.md" in message + + +def test_the_escape_hatch_does_not_excuse_a_missing_label(monkeypatch: pytest.MonkeyPatch) -> None: + _inspect_prints(monkeypatch, "") + monkeypatch.setattr(settings, "allow_image_skew", True) + with pytest.raises(DockerRunError, match=r"org\.coder-eval\.version"): + dr._preflight_image_contract("img", None) + + +async def test_a_dockerfile_image_is_now_version_checked(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + dockerfile = tmp_path / "Dockerfile" + dockerfile.write_text("FROM coder-eval-agent:0.0.1\n", encoding="utf-8") + runner = _runner(tmp_path, dockerfile_path=str(dockerfile)) + monkeypatch.setattr(runner, "_build_image", lambda: "coder-eval-task-skew:built") + _inspect_prints(monkeypatch, "0.0.1") + + launches, error = await _run_until_launch(runner, monkeypatch) + assert launches == [] + assert isinstance(error, DockerRunError) and "but the host runs" in str(error) + + +def test_a_matching_version_passes(monkeypatch: pytest.MonkeyPatch) -> None: + _inspect_prints(monkeypatch, HOST_VERSION) + dr._preflight_image_contract("img", None) + + +def test_an_unknown_host_version_warns_and_continues( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + """A source checkout: skew is not computable, which is not a refusal.""" + + def _no_package(_name: str) -> str: + raise PackageNotFoundError(_name) + + monkeypatch.setattr("importlib.metadata.version", _no_package) + _inspect_prints(monkeypatch, "0.0.1") + + with caplog.at_level(logging.WARNING, logger=dr.logger.name): + dr._preflight_image_contract("img", None) + + assert any("cannot be checked against the host" in r.getMessage() for r in caplog.records) + + +async def test_an_inspect_failure_still_falls_through_to_docker_run( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A missing local image is `docker run`'s error to report, not the preflight's.""" + _inspect_prints(monkeypatch, None) + launches, error = await _run_until_launch(_runner(tmp_path), monkeypatch) + assert len(launches) == 1 + assert str(error) == _LAUNCH_SENTINEL diff --git a/tests/test_preservation_mode.py b/tests/test_preservation_mode.py index b7e1e480..cc5f69b4 100644 --- a/tests/test_preservation_mode.py +++ b/tests/test_preservation_mode.py @@ -4,10 +4,12 @@ import sys import pytest +from pydantic import ValidationError -from coder_eval.models import PreservationMode, SandboxConfig +from coder_eval.models import ContainerContext, PreservationMode, SandboxConfig from coder_eval.orchestration.config import resolve_preservation_mode from coder_eval.sandbox import Sandbox +from tests._container_contract import contract_payload class TestResolvePreservationMode: @@ -88,24 +90,17 @@ def test_setup_failure_clears_self_created_tempdir(monkeypatch): assert sandbox.sandbox_dir is None -def test_docker_runner_forwards_mode_and_container_reads_it_back(): - """The host serializes preservation_mode.value; the container re-parses it (round-trip).""" - # Host side: DockerRunner stamps the resolved mode's .value into context.json. - value = PreservationMode.DIRECT_WRITE.value - assert value == "DIRECT_WRITE" - # Container side: run_task_internal parses it back, and falls back to DIRECT_WRITE - # (the docker default) when the key is absent (no host plumbed it). - assert ( - PreservationMode({"preservation_mode": value}.get("preservation_mode", value)) is PreservationMode.DIRECT_WRITE - ) - assert ( - PreservationMode({}.get("preservation_mode", PreservationMode.DIRECT_WRITE.value)) - is PreservationMode.DIRECT_WRITE - ) - assert ( - PreservationMode({"preservation_mode": "MOVE_ON_WRITE"}.get("preservation_mode", value)) - is PreservationMode.MOVE_ON_WRITE - ) +@pytest.mark.parametrize("mode", list(PreservationMode)) +def test_docker_runner_forwards_mode_and_container_reads_it_back(mode): + """The host resolves the mode; the container parses back exactly that mode from the contract.""" + staged = ContainerContext.model_validate(contract_payload(preservation_mode=mode)).model_dump_json() + assert ContainerContext.model_validate_json(staged).preservation_mode is mode + + +def test_the_container_has_no_preservation_mode_fallback(): + """An absent key is a host/image skew, not a request for the docker default.""" + with pytest.raises(ValidationError, match="preservation_mode"): + ContainerContext.model_validate(contract_payload(omit=("preservation_mode",))) def test_clear_rerun_artifacts_removes_only_existing(tmp_path): diff --git a/tests/test_prose_budget_commands.py b/tests/test_prose_budget_commands.py new file mode 100644 index 00000000..4e72cbec --- /dev/null +++ b/tests/test_prose_budget_commands.py @@ -0,0 +1,35 @@ +"""`prose_budget`'s Typer-command exemption must name exactly the commands the CLI registers.""" + +from __future__ import annotations + +import inspect +from collections.abc import Iterator +from pathlib import Path +from typing import Any + +import click +import typer.main + +import coder_eval +from coder_eval.cli import app +from tests.lint.prose_budget import _TYPER_COMMANDS + + +def _callbacks(command: click.Command) -> Iterator[Any]: + if isinstance(command, click.Group): + for sub in command.commands.values(): + yield from _callbacks(sub) + elif command.callback is not None: + yield inspect.unwrap(command.callback) + + +def test_every_exemption_names_a_registered_command_and_back() -> None: + """A removed command must not leave its exemption behind, and a new command without + one has its docstring budgeted as prose. Both directions fail here.""" + repo = Path(coder_eval.__file__).resolve().parents[2] + registered = { + (Path(inspect.getfile(fn)).resolve().relative_to(repo).as_posix(), fn.__name__) + for fn in _callbacks(typer.main.get_command(app)) + } + + assert registered == set(_TYPER_COMMANDS) diff --git a/tests/test_reference_permissions.py b/tests/test_reference_permissions.py index d49dc4ec..ec1aa7ec 100644 --- a/tests/test_reference_permissions.py +++ b/tests/test_reference_permissions.py @@ -494,9 +494,9 @@ def test_host_run_does_not_enforce(self, tmp_path, monkeypatch): def test_in_container_enforces_even_though_driver_reads_tempdir(self, tmp_path, monkeypatch): """REGRESSION GUARD for a silent-disable trap. - `run_task_internal_command` rewrites `driver: docker` -> `tempdir` before - constructing the Orchestrator, because nested docker is impossible in the - image. So inside the container the driver reads "tempdir". Gating on the + The host stages a container's task with `driver: tempdir`, because nested + docker is impossible in the image. So inside the container the driver + reads "tempdir". Gating on the driver would therefore disable the anti-cheat window on exactly the path that needs it — the gate must key on CODER_EVAL_IN_CONTAINER instead. """ @@ -678,8 +678,8 @@ def _container_sandbox(task_dir: Path, sandbox_dir: Path, monkeypatch) -> "objec """A real Sandbox that reports itself as running inside a docker container. `enforces_permission_windows` keys on CODER_EVAL_IN_CONTAINER rather than - `config.driver`, precisely because the in-container entry point rewrites - the driver to "tempdir" — so this fixture mirrors production by leaving the + `config.driver`, precisely because the host stages a container's task with + the driver at "tempdir" — so this fixture mirrors production by leaving the driver at "tempdir" and setting only the env var. """ from coder_eval.models import SandboxConfig diff --git a/tests/test_regrade.py b/tests/test_regrade.py index d92a96f2..576b8399 100644 --- a/tests/test_regrade.py +++ b/tests/test_regrade.py @@ -12,7 +12,6 @@ import json import logging -from datetime import timedelta from pathlib import Path from typing import ClassVar @@ -416,9 +415,9 @@ def test_a_tempdir_row_never_starts_a_container(self) -> None: 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 + The host stages a container's task with `driver: tempdir`, so a + driver-based test would read a value that has already been resolved + — the same trap the reference-permission window documents. Without this gate a grading container dispatches a grading container. """ @@ -640,6 +639,43 @@ async def test_a_grading_run_stages_the_prior_row_and_flags_the_regrade(self, tm assert recovered.task_id == prior.task_id assert recovered.final_status is FinalStatus.NOT_GRADED + async def test_the_staged_prior_carries_no_echo_of_its_own(self, tmp_path: Path) -> None: + """An image that honors `regrade` but predates the echo keeps the prior row's + environment_info (the seed merge lets the prior win). If `prior.json` still held a + matching echo from an earlier identical dispatch, that stale copy would pass the + host's check for a container that never echoed at all.""" + from coder_eval.path_utils import PRIOR_RESULT_FILENAME + + ws = tmp_path / "ws" + ws.mkdir() + prior = _result(weighted_score=None) + prior.environment_info["container_contract"] = {"grade": True, "regrade": True} + prior.environment_info["coder_eval"] = "9.9.9" + runner = self._runner(tmp_path, prior_result=prior, grade_workspace=ws) + + staged = tmp_path / "input" + staged.mkdir() + await runner._stage_inputs(staged) + + recovered = EvaluationResult.model_validate_json((staged / PRIOR_RESULT_FILENAME).read_text(encoding="utf-8")) + assert "container_contract" not in recovered.environment_info + assert recovered.environment_info["coder_eval"] == "9.9.9", "only the echo is stripped" + assert "container_contract" in prior.environment_info, "the in-memory prior row must not be mutated" + + def test_a_refused_grading_record_is_folded_back_beside_the_row(self, tmp_path: Path) -> None: + """The grading container runs in a scratch dir that is deleted afterwards, so a + record the contract echo refused must be rescued, or the evidence is gone.""" + from coder_eval.orchestration.regrade import _fold_back_container_logs + + scratch = tmp_path / "scratch" + scratch.mkdir() + (scratch / "task.json.unhonored").write_text('{"refused": true}', encoding="utf-8") + row = tmp_path / "row" + + _fold_back_container_logs(scratch, row) + + assert (row / "task.json.unhonored").read_text(encoding="utf-8") == '{"refused": true}' + 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.""" @@ -698,64 +734,8 @@ async def test_a_grading_run_forwards_the_hosts_own_task_file_for_the_record(sel 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)) +# A grade that came back as a fresh agent run is refused by the contract echo: +# tests/test_detached_grading_boundaries.py::TestContractEcho. class TestContainerDispatchIsInsideTheTrustGate: diff --git a/tests/test_aggregate.py b/tests/test_run_summary_rebuild.py similarity index 56% rename from tests/test_aggregate.py rename to tests/test_run_summary_rebuild.py index 4ce332a8..6fef0934 100644 --- a/tests/test_aggregate.py +++ b/tests/test_run_summary_rebuild.py @@ -1,7 +1,9 @@ -"""Tests for the run-summary seam: build_run_summary / recover_task_results / `aggregate`.""" +"""The run-summary seam: build_run_summary, recover_task_results, rebuild_run_summary and `report --rebuild`.""" import json import logging +import re +import sys from datetime import datetime from pathlib import Path @@ -9,11 +11,25 @@ from typer.testing import CliRunner from coder_eval.cli import app -from coder_eval.models import AgentKind, EvaluationResult, FinalStatus, TaskResult +from coder_eval.models import AgentKind, EvaluationResult, FinalStatus, RunSummary, TaskResult from coder_eval.orchestration.batch import build_run_summary, recover_task_results, write_run_summary +from coder_eval.orchestration.run_summary_rebuild import find_run_root, rebuild_run_summary from coder_eval.path_utils import build_task_run_dir +_ANSI_RE = re.compile(r"\x1b\[[0-9;]*m") + + +def _shows(output: str, text: str) -> bool: + """Whether the console ``output`` shows ``text``. Rich styles output and hard-wraps long + tokens such as paths at any character, so both sides are compared with all whitespace removed.""" + + def squash(s: str) -> str: + return "".join(_ANSI_RE.sub("", s).split()) + + return squash(text) in squash(output) + + def _eval(task_id: str, *, variant_id: str = "default", status: FinalStatus = FinalStatus.SUCCESS) -> EvaluationResult: return EvaluationResult( task_id=task_id, @@ -44,6 +60,17 @@ def _write_task_json(run_dir: Path, result: EvaluationResult, *, replicate_index return path +def _write_prior(run_dir: Path, **fields: object) -> None: + """Write a minimal prior run.json carrying the given run-level fields.""" + run_dir.mkdir(parents=True, exist_ok=True) + prior = {"run_id": run_dir.name, "task_results": [{"task_id": "a"}], **fields} + (run_dir / "run.json").write_text(json.dumps(prior), encoding="utf-8") + + +def _run_json(run_dir: Path) -> dict: + return json.loads((run_dir / "run.json").read_text(encoding="utf-8")) + + # --- build_run_summary (pure aggregation) ------------------------------------- @@ -213,24 +240,62 @@ def test_write_run_summary_emits_run_json_and_md(tmp_path: Path) -> None: assert on_disk["tasks_succeeded"] == 1 -# --- `coder-eval aggregate` CLI ----------------------------------------------- +# --- rebuild_run_summary ------------------------------------------------------ -def test_aggregate_cli_rebuilds_run_json(tmp_path: Path) -> None: - _write_task_json(tmp_path, _eval("a", status=FinalStatus.SUCCESS)) - _write_task_json(tmp_path, _eval("b", status=FinalStatus.ERROR)) +@pytest.mark.skipif(sys.platform == "win32", reason="creating a symlink needs a privilege on Windows") +@pytest.mark.parametrize("name", ["run.json", "run.md"]) +def test_rebuild_refuses_to_write_through_a_symlink(tmp_path: Path, name: str) -> None: + """A run dir is shareable; a planted link would turn the rebuild into an overwrite of any file.""" + run_dir = tmp_path / "run" + _write_task_json(run_dir, _eval("a")) + victim = tmp_path / "victim" + victim.write_text("keep me", encoding="utf-8") + (run_dir / name).symlink_to(victim) - result = CliRunner().invoke(app, ["aggregate", str(tmp_path)]) + with pytest.raises(ValueError, match="symlink"): + rebuild_run_summary(run_dir) + + assert victim.read_text(encoding="utf-8") == "keep me" + + +@pytest.mark.skipif(sys.platform == "win32", reason="creating a symlink needs a privilege on Windows") +def test_report_rebuild_refuses_a_symlinked_run_json(tmp_path: Path) -> None: + run_dir = tmp_path / "run" + _write_task_json(run_dir, _eval("a")) + victim = tmp_path / "victim.json" + victim.write_text("keep me", encoding="utf-8") + (run_dir / "run.json").symlink_to(victim) + + result = CliRunner().invoke(app, ["report", str(run_dir), "--rebuild"]) + + assert result.exit_code == 1, result.output + assert victim.read_text(encoding="utf-8") == "keep me" + assert _shows(result.output, "is a symlink"), result.output + + +def test_rebuild_returns_none_on_an_empty_run_dir(tmp_path: Path) -> None: + assert rebuild_run_summary(tmp_path) is None + assert not (tmp_path / "run.json").exists() - assert result.exit_code == 0, result.output - run_json = json.loads((tmp_path / "run.json").read_text(encoding="utf-8")) - assert run_json["tasks_run"] == 2 - assert run_json["tasks_succeeded"] == 1 - assert run_json["tasks_error"] == 1 - assert (tmp_path / "run.md").exists() +def test_rebuild_does_not_print( + tmp_path: Path, capsys: pytest.CaptureFixture[str], caplog: pytest.LogCaptureFixture +) -> None: + """Callable from both `report` and `evaluate` only because it prints nothing — even the + malformed-entry path, which is a warning in the log instead.""" + _write_task_json(tmp_path, _eval("a")) + _write_prior(tmp_path, skipped_tasks=["not-a-dict", {"reason": "missing required path"}]) + + with caplog.at_level(logging.WARNING): + assert rebuild_run_summary(tmp_path) is not None + + captured = capsys.readouterr() + assert (captured.out, captured.err) == ("", "") + assert any("malformed skipped_tasks" in m for m in caplog.messages) -def test_aggregate_cli_carries_prior_tags(tmp_path: Path) -> None: + +def test_rebuild_carries_prior_tags(tmp_path: Path) -> None: """tags/source-path are static metadata — carried from an existing run.json.""" _write_task_json(tmp_path, _eval("a", status=FinalStatus.SUCCESS)) (tmp_path / "run.json").write_text( @@ -243,67 +308,36 @@ def test_aggregate_cli_carries_prior_tags(tmp_path: Path) -> None: encoding="utf-8", ) - result = CliRunner().invoke(app, ["aggregate", str(tmp_path)]) + rebuild_run_summary(tmp_path) - assert result.exit_code == 0, result.output - run_json = json.loads((tmp_path / "run.json").read_text(encoding="utf-8")) + run_json = _run_json(tmp_path) assert run_json["max_parallel"] == 8 (row,) = run_json["task_results"] assert row["tags"] == ["windows"] assert row["task_path"] == "tasks/a.yaml" -def test_aggregate_cli_to_separate_output_dir(tmp_path: Path) -> None: - src = tmp_path / "src" - out = tmp_path / "out" - out.mkdir() - _write_task_json(src, _eval("a", status=FinalStatus.SUCCESS)) - - result = CliRunner().invoke(app, ["aggregate", str(src), "-o", str(out)]) - - assert result.exit_code == 0, result.output - assert (out / "run.json").exists() - assert not (src / "run.json").exists() - assert json.loads((out / "run.json").read_text(encoding="utf-8"))["run_id"] == "out" - - -def test_aggregate_cli_errors_on_empty_dir(tmp_path: Path) -> None: - result = CliRunner().invoke(app, ["aggregate", str(tmp_path)]) - assert result.exit_code == 1 - assert "no finalized task.json" in result.output - - -def _write_prior(run_dir: Path, **fields: object) -> None: - """Write a minimal prior run.json carrying the given run-level fields.""" - run_dir.mkdir(parents=True, exist_ok=True) - (run_dir / "run.json").write_text(json.dumps({"task_results": [{"task_id": "a"}], **fields}), encoding="utf-8") - - -def test_aggregate_cli_uses_prior_window_timestamps(tmp_path: Path) -> None: +def test_rebuild_uses_prior_window_timestamps(tmp_path: Path) -> None: """When the prior run.json carries start/end, total_duration uses that real wall-clock.""" _write_task_json(tmp_path, _eval("a")) _write_prior(tmp_path, start_time="2026-01-01T12:00:00", end_time="2026-01-01T12:10:00") - result = CliRunner().invoke(app, ["aggregate", str(tmp_path)]) + rebuild_run_summary(tmp_path) - assert result.exit_code == 0, result.output - run_json = json.loads((tmp_path / "run.json").read_text(encoding="utf-8")) - assert run_json["total_duration_seconds"] == 600.0 + assert _run_json(tmp_path)["total_duration_seconds"] == 600.0 -def test_aggregate_cli_malformed_prior_timestamps_fall_back_to_results(tmp_path: Path) -> None: +def test_rebuild_malformed_prior_timestamps_fall_back_to_results(tmp_path: Path) -> None: """A bad timestamp string falls back to the results-derived window (started_at + duration).""" _write_task_json(tmp_path, _eval("a")) # started_at 12:00:00, duration_seconds 2.0 _write_prior(tmp_path, start_time="not-a-date", end_time="also-bad") - result = CliRunner().invoke(app, ["aggregate", str(tmp_path)]) + rebuild_run_summary(tmp_path) - assert result.exit_code == 0, result.output - run_json = json.loads((tmp_path / "run.json").read_text(encoding="utf-8")) - assert run_json["total_duration_seconds"] == 2.0 + assert _run_json(tmp_path)["total_duration_seconds"] == 2.0 -def test_aggregate_cli_carries_skipped_tasks_and_drops_malformed(tmp_path: Path) -> None: +def test_rebuild_carries_skipped_tasks_and_drops_malformed(tmp_path: Path) -> None: """Valid skipped_tasks carry over; a non-dict and a schema-invalid dict drop without crashing.""" _write_task_json(tmp_path, _eval("a")) _write_prior( @@ -315,33 +349,157 @@ def test_aggregate_cli_carries_skipped_tasks_and_drops_malformed(tmp_path: Path) ], ) - result = CliRunner().invoke(app, ["aggregate", str(tmp_path)]) + rebuild_run_summary(tmp_path) - assert result.exit_code == 0, result.output # must NOT abort on the bad entries - run_json = json.loads((tmp_path / "run.json").read_text(encoding="utf-8")) + run_json = _run_json(tmp_path) assert len(run_json["skipped_tasks"]) == 1 assert run_json["skipped_tasks"][0]["path"] == "tasks/skip_me.yaml" -def test_aggregate_cli_coerces_missing_or_zero_max_parallel_to_one(tmp_path: Path) -> None: +def test_rebuild_coerces_missing_or_zero_max_parallel_to_one(tmp_path: Path) -> None: """RunSummary requires max_parallel >= 1; a missing or falsy prior value coerces to 1.""" _write_task_json(tmp_path, _eval("a")) _write_prior(tmp_path) # no max_parallel key - assert CliRunner().invoke(app, ["aggregate", str(tmp_path)]).exit_code == 0 - assert json.loads((tmp_path / "run.json").read_text(encoding="utf-8"))["max_parallel"] == 1 + rebuild_run_summary(tmp_path) + assert _run_json(tmp_path)["max_parallel"] == 1 _write_prior(tmp_path, max_parallel=0) # falsy → still coerced to 1 - assert CliRunner().invoke(app, ["aggregate", str(tmp_path)]).exit_code == 0 - assert json.loads((tmp_path / "run.json").read_text(encoding="utf-8"))["max_parallel"] == 1 + rebuild_run_summary(tmp_path) + assert _run_json(tmp_path)["max_parallel"] == 1 + + +@pytest.mark.skipif(sys.platform == "win32", reason="creating a symlink needs a privilege on Windows") +def test_rebuild_names_the_run_after_the_resolved_directory(tmp_path: Path) -> None: + """`runs/latest` is a symlink; the run id must be the run's own directory name, not `latest`.""" + run_dir = tmp_path / "2026-06-22_14-32-27" + _write_task_json(run_dir, _eval("a")) + latest = tmp_path / "latest" + latest.symlink_to(run_dir) + summary = rebuild_run_summary(latest) -def test_aggregate_cli_output_rejects_a_file(tmp_path: Path) -> None: - """`-o` is a directory (file_okay=False) — pointing it at a file is a clean usage error.""" + assert summary is not None and summary.run_id == "2026-06-22_14-32-27" + + +# --- find_run_root ------------------------------------------------------------ + + +def test_find_run_root_walks_up_to_the_nearest_run_json(tmp_path: Path) -> None: + _write_prior(tmp_path) + task_dir = _write_task_json(tmp_path, _eval("a")).parent + + assert find_run_root(task_dir) == tmp_path + + +def test_find_run_root_walks_past_the_working_directory_for_a_relative_path( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """`evaluate .` from inside a task dir: `Path(".").parents` is empty unless resolved first.""" + _write_prior(tmp_path) + task_dir = _write_task_json(tmp_path, _eval("a")).parent + monkeypatch.chdir(task_dir) + + assert find_run_root(Path(".")) == tmp_path.resolve() + + +def test_find_run_root_walks_past_another_tools_run_json(tmp_path: Path) -> None: + """A generic `run.json` from another tool must never be mistaken for a run, and later overwritten.""" + (tmp_path / "run.json").write_text('{"name": "someone else"}', encoding="utf-8") + task_dir = _write_task_json(tmp_path / "project" / "copied", _eval("a")).parent + + assert find_run_root(task_dir) is None + + +def test_find_run_root_returns_none_outside_a_run(tmp_path: Path) -> None: + task_dir = _write_task_json(tmp_path / "copied-out", _eval("a")).parent + + assert find_run_root(task_dir) is None + + +# --- `coder-eval report --rebuild` -------------------------------------------- + + +def test_report_rebuild_writes_run_json(tmp_path: Path) -> None: + _write_task_json(tmp_path, _eval("a", status=FinalStatus.SUCCESS)) + _write_task_json(tmp_path, _eval("b", status=FinalStatus.ERROR)) + + result = CliRunner().invoke(app, ["report", str(tmp_path), "--rebuild"]) + + assert result.exit_code == 0, result.output + run_json = _run_json(tmp_path) + assert run_json["tasks_run"] == 2 + assert run_json["tasks_succeeded"] == 1 + assert run_json["tasks_error"] == 1 + assert (tmp_path / "run.md").exists() + + +def test_report_rebuild_output_matches_the_recorded_counts_line(tmp_path: Path) -> None: + _write_task_json(tmp_path, _eval("a", status=FinalStatus.SUCCESS)) + _write_task_json(tmp_path, _eval("b", status=FinalStatus.FAILURE)) + _write_task_json(tmp_path, _eval("c", status=FinalStatus.ERROR)) + + result = CliRunner().invoke(app, ["report", str(tmp_path), "--rebuild"]) + + assert result.exit_code == 0, result.output + summary = RunSummary.model_validate_json((tmp_path / "run.json").read_text(encoding="utf-8")) + counts = f"{summary.tasks_succeeded} ok / {summary.tasks_failed} fail / {summary.tasks_error} err" + expected = f"Aggregated {summary.tasks_run} task(s) ({counts}) → {tmp_path / 'run.json'}" + assert _shows(result.output, expected), result.output + + +def test_report_rebuild_reports_the_not_graded_bucket(tmp_path: Path) -> None: + """A rebuild is the step right after `execute`, so the ungraded bucket must be named.""" + _write_task_json(tmp_path, _eval("a", status=FinalStatus.NOT_GRADED)) + _write_task_json(tmp_path, _eval("b", status=FinalStatus.SUCCESS)) + + result = CliRunner().invoke(app, ["report", str(tmp_path), "--rebuild"]) + + assert result.exit_code == 0, result.output + summary = RunSummary.model_validate_json((tmp_path / "run.json").read_text(encoding="utf-8")) + assert summary.tasks_not_graded == 1 + assert _shows(result.output, f"/ {summary.tasks_not_graded} not graded)"), result.output + + +def test_report_rebuild_on_an_empty_dir_exits_1(tmp_path: Path) -> None: + result = CliRunner().invoke(app, ["report", str(tmp_path), "--rebuild"]) + assert result.exit_code == 1 + assert _shows(result.output, "no finalized task.json"), result.output + + +@pytest.mark.parametrize("extra", [["--format", "md"], ["--format", "html"], ["--output", "summary.md"]]) +def test_report_rebuild_refuses_format_and_output(tmp_path: Path, extra: list[str]) -> None: + """--rebuild writes in place; a --format or --output beside it has no meaning, so it is refused.""" _write_task_json(tmp_path, _eval("a")) - a_file = tmp_path / "not_a_dir.txt" - a_file.write_text("x", encoding="utf-8") - result = CliRunner().invoke(app, ["aggregate", str(tmp_path), "-o", str(a_file)]) + result = CliRunner().invoke(app, ["report", str(tmp_path), "--rebuild", *extra]) + + assert result.exit_code == 2, result.output + assert not (tmp_path / "run.json").exists() + + +def test_report_rebuild_refuses_a_directory_inside_a_run(tmp_path: Path) -> None: + """A run.json written below the root would make every later rebuild of the root drop those rows.""" + _write_task_json(tmp_path, _eval("a")) + _write_prior(tmp_path) + variant_dir = tmp_path / "default" + + result = CliRunner().invoke(app, ["report", str(variant_dir), "--rebuild"]) + + assert result.exit_code == 2, result.output + assert not (variant_dir / "run.json").exists() - assert result.exit_code != 0 # Typer rejects before our code runs (no raw OSError traceback) + +def test_report_rebuild_refuses_a_task_directory(tmp_path: Path) -> None: + task_dir = _write_task_json(tmp_path / "copied-out", _eval("a")).parent + + result = CliRunner().invoke(app, ["report", str(task_dir), "--rebuild"]) + + assert result.exit_code == 2, result.output + assert not (task_dir / "run.json").exists() + + +def test_aggregate_is_no_longer_a_command(tmp_path: Path) -> None: + result = CliRunner().invoke(app, ["aggregate", str(tmp_path)]) + assert result.exit_code == 2 + assert _shows(result.output, "No such command"), result.output