Skip an unwritable target instead of aborting the run - #9
Conversation
Two defects, both surfaced by pointing Codex at its real skills directory. `is_nix_managed` used a single `read_link`, so it saw only the first hop of a symlink chain. Codex's skills root here is `~/.codex/skills` -> `~/.agents/skills` -> `~/.local/state/construct/current` -> `/nix/store/...`; the first hop is an ordinary path, so engram reported the directory as writable right up until the write failed. It canonicalizes now. The write failure was worse than the missing warning: the `EROFS` propagated out of `install`, so a full run aborted at Codex and every harness after it was never reached. A read-only directory is a fact about the machine, not an engram error — a declaratively-managed skills root is *supposed* to be immutable — so it is now reported per file, counted as skipped, and the run continues. Verified against the real machine: `install` reports 3 unwritable Codex files with the Nix warning attached, and still installs Claude Code, OpenClaude, Opencode, and Antigravity. Gates: fmt, clippy -D warnings, 256 tests, REUSE 3.3, makeinfo clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016i16R4GhdSffsboRYq97Fs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f61e1e1deb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| match managed_file::write_managed(&path, &body, WritePolicy::Owned, dry_run) { | ||
| Ok(w) => w, | ||
| Err(e) if is_unwritable(&e) => { |
There was a problem hiding this comment.
Route plugin manifests through the unwritable skip path
When Antigravity's plugin directory is read-only, this handler skips the three SKILL.md writes, but the subsequent plugin.json write still uses a bare ? and propagates the same PermissionDenied/ReadOnlyFilesystem error. Consequently, this plugin-backed target still makes install fail instead of returning the per-file results promised by this change; apply the same recovery path to the manifest write.
Useful? React with 👍 / 👎.
Two defects, both surfaced immediately by pointing Codex at its real skills directory in #8.
1.
is_nix_managedonly saw the first hopIt used a single
read_link. Codex's skills root here is a chain:The first hop is an ordinary path, so engram reported the directory as writable — and emitted no Nix warning — right up until the write failed. It canonicalizes now.
2. The write failure aborted everything
Worse than the missing warning: the
EROFSpropagated out ofinstall, so a full run stopped at Codex and every harness after it was never reached.A read-only directory is a fact about the machine, not an engram error — a declaratively-managed skills root is supposed to be immutable. It's now reported per file, counted as
skipped, and the run continues.Verified on the real machine
Before this change, that same command exited with:
{"error":{"code":"INTERNAL_ERROR","message":"failed to install commands: Read-only file system (os error 30)"}}The new test makes a skills root
0o555and asserts both halves: the reason is reported, and Claude Code is still installed afterwards.Gates
fmt·clippy -D warnings· 256 tests · REUSE 3.3 ·makeinfoclean🤖 Generated with Claude Code
https://claude.ai/code/session_016i16R4GhdSffsboRYq97Fs