diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index daf36b7b4..a42d445b6 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -10,22 +10,48 @@ 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 + # 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 }}" - 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 + 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' "$member/package.json")" = "true" ]; then + continue + fi + 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" + 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/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 new file mode 100644 index 000000000..2bba83278 --- /dev/null +++ b/scripts/lib/version-lockstep.ts @@ -0,0 +1,80 @@ +import { readTextFile } from "@effectionx/fs"; +import type { Operation } from "effection"; +import { z } from "zod"; + +import { parseBunLockfile } from "./bun-lockfile.ts"; +import { publishableMembers } from "./publishable-members.ts"; + +const VersionSchema = z.object({ version: z.string() }); + +/** The two manifests a publishable member declares its version in. */ +const MANIFESTS = ["deno.json", "package.json"]; + +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; +} + +/** + * 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. + * + * 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); + const findings: string[] = []; + const declared = new Map(); + + for (const member of members) { + 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(version, [...(declared.get(version) ?? []), 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/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 614262683..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}`); } }); @@ -236,3 +237,42 @@ 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* publishableMembers(repoRoot); + + // 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* publishableMembers(repoRoot)) + .filter((member) => commands.includes(`${member.dir}/deno.json`)) + .map((member) => member.dir); + + 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 new file mode 100644 index 000000000..939ad6adb --- /dev/null +++ b/scripts/tests/version-lockstep.test.ts @@ -0,0 +1,261 @@ +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, publishedName } from "../lib/publishable-members.ts"; +import { versionLockstepFindings } from "../lib/version-lockstep.ts"; + +const repoRoot = new URL("../../", import.meta.url); + +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; + /** What both manifests declare, unless `denoVersion` overrides `deno.json`. */ + version?: string; + denoVersion?: 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)); + 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 + // 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", + 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]); + + 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", + denoName: "@executablemd/behind", + packageName: "@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, denoVersion: "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", denoName: "@executablemd/scoped", packageName: "@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", + ]); + }); + + /** + * 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", + denoName: "@executablemd/support", + packageName: "@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", denoName: "outside-tool", packageName: "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 read 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..2eaa7c3d2 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,40 @@ 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` -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. 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. `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 +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 +90,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 +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 `packages/cli/deno.json`; 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 @@ -121,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