diff --git a/.changes/pr-pipeline/fix-lane-room-recompute-9577a1c3.md b/.changes/pr-pipeline/fix-lane-room-recompute-9577a1c3.md new file mode 100644 index 0000000000..f271ab64ae --- /dev/null +++ b/.changes/pr-pipeline/fix-lane-room-recompute-9577a1c3.md @@ -0,0 +1,7 @@ +--- +bump: patch +--- + +### Fixed + +- A pending lane merge no longer makes `enable-auto-merge` disarm every armed lanes App pull request. The post-merge step takes about as long as the gate's two-minute reuse age, so the lane's room read the report as too old. It now keeps the report while nothing it was read from has moved, and otherwise checks again as the gate does, with the lane records the last run kept, so that check reads them without an artifact or run read each. When the room is held, by a refusal or because it cannot be read, the run raises one warning annotation and step-summary line naming the reason and each lanes App pull request it left unarmed or, in report-only mode, that a disarm would have turned off. A pull request still armed because its disarm failed gets an error annotation and summary line instead. It posts no comment on the pull requests. diff --git a/.changes/source-control/fix-lane-room-recompute-9577a1c3.md b/.changes/source-control/fix-lane-room-recompute-9577a1c3.md new file mode 100644 index 0000000000..c4f7f5ad94 --- /dev/null +++ b/.changes/source-control/fix-lane-room-recompute-9577a1c3.md @@ -0,0 +1,7 @@ +--- +bump: patch +--- + +### Fixed + +- `babysit_verify_evidence.py lane-room` no longer reports the lane's room unreadable when the run's post-merge answer is past the gate's two-minute reuse age only because a lane merge is still unverified (`unverified-stale`). It keeps the answer when nothing it was read from has moved: main's head, its config commit, the demotion records, and the unverified merges' full and canary runs (`babysit_postmerge.pending_runs_unchanged`, one run listing: none reached a conclusion since, none is past the dispatch deadline, and no quarantine that excused a failing suite has expired). Otherwise it runs the post-merge check again as the gate does (`babysit_merge_policy.lane_room` with the gate's `post_merge`), so a failure found since still refuses. A check that fails or cannot be read leaves the room unreadable, and every other refusal reads as before. `babysit_postmerge.gate_reader` is the one post-merge reader the gate and the room share. The post-merge report now carries `excusesExpireAt`, when the earliest quarantine that excused a failing suite expires. `SINCE_CLOCK_MARGIN` is now 60 seconds, which covers clock skew only, because `computedAt` is stamped before the reads it covers. It applies to the demotion re-check and to the run check. diff --git a/.github/scripts/pr-merge/enable-auto-merge.mjs b/.github/scripts/pr-merge/enable-auto-merge.mjs index d1244997ed..49b873fae2 100755 --- a/.github/scripts/pr-merge/enable-auto-merge.mjs +++ b/.github/scripts/pr-merge/enable-auto-merge.mjs @@ -53,7 +53,13 @@ // 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. +// armed and every armed lanes App pull request is disarmed. A report past +// its reuse age only for a lane merge still unverified is kept while +// nothing it was read from has moved, else checked again as the gate +// does. A held room raises one warning annotation and step-summary line +// naming why and each lane pull request left unarmed or, report-only, +// that a disarm would have turned off, and one error and summary line +// naming each one still armed because its disarm failed. // - 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 @@ -108,7 +114,7 @@ // run cache, as the gate's // GITHUB_API_URL import { spawnSync } from "node:child_process"; -import { readFileSync } from "node:fs"; +import { appendFileSync, readFileSync } from "node:fs"; import process from "node:process"; import { pathToFileURL } from "node:url"; @@ -619,22 +625,26 @@ export async function main({ env = process.env, github, verify, room, sleep = (m 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. + // An unknown room holds as a refusal does: unknown holds. + const laneHeld = + lane?.refusal != null ? `pr-merge's gate holds its lane off (${lane.refusal})` : lane?.unreadable != null ? `the lane's room could not be read (${lane.unreadable})` : null; 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 (laneHeld !== null) { + return { disarm: true, reason: laneHeld }; } 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; }; + // Each lane pull request the room's refusal or unread room holds, by + // what became of it: unarmed (never armed, or disarmed), armed (its + // disarm failed, so the hold is not enforced) or report-only (a disarm + // would have run). + const roomHeld = new Map(); let failed = 0; const minutes = riskEscalationMinutes(env.RESOLVED_CONFIG ?? ""); // A reader run's per pull request refusal (a hold path or cost hold, or @@ -684,6 +694,9 @@ export async function main({ env = process.env, github, verify, room, sleep = (m 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?.disarm) { + roomHeld.set(pull.number, pull.autoMergeRequest != null ? "armed" : "unarmed"); + } if (held !== null) { ({ action, reason } = { action: held.disarm && pull.autoMergeRequest != null ? "disarm" : "none", reason: held.reason }); } @@ -736,6 +749,9 @@ export async function main({ env = process.env, github, verify, room, sleep = (m continue; } if (!write) { + if (roomHeld.get(pull.number) === "armed") { + roomHeld.set(pull.number, "report-only"); + } console.log(`enable-auto-merge: #${pull.number} would ${action} (report-only): ${escapeCommand(reason)}`); await note(pull, detail, { clears, reason }); continue; @@ -757,6 +773,9 @@ export async function main({ env = process.env, github, verify, room, sleep = (m console.log(`enable-auto-merge: #${pull.number} armed at ${pull.headRefOid}`); } else { await github("POST", "/graphql", { query: DISARM_MUTATION, variables: { id: pull.id } }); + if (roomHeld.has(pull.number)) { + roomHeld.set(pull.number, "unarmed"); + } console.log(`enable-auto-merge: #${pull.number} disarmed: ${escapeCommand(reason)}`); } } catch (error) { @@ -769,6 +788,31 @@ export async function main({ env = process.env, github, verify, room, sleep = (m await note(pull, detail, { clears, reason }); } } + // One warning (and an error while a disarm failed) and their summary + // lines per run, not a comment per pull request: the room is read again + // every scheduled run. + const held = (state) => [...roomHeld].filter(([, outcome]) => outcome === state).map(([number]) => `#${number}`).join(", "); + const lines = []; + const warned = [ + ["left unarmed", held("unarmed")], + ["report-only, a disarm would have run", held("report-only")], + ].filter(([, numbers]) => numbers); + if (warned.length > 0) { + lines.push(["warning", escapeCommand(`${laneHeld}; ${warned.map(([what, numbers]) => `${what}: ${numbers}`).join("; ")}`)]); + } + if (held("armed")) { + lines.push(["error", escapeCommand(`${laneHeld}; still armed, its disarm failed, so the hold is not enforced: ${held("armed")}`)]); + } + for (const [level, text] of lines) { + console.log(`::${level} title=enable-auto-merge::${text}`); + } + if (lines.length > 0 && env.GITHUB_STEP_SUMMARY) { + try { + appendFileSync(env.GITHUB_STEP_SUMMARY, lines.map(([, text]) => `enable-auto-merge: ${text}\n`).join("")); + } catch (error) { + console.log(`enable-auto-merge: the step summary was not written: ${describe(error)}`); + } + } return failed === 0 ? 0 : 1; } catch (error) { console.log(`::error title=enable-auto-merge::${describe(error)}`); diff --git a/.github/scripts/pr-merge/enable-auto-merge.test.mjs b/.github/scripts/pr-merge/enable-auto-merge.test.mjs index 27b4a21463..2bf6d42e46 100644 --- a/.github/scripts/pr-merge/enable-auto-merge.test.mjs +++ b/.github/scripts/pr-merge/enable-auto-merge.test.mjs @@ -8,7 +8,7 @@ // 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"; +import { mkdtempSync, readFileSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import process from "node:process"; @@ -1114,6 +1114,71 @@ test("a room that cannot be read arms nothing and disarms every armed lane pull assert.deepEqual(writes(), [{ mutation: "disarm", id: "PR_2" }]); }); +// A held room that logs only to the console leaves nothing on the run +// saying why nothing armed. One annotation and one summary +// line per run, naming the reason and every lane pull request left unarmed. +test("a held room raises one warning and one step-summary line naming why and each pull request left unarmed", 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" } }), pull(4, { isDraft: true })]; + const cases = [ + [{ ...ROOM, unreadable: "post-merge answer unverified-stale" }, "the lane's room could not be read (post-merge answer unverified-stale); left unarmed: #1, #2"], + [{ ...ROOM, refusal: "rung-off:demotion" }, "pr-merge's gate holds its lane off (rung-off:demotion); left unarmed: #1, #2"], + ]; + for (const [lane, expected] of cases) { + const summary = join(mkdtempSync(join(tmpdir(), "enable-auto-merge-summary-")), "summary.md"); + writeFileSync(summary, ""); + const { lines } = await run(pulls, { GITHUB_STEP_SUMMARY: summary }, {}, lane); + assert.deepEqual( + lines.filter((line) => line.startsWith("::warning")), + [`::warning title=enable-auto-merge::${expected}`], + ); + assert.equal(readFileSync(summary, "utf8"), `enable-auto-merge: ${expected}\n`); + } + // A lane with room raises neither. + const summary = join(mkdtempSync(join(tmpdir(), "enable-auto-merge-summary-")), "summary.md"); + writeFileSync(summary, ""); + const { lines, writes } = await run(pulls, { GITHUB_STEP_SUMMARY: summary }); + assert.deepEqual(writes, [{ mutation: "arm", id: "PR_1", head: HEAD }]); + assert.deepEqual( + lines.filter((line) => line.startsWith("::warning")), + [], + ); + assert.equal(readFileSync(summary, "utf8"), ""); +}); + +// A pull request whose disarm failed, or that report-only left armed, is +// still armed: naming it "left unarmed" would say the hold was enforced when +// it was not. +test("a held room names a pull request whose disarm failed as still armed, at error level", async () => { + const armed = { enabledAt: "2026-10-23T10:00:00Z" }; + const summary = join(mkdtempSync(join(tmpdir(), "enable-auto-merge-summary-")), "summary.md"); + writeFileSync(summary, ""); + const lane = { ...ROOM, refusal: "rung-off:demotion" }; + const { lines, code } = await run([pull(1), pull(2, { autoMergeRequest: armed })], { GITHUB_STEP_SUMMARY: summary }, { disarmError: new Error("POST /graphql failed with status 502") }, lane); + assert.equal(code, 1); + const why = "pr-merge's gate holds its lane off (rung-off:demotion)"; + assert.deepEqual( + lines.filter((line) => /^::(warning|error) title=enable-auto-merge::pr-merge/.test(line)), + [`::warning title=enable-auto-merge::${why}; left unarmed: #1`, `::error title=enable-auto-merge::${why}; still armed, its disarm failed, so the hold is not enforced: #2`], + ); + assert.equal(readFileSync(summary, "utf8"), `enable-auto-merge: ${why}; left unarmed: #1\nenable-auto-merge: ${why}; still armed, its disarm failed, so the hold is not enforced: #2\n`); +}); + +test("a held room in report-only names the armed pull request a disarm would have turned off, not as unarmed", async () => { + const armed = { enabledAt: "2026-10-23T10:00:00Z" }; + const summary = join(mkdtempSync(join(tmpdir(), "enable-auto-merge-summary-")), "summary.md"); + writeFileSync(summary, ""); + const lane = { ...ROOM, unreadable: "post-merge answer missing or unreadable" }; + const { lines, writes } = await run([pull(1), pull(2, { autoMergeRequest: armed })], { GITHUB_STEP_SUMMARY: summary, REPORT_ONLY: "true" }, {}, lane); + assert.deepEqual(writes, []); + const expected = "the lane's room could not be read (post-merge answer missing or unreadable); left unarmed: #1; report-only, a disarm would have run: #2"; + assert.deepEqual( + lines.filter((line) => line.startsWith("::warning") || line.startsWith("::error")), + [`::warning title=enable-auto-merge::${expected}`], + ); + assert.equal(readFileSync(summary, "utf8"), `enable-auto-merge: ${expected}\n`); +}); + // 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" }; diff --git a/.github/scripts/pr-merge/workflow.test.mjs b/.github/scripts/pr-merge/workflow.test.mjs index fce9696f03..191a58a8aa 100644 --- a/.github/scripts/pr-merge/workflow.test.mjs +++ b/.github/scripts/pr-merge/workflow.test.mjs @@ -218,6 +218,25 @@ test("enable-auto-merge reads the lane's room from this run's post-merge-dispatc assert.ok(names.indexOf("Read this run's post-merge report") < names.indexOf("Arm and disarm auto-merge")); }); +// A room that checks the post-merge answer again reads the lane records the +// last run kept, as post-merge-dispatch's check does, so the check answers +// them with no artifact or run read each; without them the check can run +// past the room's VERIFY_TIMEOUT_MS. +test("enable-auto-merge hands the room's post-merge check the lane records the last run kept", () => { + const arm = job("enable-auto-merge"); + const records = /- name: Read the lane records the last run kept\n([\s\S]*?)(?=\n {6}- name: )/.exec(arm)?.[1]; + assert.ok(records, "a lane-records read"); + assert.match(records, /\n {8}continue-on-error: true\n/); + assert.match(records, /\n {10}ACTIONS_TOKEN: \$\{\{ github\.token \}\}\n {10}LANE_RECORDS_PATH: \$\{\{ runner\.temp \}\}\/lane-records\.json\n/); + assert.match(records, /\n {8}run: node \.base\/\.github\/scripts\/pr-merge\/lane-records\.mjs read\n/); + const step = /- name: Arm and disarm auto-merge\n([\s\S]*?)(?=\n {6}- name: )/.exec(arm)[1]; + assert.match(step, /\n {10}BABYSIT_POSTMERGE_LANE_RECORDS: \$\{\{ runner\.temp \}\}\/lane-records\.json\n/); + const names = [...arm.matchAll(/\n {6}- name: (.*)/g)].map((m) => m[1]); + assert.ok(names.indexOf("Read the lane records the last run kept") < names.indexOf("Arm and disarm auto-merge")); + // Within the job's own permissions: the read needs actions: read alone. + assert.match(arm, /\n {6}actions: read #/); +}); + 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]); diff --git a/.github/workflows/pr-merge.yml b/.github/workflows/pr-merge.yml index 415e65bf31..10879ebb7b 100644 --- a/.github/workflows/pr-merge.yml +++ b/.github/workflows/pr-merge.yml @@ -1496,6 +1496,19 @@ jobs: name: pr-merge-post-merge-report path: ${{ runner.temp }}/post-merge + # The lane records earlier checks proved, as post-merge-dispatch reads + # them, for a room whose post-merge answer must be checked again: the + # check answers those records with no read. Any error leaves no file, + # and the check reads each record. Rules: lane-records.mjs. + - name: Read the lane records the last run kept + if: steps.lane-token.outputs.token != '' && (github.event_name == 'schedule' || github.event_name == 'workflow_dispatch') + continue-on-error: true + timeout-minutes: 2 + env: + ACTIONS_TOKEN: ${{ github.token }} + LANE_RECORDS_PATH: ${{ runner.temp }}/lane-records.json + run: node .base/.github/scripts/pr-merge/lane-records.mjs read + # While merge.report-only is anything but false it reports each # decision and writes nothing. - name: Arm and disarm auto-merge @@ -1514,6 +1527,8 @@ jobs: # 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 }} + # The lane records read above, for the room's post-merge check. + BABYSIT_POSTMERGE_LANE_RECORDS: ${{ runner.temp }}/lane-records.json # 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. diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index 912bf26096..bf9e5ac16b 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -1113,12 +1113,20 @@ 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 landed since that it does not count), it holds as a refusal does; one past the gate's +two-minute reuse age only because a lane merge is still unverified is kept while main's head, its +config commit, the demotion records and the unverified merges' full and canary runs are as they +were (no run reached a conclusion since, no merge is past the six-hour dispatch deadline, and no +quarantine that excused a failing suite has expired), and otherwise checked again as the gate does, +with the lane records the last run kept, holding only when that check cannot be read): 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. +as a refusal does. A held room raises one warning annotation and step-summary line naming why and +each lanes App pull request it left unarmed or, report-only, that a disarm would have turned off, +and one error annotation and summary line naming each one still armed because its disarm failed; +it posts no comment on the pull requests. 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 diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py index f84bd6be48..0864e1f4cd 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py @@ -3285,16 +3285,9 @@ def assess_unattended( the review evidence and the score. Raises `PolicyReadError` when the pull request or its pipeline config cannot be read.""" - def post_merge(floor: datetime, identity: str) -> merge_policy.PostMergeGate: - verdict = postmerge.check( - repo, - agent_identity=identity, - is_human=postmerge._default_is_human(repo, identity, self_logins), - merged_since=floor, - pinned_sha=postmerge.pin_reader(repo, identity), - lane_merges_only=authority == merge_policy.PR_MERGE, - ) - return postmerge.gate_from_report(verdict.report()) + post_merge = postmerge.gate_reader( + repo, self_logins, lane_merges_only=authority == merge_policy.PR_MERGE + ) def promotion(window_start: datetime | None) -> merge_policy.Promotion: return merge_policy.read_promotion( 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 db97518715..1e6b000243 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 @@ -3881,8 +3881,10 @@ class PostMergeGate: "config-unreadable", } ) -# Runner clocks may run ahead of GitHub's; `since` starts this much earlier. -SINCE_CLOCK_MARGIN = timedelta(minutes=5) +# Clock skew alone: a runner clock may run ahead of GitHub's, so `since` +# starts this much earlier. `computed_at` is stamped before the reads it +# covers, so no read time needs covering. +SINCE_CLOCK_MARGIN = timedelta(seconds=60) @dataclass(frozen=True) @@ -4503,18 +4505,87 @@ def report(self) -> dict[str, Any]: } +def _still_current( + repo: str, + default: str, + identity: str, + shared: SharedPostMerge, + runs_unchanged: Callable[[SharedPostMerge, str], bool], +) -> bool: + """Whether the post-merge check would answer `shared` again, in order: + the default branch head is its `main_sha`, the head's config commit is its + `config_sha` (with the demotion unchanged, the same floor), no demotion + record was filed or changed since (`_demotion_changed`), and + `runs_unchanged`: none of its unverified merges' verifying runs reached a + conclusion since. Raises `PolicyReadError`.""" + owner, name = _split_repo(repo) + _, head = _default_branch_head(repo, default) + return ( + head == shared.main_sha + and newest_config_commit(repo, head) == shared.config_sha + and not _demotion_changed(owner, name, shared, identity) + and runs_unchanged(shared, default) + ) + + +def _recompute_shared( + repo: str, + default: str, + identity: str, + self_logins: frozenset[str], + post_merge: Callable[[datetime, str], PostMergeGate], + now: datetime | None, +) -> SharedPostMerge | None: + """The run's shared answer read afresh at the default branch head, as + `assess` reads it for a pr-merge candidate: the head, its config commit, + the demotion, the floor, then the post-merge check, bound at a + `computed_at` taken before them. None when the check is not readable. + Raises `PolicyReadError`.""" + computed_at = now or datetime.now(UTC) + _, head = _default_branch_head(repo, default) + config_sha = newest_config_commit(repo, head) + demotion = read_demotion( + repo, + agent_identity=identity, + self_logins=self_logins, + permission=permission_reader(repo), + ) + floor = merged_since_floor(repo, head, demotion, PR_MERGE) + gate = post_merge(floor, identity) + if gate.report.get("readable") is not True: + return None + return SharedPostMerge( + main_sha=head, + config_sha=config_sha, + identity=identity, + self_logins=self_logins, + lane_merges_only=True, + computed_at=computed_at, + demotion=demotion, + floor=floor, + gate=gate, + ) + + def lane_room( repo: str, shared: SharedPostMerge | None, *, lane_rung_cap: str | None, now: datetime | None = None, + post_merge: Callable[[datetime, str], PostMergeGate] | None = None, + runs_unchanged: Callable[[SharedPostMerge, str], bool] | 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`. + `post_merge` is `assess`'s: an answer refused only as `unverified-stale` + is kept when nothing it was read from has moved (`_still_current`, with + `runs_unchanged(shared, branch)` for its unverified merges' runs), else + read again as the gate reads it (`_recompute_shared`), and a readable + one stands in for `shared`. - A config naming another authority stops (`enabled_authority`). - A post-merge refusal stops (`post-merge:`). @@ -4530,7 +4601,10 @@ def lane_room( 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.""" + since, or a lane merge landed since that it does not count. An + `unverified-stale` one is kept or read again instead, since a lane + merge's verifying run may have passed or failed since; a read that fails + or is not readable leaves it `unreadable`.""" if shared is None: return LaneRoom(unreadable="post-merge answer missing or unreadable") try: @@ -4558,6 +4632,25 @@ def lane_room( authority=PR_MERGE, now=now, ) + if unusable == "unverified-stale" and post_merge is not None: + # Kept when nothing it was read from moved, else read again. + try: + current = runs_unchanged is not None and _still_current( + repo, default, identity, shared, runs_unchanged + ) + except (RuntimeError, ValueError, OSError): + current = False + if not current: + try: + fresh = _recompute_shared( + repo, default, identity, shared.self_logins, post_merge, now + ) + except (RuntimeError, ValueError, OSError) as exc: + return LaneRoom(unreadable=f"post-merge answer {unusable}: {exc}") + if fresh is None: + return LaneRoom(unreadable=f"post-merge answer {unusable}: unreadable") + shared = fresh + unusable = None if unusable: return LaneRoom(unreadable=f"post-merge answer {unusable}") if shared.gate.refusal: diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py index 293f8dc5a4..50284dfd69 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py @@ -303,6 +303,8 @@ class PostMergeVerdict: # for a fresh run. hold: str | None = None recover_dispatch: bool = False + # When the earliest quarantine that excused a failing suite expires. + excuses_expire_at: datetime | None = None @property def ok(self) -> bool: @@ -370,6 +372,9 @@ def report(self) -> dict[str, Any]: "needsDispatch": self.needs_dispatch, "warnings": list(self.warnings), "branch": self.branch, + "excusesExpireAt": ( + self.excuses_expire_at.isoformat() if self.excuses_expire_at else None + ), } @@ -1070,6 +1075,9 @@ class _Context: quarantine_issues: dict[str, dict[str, Any]] | None = None # Excused failing suites, by (suite, sighting). quarantines: dict[tuple[str, str], Quarantine] = field(default_factory=dict) + # The earliest time a quarantine that excused a failing suite expires, + # after which the same runs judge differently (`excuse_until`). + excuses_expire_at: datetime | None = None # The branch's dispatch runs at each commit, read by SHA. dispatch_at_cache: dict[str, dict[int, Run]] = field(default_factory=dict) # The lane records file (`LANE_RECORDS_ENV`), its records once loaded, @@ -1164,16 +1172,18 @@ def expired(self, run: Run, attempt: int, checks: frozenset[str]) -> frozenset[s return frozenset() issues = self._quarantine_issues() days = self._quarantine_days(run.head_sha) - return frozenset( - check - for check, suites in named.items() - if any( - not is_gate_suite(suite) - and (issue := issues.get(suite)) is not None - and self.now >= _time(issue.get("created_at")) + timedelta(days=days) - for suite in suites - ) - ) + expired: set[str] = set() + for check, suites in named.items(): + for suite in suites: + issue = None if is_gate_suite(suite) else issues.get(suite) + if issue is None: + continue + ends = _time(issue.get("created_at")) + timedelta(days=days) + if self.now >= ends: + expired.add(check) + else: + self.excuse_until(ends) + return frozenset(expired) def demoting( self, run: Run, attempt: int, checks: frozenset[str] @@ -1205,17 +1215,27 @@ def demoting( demote.add(check) continue issue = issues.get(suite) - if issue is not None and self.now >= _time( - issue.get("created_at") - ) + timedelta(days=days): + ends = ( + None + if issue is None + else _time(issue.get("created_at")) + timedelta(days=days) + ) + if ends is not None and self.now >= ends: demote.add(check) else: + if ends is not None: + self.excuse_until(ends) excused.append(Quarantine(suite, sighting, run.head_sha)) for q in excused: self.quarantines[(q.suite, q.run_url)] = q self.demoting_cache[key] = frozenset(demote) return self.demoting_cache[key] + def excuse_until(self, ends: datetime) -> None: + """Note a quarantine excuse that holds until `ends`.""" + if self.excuses_expire_at is None or ends < self.excuses_expire_at: + self.excuses_expire_at = ends + def failing_suites( self, run_id: int, attempt: int, check: str ) -> frozenset[str] | None: @@ -2182,6 +2202,7 @@ def lane_prs_at(sha: str) -> str | None: warnings=tuple(warnings), branch=branch, quarantines=tuple(ctx.quarantines.values()), + excuses_expire_at=ctx.excuses_expire_at, ) @@ -3097,6 +3118,92 @@ def gate_from_report(report: dict[str, Any]) -> policy.PostMergeGate: ) +def gate_reader( + repo: str, self_logins: Collection[str], *, lane_merges_only: bool +) -> Callable[[datetime, str], policy.PostMergeGate]: + """The merge gate's `post_merge(floor, identity)`: this check from + `floor`, as `gate_from_report` reads it.""" + + def post_merge(floor: datetime, identity: str) -> policy.PostMergeGate: + verdict = check( + repo, + agent_identity=identity, + is_human=_default_is_human(repo, identity, self_logins), + merged_since=floor, + pinned_sha=pin_reader(repo, identity), + lane_merges_only=lane_merges_only, + ) + return gate_from_report(verdict.report()) + + return post_merge + + +VERIFYING_WORKFLOWS = frozenset( + f".github/workflows/{w}" for w in (FULL_RUN_WORKFLOW, CANARY_WORKFLOW) +) + + +def pending_runs_unchanged( + repo: str, + shared: policy.SharedPostMerge, + branch: str, + *, + now: datetime | None = None, +) -> bool: + """Whether `check` would judge `shared`'s unverified merges as it did, + from one run listing: no full or canary run on `branch` created since the + earliest of them completed or was re-run since `computed_at` (less + `policy.SINCE_CLOCK_MARGIN`), none has passed `DISPATCH_DEADLINE`, after + which a missing dispatch or pin is a failure, and no quarantine that + excused a failing suite has expired (`excusesExpireAt`; an answer + without the field is False). Those are `_judge`'s time-based excuses. A + run started since and still running only stays pending. A listing that + cannot be read or is not whole, or an entry without a merge time, is + False.""" + pending = [ + m + for m in map(json_object, json_array(shared.gate.report.get("autoMerges"))) + if m.get("verified") is not True + ] + report = shared.gate.report + if not pending or "excusesExpireAt" not in report: + return False + now = now or datetime.now(UTC) + try: + earliest = min(_time(m.get("mergedAt")) for m in pending) + excuse = report["excusesExpireAt"] + if now > earliest + DISPATCH_DEADLINE or ( + excuse is not None and now >= _time(excuse) + ): + return False + owner, name = _split_repo(repo) + created = quote( + f">={earliest.astimezone(UTC).strftime('%Y-%m-%dT%H:%M:%SZ')}", safe="" + ) + payload = json_object( + _read( + [ + "api", + f"repos/{owner}/{name}/actions/runs?branch={quote(branch, safe='')}" + f"&created={created}&per_page=100", + ], + "verifying runs", + ) + ) + runs = json_array(payload.get("workflow_runs")) + if payload.get("total_count") != len(runs): + return False + since = shared.computed_at - policy.SINCE_CLOCK_MARGIN + return not any( + run.get("path") in VERIFYING_WORKFLOWS + and run.get("status") == "completed" + and _time(run.get("updated_at")) >= since + for run in map(json_object, runs) + ) + except PostMergeReadError: + return False + + def read_shared(path: str) -> policy.SharedPostMerge | None: """The run's shared answer from this script's own report at `path`, or None when it is missing, malformed, unreadable as a verdict, or has no 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 0c4725cc5e..3555b90074 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 @@ -1548,7 +1548,9 @@ def main(argv: list[str] | None = None, *, now: datetime | None = None) -> int: 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).""" + the workflow read (empty is no cap; missing holds the rung off). A report + refused only as `unverified-stale` is kept while nothing it was read from + has moved, else checked again.""" parser = argparse.ArgumentParser( prog="babysit_verify_evidence.py", allow_abbrev=False ) @@ -1570,7 +1572,17 @@ def main(argv: list[str] | None = None, *, now: datetime | None = None) -> int: 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) + answer = merge_policy.lane_room( + args.repo, + shared, + lane_rung_cap=args.rung_cap, + # The gate's recompute, bound as the room binds, and the one run + # listing that spares it while the pending merges' runs hold. + post_merge=postmerge.gate_reader(args.repo, (), lane_merges_only=True), + runs_unchanged=lambda answer, branch: postmerge.pending_runs_unchanged( + args.repo, answer, branch + ), + ) print(json.dumps(answer.report())) return 0 if ( 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 index 581849973b..1e33fff1fc 100644 --- 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 @@ -22,6 +22,7 @@ sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent.parent)) import babysit_merge_policy as policy # noqa: E402 +import babysit_postmerge as pm # noqa: E402 import babysit_verify_evidence as ve # noqa: E402 REPO = "example-org/example-repo" @@ -30,6 +31,20 @@ 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) +# The default branch head and floor a recompute reads. +NEW_MAIN = "e" * 40 +NEW_FLOOR = FLOOR + timedelta(minutes=1) +# A passing answer with lane merge #5 still unverified, read three minutes on: +# past the two minutes the gate reuses it. +PENDING = { + "readable": True, + "ok": True, + "autoMerges": [ + {"number": 5, "verified": False, "mergedAt": "2026-10-01T00:00:00Z"} + ], + "excusesExpireAt": None, +} +STALE_NOW = FLOOR + timedelta(minutes=3) def config(**overrides: object) -> policy.PipelineConfig: @@ -47,7 +62,11 @@ def config(**overrides: object) -> policy.PipelineConfig: def shared( - *, demoted: bool = False, refusal: str | None = None, identity: str = IDENTITY + *, + demoted: bool = False, + refusal: str | None = None, + identity: str = IDENTITY, + report: dict[str, object] | None = None, ) -> policy.SharedPostMerge: return policy.SharedPostMerge( main_sha=MAIN, @@ -60,10 +79,40 @@ def shared( demoted, "record-open" if demoted else None, (7,) if demoted else () ), floor=FLOOR, - gate=policy.PostMergeGate(refusal, {}, lambda merge: True), + gate=policy.PostMergeGate(refusal, report or {}, lambda merge: True), ) +class Recheck: + """A fake post-merge check: `gate` or `error`, with each floor and + identity it was asked for.""" + + def __init__( + self, gate: policy.PostMergeGate | None = None, error: Exception | None = None + ) -> None: + self.gate, self.error = gate, error + self.asked: list[tuple[datetime, str]] = [] + + def post_merge(self, floor: datetime, identity: str) -> policy.PostMergeGate: + self.asked.append((floor, identity)) + if self.error is not None: + raise self.error + assert self.gate is not None + return self.gate + + +def run( + workflow: str, *, status: str, updated: datetime, created: datetime = FLOOR +) -> dict[str, object]: + """One row of the actions runs listing.""" + return { + "path": f".github/workflows/{workflow}", + "status": status, + "created_at": created.isoformat(), + "updated_at": updated.isoformat(), + } + + class LaneRoom(unittest.TestCase): def room( self, @@ -75,6 +124,13 @@ def room( config_error: Exception | None = None, now: datetime = NOW, demotion_changed: bool = False, + post_merge: object = None, + runs_unchanged: object = None, + head: str = NEW_MAIN, + config_commit: str = MAIN, + fresh_floor: datetime = NEW_FLOOR, + head_error: Exception | None = None, + fresh_demotion: policy.Demotion | None = None, ) -> policy.LaneRoom: seen: dict[str, object] = {} @@ -99,14 +155,35 @@ def in_flight(repo: str, **kwargs: object) -> policy.InFlight: ), # 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, "newest_config_commit", return_value=config_commit + ), mock.patch.object(policy, "_get", return_value=(200, {"sha": MAIN})), + # The recompute's reads at the default branch head: a newer head, + # no demotion record, and a later floor than the old answer's. + mock.patch.object( + policy, + "_default_branch_head", + side_effect=head_error, + return_value=("main", head), + ), + mock.patch.object(policy, "permission_reader"), + mock.patch.object( + policy, + "read_demotion", + return_value=fresh_demotion or policy.Demotion(False, None, ()), + ), + mock.patch.object(policy, "merged_since_floor", return_value=fresh_floor), ): + kwargs = {"post_merge": post_merge} if post_merge is not None else {} + if runs_unchanged is not None: + kwargs["runs_unchanged"] = runs_unchanged result = policy.lane_room( REPO, answer if answer is not None else shared(), lane_rung_cap=cap, now=now, + **kwargs, # type: ignore[arg-type] ) self.seen = seen return result @@ -168,14 +245,25 @@ def test_an_answer_the_gate_would_not_reuse_is_unreadable(self) -> None: 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), + # Inside the two minutes: past them the answer is read again, + # demotion included. + "demotion changed": dict( + demotion_changed=True, now=FLOOR + timedelta(minutes=1) + ), } + recheck = Recheck(policy.PostMergeGate(None, {"readable": True}, None)) for name, kwargs in cases.items(): with self.subTest(name): - room = self.room(**kwargs) # type: ignore[arg-type] + room = self.room( + shared(report=PENDING), + post_merge=recheck.post_merge, + **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") + # Only an unverified-stale answer is read again. + self.assertEqual(recheck.asked, []) 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 @@ -187,13 +275,15 @@ def refusal(repo: str, answer: object, **kwargs: object) -> str: seen.append(kwargs) return "new-merge:#7" + recheck = Recheck(policy.PostMergeGate(None, {"readable": True}, None)) with mock.patch.object(policy, "shared_refusal", side_effect=refusal): - room = self.room() + room = self.room(post_merge=recheck.post_merge) 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") + self.assertEqual(recheck.asked, []) # Bound as the gate binds a candidate at the default branch head. self.assertEqual( { @@ -210,6 +300,202 @@ def refusal(repo: str, answer: object, **kwargs: object) -> str: ) self.assertEqual(seen[0]["self_logins"], frozenset()) + def test_a_pending_merge_past_two_minutes_is_read_again_and_counted( + self, + ) -> None: + # The post-merge step takes nearly the two minutes the gate reuses an + # answer for, so a pending lane merge must not leave the room unreadable. + verified = { + "readable": True, + "ok": True, + "autoMerges": [{"number": 5, "verified": True}], + } + fresh = policy.PostMergeGate(None, verified, lambda merge: True) + recheck = Recheck(fresh) + room = self.room( + shared(report=PENDING), + now=STALE_NOW, + post_merge=recheck.post_merge, + ) + self.assertEqual( + (room.refusal, room.unreadable, room.count, room.cap, room.rung), + (None, None, 2, 5, "C2"), + ) + # The check ran from the floor read again, as the identity. + self.assertEqual(recheck.asked, [(NEW_FLOOR, IDENTITY)]) + # The in-flight count judges merges by the fresh answer, from its floor. + self.assertIs(self.seen["verified"], fresh.verified) + self.assertEqual(self.seen["merged_since"], NEW_FLOOR) + + def test_a_recompute_that_finds_a_failure_refuses(self) -> None: + # A verification failure after computedAt still holds the lane off. + failed = { + "readable": True, + "ok": False, + "failures": [{"signal": "red-main", "number": 5}], + } + recheck = Recheck(policy.PostMergeGate("failure:red-main#5", failed, None)) + room = self.room( + shared(report=PENDING), + now=STALE_NOW, + post_merge=recheck.post_merge, + ) + self.assertEqual( + (room.refusal, room.unreadable), ("post-merge:failure:red-main#5", None) + ) + self.assertFalse(self.seen, "no in-flight read after a refusal") + + def test_a_demotion_record_filed_since_holds_a_recomputed_room_off( + self, + ) -> None: + # Past two minutes the age refuses before the demotion is re-checked, + # so the recompute reads the demotion again. + recheck = Recheck(policy.PostMergeGate(None, PENDING, lambda merge: False)) + room = self.room( + shared(report=PENDING), + now=STALE_NOW, + post_merge=recheck.post_merge, + fresh_demotion=policy.Demotion(True, "record-open", (8,)), + ) + self.assertEqual(room.refusal, "rung-off:demotion") + + def test_a_failed_recompute_leaves_the_room_unreadable(self) -> None: + unreadable = {"readable": False, "ok": False, "reason": "runs-unreadable"} + cases = { + "check raises": Recheck(error=RuntimeError("runs 502")), + "check unreadable": Recheck( + policy.PostMergeGate("runs-unreadable", unreadable, None) + ), + } + for name, recheck in cases.items(): + with self.subTest(name): + room = self.room( + shared(report=PENDING), + now=STALE_NOW, + post_merge=recheck.post_merge, + ) + self.assertIsNone(room.refusal) + self.assertTrue( + str(room.unreadable).startswith( + "post-merge answer unverified-stale" + ), + room.unreadable, + ) + # A head that cannot be read is a failed recompute too. + recheck = Recheck(policy.PostMergeGate(None, {"readable": True}, None)) + room = self.room( + shared(report=PENDING), + now=STALE_NOW, + post_merge=recheck.post_merge, + head_error=policy.PolicyReadError("head 502"), + ) + self.assertIsNone(room.refusal) + self.assertTrue(room.unreadable) + self.assertEqual(recheck.asked, []) + # With no check to run, it stays unreadable as before. + room = self.room(shared(report=PENDING), now=STALE_NOW) + self.assertEqual(room.unreadable, "post-merge answer unverified-stale") + self.assertFalse(self.seen, "no in-flight read after an unusable answer") + + def current(self, **kwargs: object) -> tuple[policy.LaneRoom, Recheck, list]: + """The room for a pending answer read three minutes on, with the + default branch head, config commit and demotion all as it was read + unless `kwargs` moves one; `runs` is what the run check answers.""" + runs = kwargs.pop("runs", True) + asked: list[tuple[object, str]] = [] + + def runs_unchanged(answer: object, branch: str) -> bool: + asked.append((answer, branch)) + return runs(answer, branch) if callable(runs) else bool(runs) + + recheck = Recheck( + kwargs.pop("gate", policy.PostMergeGate(None, PENDING, lambda merge: False)) + ) + room = self.room( + kwargs.pop("answer", shared(report=PENDING)), + now=STALE_NOW, + post_merge=recheck.post_merge, + runs_unchanged=runs_unchanged, + **{"head": MAIN, **kwargs}, # type: ignore[arg-type] + ) + return room, recheck, asked + + def test_an_answer_nothing_has_moved_since_is_kept_without_a_check( + self, + ) -> None: + answer = shared(report=PENDING) + room, recheck, asked = self.current(answer=answer) + self.assertEqual( + (room.refusal, room.unreadable, room.count, room.cap), (None, None, 2, 5) + ) + self.assertEqual(recheck.asked, [], "no full post-merge check") + self.assertEqual(asked, [(answer, "main")]) + # Counted from the kept answer: its floor and its verified merges. + self.assertEqual(self.seen["merged_since"], FLOOR) + self.assertIs(self.seen["verified"], answer.gate.verified) + + def test_anything_moved_since_reads_the_answer_again(self) -> None: + cases = { + "(a) main's head moved": dict(head=NEW_MAIN), + # The same floor, so the gate's reuse rule does not refuse first. + "(b) the config commit moved": dict( + config_commit="f" * 40, fresh_floor=FLOOR + ), + "(c) a demotion record changed": dict(demotion_changed=True), + "(d) a verifying run reached a conclusion": dict(runs=False), + } + for name, kwargs in cases.items(): + with self.subTest(name): + room, recheck, _ = self.current(**kwargs) + self.assertEqual(len(recheck.asked), 1, "one full post-merge check") + self.assertEqual((room.refusal, room.unreadable), (None, None)) + + def test_a_quarantine_that_expired_since_reads_the_answer_again(self) -> None: + # (e): the answer excused a failing suite under a quarantine that + # expired a minute after it was read, so the same red run now demotes. + report = { + **PENDING, + "excusesExpireAt": (FLOOR + timedelta(minutes=1)).isoformat(), + } + listing = {"total_count": 0, "workflow_runs": []} + with mock.patch.object(pm, "_read", return_value=listing): + room, recheck, _ = self.current( + answer=shared(report=report), + runs=lambda answer, branch: pm.pending_runs_unchanged( + REPO, answer, branch, now=STALE_NOW + ), + ) + self.assertEqual(len(recheck.asked), 1, "one full post-merge check") + self.assertEqual((room.refusal, room.unreadable), (None, None)) + + def test_a_pending_dispatch_run_that_completed_red_refuses(self) -> None: + # The case the two-minute rule exists for: main's head has not moved, + # but merge #5's dispatch run finished red after the answer was read. + listing = { + "total_count": 1, + "workflow_runs": [ + run( + "pr-require-checks.yml", + status="completed", + updated=STALE_NOW - timedelta(seconds=30), + ) + ], + } + failed = { + "readable": True, + "ok": False, + "failures": [{"signal": "dispatch", "number": 5}], + } + with mock.patch.object(pm, "_read", return_value=listing): + room, recheck, _ = self.current( + runs=lambda answer, branch: pm.pending_runs_unchanged( + REPO, answer, branch, now=STALE_NOW + ), + gate=policy.PostMergeGate("failure:dispatch#5", failed, None), + ) + self.assertEqual(len(recheck.asked), 1) + self.assertEqual(room.refusal, "post-merge:failure:dispatch#5") + 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) @@ -240,14 +526,120 @@ def test_a_failed_read_refuses_nothing_and_allows_nothing(self) -> None: self.assertTrue(room.unreadable) +class PendingRunsUnchanged(unittest.TestCase): + """(d): the verifying runs of the answer's unverified merges, from one + listing. Expected from the check's own rule (`_judge`): a full or canary + run that reaches a conclusion changes the verdict, one still running + leaves it pending, and past `DISPATCH_DEADLINE` a merge with no dispatch + run fails.""" + + def unchanged( + self, + rows: list[dict[str, object]], + *, + total: int | None = None, + now=STALE_NOW, + report: dict[str, object] = PENDING, + ) -> tuple[bool, list[list[str]]]: + calls: list[list[str]] = [] + + def read(args: list[str], label: str) -> dict[str, object]: + calls.append(args) + return { + "total_count": len(rows) if total is None else total, + "workflow_runs": rows, + } + + with mock.patch.object(pm, "_read", side_effect=read): + answer = pm.pending_runs_unchanged( + REPO, shared(report=report), "main", now=now + ) + return answer, calls + + def test_runs_still_running_or_finished_before_the_answer_hold(self) -> None: + rows = [ + run("pr-require-checks.yml", status="in_progress", updated=STALE_NOW), + run( + "post-merge-detect-silent-revert.yml", + status="queued", + updated=STALE_NOW, + ), + # Finished before the answer was read (less the clock margin). + run( + "post-merge-detect-silent-revert.yml", + status="completed", + updated=FLOOR - policy.SINCE_CLOCK_MARGIN - timedelta(seconds=1), + ), + # Another workflow's run judges nothing. + run("ci-status.yml", status="completed", updated=STALE_NOW), + ] + answer, calls = self.unchanged(rows) + self.assertTrue(answer) + # One listing call, of runs on main created since merge #5. + self.assertEqual(len(calls), 1) + self.assertIn("branch=main", calls[0][1]) + self.assertIn("created=%3E%3D2026-10-01T00%3A00%3A00Z", calls[0][1]) + + def test_the_clock_margin_is_sixty_seconds_of_skew(self) -> None: + # computedAt is stamped before the runs are read, so the margin covers + # only clock skew: 60 s. + def finished(before: timedelta) -> dict[str, object]: + return run( + "pr-require-checks.yml", status="completed", updated=FLOOR - before + ) + + self.assertFalse(self.unchanged([finished(timedelta(seconds=59))])[0]) + self.assertTrue(self.unchanged([finished(timedelta(seconds=61))])[0]) + + def test_a_conclusion_since_a_partial_listing_or_the_deadline_moves_it( + self, + ) -> None: + done = run("pr-require-checks.yml", status="completed", updated=STALE_NOW) + canary = run( + "post-merge-detect-silent-revert.yml", status="completed", updated=FLOOR + ) + self.assertFalse(self.unchanged([done])[0]) + self.assertFalse(self.unchanged([canary])[0]) + self.assertFalse(self.unchanged([], total=101)[0]) + # A quarantine excuse that ended since, and an answer that does not + # say when its excuses end. + self.assertFalse( + self.unchanged( + [], report={**PENDING, "excusesExpireAt": FLOOR.isoformat()} + )[0] + ) + self.assertFalse( + self.unchanged( + [], report={k: v for k, v in PENDING.items() if k != "excusesExpireAt"} + )[0] + ) + later = (STALE_NOW + timedelta(days=1)).isoformat() + self.assertTrue( + self.unchanged([], report={**PENDING, "excusesExpireAt": later})[0] + ) + self.assertFalse( + self.unchanged([], now=FLOOR + pm.DISPATCH_DEADLINE + timedelta(seconds=1))[ + 0 + ] + ) + + class LaneRoomCommand(unittest.TestCase): def test_it_prints_the_room_from_the_post_merge_report(self) -> None: seen: list[object] = [] + hooks: dict[str, object] = {} + def room( - repo: str, answer: object, *, lane_rung_cap: str | None + repo: str, + answer: object, + *, + lane_rung_cap: str | None, + post_merge: object, + runs_unchanged: object, ) -> policy.LaneRoom: seen.append((repo, answer, lane_rung_cap)) + hooks.update(post_merge=post_merge, runs_unchanged=runs_unchanged) return policy.LaneRoom(count=1, cap=5, queued=frozenset({3}), rung="C2") out = io.StringIO() @@ -283,6 +675,8 @@ def room( "rung": "C2", }, ) + self.assertTrue(callable(hooks["post_merge"])) + self.assertTrue(callable(hooks["runs_unchanged"])) def test_a_missing_report_is_unreadable(self) -> None: out = io.StringIO() diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_postmerge_quarantine.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_postmerge_quarantine.py index 04efd65b96..3f8e2ca8cb 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_postmerge_quarantine.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_postmerge_quarantine.py @@ -275,6 +275,29 @@ def test_an_open_quarantine_takes_an_out_of_set_sighting(self) -> None: (post,) = self.posts() self.assertIn(f"body={pm.QUARANTINE_NOTE}{SIGHTING}", post) + def test_the_answer_says_when_its_quarantine_excuse_ends(self) -> None: + # The lane room keeps an answer only until then: the quarantine + # issue's creation plus the default 7 quarantine days. + self.quarantine_issue(FLAKY, "2026-10-10T09:00:00Z") + self.red_dispatch(bash_log((FLAKY, False))) + report = self.check().report() + self.assertEqual(report["failures"], []) + self.assertEqual(report["excusesExpireAt"], "2026-10-17T09:00:00+00:00") + # A baseline failure excused by a live quarantine ends the same way. + self.gh.issues.clear() + self.quarantine_issue(FLAKY, "2026-10-10T10:00:00Z") + self.red_baseline() + report = self.check().report() + self.assertEqual(report["failures"], []) + self.assertEqual(report["excusesExpireAt"], "2026-10-17T10:00:00+00:00") + + def test_an_answer_nothing_excuses_has_no_excuse_end(self) -> None: + # A suite with no quarantine issue yet is excused with no end. + self.red_dispatch(bash_log((FLAKY, False))) + report = self.check().report() + self.assertEqual(report["failures"], []) + self.assertIsNone(report["excusesExpireAt"]) + def test_the_selector_scripts_are_gate_suites(self) -> None: for suite in ( "scripts/plan-test-lanes.test.sh",