fix(agent): escalate mcp-store tool approvals to the user

packages/agent/src/adapters/claude/hooks.test.ts

@@ -16,6 +16,11 @@ import {
   createTaskHook,
   type EnrichedReadCache,
 } from "./hooks";
+import {
+  clearMcpToolApprovalCache,
+  clearMcpToolMetadataCache,
+  setMcpToolApprovalStates,
+} from "./mcp/tool-metadata";
 import type {
   PermissionCheckResult,
   SettingsManager,
@@ -313,6 +318,81 @@ describe("createPreToolUseHook", () => {
       hookSpecificOutput: { permissionDecision: "ask" },
     });
   });
+
+  test("routes needs_approval MCP Store tool to canUseTool via ask", async () => {
+    clearMcpToolMetadataCache();
+    clearMcpToolApprovalCache();
+    setMcpToolApprovalStates({
+      mcp__granola__query_granola_meetings: "needs_approval",
+    });
+    const settingsManager = buildSettingsManagerStub({ decision: "ask" });
+    const hook = createPreToolUseHook(settingsManager, logger);
+    const result = await hook(
+      buildPreToolUseHookInput("mcp__granola__query_granola_meetings", {}),
+      undefined,
+      { signal: new AbortController().signal },
+    );
+
+    expect(result).toMatchObject({
+      continue: true,
+      hookSpecificOutput: {
+        hookEventName: "PreToolUse",
+        permissionDecision: "ask",
+      },
+    });
+    clearMcpToolMetadataCache();
+    clearMcpToolApprovalCache();
+  });
+
+  test("denies do_not_use MCP Store tool", async () => {
+    clearMcpToolMetadataCache();
+    clearMcpToolApprovalCache();
+    setMcpToolApprovalStates({
+      mcp__granola__query_granola_meetings: "do_not_use",
+    });
+    const settingsManager = buildSettingsManagerStub({ decision: "ask" });
+    const hook = createPreToolUseHook(settingsManager, logger);
+    const result = await hook(
+      buildPreToolUseHookInput("mcp__granola__query_granola_meetings", {}),
+      undefined,
+      { signal: new AbortController().signal },
+    );
+
+    expect(result).toMatchObject({
+      continue: true,
+      hookSpecificOutput: {
+        hookEventName: "PreToolUse",
+        permissionDecision: "deny",
+      },
+    });
+    clearMcpToolMetadataCache();
+    clearMcpToolApprovalCache();
+  });
+
+  test("respects an explicit allow rule for a needs_approval MCP Store tool", async () => {
+    clearMcpToolMetadataCache();
+    clearMcpToolApprovalCache();
+    setMcpToolApprovalStates({
+      mcp__granola__query_granola_meetings: "needs_approval",
+    });
+    const settingsManager = buildSettingsManagerStub({
+      decision: "allow",
+      rule: "mcp__granola__query_granola_meetings",
+      source: "allow",
+    });
+    const hook = createPreToolUseHook(settingsManager, logger);
+    const result = await hook(
+      buildPreToolUseHookInput("mcp__granola__query_granola_meetings", {}),
+      undefined,
+      { signal: new AbortController().signal },
+    );
+
+    expect(result).toMatchObject({
+      hookSpecificOutput: { permissionDecision: "allow" },
+    });
+    clearMcpToolMetadataCache();
+    clearMcpToolApprovalCache();
+  });
 });
 
 describe("createSignedCommitGuardHook", () => {

packages/agent/src/adapters/claude/hooks.ts

@@ -7,6 +7,7 @@ import type { Logger } from "../../utils/logger";
 import { SIGNED_COMMIT_QUALIFIED_TOOL_NAME } from "../signed-commit-shared";
 import { stripCatLineNumbers } from "./conversion/sdk-to-acp";
 import type { TaskState } from "./conversion/task-state";
+import { getMcpToolApprovalState } from "./mcp/tool-metadata";
 import {
   extractPostHogSubTool,
   isPostHogDestructiveSubTool,
@@ -410,6 +411,45 @@ export const createPreToolUseHook =
       );
     }
 
+    // MCP Store per-tool approval. Cloud sessions run in bypassPermissions,
+    // which skips canUseTool entirely — so this PreToolUse hook is the only
+    // gate that runs for these tools. Force a decision here: deny blocked
+    // tools outright, and route needs_approval tools back through canUseTool
+    // (via "ask") so handleMcpApprovalFlow relays the approval dialog to the
+    // desktop and persists the result to the backend. An explicit allow rule
+    // (e.g. a prior "always allow") takes precedence so the prompt sticks.
+    const mcpApprovalState = getMcpToolApprovalState(toolName);
+    if (mcpApprovalState === "do_not_use") {
+      logger.info(`[PreToolUseHook] Blocking do_not_use MCP tool: ${toolName}`);
+      return {
+        continue: true,
+        hookSpecificOutput: {
+          hookEventName: "PreToolUse" as const,
+          permissionDecision: "deny" as const,
+          permissionDecisionReason:
+            "This tool has been blocked. To re-enable it, go to Settings > MCP Servers in PostHog Code.",
+        },
+      };
+    }
+    if (
+      mcpApprovalState === "needs_approval" &&
+      permissionCheck.decision !== "allow" &&
+      permissionCheck.decision !== "deny"
+    ) {
+      logger.info(
+        `[PreToolUseHook] Routing needs_approval MCP tool to canUseTool: ${toolName}`,
+      );
+      return {
+        continue: true,
+        hookSpecificOutput: {
+          hookEventName: "PreToolUse" as const,
+          permissionDecision: "ask" as const,
+          permissionDecisionReason:
+            "This MCP tool requires approval before it can be used.",
+        },
+      };
+    }
+
     // Defer destructive PostHog exec sub-tools to canUseTool so the
     // sub-tool gate can re-prompt. Returning `{ continue: true }` is
     // not enough — the SDK then falls back to its default permission

packages/agent/src/adapters/claude/mcp/tool-metadata.test.ts

@@ -1,8 +1,10 @@
 import { beforeEach, describe, expect, it } from "vitest";
 import {
+  clearMcpToolApprovalCache,
   clearMcpToolMetadataCache,
   getMcpToolApprovalState,
   getMcpToolMetadata,
+  getMcpToolMetadataKey,
   isMcpToolReadOnly,
   sanitizeMcpServerName,
   setMcpToolApprovalStates,
@@ -11,6 +13,7 @@ import {
 describe("tool-metadata approval states", () => {
   beforeEach(() => {
     clearMcpToolMetadataCache();
+    clearMcpToolApprovalCache();
   });
 
   describe("setMcpToolApprovalStates", () => {
@@ -56,12 +59,50 @@ describe("tool-metadata approval states", () => {
       expect(getMcpToolApprovalState("mcp__server__unknown")).toBeUndefined();
     });
 
+    it("normalizes MCP server names before looking up approval state", () => {
+      setMcpToolApprovalStates({
+        mcp__Granola_Meetings__query_granola_meetings: "needs_approval",
+      });
+
+      expect(
+        getMcpToolApprovalState(
+          "mcp__Granola Meetings__query_granola_meetings",
+        ),
+      ).toBe("needs_approval");
+    });
+
+    it("resolves a bare upstream tool name when it uniquely maps to an MCP Store tool", () => {
+      setMcpToolApprovalStates({
+        mcp__Granola_Meetings__query_granola_meetings: "needs_approval",
+      });
+
+      expect(getMcpToolApprovalState("query_granola_meetings")).toBe(
+        "needs_approval",
+      );
+      expect(getMcpToolMetadataKey("query_granola_meetings")).toBe(
+        "mcp__Granola_Meetings__query_granola_meetings",
+      );
+    });
+
     it("returns the correct state", () => {
       setMcpToolApprovalStates({
         mcp__s__t: "needs_approval",
       });
       expect(getMcpToolApprovalState("mcp__s__t")).toBe("needs_approval");
     });
+
+    it("survives a metadata cache clear (MCP server refresh/reconnect)", () => {
+      setMcpToolApprovalStates({
+        mcp__granola__query_granola_meetings: "needs_approval",
+      });
+
+      // A server reconnect wipes the metadata cache mid-session.
+      clearMcpToolMetadataCache();
+
+      expect(
+        getMcpToolApprovalState("mcp__granola__query_granola_meetings"),
+      ).toBe("needs_approval");
+    });
   });
 
   describe("isMcpToolReadOnly with approval states", () => {

packages/agent/src/adapters/claude/mcp/tool-metadata.ts

@@ -1,4 +1,11 @@
 import type { McpServerStatus, Query } from "@anthropic-ai/claude-agent-sdk";
+import {
+  buildMcpToolKey,
+  normalizeMcpToolKey,
+  parseMcpToolKey,
+  resolveMcpStoreToolKey,
+  sanitizeMcpServerName,
+} from "../../../mcp-store/tool-keys";
 import { Logger } from "../../../utils/logger";
 
 export type McpToolApprovalState = "approved" | "needs_approval" | "do_not_use";
@@ -9,22 +16,25 @@ export type McpToolApprovals = Record<string, McpToolApprovalState>;
 export interface McpToolMetadata {
   readOnly: boolean;
   name: string;
+  serverName?: string;
   description?: string;
   approvalState?: McpToolApprovalState;
 }
 
 const mcpToolMetadataCache: Map<string, McpToolMetadata> = new Map();
 
+// Per-tool approval state lives in its own store, keyed by normalized tool key.
+// It must outlive the metadata cache: `clearMcpToolMetadataCache()` runs on
+// every MCP server refresh/reconnect, and approval state is session config
+// (set once from the start payload), not volatile per-connection metadata.
+// Keeping them separate means a reconnect can't silently drop approvals and
+// let a needs_approval tool slip through the gate.
+const mcpToolApprovalCache: Map<string, McpToolApprovalState> = new Map();
+
 const PENDING_RETRY_INTERVAL_MS = 1_000;
 const PENDING_MAX_RETRIES = 10;
 
-export function sanitizeMcpServerName(name: string): string {
-  return name.replace(/[^a-zA-Z0-9_-]/g, "_");
-}
-
-function buildToolKey(serverName: string, toolName: string): string {
-  return `mcp__${serverName}__${toolName}`;
-}
+export { sanitizeMcpServerName };
 
 function delay(ms: number): Promise<void> {
   return new Promise((resolve) => setTimeout(resolve, ms));
@@ -56,15 +66,17 @@ export async function fetchMcpToolMetadata(
 
       let readOnlyCount = 0;
       for (const tool of server.tools) {
-        const toolKey = buildToolKey(server.name, tool.name);
+        const toolKey = buildMcpToolKey(server.name, tool.name);
         const readOnly = tool.annotations?.readOnly === true;
 
         const existing = mcpToolMetadataCache.get(toolKey);
         mcpToolMetadataCache.set(toolKey, {
           readOnly,
           name: tool.name,
+          serverName: server.name,
           description: tool.description,
-          approvalState: existing?.approvalState,
+          approvalState:
+            mcpToolApprovalCache.get(toolKey) ?? existing?.approvalState,
         });
         if (readOnly) readOnlyCount++;
       }
@@ -99,11 +111,24 @@ export async function fetchMcpToolMetadata(
 export function getMcpToolMetadata(
   toolName: string,
 ): McpToolMetadata | undefined {
-  return mcpToolMetadataCache.get(toolName);
+  const key = getMcpToolMetadataKey(toolName);
+  return key ? mcpToolMetadataCache.get(key) : undefined;
+}
+
+export function getMcpToolMetadataKey(toolName: string): string | undefined {
+  const resolvedKey = resolveMcpStoreToolKey(toolName, {
+    approvals: Object.fromEntries(
+      [...mcpToolMetadataCache.keys()].map((key) => [key, true]),
+    ),
+  });
+  const normalizedToolName = normalizeMcpToolKey(toolName);
+  if (mcpToolMetadataCache.has(toolName)) return toolName;
+  if (mcpToolMetadataCache.has(normalizedToolName)) return normalizedToolName;
+  return resolvedKey ?? undefined;
 }
 
 export function isMcpToolReadOnly(toolName: string): boolean {
-  const metadata = mcpToolMetadataCache.get(toolName);
+  const metadata = getMcpToolMetadata(toolName);
   return metadata?.readOnly === true;
 }
 
@@ -125,18 +150,41 @@ export function getCachedMcpTools(): McpToolMetadata[] {
 export function getMcpToolApprovalState(
   toolName: string,
 ): McpToolApprovalState | undefined {
-  return mcpToolMetadataCache.get(toolName)?.approvalState;
+  if (mcpToolApprovalCache.size === 0) {
+    return undefined;
+  }
+  // Direct/normalized hit first, then resolve server-name or title variants
+  // against the known approval keys. Reads the approval store (not the
+  // metadata cache) so it survives MCP metadata refreshes.
+  const direct =
+    mcpToolApprovalCache.get(toolName) ??
+    mcpToolApprovalCache.get(normalizeMcpToolKey(toolName));
+  if (direct) {
+    return direct;
+  }
+  const resolvedKey = resolveMcpStoreToolKey(toolName, {
+    approvals: Object.fromEntries(
+      [...mcpToolApprovalCache.keys()].map((key) => [key, true]),
+    ),
+  });
+  return resolvedKey ? mcpToolApprovalCache.get(resolvedKey) : undefined;
 }
 
 export function setMcpToolApprovalStates(approvals: McpToolApprovals): void {
   for (const [toolKey, approvalState] of Object.entries(approvals)) {
-    const existing = mcpToolMetadataCache.get(toolKey);
+    const normalizedToolKey = normalizeMcpToolKey(toolKey);
+    mcpToolApprovalCache.set(normalizedToolKey, approvalState);
+    // Mirror onto the metadata cache entry when present so UI/debug reads stay
+    // consistent; the approval store above remains the source of truth.
+    const existing = mcpToolMetadataCache.get(normalizedToolKey);
     if (existing) {
       existing.approvalState = approvalState;
     } else {
-      mcpToolMetadataCache.set(toolKey, {
+      const parsed = parseMcpToolKey(normalizedToolKey);
+      mcpToolMetadataCache.set(normalizedToolKey, {
         readOnly: false,
-        name: toolKey,
+        name: parsed?.toolName ?? normalizedToolKey,
+        serverName: parsed?.serverName,
         approvalState,
       });
     }
@@ -146,3 +194,9 @@ export function setMcpToolApprovalStates(approvals: McpToolApprovals): void {
 export function clearMcpToolMetadataCache(): void {
   mcpToolMetadataCache.clear();
 }
+
+/** Reset per-tool approval state. Approvals survive metadata refreshes, so
+ *  this is only for session teardown and tests — not the per-refresh clear. */
+export function clearMcpToolApprovalCache(): void {
+  mcpToolApprovalCache.clear();
+}