Skip to content

perf(pr-pipeline): cut ci-status's GITHUB_TOKEN reads #9433

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kyle-sexton merged 6 commits into from
Oct 23, 2026
Merged

perf(pr-pipeline): cut ci-status's GITHUB_TOKEN reads #9433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cc490aa
perf(pr-pipeline): cut ci-status's GITHUB_TOKEN reads
kyle-sexton Oct 23, 2026
0ead0d5
docs(pr-pipeline): scope the checkout REST note to v7.0.1
kyle-sexton Oct 23, 2026
5d6d6fa
Merge branch 'main' into perf/ci-status-fewer-token-calls
kyle-sexton Oct 23, 2026
c3a40c7
test(pr-pipeline): pin the broker step's multi-page file read
kyle-sexton Oct 23, 2026
5529f07
fix(pr-pipeline): opt the base-config checkout out of the lane feed
kyle-sexton Oct 23, 2026
c0ad9ba
Merge remote-tracking branch 'origin/main' into perf/ci-status-fewer-…
kyle-sexton Oct 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 73 additions & 42 deletions .github/workflows/pr-require-checks.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3109,11 +3109,13 @@ jobs:
# (`rate-limit-wait-seconds` on check-contract and aggregate-results), 45 s
# of own-gate retries, 7 s of status-write retries and the 90 s
# `rerun-wait-seconds` wait, about four and a half minutes, plus up to
# 15 s of retry sleeps for each of the hold-lift step's three reads, the
# 15 s of retry sleeps for each of the hold-lift step's two reads, the
# review-lane step's four and the lanes-hold step's eight, about four
# minutes, plus the cost-hold step's reads: three for most pull requests,
# minutes, plus the cost-hold step's reads: one for most pull requests,
# which stop at the label events; for a lane-labeled one whose body cites a
# decision, about ten, and two more per review run since the window opened.
# decision, about eight, and two more per review run since the window
# opened, plus the base-config checkout's fetch (seconds; its retries
# replace the contents reads' 15 s each).
timeout-minutes: 11
permissions:
# `yield-to-full-run` lists this workflow's runs on the head SHA, and
Expand Down Expand Up @@ -3163,12 +3165,15 @@ jobs:
BROKER_PATHS='.github/actions/resolve-config/package.json
.github/actions/resolve-config/package-lock.json'
for attempt in 1 2 3; do
if files=$(gh api "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/files?per_page=100" --paginate --jq '.[] | .filename, (.previous_filename // empty)'); then
if pages=$(gh api "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/files?per_page=100" --paginate); then
break
fi
[ "$attempt" -lt 3 ] || { echo "::error::could not read the files of #$PR_NUMBER; re-run this run."; exit 1; }
sleep $((attempt * 5))
done
files=$(jq -r '.[] | .filename, (.previous_filename // empty)' <<<"$pages")
# The review-lane step reads the same list from here instead of again.
printf '%s\n' "$pages" >"$RUNNER_TEMP/pr-files.json"
touched=
for path in $BROKER_PATHS; do
if grep -qxF "$path" <<<"$files"; then touched="$touched $path"; fi
Expand Down Expand Up @@ -3252,6 +3257,32 @@ jobs:
msg="draft: lanes not run; the ready_for_review run lints and tests this SHA"
echo "::notice::$msg"
echo "$msg" >>"$GITHUB_STEP_SUMMARY"
# THE HOLD STEPS READ THEIR CONFIG FROM THE BASE COMMIT, ONCE. The tracker
# config names the human-gated label and the trusted-actor list names who
# may lift a hold; both are read at the base so the pull request cannot
# rewrite its own hold. A sparse checkout of the two files at the base SHA
# is a git fetch, not a REST request: as of actions/checkout v7.0.1 its
# API calls (default branch, object format, the stale-merge tracepoint, the
# no-git archive download) all return early for a 40-hex `ref` on a runner
# with git; recheck on a checkout bump. It checks out no pull request
# code: the ref is the base commit, nothing here is executed, and no
# credential is persisted.
# The steps below read BASE_CONFIG: empty when this step did not succeed,
# which each of them treats as an unreadable config; a file missing from
# a checkout that succeeded is a file the base commit does not have.
- name: Check out the base commit's hold config
id: base_config
if: ${{ !cancelled() && (github.event_name == 'pull_request' || github.event_name == 'merge_group') }}
continue-on-error: true
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
ref: ${{ github.event.pull_request.base.sha || github.event.merge_group.base_sha }}
path: base-config
persist-credentials: false
sparse-checkout: |
/.work-item-tracker.json
/.github/standards/trusted-actors/trusted-actors.json
sparse-checkout-cone-mode: false
# A COST HOLD THE OWNER ALREADY DECIDED COMES OFF. A review lane re-holds
# every new head, so its `cost` hold went back on after the owner had
# recorded the cost decision. This step clears a hold only when all
Expand Down Expand Up @@ -3294,6 +3325,7 @@ jobs:
BASE_SHA: ${{ github.event.pull_request.base.sha }}
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
LANES_APP_ID: ${{ vars.AUTOMATION_LANES_APP_SENDER_ID }}
BASE_CONFIG: ${{ steps.base_config.outcome == 'success' && format('{0}/base-config', github.workspace) || '' }}
run: |
keep() {
echo "The cost hold stays for a person: $1."
Expand All @@ -3314,15 +3346,18 @@ jobs:
[[ $BASE_SHA =~ ^[0-9a-f]{40}$ && $HEAD_SHA =~ ^[0-9a-f]{40}$ && $PR_NUMBER =~ ^[1-9][0-9]*$ ]] || keep "the event names no commit or pull request"
[[ $LANES_APP_ID =~ ^[1-9][0-9]*$ ]] || keep "AUTOMATION_LANES_APP_SENDER_ID is not set"
repo=$GITHUB_REPOSITORY
tracker=$(read_api -H 'Accept: application/vnd.github.raw+json' "repos/$repo/contents/.work-item-tracker.json?ref=$BASE_SHA") && rc=0 || rc=$?
[ "$rc" -ne 1 ] || keep "the tracker config is unreadable"
[ -d "$BASE_CONFIG" ] || keep "the tracker config is unreadable"
tracker=
if [ -e "$BASE_CONFIG/.work-item-tracker.json" ]; then
tracker=$(cat "$BASE_CONFIG/.work-item-tracker.json") || keep "the tracker config is unreadable"
fi
label=needs-human
if [ "$rc" -eq 0 ] && [ -n "$tracker" ]; then
if [ -n "$tracker" ]; then
label=$(jq -er '[objects | .config | objects | .role_labels | objects
| .["human-gated"] | strings | gsub("^\\s+|\\s+$"; "")][0] // ""
| if . == "" then "needs-human" else . end' <<<"$tracker") || keep "the tracker config is invalid"
fi
actors=$(read_api -H 'Accept: application/vnd.github.raw+json' "repos/$repo/contents/.github/standards/trusted-actors/trusted-actors.json?ref=$BASE_SHA") || keep "the trusted-actor list is unreadable"
actors=$(cat "$BASE_CONFIG/.github/standards/trusted-actors/trusted-actors.json") || keep "the trusted-actor list is unreadable"
humans=$(jq -ec '[.actors[] | select(.kind == "human") | .id | numbers] | select(length > 0)' <<<"$actors") || keep "the trusted-actor list names no person"
events=$(read_api "repos/$repo/issues/$PR_NUMBER/events?per_page=100" --paginate) || keep "the events are unreadable"
# The window opens at the newest removal by a trusted person.
Expand Down Expand Up @@ -3458,21 +3493,17 @@ jobs:
GH_TOKEN: ${{ github.token }}
PR_NUMBER: ${{ github.event.pull_request.number }}
BASE_SHA: ${{ github.event.pull_request.base.sha }}
BASE_CONFIG: ${{ steps.base_config.outcome == 'success' && format('{0}/base-config', github.workspace) || '' }}
run: |
if [[ ! $BASE_SHA =~ ^[0-9a-f]{40}$ ]]; then
echo "::error::the base commit '$BASE_SHA' is not a commit SHA."
exit 1
fi
for attempt in 1 2 3; do
if tracker=$(gh api -H 'Accept: application/vnd.github.raw+json' "repos/$GITHUB_REPOSITORY/contents/.work-item-tracker.json?ref=$BASE_SHA" 2>"$RUNNER_TEMP/tracker.err"); then
break
elif grep -q 'HTTP 404' "$RUNNER_TEMP/tracker.err"; then
tracker=
break
fi
[ "$attempt" -lt 3 ] || { echo "::error::could not read .work-item-tracker.json at $BASE_SHA; re-run this run."; exit 1; }
sleep $((attempt * 5))
done
tracker=
if [ ! -d "$BASE_CONFIG" ] || { [ -e "$BASE_CONFIG/.work-item-tracker.json" ] && ! tracker=$(cat "$BASE_CONFIG/.work-item-tracker.json"); }; then
echo "::error::could not read .work-item-tracker.json at $BASE_SHA; re-run this run."
exit 1
fi
human_gated=needs-human
if [ -n "$tracker" ]; then
human_gated=$(jq -er '[objects | .config | objects | .role_labels | objects
Expand Down Expand Up @@ -3533,6 +3564,7 @@ jobs:
PR_NUMBER: ${{ github.event.pull_request.number }}
BASE_SHA: ${{ github.event.pull_request.base.sha }}
HUMAN_GATED: ${{ steps.human_gated.outputs.label }}
BASE_CONFIG: ${{ steps.base_config.outcome == 'success' && format('{0}/base-config', github.workspace) || '' }}
run: |
if [[ ! $BASE_SHA =~ ^[0-9a-f]{40}$ ]]; then
echo "::error::the base commit '$BASE_SHA' is not a commit SHA."
Expand Down Expand Up @@ -3568,13 +3600,10 @@ jobs:
echo "No review lane has held #$PR_NUMBER."
exit 0
fi
for attempt in 1 2 3; do
if actors=$(gh api -H 'Accept: application/vnd.github.raw+json' "repos/$GITHUB_REPOSITORY/contents/.github/standards/trusted-actors/trusted-actors.json?ref=$BASE_SHA"); then
break
fi
[ "$attempt" -lt 3 ] || { echo "::error::could not read the trusted-actor list at $BASE_SHA; re-run this run."; exit 1; }
sleep $((attempt * 5))
done
if [ ! -d "$BASE_CONFIG" ] || ! actors=$(cat "$BASE_CONFIG/.github/standards/trusted-actors/trusted-actors.json"); then
echo "::error::could not read the trusted-actor list at $BASE_SHA; re-run this run."
exit 1
fi
humans=$(jq -er '[.actors[] | select(.kind == "human") | .id | numbers | tostring] | select(length > 0) | join(" ")' <<<"$actors") || {
echo "::error::the trusted-actor list at $BASE_SHA names no person."
exit 1
Expand Down Expand Up @@ -3656,13 +3685,19 @@ jobs:
exit 1
fi
pr=$(read_api "#$PR_NUMBER" "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER") || exit 1
for attempt in 1 2 3; do
if files=$(gh api "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/files?per_page=100" --paginate --jq '.[] | [.filename, (.previous_filename // "")] | @tsv'); then
break
fi
[ "$attempt" -lt 3 ] || { echo "::error::could not read the files of #$PR_NUMBER; re-run this run."; exit 1; }
sleep $((attempt * 5))
done
# The broker step above leaves the file list it read; read it again
# only when that step could not.
if [ -s "$RUNNER_TEMP/pr-files.json" ]; then
files=$(jq -r '.[] | [.filename, (.previous_filename // "")] | @tsv' "$RUNNER_TEMP/pr-files.json")
else
for attempt in 1 2 3; do
if files=$(gh api "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/files?per_page=100" --paginate --jq '.[] | [.filename, (.previous_filename // "")] | @tsv'); then
break
fi
[ "$attempt" -lt 3 ] || { echo "::error::could not read the files of #$PR_NUMBER; re-run this run."; exit 1; }
sleep $((attempt * 5))
done
fi
if [ "$(grep -c . <<<"$files")" -lt "$(jq '.changed_files // 0' <<<"$pr")" ]; then
echo "::error::GitHub listed fewer files than #$PR_NUMBER changes, so its hold-paths cannot be checked; a person merges it."
exit 1
Expand Down Expand Up @@ -3748,6 +3783,7 @@ jobs:
env:
GH_TOKEN: ${{ github.token }}
BASE_SHA: ${{ github.event.merge_group.base_sha }}
BASE_CONFIG: ${{ steps.base_config.outcome == 'success' && format('{0}/base-config', github.workspace) || '' }}
run: |
pr=$(sed -nE 's#^(refs/heads/)?gh-readonly-queue/.+/pr-([0-9]+)-[0-9a-f]{40}$#\2#p' <<<"$GITHUB_REF")
if [ -z "$pr" ]; then
Expand All @@ -3758,16 +3794,11 @@ jobs:
echo "::error::the base commit '$BASE_SHA' is not a commit SHA."
exit 1
fi
for attempt in 1 2 3; do
if tracker=$(gh api -H 'Accept: application/vnd.github.raw+json' "repos/$GITHUB_REPOSITORY/contents/.work-item-tracker.json?ref=$BASE_SHA" 2>"$RUNNER_TEMP/tracker.err"); then
break
elif grep -q 'HTTP 404' "$RUNNER_TEMP/tracker.err"; then
tracker=
break
fi
[ "$attempt" -lt 3 ] || { echo "::error::could not read .work-item-tracker.json at $BASE_SHA; re-run this run."; exit 1; }
sleep $((attempt * 5))
done
tracker=
if [ ! -d "$BASE_CONFIG" ] || { [ -e "$BASE_CONFIG/.work-item-tracker.json" ] && ! tracker=$(cat "$BASE_CONFIG/.work-item-tracker.json"); }; then
echo "::error::could not read .work-item-tracker.json at $BASE_SHA; re-run this run."
exit 1
fi
human_gated=needs-human
if [ -n "$tracker" ]; then
human_gated=$(jq -er '[objects | .config | objects | .role_labels | objects
Expand Down
27 changes: 15 additions & 12 deletions scripts/ci-broker-pin.test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
#!/usr/bin/env bash
# Runs the `Require a broker deploy PR for a resolve-config dependency change` step body of
# .github/workflows/pr-require-checks.yml's ci-status under the shell Actions
# gives a step with no `shell:` (bash -e), against a stub `gh` that applies the
# step's own --jq filter to a pull-request files fixture.
# gives a step with no `shell:` (bash -e), against a stub `gh` that prints a
# pull-request files fixture. The step leaves the pages it read under
# RUNNER_TEMP for the review-lane step.
#
# The lanes token broker in melodic-software/azure-iac loads resolve-config's
# modules from main's tip but imports ajv and yaml from its own deployment, and
Expand Down Expand Up @@ -55,20 +56,15 @@ else
fail "step is not inside ci-status before the aggregate (lines: $order)"
fi

# The stub logs its arguments, fails the first STUB_FAILS calls, then applies
# the --jq argument to STUB_JSON, as gh does to each page.
# The stub logs its arguments, fails the first STUB_FAILS calls, then prints
# STUB_JSON, one page.
mkdir -p "$TMP_ROOT/bin"
cat >"$TMP_ROOT/bin/gh" <<'EOF'
#!/usr/bin/env bash
printf '%s\n' "$*" >>"$STUB_LOG"
calls=$(wc -l <"$STUB_LOG")
[[ "$calls" -gt "${STUB_FAILS:-0}" ]] || exit 1
filter=
while [[ $# -gt 0 ]]; do
[[ "$1" == --jq ]] && { filter="$2"; shift; }
shift
done
jq -r "$filter" <<<"$STUB_JSON"
printf '%s\n' "$STUB_JSON"
EOF
printf '#!/usr/bin/env bash\n' >"$TMP_ROOT/bin/sleep"
chmod +x "$TMP_ROOT/bin/gh" "$TMP_ROOT/bin/sleep"
Expand All @@ -87,15 +83,20 @@ files() {
expect() {
local label="$1" want_rc="$2" want_calls="$3" out rc calls
: >"$TMP_ROOT/log"
rm -f "$TMP_ROOT/pr-files.json"
out="$(PATH="$TMP_ROOT/bin:$PATH" STUB_LOG="$TMP_ROOT/log" STUB_JSON="$4" PR_BODY="$5" STUB_FAILS="$6" \
PR_NUMBER=8313 GITHUB_REPOSITORY=o/r bash -e "$TMP_ROOT/body.sh" 2>&1)" && rc=0 || rc=$?
PR_NUMBER=8313 GITHUB_REPOSITORY=o/r RUNNER_TEMP="$TMP_ROOT" bash -e "$TMP_ROOT/body.sh" 2>&1)" && rc=0 || rc=$?
calls=$(wc -l <"$TMP_ROOT/log")
if [[ "$rc" -ne "$want_rc" ]]; then
fail "$label: expected rc=$want_rc got rc=$rc out='$out'"
elif [[ "$calls" -ne "$want_calls" ]]; then
fail "$label: expected $want_calls gh call(s) got $calls"
elif [[ "$want_calls" -gt 0 && "$(sort -u "$TMP_ROOT/log")" != "api repos/o/r/pulls/8313/files?per_page=100 --paginate --jq "* ]]; then
elif [[ "$want_calls" -gt 0 && "$(sort -u "$TMP_ROOT/log")" != "api repos/o/r/pulls/8313/files?per_page=100 --paginate" ]]; then
fail "$label: unexpected gh call '$(sort -u "$TMP_ROOT/log")'"
elif [[ "$want_calls" -gt "$6" ]] && ! jq -se --argjson want "$(jq -sc . <<<"$4")" '. == $want' "$TMP_ROOT/pr-files.json" >/dev/null 2>&1; then
fail "$label: the pages read were not left for the review-lane step: '$(cat "$TMP_ROOT/pr-files.json" 2>&1)'"
elif [[ "$want_calls" -le "$6" && -e "$TMP_ROOT/pr-files.json" ]]; then
fail "$label: an unread file list left a pr-files.json"
elif [[ -n "${7:-}" && "$out" != *"$7"* ]]; then
fail "$label: output lacks '$7': '$out'"
else
Expand All @@ -117,6 +118,8 @@ expect "files the broker does not load need no line" 0 1 \
expect "a same-named file elsewhere needs no line" 0 1 "$(files "package.json" "$RC/sub/package.json" "$RC/package.json.orig")" "" 0
expect "a rename away from a dependency path fails" 1 1 \
"[{\"filename\":\"$RC/old/package.json\",\"previous_filename\":\"$RC/package.json\",\"status\":\"renamed\"}]" "" 0 "$RC/package.json"
expect "a dependency path on the second page fails" 1 1 "$(files README.md)$(files "$RC/package.json")" "" 0 "$RC/package.json"
expect "a dependency path on the second page passes with the line" 0 1 "$(files README.md)$(files "$RC/package.json")" "$LINE" 0
expect "a CRLF body line counts" 0 1 "$(files "$RC/package.json")" $'Closes #1\r\n'"$LINE"$'\r\n## Summary\r' 0
expect "trailing blanks after the number count" 0 1 "$(files "$RC/package.json")" "$LINE " 0
expect "the line inside other text does not count" 1 1 "$(files "$RC/package.json")" "see $LINE" 0
Expand Down
Loading
Loading