From 9a6ba94c4b778fd6cd37222052ba39d925d902e0 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 9 Jul 2026 18:16:36 -0700 Subject: [PATCH 1/2] fix(review): per-actor rate ceiling + shared budget check on unlinked-issue AI verifier The unlinked-issue-match AI verifier had no cost bound of its own: an actor opening many low-quality PRs across many repos could trigger arbitrarily many AI calls in a short window, and the verifier's own candidate loop could run against the shared daily neuron budget with no awareness of it. Add two gates ahead of the verification loop, both fail-safe toward "proceed normally" on a read error so a DB hiccup can never silently disable the guardrail: a per-actor rate ceiling (tracked via a new audit-event type) and a check against the same shared daily neuron budget every other free-tier AI feature draws from. Fixes #4515 --- src/db/repositories.ts | 15 +++ src/review/unlinked-issue-guardrail.ts | 83 ++++++++++++++- src/review/unlinked-issue-match.ts | 6 +- src/signals/unlinked-issue-candidates.ts | 6 +- test/unit/unlinked-issue-guardrail.test.ts | 115 ++++++++++++++++++++- 5 files changed, 216 insertions(+), 9 deletions(-) diff --git a/src/db/repositories.ts b/src/db/repositories.ts index b22e6cc95f..16093dce7f 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -2636,6 +2636,21 @@ export async function mostRecentAuditEventForOtherTarget(env: Env, actor: string return rows[0]?.createdAt ?? null; } +/** Count-returning, cross-repo variant (#4515): how many recent events of this type has this actor generated, + * across EVERY target (repo/PR), within the recency window? Unlike {@link countRecentAuditEventsForActorAndTarget} + * (scoped to one target thread), this counts an actor's ACTIVITY VOLUME irrespective of which PR/repo each + * event landed on -- backs a per-actor rate ceiling on an expensive operation (e.g. a paid AI call) that a + * single-target-scoped counter would never catch for an actor spreading attempts across many repos. */ +export async function countRecentAuditEventsForActor(env: Env, actor: string, eventType: string, sinceIso: string): Promise { + const db = getDb(env.DB); + const [row] = await db + .select({ count: sql`count(*)` }) + .from(auditEvents) + .where(and(eq(auditEvents.actor, actor), eq(auditEvents.eventType, eventType), gte(auditEvents.createdAt, sinceIso))); + /* v8 ignore next -- count(*) always returns exactly one row; the empty-array guard only satisfies the destructure type. */ + return row?.count ?? 0; +} + /** Count-returning variant of {@link hasRecentAuditEvent}, additionally scoped to one `targetKey` (e.g. a single * `owner/repo#123` PR/issue) rather than the actor's activity across the whole repo. Backs the review-request * nagging cooldown (#2463): counting how many `@gittensory` pings a contributor has sent on ONE thread within diff --git a/src/review/unlinked-issue-guardrail.ts b/src/review/unlinked-issue-guardrail.ts index 7ca8a07176..ab4f20dbf4 100644 --- a/src/review/unlinked-issue-guardrail.ts +++ b/src/review/unlinked-issue-guardrail.ts @@ -13,11 +13,20 @@ // repo that hasn't opted in (the default) or a PR that already links an issue (the common case) pays // nothing beyond two boolean checks. -import { getFreshOfficialMinerDetection, mostRecentAuditEventForOtherTarget, listOpenIssues, recordAuditEvent, upsertOfficialMinerDetection } from "../db/repositories"; +import { + countRecentAuditEventsForActor, + getFreshOfficialMinerDetection, + mostRecentAuditEventForOtherTarget, + listOpenIssues, + recordAuditEvent, + sumAiEstimatedNeuronsSince, + upsertOfficialMinerDetection, +} from "../db/repositories"; import { fetchOfficialGittensorMiner } from "../gittensor/api"; -import { findUnlinkedIssueCandidates, type CandidateOpenIssue } from "../signals/unlinked-issue-candidates"; +import { clampNumber, estimateNeurons, utcDayStartIso } from "../services/ai-review"; +import { findUnlinkedIssueCandidates, MAX_CANDIDATES, type CandidateOpenIssue } from "../signals/unlinked-issue-candidates"; import type { UnlinkedIssueGuardrailConfig } from "../types"; -import { verifyUnlinkedIssueMatch } from "./unlinked-issue-match"; +import { DIFF_CHAR_BUDGET, MAX_TOKENS, verifyUnlinkedIssueMatch } from "./unlinked-issue-match"; /** Shared with any future reader that wants to correlate these holds/closes across repos for one contributor. */ export const UNLINKED_ISSUE_MATCH_AUDIT_EVENT_TYPE = "github_app.unlinked_issue_match_hold"; @@ -35,6 +44,56 @@ const VELOCITY_EXCEPTION_MAX_GAP_MS = 60 * 60 * 1000; const OFFICIAL_MINER_DETECTION_TTL_MS = 5 * 60 * 1000; const OFFICIAL_MINER_DETECTION_UNAVAILABLE_TTL_MS = 60 * 1000; +// #4515: every candidate below costs one real (if small) AI call, so two cost-control gates run ahead of the +// loop -- a per-actor RATE ceiling, and a check against the shared daily neuron budget every other free-tier +// AI feature draws from (sumAiEstimatedNeuronsSince/AI_DAILY_NEURON_BUDGET, mirroring ai-slop.ts's own +// pre-call budget check). Both are cost controls, not correctness gates: on any read failure they fail +// toward "proceed as if this layer didn't exist" (full verification runs), never toward silently disabling +// the guardrail they sit in front of by skipping straight to `undefined`. +export const UNLINKED_ISSUE_VERIFY_ATTEMPT_AUDIT_EVENT_TYPE = "github_app.unlinked_issue_verify_attempt"; +// Generous by design: a legitimate contributor never approaches this in an hour even opening several PRs +// back-to-back. Sized to catch a scripted/abusive burst hammering the AI verifier, not ordinary human cadence. +const VERIFY_RATE_CEILING_MAX_ATTEMPTS = 15; +const VERIFY_RATE_CEILING_WINDOW_MS = 60 * 60 * 1000; +// Flat overhead for the parts of the verifier's prompt that aren't the (already-bounded) diff -- the system +// prompt, PR title/body, and candidate issue title/body. None of these are cheaply boundable per candidate +// ahead of time, so this deliberately over-, never under-, estimates: a budget check must never undercount. +const VERIFY_PROMPT_OVERHEAD_CHAR_ESTIMATE = 2_000; +// verifyUnlinkedIssueMatch tries a primary model and, ONLY on a thrown error, a fallback -- two calls is the +// real worst case per candidate, not the common case, but this budget check must size for the worst case. +const VERIFY_MAX_MODEL_ATTEMPTS_PER_CANDIDATE = 2; + +/** Has this actor already run the AI verifier at or beyond the rate ceiling in the last window, across every + * repo/PR? Fail-safe: a read error resolves to "not rate-limited," so the pre-#4515 unconditional- + * verification behavior takes over rather than a DB hiccup silently disabling this guardrail. */ +async function isOverUnlinkedIssueVerifyRateCeiling(env: Env, authorLogin: string): Promise { + const sinceIso = new Date(Date.now() - VERIFY_RATE_CEILING_WINDOW_MS).toISOString(); + const count = await countRecentAuditEventsForActor(env, authorLogin, UNLINKED_ISSUE_VERIFY_ATTEMPT_AUDIT_EVENT_TYPE, sinceIso).catch(() => 0); + return count >= VERIFY_RATE_CEILING_MAX_ATTEMPTS; +} + +/** Would verifying `candidateCount` candidates (each up to {@link VERIFY_MAX_MODEL_ATTEMPTS_PER_CANDIDATE} + * model calls) risk exceeding the shared daily AI neuron budget -- the same counter/env var every other + * free-tier AI feature draws from? `candidateCount` is defensively re-clamped to {@link MAX_CANDIDATES}: the + * caller already bounds it there, but this estimate must never balloon even if that invariant ever slips. + * Fail-safe for the same reason as the rate ceiling above: a read error resolves to "budget available." */ +async function isUnlinkedIssueVerifyBudgetExceeded(env: Env, candidateCount: number): Promise { + const worstCaseCandidateCount = Math.min(candidateCount, MAX_CANDIDATES); + const estimatedNeurons = estimateNeurons( + DIFF_CHAR_BUDGET + VERIFY_PROMPT_OVERHEAD_CHAR_ESTIMATE, + MAX_TOKENS, + worstCaseCandidateCount * VERIFY_MAX_MODEL_ATTEMPTS_PER_CANDIDATE, + ); + // Resolved IDENTICALLY to ai-slop.ts's own pre-call check -- both features sum into the same + // sumAiEstimatedNeuronsSince counter, so a divergent default/ceiling here would under- or over-count + // against the one real shared budget. + const rawNeuronBudget = Number(env.AI_DAILY_NEURON_BUDGET); + const budget = clampNumber(env.AI_DAILY_NEURON_BUDGET && Number.isFinite(rawNeuronBudget) ? rawNeuronBudget : 10_000_000, 0, 10_000_000); + const used = await sumAiEstimatedNeuronsSince(env, utcDayStartIso()).catch(() => 0); + const remainingBudget = Math.max(0, budget - used); + return estimatedNeurons > remainingBudget; +} + /** Minimal cached miner-identity check, deliberately independent of processors.ts's getCachedOfficialMinerDetection * (same cache table and TTLs, no audit-log side effect -- this call site doesn't need one). Fail-safe: any * lookup failure resolves to "not a confirmed miner," never the reverse. */ @@ -101,6 +160,19 @@ async function recordUnlinkedIssueMatchOccurrence(env: Env, repoFullName: string }).catch(() => undefined); } +/** Record that the AI verifier actually ran against one candidate, so {@link isOverUnlinkedIssueVerifyRateCeiling} + * accumulates this actor's volume correctly across every repo/PR they touch, not just this thread. Fire-and- + * forget, same rationale as {@link recordUnlinkedIssueMatchOccurrence}: a write failure must never block the gate. */ +async function recordUnlinkedIssueVerifyAttempt(env: Env, repoFullName: string, pullNumber: number, authorLogin: string): Promise { + await recordAuditEvent(env, { + eventType: UNLINKED_ISSUE_VERIFY_ATTEMPT_AUDIT_EVENT_TYPE, + actor: authorLogin, + targetKey: unlinkedIssueMatchTargetKey(repoFullName, pullNumber), + outcome: "completed", + detail: "unlinked-issue-match AI verifier invoked", + }).catch(() => undefined); +} + /** * Resolve the unlinked-issue-match disposition for one PR, or `undefined` when nothing should hold or close * it. Checks candidates in the pre-filter's ranked order and acts on the FIRST one that clears @@ -124,7 +196,12 @@ export async function resolveUnlinkedIssueMatchDisposition(env: Env, input: Reso }); if (candidates.length === 0) return undefined; const authorLogin = input.prAuthorLogin?.trim() || null; + // #4515: cost-control gates ahead of the AI loop below. An unidentifiable author can't be rate-limited + // individually (nothing to key the ceiling on), so only the shared budget check applies to them. + if (authorLogin && (await isOverUnlinkedIssueVerifyRateCeiling(env, authorLogin))) return undefined; + if (await isUnlinkedIssueVerifyBudgetExceeded(env, candidates.length)) return undefined; for (const candidate of candidates) { + if (authorLogin) await recordUnlinkedIssueVerifyAttempt(env, input.repoFullName, input.pullNumber, authorLogin); const verdict = await verifyUnlinkedIssueMatch(env, { prTitle: input.prTitle, prBody: input.prBody, diff --git a/src/review/unlinked-issue-match.ts b/src/review/unlinked-issue-match.ts index 86254394bb..9d5f03efdc 100644 --- a/src/review/unlinked-issue-match.ts +++ b/src/review/unlinked-issue-match.ts @@ -20,9 +20,11 @@ export type UnlinkedIssueMatchVerdict = { const NO_MATCH: UnlinkedIssueMatchVerdict = { matched: false, confidence: 0, evidence: "" }; -const MAX_TOKENS = 400; +// Exported so the guardrail orchestrator (unlinked-issue-guardrail.ts, #4515) can size its own worst-case +// per-PR AI-spend estimate off the same numbers, rather than a second, driftable copy of them. +export const MAX_TOKENS = 400; // This check only needs enough diff to judge scope overlap, not the full multi-file review budget. -const DIFF_CHAR_BUDGET = 6_000; +export const DIFF_CHAR_BUDGET = 6_000; type AiRunner = { run: (model: string, options: unknown, extra?: unknown) => Promise }; diff --git a/src/signals/unlinked-issue-candidates.ts b/src/signals/unlinked-issue-candidates.ts index 491a7a4fd7..b78b330369 100644 --- a/src/signals/unlinked-issue-candidates.ts +++ b/src/signals/unlinked-issue-candidates.ts @@ -27,8 +27,10 @@ export type FindUnlinkedIssueCandidatesInput = { }; // Bound the AI-verifier fan-out per PR: even a repo with hundreds of open issues only ever sends its -// top-scoring handful for a real (paid/self-host-compute) AI call. -const MAX_CANDIDATES = 3; +// top-scoring handful for a real (paid/self-host-compute) AI call. Exported so the guardrail orchestrator +// (unlinked-issue-guardrail.ts, #4515) can size its own worst-case per-PR AI-spend estimate off the same +// number, rather than a second, driftable copy of this constant. +export const MAX_CANDIDATES = 3; // A path/basename mention is a much stronger signal than shared vocabulary — worth several tokens' score, // and (deliberately) enough on its own to qualify a candidate even with zero token overlap (an issue that // names the exact file this PR touches is worth checking regardless of shared wording). diff --git a/test/unit/unlinked-issue-guardrail.test.ts b/test/unit/unlinked-issue-guardrail.test.ts index 0c1591d36c..128b3c7ea4 100644 --- a/test/unit/unlinked-issue-guardrail.test.ts +++ b/test/unit/unlinked-issue-guardrail.test.ts @@ -1,7 +1,11 @@ import { describe, expect, it, vi } from "vitest"; import { createTestEnv } from "../helpers/d1"; -import { upsertIssueFromGitHub, hasRecentAuditEvent } from "../../src/db/repositories"; -import { resolveUnlinkedIssueMatchDisposition, UNLINKED_ISSUE_MATCH_AUDIT_EVENT_TYPE } from "../../src/review/unlinked-issue-guardrail"; +import { countRecentAuditEventsForActor, recordAuditEvent, upsertIssueFromGitHub, hasRecentAuditEvent } from "../../src/db/repositories"; +import { + resolveUnlinkedIssueMatchDisposition, + UNLINKED_ISSUE_MATCH_AUDIT_EVENT_TYPE, + UNLINKED_ISSUE_VERIFY_ATTEMPT_AUDIT_EVENT_TYPE, +} from "../../src/review/unlinked-issue-guardrail"; import type { UnlinkedIssueGuardrailConfig } from "../../src/types"; function config(overrides: Partial = {}): UnlinkedIssueGuardrailConfig { @@ -364,4 +368,111 @@ describe("resolveUnlinkedIssueMatchDisposition", () => { expect(second?.kind).toBe("hold"); }); }); + + describe("cost-control gates ahead of the AI verifier (#4515)", () => { + async function seedVerifyAttempts(env: Awaited>, actor: string, count: number) { + for (let i = 0; i < count; i++) { + await recordAuditEvent(env, { + eventType: UNLINKED_ISSUE_VERIFY_ATTEMPT_AUDIT_EVENT_TYPE, + actor, + targetKey: `owner/other-repo#${900 + i}`, + outcome: "completed", + detail: "seed", + }); + } + } + + it("skips AI verification entirely once the per-actor rate ceiling is already met, even on a brand new PR", async () => { + const run = vi.fn(async () => ({ response: JSON.stringify(aiVerdict()) })); + const env = createTestEnv({ AI: { run } as unknown as Ai }); + await seedIssue(env, 7, "webhook retry duplicate bug", "retries duplicate events under load, needs a dedup key"); + await seedVerifyAttempts(env, "contributor-a", 15); + + const result = await resolveUnlinkedIssueMatchDisposition(env, { ...BASE_INPUT, config: config() }); + expect(result).toBeUndefined(); + expect(run).not.toHaveBeenCalled(); + }); + + it("still verifies normally just below the rate ceiling (boundary check)", async () => { + const run = vi.fn(async () => ({ response: JSON.stringify(aiVerdict()) })); + const env = createTestEnv({ AI: { run } as unknown as Ai }); + await seedIssue(env, 7, "webhook retry duplicate bug", "retries duplicate events under load, needs a dedup key"); + await seedVerifyAttempts(env, "contributor-a", 14); + + const result = await resolveUnlinkedIssueMatchDisposition(env, { ...BASE_INPUT, config: config() }); + expect(result?.kind).toBe("hold"); + expect(run).toHaveBeenCalled(); + }); + + it("does not count a DIFFERENT actor's verify attempts toward this actor's ceiling", async () => { + const run = vi.fn(async () => ({ response: JSON.stringify(aiVerdict()) })); + const env = createTestEnv({ AI: { run } as unknown as Ai }); + await seedIssue(env, 7, "webhook retry duplicate bug", "retries duplicate events under load, needs a dedup key"); + await seedVerifyAttempts(env, "someone-else", 20); + + const result = await resolveUnlinkedIssueMatchDisposition(env, { ...BASE_INPUT, config: config() }); + expect(result?.kind).toBe("hold"); + expect(run).toHaveBeenCalled(); + }); + + it("records a verify-attempt audit event for the candidate it checks, so the ceiling accumulates across separate PRs", async () => { + const run = vi.fn(async () => ({ response: JSON.stringify(aiVerdict()) })); + const env = createTestEnv({ AI: { run } as unknown as Ai }); + await seedIssue(env, 7, "webhook retry duplicate bug", "retries duplicate events under load, needs a dedup key"); + + await resolveUnlinkedIssueMatchDisposition(env, { ...BASE_INPUT, config: config() }); + + expect(await countRecentAuditEventsForActor(env, "contributor-a", UNLINKED_ISSUE_VERIFY_ATTEMPT_AUDIT_EVENT_TYPE, "2000-01-01T00:00:00.000Z")).toBe(1); + }); + + it("fails open (verification still runs) when the rate-ceiling read itself errors", async () => { + const run = vi.fn(async () => ({ response: JSON.stringify(aiVerdict()) })); + const env = createTestEnv({ AI: { run } as unknown as Ai }); + await seedIssue(env, 7, "webhook retry duplicate bug", "retries duplicate events under load, needs a dedup key"); + const realPrepare = env.DB.prepare.bind(env.DB); + env.DB.prepare = ((sql: string) => { + if (/SELECT.*FROM.*audit_events/i.test(sql)) throw new Error("d1 down"); + return realPrepare(sql); + }) as typeof env.DB.prepare; + + const result = await resolveUnlinkedIssueMatchDisposition(env, { ...BASE_INPUT, config: config() }); + expect(result?.kind).toBe("hold"); + expect(run).toHaveBeenCalled(); + }); + + it("skips AI verification entirely when the shared daily neuron budget is exhausted", async () => { + const run = vi.fn(async () => ({ response: JSON.stringify(aiVerdict()) })); + const env = createTestEnv({ AI: { run } as unknown as Ai, AI_DAILY_NEURON_BUDGET: "1" }); + await seedIssue(env, 7, "webhook retry duplicate bug", "retries duplicate events under load, needs a dedup key"); + + const result = await resolveUnlinkedIssueMatchDisposition(env, { ...BASE_INPUT, config: config() }); + expect(result).toBeUndefined(); + expect(run).not.toHaveBeenCalled(); + }); + + it("defaults the shared neuron budget HIGH (10M) when AI_DAILY_NEURON_BUDGET is unset — does not spuriously block verification", async () => { + const run = vi.fn(async () => ({ response: JSON.stringify(aiVerdict()) })); + const env = createTestEnv({ AI: { run } as unknown as Ai, AI_DAILY_NEURON_BUDGET: "" }); + await seedIssue(env, 7, "webhook retry duplicate bug", "retries duplicate events under load, needs a dedup key"); + + const result = await resolveUnlinkedIssueMatchDisposition(env, { ...BASE_INPUT, config: config() }); + expect(result?.kind).toBe("hold"); + expect(run).toHaveBeenCalled(); + }); + + it("fails open (verification still runs) when the shared-budget read itself errors", async () => { + const run = vi.fn(async () => ({ response: JSON.stringify(aiVerdict()) })); + const env = createTestEnv({ AI: { run } as unknown as Ai }); + await seedIssue(env, 7, "webhook retry duplicate bug", "retries duplicate events under load, needs a dedup key"); + const realPrepare = env.DB.prepare.bind(env.DB); + env.DB.prepare = ((sql: string) => { + if (/SELECT.*FROM.*ai_usage_events/i.test(sql)) throw new Error("d1 down"); + return realPrepare(sql); + }) as typeof env.DB.prepare; + + const result = await resolveUnlinkedIssueMatchDisposition(env, { ...BASE_INPUT, config: config() }); + expect(result?.kind).toBe("hold"); + expect(run).toHaveBeenCalled(); + }); + }); }); From b1540df6cfd65e668ed327cea0ffadee1cdf02a1 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 9 Jul 2026 19:07:29 -0700 Subject: [PATCH 2/2] fix(review): record unlinked-issue verifier's own AI spend into the shared budget counter Review feedback on #4551: the shared-budget check added in this PR read sumAiEstimatedNeuronsSince before running the verifier, but nothing wrote this feature's own actual calls back into that counter -- a free rider that respected every other feature's spend while never counting its own, so the true aggregate could silently exceed AI_DAILY_NEURON_BUDGET by however much this feature used. Record the same worst-case per-candidate estimate the budget check itself uses, mirroring ai-slop.ts's pre-computed-estimate recording convention. --- src/review/unlinked-issue-guardrail.ts | 31 ++++++++++++++++- test/unit/unlinked-issue-guardrail.test.ts | 40 +++++++++++++++++++++- 2 files changed, 69 insertions(+), 2 deletions(-) diff --git a/src/review/unlinked-issue-guardrail.ts b/src/review/unlinked-issue-guardrail.ts index ab4f20dbf4..aabd127cd2 100644 --- a/src/review/unlinked-issue-guardrail.ts +++ b/src/review/unlinked-issue-guardrail.ts @@ -18,12 +18,13 @@ import { getFreshOfficialMinerDetection, mostRecentAuditEventForOtherTarget, listOpenIssues, + recordAiUsageEvent, recordAuditEvent, sumAiEstimatedNeuronsSince, upsertOfficialMinerDetection, } from "../db/repositories"; import { fetchOfficialGittensorMiner } from "../gittensor/api"; -import { clampNumber, estimateNeurons, utcDayStartIso } from "../services/ai-review"; +import { BEST_REVIEW_MODELS, RELIABLE_FALLBACK_MODELS, clampNumber, estimateNeurons, utcDayStartIso } from "../services/ai-review"; import { findUnlinkedIssueCandidates, MAX_CANDIDATES, type CandidateOpenIssue } from "../signals/unlinked-issue-candidates"; import type { UnlinkedIssueGuardrailConfig } from "../types"; import { DIFF_CHAR_BUDGET, MAX_TOKENS, verifyUnlinkedIssueMatch } from "./unlinked-issue-match"; @@ -62,6 +63,12 @@ const VERIFY_PROMPT_OVERHEAD_CHAR_ESTIMATE = 2_000; // verifyUnlinkedIssueMatch tries a primary model and, ONLY on a thrown error, a fallback -- two calls is the // real worst case per candidate, not the common case, but this budget check must size for the worst case. const VERIFY_MAX_MODEL_ATTEMPTS_PER_CANDIDATE = 2; +// Review feedback on #4551: the budget check above reads sumAiEstimatedNeuronsSince, but without a writer +// this feature's own real AI spend never contributed to that counter -- a free rider that respects every +// OTHER feature's usage but never counts its own, so the true aggregate spend could silently exceed +// AI_DAILY_NEURON_BUDGET by however much this feature actually used. recordUnlinkedIssueVerifyUsage (below) +// closes that gap by recording into the SAME shared ai_usage_events table this check reads from. +const UNLINKED_ISSUE_VERIFY_USAGE_FEATURE = "unlinked_issue_verify"; /** Has this actor already run the AI verifier at or beyond the rate ceiling in the last window, across every * repo/PR? Fail-safe: a read error resolves to "not rate-limited," so the pre-#4515 unconditional- @@ -94,6 +101,25 @@ async function isUnlinkedIssueVerifyBudgetExceeded(env: Env, candidateCount: num return estimatedNeurons > remainingBudget; } +/** Record ONE candidate's actual AI spend into the shared `ai_usage_events` ledger -- the SAME table + * {@link isUnlinkedIssueVerifyBudgetExceeded} sums from, so this feature's own usage counts against the + * budget it itself enforces on others (see the module-level comment on {@link UNLINKED_ISSUE_VERIFY_USAGE_FEATURE}). + * Records the same worst-case per-candidate estimate the budget check itself uses -- a deliberate over-count + * (verifyUnlinkedIssueMatch's common case is ONE model call, not the two this sizes for), not a precise + * post-hoc token read, mirroring ai-slop.ts's own pre-computed-estimate recording convention. Best-effort: a + * write failure is swallowed (telemetry must never block the gate). */ +async function recordUnlinkedIssueVerifyUsage(env: Env, repoFullName: string, pullNumber: number): Promise { + const estimatedNeurons = estimateNeurons(DIFF_CHAR_BUDGET + VERIFY_PROMPT_OVERHEAD_CHAR_ESTIMATE, MAX_TOKENS, VERIFY_MAX_MODEL_ATTEMPTS_PER_CANDIDATE); + await recordAiUsageEvent(env, { + feature: UNLINKED_ISSUE_VERIFY_USAGE_FEATURE, + route: "github_app.unlinked_issue_verify", + model: [BEST_REVIEW_MODELS[0], RELIABLE_FALLBACK_MODELS[0]].join("+"), + status: "ok", + estimatedNeurons, + detail: `unlinked-issue-match verification for ${repoFullName}#${pullNumber}`, + }).catch(() => undefined); +} + /** Minimal cached miner-identity check, deliberately independent of processors.ts's getCachedOfficialMinerDetection * (same cache table and TTLs, no audit-log side effect -- this call site doesn't need one). Fail-safe: any * lookup failure resolves to "not a confirmed miner," never the reverse. */ @@ -202,6 +228,9 @@ export async function resolveUnlinkedIssueMatchDisposition(env: Env, input: Reso if (await isUnlinkedIssueVerifyBudgetExceeded(env, candidates.length)) return undefined; for (const candidate of candidates) { if (authorLogin) await recordUnlinkedIssueVerifyAttempt(env, input.repoFullName, input.pullNumber, authorLogin); + // Record spend regardless of authorLogin -- an AI call happens either way; only the PER-ACTOR rate + // ceiling above needs a known actor, this shared-budget accounting does not. + await recordUnlinkedIssueVerifyUsage(env, input.repoFullName, input.pullNumber); const verdict = await verifyUnlinkedIssueMatch(env, { prTitle: input.prTitle, prBody: input.prBody, diff --git a/test/unit/unlinked-issue-guardrail.test.ts b/test/unit/unlinked-issue-guardrail.test.ts index 128b3c7ea4..c2b967f650 100644 --- a/test/unit/unlinked-issue-guardrail.test.ts +++ b/test/unit/unlinked-issue-guardrail.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it, vi } from "vitest"; import { createTestEnv } from "../helpers/d1"; -import { countRecentAuditEventsForActor, recordAuditEvent, upsertIssueFromGitHub, hasRecentAuditEvent } from "../../src/db/repositories"; +import { countRecentAuditEventsForActor, recordAuditEvent, sumAiEstimatedNeuronsSince, upsertIssueFromGitHub, hasRecentAuditEvent } from "../../src/db/repositories"; import { resolveUnlinkedIssueMatchDisposition, UNLINKED_ISSUE_MATCH_AUDIT_EVENT_TYPE, @@ -425,6 +425,44 @@ describe("resolveUnlinkedIssueMatchDisposition", () => { expect(await countRecentAuditEventsForActor(env, "contributor-a", UNLINKED_ISSUE_VERIFY_ATTEMPT_AUDIT_EVENT_TYPE, "2000-01-01T00:00:00.000Z")).toBe(1); }); + it("records this candidate's actual spend into the SAME shared ai_usage_events counter the budget check reads (review feedback on #4551)", async () => { + const run = vi.fn(async () => ({ response: JSON.stringify(aiVerdict()) })); + const env = createTestEnv({ AI: { run } as unknown as Ai }); + await seedIssue(env, 7, "webhook retry duplicate bug", "retries duplicate events under load, needs a dedup key"); + + const usedBefore = await sumAiEstimatedNeuronsSince(env, "2000-01-01T00:00:00.000Z"); + expect(usedBefore).toBe(0); + + await resolveUnlinkedIssueMatchDisposition(env, { ...BASE_INPUT, config: config() }); + + const usedAfter = await sumAiEstimatedNeuronsSince(env, "2000-01-01T00:00:00.000Z"); + expect(usedAfter).toBeGreaterThan(0); + }); + + it("records spend even when the author login is unknown (unlike the rate ceiling, this does not need a known actor)", async () => { + const run = vi.fn(async () => ({ response: JSON.stringify(aiVerdict()) })); + const env = createTestEnv({ AI: { run } as unknown as Ai }); + await seedIssue(env, 7, "webhook retry duplicate bug", "retries duplicate events under load, needs a dedup key"); + + await resolveUnlinkedIssueMatchDisposition(env, { ...BASE_INPUT, prAuthorLogin: null, config: config() }); + + expect(await sumAiEstimatedNeuronsSince(env, "2000-01-01T00:00:00.000Z")).toBeGreaterThan(0); + }); + + it("swallows a usage-recording write failure without affecting the verification result", async () => { + const run = vi.fn(async () => ({ response: JSON.stringify(aiVerdict()) })); + const env = createTestEnv({ AI: { run } as unknown as Ai }); + await seedIssue(env, 7, "webhook retry duplicate bug", "retries duplicate events under load, needs a dedup key"); + const realPrepare = env.DB.prepare.bind(env.DB); + env.DB.prepare = ((sql: string) => { + if (/INSERT INTO.*ai_usage_events/i.test(sql)) throw new Error("d1 down"); + return realPrepare(sql); + }) as typeof env.DB.prepare; + + const result = await resolveUnlinkedIssueMatchDisposition(env, { ...BASE_INPUT, config: config() }); + expect(result?.kind).toBe("hold"); + }); + it("fails open (verification still runs) when the rate-ceiling read itself errors", async () => { const run = vi.fn(async () => ({ response: JSON.stringify(aiVerdict()) })); const env = createTestEnv({ AI: { run } as unknown as Ai });