diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 19c6af4d47..1a8e7df918 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -2370,6 +2370,70 @@ async function ciReReviewCoalesced( ); } +// Issue-side wake coalescing (#2371): a DEDICATED key namespace, distinct from ciReReviewCoalesced's +// `ci-coalesce:` window. The two triggers are semantically different — CI-completion webhooks for the same run +// are interchangeable (whichever wins the race re-fetches the SAME already-settled CI state), but an issue-side +// label/assignment change is not: a completely unrelated CI re-review claiming the shared window would silently +// suppress a genuinely different issue-side signal, leaving the PR on stale linked-issue state until the window +// expires or the sweep eventually reaches it. Reusing ciReReviewCoalesced's key made that cross-domain collision +// possible; a separate namespace confines coalescing to a burst of same-PR issue-side events. +async function issueLinkedPrReReviewCoalesced( + env: Env, + repoFullName: string, + prNumber: number, +): Promise { + return ciCompletionCoalesced( + env, + `issue-link-coalesce:${repoFullName.toLowerCase()}#${prNumber}`, + ); +} + +// Unlike CI-completion events, same-PR issue-side events are NOT interchangeable within the coalesce window: an +// add-then-remove label or assign-then-unassign sequence carries genuinely DIFFERENT states, so silently dropping +// every event after the first (as ciCompletionCoalesced's plain throttle does) can leave the PR on a stale +// verdict for up to the window's length. Schedule exactly ONE trailing agent-regate-pr re-review to run just +// after the window closes, guaranteeing the LATEST state is always eventually captured — deduped (its own +// window, same TTL) so a burst of N coalesced events schedules ONE trailing job, not N. Reuses the existing +// agent-regate-pr sweep-unit job (already rate-limit-aware and retried), not a new job type (#2371). +async function scheduleTrailingIssueLinkedReReview( + env: Env, + deliveryId: string, + installationId: number, + repoFullName: string, + prNumber: number, +): Promise { + const key = `issue-link-trailing:${repoFullName.toLowerCase()}#${prNumber}`; + // Check-then-claim, but the CLAIM only happens after the send actually succeeds (#2371 follow-up): claiming + // eagerly (as ciCompletionCoalesced's own combined check-and-set does) would record "a trailing re-review is + // scheduled" even when the enqueue itself throws, permanently swallowing the guarantee this function exists to + // provide for the rest of the window — a later coalesced event would see the marker held and skip retrying, + // even though nothing was actually queued. + if (await getTransientKey(env, key)) return; + try { + await env.JOBS.send( + { + type: "agent-regate-pr", + deliveryId, + repoFullName, + prNumber, + installationId, + }, + { delaySeconds: CI_COALESCE_WINDOW_SECONDS }, + ); + } catch (error) { + console.log( + JSON.stringify({ + ev: "issue_link_trailing_enqueue_failed", + repoFullName, + pull: prNumber, + message: errorMessage(error).slice(0, 120), + }), + ); + return; // do NOT claim — a later coalesced event in this window should retry the enqueue + } + await putTransientKey(env, key, "1", CI_COALESCE_WINDOW_SECONDS); +} + async function ciHeadShaResolutionCoalesced( env: Env, repoFullName: string, @@ -2608,6 +2672,73 @@ async function maybeReReviewOnCiCompletion( return true; } +/** + * Wake linked PRs on an issue-side signal (#2259). Labeling/unlabeling (e.g. maintainer-only) or + * assigning/unassigning on a linked ISSUE can flip a linked-issue hard-rule verdict, but that only gets + * re-evaluated when the PR ITSELF receives a webhook or the staleness-ordered sweep eventually reaches it — + * which can lag for many cycles on a repo with more than a few open PRs. Re-review every OPEN PR that links + * this issue promptly instead of waiting. Uses its OWN coalesce window (issueLinkedPrReReviewCoalesced, + * DISTINCT from CI-completion's — #2371): the two triggers are not interchangeable, so a shared window let an + * unrelated CI re-review silently suppress a genuinely different issue-side signal. Within the issue-side + * window itself, same-PR events are ALSO not interchangeable (an add-then-remove or assign-then-unassign + * sequence carries genuinely different states), so a coalesced event schedules a trailing re-review + * (scheduleTrailingIssueLinkedReReview) instead of silently dropping the state it represents. + */ +async function maybeReReviewOnLinkedIssueChange( + env: Env, + deliveryId: string, + eventName: string, + payload: GitHubWebhookPayload, +): Promise { + if (eventName !== "issues") return false; + if ( + payload.action !== "labeled" && + payload.action !== "unlabeled" && + payload.action !== "assigned" && + payload.action !== "unassigned" + ) + return false; + const repoFullName = payload.repository?.full_name; + const installationId = getInstallationId(payload); + const issueNumber = payload.issue?.number; + if (!repoFullName || !installationId || !issueNumber) return false; + if (isConvergenceRepoAllowed(env, repoFullName)) { + const openPullRequests = await listOpenPullRequests(env, repoFullName); + const linkingPrNumbers = openPullRequests + .filter((pr) => pr.linkedIssues.includes(issueNumber)) + .map((pr) => pr.number); + for (const prNumber of linkingPrNumbers) { + if (await issueLinkedPrReReviewCoalesced(env, repoFullName, prNumber)) { + await scheduleTrailingIssueLinkedReReview( + env, + deliveryId, + installationId, + repoFullName, + prNumber, + ); + continue; + } + await reReviewStoredPullRequest( + env, + deliveryId, + installationId, + repoFullName, + prNumber, + ); + } + } + await recordWebhookEvent(env, { + deliveryId, + eventName, + action: payload.action, + installationId, + repositoryFullName: repoFullName, + payloadHash: "processed", + status: "processed", + }); + return true; +} + /** * deployment_status (success/failure) → re-review the associated PR so the before/after visual capture fills the * "after" cell once the preview deploy finishes (or flips to a deploy-failed note). Mirrors reviewbot's @@ -3494,6 +3625,13 @@ async function processGitHubWebhook( await maybeCaptureOnDeploymentStatus(env, deliveryId, eventName, payload) ) return; + // Linked-issue label/assignment change (#2259) — an `issues` event carries no `payload.pull_request` either, + // so it must be handled here alongside the other non-PR wake triggers: it re-reviews every open PR that + // links this issue promptly, instead of waiting for a PR-side webhook or the staleness-ordered sweep. + if ( + await maybeReReviewOnLinkedIssueChange(env, deliveryId, eventName, payload) + ) + return; if (payload.repository?.full_name && payload.pull_request) { const repoFullName = payload.repository.full_name; diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 17df8c7e08..45afa249e4 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -1541,6 +1541,377 @@ describe("queue processors", () => { }); }); + it("issue label change wakes the linked PR's hard-rule re-evaluation promptly (#2259)", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), GITTENSORY_REVIEW_REPOS: "owner/agent-repo" }); + 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" }); + // Links issue #1 — the issue the "labeled" event below fires on. + await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 7, title: "Linking PR", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, labels: [], body: "Closes #1" }); + let checkRunsFetched = false; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.endsWith("/pulls/7")) return Response.json({ number: 7, title: "Linking PR", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, labels: [], body: "Closes #1" }); + if (url.includes("/commits/a7/check-runs")) { checkRunsFetched = true; return Response.json({ total_count: 0, check_runs: [] }); } + if (url.includes("/commits/a7/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/1")) return Response.json({ number: 1, title: "Issue", state: "open", labels: [{ name: "maintainer-only" }], user: { login: "owner" } }); + if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } }); + return Response.json({}); + }); + + await processJob(env, { + type: "github-webhook", + deliveryId: "issue-label-wake", + 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, + }); + + // The linked PR was re-reviewed promptly off the issue-side signal, not left for the next PR-side webhook or + // the staleness-ordered sweep. + expect(checkRunsFetched).toBe(true); + const webhookRow = await env.DB.prepare("select status from webhook_events where delivery_id = ?").bind("issue-label-wake").first<{ status: string }>(); + expect(webhookRow?.status).toBe("processed"); + }); + + it("issue label change does NOT wake an open PR that links a DIFFERENT issue", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), GITTENSORY_REVIEW_REPOS: "owner/agent-repo" }); + 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" }); + // Links issue #99 — the "labeled" event below fires on issue #1, which this PR does NOT link. + await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 7, title: "Unrelated PR", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, labels: [], body: "Closes #99" }); + let checkRunsFetched = false; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + checkRunsFetched ||= input.toString().includes("/commits/a7/check-runs"); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + return Response.json({}); + }); + + await processJob(env, { + type: "github-webhook", + deliveryId: "issue-label-no-link", + 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, + }); + + expect(checkRunsFetched).toBe(false); // PR #7 links #99, not #1 — never re-reviewed + }); + + it("issue label change is dormant on a repo outside the GITTENSORY_REVIEW_REPOS convergence allowlist", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), GITTENSORY_REVIEW_REPOS: "" }); + 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" }); + await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 7, title: "Linking PR", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, labels: [], body: "Closes #1" }); + let checkRunsFetched = false; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + checkRunsFetched ||= url.includes("/commits/a7/check-runs"); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + return Response.json({}); + }); + + await processJob(env, { + type: "github-webhook", + deliveryId: "issue-label-not-converged", + 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, + }); + + expect(checkRunsFetched).toBe(false); // dormant default: not in the convergence allowlist + const webhookRow = await env.DB.prepare("select status from webhook_events where delivery_id = ?").bind("issue-label-not-converged").first<{ status: string }>(); + expect(webhookRow?.status).toBe("processed"); // still marked handled — only the re-review work is skipped + }); + + it("issue label change no-ops on a malformed payload missing the issue number", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), GITTENSORY_REVIEW_REPOS: "owner/agent-repo" }); + let fetchCount = 0; + vi.stubGlobal("fetch", async () => { + fetchCount += 1; + return Response.json({}); + }); + + await processJob(env, { + type: "github-webhook", + deliveryId: "issue-label-no-issue-number", + eventName: "issues", + payload: { + action: "labeled", + repository: { name: "agent-repo", full_name: "owner/agent-repo", private: false, owner: { login: "owner" } }, + installation: { id: 9001 }, + label: { name: "maintainer-only" }, + // No `issue` field at all — GitHub always sends one, but the handler must not assume it. + } as never, + }); + + expect(fetchCount).toBe(0); // never even minted a token — bailed before touching GitHub + }); + + it("REGRESSION (#2371): an unrelated CI-completion coalesce claim does NOT suppress the issue-side wake for the same PR", async () => { + // The two triggers are not interchangeable: a CI-completion webhook re-review and an issue-side + // label/assignment re-review answer different questions. Sharing one coalesce window let a completely + // unrelated CI re-review silently swallow a genuine issue-side signal, leaving the PR on stale linked-issue + // state until the window expired or the sweep eventually reached it. The issue-side wake must use its OWN + // window and proceed regardless of what the CI-completion window holds. + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), GITTENSORY_REVIEW_REPOS: "owner/agent-repo" }); + 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" }); + await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 7, title: "Linking PR", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, labels: [], body: "Closes #1" }); + // A CI completion for this exact PR claimed the CI-completion window moments earlier — a wholly separate + // trigger from the issue-side label change below. + await env.SELFHOST_TRANSIENT_CACHE?.set("ci-coalesce:owner/agent-repo#7", "1", 60); + let checkRunsFetched = false; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.endsWith("/pulls/7")) return Response.json({ number: 7, title: "Linking PR", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, labels: [], body: "Closes #1" }); + if (url.includes("/commits/a7/check-runs")) { checkRunsFetched = true; return Response.json({ total_count: 0, check_runs: [] }); } + if (url.includes("/commits/a7/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/1")) return Response.json({ number: 1, title: "Issue", state: "open", labels: [{ name: "maintainer-only" }], user: { login: "owner" } }); + if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } }); + return Response.json({}); + }); + + await processJob(env, { + type: "github-webhook", + deliveryId: "issue-label-not-suppressed-by-ci-coalesce", + 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, + }); + + expect(checkRunsFetched).toBe(true); // the CI window's claim is irrelevant to the issue-side wake + }); + + it("issue label change coalesces a burst of same-PR issue-side signals within its OWN window (#2371)", async () => { + // The issue-side window's legitimate purpose: bound FREQUENCY for a burst of label/assignment churn on the + // same PR, without depending on (or being defeated by) the unrelated CI-completion window. + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), GITTENSORY_REVIEW_REPOS: "owner/agent-repo" }); + 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" }); + await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 7, title: "Linking PR", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, labels: [], body: "Closes #1" }); + let fetchCallCount = 0; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + fetchCallCount += 1; + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.endsWith("/pulls/7")) return Response.json({ number: 7, title: "Linking PR", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, labels: [], body: "Closes #1" }); + if (url.includes("/commits/a7/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/a7/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/1")) return Response.json({ number: 1, title: "Issue", state: "open", labels: [{ name: "maintainer-only" }] }); + if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } }); + return Response.json({}); + }); + const labeled = (deliveryId: string) => ({ + type: "github-webhook" as const, + deliveryId, + 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, + }); + + await processJob(env, labeled("issue-label-burst-1")); + expect(fetchCallCount).toBeGreaterThan(0); // sanity: the first signal genuinely re-reviewed + const fetchCallCountAfterFirst = fetchCallCount; + + await processJob(env, labeled("issue-label-burst-2")); + + // Second signal within the window coalesces — no additional GitHub interaction at all, not even a token mint. + expect(fetchCallCount).toBe(fetchCallCountAfterFirst); + }); + + 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 + // re-review captures the ADD; the second is coalesced (per the window's frequency bound) but must not + // silently drop the REMOVE — it schedules a trailing agent-regate-pr re-review to run just after the + // window closes, so the PR converges on the LATEST (removed) state instead of staying stuck on the ADD. + 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" }); + await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 7, title: "Linking PR", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, labels: [], body: "Closes #1" }); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.endsWith("/pulls/7")) return Response.json({ number: 7, title: "Linking PR", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, labels: [], body: "Closes #1" }); + if (url.includes("/commits/a7/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/a7/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/1")) return Response.json({ number: 1, title: "Issue", state: "open", labels: [] }); + if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } }); + return Response.json({}); + }); + const event = (deliveryId: string, action: "labeled" | "unlabeled") => ({ + type: "github-webhook" as const, + deliveryId, + eventName: "issues", + payload: { + action, + 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: action === "labeled" ? [{ name: "maintainer-only" }] : [] }, + label: { name: "maintainer-only" }, + } as never, + }); + + await processJob(env, event("issue-add-then-remove-1", "labeled")); + expect(sent).toEqual([]); // the FIRST event re-reviews live — no trailing job needed yet + + await processJob(env, event("issue-add-then-remove-2", "unlabeled")); + // The REMOVE was coalesced (same window), so it must schedule exactly one trailing re-review for the PR, + // delayed past the window's close, rather than being silently dropped. + expect(sent).toEqual([ + { + message: expect.objectContaining({ type: "agent-regate-pr", repoFullName: "owner/agent-repo", prNumber: 7, installationId: 9001 }), + options: { delaySeconds: 60 }, + }, + ]); + + await processJob(env, event("issue-add-then-remove-3", "labeled")); + // A THIRD coalesced event in the same window must not schedule a second, redundant trailing job. + expect(sent).toHaveLength(1); + }); + + it("a failed trailing-re-review enqueue is swallowed — best-effort, the sweep remains the ultimate backstop (#2371)", async () => { + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + GITTENSORY_REVIEW_REPOS: "owner/agent-repo", + JOBS: { async send() { throw new Error("queue unavailable"); } } 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" }); + await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 7, title: "Linking PR", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, labels: [], body: "Closes #1" }); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.endsWith("/pulls/7")) return Response.json({ number: 7, title: "Linking PR", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, labels: [], body: "Closes #1" }); + if (url.includes("/commits/a7/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/a7/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/1")) return Response.json({ number: 1, title: "Issue", state: "open", labels: [{ name: "maintainer-only" }] }); + if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } }); + return Response.json({}); + }); + const labeled = (deliveryId: string) => ({ + type: "github-webhook" as const, + deliveryId, + 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, + }); + + await expect(processJob(env, labeled("issue-enqueue-fail-1"))).resolves.toBeUndefined(); // live re-review, no enqueue on this path + // The second (coalesced) event exercises scheduleTrailingIssueLinkedReReview's env.JOBS.send — its failure + // must be swallowed, not thrown into the webhook handler. + await expect(processJob(env, labeled("issue-enqueue-fail-2"))).resolves.toBeUndefined(); + }); + + it("REGRESSION: a TRANSIENT trailing-re-review enqueue failure does not permanently forfeit the trailing job — the next coalesced event retries", async () => { + // The dedupe marker must be claimed only AFTER env.JOBS.send actually succeeds. Claiming it eagerly (before + // the send settles) would let a transient queue failure permanently swallow the guarantee: every later + // coalesced event in the SAME window would see the marker already held and skip retrying, even though + // nothing was ever actually queued. + const sent: Array<{ message: import("../../src/types").JobMessage; options?: QueueSendOptions }> = []; + let sendAttempts = 0; + 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) { + sendAttempts += 1; + if (sendAttempts === 1) throw new Error("queue transiently unavailable"); + 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" }); + await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 7, title: "Linking PR", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, labels: [], body: "Closes #1" }); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.endsWith("/pulls/7")) return Response.json({ number: 7, title: "Linking PR", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, labels: [], body: "Closes #1" }); + if (url.includes("/commits/a7/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/a7/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/1")) return Response.json({ number: 1, title: "Issue", state: "open", labels: [{ name: "maintainer-only" }] }); + if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } }); + return Response.json({}); + }); + const labeled = (deliveryId: string) => ({ + type: "github-webhook" as const, + deliveryId, + 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, + }); + + await processJob(env, labeled("issue-transient-retry-1")); // live re-review, no enqueue + await processJob(env, labeled("issue-transient-retry-2")); // coalesced — the FIRST send attempt, throws + expect(sendAttempts).toBe(1); + expect(sent).toEqual([]); // the failed attempt must NOT have claimed the marker + + await processJob(env, labeled("issue-transient-retry-3")); // still coalesced — retries the enqueue, succeeds + expect(sendAttempts).toBe(2); + expect(sent).toEqual([ + { message: expect.objectContaining({ type: "agent-regate-pr", repoFullName: "owner/agent-repo", prNumber: 7 }), options: { delaySeconds: 60 } }, + ]); + + await processJob(env, labeled("issue-transient-retry-4")); // coalesced again — the successful claim now dedupes further retries + expect(sendAttempts).toBe(2); + }); + it("#4 stale-surface repair: a rebased PR resyncs + re-reviews at the new head, and the marker survives the resync", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); await upsertInstallation(env, { action: "created", installation: { id: 9001, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: {}, events: [] } });