From bbae5c2c0936554104b07630b142fd71fe1d839e Mon Sep 17 00:00:00 2001 From: hwisu Date: Thu, 17 Sep 2026 22:34:05 +0900 Subject: [PATCH] fix: show shell commands in permission prompts (#1070) ## Summary - use the canonical tool-call title for permission prompts, so shell approvals show the exact command instead of the model-authored description - map `PowerShell` through the same shell presentation path as `Bash` - complete the PowerShell terminal lifecycle with matching `terminal_info`, `terminal_output`, and `terminal_exit` metadata, including failed commands - add focused regression coverage without changing the existing Bash lifecycle tests Fixes #1068 ## PowerShell input and result contract `PowerShell` is a distinct tool name, but its documented input fields match `Bash`: the executable command is in `command`, while `description` is an optional human-readable summary. The Claude Code hooks reference documents the shared `command`, `description`, `timeout`, and `run_in_background` fields, and the tools reference likewise states that PowerShell hooks receive the command in `tool_input.command`. Anthropic closed [claude-code#83647](https://github.com/anthropics/claude-code/issues/83647#issuecomment-5310033500) after documenting this equivalence. Reusing the SDK's exported `BashInput` shape for the common input fields is therefore intentional; the SDK currently does not export a separate `PowerShellInput` type. PowerShell results do not need a PowerShell-specific output assumption here. The result mapper first shape-checks structured stdout/stderr and otherwise falls back to the standard raw `tool_result` string or content-block array. Routing PowerShell through that existing mapper preserves this fallback while completing the adapter's capability-gated terminal sequence: 1. initial `tool_call` with `terminal_info` 2. `tool_call_update` with `terminal_output` 3. final `tool_call_update` with `terminal_exit` All three events use the original tool-use ID, so the client can reconcile and close the terminal. Failed PowerShell results also bypass the generic error renderer and finish the terminal with a non-zero display exit code. The permission presentation introduced in #1004 already handled `Bash` and `PowerShell` together when choosing a description-based permission title, but the canonical tool presentation and terminal result paths only handled `Bash`. This PR completes both paths. References: - [Claude Code hooks: PowerShell input](https://code.claude.com/docs/en/hooks#powershell) - [Claude Code tools reference: PowerShell tool](https://code.claude.com/docs/en/tools-reference#powershell-tool) - [Anthropic schema confirmation](https://github.com/anthropics/claude-code/issues/83647#issuecomment-5310033500) ## Verification - `npm run check` - `npm run build` - `npm run test:run -- src/tests/tools.test.ts -t "PowerShell terminal output"` (1 passed) - full suite with an empty temporary Claude config: `CLAUDE_CONFIG_DIR= npm run test:run` (1070 passed, 27 skipped) --------- Co-authored-by: Claude Opus 5 Co-authored-by: Ben Brandt --- docs/permission-extension.md | 2 + src/acp-agent.ts | 16 ++--- src/permissions/presentation.ts | 13 ++-- src/tests/acp-agent.test.ts | 29 +++++++- src/tests/permission-presentation.test.ts | 75 +++++++++++++++------ src/tests/tools.test.ts | 82 +++++++++++++++++++++++ src/tools.ts | 8 ++- 7 files changed, 186 insertions(+), 39 deletions(-) diff --git a/docs/permission-extension.md b/docs/permission-extension.md index 92415186..ce5e02cb 100644 --- a/docs/permission-extension.md +++ b/docs/permission-extension.md @@ -84,6 +84,8 @@ The permission request carries the same standard ACP tool information used for n Compact text removes control characters, collapses whitespace where appropriate, and enforces length limits. Invalid optional presentation text is omitted instead of being truncated into misleading UI. +Shell permission titles (`Bash` and `PowerShell`) preserve the full command verbatim, including +whitespace and line breaks, without applying compact-text normalization or length limits. Permission options are fixed. When a durable suggestion contains a command prefix, path, host, or other rule, the adapter includes that value directly in `PermissionOption.name`, for example diff --git a/src/acp-agent.ts b/src/acp-agent.ts index 2b7ce56f..6eda77cb 100644 --- a/src/acp-agent.ts +++ b/src/acp-agent.ts @@ -9248,10 +9248,10 @@ function shouldEmitToolCall(toolName: string): boolean { return toolName !== "TodoWrite" && !isTaskTool(toolName); } -/** Build the Claude Code-specific metadata for a tool call. Bash descriptions - * are kept out of ACP's standard `title`, which clients may use as the shell - * command preview, while still giving clients access to Claude's concise - * human-readable title. */ +/** Build the Claude Code-specific metadata for a tool call. Shell (Bash and + * PowerShell) descriptions are kept out of ACP's standard `title`, which + * clients may use as the shell command preview, while still giving clients + * access to Claude's concise human-readable title. */ function claudeCodeMetaFromToolUse( toolUse: { name: string; @@ -9260,7 +9260,7 @@ function claudeCodeMetaFromToolUse( cwd?: string, ): NonNullable { const description = - toolUse.name === "Bash" && + (toolUse.name === "Bash" || toolUse.name === "PowerShell") && toolUse.input !== null && typeof toolUse.input === "object" && "description" in toolUse.input && @@ -9370,8 +9370,8 @@ function resolveSkillPath(skillName: string, cwd?: string): string | undefined { * notification for a tool_use. Shared by every site that surfaces a tool call: * the streamed tool_use path (first encounter → tool_call, later encounter → * refine) and the permission flow (`ensureToolCallEmitted`), so they can't - * drift. The initial `tool_call` carries `status: "pending"` and, for Bash, the - * `terminal_info` _meta that the later `terminal_output`/`terminal_exit` + * drift. The initial `tool_call` carries `status: "pending"` and, for shell tools, + * the `terminal_info` _meta that the later `terminal_output`/`terminal_exit` * updates key off of, and the programmatic tool `name` (ACP's tool-call-name * RFD); a refining `tool_call_update` carries none of these. `name` is set * once at first report — on a v1 update, omitting it means "unchanged", and @@ -9395,7 +9395,7 @@ function toolCallNotification( return { _meta: { claudeCode: claudeCodeMetaFromToolUse(toolUse, cwd), - ...(toolUse.name === "Bash" && supportsTerminalOutput + ...((toolUse.name === "Bash" || toolUse.name === "PowerShell") && supportsTerminalOutput ? { terminal_info: { terminal_id: toolUse.id } } : {}), } satisfies ToolUpdateMeta, diff --git a/src/permissions/presentation.ts b/src/permissions/presentation.ts index 2c273777..9de17f64 100644 --- a/src/permissions/presentation.ts +++ b/src/permissions/presentation.ts @@ -78,13 +78,14 @@ export function buildClaudePermissionPresentation( // the approval never maintains a second, divergent name for the operation. // decisionReason is temporarily exposed as the permission description so // its actual SDK values can be inspected; it remains diagnostic policy text. - const shellTitle = - value.toolName === "Bash" || value.toolName === "PowerShell" - ? (compactText(value.input.description) ?? value.toolName) - : undefined; - const toolCallTitle = shellTitle ?? subjectTitle ?? info.title; + const toolCallTitle = subjectTitle ?? info.title; const permissionTitle = value.toolName === "ExitPlanMode" ? "Ready to code?" : toolCallTitle; - const title = humanText(permissionTitle, 4_000, true) ?? "Use tool?"; + // Shell titles are executable input: compacting whitespace changes quoted + // arguments and comment boundaries, and length limits can hide the command. + const title = + value.toolName === "Bash" || value.toolName === "PowerShell" + ? permissionTitle + : (humanText(permissionTitle, 4_000, true) ?? "Use tool?"); const decisionReason = humanText(value.decisionReason, 4_000); const description = decisionReason ? `Reason: ${decisionReason}` : undefined; return { diff --git a/src/tests/acp-agent.test.ts b/src/tests/acp-agent.test.ts index aff95282..0d09b14e 100644 --- a/src/tests/acp-agent.test.ts +++ b/src/tests/acp-agent.test.ts @@ -3594,7 +3594,7 @@ describe("permission request cancellation", () => { { kind: "reject_once", name: "No", optionId: "reject" }, ]); expect(request?._meta).toEqual({ - permission: { version: 1, title: "Bash" }, + permission: { version: 1, title: "ls" }, }); }); @@ -3933,6 +3933,29 @@ describe("tool_call emitted before permission request", () => { expect(result).toMatchObject({ behavior: "allow" }); }); + it("carries the PowerShell description in claudeCode meta like Bash", async () => { + const { agent, updates } = setup(); + + await agent.canUseTool("session-1")( + "PowerShell", + { command: "Get-ChildItem", description: "List files" }, + { + signal: new AbortController().signal, + suggestions: [], + toolUseID: "tool-1", + } as any, + ); + + expect(updates[0].update).toMatchObject({ + sessionUpdate: "tool_call", + toolCallId: "tool-1", + title: "Get-ChildItem", + _meta: { + claudeCode: { toolName: "PowerShell", title: "List files" }, + }, + }); + }); + it("does not re-emit the tool_call when the stream already surfaced it", async () => { const { agent, events } = setup(); agent.sessions["session-1"]!.emittedToolCalls.add("tool-1"); @@ -4292,7 +4315,7 @@ describe("canUseTool in bypassPermissions mode", () => { } as any); expect(request?.options.map((option) => option.optionId)).toEqual(["allow-once", "reject"]); - expect(request?._meta).toEqual({ permission: { version: 1, title: "Bash" } }); + expect(request?._meta).toEqual({ permission: { version: 1, title: "rm -rf build" } }); }); it("leads with the reject option and forwards the hint when the CLI defaults to no", async () => { @@ -4328,7 +4351,7 @@ describe("canUseTool in bypassPermissions mode", () => { "allow_always", ]); expect(request?._meta).toEqual({ - permission: { version: 1, title: "Bash", defaultToNo: true }, + permission: { version: 1, title: "rm -rf build", defaultToNo: true }, }); expect(result).toMatchObject({ behavior: "deny" }); }); diff --git a/src/tests/permission-presentation.test.ts b/src/tests/permission-presentation.test.ts index 083aa090..7e6b7dc6 100644 --- a/src/tests/permission-presentation.test.ts +++ b/src/tests/permission-presentation.test.ts @@ -111,7 +111,7 @@ describe("Claude permission ACP v1 presentation", () => { defaultToNo: true, }); expect(presentation._meta).toEqual({ - permission: { version: 1, title: "Bash", defaultToNo: true }, + permission: { version: 1, title: "rm -rf build", defaultToNo: true }, }); expect( buildClaudePermissionPresentation({ @@ -120,10 +120,10 @@ describe("Claude permission ACP v1 presentation", () => { toolUseID: "tool-1", defaultToNo: false, })._meta, - ).toEqual({ permission: { version: 1, title: "Bash" } }); + ).toEqual({ permission: { version: 1, title: "rm -rf build" } }); }); - it("uses the human command description as the permission title", () => { + it("keeps command descriptions and decision reasons in their presentation fields", () => { const input = { command: "npm test", description: "Run the tests" }; const presentation = buildClaudePermissionPresentation({ toolName: "Bash", @@ -133,12 +133,8 @@ describe("Claude permission ACP v1 presentation", () => { description: "Run npm tests", decisionReason: "Needed to verify the change.", }); - expect(presentation._meta).toEqual({ - permission: { - version: 1, - title: "Run the tests", - description: "Reason: Needed to verify the change.", - }, + expect(presentation._meta).toMatchObject({ + permission: { description: "Reason: Needed to verify the change." }, }); expect(presentation.toolCall).toMatchObject({ toolCallId: "tool-1", @@ -146,31 +142,72 @@ describe("Claude permission ACP v1 presentation", () => { kind: "execute", status: "pending", rawInput: input, - title: "Run the tests", }); + expect(presentation.toolCall.content).toEqual([ + { type: "content", content: { type: "text", text: "Run the tests" } }, + ]); expect(presentation.toolCall.rawInput).toBe(input); }); + // `command` is required, so this only shows while the input is still + // streaming; both shells share the standard terminal card in that state. it.each(["Bash", "PowerShell"])( - "uses the %s tool name when no human command description is available", + "uses the Terminal fallback for %s when no command is available yet", (toolName) => { - const input = { command: "echo raw command" }; + const input = {}; const presentation = buildClaudePermissionPresentation({ toolName, input, toolUseID: `tool-${toolName}`, }); - expect(presentation._meta).toEqual({ - permission: { version: 1, title: toolName }, - }); - expect(presentation.toolCall).toMatchObject({ - title: toolName, - rawInput: input, - }); + expect(presentation._meta).toEqual({ permission: { version: 1, title: "Terminal" } }); + expect(presentation.toolCall).toMatchObject({ title: "Terminal", rawInput: input }); }, ); + it.each([ + ["Bash", "ls -la ~/.config/zed"], + ["PowerShell", "Get-ChildItem $HOME\\.config\\zed"], + ])( + "shows the exact %s command instead of its model-authored description", + (toolName, command) => { + const input = { command, description: "List files in current directory" }; + const presentation = buildClaudePermissionPresentation({ + toolName, + input, + toolUseID: `tool-${toolName}`, + }); + + expect(presentation._meta).toMatchObject({ permission: { title: command } }); + expect(presentation.toolCall.title).toBe(command); + }, + ); + + describe.each(["Bash", "PowerShell"])("%s command fidelity", (toolName) => { + it.each([ + { label: "quoted spaces and tabs", command: 'echo "a b\tc"' }, + { label: "newlines after comments", command: "echo first # first command\necho second" }, + { label: "surrounding whitespace", command: " \techo first\n" }, + { + label: "commands longer than 4,000 characters", + command: `echo "${"x".repeat(4_001)}"\necho last`, + }, + ])("preserves $label in the approval title", ({ command }) => { + const input = { command, description: "Run the requested command" }; + const presentation = buildClaudePermissionPresentation({ + toolName, + input, + toolUseID: `tool-${toolName}`, + supportsTerminalOutput: true, + }); + + expect(presentation._meta).toEqual({ permission: { version: 1, title: command } }); + expect(presentation.toolCall.title).toBe(command); + expect(presentation.toolCall.rawInput).toBe(input); + }); + }); + it("keeps the WebFetch URL in structured tool input", () => { const input = { url: "https://example.com/docs", prompt: "Read the API reference" }; const presentation = buildClaudePermissionPresentation({ diff --git a/src/tests/tools.test.ts b/src/tests/tools.test.ts index 24ce4c41..77d19306 100644 --- a/src/tests/tools.test.ts +++ b/src/tests/tools.test.ts @@ -1726,6 +1726,88 @@ describe("Bash terminal output", () => { }); }); +describe("PowerShell terminal output", () => { + it("emits output and exit for the terminal announced by the tool call", () => { + const id = "toolu_powershell"; + const toolUse = { + type: "tool_use" as const, + id, + name: "PowerShell", + input: { command: "Get-ChildItem" }, + }; + const toolResult: ToolResultBlockParam = { + type: "tool_result", + tool_use_id: id, + content: "Get-ChildItem: command failed", + is_error: true, + }; + const toolUseCache: ToolUseCache = {}; + const client = {} as AcpClient; + const logger: Logger = { log: () => {}, error: () => {} }; + const options = { + registerHooks: false, + clientCapabilities: { _meta: { terminal_output: true } }, + }; + const notifications = [ + ...toAcpNotifications( + [toolUse], + "assistant", + "test-session", + toolUseCache, + client, + logger, + options, + ), + ...toAcpNotifications( + [toolResult], + "user", + "test-session", + toolUseCache, + client, + logger, + options, + ), + ]; + + expect(notifications).toHaveLength(3); + const [started, output, exited] = notifications.map(({ update }) => update); + expect(started).toMatchObject({ + sessionUpdate: "tool_call", + toolCallId: id, + status: "pending", + title: "Get-ChildItem", + kind: "execute", + content: [{ type: "terminal", terminalId: id }], + _meta: { terminal_info: { terminal_id: id } }, + }); + expect(output).toMatchObject({ + sessionUpdate: "tool_call_update", + toolCallId: id, + _meta: { + terminal_output: { + terminal_id: id, + data: "Get-ChildItem: command failed", + }, + }, + }); + expect(output).not.toHaveProperty("status"); + expect(exited).toMatchObject({ + sessionUpdate: "tool_call_update", + toolCallId: id, + status: "failed", + content: [{ type: "terminal", terminalId: id }], + _meta: { + terminal_exit: { + terminal_id: id, + exit_code: 1, + signal: null, + }, + }, + }); + expect(exited).not.toHaveProperty("rawOutput"); + }); +}); + describe("toolInfoFromToolUse - ExitPlanMode", () => { it("should include plan text in content when input.plan is provided", () => { const toolUse = { diff --git a/src/tools.ts b/src/tools.ts index cf55bbff..614ddabc 100644 --- a/src/tools.ts +++ b/src/tools.ts @@ -156,7 +156,8 @@ export function toolInfoFromToolUse( }; } - case "Bash": { + case "Bash": + case "PowerShell": { const input = toolUse.input as BashInput | undefined; return { title: input?.command ? input.command : "Terminal", @@ -641,7 +642,7 @@ export function toolUpdateFromToolResult( toolResult.is_error && toolResult.content && toolResult.content.length > 0 && - !(toolUse?.name === "Bash" && supportsTerminalOutput) + !((toolUse?.name === "Bash" || toolUse?.name === "PowerShell") && supportsTerminalOutput) ) { // Only return errors return toAcpContentUpdate(toolResult.content, true); @@ -733,7 +734,8 @@ export function toolUpdateFromToolResult( return {}; } - case "Bash": { + case "Bash": + case "PowerShell": { const result = toolResult.content; // The terminal was announced under the tool_use's own id (see // `toolInfoFromToolUse`), so key the output/exit metas off that: it is the