Skip to content

feat: give the host→container boundary a contract and fold aggregate into report --rebuild - #178

Open
uipreliga wants to merge 9 commits into
mainfrom
feat/container-contract-and-command-surface
Open

uipreliga wants to merge 9 commits into
mainfrom
feat/container-contract-and-command-surface

Conversation

@uipreliga

@uipreliga uipreliga commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Gives the host→container boundary of driver: docker a typed contract that both sides must echo, and folds coder-eval aggregate into coder-eval report --rebuild. Rebased onto a2498770 (#177, #176 and #180 merged), so it follows the prose rules those PRs put in force on src/ and tests/.

Breaking changes

  • A skewed or unlabeled driver: docker image is refused before the container starts. The host reads the image's org.coder-eval.version label. If the label is missing, or differs from the installed coder-eval, the run fails. This applies to dockerfile_path images too. Set ALLOW_IMAGE_SKEW=1 (not an empty string) to run a deliberately different image; a missing label is never excused.
  • An image older than the host→container contract is refused. The container must echo the contract it parsed. A result with no echo, or a different one, is moved to task.json.unhonored, and a synthetic ERROR task.json takes its place. This replaces task.json.graded and task.json.rerun.
  • coder-eval aggregate is removed. Use coder-eval report <run_dir> --rebuild.
  • aggregate -o/--output <dir> has no replacement. --rebuild writes in place; gather task directories into the target directory first.

Downstream: UiPath/skills#3318 sets ALLOW_IMAGE_SKEW on the one workflow that tests an ad-hoc image tag against the pinned wheel. It is safe to merge before this release.

What changes

  1. ContainerContext contract. context.json is a strict Pydantic model: every field is required, unknown keys are refused, and grade, regrade and replicate_index are strictly typed. A host/image mismatch fails with exit 2 and names the field, instead of falling back to a default.
  2. Contract echo. The container writes the contract it parsed into environment_info["container_contract"], after the regrade seed so a prior row cannot overwrite it. One host check, _assert_contract_echoed, replaces _assert_grade_honored, _assert_regrade_honored and _warn_on_version_mismatch. The image-version preflight is now the single reader of the label and refuses instead of warning.
  3. Driver rewrite moved host-side. The host stages the task with driver: tempdir and sends the authored sandbox in the contract, so task.json still records driver: docker and the container needs no CE051 exemption.
  4. report --rebuild. A printer-free orchestration/run_summary_rebuild.py backs it. It refuses --format, --output, a task directory, a directory inside another run, and a symlinked run.json/run.md.
  5. evaluate refreshes run.json. coder-eval evaluate <task_run_dir> rebuilds the owning run's run.json itself. It is best-effort (never changes the exit code), accepts only a coder-eval run summary on the walk up, and skips when --run-dir sits inside the run. write_run_summary now writes atomically.
  6. run/execute option sharing: dropped. A factory module was built and gave byte-identical --help, but review found it saved no lines, so cli/ is unchanged. A test now pins every shared flag as identical on both commands.

Rationale lives in .claude/notes/isolation.md (§ The container contract, § The image version preflight, § The contract echo) and .claude/notes/orchestration.md (§ The host-side driver rewrite). Code carries the contract and a Rationale: pointer.

Rebase onto main

14 conflicts across 7 of the 8 commits, all resolved to main's shape plus this branch's facts:

  • Notes. Main reflowed .claude/notes/*.md to 88 columns. Both sides were reflowed with the same wrap (checked: it reproduces main's file byte for byte from the old one), then merged, so the resulting diff is content only.
  • Slimmed docstrings (CE021, CE051, CE052, one test). Main's shorter form is kept. Where this branch changed a fact in it (the driver rewrite is now in DockerRunner._stage_inputs, and the image preflight refuses), the fact is updated in the docstring and in .claude/notes/lint-rules.md.
  • reports_stats.py was split by refactor(reports): generate the pricing mirror and split the reports layer #176. The container_contract exclusion moved to reports/helpers.py::ENV_TABLE_EXCLUDE.
  • prose_budget._TYPER_COMMANDS keys are repo-relative since feat(lint): gate tests/ on the prose rules, and cap the comment run as well as the file total #180. The aggregate entry is removed from the new form, and the exemption-list test compares repo-relative paths.

One follow-up commit applies main's prose rules to this branch's own additions: no history wording in docstrings, and module-level imports in the new CLI helpers and tests/test_container_context.py.

Worth a human's eye

  • ALLOW_IMAGE_SKEW is a Settings field, not a task field. Image freshness is a property of the operator's machine, not of the evaluation.
  • The echo is not a security boundary. The run dir is bind-mounted writable into the agent's container, so an agent can forge container_contract as easily as the verdict beside it. It proves the harness code honored the contract.
  • Seen during the live check, not caused by this PR:
    • With --type codex and no --model, the task's default claude-sonnet-4-6 went to the Azure Codex endpoint. The turn ended with 0 messages, 0 tokens and error: null, and the row was recorded as FAILURE. The Codex adapter should record this as an error.
    • Without --run-dir, evaluate writes grade.log and grade.docker.log to its own new runs/<timestamp>/, not beside the graded row.

Verification

After the rebase (ce5d761e): make verify — ruff format, ruff check, pyright 0 errors, 671 custom lint, prose gate 0 essays, 5,899 passed, 2 skipped, 92.74% coverage.

Before the rebase (4d79e933), not repeated after it:

  • Every phase had a spec-compliance and a quality review; a final whole-change review found and fixed three Medium issues (stale echo in prior.json, a refused row vanishing from run.json, the refused record lost on the grading path).
  • run --help and execute --help byte-identical to the pre-change baseline.
  • A real driver: docker task against a freshly built make docker-image image (0.12.1, label matches the host). tasks/agents/claude_hello_world_docker.yaml with --type codex --model gpt-5.4: SUCCESS, 1.0, 3/3. The echo holds all 10 contract keys with grade: true, regrade: false, and task.json records driver: docker.
  • Detached grade of that row: coder-eval evaluate <row> --allow-recorded-commands graded in a container of the task's image, SUCCESS → SUCCESS, 3/3, exit 0. The echo shows regrade: true, started_at is unchanged (the agent did not run again), task.execute.json was kept, run.json was refreshed, and no graded_on_host stamp was set.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Sep 15, 2026

Copy link
Copy Markdown

Claude finished @uipreliga's task in 1m 37s —— View job


Code Review

Todo List

  • Read .github/code_review.md for review guidelines
  • Read CLAUDE.md for project conventions
  • Review the full diff (git diff origin/main...HEAD)
  • Analyze key architectural changes
  • Check cross-file consistency
  • Identify what's missing
  • Provide structured feedback

Starting code review...

@uipreliga
uipreliga changed the base branch from main to docs/slim-source-prose September 15, 2026 15:05
@akshaylive

Copy link
Copy Markdown
Collaborator

One thin I'm unsure about it it's behavior on custom docker images. Skills repo uses a custom docker image based on the coder-eval docker image. In this scenario, do we force consumers to set the env var? If not this PR LGTM.

@uipreliga
uipreliga force-pushed the docs/slim-source-prose branch from af5e07c to 4941681 Compare September 16, 2026 02:39
Base automatically changed from docs/slim-source-prose to main September 16, 2026 02:46
@uipreliga

Copy link
Copy Markdown
Collaborator Author

Good question — no, consumers don't need to set anything.

A custom image built FROM coder-eval-agent:<ver> inherits the org.coder-eval.version label from the base (Docker propagates Config.Labels), so the preflight passes with no LABEL line of its own. The skills image is exactly this shape: tests/docker/Dockerfile is FROM ${CODER_EVAL_IMAGE}, which run-coder-eval.yml sets to ghcr.io/uipath/coder-eval-agent:${CE_IMAGE_TAG}, and that tag defaults to the pinned wheel version from tests/.coder-eval-version. Image and host agree, so the check is silent. The overlay adds only npm/dotnet/SDK layers; it never reinstalls coder-eval. The one skills path where the two differ on purpose is a coder_eval_image_tag override, and UiPath/skills#3318 sets the escape hatch there.

A consumer only hits the refusal when their base is pinned to an older coder-eval than the host wheel (or a stale latest) — which is the skew the check exists for. The fix is to rebuild the derived image against a matching base, not the escape hatch; the error text names both the rebuild command and the variable.

Two things ALLOW_IMAGE_SKEW deliberately does not cover:

  • A missing label. An image that doesn't descend from the framework image — for example one built with the runtime kit — must apply LABEL org.coder-eval.version itself.
  • The contract echo. An overlay that reinstalls coder-eval keeps the inherited label but runs different code, so the result is refused.

One nit for anyone wiring it up: the value must be 1 or 0, never an empty string — ALLOW_IMAGE_SKEW= fails the boolean parse at CLI startup.

All of this is in docs/DOCKER_ISOLATION.md § Using a pre-built custom image, plus the Troubleshooting custom images table.

@akshaylive akshaylive left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean PR. At some point we need to change the documentation structure. Ship it!

uipreliga and others added 9 commits September 16, 2026 13:24
Replace the json.loads + dict.get + isinstance ladder at the host→container
boundary with a Pydantic model: every field required, extra="forbid",
StrictBool for grade/regrade, StrictInt for replicate_index. The host builds
the model in _stage_inputs and keeps it on DockerRunner._staged_context.

BREAKING CHANGE: a container image and host that disagree about the
context.json keys now fail at parse time (exit 2, naming the field) instead
of falling back to defaults (grade=True, host_task_file=None, the staged
source_yaml).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…oed its contract

The image version preflight now refuses instead of warning, for dockerfile_path
images too, and is the single reader of the org.coder-eval.version label. The
container echoes the ContainerContext it parsed into
environment_info["container_contract"], written after the regrade seed so the
prior row cannot overwrite it. One host guard, _assert_contract_echoed,
replaces _assert_grade_honored, _assert_regrade_honored and
_warn_on_version_mismatch, and quarantines a refused record to
task.json.unhonored. A host grade drops a prior row's stale echo, and the echo
is kept out of the rendered Environment table.

BREAKING CHANGE: a driver: docker run is refused before the container starts
when the image's org.coder-eval.version label is missing or differs from the
host's installed coder-eval. Set ALLOW_IMAGE_SKEW=1 (not an empty string) to
run a deliberately different image; a missing label is never excused. An image
that predates the host→container contract is refused after the run, and its
record is moved to task.json.unhonored instead of task.json.graded /
task.json.rerun.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t-side

DockerRunner._stage_inputs now stages task.yaml as the execution copy with
driver: tempdir and sends the authored sandbox block in the contract as
ContainerContext.authored_sandbox. The in-container entry point no longer
rewrites the driver; it records authored_task from ctx.authored_sandbox, so a
docker run's task.json still says driver: docker. The CE051 exemption moves
with the rewrite: the two sites are now host staging and opt-in host grading.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Extract the run-summary rebuild into a printer-free
orchestration/run_summary_rebuild.py (rebuild_run_summary, find_run_root) and
expose it as `coder-eval report <run_dir> --rebuild`, which prints the same
counts line and note. --rebuild refuses --format, --output, a task directory
and a directory inside another run. The rebuilt run id now comes from the
resolved directory, so `runs/latest --rebuild` records the real run id.

BREAKING CHANGE: `coder-eval aggregate` is removed; use
`coder-eval report <run_dir> --rebuild`. Its `-o/--output <dir>` option has no
replacement: --rebuild writes in place, so gather task directories into the
target directory first.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… grade

`coder-eval evaluate <task_run_dir>` now rebuilds the owning run's run.json
and run.md itself after writing the verdict back, instead of telling the user
to run a second command. The refresh is best-effort: any failure only warns,
and the exit code is always the verdict's.

find_run_root accepts only a run.json that is a coder-eval run summary (a JSON
object with run_id and task_results), so a row copied into another tool's tree
never overwrites that tool's run.json. rebuild_run_summary refuses to write
through a symlinked run.json or run.md; both evaluate and report --rebuild
inherit that, since it is their one shared write path. The refresh is skipped
when the grading --run-dir sits inside the owning run, where its own task.json
would count as a second row. write_run_summary now writes atomically, and the
new console lines escape Rich markup in paths.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…mmand exemption list

Two review findings from the container-contract work, closed as tests:
- the host compares the container's echo with its own dump, so a validator
  under authored_sandbox that is not idempotent across a JSON round trip would
  refuse every docker run; the round trip now runs over a maximal SandboxConfig;
- prose_budget's _TYPER_COMMANDS kept a dead entry for the deleted aggregate
  command; it must now equal the commands the CLI registers.

Two more candidates that need type information or are conventions are recorded
in .claude/harness-candidates.md.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…cally

Phase 6 planned to declare the flags `run` and `execute` share once, through
option factories. The factory module was built and produced byte-identical
--help output, but review found it saved no lines and turned each new shared
flag into a five-place edit, so it was dropped and cli/ is unchanged.

What remains is the drift guard: a test that compares every click attribute a
shared flag can show or validate between the two commands, and asserts that
--resume and --format stay command-specific.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Strip the prior row's container_contract from the staged prior.json. An image
  that honors regrade but predates the echo keeps the prior environment_info,
  so a matching echo from an earlier identical dispatch passed as its own.
- A result refused by the contract echo now leaves a synthetic ERROR task.json
  in place of the quarantined record, like every other container failure, so
  the row no longer vanishes from the next run.json rebuild.
- On a detached grade, the refused task.json.unhonored is folded back from the
  scratch directory beside the graded row, and the error no longer suggests
  --allow-host-grading when the image itself was refused.
- Docstrings, the stale "version checks only warn" wording, the dead None branch
  in _quarantine_record, the run.json environment note, and a cwd-independent
  task path for the evaluate refresh tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rebased onto a249877, which turned the prose gate on for tests/ (#180) and
hoisted function-local imports out of src/ (#176). Brings this branch's own
additions in line:

- Three test docstrings stated history ("Previously ...", "used to hide
  behind", "Accepting it recorded"); they now state the contract.
- `report_command` and `evaluate_command` import `run_summary_rebuild` at module
  level. `evaluate_command` keeps the module import because its tests patch
  functions on that module.
- `tests/test_container_context.py` (new in this branch) imports at module level.
- Reflowed one note paragraph to the notes' 88-column width and rejoined a
  split docstring line in `regrade._should_grade_in_container`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@uipreliga
uipreliga force-pushed the feat/container-contract-and-command-surface branch from 4d79e93 to ce5d761 Compare September 16, 2026 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants