From 21296ed5127d32f5ac11ec0b5816a4439f4339a7 Mon Sep 17 00:00:00 2001 From: Taras Mankovski Date: Tue, 22 Sep 2026 04:14:32 -0400 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Hold=20every=20publ?= =?UTF-8?q?ishable=20manifest=20to=20one=20version=20before=20the=20tag?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v0.13.0 published five binaries and not one package. The release branch had been bumped before `packages/git` existed and merged after it, so git stayed at 0.12.1 while every sibling moved to 0.13.0 — and the two tag-time gates disagreed about that. `publish-packages.yml` reads all eleven manifests and refused. `release.yml` read `packages/cli/deno.json` alone, matched, and published the irreversible half. Both gates now read the same set, and a third reads it before the tag exists. `scripts/lib/version-lockstep.ts` walks the workspace and reports every manifest that declares a different version from its siblings, and every `bun.lock` workspace entry gone stale or missing; its suite runs it against this repository, which is where the answer has to be true. `release.yml`'s preflight walks `packages/*` instead of naming one manifest, so a package added later joins the gate by existing. `packages/git` reaches 0.13.0 here, with the `bun.lock` entry it never had, so the workspace the new check guards is one it passes. --- .github/workflows/release.yml | 39 +++- bun.lock | 17 ++ packages/git/deno.json | 2 +- packages/git/package.json | 2 +- scripts/lib/bun-lockfile.ts | 71 +++++++ scripts/lib/version-lockstep.ts | 131 +++++++++++++ .../tests/publish-workflow-membership.test.ts | 40 ++++ scripts/tests/version-lockstep.test.ts | 183 ++++++++++++++++++ specs/release-process-spec.md | 43 ++-- 9 files changed, 503 insertions(+), 25 deletions(-) create mode 100644 scripts/lib/bun-lockfile.ts create mode 100644 scripts/lib/version-lockstep.ts create mode 100644 scripts/tests/version-lockstep.test.ts diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index daf36b7b4..3a2b29f63 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -10,22 +10,43 @@ permissions: attestations: write jobs: - # Refuse a tag the manifests do not declare (spec §2) — the binary's - # --version comes from packages/cli/deno.json. On failure, the mistake is made - # visible on the release itself, not just in this log. + # Refuse a tag the manifests do not declare (spec §2). Every publishable + # manifest, not only the one the binary reads its --version from: v0.13.0 + # published binaries and no packages because this gate read + # packages/cli/deno.json alone and passed, while publish-packages.yml read all + # of them and refused. The weaker gate was the one standing in front of the + # irreversible half. On failure, the mistake is made visible on the release + # itself, not just in this log. preflight: runs-on: ubuntu-latest steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 - - name: Tag matches the manifest version + - name: Tag matches every publishable manifest run: | TAG="${{ github.ref_name }}" - declared="$(jq -r .version packages/cli/deno.json)" - if [ "v$declared" != "$TAG" ]; then - echo "::error::packages/cli/deno.json declares $declared but the tag is $TAG — bump the manifests first" - exit 1 - fi + mismatched=0 + # Walked rather than listed, so a new package joins this gate by + # existing — which is how packages/git came to be absent from it. + for manifest in packages/*/package.json; do + member="$(dirname "$manifest")" + name="$(jq -r '.name // ""' "$manifest")" + case "$name" in + @executablemd/*) ;; + *) continue ;; + esac + if [ "$(jq -r '.private // false' "$manifest")" = "true" ]; then + continue + fi + for declared_in in "$manifest" "$member/deno.json"; do + declared="$(jq -r '.version // ""' "$declared_in")" + if [ "v$declared" != "$TAG" ]; then + echo "::error::$declared_in declares $declared but the tag is $TAG — bump the manifests first" + mismatched=1 + fi + done + done + [ "$mismatched" -eq 0 ] - name: Flag the release when the tag cannot build if: failure() diff --git a/bun.lock b/bun.lock index c23e45f2a..57f454416 100644 --- a/bun.lock +++ b/bun.lock @@ -36,6 +36,7 @@ "@executablemd/code-review-agent": "workspace:*", "@executablemd/core": "workspace:*", "@executablemd/durable-streams": "workspace:*", + "@executablemd/git": "workspace:*", "@executablemd/runtime": "workspace:*", "@executablemd/test-agent": "workspace:*", "@executablemd/test-support": "workspace:*", @@ -70,6 +71,7 @@ "@executablemd/acp": "workspace:*", "@executablemd/core": "workspace:*", "@executablemd/durable-streams": "workspace:*", + "@executablemd/git": "workspace:*", "@executablemd/runtime": "workspace:*", "@executablemd/test-agent": "workspace:*", "@executablemd/testing": "workspace:*", @@ -125,6 +127,19 @@ "effection": "4.1.0", }, }, + "packages/git": { + "name": "@executablemd/git", + "version": "0.13.0", + "dependencies": { + "@effectionx/context-api": "0.6.0", + "@effectionx/fs": "0.3.0", + "@executablemd/core": "workspace:*", + "@executablemd/durable-streams": "workspace:*", + "@executablemd/runtime": "workspace:*", + "@executablemd/workflow": "workspace:*", + "effection": "4.1.0", + }, + }, "packages/runtime": { "name": "@executablemd/runtime", "version": "0.13.0", @@ -325,6 +340,8 @@ "@executablemd/durable-streams": ["@executablemd/durable-streams@workspace:packages/durable-streams"], + "@executablemd/git": ["@executablemd/git@workspace:packages/git"], + "@executablemd/runtime": ["@executablemd/runtime@workspace:packages/runtime"], "@executablemd/test-agent": ["@executablemd/test-agent@workspace:packages/test-agent"], diff --git a/packages/git/deno.json b/packages/git/deno.json index d25e314ff..c88a0ca30 100644 --- a/packages/git/deno.json +++ b/packages/git/deno.json @@ -1,6 +1,6 @@ { "name": "@executablemd/git", - "version": "0.12.1", + "version": "0.13.0", "license": "MIT", "exports": { ".": "./mod.ts", diff --git a/packages/git/package.json b/packages/git/package.json index 368b6d4d5..bbbaa0c84 100644 --- a/packages/git/package.json +++ b/packages/git/package.json @@ -1,6 +1,6 @@ { "name": "@executablemd/git", - "version": "0.12.1", + "version": "0.13.0", "description": "The Git Plugin for executable.md: repositories, worktrees, Git operations, pull requests and issues, retained in a workflow run's Workspace.", "type": "module", "exports": { diff --git a/scripts/lib/bun-lockfile.ts b/scripts/lib/bun-lockfile.ts new file mode 100644 index 000000000..59ab04d93 --- /dev/null +++ b/scripts/lib/bun-lockfile.ts @@ -0,0 +1,71 @@ +import { z } from "zod"; + +const LockfileSchema = z.object({ + workspaces: z.record( + z.string(), + z.object({ + name: z.string().optional(), + version: z.string().optional(), + }), + ), +}); + +/** What `bun.lock` records for one workspace member. */ +export interface LockedWorkspace { + name?: string; + version?: string; +} + +/** + * Bun writes its text lockfile as JSON with trailing commas, and nothing else + * JSON refuses — no comments, no unquoted keys, no single-quoted strings. So + * the file becomes parseable by dropping every comma whose next non-whitespace + * character closes its container. A comma inside a string value is not one of + * those, which is why this walks the text instead of matching it. + */ +function withoutTrailingCommas(text: string): string { + let out = ""; + let inString = false; + let escaped = false; + + for (let index = 0; index < text.length; index += 1) { + const char = text[index]; + + if (inString) { + out += char; + if (escaped) { + escaped = false; + } else if (char === "\\") { + escaped = true; + } else if (char === '"') { + inString = false; + } + continue; + } + + if (char === '"') { + inString = true; + out += char; + continue; + } + + if (char === ",") { + let next = index + 1; + while (next < text.length && /\s/.test(text[next])) { + next += 1; + } + if (text[next] === "}" || text[next] === "]") { + continue; + } + } + + out += char; + } + + return out; +} + +/** Every workspace member `text` records, keyed by its root-relative directory. */ +export function parseBunLockfile(text: string): Record { + return LockfileSchema.parse(JSON.parse(withoutTrailingCommas(text))).workspaces; +} diff --git a/scripts/lib/version-lockstep.ts b/scripts/lib/version-lockstep.ts new file mode 100644 index 000000000..f3e7ad3f5 --- /dev/null +++ b/scripts/lib/version-lockstep.ts @@ -0,0 +1,131 @@ +import { readTextFile } from "@effectionx/fs"; +import type { Operation } from "effection"; +import { z } from "zod"; + +import { parseBunLockfile } from "./bun-lockfile.ts"; +import { listWorkspacePaths } from "./workspace.ts"; + +const SCOPE = "@executablemd/"; + +const RootSchema = z.object({ workspace: z.array(z.string()) }); +const IdentitySchema = z.object({ name: z.string(), private: z.boolean().optional() }); +const VersionSchema = z.object({ version: z.string() }); + +/** One manifest of a publishable member, and the version it declares. */ +export interface ManifestVersion { + /** Root-relative, e.g. `packages/git/deno.json`. */ + path: string; + /** `undefined` when the file is absent or declares no version. */ + version: string | undefined; +} + +/** A workspace member that a tagged release publishes. */ +export interface PublishableMember { + /** Root-relative directory, e.g. `packages/git`. */ + dir: string; + name: string; + manifests: ManifestVersion[]; +} + +function* declaredVersion(url: URL): Operation { + let text: string; + try { + text = yield* readTextFile(url); + } catch { + return undefined; + } + const parsed = VersionSchema.safeParse(JSON.parse(text)); + return parsed.success ? parsed.data.version : undefined; +} + +/** + * Every `@executablemd` workspace member a tagged release publishes, walked + * from the root `workspace` globs rather than a list, so this and + * `bumpManifests` cannot come to disagree about who is in the release. + * + * Identity and exclusion both come from `package.json`: a private member omits + * `deno.json`'s `name` and `exports` so `deno publish` finds no entry, which + * leaves `package.json` as the only manifest every member fills in. + */ +export function* publishableMembers(repoRoot: URL): Operation { + const root = RootSchema.parse(JSON.parse(yield* readTextFile(new URL("deno.json", repoRoot)))); + const found: PublishableMember[] = []; + + for (const dir of yield* listWorkspacePaths(root.workspace, repoRoot)) { + let identity: string; + try { + identity = yield* readTextFile(new URL(`${dir}/package.json`, repoRoot)); + } catch { + continue; + } + const parsed = IdentitySchema.safeParse(JSON.parse(identity)); + if (!parsed.success || !parsed.data.name.startsWith(SCOPE) || parsed.data.private === true) { + continue; + } + + const manifests: ManifestVersion[] = []; + for (const manifest of ["deno.json", "package.json"]) { + manifests.push({ + path: `${dir}/${manifest}`, + version: yield* declaredVersion(new URL(`${dir}/${manifest}`, repoRoot)), + }); + } + found.push({ dir, name: parsed.data.name, manifests }); + } + + return found; +} + +/** + * Everything that breaks version lockstep in the workspace at `repoRoot`, as + * messages naming the manifest at fault. An empty list is the whole claim: the + * release publishes one version, and every manifest and the lockfile declare + * it. + * + * Both tag-time gates make this assertion after the tag has been pushed, which + * is after the binaries have published. Reading it here moves the answer to + * the moment the drift is introduced. + */ +export function* versionLockstepFindings(repoRoot: URL): Operation { + const members = yield* publishableMembers(repoRoot); + const findings: string[] = []; + const declared = new Map(); + + for (const member of members) { + for (const manifest of member.manifests) { + if (manifest.version === undefined) { + findings.push(`${manifest.path} declares no version`); + continue; + } + declared.set(manifest.version, [...(declared.get(manifest.version) ?? []), manifest.path]); + } + } + + if (declared.size > 1) { + // Insertion order, which is the workspace walk's own sorted order, so the + // message reads the same way twice. + const groups = [...declared].map(([version, paths]) => `${version} (${paths.join(", ")})`); + findings.push(`the workspace declares more than one version: ${groups.join("; ")}`); + } + + const locked = parseBunLockfile(yield* readTextFile(new URL("bun.lock", repoRoot))); + // The lockfile is compared against the version only once the manifests agree + // on one. Against a workspace that does not, every entry would be reported + // for a mismatch the finding above already names. + const [expected] = declared.size === 1 ? [...declared.keys()] : [undefined]; + + for (const member of members) { + const entry = locked[member.dir]; + if (entry === undefined) { + findings.push(`bun.lock has no workspace entry for ${member.dir}`); + continue; + } + if (expected !== undefined && entry.version !== expected) { + findings.push( + `bun.lock records ${member.dir} at ${entry.version ?? "no version"}, not ${expected}`, + ); + } + } + + return findings; +} diff --git a/scripts/tests/publish-workflow-membership.test.ts b/scripts/tests/publish-workflow-membership.test.ts index 614262683..a90fe47fa 100644 --- a/scripts/tests/publish-workflow-membership.test.ts +++ b/scripts/tests/publish-workflow-membership.test.ts @@ -236,3 +236,43 @@ describe("publish-packages.yml membership", () => { } }); }); + +/** + * 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 + * disagree: `release.yml` read `packages/cli/deno.json` alone, passed, and + * published binaries that no package release would ever join, while + * `publish-packages.yml` read all eleven and refused on `packages/git`. + */ +describe("tag-time version gates", () => { + it("reads every publishable manifest before publishing packages", function* () { + const generated = yield* workflow(); + const publishable = (yield* members()).filter((member) => !member.isPrivate); + + // Non-vacuous: a sweep over no members would find nothing missing. + expect(publishable.length).toBeGreaterThan(0); + + for (const member of publishable) { + expect(generated).toContain(`${member.dir}/deno.json`); + } + }); + + /** + * The binary gate reaches the same set by walking the workspace, so a package + * added after it was written joins it by existing. Naming one member is the + * shape that failed, and it is what this refuses. + */ + it("reaches the same set by walking the workspace before publishing binaries", function* () { + const commands = (yield* readTextFile(RELEASE_WORKFLOW)) + .split("\n") + .filter((line) => !line.trim().startsWith("#")) + .join("\n"); + const named = (yield* members()) + .filter((member) => !member.isPrivate) + .filter((member) => commands.includes(`${member.dir}/deno.json`)) + .map((member) => member.dir); + + expect(commands).toContain("for manifest in packages/*/package.json"); + expect(named).toEqual([]); + }); +}); diff --git a/scripts/tests/version-lockstep.test.ts b/scripts/tests/version-lockstep.test.ts new file mode 100644 index 000000000..c4f911ea2 --- /dev/null +++ b/scripts/tests/version-lockstep.test.ts @@ -0,0 +1,183 @@ +import { describe, it } from "@executablemd/test-support/bdd"; +import { expect } from "@executablemd/test-support/expect"; +import type { Operation } from "effection"; +import { ensureDir, writeTextFile } from "@effectionx/fs"; +import { useTempDirectory } from "@executablemd/test-support/temp"; +import { pathToFileURL } from "node:url"; + +import { parseBunLockfile } from "../lib/bun-lockfile.ts"; +import { publishableMembers, versionLockstepFindings } from "../lib/version-lockstep.ts"; + +const repoRoot = new URL("../../", import.meta.url); + +interface MemberSpec { + dir: string; + name: string; + /** What both manifests declare, unless `deno` overrides `deno.json`. */ + version?: string; + deno?: string; + private?: boolean; + /** What `bun.lock` records; omitted writes no entry for the member. */ + locked?: string; +} + +/** A workspace of `members`, with the `bun.lock` their `locked` versions describe. */ +function* workspace(members: MemberSpec[]): Operation { + const base = yield* useTempDirectory("version-lockstep-"); + const root = pathToFileURL(`${base}/`); + + yield* writeTextFile( + new URL("deno.json", root), + `${JSON.stringify({ workspace: ["packages/*"] }, null, 2)}\n`, + ); + + const entries = [' "": {\n "name": "root",\n },']; + for (const member of members) { + yield* ensureDir(new URL(`packages/${member.dir}/`, root)); + yield* writeTextFile( + new URL(`packages/${member.dir}/package.json`, root), + `${JSON.stringify( + { name: member.name, version: member.version, private: member.private }, + null, + 2, + )}\n`, + ); + yield* writeTextFile( + new URL(`packages/${member.dir}/deno.json`, root), + `${JSON.stringify( + { name: member.name, version: member.deno ?? member.version, exports: "./mod.ts" }, + null, + 2, + )}\n`, + ); + if (member.locked !== undefined) { + entries.push( + ` "packages/${member.dir}": {\n "name": "${member.name}",\n "version": "${member.locked}",\n },`, + ); + } + } + + // Written with the trailing commas Bun writes, so every case reads the + // lockfile through the same parse the repository's own does. + yield* writeTextFile( + new URL("bun.lock", root), + `{\n "lockfileVersion": 1,\n "workspaces": {\n${entries.join("\n")}\n },\n}\n`, + ); + + return root; +} + +/** One publishable member, in lockstep, as the baseline every case varies. */ +const SCOPED: MemberSpec = { + dir: "scoped", + name: "@executablemd/scoped", + version: "1.0.0", + locked: "1.0.0", +}; + +describe("versionLockstepFindings", () => { + it("reports nothing when every manifest and the lockfile agree", function* () { + const root = yield* workspace([SCOPED]); + + expect(yield* versionLockstepFindings(root)).toEqual([]); + // Non-vacuous: a walk that found no members would report nothing too. + expect((yield* publishableMembers(root)).map((member) => member.dir)).toEqual([ + "packages/scoped", + ]); + }); + + it("reports a member the bump left behind", function* () { + const root = yield* workspace([ + SCOPED, + { dir: "behind", name: "@executablemd/behind", version: "0.9.0", locked: "0.9.0" }, + ]); + + expect(yield* versionLockstepFindings(root)).toEqual([ + "the workspace declares more than one version: 0.9.0 (packages/behind/deno.json, " + + "packages/behind/package.json); 1.0.0 (packages/scoped/deno.json, " + + "packages/scoped/package.json)", + ]); + }); + + it("reports one member whose two manifests disagree", function* () { + const root = yield* workspace([{ ...SCOPED, deno: "0.9.0" }]); + + expect(yield* versionLockstepFindings(root)).toEqual([ + "the workspace declares more than one version: 0.9.0 (packages/scoped/deno.json); " + + "1.0.0 (packages/scoped/package.json)", + ]); + }); + + it("reports a manifest that declares no version", function* () { + const root = yield* workspace([{ dir: "scoped", name: "@executablemd/scoped" }]); + + expect(yield* versionLockstepFindings(root)).toEqual([ + "packages/scoped/deno.json declares no version", + "packages/scoped/package.json declares no version", + "bun.lock has no workspace entry for packages/scoped", + ]); + }); + + it("holds no private member to the release version", function* () { + const root = yield* workspace([ + SCOPED, + { dir: "support", name: "@executablemd/support", version: "0.0.0", private: true }, + ]); + + expect(yield* versionLockstepFindings(root)).toEqual([]); + }); + + it("holds no member outside the @executablemd scope to it either", function* () { + const root = yield* workspace([ + SCOPED, + { dir: "outside", name: "outside-tool", version: "7.7.7" }, + ]); + + expect(yield* versionLockstepFindings(root)).toEqual([]); + }); + + it("reports a member the lockfile has never heard of", function* () { + const root = yield* workspace([{ ...SCOPED, locked: undefined }]); + + expect(yield* versionLockstepFindings(root)).toEqual([ + "bun.lock has no workspace entry for packages/scoped", + ]); + }); + + it("reports a lockfile entry left at the previous release", function* () { + const root = yield* workspace([{ ...SCOPED, locked: "0.9.0" }]); + + expect(yield* versionLockstepFindings(root)).toEqual([ + "bun.lock records packages/scoped at 0.9.0, not 1.0.0", + ]); + }); + + /** + * The gate against the tree it guards. `packages/git` reached `main` at + * `0.12.1` while every sibling moved to `0.13.0`, and `v0.13.0` published + * binaries and no packages because only the package gate reads every + * manifest — this is that state, asserted where a pull request can see it. + */ + it("holds this workspace in lockstep", function* () { + expect(yield* versionLockstepFindings(repoRoot)).toEqual([]); + expect((yield* publishableMembers(repoRoot)).length).toBeGreaterThan(1); + }); +}); + +describe("parseBunLockfile", () => { + it("reads the workspace members Bun's trailing commas would hide from JSON", function* () { + const parsed = parseBunLockfile( + `{\n "workspaces": {\n "packages/one": {\n "version": "1.2.3",\n },\n },\n}\n`, + ); + + expect(parsed).toEqual({ "packages/one": { version: "1.2.3" } }); + }); + + it("keeps a comma that closes nothing because it is inside a string", function* () { + const parsed = parseBunLockfile( + `{\n "workspaces": {\n "packages/one": {\n "name": "weird, }",\n "version": "1.2.3",\n },\n },\n}\n`, + ); + + expect(parsed["packages/one"]).toEqual({ name: "weird, }", version: "1.2.3" }); + }); +}); diff --git a/specs/release-process-spec.md b/specs/release-process-spec.md index 49a90be12..af8601b4c 100644 --- a/specs/release-process-spec.md +++ b/specs/release-process-spec.md @@ -37,7 +37,7 @@ sequenceDiagram M->>GH: publish the draft → tag vX.Y.Z from main GH->>R: push: tags v* GH->>PP: push: tags v* - R->>R: validate tag matches packages/cli/deno.json + R->>R: validate tag matches every manifest R->>GH: compile and attest xmd per target,
attach binaries + checksums to the release PP->>PP: validate tag matches every manifest,
wait for release.yml to succeed PP->>PO: one call per package,
needs-ordered (deps first) @@ -48,18 +48,30 @@ sequenceDiagram ## 2. Version lockstep -Every publishable package (`packages/core`, `packages/cli`, -`packages/durable-streams`, `packages/runtime`, `packages/testing`, -`packages/code-review-agent`, `packages/test-agent`, `packages/acp`, -`packages/web`, `packages/workflow`) declares the same version in its `deno.json` and -`package.json`. A member marked `"private": true` is outside the lockstep -because it never publishes — `packages/test-support` is the one, and it stays -at `0.0.0`. `packages/cli/src/cli.ts` +Every publishable package declares the same version in its `deno.json` and +`package.json`. Membership is the workspace, not a list: a `packages/*` member +whose `package.json` `name` is under `@executablemd` is publishable, so a new +package joins the lockstep by existing. A member marked `"private": true` is +outside it because it never publishes — `packages/test-support` is the one, and +it stays at `0.0.0`. `packages/cli/src/cli.ts` imports `packages/cli/deno.json` and reads `version` from it, so the compiled binary reports the manifest version — the manifests -are the single source. The npm version derives from the tag, and both -workflows refuse a tag the manifests do not declare, so the two cannot -diverge. +are the single source. + +Three checks hold that, and they read the same set. `scripts/lib/version-lockstep.ts` +answers it for a pull request, where drift is introduced and where it is still +cheap: it walks the workspace and reports every manifest that declares a +different version from its siblings, and every `bun.lock` workspace entry that +has gone stale or missing. The other two are the tag-time gates — `release.yml` +before the binaries, `publish-packages.yml` before the packages — and each +refuses a tag every publishable manifest does not declare, so the tag and the +manifests cannot diverge. + +A gate that reads fewer manifests than the other is the failure this +arrangement exists to prevent. `v0.13.0` published its binaries and none of its +packages because `release.yml` read `packages/cli/deno.json` alone: the release +branch had been bumped before `packages/git` existed, cli matched the tag, and +only the package gate noticed that git did not. To cut a release: run `deno task bump ` (stamps every manifest), restamp the workspace versions in `bun.lock`, merge to `main`, then publish the @@ -68,8 +80,10 @@ draft release — its tag follows the manifests (§3). `bun.lock` records a `version` for every workspace member, and the bump task does not touch it — `bun install` will not restamp those entries either, since they already satisfy the lockfile. Left alone they keep the previous release's -number. Only the members whose `name` is an `@executablemd` package change; an -unrelated dependency that happens to share the old version number must not. +number, which the lockstep check reports, so restamping them is part of cutting +the release rather than a tidying step afterwards. Only the members whose `name` +is an `@executablemd` package change; an unrelated dependency that happens to +share the old version number must not. The bump touches nothing else. PR Review and Repo Analysis prepare and build the checked-out revision with `deno task setup` and `deno task build`, then run @@ -87,7 +101,8 @@ documents at the revision it checks. the manifests need bumping; once bumped, the banner clears and the draft's tag and title default to the guard-passing `v`. - **`release.yml`** (`push: tags v*`): a preflight job validates the tag - against `packages/cli/deno.json`; on mismatch it flags the just-published release on + against every publishable manifest, walked from `packages/*` so a new package + joins by existing; on mismatch it flags the just-published release on the Releases page — caution note in the notes, a failed title, and the prerelease marker — so a forgotten bump is visible where the release was made, then refuses to build. On a valid tag it compiles From 6cc409feea11b22916d3555427e78e4aefe99055 Mon Sep 17 00:00:00 2001 From: Taras Mankovski Date: Tue, 22 Sep 2026 04:56:48 -0400 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Select=20release=20?= =?UTF-8?q?membership=20from=20deno.json,=20in=20every=20gate=20at=20once?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found the correction incomplete: the new check and the widened preflight read membership from `package.json`'s name, while the publish generator, the npm builder and `bumpManifests` read it from `deno.json`'s. A member the two manifests name differently therefore publishes packages while both gates ignore it — the same partial release the PR set out to prevent, reached another way. `scripts/lib/publishable-members.ts` now holds that rule once: identity from `deno.json`'s name, exclusion from `package.json`'s `private`, both manifests required. The lockstep check, `release.yml`'s preflight and the two membership assertions in the workflow suite all read it. The repository's own manifests agree about every name, so nothing in the tree can tell the two rules apart. `publishable-membership-agreement.test.ts` builds workspaces where they disagree on purpose and runs all three selectors over them — `publishableMembers`, the preflight's own shell lifted out of `release.yml`, and the real generator document over the fixture, since an eval block's selection rule cannot be imported (#237). Reverting either selector to `package.json` makes it fail. --- .github/workflows/release.yml | 15 +- scripts/lib/publishable-members.ts | 72 ++++++ scripts/lib/version-lockstep.ts | 79 ++----- scripts/runtime-test-exclusions.ts | 6 + .../tests/publish-workflow-membership.test.ts | 14 +- .../publishable-membership-agreement.test.ts | 208 ++++++++++++++++++ scripts/tests/version-lockstep.test.ts | 138 +++++++++--- specs/release-process-spec.md | 64 +++--- 8 files changed, 461 insertions(+), 135 deletions(-) create mode 100644 scripts/lib/publishable-members.ts create mode 100644 scripts/tests/publishable-membership-agreement.test.ts diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 3a2b29f63..a42d445b6 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -22,23 +22,28 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + # Membership is deno.json's name and package.json's private, which is the + # pair scripts/gen-publish-workflow.md selects the publish jobs on. Read + # from package.json's name instead and this gate admits a different set + # than the one that publishes, which is the same partial release by + # another route. Walked rather than listed, so a new package joins by + # existing — which is how packages/git came to be absent from it. - name: Tag matches every publishable manifest run: | TAG="${{ github.ref_name }}" mismatched=0 - # Walked rather than listed, so a new package joins this gate by - # existing — which is how packages/git came to be absent from it. - for manifest in packages/*/package.json; do + for manifest in packages/*/deno.json; do member="$(dirname "$manifest")" + [ -f "$member/package.json" ] || continue name="$(jq -r '.name // ""' "$manifest")" case "$name" in @executablemd/*) ;; *) continue ;; esac - if [ "$(jq -r '.private // false' "$manifest")" = "true" ]; then + if [ "$(jq -r '.private // false' "$member/package.json")" = "true" ]; then continue fi - for declared_in in "$manifest" "$member/deno.json"; do + for declared_in in "$manifest" "$member/package.json"; do declared="$(jq -r '.version // ""' "$declared_in")" if [ "v$declared" != "$TAG" ]; then echo "::error::$declared_in declares $declared but the tag is $TAG — bump the manifests first" diff --git a/scripts/lib/publishable-members.ts b/scripts/lib/publishable-members.ts new file mode 100644 index 000000000..3ca9eb2ad --- /dev/null +++ b/scripts/lib/publishable-members.ts @@ -0,0 +1,72 @@ +import { readTextFile } from "@effectionx/fs"; +import type { Operation } from "effection"; +import { z } from "zod"; + +import { listWorkspacePaths } from "./workspace.ts"; + +export const SCOPE = "@executablemd/"; + +const RootSchema = z.object({ workspace: z.array(z.string()) }); +const IdentitySchema = z.object({ name: z.string() }); +const PublicationSchema = z.object({ private: z.boolean().optional() }); + +/** A workspace member a tagged release publishes. */ +export interface PublishableMember { + /** Root-relative directory, e.g. `packages/git`. */ + dir: string; + /** The name it publishes under, from `deno.json`. */ + name: string; +} + +/** + * The name a member publishes under, or `undefined` when it publishes nothing. + * + * Identity is `deno.json`'s `name` and the exclusion is `package.json`'s + * `private`, which is the pair `scripts/gen-publish-workflow.md` selects the + * publish jobs on and `bumpManifests` stamps on. Nothing requires the two + * manifests to agree about a member's name, so a gate that read + * `package.json`'s would admit a different set than the one that actually + * publishes — and a set the binary gate and the package gate disagree about is + * how a tag comes to publish one half of a release. + * + * A member missing either manifest publishes nothing: no `deno.json` is no JSR + * entry, and no `package.json` is no npm package. + */ +export function publishedName(denoJson: unknown, packageJson: unknown): string | undefined { + const identity = IdentitySchema.safeParse(denoJson); + if (!identity.success || !identity.data.name.startsWith(SCOPE)) { + return undefined; + } + const publication = PublicationSchema.safeParse(packageJson); + if (publication.success && publication.data.private === true) { + return undefined; + } + return identity.data.name; +} + +/** + * Every member a tagged release publishes, walked from the root `workspace` + * globs rather than a list, so a new package joins by existing. + */ +export function* publishableMembers(repoRoot: URL): Operation { + const root = RootSchema.parse(JSON.parse(yield* readTextFile(new URL("deno.json", repoRoot)))); + const found: PublishableMember[] = []; + + for (const dir of yield* listWorkspacePaths(root.workspace, repoRoot)) { + let denoJson: unknown; + let packageJson: unknown; + try { + denoJson = JSON.parse(yield* readTextFile(new URL(`${dir}/deno.json`, repoRoot))); + packageJson = JSON.parse(yield* readTextFile(new URL(`${dir}/package.json`, repoRoot))); + } catch { + continue; + } + + const name = publishedName(denoJson, packageJson); + if (name !== undefined) { + found.push({ dir, name }); + } + } + + return found; +} diff --git a/scripts/lib/version-lockstep.ts b/scripts/lib/version-lockstep.ts index f3e7ad3f5..2bba83278 100644 --- a/scripts/lib/version-lockstep.ts +++ b/scripts/lib/version-lockstep.ts @@ -3,29 +3,12 @@ import type { Operation } from "effection"; import { z } from "zod"; import { parseBunLockfile } from "./bun-lockfile.ts"; -import { listWorkspacePaths } from "./workspace.ts"; +import { publishableMembers } from "./publishable-members.ts"; -const SCOPE = "@executablemd/"; - -const RootSchema = z.object({ workspace: z.array(z.string()) }); -const IdentitySchema = z.object({ name: z.string(), private: z.boolean().optional() }); const VersionSchema = z.object({ version: z.string() }); -/** One manifest of a publishable member, and the version it declares. */ -export interface ManifestVersion { - /** Root-relative, e.g. `packages/git/deno.json`. */ - path: string; - /** `undefined` when the file is absent or declares no version. */ - version: string | undefined; -} - -/** A workspace member that a tagged release publishes. */ -export interface PublishableMember { - /** Root-relative directory, e.g. `packages/git`. */ - dir: string; - name: string; - manifests: ManifestVersion[]; -} +/** The two manifests a publishable member declares its version in. */ +const MANIFESTS = ["deno.json", "package.json"]; function* declaredVersion(url: URL): Operation { let text: string; @@ -38,53 +21,17 @@ function* declaredVersion(url: URL): Operation { return parsed.success ? parsed.data.version : undefined; } -/** - * Every `@executablemd` workspace member a tagged release publishes, walked - * from the root `workspace` globs rather than a list, so this and - * `bumpManifests` cannot come to disagree about who is in the release. - * - * Identity and exclusion both come from `package.json`: a private member omits - * `deno.json`'s `name` and `exports` so `deno publish` finds no entry, which - * leaves `package.json` as the only manifest every member fills in. - */ -export function* publishableMembers(repoRoot: URL): Operation { - const root = RootSchema.parse(JSON.parse(yield* readTextFile(new URL("deno.json", repoRoot)))); - const found: PublishableMember[] = []; - - for (const dir of yield* listWorkspacePaths(root.workspace, repoRoot)) { - let identity: string; - try { - identity = yield* readTextFile(new URL(`${dir}/package.json`, repoRoot)); - } catch { - continue; - } - const parsed = IdentitySchema.safeParse(JSON.parse(identity)); - if (!parsed.success || !parsed.data.name.startsWith(SCOPE) || parsed.data.private === true) { - continue; - } - - const manifests: ManifestVersion[] = []; - for (const manifest of ["deno.json", "package.json"]) { - manifests.push({ - path: `${dir}/${manifest}`, - version: yield* declaredVersion(new URL(`${dir}/${manifest}`, repoRoot)), - }); - } - found.push({ dir, name: parsed.data.name, manifests }); - } - - return found; -} - /** * Everything that breaks version lockstep in the workspace at `repoRoot`, as * messages naming the manifest at fault. An empty list is the whole claim: the * release publishes one version, and every manifest and the lockfile declare * it. * - * Both tag-time gates make this assertion after the tag has been pushed, which - * is after the binaries have published. Reading it here moves the answer to - * the moment the drift is introduced. + * Membership comes from `publishableMembers`, so this reads the same set the + * publish workflow generates jobs for. Both tag-time gates make the same + * assertion, but only after the tag has been pushed — which is after the + * binaries have published. Reading it here moves the answer to the moment the + * drift is introduced. */ export function* versionLockstepFindings(repoRoot: URL): Operation { const members = yield* publishableMembers(repoRoot); @@ -92,12 +39,14 @@ export function* versionLockstepFindings(repoRoot: URL): Operation { const declared = new Map(); for (const member of members) { - for (const manifest of member.manifests) { - if (manifest.version === undefined) { - findings.push(`${manifest.path} declares no version`); + for (const manifest of MANIFESTS) { + const path = `${member.dir}/${manifest}`; + const version = yield* declaredVersion(new URL(path, repoRoot)); + if (version === undefined) { + findings.push(`${path} declares no version`); continue; } - declared.set(manifest.version, [...(declared.get(manifest.version) ?? []), manifest.path]); + declared.set(version, [...(declared.get(version) ?? []), path]); } } diff --git a/scripts/runtime-test-exclusions.ts b/scripts/runtime-test-exclusions.ts index 9607be6d0..13057e7aa 100644 --- a/scripts/runtime-test-exclusions.ts +++ b/scripts/runtime-test-exclusions.ts @@ -83,6 +83,12 @@ const DENO_ONLY_TOOLING: RuntimeExclusion[] = [ "subject is scripts/build-web-client.ts, which runs `deno bundle` and calls Deno.execPath()/makeTempFile — Deno-only", issue: DERIVED_SCOPE, }, + { + path: "scripts/tests/publishable-membership-agreement.test.ts", + reason: + "runs the publish-workflow generator over a fixture workspace by spawning the CLI through Deno.execPath(), which is the only way to exercise an eval block's selection rule (#237); the rule itself is asserted portably by publishedName's cases in version-lockstep.test.ts, and what this adds — that the generator and the release preflight reach that same rule — is Deno's own release tooling", + issue: DERIVED_SCOPE, + }, { path: "scripts/tests/prepared-state.test.ts", reason: diff --git a/scripts/tests/publish-workflow-membership.test.ts b/scripts/tests/publish-workflow-membership.test.ts index a90fe47fa..1008a3b43 100644 --- a/scripts/tests/publish-workflow-membership.test.ts +++ b/scripts/tests/publish-workflow-membership.test.ts @@ -4,6 +4,7 @@ import type { Operation } from "effection"; import { readdir, readTextFile } from "@effectionx/fs"; import { compileArguments, COMPILE_ENTRYPOINT } from "../lib/compile.ts"; +import { publishableMembers } from "../lib/publishable-members.ts"; import { RELEASE_TARGET } from "../lib/release-targets.ts"; import { listWorkspacePaths } from "../lib/workspace.ts"; @@ -200,13 +201,13 @@ describe("release.yml binary compilation", () => { describe("publish-packages.yml membership", () => { it("publishes every non-private member to npm", function* () { - const all = yield* members(); + const publishable = yield* publishableMembers(repoRoot); const generated = yield* workflow(); // Non-vacuous: the workspace always has publishable members. - expect(all.filter((member) => !member.isPrivate).length).toBeGreaterThan(0); + expect(publishable.length).toBeGreaterThan(0); - for (const member of all.filter((member) => !member.isPrivate)) { + for (const member of publishable) { expect(generated).toContain(`package: ${member.dir}`); } }); @@ -247,7 +248,7 @@ describe("publish-packages.yml membership", () => { describe("tag-time version gates", () => { it("reads every publishable manifest before publishing packages", function* () { const generated = yield* workflow(); - const publishable = (yield* members()).filter((member) => !member.isPrivate); + const publishable = yield* publishableMembers(repoRoot); // Non-vacuous: a sweep over no members would find nothing missing. expect(publishable.length).toBeGreaterThan(0); @@ -267,12 +268,11 @@ describe("tag-time version gates", () => { .split("\n") .filter((line) => !line.trim().startsWith("#")) .join("\n"); - const named = (yield* members()) - .filter((member) => !member.isPrivate) + const named = (yield* publishableMembers(repoRoot)) .filter((member) => commands.includes(`${member.dir}/deno.json`)) .map((member) => member.dir); - expect(commands).toContain("for manifest in packages/*/package.json"); + expect(commands).toContain("for manifest in packages/*/deno.json"); expect(named).toEqual([]); }); }); diff --git a/scripts/tests/publishable-membership-agreement.test.ts b/scripts/tests/publishable-membership-agreement.test.ts new file mode 100644 index 000000000..be4cc4913 --- /dev/null +++ b/scripts/tests/publishable-membership-agreement.test.ts @@ -0,0 +1,208 @@ +/** + * The three selectors that decide what a tag publishes, run over one workspace + * they must all read the same way. + * + * `scripts/gen-publish-workflow.md` selects publish jobs on `deno.json`'s name + * and `package.json`'s `private`. A gate that selected on `package.json`'s name + * instead would admit a different set, and the half that disagreed would + * publish alone — which is the partial release this whole arrangement exists to + * prevent. The repository's own manifests agree about every name, so nothing + * there can tell the two rules apart; these fixtures make them disagree on + * purpose. + * + * Both the preflight and the generator are executed as the release runs them, + * not reimplemented: the preflight's shell is lifted out of `release.yml`, and + * the generator is the real document over a fixture workspace, because an eval + * block's selection rule cannot be imported (#237). + */ + +import { describe, it } from "@executablemd/test-support/bdd"; +import { expect } from "@executablemd/test-support/expect"; +import type { Operation } from "effection"; +import { ensureDir, readTextFile, writeTextFile } from "@effectionx/fs"; +import { exec } from "@effectionx/process"; +import { useTempDirectory } from "@executablemd/test-support/temp"; +import { fileURLToPath, pathToFileURL } from "node:url"; + +import { publishableMembers } from "../lib/publishable-members.ts"; + +const repoRoot = new URL("../../", import.meta.url); + +/** One version for the whole fixture; these cases are about membership. */ +const VERSION = "1.0.0"; + +/** A tag no fixture manifest declares, so the preflight reports every member it selected. */ +const FOREIGN_TAG = "v9.9.9"; + +interface MemberSpec { + dir: string; + /** `deno.json`'s name; omitted writes no `deno.json` at all. */ + denoName?: string; + /** `package.json`'s name; omitted writes no `package.json` at all. */ + packageName?: string; + private?: boolean; +} + +function* fixture(members: MemberSpec[]): Operation { + const base = yield* useTempDirectory("membership-agreement-"); + const root = pathToFileURL(`${base}/`); + + yield* writeTextFile( + new URL("deno.json", root), + `${JSON.stringify({ workspace: ["packages/*"] }, null, 2)}\n`, + ); + yield* ensureDir(new URL(".github/workflows/", root)); + + for (const member of members) { + yield* ensureDir(new URL(`packages/${member.dir}/`, root)); + if (member.denoName !== undefined) { + yield* writeTextFile( + new URL(`packages/${member.dir}/deno.json`, root), + `${JSON.stringify( + { name: member.denoName, version: VERSION, exports: "./mod.ts" }, + null, + 2, + )}\n`, + ); + } + if (member.packageName !== undefined) { + yield* writeTextFile( + new URL(`packages/${member.dir}/package.json`, root), + `${JSON.stringify( + { name: member.packageName, version: VERSION, private: member.private }, + null, + 2, + )}\n`, + ); + } + } + + return root; +} + +/** The `run:` body of `release.yml`'s preflight step, dedented, with the tag substituted. */ +function* preflightScript(tag: string): Operation { + const lines = (yield* readTextFile(new URL(".github/workflows/release.yml", repoRoot))).split( + "\n", + ); + const step = lines.findIndex((line) => + line.includes("name: Tag matches every publishable manifest"), + ); + expect(step).toBeGreaterThan(-1); + const opens = lines.findIndex((line, index) => index > step && line.trim() === "run: |"); + expect(opens).toBeGreaterThan(step); + + const indent = lines[opens].length - lines[opens].trimStart().length + 2; + const body: string[] = []; + for (const line of lines.slice(opens + 1)) { + if (line.trim() !== "" && line.search(/\S/) < indent) { + break; + } + body.push(line.slice(indent)); + } + + return body.join("\n").replace("${{ github.ref_name }}", tag); +} + +/** + * The members `release.yml`'s preflight selects, read back from the mismatches + * it reports: against a tag no manifest declares, it names both manifests of + * every member it looked at and nothing else. + */ +function* fromPreflight(root: URL): Operation { + const script = new URL("preflight.sh", root); + yield* writeTextFile(script, `${yield* preflightScript(FOREIGN_TAG)}\n`); + + const run = yield* exec("sh", { + arguments: [fileURLToPath(script)], + cwd: fileURLToPath(root), + }).join(); + + const selected = new Set(); + for (const [, path] of run.stdout.matchAll(/::error::(packages\/[^/]+)\/[^\s]+ declares/g)) { + selected.add(path); + } + return [...selected].toSorted(); +} + +/** The members the real generator writes publish jobs for. */ +function* fromGenerator(root: URL): Operation { + yield* exec(Deno.execPath(), { + arguments: [ + "run", + "--allow-all", + // Without it Deno reads the fixture's own deno.json and no import resolves. + "--config", + fileURLToPath(new URL("deno.json", repoRoot)), + fileURLToPath(new URL("packages/cli/src/deno.ts", repoRoot)), + "run", + fileURLToPath(new URL("scripts/gen-publish-workflow.md", repoRoot)), + ], + cwd: fileURLToPath(root), + }).join(); + + // Read rather than trust the exit status: a failing eval block writes an + // ERROR comment and leaves the CLI reporting success (#237), so an absent or + // empty workflow has to fail this here. + const generated = yield* readTextFile(new URL(".github/workflows/publish-packages.yml", root)); + return [...generated.matchAll(/package: (packages\/\S+)/g)].map(([, dir]) => dir).toSorted(); +} + +function* fromLibrary(root: URL): Operation { + return (yield* publishableMembers(root)).map((member) => member.dir).toSorted(); +} + +describe("publishable membership", () => { + /** + * Every shape that can make the two manifest names disagree, in one + * workspace. At the commit this test was written against, `by-package-name` + * alone would have split the three: the generator published it, and both + * gates ignored it. + */ + it("is one set, whichever of the three selectors reads it", function* () { + const root = yield* fixture([ + { + dir: "ordinary", + denoName: "@executablemd/ordinary", + packageName: "@executablemd/ordinary", + }, + { dir: "by-deno-name", denoName: "@executablemd/renamed", packageName: "renamed-on-npm" }, + { + dir: "by-package-name", + denoName: "local-tool", + packageName: "@executablemd/looks-published", + }, + { + dir: "withheld", + denoName: "@executablemd/withheld", + packageName: "@executablemd/withheld", + private: true, + }, + { dir: "no-package-json", denoName: "@executablemd/half" }, + { dir: "no-deno-json", packageName: "@executablemd/half-again" }, + ]); + + const library = yield* fromLibrary(root); + + // The set every selector has to reach: identity from deno.json, exclusion + // from package.json, and both manifests present. + expect(library).toEqual(["packages/by-deno-name", "packages/ordinary"]); + expect(yield* fromPreflight(root)).toEqual(library); + expect(yield* fromGenerator(root)).toEqual(library); + }); + + /** + * Non-vacuous: the three-way comparison above would also hold if every + * selector returned nothing, which is what a fixture the generator refused to + * read would produce. + */ + it("selects nothing from a workspace whose members all publish nothing", function* () { + const root = yield* fixture([ + { dir: "local", denoName: "local-tool", packageName: "local-tool" }, + ]); + + expect(yield* fromLibrary(root)).toEqual([]); + expect(yield* fromPreflight(root)).toEqual([]); + expect(yield* fromGenerator(root)).toEqual([]); + }); +}); diff --git a/scripts/tests/version-lockstep.test.ts b/scripts/tests/version-lockstep.test.ts index c4f911ea2..939ad6adb 100644 --- a/scripts/tests/version-lockstep.test.ts +++ b/scripts/tests/version-lockstep.test.ts @@ -6,16 +6,20 @@ import { useTempDirectory } from "@executablemd/test-support/temp"; import { pathToFileURL } from "node:url"; import { parseBunLockfile } from "../lib/bun-lockfile.ts"; -import { publishableMembers, versionLockstepFindings } from "../lib/version-lockstep.ts"; +import { publishableMembers, publishedName } from "../lib/publishable-members.ts"; +import { versionLockstepFindings } from "../lib/version-lockstep.ts"; const repoRoot = new URL("../../", import.meta.url); interface MemberSpec { dir: string; - name: string; - /** What both manifests declare, unless `deno` overrides `deno.json`. */ + /** `deno.json`'s name; omitted writes no `deno.json` at all. */ + denoName?: string; + /** `package.json`'s name; omitted writes no `package.json` at all. */ + packageName?: string; + /** What both manifests declare, unless `denoVersion` overrides `deno.json`. */ version?: string; - deno?: string; + denoVersion?: string; private?: boolean; /** What `bun.lock` records; omitted writes no entry for the member. */ locked?: string; @@ -34,27 +38,33 @@ function* workspace(members: MemberSpec[]): Operation { const entries = [' "": {\n "name": "root",\n },']; for (const member of members) { yield* ensureDir(new URL(`packages/${member.dir}/`, root)); - yield* writeTextFile( - new URL(`packages/${member.dir}/package.json`, root), - `${JSON.stringify( - { name: member.name, version: member.version, private: member.private }, - null, - 2, - )}\n`, - ); - yield* writeTextFile( - new URL(`packages/${member.dir}/deno.json`, root), - `${JSON.stringify( - { name: member.name, version: member.deno ?? member.version, exports: "./mod.ts" }, - null, - 2, - )}\n`, - ); - if (member.locked !== undefined) { - entries.push( - ` "packages/${member.dir}": {\n "name": "${member.name}",\n "version": "${member.locked}",\n },`, + if (member.denoName !== undefined) { + yield* writeTextFile( + new URL(`packages/${member.dir}/deno.json`, root), + `${JSON.stringify( + { + name: member.denoName, + version: member.denoVersion ?? member.version, + exports: "./mod.ts", + }, + null, + 2, + )}\n`, + ); + } + if (member.packageName !== undefined) { + yield* writeTextFile( + new URL(`packages/${member.dir}/package.json`, root), + `${JSON.stringify( + { name: member.packageName, version: member.version, private: member.private }, + null, + 2, + )}\n`, ); } + if (member.locked !== undefined) { + entries.push(` "packages/${member.dir}": {\n "version": "${member.locked}",\n },`); + } } // Written with the trailing commas Bun writes, so every case reads the @@ -70,11 +80,42 @@ function* workspace(members: MemberSpec[]): Operation { /** One publishable member, in lockstep, as the baseline every case varies. */ const SCOPED: MemberSpec = { dir: "scoped", - name: "@executablemd/scoped", + denoName: "@executablemd/scoped", + packageName: "@executablemd/scoped", version: "1.0.0", locked: "1.0.0", }; +describe("publishedName", () => { + it("names a member from deno.json, whatever package.json calls it", function* () { + expect(publishedName({ name: "@executablemd/renamed" }, { name: "renamed-on-npm" })).toEqual( + "@executablemd/renamed", + ); + }); + + /** + * The inverse, and the reason identity cannot come from `package.json`: this + * member publishes nothing, because the publish generator and the npm + * builder both read `deno.json`. + */ + it("names no member whose scope is only in package.json", function* () { + expect( + publishedName({ name: "local-tool" }, { name: "@executablemd/looks-published" }), + ).toBeUndefined(); + }); + + it("names no private member", function* () { + const withheld = { name: "@executablemd/support" }; + expect( + publishedName(withheld, { name: "@executablemd/support", private: true }), + ).toBeUndefined(); + }); + + it("names no member that declares no name at all", function* () { + expect(publishedName({ version: "1.0.0" }, { name: "@executablemd/nameless" })).toBeUndefined(); + }); +}); + describe("versionLockstepFindings", () => { it("reports nothing when every manifest and the lockfile agree", function* () { const root = yield* workspace([SCOPED]); @@ -89,7 +130,13 @@ describe("versionLockstepFindings", () => { it("reports a member the bump left behind", function* () { const root = yield* workspace([ SCOPED, - { dir: "behind", name: "@executablemd/behind", version: "0.9.0", locked: "0.9.0" }, + { + dir: "behind", + denoName: "@executablemd/behind", + packageName: "@executablemd/behind", + version: "0.9.0", + locked: "0.9.0", + }, ]); expect(yield* versionLockstepFindings(root)).toEqual([ @@ -100,7 +147,7 @@ describe("versionLockstepFindings", () => { }); it("reports one member whose two manifests disagree", function* () { - const root = yield* workspace([{ ...SCOPED, deno: "0.9.0" }]); + const root = yield* workspace([{ ...SCOPED, denoVersion: "0.9.0" }]); expect(yield* versionLockstepFindings(root)).toEqual([ "the workspace declares more than one version: 0.9.0 (packages/scoped/deno.json); " + @@ -109,7 +156,9 @@ describe("versionLockstepFindings", () => { }); it("reports a manifest that declares no version", function* () { - const root = yield* workspace([{ dir: "scoped", name: "@executablemd/scoped" }]); + const root = yield* workspace([ + { dir: "scoped", denoName: "@executablemd/scoped", packageName: "@executablemd/scoped" }, + ]); expect(yield* versionLockstepFindings(root)).toEqual([ "packages/scoped/deno.json declares no version", @@ -118,10 +167,39 @@ describe("versionLockstepFindings", () => { ]); }); + /** + * The member the reviewed commit let through: the publish workflow generates + * a job for it, so the release version has to cover it too. + */ + it("holds a member whose npm name differs from its JSR name", function* () { + const root = yield* workspace([ + SCOPED, + { + dir: "renamed", + denoName: "@executablemd/renamed", + packageName: "renamed-on-npm", + version: "0.9.0", + locked: "0.9.0", + }, + ]); + + expect(yield* versionLockstepFindings(root)).toEqual([ + "the workspace declares more than one version: 0.9.0 (packages/renamed/deno.json, " + + "packages/renamed/package.json); 1.0.0 (packages/scoped/deno.json, " + + "packages/scoped/package.json)", + ]); + }); + it("holds no private member to the release version", function* () { const root = yield* workspace([ SCOPED, - { dir: "support", name: "@executablemd/support", version: "0.0.0", private: true }, + { + dir: "support", + denoName: "@executablemd/support", + packageName: "@executablemd/support", + version: "0.0.0", + private: true, + }, ]); expect(yield* versionLockstepFindings(root)).toEqual([]); @@ -130,7 +208,7 @@ describe("versionLockstepFindings", () => { it("holds no member outside the @executablemd scope to it either", function* () { const root = yield* workspace([ SCOPED, - { dir: "outside", name: "outside-tool", version: "7.7.7" }, + { dir: "outside", denoName: "outside-tool", packageName: "outside-tool", version: "7.7.7" }, ]); expect(yield* versionLockstepFindings(root)).toEqual([]); @@ -155,7 +233,7 @@ describe("versionLockstepFindings", () => { /** * The gate against the tree it guards. `packages/git` reached `main` at * `0.12.1` while every sibling moved to `0.13.0`, and `v0.13.0` published - * binaries and no packages because only the package gate reads every + * binaries and no packages because only the package gate read every * manifest — this is that state, asserted where a pull request can see it. */ it("holds this workspace in lockstep", function* () { diff --git a/specs/release-process-spec.md b/specs/release-process-spec.md index af8601b4c..2eaa7c3d2 100644 --- a/specs/release-process-spec.md +++ b/specs/release-process-spec.md @@ -48,24 +48,34 @@ sequenceDiagram ## 2. Version lockstep -Every publishable package declares the same version in its `deno.json` and -`package.json`. Membership is the workspace, not a list: a `packages/*` member -whose `package.json` `name` is under `@executablemd` is publishable, so a new -package joins the lockstep by existing. A member marked `"private": true` is -outside it because it never publishes — `packages/test-support` is the one, and -it stays at `0.0.0`. `packages/cli/src/cli.ts` -imports `packages/cli/deno.json` and reads `version` +**A workspace member is publishable when its `deno.json` declares a `name` +under `@executablemd`, its `package.json` does not declare `"private": true`, +and both manifests are present.** That is the whole definition, and everything +that selects release members uses it: the publish-workflow generator, the npm +builder, `deno task bump`, both tag-time gates and the pull-request check. +Membership is therefore the workspace and not a list — a new package joins by +existing. `packages/test-support` is the one private member, and it stays at +`0.0.0`. + +Identity comes from `deno.json` because that is the name the packages actually +publish under. Nothing requires the two manifests to agree about it, so a +selector reading `package.json`'s name instead would admit a different set, and +a set the gates disagree about is a tag that publishes one half of a release. +Both manifests must be present for the same reason: no `deno.json` is no JSR +entry, and no `package.json` is no npm package. + +Every publishable member declares the same version in both its manifests. +`packages/cli/src/cli.ts` imports `packages/cli/deno.json` and reads `version` from it, so the compiled binary reports the manifest version — the manifests are the single source. -Three checks hold that, and they read the same set. `scripts/lib/version-lockstep.ts` -answers it for a pull request, where drift is introduced and where it is still -cheap: it walks the workspace and reports every manifest that declares a -different version from its siblings, and every `bun.lock` workspace entry that -has gone stale or missing. The other two are the tag-time gates — `release.yml` -before the binaries, `publish-packages.yml` before the packages — and each -refuses a tag every publishable manifest does not declare, so the tag and the -manifests cannot diverge. +Three checks hold that. `scripts/lib/version-lockstep.ts` answers it for a pull +request, where the drift is introduced and where it is still cheap: it reports +every manifest that declares a different version from its siblings, and every +`bun.lock` workspace entry that has gone stale or missing. The other two are the +tag-time gates — `release.yml` before the binaries, `publish-packages.yml` +before the packages — and each refuses a tag every publishable manifest does not +declare, so the tag and the manifests cannot diverge. A gate that reads fewer manifests than the other is the failure this arrangement exists to prevent. `v0.13.0` published its binaries and none of its @@ -101,8 +111,8 @@ documents at the revision it checks. the manifests need bumping; once bumped, the banner clears and the draft's tag and title default to the guard-passing `v`. - **`release.yml`** (`push: tags v*`): a preflight job validates the tag - against every publishable manifest, walked from `packages/*` so a new package - joins by existing; on mismatch it flags the just-published release on + against both manifests of every publishable member (§2); on mismatch it flags + the just-published release on the Releases page — caution note in the notes, a failed title, and the prerelease marker — so a forgotten bump is visible where the release was made, then refuses to build. On a valid tag it compiles @@ -136,17 +146,15 @@ documents at the revision it checks. - **`publish-packages.yml`** (`push: tags v*`): GENERATED by `scripts/gen-publish-workflow.md` — an executable markdown document that expands the root `workspace` entries (including one-level globs such as - `packages/*`) and derives the jobs from the member manifests it finds, a - member without a `deno.json` naming an `@executablemd` package being skipped - — never edited by hand. A member whose `package.json` declares - `"private": true` is also skipped and appears in no npm job, so a package can - land its foundation on `main` before it is ready to publish; clearing the flag - adds it back on the next regeneration. Such a member also declares no - `deno.json` `name` and no `exports`, so `deno install` warns about neither an - unpublishable name nor a missing `exports`, and `deno publish` finds no JSR - entry to publish either; both fields land in the same PR that clears the - private flag. The two conditions are independent rules, and no repository - member reaches the second one on its own, so + `packages/*`) and derives one job per publishable member (§2) — never edited + by hand. A member held back by `"private": true` appears in no npm job, so a + package can land its foundation on `main` before it is ready to publish; + clearing the flag adds it back on the next regeneration. Such a member also + declares no `deno.json` `name` and no `exports`, so `deno install` warns about + neither an unpublishable name nor a missing `exports`, and `deno publish` + finds no JSR entry to publish either; both fields land in the same PR that + clears the private flag. The two conditions are independent rules, and no + repository member reaches the second one on its own, so `scripts/tests/publish-workflow-generator.test.ts` runs the generator over a fixture member that holds a full JSR identity and declares `"private": true`. Run