XS✔ ◾ Fix: Claude Code mode ignores Wait approval mode and auto-executes tools

AGENTS.md

@@ -589,9 +589,12 @@ server instead of re-declaring each upstream server to Claude:
   - `GET /tools` → the full server-prefixed tool list (names + descriptions + JSON
     Schema). Returns `[]` (never an error) when no enabled servers are healthy.
   - `POST /tools/call` → executes one tool by name through the app's tool-approval
-    policy SERVER-SIDE, so a headless run never hangs on an interactive prompt
-    (`yolo` runs everything; `ask`/`wait` run only whitelisted tools, otherwise return
-    a structured "not approved" envelope, never a transport error).
+    policy SERVER-SIDE: `yolo` runs everything; `ask`/`wait` gate on the whitelist —
+    `ask` returns a structured "not approved" envelope for anything not whitelisted
+    (never prompts, since a headless caller can't answer), while `wait` raises the
+    SAME approval dialog+countdown the OpenAI backend shows and blocks the call until
+    the user responds or the countdown auto-approves (never a transport error either
+    way).
 - **Wiring**: `McpToolBridge` (`src/backend/services/mcp/mcp-tool-bridge.ts`) flattens
   the AI-SDK `ToolSet` into wire summaries and enforces the approval policy; the front
   door (`mcp-serve.ts`) maps a `{ok:false}` envelope to an MCP `isError` result.

src/backend/services/cli-bridge/bridge-router.ts

@@ -52,6 +52,7 @@ export interface BridgeServices {
       name: string,
       args?: Record<string, unknown>,
       serverFilter?: string[],
+      shaveId?: string,
     ): Promise<ToolCallResult>;
   };
 }
@@ -325,6 +326,7 @@ async function routeTools(
       parsed.data.name,
       parsed.data.arguments ?? {},
       parsed.data.serverFilter,
+      parsed.data.shaveId,
     );
     // A tool-level failure (incl. a structured "not approved") is still a
     // successful BRIDGE response — the envelope carries {ok:false,...} so the

src/backend/services/cli-bridge/cli-bridge-server.ts

@@ -16,6 +16,7 @@ import { McpToolBridge } from "../mcp/mcp-tool-bridge";
 import { applyOpenAtLoginSetting } from "../settings/login-item";
 import { LlmStorage } from "../storage/llm-storage";
 import { UserSettingsStorage } from "../storage/user-settings-storage";
+import { UserInteractionService } from "../user-interaction/user-interaction-service";
 import { type BridgeServices, routeRequest } from "./bridge-router";
 
 const MAX_BODY_BYTES = 256 * 1024; // 256 KB is plenty for config payloads.
@@ -313,8 +314,8 @@ export function createDefaultServices(): BridgeServices {
       async listTools(serverFilter) {
         return (await getToolBridge()).listTools(serverFilter);
       },
-      async callTool(name, args, serverFilter) {
-        return (await getToolBridge()).callTool(name, args, serverFilter);
+      async callTool(name, args, serverFilter, shaveId) {
+        return (await getToolBridge()).callTool(name, args, serverFilter, shaveId);
       },
     },
   };
@@ -323,7 +324,9 @@ export function createDefaultServices(): BridgeServices {
 /** Build a {@link McpToolBridge} wired to the live singletons. */
 async function getToolBridge(): Promise<McpToolBridge> {
   const manager = await MCPServerManager.getInstanceAsync();
-  return new McpToolBridge(manager, {
-    getSettingsAsync: () => UserSettingsStorage.getInstance().getSettingsAsync(),
-  });
+  return new McpToolBridge(
+    manager,
+    { getSettingsAsync: () => UserSettingsStorage.getInstance().getSettingsAsync() },
+    UserInteractionService.getInstance(),
+  );
 }

src/backend/services/cli-bridge/front-door.integration.test.ts

@@ -1,11 +1,16 @@
 import { createServer, type Server as HttpServer } from "node:http";
 import type { AddressInfo } from "node:net";
 import type { ToolSet } from "ai";
-import { afterEach, describe, expect, it } from "vitest";
+import { afterEach, describe, expect, it, vi } from "vitest";
 import { z } from "zod";
 import { BridgeClient } from "../../../cli/bridge-client";
 import { callToolViaBridge, listToolsViaBridge } from "../../../cli/mcp-serve";
-import { McpToolBridge, type ToolBridgeManager } from "../mcp/mcp-tool-bridge";
+import type { ToolApprovalDecision } from "../../../shared/types/mcp";
+import {
+  McpToolBridge,
+  type ToolBridgeManager,
+  type ToolBridgeUserInteraction,
+} from "../mcp/mcp-tool-bridge";
 import { type BridgeServices, routeRequest } from "./bridge-router";
 
 /**
@@ -79,10 +84,18 @@ function makeManager(whitelist: string[]): ToolBridgeManager {
 }
 
 /** Build the real BridgeServices, with only the tools front-door wired to a real bridge. */
-function makeServices(approvalMode: "yolo" | "ask" | "wait", whitelist: string[]): BridgeServices {
-  const bridge = new McpToolBridge(makeManager(whitelist), {
-    getSettingsAsync: async () => ({ toolApprovalMode: approvalMode }),
-  });
+function makeServices(
+  approvalMode: "yolo" | "ask" | "wait",
+  whitelist: string[],
+  userInteraction: ToolBridgeUserInteraction = {
+    requestToolApproval: async () => ({ kind: "approve" }),
+  },
+): BridgeServices {
+  const bridge = new McpToolBridge(
+    makeManager(whitelist),
+    { getSettingsAsync: async () => ({ toolApprovalMode: approvalMode }) },
+    userInteraction,
+  );
   // Only the routes this test exercises need real backing; the rest can throw.
   const notUsed = () => {
     throw new Error("not part of the front-door path");
@@ -99,7 +112,12 @@ function makeServices(approvalMode: "yolo" | "ask" | "wait", whitelist: string[]
     settings: { getSettingsAsync: notUsed, updateSettingsAsync: notUsed },
     tools: {
       listTools: () => bridge.listTools(),
-      callTool: (name: string, args?: Record<string, unknown>) => bridge.callTool(name, args),
+      callTool: (
+        name: string,
+        args?: Record<string, unknown>,
+        serverFilter?: string[],
+        shaveId?: string,
+      ) => bridge.callTool(name, args, serverFilter, shaveId),
     },
   } as unknown as BridgeServices;
 }
@@ -210,6 +228,84 @@ describe("front-door integration — mcp-serve → BridgeClient → router → M
     expect(result.content).toEqual([{ type: "text", text: "Created issue: OK" }]);
   });
 
+  it("raises the approval prompt for a non-whitelisted tool under 'wait' and runs it once APPROVED (#920)", async () => {
+    const requestToolApproval = vi
+      .fn<ToolBridgeUserInteraction["requestToolApproval"]>()
+      .mockResolvedValue({ kind: "approve" } satisfies ToolApprovalDecision);
+    ({ server, port } = await startBridge(
+      makeServices("wait", /* whitelist */ [], { requestToolApproval }),
+    ));
+    const client = makeClient(port);
+
+    const result = await callToolViaBridge(client, "GitHub__create_issue", { title: "Bug" });
+
+    // The bug this closes (#920): Claude Code mode used to run this tool WITHOUT ever consulting
+    // the approval flow. Proving the mock was actually invoked, over the real HTTP round-trip, is
+    // the regression guard.
+    expect(requestToolApproval).toHaveBeenCalledTimes(1);
+    expect(requestToolApproval).toHaveBeenCalledWith(
+      "GitHub__create_issue",
+      { title: "Bug" },
+      expect.objectContaining({}),
+    );
+    expect(result.isError).toBe(false);
+    expect(result.content).toEqual([{ type: "text", text: "Created issue: Bug" }]);
+  });
+
+  it("does NOT run a non-whitelisted tool under 'wait' when the user denies it", async () => {
+    const requestToolApproval = vi
+      .fn<ToolBridgeUserInteraction["requestToolApproval"]>()
+      .mockResolvedValue({ kind: "deny_stop", feedback: "no" } satisfies ToolApprovalDecision);
+    ({ server, port } = await startBridge(
+      makeServices("wait", /* whitelist */ [], { requestToolApproval }),
+    ));
+    const client = makeClient(port);
+
+    const result = await callToolViaBridge(client, "GitHub__create_issue", { title: "Bug" });
+
+    expect(requestToolApproval).toHaveBeenCalledTimes(1);
+    expect(result.isError).toBe(true);
+    expect((result.content[0] as { text?: string }).text).toMatch(/not approved/i);
+  });
+
+  it("runs a WHITELISTED tool under 'wait' without prompting", async () => {
+    const requestToolApproval = vi.fn<ToolBridgeUserInteraction["requestToolApproval"]>();
+    ({ server, port } = await startBridge(
+      makeServices("wait", ["GitHub__create_issue"], { requestToolApproval }),
+    ));
+    const client = makeClient(port);
+
+    const result = await callToolViaBridge(client, "GitHub__create_issue", { title: "OK" });
+
+    expect(requestToolApproval).not.toHaveBeenCalled();
+    expect(result.isError).toBe(false);
+    expect(result.content).toEqual([{ type: "text", text: "Created issue: OK" }]);
+  });
+
+  it("forwards shaveId end-to-end from callToolViaBridge to McpToolBridge (#920 per-shave auto-approve)", async () => {
+    const requestToolApproval = vi
+      .fn<ToolBridgeUserInteraction["requestToolApproval"]>()
+      .mockResolvedValue({ kind: "approve" } satisfies ToolApprovalDecision);
+    ({ server, port } = await startBridge(
+      makeServices("wait", /* whitelist */ [], { requestToolApproval }),
+    ));
+    const client = makeClient(port);
+
+    await callToolViaBridge(
+      client,
+      "GitHub__create_issue",
+      { title: "Bug" },
+      undefined,
+      "shave-42",
+    );
+
+    expect(requestToolApproval).toHaveBeenCalledWith(
+      "GitHub__create_issue",
+      { title: "Bug" },
+      expect.objectContaining({ shaveId: "shave-42" }),
+    );
+  });
+
   it("rejects an unauthenticated request at the HTTP boundary", async () => {
     await boot("yolo");
     const badClient = new BridgeClient({

src/backend/services/mcp/local-claude-orchestrator.test.ts

@@ -215,11 +215,14 @@ describe("LocalClaudeOrchestrator", () => {
       expect(argv).not.toContain("bypassPermissions");
     });
 
-    it("wait -> --permission-mode bypassPermissions (headless can't defer the prompt)", () => {
+    it("wait -> --permission-mode bypassPermissions at the CLI layer (MCP tools are gated by the bridge instead, #920)", () => {
       const orch = new LocalClaudeOrchestrator();
       const argv = orch.buildArgv("/tmp/cfg.json", "/tmp/prompt.txt", "wait", []);
-      // main maps `wait` -> bypassPermissions (headless can't show the deferred dialog), so an empty
-      // whitelist still runs every tool. There is no --allowedTools because none was passed.
+      // `wait` still bypasses Claude's OWN built-in-tool permission system at the CLI layer (there is
+      // no bridge in front of those to defer to) — there is no --allowedTools because none was
+      // passed. This does NOT mean MCP tools run unchecked: McpToolBridge.callTool enforces the real
+      // wait-mode approval dialog+countdown server-side, independent of this flag (see
+      // mcp-tool-bridge.test.ts).
       expect(argv).not.toContain("--allowedTools");
       expect(argv).toContain("bypassPermissions");
     });

src/backend/services/mcp/local-claude-orchestrator.ts

@@ -6,6 +6,7 @@ import { join } from "node:path";
 import {
   CLI_BRIDGE_PORT_ENV,
   CLI_BRIDGE_SERVER_FILTER_ENV,
+  CLI_BRIDGE_SHAVE_ID_ENV,
   CLI_BRIDGE_TOKEN_ENV,
 } from "../../../shared/cli-bridge/protocol";
 import type { ToolApprovalMode } from "../../../shared/types/user-settings";
@@ -187,7 +188,8 @@ export class LocalClaudeOrchestrator implements IBacklogOrchestrator {
     await this.ensureClaudeAvailable();
 
     const manager = this.serverManager ?? (await MCPServerManager.getInstanceAsync());
-    const frontDoor = this.frontDoor ?? (await resolveFrontDoorConfig(options.serverFilter));
+    const frontDoor =
+      this.frontDoor ?? (await resolveFrontDoorConfig(options.serverFilter, options.shaveId));
     const approvalMode = (await this.getSettingsStorage().getSettingsAsync()).toolApprovalMode;
 
     const systemPrompt = this.buildSystemPrompt(
@@ -265,18 +267,10 @@ export class LocalClaudeOrchestrator implements IBacklogOrchestrator {
       console.warn(`[LocalClaudeOrchestrator] ${msg}`);
     }
 
-    // Under "wait", the OpenAI path shows a 15s approval dialog the user can still deny within. A
-    // headless `claude -p` can't render that deferred prompt, so "wait" runs EVERY tool immediately
-    // (effectively YOLO) under the local backend. Tell the user so the safe-looking "wait" setting
-    // doesn't silently widen tool execution without any signal.
-    if (approvalMode === "wait") {
-      const msg =
-        'Claude Code (local) cannot show the "wait" approval dialog because it runs headlessly, ' +
-        "so under this backend every tool runs immediately without a chance to deny (the same as " +
-        '"YOLO"). Switch to "ask" to restrict the run to whitelisted tools only.';
-      notices.push(msg);
-      console.warn(`[LocalClaudeOrchestrator] ${msg}`);
-    }
+    // "wait" needs no caveat here (#920): a non-whitelisted MCP tool call is gated by
+    // `McpToolBridge.callTool` in the main process, which raises the same approval dialog+countdown
+    // the OpenAI backend shows and blocks the run until the user responds or the countdown
+    // auto-approves — the same behaviour as the OpenAI path, not a silent widening to YOLO.
 
     for (const text of notices) {
       onStep?.({ type: "reasoning", reasoning: JSON.stringify({ type: "text", text }) });
@@ -366,17 +360,25 @@ Embed this URL in the task content that you create. Follow user requirements STR
   /**
    * Maps the approval mode to Claude Code's permission flags.
    *
-   * - `yolo` → `--permission-mode bypassPermissions` (run every tool immediately).
-   * - `wait` → ALSO `--permission-mode bypassPermissions`. In the OpenAI backend `wait` shows the
-   *   approval prompt but AUTO-APPROVES after a delay, so a non-whitelisted tool the user doesn't
-   *   dismiss still RUNS. A headless `claude -p` can't render a deferred prompt, so the faithful
-   *   analogue of "auto-approve after a delay" is to bypass permissions — otherwise `wait` would
-   *   silently HARD-DENY non-whitelisted tools, diverging from the OpenAI behaviour for the same
-   *   setting. (See requestToolApproval's `wait` -> auto-approve path.)
+   * These flags only control Claude's OWN built-in tools (Read/Write/Bash/etc.) at the CLI layer.
+   * MCP tools (everything YakShaver actually cares about — GitHub/Jira/Azure DevOps/etc.) do NOT
+   * stop at this layer: every `tools/call` for the single `yakshaver` front-door is proxied to
+   * {@link McpToolBridge.callTool} in the main process, which enforces the REAL approval policy
+   * server-side regardless of `--permission-mode` (#920) — including, under `wait`, raising the
+   * same approval dialog+countdown the OpenAI backend shows.
+   *
+   * - `yolo` → `--permission-mode bypassPermissions` (run every built-in tool immediately; MCP
+   *   tools run immediately too, per the bridge's own `yolo` handling).
+   * - `wait` → ALSO `--permission-mode bypassPermissions`, so Claude's own built-in tools aren't
+   *   hard-denied at the CLI layer (there is no bridge in front of those to defer to). MCP tools are
+   *   NOT auto-approved here — the bridge gates them on the whitelist and prompts via the approval
+   *   dialog for anything not already whitelisted, so `wait` behaves the same as the OpenAI backend
+   *   for the tools that matter.
    * - `ask` → `--allowedTools <whitelist>` + `--permission-mode dontAsk`. `--allowedTools` only
    *   auto-approves the listed tools; on its own (default permission mode) a non-whitelisted tool
    *   would trigger an interactive prompt the headless run can't answer and the run can hang/abort.
-   *   `dontAsk` converts any such prompt into an outright denial, so the run never blocks.
+   *   `dontAsk` converts any such prompt into an outright denial, so the run never blocks. (MCP
+   *   tools are additionally gated by the bridge itself, same as always.)
    *
    * `--strict-mcp-config` ensures Claude only uses the servers we serialized into `--mcp-config`,
    * ignoring any ambient project `.mcp.json` / `~/.claude` MCP config so the embedded run is
@@ -706,7 +708,10 @@ Embed this URL in the task content that you create. Follow user requirements STR
  * Lazily imports electron + the bridge server so the pure argv/config unit tests (which always
  * inject a `frontDoor`) never touch these singletons.
  */
-async function resolveFrontDoorConfig(serverFilter?: string[]): Promise<YakshaverFrontDoorConfig> {
+async function resolveFrontDoorConfig(
+  serverFilter?: string[],
+  shaveId?: string,
+): Promise<YakshaverFrontDoorConfig> {
   const { app } = await import("electron");
   const { CliBridgeServer } = await import("../cli-bridge/cli-bridge-server");
 
@@ -723,6 +728,11 @@ async function resolveFrontDoorConfig(serverFilter?: string[]): Promise<Yakshave
   if (serverFilter && serverFilter.length > 0) {
     env[CLI_BRIDGE_SERVER_FILTER_ENV] = serverFilter.join(",");
   }
+  // Forward the shave id so a `wait`-mode approval prompt raised via the bridge (#920) can honour
+  // the same per-shave auto-approve override the OpenAI backend supports.
+  if (shaveId) {
+    env[CLI_BRIDGE_SHAVE_ID_ENV] = shaveId;
+  }
 
   return { command: process.execPath, cliEntryPath, env };
 }