From df1d21196ef905304b43b69f729c5ce8cd2e88cd Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sun, 25 Oct 2026 23:40:24 -0400 Subject: [PATCH 1/8] fix(pr-merge): clear a native gate's needs-human label once its cause is gone Under merge.lane-prs native, enable-auto-merge writes every lanes App PR's pr-merge needs-human label, and its holdReason then keeps the PR from being judged again, so the label stayed until a person removed it (#10090). clear-escalations now judges enable-auto-merge's own person refusal comment (refusalBody, byte for byte) again at the current head with the gate's reader, and removes the label as refusal-cleared only when nothing refuses or what refuses is a lane's (verifyClears lane). A person's refusal, a risk hold (security finding, trust gate, cost), an unread answer, a failed read, and a hold-path or cost-path refusal keep it. Labels a person set and other lanes' escalations are untouched, as before. At most three reads a run; the step gets the reader's Actions token and a 15-minute timeout. Co-Authored-By: Claude Opus 5.5 --- .../scripts/pr-merge/clear-escalations.mjs | 74 +++++++++++- .../pr-merge/clear-escalations.test.mjs | 111 +++++++++++++++++- .github/workflows/pr-merge.yml | 9 +- 3 files changed, 181 insertions(+), 13 deletions(-) diff --git a/.github/scripts/pr-merge/clear-escalations.mjs b/.github/scripts/pr-merge/clear-escalations.mjs index 641460695a..2d955b440c 100755 --- a/.github/scripts/pr-merge/clear-escalations.mjs +++ b/.github/scripts/pr-merge/clear-escalations.mjs @@ -52,6 +52,17 @@ // reason and the head; a sweep that finds that comment for this escalation // and head only retries the removal. At most CLEARS_PER_RUN a run. // +// enable-auto-merge's own person refusal comment (refusalBody, byte for byte) +// on a lanes App pull request, the only pr-merge escalation there, whose +// reason is the gate's verifier or risk answer (verifyHold's), is judged +// again at the current head instead: the gate's reader (`evidence`, as +// enable-auto-merge reads it, the room's rung cap passed as C3 so the scorer +// answers at the config's rung) runs, and the label comes off as +// `refusal-cleared` only when nothing refuses or what refuses is a lane's +// (verifyClears `lane`). A person's refusal, a risk hold, an unread answer and +// a failed read keep it; so does a hold or cost path, which no reader answer +// clears. At most NATIVE_READS_PER_RUN reads a run. +// // Environment: // GH_TOKEN the lane App token (pull-requests and issues write) // GITHUB_REPOSITORY owner/repo @@ -63,6 +74,8 @@ // LANES_BOT_LOGIN the lanes App bot login (default // melodic-automation-lanes[bot]) // GITHUB_API_URL +// BABYSIT_VERIFY_GH_TOKEN the gate reader's Actions token (the job's +// GITHUB_TOKEN), as enable-auto-merge passes it import process from "node:process"; import { pathToFileURL } from "node:url"; @@ -84,6 +97,7 @@ import { spoken, } from "../pr-unblock/gate-source.mjs"; import { isProtectedPath } from "../pr-unblock/thread-scope.mjs"; +import { pythonVerify, REFUSAL_MARKER, refusalBody, verifyClears, verifyHold } from "./enable-auto-merge.mjs"; const DEFAULT_BOT = "melodic-automation-lanes[bot]"; const REASON = /^- Reason: `([a-z-]+)`$/m; @@ -110,6 +124,12 @@ const COMPARE_CAP = 300; const FILES_CAP = 3000; // Each clear lets the lanes run on the PR again (judgment). export const CLEARS_PER_RUN = 5; +// Each reader run takes up to enable-auto-merge's VERIFY_TIMEOUT_MS; the +// step's timeout-minutes (pr-merge.yml) allows this many (judgment). +export const NATIVE_READS_PER_RUN = 3; +const REFUSAL_CLEARED = "refusal-cleared"; +// verifyHold's reasons for an answer that refuses: the only ones re-judged. +const JUDGED = ["pr-merge's gate holds it on its risk score (", "pr-merge's gate refuses its verifier verdict (", "pr-merge's lane does not take its issue link ("]; const escapeCommand = (text) => String(text).replaceAll(/[\r\n%]/g, " "); const reportError = (number, error) => @@ -144,7 +164,9 @@ function mergeable(pull) { const keepNote = (label) => `To keep a person on it, add \`${label}\` back: a label a person adds is never removed.`; export const clearedComment = (reason, head, why, label) => - reason === HEAD_MOVED + reason === REFUSAL_CLEARED + ? `${clearedMarker(reason, head)}\nThe pr-merge lane left this pull request for a person, and at \`${head}\` that no longer holds (\`${REFUSAL_CLEARED}\`): ${why}. pr-merge removed \`${label}\`, so the next scheduled run judges it again. ${keepNote(label)}` + : reason === HEAD_MOVED ? `${clearedMarker(reason, head)}\nThe pr-merge lane escalated this pull request, and its head has moved (\`${HEAD_MOVED}\`): ${why}. pr-merge removed \`${label}\` so it judges \`${head}\` afresh, and escalates again if its refusal still holds. ${keepNote(label)}` : `${clearedMarker(reason, head)}\nThe pr-unblock lane escalated this pull request as \`${reason.replaceAll(",", "`, `")}\`, and that no longer holds at \`${head}\`: ${why}. pr-merge removed \`${label}\`, so the lanes run on it again. ${keepNote(label)}`; @@ -162,6 +184,18 @@ function prMergeEscalations(appComments) { }); } +// enable-auto-merge's person refusal comment, when it is the only pr-merge +// escalation in `merged` and verifyHold wrote its reason: { head, reason }. +function nativeRefusal(appComments, merged, label) { + if (merged.length !== 1 || merged[0].marker !== REFUSAL_MARKER || merged[0].head === null) { + return null; + } + const newest = appComments.filter((item) => lines(item.body).slice(0, 2).includes(REFUSAL_MARKER)).at(-1); + const reason = lines(newest.body).slice(5, -1).join("\n"); + const { head } = merged[0]; + return JUDGED.some((prefix) => reason.startsWith(prefix)) && newest.body === refusalBody(true, reason, head, label) ? { head, reason } : null; +} + // Whether the lanes App comment written last at or before `since` that carries // any marker other than this script's own is pr-merge's alone, and was written // within PAIRED_MS of it: pr-merge writes its comment, then the label. A @@ -183,7 +217,7 @@ function pairedWithPrMerge(appComments, since) { } // What to clear on `number`, or { reason } saying why nothing is. -async function verdict({ github, repo, number, label, bot, mainSha, lanePrs }) { +async function verdict({ github, repo, number, label, bot, mainSha, lanePrs, verify, reads }) { const timeline = await paginate(github, `/repos/${repo}/issues/${number}/timeline`); const { labeled, since, lifted } = labelHistory(timeline, label); if (!isApp(labeled?.actor, bot)) { @@ -240,6 +274,27 @@ async function verdict({ github, repo, number, label, bot, mainSha, lanePrs }) { if (unread !== undefined) { return { reason: `the pr-merge comment carrying ${unread.marker} names no head this script can read` }; } + const native = isApp(pull.user, bot) ? nativeRefusal(appComments, merged, label) : null; + if (native !== null) { + if (reads.left <= 0) { + return { reason: `enable-auto-merge's refusal waits: ${NATIVE_READS_PER_RUN} reader runs this run` }; + } + reads.left -= 1; + let evidence; + try { + evidence = await verify({ number, headRefOid: pull.head.sha, baseRefOid: pull.base?.sha }, "C3"); + } catch (error) { + return { reason: `the gate's reader did not answer: ${escapeCommand(error?.message ?? error)}` }; + } + const held = verifyHold(evidence, pull.head.sha); + if (held !== null && verifyClears(evidence, pull.head.sha) !== "lane") { + return { reason: `enable-auto-merge's refusal still needs a person at ${pull.head.sha}: ${held}` }; + } + return cleared(timeline, bot, since, [REFUSAL_CLEARED], pull.head.sha, [ + `it refused \`${native.head}\` (${native.reason})`, + held === null ? "the gate now takes its issue link, verifier verdict and risk score" : `what refuses now is one the lanes clear: ${held}`, + ]); + } if (merged.some(({ head }) => head === pull.head.sha)) { return { reason: `a pr-merge escalation names the current head ${pull.head.sha}` }; } @@ -257,15 +312,20 @@ async function verdict({ github, repo, number, label, bot, mainSha, lanePrs }) { why.push(cleared); } } + return cleared(timeline, bot, since, reasons, pull.head.sha, why); +} + +// The clear of `reasons` at `head`, and whether its comment is already posted. +function cleared(timeline, bot, since, reasons, head, why) { const recorded = reasons.join(","); - const marker = clearedMarker(recorded, pull.head.sha); + const marker = clearedMarker(recorded, head); const explained = timeline.some( (item) => item.event === "commented" && isApp(item.user, bot) && Date.parse(item.created_at) >= since && lines(item.body)[0] === marker, ); - return { reason: null, recorded, head: pull.head.sha, why: why.join("; "), explained }; + return { reason: null, recorded, head, why: why.join("; "), explained }; } -export async function main({ env = process.env, github } = {}) { +export async function main({ env = process.env, github, verify } = {}) { try { if (!REPO.test(env.GITHUB_REPOSITORY ?? "")) { throw new Error("GITHUB_REPOSITORY is not owner/repo"); @@ -278,6 +338,8 @@ export async function main({ env = process.env, github } = {}) { const bot = env.LANES_BOT_LOGIN || DEFAULT_BOT; const label = humanGatedLabelFrom(env.TRACKER_CONFIG || ".base/.work-item-tracker.json"); github ??= createGitHub({ token: env.GH_TOKEN, apiUrl: env.GITHUB_API_URL || undefined }); + verify ??= pythonVerify(env); + const reads = { left: NATIVE_READS_PER_RUN }; const tip = await github("GET", `/repos/${repo}/git/ref/heads/main`); const mainSha = tip?.ref === "refs/heads/main" ? tip.object?.sha : undefined; if (!SHA.test(String(mainSha))) { @@ -294,7 +356,7 @@ export async function main({ env = process.env, github } = {}) { continue; } try { - const result = await verdict({ github, repo, number, label, bot, mainSha, lanePrs: env.LANE_PRS }); + const result = await verdict({ github, repo, number, label, bot, mainSha, lanePrs: env.LANE_PRS, verify, reads }); if (result.reason !== null) { console.log(`clear-escalations: #${number} kept: ${result.reason}`); continue; diff --git a/.github/scripts/pr-merge/clear-escalations.test.mjs b/.github/scripts/pr-merge/clear-escalations.test.mjs index 9f688c80ab..31d2ec7d46 100644 --- a/.github/scripts/pr-merge/clear-escalations.test.mjs +++ b/.github/scripts/pr-merge/clear-escalations.test.mjs @@ -9,7 +9,8 @@ import assert from "node:assert/strict"; import { test } from "node:test"; -import { main } from "./clear-escalations.mjs"; +import { main, NATIVE_READS_PER_RUN } from "./clear-escalations.mjs"; +import { refusalBody } from "./enable-auto-merge.mjs"; const REPO = "melodic-software/claude-code-plugins"; const BOT = { login: "melodic-automation-lanes[bot]", type: "Bot" }; @@ -30,8 +31,14 @@ const escalated = (reason = "workflow-staged", { user = BOT, at = "2026-10-18T10 // One open pull request (number 7) labeled needs-human; `timeline` is its timeline and // `mainFiles` the files main changed since its merge base with the head. -function world({ timeline, mainFiles = [{ filename: "README.md" }], mergeable = false, pullFiles = [], author }) { +// `evidence` is what the gate's reader (enable-auto-merge pythonVerify) answers. +function world({ timeline, mainFiles = [{ filename: "README.md" }], mergeable = false, pullFiles = [], author, evidence = () => null }) { const calls = []; + const reads = []; + const verify = async (pull, rung) => { + reads.push({ pull, rung }); + return evidence(pull); + }; const github = async (method, path, body) => { calls.push({ method, path, body }); const route = path.replace(/[?&]per_page=100&page=\d+$/, ""); @@ -45,7 +52,7 @@ function world({ timeline, mainFiles = [{ filename: "README.md" }], mergeable = return path.endsWith("page=1") ? timeline : []; } if (method === "GET" && route === `/repos/${REPO}/pulls/7`) { - return { number: 7, state: "open", user: author, base: { ref: "main" }, head: { sha: HEAD }, labels: [{ name: "needs-human" }], mergeable }; + return { number: 7, state: "open", user: author, base: { ref: "main", sha: MAIN }, head: { sha: HEAD }, labels: [{ name: "needs-human" }], mergeable }; } if (method === "GET" && route === `/repos/${REPO}/compare/${HEAD}...${MAIN}`) { return { files: mainFiles }; @@ -58,7 +65,7 @@ function world({ timeline, mainFiles = [{ filename: "README.md" }], mergeable = } throw new Error(`unexpected ${method} ${path}`); }; - return { github, calls, writes: () => calls.filter((call) => call.method !== "GET") }; + return { github, verify, reads, calls, writes: () => calls.filter((call) => call.method !== "GET") }; } const run = async (w, env = {}) => { @@ -66,7 +73,7 @@ const run = async (w, env = {}) => { const original = console.log; console.log = (line) => logs.push(line); try { - return { code: await main({ env: { ...ENV, ...env }, github: w.github }), logs }; + return { code: await main({ env: { ...ENV, ...env }, github: w.github, verify: w.verify }), logs }; } finally { console.log = original; } @@ -486,3 +493,97 @@ test("a hold marker from github-actions[bot] with no lane footer, or from anothe assert.equal(w.writes().at(-1)?.method, "DELETE", hold.user.login); } }); + +// enable-auto-merge's own person refusal comment (refusalBody), then its +// label: under merge.lane-prs native the only writer of a lanes App PR's +// pr-merge label. #10090's shape: a risk hold on unbound reviews at the head +// the PR still has. +const UNBOUND = "pr-merge's gate holds it on its risk score (evidence-unbound:claude-review: not bound: rerun-not-allowed)"; +const nativeEscalated = (reason, head = HEAD) => mergeEscalated(refusalBody(true, reason, head, "needs-human")); +const clean = (pull) => ({ head: pull.headRefOid, refusal: null, ineligible: null, reviews: null }); + +test("enable-auto-merge's label at the current head comes off once the gate's reader no longer refuses there", async () => { + const w = world({ timeline: nativeEscalated(UNBOUND), author: BOT, evidence: clean }); + const { code, logs } = await run(w, { LANE_PRS: "native" }); + assert.equal(code, 0, logs.join("\n")); + assert.deepEqual(w.reads, [{ pull: { number: 7, headRefOid: HEAD, baseRefOid: MAIN }, rung: "C3" }]); + const [comment, removal] = w.writes(); + assert.equal(w.writes().length, 2); + assert.equal(comment.body.body.split("\n")[0], ``); + assert.match(comment.body.body, new RegExp(`\`${HEAD}\``)); + assert.deepEqual(removal, { method: "DELETE", path: `/repos/${REPO}/issues/7/labels/needs-human`, body: undefined }); +}); + +test("enable-auto-merge's label comes off when the refusal is now one the lanes clear", async () => { + const w = world({ timeline: nativeEscalated(UNBOUND), author: BOT, evidence: (pull) => ({ ...clean(pull), refusal: "verify:no-pass", needsHuman: false }) }); + await run(w, { LANE_PRS: "native" }); + assert.equal(w.writes().at(-1)?.method, "DELETE"); +}); + +test("enable-auto-merge's label stays while its cause, or another a person must clear, holds at the head", async () => { + for (const evidence of [ + // A security finding or a trusted-actor widening stays a person's. + (pull) => ({ ...clean(pull), reviews: "security-finding: 1 open CRITICAL finding" }), + (pull) => ({ ...clean(pull), reviews: "trust-gate:trusted-actors: widens the trusted actors" }), + // A verifier refusal the gate marks needsHuman, an issue link the lane does not take. + (pull) => ({ ...clean(pull), refusal: "verify:untrusted-spec", needsHuman: true }), + (pull) => ({ ...clean(pull), ineligible: "untrusted-provenance" }), + // Unread, read at another head, or a read that fails. + () => null, + () => ({ ...clean({ headRefOid: OLD }) }), + () => { + throw new Error("babysit_verify_evidence.py exited 1"); + }, + ]) { + const w = world({ timeline: nativeEscalated(UNBOUND), author: BOT, evidence }); + await run(w, { LANE_PRS: "native" }); + assert.deepEqual(w.writes(), [], String(evidence)); + } +}); + +test("enable-auto-merge's hold-path or cost-path label is never judged on the reader's answer", async () => { + for (const reason of ["it changes .github/workflows/ci.yml, a hold path", "it changes infra/main.ts, a cost path"]) { + const w = world({ timeline: nativeEscalated(reason), author: BOT, evidence: clean }); + await run(w, { LANE_PRS: "native" }); + assert.deepEqual(w.writes(), [], reason); + assert.deepEqual(w.reads, [], reason); + } +}); + +test("a label a person added, or added back, beside enable-auto-merge's comment is kept unread", async () => { + for (const timeline of [ + [commented(BOT, refusalBody(true, UNBOUND, HEAD, "needs-human"), "2026-10-18T09:59:58Z"), labeled(OWNER, "2026-10-18T10:00:00Z")], + [...nativeEscalated(UNBOUND), unlabeled(OWNER, "2026-10-18T11:00:00Z"), labeled(OWNER, "2026-10-18T11:00:10Z")], + ]) { + const w = world({ timeline, author: BOT, evidence: clean }); + await run(w, { LANE_PRS: "native" }); + assert.deepEqual(w.writes(), []); + assert.deepEqual(w.reads, []); + } +}); + +test("a pr-merge person comment not in enable-auto-merge's exact shape is not re-judged", async () => { + const edited = `${refusalBody(true, UNBOUND, HEAD, "needs-human")}\nedited`; + const w = world({ timeline: mergeEscalated(edited), author: BOT, evidence: clean }); + await run(w, { LANE_PRS: "native" }); + assert.deepEqual(w.writes(), []); + assert.deepEqual(w.reads, []); +}); + +test("enable-auto-merge's refusal at an old head is judged at the current one", async () => { + const still = world({ timeline: nativeEscalated(UNBOUND, OLD), author: BOT, evidence: (pull) => ({ ...clean(pull), reviews: "security-finding: 1 open CRITICAL finding" }) }); + await run(still, { LANE_PRS: "native" }); + assert.deepEqual(still.writes(), []); + const gone = world({ timeline: nativeEscalated(UNBOUND, OLD), author: BOT, evidence: clean }); + await run(gone, { LANE_PRS: "native" }); + assert.equal(gone.writes()[0].body.body.split("\n")[0], ``); +}); + +test("at most NATIVE_READS_PER_RUN reader runs a run", async () => { + const w = world({ timeline: nativeEscalated(UNBOUND), author: BOT, evidence: () => null }); + const held = Array.from({ length: NATIVE_READS_PER_RUN + 2 }, (_, i) => ({ number: 7, pull_request: {}, i })); + const github = async (method, path, body) => (method === "GET" && path.includes("issues?state=open") ? (path.endsWith("page=1") ? held : []) : w.github(method, path, body)); + const { logs } = await run({ ...w, github }); + assert.equal(w.reads.length, NATIVE_READS_PER_RUN); + assert.equal(logs.filter((line) => /reader runs this run/.test(line)).length, 2); +}); diff --git a/.github/workflows/pr-merge.yml b/.github/workflows/pr-merge.yml index 1569957bdc..5e2e115f7a 100644 --- a/.github/workflows/pr-merge.yml +++ b/.github/workflows/pr-merge.yml @@ -1324,16 +1324,21 @@ jobs: # reason, feedback-protected-path), after a comment naming the reason # and the head: never a label a person set, and never after anyone but # the App commented or reviewed since it went on. At most five a run, and not - # while the lane is report-only. + # while the lane is report-only. A label enable-auto-merge set is judged + # again with the gate's reader at the head, at most three reads a run + # of up to four minutes each. # Rules: .github/scripts/pr-merge/clear-escalations.mjs. - name: Clear escalations whose reason no longer holds if: >- always() && github.event_name == 'schedule' && steps.lane-token.outputs.token != '' && steps.mode.outputs.report-only == 'false' - timeout-minutes: 3 + timeout-minutes: 15 env: GH_TOKEN: ${{ steps.lane-token.outputs.token }} + BABYSIT_VERIFY_GH_TOKEN: ${{ github.token }} + API_BUDGET_TOKEN: lanes-app + API_BUDGET_OVERRIDE_TOKEN: github-token KILL_SWITCH: ${{ steps.kill.outputs.proceed == 'true' && 'proceed' || '' }} LANE_PRS: ${{ steps.mode.outputs.lane-prs }} run: node .base/.github/scripts/pr-merge/clear-escalations.mjs From 9775cddaca4637e82ea1d0c1215425a80c294c21 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 26 Oct 2026 05:39:33 -0400 Subject: [PATCH 2/8] fix(pr-merge): re-judge native refusals only at their own head and keep a hidden cause clear-escalations no longer clears a risk or issue-link label on a reader answer that refuses for another reason (a verdict refusal hides the risk score), and never clears a verifier label on an unreadable or missing verdict or an unreadable risk score. It re-judges enable-auto-merge's refusal only under merge.lane-prs native, only at the head the comment names (a moved head takes main's head-moved path), and not when the comment was edited. The sweep starts at an offset that moves with the run number, so labels past the three-read cap are read on a later run, and the clear step shares the gate step's run cache, listing and head-ref map. enable-auto-merge exports READER_HOLDS, the reason prefixes verifyHold writes, and tells a person that pr-merge takes a reader refusal's label off itself; a comment in the earlier wording still stands. Co-Authored-By: Claude Opus 5.5 --- .../scripts/pr-merge/clear-escalations.mjs | 80 +++++--- .../pr-merge/clear-escalations.test.mjs | 182 +++++++++++++++--- .../scripts/pr-merge/enable-auto-merge.mjs | 47 +++-- .../pr-merge/enable-auto-merge.test.mjs | 24 ++- .github/workflows/pr-merge.yml | 10 +- 5 files changed, 273 insertions(+), 70 deletions(-) diff --git a/.github/scripts/pr-merge/clear-escalations.mjs b/.github/scripts/pr-merge/clear-escalations.mjs index ff69bea6f5..ba63bdd5bd 100755 --- a/.github/scripts/pr-merge/clear-escalations.mjs +++ b/.github/scripts/pr-merge/clear-escalations.mjs @@ -53,16 +53,22 @@ // reason and the head; a sweep that finds that comment for this escalation // and head only retries the removal. At most CLEARS_PER_RUN a run. // -// enable-auto-merge's own person refusal comment (refusalBody, byte for byte) -// on a lanes App pull request, the only pr-merge escalation there, whose -// reason is the gate's verifier or risk answer (verifyHold's), is judged -// again at the current head instead: the gate's reader (`evidence`, as -// enable-auto-merge reads it, the room's rung cap passed as C3 so the scorer -// answers at the config's rung) runs, and the label comes off as -// `refusal-cleared` only when nothing refuses or what refuses is a lane's -// (verifyClears `lane`). A person's refusal, a risk hold, an unread answer and -// a failed read keep it; so does a hold or cost path, which no reader answer -// clears. At most NATIVE_READS_PER_RUN reads a run. +// Under `merge.lane-prs: native`, enable-auto-merge's own person refusal +// comment (one of personBodies, unedited) on a lanes App pull request, the +// only pr-merge escalation there, naming the current head, whose reason is +// the gate's issue-link, verifier or risk answer (READER_HOLDS), is judged +// again at that head: the gate's reader (`evidence`, as enable-auto-merge +// reads it, the room's rung cap passed as C3 so the scorer answers at the +// config's rung) runs, and the label comes off as `refusal-cleared` when +// nothing refuses, or when the answer was read whole and what refuses now +// is a lane's (verifyClears `lane`) for the same cause: for a verifier +// reason a verdict that is neither unreadable nor missing, for a risk reason +// a risk score, which the reader computes only once the link and verdict +// pass. An issue-link reason comes off only when nothing refuses. A person's +// refusal, an unread answer and a failed read keep it; so does a hold or +// cost path, which no reader answer clears. At most NATIVE_READS_PER_RUN +// reads a run, starting at an offset that moves with the run number +// (rotated), so labels past the cap are read on a later run. // // Environment: // GH_TOKEN the lane App token (pull-requests and issues write) @@ -70,11 +76,15 @@ // KILL_SWITCH `proceed` from the kill-switch step; anything else stops // TRACKER_CONFIG the base .work-item-tracker.json, for the human-gated // label (default .base/.work-item-tracker.json) +// LANE_PRS merge.lane-prs as resolve-config resolved it; only +// `native` re-judges enable-auto-merge's refusal // LANES_BOT_LOGIN the lanes App bot login (default // melodic-automation-lanes[bot]) // GITHUB_API_URL -// BABYSIT_VERIFY_GH_TOKEN the gate reader's Actions token (the job's -// GITHUB_TOKEN), as enable-auto-merge passes it +// GITHUB_RUN_NUMBER where the sweep starts (rotated) +// BABYSIT_VERIFY_GH_TOKEN, BABYSIT_VERIFY_LISTING, BABYSIT_VERIFY_HEAD_REFS, +// BABYSIT_RUN_CACHE the gate reader's Actions token and the run's shared +// reads, as enable-auto-merge passes them import process from "node:process"; import { pathToFileURL } from "node:url"; @@ -96,7 +106,7 @@ import { spoken, } from "../pr-unblock/gate-source.mjs"; import { isProtectedPath } from "../pr-unblock/thread-scope.mjs"; -import { pythonVerify, REFUSAL_MARKER, refusalBody, verifyClears, verifyHold } from "./enable-auto-merge.mjs"; +import { personBodies, pythonVerify, READER_HOLDS, REFUSAL_MARKER, rotated, verifyClears, verifyHold } from "./enable-auto-merge.mjs"; const DEFAULT_BOT = "melodic-automation-lanes[bot]"; const REASON = /^- Reason: `([a-z-]+)`$/m; @@ -127,8 +137,10 @@ export const CLEARS_PER_RUN = 5; // step's timeout-minutes (pr-merge.yml) allows this many (judgment). export const NATIVE_READS_PER_RUN = 3; const REFUSAL_CLEARED = "refusal-cleared"; -// verifyHold's reasons for an answer that refuses: the only ones re-judged. -const JUDGED = ["pr-merge's gate holds it on its risk score (", "pr-merge's gate refuses its verifier verdict (", "pr-merge's lane does not take its issue link ("]; +// babysit_verify_evidence.py's verdicts and risk answer for a read that did +// not finish (UNREADABLE, MISSING, `reviews: "unreadable: …"`). +const UNREAD = Object.freeze(["verify:unreadable", "verify:missing"]); +const UNREAD_REVIEWS = /^unreadable:/; const escapeCommand = (text) => String(text).replaceAll(/[\r\n%]/g, " "); const reportError = (number, error) => @@ -184,15 +196,34 @@ function prMergeEscalations(appComments) { } // enable-auto-merge's person refusal comment, when it is the only pr-merge -// escalation in `merged` and verifyHold wrote its reason: { head, reason }. -function nativeRefusal(appComments, merged, label) { - if (merged.length !== 1 || merged[0].marker !== REFUSAL_MARKER || merged[0].head === null) { +// escalation in `merged`, names `head`, verifyHold wrote its reason, and +// nothing edited it since (enable-auto-merge posts a person's anew): +// { kind, reason }, kind a READER_HOLDS key. +function nativeRefusal(appComments, merged, label, head) { + if (merged.length !== 1 || merged[0].marker !== REFUSAL_MARKER || merged[0].head !== head) { return null; } const newest = appComments.filter((item) => lines(item.body).slice(0, 2).includes(REFUSAL_MARKER)).at(-1); + if (!(Date.parse(newest.updated_at) <= Date.parse(newest.created_at))) { + return null; + } const reason = lines(newest.body).slice(5, -1).join("\n"); - const { head } = merged[0]; - return JUDGED.some((prefix) => reason.startsWith(prefix)) && newest.body === refusalBody(true, reason, head, label) ? { head, reason } : null; + const kind = Object.keys(READER_HOLDS).find((key) => reason.startsWith(READER_HOLDS[key])); + return kind !== undefined && personBodies(reason, head, label).includes(newest.body) ? { kind, reason } : null; +} + +// Whether the reader's `evidence` at `head` lifts a `kind` refusal: nothing +// refuses, or the answer was read whole and what refuses is a lane's for the +// same cause. A risk score is computed only once the link and the verdict +// pass, so a refusal of either hides it. +function liftsRefusal(kind, evidence, head) { + if (verifyHold(evidence, head) === null) { + return true; + } + if (kind === "link" || verifyClears(evidence, head) !== "lane" || UNREAD.includes(evidence.refusal) || UNREAD_REVIEWS.test(evidence.reviews ?? "")) { + return false; + } + return kind === "verdict" || (evidence.refusal === null && evidence.ineligible === null); } // Whether the lanes App comment written last at or before `since` that carries @@ -277,7 +308,8 @@ async function verdict({ github, repo, number, label, bot, mainSha, lanePrs, ver if (unread !== undefined) { return { reason: `the pr-merge comment carrying ${unread.marker} names no head this script can read` }; } - const native = isApp(pull.user, bot) ? nativeRefusal(appComments, merged, label) : null; + // A refusal at an old head takes the head-moved path below. + const native = lanePrs === "native" && isApp(pull.user, bot) ? nativeRefusal(appComments, merged, label, pull.head.sha) : null; if (native !== null) { if (reads.left <= 0) { return { reason: `enable-auto-merge's refusal waits: ${NATIVE_READS_PER_RUN} reader runs this run` }; @@ -290,11 +322,11 @@ async function verdict({ github, repo, number, label, bot, mainSha, lanePrs, ver return { reason: `the gate's reader did not answer: ${escapeCommand(error?.message ?? error)}` }; } const held = verifyHold(evidence, pull.head.sha); - if (held !== null && verifyClears(evidence, pull.head.sha) !== "lane") { + if (!liftsRefusal(native.kind, evidence, pull.head.sha)) { return { reason: `enable-auto-merge's refusal still needs a person at ${pull.head.sha}: ${held}` }; } return cleared(timeline, bot, since, [REFUSAL_CLEARED], pull.head.sha, [ - `it refused \`${native.head}\` (${native.reason})`, + `it refused this head (${native.reason})`, held === null ? "the gate now takes its issue link, verifier verdict and risk score" : `what refuses now is one the lanes clear: ${held}`, ]); } @@ -350,7 +382,7 @@ export async function main({ env = process.env, github, verify } = {}) { ); let failed = 0; let cleared = 0; - for (const { number } of held) { + for (const { number } of rotated(held, Number(env.GITHUB_RUN_NUMBER))) { if (cleared >= CLEARS_PER_RUN) { console.log(`clear-escalations: #${number} not checked: ${CLEARS_PER_RUN} cleared this run`); continue; diff --git a/.github/scripts/pr-merge/clear-escalations.test.mjs b/.github/scripts/pr-merge/clear-escalations.test.mjs index c662056f49..4e63686326 100644 --- a/.github/scripts/pr-merge/clear-escalations.test.mjs +++ b/.github/scripts/pr-merge/clear-escalations.test.mjs @@ -32,7 +32,7 @@ const escalated = (reason = "workflow-staged", { user = BOT, at = "2026-10-18T10 // One open pull request (number 7) labeled needs-human; `timeline` is its timeline and // `mainFiles` the files main changed since its merge base with the head. // `evidence` is what the gate's reader (enable-auto-merge pythonVerify) answers. -function world({ timeline, mainFiles = [{ filename: "README.md" }], mergeable = false, pullFiles = [], author, evidence = () => null }) { +function world({ timeline, mainFiles = [{ filename: "README.md" }], mergeable = false, pullFiles = [], author, evidence = () => null, numbers = [7] }) { const calls = []; const reads = []; const verify = async (pull, rung) => { @@ -46,13 +46,14 @@ function world({ timeline, mainFiles = [{ filename: "README.md" }], mergeable = return { ref: "refs/heads/main", object: { sha: MAIN } }; } if (method === "GET" && route === `/repos/${REPO}/issues?state=open&labels=needs-human`) { - return path.endsWith("page=1") ? [{ number: 7, pull_request: {}, labels: [{ name: "needs-human" }] }] : []; + return path.endsWith("page=1") ? numbers.map((number) => ({ number, pull_request: {}, labels: [{ name: "needs-human" }] })) : []; } - if (method === "GET" && route === `/repos/${REPO}/issues/7/timeline`) { + const [, kind, number] = /^\/repos\/[^/]+\/[^/]+\/(issues|pulls)\/(\d+)(?:\/timeline)?$/.exec(route) ?? []; + if (method === "GET" && kind === "issues" && route.endsWith("/timeline") && numbers.includes(Number(number))) { return path.endsWith("page=1") ? timeline : []; } - if (method === "GET" && route === `/repos/${REPO}/pulls/7`) { - return { number: 7, state: "open", user: author, base: { ref: "main", sha: MAIN }, head: { sha: HEAD }, labels: [{ name: "needs-human" }], mergeable }; + if (method === "GET" && kind === "pulls" && numbers.includes(Number(number))) { + return { number: Number(number), state: "open", user: author, base: { ref: "main", sha: MAIN }, head: { sha: HEAD }, labels: [{ name: "needs-human" }], mergeable }; } if (method === "GET" && route === `/repos/${REPO}/compare/${HEAD}...${MAIN}`) { return { files: mainFiles }; @@ -511,27 +512,64 @@ test("a hold marker from github-actions[bot] with no lane footer, or from anothe // enable-auto-merge's own person refusal comment (refusalBody), then its // label: under merge.lane-prs native the only writer of a lanes App PR's // pr-merge label. #10090's shape: a risk hold on unbound reviews at the head -// the PR still has. +// the PR still has. Reader answers use babysit_verify_evidence.py's own +// constants (FAIL, MISSING, UNREADABLE, the `unreadable:` reviews answer). const UNBOUND = "pr-merge's gate holds it on its risk score (evidence-unbound:claude-review: not bound: rerun-not-allowed)"; +const SECURITY = "pr-merge's gate holds it on its risk score (security-finding: 1 open CRITICAL finding)"; +const REPEATED = "pr-merge's gate refuses its verifier verdict (verify:repeated-fail)"; +const UNTRUSTED = "pr-merge's lane does not take its issue link (untrusted-provenance)"; +const NATIVE = { LANE_PRS: "native" }; const nativeEscalated = (reason, head = HEAD) => mergeEscalated(refusalBody(true, reason, head, "needs-human")); const clean = (pull) => ({ head: pull.headRefOid, refusal: null, ineligible: null, reviews: null }); +const refusing = (refusal) => (pull) => ({ head: pull.headRefOid, refusal, ineligible: null }); +const DELETE = { method: "DELETE", path: `/repos/${REPO}/issues/7/labels/needs-human`, body: undefined }; test("enable-auto-merge's label at the current head comes off once the gate's reader no longer refuses there", async () => { const w = world({ timeline: nativeEscalated(UNBOUND), author: BOT, evidence: clean }); - const { code, logs } = await run(w, { LANE_PRS: "native" }); + const { code, logs } = await run(w, NATIVE); assert.equal(code, 0, logs.join("\n")); assert.deepEqual(w.reads, [{ pull: { number: 7, headRefOid: HEAD, baseRefOid: MAIN }, rung: "C3" }]); const [comment, removal] = w.writes(); assert.equal(w.writes().length, 2); assert.equal(comment.body.body.split("\n")[0], ``); assert.match(comment.body.body, new RegExp(`\`${HEAD}\``)); - assert.deepEqual(removal, { method: "DELETE", path: `/repos/${REPO}/issues/7/labels/needs-human`, body: undefined }); + assert.deepEqual(removal, DELETE); }); -test("enable-auto-merge's label comes off when the refusal is now one the lanes clear", async () => { - const w = world({ timeline: nativeEscalated(UNBOUND), author: BOT, evidence: (pull) => ({ ...clean(pull), refusal: "verify:no-pass", needsHuman: false }) }); - await run(w, { LANE_PRS: "native" }); - assert.equal(w.writes().at(-1)?.method, "DELETE"); +test("enable-auto-merge's label comes off when the reader now reads a refusal the lanes clear for the same cause", async () => { + for (const [reason, evidence] of [ + // A verifier label: any verdict the lanes clear, read whole. + [REPEATED, refusing("verify:fail")], + [REPEATED, refusing("verify:earlier-fail")], + // A risk label: a risk score read whole, now naming only lane causes. + [UNBOUND, (pull) => ({ ...clean(pull), reviews: "evidence-unbound:claude-review: not bound: rerun-allowed" })], + ]) { + const w = world({ timeline: nativeEscalated(reason), author: BOT, evidence }); + await run(w, NATIVE); + assert.deepEqual(w.writes().at(-1), DELETE, `${reason} ${evidence}`); + } +}); + +// A risk score is read only when the verdict passes (babysit_verify_evidence.py +// `evidence`), so a refusal or an unread answer hides it: the label stays. +test("a risk, verifier or issue-link label stays when the reader's answer no longer shows its cause", async () => { + for (const [reason, evidence] of [ + [SECURITY, refusing("verify:unreadable")], + [SECURITY, refusing("verify:missing")], + [SECURITY, refusing("verify:fail")], + [SECURITY, (pull) => ({ ...clean(pull), ineligible: "not-agent-ready" })], + [SECURITY, (pull) => ({ ...clean(pull), reviews: "unreadable: no agent identity" })], + [REPEATED, refusing("verify:unreadable")], + [REPEATED, refusing("verify:missing")], + [REPEATED, (pull) => ({ ...clean(pull), reviews: "unreadable: the lane's rung is off" })], + [UNTRUSTED, (pull) => ({ ...clean(pull), ineligible: "not-agent-ready" })], + [UNTRUSTED, refusing("verify:fail")], + ]) { + const w = world({ timeline: nativeEscalated(reason), author: BOT, evidence }); + await run(w, NATIVE); + assert.equal(w.reads.length, 1, `${reason} ${evidence}`); + assert.deepEqual(w.writes(), [], `${reason} ${evidence}`); + } }); test("enable-auto-merge's label stays while its cause, or another a person must clear, holds at the head", async () => { @@ -539,8 +577,8 @@ test("enable-auto-merge's label stays while its cause, or another a person must // A security finding or a trusted-actor widening stays a person's. (pull) => ({ ...clean(pull), reviews: "security-finding: 1 open CRITICAL finding" }), (pull) => ({ ...clean(pull), reviews: "trust-gate:trusted-actors: widens the trusted actors" }), - // A verifier refusal the gate marks needsHuman, an issue link the lane does not take. - (pull) => ({ ...clean(pull), refusal: "verify:untrusted-spec", needsHuman: true }), + // A verifier refusal a person must clear, an issue link the trust hold refuses. + refusing("verify:untrusted-spec"), (pull) => ({ ...clean(pull), ineligible: "untrusted-provenance" }), // Unread, read at another head, or a read that fails. () => null, @@ -550,7 +588,7 @@ test("enable-auto-merge's label stays while its cause, or another a person must }, ]) { const w = world({ timeline: nativeEscalated(UNBOUND), author: BOT, evidence }); - await run(w, { LANE_PRS: "native" }); + await run(w, NATIVE); assert.deepEqual(w.writes(), [], String(evidence)); } }); @@ -558,7 +596,7 @@ test("enable-auto-merge's label stays while its cause, or another a person must test("enable-auto-merge's hold-path or cost-path label is never judged on the reader's answer", async () => { for (const reason of ["it changes .github/workflows/ci.yml, a hold path", "it changes infra/main.ts, a cost path"]) { const w = world({ timeline: nativeEscalated(reason), author: BOT, evidence: clean }); - await run(w, { LANE_PRS: "native" }); + await run(w, NATIVE); assert.deepEqual(w.writes(), [], reason); assert.deepEqual(w.reads, [], reason); } @@ -570,34 +608,118 @@ test("a label a person added, or added back, beside enable-auto-merge's comment [...nativeEscalated(UNBOUND), unlabeled(OWNER, "2026-10-18T11:00:00Z"), labeled(OWNER, "2026-10-18T11:00:10Z")], ]) { const w = world({ timeline, author: BOT, evidence: clean }); - await run(w, { LANE_PRS: "native" }); + await run(w, NATIVE); assert.deepEqual(w.writes(), []); assert.deepEqual(w.reads, []); } }); -test("a pr-merge person comment not in enable-auto-merge's exact shape is not re-judged", async () => { - const edited = `${refusalBody(true, UNBOUND, HEAD, "needs-human")}\nedited`; - const w = world({ timeline: mergeEscalated(edited), author: BOT, evidence: clean }); - await run(w, { LANE_PRS: "native" }); - assert.deepEqual(w.writes(), []); +test("a pr-merge person comment not in enable-auto-merge's exact shape, or edited since, is not re-judged", async () => { + const body = refusalBody(true, UNBOUND, HEAD, "needs-human"); + for (const timeline of [ + mergeEscalated(`${body}\nedited`), + [commented(BOT, body, "2026-10-18T09:00:00Z", "2026-10-18T09:59:58Z"), labeled(BOT, "2026-10-18T10:00:00Z")], + ]) { + const w = world({ timeline, author: BOT, evidence: clean }); + await run(w, NATIVE); + assert.deepEqual(w.writes(), []); + assert.deepEqual(w.reads, []); + } +}); + +// The person text an earlier enable-auto-merge posted still names the cause. +test("enable-auto-merge's refusal in its earlier wording is judged again", async () => { + const earlier = [ + "", + "", + `Unattended merge: native auto-merge leaves this pull request unarmed at head \`${HEAD}\`. A person must clear this reason, so the pr-merge lane added the \`needs-human\` label: fix the reason, then remove the label, and the next scheduled run judges it again.`, + "", + "```text", + UNBOUND, + "```", + ].join("\n"); + const w = world({ timeline: mergeEscalated(earlier), author: BOT, evidence: clean }); + await run(w, NATIVE); + assert.equal(w.reads.length, 1); + assert.deepEqual(w.writes().at(-1), DELETE); +}); + +// Main's head-moved rule (#9826): no read; enable-auto-merge judges the new +// head and labels it again only for a cause a person must clear. +test("enable-auto-merge's refusal at an old head comes off as head-moved without a read", async () => { + const w = world({ timeline: nativeEscalated(SECURITY, OLD), author: BOT, evidence: clean }); + await run(w, NATIVE); + assert.deepEqual(w.reads, []); + assert.equal(w.writes()[0].body.body.split("\n")[0], ``); + assert.deepEqual(w.writes()[1], DELETE); +}); + +test("enable-auto-merge's comment on a pull request a person opened is not read", async () => { + const w = world({ timeline: nativeEscalated(UNBOUND), author: OWNER, evidence: clean }); + await run(w, NATIVE); assert.deepEqual(w.reads, []); + assert.deepEqual(w.writes(), []); +}); + +test("enable-auto-merge's comment beside another pr-merge escalation is not read", async () => { + for (const other of [verifyBody(HEAD), sharedCopyBody(HEAD)]) { + const timeline = [commented(BOT, other, "2026-10-18T09:59:50Z"), ...nativeEscalated(UNBOUND)]; + const w = world({ timeline, author: BOT, evidence: clean }); + await run(w, NATIVE); + assert.deepEqual(w.reads, [], other); + assert.deepEqual(w.writes(), [], other); + } }); -test("enable-auto-merge's refusal at an old head is judged at the current one", async () => { - const still = world({ timeline: nativeEscalated(UNBOUND, OLD), author: BOT, evidence: (pull) => ({ ...clean(pull), reviews: "security-finding: 1 open CRITICAL finding" }) }); - await run(still, { LANE_PRS: "native" }); - assert.deepEqual(still.writes(), []); - const gone = world({ timeline: nativeEscalated(UNBOUND, OLD), author: BOT, evidence: clean }); - await run(gone, { LANE_PRS: "native" }); - assert.equal(gone.writes()[0].body.body.split("\n")[0], ``); +test("enable-auto-merge's comment is not read under merge.lane-prs gate", async () => { + for (const env of [{ LANE_PRS: "gate" }, {}]) { + const w = world({ timeline: nativeEscalated(UNBOUND), author: BOT, evidence: clean }); + await run(w, env); + assert.deepEqual(w.reads, [], JSON.stringify(env)); + assert.deepEqual(w.writes(), [], JSON.stringify(env)); + } +}); + +// enable-auto-merge puts the label back under its standing comment after +// this script took it off; the put-back is judged again like the first. +test("a label enable-auto-merge put back after this script cleared it is judged again", async () => { + const timeline = [ + ...nativeEscalated(UNBOUND), + commented(BOT, `\ncleared`, "2026-10-18T11:00:00Z"), + unlabeled(BOT, "2026-10-18T11:00:01Z"), + labeled(BOT, "2026-10-18T12:00:00Z"), + ]; + const kept = world({ timeline, author: BOT, evidence: (pull) => ({ ...clean(pull), reviews: "security-finding: 1 open CRITICAL finding" }) }); + await run(kept, NATIVE); + assert.equal(kept.reads.length, 1); + assert.deepEqual(kept.writes(), []); + const gone = world({ timeline, author: BOT, evidence: clean }); + await run(gone, NATIVE); + assert.equal(gone.writes().length, 2); + assert.deepEqual(gone.writes()[1], DELETE); }); test("at most NATIVE_READS_PER_RUN reader runs a run", async () => { const w = world({ timeline: nativeEscalated(UNBOUND), author: BOT, evidence: () => null }); const held = Array.from({ length: NATIVE_READS_PER_RUN + 2 }, (_, i) => ({ number: 7, pull_request: {}, i })); const github = async (method, path, body) => (method === "GET" && path.includes("issues?state=open") ? (path.endsWith("page=1") ? held : []) : w.github(method, path, body)); - const { logs } = await run({ ...w, github }); + const { logs } = await run({ ...w, github }, NATIVE); assert.equal(w.reads.length, NATIVE_READS_PER_RUN); assert.equal(logs.filter((line) => /reader runs this run/.test(line)).length, 2); }); + +// More labels held for a person than reads a run: the start moves with the +// run number, so each one is read within as many runs as there are labels. +test("labels past the read cap are read on a later run", async () => { + const numbers = Array.from({ length: NATIVE_READS_PER_RUN + 1 }, (_, i) => 7 + i); + const read = new Set(); + for (let runNumber = 1; runNumber <= numbers.length; runNumber += 1) { + const w = world({ timeline: nativeEscalated(SECURITY), author: BOT, evidence: (pull) => ({ ...clean(pull), reviews: "security-finding: 1 open CRITICAL finding" }), numbers }); + await run(w, { ...NATIVE, GITHUB_RUN_NUMBER: String(runNumber) }); + assert.equal(w.reads.length, NATIVE_READS_PER_RUN); + for (const { pull } of w.reads) { + read.add(pull.number); + } + } + assert.deepEqual([...read].sort((a, b) => a - b), numbers); +}); diff --git a/.github/scripts/pr-merge/enable-auto-merge.mjs b/.github/scripts/pr-merge/enable-auto-merge.mjs index a99d5ca353..f946defc62 100755 --- a/.github/scripts/pr-merge/enable-auto-merge.mjs +++ b/.github/scripts/pr-merge/enable-auto-merge.mjs @@ -99,8 +99,10 @@ // denied path or the floor, which only a trusted approval lifts; and the // lane's own hold line, owner decision Q-HOLDLINE) also gets the human-gated // label, put back while the comment stands unless a person removed it -// since. clear-escalations takes the label off once the head moves, and -// this run labels the new head again only if its cause still holds. Holds a +// since. clear-escalations takes the label off once the head moves, or, for +// an issue-link, verifier or risk reason, once the gate's reader at the same +// head reads the cause gone; this run labels it again only if its cause +// still holds. Holds a // person already set (labels, their own body hold, a veto), the room and // failed reads write nothing. // Arming passes the head just read as expectedHeadOid, so a push in between @@ -424,6 +426,15 @@ async function changedFiles(github, slug, number) { return null; } +// How verifyHold opens the reason for each answer that refuses: the issue +// link, the verifier verdict and the risk score. +export const READER_HOLDS = Object.freeze({ + link: "pr-merge's lane does not take its issue link (", + verdict: "pr-merge's gate refuses its verifier verdict (", + risk: "pr-merge's gate holds it on its risk score (", +}); +const readerHold = (reason) => Object.values(READER_HOLDS).some((prefix) => String(reason).startsWith(prefix)); + // Why the gate's `evidence --lane-link` (babysit_verify_evidence.py's report) // keeps a pull request at `head` from merging, or null when it allows it. export function verifyHold(evidence, head) { @@ -434,17 +445,17 @@ export function verifyHold(evidence, head) { return "its verifier verdict was read at another head"; } if (typeof evidence.ineligible === "string") { - return `pr-merge's lane does not take its issue link (${escapeCommand(evidence.ineligible)})`; + return `${READER_HOLDS.link}${escapeCommand(evidence.ineligible)})`; } if (evidence.refusal !== null) { - return `pr-merge's gate refuses its verifier verdict (${escapeCommand(evidence.refusal)})`; + return `${READER_HOLDS.verdict}${escapeCommand(evidence.refusal)})`; } // A line without the link's or the reviews' answer read neither: it allows nothing. if (evidence.ineligible !== null) { return "its issue link could not be read"; } if (typeof evidence.reviews === "string") { - return `pr-merge's gate holds it on its risk score (${escapeCommand(evidence.reviews)})`; + return `${READER_HOLDS.risk}${escapeCommand(evidence.reviews)})`; } return evidence.reviews === null ? null : "its risk score could not be read"; } @@ -503,13 +514,23 @@ export function ejectionHold(evidence) { // refusal the interactive queue's escalation marker first (work-items // escalation-marker.md), as the gate's Dependabot escalation carries it. A // function of the class, the reason and the head only, so an unchanged -// refusal renders the same body. +// refusal renders the same body. A person's reader refusal (READER_HOLDS) +// says pr-merge takes the label off once it reads the reason gone; any other +// person's reason, which only a push or a person lifts, says to remove it. export const REFUSAL_MARKER = ""; export const ATTEND_QUEUE_MARKER = ""; +const LANE_WHO = "The lanes clear this reason on their own; the next scheduled run judges it again."; +const personWho = (label) => + `A person must clear this reason, so the pr-merge lane added the \`${label}\` label: fix the reason, then remove the label, and the next scheduled run judges it again.`; +const readerWho = (label) => + `A person must clear this reason, so the pr-merge lane added the \`${label}\` label: fix the reason, and pr-merge takes the label off once a scheduled run reads it gone at this head, or once the head moves, then judges it again.`; export function refusalBody(person, reason, head, label) { - const who = person - ? `A person must clear this reason, so the pr-merge lane added the \`${label}\` label: fix the reason, then remove the label, and the next scheduled run judges it again.` - : "The lanes clear this reason on their own; the next scheduled run judges it again."; + return refusalText(person, person ? (readerHold(reason) ? readerWho : personWho)(label) : LANE_WHO, reason, head); +} +// Each body a person's refusal of `reason` at `head` may stand in: the +// current wording, then a reader refusal's earlier one. +export const personBodies = (reason, head, label) => [...new Set([refusalBody(true, reason, head, label), refusalText(true, personWho(label), reason, head)])]; +function refusalText(person, who, reason, head) { return [ ...(person ? [ATTEND_QUEUE_MARKER] : []), REFUSAL_MARKER, @@ -567,9 +588,13 @@ export function refusalWrites(pull, comments, { clears, reason, comment }, { bot // A person's comment already standing while the label is off (a pull // request carrying it never gets here: holdReason holds it): the label // goes back on unless a person removed it after the comment last changed, - // which the caller reads from the timeline (`liftedSince`). + // which the caller reads from the timeline (`liftedSince`). Either wording + // of a person's comment stands (personBodies). + if (body === person && personBodies(reason, pull.headRefOid, label).includes(prior?.body)) { + return { body: null, commentId: prior.databaseId ?? null, label, liftedSince: String(prior.updatedAt ?? prior.createdAt) }; + } if (prior?.body === body) { - return body === person ? { body: null, commentId: prior.databaseId ?? null, label, liftedSince: String(prior.updatedAt ?? prior.createdAt) } : null; + return null; } // An escalation to a person posts a new comment: an edit notifies no one // and keeps the old date, so the label would stand unexplained. diff --git a/.github/scripts/pr-merge/enable-auto-merge.test.mjs b/.github/scripts/pr-merge/enable-auto-merge.test.mjs index 71319c2fe7..2ca6a1870e 100644 --- a/.github/scripts/pr-merge/enable-auto-merge.test.mjs +++ b/.github/scripts/pr-merge/enable-auto-merge.test.mjs @@ -256,11 +256,16 @@ const laneNote = (reason) => reason, "```", ].join("\n"); -const personNote = (reason) => +// A reader refusal (issue link, verdict, risk score) is one clear-escalations +// takes the label off for at the same head; any other a person removes. +const READER_REASON = /^pr-merge's (?:lane does not take its issue link|gate refuses its verifier verdict|gate holds it on its risk score) \(/; +const PERSON_WHO = "fix the reason, then remove the label, and the next scheduled run judges it again."; +const READER_WHO = "fix the reason, and pr-merge takes the label off once a scheduled run reads it gone at this head, or once the head moves, then judges it again."; +const personNote = (reason, who = READER_REASON.test(reason) ? READER_WHO : PERSON_WHO) => [ "", "", - `Unattended merge: native auto-merge leaves this pull request unarmed at head \`${HEAD}\`. A person must clear this reason, so the pr-merge lane added the \`needs-human\` label: fix the reason, then remove the label, and the next scheduled run judges it again.`, + `Unattended merge: native auto-merge leaves this pull request unarmed at head \`${HEAD}\`. A person must clear this reason, so the pr-merge lane added the \`needs-human\` label: ${who}`, "", "```text", reason, @@ -460,6 +465,21 @@ test("an unchanged person's refusal is not posted again", () => { assert.deepEqual(writes, { body: null, commentId: 77, label: "needs-human", liftedSince: "2026-10-23T10:00:00Z" }); }); +// A person's reader refusal posted in the earlier wording still stands: no +// second comment, and the label goes back as for the current wording. +test("a person's reader refusal in its earlier wording is not posted again", () => { + const reason = "pr-merge's gate holds it on its risk score (security-finding: 1 CRITICAL)"; + const writes = refusalWrites( + { headRefOid: HEAD }, + [lanesSaid(personNote(reason, PERSON_WHO))], + { clears: "person", reason }, + { bot: LANES.login, label: "needs-human" }, + ); + assert.deepEqual(writes, { body: null, commentId: 77, label: "needs-human", liftedSince: "2026-10-23T10:00:00Z" }); + assert.match(refusalBody(true, reason, HEAD, "needs-human"), /pr-merge takes the label off/); + assert.doesNotMatch(refusalBody(true, reason, HEAD, "needs-human"), /then remove the label/); +}); + test("a lane's refusal over a person's comment still edits it, with no label", () => { const writes = refusalWrites( { headRefOid: HEAD }, diff --git a/.github/workflows/pr-merge.yml b/.github/workflows/pr-merge.yml index 5e2e115f7a..47d1f73a8e 100644 --- a/.github/workflows/pr-merge.yml +++ b/.github/workflows/pr-merge.yml @@ -1324,9 +1324,10 @@ jobs: # reason, feedback-protected-path), after a comment naming the reason # and the head: never a label a person set, and never after anyone but # the App commented or reviewed since it went on. At most five a run, and not - # while the lane is report-only. A label enable-auto-merge set is judged - # again with the gate's reader at the head, at most three reads a run - # of up to four minutes each. + # while the lane is report-only. Under merge.lane-prs native, a label + # enable-auto-merge set at the current head is judged again with the + # gate's reader, at most three reads a run of up to four minutes each, + # sharing the gate step's run cache, listing and head-ref map. # Rules: .github/scripts/pr-merge/clear-escalations.mjs. - name: Clear escalations whose reason no longer holds if: >- @@ -1337,6 +1338,9 @@ jobs: env: GH_TOKEN: ${{ steps.lane-token.outputs.token }} BABYSIT_VERIFY_GH_TOKEN: ${{ github.token }} + BABYSIT_VERIFY_LISTING: ${{ runner.temp }}/verify-listing.json + BABYSIT_VERIFY_HEAD_REFS: ${{ runner.temp }}/head-refs-in.json + BABYSIT_RUN_CACHE: ${{ runner.temp }}/babysit-run-cache.json API_BUDGET_TOKEN: lanes-app API_BUDGET_OVERRIDE_TOKEN: github-token KILL_SWITCH: ${{ steps.kill.outputs.proceed == 'true' && 'proceed' || '' }} From 9d04e31666156a8df67f1670c2520d2fb020b623 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 26 Oct 2026 05:50:33 -0400 Subject: [PATCH 3/8] fix(pr-merge): keep a verifier label when the reader stops at the issue link The reader answers an ineligible link early, with verify:ineligible and no verdict, so a lane-class link answer no longer lifts a verifier label. The reader-refusal wording now says the same-head clear happens only under merge.lane-prs native, since enable-auto-merge also runs under gate. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/pr-merge/clear-escalations.mjs | 16 +++++++++------- .../scripts/pr-merge/clear-escalations.test.mjs | 9 ++++++--- .github/scripts/pr-merge/enable-auto-merge.mjs | 2 +- .../scripts/pr-merge/enable-auto-merge.test.mjs | 2 +- 4 files changed, 17 insertions(+), 12 deletions(-) diff --git a/.github/scripts/pr-merge/clear-escalations.mjs b/.github/scripts/pr-merge/clear-escalations.mjs index ba63bdd5bd..7425d6b8f0 100755 --- a/.github/scripts/pr-merge/clear-escalations.mjs +++ b/.github/scripts/pr-merge/clear-escalations.mjs @@ -62,9 +62,10 @@ // config's rung) runs, and the label comes off as `refusal-cleared` when // nothing refuses, or when the answer was read whole and what refuses now // is a lane's (verifyClears `lane`) for the same cause: for a verifier -// reason a verdict that is neither unreadable nor missing, for a risk reason -// a risk score, which the reader computes only once the link and verdict -// pass. An issue-link reason comes off only when nothing refuses. A person's +// reason a verdict, read once the link passes, that is neither unreadable +// nor missing; for a risk reason a risk score, which the reader computes +// only once the link and verdict pass. An issue-link reason comes off only +// when nothing refuses. A person's // refusal, an unread answer and a failed read keep it; so does a hold or // cost path, which no reader answer clears. At most NATIVE_READS_PER_RUN // reads a run, starting at an offset that moves with the run number @@ -214,16 +215,17 @@ function nativeRefusal(appComments, merged, label, head) { // Whether the reader's `evidence` at `head` lifts a `kind` refusal: nothing // refuses, or the answer was read whole and what refuses is a lane's for the -// same cause. A risk score is computed only once the link and the verdict -// pass, so a refusal of either hides it. +// same cause. The reader reads the verdict only once the link passes, and +// the risk score only once the verdict passes too, so a refusal of either +// hides what comes after it. function liftsRefusal(kind, evidence, head) { if (verifyHold(evidence, head) === null) { return true; } - if (kind === "link" || verifyClears(evidence, head) !== "lane" || UNREAD.includes(evidence.refusal) || UNREAD_REVIEWS.test(evidence.reviews ?? "")) { + if (kind === "link" || evidence.ineligible !== null || verifyClears(evidence, head) !== "lane" || UNREAD.includes(evidence.refusal) || UNREAD_REVIEWS.test(evidence.reviews ?? "")) { return false; } - return kind === "verdict" || (evidence.refusal === null && evidence.ineligible === null); + return kind === "verdict" || evidence.refusal === null; } // Whether the lanes App comment written last at or before `since` that carries diff --git a/.github/scripts/pr-merge/clear-escalations.test.mjs b/.github/scripts/pr-merge/clear-escalations.test.mjs index 4e63686326..f6885979ce 100644 --- a/.github/scripts/pr-merge/clear-escalations.test.mjs +++ b/.github/scripts/pr-merge/clear-escalations.test.mjs @@ -522,6 +522,8 @@ const NATIVE = { LANE_PRS: "native" }; const nativeEscalated = (reason, head = HEAD) => mergeEscalated(refusalBody(true, reason, head, "needs-human")); const clean = (pull) => ({ head: pull.headRefOid, refusal: null, ineligible: null, reviews: null }); const refusing = (refusal) => (pull) => ({ head: pull.headRefOid, refusal, ineligible: null }); +// The reader's early answer for a link it does not take: no verdict, no risk score. +const ineligible = (why) => (pull) => ({ head: pull.headRefOid, refusal: "verify:ineligible", ineligible: why }); const DELETE = { method: "DELETE", path: `/repos/${REPO}/issues/7/labels/needs-human`, body: undefined }; test("enable-auto-merge's label at the current head comes off once the gate's reader no longer refuses there", async () => { @@ -557,12 +559,13 @@ test("a risk, verifier or issue-link label stays when the reader's answer no lon [SECURITY, refusing("verify:unreadable")], [SECURITY, refusing("verify:missing")], [SECURITY, refusing("verify:fail")], - [SECURITY, (pull) => ({ ...clean(pull), ineligible: "not-agent-ready" })], + [SECURITY, ineligible("not-agent-ready")], [SECURITY, (pull) => ({ ...clean(pull), reviews: "unreadable: no agent identity" })], [REPEATED, refusing("verify:unreadable")], [REPEATED, refusing("verify:missing")], + [REPEATED, ineligible("not-agent-ready")], [REPEATED, (pull) => ({ ...clean(pull), reviews: "unreadable: the lane's rung is off" })], - [UNTRUSTED, (pull) => ({ ...clean(pull), ineligible: "not-agent-ready" })], + [UNTRUSTED, ineligible("not-agent-ready")], [UNTRUSTED, refusing("verify:fail")], ]) { const w = world({ timeline: nativeEscalated(reason), author: BOT, evidence }); @@ -579,7 +582,7 @@ test("enable-auto-merge's label stays while its cause, or another a person must (pull) => ({ ...clean(pull), reviews: "trust-gate:trusted-actors: widens the trusted actors" }), // A verifier refusal a person must clear, an issue link the trust hold refuses. refusing("verify:untrusted-spec"), - (pull) => ({ ...clean(pull), ineligible: "untrusted-provenance" }), + ineligible("untrusted-provenance"), // Unread, read at another head, or a read that fails. () => null, () => ({ ...clean({ headRefOid: OLD }) }), diff --git a/.github/scripts/pr-merge/enable-auto-merge.mjs b/.github/scripts/pr-merge/enable-auto-merge.mjs index f946defc62..fc15593169 100755 --- a/.github/scripts/pr-merge/enable-auto-merge.mjs +++ b/.github/scripts/pr-merge/enable-auto-merge.mjs @@ -523,7 +523,7 @@ const LANE_WHO = "The lanes clear this reason on their own; the next scheduled r const personWho = (label) => `A person must clear this reason, so the pr-merge lane added the \`${label}\` label: fix the reason, then remove the label, and the next scheduled run judges it again.`; const readerWho = (label) => - `A person must clear this reason, so the pr-merge lane added the \`${label}\` label: fix the reason, and pr-merge takes the label off once a scheduled run reads it gone at this head, or once the head moves, then judges it again.`; + `A person must clear this reason, so the pr-merge lane added the \`${label}\` label: fix the reason, and pr-merge takes the label off once the head moves or, under \`merge.lane-prs: native\`, once a scheduled run reads the reason gone at this head; the next scheduled run then judges it again.`; export function refusalBody(person, reason, head, label) { return refusalText(person, person ? (readerHold(reason) ? readerWho : personWho)(label) : LANE_WHO, reason, head); } diff --git a/.github/scripts/pr-merge/enable-auto-merge.test.mjs b/.github/scripts/pr-merge/enable-auto-merge.test.mjs index 2ca6a1870e..b1db3ff89a 100644 --- a/.github/scripts/pr-merge/enable-auto-merge.test.mjs +++ b/.github/scripts/pr-merge/enable-auto-merge.test.mjs @@ -260,7 +260,7 @@ const laneNote = (reason) => // takes the label off for at the same head; any other a person removes. const READER_REASON = /^pr-merge's (?:lane does not take its issue link|gate refuses its verifier verdict|gate holds it on its risk score) \(/; const PERSON_WHO = "fix the reason, then remove the label, and the next scheduled run judges it again."; -const READER_WHO = "fix the reason, and pr-merge takes the label off once a scheduled run reads it gone at this head, or once the head moves, then judges it again."; +const READER_WHO = "fix the reason, and pr-merge takes the label off once the head moves or, under `merge.lane-prs: native`, once a scheduled run reads the reason gone at this head; the next scheduled run then judges it again."; const personNote = (reason, who = READER_REASON.test(reason) ? READER_WHO : PERSON_WHO) => [ "", From 6c4b37179ea1a817ca4e9b6111c339b60ad9677e Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 26 Oct 2026 06:17:20 -0400 Subject: [PATCH 4/8] fix(pr-merge): clear a person's recorded cause only when nothing refuses A lane-class reader answer can hide a cause a person must clear: an unbound security review never reads its findings, a partial file listing never checks cost, trust or high paths, and a verdict that is not a pass never checks the spec's trust. A label whose recorded reason carries a PERSON_RISK code or a PERSON_REFUSALS verdict now comes off only when the reader refuses nothing; the lane-answer relaxation stays for recorded causes the lanes clear, such as #10090's evidence-unbound label. Co-Authored-By: Claude Opus 5.5 --- .../scripts/pr-merge/clear-escalations.mjs | 42 +++++++++++-------- .../pr-merge/clear-escalations.test.mjs | 28 +++++++++++-- .../scripts/pr-merge/enable-auto-merge.mjs | 2 +- 3 files changed, 51 insertions(+), 21 deletions(-) diff --git a/.github/scripts/pr-merge/clear-escalations.mjs b/.github/scripts/pr-merge/clear-escalations.mjs index 7425d6b8f0..ae19c928e2 100755 --- a/.github/scripts/pr-merge/clear-escalations.mjs +++ b/.github/scripts/pr-merge/clear-escalations.mjs @@ -60,14 +60,16 @@ // again at that head: the gate's reader (`evidence`, as enable-auto-merge // reads it, the room's rung cap passed as C3 so the scorer answers at the // config's rung) runs, and the label comes off as `refusal-cleared` when -// nothing refuses, or when the answer was read whole and what refuses now -// is a lane's (verifyClears `lane`) for the same cause: for a verifier -// reason a verdict, read once the link passes, that is neither unreadable -// nor missing; for a risk reason a risk score, which the reader computes -// only once the link and verdict pass. An issue-link reason comes off only -// when nothing refuses. A person's -// refusal, an unread answer and a failed read keep it; so does a hold or -// cost path, which no reader answer clears. At most NATIVE_READS_PER_RUN +// nothing refuses. A recorded cause the lanes clear (a label set before it +// became a lane's) also comes off when the answer was read whole and what +// refuses now is a lane's (verifyClears `lane`) for the same cause: for a +// verifier reason a verdict, read once the link passes, that is neither +// unreadable nor missing; for a risk reason a risk score, which the reader +// computes only once the link and verdict pass. An issue-link reason, and a +// recorded cause a person must clear (PERSON_RISK, PERSON_REFUSALS), come +// off only when nothing refuses. A person's refusal, an unread answer and a +// failed read keep it; so does a hold or cost path, which no reader answer +// clears. At most NATIVE_READS_PER_RUN // reads a run, starting at an offset that moves with the run number // (rotated), so labels past the cap are read on a later run. // @@ -107,7 +109,7 @@ import { spoken, } from "../pr-unblock/gate-source.mjs"; import { isProtectedPath } from "../pr-unblock/thread-scope.mjs"; -import { personBodies, pythonVerify, READER_HOLDS, REFUSAL_MARKER, rotated, verifyClears, verifyHold } from "./enable-auto-merge.mjs"; +import { PERSON_REFUSALS, PERSON_RISK, personBodies, pythonVerify, READER_HOLDS, REFUSAL_MARKER, rotated, verifyClears, verifyHold } from "./enable-auto-merge.mjs"; const DEFAULT_BOT = "melodic-automation-lanes[bot]"; const REASON = /^- Reason: `([a-z-]+)`$/m; @@ -213,16 +215,22 @@ function nativeRefusal(appComments, merged, label, head) { return kind !== undefined && personBodies(reason, head, label).includes(newest.body) ? { kind, reason } : null; } -// Whether the reader's `evidence` at `head` lifts a `kind` refusal: nothing -// refuses, or the answer was read whole and what refuses is a lane's for the -// same cause. The reader reads the verdict only once the link passes, and -// the risk score only once the verdict passes too, so a refusal of either -// hides what comes after it. -function liftsRefusal(kind, evidence, head) { +// Whether the reader's `evidence` at `head` lifts the `kind` refusal recorded +// as `reason`: nothing refuses, or, for a recorded cause the lanes clear (a +// label set before that cause became a lane's), the answer was read whole and +// what refuses is a lane's for the same cause. A recorded cause a person must +// clear (PERSON_RISK, PERSON_REFUSALS) lifts only when nothing refuses: an +// unbound review or a partial file listing is a lane-class answer that never +// checked it, and the spec's trust is checked only on a pass. The reader reads +// the verdict only once the link passes, and the risk score only once the +// verdict passes too, so a refusal of either hides what comes after it. +function liftsRefusal(kind, reason, evidence, head) { if (verifyHold(evidence, head) === null) { return true; } - if (kind === "link" || evidence.ineligible !== null || verifyClears(evidence, head) !== "lane" || UNREAD.includes(evidence.refusal) || UNREAD_REVIEWS.test(evidence.reviews ?? "")) { + const recorded = reason.slice(READER_HOLDS[kind].length, -1); + const person = kind === "risk" ? PERSON_RISK.test(recorded) : PERSON_REFUSALS.includes(recorded); + if (kind === "link" || person || evidence.ineligible !== null || verifyClears(evidence, head) !== "lane" || UNREAD.includes(evidence.refusal) || UNREAD_REVIEWS.test(evidence.reviews ?? "")) { return false; } return kind === "verdict" || evidence.refusal === null; @@ -324,7 +332,7 @@ async function verdict({ github, repo, number, label, bot, mainSha, lanePrs, ver return { reason: `the gate's reader did not answer: ${escapeCommand(error?.message ?? error)}` }; } const held = verifyHold(evidence, pull.head.sha); - if (!liftsRefusal(native.kind, evidence, pull.head.sha)) { + if (!liftsRefusal(native.kind, native.reason, evidence, pull.head.sha)) { return { reason: `enable-auto-merge's refusal still needs a person at ${pull.head.sha}: ${held}` }; } return cleared(timeline, bot, since, [REFUSAL_CLEARED], pull.head.sha, [ diff --git a/.github/scripts/pr-merge/clear-escalations.test.mjs b/.github/scripts/pr-merge/clear-escalations.test.mjs index f6885979ce..6a1027ebf9 100644 --- a/.github/scripts/pr-merge/clear-escalations.test.mjs +++ b/.github/scripts/pr-merge/clear-escalations.test.mjs @@ -518,6 +518,10 @@ const UNBOUND = "pr-merge's gate holds it on its risk score (evidence-unbound:cl const SECURITY = "pr-merge's gate holds it on its risk score (security-finding: 1 open CRITICAL finding)"; const REPEATED = "pr-merge's gate refuses its verifier verdict (verify:repeated-fail)"; const UNTRUSTED = "pr-merge's lane does not take its issue link (untrusted-provenance)"; +const UNTRUSTED_SPEC = "pr-merge's gate refuses its verifier verdict (verify:untrusted-spec)"; +const COST = "pr-merge's gate holds it on its risk score (cost-review-required: infra/main.tf)"; +// Labels enable-auto-merge set before #9826 made these lane causes. +const SPEC_CHANGED = "pr-merge's gate refuses its verifier verdict (verify:spec-changed)"; const NATIVE = { LANE_PRS: "native" }; const nativeEscalated = (reason, head = HEAD) => mergeEscalated(refusalBody(true, reason, head, "needs-human")); const clean = (pull) => ({ head: pull.headRefOid, refusal: null, ineligible: null, reviews: null }); @@ -540,9 +544,9 @@ test("enable-auto-merge's label at the current head comes off once the gate's re test("enable-auto-merge's label comes off when the reader now reads a refusal the lanes clear for the same cause", async () => { for (const [reason, evidence] of [ - // A verifier label: any verdict the lanes clear, read whole. - [REPEATED, refusing("verify:fail")], - [REPEATED, refusing("verify:earlier-fail")], + // A lane-class verifier label: any verdict the lanes clear, read whole. + [SPEC_CHANGED, refusing("verify:fail")], + [SPEC_CHANGED, refusing("verify:earlier-fail")], // A risk label: a risk score read whole, now naming only lane causes. [UNBOUND, (pull) => ({ ...clean(pull), reviews: "evidence-unbound:claude-review: not bound: rerun-allowed" })], ]) { @@ -567,6 +571,14 @@ test("a risk, verifier or issue-link label stays when the reader's answer no lon [REPEATED, (pull) => ({ ...clean(pull), reviews: "unreadable: the lane's rung is off" })], [UNTRUSTED, ineligible("not-agent-ready")], [UNTRUSTED, refusing("verify:fail")], + [SPEC_CHANGED, refusing("verify:unreadable")], + // A person's recorded cause clears only when nothing refuses: an unbound + // review or a partial file listing never read it, and a verdict that is not + // a pass never checks the spec's trust. + [SECURITY, (pull) => ({ ...clean(pull), reviews: "evidence-unbound:claude-security-review: not bound: threads-unread" })], + [COST, (pull) => ({ ...clean(pull), reviews: "files-incomplete: the changed-file listing did not read whole" })], + [UNTRUSTED_SPEC, refusing("verify:fail")], + [REPEATED, refusing("verify:fail")], ]) { const w = world({ timeline: nativeEscalated(reason), author: BOT, evidence }); await run(w, NATIVE); @@ -674,6 +686,16 @@ test("enable-auto-merge's comment beside another pr-merge escalation is not read } }); +test("another pr-merge escalation alone at the head is not read as enable-auto-merge's", async () => { + for (const other of [verifyBody(HEAD), sharedCopyBody(HEAD)]) { + const w = world({ timeline: mergeEscalated(other), author: BOT, evidence: clean }); + const { code, logs } = await run(w, NATIVE); + assert.equal(code, 0, logs.join("\n")); + assert.deepEqual(w.reads, [], other); + assert.deepEqual(w.writes(), [], other); + } +}); + test("enable-auto-merge's comment is not read under merge.lane-prs gate", async () => { for (const env of [{ LANE_PRS: "gate" }, {}]) { const w = world({ timeline: nativeEscalated(UNBOUND), author: BOT, evidence: clean }); diff --git a/.github/scripts/pr-merge/enable-auto-merge.mjs b/.github/scripts/pr-merge/enable-auto-merge.mjs index fc15593169..cd85d9acfa 100755 --- a/.github/scripts/pr-merge/enable-auto-merge.mjs +++ b/.github/scripts/pr-merge/enable-auto-merge.mjs @@ -469,7 +469,7 @@ export function verifyHold(evidence, head) { // (babysit_verify_evidence.lane_risk_refusal). export const PERSON_REFUSALS = Object.freeze(["verify:untrusted-spec", "verify:repeated-fail"]); export const TRUST_REFUSALS = Object.freeze(["untrusted-provenance", "untrusted-provenance-removed-by-non-human", "issue-labels-unreadable"]); -const PERSON_RISK = /(?:^|; )(?:security-finding|trust-gate:[a-z-]+|cost-review-required|high-path:\*\*):/; +export const PERSON_RISK = /(?:^|; )(?:security-finding|trust-gate:[a-z-]+|cost-review-required|high-path:\*\*):/; // Who clears the refusal verifyHold reads in `evidence` at `head`: `person` // for a PERSON_REFUSALS refusal, an issue link the trust hold refuses From 2fcd8c9c4d0c490249ef11a9898c4e544e13dea6 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 26 Oct 2026 07:05:38 -0400 Subject: [PATCH 5/8] fix(pr-merge): lift a lane answer only for recorded causes on an explicit lane list clear-escalations now treats a recorded refusal as a lane's only when every "; "-separated risk code, or the verdict, is on LANE_CAUSES: the holding lane codes babysit_risk.py emits (files-incomplete, evidence-unbound:, the manifest review), the reader's unread score, and every verifier refusal but untrusted-spec and repeated-fail. Any other code, an unknown one or a configured high path included, and every issue link, comes off only when the reader refuses nothing. The check no longer reads enable-auto-merge's PERSON_RISK or PERSON_REFUSALS, so it fails closed whatever those lists become. Co-Authored-By: Claude Opus 5.5 --- .../scripts/pr-merge/clear-escalations.mjs | 59 +++++++++++++------ .../pr-merge/clear-escalations.test.mjs | 14 +++++ .../scripts/pr-merge/enable-auto-merge.mjs | 2 +- 3 files changed, 55 insertions(+), 20 deletions(-) diff --git a/.github/scripts/pr-merge/clear-escalations.mjs b/.github/scripts/pr-merge/clear-escalations.mjs index ae19c928e2..a10df6568c 100755 --- a/.github/scripts/pr-merge/clear-escalations.mjs +++ b/.github/scripts/pr-merge/clear-escalations.mjs @@ -60,17 +60,17 @@ // again at that head: the gate's reader (`evidence`, as enable-auto-merge // reads it, the room's rung cap passed as C3 so the scorer answers at the // config's rung) runs, and the label comes off as `refusal-cleared` when -// nothing refuses. A recorded cause the lanes clear (a label set before it -// became a lane's) also comes off when the answer was read whole and what -// refuses now is a lane's (verifyClears `lane`) for the same cause: for a -// verifier reason a verdict, read once the link passes, that is neither -// unreadable nor missing; for a risk reason a risk score, which the reader -// computes only once the link and verdict pass. An issue-link reason, and a -// recorded cause a person must clear (PERSON_RISK, PERSON_REFUSALS), come -// off only when nothing refuses. A person's refusal, an unread answer and a -// failed read keep it; so does a hold or cost path, which no reader answer -// clears. At most NATIVE_READS_PER_RUN -// reads a run, starting at an offset that moves with the run number +// nothing refuses. A recorded cause the lanes clear (every code on +// LANE_CAUSES; a label set before it became a lane's) also comes off when the +// answer was read whole and what refuses now is a lane's (verifyClears +// `lane`) for the same cause: for a verifier reason a verdict, read once the +// link passes, that is neither unreadable nor missing; for a risk reason a +// risk score, which the reader computes only once the link and verdict pass. +// An issue-link reason, and any recorded code off that list, an unknown one +// included, come off only when nothing refuses. A person's refusal, an unread +// answer and a failed read keep it; so does a hold or cost path, which no +// reader answer clears. At most NATIVE_READS_PER_RUN reads a run, starting at +// an offset that moves with the run number // (rotated), so labels past the cap are read on a later run. // // Environment: @@ -109,7 +109,7 @@ import { spoken, } from "../pr-unblock/gate-source.mjs"; import { isProtectedPath } from "../pr-unblock/thread-scope.mjs"; -import { PERSON_REFUSALS, PERSON_RISK, personBodies, pythonVerify, READER_HOLDS, REFUSAL_MARKER, rotated, verifyClears, verifyHold } from "./enable-auto-merge.mjs"; +import { personBodies, pythonVerify, READER_HOLDS, REFUSAL_MARKER, rotated, verifyClears, verifyHold } from "./enable-auto-merge.mjs"; const DEFAULT_BOT = "melodic-automation-lanes[bot]"; const REASON = /^- Reason: `([a-z-]+)`$/m; @@ -144,6 +144,29 @@ const REFUSAL_CLEARED = "refusal-cleared"; // not finish (UNREADABLE, MISSING, `reviews: "unreadable: …"`). const UNREAD = Object.freeze(["verify:unreadable", "verify:missing"]); const UNREAD_REVIEWS = /^unreadable:/; +// The recorded causes the lanes clear, the only ones a lane-class answer may +// lift; every other code, an unknown one included, is a person's. Risk codes +// (an entry ending in ":" is a prefix): babysit_risk.py's holding MED codes +// files-incomplete and evidence-unbound:, the manifest review, and the +// reader's own unread score (babysit_verify_evidence.py lane_risk_refusal). +// Verdicts: babysit_verify_evidence.py's refusals but untrusted-spec and +// repeated-fail. +export const LANE_CAUSES = Object.freeze({ + risk: Object.freeze(["files-incomplete", "evidence-unbound:", "manifest-review-required", "unreadable"]), + verdict: Object.freeze(["verify:fail", "verify:earlier-fail", "verify:missing", "verify:unreadable", "verify:too-old", "verify:spec-changed", "verify:ineligible"]), +}); + +// Whether a `kind` refusal recorded as `reason` names only causes the lanes +// clear: each "; "-separated risk code, or the verdict, on LANE_CAUSES. An +// issue link is never one: it lifts only when nothing refuses. +function laneCause(kind, reason) { + const recorded = reason.slice(READER_HOLDS[kind].length, -1); + if (kind === "verdict") { + return LANE_CAUSES.verdict.includes(recorded); + } + const codes = recorded.split("; ").map((part) => part.split(": ")[0]); + return kind === "risk" && codes.every((code) => LANE_CAUSES.risk.some((entry) => (entry.endsWith(":") ? code.startsWith(entry) : code === entry))); +} const escapeCommand = (text) => String(text).replaceAll(/[\r\n%]/g, " "); const reportError = (number, error) => @@ -218,19 +241,17 @@ function nativeRefusal(appComments, merged, label, head) { // Whether the reader's `evidence` at `head` lifts the `kind` refusal recorded // as `reason`: nothing refuses, or, for a recorded cause the lanes clear (a // label set before that cause became a lane's), the answer was read whole and -// what refuses is a lane's for the same cause. A recorded cause a person must -// clear (PERSON_RISK, PERSON_REFUSALS) lifts only when nothing refuses: an -// unbound review or a partial file listing is a lane-class answer that never -// checked it, and the spec's trust is checked only on a pass. The reader reads +// what refuses is a lane's for the same cause. Any other recorded cause +// (laneCause) lifts only when nothing refuses: an unbound review or a partial +// file listing is a lane-class answer that never checked it, and the spec's +// trust is checked only on a pass. The reader reads // the verdict only once the link passes, and the risk score only once the // verdict passes too, so a refusal of either hides what comes after it. function liftsRefusal(kind, reason, evidence, head) { if (verifyHold(evidence, head) === null) { return true; } - const recorded = reason.slice(READER_HOLDS[kind].length, -1); - const person = kind === "risk" ? PERSON_RISK.test(recorded) : PERSON_REFUSALS.includes(recorded); - if (kind === "link" || person || evidence.ineligible !== null || verifyClears(evidence, head) !== "lane" || UNREAD.includes(evidence.refusal) || UNREAD_REVIEWS.test(evidence.reviews ?? "")) { + if (!laneCause(kind, reason) || evidence.ineligible !== null || verifyClears(evidence, head) !== "lane" || UNREAD.includes(evidence.refusal) || UNREAD_REVIEWS.test(evidence.reviews ?? "")) { return false; } return kind === "verdict" || evidence.refusal === null; diff --git a/.github/scripts/pr-merge/clear-escalations.test.mjs b/.github/scripts/pr-merge/clear-escalations.test.mjs index 6a1027ebf9..f37e2ed829 100644 --- a/.github/scripts/pr-merge/clear-escalations.test.mjs +++ b/.github/scripts/pr-merge/clear-escalations.test.mjs @@ -526,6 +526,8 @@ const NATIVE = { LANE_PRS: "native" }; const nativeEscalated = (reason, head = HEAD) => mergeEscalated(refusalBody(true, reason, head, "needs-human")); const clean = (pull) => ({ head: pull.headRefOid, refusal: null, ineligible: null, reviews: null }); const refusing = (refusal) => (pull) => ({ head: pull.headRefOid, refusal, ineligible: null }); +const risk = (codes) => `pr-merge's gate holds it on its risk score (${codes})`; +const reviewsNow = (reviews) => (pull) => ({ ...clean(pull), reviews }); // The reader's early answer for a link it does not take: no verdict, no risk score. const ineligible = (why) => (pull) => ({ head: pull.headRefOid, refusal: "verify:ineligible", ineligible: why }); const DELETE = { method: "DELETE", path: `/repos/${REPO}/issues/7/labels/needs-human`, body: undefined }; @@ -579,6 +581,18 @@ test("a risk, verifier or issue-link label stays when the reader's answer no lon [COST, (pull) => ({ ...clean(pull), reviews: "files-incomplete: the changed-file listing did not read whole" })], [UNTRUSTED_SPEC, refusing("verify:fail")], [REPEATED, refusing("verify:fail")], + // An issue link the trust hold refused, under a lane-class risk answer. + [UNTRUSTED, reviewsNow("evidence-unbound:claude-review: not bound: rerun-not-allowed")], + // A recorded code not known to be a lane's counts as a person's: an + // unknown one, a configured high path, or a person's beside a lane's. + [risk("a-new-code: x"), reviewsNow("evidence-unbound:claude-review: not bound: rerun-allowed")], + [risk("high-path:.github/workflows/**: .github/workflows/ci.yml"), reviewsNow("files-incomplete: the listing did not read whole")], + [risk("evidence-unbound:claude-review: not bound: x; security-finding: 1 open"), reviewsNow("evidence-unbound:claude-review: not bound: x")], + // A lane's recorded cause still needs the answer read whole. + [UNBOUND, reviewsNow("unreadable: no agent identity")], + [SPEC_CHANGED, reviewsNow("unreadable: no agent identity")], + [UNBOUND, refusing("verify:fail")], + [SPEC_CHANGED, ineligible("not-agent-ready")], ]) { const w = world({ timeline: nativeEscalated(reason), author: BOT, evidence }); await run(w, NATIVE); diff --git a/.github/scripts/pr-merge/enable-auto-merge.mjs b/.github/scripts/pr-merge/enable-auto-merge.mjs index 195ec76f6b..64acefe87d 100755 --- a/.github/scripts/pr-merge/enable-auto-merge.mjs +++ b/.github/scripts/pr-merge/enable-auto-merge.mjs @@ -469,7 +469,7 @@ export function verifyHold(evidence, head) { // (babysit_verify_evidence.lane_risk_refusal). export const PERSON_REFUSALS = Object.freeze(["verify:untrusted-spec", "verify:repeated-fail"]); export const TRUST_REFUSALS = Object.freeze(["untrusted-provenance", "untrusted-provenance-removed-by-non-human", "issue-labels-unreadable"]); -export const PERSON_RISK = /(?:^|; )(?:security-finding|trust-gate:[a-z-]+|cost-review-required|high-path:\*\*):/; +const PERSON_RISK = /(?:^|; )(?:security-finding|trust-gate:[a-z-]+|cost-review-required|high-path:\*\*):/; // Who clears the refusal verifyHold reads in `evidence` at `head`: `person` // for a PERSON_REFUSALS refusal, an issue link the trust hold refuses From e987f31096b75772d680d04d6abf13f9d5d84641 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 26 Oct 2026 07:23:40 -0400 Subject: [PATCH 6/8] fix(pr-merge): judge the current refusal on the same lane list as the recorded one A label now lifts on a refusing answer only when every code recorded and every code refusing now is on LANE_CAUSES for its kind, and the reader got at least as far as the recorded cause (link, then verdict, then risk). The list is per kind with no branches: issue links have none, verdicts are FAIL, EARLIER_FAIL and SPEC_CHANGED (too-old recurs at the same head, ineligible always comes with a link answer, missing and unreadable are no answer), and risk codes are files-incomplete, evidence-unbound: and the manifest review. The reader's own unread link or score is no longer a lane cause, and a current high-path or other person code keeps a recorded lane label. Co-Authored-By: Claude Opus 5.5 --- .../scripts/pr-merge/clear-escalations.mjs | 91 +++++++++---------- .../pr-merge/clear-escalations.test.mjs | 13 +++ 2 files changed, 56 insertions(+), 48 deletions(-) diff --git a/.github/scripts/pr-merge/clear-escalations.mjs b/.github/scripts/pr-merge/clear-escalations.mjs index a10df6568c..9c0cf010f6 100755 --- a/.github/scripts/pr-merge/clear-escalations.mjs +++ b/.github/scripts/pr-merge/clear-escalations.mjs @@ -60,18 +60,15 @@ // again at that head: the gate's reader (`evidence`, as enable-auto-merge // reads it, the room's rung cap passed as C3 so the scorer answers at the // config's rung) runs, and the label comes off as `refusal-cleared` when -// nothing refuses. A recorded cause the lanes clear (every code on -// LANE_CAUSES; a label set before it became a lane's) also comes off when the -// answer was read whole and what refuses now is a lane's (verifyClears -// `lane`) for the same cause: for a verifier reason a verdict, read once the -// link passes, that is neither unreadable nor missing; for a risk reason a -// risk score, which the reader computes only once the link and verdict pass. -// An issue-link reason, and any recorded code off that list, an unknown one -// included, come off only when nothing refuses. A person's refusal, an unread -// answer and a failed read keep it; so does a hold or cost path, which no -// reader answer clears. At most NATIVE_READS_PER_RUN reads a run, starting at -// an offset that moves with the run number -// (rotated), so labels past the cap are read on a later run. +// nothing refuses. It also comes off when every code recorded and every code +// refusing now is on LANE_CAUSES (a label set before its cause became a +// lane's), and the reader got at least as far as the recorded cause: the link, +// then the verdict, then the risk score, each read only once the one before +// passes. An issue-link reason, and any code off that list, an unknown one +// included, come off only when nothing refuses. An unread answer and a failed +// read keep it; so does a hold or cost path, which no reader answer clears. At +// most NATIVE_READS_PER_RUN reads a run, starting at an offset that moves with +// the run number (rotated), so labels past the cap are read on a later run. // // Environment: // GH_TOKEN the lane App token (pull-requests and issues write) @@ -109,7 +106,7 @@ import { spoken, } from "../pr-unblock/gate-source.mjs"; import { isProtectedPath } from "../pr-unblock/thread-scope.mjs"; -import { personBodies, pythonVerify, READER_HOLDS, REFUSAL_MARKER, rotated, verifyClears, verifyHold } from "./enable-auto-merge.mjs"; +import { personBodies, pythonVerify, READER_HOLDS, REFUSAL_MARKER, rotated, verifyHold } from "./enable-auto-merge.mjs"; const DEFAULT_BOT = "melodic-automation-lanes[bot]"; const REASON = /^- Reason: `([a-z-]+)`$/m; @@ -140,32 +137,33 @@ export const CLEARS_PER_RUN = 5; // step's timeout-minutes (pr-merge.yml) allows this many (judgment). export const NATIVE_READS_PER_RUN = 3; const REFUSAL_CLEARED = "refusal-cleared"; -// babysit_verify_evidence.py's verdicts and risk answer for a read that did -// not finish (UNREADABLE, MISSING, `reviews: "unreadable: …"`). -const UNREAD = Object.freeze(["verify:unreadable", "verify:missing"]); -const UNREAD_REVIEWS = /^unreadable:/; -// The recorded causes the lanes clear, the only ones a lane-class answer may -// lift; every other code, an unknown one included, is a person's. Risk codes -// (an entry ending in ":" is a prefix): babysit_risk.py's holding MED codes -// files-incomplete and evidence-unbound:, the manifest review, and the -// reader's own unread score (babysit_verify_evidence.py lane_risk_refusal). -// Verdicts: babysit_verify_evidence.py's refusals but untrusted-spec and -// repeated-fail. +// The causes the lanes clear, by READER_HOLDS kind: the only ones a +// lane-class answer may lift, recorded or read now. Every other code, an +// unknown one included, is a person's. babysit_risk.PERSON_CODES (#10249) is +// to become the single source; until then the list is explicit, so it fails +// closed. +// - risk (an entry ending in ":" is a prefix): babysit_risk.py's holding MED +// codes files-incomplete (:938) and evidence-unbound: (:978-998), and +// the manifest review (:598). Not the reader's `unreadable: …` score. +// files-incomplete and the manifest review join their items with "; ", so +// a reason with two or more items reads as unknown codes and stays. +// - verdict: babysit_verify_evidence.py's FAIL, EARLIER_FAIL and SPEC_CHANGED, +// which a push clears. Not untrusted-spec or repeated-fail (a person's), +// too-old (it recurs at the same head; #10249 makes it a person's), missing +// or unreadable (no answer), or ineligible (always paired with a link +// answer, which verifyHold reads first). +// - link: none; an issue link lifts only when nothing refuses. export const LANE_CAUSES = Object.freeze({ - risk: Object.freeze(["files-incomplete", "evidence-unbound:", "manifest-review-required", "unreadable"]), - verdict: Object.freeze(["verify:fail", "verify:earlier-fail", "verify:missing", "verify:unreadable", "verify:too-old", "verify:spec-changed", "verify:ineligible"]), + link: Object.freeze([]), + verdict: Object.freeze(["verify:fail", "verify:earlier-fail", "verify:spec-changed"]), + risk: Object.freeze(["files-incomplete", "evidence-unbound:", "manifest-review-required"]), }); -// Whether a `kind` refusal recorded as `reason` names only causes the lanes -// clear: each "; "-separated risk code, or the verdict, on LANE_CAUSES. An -// issue link is never one: it lifts only when nothing refuses. +// Whether the `kind` reason (verifyHold's) names only causes the lanes clear: +// each "; "-separated code on LANE_CAUSES[kind]. function laneCause(kind, reason) { - const recorded = reason.slice(READER_HOLDS[kind].length, -1); - if (kind === "verdict") { - return LANE_CAUSES.verdict.includes(recorded); - } - const codes = recorded.split("; ").map((part) => part.split(": ")[0]); - return kind === "risk" && codes.every((code) => LANE_CAUSES.risk.some((entry) => (entry.endsWith(":") ? code.startsWith(entry) : code === entry))); + const codes = reason.slice(READER_HOLDS[kind].length, -1).split("; ").map((part) => part.split(": ")[0]); + return codes.every((code) => LANE_CAUSES[kind].some((entry) => (entry.endsWith(":") ? code.startsWith(entry) : code === entry))); } const escapeCommand = (text) => String(text).replaceAll(/[\r\n%]/g, " "); @@ -239,22 +237,19 @@ function nativeRefusal(appComments, merged, label, head) { } // Whether the reader's `evidence` at `head` lifts the `kind` refusal recorded -// as `reason`: nothing refuses, or, for a recorded cause the lanes clear (a -// label set before that cause became a lane's), the answer was read whole and -// what refuses is a lane's for the same cause. Any other recorded cause -// (laneCause) lifts only when nothing refuses: an unbound review or a partial -// file listing is a lane-class answer that never checked it, and the spec's -// trust is checked only on a pass. The reader reads -// the verdict only once the link passes, and the risk score only once the -// verdict passes too, so a refusal of either hides what comes after it. +// as `reason`: nothing refuses, or both the recorded cause and what refuses +// now are the lanes' (laneCause), and the reader got at least as far as the +// recorded cause. It reads the link, then the verdict, then the risk score +// (READER_HOLDS order), each only once the one before passes, so a refusal +// that stops earlier hides the recorded cause. function liftsRefusal(kind, reason, evidence, head) { - if (verifyHold(evidence, head) === null) { + const held = verifyHold(evidence, head); + if (held === null) { return true; } - if (!laneCause(kind, reason) || evidence.ineligible !== null || verifyClears(evidence, head) !== "lane" || UNREAD.includes(evidence.refusal) || UNREAD_REVIEWS.test(evidence.reviews ?? "")) { - return false; - } - return kind === "verdict" || evidence.refusal === null; + const order = Object.keys(READER_HOLDS); + const now = order.find((key) => held.startsWith(READER_HOLDS[key])); + return order.indexOf(now) >= order.indexOf(kind) && laneCause(kind, reason) && laneCause(now, held); } // Whether the lanes App comment written last at or before `since` that carries diff --git a/.github/scripts/pr-merge/clear-escalations.test.mjs b/.github/scripts/pr-merge/clear-escalations.test.mjs index f37e2ed829..ae930ee0c9 100644 --- a/.github/scripts/pr-merge/clear-escalations.test.mjs +++ b/.github/scripts/pr-merge/clear-escalations.test.mjs @@ -551,6 +551,8 @@ test("enable-auto-merge's label comes off when the reader now reads a refusal th [SPEC_CHANGED, refusing("verify:earlier-fail")], // A risk label: a risk score read whole, now naming only lane causes. [UNBOUND, (pull) => ({ ...clean(pull), reviews: "evidence-unbound:claude-review: not bound: rerun-allowed" })], + // A lane verdict now passes, and the risk score it hid names only lane causes. + [SPEC_CHANGED, reviewsNow("evidence-unbound:claude-review: not bound: rerun-allowed")], ]) { const w = world({ timeline: nativeEscalated(reason), author: BOT, evidence }); await run(w, NATIVE); @@ -593,6 +595,17 @@ test("a risk, verifier or issue-link label stays when the reader's answer no lon [SPEC_CHANGED, reviewsNow("unreadable: no agent identity")], [UNBOUND, refusing("verify:fail")], [SPEC_CHANGED, ineligible("not-agent-ready")], + // A link reason is never a lane's, the reader's own unread link included; + // a code is matched whole; a lane cause recorded never lifts a person's now. + ["pr-merge's lane does not take its issue link (unreadable: timed out)", reviewsNow("files-incomplete: the listing did not read whole")], + [risk("files-incomplete-x: d"), reviewsNow("files-incomplete: the listing did not read whole")], + [risk("trust-gate:trusted-actors: widens"), reviewsNow("files-incomplete: the listing did not read whole")], + ["pr-merge's gate refuses its verifier verdict (verify:fail-x)", refusing("verify:fail")], + [risk("files-incomplete: the listing did not read whole"), reviewsNow("high-path:infra/**: infra/main.tf")], + // Off the lane list: a refusal that recurs at the same head, or one the + // reader always pairs with a link answer. + ["pr-merge's gate refuses its verifier verdict (verify:too-old)", refusing("verify:fail")], + ["pr-merge's gate refuses its verifier verdict (verify:ineligible)", refusing("verify:fail")], ]) { const w = world({ timeline: nativeEscalated(reason), author: BOT, evidence }); await run(w, NATIVE); From 6433994ea3d5edc970f7935f5c17c12651b7e2d1 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 26 Oct 2026 07:48:01 -0400 Subject: [PATCH 7/8] fix(pr-merge): size the merge job for the escalation clear's reader runs The merge job's 30-minute ceiling covered the gate's sweep alone; the escalation clear now takes up to 15 minutes for its three reader runs, so a long sweep followed by slow reads could hit the job timeout and cancel the closing steps. The ceiling is 45 and a workflow test checks it covers the gate's sizing plus the clear step's timeout. The clear-escalations test for kept answers now also asserts exit 0, so a null reader answer is kept as a verdict rather than caught as a per-PR error. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/pr-merge/clear-escalations.test.mjs | 4 +++- .github/scripts/pr-merge/workflow.test.mjs | 11 +++++++++++ .github/workflows/pr-merge.yml | 9 +++++---- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/.github/scripts/pr-merge/clear-escalations.test.mjs b/.github/scripts/pr-merge/clear-escalations.test.mjs index ae930ee0c9..ec046d0da5 100644 --- a/.github/scripts/pr-merge/clear-escalations.test.mjs +++ b/.github/scripts/pr-merge/clear-escalations.test.mjs @@ -630,7 +630,9 @@ test("enable-auto-merge's label stays while its cause, or another a person must }, ]) { const w = world({ timeline: nativeEscalated(UNBOUND), author: BOT, evidence }); - await run(w, NATIVE); + const { code, logs } = await run(w, NATIVE); + // Kept as a verdict, never a per-PR error: a null answer included. + assert.equal(code, 0, `${evidence}\n${logs.join("\n")}`); assert.deepEqual(w.writes(), [], String(evidence)); } }); diff --git a/.github/scripts/pr-merge/workflow.test.mjs b/.github/scripts/pr-merge/workflow.test.mjs index 4ead07fbed..f434ccfdb6 100644 --- a/.github/scripts/pr-merge/workflow.test.mjs +++ b/.github/scripts/pr-merge/workflow.test.mjs @@ -358,6 +358,17 @@ test("enable-auto-merge's step timeouts fit inside its job's", () => { assert.ok(steps.reduce((sum, minutes) => sum + minutes, 0) < total, `${steps.join(" + ")} inside ${total}`); }); +// The merge job's sizing comment: about a minute before the loop, then the +// longer of the gate budget (10) plus a candidate's read limit (4) and a +// deferred candidate's ceiling (20), plus one read's own timeout (4); the +// escalation clear's own timeout comes on top. +test("the merge job's ceiling covers the gate's sweep and the escalation clear", () => { + const gate = job("merge"); + const total = Number(/\n {4}timeout-minutes: ([0-9]+)\n/.exec(gate)[1]); + const clear = Number(/- name: Clear escalations whose reason no longer holds\n[\s\S]*?\n {8}timeout-minutes: ([0-9]+)\n/.exec(gate)[1]); + assert.ok(total >= 1 + Math.max(10 + 4, 20) + 4 + clear, `${total} minutes for the gate and a ${clear}-minute clear`); +}); + test("enable-auto-merge asks the broker for its own activity's merge token", () => { const arm = job("enable-auto-merge"); const steps = [...arm.matchAll(/\n {10}lane: (.*)\n {10}activity: (.*)\n/g)].map((m) => `${m[1]}/${m[2]}`); diff --git a/.github/workflows/pr-merge.yml b/.github/workflows/pr-merge.yml index 777a29ba43..cdf0013235 100644 --- a/.github/workflows/pr-merge.yml +++ b/.github/workflows/pr-merge.yml @@ -483,12 +483,13 @@ jobs: # A ceiling above the gate's time budget (merge.gate-budget-minutes, 10), # which is what ends a long sweep: the budget stops between candidates, # a timeout can stop one between its check and its merge-queue write. - # 30 leaves about a minute before the loop, then the longer of the + # 45 leaves about a minute before the loop, then the longer of the # budget plus the last candidate's read limit (merge.gate-candidate-minutes, # 4) and a deferred candidate's ceiling (merge.gate-deferred-minutes, 20; - # it is always gated first), plus one read's own timeout, the writes and - # the closing steps. - timeout-minutes: 30 + # it is always gated first), plus one read's own timeout, the escalation + # clear's 15 (three reader runs of 4), the writes and the closing steps + # (workflow.test.mjs checks it). + timeout-minutes: 45 permissions: contents: read # check out main pull-requests: read # list the open PRs From ecf1fda80410ad8e5aab76aa923e4c00c314a18c Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 26 Oct 2026 09:30:57 -0400 Subject: [PATCH 8/8] fix(pr-merge): never lift a person-class needs-human at its head Owner decision Q-CLEAR B: clear-escalations re-judges enable-auto-merge's refusal at the same head only when the recorded cause is lane-class (every code on LANE_CAUSES). A person-class recorded cause (issue link, security finding, trust gate, cost, high path, untrusted spec, repeated fail, unknown codes) is not read and not lifted, even when the reader finds nothing refusing; a person removes the label, or head-moved clears it on a push. enable-auto-merge's person refusal wording goes back to main's ("fix the reason, then remove the label"): every label it sets is person-class, so the same-head auto-removal promise no longer holds for any of them. Co-Authored-By: Claude Opus 5.5 --- .../fix-clear-native-escalations-6e1b4a92.md | 8 ++ .../scripts/pr-merge/clear-escalations.mjs | 68 +++++----- .../pr-merge/clear-escalations.test.mjs | 120 ++++++++++-------- .../scripts/pr-merge/enable-auto-merge.mjs | 33 ++--- .../pr-merge/enable-auto-merge.test.mjs | 24 +--- docs/conventions/pr-pipeline/README.md | 1 + 6 files changed, 121 insertions(+), 133 deletions(-) create mode 100644 .changes/pr-pipeline/fix-clear-native-escalations-6e1b4a92.md diff --git a/.changes/pr-pipeline/fix-clear-native-escalations-6e1b4a92.md b/.changes/pr-pipeline/fix-clear-native-escalations-6e1b4a92.md new file mode 100644 index 0000000000..0374a5d994 --- /dev/null +++ b/.changes/pr-pipeline/fix-clear-native-escalations-6e1b4a92.md @@ -0,0 +1,8 @@ +--- +bump: patch +--- + +### Fixed + +- Under `merge.lane-prs: native`, `clear-escalations.mjs` judges `enable-auto-merge`'s person refusal again at the head it was raised at, but only when its recorded verifier or risk cause is one the lanes clear (every code on `LANE_CAUSES`, a label set before its cause became a lane's): the gate's reader runs, and the label comes off as `refusal-cleared` when nothing refuses, or only lane causes refuse, read at least as far as the recorded one. A recorded cause a person clears (an issue link, a security finding, a trust-gate widening, a cost hold, a high path, an untrusted spec, the repeated-fail cap, an unknown code) is never read and never lifted at its head (owner decision Q-CLEAR B); a person removes the label, or the `head-moved` clear does once the head moves. At most three reader runs a run, rotated with the run number. +- `pr-merge.yml`'s merge job allows 45 minutes, up from 30, and the escalation clear step 15, up from 3, so the clear's reader runs fit beside the gate's own budget. diff --git a/.github/scripts/pr-merge/clear-escalations.mjs b/.github/scripts/pr-merge/clear-escalations.mjs index 9c0cf010f6..1a31f93b22 100755 --- a/.github/scripts/pr-merge/clear-escalations.mjs +++ b/.github/scripts/pr-merge/clear-escalations.mjs @@ -54,21 +54,24 @@ // and head only retries the removal. At most CLEARS_PER_RUN a run. // // Under `merge.lane-prs: native`, enable-auto-merge's own person refusal -// comment (one of personBodies, unedited) on a lanes App pull request, the -// only pr-merge escalation there, naming the current head, whose reason is -// the gate's issue-link, verifier or risk answer (READER_HOLDS), is judged -// again at that head: the gate's reader (`evidence`, as enable-auto-merge -// reads it, the room's rung cap passed as C3 so the scorer answers at the -// config's rung) runs, and the label comes off as `refusal-cleared` when -// nothing refuses. It also comes off when every code recorded and every code -// refusing now is on LANE_CAUSES (a label set before its cause became a -// lane's), and the reader got at least as far as the recorded cause: the link, -// then the verdict, then the risk score, each read only once the one before -// passes. An issue-link reason, and any code off that list, an unknown one -// included, come off only when nothing refuses. An unread answer and a failed -// read keep it; so does a hold or cost path, which no reader answer clears. At -// most NATIVE_READS_PER_RUN reads a run, starting at an offset that moves with -// the run number (rotated), so labels past the cap are read on a later run. +// comment (refusalBody, unedited) on a lanes App pull request, the only +// pr-merge escalation there, naming the current head, is judged again at that +// head only when its recorded cause is the lanes' (owner decision Q-CLEAR B): +// a verifier or risk reason whose every code is on LANE_CAUSES, a label set +// before its cause became a lane's. Then the gate's reader (`evidence`, as +// enable-auto-merge reads it, the room's rung cap passed as C3 so the scorer +// answers at the config's rung) runs, and the label comes off as +// `refusal-cleared` when nothing refuses, or when every code refusing now is +// on LANE_CAUSES too and the reader got at least as far as the recorded +// cause: the link, then the verdict, then the risk score, each read only once +// the one before passes. A person's recorded cause (an issue link, any code +// off that list, an unknown one included) is never read and never lifted at +// that head: a person removes the label, or a push clears it as head-moved +// above and enable-auto-merge labels the new head again if the cause holds. +// An unread answer and a failed read keep it; so does a hold or cost path, +// which no reader answer clears. At most NATIVE_READS_PER_RUN reads a run, +// starting at an offset that moves with the run number (rotated), so labels +// past the cap are read on a later run. // // Environment: // GH_TOKEN the lane App token (pull-requests and issues write) @@ -106,7 +109,7 @@ import { spoken, } from "../pr-unblock/gate-source.mjs"; import { isProtectedPath } from "../pr-unblock/thread-scope.mjs"; -import { personBodies, pythonVerify, READER_HOLDS, REFUSAL_MARKER, rotated, verifyHold } from "./enable-auto-merge.mjs"; +import { pythonVerify, READER_HOLDS, REFUSAL_MARKER, refusalBody, rotated, verifyHold } from "./enable-auto-merge.mjs"; const DEFAULT_BOT = "melodic-automation-lanes[bot]"; const REASON = /^- Reason: `([a-z-]+)`$/m; @@ -137,9 +140,9 @@ export const CLEARS_PER_RUN = 5; // step's timeout-minutes (pr-merge.yml) allows this many (judgment). export const NATIVE_READS_PER_RUN = 3; const REFUSAL_CLEARED = "refusal-cleared"; -// The causes the lanes clear, by READER_HOLDS kind: the only ones a -// lane-class answer may lift, recorded or read now. Every other code, an -// unknown one included, is a person's. babysit_risk.PERSON_CODES (#10249) is +// The causes the lanes clear, by READER_HOLDS kind: the only recorded ones +// this script lifts at the same head, and the only ones a lift tolerates +// refusing now. Every other code, an unknown one included, is a person's. babysit_risk.PERSON_CODES (#10249) is // to become the single source; until then the list is explicit, so it fails // closed. // - risk (an entry ending in ":" is a prefix): babysit_risk.py's holding MED @@ -152,7 +155,7 @@ const REFUSAL_CLEARED = "refusal-cleared"; // too-old (it recurs at the same head; #10249 makes it a person's), missing // or unreadable (no answer), or ineligible (always paired with a link // answer, which verifyHold reads first). -// - link: none; an issue link lifts only when nothing refuses. +// - link: none; an issue link is a person's. export const LANE_CAUSES = Object.freeze({ link: Object.freeze([]), verdict: Object.freeze(["verify:fail", "verify:earlier-fail", "verify:spec-changed"]), @@ -233,23 +236,23 @@ function nativeRefusal(appComments, merged, label, head) { } const reason = lines(newest.body).slice(5, -1).join("\n"); const kind = Object.keys(READER_HOLDS).find((key) => reason.startsWith(READER_HOLDS[key])); - return kind !== undefined && personBodies(reason, head, label).includes(newest.body) ? { kind, reason } : null; + return kind !== undefined && refusalBody(true, reason, head, label) === newest.body ? { kind, reason } : null; } -// Whether the reader's `evidence` at `head` lifts the `kind` refusal recorded -// as `reason`: nothing refuses, or both the recorded cause and what refuses -// now are the lanes' (laneCause), and the reader got at least as far as the -// recorded cause. It reads the link, then the verdict, then the risk score -// (READER_HOLDS order), each only once the one before passes, so a refusal -// that stops earlier hides the recorded cause. -function liftsRefusal(kind, reason, evidence, head) { +// Whether the reader's `evidence` at `head` lifts the `kind` refusal, whose +// recorded cause the caller has found the lanes' (laneCause): nothing +// refuses, or what refuses now is the lanes' too and the reader got at least +// as far as the recorded cause. It reads the link, then the verdict, then the +// risk score (READER_HOLDS order), each only once the one before passes, so a +// refusal that stops earlier hides the recorded cause. +function liftsRefusal(kind, evidence, head) { const held = verifyHold(evidence, head); if (held === null) { return true; } const order = Object.keys(READER_HOLDS); const now = order.find((key) => held.startsWith(READER_HOLDS[key])); - return order.indexOf(now) >= order.indexOf(kind) && laneCause(kind, reason) && laneCause(now, held); + return order.indexOf(now) >= order.indexOf(kind) && laneCause(now, held); } // Whether the lanes App comment written last at or before `since` that carries @@ -336,6 +339,11 @@ async function verdict({ github, repo, number, label, bot, mainSha, lanePrs, ver } // A refusal at an old head takes the head-moved path below. const native = lanePrs === "native" && isApp(pull.user, bot) ? nativeRefusal(appComments, merged, label, pull.head.sha) : null; + // A person's recorded cause stays until a person removes the label or the + // head moves (owner decision Q-CLEAR B): no reader answer lifts it. + if (native !== null && !laneCause(native.kind, native.reason)) { + return { reason: `enable-auto-merge's refusal at ${pull.head.sha} is one a person clears (${native.reason})` }; + } if (native !== null) { if (reads.left <= 0) { return { reason: `enable-auto-merge's refusal waits: ${NATIVE_READS_PER_RUN} reader runs this run` }; @@ -348,7 +356,7 @@ async function verdict({ github, repo, number, label, bot, mainSha, lanePrs, ver return { reason: `the gate's reader did not answer: ${escapeCommand(error?.message ?? error)}` }; } const held = verifyHold(evidence, pull.head.sha); - if (!liftsRefusal(native.kind, native.reason, evidence, pull.head.sha)) { + if (!liftsRefusal(native.kind, evidence, pull.head.sha)) { return { reason: `enable-auto-merge's refusal still needs a person at ${pull.head.sha}: ${held}` }; } return cleared(timeline, bot, since, [REFUSAL_CLEARED], pull.head.sha, [ diff --git a/.github/scripts/pr-merge/clear-escalations.test.mjs b/.github/scripts/pr-merge/clear-escalations.test.mjs index ec046d0da5..542a3c9a83 100644 --- a/.github/scripts/pr-merge/clear-escalations.test.mjs +++ b/.github/scripts/pr-merge/clear-escalations.test.mjs @@ -560,52 +560,77 @@ test("enable-auto-merge's label comes off when the reader now reads a refusal th } }); +// Owner decision Q-CLEAR B: a label whose recorded cause is a person's is +// never lifted at the head it was raised at, whatever the reader answers; a +// person removes it, or a push clears it as head-moved. It is not read. +const PERSON_CLASS = [ + SECURITY, + risk("trust-gate:trusted-actors: widens the trusted actors"), + COST, + risk("high-path:**: a symlink"), + risk("high-path:.github/workflows/**: .github/workflows/ci.yml"), + UNTRUSTED_SPEC, + REPEATED, + UNTRUSTED, + // A link reason, the reader's own unread link included. + "pr-merge's lane does not take its issue link (unreadable: timed out)", + // Off the lane list: a refusal that recurs at the same head, or one the + // reader always pairs with a link answer. + "pr-merge's gate refuses its verifier verdict (verify:too-old)", + "pr-merge's gate refuses its verifier verdict (verify:ineligible)", + // A code not known to be a lane's, matched whole, or a person's beside a lane's. + risk("a-new-code: x"), + risk("files-incomplete-x: d"), + "pr-merge's gate refuses its verifier verdict (verify:fail-x)", + risk("evidence-unbound:claude-review: not bound: x; security-finding: 1 open"), +]; + +test("a person-class label is never removed at its head, even when nothing refuses", async () => { + for (const reason of PERSON_CLASS) { + for (const evidence of [clean, reviewsNow("evidence-unbound:claude-review: not bound: rerun-allowed"), refusing("verify:fail")]) { + const w = world({ timeline: nativeEscalated(reason), author: BOT, evidence }); + const { code, logs } = await run(w, NATIVE); + assert.equal(code, 0, `${reason}\n${logs.join("\n")}`); + assert.deepEqual(w.reads, [], reason); + assert.deepEqual(w.writes(), [], reason); + assert.match(logs.join("\n"), /one a person clears/, reason); + } + } +}); + +test("a lane-class label comes off at its head once nothing refuses", async () => { + for (const reason of [ + SPEC_CHANGED, + "pr-merge's gate refuses its verifier verdict (verify:fail)", + "pr-merge's gate refuses its verifier verdict (verify:earlier-fail)", + UNBOUND, + risk("files-incomplete: the changed-file listing did not read whole"), + risk("manifest-review-required: plugins/x/.claude-plugin/plugin.json"), + ]) { + const w = world({ timeline: nativeEscalated(reason), author: BOT, evidence: clean }); + const { code, logs } = await run(w, NATIVE); + assert.equal(code, 0, logs.join("\n")); + assert.equal(w.reads.length, 1, reason); + assert.equal(w.writes()[0].body.body.split("\n")[0], ``, reason); + assert.deepEqual(w.writes()[1], DELETE, reason); + } +}); + // A risk score is read only when the verdict passes (babysit_verify_evidence.py -// `evidence`), so a refusal or an unread answer hides it: the label stays. -test("a risk, verifier or issue-link label stays when the reader's answer no longer shows its cause", async () => { +// `evidence`), so a refusal or an unread answer hides a lane's recorded cause: +// the label stays. +test("a lane-class label stays when the reader's answer no longer shows its cause", async () => { for (const [reason, evidence] of [ - [SECURITY, refusing("verify:unreadable")], - [SECURITY, refusing("verify:missing")], - [SECURITY, refusing("verify:fail")], - [SECURITY, ineligible("not-agent-ready")], - [SECURITY, (pull) => ({ ...clean(pull), reviews: "unreadable: no agent identity" })], - [REPEATED, refusing("verify:unreadable")], - [REPEATED, refusing("verify:missing")], - [REPEATED, ineligible("not-agent-ready")], - [REPEATED, (pull) => ({ ...clean(pull), reviews: "unreadable: the lane's rung is off" })], - [UNTRUSTED, ineligible("not-agent-ready")], - [UNTRUSTED, refusing("verify:fail")], [SPEC_CHANGED, refusing("verify:unreadable")], - // A person's recorded cause clears only when nothing refuses: an unbound - // review or a partial file listing never read it, and a verdict that is not - // a pass never checks the spec's trust. - [SECURITY, (pull) => ({ ...clean(pull), reviews: "evidence-unbound:claude-security-review: not bound: threads-unread" })], - [COST, (pull) => ({ ...clean(pull), reviews: "files-incomplete: the changed-file listing did not read whole" })], - [UNTRUSTED_SPEC, refusing("verify:fail")], - [REPEATED, refusing("verify:fail")], - // An issue link the trust hold refused, under a lane-class risk answer. - [UNTRUSTED, reviewsNow("evidence-unbound:claude-review: not bound: rerun-not-allowed")], - // A recorded code not known to be a lane's counts as a person's: an - // unknown one, a configured high path, or a person's beside a lane's. - [risk("a-new-code: x"), reviewsNow("evidence-unbound:claude-review: not bound: rerun-allowed")], - [risk("high-path:.github/workflows/**: .github/workflows/ci.yml"), reviewsNow("files-incomplete: the listing did not read whole")], - [risk("evidence-unbound:claude-review: not bound: x; security-finding: 1 open"), reviewsNow("evidence-unbound:claude-review: not bound: x")], - // A lane's recorded cause still needs the answer read whole. + [SPEC_CHANGED, refusing("verify:missing")], [UNBOUND, reviewsNow("unreadable: no agent identity")], [SPEC_CHANGED, reviewsNow("unreadable: no agent identity")], [UNBOUND, refusing("verify:fail")], [SPEC_CHANGED, ineligible("not-agent-ready")], - // A link reason is never a lane's, the reader's own unread link included; - // a code is matched whole; a lane cause recorded never lifts a person's now. - ["pr-merge's lane does not take its issue link (unreadable: timed out)", reviewsNow("files-incomplete: the listing did not read whole")], - [risk("files-incomplete-x: d"), reviewsNow("files-incomplete: the listing did not read whole")], - [risk("trust-gate:trusted-actors: widens"), reviewsNow("files-incomplete: the listing did not read whole")], - ["pr-merge's gate refuses its verifier verdict (verify:fail-x)", refusing("verify:fail")], + // A lane cause recorded never lifts a person's now. [risk("files-incomplete: the listing did not read whole"), reviewsNow("high-path:infra/**: infra/main.tf")], - // Off the lane list: a refusal that recurs at the same head, or one the - // reader always pairs with a link answer. - ["pr-merge's gate refuses its verifier verdict (verify:too-old)", refusing("verify:fail")], - ["pr-merge's gate refuses its verifier verdict (verify:ineligible)", refusing("verify:fail")], + [UNBOUND, reviewsNow("security-finding: 1 open CRITICAL finding")], + [SPEC_CHANGED, refusing("verify:untrusted-spec")], ]) { const w = world({ timeline: nativeEscalated(reason), author: BOT, evidence }); await run(w, NATIVE); @@ -671,23 +696,6 @@ test("a pr-merge person comment not in enable-auto-merge's exact shape, or edite } }); -// The person text an earlier enable-auto-merge posted still names the cause. -test("enable-auto-merge's refusal in its earlier wording is judged again", async () => { - const earlier = [ - "", - "", - `Unattended merge: native auto-merge leaves this pull request unarmed at head \`${HEAD}\`. A person must clear this reason, so the pr-merge lane added the \`needs-human\` label: fix the reason, then remove the label, and the next scheduled run judges it again.`, - "", - "```text", - UNBOUND, - "```", - ].join("\n"); - const w = world({ timeline: mergeEscalated(earlier), author: BOT, evidence: clean }); - await run(w, NATIVE); - assert.equal(w.reads.length, 1); - assert.deepEqual(w.writes().at(-1), DELETE); -}); - // Main's head-moved rule (#9826): no read; enable-auto-merge judges the new // head and labels it again only for a cause a person must clear. test("enable-auto-merge's refusal at an old head comes off as head-moved without a read", async () => { @@ -768,7 +776,7 @@ test("labels past the read cap are read on a later run", async () => { const numbers = Array.from({ length: NATIVE_READS_PER_RUN + 1 }, (_, i) => 7 + i); const read = new Set(); for (let runNumber = 1; runNumber <= numbers.length; runNumber += 1) { - const w = world({ timeline: nativeEscalated(SECURITY), author: BOT, evidence: (pull) => ({ ...clean(pull), reviews: "security-finding: 1 open CRITICAL finding" }), numbers }); + const w = world({ timeline: nativeEscalated(UNBOUND), author: BOT, evidence: reviewsNow("unreadable: no agent identity"), numbers }); await run(w, { ...NATIVE, GITHUB_RUN_NUMBER: String(runNumber) }); assert.equal(w.reads.length, NATIVE_READS_PER_RUN); for (const { pull } of w.reads) { diff --git a/.github/scripts/pr-merge/enable-auto-merge.mjs b/.github/scripts/pr-merge/enable-auto-merge.mjs index 39d0c83dcd..ea8bb0bb65 100755 --- a/.github/scripts/pr-merge/enable-auto-merge.mjs +++ b/.github/scripts/pr-merge/enable-auto-merge.mjs @@ -99,10 +99,8 @@ // denied path or the floor, which only a trusted approval lifts; and the // lane's own hold line, owner decision Q-HOLDLINE) also gets the human-gated // label, put back while the comment stands unless a person removed it -// since. clear-escalations takes the label off once the head moves, or, for -// an issue-link, verifier or risk reason, once the gate's reader at the same -// head reads the cause gone; this run labels it again only if its cause -// still holds. Holds a +// since. clear-escalations takes the label off once the head moves, and +// this run labels the new head again only if its cause still holds. Holds a // person already set (labels, their own body hold, a veto), the room and // failed reads write nothing. // Arming passes the head just read as expectedHeadOid, so a push in between @@ -434,7 +432,6 @@ export const READER_HOLDS = Object.freeze({ verdict: "pr-merge's gate refuses its verifier verdict (", risk: "pr-merge's gate holds it on its risk score (", }); -const readerHold = (reason) => Object.values(READER_HOLDS).some((prefix) => String(reason).startsWith(prefix)); // Why the gate's `evidence --lane-link` (babysit_verify_evidence.py's report) // keeps a pull request at `head` from merging, or null when it allows it. @@ -515,23 +512,13 @@ export function ejectionHold(evidence) { // refusal the interactive queue's escalation marker first (work-items // escalation-marker.md), as the gate's Dependabot escalation carries it. A // function of the class, the reason and the head only, so an unchanged -// refusal renders the same body. A person's reader refusal (READER_HOLDS) -// says pr-merge takes the label off once it reads the reason gone; any other -// person's reason, which only a push or a person lifts, says to remove it. +// refusal renders the same body. export const REFUSAL_MARKER = ""; export const ATTEND_QUEUE_MARKER = ""; -const LANE_WHO = "The lanes clear this reason on their own; the next scheduled run judges it again."; -const personWho = (label) => - `A person must clear this reason, so the pr-merge lane added the \`${label}\` label: fix the reason, then remove the label, and the next scheduled run judges it again.`; -const readerWho = (label) => - `A person must clear this reason, so the pr-merge lane added the \`${label}\` label: fix the reason, and pr-merge takes the label off once the head moves or, under \`merge.lane-prs: native\`, once a scheduled run reads the reason gone at this head; the next scheduled run then judges it again.`; export function refusalBody(person, reason, head, label) { - return refusalText(person, person ? (readerHold(reason) ? readerWho : personWho)(label) : LANE_WHO, reason, head); -} -// Each body a person's refusal of `reason` at `head` may stand in: the -// current wording, then a reader refusal's earlier one. -export const personBodies = (reason, head, label) => [...new Set([refusalBody(true, reason, head, label), refusalText(true, personWho(label), reason, head)])]; -function refusalText(person, who, reason, head) { + const who = person + ? `A person must clear this reason, so the pr-merge lane added the \`${label}\` label: fix the reason, then remove the label, and the next scheduled run judges it again.` + : "The lanes clear this reason on their own; the next scheduled run judges it again."; return [ ...(person ? [ATTEND_QUEUE_MARKER] : []), REFUSAL_MARKER, @@ -589,13 +576,9 @@ export function refusalWrites(pull, comments, { clears, reason, comment }, { bot // A person's comment already standing while the label is off (a pull // request carrying it never gets here: holdReason holds it): the label // goes back on unless a person removed it after the comment last changed, - // which the caller reads from the timeline (`liftedSince`). Either wording - // of a person's comment stands (personBodies). - if (body === person && personBodies(reason, pull.headRefOid, label).includes(prior?.body)) { - return { body: null, commentId: prior.databaseId ?? null, label, liftedSince: String(prior.updatedAt ?? prior.createdAt) }; - } + // which the caller reads from the timeline (`liftedSince`). if (prior?.body === body) { - return null; + return body === person ? { body: null, commentId: prior.databaseId ?? null, label, liftedSince: String(prior.updatedAt ?? prior.createdAt) } : null; } // An escalation to a person posts a new comment: an edit notifies no one // and keeps the old date, so the label would stand unexplained. diff --git a/.github/scripts/pr-merge/enable-auto-merge.test.mjs b/.github/scripts/pr-merge/enable-auto-merge.test.mjs index 7e92580af6..2e247a36df 100644 --- a/.github/scripts/pr-merge/enable-auto-merge.test.mjs +++ b/.github/scripts/pr-merge/enable-auto-merge.test.mjs @@ -257,16 +257,11 @@ const laneNote = (reason) => reason, "```", ].join("\n"); -// A reader refusal (issue link, verdict, risk score) is one clear-escalations -// takes the label off for at the same head; any other a person removes. -const READER_REASON = /^pr-merge's (?:lane does not take its issue link|gate refuses its verifier verdict|gate holds it on its risk score) \(/; -const PERSON_WHO = "fix the reason, then remove the label, and the next scheduled run judges it again."; -const READER_WHO = "fix the reason, and pr-merge takes the label off once the head moves or, under `merge.lane-prs: native`, once a scheduled run reads the reason gone at this head; the next scheduled run then judges it again."; -const personNote = (reason, who = READER_REASON.test(reason) ? READER_WHO : PERSON_WHO) => +const personNote = (reason) => [ "", "", - `Unattended merge: native auto-merge leaves this pull request unarmed at head \`${HEAD}\`. A person must clear this reason, so the pr-merge lane added the \`needs-human\` label: ${who}`, + `Unattended merge: native auto-merge leaves this pull request unarmed at head \`${HEAD}\`. A person must clear this reason, so the pr-merge lane added the \`needs-human\` label: fix the reason, then remove the label, and the next scheduled run judges it again.`, "", "```text", reason, @@ -466,21 +461,6 @@ test("an unchanged person's refusal is not posted again", () => { assert.deepEqual(writes, { body: null, commentId: 77, label: "needs-human", liftedSince: "2026-10-23T10:00:00Z" }); }); -// A person's reader refusal posted in the earlier wording still stands: no -// second comment, and the label goes back as for the current wording. -test("a person's reader refusal in its earlier wording is not posted again", () => { - const reason = "pr-merge's gate holds it on its risk score (security-finding: 1 CRITICAL)"; - const writes = refusalWrites( - { headRefOid: HEAD }, - [lanesSaid(personNote(reason, PERSON_WHO))], - { clears: "person", reason }, - { bot: LANES.login, label: "needs-human" }, - ); - assert.deepEqual(writes, { body: null, commentId: 77, label: "needs-human", liftedSince: "2026-10-23T10:00:00Z" }); - assert.match(refusalBody(true, reason, HEAD, "needs-human"), /pr-merge takes the label off/); - assert.doesNotMatch(refusalBody(true, reason, HEAD, "needs-human"), /then remove the label/); -}); - test("a lane's refusal over a person's comment still edits it, with no label", () => { const writes = refusalWrites( { headRefOid: HEAD }, diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index b57fba8573..4ff65dd9e0 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -538,6 +538,7 @@ head: | `unresolved-conflict`, `protected-path-conflict` | GitHub reports the PR mergeable | | `feedback-protected-path` | the PR changes nothing under `.github/workflows/` | | `head-moved` (pr-merge's `babysit-merge-verify-escalation`, `babysit-merge-risk` and `babysit-merge-shared-copy-escalation`, with no pr-unblock escalation since the label, and the marked App comment written last before the label is pr-merge's and within a minute of it) | the newest App comment carrying each marker names, as ``at head `` ``, a 40-hex head other than the current one; pr-merge then judges the new head and escalates again if its refusal still holds | +| `refusal-cleared` (under `merge.lane-prs: native`, `enable-auto-merge`'s own unedited person refusal comment on a lanes App pull request, naming the current head, whose recorded verifier or risk cause is one the lanes clear: every code on `LANE_CAUSES`) | the gate's reader at that head finds nothing refusing, or only causes the lanes clear, read at least as far as the recorded one. A recorded cause a person clears (an issue link, a security finding, a trust-gate widening, a cost hold, a high path, an untrusted spec, the repeated-fail cap, any code off that list) is never read or lifted at its head (owner decision Q-CLEAR B): a person removes the label, or the `head-moved` clear does once the head moves. At most three reader runs a run | It removes only a label whose newest `labeled` event has the lanes App as its actor, and keeps it when a person commented, reviewed or left a review comment since that event (a bot account, such