From 1ea06a2b14ae0e23438f4c47821da5a94a86b738 Mon Sep 17 00:00:00 2001 From: Ruth-2424 Date: Wed, 14 Oct 2026 11:11:27 -0400 Subject: [PATCH 1/5] build(release): install the packed release on its own before publishing, and on pull requests A package that works from a checkout but not from npm reaches everyone who installs it: a file left out of `files`, a dependency only devDependencies supply, or a bin npm's link can't run. scripts/pack-installs-alone.mjs packs the package as `npm publish` does, installs the tarball alone into an empty directory with its dependencies from the registry, and checks what the installed package.json offers: - every file it names is in the package: main, types, each exports target under every condition (the bun and types targets included), at least one packed file for each export pattern, and each bin. Before packing it checks those files exist in the tree, so an unbuilt tree says "build first"; - each entry point loads as its consumers load it, in its own process with a 60 s limit: import() where Node's import conditions pick a target (a JSON target with its import attribute), require() where only require serves it, nothing for a types-only subpath; without exports, the root loads from main or index.js; - each bin starts with a '#!' line, without which npm's link can't run it, and a node bin parses (node --check). Bins aren't run. Where it runs: - publish.yml: a pack-check job with contents: read only, which publish needs. Its install runs dependencies' install scripts, so it stays out of the job that holds id-token: write; publish itself now installs with --ignore-scripts, since neither tsc nor npm publish needs one (esbuild's postinstall is the tree's only one). The GitHub release notes don't wait for it. A dispatch on a branch runs it and publishes nothing. - ci.yml: the same check on pull requests and pushes to main, on ubuntu with Node 24. Its install resolves from the registry as it stands, so a new upstream release can turn it red on a change that touched nothing it installs; rerunning it on main tells the two apart. This repository passes it once agent-framework-mcp has its '#!' line (#346, which this builds on); without it, the check stops at that bin. Tests (test/pack-installs-alone.test.ts, 8), in throwaway packages packed and installed offline: every kind of entry passing, string and conditional exports, the index.js fallback, a bin-only package, an unbuilt tree, files left out of `files` (a bun target, a types file, a pattern), a devDependencies-only import, a failing subpath, a bin with no '#!' line or that doesn't parse, and a dependency that can't be installed. Each piece of the check, removed on its own, fails the tests aimed at it. Read before opening by Naomi-2422, who ran the publish job's steps: `npm publish --dry-run` after `npm install --ignore-scripts` still runs prepublishOnly's release-hold check. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 32 +++++ .github/workflows/publish.yml | 41 +++++- CONTRIBUTING.md | 31 +++-- scripts/pack-installs-alone.mjs | 215 +++++++++++++++++++++++++++++++ test/pack-installs-alone.test.ts | 178 +++++++++++++++++++++++++ 5 files changed, 485 insertions(+), 12 deletions(-) create mode 100644 scripts/pack-installs-alone.mjs create mode 100644 test/pack-installs-alone.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2bd45fc08..e2284ee53 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -57,3 +57,35 @@ jobs: # later PR's qualification. - name: Test unforced (work in flight at stop) run: node --test dist/test/stop-with-work-in-flight.test.js dist/test/stop-with-host-operations.test.js dist/test/conversation-routing.test.js + + # The release as npm would pack it, installed on its own and loaded, as + # the publish workflow checks before it publishes (see + # scripts/pack-installs-alone.mjs): here a change that leaves a file out + # of the package, or imports what only devDependencies supply, fails + # before it merges, not after a tag and its release notes are out. The + # install resolves dependencies from the registry as it stands, so a new + # upstream release can turn it red on a change that touched nothing it + # installs: when it fails, rerunning it on main tells a change's own break + # from the registry's drift. + pack-check: + name: Packed release installs on its own + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 + with: + persist-credentials: false + + - name: Setup Node.js + uses: actions/setup-node@249970729cb0ef3589644e2896645e5dc5ba9c38 # v6.5.0 + with: + node-version: 24 + + - name: Install dependencies + run: npm install + + - name: Build + run: npm run build + + - name: Packed release installs on its own + run: node scripts/pack-installs-alone.mjs diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 4df046667..d4965a1ff 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -52,10 +52,44 @@ jobs: - name: Test run: npm test + # The tarball npm publish makes from this tree, installed alone into an + # empty directory with its dependencies from npm, and loaded: a file + # missing from `files`, a dependency only devDependencies supplied, or a + # bin npm's link can't run fails here, not for whoever installs the + # release. A job of its own, so the install scripts it runs never see the + # publish job's id-token; publish needs it, the GitHub release notes + # don't. It runs on a dispatch too, which publishes nothing. + pack-check: + name: Packed release installs on its own + runs-on: ubuntu-latest + permissions: + contents: read + + steps: + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 + with: + persist-credentials: false + + - name: Setup Node.js + uses: actions/setup-node@249970729cb0ef3589644e2896645e5dc5ba9c38 # v6.5.0 + with: + node-version: 24 + + - name: Install dependencies + run: npm install + + # npm publish builds again first, through prepublishOnly, from the + # same sources. + - name: Build + run: npm run build + + - name: Packed release installs on its own + run: node scripts/pack-installs-alone.mjs + publish: name: Publish to npm runs-on: ubuntu-latest - needs: build-and-test + needs: [build-and-test, pack-check] if: startsWith(github.ref, 'refs/tags/v') # OIDC trusted publishing: GitHub mints a short-lived id-token, npm CLI @@ -89,8 +123,11 @@ jobs: node-version: 24 registry-url: https://registry.npmjs.org + # --ignore-scripts: this job holds the id-token, and neither the build + # nor npm publish needs a dependency's install script. The pack-check + # job runs them, as an install from npm does, without the id-token. - name: Install dependencies - run: npm install + run: npm install --ignore-scripts - name: Build run: npm run build diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index cb4183a61..3a9c417fa 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -135,15 +135,23 @@ never do. (subsections emitted in `### Breaking` / `### Added` / `### Changed` / `### Fixed` order), deletes the consumed fragments, keeps a fresh empty `Unreleased` above, and refuses to release when there is nothing to - release; npm then commits and tags. `git push --follow-tags` triggers - CI, which refuses a tag with no matching changelog section, publishes - `@animalabs/agent-framework` to npm, and creates the GitHub release with - that section as its notes. The two release jobs are independent: some - consumers run github-clone checkouts, so release notes must exist even - when npm publish fails. Both need the build job, so a failed build stops - both; past it, so do a tag with no changelog section, which neither can - release, and an open release hold (below), which each checks. Version - bumps are a maintainer release-time action, not part of feature PRs. + release; npm then commits and tags. `git push --follow-tags` triggers CI, + which refuses a tag with no matching changelog section, checks in a job + of its own that the packed release installs on its own + (`node scripts/pack-installs-alone.mjs` packs it, installs the tarball + alone into an empty directory with its dependencies from npm, checks + that every file `package.json` names is in it, loads each entry point and + checks each bin's `#!` line; to run it by hand, `npm run build` first, + for example before `git push --follow-tags`, or dispatch the workflow on + a branch, where it publishes nothing), publishes + `@animalabs/agent-framework` to npm once that check has passed, and + creates the GitHub release with that section as its notes. The two + release jobs are independent: some consumers run github-clone checkouts, + so release notes must exist even when npm publish fails. Both need the + build job, so a failed build stops both; past it, so do a tag with no + changelog section, which neither can release, and an open release hold + (below), which each checks. Version bumps are a maintainer release-time + action, not part of feature PRs. - **Breaking changes.** A dependent's caret range takes the later versions of its line (0.22.x before 1.0, 1.x from 1.0, and 0.0.z alone), so a breaking change released as a patch reaches each dependent whose @@ -194,7 +202,10 @@ branches. The `pretest` hook copies `test/fixtures/*.mjs` into `dist/`. Push-time CI (`ci.yml`) builds, typechecks and tests every push and PR on ubuntu and macos. Lockfiles are deliberately gitignored in this repo, so CI -installs with plain `npm install` and there is no lock to keep in sync. +installs with plain `npm install` and there is no lock to keep in sync. It +also runs the pack check the publish workflow runs before publishing +(`node scripts/pack-installs-alone.mjs`, on ubuntu), so a file left out of +the package, or an import only `devDependencies` supply, fails the PR. When working against local sibling checkouts rather than published versions, see the ecosystem workspace setup — `bun install` at the `rust-connectome` diff --git a/scripts/pack-installs-alone.mjs b/scripts/pack-installs-alone.mjs new file mode 100644 index 000000000..f0a56c233 --- /dev/null +++ b/scripts/pack-installs-alone.mjs @@ -0,0 +1,215 @@ +// Packs this package as `npm publish` packs it, installs the tarball alone +// into an empty directory with its dependencies from the registry, as +// someone installing the release does, and checks what the installed +// package.json offers: +// - every file it names is in the package: `main`, `types`, each `exports` +// target under every condition (the `bun` and `types` targets too), at +// least one file for each export pattern, and each `bin`; +// - each entry point loads as a consumer loads it: the root and each +// subpath through import() (a JSON target with its import attribute), or +// through require() when only `require` serves it; +// - each bin starts with a `#!` line, without which npm's link can't run +// it as a command, and a node bin parses (`node --check`). Bins aren't +// run: running one, such as the MCP server, would start it. +// Before packing it checks that the files package.json names exist in this +// tree, so an unbuilt tree says so rather than looking like a packaging +// fault. The publish workflow runs it after its build and before +// `npm publish` (which builds again from the same sources first); to check +// by hand, run `npm run build` and then this, from the repository root. +import { execFileSync } from "node:child_process"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +const LOAD_TIMEOUT_MS = 60_000; +const INSTALL_TIMEOUT_MS = 10 * 60_000; + +class CheckError extends Error {} + +/** The target a set of conditions picks from an exports value, as Node + * resolves it: the first key, in order, that is active or `default`. */ +function pick(value, conditions) { + if (typeof value === "string") return value; + if (Array.isArray(value)) { + for (const item of value) { + const found = pick(item, conditions); + if (found !== undefined) return found; + } + return undefined; + } + if (value && typeof value === "object") { + for (const [key, inner] of Object.entries(value)) { + if (key === "default" || conditions.includes(key)) { + const found = pick(inner, conditions); + if (found !== undefined) return found; + } + } + } + return undefined; +} + +/** Every file target an exports value names, under any condition. */ +function allTargets(value, out = []) { + if (typeof value === "string") out.push(value); + else if (Array.isArray(value)) for (const item of value) allTargets(item, out); + else if (value && typeof value === "object") for (const inner of Object.values(value)) allTargets(inner, out); + return out; +} + +/** The package's subpaths, `.` included, each with its exports value: an + * exports that names no subpath (a string, an array, or an object of + * conditions, none of whose keys starts with '.') is the root's. A null + * subpath, which blocks its path, names no file and loads nothing. */ +function subpaths(exports) { + if (exports == null) return []; + if (!Object.keys(exports).some((key) => key.startsWith("."))) return [[".", exports]]; + return Object.entries(exports); +} + +/** `bin` as npm reads it: a path for the package's own name, or a map. */ +function binsOf(pkg) { + if (typeof pkg.bin === "string") return { [pkg.name.split("/").pop()]: pkg.bin }; + return pkg.bin && typeof pkg.bin === "object" ? pkg.bin : {}; +} + +/** The files package.json names (pattern exports aside), each with what names it. */ +function namedFiles(pkg) { + const named = []; + if (typeof pkg.main === "string") named.push([pkg.main, "main"]); + for (const field of ["types", "typings"]) if (typeof pkg[field] === "string") named.push([pkg[field], field]); + for (const [key, value] of subpaths(pkg.exports)) { + if (key.includes("*")) continue; + for (const target of allTargets(value)) named.push([target, `exports['${key}']`]); + } + for (const [bin, path] of Object.entries(binsOf(pkg))) named.push([path, `bin '${bin}'`]); + return named; +} + +/** Runs a command with its output going to ours; a failure throws `message`. */ +function run(file, args, options, message) { + try { + execFileSync(file, args, { stdio: ["ignore", "inherit", "inherit"], ...options }); + } catch (error) { + throw new CheckError(error?.code === "ETIMEDOUT" ? `${message.split(" (")[0]}: it didn't finish within ${options.timeout / 1000} s.` : message); + } +} + +function main() { + const source = JSON.parse(readFileSync("package.json", "utf8")); + const unbuilt = namedFiles(source).filter(([path]) => !existsSync(path)); + if (unbuilt.length > 0) { + throw new CheckError( + `${unbuilt.map(([path, by]) => `${path} (${by})`).join(", ")} ${unbuilt.length === 1 ? "doesn't" : "don't"} exist in this tree, ` + + "so there is nothing to pack yet: build first (npm run build), or correct what names it.", + ); + } + const work = mkdtempSync(join(tmpdir(), "pack-installs-alone-")); + try { + let packed; + try { + const out = execFileSync("npm", ["pack", "--json", "--pack-destination", work], { + encoding: "utf8", + stdio: ["ignore", "pipe", "inherit"], + maxBuffer: 64 * 1024 * 1024, + }); + packed = JSON.parse(out)[0]; + } catch (error) { + if (error?.stdout) process.stderr.write(error.stdout); + throw new CheckError("npm pack failed (its output is above)."); + } + const tarball = join(work, packed.filename); + const dir = join(work, "install"); + mkdirSync(dir); + writeFileSync(join(dir, "package.json"), JSON.stringify({ name: "pack-installs-alone", private: true })); + run( + "npm", + ["install", "--no-audit", "--no-fund", "--no-package-lock", tarball], + { cwd: dir, timeout: INSTALL_TIMEOUT_MS }, + `${packed.filename} didn't install on its own into an empty directory (npm's output is above). A dependency ` + + "that isn't on the registry does that, or a registry or network error, which a re-run gets past.", + ); + const installed = join(dir, "node_modules", ...packed.name.split("/")); + // What a consumer reads: the manifest in the tarball. + const pkg = JSON.parse(readFileSync(join(installed, "package.json"), "utf8")); + const files = packed.files.map((file) => file.path); + + // This tree has each named file (checked before packing), so one the + // package lacks is one `files` leaves out. + const missing = namedFiles(pkg).filter(([path]) => !existsSync(join(installed, path))); + for (const [key, value] of subpaths(pkg.exports)) { + if (!key.includes("*")) continue; + for (const target of allTargets(value)) { + const [head, tail] = target.replace(/^\.\//, "").split("*"); + if (!files.some((file) => file.startsWith(head) && file.endsWith(tail ?? ""))) { + missing.push([target, `exports['${key}'], a pattern no packed file matches`]); + } + } + } + if (missing.length > 0) { + throw new CheckError( + `${missing.map(([path, by]) => `${path} (${by})`).join(", ")} ${missing.length === 1 ? "isn't" : "aren't"} in ` + + `${packed.filename}: add ${missing.length === 1 ? "it" : "them"} to package.json's \`files\`.`, + ); + } + + // The entry points, loaded as a consumer loads them, each in its own process. + const loads = []; + const roots = subpaths(pkg.exports); + if (pkg.exports === undefined && (typeof pkg.main === "string" || existsSync(join(installed, "index.js")))) { + loads.push([pkg.name, "import"]); + } + for (const [key, value] of roots) { + if (key.includes("*")) continue; + const specifier = key === "." ? pkg.name : `${pkg.name}${key.slice(1)}`; + const imported = pick(value, ["node", "import"]); + if (imported !== undefined) loads.push([specifier, imported.endsWith(".json") ? "import json" : "import"]); + else if (pick(value, ["node", "require"]) !== undefined) loads.push([specifier, "require"]); + } + for (const [specifier, how] of loads) { + const code = + how === "require" + ? `require(${JSON.stringify(specifier)}); process.exit(0);` + : `await import(${JSON.stringify(specifier)}${how === "import json" ? ', { with: { type: "json" } }' : ""}); process.exit(0);`; + run( + process.execPath, + how === "require" ? ["-e", code] : ["--input-type=module", "-e", code], + { cwd: dir, timeout: LOAD_TIMEOUT_MS }, + `loading '${specifier}' (${how}) from the installed ${packed.filename} failed (above). A dependency only ` + + "devDependencies supplied does that, as does a file it imports that the package leaves out.", + ); + } + + const bins = binsOf(pkg); + for (const [bin, path] of Object.entries(bins)) { + const file = join(installed, path); + const first = readFileSync(file, "utf8").split("\n", 1)[0]; + if (!first.startsWith("#!")) { + throw new CheckError( + `bin '${bin}' (${path}) has no '#!' line, so npm's link can't run it as a command: start its source with '#!/usr/bin/env node'.`, + ); + } + if (/\bnode\b/.test(first)) { + run(process.execPath, ["--check", file], {}, `bin '${bin}' (${path}) in ${packed.filename} doesn't parse (above).`); + } + } + + const patterns = roots.filter(([key]) => key.includes("*")).map(([key]) => key); + console.log( + `${packed.filename} (${packed.entryCount} files) installs on its own; every file package.json names is in it` + + (loads.length > 0 ? `; loaded ${loads.map(([s, how]) => `'${s}' (${how})`).join(", ")}` : "") + + (Object.keys(bins).length > 0 ? `; bins start with '#!', and node bins parse: ${Object.keys(bins).join(", ")}` : "") + + (patterns.length > 0 ? `; export patterns matched, not loaded: ${patterns.join(", ")}` : "") + + ".", + ); + } finally { + rmSync(work, { recursive: true, force: true }); + } +} + +try { + main(); +} catch (error) { + if (!(error instanceof CheckError)) throw error; + console.error(`pack-installs-alone: ${error.message}`); + process.exitCode = 1; +} diff --git a/test/pack-installs-alone.test.ts b/test/pack-installs-alone.test.ts new file mode 100644 index 000000000..0dc2ab7f6 --- /dev/null +++ b/test/pack-installs-alone.test.ts @@ -0,0 +1,178 @@ +// Black-box coverage of scripts/pack-installs-alone.mjs: each case runs the +// real script in a throwaway package that has no dependencies, so npm packs +// and installs it offline, with its own cache and user config. +import { after, test } from "node:test"; +import assert from "node:assert/strict"; +import { execFile } from "node:child_process"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const SCRIPT = fileURLToPath(new URL("../../scripts/pack-installs-alone.mjs", import.meta.url)); + +const made: string[] = []; +after(() => { + for (const dir of made) rmSync(dir, { recursive: true, force: true }); +}); + +/** A throwaway package: package.json from `pkg`, and `files` (path to text). */ +function fixture(pkg: Record, files: Record): string { + const dir = mkdtempSync(join(tmpdir(), "pack-installs-alone-test-")); + made.push(dir); + writeFileSync(join(dir, "package.json"), JSON.stringify({ name: "@zz/fixture", version: "1.0.0", type: "module", ...pkg })); + for (const [path, text] of Object.entries(files)) { + mkdirSync(dirname(join(dir, path)), { recursive: true }); + writeFileSync(join(dir, path), text); + } + return dir; +} + +function check(dir: string): Promise<{ code: number; stdout: string; stderr: string }> { + // npm's home and cache, outside the package, so npm pack doesn't take them in. + const home = mkdtempSync(join(tmpdir(), "pack-installs-alone-home-")); + made.push(home); + const env = { + PATH: process.env.PATH ?? "", + HOME: home, + npm_config_cache: join(home, ".npm-cache"), + npm_config_userconfig: join(home, ".npmrc"), + npm_config_offline: "true", + npm_config_update_notifier: "false", + }; + return new Promise((resolve) => { + execFile(process.execPath, [SCRIPT], { cwd: dir, env }, (error, stdout, stderr) => { + resolve({ code: error ? Number(error.code ?? 1) : 0, stdout, stderr }); + }); + }); +} + +const ENTRY = { "dist/index.js": "export const ok = true;\n", "dist/extra.js": "export const extra = true;\n" }; +const NODE_BIN = "#!/usr/bin/env node\nthrow new Error('zz-serve ran');\n"; + +test("a package whose files carry everything it names installs alone, and each entry point loads", async () => { + const dir = fixture( + { + files: ["dist", "src"], + exports: { + ".": { bun: "./src/index.ts", types: "./dist/index.d.ts", import: "./dist/index.js" }, + "./extra": "./dist/extra.js", + "./package.json": "./package.json", + "./legacy": { require: "./dist/legacy.cjs" }, + "./types": { types: "./dist/types.d.ts" }, + "./internal": null, + "./parts/*": "./dist/*.js", + }, + bin: { "zz-serve": "./dist/serve.js", "zz-shell": "./dist/shell.sh" }, + }, + { + ...ENTRY, + "src/index.ts": "export const ok = true;\n", + "dist/index.d.ts": "export declare const ok: boolean;\n", + "dist/types.d.ts": "export type T = 1;\n", + "dist/legacy.cjs": "module.exports = { legacy: true };\n", + "dist/serve.js": NODE_BIN, + "dist/shell.sh": "#!/bin/sh\necho zz-shell\n", + }, + ); + const result = await check(dir); + assert.equal(result.code, 0, result.stderr); + assert.doesNotMatch(result.stderr, /zz-serve ran/, "a bin is never run"); + assert.match( + result.stdout, + /^zz-fixture-1\.0\.0\.tgz \(\d+ files\) installs on its own; every file package\.json names is in it; loaded '@zz\/fixture' \(import\), '@zz\/fixture\/extra' \(import\), '@zz\/fixture\/package\.json' \(import json\), '@zz\/fixture\/legacy' \(require\); bins start with '#!', and node bins parse: zz-serve, zz-shell; export patterns matched, not loaded: \.\/parts\/\*\.\n$/m, + ); +}); + +test("a string exports, an exports of conditions, and main without exports each load as the root", async () => { + for (const pkg of [ + { files: ["dist"], exports: "./dist/index.js" }, + { files: ["dist"], exports: { import: "./dist/index.js", default: "./dist/index.js" } }, + { files: ["dist"], main: "./dist/index.js" }, + ]) { + const result = await check(fixture(pkg, ENTRY)); + assert.equal(result.code, 0, result.stderr); + assert.match(result.stdout, /; loaded '@zz\/fixture' \(import\)\.\n$/m, JSON.stringify(pkg)); + } +}); + +test("with neither exports nor main, the root is loaded from its index.js, and a bin-only package loads nothing", async () => { + const fallback = await check(fixture({}, { "index.js": "export const ok = true;\n" })); + assert.equal(fallback.code, 0, fallback.stderr); + assert.match(fallback.stdout, /; loaded '@zz\/fixture' \(import\)\.\n$/m); + const binOnly = await check(fixture({ files: ["dist"], bin: "./dist/serve.js" }, { "dist/serve.js": NODE_BIN })); + assert.equal(binOnly.code, 0, binOnly.stderr); + assert.match(binOnly.stdout, /installs on its own; every file package\.json names is in it; bins start with '#!', and node bins parse: fixture\.\n$/m); +}); + +test("a tree that hasn't been built, or a main beside exports that names no file, says so before packing", async () => { + const main = await check(fixture({ files: ["dist"], main: "./dist/gone.js", exports: "./dist/index.js" }, ENTRY)); + assert.equal(main.code, 1); + assert.match(main.stderr, /^pack-installs-alone: \.\/dist\/gone\.js \(main\) doesn't exist in this tree/); + const result = await check(fixture({ files: ["dist"], exports: { ".": "./dist/index.js" }, bin: { "zz-serve": "./dist/serve.js" } }, {})); + assert.equal(result.code, 1); + assert.equal( + result.stderr, + "pack-installs-alone: ./dist/index.js (exports['.']), ./dist/serve.js (bin 'zz-serve') don't exist in this tree, so there is nothing to pack yet: build first (npm run build), or correct what names it.\n", + ); +}); + +test("a file the package names but leaves out of files fails, under any condition", async () => { + const cases: Array<[Record, Record, RegExp]> = [ + // the bun and types targets of the root, which no import() reads + [{ files: ["dist"], exports: { ".": { bun: "./src/index.ts", import: "./dist/index.js" } } }, { ...ENTRY, "src/index.ts": "" }, /\.\/src\/index\.ts \(exports\['\.'\]\) isn't in zz-fixture-1\.0\.0\.tgz: add it to package\.json's `files`\./], + [{ files: ["dist/index.js"], types: "./dist/index.d.ts", exports: "./dist/index.js" }, { ...ENTRY, "dist/index.d.ts": "" }, /\.\/dist\/index\.d\.ts \(types\) isn't in/], + // a pattern that matches nothing packed + [{ files: ["dist/index.js"], exports: { ".": "./dist/index.js", "./parts/*": "./parts/*.js" } }, { ...ENTRY, "parts/a.js": "" }, /\.\/parts\/\*\.js \(exports\['\.\/parts\/\*'\], a pattern no packed file matches\) isn't in/], + ]; + for (const [pkg, files, message] of cases) { + const result = await check(fixture(pkg, files)); + assert.equal(result.code, 1, JSON.stringify(pkg)); + assert.match(result.stderr, message); + } +}); + +test("an entry point that doesn't load fails, naming it: a devDependencies-only import, or a subpath's", async () => { + const devOnly = await check(fixture( + { files: ["dist"], main: "./dist/index.js", devDependencies: { "zz-dev-only": "1.0.0" } }, + { "dist/index.js": "import 'zz-dev-only';\nexport const ok = true;\n" }, + )); + assert.equal(devOnly.code, 1); + assert.match(devOnly.stderr, /Cannot find package 'zz-dev-only'/); + assert.match(devOnly.stderr, /pack-installs-alone: loading '@zz\/fixture' \(import\) from the installed zz-fixture-1\.0\.0\.tgz failed \(above\)\. A dependency only devDependencies supplied does that, as does a file it imports that the package leaves out\.\n$/); + + const subpath = await check(fixture( + { files: ["dist"], exports: { ".": "./dist/index.js", "./extra": "./dist/extra.js" } }, + { ...ENTRY, "dist/extra.js": "import './left-out.js';\n" }, + )); + assert.equal(subpath.code, 1); + assert.match(subpath.stderr, /pack-installs-alone: loading '@zz\/fixture\/extra' \(import\) from the installed/); +}); + +test("a bin needs a '#!' line, and a node bin must parse; none is run", async () => { + const noShebang = await check(fixture({ files: ["dist"], main: "./dist/index.js", bin: { "zz-serve": "./dist/serve.js" } }, { + ...ENTRY, "dist/serve.js": "/**\n * no shebang\n */\nexport {};\n", + })); + assert.equal(noShebang.code, 1); + assert.equal( + noShebang.stderr, + "pack-installs-alone: bin 'zz-serve' (./dist/serve.js) has no '#!' line, so npm's link can't run it as a command: start its source with '#!/usr/bin/env node'.\n", + ); + const unparsed = await check(fixture({ files: ["dist"], main: "./dist/index.js", bin: "./dist/serve.js" }, { + ...ENTRY, "dist/serve.js": "#!/usr/bin/env node\nexport const = ;\n", + })); + assert.equal(unparsed.code, 1); + assert.match(unparsed.stderr, /pack-installs-alone: bin 'fixture' \(\.\/dist\/serve\.js\) in zz-fixture-1\.0\.0\.tgz doesn't parse \(above\)\.\n$/); +}); + +test("a dependency that can't be installed fails the install, saying what does that", async () => { + const result = await check(fixture( + { files: ["dist"], main: "./dist/index.js", dependencies: { "zz-not-on-the-registry": "1.0.0" } }, + ENTRY, + )); + assert.equal(result.code, 1); + assert.match( + result.stderr, + /pack-installs-alone: zz-fixture-1\.0\.0\.tgz didn't install on its own into an empty directory \(npm's output is above\)\. A dependency that isn't on the registry does that, or a registry or network error, which a re-run gets past\.\n$/, + ); +}); From d2586958740b0f05a6cef100ee0e5d1342ccecc0 Mon Sep 17 00:00:00 2001 From: Ruth-2424 Date: Wed, 14 Oct 2026 11:39:34 -0400 Subject: [PATCH 2/5] build(release): the pack check reads repeated '*', null conditions and a JSON main as Node does From Greptile's review of #349: - An export pattern's target can repeat '*' (./dist/*/*.js), and Node puts the same text in place of each. The check matched packed files against the target's first '*' alone, so dist/a/a.js counted as missing; it now matches the whole target, with the same text at every '*'. - A null target under a matched condition blocks the subpath for those conditions, and stops Node's search. pick() read it as no match and went on to `default`, so a package with { import: null, require: ... } was imported, which Node refuses, instead of required. pick() now answers null there, and a subpath whose import is blocked loads by require() when require serves it. - A JSON main with no exports loads with the JSON import attribute, as a JSON exports target already did. Tests (test/pack-installs-alone.test.ts, 9): a case with a repeated-'*' pattern, a blocked import beside a require, and a JSON main, each passing; it fails with each of the three fixes removed. Co-Authored-By: Claude Opus 5.5 --- scripts/pack-installs-alone.mjs | 19 +++++++++++++------ test/pack-installs-alone.test.ts | 20 ++++++++++++++++++++ 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/scripts/pack-installs-alone.mjs b/scripts/pack-installs-alone.mjs index f0a56c233..6b8dff619 100644 --- a/scripts/pack-installs-alone.mjs +++ b/scripts/pack-installs-alone.mjs @@ -27,8 +27,12 @@ const INSTALL_TIMEOUT_MS = 10 * 60_000; class CheckError extends Error {} /** The target a set of conditions picks from an exports value, as Node - * resolves it: the first key, in order, that is active or `default`. */ + * resolves it: the first key, in order, that is active or `default`. A + * `null` it reaches blocks the subpath for those conditions, so it answers + * null, which stops the search as Node's does; undefined means no key + * matched. */ function pick(value, conditions) { + if (value === null) return null; if (typeof value === "string") return value; if (Array.isArray(value)) { for (const item of value) { @@ -139,8 +143,10 @@ function main() { for (const [key, value] of subpaths(pkg.exports)) { if (!key.includes("*")) continue; for (const target of allTargets(value)) { - const [head, tail] = target.replace(/^\.\//, "").split("*"); - if (!files.some((file) => file.startsWith(head) && file.endsWith(tail ?? ""))) { + // Node puts the same text in place of every `*` in a target. + const parts = target.replace(/^\.\//, "").split("*").map((part) => part.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")); + const matches = new RegExp(`^${parts[0]}${parts.slice(1).map((part, i) => (i === 0 ? `(.+)${part}` : `\\1${part}`)).join("")}$`); + if (!files.some((file) => matches.test(file))) { missing.push([target, `exports['${key}'], a pattern no packed file matches`]); } } @@ -156,14 +162,15 @@ function main() { const loads = []; const roots = subpaths(pkg.exports); if (pkg.exports === undefined && (typeof pkg.main === "string" || existsSync(join(installed, "index.js")))) { - loads.push([pkg.name, "import"]); + loads.push([pkg.name, typeof pkg.main === "string" && pkg.main.endsWith(".json") ? "import json" : "import"]); } for (const [key, value] of roots) { if (key.includes("*")) continue; const specifier = key === "." ? pkg.name : `${pkg.name}${key.slice(1)}`; const imported = pick(value, ["node", "import"]); - if (imported !== undefined) loads.push([specifier, imported.endsWith(".json") ? "import json" : "import"]); - else if (pick(value, ["node", "require"]) !== undefined) loads.push([specifier, "require"]); + const required = pick(value, ["node", "require"]); + if (typeof imported === "string") loads.push([specifier, imported.endsWith(".json") ? "import json" : "import"]); + else if (typeof required === "string") loads.push([specifier, "require"]); } for (const [specifier, how] of loads) { const code = diff --git a/test/pack-installs-alone.test.ts b/test/pack-installs-alone.test.ts index 0dc2ab7f6..ed09ad865 100644 --- a/test/pack-installs-alone.test.ts +++ b/test/pack-installs-alone.test.ts @@ -96,6 +96,26 @@ test("a string exports, an exports of conditions, and main without exports each } }); +test("a pattern whose target repeats '*', an import blocked by null beside a require, and a JSON main each pass", async () => { + const repeated = await check( + fixture({ files: ["dist"], exports: { ".": "./dist/index.js", "./parts/*": "./dist/*/*.js" } }, { ...ENTRY, "dist/a/a.js": "export const a = 1;\n" }), + ); + assert.equal(repeated.code, 0, repeated.stderr); + assert.match(repeated.stdout, /; export patterns matched, not loaded: \.\/parts\/\*\.\n$/m); + // Node stops at the null that `import` reaches, so a consumer can only require it. + const blocked = await check( + fixture( + { files: ["dist"], exports: { ".": { import: null, require: "./dist/legacy.cjs", default: "./dist/index.js" } } }, + { ...ENTRY, "dist/legacy.cjs": "module.exports = { legacy: true };\n" }, + ), + ); + assert.equal(blocked.code, 0, blocked.stderr); + assert.match(blocked.stdout, /; loaded '@zz\/fixture' \(require\)\.\n$/m); + const json = await check(fixture({ files: ["data.json"], main: "./data.json" }, { "data.json": '{"ok":true}\n' })); + assert.equal(json.code, 0, json.stderr); + assert.match(json.stdout, /; loaded '@zz\/fixture' \(import json\)\.\n$/m); +}); + test("with neither exports nor main, the root is loaded from its index.js, and a bin-only package loads nothing", async () => { const fallback = await check(fixture({}, { "index.js": "export const ok = true;\n" })); assert.equal(fallback.code, 0, fallback.stderr); From 9e63d8dc009f6e177949ed53ab44724624a0976a Mon Sep 17 00:00:00 2001 From: Ruth-2424 Date: Fri, 16 Oct 2026 01:07:40 -0400 Subject: [PATCH 3/5] build(release): the pack check after its review's round 1: a nested install, npm 12's pack JSON, bins and the bun target loaded Answers Anarchid's round-1 review of #349 (R1-1 to R1-5). - R1-1: the install is nested (--install-strategy=nested), so the package reaches only its own dependencies. A package it imports but lists only in devDependencies now fails to load even when another dependency brings that package in, as context-manager brings chronicle and membrane, and discord.js brings ws. - R1-2: npm pack runs with --foreground-scripts=false, so a prepack's output can't land ahead of the JSON, and its --json output is read as npm 11's array or npm 12's object keyed by name. Anything else that isn't exactly one record with a filename fails by name, instead of throwing. - R1-3: a node bin's imports are loaded without running the bin: a load hook (module.register) puts a throw after its '#!' line, and ESM loads a module's imports before its body, so the bin throws a sentinel once its imports have loaded and before any of its own code. With bun on PATH, each subpath whose exports names a `bun` condition is imported under bun too, and the pack-check jobs set up bun 1.3.14 for that; without bun, the success line says the bun targets were checked for presence only. Every declaration file the build wrote beside `types` must be in the package. `exports: null` falls back to `main`, as Node does. - R1-4: the publish workflow says, beside its publish step, that it publishes from the tree, and that publishing the checked tarball would skip prepublishOnly's release-hold check. - R1-5: a failed load names a dependency's new release among its causes, with a rerun on main to tell. Tests (test/pack-installs-alone.test.ts, 13): a package imported only through another dependency's copy fails, and passes once listed; a bin whose imports the root never reaches fails to load, and one whose imports load isn't run; a bun target loads under bun, with a stand-in for bun that imports under Node's `bun` condition, and one that doesn't load fails by name; a declaration left out fails; `exports: null` loads `main`; npm 12's JSON shape and a chatty prepack pass. Each piece, removed on its own, fails the test aimed at it. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 6 ++ .github/workflows/publish.yml | 18 +++- CONTRIBUTING.md | 13 +-- scripts/pack-installs-alone.mjs | 139 +++++++++++++++++++++++++++---- test/pack-installs-alone.test.ts | 116 +++++++++++++++++++++++--- 5 files changed, 257 insertions(+), 35 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e2284ee53..9190334fd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -87,5 +87,11 @@ jobs: - name: Build run: npm run build + # bun loads the package's `bun` export target, which Node never reads. + - name: Setup Bun + uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 + with: + bun-version: 1.3.14 + - name: Packed release installs on its own run: node scripts/pack-installs-alone.mjs diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index d4965a1ff..235d4e188 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -53,10 +53,11 @@ jobs: run: npm test # The tarball npm publish makes from this tree, installed alone into an - # empty directory with its dependencies from npm, and loaded: a file - # missing from `files`, a dependency only devDependencies supplied, or a - # bin npm's link can't run fails here, not for whoever installs the - # release. A job of its own, so the install scripts it runs never see the + # empty directory with its dependencies from npm (nested, so the package + # reaches only its own), and loaded, under Node and, for its bun target, + # bun: a file missing from `files`, a dependency only devDependencies + # supplied, or a bin npm's link can't run fails here, not for whoever + # installs the release. A job of its own, so the install scripts it runs never see the # publish job's id-token; publish needs it, the GitHub release notes # don't. It runs on a dispatch too, which publishes nothing. pack-check: @@ -83,6 +84,12 @@ jobs: - name: Build run: npm run build + # bun loads the package's `bun` export target, which Node never reads. + - name: Setup Bun + uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 + with: + bun-version: 1.3.14 + - name: Packed release installs on its own run: node scripts/pack-installs-alone.mjs @@ -132,6 +139,9 @@ jobs: - name: Build run: npm run build + # This publishes from the tree, not the tarball pack-check checked: npm + # skips a package's own lifecycle scripts for a tarball, so publishing + # one would also skip prepublishOnly's release-hold check. - name: Publish run: npm publish --access public --provenance env: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3a9c417fa..ddac7d9c0 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -139,11 +139,14 @@ never do. which refuses a tag with no matching changelog section, checks in a job of its own that the packed release installs on its own (`node scripts/pack-installs-alone.mjs` packs it, installs the tarball - alone into an empty directory with its dependencies from npm, checks - that every file `package.json` names is in it, loads each entry point and - checks each bin's `#!` line; to run it by hand, `npm run build` first, - for example before `git push --follow-tags`, or dispatch the workflow on - a branch, where it publishes nothing), publishes + alone into an empty directory with its dependencies from npm, nested so + the package reaches only its own, checks that every file `package.json` + names is in it, declaration files included, loads each entry point under + Node and, with bun on PATH, its `bun` target under bun, and checks that + each bin has its `#!` line and that a node bin's imports load, without + running it; to run it by hand, `npm run build` first, for example before + `git push --follow-tags`, or dispatch the workflow on a branch, where it + publishes nothing), publishes `@animalabs/agent-framework` to npm once that check has passed, and creates the GitHub release with that section as its notes. The two release jobs are independent: some consumers run github-clone checkouts, diff --git a/scripts/pack-installs-alone.mjs b/scripts/pack-installs-alone.mjs index 6b8dff619..9a752125f 100644 --- a/scripts/pack-installs-alone.mjs +++ b/scripts/pack-installs-alone.mjs @@ -1,28 +1,45 @@ // Packs this package as `npm publish` packs it, installs the tarball alone // into an empty directory with its dependencies from the registry, as // someone installing the release does, and checks what the installed -// package.json offers: +// package.json offers. The install is nested (npm's --install-strategy +// nested), so the package reaches only its own dependencies: a package it +// imports but only devDependencies lists fails to load even when another +// dependency brings that package in. // - every file it names is in the package: `main`, `types`, each `exports` // target under every condition (the `bun` and `types` targets too), at -// least one file for each export pattern, and each `bin`; +// least one file for each export pattern, each `bin`, and each +// declaration file the build wrote beside `types`; // - each entry point loads as a consumer loads it: the root and each // subpath through import() (a JSON target with its import attribute), or -// through require() when only `require` serves it; +// through require() when only `require` serves it, and, when bun is on +// PATH, each `bun` target through bun's import(); // - each bin starts with a `#!` line, without which npm's link can't run -// it as a command, and a node bin parses (`node --check`). Bins aren't -// run: running one, such as the MCP server, would start it. +// it as a command, and a node bin's imports load. Bins aren't run: a +// load hook makes the bin throw as soon as its imports have loaded, +// before its own code, which for the MCP server would start it. // Before packing it checks that the files package.json names exist in this // tree, so an unbuilt tree says so rather than looking like a packaging // fault. The publish workflow runs it after its build and before // `npm publish` (which builds again from the same sources first); to check // by hand, run `npm run build` and then this, from the repository root. -import { execFileSync } from "node:child_process"; -import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { execFileSync, spawnSync } from "node:child_process"; +import { existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, realpathSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { dirname, join, relative } from "node:path"; +import { pathToFileURL } from "node:url"; const LOAD_TIMEOUT_MS = 60_000; const INSTALL_TIMEOUT_MS = 10 * 60_000; +// What the load hook makes a bin throw once its imports have loaded. +const LINKED = "__pack_installs_alone_linked__"; +const HOOK = `export async function load(url, context, nextLoad) { + const result = await nextLoad(url, context); + if (url !== process.env.PACK_INSTALLS_ALONE_BIN || result.format !== "module") return result; + const lines = String(result.source).split("\\n"); + lines.splice(lines[0].startsWith("#!") ? 1 : 0, 0, 'throw "${LINKED}";'); + return { ...result, source: lines.join("\\n") }; +} +`; class CheckError extends Error {} @@ -89,6 +106,30 @@ function namedFiles(pkg) { return named; } +/** Each declaration file under `dir` in this tree, as a path from here. */ +function declarations(dir) { + if (!existsSync(dir)) return []; + const out = []; + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const path = join(dir, entry.name); + if (entry.isDirectory()) out.push(...declarations(path)); + else if (entry.name.endsWith(".d.ts")) out.push(path); + } + return out; +} + +/** Whether an exports value names `condition` anywhere. */ +function hasCondition(value, condition) { + if (Array.isArray(value)) return value.some((item) => hasCondition(item, condition)); + if (!value || typeof value !== "object") return false; + return Object.entries(value).some(([key, inner]) => key === condition || hasCondition(inner, condition)); +} + +/** Whether `bun` runs here. */ +function bunOnPath() { + return spawnSync("bun", ["--version"], { stdio: "ignore" }).status === 0; +} + /** Runs a command with its output going to ours; a failure throws `message`. */ function run(file, args, options, message) { try { @@ -109,25 +150,39 @@ function main() { } const work = mkdtempSync(join(tmpdir(), "pack-installs-alone-")); try { - let packed; + let out = ""; try { - const out = execFileSync("npm", ["pack", "--json", "--pack-destination", work], { + // Lifecycle scripts' output would land ahead of the JSON in the + // foreground, so they run in the background. + out = execFileSync("npm", ["pack", "--json", "--foreground-scripts=false", "--pack-destination", work], { encoding: "utf8", stdio: ["ignore", "pipe", "inherit"], maxBuffer: 64 * 1024 * 1024, }); - packed = JSON.parse(out)[0]; } catch (error) { if (error?.stdout) process.stderr.write(error.stdout); throw new CheckError("npm pack failed (its output is above)."); } + // npm 11 prints an array of records, npm 12 an object keyed by name. + let records; + try { + const parsed = JSON.parse(out); + records = Array.isArray(parsed) ? parsed : Object.values(parsed ?? {}); + } catch { + records = []; + } + if (records.length !== 1 || typeof records[0]?.filename !== "string") { + process.stderr.write(out); + throw new CheckError("npm pack --json didn't print one record with a filename (its output is above)."); + } + const packed = records[0]; const tarball = join(work, packed.filename); const dir = join(work, "install"); mkdirSync(dir); writeFileSync(join(dir, "package.json"), JSON.stringify({ name: "pack-installs-alone", private: true })); run( "npm", - ["install", "--no-audit", "--no-fund", "--no-package-lock", tarball], + ["install", "--no-audit", "--no-fund", "--no-package-lock", "--install-strategy=nested", tarball], { cwd: dir, timeout: INSTALL_TIMEOUT_MS }, `${packed.filename} didn't install on its own into an empty directory (npm's output is above). A dependency ` + "that isn't on the registry does that, or a registry or network error, which a re-run gets past.", @@ -140,6 +195,16 @@ function main() { // This tree has each named file (checked before packing), so one the // package lacks is one `files` leaves out. const missing = namedFiles(pkg).filter(([path]) => !existsSync(join(installed, path))); + // The declaration files the build wrote beside `types`, which it imports. + const types = typeof pkg.types === "string" ? pkg.types : typeof pkg.typings === "string" ? pkg.typings : undefined; + if (types !== undefined) { + for (const declaration of declarations(dirname(types))) { + const path = relative(".", declaration); + // `types` itself is already named above. + if (path === relative(".", types)) continue; + if (!existsSync(join(installed, path))) missing.push([path, `a declaration beside ${types}`]); + } + } for (const [key, value] of subpaths(pkg.exports)) { if (!key.includes("*")) continue; for (const target of allTargets(value)) { @@ -161,7 +226,7 @@ function main() { // The entry points, loaded as a consumer loads them, each in its own process. const loads = []; const roots = subpaths(pkg.exports); - if (pkg.exports === undefined && (typeof pkg.main === "string" || existsSync(join(installed, "index.js")))) { + if (pkg.exports == null && (typeof pkg.main === "string" || existsSync(join(installed, "index.js")))) { loads.push([pkg.name, typeof pkg.main === "string" && pkg.main.endsWith(".json") ? "import json" : "import"]); } for (const [key, value] of roots) { @@ -182,7 +247,24 @@ function main() { how === "require" ? ["-e", code] : ["--input-type=module", "-e", code], { cwd: dir, timeout: LOAD_TIMEOUT_MS }, `loading '${specifier}' (${how}) from the installed ${packed.filename} failed (above). A dependency only ` + - "devDependencies supplied does that, as does a file it imports that the package leaves out.", + "devDependencies supplied does that, as does a file it imports that the package leaves out, or a " + + "dependency's new release (rerun this on main to tell).", + ); + } + // bun reads the `bun` condition, which Node never does. + const bunLoads = []; + const bun = bunOnPath(); + for (const [key, value] of roots) { + if (key.includes("*") || !hasCondition(value, "bun")) continue; + const specifier = key === "." ? pkg.name : `${pkg.name}${key.slice(1)}`; + bunLoads.push(specifier); + if (!bun) continue; + run( + "bun", + ["-e", `await import(${JSON.stringify(specifier)}); process.exit(0);`], + { cwd: dir, timeout: LOAD_TIMEOUT_MS }, + `loading '${specifier}' under bun, from its bun target, in the installed ${packed.filename} failed (above). ` + + "A file it imports that the package leaves out does that, or a dependency only devDependencies supplied.", ); } @@ -196,7 +278,27 @@ function main() { ); } if (/\bnode\b/.test(first)) { - run(process.execPath, ["--check", file], {}, `bin '${bin}' (${path}) in ${packed.filename} doesn't parse (above).`); + // The hook makes the bin throw once its imports have loaded, so its + // own code never runs; any other failure is the bin's. + const hook = join(work, "bin-hook.mjs"); + const register = join(work, "bin-register.mjs"); + writeFileSync(hook, HOOK); + writeFileSync(register, `import { register } from "node:module";\nregister(${JSON.stringify(pathToFileURL(hook).href)});\n`); + const loaded = spawnSync(process.execPath, ["--import", pathToFileURL(register).href, file], { + cwd: dir, + encoding: "utf8", + timeout: LOAD_TIMEOUT_MS, + env: { ...process.env, PACK_INSTALLS_ALONE_BIN: pathToFileURL(realpathSync(file)).href }, + }); + if (!`${loaded.stderr}`.includes(LINKED)) { + process.stderr.write(loaded.stderr ?? ""); + throw new CheckError( + loaded.error?.code === "ETIMEDOUT" + ? `loading bin '${bin}' (${path}) in ${packed.filename} didn't finish within ${LOAD_TIMEOUT_MS / 1000} s.` + : `bin '${bin}' (${path}) in ${packed.filename} doesn't load (above): a package it imports that only ` + + "devDependencies supplies does that, as does a file the package leaves out, or a syntax error.", + ); + } } } @@ -204,7 +306,12 @@ function main() { console.log( `${packed.filename} (${packed.entryCount} files) installs on its own; every file package.json names is in it` + (loads.length > 0 ? `; loaded ${loads.map(([s, how]) => `'${s}' (${how})`).join(", ")}` : "") + - (Object.keys(bins).length > 0 ? `; bins start with '#!', and node bins parse: ${Object.keys(bins).join(", ")}` : "") + + (bunLoads.length > 0 + ? bun + ? `; loaded under bun: ${bunLoads.map((s) => `'${s}'`).join(", ")}` + : `; bun targets present, not loaded (bun isn't on PATH): ${bunLoads.map((s) => `'${s}'`).join(", ")}` + : "") + + (Object.keys(bins).length > 0 ? `; bins start with '#!', and node bins' imports load: ${Object.keys(bins).join(", ")}` : "") + (patterns.length > 0 ? `; export patterns matched, not loaded: ${patterns.join(", ")}` : "") + ".", ); diff --git a/test/pack-installs-alone.test.ts b/test/pack-installs-alone.test.ts index ed09ad865..2df832d73 100644 --- a/test/pack-installs-alone.test.ts +++ b/test/pack-installs-alone.test.ts @@ -1,9 +1,12 @@ // Black-box coverage of scripts/pack-installs-alone.mjs: each case runs the -// real script in a throwaway package that has no dependencies, so npm packs -// and installs it offline, with its own cache and user config. +// real script in a throwaway package whose dependencies, if any, are local +// tarballs, so npm packs and installs it offline, with its own cache and +// user config. A `bun` shim in front of PATH decides whether bun is there: +// by default it isn't; BUN_BY_NODE stands bun in with Node under the `bun` +// condition, so a bun target loads as bun would load it. import { after, test } from "node:test"; import assert from "node:assert/strict"; -import { execFile } from "node:child_process"; +import { execFile, execFileSync } from "node:child_process"; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; @@ -28,12 +31,25 @@ function fixture(pkg: Record, files: Record): s return dir; } -function check(dir: string): Promise<{ code: number; stdout: string; stderr: string }> { +/** A directory holding one executable `name` with `text`. */ +function shim(name: string, text: string): string { + const dir = mkdtempSync(join(tmpdir(), "pack-installs-alone-shim-")); + made.push(dir); + writeFileSync(join(dir, name), text, { mode: 0o755 }); + return dir; +} +const NO_BUN = shim("bun", "#!/bin/sh\nexit 127\n"); +const BUN_BY_NODE = shim( + "bun", + `#!/bin/sh\n[ "$1" = --version ] && { echo 0.0.0-node; exit 0; }\n[ "$1" = -e ] && exec ${JSON.stringify(process.execPath)} --conditions=bun --input-type=module -e "$2"\nexit 2\n`, +); + +function check(dir: string, bun = NO_BUN, extraPath = ""): Promise<{ code: number; stdout: string; stderr: string }> { // npm's home and cache, outside the package, so npm pack doesn't take them in. const home = mkdtempSync(join(tmpdir(), "pack-installs-alone-home-")); made.push(home); const env = { - PATH: process.env.PATH ?? "", + PATH: `${extraPath}${bun}:${process.env.PATH ?? ""}`, HOME: home, npm_config_cache: join(home, ".npm-cache"), npm_config_userconfig: join(home, ".npmrc"), @@ -80,7 +96,7 @@ test("a package whose files carry everything it names installs alone, and each e assert.doesNotMatch(result.stderr, /zz-serve ran/, "a bin is never run"); assert.match( result.stdout, - /^zz-fixture-1\.0\.0\.tgz \(\d+ files\) installs on its own; every file package\.json names is in it; loaded '@zz\/fixture' \(import\), '@zz\/fixture\/extra' \(import\), '@zz\/fixture\/package\.json' \(import json\), '@zz\/fixture\/legacy' \(require\); bins start with '#!', and node bins parse: zz-serve, zz-shell; export patterns matched, not loaded: \.\/parts\/\*\.\n$/m, + /^zz-fixture-1\.0\.0\.tgz \(\d+ files\) installs on its own; every file package\.json names is in it; loaded '@zz\/fixture' \(import\), '@zz\/fixture\/extra' \(import\), '@zz\/fixture\/package\.json' \(import json\), '@zz\/fixture\/legacy' \(require\); bun targets present, not loaded \(bun isn't on PATH\): '@zz\/fixture'; bins start with '#!', and node bins' imports load: zz-serve, zz-shell; export patterns matched, not loaded: \.\/parts\/\*\.\n$/m, ); }); @@ -122,7 +138,7 @@ test("with neither exports nor main, the root is loaded from its index.js, and a assert.match(fallback.stdout, /; loaded '@zz\/fixture' \(import\)\.\n$/m); const binOnly = await check(fixture({ files: ["dist"], bin: "./dist/serve.js" }, { "dist/serve.js": NODE_BIN })); assert.equal(binOnly.code, 0, binOnly.stderr); - assert.match(binOnly.stdout, /installs on its own; every file package\.json names is in it; bins start with '#!', and node bins parse: fixture\.\n$/m); + assert.match(binOnly.stdout, /installs on its own; every file package\.json names is in it; bins start with '#!', and node bins' imports load: fixture\.\n$/m); }); test("a tree that hasn't been built, or a main beside exports that names no file, says so before packing", async () => { @@ -159,7 +175,7 @@ test("an entry point that doesn't load fails, naming it: a devDependencies-only )); assert.equal(devOnly.code, 1); assert.match(devOnly.stderr, /Cannot find package 'zz-dev-only'/); - assert.match(devOnly.stderr, /pack-installs-alone: loading '@zz\/fixture' \(import\) from the installed zz-fixture-1\.0\.0\.tgz failed \(above\)\. A dependency only devDependencies supplied does that, as does a file it imports that the package leaves out\.\n$/); + assert.match(devOnly.stderr, /pack-installs-alone: loading '@zz\/fixture' \(import\) from the installed zz-fixture-1\.0\.0\.tgz failed \(above\)\. A dependency only devDependencies supplied does that, as does a file it imports that the package leaves out, or a dependency's new release \(rerun this on main to tell\)\.\n$/); const subpath = await check(fixture( { files: ["dist"], exports: { ".": "./dist/index.js", "./extra": "./dist/extra.js" } }, @@ -169,7 +185,7 @@ test("an entry point that doesn't load fails, naming it: a devDependencies-only assert.match(subpath.stderr, /pack-installs-alone: loading '@zz\/fixture\/extra' \(import\) from the installed/); }); -test("a bin needs a '#!' line, and a node bin must parse; none is run", async () => { +test("a bin needs a '#!' line, and a node bin's imports must load; none is run", async () => { const noShebang = await check(fixture({ files: ["dist"], main: "./dist/index.js", bin: { "zz-serve": "./dist/serve.js" } }, { ...ENTRY, "dist/serve.js": "/**\n * no shebang\n */\nexport {};\n", })); @@ -182,7 +198,87 @@ test("a bin needs a '#!' line, and a node bin must parse; none is run", async () ...ENTRY, "dist/serve.js": "#!/usr/bin/env node\nexport const = ;\n", })); assert.equal(unparsed.code, 1); - assert.match(unparsed.stderr, /pack-installs-alone: bin 'fixture' \(\.\/dist\/serve\.js\) in zz-fixture-1\.0\.0\.tgz doesn't parse \(above\)\.\n$/); + assert.match(unparsed.stderr, /pack-installs-alone: bin 'fixture' \(\.\/dist\/serve\.js\) in zz-fixture-1\.0\.0\.tgz doesn't load \(above\): a package it imports that only devDependencies supplies does that, as does a file the package leaves out, or a syntax error\.\n$/); + // A bin whose imports the root never reaches: the root loads, the bin doesn't. + const devOnlyBin = await check(fixture( + { files: ["dist"], main: "./dist/index.js", bin: { "zz-serve": "./dist/serve.js" }, devDependencies: { "zz-dev-only": "1.0.0" } }, + { ...ENTRY, "dist/serve.js": "#!/usr/bin/env node\nimport 'zz-dev-only';\nthrow new Error('zz-serve ran');\n" }, + )); + assert.equal(devOnlyBin.code, 1); + assert.match(devOnlyBin.stderr, /Cannot find package 'zz-dev-only'/); + assert.doesNotMatch(devOnlyBin.stderr, /zz-serve ran/); + assert.match(devOnlyBin.stderr, /pack-installs-alone: bin 'zz-serve' \(\.\/dist\/serve\.js\) in zz-fixture-1\.0\.0\.tgz doesn't load \(above\)/); + // One whose imports load is never run either. + const imports = await check(fixture( + { files: ["dist"], main: "./dist/index.js", bin: { "zz-serve": "./dist/serve.js" } }, + { ...ENTRY, "dist/serve.js": "#!/usr/bin/env node\nimport { ok } from './index.js';\nthrow new Error('zz-serve ran ' + ok);\n" }, + )); + assert.equal(imports.code, 0, imports.stderr); + assert.doesNotMatch(imports.stderr, /zz-serve ran/); +}); + +test("a package imported only through another dependency's copy fails, since the install is nested", async () => { + // zz-cm depends on zz-chronicle; the fixture imports zz-chronicle but lists + // it only in devDependencies, so a hoisted install would hand it zz-cm's. + const tarballs = mkdtempSync(join(tmpdir(), "pack-installs-alone-deps-")); + made.push(tarballs); + const pack = (pkg: Record, files: Record) => { + const dir = fixture(pkg, files); + const out = execFileSync("npm", ["pack", "--json", "--pack-destination", tarballs], { cwd: dir, encoding: "utf8", env: { ...process.env, npm_config_offline: "true" } }); + const parsed = JSON.parse(out); + return join(tarballs, (Array.isArray(parsed) ? parsed[0] : Object.values(parsed)[0] as { filename: string }).filename); + }; + const chronicle = pack({ name: "zz-chronicle", main: "./index.js" }, { "index.js": "export const chronicle = true;\n" }); + const cm = pack({ name: "zz-cm", main: "./index.js", dependencies: { "zz-chronicle": `file:${chronicle}` } }, { "index.js": "import 'zz-chronicle';\nexport const cm = true;\n" }); + const imports = { "dist/index.js": "import 'zz-cm';\nimport 'zz-chronicle';\nexport const ok = true;\n" }; + const devOnly = await check(fixture( + { files: ["dist"], main: "./dist/index.js", dependencies: { "zz-cm": `file:${cm}` }, devDependencies: { "zz-chronicle": `file:${chronicle}` } }, + imports, + )); + assert.equal(devOnly.code, 1); + assert.match(devOnly.stderr, /Cannot find package 'zz-chronicle'/); + const listed = await check(fixture( + { files: ["dist"], main: "./dist/index.js", dependencies: { "zz-cm": `file:${cm}`, "zz-chronicle": `file:${chronicle}` } }, + imports, + )); + assert.equal(listed.code, 0, listed.stderr); +}); + +test("with bun on PATH, a bun target is loaded under bun, and one that doesn't load fails, naming it", async () => { + const pkg = { files: ["dist", "src"], exports: { ".": { bun: "./src/index.js", import: "./dist/index.js" } } }; + const loads = await check(fixture(pkg, { ...ENTRY, "src/index.js": "export const ok = 'bun';\n" }), BUN_BY_NODE); + assert.equal(loads.code, 0, loads.stderr); + assert.match(loads.stdout, /; loaded '@zz\/fixture' \(import\); loaded under bun: '@zz\/fixture'\.\n$/m); + const broken = await check(fixture(pkg, { ...ENTRY, "src/index.js": "import './left-out.js';\n" }), BUN_BY_NODE); + assert.equal(broken.code, 1); + assert.match(broken.stderr, /pack-installs-alone: loading '@zz\/fixture' under bun, from its bun target, in the installed zz-fixture-1\.0\.0\.tgz failed \(above\)\./); +}); + +test("a declaration file the build wrote beside types, left out of files, fails, and exports: null falls back to main", async () => { + const declarations = await check(fixture( + { files: ["dist/**/*.js", "dist/index.d.ts"], main: "./dist/index.js", types: "./dist/index.d.ts" }, + { ...ENTRY, "dist/index.d.ts": "export * from './extra.js';\n", "dist/extra.d.ts": "export declare const extra: boolean;\n" }, + )); + assert.equal(declarations.code, 1); + assert.match(declarations.stderr, /dist\/extra\.d\.ts \(a declaration beside \.\/dist\/index\.d\.ts\) isn't in zz-fixture-1\.0\.0\.tgz/); + const nullExports = await check(fixture({ files: ["dist"], main: "./dist/index.js", exports: null }, ENTRY)); + assert.equal(nullExports.code, 0, nullExports.stderr); + assert.match(nullExports.stdout, /; loaded '@zz\/fixture' \(import\)\.\n$/m); +}); + +test("npm pack's JSON is read in npm 12's shape too, and a prepack's output doesn't break it", async () => { + // npm 12 prints an object keyed by package name: a shim in front of the + // real npm rewrites npm 11's array that way. + const realNpm = execFileSync("sh", ["-c", "command -v npm"], { encoding: "utf8" }).trim(); + const npm12 = shim( + "npm", + `#!/usr/bin/env node\nconst { execFileSync } = require("node:child_process");\nconst args = process.argv.slice(2);\ntry {\n const out = execFileSync(${JSON.stringify(realNpm)}, args, { encoding: "utf8", stdio: ["inherit", "pipe", "inherit"] });\n if (args[0] === "pack" && args.includes("--json")) {\n const [record] = JSON.parse(out);\n process.stdout.write(JSON.stringify({ [record.name]: record }));\n } else process.stdout.write(out);\n} catch (error) { process.stdout.write(error.stdout ?? ""); process.exit(error.status ?? 1); }\n`, + ); + const twelve = await check(fixture({ files: ["dist"], main: "./dist/index.js" }, ENTRY), NO_BUN, `${npm12}:`); + assert.equal(twelve.code, 0, twelve.stderr); + assert.match(twelve.stdout, /^zz-fixture-1\.0\.0\.tgz \(\d+ files\) installs on its own;/m); + const chatty = await check(fixture({ files: ["dist"], main: "./dist/index.js", scripts: { prepack: "echo zz: packing" } }, ENTRY)); + assert.equal(chatty.code, 0, chatty.stderr); }); test("a dependency that can't be installed fails the install, saying what does that", async () => { From f5267d7be7ce020549d26bddf4d730810e075a32 Mon Sep 17 00:00:00 2001 From: Ruth-2424 Date: Fri, 16 Oct 2026 02:01:02 -0400 Subject: [PATCH 4/5] build(release): a CommonJS bin is parsed, never run; every load failure names a new release; pack-check builds as publish does From the read of round 1: - A CommonJS bin's requires run with its code, so its imports can't be loaded without running it, and the load hook left it to run in full. The hook now throws in place of loading a bin whose format isn't `module`, before anything is evaluated, and such a bin is only parsed (`node --check`). Node decides the format, by `.cjs` or the package's `type`. The success line lists the bins that start with '#!', the ES module bins whose imports load, and the CommonJS bins parsed. - The bun-load and bin-load failures name a dependency's new release among their causes, as the import failure does. - The test of npm 12's pack JSON passes npm 12's own object through its shim: it assumed an array, so under a real npm 12 it failed. - The pack-check jobs install the tree with --ignore-scripts, as publish does, so a build that came to need a dependency's install script fails on a pull request rather than in publish after the tag. Tests: a CommonJS bin, by `type: "commonjs"` and by `.cjs` in an ES module package, is parsed and its code doesn't run; one that doesn't parse fails. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 4 ++- .github/workflows/publish.yml | 4 ++- CONTRIBUTING.md | 8 ++--- scripts/pack-installs-alone.mjs | 55 ++++++++++++++++++++++---------- test/pack-installs-alone.test.ts | 38 ++++++++++++++++++---- 5 files changed, 79 insertions(+), 30 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9190334fd..da0a78f07 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -81,8 +81,10 @@ jobs: with: node-version: 24 + # As publish installs, so a build that came to need a dependency's + # install script fails here, before a tag, rather than in publish. - name: Install dependencies - run: npm install + run: npm install --ignore-scripts - name: Build run: npm run build diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 235d4e188..02b7466ea 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -76,8 +76,10 @@ jobs: with: node-version: 24 + # As publish installs, so a build that came to need a dependency's + # install script fails here, before a tag, rather than in publish. - name: Install dependencies - run: npm install + run: npm install --ignore-scripts # npm publish builds again first, through prepublishOnly, from the # same sources. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ddac7d9c0..bb0328d5a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -143,10 +143,10 @@ never do. the package reaches only its own, checks that every file `package.json` names is in it, declaration files included, loads each entry point under Node and, with bun on PATH, its `bun` target under bun, and checks that - each bin has its `#!` line and that a node bin's imports load, without - running it; to run it by hand, `npm run build` first, for example before - `git push --follow-tags`, or dispatch the workflow on a branch, where it - publishes nothing), publishes + each bin has its `#!` line and that an ES module node bin's imports load, + without running it, a CommonJS one being only parsed; to run it by hand, + `npm run build` first, for example before `git push --follow-tags`, or + dispatch the workflow on a branch, where it publishes nothing), publishes `@animalabs/agent-framework` to npm once that check has passed, and creates the GitHub release with that section as its notes. The two release jobs are independent: some consumers run github-clone checkouts, diff --git a/scripts/pack-installs-alone.mjs b/scripts/pack-installs-alone.mjs index 9a752125f..f05480564 100644 --- a/scripts/pack-installs-alone.mjs +++ b/scripts/pack-installs-alone.mjs @@ -14,9 +14,11 @@ // through require() when only `require` serves it, and, when bun is on // PATH, each `bun` target through bun's import(); // - each bin starts with a `#!` line, without which npm's link can't run -// it as a command, and a node bin's imports load. Bins aren't run: a -// load hook makes the bin throw as soon as its imports have loaded, -// before its own code, which for the MCP server would start it. +// it as a command, and an ES module node bin's imports load. Bins aren't +// run: a load hook makes the bin throw as soon as its imports have +// loaded, before its own code, which for the MCP server would start it. +// A CommonJS bin's requires run with its code, so the hook stops it +// before it loads, and it's only parsed (`node --check`). // Before packing it checks that the files package.json names exist in this // tree, so an unbuilt tree says so rather than looking like a packaging // fault. The publish workflow runs it after its build and before @@ -30,11 +32,15 @@ import { pathToFileURL } from "node:url"; const LOAD_TIMEOUT_MS = 60_000; const INSTALL_TIMEOUT_MS = 10 * 60_000; -// What the load hook makes a bin throw once its imports have loaded. +// What the load hook makes an ES module bin throw once its imports have +// loaded, and what it throws in place of loading a bin that isn't one. const LINKED = "__pack_installs_alone_linked__"; +const NOT_MODULE = "__pack_installs_alone_not_module__"; const HOOK = `export async function load(url, context, nextLoad) { const result = await nextLoad(url, context); - if (url !== process.env.PACK_INSTALLS_ALONE_BIN || result.format !== "module") return result; + if (url !== process.env.PACK_INSTALLS_ALONE_BIN) return result; + // A CommonJS bin's requires run with its code, so it isn't loaded at all. + if (result.format !== "module") throw new Error("${NOT_MODULE}"); const lines = String(result.source).split("\\n"); lines.splice(lines[0].startsWith("#!") ? 1 : 0, 0, 'throw "${LINKED}";'); return { ...result, source: lines.join("\\n") }; @@ -264,11 +270,14 @@ function main() { ["-e", `await import(${JSON.stringify(specifier)}); process.exit(0);`], { cwd: dir, timeout: LOAD_TIMEOUT_MS }, `loading '${specifier}' under bun, from its bun target, in the installed ${packed.filename} failed (above). ` + - "A file it imports that the package leaves out does that, or a dependency only devDependencies supplied.", + "A file it imports that the package leaves out does that, a dependency only devDependencies supplied, or " + + "a dependency's new release (rerun this on main to tell).", ); } const bins = binsOf(pkg); + const binsLoaded = []; + const binsParsed = []; for (const [bin, path] of Object.entries(bins)) { const file = join(installed, path); const first = readFileSync(file, "utf8").split("\n", 1)[0]; @@ -278,8 +287,9 @@ function main() { ); } if (/\bnode\b/.test(first)) { - // The hook makes the bin throw once its imports have loaded, so its - // own code never runs; any other failure is the bin's. + // The hook makes an ES module bin throw once its imports have loaded, + // so its own code never runs, and stops a CommonJS bin before it + // loads, which is then only parsed; any other failure is the bin's. const hook = join(work, "bin-hook.mjs"); const register = join(work, "bin-register.mjs"); writeFileSync(hook, HOOK); @@ -290,15 +300,24 @@ function main() { timeout: LOAD_TIMEOUT_MS, env: { ...process.env, PACK_INSTALLS_ALONE_BIN: pathToFileURL(realpathSync(file)).href }, }); - if (!`${loaded.stderr}`.includes(LINKED)) { - process.stderr.write(loaded.stderr ?? ""); - throw new CheckError( - loaded.error?.code === "ETIMEDOUT" - ? `loading bin '${bin}' (${path}) in ${packed.filename} didn't finish within ${LOAD_TIMEOUT_MS / 1000} s.` - : `bin '${bin}' (${path}) in ${packed.filename} doesn't load (above): a package it imports that only ` + - "devDependencies supplies does that, as does a file the package leaves out, or a syntax error.", - ); + const stderr = `${loaded.stderr ?? ""}`; + if (stderr.includes(LINKED)) { + binsLoaded.push(bin); + continue; } + if (stderr.includes(NOT_MODULE)) { + run(process.execPath, ["--check", file], {}, `bin '${bin}' (${path}) in ${packed.filename} doesn't parse (above).`); + binsParsed.push(bin); + continue; + } + process.stderr.write(stderr); + throw new CheckError( + loaded.error?.code === "ETIMEDOUT" + ? `loading bin '${bin}' (${path}) in ${packed.filename} didn't finish within ${LOAD_TIMEOUT_MS / 1000} s.` + : `bin '${bin}' (${path}) in ${packed.filename} doesn't load (above): a package it imports that only ` + + "devDependencies supplies does that, as does a file the package leaves out, a syntax error, or a " + + "dependency's new release (rerun this on main to tell).", + ); } } @@ -311,7 +330,9 @@ function main() { ? `; loaded under bun: ${bunLoads.map((s) => `'${s}'`).join(", ")}` : `; bun targets present, not loaded (bun isn't on PATH): ${bunLoads.map((s) => `'${s}'`).join(", ")}` : "") + - (Object.keys(bins).length > 0 ? `; bins start with '#!', and node bins' imports load: ${Object.keys(bins).join(", ")}` : "") + + (Object.keys(bins).length > 0 ? `; bins start with '#!': ${Object.keys(bins).join(", ")}` : "") + + (binsLoaded.length > 0 ? `; ES module bins' imports load: ${binsLoaded.join(", ")}` : "") + + (binsParsed.length > 0 ? `; CommonJS bins parse, not loaded: ${binsParsed.join(", ")}` : "") + (patterns.length > 0 ? `; export patterns matched, not loaded: ${patterns.join(", ")}` : "") + ".", ); diff --git a/test/pack-installs-alone.test.ts b/test/pack-installs-alone.test.ts index 2df832d73..9e21277ef 100644 --- a/test/pack-installs-alone.test.ts +++ b/test/pack-installs-alone.test.ts @@ -7,7 +7,7 @@ import { after, test } from "node:test"; import assert from "node:assert/strict"; import { execFile, execFileSync } from "node:child_process"; -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; @@ -96,7 +96,7 @@ test("a package whose files carry everything it names installs alone, and each e assert.doesNotMatch(result.stderr, /zz-serve ran/, "a bin is never run"); assert.match( result.stdout, - /^zz-fixture-1\.0\.0\.tgz \(\d+ files\) installs on its own; every file package\.json names is in it; loaded '@zz\/fixture' \(import\), '@zz\/fixture\/extra' \(import\), '@zz\/fixture\/package\.json' \(import json\), '@zz\/fixture\/legacy' \(require\); bun targets present, not loaded \(bun isn't on PATH\): '@zz\/fixture'; bins start with '#!', and node bins' imports load: zz-serve, zz-shell; export patterns matched, not loaded: \.\/parts\/\*\.\n$/m, + /^zz-fixture-1\.0\.0\.tgz \(\d+ files\) installs on its own; every file package\.json names is in it; loaded '@zz\/fixture' \(import\), '@zz\/fixture\/extra' \(import\), '@zz\/fixture\/package\.json' \(import json\), '@zz\/fixture\/legacy' \(require\); bun targets present, not loaded \(bun isn't on PATH\): '@zz\/fixture'; bins start with '#!': zz-serve, zz-shell; ES module bins' imports load: zz-serve; export patterns matched, not loaded: \.\/parts\/\*\.\n$/m, ); }); @@ -138,7 +138,7 @@ test("with neither exports nor main, the root is loaded from its index.js, and a assert.match(fallback.stdout, /; loaded '@zz\/fixture' \(import\)\.\n$/m); const binOnly = await check(fixture({ files: ["dist"], bin: "./dist/serve.js" }, { "dist/serve.js": NODE_BIN })); assert.equal(binOnly.code, 0, binOnly.stderr); - assert.match(binOnly.stdout, /installs on its own; every file package\.json names is in it; bins start with '#!', and node bins' imports load: fixture\.\n$/m); + assert.match(binOnly.stdout, /installs on its own; every file package\.json names is in it; bins start with '#!': fixture; ES module bins' imports load: fixture\.\n$/m); }); test("a tree that hasn't been built, or a main beside exports that names no file, says so before packing", async () => { @@ -198,7 +198,7 @@ test("a bin needs a '#!' line, and a node bin's imports must load; none is run", ...ENTRY, "dist/serve.js": "#!/usr/bin/env node\nexport const = ;\n", })); assert.equal(unparsed.code, 1); - assert.match(unparsed.stderr, /pack-installs-alone: bin 'fixture' \(\.\/dist\/serve\.js\) in zz-fixture-1\.0\.0\.tgz doesn't load \(above\): a package it imports that only devDependencies supplies does that, as does a file the package leaves out, or a syntax error\.\n$/); + assert.match(unparsed.stderr, /pack-installs-alone: bin 'fixture' \(\.\/dist\/serve\.js\) in zz-fixture-1\.0\.0\.tgz doesn't load \(above\): a package it imports that only devDependencies supplies does that, as does a file the package leaves out, a syntax error, or a dependency's new release \(rerun this on main to tell\)\.\n$/); // A bin whose imports the root never reaches: the root loads, the bin doesn't. const devOnlyBin = await check(fixture( { files: ["dist"], main: "./dist/index.js", bin: { "zz-serve": "./dist/serve.js" }, devDependencies: { "zz-dev-only": "1.0.0" } }, @@ -217,6 +217,30 @@ test("a bin needs a '#!' line, and a node bin's imports must load; none is run", assert.doesNotMatch(imports.stderr, /zz-serve ran/); }); +test("a CommonJS bin is parsed, never run, whatever marks it CommonJS, and one that doesn't parse fails", async () => { + const marks = mkdtempSync(join(tmpdir(), "pack-installs-alone-marks-")); + made.push(marks); + const bin = (mark: string) => + `#!/usr/bin/env node\nrequire("node:fs").writeFileSync(${JSON.stringify(join(marks, mark))}, "ran");\nconsole.error("zz CommonJS bin ran");\n`; + // CommonJS by the package's type, and by a .cjs extension in an ES module package. + for (const [mark, pkg, path] of [ + ["typed", { type: "commonjs", files: ["dist"], main: "./dist/index.cjs", bin: { "zz-cjs": "./dist/serve.js" } }, "dist/serve.js"], + ["extension", { files: ["dist"], main: "./dist/index.js", bin: { "zz-cjs": "./dist/serve.cjs" } }, "dist/serve.cjs"], + ] as const) { + const result = await check(fixture(pkg, { ...ENTRY, "dist/index.cjs": "exports.ok = true;\n", [path]: bin(mark) })); + assert.equal(result.code, 0, result.stderr); + assert.match(result.stdout, /; bins start with '#!': zz-cjs; CommonJS bins parse, not loaded: zz-cjs\.\n$/m); + assert.equal(existsSync(join(marks, mark)), false, `the ${mark} CommonJS bin ran`); + assert.doesNotMatch(result.stderr, /zz CommonJS bin ran/); + } + const unparsed = await check(fixture( + { type: "commonjs", files: ["dist"], main: "./dist/index.cjs", bin: { "zz-cjs": "./dist/serve.js" } }, + { "dist/index.cjs": "exports.ok = true;\n", "dist/serve.js": "#!/usr/bin/env node\nconst = ;\n" }, + )); + assert.equal(unparsed.code, 1); + assert.match(unparsed.stderr, /pack-installs-alone: bin 'zz-cjs' \(\.\/dist\/serve\.js\) in zz-fixture-1\.0\.0\.tgz doesn't parse \(above\)\.\n$/); +}); + test("a package imported only through another dependency's copy fails, since the install is nested", async () => { // zz-cm depends on zz-chronicle; the fixture imports zz-chronicle but lists // it only in devDependencies, so a hoisted install would hand it zz-cm's. @@ -251,7 +275,7 @@ test("with bun on PATH, a bun target is loaded under bun, and one that doesn't l assert.match(loads.stdout, /; loaded '@zz\/fixture' \(import\); loaded under bun: '@zz\/fixture'\.\n$/m); const broken = await check(fixture(pkg, { ...ENTRY, "src/index.js": "import './left-out.js';\n" }), BUN_BY_NODE); assert.equal(broken.code, 1); - assert.match(broken.stderr, /pack-installs-alone: loading '@zz\/fixture' under bun, from its bun target, in the installed zz-fixture-1\.0\.0\.tgz failed \(above\)\./); + assert.match(broken.stderr, /pack-installs-alone: loading '@zz\/fixture' under bun, from its bun target, in the installed zz-fixture-1\.0\.0\.tgz failed \(above\)\. A file it imports that the package leaves out does that, a dependency only devDependencies supplied, or a dependency's new release \(rerun this on main to tell\)\.\n$/); }); test("a declaration file the build wrote beside types, left out of files, fails, and exports: null falls back to main", async () => { @@ -268,11 +292,11 @@ test("a declaration file the build wrote beside types, left out of files, fails, test("npm pack's JSON is read in npm 12's shape too, and a prepack's output doesn't break it", async () => { // npm 12 prints an object keyed by package name: a shim in front of the - // real npm rewrites npm 11's array that way. + // real npm rewrites npm 11's array that way, and passes npm 12's through. const realNpm = execFileSync("sh", ["-c", "command -v npm"], { encoding: "utf8" }).trim(); const npm12 = shim( "npm", - `#!/usr/bin/env node\nconst { execFileSync } = require("node:child_process");\nconst args = process.argv.slice(2);\ntry {\n const out = execFileSync(${JSON.stringify(realNpm)}, args, { encoding: "utf8", stdio: ["inherit", "pipe", "inherit"] });\n if (args[0] === "pack" && args.includes("--json")) {\n const [record] = JSON.parse(out);\n process.stdout.write(JSON.stringify({ [record.name]: record }));\n } else process.stdout.write(out);\n} catch (error) { process.stdout.write(error.stdout ?? ""); process.exit(error.status ?? 1); }\n`, + `#!/usr/bin/env node\nconst { execFileSync } = require("node:child_process");\nconst args = process.argv.slice(2);\ntry {\n const out = execFileSync(${JSON.stringify(realNpm)}, args, { encoding: "utf8", stdio: ["inherit", "pipe", "inherit"] });\n if (args[0] === "pack" && args.includes("--json")) {\n const parsed = JSON.parse(out);\n // npm 12 prints the object already; npm 11's array is rewritten.\n process.stdout.write(Array.isArray(parsed) ? JSON.stringify({ [parsed[0].name]: parsed[0] }) : out);\n } else process.stdout.write(out);\n} catch (error) { process.stdout.write(error.stdout ?? ""); process.exit(error.status ?? 1); }\n`, ); const twelve = await check(fixture({ files: ["dist"], main: "./dist/index.js" }, ENTRY), NO_BUN, `${npm12}:`); assert.equal(twelve.code, 0, twelve.stderr); From aa7f8ba7f1097b49efb587f96bb8cb66ceb17de4 Mon Sep 17 00:00:00 2001 From: Ruth-2424 Date: Fri, 16 Oct 2026 16:40:54 -0400 Subject: [PATCH 5/5] build(release): the pack check after an internal review of round 1 An internal review read round 1 (f5267d7b). What it found and what changed: - Declarations. The check now takes the form R1-3 of the maintainer's review suggested: in a package with types (`types`, `typings` or a `types` condition), the `.d.ts` the build wrote beside each `.js` it packs must be in the package. The rule it replaces, every declaration under the directory of `types`, took in node_modules for a root-level `types`, ran only from a top-level `types` or `typings`, and required declarations no packed module has beside it, such as a test helper's. - A subpath with both an import target and a different require target loaded only the import; it now loads both, as each kind of consumer does. - The success line listed every bin under "start with '#!'" without naming the ones it didn't load. A bin whose '#!' line names no node is now named as not loaded. The line also says "static imports", and names the package.json fields whose files it checks. - npm pack's record is checked for its files list as well as its filename. The file count comes from that list, and the install path from this tree's package.json, so a record short of either field fails by name. - The hook's ': 0' arm, which no bin reached, is gone, and so are the Array branches of allTargets and hasCondition, which Object.values and Object.entries already cover. pick keeps its own, now tested. - Text: the header says the nested install is stricter than npm's default, that the loads run under the Node that runs the script, that export patterns are matched and not loaded, and that the bun load imports the package's name. The bun failure message no longer says it loaded the bun target. CONTRIBUTING points at the header for the list of checks and says CI installs with npm install rather than npm ci, with bun set up for the pack check. The workflow comments say why bun is 1.3.14 (connectome-host's CI runs this package under it), what the publish job's install leaves to pack-check, and that in publish.yml pack-check runs at the tag. New tests: typings and an exports array; a dual subpath; a bin whose '#!' line names no node; a bin that imports a CommonJS file; a declaration left out beside a packed module whether types, typings or a types condition gives the package types; none required beside a module the package doesn't pack (a root-level types, a test helper's and a dependency's declarations), or in a package without types; and npm pack output that isn't one record with its files. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 1 + .github/workflows/publish.yml | 16 +-- CONTRIBUTING.md | 20 ++-- scripts/pack-installs-alone.mjs | 176 ++++++++++++++++--------------- test/pack-installs-alone.test.ts | 69 +++++++++--- 5 files changed, 164 insertions(+), 118 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index da0a78f07..4267a42f4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -90,6 +90,7 @@ jobs: run: npm run build # bun loads the package's `bun` export target, which Node never reads. + # 1.3.14 is the bun connectome-host's CI runs this package under. - name: Setup Bun uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 with: diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 02b7466ea..25bf86132 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -54,11 +54,11 @@ jobs: # The tarball npm publish makes from this tree, installed alone into an # empty directory with its dependencies from npm (nested, so the package - # reaches only its own), and loaded, under Node and, for its bun target, - # bun: a file missing from `files`, a dependency only devDependencies - # supplied, or a bin npm's link can't run fails here, not for whoever - # installs the release. A job of its own, so the install scripts it runs never see the - # publish job's id-token; publish needs it, the GitHub release notes + # reaches only its own), and loaded under Node and under bun: a file + # missing from `files`, a dependency only devDependencies supplied, or a + # bin npm's link can't run fails here, not for whoever installs the + # release. A job of its own, so the install scripts it runs never see + # the publish job's id-token; publish needs it, the GitHub release notes # don't. It runs on a dispatch too, which publishes nothing. pack-check: name: Packed release installs on its own @@ -77,7 +77,7 @@ jobs: node-version: 24 # As publish installs, so a build that came to need a dependency's - # install script fails here, before a tag, rather than in publish. + # install script fails here rather than in publish. - name: Install dependencies run: npm install --ignore-scripts @@ -87,6 +87,7 @@ jobs: run: npm run build # bun loads the package's `bun` export target, which Node never reads. + # 1.3.14 is the bun connectome-host's CI runs this package under. - name: Setup Bun uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 with: @@ -134,7 +135,8 @@ jobs: # --ignore-scripts: this job holds the id-token, and neither the build # nor npm publish needs a dependency's install script. The pack-check - # job runs them, as an install from npm does, without the id-token. + # job's install of the packed release runs them, as an install from + # npm does, without the id-token. - name: Install dependencies run: npm install --ignore-scripts diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index bb0328d5a..595028329 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -140,13 +140,10 @@ never do. of its own that the packed release installs on its own (`node scripts/pack-installs-alone.mjs` packs it, installs the tarball alone into an empty directory with its dependencies from npm, nested so - the package reaches only its own, checks that every file `package.json` - names is in it, declaration files included, loads each entry point under - Node and, with bun on PATH, its `bun` target under bun, and checks that - each bin has its `#!` line and that an ES module node bin's imports load, - without running it, a CommonJS one being only parsed; to run it by hand, - `npm run build` first, for example before `git push --follow-tags`, or - dispatch the workflow on a branch, where it publishes nothing), publishes + the package reaches only its own, and checks the installed package, as + the script's header lists; to run it by hand, `npm run build` first, for + example before `git push --follow-tags`, or dispatch the workflow on a + branch, where it publishes nothing), publishes `@animalabs/agent-framework` to npm once that check has passed, and creates the GitHub release with that section as its notes. The two release jobs are independent: some consumers run github-clone checkouts, @@ -205,10 +202,11 @@ branches. The `pretest` hook copies `test/fixtures/*.mjs` into `dist/`. Push-time CI (`ci.yml`) builds, typechecks and tests every push and PR on ubuntu and macos. Lockfiles are deliberately gitignored in this repo, so CI -installs with plain `npm install` and there is no lock to keep in sync. It -also runs the pack check the publish workflow runs before publishing -(`node scripts/pack-installs-alone.mjs`, on ubuntu), so a file left out of -the package, or an import only `devDependencies` supply, fails the PR. +installs with `npm install` rather than `npm ci`, and there is no lock to +keep in sync. It also runs the pack check the publish workflow runs before +publishing (`node scripts/pack-installs-alone.mjs`, on ubuntu, with bun set +up), so a file left out of the package, or an import only `devDependencies` +supply, fails the PR. When working against local sibling checkouts rather than published versions, see the ecosystem workspace setup — `bun install` at the `rust-connectome` diff --git a/scripts/pack-installs-alone.mjs b/scripts/pack-installs-alone.mjs index f05480564..c683b5957 100644 --- a/scripts/pack-installs-alone.mjs +++ b/scripts/pack-installs-alone.mjs @@ -1,33 +1,37 @@ // Packs this package as `npm publish` packs it, installs the tarball alone -// into an empty directory with its dependencies from the registry, as -// someone installing the release does, and checks what the installed -// package.json offers. The install is nested (npm's --install-strategy -// nested), so the package reaches only its own dependencies: a package it -// imports but only devDependencies lists fails to load even when another -// dependency brings that package in. -// - every file it names is in the package: `main`, `types`, each `exports` -// target under every condition (the `bun` and `types` targets too), at -// least one file for each export pattern, each `bin`, and each -// declaration file the build wrote beside `types`; +// into an empty directory with its dependencies from the registry, and +// checks, under the Node that runs this, what the installed package.json +// offers. The install is nested (npm's --install-strategy nested), which is +// stricter than npm's default: the package reaches only its own +// dependencies, so a package it imports but only devDependencies lists +// fails to load even when another dependency brings that package in. +// - every file its `main`, `types` (or `typings`), `exports` and `bin` +// name is in the package: each `exports` target under every condition +// (the `bun` and `types` targets too), at least one file for each export +// pattern, and each `bin`; and, in a package with types (`types`, +// `typings` or a `types` condition), so is the `.d.ts` the build wrote +// beside each `.js` it packs; // - each entry point loads as a consumer loads it: the root and each -// subpath through import() (a JSON target with its import attribute), or -// through require() when only `require` serves it, and, when bun is on -// PATH, each `bun` target through bun's import(); +// subpath, patterns aside, through import() (a JSON target with its +// import attribute), and through require() where `require` picks a file +// import() doesn't; and, when bun is on PATH, each one with a `bun` +// condition through bun's import() of its name; // - each bin starts with a `#!` line, without which npm's link can't run -// it as a command, and an ES module node bin's imports load. Bins aren't -// run: a load hook makes the bin throw as soon as its imports have -// loaded, before its own code, which for the MCP server would start it. -// A CommonJS bin's requires run with its code, so the hook stops it -// before it loads, and it's only parsed (`node --check`). +// it as a command, and an ES module node bin's static imports load. +// Bins aren't run: a load hook makes the bin throw as soon as its +// imports have loaded, before its own code, which for the MCP server +// would start it. A CommonJS bin's requires run with its code, so the +// hook stops it before it loads, and it's only parsed (`node --check`). +// A bin whose `#!` line names no node is checked for that line alone. // Before packing it checks that the files package.json names exist in this // tree, so an unbuilt tree says so rather than looking like a packaging // fault. The publish workflow runs it after its build and before // `npm publish` (which builds again from the same sources first); to check // by hand, run `npm run build` and then this, from the repository root. import { execFileSync, spawnSync } from "node:child_process"; -import { existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, realpathSync, rmSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; -import { dirname, join, relative } from "node:path"; +import { join, relative } from "node:path"; import { pathToFileURL } from "node:url"; const LOAD_TIMEOUT_MS = 60_000; @@ -41,8 +45,9 @@ const HOOK = `export async function load(url, context, nextLoad) { if (url !== process.env.PACK_INSTALLS_ALONE_BIN) return result; // A CommonJS bin's requires run with its code, so it isn't loaded at all. if (result.format !== "module") throw new Error("${NOT_MODULE}"); + // Only a bin whose first line starts with '#!' gets here. const lines = String(result.source).split("\\n"); - lines.splice(lines[0].startsWith("#!") ? 1 : 0, 0, 'throw "${LINKED}";'); + lines.splice(1, 0, 'throw "${LINKED}";'); return { ...result, source: lines.join("\\n") }; } `; @@ -75,10 +80,10 @@ function pick(value, conditions) { return undefined; } -/** Every file target an exports value names, under any condition. */ +/** Every file target an exports value names, under any condition (an + * array's items are its values). */ function allTargets(value, out = []) { if (typeof value === "string") out.push(value); - else if (Array.isArray(value)) for (const item of value) allTargets(item, out); else if (value && typeof value === "object") for (const inner of Object.values(value)) allTargets(inner, out); return out; } @@ -112,21 +117,9 @@ function namedFiles(pkg) { return named; } -/** Each declaration file under `dir` in this tree, as a path from here. */ -function declarations(dir) { - if (!existsSync(dir)) return []; - const out = []; - for (const entry of readdirSync(dir, { withFileTypes: true })) { - const path = join(dir, entry.name); - if (entry.isDirectory()) out.push(...declarations(path)); - else if (entry.name.endsWith(".d.ts")) out.push(path); - } - return out; -} - -/** Whether an exports value names `condition` anywhere. */ +/** Whether an exports value names `condition` anywhere (an array's keys + * are its indexes). */ function hasCondition(value, condition) { - if (Array.isArray(value)) return value.some((item) => hasCondition(item, condition)); if (!value || typeof value !== "object") return false; return Object.entries(value).some(([key, inner]) => key === condition || hasCondition(inner, condition)); } @@ -177,11 +170,12 @@ function main() { } catch { records = []; } - if (records.length !== 1 || typeof records[0]?.filename !== "string") { + if (records.length !== 1 || typeof records[0]?.filename !== "string" || !Array.isArray(records[0]?.files)) { process.stderr.write(out); - throw new CheckError("npm pack --json didn't print one record with a filename (its output is above)."); + throw new CheckError("npm pack --json didn't print one record with a filename and its files (its output is above)."); } const packed = records[0]; + const files = packed.files.map((file) => file.path); const tarball = join(work, packed.filename); const dir = join(work, "install"); mkdirSync(dir); @@ -193,22 +187,24 @@ function main() { `${packed.filename} didn't install on its own into an empty directory (npm's output is above). A dependency ` + "that isn't on the registry does that, or a registry or network error, which a re-run gets past.", ); - const installed = join(dir, "node_modules", ...packed.name.split("/")); + const installed = join(dir, "node_modules", ...source.name.split("/")); // What a consumer reads: the manifest in the tarball. const pkg = JSON.parse(readFileSync(join(installed, "package.json"), "utf8")); - const files = packed.files.map((file) => file.path); // This tree has each named file (checked before packing), so one the // package lacks is one `files` leaves out. - const missing = namedFiles(pkg).filter(([path]) => !existsSync(join(installed, path))); - // The declaration files the build wrote beside `types`, which it imports. - const types = typeof pkg.types === "string" ? pkg.types : typeof pkg.typings === "string" ? pkg.typings : undefined; - if (types !== undefined) { - for (const declaration of declarations(dirname(types))) { - const path = relative(".", declaration); - // `types` itself is already named above. - if (path === relative(".", types)) continue; - if (!existsSync(join(installed, path))) missing.push([path, `a declaration beside ${types}`]); + const named = namedFiles(pkg); + const missing = named.filter(([path]) => !existsSync(join(installed, path))); + // A consumer's TypeScript reads the declaration beside each module it + // reaches, so a package with types carries the one the build wrote + // beside each module it packs. A named one was checked above. + if (typeof pkg.types === "string" || typeof pkg.typings === "string" || hasCondition(pkg.exports, "types")) { + const checked = new Set([...files, ...named.map(([path]) => relative(".", path))]); + for (const file of files) { + const declaration = file.replace(/\.js$/, ".d.ts"); + if (declaration !== file && !checked.has(declaration) && existsSync(declaration)) { + missing.push([declaration, `the declaration beside ${file}`]); + } } } for (const [key, value] of subpaths(pkg.exports)) { @@ -241,7 +237,7 @@ function main() { const imported = pick(value, ["node", "import"]); const required = pick(value, ["node", "require"]); if (typeof imported === "string") loads.push([specifier, imported.endsWith(".json") ? "import json" : "import"]); - else if (typeof required === "string") loads.push([specifier, "require"]); + if (typeof required === "string" && required !== imported) loads.push([specifier, "require"]); } for (const [specifier, how] of loads) { const code = @@ -257,7 +253,9 @@ function main() { "dependency's new release (rerun this on main to tell).", ); } - // bun reads the `bun` condition, which Node never does. + // bun reads the `bun` condition, which Node never does. Importing the + // package's name under bun loads what a bun consumer gets: the `bun` + // target where `bun` is the first condition bun matches. const bunLoads = []; const bun = bunOnPath(); for (const [key, value] of roots) { @@ -269,7 +267,7 @@ function main() { "bun", ["-e", `await import(${JSON.stringify(specifier)}); process.exit(0);`], { cwd: dir, timeout: LOAD_TIMEOUT_MS }, - `loading '${specifier}' under bun, from its bun target, in the installed ${packed.filename} failed (above). ` + + `loading '${specifier}' under bun from the installed ${packed.filename} failed (above). ` + "A file it imports that the package leaves out does that, a dependency only devDependencies supplied, or " + "a dependency's new release (rerun this on main to tell).", ); @@ -278,6 +276,7 @@ function main() { const bins = binsOf(pkg); const binsLoaded = []; const binsParsed = []; + const binsNotNode = []; for (const [bin, path] of Object.entries(bins)) { const file = join(installed, path); const first = readFileSync(file, "utf8").split("\n", 1)[0]; @@ -286,44 +285,46 @@ function main() { `bin '${bin}' (${path}) has no '#!' line, so npm's link can't run it as a command: start its source with '#!/usr/bin/env node'.`, ); } - if (/\bnode\b/.test(first)) { - // The hook makes an ES module bin throw once its imports have loaded, - // so its own code never runs, and stops a CommonJS bin before it - // loads, which is then only parsed; any other failure is the bin's. - const hook = join(work, "bin-hook.mjs"); - const register = join(work, "bin-register.mjs"); - writeFileSync(hook, HOOK); - writeFileSync(register, `import { register } from "node:module";\nregister(${JSON.stringify(pathToFileURL(hook).href)});\n`); - const loaded = spawnSync(process.execPath, ["--import", pathToFileURL(register).href, file], { - cwd: dir, - encoding: "utf8", - timeout: LOAD_TIMEOUT_MS, - env: { ...process.env, PACK_INSTALLS_ALONE_BIN: pathToFileURL(realpathSync(file)).href }, - }); - const stderr = `${loaded.stderr ?? ""}`; - if (stderr.includes(LINKED)) { - binsLoaded.push(bin); - continue; - } - if (stderr.includes(NOT_MODULE)) { - run(process.execPath, ["--check", file], {}, `bin '${bin}' (${path}) in ${packed.filename} doesn't parse (above).`); - binsParsed.push(bin); - continue; - } - process.stderr.write(stderr); - throw new CheckError( - loaded.error?.code === "ETIMEDOUT" - ? `loading bin '${bin}' (${path}) in ${packed.filename} didn't finish within ${LOAD_TIMEOUT_MS / 1000} s.` - : `bin '${bin}' (${path}) in ${packed.filename} doesn't load (above): a package it imports that only ` + - "devDependencies supplies does that, as does a file the package leaves out, a syntax error, or a " + - "dependency's new release (rerun this on main to tell).", - ); + if (!/\bnode\b/.test(first)) { + binsNotNode.push(bin); + continue; + } + // The hook makes an ES module bin throw once its imports have loaded, + // so its own code never runs, and stops a CommonJS bin before it + // loads, which is then only parsed; any other failure is the bin's. + const hook = join(work, "bin-hook.mjs"); + const register = join(work, "bin-register.mjs"); + writeFileSync(hook, HOOK); + writeFileSync(register, `import { register } from "node:module";\nregister(${JSON.stringify(pathToFileURL(hook).href)});\n`); + const loaded = spawnSync(process.execPath, ["--import", pathToFileURL(register).href, file], { + cwd: dir, + encoding: "utf8", + timeout: LOAD_TIMEOUT_MS, + env: { ...process.env, PACK_INSTALLS_ALONE_BIN: pathToFileURL(realpathSync(file)).href }, + }); + const stderr = `${loaded.stderr ?? ""}`; + if (stderr.includes(LINKED)) { + binsLoaded.push(bin); + continue; + } + if (stderr.includes(NOT_MODULE)) { + run(process.execPath, ["--check", file], {}, `bin '${bin}' (${path}) in ${packed.filename} doesn't parse (above).`); + binsParsed.push(bin); + continue; } + process.stderr.write(stderr); + throw new CheckError( + loaded.error?.code === "ETIMEDOUT" + ? `loading bin '${bin}' (${path}) in ${packed.filename} didn't finish within ${LOAD_TIMEOUT_MS / 1000} s.` + : `bin '${bin}' (${path}) in ${packed.filename} doesn't load (above): a package it imports that only ` + + "devDependencies supplies does that, as does a file the package leaves out, a syntax error, or a " + + "dependency's new release (rerun this on main to tell).", + ); } const patterns = roots.filter(([key]) => key.includes("*")).map(([key]) => key); console.log( - `${packed.filename} (${packed.entryCount} files) installs on its own; every file package.json names is in it` + + `${packed.filename} (${files.length} files) installs on its own; every file package.json's main, types, exports and bin name is in it` + (loads.length > 0 ? `; loaded ${loads.map(([s, how]) => `'${s}' (${how})`).join(", ")}` : "") + (bunLoads.length > 0 ? bun @@ -331,8 +332,9 @@ function main() { : `; bun targets present, not loaded (bun isn't on PATH): ${bunLoads.map((s) => `'${s}'`).join(", ")}` : "") + (Object.keys(bins).length > 0 ? `; bins start with '#!': ${Object.keys(bins).join(", ")}` : "") + - (binsLoaded.length > 0 ? `; ES module bins' imports load: ${binsLoaded.join(", ")}` : "") + + (binsLoaded.length > 0 ? `; ES module bins' static imports load: ${binsLoaded.join(", ")}` : "") + (binsParsed.length > 0 ? `; CommonJS bins parse, not loaded: ${binsParsed.join(", ")}` : "") + + (binsNotNode.length > 0 ? `; bins whose '#!' line names no node, not loaded: ${binsNotNode.join(", ")}` : "") + (patterns.length > 0 ? `; export patterns matched, not loaded: ${patterns.join(", ")}` : "") + ".", ); diff --git a/test/pack-installs-alone.test.ts b/test/pack-installs-alone.test.ts index 9e21277ef..c6ac44e27 100644 --- a/test/pack-installs-alone.test.ts +++ b/test/pack-installs-alone.test.ts @@ -75,6 +75,7 @@ test("a package whose files carry everything it names installs alone, and each e "./extra": "./dist/extra.js", "./package.json": "./package.json", "./legacy": { require: "./dist/legacy.cjs" }, + "./dual": { import: "./dist/index.js", require: "./dist/legacy.cjs" }, "./types": { types: "./dist/types.d.ts" }, "./internal": null, "./parts/*": "./dist/*.js", @@ -96,14 +97,15 @@ test("a package whose files carry everything it names installs alone, and each e assert.doesNotMatch(result.stderr, /zz-serve ran/, "a bin is never run"); assert.match( result.stdout, - /^zz-fixture-1\.0\.0\.tgz \(\d+ files\) installs on its own; every file package\.json names is in it; loaded '@zz\/fixture' \(import\), '@zz\/fixture\/extra' \(import\), '@zz\/fixture\/package\.json' \(import json\), '@zz\/fixture\/legacy' \(require\); bun targets present, not loaded \(bun isn't on PATH\): '@zz\/fixture'; bins start with '#!': zz-serve, zz-shell; ES module bins' imports load: zz-serve; export patterns matched, not loaded: \.\/parts\/\*\.\n$/m, + /^zz-fixture-1\.0\.0\.tgz \(\d+ files\) installs on its own; every file package\.json's main, types, exports and bin name is in it; loaded '@zz\/fixture' \(import\), '@zz\/fixture\/extra' \(import\), '@zz\/fixture\/package\.json' \(import json\), '@zz\/fixture\/legacy' \(require\), '@zz\/fixture\/dual' \(import\), '@zz\/fixture\/dual' \(require\); bun targets present, not loaded \(bun isn't on PATH\): '@zz\/fixture'; bins start with '#!': zz-serve, zz-shell; ES module bins' static imports load: zz-serve; bins whose '#!' line names no node, not loaded: zz-shell; export patterns matched, not loaded: \.\/parts\/\*\.\n$/m, ); }); -test("a string exports, an exports of conditions, and main without exports each load as the root", async () => { +test("a string exports, an exports of conditions, an exports array, and main without exports each load as the root", async () => { for (const pkg of [ { files: ["dist"], exports: "./dist/index.js" }, { files: ["dist"], exports: { import: "./dist/index.js", default: "./dist/index.js" } }, + { files: ["dist"], exports: ["./dist/index.js"] }, { files: ["dist"], main: "./dist/index.js" }, ]) { const result = await check(fixture(pkg, ENTRY)); @@ -138,7 +140,7 @@ test("with neither exports nor main, the root is loaded from its index.js, and a assert.match(fallback.stdout, /; loaded '@zz\/fixture' \(import\)\.\n$/m); const binOnly = await check(fixture({ files: ["dist"], bin: "./dist/serve.js" }, { "dist/serve.js": NODE_BIN })); assert.equal(binOnly.code, 0, binOnly.stderr); - assert.match(binOnly.stdout, /installs on its own; every file package\.json names is in it; bins start with '#!': fixture; ES module bins' imports load: fixture\.\n$/m); + assert.match(binOnly.stdout, /installs on its own; every file package\.json's main, types, exports and bin name is in it; bins start with '#!': fixture; ES module bins' static imports load: fixture\.\n$/m); }); test("a tree that hasn't been built, or a main beside exports that names no file, says so before packing", async () => { @@ -158,6 +160,7 @@ test("a file the package names but leaves out of files fails, under any conditio // the bun and types targets of the root, which no import() reads [{ files: ["dist"], exports: { ".": { bun: "./src/index.ts", import: "./dist/index.js" } } }, { ...ENTRY, "src/index.ts": "" }, /\.\/src\/index\.ts \(exports\['\.'\]\) isn't in zz-fixture-1\.0\.0\.tgz: add it to package\.json's `files`\./], [{ files: ["dist/index.js"], types: "./dist/index.d.ts", exports: "./dist/index.js" }, { ...ENTRY, "dist/index.d.ts": "" }, /\.\/dist\/index\.d\.ts \(types\) isn't in/], + [{ files: ["dist/index.js"], typings: "./dist/index.d.ts", exports: "./dist/index.js" }, { ...ENTRY, "dist/index.d.ts": "" }, /\.\/dist\/index\.d\.ts \(typings\) isn't in/], // a pattern that matches nothing packed [{ files: ["dist/index.js"], exports: { ".": "./dist/index.js", "./parts/*": "./parts/*.js" } }, { ...ENTRY, "parts/a.js": "" }, /\.\/parts\/\*\.js \(exports\['\.\/parts\/\*'\], a pattern no packed file matches\) isn't in/], ]; @@ -208,13 +211,19 @@ test("a bin needs a '#!' line, and a node bin's imports must load; none is run", assert.match(devOnlyBin.stderr, /Cannot find package 'zz-dev-only'/); assert.doesNotMatch(devOnlyBin.stderr, /zz-serve ran/); assert.match(devOnlyBin.stderr, /pack-installs-alone: bin 'zz-serve' \(\.\/dist\/serve\.js\) in zz-fixture-1\.0\.0\.tgz doesn't load \(above\)/); - // One whose imports load is never run either. + // One whose imports load is never run either, and the hook leaves what it + // imports alone: a CommonJS one among them doesn't make the bin CommonJS. const imports = await check(fixture( { files: ["dist"], main: "./dist/index.js", bin: { "zz-serve": "./dist/serve.js" } }, - { ...ENTRY, "dist/serve.js": "#!/usr/bin/env node\nimport { ok } from './index.js';\nthrow new Error('zz-serve ran ' + ok);\n" }, + { + ...ENTRY, + "dist/legacy.cjs": "module.exports = { legacy: true };\n", + "dist/serve.js": "#!/usr/bin/env node\nimport { ok } from './index.js';\nimport legacy from './legacy.cjs';\nthrow new Error('zz-serve ran ' + ok + legacy.legacy);\n", + }, )); assert.equal(imports.code, 0, imports.stderr); assert.doesNotMatch(imports.stderr, /zz-serve ran/); + assert.match(imports.stdout, /; bins start with '#!': zz-serve; ES module bins' static imports load: zz-serve\.\n$/m); }); test("a CommonJS bin is parsed, never run, whatever marks it CommonJS, and one that doesn't parse fails", async () => { @@ -275,21 +284,55 @@ test("with bun on PATH, a bun target is loaded under bun, and one that doesn't l assert.match(loads.stdout, /; loaded '@zz\/fixture' \(import\); loaded under bun: '@zz\/fixture'\.\n$/m); const broken = await check(fixture(pkg, { ...ENTRY, "src/index.js": "import './left-out.js';\n" }), BUN_BY_NODE); assert.equal(broken.code, 1); - assert.match(broken.stderr, /pack-installs-alone: loading '@zz\/fixture' under bun, from its bun target, in the installed zz-fixture-1\.0\.0\.tgz failed \(above\)\. A file it imports that the package leaves out does that, a dependency only devDependencies supplied, or a dependency's new release \(rerun this on main to tell\)\.\n$/); + assert.match(broken.stderr, /pack-installs-alone: loading '@zz\/fixture' under bun from the installed zz-fixture-1\.0\.0\.tgz failed \(above\)\. A file it imports that the package leaves out does that, a dependency only devDependencies supplied, or a dependency's new release \(rerun this on main to tell\)\.\n$/); }); -test("a declaration file the build wrote beside types, left out of files, fails, and exports: null falls back to main", async () => { - const declarations = await check(fixture( - { files: ["dist/**/*.js", "dist/index.d.ts"], main: "./dist/index.js", types: "./dist/index.d.ts" }, - { ...ENTRY, "dist/index.d.ts": "export * from './extra.js';\n", "dist/extra.d.ts": "export declare const extra: boolean;\n" }, - )); - assert.equal(declarations.code, 1); - assert.match(declarations.stderr, /dist\/extra\.d\.ts \(a declaration beside \.\/dist\/index\.d\.ts\) isn't in zz-fixture-1\.0\.0\.tgz/); +test("the declaration the build wrote beside a packed module, left out of files, fails, whether types, typings or a types condition gives the package types, and exports: null falls back to main", async () => { + const declarations = { "dist/index.d.ts": "export * from './extra.js';\n", "dist/extra.d.ts": "export declare const extra: boolean;\n" }; + for (const typed of [ + { types: "./dist/index.d.ts" }, + { typings: "./dist/index.d.ts" }, + { exports: { ".": { types: "./dist/index.d.ts", import: "./dist/index.js" } } }, + ]) { + const result = await check(fixture({ files: ["dist/**/*.js", "dist/index.d.ts"], main: "./dist/index.js", ...typed }, { ...ENTRY, ...declarations })); + assert.equal(result.code, 1, JSON.stringify(typed)); + assert.match(result.stderr, /: dist\/extra\.d\.ts \(the declaration beside dist\/extra\.js\) isn't in zz-fixture-1\.0\.0\.tgz/); + } const nullExports = await check(fixture({ files: ["dist"], main: "./dist/index.js", exports: null }, ENTRY)); assert.equal(nullExports.code, 0, nullExports.stderr); assert.match(nullExports.stdout, /; loaded '@zz\/fixture' \(import\)\.\n$/m); }); +test("no declaration is required beside a module the package doesn't pack, or in a package without types", async () => { + // A root-level types, with a packed module and its declaration beside it, + // and a test's and a dependency's declarations, which stay out. + const root = await check(fixture( + { files: ["index.js", "index.d.ts", "lib.js", "lib.d.ts"], main: "./index.js", types: "./index.d.ts" }, + { + "index.js": "export * from './lib.js';\n", + "index.d.ts": "export * from './lib.js';\n", + "lib.js": "export const ok = true;\n", + "lib.d.ts": "export declare const ok: boolean;\n", + "test/helper.js": "export {};\n", + "test/helper.d.ts": "export {};\n", + "node_modules/zz-dep/index.d.ts": "export {};\n", + }, + )); + assert.equal(root.code, 0, root.stderr); + // Without types, a declaration the build wrote may stay out. + const untyped = await check(fixture({ files: ["dist/**/*.js"], main: "./dist/index.js" }, { ...ENTRY, "dist/index.d.ts": "export declare const ok: boolean;\n" })); + assert.equal(untyped.code, 0, untyped.stderr); +}); + +test("npm pack output that isn't one record with a filename and its files fails, saying so", async () => { + for (const printed of ["zz: not JSON", JSON.stringify({ "@zz/fixture": { filename: "zz-fixture-1.0.0.tgz" } })]) { + const npm = shim("npm", `#!/bin/sh\nprintf '%s\\n' ${JSON.stringify(printed)}\n`); + const result = await check(fixture({ files: ["dist"], main: "./dist/index.js" }, ENTRY), NO_BUN, `${npm}:`); + assert.equal(result.code, 1, printed); + assert.equal(result.stderr, `${printed}\npack-installs-alone: npm pack --json didn't print one record with a filename and its files (its output is above).\n`); + } +}); + test("npm pack's JSON is read in npm 12's shape too, and a prepack's output doesn't break it", async () => { // npm 12 prints an object keyed by package name: a shim in front of the // real npm rewrites npm 11's array that way, and passes npm 12's through.