From 6479fc442e861a31611ed75f3a78c16a795d8991 Mon Sep 17 00:00:00 2001 From: Justin Murray Date: Fri, 31 Jul 2026 15:42:21 -0400 Subject: [PATCH] fix(ci): publish npm tarballs packed by bun so workspace:* resolves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `npm publish` run from inside a package directory does not rewrite `workspace:*` dependency specs — that substitution only happens for npm-managed workspaces, and this repo installs with bun. The release workflow did `cd packages/client && npm publish`, so the literal spec shipped to the registry: "dependencies": { "@memory.build/protocol": "workspace:*" } Every consumer install of @memory.build/client@0.6.2 then failed with: error: Workspace dependency "@memory.build/protocol" not found This is a real runtime break, not just a metadata wart: the built dist/index.js genuinely imports @memory.build/protocol{,/fields, /headers,/meta}. `bun pm pack` does resolve workspace protocols to the concrete version, so pack with bun and hand npm the finished tarball. npm still performs the upload, so OIDC trusted publishing and provenance are unaffected. Also add a verify gate that fails the release if any workspace:/file:/ link:/portal: spec survives packing, so this cannot silently reach the registry again. Two sharp edges worth recording: - `npm publish ` is parsed as a GitHub `org/repo` shorthand (it tries to `git ls-remote` it), so the tarball paths need a leading `./`. - `prepublishOnly` hooks do not fire when publishing a pre-made tarball; the workflow's explicit build steps are what produce `dist/`. test:unit now scans ./scripts too, so the new test actually runs in CI. The find roots are `./`-prefixed because bun test treats bare arguments as filters rather than paths, which silently dropped the new file. Fixes TNT-254 --- .github/workflows/release.yml | 33 +++++- package.json | 2 +- scripts/npm/verify-tarballs.test.ts | 54 ++++++++++ scripts/npm/verify-tarballs.ts | 152 ++++++++++++++++++++++++++++ 4 files changed, 238 insertions(+), 3 deletions(-) create mode 100644 scripts/npm/verify-tarballs.test.ts create mode 100644 scripts/npm/verify-tarballs.ts diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 9f45ea28..077ad60e 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -54,11 +54,40 @@ jobs: - name: Install npm 11.x (OIDC trusted publishing; 12.0.0 breaks provenance) run: npm install -g 'npm@^11.5.1' + # Pack with bun, publish the tarball with npm. + # + # `npm publish` from inside a package dir does NOT rewrite + # `workspace:*` dependency specs — that substitution only happens for + # npm-managed workspaces, and this repo installs with bun. So publishing + # directly shipped a literal `"@memory.build/protocol": "workspace:*"` + # in @memory.build/client's manifest, and every consumer install died + # with `Workspace dependency "@memory.build/protocol" not found`. + # + # `bun pm pack` resolves workspace protocols to the concrete version + # (`"@memory.build/protocol": ""`), so pack with + # bun and hand npm the finished tarball. npm still owns the upload, so + # OIDC trusted publishing and provenance work exactly as before. + # + # Note the packages' `prepublishOnly` build hooks do not fire when + # publishing a pre-made tarball — the explicit build steps above are + # what produce `dist/`. + - name: Pack npm tarballs (resolves workspace:* specs) + run: | + mkdir -p npm-tarballs + ./bun pm pack --cwd packages/protocol --destination "$PWD/npm-tarballs" + ./bun pm pack --cwd packages/client --destination "$PWD/npm-tarballs" + + # Fail loudly rather than shipping another broken manifest. + - name: Verify no unresolved workspace specs survived packing + run: ./bun scripts/npm/verify-tarballs.ts ./npm-tarballs + + # The `./` prefix is required: npm reads a bare relative path as a GitHub + # `org/repo` shorthand and tries to `git ls-remote` it. - name: Publish @memory.build/protocol - run: cd packages/protocol && npm publish --access public + run: npm publish --access public "./npm-tarballs/memory.build-protocol-${GITHUB_REF_NAME#v}.tgz" - name: Publish @memory.build/client - run: cd packages/client && npm publish --access public + run: npm publish --access public "./npm-tarballs/memory.build-client-${GITHUB_REF_NAME#v}.tgz" build-cli: # Build on macOS so the darwin binary can be ad-hoc signed. diff --git a/package.json b/package.json index ab0ac6e2..77570913 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,7 @@ "test:db:clean:all": "./bun scripts/clean-test-schemas.ts --all", "test:e2e": "./bun run test:db:clean && ./bun test --timeout 60000 ./e2e", "test:harness-smoke": "find ./packages/cli/harness-smoke -name '*.smoke.ts' -print0 | xargs -0 ./bun test --timeout 150000", - "test:unit": "find packages -name '*.test.ts' ! -name '*.integration.test.ts' -print0 | xargs -0 ./bun test --parallel=2", + "test:unit": "find ./packages ./scripts -name '*.test.ts' ! -name '*.integration.test.ts' ! -path '*/node_modules/*' -print0 | xargs -0 ./bun test --parallel=2", "typecheck": "tsc --noEmit && ./bun run web:typecheck" }, "devDependencies": { diff --git a/scripts/npm/verify-tarballs.test.ts b/scripts/npm/verify-tarballs.test.ts new file mode 100644 index 00000000..8fca92f4 --- /dev/null +++ b/scripts/npm/verify-tarballs.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, test } from "bun:test"; + +import { findUnpublishableSpecs } from "./verify-tarballs.ts"; + +describe("findUnpublishableSpecs", () => { + test("flags the workspace:* spec that broke @memory.build/client@0.6.2", () => { + const problems = findUnpublishableSpecs({ + name: "@memory.build/client", + version: "0.6.2", + dependencies: { "@memory.build/protocol": "workspace:*" }, + }); + + expect(problems).toEqual([ + { + field: "dependencies", + name: "@memory.build/protocol", + spec: "workspace:*", + }, + ]); + }); + + test("accepts a manifest whose workspace spec was resolved by bun pm pack", () => { + expect( + findUnpublishableSpecs({ + name: "@memory.build/client", + version: "0.6.2", + dependencies: { "@memory.build/protocol": "0.6.2" }, + peerDependencies: { typescript: "^5.0.0" }, + }), + ).toEqual([]); + }); + + test("checks every dependency field, not just dependencies", () => { + const problems = findUnpublishableSpecs({ + dependencies: { a: "1.0.0" }, + devDependencies: { b: "workspace:^" }, + peerDependencies: { c: "link:../c" }, + optionalDependencies: { d: "file:../d" }, + }); + + expect(problems.map((p) => `${p.field}.${p.name}`).sort()).toEqual([ + "devDependencies.b", + "optionalDependencies.d", + "peerDependencies.c", + ]); + }); + + test("tolerates manifests with no dependency fields or odd shapes", () => { + expect(findUnpublishableSpecs({ name: "x", version: "1.0.0" })).toEqual([]); + expect(findUnpublishableSpecs({ dependencies: null })).toEqual([]); + expect(findUnpublishableSpecs(null)).toEqual([]); + expect(findUnpublishableSpecs("nonsense")).toEqual([]); + }); +}); diff --git a/scripts/npm/verify-tarballs.ts b/scripts/npm/verify-tarballs.ts new file mode 100644 index 00000000..f76eef4a --- /dev/null +++ b/scripts/npm/verify-tarballs.ts @@ -0,0 +1,152 @@ +#!/usr/bin/env bun + +// Verifies packed npm tarballs before they are published. +// +// The bug this exists to prevent: `npm publish` run from inside a package +// directory does not rewrite `workspace:*` dependency specs (npm only does that +// for npm-managed workspaces, and this repo installs with bun). That shipped +// @memory.build/client@0.6.2 with a literal +// +// "dependencies": { "@memory.build/protocol": "workspace:*" } +// +// so every consumer install failed with +// `Workspace dependency "@memory.build/protocol" not found`. +// +// The release workflow now packs with `bun pm pack` (which resolves workspace +// protocols to concrete versions) and publishes the resulting tarball. This +// script asserts that actually happened, so a regression fails the release +// instead of reaching the registry. +// +// Usage: +// ./bun scripts/npm/verify-tarballs.ts [...] + +import { readdir } from "node:fs/promises"; +import { basename, join } from "node:path"; + +const DEPENDENCY_FIELDS = [ + "dependencies", + "devDependencies", + "peerDependencies", + "optionalDependencies", +] as const; + +/** A spec that must never reach the registry: unresolvable by consumers. */ +const UNPUBLISHABLE_SPEC_PREFIXES = ["workspace:", "file:", "link:", "portal:"]; + +function fail(message: string): never { + console.error(`error: ${message}`); + process.exit(1); +} + +/** Read `package/package.json` out of a tarball, via tar on stdout. */ +async function readPackedManifest(tarball: string): Promise { + const proc = Bun.spawn(["tar", "-xzOf", tarball, "package/package.json"], { + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + if (exitCode !== 0) { + fail(`could not read package.json from ${tarball}: ${stderr.trim()}`); + } + try { + return JSON.parse(stdout); + } catch (cause) { + fail(`package.json in ${tarball} is not valid JSON: ${String(cause)}`); + } +} + +/** Collect every `.tgz` implied by the given files/directories. */ +async function resolveTarballPaths(inputs: string[]): Promise { + const tarballs: string[] = []; + for (const input of inputs) { + const stat = await Bun.file(input) + .stat() + .catch(() => null); + if (stat?.isDirectory()) { + const entries = await readdir(input); + tarballs.push( + ...entries.filter((e) => e.endsWith(".tgz")).map((e) => join(input, e)), + ); + } else if (stat) { + tarballs.push(input); + } else { + fail(`no such file or directory: ${input}`); + } + } + return tarballs.sort(); +} + +/** + * Check one packed manifest, returning human-readable problems. + * + * Exported for tests; the checks are pure so they can be exercised without + * building or packing anything. + */ +export function findUnpublishableSpecs( + manifest: unknown, +): { field: string; name: string; spec: string }[] { + const problems: { field: string; name: string; spec: string }[] = []; + if (typeof manifest !== "object" || manifest === null) return problems; + const record = manifest as Record; + + for (const field of DEPENDENCY_FIELDS) { + const deps = record[field]; + if (typeof deps !== "object" || deps === null) continue; + for (const [name, spec] of Object.entries( + deps as Record, + )) { + if (typeof spec !== "string") continue; + if (UNPUBLISHABLE_SPEC_PREFIXES.some((p) => spec.startsWith(p))) { + problems.push({ field, name, spec }); + } + } + } + return problems; +} + +// --- Main --- + +if (import.meta.main) { + const inputs = process.argv.slice(2); + if (inputs.length === 0) { + fail("usage: verify-tarballs.ts [...]"); + } + + const tarballs = await resolveTarballPaths(inputs); + if (tarballs.length === 0) { + fail(`no .tgz tarballs found in: ${inputs.join(", ")}`); + } + + let failed = false; + for (const tarball of tarballs) { + const manifest = await readPackedManifest(tarball); + const { name, version } = manifest as { name?: string; version?: string }; + const problems = findUnpublishableSpecs(manifest); + + if (problems.length > 0) { + failed = true; + console.error(`\n${basename(tarball)} (${name}@${version}) is broken:`); + for (const { field, name: dep, spec } of problems) { + console.error( + ` ${field}.${dep} = "${spec}" (unresolvable for consumers)`, + ); + } + } else { + console.log(`ok ${name}@${version} (${basename(tarball)})`); + } + } + + if (failed) { + console.error( + "\nPack tarballs with `bun pm pack` (it resolves workspace: specs to " + + "concrete versions), not `npm publish` from the package directory.", + ); + process.exit(1); + } + + console.log(`\nVerified ${tarballs.length} tarball(s).`); +}