diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 399d48a7e2..dd7d719def 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -7751,14 +7751,18 @@ export async function runLinkedIssueSatisfactionForAdvisory( model: args.settings.aiReviewModel ?? storedKey.model, } : null; - // #linked-issue-satisfaction-cache: the assessment's LLM call is fully deterministic given the same head SHA - // + linked issue number (no RAG/grounding/enrichment feeds into it), so a repeated scheduled sweep pass at - // an unchanged head+issue reuses the stored result instead of re-spending up to 6 free-tier attempts (or a - // BYOK call) on every tick — mirrors ai_slop_cache's confirmed-in-production motivation exactly. + const diff = buildAiReviewDiff(args.files); + // #linked-issue-satisfaction-cache: the assessment's LLM call is fully deterministic for the same + // reviewer configuration and prompt. GitHub issue/PR text can be edited without changing the head SHA, so + // those prompt fields are part of this fingerprint rather than relying only on the row key. const inputFingerprint = await linkedIssueSatisfactionCacheInputFingerprint({ byok: Boolean(providerKey), provider: providerKey?.provider, model: providerKey?.model, + issueText, + prTitle: args.pr.title, + prBody: args.pr.body ?? undefined, + diff, }); const cached = await getCachedLinkedIssueSatisfaction( env, @@ -7798,7 +7802,7 @@ export async function runLinkedIssueSatisfactionForAdvisory( issueText, prTitle: args.pr.title, prBody: args.pr.body ?? undefined, - diff: buildAiReviewDiff(args.files), + diff, actor: args.author, providerKey, }); diff --git a/src/review/linked-issue-satisfaction-cache-input.ts b/src/review/linked-issue-satisfaction-cache-input.ts index 252802f06e..c09dd619ea 100644 --- a/src/review/linked-issue-satisfaction-cache-input.ts +++ b/src/review/linked-issue-satisfaction-cache-input.ts @@ -1,19 +1,20 @@ import { sha256Hex } from "../utils/crypto"; -// #linked-issue-satisfaction-cache: mirrors ai-slop-cache-input.ts's fingerprint discipline exactly (kept as -// its own small module -- not reused directly -- so the two caches' version strings never collide/alias each -// other in stored rows). The satisfaction assessment's only input that can change independently of the PR's -// head SHA is which provider writes the opinion: the free/default reviewer vs. a maintainer's BYOK key/model -// (see LinkedIssueSatisfactionRunInput in ../services/linked-issue-satisfaction-run). Issue title/body/diff are -// pinned to the head SHA (a fresh commit is what invalidates the cache row itself) and the linked issue number -// is a SEPARATE primary-key column (not folded into this fingerprint) -- see the cache table's migration doc -// for why a changed primary linked issue must miss the cache rather than replay a different issue's verdict. -export const LINKED_ISSUE_SATISFACTION_CACHE_INPUT_VERSION = "linked-issue-satisfaction-input:v1"; +// #linked-issue-satisfaction-cache: mirrors ai-slop-cache-input.ts's fingerprint discipline, but includes +// every prompt input that is not already represented by the row key. The row key handles repo/pull/head SHA +// and primary linked issue number; this fingerprint handles reviewer configuration plus mutable GitHub text +// (issue title/body and PR title/body) so edits cannot replay a verdict for an older prompt. Diff text is +// included defensively too, keeping the cache tied to exactly the model prompt that produced the opinion. +export const LINKED_ISSUE_SATISFACTION_CACHE_INPUT_VERSION = "linked-issue-satisfaction-input:v2"; export type LinkedIssueSatisfactionCacheInput = { byok: boolean; provider: string | null | undefined; model: string | null | undefined; + issueText?: string | null | undefined; + prTitle?: string | null | undefined; + prBody?: string | null | undefined; + diff?: string | null | undefined; }; export async function linkedIssueSatisfactionCacheInputFingerprint(input: LinkedIssueSatisfactionCacheInput): Promise { @@ -22,6 +23,10 @@ export async function linkedIssueSatisfactionCacheInputFingerprint(input: Linked input.byok ? "1" : "0", input.provider ?? "", input.model ?? "", + input.issueText ?? "", + input.prTitle ?? "", + input.prBody ?? "", + input.diff ?? "", ].join("|"); return `${LINKED_ISSUE_SATISFACTION_CACHE_INPUT_VERSION}:${await sha256Hex(payload)}`; } diff --git a/test/unit/linked-issue-satisfaction-cache.test.ts b/test/unit/linked-issue-satisfaction-cache.test.ts index eb265cf53c..94d0b5d351 100644 --- a/test/unit/linked-issue-satisfaction-cache.test.ts +++ b/test/unit/linked-issue-satisfaction-cache.test.ts @@ -122,6 +122,38 @@ describe("linkedIssueSatisfactionCacheInputFingerprint", () => { expect(withUndefined).toBe(withNull); }); + it("differs when mutable prompt text changes (regression for stale linked-issue gate cache)", async () => { + const before = await linkedIssueSatisfactionCacheInputFingerprint({ + byok: false, + provider: null, + model: null, + issueText: "Need an SSE endpoint", + prTitle: "Add SSE endpoint", + prBody: "Closes the SSE issue", + diff: "+app.get('/stream', sse)", + }); + const editedIssue = await linkedIssueSatisfactionCacheInputFingerprint({ + byok: false, + provider: null, + model: null, + issueText: "Need a GraphQL subscription", + prTitle: "Add SSE endpoint", + prBody: "Closes the SSE issue", + diff: "+app.get('/stream', sse)", + }); + const editedPr = await linkedIssueSatisfactionCacheInputFingerprint({ + byok: false, + provider: null, + model: null, + issueText: "Need an SSE endpoint", + prTitle: "Add GraphQL subscription", + prBody: "Closes the GraphQL issue", + diff: "+app.get('/stream', sse)", + }); + expect(editedIssue).not.toBe(before); + expect(editedPr).not.toBe(before); + }); + it("never collides with the ai_slop_cache fingerprint namespace even for identical inputs", async () => { const { aiSlopCacheInputFingerprint } = await import("../../src/review/ai-slop-cache-input"); const slop = await aiSlopCacheInputFingerprint({ byok: false, provider: null, model: null }); diff --git a/test/unit/linked-issue-satisfaction-run.test.ts b/test/unit/linked-issue-satisfaction-run.test.ts index 4366600681..fdb1979081 100644 --- a/test/unit/linked-issue-satisfaction-run.test.ts +++ b/test/unit/linked-issue-satisfaction-run.test.ts @@ -1,6 +1,6 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { runGittensoryLinkedIssueSatisfaction, type LinkedIssueSatisfactionRunInput } from "../../src/services/linked-issue-satisfaction-run"; -import { processJob, runLinkedIssueSatisfactionForAdvisory } from "../../src/queue/processors"; +import { buildAiReviewDiff, processJob, runLinkedIssueSatisfactionForAdvisory } from "../../src/queue/processors"; import { evaluateGateCheck } from "../../src/rules/advisory"; import { getCachedLinkedIssueSatisfaction, @@ -294,6 +294,9 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" { repoFullName: "acme/widgets", pullNumber: 7, path: "src/a.ts", status: "modified", additions: 40, deletions: 2, changes: 42, payload: { patch: "@@\n+app.get('/stream', sse);" } }, ]; const pr = { number: 7, title: "Add SSE stream endpoint", body: "Implements the requested SSE stream.", linkedIssues: [1275] }; + const issueText = "Enrich SN74 Gittensor — add SSE stream\n\nWe need a live SSE stream surface for SN74 Gittensor."; + 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; const blockMode = { linkedIssueSatisfactionGateMode: "block", aiReviewByok: false } as RepositorySettings; @@ -532,7 +535,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" stubIssueFetch(); const run = vi.fn(async () => ({ response: satisfactionJson({ status: "unaddressed", confidence: 0.9 }) })); const env = enabledEnv(run); - const fingerprint = await linkedIssueSatisfactionCacheInputFingerprint({ byok: false, provider: null, model: null }); + const fingerprint = await processorFingerprint(); await putCachedLinkedIssueSatisfaction(env, "acme/widgets", 7, "sha7", 1275, fingerprint, { status: "ok", result: { status: "addressed", rationale: "cached: looks done", confidence: 0.8 }, @@ -548,7 +551,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" stubIssueFetch(); const run = vi.fn(); const env = enabledEnv(run); - const fingerprint = await linkedIssueSatisfactionCacheInputFingerprint({ byok: false, provider: null, model: null }); + const fingerprint = await processorFingerprint(); await putCachedLinkedIssueSatisfaction(env, "acme/widgets", 7, "sha7", 1275, fingerprint, { status: "ok", result: { status: "addressed", rationale: "cached: looks done", confidence: 0.8 }, @@ -589,7 +592,7 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" await runLinkedIssueSatisfactionForAdvisory(env, { settings: advisoryMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }); expect(run).toHaveBeenCalledTimes(1); - const fingerprint = await linkedIssueSatisfactionCacheInputFingerprint({ byok: false, provider: null, model: null }); + const fingerprint = await processorFingerprint(); const cached = await getCachedLinkedIssueSatisfaction(env, "acme/widgets", 7, "sha7", 1275, fingerprint); expect(cached).toMatchObject({ status: "ok", result: { status: "addressed" } }); @@ -598,6 +601,27 @@ describe("runLinkedIssueSatisfactionForAdvisory (processor wiring, #1961/#3906)" expect(run).toHaveBeenCalledTimes(1); // still 1 — second pass was a cache hit }); + it("misses the cache when editable issue or PR text changes at the same head SHA", async () => { + stubIssueFetch({ body: "We need a live SSE stream surface for SN74 Gittensor." }); + const run = vi.fn(async () => ({ response: satisfactionJson({ status: "addressed", rationale: "The SSE endpoint satisfies the original ask." }) })); + const env = enabledEnv(run); + const adv = advisory(); + await expect( + runLinkedIssueSatisfactionForAdvisory(env, { settings: blockMode, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, confirmedContributor: true, installationId: 1 }), + ).resolves.toMatchObject({ status: "addressed" }); + + stubIssueFetch({ body: "We now need a GraphQL subscription instead of an SSE stream." }); + run.mockResolvedValueOnce({ response: satisfactionJson({ status: "unaddressed", confidence: 0.9, rationale: "The changed issue asks for GraphQL, but the diff still adds SSE." }) }); + const changedAdvisory = advisory(); + const changedPr = { ...pr, title: "Add SSE endpoint for the old issue", body: "Still only implements SSE." }; + await expect( + runLinkedIssueSatisfactionForAdvisory(env, { settings: blockMode, advisory: changedAdvisory, repoFullName: "acme/widgets", pr: changedPr, author: "alice", files, confirmedContributor: true, installationId: 1 }), + ).resolves.toMatchObject({ status: "unaddressed" }); + + expect(run).toHaveBeenCalledTimes(2); + expect(changedAdvisory.findings).toContainEqual(expect.objectContaining({ code: "linked_issue_scope_mismatch" })); + }); + it("misses the cache when the PR's primary linked issue number changes, even at the same head SHA", async () => { stubIssueFetch(); const run = vi.fn(async () => ({ response: satisfactionJson({ status: "addressed" }) })); @@ -785,10 +809,15 @@ describe("linked-issue satisfaction wired end-to-end through the real webhook pi expect(gatePatchBody.conclusion).toBe("failure"); expect(gatePatchBody.output?.title).toContain("Linked issue does not appear to be satisfied"); - // The assessment was cached under the PR's primary linked issue number. - const fingerprint = await linkedIssueSatisfactionCacheInputFingerprint({ byok: false, provider: null, model: null }); - const cached = await getCachedLinkedIssueSatisfaction(env, "JSONbored/metagraphed", 3910, "realvenus3910", 1275, fingerprint); - expect(cached).toMatchObject({ status: "ok", result: { status: "unaddressed" } }); + // The assessment was cached under the PR's primary linked issue number. The fingerprint itself includes + // prompt text, which this end-to-end test does not need to reconstruct from the webhook pipeline. + const cached = await env.DB.prepare( + "SELECT status, result_json AS resultJson FROM linked_issue_satisfaction_cache WHERE repo_full_name = ? AND pull_number = ? AND head_sha = ? AND linked_issue_number = ?", + ) + .bind("JSONbored/metagraphed", 3910, "realvenus3910", 1275) + .first<{ status: string; resultJson: string }>(); + expect(cached?.status).toBe("ok"); + expect(JSON.parse(cached?.resultJson ?? "{}")?.status).toBe("unaddressed"); }); it("OFF mode (default): no fetch, no model spend, no cache row, and the comment never mentions linked-issue satisfaction at all — byte-identical to before this feature existed", async () => {