diff --git a/src/queue/ai-review-orchestration.ts b/src/queue/ai-review-orchestration.ts index b2319f5bb8..6137c2009d 100644 --- a/src/queue/ai-review-orchestration.ts +++ b/src/queue/ai-review-orchestration.ts @@ -286,6 +286,9 @@ export function aiReviewAttemptFailedResult( export async function shouldStartAiReviewForAdvisory( env: Env, args: { + // Threaded straight into shouldRequirePublicAiReviewForAdvisory's hard gate below -- a "paused" repo must + // never be told a review will run, matching runAiReviewForAdvisory's own mode==="paused" stop. + mode: AgentActionMode; settings: RepositorySettings; advisory: Pick>, "headSha">; repoFullName: string; @@ -352,6 +355,7 @@ export function resolveAiReviewableAuthor( * exact reason whenever `aiReviewWillRun` ends up false for a cause none of the OTHER (already-audited) paths * — author blacklist, manual-review freeze, auto-review skip, one-shot reuse, reputation skip — cover. */ export type PublicAiReviewGateSkipReason = + | "paused" | "skip_ai_review_requested" | "ai_review_mode_off" | "author_not_reviewable" @@ -363,6 +367,11 @@ export type PublicAiReviewGateSkipReason = export function resolvePublicAiReviewGateSkipReason( env: Env, args: { + // #9692: evaluated FIRST, ahead of every other reason -- a paused repo's publish pass still computes the + // gate verdict (only GitHub writes are suppressed), so this predicate must independently know about the + // same stop runAiReviewForAdvisory applies as its own first check, or the "expected a review" flag built + // from this function drifts out of sync with reality on every paused pass. + mode: AgentActionMode; settings: RepositorySettings; advisory: Pick>, "headSha">; repoFullName: string; @@ -372,6 +381,7 @@ export function resolvePublicAiReviewGateSkipReason( }, ): PublicAiReviewGateSkipReason | null { const reviewableAuthor = resolveAiReviewableAuthor(args.settings, args.confirmedContributor); + if (args.mode === "paused") return "paused"; if (args.skipAiReview) return "skip_ai_review_requested"; if (args.settings.aiReviewMode === "off") return "ai_review_mode_off"; if (!reviewableAuthor) return "author_not_reviewable"; @@ -385,6 +395,7 @@ export function resolvePublicAiReviewGateSkipReason( export function shouldRequirePublicAiReviewForAdvisory( env: Env, args: { + mode: AgentActionMode; settings: RepositorySettings; advisory: Pick>, "headSha">; repoFullName: string; diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 88e1848814..c6524e5f1c 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -8364,6 +8364,7 @@ async function resolvePullRequestFilesForReview( export function maybeAddRequiredAutoReviewSkipHold( env: Env, args: { + mode: AgentActionMode; settings: RepositorySettings; advisory: Pick>, "headSha" | "findings">; repoFullName: string; @@ -8410,6 +8411,7 @@ export function maybeAddRequiredAutoReviewSkipHold( export function maybeAddPromptInjectionHold( env: Env, args: { + mode: AgentActionMode; settings: RepositorySettings; advisory: Pick>, "headSha" | "findings">; repoFullName: string; @@ -8443,6 +8445,7 @@ export function maybeAddPromptInjectionHold( export function maybeAddReputationSkipHold( env: Env, args: { + mode: AgentActionMode; settings: RepositorySettings; advisory: Pick>, "headSha" | "findings">; repoFullName: string; @@ -11204,6 +11207,7 @@ async function maybePublishPrPublicSurface( // inline findings at the publish site below, mirroring findingCategories. fixHandoffEnabledForReview = shouldEmitFixHandoff(env, repoFullName, reviewManifestForAutoReview?.review.fixHandoff ?? undefined); maybeAddRequiredAutoReviewSkipHold(env, { + mode, settings, advisory, repoFullName, @@ -11227,6 +11231,7 @@ async function maybePublishPrPublicSurface( // auto-merge, i.e. suspicion would BUY less scrutiny. Exact sibling of the contributor-controlled skip // hold above; a no-op when the repo does not require blocking AI review, or when no skip fired. maybeAddReputationSkipHold(env, { + mode, settings, advisory, repoFullName, @@ -11239,6 +11244,7 @@ async function maybePublishPrPublicSurface( // #9035: a caught reviewer-manipulation attempt is evidence of intent, not a code-quality observation, and // must not buy an automated decision. Same fail-closed shape as the two holds above. maybeAddPromptInjectionHold(env, { + mode, settings, advisory, repoFullName, @@ -11268,6 +11274,7 @@ async function maybePublishPrPublicSurface( !autoReviewSkipReason && !oneShotPriorReview && (await shouldStartAiReviewForAdvisory(env, { + mode, settings, advisory, repoFullName, @@ -11383,6 +11390,7 @@ async function maybePublishPrPublicSurface( // the remaining silent branch: a forced pass that ends with no fresh review always now has exactly one of // a reuse event, a reuse-unavailable event, a reputation-skip hold, or this event in the audit trail. const publicGateSkipReason = resolvePublicAiReviewGateSkipReason(env, { + mode, settings, advisory, repoFullName, diff --git a/test/unit/ai-review-advisory.test.ts b/test/unit/ai-review-advisory.test.ts index 9089cee498..107870a024 100644 --- a/test/unit/ai-review-advisory.test.ts +++ b/test/unit/ai-review-advisory.test.ts @@ -1,6 +1,6 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { buildAiReviewDiff, claimAiReviewLock, runAiReviewForAdvisory, shouldStartAiReviewForAdvisory } from "../../src/queue/processors"; -import { resolveAiReviewableAuthor } from "../../src/queue/ai-review-orchestration"; +import { resolveAiReviewableAuthor, resolvePublicAiReviewGateSkipReason } from "../../src/queue/ai-review-orchestration"; import { BEST_REVIEW_MODELS, INCOHERENT_DIFF_ASSESSMENT } from "../../src/services/ai-review"; import * as posthogModule from "../../src/selfhost/posthog"; import { recordAiUsageEvent, upsertRepositoryAiKey } from "../../src/db/repositories"; @@ -144,7 +144,7 @@ describe("resolveAiReviewableAuthor invariant (#orb-ai-review-always-review)", ( describe("shouldStartAiReviewForAdvisory", () => { const enabledEnv = () => aiEnv(async () => ({ response: notesOnlyJson() })); - const base = { settings: { aiReviewMode: "advisory", gatePack: "gittensor" } as RepositorySettings, advisory: advisory(), repoFullName: "acme/widgets", author: "alice", confirmedContributor: true }; + const base = { mode: "live" as const, settings: { aiReviewMode: "advisory", gatePack: "gittensor" } as RepositorySettings, advisory: advisory(), repoFullName: "acme/widgets", author: "alice", confirmedContributor: true }; it("matches the AI review entry gates before the reviewing placeholder is posted", async () => { await expect(shouldStartAiReviewForAdvisory(enabledEnv(), base)).resolves.toBe(true); @@ -222,6 +222,50 @@ describe("shouldStartAiReviewForAdvisory", () => { ).resolves.toBe(false); await expect(shouldStartAiReviewForAdvisory(env, { ...base, forceAiReview: true, skipAiReview: true })).resolves.toBe(false); }); + + // #9692: a paused repo must never be told a review will run -- runAiReviewForAdvisory already refuses to + // spend on mode: "paused" as its own first check, and this predicate must agree, or the caller's "was a + // review expected" flag drifts out of sync with what actually happened (the root cause of the + // ai_review_public_summary_missing false alarm this issue fixes). + it("#9692: mode 'paused' never starts a review, even on an otherwise fully-eligible PR", async () => { + await expect(shouldStartAiReviewForAdvisory(enabledEnv(), { ...base, mode: "paused" })).resolves.toBe(false); + // forceAiReview bypasses only the reputation skip, never this hard gate. + await expect(shouldStartAiReviewForAdvisory(enabledEnv(), { ...base, mode: "paused", forceAiReview: true })).resolves.toBe(false); + }); +}); + +describe("resolvePublicAiReviewGateSkipReason (#9692)", () => { + const gateEnv = () => aiEnv(async () => ({ response: notesOnlyJson() })); + const gateArgs = { + mode: "live" as const, + settings: { aiReviewMode: "advisory", gatePack: "gittensor" } as RepositorySettings, + advisory: advisory(), + repoFullName: "acme/widgets", + author: "alice", + confirmedContributor: true, + }; + + it("returns 'paused' for mode: 'paused' on an otherwise fully-eligible PR", () => { + expect(resolvePublicAiReviewGateSkipReason(gateEnv(), { ...gateArgs, mode: "paused" })).toBe("paused"); + }); + + it("PRECEDENCE: 'paused' wins even when a different skip reason would also apply (skipAiReview / mode off)", () => { + expect(resolvePublicAiReviewGateSkipReason(gateEnv(), { ...gateArgs, mode: "paused", skipAiReview: true })).toBe("paused"); + expect( + resolvePublicAiReviewGateSkipReason(gateEnv(), { + ...gateArgs, + mode: "paused", + settings: { aiReviewMode: "off" } as RepositorySettings, + }), + ).toBe("paused"); + }); + + it("mode: 'live' or 'dry_run' falls through to the pre-existing reasons, never 'paused'", () => { + expect(resolvePublicAiReviewGateSkipReason(gateEnv(), { ...gateArgs, skipAiReview: true })).toBe("skip_ai_review_requested"); + expect(resolvePublicAiReviewGateSkipReason(gateEnv(), { ...gateArgs, mode: "dry_run", skipAiReview: true })).toBe("skip_ai_review_requested"); + expect(resolvePublicAiReviewGateSkipReason(gateEnv(), gateArgs)).toBeNull(); + expect(resolvePublicAiReviewGateSkipReason(gateEnv(), { ...gateArgs, mode: "dry_run" })).toBeNull(); + }); }); describe("runAiReviewForAdvisory", () => { diff --git a/test/unit/auto-review-wiring.test.ts b/test/unit/auto-review-wiring.test.ts index ab99d29756..07021e589a 100644 --- a/test/unit/auto-review-wiring.test.ts +++ b/test/unit/auto-review-wiring.test.ts @@ -417,6 +417,7 @@ describe("review.auto_review wiring (#1954)", () => { const advisory = { headSha: "sha", findings: [] as unknown[] }; const added = maybeAddPromptInjectionHold(blockingEnv, { + mode: "live", settings, advisory: advisory as never, repoFullName: "acme/widgets", @@ -434,6 +435,7 @@ describe("review.auto_review wiring (#1954)", () => { const untouched = { headSha: "sha", findings: [] as unknown[] }; expect( maybeAddPromptInjectionHold(blockingEnv, { + mode: "live", settings, advisory: untouched as never, repoFullName: "acme/widgets", @@ -449,6 +451,7 @@ describe("review.auto_review wiring (#1954)", () => { const offAdvisory = { headSha: "sha", findings: [] as unknown[] }; expect( maybeAddPromptInjectionHold(offEnv, { + mode: "live", settings: { gatePack: "oss-anti-slop", aiReviewMode: "off" } as never, advisory: offAdvisory as never, repoFullName: "acme/widgets", @@ -468,6 +471,7 @@ describe("review.auto_review wiring (#1954)", () => { const settings = { gatePack: "oss-anti-slop", aiReviewMode: "block", aiReviewAllAuthors: false } as never; const advisory = { headSha: "sha", findings: [] as unknown[] }; const added = maybeAddReputationSkipHold(blockingEnv, { + mode: "live", settings, advisory: advisory as never, repoFullName: "acme/widgets", @@ -486,6 +490,7 @@ describe("review.auto_review wiring (#1954)", () => { const advisory = { headSha: "sha", findings: [] as unknown[] }; expect( maybeAddReputationSkipHold(blockingEnv, { + mode: "live", settings, advisory: advisory as never, repoFullName: "acme/widgets", @@ -502,6 +507,7 @@ describe("review.auto_review wiring (#1954)", () => { const settings = { gatePack: "oss-anti-slop", aiReviewMode: "block", aiReviewAllAuthors: false } as never; const advisory = { headSha: "sha", findings: [] as unknown[] }; const added = maybeAddReputationSkipHold(blockingEnv, { + mode: "live", settings, advisory: advisory as never, repoFullName: "acme/widgets", @@ -517,6 +523,7 @@ describe("review.auto_review wiring (#1954)", () => { const untouched = { headSha: "sha", findings: [] as unknown[] }; expect( maybeAddReputationSkipHold(blockingEnv, { + mode: "live", settings, advisory: untouched as never, repoFullName: "acme/widgets", @@ -532,6 +539,7 @@ describe("review.auto_review wiring (#1954)", () => { const reviewOff = { headSha: "sha", findings: [] as unknown[] }; expect( maybeAddReputationSkipHold(blockingEnv, { + mode: "live", settings: { gatePack: "oss-anti-slop", aiReviewMode: "off", aiReviewAllAuthors: false } as never, advisory: reviewOff as never, repoFullName: "acme/widgets", @@ -548,6 +556,7 @@ describe("review.auto_review wiring (#1954)", () => { const added = maybeAddRequiredAutoReviewSkipHold( { AI_SUMMARIES_ENABLED: "true", AI_PUBLIC_COMMENTS_ENABLED: "true", AI: {} } as Env, { + mode: "live", settings: { gatePack: "oss-anti-slop", aiReviewMode: "block", aiReviewAllAuthors: false } as any, advisory, repoFullName: "acme/widgets", @@ -569,6 +578,7 @@ describe("review.auto_review wiring (#1954)", () => { const trustedSkip = { headSha: "sha", findings: [] }; expect( maybeAddRequiredAutoReviewSkipHold({ AI_SUMMARIES_ENABLED: "true", AI_PUBLIC_COMMENTS_ENABLED: "true", AI: {} } as Env, { + mode: "live", settings: { gatePack: "oss-anti-slop", aiReviewMode: "block", aiReviewAllAuthors: false } as any, advisory: trustedSkip, repoFullName: "acme/widgets", @@ -582,6 +592,7 @@ describe("review.auto_review wiring (#1954)", () => { const disabledAi = { headSha: "sha", findings: [] }; expect( maybeAddRequiredAutoReviewSkipHold({ AI_SUMMARIES_ENABLED: "true", AI_PUBLIC_COMMENTS_ENABLED: "true", AI: {} } as Env, { + mode: "live", settings: { gatePack: "oss-anti-slop", aiReviewMode: "off", aiReviewAllAuthors: false } as any, advisory: disabledAi, repoFullName: "acme/widgets", @@ -593,4 +604,53 @@ describe("review.auto_review wiring (#1954)", () => { expect(disabledAi.findings).toEqual([]); }); + // #9692: a paused repo suppresses these holds the same way "AI review off" already does -- nothing was + // expected to run this pass, so a skip is not a suppression worth holding for. + it("#9692: mode 'paused' suppresses every sibling hold, even when its own skip condition fired", () => { + const blockingEnv = { AI_SUMMARIES_ENABLED: "true", AI_PUBLIC_COMMENTS_ENABLED: "true", AI: {} } as Env; + const settings = { gatePack: "oss-anti-slop", aiReviewMode: "block", aiReviewAllAuthors: false } as never; + + const reputationAdvisory = { headSha: "sha", findings: [] as unknown[] }; + expect( + maybeAddReputationSkipHold(blockingEnv, { + mode: "paused", + settings, + advisory: reputationAdvisory as never, + repoFullName: "acme/widgets", + author: "burst-farmer", + confirmedContributor: false, + reputationSkipped: true, + }), + ).toBe(false); + expect(reputationAdvisory.findings).toEqual([]); + + const injectionAdvisory = { headSha: "sha", findings: [] as unknown[] }; + expect( + maybeAddPromptInjectionHold(blockingEnv, { + mode: "paused", + settings, + advisory: injectionAdvisory as never, + repoFullName: "acme/widgets", + author: "attacker", + confirmedContributor: false, + injectionDetected: true, + }), + ).toBe(false); + expect(injectionAdvisory.findings).toEqual([]); + + const autoReviewAdvisory = { headSha: "sha", findings: [] as unknown[] }; + expect( + maybeAddRequiredAutoReviewSkipHold(blockingEnv, { + mode: "paused", + settings: settings as any, + advisory: autoReviewAdvisory as never, + repoFullName: "acme/widgets", + author: "alice", + confirmedContributor: false, + autoReviewSkipReason: "review skipped (WIP title)", + }), + ).toBe(false); + expect(autoReviewAdvisory.findings).toEqual([]); + }); + }); diff --git a/test/unit/gate-integrity-invariants.test.ts b/test/unit/gate-integrity-invariants.test.ts index ed0a06059b..23487f6831 100644 --- a/test/unit/gate-integrity-invariants.test.ts +++ b/test/unit/gate-integrity-invariants.test.ts @@ -115,6 +115,7 @@ describe("reputation-triggered and contributor-controlled AI-review skips hold i const contributorAdvisory = advisoryStub(); const contributorHeld = maybeAddRequiredAutoReviewSkipHold(env, { + mode: "live", settings, advisory: contributorAdvisory as never, repoFullName: "acme/widgets", @@ -125,6 +126,7 @@ describe("reputation-triggered and contributor-controlled AI-review skips hold i const reputationAdvisory = advisoryStub(); const reputationHeld = maybeAddReputationSkipHold(env, { + mode: "live", settings, advisory: reputationAdvisory as never, repoFullName: "acme/widgets", @@ -150,6 +152,7 @@ describe("reputation-triggered and contributor-controlled AI-review skips hold i const contributorAdvisory = advisoryStub(); expect( maybeAddRequiredAutoReviewSkipHold(env, { + mode: "live", settings, advisory: contributorAdvisory as never, repoFullName: "acme/widgets", @@ -163,6 +166,7 @@ describe("reputation-triggered and contributor-controlled AI-review skips hold i const reputationAdvisory = advisoryStub(); expect( maybeAddReputationSkipHold(env, { + mode: "live", settings, advisory: reputationAdvisory as never, repoFullName: "acme/widgets", diff --git a/test/unit/queue-2.test.ts b/test/unit/queue-2.test.ts index fbe0a7ec43..c594589a5e 100644 --- a/test/unit/queue-2.test.ts +++ b/test/unit/queue-2.test.ts @@ -1594,6 +1594,81 @@ describe("queue processors", () => { expect(audit?.n).toBe(0); }); + it("REGRESSION (#9692): a paused repo never fires the ai_review_public_summary_missing alarm, records a single paused gate-skip, and starts no review-evasion tracking", async () => { + let aiCalls = 0; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { + run: async () => { + aiCalls += 1; + return { response: JSON.stringify({ assessment: "Should never run.", blockers: [], nits: [], suggestions: [] }) }; + }, + } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await persistRegistrySnapshot( + asCloudEnv(env), + normalizeRegistryPayload( + { "JSONbored/gittensory": { emission_share: 0.01, issue_discovery_share: 0 } }, + { kind: "raw-github", url: "https://example.test" }, + "2026-05-23T00:00:00.000Z", + ), + ); + await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 123); + await upsertRepositorySettings(env, { + repoFullName: "JSONbored/gittensory", + autoLabelEnabled: false, + gatePack: "oss-anti-slop", + agentPaused: true, + }); + await upsertRepoFocusManifest(env, "JSONbored/gittensory", { settings: { commentMode: "all_prs", publicSurface: "comment_only", checkRunMode: "off", reviewCheckMode: "required", aiReviewMode: "block" } }); + await upsertOfficialMinerDetection(env, "contributor", { status: "confirmed", snapshot: queueMinerSnapshot("contributor") }, 60_000); + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/pulls/11/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+export const ok = true;" }]); + if (url.endsWith("/pulls/11")) return Response.json({ number: 11, title: "Clean PR", state: "open", user: { login: "contributor" }, head: { sha: "a11" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/commits/a11/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/a11/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/11/comments") && method === "GET") return Response.json([]); + if (url.includes("/issues/1")) return Response.json({ number: 1, title: "Issue", state: "open", labels: [], user: { login: "reporter" } }); + if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } }); + return Response.json({}); + }); + + await expect( + processJob(env, { + type: "github-webhook", + deliveryId: "paused-repo-ai-review-gate-skip", + eventName: "pull_request", + payload: { + action: "opened", + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + pull_request: { number: 11, title: "Clean PR", state: "open", user: { login: "contributor" }, head: { sha: "a11" }, labels: [], body: "Closes #1" }, + }, + }), + ).resolves.toBeUndefined(); + + expect(aiCalls).toBe(0); // the paused repo never spends the LLM call + const missingSummaryAudit = await env.DB.prepare("select count(*) as n from audit_events where event_type = ?") + .bind("github_app.ai_review_public_summary_missing") + .first<{ n: number }>(); + expect(missingSummaryAudit?.n).toBe(0); + const gateSkipRows = await env.DB.prepare("select metadata_json from audit_events where event_type = ? and target_key = ?") + .bind("github_app.ai_review_public_gate_skipped", "JSONbored/gittensory#11") + .all<{ metadata_json: string }>(); + expect(gateSkipRows.results).toHaveLength(1); + expect(JSON.parse(gateSkipRows.results[0]?.metadata_json ?? "{}")).toMatchObject({ reason: "paused" }); + const trackingRow = await env.DB.prepare("select count(*) as n from active_review_tracking where repo_full_name = ? and pull_number = ?") + .bind("JSONbored/gittensory", 11) + .first<{ n: number }>(); + expect(trackingRow?.n).toBe(0); + }); + it("publishes a non-cacheable AI-unavailable note when no reviewer returns usable output", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), diff --git a/test/unit/reputation-wiring.test.ts b/test/unit/reputation-wiring.test.ts index a5882922d8..79cf2948ce 100644 --- a/test/unit/reputation-wiring.test.ts +++ b/test/unit/reputation-wiring.test.ts @@ -217,6 +217,7 @@ describe("reputation check threaded from caller to callee, not re-derived (#4507 const before = spy.mock.calls.length; const willRun = await shouldStartAiReviewForAdvisory(env, { + mode: "live", settings: { aiReviewMode: "advisory" } as RepositorySettings, advisory: adv, repoFullName: "acme/widgets",