From 5eef239dfd774331d622aaa4762f42fa632121ab Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Thu, 12 Nov 2026 05:08:27 -0500 Subject: [PATCH] fix(ci): defer proposals for merged release commits --- docs/development/releasing.md | 7 + scripts/ci/scopes.mjs | 3 +- scripts/lib/release-notes-publication.mjs | 48 +----- scripts/lib/release-proposal.d.mts | 16 ++ scripts/lib/release-proposal.mjs | 20 +++ scripts/lib/release-pull-request.mjs | 60 +++++++ scripts/release-proposal.mjs | 57 ++++--- .../boundary/filesystem/ciChangeScope.test.ts | 2 + .../boundary/filesystem/releaseNotes.test.ts | 13 ++ .../filesystem/releaseProposal.test.ts | 146 +++++++++++++++++- 10 files changed, 304 insertions(+), 68 deletions(-) create mode 100644 scripts/lib/release-pull-request.mjs diff --git a/docs/development/releasing.md b/docs/development/releasing.md index 8185768320..e0e8942595 100644 --- a/docs/development/releasing.md +++ b/docs/development/releasing.md @@ -4,6 +4,13 @@ Pushes to main automatically open or refresh a Release Please PR with the next version and changelog. Review its migration notes and wait for its current CI checks to pass. Merging that PR into main starts publication automatically. +The push caused by merging a same-repository Release Please PR skips proposal +generation: the separate merged-release workflow owns tag creation and +publication. This avoids looking for the new version's tag before that workflow +has created it. The next ordinary main push resumes proposals. API lookup +failures and missing release boundaries on ordinary pushes or manual preparation +still fail closed. + The Release workflow validates the merged release metadata and ancestry before creating a tag. npm and MCP Registry publication use the exact reviewed merge SHA returned by Release Please. Before either registry is published, the release diff --git a/scripts/ci/scopes.mjs b/scripts/ci/scopes.mjs index cc794ea215..eafcec83d5 100644 --- a/scripts/ci/scopes.mjs +++ b/scripts/ci/scopes.mjs @@ -242,7 +242,8 @@ const PATH_SCOPES = { "scripts/windows-capabilities.mjs", "scripts/prepack.mjs", "scripts/prepare.mjs", - "scripts/lib/release-proposal.mjs", + "scripts/lib/release-proposal.*", + "scripts/lib/release-pull-request.mjs", "patches/release-please+17.3.0.patch", "tests/boundary/filesystem/releaseProposal.test.ts", "scripts/rea.mjs", diff --git a/scripts/lib/release-notes-publication.mjs b/scripts/lib/release-notes-publication.mjs index 8d4779053d..8be8b543c1 100644 --- a/scripts/lib/release-notes-publication.mjs +++ b/scripts/lib/release-notes-publication.mjs @@ -1,5 +1,6 @@ import { valid } from "semver"; import { z } from "zod"; +import { readMergedReleasePullRequest } from "./release-pull-request.mjs"; import { releaseInventorySchema, verifyInventoryHistory, @@ -8,17 +9,6 @@ import { const sha = z.string().regex(/^[a-f0-9]{40}$/u); const repository = z.string().regex(/^[\w.-]+\/[\w.-]+$/u); -const pullRequest = z.object({ - number: z.number().int().positive(), - body: z.string(), - merged_at: z.string().nullable(), - merge_commit_sha: sha.nullable(), - head: z.object({ - ref: z.string(), - repo: z.object({ full_name: z.string() }).nullable(), - }), - labels: z.array(z.object({ name: z.string() })), -}); /** Read the reviewed Git snapshot and live PR before Release Please can mutate tags. */ export async function verifyNotesPublication(run, options) { @@ -39,41 +29,13 @@ export async function verifyNotesPublication(run, options) { .parse(JSON.parse(await read("package.json"))); if (valid(version) !== version || version.includes("+")) throw new Error("Invalid reviewed package version"); - let candidates; - if (options.pr !== undefined) { - const number = z.coerce.number().int().positive().parse(options.pr); - candidates = [ - pullRequest.parse( - JSON.parse(await run("gh", ["api", `repos/${repo}/pulls/${number}`])), - ), - ]; - } else { - candidates = z - .array(z.array(pullRequest)) - .parse( - JSON.parse( - await run("gh", [ - "api", - "--paginate", - "--slurp", - `repos/${repo}/commits/${source}/pulls?per_page=100`, - ]), - ), - ) - .flat(); - } - const matching = candidates.filter( - (pr) => - pr.merged_at !== null && - pr.merge_commit_sha === source && - pr.head.repo?.full_name.toLowerCase() === repo.toLowerCase() && - pr.head.ref.startsWith("release-please--branches--"), - ); - if (matching.length !== 1) + const pr = await readMergedReleasePullRequest(run, { ...options, source }); + if (pr === undefined) throw new Error( "Expected exactly one merged Release Please PR for the reviewed source", ); - const pr = matching[0]; + if (pr.body === null) + throw new Error("The merged release PR has no release notes body"); const path = `docs/releases/${version}.contributions.json`; const present = ( diff --git a/scripts/lib/release-proposal.d.mts b/scripts/lib/release-proposal.d.mts index 76aaaaa5d3..ea47f7082d 100644 --- a/scripts/lib/release-proposal.d.mts +++ b/scripts/lib/release-proposal.d.mts @@ -6,3 +6,19 @@ export function releaseProposalSearchDepth( run: (command: string, args: string[]) => Promise, sourceSha: string, ): Promise; + +export function createReleaseProposal( + run: (command: string, args: string[]) => Promise, + context: { + repository: string; + sourceSha: string; + targetBranch: string; + eventName: string | undefined; + }, + loadManifest: (depth: number) => Promise<{ + createPullRequests(): Promise>; + }>, +): Promise< + | { kind: "release-merge"; pullRequest: number } + | { kind: "proposal"; pullRequests: T[] } +>; diff --git a/scripts/lib/release-proposal.mjs b/scripts/lib/release-proposal.mjs index c6b4e5bc2c..bd6e609c63 100644 --- a/scripts/lib/release-proposal.mjs +++ b/scripts/lib/release-proposal.mjs @@ -5,6 +5,7 @@ import { dirname, relative, sep } from "node:path"; import { promisify } from "node:util"; import { fileURLToPath } from "node:url"; import { requireCompleteReleaseHistory } from "./release-notes.mjs"; +import { readMergedReleasePullRequest } from "./release-pull-request.mjs"; const execute = promisify(execFile); const require = createRequire(import.meta.url); @@ -74,3 +75,22 @@ export async function releaseProposalSearchDepth(run, sourceSha) { } return count; } + +/** The merged-release workflow owns tag creation; its push needs no proposal. */ +export async function createReleaseProposal(run, context, loadManifest) { + if (context.eventName === "push" && context.targetBranch === "main") { + const release = await readMergedReleasePullRequest(run, { + repository: context.repository, + source: context.sourceSha, + targetBranch: context.targetBranch, + }); + if (release !== undefined) + return { kind: "release-merge", pullRequest: release.number }; + } + const depth = await releaseProposalSearchDepth(run, context.sourceSha); + const manifest = await loadManifest(depth); + return { + kind: "proposal", + pullRequests: (await manifest.createPullRequests()).filter(Boolean), + }; +} diff --git a/scripts/lib/release-pull-request.mjs b/scripts/lib/release-pull-request.mjs new file mode 100644 index 0000000000..b4cac61db9 --- /dev/null +++ b/scripts/lib/release-pull-request.mjs @@ -0,0 +1,60 @@ +import { z } from "zod"; + +const sha = z.string().regex(/^[a-f0-9]{40}$/u); +const repository = z.string().regex(/^[\w.-]+\/[\w.-]+$/u); +const pullRequest = z.object({ + number: z.number().int().positive(), + body: z.string().nullable(), + merged_at: z.string().nullable(), + merge_commit_sha: sha.nullable(), + base: z.object({ ref: z.string() }), + head: z.object({ + ref: z.string(), + repo: z.object({ full_name: z.string() }).nullable(), + }), + labels: z.array(z.object({ name: z.string() })), +}); + +/** Own REST release-PR identity for proposal admission and publication checks. */ +export async function readMergedReleasePullRequest(run, options) { + const repo = repository.parse(options.repository); + const source = sha.parse(options.source); + let candidates; + if (options.pr !== undefined) { + const number = z.coerce.number().int().positive().parse(options.pr); + candidates = [ + pullRequest.parse( + JSON.parse(await run("gh", ["api", `repos/${repo}/pulls/${number}`])), + ), + ]; + } else { + candidates = z + .array(z.array(pullRequest)) + .parse( + JSON.parse( + await run("gh", [ + "api", + "--paginate", + "--slurp", + `repos/${repo}/commits/${source}/pulls?per_page=100`, + ]), + ), + ) + .flat(); + } + const matching = candidates.filter( + (pr) => + pr.merged_at !== null && + pr.merge_commit_sha === source && + pr.head.repo?.full_name.toLowerCase() === repo.toLowerCase() && + pr.head.ref.startsWith("release-please--branches--") && + (options.targetBranch === undefined || + (pr.base.ref === options.targetBranch && + pr.head.ref.startsWith( + `release-please--branches--${options.targetBranch}--`, + ))), + ); + if (matching.length > 1) + throw new Error("Multiple merged Release Please PRs match the source"); + return matching[0]; +} diff --git a/scripts/release-proposal.mjs b/scripts/release-proposal.mjs index 78a22c56b4..0b7cd49f81 100644 --- a/scripts/release-proposal.mjs +++ b/scripts/release-proposal.mjs @@ -4,8 +4,8 @@ import { appendFile } from "node:fs/promises"; import { parseArgs, promisify } from "node:util"; import { applyReleasePleasePatch, + createReleaseProposal, describeReleaseProposalError, - releaseProposalSearchDepth, } from "./lib/release-proposal.mjs"; const execute = promisify(execFile); @@ -27,33 +27,44 @@ async function main() { ) { throw new Error("Expected GITHUB_REPOSITORY, GH_TOKEN and --target-branch"); } - const depth = await releaseProposalSearchDepth( - async (command, args) => - (await execute(command, args, { encoding: "utf8" })).stdout, - values["source-sha"], - ); - // Patch before import: only proposal generation uses this maintained upstream fix. - // Publication continues through the pinned release-please action. - await applyReleasePleasePatch(); - const { GitHub, Manifest } = await import("release-please"); const [owner, repo] = repository.split("/"); const branch = values["target-branch"]; - const github = await GitHub.create({ - owner, - repo, - token, - defaultBranch: branch, - }); - const manifest = await Manifest.fromManifest( - github, - branch, - undefined, - undefined, + const result = await createReleaseProposal( + async (command, args) => + ( + await execute(command, args, { + encoding: "utf8", + maxBuffer: 64 * 1024 * 1024, + }) + ).stdout, { - commitSearchDepth: depth, + repository, + targetBranch: branch, + sourceSha: values["source-sha"], + eventName: process.env.GITHUB_EVENT_NAME, + }, + async (depth) => { + // Patch before import: publication uses the pinned action separately. + await applyReleasePleasePatch(); + const { GitHub, Manifest } = await import("release-please"); + const github = await GitHub.create({ + owner, + repo, + token, + defaultBranch: branch, + }); + return Manifest.fromManifest(github, branch, undefined, undefined, { + commitSearchDepth: depth, + }); }, ); - const prs = (await manifest.createPullRequests()).filter(Boolean); + if (result.kind === "release-merge") { + const message = `Release PR #${result.pullRequest} merged at ${values["source-sha"]}; the release workflow owns publication. Proposal generation resumes on the next main push.\n`; + process.stdout.write(message); + if (process.env.GITHUB_STEP_SUMMARY) + await appendFile(process.env.GITHUB_STEP_SUMMARY, message); + } + const prs = result.kind === "proposal" ? result.pullRequests : []; if (process.env.GITHUB_OUTPUT) { await appendFile( process.env.GITHUB_OUTPUT, diff --git a/tests/boundary/filesystem/ciChangeScope.test.ts b/tests/boundary/filesystem/ciChangeScope.test.ts index eaa08dffdd..d3b35e28d3 100644 --- a/tests/boundary/filesystem/ciChangeScope.test.ts +++ b/tests/boundary/filesystem/ciChangeScope.test.ts @@ -693,6 +693,8 @@ it.each([ { code: true, runtime: false, browser: false, package: false }, ], ["scripts/lib/release-proposal.mjs", { code: true, package: true }], + ["scripts/lib/release-proposal.d.mts", { code: true, package: true }], + ["scripts/lib/release-pull-request.mjs", { code: true, package: true }], ["patches/release-please+17.3.0.patch", { code: true, package: true }], [ "tests/boundary/filesystem/releaseProposal.test.ts", diff --git a/tests/boundary/filesystem/releaseNotes.test.ts b/tests/boundary/filesystem/releaseNotes.test.ts index 4ec741a90c..e56454ad73 100644 --- a/tests/boundary/filesystem/releaseNotes.test.ts +++ b/tests/boundary/filesystem/releaseNotes.test.ts @@ -583,6 +583,7 @@ it("checks the live merged PR and exact Git snapshot before either publication r body: f.notes, merged_at: "2026-10-11T00:00:00Z", merge_commit_sha: source, + base: { ref: "main" }, head: { ref: "release-please--branches--main--components--rea-agents", repo: { full_name: "fixture/rea" }, @@ -615,6 +616,18 @@ it("checks the live merged PR and exact Git snapshot before either publication r "--source", source, ]); + await writeFile(prFile, JSON.stringify({ ...pr, body: null })); + await expect( + f.invoke([ + "publication", + "--repository", + "fixture/rea", + "--source", + source, + ]), + ).rejects.toMatchObject({ + stderr: expect.stringContaining("no release notes body"), + }); await writeFile( prFile, JSON.stringify({ diff --git a/tests/boundary/filesystem/releaseProposal.test.ts b/tests/boundary/filesystem/releaseProposal.test.ts index 234d4967d9..d708c8fd93 100644 --- a/tests/boundary/filesystem/releaseProposal.test.ts +++ b/tests/boundary/filesystem/releaseProposal.test.ts @@ -15,6 +15,7 @@ import { z } from "zod"; import { applyReleasePleasePatch, + createReleaseProposal, describeReleaseProposalError, releaseProposalSearchDepth, } from "../../../scripts/lib/release-proposal.mjs"; @@ -410,6 +411,145 @@ async function releaseFixture({ return { manifest, github, requests, pages, historyQueries }; } +const mergedRelease = { + number: 7104, + body: "Release notes", + merged_at: "2026-11-12T08:58:31Z", + merge_commit_sha: latest, + base: { ref: "main" }, + head: { + ref: "release-please--branches--main--components--rea-agents", + repo: { full_name: "fixture/rea" }, + }, + labels: [], +}; +const proposalContext = { + repository: "fixture/rea", + sourceSha: latest, + targetBranch: "main", + eventName: "push", +}; +const proposalReads = + (pages: unknown) => async (command: string, args: string[]) => { + if (command === "gh") return JSON.stringify(pages); + if (command === "git" && args[0] === "rev-parse") return "false\n"; + if (command === "git" && args[0] === "rev-list") return "2\n"; + throw new Error(`Unexpected proposal read: ${command} ${args.join(" ")}`); + }; + +it("defers the release merge's push while its tag has not been created", async () => { + const fixture = await releaseFixture({ releases: [] }); + // This is the original failure while the publication workflow creates the tag. + await expect(fixture.manifest.createPullRequests()).rejects.toThrow( + "no matching release or tag found", + ); + const loadManifest = vi.fn(async () => fixture.manifest); + await expect( + createReleaseProposal( + proposalReads([[], [mergedRelease]]), + proposalContext, + loadManifest, + ), + ).resolves.toEqual({ kind: "release-merge", pullRequest: 7104 }); + expect(loadManifest).not.toHaveBeenCalled(); +}); + +it.each([ + { name: "ordinary push", prs: [] }, + { name: "unmerged PR", prs: [{ ...mergedRelease, merged_at: null }] }, + { + name: "ordinary PR with no description", + prs: [ + { + ...mergedRelease, + body: null, + head: { ...mergedRelease.head, ref: "feature" }, + }, + ], + }, + { + name: "deleted head repository", + prs: [{ ...mergedRelease, head: { ...mergedRelease.head, repo: null } }], + }, + { + name: "different merge commit", + prs: [{ ...mergedRelease, merge_commit_sha: baseline }], + }, + { + name: "fork release branch", + prs: [ + { + ...mergedRelease, + head: { ...mergedRelease.head, repo: { full_name: "fork/rea" } }, + }, + ], + }, + { + name: "ordinary feature PR", + prs: [ + { ...mergedRelease, head: { ...mergedRelease.head, ref: "feature" } }, + ], + }, + { + name: "another base branch", + prs: [{ ...mergedRelease, base: { ref: "release/6.7.0" } }], + }, +])("keeps a genuinely missing release fatal for $name", async ({ prs }) => { + const fixture = await releaseFixture({ releases: [] }); + await expect( + createReleaseProposal( + proposalReads([prs]), + proposalContext, + async () => fixture.manifest, + ), + ).rejects.toThrow("no matching release or tag found"); +}); + +it.each(["workflow_dispatch", undefined])( + "does not defer manual proposal preparation for event %s", + async (eventName) => { + const fixture = await releaseFixture({ releases: [] }); + const run = vi.fn(proposalReads([[mergedRelease]])); + await expect( + createReleaseProposal( + run, + { ...proposalContext, eventName }, + async () => fixture.manifest, + ), + ).rejects.toThrow("no matching release or tag found"); + expect(run.mock.calls.every(([command]) => command === "git")).toBe(true); + }, +); + +it.each([ + { name: "malformed API response", pages: { message: "unavailable" } }, + { + name: "ambiguous release identity", + pages: [[mergedRelease, { ...mergedRelease, number: 7105 }]], + }, +])("fails closed on $name before preparing a proposal", async ({ pages }) => { + const loadManifest = vi.fn(); + await expect( + createReleaseProposal(proposalReads(pages), proposalContext, loadManifest), + ).rejects.toThrow(); + expect(loadManifest).not.toHaveBeenCalled(); +}); + +it("preserves an API failure instead of treating the push as ordinary", async () => { + const failure = new Error("GitHub lookup unavailable"); + const loadManifest = vi.fn(); + await expect( + createReleaseProposal( + async () => { + throw failure; + }, + proposalContext, + loadManifest, + ), + ).rejects.toBe(failure); + expect(loadManifest).not.toHaveBeenCalled(); +}); + it("builds the root Node proposal without commit-file lookups and retains extra-file updates", async () => { const fixture = await releaseFixture(); const candidates = await fixture.manifest.buildPullRequests(); @@ -903,7 +1043,11 @@ const releasePatchFixture = async () => { await mkdir(directory, { recursive: true }); await writeFile(join(project, "package.json"), '{"type":"module"}'); await writeFile(patchPath, patch); - for (const script of ["release-proposal.mjs", "release-notes.mjs"]) + for (const script of [ + "release-proposal.mjs", + "release-notes.mjs", + "release-pull-request.mjs", + ]) await copyFile( join(source, "scripts/lib", script), join(project, "scripts/lib", script),