mirror/rea
1
0
Fork 0
mirror of https://github.com/morluto/rea synced 2026-10-25 22:08:11 -04:00
Code Issues Packages Projects Releases Wiki Activity

fix(release): recover transient commit-history reads

Browse Source
This commit is contained in:
morluto
2026-10-24 16:47:56 +08:00
parent 3ce311757d
commit e3a6ce40f5
3 changed files with 429 additions and 67 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/releasing.md
+17 -7
View File
@@ -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
patches/release-please+17.3.0.patch
+110 -4
View File
@@ -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,
});
}
tests/boundary/filesystem/releaseProposal.test.ts
+302 -56
View File
@@ -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<typeof associatedPullRequest>[];
};
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<string, unknown>;
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<string, unknown> = {
@@ -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 },