From 2bd6b7b0489388872b5d2d2d6b9f80abe1ec66ef Mon Sep 17 00:00:00 2001 From: Taras Mankovski Date: Tue, 22 Sep 2026 20:37:58 -0400 Subject: [PATCH 1/3] =?UTF-8?q?=F0=9F=90=9B=20Build=20npm=20packages=20fro?= =?UTF-8?q?m=20their=20local=20release=20closure?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A tagged publish waited on npm indexing a sibling the previous job had just published, because the builder rewrote every `workspace:*` dependency to `^` before calling dnt. `publish-one.yml` bounded that wait with four attempts and 15-second sleeps; the observed lag exceeded it on 0.12.0, 0.12.1 and 0.13.1, and 0.13.1 reached npm only after four rounds of rerunning failed jobs. `scripts/build-npm.ts` now builds in two phases. Phase 1 builds the requested package's internal dependencies depth-first from the same checkout, each at most once, and hands dnt absolute `file:` ranges naming those artifacts, so the install and the type check never reach the registry for a package from this release. Phase 2 finalizes every manifest in the closure together, once the last dnt call has returned, replacing each internal `file:` range with the sibling's `^`. Together, not per package: in a chain A → B → C, rewriting B when its own build finishes puts C's registry version back in front of A's install. Before reporting success the builder refuses any dependency range still starting with `workspace:` or `file:`, and any string naming the checkout's path — a release gate, so an unexpected local dependency fails the build instead of being normalized into something npm would accept. An internal dependency no workspace member declares is refused by name, with no registry fallback. `DNT_LOCAL_SIBLINGS` is gone: the default path is what it used to provide, and the artifact a developer builds is now the artifact a release publishes. `DNT_SKIP_INSTALL` keeps its leaf-only contract. `publish-one.yml` builds once and keeps its already-published guard; `publish-packages.yml` keeps its dependency-ordered `needs:`, which is now a publication guarantee rather than a build one — a failed upstream still withholds its dependents, so whatever npm holds is dependency-closed. The builder is importable, so the regression drives the real path over a three-member closure whose versions npm has never seen. --- .github/workflows/publish-one.yml | 16 +- scripts/build-npm.ts | 299 +++++++++++++++--- scripts/tests/adapter-npm-package.test.ts | 1 - scripts/tests/build-npm.test.ts | 218 ++++++++++++- scripts/tests/cli-npm-bin.test.ts | 56 +++- .../tests/publish-workflow-membership.test.ts | 28 ++ specs/release-process-spec.md | 90 ++++-- 7 files changed, 602 insertions(+), 106 deletions(-) diff --git a/.github/workflows/publish-one.yml b/.github/workflows/publish-one.yml index 82a026a0a..1ef6b4b3c 100644 --- a/.github/workflows/publish-one.yml +++ b/.github/workflows/publish-one.yml @@ -56,18 +56,12 @@ jobs: - name: Build the browser bundle run: deno task build:web + # One attempt. The builder constructs this package's internal closure from + # this checkout, so nothing here waits on npm indexing a sibling the + # previous job published — the wait the retry loop used to bound, and + # never reliably did (#843). - name: Build with dnt - # Retry to absorb npm propagation of a just-published sibling dependency. - run: | - set -eu - for attempt in 1 2 3 4; do - if deno run -A scripts/build-npm.ts "${{ inputs.package }}" "${{ inputs.version }}"; then - exit 0 - fi - [ "$attempt" -lt 4 ] && sleep 15 || true - done - echo "build failed after 4 attempts" >&2 - exit 1 + run: deno run -A scripts/build-npm.ts "${{ inputs.package }}" "${{ inputs.version }}" # Idempotent: skip a version npm already has, so tag re-runs succeed. - name: Publish to npm diff --git a/scripts/build-npm.ts b/scripts/build-npm.ts index df634e3ee..447bc9689 100644 --- a/scripts/build-npm.ts +++ b/scripts/build-npm.ts @@ -11,14 +11,27 @@ * Everything published is derived from the member's own deno.json (name, * exports) and package.json (dependencies, description, bin) — those manifests * are the single source of truth. Internal @executablemd siblings are declared - * as external npm dependencies (resolved to the sibling's own version), never - * inlined, so each published package resolves them from npm. + * as external npm dependencies, never inlined. * - * `DNT_LOCAL_SIBLINGS=1` builds each internal sibling first and depends on those - * artifacts by path instead of by published version, so a branch can build and - * type-check against its own workspace sources. The resulting package.json names - * local directories and is therefore unpublishable; release workflows never set - * the variable. + * A build happens in two phases, and the split is what keeps a release off the + * registry's clock. + * + * **Phase 1 builds the local closure.** The requested package's internal + * dependencies are built first, depth-first, each at most once, and handed to + * dnt as absolute `file:` ranges pointing at the artifacts this same invocation + * produced. Nothing asks npm for a package from the same release. + * + * **Phase 2 finalizes every manifest together**, once the last dnt call has + * returned, replacing each internal `file:` range with the sibling's + * `^`. It has to be every manifest at once: in a chain A → B → C, + * rewriting B the moment its own dnt call returns puts C's registry version + * back in front of A's install, which is the race one level down. No install or + * typecheck runs after finalization begins, and a surviving local reference + * fails the build rather than reaching `npm publish`. + * + * `DNT_SKIP_INSTALL=1` still skips the install and typecheck for a leaf package + * that declares no `workspace:*` dependency, for exercising the tooling. It + * refuses anything else, and release workflows never set it. */ import { ensure, exit, main, scoped, until } from "effection"; @@ -126,36 +139,69 @@ interface WorkspaceMember { version: string; } +/** + * What one invocation reports as it runs. Diagnostic observation only: an + * observer chooses no build, no dependency resolution and no finalization + * policy, which is what keeps the regression watching the real path instead of + * a second one. + */ +export type BuildEvent = + | { + readonly type: "package-build-started"; + readonly package: string; + /** Exactly what dnt was handed, so a test can see the local ranges. */ + readonly dependencies: Readonly>; + } + | { readonly type: "package-build-completed"; readonly package: string } + | { readonly type: "closure-finalization-started" } + | { readonly type: "package-manifest-finalized"; readonly package: string }; + +export interface BuildNpmOptions { + /** The workspace root the closure is built from. */ + repoRoot: URL; + /** The requested member's directory, e.g. `packages/cli`. */ + package: string; + /** The npm version for the requested artifact; siblings use their own. */ + version: string; + observe?: (event: BuildEvent) => Operation; +} + +/** One package this invocation built, awaiting closure-wide finalization. */ +interface BuiltArtifact { + name: string; + dir: string; + /** Internal dependency name -> the workspace version to finalize it to. */ + internal: Record; +} + interface BuildContext { repoRoot: URL; rootDeno: z.infer; members: Record; - /** Depend on locally built sibling artifacts instead of published versions. */ - localSiblings: boolean; skipInstall: boolean; - /** Package names already built in this process, so a diamond builds once. */ - built: Set; + /** Artifacts already built in this process, so a diamond builds once. */ + built: Map; + observe: (event: BuildEvent) => Operation; } -await main(function* (args) { - const pkgArg = args[0]; - const version = args[1] ?? "0.0.0-dev"; - - if (!pkgArg) { - console.error("usage: build-npm.ts [version]"); - yield* exit(1); - return; - } +/** + * npm resolves a relative `file:` against the dependent's own location, which + * differs for a sibling installed under another package's node_modules, so the + * range this invocation hands dnt is absolute. + */ +function localRange(repoRoot: URL, dir: string): string { + return `file:${fromFileUrl(new URL(`${dir}/npm`, repoRoot))}`; +} - // Before anything is emitted. Copying the documentation assets is not the - // same as validating them: a package built from a set that has drifted from - // the components it documents would install cleanly and refuse the first time - // somebody asked it for documentation. The same assembly the run profile uses - // runs here, so a missing, unknown or duplicated section fails the build for - // exactly the reason it would fail a run. - yield* validateDocumentation(); +function* observeNothing(): Operation {} - const repoRoot = new URL("../", import.meta.url); +/** + * Build `options.package` and its internal closure, and leave every generated + * manifest publishable. This is the whole builder; the command below is an + * adapter over it, so the regression exercises the release path itself. + */ +export function* buildNpmPackage(options: BuildNpmOptions): Operation { + const { repoRoot } = options; const rootDeno = RootDenoSchema.parse( JSON.parse(yield* readTextFile(new URL("deno.json", repoRoot))), @@ -175,15 +221,18 @@ await main(function* (args) { } } - yield* buildPackage(pkgArg, version, { + const ctx: BuildContext = { repoRoot, rootDeno, members, - localSiblings: Deno.env.get("DNT_LOCAL_SIBLINGS") === "1", skipInstall: Deno.env.get("DNT_SKIP_INSTALL") === "1", - built: new Set(), - }); -}); + built: new Map(), + observe: options.observe ?? observeNothing, + }; + + yield* buildPackage(options.package, options.version, ctx); + yield* finalizeClosure(ctx); +} function* buildPackage(pkgArg: string, version: string, ctx: BuildContext): Operation { const { repoRoot, rootDeno, skipInstall } = ctx; @@ -200,9 +249,10 @@ function* buildPackage(pkgArg: string, version: string, ctx: BuildContext): Oper ); // Dependencies come from package.json verbatim, except internal siblings - // (workspace:* protocol) which resolve to the sibling's own version range — - // or, with local siblings, to the artifact this process just built for it. + // (workspace:* protocol), which are built first and named by the artifact + // this invocation just produced. Phase 2 turns those into version ranges. const dependencies: Record = {}; + const internal: Record = {}; for (const [name, range] of Object.entries(packageJson.dependencies ?? {})) { if (!name.startsWith(INTERNAL_SCOPE)) { dependencies[name] = range; @@ -210,19 +260,17 @@ function* buildPackage(pkgArg: string, version: string, ctx: BuildContext): Oper } const member = ctx.members[name]; if (!member) { - throw new Error(`no workspace version found for internal dependency "${name}"`); - } - if (!ctx.localSiblings) { - dependencies[name] = `^${member.version}`; - continue; + // No registry fallback: reaching npm for an unmapped internal name is how + // a misspelled or removed member would silently restore the release race. + throw new Error( + `"${denoJson.name}" depends on internal package "${name}", which is not a workspace member`, + ); } if (!ctx.built.has(name)) { yield* buildPackage(member.dir, member.version, ctx); } - // An absolute path: npm resolves a relative `file:` against the dependent's - // own location, which differs for a sibling installed under another - // package's node_modules. - dependencies[name] = `file:${fromFileUrl(new URL(`${member.dir}/npm`, repoRoot))}`; + internal[name] = member.version; + dependencies[name] = localRange(repoRoot, member.dir); } // Library entry points come from deno.json exports. An executable comes from @@ -311,6 +359,8 @@ function* buildPackage(pkgArg: string, version: string, ctx: BuildContext): Oper ), ); + yield* ctx.observe({ type: "package-build-started", package: denoJson.name, dependencies }); + // The build tree is removed when this scope closes, before the finished // package is completed below. yield* scoped(function* () { @@ -323,10 +373,10 @@ function* buildPackage(pkgArg: string, version: string, ctx: BuildContext): Oper importMap: join(srcCopy, "deno.json"), shims: { deno: false }, test: false, - // Internal @executablemd deps are published tier-by-tier, so a downstream - // package's siblings are already on npm when it builds in CI. For local - // builds (before siblings are published) set DNT_SKIP_INSTALL=1 to skip - // the npm install + type check that would otherwise 404 on them. + // The install resolves internal siblings from the artifacts phase 1 + // already built, so it never reaches npm for a package from this + // release. DNT_SKIP_INSTALL=1 drops the install and typecheck entirely, + // which only a leaf package may ask for (refused above). skipNpmInstall: skipInstall, typeCheck: skipInstall ? false : "single", declaration: "separate", @@ -383,8 +433,153 @@ function* buildPackage(pkgArg: string, version: string, ctx: BuildContext): Oper yield* copyFile(new URL(asset, pkgDir), target); } - ctx.built.add(denoJson.name); - const provenance = - ctx.localSiblings && workspaceDeps.length > 0 ? " (local siblings — not publishable)" : ""; - console.log(`built ${denoJson.name}@${version} -> ${pkgArg}/npm${provenance}`); + ctx.built.set(denoJson.name, { name: denoJson.name, dir: pkgArg, internal }); + yield* ctx.observe({ type: "package-build-completed", package: denoJson.name }); + console.log(`built ${denoJson.name}@${version} -> ${pkgArg}/npm`); +} + +/** The dependency maps npm reads, so validation misses none of them. */ +const DEPENDENCY_FIELDS = [ + "dependencies", + "devDependencies", + "peerDependencies", + "optionalDependencies", +]; + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +/** Every string anywhere in `value`, so a workspace path cannot hide in a field nobody checks. */ +function* strings(value: unknown): Generator { + if (typeof value === "string") { + yield value; + return; + } + if (Array.isArray(value)) { + for (const item of value) { + yield* strings(item); + } + return; + } + if (isRecord(value)) { + for (const item of Object.values(value)) { + yield* strings(item); + } + } +} + +/** + * Why `manifest` cannot be published, or `undefined` when it can. + * + * A release gate rather than cleanup. A known internal local range is rewritten + * by the caller before this runs; anything local still standing is an + * unexpected dependency, and normalizing it away would be the one edit that + * makes an unpublishable artifact look fine. + */ +function unpublishable(manifest: unknown, repoRoot: URL): string | undefined { + if (!isRecord(manifest)) { + return "it is not an object"; + } + + for (const field of DEPENDENCY_FIELDS) { + const map = manifest[field]; + if (!isRecord(map)) { + continue; + } + for (const [name, range] of Object.entries(map)) { + if ( + typeof range === "string" && + (range.startsWith("workspace:") || range.startsWith("file:")) + ) { + return `${field}["${name}"] is the local range ${range}`; + } + } + } + + const nativeRoot = fromFileUrl(repoRoot).replace(new RegExp(`${sep}$`), ""); + const urlRoot = repoRoot.href.replace(/\/$/, ""); + for (const value of strings(manifest)) { + if (value.includes(nativeRoot)) { + return `it names the workspace path ${nativeRoot}`; + } + if (value.includes(urlRoot)) { + return `it names the workspace URL ${urlRoot}`; + } + } + + return undefined; +} + +/** + * Phase 2. Every artifact this invocation produced becomes publishable at once, + * after the last dnt call returned — see the two-phase note at the top of this + * file for why it cannot happen package by package. + */ +function* finalizeClosure(ctx: BuildContext): Operation { + yield* ctx.observe({ type: "closure-finalization-started" }); + + const candidates: Array<{ name: string; url: URL; manifest: Record }> = []; + + for (const artifact of ctx.built.values()) { + const url = new URL(`${artifact.dir}/npm/package.json`, ctx.repoRoot); + const manifest: unknown = JSON.parse(yield* readTextFile(url)); + if (!isRecord(manifest)) { + throw new Error(`${artifact.name}'s generated package.json is not an object`); + } + for (const field of DEPENDENCY_FIELDS) { + const map = manifest[field]; + if (!isRecord(map)) { + continue; + } + for (const [name, version] of Object.entries(artifact.internal)) { + const member = ctx.members[name]; + if (member && map[name] === localRange(ctx.repoRoot, member.dir)) { + map[name] = `^${version}`; + } + } + } + candidates.push({ name: artifact.name, url, manifest }); + } + + // Every manifest is judged before any is written, so a closure that cannot be + // published in full is never half-published. + for (const candidate of candidates) { + const refusal = unpublishable(candidate.manifest, ctx.repoRoot); + if (refusal !== undefined) { + throw new Error(`${candidate.name} cannot be published: ${refusal}`); + } + } + + for (const candidate of candidates) { + yield* writeTextFile(candidate.url, `${JSON.stringify(candidate.manifest, null, 2)}\n`); + yield* ctx.observe({ type: "package-manifest-finalized", package: candidate.name }); + } +} + +if (import.meta.main) { + await main(function* (args) { + const pkgArg = args[0]; + const version = args[1] ?? "0.0.0-dev"; + + if (!pkgArg) { + console.error("usage: build-npm.ts [version]"); + yield* exit(1); + return; + } + + // Before anything is emitted. Copying the documentation assets is not the + // same as validating them: a package built from a set that has drifted from + // the components it documents would install cleanly and refuse the first + // time somebody asked it for documentation. The same assembly the run + // profile uses runs here, so a missing, unknown or duplicated section fails + // the build for exactly the reason it would fail a run. + yield* validateDocumentation(); + + yield* buildNpmPackage({ + repoRoot: new URL("../", import.meta.url), + package: pkgArg, + version, + }); + }); } diff --git a/scripts/tests/adapter-npm-package.test.ts b/scripts/tests/adapter-npm-package.test.ts index bb406f65d..1277d5480 100644 --- a/scripts/tests/adapter-npm-package.test.ts +++ b/scripts/tests/adapter-npm-package.test.ts @@ -67,7 +67,6 @@ function useBuiltPackage(): Operation { // cache and npm configuration, and a bare pair would drop both. env: { ...inheritedEnvironment(), - DNT_LOCAL_SIBLINGS: "1", npm_config_legacy_peer_deps: "true", }, }); diff --git a/scripts/tests/build-npm.test.ts b/scripts/tests/build-npm.test.ts index 3f2aaf3b0..fd34fe497 100644 --- a/scripts/tests/build-npm.test.ts +++ b/scripts/tests/build-npm.test.ts @@ -3,9 +3,13 @@ import { expect } from "@executablemd/test-support/expect"; import { ensure } from "effection"; import type { Operation } from "effection"; import { exec, Stdio } from "@effectionx/process"; -import { exists, readTextFile, rm } from "@effectionx/fs"; +import { ensureDir, exists, readTextFile, rm, writeTextFile } from "@effectionx/fs"; +import { useTempDirectory } from "@executablemd/test-support/temp"; import path from "node:path"; -import { fileURLToPath } from "node:url"; +import { fileURLToPath, pathToFileURL } from "node:url"; + +import { buildNpmPackage } from "../build-npm.ts"; +import type { BuildEvent } from "../build-npm.ts"; const ROOT = fileURLToPath(new URL("../../", import.meta.url)); @@ -110,3 +114,213 @@ describe("build-npm skip-install mode", () => { } }); }); + +/** + * A version no `@executablemd/fixture-*` package has on npm, so nothing here + * resolves unless the artifact beside it was the one consumed. + */ +const FIXTURE_VERSION = "9.9.9-closure"; + +function scoped(name: string): string { + return `@executablemd/fixture-${name}`; +} + +const C_SOURCE = `export interface CValue {\n readonly c: string;\n}\nexport const c: CValue = { c: "c" };\n`; + +function* member( + root: URL, + name: string, + dependencies: Record, + source: string, +): Operation { + yield* ensureDir(new URL(`packages/${name}/`, root)); + yield* writeTextFile( + new URL(`packages/${name}/deno.json`, root), + `${JSON.stringify({ name: scoped(name), version: FIXTURE_VERSION, exports: "./mod.ts" }, null, 2)}\n`, + ); + yield* writeTextFile( + new URL(`packages/${name}/package.json`, root), + `${JSON.stringify( + { name: scoped(name), version: FIXTURE_VERSION, type: "module", dependencies }, + null, + 2, + )}\n`, + ); + yield* writeTextFile(new URL(`packages/${name}/mod.ts`, root), source); +} + +/** + * `a -> b -> c`, in a workspace of its own. Never the repository's members: the + * point is a closure whose versions npm has never seen. + */ +function* closureWorkspace(): Operation { + const base = yield* useTempDirectory("build-npm-closure-"); + const root = pathToFileURL(`${base}/`); + + yield* writeTextFile( + new URL("deno.json", root), + `${JSON.stringify({ workspace: ["packages/*"], imports: {} }, null, 2)}\n`, + ); + + // `b` takes a type from `c`, so `b`'s own typecheck resolves `c` through the + // artifact phase 1 built for it. A chain of plain values would compile even + // if `c` were never consumed as a package at all. + yield* member(root, "c", {}, C_SOURCE); + yield* member( + root, + "b", + { [scoped("c")]: "workspace:*" }, + `import type { CValue } from "${scoped("c")}";\nimport { c } from "${scoped( + "c", + )}";\nexport const b: CValue = c;\n`, + ); + yield* member( + root, + "a", + { [scoped("b")]: "workspace:*" }, + `import { b } from "${scoped("b")}";\nexport const a = b.c + "a";\n`, + ); + + return root; +} + +function* manifestOf(root: URL, name: string): Operation> { + const text = yield* readTextFile(new URL(`packages/${name}/npm/package.json`, root)); + return JSON.parse(text).dependencies ?? {}; +} + +function localRange(root: URL, name: string): string { + return `file:${fileURLToPath(new URL(`packages/${name}/npm`, root))}`; +} + +/** + * The builder's own two phases, observed on a closure npm has never published. + * + * The repository's own members cannot tell a local closure apart from a + * registry build, because every version they name is already on npm. These + * fixtures can: nothing here resolves unless the artifact beside it was the one + * consumed. + */ +describe("build-npm local closure", () => { + it("N1: hands dnt the artifacts it just built, for the whole closure", function* () { + const root = yield* closureWorkspace(); + const events: BuildEvent[] = []; + + yield* buildNpmPackage({ + repoRoot: root, + package: "packages/a", + version: FIXTURE_VERSION, + *observe(event) { + events.push(event); + }, + }); + + const started = events.filter((event) => event.type === "package-build-started"); + expect(started.map((event) => event.package)).toEqual([scoped("c"), scoped("b"), scoped("a")]); + expect(started[1].dependencies).toEqual({ [scoped("c")]: localRange(root, "c") }); + expect(started[2].dependencies).toEqual({ [scoped("b")]: localRange(root, "b") }); + }); + + it("N2: finalizes nothing until every build in the closure has returned", function* () { + const root = yield* closureWorkspace(); + const events: BuildEvent[] = []; + const atStartOfA: Record[] = []; + + yield* buildNpmPackage({ + repoRoot: root, + package: "packages/a", + version: FIXTURE_VERSION, + *observe(event) { + events.push(event); + if (event.type === "package-build-started" && event.package === scoped("a")) { + // b is built by now; it must still be naming c locally, or a's own + // install would be resolving c from the registry. + atStartOfA.push(yield* manifestOf(root, "b")); + } + }, + }); + + expect(atStartOfA).toEqual([{ [scoped("c")]: localRange(root, "c") }]); + + const order = events.map((event) => event.type); + const finalization = order.indexOf("closure-finalization-started"); + expect(finalization).toBeGreaterThan(-1); + expect(order.lastIndexOf("package-build-completed")).toBeLessThan(finalization); + expect(order.lastIndexOf("package-build-started")).toBeLessThan(finalization); + expect(order.filter((type) => type === "package-manifest-finalized")).toHaveLength(3); + }); + + it("N3: leaves every manifest in the closure publishable", function* () { + const root = yield* closureWorkspace(); + + yield* buildNpmPackage({ repoRoot: root, package: "packages/a", version: FIXTURE_VERSION }); + + expect(yield* manifestOf(root, "a")).toEqual({ [scoped("b")]: `^${FIXTURE_VERSION}` }); + expect(yield* manifestOf(root, "b")).toEqual({ [scoped("c")]: `^${FIXTURE_VERSION}` }); + expect(yield* manifestOf(root, "c")).toEqual({}); + + const workspacePath = fileURLToPath(root).replace(/\/$/, ""); + for (const name of ["a", "b", "c"]) { + const text = yield* readTextFile(new URL(`packages/${name}/npm/package.json`, root)); + expect({ name, leaked: text.includes(workspacePath) || text.includes("workspace:") }).toEqual( + { name, leaked: false }, + ); + } + }); + + it("N5: refuses an internal dependency no workspace member declares", function* () { + const root = yield* closureWorkspace(); + yield* member( + root, + "b", + { [scoped("c")]: "workspace:*", [scoped("absent")]: "workspace:*" }, + `import { c } from "${scoped("c")}";\nexport const b = c;\n`, + ); + let caught: unknown; + + try { + yield* buildNpmPackage({ repoRoot: root, package: "packages/a", version: FIXTURE_VERSION }); + } catch (error) { + caught = error; + } + + expect(caught).toMatchObject({ + message: `"${scoped("b")}" depends on internal package "${scoped( + "absent", + )}", which is not a workspace member`, + }); + // Refused before the dependent was built, so no artifact claims otherwise. + expect(yield* exists(new URL("packages/b/npm/package.json", root))).toBe(false); + }); + + it("N5: refuses a publishable manifest that kept an unrelated local dependency", function* () { + const root = yield* closureWorkspace(); + yield* ensureDir(new URL("vendor/local-dep/", root)); + yield* writeTextFile( + new URL("vendor/local-dep/package.json", root), + `${JSON.stringify({ name: "fixture-local-dep", version: "1.0.0", type: "module" }, null, 2)}\n`, + ); + yield* writeTextFile(new URL("vendor/local-dep/index.js", root), "export default {};\n"); + yield* member( + root, + "c", + { "fixture-local-dep": `file:${fileURLToPath(new URL("vendor/local-dep", root))}` }, + C_SOURCE, + ); + let caught: unknown; + + try { + yield* buildNpmPackage({ repoRoot: root, package: "packages/a", version: FIXTURE_VERSION }); + } catch (error) { + caught = error; + } + + // Consumable during dnt, refused at the gate: normalizing it away is what + // would turn an unpublishable artifact into one npm would accept. + expect(caught).toMatchObject({ + message: `${scoped("c")} cannot be published: dependencies["fixture-local-dep"] is the local range file:${fileURLToPath( + new URL("vendor/local-dep", root), + )}`, + }); + }); +}); diff --git a/scripts/tests/cli-npm-bin.test.ts b/scripts/tests/cli-npm-bin.test.ts index a659f872e..abe5fbb8d 100644 --- a/scripts/tests/cli-npm-bin.test.ts +++ b/scripts/tests/cli-npm-bin.test.ts @@ -5,12 +5,11 @@ * `@executablemd/cli@0.5.0` off npm compiles fine under Deno and fails only * here. * - * The build runs with `DNT_LOCAL_SIBLINGS=1`, so packages/cli and every - * @executablemd sibling it depends on are built from this branch's sources. A - * release build resolves those siblings from npm instead, which type-checks the - * branch against the *previous* release — green until a branch changes a shared - * API, then red for a reason the branch cannot fix. This is also the only - * coverage of the local-sibling build mode. + * The build is the ordinary one a release runs, so packages/cli and every + * @executablemd sibling it depends on are built from this branch's sources and + * consumed as artifacts. Nothing here resolves a sibling from npm, which is + * what used to type-check a branch against the *previous* release — green until + * a branch changed a shared API, then red for a reason the branch could not fix. */ import { describe, it } from "@executablemd/test-support/bdd"; import { expect } from "@executablemd/test-support/expect"; @@ -159,9 +158,6 @@ function* buildCliPackage(version: string): Operation { // match against the pinned 4.x prerelease — the same allowance // publish-one.yml makes. NPM_CONFIG_LEGACY_PEER_DEPS: "true", - // Build the siblings from this branch rather than resolving the last - // published versions of them. - DNT_LOCAL_SIBLINGS: "1", }, }).join(); } @@ -450,6 +446,48 @@ describe("npm CLI package", { sanitizeOps: false, sanitizeResources: false }, () * Run from a directory that is not the package, because the lookup must be * beside the module and never beside the caller. */ + /** + * N6. The bin above ran against the closure this build produced, and the same + * closure has to be publishable afterwards — not a verification artifact that + * works locally and names directories npm would reject. + */ + it("leaves every manifest in the closure publishable", function* () { + yield* ensure(removeNpmOutput); + const { version } = yield* readManifest(PKG_DIR, "deno.json"); + const built = yield* buildCliPackage(version ?? "0.0.0-dev"); + if (built.code !== 0) { + throw new Error(`build-npm.ts exited ${built.code}\n${built.stderr}`); + } + + const source = yield* readManifest(PKG_DIR, "package.json"); + const internal = Object.keys(source.dependencies ?? {}).filter((name) => + name.startsWith("@executablemd/"), + ); + // Non-vacuous: the CLI is the deepest closure in the workspace. + expect(internal.length).toBeGreaterThan(1); + + const emitted = yield* readEmittedManifest(); + for (const name of internal) { + expect({ name, range: emitted.dependencies?.[name] }).toEqual({ + name, + range: `^${version}`, + }); + } + + // Every sibling the closure built, not only the root that was requested. + for (const name of internal) { + const dir = path.join("packages", name.slice("@executablemd/".length)); + const sibling = yield* readManifest(dir, "npm", "package.json"); + for (const [dependency, range] of Object.entries(sibling.dependencies ?? {})) { + expect({ sibling: name, dependency, local: range.startsWith("file:") }).toEqual({ + sibling: name, + dependency, + local: false, + }); + } + } + }); + it("reports the same Component identity the source tree ships", function* () { yield* ensure(removeNpmOutput); const { version } = yield* readManifest(PKG_DIR, "deno.json"); diff --git a/scripts/tests/publish-workflow-membership.test.ts b/scripts/tests/publish-workflow-membership.test.ts index 1008a3b43..236fe01cb 100644 --- a/scripts/tests/publish-workflow-membership.test.ts +++ b/scripts/tests/publish-workflow-membership.test.ts @@ -238,6 +238,34 @@ describe("publish-packages.yml membership", () => { }); }); +/** + * The reusable publish job builds once. Four attempts with sleeps between them + * were how this repository tolerated npm indexing a sibling the previous job + * had just published; they bounded the race rather than removing it, and three + * consecutive releases lost to it anyway (#843). The builder now constructs the + * closure from the checkout, so a retry here would only be timing-based + * recovery for a wait that no longer exists. + */ +describe("publish-one.yml build step", () => { + it("invokes the builder once, with no propagation retry", function* () { + const commands = (yield* readTextFile(PUBLISH_ONE_WORKFLOW)) + .split("\n") + .filter((line) => !line.trim().startsWith("#")) + .join("\n"); + + const invocations = commands.split("scripts/build-npm.ts").length - 1; + expect(invocations).toBe(1); + + for (const timing of ["sleep", "for attempt", "attempts"]) { + expect({ timing, present: commands.includes(timing) }).toEqual({ timing, present: false }); + } + + // The guard that makes a tag rerun idempotent is a different mechanism and + // stays: removing the retry must not remove it. + expect(commands).toContain("is already on npm — skipping"); + }); +}); + /** * Both workflows refuse a tag the manifests do not declare, and they have to * refuse it for the same set of manifests. v0.13.0 is what it costs when they diff --git a/specs/release-process-spec.md b/specs/release-process-spec.md index 2eaa7c3d2..2c1f3a15a 100644 --- a/specs/release-process-spec.md +++ b/specs/release-process-spec.md @@ -165,9 +165,17 @@ documents at the revision it checks. if the binary build fails. It then fans out one `publish-one.yml` call per package, ordered with `needs:` so dependencies publish before dependents (leaves run in parallel), plus one `jsr` job for the whole workspace. + + That ordering is a publication guarantee, not a build one. Each job builds its + own closure from the tag's checkout, so no job waits for another's artifact to + reach npm. What the edges still buy is that a failed upstream publish + withholds every dependent: whatever npm ends up holding is dependency-closed, + and never a dependent whose dependency never published. - **`publish-one.yml`** (`workflow_call`, inputs `package`/`version`): builds one package with dnt (`scripts/build-npm.ts`) and publishes it to npm. Runs in - the `npm-publish` environment. npm publishing is idempotent: it skips an + the `npm-publish` environment. The build is one attempt: it constructs its own + closure from the checkout, so there is no registry propagation left for a + retry to wait out. npm publishing is idempotent: it skips an already-published version. Library entry points come from the member's `deno.json` `exports`; an executable comes from its `package.json` `bin`, so the npm CLI ships `packages/cli/src/node.ts` while JSR gets the Deno @@ -180,28 +188,45 @@ something a package cannot ship, because npm strips `.npmrc` from published tarballs. Verify it on the emitted manifest: after a `scripts/build-npm.ts` run, `packages//npm/package.json` declares no `@jsr/*` dependency. -### Local builds - -A normal build installs the package's dependencies from npm and resolves its -siblings to their published versions. That is the path `publish-one.yml` runs. - -`DNT_SKIP_INSTALL=1` skips that install and the type check, for exercising the -tooling before a version reaches npm. It covers only packages that declare no -`workspace:*` dependencies. dnt emits through TypeScript, which resolves from -the output directory, so the install is what supplies a sibling's declarations; -without it a sibling resolves to its workspace source and lands in the package. -The builder therefore refuses a package that declares one, naming the -dependencies and leaving the output directory empty. Release workflows never set -the variable. - -`DNT_LOCAL_SIBLINGS=1` builds each internal sibling first — depth-first over the -`workspace:*` dependencies, once per package — and depends on those artifacts by -absolute path (`file:/npm`) instead of by published version. The -build therefore type-checks against the sources in the working tree, which is -what a branch changing a shared API needs and what the `packages/cli` npm suite -runs. The emitted package.json names local directories, so an artifact built this -way is a verification artifact, never a publishable one. Release workflows never -set the variable. +### Building a package + +One build serves both purposes, and it runs in two phases. + +**Phase 1 builds the local closure.** The requested package's internal +dependencies are built first — depth-first over the `workspace:*` dependencies, +each at most once, so a diamond shares one artifact — and are handed to dnt as +absolute `file:/npm` ranges naming the artifacts this same +invocation produced. The install and the type check therefore resolve every +sibling from the working tree, which is what a branch changing a shared API +needs, and no build asks npm for a package from its own release. + +**Phase 2 finalizes every manifest together**, once the last dnt call has +returned. Each internal `file:` range becomes the sibling's `^`, taken +from the workspace manifests rather than from the version on the command line. +It is the whole closure at once and not each package as its own build finishes: +in a chain A → B → C, rewriting B early puts C's registry version back in front +of A's install. No install or type check runs after finalization begins. + +**The result is publishable, or the build fails.** Before reporting success the +builder inspects every generated manifest and refuses a dependency range +beginning with `workspace:` or `file:`, and any string naming the checkout's +path or `file:` URL. A known internal range is rewritten; anything else local is +an unexpected dependency, and normalizing it away is exactly the edit that would +make an unpublishable artifact look fine. An internal dependency that no +workspace member declares is refused by name before its dependent is built — +there is no registry fallback, because falling back is how a misspelled member +would quietly restore the wait this design removes. + +There is no separate verification mode: the artifact a developer builds is the +artifact a release publishes. + +`DNT_SKIP_INSTALL=1` skips the install and the type check, for exercising the +tooling. It covers only packages that declare no `workspace:*` dependencies. dnt +emits through TypeScript, which resolves from the output directory, so the +install is what supplies a sibling's declarations; without it a sibling resolves +to its workspace source and lands in the package. The builder therefore refuses +a package that declares one, naming the dependencies and leaving the output +directory empty. Release workflows never set the variable. ### JSR publishing @@ -284,14 +309,17 @@ publish was never established, so nothing here relies on it. GitHub Actions as the package's trusted publisher with the values in §4's table. It never publishes `latest` — the first tagged release does that. -The reservation is empty because the real artifact cannot be the record that -makes publishing possible. A package declaring `workspace:*` dependencies -resolves its siblings from the registry at build time, so its first artifact -cannot be built until those siblings are published — and they cannot be -published to a package that does not exist. An artifact with no dependencies at -all has no such cycle, which is what lets a package declaring siblings — -`@executablemd/acp` and `@executablemd/test-agent` among them — be bootstrapped -at all. +The reservation is empty because nothing about it needs to be otherwise. It +exists to make the name resolvable so `npm trust` can be configured against it, +and an empty artifact carries no dependency, no entry point and no claim about +the package's contents for `latest` to inherit by accident. + +It is not a workaround for a build that cannot run. A package declaring +`workspace:*` dependencies builds its siblings from the same checkout (§3), so +its first artifact can be built before any of them is published. That was not +true when this procedure was written, and the belief that it was is what made +the reservation look forced rather than chosen; #152 records the version of this +step that assumed it. `0.0.0-bootstrap.0` is never a release version, so `publish-one.yml`'s already-published guard never matches it: the first tagged release publishes its From 4d48f4fc31755711cf065ff29d7467be5c19522d Mon Sep 17 00:00:00 2001 From: Taras Mankovski Date: Wed, 23 Sep 2026 07:29:53 -0400 Subject: [PATCH 2/3] =?UTF-8?q?=F0=9F=A7=AA=20Prove=20closure=20finalizati?= =?UTF-8?q?on=20with=20packed=20local=20dependencies?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit N4 was returned as unprovable, and that was wrong about the mechanism rather than about the contract. npm symlinks a directory `file:` dependency by default, so a dependent reaches whatever the sibling's own build left in its `node_modules` and an early-finalized sibling costs nothing. npm's supported `install-links=true` packs and installs it as an ordinary dependency instead, which leaves the sibling's own manifest as the only statement of where its dependencies come from — and that is the discriminator. N4 now runs the real builder twice under an invocation-private npm configuration: a positive control, so packed local artifacts are known to work before a failure means anything, then the same closure with `b` rewritten to the registry range the moment its build completes. `a`'s install fails, no manifest is finalized, and `a` never completes. The registry it would have to reach is unreachable and retries are off, so the failure is hermetic and immediate rather than whatever npmjs.org happens to answer. The environment is restored however the case ends; every other build in the file is the ordinary one. Setting `install-links=false` makes the negative control pass, which is what makes the scoped configuration load-bearing rather than decoration. The three places that explained phase 2 by claiming an early rewrite puts a registry version in front of the dependent's install now say what is actually true: the cost of finalizing early depends on how npm installs a local directory and on the sibling's own build residue, and finalizing the closure at the end is what makes the build independent of both. --- scripts/build-npm.ts | 13 +++-- scripts/tests/build-npm.test.ts | 99 ++++++++++++++++++++++++++++++++- specs/release-process-spec.md | 11 +++- 3 files changed, 113 insertions(+), 10 deletions(-) diff --git a/scripts/build-npm.ts b/scripts/build-npm.ts index 447bc9689..7f3b2484f 100644 --- a/scripts/build-npm.ts +++ b/scripts/build-npm.ts @@ -23,11 +23,14 @@ * * **Phase 2 finalizes every manifest together**, once the last dnt call has * returned, replacing each internal `file:` range with the sibling's - * `^`. It has to be every manifest at once: in a chain A → B → C, - * rewriting B the moment its own dnt call returns puts C's registry version - * back in front of A's install, which is the race one level down. No install or - * typecheck runs after finalization begins, and a surviving local reference - * fails the build rather than reaching `npm publish`. + * `^`. It has to be every manifest at once: rewriting B when its own + * dnt call returns leaves it describing a dependency only the registry could + * supply, and whether A survives that then rests on how npm installs a local + * directory and on what B's own build left in its `node_modules` — no cost + * under the default symlink layout, a failed install under + * `install-links=true`. Finalizing the closure at the end depends on neither. + * No install or typecheck runs after finalization begins, and a surviving local + * reference fails the build rather than reaching `npm publish`. * * `DNT_SKIP_INSTALL=1` still skips the install and typecheck for a leaf package * that declares no `workspace:*` dependency, for exercising the tooling. It diff --git a/scripts/tests/build-npm.test.ts b/scripts/tests/build-npm.test.ts index fd34fe497..595bf8516 100644 --- a/scripts/tests/build-npm.test.ts +++ b/scripts/tests/build-npm.test.ts @@ -193,6 +193,51 @@ function localRange(root: URL, name: string): string { return `file:${fileURLToPath(new URL(`packages/${name}/npm`, root))}`; } +/** + * npm's supported packed-dependency layout, for the length of one case. + * + * By default npm symlinks a directory `file:` dependency, so a dependent can + * reach whatever the sibling's own build left in its `node_modules` — which + * hides what a finalized sibling would actually cost. `install-links=true` + * packs and installs it as an ordinary dependency instead, so the sibling's own + * manifest is the only thing that says where its dependencies come from. + * + * The cache and registry are invocation-private, and the registry is + * unreachable, so a range that has to be resolved fails here rather than + * depending on what npmjs.org happens to answer. The environment is restored + * however the case ends: every other build in this file is the ordinary one. + */ +function* usePackedLocalDependencies(): Operation { + const cache = yield* useTempDirectory("npm-cache-"); + const scopedEnvironment: Record = { + NPM_CONFIG_INSTALL_LINKS: "true", + NPM_CONFIG_CACHE: cache, + NPM_CONFIG_REGISTRY: "http://127.0.0.1:1/", + NPM_CONFIG_AUDIT: "false", + NPM_CONFIG_FUND: "false", + // The unreachable registry is the expected outcome here, not a flake worth + // waiting out; npm's default retries would spend minutes proving it. + NPM_CONFIG_FETCH_RETRIES: "0", + }; + + const restore = new Map( + Object.keys(scopedEnvironment).map((key) => [key, Deno.env.get(key)]), + ); + // Registered before anything is set, so a halt between the two still restores. + yield* ensure(() => { + for (const [key, value] of restore) { + if (value === undefined) { + Deno.env.delete(key); + } else { + Deno.env.set(key, value); + } + } + }); + for (const [key, value] of Object.entries(scopedEnvironment)) { + Deno.env.set(key, value); + } +} + /** * The builder's own two phases, observed on a closure npm has never published. * @@ -233,8 +278,10 @@ describe("build-npm local closure", () => { *observe(event) { events.push(event); if (event.type === "package-build-started" && event.package === scoped("a")) { - // b is built by now; it must still be naming c locally, or a's own - // install would be resolving c from the registry. + // b is built by now, and must still name c locally. Once b + // describes c by a registry range, whether a's install survives + // depends on npm's layout and on b's own build residue — which is + // exactly what this contract refuses to rest on. atStartOfA.push(yield* manifestOf(root, "b")); } }, @@ -268,6 +315,54 @@ describe("build-npm local closure", () => { } }); + /** + * N4. What an early-finalized `b` costs, once `b` is installed the way a + * published `b` would be. + * + * The positive control comes first: packed local artifacts have to work + * before a failure afterwards means anything. + */ + it("N4: a child finalized before its dependent builds fails that build", function* () { + yield* usePackedLocalDependencies(); + + const sound = yield* closureWorkspace(); + yield* buildNpmPackage({ repoRoot: sound, package: "packages/a", version: FIXTURE_VERSION }); + expect(yield* manifestOf(sound, "b")).toEqual({ [scoped("c")]: `^${FIXTURE_VERSION}` }); + + const broken = yield* closureWorkspace(); + const events: BuildEvent[] = []; + let caught: unknown; + + try { + yield* buildNpmPackage({ + repoRoot: broken, + package: "packages/a", + version: FIXTURE_VERSION, + *observe(event) { + events.push(event); + if (event.type === "package-build-completed" && event.package === scoped("b")) { + const manifest = new URL("packages/b/npm/package.json", broken); + const parsed = JSON.parse(yield* readTextFile(manifest)); + parsed.dependencies[scoped("c")] = `^${FIXTURE_VERSION}`; + yield* writeTextFile(manifest, `${JSON.stringify(parsed, null, 2)}\n`); + } + }, + }); + } catch (error) { + caught = error; + } + + expect(caught).toBeInstanceOf(Error); + // Before `a` completed and before anything was finalized: the closure never + // reaches a state where a half-rewritten set could be mistaken for output. + expect(events.map((event) => event.type)).not.toContain("closure-finalization-started"); + expect( + events + .filter((event) => event.type === "package-build-completed") + .map((event) => event.package), + ).toEqual([scoped("c"), scoped("b")]); + }); + it("N5: refuses an internal dependency no workspace member declares", function* () { const root = yield* closureWorkspace(); yield* member( diff --git a/specs/release-process-spec.md b/specs/release-process-spec.md index 2c1f3a15a..189f38fec 100644 --- a/specs/release-process-spec.md +++ b/specs/release-process-spec.md @@ -203,9 +203,14 @@ needs, and no build asks npm for a package from its own release. **Phase 2 finalizes every manifest together**, once the last dnt call has returned. Each internal `file:` range becomes the sibling's `^`, taken from the workspace manifests rather than from the version on the command line. -It is the whole closure at once and not each package as its own build finishes: -in a chain A → B → C, rewriting B early puts C's registry version back in front -of A's install. No install or type check runs after finalization begins. +It is the whole closure at once and not each package as its own build +finishes. A sibling rewritten early describes a dependency only the registry +could supply, and whether the rest of the closure survives that depends on how +npm installs a local directory and on what that sibling's own build left in its +`node_modules`: nothing goes wrong under the default symlink layout, while under +`install-links=true` the dependent's install fails. Finalizing at the end is +what makes the build independent of both. No install or type check runs after +finalization begins. **The result is publishable, or the build fails.** Before reporting success the builder inspects every generated manifest and refuses a dependency range From 6aff0f473cb8ce7f762a846481b6c8fabf2212b1 Mon Sep 17 00:00:00 2001 From: Taras Mankovski Date: Wed, 23 Sep 2026 23:14:58 -0400 Subject: [PATCH 3/3] =?UTF-8?q?=F0=9F=A7=AA=20Hold=20N4=20to=20the=20failu?= =?UTF-8?q?re=20it=20is=20about?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `expect(caught).toBeInstanceOf(Error)` accepted any throw, including one from the observer that applies the mutation — so a negative control that never reached `a` would have passed while proving nothing. The build-start events must now be exactly `c`, `b`, `a`, which is what says the mutation was applied and `a` entered dnt; an observer that threw leaves that list one short. The caught error must be dnt's `npm install failed with exit code 1`, which is what says it failed where a registry range has to be resolved rather than anywhere else in the run. The proof that only `c` and `b` complete and that finalization never starts is unchanged. Making the observer throw before its rewrite now fails the case. --- scripts/tests/build-npm.test.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/scripts/tests/build-npm.test.ts b/scripts/tests/build-npm.test.ts index 595bf8516..1aec03e3b 100644 --- a/scripts/tests/build-npm.test.ts +++ b/scripts/tests/build-npm.test.ts @@ -352,7 +352,18 @@ describe("build-npm local closure", () => { caught = error; } - expect(caught).toBeInstanceOf(Error); + // `a` reached dnt, which is what makes the failure below evidence about the + // mutation rather than about the observer that applied it: an observer that + // threw would leave this list one short. + expect( + events + .filter((event) => event.type === "package-build-started") + .map((event) => event.package), + ).toEqual([scoped("c"), scoped("b"), scoped("a")]); + + // And it failed where a registry range has to be resolved. + expect(caught).toMatchObject({ message: "npm install failed with exit code 1" }); + // Before `a` completed and before anything was finalized: the closure never // reaches a state where a half-rewritten set could be mistaken for output. expect(events.map((event) => event.type)).not.toContain("closure-finalization-started");