fix(setup): record the session-start hook as a search-path name (task 0043) #52

Merged
alexion merged 1 commits from task-0043-hook-records-bare-binary-name into main 2026-07-20 13:16:08 -04:00
Owner

Task: .claude/tasks/0043-hook-records-bare-binary-name.md

Summary

The SessionStart hook recorded the entrypoint's absolute path on every wrapper-based install, so it broke on upgrade — silently, since a hook that cannot execute simply does not run.

setup hooks now resolves gitea-axi on PATH itself and hands that location to the SDK, whose realpath test then succeeds and records the bare, upgrade-stable name.
A candidate qualifies only if it resolves to the running entrypoint: by realpath for a symlink (npm's shape), or by naming the entrypoint in its text for a generated wrapper, following the chain — a Nix install turns out to be two hops, bin/gitea-axibin/.gitea-axi-wrappeddist/main.js.
A same-named binary that is some other program is refused, and the absolute entrypoint path stands as the fallback exactly as before.

The related defect on the same line goes with it.
The SDK recognises its own hook by finding the marker inside the recorded command, so an entrypoint path without gitea-axi in it made a re-run append a duplicate instead of updating in place.
setup hooks now prunes duplicates by matching the exact command it records — independent of that command's shape, and unable to mistake another tool's hook for its own.

Consequently package.nix no longer renames its build tree, and the setup help text no longer tells the user to re-run hooks after an upgrade.
The decision is ADR 0019; ADR 0009's addendum and the spec's "Resolved verification item" section are amended, both having recorded conclusions this change supersedes.

Deviations

The first cut accepted any executable named gitea-axi on PATH.
That passed the SDK's check only because the path handed over trivially matched itself, making the check a tautology, and would have recorded a bare name for a binary shadowing this one — against the criterion's "resolves to the running program".
Hence the stricter symlink-or-wrapper rule, which in turn required following the wrapper chain once the real Nix binary showed it was two hops rather than one.

An early pruning predicate was recorded === command || recorded.includes("gitea-axi"), which reintroduced the exact substring coupling the task removes and could have deleted an unrelated tool's hook.
It is now exact-equality only; that suffices, because a re-run records an identical command and the upgrade case is handled by the bare name being stable.

Criterion 6 was met by driving the built binary rather than by a test, since no test tier installs a wrapper.
Against result/bin/gitea-axi: on PATHgitea-axi; off PATH → the store entrypoint path; impostor on PATH → fallback; re-runs in both modes → exactly one entry.
A globally npm install-ed pack records gitea-axi through its symlinked bin.

Review

Risk

Overall: MEDIUM

  • Blast radius: Low — confined to setup hooks plus a new src/hooks.ts; no other caller imports either symbol.
  • Reversibility: Low — pure revert; the only persistent effect is a rewritten settings.json/hooks.json, which re-running restores.
  • Test coverage: Low — test/hooks.test.ts covers resolution and pruning directly; test/setup.test.ts drives the CLI for the on-PATH, off-PATH and impostor branches.
  • Sensitive domain: Medium — rewrites user-owned agent configuration in place and changes what executes at session start.
  • Size & complexity: Medium — ~270 lines of new code and tests; the wrapper-chain walk is the non-obvious piece.
  • Runtime criticality: Low — setup is an explicit one-off install command, not on any request path.

Unaddressed findings

Standards — prose rationale is restated in five places (ADR 0019, the spec, CLAUDE.md, hooks.ts, setup.ts).
Kept: each audience reads exactly one of them, and the two source comments are the ones a reader hits without knowing an ADR exists.

Standards — pruneDuplicateManagedHooks casts through HookSettings/HookGroup/HookEntry rather than narrowing once.
Kept: the input is arbitrary parsed JSON from a user-owned file, and a validating parser for a shape the SDK owns would be a larger commitment than this change warrants.

All other findings were fixed in the diff.

Task: [`.claude/tasks/0043-hook-records-bare-binary-name.md`](.claude/tasks/0043-hook-records-bare-binary-name.md) ## Summary The SessionStart hook recorded the entrypoint's absolute path on every wrapper-based install, so it broke on upgrade — silently, since a hook that cannot execute simply does not run. `setup hooks` now resolves `gitea-axi` on `PATH` itself and hands that location to the SDK, whose realpath test then succeeds and records the bare, upgrade-stable name. A candidate qualifies only if it resolves to the running entrypoint: by realpath for a symlink (npm's shape), or by naming the entrypoint in its text for a generated wrapper, following the chain — a Nix install turns out to be two hops, `bin/gitea-axi` → `bin/.gitea-axi-wrapped` → `dist/main.js`. A same-named binary that is some other program is refused, and the absolute entrypoint path stands as the fallback exactly as before. The related defect on the same line goes with it. The SDK recognises its own hook by finding the marker *inside* the recorded command, so an entrypoint path without `gitea-axi` in it made a re-run append a duplicate instead of updating in place. `setup hooks` now prunes duplicates by matching the exact command it records — independent of that command's shape, and unable to mistake another tool's hook for its own. Consequently `package.nix` no longer renames its build tree, and the `setup` help text no longer tells the user to re-run hooks after an upgrade. The decision is [ADR 0019](.claude/adr/0019-hook-records-search-path-name.md); ADR 0009's addendum and the spec's "Resolved verification item" section are amended, both having recorded conclusions this change supersedes. ### Deviations The first cut accepted any executable named `gitea-axi` on `PATH`. That passed the SDK's check only because the path handed over trivially matched itself, making the check a tautology, and would have recorded a bare name for a binary shadowing this one — against the criterion's "resolves to the running program". Hence the stricter symlink-or-wrapper rule, which in turn required following the wrapper chain once the real Nix binary showed it was two hops rather than one. An early pruning predicate was `recorded === command || recorded.includes("gitea-axi")`, which reintroduced the exact substring coupling the task removes and could have deleted an unrelated tool's hook. It is now exact-equality only; that suffices, because a re-run records an identical command and the upgrade case is handled by the bare name being stable. Criterion 6 was met by driving the built binary rather than by a test, since no test tier installs a wrapper. Against `result/bin/gitea-axi`: on `PATH` → `gitea-axi`; off `PATH` → the store entrypoint path; impostor on `PATH` → fallback; re-runs in both modes → exactly one entry. A globally `npm install`-ed pack records `gitea-axi` through its symlinked `bin`. ## Review ### Risk **Overall: MEDIUM** - Blast radius: Low — confined to `setup hooks` plus a new `src/hooks.ts`; no other caller imports either symbol. - Reversibility: Low — pure revert; the only persistent effect is a rewritten `settings.json`/`hooks.json`, which re-running restores. - Test coverage: Low — `test/hooks.test.ts` covers resolution and pruning directly; `test/setup.test.ts` drives the CLI for the on-`PATH`, off-`PATH` and impostor branches. - Sensitive domain: Medium — rewrites user-owned agent configuration in place and changes what executes at session start. - Size & complexity: Medium — ~270 lines of new code and tests; the wrapper-chain walk is the non-obvious piece. - Runtime criticality: Low — `setup` is an explicit one-off install command, not on any request path. ### Unaddressed findings **Standards — prose rationale is restated in five places** (ADR 0019, the spec, `CLAUDE.md`, `hooks.ts`, `setup.ts`). Kept: each audience reads exactly one of them, and the two source comments are the ones a reader hits without knowing an ADR exists. **Standards — `pruneDuplicateManagedHooks` casts through `HookSettings`/`HookGroup`/`HookEntry` rather than narrowing once.** Kept: the input is arbitrary parsed JSON from a user-owned file, and a validating parser for a shape the SDK owns would be a larger commitment than this change warrants. All other findings were fixed in the diff.
alexion added 1 commit 2026-07-20 13:13:17 -04:00
fix(setup): record the session-start hook as a search-path name (task 0043)
All checks were successful
CI / test (22) (pull_request) Successful in 47s
CI / test (true, 24) (pull_request) Successful in 1m6s
CI / flake (pull_request) Successful in 3s
CI / test (22) (push) Successful in 49s
CI / test (true, 24) (push) Successful in 1m5s
CI / flake (push) Successful in 2s
27aad04984
The SDK records a bare, upgrade-stable binary name only when a PATH entry
realpath-matches the execPath it is handed. From the module-relative
entrypoint that can only succeed under npm, which symlinks its bin entry
straight at it; a wrapper-based install never can, because a script that
invokes a file does not resolve to that file. So every wrapper install --
Nix, a shim, a generated .cmd -- recorded an absolute path that moves on
upgrade, and a session-start hook that cannot execute fails silently.

`setup hooks` now resolves gitea-axi on PATH itself and hands that
location to the SDK, so the bare name is recorded. A candidate qualifies
only if it resolves to the running entrypoint -- by realpath for a
symlink, or by naming it in its text for a wrapper, following the chain,
since a Nix install is two hops. A same-named binary that is some other
program does not qualify, and the absolute entrypoint path stands as the
fallback exactly as before.

The related defect on the same line goes too: the SDK recognises its hook
by finding the marker inside the recorded command, so an entrypoint path
without "gitea-axi" in it made a re-run append a duplicate rather than
update in place. `setup hooks` now prunes duplicates by matching the
exact command it records, which is independent of that command's shape
and cannot mistake another tool's hook for its own.

With the coupling gone, package.nix no longer renames its build tree; the
fast tier runs from /build/source and its idempotency test passes there.
The help text's instruction to re-run hooks after an upgrade is deleted,
having become false.

Verified against the built Nix binary and a globally npm-installed pack:
both record the bare name, both fall back to the absolute path when the
name is absent, an impostor on PATH is refused, and re-runs leave one
entry. Decision recorded as ADR 0019; ADR 0009's addendum is amended.
alexion merged commit 27aad04984 into main 2026-07-20 13:16:08 -04:00
alexion deleted branch task-0043-hook-records-bare-binary-name 2026-07-20 13:16:08 -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#52