From 2cb29693c3d574c4898832047fe309ac1ad00e9d Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Tue, 3 Nov 2026 00:21:16 +0800 Subject: [PATCH] fix(ci): preserve release proposal overflow notes --- patches/release-please+17.3.0.patch | 45 +++++++++ scripts/lib/release-proposal.d.mts | 2 + scripts/lib/release-proposal.mjs | 9 ++ scripts/release-proposal.mjs | 3 +- .../filesystem/releaseProposal.test.ts | 94 +++++++++++++++++++ 5 files changed, 152 insertions(+), 1 deletion(-) diff --git a/patches/release-please+17.3.0.patch b/patches/release-please+17.3.0.patch index 9acd148174..54c1c79230 100644 --- a/patches/release-please+17.3.0.patch +++ b/patches/release-please+17.3.0.patch @@ -81,6 +81,51 @@ // In this case, there is no squashed merge commit. This could be a simple // merge commit, a rebase merge commit, or a direct commit to the branch. // Fallback to fetching the list of commits from the REST API. In the future +@@ -1154,15 +1154,40 @@ + * @returns {string} HTML URL of the new file + */ + async createFileOnNewBranch(filename, contents, newBranchName, baseBranchName) { +- // create or update new branch to match base branch +- await this.forkBranch(newBranchName, baseBranchName); +- // use the single file upload API ++ // Preserve existing notes until the replacement file is committed. ++ // A failed write must not leave an open proposal pointing at a missing file. ++ if (!(await this.getBranchSha(newBranchName))) { ++ const baseSha = await this.getBranchSha(baseBranchName); ++ if (!baseSha) { ++ throw new Error(`Unable to find base branch: ${baseBranchName}`); ++ } ++ await this.createNewBranch(newBranchName, baseSha); ++ } ++ let existingSha; ++ try { ++ const { data } = await this.octokit.repos.getContent({ ++ owner: this.repository.owner, ++ repo: this.repository.repo, ++ path: filename, ++ ref: newBranchName, ++ }); ++ if (Array.isArray(data) || data.type !== 'file' || !data.sha) { ++ throw new Error(`Expected a file at ${filename} on branch: ${newBranchName}`); ++ } ++ existingSha = data.sha; ++ } ++ catch (error) { ++ if (!(error instanceof request_error_1.RequestError && error.status === 404)) { ++ throw error; ++ } ++ } + const { data: { content }, } = await this.octokit.repos.createOrUpdateFileContents({ + owner: this.repository.owner, + repo: this.repository.repo, + path: filename, + // contents need to be base64 encoded +- content: Buffer.from(contents, 'binary').toString('base64'), ++ content: Buffer.from(contents, 'utf8').toString('base64'), ++ ...(existingSha ? { sha: existingSha } : {}), + message: 'Saving release notes', + branch: newBranchName, + }); --- a/build/src/github.d.ts +++ b/build/src/github.d.ts @@ -47,6 +47,8 @@ diff --git a/scripts/lib/release-proposal.d.mts b/scripts/lib/release-proposal.d.mts index bd92aaba19..76aaaaa5d3 100644 --- a/scripts/lib/release-proposal.d.mts +++ b/scripts/lib/release-proposal.d.mts @@ -1,5 +1,7 @@ export function applyReleasePleasePatch(directory?: string): Promise; +export function describeReleaseProposalError(error: unknown): string; + export function releaseProposalSearchDepth( run: (command: string, args: string[]) => Promise, sourceSha: string, diff --git a/scripts/lib/release-proposal.mjs b/scripts/lib/release-proposal.mjs index 1bbb709c61..c6b4e5bc2c 100644 --- a/scripts/lib/release-proposal.mjs +++ b/scripts/lib/release-proposal.mjs @@ -13,6 +13,15 @@ const patch = fileURLToPath( new URL("../../patches/release-please+17.3.0.patch", import.meta.url), ); +export function describeReleaseProposalError(error) { + if (!(error instanceof Error)) return String(error); + const description = error.message || error.name || "Unknown error"; + const status = error.status; + return Number.isInteger(status) && status >= 400 && status <= 599 + ? `${description} (HTTP ${status})` + : description; +} + export async function applyReleasePleasePatch( directory = dirname(require.resolve("release-please/package.json")), ) { diff --git a/scripts/release-proposal.mjs b/scripts/release-proposal.mjs index 3f85424ef3..78a22c56b4 100644 --- a/scripts/release-proposal.mjs +++ b/scripts/release-proposal.mjs @@ -4,6 +4,7 @@ import { appendFile } from "node:fs/promises"; import { parseArgs, promisify } from "node:util"; import { applyReleasePleasePatch, + describeReleaseProposalError, releaseProposalSearchDepth, } from "./lib/release-proposal.mjs"; @@ -63,7 +64,7 @@ async function main() { await main().catch((error) => { process.stderr.write( - `Release proposal failed: ${error instanceof Error ? error.message : String(error)}\n`, + `Release proposal failed: ${describeReleaseProposalError(error)}\n`, ); process.exitCode = 1; }); diff --git a/tests/boundary/filesystem/releaseProposal.test.ts b/tests/boundary/filesystem/releaseProposal.test.ts index f952091603..c5d27b566f 100644 --- a/tests/boundary/filesystem/releaseProposal.test.ts +++ b/tests/boundary/filesystem/releaseProposal.test.ts @@ -10,11 +10,13 @@ import { createRequire } from "node:module"; import { dirname, join } from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; import { promisify } from "node:util"; +import { RequestError } from "@octokit/request-error"; import { afterEach, beforeAll, expect, it, vi } from "vitest"; import { z } from "zod"; import { applyReleasePleasePatch, + describeReleaseProposalError, releaseProposalSearchDepth, } from "../../../scripts/lib/release-proposal.mjs"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; @@ -33,6 +35,16 @@ const silent = { trace() {}, }; +it("reports an HTTP status when a release proposal error has no message", () => { + const failure = Object.assign(new Error(""), { + name: "HttpError", + status: 504, + request: { headers: { authorization: "secret" } }, + }); + expect(describeReleaseProposalError(failure)).toBe("HttpError (HTTP 504)"); + expect(describeReleaseProposalError(new Error("broken"))).toBe("broken"); +}); + beforeAll(async () => { upstream = await import("release-please"); githubRuntime = require("release-please/build/src/github.js"); @@ -40,6 +52,88 @@ beforeAll(async () => { afterEach(() => vi.restoreAllMocks()); +it("preserves the existing overflow notes if their replacement fails", async () => { + const write = vi + .fn() + .mockRejectedValueOnce(new Error("upload failed")) + .mockResolvedValueOnce({ + data: { content: { html_url: "https://example.test/notes" } }, + }); + const getContent = vi.fn().mockResolvedValue({ + data: { type: "file", sha: "existing-blob" }, + }); + const createNewBranch = vi.fn(); + const forkBranch = vi.fn(); + const github = { + repository: { owner: "fixture", repo: "rea" }, + getBranchSha: vi.fn().mockResolvedValue("existing-branch"), + createNewBranch, + forkBranch, + octokit: { repos: { getContent, createOrUpdateFileContents: write } }, + } as unknown as Awaited>; + const update = () => + upstream.GitHub.prototype.createFileOnNewBranch.call( + github, + "release-notes.md", + "Notes: café", + "notes", + "main", + ); + await expect(update()).rejects.toThrow("upload failed"); + expect(forkBranch).not.toHaveBeenCalled(); + expect(createNewBranch).not.toHaveBeenCalled(); + await expect(update()).resolves.toBe("https://example.test/notes"); + expect(write).toHaveBeenLastCalledWith( + expect.objectContaining({ + branch: "notes", + sha: "existing-blob", + content: Buffer.from("Notes: café", "utf8").toString("base64"), + }), + ); +}); + +it("creates overflow notes on a new branch without requiring an existing file", async () => { + const write = vi.fn().mockResolvedValue({ + data: { content: { html_url: "https://example.test/notes" } }, + }); + const createNewBranch = vi.fn(); + const github = { + repository: { owner: "fixture", repo: "rea" }, + getBranchSha: vi + .fn() + .mockResolvedValueOnce(undefined) + .mockResolvedValueOnce("base-sha"), + createNewBranch, + octokit: { + repos: { + getContent: vi.fn().mockRejectedValue( + new RequestError("missing", 404, { + request: { + method: "GET", + url: "https://example.test/notes", + headers: {}, + }, + }), + ), + createOrUpdateFileContents: write, + }, + }, + } as unknown as Awaited>; + await expect( + upstream.GitHub.prototype.createFileOnNewBranch.call( + github, + "release-notes.md", + "Notes", + "notes", + "main", + ), + ).resolves.toBe("https://example.test/notes"); + expect(createNewBranch).toHaveBeenCalledWith("notes", "base-sha"); + expect(write).toHaveBeenCalledWith( + expect.not.objectContaining({ sha: expect.anything() }), + ); +}); + function respond(value: unknown, status = 200) { return new Response(JSON.stringify(value), { status,