From c8fe8336b905c7ed6f222ba2eb8d71bbc33b1862 Mon Sep 17 00:00:00 2001 From: ultrahighsuper Date: Fri, 17 Jul 2026 03:03:56 +0900 Subject: [PATCH] fix(review): guard fix-handoff suggestion blocks against an embedded code fence (#6632) `buildFixHandoffBlock` and `fixHandoffAggregateItem` in src/review/fix-handoff-render.ts splice a finding's suggestion text into a fenced code block without checking whether the suggestion itself contains a ``` sequence. A suggestion that demonstrates a fenced code example would close the outer fence early and break the rendered markdown. Add the same guard the sibling renderer inline-suggestion-anchor.ts's `safeSuggestionBlock` already uses -- skip the fenced block entirely when the suggestion contains ``` -- to BOTH call sites. The structured `suggestedChange` field (raw data, not a fenced block) is unchanged; only the markdown rendering is guarded. Extends test/unit/fix-handoff-render.test.ts with a regression for each function: a suggestion containing a ``` fence renders no fenced block (no stray ``` leaks into the output), while the existing present/absent/whitespace-only cases keep their behavior. Closes #6632 --- src/review/fix-handoff-render.ts | 9 +++++++-- test/unit/fix-handoff-render.test.ts | 13 +++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/review/fix-handoff-render.ts b/src/review/fix-handoff-render.ts index 119d5abf4c..4dd86ecbd2 100644 --- a/src/review/fix-handoff-render.ts +++ b/src/review/fix-handoff-render.ts @@ -58,7 +58,10 @@ export function buildFixHandoffBlock(finding: InlineFinding): FixHandoffBlock { const location = hasLine ? `${safePath}:${line}` : `${safePath} (no specific line)`; const label = finding.severity === "blocker" ? "Blocker" : "Nit"; const suggestedChange = finding.suggestion?.trim() || undefined; - const suggestionBlock = suggestedChange ? `\n\nSuggested change:\n\`\`\`\n${suggestedChange}\n\`\`\`` : ""; + // Skip the fenced block when the suggestion itself contains a ``` sequence, which would close the outer fence + // early and break the rendered markdown -- matching inline-suggestion-anchor.ts's safeSuggestionBlock guard. + const suggestionBlock = + suggestedChange && !suggestedChange.includes("```") ? `\n\nSuggested change:\n\`\`\`\n${suggestedChange}\n\`\`\`` : ""; const body = [ FIX_HANDOFF_MARKER, `**Fix handoff — ${label} at \`${location}\`**`, @@ -106,7 +109,9 @@ function fixHandoffAggregateItem(finding: InlineFinding, index: number): string const location = hasLine ? `${safePath}:${finding.line}` : `${safePath} (no specific line)`; const label = finding.severity === "blocker" ? "Blocker" : "Nit"; const suggestion = finding.suggestion?.trim(); - const suggestionBlock = suggestion ? `\n \`\`\`\n ${suggestion.replace(/\n/g, "\n ")}\n \`\`\`` : ""; + // Same fence-safety guard as buildFixHandoffBlock / safeSuggestionBlock: an embedded ``` would break the block. + const suggestionBlock = + suggestion && !suggestion.includes("```") ? `\n \`\`\`\n ${suggestion.replace(/\n/g, "\n ")}\n \`\`\`` : ""; return `${index + 1}. **${label} at \`${location}\`** — ${finding.body}${suggestionBlock}`; } diff --git a/test/unit/fix-handoff-render.test.ts b/test/unit/fix-handoff-render.test.ts index c7684454c5..647c067b3d 100644 --- a/test/unit/fix-handoff-render.test.ts +++ b/test/unit/fix-handoff-render.test.ts @@ -39,6 +39,14 @@ describe("buildFixHandoffBlock (#2175)", () => { expect(block.body).not.toContain("Suggested change:"); }); + it("skips the fenced block when the suggestion itself contains a ``` sequence (#6632)", () => { + const block = buildFixHandoffBlock(finding({ suggestion: "Replace with:\n```\nconst x = 1;\n```" })); + // The embedded fence would close the outer block early, so the fenced rendering is skipped entirely + // (matching safeSuggestionBlock); no stray ``` leaks into the rendered markdown. + expect(block.body).not.toContain("Suggested change:"); + expect(block.body).not.toContain("```"); + }); + it("yields a path-only block (line 0) when the finding has no commentable line (line <= 0)", () => { const block = buildFixHandoffBlock(finding({ line: 0 })); expect(block.line).toBe(0); @@ -123,6 +131,11 @@ describe("buildFixHandoffAggregateBlock (#5102)", () => { expect(buildFixHandoffAggregateBlock([finding({ suggestion: " " })])?.body).not.toContain("```"); }); + it("skips the fenced suggestion block when the suggestion contains a ``` sequence (#6632)", () => { + const block = buildFixHandoffAggregateBlock([finding({ suggestion: "```\nconst x = 1;\n```" })]); + expect(block?.body).not.toContain("```"); // embedded fence would break the item's block; skipped entirely + }); + it("always includes the exact LOCAL_WRITE_BOUNDARY text (boundary-safe)", () => { const block = buildFixHandoffAggregateBlock([finding()]); expect(block?.boundary).toBe(LOCAL_WRITE_BOUNDARY);