diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 914826e09..a4b5fbb1e 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed ACP bash permission requests to include execute tool metadata and command content so clients can render command approval prompts consistently. ([#1189](https://github.com/can1357/oh-my-pi/issues/1189)) + ### Added - Added scoped service tier values to the `serviceTier` setting: `priority (OpenAI only)` and `priority (Claude only)`. They let you opt into premium processing on one provider family without paying premium costs on the other when switching models mid-session. `/fast on` continues to set the unscoped `"priority"` (active everywhere supported); `/fast status` and `isFastModeEnabled()` now report `on` for any scoped value too. diff --git a/packages/coding-agent/src/modes/acp/acp-client-bridge.ts b/packages/coding-agent/src/modes/acp/acp-client-bridge.ts index daebd4f80..20ceace1d 100644 --- a/packages/coding-agent/src/modes/acp/acp-client-bridge.ts +++ b/packages/coding-agent/src/modes/acp/acp-client-bridge.ts @@ -124,6 +124,7 @@ async function requestPermission( ...(toolCall.kind ? { kind: toolCall.kind as ToolCallUpdate["kind"] } : {}), ...(toolCall.status ? { status: toolCall.status as ToolCallUpdate["status"] } : {}), ...(toolCall.rawInput !== undefined ? { rawInput: toolCall.rawInput } : {}), + ...(toolCall.content ? { content: toolCall.content as ToolCallUpdate["content"] } : {}), ...(toolCall.locations ? { locations: toolCall.locations } : {}), }; const acpOptions: AcpPermissionOption[] = options.map(option => ({ diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index eab4d6e3d..ea44daa77 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -3113,6 +3113,13 @@ export class AgentSession { if (!permissionIntent) { return await target.execute(toolCallId, args as never, signal, onUpdate, ctx); } + const command = + target.name === "bash" && args && typeof args === "object" && !Array.isArray(args) + ? getStringProperty(args as Record, "command") + : undefined; + const commandContent = command + ? [{ type: "content" as const, content: { type: "text" as const, text: `$ ${command}` } }] + : undefined; // Short-circuit on persisted decisions. const persisted = this.#acpPermissionDecisions.get(permissionIntent.cacheKey); if (persisted === "allow_always") { @@ -3137,8 +3144,10 @@ export class AgentSession { toolCallId, toolName: target.name, title: permissionIntent.title, + ...(target.name === "bash" ? { kind: "execute" } : {}), status: "pending", rawInput: args, + ...(commandContent ? { content: commandContent } : {}), locations: extractPermissionLocations( args, this.sessionManager.getCwd(), diff --git a/packages/coding-agent/src/session/client-bridge.ts b/packages/coding-agent/src/session/client-bridge.ts index 0043e6262..d3db19486 100644 --- a/packages/coding-agent/src/session/client-bridge.ts +++ b/packages/coding-agent/src/session/client-bridge.ts @@ -27,6 +27,7 @@ export interface ClientBridgePermissionToolCall { kind?: string; status?: "pending" | "in_progress" | "completed" | "failed"; rawInput?: unknown; + content?: unknown[]; locations?: { path: string; line?: number }[]; } diff --git a/packages/coding-agent/test/acp-client-bridge.test.ts b/packages/coding-agent/test/acp-client-bridge.test.ts index b5d75e08b..62946af23 100644 --- a/packages/coding-agent/test/acp-client-bridge.test.ts +++ b/packages/coding-agent/test/acp-client-bridge.test.ts @@ -22,6 +22,7 @@ describe("ACP client bridge permission requests", () => { kind: "execute", status: "pending", rawInput: { command: "echo hi" }, + content: [{ type: "content", content: { type: "text", text: "$ echo hi" } }], }, [{ optionId: "allow_once", name: "Allow once", kind: "allow_once" }], ); @@ -32,6 +33,7 @@ describe("ACP client bridge permission requests", () => { kind: "execute", status: "pending", rawInput: { command: "echo hi" }, + content: [{ type: "content", content: { type: "text", text: "$ echo hi" } }], }); }); }); diff --git a/packages/coding-agent/test/agent-session-acp-permission.test.ts b/packages/coding-agent/test/agent-session-acp-permission.test.ts index 7ee9f6d20..94c6936c8 100644 --- a/packages/coding-agent/test/agent-session-acp-permission.test.ts +++ b/packages/coding-agent/test/agent-session-acp-permission.test.ts @@ -501,6 +501,43 @@ it("permission requests report the gated tool call as pending", async () => { expect(bashTool.executeCalls).toBe(1); }); +it("bash permission requests include execute metadata and command content", async () => { + const bashTool = makeFakeTool("bash"); + const requests: ClientBridgePermissionToolCall[] = []; + const bridge: ClientBridge = { + capabilities: { requestPermission: true }, + async requestPermission(toolCall, _options, _signal) { + requests.push(toolCall); + return { outcome: "selected", optionId: "allow_once", kind: "allow_once" }; + }, + }; + session = await createSession([bashTool], bridge); + + await session.setActiveToolsByName(["bash"]); + const wrappedBash = session.agent.state.tools.find(t => t.name === "bash"); + expect(wrappedBash).toBeDefined(); + + await wrappedBash!.execute( + "call-bash-rich", + { command: "git status --short" }, + undefined, + undefined as never, + undefined as never, + ); + + expect(requests).toHaveLength(1); + expect(requests[0]).toMatchObject({ + toolCallId: "call-bash-rich", + toolName: "bash", + title: "git status --short", + kind: "execute", + status: "pending", + rawInput: { command: "git status --short" }, + content: [{ type: "content", content: { type: "text", text: "$ git status --short" } }], + }); + expect(bashTool.executeCalls).toBe(1); +}); + it("ordinary edit calls still bypass ACP permission after rejecting edit moves forever", async () => { const editTool = makeFakeTool("edit"); const bridge = makeBridge({ outcome: "selected", optionId: "reject_always", kind: "reject_always" });