From 6ca0907de10dff97608d4b0957313a091303097f Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 3 Nov 2026 00:24:44 -0500 Subject: [PATCH 1/2] fix(pr-pipeline): name the merge-holds step for what it holds and drop the stale status re-run Rename ci-status's merge-holds step to say what it holds now: the kill switch, hold paths, verifier holds and a member's veto. pr-verify's held-run check keeps taking the old name, because a run reports the step names of its own commit, re-runs too. Drop "no body hold" from the merge-holds verdict line. The step's grep still takes the old line as well: it reads the module from main's tip, which a queued group carrying this change has not reached. Stop re-running ci-status when a pr-unblock run completes. ci-status reads no pr-unblock/* status, and the labels that run writes start their own run, so pr-unblock also leaves the workflow's upstreams and concurrency groups. Correct the docs: a member's veto comment holds a lanes App pull request, and the seven hold places are the 2026-11-01 count. Closes #12446 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VQb9Q49h99qKdrunU4xw3S --- .../fix-12446-hold-step-names-80d0f286.md | 17 +++ .../actions/check-trusted-trigger/README.md | 13 +- .../reconcile.test.mjs | 2 +- .../pr-merge/enable-auto-merge.test.mjs | 2 +- .github/scripts/pr-merge/merge-holds.mjs | 2 +- .github/scripts/pr-merge/merge-holds.test.mjs | 10 +- .../pr-unblock/workflow-gates.test.mjs | 22 ++-- .github/scripts/pr-verify/resolve-pr.mjs | 12 +- .github/scripts/pr-verify/resolve-pr.test.mjs | 13 +- .github/workflows/pr-feedback-dispatch.yml | 120 ++++-------------- .github/workflows/pr-require-checks.yml | 7 +- docs/contracts/loop-lane/README.md | 6 +- docs/conventions/pr-pipeline/README.md | 8 +- scripts/ci-lanes-pr-hold.test.sh | 22 ++-- scripts/ci-rerun-ci-status.test.sh | 102 ++------------- 15 files changed, 112 insertions(+), 246 deletions(-) create mode 100644 .changes/pr-pipeline/fix-12446-hold-step-names-80d0f286.md diff --git a/.changes/pr-pipeline/fix-12446-hold-step-names-80d0f286.md b/.changes/pr-pipeline/fix-12446-hold-step-names-80d0f286.md new file mode 100644 index 00000000000..a8d96e4e19a --- /dev/null +++ b/.changes/pr-pipeline/fix-12446-hold-step-names-80d0f286.md @@ -0,0 +1,17 @@ +--- +bump: patch +--- + +### Changed + +- ci-status's merge-holds step is now `Fail a lanes App pull request on kill switch, hold path, verifier or veto`. pr-verify's held-run check still takes the earlier name, which runs from before the rename report. +- The merge-holds verdict line is now `no member veto and no hold path.`. ci-status still takes the earlier line, for a run whose module comes from a main tip behind its workflow. + +### Removed + +- `pr-feedback-dispatch.yml` no longer re-runs ci-status when a pr-unblock run completes: ci-status reads no `pr-unblock/*` status. + +### Fixed + +- The loop-lane contract no longer says a pull request comment is never a hold: a member's veto comment holds a lanes App pull request. +- The "Not settled yet" note on holds gives the seven places as the 2026-11-01 count, not today's. diff --git a/.github/actions/check-trusted-trigger/README.md b/.github/actions/check-trusted-trigger/README.md index 0705a99172b..b3095e601a8 100644 --- a/.github/actions/check-trusted-trigger/README.md +++ b/.github/actions/check-trusted-trigger/README.md @@ -193,10 +193,8 @@ condition 1 and the actor and author parts of condition 2). `ci-status-rerun-workflow` adds a second recorder for the upstream run only: `pr-feedback-dispatch.yml`'s `rerun-ci-status` job, which uploads `rerun-ci-status---` immediately before it re-runs a ci-status job, - naming the attempt that re-run starts. Its upstream is a completed review run, a completed - `pr-unblock.yml` run whose newest `pr-unblock/*` status on the PR's live head may be newer than - ci-status's read, or a completed `pr-review-approved.yml` relay of an approval by a listed - human. A repeated stop costs one extra re-run, bounded by the dispatchers' caps. A github-actions[bot] upstream at attempt 2 or later is + naming the attempt that re-run starts. Its upstream is a completed review run or a completed + `pr-review-approved.yml` relay of an approval by a listed human. A github-actions[bot] upstream at attempt 2 or later is checked as above against `GET /actions/runs/{id}/attempts/{attempt}` and that name (at attempt 2 the reconciler's name is read first), except that the recording run's `event` must be `workflow_run` and its `actor` is not checked, since such a run runs the default branch's file, @@ -215,12 +213,7 @@ condition 1 and the actor and author parts of condition 2). reconciler's attempt 2 whose `reconcile-rerun--2-` record passes the reconciler re-run check above, unchanged (`reconcilerRecordVouches`, run through `reconciler-record.mjs`); another re-runner's re-run of ci-status goes ahead unrecorded and is - refused here. A `pr-unblock.yml` upstream is recorded only when its `report-stop-status` job - succeeded and it is at attempt 1 started by github-actions[bot], or was started by a listed - human. Any workflow's `GITHUB_TOKEN` with `actions: write`, a same-repository PR branch's - included, can dispatch one, so such a holder can cause one recorded re-run of ci-status at the - PR's live head when its newest `pr-unblock/*` status may be newer than ci-status's read: the - class this residual already accepts. A relay + refused here. A relay upstream is recorded only at attempt 1, whose triggering actor is the reviewer, a listed human; a relay re-run re-runs nothing. This check does not repeat that one: the record's listing names neither the review run nor its re-runner. diff --git a/.github/scripts/maintenance-reconcile-lost-runs/reconcile.test.mjs b/.github/scripts/maintenance-reconcile-lost-runs/reconcile.test.mjs index 1daa36d3b8b..71b141e76e7 100644 --- a/.github/scripts/maintenance-reconcile-lost-runs/reconcile.test.mjs +++ b/.github/scripts/maintenance-reconcile-lost-runs/reconcile.test.mjs @@ -1770,7 +1770,7 @@ test("a kill-switch re-run shares the cap of 3, and one re-run since the listing test("ci-status's lanes-hold step still fails a lanes App PR with the kill-switch note the reconciler matches", () => { const workflow = readFileSync(new URL("../../workflows/pr-require-checks.yml", import.meta.url), "utf8"); - const step = /- name: Fail a lanes App pull request its body or a member holds\n(?:(?:[ ]{8,}.*)?\n)+/.exec(workflow)?.[0] ?? ""; + const step = /- name: Fail a lanes App pull request on kill switch, hold path, verifier or veto\n(?:(?:[ ]{8,}.*)?\n)+/.exec(workflow)?.[0] ?? ""; assert.match(step, /LANES_DISABLED: \$\{\{ vars\.CLAUDE_LANES_DISABLED \}\}/); // The step prints merge-holds.mjs switchHold's reason as its error. assert.match(step, /const \{ switchHold \} = await import\(/); diff --git a/.github/scripts/pr-merge/enable-auto-merge.test.mjs b/.github/scripts/pr-merge/enable-auto-merge.test.mjs index 419273fddae..78a7b4cbc6c 100644 --- a/.github/scripts/pr-merge/enable-auto-merge.test.mjs +++ b/.github/scripts/pr-merge/enable-auto-merge.test.mjs @@ -3874,7 +3874,7 @@ test("merge-holds.mjs holds no hold path from the command line with PATH_HOLDS f const gate = [".github/scripts/pr-merge/clear-escalations.mjs"]; const off = cli([gate], "false"); assert.equal(off.status, 0, off.stdout); - assert.ok(off.stdout.split("\n").includes("#7: no body hold, no member veto and no hold path."), off.stdout); + assert.ok(off.stdout.split("\n").includes("#7: no member veto and no hold path."), off.stdout); assert.ok(off.stdout.includes("#7: path holds are off (merge.diff-check.path-holds), so nothing holds it for this: it changes .github/scripts/pr-merge/clear-escalations.mjs, a hold path."), off.stdout); for (const on of [undefined, "true"]) { const held = cli([gate], on); diff --git a/.github/scripts/pr-merge/merge-holds.mjs b/.github/scripts/pr-merge/merge-holds.mjs index 1dda0f00745..df7276b75af 100755 --- a/.github/scripts/pr-merge/merge-holds.mjs +++ b/.github/scripts/pr-merge/merge-holds.mjs @@ -1692,7 +1692,7 @@ export async function main(argv = process.argv.slice(2), { env = process.env, gi console.log(`::error title=${VERIFIER_TITLE}::#${number}: ${escape(reason)}.`); } if (!reasons.length) { - console.log(`#${number}: no body hold, no member veto and no hold path.`); + console.log(`#${number}: no member veto and no hold path.`); } return reasons.length || verifier.length ? 1 : 0; } catch (error) { diff --git a/.github/scripts/pr-merge/merge-holds.test.mjs b/.github/scripts/pr-merge/merge-holds.test.mjs index 38ce0a65140..dcece4b82a5 100644 --- a/.github/scripts/pr-merge/merge-holds.test.mjs +++ b/.github/scripts/pr-merge/merge-holds.test.mjs @@ -484,7 +484,7 @@ test("ci-status holds no other trusted path or manifest change, only a configure ]) { const passed = await ci([{ filename: path, status: "modified" }]); assert.equal(passed.code, 0, `${path}: ${passed.out}`); - assert.match(passed.out, /no body hold, no member veto and no hold path\./); + assert.match(passed.out, /no member veto and no hold path\./); } const held = await ci([{ filename: "docs/held.md", status: "modified" }], { denied: ["docs/held.md"] }); assert.equal(held.code, 1); @@ -515,7 +515,7 @@ test("ci-status holds a trusted-actor list widening until a trusted person appro assert.equal(changed.code, 1, changed.out); const removed = await ci(modified, { list: { [BASE]: listOf(KYLE, LANES), [HEAD]: listOf(KYLE) } }); assert.equal(removed.code, 0, removed.out); - assert.match(removed.out, /no body hold, no member veto and no hold path\./); + assert.match(removed.out, /no member veto and no hold path\./); // A side that does not read, a repeated key, or a rename holds. assert.equal((await ci(modified, { list: { [BASE]: listOf(KYLE, LANES) } })).code, 1); assert.equal((await ci(modified, { list: { [BASE]: listOf(KYLE, LANES), [HEAD]: listOf(KYLE).replace('"version": 1', '"version": 1, "version": 1') } })).code, 1); @@ -529,7 +529,7 @@ test("ci-status holds no body hold text, even a person's; a hold path or a membe const person = { __typename: "User" }; const written = await ci(["docs/a.md"], { body, writers: { author: person, userContentEdits: { totalCount: 0, nodes: [] } } }); assert.equal(written.code, 0, written.out); - assert.match(written.out, /^#99: no body hold, no member veto and no hold path\.$/m); + assert.match(written.out, /^#99: no member veto and no hold path\.$/m); const pathed = await ci(["docs/held.md"], { body, denied: ["docs/held.md"] }); assert.equal(pathed.code, 1, pathed.out); assert.match(pathed.out, /it changes docs\/held\.md, a hold path/); @@ -550,7 +550,7 @@ test("ci-status holds a cost path that an approval does not lift, and the owner' assert.match(approved.out, /a cost path/); const decided = await ci(["infra/main.tf"], { body: `Closes #40\n\n${LINK}` }); assert.equal(decided.code, 0, decided.out); - assert.match(decided.out, /no body hold, no member veto and no hold path\./); + assert.match(decided.out, /no member veto and no hold path\./); assert.equal((await ci(["infra/main.tf"], { body: `Closes #40\n\n${LINK}`, options: { node: { lastEditedAt: "2026-10-22T13:00:00Z" } } })).code, 1); }); @@ -980,7 +980,7 @@ test("in the merge queue, a FAIL the reader counts holds a passed head", async ( assert.doesNotMatch(out, /no pr-verify FAIL hold/); // A verifier hold is not a body hold: no untitled error line. assert.doesNotMatch(out, /^::error::/m); - assert.match(out, /^#99: no body hold, no member veto and no hold path\.$/m); + assert.match(out, /^#99: no member veto and no hold path\.$/m); }); test("in the merge queue, the reader's verify:unreadable is a pr-verify hold beside a body hold", async () => { diff --git a/.github/scripts/pr-unblock/workflow-gates.test.mjs b/.github/scripts/pr-unblock/workflow-gates.test.mjs index 716e175e345..43bcd22f5cc 100644 --- a/.github/scripts/pr-unblock/workflow-gates.test.mjs +++ b/.github/scripts/pr-unblock/workflow-gates.test.mjs @@ -371,9 +371,9 @@ test("pr-feedback-dispatch.yml re-runs ci-status for a completed review run of a assert.equal(runs({ "github.ref": "refs/heads/feat/x" }), false); }); -// A pr-unblock run (its status write starts no workflow) and a relayed -// approval change what ci-status reads, so each re-runs it too. -test("pr-feedback-dispatch.yml re-runs ci-status after a pr-unblock dispatch on main or a relayed review", () => { +// A relayed approval changes what ci-status reads, so it re-runs it too. A +// pr-unblock run does not: ci-status reads no pr-unblock/* status. +test("pr-feedback-dispatch.yml re-runs ci-status after a relayed review, not a pr-unblock dispatch on main", () => { const gate = jobIf("pr-feedback-dispatch.yml", "rerun-ci-status"); const repo = "melodic-software/claude-code-plugins"; const base = { @@ -383,20 +383,15 @@ test("pr-feedback-dispatch.yml re-runs ci-status after a pr-unblock dispatch on "github.event.workflow_run.repository.full_name": repo, "github.event.workflow_run.head_repository.full_name": repo, }; - const unblock = (o = {}) => + assert.equal( evaluate(gate, { ...base, "github.event.workflow_run.path": ".github/workflows/pr-unblock.yml", "github.event.workflow_run.event": "workflow_dispatch", "github.event.workflow_run.head_branch": "main", - ...o, - }); - assert.equal(unblock(), true); - assert.equal(unblock({ "github.event.workflow_run.conclusion": "failure" }), true); - assert.equal(unblock({ "github.event.workflow_run.conclusion": "skipped" }), false); - assert.equal(unblock({ "github.event.workflow_run.head_branch": "feat/x" }), false); - assert.equal(unblock({ "github.event.workflow_run.event": "pull_request" }), false); - assert.equal(unblock({ "github.event.workflow_run.head_repository.full_name": "fork/repo" }), false); + }), + false, + ); const review = (o = {}) => evaluate(gate, { ...base, @@ -413,7 +408,8 @@ test("pr-feedback-dispatch.yml re-runs ci-status after a pr-unblock dispatch on assert.equal(review({ "github.ref": "refs/heads/feat/x" }), false); const text = readFileSync(new URL("../../workflows/pr-feedback-dispatch.yml", import.meta.url), "utf8"); const upstreams = text.match(/\n workflows: \[([^\]]*)\]\n/)[1].split(",").map((name) => name.trim()); - for (const name of ["pr-unblock", "pr-review-approved"]) assert.ok(upstreams.includes(name), `${name} is a workflow_run upstream`); + assert.ok(upstreams.includes("pr-review-approved"), "pr-review-approved is a workflow_run upstream"); + assert.ok(!upstreams.includes("pr-unblock"), "pr-unblock is no workflow_run upstream"); }); // The relay starts a runner only for an approval of a same-repository PR. diff --git a/.github/scripts/pr-verify/resolve-pr.mjs b/.github/scripts/pr-verify/resolve-pr.mjs index ee67495f6e4..308f593ad12 100644 --- a/.github/scripts/pr-verify/resolve-pr.mjs +++ b/.github/scripts/pr-verify/resolve-pr.mjs @@ -254,8 +254,12 @@ const laneAuthored = (user, laneIdentity) => const ownerAuthored = (user, ids) => (user?.type === "User" || user?.type === "Bot") && ids.includes(user.id); // pr-require-checks.yml's ci-status step that runs merge-holds.mjs on a lanes -// App pull request, and the step after it that aggregates the lanes. -export const HOLD_STEP = "Fail a lanes App pull request its body or a member holds"; +// App pull request, and the step after it that aggregates the lanes. A run +// reports its own commit's step names, on a re-run too, so a held head whose +// newest run predates the current name still reports OLD_HOLD_STEP. +export const HOLD_STEP = "Fail a lanes App pull request on kill switch, hold path, verifier or veto"; +const OLD_HOLD_STEP = "Fail a lanes App pull request its body or a member holds"; +const HOLD_STEPS = [HOLD_STEP, OLD_HOLD_STEP]; export const AGGREGATE_STEP = "Aggregate lane results"; // The ci-status step that runs ci-workflows' check-contract (title, // `do-not-merge` label, issue linkage), and the one error line its run.sh @@ -317,7 +321,7 @@ async function heldShape(read) { const names = failed.map((step) => step?.name); if ( failed.length === 0 || - failed.some((step) => step.conclusion !== "failure" || ![HOLD_STEP, CONTRACT_STEP].includes(step.name)) || + failed.some((step) => step.conclusion !== "failure" || ![...HOLD_STEPS, CONTRACT_STEP].includes(step.name)) || new Set(names).size !== names.length || !steps.some((step) => step?.name === AGGREGATE_STEP && step.conclusion === "success") ) { @@ -338,7 +342,7 @@ async function heldOnly(readLog, { ciId, names }, number) { return ( errors.length - reasons.length === names.length && person.length + label.length === reasons.length && - (person.length > 0) === names.includes(HOLD_STEP) && + (person.length > 0) === names.some((name) => HOLD_STEPS.includes(name)) && label.length === (names.includes(CONTRACT_STEP) ? 1 : 0) ); } catch { diff --git a/.github/scripts/pr-verify/resolve-pr.test.mjs b/.github/scripts/pr-verify/resolve-pr.test.mjs index fca8bbd1509..803c4a82c77 100644 --- a/.github/scripts/pr-verify/resolve-pr.test.mjs +++ b/.github/scripts/pr-verify/resolve-pr.test.mjs @@ -393,7 +393,7 @@ const CI_STEPS = [ ["Note a draft", "skipped"], ["Fail a pull request carrying the human-gated label", "success"], ["Fail a head a pr-unblock lane stopped", "success"], - ["Fail a lanes App pull request its body or a member holds", "failure"], + ["Fail a lanes App pull request on kill switch, hold path, verifier or veto", "failure"], ["Check each skipped lane against scope", "success"], ["Aggregate lane results", "success"], ["Complete job", "success"], @@ -470,6 +470,17 @@ test("a run that failed only on do-not-merge, alone or beside a hold an approval assert.equal(await held(`${contractLog(DO_NOT_MERGE_LINE)}\r\n${logOf(PATH_HOLD)}`, failing(CONTRACT_STEP, HOLD_STEP)), "ok"); }); +// A run reports its own commit's step names, so a held run from before the +// hold step's rename still reports the earlier name. +test("a held run that still reports the hold step's earlier name is judged the same way", async () => { + const oldName = "Fail a lanes App pull request its body or a member holds"; + const jobs = heldJobs(ciJob(CI_STEPS.map(([n, c]) => [n === HOLD_STEP ? oldName : n, c]))); + assert.equal(await held(logOf(PATH_HOLD), { jobs }), "ok"); + assert.equal(await held(logOf(COST), { jobs }), "not-a-passing-run"); + assert.equal(await held(`${contractLog(DO_NOT_MERGE_LINE)}\r\n${logOf(PATH_HOLD)}`, { jobs: heldJobs(ciJob(CI_STEPS.map(([n, c]) => [n === HOLD_STEP ? oldName : n, n === CONTRACT_STEP ? "failure" : c]))) }), "ok"); + assert.equal(await held(logOf(), { jobs }), "not-a-passing-run"); +}); + test("a contract failure other than do-not-merge, or a label line its step does not account for, starts nothing", async () => { const TITLE = "pr-contract: the PR title does not follow Conventional Commits"; const cases = { diff --git a/.github/workflows/pr-feedback-dispatch.yml b/.github/workflows/pr-feedback-dispatch.yml index d49a4c70de3..07931a0b0ac 100644 --- a/.github/workflows/pr-feedback-dispatch.yml +++ b/.github/workflows/pr-feedback-dispatch.yml @@ -79,12 +79,9 @@ name: pr-feedback-dispatch # rerun-ci-status job, which re-runs the head's ci-status once the head's # last review lane has finished, so it reads both reviews, and records each # re-run for pr-verify's trigger gate (see that job). So does a completed -# pr-unblock.yml run, whose report-stop-status job sets or clears a -# pr-unblock/* status ci-status holds on, and a completed # pr-review-approved.yml run, a trusted person's approval that lifts a -# ci-status hold. Neither has an event of its own: the status is posted with -# a GITHUB_TOKEN, whose events start no workflow, and a review event would -# run the PR's copy of this file. +# ci-status hold. An approval has no event of its own here: a review event +# would run the PR's copy of this file. # # Every completed review run at a head starts this file, so a run leaves a # record for the next one at that head in the Actions cache @@ -96,7 +93,7 @@ name: pr-feedback-dispatch on: # workflow_run runs this file from the default branch, never a PR's copy. workflow_run: # zizmor: ignore[dangerous-triggers] - workflows: [pr-review-hosted, pr-review-security-hosted, pr-verify, pr-unblock-dispatch, pr-merge, pr-unblock, pr-review-approved, pr-review-comment-relay] + workflows: [pr-review-hosted, pr-review-security-hosted, pr-verify, pr-unblock-dispatch, pr-merge, pr-review-approved, pr-review-comment-relay] types: [completed] permissions: {} @@ -106,17 +103,12 @@ permissions: {} # the sweeps share one. Runs at a head wait in line (`queue: max`): GitHub # otherwise keeps one pending run per group and cancels it for a newer one, # so a pr-merge completion, or a review re-run's, could drop the later review -# lane's run and with it the one rerun-ci-status after both lanes. A -# pr-unblock.yml run's head is main's, so its runs share one group per -# activity and PR (its run name), apart from that head's group; the -# rerun-ci-status job reads the run's attempt again just before it re-runs, -# which makes two groups re-running one attempt twice rare, not impossible. +# lane's run and with it the one rerun-ci-status after both lanes. concurrency: group: >- pr-feedback-dispatch-${{ github.event.workflow_run.path == '.github/workflows/pr-verify.yml' && format('verify-{0}', github.event.workflow_run.id) || github.event.workflow_run.path == '.github/workflows/pr-unblock-dispatch.yml' && 'sweep' - || github.event.workflow_run.path == '.github/workflows/pr-unblock.yml' && format('unblock-{0}', github.event.workflow_run.display_title) || github.event.workflow_run.head_sha }} cancel-in-progress: false queue: max @@ -531,9 +523,8 @@ jobs: # only be red, and that lane's own completion starts this job again. Each # run of this job starts after its own lane completed, so the later lane's # run finds the earlier lane finished; the per-head concurrency group keeps - # the two from re-running ci-status at once, and a pr-unblock run's group, - # which differs, rarely re-runs the same attempt, not never: a last read of - # the run's attempt comes just before the re-run. A head with no run of the + # the two from re-running ci-status at once, and a last read of the run's + # attempt comes just before the re-run. A head with no run of the # other lane (one lane only) re-runs on this lane alone. A head the PR has # moved past, or a closed PR, is left alone. A ci-status job not started # yet, or one that started more than 60 s after both lanes finished (the @@ -557,20 +548,13 @@ jobs: # another re-runner's re-run of ci-status goes ahead unrecorded, and the # gate refuses it. # - # Two more upstreams change what ci-status reads, and each is judged as a - # finished lane is, with both review lanes as the other lanes: - # - A pr-unblock.yml run (a workflow_dispatch on main, named - # `pr-unblock #`): the PR comes from its run name and the - # head from the live PR, and the newest pr-unblock/* status - # github-actions[bot] set on that head stands in for the lane's finish - # time. No such status, nothing to re-run. Recorded only at attempt 1 - # started by github-actions[bot] (the dispatchers' GITHUB_TOKEN) or when a - # listed human started it. - # - A pr-review-approved.yml run (an approving review, relayed): re-run only - # when its triggering actor, the reviewer, is a listed human, checked - # before any API call, and recorded. ci-status still judges the review. - # A status event itself starts nothing: report-stop-status posts with a - # GITHUB_TOKEN, whose events start no workflow run. + # One more upstream changes what ci-status reads, and is judged as a finished + # lane is, with both review lanes as the other lanes: a pr-review-approved.yml + # run (an approving review, relayed), re-run only when its triggering actor, + # the reviewer, is a listed human, checked before any API call, and recorded. + # ci-status still judges the review. + # A pr-unblock.yml run is no upstream: ci-status reads no pr-unblock/* status, + # and the labels that run writes start their own pull_request run. # Pinned by scripts/ci-rerun-ci-status.test.sh, which runs the find body. rerun-ci-status: if: >- @@ -578,9 +562,6 @@ jobs: && ((github.event.workflow_run.path == '.github/workflows/pr-review-hosted.yml' || github.event.workflow_run.path == '.github/workflows/pr-review-security-hosted.yml') && github.event.workflow_run.event == 'pull_request' - || github.event.workflow_run.path == '.github/workflows/pr-unblock.yml' - && github.event.workflow_run.event == 'workflow_dispatch' - && github.event.workflow_run.head_branch == 'main' || github.event.workflow_run.path == '.github/workflows/pr-review-approved.yml' && github.event.workflow_run.event == 'pull_request_review') && github.event.workflow_run.conclusion != 'skipped' @@ -656,7 +637,6 @@ jobs: REVIEW_ATTEMPT: ${{ github.event.workflow_run.run_attempt }} REVIEW_RERUNNER_ID: ${{ github.event.workflow_run.triggering_actor.id }} REVIEW_RUN_ID: ${{ github.event.workflow_run.id }} - RUN_TITLE: ${{ github.event.workflow_run.display_title }} DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} TRUSTED_ACTORS: .base/.github/standards/trusted-actors/trusted-actors.json RECONCILER_WORKFLOW: .github/workflows/maintenance-reconcile-lost-runs.yml @@ -674,7 +654,7 @@ jobs: echo "::error::the review run's head '$HEAD_SHA' is not a commit SHA." exit 1 fi - unblock=.github/workflows/pr-unblock.yml approved=.github/workflows/pr-review-approved.yml + approved=.github/workflows/pr-review-approved.yml # listed_human: the upstream run's triggering actor is a person on the # default branch's list; an unreadable list or id is not. listed_human() { @@ -692,11 +672,6 @@ jobs: echo "Relay attempt ${REVIEW_ATTEMPT:-?} names its re-runner, not the reviewer; nothing to re-run." exit 0 fi - # A pr-unblock run is a dispatch on main; its run name names the PR. - if [[ $LANE_PATH == "$unblock" ]]; then - PR_NUMBER= - [[ ! $RUN_TITLE =~ ^pr-unblock\ [a-z-]+\ \#([1-9][0-9]{0,9})$ ]] || PR_NUMBER=${BASH_REMATCH[1]} - fi if [[ ! $PR_NUMBER =~ ^[1-9][0-9]{0,9}$ ]]; then echo "::notice::the upstream run names no pull request; nothing to re-run." exit 0 @@ -709,9 +684,9 @@ jobs: case $LANE_PATH in .github/workflows/pr-review-hosted.yml) others='pr-review-security-hosted.yml' ;; .github/workflows/pr-review-security-hosted.yml) others='pr-review-hosted.yml' ;; - "$unblock" | "$approved") others='pr-review-hosted.yml pr-review-security-hosted.yml' ;; + "$approved") others='pr-review-hosted.yml pr-review-security-hosted.yml' ;; *) - echo "::error::'$LANE_PATH' is not a review lane, pr-unblock or an approval relay." + echo "::error::'$LANE_PATH' is not a review lane or an approval relay." exit 1 ;; esac @@ -725,11 +700,9 @@ jobs: # completion exists only while the other lane is running), and # maintenance-reconcile-lost-runs re-runs one red only on a pending # or missing review, at attempt 1, after its PENDING_BACKOFF_MINUTES. - # Should both re-run it, the started_at check below ends this job, - # for a review-lane upstream, on an attempt that started after the - # lanes finished (a pr-unblock run with a full status page skips that - # check). What overlap remains is two POSTs inside check-then-act - # gaps, which GitHub would refuse while the first re-run is going + # Should both re-run it, the started_at check below ends this job on + # an attempt that started after the lanes finished. What overlap + # remains is two POSTs inside check-then-act gaps, which GitHub would refuse while the first re-run is going # (UNVERIFIED) rather than start a third attempt. Should both land, # the extra attempt has no record of its own, so pr-verify refuses # it: the head is stranded for a person, never let through. @@ -793,44 +766,7 @@ jobs: and .head_repository.full_name == $repo)] | max_by(.id) // empty' } - done_at=$LANE_DONE_AT pr='' page_full=false - # A pr-unblock run's head is main's: the PR's live head is the one. Its - # newest pr-unblock/* status github-actions[bot] set (the writer - # ci-status reads, merge-holds.mjs unblockStops) stands in for a lane's - # finish: when it may be newer than ci-status's read, ci-status is re-run. - # A repeated stop costs one extra re-run, bounded by the dispatchers' caps. - if [[ $LANE_PATH == "$unblock" ]]; then - # Any workflow's GITHUB_TOKEN can dispatch pr-unblock.yml, and a run its - # trust gate refused still completes. Only its report-stop-status job, - # behind that gate, writes a status, so its success is required; it - # does not prove a status was written (the statuses below decide that). - if [[ ! $REVIEW_RUN_ID =~ ^[1-9][0-9]{0,19}$ ]]; then - echo "::error::the pr-unblock run id '$REVIEW_RUN_ID' is not a number." - exit 1 - fi - read_api unblock_jobs "the jobs of pr-unblock run $REVIEW_RUN_ID" \ - "repos/$GITHUB_REPOSITORY/actions/runs/$REVIEW_RUN_ID/jobs?filter=latest&per_page=100" - if [ "$(jq -r '[.jobs[] | select(.name == "report-stop-status")] | max_by(.id) | .conclusion // ""' <<<"$unblock_jobs")" != success ]; then - echo "pr-unblock run $REVIEW_RUN_ID wrote no status (its report-stop-status job did not succeed); nothing to re-run." - exit 0 - fi - read_api pr "#$PR_NUMBER" "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER" - HEAD_SHA=$(jq -r 'select(.state == "open") | .head.sha // ""' <<<"$pr") - if [[ ! $HEAD_SHA =~ ^[0-9a-f]{40}$ ]]; then - echo "#$PR_NUMBER is closed or has no head; nothing to re-run." - exit 0 - fi - read_api statuses "the statuses of $HEAD_SHA" "repos/$GITHUB_REPOSITORY/commits/$HEAD_SHA/statuses?per_page=100" - # One page, newest first. A full page may hide more, so it fails open: - # neither exit below that could skip the re-run is taken. - [ "$(jq 'length >= 100' <<<"$statuses")" != true ] || page_full=true - done_at=$(jq -r '[.[] | select((.context | startswith("pr-unblock/")) and .creator.login == "github-actions[bot]")] - | first | .created_at // ""' <<<"$statuses") - if [[ ! $done_at =~ $utc && $page_full != true ]]; then - echo "No pr-unblock/* status on $HEAD_SHA; ci-status holds on none, so nothing to re-run." - exit 0 - fi - fi + done_at=$LANE_DONE_AT for other in $others; do read_api runs "the $other runs on $HEAD_SHA" \ "repos/$GITHUB_REPOSITORY/actions/workflows/$other/runs?head_sha=$HEAD_SHA&event=pull_request&per_page=100" @@ -866,7 +802,7 @@ jobs: esac fi started_at=$(jq -r '.started_at // ""' <<<"${job:-null}") - if [ "$page_full" != true ] && [ "$(jq -r --arg s "$started_at" --arg d "$done_at" \ + if [ "$(jq -r --arg s "$started_at" --arg d "$done_at" \ 'try (($s | fromdateiso8601) > ($d | fromdateiso8601) + 60) catch false' <</dev/null; rm -rf "$TMP_ROOT"' EX export RUNNER_TEMP="$TMP_ROOT" ci_gh_read::install "$WORKFLOW" "$TMP_ROOT" || fail "the ci-status gh_read helper step is missing or fails" -NAME='Fail a lanes App pull request its body or a member holds' +NAME='Fail a lanes App pull request on kill switch, hold path, verifier or veto' step="$(awk -v name=" - name: $NAME" '$0 == name { on = 1; print; next } on && /^ - / { exit } on { print }' "$WORKFLOW")" @@ -331,7 +331,7 @@ export STUB_CONFIG=$'merge:\n diff-check:\n denied-paths: [docs/held/**]\n KYLE_VETO="$(comment kyle User MEMBER 2026-10-23T10:00:00Z 'Please do not merge this yet.')" STUB_PULL="$(pull 'Closes #1')" -expect "a lanes App pull request with no hold passes" 0 3 "#7: no body hold, no member veto and no hold path" +expect "a lanes App pull request with no hold passes" 0 3 "#7: no member veto and no hold path" STUB_PULL="$(pull 'Closes #1' '{"id":153232337,"login":"kyle-sexton","type":"User"}')" expect "a person's pull request is left to pr-merge's gate, with no discussion read" 0 0 "not the lanes App's" STUB_PULL="$(pull 'Closes #1' '{"id":1,"login":"melodic-automation-lanes[bot]","type":"User"}')" @@ -352,7 +352,7 @@ for user in '{"id":153232337,"login":"kyle-sexton","type":"User"}' '{"id":496993 expect_pull_reads "a non-App payload author ($(jq -r .login <<<"$user")) reads no pull" 0 done STUB_PULL="$(pull 'Closes #1')" -expect "the lanes App's payload author goes on to the live read" 0 3 "#7: no body hold, no member veto and no hold path" +expect "the lanes App's payload author goes on to the live read" 0 3 "#7: no member veto and no hold path" expect_pull_reads "the lanes App's pull request is read once" 1 STUB_PULL="$(pull 'Closes #1' '{"id":153232337,"login":"kyle-sexton","type":"User"}')" for payload in "STUB_AUTHOR_ID=" "STUB_AUTHOR_ID=x" "STUB_AUTHOR_TYPE="; do @@ -586,11 +586,11 @@ for gated in "$restore" "$carry"; do done STUB_PULL="$(pull $'Closes #1\n\nHold: wait for the canary')" -expect "a Hold: line in the body does not hold: do-not-merge is the veto" 0 3 "no body hold, no member veto and no hold path" +expect "a Hold: line in the body does not hold: do-not-merge is the veto" 0 3 "no member veto and no hold path" STUB_PULL="$(pull $'Closes #1\r\n hold: owner call')" -expect "an indented, lower-case hold line after a CRLF does not hold" 0 3 "no body hold, no member veto and no hold path" +expect "an indented, lower-case hold line after a CRLF does not hold" 0 3 "no member veto and no hold path" STUB_PULL="$(pull $'Closes #1\n\nDo not merge before Friday.')" -expect "'do not merge' in the body does not hold" 0 3 "no body hold, no member veto and no hold path" +expect "'do not merge' in the body does not hold" 0 3 "no member veto and no hold path" STUB_PULL="$(pull $'Closes #1\n\nThe threshold: unchanged')" expect "a line that only contains 'hold:' later does not hold" 0 3 @@ -651,7 +651,7 @@ STUB_PR_FILES="$(page '[["docs/guide.md"]]')" \ expect "broker pages listing fewer files than changed_files fail closed" 1 0 "GitHub listed fewer files than #7 changes" expect_file_reads "the short broker pages are not read again" 0 STUB_PULL="$(pull 'Closes #1')" -STUB_PR_FILES="$(page '[["docs/guide.md"]]')" expect "clean broker pages pass" 0 3 "#7: no body hold, no member veto and no hold path" +STUB_PR_FILES="$(page '[["docs/guide.md"]]')" expect "clean broker pages pass" 0 3 "#7: no member veto and no hold path" expect_file_reads "clean broker pages leave no files read" 0 STUB_FILES='[["docs/held/a.md"]]' STUB_PR_FILES='not json' \ expect "unparsable broker pages fall back to the live read" 1 3 "it changes docs/held/a.md, a hold path" @@ -968,7 +968,7 @@ fi STUB_ZIP_503=4 expect "in the merge queue, a record download that answers 503 four times fails closed" 1 3 "::error::could not read the verdict of artifact 110; re-run this run." if [[ "$(zips)" -eq 4 ]]; then ok "the record download is sent at most four times"; else fail "the record download was sent $(zips) times"; fi STUB_NO_RECORD='' STUB_RECORD='{"linked_issues":[{"number":9}],"spec_kind":"closes"}' STUB_REVIEWS="[$(approval kyle-sexton 153232337 APPROVED 2026-10-23T11:00:00Z "$SHA")]" -expect "in the merge queue, a trusted approval of the head lifts a changed link" 0 3 "#7: no body hold, no member veto and no hold path." +expect "in the merge queue, a trusted approval of the head lifts a changed link" 0 3 "#7: no member veto and no hold path." STUB_RECORD='' STUB_REVIEWS='' STUB_ARTIFACTS="{\"pr-verify-7-$SHA-pass-notes-$DIGEST\":[11]}" expect "in the merge queue, a digest-named pass-notes at the head passes" 0 3 "no verifier hold: pr-verify passed this head" @@ -1029,6 +1029,8 @@ STUB_PULL="$(pull 'Closes #1')" # Each stand-in module answers the kill switch (switchHold) and prints its # verdict lines only when run as main, as merge-holds.mjs does. stub_module() { printf 'export const switchHold = () => null;\nif (process.argv.length > 3) {\n%s\n}\n' "$2" >"$1"; } +# The stand-ins print the verdict line's earlier wording, which the step still +# takes, since a queued group can run the module from a main tip behind its own. stub_module "$TMP_ROOT/old.mjs" 'console.log("#7: no body hold, no member veto and no hold path.");' STUB_ARTIFACTS="{\"pr-verify-7-$SHA-pass\":[11]}" STUB_MODULE="$TMP_ROOT/old.mjs" expect "in the merge queue, a base module that ignores the verdicts fails closed" 1 3 "::error::merge-holds.mjs exited 0 without its verifier verdict" @@ -1039,7 +1041,7 @@ expect "in the merge queue, a base module that ignores the reader's report fails unset STUB_MODULE STUB_ARTIFACTS="{\"pr-verify-7-$SHA-pass\":[11]}" STUB_PULL="$(pull $'Closes #1\nHold: owner call')" -expect "in the merge queue, a body hold does not fail the group" 0 3 "no body hold, no member veto and no hold path" +expect "in the merge queue, a body hold does not fail the group" 0 3 "no member veto and no hold path" STUB_PULL="$(pull 'Closes #1')" STUB_ISSUE="[$KYLE_VETO]" expect "in the merge queue, a member's veto fails the group" 1 3 "kyle asked not to merge it" STUB_ISSUE="[$KYLE_VETO,$(comment kyle User MEMBER 2026-10-23T11:00:00Z 'ok, merge it')]" diff --git a/scripts/ci-rerun-ci-status.test.sh b/scripts/ci-rerun-ci-status.test.sh index 5fe2ba028bb..1b4c0018576 100755 --- a/scripts/ci-rerun-ci-status.test.sh +++ b/scripts/ci-rerun-ci-status.test.sh @@ -59,7 +59,7 @@ for env in 'HEAD_SHA: ${{ github.event.workflow_run.head_sha }}' 'PR_NUMBER: ${{ 'LANE_PATH: ${{ github.event.workflow_run.path }}' 'LANE_DONE_AT: ${{ github.event.workflow_run.updated_at }}' "POLL_LIMIT_SECONDS: '600'" \ 'REVIEW_ATTEMPT: ${{ github.event.workflow_run.run_attempt }}' 'REVIEW_RERUNNER_ID: ${{ github.event.workflow_run.triggering_actor.id }}' \ 'TRUSTED_ACTORS: .base/.github/standards/trusted-actors/trusted-actors.json' \ - 'REVIEW_RUN_ID: ${{ github.event.workflow_run.id }}' 'RUN_TITLE: ${{ github.event.workflow_run.display_title }}' 'DEFAULT_BRANCH: ${{ github.event.repository.default_branch }}' \ + 'REVIEW_RUN_ID: ${{ github.event.workflow_run.id }}' 'DEFAULT_BRANCH: ${{ github.event.repository.default_branch }}' \ 'RECONCILER_WORKFLOW: .github/workflows/maintenance-reconcile-lost-runs.yml' \ 'RECONCILER_RECORD: .base/.github/actions/check-trusted-trigger/reconciler-record.mjs' \ 'API_BUDGET_SH: .base/.github/actions/log-api-budget/api-budget.bash' \ @@ -181,7 +181,6 @@ case "$*" in ok200 "$STUB_PR" fi ;; -*/commits/*/statuses*) ok200 "$STUB_STATUSES" ;; */actions/workflows/pr-review-hosted.yml/runs* | */actions/workflows/pr-review-security-hosted.yml/runs*) [[ -z "${STUB_LANE_FAIL:-}" ]] || exit 1 ok200 "$STUB_LANE_RUNS" @@ -190,7 +189,6 @@ case "$*" in [[ -z "${STUB_RUNS_FAIL:-}" ]] || exit 1 ok200 "$STUB_RUNS" ;; -*/actions/runs/31/jobs*) ok200 "$STUB_UNBLOCK_JOBS" ;; */actions/runs/*/jobs*) ok200 "$STUB_JOBS" ;; "api -i repos/o/r/actions/runs/"[0-9]*) n=$(grep -c '^api -i repos/o/r/actions/runs/[0-9]*$' "$STUB_LOG") @@ -247,7 +245,7 @@ STUB KYLE=153232337 LANES_APP=337595936 STRANGER=9999001 jq -n --argjson h "$KYLE" --argjson b "$LANES_APP" '{version: 1, actors: [{id: $h, login: "kyle-sexton", kind: "human"}, {id: $b, login: "melodic-automation-lanes[bot]", kind: "bot"}]}' >"$TMP_ROOT/trusted-actors.json" -export STUB_UNBLOCK_JOBS STUB_STATUSES STUB_PR STUB_LANE_RUNS STUB_RUNS STUB_JOBS STUB_RUN_SEQ +export STUB_PR STUB_LANE_RUNS STUB_RUNS STUB_JOBS STUB_RUN_SEQ expect() { local label="$1" want_rc="$2" out rc : >"$TMP_ROOT/log" @@ -259,7 +257,7 @@ expect() { PR_NUMBER="${PR_NUMBER-42}" LANE_PATH="${LANE_PATH-$CODE}" LANE_DONE_AT="${LANE_DONE_AT-$DONE}" \ GITHUB_OUTPUT="$TMP_ROOT/out" RECORD_FILE="$TMP_ROOT/record.json" GITHUB_RUN_ID=555 \ REVIEW_ATTEMPT="${REVIEW_ATTEMPT-1}" REVIEW_RERUNNER_ID="${REVIEW_RERUNNER_ID-$KYLE}" \ - REVIEW_RUN_ID="${REVIEW_RUN_ID-3}" RUN_TITLE="${RUN_TITLE-}" RECONCILER_RECORD="${RECONCILER_RECORD-$TMP_ROOT/record-stub.mjs}" STUB_RECORD_RC="${STUB_RECORD_RC-3}" \ + REVIEW_RUN_ID="${REVIEW_RUN_ID-3}" RECONCILER_RECORD="${RECONCILER_RECORD-$TMP_ROOT/record-stub.mjs}" STUB_RECORD_RC="${STUB_RECORD_RC-3}" \ TRUSTED_ACTORS="${TRUSTED_ACTORS-$TMP_ROOT/trusted-actors.json}" RUNNER_TEMP="$TMP_ROOT/rt" \ API_BUDGET_SH="${API_BUDGET_SH-$ROOT/.github/actions/log-api-budget/api-budget.bash}" POLL_SECONDS=60 POLL_LIMIT_SECONDS=600 bash -e "$TMP_ROOT/body.sh" 2>&1)" && rc=0 || rc=$? if [[ "$rc" -ne "$want_rc" ]]; then @@ -424,7 +422,8 @@ PR_NUMBER='' expect "a review run with no pull request is left alone" 0 "names n if [[ -s "$TMP_ROOT/log" ]]; then fail "a run with no pull request read the API"; else ok "a run with no pull request reads nothing"; fi PR_NUMBER='1;id' expect "a pull request number that is not a number is left alone" 0 "names no pull request" HEAD_SHA=main expect "a head that is not a commit SHA fails" 1 "::error::the review run's head 'main' is not a commit SHA." -LANE_PATH=.github/workflows/pr-verify.yml expect "a run of another file fails" 1 "::error::'.github/workflows/pr-verify.yml' is not a review lane, pr-unblock or an approval relay." +LANE_PATH=.github/workflows/pr-verify.yml expect "a run of another file fails" 1 "::error::'.github/workflows/pr-verify.yml' is not a review lane or an approval relay." +LANE_PATH=.github/workflows/pr-unblock.yml expect "a pr-unblock run is no upstream" 1 "::error::'.github/workflows/pr-unblock.yml' is not a review lane or an approval relay." LANE_DONE_AT='2026-10-23 10:00' expect "a finish time that is not a UTC timestamp fails" 1 "is not a UTC timestamp." if [[ -s "$TMP_ROOT/log" ]]; then fail "a bad finish time read the API"; else ok "a bad finish time reads nothing"; fi @@ -526,93 +525,11 @@ if [[ "$(record_checks)" == 0 ]]; then ok "a non-bot re-runner never asks the re REVIEW_ATTEMPT=2 REVIEW_RERUNNER_ID=$ACTIONS_BOT REVIEW_RUN_ID='' STUB_RECORD_RC=0 expect "a reconciler re-run with no review run id is not recorded" 0 "no reconciler record vouches" check_unrecorded "a missing review run id hands on no record" -# A pr-unblock run (a dispatch on main) and a relayed approval also re-run -# ci-status, judged as a finished lane with both review lanes as the others. -UNBLOCK=.github/workflows/pr-unblock.yml APPROVED=.github/workflows/pr-review-approved.yml -MAIN_TIP=fedcba9876543210fedcba9876543210fedcba98 BOT=41898282 -# status [creator] -status() { jq -cn --arg c "$1" --arg s "$2" --arg at "$3" --arg by "${4:-github-actions[bot]}" '{context: $c, state: $s, created_at: $at, creator: {login: $by}}'; } +# A relayed approval also re-runs ci-status, judged as a finished lane with +# both review lanes as the others. +APPROVED=.github/workflows/pr-review-approved.yml BOTH_LANES="$(runs "$(run 3 completed "$HEAD" "$CODE" pull_request "$DONE")" "$(run 4 completed "$HEAD" "$SEC" pull_request "$DONE")")" -unblock_case() { - HEAD_SHA=$MAIN_TIP PR_NUMBER='' LANE_PATH=$UNBLOCK RUN_TITLE="${TITLE-pr-unblock address-feedback #42}" REVIEW_RUN_ID=31 \ - REVIEW_RERUNNER_ID="${RERUNNER-$BOT}" expect "$@" -} -# unblock_jobs : the pr-unblock run's jobs. -unblock_jobs() { jq -cn --arg c "$1" '{jobs: [{id: 1, name: "address-feedback-select", conclusion: "success"}, {id: 2, name: "report-stop-status", conclusion: $c}]}'; } -STUB_UNBLOCK_JOBS="$(unblock_jobs success)" STUB_PR="$(pr)" STUB_LANE_RUNS="$BOTH_LANES" STUB_RUNS="$(runs "$(run 9)")" STUB_JOBS="$(cijob completed)" STUB_RUN_SEQ='' -STUB_STATUSES="[$(status ci/other failure "$AFTER"),$(status pr-unblock/address-feedback success "$LATER"),$(status pr-unblock/address-feedback failure "$EARLY")]" -unblock_case "a pr-unblock status set after ci-status read the head re-runs it" 0 "Re-running ci-status (job 77) of run 9 on $HEAD as attempt 2." -check_reran 1 "the PR's live head, not main's, is re-run and recorded" -check_calls 9 "a pr-unblock run reads its jobs, the PR twice, the statuses, both lanes, the runs, the jobs and the run again" -if grep -qxF "api -i repos/o/r/commits/$HEAD/statuses?per_page=100" "$TMP_ROOT/log" && ! grep -q "$MAIN_TIP" "$TMP_ROOT/log" && - [[ "$(grep -c '/pulls/42$' "$TMP_ROOT/log")" == 2 ]]; then - ok "the live head's statuses are read, main's tip never, and the PR again before the re-run" -else - fail "reads: $(paste -sd'|' "$TMP_ROOT/log")" -fi -if [[ "$(jq -r '"\(.lane_path) \(.reason)"' "$TMP_ROOT/record.json")" == "$UNBLOCK a pr-unblock status changed after ci-status read it" ]]; then - ok "the record names the pr-unblock run and why" -else - fail "record: $(cat "$TMP_ROOT/record.json")" -fi -STUB_STATUSES="[$(status ci/other success "$AFTER"),$(status pr-unblock/address-feedback success "$AFTER" someone-else)]" -unblock_case "a status of an unlisted context or writer is no change ci-status reads" 0 "No pr-unblock/* status on $HEAD; ci-status holds on none" -check_reran 0 "nothing is re-run for a status ci-status does not read" -check_calls 3 "it stops after its jobs, the PR and its statuses" -STUB_STATUSES="[$(status pr-unblock/fix-ci success "$EARLY")]" STUB_LANE_RUNS="$(runs "$(run 3 completed "$HEAD" "$CODE" pull_request "$EARLY")")" -STUB_JOBS="$(cijob completed "$DONE")" unblock_case "a ci-status started after the newest pr-unblock status read it" 0 "after both lanes finished ($EARLY); it read them." -check_reran 0 "a ci-status that read the status is left alone" -STUB_LANE_RUNS="$BOTH_LANES" STUB_STATUSES="[$(status pr-unblock/address-feedback success "$LATER")]" -STUB_PR="$(pr closed)" unblock_case "a closed PR is left alone" 0 "#42 is closed or has no head; nothing to re-run." -check_calls 2 "a closed PR stops after its jobs and the PR" -for title in 'pr-unblock address-feedback #0' 'pr-unblock address-feedback 42' 'other #42' 'pr-unblock address-feedback #42 x'; do - TITLE=$title unblock_case "run name '$title' names no PR" 0 "the upstream run names no pull request" - if grep -q "^api " "$TMP_ROOT/log"; then fail "a run name with no PR reaches no API: $(paste -sd"|" "$TMP_ROOT/log")"; else ok "a run name with no PR reaches no API"; fi -done -# unblock_jobs , or no such job: the run wrote no status. -for c in failure skipped 'no conclusion'; do - STUB_UNBLOCK_JOBS="$(unblock_jobs "${c/no conclusion/}")" unblock_case "a pr-unblock run whose report-stop-status has $c re-runs nothing" 0 "wrote no status (its report-stop-status job did not succeed)" - check_reran 0 "a run whose report-stop-status has $c re-runs nothing" - check_calls 1 "it stops after the run's jobs ($c)" -done -STUB_UNBLOCK_JOBS='{"jobs":[]}' unblock_case "a pr-unblock run with no report-stop-status job re-runs nothing" 0 "wrote no status (its report-stop-status job did not succeed)" -check_reran 0 "a run with no report-stop-status job re-runs nothing" -check_calls 1 "it stops after the run's jobs (no such job)" -# Statuses come newest first, as GitHub lists them. Any github-actions[bot] -# pr-unblock/* status newer than ci-status's start less the 60 s margin -# re-runs it, whatever its state: a repeated stop costs one extra re-run. -STARTED=2026-10-23T09:55:00Z -STUB_STATUSES="[$(status pr-unblock/address-feedback failure "$LATER"),$(status pr-unblock/address-feedback failure "$EARLY")]" -STUB_JOBS="$(cijob completed "$STARTED")" unblock_case "a failure re-posted over a failure re-runs ci-status" 0 "Re-running ci-status (job 77)" -check_reran 1 "a repeated stop re-runs once" -# fix-ci fails 90 s after ci-status started and clears at 200 s: ci-status -# may have read the failure, so the clear re-runs it. -STUB_STATUSES="[$(status pr-unblock/fix-ci success 2026-10-23T09:58:20Z),$(status pr-unblock/fix-ci failure 2026-10-23T09:56:30Z)]" -STUB_LANE_RUNS="$(runs "$(run 3 completed "$HEAD" "$CODE" pull_request "$EARLY")")" -STUB_JOBS="$(cijob completed "$STARTED")" unblock_case "a stop and clear both after ci-status started re-run it" 0 "Re-running ci-status (job 77)" -check_reran 1 "a clear after an unseen-or-seen stop re-runs" -# A status set more than 60 s before ci-status started was read. -STUB_STATUSES="[$(status pr-unblock/fix-ci success 2026-10-23T09:53:59Z)]" -STUB_JOBS="$(cijob completed "$STARTED")" unblock_case "a status ci-status started after is already read" 0 "after both lanes finished (2026-10-23T09:53:59Z); it read them." -check_reran 0 "an already read status re-runs nothing" -# A full first page may hide a newer pr-unblock status: re-run anyway. -page="$(jq -cn --arg at 2026-10-23T09:00:00Z '[range(100) | {context: "ci/other", state: "success", created_at: $at, creator: {login: "someone"}}]')" -STUB_STATUSES=$page STUB_JOBS="$(cijob completed "$STARTED")" unblock_case "a full page with no pr-unblock status re-runs ci-status" 0 "Re-running ci-status (job 77)" -check_reran 1 "a full page fails open" -STUB_STATUSES="$(jq -c --arg at 2026-10-23T09:00:00Z '.[0] = {context: "pr-unblock/fix-ci", state: "success", created_at: $at, creator: {login: "github-actions[bot]"}}' <<<"$page")" -STUB_JOBS="$(cijob completed "$STARTED")" unblock_case "a full page whose pr-unblock status ci-status read still re-runs it" 0 "Re-running ci-status (job 77)" -check_reran 1 "a full page skips the already-read exit" -STUB_LANE_RUNS="$BOTH_LANES" -STUB_STATUSES="[$(status pr-unblock/address-feedback success "$LATER"),$(status pr-unblock/address-feedback failure "$EARLY")]" -# The head moving between the first PR read and the one before the re-run is left alone. -STUB_PR_SEQ="$(pr) -$(pr open "${HEAD//0/e}")" unblock_case "a head that moved while ci-status was read is left alone" 0 "#42 is closed or no longer at $HEAD; nothing to re-run." -check_reran 0 "a superseded head is neither re-run nor recorded" -RERUNNER=$STRANGER unblock_case "a pr-unblock run another account dispatched still re-runs" 0 "the re-run is not recorded" -if [[ "$(cat "$TMP_ROOT/out")" == "job-id=77" && ! -e "$TMP_ROOT/record.json" ]]; then ok "it is re-run unrecorded"; else fail "outputs: $(paste -sd'|' "$TMP_ROOT/out")"; fi -RERUNNER=$KYLE unblock_case "a pr-unblock run a listed human dispatched is recorded" 0 "Re-running ci-status (job 77)" -check_reran 1 "a listed human's dispatch is recorded" approval_case() { LANE_PATH=$APPROVED REVIEW_RERUNNER_ID="${RERUNNER-$KYLE}" expect "$@"; } approval_case "a listed person's approval re-runs ci-status" 0 "Re-running ci-status (job 77) of run 9 on $HEAD as attempt 2." @@ -644,13 +561,10 @@ STUB_RUN_SEQ='' # for the lost-run reconciler, from every upstream; a person's PR is re-run. lanes_pr() { jq -cn --arg sha "$HEAD" --argjson id "${1:-$LANES_APP}" --arg type "${2:-Bot}" '{state: "open", head: {sha: $sha}, user: {id: $id, type: $type}}'; } STUB_LANE_RUNS="$BOTH_LANES" STUB_RUNS="$(runs "$(run 9)")" STUB_JOBS="$(cijob completed)" -STUB_STATUSES="[$(status pr-unblock/address-feedback success "$LATER")]" for proceed in false ''; do STUB_PR="$(lanes_pr)" LANES_PROCEED=$proceed APP_SENDER_ID=$LANES_APP expect "proceed '${proceed:-unread}': a lanes App PR after a review lane is not re-run" 0 "::notice::the lanes kill switch is on or unread, so ci-status on lanes App #42 is not re-run" check_reran 0 "proceed '${proceed:-unread}': no job id or record after a review lane" - LANES_PROCEED=$proceed APP_SENDER_ID=$LANES_APP unblock_case "proceed '${proceed:-unread}': a lanes App PR after pr-unblock is not re-run" 0 "is not re-run; the lost-run reconciler" - check_reran 0 "proceed '${proceed:-unread}': no job id or record after pr-unblock" LANES_PROCEED=$proceed APP_SENDER_ID=$LANES_APP approval_case "proceed '${proceed:-unread}': a lanes App PR after an approval is not re-run" 0 "is not re-run; the lost-run reconciler" check_reran 0 "proceed '${proceed:-unread}': no job id or record after an approval" done From ead7f9c9ccb3f0b8523857497909441bb696ea6a Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 3 Nov 2026 00:34:59 -0500 Subject: [PATCH 2/2] docs(pr-pipeline): correct the stale hold text and audit citations The native auto-merge section, the reconciler's comment and the hold test's header still described a body hold and a pr-unblock stop re-run that no longer exist. The outage audit's two pr-feedback-dispatch.yml citations moved with the re-run's removal. Refs: #12446 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VQb9Q49h99qKdrunU4xw3S --- .../maintenance-reconcile-lost-runs/reconcile.mjs | 1 - docs/conventions/pr-pipeline/README.md | 4 ++-- docs/outage-posture-audit.md | 4 ++-- scripts/ci-lanes-pr-hold.test.sh | 10 +++++----- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/.github/scripts/maintenance-reconcile-lost-runs/reconcile.mjs b/.github/scripts/maintenance-reconcile-lost-runs/reconcile.mjs index 6d1855bcce6..157ad532c91 100755 --- a/.github/scripts/maintenance-reconcile-lost-runs/reconcile.mjs +++ b/.github/scripts/maintenance-reconcile-lost-runs/reconcile.mjs @@ -909,7 +909,6 @@ async function plan({ repo, call, say, since, now, readStatus, judgeSpec, laneAp // A newest checks run that failed only in ci-status: a review a kill // switch stopped is re-run once nothing would stop it again, a lanes App // PR's that the kill switch failed is re-run now that the switch is off, - // any PR's that a pr-unblock stop failed once no stop fails the head, // and any PR's that a running review failed once the reviews are done. // The tested run's jobs are reused when it is that run. if (set.length === 0 && isRedChecks(checks)) { diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index 4c22185ec5a..7ec7af6b015 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -1949,8 +1949,8 @@ nothing. A pr-unblock run the switch stopped mid-run posts its check neutral `ki dispatcher counts as handling the head, so the next dispatcher run after the lift (the half-hourly `pr-unblock-dispatch.yml` tick, or a review completing at the head) dispatches it again, at most `limits.limit-stops-per-head` times per head, `SWEEP_HEAD_CAP` for `address-feedback`. -`ci-status` enforces the same holds on every lanes App pull request, the two text holds and the -hold paths, so one added after arming stops the merge without waiting for this job: its step +`ci-status` enforces a member's veto comment and the hold paths on every lanes App pull request, +so one added after arming stops the merge without waiting for this job: its step `Fail a lanes App pull request on kill switch, hold path, verifier or veto` runs `.github/scripts/pr-merge/merge-holds.mjs` as main has it, the module `enable-auto-merge.mjs` arms by, over the live body, discussion and changed files (renamed-from paths included) against main's diff --git a/docs/outage-posture-audit.md b/docs/outage-posture-audit.md index 303b0bb8cad..56274d27b7c 100644 --- a/docs/outage-posture-audit.md +++ b/docs/outage-posture-audit.md @@ -175,8 +175,8 @@ The lane runs from `.github/workflows/pr-unblock.yml`, with the dispatchers | 2 | gap | `.github/scripts/pr-unblock/dispatch.mjs:254`, `.github/scripts/pr-unblock/dispatch.mjs:835` | A non-5xx read failure counts as `failed` and exits 1. | | 3 | gap | `.github/scripts/pr-unblock/dispatch.mjs:229`, `.github/scripts/pr-unblock/limit-stop.mjs:21` | Failed runs, outage failures included, use up a cap for each head that blocks re-dispatch until a new push. That is a stored breaker. | | 4 | gap | `.github/scripts/pr-unblock/escalation-routes.json:27`, `.github/scripts/pr-unblock/report-escalation.sh:106` | A job fault, such as a lane token that was not obtained, files a fail route, and an unlisted reason holds the PR for a person. Its `clear` mode (`:74`) does clear its own status. | -| 5 | gap | `.github/workflows/pr-unblock-dispatch.yml:109`, `.github/workflows/pr-feedback-dispatch.yml:381` | `degraded` gates the dispatch steps. | -| 6 | gap | `.github/scripts/pr-unblock/gate-source.mjs:447`, `.github/workflows/pr-feedback-dispatch.yml:118` | `Promise.all` reads in parallel, and the sweep group uses `queue: max`. The `etag` use (`request-budget.mjs:86`) and the cron at minutes 9 and 39 are fine. | +| 5 | gap | `.github/workflows/pr-unblock-dispatch.yml:109`, `.github/workflows/pr-feedback-dispatch.yml:388` | `degraded` gates the dispatch steps. | +| 6 | gap | `.github/scripts/pr-unblock/gate-source.mjs:447`, `.github/workflows/pr-feedback-dispatch.yml:111` | `Promise.all` reads in parallel, and the sweep group uses `queue: max` (`:114`). The `etag` use (`request-budget.mjs:86`) and the cron at minutes 9 and 39 are fine. | ## pr-merge diff --git a/scripts/ci-lanes-pr-hold.test.sh b/scripts/ci-lanes-pr-hold.test.sh index a9c0a2da8e9..99188a7b9e9 100755 --- a/scripts/ci-lanes-pr-hold.test.sh +++ b/scripts/ci-lanes-pr-hold.test.sh @@ -4,11 +4,11 @@ # gives a step with no `shell:` (bash -e), against stub `gh` and `sleep`. # # Native auto-merge (pr-merge.yml's enable-auto-merge job) merges a lanes App -# pull request without pr-merge's gate, so ci-status holds it on the holds -# that gate honors: a body hold (a `Hold:` line or "do not merge"), a member's -# newest comment saying not to merge, a changed or renamed-from path -# matching the merge lane's denied paths, and a trusted-actor list change that -# adds or widens an entry. It reads them live, on a pull +# pull request without pr-merge's gate, so ci-status holds it on the kill +# switch, a member's newest comment saying not to merge, a changed or +# renamed-from path matching the merge lane's denied paths, and a trusted-actor +# list change that adds or widens an entry. Body text holds nothing: the +# do-not-merge label is the veto. It reads them live, on a pull # request event and in the merge queue, runs the base commit's # merge-holds.mjs, and fails closed when it cannot tell. In the merge queue it # also runs pr-merge's verdict reader (babysit_verify_evidence.py, stubbed