You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Three defects in outbound GitHub rate-limit handling. The first periodically erases a real exhaustion backoff, causing a parked job cohort to wake into 403s every 30 minutes. The second disobeys GitHub's own Retry-After for secondary limits in a way GitHub documents as escalating the block. The third blinds the pacing check that protects the heaviest REST consumers.
1. App-JWT observations are recorded under installation admission keys
src/github/app.ts:406-432 — getAppInstallation authenticates with the App JWT, which has its own separate 5000/hr bucket, but records the response's x-ratelimit-remaining into github_rate_limit_observations under admissionKey = installation:{id} (key built at :406, recorded at :423).
Consumers read only the single newest REST row per key (src/selfhost/pg-queue.ts:1917-1931 — ORDER BY observed_at DESC LIMIT 1; src/github/rate-limit.ts:33-38 — find() on newest-first), and src/selfhost/queue-common.ts:574-588 prefers whichever of the local/DB observation is newer — which is the JWT-bucket row.
Scenario: installation 123's bucket is genuinely exhausted (last real row: remaining = 0, reset in 40 min) and its jobs are correctly parked. refresh-installation-health runs every 30 minutes unconditionally (queue-common.ts:113-117) and calls getAppInstallation per installation, writing installation:123 / rest / remaining ≈ 4990. Every parked job's next admission check — including releaseStaleForegroundDeferrals (pg-queue.ts:822-899), which exists precisely to wake jobs the moment admission clears — now sees a healthy newest row, admits the whole cohort, burns inline retries against 403s, re-records 0, and re-parks. This repeats every 30 minutes for the rest of the exhaustion window.
Record JWT-authenticated observations under a distinct key (app-jwt:{appId} — the githubAdmissionKeyScope "other"/"global" scopes already exist) or with resource: "rest_app_jwt", or do not record remaining/reset for them at all.
Test: a JWT-bucket observation does not clear an installation's exhaustion backoff.
2. Retry-After is truncated to 8 s and retried without jitter
GitHub's secondary-limit Retry-After is typically 60 s. The client retries at 8 s, up to 3 times per call (:461-487) — exactly what GitHub's documentation warns can extend or escalate a secondary block. The code does correctly distinguish primary from secondary (isRateLimitedResponse, :411-421) and then disregards the instruction.
There is also no jitter at this layer (a fixed 500/1000/2000 ms ladder), so the up-to-8 concurrent workers that tripped the same limit retry in near-lockstep — roughly 32 requests into an active abuse window within ~4 s. The queue layer honours Retry-After properly with jitter (src/selfhost/queue-common.ts:662-709); the inline layer undercuts it.
When Retry-After exceeds the cap, stop burning inline retries — return the response so the caller/queue path handles it with the correct delay and jitter (that machinery already exists).
Add jitter to the inline ladder for the sub-cap case.
Test: a 60 s secondary-limit Retry-After results in no inline retry and a correctly-delayed queue retry.
3. GraphQL observations crowd REST rows out of the pacing window
src/github/rate-limit.ts:33-36 fetches the latest 10 rows of any resource for the key and then find()s the first with resource === "rest". But githubGraphQl writes a resource: "graphql" row per call under the sameinstallation:{id} key (src/github/backfill.ts:4874).
A GraphQL-heavy stretch (chunked contributor-activity search, PR-detail fallbacks, repo totals) writes 10+ consecutive graphql rows, so the REST row falls outside the window, rest is undefined, and the helper returns "no wait" even with REST at 0. This blinds exactly the in-job checks that pace the heaviest REST consumers: backfill.ts:551, :653, :1627, and processors.ts:1536, :1997. (The pg-queue SQL path is unaffected — it filters resource='rest' in SQL.)
Filter by resource in the query — add a resource parameter to listLatestGitHubRateLimitObservations — instead of post-filtering a mixed 10-row window.
Test: 10 graphql rows followed by an exhausted REST row ⇒ the helper still reports "wait". The existing test only covers resource: "rest" rows, which is why this was missed.
Lower priority, recorded
GraphQL point budget is persisted but never gated, and a 200 response carrying errors[].type: "RATE_LIMITED" is never classified as a rate limit (client.ts:411-412, backfill.ts:5079-5089 both require 403/429). Individual consumers fail closed per call, but nothing slows down: an exhausted GraphQL hour degrades review-thread reads to "unreadable" at full request cadence, while resource='graphql' rows sit unread.
forceRefresh mints bypass the single-flight (app.ts:145, placed before the inFlightMints join), so N concurrent 403s cause N simultaneous token mints — the exact herd inFlightMints exists to prevent. A permanent permission gap turns this into one extra mint per operation, indefinitely, plus a fleet-shared-cache eviction that can clobber a peer's fresh token (:172-179).
The fleet sweep is gated on an unscoped global observation (src/index.ts:162): whichever bucket wrote the globally-newest REST row decides, so an exhausted public-token bucket can skip agent-regate-sweep fleet-wide for up to an hour while every installation bucket is healthy.
githubRateLimitRetryDelayMs cannot recognise GitHubApiError (queue-common.ts:662-695 reads err.status/err.response.headers; GitHubApiError exposes statusCode/rateLimitResetAt). Residual today because backfill converts these internally, but any new call site that lets one propagate silently loses rate-limit-aware retry.
Summary
Three defects in outbound GitHub rate-limit handling. The first periodically erases a real exhaustion backoff, causing a parked job cohort to wake into 403s every 30 minutes. The second disobeys GitHub's own
Retry-Afterfor secondary limits in a way GitHub documents as escalating the block. The third blinds the pacing check that protects the heaviest REST consumers.1. App-JWT observations are recorded under installation admission keys
src/github/app.ts:406-432—getAppInstallationauthenticates with the App JWT, which has its own separate 5000/hr bucket, but records the response'sx-ratelimit-remainingintogithub_rate_limit_observationsunderadmissionKey = installation:{id}(key built at:406, recorded at:423).Consumers read only the single newest REST row per key (
src/selfhost/pg-queue.ts:1917-1931—ORDER BY observed_at DESC LIMIT 1;src/github/rate-limit.ts:33-38—find()on newest-first), andsrc/selfhost/queue-common.ts:574-588prefers whichever of the local/DB observation is newer — which is the JWT-bucket row.Scenario: installation 123's bucket is genuinely exhausted (last real row: remaining = 0, reset in 40 min) and its jobs are correctly parked.
refresh-installation-healthruns every 30 minutes unconditionally (queue-common.ts:113-117) and callsgetAppInstallationper installation, writinginstallation:123 / rest / remaining ≈ 4990. Every parked job's next admission check — includingreleaseStaleForegroundDeferrals(pg-queue.ts:822-899), which exists precisely to wake jobs the moment admission clears — now sees a healthy newest row, admits the whole cohort, burns inline retries against 403s, re-records 0, and re-parks. This repeats every 30 minutes for the rest of the exhaustion window.app-jwt:{appId}— thegithubAdmissionKeyScope"other"/"global" scopes already exist) or withresource: "rest_app_jwt", or do not record remaining/reset for them at all.2.
Retry-Afteris truncated to 8 s and retried without jittersrc/github/client.ts:426-433:GitHub's secondary-limit
Retry-Afteris typically 60 s. The client retries at 8 s, up to 3 times per call (:461-487) — exactly what GitHub's documentation warns can extend or escalate a secondary block. The code does correctly distinguish primary from secondary (isRateLimitedResponse,:411-421) and then disregards the instruction.There is also no jitter at this layer (a fixed 500/1000/2000 ms ladder), so the up-to-8 concurrent workers that tripped the same limit retry in near-lockstep — roughly 32 requests into an active abuse window within ~4 s. The queue layer honours
Retry-Afterproperly with jitter (src/selfhost/queue-common.ts:662-709); the inline layer undercuts it.Retry-Afterexceeds the cap, stop burning inline retries — return the response so the caller/queue path handles it with the correct delay and jitter (that machinery already exists).Retry-Afterresults in no inline retry and a correctly-delayed queue retry.3. GraphQL observations crowd REST rows out of the pacing window
src/github/rate-limit.ts:33-36fetches the latest 10 rows of any resource for the key and thenfind()s the first withresource === "rest". ButgithubGraphQlwrites aresource: "graphql"row per call under the sameinstallation:{id}key (src/github/backfill.ts:4874).A GraphQL-heavy stretch (chunked contributor-activity search, PR-detail fallbacks, repo totals) writes 10+ consecutive graphql rows, so the REST row falls outside the window,
restisundefined, and the helper returns "no wait" even with REST at 0. This blinds exactly the in-job checks that pace the heaviest REST consumers:backfill.ts:551,:653,:1627, andprocessors.ts:1536,:1997. (The pg-queue SQL path is unaffected — it filtersresource='rest'in SQL.)resourceparameter tolistLatestGitHubRateLimitObservations— instead of post-filtering a mixed 10-row window.resource: "rest"rows, which is why this was missed.Lower priority, recorded
200response carryingerrors[].type: "RATE_LIMITED"is never classified as a rate limit (client.ts:411-412,backfill.ts:5079-5089both require 403/429). Individual consumers fail closed per call, but nothing slows down: an exhausted GraphQL hour degrades review-thread reads to "unreadable" at full request cadence, whileresource='graphql'rows sit unread.forceRefreshmints bypass the single-flight (app.ts:145, placed before theinFlightMintsjoin), so N concurrent 403s cause N simultaneous token mints — the exact herdinFlightMintsexists to prevent. A permanent permission gap turns this into one extra mint per operation, indefinitely, plus a fleet-shared-cache eviction that can clobber a peer's fresh token (:172-179).src/index.ts:162): whichever bucket wrote the globally-newest REST row decides, so an exhausted public-token bucket can skipagent-regate-sweepfleet-wide for up to an hour while every installation bucket is healthy.githubRateLimitRetryDelayMscannot recogniseGitHubApiError(queue-common.ts:662-695readserr.status/err.response.headers;GitHubApiErrorexposesstatusCode/rateLimitResetAt). Residual today because backfill converts these internally, but any new call site that lets one propagate silently loses rate-limit-aware retry.src/github/graphql-cache.ts:67-71), so every hourly rotation invalidates the namespace — the problem fix(github): key the GitHub response cache by installation identity, not the raw token #2538 already fixed for REST by keying on admission identity. The single-flight key already threadsadmissionKey; the cache key does not.