From 39c44637d8ce4c847c61e0595743b1df1d9e55d6 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 3 Nov 2026 23:59:44 -0500 Subject: [PATCH 1/2] perf(pr-verify): skip the pr-merge wake for an armed or held pull request The wake's one author read now also returns whether the pull request is already armed for auto-merge and whether it carries do-not-merge. A lanes App pull request in either state gets a notice and no pr-merge dispatch: the scheduled sweep runs the same decide() on it, disarms included, and ci-status holds do-not-merge in the merge queue, so the wake only spent a pr-merge run. Person pull requests stay unwoken: the token broker mints for a verdict wake only when the lanes App opened the pull request. Co-Authored-By: Claude Opus 5.5 --- .../perf-wake-skip-armed-held-e1011cc7.md | 5 +++ .github/scripts/pr-verify/workflow.test.mjs | 42 ++++++++++++------- .github/workflows/pr-verify.yml | 23 +++++++--- docs/conventions/pr-pipeline/README.md | 3 +- 4 files changed, 53 insertions(+), 20 deletions(-) create mode 100644 .changes/pr-pipeline/perf-wake-skip-armed-held-e1011cc7.md diff --git a/.changes/pr-pipeline/perf-wake-skip-armed-held-e1011cc7.md b/.changes/pr-pipeline/perf-wake-skip-armed-held-e1011cc7.md new file mode 100644 index 00000000000..f412aa2fda6 --- /dev/null +++ b/.changes/pr-pipeline/perf-wake-skip-armed-held-e1011cc7.md @@ -0,0 +1,5 @@ +--- +bump: none +--- + +No release: pr-verify's `wake-pr-merge` job no longer dispatches pr-merge for a lanes App pull request that is already armed or carries `do-not-merge`; the scheduled sweep makes the same decision for those, so only the wake's run count changes. diff --git a/.github/scripts/pr-verify/workflow.test.mjs b/.github/scripts/pr-verify/workflow.test.mjs index f47e3fa8de9..be6bc692194 100644 --- a/.github/scripts/pr-verify/workflow.test.mjs +++ b/.github/scripts/pr-verify/workflow.test.mjs @@ -1202,20 +1202,34 @@ test("wake-pr-merge dispatches nothing on a fail, no verdict or a pull request n } }); -test("wake-pr-merge wakes only a lanes App pull request when it can read the author", () => { - const read = `api repos/${REPO}/pulls/41 --jq "\\(.user.id) \\(.user.type)"`; - const dispatch = `workflow run pr-merge.yml -R ${REPO} --ref main -f pr=41`; - const pass = { KEY_PR: "41", KEY_VERDICT: "pass", LANES_APP_ID: "7" }; - // The lanes App's pull request: read, then woken. - assert.deepEqual(wake({ ...pass, GH_AUTHOR: "7 Bot" }).gh, [read, dispatch]); - // Another author, or the App's id on a User: read, nothing woken. - for (const author of ["9 User", "7 User", "8 Bot"]) { - const run = wake({ ...pass, GH_AUTHOR: author }); +const WAKE_READ = `api repos/${REPO}/pulls/41 --jq "\\(.user.id) \\(.user.type) \\(.auto_merge != null) \\([.labels[].name] | index("do-not-merge") != null)"`; +const WAKE_DISPATCH = `workflow run pr-merge.yml -R ${REPO} --ref main -f pr=41`; +const WAKE_PASS = { KEY_PR: "41", KEY_VERDICT: "pass", LANES_APP_ID: "7" }; + +test("wake-pr-merge wakes only an unarmed, unheld lanes App pull request when it can read it", () => { + // The lanes App's pull request, unarmed and unheld: read, then woken. + assert.deepEqual(wake({ ...WAKE_PASS, GH_AUTHOR: "7 Bot false false" }).gh, [WAKE_READ, WAKE_DISPATCH]); + // A person's, Dependabot's, another App's, or the App's id on a User: read, nothing woken. + for (const author of ["9 User false false", "7 User false false", "8 Bot false false", "49699333 Bot false false"]) { + const run = wake({ ...WAKE_PASS, GH_AUTHOR: author }); assert.equal(run.status, 0, author); - assert.deepEqual(run.gh, [read], author); - assert.match(run.stdout, /not a lanes App pull request; nothing woken/, author); + assert.deepEqual(run.gh, [WAKE_READ], author); + assert.match(run.stdout, /::notice title=pr-verify wake::pr=41 verdict=pass not a lanes App pull request; nothing woken/, author); + } + // A failed read, or no usable lanes App id: woken anyway, pr-merge decides. + assert.deepEqual(wake(WAKE_PASS).gh, [WAKE_READ, WAKE_DISPATCH]); + for (const id of ["", "x"]) assert.deepEqual(wake({ ...WAKE_PASS, LANES_APP_ID: id, GH_AUTHOR: "9 User false false" }).gh, [WAKE_DISPATCH], id); +}); + +test("wake-pr-merge wakes no lanes App pull request already armed or held by do-not-merge", () => { + for (const [answer, why] of [ + ["7 Bot true false", "already armed"], + ["7 Bot true true", "already armed"], + ["7 Bot false true", "held \\(do-not-merge\\)"], + ]) { + const run = wake({ ...WAKE_PASS, KEY_VERDICT: "pass-notes", GH_AUTHOR: answer }); + assert.equal(run.status, 0, answer); + assert.deepEqual(run.gh, [WAKE_READ], answer); + assert.match(run.stdout, new RegExp(`::notice title=pr-verify wake::pr=41 verdict=pass-notes ${why}; nothing woken`), answer); } - // A failed read, or no usable lanes App id: woken anyway, pr-merge filters. - assert.deepEqual(wake(pass).gh, [read, dispatch]); - for (const id of ["", "x"]) assert.deepEqual(wake({ ...pass, LANES_APP_ID: id, GH_AUTHOR: "9 User" }).gh, [dispatch], id); }); diff --git a/.github/workflows/pr-verify.yml b/.github/workflows/pr-verify.yml index c160c0b4717..9fc1d56d599 100644 --- a/.github/workflows/pr-verify.yml +++ b/.github/workflows/pr-verify.yml @@ -5562,15 +5562,28 @@ jobs: echo "::warning::wake-pr-merge: the passing job named no pull request number; nothing woken" exit 0 fi - # pr-merge arms only lanes App pull requests, so one author read here - # spares every other pass a whole pr-merge run. A failed read, or no - # lanes App id, dispatches anyway: pr-merge filters by author itself. + # pr-merge arms only lanes App pull requests, and the token broker + # mints for this wake only on one, so one read here spares every + # other pass a whole pr-merge run. One already armed or held by + # do-not-merge needs no wake: the scheduled sweep runs the same + # decision on it, disarms included, and ci-status holds do-not-merge + # in the merge queue. A failed read, or no lanes App id, dispatches + # anyway: pr-merge decides all of this itself. if [[ ${LANES_APP_ID:-} =~ ^[1-9][0-9]*$ ]] && - author=$(gh api "repos/$GITHUB_REPOSITORY/pulls/$pr" --jq '"\(.user.id) \(.user.type)"'); then - if [ "$author" != "$LANES_APP_ID Bot" ]; then + read -r id type armed held < <(gh api "repos/$GITHUB_REPOSITORY/pulls/$pr" \ + --jq '"\(.user.id) \(.user.type) \(.auto_merge != null) \([.labels[].name] | index("do-not-merge") != null)"'); then + if [ "$id $type" != "$LANES_APP_ID Bot" ]; then echo "::notice title=pr-verify wake::pr=$pr verdict=$verdict not a lanes App pull request; nothing woken" exit 0 fi + if [ "$armed" = true ]; then + echo "::notice title=pr-verify wake::pr=$pr verdict=$verdict already armed; nothing woken" + exit 0 + fi + if [ "$held" = true ]; then + echo "::notice title=pr-verify wake::pr=$pr verdict=$verdict held (do-not-merge); nothing woken" + exit 0 + fi fi if gh workflow run pr-merge.yml -R "$GITHUB_REPOSITORY" --ref main -f pr="$pr"; then echo "::notice title=pr-verify wake::pr=$pr verdict=$verdict pr-merge dispatched" diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index 97dd293411e..dab91b24fa6 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -1961,7 +1961,8 @@ the body must still close, or with no closing issue name by a plain `Refs:` line pass judged, linked the same way, alone or beside others (`merge-holds.mjs` `linkHold`), so a body edited after arming to drop its link or name another issue does not merge on that pass. A pass or pass-notes verdict wakes it for that one pull request: `pr-verify.yml`'s `wake-pr-merge` job -dispatches a PR-scoped `pr-merge.yml` run (`-f pr=`, run name `pr-merge: arm #`) that +reads the pull request once and, for a lanes App one that is not already armed and carries no +`do-not-merge`, dispatches a PR-scoped `pr-merge.yml` run (`-f pr=`, run name `pr-merge: arm #`) that reads and arms only that pull request, its lane room from the last sweep's post-merge report, and skips the merge job, `post-merge-dispatch` and every other sweep job. That report stands only while `settle --idle-tick` keeps it; when it does not, the wake never reuses it but runs the From e1ab0dca8a5e34b49a24761032e20bd5a0995886 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:06:14 -0500 Subject: [PATCH 2/2] fix(pr-verify): let gh's exit status decide a failed wake read Reading the author through process substitution took read's status, not gh's: a multi-line HTTP error body made read succeed on its first line, and the wake logged "not a lanes App pull request" instead of dispatching as a failed read should. The answer is captured first, so a failed gh call dispatches as before. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/pr-verify/workflow.test.mjs | 7 +++++-- .github/workflows/pr-verify.yml | 5 +++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/.github/scripts/pr-verify/workflow.test.mjs b/.github/scripts/pr-verify/workflow.test.mjs index 6b828831875..1baad8f699d 100644 --- a/.github/scripts/pr-verify/workflow.test.mjs +++ b/.github/scripts/pr-verify/workflow.test.mjs @@ -1187,10 +1187,11 @@ const wake = (env) => { mkdirSync(bin); const args = path.join(dir, "args"); writeFileSync(args, ""); - // The stand-in answers an author read with GH_AUTHOR, or fails it when that is unset. + // The stand-in answers an author read with GH_AUTHOR, fails it when that is + // unset, or prints GH_ERROR_BODY and fails, as gh does on an HTTP error. writeFileSync( path.join(bin, "gh"), - `#!/bin/sh\necho "$@" >>'${args}'\nif [ "$1" = api ]; then [ -n "$GH_AUTHOR" ] || exit 1; echo "$GH_AUTHOR"; fi\n`, + `#!/bin/sh\necho "$@" >>'${args}'\nif [ "$1" = api ]; then [ -z "$GH_ERROR_BODY" ] || { printf '%s' "$GH_ERROR_BODY"; exit 1; }; [ -n "$GH_AUTHOR" ] || exit 1; echo "$GH_AUTHOR"; fi\n`, { mode: 0o755 }, ); const run = spawnSync("bash", ["--noprofile", "--norc", "-c", script], { @@ -1238,6 +1239,8 @@ test("wake-pr-merge wakes only an unarmed, unheld lanes App pull request when it } // A failed read, or no usable lanes App id: woken anyway, pr-merge decides. assert.deepEqual(wake(WAKE_PASS).gh, [WAKE_READ, WAKE_DISPATCH]); + // A failed read whose error body spans lines (an edge 502 page) is still a failed read. + assert.deepEqual(wake({ ...WAKE_PASS, GH_ERROR_BODY: "\n502 Bad Gateway\n\n" }).gh, [WAKE_READ, WAKE_DISPATCH]); for (const id of ["", "x"]) assert.deepEqual(wake({ ...WAKE_PASS, LANES_APP_ID: id, GH_AUTHOR: "9 User false false" }).gh, [WAKE_DISPATCH], id); }); diff --git a/.github/workflows/pr-verify.yml b/.github/workflows/pr-verify.yml index 7aad2fbba68..03ec81ee583 100644 --- a/.github/workflows/pr-verify.yml +++ b/.github/workflows/pr-verify.yml @@ -5720,8 +5720,9 @@ jobs: # in the merge queue. A failed read, or no lanes App id, dispatches # anyway: pr-merge decides all of this itself. if [[ ${LANES_APP_ID:-} =~ ^[1-9][0-9]*$ ]] && - read -r id type armed held < <(gh api "repos/$GITHUB_REPOSITORY/pulls/$pr" \ - --jq '"\(.user.id) \(.user.type) \(.auto_merge != null) \([.labels[].name] | index("do-not-merge") != null)"'); then + answer=$(gh api "repos/$GITHUB_REPOSITORY/pulls/$pr" \ + --jq '"\(.user.id) \(.user.type) \(.auto_merge != null) \([.labels[].name] | index("do-not-merge") != null)"') && + read -r id type armed held <<<"$answer"; then if [ "$id $type" != "$LANES_APP_ID Bot" ]; then echo "::notice title=pr-verify wake::pr=$pr verdict=$verdict not a lanes App pull request; nothing woken" exit 0