diff --git a/src/CodexEventHandler.ts b/src/CodexEventHandler.ts index 7b567541..ae5e008b 100644 --- a/src/CodexEventHandler.ts +++ b/src/CodexEventHandler.ts @@ -63,6 +63,7 @@ import { fuzzyFileSearchToolCallId, } from "./CodexToolCallMapper"; import { stripShellPrefix } from "./CommandUtils"; +import {commandToolName, functionToolName} from "./ToolCallName"; import {createTerminalOutputMeta, type TerminalOutputMode} from "./TerminalOutputMode"; import { createCodexMessagePhaseMeta, @@ -765,10 +766,16 @@ export class CodexEventHandler { private async completeItemEvent(event: ItemCompletedNotification): Promise { switch (event.item.type) { case "fileChange": + return { + sessionUpdate: "tool_call_update", + toolCallId: event.item.id, + status: event.item.status === "completed" ? "completed" : "failed", + } case "dynamicToolCall": return { sessionUpdate: "tool_call_update", toolCallId: event.item.id, + name: functionToolName(event.item.tool, event.item.namespace), status: event.item.status === "completed" ? "completed" : "failed", } case "mcpToolCall": @@ -1006,9 +1013,11 @@ export class CodexEventHandler { } private completeCommandExecutionEvent(item: ThreadItem & { "type": "commandExecution" }): UpdateSessionEvent { + const name = commandToolName(item.source); const update: UpdateSessionEvent = { sessionUpdate: "tool_call_update", toolCallId: item.id, + ...(name === undefined ? {} : {name}), status: item.status === "completed" ? "completed" : "failed", rawOutput: { formatted_output: item.aggregatedOutput ?? "", diff --git a/src/CodexToolCallMapper.ts b/src/CodexToolCallMapper.ts index d1b4cbb9..34f43bcf 100644 --- a/src/CodexToolCallMapper.ts +++ b/src/CodexToolCallMapper.ts @@ -33,6 +33,7 @@ import { type TerminalOutputMode, } from "./TerminalOutputMode"; import {createContextCompactionMeta} from "./ContextCompactionMeta"; +import {commandToolName, functionToolName} from "./ToolCallName"; type CodexItemStatus = CommandExecutionStatus | PatchApplyStatus | McpToolCallStatus | DynamicToolCallStatus | CollabAgentToolCallStatus; type AcpToolCallStatus = "pending" | "in_progress" | "completed" | "failed"; @@ -81,14 +82,19 @@ export async function createFileChangeUpdate( } export async function createCommandExecutionUpdate(item: CommandExecutionItem): Promise { + const name = commandToolName(item.source); const commandAction = item.commandActions.length === 1 ? item.commandActions[0] : undefined; if (commandAction) { - return createCommandActionEvent(item.id, item.status, item.cwd, commandAction); + return { + ...createCommandActionEvent(item.id, item.status, item.cwd, commandAction), + ...(name === undefined ? {} : {name}), + }; } const command = stripShellPrefix(item.command); return createTerminalCommandEvent({ sessionUpdate: "tool_call", toolCallId: item.id, + ...(name === undefined ? {} : {name}), kind: "execute", title: command, status: toAcpStatus(item.status), @@ -157,7 +163,10 @@ export async function createMcpToolCallUpdate( export async function createDynamicToolCallUpdate( item: ThreadItem & { type: "dynamicToolCall" } ): Promise { - return createExecuteToolCallUpdate(item, item.tool, { arguments: item.arguments }) + return { + ...await createExecuteToolCallUpdate(item, item.tool, { arguments: item.arguments }), + name: functionToolName(item.tool, item.namespace), + }; } export function createImageViewUpdate( @@ -168,6 +177,7 @@ export function createImageViewUpdate( sessionUpdate: "tool_call", toolCallId: item.id, kind: "read", + name: "view_image", title: `View Image ${displayPath}`, status: "completed", content: [createContent({ @@ -269,7 +279,7 @@ export async function createExecuteToolCallUpdate( title: string, rawInput?: Record, rawOutput?: Record, -): Promise { +): Promise { return { sessionUpdate: "tool_call", toolCallId: item.id, diff --git a/src/ResponseItemHistoryFallback.ts b/src/ResponseItemHistoryFallback.ts index 7bcba3e0..09f1a0fd 100644 --- a/src/ResponseItemHistoryFallback.ts +++ b/src/ResponseItemHistoryFallback.ts @@ -7,6 +7,7 @@ import type { CommandAction, Thread, ThreadItem } from "./app-server/v2"; import { createCommandActionEvent } from "./CodexToolCallMapper"; import { createTerminalOutputMeta, type TerminalOutputMode } from "./TerminalOutputMode"; import { createAgentMessageChunk, createCodexMessagePhaseMeta } from "./ContentChunks"; +import { functionToolName } from "./ToolCallName"; type JsonRecord = Record; type AcpToolCallEvent = Extract; @@ -368,6 +369,7 @@ function createFunctionCallUpdate(item: JsonRecord): LegacyFunctionCallUpdate | if (!toolCallId || !name) { return null; } + const toolName = functionToolName(name, stringValue(item["namespace"])); const isExecCommand = name === "exec_command"; const args = parseFunctionArguments(item["arguments"]); @@ -376,7 +378,10 @@ function createFunctionCallUpdate(item: JsonRecord): LegacyFunctionCallUpdate | const commandAction = command ? inferCommandAction(command, cwd) : null; if (commandAction) { return { - update: createCommandActionEvent(toolCallId, "inProgress", cwd, commandAction), + update: { + ...createCommandActionEvent(toolCallId, "inProgress", cwd, commandAction), + name: toolName, + }, usesTerminal: false, isExecCommand, }; @@ -385,6 +390,7 @@ function createFunctionCallUpdate(item: JsonRecord): LegacyFunctionCallUpdate | const update: AcpToolCallEvent = { sessionUpdate: "tool_call", toolCallId, + name: toolName, kind: toolKindForFunctionCall(name), title: titleForFunctionCall(name, args), status: "in_progress", diff --git a/src/ToolCallName.ts b/src/ToolCallName.ts new file mode 100644 index 00000000..a83d39f8 --- /dev/null +++ b/src/ToolCallName.ts @@ -0,0 +1,19 @@ +import type {CommandExecutionSource} from "./app-server/v2"; + +/** Matches Codex's flattened ToolName at boundaries that require a single string. */ +export function functionToolName(name: string, namespace?: string | null): string { + return `${namespace ?? ""}${name}`; +} + +export function commandToolName(source: CommandExecutionSource): string | undefined { + switch (source) { + case "unifiedExecStartup": + return "exec_command"; + case "unifiedExecInteraction": + return "write_stdin"; + // These sources do not identify a unique model tool. + case "agent": + case "userShell": + return undefined; + } +} diff --git a/src/__tests__/CodexACPAgent/approval-events.test.ts b/src/__tests__/CodexACPAgent/approval-events.test.ts index 11704f6a..138d5d92 100644 --- a/src/__tests__/CodexACPAgent/approval-events.test.ts +++ b/src/__tests__/CodexACPAgent/approval-events.test.ts @@ -90,6 +90,46 @@ describe("Approval Events", () => { } describe("command approvals", () => { + it.each([ + ["unifiedExecStartup", "exec_command"], + ["unifiedExecInteraction", "write_stdin"], + ["agent", undefined], + ["userShell", undefined], + ] as const)("uses only a known tool name for %s command approvals", async (source, name) => { + const prompt = setupSessionWithPendingPrompt(); + fixture.sendServerNotification({ + method: "item/started", + params: { + threadId: sessionId, + turnId: "turn-1", + startedAtMs: 0, + item: { + type: "commandExecution", + id: "command-item", + pluginId: null, + scriptPath: null, + command: "npm test", + cwd: "/workspace", + processId: null, + source, + status: "inProgress", + commandActions: [], + aggregatedOutput: null, + exitCode: null, + durationMs: null, + }, + }, + }); + await fixture.getCodexAcpClient().waitForSessionNotifications(sessionId); + fixture.clearAcpConnectionDump(); + fixture.setPermissionResponse({outcome: {outcome: "selected", optionId: ApprovalOptionId.AllowOnce}}); + + await fixture.sendServerRequest("item/commandExecution/requestApproval", commandParams(["accept", "cancel"])); + + expect(permissionRequest().toolCall.name).toBe(name); + await finish(prompt); + }); + it("emits an autonomous ACP v1 snapshot and maps explicit reject to decline", async () => { const prompt = setupSessionWithPendingPrompt(); fixture.setPermissionResponse({outcome: {outcome: "selected", optionId: ApprovalOptionId.Decline}}); @@ -653,6 +693,7 @@ describe("Approval Events", () => { expect(permissionRequest()).toMatchObject({ toolCall: { toolCallId: "permissions-item", + name: "request_permissions", kind: "other", status: "pending", title: "Additional sandbox permissions", diff --git a/src/__tests__/CodexACPAgent/data/dynamic-tool-completed.json b/src/__tests__/CodexACPAgent/data/dynamic-tool-completed.json index 7ef45264..b991960a 100644 --- a/src/__tests__/CodexACPAgent/data/dynamic-tool-completed.json +++ b/src/__tests__/CodexACPAgent/data/dynamic-tool-completed.json @@ -6,6 +6,7 @@ "update": { "sessionUpdate": "tool_call_update", "toolCallId": "dyn-tool-123", + "name": "list_apps", "status": "completed" } } diff --git a/src/__tests__/CodexACPAgent/data/dynamic-tool-in-progress.json b/src/__tests__/CodexACPAgent/data/dynamic-tool-in-progress.json index a8400b86..ca073327 100644 --- a/src/__tests__/CodexACPAgent/data/dynamic-tool-in-progress.json +++ b/src/__tests__/CodexACPAgent/data/dynamic-tool-in-progress.json @@ -13,7 +13,8 @@ "arguments": { "includeDisabled": false } - } + }, + "name": "list_apps" } } ] diff --git a/src/__tests__/CodexACPAgent/data/load-session-history.json b/src/__tests__/CodexACPAgent/data/load-session-history.json index 69644617..ada6047b 100644 --- a/src/__tests__/CodexACPAgent/data/load-session-history.json +++ b/src/__tests__/CodexACPAgent/data/load-session-history.json @@ -325,7 +325,8 @@ "arguments": { "includeDisabled": false } - } + }, + "name": "list_apps" } } ] @@ -339,6 +340,7 @@ "sessionUpdate": "tool_call", "toolCallId": "item-image-view-1", "kind": "read", + "name": "view_image", "title": "View Image /test/project/input.png", "status": "completed", "content": [ 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 c78620f2..c1d21f4a 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 @@ -192,7 +192,8 @@ "toolCallId": "call-rg", "status": "in_progress", "kind": "search", - "title": "Search for 'Service' in src" + "title": "Search for 'Service' in src", + "name": "exec_command" } } ] @@ -223,7 +224,8 @@ "toolCallId": "call-rg-failed", "status": "in_progress", "kind": "search", - "title": "Search for 'Missing' in src" + "title": "Search for 'Missing' in src", + "name": "exec_command" } } ] @@ -259,7 +261,8 @@ { "path": "/test/project/src/index.ts" } - ] + ], + "name": "exec_command" } } ] @@ -290,7 +293,8 @@ "toolCallId": "call-ls", "status": "in_progress", "kind": "read", - "title": "List files" + "title": "List files", + "name": "exec_command" } } ] diff --git a/src/__tests__/CodexACPAgent/data/response-item-history-tool-names.json b/src/__tests__/CodexACPAgent/data/response-item-history-tool-names.json new file mode 100644 index 00000000..12266984 --- /dev/null +++ b/src/__tests__/CodexACPAgent/data/response-item-history-tool-names.json @@ -0,0 +1,89 @@ +[ + { + "sessionUpdate": "tool_call", + "toolCallId": "call-search", + "status": "in_progress", + "kind": "search", + "title": "Search for 'Needle' in src", + "name": "exec_command" + }, + { + "sessionUpdate": "tool_call", + "toolCallId": "call-terminal", + "name": "exec_command", + "kind": "execute", + "title": "npm test", + "status": "in_progress", + "content": [ + { + "type": "terminal", + "terminalId": "call-terminal" + } + ], + "rawInput": { + "command": "npm test", + "cwd": "/workspace", + "arguments": { + "cmd": "npm test", + "workdir": "/workspace", + "yield_time_ms": 1000 + } + }, + "_meta": { + "terminal_info": { + "cwd": "/workspace", + "terminal_id": "call-terminal" + } + } + }, + { + "sessionUpdate": "tool_call", + "toolCallId": "call-patch", + "name": "apply_patch", + "kind": "edit", + "title": "Apply patch", + "status": "in_progress", + "rawInput": { + "name": "apply_patch", + "arguments": { + "patch": "*** Begin Patch\n*** End Patch" + } + } + }, + { + "sessionUpdate": "tool_call", + "toolCallId": "call-mcp", + "name": "mcp__docs__find_page", + "kind": "other", + "title": "mcp__docs__find_page", + "status": "in_progress", + "rawInput": { + "name": "mcp__docs__find_page", + "arguments": { + "query": "Tool names" + } + } + }, + { + "sessionUpdate": "tool_call", + "toolCallId": "call-namespaced", + "name": "functions.read_file", + "kind": "other", + "title": "read_file", + "status": "in_progress", + "rawInput": { + "name": "read_file", + "arguments": { + "path": "README.md" + } + } + }, + { + "sessionUpdate": "tool_call_update", + "toolCallId": "call-search", + "status": "completed", + "rawOutput": { + "output": "Chunk ID: search\nProcess exited with code 0\nOutput:\nsrc/index.ts\n" + } + } +] diff --git a/src/__tests__/CodexACPAgent/data/tool-call-command-names.json b/src/__tests__/CodexACPAgent/data/tool-call-command-names.json new file mode 100644 index 00000000..156ab43b --- /dev/null +++ b/src/__tests__/CodexACPAgent/data/tool-call-command-names.json @@ -0,0 +1,208 @@ +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "tool-names-session", + "update": { + "sessionUpdate": "tool_call", + "toolCallId": "unifiedExecStartup-execute", + "name": "exec_command", + "kind": "execute", + "title": "npm test", + "status": "in_progress", + "content": [ + { + "type": "terminal", + "terminalId": "unifiedExecStartup-execute" + } + ], + "rawInput": { + "command": "npm test", + "cwd": "/repo" + }, + "_meta": { + "terminal_info": { + "cwd": "/repo", + "terminal_id": "unifiedExecStartup-execute" + } + } + } + } + ] +} +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "tool-names-session", + "update": { + "sessionUpdate": "tool_call", + "toolCallId": "unifiedExecStartup-read", + "status": "in_progress", + "kind": "read", + "title": "Read file '/repo/config.json'", + "locations": [ + { + "path": "/repo/config.json" + } + ], + "name": "exec_command" + } + } + ] +} +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "tool-names-session", + "update": { + "sessionUpdate": "tool_call", + "toolCallId": "unifiedExecInteraction-execute", + "name": "write_stdin", + "kind": "execute", + "title": "npm test", + "status": "in_progress", + "content": [ + { + "type": "terminal", + "terminalId": "unifiedExecInteraction-execute" + } + ], + "rawInput": { + "command": "npm test", + "cwd": "/repo" + }, + "_meta": { + "terminal_info": { + "cwd": "/repo", + "terminal_id": "unifiedExecInteraction-execute" + } + } + } + } + ] +} +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "tool-names-session", + "update": { + "sessionUpdate": "tool_call", + "toolCallId": "unifiedExecInteraction-read", + "status": "in_progress", + "kind": "read", + "title": "Read file '/repo/config.json'", + "locations": [ + { + "path": "/repo/config.json" + } + ], + "name": "write_stdin" + } + } + ] +} +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "tool-names-session", + "update": { + "sessionUpdate": "tool_call", + "toolCallId": "agent-execute", + "kind": "execute", + "title": "npm test", + "status": "in_progress", + "content": [ + { + "type": "terminal", + "terminalId": "agent-execute" + } + ], + "rawInput": { + "command": "npm test", + "cwd": "/repo" + }, + "_meta": { + "terminal_info": { + "cwd": "/repo", + "terminal_id": "agent-execute" + } + } + } + } + ] +} +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "tool-names-session", + "update": { + "sessionUpdate": "tool_call", + "toolCallId": "agent-read", + "status": "in_progress", + "kind": "read", + "title": "Read file '/repo/config.json'", + "locations": [ + { + "path": "/repo/config.json" + } + ] + } + } + ] +} +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "tool-names-session", + "update": { + "sessionUpdate": "tool_call", + "toolCallId": "userShell-execute", + "kind": "execute", + "title": "npm test", + "status": "in_progress", + "content": [ + { + "type": "terminal", + "terminalId": "userShell-execute" + } + ], + "rawInput": { + "command": "npm test", + "cwd": "/repo" + }, + "_meta": { + "terminal_info": { + "cwd": "/repo", + "terminal_id": "userShell-execute" + } + } + } + } + ] +} +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "tool-names-session", + "update": { + "sessionUpdate": "tool_call", + "toolCallId": "userShell-read", + "status": "in_progress", + "kind": "read", + "title": "Read file '/repo/config.json'", + "locations": [ + { + "path": "/repo/config.json" + } + ] + } + } + ] +} \ No newline at end of file diff --git a/src/__tests__/CodexACPAgent/data/tool-call-completed-name.json b/src/__tests__/CodexACPAgent/data/tool-call-completed-name.json new file mode 100644 index 00000000..2441f4c4 --- /dev/null +++ b/src/__tests__/CodexACPAgent/data/tool-call-completed-name.json @@ -0,0 +1,18 @@ +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "tool-names-session", + "update": { + "sessionUpdate": "tool_call_update", + "toolCallId": "unifiedExecStartup-execute", + "name": "exec_command", + "status": "completed", + "rawOutput": { + "formatted_output": "", + "exit_code": null + } + } + } + ] +} \ No newline at end of file diff --git a/src/__tests__/CodexACPAgent/data/tool-call-dynamic-names.json b/src/__tests__/CodexACPAgent/data/tool-call-dynamic-names.json new file mode 100644 index 00000000..06e22ad7 --- /dev/null +++ b/src/__tests__/CodexACPAgent/data/tool-call-dynamic-names.json @@ -0,0 +1,105 @@ +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "tool-names-session", + "update": { + "sessionUpdate": "tool_call", + "toolCallId": "plain-tool", + "kind": "execute", + "title": "read_file", + "status": "in_progress", + "rawInput": { + "arguments": { + "path": "/repo/config.json" + } + }, + "name": "read_file" + } + } + ] +} +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "tool-names-session", + "update": { + "sessionUpdate": "tool_call_update", + "toolCallId": "plain-tool", + "name": "read_file", + "status": "completed" + } + } + ] +} +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "tool-names-session", + "update": { + "sessionUpdate": "tool_call", + "toolCallId": "functions.tool", + "kind": "execute", + "title": "read_file", + "status": "in_progress", + "rawInput": { + "arguments": { + "path": "/repo/config.json" + } + }, + "name": "functions.read_file" + } + } + ] +} +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "tool-names-session", + "update": { + "sessionUpdate": "tool_call_update", + "toolCallId": "functions.tool", + "name": "functions.read_file", + "status": "completed" + } + } + ] +} +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "tool-names-session", + "update": { + "sessionUpdate": "tool_call", + "toolCallId": "toolstool", + "kind": "execute", + "title": "read_file", + "status": "in_progress", + "rawInput": { + "arguments": { + "path": "/repo/config.json" + } + }, + "name": "toolsread_file" + } + } + ] +} +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "tool-names-session", + "update": { + "sessionUpdate": "tool_call_update", + "toolCallId": "toolstool", + "name": "toolsread_file", + "status": "completed" + } + } + ] +} \ No newline at end of file diff --git a/src/__tests__/CodexACPAgent/data/view-image-flow.json b/src/__tests__/CodexACPAgent/data/view-image-flow.json index 6930a7f8..8fd5c5d1 100644 --- a/src/__tests__/CodexACPAgent/data/view-image-flow.json +++ b/src/__tests__/CodexACPAgent/data/view-image-flow.json @@ -7,6 +7,7 @@ "sessionUpdate": "tool_call", "toolCallId": "view-image-1", "kind": "read", + "name": "view_image", "title": "View Image /tmp/codex/input.png", "status": "completed", "content": [ diff --git a/src/__tests__/CodexACPAgent/response-item-history-fallback.test.ts b/src/__tests__/CodexACPAgent/response-item-history-fallback.test.ts index aaeef097..780225c0 100644 --- a/src/__tests__/CodexACPAgent/response-item-history-fallback.test.ts +++ b/src/__tests__/CodexACPAgent/response-item-history-fallback.test.ts @@ -5,6 +5,46 @@ import { parseResponseItemHistoryFallback } from "../../ResponseItemHistoryFallb type ToolCallUpdate = Extract; describe("ResponseItemHistoryFallback", () => { + it("preserves programmatic function names alongside invocation titles", async () => { + const updates = parseResponseItemHistoryFallback(jsonl([ + functionCall("call-search", "rg \"Needle\" src"), + functionCall("call-terminal", "npm test"), + { + type: "response_item", + payload: { + type: "function_call", + call_id: "call-patch", + name: "apply_patch", + arguments: JSON.stringify({ patch: "*** Begin Patch\n*** End Patch" }), + }, + }, + { + type: "response_item", + payload: { + type: "function_call", + call_id: "call-mcp", + name: "mcp__docs__find_page", + arguments: JSON.stringify({ query: "Tool names" }), + }, + }, + { + type: "response_item", + payload: { + type: "function_call", + call_id: "call-namespaced", + namespace: "functions.", + name: "read_file", + arguments: JSON.stringify({ path: "README.md" }), + }, + }, + functionCallOutput("call-search", "Chunk ID: search\nProcess exited with code 0\nOutput:\nsrc/index.ts\n"), + ]), "terminal_output"); + + await expect(`${JSON.stringify(updates, null, 2)}\n`).toMatchFileSnapshot( + "data/response-item-history-tool-names.json", + ); + }); + it("recovers only missing function calls for mixed parsed histories", () => { const updates = parseResponseItemHistoryFallback(jsonl([ functionCall("call-existing", "rg \"Existing\" src"), diff --git a/src/__tests__/CodexACPAgent/tool-call-name.test.ts b/src/__tests__/CodexACPAgent/tool-call-name.test.ts new file mode 100644 index 00000000..4c876edd --- /dev/null +++ b/src/__tests__/CodexACPAgent/tool-call-name.test.ts @@ -0,0 +1,99 @@ +import {describe, expect, it} from "vitest"; +import type {ServerNotification} from "../../app-server"; +import type {CommandExecutionSource, ThreadItem} from "../../app-server/v2"; +import { + createCodexMockTestFixture, + createTestSessionState, + setupPromptAndSendNotifications, +} from "../acp-test-utils"; + +const sessionId = "tool-names-session"; + +function commandItem(source: CommandExecutionSource, read: boolean): Extract { + return { + type: "commandExecution", + id: `${source}-${read ? "read" : "execute"}`, + source, + pluginId: null, + scriptPath: null, + command: read ? "cat /repo/config.json" : "npm test", + cwd: "/repo", + processId: null, + status: "inProgress", + commandActions: read + ? [{type: "read", command: "cat /repo/config.json", name: "cat", path: "/repo/config.json"}] + : [], + aggregatedOutput: null, + exitCode: null, + durationMs: null, + }; +} + +function started(item: ThreadItem): ServerNotification { + return { + method: "item/started", + params: {threadId: sessionId, turnId: "turn-id", startedAtMs: 0, item}, + }; +} + +describe("tool call names", () => { + it("reports known execution tools independently of command titles and kinds", async () => { + const fixture = createCodexMockTestFixture(); + const sources: CommandExecutionSource[] = [ + "unifiedExecStartup", "unifiedExecInteraction", "agent", "userShell", + ]; + await setupPromptAndSendNotifications( + fixture, + sessionId, + createTestSessionState({sessionId}), + sources.flatMap(source => [started(commandItem(source, false)), started(commandItem(source, true))]), + ); + + await expect(fixture.getAcpConnectionDump([])).toMatchFileSnapshot("data/tool-call-command-names.json"); + }); + + it("reports dynamic tool identity on the first event and leaves it unchanged on completion", async () => { + const fixture = createCodexMockTestFixture(); + const items: Extract[] = [null, "functions.", "tools"].map(namespace => ({ + type: "dynamicToolCall", + id: namespace === null ? "plain-tool" : `${namespace}tool`, + namespace, + tool: "read_file", + arguments: {path: "/repo/config.json"}, + status: "inProgress", + contentItems: null, + success: null, + durationMs: null, + })); + const notifications = items.flatMap((item): ServerNotification[] => [ + started(item), + { + method: "item/completed", + params: { + threadId: sessionId, + turnId: "turn-id", + completedAtMs: 0, + item: {...item, status: "completed", success: true}, + }, + }, + ]); + await setupPromptAndSendNotifications(fixture, sessionId, createTestSessionState({sessionId}), notifications); + + await expect(fixture.getAcpConnectionDump([])).toMatchFileSnapshot("data/tool-call-dynamic-names.json"); + }); + + it("includes known names when completion is the first available report", async () => { + const fixture = createCodexMockTestFixture(); + const notifications: ServerNotification[] = [{ + method: "item/completed", + params: { + threadId: sessionId, + turnId: "turn-id", + completedAtMs: 0, + item: {...commandItem("unifiedExecStartup", false), status: "completed"}, + }, + }]; + await setupPromptAndSendNotifications(fixture, sessionId, createTestSessionState({sessionId}), notifications); + await expect(fixture.getAcpConnectionDump([])).toMatchFileSnapshot("data/tool-call-completed-name.json"); + }); +}); diff --git a/src/__tests__/PermissionLifecycleContext.test.ts b/src/__tests__/PermissionLifecycleContext.test.ts index ad90788d..3072fcd7 100644 --- a/src/__tests__/PermissionLifecycleContext.test.ts +++ b/src/__tests__/PermissionLifecycleContext.test.ts @@ -54,6 +54,32 @@ function fileChangeStarted(id: string, threadId: string): ServerNotification { }; } +function commandStarted(id: string, threadId: string): Extract { + return { + method: "item/started", + params: { + threadId, + turnId: `turn-${threadId}`, + startedAtMs: 0, + item: { + type: "commandExecution", + id, + pluginId: null, + scriptPath: null, + command: "npm test", + cwd: "/workspace", + processId: null, + source: "unifiedExecStartup", + status: "inProgress", + commandActions: [], + aggregatedOutput: null, + exitCode: null, + durationMs: null, + }, + }, + }; +} + function turnCompleted(threadId: string): ServerNotification { return { method: "turn/completed", @@ -123,6 +149,8 @@ describe("PermissionLifecycleContext", () => { prompt.handleNotification(mcpStarted("call-b", "turn-b", "child-b")); prompt.handleNotification(fileChangeStarted("shared-file-change", "child-a")); prompt.handleNotification(fileChangeStarted("shared-file-change", "child-b")); + prompt.handleNotification(commandStarted("shared-command", "child-a")); + prompt.handleNotification(commandStarted("shared-command", "child-b")); prompt.handleNotification(turnCompleted("child-b")); @@ -130,6 +158,27 @@ describe("PermissionLifecycleContext", () => { expect(prompt.popPendingMcpApproval("child-b", "server")).toBeUndefined(); expect(prompt.fileChange("child-a", "shared-file-change")?.changes[0]?.path).toBe("/child-a.txt"); expect(prompt.fileChange("child-b", "shared-file-change")).toBeUndefined(); + expect(prompt.commandName("child-a", "shared-command")).toBe("exec_command"); + expect(prompt.commandName("child-b", "shared-command")).toBeUndefined(); + }); + + it("clears a completed command's name", () => { + const prompt = new PermissionLifecycleContext(sessionState()).beginPrompt(); + const notification = commandStarted("command", "thread"); + prompt.handleNotification(notification); + expect(prompt.commandName("thread", "command")).toBe("exec_command"); + + prompt.handleNotification({ + method: "item/completed", + params: { + threadId: notification.params.threadId, + turnId: notification.params.turnId, + completedAtMs: 1, + item: notification.params.item, + }, + }); + + expect(prompt.commandName("thread", "command")).toBeUndefined(); }); it("does not allocate a synthetic ID for native ACP elicitation", async () => { diff --git a/src/permissions/CodexApprovalHandler.ts b/src/permissions/CodexApprovalHandler.ts index 38a639ee..a62bcb5f 100644 --- a/src/permissions/CodexApprovalHandler.ts +++ b/src/permissions/CodexApprovalHandler.ts @@ -50,7 +50,7 @@ export class CodexApprovalHandler implements ApprovalHandler { try { const response = await this.requestPermission({ sessionId: params.threadId, - toolCall: commandToolCall(authoritativeParams), + toolCall: commandToolCall(authoritativeParams, this.permissionContext), options: decisions.map(({option}) => option), _meta: requestPermissionMeta( params.networkApprovalContext ? CODEX_NETWORK_PERMISSION_TITLE : CODEX_COMMAND_PERMISSION_TITLE, diff --git a/src/permissions/lifecycle.ts b/src/permissions/lifecycle.ts index e2dd7bbb..c48c2bd5 100644 --- a/src/permissions/lifecycle.ts +++ b/src/permissions/lifecycle.ts @@ -1,6 +1,7 @@ import type {SessionState} from "../CodexAcpServer"; import type {ServerNotification} from "../app-server"; import type {ThreadItem} from "../app-server/v2"; +import {commandToolName} from "../ToolCallName"; type FileChangeItem = ThreadItem & {type: "fileChange"}; @@ -22,6 +23,7 @@ export class PermissionLifecycleContext { /** Prompt-scoped permission presentation and MCP correlation state. */ export class PermissionPromptContext { + private readonly commandNames = new Map>(); private readonly fileChanges = new Map>(); private readonly pendingMcpApprovals = new Map>(); @@ -50,6 +52,10 @@ export class PermissionPromptContext { return this.fileChanges.get(threadId)?.get(itemId); } + commandName(threadId: string, itemId: string): string | undefined { + return this.commandNames.get(threadId)?.get(itemId); + } + popPendingMcpApproval(threadId: string, serverName: string): string | undefined { const byServer = this.pendingMcpApprovals.get(threadId); if (!byServer) return undefined; @@ -66,6 +72,15 @@ export class PermissionPromptContext { } private handleItemStarted(threadId: string, item: ThreadItem): void { + if (item.type === "commandExecution") { + const name = commandToolName(item.source); + if (name !== undefined) { + const byItem = this.commandNames.get(threadId) ?? new Map(); + byItem.set(item.id, name); + this.commandNames.set(threadId, byItem); + } + return; + } if (item.type === "fileChange") { const byItem = this.fileChanges.get(threadId) ?? new Map(); byItem.set(item.id, item); @@ -81,6 +96,12 @@ export class PermissionPromptContext { } private handleItemCompleted(threadId: string, item: ThreadItem): void { + if (item.type === "commandExecution") { + const byItem = this.commandNames.get(threadId); + byItem?.delete(item.id); + if (byItem?.size === 0) this.commandNames.delete(threadId); + return; + } if (item.type === "fileChange") { const byItem = this.fileChanges.get(threadId); byItem?.delete(item.id); @@ -99,6 +120,7 @@ export class PermissionPromptContext { } private clearTransientState(threadId: string): void { + this.commandNames.delete(threadId); this.fileChanges.delete(threadId); this.pendingMcpApprovals.delete(threadId); } diff --git a/src/permissions/presentation.ts b/src/permissions/presentation.ts index d2b9cede..413d3b20 100644 --- a/src/permissions/presentation.ts +++ b/src/permissions/presentation.ts @@ -15,7 +15,11 @@ type CommandPresentationParams = CommandExecutionRequestApprovalParams & { additionalPermissions?: AdditionalPermissionProfile | null; }; -export function commandToolCall(params: CommandPresentationParams): acp.ToolCallUpdate { +export function commandToolCall( + params: CommandPresentationParams, + permissionContext: PermissionPromptContext, +): acp.ToolCallUpdate { + const name = permissionContext.commandName(params.threadId, params.itemId); const network = params.networkApprovalContext; const rawInput = { ...(params.command ? {command: stripShellPrefix(params.command)} : {}), @@ -30,6 +34,7 @@ export function commandToolCall(params: CommandPresentationParams): acp.ToolCall : []; return { toolCallId: params.itemId, + ...(name !== undefined ? {name} : {}), kind: "execute", status: "pending", title: network @@ -71,6 +76,7 @@ export function additionalPermissionsToolCall( const content = permissionProfileContent(permissions); return { toolCallId: itemId, + name: "request_permissions", kind: "other", status: "pending", title: "Additional sandbox permissions",