From c482234897d151ed56dcb557bc71e4130799d544 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 27 Oct 2026 13:54:07 -0400 Subject: [PATCH 1/5] perf(pr-merge): batch main's whole-suite runs and stop sleeping after the dispatch dispatch-full-run and post-merge-dispatch read main's newest pr-require-checks dispatches first and start none within 30 minutes of the last; post-merge-dispatch starts main's tip on its next scheduled run when no run covers it, for any merge and whatever the kill switch says. dispatch-full-run ends after it dispatches; post-merge-dispatch sends a red full run to post-merge-demotion-alert.yml once per run. pr-require-checks.yml gives attempt 1 of a main dispatch one concurrency group with no queue key and cancel-in-progress false; a re-run keeps its own group. The post-merge check and the alert skip a run that group replaced before it started (cancelled, no job). Co-Authored-By: Claude Opus 5.5 --- ...perf-main-whole-suite-batching-9548bbe5.md | 10 + ...perf-main-whole-suite-batching-3c4daaf0.md | 7 + .github/scripts/pr-merge/merge-job.test.mjs | 246 +++++++++++- .github/scripts/pr-merge/workflow.test.mjs | 351 ++++++++++-------- .../workflows/post-merge-demotion-alert.yml | 24 +- .github/workflows/pr-merge.yml | 313 +++++++++++----- .github/workflows/pr-require-checks.yml | 31 +- docs/conventions/pr-pipeline/README.md | 7 +- .../babysit-prs/scripts/babysit_postmerge.py | 42 ++- .../scripts/tests/test_babysit_postmerge.py | 67 ++++ scripts/post-merge-demotion-alert.test.sh | 12 + 11 files changed, 839 insertions(+), 271 deletions(-) create mode 100644 .changes/pr-pipeline/perf-main-whole-suite-batching-9548bbe5.md create mode 100644 .changes/source-control/perf-main-whole-suite-batching-3c4daaf0.md diff --git a/.changes/pr-pipeline/perf-main-whole-suite-batching-9548bbe5.md b/.changes/pr-pipeline/perf-main-whole-suite-batching-9548bbe5.md new file mode 100644 index 0000000000..ecb602a381 --- /dev/null +++ b/.changes/pr-pipeline/perf-main-whole-suite-batching-9548bbe5.md @@ -0,0 +1,10 @@ +--- +bump: patch +--- + +### Changed + +- pr-merge starts at most one full run of main per 30 minutes, of main's tip, covering every merge since the last. dispatch-full-run and post-merge-dispatch read the newest pr-require-checks.yml dispatches on main first (`Read main's full-run cover`): within the cap they start nothing, and post-merge-dispatch starts one on its next scheduled run for a tip no full run covers, whoever merged and whatever the kill switch says (`.github/workflows/pr-merge.yml`). +- dispatch-full-run ends once it dispatches, where it held a runner up to 39 minutes polling the run; its timeout drops from 45 to 10 minutes. post-merge-dispatch now sends a red full run to post-merge-demotion-alert.yml on its next scheduled run, up to 15 minutes after the run ends, once per run (`pr-merge-alerted-`), and skips a run that never started. +- pr-require-checks.yml's dispatches on main (attempt 1) share one concurrency group with no `queue` key and `cancel-in-progress` false: one runs, one waits, and a newer one replaces the waiting one. A re-run and every other run keep their own group and cancel. +- post-merge-demotion-alert.yml files nothing for a cancelled full run with no job, one the concurrency group replaced before it started. diff --git a/.changes/source-control/perf-main-whole-suite-batching-3c4daaf0.md b/.changes/source-control/perf-main-whole-suite-batching-3c4daaf0.md new file mode 100644 index 0000000000..2e6b5d7502 --- /dev/null +++ b/.changes/source-control/perf-main-whole-suite-batching-3c4daaf0.md @@ -0,0 +1,7 @@ +--- +bump: patch +--- + +### Fixed + +- The post-merge check (`babysit_postmerge.py`) skips a full run cancelled with no job on attempt 1, one pr-require-checks.yml's main-dispatch concurrency group replaced before it started: it no longer holds a merge as attribution-pending, asks for its re-run, or serves as the baseline of a later red run. A run cancelled after its jobs started still reads red. diff --git a/.github/scripts/pr-merge/merge-job.test.mjs b/.github/scripts/pr-merge/merge-job.test.mjs index 38845adbf7..294ee72f6d 100644 --- a/.github/scripts/pr-merge/merge-job.test.mjs +++ b/.github/scripts/pr-merge/merge-job.test.mjs @@ -53,10 +53,13 @@ test("the re-dispatch for earlier lane merges waits on neither the listing nor t "Read the lane records the last run kept", "Check earlier lane merges", "Keep the post-merge report for enable-auto-merge", + "Read main's full-run cover", "Start the full CI run on main", "Record the dispatched run", "Re-run a red lane-dispatched full run", "Record the re-run", + "Send a red full run to the demotion alert", + "Record the alert sent", ], ); assert.doesNotMatch(early, /steps\.candidates|steps\.gate|needs\.merge/); @@ -72,7 +75,7 @@ test("the early re-dispatch keeps the trusted mechanics and skips untrusted even assert.match(condition, /\(github\.event_name == 'schedule' \|\| github\.event_name == 'workflow_dispatch'\)/); const list = stepsOf(early); const dispatch = step("Start the full CI run on main", list); - assert.match(dispatch, /if: steps\.post-merge\.outputs\.needs-dispatch == 'true'/); + assert.match(dispatch, /steps\.post-merge\.outputs\.needs-dispatch == 'true'/); assert.match(dispatch, /actions\/workflows\/pr-require-checks\.yml\/dispatches"[\s\S]*-f ref=main /); const upload = step("Record the dispatched run", list); assert.match(upload, /uses: actions\/upload-artifact@/); @@ -1463,9 +1466,9 @@ test("a rate-limited first read stops the step red before the re-run, naming the assert.match(run.output, /^stop=the core pool until 2100-01-01T00:00:00Z$/m); }); -test("post-merge-dispatch's rate-limited output also carries the re-run step's stop", () => { +test("post-merge-dispatch's rate-limited output also carries the re-run and alert steps' stops", () => { const header = `${jobBlock("post-merge-dispatch").split("\n steps:\n")[0]}\n`; - assert.match(header, /\n {6}rate-limited: \$\{\{ steps\.post-merge\.outputs\.rate-limited \|\| steps\.dispatch\.outputs\.stop \|\| steps\.rerun\.outputs\.stop \}\}\n/); + assert.match(header, /\n {6}rate-limited: \$\{\{ steps\.post-merge\.outputs\.rate-limited \|\| steps\.dispatch\.outputs\.stop \|\| steps\.rerun\.outputs\.stop \|\| steps\.alert\.outputs\.stop \}\}\n/); }); test("the re-run step refuses a run id that is not a number", () => { @@ -1490,6 +1493,243 @@ test("the re-run record is named by run and attempt, uploaded only after a recor assert.match(upload, /\n {10}name: pr-merge-rerun-\$\{\{ steps\.post-merge\.outputs\.lane-rerun \}\}-attempt-2\n/); }); +// The alert send's step, run with .base at this checkout (the rate guard) +// against a fake gh that answers, through the guard's -i form and the step's +// --jq filter, a listing of pr-require-checks.yml dispatches on main (`runs`: +// {id, attempt, status, conclusion, sha, actor}), each later attempt's +// attempt 1 (`firsts` by id), each run's attempt-1 job count (`jobs` by id, +// default 3) and its pr-merge-alerted- record count (`sent` by id, +// default 0). A read whose answer is "x" fails with a 502, "limited" with +// the rate-limited 403. `post` answers each send in turn: "ok", "unsent" (a +// TLS handshake timeout, before the request left), "502" or "limited" (a +// 429). The send replaces dispatch-full-run's wait step (efficiency plan item +// 7(b)): GITHUB_TOKEN started the full run, so it raises no workflow_run +// event, and post-merge-demotion-alert.yml hears of a red one only from here. +const ALERT_SHA = "0123456789abcdef0123456789abcdef01234567"; +const alertSend = (id, sha = ALERT_SHA) => + `api -X POST repos/o/r/actions/workflows/post-merge-demotion-alert.yml/dispatches -f ref=main -f inputs[run-id]=${id} -f inputs[head-sha]=${sha}`; +const runAlert = ({ runs = [], firsts = {}, jobs = {}, sent = {}, post = ["ok"], listing } = {}) => { + const dir = mkdtempSync(join(tmpdir(), "pr-merge-alert-")); + try { + const bin = join(dir, "bin"); + mkdirSync(bin); + const fixture = (name, value) => writeFileSync(join(dir, name), typeof value === "string" ? value : JSON.stringify(value)); + fixture( + "listing", + listing ?? + { + workflow_runs: runs.map((r) => ({ + id: r.id, + run_attempt: r.attempt ?? 1, + status: r.status ?? "completed", + conclusion: r.conclusion ?? null, + head_sha: r.sha ?? ALERT_SHA, + head_branch: "main", + actor: { login: r.actor ?? "github-actions[bot]" }, + })), + }, + ); + for (const [id, a] of Object.entries(firsts)) fixture(`first.${id}`, a === "x" || a === "limited" ? a : { status: "completed", ...a }); + for (const r of runs) { + const n = jobs[r.id] ?? 3; + fixture(`jobs.${r.id}`, n === "x" || n === "limited" ? n : { total_count: n, jobs: [] }); + const s = sent[r.id] ?? 0; + fixture(`sent.${r.id}`, s === "x" || s === "limited" ? s : { total_count: s, artifacts: [] }); + } + writeFileSync( + join(bin, "gh"), + `#!/usr/bin/env bash +inc=; args=() +for a in "$@"; do if [ "$a" = -i ]; then inc=1; else args+=("$a"); fi; done +set -- "\${args[@]}" +echo "$*" >>"${dir}/calls" +filter=.; for ((i = 1; i <= $#; i++)); do if [ "\${!i}" = --jq ]; then j=$((i + 1)); filter="\${!j}"; fi; done +endpoint=$(printf '%s\\n' "$@" | grep -m1 '^repos/') +head() { [ -z "$inc" ] || printf "$1"; } +if [ "$2" = -X ]; then + p=(${[post].flat().join(" ")}); s=$(($(cat "${dir}/posts" 2>/dev/null || echo 0) + 1)); echo "$s" >"${dir}/posts" + case "\${p[$((s - 1))]:-\${p[-1]}}" in + ok) head 'HTTP/2.0 204 No Content\\r\\n\\r\\n'; exit 0 ;; + unsent) echo 'Post "https://api.github.com/repos/o/r/actions/workflows/post-merge-demotion-alert.yml/dispatches": net/http: TLS handshake timeout' >&2; exit 1 ;; + 502) head 'HTTP/2.0 502 Bad Gateway\\r\\n\\r\\n{}\\n'; echo "gh: Server Error (HTTP 502)" >&2; exit 1 ;; + limited) head 'HTTP/2.0 429 Too Many Requests\\r\\nRetry-After: 60\\r\\n\\r\\n{"message": "Too many requests"}\\n'; echo "gh: Too Many Requests (HTTP 429)" >&2; exit 1 ;; + esac +fi +case "$endpoint" in + */workflows/pr-require-checks.yml/runs\\?*) f=listing ;; + */attempts/1/jobs*) f=jobs.$(sed -E 's#.*/runs/([0-9]+)/.*#\\1#' <<<"$endpoint") ;; + */attempts/1) f=first.$(sed -E 's#.*/runs/([0-9]+)/.*#\\1#' <<<"$endpoint") ;; + */artifacts\\?name=pr-merge-alerted-*) f=sent.$(sed -E 's#.*pr-merge-alerted-([0-9]+).*#\\1#' <<<"$endpoint") ;; + *) echo "gh: Not Found (HTTP 404)" >&2; exit 1 ;; +esac +v=$(cat "${dir}/$f") +if [ "$v" = x ]; then head 'HTTP/2.0 502 Bad Gateway\\r\\n\\r\\n'; echo '{}'; echo "gh: Server Error (HTTP 502)" >&2; exit 1; fi +if [ "$v" = limited ]; then head '${LIMITED_403}'; echo '${RATE_LIMITED_STDERR}' >&2; exit 1; fi +head 'HTTP/2.0 200 OK\\r\\n\\r\\n' +jq -r "$filter" <<<"$v" +`, + ); + writeFileSync(join(bin, "sleep"), `#!/usr/bin/env bash\necho "$1" >>"${dir}/slept"\n`); + chmodSync(join(bin, "gh"), 0o755); + chmodSync(join(bin, "sleep"), 0o755); + symlinkSync(root, join(dir, ".base")); + const result = spawnSync("bash", ["-c", runScript("Send a red full run to the demotion alert", earlySteps())], { + cwd: dir, + encoding: "utf8", + env: { + ...process.env, + PATH: `${bin}:${process.env.PATH}`, + RUNNER_TEMP: dir, + GITHUB_OUTPUT: join(dir, "output"), + GITHUB_RUN_ID: "700", + REPO: "o/r", + LOOKBACK_HOURS: /\n {10}LOOKBACK_HOURS: (\d+)\n/.exec(step("Send a red full run to the demotion alert", earlySteps()))[1], + }, + }); + const read = (name) => (existsSync(join(dir, name)) ? readFileSync(join(dir, name), "utf8") : ""); + const calls = read("calls").split("\n").filter(Boolean); + return { + status: result.status, + out: result.stdout + result.stderr, + calls, + sends: calls.filter((c) => c.includes("-X POST")), + output: read("output"), + record: read("alerted.json"), + slept: read("slept").split("\n").filter(Boolean).map(Number), + }; + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}; + +test("a red full run is sent to post-merge-demotion-alert.yml once, by its run id and head SHA, and recorded", () => { + const run = runAlert({ runs: [{ id: 41, conclusion: "failure" }] }); + assert.equal(run.status, 0, run.out); + assert.deepEqual(run.sends, [alertSend(41)]); + assert.match(run.output, /^run-id=41$/m); + assert.deepEqual(JSON.parse(run.record), { alerter_run_id: 700, run_id: 41 }); + assert.ok(run.calls[0].includes("actions/workflows/pr-require-checks.yml/runs?event=workflow_dispatch&branch=main&created=%3E%3D"), run.calls[0]); + const upload = step("Record the alert sent", earlySteps()); + assert.match(upload, /\n {8}if: "!cancelled\(\) && steps\.alert\.outputs\.run-id != ''"\n/); + assert.match(upload, /uses: actions\/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 /); + assert.match(upload, /\n {10}name: pr-merge-alerted-\$\{\{ steps\.alert\.outputs\.run-id \}\}\n/); +}); + +// babysit_postmerge.py's FAILED_CONCLUSIONS, the conclusions post-merge +// demotion counts as red: a cancel once the jobs started is red too. +test("every red conclusion of a full run is sent", () => { + for (const conclusion of ["failure", "timed_out", "cancelled", "startup_failure", "action_required"]) { + const run = runAlert({ runs: [{ id: 41, conclusion }] }); + assert.equal(run.status, 0, `${conclusion}: ${run.out}`); + assert.deepEqual(run.sends, [alertSend(41)], conclusion); + } +}); + +test("a green, running or other actor's run, one already sent, and one the group replaced are sent nowhere", () => { + const run = runAlert({ + runs: [ + { id: 40, conclusion: "success" }, + { id: 41, conclusion: "skipped" }, + { id: 42, status: "in_progress" }, + { id: 43, conclusion: "failure", actor: "maintainer" }, + { id: 44, conclusion: "failure" }, + { id: 45, conclusion: "cancelled" }, + ], + sent: { 44: 1 }, + jobs: { 45: 0 }, + }); + assert.equal(run.status, 0, run.out); + assert.deepEqual(run.sends, []); + assert.match(run.out, /run 45 was replaced before it started: no signal/); + assert.equal(run.record, ""); +}); + +// The re-run step may have made attempt 2 first: attempt 1 decides. +test("a run past attempt 1 is judged on attempt 1", () => { + const red = runAlert({ runs: [{ id: 41, attempt: 2, conclusion: "success" }], firsts: { 41: { conclusion: "failure" } } }); + assert.deepEqual(red.sends, [alertSend(41)], red.out); + const green = runAlert({ runs: [{ id: 41, attempt: 2, conclusion: "failure" }], firsts: { 41: { conclusion: "success" } } }); + assert.deepEqual(green.sends, [], green.out); +}); + +test("one run per job: the oldest red one unsent, the next on the next run", () => { + const run = runAlert({ + runs: [ + { id: 52, conclusion: "failure", sha: "89abcdef0123456789abcdef0123456789abcdef" }, + { id: 51, conclusion: "failure" }, + ], + }); + assert.deepEqual(run.sends, [alertSend(51)]); +}); + +test("a failed read of one run's jobs or record sends nothing for it and moves on", () => { + const run = runAlert({ + runs: [ + { id: 41, conclusion: "cancelled" }, + { id: 42, conclusion: "failure" }, + { id: 43, conclusion: "failure" }, + ], + jobs: { 41: "x" }, + sent: { 42: "x" }, + }); + assert.equal(run.status, 0, run.out); + assert.deepEqual(run.sends, [alertSend(43)]); + assert.match(run.out, /::warning::the jobs read of run 41 failed/); + assert.match(run.out, /::warning::the record read of run 42 failed/); + const listing = runAlert({ listing: "x" }); + assert.equal(listing.status, 0); + assert.match(listing.out, /::warning::the listing of main's full runs failed/); +}); + +// The owner outage rule (Q677: at most three attempts, a capped full-jitter +// backoff), sent again only when the request never left, as the dispatch. +test("an alert send that never left is sent again after a jittered wait, at most three times", () => { + const sent = runAlert({ runs: [{ id: 41, conclusion: "failure" }], post: ["unsent", "unsent", "ok"] }); + assert.equal(sent.status, 0, sent.out); + assert.deepEqual(sent.sends, [alertSend(41), alertSend(41), alertSend(41)]); + assert.equal(sent.slept.length, 2); + assert.ok(sent.slept[0] >= 1 && sent.slept[0] <= 10, `${sent.slept[0]}`); + assert.ok(sent.slept[1] >= 1 && sent.slept[1] <= 20, `${sent.slept[1]}`); + const spent = runAlert({ runs: [{ id: 41, conclusion: "failure" }], post: ["unsent"] }); + assert.equal(spent.status, 0, spent.out); + assert.equal(spent.sends.length, 3); + assert.match(spent.out, /::warning::could not send run 41 to post-merge-demotion-alert\.yml after 3 attempt\(s\); the next run tries again/); + assert.equal(spent.record, ""); + // A send that may have reached GitHub is not sent again. + const answered = runAlert({ runs: [{ id: 41, conclusion: "failure" }], post: ["502", "ok"] }); + assert.deepEqual(answered.sends, [alertSend(41)]); + assert.equal(answered.record, ""); +}); + +test("a rate-limited read or send stops red with the stop as the job's output, sending nothing more", () => { + const send = runAlert({ runs: [{ id: 41, conclusion: "failure" }, { id: 42, conclusion: "failure" }], post: ["limited"] }); + assert.equal(send.status, 1, send.out); + assert.deepEqual(send.sends, [alertSend(41)]); + assert.match(send.out, /::error title=GitHub rate limit::the send of run 41 to post-merge-demotion-alert\.yml was rate limited/); + assert.match(send.output, /^rate-limited=true$/m); + assert.match(send.output, /^stop=/m); + const read = runAlert({ runs: [{ id: 41, conclusion: "failure" }], sent: { 41: "limited" } }); + assert.equal(read.status, 1, read.out); + assert.deepEqual(read.sends, []); + assert.match(read.out, /::error title=GitHub rate limit::the record read of run 41 was rate limited/); + assert.match(jobBlock("post-merge-dispatch"), /\n {6}rate-limited: \$\{\{ [^\n]*\|\| steps\.alert\.outputs\.stop \}\}\n/); +}); + +// Every merge's run is sent, as dispatch-full-run starts it: whoever merged, +// whatever the kill switch and report-only say, and after a dispatch or +// re-run that failed for any reason but a rate limit. +test("the alert send runs whatever the switches say, after the re-run, and stops only after a rate limit", () => { + const list = earlySteps(); + const alert = step("Send a red full run to the demotion alert", list); + assert.match( + alert, + /\n {8}if: >-\n {10}!cancelled\(\)\n {10}&& steps\.post-merge\.outputs\.code != '75'\n {10}&& steps\.dispatch\.outputs\.rate-limited != 'true'\n {10}&& steps\.rerun\.outputs\.rate-limited != 'true'\n/, + ); + assert.doesNotMatch(alert, /steps\.(kill|mode)\./); + const names = list.map((s) => s.name); + assert.deepEqual(names.slice(-2), ["Send a red full run to the demotion alert", "Record the alert sent"]); +}); + // The merge job's check, run with this run's post-merge-dispatch report kept // in post-merge-kept (or none) and a fake python3: `settle` answers whether // the report stands (exit `settle`), any other call is the check, which marks diff --git a/.github/scripts/pr-merge/workflow.test.mjs b/.github/scripts/pr-merge/workflow.test.mjs index 1ce3588696..cc29f1d263 100644 --- a/.github/scripts/pr-merge/workflow.test.mjs +++ b/.github/scripts/pr-merge/workflow.test.mjs @@ -1,10 +1,12 @@ // pr-merge.yml's main-verification trigger. Expected values come from the -// owner decision of 2026-10-18 (every merge to main starts the full run, -// whoever merged), GitHub's concurrency rules (a group holds one running and -// one pending job, and a newer pending job replaces the older one unless -// `queue: max` is set), and the rule babysit_postmerge.py and -// post-merge-demotion-alert.yml share: a cancelled full run counts as red, so -// no pr-require-checks run may be cancelled by concurrency. +// owner decision of 2026-10-18 (every merge to main gets a full run, whoever +// merged), the efficiency plan of 2026-10-26 (items 5 and 7(b): at most one +// full run of main per 30 minutes, and no wait on the run), GitHub's +// concurrency rules (a group holds one running and one pending job, and a +// newer pending job replaces the older one unless `queue: max` is set), and +// the rule babysit_postmerge.py and post-merge-demotion-alert.yml share: a +// cancelled full run counts as red, unless it never started, so no running +// pr-require-checks run may be cancelled by concurrency. import assert from "node:assert/strict"; import { spawn, spawnSync } from "node:child_process"; import { existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; @@ -45,191 +47,187 @@ test("push dispatches share one group that queues one and cancels nothing", () = assert.doesNotMatch(dispatch, /\n {6}queue:/); }); -test("a push job holds the group until the dispatched run finishes", () => { +// The job ends once it dispatches (efficiency plan item 7(b), owner +// decision): post-merge-dispatch sends a red run to the alert on the next +// scheduled run, so no step polls the run or sleeps on a held runner. +test("dispatch-full-run ends once it dispatches: no wait on the run it started", () => { const dispatch = job("dispatch-full-run"); - const record = dispatch.indexOf("- name: Record the dispatched run"); - const wait = dispatch.indexOf("- name: Wait for the full run to finish"); - assert.ok(record >= 0 && wait > record); - const step = dispatch.slice(wait); - assert.match(step, /\n {8}if: github\.event_name == 'push' && steps\.dispatch\.outputs\.run-id != ''\n/); - assert.doesNotMatch(step, /gh run watch|--exit-status/); + assert.deepEqual( + [...dispatch.matchAll(/\n {6}- name: (.*)/g)].map((m) => m[1]), + ["Read main's full-run cover", "Start the full CI run on main", "Record the dispatched run"], + ); + assert.doesNotMatch(dispatch, /sleep 180|actions\/runs\/\$RUN_ID|post-merge-demotion-alert\.yml\/dispatches/); + const timeout = Number(/\n {4}timeout-minutes: (\d+)\n/.exec(dispatch)[1]); + assert.ok(timeout <= 10, `timeout-minutes ${timeout}`); }); -// The wait step's body, run under bash against a stub gh (one canned run -// read, then any write logged) and a stub sleep. The alert workflow hears of -// a failed run only through this dispatch: GITHUB_TOKEN started the run, so -// it raises no workflow_run event. const HEAD = "0123456789abcdef0123456789abcdef01234567"; -const SEND = `api -X POST repos/o/r/actions/workflows/post-merge-demotion-alert.yml/dispatches -f ref=main -f inputs[run-id]=4242 -f inputs[head-sha]=${HEAD}`; -// With notModified N, the first read finds the run in progress with ETag -// "e1", the next N reads (sent with If-None-Match on it) are answered 304 -// with gh's exit 1, and the one after finds it completed. With limited, every -// read is refused with limitedMessage. Each entry of sendErrors fails one send -// in turn with that stderr; the send after them succeeds, unless -// dispatchFails. // gh's stderr for a POST GitHub refused for a rate limit, as run 39906176584 // logged it (pr-merge.yml, dispatch-full-run, "Start the full CI run on main", // 2026-10-23T05:30:57Z). const RATE_LIMITED_STDERR = "gh: API rate limit exceeded for installation. If you reach out to GitHub Support for help, please include the request ID 8000:1DF2D:1EE39F:657D15:6ADAF110 and timestamp 2026-10-23 05:30:57 UTC. For more on scraping GitHub and how it may affect your rights, please review our Terms of Service (https://docs.github.com/en/site-policy/github-terms/github-terms-of-service) (HTTP 403)"; -function runWait(conclusion, { dispatchFails = false, sendErrors = [], notModified = 0, limited = false, limitedMessage = RATE_LIMITED_STDERR } = {}) { - const step = job("dispatch-full-run").split("\n - name: Wait for the full run to finish\n")[1]; - const body = /\n {8}run: \|\n((?: {10}.*\n|\s*\n)+)/ - .exec(step)[1] - .split("\n") - .map((l) => l.slice(10)) - .join("\n"); - const dir = mkdtempSync(join(tmpdir(), "pr-merge-wait-")); + +// The cover step's body, run under bash against a stub gh that applies the +// step's --jq filter to a canned listing of pr-require-checks.yml dispatches +// on main, newest first, as the runs API lists them. `runs` entries are +// {minutesAgo, sha, actor, conclusion}. The cap (owner decision, efficiency +// plan item 5) is one full run of main per 30 minutes. +const coverBody = (id) => { + const block = job(id).split("\n - name: Read main's full-run cover\n")[1].split("\n - name: ")[0]; + return { block, body: runBody(block) }; +}; +function runCover(runs, { jobId = "dispatch-full-run", fails = false, tip = HEAD } = {}) { + const dir = mkdtempSync(join(tmpdir(), "pr-merge-cover-")); mkdirSync(join(dir, "bin")); - writeFileSync(join(dir, "bin", "sleep"), `#!/bin/sh\necho "$*" >>"${dir}/sleeps"\n`, { - mode: 0o755, - }); - writeFileSync(join(dir, "n304"), String(notModified)); - sendErrors.forEach((stderr, i) => writeFileSync(join(dir, `send-stderr-${i + 1}`), stderr)); - const done = `{"status": "completed", "conclusion": "${conclusion}", "head_sha": "${HEAD}"}`; - const first = notModified > 0 ? `{"status": "in_progress", "conclusion": null, "head_sha": "${HEAD}"}` : done; + const now = Date.now(); + const listing = { + workflow_runs: runs.map((r, i) => ({ + id: 100 - i, + head_sha: r.sha ?? HEAD, + head_branch: "main", + created_at: new Date(now - r.minutesAgo * 60_000).toISOString().replace(/\.\d{3}Z$/, "Z"), + actor: { login: r.actor ?? "github-actions[bot]" }, + status: r.conclusion ? "completed" : "in_progress", + conclusion: r.conclusion ?? null, + })), + }; + writeFileSync(join(dir, "listing.json"), JSON.stringify(listing)); writeFileSync( join(dir, "bin", "gh"), `#!/usr/bin/env bash printf '%s\\n' "$*" >>"${dir}/log" -case "$*" in - *-X\\ POST*) - s=$(($(cat "${dir}/sends" 2>/dev/null || echo 0) + 1)); echo "$s" >"${dir}/sends" - if [ -f "${dir}/send-stderr-$s" ]; then cat "${dir}/send-stderr-$s" >&2; exit 1; fi - exit ${dispatchFails ? 1 : 0} ;; - ${limited ? `*) echo '${limitedMessage}' >&2; exit 1 ;;` : ""} - *'-H If-None-Match: "e1"') - n=$(cat "${dir}/n304") - if [ "$n" -gt 0 ]; then - echo $((n - 1)) >"${dir}/n304" - printf 'HTTP/2.0 304 Not Modified\\r\\nEtag: "e1"\\r\\n\\r\\n' - echo 'gh: HTTP 304' >&2 - exit 1 - fi - printf 'HTTP/2.0 200 OK\\r\\nEtag: "e2"\\r\\n\\r\\n%s\\n' '${done}' ;; - *) printf 'HTTP/2.0 200 OK\\r\\nEtag: "e1"\\r\\n\\r\\n%s\\n' '${first}' ;; -esac +${fails ? `echo 'gh: Server Error (HTTP 502)' >&2; exit 1` : ""} +while [ $# -gt 0 ]; do if [ "$1" = --jq ]; then exec jq -c "$2" "${dir}/listing.json"; fi; shift; done +exit 1 `, { mode: 0o755 }, ); - writeFileSync(join(dir, "body.sh"), body); + writeFileSync(join(dir, "body.sh"), coverBody(jobId).body); + writeFileSync(join(dir, "output"), ""); const result = spawnSync("bash", ["-e", join(dir, "body.sh")], { env: { ...process.env, PATH: `${join(dir, "bin")}:${process.env.PATH}`, GH_TOKEN: "x", REPO: "o/r", - RUN_ID: "4242", + TIP: tip, + CAP_MINUTES: /\n {10}CAP_MINUTES: (\d+)\n/.exec(coverBody(jobId).block)[1], + GITHUB_OUTPUT: join(dir, "output"), RUNNER_TEMP: dir, }, encoding: "utf8", }); + const output = readFileSync(join(dir, "output"), "utf8"); const log = existsSync(join(dir, "log")) ? readFileSync(join(dir, "log"), "utf8") : ""; - const sleeps = existsSync(join(dir, "sleeps")) ? readFileSync(join(dir, "sleeps"), "utf8").split("\n").filter(Boolean) : []; rmSync(dir, { recursive: true, force: true }); return { status: result.status, out: result.stdout + result.stderr, - writes: log.split("\n").filter((l) => l.includes("-X POST")), - reads: log.split("\n").filter((l) => l.startsWith("api -i ")), - sleeps, + capped: /^capped=true$/m.test(output), + uncovered: /^uncovered=true$/m.test(output), + writes: log.split("\n").filter((l) => l.includes("-X")), }; } - -test("a rate-limited read of the full run stops the wait red after one read", () => { - const { status, out, reads, writes } = runWait("failure", { limited: true }); - assert.equal(status, 1, out); - assert.equal(reads.length, 1); - assert.deepEqual(writes, []); - assert.match(out, /::error title=GitHub rate limit::the read of run 4242 was rate limited/); +const OLDER = "89abcdef0123456789abcdef0123456789abcdef"; + +test("both jobs read main's cover with the same step body, a 30-minute cap and the run's own SHA as the tip", () => { + const a = coverBody("dispatch-full-run"); + const b = coverBody("post-merge-dispatch"); + assert.equal(a.body, b.body); + for (const { block } of [a, b]) { + assert.match(block, /\n {10}CAP_MINUTES: 30\n/); + assert.match(block, /\n {10}TIP: \$\{\{ github\.sha \}\}\n/); + assert.match(block, /^ {8}id: cover\n/); + } + assert.match(a.body, /actions\/workflows\/pr-require-checks\.yml\/runs\?event=workflow_dispatch&branch=main/); }); -// gh's bare 429 message names no rate limit, and is one all the same. -test("a bare HTTP 429 on the read of the full run stops the wait red after one read", () => { - const { status, out, reads, writes } = runWait("failure", { limited: true, limitedMessage: "gh: Too Many Requests (HTTP 429)" }); - assert.equal(status, 1, out); - assert.equal(reads.length, 1); - assert.deepEqual(writes, []); - assert.match(out, /::error title=GitHub rate limit::the read of run 4242 was rate limited/); +test("a full run of main started within 30 minutes caps the next dispatch", () => { + const r = runCover([{ minutesAgo: 12, sha: OLDER }]); + assert.equal(r.status, 0, r.out); + assert.equal(r.capped, true); + assert.equal(r.uncovered, true); + assert.match(r.out, /::notice::a full run of main was started at .*, within 30 minutes/); + assert.deepEqual(r.writes, []); }); -// A 304 is the run as last read: the wait keeps its three-minute pace and -// sends the red run once it completes, as with plain reads. -test("a poll answered 304 waits on, then sends the red run once it completes", () => { - const { status, writes, reads, sleeps } = runWait("failure", { notModified: 2 }); - assert.equal(status, 0); - assert.deepEqual(writes, [SEND]); - assert.deepEqual(sleeps, ["180", "180", "180", "180"]); - assert.deepEqual(reads, [ - "api -i repos/o/r/actions/runs/4242", - 'api -i repos/o/r/actions/runs/4242 -H If-None-Match: "e1"', - 'api -i repos/o/r/actions/runs/4242 -H If-None-Match: "e1"', - 'api -i repos/o/r/actions/runs/4242 -H If-None-Match: "e1"', - ]); +test("a full run of main older than 30 minutes caps nothing, and a run at the tip covers it", () => { + const old = runCover([{ minutesAgo: 31, sha: OLDER }]); + assert.equal(old.capped, false); + assert.equal(old.uncovered, true); + const atTip = runCover([{ minutesAgo: 45, conclusion: "success" }]); + assert.equal(atTip.capped, false); + assert.equal(atTip.uncovered, false); + assert.deepEqual(runCover([]), { status: 0, out: "", capped: false, uncovered: true, writes: [] }); }); -// The alert serializes on signal and head SHA, so the send names the run's -// head SHA as the run reports it, not this workflow's push SHA. -test("a red full run is sent to post-merge-demotion-alert.yml by its run id and head SHA", () => { - const { status, writes } = runWait("failure"); - assert.equal(status, 0); - assert.deepEqual(writes, [SEND]); +// A cancelled run is no cover: replaced before it started, or cancelled once +// running, which reads red. Only github-actions[bot]'s dispatches count: the +// post-merge check credits no one else's for a lane merge. +test("a cancelled run or another actor's dispatch neither caps nor covers", () => { + const r = runCover([ + { minutesAgo: 5, conclusion: "cancelled" }, + { minutesAgo: 8, actor: "maintainer" }, + { minutesAgo: 40, sha: OLDER, conclusion: "success" }, + ]); + assert.equal(r.capped, false, r.out); + assert.equal(r.uncovered, true); }); -// babysit_postmerge.py's FAILED_CONCLUSIONS, the conclusions post-merge -// demotion counts as red: a cancel on a time-budget overrun is red too. -test("every red conclusion of a full run is sent to post-merge-demotion-alert.yml", () => { - for (const conclusion of ["failure", "timed_out", "cancelled", "startup_failure", "action_required"]) { - const { status, writes } = runWait(conclusion); - assert.equal(status, 0, conclusion); - assert.deepEqual(writes, [SEND], conclusion); - } +test("a failed read sets neither output, so the run dispatches as with no cap", () => { + const r = runCover([{ minutesAgo: 5 }], { fails: true }); + assert.equal(r.status, 0); + assert.equal(r.capped, false); + assert.equal(r.uncovered, false); + assert.match(r.out, /::warning::the read of main's full runs failed; no cap applies to this run/); }); -test("a green, skipped or neutral full run is sent nowhere", () => { - for (const conclusion of ["success", "skipped", "neutral"]) { - const { status, writes } = runWait(conclusion); - assert.equal(status, 0, conclusion); - assert.deepEqual(writes, [], conclusion); - } +// The dispatch steps' conditions, evaluated over the cover and post-merge +// outputs: dispatch-full-run starts nothing within the cap; post-merge-dispatch +// starts main's tip when no run covers it, for any merge, and keeps the lane's +// own request, both outside the cap. +const stepIf = (id, name) => { + const block = job(id).split(`\n - name: ${name}\n`)[1].split("\n - name: ")[0]; + const text = /\n {8}if: (?:>-\n((?: {10}.*\n)+)|(.*)\n)/.exec(block); + return (text[1] ?? text[2]).replaceAll("\n", " "); +}; +const evalStep = (text, steps) => + new Function("steps", `return (${text.replace(/\.([a-z]+(?:-[a-z]+)+)/g, '["$1"]')});`)(steps); +const outs = ({ capped = "", uncovered = "", needs = "false" } = {}) => ({ + cover: { outputs: { capped, uncovered } }, + "post-merge": { outputs: { "needs-dispatch": needs } }, }); -test("a send that fails fails the job", () => { - const { status, out } = runWait("failure", { dispatchFails: true }); - assert.equal(status, 1); - assert.match(out, /::error::could not send run 4242 to post-merge-demotion-alert\.yml/); +test("dispatch-full-run starts nothing within the cap, and anything outside it", () => { + const text = stepIf("dispatch-full-run", "Start the full CI run on main"); + assert.equal(evalStep(text, outs({ capped: "true" })), false); + assert.equal(evalStep(text, outs({ uncovered: "true" })), true); + // A failed cover read sets nothing: the dispatch goes ahead. + assert.equal(evalStep(text, outs()), true); }); -// The owner outage rule (Q677: at most three attempts, a capped full-jitter -// backoff), sent again only when the request never left, as the dispatch. -// The 180 s entries are the wait's own polls; the alert's waits come after. -test("an alert send that never left is sent again after a jittered wait, at most three times", () => { - const timeout = 'Post "https://api.github.com/repos/o/r/actions/workflows/post-merge-demotion-alert.yml/dispatches": net/http: TLS handshake timeout'; - const sent = runWait("failure", { sendErrors: [timeout, timeout] }); - assert.equal(sent.status, 0, sent.out); - assert.deepEqual(sent.writes, [SEND, SEND, SEND]); - const waits = sent.sleeps.slice(1).map(Number); - assert.equal(waits.length, 2); - assert.ok(waits[0] >= 1 && waits[0] <= 10, `${waits[0]}`); - assert.ok(waits[1] >= 1 && waits[1] <= 20, `${waits[1]}`); - const spent = runWait("failure", { sendErrors: [timeout, timeout, timeout] }); - assert.equal(spent.status, 1, spent.out); - assert.equal(spent.writes.length, 3); - assert.match(spent.out, /::error::could not send run 4242 to post-merge-demotion-alert\.yml after 3 attempt\(s\)/); - // A send that may have reached GitHub is not sent again. - const answered = runWait("failure", { sendErrors: ["gh: Server Error (HTTP 502)", ""] }); - assert.equal(answered.status, 1); - assert.deepEqual(answered.writes, [SEND]); +test("post-merge-dispatch starts main's uncovered tip for any merge, outside the cap only", () => { + const text = stepIf("post-merge-dispatch", "Start the full CI run on main"); + const expr = text; + assert.equal(evalStep(expr, outs({ uncovered: "true" })), true, "a skipped merge of anyone's"); + assert.equal(evalStep(expr, outs({ uncovered: "true", capped: "true" })), false); + assert.equal(evalStep(expr, outs({ needs: "true" })), true, "the lane's own request"); + assert.equal(evalStep(expr, outs({ needs: "true", capped: "true" })), false); + assert.equal(evalStep(expr, outs()), false, "a covered tip"); }); -test("a rate-limited alert send is sent once and stops red", () => { - for (const stderr of [RATE_LIMITED_STDERR, "gh: Too Many Requests (HTTP 429)"]) { - const { status, out, writes, sleeps } = runWait("failure", { sendErrors: [stderr] }); - assert.equal(status, 1, out); - assert.deepEqual(writes, [SEND]); - assert.deepEqual(sleeps, ["180"]); - assert.match(out, /::error title=GitHub rate limit::the send of run 4242 to post-merge-demotion-alert\.yml was rate limited/); - } +// The backstop must not depend on the lane's switches: dispatch-full-run +// starts a run for every merge while the kill switch or report-only is on, +// so the step that covers a merge the cap skipped reads the cover the same +// way, after the steps those switches skip. +test("post-merge-dispatch reads the cover whatever the kill switch and report-only say", () => { + const dispatch = job("post-merge-dispatch"); + const block = dispatch.split("\n - name: Read main's full-run cover\n")[1].split("\n - name: ")[0]; + assert.doesNotMatch(block, /\n {8}if:/); + const names = [...dispatch.matchAll(/\n {6}- name: (.*)/g)].map((m) => m[1]); + assert.equal(names.indexOf("Read main's full-run cover") + 1, names.indexOf("Start the full CI run on main")); + assert.ok(names.indexOf("Check earlier lane merges") < names.indexOf("Read main's full-run cover")); }); test("the dispatch job's token is actions: write and nothing else", () => { @@ -240,12 +238,67 @@ test("the dispatch job's token is actions: write and nothing else", () => { ); }); -test("pr-require-checks.yml never cancels a dispatched or scheduled run", () => { +// pr-require-checks.yml's concurrency, evaluated per event. Main's dispatches +// share one group with no `queue` key: GitHub then keeps one running and one +// pending, "When a new job or workflow run is queued, any existing pending job +// or workflow run in the same group is canceled and replaced", whereas +// `queue: max` keeps up to 100 pending, which would batch nothing +// (https://docs.github.com/en/actions/how-tos/write-workflows/choose-when-workflows-run/control-workflow-concurrency). +// That group never cancels a running run, which reads red to the post-merge +// check; every other group keeps its cancel. +const checksConcurrency = () => { const checks = read("pr-require-checks.yml"); + const block = /\nconcurrency:\n {2}group: >-\n([\s\S]*?)\n {2}cancel-in-progress: (.*)\n(?=\S|\n)/.exec(checks); + assert.ok(block, "a workflow-level concurrency block"); + const unwrap = (text) => text.trim().replace(/^\$\{\{\s*/, "").replace(/\s*\}\}$/, "").replaceAll("\n", " "); + const evaluate = (text, github) => + new Function("github", "contains", "fromJSON", "format", `return (${unwrap(text)});`)( + github, + (list, item) => list.includes(item), + JSON.parse, + (template, ...values) => template.replace(/\{(\d+)\}/g, (_, i) => String(values[i])), + ); + return { checks, block, group: (github) => evaluate(block[1], github), cancel: (github) => evaluate(block[2], github) }; +}; +const prEvent = (action, number = 5) => ({ + event_name: "pull_request", + ref: `refs/pull/${number}/merge`, + repository: "o/r", + workflow: "pr-require-checks", + run_id: 900 + number, + event: { action, pull_request: { number, head: { repo: { full_name: "o/r" } } }, changes: {} }, +}); + +test("pr-require-checks.yml holds main's dispatches in one group that cancels nothing", () => { + const { checks, block, group, cancel } = checksConcurrency(); const on = checks.slice(checks.indexOf("\non:\n"), checks.indexOf("\npermissions:\n")); assert.doesNotMatch(on, /\n {2}push:/); - const group = /\nconcurrency:\n {2}group: >-\n([\s\S]*?)\n {2}cancel-in-progress:/.exec(checks)[1]; - assert.match(group, /github\.event\.pull_request\.number \|\| github\.run_id/); + assert.doesNotMatch(block[0], /queue:/); + const main = (run_id, run_attempt = 1) => ({ event_name: "workflow_dispatch", ref: "refs/heads/main", workflow: "pr-require-checks", run_id, run_attempt, event: { pull_request: { head: { repo: {} } }, changes: {} } }); + assert.equal(group(main(1)), group(main(2))); + assert.equal(cancel(main(1)), false); + // A re-run keeps its own group, so no newer dispatch replaces the one + // re-run a red run waits on (babysit_postmerge.py reads it red otherwise). + assert.equal(group(main(3, 2)), "pr-require-checks-3"); + assert.equal(cancel(main(3, 2)), true); +}); + +test("pr-require-checks.yml keeps every other run's group and cancel", () => { + const { group, cancel } = checksConcurrency(); + // A missing event field reads null in an expression; this stands in for it. + const none = { pull_request: { head: { repo: {} } }, changes: {} }; + const other = [ + { event_name: "workflow_dispatch", ref: "refs/heads/feat/x", workflow: "pr-require-checks", run_id: 7, event: none }, + { event_name: "schedule", ref: "refs/heads/main", workflow: "pr-require-checks", run_id: 8, event: none }, + { event_name: "merge_group", ref: "refs/heads/gh-readonly-queue/main/pr-1", workflow: "pr-require-checks", run_id: 9, event: none }, + ]; + for (const github of other) { + assert.equal(group(github), `pr-require-checks-${github.run_id}`, github.event_name); + assert.equal(cancel(github), true, github.event_name); + } + assert.equal(group(prEvent("synchronize")), "pr-require-checks-5"); + assert.equal(group(prEvent("labeled")), "ci-contract-5-905"); + assert.equal(cancel(prEvent("synchronize")), true); }); // Only a scheduled or hand-started run arms (enable-auto-merge.mjs @@ -772,13 +825,11 @@ test("a rate limit after a TLS handshake timeout still stops red, with no furthe } }); -// Only a lane merge is picked up by the scheduled post-merge check; any other -// merge waits for the next push or pr-require-checks.yml's own schedule. +// post-merge-dispatch's cover starts main's tip for any merge no run covers. test("the exhausted-retry warning names who starts the run next", () => { - assert.match(read("pr-require-checks.yml"), /\n {2}schedule:\n {4}- cron: /); for (const jobId of ["dispatch-full-run", "post-merge-dispatch"]) { const r = runDispatch([refused()], { jobId, event: "schedule" }); - assert.match(r.out, /::warning::no full CI run started .*; a scheduled pr-merge run starts one only for a lane merge, and any other merge waits for the next push or pr-require-checks' own schedule$/m, jobId); + assert.match(r.out, /::warning::no full CI run started .*; the next scheduled pr-merge run starts one for main's tip$/m, jobId); } }); @@ -807,13 +858,11 @@ test("a dispatch that returns no run id is not sent again and records nothing", assert.doesNotMatch(r.output, /run-id=/); }); -test("a dispatch that failed skips the record and the wait", () => { +test("a dispatch that failed skips the record", () => { for (const id of ["dispatch-full-run", "post-merge-dispatch"]) { const record = job(id).split("\n - name: Record the dispatched run\n")[1]; assert.match(record, /^ {8}if: steps\.dispatch\.outputs\.run-id != ''\n/, id); } - const wait = job("dispatch-full-run").split("\n - name: Wait for the full run to finish\n")[1]; - assert.match(wait, /^ {8}if: github\.event_name == 'push' && steps\.dispatch\.outputs\.run-id != ''\n/); }); // Each job's "Check who started the run" step, run under bash from a directory diff --git a/.github/workflows/post-merge-demotion-alert.yml b/.github/workflows/post-merge-demotion-alert.yml index 5e4b2778c7..fde02420ea 100644 --- a/.github/workflows/post-merge-demotion-alert.yml +++ b/.github/workflows/post-merge-demotion-alert.yml @@ -37,10 +37,10 @@ name: post-merge-demotion-alert # second intake dispatch. The record keeps needs-human, and closing a fix issue # changes nothing on it. # -# A full run pr-merge.yml starts after each merge uses GITHUB_TOKEN, so its -# completion raises no workflow_run event. pr-merge's dispatch-full-run job -# sends a red one here through workflow_dispatch with its run id and head SHA -# instead; the first step reads that run and refuses anything but a red +# A full run pr-merge.yml starts for main's merges uses GITHUB_TOKEN, so its +# completion raises no workflow_run event. pr-merge's post-merge-dispatch job +# sends a red one here on its next scheduled run, through workflow_dispatch +# with its run id and head SHA, instead; the first step reads that run and refuses anything but a red # pr-require-checks run on this repository's main that github-actions[bot] # dispatched, at that head SHA. Red is # any conclusion in babysit_postmerge.py's FAILED_CONCLUSIONS. A red run @@ -76,7 +76,7 @@ on: workflows: [pr-require-checks, post-merge-detect-silent-revert] types: [completed] branches: [main] - # pr-merge.yml's dispatch-full-run job, with GITHUB_TOKEN, or a listed + # pr-merge.yml's post-merge-dispatch job, with GITHUB_TOKEN, or a listed # human (trusted-actors.json on main) names a red full run that raised no # workflow_run event. Both inputs are checked before use, and the head SHA # must be the run's own. @@ -287,8 +287,11 @@ jobs: exit 0 ;; esac - # Neither signal's runs are cancelled by concurrency, so a cancel - # comes from an actor with actions: write and counts as a failure. + # A cancel counts as a failure: pr-require-checks.yml cancels no + # running full run, so it came from an actor with actions: write or + # a time-budget overrun. A dispatch the main-dispatch concurrency + # group replaced before it started is cancelled too, with no job: + # that is no signal, and the check below files nothing for it. # A re-run that passes is recorded on the record; it clears nothing. # A dispatched run also files on action_required, as the Read step # accepts every conclusion in babysit_postmerge.py's FAILED_CONCLUSIONS. @@ -325,6 +328,13 @@ jobs: echo "::error::run attempt '$RUN_ATTEMPT' is not a positive integer" exit 1 fi + # A failed read is a run that started: it files as before. + if [[ "$RUN_CONCLUSION" == cancelled && "$signal" != canary ]] && + jobs="$(gh api "repos/$GITHUB_REPOSITORY/actions/runs/$RUN_ID/attempts/$RUN_ATTEMPT/jobs?per_page=1" \ + --jq '.total_count // (.jobs | length)')" && [[ "$jobs" == 0 ]]; then + echo "$RUN_WORKFLOW was cancelled before any job started (a newer dispatch replaced it): nothing to file" + exit 0 + fi newly='' if [[ -z "$passed" && "$signal" != canary ]]; then # Re-run the failed jobs once and wait here for that attempt: a diff --git a/.github/workflows/pr-merge.yml b/.github/workflows/pr-merge.yml index 6b7c137e22..543334b281 100644 --- a/.github/workflows/pr-merge.yml +++ b/.github/workflows/pr-merge.yml @@ -37,8 +37,11 @@ name: pr-merge # One exception to the switch: when the merge queue lands any PR on main, # whoever enqueued it, the push starts a full CI run of main (the # dispatch-full-run job), even while the switch or report-only is on, since it -# merges nothing. That run raises no workflow_run event (GITHUB_TOKEN started -# it), so when it ends red the same job sends its run id and head SHA to +# merges nothing; at most one per 30 minutes, and a merge that cap skips gets +# its run of main's tip from the post-merge-dispatch job on the next scheduled +# run. That run raises no workflow_run event (GITHUB_TOKEN started it), so +# when it ends red the post-merge-dispatch job on the next scheduled run sends +# its run id and head SHA to # post-merge-demotion-alert.yml through workflow_dispatch, which files the # post-merge record and fix issues and reports the red-main culprit once the # re-run fails too. A red run this workflow dispatched gets its one re-run @@ -108,7 +111,9 @@ jobs: # fail on a GitHub 5xx, nor after its gate loop, which can take many # minutes. It reads with this job's own token, so the merge job's Actions # token stays read-only. It also starts the one re-run of a red full run the - # lane dispatched, which the lanes App cannot start. Same mechanics as + # lane dispatched, which the lanes App cannot start, starts main's tip when + # no full run covers it and none started within the cap, and sends a red + # full run to post-merge-demotion-alert.yml. Same mechanics as # dispatch-full-run: ref main, and the pr-merge-dispatched- artifact the # post-merge check trusts. Not on pull_request_target, whose dispatch the # check does not trust; that event's merge job still runs the check. @@ -121,13 +126,13 @@ jobs: permissions: contents: read # check out main pull-requests: read # read lane merges and their merge-queue entries - actions: write # read full runs and their records; start pr-require-checks.yml on main and re-run a red lane-dispatched run + actions: write # read full runs and their records; start pr-require-checks.yml on main, re-run a red lane-dispatched run, and send a red one to post-merge-demotion-alert.yml checks: read # read a failed full run's annotations issues: read # read the demotion records the merge floor depends on outputs: # The pool and reset a rate limit stopped this job's GITHUB_TOKEN calls on, # if one did: the merge and enable-auto-merge jobs then send nothing. - rate-limited: ${{ steps.post-merge.outputs.rate-limited || steps.dispatch.outputs.stop || steps.rerun.outputs.stop }} + rate-limited: ${{ steps.post-merge.outputs.rate-limited || steps.dispatch.outputs.stop || steps.rerun.outputs.stop || steps.alert.outputs.stop }} steps: - name: Check out the default branch tip uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -247,15 +252,48 @@ jobs: if-no-files-found: error # The same body as dispatch-full-run's step of this name; see there. + # It runs whether or not the post-merge check did: a merge the cap + # skipped gets its run here, while the kill switch is on too. + - name: Read main's full-run cover + id: cover + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + TIP: ${{ github.sha }} + CAP_MINUTES: 30 + run: | + set -euo pipefail + if ! runs=$(gh api "repos/$REPO/actions/workflows/pr-require-checks.yml/runs?event=workflow_dispatch&branch=main&per_page=20" \ + --jq '[.workflow_runs[] | select(.actor.login == "github-actions[bot]" and .head_branch == "main" and .conclusion != "cancelled") | {sha: .head_sha, created: .created_at}]' 2>"$RUNNER_TEMP/cover-stderr"); then + cat "$RUNNER_TEMP/cover-stderr" >&2 + echo "::warning::the read of main's full runs failed; no cap applies to this run" + exit 0 + fi + since=$(date -u -d "@$(($(date +%s) - CAP_MINUTES * 60))" +%Y-%m-%dT%H:%M:%SZ) + recent=$(jq -r --arg since "$since" '[.[] | select(.created >= $since) | .created] | max // ""' <<<"$runs") + if [ -n "$recent" ]; then + echo "::notice::a full run of main was started at $recent, within $CAP_MINUTES minutes; the first push or scheduled pr-merge run after that window starts one for main's tip" + echo "capped=true" >>"$GITHUB_OUTPUT" + fi + if ! jq -e --arg tip "$TIP" 'any(.[]; .sha == $tip)' <<<"$runs" >/dev/null; then + echo "uncovered=true" >>"$GITHUB_OUTPUT" + fi + + # The same body as dispatch-full-run's step of this name; see there. + # It also starts main's tip when no full run covers it and none started + # within the cap, and starts nothing within the cap. # 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 # of it). Add 180 s here, the re-run step's POST waits (up to 30 s) and - # read-back sleeps (up to 170 s) and about 20 s of reads and uploads: - # about 9 minutes, which leaves 6 for slow reads. A timeout after the re-run POST would leave + # read-back sleeps (up to 170 s), the alert send's waits (up to 30 s) + # and about 30 s of reads and uploads: about 9.5 minutes, which leaves + # 5 for slow reads. A timeout after the re-run POST would leave # its attempt 2 unrecorded, so the cap gives way, not the timeout. - name: Start the full CI run on main id: dispatch - if: steps.post-merge.outputs.needs-dispatch == 'true' + if: >- + steps.cover.outputs.capped != 'true' + && (steps.post-merge.outputs.needs-dispatch == 'true' || steps.cover.outputs.uncovered == 'true') env: GH_TOKEN: ${{ github.token }} REPO: ${{ github.repository }} @@ -338,7 +376,7 @@ jobs: waited=$((waited + wait)) done if [ -z "$id" ]; then - echo "::warning::no full CI run started for $what after $try attempt(s): $why; a scheduled pr-merge run starts one only for a lane merge, and any other merge waits for the next push or pr-require-checks' own schedule" + echo "::warning::no full CI run started for $what after $try attempt(s): $why; the next scheduled pr-merge run starts one for main's tip" exit 0 fi echo "started run $id" @@ -460,6 +498,117 @@ jobs: retention-days: 90 if-no-files-found: error + # A full run GITHUB_TOKEN started raises no workflow_run event, so + # post-merge-demotion-alert.yml hears of a red one only from here. Each + # scheduled or hand-started run sends the oldest red pr-require-checks.yml + # dispatch on main that github-actions[bot] started within + # LOOKBACK_HOURS and no earlier run sent, by its run id and head SHA + # (which keys the alert's concurrency group), and records it as + # pr-merge-alerted-; one run per job, so a send costs one alert job, + # and a second red run goes on the next run. Red is attempt 1 concluding + # in babysit_postmerge.py's FAILED_CONCLUSIONS, except a run the + # concurrency group replaced before it started (cancelled, no job). Any + # merge, whoever merged and whatever the kill switch says, as + # dispatch-full-run starts the run. A rate limit stops the step red, + # as the re-run step's; any other failed read or send sends nothing more + # and leaves the run to the next one. LOOKBACK_HOURS: a red run is sent + # within 15 minutes of its end; 6 hours outlasts a day of missed + # schedules many times over (judgment). + - name: Send a red full run to the demotion alert + id: alert + if: >- + !cancelled() + && steps.post-merge.outputs.code != '75' + && steps.dispatch.outputs.rate-limited != 'true' + && steps.rerun.outputs.rate-limited != 'true' + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + LOOKBACK_HOURS: 6 + run: | + set -euo pipefail + . .base/.github/actions/log-api-budget/api-budget.bash + . .base/.github/scripts/pr-merge/rate-stop.bash + api_budget_wrap_gh github-token + err=$RUNNER_TEMP/alert-stderr + stopped() { + local stop + cat "$err" >&2 + stop=$(rate_stop_from "$err" "$1") + [ -n "$stop" ] || return 0 + echo "::error title=GitHub rate limit::$1 was rate limited ($stop); nothing more is sent" + echo "rate-limited=true" >>"$GITHUB_OUTPUT" + echo "stop=$stop" >>"$GITHUB_OUTPUT" + exit 1 + } + since=$(date -u -d "@$(($(date +%s) - LOOKBACK_HOURS * 3600))" +%Y-%m-%dT%H:%M:%SZ) + if ! runs=$(gh api "repos/$REPO/actions/workflows/pr-require-checks.yml/runs?event=workflow_dispatch&branch=main&created=%3E%3D$since&per_page=100" \ + --jq '.workflow_runs | sort_by(.id) | .[] | select(.actor.login == "github-actions[bot]" and .head_branch == "main") | [.id, .run_attempt, .status, (.conclusion // "none"), .head_sha] | @tsv' 2>"$err"); then + stopped "the listing of main's full runs" + echo "::warning::the listing of main's full runs failed; the next run sends any red one" + exit 0 + fi + while IFS=$'\t' read -r id attempt status conclusion sha; do + [[ $id =~ ^[1-9][0-9]{0,14}$ && $sha =~ ^[0-9a-f]{40}$ ]] || continue + if [ "$attempt" != 1 ]; then + if ! state=$(gh api "repos/$REPO/actions/runs/$id/attempts/1" --jq '.status + " " + (.conclusion // "none")' 2>"$err"); then + stopped "the read of run $id attempt 1" + echo "::warning::the read of run $id attempt 1 failed; the next run tries again" + continue + fi + read -r status conclusion <<<"$state" + fi + [ "$status" = completed ] || continue + [[ $conclusion =~ ^(failure|timed_out|cancelled|startup_failure|action_required)$ ]] || continue + if [ "$conclusion" = cancelled ]; then + if ! jobs=$(gh api "repos/$REPO/actions/runs/$id/attempts/1/jobs?per_page=1" --jq .total_count 2>"$err"); then + stopped "the jobs read of run $id" + echo "::warning::the jobs read of run $id failed; the next run tries again" + continue + fi + if [ "$jobs" = 0 ]; then + echo "run $id was replaced before it started: no signal" + continue + fi + fi + if ! sent=$(gh api "repos/$REPO/actions/artifacts?name=pr-merge-alerted-$id&per_page=1" --jq .total_count 2>"$err"); then + stopped "the record read of run $id" + echo "::warning::the record read of run $id failed; the next run tries again" + continue + fi + [ "$sent" = 0 ] || continue + # As the dispatch: at most three sends, sent again after a capped + # full-jitter wait only when gh's error shows the request never + # left. A rate limit is never sent again. + for try in 1 2 3; do + if gh api -X POST "repos/$REPO/actions/workflows/post-merge-demotion-alert.yml/dispatches" \ + -f ref=main -f "inputs[run-id]=$id" -f "inputs[head-sha]=$sha" >/dev/null 2>"$err"; then + echo "sent run $id ($conclusion) to post-merge-demotion-alert.yml" + echo "{\"alerter_run_id\":$GITHUB_RUN_ID,\"run_id\":$id}" >"$RUNNER_TEMP/alerted.json" + echo "run-id=$id" >>"$GITHUB_OUTPUT" + exit 0 + fi + stopped "the send of run $id to post-merge-demotion-alert.yml" + if [ "$try" = 3 ] || ! grep -qE 'dial tcp|tls: (handshake|first record)|net/http: TLS handshake timeout' "$err"; then + break + fi + wait=$((RANDOM % (10 * 2 ** (try - 1)) + 1)) + echo "::notice::attempt $try: the connection failed before the send; waiting ${wait}s, then sending again" + sleep "$wait" + done + echo "::warning::could not send run $id to post-merge-demotion-alert.yml after $try attempt(s); the next run tries again" + exit 0 + done <<<"$runs" + + - name: Record the alert sent + if: "!cancelled() && steps.alert.outputs.run-id != ''" + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: + name: pr-merge-alerted-${{ steps.alert.outputs.run-id }} + path: ${{ runner.temp }}/alerted.json + retention-days: 7 + if-no-files-found: error + merge: # On the schedule and a hand-started run it waits for post-merge-dispatch # and reuses its report (the post-merge check below). Where that job is @@ -1869,15 +2018,17 @@ jobs: # that failed. A lane merge can get a second run from it when it checks the # merge before the push dispatch's run exists. # - # BURSTS COLLAPSE HERE, NOT IN pr-require-checks.yml. Push jobs share one - # group without `cancel-in-progress`, so one runs while one waits; a newer - # push replaces the waiting job, which is cancelled before it dispatches - # anything. The running job holds the group until the run it started - # finishes, so the waiting one dispatches main's tip then, which contains - # every merge it replaced. A pending pr-require-checks run is never - # cancelled: babysit_postmerge.py and post-merge-demotion-alert.yml both - # count a cancelled full run as red. Every other event takes a group of its - # own and does not wait. + # BURSTS COLLAPSE IN TWO PLACES. This job starts no run while one of main + # started within the cap (the cover step), and pr-require-checks.yml holds + # main's dispatches in one group with no `queue` key: one runs, one waits, + # and a newer one replaces the waiting one before it starts. A running full + # run is never cancelled: babysit_postmerge.py and + # post-merge-demotion-alert.yml both count a cancelled full run as red, and + # skip a replaced one (cancelled, no job). Push jobs share one group without + # `cancel-in-progress`, so two pushes never read the cover at once. The job + # ends once it dispatches: post-merge-dispatch sends a red run to + # post-merge-demotion-alert.yml on the next scheduled run. Every other event + # takes a group of its own and does not wait. # A lanes App pull request whose refusal the lanes clear shows it as a # failing pr-merge/unarmed commit status on its head once the refusal has # stood merge.risk-escalation-minutes, never the human-gated label, and a @@ -1926,12 +2077,49 @@ jobs: group: ${{ github.event_name == 'push' && 'pr-merge-verify-main' || format('pr-merge-dispatch-{0}', github.run_id) }} cancel-in-progress: false runs-on: ubuntu-24.04 - # A push job waits for the full run, whose longest lane is capped at 16 - # minutes, plus time in the runner queue. - timeout-minutes: 45 + # One read and up to three POSTs, whose waits WAIT_CAP bounds. + timeout-minutes: 10 permissions: - actions: write # start pr-require-checks.yml on main, read its status, and send a red one to post-merge-demotion-alert.yml + actions: write # read main's full runs and start pr-require-checks.yml on main steps: + # At most one full run of main per CAP_MINUTES, each of main's tip, + # covering every merge since the last. The step reads the newest + # pr-require-checks.yml dispatches on main that github-actions[bot] + # started, skipping a cancelled one (replaced before it started, or + # cancelled later, which reads red): capped=true when one was created + # within CAP_MINUTES, uncovered=true when none is at TIP. A failed read + # sets neither, so the run dispatches as it would with no cap. A merge + # the cap skips is covered by the next push after the window or by + # post-merge-dispatch on the next scheduled run, whose dispatch starts + # main's tip once nothing covers it, whoever merged and whatever the + # kill switch says. CAP_MINUTES: the efficiency plan of 2026-10-26 + # (item 5, owner decision): 116 runs a day at a median 84.1 runner-min + # become at most 48. + - name: Read main's full-run cover + id: cover + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + TIP: ${{ github.sha }} + CAP_MINUTES: 30 + run: | + set -euo pipefail + if ! runs=$(gh api "repos/$REPO/actions/workflows/pr-require-checks.yml/runs?event=workflow_dispatch&branch=main&per_page=20" \ + --jq '[.workflow_runs[] | select(.actor.login == "github-actions[bot]" and .head_branch == "main" and .conclusion != "cancelled") | {sha: .head_sha, created: .created_at}]' 2>"$RUNNER_TEMP/cover-stderr"); then + cat "$RUNNER_TEMP/cover-stderr" >&2 + echo "::warning::the read of main's full runs failed; no cap applies to this run" + exit 0 + fi + since=$(date -u -d "@$(($(date +%s) - CAP_MINUTES * 60))" +%Y-%m-%dT%H:%M:%SZ) + recent=$(jq -r --arg since "$since" '[.[] | select(.created >= $since) | .created] | max // ""' <<<"$runs") + if [ -n "$recent" ]; then + echo "::notice::a full run of main was started at $recent, within $CAP_MINUTES minutes; the first push or scheduled pr-merge run after that window starts one for main's tip" + echo "capped=true" >>"$GITHUB_OUTPUT" + fi + if ! jq -e --arg tip "$TIP" 'any(.[]; .sha == $tip)' <<<"$runs" >/dev/null; then + echo "uncovered=true" >>"$GITHUB_OUTPUT" + fi + # At most three POSTs. Any 429, or a 403 with a retry-after, remaining # 0, or a message naming a rate limit, is never sent again: # GitHub may ban an integration that keeps calling while limited, so the @@ -1947,15 +2135,13 @@ jobs: # second run would have no record. The waits sum to # at most WAIT_CAP seconds; one that does not fit is not taken. A # dispatch that still fails sets no run id and ends green with a - # warning: nothing is recorded or waited on. The post-merge-dispatch - # job's check on the next scheduled run starts the run only for a lane - # merge; any other merge waits for the next push or pr-require-checks' - # own schedule. - # WAIT_CAP: 45 minutes, less the wait step's 39, its alert send's - # waits (up to 30 s) and about 1 for the reads and uploads, leaves - # 4.5; 180 s of it. + # warning: nothing is recorded. The post-merge-dispatch job on the next + # scheduled run starts one for main's tip, which covers the merge. + # WAIT_CAP: of the 10 minutes, about 1 goes to the cover read and the + # record upload; 180 s of the rest. - name: Start the full CI run on main id: dispatch + if: steps.cover.outputs.capped != 'true' env: GH_TOKEN: ${{ github.token }} REPO: ${{ github.repository }} @@ -2038,7 +2224,7 @@ jobs: waited=$((waited + wait)) done if [ -z "$id" ]; then - echo "::warning::no full CI run started for $what after $try attempt(s): $why; a scheduled pr-merge run starts one only for a lane merge, and any other merge waits for the next push or pr-require-checks' own schedule" + echo "::warning::no full CI run started for $what after $try attempt(s): $why; the next scheduled pr-merge run starts one for main's tip" exit 0 fi echo "started run $id" @@ -2057,73 +2243,6 @@ jobs: retention-days: 90 if-no-files-found: error - # Holds the push group until the run finishes, so a burst of merges - # waits here as one pending job. One REST read every three minutes, 13 - # over 39 minutes; the run's result is the merge job's post-merge - # check's to judge, not this job's. - # A red run raises no workflow_run event (this token started it), so - # this step sends its id and head SHA (which keys the alert's - # concurrency group) to post-merge-demotion-alert.yml, which waits - # for a re-run before it files or reports anything. Red is any - # conclusion in babysit_postmerge.py's FAILED_CONCLUSIONS: a cancel on - # a time-budget overrun is red too. Every merge is a push to main, so - # every merge's run passes here. - - name: Wait for the full run to finish - if: github.event_name == 'push' && steps.dispatch.outputs.run-id != '' - env: - GH_TOKEN: ${{ github.token }} - REPO: ${{ github.repository }} - RUN_ID: ${{ steps.dispatch.outputs.run-id }} - run: | - set -euo pipefail - # Each read after the first is conditional on the last ETag: a 304 - # (gh exits 1) means the run is as last read, still going. - cond=() - for _ in $(seq 13); do - sleep 180 - if ! out=$(gh api -i "repos/$REPO/actions/runs/$RUN_ID" "${cond[@]}" 2>"$RUNNER_TEMP/wait-stderr"); then - cat "$RUNNER_TEMP/wait-stderr" >&2 - # A read refused for a rate limit, a bare 429 included, is never - # retried. This job has no checkout of the shared guard, so the - # match is the pattern rate-stop.bash's rate_stop_from reads. - if grep -qiE 'rate limit|RATE_LIMITED|abuse detection|HTTP 429' "$RUNNER_TEMP/wait-stderr"; then - echo "::error title=GitHub rate limit::the read of run $RUN_ID was rate limited; nothing more is sent" - exit 1 - fi - continue - fi - etag=$(sed '/^\r\?$/q' <<<"$out" | tr -d '\r' | awk -F': *' 'tolower($1) == "etag" { print $2; exit }') - cond=() - [ -z "$etag" ] || cond=(-H "If-None-Match: $etag") - state=$(sed '1,/^\r\?$/d' <<<"$out" | jq -r '.status + " " + .head_sha + " " + (.conclusion // "")') || continue - read -r status head_sha conclusion <<<"$state" - if [ "$status" = completed ]; then - echo "run $RUN_ID completed: $conclusion" - [[ "$conclusion" =~ ^(failure|timed_out|cancelled|startup_failure|action_required)$ ]] || exit 0 - # As the dispatch: at most three sends, sent again after a capped - # full-jitter wait only when gh's error shows the request never - # left. A rate limit is never sent again. - for try in 1 2 3; do - gh api -X POST "repos/$REPO/actions/workflows/post-merge-demotion-alert.yml/dispatches" \ - -f ref=main -f "inputs[run-id]=$RUN_ID" -f "inputs[head-sha]=$head_sha" >/dev/null 2>"$RUNNER_TEMP/alert-stderr" && exit 0 - cat "$RUNNER_TEMP/alert-stderr" >&2 - if grep -qiE 'rate limit|RATE_LIMITED|abuse detection|HTTP 429' "$RUNNER_TEMP/alert-stderr"; then - echo "::error title=GitHub rate limit::the send of run $RUN_ID to post-merge-demotion-alert.yml was rate limited; nothing more is sent" - exit 1 - fi - if [ "$try" = 3 ] || ! grep -qE 'dial tcp|tls: (handshake|first record)|net/http: TLS handshake timeout' "$RUNNER_TEMP/alert-stderr"; then - break - fi - wait=$((RANDOM % (10 * 2 ** (try - 1)) + 1)) - echo "::notice::attempt $try: the connection failed before the send; waiting ${wait}s, then sending again" - sleep "$wait" - done - echo "::error::could not send run $RUN_ID to post-merge-demotion-alert.yml after $try attempt(s); the next scheduled full run reports a red that lasts" - exit 1 - fi - done - echo "::warning::run $RUN_ID did not finish within 39 minutes; releasing the group" - # Each candidate the gate judged shows its verdict on its head as one # `pr-merge / gate` check run: neutral, titled with the top refusal, or # success over a refusal once enqueued or merged. Not `ci-status`, so never diff --git a/.github/workflows/pr-require-checks.yml b/.github/workflows/pr-require-checks.yml index 1840119249..cda91abcec 100644 --- a/.github/workflows/pr-require-checks.yml +++ b/.github/workflows/pr-require-checks.yml @@ -1,9 +1,9 @@ name: pr-require-checks on: - # A dispatch runs the full contract corpus. pr-merge.yml's - # `dispatch-full-run` starts one on main after every merge, whoever merged, - # collapsing a burst of merges into one run of the newest tip. + # A dispatch runs the full contract corpus. pr-merge.yml starts one on main + # for the merges since the last, whoever merged, at most one per 30 minutes, + # each of the newest tip. workflow_dispatch: # NO PUSH TRIGGER. Every commit reaches main through the merge queue (the # repository's merge-queue ruleset, with the org `pull_request` rule and no @@ -70,7 +70,8 @@ on: permissions: contents: read -# THE GROUP BRANCHES ON THE PREDICATE, AND EVERY BRANCH CANCELS. A contract-only +# THE GROUP BRANCHES ON THE PREDICATE, AND EVERY BRANCH BUT MAIN'S DISPATCH +# CANCELS. A contract-only # run takes a group of its own keyed on `github.run_id`, so it can neither be # cancelled nor evict anything, and it waits for nothing: its `ci-status` reads # the `ci-lanes` status once and finishes (see the `ci-status` job). The @@ -83,16 +84,28 @@ permissions: # The full branch keeps one group per pull request, so a `synchronize` still # cancels everything on the old SHA; the PR number scopes cancellation to # exactly one pull request, where `head_ref` would collide across same-named -# fork branches. A merge-group, schedule or dispatch run keeps the unique -# `github.run_id` group, so it is never cancelled. +# fork branches. A merge-group, schedule or other-branch dispatch run keeps +# the unique `github.run_id` group, so it is never cancelled. +# +# A dispatch on main (pr-merge.yml's whole-suite run of main's tip) shares one +# group with no `queue` key and `cancel-in-progress` false: one runs, one waits, +# and a newer dispatch replaces the waiting one, which GitHub cancels before +# any job starts. The newer one holds a later tip, so it covers every merge the +# replaced one did. A running whole-suite run is never cancelled: the post-merge +# check and post-merge-demotion-alert.yml read a cancelled run as red, and read +# a replaced one (cancelled, no job) as no run. Only attempt 1 joins it: a +# re-run keeps its own `github.run_id` group, so a newer dispatch can never +# replace the one re-run a red run waits on. concurrency: group: >- - ${{ (github.event.pull_request.head.repo.full_name == github.repository && + ${{ (github.event_name == 'workflow_dispatch' && github.ref == 'refs/heads/main' && github.run_attempt == 1) + && 'pr-require-checks-main-dispatch' + || ((github.event.pull_request.head.repo.full_name == github.repository && (contains(fromJSON('["labeled","unlabeled"]'), github.event.action) || (github.event.action == 'edited' && !github.event.changes.base))) && format('ci-contract-{0}-{1}', github.event.pull_request.number, github.run_id) - || format('{0}-{1}', github.workflow, github.event.pull_request.number || github.run_id) }} - cancel-in-progress: true + || format('{0}-{1}', github.workflow, github.event.pull_request.number || github.run_id)) }} + cancel-in-progress: ${{ github.event_name != 'workflow_dispatch' || github.ref != 'refs/heads/main' || github.run_attempt != 1 }} # Quality lanes run the ci-workflows composite actions (referenced by SHA, # bumped via Dependabot — never copied) against this repo's exact standards diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index a742295103..137edbbd14 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -2574,8 +2574,11 @@ tests are in [`.github/scripts/post-merge-demotion-alert/`](../../../.github/scripts/post-merge-demotion-alert/). The full run pr-merge starts after each merge uses `GITHUB_TOKEN`, so its completion raises no -`workflow_run` event. When that run ends red, pr-merge's `dispatch-full-run` job sends its run id to -the alert workflow through `workflow_dispatch`. The alert accepts the id only from +`workflow_run` event. When that run ends red, pr-merge's `post-merge-dispatch` job sends its run id +to the alert workflow through `workflow_dispatch` on its next scheduled run, up to 15 minutes +later. A dispatch `pr-require-checks.yml`'s main-dispatch concurrency group replaced before it +started (cancelled, no job) is no signal: it is never sent, and neither the alert nor the +post-merge check reads it as red. The alert accepts the id only from `github-actions[bot]` or a listed human (`trusted-actors.json` on `main`), never the lanes App, and only for attempt 1 of a red `pr-require-checks.yml` run on this repository's `main` that `github-actions[bot]` dispatched. Red is any conclusion `babysit_postmerge.py` counts as failed: 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 b6328c60ab..506b23a973 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py @@ -1090,6 +1090,7 @@ class _Context: 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) + replaced_cache: dict[int, bool] = field(default_factory=dict) # The lane records file (`LANE_RECORDS_ENV`), its records once loaded, # and the final records this check proved, which `save_lane_records` adds. lane_records_path: str | None = field( @@ -1333,6 +1334,35 @@ def on_branch(self, run: Run) -> bool: run.head_sha, self._branch_tip() ) + def replaced(self, run: Run) -> bool: + """Whether `run` is one pr-require-checks.yml's main-dispatch + concurrency group cancelled while it waited, replaced by a newer + dispatch: attempt 1 cancelled with no job. It never ran, so it is no + signal at all, neither red nor pending; the run that replaced it holds + a later tip. A run cancelled once its jobs started stays red.""" + if ( + run.attempt != 1 + or run.status != "completed" + or run.conclusion != "cancelled" + ): + return False + if run.id not in self.replaced_cache: + listing = json_object( + _read( + [ + "api", + f"repos/{self.owner}/{self.name}/actions/runs/{run.id}" + "/attempts/1/jobs?per_page=1", + ], + f"run {run.id} attempt 1 jobs", + ) + ) + total = listing.get("total_count") + if not isinstance(total, int): + raise PostMergeReadError(f"run {run.id}: an unknown jobs listing") + self.replaced_cache[run.id] = total == 0 + return self.replaced_cache[run.id] + def _newest_on_branch(self, runs: list[Run]) -> Run | None: """The newest of `runs` on the branch (`on_branch`).""" return next( @@ -1401,6 +1431,7 @@ def _baseline(self, signal: str, before: datetime) -> frozenset[str] | None: if r.created_at < before and r.head_branch == self.branch and r.status == "completed" + and not self.replaced(r) ] newest = self._newest_on_branch(runs) self.baseline_cache[key] = ( @@ -2039,6 +2070,7 @@ def passes(run: Run) -> bool: if run_id not in listed and run.created_at <= deadline and run.actor in (LANE_DISPATCH_ACTOR, ctx.identity) + and not ctx.replaced(run) ] if missing: raise PostMergeReadError( @@ -2122,6 +2154,7 @@ def _lane_reruns(ctx: _Context, now: datetime, tally: _Tally) -> list[str]: and run.attempt == 1 and ctx.outcome(run, attributable=True)[0] == RERUN and ctx.lane_dispatched(run) + and not ctx.replaced(run) # Only a run that could be marked pays for the ancestry read, # and one off the branch never reaches the affected set. and ctx.on_branch(run) @@ -2244,9 +2277,14 @@ def lane_prs_at(sha: str) -> str | None: ) ] # A listing filtered by branch name also returns runs on a tag named - # like the branch, whose red would be charged to the merge. + # like the branch, whose red would be charged to the merge. A run the + # concurrency group replaced before it started is no signal. full_runs = sorted( - (item for item in full_runs if ctx.on_branch(item[1])), + ( + item + for item in full_runs + if ctx.on_branch(item[1]) and not ctx.replaced(item[1]) + ), key=lambda item: item[1].created_at, ) canary_runs = sorted( diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_postmerge.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_postmerge.py index e891547b01..c1945cf160 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_postmerge.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_postmerge.py @@ -2518,6 +2518,73 @@ def test_a_dispatch_run_with_no_failed_job_does_not_verify(self) -> None: self.assertEqual([f.signal for f in verdict.failures], ["dispatch"]) +class ReplacedRuns(CheckCase): + """pr-require-checks.yml's main-dispatch concurrency group keeps one run + pending and cancels it when a newer dispatch takes its place + (https://docs.github.com/en/actions/how-tos/write-workflows/choose-when-workflows-run/control-workflow-concurrency). + Such a run never starts: GitHub lists it cancelled with no job, as runs + 40192695442 and 40112740615 of work-resolve-issue.yml read on 2026-10-27. + It is no signal; a run cancelled once its jobs started is still red.""" + + REPLACED = 10 + + def setUp(self) -> None: + super().setUp() + self.gh.add_runs("post-merge-detect-silent-revert.yml", "push", canary_run(21)) + self.gh.add_runs( + "pr-require-checks.yml", + "workflow_dispatch", + dispatch_run( + self.REPLACED, + "cancelled", + head_sha=MERGE_SHA, + created_at="2026-10-12T10:02:00Z", + ), + dispatch_run(11), + ) + + def test_a_replaced_run_neither_holds_nor_reruns_the_merge(self) -> None: + self.gh.jobs[(self.REPLACED, 1)] = [] + verdict = self.check() + self.assertTrue(verdict.merges[0].verified, verdict.merges[0].pending) + self.assertEqual(verdict.reruns, ()) + self.assertEqual(verdict.failures, ()) + + def test_a_run_cancelled_after_its_jobs_started_still_waits_for_its_rerun( + self, + ) -> None: + verdict = self.check() + self.assertFalse(verdict.merges[0].verified) + self.assertIn(f"attribution-pending:{self.REPLACED}", verdict.merges[0].pending) + self.assertEqual([r.run_id for r in verdict.reruns], [self.REPLACED]) + + def test_a_replaced_run_is_no_baseline(self) -> None: + # Before the merge: a red run, then a replaced one. The red run is the + # baseline, so its failing check is no regression of the merge's run. + self.gh.runs[("pr-require-checks.yml", "workflow_dispatch")] = [ + dispatch_run( + 5, "failure", head_sha=HEAD_SHA, created_at="2026-10-12T09:20:00Z" + ), + dispatch_run( + 6, "cancelled", head_sha=HEAD_SHA, created_at="2026-10-12T09:30:00Z" + ), + ] + self.gh.jobs[(6, 1)] = [] + ctx = pm._Context( + pm._split_repo(REPO)[0], + pm._split_repo(REPO)[1], + IDENTITY, + branch="main", + now=datetime(2026, 10, 12, 11, 0, tzinfo=UTC), + ) + self.assertEqual( + ctx._baseline( + pm.SIGNAL_DISPATCH, datetime(2026, 10, 12, 10, 0, tzinfo=UTC) + ), + frozenset({"test-bash"}), + ) + + class DispatchCoverage(CheckCase): """Design "Post-merge" plus the accepted deadlock rule: an unverified auto-merge with no identity dispatch run on a SHA containing it needs one; diff --git a/scripts/post-merge-demotion-alert.test.sh b/scripts/post-merge-demotion-alert.test.sh index bcbd5fe362..22d43c493f 100755 --- a/scripts/post-merge-demotion-alert.test.sh +++ b/scripts/post-merge-demotion-alert.test.sh @@ -833,6 +833,18 @@ quiet "a dispatched canary run files nothing" "$CANARY" workflow_dispatch failur quiet "a successful scheduled run files nothing" "$FULL" schedule success main o/r quiet "a successful canary push run files nothing" "$CANARY" push success main o/r CONCLUSION=cancelled files "a cancelled scheduled run files a record" schedule "$FULL" schedule '[]' +# pr-require-checks.yml's main-dispatch group replaces a waiting dispatch with +# a newer one, cancelling it before any job starts (GitHub lists such a run +# cancelled with no job, as work-resolve-issue.yml runs 40192695442 and +# 40112740615 read on 2026-10-27): no signal, so nothing is filed or re-run. +ATTEMPT=1 JOBS='' run_step "$FULL" workflow_dispatch cancelled main o/r "$SHA" '[]' +if [[ "$RC" -eq 0 && "$(grep -c '^POST ' <<<"$LOG")" -eq 0 && "$OUT" == *"cancelled before any job started"* ]]; then + ok "a dispatched run cancelled before any job started files nothing" +else + fail "replaced dispatch: rc=$RC log='$LOG' out='$OUT'" +fi +# One cancelled after its jobs started is still red. +CONCLUSION=cancelled files "a dispatched run cancelled once its jobs started files a record" dispatch "$FULL" workflow_dispatch '[]' quiet "a failed run on another branch files nothing" "$FULL" workflow_dispatch failure feat/x o/r quiet "a failed run from a fork's main files nothing" "$FULL" schedule failure main fork/r quiet "another workflow files nothing" "some-other-workflow" schedule failure main o/r From db2e8471c60d76fcd256aeb7b31c5462e811b2dd Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 27 Oct 2026 15:35:25 -0400 Subject: [PATCH 2/5] fix(pr-merge): stop on a rate-limited cover read and gate the scheduled job on the kill switch A cover read refused for a rate limit now stops the step red with rate-limited=true. post-merge-dispatch classifies it through rate-stop.bash's rate_stop_from, as its alert step does, and adds the stop to the job's rate-limited output, which the merge and enable-auto-merge jobs already stop on; its dispatch, re-run and alert steps skip after it. dispatch-full-run has no checkout of the guard, so it matches the pattern rate_stop_from reads, as its removed wait step did, and its dispatch skips. post-merge-dispatch reads the cover and dispatches only while the lane is live (kill switch off, report-only off), as its post-merge check does, and sends a red run to the alert only with the kill switch off. dispatch-full-run's push carve-out stays. The tests that pinned the cover and alert as running whatever the switches say now pin the reviewed behavior instead. Co-Authored-By: Claude Opus 5.5 --- ...perf-main-whole-suite-batching-9548bbe5.md | 4 +- .github/scripts/pr-merge/merge-job.test.mjs | 100 ++++++++++++++++-- .github/scripts/pr-merge/workflow.test.mjs | 56 +++++++--- .github/workflows/pr-merge.yml | 71 +++++++++---- docs/conventions/pr-pipeline/README.md | 4 +- 5 files changed, 191 insertions(+), 44 deletions(-) diff --git a/.changes/pr-pipeline/perf-main-whole-suite-batching-9548bbe5.md b/.changes/pr-pipeline/perf-main-whole-suite-batching-9548bbe5.md index ecb602a381..3952c33e3c 100644 --- a/.changes/pr-pipeline/perf-main-whole-suite-batching-9548bbe5.md +++ b/.changes/pr-pipeline/perf-main-whole-suite-batching-9548bbe5.md @@ -4,7 +4,7 @@ bump: patch ### Changed -- pr-merge starts at most one full run of main per 30 minutes, of main's tip, covering every merge since the last. dispatch-full-run and post-merge-dispatch read the newest pr-require-checks.yml dispatches on main first (`Read main's full-run cover`): within the cap they start nothing, and post-merge-dispatch starts one on its next scheduled run for a tip no full run covers, whoever merged and whatever the kill switch says (`.github/workflows/pr-merge.yml`). -- dispatch-full-run ends once it dispatches, where it held a runner up to 39 minutes polling the run; its timeout drops from 45 to 10 minutes. post-merge-dispatch now sends a red full run to post-merge-demotion-alert.yml on its next scheduled run, up to 15 minutes after the run ends, once per run (`pr-merge-alerted-`), and skips a run that never started. +- pr-merge starts at most one full run of main per 30 minutes, of main's tip, covering every merge since the last. dispatch-full-run and post-merge-dispatch read the newest pr-require-checks.yml dispatches on main first (`Read main's full-run cover`): within the cap they start nothing, and post-merge-dispatch starts one on its next scheduled run for a tip no full run covers, whoever merged, while the lane is live (kill switch and report-only off). A read of the cover refused for a rate limit stops the job red and sends nothing more (`.github/workflows/pr-merge.yml`). +- dispatch-full-run ends once it dispatches, where it held a runner up to 39 minutes polling the run; its timeout drops from 45 to 10 minutes. post-merge-dispatch now sends a red full run to post-merge-demotion-alert.yml on its next scheduled run with the kill switch off, up to 15 minutes after the run ends, once per run (`pr-merge-alerted-`), and skips a run that never started. - pr-require-checks.yml's dispatches on main (attempt 1) share one concurrency group with no `queue` key and `cancel-in-progress` false: one runs, one waits, and a newer one replaces the waiting one. A re-run and every other run keep their own group and cancel. - post-merge-demotion-alert.yml files nothing for a cancelled full run with no job, one the concurrency group replaced before it started. diff --git a/.github/scripts/pr-merge/merge-job.test.mjs b/.github/scripts/pr-merge/merge-job.test.mjs index 294ee72f6d..13062c8fc9 100644 --- a/.github/scripts/pr-merge/merge-job.test.mjs +++ b/.github/scripts/pr-merge/merge-job.test.mjs @@ -1466,9 +1466,9 @@ test("a rate-limited first read stops the step red before the re-run, naming the assert.match(run.output, /^stop=the core pool until 2100-01-01T00:00:00Z$/m); }); -test("post-merge-dispatch's rate-limited output also carries the re-run and alert steps' stops", () => { +test("post-merge-dispatch's rate-limited output also carries the cover, re-run and alert steps' stops", () => { const header = `${jobBlock("post-merge-dispatch").split("\n steps:\n")[0]}\n`; - assert.match(header, /\n {6}rate-limited: \$\{\{ steps\.post-merge\.outputs\.rate-limited \|\| steps\.dispatch\.outputs\.stop \|\| steps\.rerun\.outputs\.stop \|\| steps\.alert\.outputs\.stop \}\}\n/); + assert.match(header, /\n {6}rate-limited: \$\{\{ steps\.post-merge\.outputs\.rate-limited \|\| steps\.cover\.outputs\.stop \|\| steps\.dispatch\.outputs\.stop \|\| steps\.rerun\.outputs\.stop \|\| steps\.alert\.outputs\.stop \}\}\n/); }); test("the re-run step refuses a run id that is not a number", () => { @@ -1484,7 +1484,7 @@ test("the re-run record is named by run and attempt, uploaded only after a recor // and nothing is sent after the dispatch stopped on a rate limit. assert.match( rerun, - /\n {8}if: >-\n {10}!cancelled\(\)\n {10}&& steps\.dispatch\.outputs\.rate-limited != 'true'\n {10}&& contains\(fromJSON\('\["0","1","5"\]'\), steps\.post-merge\.outputs\.code\)\n {10}&& steps\.post-merge\.outputs\.lane-rerun != ''\n/, + /\n {8}if: >-\n {10}!cancelled\(\)\n {10}&& steps\.cover\.outputs\.rate-limited != 'true'\n {10}&& steps\.dispatch\.outputs\.rate-limited != 'true'\n {10}&& contains\(fromJSON\('\["0","1","5"\]'\), steps\.post-merge\.outputs\.code\)\n {10}&& steps\.post-merge\.outputs\.lane-rerun != ''\n/, ); assert.match(rerun, /\n {10}GH_TOKEN: \$\{\{ github\.token \}\}\n/); const upload = step("Record the re-run", list); @@ -1715,21 +1715,103 @@ test("a rate-limited read or send stops red with the stop as the job's output, s assert.match(jobBlock("post-merge-dispatch"), /\n {6}rate-limited: \$\{\{ [^\n]*\|\| steps\.alert\.outputs\.stop \}\}\n/); }); -// Every merge's run is sent, as dispatch-full-run starts it: whoever merged, -// whatever the kill switch and report-only say, and after a dispatch or -// re-run that failed for any reason but a rate limit. -test("the alert send runs whatever the switches say, after the re-run, and stops only after a rate limit", () => { +// Every merge's run is sent, as dispatch-full-run starts it, whoever merged, +// after a dispatch or re-run that failed for any reason but a rate limit, and +// only while the kill switch is off (review decision on #10491: the scheduled +// job acts only with the switch off; report-only merges nothing, so the alert +// still runs under it). +test("the alert send runs after the re-run while the switch is off, and stops after a rate limit", () => { const list = earlySteps(); const alert = step("Send a red full run to the demotion alert", list); assert.match( alert, - /\n {8}if: >-\n {10}!cancelled\(\)\n {10}&& steps\.post-merge\.outputs\.code != '75'\n {10}&& steps\.dispatch\.outputs\.rate-limited != 'true'\n {10}&& steps\.rerun\.outputs\.rate-limited != 'true'\n/, + /\n {8}if: >-\n {10}!cancelled\(\)\n {10}&& steps\.kill\.outputs\.proceed == 'true'\n {10}&& steps\.post-merge\.outputs\.code != '75'\n {10}&& steps\.cover\.outputs\.rate-limited != 'true'\n {10}&& steps\.dispatch\.outputs\.rate-limited != 'true'\n {10}&& steps\.rerun\.outputs\.rate-limited != 'true'\n/, ); - assert.doesNotMatch(alert, /steps\.(kill|mode)\./); const names = list.map((s) => s.name); assert.deepEqual(names.slice(-2), ["Send a red full run to the demotion alert", "Record the alert sent"]); }); +// The scheduled job's cover read, run with .base at this checkout (the rate +// guard) against a fake gh whose listing answers `answer`: "limited" with the +// rate-limited 403, "x" with a 502. +const runPostMergeCover = (answer) => { + const dir = mkdtempSync(join(tmpdir(), "pr-merge-pm-cover-")); + try { + const bin = join(dir, "bin"); + mkdirSync(bin); + writeFileSync( + join(bin, "gh"), + `#!/usr/bin/env bash +inc=; args=() +for a in "$@"; do if [ "$a" = -i ]; then inc=1; else args+=("$a"); fi; done +echo "\${args[*]}" >>"${dir}/calls" +if [ "${answer}" = limited ]; then [ -z "$inc" ] || printf '${LIMITED_403}'; echo '${RATE_LIMITED_STDERR}' >&2; exit 1; fi +[ -z "$inc" ] || printf 'HTTP/2.0 502 Bad Gateway\\r\\n\\r\\n'; echo '{}'; echo "gh: Server Error (HTTP 502)" >&2; exit 1 +`, + ); + chmodSync(join(bin, "gh"), 0o755); + symlinkSync(root, join(dir, ".base")); + const result = spawnSync("bash", ["-c", runScript("Read main's full-run cover", earlySteps())], { + cwd: dir, + encoding: "utf8", + env: { ...process.env, PATH: `${bin}:${process.env.PATH}`, RUNNER_TEMP: dir, GITHUB_OUTPUT: join(dir, "output"), REPO: "o/r", TIP: ALERT_SHA, CAP_MINUTES: "30" }, + }); + const read = (name) => (existsSync(join(dir, name)) ? readFileSync(join(dir, name), "utf8") : ""); + return { status: result.status, out: result.stdout + result.stderr, output: read("output"), calls: read("calls").split("\n").filter(Boolean) }; + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}; + +// stop-on-limit (docs/conventions/pr-pipeline/README.md, rate-stop.bash): a +// cover read refused for a rate limit stops the step red with the stop as the +// job's rate-limited output, and no later step of the job calls GitHub. +test("a rate-limited cover read stops post-merge-dispatch red, and nothing after it calls GitHub", () => { + const limited = runPostMergeCover("limited"); + assert.equal(limited.status, 1, limited.out); + assert.equal(limited.calls.length, 1); + assert.match(limited.out, /::error title=GitHub rate limit::the read of main's full runs was rate limited \(the core pool until 2100-01-01T00:00:00Z\)/); + assert.match(limited.output, /^rate-limited=true$/m); + assert.match(limited.output, /^stop=the core pool until 2100-01-01T00:00:00Z$/m); + // Any other failed read is no stop: no cap applies, as before. + const other = runPostMergeCover("x"); + assert.equal(other.status, 0, other.out); + assert.doesNotMatch(other.output, /rate-limited|stop=/); + assert.match(other.out, /::warning::the read of main's full runs failed; no cap applies to this run/); + + const list = earlySteps(); + const live = { "steps.kill.outputs.proceed": "true", "steps.mode.outputs.live": "true", "steps.post-merge.outputs.code": "0", "steps.post-merge.outputs.needs-dispatch": "true", "steps.post-merge.outputs.lane-rerun": "11", "steps.cover.outputs.uncovered": "true" }; + const after = ["Start the full CI run on main", "Re-run a red lane-dispatched full run", "Send a red full run to the demotion alert"]; + // Control: with the read answered, each later API step runs. + for (const name of after) assert.equal(stepRuns(step(name, list), (p) => live[p] ?? "", "success"), true, name); + const stopped = { ...live, "steps.cover.outputs.rate-limited": "true", "steps.cover.outputs.stop": "the core pool until 2100-01-01T00:00:00Z" }; + for (const name of after) assert.equal(stepRuns(step(name, list), (p) => stopped[p] ?? "", "failure"), false, name); + // The merge and enable-auto-merge jobs stop on that output. + for (const id of ["merge", "enable-auto-merge"]) { + const first = stepsOf(jobBlock(id)).find((s) => s.name === "Stop on post-merge-dispatch's rate limit"); + assert.ok(first, id); + assert.match(first.block, /\n {8}if: needs\.post-merge-dispatch\.outputs\.rate-limited != ''\n/); + } +}); + +// Review decision on #10491: post-merge-dispatch reads the cover, dispatches +// and sends to the alert only with the kill switch off, and reads the cover +// and dispatches only while the lane is live (report-only off), as the +// post-merge check did. dispatch-full-run's push carve-out stays. +test("the kill switch stops the scheduled job's cover read, dispatch and alert send", () => { + const list = earlySteps(); + const gated = ["Read main's full-run cover", "Start the full CI run on main", "Send a red full run to the demotion alert"]; + const base = { "steps.cover.outputs.uncovered": "true", "steps.post-merge.outputs.needs-dispatch": "true" }; + const killed = { ...base, "steps.kill.outputs.proceed": "false" }; + for (const name of gated) assert.equal(stepRuns(step(name, list), (p) => killed[p] ?? "", "success"), false, `${name} with the switch on`); + const reportOnly = { ...base, "steps.kill.outputs.proceed": "true" }; + assert.equal(stepRuns(step("Read main's full-run cover", list), (p) => reportOnly[p] ?? "", "success"), false, "cover while report-only"); + assert.equal(stepRuns(step("Start the full CI run on main", list), (p) => reportOnly[p] ?? "", "success"), false, "dispatch while report-only"); + assert.equal(stepRuns(step("Send a red full run to the demotion alert", list), (p) => reportOnly[p] ?? "", "success"), true, "the alert merges nothing"); + const live = { ...reportOnly, "steps.mode.outputs.live": "true" }; + for (const name of gated) assert.equal(stepRuns(step(name, list), (p) => live[p] ?? "", "success"), true, `${name} while live`); +}); + // The merge job's check, run with this run's post-merge-dispatch report kept // in post-merge-kept (or none) and a fake python3: `settle` answers whether // the report stands (exit `settle`), any other call is the check, which marks diff --git a/.github/scripts/pr-merge/workflow.test.mjs b/.github/scripts/pr-merge/workflow.test.mjs index cc29f1d263..1f097fb274 100644 --- a/.github/scripts/pr-merge/workflow.test.mjs +++ b/.github/scripts/pr-merge/workflow.test.mjs @@ -77,7 +77,7 @@ const coverBody = (id) => { const block = job(id).split("\n - name: Read main's full-run cover\n")[1].split("\n - name: ")[0]; return { block, body: runBody(block) }; }; -function runCover(runs, { jobId = "dispatch-full-run", fails = false, tip = HEAD } = {}) { +function runCover(runs, { jobId = "dispatch-full-run", fails = false, limited = false, tip = HEAD } = {}) { const dir = mkdtempSync(join(tmpdir(), "pr-merge-cover-")); mkdirSync(join(dir, "bin")); const now = Date.now(); @@ -98,6 +98,7 @@ function runCover(runs, { jobId = "dispatch-full-run", fails = false, tip = HEAD `#!/usr/bin/env bash printf '%s\\n' "$*" >>"${dir}/log" ${fails ? `echo 'gh: Server Error (HTTP 502)' >&2; exit 1` : ""} +${limited ? `echo '${RATE_LIMITED_STDERR}' >&2; exit 1` : ""} while [ $# -gt 0 ]; do if [ "$1" = --jq ]; then exec jq -c "$2" "${dir}/listing.json"; fi; shift; done exit 1 `, @@ -126,15 +127,24 @@ exit 1 out: result.stdout + result.stderr, capped: /^capped=true$/m.test(output), uncovered: /^uncovered=true$/m.test(output), + limited: /^rate-limited=true$/m.test(output), writes: log.split("\n").filter((l) => l.includes("-X")), }; } const OLDER = "89abcdef0123456789abcdef0123456789abcdef"; -test("both jobs read main's cover with the same step body, a 30-minute cap and the run's own SHA as the tip", () => { +// The two copies differ only in how a failed read is classified: +// post-merge-dispatch sources the shared rate guard from its checkout, +// dispatch-full-run has none and matches the pattern the guard reads. +test("both jobs read main's cover with the same cap logic, a 30-minute cap and the run's own SHA as the tip", () => { const a = coverBody("dispatch-full-run"); const b = coverBody("post-merge-dispatch"); - assert.equal(a.body, b.body); + const afterRead = (body) => body.slice(body.indexOf('\n echo "::warning::the read of main\'s full runs failed')); + assert.equal(afterRead(a.body), afterRead(b.body)); + assert.match(a.body, /grep -qiE 'rate limit\|RATE_LIMITED\|abuse detection\|HTTP 429' "\$RUNNER_TEMP\/cover-stderr"/); + assert.match(b.body, /\. \.base\/\.github\/scripts\/pr-merge\/rate-stop\.bash\n/); + assert.match(b.body, /stop=\$\(rate_stop_from "\$RUNNER_TEMP\/cover-stderr" /); + for (const body of [a.body, b.body]) assert.match(body, /echo "rate-limited=true" >>"\$GITHUB_OUTPUT"\n(?: +echo "stop=\$stop" >>"\$GITHUB_OUTPUT"\n)? +exit 1\n/); for (const { block } of [a, b]) { assert.match(block, /\n {10}CAP_MINUTES: 30\n/); assert.match(block, /\n {10}TIP: \$\{\{ github\.sha \}\}\n/); @@ -159,7 +169,7 @@ test("a full run of main older than 30 minutes caps nothing, and a run at the ti const atTip = runCover([{ minutesAgo: 45, conclusion: "success" }]); assert.equal(atTip.capped, false); assert.equal(atTip.uncovered, false); - assert.deepEqual(runCover([]), { status: 0, out: "", capped: false, uncovered: true, writes: [] }); + assert.deepEqual(runCover([]), { status: 0, out: "", capped: false, uncovered: true, limited: false, writes: [] }); }); // A cancelled run is no cover: replaced before it started, or cancelled once @@ -180,9 +190,24 @@ test("a failed read sets neither output, so the run dispatches as with no cap", assert.equal(r.status, 0); assert.equal(r.capped, false); assert.equal(r.uncovered, false); + assert.equal(r.limited, false); assert.match(r.out, /::warning::the read of main's full runs failed; no cap applies to this run/); }); +// stop-on-limit (docs/conventions/pr-pipeline/README.md): a read refused for a +// rate limit is no ordinary failure. The step stops red with rate-limited=true +// and the dispatch after it sends nothing. +test("a rate-limited read stops dispatch-full-run red, and the dispatch is skipped", () => { + const r = runCover([{ minutesAgo: 5 }], { limited: true }); + assert.equal(r.status, 1, r.out); + assert.equal(r.limited, true); + assert.equal(r.capped, false); + assert.match(r.out, /::error title=GitHub rate limit::the read of main's full runs was rate limited; nothing more is sent/); + assert.doesNotMatch(r.out, /no cap applies/); + const text = stepIf("dispatch-full-run", "Start the full CI run on main"); + assert.equal(evalStep(text, outs({ limited: "true", uncovered: "true" })), false); +}); + // The dispatch steps' conditions, evaluated over the cover and post-merge // outputs: dispatch-full-run starts nothing within the cap; post-merge-dispatch // starts main's tip when no run covers it, for any merge, and keeps the lane's @@ -194,8 +219,10 @@ const stepIf = (id, name) => { }; const evalStep = (text, steps) => new Function("steps", `return (${text.replace(/\.([a-z]+(?:-[a-z]+)+)/g, '["$1"]')});`)(steps); -const outs = ({ capped = "", uncovered = "", needs = "false" } = {}) => ({ - cover: { outputs: { capped, uncovered } }, +const outs = ({ capped = "", uncovered = "", needs = "false", limited = "", proceed = "true", live = "true" } = {}) => ({ + kill: { outputs: { proceed } }, + mode: { outputs: { live } }, + cover: { outputs: { capped, uncovered, "rate-limited": limited } }, "post-merge": { outputs: { "needs-dispatch": needs } }, }); @@ -215,19 +242,24 @@ test("post-merge-dispatch starts main's uncovered tip for any merge, outside the assert.equal(evalStep(expr, outs({ needs: "true" })), true, "the lane's own request"); assert.equal(evalStep(expr, outs({ needs: "true", capped: "true" })), false); assert.equal(evalStep(expr, outs()), false, "a covered tip"); + assert.equal(evalStep(expr, outs({ uncovered: "true", limited: "true" })), false, "a rate-limited cover read"); + assert.equal(evalStep(expr, outs({ uncovered: "true", proceed: "false", live: "" })), false, "the kill switch on"); + assert.equal(evalStep(expr, outs({ uncovered: "true", live: "" })), false, "report-only on"); }); -// The backstop must not depend on the lane's switches: dispatch-full-run -// starts a run for every merge while the kill switch or report-only is on, -// so the step that covers a merge the cap skipped reads the cover the same -// way, after the steps those switches skip. -test("post-merge-dispatch reads the cover whatever the kill switch and report-only say", () => { +// Review decision on #10491: the scheduled job reads the cover only while the +// lane is live (kill switch off, report-only off), as its post-merge check +// does; dispatch-full-run's push carve-out still starts a run per merge. +test("post-merge-dispatch reads the cover only while the lane is live", () => { const dispatch = job("post-merge-dispatch"); const block = dispatch.split("\n - name: Read main's full-run cover\n")[1].split("\n - name: ")[0]; - assert.doesNotMatch(block, /\n {8}if:/); + assert.match(block, /\n {8}if: steps\.kill\.outputs\.proceed == 'true' && steps\.mode\.outputs\.live == 'true'\n/); const names = [...dispatch.matchAll(/\n {6}- name: (.*)/g)].map((m) => m[1]); assert.equal(names.indexOf("Read main's full-run cover") + 1, names.indexOf("Start the full CI run on main")); assert.ok(names.indexOf("Check earlier lane merges") < names.indexOf("Read main's full-run cover")); + const cover = job("dispatch-full-run").split("\n - name: Read main's full-run cover\n")[1].split("\n - name: ")[0]; + assert.doesNotMatch(cover, /\n {8}if:/); + assert.doesNotMatch(job("dispatch-full-run"), /steps\.(kill|mode)\./); }); test("the dispatch job's token is actions: write and nothing else", () => { diff --git a/.github/workflows/pr-merge.yml b/.github/workflows/pr-merge.yml index 543334b281..dc427b9671 100644 --- a/.github/workflows/pr-merge.yml +++ b/.github/workflows/pr-merge.yml @@ -39,9 +39,9 @@ name: pr-merge # dispatch-full-run job), even while the switch or report-only is on, since it # merges nothing; at most one per 30 minutes, and a merge that cap skips gets # its run of main's tip from the post-merge-dispatch job on the next scheduled -# run. That run raises no workflow_run event (GITHUB_TOKEN started it), so -# when it ends red the post-merge-dispatch job on the next scheduled run sends -# its run id and head SHA to +# run while the lane is live. That run raises no workflow_run event +# (GITHUB_TOKEN started it), so when it ends red the post-merge-dispatch job +# on the next scheduled run with the switch off sends its run id and head SHA to # post-merge-demotion-alert.yml through workflow_dispatch, which files the # post-merge record and fix issues and reports the red-main culprit once the # re-run fails too. A red run this workflow dispatched gets its one re-run @@ -132,7 +132,7 @@ jobs: outputs: # The pool and reset a rate limit stopped this job's GITHUB_TOKEN calls on, # if one did: the merge and enable-auto-merge jobs then send nothing. - rate-limited: ${{ steps.post-merge.outputs.rate-limited || steps.dispatch.outputs.stop || steps.rerun.outputs.stop || steps.alert.outputs.stop }} + rate-limited: ${{ steps.post-merge.outputs.rate-limited || steps.cover.outputs.stop || steps.dispatch.outputs.stop || steps.rerun.outputs.stop || steps.alert.outputs.stop }} steps: - name: Check out the default branch tip uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -252,10 +252,15 @@ jobs: if-no-files-found: error # The same body as dispatch-full-run's step of this name; see there. - # It runs whether or not the post-merge check did: a merge the cap - # skipped gets its run here, while the kill switch is on too. + # It runs whether or not the post-merge check did, while the lane is + # live: a merge the cap skipped gets its run here. With the kill switch + # or report-only on, this job sends nothing; dispatch-full-run's push + # carve-out still starts a run per merge outside the cap. A read stopped + # on a rate limit stops the step red, as the alert step's, and its stop + # is the job's rate-limited output: nothing after it calls GitHub. - name: Read main's full-run cover id: cover + if: steps.kill.outputs.proceed == 'true' && steps.mode.outputs.live == 'true' env: GH_TOKEN: ${{ github.token }} REPO: ${{ github.repository }} @@ -263,9 +268,19 @@ jobs: CAP_MINUTES: 30 run: | set -euo pipefail + . .base/.github/actions/log-api-budget/api-budget.bash + . .base/.github/scripts/pr-merge/rate-stop.bash + api_budget_wrap_gh github-token if ! runs=$(gh api "repos/$REPO/actions/workflows/pr-require-checks.yml/runs?event=workflow_dispatch&branch=main&per_page=20" \ --jq '[.workflow_runs[] | select(.actor.login == "github-actions[bot]" and .head_branch == "main" and .conclusion != "cancelled") | {sha: .head_sha, created: .created_at}]' 2>"$RUNNER_TEMP/cover-stderr"); then cat "$RUNNER_TEMP/cover-stderr" >&2 + stop=$(rate_stop_from "$RUNNER_TEMP/cover-stderr" "the read of main's full runs") + if [ -n "$stop" ]; then + echo "::error title=GitHub rate limit::the read of main's full runs was rate limited ($stop); nothing more is sent" + echo "rate-limited=true" >>"$GITHUB_OUTPUT" + echo "stop=$stop" >>"$GITHUB_OUTPUT" + exit 1 + fi echo "::warning::the read of main's full runs failed; no cap applies to this run" exit 0 fi @@ -292,7 +307,10 @@ jobs: - name: Start the full CI run on main id: dispatch if: >- - steps.cover.outputs.capped != 'true' + steps.kill.outputs.proceed == 'true' + && steps.mode.outputs.live == 'true' + && steps.cover.outputs.rate-limited != 'true' + && steps.cover.outputs.capped != 'true' && (steps.post-merge.outputs.needs-dispatch == 'true' || steps.cover.outputs.uncovered == 'true') env: GH_TOKEN: ${{ github.token }} @@ -406,6 +424,7 @@ jobs: id: rerun if: >- !cancelled() + && steps.cover.outputs.rate-limited != 'true' && steps.dispatch.outputs.rate-limited != 'true' && contains(fromJSON('["0","1","5"]'), steps.post-merge.outputs.code) && steps.post-merge.outputs.lane-rerun != '' @@ -508,17 +527,22 @@ jobs: # and a second red run goes on the next run. Red is attempt 1 concluding # in babysit_postmerge.py's FAILED_CONCLUSIONS, except a run the # concurrency group replaced before it started (cancelled, no job). Any - # merge, whoever merged and whatever the kill switch says, as - # dispatch-full-run starts the run. A rate limit stops the step red, - # as the re-run step's; any other failed read or send sends nothing more - # and leaves the run to the next one. LOOKBACK_HOURS: a red run is sent - # within 15 minutes of its end; 6 hours outlasts a day of missed - # schedules many times over (judgment). + # merge, whoever merged, as dispatch-full-run starts the run, but only + # while the kill switch is off: with it on this job sends nothing, and a + # red run is sent on the first run after it clears, within + # LOOKBACK_HOURS. Report-only does not stop it, since the alert merges + # nothing. A rate limit stops the step red, as the re-run step's, and + # one the cover read stopped on skips it; any other failed read or send + # sends nothing more and leaves the run to the next one. LOOKBACK_HOURS: + # a red run is sent within 15 minutes of its end; 6 hours outlasts a day + # of missed schedules many times over (judgment). - name: Send a red full run to the demotion alert id: alert if: >- !cancelled() + && steps.kill.outputs.proceed == 'true' && steps.post-merge.outputs.code != '75' + && steps.cover.outputs.rate-limited != 'true' && steps.dispatch.outputs.rate-limited != 'true' && steps.rerun.outputs.rate-limited != 'true' env: @@ -2088,11 +2112,15 @@ jobs: # started, skipping a cancelled one (replaced before it started, or # cancelled later, which reads red): capped=true when one was created # within CAP_MINUTES, uncovered=true when none is at TIP. A failed read - # sets neither, so the run dispatches as it would with no cap. A merge - # the cap skips is covered by the next push after the window or by - # post-merge-dispatch on the next scheduled run, whose dispatch starts - # main's tip once nothing covers it, whoever merged and whatever the - # kill switch says. CAP_MINUTES: the efficiency plan of 2026-10-26 + # sets neither, so the run dispatches as it would with no cap, except a + # read refused for a rate limit, which stops the step red with + # rate-limited=true and sends nothing more. This job has no checkout of + # the shared guard, so the match is the pattern rate-stop.bash's + # rate_stop_from reads; post-merge-dispatch's copy of this step sources + # that guard. A merge the cap skips is covered by the next push after + # the window or, while the lane is live, by post-merge-dispatch on the + # next scheduled run, whose dispatch starts main's tip once nothing + # covers it, whoever merged. CAP_MINUTES: the efficiency plan of 2026-10-26 # (item 5, owner decision): 116 runs a day at a median 84.1 runner-min # become at most 48. - name: Read main's full-run cover @@ -2107,6 +2135,11 @@ jobs: if ! runs=$(gh api "repos/$REPO/actions/workflows/pr-require-checks.yml/runs?event=workflow_dispatch&branch=main&per_page=20" \ --jq '[.workflow_runs[] | select(.actor.login == "github-actions[bot]" and .head_branch == "main" and .conclusion != "cancelled") | {sha: .head_sha, created: .created_at}]' 2>"$RUNNER_TEMP/cover-stderr"); then cat "$RUNNER_TEMP/cover-stderr" >&2 + if grep -qiE 'rate limit|RATE_LIMITED|abuse detection|HTTP 429' "$RUNNER_TEMP/cover-stderr"; then + echo "::error title=GitHub rate limit::the read of main's full runs was rate limited; nothing more is sent" + echo "rate-limited=true" >>"$GITHUB_OUTPUT" + exit 1 + fi echo "::warning::the read of main's full runs failed; no cap applies to this run" exit 0 fi @@ -2141,7 +2174,7 @@ jobs: # record upload; 180 s of the rest. - name: Start the full CI run on main id: dispatch - if: steps.cover.outputs.capped != 'true' + if: steps.cover.outputs.rate-limited != 'true' && steps.cover.outputs.capped != 'true' env: GH_TOKEN: ${{ github.token }} REPO: ${{ github.repository }} diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index 137edbbd14..b4a5a27321 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -2575,8 +2575,8 @@ tests are in The full run pr-merge starts after each merge uses `GITHUB_TOKEN`, so its completion raises no `workflow_run` event. When that run ends red, pr-merge's `post-merge-dispatch` job sends its run id -to the alert workflow through `workflow_dispatch` on its next scheduled run, up to 15 minutes -later. A dispatch `pr-require-checks.yml`'s main-dispatch concurrency group replaced before it +to the alert workflow through `workflow_dispatch` on its next scheduled run with the kill switch +off, up to 15 minutes later. A dispatch `pr-require-checks.yml`'s main-dispatch concurrency group replaced before it started (cancelled, no job) is no signal: it is never sent, and neither the alert nor the post-merge check reads it as red. The alert accepts the id only from `github-actions[bot]` or a listed human (`trusted-actors.json` on `main`), never the lanes App, From b223bd5de6e99da04145412ffe007c5c1847c2da Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 27 Oct 2026 15:46:47 -0400 Subject: [PATCH 3/5] fix(pr-merge): stop the scheduled alert send while report-only is on The kill switch and report-only now stop everything post-merge-dispatch does that spends API or starts a run: the cover read, the pr-require-checks.yml dispatch on main and the alert dispatch. The alert step had only the kill-switch condition; it now also needs the lane live. dispatch-full-run's push carve-out stays. A test runs the kill-switch action with CLAUDE_LANES_DISABLED and with CLAUDE_LANES_REPO_DISABLED set, then evaluates every later step of the scheduled job over its output: none that starts a full run or sends an alert runs. Co-Authored-By: Claude Opus 5.5 --- ...perf-main-whole-suite-batching-9548bbe5.md | 2 +- .github/scripts/pr-merge/merge-job.test.mjs | 74 ++++++++++++++----- .github/workflows/pr-merge.yml | 13 ++-- docs/conventions/pr-pipeline/README.md | 4 +- 4 files changed, 67 insertions(+), 26 deletions(-) diff --git a/.changes/pr-pipeline/perf-main-whole-suite-batching-9548bbe5.md b/.changes/pr-pipeline/perf-main-whole-suite-batching-9548bbe5.md index 3952c33e3c..85552a1f5d 100644 --- a/.changes/pr-pipeline/perf-main-whole-suite-batching-9548bbe5.md +++ b/.changes/pr-pipeline/perf-main-whole-suite-batching-9548bbe5.md @@ -5,6 +5,6 @@ bump: patch ### Changed - pr-merge starts at most one full run of main per 30 minutes, of main's tip, covering every merge since the last. dispatch-full-run and post-merge-dispatch read the newest pr-require-checks.yml dispatches on main first (`Read main's full-run cover`): within the cap they start nothing, and post-merge-dispatch starts one on its next scheduled run for a tip no full run covers, whoever merged, while the lane is live (kill switch and report-only off). A read of the cover refused for a rate limit stops the job red and sends nothing more (`.github/workflows/pr-merge.yml`). -- dispatch-full-run ends once it dispatches, where it held a runner up to 39 minutes polling the run; its timeout drops from 45 to 10 minutes. post-merge-dispatch now sends a red full run to post-merge-demotion-alert.yml on its next scheduled run with the kill switch off, up to 15 minutes after the run ends, once per run (`pr-merge-alerted-`), and skips a run that never started. +- dispatch-full-run ends once it dispatches, where it held a runner up to 39 minutes polling the run; its timeout drops from 45 to 10 minutes. post-merge-dispatch now sends a red full run to post-merge-demotion-alert.yml on its next scheduled run while the lane is live (kill switch and report-only off), up to 15 minutes after the run ends, once per run (`pr-merge-alerted-`), and skips a run that never started. - pr-require-checks.yml's dispatches on main (attempt 1) share one concurrency group with no `queue` key and `cancel-in-progress` false: one runs, one waits, and a newer one replaces the waiting one. A re-run and every other run keep their own group and cancel. - post-merge-demotion-alert.yml files nothing for a cancelled full run with no job, one the concurrency group replaced before it started. diff --git a/.github/scripts/pr-merge/merge-job.test.mjs b/.github/scripts/pr-merge/merge-job.test.mjs index 13062c8fc9..dcbae3de3e 100644 --- a/.github/scripts/pr-merge/merge-job.test.mjs +++ b/.github/scripts/pr-merge/merge-job.test.mjs @@ -1717,15 +1717,14 @@ test("a rate-limited read or send stops red with the stop as the job's output, s // Every merge's run is sent, as dispatch-full-run starts it, whoever merged, // after a dispatch or re-run that failed for any reason but a rate limit, and -// only while the kill switch is off (review decision on #10491: the scheduled -// job acts only with the switch off; report-only merges nothing, so the alert -// still runs under it). -test("the alert send runs after the re-run while the switch is off, and stops after a rate limit", () => { +// only while the lane is live (review decision on #10491: the kill switch and +// report-only stop everything the scheduled job spends API on). +test("the alert send runs after the re-run while the lane is live, and stops after a rate limit", () => { const list = earlySteps(); const alert = step("Send a red full run to the demotion alert", list); assert.match( alert, - /\n {8}if: >-\n {10}!cancelled\(\)\n {10}&& steps\.kill\.outputs\.proceed == 'true'\n {10}&& steps\.post-merge\.outputs\.code != '75'\n {10}&& steps\.cover\.outputs\.rate-limited != 'true'\n {10}&& steps\.dispatch\.outputs\.rate-limited != 'true'\n {10}&& steps\.rerun\.outputs\.rate-limited != 'true'\n/, + /\n {8}if: >-\n {10}!cancelled\(\)\n {10}&& steps\.kill\.outputs\.proceed == 'true'\n {10}&& steps\.mode\.outputs\.live == 'true'\n {10}&& steps\.post-merge\.outputs\.code != '75'\n {10}&& steps\.cover\.outputs\.rate-limited != 'true'\n {10}&& steps\.dispatch\.outputs\.rate-limited != 'true'\n {10}&& steps\.rerun\.outputs\.rate-limited != 'true'\n/, ); const names = list.map((s) => s.name); assert.deepEqual(names.slice(-2), ["Send a red full run to the demotion alert", "Record the alert sent"]); @@ -1794,22 +1793,63 @@ test("a rate-limited cover read stops post-merge-dispatch red, and nothing after } }); -// Review decision on #10491: post-merge-dispatch reads the cover, dispatches -// and sends to the alert only with the kill switch off, and reads the cover -// and dispatches only while the lane is live (report-only off), as the -// post-merge check did. dispatch-full-run's push carve-out stays. -test("the kill switch stops the scheduled job's cover read, dispatch and alert send", () => { +// Review decision on #10491: the kill switch and report-only stop everything +// post-merge-dispatch does that spends API or starts a run: the cover read, +// the pr-require-checks.yml dispatch on main and the alert dispatch. +// dispatch-full-run's push carve-out stays. +const spending = [ + "Read main's full-run cover", + "Start the full CI run on main", + "Record the dispatched run", + "Re-run a red lane-dispatched full run", + "Record the re-run", + "Send a red full run to the demotion alert", + "Record the alert sent", +]; +test("the kill switch and report-only stop the scheduled job's cover read, dispatch and alert send", () => { const list = earlySteps(); - const gated = ["Read main's full-run cover", "Start the full CI run on main", "Send a red full run to the demotion alert"]; const base = { "steps.cover.outputs.uncovered": "true", "steps.post-merge.outputs.needs-dispatch": "true" }; - const killed = { ...base, "steps.kill.outputs.proceed": "false" }; - for (const name of gated) assert.equal(stepRuns(step(name, list), (p) => killed[p] ?? "", "success"), false, `${name} with the switch on`); const reportOnly = { ...base, "steps.kill.outputs.proceed": "true" }; - assert.equal(stepRuns(step("Read main's full-run cover", list), (p) => reportOnly[p] ?? "", "success"), false, "cover while report-only"); - assert.equal(stepRuns(step("Start the full CI run on main", list), (p) => reportOnly[p] ?? "", "success"), false, "dispatch while report-only"); - assert.equal(stepRuns(step("Send a red full run to the demotion alert", list), (p) => reportOnly[p] ?? "", "success"), true, "the alert merges nothing"); + for (const name of spending) assert.equal(stepRuns(step(name, list), (p) => reportOnly[p] ?? "", "success"), false, `${name} while report-only`); const live = { ...reportOnly, "steps.mode.outputs.live": "true" }; - for (const name of gated) assert.equal(stepRuns(step(name, list), (p) => live[p] ?? "", "success"), true, `${name} while live`); + for (const name of ["Read main's full-run cover", "Start the full CI run on main", "Send a red full run to the demotion alert"]) { + assert.equal(stepRuns(step(name, list), (p) => live[p] ?? "", "success"), true, `${name} while live`); + } +}); + +// The kill switch's own action, run with CLAUDE_LANES_DISABLED or +// CLAUDE_LANES_REPO_DISABLED set, then every later step of the scheduled job +// evaluated over its output: no step that starts a pr-require-checks.yml run +// or sends to the alert runs, nor the steps those switches skip. +test("with either kill switch set, the scheduled job starts no full run and sends no alert", () => { + const list = earlySteps(); + const killAt = list.findIndex((s) => s.name === "Check the kill switch"); + assert.match(list[killAt].block, /\n {10}value: \$\{\{ vars\.CLAUDE_LANES_DISABLED \}\}\n {10}repo-value: \$\{\{ vars\.CLAUDE_LANES_REPO_DISABLED \}\}\n/); + const killSwitch = (org, repo) => { + const dir = mkdtempSync(join(tmpdir(), "pr-merge-kill-")); + try { + const out = join(dir, "output"); + writeFileSync(out, ""); + execFileSync("node", [join(root, ".github/actions/check-kill-switch/check-kill-switch.mjs")], { + env: { ...process.env, KILL_SWITCH_VALUE: org, REPO_KILL_SWITCH_VALUE: repo, GITHUB_OUTPUT: out }, + }); + return /^proceed<<(\S+)\n(.*)\n\1$/m.exec(readFileSync(out, "utf8"))[2]; + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }; + assert.equal(killSwitch("false", "false"), "true", "control: both off proceeds"); + for (const [org, repo] of [["true", "false"], ["false", "true"]]) { + const proceed = killSwitch(org, repo); + assert.equal(proceed, "false", `CLAUDE_LANES_DISABLED=${org} CLAUDE_LANES_REPO_DISABLED=${repo}`); + const context = (p) => (p === "steps.kill.outputs.proceed" ? proceed : ""); + for (const { name, block } of list.slice(killAt + 1)) { + if (["Check earlier lane merges", "Read the lane records the last run kept", "Read report-only", ...spending].includes(name)) { + assert.equal(stepRuns(block, context, "success"), false, `${name} with ${org}/${repo}`); + } + } + } + assert.doesNotMatch(jobBlock("dispatch-full-run"), /steps\.(kill|mode)\./, "the push carve-out stays"); }); // The merge job's check, run with this run's post-merge-dispatch report kept diff --git a/.github/workflows/pr-merge.yml b/.github/workflows/pr-merge.yml index dc427b9671..ed50b486ad 100644 --- a/.github/workflows/pr-merge.yml +++ b/.github/workflows/pr-merge.yml @@ -41,7 +41,7 @@ name: pr-merge # its run of main's tip from the post-merge-dispatch job on the next scheduled # run while the lane is live. That run raises no workflow_run event # (GITHUB_TOKEN started it), so when it ends red the post-merge-dispatch job -# on the next scheduled run with the switch off sends its run id and head SHA to +# on the next scheduled run while the lane is live sends its run id and head SHA to # post-merge-demotion-alert.yml through workflow_dispatch, which files the # post-merge record and fix issues and reports the red-main culprit once the # re-run fails too. A red run this workflow dispatched gets its one re-run @@ -254,7 +254,8 @@ jobs: # The same body as dispatch-full-run's step of this name; see there. # It runs whether or not the post-merge check did, while the lane is # live: a merge the cap skipped gets its run here. With the kill switch - # or report-only on, this job sends nothing; dispatch-full-run's push + # or report-only on, this job reads and sends nothing that spends API or + # starts a run; dispatch-full-run's push # carve-out still starts a run per merge outside the cap. A read stopped # on a rate limit stops the step red, as the alert step's, and its stop # is the job's rate-limited output: nothing after it calls GitHub. @@ -528,10 +529,9 @@ jobs: # in babysit_postmerge.py's FAILED_CONCLUSIONS, except a run the # concurrency group replaced before it started (cancelled, no job). Any # merge, whoever merged, as dispatch-full-run starts the run, but only - # while the kill switch is off: with it on this job sends nothing, and a - # red run is sent on the first run after it clears, within - # LOOKBACK_HOURS. Report-only does not stop it, since the alert merges - # nothing. A rate limit stops the step red, as the re-run step's, and + # while the lane is live: with the kill switch or report-only on this + # job reads and sends nothing, and a red run is sent on the first run + # after both clear, within LOOKBACK_HOURS. A rate limit stops the step red, as the re-run step's, and # one the cover read stopped on skips it; any other failed read or send # sends nothing more and leaves the run to the next one. LOOKBACK_HOURS: # a red run is sent within 15 minutes of its end; 6 hours outlasts a day @@ -541,6 +541,7 @@ jobs: if: >- !cancelled() && steps.kill.outputs.proceed == 'true' + && steps.mode.outputs.live == 'true' && steps.post-merge.outputs.code != '75' && steps.cover.outputs.rate-limited != 'true' && steps.dispatch.outputs.rate-limited != 'true' diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index b4a5a27321..0143c5c0bb 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -2575,8 +2575,8 @@ tests are in The full run pr-merge starts after each merge uses `GITHUB_TOKEN`, so its completion raises no `workflow_run` event. When that run ends red, pr-merge's `post-merge-dispatch` job sends its run id -to the alert workflow through `workflow_dispatch` on its next scheduled run with the kill switch -off, up to 15 minutes later. A dispatch `pr-require-checks.yml`'s main-dispatch concurrency group replaced before it +to the alert workflow through `workflow_dispatch` on its next scheduled run while the lane is +live (kill switch and report-only off), up to 15 minutes later. A dispatch `pr-require-checks.yml`'s main-dispatch concurrency group replaced before it started (cancelled, no job) is no signal: it is never sent, and neither the alert nor the post-merge check reads it as red. The alert accepts the id only from `github-actions[bot]` or a listed human (`trusted-actors.json` on `main`), never the lanes App, From 157ea419b2bdde749c0c0adc0c30f4a7d5cb5ace Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 27 Oct 2026 16:23:02 -0400 Subject: [PATCH 4/5] fix(pr-merge): count an alert record only from pr-merge.yml's own run on main The alert step treated any artifact named pr-merge-alerted- as a send already made, whoever uploaded it, so a pull request run could suppress a red run's alert. It now asks babysit_postmerge.py --lane-alerted, which accepts the record only from a sender the dispatch and re-run records' rule accepts (_lane_sender: this repository's pr-merge.yml, a schedule or workflow_dispatch run, on main, head commit on main's tip). _lane_record and the new lane_alerted share one _lane_proof helper. Exit 75 stops the step red as a rate limit; any other failed read sends nothing for that run, as before. Co-Authored-By: Claude Opus 5.5 --- ...perf-main-whole-suite-batching-9548bbe5.md | 2 +- ...perf-main-whole-suite-batching-3c4daaf0.md | 4 + .github/scripts/pr-merge/merge-job.test.mjs | 52 ++++++++++- .github/workflows/pr-merge.yml | 29 ++++-- .../babysit-prs/scripts/babysit_postmerge.py | 91 +++++++++++++++---- .../scripts/tests/test_babysit_postmerge.py | 67 ++++++++++++++ 6 files changed, 217 insertions(+), 28 deletions(-) diff --git a/.changes/pr-pipeline/perf-main-whole-suite-batching-9548bbe5.md b/.changes/pr-pipeline/perf-main-whole-suite-batching-9548bbe5.md index 85552a1f5d..634aa3c64c 100644 --- a/.changes/pr-pipeline/perf-main-whole-suite-batching-9548bbe5.md +++ b/.changes/pr-pipeline/perf-main-whole-suite-batching-9548bbe5.md @@ -5,6 +5,6 @@ bump: patch ### Changed - pr-merge starts at most one full run of main per 30 minutes, of main's tip, covering every merge since the last. dispatch-full-run and post-merge-dispatch read the newest pr-require-checks.yml dispatches on main first (`Read main's full-run cover`): within the cap they start nothing, and post-merge-dispatch starts one on its next scheduled run for a tip no full run covers, whoever merged, while the lane is live (kill switch and report-only off). A read of the cover refused for a rate limit stops the job red and sends nothing more (`.github/workflows/pr-merge.yml`). -- dispatch-full-run ends once it dispatches, where it held a runner up to 39 minutes polling the run; its timeout drops from 45 to 10 minutes. post-merge-dispatch now sends a red full run to post-merge-demotion-alert.yml on its next scheduled run while the lane is live (kill switch and report-only off), up to 15 minutes after the run ends, once per run (`pr-merge-alerted-`), and skips a run that never started. +- dispatch-full-run ends once it dispatches, where it held a runner up to 39 minutes polling the run; its timeout drops from 45 to 10 minutes. post-merge-dispatch now sends a red full run to post-merge-demotion-alert.yml on its next scheduled run while the lane is live (kill switch and report-only off), up to 15 minutes after the run ends, once per run (`pr-merge-alerted-`, counted only when its own run on main uploaded it, by `babysit_postmerge.py --lane-alerted`), and skips a run that never started. - pr-require-checks.yml's dispatches on main (attempt 1) share one concurrency group with no `queue` key and `cancel-in-progress` false: one runs, one waits, and a newer one replaces the waiting one. A re-run and every other run keep their own group and cancel. - post-merge-demotion-alert.yml files nothing for a cancelled full run with no job, one the concurrency group replaced before it started. diff --git a/.changes/source-control/perf-main-whole-suite-batching-3c4daaf0.md b/.changes/source-control/perf-main-whole-suite-batching-3c4daaf0.md index 2e6b5d7502..bcbe98fbb9 100644 --- a/.changes/source-control/perf-main-whole-suite-batching-3c4daaf0.md +++ b/.changes/source-control/perf-main-whole-suite-batching-3c4daaf0.md @@ -5,3 +5,7 @@ bump: patch ### Fixed - The post-merge check (`babysit_postmerge.py`) skips a full run cancelled with no job on attempt 1, one pr-require-checks.yml's main-dispatch concurrency group replaced before it started: it no longer holds a merge as attribution-pending, asks for its re-run, or serves as the baseline of a later red run. A run cancelled after its jobs started still reads red. + +### Added + +- `babysit_postmerge.py --lane-alerted RUN_ID --branch ` exits 0 when pr-merge.yml's post-merge-dispatch on the branch already sent that full run to post-merge-demotion-alert.yml, 1 when not, 3 when unreadable. It counts the `pr-merge-alerted-` record only from a sender the dispatch and re-run records' rule accepts (`_lane_sender`), so a same-name artifact from any other run is no record. diff --git a/.github/scripts/pr-merge/merge-job.test.mjs b/.github/scripts/pr-merge/merge-job.test.mjs index dcbae3de3e..b1ac4caad5 100644 --- a/.github/scripts/pr-merge/merge-job.test.mjs +++ b/.github/scripts/pr-merge/merge-job.test.mjs @@ -1506,6 +1506,20 @@ test("the re-run record is named by run and attempt, uploaded only after a recor // 7(b)): GITHUB_TOKEN started the full run, so it raises no workflow_run // event, and post-merge-demotion-alert.yml hears of a red one only from here. const ALERT_SHA = "0123456789abcdef0123456789abcdef01234567"; +// main's tip, and the run of pr-merge.yml's post-merge-dispatch on it that a +// trusted pr-merge-alerted- record comes from (babysit_postmerge.py +// _lane_sender: this repository's pr-merge.yml, a schedule or +// workflow_dispatch run, head_branch main, head commit on main's tip). +const ALERT_TIP = "fedcba9876543210fedcba9876543210fedcba98"; +const ALERT_SENDER = { + id: 900, + path: ".github/workflows/pr-merge.yml", + event: "schedule", + head_branch: "main", + head_sha: ALERT_TIP, + repository: { full_name: "o/r" }, + head_repository: { full_name: "o/r" }, +}; const alertSend = (id, sha = ALERT_SHA) => `api -X POST repos/o/r/actions/workflows/post-merge-demotion-alert.yml/dispatches -f ref=main -f inputs[run-id]=${id} -f inputs[head-sha]=${sha}`; const runAlert = ({ runs = [], firsts = {}, jobs = {}, sent = {}, post = ["ok"], listing } = {}) => { @@ -1534,8 +1548,17 @@ const runAlert = ({ runs = [], firsts = {}, jobs = {}, sent = {}, post = ["ok"], const n = jobs[r.id] ?? 3; fixture(`jobs.${r.id}`, n === "x" || n === "limited" ? n : { total_count: n, jobs: [] }); const s = sent[r.id] ?? 0; - fixture(`sent.${r.id}`, s === "x" || s === "limited" ? s : { total_count: s, artifacts: [] }); + const record = (sender) => ({ id: 1, name: `pr-merge-alerted-${r.id}`, workflow_run: { id: sender } }); + if (s === "x" || s === "limited") fixture(`sent.${r.id}`, s); + else if (typeof s === "number") fixture(`sent.${r.id}`, { total_count: s, artifacts: Array.from({ length: s }, () => record(900)) }); + else { + // One same-name record from run 901: the trusted sender with `s` over it. + fixture(`sent.${r.id}`, { total_count: 1, artifacts: [record(901)] }); + fixture("run.901", { ...ALERT_SENDER, id: 901, ...s }); + } } + fixture("run.900", ALERT_SENDER); + fixture("tip", { object: { sha: ALERT_TIP } }); writeFileSync( join(bin, "gh"), `#!/usr/bin/env bash @@ -1560,6 +1583,8 @@ case "$endpoint" in */attempts/1/jobs*) f=jobs.$(sed -E 's#.*/runs/([0-9]+)/.*#\\1#' <<<"$endpoint") ;; */attempts/1) f=first.$(sed -E 's#.*/runs/([0-9]+)/.*#\\1#' <<<"$endpoint") ;; */artifacts\\?name=pr-merge-alerted-*) f=sent.$(sed -E 's#.*pr-merge-alerted-([0-9]+).*#\\1#' <<<"$endpoint") ;; + repos/o/r/actions/runs/90[01]) f=run.\${endpoint##*/} ;; + repos/o/r/git/ref/heads/main) f=tip ;; *) echo "gh: Not Found (HTTP 404)" >&2; exit 1 ;; esac v=$(cat "${dir}/$f") @@ -1652,6 +1677,31 @@ test("a run past attempt 1 is judged on attempt 1", () => { assert.deepEqual(green.sends, [], green.out); }); +// Review finding on #10491 (claude-security-review): a run sent counts only +// when pr-merge.yml's own post-merge-dispatch on main uploaded its record, by +// babysit_postmerge.py's sender rule (_lane_sender), which this step runs. A +// same-name artifact from any other run, such as a pull request run, does +// not suppress the send. +test("a same-name record from a run that is not pr-merge.yml's on main does not suppress the alert", () => { + const trusted = runAlert({ runs: [{ id: 41, conclusion: "failure" }], sent: { 41: {} } }); + assert.equal(trusted.status, 0, trusted.out); + assert.deepEqual(trusted.sends, [], "control: the lane's own record is a send already made"); + assert.ok(trusted.calls.includes("api repos/o/r/actions/runs/901"), trusted.calls.join("\n")); + for (const forged of [ + { event: "pull_request" }, + { event: "pull_request_target" }, + { event: "push" }, + { path: ".github/workflows/ci.yml" }, + { head_branch: "feature" }, + { head_repository: { full_name: "outsider/r" } }, + { repository: { full_name: "outsider/r" } }, + ]) { + const run = runAlert({ runs: [{ id: 41, conclusion: "failure" }], sent: { 41: forged } }); + assert.equal(run.status, 0, run.out); + assert.deepEqual(run.sends, [alertSend(41)], JSON.stringify(forged)); + } +}); + test("one run per job: the oldest red one unsent, the next on the next run", () => { const run = runAlert({ runs: [ diff --git a/.github/workflows/pr-merge.yml b/.github/workflows/pr-merge.yml index ed50b486ad..aeff8752b5 100644 --- a/.github/workflows/pr-merge.yml +++ b/.github/workflows/pr-merge.yml @@ -596,12 +596,29 @@ jobs: continue fi fi - if ! sent=$(gh api "repos/$REPO/actions/artifacts?name=pr-merge-alerted-$id&per_page=1" --jq .total_count 2>"$err"); then - stopped "the record read of run $id" - echo "::warning::the record read of run $id failed; the next run tries again" - continue - fi - [ "$sent" = 0 ] || continue + # Already sent only when pr-merge.yml's own post-merge-dispatch on + # main uploaded the record (_Context.lane_alerted, the sender rule + # of the dispatch and re-run records): a same-name artifact any + # other run uploads is no record. 0 sent, 1 not, 75 a rate limit. + code=0 + python3 .base/plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py \ + --repo "$REPO" --branch main --lane-alerted "$id" /dev/null 2>"$err" || code=$? + case $code in + 0) continue ;; + 1) ;; + 75) + stopped "the record read of run $id" + echo "::error title=GitHub rate limit::the record read of run $id was rate limited (an unrecorded pool until its reset); nothing more is sent" + echo "rate-limited=true" >>"$GITHUB_OUTPUT" + echo "stop=an unrecorded pool until its reset" >>"$GITHUB_OUTPUT" + exit 1 + ;; + *) + cat "$err" >&2 + echo "::warning::the record read of run $id failed (exit $code); the next run tries again" + continue + ;; + esac # As the dispatch: at most three sends, sent again after a capped # full-jitter wait only when gh's error shows the request never # left. A rate limit is never sent again. 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 506b23a973..2277d3924f 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py @@ -98,6 +98,11 @@ # the re-run record. LANE_DISPATCH_EVENTS = frozenset({"schedule", "workflow_dispatch", "push"}) LANE_RERUN_EVENTS = frozenset({"schedule", "workflow_dispatch"}) +# post-merge-dispatch also sends a red full run to post-merge-demotion-alert.yml +# and records it in an artifact named ``, the one job and events +# of the re-run record; `_Context.lane_alerted` reads it. +LANE_ALERT_ARTIFACT = "pr-merge-alerted-" +LANE_ALERT_EVENTS = LANE_RERUN_EVENTS # A record found once stays found: pr-merge.yml names a file here that holds # the records earlier checks proved (.github/scripts/pr-merge/lane-records.mjs # reads it from the last trusted pr-merge run's artifact). A record in it is @@ -1558,28 +1563,47 @@ def _lane_record( if self._carried_record(artifact, events): self.lane_cache[artifact] = True return True - repo = f"{self.owner}/{self.name}" + proof = self._lane_proof(artifact, events, reread=reread) + if proof is not None: + self._prove(run, artifact, *proof) + self.lane_cache[artifact] = proof is not None + return self.lane_cache[artifact] + + def lane_alerted(self, run_id: int) -> bool: + """Whether post-merge-dispatch already sent full run `run_id` to + post-merge-demotion-alert.yml: its `pr-merge-alerted-` artifact was + uploaded by a run `_lane_sender` accepts. A same-name artifact any + other run uploaded is no record. A failed read raises.""" + return ( + self._lane_proof(f"{LANE_ALERT_ARTIFACT}{run_id}", LANE_ALERT_EVENTS) + is not None + ) + + def _lane_proof( + self, artifact: str, events: frozenset[str], *, reread: bool = False + ) -> tuple[dict[str, Any], dict[str, Any]] | None: + """The first artifact named `artifact` whose uploading run passes + `_lane_sender`, with that run's record; None when no such run uploaded + one. With `reread`, an empty listing is read once more, and a second + empty one is noted in `lane_empty`. A failed read raises.""" + repo = f"{self.owner}/{self.name}" + senders = self._lane_senders(repo, artifact) + if not senders and reread: senders = self._lane_senders(repo, artifact) - if not senders and reread: - senders = self._lane_senders(repo, artifact) - if not senders: - self.lane_empty.add(artifact) - found = False - for sender, item in senders.items(): - if not isinstance(sender, int): - continue - record = json_object( - _read( - ["api", f"repos/{repo}/actions/runs/{sender}"], - f"run {sender}", - ) + if not senders: + self.lane_empty.add(artifact) + for sender, item in senders.items(): + if not isinstance(sender, int): + continue + record = json_object( + _read( + ["api", f"repos/{repo}/actions/runs/{sender}"], + f"run {sender}", ) - if self._lane_sender(record, events): - found = True - self._prove(run, artifact, item, record) - break - self.lane_cache[artifact] = found - return self.lane_cache[artifact] + ) + if self._lane_sender(record, events): + return item, record + return None def _lane_senders(self, repo: str, artifact: str) -> dict[Any, dict[str, Any]]: """The runs that uploaded an artifact named `artifact`: each run's id @@ -3549,6 +3573,18 @@ def _preflight(repo: str, identity: str | None, self_logins: Collection[str]) -> return 2 if refusal else 0 +def _lane_alerted(repo: str, branch: str, run_id: int) -> int: + """`--lane-alerted`: 0 when `_Context.lane_alerted`, 1 when not, 3 when a + read failed. A rate limit exits 75 through babysit_gh.""" + try: + owner, name = _split_repo(repo) + alerted = _Context(owner, name, "", branch=branch).lane_alerted(run_id) + except PostMergeReadError as exc: + print(f"lane-alerted {run_id}: {exc}", file=sys.stderr) + return EXIT_UNREADABLE + return 0 if alerted else EXIT_FAILURE + + def _judge_run(args: argparse.Namespace) -> int: """`--judge-run`: which of one red full run's newly failing checks demote (`_Context.demoting`), with the identity read from the default branch @@ -3697,6 +3733,17 @@ def main(argv: list[str] | None = None) -> int: action="store_true", help="with --judge-run: file or comment on each excused suite's quarantine issue", ) + mode.add_argument( + "--lane-alerted", + type=int, + metavar="RUN_ID", + help=( + "with --branch: exit 0 when pr-merge.yml's post-merge-dispatch on the " + "branch already sent full run RUN_ID to post-merge-demotion-alert.yml " + "(its pr-merge-alerted- record, from a sender `_lane_sender` " + "accepts), 1 when not, 3 unreadable; read-only" + ), + ) mode.add_argument( "--preflight", action="store_true", @@ -3709,6 +3756,10 @@ def main(argv: list[str] | None = None) -> int: if args.preflight: return _preflight(args.repo, args.agent_identity, args.self_login) + if args.lane_alerted is not None: + if not args.branch: + parser.error("--lane-alerted needs --branch") + return _lane_alerted(args.repo, args.branch, args.lane_alerted) if args.file_quarantine and args.judge_run is None: parser.error("--file-quarantine needs --judge-run") if args.judge_run is not None: diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_postmerge.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_postmerge.py index c1945cf160..71e96f63e3 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_postmerge.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_postmerge.py @@ -1648,6 +1648,73 @@ def test_record_for_another_actors_dispatch_does_not_verify(self) -> None: self.assertFalse(self.check(pinned=None).merges[0].verified) +class LaneAlerted(CheckCase): + """pr-merge.yml's alert step skips a red full run only when its own + post-merge-dispatch on main recorded the send (`pr-merge-alerted-`): + the same sender rule as the dispatch and re-run records. A same-name + artifact any other run uploaded is no record (review finding on #10491).""" + + SENDER = 600 + + def alerted(self, **sender: Any) -> None: + self.gh.artifacts["pr-merge-alerted-11"] = [ + { + "id": 7, + "name": "pr-merge-alerted-11", + "workflow_run": {"id": self.SENDER}, + } + ] + record = { + "id": self.SENDER, + "path": ".github/workflows/pr-merge.yml", + "event": "schedule", + "head_branch": "main", + "head_sha": LATER_SHA, + "repository": {"full_name": REPO}, + "head_repository": {"full_name": REPO}, + } + record.update(sender) + self.gh.run_records[self.SENDER] = record + + def cli(self) -> int: + with contextlib.redirect_stderr(io.StringIO()): + return pm.main(["--repo", REPO, "--branch", "main", "--lane-alerted", "11"]) + + def test_a_record_from_post_merge_dispatch_on_main_counts(self) -> None: + for event in ("schedule", "workflow_dispatch"): + self.alerted(event=event) + self.assertEqual(self.cli(), 0, event) + self.assertEqual(self.gh.writes(), []) + + def test_a_same_name_record_from_any_other_run_does_not_count(self) -> None: + for sender in ( + {"event": "pull_request"}, + {"event": "pull_request_target"}, + {"event": "push"}, + {"path": ".github/workflows/ci.yml"}, + {"head_branch": "feature"}, + {"repository": {"full_name": "example-org/other"}}, + {"head_repository": {"full_name": "outsider/example-repo"}}, + {"head_sha": OTHER_SHA}, + ): + self.alerted(**sender) + self.assertEqual(self.cli(), 1, sender) + + def test_no_record_does_not_count(self) -> None: + self.assertEqual(self.cli(), 1) + + def test_an_unreadable_record_or_sender_exits_3(self) -> None: + self.alerted() + self.gh.fail_reads.add(f"actions/runs/{self.SENDER}") + self.assertEqual(self.cli(), 3) + self.gh.fail_reads = {"artifacts?name=pr-merge-alerted-11"} + self.assertEqual(self.cli(), 3) + + def test_it_needs_a_branch(self) -> None: + with contextlib.redirect_stderr(io.StringIO()), self.assertRaises(SystemExit): + pm.main(["--repo", REPO, "--lane-alerted", "11"]) + + class EmptyLaneRecordListing(CheckCase): """An artifact listing that answers empty for a run the lane dispatched (a degraded API) must not read as "not the lane's run": the listing is From f6a23475181b4981161ba734582ca22b52c84e73 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 27 Oct 2026 17:24:54 -0400 Subject: [PATCH 5/5] fix(pr-merge): send a red run only on --lane-alerted's not-recorded code --lane-alerted answered "not recorded" with exit 1, which is also what Python exits with on any uncaught exception, so a crashing check fell through to the send and sent the same red run again every tick. It now answers EXIT_NOT_ALERTED (4), a code no crash (1), argparse refusal (2), failed read (3) or rate limit (75) produces. The alert step sends only on 4; 1 and every other unknown code warn and leave the run to the next run, as a failed read does. Co-Authored-By: Claude Opus 5.5 --- ...perf-main-whole-suite-batching-3c4daaf0.md | 2 +- .github/scripts/pr-merge/merge-job.test.mjs | 22 ++++++++++++++++++- .github/workflows/pr-merge.yml | 7 ++++-- .../babysit-prs/scripts/babysit_postmerge.py | 12 ++++++---- .../scripts/tests/test_babysit_postmerge.py | 9 ++++++-- 5 files changed, 42 insertions(+), 10 deletions(-) diff --git a/.changes/source-control/perf-main-whole-suite-batching-3c4daaf0.md b/.changes/source-control/perf-main-whole-suite-batching-3c4daaf0.md index bcbe98fbb9..5fef4033a9 100644 --- a/.changes/source-control/perf-main-whole-suite-batching-3c4daaf0.md +++ b/.changes/source-control/perf-main-whole-suite-batching-3c4daaf0.md @@ -8,4 +8,4 @@ bump: patch ### Added -- `babysit_postmerge.py --lane-alerted RUN_ID --branch ` exits 0 when pr-merge.yml's post-merge-dispatch on the branch already sent that full run to post-merge-demotion-alert.yml, 1 when not, 3 when unreadable. It counts the `pr-merge-alerted-` record only from a sender the dispatch and re-run records' rule accepts (`_lane_sender`), so a same-name artifact from any other run is no record. +- `babysit_postmerge.py --lane-alerted RUN_ID --branch ` exits 0 when pr-merge.yml's post-merge-dispatch on the branch already sent that full run to post-merge-demotion-alert.yml, 4 when not, 3 when unreadable; 4 is a code no crash produces, and pr-merge.yml sends only on it. It counts the `pr-merge-alerted-` record only from a sender the dispatch and re-run records' rule accepts (`_lane_sender`), so a same-name artifact from any other run is no record. diff --git a/.github/scripts/pr-merge/merge-job.test.mjs b/.github/scripts/pr-merge/merge-job.test.mjs index b1ac4caad5..54d4d30a5c 100644 --- a/.github/scripts/pr-merge/merge-job.test.mjs +++ b/.github/scripts/pr-merge/merge-job.test.mjs @@ -1522,7 +1522,7 @@ const ALERT_SENDER = { }; const alertSend = (id, sha = ALERT_SHA) => `api -X POST repos/o/r/actions/workflows/post-merge-demotion-alert.yml/dispatches -f ref=main -f inputs[run-id]=${id} -f inputs[head-sha]=${sha}`; -const runAlert = ({ runs = [], firsts = {}, jobs = {}, sent = {}, post = ["ok"], listing } = {}) => { +const runAlert = ({ runs = [], firsts = {}, jobs = {}, sent = {}, post = ["ok"], listing, python } = {}) => { const dir = mkdtempSync(join(tmpdir(), "pr-merge-alert-")); try { const bin = join(dir, "bin"); @@ -1597,6 +1597,8 @@ jq -r "$filter" <<<"$v" writeFileSync(join(bin, "sleep"), `#!/usr/bin/env bash\necho "$1" >>"${dir}/slept"\n`); chmodSync(join(bin, "gh"), 0o755); chmodSync(join(bin, "sleep"), 0o755); + // `python`: a stand-in for babysit_postmerge.py's process, as a bash body. + if (python) writeFileSync(join(bin, "python3"), `#!/usr/bin/env bash\n${python}\n`, { mode: 0o755 }); symlinkSync(root, join(dir, ".base")); const result = spawnSync("bash", ["-c", runScript("Send a red full run to the demotion alert", earlySteps())], { cwd: dir, @@ -1702,6 +1704,24 @@ test("a same-name record from a run that is not pr-merge.yml's on main does not } }); +// Review finding on #10491 (claude-review): only --lane-alerted's "not +// recorded" code (EXIT_NOT_ALERTED, 4) sends. An uncaught exception exits 1 +// with a traceback, and any code but 0, 4 and 75 leaves the run to the next +// run, so a crashing check never sends the same run again each tick. +test("a record check that crashes or answers an unknown code sends nothing", () => { + const traceback = 'echo "Traceback (most recent call last):" >&2; echo "TypeError: unexpected" >&2; exit 1'; + for (const python of [traceback, "exit 2", "exit 3", "exit 5"]) { + const run = runAlert({ runs: [{ id: 41, conclusion: "failure" }], python }); + assert.equal(run.status, 0, run.out); + assert.deepEqual(run.sends, [], python); + assert.match(run.out, /::warning::the record read of run 41 failed \(exit \d\); the next run tries again/); + assert.equal(run.record, ""); + } + // Control: the not-recorded code sends. + const missing = runAlert({ runs: [{ id: 41, conclusion: "failure" }], python: "exit 4" }); + assert.deepEqual(missing.sends, [alertSend(41)], missing.out); +}); + test("one run per job: the oldest red one unsent, the next on the next run", () => { const run = runAlert({ runs: [ diff --git a/.github/workflows/pr-merge.yml b/.github/workflows/pr-merge.yml index aeff8752b5..cc83ccfbd2 100644 --- a/.github/workflows/pr-merge.yml +++ b/.github/workflows/pr-merge.yml @@ -599,13 +599,16 @@ jobs: # Already sent only when pr-merge.yml's own post-merge-dispatch on # main uploaded the record (_Context.lane_alerted, the sender rule # of the dispatch and re-run records): a same-name artifact any - # other run uploads is no record. 0 sent, 1 not, 75 a rate limit. + # other run uploads is no record. 0 sent, 4 not sent, 75 a rate + # limit. Only 4 sends: 1 (a crash), 3 (a failed read) and any other + # code leave the run to the next run, so a broken check never sends + # the same run again each tick. code=0 python3 .base/plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py \ --repo "$REPO" --branch main --lane-alerted "$id" /dev/null 2>"$err" || code=$? case $code in 0) continue ;; - 1) ;; + 4) ;; 75) stopped "the record read of run $id" echo "::error title=GitHub rate limit::the record read of run $id was rate limited (an unrecorded pool until its reset); nothing more is sent" 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 2277d3924f..db6feb794f 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py @@ -211,6 +211,9 @@ GREEN, FLAKE, RED, RERUN, PENDING = "green", "flake", "red", "rerun", "pending" EXIT_FAILURE, EXIT_UNREADABLE, EXIT_WRITE_FAILED, EXIT_UNVERIFIED = 1, 3, 4, 5 +# `--lane-alerted`'s "not recorded": a code no crash (1), argparse refusal (2), +# failed read (3) or rate limit (75) produces, so only it lets pr-merge.yml send. +EXIT_NOT_ALERTED = 4 IsHuman = Callable[[Any], bool] @@ -3574,15 +3577,16 @@ def _preflight(repo: str, identity: str | None, self_logins: Collection[str]) -> def _lane_alerted(repo: str, branch: str, run_id: int) -> int: - """`--lane-alerted`: 0 when `_Context.lane_alerted`, 1 when not, 3 when a - read failed. A rate limit exits 75 through babysit_gh.""" + """`--lane-alerted`: 0 when `_Context.lane_alerted`, EXIT_NOT_ALERTED (4) + when not, 3 when a read failed. A rate limit exits 75 through babysit_gh, + and an uncaught exception exits 1: neither is 4, so neither sends.""" try: owner, name = _split_repo(repo) alerted = _Context(owner, name, "", branch=branch).lane_alerted(run_id) except PostMergeReadError as exc: print(f"lane-alerted {run_id}: {exc}", file=sys.stderr) return EXIT_UNREADABLE - return 0 if alerted else EXIT_FAILURE + return 0 if alerted else EXIT_NOT_ALERTED def _judge_run(args: argparse.Namespace) -> int: @@ -3741,7 +3745,7 @@ def main(argv: list[str] | None = None) -> int: "with --branch: exit 0 when pr-merge.yml's post-merge-dispatch on the " "branch already sent full run RUN_ID to post-merge-demotion-alert.yml " "(its pr-merge-alerted- record, from a sender `_lane_sender` " - "accepts), 1 when not, 3 unreadable; read-only" + "accepts), 4 when not, 3 unreadable; read-only" ), ) mode.add_argument( diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_postmerge.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_postmerge.py index 71e96f63e3..fc918a89ee 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_postmerge.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_postmerge.py @@ -1698,10 +1698,10 @@ def test_a_same_name_record_from_any_other_run_does_not_count(self) -> None: {"head_sha": OTHER_SHA}, ): self.alerted(**sender) - self.assertEqual(self.cli(), 1, sender) + self.assertEqual(self.cli(), pm.EXIT_NOT_ALERTED, sender) def test_no_record_does_not_count(self) -> None: - self.assertEqual(self.cli(), 1) + self.assertEqual(self.cli(), pm.EXIT_NOT_ALERTED) def test_an_unreadable_record_or_sender_exits_3(self) -> None: self.alerted() @@ -1710,6 +1710,11 @@ def test_an_unreadable_record_or_sender_exits_3(self) -> None: self.gh.fail_reads = {"artifacts?name=pr-merge-alerted-11"} self.assertEqual(self.cli(), 3) + def test_not_recorded_is_a_code_no_crash_or_refusal_produces(self) -> None: + # pr-merge.yml sends only on this code: an uncaught exception exits 1, + # argparse 2, a failed read 3 and a rate limit 75. + self.assertNotIn(pm.EXIT_NOT_ALERTED, (0, 1, 2, pm.EXIT_UNREADABLE, 75)) + def test_it_needs_a_branch(self) -> None: with contextlib.redirect_stderr(io.StringIO()), self.assertRaises(SystemExit): pm.main(["--repo", REPO, "--lane-alerted", "11"])