From 94d503c310e09443027ce6911c8034d95eb2c160 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Mon, 19 Oct 2026 13:49:16 +0000 Subject: [PATCH] gh-prs-merge: wait out a spent GitHub rate limit, and fail loudly past it (0.68.3) A rate-limited `gh search prs` was reported as "skipped inaccessible or invalid scope", every scope warned, nothing was found, and the run printed an all-zero summary with exit 0: a sweep that did nothing, reported as a sweep with nothing to do. Gh now recognises GitHub's primary and secondary rate-limit refusals, reads the reset from `gh api rate_limit` (free, answers while everything else is refused), and waits it out when it is within --rate-wait (default 900s), then retries. A reset further off throws RateLimitError, which the sweep counts as FAILED for that scope, so the run exits 1 and says when to retry. Co-Authored-By: Claude Opus 5.5 --- bin/gh-prs-merge.ts | 11 ++++- package.json | 2 +- src/gh.ts | 98 ++++++++++++++++++++++++++++++++++++++++-- src/prs-merge.ts | 9 +++- test/prs-merge.test.ts | 90 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 203 insertions(+), 7 deletions(-) diff --git a/bin/gh-prs-merge.ts b/bin/gh-prs-merge.ts index f4b78eb..eef33e9 100755 --- a/bin/gh-prs-merge.ts +++ b/bin/gh-prs-merge.ts @@ -32,6 +32,8 @@ Options: --no-ready-drafts Ignore draft PRs instead of marking them ready --fix Repair mechanical blockers, then re-evaluate --fix-wait SECONDS How long --fix waits on running checks; default 600 + --rate-wait SECONDS Longest wait for a spent GitHub API quota to reset; + default 900. A later reset fails the run, exit 1 -h, --help Show this help Eligibility: @@ -83,7 +85,7 @@ Fixing (--fix): async function main(argv: string[]): Promise { const parsed = parseArgs(argv, { boolean: ['--apply', '--allow-no-checks', '--ready-drafts', '--no-ready-drafts', '--fix', '--no-fix', '-h', '--help'], - string: ['--orgs', '--users', '--limit', '--fix-wait'], + string: ['--orgs', '--users', '--limit', '--fix-wait', '--rate-wait'], }); if (parsed.flags.has('-h') || parsed.flags.has('--help')) { @@ -115,7 +117,12 @@ async function main(argv: string[]): Promise { throw new UsageError('--fix requires --apply'); } - const summary = await sweep(options, new Gh(), (line) => { + const gh = new Gh({ + rateWaitMs: integer(parsed.values, '--rate-wait', 900, { max: 3_600 }) * 1000, + onRateLimit: (text) => process.stderr.write(`WAIT: ${text}\n`), + }); + + const summary = await sweep(options, gh, (line) => { const text = render(line); if (line.kind === 'failed' || line.kind === 'fixme' || line.kind === 'warn') { process.stderr.write(`${text}\n`); diff --git a/package.json b/package.json index 7d5529c..2ccd50f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@profullstack/cli-tools", - "version": "0.68.2", + "version": "0.68.3", "private": true, "description": "Local command-line tools, in TypeScript, exposed on PATH.", "type": "module", diff --git a/src/gh.ts b/src/gh.ts index c6c7552..97db2f6 100644 --- a/src/gh.ts +++ b/src/gh.ts @@ -27,6 +27,52 @@ export class GhError extends Error { } } +/** + * GitHub refused the call because the token's quota is spent. + * + * Kept apart from GhError because it is not a verdict on the thing asked + * about. The sweep used to read a rate-limited search as "this org is + * inaccessible", warn, find no PRs anywhere, and print an all-zero summary + * with exit 0 — a run that did nothing, reported as a run with nothing to do. + */ +export class RateLimitError extends GhError { + readonly resetAt: Date; + + constructor(message: string, resetAt: Date, result?: RunResult) { + super(message, result); + this.name = 'RateLimitError'; + this.resetAt = resetAt; + } +} + +/** + * Primary ("API rate limit exceeded", GraphQL and REST alike) and secondary + * ("You have exceeded a secondary rate limit") refusals. A rate-limited call + * was not executed, so retrying it — even a merge — cannot do anything twice. + */ +export function isRateLimited(stderr: string): boolean { + return /rate limit (already )?exceeded|exceeded a secondary rate limit|HTTP 429/i.test(stderr); +} + +/** + * When the quota that ran out comes back, from `gh api rate_limit`. + * + * That endpoint is free: it does not count against any quota, so it still + * answers while everything else is refused. A secondary limit leaves every + * bucket with quota remaining and documents no reset, so it gets a minute. + */ +export function parseRateLimitReset(raw: unknown, now: number): Date { + const resources = (raw as { resources?: Record }) + ?.resources; + let latest = 0; + for (const bucket of Object.values(resources ?? {})) { + if (bucket?.remaining === 0 && typeof bucket.reset === 'number') { + latest = Math.max(latest, bucket.reset * 1000); + } + } + return new Date(latest > now ? latest : now + 60_000); +} + /** Values GitHub documents for `mergeable`, plus the honest fallback. */ export const MERGEABLE = ['MERGEABLE', 'CONFLICTING', 'UNKNOWN'] as const; export type Mergeable = (typeof MERGEABLE)[number]; @@ -202,22 +248,68 @@ export function parseMergeAsync(stdout: string): MergeAsyncState | undefined { export interface GhOptions { /** Swap in a fake for tests. */ exec?: typeof run; + sleep?: typeof sleep; + /** + * Longest a single rate-limit wait may be. A reset further off than this + * throws RateLimitError instead of sitting silent for most of an hour. + * Default 15 minutes; 0 never waits. + */ + rateWaitMs?: number; + /** Told before each rate-limit wait, so the pause is not a silent hang. */ + onRateLimit?: (text: string) => void; } export class Gh { private readonly exec: typeof run; + private readonly sleep: typeof sleep; + private readonly rateWaitMs: number; + private readonly onRateLimit: (text: string) => void; constructor(options: GhOptions = {}) { this.exec = options.exec ?? run; + this.sleep = options.sleep ?? sleep; + this.rateWaitMs = options.rateWaitMs ?? 15 * 60_000; + this.onRateLimit = options.onRateLimit ?? (() => {}); } private async call(args: readonly string[]): Promise { // GH_PAGER=cat so a configured pager cannot block on a TTY that is not // there. The bash version set this on every call site and missed none by // luck rather than design. - return this.exec('gh', args, { - env: { ...process.env, GH_PAGER: 'cat', CLICOLOR: '0' }, - }); + const env = { ...process.env, GH_PAGER: 'cat', CLICOLOR: '0' }; + + for (;;) { + const result = await this.exec('gh', args, { env }); + if (result.code === 0 || !isRateLimited(result.stderr)) return result; + + const resetAt = await this.rateLimitReset(env); + // A few seconds past the reset: GitHub's clock and ours are not one clock. + const waitMs = resetAt.getTime() - Date.now() + 5_000; + const when = `${resetAt.toLocaleTimeString()} (in ${Math.ceil(waitMs / 60_000)}m)`; + + if (waitMs > this.rateWaitMs) { + throw new RateLimitError( + `GitHub API rate limit exhausted; it resets at ${when}, past the ` + + `${Math.round(this.rateWaitMs / 60_000)}m --rate-wait budget — ${result.stderr.trim()}`, + resetAt, + result, + ); + } + + this.onRateLimit(`GitHub API rate limit exhausted; waiting until it resets at ${when}`); + await this.sleep(waitMs); + } + } + + private async rateLimitReset(env: NodeJS.ProcessEnv): Promise { + const result = await this.exec('gh', ['api', 'rate_limit'], { env }); + let raw: unknown; + try { + raw = result.code === 0 ? JSON.parse(result.stdout) : undefined; + } catch { + raw = undefined; + } + return parseRateLimitReset(raw, Date.now()); } private async json( diff --git a/src/prs-merge.ts b/src/prs-merge.ts index 7dff78e..a82ddf7 100644 --- a/src/prs-merge.ts +++ b/src/prs-merge.ts @@ -1,4 +1,4 @@ -import { Gh, type Check, type MergeState, type PullRequest, type WorkflowRun } from './gh.ts'; +import { Gh, RateLimitError, type Check, type MergeState, type PullRequest, type WorkflowRun } from './gh.ts'; import { sleep } from './exec.ts'; export interface MergeOptions { @@ -174,6 +174,13 @@ export async function sweep( if (!found.has(pr.url)) found.set(pr.url, pr.createdAt); } } catch (error) { + // Not a verdict on the scope: its PRs were never looked at, so the + // run has to fail rather than summarise zero of them as all there is. + if (error instanceof RateLimitError) { + emit({ kind: 'failed', url: `${qualifier}:${owner}`, reason: error.message }); + summary.failed += 1; + continue; + } emit({ kind: 'warn', text: `skipped inaccessible or invalid scope ${qualifier}:${owner} — ${ diff --git a/test/prs-merge.test.ts b/test/prs-merge.test.ts index 338f30d..0e55e76 100644 --- a/test/prs-merge.test.ts +++ b/test/prs-merge.test.ts @@ -6,6 +6,8 @@ import { parsePullRequest, parseRunsAwaitingApproval, parsePullRequestUrl, + parseRateLimitReset, + isRateLimited, GhError, } from '../src/gh.ts'; import type { RunResult } from '../src/exec.ts'; @@ -631,6 +633,94 @@ describe('sweep', () => { }); }); +describe('rate limit', () => { + const LIMITED = 'GraphQL: API rate limit already exceeded for user ID 27381.'; + + /** `gh search` refused `failures` times, then answers; rate_limit says reset in `resetInS`. */ + function limitedGh(failures: number, resetInS: number, rateWaitMs: number) { + const calls: string[] = []; + const waits: number[] = []; + const notices: string[] = []; + const gh = new Gh({ + rateWaitMs, + sleep: async (ms) => { + waits.push(ms); + }, + onRateLimit: (text) => notices.push(text), + exec: async (_f, args) => { + calls.push(args.join(' ')); + if (args[0] === 'api' && args[1] === 'rate_limit') { + const reset = Math.floor(Date.now() / 1000) + resetInS; + return { + code: 0, + stdout: JSON.stringify({ + resources: { + core: { remaining: 4000, reset: reset + 900 }, + graphql: { remaining: 0, reset }, + }, + }), + stderr: '', + }; + } + if (args[0] === 'search') { + if (failures > 0) { + failures -= 1; + return { code: 1, stdout: '', stderr: LIMITED }; + } + return { code: 0, stdout: '[]', stderr: '' }; + } + return { code: 0, stdout: '[]', stderr: '' }; + }, + }); + return { gh, calls, waits, notices }; + } + + it('recognises primary and secondary refusals, and nothing else', () => { + expect(isRateLimited(LIMITED)).toBe(true); + expect(isRateLimited('HTTP 403: API rate limit exceeded for user ID 1.')).toBe(true); + expect(isRateLimited('You have exceeded a secondary rate limit.')).toBe(true); + expect(isRateLimited('Could not resolve to an Organization')).toBe(false); + }); + + it('takes the reset of the bucket that ran dry, not one with quota left', () => { + const now = 1_000_000_000_000; + const reset = parseRateLimitReset( + { + resources: { + core: { remaining: 10, reset: now / 1000 + 3000 }, + graphql: { remaining: 0, reset: now / 1000 + 120 }, + }, + }, + now, + ); + expect(reset.getTime()).toBe(now + 120_000); + expect(parseRateLimitReset(undefined, now).getTime()).toBe(now + 60_000); + }); + + it('waits out a reset inside the budget, then sweeps', async () => { + const { gh, waits, notices } = limitedGh(1, 60, 15 * 60_000); + const { summary, text } = await runSweep(gh, baseOptions()); + + expect(waits).toHaveLength(1); + expect(notices[0]).toContain('waiting until it resets'); + expect(text).not.toContain('FAILED'); + expect(summary.failed).toBe(0); + }); + + it('fails the run, not just warns, when the reset is past the budget', async () => { + const { gh, waits } = limitedGh(99, 45 * 60, 15 * 60_000); + const { summary, text } = await runSweep( + gh, + baseOptions({ orgs: ['acme', 'other'], users: ['someone'] }), + ); + + expect(waits).toHaveLength(0); + expect(text).toContain('FAILED org:acme — GitHub API rate limit exhausted'); + expect(text).not.toContain('skipped inaccessible'); + expect(summary.failed).toBe(3); + }); +}); + describe('defaults', () => { it('waits ten minutes and polls every twenty seconds', () => { expect(defaults.fixWaitMs).toBe(600_000);