Skip to content

Commit 010f53b

Browse files
committed
Always include file change diffs in ACP updates
1 parent e7740de commit 010f53b

4 files changed

Lines changed: 158 additions & 12 deletions

File tree

src/CodexToolCallMapper.ts

Lines changed: 69 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -368,22 +368,22 @@ async function createPatchContent(change: FileUpdateChange): Promise<ToolCallCon
368368

369369
const oldContent = change.kind.type === "add" ? "" : await readFile(change.path, { encoding: "utf8" }).catch(() => null);
370370
if (oldContent === null) {
371-
return null;
371+
return isUnifiedDiff(change.diff) ? createPatchContentFromUnifiedDiff(change) : null;
372372
}
373373

374374
const newContent = applyPatch(oldContent, change.diff);
375-
if (newContent === false) {
376-
return null;
375+
if (newContent !== false) {
376+
return {
377+
type: "diff",
378+
oldText: change.kind.type === "add" ? null : oldContent,
379+
newText: newContent,
380+
path: change.path,
381+
_meta: {
382+
kind: change.kind.type,
383+
},
384+
};
377385
}
378-
return {
379-
type: "diff",
380-
oldText: change.kind.type === "add" ? null : oldContent,
381-
newText: newContent,
382-
path: change.path,
383-
_meta: {
384-
kind: change.kind.type,
385-
},
386-
};
386+
return createPatchContentFromUnifiedDiff(change);
387387
}
388388

389389
function isUnifiedDiff(content: string): boolean {
@@ -472,3 +472,60 @@ function patchToDeletedContent(unifiedDiff: string): string | null {
472472
return null;
473473
}
474474
}
475+
476+
function createPatchContentFromUnifiedDiff(change: FileUpdateChange): ToolCallContent | null {
477+
const diffContent = patchToDiffContent(change.diff);
478+
if (diffContent === null) {
479+
return null;
480+
}
481+
482+
return {
483+
type: "diff",
484+
oldText: change.kind.type === "add" ? null : diffContent.oldText,
485+
newText: change.kind.type === "delete" ? "" : diffContent.newText,
486+
path: change.path,
487+
_meta: {
488+
kind: change.kind.type,
489+
},
490+
};
491+
}
492+
493+
function patchToDiffContent(unifiedDiff: string): { oldText: string; newText: string } | null {
494+
try {
495+
const [patch] = parsePatch(unifiedDiff);
496+
if (!patch || patch.hunks.length === 0) {
497+
return null;
498+
}
499+
500+
const oldLines: string[] = [];
501+
const newLines: string[] = [];
502+
503+
for (const hunk of patch.hunks) {
504+
for (const line of hunk.lines) {
505+
if (line === "\\ No newline at end of file") {
506+
continue;
507+
}
508+
if (line.startsWith(" ")) {
509+
const text = line.slice(1);
510+
oldLines.push(text);
511+
newLines.push(text);
512+
continue;
513+
}
514+
if (line.startsWith("-")) {
515+
oldLines.push(line.slice(1));
516+
continue;
517+
}
518+
if (line.startsWith("+")) {
519+
newLines.push(line.slice(1));
520+
}
521+
}
522+
}
523+
524+
return {
525+
oldText: oldLines.join("\n"),
526+
newText: newLines.join("\n"),
527+
};
528+
} catch {
529+
return null;
530+
}
531+
}

src/__tests__/CodexACPAgent/data/approval-file-change-from-turn-diff.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,13 @@
3232
"type": "text",
3333
"text": "Updating config file"
3434
}
35+
},
36+
{
37+
"type": "diff",
38+
"oldText": "{\"feature\":false}",
39+
"newText": "{\"feature\":true}",
40+
"path": "test/project/config.json",
41+
"_meta": "_meta"
3542
}
3643
]
3744
},
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
{
2+
"method": "sessionUpdate",
3+
"args": [
4+
{
5+
"sessionId": "test-session-id",
6+
"update": {
7+
"sessionUpdate": "tool_call",
8+
"toolCallId": "file-change-already-applied",
9+
"title": "/test/project/OldFile.kt",
10+
"kind": "edit",
11+
"status": "completed",
12+
"content": [
13+
{
14+
"type": "diff",
15+
"oldText": "package test.project\n\nclass OldFile {}",
16+
"newText": "package test.project\n\nclass OldFile { fun hello() = \"Hello\" }",
17+
"path": "/test/project/OldFile.kt",
18+
"_meta": {
19+
"kind": "update"
20+
}
21+
}
22+
],
23+
"locations": [
24+
{
25+
"path": "/test/project/OldFile.kt"
26+
}
27+
],
28+
"rawInput": {
29+
"changes": [
30+
{
31+
"path": "/test/project/OldFile.kt",
32+
"kind": {
33+
"type": "update"
34+
},
35+
"diff": "--- /test/project/OldFile.kt\n+++ /test/project/OldFile.kt\n@@ -1,3 +1,3 @@\n package test.project\n \n-class OldFile {}\n+class OldFile { fun hello() = \"Hello\" }"
36+
}
37+
]
38+
}
39+
}
40+
}
41+
]
42+
}

src/__tests__/CodexACPAgent/file-change-events.test.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,4 +338,44 @@ describe('CodexEventHandler - file change events', () => {
338338
'data/file-change-completion-update.json'
339339
);
340340
});
341+
342+
it('should emit diff content when the file is already in the post-edit state', async () => {
343+
mockFileContent('/test/project/OldFile.kt', 'package test.project\n\nclass OldFile { fun hello() = "Hello" }');
344+
345+
const updateDiff = [
346+
'--- /test/project/OldFile.kt',
347+
'+++ /test/project/OldFile.kt',
348+
'@@ -1,3 +1,3 @@',
349+
' package test.project',
350+
' ',
351+
'-class OldFile {}',
352+
'+class OldFile { fun hello() = "Hello" }',
353+
].join('\n');
354+
355+
const updateNotification = {
356+
method: 'item/started',
357+
params: {
358+
threadId: 'thread-1',
359+
turnId: 'turn-1',
360+
item: {
361+
type: 'fileChange',
362+
id: 'file-change-already-applied',
363+
changes: [
364+
{
365+
path: '/test/project/OldFile.kt',
366+
kind: { type: 'update' },
367+
diff: updateDiff,
368+
},
369+
],
370+
status: 'completed',
371+
},
372+
},
373+
} as ServerNotification;
374+
375+
await setupPromptAndSendNotifications(mockFixture, sessionId, sessionState, [updateNotification]);
376+
377+
await expect(mockFixture.getAcpConnectionDump(['id'])).toMatchFileSnapshot(
378+
'data/file-change-update-already-applied.json'
379+
);
380+
});
341381
});

0 commit comments

Comments
 (0)