Skip to content

fix(claude-mem,claude-ollama): enumerate chown paths under ~/.claude - #276

Merged
mdelapenya merged 8 commits into
mainfrom
fix-readonly-skills-chown-mem-ollama
Sep 9, 2026
Merged

fix(claude-mem,claude-ollama): enumerate chown paths under ~/.claude#276
mdelapenya merged 8 commits into
mainfrom
fix-readonly-skills-chown-mem-ollama

Conversation

@mdelapenya

Copy link
Copy Markdown
Member

Same root cause as the claude-sbx-statusline fix in #275: the shared skills directory under ~/.claude is mounted read-only by default, and a recursive chown over the parent recurses into it and fails.

  • claude-ollama: the install step's chown -R hard-fails sandbox create.
  • claude-mem: the startup trap's chown -R fails silently, leaving root-owned files under ~/.claude.
  • Both now chown exactly the paths their root steps create. SPEC-v2.md's ownership example taught the recursive idiom and is updated to the enumerated one.

Validation:

  • TCK passes for claude-ollama (the new chown line executes in-container).
  • claude-mem's TCK fails on a pre-existing upstream installer regression, unrelated and reproduced on main: docker/sandboxes#5714.
  • The TCK cannot reproduce the read-only skills mount; that behavior is only verifiable e2e against a current engine.

Note: this touches spec/, which expands the e2e matrix to all kits — the ecc nightly leg is expected red (docker/sandboxes#5539).

@mdelapenya
mdelapenya requested a review from a team as a code owner September 7, 2026 10:57
@mdelapenya
mdelapenya force-pushed the fix-readonly-skills-chown-mem-ollama branch from 9b285e7 to 60cfd96 Compare September 7, 2026 11:49
Comment thread claude-mem/spec.yaml Outdated
dvdksn
dvdksn previously approved these changes Sep 8, 2026
mdelapenya and others added 5 commits September 9, 2026 08:50
The shared skills directory under ~/.claude is mounted read-only by
default; chown -R agent:agent /home/agent/.claude recurses into it and
fails — a hard create failure in claude-ollama's install step, and
silent root-owned leftovers from claude-mem's startup trap. Chown
exactly the paths each root step creates instead, and update the
SPEC-v2.md ownership example, which taught the recursive idiom.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Manuel de la Peña <manuel.delapena@docker.com>
… helper

The helper only defused chown -R agent:agent, so a kit using the
enumerated chown agent:agent <paths> form runs it live in the test —
a silent no-op when the test user is agent, a set -e abort in CI where
it is not.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Manuel de la Peña <manuel.delapena@docker.com>
The enumerated chown named settings.json unconditionally, so when the script exited before that file existed the trap's chown failed on a missing path. Under `set -e` a failing command inside an EXIT trap replaces the script's exit status, which turned a tolerated early failure into a failed install command, and so a failed sandbox creation. The recursive form it replaced did not have this problem: it simply found nothing to chown.

Each enumerated path is now existence-checked, so absence is tolerated while a genuine chown failure still surfaces. Verified under dash, which is what runs these commands.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Manuel de la Peña <manuel.delapena@docker.com>
…t mode

The guidance justified enumerating paths by asserting that a recursive chown over the parent fails because the shared skills directory is mounted read-only there. That couples normative guidance to one particular runtime mechanism, which is in flux. The durable reason is ownership scope: ~/.claude holds runtime-managed content a kit does not own, so recursing over the parent claims paths outside the kit's control.

The advice is unchanged; only its justification is, so the text now holds regardless of how the runtime exposes the store.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Manuel de la Peña <manuel.delapena@docker.com>
The comment credited a trailing space that is not in either pattern. What actually prevents double-substitution is that "chown -R agent:agent" does not contain "chown agent:agent" as a substring, because "-R " sits between them.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Manuel de la Peña <manuel.delapena@docker.com>
@mdelapenya
mdelapenya force-pushed the fix-readonly-skills-chown-mem-ollama branch from cd9cb04 to 24c3d18 Compare September 9, 2026 08:50
@mdelapenya
mdelapenya merged commit 96a0e00 into main Sep 9, 2026
207 of 209 checks passed
@mdelapenya
mdelapenya deleted the fix-readonly-skills-chown-mem-ollama branch September 9, 2026 15:54
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.

3 participants