Skip to content

Commit 22546c5

Browse files
HaMMeRSIHaMMeRSI
committed
fix(history): restore attachments during session replay
1 parent ca66e03 commit 22546c5

6 files changed

Lines changed: 149 additions & 41 deletions

File tree

‎src/CodexAcpServer.ts‎

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import * as acp from "@agentclientprotocol/sdk";
22
import {RequestError, type SessionId, type SessionModeState} from "@agentclientprotocol/sdk";
3+
import {readFile} from "node:fs/promises";
4+
import {extname} from "node:path";
5+
import {fileURLToPath, pathToFileURL} from "node:url";
36
import {CodexEventHandler} from "./CodexEventHandler";
47
import {CodexApprovalHandler} from "./CodexApprovalHandler";
58
import {CodexElicitationHandler} from "./CodexElicitationHandler";
@@ -80,6 +83,7 @@ import {
8083
createAgentTextMessageChunk,
8184
createAgentTextThoughtChunk,
8285
createUserMessageChunk,
86+
visibleUserMessageText,
8387
} from "./ContentChunks";
8488
import {
8589
sameThreadGoalSnapshot,
@@ -1232,13 +1236,11 @@ export class CodexAcpServer {
12321236
}
12331237
}
12341238

1235-
private createUserMessageUpdates(item: ThreadItem & { type: "userMessage" }): UpdateSessionEvent[] {
1239+
private async createUserMessageUpdates(item: ThreadItem & { type: "userMessage" }): Promise<UpdateSessionEvent[]> {
12361240
const updates: UpdateSessionEvent[] = [];
1237-
const messageId = item.id;
12381241
for (const input of item.content) {
1239-
const blocks = this.userInputToContentBlocks(input);
1240-
for (const block of blocks) {
1241-
updates.push(createUserMessageChunk(block, messageId));
1242+
for (const block of await this.userInputToContentBlocks(input)) {
1243+
updates.push(createUserMessageChunk(block, item.id));
12421244
}
12431245
}
12441246
return updates;
@@ -1291,20 +1293,39 @@ export class CodexAcpServer {
12911293
};
12921294
}
12931295

1294-
private userInputToContentBlocks(input: UserInput): acp.ContentBlock[] {
1296+
private async userInputToContentBlocks(input: UserInput): Promise<acp.ContentBlock[]> {
12951297
switch (input.type) {
1296-
case "text":
1297-
return input.text.length > 0 ? [{ type: "text", text: input.text }] : [];
1298-
case "image":
1298+
case "text": {
1299+
const visibleText = visibleUserMessageText(input.text);
1300+
return visibleText.length > 0 ? [{ type: "text", text: visibleText }] : [];
1301+
}
1302+
case "image": {
1303+
const match = /^data:(image\/[^;]+);base64,(.+)$/.exec(input.url);
1304+
const mimeType = match?.[1];
1305+
const data = match?.[2];
1306+
if (mimeType && data) {
1307+
return [{ type: "image", mimeType, data }];
1308+
}
12991309
return [{ type: "text", text: this.formatUriAsLink("image", input.url) }];
1310+
}
13001311
case "localImage": {
1301-
const uri = input.path.startsWith("file://") ? input.path : `file://${input.path}`;
1312+
const path = input.path.startsWith("file://") ? fileURLToPath(input.path) : input.path;
1313+
const uri = pathToFileURL(path).href;
1314+
const extension = extname(path).slice(1).toLowerCase();
1315+
const data = extension ? await readFile(path).catch(() => null) : null;
1316+
if (data) {
1317+
const mimeType = `image/${extension === "jpg" ? "jpeg" : extension}`;
1318+
return [{ type: "image", data: data.toString("base64"), mimeType, uri }];
1319+
}
13021320
return [{ type: "text", text: this.formatUriAsLink(null, uri) }];
13031321
}
13041322
case "skill":
13051323
return [{ type: "text", text: `skill:${input.name} (${input.path})` }];
1324+
case "mention": {
1325+
const uri = input.path.startsWith("file://") ? input.path : pathToFileURL(input.path).href;
1326+
return [{ type: "resource_link", name: input.name, uri }];
1327+
}
13061328
}
1307-
return [];
13081329
}
13091330

13101331
private formatUriAsLink(name: string | null, uri: string): string {

‎src/ContentChunks.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,20 @@ import type {UpdateSessionEvent} from "./ACPSessionConnection";
33

44
type AcpMeta = Record<string, unknown>;
55

6+
const FILES_MENTIONED_HEADER = "# Files mentioned by the user:\n";
7+
const REQUEST_MARKER = "\n## My request for Codex:\n";
8+
9+
export function visibleUserMessageText(text: string): string {
10+
const normalized = text.trimStart();
11+
const requestIndex = normalized.indexOf(REQUEST_MARKER);
12+
13+
if (normalized.startsWith(FILES_MENTIONED_HEADER) && requestIndex !== -1) {
14+
return normalized.slice(requestIndex + REQUEST_MARKER.length);
15+
}
16+
17+
return text;
18+
}
19+
620
export function createCodexMessagePhaseMeta(phase: string | null | undefined): AcpMeta | undefined {
721
if (!phase) {
822
return undefined;

‎src/ResponseItemHistoryFallback.ts‎

Lines changed: 5 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { stripShellPrefix } from "./CommandUtils";
66
import type { CommandAction, Thread, ThreadItem } from "./app-server/v2";
77
import { createCommandActionEvent } from "./CodexToolCallMapper";
88
import { createTerminalOutputMeta, type TerminalOutputMode } from "./TerminalOutputMode";
9-
import { createAgentMessageChunk, createCodexMessagePhaseMeta } from "./ContentChunks";
9+
import { createAgentMessageChunk, createCodexMessagePhaseMeta, createUserMessageChunk, visibleUserMessageText } from "./ContentChunks";
1010

1111
type JsonRecord = Record<string, unknown>;
1212
type AcpToolCallEvent = Extract<UpdateSessionEvent, { sessionUpdate: "tool_call" }>;
@@ -262,18 +262,12 @@ function createEventMsgUpdates(record: JsonRecord): UpdateSessionEvent[] | null
262262
}
263263

264264
function createUserMessageEventUpdates(payload: JsonRecord): UpdateSessionEvent[] {
265-
const blocks: ContentBlock[] = [];
266-
const message = stringValue(payload["message"]);
267-
if (message !== null && message.length > 0) {
268-
blocks.push({ type: "text", text: message });
265+
const text = visibleUserMessageText(stringValue(payload["message"]) ?? "");
266+
if (text.length > 0) {
267+
return [createUserMessageChunk({ type: "text", text })];
269268
}
270-
blocks.push(...imageBlocks(payload["images"]));
271-
blocks.push(...imageBlocks(payload["local_images"]));
272269

273-
return blocks.map((content) => ({
274-
sessionUpdate: "user_message_chunk",
275-
content,
276-
}));
270+
return [];
277271
}
278272

279273
function createAgentReasoningEventUpdates(payload: JsonRecord): UpdateSessionEvent[] {
@@ -288,22 +282,6 @@ function createAgentReasoningEventUpdates(payload: JsonRecord): UpdateSessionEve
288282
}];
289283
}
290284

291-
function imageBlocks(images: unknown): ContentBlock[] {
292-
if (!Array.isArray(images)) {
293-
return [];
294-
}
295-
296-
return images.flatMap((image): ContentBlock[] => {
297-
if (typeof image === "string") {
298-
return [{ type: "text", text: `[@image](${image})` }];
299-
}
300-
301-
const record = asRecord(image);
302-
const path = record ? stringValue(record["path"]) ?? stringValue(record["url"]) : null;
303-
return path ? [{ type: "text", text: `[@image](${path})` }] : [];
304-
});
305-
}
306-
307285
function contentBlocksFromResponseContent(content: unknown): ContentBlock[] {
308286
if (!Array.isArray(content)) {
309287
return [];

‎src/__tests__/CodexACPAgent/data/load-session-history.json‎

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,58 @@
151151
}
152152
]
153153
}
154+
{
155+
"method": "sessionUpdate",
156+
"args": [
157+
{
158+
"sessionId": "session-1",
159+
"update": {
160+
"sessionUpdate": "user_message_chunk",
161+
"messageId": "item-user-1",
162+
"content": {
163+
"type": "image",
164+
"mimeType": "image/png",
165+
"data": "dGVzdCBpbWFnZQ=="
166+
}
167+
}
168+
}
169+
]
170+
}
171+
{
172+
"method": "sessionUpdate",
173+
"args": [
174+
{
175+
"sessionId": "session-1",
176+
"update": {
177+
"sessionUpdate": "user_message_chunk",
178+
"messageId": "item-user-1",
179+
"content": {
180+
"type": "image",
181+
"data": "dGVzdCBpbWFnZQ==",
182+
"mimeType": "image/png",
183+
"uri": "file:///tmp/codex-acp-load-session-image.png"
184+
}
185+
}
186+
}
187+
]
188+
}
189+
{
190+
"method": "sessionUpdate",
191+
"args": [
192+
{
193+
"sessionId": "session-1",
194+
"update": {
195+
"sessionUpdate": "user_message_chunk",
196+
"messageId": "item-user-1",
197+
"content": {
198+
"type": "resource_link",
199+
"name": "notes.txt",
200+
"uri": "file:///test/project/notes.txt"
201+
}
202+
}
203+
}
204+
]
205+
}
154206
{
155207
"method": "sessionUpdate",
156208
"args": [
@@ -429,4 +481,4 @@
429481
}
430482
}
431483
]
432-
}
484+
}

‎src/__tests__/CodexACPAgent/load-session.test.ts‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,16 @@ import type * as acp from "@agentclientprotocol/sdk";
33
import { mkdtemp, rm, writeFile } from "node:fs/promises";
44
import { tmpdir } from "node:os";
55
import { join } from "node:path";
6+
import { pathToFileURL } from "node:url";
67
import { createCodexMockTestFixture, createTestModel } from "../acp-test-utils";
78
import type { Model, Thread, ThreadGoal } from "../../app-server/v2";
89

910
describe("CodexACPAgent - loadSession", () => {
1011
it("should replay history during loadSession", async () => {
12+
const localImageDirectory = await mkdtemp(join(tmpdir(), "codex-acp-load-session-"));
13+
const localImagePath = join(localImageDirectory, "image.png");
14+
await writeFile(localImagePath, "test image");
15+
1116
const fixture = createCodexMockTestFixture();
1217
const codexAcpAgent = fixture.getCodexAcpAgent();
1318
const codexAcpClient = fixture.getCodexAcpClient();
@@ -82,8 +87,15 @@ describe("CodexACPAgent - loadSession", () => {
8287
id: "item-user-1",
8388
clientId: null,
8489
content: [
85-
{ type: "text", text: "Hi", text_elements: [] },
90+
{
91+
type: "text",
92+
text: `\n# Files mentioned by the user:\n\n## image.png: ${localImagePath}\n\n## My request for Codex:\nHi`,
93+
text_elements: [],
94+
},
8695
{ type: "image", url: "https://example.com/image.png" },
96+
{ type: "image", url: "data:image/png;base64,dGVzdCBpbWFnZQ==" },
97+
{ type: "localImage", path: localImagePath },
98+
{ type: "mention", name: "notes.txt", path: "/test/project/notes.txt" },
8799
],
88100
},
89101
{
@@ -223,9 +235,14 @@ describe("CodexACPAgent - loadSession", () => {
223235
includeTurns: true,
224236
});
225237
expect(codexAppServerClient.threadGoalGet).toHaveBeenCalledWith({ threadId: thread.id });
226-
await expect(fixture.getAcpConnectionDump([])).toMatchFileSnapshot(
238+
const replay = fixture.getAcpConnectionDump([]).replaceAll(
239+
pathToFileURL(localImagePath).href,
240+
"file:///tmp/codex-acp-load-session-image.png",
241+
);
242+
await expect(`${replay}\n`).toMatchFileSnapshot(
227243
"data/load-session-history.json"
228244
);
245+
await rm(localImageDirectory, { recursive: true });
229246
});
230247

231248
it("should not recover session mcp servers during loadSession when request omits them", async () => {

‎src/__tests__/CodexACPAgent/response-item-history-fallback.test.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,24 @@ describe("ResponseItemHistoryFallback", () => {
5555
expect(thoughtTexts(updates)).toEqual(["Need to inspect the directory."]);
5656
});
5757

58+
it("leaves user attachments to thread history", () => {
59+
const updates = parseResponseItemHistoryFallback(jsonl([
60+
{
61+
type: "event_msg",
62+
payload: {
63+
type: "user_message",
64+
message: "\n# Files mentioned by the user:\n\n## screenshot.png: /tmp/screenshot.png\n\n## My request for Codex:\nInspect the screenshot",
65+
images: [],
66+
local_images: ["/tmp/screenshot.png"],
67+
},
68+
},
69+
functionCall("call-missing", "ls"),
70+
functionCallOutput("call-missing", "Chunk ID: missing\nProcess exited with code 0\nOutput:\nREADME.md\n"),
71+
]), "terminal_output");
72+
73+
expect(userMessageTexts(updates)).toEqual(["Inspect the screenshot"]);
74+
});
75+
5876
it("preserves assistant message phase metadata from response items", () => {
5977
const updates = parseResponseItemHistoryFallback(jsonl([
6078
{
@@ -154,6 +172,14 @@ function thoughtTexts(updates: UpdateSessionEvent[] | null): string[] {
154172
.flatMap((update) => update.content.type === "text" ? [update.content.text] : []);
155173
}
156174

175+
function userMessageTexts(updates: UpdateSessionEvent[] | null): string[] {
176+
return (updates ?? [])
177+
.filter((update): update is Extract<UpdateSessionEvent, { sessionUpdate: "user_message_chunk" }> => (
178+
update.sessionUpdate === "user_message_chunk"
179+
))
180+
.flatMap((update) => update.content.type === "text" ? [update.content.text] : []);
181+
}
182+
157183
function agentMessageMetas(updates: UpdateSessionEvent[] | null): unknown[] {
158184
return (updates ?? [])
159185
.filter((update): update is Extract<UpdateSessionEvent, { sessionUpdate: "agent_message_chunk" }> => (

0 commit comments

Comments
 (0)