diff --git a/.github/scripts/pr-merge/merge-holds.mjs b/.github/scripts/pr-merge/merge-holds.mjs index ea2457e6d14..162802a226c 100755 --- a/.github/scripts/pr-merge/merge-holds.mjs +++ b/.github/scripts/pr-merge/merge-holds.mjs @@ -1452,6 +1452,19 @@ export function evidenceHold(report, head) { } // The evidence file's verdict, a file that does not read included. +// The reader's refusal of this head's pass (one of PASS_REFUSALS), or null. +// A refusal the workflow step got before reading any pass names leaves +// verdictHold no pass to read, so the hold names that refusal, not a missing +// pass. Any other refusal is evidenceHold's. +export function passRefusalOf(evidence, head) { + return evidence?.head === head && PASS_REFUSALS.includes(evidence.refusal) ? evidence.refusal : null; +} + +// Owner decision (2026-11-04, "Hold until re-judged"): a pass the linked issue +// changed after holds in the queue too, read or not, until pr-verify judges +// the head again (the lost-run reconciler re-runs it) or an approval lifts it. +const STALE = "verify:spec-changed"; + function evidenceOf(file, head) { try { return evidenceHold(JSON.parse(readFileSync(file, "utf8")), head); @@ -1676,7 +1689,21 @@ export async function main(argv = process.argv.slice(2), { env = process.env, gi // (VERIFIER_TITLE): they share this step with the path holds. const verifier = []; if (verdictsFile) { - const unverified = verdictHold(lines(verdictsFile).map((line) => JSON.parse(line)), env.GITHUB_REPOSITORY); + const records = lines(verdictsFile).map((line) => JSON.parse(line)); + let unverified = verdictHold(records, env.GITHUB_REPOSITORY); + let refused = null; + if (evidenceFile) { + try { + refused = passRefusalOf(JSON.parse(readFileSync(evidenceFile, "utf8")), head); + } catch { + refused = null; + } + } + if (refused === STALE) { + unverified = `pr-verify's pass on this head is stale (${STALE}): its linked issue changed after pr-verify judged it`; + } else if (unverified && records.length === 0 && refused) { + unverified = `pr-verify's reader refuses this head's pass (${refused})`; + } // An approval lifts the verifier hold whole, the link with it. const relinked = unverified || approved.length ? null : linkFile ? linkOf(linkFile, body, repo) : "its issue link could not be checked against its verifier pass"; if (!unverified && !relinked) { diff --git a/.github/scripts/pr-merge/merge-holds.test.mjs b/.github/scripts/pr-merge/merge-holds.test.mjs index 58e64e595cc..1c842dc0a7f 100644 --- a/.github/scripts/pr-merge/merge-holds.test.mjs +++ b/.github/scripts/pr-merge/merge-holds.test.mjs @@ -20,7 +20,7 @@ import { join } from "node:path"; import { test } from "node:test"; import * as holdsModule from "./merge-holds.mjs"; -import { BODY_WRITERS_QUERY, CARRY_REFUSES, COMMENT_EDITED_QUERY, approvalCarry, ungatedSince, HOLD_PATH_CLASSES, HOLD_PATH_FLOOR, PULL_OPENED_QUERY, REVIEW_FLOOR, baseMergeCarry, baseMergesOnly, changeOf, mergedParents, costDecision, costHold, costNotices, decidedHead, evidenceHold, laneChange, linkHold, main, pathHold, sameNetChange, unranHead } from "./merge-holds.mjs"; +import { BODY_WRITERS_QUERY, CARRY_REFUSES, COMMENT_EDITED_QUERY, approvalCarry, ungatedSince, HOLD_PATH_CLASSES, HOLD_PATH_FLOOR, PULL_OPENED_QUERY, REVIEW_FLOOR, baseMergeCarry, baseMergesOnly, changeOf, mergedParents, costDecision, costHold, costNotices, decidedHead, evidenceHold, laneChange, linkHold, main, passRefusalOf, pathHold, sameNetChange, unranHead } from "./merge-holds.mjs"; import { tempDir } from "../../../lib/test-tmp.mjs"; const REPO = "owner/repo"; @@ -967,10 +967,39 @@ test("in the merge queue, a reader with no FAIL hold lets a passed head through" assert.match(out, /^#99: no verifier hold: pr-verify passed this head\.$/m); }); -test("in the merge queue, a reader that refuses only on the pass leaves that to verdictHold", async () => { +// Owner decision 2026-11-04, "Hold until re-judged": a stale pass holds in the +// queue even when the pass is read; an approval of the head lifts it. +test("in the merge queue, a pass the linked issue changed after holds until pr-verify judges the head again", async () => { const { code, out } = await queued(report("verify:spec-changed")); - assert.equal(code, 0, out); + assert.equal(code, 1, out); assert.match(out, /^#99: no pr-verify FAIL hold: the reader refuses this head only on its pass \(verify:spec-changed\)\.$/m); + assert.match(out, /#99: pr-verify's pass on this head is stale \(verify:spec-changed\): its linked issue changed after pr-verify judged it; wait for pr-verify to pass it/); + // The other pass refusals still leave a read pass standing. + for (const refusal of ["verify:missing", "verify:untrusted-spec"]) { + const read = await queued(report(refusal)); + assert.equal(read.code, 0, `${refusal}: ${read.out}`); + } +}); + +test("in the merge queue, a pass refusal with no pass names read holds on that refusal, not on a missing pass", async () => { + const refused = await ci(["docs/a.md"], { queue: { verdicts: [], link: QUEUE_LINK, evidence: report("verify:untrusted-spec") } }); + assert.equal(refused.code, 1, refused.out); + assert.match(refused.out, /#99: pr-verify's reader refuses this head's pass \(verify:untrusted-spec\); wait for pr-verify to pass it/); + assert.doesNotMatch(refused.out, /pr-verify has not passed this head/); + const stale = await ci(["docs/a.md"], { queue: { verdicts: [], link: QUEUE_LINK, evidence: report("verify:spec-changed") } }); + assert.match(stale.out, /pr-verify's pass on this head is stale \(verify:spec-changed\)/); + // No refusal, one for another head, or a FAIL class (evidenceHold's) keeps + // the missing-pass text and never calls a FAIL a pass refusal. + for (const evidence of [report(null), report("verify:spec-changed", { head: OLD }), report("verify:fail")]) { + const missing = await ci(["docs/a.md"], { queue: { verdicts: [], link: QUEUE_LINK, evidence } }); + assert.equal(missing.code, 1, missing.out); + assert.match(missing.out, /pr-verify has not passed this head/); + assert.doesNotMatch(missing.out, /refuses this head's pass/); + } + assert.equal(passRefusalOf(report("verify:missing"), HEAD), "verify:missing"); + assert.equal(passRefusalOf(report("verify:fail"), HEAD), null); + assert.equal(passRefusalOf(report(null), HEAD), null); + assert.equal(passRefusalOf(null, HEAD), null); }); test("in the merge queue, a FAIL the reader counts holds a passed head", async () => { diff --git a/scripts/ci-lanes-pr-hold.test.sh b/scripts/ci-lanes-pr-hold.test.sh index 44e19da5985..fedb7ba9a62 100755 --- a/scripts/ci-lanes-pr-hold.test.sh +++ b/scripts/ci-lanes-pr-hold.test.sh @@ -878,7 +878,9 @@ evidence() { jq -nc --arg refusal "$1" --arg head "${2:-$SHA}" '{refusal: $refus for refusal in verify:fail verify:earlier-fail verify:repeated-fail verify:unreadable verify:too-old; do STUB_EVIDENCE="$(evidence "$refusal")" expect "in the merge queue, the reader's $refusal holds a passed head" 1 3 "::error title=pr-verify hold::#7: pr-verify's reader refuses this head ($refusal: why)" done -STUB_EVIDENCE="$(evidence verify:spec-changed)" expect "in the merge queue, a refusal a pass decides is left to the pass checks" 0 3 "#7: no pr-verify FAIL hold: the reader refuses this head only on its pass (verify:spec-changed)." +# Owner decision 2026-11-04, "Hold until re-judged": a stale pass holds even when read. +STUB_EVIDENCE="$(evidence verify:spec-changed)" expect "in the merge queue, a stale pass holds until pr-verify judges the head again" 1 3 "#7: pr-verify's pass on this head is stale (verify:spec-changed)" +STUB_EVIDENCE="$(evidence verify:untrusted-spec)" expect "in the merge queue, another pass refusal is left to the pass checks" 0 3 "#7: no pr-verify FAIL hold: the reader refuses this head only on its pass (verify:untrusted-spec)." STUB_EVIDENCE="$(evidence verify:fail)" STUB_REVIEWS="[$(approval kyle-sexton 153232337 APPROVED 2026-10-23T11:00:00Z "$SHA")]" \ expect "in the merge queue, a trusted approval of the head lifts the reader's FAIL hold" 0 3 "no pr-verify FAIL hold: kyle-sexton approved this head" STUB_EVIDENCE="$(evidence verify:fail)" STUB_REVIEWS="[$(approval kyle-sexton 153232337 APPROVED 2026-10-23T11:00:00Z "$OLD")]" \