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
17 changes: 10 additions & 7 deletions src/selfhost/maintenance-admission.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,16 @@
// FOREGROUND_QUEUE_PRIORITY_FLOOR, see queue-common.ts) -- must always win a resource race against periodic
// maintenance sweeps (contributor evidence, burden forecasts, RAG re-indexing, drift scans, product rollups,
// notifications...). Those sweeps already run on a conservative cadence (every 30min/hourly/6-hourly, see
// index.ts's enqueueScheduledJobs) and already yield to an EXHAUSTED GitHub REST budget
// (shouldWaitForGitHubRateLimit) -- this module adds an ORTHOGONAL signal: is the box itself under load RIGHT
// NOW (a live-work backlog, an aging live job, a hot host CPU), independent of whether GitHub's API happens to
// be rate-limited. The queue backends (sqlite-queue.ts / pg-queue.ts) consult this at CLAIM time, the same way
// they already consult GitHub rate-limit admission: a denied maintenance job is pushed back to 'pending' with
// a jittered future run_after -- its original enqueue time is left untouched, so the age-based trickle below
// still works -- never dropped and never run early.
// index.ts's enqueueScheduledJobs); the subset that makes real GitHub REST calls ALSO yields to an EXHAUSTED
// GitHub REST budget (shouldWaitForGitHubRateLimit) via isGitHubBudgetBackgroundJob / GITHUB_BUDGET_BACKGROUND_TYPES
// (queue-common.ts) -- purely-internal sweeps that touch no GitHub API (product-usage rollups, retention
// pruning, notification delivery, and similar) have no such budget to yield to and correctly aren't in that set.
// This module adds an ORTHOGONAL signal on top of whichever of those a job type already has: is the box itself
// under load RIGHT NOW (a live-work backlog, an aging live job, a hot host CPU), independent of whether GitHub's
// API happens to be rate-limited. The queue backends (sqlite-queue.ts / pg-queue.ts) consult this at CLAIM time,
// the same way they already consult GitHub rate-limit admission where applicable: a denied maintenance job is
// pushed back to 'pending' with a jittered future run_after -- its original enqueue time is left untouched, so
// the age-based trickle below still works -- never dropped and never run early.
//
// TRICKLE: a maintenance job that has been pending since `maxDeferAgeMs` is force-admitted regardless of
// current pressure, so a box under SUSTAINED load can never starve maintenance work forever -- it just runs at
Expand Down
28 changes: 28 additions & 0 deletions src/selfhost/queue-common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,29 @@ const GITHUB_BUDGET_BACKGROUND_TYPES = new Set<string>([
"refresh-contributor-activity",
"build-burden-forecasts",
"rag-index-repo",
// #4505: found via a systematic audit of every MAINTENANCE_JOB_TYPES member against this set (prompted by
// reconcile-open-prs below) -- each of these five genuinely makes real GitHub REST calls (directly, or
// transitively via resolveRepositorySettings -> loadRepoFocusManifest's cache-miss fetch of .gittensory.json)
// but was missing from this set, contradicting this module's own header comment.
//
// runOpenPrReconciliation makes real, potentially large paginated GitHub REST calls per watched repo (up to
// RECONCILE_OPEN_PRS_MAX_PAGES per repo, plus a catch-up fetch per missing PR found). Flag-gated OFF by
// default today (GITTENSORY_PR_RECONCILIATION) -- this closes the gap before anyone enables it.
"reconcile-open-prs",
// fanOutBacklogConvergenceSweepJobs / sweepRepoBacklogConvergence both call resolveRepositorySettings per
// repo. Runs every 30 min, unconditional for self-hosted runtimes -- active in production today.
"backlog-convergence-sweep",
// selfTuneRepos calls resolveRepositorySettings per registered repo to check acting-autonomy + the per-repo
// opt-out. Hourly, flag-gated OFF by default (GITTENSORY_REVIEW_SELFTUNE).
"selftune",
// refreshInstallationHealthRecords calls getAppInstallation (a direct, unprotected `GET /app/installations/{id}`
// REST call) per installation, PLUS resolveRepositorySettings per installed repo. Runs every 30 min,
// UNCONDITIONAL (not behind any flag) -- the most severe of these five, since it is exercised in every
// deployment today, not just after an operator opts into a flag.
"refresh-installation-health",
// runReviewRecapJob calls loadRepoFocusManifest directly for its one repo. Not yet cron-enqueued (manual/API
// trigger only today, per its own doc comment), but still worth gating against a rapid repeated manual trigger.
"generate-review-recap",
]);
const PRIORITY_BY_TYPE = new Map([
["agent-regate-pr", AGENT_REGATE_PRIORITY],
Expand Down Expand Up @@ -915,6 +938,7 @@ export function jobCoalesceKey(payload: string): string | null {
case "ops-alerts":
case "selftune":
case "retry-orb-relay":
case "reconcile-open-prs":
return type;
case "backfill-registered-repos":
return keyOf(
Expand All @@ -941,6 +965,10 @@ export function jobCoalesceKey(payload: string): string | null {
);
case "generate-signal-snapshots":
case "build-burden-forecasts":
// #4505: no case existed for this single-repo job type at all, so it fell through to the untyped `null`
// below -- every enqueue (repeated manual/API triggers today; a future cron trigger per its own doc
// comment) inserted a fresh duplicate row instead of coalescing into an already-pending/processing one.
case "generate-review-recap":
return keyOf(type, normalizedRepo(message.repoFullName) ?? "all");
case "build-contributor-evidence":
case "build-contributor-decision-packs": {
Expand Down
37 changes: 37 additions & 0 deletions test/unit/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,43 @@ describe("worker entrypoint", () => {
expect(retries).toEqual([]);
});

it("INVARIANT (#4505): pre-yields a reconcile-open-prs job while the shared GitHub REST budget is exhausted", async () => {
vi.useFakeTimers({ toFake: ["Date"] });
vi.setSystemTime(new Date("2026-06-24T12:00:00.000Z"));
const env = createTestEnv();
// reconcile-open-prs has no per-installation field, so it draws from the SAME shared (no-admissionKey)
// observation refresh-registry's own equivalent test above uses.
await recordGitHubRateLimitObservation(env, { repoFullName: "owner/repo", resource: "rest", path: "/x", statusCode: 200, limitValue: 5000, remaining: 5, resetAt: "2026-06-24T12:30: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 }> = [];
env.JOBS = {
async send(message: import("../../src/types").JobMessage, options?: { delaySeconds?: number }) {
requeued.push({ message, ...(options?.delaySeconds === undefined ? {} : { delaySeconds: options.delaySeconds }) });
},
} as unknown as Queue;
const batch = {
messages: [
{
id: "reconcile-tick",
body: { type: "reconcile-open-prs", requestedBy: "schedule" },
ack: () => acked.push("reconcile-tick"),
retry: (options?: { delaySeconds?: number }) => retries.push(options),
},
],
} as unknown as MessageBatch<import("../../src/types").JobMessage>;

await worker.queue(batch, env);

// Pre-yielded, not run: acked (not retried, preserving retry budget) and re-queued after the reset --
// BEFORE this fix, reconcile-open-prs was missing from GITHUB_BUDGET_BACKGROUND_TYPES, so this exhausted
// budget would have been silently ignored and runOpenPrReconciliation would have run immediately.
expect(acked).toEqual(["reconcile-tick"]);
expect(retries).toEqual([]);
expect(requeued).toEqual([{ message: { type: "reconcile-open-prs", requestedBy: "schedule" }, delaySeconds: 900 }]); // delayUntil clamps to [30, 900]
vi.useRealTimers();
});

it("runs scheduled jobs through waitUntil", async () => {
const env = createTestEnv();
vi.stubGlobal("fetch", async (input: RequestInfo | URL) => {
Expand Down
45 changes: 44 additions & 1 deletion test/unit/selfhost-queue-common.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,36 @@ describe("self-host queue common helpers", () => {
expect(isGitHubBudgetBackgroundJob({ type: "agent-regate-sweep", requestedBy: "schedule" })).toBe(true);
expect(isGitHubBudgetBackgroundJob({ type: "backfill-repo-segment", requestedBy: "schedule", repoFullName: "owner/repo", segment: "open_pull_requests" })).toBe(true);
expect(isGitHubBudgetBackgroundJob({ type: "rag-index-repo", requestedBy: "schedule" })).toBe(true);
expect(isGitHubBudgetBackgroundJob({ type: "refresh-installation-health", requestedBy: "schedule" })).toBe(false);
});

it("REGRESSION (#4505): every maintenance job type confirmed to make real GitHub REST calls is GitHub-budget-gated", () => {
// refreshInstallationHealthRecords calls getAppInstallation (a direct REST call) per installation, plus
// resolveRepositorySettings per installed repo -- runs every 30 min, UNCONDITIONALLY, in every deployment.
expect(isGitHubBudgetBackgroundJob({ type: "refresh-installation-health", requestedBy: "schedule" })).toBe(true);
// fanOutBacklogConvergenceSweepJobs / sweepRepoBacklogConvergence call resolveRepositorySettings per repo.
expect(isGitHubBudgetBackgroundJob({ type: "backlog-convergence-sweep", requestedBy: "schedule" })).toBe(true);
// selfTuneRepos calls resolveRepositorySettings per registered repo.
expect(isGitHubBudgetBackgroundJob({ type: "selftune", requestedBy: "schedule" })).toBe(true);
// runReviewRecapJob calls loadRepoFocusManifest directly.
expect(isGitHubBudgetBackgroundJob({ type: "generate-review-recap", requestedBy: "schedule", repoFullName: "owner/repo" })).toBe(true);
// reconcile-open-prs: runOpenPrReconciliation makes large paginated GitHub REST calls per watched repo.
expect(isGitHubBudgetBackgroundJob({ type: "reconcile-open-prs", requestedBy: "schedule" })).toBe(true);
});

it("REGRESSION (#4505): maintenance job types confirmed to make NO GitHub REST calls stay OFF the GitHub budget (never wrongly gated)", () => {
// Verified local-only (D1 reads/writes, or dispatching an already-gated job type) during the #4505 audit --
// asserting these stay false catches a future accidental over-broad addition to GITHUB_BUDGET_BACKGROUND_TYPES,
// which would make a purely-internal job wait on a GitHub rate-limit budget it never draws from.
expect(isGitHubBudgetBackgroundJob({ type: "refresh-registry", requestedBy: "schedule" })).toBe(false);
expect(isGitHubBudgetBackgroundJob({ type: "refresh-scoring-model", requestedBy: "schedule" })).toBe(false);
expect(isGitHubBudgetBackgroundJob({ type: "repair-data-fidelity", requestedBy: "schedule" })).toBe(false);
expect(isGitHubBudgetBackgroundJob({ type: "rollup-product-usage", requestedBy: "schedule" })).toBe(false);
expect(isGitHubBudgetBackgroundJob({ type: "prune-retention", requestedBy: "schedule", dryRun: false })).toBe(false);
expect(isGitHubBudgetBackgroundJob({ type: "generate-weekly-value-report", requestedBy: "schedule" })).toBe(false);
expect(isGitHubBudgetBackgroundJob({ type: "generate-maintainer-recap", requestedBy: "schedule" })).toBe(false);
expect(isGitHubBudgetBackgroundJob({ type: "generate-signal-snapshots", requestedBy: "schedule" })).toBe(false);
expect(isGitHubBudgetBackgroundJob({ type: "ops-alerts", requestedBy: "schedule" })).toBe(false);
expect(isGitHubBudgetBackgroundJob({ type: "sweep-liveness-watchdog", requestedBy: "schedule" })).toBe(false);
});

describe("isScheduledRegateSweepJob", () => {
Expand Down Expand Up @@ -1011,6 +1040,20 @@ describe("self-host queue common helpers", () => {
).toBeNull();
});

it("REGRESSION (#4505): generate-review-recap coalesces per-repo instead of falling through to null (previously had no case at all)", () => {
expect(jobCoalesceKey(payload({ type: "generate-review-recap", requestedBy: "api", repoFullName: "JSONbored/Gittensory" }))).toBe("generate-review-recap:jsonbored/gittensory");
// A second trigger for the SAME repo produces the identical key, so pg-queue.ts's pending-only coalesce
// path merges it into the first instead of inserting a duplicate row.
expect(jobCoalesceKey(payload({ type: "generate-review-recap", requestedBy: "api", repoFullName: "jsonbored/gittensory" }))).toBe("generate-review-recap:jsonbored/gittensory");
// A DIFFERENT repo gets a distinct key -- never coalesced together.
expect(jobCoalesceKey(payload({ type: "generate-review-recap", requestedBy: "api", repoFullName: "owner/other-repo" }))).toBe("generate-review-recap:owner/other-repo");
});

it("REGRESSION (#4505): refresh-installation-health and selftune coalesce to a single global slot (unaffected by the GitHub-budget fix)", () => {
expect(jobCoalesceKey(payload({ type: "refresh-installation-health", requestedBy: "schedule" }))).toBe("refresh-installation-health");
expect(jobCoalesceKey(payload({ type: "selftune", requestedBy: "schedule" }))).toBe("selftune");
});

it("orders per-PR re-gate jobs by GitHub PR creation time, with a deterministic legacy fallback", () => {
expect(
jobClaimSortKey(
Expand Down
Loading