From 39220f8653dd59256d418bd1e7e27d5f52f625fb Mon Sep 17 00:00:00 2001
From: yslee <yslee@abutton.com>
Date: Mon, 6 Apr 2026 16:52:16 +0900
Subject: [PATCH] fix(review): show P4 added files in code review
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

P4 added files (p4 add) were not appearing in the review UI on Windows.
The root cause was \r\n line endings from P4 command output — splitting
on \n left \r in file paths, causing Bun.file() to fail silently.

- Normalize \r\n to \n in runP4() stdout for all P4 commands
- Strip \r\n from file content in getNewFileDiff()
- Replace fragile index-based p4 where parsing with -ztag structured
  output, which is also more robust for stream depots and overlays

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 packages/server/p4.ts | 46 ++++++++++++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 16 deletions(-)

diff --git a/packages/server/p4.ts b/packages/server/p4.ts
index 9dcaa0f0..28746caf 100644
--- a/packages/server/p4.ts
+++ b/packages/server/p4.ts
@@ -32,7 +32,7 @@ async function runP4(
     proc.exited,
   ]);
 
-  return { stdout, stderr, exitCode };
+  return { stdout: stdout.replace(/\r\n/g, "\n"), stderr, exitCode };
 }
 
 // --- Path helpers ---
@@ -248,7 +248,7 @@ async function getNewFileDiff(
 ): Promise<string> {
   try {
     const content = await Bun.file(localPath).text();
-    const lines = content.split("\n");
+    const lines = content.replace(/\r\n/g, "\n").split("\n");
     const header = [
       `diff --git a/${relativePath} b/${relativePath}`,
       "new file mode 100644",
@@ -274,23 +274,37 @@ async function batchResolveDepotPaths(
   const result = new Map<string, { localPath: string; relativePath: string }>();
   if (depotPaths.length === 0) return result;
 
-  const whereResult = await runP4(["where", ...depotPaths], { cwd });
+  // Use -ztag for structured output — avoids fragile index-based parsing
+  // that breaks with stream depots, overlays, or unmapped path error lines
+  const whereResult = await runP4(["-ztag", "where", ...depotPaths], { cwd });
   if (whereResult.exitCode !== 0) return result;
 
-  const outputLines = whereResult.stdout.trim().split("\n");
-  for (let i = 0; i < outputLines.length && i < depotPaths.length; i++) {
-    const whereOutput = outputLines[i];
-    const rootIdx = whereOutput.indexOf(normalizePath(normalizedRoot).length > 0 ? normalizedRoot : "NOMATCH");
-    // Find the local path by looking for the client root
-    const clientRootBackslash = normalizedRoot.replace(/\//g, "\\");
-    const bsIdx = whereOutput.indexOf(clientRootBackslash);
-    const fwIdx = whereOutput.indexOf(normalizedRoot);
-    const idx = bsIdx !== -1 ? bsIdx : fwIdx;
-    if (idx === -1) continue;
-
-    const localPath = normalizePath(whereOutput.slice(idx));
+  let currentDepot = "";
+  let currentPath = "";
+
+  for (const line of whereResult.stdout.split("\n")) {
+    const tagMatch = line.match(/^\.\.\. (\w+) (.+)/);
+    if (!tagMatch) {
+      if (currentDepot && currentPath) {
+        const localPath = normalizePath(currentPath);
+        const relativePath = toRelativePath(localPath, normalizedRoot);
+        result.set(currentDepot, { localPath, relativePath });
+      }
+      currentDepot = "";
+      currentPath = "";
+      continue;
+    }
+
+    const [, field, value] = tagMatch;
+    if (field === "depotFile") currentDepot = value;
+    if (field === "path") currentPath = value;
+  }
+
+  // Handle last record (no trailing blank line)
+  if (currentDepot && currentPath) {
+    const localPath = normalizePath(currentPath);
     const relativePath = toRelativePath(localPath, normalizedRoot);
-    result.set(depotPaths[i], { localPath, relativePath });
+    result.set(currentDepot, { localPath, relativePath });
   }
 
   return result;