From 074ba3be7f2a723e34ce190a11f0b2935156edf0 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Mon, 27 Jul 2026 22:23:04 -0700 Subject: [PATCH 1/2] fix(ledger,drift): retry an unanchored tip, stop truncating the anchor log, and cap the third advisory (#9489, #9491) 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, and every hourly tick then sees tipUnchanged and returns "unchanged", so the tip carries no valid external anchor indefinitely: exactly the unanchored window the feature exists to bound. It was also backend-blind, since the newest row won regardless of which backend wrote it, so git succeeding masked rekor failing at the same seq. The scheduler now asks per backend whether THIS rowHash has a successful anchor, and retries on the hourly tick when it does not. loadDecisionLedgerTip read COUNT(*) and the tip row as two statements under Promise.all. The seq/totalCount pair is precisely how a verifier detects truncation-and-rechaining, so a concurrent append between them produced totalCount = seq + 1 beside the OLD rowHash -- an internally inconsistent checkpoint that anchoring then signs and publishes to Rekor, unretractably, as a FALSE tamper signal about the maintainer's own ledger. One statement now reads both from a single snapshot. For a file between 1 MB and 100 MB the Contents API returns content:"" with encoding:"none", and because typeof "" === "string" the old code accepted that as the file's contents and rewrote the whole anchor log to a single line while reporting ok. At ~300 bytes per line hourly that lands 4-5 months out; git history keeps the commits but the file a skeptic is told to read shrinks, which is indistinguishable from tampering. It now refuses and records a failed attempt, leaving the log untouched and the gap visible on the public attempt log. The linked-issue satisfaction advisory was the one paid LLM call in its family with no per-PR commit cap -- ai_slop and ai_review both stop past auto_pause_after_reviewed_commits -- so a long-lived PR kept paying for a fresh assessment on every push after its siblings had stopped. --- src/queue/processors.ts | 15 +++ src/review/decision-record.ts | 20 ++-- src/review/ledger-anchor-git.ts | 14 ++- src/review/ledger-anchor-persistence.ts | 25 +++++ src/review/ledger-anchor-scheduler.ts | 31 +++++- test/unit/ledger-anchor-git.test.ts | 62 ++++++++++++ test/unit/ledger-anchor-scheduler.test.ts | 48 +++++++++ .../linked-issue-satisfaction-run.test.ts | 98 ++++++++++++------- 8 files changed, 269 insertions(+), 44 deletions(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index eb109934d0..5e852dc201 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,13 @@ 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, + 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..1ebdffbae0 100644 --- a/src/review/ledger-anchor-git.ts +++ b/src/review/ledger-anchor-git.ts @@ -64,7 +64,19 @@ 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). + if (data.encoding === "none" || (data.content === "" && (data.size ?? 0) > 0)) { + 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..8835d79253 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,25 @@ 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. + 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..701cf7668a 100644 --- a/test/unit/ledger-anchor-git.test.ts +++ b/test/unit/ledger-anchor-git.test.ts @@ -144,3 +144,65 @@ 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) => { + if (route.startsWith("GET")) return { data: { content: "", encoding: "none", size: 2_000_000, 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/); + }); + + 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("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-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(); From f8b4e16d0f389594314a6983ee732ba66da261de Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Mon, 27 Jul 2026 22:29:17 -0700 Subject: [PATCH 2/2] fix(ci): unpin a hardcoded cache-input version that broke main, and cover the new ledger paths queue-4 asserted /^ai-review-input:v6:/ against a literal, so the v6 -> v7 bump in #9513 broke it on main. It now asserts against AI_REVIEW_CACHE_INPUT_VERSION itself: the property that matters is that the fingerprint carries the CURRENT version, not which version that happens to be, so a future bump cannot break it again. Adds per-backend anchor-lookup coverage (a failed attempt is not anchored; one backend's success does not mask another's failure; a success at a different rowHash does not anchor this tip), the three Contents-API body shapes, and the commit-threshold cap regression. Two fail-open catches are annotated: an unreadable counter must not SUPPRESS the advisory, and an unreadable anchors table degrades to the pre-#9489 scheduling behaviour rather than forcing an anchor every tick. --- src/queue/processors.ts | 2 + src/review/ledger-anchor-git.ts | 3 +- src/review/ledger-anchor-scheduler.ts | 2 + test/unit/ledger-anchor-git.test.ts | 37 +++++++++++++- test/unit/ledger-anchor-persistence.test.ts | 53 ++++++++++++++++++++- test/unit/queue-4.test.ts | 7 ++- 6 files changed, 99 insertions(+), 5 deletions(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 5e852dc201..1d7d4dbaa4 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -10674,6 +10674,8 @@ async function maybePublishPrPublicSurface( // 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/ledger-anchor-git.ts b/src/review/ledger-anchor-git.ts index 1ebdffbae0..5bb6207b24 100644 --- a/src/review/ledger-anchor-git.ts +++ b/src/review/ledger-anchor-git.ts @@ -72,7 +72,8 @@ export async function submitToGitAnchor(env: Env, signed: SignedLedgerAnchor, oc // 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). - if (data.encoding === "none" || (data.content === "" && (data.size ?? 0) > 0)) { + 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`, ); diff --git a/src/review/ledger-anchor-scheduler.ts b/src/review/ledger-anchor-scheduler.ts index 8835d79253..25ca625d67 100644 --- a/src/review/ledger-anchor-scheduler.ts +++ b/src/review/ledger-anchor-scheduler.ts @@ -96,6 +96,8 @@ export async function runScheduledLedgerAnchor(env: Env, options: { isHourly: bo // "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, diff --git a/test/unit/ledger-anchor-git.test.ts b/test/unit/ledger-anchor-git.test.ts index 701cf7668a..b99c07b533 100644 --- a/test/unit/ledger-anchor-git.test.ts +++ b/test/unit/ledger-anchor-git.test.ts @@ -157,7 +157,8 @@ describe("oversized anchor log (#9489)", () => { const signed = makeSignedAnchor(1); const octokit = { request: vi.fn(async (route: string) => { - if (route.startsWith("GET")) return { data: { content: "", encoding: "none", size: 2_000_000, sha: "abc" } }; + // 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: {} }; }), }; @@ -171,6 +172,7 @@ describe("oversized anchor log (#9489)", () => { 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 () => { @@ -191,6 +193,39 @@ describe("oversized anchor log (#9489)", () => { 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(); 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/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();