fix(devcontainer): mount the cache volumes at the roots the tools use - #234
Merged
Conversation
Both named volumes missed most of what they were meant to persist, and neither failed loudly. Measured in a container built from current main: ~/.cache/bazel mounted (ud-bazel-cache) ~/.cache/go-build container-local, rebuilt every recreate ~/.cache/bazelisk container-local -- a fresh 61M bazel download each time ~/.cache/pre-commit, ~/.cache/uv container-local /go/pkg (mod 544M + sumdb) container-local ~/go mounted (ud-go-cache), written by nothing Two separate bugs. `ud-bazel-cache` targeted one directory *inside* the cache root, leaving its four siblings ephemeral. `ud-go-cache` targeted `~/go`, but `features/go` bakes GOPATH=/go into the image ENV; running the image with no volumes at all shows no `~/go` and no `/go/pkg`, so that mount was writing nowhere while the real Go state was discarded. Two volumes now, each at the root of what it persists: `ud-cache` -> `~/.cache` and `ud-go-pkg-cache` -> `/go/pkg`. Roots rather than per-tool directories, because per-tool only persists what someone remembered to enumerate -- which is exactly what drifted here, with `bazelisk` missing and nothing noticing. Not all of `/go`: `bin/` there is image content (the feature builds ten tools at image-build time) and Docker seeds a volume from the image only while the volume is empty, so a volume over `/go` would freeze `bin/` at first-mount contents and a later feature bump would install tools nobody sees. Not `/go/pkg/mod` either: the checksum-db cache is its sibling at `/go/pkg/sumdb`, and the root-owned mountpoint parent breaks `go install` with `open /go/pkg/sumdb/...: no such file or directory`. `$GOPATH/pkg` is the boundary that holds -- `pkg/` derived, `bin/` artifact. post-create.sh's chown follows the new targets and is guarded on current ownership: unconditional `chown -R` over a warm `~/.cache` is minutes of re-asserting what is already correct, and on `/go` it would flatten the go feature's `vscode:golang` group. Verified in the devcontainer. Sentinels written in one container survived `devcontainer up --remove-existing-container` in both volumes, alongside 1.9G go-build, 61M bazelisk, 13M pre-commit, 544M mod and 12K sumdb. `/go/bin` is unshadowed -- golint/goplay/revive/staticcheck keep the image's Aug 18 date next to the six post-create installs' Aug 19 -- and `/go` keeps `vscode:golang 2775`. A probe with fresh volumes on the bare image confirms both targets arrive root-owned and unwritable by `vscode`, so the chown is load-bearing for each. `bazel test //...` is 22/22 and `pre-commit run --all-files` passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Syndic
commented
Aug 19, 2026
Syndic
commented
Aug 19, 2026
Syndic
commented
Aug 19, 2026
Review feedback on the three points raised. The chown test asserted only what the loop iterates over. `chown_targets` matched the `for volume_target in …; do` header alone, so a body that chowned a literal -- or stopped chowning entirely -- kept returning the right paths and kept the test green, while the container came up with /go/pkg root-owned and the first `go install` died on permission denied. The docstring had the rationale backwards too: reading only the header is what lets the header and the body disagree. It now matches the loop as a unit, header through `done`, and requires the body to chown `"$volume_target"` on one line -- a `chown` somewhere and the variable mentioned somewhere later is not a chown of it. Both bodies the review named now raise, and the guarded real body still parses, which is its own test since the chown is neither the first nor the only line. The ownership guard tested `%u` while the chown sets `$(id -u):$(id -g)`, so a GID-only drift was skipped. Now `%u:%g`, which costs nothing: `sudo chgrp root /go/pkg` in a live container makes the old test skip and the new one fire. The guard reads the mount root, so it cannot heal a root-owned entry left *inside* an otherwise-correct tree -- a tool run under sudo in here, or a `chown -R` interrupted partway, which visits pre-order and so fixes the root first. Keeping the guard, since a full walk costs roughly what it saves, but the comment claimed a self-heal it only partly delivers; it now says which case it heals, which it does not, and how to recover by hand. The narrower statement made the earlier self-heal clause redundant, so that paragraph is merged rather than left to drift against it. CLAUDE.md said go-build was 967M, a figure inherited from the superseded attempt; this PR measured 1.9G in both the gap table and the persistence output, and the doc is the copy that outlives the PR. Verified: both review mutations fail the test and the real script passes; recreated the container, sentinels and all five caches survived, /go/bin still unshadowed and /go still vscode:golang 2775; `bazel test //...` 22/22; `pre-commit run --all-files` passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Syndic
commented
Aug 19, 2026
Review follow-up. `_CHOWN_OF_LOOP_VAR_RE` ran against the raw loop body,
so a chown that had been *commented out* still satisfied it. Reproduced
against the committed code before fixing:
PASSES real body PASSES commented-out chown
PASSES chown other path, same line caught chown literal
caught no chown at all
The realistic trigger is the one post-create.sh now sends people into: a
root-owned entry inside ~/.cache, someone comments the chown out to test
a hypothesis by hand, and does not restore it. `chown_targets` returns
both paths, the coupling test passes, and the next fresh create comes up
with both volumes root-owned -- the failure the body assertion exists to
catch.
Comment lines are stripped before the match now, per the suggestion.
Also closed the second row, which the review flagged as contrived and
worth leaving: `[^;&|\n]` bounds the gap between `chown` and the path, so
the path has to be an argument of the chown rather than a mention on a
line it happens to share. It is one character class, and without it the
assertion does not mean what its name says.
The `\n` in that class is load-bearing and cost a red test to find. A
negated character class matches newlines even without DOTALL, so the
first cut -- `[^;&|]` -- silently un-did the line-bound property the
comment above it is careful about, and
test_chown_targets_requires_the_chown_and_the_path_on_one_line caught it.
The comment now says why the `\n` is not redundant.
Four tests: the commented-out body, a chained command per separator, and
-- guarding the other direction -- a chown whose path is followed by
`|| true`, which is still a chown of that path and must keep passing.
Verified: every row of the review matrix now resolves as intended, the
real post-create.sh still parses to both targets, `bazel test //...`
22/22, `pre-commit run --all-files` passes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 19, 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.
The gap
Both named volumes missed most of what they were supposed to persist. Measured in a container built from current
main(3b10601), withfindmntnaming which paths were actually volumes:~/.cache/bazelGOCACHE~/.cache/go-build~/.cache/bazelisk~/.cache/pre-commit~/.cache/uv$GOPATH/pkg(module + checksum-db caches)/go/pkg~/goTwo separate bugs:
ud-bazel-cachetargeted~/.cache/bazel, one directory inside the cache root, so its four siblings were container-local.ud-go-cachetargeted~/go, butfeatures/gobakesGOPATH=/gointo the image ENV. Running the image with no volumes at all is the check that settles it:So
~/gois a path nothing in the container writes, while the real Go state at/go/pkgwas discarded on every rebuild.Neither fails loudly. The container just rebuilds those caches every time, which reads as "devcontainers are slow".
The fix
Two volumes, each at the root of what it persists:
ud-cache→~/.cacheud-go-pkg-cache→/go/pkgWhy the whole of
~/.cache, not per-tool volumes. Per-tool only persists what someone remembered to enumerate, which is precisely the failure being fixed —bazeliskwas silently missing and nobody noticed. Mounting the root means a tool that starts caching under XDG is covered with no edit. Nothing under it wants to stay ephemeral either: every entry is content-addressed or key-validated by its own tool, and CI builds cold, so a stale local cache can't reachmain.Why
$GOPATH/pkgand not$GOPATH./go/binis image content —features/gobuilds ten tools there at image-build time — and Docker only seeds a volume from the image while the volume is empty. A volume over all of/gowould freezebin/at whatever the image held on first mount, so a later feature bump would install tools nobody ever sees. Persisting it buys nothing anyway:post-create.shreinstalls its six pinned tools over the image's copies on every create.pkg/is the derived half of GOPATH,bin/the artifact half.Why not
$GOMODCACHEalone. The checksum-db cache is a sibling of the module cache at/go/pkg/sumdb, and Docker creates the/go/pkgmountpoint parent root-owned, so the firstgo installdies onopen /go/pkg/sumdb/sum.golang.org/latest: no such file or directory.Nesting a
~/.cachevolume under the existing~/.cache/bazelone would have worked mechanically, but leaves two volumes with overlapping meaning and no axis separating them —bazel clean --expungealready gives per-tool discard from inside the container. Soud-bazel-cacheis retired rather than kept.post-create.sh's chown follows the new targets and is now guarded on current ownership. Unconditionalchown -Rover a warm~/.cacheis minutes spent re-asserting ownership that is already correct, and it would flatten the go feature'svscode:golanggroup.Validation
All run in the devcontainer.
Persistence. Sentinels written in one container, then
devcontainer up --remove-existing-container, then read back in the next:/go/binis not shadowed — the four toolspost-create.shdoes not install keep the image's date next to the six it does:Ownership — the go feature's group survives on
/go, and only the mount targets are re-owned:The chown is load-bearing for both targets, probed with fresh volumes against the bare image so no hook had run:
That corrected a claim carried over from the earlier attempt at this change, which said
/go/pkgwas the target that "actually arrives root-owned". The image has a directory at neither, so both do; the comment and the CLAUDE.md bullet say that now.Warm effect. In the freshly recreated container:
go build ./...0.70s,pre-commit install-hooks0.35s. The recreate'spostCreateCommanddid not re-download bazel 9.2.0 anduv syncreportedChecked 6 packagesrather than installing them — both were a download and an install on the pre-change container in the same session.Suite.
bazel test //...— 22/22 pass.pre-commit run --all-files— all seven hooks pass.New tests are mutation-checked — each of these makes
//.devcontainer:test_devcontainer_configfail, and reverting makes it pass:/go(with the chown loop widened to match, so it is the root assertion catching it, not the agreement one)~/.cache/bazel/go/pkgfrom the chown loopchownlineNotes for the reviewer
docker volume rm ud-bazel-cache ud-go-cachereclaims the two now-unreferenced volumes (~23G forud-bazel-cacheon this host). Don't run it while other worktrees' containers are still up — they hold the old mounts until they are recreated.ud-go-cacheat~/gofull of files that a container on this branch wrote at/go, which reads as "~/gowas live" and isn't.findmnt, ordocker runagainst the bare image, is what settles it..claude/CLAUDE.mdgains a "Devcontainer cache volumes" section — expect a possible trivial conflict with ci(bazel): namespace the repository cache per workflow #229 and ci: cache Go modules at every setup-go call site #230, which also append sections to that file.source=directives, and move the check into the editor #232 touched four of its five files. Re-applied cleanly on 3b10601 rather than rebased.govulncheckis red on pristinemainright now (four Go 1.26.5 stdlib advisories, fixed in 1.26.6). Pre-existing and unrelated to this branch.🤖 Generated with Claude Code