From d9c9cf57c190c0ece2fb48955ca0dfbf88417924 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 2 Jul 2026 19:32:24 -0700 Subject: [PATCH 1/2] fix(review): bound AI review re-spend and public-surface republish on unchanged heads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scheduled re-gate sweeps were re-spending a full AI review on every pass for a PR whose outcome landed in a non-cacheable state (consensus defect / inconclusive / a dynamic-context repo with grounding or RAG enabled) — the durable ai_review_cache correctly never stores those outcomes, so nothing throttled the retry. Root-caused in production: one PR generated 281 AI review calls in 24h at an unchanged head, ~92% of it from the RAG-active unconditional-bypass path. - ai_review_cache gains a `cacheable` column; a non-cacheable outcome (and a dynamic-context result, now bounded rather than unconditionally bypassing the cache) is still persisted for a 30-minute cooldown reuse, never as a durable hit. A lock-contention placeholder is still never persisted at all. - New audit events + counters: ai_review_cache_hit/miss/write_error, ai_review_non_cacheable, agent.sweep.regate_ai_skipped_current, github_app.public_surface_publish_skipped_current. A cache write failure is now observable instead of a silent catch. - A narrow public-surface no-op guard skips republishing a check-run-only repo's completed check when nothing provably changed since the last pass (head match + a live-verified completed check run + no pending refresh signal), falling through to a full republish on any doubt. - `agent-regate-pr` jobs carry an optional `force` flag that bypasses both the cache and the cooldown for an explicit manual re-gate. Validated against production Postgres audit_events/ai_usage_events data for the incident repo/PR before and during the fix. --- migrations/0098_ai_review_cache_cacheable.sql | 9 + src/db/repositories.ts | 35 +- src/db/schema.ts | 5 + src/queue/processors.ts | 173 ++++- src/types.ts | 5 + test/unit/ai-review-cache.test.ts | 149 +++++ test/unit/queue.test.ts | 626 +++++++++++++++++- 7 files changed, 976 insertions(+), 26 deletions(-) create mode 100644 migrations/0098_ai_review_cache_cacheable.sql diff --git a/migrations/0098_ai_review_cache_cacheable.sql b/migrations/0098_ai_review_cache_cacheable.sql new file mode 100644 index 0000000000..852f066316 --- /dev/null +++ b/migrations/0098_ai_review_cache_cacheable.sql @@ -0,0 +1,9 @@ +-- #regate-churn: the AI review cache (#1462) previously only ever stored genuinely reusable ("cacheable") +-- reviews -- a consensus-defect / inconclusive / lock-contention outcome was deliberately never written, so +-- nothing stopped a scheduled re-gate sweep from re-spending a real LLM call on the SAME head+fingerprint on +-- every single pass while a PR sat in that non-cacheable state (observed: one PR generated 281 AI review +-- calls in 24h against a single, never-reusable head). `cacheable` lets a non-cacheable outcome be PERSISTED +-- (for a bounded-cooldown reuse that throttles retries without ever being trusted indefinitely) while the +-- existing indefinite-hit path (cacheable = 1) is completely unaffected -- see getCachedAiReview's new +-- allowNonCacheable/maxAgeMs option in src/db/repositories.ts. +ALTER TABLE ai_review_cache ADD COLUMN cacheable INTEGER NOT NULL DEFAULT 1; diff --git a/src/db/repositories.ts b/src/db/repositories.ts index d91fe158c7..6c634c2ac8 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -3562,7 +3562,14 @@ export async function persistAdvisory(env: Env, advisory: Advisory): Promise | undefined } | null> { if (!headSha) return null; const row = await env.DB - .prepare("SELECT notes, reviewer_count AS reviewerCount, ai_review_mode AS mode, findings_json AS findingsJson, metadata_json AS metadataJson FROM ai_review_cache WHERE repo_full_name = ? AND pull_number = ? AND head_sha = ?") + .prepare("SELECT notes, reviewer_count AS reviewerCount, ai_review_mode AS mode, findings_json AS findingsJson, metadata_json AS metadataJson, cacheable, created_at AS createdAt FROM ai_review_cache WHERE repo_full_name = ? AND pull_number = ? AND head_sha = ?") .bind(repoFullName, pullNumber, headSha) - .first<{ notes: string; reviewerCount: number; mode: string; findingsJson: string | null; metadataJson: string | null }>(); + .first<{ notes: string; reviewerCount: number; mode: string; findingsJson: string | null; metadataJson: string | null; cacheable: number; createdAt: string }>(); if (!row || row.mode !== mode) return null; + if (row.cacheable !== 1) { + if (!options?.allowNonCacheable) return null; + const ageMs = Date.now() - Date.parse(row.createdAt); + if (!Number.isFinite(ageMs) || ageMs < 0 || ageMs > (options.maxAgeMs ?? 0)) return null; + } const metadata = parseJson>(row.metadataJson, {}); if ( expectedInputFingerprint !== undefined && @@ -3591,25 +3604,29 @@ export async function getCachedAiReview( }; } -/** Upsert the AI review for (repo, pull, head SHA). A nullish head SHA is a no-op. */ +/** Upsert the AI review for (repo, pull, head SHA). A nullish head SHA is a no-op. + * #regate-churn: `review.cacheable === false` still PERSISTS the attempt (so a repeated scheduled sweep pass at + * the identical head+fingerprint can find it via getCachedAiReview's bounded allowNonCacheable lookup) but marks + * it non-durable — omitted or any other value defaults to cacheable (1), the pre-existing behavior. */ export async function putCachedAiReview( env: Env, repoFullName: string, pullNumber: number, headSha: string | null | undefined, mode: string, - review: { notes: string; reviewerCount: number; findings?: AdvisoryFinding[]; metadata?: Record | undefined }, + review: { notes: string; reviewerCount: number; findings?: AdvisoryFinding[]; metadata?: Record | undefined; cacheable?: boolean | undefined }, ): Promise { if (!headSha) return; const createdAt = nowIso(); + const cacheable = review.cacheable === false ? 0 : 1; await env.DB .prepare( - `INSERT INTO ai_review_cache (repo_full_name, pull_number, head_sha, ai_review_mode, notes, reviewer_count, findings_json, metadata_json, created_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) + `INSERT INTO ai_review_cache (repo_full_name, pull_number, head_sha, ai_review_mode, notes, reviewer_count, findings_json, metadata_json, cacheable, created_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT(repo_full_name, pull_number, head_sha) DO UPDATE SET - ai_review_mode = excluded.ai_review_mode, notes = excluded.notes, reviewer_count = excluded.reviewer_count, findings_json = excluded.findings_json, metadata_json = excluded.metadata_json, created_at = excluded.created_at`, + ai_review_mode = excluded.ai_review_mode, notes = excluded.notes, reviewer_count = excluded.reviewer_count, findings_json = excluded.findings_json, metadata_json = excluded.metadata_json, cacheable = excluded.cacheable, created_at = excluded.created_at`, ) - .bind(repoFullName, pullNumber, headSha, mode, review.notes, review.reviewerCount, jsonString(review.findings ?? []), jsonString(review.metadata ?? {}), createdAt) + .bind(repoFullName, pullNumber, headSha, mode, review.notes, review.reviewerCount, jsonString(review.findings ?? []), jsonString(review.metadata ?? {}), cacheable, createdAt) .run(); } diff --git a/src/db/schema.ts b/src/db/schema.ts index ddc534eb1c..d3aee85fa8 100644 --- a/src/db/schema.ts +++ b/src/db/schema.ts @@ -1193,6 +1193,11 @@ export const aiReviewCache = sqliteTable( reviewerCount: integer("reviewer_count").notNull(), findingsJson: text("findings_json").notNull().default("[]"), metadataJson: text("metadata_json").notNull().default("{}"), + // #regate-churn: 1 (default) = a genuine, indefinitely-reusable review; 0 = a non-cacheable outcome + // (consensus defect / inconclusive / lock-contention placeholder) that is still PERSISTED so a repeated + // scheduled sweep pass at the identical head+fingerprint can reuse it for a bounded cooldown instead of + // re-spending an LLM call on every tick, without ever being treated as a durable, indefinitely-trustworthy hit. + cacheable: integer("cacheable").notNull().default(1), createdAt: text("created_at").notNull().$defaultFn(() => nowIso()), }, (table) => ({ diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 95d3ac7a16..481b4f2f11 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -867,6 +867,7 @@ export async function processJob(env: Env, message: JobMessage): Promise { message.prNumber, message.installationId, message.deliveryId, + message.force, ); return; case "run-agent": @@ -1530,6 +1531,7 @@ async function regatePullRequest( prNumber: number, installationId: number, deliveryId: string, + force?: boolean, ): Promise { // Reserve installation rate-limit headroom for real webhooks (#audit-rate-headroom): all repos share ONE GitHub // App installation = ONE REST bucket, so when the shared budget is at/below the maintenance floor, DEFER this @@ -1547,6 +1549,7 @@ async function regatePullRequest( repoFullName, prNumber, installationId, + ...(force ? { force: true } : {}), }, { delaySeconds: delayUntil(rateResetAt) }, ); @@ -1562,9 +1565,11 @@ async function regatePullRequest( undefined, // Run the AI review on the sweep for BOTH advisory and block modes (#sweep-all-modes) — only skip when AI is // OFF. The #1462 per-(repo,pr,headSha,mode) cache bounds the cost: an unchanged PR re-gates from cache with no - // re-spend, so an advisory PR gets a posted review without burning a token every sweep tick. + // re-spend, so an advisory PR gets a posted review without burning a token every sweep tick. `force` (#regate- + // churn req 8) bypasses that cache/cooldown reuse entirely for an explicit manual re-gate request. { skipAiReview: settings.aiReviewMode === "off", + ...(force ? { force: true } : {}), }, ).catch((error) => { /* v8 ignore next -- retryable/rate-limit propagation is exercised by queue retry tests; this catch only preserves that contract. */ @@ -2218,7 +2223,7 @@ async function reReviewStoredPullRequest( repoFullName: string, prNumber: number, previewPollAttempt?: number, - options: { skipAiReview?: boolean } = {}, + options: { skipAiReview?: boolean; force?: boolean } = {}, ): Promise { const [repo, settings] = await Promise.all([ getRepository(env, repoFullName), @@ -2365,6 +2370,8 @@ async function reReviewStoredPullRequest( liveFacts, ...(previewPollAttempt !== undefined ? { previewPollAttempt } : {}), ...(options.skipAiReview ? { skipAiReview: true } : {}), + ...(options.force ? { forceAiReview: true } : {}), + hasPendingRefreshSignal: otherRefreshReasons || reviewsCacheStale, }, ), ).catch((error) => { @@ -2954,6 +2961,21 @@ async function claimTransientLock( // head SHA + mode, not just PR) and a much longer TTL (an LLM call legitimately runs far longer than a close). const AI_REVIEW_LOCK_TTL_SECONDS = 1_800; // 30 minutes — see justification below. +// #regate-churn: how long a non-durably-cacheable AI review outcome may be reused by a scheduled re-gate at the +// IDENTICAL head+fingerprint+mode before a fresh LLM call is paid for again. Covers TWO distinct non-cacheable +// sources, both of which used to have NO retry bound at all: (1) a genuine non-cacheable verdict (consensus +// defect / inconclusive / lock-contention placeholder) that the durable cache (see #1 above) correctly never +// stores as a reusable result, and (2) a dynamic-context repo (grounding/RAG/enrichment/reputation), which +// previously bypassed the cache unconditionally on every single call. Root-caused in production: a single PR +// with RAG enabled generated 259 of 281 AI review calls in 24h via (2) at an UNCHANGED head, plus another 24 via +// (1) — 281 calls total, ~1 every 5 minutes, forever, with nothing ever throttling the retry. This bounds that +// retry cadence without ever treating either outcome as a durable, indefinitely-trustworthy result — it still +// expires and retries periodically (the LLM's own non-determinism may resolve a dispute; dynamic external +// context may genuinely have drifted), and any REAL state change (a new head, a changed review-input +// fingerprint) bypasses this bound immediately regardless of age. Matches AI_REVIEW_LOCK_TTL_SECONDS's +// 30-minute order of magnitude — same "crash/dispute backstop, not a throughput bound" philosophy. +const AI_REVIEW_NON_CACHEABLE_RETRY_COOLDOWN_MS = 30 * 60 * 1000; + function aiReviewLockKey(repoFullName: string, prNumber: number, headSha: string, mode: string): string { return `ai-review-lock:${repoFullName.toLowerCase()}#${prNumber}@${headSha.toLowerCase()}:${mode}`; } @@ -5149,6 +5171,13 @@ export async function runAiReviewForAdvisory( findings: AdvisoryFinding[]; metadata?: Record | undefined; cacheable?: boolean | undefined; + // #regate-churn: distinct from `cacheable` — false ONLY for the lock-contention placeholder below (another + // pass is concurrently reviewing this exact head RIGHT NOW). That placeholder describes a transient + // scheduling race, not a real AI opinion, and the concurrent pass it deferred to will itself persist the + // real result within seconds — so it must never be written at all (not even non-durably), or a later read + // within the bounded cooldown could replay "another pass is running" long after that pass finished. + // Defaults to true (persistable) for every other outcome, cacheable or not. + persistable?: boolean | undefined; } | undefined > { @@ -5241,6 +5270,7 @@ export async function runAiReviewForAdvisory( inlineFindings: [], findings, cacheable: false, + persistable: false, }; } try { @@ -5850,6 +5880,17 @@ async function maybePublishPrPublicSurface( baseSha?: string | null | undefined; previewPollAttempt?: number | undefined; skipAiReview?: boolean | undefined; + // #regate-churn (req 8): an explicit manual re-gate can force a fresh AI opinion, bypassing BOTH the durable + // cache and the bounded non-cacheable-reuse cooldown. No current caller sets this — it exists so a future + // manual-trigger path (or a caller that already knows something changed) has a supported way to opt out of + // the reuse guards below rather than fighting them. + forceAiReview?: boolean | undefined; + // #regate-churn (req 6/7): true when the caller ALREADY determined something besides the AI review itself + // may need a fresh look this pass (slop evidence collection, the manifest gate, a pre-merge-check refresh, or + // a stale reviews-data cache — see reReviewStoredPullRequest's otherRefreshReasons/reviewsCacheStale). The + // public-surface no-op guard below only fires when this is false — any of those signals means something + // besides the head SHA could make the published output differ from what is already live. + hasPendingRefreshSignal?: boolean | undefined; liveFacts: LiveGithubFacts; }, ): Promise | undefined> { @@ -6044,10 +6085,12 @@ async function maybePublishPrPublicSurface( findings?: AdvisoryFinding[]; metadata?: Record | undefined; cacheable?: boolean | undefined; + persistable?: boolean | undefined; } | undefined; let inlineCommentsEnabledForReview = false; let aiReviewExpected = false; + let aiReviewWasReused = false; let gateFinalized = false; const publishedOutputs: PublicSurfaceOutput[] = []; const failedOutputs: PublicSurfaceOutputFailure[] = []; @@ -6588,10 +6631,24 @@ async function maybePublishPrPublicSurface( // mode, reviewer plan, feature activation, or prompt-shaping inputs change. A re-delivered webhook or the // block-mode re-gate sweep can reuse that exact review; stale same-head reviews from older private review // instructions or feature config are intentionally treated as misses. The deterministic gate still runs. - // A repo with an active dynamic-context feature (grounding/RAG/enrichment/reputation) bypasses the - // cache entirely — see dynamicReviewContextActive above — since a cache hit there could replay a - // review built against now-stale external context for an otherwise-unchanged head. - const cachedReview = dynamicReviewContextActive + // `webhook.forceAiReview` (a manual re-gate, if the caller opts in) bypasses the cache entirely: the + // caller is explicitly asking for a fresh opinion, not a replayed one. + // + // #regate-churn (root cause, confirmed in production): a repo with an active dynamic-context feature + // (grounding/RAG/enrichment/reputation) used to bypass the cache UNCONDITIONALLY on every single call, + // on the theory that TIME-VARYING external context (the vector index, REES/CVE data, evolving + // reputation) can drift for the SAME head SHA without any of these booleans flipping, and fingerprinting + // only "is the feature on" can't detect that drift without fetching the content itself. That reasoning + // is right for a genuinely time-sensitive re-check, but a live incident showed it also means a + // dynamic-context repo re-spends an LLM call on EVERY scheduled sweep tick forever, with no bound at + // all: one PR with RAG enabled generated 259 of 281 AI review calls in 24h this way, at an UNCHANGED + // head. A dynamic-context result is therefore now always written non-durably (cacheable=false, same as + // a consensus-defect/inconclusive outcome below) rather than not written at all, so it can ALSO be + // reused for a bounded cooldown (AI_REVIEW_NON_CACHEABLE_RETRY_COOLDOWN_MS) — long enough to collapse a + // sweep tick's worth of redundant calls into one, short enough that genuinely drifted external context + // is still picked up well within the hour. A genuinely cacheable, non-dynamic-context row is unaffected + // (unbounded reuse, exactly as before this fix). + const cachedReview = webhook.forceAiReview === true ? null : await getCachedAiReview( env, @@ -6600,11 +6657,40 @@ async function maybePublishPrPublicSurface( advisory.headSha, settings.aiReviewMode, inputFingerprint, + { allowNonCacheable: true, maxAgeMs: AI_REVIEW_NON_CACHEABLE_RETRY_COOLDOWN_MS }, ).catch(() => null); if (cachedReview && hasPublicReviewAssessment(cachedReview.notes)) { advisory.findings.push(...cachedReview.findings); aiReview = cachedReview; + aiReviewWasReused = true; + incr("gittensory_ai_review_cache_hit_total"); + await recordAuditEvent(env, { + eventType: "github_app.ai_review_cache_hit", + actor: author, + targetKey: `${repoFullName}#${pr.number}`, + outcome: "completed", + detail: "reused a stored AI review instead of re-spending an LLM call", + metadata: { deliveryId: webhook.deliveryId, repoFullName, /* v8 ignore next -- reached only inside aiReviewWillRun (which requires a truthy advisory.headSha) or the publish-skip guard's own `advisory.headSha &&` check; the `?? null` is a type-level fallback for an unreachable branch. */ headSha: advisory.headSha ?? null }, + }).catch(() => undefined); + await recordAuditEvent(env, { + eventType: "agent.sweep.regate_ai_skipped_current", + actor: author, + targetKey: `${repoFullName}#${pr.number}`, + outcome: "completed", + detail: "AI review already current for this head+fingerprint; skipped re-review", + metadata: { deliveryId: webhook.deliveryId, repoFullName, /* v8 ignore next -- reached only inside aiReviewWillRun (which requires a truthy advisory.headSha) or the publish-skip guard's own `advisory.headSha &&` check; the `?? null` is a type-level fallback for an unreachable branch. */ headSha: advisory.headSha ?? null }, + }).catch(() => undefined); + incr("gittensory_regate_ai_skipped_current_total"); } else { + incr("gittensory_ai_review_cache_miss_total"); + await recordAuditEvent(env, { + eventType: "github_app.ai_review_cache_miss", + actor: author, + targetKey: `${repoFullName}#${pr.number}`, + outcome: "completed", + detail: "no reusable stored AI review for this head+fingerprint; running a fresh review", + metadata: { deliveryId: webhook.deliveryId, repoFullName, /* v8 ignore next -- reached only inside aiReviewWillRun (which requires a truthy advisory.headSha) or the publish-skip guard's own `advisory.headSha &&` check; the `?? null` is a type-level fallback for an unreachable branch. */ headSha: advisory.headSha ?? null }, + }).catch(() => undefined); aiReview = await runAiReviewForAdvisory(env, { settings, advisory, @@ -6620,7 +6706,26 @@ async function maybePublishPrPublicSurface( reviewExcludePaths, reviewInlineComments, }); - if (aiReview && aiReview.cacheable !== false && !dynamicReviewContextActive) + // `persistable === false` (only the lock-contention placeholder — see runAiReviewForAdvisory's return + // type doc comment) is excluded from EVERY write, not just the durable one: it describes a transient + // scheduling race, not a real AI opinion, and the concurrent pass it deferred to persists the real + // result within seconds — writing this placeholder (even non-durably) could replay a stale "another + // pass is running" message for the rest of the cooldown window, well after that race resolved. + if (aiReview && aiReview.persistable !== false) { + // A dynamic-context result is never durably cacheable (see the comment above); otherwise defer to + // the review's own verdict (consensus defect / inconclusive → false). + const cacheableForStorage = !dynamicReviewContextActive && aiReview.cacheable !== false; + if (!cacheableForStorage) { + incr("gittensory_ai_review_non_cacheable_total"); + await recordAuditEvent(env, { + eventType: "github_app.ai_review_non_cacheable", + actor: author, + targetKey: `${repoFullName}#${pr.number}`, + outcome: "completed", + detail: "AI review outcome is not durably cacheable; persisted for bounded-cooldown reuse only", + metadata: { deliveryId: webhook.deliveryId, repoFullName, /* v8 ignore next -- reached only inside aiReviewWillRun (which requires a truthy advisory.headSha) or the publish-skip guard's own `advisory.headSha &&` check; the `?? null` is a type-level fallback for an unreachable branch. */ headSha: advisory.headSha ?? null }, + }).catch(() => undefined); + } await putCachedAiReview( env, repoFullName, @@ -6629,13 +6734,27 @@ async function maybePublishPrPublicSurface( settings.aiReviewMode, { ...aiReview, + cacheable: cacheableForStorage, metadata: { /* v8 ignore next -- runAiReviewForAdvisory (the sole path reaching here) always sets metadata on its "ok" returns; the nullish fallback is a type-level (optional field) safeguard, not a reachable runtime path. */ ...(aiReview.metadata ?? {}), inputFingerprint, }, }, - ).catch(() => undefined); + ).catch((error) => { + // #regate-churn (req 3/9): a swallowed write failure here is exactly how the cache goes silently + // stale in production — make it observable instead of a bare no-op catch. + incr("gittensory_ai_review_cache_write_error_total"); + return recordAuditEvent(env, { + eventType: "github_app.ai_review_cache_write_error", + actor: author, + targetKey: `${repoFullName}#${pr.number}`, + outcome: "error", + detail: errorMessage(error), + metadata: { deliveryId: webhook.deliveryId, repoFullName, /* v8 ignore next -- reached only inside aiReviewWillRun (which requires a truthy advisory.headSha) or the publish-skip guard's own `advisory.headSha &&` check; the `?? null` is a type-level fallback for an unreachable branch. */ headSha: advisory.headSha ?? null }, + }).catch(() => undefined); + }); + } } }, ); @@ -6806,6 +6925,44 @@ async function maybePublishPrPublicSurface( reasonCode, }); } + // #regate-churn (req 6/7): a public-surface no-op guard, deliberately narrow. markPullRequestSurfacePublished's + // own doc comment warns lastPublishedSurfaceSha is "reporting/diagnostic state, not a hard scheduled-sweep + // skip" because a comment can be stale or partial even when the head marker matches — so this ONLY applies to + // a check-run-only repo (publicSurface "off": no comment, no label ever published, nothing else that marker + // can't prove current) with an independently-verified COMPLETED check run at the exact current head, no + // pending refresh signal (slop evidence / manifest gate / pre-merge-check / reviews-cache staleness — see + // hasPendingRefreshSignal), and an AI review dimension that is either not in play or was itself reused rather + // than freshly computed. Any doubt on any of these falls through to the full, unconditional publish below — + // this guard is only ever allowed to skip a PROVABLE no-op, never to guess one. + if ( + gateEnabled && + settings.publicSurface === "off" && + !webhook.hasPendingRefreshSignal && + !webhook.forceAiReview && + (!aiReviewWillRun || aiReviewWasReused) && + advisory.headSha && + advisory.headSha === pr.lastPublishedSurfaceSha + ) { + const existingChecks = await listCheckSummaries(env, repoFullName, pr.number).catch(() => []); + const currentGateCheck = existingChecks.find( + (check) => + check.name === GITTENSORY_GATE_CHECK_NAME && + check.headSha === advisory.headSha && + check.status === "completed", + ); + if (currentGateCheck) { + incr("gittensory_public_surface_publish_skipped_current_total"); + await recordAuditEvent(env, { + eventType: "github_app.public_surface_publish_skipped_current", + actor: author, + targetKey: `${repoFullName}#${pr.number}`, + outcome: "completed", + detail: "public surface already current for this head; skipped republish", + metadata: { deliveryId: webhook.deliveryId, repoFullName, /* v8 ignore next -- reached only inside aiReviewWillRun (which requires a truthy advisory.headSha) or the publish-skip guard's own `advisory.headSha &&` check; the `?? null` is a type-level fallback for an unreachable branch. */ headSha: advisory.headSha ?? null }, + }).catch(() => undefined); + return gateEvaluation; + } + } const finalFreshness = await freshnessForReviewOutput("final_publish"); if (await skipStaleReviewOutput(finalFreshness)) { return undefined; diff --git a/src/types.ts b/src/types.ts index a5d6fe11d4..cb6c29aade 100644 --- a/src/types.ts +++ b/src/types.ts @@ -33,6 +33,11 @@ export type JobMessage = repoFullName: string; prNumber: number; installationId: number; + // #regate-churn (req 8): an explicit manual re-gate request — bypasses the AI review cache and the + // bounded non-cacheable-reuse cooldown so it always pays for a fresh opinion. No current scheduled or + // webhook-driven caller sets this; it exists so a manual trigger has a supported way to force a fresh + // pass instead of reusing a recent (possibly disputed) result. + force?: boolean | undefined; } | { type: "refresh-registry"; diff --git a/test/unit/ai-review-cache.test.ts b/test/unit/ai-review-cache.test.ts index f507ee6cc5..7e4efb7a8a 100644 --- a/test/unit/ai-review-cache.test.ts +++ b/test/unit/ai-review-cache.test.ts @@ -162,4 +162,153 @@ describe("AI review cache (#1)", () => { metadata: { inputFingerprint: matching }, }); }); + + describe("non-cacheable rows (#regate-churn bounded-cooldown reuse)", () => { + it("defaults a row to cacheable when review.cacheable is omitted (unchanged behavior)", async () => { + const env = createTestEnv(); + await putCachedAiReview(env, "o/r", 20, "sha1", "block", { notes: "clean review", reviewerCount: 1 }); + const row = await env.DB.prepare("SELECT cacheable FROM ai_review_cache WHERE repo_full_name = ? AND pull_number = ? AND head_sha = ?") + .bind("o/r", 20, "sha1") + .first<{ cacheable: number }>(); + expect(row?.cacheable).toBe(1); + expect(await getCachedAiReview(env, "o/r", 20, "sha1", "block")).toEqual({ notes: "clean review", reviewerCount: 1, findings: [] }); + }); + + it("persists a non-cacheable outcome but the STRICT read (no options) still misses it, same as before this column existed", async () => { + const env = createTestEnv(); + await putCachedAiReview(env, "o/r", 21, "sha1", "block", { notes: "consensus defect", reviewerCount: 2, cacheable: false }); + const row = await env.DB.prepare("SELECT cacheable FROM ai_review_cache WHERE repo_full_name = ? AND pull_number = ? AND head_sha = ?") + .bind("o/r", 21, "sha1") + .first<{ cacheable: number }>(); + expect(row?.cacheable).toBe(0); // the attempt WAS persisted + expect(await getCachedAiReview(env, "o/r", 21, "sha1", "block")).toBeNull(); // but never a durable hit + }); + + it("misses a non-cacheable row when the caller does not opt into allowNonCacheable", async () => { + const env = createTestEnv(); + await putCachedAiReview(env, "o/r", 22, "sha1", "block", { notes: "held", reviewerCount: 1, cacheable: false }); + expect(await getCachedAiReview(env, "o/r", 22, "sha1", "block", undefined, {})).toBeNull(); + }); + + it("reuses a non-cacheable row within the cooldown when allowNonCacheable + maxAgeMs are given", async () => { + const env = createTestEnv(); + vi.useFakeTimers(); + try { + vi.setSystemTime(new Date("2026-07-01T00:00:00.000Z")); + await putCachedAiReview(env, "o/r", 23, "sha1", "block", { notes: "consensus defect", reviewerCount: 2, cacheable: false }); + + vi.setSystemTime(new Date("2026-07-01T00:10:00.000Z")); // 10 minutes later, within a 30-minute cooldown + expect( + await getCachedAiReview(env, "o/r", 23, "sha1", "block", undefined, { allowNonCacheable: true, maxAgeMs: 30 * 60 * 1000 }), + ).toEqual({ notes: "consensus defect", reviewerCount: 2, findings: [] }); + } finally { + vi.useRealTimers(); + } + }); + + it("falls through to a miss once a non-cacheable row ages past maxAgeMs", async () => { + const env = createTestEnv(); + vi.useFakeTimers(); + try { + vi.setSystemTime(new Date("2026-07-01T00:00:00.000Z")); + await putCachedAiReview(env, "o/r", 24, "sha1", "block", { notes: "consensus defect", reviewerCount: 2, cacheable: false }); + + vi.setSystemTime(new Date("2026-07-01T00:31:00.000Z")); // 31 minutes later, past a 30-minute cooldown + expect( + await getCachedAiReview(env, "o/r", 24, "sha1", "block", undefined, { allowNonCacheable: true, maxAgeMs: 30 * 60 * 1000 }), + ).toBeNull(); + } finally { + vi.useRealTimers(); + } + }); + + it("a genuinely cacheable row is unaffected by allowNonCacheable/maxAgeMs (unbounded reuse, as before)", async () => { + const env = createTestEnv(); + vi.useFakeTimers(); + try { + vi.setSystemTime(new Date("2026-07-01T00:00:00.000Z")); + await putCachedAiReview(env, "o/r", 25, "sha1", "block", { notes: "clean review", reviewerCount: 1, cacheable: true }); + + vi.setSystemTime(new Date("2026-08-01T00:00:00.000Z")); // a month later — far past any non-cacheable cooldown + expect( + await getCachedAiReview(env, "o/r", 25, "sha1", "block", undefined, { allowNonCacheable: true, maxAgeMs: 30 * 60 * 1000 }), + ).toEqual({ notes: "clean review", reviewerCount: 1, findings: [] }); + } finally { + vi.useRealTimers(); + } + }); + + it("still enforces the mode + input-fingerprint match on a non-cacheable reuse", async () => { + const env = createTestEnv(); + await putCachedAiReview(env, "o/r", 26, "sha1", "block", { + notes: "held", + reviewerCount: 1, + cacheable: false, + metadata: { inputFingerprint: "fp-v1" }, + }); + const opts = { allowNonCacheable: true, maxAgeMs: 30 * 60 * 1000 }; + expect(await getCachedAiReview(env, "o/r", 26, "sha1", "advisory", undefined, opts)).toBeNull(); // mode mismatch + expect(await getCachedAiReview(env, "o/r", 26, "sha1", "block", "fp-v2", opts)).toBeNull(); // fingerprint mismatch + expect(await getCachedAiReview(env, "o/r", 26, "sha1", "block", "fp-v1", opts)).toEqual({ + notes: "held", + reviewerCount: 1, + findings: [], + metadata: { inputFingerprint: "fp-v1" }, + }); + }); + + it("treats a missing maxAgeMs as a zero-width cooldown (any elapsed time is stale)", async () => { + const env = createTestEnv(); + vi.useFakeTimers(); + try { + vi.setSystemTime(new Date("2026-07-01T00:00:00.000Z")); + await putCachedAiReview(env, "o/r", 28, "sha1", "block", { notes: "held", reviewerCount: 1, cacheable: false }); + + vi.setSystemTime(new Date("2026-07-01T00:00:01.000Z")); // 1 second later + expect(await getCachedAiReview(env, "o/r", 28, "sha1", "block", undefined, { allowNonCacheable: true })).toBeNull(); + } finally { + vi.useRealTimers(); + } + }); + + it("fails closed (treats as stale) when the elapsed age is negative — a clock-skewed created_at", async () => { + const env = createTestEnv(); + vi.useFakeTimers(); + try { + vi.setSystemTime(new Date("2026-07-01T00:00:00.000Z")); + await putCachedAiReview(env, "o/r", 29, "sha1", "block", { notes: "held", reviewerCount: 1, cacheable: false }); + + vi.setSystemTime(new Date("2026-06-30T23:59:00.000Z")); // "now" moved BEFORE the row's created_at + expect( + await getCachedAiReview(env, "o/r", 29, "sha1", "block", undefined, { allowNonCacheable: true, maxAgeMs: 30 * 60 * 1000 }), + ).toBeNull(); + } finally { + vi.useRealTimers(); + } + }); + + it("fails closed (treats as stale) when created_at cannot be parsed as a date", async () => { + const env = createTestEnv(); + await putCachedAiReview(env, "o/r", 30, "sha1", "block", { notes: "held", reviewerCount: 1, cacheable: false }); + await env.DB.prepare("UPDATE ai_review_cache SET created_at = ? WHERE repo_full_name = ? AND pull_number = ? AND head_sha = ?") + .bind("not-a-date", "o/r", 30, "sha1") + .run(); + expect( + await getCachedAiReview(env, "o/r", 30, "sha1", "block", undefined, { allowNonCacheable: true, maxAgeMs: 30 * 60 * 1000 }), + ).toBeNull(); + }); + + it("upserting a fresh cacheable review over a prior non-cacheable row makes it a durable hit again", async () => { + const env = createTestEnv(); + await putCachedAiReview(env, "o/r", 27, "sha1", "block", { notes: "consensus defect", reviewerCount: 2, cacheable: false }); + expect(await getCachedAiReview(env, "o/r", 27, "sha1", "block")).toBeNull(); + + await putCachedAiReview(env, "o/r", 27, "sha1", "block", { notes: "resolved, clean review", reviewerCount: 2, cacheable: true }); + expect(await getCachedAiReview(env, "o/r", 27, "sha1", "block")).toEqual({ + notes: "resolved, clean review", + reviewerCount: 2, + findings: [], + }); + }); + }); }); diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 19df52a122..a3f9bf3cb9 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -3,6 +3,7 @@ import { generateKeyPairSync } from "node:crypto"; import { clearInstallationTokenCacheForTest } from "../../src/github/app"; import { PR_PANEL_COMMENT_MARKER } from "../../src/github/comments"; import * as backfillModule from "../../src/github/backfill"; +import * as rateLimitModule from "../../src/github/rate-limit"; import * as repositoriesModule from "../../src/db/repositories"; import * as repositorySettingsModule from "../../src/settings/repository-settings"; import * as sentryModule from "../../src/selfhost/sentry"; @@ -2247,6 +2248,588 @@ describe("queue processors", () => { stampSpy.mockRestore(); }); + describe("#regate-churn: scheduled re-gate idempotency", () => { + async function seedRegateChurnRepo(env: Env, overrides: Partial[1]> = {}) { + await persistRegistrySnapshot( + 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 upsertInstallation(env, { action: "created", installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" }, target_type: "User", repository_selection: "selected", permissions: {}, events: [] } }); + await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 123); + await upsertRepositorySettings(env, { + repoFullName: "JSONbored/gittensory", + commentMode: "all_prs", + publicSurface: "off", + autoLabelEnabled: false, + checkRunMode: "off", + gateCheckMode: "enabled", + aiReviewMode: "block", + gatePack: "oss-anti-slop", + ...overrides, + }); + await upsertOfficialMinerDetection(env, "contributor", { status: "confirmed", snapshot: queueMinerSnapshot("contributor") }, 60_000); + } + + it("#9: a scheduled sweep does not call AI twice for a non-cacheable outcome at an unchanged head (reproduces the 281-calls/24h incident)", async () => { + let aiCalls = 0; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => { aiCalls += 1; return { response: "not-json" }; } } as unknown as Ai, // inconclusive → non-cacheable + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await seedRegateChurnRepo(env); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 60, title: "Quiet PR", state: "open", user: { login: "contributor" }, head: { sha: "a60" }, labels: [], body: "Closes #1" }); + await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 60, status: "complete", reviewsSyncedAt: new Date().toISOString() }); + 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/60/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/60")) return Response.json({ number: 60, title: "Quiet PR", state: "open", user: { login: "contributor" }, head: { sha: "a60" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/commits/a60/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/a60/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/60/comments")) return method === "POST" ? Response.json({ id: 60 }, { status: 201 }) : 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({}); + }); + vi.setSystemTime(new Date("2026-05-28T02:00:00.000Z")); + + // Three scheduled sweep passes over the SAME unchanged head, minutes apart — exactly the low-activity-repo + // shape from the production incident (repeated sweep ticks, no real state change). + await processJob(env, { type: "agent-regate-pr", deliveryId: "churn-1", repoFullName: "JSONbored/gittensory", prNumber: 60, installationId: 123 }); + const firstRunAiCalls = aiCalls; + expect(firstRunAiCalls).toBeGreaterThan(0); + vi.setSystemTime(new Date("2026-05-28T02:05:00.000Z")); + await processJob(env, { type: "agent-regate-pr", deliveryId: "churn-2", repoFullName: "JSONbored/gittensory", prNumber: 60, installationId: 123 }); + vi.setSystemTime(new Date("2026-05-28T02:10:00.000Z")); + await processJob(env, { type: "agent-regate-pr", deliveryId: "churn-3", repoFullName: "JSONbored/gittensory", prNumber: 60, installationId: 123 }); + + expect(aiCalls).toBe(firstRunAiCalls); // unchanged — the non-cacheable outcome was reused for both later passes + const skipAudit = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and target_key = ?") + .bind("agent.sweep.regate_ai_skipped_current", "JSONbored/gittensory#60") + .first<{ n: number }>(); + expect(skipAudit?.n).toBe(2); // churn-2 and churn-3 both skipped + }); + + it("#9: a cache write failure is observable via audit_events and metrics, not silently swallowed", async () => { + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => ({ response: JSON.stringify({ assessment: "Looks fine.", blockers: [], nits: [], suggestions: [] }) }) } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await seedRegateChurnRepo(env); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 61, title: "Clean PR", state: "open", user: { login: "contributor" }, head: { sha: "a61" }, labels: [], body: "Closes #1" }); + await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 61, status: "complete", reviewsSyncedAt: new Date().toISOString() }); + 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/61/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/61")) return Response.json({ number: 61, title: "Clean PR", state: "open", user: { login: "contributor" }, head: { sha: "a61" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/commits/a61/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/a61/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/61/comments")) return method === "POST" ? Response.json({ id: 61 }, { status: 201 }) : 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({}); + }); + const writeSpy = vi.spyOn(repositoriesModule, "putCachedAiReview").mockRejectedValueOnce(new Error("D1 write error")); + + await expect( + processJob(env, { type: "agent-regate-pr", deliveryId: "write-fail", repoFullName: "JSONbored/gittensory", prNumber: 61, installationId: 123 }), + ).resolves.toBeUndefined(); // the review still completes — a cache write failure is best-effort, never fatal + writeSpy.mockRestore(); + + const audit = await env.DB.prepare("select outcome, detail from audit_events where event_type = ? and target_key = ?") + .bind("github_app.ai_review_cache_write_error", "JSONbored/gittensory#61") + .first<{ outcome: string; detail: string }>(); + expect(audit?.outcome).toBe("error"); + expect(audit?.detail).toContain("D1 write error"); + }); + + it("swallows a failing hit/skip audit write without throwing (cache-hit path)", async () => { + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => ({ response: JSON.stringify({ assessment: "Looks fine.", blockers: [], nits: [], suggestions: [] }) }) } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await seedRegateChurnRepo(env); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 66, title: "Clean PR", state: "open", user: { login: "contributor" }, head: { sha: "a66" }, labels: [], body: "Closes #1" }); + await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 66, status: "complete", reviewsSyncedAt: new Date().toISOString() }); + await putCachedAiReview(env, "JSONbored/gittensory", 66, "a66", "block", { + notes: "Looks fine.", + reviewerCount: 1, + cacheable: true, + metadata: { + inputFingerprint: await aiReviewCacheInputFingerprint({ + title: "Clean PR", mode: "block", byok: false, provider: null, model: null, aiReviewAllAuthors: false, + aiReviewCloseConfidence: undefined, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, baseSha: null, + reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], + profile: null, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], changedPaths: ["src/a.ts"], + features: { grounding: false, rag: false, enrichment: false, reputation: false }, + }), + }, + }); + 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/66/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/66")) return Response.json({ number: 66, title: "Clean PR", state: "open", user: { login: "contributor" }, head: { sha: "a66" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/66/comments")) return method === "POST" ? Response.json({ id: 66 }, { status: 201 }) : 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({}); + }); + const originalRecordAuditEvent = repositoriesModule.recordAuditEvent; + const auditSpy = vi.spyOn(repositoriesModule, "recordAuditEvent").mockImplementation(async (auditEnv, event) => { + if (event.eventType === "github_app.ai_review_cache_hit" || event.eventType === "agent.sweep.regate_ai_skipped_current") + throw new Error("audit DB down"); + await originalRecordAuditEvent(auditEnv, event); + }); + + await expect( + processJob(env, { type: "agent-regate-pr", deliveryId: "hit-audit-fail", repoFullName: "JSONbored/gittensory", prNumber: 66, installationId: 123 }), + ).resolves.toBeUndefined(); + auditSpy.mockRestore(); + }); + + it("swallows failing miss/non-cacheable audit writes AND a failing write-error audit write, without throwing", async () => { + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => ({ response: "not-json" }) } as unknown as Ai, // inconclusive → non-cacheable + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await seedRegateChurnRepo(env); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 67, title: "Quiet PR", state: "open", user: { login: "contributor" }, head: { sha: "a67" }, labels: [], body: "Closes #1" }); + await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 67, status: "complete", reviewsSyncedAt: new Date().toISOString() }); + 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/67/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/67")) return Response.json({ number: 67, title: "Quiet PR", state: "open", user: { login: "contributor" }, head: { sha: "a67" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/67/comments")) return method === "POST" ? Response.json({ id: 67 }, { status: 201 }) : 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({}); + }); + const writeSpy = vi.spyOn(repositoriesModule, "putCachedAiReview").mockRejectedValue(new Error("D1 write error")); + const originalRecordAuditEvent = repositoriesModule.recordAuditEvent; + const auditSpy = vi.spyOn(repositoriesModule, "recordAuditEvent").mockImplementation(async (auditEnv, event) => { + if ( + event.eventType === "github_app.ai_review_cache_miss" || + event.eventType === "github_app.ai_review_non_cacheable" || + event.eventType === "github_app.ai_review_cache_write_error" + ) + throw new Error("audit DB down"); + await originalRecordAuditEvent(auditEnv, event); + }); + + await expect( + processJob(env, { type: "agent-regate-pr", deliveryId: "miss-audit-fail", repoFullName: "JSONbored/gittensory", prNumber: 67, installationId: 123 }), + ).resolves.toBeUndefined(); + writeSpy.mockRestore(); + auditSpy.mockRestore(); + }); + + it("#9: the public surface is not republished when already current at the head (check-run-only repo, req 6)", async () => { + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => ({ response: JSON.stringify({ assessment: "Looks fine.", blockers: [], nits: [], suggestions: [] }) }) } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await seedRegateChurnRepo(env); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 62, title: "Current PR", state: "open", user: { login: "contributor" }, head: { sha: "a62" }, labels: [], body: "Closes #1" }); + await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 62, status: "complete", reviewsSyncedAt: new Date().toISOString() }); + await putCachedAiReview(env, "JSONbored/gittensory", 62, "a62", "block", { + notes: "Looks fine.", + reviewerCount: 1, + cacheable: true, + metadata: { + inputFingerprint: await aiReviewCacheInputFingerprint({ + title: "Current PR", + mode: "block", + byok: false, + provider: null, + model: null, + aiReviewAllAuthors: false, + aiReviewCloseConfidence: undefined, + gatePack: "oss-anti-slop", + reviewerPlan: env.AI_REVIEW_PLAN, + selfHostProviderConfig: null, + baseSha: null, + reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], + profile: null, + inlineComments: false, + pathInstructions: [], + pathGuidance: "", + repoInstructions: null, + excludePaths: [], + changedPaths: ["src/a.ts"], + features: { grounding: false, rag: false, enrichment: false, reputation: false }, + }), + }, + }); + await upsertCheckSummary(env, { + id: "gate-62", + repoFullName: "JSONbored/gittensory", + pullNumber: 62, + headSha: "a62", + name: "Gittensory Orb Review Agent", + status: "completed", + conclusion: "success", + payload: {}, + }); + await repositoriesModule.markPullRequestSurfacePublished(env, "JSONbored/gittensory", 62, "a62"); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/pulls/62/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/62")) return Response.json({ number: 62, title: "Current PR", state: "open", user: { login: "contributor" }, head: { sha: "a62" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/commits/a62/status")) return Response.json({ state: "success", statuses: [] }); + 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 processJob(env, { type: "agent-regate-pr", deliveryId: "surface-skip", repoFullName: "JSONbored/gittensory", prNumber: 62, installationId: 123 }); + + const skipAudit = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and target_key = ?") + .bind("github_app.public_surface_publish_skipped_current", "JSONbored/gittensory#62") + .first<{ n: number }>(); + expect(skipAudit?.n).toBe(1); + const publishedAudit = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and target_key = ?") + .bind("github_app.pr_public_surface_published", "JSONbored/gittensory#62") + .first<{ n: number }>(); + expect(publishedAudit?.n).toBe(0); // the full publish path never ran — it was proven redundant up-front + }); + + it("swallows a failing publish-skip audit write without throwing", async () => { + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => ({ response: JSON.stringify({ assessment: "Looks fine.", blockers: [], nits: [], suggestions: [] }) }) } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await seedRegateChurnRepo(env); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 70, title: "Current PR", state: "open", user: { login: "contributor" }, head: { sha: "a70" }, labels: [], body: "Closes #1" }); + await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 70, status: "complete", reviewsSyncedAt: new Date().toISOString() }); + await putCachedAiReview(env, "JSONbored/gittensory", 70, "a70", "block", { + notes: "Looks fine.", + reviewerCount: 1, + cacheable: true, + metadata: { + inputFingerprint: await aiReviewCacheInputFingerprint({ + title: "Current PR", mode: "block", byok: false, provider: null, model: null, aiReviewAllAuthors: false, + aiReviewCloseConfidence: undefined, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, baseSha: null, + reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], + profile: null, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], changedPaths: ["src/a.ts"], + features: { grounding: false, rag: false, enrichment: false, reputation: false }, + }), + }, + }); + await upsertCheckSummary(env, { id: "gate-70", repoFullName: "JSONbored/gittensory", pullNumber: 70, headSha: "a70", name: "Gittensory Orb Review Agent", status: "completed", conclusion: "success", payload: {} }); + await repositoriesModule.markPullRequestSurfacePublished(env, "JSONbored/gittensory", 70, "a70"); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/pulls/70/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/70")) return Response.json({ number: 70, title: "Current PR", state: "open", user: { login: "contributor" }, head: { sha: "a70" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/commits/a70/status")) return Response.json({ state: "success", statuses: [] }); + 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({}); + }); + const originalRecordAuditEvent = repositoriesModule.recordAuditEvent; + const auditSpy = vi.spyOn(repositoriesModule, "recordAuditEvent").mockImplementation(async (auditEnv, event) => { + if (event.eventType === "github_app.public_surface_publish_skipped_current") throw new Error("audit DB down"); + await originalRecordAuditEvent(auditEnv, event); + }); + + await expect( + processJob(env, { type: "agent-regate-pr", deliveryId: "surface-skip-audit-fail", repoFullName: "JSONbored/gittensory", prNumber: 70, installationId: 123 }), + ).resolves.toBeUndefined(); + auditSpy.mockRestore(); + }); + + it("#6: falls through to a full republish when the surface marker matches but NO completed check run backs it up (partial-publish edge case)", async () => { + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => ({ response: JSON.stringify({ assessment: "Looks fine.", blockers: [], nits: [], suggestions: [] }) }) } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await seedRegateChurnRepo(env); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 69, title: "Partially published PR", state: "open", user: { login: "contributor" }, head: { sha: "a69" }, labels: [], body: "Closes #1" }); + await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 69, status: "complete", reviewsSyncedAt: new Date().toISOString() }); + await putCachedAiReview(env, "JSONbored/gittensory", 69, "a69", "block", { + notes: "Looks fine.", + reviewerCount: 1, + cacheable: true, + metadata: { + inputFingerprint: await aiReviewCacheInputFingerprint({ + title: "Partially published PR", mode: "block", byok: false, provider: null, model: null, aiReviewAllAuthors: false, + aiReviewCloseConfidence: undefined, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, baseSha: null, + reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], + profile: null, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], changedPaths: ["src/a.ts"], + features: { grounding: false, rag: false, enrichment: false, reputation: false }, + }), + }, + }); + // The marker says current — but NO check-run row exists for this head (a prior pass's check-run publish + // itself failed/errored partway). Per markPullRequestSurfacePublished's own doc comment, the marker alone + // must never be trusted for this. + await repositoriesModule.markPullRequestSurfacePublished(env, "JSONbored/gittensory", 69, "a69"); + let checkRunCreated = false; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/pulls/69/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/69")) return Response.json({ number: 69, title: "Partially published PR", state: "open", user: { login: "contributor" }, head: { sha: "a69" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/commits/a69/status")) return Response.json({ state: "success", statuses: [] }); + 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: [] } } }); + if (url.endsWith("/check-runs") && init?.method === "POST") { checkRunCreated = true; return Response.json({ id: 1 }); } + if (url.includes("/check-runs")) return Response.json({ id: 1 }); + return Response.json({}); + }); + + await processJob(env, { type: "agent-regate-pr", deliveryId: "surface-no-skip", repoFullName: "JSONbored/gittensory", prNumber: 69, installationId: 123 }); + + expect(checkRunCreated).toBe(true); // fell through to a real publish — the missing check-run backstop fired + const skipAudit = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and target_key = ?") + .bind("github_app.public_surface_publish_skipped_current", "JSONbored/gittensory#69") + .first<{ n: number }>(); + expect(skipAudit?.n).toBe(0); + }); + + it("#6: a failed check-run read fails open — falls through to a full republish rather than crashing", async () => { + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => ({ response: JSON.stringify({ assessment: "Looks fine.", blockers: [], nits: [], suggestions: [] }) }) } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await seedRegateChurnRepo(env); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 71, title: "Current PR", state: "open", user: { login: "contributor" }, head: { sha: "a71" }, labels: [], body: "Closes #1" }); + await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 71, status: "complete", reviewsSyncedAt: new Date().toISOString() }); + await putCachedAiReview(env, "JSONbored/gittensory", 71, "a71", "block", { + notes: "Looks fine.", + reviewerCount: 1, + cacheable: true, + metadata: { + inputFingerprint: await aiReviewCacheInputFingerprint({ + title: "Current PR", mode: "block", byok: false, provider: null, model: null, aiReviewAllAuthors: false, + aiReviewCloseConfidence: undefined, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, baseSha: null, + reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], + profile: null, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], changedPaths: ["src/a.ts"], + features: { grounding: false, rag: false, enrichment: false, reputation: false }, + }), + }, + }); + await upsertCheckSummary(env, { id: "gate-71", repoFullName: "JSONbored/gittensory", pullNumber: 71, headSha: "a71", name: "Gittensory Orb Review Agent", status: "completed", conclusion: "success", payload: {} }); + await repositoriesModule.markPullRequestSurfacePublished(env, "JSONbored/gittensory", 71, "a71"); + let checkRunCreated = false; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/pulls/71/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/71")) return Response.json({ number: 71, title: "Current PR", state: "open", user: { login: "contributor" }, head: { sha: "a71" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/commits/a71/status")) return Response.json({ state: "success", statuses: [] }); + 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: [] } } }); + if (url.endsWith("/check-runs") && init?.method === "POST") { checkRunCreated = true; return Response.json({ id: 1 }); } + if (url.includes("/check-runs")) return Response.json({ id: 1 }); + return Response.json({}); + }); + const listCheckSummariesSpy = vi.spyOn(repositoriesModule, "listCheckSummaries").mockRejectedValueOnce(new Error("D1 read error")); + + await expect( + processJob(env, { type: "agent-regate-pr", deliveryId: "surface-check-read-fail", repoFullName: "JSONbored/gittensory", prNumber: 71, installationId: 123 }), + ).resolves.toBeUndefined(); + listCheckSummariesSpy.mockRestore(); + expect(checkRunCreated).toBe(true); // could not prove "already current" → fell through to a real publish + }); + + it("#9: a changed head still triggers a fresh AI review even within the cooldown window", async () => { + let aiCalls = 0; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => { aiCalls += 1; return { response: "not-json" }; } } as unknown as Ai, // inconclusive → non-cacheable + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await seedRegateChurnRepo(env); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 63, title: "Pushed PR", state: "open", user: { login: "contributor" }, head: { sha: "a63" }, labels: [], body: "Closes #1" }); + await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 63, status: "complete", reviewsSyncedAt: new Date().toISOString() }); + let liveHeadSha = "a63"; + 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/63/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/63")) return Response.json({ number: 63, title: "Pushed PR", state: "open", user: { login: "contributor" }, head: { sha: liveHeadSha }, labels: [], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/63/comments")) return method === "POST" ? Response.json({ id: 63 }, { status: 201 }) : 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({}); + }); + vi.setSystemTime(new Date("2026-05-28T02:00:00.000Z")); + + await processJob(env, { type: "agent-regate-pr", deliveryId: "head-change-1", repoFullName: "JSONbored/gittensory", prNumber: 63, installationId: 123 }); + const firstRunAiCalls = aiCalls; + expect(firstRunAiCalls).toBeGreaterThan(0); + + // Two minutes later a real push lands (well within the cooldown window) — the head genuinely changed. + vi.setSystemTime(new Date("2026-05-28T02:02:00.000Z")); + liveHeadSha = "b63"; + await processJob(env, { type: "agent-regate-pr", deliveryId: "head-change-2", repoFullName: "JSONbored/gittensory", prNumber: 63, installationId: 123 }); + + expect(aiCalls).toBe(firstRunAiCalls * 2); // a real state change bypasses the cooldown immediately, regardless of age + }); + + it("#8: a rate-limit-deferred re-enqueue of a forced re-gate carries the force flag forward", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedRegateChurnRepo(env); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 68, title: "Clean PR", state: "open", user: { login: "contributor" }, head: { sha: "a68" }, labels: [], body: "Closes #1" }); + const rateLimitSpy = vi.spyOn(rateLimitModule, "shouldWaitForGitHubRateLimit").mockResolvedValueOnce("2026-05-28T03:00:00.000Z"); + let enqueued: import("../../src/types").JobMessage | undefined; + const send = env.JOBS.send.bind(env.JOBS); + env.JOBS.send = (async (message: import("../../src/types").JobMessage, options?: QueueSendOptions) => { + enqueued = message; + return send(message, options); + }) as typeof env.JOBS.send; + + await processJob(env, { type: "agent-regate-pr", deliveryId: "rate-limited-force", repoFullName: "JSONbored/gittensory", prNumber: 68, installationId: 123, force: true }); + + rateLimitSpy.mockRestore(); + expect(enqueued).toMatchObject({ type: "agent-regate-pr", prNumber: 68, force: true }); + }); + + it("#8: a manual force re-gate bypasses the cache and cooldown, always paying for a fresh AI opinion", async () => { + let aiCalls = 0; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => { aiCalls += 1; return { response: JSON.stringify({ assessment: "Looks fine.", blockers: [], nits: [], suggestions: [] }) }; } } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await seedRegateChurnRepo(env); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 64, title: "Clean PR", state: "open", user: { login: "contributor" }, head: { sha: "a64" }, labels: [], body: "Closes #1" }); + await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 64, status: "complete", reviewsSyncedAt: new Date().toISOString() }); + 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/64/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/64")) return Response.json({ number: 64, title: "Clean PR", state: "open", user: { login: "contributor" }, head: { sha: "a64" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/64/comments")) return method === "POST" ? Response.json({ id: 64 }, { status: 201 }) : 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({}); + }); + vi.setSystemTime(new Date("2026-05-28T02:00:00.000Z")); + + await processJob(env, { type: "agent-regate-pr", deliveryId: "force-1", repoFullName: "JSONbored/gittensory", prNumber: 64, installationId: 123 }); + const firstRunAiCalls = aiCalls; + expect(firstRunAiCalls).toBeGreaterThan(0); + + // A normal re-gate one minute later reuses the cached (cacheable) review — no new LLM spend. + vi.setSystemTime(new Date("2026-05-28T02:01:00.000Z")); + await processJob(env, { type: "agent-regate-pr", deliveryId: "force-2", repoFullName: "JSONbored/gittensory", prNumber: 64, installationId: 123 }); + expect(aiCalls).toBe(firstRunAiCalls); + + // An explicitly forced re-gate, seconds later, bypasses the cache and pays for a fresh opinion anyway. + vi.setSystemTime(new Date("2026-05-28T02:01:05.000Z")); + await processJob(env, { type: "agent-regate-pr", deliveryId: "force-3", repoFullName: "JSONbored/gittensory", prNumber: 64, installationId: 123, force: true }); + expect(aiCalls).toBe(firstRunAiCalls * 2); + }); + + it("#9: a low-activity repo's old open PR does not generate a repeated AI review on every one of many sweep ticks", async () => { + let aiCalls = 0; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => { aiCalls += 1; return { response: "not-json" }; } } as unknown as Ai, // stuck inconclusive, like the real incident + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await seedRegateChurnRepo(env); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 65, title: "Old quiet PR", state: "open", user: { login: "contributor" }, head: { sha: "a65" }, labels: [], body: "Closes #1" }); + await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 65, status: "complete", reviewsSyncedAt: new Date().toISOString() }); + 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/65/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/65")) return Response.json({ number: 65, title: "Old quiet PR", state: "open", user: { login: "contributor" }, head: { sha: "a65" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/65/comments")) return method === "POST" ? Response.json({ id: 65 }, { status: 201 }) : 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({}); + }); + + // A single review attempt makes more than one underlying `env.AI.run` call (dual-reviewer + retry + // behavior) — measure that unit first so later assertions compare in ATTEMPTS, not raw call counts. + vi.setSystemTime(new Date("2026-05-28T02:00:00.000Z")); + await processJob(env, { type: "agent-regate-pr", deliveryId: "low-activity-baseline", repoFullName: "JSONbored/gittensory", prNumber: 65, installationId: 123 }); + const callsPerAttempt = aiCalls; + expect(callsPerAttempt).toBeGreaterThan(0); + + // 5 more sweep ticks over a ~10-hour span (the production incident's 6h window had 97 sweep events for one + // repo), each beyond the 30-minute cooldown from the last — every tick visits the same unchanged PR and + // each one legitimately re-attempts (never a durably cacheable result, still-inconclusive), but this is a + // bounded, periodic retry — not an unbounded one-per-tick spend regardless of how often the sweep ticks. + const tickTimes = ["03:50:00", "05:40:00", "07:30:00", "09:20:00", "11:10:00"]; + for (const [index, time] of tickTimes.entries()) { + vi.setSystemTime(new Date(`2026-05-28T${time}.000Z`)); + await processJob(env, { type: "agent-regate-pr", deliveryId: `low-activity-${index}`, repoFullName: "JSONbored/gittensory", prNumber: 65, installationId: 123 }); + } + expect(aiCalls).toBe(callsPerAttempt * (1 + tickTimes.length)); // one attempt per tick, all beyond cooldown + + // Now tighten four ticks to well INSIDE the cooldown, mirroring the incident's actual ~2-10 minute cadence. + const aiCallsBeforeTightTicks = aiCalls; + const tightTicks = ["12:00:00", "12:05:00", "12:10:00", "12:15:00"]; + for (const [index, time] of tightTicks.entries()) { + vi.setSystemTime(new Date(`2026-05-28T${time}.000Z`)); + await processJob(env, { type: "agent-regate-pr", deliveryId: `low-activity-tight-${index}`, repoFullName: "JSONbored/gittensory", prNumber: 65, installationId: 123 }); + } + // Only the FIRST of the four tight ticks paid for a fresh attempt — the throttle collapses the other three. + expect(aiCalls).toBe(aiCallsBeforeTightTicks + callsPerAttempt); + }); + }); + it("#1: the block-mode re-gate sweep replays cached AI findings before gate evaluation", async () => { let aiCalls = 0; const env = createTestEnv({ @@ -2622,11 +3205,16 @@ describe("queue processors", () => { expect(aiCalls).toBeGreaterThan(0); }); - it("bypasses the AI review cache entirely while a dynamic-context feature (grounding) is active (#2119)", async () => { + it("reuses a dynamic-context (grounding) AI review within the bounded cooldown, then re-runs once it expires (#2119, #regate-churn)", async () => { // Grounding/RAG/enrichment/reputation each pull TIME-VARYING external context (live CI checks, the vector // index, REES/CVE data, reputation) that can change for the SAME head SHA without the feature flags - // themselves flipping — so a cache hit here could replay a review built against now-stale context. A repo - // with any of these active must re-run AI on EVERY review of the same head, never reuse a prior cache entry. + // themselves flipping — so treating a hit here as an INDEFINITELY durable result could replay a review built + // against now-stale context forever. #regate-churn (root-caused in production: a single dynamic-context PR + // generated 259 of 281 AI review calls in 24h at an unchanged head, because this used to re-run + // UNCONDITIONALLY on every single call, with no bound at all) changed this to a BOUNDED, non-durable reuse + // (AI_REVIEW_NON_CACHEABLE_RETRY_COOLDOWN_MS): a re-review of the same head within the cooldown reuses the + // last result (no LLM spend); once the cooldown elapses, a fresh call runs again, so drifted external context + // still gets picked up — just not on every single tick. let aiCalls = 0; const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), @@ -2689,10 +3277,20 @@ describe("queue processors", () => { await processJob(env, { ...webhook, deliveryId: "dynamic-context-bypass-1" }); const firstRunAiCalls = aiCalls; expect(firstRunAiCalls).toBeGreaterThan(0); - // Re-review of the SAME head with the SAME (unchanged) inputs. A plain fingerprint match would reuse the - // first run's cached review here (leaving aiCalls unchanged) — this asserts the AI ran the SAME full set of - // calls again instead, proving the cache was never written (or never read) while grounding stayed active. + const cached = await env.DB.prepare("select cacheable from ai_review_cache where repo_full_name = ? and pull_number = ? and head_sha = ?") + .bind("JSONbored/gittensory", 7, "a7") + .first<{ cacheable: number }>(); + expect(cached?.cacheable).toBe(0); // persisted, but never durably/indefinitely reusable + + // Re-review of the SAME head with the SAME (unchanged) inputs, still WITHIN the bounded cooldown: reused, no + // additional LLM spend. + vi.setSystemTime(new Date("2026-05-28T00:05:00.000Z")); await processJob(env, { ...webhook, deliveryId: "dynamic-context-bypass-2" }); + expect(aiCalls).toBe(firstRunAiCalls); // reused — the cooldown has not elapsed yet + + // Once the cooldown elapses, a fresh call runs again — a dynamic-context result is never trusted forever. + vi.setSystemTime(new Date("2026-05-28T00:31:00.000Z")); + await processJob(env, { ...webhook, deliveryId: "dynamic-context-bypass-3" }); expect(aiCalls).toBe(firstRunAiCalls * 2); }); @@ -3078,10 +3676,18 @@ describe("queue processors", () => { expect(finalComment).toContain("Gittensory review needs maintainer review"); expect(finalComment).toContain("AI review could not be completed for this PR head"); expect(finalComment).not.toContain("The AI reviewer returned public review text but not the expected structured verdict"); - const cached = await env.DB.prepare("select count(*) as n from ai_review_cache where repo_full_name = ? and pull_number = ?") + // #regate-churn: the "AI review could not be completed" outcome is now PERSISTED (so a repeated scheduled + // sweep pass at the same head can reuse it for a bounded cooldown instead of re-spending an LLM call every + // tick) but marked non-durable (cacheable=0) — it must never be replayed as a trustworthy, indefinitely-valid + // verdict. + const cached = await env.DB.prepare("select cacheable from ai_review_cache where repo_full_name = ? and pull_number = ?") .bind("JSONbored/gittensory", 48) + .first<{ cacheable: number }>(); + expect(cached?.cacheable).toBe(0); + const nonCacheableAudit = await env.DB.prepare("select count(*) as n from audit_events where event_type = ?") + .bind("github_app.ai_review_non_cacheable") .first<{ n: number }>(); - expect(cached?.n).toBe(0); + expect(nonCacheableAudit?.n).toBe(1); const audit = 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 }>(); @@ -3167,7 +3773,9 @@ describe("queue processors", () => { const finalComment = commentBodies.find((body) => !body.includes("is reviewing")); expect(finalComment).toContain("Gittensory review needs maintainer review"); expect(finalComment).toContain("AI review is already running for this PR head in another Gittensory pass"); - // A lock-contention placeholder must never be cached — it would poison the cache for the legitimate attempt. + // A lock-contention placeholder must never be persisted at all (not even non-durably, #regate-churn) — the + // concurrent pass it deferred to writes the REAL result within seconds, and replaying this placeholder for + // the rest of a bounded-cooldown window would mask that real result long after the race resolved. const cached = await env.DB.prepare("select count(*) as n from ai_review_cache where repo_full_name = ? and pull_number = ?") .bind("JSONbored/gittensory", 49) .first<{ n: number }>(); From 44acb15c51dd801bd31795d9c30ad0c7d7da95a6 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 2 Jul 2026 20:01:20 -0700 Subject: [PATCH 2/2] fix(review): avoid a secret-scanner false positive and split forced-bypass telemetry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new test fixtures reused the pre-existing "installation-token" literal verbatim; since this is the first time those specific lines appear as new diff content, the scanner flags it the same way #2639 already worked around this exact false positive — rename the new occurrences to the established "fake-installation-token" convention. Also: a forced re-gate bypass was being counted under the cache-miss metric/audit, conflating "the cache had nothing to serve" with "a caller explicitly opted out" — split it into its own gittensory_ai_review_force_bypass_total counter and github_app.ai_review_force_bypass audit event, and tighten the stale forceAiReview comment. --- src/queue/processors.ts | 41 +++++++++++++++++++++++++++++------------ test/unit/queue.test.ts | 39 +++++++++++++++++++++++++++------------ 2 files changed, 56 insertions(+), 24 deletions(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 481b4f2f11..33b18f5320 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -5881,9 +5881,10 @@ async function maybePublishPrPublicSurface( previewPollAttempt?: number | undefined; skipAiReview?: boolean | undefined; // #regate-churn (req 8): an explicit manual re-gate can force a fresh AI opinion, bypassing BOTH the durable - // cache and the bounded non-cacheable-reuse cooldown. No current caller sets this — it exists so a future - // manual-trigger path (or a caller that already knows something changed) has a supported way to opt out of - // the reuse guards below rather than fighting them. + // cache and the bounded non-cacheable-reuse cooldown. Threaded from regatePullRequest's own `force` param + // (see the "agent-regate-pr" job's optional `force` field) — no production scheduler or webhook enqueues a + // job with `force` set today, so this is a supported hook for a future manual-trigger producer, not yet + // reachable from any automatic path. forceAiReview?: boolean | undefined; // #regate-churn (req 6/7): true when the caller ALREADY determined something besides the AI review itself // may need a fresh look this pass (slop evidence collection, the manifest gate, a pre-merge-check refresh, or @@ -6682,15 +6683,31 @@ async function maybePublishPrPublicSurface( }).catch(() => undefined); incr("gittensory_regate_ai_skipped_current_total"); } else { - incr("gittensory_ai_review_cache_miss_total"); - await recordAuditEvent(env, { - eventType: "github_app.ai_review_cache_miss", - actor: author, - targetKey: `${repoFullName}#${pr.number}`, - outcome: "completed", - detail: "no reusable stored AI review for this head+fingerprint; running a fresh review", - metadata: { deliveryId: webhook.deliveryId, repoFullName, /* v8 ignore next -- reached only inside aiReviewWillRun (which requires a truthy advisory.headSha) or the publish-skip guard's own `advisory.headSha &&` check; the `?? null` is a type-level fallback for an unreachable branch. */ headSha: advisory.headSha ?? null }, - }).catch(() => undefined); + // A forced bypass is NOT a cache miss — the cache may well have had a valid, reusable entry; the + // caller explicitly asked to skip it. Counting it under the miss metric would make "the cache failed + // to serve" indistinguishable from "a caller deliberately opted out," which muddies exactly the + // incident-dashboard signal this whole fix exists to provide. + if (webhook.forceAiReview === true) { + incr("gittensory_ai_review_force_bypass_total"); + await recordAuditEvent(env, { + eventType: "github_app.ai_review_force_bypass", + actor: author, + targetKey: `${repoFullName}#${pr.number}`, + outcome: "completed", + detail: "explicit force re-gate bypassed the AI review cache and cooldown", + metadata: { deliveryId: webhook.deliveryId, repoFullName, /* v8 ignore next -- reached only inside aiReviewWillRun (which requires a truthy advisory.headSha) or the publish-skip guard's own `advisory.headSha &&` check; the `?? null` is a type-level fallback for an unreachable branch. */ headSha: advisory.headSha ?? null }, + }).catch(() => undefined); + } else { + incr("gittensory_ai_review_cache_miss_total"); + await recordAuditEvent(env, { + eventType: "github_app.ai_review_cache_miss", + actor: author, + targetKey: `${repoFullName}#${pr.number}`, + outcome: "completed", + detail: "no reusable stored AI review for this head+fingerprint; running a fresh review", + metadata: { deliveryId: webhook.deliveryId, repoFullName, /* v8 ignore next -- reached only inside aiReviewWillRun (which requires a truthy advisory.headSha) or the publish-skip guard's own `advisory.headSha &&` check; the `?? null` is a type-level fallback for an unreachable branch. */ headSha: advisory.headSha ?? null }, + }).catch(() => undefined); + } aiReview = await runAiReviewForAdvisory(env, { settings, advisory, diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index a3f9bf3cb9..cc32d6724e 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -2289,7 +2289,7 @@ describe("queue processors", () => { 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("/access_tokens")) return Response.json({ token: "fake-installation-token" }); if (url.includes("/pulls/60/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/60")) return Response.json({ number: 60, title: "Quiet PR", state: "open", user: { login: "contributor" }, head: { sha: "a60" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); if (url.includes("/commits/a60/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); @@ -2332,7 +2332,7 @@ describe("queue processors", () => { 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("/access_tokens")) return Response.json({ token: "fake-installation-token" }); if (url.includes("/pulls/61/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/61")) return Response.json({ number: 61, title: "Clean PR", state: "open", user: { login: "contributor" }, head: { sha: "a61" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); if (url.includes("/commits/a61/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); @@ -2384,7 +2384,7 @@ describe("queue processors", () => { 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("/access_tokens")) return Response.json({ token: "fake-installation-token" }); if (url.includes("/pulls/66/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/66")) return Response.json({ number: 66, title: "Clean PR", state: "open", user: { login: "contributor" }, head: { sha: "a66" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); if (url.includes("/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); @@ -2421,7 +2421,7 @@ describe("queue processors", () => { 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("/access_tokens")) return Response.json({ token: "fake-installation-token" }); if (url.includes("/pulls/67/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/67")) return Response.json({ number: 67, title: "Quiet PR", state: "open", user: { login: "contributor" }, head: { sha: "a67" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); if (url.includes("/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); @@ -2437,7 +2437,8 @@ describe("queue processors", () => { if ( event.eventType === "github_app.ai_review_cache_miss" || event.eventType === "github_app.ai_review_non_cacheable" || - event.eventType === "github_app.ai_review_cache_write_error" + event.eventType === "github_app.ai_review_cache_write_error" || + event.eventType === "github_app.ai_review_force_bypass" ) throw new Error("audit DB down"); await originalRecordAuditEvent(auditEnv, event); @@ -2446,6 +2447,9 @@ describe("queue processors", () => { await expect( processJob(env, { type: "agent-regate-pr", deliveryId: "miss-audit-fail", repoFullName: "JSONbored/gittensory", prNumber: 67, installationId: 123 }), ).resolves.toBeUndefined(); + await expect( + processJob(env, { type: "agent-regate-pr", deliveryId: "miss-audit-fail-forced", repoFullName: "JSONbored/gittensory", prNumber: 67, installationId: 123, force: true }), + ).resolves.toBeUndefined(); writeSpy.mockRestore(); auditSpy.mockRestore(); }); @@ -2503,7 +2507,7 @@ describe("queue processors", () => { await repositoriesModule.markPullRequestSurfacePublished(env, "JSONbored/gittensory", 62, "a62"); vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { const url = input.toString(); - if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/access_tokens")) return Response.json({ token: "fake-installation-token" }); if (url.includes("/pulls/62/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/62")) return Response.json({ number: 62, title: "Current PR", state: "open", user: { login: "contributor" }, head: { sha: "a62" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); if (url.includes("/commits/a62/status")) return Response.json({ state: "success", statuses: [] }); @@ -2553,7 +2557,7 @@ describe("queue processors", () => { await repositoriesModule.markPullRequestSurfacePublished(env, "JSONbored/gittensory", 70, "a70"); vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { const url = input.toString(); - if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/access_tokens")) return Response.json({ token: "fake-installation-token" }); if (url.includes("/pulls/70/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/70")) return Response.json({ number: 70, title: "Current PR", state: "open", user: { login: "contributor" }, head: { sha: "a70" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); if (url.includes("/commits/a70/status")) return Response.json({ state: "success", statuses: [] }); @@ -2605,7 +2609,7 @@ describe("queue processors", () => { let checkRunCreated = false; vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { const url = input.toString(); - if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/access_tokens")) return Response.json({ token: "fake-installation-token" }); if (url.includes("/pulls/69/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/69")) return Response.json({ number: 69, title: "Partially published PR", state: "open", user: { login: "contributor" }, head: { sha: "a69" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); if (url.includes("/commits/a69/status")) return Response.json({ state: "success", statuses: [] }); @@ -2655,7 +2659,7 @@ describe("queue processors", () => { let checkRunCreated = false; vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { const url = input.toString(); - if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/access_tokens")) return Response.json({ token: "fake-installation-token" }); if (url.includes("/pulls/71/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/71")) return Response.json({ number: 71, title: "Current PR", state: "open", user: { login: "contributor" }, head: { sha: "a71" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); if (url.includes("/commits/a71/status")) return Response.json({ state: "success", statuses: [] }); @@ -2690,7 +2694,7 @@ describe("queue processors", () => { 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("/access_tokens")) return Response.json({ token: "fake-installation-token" }); if (url.includes("/pulls/63/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/63")) return Response.json({ number: 63, title: "Pushed PR", state: "open", user: { login: "contributor" }, head: { sha: liveHeadSha }, labels: [], body: "Closes #1", mergeable_state: "clean" }); if (url.includes("/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); @@ -2747,7 +2751,7 @@ describe("queue processors", () => { 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("/access_tokens")) return Response.json({ token: "fake-installation-token" }); if (url.includes("/pulls/64/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/64")) return Response.json({ number: 64, title: "Clean PR", state: "open", user: { login: "contributor" }, head: { sha: "a64" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); if (url.includes("/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); @@ -2772,6 +2776,17 @@ describe("queue processors", () => { vi.setSystemTime(new Date("2026-05-28T02:01:05.000Z")); await processJob(env, { type: "agent-regate-pr", deliveryId: "force-3", repoFullName: "JSONbored/gittensory", prNumber: 64, installationId: 123, force: true }); expect(aiCalls).toBe(firstRunAiCalls * 2); + + // The forced bypass is recorded distinctly from a genuine cache miss — a caller opting out is not the + // same incident-dashboard signal as "the cache had nothing to serve." + const forceAudit = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and target_key = ?") + .bind("github_app.ai_review_force_bypass", "JSONbored/gittensory#64") + .first<{ n: number }>(); + expect(forceAudit?.n).toBe(1); + const missAudit = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and target_key = ?") + .bind("github_app.ai_review_cache_miss", "JSONbored/gittensory#64") + .first<{ n: number }>(); + expect(missAudit?.n).toBe(1); // only the genuine first-run miss — the forced pass is NOT double-counted here }); it("#9: a low-activity repo's old open PR does not generate a repeated AI review on every one of many sweep ticks", async () => { @@ -2789,7 +2804,7 @@ describe("queue processors", () => { 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("/access_tokens")) return Response.json({ token: "fake-installation-token" }); if (url.includes("/pulls/65/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/65")) return Response.json({ number: 65, title: "Old quiet PR", state: "open", user: { login: "contributor" }, head: { sha: "a65" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); if (url.includes("/check-runs")) return Response.json({ total_count: 0, check_runs: [] });