diff --git a/src/queue/processors.ts b/src/queue/processors.ts index eb109934d0..1d7d4dbaa4 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -8513,9 +8513,17 @@ export async function runLinkedIssueSatisfactionForAdvisory( files: Awaited>; confirmedContributor: boolean; installationId: number; + /** #9491: the same per-PR commit cap its two siblings already honor -- `ai_slop` (slop-detection.ts's + * `commitThresholdReached`) and `ai_review` (focus-manifest's auto_pause_after_reviewed_commits). This + * advisory was the one paid LLM call in the family with NO cap, so a long-lived PR kept paying for a + * fresh assessment on every push after the other two had stopped. */ + commitThresholdReached: boolean; }, ): Promise<{ status: "addressed" | "partial" | "unaddressed"; rationale: string } | null> { if (args.mode === "paused" || !args.confirmedContributor || !args.advisory.headSha) return null; + // #9491: honor the cap the siblings honor. Deliberately no reuse-for-display fallback, matching + // slop-detection's identical early return -- past the cap this feature simply stops spending. + if (args.commitThresholdReached) return null; const primaryIssueNumber = args.pr.linkedIssues[0]; if (primaryIssueNumber === undefined) return null; try { @@ -10661,6 +10669,15 @@ async function maybePublishPrPublicSurface( files: await getReviewFiles(), confirmedContributor, installationId, + // #9491: the same threshold its ai_slop sibling applies. countPublishedAiReviewHeads is the shared + // per-PR reviewed-commit counter both features key on; failing open to 0 (never capped) matches the + // sibling's own .catch(() => 0) rather than silently suppressing the advisory on a read error. + commitThresholdReached: isAutoReviewCommitThresholdReached( + autoReviewConfig, + /* v8 ignore next -- fail-open: a counter read error must not silently SUPPRESS the advisory, so it + degrades to 0 (never capped), matching the ai_slop sibling's identical .catch(() => 0). */ + await countPublishedAiReviewHeads(env, repoFullName, pr.number).catch(() => 0), + ), }); } } diff --git a/src/review/decision-record.ts b/src/review/decision-record.ts index 8587314869..f1e9897452 100644 --- a/src/review/decision-record.ts +++ b/src/review/decision-record.ts @@ -344,13 +344,19 @@ export async function ledgerRowHash(prevHash: string, fields: LedgerRowFields): * only needs "what is the tip right now" (the scheduled anchoring job, #9274) shouldn't pay for a * self-consistency walk it isn't asking for. */ export async function loadDecisionLedgerTip(env: Env): Promise<{ seq: number; rowHash: string; totalCount: number }> { - const [totalRow, tipRow] = await Promise.all([ - env.DB.prepare("SELECT COUNT(*) AS n FROM decision_ledger").first<{ n: number }>(), - env.DB.prepare("SELECT seq, row_hash AS rowHash FROM decision_ledger ORDER BY seq DESC LIMIT 1").first<{ seq: number; rowHash: string }>(), - ]); - /* v8 ignore next -- defensive: a bare COUNT(*) always returns exactly one row, mirroring - * verifyDecisionLedger's identical note. */ - return { seq: tipRow?.seq ?? 0, rowHash: tipRow?.rowHash ?? LEDGER_GENESIS_HASH, totalCount: totalRow?.n ?? 0 }; + // #9489: ONE statement, not two under Promise.all. The seq/totalCount pair is exactly how a verifier detects + // truncation-and-rechaining (see buildLedgerAnchorPayload's own doc), so reading them separately meant a + // concurrent appendDecisionLedger landing between the two queries produced totalCount = seq + 1 alongside + // the OLD rowHash -- an internally inconsistent checkpoint. Anchoring then signs it and publishes it to + // Rekor, unretractably, as a FALSE tamper signal about the maintainer's own ledger. A single statement reads + // both from one snapshot, so the pair can never disagree. + const row = await env.DB + .prepare("SELECT (SELECT COUNT(*) FROM decision_ledger) AS n, seq, row_hash AS rowHash FROM decision_ledger ORDER BY seq DESC LIMIT 1") + .first<{ n: number; seq: number; rowHash: string }>(); + // An empty ledger returns no row at all (the ORDER BY ... LIMIT 1 has nothing to return), which is the + // genesis case rather than a defensive one. + if (!row) return { seq: 0, rowHash: LEDGER_GENESIS_HASH, totalCount: 0 }; + return { seq: row.seq, rowHash: row.rowHash, totalCount: row.n }; } export async function appendDecisionLedger(env: Env, recordId: string, recordDigest: string, attempts = 3): Promise { diff --git a/src/review/ledger-anchor-git.ts b/src/review/ledger-anchor-git.ts index 458cff2f38..5bb6207b24 100644 --- a/src/review/ledger-anchor-git.ts +++ b/src/review/ledger-anchor-git.ts @@ -64,7 +64,20 @@ export async function submitToGitAnchor(env: Env, signed: SignedLedgerAnchor, oc let existingContent = ""; try { const response = await octokit.request("GET /repos/{owner}/{repo}/contents/{path}", { owner, repo, path, ref: branch }); - const data = response.data as { content?: string; sha?: string }; + const data = response.data as { content?: string; sha?: string; size?: number; encoding?: string }; + // #9489: for a file between 1 MB and 100 MB the Contents API returns `content: ""` with + // `encoding: "none"` -- and `typeof "" === "string"`, so the old code accepted it as the file's real + // contents and the PUT below REWROTE the whole log to a single line. At ~300 bytes per line and an + // hourly cadence that lands roughly 4-5 months out. Git history would still hold the truncated commits, + // but the file a skeptic is told to read shrinks -- indistinguishable from the tampering this module's + // own header teaches them to look for. Refuse rather than silently truncate: an operator rotating the + // file is a deliberate act, and an unanchored tip is loudly visible on the public attempt log (#9271). + const bodyElided = data.encoding === "none" || (data.content === "" && typeof data.size === "number" && data.size > 0); + if (bodyElided) { + throw new Error( + `anchor log ${path} is too large for the Contents API (size=${data.size ?? "unknown"} bytes); rotate the file or switch to the Git Data API before anchoring can resume`, + ); + } if (typeof data.content === "string") existingContent = decodeBase64(data.content); existingSha = data.sha; } catch (error) { diff --git a/src/review/ledger-anchor-persistence.ts b/src/review/ledger-anchor-persistence.ts index 35f4bc0157..544337ca2b 100644 --- a/src/review/ledger-anchor-persistence.ts +++ b/src/review/ledger-anchor-persistence.ts @@ -165,3 +165,28 @@ export async function loadLastLedgerAnchorAttempt(env: Env): Promise<{ seq: numb }>(); return row == null ? null : row; } + +/** + * #9489: does the CURRENT tip still lack a successful anchor? + * + * {@link loadLastLedgerAnchorAttempt} deliberately returns the newest attempt regardless of status, so the + * scheduler advances to newer tips rather than hammering a stale checkpoint. But that made a failed attempt at + * a QUIET tip unrecoverable: Rekor 429s at seq N, the ledger goes quiet (a weekend), every hourly tick then + * sees `tipUnchanged` and returns "unchanged" -- so the tip carries NO valid external anchor indefinitely, + * which is precisely the unanchored window the feature exists to bound. + * + * It is also backend-blind: git succeeding at seq N masked rekor failing at the same seq, because the newest + * attempt row won regardless of which backend wrote it. Asking per-backend "is there an OK row for this exact + * rowHash" answers both. + */ +export async function anchorBackendsMissingForRowHash(env: Env, rowHash: string, backends: readonly string[]): Promise { + if (backends.length === 0) return []; + const placeholders = backends.map((_, index) => `?${index + 2}`).join(", "); + const { results } = await env.DB.prepare( + `SELECT DISTINCT backend FROM decision_ledger_anchors WHERE row_hash = ?1 AND status = 'ok' AND backend IN (${placeholders})`, + ) + .bind(rowHash, ...backends) + .all<{ backend: string }>(); + const anchored = new Set(results.map((row) => row.backend)); + return backends.filter((backend) => !anchored.has(backend)); +} diff --git a/src/review/ledger-anchor-scheduler.ts b/src/review/ledger-anchor-scheduler.ts index aeee7c247b..25ca625d67 100644 --- a/src/review/ledger-anchor-scheduler.ts +++ b/src/review/ledger-anchor-scheduler.ts @@ -5,7 +5,7 @@ import { errorMessage, nowIso } from "../utils/json"; import { buildLedgerAnchorPayload, currentAnchorKey, parseAnchorPublicKeys, signLedgerAnchorPayload, type SignedLedgerAnchor } from "./ledger-anchor"; import { loadDecisionLedgerTip } from "./decision-record"; -import { loadLastLedgerAnchorAttempt, recordLedgerAnchorAttempt, type LedgerAnchorBackend } from "./ledger-anchor-persistence"; +import { anchorBackendsMissingForRowHash, loadLastLedgerAnchorAttempt, recordLedgerAnchorAttempt, type LedgerAnchorBackend } from "./ledger-anchor-persistence"; import { submitToRekor } from "./ledger-anchor-rekor"; import type { LedgerAnchorGitTarget } from "./ledger-anchor-git"; @@ -15,7 +15,7 @@ export const LEDGER_ANCHOR_SEQ_THRESHOLD = 256; export type LedgerAnchorScheduleDecision = | { shouldAnchor: false; reason: "empty_ledger" | "unchanged" } - | { shouldAnchor: true; reason: "hourly" | "seq_threshold" }; + | { shouldAnchor: true; reason: "hourly" | "seq_threshold" | "retry_unanchored" }; /** * Pure scheduling decision: given the live tip and the last attempt (regardless of that attempt's own @@ -30,6 +30,9 @@ export function decideLedgerAnchorSchedule(input: { currentTip: { seq: number; rowHash: string }; lastAnchor: { seq: number; rowHash: string } | null; seqThreshold: number; + /** #9489: backends with NO successful anchor for the current tip's rowHash. Non-empty means the tip is + * unanchored on at least one backend even though nothing has moved, so an hourly tick should retry. */ + unanchoredBackends?: readonly string[] | undefined; }): LedgerAnchorScheduleDecision { if (input.currentTip.seq === 0) return { shouldAnchor: false, reason: "empty_ledger" }; @@ -38,6 +41,13 @@ export function decideLedgerAnchorSchedule(input: { const tipUnchanged = input.lastAnchor !== null && input.lastAnchor.rowHash === input.currentTip.rowHash; if (input.isHourly && !tipUnchanged) return { shouldAnchor: true, reason: "hourly" }; + // #9489: an unchanged tip is only "nothing to do" if it is actually ANCHORED. The previous check compared + // against the newest ATTEMPT regardless of status, so a failure at a quiet tip was never retried -- Rekor + // 429s at seq N, the ledger goes quiet, and every later tick reports "unchanged" while the tip carries no + // valid external anchor at all. Retrying on the hourly tick gives it a bounded, self-healing cadence. + if (input.isHourly && tipUnchanged && input.unanchoredBackends && input.unanchoredBackends.length > 0) { + return { shouldAnchor: true, reason: "retry_unanchored" }; + } return { shouldAnchor: false, reason: "unchanged" }; } @@ -75,10 +85,27 @@ export type LedgerAnchorSchedulerDeps = { * a total anchoring failure cannot propagate anywhere. Returns the scheduling decision so a caller/test can * observe WHY nothing happened, without needing a second query. */ +/** #9489: the backends whose success actually matters for "is this tip anchored". `ots` is tracked-but-not- + * built (#9267), so requiring it would make every tip permanently unanchored. */ +const ANCHOR_BACKENDS_REQUIRING_SUCCESS = ["rekor", "git"] as const; + export async function runScheduledLedgerAnchor(env: Env, options: { isHourly: boolean; now?: string }, deps: LedgerAnchorSchedulerDeps = {}): Promise { const now = options.now ?? nowIso(); const [tip, lastAnchor] = await Promise.all([loadDecisionLedgerTip(env), loadLastLedgerAnchorAttempt(env)]); - const decision = decideLedgerAnchorSchedule({ isHourly: options.isHourly, currentTip: tip, lastAnchor, seqThreshold: LEDGER_ANCHOR_SEQ_THRESHOLD }); + // #9489: which backends still have NO successful anchor for this exact tip. An unchanged tip is only + // "nothing to do" when it is genuinely anchored -- otherwise a failure at a quiet tip is never retried, and + // a success on one backend masks a failure on the other, since the newest-attempt row wins regardless of + // which backend wrote it. + /* v8 ignore next -- fail-open: an unreadable anchors table degrades to "nothing known to be missing", i.e. + exactly the pre-#9489 scheduling behaviour, rather than forcing an anchor on every tick. */ + const unanchoredBackends = await anchorBackendsMissingForRowHash(env, tip.rowHash, ANCHOR_BACKENDS_REQUIRING_SUCCESS).catch(() => []); + const decision = decideLedgerAnchorSchedule({ + isHourly: options.isHourly, + currentTip: tip, + lastAnchor, + seqThreshold: LEDGER_ANCHOR_SEQ_THRESHOLD, + unanchoredBackends, + }); if (!decision.shouldAnchor) return decision; const keys = parseAnchorPublicKeys(env.LOOPOVER_LEDGER_ANCHOR_KEYS); diff --git a/test/unit/ledger-anchor-git.test.ts b/test/unit/ledger-anchor-git.test.ts index 78cff33c50..b99c07b533 100644 --- a/test/unit/ledger-anchor-git.test.ts +++ b/test/unit/ledger-anchor-git.test.ts @@ -144,3 +144,100 @@ describe("submitToGitAnchor (#9273)", () => { await expect(submitToGitAnchor(env, signed, { request } as GitHubContentsRequester, TARGET)).resolves.toBeUndefined(); }); }); + +// #9489: for a file between 1 MB and 100 MB the Contents API returns `content: ""` with `encoding: "none"`. +// Because `typeof "" === "string"`, the old code accepted that as the file's real contents and the PUT +// REWROTE the whole anchor log to a single line -- reporting status "ok" while destroying it. At ~300 bytes +// per line on an hourly cadence that lands roughly 4-5 months out. Git history keeps the truncated commits, +// but the file a skeptic is told to read shrinks, which is indistinguishable from the tampering this module's +// own header teaches them to look for. +describe("oversized anchor log (#9489)", () => { + it("REGRESSION: refuses to write when the Contents API elides the body, instead of truncating the log", async () => { + const env = createTestEnv(); + const signed = makeSignedAnchor(1); + const octokit = { + request: vi.fn(async (route: string) => { + // No `size` here: GitHub does not always include it, and the error message must still read cleanly. + if (route.startsWith("GET")) return { data: { content: "", encoding: "none", sha: "abc" } }; + return { data: {} }; + }), + }; + + // submitToGitAnchor never rejects by contract -- it records the attempt instead -- so the refusal shows up + // as a FAILED anchor rather than a thrown error. Both properties matter: + await expect(submitToGitAnchor(env, signed, octokit as unknown as GitHubContentsRequester, TARGET)).resolves.toBeUndefined(); + // 1. no PUT was attempted, so the existing log is untouched rather than rewritten to a single line; + expect(octokit.request.mock.calls.some(([route]) => String(route).startsWith("PUT"))).toBe(false); + // 2. the failure is visible on the public attempt log, so the unanchored tip is loud rather than silent. + const { anchors } = await loadPublicLedgerAnchors(env); + expect(anchors[0]).toMatchObject({ status: "failed" }); + expect(String((anchors[0] as { error?: string }).error)).toMatch(/too large for the Contents API/); + expect(String((anchors[0] as { error?: string }).error)).toContain("size=unknown"); // degrades cleanly + }); + + it("INVARIANT: an ordinary small file still appends normally", async () => { + const env = createTestEnv(); + const signed = makeSignedAnchor(1); + const octokit = { + request: vi.fn(async (route: string, _params?: Record) => { + if (route.startsWith("GET")) return { data: { content: Buffer.from("existing line\n").toString("base64"), encoding: "base64", size: 14, sha: "abc" } }; + return { data: {} }; + }), + }; + + await submitToGitAnchor(env, signed, octokit as unknown as GitHubContentsRequester, TARGET); + + const put = octokit.request.mock.calls.find((call) => String(call[0]).startsWith("PUT")); + expect(put).toBeDefined(); + const written = Buffer.from(String((put?.[1] as Record | undefined)?.["content"] ?? ""), "base64").toString("utf8"); + expect(written).toContain("existing line"); // appended, not replaced + }); + + it("also refuses on the size-only shape, where the body is elided without encoding:none", async () => { + // GitHub has returned both shapes for an over-threshold file; keying on either alone would miss one. + const env = createTestEnv(); + const signed = makeSignedAnchor(1); + const octokit = { + request: vi.fn(async (route: string, _params?: Record) => { + if (route.startsWith("GET")) return { data: { content: "", size: 1_500_000, sha: "abc" } }; + return { data: {} }; + }), + }; + + await submitToGitAnchor(env, signed, octokit as unknown as GitHubContentsRequester, TARGET); + + expect(octokit.request.mock.calls.some((call) => String(call[0]).startsWith("PUT"))).toBe(false); + }); + + it("INVARIANT: an empty body with NO size field is treated as empty, not elided", async () => { + // Without a size there is no evidence the body was withheld, so refusing would block the very first anchor + // on a freshly created empty file. Appending is the safe reading. + const env = createTestEnv(); + const signed = makeSignedAnchor(1); + const octokit = { + request: vi.fn(async (route: string, _params?: Record) => { + if (route.startsWith("GET")) return { data: { content: "", sha: "abc" } }; + return { data: {} }; + }), + }; + + await submitToGitAnchor(env, signed, octokit as unknown as GitHubContentsRequester, TARGET); + + expect(octokit.request.mock.calls.some((call) => String(call[0]).startsWith("PUT"))).toBe(true); + }); + + it("INVARIANT: a genuinely empty (0-byte) file is not mistaken for an elided one", async () => { + // size 0 with content "" is a real empty file -- the first anchor should append to it, not refuse. + const env = createTestEnv(); + const signed = makeSignedAnchor(1); + const octokit = { + request: vi.fn(async (route: string) => { + if (route.startsWith("GET")) return { data: { content: "", encoding: "base64", size: 0, sha: "abc" } }; + return { data: {} }; + }), + }; + + await expect(submitToGitAnchor(env, signed, octokit as unknown as GitHubContentsRequester, TARGET)).resolves.toBeUndefined(); + expect(octokit.request.mock.calls.some(([route]) => String(route).startsWith("PUT"))).toBe(true); + }); +}); diff --git a/test/unit/ledger-anchor-persistence.test.ts b/test/unit/ledger-anchor-persistence.test.ts index 8ec85be3d4..a8884b058c 100644 --- a/test/unit/ledger-anchor-persistence.test.ts +++ b/test/unit/ledger-anchor-persistence.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import { createTestEnv } from "../helpers/d1"; -import { loadLastLedgerAnchorAttempt, loadPublicLedgerAnchors, recordLedgerAnchorAttempt, type LedgerAnchorAttemptInput } from "../../src/review/ledger-anchor-persistence"; +import { loadLastLedgerAnchorAttempt, loadPublicLedgerAnchors, recordLedgerAnchorAttempt, type LedgerAnchorAttemptInput , anchorBackendsMissingForRowHash } from "../../src/review/ledger-anchor-persistence"; import { buildLedgerAnchorPayload } from "../../src/review/ledger-anchor"; // #9271 (epic #9267). The load-bearing property here is that a FAILURE is recorded and served exactly like a @@ -151,3 +151,54 @@ describe("loadLastLedgerAnchorAttempt (#9274)", () => { expect(await loadLastLedgerAnchorAttempt(env)).toEqual({ seq: 2, rowHash: "b".repeat(64) }); }); }); + +// #9489: "is this tip anchored" must be asked PER BACKEND against the exact rowHash. Asking only "what was the +// newest attempt" meant a failure at a quiet tip was never retried, and one backend's success masked another's +// failure because the newest row won regardless of which backend wrote it. +describe("anchorBackendsMissingForRowHash (#9489)", () => { + const record = (env: Env, rowHash: string, backend: string, status: "ok" | "failed") => + env.DB.prepare( + "INSERT INTO decision_ledger_anchors (id, seq, row_hash, payload_json, signature, key_id, backend, status, created_at) VALUES (?, 1, ?, '{}', 'sig', 'k1', ?, ?, ?)", + ) + .bind(`${backend}-${status}-${rowHash}-${Math.random()}`, rowHash, backend, status, new Date().toISOString()) + .run(); + + it("reports a backend with NO row at all as missing", async () => { + const env = createTestEnv(); + expect(await anchorBackendsMissingForRowHash(env, "hash-a", ["rekor", "git"])).toEqual(["rekor", "git"]); + }); + + it("REGRESSION: a FAILED attempt does not count as anchored -- otherwise it is never retried", async () => { + const env = createTestEnv(); + await record(env, "hash-a", "rekor", "failed"); + expect(await anchorBackendsMissingForRowHash(env, "hash-a", ["rekor"])).toEqual(["rekor"]); + }); + + it("REGRESSION: one backend's success does not mask another's failure", async () => { + const env = createTestEnv(); + await record(env, "hash-a", "git", "ok"); + await record(env, "hash-a", "rekor", "failed"); + expect(await anchorBackendsMissingForRowHash(env, "hash-a", ["rekor", "git"])).toEqual(["rekor"]); + }); + + it("INVARIANT: a fully anchored tip reports nothing missing", async () => { + const env = createTestEnv(); + await record(env, "hash-a", "git", "ok"); + await record(env, "hash-a", "rekor", "ok"); + expect(await anchorBackendsMissingForRowHash(env, "hash-a", ["rekor", "git"])).toEqual([]); + }); + + it("INVARIANT: a success at a DIFFERENT rowHash does not anchor this tip", async () => { + // The whole point is per-tip, not per-ledger: an older anchored checkpoint says nothing about the tip. + const env = createTestEnv(); + await record(env, "hash-old", "rekor", "ok"); + expect(await anchorBackendsMissingForRowHash(env, "hash-new", ["rekor"])).toEqual(["rekor"]); + }); + + it("short-circuits on an empty backend list without querying", async () => { + const env = createTestEnv(); + const spy = vi.spyOn(env.DB, "prepare"); + expect(await anchorBackendsMissingForRowHash(env, "hash-a", [])).toEqual([]); + expect(spy).not.toHaveBeenCalled(); + }); +}); diff --git a/test/unit/ledger-anchor-scheduler.test.ts b/test/unit/ledger-anchor-scheduler.test.ts index 6d165c4982..f64de3af9b 100644 --- a/test/unit/ledger-anchor-scheduler.test.ts +++ b/test/unit/ledger-anchor-scheduler.test.ts @@ -209,3 +209,51 @@ describe("runScheduledLedgerAnchor (#9274)", () => { expect(anchors[0]).toMatchObject({ status: "ok" }); }); }); + +// #9489: loadLastLedgerAnchorAttempt deliberately returns the newest attempt REGARDLESS of status, so the +// scheduler advances to newer tips rather than hammering a stale checkpoint. But that made a FAILURE at a +// QUIET tip unrecoverable: Rekor 429s at seq N, the ledger goes quiet (a weekend), and every hourly tick then +// sees tipUnchanged and returns "unchanged" -- so the tip carries NO valid external anchor indefinitely, which +// is exactly the unanchored window this feature exists to bound. It was also backend-blind: git succeeding at +// seq N masked rekor failing at the same seq, because the newest row won regardless of which backend wrote it. +describe("retrying an unanchored tip (#9489)", () => { + const tip = { seq: 5, rowHash: "hash-5" }; + const sameTipAnchor = { seq: 5, rowHash: "hash-5" }; + + it("REGRESSION: an hourly tick RETRIES an unchanged tip that no backend has successfully anchored", () => { + expect( + decideLedgerAnchorSchedule({ isHourly: true, currentTip: tip, lastAnchor: sameTipAnchor, seqThreshold: 100, unanchoredBackends: ["rekor"] }), + ).toEqual({ shouldAnchor: true, reason: "retry_unanchored" }); + }); + + it("REGRESSION: a PARTIALLY anchored tip still retries, so one backend's success cannot mask another's failure", () => { + expect( + decideLedgerAnchorSchedule({ isHourly: true, currentTip: tip, lastAnchor: sameTipAnchor, seqThreshold: 100, unanchoredBackends: ["git"] }).shouldAnchor, + ).toBe(true); + }); + + it("INVARIANT: a fully anchored unchanged tip stays quiet -- no re-anchoring churn", () => { + expect( + decideLedgerAnchorSchedule({ isHourly: true, currentTip: tip, lastAnchor: sameTipAnchor, seqThreshold: 100, unanchoredBackends: [] }), + ).toEqual({ shouldAnchor: false, reason: "unchanged" }); + }); + + it("INVARIANT: an unanchored tip does NOT force a non-hourly tick to anchor", () => { + // The retry rides the hourly cadence rather than firing on every tick, which is what bounds the backoff. + expect( + decideLedgerAnchorSchedule({ isHourly: false, currentTip: tip, lastAnchor: sameTipAnchor, seqThreshold: 100, unanchoredBackends: ["rekor"] }), + ).toEqual({ shouldAnchor: false, reason: "unchanged" }); + }); + + it("INVARIANT: an empty ledger is still never anchored, whatever the backend state says", () => { + expect( + decideLedgerAnchorSchedule({ isHourly: true, currentTip: { seq: 0, rowHash: "genesis" }, lastAnchor: null, seqThreshold: 100, unanchoredBackends: ["rekor", "git"] }), + ).toEqual({ shouldAnchor: false, reason: "empty_ledger" }); + }); + + it("INVARIANT: omitting unanchoredBackends preserves the pre-#9489 behaviour exactly", () => { + expect( + decideLedgerAnchorSchedule({ isHourly: true, currentTip: tip, lastAnchor: sameTipAnchor, seqThreshold: 100 }), + ).toEqual({ shouldAnchor: false, reason: "unchanged" }); + }); +}); diff --git a/test/unit/linked-issue-satisfaction-run.test.ts b/test/unit/linked-issue-satisfaction-run.test.ts index 527e4da47a..22079a8ae2 100644 --- a/test/unit/linked-issue-satisfaction-run.test.ts +++ b/test/unit/linked-issue-satisfaction-run.test.ts @@ -366,6 +366,36 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" const processorFingerprint = () => linkedIssueSatisfactionCacheInputFingerprint({ byok: false, provider: null, model: null, issueText, prTitle: pr.title, prBody: pr.body, diff: buildAiReviewDiff(files) }); const advisoryMode = { linkedIssueSatisfactionGateMode: "advisory", aiReviewByok: false } as RepositorySettings; + + // #9491: the #9399 sibling-drift class again. This advisory is one of three paid LLM calls in the same family, + // and the other two already stop spending past a per-PR reviewed-commit cap -- ai_slop via slop-detection's + // `commitThresholdReached`, ai_review via the focus manifest's auto_pause_after_reviewed_commits. This one had + // NO cap, so a long-lived PR kept paying for a fresh assessment on every push after its siblings had stopped. + describe("commit-threshold cap parity (#9491)", () => { + it("REGRESSION: stops spending once the per-PR commit threshold is reached", async () => { + const run = vi.fn(); + const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { + mode: "live", + settings: advisoryMode, + advisory: advisory(), + repoFullName: "acme/widgets", + pr, + author: "alice", + files, + confirmedContributor: true, + installationId: 1, + commitThresholdReached: true, + }); + + expect(result).toBeNull(); + expect(run).not.toHaveBeenCalled(); // the whole point: no LLM call is paid for + }); + + // The below-threshold path is covered by every other test in this suite, which all pass + // commitThresholdReached: false and assert the assessment runs -- so the cap does not disable the feature. + + }); + const blockMode = { linkedIssueSatisfactionGateMode: "block", aiReviewByok: false } as RepositorySettings; function stubFetch(handler: (url: string) => Response | Promise): void { @@ -384,7 +414,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" stubIssueFetch(); const run = vi.fn(); const adv = advisory(); - const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "mallory", files, confirmedContributor: false, installationId: 1 }); + const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "mallory", files, confirmedContributor: false, installationId: 1, commitThresholdReached: false }); expect(result).toBeNull(); expect(run).not.toHaveBeenCalled(); expect(adv.findings).toEqual([]); @@ -394,7 +424,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" stubIssueFetch(); const run = vi.fn(); const adv = advisory(); - const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { mode: "paused", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }); + const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { mode: "paused", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); expect(result).toBeNull(); expect(run).not.toHaveBeenCalled(); }); @@ -404,7 +434,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" const noSha = advisory(); delete (noSha as Partial).headSha; const run = vi.fn(); - const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { mode: "live", settings: advisoryMode, advisory: noSha, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }); + const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { mode: "live", settings: advisoryMode, advisory: noSha, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); expect(result).toBeNull(); expect(run).not.toHaveBeenCalled(); }); @@ -414,7 +444,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" const env = enabledEnv(run); vi.stubGlobal("fetch", vi.fn()); const adv = advisory(); - const result = await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr: { ...pr, linkedIssues: [] }, author: "alice", files, confirmedContributor: true, installationId: 1 }); + const result = await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr: { ...pr, linkedIssues: [] }, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); expect(result).toBeNull(); expect(run).not.toHaveBeenCalled(); }); @@ -423,7 +453,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" stubIssueFetch(); const run = vi.fn(async () => ({ response: satisfactionJson({ status: "addressed" }) })); const adv = advisory(); - const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }); + const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); expect(result).toMatchObject({ status: "addressed" }); }); @@ -432,7 +462,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" const run = vi.fn(async () => ({ response: satisfactionJson({ status: "addressed" }) })); const adv = advisory(); const { body: _omit, ...prWithoutBody } = pr; - const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr: prWithoutBody, author: "alice", files, confirmedContributor: true, installationId: 1 }); + const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr: prWithoutBody, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); expect(result).toMatchObject({ status: "addressed" }); expect(run).toHaveBeenCalledTimes(1); }); @@ -441,7 +471,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" stubFetch((url) => (url.includes("/access_tokens") ? Response.json({ token: "t" }) : new Response("missing", { status: 404 }))); const run = vi.fn(); const adv = advisory(); - const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }); + const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); expect(result).toBeNull(); expect(run).not.toHaveBeenCalled(); }); @@ -450,7 +480,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" stubIssueFetch({ title: "", body: "" }); const run = vi.fn(); const adv = advisory(); - const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }); + const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); expect(result).toBeNull(); expect(run).not.toHaveBeenCalled(); }); @@ -460,7 +490,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" const env = { ...enabledEnv(async () => ({ response: satisfactionJson() })), DB: undefined } as unknown as Env; const adv = advisory(); await expect( - runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }), + runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }), ).resolves.toBeNull(); expect(adv.findings).toEqual([]); }); @@ -470,7 +500,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" stubIssueFetch(); const run = vi.fn(async () => ({ response: satisfactionJson({ status: "unaddressed", confidence: 0.9, rationale: "The linked issue asks for an SSE stream; this PR adds an unrelated REST endpoint." }) })); const adv = advisory(); - const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { mode: "live", settings: blockMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }); + const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { mode: "live", settings: blockMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); expect(result).toMatchObject({ status: "unaddressed" }); expect(adv.findings).toHaveLength(1); @@ -485,7 +515,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" stubIssueFetch(); const run = vi.fn(async () => ({ response: satisfactionJson({ status: "unaddressed", confidence: 0.9 }) })); const adv = advisory(); - const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }); + const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); expect(result).toMatchObject({ status: "unaddressed" }); expect(adv.findings).toEqual([]); // advisory mode never restates the gap as a generic finding/Nit @@ -499,7 +529,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" stubIssueFetch(); const run = vi.fn(async () => ({ response: satisfactionJson({ status: "unaddressed", confidence: 0.9 }) })); const env = enabledEnv(run); - await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: blockMode, advisory: advisory(), repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }); + await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: blockMode, advisory: advisory(), repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); const history = await createSignalStore(env).queryRuleHistory("linked_issue_scope_mismatch", 0); expect(history.fired).toHaveLength(1); @@ -519,7 +549,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" const env = enabledEnv(run); const longBody = "x".repeat(MAX_BODY_CHARS + 500); const longBodyPr = { ...pr, body: longBody }; - await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: blockMode, advisory: advisory(), repoFullName: "acme/widgets", pr: longBodyPr, author: "alice", files, confirmedContributor: true, installationId: 1 }); + await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: blockMode, advisory: advisory(), repoFullName: "acme/widgets", pr: longBodyPr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); const history = await createSignalStore(env).queryRuleHistory("linked_issue_scope_mismatch", 0); expect(history.fired).toHaveLength(1); @@ -544,7 +574,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" const modelJson = satisfactionJson({ status: "unaddressed", confidence: 0.9, rationale: "x".repeat(MAX_MODEL_RESPONSE_CHARS + 500) }); const run = vi.fn(async () => ({ response: modelJson })); const env = enabledEnv(run); - await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: blockMode, advisory: advisory(), repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }); + await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: blockMode, advisory: advisory(), repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); const history = await createSignalStore(env).queryRuleHistory("linked_issue_scope_mismatch", 0); const metadata = history.fired[0]?.metadata as Record; @@ -557,7 +587,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" const run = vi.fn(async () => ({ response: satisfactionJson({ status: "unaddressed", confidence: 0.9 }) })); const env = enabledEnv(run); const bodylessPr = { ...pr, body: null as unknown as string }; - await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: blockMode, advisory: advisory(), repoFullName: "acme/widgets", pr: bodylessPr, author: "alice", files, confirmedContributor: true, installationId: 1 }); + await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: blockMode, advisory: advisory(), repoFullName: "acme/widgets", pr: bodylessPr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); const history = await createSignalStore(env).queryRuleHistory("linked_issue_scope_mismatch", 0); expect((history.fired[0]?.metadata as Record).prBody).toBe(""); @@ -567,7 +597,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" stubIssueFetch(); const run = vi.fn(async () => ({ response: satisfactionJson({ status: "unaddressed", confidence: 0.9 }) })); const env = enabledEnv(run); - await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: advisoryMode, advisory: advisory(), repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }); + await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: advisoryMode, advisory: advisory(), repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); expect((await createSignalStore(env).queryRuleHistory("linked_issue_scope_mismatch", 0)).fired).toEqual([]); }); @@ -576,7 +606,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" stubIssueFetch(); const run = vi.fn(async () => ({ response: satisfactionJson({ status }) })); const env = enabledEnv(run); - await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: blockMode, advisory: advisory(), repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }); + await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: blockMode, advisory: advisory(), repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); expect((await createSignalStore(env).queryRuleHistory("linked_issue_scope_mismatch", 0)).fired).toEqual([]); } }); @@ -592,7 +622,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" }); const run = vi.fn(async () => ({ response: satisfactionJson({ status: "unaddressed", confidence: 0.9 }) })); const adv = advisory(); - const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { mode: "live", settings: blockMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }); + const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { mode: "live", settings: blockMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); expect(result).toMatchObject({ status: "unaddressed" }); // normal return value unaffected expect(adv.findings).toHaveLength(1); // the blocker still lands }); @@ -601,7 +631,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" stubIssueFetch(); const run = vi.fn(async () => ({ response: satisfactionJson({ status: "partial" }) })); const adv = advisory(); - const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { mode: "live", settings: blockMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }); + const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { mode: "live", settings: blockMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); expect(result).toMatchObject({ status: "partial" }); expect(adv.findings).toEqual([]); const gate = evaluateGateCheck(adv, { linkedIssueSatisfactionGateMode: "block" }); @@ -612,7 +642,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" stubIssueFetch(); const run = vi.fn(async () => ({ response: satisfactionJson({ status: "unaddressed", confidence: 0.1 }) })); const adv = advisory(); - const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { mode: "live", settings: blockMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }); + const result = await runLinkedIssueSatisfactionForAdvisory(enabledEnv(run), { mode: "live", settings: blockMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); expect(result).toBeNull(); expect(adv.findings).toEqual([]); const gate = evaluateGateCheck(adv, { linkedIssueSatisfactionGateMode: "block" }); @@ -638,7 +668,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" return new Response("not found", { status: 404 }); }); const adv = advisory(); - const result = await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: { linkedIssueSatisfactionGateMode: "advisory", aiReviewByok: true } as RepositorySettings, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }); + const result = await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: { linkedIssueSatisfactionGateMode: "advisory", aiReviewByok: true } as RepositorySettings, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); expect(result).toMatchObject({ status: "addressed" }); expect(workersRun).not.toHaveBeenCalled(); }); @@ -670,6 +700,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" files, confirmedContributor: true, installationId: 1, + commitThresholdReached: false, }); expect(result).toMatchObject({ status: "addressed" }); expect(workersRun).not.toHaveBeenCalled(); @@ -704,6 +735,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" files, confirmedContributor: true, installationId: 1, + commitThresholdReached: false, }); expect(result).toMatchObject({ status: "partial" }); expect(workersRun).toHaveBeenCalled(); @@ -723,7 +755,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" estimatedNeurons: 12, }); const adv = advisory(); - const result = await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }); + const result = await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); expect(run).not.toHaveBeenCalled(); expect(result).toEqual({ status: "addressed", rationale: "cached: looks done" }); }); @@ -741,7 +773,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" const repositoriesModule = await import("../../src/db/repositories"); const auditSpy = vi.spyOn(repositoriesModule, "recordAuditEvent").mockRejectedValueOnce(new Error("D1 audit write error")); const adv = advisory(); - const result = await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }); + const result = await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); expect(run).not.toHaveBeenCalled(); // still a cache hit despite the audit-write failure expect(result).toEqual({ status: "addressed", rationale: "cached: looks done" }); auditSpy.mockRestore(); @@ -759,7 +791,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" }); const adv = advisory(); await expect( - runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }), + runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }), ).resolves.toMatchObject({ status: "addressed" }); // never throws, even with both the cache write AND its own audit write failing writeSpy.mockRestore(); auditSpy.mockRestore(); @@ -770,7 +802,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" const run = vi.fn(async () => ({ response: satisfactionJson({ status: "addressed" }) })); const env = enabledEnv(run); const adv = advisory(); - await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }); + await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); expect(run).toHaveBeenCalledTimes(1); const fingerprint = await processorFingerprint(); @@ -778,7 +810,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" expect(cached).toMatchObject({ status: "ok", result: { status: "addressed" } }); const adv2 = advisory(); - await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: advisoryMode, advisory: adv2, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }); + await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: advisoryMode, advisory: adv2, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); expect(run).toHaveBeenCalledTimes(1); // still 1 — second pass was a cache hit }); @@ -788,7 +820,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" const env = enabledEnv(run); const adv = advisory(); await expect( - runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: blockMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }), + runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: blockMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }), ).resolves.toMatchObject({ status: "addressed" }); stubIssueFetch({ body: "We now need a GraphQL subscription instead of an SSE stream." }); @@ -796,7 +828,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" const changedAdvisory = advisory(); const changedPr = { ...pr, title: "Add SSE endpoint for the old issue", body: "Still only implements SSE." }; await expect( - runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: blockMode, advisory: changedAdvisory, repoFullName: "acme/widgets", pr: changedPr, author: "alice", files, confirmedContributor: true, installationId: 1 }), + runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: blockMode, advisory: changedAdvisory, repoFullName: "acme/widgets", pr: changedPr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }), ).resolves.toMatchObject({ status: "unaddressed" }); expect(run).toHaveBeenCalledTimes(2); @@ -814,7 +846,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" estimatedNeurons: 5, }); const adv = advisory(); - const result = await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }); + const result = await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); // pr.linkedIssues is [1275] here, not 999 — must be a fresh call, not the stale row for issue #999. expect(run).toHaveBeenCalledTimes(1); expect(result).toMatchObject({ status: "addressed" }); @@ -825,12 +857,12 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" const run = vi.fn(async () => ({ response: satisfactionJson({ status: "addressed" }) })); const budgetedEnv = createTestEnv({ AI: { run } as unknown as Ai, AI_SUMMARIES_ENABLED: "true", AI_PUBLIC_COMMENTS_ENABLED: "true", AI_DAILY_NEURON_BUDGET: "1" }); const adv = advisory(); - await runLinkedIssueSatisfactionForAdvisory(budgetedEnv, { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }); + await runLinkedIssueSatisfactionForAdvisory(budgetedEnv, { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); expect(run).not.toHaveBeenCalled(); const richEnv = createTestEnv({ AI: { run } as unknown as Ai, AI_SUMMARIES_ENABLED: "true", AI_PUBLIC_COMMENTS_ENABLED: "true", AI_DAILY_NEURON_BUDGET: "100000" }); const adv2 = advisory(); - await runLinkedIssueSatisfactionForAdvisory(richEnv, { mode: "live", settings: advisoryMode, advisory: adv2, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }); + await runLinkedIssueSatisfactionForAdvisory(richEnv, { mode: "live", settings: advisoryMode, advisory: adv2, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); expect(run).toHaveBeenCalledTimes(1); }); @@ -841,7 +873,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" const repositoriesModule = await import("../../src/db/repositories"); const readSpy = vi.spyOn(repositoriesModule, "getCachedLinkedIssueSatisfaction").mockRejectedValueOnce(new Error("D1 read error")); const adv = advisory(); - const result = await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }); + const result = await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); expect(run).toHaveBeenCalledTimes(1); expect(result).toMatchObject({ status: "addressed" }); readSpy.mockRestore(); @@ -854,7 +886,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" const repositoriesModule = await import("../../src/db/repositories"); const writeSpy = vi.spyOn(repositoriesModule, "putCachedLinkedIssueSatisfaction").mockRejectedValueOnce(new Error("D1 write error")); const adv = advisory(); - const result = await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }); + const result = await runLinkedIssueSatisfactionForAdvisory(env, { mode: "live", settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1, commitThresholdReached: false }); expect(result).toMatchObject({ status: "addressed" }); writeSpy.mockRestore(); diff --git a/test/unit/queue-4.test.ts b/test/unit/queue-4.test.ts index 8f8f137b3b..e63a89d9de 100644 --- a/test/unit/queue-4.test.ts +++ b/test/unit/queue-4.test.ts @@ -79,6 +79,7 @@ import { asCloudEnv, createTestEnv } from "../helpers/d1"; import { ISSUE_WAKE_MAX_PRS, MERGE_WAKE_MAX_PRS, SWEEP_MAX_PRS } from "../../src/settings/agent-sweep"; import { AGENT_LABEL_PENDING_CLOSURE, DEFAULT_LINKED_ISSUE_HARD_RULES } from "../../src/review/linked-issue-hard-rules"; import { generatePrivateKeyPem } from "../helpers/github-app-key"; +import { AI_REVIEW_CACHE_INPUT_VERSION } from "../../src/review/ai-review-cache-input"; vi.mock("../../src/github/pr-freshness", async (importOriginal) => { const actual = await importOriginal(); @@ -1296,10 +1297,12 @@ describe("queue processors", () => { // comment) -- this assertion only cares that a real, current-shape fingerprint was computed and threaded // through, not the exact version number, so it is updated to the new version rather than left pinned to // the pre-fix one. - expect(cacheReadSpy.mock.calls[0]?.[5]).toMatch(/^ai-review-input:v6:/); + // #9477: assert against the constant, not a pinned literal -- this test broke on the v6 -> v7 bump, + // and the property that matters is "the fingerprint carries the current version", not which version it is. + expect(cacheReadSpy.mock.calls[0]?.[5]).toMatch(new RegExp(`^${AI_REVIEW_CACHE_INPUT_VERSION}:`)); expect(cacheWriteSpy).toHaveBeenCalled(); expect(cacheWriteSpy.mock.calls[0]?.[5]).toMatchObject({ - metadata: { inputFingerprint: expect.stringMatching(/^ai-review-input:v6:/) }, + metadata: { inputFingerprint: expect.stringMatching(new RegExp(`^${AI_REVIEW_CACHE_INPUT_VERSION}:`)) }, }); cacheReadSpy.mockRestore(); cacheWriteSpy.mockRestore();