fix: merge user-provided disallowedTools instead of overwriting (agentclientprotocol#335)

The disallowedTools merge fix from agentclientprotocol#295 was regressed by agentclientprotocol#316 when the
options object was restructured. Restore the merge behavior and add
regression tests for all three documented merge fields (disallowedTools,
hooks, mcpServers).

Fixes agentclientprotocol#334

Author: blimmer

src/acp-agent.ts

@@ -1251,7 +1251,7 @@ export class ClaudeAcpAgent implements Agent {
         : isStaticBinary()
           ? { pathToClaudeCodeExecutable: process.execPath }
           : {}),
-      disallowedTools,
+      disallowedTools: [...(userProvidedOptions?.disallowedTools || []), ...disallowedTools],
       tools: { type: "preset", preset: "claude_code" },
       hooks: {
         ...userProvidedOptions?.hooks,

src/tests/create-session-options.test.ts

@@ -0,0 +1,185 @@
+import { describe, it, expect, beforeEach, vi } from "vitest";
+import { AgentSideConnection, SessionNotification } from "@agentclientprotocol/sdk";
+import type { Options } from "@anthropic-ai/claude-agent-sdk";
+import type { ClaudeAcpAgent as ClaudeAcpAgentType } from "../acp-agent.js";
+
+let capturedOptions: Options | undefined;
+vi.mock("@anthropic-ai/claude-agent-sdk", async () => {
+  const actual = await vi.importActual<typeof import("@anthropic-ai/claude-agent-sdk")>(
+    "@anthropic-ai/claude-agent-sdk",
+  );
+  return {
+    ...actual,
+    query: (args: { prompt: unknown; options: Options }) => {
+      capturedOptions = args.options;
+      return {
+        initializationResult: async () => ({
+          models: [
+            { value: "claude-sonnet-4-5", displayName: "Claude Sonnet", description: "Fast" },
+          ],
+        }),
+        setModel: async () => {},
+        supportedCommands: async () => [],
+        [Symbol.asyncIterator]: async function* () {},
+      };
+    },
+  };
+});
+
+vi.mock("../tools.js", async () => {
+  const actual = await vi.importActual<typeof import("../tools.js")>("../tools.js");
+  return {
+    ...actual,
+    registerHookCallback: vi.fn(),
+  };
+});
+
+describe("createSession options merging", () => {
+  let agent: ClaudeAcpAgentType;
+  let ClaudeAcpAgent: typeof ClaudeAcpAgentType;
+
+  function createMockClient(): AgentSideConnection {
+    return {
+      sessionUpdate: async (_notification: SessionNotification) => {},
+      requestPermission: async () => ({ outcome: { outcome: "cancelled" } }),
+      readTextFile: async () => ({ content: "" }),
+      writeTextFile: async () => ({}),
+    } as unknown as AgentSideConnection;
+  }
+
+  beforeEach(async () => {
+    capturedOptions = undefined;
+
+    vi.resetModules();
+    const acpAgent = await import("../acp-agent.js");
+    ClaudeAcpAgent = acpAgent.ClaudeAcpAgent;
+
+    agent = new ClaudeAcpAgent(createMockClient());
+  });
+
+  it("merges user-provided disallowedTools with ACP internal list", async () => {
+    await agent.newSession({
+      cwd: "/test",
+      mcpServers: [],
+      _meta: {
+        claudeCode: {
+          options: {
+            disallowedTools: ["WebSearch", "WebFetch"],
+          },
+        },
+      },
+    });
+
+    // User-provided tools should be present
+    expect(capturedOptions!.disallowedTools).toContain("WebSearch");
+    expect(capturedOptions!.disallowedTools).toContain("WebFetch");
+    // ACP's internal disallowed tool should also be present
+    expect(capturedOptions!.disallowedTools).toContain("AskUserQuestion");
+  });
+
+  it("works when user provides no disallowedTools", async () => {
+    await agent.newSession({
+      cwd: "/test",
+      mcpServers: [],
+    });
+
+    expect(capturedOptions!.disallowedTools).toContain("AskUserQuestion");
+  });
+
+  it("works when user provides empty disallowedTools", async () => {
+    await agent.newSession({
+      cwd: "/test",
+      mcpServers: [],
+      _meta: {
+        claudeCode: {
+          options: {
+            disallowedTools: [],
+          },
+        },
+      },
+    });
+
+    expect(capturedOptions!.disallowedTools).toContain("AskUserQuestion");
+  });
+
+  it("includes both user and built-in disallowed tools when disableBuiltInTools is true", async () => {
+    await agent.newSession({
+      cwd: "/test",
+      mcpServers: [],
+      _meta: {
+        disableBuiltInTools: true,
+        claudeCode: {
+          options: {
+            disallowedTools: ["CustomTool"],
+          },
+        },
+      },
+    });
+
+    const disallowed = capturedOptions!.disallowedTools!;
+    // User-provided
+    expect(disallowed).toContain("CustomTool");
+    // ACP internal
+    expect(disallowed).toContain("AskUserQuestion");
+    // Built-in tools disabled by disableBuiltInTools
+    expect(disallowed).toContain("Read");
+    expect(disallowed).toContain("Write");
+    expect(disallowed).toContain("Bash");
+  });
+
+  it("merges user-provided hooks with ACP hooks", async () => {
+    const userPreToolUseHook = { hooks: [{ command: "echo pre" }] };
+
+    await agent.newSession({
+      cwd: "/test",
+      mcpServers: [],
+      _meta: {
+        claudeCode: {
+          options: {
+            hooks: {
+              PreToolUse: [userPreToolUseHook],
+              PostToolUse: [{ hooks: [{ command: "echo user-post" }] }],
+            },
+          },
+        },
+      },
+    });
+
+    // User's PreToolUse hooks should be preserved
+    expect(capturedOptions!.hooks?.PreToolUse).toEqual([userPreToolUseHook]);
+    // PostToolUse should contain both user and ACP hooks
+    expect(capturedOptions!.hooks?.PostToolUse).toHaveLength(2);
+  });
+
+  it("merges user-provided mcpServers with ACP mcpServers", async () => {
+    await agent.newSession({
+      cwd: "/test",
+      mcpServers: [
+        {
+          name: "acp-server",
+          command: "node",
+          args: ["acp-server.js"],
+          env: [],
+        },
+      ],
+      _meta: {
+        claudeCode: {
+          options: {
+            mcpServers: {
+              "user-server": {
+                type: "stdio",
+                command: "node",
+                args: ["server.js"],
+              },
+            },
+          },
+        },
+      },
+    });
+
+    // User-provided MCP server should be present
+    expect(capturedOptions!.mcpServers).toHaveProperty("user-server");
+    // ACP-provided MCP server should also be present
+    expect(capturedOptions!.mcpServers).toHaveProperty("acp-server");
+  });
+});