From 8f32c51039f4ffa0c084d9c778338d804cf2299f Mon Sep 17 00:00:00 2001 From: Jeff <158072326+jeffrey701@users.noreply.github.com> Date: Wed, 15 Jul 2026 01:20:48 -0400 Subject: [PATCH] fix(review): hash a JSON payload in linkedIssueSatisfactionCacheInputFingerprint The fingerprint "|"-joined free-form GitHub text fields (issueText, prTitle, prBody, diff), so an unescaped "|" inside one field could shift a boundary and make two genuinely different inputs serialize identically and collide on the same cache key. Build the payload with JSON.stringify, matching the sibling aiSlopCacheInputFingerprint, so distinct inputs always produce distinct fingerprints. Closes #5939 --- .../linked-issue-satisfaction-cache-input.ts | 26 +++++++++++-------- .../linked-issue-satisfaction-cache.test.ts | 10 +++++++ 2 files changed, 25 insertions(+), 11 deletions(-) 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();