perf(ci): measure unattributed GITHUB_TOKEN calls and cut three without changing behavior

.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 <token>=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.

.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
-  `<name>-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 <name>=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 `<name>-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

.github/actions/log-api-budget/api-budget.mjs

@@ -2,29 +2,7 @@
 // $RUNNER_TEMP/api-budget/<token> 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";

.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/);

.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/<token>
+// 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;
       },

.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.

.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

.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", () => {

.github/scripts/pr-unblock/report-escalation.sh

@@ -145,7 +145,7 @@ key_line="<!-- pr-unblock resume-key=$HEAD_SHA -->"
 
 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')

.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.

.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}`)));
     }