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
13 changes: 9 additions & 4 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ import {
resolveAgentPermissionReadiness,
} from "../settings/agent-execution";
import {
ISSUE_WAKE_MAX_PRS,
SWEEP_FANOUT_DEDUP_MS,
SWEEP_MAX_PRS,
isRegateSweepDraining,
Expand Down Expand Up @@ -4419,12 +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 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 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, SWEEP_MAX_PRS)
.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;
Expand Down
8 changes: 8 additions & 0 deletions src/settings/agent-sweep.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
57 changes: 52 additions & 5 deletions test/unit/queue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -3230,13 +3230,60 @@ 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 (#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 () => {
Expand Down