docs: record that the session-start hook stores an absolute path (task 0042) #51

Merged
alexion merged 2 commits from task-0042-verify-hook-path-resolution into main 2026-07-20 12:43:35 -04:00
Owner

Tasks: 0042-verify-hook-path-resolution.md (implemented) · 0043 · 0044 · 0045 (planned)

Summary

Resolves the nix-flake-packaging spec's one open verification item — whether the session-start hook records the entrypoint's absolute path or the bare binary name — by observation against the installed SDK, then breaks the follow-on work into tasks.

The answer is the unfavourable one. resolvePortableHookCommand returns the bare name only when a PATH entry realpath-matches the entrypoint. npm symlinks its bin entry straight at dist/main.js, so that match succeeds; Nix installs a generated wrapper script invoking node <path>, whose realpath is the wrapper, so the match cannot succeed and the absolute store path is recorded.

Verified twice: by probing the SDK with both install shapes, and by building the flake and driving the installed binary against a temporary HOME, which wrote a /nix/store/... path into ~/.claude/settings.json.

Changes in this PR:

  • src/commands/setup.ts — the help text says to re-run setup hooks after an upgrade.
  • .claude/spec/nix-flake-packaging.md — the open verification item becomes a resolved finding, with the mechanism recorded.
  • CLAUDE.md — a Gotchas entry, since the belief that binaryNames makes the hook portable was wrong.
  • package.nix — comment only; its forward reference to this task is corrected.
  • .claude/tasks/00430045 — the follow-on breakdown described below.
  • .claude/CONTEXT.md — four new terms, and amendments to setup and SessionStart hook.

No behaviour changes. Typecheck and the fast tier (410 tests) pass.

The mitigation's framing turned out to be too narrow

Grilling the result found a larger defect underneath the one this task was written for.

The maintainer's agent configuration is generated declaratively, so ~/.claude/settings.json and the installed Agent Skill are both read-only symlinks into the Nix store. setup hooks cannot write at all; setup crashes on an unhandled filesystem error. The Skill has been hand-copied into the operator's own configuration and has already drifted — 48 lines there against the package's 39.

So the recorded path's shape is one defect, and the deeper one is that setup is write-only against a target some operators cannot let it write. Prior art draws the line in the same place: zoxide and atuin record a bare name into user-owned config, while home-manager persists absolute store paths precisely because it regenerates the file. gitea-axi writes user-owned state and should follow the first convention — and should also offer the second path for operators who generate theirs.

Three tasks follow:

  • 0043 — record the bare binary name, resolved through PATH, so the hook survives an upgrade on any wrapper-based install. Fixes the isManagedHook marker-substring coupling with it, and drops package.nix's postUnpack rename.
  • 0044 — report an unwritable target as a structured error that names no cause, rather than crashing.
  • 0045 — the declarative install path: a stable Skill location, Skill and hook specification exposed as Nix-consumable attributes, one committed hook specification read by both the Nix expression and the test suite, and a home-manager module wiring them. Blocked by 0043, whose bare name that specification declares.

The module follows the conventions measured across all 445 home-manager programs/* modules: mkEnableOption gate so import is inert (91%), overridable package option (87%), module installs the package by default (79%), nullable = true as the documented opt-out (51%).

Deviations

package.nix's postUnpack rename was kept, not deleted. Task 0042's evidence section says to delete it "as part of this task", which conflicts with its own acceptance criterion 4. The conflict resolves on that section's wording: the deletion is conditioned on "once the hook no longer depends on the entrypoint path", which criterion 4 puts out of scope. Deleting it now would leave the build tree at a path with no gitea-axi segment — which isManagedHook still requires — and test/setup.test.ts would fail inside checkPhase. Its comment was rewritten instead. Task 0043 removes it.

The re-run-after-upgrade help text stays, knowing 0043 removes it. It is accurate until then, and 0043 carries its removal as an acceptance criterion. Reverting it pre-emptively would leave the branch documenting a silent failure it does nothing about.

One sentence beyond the strict ask. The help text asserted "Both are idempotent" unqualified, which the finding recorded in this same commit makes false for the duplicate-append case. It is now caveated rather than left contradicting the spec.

The spec is left contradicting itself, deliberately. nix-flake-packaging.md still lists a home-manager module under Out of Scope. Revising that is task 0045's job — it is a design decision reversal that belongs with an ADR and the work, not a task-writing side effect.

Review

Risk

Overall: LOW

  • Blast radius: Low — documentation and task files; the only code change is help text.
  • Reversibility: Low — pure text edits, no schema, migration, or published API change.
  • Test coverage: Low — no behavioural change to test; typecheck and the fast tier pass.
  • Sensitive domain: Low — no auth, permissions, concurrency, or data handling touched.
  • Size & complexity: Low — no control flow.
  • Runtime criticality: Low — the single code hunk is a help string, off any hot path.

Unaddressed findings

Standards — possible Duplicated Code / Shotgun Surgery (judgement call). The wrapper-vs-symlink realpath mechanism is stated in the spec, in CLAUDE.md, and in package.nix's comment, so a future correction touches several files. Not addressed: the three are deliberately different audiences (design record, agent gotcha, inline rationale), and collapsing them into cross-references would make each less useful at its own site.

Tasks: [`0042-verify-hook-path-resolution.md`](.claude/tasks/0042-verify-hook-path-resolution.md) (implemented) · [`0043`](.claude/tasks/0043-hook-records-bare-binary-name.md) · [`0044`](.claude/tasks/0044-setup-fails-clean-on-unwritable-targets.md) · [`0045`](.claude/tasks/0045-declarative-nix-outputs-and-hm-module.md) (planned) ## Summary Resolves the `nix-flake-packaging` spec's one open verification item — whether the session-start hook records the entrypoint's absolute path or the bare binary name — by observation against the installed SDK, then breaks the follow-on work into tasks. The answer is the unfavourable one. `resolvePortableHookCommand` returns the bare name only when a `PATH` entry *realpath-matches* the entrypoint. npm symlinks its `bin` entry straight at `dist/main.js`, so that match succeeds; Nix installs a generated wrapper script invoking `node <path>`, whose realpath is the wrapper, so the match cannot succeed and the absolute store path is recorded. Verified twice: by probing the SDK with both install shapes, and by building the flake and driving the installed binary against a temporary `HOME`, which wrote a `/nix/store/...` path into `~/.claude/settings.json`. Changes in this PR: - `src/commands/setup.ts` — the help text says to re-run `setup hooks` after an upgrade. - `.claude/spec/nix-flake-packaging.md` — the open verification item becomes a resolved finding, with the mechanism recorded. - `CLAUDE.md` — a Gotchas entry, since the belief that `binaryNames` makes the hook portable was wrong. - `package.nix` — comment only; its forward reference to this task is corrected. - `.claude/tasks/0043`–`0045` — the follow-on breakdown described below. - `.claude/CONTEXT.md` — four new terms, and amendments to `setup` and `SessionStart hook`. No behaviour changes. Typecheck and the fast tier (410 tests) pass. ## The mitigation's framing turned out to be too narrow Grilling the result found a larger defect underneath the one this task was written for. The maintainer's agent configuration is generated declaratively, so `~/.claude/settings.json` and the installed Agent Skill are both **read-only symlinks into the Nix store**. `setup hooks` cannot write at all; `setup` crashes on an unhandled filesystem error. The Skill has been hand-copied into the operator's own configuration and has **already drifted** — 48 lines there against the package's 39. So the recorded path's *shape* is one defect, and the deeper one is that `setup` is write-only against a target some operators cannot let it write. Prior art draws the line in the same place: zoxide and atuin record a bare name into user-owned config, while home-manager persists absolute store paths precisely because it regenerates the file. gitea-axi writes user-owned state and should follow the first convention — and should also offer the second path for operators who generate theirs. Three tasks follow: - **0043** — record the bare binary name, resolved through `PATH`, so the hook survives an upgrade on any wrapper-based install. Fixes the `isManagedHook` marker-substring coupling with it, and drops `package.nix`'s `postUnpack` rename. - **0044** — report an unwritable target as a structured error that names no cause, rather than crashing. - **0045** — the declarative install path: a stable Skill location, Skill and hook specification exposed as Nix-consumable attributes, one committed hook specification read by both the Nix expression and the test suite, and a home-manager module wiring them. Blocked by 0043, whose bare name that specification declares. The module follows the conventions measured across all 445 home-manager `programs/*` modules: `mkEnableOption` gate so import is inert (91%), overridable `package` option (87%), module installs the package by default (79%), `nullable = true` as the documented opt-out (51%). ## Deviations **`package.nix`'s `postUnpack` rename was kept, not deleted.** Task 0042's evidence section says to delete it "as part of this task", which conflicts with its own acceptance criterion 4. The conflict resolves on that section's wording: the deletion is conditioned on "once the hook no longer depends on the entrypoint path", which criterion 4 puts out of scope. Deleting it now would leave the build tree at a path with no `gitea-axi` segment — which `isManagedHook` still requires — and `test/setup.test.ts` would fail inside `checkPhase`. Its comment was rewritten instead. Task 0043 removes it. **The re-run-after-upgrade help text stays, knowing 0043 removes it.** It is accurate until then, and 0043 carries its removal as an acceptance criterion. Reverting it pre-emptively would leave the branch documenting a silent failure it does nothing about. **One sentence beyond the strict ask.** The help text asserted "Both are idempotent" unqualified, which the finding recorded in this same commit makes false for the duplicate-append case. It is now caveated rather than left contradicting the spec. **The spec is left contradicting itself, deliberately.** `nix-flake-packaging.md` still lists a home-manager module under *Out of Scope*. Revising that is task 0045's job — it is a design decision reversal that belongs with an ADR and the work, not a task-writing side effect. ## Review ### Risk **Overall: LOW** - Blast radius: Low — documentation and task files; the only code change is help text. - Reversibility: Low — pure text edits, no schema, migration, or published API change. - Test coverage: Low — no behavioural change to test; typecheck and the fast tier pass. - Sensitive domain: Low — no auth, permissions, concurrency, or data handling touched. - Size & complexity: Low — no control flow. - Runtime criticality: Low — the single code hunk is a help string, off any hot path. ### Unaddressed findings **Standards — possible Duplicated Code / Shotgun Surgery (judgement call).** The wrapper-vs-symlink realpath mechanism is stated in the spec, in `CLAUDE.md`, and in `package.nix`'s comment, so a future correction touches several files. Not addressed: the three are deliberately different audiences (design record, agent gotcha, inline rationale), and collapsing them into cross-references would make each less useful at its own site.
alexion added 1 commit 2026-07-20 10:04:47 -04:00
docs: record that the session-start hook stores an absolute path (task 0042)
All checks were successful
CI / test (22) (pull_request) Successful in 46s
CI / test (true, 24) (pull_request) Successful in 1m6s
CI / flake (pull_request) Successful in 3s
397da5d7a6
Resolve the nix-flake-packaging spec's open verification item by observation
rather than inference, and act on the unfavourable answer.

`resolvePortableHookCommand` in axi-sdk-js returns the bare binary name only
when a PATH entry realpath-matches the entrypoint. npm symlinks its bin entry
straight at dist/main.js and satisfies that; Nix installs a generated wrapper
script whose realpath is the wrapper, so the absolute store path is recorded
instead. Verified by probing the SDK with both install shapes and by driving
the flake-built binary against a temporary HOME.

The path is content-addressed, so it moves on every rebuild and is eventually
collected, and a session-start hook that cannot execute fails silently. The
mitigation is documentation, per the decision recorded when the item was
opened: the setup help text now says to re-run `setup hooks` after an upgrade.

How the setup command constructs the hook is deliberately unchanged. Preferring
the bare name is the right answer for every wrapper-based install, not a Nix
special case, so it belongs in a successor task with its own ADR alongside the
related `isManagedHook` substring defect. package.nix's postUnpack rename
therefore stays; its comment no longer promises this task will remove it.
alexion added 1 commit 2026-07-20 12:29:49 -04:00
docs: break the declarative install path into tasks (tasks 0043-0045)
All checks were successful
CI / test (22) (pull_request) Successful in 48s
CI / test (true, 24) (pull_request) Successful in 1m4s
CI / flake (pull_request) Successful in 3s
CI / test (22) (push) Successful in 55s
CI / test (true, 24) (push) Successful in 1m5s
CI / flake (push) Successful in 2s
4e92dde4e4
Grilling task 0042's mitigation found its framing too narrow. Recording an
absolute entrypoint path is one defect; the deeper one is that `setup` is
write-only, and an operator whose agent configuration is generated
declaratively cannot let it write at all. On such a machine both halves fail —
`setup hooks` against a read-only settings file, and `setup` on an unhandled
filesystem error — and the Agent Skill gets hand-copied into the operator's own
configuration, where it silently drifts from the package that ships it.

Three tasks follow:

- 0043 records the bare binary name, resolved through PATH, so the hook
  survives an upgrade on any wrapper-based install. Fixes the marker-substring
  coupling with it and drops the derivation's build-tree rename.
- 0044 reports an unwritable target as a structured error naming no cause,
  rather than crashing.
- 0045 adds the declarative install path: a stable Skill location, the Skill
  and hook specification exposed as Nix-consumable attributes, one committed
  hook specification read by both the expression and the test suite, and a
  home-manager module wiring them. Blocked by 0043, whose bare name the
  specification declares.

CONTEXT.md gains the four terms this settled and amends `setup` and
`SessionStart hook`, which described the imperative path as the only one.
Entries for unbuilt work name the task that lands them, so the glossary does
not assert behaviour the code lacks.

The re-run-after-upgrade help text this branch added stays as it is: accurate
until 0043 removes it, which that task carries as a criterion.
alexion merged commit 4e92dde4e4 into main 2026-07-20 12:43:35 -04:00
alexion deleted branch task-0042-verify-hook-path-resolution 2026-07-20 12:43:35 -04:00
Sign in to join this conversation.
No Reviewers
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: alexion/gitea-axi#51