feat(anvil): containerized execution - simplified - #83
Merged
martinhavelka (wukchung) merged 111 commits intoAug 31, 2026
Conversation
The 0.4.0 backend shipped nine generated files -- two shell drivers, two image-id helpers, an entrypoint, a README -- plus a tier-routing seam (runner.just, the anvil-runner managed region) and roughly 1200 lines of duplicated .sh/.ps1. Customizing it meant owning that surface. Containerized execution is now two artifacts and one optional hook: - .anvil/container/Dockerfile (with its build-context ignore file) defines what the image contains. It installs tools by running just anvil-setup, so there is no second tool list to keep in step. A repository edits it in place; a downstream catalog replaces it via replace_artifact. - justfiles/anvil/container.just drives the engine. The image tag *is* a SHA-256 over the Dockerfile, its ignore file, rust-toolchain.toml, the optional hook and the generated recipe tree, so presence implies freshness and there is no staleness bookkeeping. - .anvil/container/hooks.ps1, when present, supplies credentials. Anvil-PreBuild returns BuildKit secrets, Anvil-PreRun returns run-time environment; both are passed by variable name so a value never reaches a process argument or an image layer, and an empty value fails closed. There is no configuration file: whether the group is emitted is a catalog decision, and the only host-specific value -- which engine to call -- is the ANVIL_CONTAINER_ENGINE variable read at run time. There is no transparent tier routing either: just anvil-pr runs natively and the container is reached only through just anvil-container <recipe>. aprz.just drops its container-specific token plumbing; inside the image a credential now arrives as an ordinary environment variable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Docker installed inside WSL without Docker Desktop is a documented, common Windows setup -- it is the one this repository's own docs describe -- and it leaves no Windows CLI behind. Invoking the engine directly therefore made the feature unreachable on exactly the configuration we tell people to build. On Windows only, and only when the engine is absent from PATH, the recipe now routes through the default WSL distribution and translates the repository root with wslpath. Paths cross that boundary with forward slashes, since the intervening shell would otherwise eat the separators and hand wslpath a mangled path. Build secrets and forwarded run-time values are exported through WSLENV, which is where the engine reads them from when it runs there. Docker Desktop and Podman ship a Windows CLI, are found on PATH, and never take this path. scripts/test-anvil-container.ps1 is a black-box walk through the feature from a user's seat: it creates a repository, generates into it, and then only does what a developer would do. Its setup is held to that standard deliberately -- if it had to hand-write a generated file or work around a defect, that would be a bug in the product rather than something the script should absorb. 43 checks, against a real Docker daemon: the emitted artifacts, a first run that builds, a second that reuses, a toolchain bump that renames the tag and a revert that restores it, a Dockerfile edit that survives regeneration, a hook whose secret reaches the build without reaching a layer and whose value reaches a recipe at run time, an empty secret that fails closed, and a nested invocation that stays native. The design doc is rewritten against the narrowed contract. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…r engine The customization story was stated three times -- in containers.md, in extensibility.md 6.1, and in the crate docs that generate the README. That happened because main already carried a container subsection in extensibility.md for the old backend, and it was rewritten in place rather than reconsidered. Three copies of the same contract drift, and the one that drifts is whichever the reader happens to find. containers.md now owns it: the levers, who each one is for, and the coupling between the Dockerfile and its ignore file, including what a fork inherits rather than replaces. extensibility.md 6.1 keeps only what is genuinely a rule of the extensibility system -- justfiles/ holds .just recipes and nothing else, enforced by CatalogBuilder::build -- and explains that containerized execution is why. The customization detail is a pointer. The crate docs keep a sentence, since a README reader needs "you can change the image", not the fork API. Host setup gains a subsection per engine. Docker and podman need different things on Windows, and the old prose buried that: Docker Desktop and podman both ship a Windows CLI, while Docker-in-WSL does not and is reached through the WSL fallback. Each path is now written out, and podman's status is stated plainly -- wired up, expected to work, verified by nothing, with the BuildKit assumptions in the credential path called out as the specific risk. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
…onestly Three fixes found by CI and by running the e2e against podman. dependency_recipe_sources became dead when the old container tests were removed. CI builds with -D warnings, so dead_code is a hard error there and a silent warning locally; the function had no remaining caller and is deleted. "natively" was missing from the dictionary, which is the whole of the spell-check failure. cargo-spellcheck cannot run on this machine (it needs libclang), so the word was found by CI rather than locally. Running the e2e with -Engine podman surfaced a genuine engine defect: podman 6.0.2 on Windows cannot mount a build secret at all. It composes its own temp path from the build context after translating it into its machine's view, then joins it with a Windows separator, and fails before the build starts. A four-line Dockerfile reproduces it with no anvil involved, and `src=` fails identically to `env=`, so there is nothing to work around on our side. The secret plumbing therefore stays as it was -- by environment variable name, so the value never touches disk. The build failure path gains one hint, conditioned on "secrets were passed and the build failed" rather than on the engine, because that error names neither the secret nor the engine. The e2e now skips the three hook sections on podman-for-Windows with the reason stated, rather than reporting a known engine limitation as failure every run, and its image cleanup matches anywhere in a reference: podman reports images fully qualified (localhost/anvil-...) where docker does not, so the old prefix match left images behind and the next run saw a warm cache it expected to be cold. Validation: docker 43/43, podman 31/31 (3 hook sections skipped), cargo test -p cargo-anvil --all-features under -D warnings, --dry-run clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (97.7%) is below the target coverage (100.0%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #83 +/- ##
======================================
Coverage 97.7% 97.7%
======================================
Files 286 286
Lines 62257 62460 +203
======================================
+ Hits 60830 61032 +202
- Misses 1427 1428 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…e tag Section 9 of the e2e was headed "hook output does not change the tag" and did not test it. One assertion rewrote the hook to byte-identical contents, which only shows the hash is deterministic; the other changed the hook body and asserted the tag *did* move, directly contradicting the comment above it. Both were really re-testing that file content is hashed, which section 7 already covers, so the invariant the design leans on -- a credential must never influence a tag -- was unverified by a test that claimed to cover it. Proving it needs the hook file to be byte-identical while what it returns differs, so the fixture hook now reads its value from the environment and the reference is resolved twice with two different values. That matters beyond tidiness: if a minted value reached the hash, two developers holding different tokens would compute different images from identical inputs, and every token rotation would force a rebuild. The file-content half is kept as its own assertion, since both halves are load-bearing and they pull in opposite directions. Found by an independent review pass over the PR. docker 44/44. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
clippy::cloned_ref_to_slice_refs fires on \paths(&[replaced.clone()])\, and the workspace denies warnings. The clone was pointless anyway -- the slice only needs to borrow. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…se review gaps An independent review pass over the PR found two functional regressions against behaviour the deleted runner.just explicitly had, plus a set of comments and assertions that claimed more than the code delivered. Every `just` value pasted into a PowerShell literal is escaped again. The removed runner.just wrapped each interpolation in replace(…, "'", "''") and the new driver had none, so a repository path containing an apostrophe broke every anvil-container recipe, and `target` let the remainder run as host PowerShell -- outside any container, and before the engine check. The image name and workdir stay unescaped because the first is regex-sanitized at definition and the second is a literal; a unit test now asserts nothing else slips through, since this convention has been lost once already. `*target` is split back into argv. Joining its parts into one argument made `anvil-container anvil-setup binstall` look for a recipe named "anvil-setup binstall", and around fifty generated recipes take a parameter -- including the one the image's own Dockerfile runs. ANVIL_CONTAINER_NO_REBUILD is checked outside the NO_CACHE guard. Nested inside it, a developer with NO_CACHE exported got a from-scratch build out of `anvil-container-status`, which is precisely what NO_REBUILD exists to prevent. The e2e's "no secret in any image layer" assertion could not fail: `history` reports the command that created each layer, not its contents, and the value is never a build argument. It now greps the image filesystem, where a written secret would actually land. Documentation corrected where it outran the code: containers.md §5 still described the src= transport that was reverted; the Dockerfile claimed the recipe refuses a floating base image, which it never checks; the COPY comment overstated what is copied; the cache-volume comment claimed collision-freedom the basename cannot provide; extensibility.md kept a conclusion whose mechanism had changed underneath it; and the crate docs omitted the WSL-only engine path this feature exists to support. Podman's second limitation -- it reads only a context-root ignore file, so the whole worktree is streamed -- is documented beside the build-secret one. docker 46/46; cargo test and clippy --all-targets --all-features clean under -D warnings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
BuildKit finds `<dockerfile>.dockerignore` on its own; buildah reads only a context-root ignore file. Without the flag every podman build streamed the whole worktree -- `target/` included -- to the daemon, and a consumer repository owning a root `.dockerignore` had that one obeyed instead, which can exclude `justfiles/` and fail the build for a reason that names nothing relevant. Named rather than probed, because the engine that does not take the flag rejects it outright. This is a capability difference between the two engines, in the same category as the uid mapping already handled, not a second code path through the feature. docker 46/46, podman 31/31. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Anvil spliced `[lints] workspace = true` into every workspace member. A member that already declares its own `[lints.*]` then carries both, which cargo rejects outright -- and because the failure is in `cargo metadata`, it takes down every check in the workspace rather than only the offending crate. The ox-tools dogfood hit exactly this: one migrated crate keeps a deliberately lenient lint set, and generating made the repository unbuildable. Generalize the existing delta opt-out, which already solves the same shape of problem for `trip_wire_patterns`, into one `region_body` decision covering both regions. A crate that owns its lints now gets an empty managed region: the region stays tracked, so dropping the crate's own lints later adopts the catalog with no further gesture, and the plan explains the deferral rather than leaving it to be discovered. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 72a310c4-f37b-497e-a10b-eb8d982d532d
…ve one `anvil-container-tag` prints the reference for the current inputs and exits. It is now the only place the content hash is computed -- the resolver asks it rather than repeating the computation -- because a publisher needs the tag before there is an image to inspect, and a published tag is only meaningful while it is the same reference the consumer will later look up. `Anvil-ResolveImage` is a third hook phase, offered the tag when nothing local matches and before a build starts. It returns the reference it made available rather than re-tagging to the local name: a local tag asserts "built here from these inputs", and a fetched image only claims that, since the hash is over source files and cannot be re-derived from layers. The returned reference is inspected before use, because the run is `--pull=never` and a hook that reported an image it never fetched would fail later and further from the cause. Every failure falls through to a local build -- a publisher that has not caught up must not block the change it has not caught up with. Resolution sits inside the NO_CACHE guard, since ignoring the cache has to mean the remote one too, and before the NO_REBUILD guard, since fetching is not building. ANVIL_CONTAINER_NO_RESOLVE skips it, and `anvil-container-status` sets it so a question about this machine cannot pull gigabytes to answer itself. The identity claim in the design doc is qualified accordingly: presence implies the current inputs by construction only for an image built here; for a resolved one the claim rests on the registry's tag immutability. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ae262693-67bf-46d4-a5af-df8da17c2553
clippy::needless_raw_string_hashes, denied workspace-wide. The assertion string contains no quote, so a plain raw string is enough. Mine for pushing without running clippy on that commit; the gate exists exactly for this.
…ints" This reverts commit 3c8d4d7.
…al' into u/mhavelka/anvil-container-acr # Conflicts: # .anvil.lock
The container documentation had drifted into narrating how the feature was built: it defended design choices against alternatives readers never saw, referenced a configuration file that was never shipped, and cited the end-to-end suite as evidence. Rewrite both the design doc and the crate docs around what the feature does and how to use it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ence `containers.md` becomes a reference specification: purpose, command surface, execution model, emitted artifacts, image identity, environment variables, the hook contract, host requirements, customization, and limitations. Every claim is stated against the implementation in `container.just`, `container.rs` and the default `Dockerfile`. Four inaccuracies are corrected: - Process identity. A Linux host passes `--user <uid>:<gid>`; `--userns keep-id`, which rootless podman needs for bind-mount ownership, is not passed. The previous text asserted both that a fork inherits "the uid mapping" and that no mapping is applied. - Podman is best-effort rather than fully supported, and all three known differences from Docker are listed together. - A new recipe subdirectory needs no ignore-file override: the build context re-admits `justfiles/` as a directory and the identity hashes it recursively. Only a Dockerfile that copies something else must replace the ignore file with it. - `hooks.ps1` is not emitted by default, which is what `local.md` already said. Behaviour previously undocumented is now specified: the per-invocation engine override, cache-volume sharing between checkouts of the same directory name, a missing image input as a hard error, the `ANVIL_SECRET_<id>` mount naming, `--pull=never` on run, and the ordinal-sort and LF-normalization properties that make the digest stable across platforms. The crate documentation in `lib.rs` keeps the short form and links out; `README.md` is regenerated from it. `extensibility.md` states the `justfiles/`-holds-recipes-only rule once and points at the customization section, whose anchor had gone stale. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7cdda814-67ed-42b6-9c60-01149a70f78d
…rose The `podman-docker` alias is removed as a justification for not probing between engines, in the recipe comment and in `containers.md`. The behaviour is unchanged: anvil still uses only the engine `ANVIL_CONTAINER_ENGINE` names. The remaining reasons -- presence is not reachability, and a silent choice between two engines splits the image cache -- carry the point on their own. Several passages are rephrased as reference documentation rather than as development notes. The engine-resolution rationale, the WSL path translation and the podman limitations were written as defect narratives, reproducing the diagnosis rather than stating the behaviour a reader needs. The podman build-secret error text is kept, since it is what a user matches against. Two further corrections: the artifact count is three, not two -- `container.just`, the `Dockerfile` and its ignore file, as `artifacts::container::all()` returns and §9 already said -- and `hooks.ps1` is described as one optional script that may define up to three functions, rather than as a single hook. `justfiles/anvil/container.just` and the three tree snapshots are regenerated from the template. `cargo run -p cargo-anvil -- anvil --dry-run` reports 78 items, all unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7cdda814-67ed-42b6-9c60-01149a70f78d
`containers.md` is reordered so each concept is introduced once, at the level a reader needs it, and detail follows the overview it depends on: interface -> artifacts -> identity -> runtime -> engines -> hook -> customization Two sections move as a result. The environment variables join the recipe table in "Command surface", since together they are the complete user-facing interface, and engine resolution and path translation move out of the execution model into "Engines and host setup", which previously repeated them from the other side. Duplication removed. The `just anvil-setup` install path was explained in the artifacts section and again under hashed inputs; the build-context scope was stated in artifacts and again under customization; the engine diagnostic policy appeared in both engine resolution and host requirements; "all three functions are optional" appeared twice in consecutive paragraphs; and the limitations section restated the platform pin, the toolchain requirement and the podman gap already covered above. Each now appears once, with a section reference where a reader might look for it elsewhere. The result is 10% shorter (3714 to 3338 words) while covering the same surface. `lib.rs` is corrected to match: it still described the feature as two generated artifacts, which the previous commit fixed only in the design document. `README.md` is regenerated from it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7cdda814-67ed-42b6-9c60-01149a70f78d
martinhavelka (wukchung)
changed the title
martinhavelka (wukchung)
changed the title
martinhavelka (wukchung)
left a comment
martinhavelka (wukchung)
left a comment
Contributor
Author
There was a problem hiding this comment.
🤖 Automated review pass over this branch: six independent models, then every finding checked against the code before it was written up. Eleven raw findings, eight survived. Both Gemini slots returned nothing, so this is a five-model pass; one slot was still running when the review was posted.
Dropped after checking:
- Spaces in a repository path breaking the WSL bind mount.
wsl.exepreserves a spaced argument; tested. image inspectthrowing under$ErrorActionPreference = 'Stop'instead of setting$LASTEXITCODE.$PSNativeCommandUseErrorActionPreferenceisFalseon pwsh 7.6.4.-split '\s+'losing argument boundaries. Real limitation, butjustflattens*targetbefore this code sees it, so the information is already gone and the comment says so.
Of the eight, the first two are worth acting on before merge; the rest are smaller. The volume finding is the one that undercuts a stated design property, and it reproduces against a real daemon.
martinhavelka (wukchung)
left a comment
martinhavelka (wukchung)
left a comment
Contributor
Author
There was a problem hiding this comment.
🤖 Follow-up. The sixth reviewer in the pass finished well after the others and is reported separately so the existing threads stay stable.
It independently reached the cache-volume finding and the double-quoted interpolation, which is worth knowing: those two were single-model findings in the first batch and are now two-model. It added three further items, all verified here before posting, plus one point that changes the shape of the volume finding rather than standing alone, which is a reply on that thread instead of a new one.
One thing it raised is deliberately not posted. It flagged the whitespace re-split of *target as a defect and proposed passing the values as positional arguments; just flattens a variadic parameter into a single string before the recipe body can see it, and a [script] recipe receives no argv, so the boundaries are gone upstream of this code and the comment above the split already says so. Real limitation, not a defect, and the same conclusion the earlier batch reached.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR rewrites cargo-anvil’s containerized execution to a much smaller, explicit surface: container runs are opt-in via just anvil-container …, the image is defined by a repository-owned .anvil/container/Dockerfile (+ Dockerfile.dockerignore), and the previous tier-routing seam (ANVIL_RUNNER, runner.just, tier-routing tests, and the large generated .anvil/container/ driver set) is removed. Documentation and templates are updated to match the new contract, including the optional PowerShell hook (hooks.ps1) for secrets/env/image resolution.
Changes:
- Remove transparent tier routing (
ANVIL_RUNNER,runner.just, routed tier recipes) so tiers always run natively and container use is explicit. - Introduce the simplified container backend:
justfiles/anvil/container.justdrives build/run and computes a content-based image tag;.anvil/container/Dockerfile*defines the image;hooks.ps1is optional. - Update docs/tests/templates to reflect the new container contract and credential handling (including APRZ token behavior).
Reviewed changes
Copilot reviewed 53 out of 54 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| justfiles/anvil/tiers.just | Removes routing via _anvil-run; tiers call their work directly. |
| justfiles/anvil/runner.just | Removes legacy tier-routing recipe file. |
| justfiles/anvil/mod.just | Drops import of runner.just. |
| justfiles/anvil/container.just | Replaces old driver invocation with new PowerShell-based engine/tag/build/run implementation. |
| justfiles/anvil/checks/aprz.just | Simplifies token acquisition to GITHUB_TOKEN or gh auth token, warns and continues unauthenticated otherwise. |
| Justfile | Removes anvil-runner managed region and ANVIL_RUNNER default. |
| crates/cargo-coverage-gate/README.md | Updates doc2readme dependency metadata blob. |
| crates/cargo-anvil/tests/tier_routing.rs | Removes tests for tier-routing seam (ANVIL_RUNNER, _anvil-run). |
| crates/cargo-anvil/tests/extensibility.rs | Updates container artifact expectations to Dockerfile/Dockerignore/hooks model and adjusts customization test. |
| crates/cargo-anvil/tests/container_upgrade.rs | Removes upgrade-path tests tied to the previous .anvil/container/ asset set relocation. |
| crates/cargo-anvil/tests/container_customization_bash.rs | Removes Bash driver contract tests for the old customization/driver system. |
| crates/cargo-anvil/templates/regions/justfile-runner.just | Removes the template for the anvil-runner region. |
| crates/cargo-anvil/templates/justfiles/anvil/tiers.just | Mirrors tier de-routing changes in templates. |
| crates/cargo-anvil/templates/justfiles/anvil/runner.just | Removes template for legacy runner.just. |
| crates/cargo-anvil/templates/justfiles/anvil/mod.just | Mirrors removal of runner.just import in templates. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/aprz.just | Mirrors APRZ token logic update in templates. |
| crates/cargo-anvil/templates/container/Dockerfile.dockerignore | Adds new build-context scoping ignore file template for Dockerfile-based image. |
| crates/cargo-anvil/templates/container/Dockerfile | Adds new default execution-image Dockerfile template (installs tools via just anvil-setup binstall). |
| crates/cargo-anvil/templates/anvil/container/run-in-container.sh | Removes legacy generated Bash driver script template. |
| crates/cargo-anvil/templates/anvil/container/run-in-container.ps1 | Removes legacy generated PowerShell driver script template. |
| crates/cargo-anvil/templates/anvil/container/README.md | Removes legacy generated container backend README template. |
| crates/cargo-anvil/templates/anvil/container/image-id.sh | Removes legacy image-id helper template (Bash). |
| crates/cargo-anvil/templates/anvil/container/image-id.ps1 | Removes legacy image-id helper template (PowerShell). |
| crates/cargo-anvil/templates/anvil/container/entrypoint.sh | Removes legacy entrypoint template. |
| crates/cargo-anvil/templates/anvil/container/Containerfile.dockerignore | Removes legacy Containerfile ignore template. |
| crates/cargo-anvil/templates/anvil/container/Containerfile | Removes legacy Containerfile template. |
| crates/cargo-anvil/src/lib.rs | Updates crate docs to describe explicit container invocation, content-based image identity, and hooks. |
| crates/cargo-anvil/src/anvil/artifacts/region.rs | Removes Justfile runner region artifact emission. |
| crates/cargo-anvil/src/anvil/artifacts/mod.rs | Removes runner artifacts and adjusts owned-file marker expectations for editable Dockerfile artifacts. |
| crates/cargo-anvil/src/anvil/artifacts/justfile.rs | Removes embedded runner.just and updates template assertions for tiers/APRZ. |
| crates/cargo-anvil/README.md | Mirrors updated containerized execution documentation. |
| crates/cargo-anvil/docs/design/README.md | Updates design-doc index and repository layout notes for the new container model. |
| crates/cargo-anvil/docs/design/local.md | Updates emitted-tree documentation for .anvil/container/ and container invocation model. |
| crates/cargo-anvil/docs/design/extensibility.md | Replaces the old “optional container runner” section with placement rules aligned to new content hashing model. |
| .spelling | Adds new container-related terms and removes obsolete Containerfile. |
| .anvil/container/run-in-container.sh | Removes legacy generated Bash driver file from this repo’s generated assets. |
| .anvil/container/run-in-container.ps1 | Removes legacy generated PowerShell driver file from this repo’s generated assets. |
| .anvil/container/README.md | Removes legacy generated README from this repo’s generated assets. |
| .anvil/container/image-id.sh | Removes legacy generated image-id helper (Bash) from this repo’s generated assets. |
| .anvil/container/image-id.ps1 | Removes legacy generated image-id helper (PowerShell) from this repo’s generated assets. |
| .anvil/container/entrypoint.sh | Removes legacy generated entrypoint from this repo’s generated assets. |
| .anvil/container/Containerfile.dockerignore | Removes legacy generated ignore file from this repo’s generated assets. |
| .anvil/container/Containerfile | Removes legacy generated Containerfile from this repo’s generated assets. |
| .anvil/container/Dockerfile.dockerignore | Adds the new generated Dockerfile ignore file to this repo’s generated assets. |
| .anvil/container/Dockerfile | Adds the new generated Dockerfile to this repo’s generated assets. |
| .anvil.lock | Updates tracked artifact set/checksums to the new container artifacts and removed runner region. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The run mounted named volumes over \ and \. An engine seeds a named volume from the image only when that volume is first created, so the first image's tools were pinned over every later tag: a tool bump renamed the tag, built a new image, and still ran the old binary. Both paths are also world-writable, so the shadowed copy was mutable and outlived the tag that named it. Cache only cargo's content-addressed registry and git downloads. Engine calls in WSL used 'wsl.exe --', which hands the command line to the login shell. A repository path containing \$ was silently truncated -- wslpath still exited 0, so the guard never fired and the wrong directory was bind-mounted -- and a ';' in a recipe argument ran on the WSL host. Use --exec, which bypasses the shell. Also: derive the image name so it is always a valid reference (a checkout in 'ox-tools (copy)' produced a trailing separator that the engine rejects); report a failed volume removal instead of always exiting 0; give a mapped uid a writable HOME; compute the tag before the presence query so status cannot report a fatal input error as 'not present'; correct the ANVIL_IN_CONTAINER comment and the documented engine override, neither of which matched the code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
anvil-aprz runs in the pr-fast group and queries the GitHub advisory API, which allows 60 requests an hour unauthenticated -- fewer than a full tier needs. Inside the image it had no route to a token at all, so the flagship 'just anvil-container anvil-pr' degraded to warnings and rate limits instead of failing with instructions. Forward GITHUB_TOKEN by name when the host already has it set, so a containerized tier authenticates exactly as the same recipe does natively and CI keeps working when it runs a tier this way. It is forwarded, never minted: running 'gh auth token' in the driver would hand a broadly-scoped credential to every recipe in the container, including the ones that never see it on the host, where anvil-aprz scopes it to itself. Forwarding by name only works if the engine can see the name, so the WSLENV bridge now covers every forwarded variable rather than only the hook's -- without that, '-e NAME' reaches an engine that cannot see NAME and silently forwards nothing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The image copied all of justfiles/ but the tag hashed only justfiles/anvil/**/*.just. A file that is copied without being hashed can change what a build produces while naming a tag that already resolves -- and because presence of the tag is checked first, the existing image is reused and the change is never built. Narrow the context to justfiles/anvil/ so the two sets agree by construction. The public catalog had no reachable symptom, since the synthetic Justfile imports only the anvil tree, but a fork that replaces the Dockerfile can copy anything it likes, and the hashed set is fixed and cannot be extended. Record that as a limitation rather than leaving it to be discovered. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 79 out of 83 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
justfiles/anvil/helpers.just:137
- _anvil-unscoped re-invokes
justto set ANVIL_IMPACT=off for the dependency tree, but without[no-exit-message]the outerjustwill add its own "Recipe ... failed" noise on failures, unlike the previous routing wrapper. Add[no-exit-message]to keep output/exit behavior focused on the underlying tier/group failure.
crates/cargo-anvil/templates/justfiles/anvil/helpers.just:137 - _anvil-unscoped re-invokes
justto set ANVIL_IMPACT=off for the dependency tree, but without[no-exit-message]the outerjustwill add its own "Recipe ... failed" noise on failures, unlike the previous routing wrapper. Add[no-exit-message]to keep output/exit behavior focused on the underlying tier/group failure.
Requiring every input to be in the index blocked the first container run of a freshly generated repository, before anything had been staged, which is the most common way the feature is met. The end-to-end suite found it: it generates a tree, runs a container, and never gets past the tag. An untracked file is in no commit, so no other checkout reproduces it and no published image depends on it. The reproducibility the index protects is a property of committed content, and framing an untracked input as an ordinary file costs none of it. A link is refused earlier, so an untracked input is a regular file by then. Git is still required and drift on a tracked file is still refused. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8e2a0c6a-d500-4acf-a91b-b443c029e866
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 79 out of 83 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
crates/cargo-anvil/src/region.rs:234
RegionPlacement::Atusestext[..offset]/text[offset..]without ensuringoffsetis on a UTF-8 char boundary. SinceAt(usize)is a public API and accepts an arbitrary byte offset, this can panic for non-boundary values; it should defensively clamp/adjust to a valid boundary before slicing.
…containment guard on Windows HOME is set by neither this image nor docker build, so the credential cleanup expanded to /.netrc and left /root/.netrc in the layer, world readable after the chmod. Both spellings are named now. The test for the containment guard was unix-gated, so the mutation run on Windows found the NotFound match guard could be replaced with true and nothing failed. The two platforms reach an unresolvable component by different routes -- an unsearchable directory, and a name the filesystem rejects -- so the test uses whichever the platform offers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8e2a0c6a-d500-4acf-a91b-b443c029e866
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 79 out of 83 changed files in this pull request and generated no new comments.
Suppressed comments (3)
.anvil/container/Dockerfile.dockerignore:5
- This file’s header says “GENERATED BY cargo-anvil. DO NOT EDIT DIRECTLY”, but the updated design docs explicitly call out
.anvil/container/Dockerfile*as a co-owned customization surface (they should carry a weaker “Managed by cargo-anvil.” marker instead). As-is, the header contradicts the intended contract and discourages the documented opt-in customization path.
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT License.
# GENERATED BY cargo-anvil. DO NOT EDIT DIRECTLY.
# Update the corresponding template in the cargo-anvil crate.
#
crates/cargo-anvil/templates/anvil/container/Dockerfile.dockerignore:5
- This template emits a
Dockerfile.dockerignoreheader that says “DO NOT EDIT DIRECTLY”, but the accompanying design docs now describe.anvil/container/Dockerfile*as intentionally customizable and marked “Managed by cargo-anvil.” The template should match that contract to avoid discouraging supported edits and to keep docs/output consistent.
.anvil/container/Dockerfile:5 - The design docs in this PR state that
.anvil/container/Dockerfile*are deliberate exceptions to the “DO NOT EDIT” marker and should be labeled as co-owned (“Managed by cargo-anvil.”) to invite supported customization in the gaps/regions. This generated Dockerfile currently has no such marker, so readers may not realize edits are supported and preserved.
# syntax=docker/dockerfile:1
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT License.
… fixed mode A fixed 100644 let a chmod +x on an untracked file change the image without changing its tag: git records nothing for it and diff --raw does not report it, so the filesystem is the only thing that can answer. The tag controls reuse inside the checkout even though untracked content has no published identity. Reading the filesystem is safe here for the same reason it is wrong for a tracked file: nothing shared depends on two platforms agreeing about a file that is in no commit. Windows has no executable bit and cannot create one, so it frames the ordinary mode. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8e2a0c6a-d500-4acf-a91b-b443c029e866
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 79 out of 83 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
crates/cargo-anvil/src/region.rs:234
RegionPlacement::Atusestext[..offset]/text[offset..]slicing with a byte offset. Ifoffsetis not on a UTF-8 character boundary, this will panic at runtime. Even if callers intend to pass safe offsets, this is an easy footgun (and the enum docs call it a byte offset). Consider validatingoffsetbefore slicing and returning an error instead of panicking.
…sion The dogfood run found it: cargo test passed on the host and failed inside the container. The fixture used a directory with no search bit, which root ignores, so canonicalize succeeded there and the expected error never came. A container run is often root -- on this box the WSL distribution is -- so the test was asserting a property of the caller's uid rather than of the code. A regular file standing where a directory is expected fails with ENOTDIR for every uid. Windows reports NotFound for that shape, so it keeps the rejected name it already used. Confirmed passing inside the container as root. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8e2a0c6a-d500-4acf-a91b-b443c029e866
… whole directive line FileInfo.UnixMode needs the PSUnixFileStat experimental feature and is absent on PowerShell 7.0, which this recipe supports and which is why ComputeHash is used rather than HashData. The property being missing is silent, so every untracked executable framed as an ordinary file and chmod +x left the tag unchanged. test -x answers on every supported version. Region placement compared the declared opening as a prefix, and '# syntax=docker/dockerfile:1' is a prefix of the equally valid '# syntax=docker/dockerfile:1.7'. Splicing at the declared length cut that directive in half. The whole first line is compared now, with either line ending. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8e2a0c6a-d500-4acf-a91b-b443c029e866
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 79 out of 83 changed files in this pull request and generated no new comments.
Suppressed comments (2)
.anvil/container/Dockerfile.dockerignore:4
- Header says "GENERATED ... DO NOT EDIT DIRECTLY", but the design docs in this PR explicitly call out
.anvil/container/Dockerfile*as co-owned/customizable and marked "Managed by cargo-anvil.". The current wording discourages the supported customization path and contradicts the docs.
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT License.
# GENERATED BY cargo-anvil. DO NOT EDIT DIRECTLY.
# Update the corresponding template in the cargo-anvil crate.
crates/cargo-anvil/templates/anvil/container/Dockerfile.dockerignore:4
- This template emits a "DO NOT EDIT DIRECTLY" header, but the design docs in this PR describe
.anvil/container/Dockerfile*as intentionally customizable/co-owned and marked "Managed by cargo-anvil.". The generated header should match that contract to avoid misleading downstream repositories.
|
…cover the cleared temporary file RegionPlacement::At carries a byte offset from a caller, and slicing on an index that is not a character boundary panics. The offset is walked back to the nearest boundary before the line rounding, which then lands on one by construction. write_file's successful-removal arm was reached only by the unix symlink test, so it was uncovered on Windows and the coverage gate failed at 99.9%. An ordinary run finds nothing to remove and takes the NotFound arm, so the case needed a test of its own. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8e2a0c6a-d500-4acf-a91b-b443c029e866
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 79 out of 83 changed files in this pull request and generated no new comments.
Suppressed comments (2)
.anvil/container/Dockerfile.dockerignore:4
- The container Dockerfile ignore file is intended to be user-editable (it controls the build context and may need to be widened when repositories add content in Dockerfile gaps), but its header currently says “DO NOT EDIT DIRECTLY”. This contradicts the documented exception for
.anvil/container/Dockerfile*and can mislead users into avoiding necessary edits.
# GENERATED BY cargo-anvil. DO NOT EDIT DIRECTLY.
# Update the corresponding template in the cargo-anvil crate.
crates/cargo-anvil/templates/anvil/container/Dockerfile.dockerignore:4
- This template is emitted into
.anvil/container/Dockerfile.dockerignore, which the design docs describe as intentionally editable (“Managed by cargo-anvil.”) so repositories can adjust build-context scope. The current “DO NOT EDIT DIRECTLY” header contradicts that contract and the docs.
…history Section 10 proves the re-entry guard runs a recipe natively inside the image. It reached that through anvil-fmt, which depends on the impact snapshot, and the fixture is a fresh git init with no origin/main to diff against -- so the check failed for a reason it does not test. It looked engine-specific because docker only reached section 10 after sections 7-9, which podman skips, but the same command fails identically on docker against the same fixture. ANVIL_IMPACT=off is the documented escape hatch and makes scoping a no-op, leaving the check measuring the guard. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8e2a0c6a-d500-4acf-a91b-b443c029e866
…ar the lock temp sibling Manifest::save wrote its .anvil.lock.tmp sibling with create+truncate, which follows a symlink left at that name and redirects the write outside the repository. Clear it first, absorbing only NotFound, matching write_file. composed_placement matched the scaffold's first line by equality, so a Dockerfile carrying an upgraded '# syntax=docker/dockerfile:1.7' fell through to byte 0 and the managed region landed above the directive, which makes BuildKit ignore the frontend pin. Match the directive key and splice after the whole line the file carries.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 79 out of 83 changed files in this pull request and generated no new comments.
Suppressed comments (2)
crates/cargo-anvil/templates/anvil/container/Dockerfile.dockerignore:4
- The template header currently says "GENERATED BY cargo-anvil. DO NOT EDIT DIRECTLY.", but
.anvil/container/Dockerfile.dockerignoreis part of the supported customization surface (repositories may need to expand the build context). Update the template header to reflect that edits are allowed/preserved.
.anvil/container/Dockerfile.dockerignore:4 - The header says "GENERATED BY cargo-anvil. DO NOT EDIT DIRECTLY.", but this file is intended to be user-editable (drift-preserved) alongside the composed Dockerfile (see design/updates.md guidance about
.anvil/container/Dockerfile*). The header should not discourage in-place customization.
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT License.
# GENERATED BY cargo-anvil. DO NOT EDIT DIRECTLY.
# Update the corresponding template in the cargo-anvil crate.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 79 out of 83 changed files in this pull request and generated no new comments.
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
crates/cargo-anvil/src/region.rs:220
offsetis clamped withmin(text.len())twice (lines 220 and 232). The first clamp is redundant and adds an extra shadow binding, which makes this insertion logic harder to read.
crates/cargo-anvil/templates/anvil/container/Dockerfile.dockerignore:4
- The template header uses "GENERATED ... DO NOT EDIT", but the docs describe
.anvil/container/Dockerfile*as intentionally customizable and marked "Managed by cargo-anvil." (seecrates/cargo-anvil/docs/design/updates.md). Aligning the header avoids discouraging in-place customization that the tool preserves by design.
.anvil/container/Dockerfile.dockerignore:4 - The header says "GENERATED ... DO NOT EDIT", but the design docs explicitly call out
.anvil/container/Dockerfile*as co-owned/customizable and marked with a weaker provenance marker ("Managed by cargo-anvil."). The current wording discourages the supported customization path and contradictscrates/cargo-anvil/docs/design/updates.md.
# GENERATED BY cargo-anvil. DO NOT EDIT DIRECTLY.
# Update the corresponding template in the cargo-anvil crate.
…ainer-minimal # Conflicts: # crates/cargo-anvil/README.md
The char-boundary walk-back was unreachable: every RegionPlacement::At producer already sits on a boundary, so the loop body was never executed, which left a line uncovered and two arithmetic mutants alive. Index the bytes instead, which cannot panic at any offset, and pin the mid-character case with a test.
Comments describe what the code does, not how it came to do it.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 79 out of 83 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
crates/cargo-anvil/src/plan.rs:563
- The doc comment refers to
Manifest::ensure_contained, but there is no such method (the lexical containment check is performed during manifest parsing via a private helper). This reference is misleading for future maintainers—please reword to describe the behavior without naming a non-existent API.
martin-kolinek
approved these changes
Aug 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Impact
Any anvil-managed repository can run its checks in a pinned Linux image with one command, on Windows or Linux, with no image to publish and nothing to configure. This replaces the 0.4.0 container backend, which shipped nine generated files into every consumer repository and could not build its own image.
For a repository that does not use containers, nothing changes: three artifacts are emitted, and no recipe behaves differently.
Architecture
Three generated artifacts, plus an optional hook.
justfiles/anvil/container.just.anvil/container/Dockerfile.anvil/container/Dockerfile.dockerignore.anvil/container/hooks.ps1The image and the checks cannot disagree about the toolset. The image installs its tools by running
just anvil-setup— the same recipe the checks use, from the same pins — so there is no second tool list to drift.The Dockerfile is composed, not owned. An owned file that invites in-place edits freezes the base digest and four tool pins the moment a repository edits it, while the tag keeps resolving — because it hashes their file. The identity scheme works perfectly and still names a stale image. Anvil owns five regions (
base-image,base,tools,setup,entry) and the four gaps between them are yours, each defined by what must already be true at that point in the build: a secondARG BASE_IMAGEbeforeFROM, a root CA before the first download, a compile dependency beforeanvil-setup, a run-time tool after it. Regions do not make anvil's content unwritable — an edit inside one is still preserved, as everywhere else in the tool — they remove the reason to edit it.Two constraints the region engine grew for this:
# syntax=docker/dockerfile:1is honoured only as line 1 and a sentinel is a comment, so it is seeded as a scaffold outside the regions; and region order is semantic, so ordering is enforced rather than assumed. Both are properties a host declares, so the engine looks them up rather than recognising one file by name.A host anvil cannot compose safely is refused, not guessed at.
upsert_regionappends a missing region at end-of-file, which is right only when the file is already the composed shape — so four states are rejected with a diagnostic and the file left byte-identical: a previous render the repository has since edited, a Dockerfile anvil has never owned, one carrying some regions but not all, and one whose regions are out of order. Refusal means the lock too: the recorded checksum is the provenance the next run reclassifies from, and reverting the edit is the clean recovery, which only exists while that entry survives. Classification happens once per host from its on-disk state, so a refusal is atomic rather than landing a write first.The image tag is a digest of its inputs: everything under
.anvil/container/,rust-toolchain.toml, and every file underjustfiles/anvil/— which is exactly what the build context copies. The container directory is walked rather than named file by file, so anything a gapCOPYs is an input too. The whole recipe tree counts becausejust anvil-setupreaches the install recipes through the tier, group and check recipes, so the routing decides whether a tool is installed just as surely astools.justdecides how. A changed input names a tag that cannot exist yet, so a build follows, and an image that is present is current by construction. The cost is that editing any recipe rebuilds the image; a tag that could name contents the image does not have would make every guarantee here meaningless.The tag covers file modes, not just bytes.
COPYcarries a file's mode into the image, so achmod +xchanges what the image contains without changing a byte. Each input is framed with its git mode. For a tracked file that mode comes from the index, because Windows has no executable bit and reading the filesystem would make two checkouts of one commit compute different tags — which would break the publisher contract above. For an untracked file it comes from the filesystem, which is safe for the same reason it is wrong for a tracked one: the file is in no commit, so nothing shared depends on two platforms agreeing about it.Making the index authoritative costs four refusals, each of which stops the run rather than naming an image that does not match what would be built:
ls-files/difffailingDockerfile<dockerfile>.dockerignore, which anvil maintains under the canonical name; a variant would build with no ignore fileThe image supplies the toolchain, never the code. Recipes always execute from the mount; the copy baked into the image exists only to install tools at build time.
Design:
docs/design/containers.md.Usage
The arguments are the argv executed inside the image, so anvil recipes are reached by naming
just. The first run builds the image; later runs start immediately.just anvil-prstill runs natively — the container is opt-in per invocation, never transparent routing.Requires Docker or Podman (
ANVIL_CONTAINER_ENGINE=podman). On Windows, an engine installed only inside WSL is reached throughwsl.exeautomatically, so Docker Desktop is not required.A fork supplies an internal image and credentials by replacing the base and tool regions and adding
hooks.ps1, which may defineAnvil-BuildSecrets(build secrets),Anvil-RunEnv(run-time environment) andAnvil-ResolveImage(pull a published image). It inherits the catalog install and the entry contract, so a different base OS is two region replacements rather than a forked 116-line file. Credentials are passed by variable name in both phases, so a value never reaches a process argument or an image layer, and an empty value fails closed.Validation
cargo test -p cargo-anvil --no-fail-fast: 462 passed, 0 failedscripts/test-anvil-container.ps1 -Engine docker— 71/71 in 10m04s, black-box against a real daemon: artifacts, build, reuse, tag changes and reverts, content in a Dockerfile gap surviving regeneration, an edit inside a region being preserved, a non-justcommand running in the image, hook secrets reaching the build but not a layer, empty secrets failing closedscripts/test-anvil-container.ps1 -Engine podman: 54/54 in 06:51. Seventeen fewer checks than Docker, because podman on Windows cannot mount build secrets, so the three hook-secret sections do not apply there.cargo anvil --dry-run— exit 0, 87 items unchanged, so the regenerate gate stays greencargo +nightly fmt -- --check,cargo clippy --workspace --all-targets --all-features -- -D warnings,just anvil-spellcheck— cleanscripts/test-anvil-dogfood.ps1: the PR tier run inside the container against this repository (just anvil-container just anvil-pr). It found a test that passed on the host and failed in the image, which no other check on this PR covers. Docker only: it drives the engine directly rather than through the e2e harness.Breaking
ANVIL_RUNNER, theanvil-runnermanaged region,justfiles/anvil/runner.justand the 0.4.0.anvil/container/asset set are gone. A repository that adopted 0.4.0 regenerates and usesjust anvil-container just <recipe>. Nothing consumes that backend today.A repository on the previous layout, where
.anvil/container/Dockerfilewas an owned file, is re-seeded into the composed form on the next run: a file tracked as owned in the lock and carrying none of the regions is a previous render, not composition. A Dockerfile the repository wrote itself is refused and left untouched, because there is no position for the regions that would not put its content aboveFROM; the refusal names the file and the recovery.artifacts::container::dockerfile()never reachedmain— it was introduced and then replaced within this branch — so this is not a break against the released API. It matters to work already built on an earlier commit of this branch: the single constructor is nowdockerfile_base_image(),dockerfile_base(),dockerfile_tools(),dockerfile_setup()anddockerfile_entry(), and a catalog replacing the whole file moves to replacing the regions it actually cares about.Version and
CHANGELOG.mdare untouched: releasing is a separate PR viascripts/release-crate.ps1.