fix(server): translate Codex file deletions as removed lines instead of added lines

Codex delete operations were displayed as edits with green (added) lines because the translation pipeline didn't distinguish deletes from edits. Now detects kind="delete" and *** Delete File directives, producing proper unified diffs with removed lines and +++ /dev/null headers.
2026-03-26 17:53:57 +07:00
parent f13178a9ad
commit a2aac3797f
2 changed files with 169 additions and 7 deletions
@@ -579,4 +579,127 @@ describe("codex tool-call mapper", () => {

    expect(item).toBeNull();
  });
+
+  it("maps apply_patch with Delete File directive into edit detail with removed lines", () => {
+    const patch = [
+      "*** Begin Patch",
+      "*** Delete File: /tmp/repo/src/dead-module.ts",
+      "*** End Patch",
+    ].join("\n");
+    const item = expectMapped(
+      mapCodexRolloutToolCall({
+        callId: "codex-delete-rollout",
+        name: "apply_patch",
+        input: patch,
+        output:
+          '{"output":"Success. Updated the following files:\\nD /tmp/repo/src/dead-module.ts\\n"}',
+        cwd: "/tmp/repo",
+      }),
+    );
+
+    expect(item.status).toBe("completed");
+    expect(item.detail.type).toBe("edit");
+    if (item.detail.type === "edit") {
+      expect(item.detail.filePath).toBe("src/dead-module.ts");
+      expect(item.detail.unifiedDiff).toContain("/dev/null");
+    }
+  });
+
+  it("maps multi-file apply_patch with update + delete into edit detail referencing the deleted file", () => {
+    // Exact data shape from real Codex rollout: update one file, delete another
+    const patch = [
+      "*** Begin Patch",
+      "*** Update File: /tmp/repo/src/app/index.tsx",
+      "@@",
+      ' import { useEffect } from "react";',
+      '-import { WELCOME_ROUTE } from "@/app-support/index-startup";',
+      "+",
+      '+const WELCOME_ROUTE = "/welcome";',
+      "*** Delete File: /tmp/repo/src/app-support/index-startup.ts",
+      "*** End Patch",
+    ].join("\n");
+
+    const item = expectMapped(
+      mapCodexRolloutToolCall({
+        callId: "codex-delete-multi",
+        name: "apply_patch",
+        input: patch,
+        output: JSON.stringify({
+          output:
+            "Success. Updated the following files:\nM /tmp/repo/src/app/index.tsx\nD /tmp/repo/src/app-support/index-startup.ts\n",
+          metadata: { exit_code: 0, duration_seconds: 0.0 },
+        }),
+        cwd: "/tmp/repo",
+      }),
+    );
+
+    expect(item.detail.type).toBe("edit");
+    if (item.detail.type === "edit") {
+      // The unified diff should contain both file sections
+      const diff = item.detail.unifiedDiff ?? "";
+      // The update file section should have normal diff lines
+      expect(diff).toContain("-import");
+      expect(diff).toContain("+const WELCOME_ROUTE");
+      // The delete file section should reference /dev/null
+      expect(diff).toContain("/dev/null");
+    }
+  });
+
+  it("maps fileChange delete with content as removed lines, not added lines", () => {
+    const item = mapCodexToolCallFromThreadItem(
+      {
+        type: "fileChange",
+        id: "codex-file-delete-with-content",
+        status: "completed",
+        changes: [
+          {
+            path: "/tmp/repo/src/dead-module.ts",
+            kind: "delete",
+            content: 'export const FOO = "bar";\nexport function hello() {}\n',
+          },
+        ],
+      },
+      { cwd: "/tmp/repo" },
+    );
+
+    expect(item).toBeTruthy();
+    expect(item?.detail.type).toBe("edit");
+    if (item?.detail.type === "edit") {
+      expect(item.detail.filePath).toBe("src/dead-module.ts");
+      const diff = item.detail.unifiedDiff ?? "";
+      // For a deletion, the content should appear as REMOVED lines (-)
+      // not as ADDED lines (+). This is the core bug.
+      expect(diff).toContain("/dev/null");
+      expect(diff).toContain("-export const FOO");
+      expect(diff).toContain("-export function hello");
+      // The content must NOT appear as added lines
+      expect(diff).not.toContain("+export const FOO");
+      expect(diff).not.toContain("+export function hello");
+    }
+  });
+
+  it("maps fileChange delete without content to edit detail with /dev/null marker", () => {
+    const item = mapCodexToolCallFromThreadItem(
+      {
+        type: "fileChange",
+        id: "codex-file-delete-no-content",
+        status: "completed",
+        changes: [
+          {
+            path: "/tmp/repo/src/dead-module.ts",
+            kind: "delete",
+          },
+        ],
+      },
+      { cwd: "/tmp/repo" },
+    );
+
+    expect(item).toBeTruthy();
+    // A delete without content should still produce a meaningful detail
+    if (item?.detail.type === "edit") {
+      expect(item.detail.filePath).toBe("src/dead-module.ts");
+      const diff = item.detail.unifiedDiff ?? "";
+      expect(diff).toContain("/dev/null");
+    }
+  });
 });
@@ -347,7 +347,7 @@ function normalizeDiffHeaderPath(rawPath: string): string {
 function codexApplyPatchToUnifiedDiff(text: string): string {
  const lines = text.replace(/\r\n/g, "\n").split("\n");
  const output: string[] = [];
-  let sawDiffBody = false;
+  let sawDiffContent = false;

  for (const line of lines) {
    const directive = parseCodexApplyPatchDirective(line);
@@ -362,6 +362,7 @@ function codexApplyPatchToUnifiedDiff(text: string): string {
        output.push(`diff --git a/${path} b/${path}`);
        output.push(`--- ${left}`);
        output.push(`+++ ${right}`);
+        sawDiffContent = true;
      }
      continue;
    }
@@ -378,22 +379,22 @@ function codexApplyPatchToUnifiedDiff(text: string): string {

    if (line.startsWith("@@")) {
      output.push(line);
-      sawDiffBody = true;
+      sawDiffContent = true;
      continue;
    }
    if (line.startsWith("+") || line.startsWith("-") || line.startsWith(" ")) {
      output.push(line);
-      sawDiffBody = true;
+      sawDiffContent = true;
      continue;
    }
    if (line.startsWith("\\ No newline at end of file")) {
      output.push(line);
-      sawDiffBody = true;
+      sawDiffContent = true;
      continue;
    }
  }

-  if (!sawDiffBody) {
+  if (!sawDiffContent) {
    return text;
  }

@@ -401,6 +402,22 @@ function codexApplyPatchToUnifiedDiff(text: string): string {
  return normalized.length > 0 ? normalized : text;
 }

+function contentToDeletionDiff(filePath: string, content: string): string {
+  const lines = content.replace(/\r\n/g, "\n").split("\n");
+  const output: string[] = [];
+  output.push(`diff --git a/${filePath} b/${filePath}`);
+  output.push(`--- a/${filePath}`);
+  output.push(`+++ /dev/null`);
+  const nonEmpty = lines.filter((l) => l.length > 0 || lines.indexOf(l) < lines.length - 1);
+  if (nonEmpty.length > 0) {
+    output.push(`@@ -1,${nonEmpty.length} +0,0 @@`);
+    for (const line of nonEmpty) {
+      output.push(`-${line}`);
+    }
+  }
+  return output.join("\n");
+}
+
 function classifyDiffLikeText(
  text: string,
 ): { isDiff: true; text: string } | { isDiff: false; text: string } {
@@ -716,6 +733,26 @@ function parseFileChangeEntries(
    .filter((entry): entry is CodexFileChangeEntry => entry !== null);
 }

+function resolveFileChangeTextFields(
+  file: CodexFileChangeEntry | undefined,
+): { unifiedDiff?: string; newString?: string } {
+  if (!file) {
+    return {};
+  }
+  const isDelete = file.kind === "delete";
+  if (isDelete && file.diff) {
+    const classified = classifyDiffLikeText(file.diff);
+    if (classified.isDiff) {
+      return { unifiedDiff: truncateDiffText(classified.text) };
+    }
+    return { unifiedDiff: truncateDiffText(contentToDeletionDiff(file.path, file.diff)) };
+  }
+  if (isDelete && !file.diff) {
+    return { unifiedDiff: contentToDeletionDiff(file.path, "") };
+  }
+  return asEditTextFields(file.diff);
+}
+
 function mapFileChangeItem(
  item: z.infer<typeof CodexFileChangeItemSchema>,
  options?: CodexMapperOptions,
@@ -739,7 +776,9 @@ function mapFileChangeItem(
          files: files.map((file) => ({
            path: file.path,
            ...(file.kind !== undefined ? { kind: file.kind } : {}),
-            ...asEditFileOutputFields(file.diff),
+            ...(file.kind === "delete"
+              ? { patch: resolveFileChangeTextFields(file).unifiedDiff }
+              : asEditFileOutputFields(file.diff)),
          })),
        }
      : {}),
@@ -749,7 +788,7 @@ function mapFileChangeItem(
  const error = item.error ?? null;
  const status = resolveStatus(item.status, error, output);
  const firstFile = files[0];
-  const firstTextFields = asEditTextFields(firstFile?.diff);
+  const firstTextFields = resolveFileChangeTextFields(firstFile);
  const hasFirstTextFields = Object.keys(firstTextFields).length > 0;
  const input = toNullableObject({
    ...inputBase,