From 61a3fd9471c1d3f0b9be4570990a1e3e0f67d77d Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Wed, 4 Nov 2026 14:38:21 -0500 Subject: [PATCH 1/3] fix(ci): name the reader's pass refusal in the merge hold, not a missing pass Co-Authored-By: Claude Opus 5.5 --- .github/scripts/pr-merge/merge-holds.mjs | 21 ++++++++++++++++++- .github/scripts/pr-merge/merge-holds.test.mjs | 18 +++++++++++++++- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/.github/scripts/pr-merge/merge-holds.mjs b/.github/scripts/pr-merge/merge-holds.mjs index ea2457e6d14..8ea802c45fe 100755 --- a/.github/scripts/pr-merge/merge-holds.mjs +++ b/.github/scripts/pr-merge/merge-holds.mjs @@ -1452,6 +1452,13 @@ 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, 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. +export function passRefusalOf(evidence, head) { + return evidence?.head === head && typeof evidence.refusal === "string" ? evidence.refusal : null; +} + function evidenceOf(file, head) { try { return evidenceHold(JSON.parse(readFileSync(file, "utf8")), head); @@ -1676,7 +1683,19 @@ 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); + if (unverified && records.length === 0 && evidenceFile) { + let refused = null; + try { + refused = passRefusalOf(JSON.parse(readFileSync(evidenceFile, "utf8")), head); + } catch { + refused = null; + } + if (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..3f50a995996 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"; @@ -973,6 +973,22 @@ test("in the merge queue, a reader that refuses only on the pass leaves that to assert.match(out, /^#99: no pr-verify FAIL hold: the reader refuses this head only on its pass \(verify:spec-changed\)\.$/m); }); +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:spec-changed") } }); + assert.equal(refused.code, 1, refused.out); + assert.match(refused.out, /#99: pr-verify's reader refuses this head's pass \(verify:spec-changed\); wait for pr-verify to pass it/); + assert.doesNotMatch(refused.out, /pr-verify has not passed this head/); + // No refusal, or one for another head, keeps the missing-pass text. + for (const evidence of [report(null), report("verify:spec-changed", { head: OLD })]) { + 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.equal(passRefusalOf(report("verify:missing"), HEAD), "verify:missing"); + 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 () => { const { code, out } = await queued(report("verify:fail", { detail: "fails at the head" })); assert.equal(code, 1, out); From 12d59ece998589dd51f3b2da6f86d9061d516837 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Wed, 4 Nov 2026 15:13:16 -0500 Subject: [PATCH 2/3] fix(ci): hold a stale pr-verify pass in the queue until pr-verify judges the head again A verify:spec-changed refusal now holds whether or not the pass was read (owner decision 2026-11-04, "Hold until re-judged"). passRefusalOf returns only PASS_REFUSALS, so a FAIL class is never named a pass refusal. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/pr-merge/merge-holds.mjs | 26 ++++++++++++------- .github/scripts/pr-merge/merge-holds.test.mjs | 25 +++++++++++++----- 2 files changed, 36 insertions(+), 15 deletions(-) diff --git a/.github/scripts/pr-merge/merge-holds.mjs b/.github/scripts/pr-merge/merge-holds.mjs index 8ea802c45fe..162802a226c 100755 --- a/.github/scripts/pr-merge/merge-holds.mjs +++ b/.github/scripts/pr-merge/merge-holds.mjs @@ -1452,13 +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, 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. +// 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 && typeof evidence.refusal === "string" ? evidence.refusal : null; + 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); @@ -1685,16 +1691,18 @@ export async function main(argv = process.argv.slice(2), { env = process.env, gi if (verdictsFile) { const records = lines(verdictsFile).map((line) => JSON.parse(line)); let unverified = verdictHold(records, env.GITHUB_REPOSITORY); - if (unverified && records.length === 0 && evidenceFile) { - let refused = null; + let refused = null; + if (evidenceFile) { try { refused = passRefusalOf(JSON.parse(readFileSync(evidenceFile, "utf8")), head); } catch { refused = null; } - if (refused) { - unverified = `pr-verify's reader refuses this head's pass (${refused})`; - } + } + 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"; diff --git a/.github/scripts/pr-merge/merge-holds.test.mjs b/.github/scripts/pr-merge/merge-holds.test.mjs index 3f50a995996..1c842dc0a7f 100644 --- a/.github/scripts/pr-merge/merge-holds.test.mjs +++ b/.github/scripts/pr-merge/merge-holds.test.mjs @@ -967,24 +967,37 @@ 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:spec-changed") } }); + 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:spec-changed\); wait for pr-verify to pass it/); + 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/); - // No refusal, or one for another head, keeps the missing-pass text. - for (const evidence of [report(null), report("verify:spec-changed", { head: OLD })]) { + 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); }); From dc7e0c86b89d2899eefc9b05aa6315cc6379a06c Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Wed, 4 Nov 2026 15:23:22 -0500 Subject: [PATCH 3/3] test(ci): pin the queue's hold on a stale pr-verify pass in the shell suite Co-Authored-By: Claude Opus 5.5 --- scripts/ci-lanes-pr-hold.test.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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")]" \