From 819d6622fe7c3666c875d6c83c984bdb32442ef8 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sun, 1 Nov 2026 10:49:05 -0500 Subject: [PATCH 1/3] perf(pr-pipeline): resolve main's tip once and fold the hold labels into GraphQL in merge-group ci-status A new ci-status step lists refs/heads/main and refs/tags/main with one unauthenticated git ls-remote and outputs the tip only when the listing succeeded, names one 40-hex head and holds no tag named main. The carry readers (carry-run-cache.mjs read, head-ref-map.mjs read, listing-carry.mjs) get it as PR_MERGE_MAIN_TIP, and onMain then makes no git/ref read; each run is still compared with the tip. With no tip, onMain reads both refs over REST as before. The hold-label step reads the queued pull request's labels in its existing GraphQL query (labels(first:100){totalCount nodes{name}}), now run before the label judgement, and fails closed when the list is cut or missing, in place of the REST issues/{n}/labels read. Co-authored-by: Claude Opus 5.5 --- .github/scripts/pr-merge/carry-run-cache.mjs | 1 + .github/scripts/pr-merge/head-ref-map.mjs | 11 ++- .../scripts/pr-merge/head-ref-map.test.mjs | 23 +++++ .github/scripts/pr-merge/listing-carry.mjs | 1 + .github/workflows/pr-require-checks.yml | 85 +++++++++++++------ scripts/ci-hold-merge-group.test.sh | 46 ++++++---- 6 files changed, 123 insertions(+), 44 deletions(-) diff --git a/.github/scripts/pr-merge/carry-run-cache.mjs b/.github/scripts/pr-merge/carry-run-cache.mjs index 79cad4119a1..57f8766336b 100755 --- a/.github/scripts/pr-merge/carry-run-cache.mjs +++ b/.github/scripts/pr-merge/carry-run-cache.mjs @@ -62,6 +62,7 @@ // RUNNER_TEMP read: where the artifact is downloaded // PR_MERGE_READS_PATH read: the job's shared GET answers // (rotation.mjs sharedReads), if set +// PR_MERGE_MAIN_TIP read: main's tip (head-ref-map.mjs onMain), if set // GITHUB_API_URL import { appendFileSync, existsSync, readFileSync, renameSync, rmSync, writeFileSync } from "node:fs"; import process from "node:process"; diff --git a/.github/scripts/pr-merge/head-ref-map.mjs b/.github/scripts/pr-merge/head-ref-map.mjs index 99a1a8592fa..111acac60a6 100755 --- a/.github/scripts/pr-merge/head-ref-map.mjs +++ b/.github/scripts/pr-merge/head-ref-map.mjs @@ -37,6 +37,7 @@ // RUNNER_TEMP read: where the artifact is downloaded // PR_MERGE_READS_PATH read: the job's shared GET answers // (rotation.mjs sharedReads), if set +// PR_MERGE_MAIN_TIP read: main's tip from the job's ls-remote, if set // GITHUB_API_URL // HEAD_REFS_PATH the map read before the gate // BABYSIT_RUN_CACHE write: this run's cache (babysit_run_cache.py) @@ -86,7 +87,10 @@ function writeAtomic(path, text) { // main's tip, read once per process by the first onMain call, and only while // no tag is named main (a run on one reports head_branch main; // check-trusted-trigger.mjs readDefaultBranchTip). A failed or refused read -// is remembered too, so every later run in the process is refused. +// is remembered too, so every later run in the process is refused. A job that +// listed refs/heads/main and refs/tags/main in one `git ls-remote` passes the +// tip as PR_MERGE_MAIN_TIP, set only when that listing held no tag named +// main, and then no process reads either ref; unset or not 40 hex, it is read. let mainTip; export function forgetMainTip() { mainTip = undefined; @@ -94,9 +98,10 @@ export function forgetMainTip() { // Whether commit `sha` ran main's own code: that tip read held, and `sha` is // main's tip or behind it (requireBehindTip). Any failed read is a no. -export async function onMain({ repo, actions, sha }) { +export async function onMain({ repo, actions, sha, env = process.env }) { if (!SHA.test(sha ?? "")) return false; - mainTip ??= readDefaultBranchTip(actions, repo, "main", "off-main"); + const listed = env.PR_MERGE_MAIN_TIP ?? ""; + mainTip ??= SHA.test(listed) ? Promise.resolve(listed) : readDefaultBranchTip(actions, repo, "main", "off-main"); try { await requireBehindTip(actions, repo, sha, await mainTip, "off-main"); return true; diff --git a/.github/scripts/pr-merge/head-ref-map.test.mjs b/.github/scripts/pr-merge/head-ref-map.test.mjs index 10418992372..c65dfb34574 100644 --- a/.github/scripts/pr-merge/head-ref-map.test.mjs +++ b/.github/scripts/pr-merge/head-ref-map.test.mjs @@ -248,3 +248,26 @@ test("onMain reads the tag named main and main's tip once per process", async () // Still refused with a client that would answer 404: the refusal holds. assert.equal(await onMain({ repo: REPO, actions, sha: sha(7) }), false); }); + +// A tip the job listed with `git ls-remote` (PR_MERGE_MAIN_TIP) replaces both +// ref reads; each run is still compared with it, and a commit off main is +// still refused. A value that is not 40 hex is ignored and the refs are read. +test("onMain takes main's tip from PR_MERGE_MAIN_TIP without a ref read", async () => { + const calls = []; + const actions = async (method, url) => { + calls.push(url); + return mainCall(url, { offMain: [sha(9)] }); + }; + const env = { PR_MERGE_MAIN_TIP: "f".repeat(40) }; + forgetMainTip(); + assert.equal(await onMain({ repo: REPO, actions, sha: sha(7), env }), true); + assert.equal(await onMain({ repo: REPO, actions, sha: sha(9), env }), false); + assert.deepEqual(calls.filter((u) => u.includes("/git/ref/")), []); + assert.deepEqual(calls, [`/repos/${REPO}/compare/${sha(7)}...${"f".repeat(40)}`, `/repos/${REPO}/compare/${sha(9)}...${"f".repeat(40)}`]); + for (const bad of ["", "F".repeat(40), "f".repeat(39), `${"f".repeat(40)}\n`]) { + calls.length = 0; + forgetMainTip(); + assert.equal(await onMain({ repo: REPO, actions, sha: sha(7), env: { PR_MERGE_MAIN_TIP: bad } }), true); + assert.equal(calls.filter((u) => u.includes("/git/ref/")).length, 2, JSON.stringify(bad)); + } +}); diff --git a/.github/scripts/pr-merge/listing-carry.mjs b/.github/scripts/pr-merge/listing-carry.mjs index 224c3214598..e2342f86d60 100755 --- a/.github/scripts/pr-merge/listing-carry.mjs +++ b/.github/scripts/pr-merge/listing-carry.mjs @@ -25,6 +25,7 @@ // RUNNER_TEMP where the artifact is downloaded // PR_MERGE_READS_PATH the job's shared GET answers // (rotation.mjs sharedReads), if set +// PR_MERGE_MAIN_TIP main's tip (head-ref-map.mjs onMain), if set // BABYSIT_VERIFY_LISTING_CARRY the file to write // GITHUB_API_URL import { renameSync, rmSync, writeFileSync } from "node:fs"; diff --git a/.github/workflows/pr-require-checks.yml b/.github/workflows/pr-require-checks.yml index f222a54e9df..6359e5e1432 100644 --- a/.github/workflows/pr-require-checks.yml +++ b/.github/workflows/pr-require-checks.yml @@ -5740,11 +5740,13 @@ jobs: # entry's label would eject an unlabeled pull request while the labeled # one still merged. A label that lands after this step passed does not stop the merge; # dequeue the pull request as well. An unparsable ref, an unreadable - # tracker config or an unreadable label list fails closed. For a pull + # tracker config or an unreadable or cut label list fails closed. The + # labels come live from one GraphQL read, which also answers the link. + # For a pull # request an automation opened it then applies the trust hold to its # linked issue (comment in the body). report-outcome's Hold: line covers # the label at PR open and enable-auto-merge at arm time, so this holds a - # label added after either: one GraphQL read, a labels read for a Refs + # label added after either: that GraphQL read, a labels read for a Refs # issue, and one timeline read per linked issue; an unreadable one, or # the base's trusted-actor list unreadable, fails closed. Pinned by # scripts/ci-hold-merge-group.test.sh, which runs this body. @@ -5797,14 +5799,29 @@ jobs: echo "::error::cannot read the human-gated role label from .work-item-tracker.json at $BASE_SHA with its merge-holds.mjs" exit 1 } - for attempt in 1 2 3; do - if labels=$(gh_read "repos/$GITHUB_REPOSITORY/issues/$pr/labels?per_page=100" --paginate --jq '.[].name'); then - break - fi - [ "$attempt" -lt 3 ] || { echo "::error::could not read the labels of #$pr; re-run this run."; exit 1; } - sleep $((attempt * 5)) - done - jq -Rsc 'split("\n") | map(select(length > 0))' <<<"$labels" >"$RUNNER_TEMP/queued-labels.json" + # read3 ...: gh_read up to three times, 5 and 10 s apart. + read3() { + local attempt + for attempt in 1 2 3; do + gh_read "$@" && return 0 + [ "$attempt" -lt 3 ] || return 1 + sleep $((attempt * 5)) + done + } + # One GraphQL read answers the pull request's live labels and, for + # the trust hold below, its author, body and closing issues. A label + # list GitHub cut (totalCount above the nodes) fails closed. + # shellcheck disable=SC2016 # GraphQL variables, not shell + link=$(read3 graphql -f query='query($o:String!,$r:String!,$n:Int!){repository(owner:$o,name:$r){pullRequest(number:$n){labels(first:100){totalCount nodes{name}} author{__typename ... on Bot{databaseId}} body closingIssuesReferences(first:10){totalCount nodes{number repository{nameWithOwner} labels(first:100){totalCount nodes{name}}}}}}}' \ + -F o="${GITHUB_REPOSITORY%%/*}" -F r="${GITHUB_REPOSITORY#*/}" -F n="$pr") || { + echo "::error::could not read the labels and issue link of #$pr; re-run this run." + exit 1 + } + jq -ec '.data.repository.pullRequest.labels | objects | select(.totalCount == (.nodes | length)) | [.nodes[].name] + | if all(.[]; type == "string") then . else error("a label name is not a string") end' <<<"$link" >"$RUNNER_TEMP/queued-labels.json" || { + echo "::error::the labels of #$pr could not all be read; a person merges it." + exit 1 + } if ! reasons=$(label_holds holds "$module" "$RUNNER_TEMP/queued-labels.json" "$pr") || [ -z "$reasons" ] || ! failing=$(jq -r 'if type == "array" then .[] else error("not a list") end' <<<"$reasons"); then echo "::error::merge-holds.mjs at $BASE_SHA could not judge the labels of #$pr; re-run this run." @@ -5828,22 +5845,8 @@ jobs: # must link exactly one issue here, as _linked_issue refuses # otherwise, so a link dropped after arming fails; another # automation's (Dependabot, standards-sync, a plugin release) that - # links none passes. A jq error fails closed. - # read3 ...: gh_read up to three times, 5 and 10 s apart. - read3() { - local attempt - for attempt in 1 2 3; do - gh_read "$@" && return 0 - [ "$attempt" -lt 3 ] || return 1 - sleep $((attempt * 5)) - done - } - # shellcheck disable=SC2016 # GraphQL variables, not shell - link=$(read3 graphql -f query='query($o:String!,$r:String!,$n:Int!){repository(owner:$o,name:$r){pullRequest(number:$n){author{__typename ... on Bot{databaseId}} body closingIssuesReferences(first:10){totalCount nodes{number repository{nameWithOwner} labels(first:100){totalCount nodes{name}}}}}}}' \ - -F o="${GITHUB_REPOSITORY%%/*}" -F r="${GITHUB_REPOSITORY#*/}" -F n="$pr") || { - echo "::error::could not read the issue link of #$pr; re-run this run." - exit 1 - } + # links none passes. A jq error fails closed. The link is the + # GraphQL answer the labels came from. author=$(jq -er '.data.repository.pullRequest.author.__typename | strings' <<<"$link") || { echo "::error::the author of #$pr could not be read; re-run this run." exit 1 @@ -5969,6 +5972,34 @@ jobs: echo "lanes=false" >>"$GITHUB_OUTPUT" echo "#$pr is not the lanes App's; the carry steps are skipped." fi + # MAIN'S TIP, LISTED ONCE FOR THE CARRY READERS. carry-run-cache.mjs, + # head-ref-map.mjs and listing-carry.mjs each run in their own process + # and take a run's carried reads only when its head commit is on main + # (head-ref-map.mjs onMain), which otherwise reads the tag named main (a + # 404 that no process shares) and main's tip over REST. One + # unauthenticated `git ls-remote` of this public repository, run outside + # any checkout, lists both refs; the tip is output only when the listing + # succeeded, names refs/heads/main once as 40 hex and holds no tag named + # main. Anything else outputs nothing, and onMain reads both refs as + # before. Each run is still compared with the tip. + - name: List main's tip for the carry readers + id: main_tip + if: ${{ !cancelled() && github.event_name == 'merge_group' && steps.queued_author.outputs.lanes != 'false' }} + continue-on-error: true + run: | + if ! refs=$(GIT_TERMINAL_PROMPT=0 timeout 60 git -C "$RUNNER_TEMP" ls-remote "https://github.com/$GITHUB_REPOSITORY" refs/heads/main refs/tags/main); then + echo "::notice::could not list main's refs; the carry readers read them over REST." + exit 0 + fi + tip=$(awk '$2 == "refs/heads/main" { print $1 }' <<<"$refs") + if awk '$2 == "refs/tags/main" || $2 == "refs/tags/main^{}" { found = 1 } END { exit !found }' <<<"$refs"; then + echo "::notice::a tag is named main; the carry readers refuse every run." + elif [[ $tip =~ ^[0-9a-f]{40}$ ]]; then + echo "sha=$tip" >>"$GITHUB_OUTPUT" + echo "main's tip: $tip" + else + echo "::notice::main's tip was not listed as one commit SHA; the carry readers read it over REST." + fi # THE MERGE QUEUE'S VERDICT READER STARTS FROM PR-MERGE'S CARRIED READS. # The reads that never change (FAIL run records, FAIL head refs, run # heads on main) that pr-merge's last scheduled or hand-started run kept, @@ -5998,6 +6029,7 @@ jobs: API_BUDGET_TOKEN: github-token CARRY_TO: ${{ runner.temp }}/pr-merge-carried.json PR_MERGE_READS_PATH: ${{ runner.temp }}/pr-merge-reads.json + PR_MERGE_MAIN_TIP: ${{ steps.main_tip.outputs.sha }} run: node .base/.github/scripts/pr-merge/carry-run-cache.mjs read - name: Start the run cache from them id: verdict_reads_carry @@ -6197,6 +6229,7 @@ jobs: BABYSIT_VERIFY_HEAD_REFS: ${{ runner.temp }}/head-refs-in.json HEAD_REFS_PATH: ${{ runner.temp }}/head-refs-in.json PR_MERGE_READS_PATH: ${{ runner.temp }}/pr-merge-reads.json + PR_MERGE_MAIN_TIP: ${{ steps.main_tip.outputs.sha }} BABYSIT_RUN_CACHE: ${{ runner.temp }}/babysit-run-cache.json API_BUDGET_TOKEN: github-token PR_NUMBER: ${{ github.event.pull_request.number }} diff --git a/scripts/ci-hold-merge-group.test.sh b/scripts/ci-hold-merge-group.test.sh index 16dbbe055f2..ca596e75f55 100755 --- a/scripts/ci-hold-merge-group.test.sh +++ b/scripts/ci-hold-merge-group.test.sh @@ -75,11 +75,12 @@ fi # The base commit's config is a fixture directory: STUB_TRACKER is its # .work-item-tracker.json (absent when unset), and STUB_CHECKOUT_FAILED leaves # BASE_CONFIG empty, as a failed checkout does. The gh stub logs its -# arguments. A labels read fails its first STUB_FAILS calls, then prints -# STUB_LABELS one per line. +# arguments. mkdir -p "$TMP_ROOT/bin" -# The link read answers STUB_LINK (default: a person's pull request), or -# fails with STUB_LINK_FAIL; a timeline read prints STUB_TIMELINE, one +# The one GraphQL read fails its first STUB_FAILS calls, or every call with +# STUB_LINK_FAIL, then answers STUB_LINK (default: a person's pull request) +# with the pull request's labels set to STUB_LABELS (comma-separated) unless +# STUB_LINK names its own; a timeline read prints STUB_TIMELINE, one # unlabeled event per line, or fails with STUB_TIMELINE_FAIL; the Refs # issue's labels read prints STUB_ISSUE_LABELS. cat >"$TMP_ROOT/bin/gh" <<'EOF' @@ -88,10 +89,12 @@ printf '%s\n' "$*" >>"$STUB_LOG" case "$*" in *graphql*) [[ -z "${STUB_LINK_FAIL:-}" ]] || exit 1 + [[ "$(grep -c graphql "$STUB_LOG")" -gt "${STUB_FAILS:-0}" ]] || exit 1 # The api-budget wrapper adds -i to a single read: the headers come first. [[ "$2" != -i ]] || printf 'HTTP/2.0 200 OK\r\nX-Ratelimit-Remaining: 4000\r\nX-Ratelimit-Resource: graphql\r\n\r\n' person='{"data":{"repository":{"pullRequest":{"author":{"__typename":"User"}}}}}' - printf '%s\n' "${STUB_LINK:-$person}" + jq -c --arg labels "${STUB_LABELS:-}" 'if .data.repository.pullRequest | has("labels") then . else + .data.repository.pullRequest.labels = ($labels | split(",") | map(select(length > 0)) | {totalCount: length, nodes: map({name: .})}) end' <<<"${STUB_LINK:-$person}" ;; */timeline*) [[ -z "${STUB_TIMELINE_FAIL:-}" ]] || exit 1 @@ -100,10 +103,6 @@ case "$*" in *issues/18/labels*) [[ -z "${STUB_ISSUE_LABELS:-}" ]] || tr ',' '\n' <<<"$STUB_ISSUE_LABELS" ;; -*/labels*) - [[ "$(grep -c '/labels' "$STUB_LOG")" -gt "${STUB_FAILS:-0}" ]] || exit 1 - [[ -z "$STUB_LABELS" ]] || tr ',' '\n' <<<"$STUB_LABELS" - ;; *) exit 1 ;; esac EOF @@ -132,6 +131,9 @@ CUSTOM='{"config":{"role_labels":{"human-gated":" owner-call "}}}' ACTORS='{"version":1,"actors":[{"id":100,"login":"maintainer","kind":"human"},{"id":200,"login":"agent-lanes[bot]","kind":"bot"}]}' # expect