diff --git a/src/review/linked-issue-satisfaction-cache-input.ts b/src/review/linked-issue-satisfaction-cache-input.ts index c09dd619ea..c08b0f771b 100644 --- a/src/review/linked-issue-satisfaction-cache-input.ts +++ b/src/review/linked-issue-satisfaction-cache-input.ts @@ -18,15 +18,19 @@ export type LinkedIssueSatisfactionCacheInput = { }; export async function linkedIssueSatisfactionCacheInputFingerprint(input: LinkedIssueSatisfactionCacheInput): Promise { - const payload = [ - LINKED_ISSUE_SATISFACTION_CACHE_INPUT_VERSION, - 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)}`; + // Structurally-delimited payload (mirrors ai-slop-cache-input.ts): a bare "|"-join of free-form + // GitHub text let an unescaped "|" inside one field shift a field boundary, so two genuinely different + // inputs could serialize identically and collide on the same fingerprint. JSON.stringify escapes the + // field values, so distinct inputs always produce distinct payloads. + const payload = { + version: LINKED_ISSUE_SATISFACTION_CACHE_INPUT_VERSION, + byok: input.byok, + provider: input.provider ?? "", + model: input.model ?? "", + issueText: input.issueText ?? "", + prTitle: input.prTitle ?? "", + prBody: input.prBody ?? "", + diff: input.diff ?? "", + }; + return `${LINKED_ISSUE_SATISFACTION_CACHE_INPUT_VERSION}:${await sha256Hex(JSON.stringify(payload))}`; } diff --git a/test/unit/linked-issue-satisfaction-cache.test.ts b/test/unit/linked-issue-satisfaction-cache.test.ts index 1d94133c61..bf1b7a894b 100644 --- a/test/unit/linked-issue-satisfaction-cache.test.ts +++ b/test/unit/linked-issue-satisfaction-cache.test.ts @@ -47,6 +47,16 @@ describe("linked-issue satisfaction cache (#1961/#3906)", () => { expect(await getCachedLinkedIssueSatisfaction(env, "o/r", 9, "sha1", 1, freeFingerprint)).toEqual({ status: "ok", result: { status: "partial", rationale: "r", confidence: 0.7 }, estimatedNeurons: 6 }); }); + it("does not collide when a '|' inside one text field would shift a delimiter boundary (#5939)", async () => { + // A bare "|"-join let an unescaped "|" move a field boundary: {issueText: "foo|bar", prTitle: "baz"} + // and {issueText: "foo", prTitle: "bar|baz"} (other fields equal) serialized identically and hashed + // to the same fingerprint. JSON.stringify escapes the field values, so the two stay distinct. + const base = { byok: false, provider: null, model: null } as const; + const a = await linkedIssueSatisfactionCacheInputFingerprint({ ...base, issueText: "foo|bar", prTitle: "baz" }); + const b = await linkedIssueSatisfactionCacheInputFingerprint({ ...base, issueText: "foo", prTitle: "bar|baz" }); + expect(a).not.toBe(b); + }); + it("upserts — a re-run at the same key replaces the stored assessment", async () => { const env = createTestEnv(); const fingerprint = await fp();