From 0bf8c8401e964f2f8ff44329fca6d463c3832b03 Mon Sep 17 00:00:00 2001 From: ghost <49853598+JSONbored@users.noreply.github.com> Date: Tue, 7 Jul 2026 02:13:48 -0700 Subject: [PATCH 1/2] fix(queue): queue all issue-linked PR regates --- src/queue/processors.ts | 7 +++---- test/unit/queue.test.ts | 14 ++++++++++---- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 026ac22780..0ac3177ad6 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -4419,12 +4419,11 @@ async function maybeReReviewOnLinkedIssueChange( if (isConvergenceRepoAllowed(env, repoFullName)) { const openPullRequests = await listOpenPullRequests(env, repoFullName); // Issue-side label/assignment changes can flip linked-issue hard-rule verdicts from mergeable to close. - // Wake a bounded prompt batch: unbounded issue-side fan-out can turn one webhook into thousands of - // foreground re-gates, exhausting queue, REST, and AI-review budgets. The regular stale sweep continues to - // converge any tail while preserving the same SWEEP_MAX_PRS source budget used by rate-aware fan-out. + // Wake every affected PR promptly: the issue-side signal can invalidate public gate state, so dropping a + // tail here would leave stale passing checks until the regular sweep eventually happens to reach it. Keep + // the actual re-gates asynchronous and staggered so the webhook does not perform expensive live reviews. const linkingPrs = openPullRequests .filter((pr) => pr.linkedIssues.includes(issueNumber)) - .slice(0, SWEEP_MAX_PRS) .map((pr) => ({ number: pr.number, createdAt: pr.createdAt ?? null })); for (const [index, pr] of linkingPrs.entries()) { const prNumber = pr.number; diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index f0b76fb71d..0004d73bd9 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -3193,7 +3193,7 @@ describe("queue processors", () => { ]); }); - it("REGRESSION: issue-side linked PR wake keeps fan-out bounded when many PRs link the same issue", async () => { + it("REGRESSION: issue-side linked PR wake queues every linked PR when many PRs link the same issue", async () => { const sent: Array<{ message: import("../../src/types").JobMessage; options?: QueueSendOptions }> = []; const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), @@ -3230,13 +3230,19 @@ describe("queue processors", () => { }); expect(fetchCount).toBe(0); - expect(sent).toHaveLength(SWEEP_MAX_PRS); + expect(sent).toHaveLength(SWEEP_MAX_PRS + 2); expect(sent.map(({ message }) => message)).toEqual( - Array.from({ length: SWEEP_MAX_PRS }, (_, index) => + Array.from({ length: SWEEP_MAX_PRS + 2 }, (_, index) => expect.objectContaining({ type: "agent-regate-pr", repoFullName: "owner/agent-repo", prNumber: index + 1, installationId: 9001 }), ), ); - expect(sent.map(({ options }) => options)).toEqual([undefined, { delaySeconds: 10 }, { delaySeconds: 20 }]); + expect(sent.map(({ options }) => options)).toEqual([ + undefined, + { delaySeconds: 10 }, + { delaySeconds: 20 }, + { delaySeconds: 30 }, + { delaySeconds: 40 }, + ]); }); it("REGRESSION (#2371): a coalesced issue-side signal schedules a trailing re-review so an add-then-remove sequence is never lost", async () => { From 56a94bf1b0ad6aef6f285cef12ca57ba04a54f9a Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 7 Jul 2026 05:36:03 -0700 Subject: [PATCH 2/2] fix(queue): bound issue-side linked-PR wake with a dedicated budget Removing the SWEEP_MAX_PRS cap entirely reintroduced the unbounded REST-fan-out risk that constant exists to prevent (a popular/tracking issue linked from hundreds of PRs would enqueue that many staggered ~9-REST-GET re-gates from one webhook). Introduces ISSUE_WAKE_MAX_PRS: a separate, larger one-shot budget for this handler, since it fires once per issue event rather than recurring every ~2 minutes like the periodic sweep SWEEP_MAX_PRS is sized for. --- src/queue/processors.ts | 12 ++++++++--- src/settings/agent-sweep.ts | 8 +++++++ test/unit/queue.test.ts | 43 ++++++++++++++++++++++++++++++++++++- 3 files changed, 59 insertions(+), 4 deletions(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 0ac3177ad6..c917913cc1 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -257,6 +257,7 @@ import { resolveAgentPermissionReadiness, } from "../settings/agent-execution"; import { + ISSUE_WAKE_MAX_PRS, SWEEP_FANOUT_DEDUP_MS, SWEEP_MAX_PRS, isRegateSweepDraining, @@ -4419,11 +4420,16 @@ async function maybeReReviewOnLinkedIssueChange( if (isConvergenceRepoAllowed(env, repoFullName)) { const openPullRequests = await listOpenPullRequests(env, repoFullName); // Issue-side label/assignment changes can flip linked-issue hard-rule verdicts from mergeable to close. - // Wake every affected PR promptly: the issue-side signal can invalidate public gate state, so dropping a - // tail here would leave stale passing checks until the regular sweep eventually happens to reach it. Keep - // the actual re-gates asynchronous and staggered so the webhook does not perform expensive live reviews. + // Wake affected PRs promptly: the issue-side signal can invalidate public gate state, so dropping every + // linked PR past a tiny cap would leave stale passing checks until the regular sweep eventually reaches + // it. But this must still be BOUNDED -- a popular/tracking issue linked from hundreds of PRs cannot be + // allowed to enqueue hundreds of ~9-REST-GET re-gates from one webhook, which is exactly the budget + // exhaustion SWEEP_MAX_PRS exists to prevent for the periodic sweep. ISSUE_WAKE_MAX_PRS is a separate, + // larger one-shot budget (see its own comment) since this handler fires once per event, not every ~2 min. + // Keep the actual re-gates asynchronous and staggered so the webhook does not perform expensive live reviews. const linkingPrs = openPullRequests .filter((pr) => pr.linkedIssues.includes(issueNumber)) + .slice(0, ISSUE_WAKE_MAX_PRS) .map((pr) => ({ number: pr.number, createdAt: pr.createdAt ?? null })); for (const [index, pr] of linkingPrs.entries()) { const prNumber = pr.number; diff --git a/src/settings/agent-sweep.ts b/src/settings/agent-sweep.ts index e71ffede59..4d2e701330 100644 --- a/src/settings/agent-sweep.ts +++ b/src/settings/agent-sweep.ts @@ -18,6 +18,14 @@ import type { PullRequestRecord } from "../types"; // to `3 × 3 × 9 × 30 ≈ 2.4k/hr`, leaving budget for live webhooks, cache misses, and branch-protection reads. export const SWEEP_MAX_PRS = 3; +// Issue-side wake budget (#3989 review): SWEEP_MAX_PRS (3) is sized for a sweep that RECURS every ~2 minutes, +// so its ceiling has to survive being multiplied by ~30 ticks/hr. This handler instead fires ONCE per issue +// label/assignment webhook, so a larger one-shot source budget is safe -- but it still must be bounded, or a +// popular/tracking issue linked from hundreds of PRs would enqueue hundreds of ~9-REST-GET re-gates from a +// single event. 25 reuses this file's own prior sweep ceiling (see SWEEP_MAX_PRS comment) as a one-shot budget: +// worst case ~25 x 9 = 225 REST calls, staggered by the same delaySeconds window the caller already uses. +export const ISSUE_WAKE_MAX_PRS = 25; + // Skip-if-fresh window: a PR touched within this span was almost certainly just gated by its webhook, so the // sweep leaves it alone for that brief moment to avoid racing the in-flight webhook review. Kept SHORT (2 min) // because the sweep is now LIGHT (re-gate + act, no AI) and runs every ~2 min — a just-approved PR must be diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 0004d73bd9..afae8856d0 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -66,7 +66,7 @@ import { fetchPullRequestFreshness, } from "../../src/github/pr-freshness"; import { createTestEnv } from "../helpers/d1"; -import { SWEEP_MAX_PRS } from "../../src/settings/agent-sweep"; +import { ISSUE_WAKE_MAX_PRS, SWEEP_MAX_PRS } from "../../src/settings/agent-sweep"; import { AGENT_LABEL_PENDING_CLOSURE, DEFAULT_LINKED_ISSUE_HARD_RULES } from "../../src/review/linked-issue-hard-rules"; vi.mock("../../src/github/pr-freshness", async (importOriginal) => { @@ -3245,6 +3245,47 @@ describe("queue processors", () => { ]); }); + it("REGRESSION (#3989 review): issue-side linked PR wake stays bounded by ISSUE_WAKE_MAX_PRS when a popular issue links far more PRs", async () => { + const sent: Array<{ message: import("../../src/types").JobMessage; options?: QueueSendOptions }> = []; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + GITTENSORY_REVIEW_REPOS: "owner/agent-repo", + JOBS: { + async send(message: import("../../src/types").JobMessage, options?: QueueSendOptions) { + sent.push(options ? { message, options } : { message }); + }, + } as unknown as Queue, + }); + await upsertInstallation(env, { action: "created", installation: { id: 9001, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: {}, events: [] } }); + await upsertRepositoryFromGitHub(env, { name: "agent-repo", full_name: "owner/agent-repo", private: false, owner: { login: "owner" } }, 9001); + await upsertRepositorySettings(env, { repoFullName: "owner/agent-repo", autonomy: { merge: "auto" }, aiReviewMode: "off", gatePack: "oss-anti-slop", gateCheckMode: "enabled", checkRunMode: "off", commentMode: "off", publicSurface: "off" }); + for (let number = 1; number <= ISSUE_WAKE_MAX_PRS + 2; number += 1) { + await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number, title: `Linking PR ${number}`, state: "open", user: { login: "contributor" }, head: { sha: `a${number}` }, labels: [], body: "Closes #1" }); + } + vi.stubGlobal("fetch", async () => Response.json({})); + + await processJob(env, { + type: "github-webhook", + deliveryId: "issue-label-popular-issue-fanout", + eventName: "issues", + payload: { + action: "labeled", + repository: { name: "agent-repo", full_name: "owner/agent-repo", private: false, owner: { login: "owner" } }, + installation: { id: 9001 }, + issue: { number: 1, title: "Issue", state: "open", labels: [{ name: "maintainer-only" }] }, + label: { name: "maintainer-only" }, + } as never, + }); + + // ISSUE_WAKE_MAX_PRS + 2 PRs link the issue, but only the first ISSUE_WAKE_MAX_PRS are enqueued -- a + // popular/tracking issue must not be able to enqueue an unbounded number of ~9-REST-GET re-gates from a + // single webhook, even though this one-shot handler's budget is intentionally larger than SWEEP_MAX_PRS. + expect(sent).toHaveLength(ISSUE_WAKE_MAX_PRS); + expect(sent.map(({ message }) => (message as { prNumber: number }).prNumber)).toEqual( + Array.from({ length: ISSUE_WAKE_MAX_PRS }, (_, index) => index + 1), + ); + }); + it("REGRESSION (#2371): a coalesced issue-side signal schedules a trailing re-review so an add-then-remove sequence is never lost", async () => { // Unlike CI-completion events, same-PR issue-side events are NOT interchangeable within the window: a // label ADD immediately followed by a REMOVE carries genuinely different states. The first event's