From 2d9297f8c3f4befd52b07c7decca58d27e943e39 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 1 Jul 2026 06:32:39 -0700 Subject: [PATCH 1/4] feat(queue): wake linked PRs promptly on an issue-side label/assignment change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit processGitHubWebhook had no eventName === "issues" branch, so labeling/unlabeling maintainer-only (or another point-bearing label) on a linked ISSUE, or assigning/unassigning the repo owner on it, never re-triggered the linked-issue hard-rule re-evaluation for PRs that link it. That check only ran when the PR itself received a webhook, or via the staleness-ordered sweep, which caps at 3 PRs per repo per ~2-minute tick with no priority signal for "a linked issue just changed" — on a busy repo this can lag for many cycles, letting a should-now-be-closeable PR auto-merge, or a should-now-be-mergeable PR stay wrongly held. Add maybeReReviewOnLinkedIssueChange, mirroring the existing CI-completion re-review handler (maybeReReviewOnCiCompletion): on a labeled/unlabeled/assigned/unassigned issues event, find every OPEN PR that links the issue and re-review it promptly. Reuses the same per-PR coalesce window CI-completion re-review already uses, and the same GITTENSORY_REVIEW_REPOS convergence allowlist gate, so this activates on exactly the same repo footprint as the analogous existing mechanism. --- src/queue/processors.ts | 62 ++++++++++++++++ test/unit/queue.test.ts | 159 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 221 insertions(+) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 7f32dfc58b..4b66188830 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -2355,6 +2355,61 @@ 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. Reuses the CI-completion coalesce window (ciReReviewCoalesced): its + * purpose is identical here — bound re-review FREQUENCY per PR, never correctness, since the re-review always + * re-fetches live state regardless of what triggered it. + */ +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 ciReReviewCoalesced(env, 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 @@ -3241,6 +3296,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 ec69054c93..431a8937ac 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -1455,6 +1455,165 @@ 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("issue label change respects the CI-completion coalesce window shared with the linked PR", 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" }); + await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 7, title: "Linking PR", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, labels: [], body: "Closes #1" }); + // PR #7 was already re-reviewed within the window (e.g. by a concurrent CI completion) — this issue-side + // signal must not double-fire a redundant re-review for the same PR within it. + 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(); + 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-coalesced", + 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); // coalesced — no redundant re-review within the window + }); + 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: [] } }); From 506c5509e4f2cd979c7c36b408c13c847c181b73 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 1 Jul 2026 14:01:40 -0700 Subject: [PATCH 2/4] fix(queue): give the issue-side wake its own coalesce window maybeReReviewOnLinkedIssueChange reused ciReReviewCoalesced's shared `ci-coalesce:{repo}#{pr}` key, but a CI-completion webhook and an issue-side label/assignment change answer different questions -- unlike concurrent CI-completion events (interchangeable: whichever wins the coalesce race re-fetches the same already-settled CI state), a completely unrelated CI re-review claiming the shared window could silently suppress a genuine issue-side signal, leaving the linked-issue verdict stale until the window expired or the sweep eventually reached the PR. Add a dedicated issueLinkedPrReReviewCoalesced using its own `issue-link-coalesce:` key namespace. Within that namespace the window still bounds frequency for a burst of same-PR issue-side churn -- its legitimate purpose, matching the CI window's own philosophy -- but it can no longer be stolen by (or steal from) an unrelated CI-completion event. Rewrite the test that asserted the CI window suppressing the issue-side wake as correct behavior into a regression test asserting the opposite, and add a dedicated test proving same-domain (issue-side) bursts still coalesce correctly. --- src/queue/processors.ts | 27 ++++++++++++++--- test/unit/queue.test.ts | 64 +++++++++++++++++++++++++++++++++++++---- 2 files changed, 81 insertions(+), 10 deletions(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 4b66188830..2e3f5bc3d0 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -2173,6 +2173,25 @@ 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 (its legitimate, +// intended purpose — bound FREQUENCY, not correctness, same as the CI window's own philosophy). +async function issueLinkedPrReReviewCoalesced( + env: Env, + repoFullName: string, + prNumber: number, +): Promise { + return ciCompletionCoalesced( + env, + `issue-link-coalesce:${repoFullName.toLowerCase()}#${prNumber}`, + ); +} + async function ciHeadShaResolutionCoalesced( env: Env, repoFullName: string, @@ -2360,9 +2379,9 @@ async function maybeReReviewOnCiCompletion( * 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. Reuses the CI-completion coalesce window (ciReReviewCoalesced): its - * purpose is identical here — bound re-review FREQUENCY per PR, never correctness, since the re-review always - * re-fetches live state regardless of what triggered it. + * 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. */ async function maybeReReviewOnLinkedIssueChange( env: Env, @@ -2388,7 +2407,7 @@ async function maybeReReviewOnLinkedIssueChange( .filter((pr) => pr.linkedIssues.includes(issueNumber)) .map((pr) => pr.number); for (const prNumber of linkingPrNumbers) { - if (await ciReReviewCoalesced(env, repoFullName, prNumber)) continue; + if (await issueLinkedPrReReviewCoalesced(env, repoFullName, prNumber)) continue; await reReviewStoredPullRequest( env, deliveryId, diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 431a8937ac..9d92fade46 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -1581,26 +1581,35 @@ describe("queue processors", () => { expect(fetchCount).toBe(0); // never even minted a token — bailed before touching GitHub }); - it("issue label change respects the CI-completion coalesce window shared with the linked PR", async () => { + 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" }); - // PR #7 was already re-reviewed within the window (e.g. by a concurrent CI completion) — this issue-side - // signal must not double-fire a redundant re-review for the same PR within it. + // 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(); - checkRunsFetched ||= url.includes("/commits/a7/check-runs"); 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-coalesced", + deliveryId: "issue-label-not-suppressed-by-ci-coalesce", eventName: "issues", payload: { action: "labeled", @@ -1611,7 +1620,50 @@ describe("queue processors", () => { } as never, }); - expect(checkRunsFetched).toBe(false); // coalesced — no redundant re-review within the window + 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("#4 stale-surface repair: a rebased PR resyncs + re-reviews at the new head, and the marker survives the resync", async () => { From e7404a1c86c4618d0c3de3dc42c65e31abfdb69a Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 1 Jul 2026 14:22:45 -0700 Subject: [PATCH 3/4] fix(queue): schedule a trailing re-review for coalesced issue-side events The prior fix gave issue-side wakes their own coalesce window (separate from CI-completion's), but same-PR issue-side events are ALSO not interchangeable with each other: an add-then-remove label or assign-then-unassign sequence within the 60s window carries genuinely different states. The plain throttle (ciCompletionCoalesced) silently drops every event after the first, so the second event's state was lost entirely -- the PR stayed on the FIRST (now-stale) state until another webhook or the sweep eventually reached it, defeating the "wake promptly" purpose this whole trigger exists for. Add scheduleTrailingIssueLinkedReReview: when an issue-side event is coalesced, schedule exactly one deduped trailing agent-regate-pr job (delaySeconds: 60, the window's length) so the LATEST state is always eventually captured shortly after the window closes. Reuses the existing rate-limit-aware, retried sweep-unit job rather than inventing a new job type; deduped via its own claim key so a burst of N coalesced events schedules ONE trailing job, not N. A failed enqueue is swallowed (best-effort -- the sweep remains the ultimate backstop). Add regression tests: an add-then-remove sequence schedules exactly one correctly-shaped trailing job, a third coalesced event does not schedule a second, and a failed enqueue never propagates into the webhook handler. --- src/queue/processors.ts | 59 ++++++++++++++++++++++-- test/unit/queue.test.ts | 100 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 155 insertions(+), 4 deletions(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 927e066b5d..47427fcde6 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -2302,8 +2302,7 @@ async function ciReReviewCoalesced( // 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 (its legitimate, -// intended purpose — bound FREQUENCY, not correctness, same as the CI window's own philosophy). +// possible; a separate namespace confines coalescing to a burst of same-PR issue-side events. async function issueLinkedPrReReviewCoalesced( env: Env, repoFullName: string, @@ -2315,6 +2314,46 @@ async function issueLinkedPrReReviewCoalesced( ); } +// 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 alreadyScheduled = await ciCompletionCoalesced( + env, + `issue-link-trailing:${repoFullName.toLowerCase()}#${prNumber}`, + ); + if (alreadyScheduled) return; + 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), + }), + ), + ); +} + async function ciHeadShaResolutionCoalesced( env: Env, repoFullName: string, @@ -2504,7 +2543,10 @@ async function maybeReReviewOnCiCompletion( * 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. + * 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, @@ -2530,7 +2572,16 @@ async function maybeReReviewOnLinkedIssueChange( .filter((pr) => pr.linkedIssues.includes(issueNumber)) .map((pr) => pr.number); for (const prNumber of linkingPrNumbers) { - if (await issueLinkedPrReReviewCoalesced(env, repoFullName, prNumber)) continue; + if (await issueLinkedPrReReviewCoalesced(env, repoFullName, prNumber)) { + await scheduleTrailingIssueLinkedReReview( + env, + deliveryId, + installationId, + repoFullName, + prNumber, + ); + continue; + } await reReviewStoredPullRequest( env, deliveryId, diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 9e5a2847bc..1af6521277 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -1672,6 +1672,106 @@ describe("queue processors", () => { 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("#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: [] } }); From 4d1372129d421b06c0073d07c9314310abc7cf16 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 1 Jul 2026 14:36:06 -0700 Subject: [PATCH 4/4] fix(queue): claim the trailing re-review marker only after the enqueue succeeds scheduleTrailingIssueLinkedReReview used ciCompletionCoalesced, which writes its dedup marker unconditionally as soon as it is called -- BEFORE the subsequent env.JOBS.send even runs. A transient queue failure would therefore leave the "trailing re-review scheduled" marker held with nothing actually queued: every later coalesced issue-side event for the same PR within the window would see the marker already claimed and skip retrying, permanently forfeiting the guarantee this function exists to provide (capturing the latest linked-issue state after a burst). Reorder to check-then-send-then-claim: read the marker first (read-only, via getTransientKey), attempt the enqueue, and write the marker (via putTransientKey) ONLY when the send actually succeeds. A failed attempt leaves the marker unclaimed, so the next coalesced event in the same window retries the enqueue instead of silently giving up. Add a regression test simulating a transient failure (first send throws, second succeeds) that proves the retry works and that a fourth coalesced event correctly dedupes against the now-successful claim. --- src/queue/processors.ts | 40 +++++++++++++++------------ test/unit/queue.test.ts | 60 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 17 deletions(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 47427fcde6..21d893988e 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -2328,21 +2328,25 @@ async function scheduleTrailingIssueLinkedReReview( repoFullName: string, prNumber: number, ): Promise { - const alreadyScheduled = await ciCompletionCoalesced( - env, - `issue-link-trailing:${repoFullName.toLowerCase()}#${prNumber}`, - ); - if (alreadyScheduled) return; - await env.JOBS.send( - { - type: "agent-regate-pr", - deliveryId, - repoFullName, - prNumber, - installationId, - }, - { delaySeconds: CI_COALESCE_WINDOW_SECONDS }, - ).catch((error) => + 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", @@ -2350,8 +2354,10 @@ async function scheduleTrailingIssueLinkedReReview( 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( diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 1af6521277..3e9a2c58ec 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -1772,6 +1772,66 @@ describe("queue processors", () => { 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: [] } });