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