feat(setup): report an unwritable target as a structured error (task 0044) #53

alexion · 2026-07-20T13:25:32-04:00

alexion commented

2026-07-20 13:25:32 -04:00

Implements .claude/tasks/0044-setup-fails-clean-on-unwritable-targets.md.

Summary

Both halves of setup assumed the files they manage are writable.
A declaratively managed target made the skill install raise a raw filesystem exception, and the hook install surface the underlying message with no guidance about what to do.

Both now fail with a new TARGET_NOT_WRITABLE code, naming the file and pointing at the general remedy — it appears to be managed by another tool, so declare the skill or hook through that configuration instead.
The error names no particular manager, because read-only is not diagnostic of one.
A target already byte-identical to the bundled copy still succeeds, since nothing needs to be written.

Verified against the built binary as well as the fast tier: read-only file, read-only directory, unreadable file, and read-only ~/.claude/settings.json.

Deviations

Two things go slightly beyond the literal criteria, both from review:

The skill half guards its comparison read as well as its write. A target the filesystem will not let us read is one it will not let us replace either, so a mode-000 file reports the same error rather than a raw exception.
The hook half collects failures as {path, detail} records rather than the SDK's flattened <path>: <message> text. This is correctness, not shape: testing the whole formatted string for an errno would misclassify an unrelated failure whose path happened to contain EACCES.

Two known limits are documented in the task's Implementation Notes rather than fixed: the hook error names the SDK's intended target rather than necessarily the blocking path (the SDK discards the error object), and an unwritable settings.json still leaves the Codex and OpenCode integrations installed because the SDK writes them first.

Review

Risk

Overall: Low

Blast radius: Low — two source files, both additive; the only shared surface is a new AxiErrorCode member and helpers with no other callers.
Reversibility: Low — pure code, no migrations or persisted state; a clean git revert.
Test coverage: Low — all new paths covered, with a root-skip guard and permission restoration in teardown.
Sensitive domain: Low/Medium — reasons about permission errnos, but only to reclassify a failure; never widens permissions or bypasses a denial.
Size & complexity: Low — small diff, one try/catch and one find.
Runtime criticality: Low — setup is a one-shot install command, not a hot path.

Unaddressed findings

Standards — Divergent Change (judgement call). errors.ts was an HTTP-response classifier and now also holds filesystem errno predicates. Kept there because that module owns error classification and construction generally, not HTTP specifically — splitting one concept across two files to preserve the narrower reading seemed worse.

Spec — partial state on the hook path. An unwritable ~/.claude/settings.json exits 1 with the other three integrations installed. Making the install transactional across integrations the SDK owns is larger than this task, and re-running after fixing the permission converges.

All other findings were fixed and are in the diff: the flattened-string parse, the -1 slice, the errno false-positive on paths, the declare parameter name, the misattributed comment, and the stale error-code list in the spec.

Implements [`.claude/tasks/0044-setup-fails-clean-on-unwritable-targets.md`](.claude/tasks/0044-setup-fails-clean-on-unwritable-targets.md). ## Summary Both halves of `setup` assumed the files they manage are writable. A declaratively managed target made the skill install raise a raw filesystem exception, and the hook install surface the underlying message with no guidance about what to do. Both now fail with a new `TARGET_NOT_WRITABLE` code, naming the file and pointing at the general remedy — it appears to be managed by another tool, so declare the skill or hook through that configuration instead. The error names no particular manager, because read-only is not diagnostic of one. A target already byte-identical to the bundled copy still succeeds, since nothing needs to be written. Verified against the built binary as well as the fast tier: read-only file, read-only directory, unreadable file, and read-only `~/.claude/settings.json`. ## Deviations Two things go slightly beyond the literal criteria, both from review: - The skill half guards its comparison read as well as its write. A target the filesystem will not let us read is one it will not let us replace either, so a mode-`000` file reports the same error rather than a raw exception. - The hook half collects failures as `{path, detail}` records rather than the SDK's flattened `<path>: <message>` text. This is correctness, not shape: testing the whole formatted string for an errno would misclassify an unrelated failure whose *path* happened to contain `EACCES`. Two known limits are documented in the task's Implementation Notes rather than fixed: the hook error names the SDK's intended target rather than necessarily the blocking path (the SDK discards the error object), and an unwritable `settings.json` still leaves the Codex and OpenCode integrations installed because the SDK writes them first. ## Review ### Risk **Overall: Low** - Blast radius: Low — two source files, both additive; the only shared surface is a new `AxiErrorCode` member and helpers with no other callers. - Reversibility: Low — pure code, no migrations or persisted state; a clean `git revert`. - Test coverage: Low — all new paths covered, with a root-skip guard and permission restoration in teardown. - Sensitive domain: Low/Medium — reasons about permission errnos, but only to reclassify a failure; never widens permissions or bypasses a denial. - Size & complexity: Low — small diff, one try/catch and one `find`. - Runtime criticality: Low — `setup` is a one-shot install command, not a hot path. ### Unaddressed findings **Standards — Divergent Change (judgement call).** `errors.ts` was an HTTP-response classifier and now also holds filesystem errno predicates. Kept there because that module owns error classification and construction generally, not HTTP specifically — splitting one concept across two files to preserve the narrower reading seemed worse. **Spec — partial state on the hook path.** An unwritable `~/.claude/settings.json` exits 1 with the other three integrations installed. Making the install transactional across integrations the SDK owns is larger than this task, and re-running after fixing the permission converges. All other findings were fixed and are in the diff: the flattened-string parse, the `-1` slice, the errno false-positive on paths, the `declare` parameter name, the misattributed comment, and the stale error-code list in the spec.

alexion added 1 commit 2026-07-20 13:25:32 -04:00

feat(setup): report an unwritable target as a structured error (task 0044)

CI / test (22) (pull_request) Successful in 50s

Details

CI / test (true, 24) (pull_request) Successful in 1m5s

Details

CI / flake (pull_request) Successful in 3s

Details

CI / test (22) (push) Successful in 47s

Details

CI / test (true, 24) (push) Successful in 1m4s

Details

CI / flake (push) Successful in 3s

Details

a1e68dc530

Both halves of `setup` assumed the files they manage are writable. A
declaratively managed target — read-only because a configuration manager
owns it, because a file is flagged immutable, or because the path is
root-owned — made the skill install raise a raw filesystem exception and
the hook install surface the underlying message with no guidance.

Both now fail with `TARGET_NOT_WRITABLE`, naming the file and pointing at
the general remedy: it appears to be managed by another tool, so declare
the skill or hook through that configuration instead. The error names no
particular manager, because read-only is not diagnostic of one.

A target already byte-identical to the bundled copy still succeeds —
nothing needs writing, so its being read-only is beside the point.

alexion merged commit a1e68dc530 into main

2026-07-20 13:33:31 -04:00

alexion deleted branch task-0044-setup-fails-clean-on-unwritable-targets

2026-07-20 13:33:31 -04:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: alexion/gitea-axi#53