cli: mount extra host directories into the container - #386
cli: mount extra host directories into the container#386woltspace-jerpint[bot] wants to merge 4 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
`woltspace start` hardcoded its bind mounts, so the container could only ever see the wolts directory. Working on a repo that lives elsewhere on the host meant copying it under $WOLTS_DIR — where `woltspace backup` then snapshots it — or hand-rolling `docker run`, which the next `woltspace start` wipes out. Adds a repeatable `--mount src:dst` flag, plus `WOLTSPACE_MOUNTS` in $WOLTS_DIR/.env as a comma-separated sticky equivalent (read the same way WOLTSPACE_PUBLIC_TUNNEL already is), so a plain `woltspace start` keeps them. Entries expand `~`, resolve to an absolute path, and are mounted rw. Two guards run before `docker run`, both failing fast with a clear message: the source directory must exist, and the target may not collide with /workspace/wolts, /workspace/woltspace, or /home/node/.claude. Purely additive — with no flag and no env var, the docker run line is byte-for-byte what it was. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
8a650bc to
03e07b3
Compare
|
Applied the review — all five points. Force-pushed as 1. Source-only, anchored at
Also confirmed the trailing-slash trap you flagged: The 2. Basename collision guard. Names both source paths. One addition beyond the ask: the same directory listed twice (once as a flag, once in 3. Fail fast on an existing container. Placed after the 4. Docs updated to the source-only form, including the read-write warning and the recreate-not-rebuild note. 5. Testing. Full matrix in the PR description — eleven cases. On read-only: left the door open deliberately rather than building it. Parsing the entry whole — no colon splitting — is what keeps a trailing |
Review feedback on the initial src:dst form. The target is now always
/mnt/<name>, derived from the source, so a user never picks one.
That deletes the platform-mount collision guard outright rather than
improving it — /workspace/wolts, /workspace/woltspace and /home/node/.claude
are unreachable by construction. /mnt is also outside /workspace, clear of the
entrypoint's chown -R, and `ls /mnt` now shows a wolt everything it can reach.
Dropping colon splitting fixes two bugs in the previous form:
- the "malformed" check compared _src to _dst *after* ~ expansion, so a
legitimate identity mount (~/code:/Users/me/code) was rejected with a
message about an entirely different problem
- _dst="${_m#*:}" took everything after the first colon, so src:dst:ro
emitted -v src:dst:ro:rw — the cryptic docker error this set out to prevent
It also keeps a trailing :ro mode available as a clean extension later, which
parsing the entry whole is what preserves.
The name uses basename on the resolved path, not ${p##*/}, which returns empty
on a trailing slash and would mount at /mnt/. / is refused explicitly.
Adds a basename collision guard: two different sources wanting the same
/mnt/<name> are rejected naming both paths, since docker's own "Duplicate
mount point" doesn't say which two clashed. The same source listed twice
(flag and .env) dedupes instead of failing.
Adds a fail-fast in the start) branch: bind mounts are fixed at docker run and
start only creates a container when none exists — a running one short-circuits,
a stopped one is docker restart'ed with its original spec — so start --mount on
an existing lodge silently did nothing. It now exits pointing at
`woltspace stop && woltspace start`, a recreate rather than a rebuild. Gated on
the flag alone; WOLTSPACE_MOUNTS applies whenever the container is created,
matching every other .env var. No auto-recreate, deferred until warranted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
f967b22 to
ef2a3fd
Compare
IFS=',' splits on commas alone, so `WOLTSPACE_MOUNTS=~/code/a, ~/notes` kept
the leading space on the second entry and failed with
--mount source not found: /Users/me/notes
where the only clue is a leading space in the path — easy to miss. Spacing out
a comma-separated list is a natural thing to write, so trim both ends of each
field after the split.
[:space:] also covers a stray \r, so a .env saved with CRLF line endings no
longer breaks the last entry. A field that is entirely whitespace — from a
trailing comma — trims to empty and is skipped by the existing guard.
Only surrounding whitespace goes: a directory whose name genuinely contains a
space still mounts (verified `~/my notes` -> /mnt/my notes).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Good catch — fixed in Trimmed both ends of each field after the _m="${_m#"${_m%%[![:space:]]*}"}"
_m="${_m%"${_m##*[![:space:]]}"}"Two things that fell out of using
Checked the obvious way this could go wrong: only surrounding whitespace is removed, so a directory whose name genuinely contains a space still works — Left flag entries untouched. The shell already handles quoting there, and Also documented the tolerance, since silently-accepted-but-undocumented is its own trap: Full matrix re-run after the change — the original eleven plus four new ones, all green. Thanks for running it on real 3.2; that's the environment I can't reach from in here. |
The mount logic had grown to ~70 lines inline in _start_container(), which is
otherwise a short function about running one docker command. The file's own
convention is top-level _helper() functions, so follow it: _build_mount_args()
fills $mount_args, and _start_container() calls it.
Three things collapse in the move:
- `cd`+`pwd` validates and resolves in one step, replacing a separate -d test
followed by a separate resolve. Same trailing-slash normalization.
- the name/source collision scan used two parallel arrays and an indexed while
loop; one `seen` array of name::src entries and a `case` match does it in
a third of the lines, and folds the same-dir-twice dedupe into the same test.
- three multi-line error sites become _mount_fail calls, which also sends them
to stderr where they belong rather than stdout.
Behaviour is unchanged — the full matrix still passes, including the no-mount
path leaving docker run byte-for-byte identical.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Restructured in The real problem: ~70 lines of mount logic sat inline in Three things genuinely collapsed:
Behaviour is unchanged. Full matrix re-run — 17 cases including all the whitespace ones and all three container-guard branches — plus a check that the no-mount path still leaves If you want it smaller still, the remaining weight is the two friendly failures, and that's a real trade: the basename-collision guard (~10 lines) and the flag-plus- |
The problem
woltspace starthardcodes its bind mounts, so the container can only ever see the wolts directory. To let a wolt work on a repo living elsewhere on your machine, today you must either copy it under$WOLTS_DIR— wherewoltspace backupthen snapshots it, and your code mixes into wolt state — or hand-rolldocker run, which the nextwoltspace startwipes out (it doesdocker rm -fand rebuilds the args from scratch).The change
A repeatable
--mount <path>onwoltspace start, taking a source path only:…plus
WOLTSPACE_MOUNTSin$WOLTS_DIR/.envas a sticky, comma-separated equivalent, so a plainwoltspace startkeeps them:WOLTSPACE_MOUNTS=~/code/my-repo,~/notesThe
.envvalue is read with the samegrep-the-env-file patternWOLTSPACE_PUBLIC_TUNNELalready uses, so there's no new config mechanism.Targets are always
/mnt/<name>. Because the user never chooses a target, collisions with/workspace/wolts,/workspace/woltspaceand/home/node/.claudeare unreachable by construction rather than guarded against — andls /mntgives a wolt a discoverable list of everything it can reach./mntalso sits outside/workspace, so it stays clear of the entrypoint'schown -R.The name comes from
basenameon the resolved path rather than${p##*/}, which returns empty on a trailing slash and would mount at/mnt/.Validation, all before
docker runand all failing fast:/is refusedDuplicate mount point: /mnt/apinever says which two clashed. The same directory listed twice (flag and.env) is a harmless no-op, deduped rather than rejected.--mounton an existing container is now an error, not a silent no-op. Bind mounts are fixed atdocker run, andstartonly reaches_start_containerwhen no container exists — a running one short-circuits at "already gnawing", a stopped one hitsdocker restart, which carries the old spec. Sostart --mounton a live lodge previously gave no mount and no error. It now exits with:…noting that's a container recreate (seconds), not
woltspace rebuild(minutes, and pointless here). Auto-recreate is deliberately not implemented — deferred until real usage warrants it. The check is gated on the explicit flag only;WOLTSPACE_MOUNTSapplies whenever the container is created, matching how every other.envvar already behaves.Purely additive: with no flag and no env var,
mount_argsis empty and thedocker runline is byte-for-byte what it was.Testing
Target shell is macOS
/bin/bash3.2 — no associative arrays, so name/source collision tracking uses parallel indexed arrays and a linear scan. The script usesset -ebut notset -u;"${arr[@]}"and${#arr[@]}on empty arrays were verified to expand to zero words with no stray empty arg. Don't addset -uwithout auditing those.Verified with
bash -n, and functionally by extracting the arg-parsing loop,_start_container(), and thestart)branch and running them against a stubbeddockeronPATHthat records its argv (the real call is> /dev/null, so the stub writes args to a file):--mountflag-v <src>:/mnt/<name>:rwreachesdocker runWOLTSPACE_MOUNTSin.envdocker runargs unchanged--mount source not found: …/--mount refuses to mount /~/code/my-repo//mnt/my-repo, not/mnt/--mount+ container exists--mount, container existsWOLTSPACE_MOUNTS+ container existsWOLTSPACE_MOUNTS=a, b(spaces after commas).envsaved with CRLF/mnt/my notesNot covered by
test/test-cli.sh— that harness does full docker build/start cycles, and this needed argv-level assertions. Happy to add a case there if you'd rather it live in the suite.Docs
.env.example—WOLTSPACE_MOUNTSas a comma-separated list of source pathsHUMANS.md— flag table row + "Mounting your own directories": the/mnt/<name>convention, that mounts are read-write against the real checkout, and that changing them needswoltspace stop && woltspace start(recreate, not rebuild)CLAUDE.md— flag, env var, and a correction to the "the only mount is…" lineNo
.versionorCHANGELOG.mdbump — perVERSIONING.mdthose happen when a release is cut, not per PR. This does add a new optional env var, so whichever release picks it up is arguably a MINOR by that doc's rules; it needs no migration, since unset means current behavior.🤖 Generated with Claude Code