From 82cdad4ce0c24be1adc068ff8481dd8319d98305 Mon Sep 17 00:00:00 2001 From: alexion Date: Sun, 19 Jul 2026 23:23:01 -0400 Subject: [PATCH] test(nix): drive the installed binary through the shared tier (task 0038) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Nix build now runs the installed-binary facet of the packaging tier against the binary it has just produced, via an `installCheckPhase` that sets `GITEA_AXI_INSTALLED_BIN` to `$out/bin/gitea-axi` and runs the new `test:installed` script. Running after `fixupPhase` means the binary under test is the wrapped one an operator actually gets, and naming it is all the phase does — the assertions stay in the shared tier, so the npm and Nix installation paths cannot drift apart in what they guarantee. `npmInstallHook` prunes dev dependencies out of the build tree before the check runs, so `preInstall` snapshots `node_modules` with `cp -al` and the check restores it by copying, leaving the snapshot intact for a replayed phase. Two defects fixed in passing, both surfaced by review: `test:installed` pins a test file by path while the packaging runner sets `passWithNoTests: true`, so moving that file would have taken the build green having asserted nothing — the same silently-inert trap `doCheck` sprang in task 0037. The script now passes `--passWithNoTests=false`. `checkPhase`'s vitest left a timestamped run cache under `node_modules/.vite`, which `npmInstallHook` copied into `$out`, shipping a stray cache and making the derivation non-reproducible. It is now removed before the install phase; `nix build --rebuild` passes. --- .../0038-nix-build-drives-installed-binary.md | 59 ++++++++++++++++-- .gitignore | 2 + CLAUDE.md | 11 ++++ PUBLISHING.md | 3 + package.json | 1 + package.nix | 60 +++++++++++++++++++ 6 files changed, 131 insertions(+), 5 deletions(-) diff --git a/.claude/tasks/0038-nix-build-drives-installed-binary.md b/.claude/tasks/0038-nix-build-drives-installed-binary.md index 0f1fe87..08b9b79 100644 --- a/.claude/tasks/0038-nix-build-drives-installed-binary.md +++ b/.claude/tasks/0038-nix-build-drives-installed-binary.md @@ -15,8 +15,57 @@ The check reuses the seam from the parameterized tier; it does not introduce a s ## Acceptance criteria -- [ ] The Nix build drives the installed binary through the shared installed-binary tier after installation. -- [ ] A binary installed without its executable bit fails the build. -- [ ] A bundled Agent Skill installed at the wrong location relative to the built output fails the build. -- [ ] The post-install phase adds no assertions of its own beyond pointing the shared tier at the installed binary. -- [ ] `nix build` still succeeds end to end on a clean checkout. +- [x] The Nix build drives the installed binary through the shared installed-binary tier after installation. +- [x] A binary installed without its executable bit fails the build. +- [x] A bundled Agent Skill installed at the wrong location relative to the built output fails the build. +- [x] The post-install phase adds no assertions of its own beyond pointing the shared tier at the installed binary. +- [x] `nix build` still succeeds end to end on a clean checkout. + +## Implementation Notes + +### Where the check hangs + +`installCheckPhase`, not `postInstall`. +It runs after `fixupPhase`, which is where `wrapProgram` has already done its work — so the binary the tier drives is the wrapped one an operator actually gets, not the bare entrypoint. +The phase sets `GITEA_AXI_INSTALLED_BIN=$out/bin/gitea-axi` and runs `npm run test:installed`, a new script that runs the installed-binary facet alone under the packaging runner configuration. +Naming the binary is all it does; the assertions stay in the tier, satisfying the fourth criterion by construction. + +`vitest.packaging.config.ts` had to join the source allowlist in `package.nix` — exactly the Gotcha task 0037 recorded, hit on the first build. + +### Dev dependencies are gone by install time + +`npmInstallHook` runs `npm prune --omit=dev` against the build tree's `node_modules` during `installPhase`, so vitest no longer exists when `installCheckPhase` runs. +`preInstall` snapshots the tree with `cp -al` first — hardlinks, so it costs neither time nor space, and the prune's deletions do not follow through to the copy — and the check restores it. +Restoring copies rather than moves, so a replayed phase (`--keep-failed` debugging) does not consume the only surviving snapshot. +Recorded as a Gotcha, since the failure is disconnected from its cause. + +### Criterion 2 holds, but by a different mechanism than the task assumed + +The task motivates the executable-bit guard with "the compiler does not set the executable bit that npm would otherwise set from the manifest's `bin` entry". +That is true of the npm path and *not* of the Nix path: `nodejsInstallExecutables` installs each `bin` entry as a generated wrapper invoking `node `, not as a symlink to the entrypoint. +So `chmod -x` on `dist/main.js` changes nothing — verified, the build stayed green — while `chmod -x` on `$out/bin/gitea-axi` fails all three tests with `EACCES`. + +The criterion as written is therefore satisfied, and was demonstrated by probe, but the bit it protects under Nix is one `makeWrapper` always sets. +The assertion earns its keep on the npm caller, where the failure the task describes is real. +This is an argument for the shared tier rather than against it: neither installation path gets to pick which guarantees it feels like offering. +Both mechanism and probe results are recorded as a Gotcha. + +The Skill-location criterion is the one doing real work under Nix — moving the installed `skills` directory aside fails the `setup` test and the build. + +### Two defects found and fixed in passing + +Both were surfaced by the review pass rather than planned. + +`test:installed` pins a test file by path, and the packaging runner sets `passWithNoTests: true`. +Renaming or moving that file would have made vitest match nothing, exit 0, and take `nix build` green having asserted nothing — the same silently-inert trap that `doCheck` sprang in task 0037. +The script now passes `--passWithNoTests=false`. + +`nix build --rebuild` reported the derivation "may not be deterministic". +The cause predates this task: `checkPhase`'s vitest leaves a run cache at `node_modules/.vite/…/results.json` recording durations and timestamps, and `npmInstallHook` copies `node_modules` into `$out` wholesale — so every build shipped a stray cache in the closure and no two outputs matched. +`checkPhase` now removes it, and `--rebuild` passes. +Strictly this belonged to 0037, but this task adds a second vitest run over the same surface, and the fix is one line in a file already being edited. + +### Scope note + +`.gitignore` gains `result` / `result-*`. +That is 0037's flake output rather than this task's, but the symlink appears the moment anyone runs `nix build` without `--no-link` and was already showing up as untracked. diff --git a/.gitignore b/.gitignore index 51bb16a..5185c85 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,5 @@ node_modules/ dist/ coverage/ bench/results/ +result +result-* diff --git a/CLAUDE.md b/CLAUDE.md index ef6a463..fc171b7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -49,6 +49,17 @@ The flip side is the point of the design — touching an ADR, a spec, a task, a `buildNpmPackage` provides **no check hook**, so `doCheck = true` on its own is silently inert — the build logs `no Makefile or custom checkPhase, doing nothing` and ships a package whose tests never ran. Running the fast tier inside the derivation requires an explicit `checkPhase`; it also needs `git` and `which` in `nativeCheckInputs` and a writable `HOME`, since some of those tests shell out to `git`. +`nodejsInstallExecutables` (inside `npmInstallHook`) installs each manifest `bin` entry as a **generated wrapper that invokes `node ` explicitly**, not as a symlink to the entrypoint. +So under Nix the executable bit on `dist/main.js` is not load-bearing — `chmod -x` on it changes nothing — while the bit on `$out/bin/gitea-axi` is, and makeWrapper always sets it. +The npm path differs: there the `bin` entry is symlinked and npm sets the bit at install time. + +`npmInstallHook` runs `npm prune --omit=dev` against the **build tree's** `node_modules` during `installPhase`, so dev dependencies (vitest included) are gone by the time `postInstall` or `installCheckPhase` runs. +Anything that needs them after install must snapshot the tree in `preInstall` first; `cp -al` is the cheap way, since the prune's deletions do not follow hardlinks. + +Running vitest inside the derivation leaves a run cache at `node_modules/.vite/…/results.json` that records durations and timestamps, and `npmInstallHook` copies `node_modules` into `$out` wholesale. +Left alone it ships a stray cache in the closure and makes the output non-reproducible — visible only under `nix build --rebuild`, which reports "may not be deterministic"; an ordinary build stays green. +`checkPhase` deletes it before the install phase runs. + nixpkgs 26.11 (the `nixos-unstable` the flake tracks) has **dropped `x86_64-darwin`**. `legacyPackages.x86_64-darwin` now *throws* rather than merely failing to build, so listing that system in the flake's `systems` breaks `nix flake show` and `nix flake check` for every system at once, not just that one. Intel macOS would need the 26.05 branch. diff --git a/PUBLISHING.md b/PUBLISHING.md index 72f8eb7..141fcdc 100644 --- a/PUBLISHING.md +++ b/PUBLISHING.md @@ -41,3 +41,6 @@ Distribution touches no Gitea API, so this smoke test is the distribution analog The two facets are `test/packaging/tarball.test.ts`, which is npm-specific by nature, and `test/packaging/installed-binary.test.ts`, which asserts what any *installed* gitea-axi must do, whatever installed it. That second facet is therefore also the check a non-npm installation path runs against its own output. Set `GITEA_AXI_INSTALLED_BIN` to the path of an already-installed binary to have it drive that one and skip the pack-and-install setup. + +The Nix build is that other caller: its `installCheckPhase` points this variable at the wrapped binary it has just installed and runs the facet alone via `npm run test:installed`. +So `nix build` and `npm run test:pack` guarantee the same things about an installed gitea-axi, and neither can quietly weaken while the other holds. diff --git a/package.json b/package.json index f13f638..a0a9efe 100644 --- a/package.json +++ b/package.json @@ -35,6 +35,7 @@ "test:coverage": "vitest run --coverage", "test:e2e": "vitest run --config vitest.e2e.config.ts", "test:pack": "vitest run --config vitest.packaging.config.ts", + "test:installed": "vitest run --config vitest.packaging.config.ts --passWithNoTests=false test/packaging/installed-binary.test.ts", "test:bench": "vitest run --config vitest.bench.config.ts", "test:bench:smoke": "vitest run --config vitest.bench-smoke.config.ts", "bench:run": "tsx bench/run.ts", diff --git a/package.nix b/package.nix index fadf40d..15f6ab9 100644 --- a/package.nix +++ b/package.nix @@ -10,6 +10,11 @@ }: let + # Where the install check finds the dev dependencies that `npmInstallHook` + # prunes out of the build tree. Named once: it is a contract between + # `preInstall`, which writes it, and `installCheckPhase`, which reads it. + devNodeModules = "$NIX_BUILD_TOP/node_modules-dev"; + # The manifest is the canonical version: the release flow bumps it, and # reading it here means a store path and a released version cannot disagree. manifest = lib.importJSON ./package.json; @@ -37,6 +42,7 @@ let ./tsconfig.json ./tsconfig.build.json ./vitest.config.ts + ./vitest.packaging.config.ts ]; }; in @@ -89,9 +95,25 @@ buildNpmPackage { export HOME=$(mktemp -d) npm run test + # vitest leaves a run cache under node_modules/.vite whose results.json + # records durations and timestamps. `npmInstallHook` copies node_modules + # into $out wholesale, so leaving it there both ships a stray cache in the + # closure and makes the output non-reproducible — `nix build --rebuild` + # reports the derivation "may not be deterministic" on that one file. + rm -rf node_modules/.vite + runHook postCheck ''; + # `npmInstallHook` prunes dev dependencies out of the build tree's + # node_modules on its way to assembling $out, which would take vitest with it + # and leave the install check with nothing to run. Snapshot the tree first — + # as hardlinks, so it costs neither time nor space, and so the prune's + # deletions do not follow through to the copy. + preInstall = '' + cp -al node_modules "${devNodeModules}" + ''; + # ADR 0018: append, never prepend. The operator's own `tea` owns the # credential store it refreshes in place, so the closure's copy is a # fresh-machine fallback rather than an override. @@ -100,6 +122,44 @@ buildNpmPackage { --suffix PATH : ${lib.makeBinPath [ git tea ]} ''; + # Drive the binary that was just installed through the shared installed-binary + # tier, which the npm distribution path drives too — so the two cannot drift + # apart in what they guarantee about an installed gitea-axi. + # + # This guards a class of failure `checkPhase` structurally cannot reach, + # because it runs against the source tree rather than an installation. The + # one that bites here is Skill resolution: `setup` locates the bundled Agent + # Skill relative to its own module location, so the built output's position + # relative to that Skill is load-bearing — an arrangement that exists only + # once installed. A probe moving the installed `skills` aside does fail this + # phase. + # + # The tier's executable-bit assertion carries less weight under Nix than + # under npm, and deliberately so: `nodejsInstallExecutables` generates a + # wrapper invoking `node ` rather than symlinking the entrypoint, so + # the bit that matters is the one on `$out/bin/gitea-axi`, which makeWrapper + # always sets. That assertion earns its keep on the npm path, where npm sets + # the bit from the manifest's `bin` entry and `tsc` does not. Sharing one + # tier means neither path picks which guarantees it feels like offering. + # + # `installCheckPhase` runs after `fixupPhase`, so the binary named here is the + # wrapped one an operator would actually get. Naming it is all this phase + # does: the assertions live in the tier, not in shell script here. + doInstallCheck = true; + installCheckPhase = '' + runHook preInstallCheck + + # Restore by copying, not moving, so the snapshot survives for a replayed + # phase — `--keep-failed` debugging, or `genericBuild` re-entered by hand. + rm -rf node_modules + cp -al "${devNodeModules}" node_modules + + export HOME=$(mktemp -d) + GITEA_AXI_INSTALLED_BIN=$out/bin/gitea-axi npm run test:installed + + runHook postInstallCheck + ''; + meta = { inherit (manifest) description homepage; -- 2.47.3