diff --git a/.changes/pr-pipeline/perf-cut-github-token-calls-3f9a1c62.md b/.changes/pr-pipeline/perf-cut-github-token-calls-3f9a1c62.md new file mode 100644 index 0000000000..c5e6ca3df2 --- /dev/null +++ b/.changes/pr-pipeline/perf-cut-github-token-calls-3f9a1c62.md @@ -0,0 +1,10 @@ +--- +bump: patch +--- + +### Changed + +- The rate guard keeps a rate-limit reading of every guarded call under `API_BUDGET_TOKEN`, so every Node script's calls reach the job-end api-budget notice, not only `budgetedFetch`'s. The log-api-budget action adds an `api-calls =N not-modified=M` line: this job's own calls, which the notice's `cost` (a counter every concurrent job shares) is not. The jobs that posted no notice now post one: intake-triage's born-triaged and refine-select, pr-verify's two judge jobs, pr-unblock's four select and context jobs, post-merge-demotion-alert's report-red-main, and the audit-lane-runs, delete-stale-branches, monitor-signals and reconcile-lost-runs jobs. No call is added. +- pr-feedback-dispatch's rerun-ci-status sends each repeat poll of the pr-require-checks run as a conditional GET on the last answer's ETag; a 304 stands for that answer, so the poll keeps its 60 s pace, its 10-minute cap and its decisions. Each 304's reading reaches the job's notice, which shows whether GitHub counts a 304 against `GITHUB_TOKEN`. +- pr-merge's dispatch-full-run sends each read of the full run after the first as a conditional GET on the last ETag; a 304 means the run is still going, so the wait keeps its three-minute reads, its 39-minute ceiling and its red-run dispatch. +- pr-unblock's report-escalation reads a pull request's comments 100 to a page, not GitHub's default 30. diff --git a/.github/actions/log-api-budget/action.yml b/.github/actions/log-api-budget/action.yml index ec3c9136e1..65038733b8 100644 --- a/.github/actions/log-api-budget/action.yml +++ b/.github/actions/log-api-budget/action.yml @@ -4,9 +4,12 @@ description: > named token, from the rate-limit readings the job's own API responses left in $RUNNER_TEMP/api-budget (api-budget.bash): one per refused (403/429) response and one for the last. GET /rate_limit is never read: it reports a - different bucket from the one the job spends. Also writes each line and a - `-cost=N` line to the step summary. A token with no reading gets no - line. Never fails the job. + different bucket from the one the job spends. Then an `api-calls =N + not-modified=M` line: N is this job's own calls, which cost (the rise in a + counter every concurrent job shares) is not, and M its 304 answers, which + GitHub does not count. Also writes each line and a `-cost=N` line to + the step summary. A token with no reading gets no line. Never fails the + job. inputs: tokens: @@ -38,6 +41,10 @@ runs: [[ -n $lines ]] || continue printf '%s\n' "$lines" sed 's/^::notice title=api-budget::/api-budget: /' <<<"$lines" >>"$GITHUB_STEP_SUMMARY" + # This job's own calls, one reading each: unlike cost, no other job's. + calls=$(awk '$1 != 304 { n++ } $1 == 304 { m++ } END { printf "api-calls %s=%d not-modified=%d", token, n, m }' token="$name" "$RUNNER_TEMP/api-budget/$name") + echo "$calls" + echo "$calls" >>"$GITHUB_STEP_SUMMARY" cost=${lines##*cost=} costs+=("$name-cost=${cost/na/unknown}") done diff --git a/.github/actions/log-api-budget/api-budget.mjs b/.github/actions/log-api-budget/api-budget.mjs index 31e40e7ab9..c1fc5a9818 100644 --- a/.github/actions/log-api-budget/api-budget.mjs +++ b/.github/actions/log-api-budget/api-budget.mjs @@ -2,29 +2,7 @@ // $RUNNER_TEMP/api-budget/ in api-budget.bash's line shape, for the // job-end notice the log-api-budget action writes. `token` names the bucket // (`github-token`, `lanes-app`); without it or RUNNER_TEMP nothing is written. -// Never throws. +// Never throws. The rate guard keeps each guarded call's reading with it, so +// it lives in rate-guard.mjs. -import { appendFileSync, mkdirSync } from "node:fs"; -import { join } from "node:path"; - -const VALUE = { limit: /^\d+$/, remaining: /^\d+$/, used: /^\d+$/, reset: /^\d+$/, resource: /^[a-z_]+$/ }; - -export function readingLine(status, headers) { - const values = Object.entries(VALUE).map(([key, re]) => { - const value = String(headers?.get?.(`x-ratelimit-${key}`) ?? ""); - return re.test(value) ? value : "na"; - }); - return `${status} ${values.join(" ")}\n`; -} - -export function recordReading({ token = process.env.API_BUDGET_TOKEN, dir = process.env.RUNNER_TEMP, status, headers }) { - if (!dir || !/^[a-z][a-z0-9-]{0,39}$/.test(token ?? "")) { - return; - } - try { - mkdirSync(join(dir, "api-budget"), { recursive: true }); - appendFileSync(join(dir, "api-budget", token), readingLine(status, headers)); - } catch { - // A reading is never worth failing the job. - } -} +export { readingLine, recordReading } from "./rate-guard.mjs"; diff --git a/.github/actions/log-api-budget/log-api-budget.test.mjs b/.github/actions/log-api-budget/log-api-budget.test.mjs index d2927a06d9..ce03c12e96 100644 --- a/.github/actions/log-api-budget/log-api-budget.test.mjs +++ b/.github/actions/log-api-budget/log-api-budget.test.mjs @@ -155,9 +155,10 @@ test("recordReading appends the same line shape, na for a missing or malformed v assert.equal(existsSync(join(dir, "api-budget", "lanes-app")), false); })); -test("the action emits each named token, writes the summary and the costs output, and reads no /rate_limit", () => +test("the action emits each named token, writes the summary, the call count and the costs output, and reads no /rate_limit", () => withDir((dir) => { - readEach(dir, "github-token", head({ used: 13 }), head({ used: 41 })); + // Two calls GitHub counted, then a conditional GET answered 304. + readEach(dir, "github-token", head({ used: 13 }), head({ used: 41 }), head({ status: 304, used: 41 })); const run = /\n {6}run: \|\n([\s\S]*)$/.exec(ACTION)[1].replace(/^ {8}/gm, ""); writeFileSync(join(dir, "summary"), ""); writeFileSync(join(dir, "output"), ""); @@ -173,10 +174,15 @@ test("the action emits each named token, writes the summary and the costs output TOKENS: "lanes-app\ngithub-token", }, }); - assert.equal(out, `${NOTICE}token=github-token ${WHERE} limit=5000 remaining=4959 used=41 reset=1792748042 resource=core cost=28\n`); + assert.equal( + out, + `${NOTICE}token=github-token ${WHERE} limit=5000 remaining=4959 used=41 reset=1792748042 resource=core cost=28\n` + + "api-calls github-token=2 not-modified=1\n", + ); assert.equal( readFileSync(join(dir, "summary"), "utf8"), - `api-budget: token=github-token ${WHERE} limit=5000 remaining=4959 used=41 reset=1792748042 resource=core cost=28\ngithub-token-cost=28\n`, + `api-budget: token=github-token ${WHERE} limit=5000 remaining=4959 used=41 reset=1792748042 resource=core cost=28\n` + + "api-calls github-token=2 not-modified=1\ngithub-token-cost=28\n", ); assert.equal(readFileSync(join(dir, "output"), "utf8"), "costs=github-token-cost=28\n"); assert.doesNotMatch(ACTION + readFileSync(LIB, "utf8"), /gh api[^\n]*rate_limit/); diff --git a/.github/actions/log-api-budget/rate-guard.mjs b/.github/actions/log-api-budget/rate-guard.mjs index 704243ed96..d6538c42e0 100644 --- a/.github/actions/log-api-budget/rate-guard.mjs +++ b/.github/actions/log-api-budget/rate-guard.mjs @@ -89,6 +89,35 @@ export function poolOf(url) { export const entryOf = (pool, token = process.env.API_BUDGET_TOKEN) => pool === "graphql" ? "graphql" : API_POOLS.includes(token) ? token : "github-token"; +// One rate-limit reading, from a response's x-ratelimit-* headers, in +// api-budget.bash's line shape, appended to $RUNNER_TEMP/api-budget/ +// for the job-end notice the log-api-budget action writes. `token` names the +// bucket (`github-token`, `lanes-app`); without it or RUNNER_TEMP nothing is +// written. It lives here, not in api-budget.mjs (which re-exports it), so the +// guard below keeps every call's reading without loading another module. +// Never throws. +const READING = { limit: /^\d+$/, remaining: /^\d+$/, used: /^\d+$/, reset: /^\d+$/, resource: /^[a-z_]+$/ }; + +export function readingLine(status, headers) { + const values = Object.entries(READING).map(([key, re]) => { + const value = String(headers?.get?.(`x-ratelimit-${key}`) ?? ""); + return re.test(value) ? value : "na"; + }); + return `${status} ${values.join(" ")}\n`; +} + +export function recordReading({ token = process.env.API_BUDGET_TOKEN, dir = process.env.RUNNER_TEMP, status, headers }) { + if (!dir || !/^[a-z][a-z0-9-]{0,39}$/.test(token ?? "")) { + return; + } + try { + mkdirSync(join(dir, "api-budget"), { recursive: true }); + appendFileSync(join(dir, "api-budget", token), readingLine(status, headers)); + } catch { + // A reading is never worth failing the job. + } +} + const intHeader = (get, name) => { const value = String(get(name) ?? ""); return /^\d{1,12}$/.test(value) ? Number(value) : null; @@ -286,11 +315,15 @@ export function createRateLimitLatch({ check, observe, belowFloor: (url) => low.get(poolOf(url)) === true, - // A fetch that checks the latch before each request and reads each answer. + // A fetch that checks the latch before each request and reads each answer, + // keeping its reading under API_BUDGET_TOKEN (recordReading). That one + // name covers every guarded call in the process, so a step sets it only + // when all its calls spend one token. guard: (fetchImpl) => async function guarded(url, init = {}) { check(url, init.method); const response = await fetchImpl(url, init); + recordReading({ status: response.status, headers: response.headers }); await observe(url, response, init.method); return response; }, diff --git a/.github/actions/log-api-budget/rate-guard.test.mjs b/.github/actions/log-api-budget/rate-guard.test.mjs index 627ec02b38..51e434296c 100644 --- a/.github/actions/log-api-budget/rate-guard.test.mjs +++ b/.github/actions/log-api-budget/rate-guard.test.mjs @@ -1,6 +1,6 @@ import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; -import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { chmodSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { test } from "node:test"; @@ -332,6 +332,33 @@ for (const vector of VECTORS.stop_file_errors) { })); } +// Every guarded answer leaves one reading for the job-end notice, under the +// bucket API_BUDGET_TOKEN names; without that name it leaves none. +test("the guard keeps a reading of each answer under API_BUDGET_TOKEN, and none without it", async () => + withDir(async (dir) => { + const saved = { token: process.env.API_BUDGET_TOKEN, temp: process.env.RUNNER_TEMP }; + process.env.RUNNER_TEMP = dir; + const ok = [200, { "x-ratelimit-limit": "5000", "x-ratelimit-remaining": "4990", "x-ratelimit-used": "10", "x-ratelimit-reset": "1792751600", "x-ratelimit-resource": "core" }]; + const options = { now: () => NOW, log: () => {}, fail: () => {}, limits: () => ({}), stopFile: null }; + try { + delete process.env.API_BUDGET_TOKEN; + await createRateLimitLatch(options).guard(fakeGitHub([ok]).fetch)(URL_A); + assert.equal(readdirSync(dir).length, 0); + process.env.API_BUDGET_TOKEN = "github-token"; + const gh = fakeGitHub([ok, [404, { ...ok[1], "x-ratelimit-remaining": "4989", "x-ratelimit-used": "11" }], [500]]); + const guarded = createRateLimitLatch(options).guard(gh.fetch); + for (const url of [URL_A, URL_A, URL_B]) { + await guarded(url); + } + assert.equal(readFileSync(join(dir, "api-budget", "github-token"), "utf8"), "200 5000 4990 10 1792751600 core\n404 5000 4989 11 1792751600 core\n500 na na na na na\n"); + } finally { + for (const [key, value] of [["API_BUDGET_TOKEN", saved.token], ["RUNNER_TEMP", saved.temp]]) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + } + })); + // No lane script calls fetch outside the guard, except the files below, each // for its stated reason. A new raw fetch fails here until it is guarded or // listed. diff --git a/.github/scripts/post-merge-demotion-alert/workflow.test.mjs b/.github/scripts/post-merge-demotion-alert/workflow.test.mjs index 5a99ffcc13..eb3a52c067 100644 --- a/.github/scripts/post-merge-demotion-alert/workflow.test.mjs +++ b/.github/scripts/post-merge-demotion-alert/workflow.test.mjs @@ -95,8 +95,11 @@ test("main's tip is checked out without credentials, and every action is pinned" }); test("the run step takes every event field through env", () => { - const run = /\n {8}run: (.*)\n?$/.exec(job)[1]; - assert.doesNotMatch(run, /\$\{\{/); + const runs = [...job.matchAll(/\n {8}run: (.*)/g)].map((match) => match[1]); + assert.ok(runs.includes("node .base/.github/scripts/post-merge-demotion-alert/report.mjs"), runs.join("\n")); + for (const run of runs) { + assert.doesNotMatch(run, /\$\{\{/); + } }); // Records are one per signal and SHA, found by a read before a create, so diff --git a/.github/scripts/pr-merge/workflow.test.mjs b/.github/scripts/pr-merge/workflow.test.mjs index 0e2e5da2d3..63cb068aec 100644 --- a/.github/scripts/pr-merge/workflow.test.mjs +++ b/.github/scripts/pr-merge/workflow.test.mjs @@ -58,7 +58,10 @@ test("a push job holds the group until the dispatched run finishes", () => { // it raises no workflow_run event. const HEAD = "0123456789abcdef0123456789abcdef01234567"; const SEND = `api -X POST repos/o/r/actions/workflows/post-merge-demotion-alert.yml/dispatches -f ref=main -f inputs[run-id]=4242 -f inputs[head-sha]=${HEAD}`; -function runWait(conclusion, { dispatchFails = false } = {}) { +// With notModified N, the first read finds the run in progress with ETag +// "e1", the next N reads (sent with If-None-Match on it) are answered 304 +// with gh's exit 1, and the one after finds it completed. +function runWait(conclusion, { dispatchFails = false, notModified = 0 } = {}) { const step = job("dispatch-full-run").split("\n - name: Wait for the full run to finish\n")[1]; const body = /\n {8}run: \|\n((?: {10}.*\n|\s*\n)+)/ .exec(step)[1] @@ -67,16 +70,28 @@ function runWait(conclusion, { dispatchFails = false } = {}) { .join("\n"); const dir = mkdtempSync(join(tmpdir(), "pr-merge-wait-")); mkdirSync(join(dir, "bin")); - writeFileSync(join(dir, "bin", "sleep"), "#!/bin/sh\nexit 0\n", { + writeFileSync(join(dir, "bin", "sleep"), `#!/bin/sh\necho "$*" >>"${dir}/sleeps"\n`, { mode: 0o755, }); + writeFileSync(join(dir, "n304"), String(notModified)); + const done = `{"status": "completed", "conclusion": "${conclusion}", "head_sha": "${HEAD}"}`; + const first = notModified > 0 ? `{"status": "in_progress", "conclusion": null, "head_sha": "${HEAD}"}` : done; writeFileSync( join(dir, "bin", "gh"), `#!/usr/bin/env bash printf '%s\\n' "$*" >>"${dir}/log" case "$*" in *-X\\ POST*) exit ${dispatchFails ? 1 : 0} ;; - *) jq -r "\${@: -1}" <<<'{"status": "completed", "conclusion": "${conclusion}", "head_sha": "${HEAD}"}' ;; + *'-H If-None-Match: "e1"') + n=$(cat "${dir}/n304") + if [ "$n" -gt 0 ]; then + echo $((n - 1)) >"${dir}/n304" + printf 'HTTP/2.0 304 Not Modified\\r\\nEtag: "e1"\\r\\n\\r\\n' + echo 'gh: HTTP 304' >&2 + exit 1 + fi + printf 'HTTP/2.0 200 OK\\r\\nEtag: "e2"\\r\\n\\r\\n%s\\n' '${done}' ;; + *) printf 'HTTP/2.0 200 OK\\r\\nEtag: "e1"\\r\\n\\r\\n%s\\n' '${first}' ;; esac `, { mode: 0o755 }, @@ -93,14 +108,32 @@ esac encoding: "utf8", }); const log = existsSync(join(dir, "log")) ? readFileSync(join(dir, "log"), "utf8") : ""; + const sleeps = existsSync(join(dir, "sleeps")) ? readFileSync(join(dir, "sleeps"), "utf8").split("\n").filter(Boolean) : []; rmSync(dir, { recursive: true, force: true }); return { status: result.status, out: result.stdout + result.stderr, writes: log.split("\n").filter((l) => l.includes("-X POST")), + reads: log.split("\n").filter((l) => l.startsWith("api -i ")), + sleeps, }; } +// A 304 is the run as last read: the wait keeps its three-minute pace and +// sends the red run once it completes, as with plain reads. +test("a poll answered 304 waits on, then sends the red run once it completes", () => { + const { status, writes, reads, sleeps } = runWait("failure", { notModified: 2 }); + assert.equal(status, 0); + assert.deepEqual(writes, [SEND]); + assert.deepEqual(sleeps, ["180", "180", "180", "180"]); + assert.deepEqual(reads, [ + "api -i repos/o/r/actions/runs/4242", + 'api -i repos/o/r/actions/runs/4242 -H If-None-Match: "e1"', + 'api -i repos/o/r/actions/runs/4242 -H If-None-Match: "e1"', + 'api -i repos/o/r/actions/runs/4242 -H If-None-Match: "e1"', + ]); +}); + // The alert serializes on signal and head SHA, so the send names the run's // head SHA as the run reports it, not this workflow's push SHA. test("a red full run is sent to post-merge-demotion-alert.yml by its run id and head SHA", () => { diff --git a/.github/scripts/pr-unblock/report-escalation.sh b/.github/scripts/pr-unblock/report-escalation.sh index 5e50cc40b5..c7f509f3d1 100755 --- a/.github/scripts/pr-unblock/report-escalation.sh +++ b/.github/scripts/pr-unblock/report-escalation.sh @@ -145,7 +145,7 @@ key_line="" labeled=$(gh api "$issue" | jq --arg l "$label" '[.labels[].name] | index($l) != null') if [[ $labeled == true ]]; then - marked=$(gh api --paginate "$issue/comments" | jq -s --arg bot "$bot" --arg m "$marker" --arg k "$key_line" \ + marked=$(gh api --paginate "$issue/comments?per_page=100" | jq -s --arg bot "$bot" --arg m "$marker" --arg k "$key_line" \ --arg r "- Reason: \`$ESCALATION_REASON\`" \ '[add[] | select(.user.login == $bot) | (.body // "") | split("\n") | map(rtrimstr("\r")) | select(.[0] == $m and (.[1] // "") == $k and any(.[]; . == $r))] | length > 0') diff --git a/.github/scripts/pr-unblock/report-escalation.test.sh b/.github/scripts/pr-unblock/report-escalation.test.sh index 5612f3cd00..ca188e0bdb 100755 --- a/.github/scripts/pr-unblock/report-escalation.test.sh +++ b/.github/scripts/pr-unblock/report-escalation.test.sh @@ -47,7 +47,7 @@ done printf '%s %s\n' "$method" "$endpoint" >>"$STUB/requests" case "$method $endpoint" in "GET repos/o/r/issues/42") cat "$STUB/issue.json" ;; -"GET repos/o/r/issues/42/comments") cat "$STUB/comments.json" ;; +"GET repos/o/r/issues/42/comments?per_page=100") cat "$STUB/comments.json" ;; "GET graphql") [[ -f $STUB/fail_graphql ]] && { echo 'stub gh: graphql failed' >&2; exit 1; } # threads.json holds an array of pages; each graphql call serves the next. diff --git a/.github/scripts/pr-unblock/request-budget.mjs b/.github/scripts/pr-unblock/request-budget.mjs index bacc1e22f5..1afbce581b 100644 --- a/.github/scripts/pr-unblock/request-budget.mjs +++ b/.github/scripts/pr-unblock/request-budget.mjs @@ -1,9 +1,9 @@ // A fetch wrapper the dispatchers' GitHub adapters share. It counts the calls // a run makes, keeps the x-ratelimit-* headers of the last response per // resource (the headers are the authoritative reading, per -// https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api) -// and appends each one for the job-end api-budget notice (api-budget.mjs), -// logs the message and retry-after of a 403 or 429 so a secondary limit can be +// https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api; +// the rate guard under retryServerErrors appends each one for the job-end +// api-budget notice), logs the message and retry-after of a 403 or 429 so a secondary limit can be // told from a primary one, and repeats a GET of a URL it already read as a // conditional request: a 304 "does not count against your primary rate limit" // when the request is authorized @@ -28,7 +28,6 @@ // the above. import { retryServerErrors } from "../../actions/check-trusted-trigger/check-trusted-trigger.mjs"; -import { recordReading } from "../../actions/log-api-budget/api-budget.mjs"; import { rateLimitLatch, resetLabel, SIGNALS } from "../../actions/log-api-budget/rate-guard.mjs"; export function budgetedFetch({ @@ -89,7 +88,6 @@ export function budgetedFetch({ const response = await fetch(url, { ...init, headers }); const header = (name) => response.headers?.get?.(name) ?? null; const resource = header("x-ratelimit-resource"); - recordReading({ status: response.status, headers: response.headers }); if (resource) { last.set(resource, ["limit", "remaining", "used", "reset"].map((key) => header(`x-ratelimit-${key}`))); } diff --git a/.github/workflows/intake-triage.yml b/.github/workflows/intake-triage.yml index a17a76f710..58792d21e9 100644 --- a/.github/workflows/intake-triage.yml +++ b/.github/workflows/intake-triage.yml @@ -70,6 +70,10 @@ jobs: # Empty when the profile sets a subagent model, which the reusable # cannot apply; the refusal step below shows why. claude-args: ${{ steps.models.outputs.settings-json == '' && steps.models.outputs.claude-args || '' }} + # Each guarded API call's rate-limit reading, for the job-end api-budget + # notice (.github/actions/log-api-budget). + env: + API_BUDGET_TOKEN: github-token steps: - name: Record when this run started id: started @@ -140,6 +144,14 @@ jobs: fi echo "ids=$ids" >> "$GITHUB_OUTPUT" + - name: Log the API budget at the end + if: always() + continue-on-error: true + timeout-minutes: 1 + uses: ./.base/.github/actions/log-api-budget # zizmor: ignore[self-repository] + with: + tokens: github-token + # The pinned reusable reads no kill switch, so this caller does. Intake has # no lane switch of its own; the org and repository switches fail closed # (check-kill-switch), and a failed job leaves proceed empty, which also @@ -490,6 +502,10 @@ jobs: # `size-split` for a Size Large issue to split into sub-issues, else `refine`. mode: ${{ steps.select.outputs.mode }} context-sha256: ${{ steps.hash.outputs.sha256 }} + # Each guarded API call's rate-limit reading, for the job-end api-budget + # notice (.github/actions/log-api-budget). + env: + API_BUDGET_TOKEN: github-token steps: - name: Check out the default branch tip uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -559,6 +575,14 @@ jobs: retention-days: 1 if-no-files-found: error + - name: Log the API budget at the end + if: always() + continue-on-error: true + timeout-minutes: 1 + uses: ./.base/.github/actions/log-api-budget # zizmor: ignore[self-repository] + with: + tokens: github-token + # The model. Its job's only token is GITHUB_TOKEN with contents: read; it # reads the context file and the default branch checkout (its working # directory) and fetches only the hosts below, and loads no project diff --git a/.github/workflows/maintenance-audit-lane-runs.yml b/.github/workflows/maintenance-audit-lane-runs.yml index 654b0f7d0a..02bfdd4734 100644 --- a/.github/workflows/maintenance-audit-lane-runs.yml +++ b/.github/workflows/maintenance-audit-lane-runs.yml @@ -58,6 +58,10 @@ jobs: pull-requests: read # list open pull requests outputs: units: ${{ steps.collect.outputs.units }} + # Each guarded API call's rate-limit reading, for the job-end api-budget + # notice (.github/actions/log-api-budget). + env: + API_BUDGET_TOKEN: github-token steps: - name: Check out the run's main commit uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -107,6 +111,14 @@ jobs: if-no-files-found: error retention-days: 7 + - name: Log the API budget at the end + if: always() + continue-on-error: true + timeout-minutes: 1 + uses: ./.base/.github/actions/log-api-budget # zizmor: ignore[self-repository] + with: + tokens: github-token + judge: needs: collect if: needs.collect.outputs.units != '' && needs.collect.outputs.units != '[]' @@ -264,6 +276,8 @@ jobs: actions: read # download the findings artifacts contents: read # check out main for the claim check issues: write # file, rewrite and comment on the digest issue + env: + API_BUDGET_TOKEN: github-token steps: - name: Check out the run's main commit uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -315,6 +329,14 @@ jobs: mkdir -p "$FINDINGS_DIR" node .base/.github/scripts/maintenance-audit-lane-runs/file-findings.mjs + - name: Log the API budget at the end + if: always() + continue-on-error: true + timeout-minutes: 1 + uses: ./.base/.github/actions/log-api-budget # zizmor: ignore[self-repository] + with: + tokens: github-token + # A dispatch sent with this token starts a run as github-actions[bot] # (41898282), which intake-triage.yml admits; intake itself uses its own # GITHUB_TOKEN and the Claude OAuth secret, no App token. At most 120 REST @@ -333,6 +355,8 @@ jobs: actions: write # dispatch intake-triage.yml and read its runs contents: read # check out main issues: write # write the state comment and file the needs-triage issue + env: + API_BUDGET_TOKEN: github-token steps: - name: Check out the run's main commit uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -363,3 +387,11 @@ jobs: LANES_APP_ID: ${{ vars.AUTOMATION_LANES_APP_SENDER_ID }} CALL_BUDGET: "120" run: node .base/.github/scripts/maintenance-audit-lane-runs/triage-sweep.mjs + + - name: Log the API budget at the end + if: always() + continue-on-error: true + timeout-minutes: 1 + uses: ./.base/.github/actions/log-api-budget # zizmor: ignore[self-repository] + with: + tokens: github-token diff --git a/.github/workflows/maintenance-delete-stale-branches.yml b/.github/workflows/maintenance-delete-stale-branches.yml index a2a2930fc6..e0b2a04c88 100644 --- a/.github/workflows/maintenance-delete-stale-branches.yml +++ b/.github/workflows/maintenance-delete-stale-branches.yml @@ -48,6 +48,10 @@ jobs: contents: write # list branches, compare, delete a branch ref issues: read # read linked issues pull-requests: read # read each branch's pull requests + # Each guarded API call's rate-limit reading, for the job-end api-budget + # notice (.github/actions/log-api-budget). + env: + API_BUDGET_TOKEN: github-token steps: - name: Check out the run's main commit uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -108,3 +112,11 @@ jobs: path: ${{ runner.temp }}/branch-cleanup-ledger.jsonl if-no-files-found: ignore retention-days: 90 + + - name: Log the API budget at the end + if: always() + continue-on-error: true + timeout-minutes: 1 + uses: ./.base/.github/actions/log-api-budget # zizmor: ignore[self-repository] + with: + tokens: github-token diff --git a/.github/workflows/maintenance-monitor-signals.yml b/.github/workflows/maintenance-monitor-signals.yml index 91e33957d5..6cc47eada5 100644 --- a/.github/workflows/maintenance-monitor-signals.yml +++ b/.github/workflows/maintenance-monitor-signals.yml @@ -45,6 +45,10 @@ jobs: contents: read # check out main issues: write # file and update the lane's own issues pull-requests: read # list merged pull requests + # Each guarded API call's rate-limit reading, for the job-end api-budget + # notice (.github/actions/log-api-budget). + env: + API_BUDGET_TOKEN: github-token steps: - name: Check out the run's main commit uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -83,3 +87,11 @@ jobs: AGENT_IDENTITY=$(yq '.merge."agent-identity" // ""' .base/docs/conventions/pr-pipeline.yaml) export AGENT_IDENTITY node .base/.github/scripts/maintenance-monitor-signals/monitor.mjs + + - name: Log the API budget at the end + if: always() + continue-on-error: true + timeout-minutes: 1 + uses: ./.base/.github/actions/log-api-budget # zizmor: ignore[self-repository] + with: + tokens: github-token diff --git a/.github/workflows/maintenance-reconcile-lost-runs.yml b/.github/workflows/maintenance-reconcile-lost-runs.yml index fb9ab5d99b..ef8458802a 100644 --- a/.github/workflows/maintenance-reconcile-lost-runs.yml +++ b/.github/workflows/maintenance-reconcile-lost-runs.yml @@ -83,6 +83,10 @@ jobs: checks: read # a cut job's annotations: whether it lost its runner contents: read # check out main pull-requests: read # list the open PRs + # Each guarded API call's rate-limit reading, for the job-end api-budget + # notice (.github/actions/log-api-budget). + env: + API_BUDGET_TOKEN: github-token steps: - name: Check out the run's main commit uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -160,3 +164,11 @@ jobs: GH_TOKEN: ${{ github.token }} GITHUB_REPOSITORY: ${{ github.repository }} run: node .base/.github/scripts/maintenance-reconcile-lost-runs/reconcile.mjs + + - name: Log the API budget at the end + if: always() + continue-on-error: true + timeout-minutes: 1 + uses: ./.base/.github/actions/log-api-budget # zizmor: ignore[self-repository] + with: + tokens: github-token diff --git a/.github/workflows/post-merge-demotion-alert.yml b/.github/workflows/post-merge-demotion-alert.yml index 023c44b0c1..aa9157507d 100644 --- a/.github/workflows/post-merge-demotion-alert.yml +++ b/.github/workflows/post-merge-demotion-alert.yml @@ -897,6 +897,8 @@ jobs: if: steps.kill.outputs.proceed == 'true' env: GITHUB_TOKEN: ${{ github.token }} + # Each call's rate-limit reading, for the api-budget notice below. + API_BUDGET_TOKEN: github-token REPOSITORY: ${{ github.repository }} BASE_PATH: .base CALL_BUDGET: "300" @@ -906,3 +908,11 @@ jobs: RUN_WORKFLOW_ID: ${{ needs.alert.outputs.workflow-id }} RUN_CREATED: ${{ needs.alert.outputs.created }} run: node .base/.github/scripts/post-merge-demotion-alert/report.mjs + + - name: Log the API budget at the end + if: always() + continue-on-error: true + timeout-minutes: 1 + uses: ./.base/.github/actions/log-api-budget # zizmor: ignore[self-repository] + with: + tokens: github-token diff --git a/.github/workflows/pr-feedback-dispatch.yml b/.github/workflows/pr-feedback-dispatch.yml index 6c8f886ff9..5ea03f1c3a 100644 --- a/.github/workflows/pr-feedback-dispatch.yml +++ b/.github/workflows/pr-feedback-dispatch.yml @@ -418,18 +418,30 @@ jobs: # completion exists only while the other lane is running, and # maintenance-reconcile-lost-runs leaves a run where only ci-status # failed alone). Any other failure is read three times, then fails. - waited=0 + # A path read again (the poll below) is sent as a conditional GET + # with the ETag of its last 200; a 304 (gh exits 1 on it) gives that + # 200's body back, as request-budget.mjs does. Its reading is kept, + # so the notice shows whether GitHub counted it. + waited=0 etag_path='' etag='' etag_body='' read_api() { - local attempt out hdr status left reset after wait + local attempt out hdr status left reset after wait cond for attempt in 1 2 3; do - if out=$(gh api -i "$3"); then + cond=() + [[ $3 != "$etag_path" || -z $etag ]] || cond=(-H "If-None-Match: $etag") + if out=$(gh api -i "$3" "${cond[@]}"); then budget "$out" printf -v "$1" '%s' "$(sed '1,/^\r\?$/d' <<<"$out")" + etag_path=$3 etag_body=${!1} + etag=$(sed '/^\r\?$/q' <<<"$out" | tr -d '\r' | awk -F': *' 'tolower($1) == "etag" { print $2; exit }') return 0 fi budget "$out" hdr=$(sed '/^\r\?$/q' <<<"$out") status=$(awk 'NR == 1 { print $2 }' <<<"$hdr") + if [[ $status == 304 && ${#cond[@]} -gt 0 ]]; then + printf -v "$1" '%s' "$etag_body" + return 0 + fi read -r left reset after < <(tr -d '\r' <<<"$hdr" | awk -F': *' '{ h[tolower($1)] = $2 } END { print h["x-ratelimit-remaining"] "-", h["x-ratelimit-reset"] "-", h["retry-after"] "-" }') left=${left%-} reset=${reset%-} after=${after%-} diff --git a/.github/workflows/pr-merge.yml b/.github/workflows/pr-merge.yml index 98d0dfc886..1569957bdc 100644 --- a/.github/workflows/pr-merge.yml +++ b/.github/workflows/pr-merge.yml @@ -1823,9 +1823,16 @@ jobs: RUN_ID: ${{ steps.dispatch.outputs.run-id }} run: | set -euo pipefail + # Each read after the first is conditional on the last ETag: a 304 + # (gh exits 1) means the run is as last read, still going. + cond=() for _ in $(seq 13); do sleep 180 - state=$(gh api "repos/$REPO/actions/runs/$RUN_ID" --jq '.status + " " + .head_sha + " " + (.conclusion // "")') || continue + out=$(gh api -i "repos/$REPO/actions/runs/$RUN_ID" "${cond[@]}") || continue + etag=$(sed '/^\r\?$/q' <<<"$out" | tr -d '\r' | awk -F': *' 'tolower($1) == "etag" { print $2; exit }') + cond=() + [ -z "$etag" ] || cond=(-H "If-None-Match: $etag") + state=$(sed '1,/^\r\?$/d' <<<"$out" | jq -r '.status + " " + .head_sha + " " + (.conclusion // "")') || continue read -r status head_sha conclusion <<<"$state" if [ "$status" = completed ]; then echo "run $RUN_ID completed: $conclusion" diff --git a/.github/workflows/pr-unblock.yml b/.github/workflows/pr-unblock.yml index 5a49c2ac3c..9b444820df 100644 --- a/.github/workflows/pr-unblock.yml +++ b/.github/workflows/pr-unblock.yml @@ -97,6 +97,10 @@ jobs: # true when another lane's escalation put the human-gated label on, so # apply-conflict-patch pushes past it (lane-hold.mjs). lane-escalated: ${{ steps.hold.outputs.lane-escalated }} + # Each guarded API call's rate-limit reading, for the job-end api-budget + # notice (.github/actions/log-api-budget). + env: + API_BUDGET_TOKEN: github-token steps: - name: Check out the default branch tip uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -236,6 +240,14 @@ jobs: BASE_SHA: ${{ steps.base-sha.outputs.sha }} run: bash "$GITHUB_WORKSPACE/.base/.github/scripts/merge-base/run-prepare.sh" + - name: Log the API budget at the end + if: always() + continue-on-error: true + timeout-minutes: 1 + uses: ./.base/.github/actions/log-api-budget # zizmor: ignore[self-repository] + with: + tokens: github-token + # A select that skipped, merged cleanly, or handed the PR to a person runs # no model job, so no resolve-conflicts check would land on the head and # dispatch.mjs would dispatch it again on every run. This job posts it @@ -527,6 +539,10 @@ jobs: # The artifact name's attempt, so a re-run of a later job alone still # finds it. context-attempt: ${{ steps.upload.outcome == 'success' && github.run_attempt || '' }} + # Each guarded API call's rate-limit reading, for the job-end api-budget + # notice (.github/actions/log-api-budget). + env: + API_BUDGET_TOKEN: github-token steps: - name: Check out the default branch tip uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -619,6 +635,14 @@ jobs: retention-days: 1 if-no-files-found: error + - name: Log the API budget at the end + if: always() + continue-on-error: true + timeout-minutes: 1 + uses: ./.base/.github/actions/log-api-budget # zizmor: ignore[self-repository] + with: + tokens: github-token + # A select that kept nothing, or handed the PR to a person, runs no model # or reply job, so neither check would land on the head and # feedback-dispatch.mjs would dispatch it again. This job posts both @@ -1254,6 +1278,10 @@ jobs: # The artifact name's attempt, so a re-run of a later job alone still # finds it. context-attempt: ${{ steps.upload.outcome == 'success' && github.run_attempt || '' }} + # Each guarded API call's rate-limit reading, for the job-end api-budget + # notice (.github/actions/log-api-budget). + env: + API_BUDGET_TOKEN: github-token steps: - name: Check out the default branch tip uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -1349,6 +1377,14 @@ jobs: retention-days: 1 if-no-files-found: error + - name: Log the API budget at the end + if: always() + continue-on-error: true + timeout-minutes: 1 + uses: ./.base/.github/actions/log-api-budget # zizmor: ignore[self-repository] + with: + tokens: github-token + # A select that skipped, or handed the PR to a person, runs no model job, # so no review-dependency check would land on the head and # dependency-dispatch.mjs would dispatch it again on every run. This job @@ -1611,6 +1647,10 @@ jobs: # The artifact name's attempt, so a re-run of the model job alone still # finds it. context-attempt: ${{ steps.upload.outcome == 'success' && github.run_attempt || '' }} + # Each guarded API call's rate-limit reading, for the job-end api-budget + # notice (.github/actions/log-api-budget). + env: + API_BUDGET_TOKEN: github-token steps: - name: Check out the default branch tip uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -1661,6 +1701,14 @@ jobs: retention-days: 1 if-no-files-found: error + - name: Log the API budget at the end + if: always() + continue-on-error: true + timeout-minutes: 1 + uses: ./.base/.github/actions/log-api-budget # zizmor: ignore[self-repository] + with: + tokens: github-token + # A fix-ci-context skip (a closed PR, a human-gated hold, or no failing # check) runs no fix-ci job, # so no fix-ci check would land on the head and fix-ci-dispatch.mjs would diff --git a/.github/workflows/pr-verify.yml b/.github/workflows/pr-verify.yml index edda0bce87..9b8319e204 100644 --- a/.github/workflows/pr-verify.yml +++ b/.github/workflows/pr-verify.yml @@ -113,6 +113,10 @@ jobs: - name: Check the trigger id: gate if: steps.kill.outputs.proceed == 'true' + # Each API call keeps its rate-limit reading for the api-budget + # notice the job logs before its last step. + env: + API_BUDGET_TOKEN: github-token uses: ./.base/.github/actions/check-trusted-trigger # zizmor: ignore[self-repository] with: trusted-actors-path: .base/.github/standards/trusted-actors/trusted-actors.json @@ -125,6 +129,7 @@ jobs: if: steps.gate.outputs.proceed == 'true' env: GITHUB_TOKEN: ${{ github.token }} + API_BUDGET_TOKEN: github-token UPSTREAM_RUN_ID: ${{ github.event.workflow_run.id }} SPEC_FILE: ${{ github.workspace }}/pr-verify-data/spec.json SPEC_TEXT_FILE: ${{ github.workspace }}/pr-verify-data/spec-text.json @@ -155,6 +160,8 @@ jobs: - name: Select the trusted PR text if: steps.agree.outcome == 'success' + env: + API_BUDGET_TOKEN: github-token uses: ./.base/.github/actions/select-trusted-text # zizmor: ignore[self-repository] with: pr-number: ${{ steps.pr.outputs.pr-number }} @@ -374,6 +381,14 @@ jobs: echo '```' } >>"$GITHUB_STEP_SUMMARY" + - name: Log the API budget + if: always() + continue-on-error: true + timeout-minutes: 1 + uses: ./.base/.github/actions/log-api-budget # zizmor: ignore[self-repository] + with: + tokens: github-token + # A clean handoff or an empty key hands the judgment on; any other # run that passed the gates and left no verdict fails here. - name: Fail without a verdict @@ -442,6 +457,10 @@ jobs: - name: Check the trigger id: gate if: steps.kill.outputs.proceed == 'true' + # Each API call keeps its rate-limit reading for the api-budget + # notice the job logs before its last step. + env: + API_BUDGET_TOKEN: github-token uses: ./.base/.github/actions/check-trusted-trigger # zizmor: ignore[self-repository] with: trusted-actors-path: .base/.github/standards/trusted-actors/trusted-actors.json @@ -454,6 +473,7 @@ jobs: if: steps.gate.outputs.proceed == 'true' env: GITHUB_TOKEN: ${{ github.token }} + API_BUDGET_TOKEN: github-token UPSTREAM_RUN_ID: ${{ github.event.workflow_run.id }} SPEC_FILE: ${{ github.workspace }}/pr-verify-data/spec.json SPEC_TEXT_FILE: ${{ github.workspace }}/pr-verify-data/spec-text.json @@ -484,6 +504,8 @@ jobs: - name: Select the trusted PR text if: steps.agree.outcome == 'success' + env: + API_BUDGET_TOKEN: github-token uses: ./.base/.github/actions/select-trusted-text # zizmor: ignore[self-repository] with: pr-number: ${{ steps.pr.outputs.pr-number }} @@ -679,6 +701,14 @@ jobs: echo '```' } >>"$GITHUB_STEP_SUMMARY" + - name: Log the API budget + if: always() + continue-on-error: true + timeout-minutes: 1 + uses: ./.base/.github/actions/log-api-budget # zizmor: ignore[self-repository] + with: + tokens: github-token + - name: Fail without a verdict if: always() && steps.agree.outcome == 'success' && steps.verdict.outcome != 'success' run: | diff --git a/scripts/ci-rerun-ci-status.test.sh b/scripts/ci-rerun-ci-status.test.sh index c1aebce650..43d1e7c3ad 100755 --- a/scripts/ci-rerun-ci-status.test.sh +++ b/scripts/ci-rerun-ci-status.test.sh @@ -127,6 +127,21 @@ if [[ -n "${STUB_LIMIT_FILE:-}" && "$*" == *"/actions/workflows/pr-require-check fi exit 1 fi +# With STUB_ETAG set, a read of one run carries that ETag; a read sent with +# If-None-Match on it is answered 304 (headers only, exit 1, as gh does) +# while STUB_NOT_MODIFIED_FILE holds a count above 0, which drops by one, and +# otherwise with the last line of STUB_RUN_SEQ. +if [[ -n "${STUB_ETAG:-}" && "$*" =~ ^api\ -i\ repos/o/r/actions/runs/[0-9]+($|\ -H) ]]; then + if [[ "$*" == *" -H If-None-Match: $STUB_ETAG" && "$(cat "$STUB_NOT_MODIFIED_FILE")" -gt 0 ]]; then + echo $(($(cat "$STUB_NOT_MODIFIED_FILE") - 1)) >"$STUB_NOT_MODIFIED_FILE" + printf 'HTTP/2.0 304 Not Modified\r\nEtag: %s\r\nX-Ratelimit-Remaining: 9\r\n\r\n' "$STUB_ETAG" + echo "gh: HTTP 304" >&2 + exit 1 + fi + if [[ "$*" == *" -H If-None-Match: "* ]]; then body=$(tail -n 1 <<<"$STUB_RUN_SEQ"); else body=$(head -n 1 <<<"$STUB_RUN_SEQ"); fi + printf 'HTTP/2.0 200 OK\r\nEtag: %s\r\nX-Ratelimit-Remaining: 9\r\n\r\n%s\n' "$STUB_ETAG" "$body" + exit 0 +fi case "$*" in */pulls/*) ok200 "$STUB_PR" ;; */actions/workflows/pr-review-hosted.yml/runs* | */actions/workflows/pr-review-security-hosted.yml/runs*) @@ -315,6 +330,19 @@ check_reran 1 "the polled run's ci-status is re-run once" if [[ "$(grep -c '^sleep 60$' "$TMP_ROOT/log")" == 2 ]]; then ok "it polled every 60 s"; else fail "polls: $(grep '^sleep' "$TMP_ROOT/log" | paste -sd'|')"; fi check_calls 6 "each poll reads the run only" if [[ "$(grep -c '/jobs?' "$TMP_ROOT/log")" == 1 ]]; then ok "the job list is read once"; else fail "job list reads: $(grep -c '/jobs?' "$TMP_ROOT/log")"; fi +# A poll read again is a conditional GET: a 304 stands for the last 200's +# body, so the run still reads in progress, and the polls keep their 60 s pace. +echo 2 >"$TMP_ROOT/not-modified" +STUB_ETAG='W/"e1"' STUB_NOT_MODIFIED_FILE="$TMP_ROOT/not-modified" \ + expect "a poll answered 304 reads the run as last read, then re-runs once it finishes" 0 "Re-running ci-status (job 77) of run 9 on $HEAD as attempt 2." +check_reran 1 "the conditionally polled run's ci-status is re-run once" +if [[ "$(grep -c '^sleep 60$' "$TMP_ROOT/log")" == 4 && "$(grep -c '^sleep' "$TMP_ROOT/log")" == 4 ]]; then ok "two 304s cost two 60 s polls and no retry"; else fail "sleeps: $(grep '^sleep' "$TMP_ROOT/log" | paste -sd'|')"; fi +check_calls 8 "each poll still sends one read" +if [[ "$(grep -c "If-None-Match: W/\"e1\"" "$TMP_ROOT/log")" == 3 && "$(grep -c '^304 ' "$TMP_ROOT/rt/api-budget/github-token")" == 2 ]]; then + ok "every repeat poll is conditional, and each 304 keeps its reading" +else + fail "conditional reads: $(grep -c 'If-None-Match' "$TMP_ROOT/log"), 304 readings: $(grep -c '^304 ' "$TMP_ROOT/rt/api-budget/github-token")" +fi STUB_RUN_SEQ="$(run 9 in_progress)" expect "a run still going after the limit fails" 1 "::error::run 9 on $HEAD has not finished after 600s" check_reran 0 "nothing is re-run past the poll limit"