feat: share Claude and Codex skills in the container feature - #588
Merged
Conversation
Reviewer's GuideThe PR updates the opt-in Claude container feature to expose Codex and Claude shared skill roots as read-only directory mounts, initializes only missing host roots, and preserves existing skills and links. Documentation and mount-derived tests now cover cross-home symlink layouts, host file replacement, write refusal through every discovery path, writable credential refresh, and the feature's non-goals. Sequence diagram for initializing protected skill mountssequenceDiagram
participant Host as Host filesystem
participant Init as init-host.sh
participant Feature as Claude container feature
participant Container as Container
Init->>Host: mkdir -p missing ~/.claude/shared-skills
Init->>Host: mkdir -p missing ~/.agents/skills
Init->>Feature: Provide existing mount roots
Feature->>Container: Mount ~/.claude/shared-skills readonly
Feature->>Container: Mount ~/.agents/skills readonly
Container-->>Feature: Preserve existing skills and links
Sequence diagram for read-only shared skill accesssequenceDiagram
participant Agent as Claude or Codex
participant Symlink as Discovery symlink
participant Mount as Read-only skill mount
participant Host as Host skill body
Agent->>Symlink: Read linked SKILL.md
Symlink->>Mount: Resolve relative target
Mount->>Host: Read current skill body
Host-->>Agent: Return skill content
Agent->>Symlink: Write linked SKILL.md
Mount-->>Agent: Refuse write with EROFS
Sequence diagram for writable Claude credential refreshsequenceDiagram
participant Claude
participant Credentials as ~/.claude/.credentials.json
participant Host
Claude->>Credentials: Refresh OAuth credentials
Credentials->>Host: Write updated credentials
Host-->>Claude: Persist refreshed credentials
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path=".devcontainer/claude-code/README.md" line_range="78-80" />
<code_context>
+still gets its skills from that repo's configuration or dotfiles setup.
+
+Dotfiles installers must skip the mounted `~/.agents/skills` root, just as they
+skip host-mounted Claude configuration. Detect ownership from the mount table;
+a container profile alone cannot tell a local directory from a host bind.
### Read-Write Mounts (Authentication & State)
- `~/.claude/.credentials.json` → OAuth access/refresh tokens
</code_context>
<issue_to_address>
**issue (broader_impact):** The documentation requires dotfiles installers to detect and skip the host-mounted `~/.agents/skills` root, but the diff adds no installer or mount-ownership handling for that path. A dotfiles refresh therefore still treats the read-only host bind as container-owned state and attempts to reconcile it, causing failures on the read-only mount or violating the promise to preserve existing skills and links.
**Triggers:** When a workspace using this feature runs its dotfiles installer or refreshes dotfiles.
**Suggested fix:** Update the dotfiles provisioning path to detect the `~/.agents/skills` mount in `/proc/self/mountinfo` and exclude that root, including its descendants, from reconciliation.
</issue_to_address>
Comment on lines
+78
to
80
| skip host-mounted Claude configuration. Detect ownership from the mount table; | ||
| a container profile alone cannot tell a local directory from a host bind. | ||
|
|
There was a problem hiding this comment.
issue (broader_impact): The documentation requires dotfiles installers to detect and skip the host-mounted ~/.agents/skills root, but the diff adds no installer or mount-ownership handling for that path. A dotfiles refresh therefore still treats the read-only host bind as container-owned state and attempts to reconcile it, causing failures on the read-only mount or violating the promise to preserve existing skills and links.
Triggers: When a workspace using this feature runs its dotfiles installer or refreshes dotfiles.
Suggested fix: Update the dotfiles provisioning path to detect the ~/.agents/skills mount in /proc/self/mountinfo and exclude that root, including its descendants, from reconciliation.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…every mount test
Widening resolve() from ~/.claude to the whole host home, so ~/.agents/skills
could be shared, also removed the only thing that refused a mount source
outside ~/.claude. The read-only agreement test inspects mounts carrying
`readonly` and nothing inspects the rest, so adding
`source=${localEnv:HOME}/.codex,target=/home/vscode/.codex,type=bind` with a
matching mkdir in the hook leaves all 53 tests green while shipping a writable
bind of a host directory the README never mentions.
Measured that way round before the guard was written, and the guard reports
['.codex'] on it.
Claude-Session: https://claude.ai/code/session_01HWRhzmUMVpUo5RB6ZNScFH
Splitting documented_paths into a home-relative accessor and a `.claude/` subset left two functions returning the same set of the same str, told apart only by their names, and each call site joined one of them against a base the type could not carry: the e2e test imported both and joined them against `home` and `home/.claude` in the same function. The subset also filtered without asserting the filter was empty, so documented_paths(READ_ONLY_HEADING) returned six of the README's seven bullets and quietly discarded `.agents/skills/`. mounted_files() consumes it and promises "a file mount added to the feature is held to healing without anyone remembering to extend a list here" -- untrue after the split for anything outside ~/.claude, which is the half of the home the feature had just started mounting. One accessor now, home-relative, with the base joined where it is used. mounted_files() collects the same three paths as before, verified by collecting its parametrization. Claude-Session: https://claude.ai/code/session_01HWRhzmUMVpUo5RB6ZNScFH
…home
removeprefix returns its input unchanged when the prefix is absent, and Path
discards its left side when the right is absolute, so the two composed into a
silent escape. Measured: with CLAUDE_CONFIG_DIR at /home/vscode/.config/claude,
removesuffix("/.claude") no-ops, every mount target falls through unchanged,
and `scratch / "/home/vscode/.claude/skills"` is /home/vscode/.claude/skills.
The scenario mkdir -p's and bind-mounts what that expression returns -- the
mkdir outside the namespace, under sudo -- and still passes, because the binds
satisfy every read and every EROFS probe it makes.
Both joins now assert the prefix they strip. The same manifest edit fails with
"CLAUDE_CONFIG_DIR /home/vscode/.config/claude does not end in /.claude"
instead, and a target outside the container home has a test of its own.
Claude-Session: https://claude.ai/code/session_01HWRhzmUMVpUo5RB6ZNScFH
Every protection assertion in the suite is that membership test, and this diff added three more call sites of it, including the one asserting the container cannot rewrite the host's shared skills. Measured on parse_mount as it stood: ...,type=bind,readonly=false -> "readonly" in mount is True ...,type=bind,ro -> "readonly" in mount is False So a writable mount spelled `readonly=false` is counted as protected -- a documented protection with nothing behind it, which is the failure test_claude_code_feature_mounts.py exists to catch -- and a genuinely read-only mount spelled with docker's own `ro` synonym is counted as writable. Both silent. parse_mount refuses all three valued spellings now, `readonly=true` included, so the question of whether the value is consulted cannot arise. Confirmed red on each of the three before the assertions were added. Claude-Session: https://claude.ai/code/session_01HWRhzmUMVpUo5RB6ZNScFH
Four hand-written copies of the mount list, and only the README's Read-Only
Mounts bullets had a test diffing them against the manifest, so the rest could
drift and one had: the `bind mount source path does not exist` section still
told a blocked developer to create three directories of seven, which leaves
devpod refusing the create for the four it did not name. It also told them to
run `echo '{}' > ~/.claude/settings.json`, which truncates the settings file of
anyone who already had one, to satisfy a bind that has not existed since the
feature stopped mounting files one at a time.
The new guard holds all four copies -- both TROUBLESHOOTING.md lists and both
by-hand mkdirs -- to set equality with the manifest's read-only sources. It
reports the three that agreed and the one that did not:
the README's '### `bind mount source path does not exist`' mkdir disagrees
with the manifest: ['.agents/skills', '.claude/shared-skills',
'.claude/skills', '.claude/wf-skills'] appears in one and not the other
Claude-Session: https://claude.ai/code/session_01HWRhzmUMVpUo5RB6ZNScFH
The body became `host_config.parent / ...`, so what the parameter wanted was the host home passed as its `.claude` child, and one caller built `tmp_path / CONFIG_DIRNAME` for no reason but to be walked back up. A caller passing an actual home was well-typed and wrong. The fixture and the parameter are the home now. Restored with it is the record the rewrite dropped: the prefix assert is not tidiness, it is there because a mount of a sibling whose name merely started with the prefix used to resolve to the configuration directory and be checked in its place, passing what the real source would have failed. Claude-Session: https://claude.ai/code/session_01HWRhzmUMVpUo5RB6ZNScFH
The mounts test's own module docstring still named the five configuration subdirectories as the whole read-only list, in the file whose job is holding that list to the manifest. The README paragraph the shared-skills section replaced carried an operational cost that has no other home: a read-only skills mount means an installer that heals its own links cannot, and `wf` run inside a container says so on every launch and continues. Dropping it left the cost undocumented while the mount it belongs to grew. Claude-Session: https://claude.ai/code/session_01HWRhzmUMVpUo5RB6ZNScFH
Collapsing the two accessors dropped the `f".claude/{path}"` re-prefix that had
been keeping these strings relative by accident. mounted_files() now feeds them
straight into `scratch_home / relative`, and Path discards its left side for an
absolute right -- the same escape 5291f90 asserted against one file over.
Measured with the README's `.claude.json` bullet spelled `~//.claude/...`:
mounted_files() -> ['.claude/.credentials.json', '/.claude/.claude.json', ...]
scratch / '/.claude/.claude.json' -> /.claude/.claude.json
unshare -m isolates mounts, not file contents, so the scenario's write-back
would land on the real path. The assertion sits in the accessor rather than in
mounted_files() because every consumer joins these onto a home.
Claude-Session: https://claude.ai/code/session_01HWRhzmUMVpUo5RB6ZNScFH
Every rule in the mounts test is set equality or a dict lookup keyed on the
mount source, so a second mount of an already-protected source is absorbed by
all of them at once: the set already contains it, the dict keeps whichever
entry came last. Docker creates both. Measured -- this line added to the
manifest leaves all 59 tests green:
source=${localEnv:HOME}/.agents/skills,target=/home/vscode/.agents/skills-rw,type=bind
That is write access to the host's shared skill bodies from inside the
container, at a second path, which is the protection this branch exists to add.
The guard I added in 1fb7138 was set-shaped too and absorbed it the same way.
Two rules that iterate instead: every mount of a documented read-only source
carries `readonly`, and no source is mounted twice. The second is what makes
the set- and dict-shaped assertions here sound rather than accidentally sound,
including nested_sources, which keys by source and had the same hole.
Claude-Session: https://claude.ai/code/session_01HWRhzmUMVpUo5RB6ZNScFH
The pattern required a trailing `(read-only mount)` comment, which only TROUBLESHOOTING.md's tree carries. The README draws the same seven paths under `### Host Machine` with comments that say `# Custom agents`, so that copy was read as empty and compared against nothing -- while the docstring told a reader "the trees" were covered. Renaming an entry there to `~/.somewhere-else/` left the suite green. The pattern no longer keys on the annotation, and both trees go through one helper that drops the `~/.claude/` root they hang from, since that bind is the writable one and is asserted elsewhere. The same edit now reports: the README's '### Host Machine' tree disagrees with the manifest: ['.agents/skills', '.somewhere-else'] appears in one and not the other Claude-Session: https://claude.ai/code/session_01HWRhzmUMVpUo5RB6ZNScFH
…e files `### What's Protected (Read-Only)` listed `CLAUDE.md` and `settings.json`, both of which are writable from the container and reach the host -- and a write to `settings.json` is host command execution, because it can name a hook command inline. It also listed three of the seven protected directories. The README's equivalent section was corrected in 9580267; this one is the copy that was missed, and it is the worse of the two to get wrong, because it is the page a developer opens when a write has just been refused. Issue 5's symptom bullet had the same inversion three lines above the sentence correcting it: it offered `~/.claude/CLAUDE.md` as the path that gives "Read-only file system", which is the one thing it cannot give. `test_no_heading_claims_protection_for_a_writable_file` is what should have caught both. It existed and was scoped to the README, so it is parametrized over both documents now, and reports all three offences on the text as it stood. Claude-Session: https://claude.ai/code/session_01HWRhzmUMVpUo5RB6ZNScFH
a9edcb0 restored the paragraph about `~/.claudeX` collapsing to a zero-length path inside the configuration directory. That collapse needed the old `${localEnv:HOME}/.claude/` prefix; this branch resolves against the host home, so `~/.claudeX` lands at `home/.claudeX` with or without the separator -- measured both ways. The comment was true of the code it was written for and false of the code it was pasted into, which is worse than no comment. It now says what the assert does protect, and points at the guard that answers the question it used to be confused with. Claude-Session: https://claude.ai/code/session_01HWRhzmUMVpUo5RB6ZNScFH
This was referenced Sep 9, 2026
Merged
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
1 participant
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.
Containers using the local
claude-codefeature could not discover host skills stored in~/.agents/skills. A Claude skill linked into~/.claude/shared-skillswas visible, but its body remained writable through the parent mount.Mount both directories read-only and create missing host mount roots without changing existing skills or links. Tests cover both shared-storage layouts, different host/container home paths, host file replacement, and write refusal through each agent's discovery links. Claude credential refresh remains writable.
This applies to the opt-in container feature. Existing containers need recreation; other workspaces keep their dotfiles provisioning. Dotfiles installers must skip host-mounted skill roots. The feature does not install Codex or mount its credentials/session state.
Validation:
Summary by Sourcery
Share host skills between Claude and Codex in the opt-in container feature while preserving existing skill configuration and protecting mounted skill content from container writes.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: