diff --git a/scripts/CLAUDE.md b/scripts/CLAUDE.md index 65a3180d..ecda14e7 100644 --- a/scripts/CLAUDE.md +++ b/scripts/CLAUDE.md @@ -31,6 +31,9 @@ Verified against actual file contents and `.github/workflows/*.yml` on 2026-07-0 - `check-pr-review-gate.mjs` — polls `gh` for PR review state, optionally requiring a named reviewer/strict head review; run by review-gate.yml and locally via `pnpm review:gate`. +- `publish-review-gate-check.mjs` — publishes the `Review gate (scheduled)` check on + every open PR head, carrying durable review state forward across force-pushes + (review-gate-reconcile.yml). ### Local-only (not invoked by any workflow) @@ -45,6 +48,16 @@ Verified against actual file contents and `.github/workflows/*.yml` on 2026-07-0 ### Supporting library +- `lib/codex-review-markers.mjs` — the pattern Codex writes on a clean top-level + review, and the reviewed commit it names. Read by `check-pr-review-gate.mjs` (as a + current-head review) and `publish-review-gate-check.mjs` (as a head this PR had); one + definition because two copies disagreeing about which commits were reviewed is the + discontinuity the gate exists to detect. +- `lib/release-branch-rewrite.mjs` — the marker a generated-branch regeneration writes + to record the head it discarded, which GitHub omits (`before_commit_id` is null). + Written by `run-release-please.mjs`, read by `publish-review-gate-check.mjs`. It + supplies a name, not permission: the named commit is still searched, and only a + marker written by `github-actions[bot]` counts. - `lib/live-run-evidence-redaction-patterns.mjs` (+ `.d.mts`) and `lib/live-run-evidence-redaction.ts` / `live-run-evidence-types.ts` / `live-run-evidence.ts` — shared redaction pattern definitions and types used by the live-evidence and diff --git a/scripts/lib/release-branch-rewrite.mjs b/scripts/lib/release-branch-rewrite.mjs new file mode 100644 index 00000000..279e7195 --- /dev/null +++ b/scripts/lib/release-branch-rewrite.mjs @@ -0,0 +1,69 @@ +// The record a generated-branch rewrite leaves behind so review continuity survives it. +// +// GitHub omits `before_commit_id` on the force-pushes Release Please performs, so the head a +// regeneration discards is named nowhere in the pull request's record: an ordinary push that was +// later rewritten away leaves no timeline entry, and a head that was never reviewed is named by no +// review either. The review gate therefore cannot prove that a finding recorded only on that head +// was carried forward, and refuses. +// +// The workflow that performs the rewrite is ours, so it can record what GitHub does not. This is +// the marker it writes, and the reader both sides share. +// +// It supplies evidence; it does not waive anything. A marker names a commit, and the gate still +// searches that commit for state belonging to that pull request. A marker naming a head with no +// state leaves the gate exactly as unconvinced as it was before. +// +// A record is written in two stages because the rewrite is not atomic with the recording of it. +// The `before` head is written first, while it is still the branch head; the `after` head is added +// once the rewrite has produced one. A record stopped in between names a discarded head and no +// replacement. The gate ignores it until a successful compare-and-swap records the created head; +// timeline events alone cannot safely attribute a later head to this marker. +const MARKER_PATTERN = + //iu; + +const SHA_PATTERN = /^[0-9a-f]{40}$/iu; + +export function formatReleaseBranchRewriteMarker({ branch, before, after = null }) { + if (!branch || /\s/u.test(branch)) { + throw new Error("A release branch rewrite marker needs a whitespace-free branch name."); + } + if (!SHA_PATTERN.test(before ?? "")) { + throw new Error("A release branch rewrite marker needs a full discarded-head SHA."); + } + if (after !== null && !SHA_PATTERN.test(after)) { + throw new Error("A release branch rewrite marker needs a full created-head SHA or none."); + } + const suffix = after === null ? "" : ` after=${after.toLowerCase()}`; + return ``; +} + +// Only the workflow that performs a rewrite can attest to what it discarded. Anyone who can +// comment can write the marker text, so the author is the whole of its authority: no human can post +// under this login. It lives beside the parser because a parser that cannot check authorship and an +// authority check kept somewhere else is how one of the two readers came to skip it. +const TRUSTED_REWRITE_RECORDER = "github-actions"; + +export function normaliseGithubLogin(login) { + return String(login ?? "") + .toLowerCase() + .replace(/\[bot\]$/u, ""); +} + +// A marker is evidence only from this author. Both readers -- the workflow that writes records and +// the gate that reads them -- ask this same question of a comment before trusting its marker. +export function isTrustedRewriteRecord(comment) { + return normaliseGithubLogin(comment?.user?.login) === TRUSTED_REWRITE_RECORDER; +} + +// Returns `{ branch, before, after }` for a comment that carries the marker, or null. `after` is +// null for a record whose rewrite had not produced a head yet. This reads the text only: pair it +// with `isTrustedRewriteRecord` before treating a marker as evidence or as recoverable state. +export function readReleaseBranchRewriteMarker(body) { + const match = MARKER_PATTERN.exec(body ?? ""); + if (!match) return null; + return { + branch: match[1], + before: match[2].toLowerCase(), + after: match[3] ? match[3].toLowerCase() : null, + }; +} diff --git a/scripts/publish-review-gate-check.mjs b/scripts/publish-review-gate-check.mjs index 6a8fe110..2c68651c 100644 --- a/scripts/publish-review-gate-check.mjs +++ b/scripts/publish-review-gate-check.mjs @@ -11,6 +11,11 @@ import { runGhText, } from "./lib/github-cli-retry.mjs"; import { readCleanTopLevelReviewCommit } from "./lib/codex-review-markers.mjs"; +import { + isTrustedRewriteRecord, + normaliseGithubLogin, + readReleaseBranchRewriteMarker, +} from "./lib/release-branch-rewrite.mjs"; const CHECK_RUN_NAME = "Review gate (scheduled)"; const REQUIRED_REVIEW_AUTHOR = "chatgpt-codex-connector"; @@ -232,11 +237,14 @@ function untraceableRewriteError() { } function loadLatestDurableReviewState(pr) { + // Read once and shared: both the rewrite records and the clean top-level review markers live in + // this one list, and fetching it twice was a duplicate of a fact the pull request states once. + const comments = loadIssueComments(pr.number); const { priorHeads: forcePushedPriorShas, hasUntraceableRewrite, unidentifiedDiscardAt, - } = loadForcePushedPriorShas(pr.number); + } = loadForcePushedPriorShas(pr.number, comments); // Reject before consulting any reachable state, not after. A state surviving on a current-line // 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 @@ -260,7 +268,7 @@ function loadLatestDurableReviewState(pr) { const priorHeads = dedupePriorHeadShas( forcePushedPriorShas, loadReviewedHeadShas(pr.number), - loadTopLevelReviewedHeadShas(pr.number, currentPrShas, forcePushedPriorShas), + loadTopLevelReviewedHeadShas(comments, currentPrShas, forcePushedPriorShas), ); const discardedLineShas = loadDiscardedLineShas(pr, priorHeads, new Set(currentPrShas)); @@ -345,17 +353,11 @@ function loadReviewedHeadShas(prNumber) { // it names was therefore a head of this pull request, so it belongs among the candidate heads -- // otherwise a head reviewed clean and then rewritten away is named in the record but never // searched. -function loadTopLevelReviewedHeadShas(prNumber, currentPrShas, forcePushedPriorHeads) { - const commentPages = JSON.parse( - runGithub( - ["api", "--paginate", "--slurp", `repos/${repo}/issues/${prNumber}/comments?per_page=100`], - "reviewed head marker discovery", - ), - ); +function loadTopLevelReviewedHeadShas(comments, currentPrShas, forcePushedPriorHeads) { const known = [...currentPrShas, ...forcePushedPriorHeads.map((head) => head.sha)]; const prefixes = new Set(); - for (const comment of flattenPages(commentPages)) { - if (normaliseLogin(comment?.user?.login) !== REQUIRED_REVIEW_AUTHOR) continue; + for (const comment of comments) { + if (normaliseGithubLogin(comment?.user?.login) !== REQUIRED_REVIEW_AUTHOR) continue; const prefix = readCleanTopLevelReviewCommit(comment.body); // A marker naming a head already in hand needs no lookup, and the common case is the current // head naming itself. @@ -384,12 +386,6 @@ function resolveCommitSha(prefix) { return commit.sha; } -function normaliseLogin(login) { - return String(login ?? "") - .toLowerCase() - .replace(/\[bot\]$/u, ""); -} - function cleanReviewState(prNumber) { return JSON.stringify({ version: 1, prNumber, findings: [] }); } @@ -560,7 +556,45 @@ function durableReviewStateBelongsToPr(state, expectedPrNumber) { return parsed?.version === 1 && parsed.prNumber === expectedPrNumber; } -function loadForcePushedPriorShas(prNumber) { +// Discarded heads a rewrite recorded out-of-band, keyed by the head it created. GitHub omits +// `before_commit_id` for a generated-branch regeneration, and the workflow performing it records +// the pair instead (#350). This supplies the missing name; it does not waive anything, because a +// head named this way is still searched for state belonging to this pull request. +function loadIssueComments(prNumber) { + return flattenPages( + JSON.parse( + runGithub( + ["api", "--paginate", "--slurp", `repos/${repo}/issues/${prNumber}/comments?per_page=100`], + "pull request comment discovery", + ), + ), + ); +} + +function loadRecordedRewriteDiscards(comments) { + const discards = new Map(); + for (const comment of comments) { + if (!isTrustedRewriteRecord(comment)) continue; + const marker = readReleaseBranchRewriteMarker(comment.body); + // An open record names a discarded head and no replacement, so it pairs with no rewrite and + // identifies nothing. A record whose heads match says the branch did not move, which is a + // recorded non-event rather than a discard. + if (!marker || marker.after === null || marker.after === marker.before) continue; + // A second marker for the same created head is two different claims about one rewrite, and + // there is no basis for preferring either. + if (discards.has(marker.after) && discards.get(marker.after) !== marker.before) { + throw durableStateRejection( + "a rewrite was recorded twice with different discarded heads, so which commit it discarded is not settled", + "conflicting recorded rewrite discards for one created head", + ); + } + discards.set(marker.after, marker.before); + } + return discards; +} + +function loadForcePushedPriorShas(prNumber, comments) { + const recordedDiscards = loadRecordedRewriteDiscards(comments); const timelinePages = JSON.parse( runGithub( [ @@ -607,7 +641,9 @@ function loadForcePushedPriorShas(prNumber) { // leaves no timeline entry at all. Recording when that happened is what lets state older than // it be recognised as possibly superseded. if (!/^[0-9a-f]{40}$/iu.test(event.before_commit_id ?? "")) { - unidentifiedDiscardAt = Math.max(unidentifiedDiscardAt ?? -Infinity, createdAt); + const recorded = recordedDiscards.get(String(event.commit_id ?? "").toLowerCase()); + if (recorded) shas.push(recorded); + else unidentifiedDiscardAt = Math.max(unidentifiedDiscardAt ?? -Infinity, createdAt); } rewrites.push({ createdAt, shas }); } diff --git a/scripts/run-release-please.d.mts b/scripts/run-release-please.d.mts index c296adf8..a2db5a6a 100644 --- a/scripts/run-release-please.d.mts +++ b/scripts/run-release-please.d.mts @@ -10,3 +10,54 @@ export function resolveReleaseTargetBranch( ): string; export function serializeGitHubOutput(outputs: Record): string; + +interface ReleaseBranchScope { + env: NodeJS.ProcessEnv | Record; + owner: string; + repo: string; + targetBranch: string; +} + +/** The heads and durable marker identities opened for this regeneration. */ +interface OpenedRewriteRecords extends ReleaseBranchScope { + headsBeforeRegeneration: Map< + string, + { head: string; recordId: number; pullRequestNumber: number } + >; +} + +export function openBranchRewriteRecords( + options: ReleaseBranchScope, +): Promise>; + +/** `false` when it could not confirm what the regeneration is about to discard. */ +export function refreshBranchRewriteRecords(options: OpenedRewriteRecords): Promise; + +export function closeBranchRewriteRecords(options: { + env: ReleaseBranchScope["env"]; + owner: string; + repo: string; + confirmedRewrites: Map< + string, + { record: { id: number; marker: { branch: string; before: string } }; after: string } + >; +}): Promise; + +export function withReleaseBranchRewriteCas( + github: { + repository: { owner: string; repo: string }; + graphql: (query: string, variables: Record) => Promise; + octokit: { + git: { + createRef: (request: Record) => Promise; + updateRef: (request: Record) => Promise; + }; + }; + }, + expected: OpenedRewriteRecords["headsBeforeRegeneration"], + confirmedRewrites: Map< + string, + { record: { id: number; marker: { branch: string; before: string } }; after: string } + >, + operation: () => Promise, +): Promise; diff --git a/scripts/run-release-please.mjs b/scripts/run-release-please.mjs index 4be4b26c..1ce3d96a 100644 --- a/scripts/run-release-please.mjs +++ b/scripts/run-release-please.mjs @@ -1,4 +1,9 @@ import { appendFile } from "node:fs/promises"; +import { + formatReleaseBranchRewriteMarker, + isTrustedRewriteRecord, + readReleaseBranchRewriteMarker, +} from "./lib/release-branch-rewrite.mjs"; import { createRequire } from "node:module"; import { pathToFileURL } from "node:url"; import { randomUUID } from "node:crypto"; @@ -40,16 +45,63 @@ export async function runReleasePlease(env = process.env) { configFile, manifestFile, ); + // Before `createReleases()`, because everything here can fail and nothing here is reversible + // once a GitHub release exists. A failure at this point costs a re-run; the same failure after a + // release is published leaves that release without its assets. + const headsBeforeRegeneration = await openBranchRewriteRecords({ + env, + owner, + repo, + targetBranch, + }); + const releases = (await releaseManifest.createReleases()).filter(Boolean); const outputs = buildReleaseOutputs(releases); - - const pullRequestManifest = await Manifest.fromManifest( - github, - targetBranch, - configFile, - manifestFile, - ); - const pullRequests = (await pullRequestManifest.createPullRequests()).filter(Boolean); + let pullRequests = []; + const confirmedRewrites = new Map(); + let regenerationError = null; + try { + const pullRequestManifest = await Manifest.fromManifest( + github, + targetBranch, + configFile, + manifestFile, + ); + // Immediately before the force-push, not at the snapshot above: see + // `refreshBranchRewriteRecords` for why the gap between the two is the dangerous part. + const recordsNameTheCurrentHeads = await refreshBranchRewriteRecords({ + env, + owner, + repo, + targetBranch, + headsBeforeRegeneration, + }); + if (!recordsNameTheCurrentHeads) { + throw new Error("Could not confirm release branch rewrite records for regeneration."); + } + pullRequests = await withReleaseBranchRewriteCas( + github, + headsBeforeRegeneration, + confirmedRewrites, + async () => (await pullRequestManifest.createPullRequests()).filter(Boolean), + ); + } catch (error) { + regenerationError = error; + } + const closeErrors = await closeBranchRewriteRecords({ env, owner, repo, confirmedRewrites }); + if (releases.length === 0 && (regenerationError || closeErrors.length > 0)) { + const errors = regenerationError ? [regenerationError, ...closeErrors] : closeErrors; + const reasons = errors.map((error) => error?.message ?? String(error)).join("; "); + throw new AggregateError( + errors, + `Could not complete release pull request regeneration: ${reasons}`, + ); + } + if (regenerationError) { + console.error( + `Could not regenerate release pull requests: ${regenerationError.message}. Existing release outputs remain available for asset publication.`, + ); + } outputs.prs_created = String(pullRequests.length > 0); if (pullRequests.length > 0) { outputs.pr = JSON.stringify(pullRequests[0]); @@ -67,6 +119,390 @@ export async function runReleasePlease(env = process.env) { return outputs; } +// Release Please regenerates by force-pushing, and GitHub records no `before_commit_id` for it, so +// the discarded head is unnameable from the pull request's record afterwards. Recording it here -- +// from the only place that still knows it -- is what lets the review gate establish continuity +// across a regeneration instead of refusing every release pull request (#342, #350). +// +// Opening the record before the rewrite rather than writing it afterwards is what makes an +// interrupted run diagnosable: a cancellation leaves the discarded SHA recorded, but a later run +// must hold if it cannot prove the rewrite's created head from its own CAS receipt. +export async function openBranchRewriteRecords({ env, owner, repo, targetBranch }) { + const heads = await readReleaseBranchHeads({ env, owner, repo, targetBranch }); + const opened = new Map(); + for (const [branch, head] of heads) { + const pullRequestNumber = await findOpenPullRequestNumber({ + env, + owner, + repo, + branch, + targetBranch, + }); + if (pullRequestNumber === null) + throw new Error( + `Retained release branch ${branch} has no open release pull request for a durable rewrite record; regeneration remains held.`, + ); + const records = await readBranchRewriteRecords({ env, owner, repo, pullRequestNumber }); + const open = records.find( + (record) => record.marker.branch === branch && record.marker.after === null, + ); + if (open) { + if (head === open.marker.before) { + opened.set(branch, { head, recordId: open.id, pullRequestNumber }); + continue; + } + throw new Error( + "An interrupted rewrite record no longer matches its branch head; regeneration remains held.", + ); + } + const created = await githubRequest( + env, + `/repos/${owner}/${repo}/issues/${pullRequestNumber}/comments`, + { + method: "POST", + body: JSON.stringify({ + body: formatReleaseBranchRewriteMarker({ branch, before: head }), + }), + }, + ); + if (!Number.isInteger(created?.id)) { + throw new Error( + "GitHub did not return an identifier for the opened release branch rewrite record.", + ); + } + opened.set(branch, { head, recordId: created.id, pullRequestNumber }); + console.log(`Opened release branch rewrite record on #${pullRequestNumber}: before=${head}`); + } + return opened; +} + +// Every confirmed receipt is attempted even if another close fails. The caller decides whether +// those named failures remain terminal: no-release runs hold, while already-created releases keep +// their asset-publication outputs. +export async function closeBranchRewriteRecords({ env, owner, repo, confirmedRewrites }) { + const errors = []; + for (const receipt of confirmedRewrites.values()) { + try { + await writeBranchRewriteRecord({ + env, + owner, + repo, + record: receipt.record, + after: receipt.after, + }); + } catch (error) { + errors.push(error); + console.error( + `Could not close a release branch rewrite record: ${error.message}. The record remains held for review.`, + ); + } + } + return errors; +} + +/** + * Confirms every open record still names the head its branch carries. `false` means the + * regeneration must not proceed. + * + * The record is opened before `createReleases()` so that a failure there costs a re-run rather + * than a published release with no assets. A changed head is not refreshed: replacing `before` + * would erase the only durable name of the earlier head. The later compare-and-swap makes the + * remaining update atomic, so this path only admits an exact snapshot. + * + * Throwing is not available here -- the release already exists and a later workflow step uploads + * its assets -- so the failure is reported instead and the caller skips the rewrite. Discarding a + * head no record names is unrecoverable and is the whole point of this mechanism; a release whose + * pull request waits for the next run is not. + */ +export async function refreshBranchRewriteRecords({ + env, + owner, + repo, + targetBranch, + headsBeforeRegeneration, +}) { + try { + await eachOpenBranchRewriteRecord( + { env, owner, repo, targetBranch, expected: headsBeforeRegeneration }, + async ({ record, head }) => { + if (head === record.marker.before) return; + throw new Error( + `Release branch ${record.marker.branch} advanced after its rewrite record was opened; its original discarded head remains recorded and this regeneration will retry.`, + ); + }, + ); + return true; + } catch (error) { + console.error( + `Could not confirm what the regeneration is about to discard: ${error.message}. Skipping the release pull request so nothing is discarded unrecorded; the next run retries.`, + ); + return false; + } +} + +// Release Please delegates generated-branch updates to an unconditional REST force-push. That +// leaves a race after the marker is refreshed: a commit that arrives before the push is discarded +// even though the marker names the earlier head. GitHub's `updateRefs` mutation accepts `beforeOid`, +// so replace recorded rewrites and the first update after a confirmed generated-branch creation +// with a compare-and-swap. A mismatch rejects the release run while an open marker remains +// durable for recovery. +export async function withReleaseBranchRewriteCas(github, expected, confirmedRewrites, operation) { + const updates = github.octokit?.git?.updateRef; + const creates = github.octokit?.git?.createRef; + if ( + typeof updates !== "function" || + typeof creates !== "function" || + typeof github.graphql !== "function" + ) { + throw new Error("Release Please did not expose the GitHub clients needed for rewrite CAS."); + } + let repositoryId = null; + const createdBranches = new Map(); + github.octokit.git.createRef = async (request) => { + const branch = String(request?.ref ?? "").replace(/^refs\/heads\//u, ""); + if (!branch.startsWith("release-please--branches--")) { + return creates.call(github.octokit.git, request); + } + const result = await creates.call(github.octokit.git, request); + const head = String(result?.data?.object?.sha ?? "").toLowerCase(); + if (!/^[0-9a-f]{40}$/u.test(head)) { + throw new Error(`GitHub did not confirm the initial head for generated branch ${branch}.`); + } + createdBranches.set(branch, head); + return result; + }; + github.octokit.git.updateRef = async (request) => { + const branch = String(request?.ref ?? "").replace(/^heads\//u, ""); + if (!branch.startsWith("release-please--branches--")) { + return updates.call(github.octokit.git, request); + } + const record = expected.get(branch); + const createdHead = createdBranches.get(branch); + if (!record && !createdHead) { + throw new Error(`Release Please tried to rewrite unrecorded generated branch ${branch}.`); + } + const afterOid = String(request?.sha ?? "").toLowerCase(); + if (!/^[0-9a-f]{40}$/u.test(afterOid)) { + throw new Error(`Release Please gave rewrite CAS an invalid destination for ${branch}.`); + } + if (repositoryId === null) { + const repository = await github.graphql( + "query ReleaseBranchRewriteRepository($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { id } }", + { owner: github.repository.owner, name: github.repository.repo }, + ); + repositoryId = repository?.repository?.id; + if (typeof repositoryId !== "string" || !repositoryId) { + throw new Error("GitHub did not return a repository id for release branch rewrite CAS."); + } + } + const result = await github.graphql( + "mutation ReleaseBranchRewriteCas($repositoryId: ID!, $refUpdates: [RefUpdate!]!) { updateRefs(input: { repositoryId: $repositoryId, refUpdates: $refUpdates }) { clientMutationId } }", + { + repositoryId, + refUpdates: [ + { + name: `refs/heads/${branch}`, + beforeOid: record?.head ?? createdHead, + afterOid, + force: true, + }, + ], + }, + ); + if (!result?.updateRefs) { + throw new Error(`GitHub did not confirm the compare-and-swap update for ${branch}.`); + } + if (record) { + confirmedRewrites.set(branch, { + record: { id: record.recordId, marker: { branch, before: record.head } }, + after: afterOid, + }); + } + return { data: { object: { sha: afterOid } } }; + }; + try { + return await operation(); + } finally { + github.octokit.git.updateRef = updates; + github.octokit.git.createRef = creates; + } +} + +/** + * The walk the refresh and the close share: each branch's open record, paired with the head its + * branch carries right now. They differ only in what they write to it. + */ +async function eachOpenBranchRewriteRecord({ env, owner, repo, targetBranch, expected }, visit) { + const heads = await readReleaseBranchHeads({ env, owner, repo, targetBranch }); + for (const [branch, expectedRecord] of expected) { + const head = heads.get(branch); + if (!head) + throw new Error(`Release branch ${branch} disappeared after its rewrite record was opened.`); + const pullRequestNumber = await findOpenPullRequestNumber({ + env, + owner, + repo, + branch, + targetBranch, + }); + if (pullRequestNumber === null || pullRequestNumber !== expectedRecord.pullRequestNumber) { + throw new Error( + `Release pull request for ${branch} changed after its rewrite record was opened.`, + ); + } + const records = await readBranchRewriteRecords({ env, owner, repo, pullRequestNumber }); + const open = records.find( + (record) => + record.id === expectedRecord.recordId && + record.marker.branch === branch && + record.marker.after === null, + ); + if (!open) { + throw new Error( + `Release branch ${branch} lost the rewrite record opened for this regeneration.`, + ); + } + await visit({ pullRequestNumber, record: open, head }); + } +} + +async function writeBranchRewriteRecord({ + env, + owner, + repo, + record, + before = record.marker.before, + after = null, +}) { + await githubRequest(env, `/repos/${owner}/${repo}/issues/comments/${record.id}`, { + method: "PATCH", + body: JSON.stringify({ + body: formatReleaseBranchRewriteMarker({ branch: record.marker.branch, before, after }), + }), + }); +} + +async function readBranchRewriteRecords({ env, owner, repo, pullRequestNumber }) { + const comments = await githubList( + env, + `/repos/${owner}/${repo}/issues/${pullRequestNumber}/comments`, + "comment list for a release pull request", + ); + const records = []; + for (const comment of comments) { + // The author is the whole of a marker's authority. Without this, anyone who can comment on a + // release pull request could have their comment treated as this workflow's own open record -- + // and completing a record rewrites the comment in place, so an unrelated comment would be + // overwritten, or an uneditable one would abort the run before any release work began. + if (!isTrustedRewriteRecord(comment)) continue; + const marker = readReleaseBranchRewriteMarker(comment?.body); + const createdAt = Date.parse(comment?.created_at ?? ""); + if (marker && Number.isInteger(comment?.id)) { + records.push({ + id: comment.id, + marker, + createdAt: Number.isFinite(createdAt) ? createdAt : 0, + }); + } + } + return records; +} + +async function findOpenPullRequestNumber({ env, owner, repo, branch, targetBranch }) { + // Head *and* base. A generated branch can carry open pull requests against more than one base, + // and only the one targeting this run's branch is the release pull request being regenerated. + // Matched on head alone, the record could be opened and closed on a different pull request + // while the force-push rewrote this one -- leaving the rewrite that mattered unrecorded. + const pulls = await githubList( + env, + `/repos/${owner}/${repo}/pulls?state=open&head=${owner}:${branch}&base=${targetBranch}`, + "pull request list for a release branch", + ); + // An empty list means no open pull request; an entry that cannot be read means the answer is + // unknown, and the two must not collapse into one. Read as "none", an unreadable entry makes the + // caller skip opening a record while the regeneration force-pushes the branch anyway -- which + // discards a head nothing named, the exact loss this mechanism exists to prevent. + if (pulls.length > 0 && !pulls.some((pull) => Number.isInteger(pull?.number))) { + throw new Error("GitHub returned a malformed pull request list for a release branch."); + } + return pulls.find((pull) => Number.isInteger(pull?.number))?.number ?? null; +} + +async function readReleaseBranchHeads({ env, owner, repo, targetBranch }) { + const prefix = `heads/release-please--branches--${targetBranch}--`; + // An indeterminate response is not an empty branch list. Reading it as one lets a rewrite proceed + // with no record of what it discarded, which is the state this whole mechanism exists to prevent + // and which nothing downstream could detect. An entry this function cannot read is the same + // claim as a response it cannot read, so it is the same refusal: skipping the entry would make + // its branch look absent, which is the reading being guarded against. + const refs = await githubList( + env, + `/repos/${owner}/${repo}/git/matching-refs/${prefix}`, + "release branch head list", + ); + const heads = new Map(); + for (const ref of refs) { + const branch = String(ref?.ref ?? "").replace(/^refs\/heads\//u, ""); + const sha = ref?.object?.sha; + if (!branch || !/^[0-9a-f]{40}$/iu.test(sha ?? "")) { + throw new Error("GitHub returned a malformed release branch head list."); + } + heads.set(branch, sha.toLowerCase()); + } + return heads; +} + +const GITHUB_PAGE_SIZE = 100; +const GITHUB_MAX_PAGES = 20; + +/** + * Every page of a GitHub list endpoint, or a throw. + * + * Three call sites read lists this script's correctness depends on, and each had its own + * single-page request and its own array check. A first page is not a list: a release pull request + * outlives a hundred comments, and the marker this mechanism has just written is the newest one, so + * it is precisely what a first-page read drops. Running out of pages throws rather than returning + * what was collected, because a short answer here is indistinguishable from "no such branch" and + * that reading is what force-pushes a branch with no record of the head it discarded. + */ +async function githubList(env, path, description) { + const items = []; + for (let page = 1; page <= GITHUB_MAX_PAGES; page += 1) { + const separator = path.includes("?") ? "&" : "?"; + const batch = await githubRequest( + env, + `${path}${separator}per_page=${GITHUB_PAGE_SIZE}&page=${page}`, + ); + if (!Array.isArray(batch)) { + throw new Error(`GitHub returned a malformed ${description}.`); + } + items.push(...batch); + if (batch.length < GITHUB_PAGE_SIZE) return items; + } + throw new Error(`GitHub returned more pages of ${description} than this script will read.`); +} + +async function githubRequest(env, path, init = {}) { + const token = env.RELEASE_PLEASE_TOKEN || env.GITHUB_TOKEN || env.GH_TOKEN; + const apiUrl = env.GITHUB_API_URL || DEFAULT_GITHUB_API_URL; + const response = await globalThis.fetch(`${apiUrl}${path}`, { + ...init, + headers: { + accept: "application/vnd.github+json", + authorization: `Bearer ${token}`, + "x-github-api-version": "2022-11-28", + ...(init.body ? { "content-type": "application/json" } : {}), + ...init.headers, + }, + }); + if (!response.ok) { + // Named rather than swallowed: a rewrite that was performed but not recorded is the case the + // review gate cannot tell apart from one that never happened. + throw new Error(`GitHub request failed: ${init.method ?? "GET"} ${path} -> ${response.status}`); + } + return response.status === 204 ? null : response.json(); +} + export function buildReleaseOutputs(releases) { const createdReleases = releases.filter(Boolean); const outputs = { diff --git a/tests/scripts/publish-review-gate-check.test.ts b/tests/scripts/publish-review-gate-check.test.ts index 8f154b53..70d08c18 100644 --- a/tests/scripts/publish-review-gate-check.test.ts +++ b/tests/scripts/publish-review-gate-check.test.ts @@ -908,6 +908,160 @@ describe("PR-head Review gate check publisher", () => { expect(publicationText).not.toContain("could not retrieve durable review state"); }); + // The workflow that rewrites a generated branch records the head it discarded, because GitHub + // does not. That turns an unnamed discard into a named one, so state older than the rewrite is + // usable again -- and the named head is still searched rather than trusted. + it("accepts older state when the rewrite recorded the head it discarded", () => { + const createdSha = "b".repeat(40); + const discardedSha = "c".repeat(40); + const { result, calls } = runScript( + ["--reconcile-open-prs", "--max-prs", "1", "--selection-offset", "0"], + [pull(1)], + cleanReviewFixture(), + [{ status: 0 }], + null, + [{ status: 0 }], + { [discardedSha]: reviewStateWithDeletedFinding(1, "state-on-recorded-discard") }, + [forcePushEvent(null, "2026-08-17T12:00:00Z", createdSha)], + {}, + 0, + null, + {}, + {}, + {}, + { [discardedSha]: "2026-08-17T11:00:00Z" }, + [ + { + user: { login: "github-actions[bot]" }, + body: ``, + }, + ], + ); + const publicationText = + calls.find((call) => call.includes("repos/lamemustafa/pack/check-runs"))?.join(" ") ?? ""; + + expect(result.status).toBe(0); + expect( + calls.some((call) => call.join(" ").includes(`commits/${discardedSha}/check-runs?`)), + ).toBe(true); + expect(publicationText).toContain("state-on-recorded-discard"); + }); + + // An interrupted run leaves a record naming a discarded head and no replacement. It pairs with + // no rewrite, so it identifies nothing and must not be read as if it did. + it("ignores a rewrite record whose rewrite never produced a head", () => { + const createdSha = "b".repeat(40); + const discardedSha = "c".repeat(40); + const { result, calls } = runScript( + ["--reconcile-open-prs", "--max-prs", "1", "--selection-offset", "0"], + [pull(1)], + cleanReviewFixture(), + [{ status: 0 }], + null, + [{ status: 0 }], + { [discardedSha]: reviewStateWithDeletedFinding(1, "state-behind-open-record") }, + [forcePushEvent(null, "2026-08-17T12:00:00Z", createdSha)], + {}, + 0, + null, + {}, + {}, + {}, + { [discardedSha]: "2026-08-17T11:00:00Z" }, + [ + { + user: { login: "github-actions[bot]" }, + body: ``, + }, + ], + ); + 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).not.toContain("state-behind-open-record"); + }); + + // Anyone who can comment can write the marker text, so the author is the whole of its authority. + it("ignores a rewrite record that a trusted workflow did not write", () => { + const createdSha = "b".repeat(40); + const discardedSha = "c".repeat(40); + const { result, calls } = runScript( + ["--reconcile-open-prs", "--max-prs", "1", "--selection-offset", "0"], + [pull(1)], + cleanReviewFixture(), + [{ status: 0 }], + null, + [{ status: 0 }], + { [discardedSha]: reviewStateWithDeletedFinding(1, "state-behind-untrusted-marker") }, + [forcePushEvent(null, "2026-08-17T12:00:00Z", createdSha)], + {}, + 0, + null, + {}, + {}, + {}, + { [discardedSha]: "2026-08-17T11:00:00Z" }, + [ + { + user: { login: "someone-else" }, + body: ``, + }, + ], + ); + const publicationText = + calls.find((call) => call.includes("repos/lamemustafa/pack/check-runs"))?.join(" ") ?? ""; + + expect(result.status).toBe(0); + expect(publicationText).toContain("conclusion=action_required"); + // The head the untrusted marker names is never made a candidate, so its state is neither + // searched nor published. The refusal that fires is the no-reachable-state one rather than + // the unnamed-discard one, and either way nothing is accepted on that marker's word. + expect( + calls.some((call) => call.join(" ").includes(`commits/${discardedSha}/check-runs?`)), + ).toBe(false); + expect(publicationText).not.toContain("state-behind-untrusted-marker"); + }); + + it("refuses when one rewrite was recorded twice with different discarded heads", () => { + const createdSha = "b".repeat(40); + const discardedSha = "c".repeat(40); + const otherSha = "d".repeat(40); + const { result, calls } = runScript( + ["--reconcile-open-prs", "--max-prs", "1", "--selection-offset", "0"], + [pull(1)], + cleanReviewFixture(), + [{ status: 0 }], + null, + [{ status: 0 }], + {}, + [forcePushEvent(null, "2026-08-17T12:00:00Z", createdSha)], + {}, + 0, + null, + {}, + {}, + {}, + {}, + [ + { + user: { login: "github-actions[bot]" }, + body: ``, + }, + { + user: { login: "github-actions[bot]" }, + body: ``, + }, + ], + ); + const publicationText = + calls.find((call) => call.includes("repos/lamemustafa/pack/check-runs"))?.join(" ") ?? ""; + + expect(result.status).toBe(0); + expect(publicationText).toContain("recorded twice with different discarded heads"); + }); + it("publishes a durable-state workspace failure without its local path", () => { const directory = mkdtempSync(path.join(tmpdir(), "pack-review-gate-private-path-")); const privatePath = path.join(directory, "not-a-directory"); diff --git a/tests/scripts/run-release-please.test.ts b/tests/scripts/run-release-please.test.ts index f55d5753..e896cf3c 100644 --- a/tests/scripts/run-release-please.test.ts +++ b/tests/scripts/run-release-please.test.ts @@ -1,18 +1,224 @@ import { readFile } from "node:fs/promises"; import { createRequire } from "node:module"; -import { describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { buildReleaseOutputs, resolveReleaseTargetBranch, runReleasePlease, serializeGitHubOutput, + withReleaseBranchRewriteCas, } from "../../scripts/run-release-please.mjs"; const require = createRequire(import.meta.url); const releasePlease = require("release-please"); +// A marker is evidence only from the workflow that wrote it, so every fixture comment carrying one +// must say who posted it. Anyone can write the text. +const RECORDER = "github-actions[bot]"; + describe("Release Please workflow wrapper", () => { + it("uses beforeOid CAS only for a recorded generated branch", async () => { + const branch = "release-please--branches--master--components--pack"; + const before = "b".repeat(40); + const after = "c".repeat(40); + const originalCreate = vi.fn(); + const originalUpdate = vi.fn().mockResolvedValue({ data: { object: { sha: after } } }); + const graphql = vi + .fn() + .mockResolvedValueOnce({ repository: { id: "repo-id" } }) + .mockResolvedValueOnce({ updateRefs: { clientMutationId: null } }); + const github = { + repository: { owner: "lamemustafa", repo: "pack" }, + graphql, + octokit: { git: { createRef: originalCreate, updateRef: originalUpdate } }, + }; + + await withReleaseBranchRewriteCas( + github, + new Map([[branch, { head: before, recordId: 1, pullRequestNumber: 2 }]]), + new Map(), + async () => github.octokit.git.updateRef({ ref: `heads/${branch}`, sha: after, force: true }), + ); + + expect(originalUpdate).not.toHaveBeenCalled(); + expect(graphql).toHaveBeenLastCalledWith( + expect.stringContaining("updateRefs"), + expect.objectContaining({ + repositoryId: "repo-id", + refUpdates: [ + expect.objectContaining({ + name: `refs/heads/${branch}`, + beforeOid: before, + afterOid: after, + force: true, + }), + ], + }), + ); + expect(github.octokit.git.updateRef).toBe(originalUpdate); + expect(github.octokit.git.createRef).toBe(originalCreate); + }); + + it("rejects a raced generated-branch update and leaves the caller's record intact", async () => { + const branch = "release-please--branches--master--components--pack"; + const before = "b".repeat(40); + const after = "c".repeat(40); + const originalCreate = vi.fn(); + const originalUpdate = vi.fn(); + const graphql = vi + .fn() + .mockResolvedValueOnce({ repository: { id: "repo-id" } }) + .mockRejectedValueOnce(new Error("Reference update failed: beforeOid does not match")); + const github = { + repository: { owner: "lamemustafa", repo: "pack" }, + graphql, + octokit: { git: { createRef: originalCreate, updateRef: originalUpdate } }, + }; + const records = new Map([[branch, { head: before, recordId: 1, pullRequestNumber: 2 }]]); + + await expect( + withReleaseBranchRewriteCas(github, records, new Map(), async () => + github.octokit.git.updateRef({ ref: `heads/${branch}`, sha: after, force: true }), + ), + ).rejects.toThrow(/beforeOid does not match/iu); + + expect(originalUpdate).not.toHaveBeenCalled(); + expect(records.get(branch)).toMatchObject({ head: before, recordId: 1 }); + expect(github.octokit.git.updateRef).toBe(originalUpdate); + expect(github.octokit.git.createRef).toBe(originalCreate); + }); + + it("refuses an unrecorded generated-branch force update", async () => { + const branch = "release-please--branches--master--components--pack"; + const originalCreate = vi.fn(); + const originalUpdate = vi.fn(); + const github = { + repository: { owner: "lamemustafa", repo: "pack" }, + graphql: vi.fn(), + octokit: { git: { createRef: originalCreate, updateRef: originalUpdate } }, + }; + + await expect( + withReleaseBranchRewriteCas(github, new Map(), new Map(), async () => + github.octokit.git.updateRef({ ref: `heads/${branch}`, sha: "c".repeat(40), force: true }), + ), + ).rejects.toThrow(/unrecorded generated branch/iu); + + expect(originalUpdate).not.toHaveBeenCalled(); + expect(github.graphql).not.toHaveBeenCalled(); + expect(github.octokit.git.createRef).toBe(originalCreate); + }); + + it("uses CAS for the first update after creating a generated branch", async () => { + const branch = "release-please--branches--master--components--pack"; + const initial = "b".repeat(40); + const after = "c".repeat(40); + const originalCreate = vi.fn().mockResolvedValue({ data: { object: { sha: initial } } }); + const originalUpdate = vi.fn(); + const graphql = vi + .fn() + .mockResolvedValueOnce({ repository: { id: "repo-id" } }) + .mockResolvedValueOnce({ updateRefs: { clientMutationId: null } }); + const github = { + repository: { owner: "lamemustafa", repo: "pack" }, + graphql, + octokit: { git: { createRef: originalCreate, updateRef: originalUpdate } }, + }; + + await withReleaseBranchRewriteCas(github, new Map(), new Map(), async () => { + await github.octokit.git.createRef({ ref: `refs/heads/${branch}`, sha: initial }); + await github.octokit.git.updateRef({ ref: `heads/${branch}`, sha: after, force: true }); + }); + + expect(originalCreate).toHaveBeenCalledOnce(); + expect(originalUpdate).not.toHaveBeenCalled(); + expect(graphql).toHaveBeenLastCalledWith( + expect.stringContaining("updateRefs"), + expect.objectContaining({ + refUpdates: [expect.objectContaining({ beforeOid: initial, afterOid: after, force: true })], + }), + ); + expect(github.octokit.git.createRef).toBe(originalCreate); + expect(github.octokit.git.updateRef).toBe(originalUpdate); + }); + + it("rejects a raced first generated-branch update after creation", async () => { + const branch = "release-please--branches--master--components--pack"; + const initial = "b".repeat(40); + const originalCreate = vi.fn().mockResolvedValue({ data: { object: { sha: initial } } }); + const originalUpdate = vi.fn(); + const github = { + repository: { owner: "lamemustafa", repo: "pack" }, + graphql: vi + .fn() + .mockResolvedValueOnce({ repository: { id: "repo-id" } }) + .mockRejectedValueOnce(new Error("Reference update failed: beforeOid does not match")), + octokit: { git: { createRef: originalCreate, updateRef: originalUpdate } }, + }; + + await expect( + withReleaseBranchRewriteCas(github, new Map(), new Map(), async () => { + await github.octokit.git.createRef({ ref: `refs/heads/${branch}`, sha: initial }); + return github.octokit.git.updateRef({ ref: `heads/${branch}`, sha: "c".repeat(40) }); + }), + ).rejects.toThrow(/beforeOid does not match/iu); + + expect(originalUpdate).not.toHaveBeenCalled(); + expect(github.octokit.git.createRef).toBe(originalCreate); + expect(github.octokit.git.updateRef).toBe(originalUpdate); + }); + + it("does not authorize an update when generated-branch creation is unconfirmed", async () => { + const branch = "release-please--branches--master--components--pack"; + const originalCreate = vi.fn().mockResolvedValue({ data: { object: { sha: "unknown" } } }); + const originalUpdate = vi.fn(); + const github = { + repository: { owner: "lamemustafa", repo: "pack" }, + graphql: vi.fn(), + octokit: { git: { createRef: originalCreate, updateRef: originalUpdate } }, + }; + + await expect( + withReleaseBranchRewriteCas(github, new Map(), new Map(), async () => { + await expect( + github.octokit.git.createRef({ ref: `refs/heads/${branch}`, sha: "b".repeat(40) }), + ).rejects.toThrow(/did not confirm the initial head/iu); + return github.octokit.git.updateRef({ ref: `heads/${branch}`, sha: "c".repeat(40) }); + }), + ).rejects.toThrow(/unrecorded generated branch/iu); + + expect(originalUpdate).not.toHaveBeenCalled(); + expect(github.graphql).not.toHaveBeenCalled(); + expect(github.octokit.git.createRef).toBe(originalCreate); + expect(github.octokit.git.updateRef).toBe(originalUpdate); + }); + + it("does not authorize an update when generated-branch creation is refused", async () => { + const branch = "release-please--branches--master--components--pack"; + const originalCreate = vi.fn().mockRejectedValue(new Error("reference already exists")); + const originalUpdate = vi.fn(); + const github = { + repository: { owner: "lamemustafa", repo: "pack" }, + graphql: vi.fn(), + octokit: { git: { createRef: originalCreate, updateRef: originalUpdate } }, + }; + + await expect( + withReleaseBranchRewriteCas(github, new Map(), new Map(), async () => { + await expect( + github.octokit.git.createRef({ ref: `refs/heads/${branch}`, sha: "b".repeat(40) }), + ).rejects.toThrow(/reference already exists/iu); + return github.octokit.git.updateRef({ ref: `heads/${branch}`, sha: "c".repeat(40) }); + }), + ).rejects.toThrow(/unrecorded generated branch/iu); + + expect(originalUpdate).not.toHaveBeenCalled(); + expect(github.graphql).not.toHaveBeenCalled(); + expect(github.octokit.git.createRef).toBe(originalCreate); + expect(github.octokit.git.updateRef).toBe(originalUpdate); + }); + it("emits root release outputs compatible with release-please-action", () => { const outputs = buildReleaseOutputs([ { @@ -99,6 +305,14 @@ describe("Release Please workflow wrapper", () => { .spyOn(releasePlease.Manifest.prototype, "createPullRequests") .mockImplementation(createPullRequests); const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + const fetched: string[] = []; + vi.stubGlobal( + "fetch", + vi.fn(async (url: string, init: RequestInit = {}) => { + fetched.push(`${String(init.method ?? "GET")} ${new URL(String(url)).pathname}`); + return { ok: true, status: 200, json: async () => [] } as unknown as Response; + }), + ); try { const outputs = await runReleasePlease({ @@ -120,11 +334,742 @@ describe("Release Please workflow wrapper", () => { prs_created: "true", release_created: "true", }); + // The only direct GitHub calls are three branch-head reads: the snapshot taken before + // anything irreversible happens, the re-read immediately before the rewrite, and the read + // after it. The middle one is the point -- the first is too old to say what the force-push + // is about to discard. Nothing is written, because this run rewrote no branch. + const headRead = + "GET /repos/lamemustafa/pack/git/matching-refs/heads/release-please--branches--master--"; + expect(fetched).toEqual([headRead, headRead]); } finally { + vi.unstubAllGlobals(); log.mockRestore(); pullRequestManifest.mockRestore(); releaseManifest.mockRestore(); getFileContentsOnBranch.mockRestore(); } }); + + it("propagates regeneration failure when no release was created", async () => { + const configContents = await readFile( + new URL("../../release-please-config.json", import.meta.url), + "utf8", + ); + const manifestContents = await readFile( + new URL("../../.release-please-manifest.json", import.meta.url), + "utf8", + ); + const createReleases = vi.fn().mockResolvedValue([]); + const createPullRequests = vi.fn().mockRejectedValue(new Error("regeneration failed")); + const getFileContentsOnBranch = vi + .spyOn(releasePlease.GitHub.prototype, "getFileContentsOnBranch") + .mockImplementation(async (...args: unknown[]) => { + const [path] = args; + if (path === "release-please-config.json") return { parsedContent: configContents }; + if (path === ".release-please-manifest.json") return { parsedContent: manifestContents }; + throw new Error(`Unexpected release-please file request: ${String(path)}`); + }); + const releaseManifest = vi + .spyOn(releasePlease.Manifest.prototype, "createReleases") + .mockImplementation(createReleases); + const pullRequestManifest = vi + .spyOn(releasePlease.Manifest.prototype, "createPullRequests") + .mockImplementation(createPullRequests); + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + vi.stubGlobal( + "fetch", + vi.fn(async () => ({ ok: true, status: 200, json: async () => [] }) as unknown as Response), + ); + + try { + await expect( + runReleasePlease({ + GITHUB_REPOSITORY: "lamemustafa/pack", + RELEASE_PLEASE_TOKEN: "test-token", + RELEASE_PLEASE_TARGET_BRANCH: "master", + }), + ).rejects.toThrow(/regeneration failed/iu); + + expect(createReleases).toHaveBeenCalledOnce(); + expect(createPullRequests).toHaveBeenCalledOnce(); + } finally { + vi.unstubAllGlobals(); + log.mockRestore(); + pullRequestManifest.mockRestore(); + releaseManifest.mockRestore(); + getFileContentsOnBranch.mockRestore(); + } + }); + + it("keeps release outputs when generated-branch CAS rejects the regeneration", async () => { + const branch = "release-please--branches--master--components--pack"; + const before = "b".repeat(40); + const marker = ``; + const originalCreate = vi.fn(); + const originalUpdate = vi.fn(); + const github = { + repository: { owner: "lamemustafa", repo: "pack", defaultBranch: "master" }, + graphql: vi + .fn() + .mockResolvedValueOnce({ repository: { id: "repo-id" } }) + .mockRejectedValueOnce(new Error("Reference update failed: beforeOid does not match")), + octokit: { git: { createRef: originalCreate, updateRef: originalUpdate } }, + }; + const createReleases = vi + .fn() + .mockResolvedValue([{ path: ".", tagName: "v0.1.1", version: "0.1.1" }]); + const createPullRequests = vi.fn(async () => + github.octokit.git.updateRef({ ref: `heads/${branch}`, sha: "c".repeat(40), force: true }), + ); + const create = vi.spyOn(releasePlease.GitHub, "create").mockResolvedValue(github); + const fromManifest = vi + .spyOn(releasePlease.Manifest, "fromManifest") + .mockResolvedValueOnce({ createReleases }) + .mockResolvedValueOnce({ createPullRequests }); + const error = vi.spyOn(console, "error").mockImplementation(() => undefined); + let markerExists = false; + vi.stubGlobal( + "fetch", + vi.fn(async (url: string, init: RequestInit = {}) => { + const path = new URL(String(url)).pathname; + if (path.includes("matching-refs")) { + return { + ok: true, + status: 200, + json: async () => [{ ref: `refs/heads/${branch}`, object: { sha: before } }], + } as unknown as Response; + } + if (path.includes("/pulls")) { + return { + ok: true, + status: 200, + json: async () => [{ number: 337 }], + } as unknown as Response; + } + if (path.includes("/comments") && String(init.method ?? "GET") === "POST") { + markerExists = true; + return { ok: true, status: 201, json: async () => ({ id: 99 }) } as unknown as Response; + } + if (path.includes("/comments") && String(init.method ?? "GET") === "PATCH") { + return { ok: true, status: 200, json: async () => ({ id: 99 }) } as unknown as Response; + } + if (path.includes("/comments")) { + return { + ok: true, + status: 200, + json: async () => + markerExists + ? [ + { + id: 99, + created_at: "2026-09-12T00:00:00Z", + user: { login: RECORDER }, + body: marker, + }, + ] + : [], + } as unknown as Response; + } + throw new Error(`Unexpected GitHub request: ${path}`); + }), + ); + + try { + const outputs = await runReleasePlease({ + GITHUB_REPOSITORY: "lamemustafa/pack", + GITHUB_TOKEN: "test-token", + GITHUB_API_URL: "https://api.github.test", + }); + + expect(createReleases).toHaveBeenCalledOnce(); + expect(createPullRequests).toHaveBeenCalledOnce(); + expect(outputs).toMatchObject({ release_created: "true", prs_created: "false" }); + expect(originalUpdate).not.toHaveBeenCalled(); + expect(error).toHaveBeenCalledWith( + expect.stringContaining("Existing release outputs remain"), + ); + } finally { + vi.unstubAllGlobals(); + error.mockRestore(); + fromManifest.mockRestore(); + create.mockRestore(); + } + }); + + it("closes a confirmed CAS receipt before propagating a later no-release failure", async () => { + const branch = "release-please--branches--master--components--pack"; + const before = "b".repeat(40); + const after = "c".repeat(40); + const marker = ``; + const originalCreate = vi.fn(); + const originalUpdate = vi.fn(); + const github = { + repository: { owner: "lamemustafa", repo: "pack", defaultBranch: "master" }, + graphql: vi + .fn() + .mockResolvedValueOnce({ repository: { id: "repo-id" } }) + .mockResolvedValueOnce({ updateRefs: { clientMutationId: null } }), + octokit: { git: { createRef: originalCreate, updateRef: originalUpdate } }, + }; + const createReleases = vi.fn().mockResolvedValue([]); + const createPullRequests = vi.fn(async () => { + await github.octokit.git.updateRef({ ref: `heads/${branch}`, sha: after, force: true }); + throw new Error("later pull request API failure"); + }); + const create = vi.spyOn(releasePlease.GitHub, "create").mockResolvedValue(github); + const fromManifest = vi + .spyOn(releasePlease.Manifest, "fromManifest") + .mockResolvedValueOnce({ createReleases }) + .mockResolvedValueOnce({ createPullRequests }); + let markerExists = false; + let closedBody = ""; + vi.stubGlobal( + "fetch", + vi.fn(async (url: string, init: RequestInit = {}) => { + const path = new URL(String(url)).pathname; + if (path.includes("matching-refs")) { + return { + ok: true, + status: 200, + json: async () => [{ ref: `refs/heads/${branch}`, object: { sha: before } }], + } as unknown as Response; + } + if (path.includes("/pulls")) { + return { + ok: true, + status: 200, + json: async () => [{ number: 337 }], + } as unknown as Response; + } + if (path.includes("/comments") && String(init.method ?? "GET") === "POST") { + markerExists = true; + return { ok: true, status: 201, json: async () => ({ id: 99 }) } as unknown as Response; + } + if (path.includes("/comments") && String(init.method ?? "GET") === "PATCH") { + closedBody = String(init.body); + return { ok: false, status: 500 } as unknown as Response; + } + if (path.includes("/comments")) { + return { + ok: true, + status: 200, + json: async () => + markerExists + ? [ + { + id: 99, + created_at: "2026-09-12T00:00:00Z", + user: { login: RECORDER }, + body: marker, + }, + ] + : [], + } as unknown as Response; + } + throw new Error(`Unexpected GitHub request: ${path}`); + }), + ); + + try { + await expect( + runReleasePlease({ + GITHUB_REPOSITORY: "lamemustafa/pack", + GITHUB_TOKEN: "test-token", + GITHUB_API_URL: "https://api.github.test", + }), + ).rejects.toThrow(/Could not complete release pull request regeneration/iu); + + expect(originalUpdate).not.toHaveBeenCalled(); + expect(closedBody).toContain(`before=${before}`); + expect(closedBody).toContain(`after=${after}`); + } finally { + vi.unstubAllGlobals(); + fromManifest.mockRestore(); + create.mockRestore(); + } + }); +}); + +describe("release branch rewrite records", () => { + const branch = "release-please--branches--master--components--pack"; + const env = { GITHUB_TOKEN: "t", GITHUB_API_URL: "https://api.github.test" }; + + function openedRecords(head: string, recordId = 99) { + return new Map([[branch, { head, recordId, pullRequestNumber: 337 }]]); + } + + function stubGitHub(handlers: { + heads?: string | null; + comments?: Array<{ id: number; body: string; user?: { login: string } }>; + pulls?: Array<{ number: number }>; + malformedHeads?: boolean; + malformedHeadEntry?: boolean; + commentPages?: Array>; + forcePushedHeads?: Array<{ commit_id?: string; created_at?: string }>; + }) { + const calls: Array<{ method: string; path: string; body?: string }> = []; + vi.stubGlobal( + "fetch", + vi.fn(async (url: string, init: RequestInit = {}) => { + const path = new URL(String(url)).pathname + new URL(String(url)).search; + calls.push({ method: String(init.method ?? "GET"), path, body: init.body as string }); + if (path.includes("matching-refs")) { + const payload = handlers.malformedHeads + ? { message: "something else" } + : handlers.malformedHeadEntry + ? [{ ref: `refs/heads/${branch}`, object: {} }] + : handlers.heads + ? [{ ref: `refs/heads/${branch}`, object: { sha: handlers.heads } }] + : []; + return { ok: true, status: 200, json: async () => payload } as unknown as Response; + } + if (path.includes("/pulls?")) { + return { + ok: true, + status: 200, + json: async () => handlers.pulls ?? [{ number: 337 }], + } as unknown as Response; + } + if (path.includes("/timeline")) { + // `commit_id` on a force-push event is the head that rewrite created -- the field the + // review gate keys recorded discards by. + return { + ok: true, + status: 200, + json: async () => + (handlers.forcePushedHeads ?? []).map((entry) => ({ + event: "head_ref_force_pushed", + created_at: entry.created_at ?? "2026-09-12T00:00:00Z", + ...entry, + })), + } as unknown as Response; + } + if (path.includes("/issues/") && path.includes("/comments")) { + if (String(init.method ?? "GET") === "POST") { + return { + ok: true, + status: 201, + json: async () => ({ id: 999 }), + } as unknown as Response; + } + if (handlers.commentPages) { + const page = Number(new URL(String(url)).searchParams.get("page") ?? "1"); + return { + ok: true, + status: 200, + json: async () => handlers.commentPages?.[page - 1] ?? [], + } as unknown as Response; + } + return { + ok: true, + status: 200, + json: async () => handlers.comments ?? [], + } as unknown as Response; + } + return { ok: true, status: 200, json: async () => ({}) } as unknown as Response; + }), + ); + return calls; + } + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("opens a record naming the head about to be discarded", async () => { + const head = "b".repeat(40); + const calls = stubGitHub({ heads: head }); + const { openBranchRewriteRecords } = await import("../../scripts/run-release-please.mjs"); + + const heads = await openBranchRewriteRecords({ + env, + owner: "lamemustafa", + repo: "pack", + targetBranch: "master", + }); + + expect(heads.get(branch)).toMatchObject({ head, pullRequestNumber: 337, recordId: 999 }); + const posted = calls.find((call) => call.method === "POST"); + expect(posted?.path).toContain("/issues/337/comments"); + expect(posted?.body).toContain(`review-gate-rewrite branch=${branch} before=${head}`); + expect(posted?.body).not.toContain("after="); + }); + + it("holds a retained generated branch without a durable rewrite marker", async () => { + const head = "b".repeat(40); + const calls = stubGitHub({ heads: head, pulls: [] }); + const { openBranchRewriteRecords } = await import("../../scripts/run-release-please.mjs"); + + await expect( + openBranchRewriteRecords({ env, owner: "lamemustafa", repo: "pack", targetBranch: "master" }), + ).rejects.toThrow(/no open release pull request.*durable rewrite record/iu); + expect(calls.find((call) => call.method === "POST")).toBeUndefined(); + }); + + it("holds an interrupted record when its branch changed", async () => { + const lostBefore = "c".repeat(40); + const created = "b".repeat(40); + const landedSince = "e".repeat(40); + const calls = stubGitHub({ + heads: landedSince, + comments: [ + { + id: 99, + user: { login: RECORDER }, + body: ``, + }, + ], + forcePushedHeads: [{ commit_id: created }], + }); + const { openBranchRewriteRecords } = await import("../../scripts/run-release-please.mjs"); + + await expect( + openBranchRewriteRecords({ env, owner: "lamemustafa", repo: "pack", targetBranch: "master" }), + ).rejects.toThrow(/interrupted rewrite record no longer matches/iu); + expect(calls.find((call) => call.method === "PATCH")).toBeUndefined(); + }); + + it("does not consult timeline events for an unchanged interrupted record", async () => { + const before = "c".repeat(40); + const calls = stubGitHub({ + heads: before, + comments: [ + { + id: 99, + user: { login: RECORDER }, + body: ``, + }, + ], + forcePushedHeads: [{ commit_id: "b".repeat(40) }, { commit_id: "d".repeat(40) }], + }); + const { openBranchRewriteRecords } = await import("../../scripts/run-release-please.mjs"); + + await expect( + openBranchRewriteRecords({ env, owner: "lamemustafa", repo: "pack", targetBranch: "master" }), + ).resolves.toBeInstanceOf(Map); + expect(calls.find((call) => call.path.includes("/timeline"))).toBeUndefined(); + }); + + it("ignores a marker written by anyone but the workflow", async () => { + // The author is the whole of a marker's authority: anyone who can comment can write the text. + // Treating a stranger's comment as an open record would rewrite that comment in place, or -- + // if it cannot be edited -- abort the run before any release work began. + const calls = stubGitHub({ + heads: "b".repeat(40), + comments: [ + { + id: 99, + user: { login: "a-passer-by" }, + body: ``, + }, + ], + }); + const { openBranchRewriteRecords } = await import("../../scripts/run-release-please.mjs"); + + await openBranchRewriteRecords({ + env, + owner: "lamemustafa", + repo: "pack", + targetBranch: "master", + }); + + expect(calls.find((call) => call.method === "PATCH")).toBeUndefined(); + }); + + it("refuses to regenerate when a pull request entry cannot be read", async () => { + // A list that is empty means no open pull request. A list whose entry cannot be read means the + // answer is unknown. Collapsing the second into the first skips the record while the rewrite + // force-pushes anyway, discarding a head nothing named. + stubGitHub({ heads: "b".repeat(40), pulls: [{ id: 1 } as unknown as { number: number }] }); + const { openBranchRewriteRecords } = await import("../../scripts/run-release-please.mjs"); + + await expect( + openBranchRewriteRecords({ env, owner: "lamemustafa", repo: "pack", targetBranch: "master" }), + ).rejects.toThrow(/malformed pull request list/iu); + }); + + it("refuses an ordinary advance after capture without replacing the recorded head", async () => { + const snapshot = "b".repeat(40); + const arrivedSince = "e".repeat(40); + const calls = stubGitHub({ + heads: arrivedSince, + comments: [ + { + id: 99, + user: { login: RECORDER }, + body: ``, + }, + ], + }); + const { refreshBranchRewriteRecords } = await import("../../scripts/run-release-please.mjs"); + + const proceeded = await refreshBranchRewriteRecords({ + env, + owner: "lamemustafa", + repo: "pack", + targetBranch: "master", + headsBeforeRegeneration: openedRecords(snapshot), + }); + + expect(proceeded).toBe(false); + expect(calls.find((call) => call.method === "PATCH")).toBeUndefined(); + expect(calls.find((call) => call.method === "POST")).toBeUndefined(); + }); + + it("refuses a force advance after capture without confusing it for an ordinary one", async () => { + const snapshot = "b".repeat(40); + const arrivedSince = "e".repeat(40); + const calls = stubGitHub({ + heads: arrivedSince, + comments: [ + { + id: 99, + user: { login: RECORDER }, + body: ``, + }, + ], + forcePushedHeads: [{ commit_id: arrivedSince }], + }); + const { refreshBranchRewriteRecords } = await import("../../scripts/run-release-please.mjs"); + + await expect( + refreshBranchRewriteRecords({ + env, + owner: "lamemustafa", + repo: "pack", + targetBranch: "master", + headsBeforeRegeneration: openedRecords(snapshot), + }), + ).resolves.toBe(false); + + expect(calls.find((call) => call.path.includes("/timeline"))).toBeUndefined(); + expect(calls.find((call) => call.method === "PATCH")).toBeUndefined(); + }); + + it("refuses to regenerate when it cannot confirm what is about to be discarded", async () => { + // Throwing is unavailable here: the release exists and a later workflow step uploads its + // assets. So the rewrite is skipped instead -- a pull request that waits for the next run is + // recoverable, a head discarded with no record of it is not. + stubGitHub({ malformedHeads: true }); + const { refreshBranchRewriteRecords } = await import("../../scripts/run-release-please.mjs"); + + const proceeded = await refreshBranchRewriteRecords({ + env, + owner: "lamemustafa", + repo: "pack", + targetBranch: "master", + headsBeforeRegeneration: openedRecords("b".repeat(40)), + }); + + expect(proceeded).toBe(false); + }); + + it("refuses to regenerate when its opened marker is no longer present", async () => { + // A successful POST is not durable evidence if a later read cannot find that exact comment. + // Continuing would force-push the head the marker was meant to preserve. + stubGitHub({ heads: "b".repeat(40), comments: [] }); + const { refreshBranchRewriteRecords } = await import("../../scripts/run-release-please.mjs"); + + await expect( + refreshBranchRewriteRecords({ + env, + owner: "lamemustafa", + repo: "pack", + targetBranch: "master", + headsBeforeRegeneration: new Map([ + [branch, { head: "b".repeat(40), recordId: 999, pullRequestNumber: 337 }], + ]), + }), + ).resolves.toBe(false); + }); + + it("refuses to regenerate when the head list is malformed", async () => { + stubGitHub({ malformedHeads: true }); + const { openBranchRewriteRecords } = await import("../../scripts/run-release-please.mjs"); + + // An indeterminate response read as "no branches" would let a rewrite proceed unrecorded, + // which nothing downstream could detect. + await expect( + openBranchRewriteRecords({ + env, + owner: "lamemustafa", + repo: "pack", + targetBranch: "master", + }), + ).rejects.toThrow(/malformed release branch head list/iu); + }); + + // An entry this function cannot read makes its branch look absent, and an absent branch is + // force-pushed with no record of the head it discarded. That is the same claim as an unreadable + // response, so it is the same refusal rather than a skipped row. + it("refuses to regenerate when one matching-ref entry is unreadable", async () => { + stubGitHub({ malformedHeadEntry: true }); + const { openBranchRewriteRecords } = await import("../../scripts/run-release-please.mjs"); + + await expect( + openBranchRewriteRecords({ + env, + owner: "lamemustafa", + repo: "pack", + targetBranch: "master", + }), + ).rejects.toThrow(/malformed release branch head list/iu); + }); + + // A release pull request outlives a hundred comments, and the marker this mechanism just wrote is + // the newest one -- exactly what a first-page read drops. Losing it leaves the force-push + // permanently unpaired, because every later run reads the same truncated page. + it("closes only the marker named by a verified CAS receipt", async () => { + const before = "c".repeat(40); + const after = "b".repeat(40); + const marker = ``; + const calls = stubGitHub({ + heads: after, + forcePushedHeads: [{ commit_id: after }], + commentPages: [ + Array.from({ length: 100 }, (_unused, index) => ({ id: index + 1, body: "chatter" })), + [{ id: 501, user: { login: RECORDER }, body: marker }], + ], + }); + const { closeBranchRewriteRecords } = await import("../../scripts/run-release-please.mjs"); + + await closeBranchRewriteRecords({ + env, + owner: "lamemustafa", + repo: "pack", + confirmedRewrites: new Map([ + [branch, { record: { id: 501, marker: { branch, before } }, after }], + ]), + }); + + const patched = calls.find((call) => call.method === "PATCH"); + expect(patched?.path).toContain("/issues/comments/501"); + expect(patched?.body).toContain(`after=${after}`); + }); + + it("writes the verified CAS destination into the marker", async () => { + const before = "c".repeat(40); + const after = "b".repeat(40); + const calls = stubGitHub({ + heads: after, + forcePushedHeads: [{ commit_id: after }], + comments: [ + { + id: 99, + user: { login: RECORDER }, + body: ``, + }, + ], + }); + const { closeBranchRewriteRecords } = await import("../../scripts/run-release-please.mjs"); + + await closeBranchRewriteRecords({ + env, + owner: "lamemustafa", + repo: "pack", + confirmedRewrites: new Map([ + [branch, { record: { id: 99, marker: { branch, before } }, after }], + ]), + }); + + const patched = calls.find((call) => call.method === "PATCH"); + expect(patched?.body).toContain(`before=${before} after=${after}`); + }); + + it("does not replace a CAS receipt with a later branch head", async () => { + // The gate looks a record up by the head the force-push created -- the timeline event's + // `commit_id`. An ordinary commit landing on the branch before this read would otherwise have + // the record name a head no event mentions, leaving the rewrite as unidentified as if nothing + // had recorded it at all. + const before = "c".repeat(40); + const created = "b".repeat(40); + const landedSince = "e".repeat(40); + const calls = stubGitHub({ + heads: landedSince, + forcePushedHeads: [{ commit_id: created }], + comments: [ + { + id: 99, + user: { login: RECORDER }, + body: ``, + }, + ], + }); + const { closeBranchRewriteRecords } = await import("../../scripts/run-release-please.mjs"); + + await closeBranchRewriteRecords({ + env, + owner: "lamemustafa", + repo: "pack", + confirmedRewrites: new Map([ + [branch, { record: { id: 99, marker: { branch, before } }, after: created }], + ]), + }); + + const patched = calls.find((call) => call.method === "PATCH"); + expect(patched?.body).toContain(`before=${before} after=${created}`); + expect(patched?.body).not.toContain(landedSince); + }); + + it("does not write a marker without a CAS receipt", async () => { + // A receipt is the only attribution evidence. Closing with an uncorroborated head would + // publish a claim about a rewrite nothing backs. + const before = "c".repeat(40); + const calls = stubGitHub({ + heads: "b".repeat(40), + forcePushedHeads: [], + comments: [ + { + id: 99, + user: { login: RECORDER }, + body: ``, + }, + ], + }); + const { closeBranchRewriteRecords } = await import("../../scripts/run-release-please.mjs"); + + await closeBranchRewriteRecords({ + env, + owner: "lamemustafa", + repo: "pack", + confirmedRewrites: new Map(), + }); + + expect(calls.find((call) => call.method === "PATCH")).toBeUndefined(); + }); + + it("looks for the release pull request by base branch as well as head", async () => { + // A generated branch can carry open pull requests against more than one base. Matched on head + // alone, the record could be opened on a different pull request while the force-push rewrote + // this one. + const calls = stubGitHub({ heads: "b".repeat(40) }); + const { openBranchRewriteRecords } = await import("../../scripts/run-release-please.mjs"); + + await openBranchRewriteRecords({ + env, + owner: "lamemustafa", + repo: "pack", + targetBranch: "master", + }); + + const lookup = calls.find((call) => call.path.includes("/pulls?")); + expect(lookup?.path).toContain("base=master"); + }); + + it("does not close a marker without a CAS receipt", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async () => ({ ok: false, status: 500 }) as unknown as Response), + ); + const { closeBranchRewriteRecords } = await import("../../scripts/run-release-please.mjs"); + + await expect( + closeBranchRewriteRecords({ + env, + owner: "lamemustafa", + repo: "pack", + confirmedRewrites: new Map(), + }), + ).resolves.toEqual([]); + }); });