From 71234c207d11018f465ee87cfaa3572a23053379 Mon Sep 17 00:00:00 2001 From: ghost <49853598+JSONbored@users.noreply.github.com> Date: Sun, 14 Jun 2026 10:56:12 -0700 Subject: [PATCH 1/2] fix: gate BYOK slop advisory by contributor --- src/queue/processors.ts | 33 ++++++++++++++++++--------------- test/unit/ai-slop.test.ts | 37 +++++++++++++++++++++++++++++++++++-- 2 files changed, 53 insertions(+), 17 deletions(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index c30a8c8fd0..3b75939805 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -949,14 +949,17 @@ export async function runAiSlopForAdvisory( author: string | null; files: Awaited>; deterministicBand: SlopBand; + confirmedContributor: boolean; }, ): Promise { if (!args.advisory.headSha) 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 slop advisory stays advisory-only regardless of which model writes it. - const storedKey = args.settings.aiReviewByok ? await getDecryptedRepositoryAiKey(env, args.repoFullName) : null; + // fallback). Because BYOK bills the maintainer, only confirmed contributors may use it; + // unconfirmed PRs fall back to Workers AI. The slop advisory stays advisory-only regardless of + // which model writes it. + const storedKey = args.settings.aiReviewByok && args.confirmedContributor ? 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 } @@ -1157,6 +1160,18 @@ async function maybePublishPrPublicSurface( scopedOverlapCount: unionScopedOverlapClusters(collisions, pr, preflight.collisions).length, }); + if (gateEnabled && author && !publicSurfaceSkipped && !official) { + official = await getCachedOfficialMinerDetection(env, author, { + targetKey: `${repoFullName}#${pr.number}`, + deliveryId: webhook.deliveryId, + }); + } + + // Only CONFIRMED gittensor contributors can be hard-blocked; everyone else (or an unavailable + // detection) gets a neutral, non-blocking gate. Gate-only runs still verify confirmation before + // evaluating blockers so confirmed contributors cannot bypass a required Gate check. + const confirmedContributor = official?.status === "confirmed"; + // Anti-slop (#530/#532): only when opted in (slopGateMode !== "off"). Surface the deterministic slop // findings as advisory context, and feed the score to the gate (it only blocks under slop: block + the // threshold). Loads files lazily so disabled repos pay nothing. @@ -1175,22 +1190,10 @@ async function maybePublishPrPublicSurface( // AI-assisted slop advisory (#533, opt-in). Reuses the already-fetched files; appends at most one // advisory-only finding. Deliberately does NOT update slopRisk — only the deterministic core blocks. if (settings.slopAiAdvisory) { - await runAiSlopForAdvisory(env, { settings, advisory, repoFullName, pr, author, files: slopFiles, deterministicBand: slop.band }); + await runAiSlopForAdvisory(env, { settings, advisory, repoFullName, pr, author, files: slopFiles, deterministicBand: slop.band, confirmedContributor }); } } - if (gateEnabled && author && !publicSurfaceSkipped && !official) { - official = await getCachedOfficialMinerDetection(env, author, { - targetKey: `${repoFullName}#${pr.number}`, - deliveryId: webhook.deliveryId, - }); - } - - // Only CONFIRMED gittensor contributors can be hard-blocked; everyone else (or an unavailable - // detection) gets a neutral, non-blocking gate. Gate-only runs still verify confirmation before - // evaluating blockers so confirmed contributors cannot bypass a required Gate check. - const confirmedContributor = official?.status === "confirmed"; - // AI maintainer review (opt-in via aiReviewMode). Mutates `advisory` with a consensus defect (if any) // BEFORE the gate evaluates, and returns advisory notes for the panel. Inside the try so any AI // failure is caught and the gate is still finalized (never left in_progress). diff --git a/test/unit/ai-slop.test.ts b/test/unit/ai-slop.test.ts index 7df1b63413..a8c61d88dc 100644 --- a/test/unit/ai-slop.test.ts +++ b/test/unit/ai-slop.test.ts @@ -275,6 +275,7 @@ describe("runAiSlopForAdvisory (processor wiring)", () => { author: "alice", files, deterministicBand: "elevated", + confirmedContributor: true, }); expect(adv.findings.map((f) => f.code)).toEqual([AI_SLOP_FINDING_CODE]); }); @@ -283,7 +284,7 @@ describe("runAiSlopForAdvisory (processor wiring)", () => { const noSha = advisory(); delete (noSha as Partial).headSha; const run = vi.fn(); - await runAiSlopForAdvisory(enabledEnv(run), { settings: noByok, advisory: noSha, repoFullName: "acme/widgets", pr, author: "alice", files, deterministicBand: "low" }); + await runAiSlopForAdvisory(enabledEnv(run), { settings: noByok, advisory: noSha, repoFullName: "acme/widgets", pr, author: "alice", files, deterministicBand: "low", confirmedContributor: true }); expect(noSha.findings).toEqual([]); expect(run).not.toHaveBeenCalled(); }); @@ -298,6 +299,7 @@ describe("runAiSlopForAdvisory (processor wiring)", () => { author: "alice", files, deterministicBand: "clean", + confirmedContributor: true, }); expect(adv.findings).toEqual([]); }); @@ -305,7 +307,7 @@ describe("runAiSlopForAdvisory (processor wiring)", () => { it("is fail-safe: a thrown error (broken DB) yields no finding and never throws", async () => { const adv = advisory(); const env = { ...enabledEnv(async () => ({ response: slopJson() })), DB: undefined } as unknown as Env; - await expect(runAiSlopForAdvisory(env, { settings: noByok, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, deterministicBand: "high" })).resolves.toBeUndefined(); + await expect(runAiSlopForAdvisory(env, { settings: noByok, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, deterministicBand: "high", confirmedContributor: true })).resolves.toBeUndefined(); expect(adv.findings).toEqual([]); }); @@ -330,10 +332,41 @@ describe("runAiSlopForAdvisory (processor wiring)", () => { author: "alice", files, deterministicBand: "elevated", + confirmedContributor: true, }); // The advisory came from the BYOK provider (high band → finding), and Workers AI was never called. expect(adv.findings.map((f) => f.code)).toEqual([AI_SLOP_FINDING_CODE]); expect(fetchMock.mock.calls[0]?.[0]).toBe("https://api.anthropic.com/v1/messages"); expect(run).not.toHaveBeenCalled(); }); + + it("does not use the maintainer BYOK key for unconfirmed contributors", async () => { + const run = vi.fn(async () => ({ response: slopJson({ band: "high" }) })); + const env = createTestEnv({ + AI: { run } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + TOKEN_ENCRYPTION_SECRET: "ai-slop-byok-test-encryption-secret-32b", + }); + await upsertRepositoryAiKey(env, { repoFullName: "acme/widgets", provider: "anthropic", key: "sk-ant-byok-slop-9999", model: null }); + const fetchMock = vi.fn(async () => new Response(JSON.stringify({ content: [{ type: "text", text: slopJson({ band: "high" }) }] }), { status: 200 })); + vi.stubGlobal("fetch", fetchMock); + const adv = advisory(); + await runAiSlopForAdvisory(env, { + settings: { aiReviewByok: true } as RepositorySettings, + advisory: adv, + repoFullName: "acme/widgets", + pr, + author: "mallory", + files, + deterministicBand: "elevated", + confirmedContributor: false, + }); + + expect(adv.findings.map((f) => f.code)).toEqual([AI_SLOP_FINDING_CODE]); + expect(fetchMock).not.toHaveBeenCalled(); + expect(run).toHaveBeenCalled(); + }); + }); From 1c278b826ea5d49879a21c4b7a43066cc2dcc5ea Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sun, 14 Jun 2026 11:35:23 -0700 Subject: [PATCH 2/2] fix(slop): fully gate the AI slop advisory on confirmed contributor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes the BYOK-only gating in this PR by closing the remaining cost vector: the AI slop advisory was still invoking FREE Workers AI for any PR author (the shared daily neuron budget), even though the AI review path (runAiReviewForAdvisory) already no-ops for unconfirmed authors. - runAiSlopForAdvisory now returns early when `!confirmedContributor` (matching the AI review path exactly): no AI spend — free OR BYOK — on a PR from an unconfirmed/untrusted author. The deterministic slop core still runs for everyone; only the paid AI layer is gated. Fail-closed: if the contributor can't be confirmed, no AI runs. - The BYOK key lookup no longer needs its own `&& confirmedContributor` guard (the early return already guarantees it). Adopts the full-gate approach from the sibling PR #727 (which also spotted this vector) while preserving this branch's BYOK integration (#728). Deliberately does NOT adopt #727's neuron-budget bump (1 -> 6): the AI review path estimates by LOGICAL free-call count, not worst-case retries, so a 6x reservation would be inconsistent and over-reserve, causing premature quota_exceeded. --- src/queue/processors.ts | 17 ++++++++++------- test/unit/ai-slop.test.ts | 8 ++++---- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 4089869b14..6cd4685e51 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -940,8 +940,10 @@ export async function runAiReviewForAdvisory( /** * 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. Fail-safe: any AI - * error is swallowed so the gate still finalizes. + * 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. */ export async function runAiSlopForAdvisory( env: Env, @@ -956,14 +958,15 @@ export async function runAiSlopForAdvisory( confirmedContributor: boolean; }, ): Promise { - if (!args.advisory.headSha) return; + // 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.confirmedContributor || !args.advisory.headSha) 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). Because BYOK bills the maintainer, only confirmed contributors may use it; - // unconfirmed PRs fall back to Workers AI. The slop advisory stays advisory-only regardless of - // which model writes it. - const storedKey = args.settings.aiReviewByok && args.confirmedContributor ? await getDecryptedRepositoryAiKey(env, args.repoFullName) : null; + // 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 } diff --git a/test/unit/ai-slop.test.ts b/test/unit/ai-slop.test.ts index a8c61d88dc..1b2ff87aa5 100644 --- a/test/unit/ai-slop.test.ts +++ b/test/unit/ai-slop.test.ts @@ -340,7 +340,7 @@ describe("runAiSlopForAdvisory (processor wiring)", () => { expect(run).not.toHaveBeenCalled(); }); - it("does not use the maintainer BYOK key for unconfirmed contributors", async () => { + it("no-ops entirely for unconfirmed contributors — neither the maintainer BYOK key nor free Workers AI is spent", async () => { const run = vi.fn(async () => ({ response: slopJson({ band: "high" }) })); const env = createTestEnv({ AI: { run } as unknown as Ai, @@ -364,9 +364,9 @@ describe("runAiSlopForAdvisory (processor wiring)", () => { confirmedContributor: false, }); - expect(adv.findings.map((f) => f.code)).toEqual([AI_SLOP_FINDING_CODE]); + // Matches the AI review path: an unconfirmed author triggers no AI spend at all, so no finding lands. + expect(adv.findings).toEqual([]); expect(fetchMock).not.toHaveBeenCalled(); - expect(run).toHaveBeenCalled(); + expect(run).not.toHaveBeenCalled(); }); - });