Skip to content

Commit d15e89b

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 8aff492 commit d15e89b

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
@@ -37,6 +37,8 @@ import {
3737
createCollabAgentToolCallUpdate,
3838
createCommandExecutionUpdate,
3939
createDynamicToolCallUpdate,
40+
createFileChangeCompleteUpdate,
41+
createFileChangePatchUpdate,
4042
createFileChangeUpdate,
4143
createGuardianApprovalReviewToolCall,
4244
createGuardianApprovalReviewToolCallUpdate,
@@ -179,6 +181,8 @@ export class CodexEventHandler {
179181
return this.createThreadGoalClearedEvent(notification.params);
180182
case "item/commandExecution/terminalInteraction":
181183
return this.createTerminalInteractionEvent(notification.params);
184+
case "item/fileChange/patchUpdated":
185+
return await createFileChangePatchUpdate(notification.params);
182186
// ignored events
183187
case "thread/deleted":
184188
case "command/exec/outputDelta":
@@ -187,7 +191,6 @@ export class CodexEventHandler {
187191
case "turn/diff/updated":
188192
case "turn/moderationMetadata":
189193
case "item/fileChange/outputDelta":
190-
case "item/fileChange/patchUpdated":
191194
case "account/updated":
192195
case "fs/changed":
193196
case "mcpServer/startupStatus/updated":
@@ -353,6 +356,7 @@ export class CodexEventHandler {
353356
private async completeItemEvent(event: ItemCompletedNotification): Promise<UpdateSessionEvent | null> {
354357
switch (event.item.type) {
355358
case "fileChange":
359+
return createFileChangeCompleteUpdate(event.item);
356360
case "dynamicToolCall":
357361
return {
358362
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 AcpToolCallEvent = Extract<UpdateSessionEvent, { sessionUpdate: "tool_call" }>;
4546

4647
function toAcpStatus(status: CodexItemStatus): AcpToolCallStatus {
@@ -56,21 +57,41 @@ function toAcpStatus(status: CodexItemStatus): AcpToolCallStatus {
5657
}
5758

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

@@ -693,23 +714,31 @@ function createContent(content: ContentBlock): ToolCallContent {
693714
};
694715
}
695716

696-
async function createPatchContent(change: FileUpdateChange): Promise<ToolCallContent | null> {
717+
async function createFileChangeContent(changes: FileUpdateChange[]): Promise<ToolCallContent[]> {
718+
const content: ToolCallContent[] = [];
719+
for (const change of changes) {
720+
content.push(...await createPatchContent(change));
721+
}
722+
return content;
723+
}
724+
725+
async function createPatchContent(change: FileUpdateChange): Promise<ToolCallContent[]> {
697726
try {
698727
switch (change.kind.type) {
699728
case "add":
700-
return await createAddFileContent(change);
729+
return [createAddFileContent(change)];
701730
case "delete":
702-
return await createDeleteFileContent(change);
731+
return [createDeleteFileContent(change)];
703732
case "update":
704-
return await createUpdateFileContent(change);
733+
return createUpdateFileContent(change);
705734
}
706735
} catch (error) {
707736
logger.log(`Error processing file update change: ${error}`);
708-
return null;
737+
return [];
709738
}
710739
}
711740

712-
async function createAddFileContent(change: FileUpdateChange): Promise<ToolCallContent | null> {
741+
function createAddFileContent(change: FileUpdateChange): ToolCallContent {
713742
return {
714743
type: "diff",
715744
oldText: null,
@@ -721,63 +750,46 @@ async function createAddFileContent(change: FileUpdateChange): Promise<ToolCallC
721750
};
722751
}
723752

724-
async function createUpdateFileContent(change: FileUpdateChange): Promise<ToolCallContent | null> {
725-
if (change.kind.type !== "update") return null;
726-
727-
const unifiedDiff = recoverCorruptedDiff(change.diff);
728-
const movePath = change.kind.move_path;
729-
730-
const oldContent = await readFileContent(change.path);
731-
if (oldContent !== null) {
732-
const patchedContent = applyPatch(oldContent, unifiedDiff);
733-
if (patchedContent === false) {
734-
// If Codex runs in full access mode, the file might already be patched.
735-
// we can verify this by checking if the reverted patch applies.
736-
const revertedPatch = revertPatch(unifiedDiff);
737-
if (revertedPatch) {
738-
const revertedContent = applyPatch(oldContent, revertedPatch);
739-
if (revertedContent !== false) {
740-
return createUpdateDiffContent(change.path, revertedContent, oldContent);
741-
}
742-
}
743-
return null;
744-
}
745-
return createUpdateDiffContent(movePath ?? change.path, oldContent, patchedContent);
746-
}
747-
748-
if (!movePath) return null;
749-
const newContent = await readFileContent(movePath);
750-
if (newContent === null) return null;
751-
752-
const revertedPatch = revertPatch(unifiedDiff);
753-
if (!revertedPatch) return null;
754-
755-
const revertedContent = applyPatch(newContent, revertedPatch);
756-
if (revertedContent === false) return null;
757-
758-
return createUpdateDiffContent(movePath, revertedContent, newContent);
759-
}
760-
761-
function revertPatch(unifiedDiff: string) {
762-
const [patch] = parsePatch(unifiedDiff);
763-
if (!patch) return null;
753+
function createUpdateFileContent(change: FileUpdateChange): ToolCallContent[] {
754+
if (change.kind.type !== "update") return [];
764755

765-
return reversePatch(patch);
756+
const patches = parsePatch(recoverCorruptedDiff(change.diff));
757+
const targetPath = change.kind.move_path ?? change.path;
758+
return patches.flatMap((patch) => patch.hunks.map((hunk) => createUpdateDiffContent(targetPath, hunk)));
766759
}
767760

768-
function createUpdateDiffContent(path: string, oldText: string, newText: string): ToolCallContent {
761+
function createUpdateDiffContent(path: string, hunk: StructuredPatchHunk): ToolCallContent {
769762
return {
770763
type: "diff",
771-
oldText,
772-
newText,
764+
oldText: createHunkText(hunk, "old"),
765+
newText: createHunkText(hunk, "new"),
773766
path,
774767
_meta: {
775768
kind: "update",
769+
old_start: hunk.oldStart,
770+
new_start: hunk.newStart,
776771
},
777772
};
778773
}
779774

780-
async function createDeleteFileContent(change: FileUpdateChange): Promise<ToolCallContent> {
775+
function createHunkText(hunk: StructuredPatchHunk, side: "old" | "new"): string {
776+
return hunk.lines.flatMap((line): string[] => {
777+
switch (line[0]) {
778+
case " ":
779+
return [line.slice(1)];
780+
case "-":
781+
return side === "old" ? [line.slice(1)] : [];
782+
case "+":
783+
return side === "new" ? [line.slice(1)] : [];
784+
case "\\":
785+
return [];
786+
default:
787+
return [];
788+
}
789+
}).join("\n");
790+
}
791+
792+
function createDeleteFileContent(change: FileUpdateChange): ToolCallContent {
781793
return {
782794
type: "diff",
783795
oldText: change.diff, // app-server always returns file content instead of diff
@@ -789,8 +801,56 @@ async function createDeleteFileContent(change: FileUpdateChange): Promise<ToolCa
789801
}
790802
}
791803

792-
async function readFileContent(filePath: string): Promise<string | null> {
793-
return await readFile(filePath, { encoding: "utf8" }).catch(() => null);
804+
function createFileChangeLocations(changes: FileUpdateChange[]): ToolCallLocation[] {
805+
const locations = new Map<string, ToolCallLocation>();
806+
const addLocation = (filePath: string, line?: number) => {
807+
const current = locations.get(filePath);
808+
if (current?.line !== undefined || (current && line === undefined)) {
809+
return;
810+
}
811+
locations.set(filePath, line === undefined ? { path: filePath } : { path: filePath, line });
812+
};
813+
814+
for (const change of changes) {
815+
switch (change.kind.type) {
816+
case "add":
817+
case "delete":
818+
addLocation(change.path);
819+
break;
820+
case "update": {
821+
const firstHunk = firstUpdateHunk(change);
822+
if (change.kind.move_path && change.kind.move_path !== change.path) {
823+
addLocation(change.path, firstHunk?.oldStart);
824+
}
825+
addLocation(change.kind.move_path ?? change.path, firstHunk?.newStart);
826+
break;
827+
}
828+
}
829+
}
830+
831+
return [...locations.values()];
832+
}
833+
834+
function firstUpdateHunk(change: FileUpdateChange): StructuredPatchHunk | undefined {
835+
try {
836+
return parsePatch(recoverCorruptedDiff(change.diff))[0]?.hunks[0];
837+
} catch {
838+
return undefined;
839+
}
840+
}
841+
842+
function createFileChangeRawInput(changes: FileUpdateChange[]) {
843+
return { changes };
844+
}
845+
846+
function createFileChangeRawOutput(item: FileChangeItem): Record<string, unknown> | undefined {
847+
if (item.status === "inProgress") {
848+
return undefined;
849+
}
850+
return {
851+
status: item.status,
852+
success: item.status === "completed",
853+
};
794854
}
795855

796856
/**

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)