Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions .claude/notes/orchestration.md
Original file line number Diff line number Diff line change
Expand Up @@ -759,6 +759,25 @@ It RAISES rather than guessing when neither is conclusive. Guessing is worse tha
grading the wrong directory makes every path-relative criterion fail as a locating
artifact rather than as a verdict, and reports that as an ordinary score.

A resolved `artifacts_dir_template` is passed in as a SECOND TRUSTED ROOT and preferred as
the candidate, since that template may place artifacts outside `run_dir` entirely. Note the
direction: the roots were WIDENED, never the check relaxed — roots stay operator-supplied,
candidates stay untrusted. Without it, `--resume` over a decoupled layout failed the
containment check, fell through to a `run_dir/artifacts` that did not exist, and raised
`RegradeError`. An artifacts dir that exists outranks the recorded `sandbox_path`, which is
merely what the run claimed.

Only `--resume` (`run_command.py`'s `_grade_resumed_tasks`) passes the resolved
`artifacts_dir` — it has the run's own `BatchRunConfig` in hand, so `resolve_artifacts_dir`
needs no new state. Detached `coder-eval evaluate <run_dir>` does not: it has no
`BatchRunConfig` to resolve a template from, only the untrusted `task.json` it's grading, and
trusting THAT to widen its own containment root would defeat the check the widening exists
to keep intact. A run made with a non-default `artifacts_dir_template` therefore still needs
`--workspace` passed explicitly to `evaluate` — the same escape hatch `RegradeError` already
names. Not a regression: `evaluate` never auto-located such a workspace before this template
existed either, and the alternative (trusting the recorded path) reopens the class of bug the
previous paragraph describes.

**Every return goes through the containment check, rooted at the RUN DIRECTORY.** Both
`sandbox_path` and `task_id` are unvalidated strings out of the run's own `task.json`, so
`"../../../../home/victim"` joins to a real directory `is_dir()` happily confirms. The
Expand Down
88 changes: 88 additions & 0 deletions .claude/notes/persistence.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,94 @@ trajectory log the run had already paid for. `grade.docker.log` exists for the s
one layer down: on the `run --resume` path `docker.log` is already the executed
container's log.

## The run layout as two directory templates

`logging_dir_template` and `artifacts_dir_template` (`BatchRunConfig`, resolved by
`path_utils.resolve_dir_template`) describe where a task's bookkeeping and its artifacts
go. They are resolved LATE — per task, which is the only point where `${variant}`,
`${task}` and `${repeat}` exist at all — and they are INDEPENDENT: artifacts nesting under
the logging dir is a default, not a law.

That independence is the point. Harbor puts logs in its own agent logs dir and artifacts at
the container's WORKDIR, two unrelated parts of the filesystem, and because artifacts were
never a child of the logging dir there is nothing to copy and nothing lands twice. The
previous design reached the same end state with a `--workspace-dir`/`--artifacts-dir` pair
that had to be passed the SAME path so an equality check could infer "don't copy"; a
coincidence standing in for an intention.

An override needs no special-casing anywhere, because substituting a template that contains
no placeholders is the identity function: `/work/output` in, `/work/output` out, down the
same code path as the default. The defaults spell out the historical layout, so an
unspecified run writes byte-identical paths — asserted in `test_path_utils.py` against the
literal old layout rather than against `build_task_run_dir`, which now calls the resolver
and would make the test vacuous.

Two implementation constraints, both load-bearing:

- **`string.Template`, never `re.sub`.** `re.sub` interprets backslashes in the
REPLACEMENT, so a Windows `run_dir` of `C:\runs\2026` comes out mangled.
- **`${task}` may contain a separator.** A dataset-expanded `task_id` is
`"<task>/<row>"` (`expand_dataset`, whose row ids are validated precisely because they
become directories), so it nests — on Windows too, where `pathlib` splits on both
separators.

`${task}` appears twice in the artifacts default (`.../${task}/${repeat}/artifacts/${task}`)
because that IS the layout on disk: the per-task run dir carries it, and
`preserve_to`/`capture_to`/DIRECT_WRITE each appended it again. Kept for compatibility, and
now one string to change rather than five call sites. The `*_as` variants
(`preserve_as`/`capture_as`) exist so a caller-supplied artifacts dir is used as the FINAL
path instead of having `task_id` appended to it a second time; `preserve_to`/`capture_to`
remain as the parent-relative wrappers.

### What run_dir still owns

`${run_dir}` is only a placeholder VALUE. But run-LEVEL files — `run.json`, `run.md`,
`experiment.*`, `resume_fingerprint.json` — still follow `--run-dir`, and its default is
CWD-RELATIVE (`runs/<timestamp>`, `config.py`). Omitting `--run-dir` therefore does not
leave those files harmlessly uncollected; when cwd is the agent's WORKDIR it writes them
INSIDE the workspace, polluting the very directory `artifacts_dir_template` names.
Confirmed live: `artifacts/work/runs/<timestamp>/run.json` in a collected Harbor trial.
Harbor consequently passes `--run-dir /tmp/coder-eval-run`, a throwaway path outside the
workspace.

Anything that used to DISCOVER per-task files by walking `run_dir` had to be given the
resolved logging dirs instead, because they need not live under `run_dir` at all and the
walk silently finds nothing: `atif_emit.emit_trajectories_for_run` (which would emit no
trajectory, leaving Harbor's token/cost totals empty) and
`logging_config.aggregate_task_logs` (which would write an empty `experiment.log`).

### A static template is single-task only

A placeholder-free template is the identity function, so every task in a multi-task
`run`/`execute` would resolve `--logging-dir`/`--artifacts-dir` to the SAME directory and
overwrite each other's `task.json`/artifacts. `run_batch` refuses this loud
(`ValueError`) when `len(resolved_tasks) > 1` and either template has no `${...}`
placeholders (`path_utils.dir_template_is_static`), mirroring the pre-existing
`--workspace-dir` + multi-task rejection. Static paths exist for exactly the single-task
case below.

### A flat run_dir needs no special case in run_batch

Harbor's single-task, static-template mode writes `task.json`/`task.html`/`task.log`/
artifacts flat at the top-level `run_dir` instead of the usual
`<variant>/<task_id>/<NN>` nesting -- the multi-task guard above already guarantees
exactly one resolved task here, so that nesting only exists to disambiguate siblings that
can't occur. A flat `run_dir` also means `trajectory.json` (`emit_trajectories_for_run`'s
sibling write) lands at a fixed, predictable path instead of requiring a recursive glob.
`run_batch`'s `run_single` needs no `workspace_dir` special case for this: `rt.run_dir` IS
the resolved `logging_dir_template`, so "flat" is simply what a static template resolves
to, not a mode this seam has to detect.

### CoderEvalAgent passes both templates as static paths

`harbor/agent.py`'s `CoderEvalAgent.run()` passes `--logging-dir`/`--artifacts-dir` as two
STATIC paths (Harbor's own agent logs dir, and the container's WORKDIR via `$(pwd)`) rather
than templates with placeholders — the case the identity-function property above exists for.
Because artifacts are no longer a child of the logging dir, nothing lands twice; because the
artifacts destination IS the workspace, `capture_as`'s self-referential guard makes the copy
a no-op. `--run-dir` still points at `_THROWAWAY_RUN_DIR` (`/tmp/coder-eval-run`), never
substituted into either template, for the reason in "What run_dir still owns" above.

## Judge persistence

A judge transcript — tool calls, raw verdict, rendered prompt, system prompt — runs 10-100
Expand Down
19 changes: 19 additions & 0 deletions .claude/notes/reporting.md
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,25 @@ container, never where the verifier looks. Confirmed live — the agent's output
every criterion scored 0 as "file does not exist". `$(pwd)` is resolved by the container's
shell at exec time and equals the WORKDIR because the exec is given no explicit cwd.

### The artifacts default is CONTAINER_WORK_DIR, not a required field

Harbor's own artifact collection runs after the agent phase but before the verifier and
container teardown, snapshotting `task.toml`'s `artifacts` source to
`<trial>/artifacts/<source stripped of its leading slash>/` on the host.
`CoderEvalAgent`'s `--workspace-dir "$(pwd)"` runs the agent in-place at the container's
WORKDIR and skips coder-eval's own copy-out, so without an `artifacts` entry nothing the
agent produced would ever be visible on the host.

Defaults to `CONTAINER_WORK_DIR` (`/work`, coder-eval's own image WORKDIR) rather than
requiring every task/experiment to restate it — a package exported by coder-eval needs
coder-eval installed in the image, which in practice means derived from
`coder-eval-agent` (a mismatch already warns; see `_MISSING_CODER_EVAL_WARNING`). Unlike
`[environment].workdir` above — deliberately left unset so the container's own WORKDIR
decides `docker exec -w` — guessing wrong here is not fatal: a nonexistent source is a
best-effort collection miss recorded in the artifact manifest, not an exit 127. Declared
as a plain string, not an `ArtifactConfig` table, because Harbor normalizes
`artifacts = ["/x"]` to `ArtifactConfig(source="/x")` itself.

### What the export carries, and what it refuses to carry

No Dockerfile is written unless the task sets `sandbox.docker.dockerfile_path` — only
Expand Down
29 changes: 28 additions & 1 deletion src/coder_eval/cli/execute_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,13 +189,38 @@ def execute_command(
"--workspace-dir",
help=(
"Run the single resolved task's agent in-place at this absolute path instead of the "
"standard run_dir/artifacts workspace (copied out to run_dir/artifacts/<task> at "
"standard artifacts workspace named by --artifacts-dir (copied out there at "
"cleanup). Requires exactly one resolved task; refused for sandbox.driver: docker "
"(the docker driver already aligns automatically via sandbox.docker.working_dir). "
"Meant for a Harbor `CoderEvalAgent` invocation, so the agent's writes land at the "
"container's own WORKDIR, where Harbor's verifier phase looks for them."
),
),
logging_dir: str | None = typer.Option(
None,
"--logging-dir",
help=(
"Where task.json/task.log go, as a path template. Placeholders: ${run_dir}, "
"${variant}, ${task}, ${repeat}. Default reproduces <run_dir>/<variant>/<task>/<NN>. "
"A static path (e.g. /logs/agent) resolves every task to itself, so it is only for a "
"single-task run (e.g. Harbor); refused for sandbox.driver: docker and for more than "
"one resolved task."
),
),
artifacts_dir: str | None = typer.Option(
None,
"--artifacts-dir",
help=(
"Where the agent's artifacts go -- the FINAL directory, same placeholders as "
"--logging-dir, and independent of it (Harbor puts logs at /logs/agent and "
"artifacts at the container's WORKDIR). When it already holds the workspace "
"there is nothing to copy. Default reproduces <run_dir>/<variant>/<task>/<NN>/"
"artifacts/<task>. A static path is only for a single-task run; refused for "
"sandbox.driver: docker (the in-container Orchestrator has no way to receive it) "
"and for more than one resolved task, and refused together with --resume (it would "
"clear an operator-supplied tree the harness did not create)."
),
),
) -> None:
"""Run evaluation tasks WITHOUT checking their success criteria.

Expand Down Expand Up @@ -250,4 +275,6 @@ def execute_command(
set_overrides=set_overrides,
format=format,
workspace_dir=workspace_dir,
logging_dir=logging_dir,
artifacts_dir=artifacts_dir,
)
Loading
Loading