From 7b0e4806a18a88f5bc8c733df04cc8d54d82649e Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Mon, 6 Jul 2026 00:36:32 -0700 Subject: [PATCH 1/3] fix(review): stop re-reviewing a PR whose CI is permanently stuck prReadyForReview finalizes (runs a full paid AI review) once CI has been pending past its staleness cap, so a PR is never silently deferred forever. But it had no memory of having already done so: a CI context that will never settle (an orphaned required context, a fork check with no webhook to ever fire) hits this exact branch again on every later evaluation and re-spends another review for a disposition already established. Confirmed live: 3 PRs whose CI never settled were each re-reviewed 200-300+ times over more than 20 hours this way -- independent of the sweep's own outage-repair attempt cap (#3747), since ordinary (non-priority) sweep candidacy and live webhook re-evaluations both still reach this function. Caps it at one finalize per head SHA via a SHA-scoped audit event (reusing the existing ledger, no new table): once already finalized for the exact current head SHA, defer again instead of paying for another review. A new commit changes the head SHA, resetting the guard so a still-stuck PR finalizes fresh. --- src/queue/processors.ts | 40 ++++++++++++++++++++++++ test/unit/queue.test.ts | 68 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 3f0902ec11..184e07bb39 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -3330,6 +3330,34 @@ async function prReadyForReview( }).catch(() => undefined); return false; } + // #orb-ci-stuck-repeat: finalizing here runs a full paid AI review -- but a permanently-stuck CI context + // (a fork check that will never report, an orphaned required context) never resolves, so every later + // evaluation of the SAME head SHA hits this exact branch again and would re-spend another review for a + // disposition already established. Confirmed live: 3 PRs whose CI never settled each burned 200-300+ full + // reviews over 20+ hours this way, at a steady few-minute cadence, entirely independent of the sweep's own + // outage-repair cap (#orb-retry-storm) since ordinary (non-priority) sweep candidacy still reaches this + // function. Cap it at one finalize per head SHA (via a SHA-scoped audit event, no new table): once already + // finalized for this exact SHA, defer again instead of paying for another review. A new commit changes the + // head SHA, which resets the guard and lets the PR finalize fresh if it's still stuck. + const guardTargetKey = `${repoFullName}#${pr.number}#${pr.headSha}`; + const alreadyFinalizedForSha = await countRecentAuditEventsForActorAndTarget( + env, + "gittensory", + CI_STUCK_FINALIZE_GUARD_EVENT_TYPE, + guardTargetKey, + new Date(Date.now() - CI_STUCK_FINALIZE_GUARD_LOOKBACK_MS).toISOString(), + ); + if (alreadyFinalizedForSha >= CI_STUCK_FINALIZE_MAX_PER_SHA) { + await recordAuditEvent(env, { + eventType: "github_app.review_deferred_ci_pending", + actor: "gittensory", + targetKey: `${repoFullName}#${pr.number}`, + outcome: "queued", + detail: "CI still stuck pending, but already finalized once for this head SHA — deferring again instead of re-spending a review", + metadata: { deliveryId, repoFullName, headSha: pr.headSha }, + }).catch(() => undefined); + return false; + } await recordAuditEvent(env, { eventType: "github_app.review_finalized_ci_stuck", actor: "gittensory", @@ -3339,11 +3367,23 @@ async function prReadyForReview( "CI stuck pending past the staleness cap — finalizing so the PR is surfaced, not silently deferred forever", metadata: { deliveryId, repoFullName }, }).catch(() => undefined); + await recordAuditEvent(env, { + eventType: CI_STUCK_FINALIZE_GUARD_EVENT_TYPE, + actor: "gittensory", + targetKey: guardTargetKey, + outcome: "completed", + detail: "recorded so a repeat evaluation of the SAME head SHA does not pay for another review", + metadata: { repoFullName, prNumber: pr.number, headSha: pr.headSha }, + }).catch(() => undefined); // fall through → return true → the gate finalizes + the PR is disposed/held, never silently stuck. } return true; } +const CI_STUCK_FINALIZE_GUARD_EVENT_TYPE = "github_app.review_finalized_ci_stuck_guard"; +const CI_STUCK_FINALIZE_MAX_PER_SHA = 1; +const CI_STUCK_FINALIZE_GUARD_LOOKBACK_MS = 7 * 24 * 60 * 60 * 1000; + // A required check pending longer than this is treated as STUCK (orphaned / never-completing — e.g. a fork check // that will never report). Past it, prReadyForReview stops deferring and finalizes the gate so the PR surfaces // (held / needs-human) instead of deferring forever. Generous so a genuinely-slow CI is never cut off early. diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index e727911d74..2f258fbccb 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -1457,6 +1457,74 @@ describe("queue processors", () => { } }); + it("REGRESSION (#orb-ci-stuck-repeat): re-evaluating the SAME stuck head SHA after it was already finalized once defers instead of paying for another review", 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: { pull_requests: "write", checks: "write" }, 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", update_branch: "auto" }, aiReviewMode: "off", gatePack: "oss-anti-slop", gateCheckMode: "enabled", checkRunMode: "off", commentMode: "off", publicSurface: "off" }); + await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 7, title: "Permanently stuck CI", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, base: { ref: "main" }, labels: [], body: "Closes #1" }); + vi.setSystemTime(new Date("2026-05-28T02:00:00.000Z")); + await env.SELFHOST_TRANSIENT_CACHE?.set( + "ci-pending-first-seen:owner/agent-repo#7:a7", + String(Date.now() - 31 * 60 * 1000), + 7 * 24 * 3600, + ); + const requiredContextsSpy = vi.spyOn(backfillModule, "fetchRequiredStatusContexts").mockResolvedValue(new Set(["trusted-required-ci"])); + const liveCiSpy = vi.spyOn(backfillModule, "fetchLiveCiAggregatePreferGraphQl").mockResolvedValue({ + ciState: "passed", + hasPending: true, + hasVisiblePending: false, + hasMissingRequiredContext: false, + failingDetails: [], + nonRequiredFailingDetails: [], + ciCompletenessWarning: null, + }); + let gateChecks = 0; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = (init?.method ?? "GET").toUpperCase(); + if (url === "https://api.gittensor.io/miners") return Response.json([]); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (/\/pulls\/7(?:\?|$)/.test(url) && method === "GET") return Response.json({ number: 7, title: "Permanently stuck CI", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, mergeable_state: "clean", labels: [], body: "Closes #1" }); + if (url.includes("/pulls/7/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+export const ok = true;" }]); + if (url.includes("/check-runs") && (method === "POST" || method === "PATCH")) { + gateChecks += 1; + return Response.json({ id: 901 }, { status: method === "POST" ? 201 : 200 }); + } + return Response.json({}); + }); + + try { + // First evaluation: CI is stuck past the cap -- this SHOULD finalize and run a real review. + await processJob(env, { type: "agent-regate-pr", deliveryId: "stuck-ci-eval-1", repoFullName: "owner/agent-repo", prNumber: 7, installationId: 9001 }); + expect(gateChecks).toBeGreaterThan(0); + const gateChecksAfterFirst = gateChecks; + const finalizedAfterFirst = await env.DB.prepare("select count(*) as n from audit_events where event_type = ?") + .bind("github_app.review_finalized_ci_stuck") + .first<{ n: number }>(); + expect(finalizedAfterFirst?.n).toBe(1); + const guardAfterFirst = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and target_key = ?") + .bind("github_app.review_finalized_ci_stuck_guard", "owner/agent-repo#7#a7") + .first<{ n: number }>(); + expect(guardAfterFirst?.n).toBe(1); + + // Second evaluation, same head SHA, CI still stuck: must NOT finalize (and NOT pay for) another review. + await processJob(env, { type: "agent-regate-pr", deliveryId: "stuck-ci-eval-2", repoFullName: "owner/agent-repo", prNumber: 7, installationId: 9001 }); + expect(gateChecks).toBe(gateChecksAfterFirst); // no additional check-run write — no second review ran + const finalizedAfterSecond = await env.DB.prepare("select count(*) as n from audit_events where event_type = ?") + .bind("github_app.review_finalized_ci_stuck") + .first<{ n: number }>(); + expect(finalizedAfterSecond?.n).toBe(1); // unchanged — guarded, not re-finalized + const deferred = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and target_key = ?") + .bind("github_app.review_deferred_ci_pending", "owner/agent-repo#7") + .first<{ n: number }>(); + expect(deferred?.n).toBe(1); // the second evaluation deferred instead + } finally { + liveCiSpy.mockRestore(); + requiredContextsSpy.mockRestore(); + } + }); + it("surfaces inferred pending CI after the stale-CI cap", 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: { pull_requests: "write", checks: "write" }, events: [] } }); From 00bd50ef57e6f11bcfda4cbc1576fc8742413128 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Mon, 6 Jul 2026 00:40:25 -0700 Subject: [PATCH 2/3] fix(review): surface repeated CI-stuck review suppression to Sentry The DB-only audit_events write from the previous commit has no operator visibility until someone queries the ledger directly, the way this bug was originally found. Add a matching structured console.error at the exact moment the guard suppresses a repeat review, so it flows through the existing forwardStructuredLogToSentry pipeline (level:"error" is deliberate -- the line's own existence IS the anomaly signal, same convention as selfhost_ai_provider_failed / selfhost_ai_providers_exhausted). --- src/queue/processors.ts | 15 +++++++++++++++ test/unit/queue.test.ts | 8 ++++++++ 2 files changed, 23 insertions(+) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 184e07bb39..a03183a241 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -3356,6 +3356,21 @@ async function prReadyForReview( detail: "CI still stuck pending, but already finalized once for this head SHA — deferring again instead of re-spending a review", metadata: { deliveryId, repoFullName, headSha: pr.headSha }, }).catch(() => undefined); + // level:"error" is deliberate, not a code failure: this line only fires once the guard above already + // stopped the wasteful re-review, so its OWN existence is the operator-visible signal (via the structured + // log → Sentry forwarder, forwardStructuredLogToSentry) that a PR's CI has been permanently stuck long + // enough to need a human — the same "surface an anomaly at error level" convention selfhost_ai_provider_ + // failed / selfhost_ai_providers_exhausted already use in src/selfhost/ai.ts. + console.error( + JSON.stringify({ + level: "error", + event: "ci_stuck_review_repeat_suppressed", + repo: repoFullName, + pullNumber: pr.number, + headSha: pr.headSha, + deliveryId, + }), + ); return false; } await recordAuditEvent(env, { diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 2f258fbccb..3943682555 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -1493,6 +1493,7 @@ describe("queue processors", () => { } return Response.json({}); }); + const errors = vi.spyOn(console, "error").mockImplementation(() => undefined); try { // First evaluation: CI is stuck past the cap -- this SHOULD finalize and run a real review. @@ -1507,6 +1508,7 @@ describe("queue processors", () => { .bind("github_app.review_finalized_ci_stuck_guard", "owner/agent-repo#7#a7") .first<{ n: number }>(); expect(guardAfterFirst?.n).toBe(1); + expect(errors.mock.calls.some(([line]) => typeof line === "string" && line.includes("ci_stuck_review_repeat_suppressed"))).toBe(false); // Second evaluation, same head SHA, CI still stuck: must NOT finalize (and NOT pay for) another review. await processJob(env, { type: "agent-regate-pr", deliveryId: "stuck-ci-eval-2", repoFullName: "owner/agent-repo", prNumber: 7, installationId: 9001 }); @@ -1519,7 +1521,13 @@ describe("queue processors", () => { .bind("github_app.review_deferred_ci_pending", "owner/agent-repo#7") .first<{ n: number }>(); expect(deferred?.n).toBe(1); // the second evaluation deferred instead + // Sentry-visible signal (via the structured-log forwarder) fires exactly once, on the guarded evaluation. + const repeatSuppressedLogs = errors.mock.calls.filter(([line]) => typeof line === "string" && line.includes("ci_stuck_review_repeat_suppressed")); + expect(repeatSuppressedLogs).toHaveLength(1); + const logged = JSON.parse(repeatSuppressedLogs[0]![0] as string) as Record; + expect(logged).toMatchObject({ level: "error", event: "ci_stuck_review_repeat_suppressed", repo: "owner/agent-repo", pullNumber: 7, headSha: "a7" }); } finally { + errors.mockRestore(); liveCiSpy.mockRestore(); requiredContextsSpy.mockRestore(); } From 963b6544f5e031e0b0edbc5e9a9286efc74504d5 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Mon, 6 Jul 2026 02:38:42 -0700 Subject: [PATCH 3/3] test(review): cover the fail-open audit writes in the CI-stuck finalize guard Both new recordAuditEvent calls in the #orb-ci-stuck-repeat guard (review_finalized_ci_stuck_guard on first finalize, review_deferred_ci_pending on a guarded repeat) swallow their own write failures via .catch(() => undefined), but neither failure path had a test -- codecov flagged both lines as uncovered patch. Add two regression tests forcing the audit_events insert to throw in each case and asserting the surrounding finalize/defer decision still completes correctly despite the swallowed write failure. --- test/unit/queue.test.ts | 116 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 3943682555..09ee3ffe39 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -1533,6 +1533,122 @@ describe("queue processors", () => { } }); + it("REGRESSION (#orb-ci-stuck-repeat, fail-open): a failed guard-audit write does not stop the first stuck-CI finalize from running its review", 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: { pull_requests: "write", checks: "write" }, 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", update_branch: "auto" }, aiReviewMode: "off", gatePack: "oss-anti-slop", gateCheckMode: "enabled", checkRunMode: "off", commentMode: "off", publicSurface: "off" }); + await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 8, title: "Permanently stuck CI, audit write fails", state: "open", user: { login: "contributor" }, head: { sha: "b8" }, base: { ref: "main" }, labels: [], body: "Closes #1" }); + vi.setSystemTime(new Date("2026-05-28T02:00:00.000Z")); + await env.SELFHOST_TRANSIENT_CACHE?.set( + "ci-pending-first-seen:owner/agent-repo#8:b8", + String(Date.now() - 31 * 60 * 1000), + 7 * 24 * 3600, + ); + const requiredContextsSpy = vi.spyOn(backfillModule, "fetchRequiredStatusContexts").mockResolvedValue(new Set(["trusted-required-ci"])); + const liveCiSpy = vi.spyOn(backfillModule, "fetchLiveCiAggregatePreferGraphQl").mockResolvedValue({ + ciState: "passed", + hasPending: true, + hasVisiblePending: false, + hasMissingRequiredContext: false, + failingDetails: [], + nonRequiredFailingDetails: [], + ciCompletenessWarning: null, + }); + let gateChecks = 0; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = (init?.method ?? "GET").toUpperCase(); + if (url === "https://api.gittensor.io/miners") return Response.json([]); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (/\/pulls\/8(?:\?|$)/.test(url) && method === "GET") return Response.json({ number: 8, title: "Permanently stuck CI, audit write fails", state: "open", user: { login: "contributor" }, head: { sha: "b8" }, mergeable_state: "clean", labels: [], body: "Closes #1" }); + if (url.includes("/pulls/8/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+export const ok = true;" }]); + if (url.includes("/check-runs") && (method === "POST" || method === "PATCH")) { + gateChecks += 1; + return Response.json({ id: 902 }, { status: method === "POST" ? 201 : 200 }); + } + return Response.json({}); + }); + const realPrepare = env.DB.prepare.bind(env.DB); + env.DB.prepare = ((sql: string) => { + if (/insert\s+into\s+["`]?audit_events["`]?/i.test(sql)) throw new Error("audit write failed"); + return realPrepare(sql); + }) as typeof env.DB.prepare; + + try { + await processJob(env, { type: "agent-regate-pr", deliveryId: "stuck-ci-audit-fail", repoFullName: "owner/agent-repo", prNumber: 8, installationId: 9001 }); + // The guard-audit write (CI_STUCK_FINALIZE_GUARD_EVENT_TYPE) failed silently, but the finalize decision + // itself (fall through -> return true) is independent of that write's success -- the review still runs. + expect(gateChecks).toBeGreaterThan(0); + } finally { + env.DB.prepare = realPrepare; + liveCiSpy.mockRestore(); + requiredContextsSpy.mockRestore(); + } + }); + + it("REGRESSION (#orb-ci-stuck-repeat, fail-open): a failed defer-audit write does not stop a guarded repeat evaluation from deferring", 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: { pull_requests: "write", checks: "write" }, 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", update_branch: "auto" }, aiReviewMode: "off", gatePack: "oss-anti-slop", gateCheckMode: "enabled", checkRunMode: "off", commentMode: "off", publicSurface: "off" }); + await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 9, title: "Permanently stuck CI, repeat defer audit write fails", state: "open", user: { login: "contributor" }, head: { sha: "c9" }, base: { ref: "main" }, labels: [], body: "Closes #1" }); + vi.setSystemTime(new Date("2026-05-28T02:00:00.000Z")); + await env.SELFHOST_TRANSIENT_CACHE?.set( + "ci-pending-first-seen:owner/agent-repo#9:c9", + String(Date.now() - 31 * 60 * 1000), + 7 * 24 * 3600, + ); + const requiredContextsSpy = vi.spyOn(backfillModule, "fetchRequiredStatusContexts").mockResolvedValue(new Set(["trusted-required-ci"])); + const liveCiSpy = vi.spyOn(backfillModule, "fetchLiveCiAggregatePreferGraphQl").mockResolvedValue({ + ciState: "passed", + hasPending: true, + hasVisiblePending: false, + hasMissingRequiredContext: false, + failingDetails: [], + nonRequiredFailingDetails: [], + ciCompletenessWarning: null, + }); + let gateChecks = 0; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = (init?.method ?? "GET").toUpperCase(); + if (url === "https://api.gittensor.io/miners") return Response.json([]); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (/\/pulls\/9(?:\?|$)/.test(url) && method === "GET") return Response.json({ number: 9, title: "Permanently stuck CI, repeat defer audit write fails", state: "open", user: { login: "contributor" }, head: { sha: "c9" }, mergeable_state: "clean", labels: [], body: "Closes #1" }); + if (url.includes("/pulls/9/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+export const ok = true;" }]); + if (url.includes("/check-runs") && (method === "POST" || method === "PATCH")) { + gateChecks += 1; + return Response.json({ id: 903 }, { status: method === "POST" ? 201 : 200 }); + } + return Response.json({}); + }); + + try { + // First evaluation succeeds normally, establishing the guard row. + await processJob(env, { type: "agent-regate-pr", deliveryId: "stuck-ci-audit-fail-1", repoFullName: "owner/agent-repo", prNumber: 9, installationId: 9001 }); + expect(gateChecks).toBeGreaterThan(0); + const gateChecksAfterFirst = gateChecks; + + // Second evaluation is guarded (defers), but its OWN audit write (review_deferred_ci_pending) fails. + const realPrepare = env.DB.prepare.bind(env.DB); + env.DB.prepare = ((sql: string) => { + if (/insert\s+into\s+["`]?audit_events["`]?/i.test(sql)) throw new Error("audit write failed"); + return realPrepare(sql); + }) as typeof env.DB.prepare; + try { + await processJob(env, { type: "agent-regate-pr", deliveryId: "stuck-ci-audit-fail-2", repoFullName: "owner/agent-repo", prNumber: 9, installationId: 9001 }); + } finally { + env.DB.prepare = realPrepare; + } + // Guarded — no additional review ran, despite the defer-audit write itself failing silently. + expect(gateChecks).toBe(gateChecksAfterFirst); + } finally { + liveCiSpy.mockRestore(); + requiredContextsSpy.mockRestore(); + } + }); + it("surfaces inferred pending CI after the stale-CI cap", 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: { pull_requests: "write", checks: "write" }, events: [] } });