diff --git a/docker-compose.yml b/docker-compose.yml index 01ae24a282..489724610a 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -578,7 +578,7 @@ services: command: - /bin/sh - -c - - "sh /backup-metrics.sh" + - "apk add --no-cache busybox-extras && sh /backup-metrics.sh" healthcheck: test: ["CMD-SHELL", "wget -qO- http://127.0.0.1:9101/metrics | grep -q '^gittensory_backup_latest_timestamp_seconds'"] interval: 30s diff --git a/prometheus/prometheus.yml b/prometheus/prometheus.yml index 6608cdd059..b2715e8e2f 100644 --- a/prometheus/prometheus.yml +++ b/prometheus/prometheus.yml @@ -33,6 +33,9 @@ scrape_configs: # Backup freshness from the read-only backup-exporter sidecar. The metrics exist only when the backup # profile is active, which keeps backup alerts opt-in with the backup feature. - job_name: gittensory-backup + # The exporter is a tiny BusyBox httpd wrapper around Prometheus text output. Prometheus v3 rejects + # blank Content-Type responses unless the scrape protocol is explicit. + fallback_scrape_protocol: PrometheusText0.0.4 static_configs: - targets: ["backup-exporter:9101"] scrape_interval: 60s diff --git a/src/index.ts b/src/index.ts index 2ed6eb1ab2..6b6a9ec63b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -17,7 +17,13 @@ import { isReviewExecutionJob, isSelfHostedReviewRuntime } from "./selfhost/revi import type { JobMessage } from "./types"; const app = createApp(); -const REGATE_BACKPRESSURE_TYPES = ["agent-regate-pr", "agent-regate-sweep"] as const; +// Scoped to the top-level fan-out TRIGGER only (#audit-sweep-fanout) — NOT "agent-regate-pr", whose per-repo +// backlog is normal, expected, and can legitimately stay nonzero for long periods (staggered/rate-deferred +// per-PR re-reviews), which is exactly what caused the prior broad backlog check to starve the scheduled sweep +// entirely. A pending/processing "agent-regate-sweep" message means a fan-out is already in flight; the +// per-repo drain guard (getLatestRegatedAt / isRegateSweepDraining) already protects individual repos once that +// single fan-out runs, so this only needs to stop a SECOND trigger from queuing up behind the first. +const REGATE_SWEEP_TRIGGER_TYPES = ["agent-regate-sweep"] as const; export { RateLimiter }; @@ -117,14 +123,17 @@ async function enqueueScheduledJobs(env: Env, controller: ScheduledController): return null; }) : null; - const regateBacklog = queueSnapshotBacklog(queueSnapshot, REGATE_BACKPRESSURE_TYPES); + const sweepTriggerBacklog = queueSnapshotBacklog(queueSnapshot, REGATE_SWEEP_TRIGGER_TYPES); let sweepThrottledUntil: string | undefined; if (selfHostedReviews) { sweepThrottledUntil = await shouldWaitForGitHubRateLimit(env, MAINTENANCE_RESERVED_HEADROOM); if (sweepThrottledUntil) { console.log(JSON.stringify({ event: "regate_sweep_throttled", resetAt: sweepThrottledUntil })); - } else if (regateBacklog > 0) { - console.log(JSON.stringify({ event: "regate_sweep_backlog_deferred", backlog: regateBacklog })); + } else if (sweepTriggerBacklog > 0) { + // A fan-out trigger is already pending/processing — skip re-arming so the queue never accumulates a + // second identical trigger behind the first (#audit-sweep-fanout). This is scoped to the trigger job + // itself; it does not look at (and is not blocked by) per-repo "agent-regate-pr" backlog. + console.log(JSON.stringify({ event: "regate_sweep_trigger_backlog_deferred", backlog: sweepTriggerBacklog })); } else { jobs.push({ type: "agent-regate-sweep", requestedBy: "schedule" }); } @@ -138,13 +147,12 @@ async function enqueueScheduledJobs(env: Env, controller: ScheduledController): // per-repo segment + per-PR detail sync — a large GitHub-budget consumer second only to the sweep. Gate it // behind the SAME maintenance headroom the sweep yields at, so when the shared REST budget is low the backfill // SKIPS this 30-min tick and hands the remaining budget to webhooks (which drive timely reviews); the next - // 30-min tick retries, and after the bucket resets the backfill resumes. The cheap single-call health jobs - // (repair-data-fidelity, refresh-installation-health) stay unconditional — they cost ~one call and keep - // installation/health state fresh even while the budget is reserved. - if (selfHostedReviews && !sweepThrottledUntil && regateBacklog === 0) { + // 30-min tick retries, and after the bucket resets the backfill resumes. Queue depth is deliberately not a + // suppressor here: unrelated pending work can stay nonzero for long periods, while rate admission on the + // queued jobs is the precise throttle. The cheap single-call health jobs (repair-data-fidelity, + // refresh-installation-health) stay unconditional. + if (selfHostedReviews && !sweepThrottledUntil) { jobs.push({ type: "backfill-registered-repos", requestedBy: "schedule", mode: isFullSyncWindow ? "full" : "light" }); - } else if (selfHostedReviews && regateBacklog > 0) { - console.log(JSON.stringify({ event: "backfill_backlog_deferred", backlog: regateBacklog })); } else if (selfHostedReviews) { console.log(JSON.stringify({ event: "backfill_throttled", resetAt: sweepThrottledUntil })); } diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 622a66f8e2..83beac8d8e 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -221,6 +221,7 @@ import { queueSnapshotBacklog, queueSnapshotFromBinding, } from "../selfhost/queue-common"; +import { aiReviewCacheInputFingerprint } from "../review/ai-review-cache-input"; import { downgradeCloseToHold, downgradeMergeToHold, @@ -350,7 +351,6 @@ import { emptyReviewRagTelemetry, isRagEnabled, } from "../review/rag-wire"; -import { aiReviewCacheInputFingerprint } from "../review/ai-review-cache-input"; import { buildReviewEnrichment, isEnrichmentEnabled, @@ -5508,6 +5508,7 @@ async function maybePublishPrPublicSurface( agent: "dual-ai", }, async () => { + const reviewManifest = await loadRepoFocusManifest(env, repoFullName).catch(() => null); // `.gittensory.yml` review.profile + review.path_instructions + review.exclude_paths (#review-profile / // #review-path-instructions / #review-exclude-paths): resolve from the manifest (cached from settings // resolution, so a cheap cache hit — no extra fetch) and thread them into the AI review. Profile shapes @@ -5519,63 +5520,118 @@ async function maybePublishPrPublicSurface( pathInstructions: reviewPathInstructions, instructions: manifestReviewInstructions, excludePaths: reviewExcludePaths, - } = resolveReviewPromptOverrides( - /* v8 ignore next -- fail-open manifest-read rejection is exercised in runAiReviewForAdvisory; this wrapper preserves the same fallback. */ - await loadRepoFocusManifest(env, repoFullName).catch(() => null), - ); + } = resolveReviewPromptOverrides(reviewManifest); inlineCommentsEnabledForReview = shouldRequestInlineFindings( env, repoFullName, reviewInlineComments, ); + const reviewFilesForAi = await getReviewFiles(); + const changedPaths = reviewFilesForAi.map((file) => file.path); // Per-repo review CONTEXT (#review-skills): fold the container-private review/AGENTS.md (or legacy // review/CLAUDE.md) guide + the matching review/skills/*.md modules into the SAME review-instructions slot, // so reviews follow each repo's conventions. // Glob-gated for cost (only skills matching the changed files are injected); absent config dir ⇒ empty ⇒ - // byte-identical prompt. getReviewFiles() is memoized, so this reuses the loaded diff. - const reviewFilesForAi = await getReviewFiles(); - const changedReviewPaths = reviewFilesForAi.map((file) => file.path); + // byte-identical prompt. getReviewFiles() is memoized, so the second call reuses the loaded diff. const reviewInstructions = [ manifestReviewInstructions, composeRepoReviewContext( await loadRepoReviewContext(repoFullName), - changedReviewPaths, + changedPaths, ), ] .map((part) => part?.trim()) .filter(Boolean) .join("\n\n") || null; - const reviewInputFingerprint = await aiReviewCacheInputFingerprint({ - changedPaths: changedReviewPaths, - env, + const convergedRepoAllowed = isConvergenceRepoAllowed(env, repoFullName); + // Resolved ONCE and reused both for the fingerprint AND the cache-bypass decision below: grounding/RAG/ + // enrichment/reputation each pull TIME-VARYING external context (live CI checks, the vector index, + // REES/CVE data, the submitter's evolving reputation) that can change for the SAME head SHA without + // any of these booleans flipping. Fingerprinting only "is the feature on" can't detect that drift + // without fetching the content itself (which would defeat caching), so a repo with ANY of these active + // bypasses the cache entirely rather than fingerprinting a value that can't prove freshness. + const dynamicReviewFeatures = { + grounding: isGroundingEnabled(env) && convergedRepoAllowed, + rag: resolveConvergedFeature(env, reviewManifest, "rag", repoFullName), + enrichment: isEnrichmentEnabled(env) && convergedRepoAllowed, + reputation: resolveConvergedFeature( + env, + reviewManifest, + "reputation", + repoFullName, + ), + }; + const dynamicReviewContextActive = + dynamicReviewFeatures.grounding || + dynamicReviewFeatures.rag || + dynamicReviewFeatures.enrichment || + dynamicReviewFeatures.reputation; + const inputFingerprint = await aiReviewCacheInputFingerprint({ + title: pr.title, mode: settings.aiReviewMode, - pr: { - baseSha: webhook.baseSha, - title: pr.title, - }, - review: { - effectiveInlineComments: inlineCommentsEnabledForReview, - excludePaths: reviewExcludePaths, - inlineComments: reviewInlineComments, - instructions: reviewInstructions, - pathInstructions: reviewPathInstructions, - profile: reviewProfile, - }, - settings, + byok: settings.aiReviewByok, + provider: settings.aiReviewProvider, + model: settings.aiReviewModel, + aiReviewAllAuthors: settings.aiReviewAllAuthors, + aiReviewCloseConfidence: settings.aiReviewCloseConfidence, + gatePack: settings.gatePack, + reviewerPlan: env.AI_REVIEW_PLAN, + selfHostProviderConfig: env.AI_REVIEW_PLAN + ? { + claudeModel: env.CLAUDE_AI_MODEL, + claudeEffort: env.CLAUDE_AI_EFFORT, + claudeTimeoutMs: env.CLAUDE_AI_TIMEOUT_MS, + codexModel: env.CODEX_AI_MODEL, + codexEffort: env.CODEX_AI_EFFORT, + codexTimeoutMs: env.CODEX_AI_TIMEOUT_MS, + ollamaBaseUrl: env.OLLAMA_AI_BASE_URL, + ollamaModel: env.OLLAMA_AI_MODEL, + openaiCompatibleBaseUrl: env.OPENAI_COMPATIBLE_AI_BASE_URL, + openaiCompatibleModel: env.OPENAI_COMPATIBLE_AI_MODEL, + openaiBaseUrl: env.OPENAI_AI_BASE_URL, + openaiModel: env.OPENAI_AI_MODEL, + anthropicBaseUrl: env.ANTHROPIC_AI_BASE_URL, + anthropicModel: env.ANTHROPIC_AI_MODEL, + } + : null, + profile: reviewProfile, + inlineComments: inlineCommentsEnabledForReview, + pathInstructions: reviewPathInstructions, + pathGuidance: resolveReviewPathInstructions( + reviewPathInstructions, + changedPaths, + ), + repoInstructions: reviewInstructions, + excludePaths: reviewExcludePaths, + changedPaths, + baseSha: webhook.baseSha, + reviewFiles: reviewFilesForAi.map((file) => ({ + path: file.path, + status: file.status, + patch: typeof file.payload?.patch === "string" ? file.payload.patch : undefined, + additions: file.additions, + deletions: file.deletions, + })), + features: dynamicReviewFeatures, }); - // #1 self-host AI-review cache: reuse a prior review for this exact (repo, pr, head SHA, mode) ONLY when - // the prompt/config inputs that affect the model output still match. Private repo instructions, RAG - // suppressions, feature flags, inline-comment mode, and BYOK model choices all change review output even - // when the head SHA is unchanged, so they are folded into the stored input fingerprint. - const cachedReview = await getCachedAiReview( - env, - repoFullName, - pr.number, - advisory.headSha, - settings.aiReviewMode, - reviewInputFingerprint, - ).catch(() => null); + // #1 self-host AI-review cache: the LLM output for a PR changes only when the code (head SHA), review + // 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 + ? null + : await getCachedAiReview( + env, + repoFullName, + pr.number, + advisory.headSha, + settings.aiReviewMode, + inputFingerprint, + ).catch(() => null); if (cachedReview && hasPublicReviewAssessment(cachedReview.notes)) { advisory.findings.push(...cachedReview.findings); aiReview = cachedReview; @@ -5595,7 +5651,7 @@ async function maybePublishPrPublicSurface( reviewExcludePaths, reviewInlineComments, }); - if (aiReview && aiReview.cacheable !== false) + if (aiReview && aiReview.cacheable !== false && !dynamicReviewContextActive) await putCachedAiReview( env, repoFullName, @@ -5607,7 +5663,7 @@ async function maybePublishPrPublicSurface( 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: reviewInputFingerprint, + inputFingerprint, }, }, ).catch(() => undefined); diff --git a/src/review/ai-review-cache-input.ts b/src/review/ai-review-cache-input.ts index 177b074084..1d2176d831 100644 --- a/src/review/ai-review-cache-input.ts +++ b/src/review/ai-review-cache-input.ts @@ -1,103 +1,162 @@ -import { sha256Hex } from "../utils/crypto"; import type { ReviewPathInstruction, ReviewProfile, } from "../signals/focus-manifest"; -import type { RepositorySettings } from "../types"; +import { sha256Hex } from "../utils/crypto"; -type StableJsonValue = - | null - | boolean - | number - | string - | StableJsonValue[] - | { [key: string]: StableJsonValue }; +export const AI_REVIEW_CACHE_INPUT_VERSION = "ai-review-input:v1"; -function stableJsonValue(value: unknown): StableJsonValue { - if (value === null || typeof value === "boolean" || typeof value === "number" || typeof value === "string") - return value; - if (Array.isArray(value)) return value.map(stableJsonValue); - if (typeof value === "object" && value !== null) { - return Object.fromEntries( - Object.entries(value as Record) - .filter(([, entryValue]) => entryValue !== undefined) - .sort(([left], [right]) => left.localeCompare(right)) - .map(([key, entryValue]) => [key, stableJsonValue(entryValue)]), - ); - } - return null; +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 + // different prompt metadata. + title: string; + mode: string; + byok: boolean; + provider: string | null | undefined; + model: string | null | undefined; + // Eligibility/interpretation settings that don't shape the prompt itself but decide whether AI runs at all + // (aiReviewAllAuthors, gatePack) or how a cached finding's embedded confidence is later interpreted + // (aiReviewCloseConfidence). None of these change what the model would output for the same prompt, but a + // repo flipping any of them warrants a fresh review rather than replaying a decision made under different + // eligibility/interpretation rules. + aiReviewAllAuthors: boolean; + aiReviewCloseConfidence: number | null | undefined; + gatePack: string | null | undefined; + reviewerPlan: + | { + combine?: string | null | undefined; + reviewers?: readonly { model?: string | null | undefined }[] | undefined; + } + | null + | undefined; + // reviewerPlan only names WHICH self-host provider(s) are active (e.g. "claude-code") -- it does not carry that + // provider's own model/effort/timeout/base-url, which are resolved separately at review-call time (see + // src/selfhost/ai.ts's buildProvider). Fingerprint those too so switching a provider's underlying model or + // endpoint (while the provider name/plan stays the same) forces a cache miss instead of reusing a review + // produced against a different configuration. Deliberately excludes API keys (secrets, and irrelevant to output). + selfHostProviderConfig: + | { + claudeModel?: string | null | undefined; + claudeEffort?: string | null | undefined; + claudeTimeoutMs?: string | null | undefined; + codexModel?: string | null | undefined; + codexEffort?: string | null | undefined; + codexTimeoutMs?: string | null | undefined; + ollamaBaseUrl?: string | null | undefined; + ollamaModel?: string | null | undefined; + openaiCompatibleBaseUrl?: string | null | undefined; + openaiCompatibleModel?: string | null | undefined; + openaiBaseUrl?: string | null | undefined; + openaiModel?: string | null | undefined; + anthropicBaseUrl?: string | null | undefined; + anthropicModel?: string | null | undefined; + } + | null + | undefined; + profile: ReviewProfile | null | undefined; + inlineComments: boolean; + pathInstructions: readonly ReviewPathInstruction[]; + pathGuidance: string; + 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; + patch?: string | null | undefined; + additions: number; + deletions: number; + }[]; + // grounding/rag/enrichment/reputation each pull TIME-VARYING external context that can change for an + // unchanged head SHA without any of these booleans flipping (live CI checks, the vector index, REES/CVE data, + // the submitter's evolving reputation) -- a boolean can't detect that drift, so the caller bypasses the cache + // entirely whenever any of these is true rather than relying on this fingerprint to catch a content change. + features: { + grounding: boolean; + rag: boolean; + enrichment: boolean; + reputation: boolean; + }; +}; + +export async function aiReviewCacheInputFingerprint(input: AiReviewCacheInput): Promise { + const payload = { + version: AI_REVIEW_CACHE_INPUT_VERSION, + title: input.title, + mode: input.mode, + byok: input.byok, + provider: input.provider ?? null, + model: input.model ?? null, + aiReviewAllAuthors: input.aiReviewAllAuthors, + aiReviewCloseConfidence: input.aiReviewCloseConfidence ?? null, + gatePack: input.gatePack ?? null, + reviewerPlan: input.reviewerPlan + ? { + combine: input.reviewerPlan.combine ?? null, + reviewers: (input.reviewerPlan.reviewers ?? []).map((reviewer) => reviewer.model ?? null), + } + : null, + selfHostProviderConfig: input.selfHostProviderConfig + ? { + claudeModel: input.selfHostProviderConfig.claudeModel ?? null, + claudeEffort: input.selfHostProviderConfig.claudeEffort ?? null, + claudeTimeoutMs: input.selfHostProviderConfig.claudeTimeoutMs ?? null, + codexModel: input.selfHostProviderConfig.codexModel ?? null, + codexEffort: input.selfHostProviderConfig.codexEffort ?? null, + codexTimeoutMs: input.selfHostProviderConfig.codexTimeoutMs ?? null, + ollamaBaseUrl: input.selfHostProviderConfig.ollamaBaseUrl ?? null, + ollamaModel: input.selfHostProviderConfig.ollamaModel ?? null, + openaiCompatibleBaseUrl: input.selfHostProviderConfig.openaiCompatibleBaseUrl ?? null, + openaiCompatibleModel: input.selfHostProviderConfig.openaiCompatibleModel ?? null, + openaiBaseUrl: input.selfHostProviderConfig.openaiBaseUrl ?? null, + openaiModel: input.selfHostProviderConfig.openaiModel ?? null, + anthropicBaseUrl: input.selfHostProviderConfig.anthropicBaseUrl ?? null, + anthropicModel: input.selfHostProviderConfig.anthropicModel ?? null, + } + : null, + profile: input.profile ?? null, + inlineComments: input.inlineComments, + pathInstructions: input.pathInstructions.map((instruction) => ({ + path: instruction.path, + instructions: instruction.instructions, + })), + pathGuidance: input.pathGuidance, + 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, + status: file.status ?? null, + patch: file.patch ?? null, + additions: file.additions, + deletions: file.deletions, + })) + .sort((left, right) => left.path.localeCompare(right.path)), + features: input.features, + }; + return `${AI_REVIEW_CACHE_INPUT_VERSION}:${await sha256Hex(stableStringify(payload))}`; } -export async function aiReviewInputFingerprint(input: unknown): Promise { - return `ai-review-input:v1:${await sha256Hex(JSON.stringify(stableJsonValue(input)))}`; +function normalizeStringList(values: readonly string[]): string[] { + return [...new Set(values.map((value) => value.trim()).filter(Boolean))].sort(); } -export async function aiReviewCacheInputFingerprint(args: { - changedPaths: string[]; - env: Partial< - Pick< - Env, - | "GITTENSORY_REVIEW_ENRICHMENT" - | "GITTENSORY_REVIEW_GROUNDING" - | "GITTENSORY_REVIEW_INLINE_COMMENTS" - | "GITTENSORY_REVIEW_RAG" - | "GITTENSORY_REVIEW_REPUTATION" - | "GITTENSORY_REVIEW_REPOS" - | "REES_ANALYZERS" - | "REES_FORWARD_GITHUB_TOKEN" - | "REES_PROFILE" - | "REES_TIMEOUT_MS" - | "REES_URL" - > - >; - mode: string; - pr: { baseSha?: string | null | undefined; title: string }; - review: { - effectiveInlineComments: boolean; - excludePaths: string[]; - inlineComments: boolean; - instructions: string | null; - pathInstructions: ReviewPathInstruction[]; - profile: ReviewProfile | null; - }; - settings: Pick< - RepositorySettings, - | "aiReviewAllAuthors" - | "aiReviewByok" - | "aiReviewCloseConfidence" - | "aiReviewModel" - | "aiReviewProvider" - | "gatePack" - >; -}): Promise { - return aiReviewInputFingerprint({ - changedPaths: args.changedPaths, - env: { - enrichment: args.env.GITTENSORY_REVIEW_ENRICHMENT ?? null, - grounding: args.env.GITTENSORY_REVIEW_GROUNDING ?? null, - inlineComments: args.env.GITTENSORY_REVIEW_INLINE_COMMENTS ?? null, - rag: args.env.GITTENSORY_REVIEW_RAG ?? null, - reesAnalyzers: args.env.REES_ANALYZERS ?? null, - reesGithubTokenForwarding: args.env.REES_FORWARD_GITHUB_TOKEN ?? null, - reesProfile: args.env.REES_PROFILE ?? null, - reesTimeoutMs: args.env.REES_TIMEOUT_MS ?? null, - reesUrl: args.env.REES_URL ?? null, - reputation: args.env.GITTENSORY_REVIEW_REPUTATION ?? null, - reviewRepos: args.env.GITTENSORY_REVIEW_REPOS ?? null, - }, - mode: args.mode, - pr: { - baseSha: args.pr.baseSha ?? null, - title: args.pr.title, - }, - review: args.review, - settings: { - aiReviewAllAuthors: args.settings.aiReviewAllAuthors, - aiReviewByok: args.settings.aiReviewByok, - aiReviewCloseConfidence: args.settings.aiReviewCloseConfidence ?? null, - aiReviewModel: args.settings.aiReviewModel ?? null, - aiReviewProvider: args.settings.aiReviewProvider ?? null, - gatePack: args.settings.gatePack, - }, - }); +function stableStringify(value: unknown): string { + if (Array.isArray(value)) return `[${value.map(stableStringify).join(",")}]`; + if (value && typeof value === "object") { + return `{${Object.entries(value) + .sort(([left], [right]) => left.localeCompare(right)) + .map(([key, nested]) => `${JSON.stringify(key)}:${stableStringify(nested)}`) + .join(",")}}`; + } + return JSON.stringify(value); } diff --git a/src/review/submitter-reputation.ts b/src/review/submitter-reputation.ts index edf373a800..82d4866c8c 100644 --- a/src/review/submitter-reputation.ts +++ b/src/review/submitter-reputation.ts @@ -208,7 +208,7 @@ export async function recordSubmissionOutcome(env: Env, project: string, submitt await storage(env) .prepare( `INSERT INTO submitter_stats (project, submitter, submissions, ${col}, last_seen) VALUES (?, ?, 1, 1, CURRENT_TIMESTAMP) - ON CONFLICT(project, submitter) DO UPDATE SET submissions = submissions + 1, ${col} = ${col} + 1, last_seen = CURRENT_TIMESTAMP`, + ON CONFLICT(project, submitter) DO UPDATE SET submissions = submitter_stats.submissions + 1, ${col} = submitter_stats.${col} + 1, last_seen = CURRENT_TIMESTAMP`, ) .bind(project, submitter) .run(); diff --git a/test/unit/ai-review-cache-input.test.ts b/test/unit/ai-review-cache-input.test.ts new file mode 100644 index 0000000000..76d6de2f2c --- /dev/null +++ b/test/unit/ai-review-cache-input.test.ts @@ -0,0 +1,244 @@ +import { + AI_REVIEW_CACHE_INPUT_VERSION, + aiReviewCacheInputFingerprint, + type AiReviewCacheInput, +} from "../../src/review/ai-review-cache-input"; + +const baseInput = (): AiReviewCacheInput => ({ + title: "Fix the retry loop", + mode: "block", + byok: false, + provider: null, + model: null, + aiReviewAllAuthors: false, + aiReviewCloseConfidence: null, + gatePack: null, + reviewerPlan: null, + selfHostProviderConfig: null, + baseSha: null, + reviewFiles: [], + profile: null, + inlineComments: false, + pathInstructions: [], + pathGuidance: "", + repoInstructions: null, + excludePaths: [], + changedPaths: ["src/a.ts"], + features: { + grounding: false, + rag: false, + enrichment: false, + reputation: false, + }, +}); + +describe("aiReviewCacheInputFingerprint", () => { + it("is stable across irrelevant path ordering and whitespace normalization", async () => { + const left = await aiReviewCacheInputFingerprint({ + ...baseInput(), + changedPaths: [" src/b.ts ", "src/a.ts", "src/a.ts"], + excludePaths: ["dist/**", " **/*.lock "], + repoInstructions: " Follow the repo guide. ", + }); + const right = await aiReviewCacheInputFingerprint({ + ...baseInput(), + changedPaths: ["src/a.ts", "src/b.ts"], + excludePaths: ["**/*.lock", "dist/**"], + repoInstructions: "Follow the repo guide.", + }); + + expect(left).toBe(right); + expect(left.startsWith(`${AI_REVIEW_CACHE_INPUT_VERSION}:`)).toBe(true); + }); + + it("changes when prompt-affecting review inputs change", async () => { + const original = await aiReviewCacheInputFingerprint({ + ...baseInput(), + reviewerPlan: { combine: "consensus", reviewers: [{ model: "a" }, { model: "b" }] }, + pathInstructions: [{ path: "src/**", instructions: "Be strict." }], + pathGuidance: "Be strict.", + features: { ...baseInput().features, rag: true }, + }); + const updated = await aiReviewCacheInputFingerprint({ + ...baseInput(), + reviewerPlan: { combine: "consensus", reviewers: [{ model: "a" }, { model: "c" }] }, + pathInstructions: [{ path: "src/**", instructions: "Be strict." }], + pathGuidance: "Be strict.", + features: { ...baseInput().features, rag: true }, + }); + + expect(updated).not.toBe(original); + }); + + it("normalizes sparse reviewer plan fields deterministically", async () => { + const omittedReviewers = await aiReviewCacheInputFingerprint({ + ...baseInput(), + reviewerPlan: {}, + }); + const explicitEmpty = await aiReviewCacheInputFingerprint({ + ...baseInput(), + reviewerPlan: { combine: null, reviewers: [] }, + }); + const sparse = await aiReviewCacheInputFingerprint({ + ...baseInput(), + reviewerPlan: { reviewers: [{}] }, + }); + const explicit = await aiReviewCacheInputFingerprint({ + ...baseInput(), + reviewerPlan: { combine: null, reviewers: [{ model: null }] }, + }); + + expect(omittedReviewers).toBe(explicitEmpty); + 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. + 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 }, + ], + }); + 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 }, + ], + }); + + expect(samePathsDifferentPatch).not.toBe(original); + expect(samePatchDifferentBase).not.toBe(original); + expect(repeated).toBe(original); + expect(reordered).toBe(reorderedAgain); + }); + + it("normalizes a file entry with no status/patch (e.g. a rename with no content change) deterministically", async () => { + const omitted = await aiReviewCacheInputFingerprint({ + ...baseInput(), + reviewFiles: [{ path: "src/a.ts", additions: 0, deletions: 0 }], + }); + const explicitNull = await aiReviewCacheInputFingerprint({ + ...baseInput(), + reviewFiles: [{ path: "src/a.ts", status: null, patch: null, additions: 0, deletions: 0 }], + }); + const withStatusAndPatch = await aiReviewCacheInputFingerprint({ + ...baseInput(), + reviewFiles: [{ path: "src/a.ts", status: "renamed", patch: "@@ -1 +1 @@", additions: 0, deletions: 0 }], + }); + + expect(omitted).toBe(explicitNull); + expect(omitted).not.toBe(withStatusAndPatch); + }); + + it("changes when a self-host provider's underlying model/effort/timeout changes, even with the same reviewer plan", async () => { + const reviewerPlan = { combine: "single", reviewers: [{ model: "claude-code" }] }; + const fullyConfigured = { + claudeModel: "sonnet", + claudeEffort: "high", + claudeTimeoutMs: "60000", + codexModel: "gpt-5", + codexEffort: "high", + codexTimeoutMs: "240000", + ollamaBaseUrl: "http://localhost:11434/v1", + ollamaModel: "llama-3.1", + openaiCompatibleBaseUrl: "http://localhost:11434/v1", + openaiCompatibleModel: "llama-3.1", + openaiBaseUrl: "https://api.openai.com/v1", + openaiModel: "gpt-5", + anthropicBaseUrl: "https://api.anthropic.com", + anthropicModel: "claude-sonnet-5", + }; + + const original = await aiReviewCacheInputFingerprint({ + ...baseInput(), + reviewerPlan, + selfHostProviderConfig: fullyConfigured, + }); + const repeated = await aiReviewCacheInputFingerprint({ + ...baseInput(), + reviewerPlan, + selfHostProviderConfig: { ...fullyConfigured }, + }); + // The reviewer PLAN (provider names) is unchanged -- only the underlying model changed. The prior + // fingerprint (reviewer.model only) would have collided here; this must now miss. + const modelChanged = await aiReviewCacheInputFingerprint({ + ...baseInput(), + reviewerPlan, + selfHostProviderConfig: { ...fullyConfigured, claudeModel: "opus" }, + }); + const effortChanged = await aiReviewCacheInputFingerprint({ + ...baseInput(), + reviewerPlan, + selfHostProviderConfig: { ...fullyConfigured, claudeEffort: "low" }, + }); + + expect(repeated).toBe(original); + expect(modelChanged).not.toBe(original); + expect(effortChanged).not.toBe(original); + }); + + it("normalizes an absent self-host provider config the same whether omitted or explicitly empty", async () => { + const nullConfig = await aiReviewCacheInputFingerprint({ ...baseInput(), selfHostProviderConfig: null }); + const emptyConfig = await aiReviewCacheInputFingerprint({ ...baseInput(), selfHostProviderConfig: {} }); + const sparseConfig = await aiReviewCacheInputFingerprint({ + ...baseInput(), + selfHostProviderConfig: { claudeModel: undefined }, + }); + + expect(emptyConfig).toBe(sparseConfig); + expect(emptyConfig).not.toBe(nullConfig); + }); + + it("changes when the PR title changes even though nothing else does (#2119)", async () => { + // The title is threaded into the reviewer prompt (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 + // against different prompt metadata. + const original = await aiReviewCacheInputFingerprint(baseInput()); + const titleChanged = await aiReviewCacheInputFingerprint({ ...baseInput(), title: "Fix the retry loop (v2)" }); + const repeated = await aiReviewCacheInputFingerprint(baseInput()); + + expect(titleChanged).not.toBe(original); + expect(repeated).toBe(original); + }); + + it("changes when aiReviewAllAuthors, aiReviewCloseConfidence, or gatePack change", async () => { + const original = await aiReviewCacheInputFingerprint(baseInput()); + const allAuthorsChanged = await aiReviewCacheInputFingerprint({ ...baseInput(), aiReviewAllAuthors: true }); + const closeConfidenceChanged = await aiReviewCacheInputFingerprint({ ...baseInput(), aiReviewCloseConfidence: 0.9 }); + const gatePackChanged = await aiReviewCacheInputFingerprint({ ...baseInput(), gatePack: "oss-anti-slop" }); + const repeated = await aiReviewCacheInputFingerprint(baseInput()); + + expect(allAuthorsChanged).not.toBe(original); + expect(closeConfidenceChanged).not.toBe(original); + expect(gatePackChanged).not.toBe(original); + expect(repeated).toBe(original); + }); +}); diff --git a/test/unit/ai-review-cache.test.ts b/test/unit/ai-review-cache.test.ts index 4595fdb54f..f507ee6cc5 100644 --- a/test/unit/ai-review-cache.test.ts +++ b/test/unit/ai-review-cache.test.ts @@ -1,11 +1,36 @@ import { describe, expect, it, vi } from "vitest"; import { getCachedAiReview, putCachedAiReview } from "../../src/db/repositories"; -import { - aiReviewCacheInputFingerprint, - aiReviewInputFingerprint, -} from "../../src/review/ai-review-cache-input"; +import { aiReviewCacheInputFingerprint, type AiReviewCacheInput } from "../../src/review/ai-review-cache-input"; import { createTestEnv } from "../helpers/d1"; +const baseFingerprintInput = (): AiReviewCacheInput => ({ + title: "Fix the retry loop", + mode: "block", + byok: false, + provider: null, + model: null, + aiReviewAllAuthors: false, + aiReviewCloseConfidence: null, + gatePack: null, + reviewerPlan: null, + selfHostProviderConfig: null, + baseSha: null, + reviewFiles: [], + profile: null, + inlineComments: false, + pathInstructions: [], + pathGuidance: "", + repoInstructions: null, + excludePaths: [], + changedPaths: ["src/changed.ts"], + features: { + grounding: false, + rag: false, + enrichment: false, + reputation: false, + }, +}); + describe("AI review cache (#1)", () => { it("misses on a nullish head SHA (read returns null; write is a no-op)", async () => { const env = createTestEnv(); @@ -108,21 +133,19 @@ describe("AI review cache (#1)", () => { it("reuses fingerprinted cache rows only when the review input fingerprint matches", async () => { const env = createTestEnv(); - const matching = await aiReviewInputFingerprint({ - instructions: "Use the current repository review guide.", - nested: { b: true, a: ["src/changed.ts"] }, - ignored: undefined, + const matching = await aiReviewCacheInputFingerprint({ + ...baseFingerprintInput(), + repoInstructions: "Use the current repository review guide.", }); - const sameDifferentKeyOrder = await aiReviewInputFingerprint({ - ignored: undefined, - nested: { a: ["src/changed.ts"], b: true }, - instructions: "Use the current repository review guide.", + const repeated = await aiReviewCacheInputFingerprint({ + ...baseFingerprintInput(), + repoInstructions: "Use the current repository review guide.", }); - const changed = await aiReviewInputFingerprint({ - instructions: "Use an older repository review guide.", - nested: { a: ["src/changed.ts"], b: true }, + const changed = await aiReviewCacheInputFingerprint({ + ...baseFingerprintInput(), + repoInstructions: "Use an older repository review guide.", }); - expect(sameDifferentKeyOrder).toBe(matching); + expect(repeated).toBe(matching); expect(changed).not.toBe(matching); await putCachedAiReview(env, "o/r", 11, "sha1", "block", { @@ -139,121 +162,4 @@ describe("AI review cache (#1)", () => { metadata: { inputFingerprint: matching }, }); }); - - it("fingerprints scalar review-input values deterministically", async () => { - const values = await Promise.all([ - aiReviewInputFingerprint(null), - aiReviewInputFingerprint(true), - aiReviewInputFingerprint(7), - aiReviewInputFingerprint("rules"), - aiReviewInputFingerprint(undefined), - ]); - expect(values[4]).toBe(values[0]); - expect(new Set(values).size).toBe(4); - await expect(aiReviewInputFingerprint("rules")).resolves.toBe(values[3]); - }); - - it("normalizes review cache fingerprint inputs from prompt, settings, and runtime config", async () => { - const base = { - changedPaths: ["src/changed.ts"], - env: {}, - mode: "block", - pr: { title: "Tighten review cache invalidation" }, - review: { - effectiveInlineComments: false, - excludePaths: [], - inlineComments: false, - instructions: "Use the current repository review guide.", - pathInstructions: [], - profile: null, - }, - settings: { - aiReviewAllAuthors: true, - aiReviewByok: false, - aiReviewCloseConfidence: undefined, - aiReviewModel: undefined, - aiReviewProvider: undefined, - gatePack: "oss-anti-slop" as const, - }, - }; - - const baseline = await aiReviewCacheInputFingerprint(base); - await expect( - aiReviewCacheInputFingerprint({ - ...base, - pr: { ...base.pr, baseSha: null }, - settings: { - ...base.settings, - aiReviewCloseConfidence: null, - aiReviewModel: null, - aiReviewProvider: null, - }, - }), - ).resolves.toBe(baseline); - await expect( - aiReviewCacheInputFingerprint({ - ...base, - review: { - ...base.review, - instructions: "Use an older repository review guide.", - }, - }), - ).resolves.not.toBe(baseline); - await expect( - aiReviewCacheInputFingerprint({ - ...base, - env: { - GITTENSORY_REVIEW_RAG: "true", - REES_URL: "https://rees.example", - REES_ANALYZERS: "secret,redos", - REES_PROFILE: "deep", - REES_TIMEOUT_MS: "12000", - REES_FORWARD_GITHUB_TOKEN: "false", - }, - }), - ).resolves.not.toBe(baseline); - }); - - it("changes the fingerprint when the configured REES endpoint URL itself changes", async () => { - const base = { - changedPaths: ["src/changed.ts"], - env: {}, - mode: "block", - pr: { title: "Tighten review cache invalidation" }, - review: { - effectiveInlineComments: false, - excludePaths: [], - inlineComments: false, - instructions: "Use the current repository review guide.", - pathInstructions: [], - profile: null, - }, - settings: { - aiReviewAllAuthors: true, - aiReviewByok: false, - aiReviewCloseConfidence: undefined, - aiReviewModel: undefined, - aiReviewProvider: undefined, - gatePack: "oss-anti-slop" as const, - }, - }; - - // Two DIFFERENT, both-truthy REES_URL values must not collide: reusing an AI review that ran - // against a different analyzer endpoint could reuse stale output produced by a different service. - const withEndpointA = await aiReviewCacheInputFingerprint({ - ...base, - env: { REES_URL: "https://rees-a.example" }, - }); - const withEndpointB = await aiReviewCacheInputFingerprint({ - ...base, - env: { REES_URL: "https://rees-b.example" }, - }); - const withEndpointARepeated = await aiReviewCacheInputFingerprint({ - ...base, - env: { REES_URL: "https://rees-a.example" }, - }); - - expect(withEndpointA).not.toBe(withEndpointB); - expect(withEndpointA).toBe(withEndpointARepeated); - }); }); diff --git a/test/unit/index.test.ts b/test/unit/index.test.ts index 3bc8c91859..5ec79d6ea9 100644 --- a/test/unit/index.test.ts +++ b/test/unit/index.test.ts @@ -277,7 +277,40 @@ describe("worker entrypoint", () => { expect(sent).toEqual([{ type: "agent-regate-sweep", requestedBy: "schedule" }]); }); - it("does not enqueue scheduled sweep/backfill work while prior regate jobs are still queued", async () => { + it("keeps enqueueing scheduled sweeps while prior per-PR regate jobs are queued (#2119)", async () => { + // Per-PR "agent-regate-pr" backlog is normal, expected, ongoing work (staggered/rate-deferred re-reviews) — + // it must NOT block the next scheduled fan-out trigger, or the sweep starves under any sustained load. + const sent: Array = []; + let snapshotCalled = false; + const env = createTestEnv({ + JOBS: { + async send(message: import("../../src/types").JobMessage) { + sent.push(message); + }, + snapshot: async () => { + snapshotCalled = true; + return { + totals: { pending: 2, processing: 0, dead: 0, due: 2 }, + byType: [{ type: "agent-regate-pr", status: "pending", count: 2, due: 2 }], + }; + }, + } as unknown as Queue, + }); + const waitUntil: Promise[] = []; + + await worker.scheduled(controllerFor("2026-05-25T05:30:00.000Z"), env, executionContext(waitUntil)); + await Promise.all(waitUntil); + + expect(sent).toEqual([ + { type: "agent-regate-sweep", requestedBy: "schedule" }, + { type: "backfill-registered-repos", requestedBy: "schedule", mode: "light" }, + { type: "repair-data-fidelity", requestedBy: "schedule" }, + { type: "refresh-installation-health", requestedBy: "schedule" }, + ]); + expect(snapshotCalled).toBe(true); + }); + + it("defers a new sweep trigger while a prior one is still pending or processing (#2119, #audit-sweep-fanout)", async () => { const sent: Array = []; const env = createTestEnv({ JOBS: { @@ -285,11 +318,8 @@ describe("worker entrypoint", () => { sent.push(message); }, snapshot: async () => ({ - totals: { pending: 2, processing: 1, dead: 0, due: 2 }, - byType: [ - { type: "agent-regate-pr", status: "pending", count: 2, due: 2 }, - { type: "agent-regate-sweep", status: "processing", count: 1, due: 0 }, - ], + totals: { pending: 0, processing: 1, dead: 0, due: 0 }, + byType: [{ type: "agent-regate-sweep", status: "processing", count: 1, due: 0 }], }), } as unknown as Queue, }); @@ -298,13 +328,16 @@ describe("worker entrypoint", () => { await worker.scheduled(controllerFor("2026-05-25T05:30:00.000Z"), env, executionContext(waitUntil)); await Promise.all(waitUntil); + // No SECOND "agent-regate-sweep" trigger is enqueued behind the one already in flight; the other :30 jobs + // are unaffected since they never depended on the (removed, broad) backlog check. expect(sent).toEqual([ + { type: "backfill-registered-repos", requestedBy: "schedule", mode: "light" }, { type: "repair-data-fidelity", requestedBy: "schedule" }, { type: "refresh-installation-health", requestedBy: "schedule" }, ]); }); - it("fails open when queue introspection is unavailable so scheduled maintenance still runs", async () => { + it("does not require queue introspection for regular review sweep scheduling", async () => { const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined); const sent: Array = []; const env = createTestEnv({ @@ -322,6 +355,8 @@ describe("worker entrypoint", () => { await worker.scheduled(controllerFor("2026-05-25T05:14:00.000Z"), env, executionContext(waitUntil)); await Promise.all(waitUntil); + // Fails OPEN on a broken snapshot binding: the sweep still enqueues, and the failure is surfaced (not + // silently swallowed) so an operator can see the introspection is unavailable. expect(sent).toEqual([{ type: "agent-regate-sweep", requestedBy: "schedule" }]); expect(warn).toHaveBeenCalledWith(expect.stringContaining("selfhost_queue_snapshot_failed")); }); diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index f9f2ec0632..e4afc1194a 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -45,6 +45,7 @@ import { putCachedAiReview, } from "../../src/db/repositories"; import { agentMaintenanceHeadMatchesGate, changedPathsForGuardrail, contributorEvidenceBatchSize, processJob } from "../../src/queue/processors"; +import { aiReviewCacheInputFingerprint } from "../../src/review/ai-review-cache-input"; import { upsertRepoFocusManifest } from "../../src/signals/focus-manifest-loader"; import { normalizeRegistryPayload } from "../../src/registry/normalize"; import { persistRegistrySnapshot } from "../../src/registry/sync"; @@ -53,7 +54,6 @@ import { fetchPullRequestFreshness, } from "../../src/github/pr-freshness"; import { createTestEnv } from "../helpers/d1"; -import { aiReviewCacheInputFingerprint } from "../../src/review/ai-review-cache-input"; vi.mock("../../src/github/pr-freshness", async (importOriginal) => { const actual = await importOriginal(); @@ -812,7 +812,7 @@ describe("queue processors", () => { expect(JSON.parse(audit?.metadata_json ?? "{}")).toMatchObject({ repoFullName: "owner/agent-repo", examined: 1 }); }); - it("agent re-gate sweep runs blocking AI review before auto-maintenance (regression)", async () => { + it("agent re-gate sweep ignores stale same-head AI cache inputs before auto-maintenance (regression)", async () => { let aiCalls = 0; const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), @@ -840,6 +840,11 @@ describe("queue processors", () => { }); await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 7, title: "Stale PR", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, labels: [], body: "Closes #1" }); await upsertPullRequestFile(env, { repoFullName: "owner/agent-repo", pullNumber: 7, path: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, payload: { patch: "@@\n+export const ok = value.length;" } }); + await putCachedAiReview(env, "owner/agent-repo", 7, "a7", "block", { + notes: "stale cached review from older review inputs", + reviewerCount: 2, + findings: [{ code: "ai_review_split", severity: "critical", title: "Old cache", detail: "Old prompt inputs." }], + }); vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { const url = input.toString(); if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); @@ -1543,25 +1548,30 @@ describe("queue processors", () => { await upsertPullRequestFile(env, { repoFullName: "owner/agent-repo", pullNumber: 7, path: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, payload: { patch: "@@\n+export const ok = value.length;" } }); // Pre-seed the AI review for this exact head SHA + mode → the sweep's block-mode review must reuse it, not re-run. const inputFingerprint = await aiReviewCacheInputFingerprint({ - changedPaths: ["src/a.ts"], - env, + title: "Stale PR", mode: "block", - pr: { title: "Stale PR", baseSha: null }, - review: { - effectiveInlineComments: false, - excludePaths: [], - inlineComments: false, - instructions: null, - pathInstructions: [], - profile: null, - }, - settings: { - aiReviewAllAuthors: false, - aiReviewByok: false, - aiReviewCloseConfidence: undefined, - aiReviewModel: undefined, - aiReviewProvider: undefined, - gatePack: "oss-anti-slop", + 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 = value.length;", 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 putCachedAiReview(env, "owner/agent-repo", 7, "a7", "block", { @@ -1678,6 +1688,161 @@ describe("queue processors", () => { expect(stickyComment.current?.body).not.toContain("is reviewing"); }); + it("computes the AI review cache fingerprint with a self-host reviewer plan and converged grounding/enrichment on (#2119)", 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", + // A self-host reviewer plan (not just BYOK/cloud provider/model) plus its underlying provider config. + AI_REVIEW_PLAN: { reviewers: [{ model: "claude-code" }], combine: "single" } as never, + CLAUDE_AI_MODEL: "sonnet", + CLAUDE_AI_EFFORT: "high", + // Grounding + enrichment ON, with the repo allowlisted for convergence, so both feature flags + // resolve past their `isXEnabled(env) && convergedRepoAllowed` check into the fingerprint. + GITTENSORY_REVIEW_GROUNDING: "true", + GITTENSORY_REVIEW_ENRICHMENT: "true", + REES_URL: "https://rees.example", + GITTENSORY_REVIEW_REPOS: "JSONbored/gittensory", + }); + 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 upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 123); + await upsertRepositorySettings(env, { + repoFullName: "JSONbored/gittensory", + commentMode: "all_prs", + publicSurface: "comment_only", + autoLabelEnabled: false, + checkRunMode: "off", + gateCheckMode: "enabled", + aiReviewMode: "block", + gatePack: "oss-anti-slop", + }); + 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/7/files")) + return Response.json([ + { filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+export const ok = true;" }, + // GitHub omits `patch` for binary/oversized files -- the fingerprint must still normalize this case. + { filename: "assets/logo.png", status: "modified", additions: 0, deletions: 0, changes: 0 }, + ]); + if (url.endsWith("/pulls/7")) return Response.json({ number: 7, title: "Clean PR", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, labels: [], body: "Closes #1" }); + if (url.includes("/commits/a7/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/a7/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/1")) return Response.json({ number: 1, title: "Issue", state: "open", labels: [], user: { login: "reporter" } }); + if (url.includes("/issues/7/comments") && method === "GET") return Response.json([]); + if (url.includes("/issues/7/comments") && method === "POST") return Response.json({ id: 1 }, { status: 201 }); + if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } }); + // REES enrichment + any other unmatched call degrade fail-open on a generic empty response. + return Response.json({}); + }); + + await processJob(env, { + type: "github-webhook", + deliveryId: "self-host-plan-converged-features", + eventName: "pull_request", + payload: { + action: "opened", + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + pull_request: { number: 7, title: "Clean PR", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, labels: [], body: "Closes #1" }, + }, + }); + + // The review ran fresh (no pre-seeded cache to reuse), reaching the fingerprint computation with the + // self-host reviewer plan, its provider config, and both converged feature checks evaluated. + expect(aiCalls).toBeGreaterThan(0); + }); + + it("bypasses the AI review cache entirely while a dynamic-context feature (grounding) is active (#2119)", 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. + 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", + GITTENSORY_REVIEW_GROUNDING: "true", + GITTENSORY_REVIEW_REPOS: "JSONbored/gittensory", + }); + 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 upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 123); + await upsertRepositorySettings(env, { + repoFullName: "JSONbored/gittensory", + commentMode: "all_prs", + publicSurface: "comment_only", + autoLabelEnabled: false, + checkRunMode: "off", + gateCheckMode: "enabled", + aiReviewMode: "block", + gatePack: "oss-anti-slop", + }); + 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/7/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+export const ok = true;" }]); + if (url.endsWith("/pulls/7")) return Response.json({ number: 7, title: "Clean PR", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, labels: [], body: "Closes #1" }); + if (url.includes("/commits/a7/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/a7/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/1")) return Response.json({ number: 1, title: "Issue", state: "open", labels: [], user: { login: "reporter" } }); + if (url.includes("/issues/7/comments") && method === "GET") return Response.json([]); + if (url.includes("/issues/7/comments") && method === "POST") return Response.json({ id: 1 }, { status: 201 }); + if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } }); + return Response.json({}); + }); + + const webhook = { + type: "github-webhook" as const, + eventName: "pull_request" as const, + payload: { + action: "opened", + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" as const } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + pull_request: { number: 7, title: "Clean PR", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, labels: [], body: "Closes #1" }, + }, + }; + 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. + await processJob(env, { ...webhook, deliveryId: "dynamic-context-bypass-2" }); + expect(aiCalls).toBe(firstRunAiCalls * 2); + }); + it("continues to final verdict when the reviewing placeholder audit write fails", async () => { const originalRecordAuditEvent = repositoriesModule.recordAuditEvent; const auditSpy = vi.spyOn(repositoriesModule, "recordAuditEvent").mockImplementation(async (auditEnv, event) => { @@ -2885,7 +3050,7 @@ describe("queue processors", () => { ]); }); - it("INVARIANT: a scheduled repo sweep fails open when queue introspection throws", async () => { + it("INVARIANT: a scheduled repo sweep does not require queue introspection", async () => { const sent: import("../../src/types").JobMessage[] = []; const env = createTestEnv({ JOBS: { diff --git a/test/unit/reputation-wiring.test.ts b/test/unit/reputation-wiring.test.ts index efb22f9fc6..eeffa446e5 100644 --- a/test/unit/reputation-wiring.test.ts +++ b/test/unit/reputation-wiring.test.ts @@ -6,7 +6,7 @@ import { shouldDowngradeToDeterministic, shouldSkipAiForReputation, } from "../../src/review/reputation-wire"; -import { getSubmitterReputation } from "../../src/review/submitter-reputation"; +import { getSubmitterReputation, recordSubmissionOutcome } from "../../src/review/submitter-reputation"; import { evaluateGateCheck } from "../../src/rules/advisory"; import type { Advisory, RepositorySettings } from "../../src/types"; import { createTestEnv } from "../helpers/d1"; @@ -303,6 +303,29 @@ describe("recordReputationOutcome + the 0046 submitter_stats migration", () => { expect(stats.closed).toBe(1); expect(stats.closeRate).toBeCloseTo(0.5, 5); }); + + it("REGRESSION: qualifies submitter_stats counters in the upsert update for Postgres", async () => { + let preparedSql = ""; + const env = { + DB: { + prepare: vi.fn((sql: string) => { + preparedSql = sql; + return { + bind: vi.fn(() => ({ + run: vi.fn(async () => ({})), + })), + }; + }), + }, + } as unknown as Env; + + await recordSubmissionOutcome(env, "acme/widgets", "alice", "merged"); + + expect(preparedSql).toContain("submissions = submitter_stats.submissions + 1"); + expect(preparedSql).toContain("merged = submitter_stats.merged + 1"); + expect(preparedSql).not.toContain("submissions = submissions + 1"); + expect(preparedSql).not.toContain("merged = merged + 1"); + }); }); describe("reputationOutcomeFromTerminalState (pure)", () => { diff --git a/test/unit/selfhost-observability-config.test.ts b/test/unit/selfhost-observability-config.test.ts index 1fa78d5d8f..6e79d457ce 100644 --- a/test/unit/selfhost-observability-config.test.ts +++ b/test/unit/selfhost-observability-config.test.ts @@ -113,6 +113,11 @@ describe("self-host observability trace config", () => { "./scripts/backup-metrics.sh:/backup-metrics.sh:ro", ]), ); + expect(backupExporter.command).toEqual([ + "/bin/sh", + "-c", + "apk add --no-cache busybox-extras && sh /backup-metrics.sh", + ]); expect(backupExporter.healthcheck?.test).toEqual([ "CMD-SHELL", "wget -qO- http://127.0.0.1:9101/metrics | grep -q '^gittensory_backup_latest_timestamp_seconds'", @@ -126,6 +131,7 @@ describe("self-host observability trace config", () => { }), expect.objectContaining({ job_name: "gittensory-backup", + fallback_scrape_protocol: "PrometheusText0.0.4", static_configs: [{ targets: ["backup-exporter:9101"] }], }), ]), diff --git a/test/unit/submitter-reputation.test.ts b/test/unit/submitter-reputation.test.ts index 5383b0155b..80b72aa8f0 100644 --- a/test/unit/submitter-reputation.test.ts +++ b/test/unit/submitter-reputation.test.ts @@ -216,17 +216,20 @@ describe("recordSubmissionOutcome / getSubmitterReputation (D1, fail-safe)", () await recordSubmissionOutcome(mkEnv(), "p", "u", "merged"); expect(seen[0]).toContain(", merged, last_seen)"); - expect(seen[0]).toContain("merged = merged + 1"); + expect(seen[0]).toContain("submissions = submitter_stats.submissions + 1"); + expect(seen[0]).toContain("merged = submitter_stats.merged + 1"); seen.length = 0; await recordSubmissionOutcome(mkEnv(), "p", "u", "closed"); expect(seen[0]).toContain(", closed, last_seen)"); - expect(seen[0]).toContain("closed = closed + 1"); + expect(seen[0]).toContain("submissions = submitter_stats.submissions + 1"); + expect(seen[0]).toContain("closed = submitter_stats.closed + 1"); seen.length = 0; await recordSubmissionOutcome(mkEnv(), "p", "u", "manual"); expect(seen[0]).toContain(", manual, last_seen)"); - expect(seen[0]).toContain("manual = manual + 1"); + expect(seen[0]).toContain("submissions = submitter_stats.submissions + 1"); + expect(seen[0]).toContain("manual = submitter_stats.manual + 1"); }); it("recordSubmissionOutcome swallows a DB error fail-safe (logs, never throws)", async () => {