Keep Codex multi-file patch diffs in the timeline (#8613)

* 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.

* fix(codex): handle path-only and moved files in patch diffs

Multi-file assembly generated headers without patch text and discarded
move destinations when reducing object kind tags. Skip textless entries
and retain move_path for generated destination headers, keeping the
single-file mapping and existing truncation limit.

Add mapper regressions for both cases, demonstrated failing on 179c3f7f6
before the fix.

* fix(codex): keep later files in bounded patch previews

Share the existing diff detail budget across files so a large first diff cannot hide later changes in canonical or legacy patch events.

Co-authored-by: Okan YILDIRIM <63511519+ragokan@users.noreply.github.com>

* fix(codex): preserve patch preview labels and move destinations

Keep complete file and hunk labels within the preview budget and count files omitted for space. Select the primary file from rendered content and retain move destinations in path-keyed and legacy events.

* fix(codex): bound generated file additions without argument overflow

* fix(codex): retain move-only files in multi-file previews

Represent moves without content changes as rename metadata and remove the provider move notice before classifying diff contents.

* fix(codex): keep move normalization inside multi-file previews

Recognize the provider move notice only while assembling multi-file diffs so the existing single-file path continues to retain its destination.

* fix(codex): retain empty additions in multi-file previews

---------

Co-authored-by: Okan YILDIRIM <63511519+ragokan@users.noreply.github.com>
Co-authored-by: paseo-bot[bot] <266920839+paseo-bot[bot]@users.noreply.github.com>
This commit is contained in:
paseo-bot[bot]
2026-11-03 04:06:44 +00:00
committed by GitHub
co-authored by Okan YILDIRIM
parent 336e7d1b89
commit 66e1523f2f
7 changed files with 746 additions and 17 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;
}
@@ -3436,6 +3436,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",
@@ -7879,3 +7960,93 @@ test("publishes a materialized image before dispatching the following turn compl
await session.close();
}
});
describe("legacy multi-file diff budget", () => {
test.each([true, false])(
"retains later file diffs when a large patch is running: %s",
(running) => {
const item = mapCodexPatchNotificationToToolCall({
callId: "large-legacy-patch",
cwd: "/repo",
running,
success: true,
changes: [
{
path: "/repo/alpha.txt",
kind: "modify",
unified_diff: "@@ -1 +1 @@\n-old\n+" + "a".repeat(14000),
},
{ path: "/repo/beta.txt", kind: "modify", unified_diff: "@@ -1 +1 @@\n-small\n+large\n" },
],
});
expect(item?.detail.type).toBe("edit");
if (item?.detail.type !== "edit") throw new Error("Expected edit detail");
expect(item.detail.unifiedDiff).toContain("diff --git a/beta.txt b/beta.txt");
expect(item.detail.unifiedDiff).toContain("+large");
expect(item.detail.unifiedDiff!.length).toBeLessThanOrEqual(12000);
},
);
});
describe("legacy multi-file move metadata", () => {
test.each([true, false])("preserves path-keyed moves while running: %s", (running) => {
const item = mapCodexPatchNotificationToToolCall({
callId: "legacy-move",
cwd: "/repo",
running,
success: true,
changes: {
"/repo/old.txt": {
type: "update",
move_path: "/repo/new.txt",
unified_diff: "@@ -1 +1 @@\n-old\n+new\n",
},
"/repo/other.txt": { type: "update", unified_diff: "@@ -1 +1 @@\n-before\n+after\n" },
},
});
if (item?.detail.type !== "edit") throw new Error("Expected edit");
expect(item.detail.unifiedDiff).toContain("diff --git a/old.txt b/new.txt");
expect(item.detail.unifiedDiff).toContain("+++ b/new.txt");
});
});
describe("legacy move-only entries", () => {
test.each([true, false])(
"keeps a move-only file alongside another edit while running: %s",
(running) => {
const item = mapCodexPatchNotificationToToolCall({
callId: "legacy-move-only",
cwd: "/repo",
running,
success: true,
changes: {
"/repo/old.txt": { type: "update", move_path: "/repo/new.txt", unified_diff: "" },
"/repo/second.txt": { type: "update", unified_diff: "@@ -1 +1 @@\n-small\n+large\n" },
},
});
if (item?.detail.type !== "edit") throw new Error("Expected edit");
expect(item.detail.unifiedDiff).toContain("rename from old.txt\nrename to new.txt");
expect(item.detail.unifiedDiff).toContain("diff --git a/second.txt b/second.txt");
},
);
});
describe("legacy empty additions", () => {
test.each([true, false])("keeps an empty new file while running: %s", (running) => {
const item = mapCodexPatchNotificationToToolCall({
callId: "legacy-empty",
cwd: "/repo",
running,
success: true,
changes: {
"/repo/empty.txt": { type: "add", content: "" },
"/repo/second.txt": { type: "update", unified_diff: "@@ -1 +1 @@\n-old\n+new\n" },
},
});
if (item?.detail.type !== "edit") throw new Error("Expected edit");
expect(item.detail.unifiedDiff).toContain(
"diff --git a/empty.txt b/empty.txt\n--- /dev/null\n+++ b/empty.txt",
);
expect(item.detail.unifiedDiff).toContain("diff --git a/second.txt b/second.txt");
});
});
@@ -55,11 +55,13 @@ import { renderPromptAttachmentAsText } from "../prompt-attachments.js";
import { composeSystemPromptParts } from "../system-prompt.js";
import { CodexAsyncQuestions, codexAsyncQuestionToTimeline } from "./codex/async-questions.js";
import {
fileChangesToPreview,
mapCodexToolCallEnvelope,
mapCodexToolCallFromThreadItem,
normalizeCommandExecutionCommand,
splitCodexMcpToolResultImages,
} from "./codex/tool-call-mapper.js";
import { normalizeCodexFilePath } from "./codex/tool-call-detail-parser.js";
import {
checkProviderLaunchAvailable,
createProviderEnv,
@@ -1402,6 +1404,15 @@ interface CodexPatchFileChange {
path: string;
kind?: string;
content?: string;
movePath?: string;
}
function codexPatchMovePath(value: unknown): string | undefined {
const record = toObjectRecord(value);
if (!record) return undefined;
const kind = toObjectRecord(record.kind);
const destination = record.move_path ?? kind?.move_path;
return typeof destination === "string" ? destination : undefined;
}
function extractPatchLikeText(value: unknown): string | undefined {
@@ -1534,6 +1545,7 @@ function parseCodexPatchChanges(changes: unknown): CodexPatchFileChange[] {
(typeof record.type === "string" && record.type) ||
undefined,
content: extractPatchLikeText(record),
movePath: codexPatchMovePath(record),
};
})
.filter((entry): entry is CodexPatchFileChange => entry !== null);
@@ -1553,6 +1565,7 @@ function parseCodexPatchChanges(changes: unknown): CodexPatchFileChange[] {
(typeof recordChanges.type === "string" && recordChanges.type) ||
undefined,
content: extractPatchLikeText(recordChanges),
movePath: codexPatchMovePath(recordChanges),
},
];
}
@@ -1572,6 +1585,7 @@ function parseCodexPatchChanges(changes: unknown): CodexPatchFileChange[] {
? ((value as { type?: string }).type ?? undefined)
: undefined,
content: extractPatchLikeText(value),
movePath: codexPatchMovePath(value),
};
})
.filter((entry): entry is CodexPatchFileChange => entry !== null);
@@ -1680,6 +1694,31 @@ function mapCodexExecNotificationToToolCall(params: {
return params.running ? toRunningToolCall(mapped) : mapped;
}
function codexPatchPreview(
files: CodexPatchFileChange[],
cwd?: string | null,
): {
filePath: string | undefined;
patchText: string | undefined;
} {
const preview =
files.length > 1
? fileChangesToPreview(
files.map((file) => ({
path: normalizeCodexFilePath(file.path, cwd) ?? file.path,
kind: file.kind,
diff: file.content,
movePath: file.movePath ? normalizeCodexFilePath(file.movePath, cwd) : undefined,
})),
)
: undefined;
const firstPath = preview?.filePath ?? files[0]?.path;
const firstPatchText = files
.map((file) => file.content?.trim())
.find((value): value is string => typeof value === "string" && value.length > 0);
return { filePath: firstPath, patchText: preview?.unifiedDiff ?? firstPatchText };
}
export function mapCodexPatchNotificationToToolCall(params: {
callId?: string | null;
changes: unknown;
@@ -1690,11 +1729,7 @@ export function mapCodexPatchNotificationToToolCall(params: {
running: boolean;
}): ToolCallTimelineItem | null {
const files = parseCodexPatchChanges(params.changes);
const firstPath = files[0]?.path;
const firstPatchText = files
.map((file) => file.content?.trim())
.find((value): value is string => typeof value === "string" && value.length > 0);
const patchText = firstPatchText;
const { filePath: firstPath, patchText } = codexPatchPreview(files, params.cwd);
const patchFields = codexPatchTextFields(patchText);
const mapped = mapCodexToolCallEnvelope({
callId: params.callId ?? null,
@@ -528,6 +528,165 @@ 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("uses the move destination in generated multi-file diff headers", () => {
const item = mapCodexToolCallFromThreadItem(
{
type: "fileChange",
id: "codex-moved-file",
status: "completed",
changes: [
{
path: "/tmp/repo/old.txt",
kind: { type: "update", move_path: "/tmp/repo/new.txt" },
diff: "@@ -1 +1 @@\n-old\n+new\n",
},
{
path: "/tmp/repo/other.txt",
kind: { type: "update", move_path: null },
diff: "@@ -1 +1 @@\n-before\n+after\n",
},
],
},
{ cwd: "/tmp/repo" },
);
expect(item).toEqual({
type: "tool_call",
callId: "codex-moved-file",
name: "apply_patch",
status: "completed",
error: null,
detail: {
type: "edit",
filePath: "old.txt",
unifiedDiff: [
"diff --git a/old.txt b/new.txt",
"--- a/old.txt",
"+++ b/new.txt",
"@@ -1 +1 @@",
"-old",
"+new",
"",
"diff --git a/other.txt b/other.txt",
"--- a/other.txt",
"+++ b/other.txt",
"@@ -1 +1 @@",
"-before",
"+after",
].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(
@@ -827,6 +986,44 @@ describe("codex tool-call mapper", () => {
});
});
it("maps multi-file path-only updates to unknown detail", () => {
const item = mapCodexToolCallFromThreadItem(
{
type: "fileChange",
id: "codex-multi-file-path-only",
status: "completed",
changes: [
{ path: "/tmp/repo/first.ts", kind: "modify" },
{ path: "/tmp/repo/second.ts", kind: { type: "update", move_path: null } },
],
},
{ cwd: "/tmp/repo" },
);
expect(item).toEqual({
type: "tool_call",
callId: "codex-multi-file-path-only",
name: "apply_patch",
status: "completed",
error: null,
detail: {
type: "unknown",
input: {
files: [
{ path: "first.ts", kind: "modify" },
{ path: "second.ts", kind: "update" },
],
},
output: {
files: [
{ path: "first.ts", kind: "modify" },
{ path: "second.ts", kind: "update" },
],
},
},
});
});
it("maps path-only apply_patch tool-call payloads to unknown detail instead of empty edit detail", () => {
const item = expectMapped(
mapCodexToolCallEnvelope({
@@ -1102,3 +1299,186 @@ describe("codex tool-call mapper", () => {
`);
});
});
describe("multi-file diff budget", () => {
it("retains a labeled diff for later files when the first diff exceeds the detail limit", () => {
const item = mapCodexToolCallFromThreadItem(
{
type: "fileChange",
id: "large-multi-file",
status: "completed",
changes: [
{
path: "/repo/alpha.txt",
kind: "modify",
diff: "@@ -1 +1 @@\n-old\n+" + "a".repeat(14000),
},
{ path: "/repo/beta.txt", kind: "modify", diff: "@@ -1 +1 @@\n-small\n+large\n" },
],
},
{ cwd: "/repo" },
);
expect(item?.detail.type).toBe("edit");
if (item?.detail.type !== "edit") throw new Error("Expected edit detail");
expect(item.detail.unifiedDiff).toContain("diff --git a/alpha.txt b/alpha.txt");
expect(item.detail.unifiedDiff).toContain("diff --git a/beta.txt b/beta.txt");
expect(item.detail.unifiedDiff).toContain("+large");
expect(item.detail.unifiedDiff!.length).toBeLessThanOrEqual(12000);
});
});
describe("multi-file preview metadata", () => {
it("preserves a path-keyed move destination", () => {
const item = mapCodexToolCallFromThreadItem(
{
type: "fileChange",
id: "moved-keyed",
status: "completed",
changes: {
"/repo/old.txt": {
type: "update",
move_path: "/repo/new.txt",
unified_diff: "@@ -1 +1 @@\n-old\n+new\n",
},
"/repo/other.txt": { type: "update", unified_diff: "@@ -1 +1 @@\n-before\n+after\n" },
},
},
{ cwd: "/repo" },
);
expect(item?.detail.type).toBe("edit");
if (item?.detail.type !== "edit") throw new Error("Expected edit");
expect(item.detail.unifiedDiff).toContain("diff --git a/old.txt b/new.txt");
expect(item.detail.unifiedDiff).toContain("+++ b/new.txt");
});
it("names the file whose diff is shown when the first entry has no content", () => {
const item = mapCodexToolCallFromThreadItem(
{
type: "fileChange",
id: "partial-content",
status: "completed",
changes: [
{ path: "/repo/first.txt", kind: "modify" },
{ path: "/repo/second.txt", kind: "modify", diff: "@@ -1 +1 @@\n-old\n+new\n" },
],
},
{ cwd: "/repo" },
);
expect(item?.detail).toMatchObject({ type: "edit", filePath: "second.txt" });
});
it("keeps whole labels and marks omitted files when many long paths exceed the preview", () => {
const paths = Array.from(
{ length: 50 },
(_, index) => `src/${String(index).padStart(2, "0")}-${"x".repeat(108)}.txt`,
);
const item = mapCodexToolCallFromThreadItem({
type: "fileChange",
id: "many-files",
status: "completed",
changes: paths.map((path) => ({ path, kind: "modify", diff: "@@ -1 +1 @@\n-old\n+new\n" })),
});
if (item?.detail.type !== "edit") throw new Error("Expected edit");
const diff = item.detail.unifiedDiff!;
const headers = diff.split("\n").filter((line) => line.startsWith("diff --git "));
expect(headers.length).toBeGreaterThan(0);
const expectedHeaders = paths.map((path) => `diff --git a/${path} b/${path}`);
expect(expectedHeaders).toEqual(expect.arrayContaining(headers));
expect(diff).toContain("-old\n+new");
expect(diff).toContain(`${paths.length - headers.length} more files omitted`);
expect(diff.length).toBeLessThanOrEqual(12000);
});
});
describe("large multi-file additions", () => {
it("bounds a generated addition without throwing before later files are mapped", () => {
const item = mapCodexToolCallFromThreadItem(
{
type: "fileChange",
id: "large-addition",
status: "completed",
changes: [
{ path: "/repo/generated.txt", kind: { type: "add" }, diff: "x\n".repeat(150000) },
{ path: "/repo/second.txt", kind: { type: "add" }, diff: "second\n" },
],
},
{ cwd: "/repo" },
);
if (item?.detail.type !== "edit") throw new Error("Expected edit");
expect(item.status).toBe("completed");
expect(item.detail.unifiedDiff).toContain("diff --git a/generated.txt b/generated.txt");
expect(item.detail.unifiedDiff).toContain("diff --git a/second.txt b/second.txt");
expect(item.detail.unifiedDiff).toContain("+second");
expect(item.detail.unifiedDiff!.length).toBeLessThanOrEqual(12000);
});
});
describe("move-only entries in multi-file previews", () => {
it("shows a move without manufacturing added file contents from provider metadata", () => {
const item = mapCodexToolCallFromThreadItem(
{
type: "fileChange",
id: "move-only",
status: "completed",
changes: [
{
path: "/repo/old.txt",
kind: { type: "update", move_path: "/repo/new.txt" },
diff: "\n\nMoved to: /repo/new.txt",
},
{ path: "/repo/second.txt", kind: "modify", diff: "@@ -1 +1 @@\n-small\n+large\n" },
],
},
{ cwd: "/repo" },
);
if (item?.detail.type !== "edit") throw new Error("Expected edit");
expect(item.detail.unifiedDiff).toContain("rename from old.txt\nrename to new.txt");
expect(item.detail.unifiedDiff).toContain("diff --git a/second.txt b/second.txt");
expect(item.detail.unifiedDiff).not.toContain("+Moved to:");
expect(item.detail.unifiedDiff).not.toContain("@@ -0,0");
});
});
describe("single-file move compatibility", () => {
it("retains the destination outside the multi-file preview path", () => {
const item = mapCodexToolCallFromThreadItem(
{
type: "fileChange",
id: "single-move",
status: "completed",
changes: [
{
path: "/repo/old.txt",
kind: { type: "update", move_path: "/repo/new.txt" },
diff: "\n\nMoved to: /repo/new.txt",
},
],
},
{ cwd: "/repo" },
);
expect(JSON.stringify(item)).toContain("new.txt");
});
});
describe("empty additions in multi-file previews", () => {
it.each(["inProgress", "completed"])(
"keeps an empty created file when the patch is %s",
(status) => {
const item = mapCodexToolCallFromThreadItem(
{
type: "fileChange",
id: "empty-addition",
status,
changes: [
{ path: "/repo/empty.txt", kind: { type: "add" }, diff: "" },
{ path: "/repo/second.txt", kind: "modify", diff: "@@ -1 +1 @@\n-old\n+new\n" },
],
},
{ cwd: "/repo" },
);
if (item?.detail.type !== "edit") throw new Error("Expected edit");
expect(item.detail.unifiedDiff).toContain(
"diff --git a/empty.txt b/empty.txt\n--- /dev/null\n+++ b/empty.txt",
);
expect(item.detail.unifiedDiff).toContain("diff --git a/second.txt b/second.txt");
},
);
});
@@ -4,6 +4,7 @@ import type { ToolCallTimelineItem } from "../../agent-sdk-types.js";
import {
extractCodexShellOutput,
normalizeToolCallStatus,
TOOL_CALL_DIFF_MAX_CHARS,
truncateDiffText,
} from "../tool-call-mapper-utils.js";
import { deriveCodexToolDetail, normalizeCodexFilePath } from "./tool-call-detail-parser.js";
@@ -400,7 +401,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) {
@@ -713,6 +714,7 @@ interface CodexFileChangeEntry {
path: string;
kind?: string;
diff?: string;
movePath?: string;
}
function parseFileChangePath(
@@ -740,6 +742,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) ||
@@ -767,10 +772,14 @@ function toFileChangeEntry(
if (!path) {
return null;
}
const kind = isRecord(entry.kind) ? entry.kind : undefined;
const movePath = entry.move_path ?? kind?.move_path;
return {
path,
kind: parseFileChangeKind(entry),
diff: parseFileChangeDiff(entry),
movePath:
typeof movePath === "string" ? normalizeCodexFilePath(movePath, options?.cwd) : undefined,
};
}
@@ -839,6 +848,105 @@ function resolveFileChangeTextFields(file: CodexFileChangeEntry | undefined): {
return asEditTextFields(file.diff);
}
function fileChangeToUnifiedDiff(file: CodexFileChangeEntry): string {
// Codex formats a content-preserving move as a notice, not added file text.
const isMoveOnlyNotice = file.movePath && file.diff?.startsWith("\n\nMoved to: ");
const { unifiedDiff, newString } = isMoveOnlyNotice ? {} : resolveFileChangeTextFields(file);
if (!unifiedDiff && !newString) {
if (file.kind === "add") {
return [
`diff --git a/${file.path} b/${file.path}`,
"--- /dev/null",
`+++ b/${file.path}`,
].join("\n");
}
return file.movePath
? [
`diff --git a/${file.path} b/${file.movePath}`,
`rename from ${file.path}`,
`rename to ${file.movePath}`,
].join("\n")
: "";
}
if (unifiedDiff?.startsWith("diff --git")) {
return unifiedDiff.replace(/\r?\n$/, "");
}
const destination = file.movePath ?? file.path;
const header = `diff --git a/${file.path} b/${destination}`;
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/${destination}`;
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} @@`);
for (const line of contentLines) lines.push(`+${line}`);
}
return lines.join("\n");
}
function splitFileDiffPreview(diff: string): { header: string; body: string } {
const hunk = diff.indexOf("\n@@");
const bodyStart = hunk < 0 ? -1 : diff.indexOf("\n", hunk + 1);
return bodyStart < 0
? { header: diff, body: "" }
: { header: diff.slice(0, bodyStart + 1), body: diff.slice(bodyStart + 1) };
}
export function fileChangesToPreview(files: CodexFileChangeEntry[]): {
filePath: string | undefined;
unifiedDiff: string;
} {
const diffs = files
.map((file) => ({ filePath: file.path, diff: fileChangeToUnifiedDiff(file) }))
.filter((file) => file.diff.length > 0);
const filePath = diffs[0]?.filePath ?? files[0]?.path;
const combined = diffs.map((file) => file.diff).join("\n\n");
if (combined.length <= TOOL_CALL_DIFF_MAX_CHARS) return { filePath, unifiedDiff: combined };
const marker = "\n...[truncated]";
const omittedNotice = (count: number) =>
count > 0 ? `\n\n...[${count} more files omitted]` : "";
const previews = diffs.map((file) => splitFileDiffPreview(file.diff));
const visible: typeof previews = [];
let minimumSize = 0;
for (const preview of previews) {
// Keep whole labels and a useful body excerpt, or explicitly omit the file.
const required =
preview.header.length + Math.max(marker.length, Math.min(80, preview.body.length));
const nextSize = minimumSize + required + (visible.length > 0 ? 2 : 0);
if (
nextSize + omittedNotice(previews.length - visible.length - 1).length >
TOOL_CALL_DIFF_MAX_CHARS
)
break;
visible.push(preview);
minimumSize = nextSize;
}
const omitted = omittedNotice(previews.length - visible.length);
const headersSize = visible.reduce((size, preview) => size + preview.header.length, 0);
const bodyBudget = Math.floor(
(TOOL_CALL_DIFF_MAX_CHARS -
headersSize -
Math.max(0, visible.length - 1) * 2 -
omitted.length) /
Math.max(1, visible.length),
);
const unifiedDiff =
visible
.map(
({ header, body }) =>
header +
(body.length <= bodyBudget ? body : body.slice(0, bodyBudget - marker.length) + marker),
)
.join("\n\n") + omitted;
return { filePath, unifiedDiff };
}
function mapFileChangeItem(
item: z.infer<typeof CodexFileChangeItemSchema>,
options?: CodexMapperOptions,
@@ -873,17 +981,17 @@ 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 preview = files.length > 1 ? fileChangesToPreview(files) : undefined;
const filePath = preview?.filePath ?? firstFile?.path;
const textFields = preview
? { unifiedDiff: preview.unifiedDiff }
: resolveFileChangeTextFields(firstFile);
const hasTextFields = Boolean(textFields.unifiedDiff || textFields.newString);
const input = toNullableObject({
...inputBase,
...(firstFile?.path && hasFirstTextFields ? { path: firstFile.path } : {}),
...(hasFirstTextFields && firstTextFields.unifiedDiff
? { patch: firstTextFields.unifiedDiff }
: {}),
...(hasFirstTextFields && firstTextFields.newString
? { content: firstTextFields.newString }
: {}),
...(filePath && hasTextFields ? { path: filePath } : {}),
...(hasTextFields && textFields.unifiedDiff ? { patch: textFields.unifiedDiff } : {}),
...(hasTextFields && textFields.newString ? { content: textFields.newString } : {}),
});
return {
@@ -224,9 +224,11 @@ export function stripReadLineNumberGutter(
return { content: stripped.join("\n"), startLine };
}
export const TOOL_CALL_DIFF_MAX_CHARS = 12_000;
export function truncateDiffText(
text: string | undefined,
maxChars: number = 12_000,
maxChars: number = TOOL_CALL_DIFF_MAX_CHARS,
): string | undefined {
if (typeof text !== "string") {
return undefined;