fix(review): widen fix-handoff code-span delimiter for backtick paths(#9289)

src/review/fix-handoff-render.ts

@@ -37,15 +37,19 @@ export type FixHandoffBlock = {
  *  parse fix-handoff blocks in a comment body without depending on markdown structure alone. */
 const FIX_HANDOFF_MARKER = "<!-- loopover:fix-handoff -->";
 
-/** Public-safe inline-code escaping for a finding path/location. GitHub comments still render markdown inside
- *  collapsibles, so neutralize delimiters that can break out of the `...` span or table-like contexts before
- *  composing the location label. */
+/** Public-safe inline-code rendering for a finding path/location. GitHub comments still render markdown inside
+ *  collapsibles, so choose a code-span delimiter longer than any backtick run inside the value instead of trying
+ *  to backslash-escape backticks (Markdown does not honor that inside code spans) — mirroring `markdownPathCode`
+ *  in `src/review/unified-comment-bridge.ts`. Entity-escape `|`/`<>` (and backslashes) first, then return the
+ *  full delimiter-wrapped span (with surrounding spaces) so callers must not re-wrap in a fixed single backtick. */
 function markdownPathCodeText(value: string): string {
-  return value
+  const safeValue = value
     .replace(/\\/g, "\\\\")
-    .replace(/`/g, "\\`")
     .replace(/\|/g, "\\|")
     .replace(/[<>]/g, (char) => (char === "<" ? "&lt;" : "&gt;"));
+  const longestBacktickRun = Math.max(0, ...Array.from(safeValue.matchAll(/`+/g), (match) => match[0].length));
+  const delimiter = "`".repeat(longestBacktickRun + 1);
+  return `${delimiter} ${safeValue} ${delimiter}`;
 }
 
 /** PURE: build a single finding's fix-handoff block. Never throws; a finding whose `line` is not a positive
@@ -54,8 +58,8 @@ function markdownPathCodeText(value: string): string {
 export function buildFixHandoffBlock(finding: InlineFinding): FixHandoffBlock {
   const hasLine = Number.isInteger(finding.line) && finding.line > 0;
   const line = hasLine ? finding.line : 0;
-  const safePath = markdownPathCodeText(finding.path);
-  const location = hasLine ? `${safePath}:${line}` : `${safePath} (no specific line)`;
+  // markdownPathCodeText returns the full delimiter-wrapped span — consume it directly (do not re-wrap).
+  const location = markdownPathCodeText(hasLine ? `${finding.path}:${line}` : `${finding.path} (no specific line)`);
   const label = finding.severity === "blocker" ? "Blocker" : "Nit";
   const suggestedChange = finding.suggestion?.trim() || undefined;
   // Skip the fenced block when the suggestion itself contains a ``` sequence, which would close the outer fence
@@ -64,7 +68,7 @@ export function buildFixHandoffBlock(finding: InlineFinding): FixHandoffBlock {
     suggestedChange && !suggestedChange.includes("```") ? `\n\nSuggested change:\n\`\`\`\n${suggestedChange}\n\`\`\`` : "";
   const body = [
     FIX_HANDOFF_MARKER,
-    `**Fix handoff — ${label} at \`${location}\`**`,
+    `**Fix handoff — ${label} at ${location}**`,
     finding.body,
     suggestionBlock,
     `\n_${LOCAL_WRITE_BOUNDARY}_`,
@@ -105,14 +109,16 @@ const FIX_HANDOFF_AGGREGATE_MARKER = "<!-- loopover:fix-handoff-aggregate -->";
  *  buildFixHandoffBlock, just indented under a shared numbered list instead of standing alone. */
 function fixHandoffAggregateItem(finding: InlineFinding, index: number): string {
   const hasLine = Number.isInteger(finding.line) && finding.line > 0;
-  const safePath = markdownPathCodeText(finding.path);
-  const location = hasLine ? `${safePath}:${finding.line}` : `${safePath} (no specific line)`;
+  // Consume markdownPathCodeText's own delimiter-wrapped span directly (see buildFixHandoffBlock).
+  const location = markdownPathCodeText(
+    hasLine ? `${finding.path}:${finding.line}` : `${finding.path} (no specific line)`,
+  );
   const label = finding.severity === "blocker" ? "Blocker" : "Nit";
   const suggestion = finding.suggestion?.trim();
   // 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}`;
+  return `${index + 1}. **${label} at ${location}** — ${finding.body}${suggestionBlock}`;
 }
 
 /** PURE: combine every current finding into ONE fix-handoff block for a single local-agent run across the

test/unit/fix-handoff-collapsible.test.ts

@@ -37,11 +37,11 @@ describe("buildFixHandoffCollapsible (#1962)", () => {
     expect(c).not.toBeNull();
     expect(c?.title).toBe("Fix handoff");
     expect(c?.body).toContain("<!-- loopover:fix-handoff -->");
-    expect(c?.body).toContain("`src/a.ts:10`");
+    expect(c?.body).toContain("` src/a.ts:10 `");
     expect(c?.body).toContain("Possible null dereference on the fetched record.");
     expect(c?.body).toContain("Suggested change:");
     // the path-only (no commentable line) block still identifies WHERE to look
-    expect(c?.body).toContain("`src/b.ts (no specific line)`");
+    expect(c?.body).toContain("` src/b.ts (no specific line) `");
   });
 
   it("escapes adversarial paths before rendering inline-code locations", () => {
@@ -55,7 +55,10 @@ describe("buildFixHandoffCollapsible (#1962)", () => {
     ]);
 
     expect(block?.path).toBe("src/x` [Review required](https://evil.example/phish) | <tag>");
-    expect(block?.body).toContain("`src/x\\` [Review required](https://evil.example/phish) \\| &lt;tag&gt;:7`");
+    // The literal backtick in the path is neutralized by choosing a longer code-span delimiter (``), not by
+    // backslash-escaping it (Markdown ignores that inside code spans) — pipe/angle-brackets stay entity-escaped (#9289).
+    expect(block?.body).toContain("`` src/x` [Review required](https://evil.example/phish) \\| &lt;tag&gt;:7 ``");
+    expect(block?.body).not.toContain("\\`"); // no backslash-escaped backtick anymore
     expect(block?.body).not.toContain("`src/x` [Review required](https://evil.example/phish) | <tag>:7`");
   });
 

test/unit/fix-handoff-render.test.ts

@@ -12,7 +12,7 @@ describe("buildFixHandoffBlock (#2175)", () => {
     const block = buildFixHandoffBlock(finding({ line: 12, severity: "blocker" }));
     expect(block).toMatchObject({ path: "src/a.ts", line: 12, severity: "blocker", instruction: "Null check missing before dereference." });
     expect(block.body).toContain("src/a.ts:12");
-    expect(block.body).toContain("**Fix handoff — Blocker at `src/a.ts:12`**");
+    expect(block.body).toContain("**Fix handoff — Blocker at ` src/a.ts:12 `**");
   });
 
   it("renders a nit label", () => {
@@ -76,6 +76,20 @@ describe("buildFixHandoffBlock (#2175)", () => {
     expect(block.instruction).toBe("Add a guard for the empty-array case.");
     expect(block.body).toContain("Add a guard for the empty-array case.");
   });
+
+  it("renders a backtick-containing path as one unbroken code span with a longer delimiter (#9289)", () => {
+    // A backtick in the path can't be neutralized by backslash-escaping inside a markdown code span, so the
+    // location is wrapped in a delimiter (here ``) longer than the longest backtick run in the value instead.
+    const block = buildFixHandoffBlock(finding({ path: "src/we`ird.ts", line: 5 }));
+    expect(block.body).toContain("**Fix handoff — Blocker at `` src/we`ird.ts:5 ``**");
+    expect(block.body).not.toContain("\\`"); // backticks are NOT backslash-escaped (markdown ignores that in spans)
+    expect(block.path).toBe("src/we`ird.ts");
+  });
+
+  it("preserves pipe/angle-bracket entity escaping alongside the widened delimiter (#9289)", () => {
+    const block = buildFixHandoffBlock(finding({ path: "src/a|b<c>.ts", line: 1 }));
+    expect(block.body).toContain("**Fix handoff — Blocker at ` src/a\\|b&lt;c&gt;.ts:1 `**");
+  });
 });
 
 describe("buildFixHandoffBlocks (#2175)", () => {
@@ -101,7 +115,7 @@ describe("buildFixHandoffAggregateBlock (#5102)", () => {
     const block = buildFixHandoffAggregateBlock([finding()]);
     expect(block?.findingCount).toBe(1);
     expect(block?.body).toContain("**Fix handoff — 1 finding across this PR**");
-    expect(block?.body).toContain("1. **Blocker at `src/a.ts:12`** — Null check missing before dereference.");
+    expect(block?.body).toContain("1. **Blocker at ` src/a.ts:12 `** — Null check missing before dereference.");
   });
 
   it("combines multiple findings into one numbered block with plural wording", () => {
@@ -111,8 +125,8 @@ describe("buildFixHandoffAggregateBlock (#5102)", () => {
     ]);
     expect(block?.findingCount).toBe(2);
     expect(block?.body).toContain("**Fix handoff — 2 findings across this PR**");
-    expect(block?.body).toContain("1. **Blocker at `a.ts:1`**");
-    expect(block?.body).toContain("2. **Nit at `b.ts:2`**");
+    expect(block?.body).toContain("1. **Blocker at ` a.ts:1 `**");
+    expect(block?.body).toContain("2. **Nit at ` b.ts:2 `**");
   });
 
   it("renders a path-only location when a finding has no commentable line", () => {
@@ -121,6 +135,14 @@ describe("buildFixHandoffAggregateBlock (#5102)", () => {
     expect(block?.body).not.toContain("src/a.ts:0");
   });
 
+  it("renders a backtick-containing path in the aggregate item as one unbroken code span (#9289)", () => {
+    // A run of two backticks forces a three-backtick delimiter (longest run + 1), keeping the span unbroken
+    // where a fixed single backtick would be closed prematurely.
+    const block = buildFixHandoffAggregateBlock([finding({ path: "src/a``b.ts", line: 3 })]);
+    expect(block?.body).toContain("1. **Blocker at ``` src/a``b.ts:3 ```** — Null check missing before dereference.");
+    expect(block?.body).not.toContain("\\`");
+  });
+
   it("includes a fenced suggestion block, indented under its list item, when present", () => {
     const block = buildFixHandoffAggregateBlock([finding({ suggestion: "if (!value) return null;" })]);
     expect(block?.body).toContain("```\n   if (!value) return null;\n   ```");

test/unit/queue-4.test.ts

@@ -6104,7 +6104,7 @@ describe("queue processors", () => {
     });
 
     expect(unifiedCommentBody).toContain("Fix handoff"); // the collapsible section is emitted
-    expect(unifiedCommentBody).toContain("Fix handoff — Blocker at `src/db.ts:2`"); // the per-finding block header + location anchor
+    expect(unifiedCommentBody).toContain("Fix handoff — Blocker at ` src/db.ts:2 `"); // the per-finding block header + location anchor
     expect(unifiedCommentBody).toContain("This query is vulnerable to SQL injection."); // the finding, handed off verbatim
     expect(unifiedCommentBody).toContain("Suggested change:"); // its suggestion carried through
   });