Skip to content

Commit 6bfdafd

Browse files
committed
[CELL-294, CELL-296] cell chrome/cell login become cell auth chrome, [[volumes]] mount accepts a single path, and thin images stop losing packages when another stack rebuilds against the shared nix-store volume
- feat(cmd/chrome, cmd/auth): move `cell chrome` under `cell auth chrome` and drop `cell login` — host-side credential bootstrap lives under one umbrella; the old top-level commands are gone - feat(cfg, runner): `[[volumes]] mount = "/foo"` expands to `/foo:/foo` — a colonless mount now means "same path on host and inside container", removing the boilerplate of repeating identical paths - fix(runner/thin_build): bake the container-local profile symlink to the just-switched home-manager `/nix/store` realpath and pin it as a per-stack GC root — a thin image no longer silently loses chromium/patchright the moment another cell builds a leaner stack against the shared nix-store volume - fix(runner/thin_build): drop `/nix/var/nix/profiles/per-user/root/profile/bin` from the baked image PATH — PATH lookup no longer resolves tools through the mutable shared-volume slot before the immutable one - test(cfg): cover `VolumeMount.Resolved()` shorthand expansion — no impact - test(runner): assert `-v /foo` becomes `-v /foo:/foo`, and that the thin builder resolves realpath, registers a per-stack GC root, and keeps the shared-volume slot off the baked PATH — no impact - test(cmd/chrome): retarget help/behavior tests to `cell auth chrome`, add regressions that `cell chrome` and `cell login` are no longer registered at the root — no impact
1 parent 6f01b74 commit 6bfdafd

7 files changed

Lines changed: 156 additions & 20 deletions

File tree

‎internal/cfg/cfg.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,22 @@ type VolumeMount struct {
132132
Mount string `toml:"mount"`
133133
}
134134

135+
// Resolved returns the mount string in `host:container[:mode]` form,
136+
// expanding the single-path shorthand where a colonless value means
137+
// "mount this path at the same path inside the container".
138+
//
139+
// Examples:
140+
//
141+
// "/foo/bar" → "/foo/bar:/foo/bar"
142+
// "/foo:/bar" → "/foo:/bar" (unchanged)
143+
// "/foo:/bar:ro" → "/foo:/bar:ro" (unchanged)
144+
func (v VolumeMount) Resolved() string {
145+
if !strings.Contains(v.Mount, ":") && v.Mount != "" {
146+
return v.Mount + ":" + v.Mount
147+
}
148+
return v.Mount
149+
}
150+
135151
// PackagesSection holds [packages] config for npm and python tools.
136152
type PackagesSection struct {
137153
Npm map[string]string `toml:"npm"`

‎internal/cfg/cfg_test.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,23 @@ func TestMerge_EnvAccumulates(t *testing.T) {
9292
}
9393
}
9494

95+
func TestVolumeMount_Resolved(t *testing.T) {
96+
cases := []struct {
97+
in, want string
98+
}{
99+
{"/host/path:/container/path", "/host/path:/container/path"},
100+
{"/host:/container:ro", "/host:/container:ro"},
101+
{"/Users/dmitry/dev/evercars/evercars-backend", "/Users/dmitry/dev/evercars/evercars-backend:/Users/dmitry/dev/evercars/evercars-backend"},
102+
{"", ""},
103+
}
104+
for _, tc := range cases {
105+
got := cfg.VolumeMount{Mount: tc.in}.Resolved()
106+
if got != tc.want {
107+
t.Errorf("Resolved(%q) = %q, want %q", tc.in, got, tc.want)
108+
}
109+
}
110+
}
111+
95112
func TestMerge_VolumesAccumulate(t *testing.T) {
96113
global := cfg.CellConfig{Volumes: []cfg.VolumeMount{{Mount: "a:b"}}}
97114
project := cfg.CellConfig{Volumes: []cfg.VolumeMount{{Mount: "c:d:ro"}}}

‎internal/runner/runner.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,7 @@ func BuildArgv(spec RunSpec, fs FS, lookPath func(string) (string, error)) []str
432432

433433
// cfg [[volumes]] entries
434434
for _, vol := range spec.CellCfg.Volumes {
435-
argv = append(argv, "-v", vol.Mount)
435+
argv = append(argv, "-v", vol.Resolved())
436436
}
437437

438438
// [ports].publish_ip — host interface prefix for `docker run -p`.

‎internal/runner/runner_test.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,18 @@ func TestArgv_ReadonlyVolume(t *testing.T) {
337337
}
338338
}
339339

340+
// Shorthand: a colonless mount path expands to `path:path`, mounting the
341+
// host path at the same path inside the container.
342+
func TestArgv_CfgVolumes_SinglePathShorthand(t *testing.T) {
343+
argv := buildArgv(t, func(s *runner.RunSpec) {
344+
s.CellCfg.Volumes = []cfg.VolumeMount{{Mount: "/Users/dmitry/dev/evercars/evercars-backend"}}
345+
})
346+
want := "/Users/dmitry/dev/evercars/evercars-backend:/Users/dmitry/dev/evercars/evercars-backend"
347+
if !hasConsecutive(argv, "-v", want) {
348+
t.Errorf("expected -v %s in argv: %v", want, argv)
349+
}
350+
}
351+
340352
// --- cfg mise ---
341353

342354
func TestArgv_MiseEnvVars(t *testing.T) {

‎internal/runner/systemprompt.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func ContainerContext(c config.Config, cellCfg cfg.CellConfig) string {
4949
fmt.Fprintf(&b, " /etc/devcell/config = %s (user build config)\n", c.ConfigDir)
5050

5151
for _, vol := range cellCfg.Volumes {
52-
parts := strings.SplitN(vol.Mount, ":", 3)
52+
parts := strings.SplitN(vol.Resolved(), ":", 3)
5353
if len(parts) >= 2 {
5454
mode := "read-write"
5555
if len(parts) == 3 && parts[2] == "ro" {
@@ -64,7 +64,7 @@ func ContainerContext(c config.Config, cellCfg cfg.CellConfig) string {
6464
fmt.Fprintf(&b, " host: %s → container: %s\n", hostDir, hostDir)
6565
fmt.Fprintf(&b, " host: %s → container: %s\n", c.HostHome, homeDir)
6666
for _, vol := range cellCfg.Volumes {
67-
parts := strings.SplitN(vol.Mount, ":", 3)
67+
parts := strings.SplitN(vol.Resolved(), ":", 3)
6868
if len(parts) >= 2 {
6969
fmt.Fprintf(&b, " host: %s → container: %s\n", parts[0], parts[1])
7070
}

‎internal/runner/thin_build.go‎

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -211,14 +211,32 @@ export PATH="/nix/var/nix/profiles/devcell-tools/bin:$PATH"
211211
echo "Running home-manager switch (nix store on volume)..."
212212
home-manager switch --flake %s#devcell-%s%s
213213
214-
# Canonical profile path — home-manager ran as root, so the user profile is at
215-
# per-user/root. The baked ENV PATH, nix-managed MCP server commands, and
216-
# entrypoint fragments all address it via the /opt/devcell path instead.
217-
# -T: replace the link itself, never create inside the target dir.
218-
ln -sfT /nix/var/nix/profiles/per-user/root/profile /opt/devcell/.local/state/nix/profiles/profile
214+
# Capture the just-switched home-manager-path as an immutable /nix/store
215+
# realpath. /nix/var/nix/profiles/per-user/root/profile is a mutable slot on
216+
# the shared devcell-nix-store docker volume — any later home-manager switch
217+
# from another container rewrites it, and every image whose profile symlink
218+
# went through that slot would silently lose packages (see CELL-322: a leaner
219+
# stack build clobbered chromium/patchright out of an ultimate container
220+
# PATH). Resolve once here and bake the resulting store path.
221+
HM_PROFILE=$(readlink -f /nix/var/nix/profiles/per-user/root/profile)
222+
if [ -z "$HM_PROFILE" ] || [ ! -d "$HM_PROFILE" ]; then
223+
echo "ERROR: could not resolve home-manager profile realpath" >&2
224+
exit 1
225+
fi
226+
227+
# Canonical profile path — points at the immutable store target, not the
228+
# shared-volume slot. -T: replace the link itself, never create inside dir.
229+
ln -sfT "$HM_PROFILE" /opt/devcell/.local/state/nix/profiles/profile
230+
231+
# Pin the resolved store path as a persistent GC root on the shared volume
232+
# so nix-collect-garbage from another container cannot reap the target our
233+
# baked-in symlink depends on. Named per-hmTarget + arch so concurrent stacks
234+
# preserve their own home-manager-path independently.
235+
mkdir -p /nix/var/nix/gcroots/devcell
236+
ln -sfT "$HM_PROFILE" /nix/var/nix/gcroots/devcell/%s%s
219237
220238
# Source home-manager session vars (sets NIX_LD for nix-ld)
221-
HM_VARS="/nix/var/nix/profiles/per-user/root/profile/etc/profile.d/hm-session-vars.sh"
239+
HM_VARS="$HM_PROFILE/etc/profile.d/hm-session-vars.sh"
222240
if [ -f "$HM_VARS" ]; then . "$HM_VARS"; fi
223241
# NIX_LD_LIBRARY_PATH: nix-ld needs this to resolve shared libs for non-nix binaries.
224242
# At runtime, entrypoint populates ~/.nix-ld-libs (merged symlink dir).
@@ -291,7 +309,7 @@ COPY entrypoint.sh /opt/devcell/.local/bin/entrypoint.sh
291309
%sENV HOME=/opt/devcell
292310
ENV USER=devcell
293311
ENV DEVCELL_PROFILE=devcell-%s
294-
ENV PATH="/nix/var/nix/profiles/devcell-tools/bin:/nix/var/nix/profiles/per-user/root/profile/bin:/opt/devcell/.local/state/nix/profiles/profile/bin:/opt/devcell/.local/bin:/nix/var/nix/profiles/default/bin:/usr/local/bin:/usr/bin:/bin"
312+
ENV PATH="/nix/var/nix/profiles/devcell-tools/bin:/opt/devcell/.local/state/nix/profiles/profile/bin:/opt/devcell/.local/bin:/nix/var/nix/profiles/default/bin:/usr/local/bin:/usr/bin:/bin"
295313
ENV SSL_CERT_FILE=/etc/ssl/certs/ca-certificates.crt
296314
ENV NIX_SSL_CERT_FILE=/etc/ssl/certs/ca-certificates.crt
297315
ENV LOCALE_ARCHIVE=/nix/var/nix/profiles/devcell-tools/lib/locale/locale-archive
@@ -318,6 +336,7 @@ echo "Done — thin image: %s"`,
318336
stack, // export DEVCELL_STACK
319337
modules, // export DEVCELL_MODULES
320338
flakeArg, hmTarget, archSuffix, // home-manager switch
339+
hmTarget, archSuffix, // /nix/var/nix/gcroots/devcell/<hmTarget><arch>
321340
cellStageCmd, // stage cell binary into $CTX (or empty)
322341
cellCopyLine, // COPY cell ... into Dockerfile (or empty)
323342
hmTarget, // ENV DEVCELL_PROFILE=devcell-<hmTarget>

‎internal/runner/thin_build_test.go‎

Lines changed: 82 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -401,18 +401,90 @@ func TestDockerHostPath_NoEnvNoChange(t *testing.T) {
401401
}
402402
}
403403

404-
// CELL-156 follow-up: home-manager runs as root in the thin builder, so the
405-
// user profile lands at /nix/var/nix/profiles/per-user/root/profile. MCP
406-
// server configs, the baked ENV PATH, and entrypoint fragments all address it
407-
// via the canonical /opt/devcell/.local/state/nix/profiles/profile path —
408-
// the builder must create that symlink or every nix-managed MCP server fails
409-
// with ENOENT at runtime.
410-
func TestThinBuildArgv_CanonicalProfileSymlink(t *testing.T) {
404+
// The canonical container-local profile path
405+
// (/opt/devcell/.local/state/nix/profiles/profile) MUST resolve to an
406+
// immutable /nix/store path, NOT to /nix/var/nix/profiles/per-user/root/profile.
407+
// The per-user/root profile lives on the shared devcell-nix-store docker
408+
// volume — every thin build's `home-manager switch` overwrites it, so a
409+
// container labelled `ultimate` will lose packages the moment someone builds
410+
// a leaner stack against the same volume (see the CELL-322 kirr.dev-540
411+
// clobber where chromium/patchright disappeared from PATH).
412+
//
413+
// The builder captures the just-switched target's realpath and bakes THAT
414+
// store path into the image. Once baked, no cross-container switch can touch
415+
// what this image sees.
416+
func TestThinBuildArgv_ProfileSymlinkResolvesToStorePath(t *testing.T) {
411417
argv := ThinBuildArgv(testCoreImage, testContainer, testVolume, testNixhome, testThinTag, testStack, "x86_64")
412418
script := argv[len(argv)-1]
413-
want := "ln -sfT /nix/var/nix/profiles/per-user/root/profile /opt/devcell/.local/state/nix/profiles/profile"
414-
if !strings.Contains(script, want) {
415-
t.Errorf("builder script must symlink canonical profile path to per-user/root profile, want %q", want)
419+
420+
// MUST NOT symlink into the mutable shared-volume slot.
421+
bad := "ln -sfT /nix/var/nix/profiles/per-user/root/profile /opt/devcell/.local/state/nix/profiles/profile"
422+
if strings.Contains(script, bad) {
423+
t.Errorf("builder must NOT redirect container profile into the shared-volume mutable slot; found: %q", bad)
424+
}
425+
426+
// MUST resolve the profile's realpath after home-manager switch.
427+
if !strings.Contains(script, "readlink -f /nix/var/nix/profiles/per-user/root/profile") {
428+
t.Error("builder must resolve the just-switched profile's realpath (readlink -f) before baking the container-local symlink")
429+
}
430+
431+
// MUST symlink the container-local profile to that resolved store path.
432+
// Match the shell expression that assigns realpath into a var and then uses it as the ln target.
433+
if !strings.Contains(script, `ln -sfT "$HM_PROFILE" /opt/devcell/.local/state/nix/profiles/profile`) {
434+
t.Error("builder must symlink /opt/devcell/.local/state/nix/profiles/profile → $HM_PROFILE (the resolved store path)")
435+
}
436+
}
437+
438+
// A store path only reachable through a symlink baked inside an image is not
439+
// a GC root — `nix-collect-garbage` on the shared volume can't see through
440+
// image layers. Without a per-stack GC root, a later cleanup wipes the
441+
// home-manager-path our container depends on, breaking every already-built
442+
// image the next time it's started fresh.
443+
//
444+
// The builder must register the resolved profile as an indirect GC root
445+
// under /nix/var/nix/gcroots/ so the store path stays alive on the volume.
446+
func TestThinBuildArgv_ProfilePinnedAsGCRoot(t *testing.T) {
447+
argv := ThinBuildArgvFull(testCoreImage, testContainer, testVolume, testNixhome, testThinTag, "local", "x86_64", testStack, "", "")
448+
script := argv[len(argv)-1]
449+
450+
if !strings.Contains(script, "/nix/var/nix/gcroots/devcell/") {
451+
t.Error("builder must register the resolved home-manager profile as a GC root under /nix/var/nix/gcroots/devcell/ so the shared-volume GC does not reap it")
452+
}
453+
454+
// Per-stack GC root name — different stacks must not clobber each other's
455+
// roots. Use the hmTarget + archSuffix combo, matching the existing flake
456+
// key convention (`devcell-<hmTarget><archSuffix>`).
457+
if !strings.Contains(script, "/nix/var/nix/gcroots/devcell/local") {
458+
t.Error("GC root name must be per-hmTarget so different stacks preserve their own home-manager-path")
459+
}
460+
}
461+
462+
// The image's runtime ENV PATH must NOT include /nix/var/nix/profiles/per-user/root/profile/bin.
463+
// That path lives on the shared devcell-nix-store volume and gets rewritten
464+
// by every `home-manager switch` from any container — leaving it on PATH
465+
// defeats the whole store-path-symlink fix because PATH lookup would still
466+
// resolve tools through the mutable slot before the immutable one.
467+
func TestThinBuildArgv_RuntimePathExcludesSharedProfileSlot(t *testing.T) {
468+
argv := ThinBuildArgv(testCoreImage, testContainer, testVolume, testNixhome, testThinTag, testStack, "x86_64")
469+
script := argv[len(argv)-1]
470+
471+
// Look at the `ENV PATH=` line specifically (the baked image env). The
472+
// build-time `export PATH=` lines inside the builder are fine — they only
473+
// affect the ephemeral builder container.
474+
envPathIdx := strings.Index(script, "ENV PATH=")
475+
if envPathIdx < 0 {
476+
t.Fatal("script must contain a baked ENV PATH= line")
477+
}
478+
envPathLine := script[envPathIdx:]
479+
if nl := strings.Index(envPathLine, "\n"); nl >= 0 {
480+
envPathLine = envPathLine[:nl]
481+
}
482+
if strings.Contains(envPathLine, "/nix/var/nix/profiles/per-user/root/profile/bin") {
483+
t.Errorf("baked ENV PATH must NOT include the mutable per-user/root profile bin; got: %s", envPathLine)
484+
}
485+
// Sanity: the container-local (now immutable) profile bin MUST still be on PATH.
486+
if !strings.Contains(envPathLine, "/opt/devcell/.local/state/nix/profiles/profile/bin") {
487+
t.Errorf("baked ENV PATH must still include /opt/devcell/.local/state/nix/profiles/profile/bin, got: %s", envPathLine)
416488
}
417489
}
418490

0 commit comments

Comments
 (0)