diff --git a/.changes/pr-pipeline/feat-native-lane-refs-arm-5c2e81a4.md b/.changes/pr-pipeline/feat-native-lane-refs-arm-5c2e81a4.md new file mode 100644 index 0000000000..0d5b9036dd --- /dev/null +++ b/.changes/pr-pipeline/feat-native-lane-refs-arm-5c2e81a4.md @@ -0,0 +1,11 @@ +--- +bump: minor +--- + +### Changed + +- The `enable-auto-merge` activity arms a lanes App pull request whose issue link the merge gate takes, a plain `Refs: #` issue included, instead of only one whose body opens with `Closes #`. The link is read with the verdict, through the gate's own rule (`babysit_verify_evidence.py evidence --lane-link`, which runs `babysit_merge_policy._linked_issue`), on the scheduled and hand-started runs only: two or more `Refs:` issues, a `Refs:` line in any other form, a number that is no issue, two closing issues, a closing issue in another repository, and a failed read all leave the pull request unarmed, or disarm it. An event run reads no link, so it never disarms on one. +- The `enable-auto-merge` activity applies the merge gate's run-level refusals, which native auto-merge would otherwise skip. Each scheduled or hand-started run waits for the same run's `post-merge-dispatch` job, which keeps its post-merge report as a one-day artifact, and reads the lane's room from that report (`babysit_verify_evidence.py lane-room`, `babysit_merge_policy.lane_room`). While a demotion record, `merge.rung`, the loop setting or the rung cap holds the lane off, or a lane merge failed its post-merge check, nothing is armed and every armed lanes App pull request is disarmed. At `merge.max-in-flight`, counting the queue's lane entries, the lane's unverified merges and armed pull requests not yet queued, nothing more is armed. A missing report, one the gate would not reuse (`shared_refusal`: older than 30 minutes, as a re-run job reads the run's first report, a demotion record filed or changed since, or a lane merge landed since that it does not count), or a room that cannot be read, holds as a refusal does: nothing is armed and every armed lanes App pull request is disarmed. The report is read whenever `post-merge-dispatch` ran, so a later step of that job failing does not disarm anything. The job now holds `pull-requests: read` for the merge-queue and merge reads. +- In the merge queue, `ci-status` holds a lanes App pull request whose verifier pass no longer binds its live issue link. It reads the counted pass's `verdict.json` (one artifact download) and the closing issues (one GraphQL read), and `merge-holds.mjs` `linkHold` holds unless the body still closes, or with no closing issue still names by a plain `Refs:` line, the one issue the pass judged, linked the same way. This is pr-merge's reader rule (`verify:spec-changed`), so a body edited after arming, such as `Refs: #A` to `Closes #B`, no longer merges on a pass for the old issue. A trusted approval of the head lifts it, as it lifts the verifier hold. `refsIssue` moves to `merge-holds.mjs`, which `resolve-pr.mjs` re-exports. +- `enable-auto-merge` holds a lanes App pull request on the merge gate's risk score: `evidence --lane-reviews` runs `babysit_risk.score` and `holding` over the inputs the gate builds, at the lane room's effective rung. Each of these leaves it unarmed or disarms it: an open CRITICAL security-lane finding or a review not bound to the head (whether or not the lane applied its hold label), a symlink, submodule or other non-regular mode, a high path, the manifest review, the trust gate, and the cost review unless the owner's decision lifts it. The job now also holds `checks: read` for the lanes' annotations. +- `ci-status` fails a lanes App pull request while the lanes kill switch (`CLAUDE_LANES_DISABLED` or `CLAUDE_LANES_REPO_DISABLED`) reads anything but `false`, so one armed before the switch was turned on stops in the merge queue. It reads the vars context and makes no API call. Owner and Dependabot pull requests are not affected. diff --git a/.changes/source-control/feat-native-lane-refs-arm-7d1f3b96.md b/.changes/source-control/feat-native-lane-refs-arm-7d1f3b96.md new file mode 100644 index 0000000000..f3735a1ea9 --- /dev/null +++ b/.changes/source-control/feat-native-lane-refs-arm-7d1f3b96.md @@ -0,0 +1,9 @@ +--- +bump: minor +--- + +### Added + +- `babysit_verify_evidence.py evidence --lane-link` first reads the pull request's issue link by the pr-merge lane's own rule (`babysit_merge_policy._linked_issue`: one closing issue in this repository, or, with none, the one issue its plain `Refs:` lines name) and adds `ineligible` to its line: null when the lane takes the link, else why not, with refusal `verify:ineligible` and no verdict or config read. A failed read refuses. The verdict read that follows takes the link read's pull request instead of reading it again. +- `babysit_verify_evidence.py lane-room` prints whether the pr-merge lane may merge one more of its own pull requests now (`babysit_merge_policy.lane_room`): the gate's run-level refusals (another authority, a post-merge refusal, the effective rung off, a demotion included) as `refusal`, a failed read, or a post-merge answer the gate would not reuse for its age or a changed demotion record (`shared_refusal`'s checks), as `unreadable`, and otherwise the merges in flight against `merge.max-in-flight`. `InFlight` now carries that count. +- `babysit_verify_evidence.py evidence --lane-reviews --base-sha --rung ` adds `reviews` to a line whose refusal is null: null when the gate would not hold the pull request on its risk score (`lane_risk_refusal`: `babysit_risk.score` and `holding` over the inputs `babysit_merge.score_unattended` builds for a lane pull request, the cost decision included), else why. A failed read, a moved head, or no rung holds. `lane-room` reports the effective rung as `rung`. diff --git a/.github/scripts/pr-merge/enable-auto-merge.mjs b/.github/scripts/pr-merge/enable-auto-merge.mjs index df063e5fec..1faa628db6 100755 --- a/.github/scripts/pr-merge/enable-auto-merge.mjs +++ b/.github/scripts/pr-merge/enable-auto-merge.mjs @@ -8,8 +8,6 @@ // A pull request is armed only when all of these hold: // - The lanes App opened it (merge.agent-identity), into main, from a branch // of this repository, and it is not a draft. -// - Its body opens with `Closes #`: a `Refs:` pull request is merged -// by a person (report-outcome.mjs). // - It carries no hold: no `do-not-merge` and no human-gated label (the // tracker's role label, `needs-human` by default), and no body hold // (merge-holds.mjs). @@ -26,12 +24,17 @@ // decision: merge-holds.costHold and costDecision, which ci-status runs too. // - No person turned its auto-merge off: the newest auto-merge-disabled event // by a user account keeps it off until a person turns it on again. -// - pr-merge's gate would take its verifier verdict at the head just read: -// babysit_verify_evidence.py `evidence` refuses nothing (verifyHold), so a -// pr-verify PASS or PASS-NOTES stands at that head, no FAIL is at it, no -// earlier FAIL is uncleared, FAILs sit at fewer than merge.max-verify-fails -// heads, and the spec is unchanged and trusted. Only the scheduled run and -// a hand-started one read it (READER_EVENTS), last, for a pull request +// - pr-merge's gate would take its issue link and its verifier verdict at the +// head just read: babysit_verify_evidence.py `evidence --lane-link` refuses +// nothing (verifyHold). The link is the gate's own rule +// (babysit_merge_policy._linked_issue): one closing issue in this +// repository, or, with none, the one issue its plain `Refs: #` lines +// name. Then a pr-verify PASS or PASS-NOTES stands at that head, no FAIL is +// at it, no earlier FAIL is uncleared, FAILs sit at fewer than +// merge.max-verify-fails heads, and the spec is unchanged and trusted. A +// read that fails, or a line without the link's answer, refuses. Only the +// scheduled run and +// a hand-started one read them (READER_EVENTS), last, for a pull request // every other rule would arm or keep armed, sharing one artifact listing // and run cache in the run. An event run (the issue lane, a person's edit, // unlabel or ready) never arms and reads no verdict: it only disarms on a @@ -40,6 +43,25 @@ // head, so a new head is armed only after the reader judges it, and in the // merge queue it checks the pass again at the head it merges // (merge-holds.mjs verdictHold). +// - pr-merge's gate would let its lane merge one more now: babysit_verify_evidence.py +// `lane-room` (babysit_merge_policy.lane_room) reads, once per reader run, +// the run-level refusals the gate applies to every lane candidate. While +// the gate holds the lane off (a demotion record, merge.rung, the loop +// setting or the rung cap off, a post-merge failure, another authority) +// nothing is armed and every armed lanes App pull request is disarmed. +// Below merge.max-in-flight, counting the merge queue's lane entries, the +// lane's unverified merges and the armed pull requests not yet queued, it +// arms; at the cap it arms nothing more. A room that cannot be read (no +// post-merge report, a failed read) holds as a refusal does: nothing is +// armed and every armed lanes App pull request is disarmed. +// - pr-merge's gate would not hold it on its risk score: `evidence +// --lane-reviews --rung ` runs the gate's scorer +// (babysit_risk.score, holding) over the inputs the gate builds, the +// review lanes' evidence bound to the head included. So an open CRITICAL +// security finding or a review not bound to the head (whether or not the +// lane applied its hold label), a symlink, submodule or other non-regular +// mode, a high path, the manifest review, the trust gate and the cost +// review hold. Read only after the verdict allows. // Arming passes the head just read as expectedHeadOid, so a push in between // fails the arm rather than arming a head nobody read. An armed pull request // that fails any rule is disarmed. While merge.report-only is true it reports @@ -57,6 +79,9 @@ // TRUSTED_ACTORS the base trusted-actors.json, for the owner of a cost // decision (default .base/.github/standards/trusted-actors/trusted-actors.json) // EVENT_NAME the run's event; only READER_EVENTS read verdicts and arm +// POSTMERGE_RESULT this run's babysit_postmerge.py report, for lane-room +// RUNG_CAP vars.CLAUDE_LANES_MERGE_RUNG_CAP (empty is no cap; +// unset holds the lane off, as the gate reads it) // BABYSIT_VERIFY_*, BABYSIT_RUN_CACHE the reader's Actions token (the // job's GITHUB_TOKEN), saved listing, head-ref map and // run cache, as the gate's @@ -78,7 +103,6 @@ const SHA = /^[0-9a-f]{40}$/; const BOT_LOGIN = /^([a-z0-9][a-z0-9-]*)\[bot\]$/; // babysit_merge_policy.MERGE_HOLD_LABELS, plus the human-gated label. const HOLD_LABELS = Object.freeze(["do-not-merge"]); -const CLOSES = /^Closes #[1-9][0-9]*\r?$/; // 100 pull requests a page; more than this many open is a listing the run // stops on rather than one it reads only part of. const MAX_PAGES = 10; @@ -87,6 +111,8 @@ const MAX_PAGES = 10; const MAX_FILE_PAGES = 3; const VERIFY_SCRIPT = ".base/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py"; +// Into main, by name: the gate reads the default branch live +// (babysit_merge_policy._base_refusal), while this assumes it is main. export const PULLS_QUERY = `query($owner: String!, $name: String!, $after: String) { repository(owner: $owner, name: $name) { pullRequests(states: OPEN, baseRefName: "main", first: 100, after: $after) { @@ -138,13 +164,7 @@ export function holdReason(pull, humanGated) { return `it carries ${label}`; } const body = String(pull.body ?? ""); - if (BODY_HOLD.test(body)) { - return "its body holds it"; - } - if (!CLOSES.test(body.split("\n")[0])) { - return "its body does not open with Closes #"; - } - return null; + return BODY_HOLD.test(body) ? "its body holds it" : null; } const whole = (connection) => Array.isArray(connection?.nodes) && connection.totalCount <= connection.nodes.length; @@ -194,7 +214,7 @@ async function changedFiles(github, slug, number) { return null; } -// Why the gate's verifier `evidence` (babysit_verify_evidence.py's report) +// 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) { if (evidence === null || typeof evidence !== "object" || !("refusal" in evidence)) { @@ -203,18 +223,35 @@ export function verifyHold(evidence, head) { if (evidence.head !== head) { return "its verifier verdict was read at another head"; } - return evidence.refusal === null ? null : `pr-merge's gate refuses its verifier verdict (${escapeCommand(evidence.refusal)})`; + if (typeof evidence.ineligible === "string") { + return `pr-merge's lane does not take its issue link (${escapeCommand(evidence.ineligible)})`; + } + if (evidence.refusal !== null) { + return `pr-merge's gate refuses its verifier 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 evidence.reviews === null ? null : "its risk score could not be read"; } // The gate's own reader, one process per pull request; the run shares its // artifact listing, head-ref map and run cache through the BABYSIT_* files. export function pythonVerify(env) { - const verify = async ({ number, headRefOid: head, baseRefOid: base }) => { + // `rung`: the lane room's effective rung, which the gate scores at. + const verify = async ({ number, headRefOid: head, baseRefOid: base }, rung) => { if (!SHA.test(String(base))) { throw new Error("its base commit is not a commit SHA"); } // --base-sha: the gate's FAIL cap, the lower of the base's and main's. - const args = [env.VERIFY_EVIDENCE_SCRIPT || VERIFY_SCRIPT, "evidence", "--repo", env.GITHUB_REPOSITORY, "--base-sha", base]; + const args = [env.VERIFY_EVIDENCE_SCRIPT || VERIFY_SCRIPT, "evidence", "--lane-link", "--lane-reviews", "--repo", env.GITHUB_REPOSITORY, "--base-sha", base]; + if (typeof rung === "string") { + args.push("--rung", rung); + } const done = spawnSync("python3", [...args, String(number), head], { encoding: "utf8", env, timeout: VERIFY_TIMEOUT_MS }); if (done.status !== 0) { throw new Error(`babysit_verify_evidence.py exited ${done.status ?? done.signal}: ${String(done.stderr ?? "").trim().split("\n").at(-1)}`); @@ -224,14 +261,56 @@ export function pythonVerify(env) { return verify; } +// The gate's run-level answer for its lane (babysit_verify_evidence.py +// `lane-room`), read with the reader's Actions token when it has one, as the +// merge job reads the post-merge check, so the reads stay off the lane App's +// quota. +export function pythonRoom(env) { + return async () => { + const args = [env.VERIFY_EVIDENCE_SCRIPT || VERIFY_SCRIPT, "lane-room", "--repo", env.GITHUB_REPOSITORY, "--postmerge-result", env.POSTMERGE_RESULT ?? ""]; + if (env.RUNG_CAP !== undefined) { + args.push("--rung-cap", env.RUNG_CAP); + } + const token = env.BABYSIT_VERIFY_GH_TOKEN; + const roomEnv = token ? { ...env, GH_TOKEN: token, API_BUDGET_TOKEN: "github-token" } : env; + const done = spawnSync("python3", args, { encoding: "utf8", env: roomEnv, timeout: VERIFY_TIMEOUT_MS }); + if (done.status !== 0) { + throw new Error(`babysit_verify_evidence.py lane-room exited ${done.status ?? done.signal}: ${String(done.stderr ?? "").trim().split("\n").at(-1)}`); + } + return JSON.parse(String(done.stdout).trim().split("\n").at(-1)); + }; +} + +// `lane-room`'s answer as the run applies it, or an unreadable one when it is +// not of that shape: { refusal, unreadable, count, cap, queued }. +export function readRoom(answer) { + const text = (value) => value === null || typeof value === "string"; + const whole = (value, least) => Number.isSafeInteger(value) && value >= least; + if ( + answer === null || + typeof answer !== "object" || + !text(answer.refusal) || + !text(answer.unreadable) || + !whole(answer.count, 0) || + !whole(answer.cap, 1) || + !Array.isArray(answer.queued) || + !answer.queued.every((number) => whole(number, 1)) || + typeof answer.rung !== "string" + ) { + return { refusal: null, unreadable: "its answer is not lane-room's", count: 0, cap: 1, queued: [], rung: "off" }; + } + return answer; +} + // The runs that read the verifier verdict and so may arm: the schedule, and a // hand-started run, which a person starts to retry a pull request that looks // stuck. export const READER_EVENTS = Object.freeze(["schedule", "workflow_dispatch"]); // The reader's per pull request limit, and when the run stops starting one: -// a read started at the budget ends by VERIFY_BUDGET_MS + VERIFY_TIMEOUT_MS, -// 9 minutes, inside the step's 10 (pr-merge.yml), so no read is cut off and -// the rest of the pull requests are left for the next run. +// the room read (at most VERIFY_TIMEOUT_MS) comes first, then a read started +// at the budget ends by VERIFY_BUDGET_MS + VERIFY_TIMEOUT_MS, 13 minutes in +// all, inside the step's 14 (pr-merge.yml), so no read is cut off and the +// rest of the pull requests are left for the next run. export const VERIFY_TIMEOUT_MS = 4 * 60_000; export const VERIFY_BUDGET_MS = 5 * 60_000; @@ -296,7 +375,7 @@ async function listPulls(github, owner, name) { throw new Error(`more than ${MAX_PAGES * 100} open pull requests into main`); } -export async function main({ env = process.env, github, verify } = {}) { +export async function main({ env = process.env, github, verify, room } = {}) { try { const repo = REPO.exec(env.GITHUB_REPOSITORY ?? ""); if (!repo) { @@ -317,11 +396,50 @@ export async function main({ env = process.env, github, verify } = {}) { verify ??= pythonVerify(env); const reads = READER_EVENTS.includes(env.EVENT_NAME); const pulls = await listPulls(github, repo[1], repo[2]); + const lanes = (pull) => pull.author?.__typename === "Bot" && pull.author.login === slug; + // Only a reader run arms, so only it reads the room. + let lane = null; + if (reads) { + try { + lane = readRoom(await (room ?? pythonRoom(env))()); + } catch (error) { + lane = readRoom(null); + lane.unreadable = escapeCommand(error?.message ?? error); + } + console.log(`enable-auto-merge: lane room: ${escapeCommand(JSON.stringify(lane))}`); + } + // After the room read, so the verdict reads keep their whole budget. const deadline = Date.now() + VERIFY_BUDGET_MS; + // Merges in flight: the gate's count, plus each armed lanes App pull + // request the queue has not taken yet, which GitHub enqueues on its own. + let inFlight = lane === null ? 0 : lane.count + pulls.filter((pull) => lanes(pull) && pull.autoMergeRequest != null && !lane.queued.includes(pull.number)).length; + // Why the room keeps `pull` from arming, with `disarm` when it also turns + // an armed one off; null when it has room. + const roomHold = (pull) => { + if (lane === null || !lanes(pull)) { + return null; + } + if (lane.refusal !== null) { + return { disarm: true, reason: `pr-merge's gate holds its lane off (${lane.refusal})` }; + } + // An unknown room holds as a refusal does: unknown holds. + if (lane.unreadable !== null) { + return { disarm: true, reason: `the lane's room could not be read (${lane.unreadable})` }; + } + if (pull.autoMergeRequest != null) { + return null; + } + return inFlight >= lane.cap ? { disarm: false, reason: `merge.max-in-flight is ${lane.cap} and ${inFlight} merges are in flight` } : null; + }; let failed = 0; for (const pull of pulls) { let { action, reason } = decide(pull, { bot: slug, humanGated, holdPaths }); let detail = null; + // Checked before the reads, so a held lane spends none. + const held = ["read", "verify", "arm"].includes(action) ? roomHold(pull) : null; + if (held !== null) { + ({ action, reason } = { action: held.disarm && pull.autoMergeRequest != null ? "disarm" : "none", reason: held.reason }); + } if (action === "read" && !reads && pull.autoMergeRequest == null) { ({ action, reason } = { action: "none", reason: "only a scheduled or hand-started run arms it, on the gate's verifier verdict" }); } @@ -349,12 +467,20 @@ export async function main({ env = process.env, github, verify } = {}) { } else if (action === "verify") { let evidence = null; try { - evidence = await verify(pull); + evidence = await verify(pull, lane?.rung); } catch (error) { console.log(`enable-auto-merge: #${pull.number} verifier verdict did not read: ${escapeCommand(error?.message ?? error)}`); } ({ action, reason } = decide(pull, { bot: slug, humanGated, holdPaths }, { ...detail, verify: evidence })); } + // A pull request armed earlier in this run counts toward the cap. + const full = action === "arm" ? roomHold(pull) : null; + if (full !== null) { + ({ action, reason } = { action: "none", reason: full.reason }); + } + if (action === "arm") { + inFlight += 1; + } if (action === "none") { if (reason !== "not the lanes App's") { console.log(`enable-auto-merge: #${pull.number} left: ${escapeCommand(reason)}`); diff --git a/.github/scripts/pr-merge/enable-auto-merge.test.mjs b/.github/scripts/pr-merge/enable-auto-merge.test.mjs index 240fd235cb..bac7dbf3a8 100644 --- a/.github/scripts/pr-merge/enable-auto-merge.test.mjs +++ b/.github/scripts/pr-merge/enable-auto-merge.test.mjs @@ -1,10 +1,11 @@ // Expected values come from the native-merge plan's arm-merge item and the // owner answers on it: a lanes App pull request is armed for native auto-merge (squash, at the head // read) with no session, and is never armed, or is disarmed, while it carries -// a `Hold:` line, do-not-merge or the human-gated label. A Refs pull request -// is merged by a person (report-outcome.mjs), so it is never armed. The body -// hold is the gate's own rule, babysit_merge_policy.BODY_MERGE_HOLD_RE, so the -// parity test reads its verdicts from Python. +// a `Hold:` line, do-not-merge or the human-gated label. Its issue link is +// the gate's own rule (docs/conventions/pr-pipeline/README.md, "Eligibility": +// one closing issue, or one plain `Refs: #` issue), read through the gate's +// Python, as is the body hold, babysit_merge_policy.BODY_MERGE_HOLD_RE, so +// the parity tests read their verdicts from Python. import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; import { mkdtempSync, writeFileSync } from "node:fs"; @@ -24,6 +25,8 @@ import { decide, READER_EVENTS, main, + pythonRoom, + readRoom, verifyHold, } from "./enable-auto-merge.mjs"; import { COMMENT_EDITED_QUERY, PULL_OPENED_QUERY, VERIFY_PATH, verdictHold } from "./merge-holds.mjs"; @@ -73,7 +76,7 @@ const said = (login, body, at, association = "OWNER", typename = "User") => ({ createdAt: at, authorAssociation: association, }); -const PASSED = { refusal: null, head: HEAD }; +const PASSED = { refusal: null, head: HEAD, ineligible: null, reviews: null }; const COST_LINK = `https://github.com/${REPO}/issues/1040#issuecomment-7`; const COST_COMMENT = { issue_url: `https://api.github.com/repos/${REPO}/issues/1040`, @@ -83,6 +86,9 @@ const COST_COMMENT = { node_id: "IC_7", created_at: "2026-10-22T12:00:00Z", }; +// lane-room's answer for a lane with room: nothing in flight of five. +const ROOM = { refusal: null, unreadable: null, count: 0, cap: 5, queued: [], rung: "C2" }; +const room = (answer = ROOM) => async () => answer; const labels = (...names) => ({ totalCount: names.length, nodes: names.map((name) => ({ name })) }); function world(pulls, { failArm = false } = {}) { @@ -130,7 +136,7 @@ function world(pulls, { failArm = false } = {}) { if (found?.verdict instanceof Error) { throw found.verdict; } - return found?.verdict ?? { refusal: null, head }; + return found?.verdict ?? { refusal: null, head, ineligible: null, reviews: null }; }; const writes = () => calls @@ -139,13 +145,13 @@ function world(pulls, { failArm = false } = {}) { return { github, writes, verify, verified, calls }; } -async function run(pulls, env = {}, options = {}) { +async function run(pulls, env = {}, options = {}, lane = ROOM) { const { github, writes, verify, verified } = world(pulls, options); const lines = []; const log = console.log; console.log = (line) => lines.push(line); try { - const code = await main({ env: { ...ENV, ...env }, github, verify }); + const code = await main({ env: { ...ENV, ...env }, github, verify, room: room(lane) }); return { code, writes: writes(), lines, verified }; } finally { console.log = log; @@ -191,15 +197,13 @@ test("an armed pull request that now holds is disarmed", async () => { ]); }); -test("only the lanes App's ready Closes pull requests from this repository are armed", async () => { +test("only the lanes App's ready pull requests from this repository are armed", async () => { const { writes } = await run([ pull(1, { author: { __typename: "User", login: "kyle-sexton" } }), pull(2, { author: { __typename: "Bot", login: "dependabot" } }), pull(3, { author: { __typename: "User", login: "melodic-automation-lanes" } }), pull(4, { isDraft: true }), pull(5, { isCrossRepository: true }), - pull(6, { body: "Refs: #12\n\n## Summary" }), - pull(7, { body: "## Summary\n\nCloses #12" }), pull(8, { headRefOid: "abc" }), pull(9, { labels: { totalCount: 101, nodes: [] } }), ]); @@ -275,7 +279,7 @@ test("the body hold agrees with the gate's BODY_MERGE_HOLD_RE", () => { // The body the issue lane opens its pull request with (report-outcome.mjs // decide), so a change to either side that stops a clean lane pull request // from arming, or arms a held one, fails here. -test("report-outcome's own pull request body arms, and its hold and Refs bodies do not", () => { +test("report-outcome's own Closes and Refs pull request bodies arm, and its hold bodies do not", () => { const plan = (overrides) => decideOutcome({ outcome: { exit: "done", reason: "", title: "docs(work-items): fix a typo", summary: "Fixed it." }, @@ -296,7 +300,7 @@ test("report-outcome's own pull request body arms, and its hold and Refs bodies assert.deepEqual(verdict({}), { action: "arm", reason: "no hold" }); assert.equal(verdict({ humanMerge: true }).action, "none"); assert.equal(verdict({ changedFiles: [".github/workflows/x.yml"] }).action, "none"); - assert.equal(verdict({ outcome: { exit: "done", reason: "", title: "t", summary: "s", link: "refs" } }).action, "none"); + assert.deepEqual(verdict({ outcome: { exit: "done", reason: "", title: "t", summary: "s", link: "refs" } }), { action: "arm", reason: "no hold" }); }); // The gate's human veto (babysit_merge.human_veto_logins): each member's @@ -515,7 +519,14 @@ test("verifyHold allows only the reader's null refusal at the head read", () => assert.match(verifyHold({ refusal: null, head: "c".repeat(40) }, HEAD), /another head/); assert.match(verifyHold(null, HEAD), /could not be read/); assert.match(verifyHold({ head: HEAD }, HEAD), /could not be read/); - assert.match(verifyHold({ refusal: "verify:too-old", head: HEAD }, HEAD), /verify:too-old/); + assert.match(verifyHold({ refusal: "verify:too-old", head: HEAD, ineligible: null }, HEAD), /verify:too-old/); + // The link's answer: a refusal names it, and a line without one allows nothing. + assert.match(verifyHold({ refusal: "verify:ineligible", head: HEAD, ineligible: "refs-issues:2" }, HEAD), /does not take its issue link \(refs-issues:2\)/); + assert.match(verifyHold({ refusal: null, head: HEAD }, HEAD), /issue link could not be read/); + assert.match(verifyHold({ refusal: null, head: HEAD, ineligible: false }, HEAD), /issue link could not be read/); + // The review lanes' answer: a hold names it, and a line without one allows nothing. + assert.match(verifyHold({ refusal: null, head: HEAD, ineligible: null, reviews: "security-finding: 1 open" }, HEAD), /holds it on its risk score \(security-finding: 1 open\)/); + assert.match(verifyHold({ refusal: null, head: HEAD, ineligible: null }, HEAD), /risk score could not be read/); }); // The merge queue's check (merge-holds.mjs verdictHold): a pass counts only @@ -600,7 +611,8 @@ test("a reader that fails, prints no JSON or reports another head arms nothing a exits: "import sys\nsys.stderr.write('boom\\n')\nsys.exit(1)\n", garbage: "print('not json')\n", other: `import json\nprint(json.dumps({"refusal": None, "head": "${"c".repeat(40)}"}))\n`, - passes: `import json, sys\nassert sys.argv[-4:-2] == ["--base-sha", "${"d".repeat(40)}"], sys.argv\nprint(json.dumps({"refusal": None, "head": sys.argv[-1]}))\n`, + unlinked: `import json, sys\nprint(json.dumps({"refusal": None, "head": sys.argv[-1]}))\n`, + passes: `import json, sys\nassert sys.argv[2:4] == ["--lane-link", "--lane-reviews"] and sys.argv[-6:-2] == ["--base-sha", "${"d".repeat(40)}", "--rung", "C2"], sys.argv\nprint(json.dumps({"refusal": None, "head": sys.argv[-1], "ineligible": None, "reviews": None}))\n`, }; for (const [name, body] of Object.entries(scripts)) { writeFileSync(join(dir, `${name}.py`), body); @@ -610,6 +622,7 @@ test("a reader that fails, prints no JSON or reports another head arms nothing a ["exits", [{ mutation: "disarm", id: "PR_2" }]], ["garbage", [{ mutation: "disarm", id: "PR_2" }]], ["other", [{ mutation: "disarm", id: "PR_2" }]], + ["unlinked", [{ mutation: "disarm", id: "PR_2" }]], ["passes", [{ mutation: "arm", id: "PR_1", head: HEAD }]], ]) { const { github, writes } = world([pull(1), pull(2, { autoMergeRequest: armed })]); @@ -617,10 +630,291 @@ test("a reader that fails, prints no JSON or reports another head arms nothing a const log = console.log; console.log = () => {}; try { - await main({ env, github }); + await main({ env, github, room: room() }); } finally { console.log = log; } assert.deepEqual(writes(), want, name); } }); + +// The issue link through the gate's own CLI (`evidence --lane-link`) and rule +// (babysit_merge_policy._linked_issue), GitHub's answers stubbed at gh and the +// verdict a pass at the head, so only the link decides. Expected values are +// docs/conventions/pr-pipeline/README.md "Eligibility": one closing issue, or +// one plain `Refs: #` issue; two `Refs:` issues refuse; a failed read refuses. +test("the gate's link rule arms a Closes or one plain Refs pull request, not two Refs or a failed read", async () => { + const dir = mkdtempSync(join(tmpdir(), "enable-auto-merge-link-")); + const wrapper = join(dir, "lane_link.py"); + writeFileSync( + wrapper, + `import json, os, subprocess, sys +sys.path.insert(0, os.environ["SCRIPTS"]) +from unittest import mock +import babysit_merge_policy as policy +import babysit_postmerge as postmerge +import babysit_verify_evidence as ve + +PULLS = json.loads(os.environ["PULLS"]) + +def gh(args, **_): + n = next(a for a in args if a.startswith("n="))[2:] + def ok(data): + return subprocess.CompletedProcess(args, 0, json.dumps({"data": {"repository": data}}), "") + if any("issueOrPullRequest" in a for a in args): + node = {"__typename": "Issue", "number": int(n), "repository": {"nameWithOwner": os.environ["GITHUB_REPOSITORY"]}} + return ok({"issueOrPullRequest": node}) + found = PULLS[n] + if found.get("error"): + return subprocess.CompletedProcess(args, 1, "", "gh: boom (HTTP 502)") + nodes = [{"number": c, "repository": {"nameWithOwner": os.environ["GITHUB_REPOSITORY"]}} for c in found["closing"]] + return ok({"pullRequest": {"body": found["body"], "closingIssuesReferences": {"totalCount": len(nodes), "nodes": nodes}}}) + +def passes(self, number, head, pr=None): + return ve.Evidence(None, head, (ve.Verdict(head, "pass", 1),), 0) + +argv = sys.argv[1:] +pr = lambda repo, number: {"number": number, "head": {"sha": argv[-1]}} +with mock.patch.object(policy, "gh_capture", gh), mock.patch.object(postmerge, "gh_capture", gh), mock.patch.object(ve.Reader, "evidence", passes), mock.patch.object(policy, "pull_request", pr), mock.patch.object(ve, "lane_risk_refusal", lambda *args: None), mock.patch.object(policy, "read_config_at", lambda repo, sha: policy.PipelineConfig(sha, "C2", "pr-merge", "agent[bot]", None, (), "c2-mechanical")), mock.patch.object(policy, "with_default_branch", lambda repo, base, *rest: base): + sys.exit(ve.main(argv)) +`, + ); + const body = (text) => `${text}\n\n## Summary\n\nDone.`; + const pulls = { + 1: { body: body("Closes #11"), closing: [11] }, + 2: { body: body("Refs: #12"), closing: [] }, + 3: { body: body("Refs: #12\nRefs: #13"), closing: [] }, + 4: { body: body("Refs: #12"), closing: [], error: true }, + 5: { body: body("Refs: #12\nRefs: #13"), closing: [] }, + }; + const armed = { enabledAt: "2026-10-23T10:00:00Z" }; + const { github, writes } = world([1, 2, 3, 4].map((n) => pull(n, { body: pulls[n].body })).concat(pull(5, { body: pulls[5].body, autoMergeRequest: armed }))); + const scripts = fileURLToPath(new URL("../../../plugins/source-control/skills/babysit-prs/scripts", import.meta.url)); + const env = { ...ENV, PATH: process.env.PATH, SCRIPTS: scripts, PULLS: JSON.stringify(pulls), VERIFY_EVIDENCE_SCRIPT: wrapper }; + const lines = []; + const log = console.log; + console.log = (line) => lines.push(line); + try { + assert.equal(await main({ env, github, room: room() }), 0); + } finally { + console.log = log; + } + assert.deepEqual(writes(), [ + { mutation: "arm", id: "PR_1", head: HEAD }, + { mutation: "arm", id: "PR_2", head: HEAD }, + { mutation: "disarm", id: "PR_5" }, + ]); + assert.ok(lines.some((line) => /#3 left: pr-merge's lane does not take its issue link \(refs-issues:2\)/.test(line)), lines.join("\n")); + assert.ok(lines.some((line) => /#4 left: pr-merge's lane does not take its issue link \(unreadable: /.test(line)), lines.join("\n")); +}); + +// An event run reads no link and no verdict, so it never disarms an armed +// Refs pull request on its link; the scheduled run judges it. +test("an event run leaves an armed Refs pull request armed", async () => { + const armed = { enabledAt: "2026-10-23T10:00:00Z" }; + const { writes, verified } = await run([pull(1, { autoMergeRequest: armed, body: "Refs: #12\n\n## Summary" })], { EVENT_NAME: "pull_request_target" }); + assert.deepEqual(writes, []); + assert.deepEqual(verified, []); +}); + +// The gate's run-level refusals, which native auto-merge would skip +// (babysit_merge_policy.lane_room): a demotion record or a rung held off +// stops the lane, and merge.max-in-flight caps what is in flight. +test("while the gate holds its lane off, nothing is armed and armed lane pull requests are disarmed", async () => { + const armed = { enabledAt: "2026-10-23T10:00:00Z" }; + for (const refusal of ["rung-off:demotion", "rung-off:rungCap", "post-merge:failure:red-main#40"]) { + const { writes, verified, lines } = await run( + [pull(1), pull(2, { autoMergeRequest: armed }), pull(3, { author: { __typename: "User", login: "kyle-sexton" }, autoMergeRequest: armed })], + {}, + {}, + { ...ROOM, refusal }, + ); + assert.deepEqual(writes, [{ mutation: "disarm", id: "PR_2" }], refusal); + // A held lane reads no verdict. + assert.deepEqual(verified, [], refusal); + assert.ok(lines.some((line) => line.includes(`#1 left: pr-merge's gate holds its lane off (${refusal})`)), refusal); + } +}); + +test("at merge.max-in-flight nothing more is armed, counting armed pull requests the queue has not taken", async () => { + const armed = { enabledAt: "2026-10-23T10:00:00Z" }; + // Four in flight by the gate's count, one armed and not queued: the cap of five is reached. + let result = await run([pull(1), pull(2, { autoMergeRequest: armed })], {}, {}, { ...ROOM, count: 4, cap: 5 }); + assert.deepEqual(result.writes, []); + assert.deepEqual(result.verified, [2]); + assert.ok(result.lines.some((line) => /#1 left: merge\.max-in-flight is 5 and 5 merges are in flight/.test(line))); + // An armed pull request already in the queue is in the gate's count, not counted twice. + result = await run([pull(1), pull(2, { autoMergeRequest: armed })], {}, {}, { ...ROOM, count: 4, cap: 5, queued: [2] }); + assert.deepEqual(result.writes, [{ mutation: "arm", id: "PR_1", head: HEAD }]); + // Each arm in the run counts: three in flight of five arms two of three. + result = await run([pull(1), pull(2), pull(3)], {}, {}, { ...ROOM, count: 3, cap: 5 }); + assert.deepEqual( + result.writes.map(({ id }) => id), + ["PR_1", "PR_2"], + ); +}); + +// Unknown holds: a room that cannot be read (no post-merge report, a failed +// read) holds as the gate's refusal does, so an armed lane pull request is +// disarmed rather than left to merge while the gate refuses. +test("a room that cannot be read arms nothing and disarms every armed lane pull request", async () => { + const armed = { enabledAt: "2026-10-23T10:00:00Z" }; + const pulls = [pull(1), pull(2, { autoMergeRequest: armed }), pull(3, { author: { __typename: "User", login: "kyle-sexton" }, autoMergeRequest: armed })]; + for (const lane of [{ ...ROOM, unreadable: "post-merge answer missing or unreadable" }, { refusal: null }, null]) { + const { writes, verified, lines } = await run(pulls, {}, {}, lane); + assert.deepEqual(writes, [{ mutation: "disarm", id: "PR_2" }], JSON.stringify(lane)); + assert.deepEqual(verified, [], JSON.stringify(lane)); + assert.ok(lines.some((line) => line.includes("#2 disarmed: the lane's room could not be read")), JSON.stringify(lane)); + } + const { github, writes } = world(pulls); + const log = console.log; + console.log = () => {}; + try { + await main({ + env: ENV, + github, + verify: async ({ headRefOid: head }) => ({ refusal: null, head, ineligible: null, reviews: null }), + room: async () => { + throw new Error("lane-room exited 1"); + }, + }); + } finally { + console.log = log; + } + assert.deepEqual(writes(), [{ mutation: "disarm", id: "PR_2" }]); +}); + +// An event run reads no room, so it never disarms on one. +test("an event run leaves an armed lane pull request armed whatever the room", async () => { + const armed = { enabledAt: "2026-10-23T10:00:00Z" }; + const { writes } = await run([pull(2, { autoMergeRequest: armed })], { EVENT_NAME: "pull_request_target" }, {}, null); + assert.deepEqual(writes, []); +}); + +test("an event run reads no room", async () => { + let asked = 0; + const { github } = world([pull(1)]); + const log = console.log; + console.log = () => {}; + try { + await main({ + env: { ...ENV, EVENT_NAME: "pull_request_target" }, + github, + room: async () => { + asked += 1; + return ROOM; + }, + }); + } finally { + console.log = log; + } + assert.equal(asked, 0); +}); + +test("readRoom takes only lane-room's shape", () => { + assert.deepEqual(readRoom(ROOM), ROOM); + for (const bad of [null, "x", { ...ROOM, cap: 0 }, { ...ROOM, count: -1 }, { ...ROOM, queued: ["3"] }, { ...ROOM, refusal: 1 }, { ...ROOM, unreadable: undefined }]) { + assert.ok(readRoom(bad).unreadable, JSON.stringify(bad)); + } +}); + +test("pythonRoom passes the post-merge report, the rung cap as read, and the reader's token", async () => { + const dir = mkdtempSync(join(tmpdir(), "enable-auto-merge-room-")); + const script = join(dir, "room.py"); + writeFileSync( + script, + `import json, os, sys +print(json.dumps({"argv": sys.argv[1:], "token": os.environ.get("GH_TOKEN"), "budget": os.environ.get("API_BUDGET_TOKEN")})) +`, + ); + const env = { PATH: process.env.PATH, GITHUB_REPOSITORY: REPO, VERIFY_EVIDENCE_SCRIPT: script, POSTMERGE_RESULT: "/tmp/post-merge.json", GH_TOKEN: "lane", BABYSIT_VERIFY_GH_TOKEN: "job" }; + // An empty cap is no cap, so it is passed; an unset one is not, which the gate reads as off. + assert.deepEqual(await pythonRoom({ ...env, RUNG_CAP: "" })(), { + argv: ["lane-room", "--repo", REPO, "--postmerge-result", "/tmp/post-merge.json", "--rung-cap", ""], + token: "job", + budget: "github-token", + }); + assert.deepEqual((await pythonRoom(env)()).argv, ["lane-room", "--repo", REPO, "--postmerge-result", "/tmp/post-merge.json"]); +}); + +// The gate holds on the security lane's open CRITICAL findings +// (babysit_risk LANES, security-finding) and on a review not bound to the +// head, whether or not the lane applied its hold label. +test("a review-lane hold the gate reads arms nothing and disarms an armed pull request", async () => { + const armed = { enabledAt: "2026-10-23T10:00:00Z" }; + const held = { refusal: null, head: HEAD, ineligible: null, reviews: "security-finding: 1 open" }; + const { writes, lines } = await run([ + pull(1, { verdict: held }), + pull(2, { autoMergeRequest: armed, verdict: { ...held, reviews: "evidence-unbound:claude-security-review: marker-missing" } }), + pull(3, { verdict: { refusal: null, head: HEAD, ineligible: null } }), + pull(4), + ]); + assert.deepEqual(writes, [ + { mutation: "disarm", id: "PR_2" }, + { mutation: "arm", id: "PR_4", head: HEAD }, + ]); + assert.ok(lines.some((line) => line.includes("#1 left: pr-merge's gate holds it on its risk score (security-finding: 1 open)"))); + assert.ok(lines.some((line) => line.includes("#3 left: its risk score could not be read"))); +}); + +// The gate's whole risk score through its own CLI and scorer +// (babysit_risk.score, holding), the reads stubbed: only a regular file that +// stays regular may rate below HIGH, so a symlink or a submodule pointer a +// lane pull request adds holds, and a plain documentation change arms. +test("a lane pull request adding a symlink or a submodule pointer is not armed, or is disarmed; a doc change arms", async () => { + const dir = mkdtempSync(join(tmpdir(), "enable-auto-merge-risk-")); + const wrapper = join(dir, "risk.py"); + writeFileSync( + wrapper, + `import json, os, sys +sys.path.insert(0, os.environ["SCRIPTS"]) +from unittest import mock +import babysit_merge +import babysit_merge_policy as policy +import babysit_verify_evidence as ve +from babysit_risk import ChangedFile, LaneEvidence + +KINDS = json.loads(os.environ["KINDS"]) +DOC = ChangedFile("docs/guide.md", None, "modified", 1, 1, True, "100644", "100644", patch="@@ -1 +1 @@\\n-old\\n+new") +EXTRA = { + "doc": (), + "symlink": (ChangedFile("docs/link", None, "added", 1, 0, True, "120000", "000000"),), + "submodule": (ChangedFile("vendor/lib", None, "modified", 1, 1, False, "160000", "160000"),), +} +number = int(sys.argv[-2]) +pr = {"number": number, "head": {"sha": sys.argv[-1]}, "base": {"sha": "d" * 40, "repo": {"private": False}}} +lanes = (LaneEvidence("claude-review", True, None, 0), LaneEvidence("claude-security-review", True, None, 0, False, 0)) +config = policy.PipelineConfig("c" * 40, "C2", "pr-merge", "agent[bot]", None, (), "c2-mechanical") +with mock.patch.object(ve.Reader, "link_refusal", lambda self, n: None), \\ + mock.patch.object(ve.Reader, "evidence", lambda self, n, h, pr=None: ve.Evidence(None, h, (), 0)), \\ + mock.patch.object(policy, "read_config_at", lambda repo, sha: config), \\ + mock.patch.object(policy, "with_default_branch", lambda repo, base, *rest: base), \\ + mock.patch.object(policy, "pull_request", lambda repo, n: pr), \\ + mock.patch.object(policy, "changed_files", lambda repo, p: ((DOC, *EXTRA[KINDS[str(number)]]), True)), \\ + mock.patch.object(policy, "with_contents", lambda repo, p, f: f), \\ + mock.patch.object(policy, "pulumi_dirs_at", lambda repo, p: ()), \\ + mock.patch.object(policy, "permission_reader", lambda repo: (lambda login: None)), \\ + mock.patch.object(babysit_merge, "_lane_evidence", lambda *args: (lanes, [])): + sys.exit(ve.main(sys.argv[1:])) +`, + ); + const armed = { enabledAt: "2026-10-23T10:00:00Z" }; + const kinds = { 1: "doc", 2: "symlink", 3: "submodule" }; + const { github, writes } = world([pull(1), pull(2), pull(3, { autoMergeRequest: armed })]); + const scripts = fileURLToPath(new URL("../../../plugins/source-control/skills/babysit-prs/scripts", import.meta.url)); + const env = { ...ENV, PATH: process.env.PATH, SCRIPTS: scripts, KINDS: JSON.stringify(kinds), VERIFY_EVIDENCE_SCRIPT: wrapper }; + const lines = []; + const log = console.log; + console.log = (line) => lines.push(line); + try { + assert.equal(await main({ env, github, room: room() }), 0); + } finally { + console.log = log; + } + assert.deepEqual(writes(), [ + { mutation: "arm", id: "PR_1", head: HEAD }, + { mutation: "disarm", id: "PR_3" }, + ]); + assert.ok(lines.some((line) => /#2 left: pr-merge's gate holds it on its risk score \(.*docs\/link/.test(line)), lines.join("\n")); + assert.ok(lines.some((line) => /#3 disarmed: pr-merge's gate holds it on its risk score \(.*vendor\/lib/.test(line)), lines.join("\n")); +}); diff --git a/.github/scripts/pr-merge/merge-holds.mjs b/.github/scripts/pr-merge/merge-holds.mjs index a41157fdb3..cdd147e8d0 100755 --- a/.github/scripts/pr-merge/merge-holds.mjs +++ b/.github/scripts/pr-merge/merge-holds.mjs @@ -26,10 +26,18 @@ // path for a pull request pr-merge's gate sends to a person. FAILs and the // repeated-FAIL cap are enable-auto-merge.mjs's, through pr-merge's own // reader, which disarms such a pull request. +// - A changed issue link, when a link file is given with the verdicts: the +// pass binds only while the live link is the one its verdict judged +// (linkHold), as pr-merge's reader binds it (babysit_verify_evidence.py +// `_pass_refusal`), so a body edited after arming to close or name another +// issue does not merge on a pass for the old one. The same approval lifts it. // // Command line: merge-holds.mjs // -// []. The verdicts file holds one {name, verdict, runId, run} +// [ ]. The link file holds {record, closing}: the +// verdict.json of the pass verdictHold counts (null when none was read) and +// the pull request's closingIssuesReferences (totalCount, nodes with number +// and repository.nameWithOwner). The verdicts file holds one {name, verdict, runId, run} // per line, run as GET actions/runs/{runId} gives it, for each unexpired artifact // named a pass at the head; GITHUB_REPOSITORY names the repository. The // discussion file holds one REST issue comment, review or review comment per @@ -48,6 +56,82 @@ import { pathToFileURL } from "node:url"; // text it tries. export const BODY_HOLD = /\bdo(?:n['’]?t| not)\s+merge\b|(?:^|\r)[^\S\r\n]*hold:/im; export const HUMAN_VETO = /\bdo(?:n['’]?t| not|-not)[\s-]*merge\b/i; + +// babysit_merge_policy.refs_issue, which test_verify_rules_sync.py keeps equal: +// the one issue the visible `Refs: #` lines of a pull request body name, +// else null. A line counts only from its first column, outside an HTML comment +// and a fenced code block; a `Refs:` line in any other form, or two numbers, +// name none. +const HTML_COMMENT = /|$)/g; +const FENCE = /^ {0,3}(`{3,}|~{3,})/; +const REFS_LINE = /^[Rr][Ee][Ff][Ss]:/; +const REFS_PLAIN = /^Refs:[ \t]*#([1-9][0-9]{0,8})[ \t]*$/; +export function refsIssue(body) { + const numbers = new Set(); + let fence = ""; + for (const line of String(body ?? "").replace(HTML_COMMENT, "").split(/\r\n|\r|\n/)) { + const marker = FENCE.exec(line); + if (fence) { + if (marker && marker[1][0] === fence[0] && marker[1].length >= fence.length && line.slice(marker[0].length).replace(/[ \t]/g, "") === "") { + fence = ""; + } + continue; + } + if (marker) { + fence = marker[1]; + continue; + } + if (!REFS_LINE.test(line)) { + continue; + } + const plain = REFS_PLAIN.exec(line); + if (!plain) { + return null; + } + numbers.add(Number(plain[1])); + } + return numbers.size === 1 ? [...numbers][0] : null; +} + +// Why the pass `record` (its verdict.json) does not bind the pull request's +// live link, or null when it does. The live link is the gate's +// (babysit_merge_policy._linked_issue): its one closing issue in `repo`, or, +// with no closing issue at all, the one issue its plain `Refs:` lines name +// (refsIssue). The pass binds when it judged that one issue, linked the same +// way (its `spec_kind`, `closes` when absent, as the reader reads an older +// record). +export function linkHold(record, body, closing, repo) { + if (record === null || typeof record !== "object" || !Array.isArray(record.linked_issues)) { + return "its verifier pass's record could not be read"; + } + const kind = record.spec_kind ?? "closes"; + if (!["closes", "refs"].includes(kind)) { + return `its verifier pass judged a spec of kind '${String(kind)}', not a linked issue`; + } + if (!Array.isArray(closing?.nodes) || !Number.isSafeInteger(closing.totalCount) || closing.totalCount !== closing.nodes.length) { + return "its closing issues could not be read"; + } + let live = null; + let linked = "refs"; + if (closing.totalCount > 0) { + const [issue] = closing.nodes; + if (closing.totalCount !== 1 || String(issue?.repository?.nameWithOwner).toLowerCase() !== String(repo).toLowerCase()) { + return "it does not close exactly one issue in this repository, which pr-merge's lane takes"; + } + live = issue.number; + linked = "closes"; + } else { + live = refsIssue(body); + } + if (!Number.isSafeInteger(live)) { + return "it names no issue pr-merge's lane takes"; + } + const judged = record.linked_issues.map((issue) => issue?.number); + if (judged.length !== 1 || judged[0] !== live || kind !== linked) { + return `its link changed since pr-verify passed it: it ${linked === "closes" ? "closes" : "refs"} #${live}, and the pass judged ${judged.map((n) => `#${n}`).join(", ") || "no issue"} (${kind})`; + } + return null; +} // babysit_merge_policy.BLOCKER_ASSOCIATIONS. const MEMBERS = new Set(["OWNER", "MEMBER", "COLLABORATOR"]); // The denied paths resolve-config adds to every config's (resolveMerge); @@ -148,7 +232,6 @@ export function costHold(files, decision) { // babysit_merge_policy COST_DECISION_LINK, DECISION_LINE and HTML_COMMENT. const DECISION_LINK = /https:\/\/github\.com\/([A-Za-z0-9-]+)\/([A-Za-z0-9._-]+)\/(?:issues|pull)\/([0-9]+)#issuecomment-([0-9]+)/g; const DECISION_LINE = /^\*\*Decision recorded\.\*\*[ \t]+Cost:/; -const HTML_COMMENT = /|$)/g; export const PULL_OPENED_QUERY = `query($owner: String!, $name: String!, $number: Int!) { repository(owner: $owner, name: $name) { pullRequest(number: $number) { createdAt closingIssuesReferences(first: 2) { totalCount nodes { number repository { nameWithOwner } } } } @@ -324,8 +407,18 @@ const escape = (text) => String(text).replaceAll("%", "%25").replaceAll("\r", "% export const humansOf = (actors) => new Set(actors.filter((actor) => actor?.kind === "human" && Number.isInteger(actor?.id)).map(({ id }) => id)); +// The link file's verdict, a file that does not read included. +function linkOf(file, body, repo) { + try { + const { record, closing } = JSON.parse(readFileSync(file, "utf8")); + return linkHold(record, body, closing, repo); + } catch { + return "its issue link could not be checked against its verifier pass"; + } +} + export async function main(argv = process.argv.slice(2), { env = process.env, github = fetchGitHub(env) } = {}) { - const [number, bodyFile, discussionFile, filesFile, deniedFile, actorsFile, head, verdictsFile] = argv; + const [number, bodyFile, discussionFile, filesFile, deniedFile, actorsFile, head, verdictsFile, linkFile] = argv; try { if ( !/^[1-9][0-9]*$/.test(String(number)) || @@ -337,7 +430,7 @@ export async function main(argv = process.argv.slice(2), { env = process.env, gi !/^[0-9a-f]{40}$/.test(String(head)) ) { throw new Error( - "usage: merge-holds.mjs []", + "usage: merge-holds.mjs [ ]", ); } const lines = (file) => readFileSync(file, "utf8").split("\n").filter((line) => line.trim()); @@ -364,8 +457,14 @@ export async function main(argv = process.argv.slice(2), { env = process.env, gi } if (verdictsFile) { const unverified = verdictHold(lines(verdictsFile).map((line) => JSON.parse(line)), env.GITHUB_REPOSITORY); - if (!unverified) { + // An approval lifts the verifier hold whole, the link with it. + const relinked = unverified || lifters.length ? null : linkFile ? linkOf(linkFile, body, repo) : "its issue link could not be checked against its verifier pass"; + if (!unverified && !relinked) { console.log(`#${number}: no verifier hold: pr-verify passed this head.`); + } else if (!unverified && lifters.length === 0) { + // pr-verify skips a head it already passed (resolve-pr.mjs `judged`), + // so only a new head or an approval recovers it. + reasons.push(`${relinked}; pr-verify does not judge this head again, so push a new head for it to judge, after which enable-auto-merge arms it again, or an approving review of this head by a person on the trusted-actor list lifts it, then re-queue`); } else if (lifters.length) { console.log(`#${number}: no verifier hold: ${lifters.join(", ")} approved this head, which lifts '${unverified}'.`); } else { diff --git a/.github/scripts/pr-merge/merge-holds.test.mjs b/.github/scripts/pr-merge/merge-holds.test.mjs index 05916fa81a..336e1d47e3 100644 --- a/.github/scripts/pr-merge/merge-holds.test.mjs +++ b/.github/scripts/pr-merge/merge-holds.test.mjs @@ -12,7 +12,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { test } from "node:test"; -import { COMMENT_EDITED_QUERY, PULL_OPENED_QUERY, costDecision, costHold, main } from "./merge-holds.mjs"; +import { COMMENT_EDITED_QUERY, PULL_OPENED_QUERY, costDecision, costHold, linkHold, main } from "./merge-holds.mjs"; const REPO = "owner/repo"; const LINK = "https://github.com/owner/repo/issues/40#issuecomment-7"; @@ -173,3 +173,31 @@ test("ci-status holds a cost path that an approval does not lift, and the owner' assert.match(decided.out, /no body hold, no member veto and no hold path\./); assert.equal((await ci(["infra/main.tf"], { body: `Closes #40\n\n${LINK}`, options: { node: { lastEditedAt: "2026-10-22T13:00:00Z" } } })).code, 1); }); + +// The live link a pass binds, by the gate's rule (README "Eligibility"): one +// closing issue in this repository, or, with none, one plain `Refs: #` +// issue; a pass binds only the issue it judged, linked the same way +// (babysit_verify_evidence.py _pass_refusal: verify:spec-changed otherwise). +test("linkHold binds a pass to the issue and link kind it judged", () => { + const closes = (...numbers) => ({ totalCount: numbers.length, nodes: numbers.map((number) => ({ number, repository: { nameWithOwner: "o/r" } })) }); + const record = (number, spec_kind) => ({ linked_issues: [{ number }], ...(spec_kind ? { spec_kind } : {}) }); + assert.equal(linkHold(record(5, "closes"), "Closes #5", closes(5), "o/r"), null); + // A record from before the kind was written judged a closing issue. + assert.equal(linkHold(record(5), "Closes #5", closes(5), "O/R"), null); + assert.equal(linkHold(record(5, "refs"), "Refs: #5\n\n## Summary", closes(), "o/r"), null); + assert.match(linkHold(record(5, "refs"), "Closes #6", closes(6), "o/r"), /link changed since pr-verify passed it: it closes #6, and the pass judged #5 \(refs\)/); + assert.match(linkHold(record(5, "refs"), "Closes #5", closes(5), "o/r"), /link changed/); + assert.match(linkHold(record(5, "closes"), "Refs: #5", closes(), "o/r"), /link changed/); + assert.match(linkHold(record(5, "refs"), "Refs: #6", closes(), "o/r"), /refs #6, and the pass judged #5/); + assert.match(linkHold(record(5, "refs"), "Refs: #5\nRefs: #6", closes(), "o/r"), /names no issue/); + assert.match(linkHold(record(5, "refs"), "", closes(), "o/r"), /names no issue/); + assert.match(linkHold(record(5, "closes"), "Closes #5", closes(5, 6), "o/r"), /exactly one issue/); + assert.match( + linkHold(record(5, "closes"), "Closes x/y#5", { totalCount: 1, nodes: [{ number: 5, repository: { nameWithOwner: "x/y" } }] }, "o/r"), + /exactly one issue/, + ); + assert.match(linkHold(record(5, "pull-request"), "", closes(), "o/r"), /kind 'pull-request'/); + assert.match(linkHold(null, "Closes #5", closes(5), "o/r"), /record could not be read/); + assert.match(linkHold({ linked_issues: [{ number: 5 }, { number: 6 }] }, "Closes #5", closes(5), "o/r"), /link changed/); + assert.match(linkHold(record(5), "Closes #5", { totalCount: 11, nodes: [] }, "o/r"), /closing issues could not be read/); +}); diff --git a/.github/scripts/pr-merge/merge-job.test.mjs b/.github/scripts/pr-merge/merge-job.test.mjs index 31647eed77..26b7f8d4e1 100644 --- a/.github/scripts/pr-merge/merge-job.test.mjs +++ b/.github/scripts/pr-merge/merge-job.test.mjs @@ -52,6 +52,7 @@ test("the re-dispatch for earlier lane merges waits on neither the listing nor t "Read report-only", "Read the lane records the last run kept", "Check earlier lane merges", + "Keep the post-merge report for enable-auto-merge", "Start the full CI run on main", "Record the dispatched run", "Re-run a red lane-dispatched full run", diff --git a/.github/scripts/pr-merge/workflow.test.mjs b/.github/scripts/pr-merge/workflow.test.mjs index 713a75f129..fce9696f03 100644 --- a/.github/scripts/pr-merge/workflow.test.mjs +++ b/.github/scripts/pr-merge/workflow.test.mjs @@ -168,7 +168,62 @@ test("the last verifier read enable-auto-merge starts ends inside its step's tim const arm = job("enable-auto-merge"); const step = /- name: Arm and disarm auto-merge\n {8}if: .*\n {8}timeout-minutes: ([0-9]+)\n/.exec(arm); assert.ok(step, "the arm step names its timeout"); - assert.ok(VERIFY_BUDGET_MS + VERIFY_TIMEOUT_MS < Number(step[1]) * 60_000); + // The room read, then the verdict budget and the last read it starts. + assert.ok(VERIFY_TIMEOUT_MS + VERIFY_BUDGET_MS + VERIFY_TIMEOUT_MS < Number(step[1]) * 60_000); +}); + +test("enable-auto-merge logs both tokens' readings at job end, before the token is revoked", () => { + const arm = job("enable-auto-merge"); + const block = /- name: Log the API budget at job end\n([\s\S]*?)(?=\n {6}- name: |$)/.exec(arm)?.[1]; + assert.ok(block, "a job-end budget step"); + assert.match(block, /\n {8}uses: \.\/\.base\/\.github\/actions\/log-api-budget /); + assert.match(block, /^ {8}id: budget-end\n {8}if: always\(\) && steps\.lane-token\.outputs\.token != ''\n/); + assert.match(block, /\n {8}continue-on-error: true\n {8}timeout-minutes: 1\n/); + assert.match(block, /\n {10}tokens: lanes-app github-token\n/); + // The reader's gh calls read into both names. + const step = /- name: Arm and disarm auto-merge\n([\s\S]*?)(?=\n {6}- name: )/.exec(arm)[1]; + assert.match(step, /\n {10}API_BUDGET_TOKEN: lanes-app\n {10}API_BUDGET_OVERRIDE_TOKEN: github-token\n/); + const names = [...arm.matchAll(/\n {6}- name: (.*)/g)].map((m) => m[1]); + assert.ok(names.indexOf("Arm and disarm auto-merge") < names.indexOf("Log the API budget at job end")); + assert.ok(names.indexOf("Log the API budget at job end") < names.indexOf("Revoke the lane token")); +}); + +// The lane's room (lane-room) reads this run's post-merge report, which +// post-merge-dispatch keeps, rather than running the check a third time per +// scheduled run, and the rung cap as the merge job passes it: an unset cap +// holds the lane off, so a step that drops it would disarm every lane pull +// request. +test("enable-auto-merge reads the lane's room from this run's post-merge-dispatch report and the rung cap", () => { + const dispatch = job("post-merge-dispatch"); + const keep = /- name: Keep the post-merge report for enable-auto-merge\n([\s\S]*?)(?=\n {6}- name: |\n*$)/.exec(dispatch)?.[1]; + assert.ok(keep, "post-merge-dispatch keeps its report"); + assert.match(keep, /uses: actions\/upload-artifact@[0-9a-f]{40} /); + assert.match(keep, /\n {10}name: pr-merge-post-merge-report\n {10}path: \$\{\{ runner\.temp \}\}\/post-merge\.json\n {10}retention-days: 1\n/); + const arm = job("enable-auto-merge"); + assert.match(arm, /^ {4}# [\s\S]*?\n {4}needs: \[post-merge-dispatch\]\n/); + assert.match(/\n {4}if: >-\n([\s\S]*?)\n {4}[a-z-]+:/.exec(arm)[1], /^ {6}!cancelled\(\)\n/); + // No check of its own: the report comes from this run's job alone. + assert.doesNotMatch(arm, /python3 [^\n]*babysit_postmerge\.py/); + const read = /- name: Read this run's post-merge report\n([\s\S]*?)(?=\n {6}- name: )/.exec(arm)?.[1]; + assert.ok(read, "a download of the report"); + assert.match(read, /uses: actions\/download-artifact@[0-9a-f]{40} /); + assert.match(read, /\n {10}name: pr-merge-post-merge-report\n/); + // Not 'success': a step after the kept report can fail the job. + assert.match(read, /needs\.post-merge-dispatch\.result != 'skipped'/); + assert.doesNotMatch(read, /run-id|github-token|repository:/); + const step = /- name: Arm and disarm auto-merge\n([\s\S]*?)(?=\n {6}- name: )/.exec(arm)[1]; + assert.match(step, /\n {10}POSTMERGE_RESULT: \$\{\{ runner\.temp \}\}\/post-merge\/post-merge\.json\n/); + assert.match(step, /\n {10}RUNG_CAP: \$\{\{ vars\.CLAUDE_LANES_MERGE_RUNG_CAP \}\}\n/); + const names = [...arm.matchAll(/\n {6}- name: (.*)/g)].map((m) => m[1]); + assert.ok(names.indexOf("Read this run's post-merge report") < names.indexOf("Arm and disarm auto-merge")); +}); + +test("enable-auto-merge's step timeouts fit inside its job's", () => { + const arm = job("enable-auto-merge"); + const total = Number(/\n {4}timeout-minutes: ([0-9]+)\n/.exec(arm)[1]); + const steps = [...arm.matchAll(/\n {8}timeout-minutes: ([0-9]+)\n/g)].map((m) => Number(m[1])); + assert.ok(steps.length >= 4, "the post-merge check, the cache restore, the arm step and the budget step name timeouts"); + assert.ok(steps.reduce((sum, minutes) => sum + minutes, 0) < total, `${steps.join(" + ")} inside ${total}`); }); test("enable-auto-merge asks the broker for its own activity's merge token", () => { @@ -186,7 +241,7 @@ test("enable-auto-merge asks the broker for its own activity's merge token", () const block = /\n {4}permissions:\n((?: {6}[a-z-]+: [a-z]+.*\n)+)/.exec(arm)[1]; assert.deepEqual( [...block.matchAll(/ {6}([a-z-]+): ([a-z]+)/g)].map((m) => `${m[1]}: ${m[2]}`), - ["contents: read", "actions: read", "id-token: write"], + ["contents: read", "actions: read", "pull-requests: read", "checks: read", "issues: read", "id-token: write"], ); // The verifier evidence is read as the gate reads it: the job's // GITHUB_TOKEN for the Actions reads, with the run's head-ref map and diff --git a/.github/scripts/pr-verify/resolve-pr.mjs b/.github/scripts/pr-verify/resolve-pr.mjs index e867b673b7..64d2240f81 100644 --- a/.github/scripts/pr-verify/resolve-pr.mjs +++ b/.github/scripts/pr-verify/resolve-pr.mjs @@ -41,6 +41,9 @@ import { loadTrustedList, } from "../../actions/check-trusted-trigger/check-trusted-trigger.mjs"; import { headRefDigest, issuesSuffix, isVerdictName, NAME } from "./write-verdict.mjs"; +import { refsIssue } from "../pr-merge/merge-holds.mjs"; + +export { refsIssue }; // The name grammar lives with its writer; the resolver checks names against it. export { isVerdictName, NAME, NAME_MAX } from "./write-verdict.mjs"; @@ -81,42 +84,6 @@ const REFS_QUERY = `query($owner: String!, $name: String!, $number: Int!) { } ${ACTOR}`; -// babysit_merge_policy.refs_issue, which test_verify_rules_sync.py keeps equal: -// the one issue the visible `Refs: #` lines of a pull request body name, -// else null. A line counts only from its first column, outside an HTML comment -// and a fenced code block; a `Refs:` line in any other form, or two numbers, -// name none. -const HTML_COMMENT = /|$)/g; -const FENCE = /^ {0,3}(`{3,}|~{3,})/; -const REFS_LINE = /^[Rr][Ee][Ff][Ss]:/; -const REFS_PLAIN = /^Refs:[ \t]*#([1-9][0-9]{0,8})[ \t]*$/; -export function refsIssue(body) { - const numbers = new Set(); - let fence = ""; - for (const line of String(body ?? "").replace(HTML_COMMENT, "").split(/\r\n|\r|\n/)) { - const marker = FENCE.exec(line); - if (fence) { - if (marker && marker[1][0] === fence[0] && marker[1].length >= fence.length && line.slice(marker[0].length).replace(/[ \t]/g, "") === "") { - fence = ""; - } - continue; - } - if (marker) { - fence = marker[1]; - continue; - } - if (!REFS_LINE.test(line)) { - continue; - } - const plain = REFS_PLAIN.exec(line); - if (!plain) { - return null; - } - numbers.add(Number(plain[1])); - } - return numbers.size === 1 ? [...numbers][0] : null; -} - // babysit_merge_policy.names_nothing: whether a GraphQL answer says only that // `repository.` names nothing, null there with every error a NOT_FOUND // at that path, as GitHub answers a `Refs: #` whose number is no issue or diff --git a/.github/workflows/pr-merge.yml b/.github/workflows/pr-merge.yml index 6bc5e4936a..9efdc2aae5 100644 --- a/.github/workflows/pr-merge.yml +++ b/.github/workflows/pr-merge.yml @@ -213,6 +213,20 @@ jobs: rerun=$(jq -r '[.reruns[]? | select(.laneDispatched == true) | .runId][0] // ""' "$RUNNER_TEMP/post-merge.json" 2>/dev/null || true) echo "lane-rerun=$rerun" >>"$GITHUB_OUTPUT" + # The check's report for this run's enable-auto-merge job, which reads + # the lane's room from it instead of running the check again. That job + # downloads it from this run only. + - name: Keep the post-merge report for enable-auto-merge + if: steps.post-merge.conclusion == 'success' + continue-on-error: true + timeout-minutes: 2 + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: + name: pr-merge-post-merge-report + path: ${{ runner.temp }}/post-merge.json + retention-days: 1 + if-no-files-found: error + # The same body as dispatch-full-run's step of this name; see there. # WAIT_CAP: of the 15 minutes, the whole job took 97-127 s on the last # ten scheduled runs read on 2026-10-23 (the post-merge check 81-111 s @@ -1312,23 +1326,33 @@ jobs: fi # Turns on GitHub's native auto-merge for each open lanes App pull request - # into main that closes an issue and carries no hold, and turns it off on + # into main that carries no hold, and turns it off on # one that now carries a hold (a `Hold:` line, do-not-merge, the human-gated # label). Every event is a wake-up: the script lists the pull requests # itself. The issue lane finishing arms the pull request it opened; a listed # human's edit, unlabel or ready disarms or arms at once; the schedule # catches what the events missed, such as a run the lanes App started, which # the broker refuses. Only the scheduled run and a hand-started one arm, on - # the verdict pr-merge's gate would take at the head (babysit_verify_evidence.py, - # read with the job's GITHUB_TOKEN from the head-ref map and run cache the - # last gate run kept); event runs read no verdict and only disarm on a hold. + # the issue link (one closing issue, or one plain `Refs:` issue) and verdict + # pr-merge's gate would take at the head, with no hold on the gate's risk + # score (babysit_verify_evidence.py `evidence --lane-link --lane-reviews`, + # its Actions reads with the job's GITHUB_TOKEN from the head-ref map and + # run cache the last gate run kept), and only while the gate's lane has + # room (`lane-room`, from this run's post-merge-dispatch report): while the + # gate holds the lane off they also disarm every armed lane pull request. + # Event runs read none of these and only disarm on a hold. # Its own job and token: the broker mints one token per # job. Rules: .github/scripts/pr-merge/enable-auto-merge.mjs. enable-auto-merge: + # On the schedule and a hand-started run it waits for post-merge-dispatch + # and reads its report; on every other event that job is skipped, and + # !cancelled() runs this one anyway. + needs: [post-merge-dispatch] # An issue run that skipped every job (a label other than agent-ready) # opened nothing. if: >- - github.ref == 'refs/heads/main' && github.event_name != 'push' + !cancelled() + && github.ref == 'refs/heads/main' && github.event_name != 'push' && (github.event_name != 'workflow_run' || github.event.workflow_run.conclusion != 'skipped') && (github.event_name != 'pull_request_target' || github.event.pull_request.head.repo.full_name == github.repository) @@ -1338,10 +1362,14 @@ jobs: group: pr-merge-enable-auto-merge queue: max runs-on: ubuntu-24.04 - timeout-minutes: 15 + # Above the sum of its steps' timeouts (workflow.test.mjs checks it). + timeout-minutes: 25 permissions: contents: read # check out main actions: read # the verifier verdicts, their runs and the head-ref map + pull-requests: read # the lane's merges and the merge queue, for the lane's room + checks: read # the review lanes' annotations, as the gate binds their evidence + issues: read # the demotion records the lane's room re-checks id-token: write # prove to the token broker which workflow this is steps: - name: Check out the default branch tip @@ -1445,11 +1473,33 @@ jobs: CARRY_TO: ${{ runner.temp }}/babysit-run-cache.json run: node .base/.github/scripts/pr-merge/carry-run-cache.mjs + # This run's post-merge report, which post-merge-dispatch kept: the + # gate's run-level answer that enable-auto-merge.mjs reads the lane's + # room from (lane-room), so a demotion or a failed lane merge stops + # native arming as it stops the gate. Downloaded from this run only (no + # run-id is given, so no other run's artifact is read). With no report + # the room is unreadable, which holds as the gate's refusal does: + # nothing is armed and every armed lane pull request is disarmed. + # Read whenever post-merge-dispatch ran: its later steps (a full run + # started or re-run) can fail after the report was kept, and the gate, + # which runs its own check, does not refuse on them. + - name: Read this run's post-merge report + if: >- + steps.lane-token.outputs.token != '' + && (github.event_name == 'schedule' || github.event_name == 'workflow_dispatch') + && needs.post-merge-dispatch.result != 'skipped' + continue-on-error: true + timeout-minutes: 2 + uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2 + with: + name: pr-merge-post-merge-report + path: ${{ runner.temp }}/post-merge + # While merge.report-only is anything but false it reports each # decision and writes nothing. - name: Arm and disarm auto-merge if: steps.lane-token.outputs.token != '' - timeout-minutes: 10 + timeout-minutes: 14 env: GH_TOKEN: ${{ steps.lane-token.outputs.token }} KILL_SWITCH: ${{ steps.kill.outputs.proceed == 'true' && 'proceed' || '' }} @@ -1459,6 +1509,15 @@ jobs: 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 + # The lane's room (lane-room): this run's post-merge report and the + # rung cap as the merge job passes it, empty meaning no cap. + POSTMERGE_RESULT: ${{ runner.temp }}/post-merge/post-merge.json + RUNG_CAP: ${{ vars.CLAUDE_LANES_MERGE_RUNG_CAP }} + # Name the two tokens' rate-limit readings for the reader's gh calls + # (api-budget.bash): the lane token by default, the job token for + # the Actions reads that carry it. + API_BUDGET_TOKEN: lanes-app + API_BUDGET_OVERRIDE_TOKEN: github-token run: | set -euo pipefail config=.base/docs/conventions/pr-pipeline.yaml @@ -1467,6 +1526,15 @@ jobs: export REPORT_ONLY LANES_BOT_LOGIN node .base/.github/scripts/pr-merge/enable-auto-merge.mjs + - name: Log the API budget at job end + id: budget-end + if: always() && steps.lane-token.outputs.token != '' + continue-on-error: true + timeout-minutes: 1 + uses: ./.base/.github/actions/log-api-budget # zizmor: ignore[self-repository] + with: + tokens: lanes-app github-token + - name: Revoke the lane token if: always() && steps.lane-token.outputs.token != '' continue-on-error: true diff --git a/.github/workflows/pr-require-checks.yml b/.github/workflows/pr-require-checks.yml index 8f586df1fc..4cc446bcd6 100644 --- a/.github/workflows/pr-require-checks.yml +++ b/.github/workflows/pr-require-checks.yml @@ -4100,6 +4100,11 @@ jobs: # it); every other pull request merges through pr-merge's gate or a # person. An unreadable pull request, module, config, file list or # discussion fails closed. + # WHILE THE LANES KILL SWITCH IS ON A LANES APP PULL REQUEST FAILS, so + # one armed before it was turned on stops in the merge queue, as the + # gate stops: CLAUDE_LANES_DISABLED and CLAUDE_LANES_REPO_DISABLED from + # the vars context, no API read, and only the exact value `false` on + # both proceeds. Every other author's pull request is not affected. # IN THE MERGE QUEUE A LANES APP PULL REQUEST ALSO NEEDS A VERIFIER PASS # AT ITS HEAD: a PASS or PASS-NOTES artifact of a completed, successful # pr-verify.yml run on main (merge-holds.mjs verdictHold), looked up by @@ -4114,6 +4119,12 @@ jobs: # new head before it is armed again; the queue still checks the head # it merges. # FAILs and the repeated-FAIL cap are enable-auto-merge.mjs's. + # THE PASS MUST STILL BIND THE LIVE LINK: the counted pass's verdict.json + # (one artifact download) and the closing issues (one GraphQL read) go + # to merge-holds.mjs linkHold, which holds when the body now closes or + # names another issue than the one pr-verify judged, as pr-merge's + # reader holds it (verify:spec-changed). A record or link that does not + # read holds. # Pinned by scripts/ci-lanes-pr-hold.test.sh, which runs this body. - name: Fail a lanes App pull request its body or a member holds if: ${{ !cancelled() && (github.event_name == 'pull_request' || github.event_name == 'merge_group') }} @@ -4122,6 +4133,10 @@ jobs: PR_NUMBER: ${{ github.event.pull_request.number }} EVENT_ACTION: ${{ github.event.action }} GROUP_BASE_SHA: ${{ github.event.merge_group.base_sha }} + # The lanes kill switches, as check-kill-switch reads them: only the + # exact value `false` lets a lanes App pull request merge. + LANES_DISABLED: ${{ vars.CLAUDE_LANES_DISABLED }} + LANES_REPO_DISABLED: ${{ vars.CLAUDE_LANES_REPO_DISABLED }} run: | . "$RUNNER_TEMP/gh-read.sh" lanes_app_id=337595936 @@ -4157,6 +4172,10 @@ jobs: echo "#$pr is not the lanes App's; pr-merge's gate or a person reads its holds." exit 0 fi + if [ "${LANES_DISABLED:-}" != false ] || [ "${LANES_REPO_DISABLED:-}" != false ]; then + echo "::error::#$pr: the lanes kill switch is on (CLAUDE_LANES_DISABLED or CLAUDE_LANES_REPO_DISABLED is not 'false'), so no lanes App pull request merges; it merges once both read 'false' and ci-status is re-run." + exit 1 + fi BASE_SHA=$GROUP_BASE_SHA if [ "$GITHUB_EVENT_NAME" != merge_group ]; then if [ "$(jq -r '.base.ref' <<<"$pull")" != main ]; then @@ -4232,18 +4251,41 @@ jobs: verdict=${name#"pr-verify-$pr-$head-"} verdict=${verdict%"-$digest"} runs=$(read_api "the $name artifacts" "repos/$GITHUB_REPOSITORY/actions/artifacts?name=$name&per_page=100" \ - --jq ".artifacts[] | select(.name == \"$name\" and .expired == false) | .workflow_run.id") || exit 1 - for run in $runs; do - [[ $run =~ ^[1-9][0-9]*$ ]] || continue + --jq ".artifacts[] | select(.name == \"$name\" and .expired == false) | \"\(.workflow_run.id) \(.id)\"") || exit 1 + while read -r run artifact; do + [[ $run =~ ^[1-9][0-9]*$ && $artifact =~ ^[1-9][0-9]*$ ]] || continue record=$(read_api "run $run" "repos/$GITHUB_REPOSITORY/actions/runs/$run") || exit 1 - jq -c --arg name "$name" --arg verdict "$verdict" --argjson id "$run" '{name: $name, verdict: $verdict, runId: $id, run: .}' <<<"$record" >>"$RUNNER_TEMP/verdicts.jsonl" || exit 1 - done + jq -c --arg name "$name" --arg verdict "$verdict" --argjson id "$run" --argjson artifact "$artifact" '{name: $name, verdict: $verdict, runId: $id, artifact: $artifact, run: .}' <<<"$record" >>"$RUNNER_TEMP/verdicts.jsonl" || exit 1 + done <<<"$runs" # A pass merge-holds.mjs verdictHold would count ends the lookup. ! jq -se --arg repo "$GITHUB_REPOSITORY" 'any(.runId as $id | .run | .id == $id and .status == "completed" and .conclusion == "success" and .path == ".github/workflows/pr-verify.yml" and .event == "workflow_run" and .head_branch == "main" and .repository.full_name == $repo and .head_repository.full_name == $repo)' "$RUNNER_TEMP/verdicts.jsonl" >/dev/null || break done - verdicts=("$RUNNER_TEMP/verdicts.jsonl") + # The counted pass's record and the closing issues, for linkHold. + artifact=$(jq -sr --arg repo "$GITHUB_REPOSITORY" '[.[] | select(.runId as $id | .run | .id == $id and .status == "completed" and .conclusion == "success" + and .path == ".github/workflows/pr-verify.yml" and .event == "workflow_run" and .head_branch == "main" + and .repository.full_name == $repo and .head_repository.full_name == $repo)] | first | .artifact // empty' "$RUNNER_TEMP/verdicts.jsonl") || exit 1 + record=null + if [[ $artifact =~ ^[1-9][0-9]*$ ]]; then + for attempt in 1 2 3; do + # gh-read-exempt: a binary archive, which gh_read's text capture would corrupt. + if gh api "repos/$GITHUB_REPOSITORY/actions/artifacts/$artifact/zip" >"$RUNNER_TEMP/verdict.zip"; then + record=$(unzip -p "$RUNNER_TEMP/verdict.zip" verdict.json | jq -c .) || record=null + break + fi + [ "$attempt" -lt 3 ] || { echo "::error::could not read the verdict of artifact $artifact; re-run this run."; exit 1; } + sleep $((attempt * 5)) + done + fi + # shellcheck disable=SC2016 # GraphQL variables, not shell + closing=$(read_api "the closing issues of #$pr" graphql -f query='query($owner:String!,$name:String!,$number:Int!){repository(owner:$owner,name:$name){pullRequest(number:$number){closingIssuesReferences(first:10){totalCount nodes{number repository{nameWithOwner}}}}}}' \ + -F owner="${GITHUB_REPOSITORY%%/*}" -F name="${GITHUB_REPOSITORY#*/}" -F number="$pr" --jq '.data.repository.pullRequest.closingIssuesReferences') || exit 1 + jq -cn --argjson record "${record:-null}" --argjson closing "${closing:-null}" '{record: $record, closing: $closing}' >"$RUNNER_TEMP/link.json" || { + echo "::error::the verdict record or closing issues of #$pr do not parse." + exit 1 + } + verdicts=("$RUNNER_TEMP/verdicts.jsonl" "$RUNNER_TEMP/link.json") fi out=$(node "$RUNNER_TEMP/merge-holds.mjs" "$pr" "$RUNNER_TEMP/pr-body.txt" "$RUNNER_TEMP/discussion.jsonl" \ "$RUNNER_TEMP/files.jsonl" "$RUNNER_TEMP/denied-paths.json" "$RUNNER_TEMP/trusted-actors.json" "$head" "${verdicts[@]}") || { diff --git a/docs/conventions/pr-pipeline.yaml b/docs/conventions/pr-pipeline.yaml index b595e9423c..df6a67ec06 100644 --- a/docs/conventions/pr-pipeline.yaml +++ b/docs/conventions/pr-pipeline.yaml @@ -390,11 +390,14 @@ activities: inputs: [pr, head-sha] enable-auto-merge: # Turns on GitHub's native auto-merge (squash, at the head it read) for - # each open lanes App pull request into main that opens with `Closes #` - # and carries no hold (a `Hold:` line, do-not-merge, the human-gated + # each open lanes App pull request into main whose issue link the merge + # gate takes (one closing issue, or one plain `Refs: #` issue) and + # that carries no hold (a `Hold:` line, do-not-merge, the human-gated # label, a member's "do not merge" comment, a head that changes a denied - # path), and turns it off on an armed one that now carries a hold; - # auto-merge a person turned off stays off. No + # path, a hold on the gate's risk score), while the gate's lane has + # room (no demotion, rung on, below merge.max-in-flight), and turns it + # off on an armed one that now carries a hold or while the gate holds the + # lane off; auto-merge a person turned off stays off. No # model. The enable-auto-merge job of .github/workflows/pr-merge.yml runs # it when the issue lane finishes, on a listed human's PR edit, and on the # schedule. @@ -787,11 +790,8 @@ merge: # (the enable-auto-merge activity arms them, ci-status gates them), and the # gate below neither lists, scores nor enqueues them. gate: the gate also # lists, scores and enqueues them. Owner PRs and Dependabot's are gated in - # both. Absent is native. This repository stays on gate until ci-status or - # enable-auto-merge requires the pr-verify verdict and the post-merge check - # judges a native merge with no pin comment: until then the gate is the only - # place either holds for a lanes App PR. - lane-prs: gate + # both. Absent is native. + lane-prs: native # A PR the gate lists merges only with a passing pr-verify.yml verdict at its # head (lane PRs under lane-prs native are not listed). Must be true at a # rung above off: the gate refuses otherwise. diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index 1fb25aaec7..9ecc12b085 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -1066,23 +1066,42 @@ drops an escalation. The lane's `enable-auto-merge` activity, the `enable-auto-merge` job of `pr-merge.yml` with its own lanes App token (`.github/scripts/pr-merge/enable-auto-merge.mjs`, no model), turns on GitHub's native auto-merge (`enablePullRequestAutoMerge`, squash, `expectedHeadOid` the head it read) for -each open lanes App pull request into main from a branch of this repository that is not a draft, -opens with `Closes #`, and carries no hold: no `Hold:` line or "do not merge" in its body +each open lanes App pull request into main from a branch of this repository that is not a draft +and carries no hold: no `Hold:` line or "do not merge" in its body (the gate's own rule), no `do-not-merge` or human-gated label, and no member (owner, member or collaborator) whose newest comment, review or inline comment with text says "do not merge" (the gate's veto rule; a later comment lifts it), and whose head changes no path in the lane's `merge.diff-check.denied-paths`, read on the files the head changes at each run, and whose -verifier verdict at that head pr-merge's gate would take: `babysit_verify_evidence.py evidence` -refuses nothing, so a PASS or PASS-NOTES of [pr-verify](#verifier-verdict) stands at the head, +issue link and verifier verdict at that head pr-merge's gate would take: +`babysit_verify_evidence.py evidence --lane-link` refuses nothing, so the link is one the gate's +[eligibility](#the-pr-merge-lane) rule takes (one closing issue in this repository, or, with none, +the one issue its plain `Refs: #` lines name; a failed read refuses), a PASS or PASS-NOTES of [pr-verify](#verifier-verdict) stands at the head, no FAIL is at it, no earlier FAIL is uncleared, FAILs sit at fewer than `merge.max-verify-fails` -heads, and the spec is unchanged and trusted. Only the scheduled run and a hand-started one read -it and arm, last, for a pull request every other rule would arm or keep armed, with the gate's -saved listing, head-ref map and run cache; an event run reads no verdict, never arms, and only -disarms on a hold, so a pull request waits up to one scheduled interval to be armed. `ci-status` +heads, and the spec is unchanged and trusted; and `evidence --lane-reviews` finds no hold on the +gate's risk score (`babysit_risk.score` over the inputs the gate builds, at the lane room's rung): +an open CRITICAL security finding or a review not bound to the head, whether or not the lane +applied its hold label, a symlink, submodule or other non-regular mode, a high path, the manifest +review, the trust gate and the cost review all hold. Each such run also +waits for the same run's `post-merge-dispatch` job and reads the lane's room from the post-merge +report that job keeps as an artifact (`babysit_verify_evidence.py lane-room`; with no report, or +one the gate would not reuse (`shared_refusal`: too old, a demotion record changed since, or a lane +merge landed since that it does not count), it holds as a refusal does): while the gate holds its lane off (a demotion record, +`merge.rung`, the loop setting or the rung cap off, a lane merge that failed its post-merge check) +nothing is armed and every armed lanes App pull request is disarmed; at `merge.max-in-flight`, +counting the queue's lane entries, the lane's unverified merges and armed pull requests the queue +has not taken, nothing more is armed; a room that cannot be read (no report, a failed read) holds +as a refusal does. +Only the scheduled run and a hand-started one read +them and arm, last, for a pull request every other rule would arm or keep armed, with the gate's +saved listing, head-ref map and run cache; an event run reads no link or verdict, never arms, and +only disarms on a hold, so a pull request waits up to one scheduled interval to be armed. `ci-status` turns an armed lanes App pull request's auto-merge off on each push to its head, so a new head is armed only after the reader judges it. It turns auto-merge off on an armed one that now fails any of those, and does not turn it back on once a person has turned it off. GitHub then merges the pull request through the merge queue -once `ci-status` passes. The issue lane finishing (`workflow_run`) wakes it, so a pull request is +once `ci-status` passes, which in the queue also holds a pass that no longer binds the live link: +the body must still close, or with no closing issue name by a plain `Refs:` line, the one issue +the pass judged, linked the same way (`merge-holds.mjs` `linkHold`), so a body edited after arming +does not merge on a pass for another issue. The issue lane finishing (`workflow_run`) wakes it, so a pull request is armed right after report-outcome opens it; a listed human's edit, unlabel or ready wakes it too, so adding a `Hold:` line disarms at once; and the 15-minute schedule arms or disarms what the events missed. The token broker mints `merge` only for `pr-merge.yml`, and outside the schedule only for a @@ -1097,6 +1116,9 @@ and the post-merge check, the full-run dispatch, re-firing refused runs and clea run unchanged. `gate` lists the lanes App's pull requests for the gate as well, which is how the lane ran before this setting; the `enable-auto-merge` job arms them in both modes. +While the lanes kill switch is on (`CLAUDE_LANES_DISABLED` or `CLAUDE_LANES_REPO_DISABLED` reads +anything but `false`), `ci-status` fails every lanes App pull request, so one armed before the switch +was turned on stops in the merge queue; owner and Dependabot pull requests are not affected. `ci-status` enforces the same holds on every lanes App pull request, the two text holds and the hold paths, so one added after arming stops the merge without waiting for this job: its step `Fail a lanes App pull request its body or a member holds` runs diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py index d668c4334b..fa0d026353 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py @@ -2870,12 +2870,14 @@ def derive_auto_merge( class InFlight: """`queued` holds every merge-queue entry's pull request number, or None when the queue was not read whole; `unverified` counts the unverified - auto-merges read, all of them when clear.""" + auto-merges read, all of them when clear; `count` the merges in flight + when clear (`max_in_flight` or more when not).""" clear: bool reason: str | None queued: frozenset[int] | None = field(default=None, compare=False) unverified: int = field(default=0, compare=False) + count: int = field(default=0, compare=False) def in_flight( @@ -2952,7 +2954,7 @@ def in_flight( return InFlight(False, reason, queued, unverified) except PolicyReadError as exc: return InFlight(False, f"unreadable: {exc}", queued, unverified) - return InFlight(True, None, queued, unverified) + return InFlight(True, None, queued, unverified, count) # --- Holds on a queued entry ------------------------------------------------------------------- @@ -3930,6 +3932,26 @@ def _demotion_changed( return False +def _shared_age_refusal(shared: SharedPostMerge, age: timedelta) -> str | None: + """`stale` past `SHARED_MAX_AGE` (or from the future), `unverified-stale` + past `SHARED_UNVERIFIED_MAX_AGE` while it passes with a merge unverified, + else None.""" + if not timedelta(0) <= age <= SHARED_MAX_AGE: + return "stale" + # A refusing answer refuses whatever was verified since, so only a + # passing one with a merge still unverified is read again. + if ( + age > SHARED_UNVERIFIED_MAX_AGE + and shared.gate.refusal is None + and any( + json_object(m).get("verified") is not True + for m in json_array(shared.gate.report.get("autoMerges")) + ) + ): + return "unverified-stale" + return None + + def shared_refusal( repo: str, shared: SharedPostMerge, @@ -3986,21 +4008,7 @@ def shared_refusal( return "floor-differs" except PolicyReadError: return "config-unreadable" - checks = ( - (not timedelta(0) <= age <= SHARED_MAX_AGE, "stale"), - # A refusing answer refuses whatever was verified since, so only a - # passing one with a merge still unverified is read again. - ( - age > SHARED_UNVERIFIED_MAX_AGE - and shared.gate.refusal is None - and any( - json_object(m).get("verified") is not True - for m in json_array(shared.gate.report.get("autoMerges")) - ), - "unverified-stale", - ), - ) - refusal = next((text for refused, text in checks if refused), None) + refusal = _shared_age_refusal(shared, age) if refusal: return refusal if not shared.demotion.active and _demotion_changed(owner, name, shared, identity): @@ -4437,3 +4445,127 @@ def assess( return UnattendedPolicy( reason, identity, details, enabled=True, promoted=promoted, inputs=inputs ) + + +# --- Room for a native lane merge ------------------------------------------------------------- + + +@dataclass(frozen=True) +class LaneRoom: + """`lane_room`'s answer. `refusal`: the gate holds the lane off, so + nothing is armed and armed lane pull requests are disarmed. `unreadable`: + a read failed, which the caller holds on as on a refusal. Otherwise + `count` merges are in flight of `cap`, `queued` holds the merge queue's + pull request numbers, and `rung` is the effective rung the gate scores + each candidate at ("off" unless the lane has room).""" + + refusal: str | None = None + unreadable: str | None = None + count: int = 0 + cap: int = 1 + queued: frozenset[int] = frozenset() + rung: str = "off" + + def report(self) -> dict[str, Any]: + return { + "refusal": self.refusal, + "unreadable": self.unreadable, + "count": self.count, + "cap": self.cap, + "queued": sorted(self.queued), + "rung": self.rung, + } + + +def lane_room( + repo: str, + shared: SharedPostMerge | None, + *, + lane_rung_cap: str | None, + now: datetime | None = None, +) -> LaneRoom: + """Whether the pr-merge lane may merge one more of its own pull requests + now, for native auto-merge, which skips `assess`: the run-level refusals + `assess` applies to every lane candidate, read once at the default branch + head, in its order. `shared` is this run's `babysit_postmerge.py` answer + (`babysit_postmerge.read_shared`), as the gate's `--postmerge-result`. + + - A config naming another authority stops (`enabled_authority`). + - A post-merge refusal stops (`post-merge:`). + - The effective rung off stops (`rung-off:`): `merge.rung`, + `babysit_loop_merge`, the rung cap (`lane_rung_cap`, as `rung_cap` + reads it) and an active demotion, an unreadable record included. A lane + pull request's class is `LANE_CLASS`, which no rung above off is below, + so no class-above-rung refusal applies. + - The merges in flight (`in_flight`, `merge.max-in-flight` at the head) + are counted, not refused: the caller arms below the cap. + + Any read failure, a missing `shared` and an in-flight count that could + not be read included, is `unreadable`. So is a `shared` the gate would + not reuse (`shared_refusal`): too old (a re-run job downloads the run's + first report, up to a day old), a demotion record filed or changed + since, or a lane merge landed since that it does not count.""" + if shared is None: + return LaneRoom(unreadable="post-merge answer missing or unreadable") + try: + default = _default_branch(repo, shared=True) + config = _default_branch_config(repo, default) + except PolicyReadError as exc: + return LaneRoom(unreadable=f"config: {exc}") + if enabled_authority(config) != PR_MERGE: + return LaneRoom(refusal=AUTHORITY_UNCONFIGURED) + identity = config.agent_identity + if not identity: + return LaneRoom(unreadable="no agent identity") + # The gate's own reuse rule, as a candidate at the default branch head + # with the gate's self logins (none): the same identity and lane, its + # age, no demotion record filed or changed since, and every lane merge + # the check would judge now counted (`new-merge:#` otherwise). + unusable = shared_refusal( + repo, + shared, + branch=default, + base_sha=config.base_sha, + identity=identity, + self_logins=frozenset(), + lane=True, + authority=PR_MERGE, + now=now, + ) + if unusable: + return LaneRoom(unreadable=f"post-merge answer {unusable}") + if shared.gate.refusal: + return LaneRoom(refusal=f"post-merge:{shared.gate.refusal}") + cap = rung_cap(repo, authority=PR_MERGE, lane_value=lane_rung_cap) + rung = effective_rung( + config_rung=config.rung, + loop_merge=config.loop_merge, + cap=cap.value, + demoted=shared.demotion.active, + ) + if rung.rung == "off": + return LaneRoom(refusal=f"rung-off:{rung.limited_by}") + flight_cap, _ = _run_max_in_flight(repo, default, config.base_sha, config) + flight = in_flight( + repo, + agent_identity=identity, + self_logins=shared.self_logins, + verified=shared.gate.verified, + branch=default, + merged_since=shared.floor, + lane_merges_only=True, + max_in_flight=flight_cap, + ) + counted = str(flight.reason).startswith(FLIGHT_COUNTED) + if flight.queued is None or not (flight.clear or counted): + return LaneRoom(unreadable=f"in-flight:{flight.reason}", cap=flight_cap) + return LaneRoom( + count=flight.count if flight.clear else flight_cap, + cap=flight_cap, + queued=flight.queued, + rung=rung.rung, + ) + + +# The `in_flight` reasons that are a count reaching the cap, not a read failure. +FLIGHT_COUNTED = ("identity-entry-queued:", "unverified-auto-merge:") diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py index 2a6a09b1b7..1e9e89626b 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py @@ -109,7 +109,7 @@ import tempfile import time import zipfile -from dataclasses import dataclass, field +from dataclasses import dataclass, field, replace from datetime import UTC, datetime, timedelta from pathlib import Path from typing import Any @@ -210,6 +210,9 @@ def verdict_name(name: str) -> re.Match[str] | None: TOO_OLD = "verify:too-old" UNTRUSTED_SPEC = "verify:untrusted-spec" SPEC_CHANGED = "verify:spec-changed" +# `evidence --lane-link`: the pull request's issue link is not one the +# pr-merge lane takes (`Reader.link_refusal`), so no verdict is read. +INELIGIBLE = "verify:ineligible" # A verdict's `intent` when its pull request closed no issue in this # repository: the title and body were the spec (write-verdict.mjs). INTENT_PULL_REQUEST = "pull-request" @@ -505,6 +508,11 @@ class Reader: _mapped: dict[int, str] | None = None # the head-ref map, once loaded _humans: frozenset[int] | None = None _humans_at: datetime | None = None # when `_humans` was read + # The pull requests `link_refusal` read, each taken by the next + # `_closing_issues` instead of reading it again. + _linked: dict[int, dict[str, Any]] = field(default_factory=dict) + # The issue number `link_refusal` took for each pull request. + linked_issue: dict[int, Any] = field(default_factory=dict) def _listing(self) -> list[dict[str, Any]]: """`_current_listing`, noting when it began to be read @@ -947,7 +955,7 @@ def _closing_issues(self, number: int) -> dict[Any, Any]: body's lastEditedAt, read as the resolver reads them (closingIssuesReferences, first 10), or, when it closes none, the one issue its body's `Refs:` lines name (`merge_policy.refs_issue`).""" - pull = json_object( + pull = self._linked.pop(number, None) or json_object( self._graphql(CLOSING_QUERY, number, "closing issues").get("pullRequest") ) refs = json_object(pull.get("closingIssuesReferences")) @@ -985,6 +993,26 @@ def _closing_issues(self, number: int) -> dict[Any, Any]: ) return live + def link_refusal(self, number: int) -> str | None: + """Why the pull request's issue link is not one the pr-merge lane + takes from a pull request the lane opened, or None when it is: the + gate's own rule (`merge_policy._linked_issue`, as `lane_eligibility` + reads it), so one closing issue in this repository, or, with none, + the one issue its plain `Refs:` lines name. A failed read refuses.""" + try: + pull = json_object( + self._graphql(CLOSING_QUERY, number, "closing issues").get( + "pullRequest" + ) + ) + issue, refusal = merge_policy._linked_issue(pull, self.repo) + except (_Unreadable, merge_policy.PolicyReadError) as exc: + return f"unreadable: {exc}" + if refusal is None: + self._linked[number] = pull + self.linked_issue[number] = issue.get("number") + return refusal + def _closers(self, live: dict[Any, Any]) -> set[int]: """The pull requests in this repository, open or closed, that close any of the closing issues `live` holds.""" @@ -1400,6 +1428,76 @@ def queued_hold(self, number: int) -> str | None: return self.evidence(number, head, pr).refusal +def lane_risk_refusal( + repo: str, + number: int, + pr: dict[str, Any], + config: merge_policy.PipelineConfig, + rung: str, + issue: int | None, +) -> str | None: + """Why pr-merge's gate would hold the pull request on its risk score, or + None: `babysit_risk.score` and `holding` over the inputs the gate builds + (`babysit_merge.score_unattended`) for a lane pull request, class + `LANE_CLASS` at `rung`, with `config` the base's tightened by the + default branch head's. So the review lanes' evidence (an open CRITICAL + security finding, a review not bound to the head), a symlink, submodule + or other non-regular mode, a high path, the manifest review, the trust + gate and the cost review all hold, the cost review lifted only by the + owner's decision on the pull request or its linked `issue` + (`merge_policy.cost_decision`). A read that fails holds.""" + # Imported here: babysit_merge imports this module. + import babysit_merge # noqa: PLC0415 + import babysit_risk # noqa: PLC0415 + + try: + identity = config.agent_identity + if not identity: + return "unreadable: no agent identity" + permission = merge_policy.permission_reader(repo) + files, complete = merge_policy.changed_files(repo, pr) + lanes, _ = babysit_merge._lane_evidence( + repo, + number, + pr, + identity, + frozenset(), + permission, + [p for f in files for p in (f.path, f.previous_path) if p] + if complete + else None, + ) + files = merge_policy.with_contents(repo, pr, files) + private = json_object(json_object(pr.get("base")).get("repo")).get("private") + risk_input = babysit_risk.RiskInput( + work_class=merge_policy.LANE_CLASS, + rung=babysit_risk.Rung[rung.upper()], + files=files, + files_complete=complete, + max_changed_lines=config.max_changed_lines, + high_paths=config.denied_paths, + lanes=lanes, + blast_radius=None, + public=True if private is False else None, + pulumi_dirs=merge_policy.pulumi_dirs_at(repo, pr), + high_floor=config.high_path_floor, + c2_contradictions=config.c2_contradictions, + max_changed_files=config.max_changed_files, + lines_optional=True, + ) + risk = babysit_risk.score(risk_input) + if any(r.code == babysit_risk.COST_CODE for r in risk.reasons): + decision = merge_policy.cost_decision( + repo, pr, issue, permission=permission, agent_identity=identity + ) + if decision: + risk = babysit_risk.score(replace(risk_input, cost_decision=decision)) + except Exception as exc: # noqa: BLE001 - any doubt holds, as the gate's + return f"unreadable: {type(exc).__name__}: {exc}" + held = babysit_risk.holding(risk) + return "; ".join(f"{r.code}: {r.detail}" for r in held) or None + + def main(argv: list[str] | None = None, *, now: datetime | None = None) -> int: """`evidence `: one pull request's evidence at `head` as one JSON line (`Evidence.report`), as pr-merge's gate judges it. @@ -1411,7 +1509,21 @@ def main(argv: list[str] | None = None, *, now: datetime | None = None) -> int: of the config at that base commit and at the default branch head, the lower one (`with_default_branch`); a config that cannot be read refuses `verify:unreadable`. A pass judged on the run's saved listing is judged - again on a listing of its own, as the gate does before it allows.""" + again on a listing of its own, as the gate does before it allows. + + With `--lane-link`, the line also carries `ineligible`: null when the + pull request's issue link is one the pr-merge lane takes + (`Reader.link_refusal`), else why not, with refusal `verify:ineligible` + and no verdict or config read. With `--lane-reviews` (and `--base-sha` + and `--rung`, the lane room's effective rung), a line whose refusal is + null also carries `reviews`: null when pr-merge's gate would not hold + the pull request on its risk score (`lane_risk_refusal`), else why. + + `lane-room --postmerge-result [--rung-cap ]`: whether the + pr-merge lane may merge one more of its own pull requests now + (`merge_policy.lane_room`, `LaneRoom.report`), from this run's + `babysit_postmerge.py` report and the `CLAUDE_LANES_MERGE_RUNG_CAP` value + the workflow read (empty is no cap; missing holds the rung off).""" parser = argparse.ArgumentParser( prog="babysit_verify_evidence.py", allow_abbrev=False ) @@ -1421,9 +1533,21 @@ def main(argv: list[str] | None = None, *, now: datetime | None = None) -> int: cmd.add_argument("--branch", default="main") cmd.add_argument("--max-fails", type=int, default=merge_policy.MAX_VERIFY_FAILS) cmd.add_argument("--base-sha", default=None) + cmd.add_argument("--lane-link", action="store_true") + cmd.add_argument("--lane-reviews", action="store_true") + cmd.add_argument("--rung", default=None) cmd.add_argument("number", type=int) cmd.add_argument("head") + room = sub.add_parser("lane-room", allow_abbrev=False) + room.add_argument("--repo", required=True) + room.add_argument("--postmerge-result", required=True) + room.add_argument("--rung-cap", default=None) args = parser.parse_args(argv) + if args.command == "lane-room": + shared = postmerge.read_shared(args.postmerge_result) + answer = merge_policy.lane_room(args.repo, shared, lane_rung_cap=args.rung_cap) + print(json.dumps(answer.report())) + return 0 if ( args.max_fails < 1 or not merge_policy.SHA_RE.fullmatch(args.head) @@ -1435,7 +1559,15 @@ def main(argv: list[str] | None = None, *, now: datetime | None = None) -> int: parser.error( "--max-fails must be 1 or more, and head and --base-sha full commit SHAs" ) - max_fails = args.max_fails + reader = Reader(args.repo, args.branch, now=now, max_fails=args.max_fails) + link: dict[str, Any] = {} + config: merge_policy.PipelineConfig | None = None + if args.lane_link: + link["ineligible"] = reader.link_refusal(args.number) + if link["ineligible"] is not None: + refusal = Evidence(INELIGIBLE, args.head, (), 0, link["ineligible"]) + print(json.dumps(refusal.report() | link)) + return 0 if args.base_sha is not None: try: base = merge_policy.read_config_at(args.repo, args.base_sha) @@ -1444,15 +1576,48 @@ def main(argv: list[str] | None = None, *, now: datetime | None = None) -> int: config = merge_policy.with_default_branch(args.repo, base) except merge_policy.PolicyReadError as exc: refusal = Evidence(UNREADABLE, args.head, (), 0, f"config: {exc}") - print(json.dumps(refusal.report())) + print(json.dumps(refusal.report() | link)) return 0 - max_fails = config.max_verify_fails - reader = Reader(args.repo, args.branch, now=now, max_fails=max_fails) - evidence = reader.evidence(args.number, args.head) + reader.max_fails = config.max_verify_fails + pr = None + if args.lane_reviews: + try: + pr = merge_policy.pull_request(args.repo, args.number) + except merge_policy.PolicyReadError as exc: + refusal = Evidence(UNREADABLE, args.head, (), 0, str(exc)) + print(json.dumps(refusal.report() | link)) + return 0 + evidence = reader.evidence(args.number, args.head, pr) if evidence.refusal is None and reader.saved: reader.refresh() - evidence = reader.evidence(args.number, args.head) - print(json.dumps(evidence.report())) + evidence = reader.evidence(args.number, args.head, pr) + if args.lane_reviews and evidence.refusal is None: + head = json_object(json_object(pr).get("head")).get("sha") + if head != args.head: + link["reviews"] = f"unreadable: the head is {head}, not {args.head}" + elif config is None: + link["reviews"] = "unreadable: no --base-sha, so no config to score at" + else: + # The lowest of the room's effective rung and this base's config. + rung = merge_policy.effective_rung( + config_rung=config.rung, + loop_merge=config.loop_merge, + cap=args.rung if args.rung in merge_policy.RUNG_RANK else "off", + demoted=False, + ).rung + link["reviews"] = ( + f"unreadable: the lane's rung is {args.rung}" + if rung == "off" + else lane_risk_refusal( + args.repo, + args.number, + json_object(pr), + config, + rung, + reader.linked_issue.get(args.number), + ) + ) + print(json.dumps(evidence.report() | link)) return 0 diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_lane_reviews.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_lane_reviews.py new file mode 100644 index 0000000000..fc9e9dfa8b --- /dev/null +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_lane_reviews.py @@ -0,0 +1,178 @@ +"""The gate's risk hold enable-auto-merge reads (`lane_risk_refusal`). + +Expected values come from the gate's own scoring rules in babysit_risk.py, +which native auto-merge would otherwise skip: only a regular file that stays +regular may rate below HIGH (`_mode_problems`), so a symlink or a submodule +pointer holds; the security lane's open CRITICAL findings hold +(`security-finding`); a lane whose evidence is not bound to the head holds; +claude-review's open findings are reported only; and a plain documentation +change by the lane merges. A read that fails holds. +""" + +from __future__ import annotations + +import contextlib +import io +import json +import pathlib +import sys +import unittest +from unittest import mock + +sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent.parent)) + +import babysit_merge # noqa: E402 +import babysit_merge_policy as policy # noqa: E402 +import babysit_verify_evidence as ve # noqa: E402 +from babysit_risk import ChangedFile, LaneEvidence # noqa: E402 + +REPO = "example-org/example-repo" +HEAD = "a" * 40 +BASE = "b" * 40 +PR = { + "number": 41, + "head": {"sha": HEAD, "ref": "feat/x"}, + "base": {"sha": BASE, "repo": {"private": False}}, +} +CONFIG = policy.PipelineConfig( + "c" * 40, "C2", "pr-merge", "agent-lanes[bot]", None, (), "c2-mechanical" +) +DOC = ChangedFile( + "docs/guide.md", + None, + "modified", + 1, + 1, + True, + "100644", + "100644", + patch="@@ -1 +1 @@\n-old\n+new", +) +SYMLINK = ChangedFile("docs/link", None, "added", 1, 0, True, "120000", "000000") +SUBMODULE = ChangedFile("vendor/lib", None, "modified", 1, 1, False, "160000", "160000") + + +def lanes( + *, code: int = 0, critical: int = 0, bound: bool = True +) -> tuple[LaneEvidence, ...]: + return ( + LaneEvidence("claude-review", True, None, code), + LaneEvidence( + "claude-security-review", + bound, + None if bound else "marker-missing", + critical if bound else None, + False, + critical if bound else None, + ), + ) + + +def refusal( + files: tuple[ChangedFile, ...] = (DOC,), evidence: object = None +) -> str | None: + evidence = lanes() if evidence is None else evidence + effect = evidence if isinstance(evidence, Exception) else None + with ( + mock.patch.object(policy, "changed_files", return_value=(files, True)), + mock.patch.object(policy, "with_contents", side_effect=lambda repo, pr, f: f), + mock.patch.object(policy, "pulumi_dirs_at", return_value=()), + mock.patch.object(policy, "permission_reader", return_value=lambda login: None), + mock.patch.object(policy, "cost_decision", return_value=None), + mock.patch.object( + babysit_merge, + "_lane_evidence", + side_effect=effect, + return_value=None if effect else (evidence, []), + ), + ): + return ve.lane_risk_refusal(REPO, 41, PR, CONFIG, "C2", 7) + + +class LaneRiskRefusal(unittest.TestCase): + def test_a_plain_doc_change_holds_nothing(self) -> None: + self.assertIsNone(refusal()) + # claude-review's findings are reported, never held. + self.assertIsNone(refusal(evidence=lanes(code=3))) + + def test_a_symlink_or_a_submodule_pointer_holds(self) -> None: + for changed in (SYMLINK, SUBMODULE): + with self.subTest(changed.path): + held = refusal((DOC, changed)) + self.assertIsNotNone(held) + self.assertIn(changed.path, str(held)) + + def test_a_critical_security_finding_holds(self) -> None: + self.assertIn( + "security-finding: 1 open", str(refusal(evidence=lanes(critical=1))) + ) + + def test_an_unbound_security_review_holds(self) -> None: + self.assertIn( + "evidence-unbound:claude-security-review", + str(refusal(evidence=lanes(bound=False))), + ) + + def test_a_failed_read_holds(self) -> None: + held = refusal( + evidence=babysit_merge.HeadPusherUnproven("head-pusher-unproven:abc") + ) + self.assertTrue(str(held).startswith("unreadable: "), held) + + +class EvidenceCommandReadsTheRisk(unittest.TestCase): + def cli( + self, + evidence: ve.Evidence, + risk: str | None, + *, + head: str = HEAD, + rung: str | None = "C2", + ) -> dict[str, object]: + out = io.StringIO() + argv = ["evidence", "--lane-reviews", "--repo", REPO, "--base-sha", BASE] + argv += ["--rung", rung] if rung is not None else [] + with ( + mock.patch.object(policy, "read_config_at", return_value=CONFIG), + mock.patch.object(policy, "with_default_branch", return_value=CONFIG), + mock.patch.object( + policy, "pull_request", return_value=PR | {"head": {"sha": head}} + ), + mock.patch.object( + ve.Reader, "evidence", lambda self, n, h, pr=None: evidence + ), + mock.patch.object(ve, "lane_risk_refusal", return_value=risk) as read, + contextlib.redirect_stdout(out), + ): + ve.main([*argv, "41", HEAD]) + self.read = read + return json.loads(out.getvalue()) + + def test_a_pass_carries_the_risk_hold_at_the_room_s_rung(self) -> None: + report = self.cli(ve.Evidence(None, HEAD, (), 0), "unsafe-path: docs/link") + self.assertEqual( + (report["refusal"], report["reviews"]), (None, "unsafe-path: docs/link") + ) + self.assertEqual(self.read.call_args.args[4], "C2") + self.assertIsNone(self.cli(ve.Evidence(None, HEAD, (), 0), None)["reviews"]) + + def test_no_rung_from_the_room_holds(self) -> None: + for rung in (None, "off", "C9"): + with self.subTest(rung=rung): + report = self.cli(ve.Evidence(None, HEAD, (), 0), None, rung=rung) + self.assertTrue(str(report["reviews"]).startswith("unreadable: ")) + self.read.assert_not_called() + + def test_a_refused_verdict_reads_no_risk(self) -> None: + report = self.cli(ve.Evidence(ve.FAIL, HEAD, (), 0), None) + self.assertNotIn("reviews", report) + self.read.assert_not_called() + + def test_a_moved_head_holds(self) -> None: + report = self.cli(ve.Evidence(None, HEAD, (), 0), None, head="d" * 40) + self.assertTrue(str(report["reviews"]).startswith("unreadable: the head is")) + self.read.assert_not_called() + + +if __name__ == "__main__": + unittest.main() diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_lane_room.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_lane_room.py new file mode 100644 index 0000000000..581849973b --- /dev/null +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_lane_room.py @@ -0,0 +1,307 @@ +"""The pr-merge lane's room for one more native merge (`lane_room`). + +Expected values come from the gate's own run-level refusals, which native +auto-merge skips: `effective_rung` (the lowest of `merge.rung`, +`babysit_loop_merge`, the rung cap and the demotion; an unknown or missing cap +is off, an empty one no cap), a post-merge refusal, an authority other than +pr-merge, and `in_flight`'s count against `merge.max-in-flight`. A read that +fails refuses nothing and allows nothing (`unreadable`). +""" + +from __future__ import annotations + +import contextlib +import io +import json +import pathlib +import sys +import unittest +from datetime import UTC, datetime, timedelta +from unittest import mock + +sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent.parent)) + +import babysit_merge_policy as policy # noqa: E402 +import babysit_verify_evidence as ve # noqa: E402 + +REPO = "example-org/example-repo" +IDENTITY = "agent-lanes[bot]" +MAIN = "c" * 40 +FLOOR = datetime(2026, 10, 1, tzinfo=UTC) +# Five minutes after the answer was computed: inside the gate's reuse age. +NOW = FLOOR + timedelta(minutes=5) + + +def config(**overrides: object) -> policy.PipelineConfig: + fields = { + "base_sha": MAIN, + "rung": "C2", + "authority": "pr-merge", + "agent_identity": IDENTITY, + "max_changed_lines": None, + "denied_paths": (), + "loop_merge": "c2-mechanical", + } + fields.update(overrides) + return policy.PipelineConfig(**fields) # type: ignore[arg-type] + + +def shared( + *, demoted: bool = False, refusal: str | None = None, identity: str = IDENTITY +) -> policy.SharedPostMerge: + return policy.SharedPostMerge( + main_sha=MAIN, + config_sha=MAIN, + identity=identity, + self_logins=frozenset(), + lane_merges_only=True, + computed_at=FLOOR, + demotion=policy.Demotion( + demoted, "record-open" if demoted else None, (7,) if demoted else () + ), + floor=FLOOR, + gate=policy.PostMergeGate(refusal, {}, lambda merge: True), + ) + + +class LaneRoom(unittest.TestCase): + def room( + self, + answer: policy.SharedPostMerge | None = None, + *, + cfg: policy.PipelineConfig | None = None, + cap: str | None = "", + flight: policy.InFlight | None = None, + config_error: Exception | None = None, + now: datetime = NOW, + demotion_changed: bool = False, + ) -> policy.LaneRoom: + seen: dict[str, object] = {} + + def in_flight(repo: str, **kwargs: object) -> policy.InFlight: + seen.update(kwargs) + return flight or policy.InFlight(True, None, frozenset({11}), 0, 2) + + with ( + mock.patch.object(policy, "_default_branch", return_value="main"), + mock.patch.object( + policy, + "_default_branch_config", + side_effect=config_error, + return_value=cfg or config(), + ), + mock.patch.object( + policy, "_run_max_in_flight", return_value=(5, "default-branch") + ), + mock.patch.object(policy, "in_flight", side_effect=in_flight), + mock.patch.object( + policy, "_demotion_changed", return_value=demotion_changed + ), + # shared_refusal's reads: the answer's own config commit and main + # head, so only its age and demotion decide. + mock.patch.object(policy, "newest_config_commit", return_value=MAIN), + mock.patch.object(policy, "_get", return_value=(200, {"sha": MAIN})), + ): + result = policy.lane_room( + REPO, + answer if answer is not None else shared(), + lane_rung_cap=cap, + now=now, + ) + self.seen = seen + return result + + def test_a_clear_lane_counts_its_merges_in_flight_against_the_cap(self) -> None: + room = self.room() + self.assertEqual( + (room.refusal, room.unreadable, room.count, room.cap, room.queued), + (None, None, 2, 5, frozenset({11})), + ) + # The rung the gate scores at: merge.rung C2, the loop setting c2. + self.assertEqual(room.rung, "C2") + self.assertEqual(self.room(cap="C2").rung, "C2") + # The lane's own merges, since the post-merge floor, judged by its check. + self.assertEqual( + ( + self.seen["lane_merges_only"], + self.seen["merged_since"], + self.seen["max_in_flight"], + ), + (True, FLOOR, 5), + ) + + def test_a_demotion_holds_the_lane_off(self) -> None: + self.assertEqual(self.room(shared(demoted=True)).refusal, "rung-off:demotion") + + def test_a_rung_cap_of_off_or_a_missing_cap_holds_it_off(self) -> None: + self.assertEqual(self.room(cap="off").refusal, "rung-off:rungCap") + self.assertEqual(self.room(cap=None).refusal, "rung-off:rungCap") + self.assertIsNone(self.room(cap="C2").refusal) + + def test_merge_rung_or_the_loop_setting_off_holds_it_off(self) -> None: + self.assertEqual( + self.room(cfg=config(rung="off")).refusal, "rung-off:merge.rung" + ) + self.assertEqual( + self.room(cfg=config(loop_merge="human-only")).refusal, + "rung-off:babysit_loop_merge", + ) + + def test_a_post_merge_failure_holds_it_off(self) -> None: + room = self.room(shared(refusal="failure:red-main#40")) + self.assertEqual(room.refusal, "post-merge:failure:red-main#40") + + def test_another_authority_holds_it_off(self) -> None: + room = self.room(cfg=config(authority="local-babysit")) + self.assertEqual(room.refusal, policy.AUTHORITY_UNCONFIGURED) + + def test_at_the_cap_it_reports_the_cap_and_refuses_nothing(self) -> None: + full = policy.InFlight(False, "identity-entry-queued:#14", frozenset({10, 14})) + room = self.room(flight=full) + self.assertEqual( + (room.refusal, room.unreadable, room.count, room.cap), (None, None, 5, 5) + ) + + def test_an_answer_the_gate_would_not_reuse_is_unreadable(self) -> None: + # The gate reads afresh past SHARED_MAX_AGE or after a demotion record + # changed (shared_refusal); a re-run job reads the run's first report. + cases = { + "stale": dict(now=FLOOR + policy.SHARED_MAX_AGE + timedelta(seconds=1)), + "future": dict(now=FLOOR - timedelta(seconds=1)), + "demotion changed": dict(demotion_changed=True), + } + for name, kwargs in cases.items(): + with self.subTest(name): + room = self.room(**kwargs) # type: ignore[arg-type] + self.assertIsNone(room.refusal) + self.assertTrue(room.unreadable) + self.assertFalse(self.seen, "no in-flight read after an unusable answer") + + def test_a_lane_merge_the_answer_does_not_count_is_unreadable(self) -> None: + # The gate's reuse rule (shared_refusal) reads afresh when a lane + # merge landed after the report was computed (`new-merge:#`), so + # its failed full run is not missed; the room holds rather than arm. + seen: list[dict[str, object]] = [] + + def refusal(repo: str, answer: object, **kwargs: object) -> str: + seen.append(kwargs) + return "new-merge:#7" + + with mock.patch.object(policy, "shared_refusal", side_effect=refusal): + room = self.room() + self.assertEqual( + (room.refusal, room.unreadable), + (None, "post-merge answer new-merge:#7"), + ) + self.assertFalse(self.seen, "no in-flight read after an unusable answer") + # Bound as the gate binds a candidate at the default branch head. + self.assertEqual( + { + k: seen[0][k] + for k in ("branch", "base_sha", "identity", "lane", "authority") + }, + { + "branch": "main", + "base_sha": MAIN, + "identity": IDENTITY, + "lane": True, + "authority": "pr-merge", + }, + ) + self.assertEqual(seen[0]["self_logins"], frozenset()) + + def test_an_active_demotion_is_not_re_read(self) -> None: + # Already refusing: a changed record cannot make it allow. + room = self.room(shared(demoted=True), demotion_changed=True) + self.assertEqual(room.refusal, "rung-off:demotion") + + def test_a_failed_read_refuses_nothing_and_allows_nothing(self) -> None: + cases = { + "no answer": dict(answer=None), + "queue": dict(flight=policy.InFlight(False, "merge-queue-unreadable")), + "enqueuer": dict( + flight=policy.InFlight( + False, "entry-enqueuer-unknown:#3", frozenset({3}) + ) + ), + "config": dict(config_error=policy.PolicyReadError("contents 502")), + "identity": dict(answer=shared(identity="someone-else[bot]")), + } + for name, kwargs in cases.items(): + with self.subTest(name): + answer = kwargs.pop("answer", shared()) + if name == "no answer": + with mock.patch.object(policy, "in_flight") as flight: + room = policy.lane_room(REPO, None, lane_rung_cap="") + flight.assert_not_called() + else: + room = self.room(answer, **kwargs) # type: ignore[arg-type] + self.assertIsNone(room.refusal) + self.assertTrue(room.unreadable) + + +class LaneRoomCommand(unittest.TestCase): + def test_it_prints_the_room_from_the_post_merge_report(self) -> None: + seen: list[object] = [] + + def room( + repo: str, answer: object, *, lane_rung_cap: str | None + ) -> policy.LaneRoom: + seen.append((repo, answer, lane_rung_cap)) + return policy.LaneRoom(count=1, cap=5, queued=frozenset({3}), rung="C2") + + out = io.StringIO() + with ( + mock.patch.object( + ve.postmerge, "read_shared", return_value="answer" + ) as read, + mock.patch.object(policy, "lane_room", side_effect=room), + contextlib.redirect_stdout(out), + ): + code = ve.main( + [ + "lane-room", + "--repo", + REPO, + "--postmerge-result", + "/tmp/x.json", + "--rung-cap", + "", + ] + ) + self.assertEqual(code, 0) + read.assert_called_once_with("/tmp/x.json") + self.assertEqual(seen, [(REPO, "answer", "")]) + self.assertEqual( + json.loads(out.getvalue()), + { + "refusal": None, + "unreadable": None, + "count": 1, + "cap": 5, + "queued": [3], + "rung": "C2", + }, + ) + + def test_a_missing_report_is_unreadable(self) -> None: + out = io.StringIO() + with contextlib.redirect_stdout(out): + ve.main( + [ + "lane-room", + "--repo", + REPO, + "--postmerge-result", + "/nonexistent/post-merge.json", + "--rung-cap", + "", + ] + ) + report = json.loads(out.getvalue()) + self.assertIsNone(report["refusal"]) + self.assertTrue(report["unreadable"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_policy.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_policy.py index d9c99ba663..bf49d8f1d4 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_policy.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_policy.py @@ -1985,6 +1985,8 @@ def test_a_cap_of_five_admits_four_in_flight_and_refuses_five(self) -> None: admitted = self.capped(5, 2, 2) self.assertEqual((admitted.clear, admitted.reason), (True, None)) self.assertEqual(admitted.unverified, 2) + # Two queued entries and two unverified merges: four in flight. + self.assertEqual(admitted.count, 4) self.assertEqual(self.capped(5, 3, 2).reason, "unverified-auto-merge:#21") self.assertEqual(self.capped(5, 5, 0).reason, "identity-entry-queued:#14") diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py index e09d28b2cf..cdf870f140 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py @@ -4120,5 +4120,93 @@ def test_an_unreadable_config_refuses(self) -> None: self.assertEqual(json.loads(out.getvalue())["refusal"], ve.UNREADABLE) +class EvidenceCommandReadsTheLaneLink(unittest.TestCase): + """`evidence --lane-link`, which enable-auto-merge.mjs runs: the issue + link the pr-merge lane takes from its own pull request, by the rule in + docs/conventions/pr-pipeline/README.md ("Eligibility"): one closing issue + here, or, with none, the one issue plain `Refs: #` lines name; two + issues refuse `refs-issues:2`, a pull request or a number naming nothing + `refs-not-an-issue`, and a failed read refuses. A refused link reads no + verdict.""" + + def cli(self, fake: FakeGitHub) -> dict[str, Any]: + out = io.StringIO() + with ( + mock.patch.object(policy, "pull_request", return_value=pull()), + mock.patch.object(postmerge, "gh_capture", side_effect=fake), + mock.patch.object(policy, "gh_capture", side_effect=fake), + contextlib.redirect_stdout(out), + ): + argv = ["evidence", "--lane-link", "--repo", REPO, str(PR), HEAD] + self.assertEqual(ve.main(argv, now=NOW), 0) + return json.loads(out.getvalue()) + + def refs(self, body: str, **refs: str) -> FakeGitHub: + fake = passing() + fake.closing, fake.body = [], body + fake.refs = {int(n.removeprefix("n")): kind for n, kind in refs.items()} + return fake + + def test_a_closing_pull_request_is_linked_and_its_pass_allows(self) -> None: + fake = passing() + report = self.cli(fake) + self.assertEqual((report["ineligible"], report["refusal"]), (None, None)) + # The verdict read takes the link read's pull request: one read. + closing = [c for c in fake.calls if any("closingIssues" in a for a in c)] + self.assertEqual(len(closing), 1) + + def test_one_plain_refs_line_is_linked_and_its_verdict_is_read(self) -> None: + fake = self.refs("Refs: #7\n\n## Summary", n7=REPO) + report = self.cli(fake) + self.assertIsNone(report["ineligible"]) + self.assertEqual(listings(fake), 1) + + def test_a_refused_link_reads_no_verdict(self) -> None: + for body, refs, want in ( + ("Refs: #7\nRefs: #8\n", {"n7": REPO, "n8": REPO}, "refs-issues:2"), + ("Refs: #7, #8\n", {"n7": REPO}, "refs-line-not-plain"), + ("refs: #7\n", {"n7": REPO}, "refs-line-not-plain"), + ("Refs: #7\n", {"n7": "PullRequest"}, "refs-not-an-issue"), + ("Refs: #9\n", {}, "refs-not-an-issue"), + ("## Summary\n", {}, "linked-issues:0"), + ): + with self.subTest(body=body): + fake = self.refs(body, **refs) + report = self.cli(fake) + self.assertEqual( + (report["ineligible"], report["refusal"]), (want, ve.INELIGIBLE) + ) + self.assertEqual(listings(fake), 0) + + def test_a_closing_issue_elsewhere_or_two_refuse(self) -> None: + for closing, want in ( + ([(7, REPO), (8, REPO)], "linked-issues:2"), + ([(7, "other-org/other-repo")], "linked-issue-other-repository"), + ): + fake = passing() + fake.closing = closing + self.assertEqual(self.cli(fake)["ineligible"], want) + + def test_a_failed_link_read_refuses(self) -> None: + for fake in (passing(), self.refs("Refs: #7\n", n7="error")): + if not fake.refs: + fake.fail.add("graphql") + report = self.cli(fake) + self.assertTrue(str(report["ineligible"]).startswith("unreadable: ")) + self.assertEqual(report["refusal"], ve.INELIGIBLE) + self.assertEqual(listings(fake), 0) + + def test_without_the_flag_no_link_is_read(self) -> None: + fake = self.refs("Refs: #7\nRefs: #8\n") + out = io.StringIO() + with ( + mock.patch.object(policy, "pull_request", return_value=pull()), + mock.patch.object(postmerge, "gh_capture", side_effect=fake), + contextlib.redirect_stdout(out), + ): + ve.main(["evidence", "--repo", REPO, str(PR), HEAD], now=NOW) + self.assertNotIn("ineligible", json.loads(out.getvalue())) + + if __name__ == "__main__": unittest.main() diff --git a/scripts/ci-lanes-pr-hold.test.sh b/scripts/ci-lanes-pr-hold.test.sh index acbcb59095..a7e574bec3 100755 --- a/scripts/ci-lanes-pr-hold.test.sh +++ b/scripts/ci-lanes-pr-hold.test.sh @@ -95,8 +95,11 @@ fi # trusted-actors.json prints the working tree's list, or answers 404 when # STUB_NO_ACTORS is set. An artifacts read by name prints the run ids # STUB_ARTIFACTS (a JSON object of name to run ids) gives that name, through -# the step's own --jq; a run read prints that id's entry of STUB_RUNS, or -# answers 404. +# the step's own --jq, each artifact's id ten times its run's; a run read +# prints that id's entry of STUB_RUNS, or answers 404. An artifact archive +# holds STUB_RECORD as its verdict.json (one judging #1, closing, by +# default), or answers 404 when STUB_NO_RECORD is set; the closing-issues +# query answers STUB_CLOSING (#1 of o/r by default) through the step's --jq. mkdir -p "$TMP_ROOT/bin" cat >"$TMP_ROOT/bin/gh" <<'EOF' #!/usr/bin/env bash @@ -106,6 +109,22 @@ discussion() { [[ "$(grep -cE '/(comments|reviews)\?' "$STUB_LOG")" -gt "${STUB_FAILS:-0}" ]] || { echo 'gh: Server Error (HTTP 502)' >&2; exit 1; } } case "$*" in +"api graphql "*closingIssuesReferences*) + closing=${STUB_CLOSING:-} + [[ -n "$closing" ]] || closing='{"totalCount":1,"nodes":[{"number":1,"repository":{"nameWithOwner":"o/r"}}]}' + jq -c --argjson closing "$closing" \ + '{data: {repository: {pullRequest: {closingIssuesReferences: $closing}}}}' <<&2; exit 1; } + record=${STUB_RECORD:-} + [[ -n "$record" ]] || record='{"linked_issues":[{"number":1}],"spec_kind":"closes"}' + python3 -c 'import io, sys, zipfile +buffer = io.BytesIO() +with zipfile.ZipFile(buffer, "w") as archive: + archive.writestr("verdict.json", sys.argv[1]) +sys.stdout.buffer.write(buffer.getvalue())' "$record" + ;; "api graphql "*) [[ -z "${STUB_DISARM_FAILS:-}" ]] || { echo 'gh: Resource not accessible by integration' >&2; exit 1; } echo '{"data":{}}' @@ -132,8 +151,8 @@ case "$*" in # The step's own --jq filter, the last argument, runs over a listing that # also holds an expired artifact of that name and an unexpired one whose # name only starts with it, both of run 11, which neither may count. - jq -c --arg name "$name" '{artifacts: ([.[$name][]? | {name: $name, expired: false, workflow_run: {id: .}}] - + [{name: $name, expired: true, workflow_run: {id: 11}}, {name: ($name + "-i1"), expired: false, workflow_run: {id: 11}}])}' \ + jq -c --arg name "$name" '{artifacts: ([.[$name][]? | {name: $name, id: (. * 10), expired: false, workflow_run: {id: .}}] + + [{name: $name, id: 110, expired: true, workflow_run: {id: 11}}, {name: ($name + "-i1"), id: 110, expired: false, workflow_run: {id: 11}}])}' \ <<<"${STUB_ARTIFACTS:-{\}}" | jq -r "${*: -1}" ;; */actions/runs/*) @@ -169,7 +188,8 @@ expect() { : >"$TMP_ROOT/log" out="$(PATH="$TMP_ROOT/bin:$PATH" STUB_LOG="$TMP_ROOT/log" STUB_MODULE="${STUB_MODULE:-$MODULE}" \ GITHUB_EVENT_NAME="${EVENT:-pull_request}" EVENT_ACTION="${ACTION-synchronize}" GITHUB_REF="${REF:-refs/pull/7/merge}" PR_NUMBER="${PR_NUMBER-7}" \ - GITHUB_REPOSITORY=o/r RUNNER_TEMP="$TMP_ROOT" GROUP_BASE_SHA="${GROUP_BASE_SHA-}" bash -e "$TMP_ROOT/body.sh" 2>&1)" && rc=0 || rc=$? + GITHUB_REPOSITORY=o/r RUNNER_TEMP="$TMP_ROOT" GROUP_BASE_SHA="${GROUP_BASE_SHA-}" \ + LANES_DISABLED="${KILL-false}" LANES_REPO_DISABLED="${REPO_KILL-false}" bash -e "$TMP_ROOT/body.sh" 2>&1)" && rc=0 || rc=$? calls=$(grep -cE '/(comments|reviews)\?' "$TMP_ROOT/log") if [[ "$rc" -ne "$want_rc" ]]; then fail "$label: expected rc=$want_rc got rc=$rc out='$out'" @@ -196,6 +216,26 @@ expect "a person's pull request is left to pr-merge's gate, with no discussion r STUB_PULL="$(pull 'Closes #1' '{"id":1,"login":"melodic-automation-lanes[bot]","type":"User"}')" expect "a user account with the lanes App's login is not the lanes App" 0 0 "not the lanes App's" +# The lanes kill switch stops every lanes App pull request, and only those. +for kill in "KILL=true" "KILL=" "REPO_KILL=true" "REPO_KILL=1"; do + STUB_PULL="$(pull 'Closes #1')" + export "${kill?}" + expect "with $kill, a lanes App pull request fails" 1 0 "the lanes kill switch is on" + STUB_PULL="$(pull 'Closes #1' '{"id":153232337,"login":"kyle-sexton","type":"User"}')" + expect "with $kill, a person's pull request is not held by it" 0 0 "not the lanes App's" + STUB_PULL="$(pull 'Closes #1' '{"id":49699333,"login":"dependabot[bot]","type":"Bot"}')" + expect "with $kill, a Dependabot pull request is not held by it" 0 0 "not the lanes App's" + STUB_PULL="$(pull 'Closes #1')" + unset KILL REPO_KILL +done +# shellcheck disable=SC2016 # a literal workflow expression, not shell +if grep -qF 'LANES_DISABLED: ${{ vars.CLAUDE_LANES_DISABLED }}' <<<"$step" && + grep -qF 'LANES_REPO_DISABLED: ${{ vars.CLAUDE_LANES_REPO_DISABLED }}' <<<"$step"; then + ok "the step reads both kill switches from the vars context" +else + fail "the step does not read both kill switches from the vars context" +fi + STUB_PULL="$(pull $'Closes #1\n\nHold: wait for the canary')" expect "a Hold: line in the body fails" 1 3 "::error::#7: its body holds it" STUB_PULL="$(pull $'Closes #1\r\n hold: owner call')" @@ -348,6 +388,27 @@ if [[ "$(lookups)" -eq 1 ]]; then else fail "a plain-named pass took $(lookups) lookups" fi +# The pass binds the link it judged (merge-holds.mjs linkHold): a body edited +# after arming to close or name another issue holds the group. +NO_CLOSING='{"totalCount":0,"nodes":[]}' +export STUB_CLOSING STUB_RECORD STUB_NO_RECORD +STUB_PULL="$(pull 'Refs: #1')" STUB_CLOSING="$NO_CLOSING" STUB_RECORD='{"linked_issues":[{"number":1}],"spec_kind":"refs"}' +expect "in the merge queue, a Refs pull request whose pass judged that issue passes" 0 3 "#7: no verifier hold: pr-verify passed this head." +STUB_PULL="$(pull 'Closes #2')" STUB_CLOSING='{"totalCount":1,"nodes":[{"number":2,"repository":{"nameWithOwner":"o/r"}}]}' +expect "in the merge queue, a Refs: #1 pass does not merge a body edited to Closes #2" 1 3 "its link changed since pr-verify passed it: it closes #2, and the pass judged #1 (refs)" +STUB_PULL="$(pull 'Closes #1')" STUB_CLOSING='' STUB_RECORD='{"linked_issues":[{"number":1}],"spec_kind":"refs"}' +expect "in the merge queue, a pass judged as a Refs slice does not cover the issue closed" 1 3 "its link changed since pr-verify passed it" +STUB_PULL="$(pull $'Refs: #1\nRefs: #3')" STUB_CLOSING="$NO_CLOSING" +expect "in the merge queue, a body naming two Refs issues holds" 1 3 "it names no issue pr-merge's lane takes" +STUB_PULL="$(pull 'Closes #1')" STUB_RECORD='' STUB_CLOSING='{"totalCount":2,"nodes":[{"number":1,"repository":{"nameWithOwner":"o/r"}},{"number":3,"repository":{"nameWithOwner":"o/r"}}]}' +expect "in the merge queue, a body closing two issues holds" 1 3 "it does not close exactly one issue in this repository" +STUB_CLOSING='' STUB_RECORD='{"verdict":"pass"}' +expect "in the merge queue, a record with no linked issues holds" 1 3 "its verifier pass's record could not be read" +STUB_RECORD='' STUB_NO_RECORD=1 +expect "in the merge queue, a record that does not download fails closed" 1 3 "could not read the verdict of artifact 110" +STUB_NO_RECORD='' STUB_RECORD='{"linked_issues":[{"number":9}],"spec_kind":"closes"}' STUB_REVIEWS="[$(approval kyle-sexton 153232337 APPROVED 2026-10-23T11:00:00Z "$SHA")]" +expect "in the merge queue, a trusted approval of the head lifts a changed link" 0 3 "#7: no body hold, no member veto and no hold path." +STUB_RECORD='' STUB_REVIEWS='' STUB_ARTIFACTS="{\"pr-verify-7-$SHA-pass-notes-$DIGEST\":[11]}" expect "in the merge queue, a digest-named pass-notes at the head passes" 0 3 "no verifier hold: pr-verify passed this head" STUB_ARTIFACTS='{}'