fix(codex): retain multi-file patch diffs in timeline details

Combine each file's diff in canonical file-change items and legacy patch
text, keeping the legacy parsers and envelope unchanged. Cover both legacy
lifecycle events and the failed status.
This commit is contained in:
Okan YILDIRIM
2026-10-23 11:19:33 +00:00
parent f3b0199c13
commit 179c3f7f62
6 changed files with 272 additions and 12 deletions
@@ -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: [
+4 -1
View File
@@ -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;
}
@@ -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",
@@ -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,
@@ -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(
@@ -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, unknown>): 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<typeof CodexFileChangeItemSchema>,
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 {