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
5 changed files with 88 additions and 13 deletions
Showing only changes of commit 397da5d7a6 - Show all commits

View File

@@ -200,15 +200,30 @@ The workflow keeps its container-and-npm shape, deliberately preserving the GitH
## Further Notes
### Open verification item
### Resolved verification item: the hook records the absolute path under Nix
The `setup` command's hook installation passes the SDK both an absolute path to the running entrypoint and the bare binary name.
Under Nix the absolute path is content-addressed: it changes on every rebuild and is eventually garbage-collected, so a hook recording it would break silently, since a session-start hook that cannot execute simply does not run.
The bare binary name strongly suggests the SDK prefers search-path resolution and treats the absolute path as a fallback, which would make this a non-issue, but this could not be confirmed during design because the dependency was not installed.
The design-time hope was that the SDK prefers search-path resolution and treats the absolute path as a fallback, which would have made this a non-issue.
It was verified against the installed dependency and against a real Nix build, and the answer is the unfavourable one: **under Nix the absolute store path is recorded**, even with the binary on `PATH`.
The decision is to verify before acting.
If the SDK does record the absolute path, the immediate mitigation is documenting that the hook setup should be re-run after an upgrade.
Changing the `setup` command to prefer the bare name would then become a separate task with its own ADR — justified on the grounds that a stable search-path name is more robust for *every* installation method, and explicitly not as a special case that detects Nix store paths in application code.
The SDK's `resolvePortableHookCommand` returns the bare name only when some `PATH` entry *realpath-matches* the entrypoint, and the absolute path otherwise.
That test is what splits the two installation methods, and the split is a property of how each one puts the binary on `PATH`:
- **npm** symlinks the `bin` entry directly at the entrypoint, so the realpath comparison succeeds and the bare name is recorded.
- **Nix** installs the `bin` entry as a *generated wrapper script* that invokes `node <path>``nodejsInstallExecutables` inside `npmInstallHook`, plus this package's own `makeWrapper` layer for `git` and `tea`.
A wrapper's realpath is the wrapper, never the entrypoint, so the comparison cannot succeed and the absolute path is recorded.
So the preference for the bare name is real, but it is unreachable through any wrapper-based install.
It is not that Nix was overlooked; it is that the mechanism keys on a filesystem relationship only the symlink shape has.
The mitigation is therefore documentation, per the decision recorded when this item was opened: the `setup` command's help text states that `setup hooks` must be re-run after an upgrade.
The failure it guards against is silent — a session-start hook that cannot execute simply does not run, so a user gets no error, only the quiet absence of their ambient dashboard.
Changing the `setup` command to prefer the bare name remains a separate task with its own ADR, justified on the grounds that a stable search-path name is more robust for *every* installation method, and explicitly not as a special case that detects Nix store paths in application code.
Two findings feed that future task.
First, the mitigation above is documentation against a silent failure, which is the weakest kind of fix.
Second, a related defect shares the same line: `isManagedHook` recognises its own hook by testing whether the recorded command string *contains* the marker `gitea-axi`, so an entrypoint path lacking that substring makes `setup hooks` append a duplicate rather than update in place, contradicting the idempotency its help text promises.
That coupling is why `package.nix` renames its build tree in `postUnpack`; the rename can be deleted once the hook no longer depends on the entrypoint path.
### Verified during design

View File

@@ -19,10 +19,10 @@ Changing the `setup` command to prefer the bare name is explicitly **not** part
## Acceptance criteria
- [ ] The SDK's actual hook-path behavior is determined by observation against the installed dependency, not inference from its interface.
- [ ] The finding is recorded where a future reader will meet it, so the question is not re-opened from scratch.
- [ ] If the absolute path is recorded, the documentation states that hook setup must be re-run after an upgrade.
- [ ] No change is made to how the `setup` command constructs the hook in this task.
- [x] The SDK's actual hook-path behavior is determined by observation against the installed dependency, not inference from its interface.
- [x] The finding is recorded where a future reader will meet it, so the question is not re-opened from scratch.
- [x] If the absolute path is recorded, the documentation states that hook setup must be re-run after an upgrade.
- [x] No change is made to how the `setup` command constructs the hook in this task.
## Evidence gathered during task 0037
@@ -47,3 +47,46 @@ Consequences for this task:
- Both defects trace to the same resolution line, so they should be weighed together.
- The stale store path is user-facing breakage on the install method task 0037 added, which argues for not letting this drift far behind it.
- `package.nix` carries a `postUnpack` rename of the build tree purely to work around the substring coupling. It is commented as a workaround and should be **deleted as part of this task**, once the hook no longer depends on the entrypoint path.
## Implementation Notes
### The observation
Task 0037's evidence was re-verified independently rather than taken on trust, since acceptance criterion 1 asks for observation and not for citation.
Two observations were made against the installed dependency.
A probe drove `resolvePortableHookCommand` directly with two synthetic install trees.
Given a `PATH` entry that is a *symlink* to the entrypoint it returned the bare name `gitea-axi`; given a `PATH` entry that is a *wrapper script* invoking `node <entrypoint>` it returned the absolute path.
Then the flake was built and the resulting binary driven for real against a temporary `HOME`.
With `$out/bin` on `PATH`, `~/.claude/settings.json` recorded:
```
/nix/store/pqxhyy5cg1rljyn78kxfpxyfpgz7rgzk-gitea-axi-0.1.0/lib/node_modules/gitea-axi/dist/main.js
```
This confirms 0037's finding and sharpens it.
The task framed the bare name as a hint that the SDK "prefers search-path resolution"; that preference is real, but it is gated on a `PATH` entry whose realpath equals the entrypoint.
npm satisfies that by symlinking its `bin` entry; Nix cannot, because `nodejsInstallExecutables` generates a wrapper script and this package adds a second `makeWrapper` layer for `git` and `tea`.
So the behaviour is not Nix-specific — it applies to *any* wrapper-based install — which strengthens the case, already recorded, that the eventual fix belongs in the `setup` command for every installation method rather than as a Nix special case.
### Deviations
**The `postUnpack` rename in `package.nix` was kept, not deleted.**
The "Evidence gathered during task 0037" section above says it "should be **deleted as part of this task**", which conflicts with acceptance criterion 4 and with the "What to build" section's statement that changing the hook to prefer the bare name is "explicitly **not** part of this task".
The conflict resolves on the Evidence section's own wording: the deletion is conditioned on "once the hook no longer depends on the entrypoint path", and establishing that precondition is exactly the out-of-scope change.
Deleting the rename now would leave the derivation's build tree at a path with no `gitea-axi` segment, which `isManagedHook`'s substring test still requires, and `test/setup.test.ts` would fail inside `checkPhase`.
The rename's comment was rewritten instead: it previously promised that task 0042 would remove the coupling, which would have become a stale forward reference the moment this task landed.
**The documentation surface is the `setup` help text.**
The repository has no README, so the command's own help is the only place a user meets this.
Criterion 4 is untouched — `binaryNames`, `execPath`, and the `installSessionStartHooks` call are all unchanged; only the `usage` string moved.
**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.
Leaving a statement the commit itself documents as untrue seemed worse than a one-line caveat, so the new paragraph notes that a stale entry may survive a re-run.
### Follow-up
The successor task is described in the spec's Further Notes but not yet written as a task file.
It should carry its own ADR and cover both defects together, since both trace to the same resolution line: the absolute-path recording, and `isManagedHook` recognising its hook by substring.
Deleting `package.nix`'s `postUnpack` rename belongs to it.

View File

@@ -71,3 +71,9 @@ Its `act` fork declares `RawContinueOnError` on the `Step` struct only — `pkg/
Gitea's own syntax-comparison page does not list the gap.
Put `continue-on-error` on each step instead: `act` and GitHub Actions both honour it there, and a job whose every step carries it concludes green on either platform.
The same fork historically ignored `jobs.<id>.if` (go-gitea#25897), so treat any job-level key as needing a check against the fork's structs rather than against GitHub's documentation.
The session-start hook installed by `setup hooks` records the entrypoint's **absolute path**, not the bare binary name, on every wrapper-based install.
`resolvePortableHookCommand` in `axi-sdk-js` 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 that invokes `node <path>`, whose realpath is the wrapper, so the match cannot succeed and the store path is recorded.
Passing `binaryNames` therefore does not make the hook portable under Nix — verify by driving the installed binary and reading `~/.claude/settings.json`, not by reading the SDK's interface.
The consequence is silent: a hook whose recorded path no longer exists does not run and does not warn.

View File

@@ -68,9 +68,12 @@ buildNpmPackage {
# finding "gitea-axi" within it. Naming the tree makes the build representative
# rather than an environment no operator ever has.
#
# This coupling is a defect, not a property worth preserving — see task 0042,
# which removes the hook's dependence on the entrypoint path entirely. Once it
# lands this rename should go with it.
# This coupling is a defect, not a property worth preserving. Task 0042
# verified the resolution behaviour and documented the mitigation, but left
# the hook's dependence on the entrypoint path in place: removing it needs its
# own ADR, since a stable search-path name is the right answer for every
# installation method and not a Nix special case. This rename goes away with
# that task, not before.
postUnpack = ''
mv "$sourceRoot" gitea-axi
export sourceRoot=gitea-axi

View File

@@ -18,6 +18,14 @@ Install gitea-axi's ambient context for agent sessions.
Both are idempotent: re-running updates the managed files in place rather than
failing. There is no postinstall script — installation is always explicit.
Re-run "setup hooks" after upgrading gitea-axi. The hook records an absolute
path to the entrypoint, which moves when the install location changes, and a
session-start hook that cannot be executed fails silently rather than warning.
This matters most for immutable installs such as Nix, where every rebuild lands
the entrypoint at a fresh path and the old one is eventually collected. When the
path moves, the re-run may leave the stale entry behind instead of replacing it;
remove it by hand if a duplicate appears.
flags:
--help Show this help
`;