From 59e5a812f9776f11bd66bf379b239f0f0924e2fa Mon Sep 17 00:00:00 2001 From: Victor Vogel Date: Mon, 19 Oct 2026 19:15:29 -0500 Subject: [PATCH] fix(server): read quoted and tab-ended Git paths in review-guide hunks Git quotes a patch filename holding a tab, quote or backslash, and ends an unquoted one holding a space with a tab. The review guide's hunk parser took the `--- a/` / `+++ b/` text literally, so those files' hunks were keyed under names that never matched the NUL-separated changed-file list and every excerpt in them was dropped. Read the header token the way Git writes it, decoding quoted names with the shared unquoteGitPatchPath. Co-Authored-By: Claude Opus 5.5 --- .../pullRequestReviewGuideNormalize.test.ts | 31 +++++++++++++++++++ .../pullRequestReviewGuideNormalize.ts | 17 ++++++++-- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/apps/server/src/pullRequest/pullRequestReviewGuideNormalize.test.ts b/apps/server/src/pullRequest/pullRequestReviewGuideNormalize.test.ts index 233013ac7146d..6794dfed5e740 100644 --- a/apps/server/src/pullRequest/pullRequestReviewGuideNormalize.test.ts +++ b/apps/server/src/pullRequest/pullRequestReviewGuideNormalize.test.ts @@ -91,6 +91,37 @@ describe("parseReviewGuideChange", () => { expect(change.hunks.get("src/gone.ts")).toEqual({ new: [], old: [{ start: 1, end: 20 }] }); expect(change.hunks.get("docs/readme.md")).toEqual({ new: [{ start: 3, end: 3 }], old: [] }); }); + + it("reads file names git quotes or ends with a tab", () => { + const unusual = [ + "diff --git a/with space.txt b/with space.txt", + "--- a/with space.txt\t", + "+++ b/with space.txt\t", + "@@ -1,0 +2 @@", + 'diff --git "a/tab\\tname.txt" "b/tab\\tname.txt"', + '--- "a/tab\\tname.txt"', + '+++ "b/tab\\tname.txt"', + "@@ -1,0 +2 @@", + 'diff --git "a/back\\\\slash.txt" "b/back\\\\slash.txt"', + '--- "a/back\\\\slash.txt"', + '+++ "b/back\\\\slash.txt"', + "@@ -1,0 +2 @@", + 'diff --git "a/gone\\tfile.txt" "b/gone\\tfile.txt"', + '--- "a/gone\\tfile.txt"', + "+++ /dev/null", + "@@ -1 +0,0 @@", + "", + ].join("\n"); + const names = ["with space.txt", "tab\tname.txt", "back\\slash.txt", "gone\tfile.txt"]; + const parsed = parseReviewGuideChange(names.join("\0"), unusual); + expect([...parsed.hunks.keys()]).toEqual(names); + expect(parsed.hunks.get("gone\tfile.txt")).toEqual({ new: [], old: [{ start: 1, end: 1 }] }); + const result = normalizeReviewGuide( + guide({ stops: [stop({ excerpts: names.slice(0, 3).map((path) => excerpt(path, 1, 5)) })] }), + parsed, + ); + expect(result.stops[0]?.excerpts).toEqual(names.slice(0, 3).map((path) => excerpt(path, 2, 2))); + }); }); describe("normalizeReviewGuide", () => { diff --git a/apps/server/src/pullRequest/pullRequestReviewGuideNormalize.ts b/apps/server/src/pullRequest/pullRequestReviewGuideNormalize.ts index 4075f19ba9ba9..184908e4bd7f2 100644 --- a/apps/server/src/pullRequest/pullRequestReviewGuideNormalize.ts +++ b/apps/server/src/pullRequest/pullRequestReviewGuideNormalize.ts @@ -1,5 +1,6 @@ import type { PullRequestReviewGuideExcerpt, PullRequestReviewGuideStop } from "@t3tools/contracts"; import type { ReviewGuideGenerationResult } from "@t3tools/provider-core/server/textGeneration"; +import { unquoteGitPatchPath } from "@t3tools/shared/gitPatchPath"; const MAX_EXCERPT_LINES = 25; @@ -25,6 +26,18 @@ const hunkRange = (start: string, count: string | undefined): LineRange | null = return lines === 0 ? null : { start: Number(start), end: Number(start) + lines - 1 }; }; +/** + * Git quotes a name holding a tab, quote or backslash, and ends an unquoted name holding a space + * with a tab, so the first literal tab is never part of what the file is called. + */ +function headerPath(line: string, prefix: string): string | null { + const rest = line.slice(4); + const tab = rest.indexOf("\t"); + const token = tab === -1 ? rest : rest.slice(0, tab); + const path = token.startsWith('"') ? unquoteGitPatchPath(token) : token; + return path.startsWith(prefix) ? path.slice(prefix.length) : null; +} + export function parseReviewGuideChange(nameOnly: string, patch: string): ReviewGuideChange { const hunks = new Map(); let oldPath: string | null = null; @@ -37,9 +50,9 @@ export function parseReviewGuideChange(nameOnly: string, patch: string): ReviewG current = null; inHunks = false; } else if (!inHunks && line.startsWith("--- ")) { - oldPath = line.startsWith("--- a/") ? line.slice(6) : null; + oldPath = headerPath(line, "a/"); } else if (!inHunks && line.startsWith("+++ ")) { - const path = line.startsWith("+++ b/") ? line.slice(6) : oldPath; + const path = headerPath(line, "b/") ?? oldPath; if (path === null) continue; current = hunks.get(path) ?? { new: [], old: [] }; hunks.set(path, current);