Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions src/db/repositories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1279,9 +1279,17 @@ export async function recordGitHubRateLimitObservation(env: Env, observation: Gi
});
}

export async function listLatestGitHubRateLimitObservations(env: Env, limit = 50): Promise<GitHubRateLimitObservationRecord[]> {
/**
* Latest observations, newest first. When `admissionKey` is given, scoped to ONLY that bucket (#audit-rate-scoping)
* — every managed installation and the separate shared public/registry token draw from DIFFERENT GitHub-side REST
* buckets, so an unscoped read can return the wrong bucket's row (e.g. a fresh public-token observation masking an
* exhausted installation bucket, or vice versa) purely because it happened to be the most recently written.
*/
export async function listLatestGitHubRateLimitObservations(env: Env, limit = 50, admissionKey?: string): Promise<GitHubRateLimitObservationRecord[]> {
const db = getDb(env.DB);
const rows = await db.select().from(githubRateLimitObservations).orderBy(desc(githubRateLimitObservations.observedAt)).limit(limit);
const rows = await (admissionKey !== undefined
? db.select().from(githubRateLimitObservations).where(eq(githubRateLimitObservations.admissionKey, admissionKey)).orderBy(desc(githubRateLimitObservations.observedAt)).limit(limit)
: db.select().from(githubRateLimitObservations).orderBy(desc(githubRateLimitObservations.observedAt)).limit(limit));
return rows.map(toGitHubRateLimitObservationRecord);
}

Expand Down
15 changes: 11 additions & 4 deletions src/github/backfill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ import {
shouldPublishReviewCheck,
} from "../review/check-names";
import { buildReviewThreadBlocker, type ReviewThreadBlocker } from "../review/review-thread-findings";
import { delayUntil, HISTORICAL_BACKFILL_RESERVED_HEADROOM, shouldWaitForGitHubRateLimit } from "./rate-limit";
import { delayUntil, HISTORICAL_BACKFILL_RESERVED_HEADROOM, LOW_REST_RATE_LIMIT_REMAINING, shouldWaitForGitHubRateLimit } from "./rate-limit";
import {
githubRateLimitAdmissionKeyForPublicToken,
githubRateLimitAdmissionKeyForToken,
Expand Down Expand Up @@ -507,7 +507,9 @@ export async function backfillRepositorySegment(
const mode = options.mode ?? "light";
const token = await tokenForRepo(env, repo);
const sourceKind: RepoSyncSegmentRecord["sourceKind"] = repo.installationId && token !== env.GITHUB_PUBLIC_TOKEN ? "installation" : "github";
const resetAt = await shouldWaitForGitHubRateLimit(env);
// Scoped to the bucket this segment's OWN reads actually draw from (#audit-rate-scoping), not whichever bucket
// was most recently observed across every installation and the shared public token.
const resetAt = await shouldWaitForGitHubRateLimit(env, LOW_REST_RATE_LIMIT_REMAINING, repoAdmissionKeyForToken(env, repo, token));
if (resetAt) {
const previous = await getRepoSyncSegment(env, repo.fullName, options.segment);
const segment = await completeSegment(env, repo, options.segment, sourceKind, mode, nowIso(), {
Expand Down Expand Up @@ -607,7 +609,9 @@ export async function backfillOpenPullRequestDetails(
const mode = options.mode ?? "light";
const token = await tokenForRepo(env, repo);
const sourceKind: RepoSyncSegmentRecord["sourceKind"] = repo.installationId && token !== env.GITHUB_PUBLIC_TOKEN ? "installation" : "github";
const resetAt = await shouldWaitForGitHubRateLimit(env);
// Scoped to the bucket this segment's OWN reads actually draw from (#audit-rate-scoping), not whichever bucket
// was most recently observed across every installation and the shared public token.
const resetAt = await shouldWaitForGitHubRateLimit(env, LOW_REST_RATE_LIMIT_REMAINING, repoAdmissionKeyForToken(env, repo, token));
if (resetAt) {
const previous = await getRepoSyncSegment(env, repo.fullName, "pull_request_files");
await env.JOBS.send({ type: "backfill-pr-details", requestedBy: "api", repoFullName: repo.fullName, ...repoInstallationPayload(repo), mode, cursor: options.cursor ?? 0 }, { delaySeconds: delayUntil(resetAt) });
Expand Down Expand Up @@ -1526,7 +1530,10 @@ async function hydrateMergedPullRequestFiles(
.map((record) => record.number),
);
const pending = merged.filter((pr) => !alreadyHydrated.has(pr.number));
const resetAt = pending.length > 0 ? await shouldWaitForGitHubRateLimit(env, HISTORICAL_BACKFILL_RESERVED_HEADROOM) : undefined;
// #audit-rate-scoping: this function already receives (and forwards to its own GitHub reads below) the caller's
// admissionKey — thread it into the budget check too instead of checking whichever bucket was most recently
// observed across every installation and the shared public token.
const resetAt = pending.length > 0 ? await shouldWaitForGitHubRateLimit(env, HISTORICAL_BACKFILL_RESERVED_HEADROOM, admissionKey) : undefined;
if (resetAt) warnings.push(`Historical merged PR file hydration deferred for ${pending.length} pull request(s): GitHub REST budget below the historical-backfill headroom floor (retry after ${resetAt}).`);
const budgeted = resetAt ? new Set<number>() : new Set(pending.slice(0, MERGED_PR_FILE_HYDRATION_BATCH_SIZE[mode]).map((pr) => pr.number));
await mapWithConcurrency(merged, concurrency, async (pr) => {
Expand Down
12 changes: 9 additions & 3 deletions src/github/rate-limit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,15 @@ export const HISTORICAL_BACKFILL_RESERVED_HEADROOM = 300;

/** The REST rate-limit reset time to wait until when the latest recorded REST budget is at/below `floor`, or
* undefined when there is headroom, no usable observation, or the reset is already in the past. Reads the latest
* recorded observation (recordGitHubRateLimitObservation writes one per GitHub call) — no live GitHub call. */
export async function shouldWaitForGitHubRateLimit(env: Env, floor: number = LOW_REST_RATE_LIMIT_REMAINING): Promise<string | undefined> {
const observations = await listLatestGitHubRateLimitObservations(env, 10);
* recorded observation (recordGitHubRateLimitObservation writes one per GitHub call) — no live GitHub call.
*
* `admissionKey`, when given, scopes the read to that bucket ONLY (#audit-rate-scoping) — every installation and
* the shared public/registry token draw from DIFFERENT GitHub-side REST buckets, so checking the caller's own
* bucket instead of "whichever bucket was most recently observed" avoids one bucket's health masking or falsely
* throttling an unrelated one. Omitted (as at a few call sites with no natural single bucket to check, e.g. a
* pre-dispatch scheduler tick) preserves the prior globally-newest-observation behavior unchanged. */
export async function shouldWaitForGitHubRateLimit(env: Env, floor: number = LOW_REST_RATE_LIMIT_REMAINING, admissionKey?: string): Promise<string | undefined> {
const observations = await listLatestGitHubRateLimitObservations(env, 10, admissionKey);
// Type-guard the find so `remaining` narrows to a number — null/undefined observations are excluded here, so the
// headroom check below needs no further nullish guard.
const rest = observations.find((observation): observation is typeof observation & { remaining: number } => observation.resource === "rest" && observation.remaining !== null && observation.remaining !== undefined);
Expand Down
12 changes: 8 additions & 4 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import { createApp } from "./api/routes";
import { RateLimiter } from "./auth/rate-limit";
import { delayUntil, shouldWaitForGitHubRateLimit, MAINTENANCE_RESERVED_HEADROOM } from "./github/rate-limit";
import { delayUntil, shouldWaitForGitHubRateLimit, LOW_REST_RATE_LIMIT_REMAINING, MAINTENANCE_RESERVED_HEADROOM } from "./github/rate-limit";
import { processDlqBatch } from "./queue/dlq";
import { processJob } from "./queue/processors";
import { isOrbBrokerEnabled } from "./orb/broker";
import { isOpsEnabled } from "./review/ops-wire";
import { isRagEnabled } from "./review/rag-wire";
import { isSelfTuneEnabled } from "./review/selftune-wire";
import {
githubRateLimitAdmissionKeyForJob,
isGitHubBudgetBackgroundJob,
queueSnapshotBacklog,
queueSnapshotFromBinding,
Expand Down Expand Up @@ -54,7 +55,9 @@ export default {
continue;
}
if (isGitHubBudgetBackgroundJob(message.body)) {
const resetAt = await shouldWaitForGitHubRateLimit(env, MAINTENANCE_RESERVED_HEADROOM).catch(() => undefined);
// Scoped to THIS job's own installation bucket (#audit-rate-scoping) — an unrelated installation's or
// the shared public token's budget must never defer (or wrongly clear) this job.
const resetAt = await shouldWaitForGitHubRateLimit(env, MAINTENANCE_RESERVED_HEADROOM, githubRateLimitAdmissionKeyForJob(message.body) ?? undefined).catch(() => undefined);
if (resetAt) {
console.log(
JSON.stringify({
Expand Down Expand Up @@ -83,8 +86,9 @@ export default {
);
// If the shared GitHub REST budget is exhausted, this failure is most likely a rate-limit — retry AFTER the
// reset so a real webhook OUTLASTS a transient rate-limit window instead of burning its retries immediately
// and being dead-lettered (the surviving event-loss path). (#audit-rate-headroom)
const resetAt = await shouldWaitForGitHubRateLimit(env).catch(() => undefined);
// and being dead-lettered (the surviving event-loss path). (#audit-rate-headroom) Scoped to THIS job's own
// bucket (#audit-rate-scoping) so an unrelated installation's exhaustion never delays this job's retry.
const resetAt = await shouldWaitForGitHubRateLimit(env, LOW_REST_RATE_LIMIT_REMAINING, githubRateLimitAdmissionKeyForJob(message.body) ?? undefined).catch(() => undefined);
if (resetAt) message.retry({ delaySeconds: delayUntil(resetAt) });
else message.retry();
}
Expand Down
9 changes: 6 additions & 3 deletions src/queue/dlq.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { getWebhookEvent, recordAuditEvent } from "../db/repositories";
import { delayUntil, shouldWaitForGitHubRateLimit } from "../github/rate-limit";
import { delayUntil, LOW_REST_RATE_LIMIT_REMAINING, shouldWaitForGitHubRateLimit } from "../github/rate-limit";
import { incr } from "../selfhost/metrics";
import { githubRateLimitAdmissionKeyForJob } from "../selfhost/queue-common";
import type { JobMessage, JsonValue } from "../types";

const DLQ_DEAD_LETTERED_METRIC = "gittensory_dlq_dead_lettered_total";
Expand Down Expand Up @@ -49,8 +50,10 @@ export async function processDlqBatch(batch: MessageBatch<JobMessage>, env: Env,
const event = await getWebhookEvent(env, webhook.deliveryId).catch(() => null);
if (event?.status !== "processed") {
// If the webhook dead-lettered because the shared GitHub REST budget was exhausted, re-drive it AFTER the
// reset (retry-until-recovered) rather than immediately re-failing it. (#audit-rate-headroom)
const resetAt = await shouldWaitForGitHubRateLimit(env).catch(() => undefined);
// reset (retry-until-recovered) rather than immediately re-failing it. (#audit-rate-headroom) Scoped to
// THIS webhook's own installation bucket (#audit-rate-scoping) so an unrelated installation's exhaustion
// never delays this re-drive.
const resetAt = await shouldWaitForGitHubRateLimit(env, LOW_REST_RATE_LIMIT_REMAINING, githubRateLimitAdmissionKeyForJob(webhook) ?? undefined).catch(() => undefined);
const options = resetAt ? { delaySeconds: delayUntil(resetAt) } : undefined;
const queue = env.WEBHOOKS;
if (queue) {
Expand Down
6 changes: 6 additions & 0 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1685,9 +1685,12 @@ async function sweepRepoRegate(
// Reserve installation rate-limit headroom for real webhook traffic (#audit-rate-headroom): with the shared REST
// budget at/below the maintenance floor, defer the WHOLE sweep until the reset rather than fanning out per-PR
// jobs that would each have to defer. Webhooks never pre-yield, so this hands the remaining budget to them.
// Scoped to THIS repo's own installation bucket (#audit-rate-scoping) — an unrelated installation's or the
// shared public token's budget must never defer (or wrongly clear) this repo's own sweep.
const sweepRateResetAt = await shouldWaitForGitHubRateLimit(
env,
MAINTENANCE_RESERVED_HEADROOM,
typeof repo?.installationId === "number" ? githubRateLimitAdmissionKeyForInstallation(repo.installationId) : undefined,
);
if (sweepRateResetAt) {
await env.JOBS.send(
Expand Down Expand Up @@ -2024,9 +2027,12 @@ async function regatePullRequest(
// repair enqueue) is current-HEAD contributor-PR-review work and gets the SAME low floor a fresh webhook
// gets — it must never be treated as background maintenance and parked behind it. Mirrors the SAME
// reclassification githubRateLimitAdmissionTargetForJob applies at the queue-admission layer.
// Scoped to THIS installation's own bucket (#audit-rate-scoping) — an unrelated installation's or the shared
// public token's budget must never defer (or wrongly clear) this PR's own re-gate.
const rateResetAt = await shouldWaitForGitHubRateLimit(
env,
isScheduledRegateSweepJob(deliveryId) ? MAINTENANCE_RESERVED_HEADROOM : LOW_REST_RATE_LIMIT_REMAINING,
githubRateLimitAdmissionKeyForInstallation(installationId),
);
if (rateResetAt) {
await env.JOBS.send(
Expand Down
6 changes: 4 additions & 2 deletions src/upstream/commit.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { githubRateLimitAdmissionKeyForPublicToken, timeoutFetch } from "../github/client";
import { shouldWaitForGitHubRateLimit } from "../github/rate-limit";
import { LOW_REST_RATE_LIMIT_REMAINING, shouldWaitForGitHubRateLimit } from "../github/rate-limit";

function upstreamCommitHeaders(token: string | undefined): Record<string, string> {
return {
Expand Down Expand Up @@ -37,7 +37,9 @@ export async function resolveUpstreamCommitSha(
// Budget-gate the NETWORK read only: a cached resolve is still served for free even under pressure; a fresh
// read is skipped (→ synthetic non-OK → null → caller falls back to the mutable ref) when the REST budget is
// at/below the low-water floor. This callback runs ONLY on a cache miss, so it never suppresses a cache hit.
githubSkipNetworkWhen: () => shouldWaitForGitHubRateLimit(env).then(Boolean),
// Scoped to the public-token bucket (#audit-rate-scoping) — the same bucket the read below actually draws
// from — so an installation's unrelated budget can never mask or falsely trip this gate.
githubSkipNetworkWhen: () => shouldWaitForGitHubRateLimit(env, LOW_REST_RATE_LIMIT_REMAINING, githubRateLimitAdmissionKeyForPublicToken()).then(Boolean),
// Give this bare-commit read the same stable, distinctly-scoped cache-key identity every installation-token
// read gets, so the shared public token rotating doesn't reset this cacheable "commit" class either (#2538).
githubRateLimitAdmissionKey: githubRateLimitAdmissionKeyForPublicToken(),
Expand Down
3 changes: 3 additions & 0 deletions test/unit/backfill-file-hydration-scoping.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -326,8 +326,11 @@ describe("GitHub PR file hydration scoping (#audit-rate-headroom)", () => {
// Below HISTORICAL_BACKFILL_RESERVED_HEADROOM (300) but above MAINTENANCE_RESERVED_HEADROOM (150) and
// LOW_REST_RATE_LIMIT_REMAINING (75) — healthy enough for the segment's own entry check and for current-PR
// convergence, but not for the least-urgent historical hydration path.
// Registry-only repo (no installation), so tokenForRepo resolves to the shared public token and
// repoAdmissionKeyForToken scopes to that bucket (#audit-rate-scoping).
await recordGitHubRateLimitObservation(env, {
repoFullName: "JSONbored/gittensory",
admissionKey: "public-token",
resource: "rest",
path: "/pulls",
statusCode: 200,
Expand Down
15 changes: 15 additions & 0 deletions test/unit/backfill.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3039,6 +3039,9 @@ describe("GitHub backfill", () => {
});
await recordGitHubRateLimitObservation(env, {
repoFullName: "JSONbored/gittensory",
// Registry-only repo (no installation), so tokenForRepo resolves to the shared public token and
// repoAdmissionKeyForToken scopes to that bucket (#audit-rate-scoping).
admissionKey: "public-token",
resource: "rest",
path: "/issues",
statusCode: 200,
Expand Down Expand Up @@ -3700,6 +3703,9 @@ describe("GitHub backfill", () => {
});
await recordGitHubRateLimitObservation(env, {
repoFullName: "JSONbored/gittensory",
// Registry-only repo (no installation), so tokenForRepo resolves to the shared public token and
// repoAdmissionKeyForToken scopes to that bucket (#audit-rate-scoping).
admissionKey: "public-token",
resource: "rest",
path: "/labels",
statusCode: 200,
Expand Down Expand Up @@ -3733,6 +3739,9 @@ describe("GitHub backfill", () => {
await seedRegisteredRepo(freshWaitEnv);
await recordGitHubRateLimitObservation(freshWaitEnv, {
repoFullName: "JSONbored/gittensory",
// Registry-only repo (no installation), so tokenForRepo resolves to the shared public token and
// repoAdmissionKeyForToken scopes to that bucket (#audit-rate-scoping).
admissionKey: "public-token",
resource: "rest",
path: "/labels",
statusCode: 200,
Expand Down Expand Up @@ -3802,6 +3811,9 @@ describe("GitHub backfill", () => {
await seedRegisteredRepo(env);
await recordGitHubRateLimitObservation(env, {
repoFullName: "JSONbored/gittensory",
// Registry-only repo (no installation), so tokenForRepo resolves to the shared public token and
// repoAdmissionKeyForToken scopes to that bucket (#audit-rate-scoping).
admissionKey: "public-token",
resource: "rest",
path: "/pulls/1/files",
statusCode: 200,
Expand Down Expand Up @@ -3831,6 +3843,9 @@ describe("GitHub backfill", () => {
await seedRegisteredRepo(env);
await recordGitHubRateLimitObservation(env, {
repoFullName: "JSONbored/gittensory",
// Registry-only repo (no installation), so tokenForRepo resolves to the shared public token and
// repoAdmissionKeyForToken scopes to that bucket (#audit-rate-scoping).
admissionKey: "public-token",
resource: "rest",
path: "/pulls",
statusCode: 200,
Expand Down
3 changes: 2 additions & 1 deletion test/unit/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,8 @@ describe("worker entrypoint", () => {
vi.useFakeTimers({ toFake: ["Date"] });
vi.setSystemTime(new Date("2026-06-24T12:00:00.000Z"));
const env = createTestEnv();
await recordGitHubRateLimitObservation(env, { repoFullName: "owner/repo", resource: "rest", path: "/x", statusCode: 200, limitValue: 5000, remaining: 120, resetAt: "2026-06-24T12:10:00.000Z", observedAt: "2026-06-24T12:00:00.000Z" });
// Scoped to this job's own installation bucket (#audit-rate-scoping) — installationId 123 below.
await recordGitHubRateLimitObservation(env, { repoFullName: "owner/repo", admissionKey: "installation:123", resource: "rest", path: "/x", statusCode: 200, limitValue: 5000, remaining: 120, resetAt: "2026-06-24T12:10:00.000Z", observedAt: "2026-06-24T12:00:00.000Z" });
const acked: string[] = [];
const retries: Array<{ delaySeconds?: number } | undefined> = [];
const requeued: Array<{ message: import("../../src/types").JobMessage; delaySeconds?: number }> = [];
Expand Down
Loading