Skip to content

claude-docker: seed settings.docker.json via entrypoint copy so in-session settings writes work - #58

Merged
dtump merged 2 commits into
mainfrom
settings-seed-copy
Jul 10, 2026
Merged

dtump merged 2 commits into
mainfrom
settings-seed-copy

Conversation

@dtump

@dtump dtump commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Problem

Changing effort (or any setting) inside the container fails:

Failed to set effort level: Failed to read raw settings from /root/.claude/settings.json:
Error: EBUSY: resource busy or locked, rename '/root/.claude/settings.json.tmp.11.…' -> '/root/.claude/settings.json'

Claude Code persists settings atomically: write settings.json.tmp.*, then rename() over settings.json. Because run.sh bind-mounted settings.docker.json directly at /root/.claude/settings.json, that path is a mountpoint, and the kernel refuses to rename over a mountpoint → EBUSY. This is inherent to single-file bind mounts — dropping :ro would not help. Side effect: /model claims "saved as your default for new sessions" while the write silently failed, and an effort reset on model switch becomes impossible to undo.

Fix

Forward the host file read-only at a seed path (/run/claude-docker/settings.json, outside the persistent volumes) and have entrypoint.sh copy it to /root/.claude/settings.json at container start:

  • the copy is a regular file, so tmp-file renames succeed and in-session settings changes work;
  • changes last for that container run and are re-seeded from the host file on the next start — nothing is ever written back to the host (same intent as before, minus the breakage);
  • when no settings.docker.json exists, nothing is seeded and whatever a previous run persisted in the home volume is left alone (preserves behavior for users who never used settings.docker.json).

Ownership subtlety: on warm volumes /root/.claude and a prior copy are HOST_UID-owned, and container root has no CAP_DAC_OVERRIDE — the entrypoint takes ownership (CAP_CHOWN) before writing; the existing chown walk hands everything back to HOST_UID.

Spec

host-config-parity amended in place (no OpenSpec change dir): the requirement's intent — the container uses your dedicated settings file — is unchanged; the previously mandated mechanism (direct bind mount at the real path) was the bug. Added a scenario pinning the rename-over-settings.json behavior.

Tests

  • All smoke cells now mount a settings fixture at the seed path and assert the entrypoint copied it into place, it carries the sentinel, and it can be atomically replaced (the exact regression).
  • The RO cell doubles as the no-seed cell (--settings=0): entrypoint must start cleanly without a forwarded settings file.
  • Cells 5/7/8 (warm volume, uid-0 legacy, ephemeral) exercise the ownership paths through the new entrypoint block.

Verified locally: seed-block simulation (overwrite stale copy, 0600 perms, rename-over succeeds, absent-seed no-op) and bash -n on all touched scripts; the full container matrix runs in CI. Requires an image rebuild to pick up the new entrypoint.sh.

Note: intentionally excludes the local pins/claude-code.env bump (kept out of this PR).

…ind mount

Claude Code persists settings by renaming a tmp file over settings.json.
With settings.docker.json bind-mounted directly at /root/.claude/settings.json
that path is a mountpoint, and rename() over a mountpoint fails with EBUSY —
so every in-session settings change (effort, model, theme) errored with
'Failed to set effort level: ... EBUSY: resource busy or locked'. It also
made /model silently claim to save a new default that never landed.

Forward the host file read-only at a seed path (/run/claude-docker/
settings.json) instead, and have entrypoint.sh copy it to
/root/.claude/settings.json at container start. The copy is a regular file,
so in-session settings writes succeed; they last for that run and are
re-seeded from the host file on the next start — never written back.

The entrypoint copy needs an ownership dance on warm volumes: the dir/file
are HOST_UID-owned from a prior run's chown walk and container root has no
CAP_DAC_OVERRIDE, so take ownership (CAP_CHOWN) before writing and let the
existing chown walk hand everything back.

Spec: host-config-parity amended in place — the requirement's intent
(container uses the dedicated settings file) is unchanged; the mandated
mechanism was the bug.

Smoke: all cells now mount a settings fixture at the seed path and assert
the copy exists, carries the sentinel, and can be atomically replaced (the
exact regression); the RO cell doubles as the no-seed cell (--settings=0).

@dtump dtump left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Review

Overview — Correct fix for the right root cause, with the regression pinned in spec and smoke tests. The capability reasoning in the entrypoint holds up against run.sh's actual --cap-drop ALL --cap-add CHOWN,SETUID,SETGID,DAC_READ_SEARCH set (run.sh:401): the seed read works via DAC_READ_SEARCH, rm+cp instead of cp -f is genuinely required without DAC_OVERRIDE, and the chown root at entrypoint.sh:25 is handed back by the existing walk (entrypoint.sh:62). One test gap to close before merge.

Blockers

None.

Code quality (fix before merge)

  1. The re-seed guarantee is exercised but not asserted. The new spec scenario promises "the next container start re-seeds settings.json from the host file" (spec.md:72), implemented by the unconditional rm+cp (entrypoint.sh:26-27) — but the warm cell reruns with the same fixture content, so a naive re-implementation like [ -f settings.json ] || cp would pass every cell: pass 2 finds the sentinel left by pass 1. Fix shape: in smoke.sh (warm branch, ~line 272), overwrite ${TMPROOT}/settings.docker.json in place with a second sentinel (e.g. SMOKE-SENTINEL-SETTINGS-V2) between the cold and warm passes — truncate-in-place keeps the inode, so the bind mount sees the new content — and pass the expected sentinel via env (e.g. EXPECT_SETTINGS_SENTINEL) instead of hardcoding it at assert-in-container.sh:327. The warm pass then proves the seed overwrote the previous copy, which is the exact behavioral contract the spec added.

Follow-ups (track as issues)

  1. The .git/config overlay very likely has the same bug this PR diagnosed. run.sh:347 bind-mounts the overlay as a single-file mount (not :ro), and the comment at run.sh:324-326 claims container-side git config / git remote add "need to succeed" with writes landing in the overlay. But git writes config the same way Claude Code writes settings: create .git/config.lock, then rename(2) it over .git/config — rename over a file mountpoint → EBUSY, no fallback in git. If confirmed, the overlay needs the same seed-copy treatment (or the comment and README trade-off text need correcting). Not blocking: pre-existing, a different feature with its own spec (container-git-config-overlay), and it needs an empirical repro first.
  2. Uncovered spec path: "no seed + warm volume leaves a previously persisted settings.json as-is" (spec.md:58, acknowledged at assert-in-container.sh:310-311 — the no-seed cell is cold-only). Cheap to fold into the same smoke change as the code-quality item above, otherwise track it; the primary regression is covered.

Strengths

  • The regression test tests the mechanism, not the symptom: assert-in-container.sh:333-342 performs the actual tmp-file rename Claude Code does, so any future return to a mountpoint (or an ownership regression that breaks the rename) fails CI in nine of ten cells, including warm, uid-0, and ephemeral.
  • The ownership-dance comment (entrypoint.sh:18-22) states the non-obvious constraint (no CAP_DAC_OVERRIDE, why rm+cp) rather than narrating the code.
  • Spec amended in place with intent preserved and the broken mechanism superseded; all three doc touchpoints (README:73, run.sh help text, spec) say the same thing.
  • Reusing cell 9 for no-seed keeps the matrix at ten cells, documented at both call site (ci.yml:201-205) and assertion site.

Summary

Land the one smoke-test improvement (assert the warm pass re-seeds with fresh content), open the .git/config-overlay EBUSY issue, then ship.

…s smoke cell

Address PR #58 review: the warm cell reran with the same fixture content,
so the spec's re-seed guarantee was exercised but never asserted — a
`[ -f ] || cp` re-implementation of the entrypoint seed block would have
passed every cell on pass 1's leftovers.

- smoke.sh rewrites the fixture IN PLACE (inode preserved, so the :ro bind
  mount sees it) with a V2 sentinel between the cold and warm passes, and
  passes the expected sentinel via EXPECT_SETTINGS_SENTINEL instead of
  hardcoding it in assert-in-container.sh.
- The warm cell gains a third pass with no seed mounted
  (EXPECT_SETTINGS=keep): a previously persisted settings.json must
  survive untouched — covers the spec's "No settings file" warm-volume
  path the review flagged as untested.

Verified with a stubbed docker driver (3 passes, correct mounts/env,
fixture inode stable across the rewrite) and a scenario matrix over
check_settings (stale-V1-on-warm fails, keep/lost/leak/absent behave).
@dtump

dtump commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator Author

Review addressed in e4123bf.

Code-quality 1 (re-seed asserted, not just exercised) — done as suggested: smoke.sh rewrites the fixture in place with SMOKE-SENTINEL-SETTINGS-V2 between the cold and warm passes (truncate keeps the inode, so the :ro bind mount sees the new content), and the expected sentinel now travels via EXPECT_SETTINGS_SENTINEL instead of being hardcoded in assert-in-container.sh. A [ -f ] || cp re-implementation of the seed block now fails the warm pass on the stale V1 sentinel.

Follow-up 2 (no seed + warm volume keeps persisted settings) — folded in, since it rides the same plumbing: the warm cell gains a third pass with the seed mount dropped and EXPECT_SETTINGS=keep; the assert script checks no seed is present, the V2 copy from the previous pass survived, and it is still atomically replaceable. Cell count stays at ten; ci.yml's inventory comment updated.

Follow-up 1 (.git/config overlay EBUSY) — opened as #60 with the repro as the first step. One wrinkle noted there: unlike settings.json, the overlay target lives inside the bind-mounted workspace, so a straight seed-copy would clobber the host's real .git/config — if the repro confirms, the fix needs a different mechanism (or the comment/README trade-off text gets corrected instead).

Verification (no docker on this machine; the full matrix runs in CI): drove smoke.sh --volstate=warm against a stubbed docker — three invocations with the right mounts/env per pass and a stable fixture inode across the rewrite — and ran check_settings standalone against a scenario matrix (seeded-ok, stale-V1-on-warm → fails, keep-ok, keep-lost → fails, seed-leak-on-keep → fails, absent-ok/absent-but-seeded, sentinel-unset → fails). bash -n clean on both scripts.

@dtump dtump left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Re-reviewed after e4123bf — all findings resolved, no new ones. Ready to merge.

  • Re-seed assertion (code quality): fixed as prescribed. The warm cell now rewrites the fixture in place with a V2 sentinel between passes (inode preserved, so the :ro bind mount sees it) and asserts the warm pass carries V2 — a [ -f ] || cp re-implementation of the seed block can no longer pass on pass 1's leftovers. Sentinel plumbing via EXPECT_SETTINGS_SENTINEL is fail-closed (unset → explicit FAIL).
  • No-seed warm-volume path (follow-up 2): covered beyond what was asked — a third EXPECT_SETTINGS=keep pass asserts the seed is absent at /run and the persisted settings.json survives untouched and stays atomically replaceable. Sound because /run is container-scoped while /root/.claude is the named volume, so the pass genuinely exercises the entrypoint's skip branch against persisted state.
  • .git/config overlay EBUSY (follow-up 1): tracked as #60.

Also checked: per-pass env injection has no duplicate -e flags, the empty-array guard matches the bash-3.2-safe idiom used elsewhere (macOS checklist unaffected), the stderr-WARN caveat update is correct (chown walk never touches /run), and ci.yml's cell inventory matches the new three-pass cell 5. CI green on this head, including the full smoke matrix.

@dtump
dtump merged commit f77f6e4 into main Jul 10, 2026
3 checks passed
@dtump
dtump deleted the settings-seed-copy branch July 10, 2026 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants