diff --git a/packages/app/src/utils/tool-call-parsers.test.ts b/packages/app/src/utils/tool-call-parsers.test.ts index c02f957a07..16c2f57f5b 100644 --- a/packages/app/src/utils/tool-call-parsers.test.ts +++ b/packages/app/src/utils/tool-call-parsers.test.ts @@ -21,6 +21,36 @@ describe("tool-call-parsers", () => { expect(parsed.find((entry) => entry.type === "add")?.content).toBe("+new"); }); + it("keeps file headers in multi-file diffs", () => { + const parsed = parseUnifiedDiff( + [ + "diff --git a/alpha.txt b/alpha.txt", + "--- a/alpha.txt", + "+++ b/alpha.txt", + "@@ -1 +1 @@", + "-color = blue", + "+color = green", + "diff --git a/beta.txt b/beta.txt", + "--- a/beta.txt", + "+++ b/beta.txt", + "@@ -1 +1 @@", + "-size = small", + "+size = large", + ].join("\n"), + ); + + expect(parsed).toEqual([ + { type: "header", content: "diff --git a/alpha.txt b/alpha.txt" }, + { type: "header", content: "@@ -1 +1 @@" }, + { type: "remove", content: "-color = blue" }, + { type: "add", content: "+color = green" }, + { type: "header", content: "diff --git a/beta.txt b/beta.txt" }, + { type: "header", content: "@@ -1 +1 @@" }, + { type: "remove", content: "-size = small" }, + { type: "add", content: "+size = large" }, + ]); + }); + it("extracts TodoWrite task entries", () => { const tasks = extractTaskEntriesFromToolCall("TodoWrite", { todos: [ diff --git a/packages/app/src/utils/tool-call-parsers.ts b/packages/app/src/utils/tool-call-parsers.ts index 1e89e2b303..7980d3ee5f 100644 --- a/packages/app/src/utils/tool-call-parsers.ts +++ b/packages/app/src/utils/tool-call-parsers.ts @@ -210,6 +210,8 @@ export function parseUnifiedDiff(diffText?: string): DiffLine[] { } const lines = splitIntoLines(diffText); + const fileHeaders = lines.filter((line) => line.startsWith("diff --git ")); + const hasMultipleFiles = fileHeaders.length > 1; const diff: DiffLine[] = []; for (const line of lines) { @@ -218,7 +220,8 @@ export function parseUnifiedDiff(diffText?: string): DiffLine[] { continue; } - if (line.startsWith("@@")) { + const isFileHeader = hasMultipleFiles && line.startsWith("diff --git "); + if (line.startsWith("@@") || isFileHeader) { diff.push({ type: "header", content: line }); continue; } diff --git a/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts b/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts index 8fca5f24c0..0ded0806a0 100644 --- a/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts +++ b/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts @@ -3007,6 +3007,87 @@ describe("Codex app-server provider", () => { } }); + test.each([ + ["patch_apply_begin", undefined, "running", null], + ["patch_apply_end", true, "completed", null], + ["patch_apply_end", false, "failed", { message: "Cannot apply patch" }], + ] as const)( + "maps both files from legacy %s notifications (success: %s)", + async (type, success, status, error) => { + const appServer = createFakeCodexAppServer(); + const session = new CodexAppServerAgentSession( + createConfig({ cwd: "/tmp/repo" }), + null, + createTestLogger(), + async () => appServer.child, + ); + try { + await session.connect(); + const nextTimelineItem = waitForNextTimelineItem(session); + appServer.child.stdout.write( + `${JSON.stringify({ + method: `codex/event/${type}`, + params: { + conversationId: "thread-1", + msg: { + type, + call_id: "patch-array-alias", + changes: [ + { + path: "/tmp/repo/src/array-alias.ts", + kind: "modify", + unified_diff: "@@\n-old\n+new\n", + }, + { + path: "/tmp/repo/src/second.ts", + kind: "modify", + unified_diff: "@@\n-before\n+after\n", + }, + ], + success, + stderr: "Cannot apply patch", + }, + }, + })}\n`, + ); + + await expect(nextTimelineItem).resolves.toEqual({ + type: "timeline", + provider: "codex", + item: { + type: "tool_call", + callId: "patch-array-alias", + name: "apply_patch", + status, + error, + detail: { + type: "edit", + filePath: "src/array-alias.ts", + unifiedDiff: [ + "diff --git a/src/array-alias.ts b/src/array-alias.ts", + "--- a/src/array-alias.ts", + "+++ b/src/array-alias.ts", + "@@", + "-old", + "+new", + "", + "diff --git a/src/second.ts b/src/second.ts", + "--- a/src/second.ts", + "+++ b/src/second.ts", + "@@", + "-before", + "+after", + ].join("\n"), + }, + }, + }); + appServer.assertNoErrors(); + } finally { + await session.close(); + } + }, + ); + test("maps Codex plan markdown to a synthetic plan tool call", () => { const item = mapCodexPlanToToolCall({ callId: "plan-turn-1", diff --git a/packages/server/src/server/agent/providers/codex-app-server-agent.ts b/packages/server/src/server/agent/providers/codex-app-server-agent.ts index 62ceb14c70..e5ba236645 100644 --- a/packages/server/src/server/agent/providers/codex-app-server-agent.ts +++ b/packages/server/src/server/agent/providers/codex-app-server-agent.ts @@ -55,11 +55,13 @@ import { composeSystemPromptParts } from "../system-prompt.js"; import { curateAgentActivity } from "../activity-curator.js"; import { CodexAsyncQuestions, codexAsyncQuestionToTimeline } from "./codex/async-questions.js"; import { + fileChangeToUnifiedDiff, mapCodexToolCallEnvelope, mapCodexToolCallFromThreadItem, normalizeCommandExecutionCommand, splitCodexMcpToolResultImages, } from "./codex/tool-call-mapper.js"; +import { normalizeCodexFilePath } from "./codex/tool-call-detail-parser.js"; import { checkProviderLaunchAvailable, createProviderEnv, @@ -1664,7 +1666,18 @@ export function mapCodexPatchNotificationToToolCall(params: { const firstPatchText = files .map((file) => file.content?.trim()) .find((value): value is string => typeof value === "string" && value.length > 0); - const patchText = firstPatchText; + const patchText = + files.length > 1 + ? files + .map((file) => + fileChangeToUnifiedDiff({ + path: normalizeCodexFilePath(file.path, params.cwd) ?? file.path, + kind: file.kind, + diff: file.content, + }), + ) + .join("\n\n") + : firstPatchText; const patchFields = codexPatchTextFields(patchText); const mapped = mapCodexToolCallEnvelope({ callId: params.callId ?? null, 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 566fd10083..9b2ea095e4 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 @@ -528,6 +528,115 @@ describe("codex tool-call mapper", () => { }); }); + it.each([ + ["inProgress", "running"], + ["completed", "completed"], + ])("keeps both fileChange diffs when the patch is %s", (status, expectedStatus) => { + const item = mapCodexToolCallFromThreadItem( + { + type: "fileChange", + id: "codex-two-files", + status, + changes: [ + { + path: "/tmp/repo/alpha.txt", + kind: { type: "update", move_path: null }, + diff: "@@ -1 +1 @@\n-color = blue\n+color = green\n", + }, + { + path: "/tmp/repo/beta.txt", + kind: { type: "update", move_path: null }, + diff: "@@ -1 +1 @@\n-size = small\n+size = large\n", + }, + ], + }, + { cwd: "/tmp/repo" }, + ); + + expect(item).toEqual({ + type: "tool_call", + callId: "codex-two-files", + name: "apply_patch", + status: expectedStatus, + error: null, + detail: { + type: "edit", + filePath: "alpha.txt", + unifiedDiff: [ + "diff --git a/alpha.txt b/alpha.txt", + "--- a/alpha.txt", + "+++ b/alpha.txt", + "@@ -1 +1 @@", + "-color = blue", + "+color = green", + "", + "diff --git a/beta.txt b/beta.txt", + "--- a/beta.txt", + "+++ b/beta.txt", + "@@ -1 +1 @@", + "-size = small", + "+size = large", + ].join("\n"), + }, + }); + }); + + it("keeps update, add, and delete changes in one fileChange diff", () => { + const item = mapCodexToolCallFromThreadItem( + { + type: "fileChange", + id: "codex-mixed-files", + status: "completed", + changes: [ + { + path: "/tmp/repo/alpha.txt", + kind: { type: "update", move_path: null }, + diff: "--- a/alpha.txt\n+++ b/alpha.txt\n@@ -1 +1 @@\n-blue\n+green\n", + }, + { path: "/tmp/repo/new.txt", kind: { type: "add" }, diff: "new\n\nvalue\n" }, + { path: "/tmp/repo/old.txt", kind: { type: "delete" }, diff: "old\n\nvalue\n" }, + ], + }, + { cwd: "/tmp/repo" }, + ); + + expect(item).toEqual({ + type: "tool_call", + callId: "codex-mixed-files", + name: "apply_patch", + status: "completed", + error: null, + detail: { + type: "edit", + filePath: "alpha.txt", + unifiedDiff: [ + "diff --git a/alpha.txt b/alpha.txt", + "--- a/alpha.txt", + "+++ b/alpha.txt", + "@@ -1 +1 @@", + "-blue", + "+green", + "", + "diff --git a/new.txt b/new.txt", + "--- /dev/null", + "+++ b/new.txt", + "@@ -0,0 +1,3 @@", + "+new", + "+", + "+value", + "", + "diff --git a/old.txt b/old.txt", + "--- a/old.txt", + "+++ /dev/null", + "@@ -1,3 +0,0 @@", + "-old", + "-", + "-value", + ].join("\n"), + }, + }); + }); + it("maps fileChange object-style change payloads keyed by path", () => { const item = expectMapped( mapCodexToolCallFromThreadItem( 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 64629bc5d9..980684191f 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 @@ -400,7 +400,7 @@ function contentToDeletionDiff(filePath: string, content: string): 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); + const nonEmpty = lines.slice(0, lines.at(-1) === "" ? -1 : undefined); if (nonEmpty.length > 0) { output.push(`@@ -1,${nonEmpty.length} +0,0 @@`); for (const line of nonEmpty) { @@ -740,6 +740,9 @@ function parseFileChangePath( } function parseFileChangeKind(entry: Record): string | undefined { + if (isRecord(entry.kind) && typeof entry.kind.type === "string") { + return entry.kind.type; + } return ( (typeof entry.kind === "string" && entry.kind) || (typeof entry.type === "string" && entry.type) || @@ -839,6 +842,28 @@ function resolveFileChangeTextFields(file: CodexFileChangeEntry | undefined): { return asEditTextFields(file.diff); } +export function fileChangeToUnifiedDiff(file: CodexFileChangeEntry): string { + const { unifiedDiff, newString } = resolveFileChangeTextFields(file); + if (unifiedDiff?.startsWith("diff --git")) { + return unifiedDiff.replace(/\r?\n$/, ""); + } + const header = `diff --git a/${file.path} b/${file.path}`; + if (unifiedDiff?.startsWith("--- ")) { + return `${header}\n${unifiedDiff.replace(/\r?\n$/, "")}`; + } + const left = file.kind === "add" ? "/dev/null" : `a/${file.path}`; + const right = file.kind === "delete" ? "/dev/null" : `b/${file.path}`; + const lines = [header, `--- ${left}`, `+++ ${right}`]; + if (unifiedDiff) { + lines.push(unifiedDiff.replace(/\r?\n$/, "")); + } else if (newString) { + const contentLines = newString.replace(/\r\n/g, "\n").replace(/\n$/, "").split("\n"); + lines.push(`@@ -0,0 +1,${contentLines.length} @@`); + lines.push(...contentLines.map((line) => `+${line}`)); + } + return lines.join("\n"); +} + function mapFileChangeItem( item: z.infer, options?: CodexMapperOptions, @@ -873,17 +898,16 @@ function mapFileChangeItem( const error = item.error ?? null; const status = normalizeToolCallStatus(item.status, error, output); const firstFile = files[0]; - const firstTextFields = resolveFileChangeTextFields(firstFile); - const hasFirstTextFields = Object.keys(firstTextFields).length > 0; + const textFields = + files.length > 1 + ? { unifiedDiff: files.map(fileChangeToUnifiedDiff).join("\n\n") } + : resolveFileChangeTextFields(firstFile); + const hasTextFields = Object.keys(textFields).length > 0; const input = toNullableObject({ ...inputBase, - ...(firstFile?.path && hasFirstTextFields ? { path: firstFile.path } : {}), - ...(hasFirstTextFields && firstTextFields.unifiedDiff - ? { patch: firstTextFields.unifiedDiff } - : {}), - ...(hasFirstTextFields && firstTextFields.newString - ? { content: firstTextFields.newString } - : {}), + ...(firstFile?.path && hasTextFields ? { path: firstFile.path } : {}), + ...(hasTextFields && textFields.unifiedDiff ? { patch: textFields.unifiedDiff } : {}), + ...(hasTextFields && textFields.newString ? { content: textFields.newString } : {}), }); return {