From ead030d1912a564d07882aef7caa8f5c04a21f2a Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Wed, 28 Oct 2026 22:12:43 -0400 Subject: [PATCH] fix(pr-pipeline): plan refire-refused's job-token client to github-token #10811 named lanes-app as the Re-fire step's API_BUDGET_TOKEN, which is right for its GH_TOKEN client. The rate guard also plans each client's read floor to the entry its token names (entryOf), so the ACTIONS_TOKEN client lost github-token's 1000 limit-cap. Pin that client to github-token with budgetLatch, now exported by rate-guard.mjs beside the latch it wraps (label-unblocked re-exports it), and correct the changelog's "No request changes". Co-Authored-By: Claude Opus 5.5 --- ...ount-every-node-github-request-4b9e21c7.md | 2 +- .github/actions/log-api-budget/rate-guard.mjs | 7 +++++ .github/scripts/pr-merge/refire-refused.mjs | 9 +++++- .../scripts/pr-merge/refire-refused.test.mjs | 31 +++++++++++++++++++ .../scripts/work-unblock/label-unblocked.mjs | 8 ++--- 5 files changed, 49 insertions(+), 8 deletions(-) diff --git a/.changes/pr-pipeline/ci-count-every-node-github-request-4b9e21c7.md b/.changes/pr-pipeline/ci-count-every-node-github-request-4b9e21c7.md index 15b5e63e21..c37a46bfa0 100644 --- a/.changes/pr-pipeline/ci-count-every-node-github-request-4b9e21c7.md +++ b/.changes/pr-pipeline/ci-count-every-node-github-request-4b9e21c7.md @@ -4,5 +4,5 @@ bump: patch ### Fixed -- The job-end `api-calls` notice counts every GitHub request the lane scripts' Node clients send. Before, the clients built by address-feedback (its select and context mains, fix-ci-context and review-dependency), pr-verify's job-log read, and three steps that named no bucket sent requests the notice never saw. A client on a second token now counts under `API_BUDGET_OVERRIDE_TOKEN` when the request's token is not the step's own `GH_TOKEN` (else `GITHUB_TOKEN`), the rule babysit's gh calls already follow. Before, the job token's calls in a lanes App step were counted as lanes-app. No request changes. +- The job-end `api-calls` notice counts every GitHub request the lane scripts' Node clients send. Before, the clients built by address-feedback (its select and context mains, fix-ci-context and review-dependency), pr-verify's job-log read, and three steps that named no bucket sent requests the notice never saw. A client on a second token now counts under `API_BUDGET_OVERRIDE_TOKEN` when the request's token is not the step's own `GH_TOKEN` (else `GITHUB_TOKEN`), the rule babysit's gh calls already follow. Before, the job token's calls in a lanes App step were counted as lanes-app. No request changes, with one side effect: the rate guard plans each pool's read floor to the entry its token names, so pr-merge's Re-fire step, which now names `lanes-app`, planned its job-token client to lanes-app's uncapped entry instead of github-token's 1000 `limit-cap`. That client is now pinned to github-token, so its readings and its read-floor warning are the job token's again. - The notice adds `graphql=K`, the GraphQL requests among the count, which spend GraphQL's own pool: `=N not-modified=M graphql=K`. diff --git a/.github/actions/log-api-budget/rate-guard.mjs b/.github/actions/log-api-budget/rate-guard.mjs index 72e43ad547..7c61d56199 100644 --- a/.github/actions/log-api-budget/rate-guard.mjs +++ b/.github/actions/log-api-budget/rate-guard.mjs @@ -456,3 +456,10 @@ export function createRateLimitLatch({ // The process-wide latch every helper shares by default. export const rateLimitLatch = createRateLimitLatch(); + +// latch with its guard pinned to token: a client on a second token keeps its +// readings under token and plans to token's limits.api entry (entryOf), not +// API_BUDGET_TOKEN's, while sharing latch's stops. +export function budgetLatch(token, latch = rateLimitLatch) { + return { ...latch, guard: (fetchImpl) => latch.guard(fetchImpl, { token }) }; +} diff --git a/.github/scripts/pr-merge/refire-refused.mjs b/.github/scripts/pr-merge/refire-refused.mjs index ad29759575..8c67555cb1 100755 --- a/.github/scripts/pr-merge/refire-refused.mjs +++ b/.github/scripts/pr-merge/refire-refused.mjs @@ -46,6 +46,7 @@ import process from "node:process"; import { pathToFileURL } from "node:url"; import { GitHubError, createGitHub, loadTrustedList, paginate } from "../../actions/check-trusted-trigger/check-trusted-trigger.mjs"; +import { budgetLatch, rateLimitLatch } from "../../actions/log-api-budget/rate-guard.mjs"; import { postingRunId } from "../../actions/report-check-run/posting-run.mjs"; import { LANES_APP_LOGIN as DEFAULT_BOT } from "./merge-holds.mjs"; @@ -267,6 +268,12 @@ async function pushEmptyCommit(github, repo, pull, path, tree) { return commit.sha; } +// The job token's client (ACTIONS_TOKEN). The step's API_BUDGET_TOKEN names the +// lane token, so this client is pinned to github-token: its readings and its +// read-floor plan (github-token's limit-cap) are the job token's. +export const actionsClient = (env, { latch = rateLimitLatch, fetchImpl } = {}) => + createGitHub({ token: env.ACTIONS_TOKEN, apiUrl: env.GITHUB_API_URL || undefined, latch: budgetLatch("github-token", latch), ...(fetchImpl ? { fetchImpl } : {}) }); + export async function main({ env = process.env, github, actions, now = Date.now() } = {}) { try { if (!REPO.test(env.GITHUB_REPOSITORY ?? "")) { @@ -284,7 +291,7 @@ export async function main({ env = process.env, github, actions, now = Date.now( throw new Error("LANES_APP_SENDER_ID is not a positive integer"); } github ??= createGitHub({ token: env.GH_TOKEN, apiUrl: env.GITHUB_API_URL || undefined }); - actions ??= createGitHub({ token: env.ACTIONS_TOKEN, apiUrl: env.GITHUB_API_URL || undefined }); + actions ??= actionsClient(env); // Main's tip, read once, so every compare and blob read in this run sees // one commit. A tag named main would make `main` ambiguous, so one must not diff --git a/.github/scripts/pr-merge/refire-refused.test.mjs b/.github/scripts/pr-merge/refire-refused.test.mjs index 04c53321c4..30cafd22a2 100644 --- a/.github/scripts/pr-merge/refire-refused.test.mjs +++ b/.github/scripts/pr-merge/refire-refused.test.mjs @@ -449,3 +449,34 @@ test("the kill switch stops the run before any API call", async () => { assert.equal(await main({ env: { ...env, KILL_SWITCH: "" }, github: w.github, actions: w.actions, now: NOW }), 0); assert.deepEqual(w.calls, []); }); + +// The Re-fire step names lanes-app (its GH_TOKEN, the lane token); the job +// token's client still counts and plans as github-token: the reading lands in +// api-budget/github-token, and its read floor uses github-token's limit-cap of +// 1000 (rate-limits.json), so 950 of 5000 used is under a tenth of the plan +// left (50 of 1000), where lanes-app's uncapped plan of 5000 would not be. +test("the job token's client counts its readings and plans its read floor as github-token", async () => { + const { actionsClient } = await import("./refire-refused.mjs"); + const { createRateLimitLatch } = await import("../../actions/log-api-budget/rate-guard.mjs"); + const { tempDir } = await import("../../../lib/test-tmp.mjs"); + const { rmSync, existsSync } = await import("node:fs"); + const dir = tempDir("refire-budget-"); + const saved = { RUNNER_TEMP: process.env.RUNNER_TEMP, API_BUDGET_TOKEN: process.env.API_BUDGET_TOKEN }; + Object.assign(process.env, { RUNNER_TEMP: dir, API_BUDGET_TOKEN: "lanes-app" }); + try { + const lines = []; + const latch = createRateLimitLatch({ log: (line) => lines.push(line), fail: () => {}, stopFile: null }); + const headers = { "x-ratelimit-limit": "5000", "x-ratelimit-remaining": "4050", "x-ratelimit-used": "950", "x-ratelimit-reset": "1792751600", "x-ratelimit-resource": "core" }; + const fetchImpl = async () => new Response("{}", { status: 200, headers }); + await actionsClient({ ACTIONS_TOKEN: "job" }, { latch, fetchImpl })("GET", "/repos/o/r/actions/runs"); + assert.equal(readFileSync(join(dir, "api-budget", "github-token"), "utf8"), "200 5000 4050 950 1792751600 core\n"); + assert.equal(existsSync(join(dir, "api-budget", "lanes-app")), false); + assert.deepEqual(lines, ["::warning::GitHub's core API has under its read floor left (50 of the planned 1000; header remaining 4050 of 5000); reads continue"]); + } finally { + for (const [key, value] of Object.entries(saved)) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + rmSync(dir, { recursive: true, force: true }); + } +}); diff --git a/.github/scripts/work-unblock/label-unblocked.mjs b/.github/scripts/work-unblock/label-unblocked.mjs index 7e7b34469e..434a7962ae 100755 --- a/.github/scripts/work-unblock/label-unblocked.mjs +++ b/.github/scripts/work-unblock/label-unblocked.mjs @@ -212,7 +212,7 @@ import { clampReset, isLaneBranch, parseOutcomeMarker, refsHoldMarker } from ".. import { readLimits, readWip } from "../work-resolve-issue/limits.mjs"; import { admissionFacts, admissionSettled, ATTENDED, attendedHold, blockersOf, CONFIRMED, decide, loadRules, NEEDS_DECISION, openPullsFor, openSubIssues, READY, refusalText, removalStands, sayWhy, stillBlocking, UNTRUSTED, untrustedHold } from "../intake-triage/admit.mjs"; import { isSplitParent, splitParentClosed } from "../intake-triage/admit.mjs"; -import { rateLimitLatch } from "../../actions/log-api-budget/rate-guard.mjs"; +import { budgetLatch } from "../../actions/log-api-budget/rate-guard.mjs"; import { createJobLogReader } from "./job-log.mjs"; const LABEL = "agent-ready"; @@ -1716,11 +1716,7 @@ const baseAdmission = (env) => { })()); }; -// The rate guard's latch with its readings recorded under token, so a second -// client's readings never mix with the App token's. -export function budgetLatch(token, latch = rateLimitLatch) { - return { ...latch, guard: (fetchImpl) => latch.guard(fetchImpl, { token }) }; -} +export { budgetLatch }; export async function main({ env = process.env, github, actions, jobLog, now = Date.now(), rate = searchRateLimit(), admission = baseAdmission(env) } = {}) { try {