diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 1d019a26a..43cb99447 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -3787,7 +3787,21 @@ async function wakeOverCapSiblingPullRequests( ): Promise { await Promise.all( siblingPrNumbers.map(async (prNumber) => { - const key = `contributor-cap-wake:${repoFullName.toLowerCase()}#${prNumber}`; + // #5385-sentry (GITTENSORY-1D): keyed by the sibling's CURRENT head SHA (not just its number) and held for + // a much longer cooldown than the 60s CI-completion-burst window CI_COALESCE_WINDOW_SECONDS was designed + // for -- a repeat over-cap discovery for an UNCHANGED head within the cooldown has nothing new to learn + // from a fresh full re-review; a new commit changes the head SHA and naturally resets the guard, mirroring + // the identical "a new commit resets the CI-stuck-finalize guard" idiom used above (#orb-ci-stuck-repeat). + // Without this, a contributor who repeatedly opens (and has auto-closed) near-duplicate PRs re-triggers + // THIS sibling's full review/gate republish every time the cap is recomputed, even though nothing about + // the sibling itself changed -- confirmed live: one PR published 27 redundant review surfaces in 2 hours + // this way, driven entirely by a different PR's open/close churn. Fail open to the tighter, headSha-less + // key on a lookup miss (PR not found locally) rather than skip the wake outright. + const sibling = await getPullRequest(env, repoFullName, prNumber).catch(() => null); + const key = sibling?.headSha + ? `contributor-cap-wake:${repoFullName.toLowerCase()}#${prNumber}#${sibling.headSha}` + : `contributor-cap-wake:${repoFullName.toLowerCase()}#${prNumber}`; + const cooldownSeconds = sibling?.headSha ? Math.round(AI_REVIEW_NON_CACHEABLE_RETRY_COOLDOWN_MS / 1000) : CI_COALESCE_WINDOW_SECONDS; if (await getTransientKey(env, key)) return; try { await env.JOBS.send({ @@ -3808,7 +3822,7 @@ async function wakeOverCapSiblingPullRequests( ); return; // do NOT claim — a later discovery should retry the enqueue } - await putTransientKey(env, key, "1", CI_COALESCE_WINDOW_SECONDS); + await putTransientKey(env, key, "1", cooldownSeconds); }), ); } diff --git a/test/unit/queue-3.test.ts b/test/unit/queue-3.test.ts index 1436bbc9c..9c05798a2 100644 --- a/test/unit/queue-3.test.ts +++ b/test/unit/queue-3.test.ts @@ -4310,7 +4310,9 @@ describe("queue processors", () => { }); // Pre-seed the coalescing key for PR56 exactly as wakeOverCapSiblingPullRequests itself would after a // first, already-successful enqueue — proving the SECOND discovery within the window skips re-enqueueing. - await env.SELFHOST_TRANSIENT_CACHE?.set("contributor-cap-wake:jsonbored/gittensory#56", "1", 60); + // #5385-sentry (GITTENSORY-1D): keyed by PR56's own head SHA ("f56", upserted below) since the sibling + // wake now resolves and includes it, not just the bare PR number. + await env.SELFHOST_TRANSIENT_CACHE?.set("contributor-cap-wake:jsonbored/gittensory#56#f56", "1", 60); await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 54, title: "Farmer PR zero", state: "open", user: { login: "farmer99" }, head: { sha: "f54" }, labels: [], body: "w" }); await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 56, title: "Farmer PR two (already over cap)", state: "open", user: { login: "farmer99" }, head: { sha: "f56" }, labels: [], body: "y" }); await upsertRepositorySettings(env, { @@ -4413,8 +4415,159 @@ describe("queue processors", () => { }), ).resolves.not.toThrow(); - // The coalescing key was NOT claimed (enqueue failed), so a later discovery can still retry. - expect(await env.SELFHOST_TRANSIENT_CACHE?.get("contributor-cap-wake:jsonbored/gittensory#56")).toBeNull(); + // The coalescing key was NOT claimed (enqueue failed), so a later discovery can still retry. Keyed by + // PR56's own head SHA ("f56", upserted above) — see the coalescing test's identical note. + expect(await env.SELFHOST_TRANSIENT_CACHE?.get("contributor-cap-wake:jsonbored/gittensory#56#f56")).toBeNull(); + }); + + it("REGRESSION (#5385-sentry, GITTENSORY-1D): a repeat sibling-wake for the SAME unchanged head is still coalesced well past the old 60s window, but a genuinely NEW commit resets the guard", async () => { + // Confirmed live: a contributor repeatedly opening (and having auto-closed) near-duplicate PRs re-triggered + // the SAME sibling's full review/gate republish every time the cap was recomputed, even though nothing + // about the sibling itself ever changed -- one PR published 27 redundant review surfaces in 2 hours this + // way. The 60s CI-completion-burst coalesce window was never meant to (and didn't) protect against a + // trigger recurring every several minutes to hours. + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await upsertInstallation(env, { + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" }, target_type: "User", repository_selection: "all", permissions: { metadata: "read", pull_requests: "write", issues: "write" }, events: ["pull_request"] }, + repositories: [{ name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }], + }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 54, title: "Farmer PR zero", state: "open", user: { login: "farmer99" }, head: { sha: "f54" }, labels: [], body: "w" }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 56, title: "Farmer PR two (already over cap)", state: "open", user: { login: "farmer99" }, head: { sha: "f56" }, labels: [], body: "y" }); + await upsertRepositorySettings(env, { + repoFullName: "JSONbored/gittensory", + commentMode: "all_prs", + publicSurface: "comment_only", + checkRunMode: "off", + reviewCheckMode: "required", + aiReviewMode: "advisory", + autonomy: { close: "auto", label: "auto" }, + contributorOpenPrCap: 2, + }); + const fanned: import("../../src/types").JobMessage[] = []; + const realSend = env.JOBS.send.bind(env.JOBS); + env.JOBS.send = (async (message: import("../../src/types").JobMessage, options?: QueueSendOptions) => { + if (message.type === "agent-regate-pr") fanned.push(message); + return realSend(message, options); + }) as typeof env.JOBS.send; + let pr56Sha = "f56"; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url === "https://api.gittensor.io/miners") return Response.json([]); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/pulls/55/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+const ok = true;" }]); + if (url.includes("/pulls/55/reviews")) return Response.json([]); + if (url.includes("/pulls/55/commits")) return Response.json([]); + // Bare GET /pulls/{54,56} is the sibling live-open-state check the cap computation live-verifies before + // treating either as a real, currently-open sibling (#2270 busy-repo bypass fix) -- both must resolve + // "open" for PR56 to be correctly identified as the (numerically highest, over-cap) sibling. + if (url.endsWith("/pulls/54")) return Response.json({ number: 54, state: "open", user: { login: "farmer99" }, head: { sha: "f54" }, mergeable_state: "clean" }); + if (url.endsWith("/pulls/56")) return Response.json({ number: 56, state: "open", user: { login: "farmer99" }, head: { sha: pr56Sha }, mergeable_state: "clean" }); + if (url.endsWith("/pulls/55")) return Response.json({ number: 55, state: "open", user: { login: "farmer99" }, head: { sha: "f55" }, mergeable_state: "clean" }); + if (url.includes("/commits/f55/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/f55/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/55/labels")) return Response.json([]); + if (url.includes("/issues/55/comments")) return Response.json([]); + return Response.json({}); + }); + const rediscoverPr56AsOverCap = (deliveryId: string) => + processJob(env, { + type: "github-webhook", + deliveryId, + eventName: "pull_request", + payload: { + action: "opened", + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + pull_request: { number: 55, title: "Farmer PR one", state: "open", user: { login: "farmer99" }, head: { sha: "f55" }, labels: [], body: "x", mergeable_state: "clean", reviewDecision: "APPROVED" }, + }, + }); + + // First discovery: PR56 is over cap -- wakes it once. + await rediscoverPr56AsOverCap("wake-first"); + expect(fanned.filter((job) => job.type === "agent-regate-pr" && job.prNumber === 56)).toHaveLength(1); + + // 90 seconds later: past the OLD 60s coalesce window, but well within the new ~30-minute cooldown for an + // UNCHANGED head SHA. A second discovery of the SAME still-over-cap sibling must NOT re-wake it. + vi.setSystemTime(new Date("2026-05-28T00:01:30.000Z")); + await rediscoverPr56AsOverCap("wake-second-still-coalesced"); + expect(fanned.filter((job) => job.type === "agent-regate-pr" && job.prNumber === 56)).toHaveLength(1); // still just the one + + // PR56 gets a genuinely NEW commit -- the guard must reset for the new head SHA so the sibling still gets + // re-evaluated eventually, not stay silenced forever regardless of what actually changes about it. + pr56Sha = "f56-v2"; + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 56, title: "Farmer PR two (already over cap)", state: "open", user: { login: "farmer99" }, head: { sha: "f56-v2" }, labels: [], body: "y" }); + await rediscoverPr56AsOverCap("wake-third-after-new-commit"); + expect(fanned.filter((job) => job.type === "agent-regate-pr" && job.prNumber === 56)).toHaveLength(2); // woken again for the new head + }); + + it("REGRESSION (#5385-sentry, GITTENSORY-1D): a sibling-wake lookup miss fails OPEN to the bare (headSha-less) key and the shorter 60s window, instead of skipping the wake", async () => { + // wakeOverCapSiblingPullRequests re-resolves the sibling's OWN current head SHA via a fresh getPullRequest + // lookup (not the already-fetched author-scoped record) so the coalescing key reflects the truly-latest + // commit. Force JUST that lookup to miss (as it would for a PR not yet/no-longer tracked locally) while + // every other getPullRequest call in this same pass resolves normally -- proving the documented fail-open + // fallback (bare key, CI_COALESCE_WINDOW_SECONDS) actually fires instead of silently dropping the wake. + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await upsertInstallation(env, { + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" }, target_type: "User", repository_selection: "all", permissions: { metadata: "read", pull_requests: "write", issues: "write" }, events: ["pull_request"] }, + repositories: [{ name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }], + }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 54, title: "Farmer PR zero", state: "open", user: { login: "farmer99" }, head: { sha: "f54" }, labels: [], body: "w" }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 56, title: "Farmer PR two (already over cap)", state: "open", user: { login: "farmer99" }, head: { sha: "f56" }, labels: [], body: "y" }); + await upsertRepositorySettings(env, { + repoFullName: "JSONbored/gittensory", + commentMode: "all_prs", + publicSurface: "comment_only", + checkRunMode: "off", + reviewCheckMode: "required", + aiReviewMode: "advisory", + autonomy: { close: "auto", label: "auto" }, + contributorOpenPrCap: 2, + }); + const realGetPullRequest = repositoriesModule.getPullRequest; + vi.spyOn(repositoriesModule, "getPullRequest").mockImplementation(async (spyEnv, fullName, number) => { + if (number === 56) return null; // simulate PR56 not being found locally at wake time + return realGetPullRequest(spyEnv, fullName, number); + }); + const setSpy = vi.spyOn(env.SELFHOST_TRANSIENT_CACHE!, "set"); + const fanned: import("../../src/types").JobMessage[] = []; + const realSend = env.JOBS.send.bind(env.JOBS); + env.JOBS.send = (async (message: import("../../src/types").JobMessage, options?: QueueSendOptions) => { + if (message.type === "agent-regate-pr") fanned.push(message); + return realSend(message, options); + }) as typeof env.JOBS.send; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url === "https://api.gittensor.io/miners") return Response.json([]); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/pulls/55/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+const ok = true;" }]); + if (url.includes("/pulls/55/reviews")) return Response.json([]); + if (url.includes("/pulls/55/commits")) return Response.json([]); + if (url.endsWith("/pulls/54")) return Response.json({ number: 54, state: "open", user: { login: "farmer99" }, head: { sha: "f54" }, mergeable_state: "clean" }); + if (url.endsWith("/pulls/56")) return Response.json({ number: 56, state: "open", user: { login: "farmer99" }, head: { sha: "f56" }, mergeable_state: "clean" }); + if (url.endsWith("/pulls/55")) return Response.json({ number: 55, state: "open", user: { login: "farmer99" }, head: { sha: "f55" }, mergeable_state: "clean" }); + if (url.includes("/commits/f55/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/f55/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/55/labels")) return Response.json([]); + if (url.includes("/issues/55/comments")) return Response.json([]); + return Response.json({}); + }); + + await processJob(env, { + type: "github-webhook", + deliveryId: "sibling-lookup-miss", + eventName: "pull_request", + payload: { + action: "opened", + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + pull_request: { number: 55, title: "Farmer PR one", state: "open", user: { login: "farmer99" }, head: { sha: "f55" }, labels: [], body: "x", mergeable_state: "clean", reviewDecision: "APPROVED" }, + }, + }); + + expect(fanned.some((job) => job.type === "agent-regate-pr" && job.prNumber === 56)).toBe(true); // still woken despite the lookup miss + const call = setSpy.mock.calls.find(([key]) => (key as string).startsWith("contributor-cap-wake:jsonbored/gittensory#56")); + expect(call?.[0]).toBe("contributor-cap-wake:jsonbored/gittensory#56"); // bare key -- no headSha suffix + expect(call?.[2]).toBe(60); // CI_COALESCE_WINDOW_SECONDS, not the ~1800s headSha-keyed cooldown }); it("contributor open-ISSUE cap (#2270): a contributor's 3rd open issue (over a cap of 2) is labeled + closed deterministically", async () => {