diff --git a/packages/server/src/server/agent/providers/codex/tool-call-mapper.test.ts b/packages/server/src/server/agent/providers/codex/tool-call-mapper.test.ts index 98d5d53c9..65c4144d0 100644 --- a/packages/server/src/server/agent/providers/codex/tool-call-mapper.test.ts +++ b/packages/server/src/server/agent/providers/codex/tool-call-mapper.test.ts @@ -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"); + } + }); }); diff --git a/packages/server/src/server/agent/providers/codex/tool-call-mapper.ts b/packages/server/src/server/agent/providers/codex/tool-call-mapper.ts index acfe447c3..a5fd872f2 100644 --- a/packages/server/src/server/agent/providers/codex/tool-call-mapper.ts +++ b/packages/server/src/server/agent/providers/codex/tool-call-mapper.ts @@ -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, 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,