diff --git a/docs/releasing.md b/docs/releasing.md index 72b51e5cb4..6c388d7211 100644 --- a/docs/releasing.md +++ b/docs/releasing.md @@ -94,13 +94,23 @@ Automatic main proposals and manual checkpoint preparation use `scripts/release-proposal.mjs` with the pinned release-please dependency. The controller applies `patches/release-please+17.3.0.patch` only when invoked; normal dependency installation and published-package installation do not apply it. -The patch avoids commit-file backfills and path splitting for a single root Node -package with the default changelog, default or always-bump-minor versioning, -no path exclusions and no explicit plugins. Other configurations retain the -upstream file attribution workflow. Associated PR metadata and commit overrides -still come from GitHub. The controller sizes commit search from the complete -selected Git history, and a missing known release boundary fails before proposal -creation. Publication continues through the pinned release-please action. +The patch avoids commit-file backfills, associated PR file lists and path splitting +for a single root Node package with the default changelog, default or +always-bump-minor versioning, no path exclusions and no explicit plugins. Other +configurations retain the upstream file attribution workflow. Commit history and +associated PR metadata still come from GitHub: release notes need the commits, +and `BEGIN_COMMIT_OVERRIDE` needs the merged PR body. Skipped file lists remain +unknown rather than being reported as empty. The controller sizes commit search +from the complete selected Git history, and a missing known release boundary +fails before proposal creation. + +History-page reads retry GitHub's canonical internal GraphQL response error within +the existing five-retry budget and exponential backoff. Each retry uses the same +cursor; permission, validation and mixed errors fail immediately. Exhaustion +stops preparation before proposal writes. The controller does not restart proposal +creation or retry mutations. An internal GraphQL error does not establish API +quota exhaustion or prove that query size caused the failure. Publication +continues through the pinned release-please action. When upgrading release-please, review the upstream manifest implementation, refresh the versioned patch and run the release proposal and workflow boundary diff --git a/patches/release-please+17.3.0.patch b/patches/release-please+17.3.0.patch index 6766660fed..f3a59d035a 100644 --- a/patches/release-please+17.3.0.patch +++ b/patches/release-please+17.3.0.patch @@ -1,6 +1,111 @@ +--- a/build/src/github.js ++++ b/build/src/github.js +@@ -78,14 +78,20 @@ + this.logger.trace('no GraphQL response, retrying'); + } + catch (err) { +- if (err.status !== 502) { ++ // Only commit-history reads opt in to transient HTTP-200 errors. ++ const retryInternalError = options?.retryInternalErrors && ++ err instanceof graphql_1.GraphqlResponseError && ++ err.errors.length > 0 && ++ err.errors.every(error => typeof error.message === 'string' && ++ error.message.startsWith('Something went wrong while executing your query')); ++ if (err.status !== 502 && !retryInternalError) { + throw err; + } + if (maxRetries === 0) { + this.logger.warn('ran out of retries and response is required'); + throw err; + } +- this.logger.info(`received 502 error, ${maxRetries} attempts remaining`); ++ this.logger.info(`received ${err.status === 502 ? '502' : 'internal GraphQL'} error, ${maxRetries} attempts remaining`); + } + maxRetries -= 1; + if (maxRetries >= 0) { +@@ -535,8 +541,9 @@ + async mergeCommitsGraphQL(targetBranch, cursor, options = {}) { + var _a, _b, _c, _d, _e, _f, _g, _h, _j; + var _k; ++ const includeFiles = options.includeFiles ?? true; + this.logger.debug(`Fetching merge commits on branch ${targetBranch} with cursor: ${cursor}`); +- const query = `query pullRequestsSince($owner: String!, $repo: String!, $num: Int!, $maxFilesChanged: Int, $targetBranch: String!, $cursor: String) { ++ const query = `query pullRequestsSince($owner: String!, $repo: String!, $num: Int!, $maxFilesChanged: Int, $includeFiles: Boolean!, $targetBranch: String!, $cursor: String) { + repository(owner: $owner, name: $repo) { + ref(qualifiedName: $targetBranch) { + target { +@@ -558,7 +565,7 @@ + mergeCommit { + oid + } +- files(first: $maxFilesChanged) { ++ files(first: $maxFilesChanged) @include(if: $includeFiles) { + nodes { + path + } +@@ -589,11 +596,12 @@ + num: (_a = options.batchSize) !== null && _a !== void 0 ? _a : 10, + targetBranch, + maxFilesChanged: 100, // max is 100 ++ includeFiles, + }; + const response = await this.graphqlRequest({ + query, + ...params, +- }); ++ }, { retryInternalErrors: true }); + if (!response) { + this.logger.warn(`Did not receive a response for query: ${query}`, params); + return null; +@@ -646,10 +654,12 @@ + title: pullRequest.title, + body: pullRequest.body, + labels: pullRequest.labels.nodes.map(node => node.name), +- files: (((_f = pullRequest.files) === null || _f === void 0 ? void 0 : _f.nodes) || []).map(node => node.path), ++ ...(includeFiles ? { ++ files: (((_f = pullRequest.files) === null || _f === void 0 ? void 0 : _f.nodes) || []).map(node => node.path), ++ } : {}), + }; + } +- if (mergePullRequest) { ++ if (includeFiles && mergePullRequest) { + if (((_h = (_g = mergePullRequest.files) === null || _g === void 0 ? void 0 : _g.pageInfo) === null || _h === void 0 ? void 0 : _h.hasNextPage) && + options.backfillFiles) { + this.logger.info(`PR #${mergePullRequest.number} has many files, backfilling`); +@@ -661,7 +671,7 @@ + commit.files = (((_j = mergePullRequest.files) === null || _j === void 0 ? void 0 : _j.nodes) || []).map(node => node.path); + } + } +- else if (options.backfillFiles) { ++ else if (includeFiles && options.backfillFiles) { + // 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 +--- a/build/src/github.d.ts ++++ b/build/src/github.d.ts +@@ -47,6 +47,8 @@ + interface CommitIteratorOptions { + maxResults?: number; + backfillFiles?: boolean; ++ /** Include PR files and permit REST backfill. Defaults to true. */ ++ includeFiles?: boolean; + batchSize?: number; + } + interface ReleaseIteratorOptions { +--- a/build/src/pull-request.d.ts ++++ b/build/src/pull-request.d.ts +@@ -6,6 +6,7 @@ + readonly title: string; + readonly body: string; + readonly labels: string[]; +- readonly files: string[]; ++ /** Unknown when commit-history file collection is explicitly disabled. */ ++ readonly files?: string[]; + readonly sha?: string; + } --- a/build/src/manifest.js +++ b/build/src/manifest.js -@@ -261,9 +261,19 @@ +@@ -261,9 +261,20 @@ this.logger.info('Collecting commits since all latest releases'); const commits = []; this.logger.debug(`commit search depth: ${this.commitSearchDepth}`); @@ -18,10 +123,11 @@ maxResults: this.commitSearchDepth, - backfillFiles: true, + backfillFiles: !rootOnlyNode, ++ includeFiles: !rootOnlyNode, batchSize: this.commitBatchSize, }); const releaseShas = new Set(Object.values(releaseShasByPath)); -@@ -272,6 +282,7 @@ +@@ -272,6 +283,7 @@ // sha => release pull request const releasePullRequestsBySha = {}; let releaseCommitsFound = 0; @@ -29,7 +135,7 @@ for await (const commit of commitGenerator) { if (releaseShas.has(commit.sha)) { if (commit.pullRequest) { -@@ -284,14 +295,17 @@ +@@ -284,14 +296,17 @@ } if (this.lastReleaseSha && this.lastReleaseSha === commit.sha) { this.logger.info(`Using configured lastReleaseSha ${this.lastReleaseSha} as last commit.`); @@ -47,7 +153,7 @@ break; } commits.push({ -@@ -301,16 +315,18 @@ +@@ -301,16 +316,18 @@ pullRequest: commit.pullRequest, }); } diff --git a/tests/boundary/filesystem/releaseProposal.test.ts b/tests/boundary/filesystem/releaseProposal.test.ts index 293d255d2a..7b4c37ad65 100644 --- a/tests/boundary/filesystem/releaseProposal.test.ts +++ b/tests/boundary/filesystem/releaseProposal.test.ts @@ -3,7 +3,7 @@ import { readFile, writeFile } from "node:fs/promises"; import { createRequire } from "node:module"; import { dirname, join } from "node:path"; import { promisify } from "node:util"; -import { beforeAll, expect, it } from "vitest"; +import { afterEach, beforeAll, expect, it, vi } from "vitest"; import { z } from "zod"; import { @@ -15,6 +15,7 @@ import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; const execute = promisify(execFile); const require = createRequire(import.meta.url); let upstream: typeof import("release-please"); +let githubRuntime: typeof import("release-please/build/src/github.js"); const baseline = "a".repeat(40); const latest = "b".repeat(40); const silent = { @@ -28,20 +29,58 @@ const silent = { beforeAll(async () => { await applyReleasePleasePatch(); upstream = await import("release-please"); + githubRuntime = require("release-please/build/src/github.js"); }); -function respond(value: unknown) { +afterEach(() => vi.restoreAllMocks()); + +function respond(value: unknown, status = 200) { return new Response(JSON.stringify(value), { - status: 200, + status, headers: { "content-type": "application/json" }, }); } -type CommitRecord = { sha: string; message: string; files: string[] }; +function associatedPullRequest(number = 42, mergeCommit = latest) { + return { + number, + title: "Original title", + baseRefName: "main", + headRefName: "feature", + body: `BEGIN_COMMIT_OVERRIDE\nfix: attributed override (#${number})\nEND_COMMIT_OVERRIDE`, + mergeCommit: { oid: mergeCommit }, + labels: { nodes: [{ name: "reviewed" }] }, + files: { + nodes: [{ path: "src/feature.ts" }], + pageInfo: { hasNextPage: false }, + }, + }; +} + +type CommitRecord = { + sha: string; + message: string; + files: string[]; + pullRequests?: ReturnType[]; +}; +type HistoryVariables = { + num: number; + cursor?: string | undefined; + includeFiles?: boolean | undefined; +}; +type HistoryFailure = { + cursor?: string; + errors: { message: string; type?: string }[]; + status?: number; +}; +const internalError = { + message: + "Something went wrong while executing your query on 2026-10-24T08:20:11Z. Please include `fixture:request` when reporting this issue.", +}; function historyPage( commits: CommitRecord[], - page: { num: number; cursor?: string | undefined }, + page: HistoryVariables, pullRequest: boolean, ) { const offset = Number(page.cursor ?? "0"); @@ -56,24 +95,15 @@ function historyPage( sha: commit.sha, message: commit.message, associatedPullRequests: { - nodes: - pullRequest && commit.sha === latest - ? [ - { - number: 42, - title: "Original title", - baseRefName: "main", - headRefName: "feature", - body: "BEGIN_COMMIT_OVERRIDE\nfix: attributed override (#42)\nEND_COMMIT_OVERRIDE", - mergeCommit: { oid: latest }, - labels: { nodes: [{ name: "reviewed" }] }, - files: { - nodes: [{ path: "src/feature.ts" }], - pageInfo: { hasNextPage: false }, - }, - }, - ] - : [], + nodes: ( + commit.pullRequests ?? + (pullRequest && commit.sha === latest + ? [associatedPullRequest()] + : []) + ).map(({ files, ...metadata }) => ({ + ...metadata, + ...(page.includeFiles === false ? {} : { files }), + })), }, })), pageInfo: { @@ -88,6 +118,40 @@ function historyPage( }; } +function releasePage() { + return { + data: { + repository: { + releases: { + nodes: [ + { + name: "6.7.0", + tag: { name: "rea-agents-6.7.0" }, + tagCommit: { oid: baseline }, + description: "", + isDraft: false, + url: "https://github.com/fixture/rea/releases/tag/rea-agents-6.7.0", + }, + ], + pageInfo: { hasNextPage: false }, + }, + }, + }, + }; +} + +const defaultConfiguration = { + "release-type": "node", + versioning: "always-bump-minor", + "include-v-in-tag": false, + packages: { + ".": { + "package-name": "rea-agents", + "extra-files": [{ type: "generic", path: "version.txt" }], + }, + }, +}; + async function releaseFixture({ config = {}, commits = [ @@ -96,26 +160,22 @@ async function releaseFixture({ ], depth = 500, pullRequest = false, + historyFailures = [], + releaseFailures = [], }: { config?: Record; commits?: CommitRecord[]; depth?: number; pullRequest?: boolean; + historyFailures?: HistoryFailure[]; + releaseFailures?: HistoryFailure["errors"][]; } = {}) { const requests: { method: string; pathname: string }[] = []; - const pages: { num: number; cursor?: string | undefined }[] = []; - const configuration = { - "release-type": "node", - versioning: "always-bump-minor", - "include-v-in-tag": false, - packages: { - ".": { - "package-name": "rea-agents", - "extra-files": [{ type: "generic", path: "version.txt" }], - }, - }, - ...config, - }; + const pages: HistoryVariables[] = []; + const historyQueries: string[] = []; + const failures = [...historyFailures]; + const releaseErrors = [...releaseFailures]; + const configuration = { ...defaultConfiguration, ...config }; const github = await upstream.GitHub.create({ owner: "fixture", repo: "rea", @@ -141,34 +201,31 @@ async function releaseFixture({ variables: z.object({ num: z.number(), cursor: z.string().optional(), + includeFiles: z.boolean().optional(), }), }) .parse(JSON.parse(options.body ?? "")); if (query.query.includes("query releases(")) { - return respond({ - data: { - repository: { - releases: { - nodes: [ - { - name: "6.7.0", - tag: { name: "rea-agents-6.7.0" }, - tagCommit: { oid: baseline }, - description: "", - isDraft: false, - url: "https://github.com/fixture/rea/releases/tag/rea-agents-6.7.0", - }, - ], - pageInfo: { hasNextPage: false }, - }, - }, - }, - }); + const errors = releaseErrors.shift(); + if (errors) return respond({ errors }); + return respond(releasePage()); } if (!query.query.includes("query pullRequestsSince(")) { throw new Error(`Unexpected GraphQL operation: ${query.query}`); } pages.push(query.variables); + historyQueries.push(query.query); + const failure = failures[0]; + if (failure && failure.cursor === query.variables.cursor) { + failures.shift(); + return respond( + { + ...historyPage(commits, query.variables, pullRequest), + errors: failure.errors, + }, + failure.status, + ); + } return respond(historyPage(commits, query.variables, pullRequest)); } const files: Record = { @@ -233,7 +290,7 @@ async function releaseFixture({ logger: silent, }, ); - return { manifest, github, requests, pages }; + return { manifest, github, requests, pages, historyQueries }; } it("builds the root Node proposal without commit-file lookups and retains extra-file updates", async () => { @@ -253,6 +310,11 @@ it("builds the root Node proposal without commit-file lookups and retains extra- ), ).toEqual([]); expect(fixture.pages.map((page) => page.num)).toEqual([10]); + expect(fixture.pages[0]?.includeFiles).toBe(false); + expect(fixture.historyQueries[0]).toContain("$includeFiles: Boolean!"); + expect(fixture.historyQueries[0]).toContain( + "files(first: $maxFilesChanged) @include(if: $includeFiles)", + ); const commits = []; for await (const commit of fixture.github.mergeCommitIterator("main", { backfillFiles: false, @@ -278,9 +340,67 @@ it("keeps real associated PR metadata, body overrides and attribution", async () labels: ["reviewed"], mergeCommitOid: latest, body: expect.stringContaining("BEGIN_COMMIT_OVERRIDE"), + files: ["src/feature.ts"], }); + expect(commits[0]?.files).toEqual(["src/feature.ts"]); + expect(fixture.pages.at(-1)?.includeFiles).toBe(true); }); +it.each([false, true])( + "preserves associated PR selection with omitted files for rebased=%s", + async (rebased) => { + const second = "c".repeat(40); + const fixture = await releaseFixture({ + commits: [ + { + sha: latest, + message: "fix: first change", + files: ["src/first.ts"], + pullRequests: [ + associatedPullRequest(7, baseline), + associatedPullRequest(), + ], + }, + { + sha: second, + message: "fix: second change", + files: ["src/second.ts"], + pullRequests: rebased ? [associatedPullRequest()] : [], + }, + { sha: baseline, message: "chore: release 6.7.0", files: [] }, + ], + }); + const full = []; + for await (const commit of fixture.github.mergeCommitIterator("main")) + full.push(commit); + const omitted = []; + for await (const commit of fixture.github.mergeCommitIterator("main", { + includeFiles: false, + backfillFiles: true, + })) + omitted.push(commit); + expect(full[0]?.pullRequest?.number).toBe(rebased ? 7 : 42); + expect(omitted).toEqual( + full.map(({ files: _files, pullRequest, ...commit }) => ({ + ...commit, + ...(pullRequest + ? { + pullRequest: Object.fromEntries( + Object.entries(pullRequest).filter(([key]) => key !== "files"), + ), + } + : {}), + })), + ); + expect(omitted.every((commit) => commit.files === undefined)).toBe(true); + expect( + fixture.requests.filter((request) => + request.pathname.includes("/commits/"), + ), + ).toEqual([]); + }, +); + it.each([ { name: "inherited exclusion", @@ -324,6 +444,7 @@ it.each([ request.pathname.includes("/commits/"), ), ).toHaveLength(2); + expect(fixture.pages.every((page) => page.includeFiles === true)).toBe(true); expect(candidates).toHaveLength(excluded ? 0 : 1); }); @@ -358,6 +479,131 @@ it("collects beyond 500 commits and rejects an unobserved known release boundary ).toBe(true); }); +it.each([ + { + name: "timestamped GraphQL internal error", + error: internalError, + status: 200, + }, + { + name: "GraphQL internal error without timestamp", + error: { + message: + "Something went wrong while executing your query. Please include `fixture:request` when reporting this issue.", + }, + status: 200, + }, + { name: "HTTP 502 error", error: internalError, status: 502 }, +])( + "retries the same history cursor after $name and discards partial data", + async ({ error, status }) => { + const sleep = vi + .spyOn(githubRuntime, "sleepInMs") + .mockResolvedValue(undefined); + const commits = Array.from({ length: 11 }, (_, index) => ({ + sha: index.toString(16).padStart(40, "0"), + message: `fix: unreleased change ${index}`, + files: ["src/change.ts"], + })); + commits.push({ sha: baseline, message: "chore: release 6.7.0", files: [] }); + const fixture = await releaseFixture({ + commits, + historyFailures: [{ cursor: "10", errors: [error], status }], + }); + const candidates = await fixture.manifest.buildPullRequests(); + const body = candidates[0]?.body.toString(); + expect(body?.match(/unreleased change 10/gu)).toHaveLength(1); + expect(fixture.pages.map((page) => page.cursor)).toEqual([ + undefined, + "10", + "10", + ]); + expect(fixture.pages[1]).toEqual(fixture.pages[2]); + expect(fixture.historyQueries[1]).toBe(fixture.historyQueries[2]); + expect(sleep.mock.calls).toEqual([[1000]]); + }, +); + +it("does not retry canonical internal errors outside commit-history reads", async () => { + const sleep = vi + .spyOn(githubRuntime, "sleepInMs") + .mockResolvedValue(undefined); + const fixture = await releaseFixture({ releaseFailures: [[internalError]] }); + await expect(fixture.manifest.createPullRequests()).rejects.toMatchObject({ + name: "GraphqlResponseError", + errors: [internalError], + }); + expect(fixture.pages).toEqual([]); + expect( + fixture.requests.filter((request) => request.pathname === "/graphql"), + ).toHaveLength(1); + expect(sleep).not.toHaveBeenCalled(); +}); + +it.each([ + { + name: "permission", + errors: [{ message: "Resource not accessible", type: "FORBIDDEN" }], + }, + { + name: "validation", + errors: [ + { message: "Field does not exist", type: "GRAPHQL_VALIDATION_FAILED" }, + ], + }, + { + name: "mixed", + errors: [ + internalError, + { message: "Resource not accessible", type: "FORBIDDEN" }, + ], + }, + { + name: "unrecognized internal", + errors: [{ message: "Another server failure", type: "INTERNAL" }], + }, + { name: "empty errors", errors: [] }, +])("does not retry $name GraphQL failures", async ({ errors }) => { + const sleep = vi + .spyOn(githubRuntime, "sleepInMs") + .mockResolvedValue(undefined); + const fixture = await releaseFixture({ historyFailures: [{ errors }] }); + await expect(fixture.manifest.createPullRequests()).rejects.toMatchObject({ + name: "GraphqlResponseError", + errors, + }); + expect(fixture.pages).toHaveLength(1); + expect(sleep).not.toHaveBeenCalled(); + expect( + fixture.requests.every( + (request) => request.method === "GET" || request.pathname === "/graphql", + ), + ).toBe(true); +}); + +it("exhausts bounded history retries without creating a partial proposal", async () => { + const sleep = vi + .spyOn(githubRuntime, "sleepInMs") + .mockResolvedValue(undefined); + const fixture = await releaseFixture({ + historyFailures: Array.from({ length: 6 }, () => ({ + errors: [internalError], + })), + }); + await expect(fixture.manifest.createPullRequests()).rejects.toMatchObject({ + name: "GraphqlResponseError", + errors: [internalError], + }); + expect(fixture.pages).toHaveLength(6); + expect(fixture.pages.every((page) => page.cursor === undefined)).toBe(true); + expect(sleep.mock.calls).toEqual([[1000], [2000], [4000], [8000], [16000]]); + expect( + fixture.requests.every( + (request) => request.method === "GET" || request.pathname === "/graphql", + ), + ).toBe(true); +}); + it("preserves a reached explicit last-release override", async () => { const fixture = await releaseFixture({ config: { "last-release-sha": latest },