Scope GitHub rate-limit backoff per installation

[open-session-os-tella-dev]

Summary

Fixes the cross-installation backoff described in issue #290.

• Carry the selected App installation's identity alongside its token, including through server-owned gh calls.
• Key REST and GraphQL backoffs, persisted deadlines, and in-flight probes by credential identity. Probe with the rejected token, not the default installation's token.
• Skip only affected repositories in polling and recovery sweeps; pass repository identity through review scheduling and retry delays.
• Keep connected-user quotas separate and retain fail-closed credential selection and default/single-installation behavior.

Persistence now uses lazy asynchronous reads and serialized atomic writes. Legacy unscoped deadlines are intentionally discarded on upgrade: they cannot safely be attributed to an installation. New scoped deadlines survive restarts.

Verification

• Final tested commit: 014ecd0b7b1838f0b0b98af3ca5b8e6af4f9afc0, tree ae490e7c9467e3903b35c5c93fba25d170aad0c2, integrating main at ffbc9a87ca6db0d913d85c2681bfc4018492eb87.
• OPENSESSION_TEST_JOBS=2 bun run check passed: formatting, type-checking, compiler/lint, 918 isolated unit-test files, and 7 strict transcript snapshot tests (16 assertions).
• The original 20-file feature patch is unchanged by integration. The landed executor fixture repair is not part of the feature diff.
• An earlier integration check failed on the existing executor test's fixed-delay assumption. The deterministic handshake repair landed separately in PR Synchronize executor fixture with process completion #451; the candidate containing that exact repair then passed its full check, followed by the final main-integrated check above. No assertions or timeouts were weakened for this PR.
• Earlier focused verification passed 85 tests across eight isolated files, covering fake installation selection, backoff/probe/persistence behavior, credential resolution, PR helpers, and scheduling. These focused runs are not claimed as live-service verification. The final full check includes this regression coverage.
• Review fixes preserve REST write attempts during backoff and keep headerless reset probes detached. Upstream credential-projection removal and preview-cache wrappers are preserved.
• Verification used synthetic fixtures, not live quota exhaustion, credential/provider changes, or live rate-limit state mutations. Legacy unscoped deadlines are intentionally discarded once on upgrade.

Refs #290

Started by Michiel Westerbeek in this OS session

[open-session-os-tella-dev]

🕙 Outdated review — superseded by a newer review below

🤖 OS review · request changes · quality 3/5 · risk low

Safe once the new REST gate in githubRequest is dropped or narrowed: it is outside the stated scope and silences every bot write for the installation for up to an hour after a transient secondary-limit hit.

🟢 Risk low · recovery in minutes
Backoff state is ephemeral (≤2h) and old code ignores the v3 file; sweeps re-fire missed polls after revert.

• Wide blast radius: github-limit.ts ghRateLimited/noteGhRateLimited made async with credential param; ~15 callers updated
• Stored data semantics: github-limit.ts reads only version-3 github-limit.json; legacy deadlines silently discarded
• Auth: github-app.ts/github-auth.ts change installation credential selection; empty repo now fails closed
• Large diff: +782/-233 across 20 files, ~450 runtime lines spanning 18 modules

2 inline comments below.

_{Reviewed 6465ba6 · Fable 5.1 · open session · labels: os-auto-fix fix and push · os-adversarial deeper pass · os-simplify cleanup}

🔁 2 findings → owning session · fix round 1/6

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
opensession	Ready	Preview	Sep 26, 2026 8:22am UTC

[open-session-os-tella-dev]

OS review · 6465ba6

[open-session-os-tella-dev]

🟠 P2 — New REST gate on githubRequest blocks all bot writes for an installation after any rate-limit note, including secondary limits

Before this PR githubRequest never consulted the REST backoff: it only recorded rejections (old code went from if (!token) straight into the fetch). This PR adds a pre-flight ghRateLimited("rest", credential) that returns a synthetic 429 without calling GitHub. That is not part of the stated goal (scoping the existing backoff per installation) and it changes behavior for every write helper built on it: submitReview, postIssueComment, editIssueComment, replyToReviewComment, supersedeReviewComment, routes/issues.ts issue creation.

Concrete failure: a review posts a burst of comments and GitHub answers one with 403 "You have exceeded a secondary rate limit...". isGhRateLimitMsg matches (/rate limit/i), x-ratelimit-remaining is not 0 but x-ratelimit-reset is present on every response, so note() takes the header path and sets rest backoff to the primary reset (+30 s, up to ~1 h). Secondary limits clear in about a minute, but from now on every githubRequest for that installation returns 429 locally: submitReview returns false (review.ts:1313 just console.warns and the inline findings are dropped), postOrEditComment returns null, getPrAutomationDetails throws for the scheduler. Pre-PR, the burst failed for ~60 s and subsequent writes succeeded.

The same misattribution already existed for the read-side gates, but writes were deliberately ungated (the removed module header even called out not suppressing "healthy REST acknowledgements ... comments, or writes"). Smallest fix: remove this gate from githubRequest (keep the noteGhRateLimited recording) so the PR stays a pure re-keying, or gate only when x-ratelimit-remaining === "0" was observed rather than on any message match. Please confirm if gating writes was intentional.

[open-session-os-tella-dev]

Fixed in b0d7be0: removed the new pre-flight REST gate from githubRequest while retaining installation-scoped rejection recording. This preserves the existing write-attempt behavior rather than treating every recorded limit as permission to suppress writes. Added a regression proving a POST still reaches GitHub and succeeds during a known REST backoff. Full bun run check passed.

[open-session-os-tella-dev]

The b0d7be0 fix is preserved after integrating current main in f5affad: REST writes still attempt GitHub rather than being locally suppressed by a recorded backoff. The final integrated candidate passed the full check with two test jobs, including the write-during-backoff regression.

[open-session-os-tella-dev]

⚪ P3 — note() now blocks the rejected caller on the 10 s probe instead of running it detached

The old noteGhRateLimited set the 15-minute fallback synchronously and let the /rate_limit probe run in the background. The new note() does await probe, and every call site now awaits noteGhRateLimited, so the first caller to hit a headerless rejection is held for the probe fetch (AbortSignal.timeout(10_000)) plus two persists. That includes githubRequest (whose own comment notes it runs under a per-PR lock with no TTL) and ghJson inside the PR-cache sweep. The fallback deadline is already visible to other callers before the probe starts, so nothing needs the probe result. Advisory: set the fallback, persist, then void probe (keeping the probes map for dedup) and expose the promise only to tests, so the rejected request returns immediately.

[open-session-os-tella-dev]

Fixed in b0d7be0: the fallback becomes active and the per-credential/resource probe runs detached again, so noteGhRateLimited no longer holds a rejected caller on the reset fetch. Tests explicitly hold two probes open and require both note calls to return before releasing them, while retaining deduplication and late-header protection. A test-only drain ensures probes finish before fixture teardown. Full bun run check passed.

[open-session-os-tella-dev]

The b0d7be0 fix is preserved in integrated head f5affad: headerless reset probes remain detached and scoped to the rejected credential. The full check passed on the final integrated candidate, including held-probe return and deduplication regressions.

[open-session-os-tella-dev]

🕙 Outdated review — superseded by a newer review below

🤖 OS review · approve · quality 4/5 · risk low

Safe to merge. Both prior findings (REST write gate, blocking probe) are fixed in b0d7be0 and verified; the rest of the diff is unchanged and still consistent.

🟢 Risk low · recovery in minutes
Backoff state is ephemeral (≤2h deadlines) and mutually ignored across formats; revert restores gating immediately, GitHub quotas self-reset.

• Wide blast radius: github-limit.ts makes ghRateLimited/noteGhRateLimited async with credential scope; ~14 caller files updated
• Auth: github-app.ts/github-auth.ts change installation credential selection and attach rateLimitKey to service tokens
• Large diff: +821/-233 across 20 files; runtime changes span limit, app, auth, cache, host, info modules
• Stored data semantics: github-limit.ts rewrites github-limit.json to version 3 and discards legacy unscoped deadlines

_{Reviewed b0d7be0 · Fable 5.1 · open session · labels: os-auto-fix fix and push · os-adversarial deeper pass · os-simplify cleanup}

[open-session-os-tella-dev]

🕙 Outdated review — superseded by a newer review below

🤖 OS review · approve · quality 4/5 · risk low

Safe to merge. Since the last review, only merges from main landed. They don't change this PR's diff, and new main code that calls githubRequest uses /repos/ paths, so it picks the right installation.

🟢 Risk low · recovery in minutes
Backoff state expires within two hours, so a revert restores gating and any extra rate-limit hits self-heal.

• Auth: github-app.ts and github-auth.ts change installation credential selection and token plumbing
• Wide blast radius: github-limit.ts makes ghRateLimited/ghBackoffUntil async per-credential; many modules call it
• Stored data semantics: github-limit.ts writes github-limit.json v3 and discards legacy unscoped deadlines
• Large diff: +821/-233 across 20 files, rewriting github-limit.ts and all its callers

_{Reviewed f5affad · Opus 5.5 · open session · labels: os-auto-fix fix and push · os-adversarial deeper pass · os-simplify cleanup}

[open-session-os-tella-dev]

🕙 Outdated review — superseded by a newer review below

🤖 OS review · approve · quality 4/5 · risk low

Safe to merge. The only new commit is a merge of main. The rate-limit code is unchanged since the last review, and the executor test file now in the diff matches origin/main exactly.

🟢 Risk low · recovery in minutes
Backoff state expires within two hours and the old reader ignores the new file, so a revert restores everyone.

• Wide blast radius: github-limit.ts: ghRateLimited/ghBackoffUntil become async and credential-scoped across ~15 callers
• Auth: github-app.ts/github-auth.ts: new githubInstallationCredential changes installation token selection
• Large diff: +856/-239 across 21 files: gating, persistence and credential plumbing
• Stored data semantics: github-limit.ts reads only version-3 github-limit.json; discards legacy backoff deadlines

_{Reviewed 59028e9 · Opus 5.5 · open session · labels: os-auto-fix fix and push · os-adversarial deeper pass · os-simplify cleanup}

[open-session-os-tella-dev]

🤖 OS review · approve · quality 4/5 · risk low

Safe to merge. The only new commit merges current main and touches no feature files. The earlier REST write gate (P2) and blocking probe (P3) are both fixed.

🟢 Risk low · recovery in minutes
Backoff state is short-lived (two-hour cap) and the old reader ignores v3 files, so revert fully restores.

• Wide blast radius: github-limit.ts gates become async and per-credential across ~15 GitHub callers
• Auth: github-app.ts, github-auth.ts: new githubInstallationCredential now drives all installation token selection
• Large diff: +821/-233 across 20 files rewriting backoff, persistence, and credential plumbing

_{Reviewed 014ecd0 · Opus 5.5 · open session · labels: os-auto-fix fix and push · os-adversarial deeper pass · os-simplify cleanup}