mirror of
https://github.com/getpaseo/paseo.git
synced 2026-11-03 15:12:48 +00:00
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.
This commit is contained in:
@@ -21,6 +21,36 @@ describe("tool-call-parsers", () => {
|
|||||||
expect(parsed.find((entry) => entry.type === "add")?.content).toBe("+new");
|
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", () => {
|
it("extracts TodoWrite task entries", () => {
|
||||||
const tasks = extractTaskEntriesFromToolCall("TodoWrite", {
|
const tasks = extractTaskEntriesFromToolCall("TodoWrite", {
|
||||||
todos: [
|
todos: [
|
||||||
|
|||||||
@@ -210,6 +210,8 @@ export function parseUnifiedDiff(diffText?: string): DiffLine[] {
|
|||||||
}
|
}
|
||||||
|
|
||||||
const lines = splitIntoLines(diffText);
|
const lines = splitIntoLines(diffText);
|
||||||
|
const fileHeaders = lines.filter((line) => line.startsWith("diff --git "));
|
||||||
|
const hasMultipleFiles = fileHeaders.length > 1;
|
||||||
const diff: DiffLine[] = [];
|
const diff: DiffLine[] = [];
|
||||||
|
|
||||||
for (const line of lines) {
|
for (const line of lines) {
|
||||||
@@ -218,7 +220,8 @@ export function parseUnifiedDiff(diffText?: string): DiffLine[] {
|
|||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (line.startsWith("@@")) {
|
const isFileHeader = hasMultipleFiles && line.startsWith("diff --git ");
|
||||||
|
if (line.startsWith("@@") || isFileHeader) {
|
||||||
diff.push({ type: "header", content: line });
|
diff.push({ type: "header", content: line });
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -3007,6 +3007,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", () => {
|
test("maps Codex plan markdown to a synthetic plan tool call", () => {
|
||||||
const item = mapCodexPlanToToolCall({
|
const item = mapCodexPlanToToolCall({
|
||||||
callId: "plan-turn-1",
|
callId: "plan-turn-1",
|
||||||
|
|||||||
@@ -55,11 +55,13 @@ import { composeSystemPromptParts } from "../system-prompt.js";
|
|||||||
import { curateAgentActivity } from "../activity-curator.js";
|
import { curateAgentActivity } from "../activity-curator.js";
|
||||||
import { CodexAsyncQuestions, codexAsyncQuestionToTimeline } from "./codex/async-questions.js";
|
import { CodexAsyncQuestions, codexAsyncQuestionToTimeline } from "./codex/async-questions.js";
|
||||||
import {
|
import {
|
||||||
|
fileChangeToUnifiedDiff,
|
||||||
mapCodexToolCallEnvelope,
|
mapCodexToolCallEnvelope,
|
||||||
mapCodexToolCallFromThreadItem,
|
mapCodexToolCallFromThreadItem,
|
||||||
normalizeCommandExecutionCommand,
|
normalizeCommandExecutionCommand,
|
||||||
splitCodexMcpToolResultImages,
|
splitCodexMcpToolResultImages,
|
||||||
} from "./codex/tool-call-mapper.js";
|
} from "./codex/tool-call-mapper.js";
|
||||||
|
import { normalizeCodexFilePath } from "./codex/tool-call-detail-parser.js";
|
||||||
import {
|
import {
|
||||||
checkProviderLaunchAvailable,
|
checkProviderLaunchAvailable,
|
||||||
createProviderEnv,
|
createProviderEnv,
|
||||||
@@ -1664,7 +1666,18 @@ export function mapCodexPatchNotificationToToolCall(params: {
|
|||||||
const firstPatchText = files
|
const firstPatchText = files
|
||||||
.map((file) => file.content?.trim())
|
.map((file) => file.content?.trim())
|
||||||
.find((value): value is string => typeof value === "string" && value.length > 0);
|
.find((value): value is string => typeof value === "string" && value.length > 0);
|
||||||
const patchText = firstPatchText;
|
const patchText =
|
||||||
|
files.length > 1
|
||||||
|
? files
|
||||||
|
.map((file) =>
|
||||||
|
fileChangeToUnifiedDiff({
|
||||||
|
path: normalizeCodexFilePath(file.path, params.cwd) ?? file.path,
|
||||||
|
kind: file.kind,
|
||||||
|
diff: file.content,
|
||||||
|
}),
|
||||||
|
)
|
||||||
|
.join("\n\n")
|
||||||
|
: firstPatchText;
|
||||||
const patchFields = codexPatchTextFields(patchText);
|
const patchFields = codexPatchTextFields(patchText);
|
||||||
const mapped = mapCodexToolCallEnvelope({
|
const mapped = mapCodexToolCallEnvelope({
|
||||||
callId: params.callId ?? null,
|
callId: params.callId ?? null,
|
||||||
|
|||||||
@@ -528,6 +528,115 @@ 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("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", () => {
|
it("maps fileChange object-style change payloads keyed by path", () => {
|
||||||
const item = expectMapped(
|
const item = expectMapped(
|
||||||
mapCodexToolCallFromThreadItem(
|
mapCodexToolCallFromThreadItem(
|
||||||
|
|||||||
@@ -400,7 +400,7 @@ function contentToDeletionDiff(filePath: string, content: string): string {
|
|||||||
output.push(`diff --git a/${filePath} b/${filePath}`);
|
output.push(`diff --git a/${filePath} b/${filePath}`);
|
||||||
output.push(`--- a/${filePath}`);
|
output.push(`--- a/${filePath}`);
|
||||||
output.push(`+++ /dev/null`);
|
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) {
|
if (nonEmpty.length > 0) {
|
||||||
output.push(`@@ -1,${nonEmpty.length} +0,0 @@`);
|
output.push(`@@ -1,${nonEmpty.length} +0,0 @@`);
|
||||||
for (const line of nonEmpty) {
|
for (const line of nonEmpty) {
|
||||||
@@ -740,6 +740,9 @@ function parseFileChangePath(
|
|||||||
}
|
}
|
||||||
|
|
||||||
function parseFileChangeKind(entry: Record<string, unknown>): string | undefined {
|
function parseFileChangeKind(entry: Record<string, unknown>): string | undefined {
|
||||||
|
if (isRecord(entry.kind) && typeof entry.kind.type === "string") {
|
||||||
|
return entry.kind.type;
|
||||||
|
}
|
||||||
return (
|
return (
|
||||||
(typeof entry.kind === "string" && entry.kind) ||
|
(typeof entry.kind === "string" && entry.kind) ||
|
||||||
(typeof entry.type === "string" && entry.type) ||
|
(typeof entry.type === "string" && entry.type) ||
|
||||||
@@ -839,6 +842,28 @@ function resolveFileChangeTextFields(file: CodexFileChangeEntry | undefined): {
|
|||||||
return asEditTextFields(file.diff);
|
return asEditTextFields(file.diff);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export function fileChangeToUnifiedDiff(file: CodexFileChangeEntry): string {
|
||||||
|
const { unifiedDiff, newString } = resolveFileChangeTextFields(file);
|
||||||
|
if (unifiedDiff?.startsWith("diff --git")) {
|
||||||
|
return unifiedDiff.replace(/\r?\n$/, "");
|
||||||
|
}
|
||||||
|
const header = `diff --git a/${file.path} b/${file.path}`;
|
||||||
|
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/${file.path}`;
|
||||||
|
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} @@`);
|
||||||
|
lines.push(...contentLines.map((line) => `+${line}`));
|
||||||
|
}
|
||||||
|
return lines.join("\n");
|
||||||
|
}
|
||||||
|
|
||||||
function mapFileChangeItem(
|
function mapFileChangeItem(
|
||||||
item: z.infer<typeof CodexFileChangeItemSchema>,
|
item: z.infer<typeof CodexFileChangeItemSchema>,
|
||||||
options?: CodexMapperOptions,
|
options?: CodexMapperOptions,
|
||||||
@@ -873,17 +898,16 @@ function mapFileChangeItem(
|
|||||||
const error = item.error ?? null;
|
const error = item.error ?? null;
|
||||||
const status = normalizeToolCallStatus(item.status, error, output);
|
const status = normalizeToolCallStatus(item.status, error, output);
|
||||||
const firstFile = files[0];
|
const firstFile = files[0];
|
||||||
const firstTextFields = resolveFileChangeTextFields(firstFile);
|
const textFields =
|
||||||
const hasFirstTextFields = Object.keys(firstTextFields).length > 0;
|
files.length > 1
|
||||||
|
? { unifiedDiff: files.map(fileChangeToUnifiedDiff).join("\n\n") }
|
||||||
|
: resolveFileChangeTextFields(firstFile);
|
||||||
|
const hasTextFields = Object.keys(textFields).length > 0;
|
||||||
const input = toNullableObject({
|
const input = toNullableObject({
|
||||||
...inputBase,
|
...inputBase,
|
||||||
...(firstFile?.path && hasFirstTextFields ? { path: firstFile.path } : {}),
|
...(firstFile?.path && hasTextFields ? { path: firstFile.path } : {}),
|
||||||
...(hasFirstTextFields && firstTextFields.unifiedDiff
|
...(hasTextFields && textFields.unifiedDiff ? { patch: textFields.unifiedDiff } : {}),
|
||||||
? { patch: firstTextFields.unifiedDiff }
|
...(hasTextFields && textFields.newString ? { content: textFields.newString } : {}),
|
||||||
: {}),
|
|
||||||
...(hasFirstTextFields && firstTextFields.newString
|
|
||||||
? { content: firstTextFields.newString }
|
|
||||||
: {}),
|
|
||||||
});
|
});
|
||||||
|
|
||||||
return {
|
return {
|
||||||
|
|||||||
Reference in New Issue
Block a user