diff --git a/migrations/0112_ai_review_cache_published.sql b/migrations/0112_ai_review_cache_published.sql new file mode 100644 index 0000000000..a255bca294 --- /dev/null +++ b/migrations/0112_ai_review_cache_published.sql @@ -0,0 +1,8 @@ +-- #regate-churn: once an AI review has actually been PUBLISHED to a PR (a real comment/check-run reached +-- GitHub), it becomes the authoritative result for that exact head+fingerprint and must never be silently +-- regenerated just because AI_REVIEW_NON_CACHEABLE_RETRY_COOLDOWN_MS elapsed -- the cooldown exists to bound +-- reuse BEFORE the first publish (e.g. overlapping sweep passes), not to force a periodic re-run of an +-- already-surfaced verdict. `published_at` (NULL until the publish step stamps it) lets getCachedAiReview treat +-- a published non-cacheable row as indefinitely reusable, same as a genuinely cacheable one, for this exact +-- head+fingerprint -- see putCachedAiReview/markAiReviewPublished in src/db/repositories.ts. +ALTER TABLE ai_review_cache ADD COLUMN published_at TEXT; diff --git a/src/db/repositories.ts b/src/db/repositories.ts index 334bbf1bc9..2bb36a11bb 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -4015,7 +4015,12 @@ 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, cacheable, created_at AS createdAt 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, published_at AS publishedAt, 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; cacheable: number; createdAt: string }>(); + .first<{ notes: string; reviewerCount: number; mode: string; findingsJson: string | null; metadataJson: string | null; cacheable: number; publishedAt: string | null; 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; + if (row.publishedAt == 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 ( @@ -4050,10 +4057,41 @@ export async function getCachedAiReview( }; } +/** #regate-churn (maintainer-gated freeze): the most recently PUBLISHED AI review for this PR, regardless of + * which head SHA it was computed against. Used ONLY when the PR is currently held for manual review — a repeat + * contributor push must not buy a fresh, real AI call (or a chance to flip the published verdict via plain LLM + * non-determinism) while the PR sits in that state; only an explicit maintainer retrigger (which bypasses this + * entirely, see `webhook.forceAiReview`) may spend a new one. A nullish/never-published PR is a miss (the caller + * falls through to a normal fresh review — this only ever REUSES an already-surfaced result, never invents one). */ +export async function getLatestPublishedAiReview( + env: Env, + repoFullName: string, + pullNumber: number, + mode: string, +): Promise<{ notes: string; reviewerCount: number; findings: AdvisoryFinding[]; metadata?: Record | undefined } | null> { + const row = await env.DB + .prepare( + "SELECT notes, reviewer_count AS reviewerCount, findings_json AS findingsJson, metadata_json AS metadataJson FROM ai_review_cache WHERE repo_full_name = ? AND pull_number = ? AND ai_review_mode = ? AND published_at IS NOT NULL ORDER BY published_at DESC LIMIT 1", + ) + .bind(repoFullName, pullNumber, mode) + .first<{ notes: string; reviewerCount: number; findingsJson: string | null; metadataJson: string | null }>(); + if (!row) return null; + const metadata = parseJson>(row.metadataJson, {}); + return { + notes: row.notes, + reviewerCount: row.reviewerCount, + findings: parseJson(row.findingsJson, []), + ...(Object.keys(metadata).length > 0 ? { metadata } : {}), + }; +} + /** 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. */ + * it non-durable — omitted or any other value defaults to cacheable (1), the pre-existing behavior. + * `published_at` is ALWAYS reset to NULL here: a write only ever happens for a genuinely fresh review (a cache + * hit never reaches this function), so any prior publish marker belongs to different, now-superseded content and + * must not leak onto it — markAiReviewPublished stamps it again once THIS content actually reaches the PR. */ export async function putCachedAiReview( env: Env, repoFullName: string, @@ -4067,15 +4105,34 @@ export async function putCachedAiReview( 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, cacheable, 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, published_at, created_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, NULL, ?) 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, cacheable = excluded.cacheable, 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, published_at = NULL, created_at = excluded.created_at`, ) .bind(repoFullName, pullNumber, headSha, mode, review.notes, review.reviewerCount, jsonString(review.findings ?? []), jsonString(review.metadata ?? {}), cacheable, createdAt) .run(); } +/** #regate-churn: stamp the AI review row for (repo, pull, head SHA) as PUBLISHED — called once the review's + * content has actually reached the PR (a comment/check-run publish completed), so a later lookup at this exact + * head+fingerprint (getCachedAiReview) treats it as indefinitely reusable regardless of the non-cacheable + * cooldown. `WHERE published_at IS NULL` keeps this idempotent and non-destructive: a later call for the same + * already-published row is a no-op rather than rewriting the timestamp. A nullish head SHA or a missing row + * (e.g. AI review was skipped/off this pass) is a harmless no-op — nothing to stamp. */ +export async function markAiReviewPublished( + env: Env, + repoFullName: string, + pullNumber: number, + headSha: string | null | undefined, +): Promise { + if (!headSha) return; + await env.DB + .prepare("UPDATE ai_review_cache SET published_at = ? WHERE repo_full_name = ? AND pull_number = ? AND head_sha = ? AND published_at IS NULL") + .bind(nowIso(), repoFullName, pullNumber, headSha) + .run(); +} + export async function replaceCollisionEdges(env: Env, repoFullName: string, edges: CollisionEdgeRecord[]): Promise { const db = getDb(env.DB); await env.DB.prepare("DELETE FROM collision_edges WHERE repo_full_name = ?").bind(repoFullName).run(); diff --git a/src/db/schema.ts b/src/db/schema.ts index 9a4cbc0bef..cd41e7f3ac 100644 --- a/src/db/schema.ts +++ b/src/db/schema.ts @@ -1277,6 +1277,10 @@ export const aiReviewCache = sqliteTable( // 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), + // #regate-churn: NULL until the review is actually published to the PR (a real comment/check-run reached + // GitHub); once stamped, getCachedAiReview treats this row as indefinitely reusable for this exact + // head+fingerprint regardless of AI_REVIEW_NON_CACHEABLE_RETRY_COOLDOWN_MS -- see markAiReviewPublished. + publishedAt: text("published_at"), createdAt: text("created_at").notNull().$defaultFn(() => nowIso()), }, (table) => ({ diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 9478309761..2555002224 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -45,7 +45,9 @@ import { markRepositoriesRemovedFromInstallation, persistAdvisory, getCachedAiReview, + getLatestPublishedAiReview, putCachedAiReview, + markAiReviewPublished, markPullRequestsRegated, markPullRequestReviewsInvalidated, markPullRequestSurfacePublished, @@ -254,6 +256,7 @@ import { } from "../selfhost/queue-common"; import { aiReviewCacheInputFingerprint } from "../review/ai-review-cache-input"; import { + AGENT_LABEL_NEEDS_REVIEW, downgradeCloseToHold, downgradeMergeToHold, MAX_REVIEW_NAG_COOLDOWN_DAYS, @@ -7492,6 +7495,11 @@ async function maybePublishPrPublicSurface( await markPullRequestSurfacePublished(env, repoFullName, pr.number, advisory.headSha).catch((error) => { console.error(JSON.stringify({ level: "warn", event: "surface_published_mark_failed", repoFullName, pullNumber: pr.number, error: errorMessage(error) })); }); + // #regate-churn: mark the AI review row for THIS head+fingerprint as durably published (a no-op when no fresh + // row was written this pass -- e.g. the frozen-reuse path above, or AI review off/skipped entirely). + await markAiReviewPublished(env, repoFullName, pr.number, advisory.headSha).catch((error) => { + console.error(JSON.stringify({ level: "warn", event: "ai_review_published_mark_failed", repoFullName, pullNumber: pr.number, error: errorMessage(error) })); + }); return gateEvaluation; }; try { @@ -7727,8 +7735,24 @@ async function maybePublishPrPublicSurface( author, settings.contributorBlacklist, ); + // #regate-churn (maintainer-gated freeze): once a PR is held for manual review -- the manual-review label is + // already on it from a PRIOR pass -- a repeat contributor push must not buy a fresh, real AI review. That is + // exactly the gaming surface this closes: iterating pushes hoping to slip a green verdict past the bot (or + // just to see what the AI says next), at real LLM cost, instead of waiting for the human judgment the hold + // exists for. Only an explicit maintainer/collaborator retrigger (the PR-panel checkbox, which sets + // `webhook.forceAiReview`) may unfreeze it. CI/mergeable facts and label/assignee reconciliation are + // UNAFFECTED — both are recomputed fresh every pass below regardless of this flag; only the AI's own + // substantive verdict/findings are pinned. The very FIRST pass that establishes the hold is never frozen: the + // label is applied by the disposition executor AFTER this pass publishes, so `pr.labels` (read at the top of + // this sweep, before that write) does not carry it yet. + const manualReviewLabel = settings.manualReviewLabel === null ? null : (settings.manualReviewLabel ?? AGENT_LABEL_NEEDS_REVIEW); + const isFrozenForManualReview = + webhook.forceAiReview !== true && + manualReviewLabel !== null && + pr.labels.some((label) => label.toLowerCase() === manualReviewLabel.toLowerCase()); const aiReviewWillRun = !authorBlacklisted && + !isFrozenForManualReview && (await shouldStartAiReviewForAdvisory(env, { settings, advisory, @@ -7738,10 +7762,33 @@ async function maybePublishPrPublicSurface( skipAiReview: webhook.skipAiReview, })); aiReviewExpected = aiReviewWillRun; + if (isFrozenForManualReview) { + const frozenReview = await getLatestPublishedAiReview(env, repoFullName, pr.number, settings.aiReviewMode).catch(() => null); + if (frozenReview && hasPublicReviewAssessment(frozenReview.notes)) { + advisory.findings.push(...frozenReview.findings); + aiReview = frozenReview; + aiReviewWasReused = true; + incr("gittensory_ai_review_frozen_reuse_total"); + await recordAuditEvent(env, { + eventType: "github_app.ai_review_frozen_reuse", + actor: author, + targetKey: `${repoFullName}#${pr.number}`, + outcome: "completed", + detail: "PR is held for manual review; reused the last published AI review instead of spending a fresh call", + /* v8 ignore next -- a truthy `frozenReview` means markAiReviewPublished previously stamped a row for + * a non-null head SHA (it no-ops on a nullish one), and an open PR does not lose its head SHA once + * set; the `?? null` is a type-level fallback for a practically-unreachable branch, mirroring the + * identical `advisory.headSha ?? null` fallbacks elsewhere in this function. */ + metadata: { deliveryId: webhook.deliveryId, repoFullName, headSha: advisory.headSha ?? null }, + }).catch(() => undefined); + } + } // Post a transient "🟪 reviewing…" placeholder BEFORE the review refresh runs so contributors never see a // stale green/yellow/red verdict while the current head is being recomputed. In-place upsert: once the final // verdict is ready it overwrites this comment. GitHub rate-limits still abort so the queue can retry instead - // of leaving a stale public surface visible. + // of leaving a stale public surface visible. `shouldPostPlaceholder` (unchanged) also gates the pre-publish + // staleness check below — that check is a general "has this pass already been superseded" abort, independent + // of the placeholder UI itself, so it must keep running whenever a placeholder would ever be eligible here. const shouldPostPlaceholder = shouldPostReviewingPlaceholder({ reviewWillRun: true, mode, @@ -7754,27 +7801,39 @@ async function maybePublishPrPublicSurface( ) ) return undefined; - const placeholderBody = `${PR_PANEL_COMMENT_MARKER}\n\n${renderReviewingPlaceholder()}`; - try { - await createOrUpdatePrIntelligenceComment( - env, - installationId, - repoFullName, - pr.number, - placeholderBody, - { mode }, - ); - } catch (error) { - /* v8 ignore next -- placeholder rate-limit propagation is covered by final-comment rate-limit tests. */ - if (isGitHubRateLimitedError(error)) throw error; - await recordAuditEvent(env, { - eventType: "github_app.reviewing_placeholder_failed", - actor: author, - targetKey: `${repoFullName}#${pr.number}`, - outcome: "error", - detail: errorMessage(error), - metadata: { deliveryId: webhook.deliveryId, repoFullName }, - }).catch(() => undefined); + // #regate-churn (req 4): only actually SHOW it when something is genuinely about to (re)run -- the PR + // already carries the exact published result for this head (a same-head scheduled sweep / CI-completion + // pass), or the review is frozen for manual review, so painting "reviewing" and then immediately + // overwriting it with the SAME final content would otherwise defeat createOrUpdatePrIntelligenceComment's + // own byte-identical no-op guard (it only ever compares against whatever is CURRENTLY posted). + // A nullish (no-head/ghost) advisory.headSha can never be "the same as last published" -- markPullRequestSurfacePublished + // itself no-ops without a real head SHA to key on, so a nullish headSha must never spuriously compare equal + // to a nullish (never-published) lastPublishedSurfaceSha -- that would wrongly suppress the placeholder on + // a genuinely first-time, no-head review (#regate-churn, no-head-ghost-pr regression). + const shouldShowPlaceholderNow = !isFrozenForManualReview && (webhook.forceAiReview === true || !advisory.headSha || advisory.headSha !== pr.lastPublishedSurfaceSha); + if (shouldShowPlaceholderNow) { + const placeholderBody = `${PR_PANEL_COMMENT_MARKER}\n\n${renderReviewingPlaceholder()}`; + try { + await createOrUpdatePrIntelligenceComment( + env, + installationId, + repoFullName, + pr.number, + placeholderBody, + { mode }, + ); + } catch (error) { + /* v8 ignore next -- placeholder rate-limit propagation is covered by final-comment rate-limit tests. */ + if (isGitHubRateLimitedError(error)) throw error; + await recordAuditEvent(env, { + eventType: "github_app.reviewing_placeholder_failed", + actor: author, + targetKey: `${repoFullName}#${pr.number}`, + outcome: "error", + detail: errorMessage(error), + metadata: { deliveryId: webhook.deliveryId, repoFullName }, + }).catch(() => undefined); + } } } if (aiReviewWillRun) { @@ -7892,7 +7951,6 @@ async function maybePublishPrPublicSurface( repoInstructions: reviewInstructions, excludePaths: reviewExcludePaths, changedPaths, - baseSha: webhook.baseSha, reviewFiles: reviewFilesForAi.map((file) => ({ path: file.path, status: file.status, diff --git a/src/review/ai-review-cache-input.ts b/src/review/ai-review-cache-input.ts index 1a3da0516c..d145a04adb 100644 --- a/src/review/ai-review-cache-input.ts +++ b/src/review/ai-review-cache-input.ts @@ -6,6 +6,16 @@ import { sha256Hex } from "../utils/crypto"; export const AI_REVIEW_CACHE_INPUT_VERSION = "ai-review-input:v1"; +// #regate-churn (root cause, confirmed in production): this fingerprint USED to also hash the PR's live +// `baseSha`, on the theory that a rebase/retarget can change the diff GitHub reports for an otherwise-unchanged +// head SHA even though `changedPaths` (just the path list) stays the same. That reasoning already had a fix -- +// `reviewFiles` below hashes the actual per-file PATCH content (not just paths), which is the real signal for +// "did the reviewed content change." Hashing raw `baseSha` on top of that was redundant when the patch is +// unchanged and actively harmful when it isn't: `baseSha` is the live tip of the base branch, which advances on +// EVERY unrelated merge to it, so on an active repo it differs on almost every evaluation regardless of whether +// this PR's own diff changed at all -- causing a same-head PR to miss the cache (and re-spend a real AI call, +// producing non-deterministic LLM output that can even flip the published verdict) on every scheduled re-gate +// sweep. Removed entirely; `reviewFiles`' patch content is the sole source of truth for reviewed-content drift. export type AiReviewCacheInput = { // The PR title is threaded into the reviewer prompt (see runAiReviewForAdvisory's pr.title), so a same-head // `edited` event that changes only the title must miss the cache rather than replay a review generated for @@ -70,12 +80,6 @@ export type AiReviewCacheInput = { repoInstructions: string | null | undefined; excludePaths: readonly string[]; changedPaths: readonly string[]; - // A rebase or retarget (new base branch, same head commit) can change the diff GitHub reports for an - // otherwise-unchanged head SHA -- changedPaths (just the path list) stays the same when the same files - // are touched against the new base, but the actual patch content reviewed differs. baseSha plus a - // per-file content digest (path/status/patch/additions/deletions -- the fields buildAiReviewDiff and the - // grounding/RAG paths actually read) closes that gap. - baseSha: string | null | undefined; reviewFiles: readonly { path: string; status?: string | null | undefined; @@ -154,7 +158,6 @@ export async function aiReviewCacheInputFingerprint(input: AiReviewCacheInput): repoInstructions: input.repoInstructions?.trim() || null, excludePaths: normalizeStringList(input.excludePaths), changedPaths: normalizeStringList(input.changedPaths), - baseSha: input.baseSha ?? null, reviewFiles: [...input.reviewFiles] .map((file) => ({ path: file.path, diff --git a/test/unit/ai-review-cache-input.test.ts b/test/unit/ai-review-cache-input.test.ts index 35c55a434c..4a91a0a83f 100644 --- a/test/unit/ai-review-cache-input.test.ts +++ b/test/unit/ai-review-cache-input.test.ts @@ -18,7 +18,6 @@ const baseInput = (): AiReviewCacheInput => ({ gatePack: null, reviewerPlan: null, selfHostProviderConfig: null, - baseSha: null, reviewFiles: [], profile: null, securityFocus: false, @@ -119,34 +118,22 @@ describe("aiReviewCacheInputFingerprint", () => { expect(sparse).toBe(explicit); }); - it("changes when the patch content or base sha differs even though the same file paths are touched (retarget/rebase)", async () => { - // A retarget (new base branch, same head commit) or certain rebases can change the diff GitHub reports - // for an otherwise-unchanged head SHA -- changedPaths (just the path list) stays identical when the - // same files are touched against the new base, but the actual reviewed content differs. + it("changes when the patch content differs even though the same file paths are touched", async () => { const original = await aiReviewCacheInputFingerprint({ ...baseInput(), - baseSha: "base1", reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@ -1 +1 @@\n-old\n+new", additions: 1, deletions: 1 }], }); const samePathsDifferentPatch = await aiReviewCacheInputFingerprint({ ...baseInput(), - baseSha: "base1", reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@ -1 +1 @@\n-old\n+completely different", additions: 1, deletions: 1 }], }); - const samePatchDifferentBase = await aiReviewCacheInputFingerprint({ - ...baseInput(), - baseSha: "base2", - reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@ -1 +1 @@\n-old\n+new", additions: 1, deletions: 1 }], - }); const repeated = await aiReviewCacheInputFingerprint({ ...baseInput(), - baseSha: "base1", reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@ -1 +1 @@\n-old\n+new", additions: 1, deletions: 1 }], }); // File order must not matter -- only content -- so a re-fetched diff in a different row order still hits. const reordered = await aiReviewCacheInputFingerprint({ ...baseInput(), - baseSha: "base1", reviewFiles: [ { path: "src/b.ts", status: "added", patch: "@@ -0,0 +1 @@\n+export {}", additions: 1, deletions: 0 }, { path: "src/a.ts", status: "modified", patch: "@@ -1 +1 @@\n-old\n+new", additions: 1, deletions: 1 }, @@ -154,7 +141,6 @@ describe("aiReviewCacheInputFingerprint", () => { }); const reorderedAgain = await aiReviewCacheInputFingerprint({ ...baseInput(), - baseSha: "base1", reviewFiles: [ { path: "src/a.ts", status: "modified", patch: "@@ -1 +1 @@\n-old\n+new", additions: 1, deletions: 1 }, { path: "src/b.ts", status: "added", patch: "@@ -0,0 +1 @@\n+export {}", additions: 1, deletions: 0 }, @@ -162,11 +148,21 @@ describe("aiReviewCacheInputFingerprint", () => { }); expect(samePathsDifferentPatch).not.toBe(original); - expect(samePatchDifferentBase).not.toBe(original); expect(repeated).toBe(original); expect(reordered).toBe(reorderedAgain); }); + // #regate-churn (root cause, confirmed in production): `baseSha` is intentionally NOT a field on + // AiReviewCacheInput any more (see the type's own doc comment) -- the type system itself now guarantees no + // caller can (re-)introduce it. It used to be included, on the theory that a rebase/retarget can change the + // diff for an unchanged head SHA even when `changedPaths` stays the same -- but `reviewFiles`' patch content + // (asserted above) already IS that signal. Hashing raw `baseSha` on top of it was redundant when the patch is + // unchanged, and actively harmful when it isn't: it is the live tip of the base branch, which advances on + // every unrelated merge, so an active repo's same-head PR missed the cache on almost every scheduled re-gate + // sweep -- re-spending a real AI call whose non-deterministic output could even flip the published verdict, + // purely because SOME OTHER PR merged to main in between. The end-to-end "pure base movement" scenario is + // covered at the queue/sweep level in test/unit/queue.test.ts (#regate-churn). + it("normalizes a file entry with no status/patch (e.g. a rename with no content change) deterministically", async () => { const omitted = await aiReviewCacheInputFingerprint({ ...baseInput(), diff --git a/test/unit/ai-review-cache.test.ts b/test/unit/ai-review-cache.test.ts index f7e9cd10bf..34b5206297 100644 --- a/test/unit/ai-review-cache.test.ts +++ b/test/unit/ai-review-cache.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from "vitest"; -import { getCachedAiReview, putCachedAiReview } from "../../src/db/repositories"; +import { getCachedAiReview, getLatestPublishedAiReview, markAiReviewPublished, putCachedAiReview } from "../../src/db/repositories"; import { aiReviewCacheInputFingerprint, type AiReviewCacheInput } from "../../src/review/ai-review-cache-input"; import { createTestEnv } from "../helpers/d1"; @@ -17,7 +17,6 @@ const baseFingerprintInput = (): AiReviewCacheInput => ({ gatePack: null, reviewerPlan: null, selfHostProviderConfig: null, - baseSha: null, reviewFiles: [], profile: null, securityFocus: false, @@ -315,4 +314,154 @@ describe("AI review cache (#1)", () => { }); }); }); + + describe("published_at — a published row is immune to the non-cacheable cooldown (#regate-churn)", () => { + it("bypasses maxAgeMs entirely once markAiReviewPublished stamps the row, even long past the cooldown", async () => { + const env = createTestEnv(); + vi.useFakeTimers(); + try { + vi.setSystemTime(new Date("2026-07-01T00:00:00.000Z")); + await putCachedAiReview(env, "o/r", 40, "sha1", "block", { notes: "dynamic-context review", reviewerCount: 1, cacheable: false }); + await markAiReviewPublished(env, "o/r", 40, "sha1"); + + vi.setSystemTime(new Date("2026-07-01T05:00:00.000Z")); // 5 hours later — far past the 30-minute cooldown + expect( + await getCachedAiReview(env, "o/r", 40, "sha1", "block", undefined, { allowNonCacheable: true, maxAgeMs: 30 * 60 * 1000 }), + ).toEqual({ notes: "dynamic-context review", reviewerCount: 1, findings: [] }); + } finally { + vi.useRealTimers(); + } + }); + + it("still misses an UNPUBLISHED non-cacheable row past the cooldown (unchanged behavior)", async () => { + const env = createTestEnv(); + vi.useFakeTimers(); + try { + vi.setSystemTime(new Date("2026-07-01T00:00:00.000Z")); + await putCachedAiReview(env, "o/r", 41, "sha1", "block", { notes: "dynamic-context review", reviewerCount: 1, cacheable: false }); + + vi.setSystemTime(new Date("2026-07-01T00:31:00.000Z")); + expect( + await getCachedAiReview(env, "o/r", 41, "sha1", "block", undefined, { allowNonCacheable: true, maxAgeMs: 30 * 60 * 1000 }), + ).toBeNull(); + } finally { + vi.useRealTimers(); + } + }); + + it("still requires allowNonCacheable even when published — the caller's own opt-in is unaffected", async () => { + const env = createTestEnv(); + await putCachedAiReview(env, "o/r", 42, "sha1", "block", { notes: "held", reviewerCount: 1, cacheable: false }); + await markAiReviewPublished(env, "o/r", 42, "sha1"); + expect(await getCachedAiReview(env, "o/r", 42, "sha1", "block")).toBeNull(); + }); + + it("still enforces mode + input-fingerprint match on a published non-cacheable reuse", async () => { + const env = createTestEnv(); + await putCachedAiReview(env, "o/r", 43, "sha1", "block", { notes: "held", reviewerCount: 1, cacheable: false, metadata: { inputFingerprint: "fp-v1" } }); + await markAiReviewPublished(env, "o/r", 43, "sha1"); + const opts = { allowNonCacheable: true, maxAgeMs: 30 * 60 * 1000 }; + expect(await getCachedAiReview(env, "o/r", 43, "sha1", "advisory", undefined, opts)).toBeNull(); // mode mismatch + expect(await getCachedAiReview(env, "o/r", 43, "sha1", "block", "fp-v2", opts)).toBeNull(); // fingerprint mismatch (content actually changed) + expect(await getCachedAiReview(env, "o/r", 43, "sha1", "block", "fp-v1", opts)).toEqual({ notes: "held", reviewerCount: 1, findings: [], metadata: { inputFingerprint: "fp-v1" } }); + }); + + it("is a no-op with no matching row (nullish head SHA, or a head that was never written)", async () => { + const env = createTestEnv(); + await expect(markAiReviewPublished(env, "o/r", 44, null)).resolves.toBeUndefined(); + await expect(markAiReviewPublished(env, "o/r", 44, "never-written-sha")).resolves.toBeUndefined(); + }); + + it("is idempotent — a second call never rewrites an already-published timestamp", async () => { + const env = createTestEnv(); + vi.useFakeTimers(); + try { + vi.setSystemTime(new Date("2026-07-01T00:00:00.000Z")); + await putCachedAiReview(env, "o/r", 45, "sha1", "block", { notes: "held", reviewerCount: 1, cacheable: false }); + await markAiReviewPublished(env, "o/r", 45, "sha1"); + const first = await env.DB.prepare("SELECT published_at AS publishedAt FROM ai_review_cache WHERE repo_full_name = ? AND pull_number = ? AND head_sha = ?").bind("o/r", 45, "sha1").first<{ publishedAt: string }>(); + + vi.setSystemTime(new Date("2026-07-01T01:00:00.000Z")); + await markAiReviewPublished(env, "o/r", 45, "sha1"); + const second = await env.DB.prepare("SELECT published_at AS publishedAt FROM ai_review_cache WHERE repo_full_name = ? AND pull_number = ? AND head_sha = ?").bind("o/r", 45, "sha1").first<{ publishedAt: string }>(); + + expect(second?.publishedAt).toBe(first?.publishedAt); + } finally { + vi.useRealTimers(); + } + }); + + it("a fresh write (a real subject change) resets published_at back to unpublished", async () => { + const env = createTestEnv(); + await putCachedAiReview(env, "o/r", 46, "sha1", "block", { notes: "held", reviewerCount: 1, cacheable: false }); + await markAiReviewPublished(env, "o/r", 46, "sha1"); + // Same head SHA, but a genuinely different review content overwrites the row (e.g. a corrected retry) — + // the NEW content has not been published yet, so the stale publish marker must not leak onto it. + await putCachedAiReview(env, "o/r", 46, "sha1", "block", { notes: "revised", reviewerCount: 1, cacheable: false }); + const row = await env.DB.prepare("SELECT published_at AS publishedAt FROM ai_review_cache WHERE repo_full_name = ? AND pull_number = ? AND head_sha = ?").bind("o/r", 46, "sha1").first<{ publishedAt: string | null }>(); + expect(row?.publishedAt).toBeNull(); + expect(await getCachedAiReview(env, "o/r", 46, "sha1", "block", undefined, { allowNonCacheable: true, maxAgeMs: 30 * 60 * 1000 })).toEqual({ notes: "revised", reviewerCount: 1, findings: [] }); + }); + }); + + describe("getLatestPublishedAiReview — maintainer-gated freeze reuse across a head-SHA change (#regate-churn)", () => { + it("misses when nothing has ever been published for this PR", async () => { + const env = createTestEnv(); + await putCachedAiReview(env, "o/r", 50, "sha1", "block", { notes: "unpublished", reviewerCount: 1 }); + expect(await getLatestPublishedAiReview(env, "o/r", 50, "block")).toBeNull(); + }); + + it("returns the most recently PUBLISHED review across DIFFERENT head SHAs (a contributor push while held)", async () => { + const env = createTestEnv(); + await putCachedAiReview(env, "o/r", 51, "sha1", "block", { notes: "first review", reviewerCount: 1 }); + await markAiReviewPublished(env, "o/r", 51, "sha1"); + // A newer head SHA exists (the contributor pushed again), but was never independently published. + await putCachedAiReview(env, "o/r", 51, "sha2", "block", { notes: "never published", reviewerCount: 1 }); + + expect(await getLatestPublishedAiReview(env, "o/r", 51, "block")).toEqual({ notes: "first review", reviewerCount: 1, findings: [] }); + }); + + it("respects the ai_review_mode filter, same as getCachedAiReview", async () => { + const env = createTestEnv(); + await putCachedAiReview(env, "o/r", 52, "sha1", "advisory", { notes: "advisory mode", reviewerCount: 1 }); + await markAiReviewPublished(env, "o/r", 52, "sha1"); + expect(await getLatestPublishedAiReview(env, "o/r", 52, "block")).toBeNull(); + expect(await getLatestPublishedAiReview(env, "o/r", 52, "advisory")).toEqual({ notes: "advisory mode", reviewerCount: 1, findings: [] }); + }); + + it("round-trips findings and metadata like getCachedAiReview", async () => { + const env = createTestEnv(); + await putCachedAiReview(env, "o/r", 53, "sha1", "block", { + notes: "held review", + reviewerCount: 2, + findings: [{ code: "ai_review_split", severity: "critical", title: "Split", detail: "One reviewer blocked." }], + metadata: { inputFingerprint: "fp-v1" }, + }); + await markAiReviewPublished(env, "o/r", 53, "sha1"); + expect(await getLatestPublishedAiReview(env, "o/r", 53, "block")).toEqual({ + notes: "held review", + reviewerCount: 2, + findings: [{ code: "ai_review_split", severity: "critical", title: "Split", detail: "One reviewer blocked." }], + metadata: { inputFingerprint: "fp-v1" }, + }); + }); + + it("picks the LATEST published head when more than one head was independently published", async () => { + const env = createTestEnv(); + vi.useFakeTimers(); + try { + vi.setSystemTime(new Date("2026-07-01T00:00:00.000Z")); + await putCachedAiReview(env, "o/r", 54, "sha1", "block", { notes: "older published review", reviewerCount: 1 }); + await markAiReviewPublished(env, "o/r", 54, "sha1"); + + vi.setSystemTime(new Date("2026-07-01T01:00:00.000Z")); + await putCachedAiReview(env, "o/r", 54, "sha2", "block", { notes: "newer published review", reviewerCount: 1 }); + await markAiReviewPublished(env, "o/r", 54, "sha2"); + + expect(await getLatestPublishedAiReview(env, "o/r", 54, "block")).toEqual({ notes: "newer published review", reviewerCount: 1, findings: [] }); + } finally { + vi.useRealTimers(); + } + }); + }); }); diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index c0f26b5412..2897198c56 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -49,6 +49,7 @@ import { upsertRepositorySettings, upsertRepositoryFromGitHub, putCachedAiReview, + markAiReviewPublished, } from "../../src/db/repositories"; import { agentMaintenanceHeadMatchesGate, changedPathsForGuardrail, claimAiReviewLock, claimPrActuationLock, contributorEvidenceBatchSize, enrichOpenPullRequestsWithChangedFiles, processJob, reconcileLiveDuplicateSiblings, releaseAiReviewLock, releasePrActuationLock } from "../../src/queue/processors"; import type { PullRequestRecord } from "../../src/types"; @@ -3074,7 +3075,7 @@ describe("queue processors", () => { metadata: { inputFingerprint: await aiReviewCacheInputFingerprint({ title: "Clean PR", mode: "block", byok: false, provider: null, model: null, aiReviewAllAuthors: false, - aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, baseSha: null, + aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], profile: null, securityFocus: false, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], changedPaths: ["src/a.ts"], features: { grounding: false, rag: false, enrichment: false, reputation: false }, @@ -3184,7 +3185,6 @@ describe("queue processors", () => { 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, securityFocus: false, @@ -3264,7 +3264,7 @@ describe("queue processors", () => { metadata: { inputFingerprint: await aiReviewCacheInputFingerprint({ title: "Current PR", mode: "block", byok: false, provider: null, model: null, aiReviewAllAuthors: false, - aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, baseSha: null, + aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], profile: null, securityFocus: false, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], changedPaths: ["src/a.ts"], features: { grounding: false, rag: false, enrichment: false, reputation: false }, @@ -3321,7 +3321,7 @@ describe("queue processors", () => { metadata: { inputFingerprint: await aiReviewCacheInputFingerprint({ title: "Current PR", mode: "block", byok: false, provider: null, model: null, aiReviewAllAuthors: false, - aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, baseSha: null, + aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], profile: null, securityFocus: false, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], changedPaths: ["src/a.ts"], features: { grounding: false, rag: false, enrichment: false, reputation: false }, @@ -3377,7 +3377,7 @@ describe("queue processors", () => { metadata: { inputFingerprint: await aiReviewCacheInputFingerprint({ title: "Current PR", mode: "block", byok: false, provider: null, model: null, aiReviewAllAuthors: false, - aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, baseSha: null, + aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], profile: null, securityFocus: false, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], changedPaths: ["src/a.ts"], features: { grounding: false, rag: false, enrichment: false, reputation: false }, @@ -3430,7 +3430,7 @@ describe("queue processors", () => { metadata: { inputFingerprint: await aiReviewCacheInputFingerprint({ title: "Current PR", mode: "block", byok: false, provider: null, model: null, aiReviewAllAuthors: false, - aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, baseSha: null, + aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], profile: null, securityFocus: false, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], changedPaths: ["src/a.ts"], features: { grounding: false, rag: false, enrichment: false, reputation: false }, @@ -3479,7 +3479,7 @@ describe("queue processors", () => { metadata: { inputFingerprint: await aiReviewCacheInputFingerprint({ title: "Partially published PR", mode: "block", byok: false, provider: null, model: null, aiReviewAllAuthors: false, - aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, baseSha: null, + aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], profile: null, securityFocus: false, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], changedPaths: ["src/a.ts"], features: { grounding: false, rag: false, enrichment: false, reputation: false }, @@ -3531,7 +3531,7 @@ describe("queue processors", () => { metadata: { inputFingerprint: await aiReviewCacheInputFingerprint({ title: "Current PR", mode: "block", byok: false, provider: null, model: null, aiReviewAllAuthors: false, - aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, baseSha: null, + aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], profile: null, securityFocus: false, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], changedPaths: ["src/a.ts"], features: { grounding: false, rag: false, enrichment: false, reputation: false }, @@ -3673,7 +3673,14 @@ describe("queue processors", () => { 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 () => { + it("#9: a low-activity repo's old open PR NEVER generates a repeated AI review across many sweep ticks once published (#regate-churn)", async () => { + // Superseded policy note: this used to assert a BOUNDED, periodic retry (one fresh attempt per tick once + // AI_REVIEW_NON_CACHEABLE_RETRY_COOLDOWN_MS elapsed) for a never-durably-cacheable (still-inconclusive) + // outcome. That was itself the incident-mitigation for #1462, but it was still an UNBOUNDED total spend + // over a PR's lifetime (one fresh call every cooldown window, forever, for as long as the PR stayed open + // and inconclusive). The `published_at` marker (this PR) makes ANY review — cacheable or not — immutable + // for its exact head+fingerprint the moment it is actually published: a tick past the cooldown no longer + // buys a fresh attempt at all; only a real content/config change or an explicit maintainer force-rerun does. let aiCalls = 0; const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), @@ -3707,27 +3714,522 @@ describe("queue processors", () => { 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. + // repo), each beyond what used to be the 30-minute cooldown — the unchanged PR's published review is now + // reused indefinitely, so NONE of these buy a fresh attempt. 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 + expect(aiCalls).toBe(callsPerAttempt); // still just the one, original attempt - // Now tighten four ticks to well INSIDE the cooldown, mirroring the incident's actual ~2-10 minute cadence. - const aiCallsBeforeTightTicks = aiCalls; + // Tighten four more ticks to well INSIDE what used to be the cooldown window — still zero additional spend. 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); + expect(aiCalls).toBe(callsPerAttempt); + }); + + describe("#regate-churn: production reproductions (#3379, #3383) and the maintainer-gated freeze", () => { + it("REPRODUCES #3379: a comment_and_label repo's unchanged PR gets no additional AI calls, no comment PATCH, and no re-created comment across repeated regate-sweep passes — label repair keeps running", 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, { publicSurface: "comment_and_label" }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 70, title: "Fix the retry loop", 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() }); + + const stickyComment: { current: { id: number; body: string } | null } = { current: null }; + let commentPosts = 0; + let commentPatches = 0; + let labelPosts = 0; + 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: "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: "Fix the retry loop", state: "open", user: { login: "contributor" }, head: { sha: "a70" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/commits/a70/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + 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("/issues/70/comments") && method === "GET") { + return Response.json(stickyComment.current ? [{ ...stickyComment.current, user: { login: "gittensory[bot]", type: "Bot" } }] : []); + } + if (url.includes("/issues/70/comments") && method === "POST") { + commentPosts += 1; + const body = String((JSON.parse(String(init?.body ?? "{}")) as { body?: string }).body ?? ""); + stickyComment.current = { id: 1, body }; + return Response.json({ id: 1 }, { status: 201 }); + } + if (url.includes("/issues/comments/1") && method === "PATCH") { + commentPatches += 1; + const body = String((JSON.parse(String(init?.body ?? "{}")) as { body?: string }).body ?? ""); + stickyComment.current = { id: 1, body }; + return Response.json({ id: 1 }, { status: 200 }); + } + // The label GET always reports "missing" -- proving label repair keeps re-applying it every pass, + // independent of the AI-review freeze/reuse logic (labels/assignees must repair without rewriting + // the final review comment). + if (url.includes("/issues/70/labels") && method === "GET") return Response.json([]); + if (url.includes("/issues/70/labels") && method === "POST") { + labelPosts += 1; + return Response.json([]); + } + 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: "webhook-open", repoFullName: "JSONbored/gittensory", prNumber: 70, installationId: 123 }); + const aiCallsAfterFirst = aiCalls; + expect(aiCallsAfterFirst).toBeGreaterThan(0); + expect(commentPosts).toBe(1); // the very first comment is a CREATE (placeholder, then patched to final) + const patchesAfterFirst = commentPatches; + expect(stickyComment.current?.body).not.toContain("is reviewing"); // settled to the final verdict + const finalBody = stickyComment.current?.body; + const labelPostsAfterFirst = labelPosts; + expect(labelPostsAfterFirst).toBeGreaterThan(0); + + // Two later scheduled regate-sweep passes, matching the production delivery-id shape, over the SAME head. + await processJob(env, { type: "agent-regate-pr", deliveryId: "regate-sweep:JSONbored/gittensory#70:1", repoFullName: "JSONbored/gittensory", prNumber: 70, installationId: 123 }); + await processJob(env, { type: "agent-regate-pr", deliveryId: "regate-sweep:JSONbored/gittensory#70:2", repoFullName: "JSONbored/gittensory", prNumber: 70, installationId: 123 }); + + expect(aiCalls).toBe(aiCallsAfterFirst); // no additional AI calls + expect(commentPosts).toBe(1); // never a second CREATE (no duplicate comment thread) + expect(commentPatches).toBe(patchesAfterFirst); // no additional PATCH -- content is byte-identical, never rewritten + expect(stickyComment.current?.body).toBe(finalBody); // the published comment never changed + expect(labelPosts).toBeGreaterThan(labelPostsAfterFirst); // label repair keeps running on every later pass + + const reuseAudit = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and target_key = ?") + .bind("github_app.ai_review_cache_hit", "JSONbored/gittensory#70") + .first<{ n: number }>(); + expect(reuseAudit?.n).toBe(2); // both later passes explicitly reused the durable cache + }); + + it("a PURE base-branch movement (no reviewed content change) triggers neither a fresh AI review nor a comment rewrite", 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, { publicSurface: "comment_only" }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 71, title: "Quiet 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() }); + + let baseSha = "main-tip-1"; + const stickyComment: { current: { id: number; body: string } | null } = { current: null }; + 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: "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;" }]); + // base.sha is the LIVE tip of the target branch -- it advances on every unrelated merge to it, with the + // reviewed file's own patch content completely unaffected (#regate-churn root cause). + if (url.endsWith("/pulls/71")) return Response.json({ number: 71, title: "Quiet PR", state: "open", user: { login: "contributor" }, head: { sha: "a71" }, base: { sha: baseSha }, labels: [], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/commits/a71/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + 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("/issues/71/comments") && method === "GET") { + return Response.json(stickyComment.current ? [{ ...stickyComment.current, user: { login: "gittensory[bot]", type: "Bot" } }] : []); + } + if (url.includes("/issues/71/comments") && method === "POST") { + const body = String((JSON.parse(String(init?.body ?? "{}")) as { body?: string }).body ?? ""); + stickyComment.current = { id: 1, body }; + return Response.json({ id: 1 }, { status: 201 }); + } + if (url.includes("/issues/comments/1") && method === "PATCH") { + const body = String((JSON.parse(String(init?.body ?? "{}")) as { body?: string }).body ?? ""); + stickyComment.current = { id: 1, body }; + return Response.json({ id: 1 }, { status: 200 }); + } + 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: "base-move-1", repoFullName: "JSONbored/gittensory", prNumber: 71, installationId: 123 }); + const aiCallsAfterFirst = aiCalls; + expect(aiCallsAfterFirst).toBeGreaterThan(0); + const finalBody = stickyComment.current?.body; + + // Main moved (some OTHER PR merged) -- the PR's own reviewed content is completely unchanged. + baseSha = "main-tip-2"; + await processJob(env, { type: "agent-regate-pr", deliveryId: "base-move-2", repoFullName: "JSONbored/gittensory", prNumber: 71, installationId: 123 }); + + expect(aiCalls).toBe(aiCallsAfterFirst); // no fresh AI review + expect(stickyComment.current?.body).toBe(finalBody); // no comment rewrite + }); + + it("a REAL contributor code change DOES trigger a fresh AI review and an updated comment", async () => { + let aiCalls = 0; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { + run: async () => { + aiCalls += 1; + return { response: JSON.stringify({ assessment: aiCalls === 1 ? "Looks fine." : "Second look also 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, { publicSurface: "comment_only" }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 72, title: "Evolving PR", state: "open", user: { login: "contributor" }, head: { sha: "a72" }, labels: [], body: "Closes #1" }); + await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 72, status: "complete", reviewsSyncedAt: new Date().toISOString() }); + + let headSha = "a72"; + let patch = "@@\n+export const ok = true;"; + const stickyComment: { current: { id: number; body: string } | null } = { current: null }; + 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: "fake-installation-token" }); + if (url.includes("/pulls/72/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch }]); + if (url.endsWith("/pulls/72")) return Response.json({ number: 72, title: "Evolving PR", state: "open", user: { login: "contributor" }, head: { sha: headSha }, labels: [], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes(`/commits/${headSha}/check-runs`)) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes(`/commits/${headSha}/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("/issues/72/comments") && method === "GET") { + return Response.json(stickyComment.current ? [{ ...stickyComment.current, user: { login: "gittensory[bot]", type: "Bot" } }] : []); + } + if (url.includes("/issues/72/comments") && method === "POST") { + const body = String((JSON.parse(String(init?.body ?? "{}")) as { body?: string }).body ?? ""); + stickyComment.current = { id: 1, body }; + return Response.json({ id: 1 }, { status: 201 }); + } + if (url.includes("/issues/comments/1") && method === "PATCH") { + const body = String((JSON.parse(String(init?.body ?? "{}")) as { body?: string }).body ?? ""); + stickyComment.current = { id: 1, body }; + return Response.json({ id: 1 }, { status: 200 }); + } + 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: "content-change-1", repoFullName: "JSONbored/gittensory", prNumber: 72, installationId: 123 }); + expect(aiCalls).toBeGreaterThan(0); + const aiCallsAfterFirst = aiCalls; + const firstBody = stickyComment.current?.body; + + // The contributor genuinely pushes new code: a new head SHA with different patch content. + headSha = "a72-v2"; + patch = "@@\n+export const ok = false; // real change"; + await processJob(env, { type: "agent-regate-pr", deliveryId: "content-change-2", repoFullName: "JSONbored/gittensory", prNumber: 72, installationId: 123 }); + + expect(aiCalls).toBeGreaterThan(aiCallsAfterFirst); // a fresh review IS allowed for genuinely new content + expect(stickyComment.current?.body).not.toBe(firstBody); // the comment reflects the new review + }); + + it("REPRODUCES the #3383 class: re-evaluating an unchanged, already-published subject never re-runs AI, so a published verdict cannot flip on its own", async () => { + let aiCalls = 0; + let secondPassStarted = false; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + // The mock is DELIBERATELY configured to flip to a blocking verdict for every call AFTER the first + // review PASS (not the first raw call -- a single pass can make more than one underlying `env.AI.run` + // call via dual-reviewer behavior, so gating on `secondPassStarted` keeps every call within one pass + // consistent). If the fix regressed and AI ran again for the same unchanged subject, this would flip + // the published gate from success to failure, exactly reproducing #3383's "held -> close" flip. + AI: { + run: async () => { + aiCalls += 1; + if (!secondPassStarted) return { response: JSON.stringify({ assessment: "Looks fine.", blockers: [], nits: [], suggestions: [] }) }; + return { response: JSON.stringify({ assessment: "Critical defect found on re-run.", blockers: ["x"], nits: [], suggestions: [] }) }; + }, + } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await seedRegateChurnRepo(env, { publicSurface: "comment_only" }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 73, title: "CI-settling PR", state: "open", user: { login: "contributor" }, head: { sha: "a73" }, labels: [], body: "Closes #1" }); + await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 73, status: "complete", reviewsSyncedAt: new Date().toISOString() }); + + const stickyComment: { current: { id: number; body: string } | null } = { current: null }; + 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: "fake-installation-token" }); + if (url.includes("/pulls/73/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/73")) return Response.json({ number: 73, title: "CI-settling PR", state: "open", user: { login: "contributor" }, head: { sha: "a73" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/commits/a73/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/a73/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("/issues/73/comments") && method === "GET") { + return Response.json(stickyComment.current ? [{ ...stickyComment.current, user: { login: "gittensory[bot]", type: "Bot" } }] : []); + } + if (url.includes("/issues/73/comments") && method === "POST") { + const body = String((JSON.parse(String(init?.body ?? "{}")) as { body?: string }).body ?? ""); + stickyComment.current = { id: 1, body }; + return Response.json({ id: 1 }, { status: 201 }); + } + if (url.includes("/issues/comments/1") && method === "PATCH") { + const body = String((JSON.parse(String(init?.body ?? "{}")) as { body?: string }).body ?? ""); + stickyComment.current = { id: 1, body }; + return Response.json({ id: 1 }, { status: 200 }); + } + if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } }); + return Response.json({}); + }); + + // Pass 1: CI already settled — the review runs to completion and publishes a clean verdict. + await processJob(env, { type: "agent-regate-pr", deliveryId: "ci-settle-1", repoFullName: "JSONbored/gittensory", prNumber: 73, installationId: 123 }); + const callsAfterFirstPass = aiCalls; + expect(callsAfterFirstPass).toBeGreaterThan(0); + expect(stickyComment.current?.body).not.toContain("Critical defect"); + secondPassStarted = true; // any call from here on would prove a flip-prone re-run happened + + // Pass 2: a later re-evaluation of the SAME unchanged subject (e.g. triggered by a check-run/check-suite + // completion webhook re-firing the review). Must reuse the published result, not spend a second AI call. + await processJob(env, { type: "agent-regate-pr", deliveryId: "ci-settle-2", repoFullName: "JSONbored/gittensory", prNumber: 73, installationId: 123 }); + expect(aiCalls).toBe(callsAfterFirstPass); // the flip-prone second pass never runs AI at all + expect(stickyComment.current?.body).not.toContain("Critical defect"); // the published verdict never flips + }); + + it("an explicit maintainer force-rerun bypasses the published snapshot and pays for a fresh AI call, with a distinct audit reason", 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, { publicSurface: "comment_only" }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 74, title: "Force re-gate PR", state: "open", user: { login: "contributor" }, head: { sha: "a74" }, labels: [], body: "Closes #1" }); + await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 74, 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: "fake-installation-token" }); + if (url.includes("/pulls/74/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/74")) return Response.json({ number: 74, title: "Force re-gate PR", state: "open", user: { login: "contributor" }, head: { sha: "a74" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/commits/a74/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/a74/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("/issues/74/comments")) return method === "POST" || method === "PATCH" ? Response.json({ id: 1 }, { status: 201 }) : Response.json([]); + 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: "force-baseline", repoFullName: "JSONbored/gittensory", prNumber: 74, installationId: 123 }); + const callsPerAttempt = aiCalls; + expect(callsPerAttempt).toBeGreaterThan(0); + + // A same-head sweep tick would normally reuse — confirm that first, then force. + await processJob(env, { type: "agent-regate-pr", deliveryId: "force-would-reuse", repoFullName: "JSONbored/gittensory", prNumber: 74, installationId: 123 }); + expect(aiCalls).toBe(callsPerAttempt); + + // An explicit maintainer/collaborator retrigger (the PR-panel checkbox) sets `force` on the job. + await processJob(env, { type: "agent-regate-pr", deliveryId: "force-retrigger", repoFullName: "JSONbored/gittensory", prNumber: 74, installationId: 123, force: true }); + expect(aiCalls).toBe(callsPerAttempt * 2); // the snapshot is bypassed -- a fresh opinion is spent + + const forceAudit = await env.DB.prepare("select outcome, detail from audit_events where event_type = ? and target_key = ?") + .bind("github_app.ai_review_force_bypass", "JSONbored/gittensory#74") + .first<{ outcome: string; detail: string }>(); + expect(forceAudit?.outcome).toBe("completed"); + expect(forceAudit?.detail).toContain("explicit force re-gate bypassed"); + }); + + it("maintainer-gated freeze: a PR already held for manual review does not spend a fresh AI call on a later contributor push, even to a NEW head SHA", async () => { + let aiCalls = 0; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => { aiCalls += 1; return { response: JSON.stringify({ assessment: "Fresh (should not happen while frozen).", blockers: [], nits: [], suggestions: [] }) }; } } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await seedRegateChurnRepo(env, { publicSurface: "comment_only" }); + // The PR is already carrying the manual-review label from a PRIOR pass (the disposition already held it), + // and a review for its ORIGINAL head SHA was already published — the exact precondition the freeze exists + // to protect: the contributor keeps pushing while waiting for a maintainer to actually look. + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 75, title: "Held PR", state: "open", user: { login: "contributor" }, head: { sha: "a75-v1" }, labels: [{ name: "manual-review" }], body: "Closes #1" }); + await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 75, status: "complete", reviewsSyncedAt: new Date().toISOString() }); + await putCachedAiReview(env, "JSONbored/gittensory", 75, "a75-v1", "block", { notes: "Original held review.", reviewerCount: 1 }); + await markAiReviewPublished(env, "JSONbored/gittensory", 75, "a75-v1"); + + const stickyComment: { current: { id: number; body: string } | null } = { current: null }; + 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: "fake-installation-token" }); + // The contributor pushed AGAIN: a genuinely new head SHA, still carrying the manual-review label. + if (url.includes("/pulls/75/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 2, deletions: 0, changes: 2, patch: "@@\n+export const ok = true;\n+export const also = 1;" }]); + if (url.endsWith("/pulls/75")) return Response.json({ number: 75, title: "Held PR", state: "open", user: { login: "contributor" }, head: { sha: "a75-v2" }, labels: [{ name: "manual-review" }], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/commits/a75-v2/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/a75-v2/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("/issues/75/comments") && method === "GET") { + return Response.json(stickyComment.current ? [{ ...stickyComment.current, user: { login: "gittensory[bot]", type: "Bot" } }] : []); + } + if (url.includes("/issues/75/comments") && method === "POST") { + const body = String((JSON.parse(String(init?.body ?? "{}")) as { body?: string }).body ?? ""); + stickyComment.current = { id: 1, body }; + return Response.json({ id: 1 }, { status: 201 }); + } + if (url.includes("/issues/comments/1") && method === "PATCH") { + const body = String((JSON.parse(String(init?.body ?? "{}")) as { body?: string }).body ?? ""); + stickyComment.current = { id: 1, body }; + return Response.json({ id: 1 }, { status: 200 }); + } + 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: "held-push-retry", repoFullName: "JSONbored/gittensory", prNumber: 75, installationId: 123 }); + + expect(aiCalls).toBe(0); // frozen -- the new push never bought a fresh AI review + expect(stickyComment.current?.body).toContain("Original held review."); // the OLD published verdict is reused + const freezeAudit = await env.DB.prepare("select outcome, detail from audit_events where event_type = ? and target_key = ?") + .bind("github_app.ai_review_frozen_reuse", "JSONbored/gittensory#75") + .first<{ outcome: string; detail: string }>(); + expect(freezeAudit?.outcome).toBe("completed"); + expect(freezeAudit?.detail).toContain("held for manual review"); + + // An explicit maintainer/collaborator retrigger unfreezes it — a fresh AI call IS spent. + await processJob(env, { type: "agent-regate-pr", deliveryId: "held-push-force-retrigger", repoFullName: "JSONbored/gittensory", prNumber: 75, installationId: 123, force: true }); + expect(aiCalls).toBeGreaterThan(0); + const bypassAudit = await env.DB.prepare("select outcome from audit_events where event_type = ? and target_key = ?") + .bind("github_app.ai_review_force_bypass", "JSONbored/gittensory#75") + .first<{ outcome: string }>(); + expect(bypassAudit?.outcome).toBe("completed"); // the retrigger genuinely bypassed the freeze, not just a coincidental reuse + }); + + it("maintainer-gated freeze never engages when manualReviewLabel is explicitly disabled (null)", async () => { + let aiCalls = 0; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => { aiCalls += 1; return { response: JSON.stringify({ assessment: "Fresh.", blockers: [], nits: [], suggestions: [] }) }; } } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await seedRegateChurnRepo(env, { publicSurface: "comment_only" }); + // manualReviewLabel is config-as-code only (.gittensory.yml), not a DB-backed repository setting. + await upsertRepoFocusManifest(env, "JSONbored/gittensory", { settings: { manualReviewLabel: null } }); + // The PR carries the literal "manual-review" text as a label, but with the mechanism disabled repo-wide + // there is no configured label to match against — the freeze must never engage on text alone. + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 76, title: "Held PR, mechanism disabled", state: "open", user: { login: "contributor" }, head: { sha: "a76" }, labels: [{ name: "manual-review" }], body: "Closes #1" }); + await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 76, status: "complete", reviewsSyncedAt: new Date().toISOString() }); + await putCachedAiReview(env, "JSONbored/gittensory", 76, "a76-old", "block", { notes: "Old.", reviewerCount: 1 }); + await markAiReviewPublished(env, "JSONbored/gittensory", 76, "a76-old"); + 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: "fake-installation-token" }); + if (url.includes("/pulls/76/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/76")) return Response.json({ number: 76, title: "Held PR, mechanism disabled", state: "open", user: { login: "contributor" }, head: { sha: "a76" }, labels: [{ name: "manual-review" }], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/commits/a76/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/a76/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("/issues/76/comments")) return method === "POST" || method === "PATCH" ? Response.json({ id: 1 }, { status: 201 }) : Response.json([]); + 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: "disabled-mechanism", repoFullName: "JSONbored/gittensory", prNumber: 76, installationId: 123 }); + + expect(aiCalls).toBeGreaterThan(0); // NOT frozen -- a fresh review runs normally + const freezeAudit = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and target_key = ?") + .bind("github_app.ai_review_frozen_reuse", "JSONbored/gittensory#76") + .first<{ n: number }>(); + expect(freezeAudit?.n).toBe(0); + }); + + it("maintainer-gated freeze: a held PR with nothing ever published falls through gracefully (no reuse, no crash, no fresh AI while frozen)", async () => { + let aiCalls = 0; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => { aiCalls += 1; return { response: JSON.stringify({ assessment: "Fresh.", blockers: [], nits: [], suggestions: [] }) }; } } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await seedRegateChurnRepo(env, { publicSurface: "comment_only" }); + // Carries the manual-review label (e.g. a non-AI hold reason such as a protected-author close-withheld + // hold), but AI review was OFF/skipped when that hold was established -- so nothing was ever published. + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 77, title: "Held, never reviewed by AI", state: "open", user: { login: "contributor" }, head: { sha: "a77" }, labels: [{ name: "manual-review" }], body: "Closes #1" }); + await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 77, 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: "fake-installation-token" }); + if (url.includes("/pulls/77/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/77")) return Response.json({ number: 77, title: "Held, never reviewed by AI", state: "open", user: { login: "contributor" }, head: { sha: "a77" }, labels: [{ name: "manual-review" }], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/commits/a77/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/a77/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("/issues/77/comments")) return method === "POST" || method === "PATCH" ? Response.json({ id: 1 }, { status: 201 }) : Response.json([]); + if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } }); + return Response.json({}); + }); + + await expect( + processJob(env, { type: "agent-regate-pr", deliveryId: "held-never-published", repoFullName: "JSONbored/gittensory", prNumber: 77, installationId: 123 }), + ).resolves.toBeUndefined(); + + expect(aiCalls).toBe(0); // still frozen -- no fresh call, even though there was nothing to reuse either + const freezeAudit = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and target_key = ?") + .bind("github_app.ai_review_frozen_reuse", "JSONbored/gittensory#77") + .first<{ n: number }>(); + expect(freezeAudit?.n).toBe(0); // nothing was actually reused, so no reuse audit either + }); + + it("swallows a getLatestPublishedAiReview read failure and a frozen-reuse audit write failure without throwing", async () => { + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => ({ response: JSON.stringify({ assessment: "Fresh.", blockers: [], nits: [], suggestions: [] }) }) } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await seedRegateChurnRepo(env, { publicSurface: "comment_only" }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 78, title: "Held PR, flaky reads", state: "open", user: { login: "contributor" }, head: { sha: "a78" }, labels: [{ name: "manual-review" }], body: "Closes #1" }); + await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 78, status: "complete", reviewsSyncedAt: new Date().toISOString() }); + await putCachedAiReview(env, "JSONbored/gittensory", 78, "a78-old", "block", { notes: "Old.", reviewerCount: 1 }); + await markAiReviewPublished(env, "JSONbored/gittensory", 78, "a78-old"); + 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: "fake-installation-token" }); + if (url.includes("/pulls/78/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/78")) return Response.json({ number: 78, title: "Held PR, flaky reads", state: "open", user: { login: "contributor" }, head: { sha: "a78" }, labels: [{ name: "manual-review" }], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/commits/a78/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/a78/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("/issues/78/comments")) return method === "POST" || method === "PATCH" ? Response.json({ id: 1 }, { status: 201 }) : Response.json([]); + if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } }); + return Response.json({}); + }); + + const readSpy = vi.spyOn(repositoriesModule, "getLatestPublishedAiReview").mockRejectedValueOnce(new Error("D1 read error")); + await expect( + processJob(env, { type: "agent-regate-pr", deliveryId: "frozen-read-fails", repoFullName: "JSONbored/gittensory", prNumber: 78, installationId: 123 }), + ).resolves.toBeUndefined(); + readSpy.mockRestore(); + + const originalRecordAuditEvent = repositoriesModule.recordAuditEvent; + const auditSpy = vi.spyOn(repositoriesModule, "recordAuditEvent").mockImplementation(async (auditEnv, event) => { + if (event.eventType === "github_app.ai_review_frozen_reuse") throw new Error("audit DB down"); + await originalRecordAuditEvent(auditEnv, event); + }); + await expect( + processJob(env, { type: "agent-regate-pr", deliveryId: "frozen-audit-fails", repoFullName: "JSONbored/gittensory", prNumber: 78, installationId: 123 }), + ).resolves.toBeUndefined(); + auditSpy.mockRestore(); }); }); + }); it("#1: the block-mode re-gate sweep replays cached AI findings before gate evaluation", async () => { let aiCalls = 0; @@ -3758,7 +4260,6 @@ describe("queue processors", () => { 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 = value.length;", additions: 1, deletions: 0 }], profile: null, securityFocus: false, @@ -4108,16 +4609,18 @@ describe("queue processors", () => { expect(aiCalls).toBeGreaterThan(0); }); - it("reuses a dynamic-context (grounding) AI review within the bounded cooldown, then re-runs once it expires (#2119, #regate-churn)", async () => { + it("reuses a dynamic-context (grounding) AI review indefinitely once published, even long past the old cooldown window (#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 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. + // themselves flipping — so treating a hit here as an INDEFINITELY durable result BEFORE it is ever published + // 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) FIRST changed this to a bounded, + // non-durable reuse (AI_REVIEW_NON_CACHEABLE_RETRY_COOLDOWN_MS) — but that bound was itself still an + // UNBOUNDED total spend over the PR's lifetime (one fresh call every cooldown window, forever). Once the + // review has actually been PUBLISHED to the PR, `published_at` makes it authoritative for its exact + // head+fingerprint regardless of how much time elapses — only a real content/config change or an explicit + // maintainer force-rerun may spend another one. let aiCalls = 0; const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), @@ -4180,21 +4683,24 @@ describe("queue processors", () => { await processJob(env, { ...webhook, deliveryId: "dynamic-context-bypass-1" }); const firstRunAiCalls = aiCalls; expect(firstRunAiCalls).toBeGreaterThan(0); - const cached = await env.DB.prepare("select cacheable from ai_review_cache where repo_full_name = ? and pull_number = ? and head_sha = ?") + const cached = await env.DB.prepare("select cacheable, published_at as publishedAt 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 + .first<{ cacheable: number; publishedAt: string | null }>(); + expect(cached?.cacheable).toBe(0); // never durably cacheable on its own merits + expect(cached?.publishedAt).not.toBeNull(); // but it WAS published to the PR this pass - // Re-review of the SAME head with the SAME (unchanged) inputs, still WITHIN the bounded cooldown: reused, no - // additional LLM spend. + // Re-review of the SAME head with the SAME (unchanged) inputs, shortly after: 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 + expect(aiCalls).toBe(firstRunAiCalls); - // 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); + // What used to be the cooldown window (30 min) elapses, then a full day, then a full month — the published + // snapshot is authoritative regardless: none of these buy a fresh call. + for (const later of ["2026-05-28T00:31:00.000Z", "2026-05-29T00:00:00.000Z", "2026-06-28T00:00:00.000Z"]) { + vi.setSystemTime(new Date(later)); + await processJob(env, { ...webhook, deliveryId: `dynamic-context-bypass-later-${later}` }); + } + expect(aiCalls).toBe(firstRunAiCalls); }); it("continues to final verdict when the reviewing placeholder audit write fails", async () => {