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 e234ff53e9..1a31f93b22 100755 --- a/.github/scripts/pr-merge/clear-escalations.mjs +++ b/.github/scripts/pr-merge/clear-escalations.mjs @@ -53,15 +53,41 @@ // 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. // +// Under `merge.lane-prs: native`, enable-auto-merge's own person refusal +// 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) // GITHUB_REPOSITORY owner/repo // 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 +// 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"; @@ -83,6 +109,7 @@ import { spoken, } from "../pr-unblock/gate-source.mjs"; import { isProtectedPath } from "../pr-unblock/thread-scope.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; @@ -109,6 +136,38 @@ 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"; +// 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 +// 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 is a person's. +export const LANE_CAUSES = Object.freeze({ + 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 the `kind` reason (verifyHold's) names only causes the lanes clear: +// each "; "-separated code on LANE_CAUSES[kind]. +function laneCause(kind, reason) { + 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, " "); const reportError = (number, error) => @@ -143,7 +202,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)}`; @@ -161,6 +222,39 @@ function prMergeEscalations(appComments) { }); } +// enable-auto-merge's person refusal comment, when it is the only pr-merge +// 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 kind = Object.keys(READER_HOLDS).find((key) => reason.startsWith(READER_HOLDS[key])); + return kind !== undefined && refusalBody(true, reason, head, label) === newest.body ? { kind, reason } : null; +} + +// 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(now, held); +} + // 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) or @@ -186,7 +280,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 }) { +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)) { @@ -243,6 +337,33 @@ async function verdict({ github, repo, number, label, bot, mainSha }) { if (unread !== undefined) { return { reason: `the pr-merge comment carrying ${unread.marker} names no head this script can read` }; } + // 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` }; + } + 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 (!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 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}`, + ]); + } if (merged.some(({ head }) => head === pull.head.sha)) { return { reason: `a pr-merge escalation names the current head ${pull.head.sha}` }; } @@ -257,15 +378,20 @@ async function verdict({ github, repo, number, label, bot, mainSha }) { 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 +404,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))) { @@ -288,13 +416,13 @@ export async function main({ env = process.env, github } = {}) { ); 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; } try { - const result = await verdict({ github, repo, number, label, bot, mainSha }); + 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 353416d4dc..542a3c9a83 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, numbers = [7] }) { 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+$/, ""); @@ -39,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" }, 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 }; @@ -58,7 +66,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 +74,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; } @@ -500,3 +508,280 @@ 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. 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 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 }); +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 }; + +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, 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, 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 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" })], + // 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); + assert.deepEqual(w.writes().at(-1), DELETE, `${reason} ${evidence}`); + } +}); + +// 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 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 [ + [SPEC_CHANGED, refusing("verify:unreadable")], + [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 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")], + [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); + 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 () => { + 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 a person must clear, an issue link the trust hold refuses. + refusing("verify:untrusted-spec"), + 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 }); + 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)); + } +}); + +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, 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, NATIVE); + assert.deepEqual(w.writes(), []); + assert.deepEqual(w.reads, []); + } +}); + +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, []); + } +}); + +// 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("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 }); + 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 }, 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(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) { + 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 ef39f342d2..ea8bb0bb65 100755 --- a/.github/scripts/pr-merge/enable-auto-merge.mjs +++ b/.github/scripts/pr-merge/enable-auto-merge.mjs @@ -425,6 +425,14 @@ 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 (", +}); + // 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) { @@ -435,17 +443,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"; } 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 2438697ca4..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 @@ -1423,6 +1424,10 @@ jobs: # 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 or after the gate's rate-limit stop. + # 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: >- @@ -1430,9 +1435,15 @@ jobs: && steps.lane-token.outputs.token != '' && steps.mode.outputs.report-only == 'false' && steps.gate.outputs.rate-limited != 'true' - timeout-minutes: 3 + timeout-minutes: 15 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' || '' }} LANE_PRS: ${{ steps.mode.outputs.lane-prs }} run: node .base/.github/scripts/pr-merge/clear-escalations.mjs 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