From 9b125e9bb1311fc9aa7ebe84eeee998665a84577 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Tue, 16 Jun 2026 16:05:40 +0200 Subject: [PATCH 1/5] Load full thread history on session resume Closes #206 --- src/CodexAcpClient.ts | 6 +- src/CodexAcpServer.ts | 28 +- src/CodexToolCallMapper.ts | 51 +- src/ResponseItemHistoryFallback.ts | 1021 +++++++++++++++++ .../data/load-session-history.json | 28 + ...ession-response-item-history-fallback.json | 202 ++++ .../CodexACPAgent/load-session.test.ts | 333 +++++- 7 files changed, 1620 insertions(+), 49 deletions(-) create mode 100644 src/ResponseItemHistoryFallback.ts create mode 100644 src/__tests__/CodexACPAgent/data/load-session-response-item-history-fallback.json diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index ec77b5e..06fab21 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -237,6 +237,10 @@ export class CodexAcpClient { threadId: request.sessionId, }); onSubscribed?.(); + const historyResponse = await this.codexClient.threadRead({ + threadId: response.thread.id, + includeTurns: true, + }); const codexModels = await this.fetchAvailableModels(); const currentModelId = this.createModelId(codexModels, response.model, response.reasoningEffort).toString(); return { @@ -244,7 +248,7 @@ export class CodexAcpClient { currentModelId: currentModelId, models: codexModels, currentServiceTier: response.serviceTier as ServiceTier ?? null, - thread: response.thread, + thread: historyResponse.thread, additionalDirectories, }; } diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index 57d4e73..48c5e42 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -33,6 +33,7 @@ import {CodexCommands} from "./CodexCommands"; import type {QuotaMeta} from "./QuotaMeta"; import {logger} from "./Logger"; import {sanitizeMcpServerName} from "./McpServerName"; +import {createResponseItemHistoryFallbackUpdates} from "./ResponseItemHistoryFallback"; import { type LegacyLoadSessionResponse, type LegacyNewSessionResponse, @@ -44,6 +45,7 @@ import { LEGACY_SET_SESSION_MODEL_METHOD, } from "./AcpExtensions"; import { + createCommandExecutionCompleteUpdate, createCommandExecutionUpdate, createDynamicToolCallUpdate, createFileChangeUpdate, @@ -850,9 +852,21 @@ export class CodexAcpServer implements acp.Agent { private async streamThreadHistory(sessionId: string, thread: Thread): Promise { const session = new ACPSessionConnection(this.connection, sessionId); + const sessionState = this.getSessionState(sessionId); + const responseItemFallbackUpdates = await createResponseItemHistoryFallbackUpdates( + thread, + sessionState.terminalOutputMode, + ); + if (responseItemFallbackUpdates) { + for (const update of responseItemFallbackUpdates) { + await session.update(update); + } + return; + } + for (const turn of thread.turns) { for (const item of turn.items) { - const updates = await this.createHistoryUpdates(item); + const updates = await this.createHistoryUpdates(item, sessionState); for (const update of updates) { await session.update(update); } @@ -860,7 +874,7 @@ export class CodexAcpServer implements acp.Agent { } } - private async createHistoryUpdates(item: ThreadItem): Promise { + private async createHistoryUpdates(item: ThreadItem, sessionState: SessionState): Promise { switch (item.type) { case "userMessage": return this.createUserMessageUpdates(item); @@ -876,8 +890,14 @@ export class CodexAcpServer implements acp.Agent { return this.createReasoningUpdates(item); case "fileChange": return [await createFileChangeUpdate(item)]; - case "commandExecution": - return [await createCommandExecutionUpdate(item)]; + case "commandExecution": { + const updates = [await createCommandExecutionUpdate(item)]; + const completeUpdate = createCommandExecutionCompleteUpdate(item, sessionState.terminalOutputMode); + if (completeUpdate) { + updates.push(completeUpdate); + } + return updates; + } case "mcpToolCall": return [await createMcpToolCallUpdate(item)]; case "dynamicToolCall": diff --git a/src/CodexToolCallMapper.ts b/src/CodexToolCallMapper.ts index 116c60b..c098bee 100644 --- a/src/CodexToolCallMapper.ts +++ b/src/CodexToolCallMapper.ts @@ -27,6 +27,10 @@ import type { } from "./app-server/v2"; import type { JsonValue } from "./app-server/serde_json/JsonValue"; import {logger} from "./Logger"; +import { + createTerminalOutputMeta, + type TerminalOutputMode, +} from "./TerminalOutputMode"; type CodexItemStatus = CommandExecutionStatus | PatchApplyStatus | McpToolCallStatus | DynamicToolCallStatus; type AcpToolCallStatus = "pending" | "in_progress" | "completed" | "failed"; @@ -87,6 +91,47 @@ export async function createCommandExecutionUpdate(item: CommandExecutionItem): }, item.id, item.cwd); } +export function createCommandExecutionCompleteUpdate( + item: CommandExecutionItem, + terminalOutputMode: TerminalOutputMode, +): UpdateSessionEvent | null { + if (item.status === "inProgress") { + return null; + } + + const update: UpdateSessionEvent = { + sessionUpdate: "tool_call_update", + toolCallId: item.id, + status: item.status === "completed" ? "completed" : "failed", + rawOutput: { + formatted_output: item.aggregatedOutput ?? "", + exit_code: item.exitCode, + }, + }; + + if (!commandExecutionUsesTerminalOutput(item)) { + return update; + } + + const terminalMeta: Record = {}; + if (item.aggregatedOutput) { + Object.assign( + terminalMeta, + createTerminalOutputMeta(terminalOutputMode, item.id, item.aggregatedOutput), + ); + } + terminalMeta["terminal_exit"] = { + exit_code: item.exitCode, + signal: null, + terminal_id: item.id, + }; + + return { + ...update, + _meta: terminalMeta, + }; +} + export async function createMcpToolCallUpdate( item: ThreadItem & { type: "mcpToolCall" } ): Promise { @@ -334,12 +379,12 @@ export function formatWebSearchTitle(item: WebSearchItem): string { } } -function createCommandActionEvent( +export function createCommandActionEvent( id: string, status: CommandExecutionStatus, cwd: string, commandAction: CommandAction -): UpdateSessionEvent { +): AcpToolCallEvent { const acpStatus = toAcpStatus(status); switch (commandAction.type) { case "read": @@ -395,7 +440,7 @@ function createTerminalCommandEvent( event: AcpToolCallEvent, terminalId: string, cwd: string, -): UpdateSessionEvent { +): AcpToolCallEvent { const { rawInput, ...eventWithoutRawInput } = event; return { ...eventWithoutRawInput, diff --git a/src/ResponseItemHistoryFallback.ts b/src/ResponseItemHistoryFallback.ts new file mode 100644 index 0000000..b9fdd41 --- /dev/null +++ b/src/ResponseItemHistoryFallback.ts @@ -0,0 +1,1021 @@ +import { readFile } from "node:fs/promises"; +import path from "node:path"; +import type { ContentBlock } from "@agentclientprotocol/sdk"; +import type { UpdateSessionEvent } from "./ACPSessionConnection"; +import { stripShellPrefix } from "./CommandUtils"; +import type { CommandAction, Thread, ThreadItem } from "./app-server/v2"; +import { createCommandActionEvent } from "./CodexToolCallMapper"; +import { createTerminalOutputMeta, type TerminalOutputMode } from "./TerminalOutputMode"; + +type JsonRecord = Record; +type AcpToolCallEvent = Extract; +type AcpToolKind = NonNullable; +type AcpToolCallUpdateStatus = NonNullable["status"]>; +type LegacyFunctionCallUpdate = { + update: AcpToolCallEvent; + usesTerminal: boolean; +}; +type ParsedShellCommand = { + tokens: string[]; +}; + +export async function createResponseItemHistoryFallbackUpdates( + thread: Thread, + terminalOutputMode: TerminalOutputMode, +): Promise { + if (!thread.path || threadHasToolItems(thread)) { + return null; + } + + let contents: string; + try { + contents = await readFile(thread.path, "utf8"); + } catch { + return null; + } + + return parseResponseItemHistoryFallback(contents, terminalOutputMode); +} + +export function parseResponseItemHistoryFallback( + contents: string, + terminalOutputMode: TerminalOutputMode, +): UpdateSessionEvent[] | null { + const updates: UpdateSessionEvent[] = []; + const terminalToolCallIds = new Set(); + let sawFunctionCall = false; + + for (const line of contents.split(/\r?\n/)) { + const record = parseJsonRecord(line); + if (!record) { + continue; + } + + const eventMsgUpdates = createEventMsgUpdates(record); + if (eventMsgUpdates) { + updates.push(...eventMsgUpdates); + continue; + } + + const item = extractResponseItem(record); + if (!item) { + continue; + } + + switch (item["type"]) { + case "message": + updates.push(...createMessageUpdates(item)); + break; + case "reasoning": + updates.push(...createReasoningUpdates(item)); + break; + case "function_call": { + const result = createFunctionCallUpdate(item); + if (!result) { + break; + } + sawFunctionCall = true; + if (result.usesTerminal) { + terminalToolCallIds.add(result.update.toolCallId); + } + updates.push(result.update); + break; + } + case "function_call_output": { + const update = createFunctionCallOutputUpdate(item, terminalOutputMode, terminalToolCallIds); + if (update) { + updates.push(update); + } + break; + } + default: + break; + } + } + + return sawFunctionCall ? updates : null; +} + +function threadHasToolItems(thread: Thread): boolean { + return thread.turns.some((turn) => turn.items.some(isToolThreadItem)); +} + +function isToolThreadItem(item: ThreadItem): boolean { + switch (item.type) { + case "commandExecution": + case "fileChange": + case "mcpToolCall": + case "dynamicToolCall": + case "collabAgentToolCall": + case "webSearch": + case "imageView": + case "imageGeneration": + return true; + case "userMessage": + case "hookPrompt": + case "agentMessage": + case "plan": + case "reasoning": + case "subAgentActivity": + case "enteredReviewMode": + case "exitedReviewMode": + case "contextCompaction": + return false; + } +} + +function parseJsonRecord(line: string): JsonRecord | null { + if (line.trim().length === 0) { + return null; + } + try { + const value: unknown = JSON.parse(line); + return asRecord(value); + } catch { + return null; + } +} + +function extractResponseItem(record: JsonRecord): JsonRecord | null { + if (record["type"] === "response_item") { + return asRecord(record["payload"]); + } + const itemType = record["type"]; + if (typeof itemType === "string" && isLegacyResponseItemType(itemType)) { + return record; + } + return null; +} + +function isLegacyResponseItemType(type: string): boolean { + switch (type) { + case "message": + case "reasoning": + case "function_call": + case "function_call_output": + return true; + default: + return false; + } +} + +function createMessageUpdates(item: JsonRecord): UpdateSessionEvent[] { + const role = item["role"]; + if (role === "user") { + // User response items can include bootstrap context; user_message events are the visible source. + return []; + } + if (role !== "assistant") { + return []; + } + + return contentBlocksFromResponseContent(item["content"]).map((content) => ({ + sessionUpdate: "agent_message_chunk", + content, + })); +} + +function createEventMsgUpdates(record: JsonRecord): UpdateSessionEvent[] | null { + if (record["type"] !== "event_msg") { + return null; + } + + const payload = asRecord(record["payload"]); + if (!payload) { + return []; + } + + switch (payload["type"]) { + case "user_message": + return createUserMessageEventUpdates(payload); + default: + return []; + } +} + +function createUserMessageEventUpdates(payload: JsonRecord): UpdateSessionEvent[] { + const blocks: ContentBlock[] = []; + const message = stringValue(payload["message"]); + if (message !== null && message.length > 0) { + blocks.push({ type: "text", text: message }); + } + blocks.push(...imageBlocks(payload["images"])); + blocks.push(...imageBlocks(payload["local_images"])); + + return blocks.map((content) => ({ + sessionUpdate: "user_message_chunk", + content, + })); +} + +function imageBlocks(images: unknown): ContentBlock[] { + if (!Array.isArray(images)) { + return []; + } + + return images.flatMap((image): ContentBlock[] => { + if (typeof image === "string") { + return [{ type: "text", text: `[@image](${image})` }]; + } + + const record = asRecord(image); + const path = record ? stringValue(record["path"]) ?? stringValue(record["url"]) : null; + return path ? [{ type: "text", text: `[@image](${path})` }] : []; + }); +} + +function contentBlocksFromResponseContent(content: unknown): ContentBlock[] { + if (!Array.isArray(content)) { + return []; + } + + return content.flatMap((entry): ContentBlock[] => { + const record = asRecord(entry); + if (!record) { + return []; + } + + const text = stringValue(record["text"]); + if (text !== null) { + return [{ type: "text", text }]; + } + + const imageUrl = stringValue(record["image_url"]); + if (imageUrl !== null) { + return [{ type: "text", text: `[@image](${imageUrl})` }]; + } + + return []; + }); +} + +function createReasoningUpdates(item: JsonRecord): UpdateSessionEvent[] { + const parts = textParts(item["summary"]); + if (parts.length === 0) { + parts.push(...textParts(item["content"])); + } + + return parts.map((text) => ({ + sessionUpdate: "agent_thought_chunk", + content: { type: "text", text }, + })); +} + +function textParts(value: unknown): string[] { + if (!Array.isArray(value)) { + return []; + } + + return value.flatMap((entry): string[] => { + if (typeof entry === "string") { + return entry.length > 0 ? [entry] : []; + } + + const record = asRecord(entry); + if (!record) { + return []; + } + + const text = stringValue(record["text"]); + return text !== null && text.length > 0 ? [text] : []; + }); +} + +function createFunctionCallUpdate(item: JsonRecord): LegacyFunctionCallUpdate | null { + const toolCallId = stringValue(item["call_id"]); + const name = stringValue(item["name"]); + if (!toolCallId || !name) { + return null; + } + + const args = parseFunctionArguments(item["arguments"]); + const command = name === "exec_command" ? commandFromFunctionArguments(args) : null; + const cwd = name === "exec_command" ? cwdFromFunctionArguments(args) : ""; + const commandAction = command ? inferCommandAction(command, cwd) : null; + if (commandAction) { + return { + update: createCommandActionEvent(toolCallId, "inProgress", cwd, commandAction), + usesTerminal: false, + }; + } + + const update: AcpToolCallEvent = { + sessionUpdate: "tool_call", + toolCallId, + kind: toolKindForFunctionCall(name), + title: titleForFunctionCall(name, args), + status: "in_progress", + rawInput: rawInputForFunctionCall(name, args), + }; + + if (!functionCallUsesTerminal(item)) { + return { update, usesTerminal: false }; + } + + return { + update: withTerminalContent(update, toolCallId, cwd), + usesTerminal: true, + }; +} + +function createFunctionCallOutputUpdate( + item: JsonRecord, + terminalOutputMode: TerminalOutputMode, + terminalToolCallIds: Set, +): UpdateSessionEvent | null { + const toolCallId = stringValue(item["call_id"]); + if (!toolCallId) { + return null; + } + + const output = outputText(item["output"]); + const exitCode = parseExitCode(output); + const status = statusFromExitCode(exitCode); + if (!terminalToolCallIds.has(toolCallId)) { + return { + sessionUpdate: "tool_call_update", + toolCallId, + status: "completed", + rawOutput: { output: item["output"] }, + }; + } + + const meta: Record = { + terminal_exit: { + exit_code: exitCode, + signal: null, + terminal_id: toolCallId, + }, + }; + if (output.length > 0) { + Object.assign(meta, createTerminalOutputMeta(terminalOutputMode, toolCallId, output)); + } + + return { + sessionUpdate: "tool_call_update", + toolCallId, + status, + rawOutput: { + formatted_output: output, + exit_code: exitCode, + }, + _meta: meta, + }; +} + +function parseFunctionArguments(value: unknown): unknown { + if (typeof value !== "string") { + return value; + } + + try { + return JSON.parse(value) as unknown; + } catch { + return value; + } +} + +function rawInputForFunctionCall(name: string, args: unknown): unknown { + if (name === "exec_command") { + const record = asRecord(args); + if (record) { + return { + command: stringValue(record["cmd"]) ?? stringValue(record["command"]), + cwd: stringValue(record["workdir"]) ?? stringValue(record["cwd"]), + arguments: args, + }; + } + } + + return { + name, + arguments: args, + }; +} + +function titleForFunctionCall(name: string, args: unknown): string { + if (name === "exec_command") { + const command = commandFromFunctionArguments(args); + return command ? stripShellPrefix(command) : "Run command"; + } + + if (name === "apply_patch") { + return "Apply patch"; + } + + if (name === "multi_tool_use.parallel") { + return "Run tools in parallel"; + } + + if (name === "view_image") { + return "View image"; + } + + return name; +} + +function toolKindForFunctionCall(name: string): AcpToolKind { + switch (name) { + case "exec_command": + case "multi_tool_use.parallel": + return "execute"; + case "apply_patch": + return "edit"; + case "view_image": + return "read"; + default: + return "other"; + } +} + +function functionCallUsesTerminal(item: JsonRecord): boolean { + return item["name"] === "exec_command"; +} + +function commandFromFunctionArguments(args: unknown): string | null { + const record = asRecord(args); + if (!record) { + return null; + } + return stringValue(record["cmd"]) ?? stringValue(record["command"]); +} + +function cwdFromFunctionArguments(args: unknown): string { + const record = asRecord(args); + if (!record) { + return ""; + } + return stringValue(record["workdir"]) ?? stringValue(record["cwd"]) ?? ""; +} + +function inferCommandAction(command: string, cwd: string): CommandAction | null { + const tokens = commandTokensForClassification(command); + if (!tokens || tokens.length === 0) { + return null; + } + + const commandName = baseCommandName(tokens[0] ?? ""); + switch (commandName) { + case "ls": + case "dir": + case "eza": + case "exa": + case "tree": + case "du": + return { + type: "listFiles", + command, + path: firstPathLikeArgument(tokens.slice(1)), + }; + case "find": + return { + type: "listFiles", + command, + path: firstFindPath(tokens.slice(1)), + }; + case "cat": + case "less": + case "more": + case "head": + case "tail": + case "nl": + case "sed": { + const readPath = lastPathLikeArgument(tokens.slice(1)); + if (!readPath) { + return null; + } + return { + type: "read", + command, + name: commandName, + path: absolutizePath(cwd, readPath), + }; + } + case "rg": + case "grep": + return inferSearchAction(command, tokens.slice(1)); + case "git": + return inferGitAction(command, tokens.slice(1)); + default: + return null; + } +} + +function commandTokensForClassification(command: string): string[] | null { + const pipeline = parseShellPipeline(command); + if (!pipeline || pipeline.length === 0) { + return null; + } + + if (pipeline.length === 1 && isShellInvocation(pipeline[0] ?? [])) { + const script = pipeline[0]?.[2]; + if (!script) { + return null; + } + const nestedPipeline = parseShellPipeline(script); + return nestedPipeline ? primaryPipelineTokens(nestedPipeline) : null; + } + + return primaryPipelineTokens(pipeline); +} + +function primaryPipelineTokens(pipeline: string[][]): string[] | null { + const primary = pipeline[0]; + if (!primary || primary.length === 0) { + return null; + } + if (pipeline.length > 1 && !pipeline.slice(1).every(isSmallFormattingCommand)) { + return null; + } + return primary; +} + +function isShellInvocation(tokens: string[]): boolean { + if (tokens.length < 3) { + return false; + } + + const commandName = baseCommandName(tokens[0] ?? ""); + if (commandName !== "bash" && commandName !== "zsh" && commandName !== "sh") { + return false; + } + + const flag = tokens[1]; + if (flag !== "-lc" && flag !== "-c") { + return false; + } + + const script = tokens[2]; + return !!script && tokens.length === 3; +} + +function inferSearchAction(command: string, args: string[]): CommandAction | null { + if (args.includes("--files")) { + return { + type: "listFiles", + command, + path: globArgument(args) ?? firstPathLikeArgument(args), + }; + } + + const positionals = positionalArguments(args, searchOptionsWithValues()); + const query = positionals[0] ?? null; + const searchPath = positionals[1] ?? null; + if (query === null && searchPath === null) { + return null; + } + + return { + type: "search", + command, + query, + path: searchPath, + }; +} + +function inferGitAction(command: string, args: string[]): CommandAction | null { + const subcommand = args[0]; + if (subcommand === "grep") { + const positionals = positionalArguments(args.slice(1), searchOptionsWithValues()); + const query = positionals[0] ?? null; + const searchPath = positionals[1] ?? null; + if (query === null && searchPath === null) { + return null; + } + return { + type: "search", + command, + query, + path: searchPath, + }; + } + + if (subcommand === "ls-files") { + return { + type: "listFiles", + command, + path: lastPathLikeArgument(args.slice(1)), + }; + } + + return null; +} + +function parseShellPipeline(command: string): string[][] | null { + const segments: string[] = []; + let current = ""; + let quote: "'" | "\"" | null = null; + + for (let index = 0; index < command.length; index += 1) { + const char = command[index]; + if (char === undefined) { + continue; + } + + if (quote) { + current += char; + if (char === quote) { + quote = null; + } else if (char === "\\" && quote === "\"" && index + 1 < command.length) { + index += 1; + current += command[index] ?? ""; + } + continue; + } + + if (char === "'" || char === "\"") { + quote = char; + current += char; + continue; + } + if (char === "\\") { + current += char; + index += 1; + current += command[index] ?? ""; + continue; + } + if (char === "|") { + if (current.trim().length === 0) { + return null; + } + segments.push(current.trim()); + current = ""; + continue; + } + if (isUnsupportedShellControlChar(char)) { + return null; + } + + current += char; + } + + if (quote || current.trim().length === 0) { + return null; + } + segments.push(current.trim()); + + const parsedSegments = segments.map(parseShellWords); + if (parsedSegments.some((segment) => segment === null)) { + return null; + } + return parsedSegments.map((segment) => segment?.tokens ?? []); +} + +function parseShellWords(command: string): ParsedShellCommand | null { + const tokens: string[] = []; + let current = ""; + let quote: "'" | "\"" | null = null; + let hadQuotedContent = false; + + for (let index = 0; index < command.length; index += 1) { + const char = command[index]; + if (char === undefined) { + continue; + } + + if (quote) { + if (char === quote) { + quote = null; + hadQuotedContent = true; + } else if (char === "\\" && quote === "\"" && index + 1 < command.length) { + index += 1; + current += command[index] ?? ""; + } else { + current += char; + } + continue; + } + + if (char === "'" || char === "\"") { + quote = char; + continue; + } + if (char === "\\") { + index += 1; + current += command[index] ?? ""; + continue; + } + if (/\s/.test(char)) { + if (current.length > 0 || hadQuotedContent) { + tokens.push(current); + current = ""; + hadQuotedContent = false; + } + continue; + } + if (isShellControlChar(char)) { + return null; + } + + current += char; + } + + if (quote) { + return null; + } + if (current.length > 0 || hadQuotedContent) { + tokens.push(current); + } + return { tokens }; +} + +function isShellControlChar(char: string): boolean { + return char === "|" || char === ";" || char === "&" || char === "<" || char === ">" + || char === "(" || char === ")" || char === "$" || char === "`"; +} + +function isUnsupportedShellControlChar(char: string): boolean { + return char === ";" || char === "&" || char === "<" || char === ">" + || char === "(" || char === ")" || char === "$" || char === "`"; +} + +function isSmallFormattingCommand(tokens: string[]): boolean { + const commandName = baseCommandName(tokens[0] ?? ""); + switch (commandName) { + case "sed": + return sedFileArguments(tokens.slice(1)).length === 0; + case "head": + case "tail": + return headTailFileArguments(tokens.slice(1)).length === 0; + case "nl": + return positionalArguments(tokens.slice(1), genericOptionsWithValues()).length === 0; + case "sort": + case "uniq": + case "wc": + case "cut": + case "tr": + case "column": + return true; + default: + return false; + } +} + +function positionalArguments(args: string[], optionsWithValues: Set): string[] { + const positionals: string[] = []; + let endOfOptions = false; + for (let index = 0; index < args.length; index += 1) { + const arg = args[index]; + if (arg === undefined) { + continue; + } + if (!endOfOptions && arg === "--") { + endOfOptions = true; + continue; + } + if (!endOfOptions && arg.startsWith("--") && arg.includes("=")) { + continue; + } + if (!endOfOptions && optionsWithValues.has(arg)) { + index += 1; + continue; + } + if (!endOfOptions && combinedShortOptionTakesValue(arg, optionsWithValues)) { + continue; + } + if (!endOfOptions && arg.startsWith("-") && arg !== "-") { + continue; + } + if (arg !== "-") { + positionals.push(arg); + } + } + return positionals; +} + +function combinedShortOptionTakesValue(arg: string, optionsWithValues: Set): boolean { + if (!arg.startsWith("-") || arg.startsWith("--") || arg.length < 3) { + return false; + } + return optionsWithValues.has(arg.slice(0, 2)); +} + +function searchOptionsWithValues(): Set { + return new Set([ + "-A", + "-B", + "-C", + "-e", + "-f", + "-g", + "-m", + "-t", + "-T", + "--after-context", + "--before-context", + "--context", + "--glob", + "--iglob", + "--max-count", + "--regexp", + "--type", + "--type-not", + ]); +} + +function firstPathLikeArgument(args: string[]): string | null { + return positionalArguments(args, genericOptionsWithValues())[0] ?? null; +} + +function lastPathLikeArgument(args: string[]): string | null { + const positionals = positionalArguments(args, genericOptionsWithValues()); + return positionals[positionals.length - 1] ?? null; +} + +function firstFindPath(args: string[]): string | null { + const first = args.find((arg) => arg !== undefined && !arg.startsWith("-")); + return first ?? null; +} + +function globArgument(args: string[]): string | null { + for (let index = 0; index < args.length; index += 1) { + const arg = args[index]; + if (arg === "-g" || arg === "--glob" || arg === "--iglob") { + return args[index + 1] ?? null; + } + const longGlob = arg?.match(/^--(?:i?glob)=(.+)$/); + if (longGlob?.[1]) { + return longGlob[1]; + } + } + return null; +} + +function genericOptionsWithValues(): Set { + return new Set([ + "-I", + "-L", + "-c", + "-d", + "-e", + "-f", + "-g", + "-m", + "-s", + "-t", + "-u", + "--exclude", + "--exclude-from", + "--glob", + ]); +} + +function sedFileArguments(args: string[]): string[] { + const files: string[] = []; + let endOfOptions = false; + for (let index = 0; index < args.length; index += 1) { + const arg = args[index]; + if (arg === undefined) { + continue; + } + if (!endOfOptions && arg === "--") { + endOfOptions = true; + continue; + } + if (!endOfOptions && arg === "-n") { + continue; + } + if (!endOfOptions && (arg === "-e" || arg === "-f")) { + index += 1; + continue; + } + if (!endOfOptions && arg.startsWith("-") && arg !== "-") { + continue; + } + if (looksLikeSedRangeScript(arg)) { + continue; + } + if (arg !== "-") { + files.push(arg); + } + } + return files; +} + +function looksLikeSedRangeScript(arg: string): boolean { + return /^(\d+|\$)?(,(\d+|\$))?[pd]$/.test(arg) + || /^(\d+|\$)?(,(\d+|\$))?p$/.test(arg); +} + +function headTailFileArguments(args: string[]): string[] { + const files: string[] = []; + let endOfOptions = false; + for (let index = 0; index < args.length; index += 1) { + const arg = args[index]; + if (arg === undefined) { + continue; + } + if (!endOfOptions && arg === "--") { + endOfOptions = true; + continue; + } + if (!endOfOptions && (arg === "-n" || arg === "-c")) { + index += 1; + continue; + } + if (!endOfOptions && (arg.startsWith("-n") || arg.startsWith("-c")) && arg.length > 2) { + continue; + } + if (!endOfOptions && arg.startsWith("-") && arg !== "-" && !/^[+-]\d/.test(arg)) { + continue; + } + if (/^[+-]\d/.test(arg)) { + continue; + } + if (arg !== "-") { + files.push(arg); + } + } + return files; +} + +function baseCommandName(commandName: string): string { + return commandName.split(/[\\/]/).pop() ?? commandName; +} + +function absolutizePath(cwd: string, targetPath: string): string { + if (path.isAbsolute(targetPath) || cwd.length === 0) { + return targetPath; + } + return path.join(cwd, targetPath); +} + +function withTerminalContent( + event: AcpToolCallEvent, + terminalId: string, + cwd: string, +): AcpToolCallEvent { + const { rawInput, ...eventWithoutRawInput } = event; + return { + ...eventWithoutRawInput, + content: [{ type: "terminal", terminalId }], + ...(rawInput === undefined ? {} : { rawInput }), + _meta: { + terminal_info: { + cwd, + terminal_id: terminalId, + }, + }, + }; +} + +function outputText(output: unknown): string { + if (typeof output === "string") { + return output; + } + + if (!Array.isArray(output)) { + return ""; + } + + return output.flatMap((entry): string[] => { + const record = asRecord(entry); + if (!record) { + return []; + } + + const text = stringValue(record["text"]); + if (text !== null) { + return [text]; + } + + const imageUrl = stringValue(record["image_url"]); + if (imageUrl !== null) { + return [`[@image](${imageUrl})`]; + } + + return []; + }).join("\n"); +} + +function parseExitCode(output: string): number | null { + const match = output.match(/Process exited with code (-?\d+)/); + if (!match) { + return null; + } + + const exitCodeText = match[1]; + if (exitCodeText === undefined) { + return null; + } + + const exitCode = Number.parseInt(exitCodeText, 10); + return Number.isFinite(exitCode) ? exitCode : null; +} + +function statusFromExitCode(exitCode: number | null): AcpToolCallUpdateStatus { + return exitCode === null || exitCode === 0 ? "completed" : "failed"; +} + +function asRecord(value: unknown): JsonRecord | null { + if (value === null || typeof value !== "object" || Array.isArray(value)) { + return null; + } + return value as JsonRecord; +} + +function stringValue(value: unknown): string | null { + return typeof value === "string" ? value : null; +} diff --git a/src/__tests__/CodexACPAgent/data/load-session-history.json b/src/__tests__/CodexACPAgent/data/load-session-history.json index 0a41c1c..c8d0b1e 100644 --- a/src/__tests__/CodexACPAgent/data/load-session-history.json +++ b/src/__tests__/CodexACPAgent/data/load-session-history.json @@ -148,6 +148,34 @@ } ] } +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "session-1", + "update": { + "sessionUpdate": "tool_call_update", + "toolCallId": "item-cmd-1", + "status": "completed", + "rawOutput": { + "formatted_output": "Added.txt\nREADME.md\n", + "exit_code": 0 + }, + "_meta": { + "terminal_output_delta": { + "data": "Added.txt\nREADME.md\n", + "terminal_id": "item-cmd-1" + }, + "terminal_exit": { + "exit_code": 0, + "signal": null, + "terminal_id": "item-cmd-1" + } + } + } + } + ] +} { "method": "sessionUpdate", "args": [ diff --git a/src/__tests__/CodexACPAgent/data/load-session-response-item-history-fallback.json b/src/__tests__/CodexACPAgent/data/load-session-response-item-history-fallback.json new file mode 100644 index 0000000..24815d8 --- /dev/null +++ b/src/__tests__/CodexACPAgent/data/load-session-response-item-history-fallback.json @@ -0,0 +1,202 @@ +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "session-legacy", + "update": { + "sessionUpdate": "available_commands_update", + "availableCommands": [ + { + "name": "mcp", + "description": "List configured Model Context Protocol (MCP) tools.", + "input": null + }, + { + "name": "skills", + "description": "List available skills.", + "input": null + }, + { + "name": "status", + "description": "Display session configuration and token usage.", + "input": null + }, + { + "name": "review", + "description": "Review uncommitted changes, or review with custom instructions.", + "input": { + "hint": "optional review instructions" + } + }, + { + "name": "review-branch", + "description": "Review changes relative to a base branch.", + "input": { + "hint": "branch name" + } + }, + { + "name": "review-commit", + "description": "Review a specific commit.", + "input": { + "hint": "commit sha" + } + }, + { + "name": "compact", + "description": "Summarize conversation to avoid hitting the context limit.", + "input": null + }, + { + "name": "logout", + "description": "Sign out of Codex. This option is available when you are logged in via ChatGPT.", + "input": null + } + ] + } + } + ] +} +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "session-legacy", + "update": { + "sessionUpdate": "user_message_chunk", + "content": { + "type": "text", + "text": "List the files" + } + } + } + ] +} +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "session-legacy", + "update": { + "sessionUpdate": "agent_thought_chunk", + "content": { + "type": "text", + "text": "Need to inspect the directory." + } + } + } + ] +} +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "session-legacy", + "update": { + "sessionUpdate": "tool_call", + "toolCallId": "call-rg", + "status": "in_progress", + "kind": "search", + "title": "Search for 'Service' in src" + } + } + ] +} +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "session-legacy", + "update": { + "sessionUpdate": "tool_call_update", + "toolCallId": "call-rg", + "status": "completed", + "rawOutput": { + "output": "Chunk ID: search123\nWall time: 0.0000 seconds\nProcess exited with code 0\nOutput:\nsrc/service.ts:export class Service {}\n" + } + } + } + ] +} +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "session-legacy", + "update": { + "sessionUpdate": "tool_call", + "toolCallId": "call-read", + "status": "in_progress", + "kind": "read", + "title": "Read file '/test/project/src/index.ts'", + "locations": [ + { + "path": "/test/project/src/index.ts" + } + ] + } + } + ] +} +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "session-legacy", + "update": { + "sessionUpdate": "tool_call_update", + "toolCallId": "call-read", + "status": "completed", + "rawOutput": { + "output": "Chunk ID: read123\nWall time: 0.0000 seconds\nProcess exited with code 0\nOutput:\n 1\tconsole.log('hi');\n" + } + } + } + ] +} +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "session-legacy", + "update": { + "sessionUpdate": "tool_call", + "toolCallId": "call-ls", + "status": "in_progress", + "kind": "read", + "title": "List files" + } + } + ] +} +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "session-legacy", + "update": { + "sessionUpdate": "tool_call_update", + "toolCallId": "call-ls", + "status": "completed", + "rawOutput": { + "output": "Chunk ID: abc123\nWall time: 0.0000 seconds\nProcess exited with code 0\nOutput:\nREADME.md\nsrc\n" + } + } + } + ] +} +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "session-legacy", + "update": { + "sessionUpdate": "agent_message_chunk", + "content": { + "type": "text", + "text": "The directory contains README.md and src." + } + } + } + ] +} \ No newline at end of file diff --git a/src/__tests__/CodexACPAgent/load-session.test.ts b/src/__tests__/CodexACPAgent/load-session.test.ts index bf52dd6..f1d29e0 100644 --- a/src/__tests__/CodexACPAgent/load-session.test.ts +++ b/src/__tests__/CodexACPAgent/load-session.test.ts @@ -1,6 +1,9 @@ import { describe, it, expect, vi } from "vitest"; import type * as acp from "@agentclientprotocol/sdk"; -import { createCodexMockTestFixture } from "../acp-test-utils"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { createCodexMockTestFixture, createTestModel } from "../acp-test-utils"; import type { Model, Thread } from "../../app-server/v2"; describe("CodexACPAgent - loadSession", () => { @@ -104,7 +107,7 @@ describe("CodexACPAgent - loadSession", () => { source: "agent", status: "completed", commandActions: [], - aggregatedOutput: null, + aggregatedOutput: "Added.txt\nREADME.md\n", exitCode: 0, durationMs: 5, }, @@ -160,9 +163,17 @@ describe("CodexACPAgent - loadSession", () => { }, ], }; + const resumeThread: Thread = { + ...thread, + turns: thread.turns.map((turn) => ({ + ...turn, + itemsView: "summary", + items: turn.items.filter((item) => item.type === "userMessage" || item.type === "agentMessage"), + })), + }; codexAppServerClient.threadResume = vi.fn().mockResolvedValue({ - thread: thread, + thread: resumeThread, model: model.id, modelProvider: "openai", cwd: "/test/project", @@ -170,6 +181,9 @@ describe("CodexACPAgent - loadSession", () => { sandbox: { type: "dangerFullAccess" }, reasoningEffort: model.defaultReasoningEffort, }); + codexAppServerClient.threadRead = vi.fn().mockResolvedValue({ + thread: thread, + }); await codexAcpAgent.initialize({ protocolVersion: 1 }); @@ -180,6 +194,10 @@ describe("CodexACPAgent - loadSession", () => { }; await codexAcpAgent.loadSession(loadParams); + expect(codexAppServerClient.threadRead).toHaveBeenCalledWith({ + threadId: thread.id, + includeTurns: true, + }); await expect(fixture.getAcpConnectionDump([])).toMatchFileSnapshot( "data/load-session-history.json" ); @@ -221,26 +239,30 @@ describe("CodexACPAgent - loadSession", () => { data: [model], nextCursor: null, }); + const thread: Thread = { + id: "session-1", + sessionId: "session-1", + parentThreadId: null, + threadSource: null, + forkedFromId: null, + preview: "", + ephemeral: false, + modelProvider: "openai", + createdAt: 0, + updatedAt: 0, + status: { type: "idle" }, + path: null, + cwd: "/test/project", + cliVersion: "0.0.0", + source: "cli", + agentNickname: null, + agentRole: null, + gitInfo: null, + name: null, + turns: [], + }; codexAppServerClient.threadResume = vi.fn().mockResolvedValue({ - thread: { - id: "session-1", - forkedFromId: null, - preview: "", - ephemeral: false, - modelProvider: "openai", - createdAt: 0, - updatedAt: 0, - status: { type: "idle" }, - path: null, - cwd: "/test/project", - cliVersion: "0.0.0", - source: "cli", - agentNickname: null, - agentRole: null, - gitInfo: null, - name: null, - turns: [], - }, + thread: thread, model: model.id, modelProvider: "openai", cwd: "/test/project", @@ -248,6 +270,9 @@ describe("CodexACPAgent - loadSession", () => { sandbox: { type: "dangerFullAccess" }, reasoningEffort: model.defaultReasoningEffort, }); + codexAppServerClient.threadRead = vi.fn().mockResolvedValue({ + thread: thread, + }); await codexAcpAgent.initialize({ protocolVersion: 1 }); await codexAcpAgent.loadSession({ @@ -259,6 +284,225 @@ describe("CodexACPAgent - loadSession", () => { expect(codexAcpAgent.getSessionState("session-1").sessionMcpServers).toEqual([]); }); + it("should recover response item function calls when app-server history omits tool items", async () => { + const fixture = createCodexMockTestFixture(); + const codexAcpAgent = fixture.getCodexAcpAgent(); + const codexAcpClient = fixture.getCodexAcpClient(); + const codexAppServerClient = fixture.getCodexAppServerClient(); + const tempDir = await mkdtemp(join(tmpdir(), "codex-acp-rollout-history-")); + + try { + const rolloutPath = join(tempDir, "rollout.jsonl"); + const rolloutRecords = [ + { + type: "response_item", + payload: { + type: "message", + role: "user", + content: [{ type: "input_text", text: "# AGENTS.md\n\nHidden bootstrap context" }], + }, + }, + { + type: "event_msg", + payload: { + type: "user_message", + message: "List the files", + images: [], + local_images: [], + text_elements: [], + }, + }, + { + type: "response_item", + payload: { + type: "message", + role: "user", + content: [{ type: "input_text", text: "List the files" }], + }, + }, + { + type: "response_item", + payload: { + type: "reasoning", + summary: [{ type: "summary_text", text: "Need to inspect the directory." }], + encrypted_content: null, + }, + }, + { + type: "response_item", + payload: { + type: "function_call", + name: "exec_command", + arguments: JSON.stringify({ + cmd: "rg \"Service\" src | head -n 20", + workdir: "/test/project", + yield_time_ms: 1000, + }), + call_id: "call-rg", + }, + }, + { + type: "response_item", + payload: { + type: "function_call_output", + call_id: "call-rg", + output: "Chunk ID: search123\nWall time: 0.0000 seconds\nProcess exited with code 0\nOutput:\nsrc/service.ts:export class Service {}\n", + }, + }, + { + type: "response_item", + payload: { + type: "function_call", + name: "exec_command", + arguments: JSON.stringify({ + cmd: "nl -ba src/index.ts | sed -n '1,40p'", + workdir: "/test/project", + yield_time_ms: 1000, + }), + call_id: "call-read", + }, + }, + { + type: "response_item", + payload: { + type: "function_call_output", + call_id: "call-read", + output: "Chunk ID: read123\nWall time: 0.0000 seconds\nProcess exited with code 0\nOutput:\n 1\tconsole.log('hi');\n", + }, + }, + { + type: "response_item", + payload: { + type: "function_call", + name: "exec_command", + arguments: JSON.stringify({ + cmd: "ls", + workdir: "/test/project", + yield_time_ms: 1000, + }), + call_id: "call-ls", + }, + }, + { + type: "response_item", + payload: { + type: "function_call_output", + call_id: "call-ls", + output: "Chunk ID: abc123\nWall time: 0.0000 seconds\nProcess exited with code 0\nOutput:\nREADME.md\nsrc\n", + }, + }, + { + type: "response_item", + payload: { + type: "message", + role: "assistant", + content: [{ type: "output_text", text: "The directory contains README.md and src." }], + phase: "final_answer", + }, + }, + ]; + await writeFile( + rolloutPath, + `${rolloutRecords.map((record) => JSON.stringify(record)).join("\n")}\n`, + "utf8", + ); + + codexAcpClient.authRequired = vi.fn().mockResolvedValue(false); + codexAcpClient.getAccount = vi.fn().mockResolvedValue({ + account: null, + requiresOpenaiAuth: false, + }); + codexAcpClient.listSkills = vi.fn().mockResolvedValue({ data: [] }); + + const model = createTestModel({ id: "gpt-5.2", displayName: "GPT-5.2" }); + codexAppServerClient.listModels = vi.fn().mockResolvedValue({ + data: [model], + nextCursor: null, + }); + + const thread: Thread = { + id: "session-legacy", + sessionId: "session-legacy", + parentThreadId: null, + threadSource: null, + forkedFromId: null, + preview: "List the files", + ephemeral: false, + modelProvider: "openai", + createdAt: 123, + updatedAt: 124, + status: { type: "idle" }, + path: rolloutPath, + cwd: "/test/project", + cliVersion: "0.139.0", + source: "vscode", + agentNickname: null, + agentRole: null, + gitInfo: null, + name: null, + turns: [ + { + id: "turn-1", + itemsView: "full", + status: "completed", + error: null, + startedAt: null, + completedAt: null, + durationMs: null, + items: [ + { + type: "userMessage", + id: "item-user-1", + clientId: null, + content: [{ type: "text", text: "List the files", text_elements: [] }], + }, + { + type: "agentMessage", + id: "item-agent-1", + text: "The directory contains README.md and src.", + phase: null, + memoryCitation: null, + }, + ], + }, + ], + }; + + codexAppServerClient.threadResume = vi.fn().mockResolvedValue({ + thread, + model: model.id, + modelProvider: "openai", + cwd: "/test/project", + approvalPolicy: "never", + sandbox: { type: "dangerFullAccess" }, + reasoningEffort: model.defaultReasoningEffort, + }); + codexAppServerClient.threadRead = vi.fn().mockResolvedValue({ + thread, + }); + + await codexAcpAgent.initialize({ + protocolVersion: 1, + clientCapabilities: { + _meta: { + terminal_output: true, + }, + }, + }); + await codexAcpAgent.loadSession({ + sessionId: thread.id, + cwd: "/test/project", + mcpServers: [], + }); + + await expect(fixture.getAcpConnectionDump([])).toMatchFileSnapshot( + "data/load-session-response-item-history-fallback.json", + ); + } finally { + await rm(tempDir, { recursive: true, force: true }); + } + }); + it("publishes MCP startup failure for explicitly requested servers during loadSession", async () => { const fixture = createCodexMockTestFixture(); const codexAcpAgent = fixture.getCodexAcpAgent(); @@ -295,26 +539,30 @@ describe("CodexACPAgent - loadSession", () => { data: [model], nextCursor: null, }); + const thread: Thread = { + id: "session-1", + sessionId: "session-1", + parentThreadId: null, + threadSource: null, + forkedFromId: null, + preview: "", + ephemeral: false, + modelProvider: "openai", + createdAt: 0, + updatedAt: 0, + status: { type: "idle" }, + path: null, + cwd: "/test/project", + cliVersion: "0.0.0", + source: "cli", + agentNickname: null, + agentRole: null, + gitInfo: null, + name: null, + turns: [], + }; codexAppServerClient.threadResume = vi.fn().mockResolvedValue({ - thread: { - id: "session-1", - forkedFromId: null, - preview: "", - ephemeral: false, - modelProvider: "openai", - createdAt: 0, - updatedAt: 0, - status: { type: "idle" }, - path: null, - cwd: "/test/project", - cliVersion: "0.0.0", - source: "cli", - agentNickname: null, - agentRole: null, - gitInfo: null, - name: null, - turns: [], - }, + thread: thread, model: model.id, modelProvider: "openai", cwd: "/test/project", @@ -322,6 +570,9 @@ describe("CodexACPAgent - loadSession", () => { sandbox: { type: "dangerFullAccess" }, reasoningEffort: model.defaultReasoningEffort, }); + codexAppServerClient.threadRead = vi.fn().mockResolvedValue({ + thread: thread, + }); await codexAcpAgent.initialize({ protocolVersion: 1 }); From 9113be30097242c527ea12f2c92988c36834df1a Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Tue, 16 Jun 2026 16:25:50 +0200 Subject: [PATCH 2/5] Preserve failed tool call status in history fallback --- src/ResponseItemHistoryFallback.ts | 2 +- ...ession-response-item-history-fallback.json | 31 +++++++++++++++++++ .../CodexACPAgent/load-session.test.ts | 21 +++++++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/ResponseItemHistoryFallback.ts b/src/ResponseItemHistoryFallback.ts index b9fdd41..8c18554 100644 --- a/src/ResponseItemHistoryFallback.ts +++ b/src/ResponseItemHistoryFallback.ts @@ -337,7 +337,7 @@ function createFunctionCallOutputUpdate( return { sessionUpdate: "tool_call_update", toolCallId, - status: "completed", + status, rawOutput: { output: item["output"] }, }; } diff --git a/src/__tests__/CodexACPAgent/data/load-session-response-item-history-fallback.json b/src/__tests__/CodexACPAgent/data/load-session-response-item-history-fallback.json index 24815d8..aff44a8 100644 --- a/src/__tests__/CodexACPAgent/data/load-session-response-item-history-fallback.json +++ b/src/__tests__/CodexACPAgent/data/load-session-response-item-history-fallback.json @@ -118,6 +118,37 @@ } ] } +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "session-legacy", + "update": { + "sessionUpdate": "tool_call", + "toolCallId": "call-rg-failed", + "status": "in_progress", + "kind": "search", + "title": "Search for 'Missing' in src" + } + } + ] +} +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "session-legacy", + "update": { + "sessionUpdate": "tool_call_update", + "toolCallId": "call-rg-failed", + "status": "failed", + "rawOutput": { + "output": "Chunk ID: search456\nWall time: 0.0000 seconds\nProcess exited with code 1\nOutput:\n" + } + } + } + ] +} { "method": "sessionUpdate", "args": [ diff --git a/src/__tests__/CodexACPAgent/load-session.test.ts b/src/__tests__/CodexACPAgent/load-session.test.ts index f1d29e0..9621bfc 100644 --- a/src/__tests__/CodexACPAgent/load-session.test.ts +++ b/src/__tests__/CodexACPAgent/load-session.test.ts @@ -349,6 +349,27 @@ describe("CodexACPAgent - loadSession", () => { output: "Chunk ID: search123\nWall time: 0.0000 seconds\nProcess exited with code 0\nOutput:\nsrc/service.ts:export class Service {}\n", }, }, + { + type: "response_item", + payload: { + type: "function_call", + name: "exec_command", + arguments: JSON.stringify({ + cmd: "rg \"Missing\" src", + workdir: "/test/project", + yield_time_ms: 1000, + }), + call_id: "call-rg-failed", + }, + }, + { + type: "response_item", + payload: { + type: "function_call_output", + call_id: "call-rg-failed", + output: "Chunk ID: search456\nWall time: 0.0000 seconds\nProcess exited with code 1\nOutput:\n", + }, + }, { type: "response_item", payload: { From b2c7191e8edbd4e8eee12579af053992e6ee2a79 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Tue, 16 Jun 2026 16:39:43 +0200 Subject: [PATCH 3/5] Restore agent reasoning from event history --- src/ResponseItemHistoryFallback.ts | 14 ++++++++++++++ src/__tests__/CodexACPAgent/load-session.test.ts | 10 +++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/ResponseItemHistoryFallback.ts b/src/ResponseItemHistoryFallback.ts index 8c18554..037bf68 100644 --- a/src/ResponseItemHistoryFallback.ts +++ b/src/ResponseItemHistoryFallback.ts @@ -190,6 +190,8 @@ function createEventMsgUpdates(record: JsonRecord): UpdateSessionEvent[] | null switch (payload["type"]) { case "user_message": return createUserMessageEventUpdates(payload); + case "agent_reasoning": + return createAgentReasoningEventUpdates(payload); default: return []; } @@ -210,6 +212,18 @@ function createUserMessageEventUpdates(payload: JsonRecord): UpdateSessionEvent[ })); } +function createAgentReasoningEventUpdates(payload: JsonRecord): UpdateSessionEvent[] { + const text = stringValue(payload["text"]); + if (text === null || text.length === 0) { + return []; + } + + return [{ + sessionUpdate: "agent_thought_chunk", + content: { type: "text", text }, + }]; +} + function imageBlocks(images: unknown): ContentBlock[] { if (!Array.isArray(images)) { return []; diff --git a/src/__tests__/CodexACPAgent/load-session.test.ts b/src/__tests__/CodexACPAgent/load-session.test.ts index 9621bfc..0b5237f 100644 --- a/src/__tests__/CodexACPAgent/load-session.test.ts +++ b/src/__tests__/CodexACPAgent/load-session.test.ts @@ -320,11 +320,19 @@ describe("CodexACPAgent - loadSession", () => { content: [{ type: "input_text", text: "List the files" }], }, }, + { + type: "event_msg", + payload: { + type: "agent_reasoning", + text: "Need to inspect the directory.", + }, + }, { type: "response_item", payload: { type: "reasoning", - summary: [{ type: "summary_text", text: "Need to inspect the directory." }], + summary: [], + content: [], encrypted_content: null, }, }, From 2674228384ed6c616cb1cf1e2c45efccf349f2d0 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Tue, 16 Jun 2026 16:41:36 +0200 Subject: [PATCH 4/5] Test thread read during session load --- src/__tests__/CodexACPAgent/CodexAcpClient.test.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index 7474c3f..4e0aaa2 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -329,6 +329,9 @@ describe('ACP server test', { timeout: 40_000 }, () => { reasoningEffort: "medium", serviceTier: null, } as any); + const threadReadSpy = vi.spyOn(codexAppServerClient, "threadRead").mockResolvedValue({ + thread: {id: "thread-id"} as any, + }); vi.spyOn(codexAppServerClient, "listModels").mockResolvedValue({ data: [createTestModel({id: "gpt-5"})], nextCursor: null, @@ -356,6 +359,10 @@ describe('ACP server test', { timeout: 40_000 }, () => { "/workspace": {trust_level: "trusted"}, "/workspace/load-extra": {trust_level: "trusted"}, }); + expect(threadReadSpy).toHaveBeenCalledWith({ + threadId: "thread-id", + includeTurns: true, + }); }); it('rejects malformed ACP additional directories', async () => { From 266d48d38fcb28661583469796820862ebf35d4d Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Tue, 16 Jun 2026 17:03:17 +0200 Subject: [PATCH 5/5] Merge fallback history with parsed thread updates --- src/CodexAcpServer.ts | 83 +++++++++-- src/ResponseItemHistoryFallback.ts | 138 +++++++++++++++--- ...ession-response-item-history-fallback.json | 15 ++ .../CodexACPAgent/load-session.test.ts | 11 ++ .../response-item-history-fallback.test.ts | 112 ++++++++++++++ 5 files changed, 328 insertions(+), 31 deletions(-) create mode 100644 src/__tests__/CodexACPAgent/response-item-history-fallback.test.ts diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index 48c5e42..4635f20 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -857,21 +857,21 @@ export class CodexAcpServer implements acp.Agent { thread, sessionState.terminalOutputMode, ); - if (responseItemFallbackUpdates) { - for (const update of responseItemFallbackUpdates) { - await session.update(update); - } - return; - } + const threadUpdates: UpdateSessionEvent[] = []; for (const turn of thread.turns) { for (const item of turn.items) { const updates = await this.createHistoryUpdates(item, sessionState); - for (const update of updates) { - await session.update(update); - } + threadUpdates.push(...updates); } } + + const updates = responseItemFallbackUpdates + ? mergeHistoryUpdates(responseItemFallbackUpdates, threadUpdates) + : threadUpdates; + for (const update of updates) { + await session.update(update); + } } private async createHistoryUpdates(item: ThreadItem, sessionState: SessionState): Promise { @@ -1495,6 +1495,71 @@ export class CodexAcpServer implements acp.Agent { } } +function mergeHistoryUpdates( + responseItemFallbackUpdates: UpdateSessionEvent[], + threadUpdates: UpdateSessionEvent[], +): UpdateSessionEvent[] { + const merged: UpdateSessionEvent[] = []; + const seen = new Set(); + let fallbackIndex = 0; + + const pushUpdate = (update: UpdateSessionEvent) => { + const key = historyUpdateKey(update); + if (key && seen.has(key)) { + return; + } + if (key) { + seen.add(key); + } + merged.push(update); + }; + + const flushFallbackThrough = (targetKey: string): boolean => { + const matchIndex = responseItemFallbackUpdates.findIndex((update, index) => ( + index >= fallbackIndex && historyUpdateKey(update) === targetKey + )); + if (matchIndex === -1) { + return false; + } + + while (fallbackIndex <= matchIndex) { + pushUpdate(responseItemFallbackUpdates[fallbackIndex]!); + fallbackIndex += 1; + } + return true; + }; + + for (const update of threadUpdates) { + const key = historyUpdateKey(update); + if (key && flushFallbackThrough(key)) { + continue; + } + pushUpdate(update); + } + + while (fallbackIndex < responseItemFallbackUpdates.length) { + pushUpdate(responseItemFallbackUpdates[fallbackIndex]!); + fallbackIndex += 1; + } + + return merged; +} + +function historyUpdateKey(update: UpdateSessionEvent): string | null { + switch (update.sessionUpdate) { + case "user_message_chunk": + case "agent_message_chunk": + case "agent_thought_chunk": + return `${update.sessionUpdate}:${JSON.stringify(update.content)}`; + case "tool_call": + return `tool_call:${update.toolCallId}:start`; + case "tool_call_update": + return `tool_call:${update.toolCallId}:update`; + default: + return null; + } +} + function getRequestedMcpServerNames(mcpServers: Array): Array { return Array.from(new Set(mcpServers.map(server => sanitizeMcpServerName(server.name)))); } diff --git a/src/ResponseItemHistoryFallback.ts b/src/ResponseItemHistoryFallback.ts index 037bf68..474642b 100644 --- a/src/ResponseItemHistoryFallback.ts +++ b/src/ResponseItemHistoryFallback.ts @@ -16,16 +16,32 @@ type AcpToolCallUpdateStatus = NonNullable { - if (!thread.path || threadHasToolItems(thread)) { + if (!thread.path) { return null; } @@ -36,16 +52,32 @@ export async function createResponseItemHistoryFallbackUpdates( return null; } - return parseResponseItemHistoryFallback(contents, terminalOutputMode); + return parseResponseItemHistoryFallback(contents, terminalOutputMode, toolCallIdsFromThread(thread)); } export function parseResponseItemHistoryFallback( contents: string, terminalOutputMode: TerminalOutputMode, + existingToolCallIds: Set = new Set(), ): UpdateSessionEvent[] | null { const updates: UpdateSessionEvent[] = []; const terminalToolCallIds = new Set(); - let sawFunctionCall = false; + const execToolCallIds = new Set(); + const skippedToolCallIds = new Set(); + const emittedToolCallIds = new Set(); + let recoveredFunctionCall = false; + let lastUpdateKey: string | null = null; + + const pushUpdates = (nextUpdates: UpdateSessionEvent[]) => { + for (const update of nextUpdates) { + const key = historyFallbackUpdateKey(update); + if (key && key === lastUpdateKey) { + continue; + } + updates.push(update); + lastUpdateKey = key; + } + }; for (const line of contents.split(/\r?\n/)) { const record = parseJsonRecord(line); @@ -55,7 +87,7 @@ export function parseResponseItemHistoryFallback( const eventMsgUpdates = createEventMsgUpdates(record); if (eventMsgUpdates) { - updates.push(...eventMsgUpdates); + pushUpdates(eventMsgUpdates); continue; } @@ -66,27 +98,48 @@ export function parseResponseItemHistoryFallback( switch (item["type"]) { case "message": - updates.push(...createMessageUpdates(item)); + pushUpdates(createMessageUpdates(item)); break; case "reasoning": - updates.push(...createReasoningUpdates(item)); + pushUpdates(createReasoningUpdates(item)); break; case "function_call": { + const toolCallId = stringValue(item["call_id"]); + if (toolCallId && existingToolCallIds.has(toolCallId)) { + skippedToolCallIds.add(toolCallId); + break; + } + if (toolCallId && emittedToolCallIds.has(toolCallId)) { + break; + } const result = createFunctionCallUpdate(item); if (!result) { break; } - sawFunctionCall = true; + recoveredFunctionCall = true; + emittedToolCallIds.add(result.update.toolCallId); if (result.usesTerminal) { terminalToolCallIds.add(result.update.toolCallId); } - updates.push(result.update); + if (result.isExecCommand) { + execToolCallIds.add(result.update.toolCallId); + } + pushUpdates([result.update]); break; } case "function_call_output": { - const update = createFunctionCallOutputUpdate(item, terminalOutputMode, terminalToolCallIds); + const toolCallId = stringValue(item["call_id"]); + if (toolCallId && skippedToolCallIds.has(toolCallId)) { + break; + } + const update = createFunctionCallOutputUpdate( + item, + terminalOutputMode, + terminalToolCallIds, + execToolCallIds, + ); if (update) { - updates.push(update); + pushUpdates([update]); } break; } @@ -95,14 +148,23 @@ export function parseResponseItemHistoryFallback( } } - return sawFunctionCall ? updates : null; + return recoveredFunctionCall ? updates : null; } -function threadHasToolItems(thread: Thread): boolean { - return thread.turns.some((turn) => turn.items.some(isToolThreadItem)); +function toolCallIdsFromThread(thread: Thread): Set { + const ids = new Set(); + for (const turn of thread.turns) { + for (const item of turn.items) { + const id = toolCallIdFromThreadItem(item); + if (id) { + ids.add(id); + } + } + } + return ids; } -function isToolThreadItem(item: ThreadItem): boolean { +function toolCallIdFromThreadItem(item: ThreadItem): string | null { switch (item.type) { case "commandExecution": case "fileChange": @@ -112,7 +174,7 @@ function isToolThreadItem(item: ThreadItem): boolean { case "webSearch": case "imageView": case "imageGeneration": - return true; + return item.id; case "userMessage": case "hookPrompt": case "agentMessage": @@ -122,7 +184,7 @@ function isToolThreadItem(item: ThreadItem): boolean { case "enteredReviewMode": case "exitedReviewMode": case "contextCompaction": - return false; + return null; } } @@ -312,6 +374,7 @@ function createFunctionCallUpdate(item: JsonRecord): LegacyFunctionCallUpdate | return { update: createCommandActionEvent(toolCallId, "inProgress", cwd, commandAction), usesTerminal: false, + isExecCommand: true, }; } @@ -325,12 +388,13 @@ function createFunctionCallUpdate(item: JsonRecord): LegacyFunctionCallUpdate | }; if (!functionCallUsesTerminal(item)) { - return { update, usesTerminal: false }; + return { update, usesTerminal: false, isExecCommand: false }; } return { update: withTerminalContent(update, toolCallId, cwd), usesTerminal: true, + isExecCommand: true, }; } @@ -338,6 +402,7 @@ function createFunctionCallOutputUpdate( item: JsonRecord, terminalOutputMode: TerminalOutputMode, terminalToolCallIds: Set, + execToolCallIds: Set, ): UpdateSessionEvent | null { const toolCallId = stringValue(item["call_id"]); if (!toolCallId) { @@ -345,8 +410,8 @@ function createFunctionCallOutputUpdate( } const output = outputText(item["output"]); - const exitCode = parseExitCode(output); - const status = statusFromExitCode(exitCode); + const exitCode = parseExitCode(item["output"], output); + const status = statusFromExitCode(exitCode, output, execToolCallIds.has(toolCallId)); if (!terminalToolCallIds.has(toolCallId)) { return { sessionUpdate: "tool_call_update", @@ -1004,7 +1069,15 @@ function outputText(output: unknown): string { }).join("\n"); } -function parseExitCode(output: string): number | null { +function parseExitCode(rawOutput: unknown, output: string): number | null { + const record = asRecord(rawOutput); + if (record) { + const exitCode = numberValue(record["exit_code"]) ?? numberValue(record["exitCode"]); + if (exitCode !== null) { + return exitCode; + } + } + const match = output.match(/Process exited with code (-?\d+)/); if (!match) { return null; @@ -1019,8 +1092,25 @@ function parseExitCode(output: string): number | null { return Number.isFinite(exitCode) ? exitCode : null; } -function statusFromExitCode(exitCode: number | null): AcpToolCallUpdateStatus { - return exitCode === null || exitCode === 0 ? "completed" : "failed"; +function statusFromExitCode( + exitCode: number | null, + output: string, + isExecCommand: boolean, +): AcpToolCallUpdateStatus { + if (exitCode !== null) { + return exitCode === 0 ? "completed" : "failed"; + } + + return isExecCommand && looksLikeCommandFailure(output) ? "failed" : "completed"; +} + +function looksLikeCommandFailure(output: string): boolean { + const trimmed = output.trim(); + if (trimmed.length === 0) { + return false; + } + + return /(^|\n)(Error|Failed|Command failed|Sandbox error|No such file or directory|Permission denied|Operation not permitted|ENOENT|EACCES)(:|\b)/i.test(trimmed); } function asRecord(value: unknown): JsonRecord | null { @@ -1033,3 +1123,7 @@ function asRecord(value: unknown): JsonRecord | null { function stringValue(value: unknown): string | null { return typeof value === "string" ? value : null; } + +function numberValue(value: unknown): number | null { + return typeof value === "number" && Number.isFinite(value) ? value : null; +} diff --git a/src/__tests__/CodexACPAgent/data/load-session-response-item-history-fallback.json b/src/__tests__/CodexACPAgent/data/load-session-response-item-history-fallback.json index aff44a8..94a65ad 100644 --- a/src/__tests__/CodexACPAgent/data/load-session-response-item-history-fallback.json +++ b/src/__tests__/CodexACPAgent/data/load-session-response-item-history-fallback.json @@ -87,6 +87,21 @@ } ] } +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "session-legacy", + "update": { + "sessionUpdate": "agent_message_chunk", + "content": { + "type": "text", + "text": "Plan:\nInspect project files" + } + } + } + ] +} { "method": "sessionUpdate", "args": [ diff --git a/src/__tests__/CodexACPAgent/load-session.test.ts b/src/__tests__/CodexACPAgent/load-session.test.ts index 0b5237f..8248eb0 100644 --- a/src/__tests__/CodexACPAgent/load-session.test.ts +++ b/src/__tests__/CodexACPAgent/load-session.test.ts @@ -485,6 +485,17 @@ describe("CodexACPAgent - loadSession", () => { clientId: null, content: [{ type: "text", text: "List the files", text_elements: [] }], }, + { + type: "reasoning", + id: "item-reasoning-1", + summary: ["Need to inspect the directory."], + content: [], + }, + { + type: "plan", + id: "item-plan-1", + text: "Inspect project files", + }, { type: "agentMessage", id: "item-agent-1", diff --git a/src/__tests__/CodexACPAgent/response-item-history-fallback.test.ts b/src/__tests__/CodexACPAgent/response-item-history-fallback.test.ts new file mode 100644 index 0000000..f481088 --- /dev/null +++ b/src/__tests__/CodexACPAgent/response-item-history-fallback.test.ts @@ -0,0 +1,112 @@ +import { describe, expect, it } from "vitest"; +import type { UpdateSessionEvent } from "../../ACPSessionConnection"; +import { parseResponseItemHistoryFallback } from "../../ResponseItemHistoryFallback"; + +type ToolCallUpdate = Extract; + +describe("ResponseItemHistoryFallback", () => { + it("recovers only missing function calls for mixed parsed histories", () => { + const updates = parseResponseItemHistoryFallback(jsonl([ + functionCall("call-existing", "rg \"Existing\" src"), + functionCallOutput("call-existing", "Chunk ID: existing\nProcess exited with code 0\nOutput:\nsrc/existing.ts\n"), + functionCall("call-missing", "rg \"Missing\" src"), + functionCallOutput("call-missing", "Chunk ID: missing\nProcess exited with code 0\nOutput:\nsrc/missing.ts\n"), + ]), "terminal_output", new Set(["call-existing"])); + + expect(toolCallIds(updates)).toEqual(["call-missing"]); + expect(toolCallUpdateStatuses(updates)).toEqual([ + { toolCallId: "call-missing", status: "completed" }, + ]); + }); + + it("does not duplicate adjacent reasoning from event and response item records", () => { + const updates = parseResponseItemHistoryFallback(jsonl([ + { + type: "event_msg", + payload: { + type: "agent_reasoning", + text: "Need to inspect the directory.", + }, + }, + { + type: "response_item", + payload: { + type: "reasoning", + summary: [{ type: "summary_text", text: "Need to inspect the directory." }], + content: [], + }, + }, + functionCall("call-search", "rg \"Needle\" src"), + functionCallOutput("call-search", "Chunk ID: search\nProcess exited with code 0\nOutput:\nsrc/index.ts\n"), + ]), "terminal_output"); + + expect(thoughtTexts(updates)).toEqual(["Need to inspect the directory."]); + }); + + it("marks exec command outputs without exit footers failed when they report command errors", () => { + const updates = parseResponseItemHistoryFallback(jsonl([ + functionCall("call-read-failed", "cat missing.txt"), + functionCallOutput("call-read-failed", "Error: No such file or directory\n"), + ]), "terminal_output"); + + expect(toolCallUpdateStatuses(updates)).toEqual([ + { toolCallId: "call-read-failed", status: "failed" }, + ]); + }); +}); + +function jsonl(records: unknown[]): string { + return `${records.map((record) => JSON.stringify(record)).join("\n")}\n`; +} + +function functionCall(callId: string, cmd: string): unknown { + return { + type: "response_item", + payload: { + type: "function_call", + name: "exec_command", + arguments: JSON.stringify({ + cmd, + workdir: "/workspace", + yield_time_ms: 1000, + }), + call_id: callId, + }, + }; +} + +function functionCallOutput(callId: string, output: string): unknown { + return { + type: "response_item", + payload: { + type: "function_call_output", + call_id: callId, + output, + }, + }; +} + +function toolCallIds(updates: UpdateSessionEvent[] | null): string[] { + return (updates ?? []) + .filter((update): update is Extract => ( + update.sessionUpdate === "tool_call" + )) + .map((update) => update.toolCallId); +} + +function toolCallUpdateStatuses(updates: UpdateSessionEvent[] | null): Array> { + return (updates ?? []) + .filter((update): update is ToolCallUpdate => update.sessionUpdate === "tool_call_update") + .map((update) => ({ + toolCallId: update.toolCallId, + status: update.status ?? null, + })); +} + +function thoughtTexts(updates: UpdateSessionEvent[] | null): string[] { + return (updates ?? []) + .filter((update): update is Extract => ( + update.sessionUpdate === "agent_thought_chunk" + )) + .flatMap((update) => update.content.type === "text" ? [update.content.text] : []); +}