Skip to content

Commit 5a36a32

Browse files
committed
feat(tool-call-mapping): emit compact file-edit diffs and raw io
Replace whole-file diff expansion with hunk-local oldText/newText plus locations, normalized rawInput/rawOutput, and patchUpdated mapping for Codex file-change tool calls.
1 parent 616c085 commit 5a36a32

10 files changed

Lines changed: 395 additions & 145 deletions

src/CodexEventHandler.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ import {
3939
createContextCompactionCompleteUpdate,
4040
createContextCompactionStartUpdate,
4141
createDynamicToolCallUpdate,
42+
createFileChangeCompleteUpdate,
43+
createFileChangePatchUpdate,
4244
createFileChangeUpdate,
4345
createGuardianApprovalReviewToolCall,
4446
createGuardianApprovalReviewToolCallUpdate,
@@ -185,6 +187,8 @@ export class CodexEventHandler {
185187
return this.createThreadGoalClearedEvent(notification.params);
186188
case "item/commandExecution/terminalInteraction":
187189
return this.createTerminalInteractionEvent(notification.params);
190+
case "item/fileChange/patchUpdated":
191+
return await createFileChangePatchUpdate(notification.params);
188192
// ignored events
189193
case "thread/deleted":
190194
case "command/exec/outputDelta":
@@ -193,7 +197,6 @@ export class CodexEventHandler {
193197
case "turn/diff/updated":
194198
case "turn/moderationMetadata":
195199
case "item/fileChange/outputDelta":
196-
case "item/fileChange/patchUpdated":
197200
case "account/updated":
198201
case "fs/changed":
199202
case "mcpServer/startupStatus/updated":
@@ -360,6 +363,7 @@ export class CodexEventHandler {
360363
private async completeItemEvent(event: ItemCompletedNotification): Promise<UpdateSessionEvent | null> {
361364
switch (event.item.type) {
362365
case "fileChange":
366+
return createFileChangeCompleteUpdate(event.item);
363367
case "dynamicToolCall":
364368
return {
365369
sessionUpdate: "tool_call_update",

src/CodexToolCallMapper.ts

Lines changed: 124 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
import type { ContentBlock, ToolCallContent } from "@agentclientprotocol/sdk";
2-
import { applyPatch, parsePatch, reversePatch } from "diff";
3-
import { readFile } from "node:fs/promises";
1+
import type { ContentBlock, ToolCallContent, ToolCallLocation } from "@agentclientprotocol/sdk";
2+
import { parsePatch, type StructuredPatchHunk } from "diff";
43
import path from "node:path";
54
import type { UpdateSessionEvent } from "./ACPSessionConnection";
65
import { stripShellPrefix } from "./CommandUtils";
@@ -13,6 +12,7 @@ import type {
1312
CommandAction,
1413
CommandExecutionStatus,
1514
DynamicToolCallStatus,
15+
FileChangePatchUpdatedNotification,
1616
FileUpdateChange,
1717
GuardianApprovalReview,
1818
GuardianApprovalReviewAction,
@@ -41,6 +41,7 @@ type GuardianApprovalReviewNotification =
4141
type WebSearchItem = ThreadItem & { type: "webSearch" };
4242
type CollabAgentToolCallItem = ThreadItem & { type: "collabAgentToolCall" };
4343
type CommandExecutionItem = ThreadItem & { type: "commandExecution" };
44+
type FileChangeItem = ThreadItem & { type: "fileChange" };
4445
type ContextCompactionItem = ThreadItem & { type: "contextCompaction" };
4546
type AcpToolCallEvent = Extract<UpdateSessionEvent, { sessionUpdate: "tool_call" }>;
4647

@@ -59,21 +60,41 @@ function toAcpStatus(status: CodexItemStatus): AcpToolCallStatus {
5960
}
6061

6162
export async function createFileChangeUpdate(
62-
item: ThreadItem & { type: "fileChange" }
63+
item: FileChangeItem
6364
): Promise<UpdateSessionEvent> {
64-
const patches: ToolCallContent[] = [];
65-
for (const change of item.changes) {
66-
const content = await createPatchContent(change);
67-
if (content) patches.push(content);
68-
// ignore unparseable diffs
69-
}
65+
const rawOutput = createFileChangeRawOutput(item);
7066
return {
7167
sessionUpdate: "tool_call",
7268
toolCallId: item.id,
7369
title: "Editing files",
7470
kind: "edit",
7571
status: toAcpStatus(item.status),
76-
content: patches,
72+
content: await createFileChangeContent(item.changes),
73+
locations: createFileChangeLocations(item.changes),
74+
rawInput: createFileChangeRawInput(item.changes),
75+
...(rawOutput === undefined ? {} : { rawOutput }),
76+
};
77+
}
78+
79+
export async function createFileChangePatchUpdate(
80+
notification: FileChangePatchUpdatedNotification,
81+
): Promise<UpdateSessionEvent> {
82+
return {
83+
sessionUpdate: "tool_call_update",
84+
toolCallId: notification.itemId,
85+
status: "in_progress",
86+
content: await createFileChangeContent(notification.changes),
87+
locations: createFileChangeLocations(notification.changes),
88+
rawInput: createFileChangeRawInput(notification.changes),
89+
};
90+
}
91+
92+
export function createFileChangeCompleteUpdate(item: FileChangeItem): UpdateSessionEvent {
93+
return {
94+
sessionUpdate: "tool_call_update",
95+
toolCallId: item.id,
96+
status: toAcpStatus(item.status),
97+
rawOutput: createFileChangeRawOutput(item),
7798
};
7899
}
79100

@@ -734,23 +755,31 @@ function createContent(content: ContentBlock): ToolCallContent {
734755
};
735756
}
736757

737-
async function createPatchContent(change: FileUpdateChange): Promise<ToolCallContent | null> {
758+
async function createFileChangeContent(changes: FileUpdateChange[]): Promise<ToolCallContent[]> {
759+
const content: ToolCallContent[] = [];
760+
for (const change of changes) {
761+
content.push(...await createPatchContent(change));
762+
}
763+
return content;
764+
}
765+
766+
async function createPatchContent(change: FileUpdateChange): Promise<ToolCallContent[]> {
738767
try {
739768
switch (change.kind.type) {
740769
case "add":
741-
return await createAddFileContent(change);
770+
return [createAddFileContent(change)];
742771
case "delete":
743-
return await createDeleteFileContent(change);
772+
return [createDeleteFileContent(change)];
744773
case "update":
745-
return await createUpdateFileContent(change);
774+
return createUpdateFileContent(change);
746775
}
747776
} catch (error) {
748777
logger.log(`Error processing file update change: ${error}`);
749-
return null;
778+
return [];
750779
}
751780
}
752781

753-
async function createAddFileContent(change: FileUpdateChange): Promise<ToolCallContent | null> {
782+
function createAddFileContent(change: FileUpdateChange): ToolCallContent {
754783
return {
755784
type: "diff",
756785
oldText: null,
@@ -762,63 +791,46 @@ async function createAddFileContent(change: FileUpdateChange): Promise<ToolCallC
762791
};
763792
}
764793

765-
async function createUpdateFileContent(change: FileUpdateChange): Promise<ToolCallContent | null> {
766-
if (change.kind.type !== "update") return null;
767-
768-
const unifiedDiff = recoverCorruptedDiff(change.diff);
769-
const movePath = change.kind.move_path;
770-
771-
const oldContent = await readFileContent(change.path);
772-
if (oldContent !== null) {
773-
const patchedContent = applyPatch(oldContent, unifiedDiff);
774-
if (patchedContent === false) {
775-
// If Codex runs in full access mode, the file might already be patched.
776-
// we can verify this by checking if the reverted patch applies.
777-
const revertedPatch = revertPatch(unifiedDiff);
778-
if (revertedPatch) {
779-
const revertedContent = applyPatch(oldContent, revertedPatch);
780-
if (revertedContent !== false) {
781-
return createUpdateDiffContent(change.path, revertedContent, oldContent);
782-
}
783-
}
784-
return null;
785-
}
786-
return createUpdateDiffContent(movePath ?? change.path, oldContent, patchedContent);
787-
}
788-
789-
if (!movePath) return null;
790-
const newContent = await readFileContent(movePath);
791-
if (newContent === null) return null;
792-
793-
const revertedPatch = revertPatch(unifiedDiff);
794-
if (!revertedPatch) return null;
795-
796-
const revertedContent = applyPatch(newContent, revertedPatch);
797-
if (revertedContent === false) return null;
798-
799-
return createUpdateDiffContent(movePath, revertedContent, newContent);
800-
}
801-
802-
function revertPatch(unifiedDiff: string) {
803-
const [patch] = parsePatch(unifiedDiff);
804-
if (!patch) return null;
794+
function createUpdateFileContent(change: FileUpdateChange): ToolCallContent[] {
795+
if (change.kind.type !== "update") return [];
805796

806-
return reversePatch(patch);
797+
const patches = parsePatch(recoverCorruptedDiff(change.diff));
798+
const targetPath = change.kind.move_path ?? change.path;
799+
return patches.flatMap((patch) => patch.hunks.map((hunk) => createUpdateDiffContent(targetPath, hunk)));
807800
}
808801

809-
function createUpdateDiffContent(path: string, oldText: string, newText: string): ToolCallContent {
802+
function createUpdateDiffContent(path: string, hunk: StructuredPatchHunk): ToolCallContent {
810803
return {
811804
type: "diff",
812-
oldText,
813-
newText,
805+
oldText: createHunkText(hunk, "old"),
806+
newText: createHunkText(hunk, "new"),
814807
path,
815808
_meta: {
816809
kind: "update",
810+
old_start: hunk.oldStart,
811+
new_start: hunk.newStart,
817812
},
818813
};
819814
}
820815

821-
async function createDeleteFileContent(change: FileUpdateChange): Promise<ToolCallContent> {
816+
function createHunkText(hunk: StructuredPatchHunk, side: "old" | "new"): string {
817+
return hunk.lines.flatMap((line): string[] => {
818+
switch (line[0]) {
819+
case " ":
820+
return [line.slice(1)];
821+
case "-":
822+
return side === "old" ? [line.slice(1)] : [];
823+
case "+":
824+
return side === "new" ? [line.slice(1)] : [];
825+
case "\\":
826+
return [];
827+
default:
828+
return [];
829+
}
830+
}).join("\n");
831+
}
832+
833+
function createDeleteFileContent(change: FileUpdateChange): ToolCallContent {
822834
return {
823835
type: "diff",
824836
oldText: change.diff, // app-server always returns file content instead of diff
@@ -830,8 +842,56 @@ async function createDeleteFileContent(change: FileUpdateChange): Promise<ToolCa
830842
}
831843
}
832844

833-
async function readFileContent(filePath: string): Promise<string | null> {
834-
return await readFile(filePath, { encoding: "utf8" }).catch(() => null);
845+
function createFileChangeLocations(changes: FileUpdateChange[]): ToolCallLocation[] {
846+
const locations = new Map<string, ToolCallLocation>();
847+
const addLocation = (filePath: string, line?: number) => {
848+
const current = locations.get(filePath);
849+
if (current?.line !== undefined || (current && line === undefined)) {
850+
return;
851+
}
852+
locations.set(filePath, line === undefined ? { path: filePath } : { path: filePath, line });
853+
};
854+
855+
for (const change of changes) {
856+
switch (change.kind.type) {
857+
case "add":
858+
case "delete":
859+
addLocation(change.path);
860+
break;
861+
case "update": {
862+
const firstHunk = firstUpdateHunk(change);
863+
if (change.kind.move_path && change.kind.move_path !== change.path) {
864+
addLocation(change.path, firstHunk?.oldStart);
865+
}
866+
addLocation(change.kind.move_path ?? change.path, firstHunk?.newStart);
867+
break;
868+
}
869+
}
870+
}
871+
872+
return [...locations.values()];
873+
}
874+
875+
function firstUpdateHunk(change: FileUpdateChange): StructuredPatchHunk | undefined {
876+
try {
877+
return parsePatch(recoverCorruptedDiff(change.diff))[0]?.hunks[0];
878+
} catch {
879+
return undefined;
880+
}
881+
}
882+
883+
function createFileChangeRawInput(changes: FileUpdateChange[]) {
884+
return { changes };
885+
}
886+
887+
function createFileChangeRawOutput(item: FileChangeItem): Record<string, unknown> | undefined {
888+
if (item.status === "inProgress") {
889+
return undefined;
890+
}
891+
return {
892+
status: item.status,
893+
success: item.status === "completed",
894+
};
835895
}
836896

837897
/**

src/__tests__/CodexACPAgent/data/file-change-add-multiple-files.json

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,37 @@
2828
"kind": "add"
2929
}
3030
}
31-
]
31+
],
32+
"locations": [
33+
{
34+
"path": "/test/project/FileA.kt"
35+
},
36+
{
37+
"path": "/test/project/FileB.kt"
38+
}
39+
],
40+
"rawInput": {
41+
"changes": [
42+
{
43+
"path": "/test/project/FileA.kt",
44+
"kind": {
45+
"type": "add"
46+
},
47+
"diff": "class FileA\n"
48+
},
49+
{
50+
"path": "/test/project/FileB.kt",
51+
"kind": {
52+
"type": "add"
53+
},
54+
"diff": "class FileB\n"
55+
}
56+
]
57+
},
58+
"rawOutput": {
59+
"status": "completed",
60+
"success": true
61+
}
3262
}
3363
}
3464
]

src/__tests__/CodexACPAgent/data/file-change-add-new-file.json

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,27 @@
1919
"kind": "add"
2020
}
2121
}
22-
]
22+
],
23+
"locations": [
24+
{
25+
"path": "/test/project/NewFile.kt"
26+
}
27+
],
28+
"rawInput": {
29+
"changes": [
30+
{
31+
"path": "/test/project/NewFile.kt",
32+
"kind": {
33+
"type": "add"
34+
},
35+
"diff": "package test.project\n\nclass NewFile {\n fun hello() = \"Hello\"\n}\n"
36+
}
37+
]
38+
},
39+
"rawOutput": {
40+
"status": "completed",
41+
"success": true
42+
}
2343
}
2444
}
2545
]

src/__tests__/CodexACPAgent/data/file-change-add-raw-content.json

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,27 @@
1919
"kind": "add"
2020
}
2121
}
22-
]
22+
],
23+
"locations": [
24+
{
25+
"path": "/test/project/RawFile.kt"
26+
}
27+
],
28+
"rawInput": {
29+
"changes": [
30+
{
31+
"path": "/test/project/RawFile.kt",
32+
"kind": {
33+
"type": "add"
34+
},
35+
"diff": "fun main() {\n println(\"Hello, World!\")\n}\n"
36+
}
37+
]
38+
},
39+
"rawOutput": {
40+
"status": "completed",
41+
"success": true
42+
}
2343
}
2444
}
2545
]

0 commit comments

Comments
 (0)