From cdb225e1ab1963fa9041149a5245ea52f55f3280 Mon Sep 17 00:00:00 2001 From: Tapish Khandelwal Date: Wed, 9 Sep 2026 05:35:40 +0530 Subject: [PATCH 1/2] fix(review-gate): accept regeneration rewrites on the generated release branch Release Please updates its release pull request by force-pushing the branch, and GitHub records those rewrites with no `before_commit_id`. #318 made an untraceable rewrite fail closed, which is right for a human branch: a force-push there can replace reviewed code without a trace. A generated release branch has no such state to protect. The bot rebuilds it from the base branch on every upstream merge, so a rewrite discards only content the next run reproduces. Failing closed there makes a release pull request unmergeable as soon as anything lands on the base branch. That is not theoretical: it blocked v0.6.0 outright (#342), with every other check green, and the discontinuity was created by an unrelated merge rather than by anything done to the release branch. Timeline: v0.5.1 released 2026-08-17, #318 landed 2026-09-07, and v0.6.0 is the first release attempt since. The exemption is evidence-based rather than name-based, because a branch name is chosen by whoever pushes. Both must hold: the head branch carries release-please's generated name for *this* pull request's own base, and the pull request is authored by a bot. A human branch wearing the generated name fails the second; a bot pull request from an ordinary branch, or one naming a base it does not target, fails the first. The acceptance is logged rather than silent. An exemption nobody can see is one nobody can audit, and this one weakens a gate that was settled deliberately. Closes #342. --- scripts/publish-review-gate-check.mjs | 32 ++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/scripts/publish-review-gate-check.mjs b/scripts/publish-review-gate-check.mjs index 62515612..f44bb509 100644 --- a/scripts/publish-review-gate-check.mjs +++ b/scripts/publish-review-gate-check.mjs @@ -222,6 +222,30 @@ function untraceableRewriteError() { ); } +// Release Please regenerates its release branch by force-pushing it, and GitHub records +// those rewrites with no `before_commit_id`, so they are untraceable by the check below. +// +// The continuity check exists so a rewrite cannot replace human-reviewed code without a +// trace. A generated release branch has no such state to protect: the bot rebuilds the +// branch from the base branch on every upstream merge, so a rewrite discards only content +// the next run reproduces. Without this, a release pull request becomes unmergeable as +// soon as anything lands on the base branch, which blocked v0.6.0 entirely (#342). +// +// Deliberately narrow, and evidence-based rather than name-based. A branch name alone is +// not evidence, because anyone who can push may choose one. Both must hold: +// - the head branch is release-please's generated name for *this* pull request's base, +// - the pull request is authored by a bot. +// A human branch named to look generated fails the second condition, and a bot pull +// request from an ordinary branch fails the first. +function isGeneratedReleasePullRequest(pr) { + const baseRef = pr?.base?.ref; + const headRef = pr?.head?.ref; + if (typeof baseRef !== "string" || baseRef.length === 0) return false; + if (typeof headRef !== "string") return false; + if (String(pr?.user?.type ?? "").toLowerCase() !== "bot") return false; + return headRef.startsWith(`release-please--branches--${baseRef}--components--`); +} + function loadLatestDurableReviewState(pr) { const { priorHeads: forcePushedPriorShas, hasUntraceableRewrite } = loadForcePushedPriorShas( pr.number, @@ -230,7 +254,13 @@ function loadLatestDurableReviewState(pr) { // commit cannot contain a finding that was observed and then deleted only on the head this // rewrite discarded, so returning it would publish success while losing that ask. Continuity // across a null `before_commit_id` cannot be proved, so no reachable state is trustworthy here. - if (hasUntraceableRewrite) throw untraceableRewriteError(); + if (hasUntraceableRewrite) { + if (!isGeneratedReleasePullRequest(pr)) throw untraceableRewriteError(); + // Logged, never silent: an exemption nobody can see is one nobody can audit. + console.log( + `Accepting an untraceable rewrite on generated release branch ${pr.head.ref}: its contents are regenerated from ${pr.base.ref} rather than carried across review.`, + ); + } const currentPrShas = loadCurrentPrCommitShas(pr); const currentPrShaSet = new Set(currentPrShas); const pendingShas = [...currentPrShas, ...forcePushedPriorShas]; From 0800a5d8fcd780660ea0a633a0a416c19ed8ac79 Mon Sep 17 00:00:00 2001 From: Tapish Khandelwal Date: Wed, 9 Sep 2026 05:35:40 +0530 Subject: [PATCH 2/2] test(review-gate): prove the release-branch exemption stays narrow Fixtures now carry `head.ref`, `base.ref` and `user.type`, matching the shape release-please actually produces (confirmed against #337: `release-please--branches--master--components--pack`, `github-actions[bot]`). Four cases: the generated release pull request accepts an untraceable rewrite and publishes success while announcing the exemption; a human-authored branch wearing the generated name, a bot pull request from an ordinary branch, and a bot branch naming a base it does not target are all still refused. Mutation-tested, each caught: - bot-author requirement dropped -> human branch wearing the name is accepted - base ref ignored, bare prefix -> wrong-base branch is accepted - guard removed entirely -> 7 of 34 fail The three refusal cases are the point. An exemption is only as good as what it declines, and the acceptance case alone would pass against a guard that exempts everything. --- .../scripts/publish-review-gate-check.test.ts | 82 ++++++++++++++++++- 1 file changed, 80 insertions(+), 2 deletions(-) diff --git a/tests/scripts/publish-review-gate-check.test.ts b/tests/scripts/publish-review-gate-check.test.ts index acaf2d33..0aa7506b 100644 --- a/tests/scripts/publish-review-gate-check.test.ts +++ b/tests/scripts/publish-review-gate-check.test.ts @@ -454,6 +454,63 @@ describe("PR-head Review gate check publisher", () => { expect(publicationText).not.toContain("output[text]"); }); + // release-please regenerates its branch by force-pushing, and GitHub records those + // rewrites with no `before_commit_id`. Failing closed there makes a release pull request + // unmergeable as soon as anything lands on the base branch, which blocked v0.6.0 + // entirely (#342). The branch carries no reviewed history to protect -- the bot rebuilds + // it from the base branch -- so the rewrite is accepted for that case only. + it("accepts an untraceable rewrite on the generated release branch", () => { + const { result, calls } = runScript( + ["--reconcile-open-prs", "--max-prs", "1", "--selection-offset", "0"], + [generatedReleasePull()], + cleanReviewFixture(), + [{ status: 0 }], + cleanDurableState(), + [{ status: 0 }], + null, + [forcePushEvent(null, "2026-08-17T00:00:00Z")], + ); + const publicationText = + calls.find((call) => call.includes("repos/lamemustafa/pack/check-runs"))?.join(" ") ?? ""; + + expect(result.status).toBe(0); + expect(publicationText).toContain("conclusion=success"); + expect(publicationText).not.toContain("GitHub did not record the prior head"); + // The exemption is announced, because one nobody can see is one nobody can audit. + expect(result.stdout).toContain("Accepting an untraceable rewrite on generated release branch"); + }); + + // The exemption is evidence-based, not name-based. Each case below satisfies part of the + // shape and must still be refused, because a branch name is chosen by whoever pushes. + it.each([ + [ + "a human-authored branch wearing the generated name", + generatedReleasePull({ userType: "User" }), + ], + ["a bot pull request from an ordinary branch", pull(1, { userType: "Bot" })], + [ + "a bot branch naming a base this pull request does not target", + generatedReleasePull({ headRef: "release-please--branches--release-1.x--components--pack" }), + ], + ])("still refuses an untraceable rewrite for %s", (_label, pullRequest) => { + const { result, calls } = runScript( + ["--reconcile-open-prs", "--max-prs", "1", "--selection-offset", "0"], + [pullRequest], + cleanReviewFixture(), + [{ status: 0 }], + cleanDurableState(), + [{ status: 0 }], + null, + [forcePushEvent(null, "2026-08-17T00:00:00Z")], + ); + const publicationText = + calls.find((call) => call.includes("repos/lamemustafa/pack/check-runs"))?.join(" ") ?? ""; + + expect(result.status).toBe(0); + expect(publicationText).toContain("conclusion=action_required"); + expect(publicationText).toContain("GitHub did not record the prior head"); + }); + it("never discards an unreachable deleted finding across an untraceable rewrite", () => { const orphanedSha = "b".repeat(40); const { result, calls } = runScript( @@ -744,17 +801,38 @@ else if (text.includes("check-runs")) { function pull( number: number, - { draft = false, state = "open", headRepo = "lamemustafa/pack" } = {}, + { + draft = false, + state = "open", + headRepo = "lamemustafa/pack", + headRef = "tapish-codex/example", + baseRef = "master", + userType = "User", + } = {}, ) { const sha = number === 1 ? headSha : String(number).repeat(40); return { number, state, draft, - head: { sha, repo: { full_name: headRepo } }, + head: { sha, ref: headRef, repo: { full_name: headRepo } }, + base: { ref: baseRef }, + user: { login: userType === "Bot" ? "github-actions[bot]" : "maintainer", type: userType }, }; } +// The shape release-please actually produces, confirmed against #337: +// head.ref release-please--branches--master--components--pack +// user github-actions[bot] (type "Bot") +function generatedReleasePull(overrides: Record = {}) { + return pull(1, { + baseRef: "master", + headRef: "release-please--branches--master--components--pack", + userType: "Bot", + ...overrides, + }); +} + function cleanDurableState(prNumber = 1) { return "review-gate-state/v1\n" + JSON.stringify({ version: 1, prNumber, findings: [] }); }