Skip to content

refactor(queue): extract AI-slop-advisory gating/orchestration into its own module #4946

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JSONbored merged 1 commit into from
Jul 11, 2026
Merged

refactor(queue): extract AI-slop-advisory gating/orchestration into its own module #4946

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
232 changes: 15 additions & 217 deletions src/queue/processors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ import {
countPublishedAiReviewHeads,
putCachedAiReview,
markAiReviewPublished,
getCachedAiSlopAdvisory,
putCachedAiSlopAdvisory,
hasPublishedAiSlopAdvisory,
getCachedLinkedIssueSatisfaction,
getLatestPublishedLinkedIssueSatisfaction,
Expand Down Expand Up @@ -285,7 +283,6 @@ import {
queueSnapshotFromBinding,
} from "../selfhost/queue-common";
import { aiReviewCacheInputFingerprint } from "../review/ai-review-cache-input";
import { aiSlopCacheInputFingerprint } from "../review/ai-slop-cache-input";
import { linkedIssueSatisfactionCacheInputFingerprint } from "../review/linked-issue-satisfaction-cache-input";
import {
AGENT_LABEL_NEEDS_REVIEW,
Expand Down Expand Up @@ -352,7 +349,15 @@ import {
type ContributorProfile,
} from "../signals/engine";
import { isDuplicateClusterWinnerByClaim, resolveDuplicateClusterWinnerNumber } from "../signals/duplicate-winner";
import { buildUnifiedReviewDiff, totalAddedLineCount } from "../review/review-diff";
import { buildAiReviewDiff, buildSecretScanDiff, buildUnifiedReviewDiff, totalAddedLineCount } from "../review/review-diff";
// #4013 step 4 (prep): buildAiReviewDiff/buildSecretScanDiff moved to review-diff.ts (a natural existing
// home -- both already wrapped buildUnifiedReviewDiff there) rather than staying here, since keeping them
// in this file would have made the new slop-detection.ts below circularly import this file just for
// buildAiReviewDiff. Re-exported so test/unit/linked-issue-satisfaction-run.test.ts,
// test/unit/ai-review-advisory.test.ts, test/unit/ai-slop.test.ts, and
// test/unit/patchless-secret-scan.test.ts's existing `import { ... } from "../../src/queue/processors"`
// keep working unchanged.
export { buildAiReviewDiff, buildSecretScanDiff } from "../review/review-diff";
import { estimateReviewEffort } from "../review/review-effort";
import { buildUnifiedCommentBody } from "../review/unified-comment-bridge";
import { isRetryableJobError, RetryableJobError } from "./retryable";
Expand All @@ -378,6 +383,12 @@ export { claimPrActuationLock, releasePrActuationLock } from "./transient-locks"
// two files circularly dependent.
import { generateSignalSnapshots, loadOpenQueueCounts } from "./signal-snapshot";
export { generateSignalSnapshots } from "./signal-snapshot";
// #4013 step 4: same shim shape for the AI-slop-advisory gating/orchestration functions -- imported here
// for this file's own internal callers, and re-exported so test/unit/advisory-ai-routing-call-sites.test.ts,
// test/unit/ai-slop.test.ts, and test/unit/gate-check-policy.test.ts's existing
// `import { ... } from "../../src/queue/processors"` keeps working unchanged.
import { runAiSlopForAdvisory, shouldCollectSlopEvidence, shouldRunSlopAiAdvisory } from "./slop-detection";
export { runAiSlopForAdvisory, shouldCollectSlopEvidence, shouldRunSlopAiAdvisory } from "./slop-detection";
import { isVisualPath } from "../review/visual/paths";
import { buildCapture, fetchShotContentBlock, hasSuccessfulBotCapture, resolveVisualRoutes, type CaptureRoute } from "../review/visual/capture";
import {
Expand Down Expand Up @@ -408,7 +419,6 @@ import {
buildSlopAssessment,
type SlopBand,
} from "../signals/slop";
import { runGittensoryAiSlopAdvisory } from "../services/ai-slop";
import { runGittensoryLinkedIssueSatisfaction } from "../services/linked-issue-satisfaction-run";
import { decidePublicSurface } from "../signals/settings-preview";
import {
Expand Down Expand Up @@ -6918,12 +6928,6 @@ export async function resolveLinkedIssueAdvisoryContext(
return { linkedIssueAuthorLogins, confirmedNoOpenLinkedIssue: !hasOpenReference };
}

export function shouldCollectSlopEvidence(
settings: Pick<RepositorySettings, "slopGateMode" | "mergeReadinessGateMode">,
): boolean {
return settings.slopGateMode !== "off" || mergeReadinessGateEnabled(settings);
}

export async function shouldRefreshFilesForPreMergeChecks(
env: Env,
repoFullName: string,
Expand All @@ -6934,12 +6938,6 @@ export async function shouldRefreshFilesForPreMergeChecks(
return checks.some((check) => check.whenPaths.length > 0);
}

export function shouldRunSlopAiAdvisory(
settings: Pick<RepositorySettings, "slopAiAdvisory" | "slopGateMode">,
): boolean {
return settings.slopAiAdvisory && settings.slopGateMode !== "off";
}

/** #one-shot-review-cadence: resolve the effective AI review re-trigger cadence. The per-repo
* `review.auto_review.cadence` manifest field (`configuredCadence`, already resolved by
* resolveReviewAutoReviewConfig) always wins when set; otherwise falls back to the operator's fleet-wide
Expand Down Expand Up @@ -7118,53 +7116,6 @@ async function resolvePullRequestFilesForReview(
return stored;
}

/** Build a bounded unified-diff string from cached PR files for the AI reviewer. Caps total size so a
* huge PR cannot blow the model context or the neuron budget; each file's patch is taken from the raw
* GitHub file payload when present. */
export function buildAiReviewDiff(
files: Awaited<ReturnType<typeof listPullRequestFiles>>,
): string {
// Source-first + hunk-aware + always-list-dropped-files (ported from reviewbot). The old blind 60k
// head-slice `break`-dropped whole files in stored order, so the file DEFINING a symbol could vanish
// while another referenced it → the model hallucinated "missing import / undefined symbol" (the #1528
// class, which survived even with grounding on). (#accuracy-gap-1)
return buildUnifiedReviewDiff(
files.map((file) => ({
path: file.path,
patch:
typeof file.payload?.patch === "string"
? file.payload.patch
: undefined,
status: file.status,
additions: file.additions,
deletions: file.deletions,
})),
);
}

/**
* Build the complete inline patch corpus for deterministic secret scanning. Unlike {@link buildAiReviewDiff},
* this is intentionally unbudgeted and does not reorder files or drop hunks: security controls must inspect
* every raw patch GitHub returned instead of the lossy AI-review prompt view.
*
* GitHub omits inline `patch` for binary/large files; {@link enrichSecretScanFilesWithPatchFallback} recovers
* scannable `+` lines for those files before this runs (see {@link maybeAddSecretLeakFinding}).
*/
export function buildSecretScanDiff(
files: Awaited<ReturnType<typeof listPullRequestFiles>>,
): string {
return files
.map((file) => {
const status = file.status ?? "modified";
const header = `### ${file.path} (${status}) +${file.additions ?? 0}/-${file.deletions ?? 0}`;
const patch =
typeof file.payload?.patch === "string" ? file.payload.patch : "";
return patch ? `${header}\n${patch}` : header;
})
.join("\n\n")
.trim();
}

/**
* Run the opt-in AI maintainer review and fold it into the gate + panel. Mutates `advisory.findings`
* with a dual-model consensus defect (when `aiReviewMode: block` and the free Workers-AI pair agrees with
Expand Down Expand Up @@ -8073,159 +8024,6 @@ export async function maybeAddLockfileTamperFinding(
}
}

/**
* AI-assisted slop advisory (opt-in `slopAiAdvisory`). Appends at most one ADVISORY-only `ai_slop_advisory`
* finding to the advisory; NEVER touches slopRisk or the gate (only the deterministic core can block). The
* caller gates on `settings.slopAiAdvisory` and reuses the already-fetched changed files. Like the AI review
* path, it runs ONLY for confirmed contributors so an unconfirmed/untrusted PR author cannot spend either the
* shared Workers AI budget or the maintainer-paid BYOK quota. Fail-safe: any AI error is swallowed so the
* gate still finalizes.
*
* `commitThresholdReached` (#ai-slop-repeat-spend): mirrors `ai_review`'s OWN `auto_pause_after_reviewed_commits`
* cap (`isAutoReviewCommitThresholdReached`) — a PR that's already been reviewed this many times at essentially
* its current state stops getting a fresh slop advisory too. Before this, every sweep pass re-ran the FULL
* advisory regardless of how many times the SAME head had already been checked (only a headSha+prompt-fingerprint
* cache guarded re-spend, so a stale PR the sweep kept re-visiting paid for a fresh attempt on every pass).
*/
export async function runAiSlopForAdvisory(
env: Env,
args: {
// See runAiReviewForAdvisory's doc comment on this same field (#token-bleed-spend-gate) -- a paused repo
// must never reach the LLM call below, independent of settings.slopAiAdvisory.
mode: AgentActionMode;
settings: RepositorySettings;
advisory: Awaited<ReturnType<typeof buildPullRequestAdvisory>>;
repoFullName: string;
pr: { number: number; title: string; body?: string | null | undefined };
author: string | null;
files: Awaited<ReturnType<typeof listPullRequestFiles>>;
deterministicBand: SlopBand;
confirmedContributor: boolean;
commitThresholdReached: boolean;
},
): Promise<void> {
// Confirmed-contributor gate (matches runAiReviewForAdvisory): no AI spend — free OR BYOK — on a PR from
// an unconfirmed author. The deterministic slop core still ran for everyone; only the AI layer is gated.
if (args.mode === "paused" || !args.confirmedContributor || !args.advisory.headSha) return;
if (args.commitThresholdReached) {
await recordAuditEvent(env, {
eventType: "github_app.ai_slop_auto_review_skipped",
actor: args.author,
targetKey: `${args.repoFullName}#${args.pr.number}`,
outcome: "completed",
detail: "slop advisory paused (commit threshold); this head has already been reviewed enough times",
metadata: { repoFullName: args.repoFullName, headSha: args.advisory.headSha },
}).catch(
/* v8 ignore next -- fail-safe: an audit write failure never blocks the handler */
() => undefined,
);
return;
}
try {
// BYOK (opt-in): reuse the repo's encrypted key + aiReviewByok flag — one BYOK key serves both AI
// features. A declared provider must match the stored key's provider, else skip BYOK (Workers-AI
// fallback). The contributor is already confirmed (early return above), so BYOK billing is authorized.
// The slop advisory stays advisory-only regardless of which model writes it.
const storedKey = args.settings.aiReviewByok
? await getDecryptedRepositoryAiKey(env, args.repoFullName)
: null;
const providerKey =
storedKey &&
(!args.settings.aiReviewProvider ||
args.settings.aiReviewProvider === storedKey.provider)
? {
provider: storedKey.provider,
key: storedKey.key,
model: args.settings.aiReviewModel ?? storedKey.model,
}
: null;
// #ai-slop-cache: repeated scheduled sweeps at an unchanged prompt reuse the stored result instead of
// re-spending up to 6 free-tier attempts (or a BYOK call) on every tick. The fingerprint includes the
// provider identity plus the prompt-shaping inputs that can drift for the same head SHA (PR edits,
// retarget/base-diff changes, or deterministic-band setting changes).
const aiSlopDiff = buildAiReviewDiff(args.files);
const inputFingerprint = await aiSlopCacheInputFingerprint({
title: args.pr.title,
body: args.pr.body ?? null,
diff: aiSlopDiff,
deterministicBand: args.deterministicBand,
byok: Boolean(providerKey),
provider: providerKey?.provider,
model: providerKey?.model,
});
const cachedSlop = await getCachedAiSlopAdvisory(env, args.repoFullName, args.pr.number, args.advisory.headSha, inputFingerprint).catch(() => null);
let result: Awaited<ReturnType<typeof runGittensoryAiSlopAdvisory>>;
if (cachedSlop) {
result = { status: "ok", finding: cachedSlop.finding, band: cachedSlop.band as SlopBand | null, estimatedNeurons: cachedSlop.estimatedNeurons };
incr("gittensory_ai_slop_cache_hit_total");
await recordAuditEvent(env, {
eventType: "github_app.ai_slop_cache_hit",
actor: args.author,
targetKey: `${args.repoFullName}#${args.pr.number}`,
outcome: "completed",
detail: "reused a stored AI slop advisory instead of re-spending an LLM call",
/* v8 ignore next -- reached only past this function's own `!args.advisory.headSha` early return, so headSha is always truthy here; the `?? null` is a type-level fallback for an unreachable branch. */
metadata: { repoFullName: args.repoFullName, headSha: args.advisory.headSha ?? null },
}).catch(() => undefined);
} else {
incr("gittensory_ai_slop_cache_miss_total");
await recordAuditEvent(env, {
eventType: "github_app.ai_slop_cache_miss",
actor: args.author,
targetKey: `${args.repoFullName}#${args.pr.number}`,
outcome: "completed",
detail: "no reusable stored AI slop advisory for this head+fingerprint; running a fresh advisory",
/* v8 ignore next -- reached only past this function's own `!args.advisory.headSha` early return, so headSha is always truthy here; the `?? null` is a type-level fallback for an unreachable branch. */
metadata: { repoFullName: args.repoFullName, headSha: args.advisory.headSha ?? null },
}).catch(() => undefined);
result = await runGittensoryAiSlopAdvisory(withAdvisoryAiEnv(env, args.settings.advisoryAiRouting?.slop === true), {
repoFullName: args.repoFullName,
prNumber: args.pr.number,
title: args.pr.title,
body: args.pr.body ?? undefined,
diff: aiSlopDiff,
actor: args.author,
deterministicBand: args.deterministicBand,
providerKey,
});
// Only "ok" actually spent the LLM call (free-tier attempts or a BYOK call) — disabled/unavailable/
// quota_exceeded all short-circuit BEFORE any provider call, so caching them would suppress a legitimate
// retry once the condition clears without having saved anything.
if (result.status === "ok") {
await putCachedAiSlopAdvisory(env, args.repoFullName, args.pr.number, args.advisory.headSha, inputFingerprint, {
status: result.status,
band: result.band,
finding: result.finding,
estimatedNeurons: result.estimatedNeurons,
}).catch((error) => {
incr("gittensory_ai_slop_cache_write_error_total");
return recordAuditEvent(env, {
eventType: "github_app.ai_slop_cache_write_error",
actor: args.author,
targetKey: `${args.repoFullName}#${args.pr.number}`,
outcome: "error",
detail: errorMessage(error),
/* v8 ignore next -- reached only past this function's own `!args.advisory.headSha` early return, so headSha is always truthy here; the `?? null` is a type-level fallback for an unreachable branch. */
metadata: { repoFullName: args.repoFullName, headSha: args.advisory.headSha ?? null },
}).catch(() => undefined);
});
}
}
if (result.status === "ok" && result.finding)
args.advisory.findings.push(result.finding);
} catch (error) {
console.error(
JSON.stringify({
level: "warn",
event: "ai_slop_failed",
repository: args.repoFullName,
pullNumber: args.pr.number,
error: errorMessage(error),
}),
);
}
}

/**
* Run the linked-issue satisfaction assessment for advisory purposes (#1961/#3906) — opt-in via
* `linkedIssueSatisfactionGateMode != "off"`. Assesses only the PR's PRIMARY (first) linked issue: v1 chooses
Expand Down
Loading