From de91f29b7e894c3875aa162f1396c1a022695a79 Mon Sep 17 00:00:00 2001 From: Alexandr Suhinin Date: Mon, 1 Jun 2026 11:13:50 +0300 Subject: [PATCH 1/6] clean: introduce exhaustive switch into `createPatchContent` function --- src/CodexToolCallMapper.ts | 84 ++++++++++++++++++++++---------------- 1 file changed, 49 insertions(+), 35 deletions(-) diff --git a/src/CodexToolCallMapper.ts b/src/CodexToolCallMapper.ts index 88654a3e..4e7d7c63 100644 --- a/src/CodexToolCallMapper.ts +++ b/src/CodexToolCallMapper.ts @@ -257,56 +257,70 @@ function createSearchTitle(query: string | null, path: string | null): string { } async function createPatchContent(change: FileUpdateChange): Promise { - if (change.kind.type === "add" && !isUnifiedDiff(change.diff)) { - // For new files, diff may contain raw file content instead of a patch. - return { - type: "diff", - oldText: null, - newText: change.diff, - path: change.path, - _meta: { - kind: "add", - }, - }; + switch (change.kind.type) { + case "add": + return await createAddFileContent(change); + case "delete": + return await createDeleteFileContent(change); + case "update": + return await createUpdateFileContent(change); } +} - if (change.kind.type === "delete") { - // If the patch deletes a file, the old content may be only available from the diff. - const oldContent = await readFile(change.path, { encoding: "utf8"} ).catch(() => - isUnifiedDiff(change.diff) ? patchToDeletedContent(change.diff) : change.diff - ); - - return { - type: "diff", - oldText: oldContent, - newText: "", - path: change.path, - _meta: { - kind: "delete", - } - } +async function createAddFileContent(change: FileUpdateChange): Promise { + let newText; + if (isUnifiedDiff(change.diff)) { + newText = applyPatch("", change.diff) + if (!newText) return null; + } else { + newText = change.diff; } + return { + type: "diff", + oldText: null, + newText: newText, + path: change.path, + _meta: { + kind: "add", + }, + }; +} - const oldContent = change.kind.type === "add" ? "" : await readFile(change.path, { encoding: "utf8" }).catch(() => null); - if (oldContent === null) { - return null; - } +async function createUpdateFileContent(change: FileUpdateChange): Promise { + const oldContent = await readFile(change.path, { encoding: "utf8" }).catch(() => null); + if (oldContent === null) return null; const newContent = applyPatch(oldContent, change.diff); - if (newContent === false) { - return null; - } + if (!newContent) return null; + return { type: "diff", - oldText: change.kind.type === "add" ? null : oldContent, + oldText: oldContent, newText: newContent, path: change.path, _meta: { - kind: change.kind.type, + kind: "update", }, }; } +async function createDeleteFileContent(change: FileUpdateChange): Promise { + // If the patch deletes a file, the old content may be only available from the diff. + const oldContent = await readFile(change.path, { encoding: "utf8"} ).catch(() => + isUnifiedDiff(change.diff) ? patchToDeletedContent(change.diff) : change.diff + ); + + return { + type: "diff", + oldText: oldContent, + newText: "", + path: change.path, + _meta: { + kind: "delete", + } + } +} + function isUnifiedDiff(content: string): boolean { return content.startsWith("--- ") || content.includes("\n--- "); } From 58d93e48a8b6b39eb348097e6a06a396427e4d01 Mon Sep 17 00:00:00 2001 From: Alexandr Suhinin Date: Mon, 1 Jun 2026 13:47:08 +0300 Subject: [PATCH 2/6] fix: suppress exceptions caused by broken unified diffs from codex --- src/CodexToolCallMapper.ts | 20 +++++++++----- .../CodexACPAgent/file-change-events.test.ts | 27 +++++++++++++++++++ 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/src/CodexToolCallMapper.ts b/src/CodexToolCallMapper.ts index 4e7d7c63..da813a55 100644 --- a/src/CodexToolCallMapper.ts +++ b/src/CodexToolCallMapper.ts @@ -20,6 +20,7 @@ import type { ThreadItem, } from "./app-server/v2"; import type { JsonValue } from "./app-server/serde_json/JsonValue"; +import {logger} from "./Logger"; type CodexItemStatus = CommandExecutionStatus | PatchApplyStatus | McpToolCallStatus | DynamicToolCallStatus; type AcpToolCallStatus = "pending" | "in_progress" | "completed" | "failed"; @@ -257,13 +258,18 @@ function createSearchTitle(query: string | null, path: string | null): string { } async function createPatchContent(change: FileUpdateChange): Promise { - switch (change.kind.type) { - case "add": - return await createAddFileContent(change); - case "delete": - return await createDeleteFileContent(change); - case "update": - return await createUpdateFileContent(change); + try { + switch (change.kind.type) { + case "add": + return await createAddFileContent(change); + case "delete": + return await createDeleteFileContent(change); + case "update": + return await createUpdateFileContent(change); + } + } catch (error) { + logger.log(`Error processing file update change: ${error}`); + return null; } } diff --git a/src/__tests__/CodexACPAgent/file-change-events.test.ts b/src/__tests__/CodexACPAgent/file-change-events.test.ts index ce00135a..e7f7ca94 100644 --- a/src/__tests__/CodexACPAgent/file-change-events.test.ts +++ b/src/__tests__/CodexACPAgent/file-change-events.test.ts @@ -1,6 +1,8 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import type { SessionState } from '../../CodexAcpServer'; import type { ServerNotification } from '../../app-server'; +import { createFileChangeUpdate } from '../../CodexToolCallMapper'; +import type { ThreadItem } from '../../app-server/v2'; import { createCodexMockTestFixture, createTestSessionState, setupPromptAndSendNotifications, type CodexMockTestFixture } from '../acp-test-utils'; import {AgentMode} from "../../AgentMode"; @@ -271,4 +273,29 @@ describe('CodexEventHandler - file change events', () => { 'data/file-change-delete-raw-content.json' ); }); + + it('should ignore broken unified diffs in file changes', async () => { + const fileChange: ThreadItem = { + type: 'fileChange', + id: 'file-change-broken-diff', + changes: [ + { + path: '/test/project/BrokenFile.kt', + kind: { type: 'add' }, + diff: +`--- /dev/null ++++ /test/project/BrokenFile.kt +@@ broken @@ ++class BrokenFile +`, + }, + ], + status: 'completed', + }; + + const updateEvent = await createFileChangeUpdate(fileChange); + expect(updateEvent).toMatchObject({ + content: [], + }); + }); }); From a639237792e20a0045886138f8ae412820b673cb Mon Sep 17 00:00:00 2001 From: Alexandr Suhinin Date: Mon, 1 Jun 2026 14:39:59 +0300 Subject: [PATCH 3/6] fix: recover corrupted unified diff content --- src/CodexToolCallMapper.ts | 11 +++++- .../CodexACPAgent/file-change-events.test.ts | 36 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/CodexToolCallMapper.ts b/src/CodexToolCallMapper.ts index da813a55..80f2a8b9 100644 --- a/src/CodexToolCallMapper.ts +++ b/src/CodexToolCallMapper.ts @@ -296,7 +296,8 @@ async function createUpdateFileContent(change: FileUpdateChange): Promise null); if (oldContent === null) return null; - const newContent = applyPatch(oldContent, change.diff); + const unifiedDiff = recoverCorruptedDiff(change.diff); + const newContent = applyPatch(oldContent, unifiedDiff); if (!newContent) return null; return { @@ -327,6 +328,14 @@ async function createDeleteFileContent(change: FileUpdateChange): Promise { content: [], }); }); + + it('should parse update diffs with move metadata appended', async () => { + mockFileContent('/test/project/OriginalFile.kt', 'old code line\n'); + + const fileChange: ThreadItem = { + type: 'fileChange', + id: 'file-change-move-metadata', + changes: [ + { + path: '/test/project/OriginalFile.kt', + kind: { + type: 'update', + move_path: '/test/project/NewFile.kt', + }, + diff: +`@@ -1 +1 @@ +-old code line ++new code line + + +Moved to: /test/project/NewFile.kt`, + }, + ], + status: 'inProgress', + }; + + const updateEvent = await createFileChangeUpdate(fileChange); + expect(updateEvent).toMatchObject({ + content: [ + { + oldText: 'old code line\n', + newText: 'new code line\n', + }, + ], + }); + }); }); From 482c59b3480d16f4c5e8026521a5ecee3a8c590d Mon Sep 17 00:00:00 2001 From: Alexandr Suhinin Date: Fri, 5 Jun 2026 14:35:30 +0300 Subject: [PATCH 4/6] fix: recover moved fileUpdate diff from new file --- src/CodexToolCallMapper.ts | 51 +++++++++++++++---- .../CodexACPAgent/file-change-events.test.ts | 38 ++++++++++++++ 2 files changed, 78 insertions(+), 11 deletions(-) diff --git a/src/CodexToolCallMapper.ts b/src/CodexToolCallMapper.ts index 80f2a8b9..3cfbe8d0 100644 --- a/src/CodexToolCallMapper.ts +++ b/src/CodexToolCallMapper.ts @@ -1,5 +1,5 @@ import type { ToolCallContent } from "@agentclientprotocol/sdk"; -import { applyPatch, parsePatch } from "diff"; +import { applyPatch, parsePatch, reversePatch } from "diff"; import { readFile } from "node:fs/promises"; import path from "node:path"; import type { UpdateSessionEvent } from "./ACPSessionConnection"; @@ -293,18 +293,44 @@ async function createAddFileContent(change: FileUpdateChange): Promise { - const oldContent = await readFile(change.path, { encoding: "utf8" }).catch(() => null); - if (oldContent === null) return null; + if (change.kind.type !== "update") return null; const unifiedDiff = recoverCorruptedDiff(change.diff); - const newContent = applyPatch(oldContent, unifiedDiff); - if (!newContent) return null; + const movePath = change.kind.move_path; + const oldContent = await readFileContent(change.path); + if (oldContent !== null) { + const patchedContent = applyPatch(oldContent, unifiedDiff); + if (patchedContent === false) return null; + return createUpdateDiffContent(movePath ?? change.path, oldContent, patchedContent); + } + + if (!movePath) return null; + const newContent = await readFileContent(movePath); + if (newContent === null) return null; + + const revertedPatch = revertPatch(unifiedDiff); + if (!revertedPatch) return null; + + const revertedContent = applyPatch(newContent, revertedPatch); + if (revertedContent === false) return null; + + return createUpdateDiffContent(movePath, revertedContent, newContent); +} + +function revertPatch(unifiedDiff: string) { + const [patch] = parsePatch(unifiedDiff); + if (!patch) return null; + + return reversePatch(patch); +} + +function createUpdateDiffContent(path: string, oldText: string, newText: string): ToolCallContent { return { type: "diff", - oldText: oldContent, - newText: newContent, - path: change.path, + oldText, + newText, + path, _meta: { kind: "update", }, @@ -313,9 +339,8 @@ async function createUpdateFileContent(change: FileUpdateChange): Promise { // If the patch deletes a file, the old content may be only available from the diff. - const oldContent = await readFile(change.path, { encoding: "utf8"} ).catch(() => - isUnifiedDiff(change.diff) ? patchToDeletedContent(change.diff) : change.diff - ); + const fileContent = await readFileContent(change.path); + const oldContent = fileContent ?? (isUnifiedDiff(change.diff) ? patchToDeletedContent(change.diff) : change.diff); return { type: "diff", @@ -328,6 +353,10 @@ async function createDeleteFileContent(change: FileUpdateChange): Promise { + return await readFile(filePath, { encoding: "utf8" }).catch(() => null); +} + /** * Fix unified diff content corrupted by codex agent. * Removes synthetic "Moved to" from the end. diff --git a/src/__tests__/CodexACPAgent/file-change-events.test.ts b/src/__tests__/CodexACPAgent/file-change-events.test.ts index dafd3034..5643816b 100644 --- a/src/__tests__/CodexACPAgent/file-change-events.test.ts +++ b/src/__tests__/CodexACPAgent/file-change-events.test.ts @@ -330,6 +330,44 @@ Moved to: /test/project/NewFile.kt`, { oldText: 'old code line\n', newText: 'new code line\n', + path: '/test/project/NewFile.kt', + }, + ], + }); + }); + + it('should parse update diffs when the original file was moved already', async () => { + mockFileContent('/test/project/NewFile.kt', 'new code line\n'); + + const fileChange: ThreadItem = { + type: 'fileChange', + id: 'file-change-moved-file-exists', + changes: [ + { + path: '/test/project/OriginalFile.kt', + kind: { + type: 'update', + move_path: '/test/project/NewFile.kt', + }, + diff: +`@@ -1 +1 @@ +-old code line ++new code line + + +Moved to: /test/project/NewFile.kt`, + }, + ], + status: 'inProgress', + }; + + const updateEvent = await createFileChangeUpdate(fileChange); + expect(updateEvent).toMatchObject({ + content: [ + { + oldText: 'old code line\n', + newText: 'new code line\n', + path: '/test/project/NewFile.kt', }, ], }); From d0832b02c253ad51222651ae28f806938b9dcd99 Mon Sep 17 00:00:00 2001 From: Alexandr Suhinin Date: Fri, 5 Jun 2026 15:54:25 +0300 Subject: [PATCH 5/6] clean: remove recovery from unified diff in add/delete file changes incorrect assumption that add/diff changes may have unified diff --- src/CodexToolCallMapper.ts | 56 +------------------ .../CodexACPAgent/file-change-events.test.ts | 47 ++++------------ .../CodexACPAgent/load-session.test.ts | 9 +-- 3 files changed, 16 insertions(+), 96 deletions(-) diff --git a/src/CodexToolCallMapper.ts b/src/CodexToolCallMapper.ts index 3cfbe8d0..c6df542e 100644 --- a/src/CodexToolCallMapper.ts +++ b/src/CodexToolCallMapper.ts @@ -274,17 +274,10 @@ async function createPatchContent(change: FileUpdateChange): Promise { - let newText; - if (isUnifiedDiff(change.diff)) { - newText = applyPatch("", change.diff) - if (!newText) return null; - } else { - newText = change.diff; - } return { type: "diff", oldText: null, - newText: newText, + newText: change.diff, // app-server always returns file content instead of diff path: change.path, _meta: { kind: "add", @@ -338,13 +331,9 @@ function createUpdateDiffContent(path: string, oldText: string, newText: string) } async function createDeleteFileContent(change: FileUpdateChange): Promise { - // If the patch deletes a file, the old content may be only available from the diff. - const fileContent = await readFileContent(change.path); - const oldContent = fileContent ?? (isUnifiedDiff(change.diff) ? patchToDeletedContent(change.diff) : change.diff); - return { type: "diff", - oldText: oldContent, + oldText: change.diff, // app-server always returns file content instead of diff newText: "", path: change.path, _meta: { @@ -364,44 +353,3 @@ async function readFileContent(filePath: string): Promise { function recoverCorruptedDiff(diff: string): string { return diff.replace(/\nMoved to: .*$/, ""); } - -function isUnifiedDiff(content: string): boolean { - return content.startsWith("--- ") || content.includes("\n--- "); -} - -/** - * Recreates the content of a deleted file from the unified diff. - * @param unifiedDiff The unified diff of the file deletion patch - */ -function patchToDeletedContent(unifiedDiff: string): string | null { - try { - const [patch] = parsePatch(unifiedDiff); - if (!patch || patch.hunks.length === 0) { - return null; - } - - const oldLines: string[] = []; - let hasNoTrailingNewlineMarker = false; - - for (const hunk of patch.hunks) { - for (const line of hunk.lines) { - if (line === "\\ No newline at end of file") { - hasNoTrailingNewlineMarker = true; - continue; - } - if (line.startsWith("-") || line.startsWith(" ")) { - oldLines.push(line.slice(1)); - } - } - } - - if (oldLines.length === 0) { - return ""; - } - - const oldText = oldLines.join("\n"); - return hasNoTrailingNewlineMarker || !unifiedDiff.endsWith("\n") ? oldText : `${oldText}\n`; - } catch { - return null; - } -} diff --git a/src/__tests__/CodexACPAgent/file-change-events.test.ts b/src/__tests__/CodexACPAgent/file-change-events.test.ts index 5643816b..c8956394 100644 --- a/src/__tests__/CodexACPAgent/file-change-events.test.ts +++ b/src/__tests__/CodexACPAgent/file-change-events.test.ts @@ -55,14 +55,7 @@ describe('CodexEventHandler - file change events', () => { { path: '/test/project/NewFile.kt', kind: { type: 'add' }, - diff: `--- /dev/null -+++ /test/project/NewFile.kt -@@ -0,0 +1,5 @@ -+package test.project -+ -+class NewFile { -+ fun hello() = "Hello" -+}`, + diff: 'package test.project\n\nclass NewFile {\n fun hello() = "Hello"\n}\n', }, ], status: 'completed', @@ -90,18 +83,12 @@ describe('CodexEventHandler - file change events', () => { { path: '/test/project/FileA.kt', kind: { type: 'add' }, - diff: `--- /dev/null -+++ /test/project/FileA.kt -@@ -0,0 +1 @@ -+class FileA`, + diff: 'class FileA\n', }, { path: '/test/project/FileB.kt', kind: { type: 'add' }, - diff: `--- /dev/null -+++ /test/project/FileB.kt -@@ -0,0 +1 @@ -+class FileB`, + diff: 'class FileB\n', }, ], status: 'completed', @@ -158,12 +145,7 @@ describe('CodexEventHandler - file change events', () => { { path: '/test/project/OldFile.kt', kind: { type: 'delete' }, - diff: `--- /test/project/OldFile.kt -+++ /dev/null -@@ -1,3 +0,0 @@ --package test.project -- --class OldFile {}`, + diff: 'package test.project\n\nclass OldFile {}', }, ], status: 'completed', @@ -224,12 +206,7 @@ describe('CodexEventHandler - file change events', () => { { path: '/test/project/OldFile.kt', kind: { type: 'delete' }, - diff: `--- /test/project/OldFile.kt -+++ /dev/null -@@ -1,3 +0,0 @@ --package test.project -- --class OldFile {}`, + diff: 'package test.project\n\nclass OldFile {}', }, ], status: 'completed', @@ -274,19 +251,19 @@ describe('CodexEventHandler - file change events', () => { ); }); - it('should ignore broken unified diffs in file changes', async () => { - const fileChange: ThreadItem = { + it('should ignore broken unified diffs in update file changes', async () => { + const fileChange: ThreadItem & { type: 'fileChange' } = { type: 'fileChange', id: 'file-change-broken-diff', changes: [ { - path: '/test/project/BrokenFile.kt', - kind: { type: 'add' }, + path: '/test/project/OldFile.kt', + kind: { type: 'update', move_path: null }, diff: -`--- /dev/null -+++ /test/project/BrokenFile.kt +`--- /test/project/OldFile.kt ++++ /test/project/OldFile.kt @@ broken @@ -+class BrokenFile ++class UpdatedFile `, }, ], diff --git a/src/__tests__/CodexACPAgent/load-session.test.ts b/src/__tests__/CodexACPAgent/load-session.test.ts index 43aaa09a..0b9263c3 100644 --- a/src/__tests__/CodexACPAgent/load-session.test.ts +++ b/src/__tests__/CodexACPAgent/load-session.test.ts @@ -108,13 +108,8 @@ describe("CodexACPAgent - loadSession", () => { { path: "/test/project/Added.txt", kind: { type: "add" }, - diff: `--- /dev/null -+++ /test/project/Added.txt -@@ -0,0 +1,2 @@ -+Hello -+World -`, - }, + diff: "Hello\nWorld\n", + } ], status: "completed", }, From 49f7b283370d60c54af3cf7e6ea7ab743e2d7ca4 Mon Sep 17 00:00:00 2001 From: Alexandr Suhinin Date: Fri, 5 Jun 2026 15:57:21 +0300 Subject: [PATCH 6/6] fix: improve matching for corrupted diff --- src/CodexToolCallMapper.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CodexToolCallMapper.ts b/src/CodexToolCallMapper.ts index c6df542e..ad2542ef 100644 --- a/src/CodexToolCallMapper.ts +++ b/src/CodexToolCallMapper.ts @@ -351,5 +351,5 @@ async function readFileContent(filePath: string): Promise { * Removes synthetic "Moved to" from the end. */ function recoverCorruptedDiff(diff: string): string { - return diff.replace(/\nMoved to: .*$/, ""); + return diff.replace(/\n\nMoved to: .*$/, ""); }