From 6a9d4d4770e3376c49d2ca44b9c6cef0da0b3052 Mon Sep 17 00:00:00 2001 From: alexion Date: Sun, 19 Jul 2026 23:36:40 -0400 Subject: [PATCH] feat(nix): add a dev shell and a checks output (task 0039) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `nix develop` now yields the toolchain the repository actually needs — Node, `git`, `tea`, and `curl` — giving a declarative answer to "what do I need to work on this", which the repository previously specified nowhere. The shell takes its Node from the package's `passthru` rather than naming `pkgs.nodejs` a second time, so development and the shipped artifact cannot drift onto different majors and cannot be set independently. `package.nix` declares that `passthru` as an interface rather than leaving the shell to read an incidental build attribute; it does not enter the derivation, so the store path is unchanged. The checks output aliases the package, so `nix flake check` builds it and thereby runs both its verification phases instead of being a silent no-op. No per-stage checks: the only stage adding coverage is the full typecheck, which spans `test/` and `bench/` and would drag the benchmark harness into the derivation's inputs, undoing the source filtering. It stays in CI. The shell deliberately omits `gitea-axi` itself. The benchmark's arm resolves that binary by name off PATH and must get the locally built `dist/main.js`, so supplying the packaged one would silently substitute the wrong artifact. --- .claude/tasks/0039-dev-shell-and-checks.md | 40 ++++++++++++-- CLAUDE.md | 6 ++- flake.nix | 62 ++++++++++++++++++++-- package.nix | 6 +++ 4 files changed, 102 insertions(+), 12 deletions(-) diff --git a/.claude/tasks/0039-dev-shell-and-checks.md b/.claude/tasks/0039-dev-shell-and-checks.md index 1191421..00a0467 100644 --- a/.claude/tasks/0039-dev-shell-and-checks.md +++ b/.claude/tasks/0039-dev-shell-and-checks.md @@ -16,8 +16,38 @@ The full typecheck stays in continuous integration, where it already runs. ## Acceptance criteria -- [ ] `nix develop` yields a shell with Node, `git`, and `tea` available. -- [ ] The build, the fast tier, and the benchmark harness's runner all work from inside that shell. -- [ ] The shell's Node and the package's Node come from one reference — changing it moves both, and they cannot be set independently. -- [ ] `nix flake check` builds the package and runs its tests, and fails when the package fails. -- [ ] No per-stage check derivations are added. +- [x] `nix develop` yields a shell with Node, `git`, and `tea` available. +- [x] The build, the fast tier, and the benchmark harness's runner all work from inside that shell. +- [x] The shell's Node and the package's Node come from one reference — changing it moves both, and they cannot be set independently. +- [x] `nix flake check` builds the package and runs its tests, and fails when the package fails. +- [x] No per-stage check derivations are added. + +## Implementation Notes + +### The single Node reference is a `passthru`, not a second `pkgs.nodejs` + +The shell takes `self.packages.${system}.gitea-axi.nodejs` rather than naming `pkgs.nodejs` again. +Naming it twice would satisfy the criterion's letter while leaving two places to edit, which is the drift the criterion exists to prevent; reading it back off the derivation means there is genuinely one reference. + +The first cut relied on `buildNpmPackage` incidentally surfacing its `nodejs` argument as a derivation attribute — which works, but only as a side effect of `inherit src nodejs`, with nothing marking it load-bearing. +Review caught that: moving or dropping that `inherit` would have broken the shell silently at a distance. +`package.nix` now declares `passthru = { inherit nodejs; };`, making it an interface with a comment saying what depends on it. +`passthru` does not enter the derivation, so the store path is unchanged and the change costs no rebuild — verified: the drv hash before and after is identical. + +### `curl` was a missing part of the toolchain + +The benchmark's `raw-api` arm shells out to `curl` (`ARM_BINARY` in `bench/guard.ts`), so the shell carries it alongside Node, `git`, and `tea`. + +### The shell deliberately does not supply `gitea-axi` + +The criterion asks that the benchmark harness's runner work from inside the shell, and it does. +A *live* arm run is a further step the shell cannot take: `provisionArmBin` resolves each arm's binary by name off `PATH`, and the `gitea-axi` arm's binary must be the locally built `dist/main.js` so a run measures the working tree rather than whatever the flake last packaged. +Putting the packaged binary on `PATH` would satisfy the lookup with the wrong artifact — a silently misleading benchmark, worse than a missing one. + +Exposing the *built* one was considered and rejected as well: `tsc` does not set an executable bit on `dist/main.js` (npm sets it at install time from the manifest's `bin` entry, which is what the packaging tier's bit assertion guards), so a `shellHook` would have had to `chmod +x` the build output on every shell entry — mutating build artifacts to work around a lookup that is the benchmark's own concern. +Both the flake and the CLAUDE.md gotcha now state the boundary rather than implying the shell covers it. + +### CLAUDE.md's toolchain gotcha was stale by construction + +It read "the repository has no dev shell yet (task 0039 adds one)". +This slice is that task, so the entry was rewritten to point at `nix develop --command`, keeping `nix shell nixpkgs#nodejs -c ...` only as the one-off outside the repository. diff --git a/CLAUDE.md b/CLAUDE.md index fc171b7..9a37727 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -36,8 +36,10 @@ Per [ADR 0001](.claude/adr/0001-diff-auth-via-tea-login-list.md) as amended, `sr The only bypass is the test hook requiring `GITEA_AXI_API_URL` + `GITEA_AXI_TOKEN` + `GITEA_AXI_REPO` together; there is no user-facing path that avoids `tea`, and `TEA_NOT_INSTALLED` exists for its absence. Neither `node`/`npm` nor `tea` is on the `PATH` in a non-interactive shell on this machine, and there is no `~/.gitconfig`. -This is a NixOS host with no global Node install, and the repository has no dev shell yet (task 0039 adds one). -Until then, prefix commands with `nix shell nixpkgs#nodejs -c ...`, adding `nixpkgs#tea` for anything that resolves credentials — including `gitea-axi pr create`. +This is a NixOS host with no global Node install, so run work through the flake's dev shell: `nix develop --command `, which carries Node, `git`, `tea`, and `curl` — enough for the build, the fast tier, the live end-to-end tier, and the benchmark harness's own runner. +That covers anything resolving credentials, including `gitea-axi pr create`. +The shell deliberately does *not* put `gitea-axi` on the `PATH`, so a live `bench:run` of the `gitea-axi` arm still needs the built binary exposed under that name itself — see the benchmark gotcha above. +`nix shell nixpkgs#nodejs -c ...` still works for a one-off outside the repository, but inside it the dev shell is the declarative answer and pins the same Node the package is built against. `node_modules/` may be absent too, so `npm ci` first. Commits need an explicit identity: `git -c user.name=alexion -c user.email=contact@alexion.dev commit ...`, matching the existing history. diff --git a/flake.nix b/flake.nix index 186f62e..a0e6561 100644 --- a/flake.nix +++ b/flake.nix @@ -19,13 +19,65 @@ "aarch64-darwin" ]; + # Hands each output both the package set and the system name — the latter + # because the shell and the checks reach back into `self.packages` for the + # system being evaluated, and `pkgs.system` is discouraged in favour of a + # considerably wordier spelling. forAllSystems = - f: nixpkgs.lib.genAttrs systems (system: f nixpkgs.legacyPackages.${system}); + f: + nixpkgs.lib.genAttrs systems ( + system: + f { + inherit system; + pkgs = nixpkgs.legacyPackages.${system}; + } + ); in { - packages = forAllSystems (pkgs: rec { - gitea-axi = pkgs.callPackage ./package.nix { }; - default = gitea-axi; - }); + packages = forAllSystems ( + { pkgs, ... }: rec { + gitea-axi = pkgs.callPackage ./package.nix { }; + default = gitea-axi; + } + ); + + # The toolchain the repository actually needs: the build and the fast tier + # want Node, the live end-to-end tier and the benchmark harness additionally + # shell out to `git`, `tea`, and `curl` — none of which the repository + # specifies anywhere else. + # + # Not `gitea-axi` itself, which the benchmark's own arm resolves by name off + # PATH: that has to be the locally built `dist/main.js`, so that a bench run + # measures the working tree rather than whatever the flake last packaged. + # Supplying it here would silently substitute the wrong binary. + devShells = forAllSystems ( + { pkgs, system }: { + default = pkgs.mkShell { + packages = [ + # The package's own Node, taken from its passthru rather than named + # a second time here. There is one reference, so development and + # the shipped artifact cannot drift onto different majors — and + # they cannot be set independently even by mistake. + self.packages.${system}.gitea-axi.nodejs + pkgs.git + pkgs.tea + # The benchmark's raw-api arm shells out to curl. + pkgs.curl + ]; + }; + } + ); + + # An alias for the package, so `nix flake check` builds it and thereby runs + # both its verification phases — the fast tier in `checkPhase`, the + # installed-binary tier in `installCheckPhase`. + # + # No granular per-stage checks: the one stage that would add coverage the + # package build does not already have is the full typecheck, which spans + # `test/` and `bench/` and would therefore drag the benchmark harness into + # the derivation's inputs — undoing the source filtering that keeps + # benchmark churn from forcing a rebuild. That typecheck stays in + # continuous integration, where it already runs. + checks = forAllSystems ({ system, ... }: { inherit (self.packages.${system}) gitea-axi; }); }; } diff --git a/package.nix b/package.nix index 15f6ab9..4537c11 100644 --- a/package.nix +++ b/package.nix @@ -160,6 +160,12 @@ buildNpmPackage { runHook postInstallCheck ''; + # The Node the package is built against, published as a declared interface + # rather than left to be read off the build environment. The flake's dev shell + # consumes exactly this, so the two cannot drift onto different majors — and + # this attribute is why that holds, so removing it breaks the shell. + passthru = { inherit nodejs; }; + meta = { inherit (manifest) description homepage; -- 2.47.3