Part of #4496. P9 — medium severity, medium confidence.
Context
The comment at src/index.ts:163-164 calls refresh-installation-health a "cheap single-call health job" that should "stay unconditional," justifying its unconditional enqueue every 30 minutes (index.ts:171, inside the minute % 30 === 0 block — notably NOT gated by selfHostedReviews, so this runs on the hosted multi-tenant cloud deployment too, where installation count matters most).
But refreshInstallationHealth (src/github/backfill.ts:1146-1149) calls listInstallations(env) — EVERY installation — then refreshInstallationHealthRecords (backfill.ts:1158-1161) does a sequential for (const installation of installations) { await refreshStoredInstallation(...) } — one await per installation, not bounded to one call. In non-broker (local) mode — the mode the hosted cloud service runs in — refreshStoredInstallation (backfill.ts:1289-1306) calls getAppInstallation (src/github/app.ts:371-390), a real GET https://api.github.com/app/installations/{id} via timeoutFetch with NO githubRateLimitAdmission/githubRateLimitAdmissionKey option at all — unlike sibling calls in the same file (app.ts:418-419, 452-453, 592-593) that do pass these. This specific call is therefore invisible to the shared rate-limit observation store entirely, not just unthrottled.
refresh-installation-health is absent from GITHUB_BUDGET_BACKGROUND_TYPES (queue-common.ts:86-98), so it gets none of the GitHub-REST-budget-specific claim-time admission comparable heavy jobs get, and its enqueue isn't conditioned on shouldWaitForGitHubRateLimit the way the backfill push a few lines above it is.
Correction from the audit's adversarial verification pass: the finding as originally framed ("zero backpressure of any kind") overstated the gap — refresh-installation-health IS listed in MAINTENANCE_JOB_TYPES (maintenance-admission.ts:43), so it DOES get real host-load/live-queue-backlog-based admission (deny + jittered requeue) at claim time. The real, narrower gap: its actual GitHub REST calls are never admission-checked or observed against GitHub's rate limit specifically (only against host CPU/queue-depth pressure), unlike every comparable heavy GitHub-calling background job in this codebase — a self-host managing N installations makes N sequential, GitHub-budget-blind JWT-authenticated calls every 30 minutes regardless of how depleted the GitHub REST budget is.
Requirements
- Either cap/bound concurrency over installations (with a reasonable limit) or add
refresh-installation-health to GITHUB_BUDGET_BACKGROUND_TYPES (or an equivalent enqueue-time guard).
- Pass a
githubRateLimitAdmission/rateLimitAdmissionKey option into getAppInstallation's timeoutFetch call so this specific call is actually observed into the shared rate-limit store, matching its siblings at app.ts:418-419, 452-453, 592-593.
- Correct the
index.ts:163-164 "cheap single-call" comment to accurately describe the per-installation loop.
- Invariant + regression tests (non-negotiable): an invariant test asserting
getAppInstallation's call during refreshStoredInstallation IS recorded into the rate-limit observation store (mirroring the sibling calls' existing coverage); a regression test with a large N-installation fixture asserting the job correctly yields/backs off when the GitHub REST budget is exhausted, instead of blindly making N calls regardless.
Deliverables
Expected outcome
refresh-installation-health's real GitHub REST calls are visible to and gated by the same GitHub-budget rate-limit convention every comparable background job already follows, closing the one concrete inconsistency the audit found in this codebase's own rate-limit-admission discipline.
References
src/index.ts:163-171 (the stale "single-call" comment + unconditional enqueue)
src/github/backfill.ts:1146-1161, 1289-1306 (the per-installation loop + the unrecorded call)
src/github/app.ts:371-390, 418-419, 452-453, 592-593 (the unrecorded call + its properly-recorded siblings)
src/selfhost/queue-common.ts:86-98 (GITHUB_BUDGET_BACKGROUND_TYPES, absent membership)
src/selfhost/maintenance-admission.ts:40-66 (the host-load admission it DOES already have)
Effort
S
Part of #4496. P9 — medium severity, medium confidence.
Context
The comment at
src/index.ts:163-164callsrefresh-installation-healtha "cheap single-call health job" that should "stay unconditional," justifying its unconditional enqueue every 30 minutes (index.ts:171, inside theminute % 30 === 0block — notably NOT gated byselfHostedReviews, so this runs on the hosted multi-tenant cloud deployment too, where installation count matters most).But
refreshInstallationHealth(src/github/backfill.ts:1146-1149) callslistInstallations(env)— EVERY installation — thenrefreshInstallationHealthRecords(backfill.ts:1158-1161) does a sequentialfor (const installation of installations) { await refreshStoredInstallation(...) }— one await per installation, not bounded to one call. In non-broker (local) mode — the mode the hosted cloud service runs in —refreshStoredInstallation(backfill.ts:1289-1306) callsgetAppInstallation(src/github/app.ts:371-390), a realGET https://api.github.com/app/installations/{id}viatimeoutFetchwith NOgithubRateLimitAdmission/githubRateLimitAdmissionKeyoption at all — unlike sibling calls in the same file (app.ts:418-419, 452-453, 592-593) that do pass these. This specific call is therefore invisible to the shared rate-limit observation store entirely, not just unthrottled.refresh-installation-healthis absent fromGITHUB_BUDGET_BACKGROUND_TYPES(queue-common.ts:86-98), so it gets none of the GitHub-REST-budget-specific claim-time admission comparable heavy jobs get, and its enqueue isn't conditioned onshouldWaitForGitHubRateLimitthe way the backfill push a few lines above it is.Correction from the audit's adversarial verification pass: the finding as originally framed ("zero backpressure of any kind") overstated the gap —
refresh-installation-healthIS listed inMAINTENANCE_JOB_TYPES(maintenance-admission.ts:43), so it DOES get real host-load/live-queue-backlog-based admission (deny + jittered requeue) at claim time. The real, narrower gap: its actual GitHub REST calls are never admission-checked or observed against GitHub's rate limit specifically (only against host CPU/queue-depth pressure), unlike every comparable heavy GitHub-calling background job in this codebase — a self-host managing N installations makes N sequential, GitHub-budget-blind JWT-authenticated calls every 30 minutes regardless of how depleted the GitHub REST budget is.Requirements
refresh-installation-healthtoGITHUB_BUDGET_BACKGROUND_TYPES(or an equivalent enqueue-time guard).githubRateLimitAdmission/rateLimitAdmissionKeyoption intogetAppInstallation'stimeoutFetchcall so this specific call is actually observed into the shared rate-limit store, matching its siblings atapp.ts:418-419, 452-453, 592-593.index.ts:163-164"cheap single-call" comment to accurately describe the per-installation loop.getAppInstallation's call duringrefreshStoredInstallationIS recorded into the rate-limit observation store (mirroring the sibling calls' existing coverage); a regression test with a large N-installation fixture asserting the job correctly yields/backs off when the GitHub REST budget is exhausted, instead of blindly making N calls regardless.Deliverables
getAppInstallation's call within the health-refresh path is recorded into the shared rate-limit observation storeindex.ts:163-164comment correctedExpected outcome
refresh-installation-health's real GitHub REST calls are visible to and gated by the same GitHub-budget rate-limit convention every comparable background job already follows, closing the one concrete inconsistency the audit found in this codebase's own rate-limit-admission discipline.References
src/index.ts:163-171(the stale "single-call" comment + unconditional enqueue)src/github/backfill.ts:1146-1161, 1289-1306(the per-installation loop + the unrecorded call)src/github/app.ts:371-390, 418-419, 452-453, 592-593(the unrecorded call + its properly-recorded siblings)src/selfhost/queue-common.ts:86-98(GITHUB_BUDGET_BACKGROUND_TYPES, absent membership)src/selfhost/maintenance-admission.ts:40-66(the host-load admission it DOES already have)Effort
S