diff --git a/.claude/tasks/0036-parameterized-installed-binary-tier.md b/.claude/tasks/0036-parameterized-installed-binary-tier.md index 9081d14..daccb7c 100644 --- a/.claude/tasks/0036-parameterized-installed-binary-tier.md +++ b/.claude/tasks/0036-parameterized-installed-binary-tier.md @@ -20,8 +20,30 @@ Nothing may assert on store paths, wrapper internals, or the arrangement of file ## Acceptance criteria -- [ ] The installed-binary assertions live separately from the tarball-shape assertions, and both still run under the packaging tier's own runner configuration. -- [ ] An environment variable naming an existing binary makes the installed-binary group drive that binary and skip pack-and-install. -- [ ] With that variable unset, the group packs, installs, and drives the result as before — the default developer experience is unchanged. -- [ ] The full packaging tier passes in both modes. -- [ ] No assertion in the installed-binary group depends on how the binary was installed. +- [x] The installed-binary assertions live separately from the tarball-shape assertions, and both still run under the packaging tier's own runner configuration. +- [x] An environment variable naming an existing binary makes the installed-binary group drive that binary and skip pack-and-install. +- [x] With that variable unset, the group packs, installs, and drives the result as before — the default developer experience is unchanged. +- [x] The full packaging tier passes in both modes. +- [x] No assertion in the installed-binary group depends on how the binary was installed. + +## Implementation Notes + +`test/packaging/packaging.test.ts` split into `tarball.test.ts` and `installed-binary.test.ts`, with the shared `npm pack` / extract / global-install mechanics factored into a non-test `npm-artifact.ts` module beside them. +The runner configuration is untouched apart from its comments: its `include` glob already matched the whole directory, so both new files run under it unchanged. + +The environment variable is `GITEA_AXI_INSTALLED_BIN`, matching the existing `GITEA_AXI_*` convention. +An empty value counts as unset, so exporting it blank behaves the same as not exporting it. +A path that does not exist fails in `beforeAll` with an explanatory message rather than letting every assertion fail on an opaque spawn `ENOENT`. + +Two deviations from the plan, both minor and both deliberate: + +The single `it` that drove the installed binary became three — usage, dashboard render, and Agent Skill installation. +The task said the assertions "do not change in character", and they do not; this only splits one case into the three behaviours the spec itself names ("the binary runs, renders, and installs its Agent Skill"), so a failure names which one broke. + +`PUBLISHING.md`'s "Verifying the packed artifact" section was rewritten to describe the two facets and document the new variable. +Not an acceptance criterion, but the section described the tier as one undifferentiated thing and would otherwise have been left stale by this change. + +Verified by running the full tier three ways: with the variable unset (both facets pack as before), with it pointing at a separately installed binary (the installed-binary facet skipped its setup — 2.2s down to 0.3s, with no `prepack` build), and with it set to the empty string (falls back to pack-and-install). + +One consequence worth flagging for task 0038: the two facets now each run `npm pack`, so `npm run test:pack` builds twice. +That is invisible to the Nix build, which will run only `test/packaging/installed-binary.test.ts` against its own installed output rather than the full tier. diff --git a/PUBLISHING.md b/PUBLISHING.md index f315187..72f8eb7 100644 --- a/PUBLISHING.md +++ b/PUBLISHING.md @@ -28,7 +28,8 @@ Bump the version first with `npm version `, which updates `pa ## Verifying the packed artifact -The packaging smoke test packs the real tarball, installs it globally into a throwaway prefix, and drives the installed binary — `--help`, the dashboard header, and `setup` finding the bundled skill: +The packaging smoke test comes in two facets: one asserts the shape of the packed tarball and its manifest, the other drives an installed binary — `--help`, the dashboard header, and `setup` finding the bundled skill. +By default it packs the real tarball and installs it globally into a throwaway prefix to produce that binary: ```sh npm run test:pack @@ -36,3 +37,7 @@ npm run test:pack It builds, packs, and fetches the runtime dependencies from the registry, so it is slower than the unit and integration tiers and is not part of the default `npm test` run. Distribution touches no Gitea API, so this smoke test is the distribution analogue of the live-Gitea end-to-end tier. + +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. diff --git a/test/packaging/installed-binary.test.ts b/test/packaging/installed-binary.test.ts new file mode 100644 index 0000000..e405992 --- /dev/null +++ b/test/packaging/installed-binary.test.ts @@ -0,0 +1,118 @@ +import { execFile, execFileSync } from "node:child_process"; +import { promisify } from "node:util"; +import { existsSync, mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { startFixtureServer } from "../fixture-server.js"; +import { installGlobally, packTarball } from "./npm-artifact.js"; + +// What an *installed* gitea-axi must do, whatever installed it: run, render +// against a Gitea, and install its Agent Skill. +// +// The binary under test comes from the environment when GITEA_AXI_INSTALLED_BIN +// names one, and otherwise from packing and globally installing the npm tarball +// right here. That makes this one seam with two callers — the npm distribution +// path and the Nix installation path — so the two cannot drift apart in what +// they guarantee. +// +// Consequently nothing below may assert on how the binary came to exist: no +// store paths, no wrapper internals, no arrangement of files within the +// installed tree. Those are implementation detail of the installation method. + +/** + * Set by a caller that has already installed gitea-axi and wants that one + * driven. An empty value counts as unset, so exporting it blank is the same as + * not exporting it at all. + */ +const providedBin = process.env.GITEA_AXI_INSTALLED_BIN || undefined; + +let workDir: string | undefined; +let binPath: string; + +// Unlike the in-process CLI-seam harness (test/harness.ts), which keeps the +// environment fully explicit so nothing leaks in, this tier spawns a real +// installed binary as a subprocess. That subprocess genuinely needs the parent +// env (PATH to resolve node/git/tea, npm config, etc.), so both helpers inherit +// `process.env` on purpose and layer the per-call `env` on top. + +/** Run the installed binary, returning its stdout. */ +function run(args: string[], env: Record = {}): string { + return execFileSync(binPath, args, { encoding: "utf8", env: { ...process.env, ...env } }); +} + +const execFileAsync = promisify(execFile); + +/** + * Run the installed binary without blocking this process's event loop, so an + * in-process fixture server can answer the CLI's HTTP calls while it runs. (A + * synchronous `execFileSync` would freeze the loop the fixture server lives on, + * deadlocking the request/response.) + */ +async function runAsync(args: string[], env: Record = {}): Promise { + const { stdout } = await execFileAsync(binPath, args, { + encoding: "utf8", + env: { ...process.env, ...env }, + }); + return stdout; +} + +beforeAll(() => { + if (providedBin !== undefined) { + // Fail here rather than letting every assertion fail on a confusing ENOENT + // from the spawn. + if (!existsSync(providedBin)) { + throw new Error( + `GITEA_AXI_INSTALLED_BIN points at ${providedBin}, which does not exist. ` + + "Unset it to have this tier pack and install the npm tarball itself.", + ); + } + binPath = providedBin; + return; + } + + workDir = mkdtempSync(join(tmpdir(), "gitea-axi-install-")); + binPath = installGlobally(packTarball(workDir), workDir); +}, 300_000); + +afterAll(() => { + if (workDir) { + rmSync(workDir, { recursive: true, force: true }); + } +}); + +describe("installed gitea-axi binary", () => { + it("is executable and prints its usage", () => { + expect(existsSync(binPath)).toBe(true); + expect(run(["--help"])).toContain("usage: gitea-axi"); + }); + + it("renders the dashboard header against a Gitea", async () => { + const server = await startFixtureServer([ + { method: "GET", path: "/api/v1/repos/o/r/pulls", body: [] }, + { method: "GET", path: "/api/v1/repos/o/r/issues", body: [] }, + ]); + try { + const dashboard = await runAsync([], { + GITEA_AXI_API_URL: server.url, + GITEA_AXI_REPO: "o/r", + GITEA_AXI_TOKEN: "x", + }); + expect(dashboard).toContain("bin:"); + expect(dashboard).toContain("description: Agent-ergonomic CLI for Gitea"); + expect(dashboard).toContain("repo: o/r"); + } finally { + await server.close(); + } + }); + + it("finds its bundled Agent Skill and installs it into HOME/.claude", () => { + const home = mkdtempSync(join(tmpdir(), "gitea-axi-home-")); + try { + expect(run(["setup"], { HOME: home })).toContain("status: installed"); + expect(existsSync(join(home, ".claude", "skills", "gitea-axi", "SKILL.md"))).toBe(true); + } finally { + rmSync(home, { recursive: true, force: true }); + } + }); +}); diff --git a/test/packaging/npm-artifact.ts b/test/packaging/npm-artifact.ts new file mode 100644 index 0000000..1b5d0bb --- /dev/null +++ b/test/packaging/npm-artifact.ts @@ -0,0 +1,53 @@ +import { execFileSync } from "node:child_process"; +import { mkdirSync } from "node:fs"; +import { join } from "node:path"; +import { fileURLToPath } from "node:url"; + +// Building the npm distribution artifact, factored out so the two packaging +// facets can each take just the part they need: the tarball facet packs and +// extracts, the installed-binary facet packs and installs. + +export const projectRoot = fileURLToPath(new URL("../..", import.meta.url)); + +/** + * Pack the real tarball into `destDir`, returning its path. + * + * `npm pack --json` reports the tarball filename; the shape shifted across npm + * majors (an array of entries on npm <12, an object keyed by package name on + * npm 12+), so accept either and pull the one filename out. + */ +export function packTarball(destDir: string): string { + const packOutput = execFileSync("npm", ["pack", "--json", "--pack-destination", destDir], { + cwd: projectRoot, + encoding: "utf8", + }); + const packResult = JSON.parse(packOutput) as unknown; + const packEntries = Array.isArray(packResult) + ? (packResult as Array<{ filename: string }>) + : Object.values(packResult as Record); + return join(destDir, packEntries[0]!.filename); +} + +/** + * Extract `tarball` into a fresh `extract/` under `destDir`, returning the + * directory npm nests everything under (`/extract/package`). + */ +export function extractTarball(tarball: string, destDir: string): string { + const extractDir = join(destDir, "extract"); + mkdirSync(extractDir); + execFileSync("tar", ["-xzf", tarball, "-C", extractDir]); + return join(extractDir, "package"); +} + +/** + * Install `tarball` globally into a throwaway prefix under `destDir`, returning + * the path of the installed binary on that prefix's bin dir. + */ +export function installGlobally(tarball: string, destDir: string): string { + const prefix = join(destDir, "prefix"); + execFileSync("npm", ["install", "-g", "--prefix", prefix, tarball], { + cwd: projectRoot, + encoding: "utf8", + }); + return join(prefix, "bin", "gitea-axi"); +} diff --git a/test/packaging/packaging.test.ts b/test/packaging/packaging.test.ts deleted file mode 100644 index 281b721..0000000 --- a/test/packaging/packaging.test.ts +++ /dev/null @@ -1,176 +0,0 @@ -import { execFile, execFileSync } from "node:child_process"; -import { promisify } from "node:util"; -import { existsSync, mkdirSync, mkdtempSync, rmSync } from "node:fs"; -import { readFileSync } from "node:fs"; -import { tmpdir } from "node:os"; -import { join } from "node:path"; -import { fileURLToPath } from "node:url"; -import { afterAll, beforeAll, describe, expect, it } from "vitest"; -import { startFixtureServer } from "../fixture-server.js"; - -// The one artifact under test: the packed npm tarball, installed globally into a -// throwaway prefix. Packing plus a global install is expensive and identical for -// every facet, so it is built once here and the four facets assert against the -// shared result. - -const projectRoot = fileURLToPath(new URL("../..", import.meta.url)); - -let workDir: string; -let extractDir: string; -let binPath: string; -/** The manifest as it ships inside the packed tarball. */ -let packedManifest: Record; - -// Unlike the in-process CLI-seam harness (test/harness.ts), which keeps the -// environment fully explicit so nothing leaks in, this tier spawns a real -// globally-installed binary as a subprocess. That subprocess genuinely needs the -// parent env (PATH to resolve node/npm/tar, npm config, etc.), so both helpers -// inherit `process.env` on purpose and layer the per-call `env` on top. - -/** Run the globally installed binary, returning its stdout. */ -function run(args: string[], env: Record = {}): string { - return execFileSync(binPath, args, { encoding: "utf8", env: { ...process.env, ...env } }); -} - -const execFileAsync = promisify(execFile); - -/** - * Run the installed binary without blocking this process's event loop, so an - * in-process fixture server can answer the CLI's HTTP calls while it runs. (A - * synchronous `execFileSync` would freeze the loop the fixture server lives on, - * deadlocking the request/response.) - */ -async function runAsync(args: string[], env: Record = {}): Promise { - const { stdout } = await execFileAsync(binPath, args, { - encoding: "utf8", - env: { ...process.env, ...env }, - }); - return stdout; -} - -beforeAll(() => { - workDir = mkdtempSync(join(tmpdir(), "gitea-axi-pack-")); - - // Pack the real tarball. `npm pack --json` reports the tarball filename; the - // shape shifted across npm majors (an array of entries on npm <12, an object - // keyed by package name on npm 12+), so accept either and pull the one - // filename out. - const packOutput = execFileSync( - "npm", - ["pack", "--json", "--pack-destination", workDir], - { cwd: projectRoot, encoding: "utf8" }, - ); - const packResult = JSON.parse(packOutput) as unknown; - const packEntries = Array.isArray(packResult) - ? (packResult as Array<{ filename: string }>) - : Object.values(packResult as Record); - const tarball = join(workDir, packEntries[0]!.filename); - - // Extract to inspect the packed manifest and confirm bundled files. npm nests - // everything under `package/`. - extractDir = join(workDir, "extract"); - mkdirSync(extractDir); - execFileSync("tar", ["-xzf", tarball, "-C", extractDir]); - packedManifest = JSON.parse( - readFileSync(join(extractDir, "package", "package.json"), "utf8"), - ) as Record; - - // Install globally into a throwaway prefix so the binary lands on a PATH-like - // bin dir we control. - const prefix = join(workDir, "prefix"); - execFileSync("npm", ["install", "-g", "--prefix", prefix, tarball], { - cwd: projectRoot, - encoding: "utf8", - }); - binPath = join(prefix, "bin", "gitea-axi"); -}, 300_000); - -afterAll(() => { - if (workDir) { - rmSync(workDir, { recursive: true, force: true }); - } -}); - -describe("npm distribution artifact", () => { - it("bundles the built CLI, the bin entry, and the Agent Skill, and declares no postinstall", () => { - expect(existsSync(join(extractDir, "package", "dist", "main.js"))).toBe(true); - expect(existsSync(join(extractDir, "package", "skills", "gitea-axi", "SKILL.md"))).toBe(true); - - const bin = packedManifest.bin as Record | undefined; - expect(bin?.["gitea-axi"]).toBe("dist/main.js"); - - const scripts = packedManifest.scripts as Record | undefined; - expect(scripts?.postinstall).toBeUndefined(); - }); - - it("excludes the bench/ harness directory from the package", () => { - expect(existsSync(join(extractDir, "package", "bench"))).toBe(false); - }); - - it("declares complete metadata: unscoped name, description, repo, license, engines, ESM type", () => { - const name = packedManifest.name as string; - expect(name).toBe("gitea-axi"); - expect(name).not.toContain("@"); - expect(name).not.toContain("/"); - - const description = packedManifest.description as string; - expect(typeof description).toBe("string"); - expect(description.length).toBeGreaterThan(0); - - const repository = packedManifest.repository as string | { url?: string } | undefined; - const repositoryUrl = typeof repository === "string" ? repository : repository?.url; - expect(repositoryUrl).toContain("gitea-axi"); - - expect(packedManifest.license).toBe("MIT"); - - const engines = packedManifest.engines as Record | undefined; - expect(engines?.node).toMatch(/20/); - - expect(packedManifest.type).toBe("module"); - }); - - it("scripts and documents the publish flow so publishing is a single command", () => { - const scripts = packedManifest.scripts as Record | undefined; - expect(scripts?.prepack).toBe("npm run build"); - - const publishConfig = packedManifest.publishConfig as Record | undefined; - expect(publishConfig?.access).toBe("public"); - - const publishingDoc = join(projectRoot, "PUBLISHING.md"); - expect(existsSync(publishingDoc)).toBe(true); - expect(readFileSync(publishingDoc, "utf8")).toMatch(/npm publish/); - }); - - it("puts a working gitea-axi on the PATH: dashboard header, --help, and setup all function", async () => { - expect(existsSync(binPath)).toBe(true); - - expect(run(["--help"])).toContain("usage: gitea-axi"); - - // Dashboard header renders against a stubbed Gitea. - const server = await startFixtureServer([ - { method: "GET", path: "/api/v1/repos/o/r/pulls", body: [] }, - { method: "GET", path: "/api/v1/repos/o/r/issues", body: [] }, - ]); - try { - const dashboard = await runAsync([], { - GITEA_AXI_API_URL: server.url, - GITEA_AXI_REPO: "o/r", - GITEA_AXI_TOKEN: "x", - }); - expect(dashboard).toContain("bin:"); - expect(dashboard).toContain("description: Agent-ergonomic CLI for Gitea"); - expect(dashboard).toContain("repo: o/r"); - } finally { - await server.close(); - } - - // `setup` installs the Agent Skill into HOME/.claude. - const home = mkdtempSync(join(tmpdir(), "gitea-axi-home-")); - try { - expect(run(["setup"], { HOME: home })).toContain("status: installed"); - expect(existsSync(join(home, ".claude", "skills", "gitea-axi", "SKILL.md"))).toBe(true); - } finally { - rmSync(home, { recursive: true, force: true }); - } - }); -}); diff --git a/test/packaging/tarball.test.ts b/test/packaging/tarball.test.ts new file mode 100644 index 0000000..463cc90 --- /dev/null +++ b/test/packaging/tarball.test.ts @@ -0,0 +1,83 @@ +import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { extractTarball, packTarball, projectRoot } from "./npm-artifact.js"; + +// The shape of the packed npm tarball and the manifest that ships inside it. +// These assertions are npm-specific by nature — no other distribution method +// produces a tarball or a packed manifest — so they stay separate from the +// installed-binary assertions, which any installation method can drive. + +let workDir: string; +/** The extracted tarball's root — npm nests everything under `package/`. */ +let packageDir: string; +/** The manifest as it ships inside the packed tarball. */ +let packedManifest: Record; + +beforeAll(() => { + workDir = mkdtempSync(join(tmpdir(), "gitea-axi-pack-")); + const tarball = packTarball(workDir); + packageDir = extractTarball(tarball, workDir); + packedManifest = JSON.parse(readFileSync(join(packageDir, "package.json"), "utf8")) as Record< + string, + unknown + >; +}, 300_000); + +afterAll(() => { + if (workDir) { + rmSync(workDir, { recursive: true, force: true }); + } +}); + +describe("npm distribution artifact", () => { + it("bundles the built CLI, the bin entry, and the Agent Skill, and declares no postinstall", () => { + expect(existsSync(join(packageDir, "dist", "main.js"))).toBe(true); + expect(existsSync(join(packageDir, "skills", "gitea-axi", "SKILL.md"))).toBe(true); + + const bin = packedManifest.bin as Record | undefined; + expect(bin?.["gitea-axi"]).toBe("dist/main.js"); + + const scripts = packedManifest.scripts as Record | undefined; + expect(scripts?.postinstall).toBeUndefined(); + }); + + it("excludes the bench/ harness directory from the package", () => { + expect(existsSync(join(packageDir, "bench"))).toBe(false); + }); + + it("declares complete metadata: unscoped name, description, repo, license, engines, ESM type", () => { + const name = packedManifest.name as string; + expect(name).toBe("gitea-axi"); + expect(name).not.toContain("@"); + expect(name).not.toContain("/"); + + const description = packedManifest.description as string; + expect(typeof description).toBe("string"); + expect(description.length).toBeGreaterThan(0); + + const repository = packedManifest.repository as string | { url?: string } | undefined; + const repositoryUrl = typeof repository === "string" ? repository : repository?.url; + expect(repositoryUrl).toContain("gitea-axi"); + + expect(packedManifest.license).toBe("MIT"); + + const engines = packedManifest.engines as Record | undefined; + expect(engines?.node).toMatch(/20/); + + expect(packedManifest.type).toBe("module"); + }); + + it("scripts and documents the publish flow so publishing is a single command", () => { + const scripts = packedManifest.scripts as Record | undefined; + expect(scripts?.prepack).toBe("npm run build"); + + const publishConfig = packedManifest.publishConfig as Record | undefined; + expect(publishConfig?.access).toBe("public"); + + const publishingDoc = join(projectRoot, "PUBLISHING.md"); + expect(existsSync(publishingDoc)).toBe(true); + expect(readFileSync(publishingDoc, "utf8")).toMatch(/npm publish/); + }); +}); diff --git a/vitest.packaging.config.ts b/vitest.packaging.config.ts index 29b05f2..1933586 100644 --- a/vitest.packaging.config.ts +++ b/vitest.packaging.config.ts @@ -2,17 +2,20 @@ import { defineConfig } from "vitest/config"; export default defineConfig({ test: { - // The packaging tier: pack the real tarball, install it globally into a - // throwaway prefix, then drive the installed binary. It builds, packs, and - // fetches runtime deps from the registry, so it is far slower than the fast - // tiers and runs on its own via `test:pack`. Distribution touches no Gitea - // API, so this smoke test is the distribution analogue of the live-Gitea - // e2e tier rather than a member of it. + // The packaging tier, in two facets. `tarball` asserts the shape of the + // packed npm tarball and its manifest; `installed-binary` drives an + // installed gitea-axi, either one named by GITEA_AXI_INSTALLED_BIN or one + // it packs and installs itself. It builds, packs, and fetches runtime deps + // from the registry, so it is far slower than the fast tiers and runs on + // its own via `test:pack`. Distribution touches no Gitea API, so this smoke + // test is the distribution analogue of the live-Gitea e2e tier rather than + // a member of it. include: ["test/packaging/**/*.test.ts"], testTimeout: 180_000, hookTimeout: 300_000, - // Keep the pack/install work in one process: the tarball is built once in a - // shared setup and reused across the assertions. + // Each facet's setup may run `npm pack` against the one shared project + // root, whose `prepack` writes `dist/`; serialize the files so two builds + // cannot race on it. fileParallelism: false, passWithNoTests: true, },