fix(mcp): improve read-only classification and approval prompt (#179)

* fix(mcp): improve read-only classification and approval prompt

Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>

* fix(test): remove unused MCP test import

Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>

* style(test): format tests/mcp.test.ts with Prettier

The previous commit shipped tests/mcp.test.ts unformatted, failing the
CI fmt:check (Lint & Test job). Run Prettier to bring it into line:
normalise quote style and expression wrapping. No behavioural change.

Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>

---------

Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>

Author: simongdavies

package-lock.json

@@ -1013,6 +1013,8 @@ const mcpWriteSafetyGate: WriteSafetyGate = async (
     ? C.err("⚠️  MCP DESTRUCTIVE operation")
     : C.warn("⚠️  MCP write operation");
 
+  spinner.stop();
+
   console.log();
   console.log(`  ${label}: ${C.label(serverName)}.${C.label(toolName)}`);
   if (argSummary) {

src/agent/index.ts

@@ -98,7 +98,7 @@ export function createMCPPluginAdapter(
           // Write-safety gate: check tools that are not known or inferred
           // read-only. The guest VM is paused during this check, so it is
           // safe to prompt the user.
-          if (gate && !isReadOnlyMCPTool(tool)) {
+          if (gate && !isReadOnlyMCPTool(tool, toolArgs)) {
             const allowed = await gate(
               conn.name,
               tool.name,

src/agent/mcp/plugin-adapter.ts

@@ -127,15 +127,54 @@ export function selectMCPTools(
   };
 }
 
-export function isReadOnlyMCPTool(tool: MCPToolSchema): boolean {
+export function isReadOnlyMCPTool(
+  tool: MCPToolSchema,
+  args?: Record<string, unknown>,
+): boolean {
   if (tool.annotations?.readOnlyHint === true) return true;
   if (tool.annotations?.destructiveHint === true) return false;
 
   const name = normaliseToolName(tool.name);
   if (WRITE_TOOL_PREFIXES.some((prefix) => name.startsWith(prefix))) {
     return false;
   }
-  return READ_ONLY_TOOL_PREFIXES.some((prefix) => name.startsWith(prefix));
+  if (READ_ONLY_TOOL_PREFIXES.some((prefix) => name.startsWith(prefix))) {
+    return true;
+  }
+
+  const operationHint = inferReadOnlyFromArgs(args);
+  if (operationHint !== null) return operationHint;
+
+  return false;
+}
+
+function inferReadOnlyFromArgs(
+  args: Record<string, unknown> | undefined,
+): boolean | null {
+  if (!args || typeof args !== "object") return null;
+
+  const candidates = [
+    args.operation,
+    args.action,
+    args.command,
+    args.method,
+    args.kind,
+    args.type,
+  ];
+
+  for (const candidate of candidates) {
+    if (typeof candidate !== "string") continue;
+
+    const value = normaliseToolName(candidate);
+    if (WRITE_TOOL_PREFIXES.some((prefix) => value.startsWith(prefix))) {
+      return false;
+    }
+    if (READ_ONLY_TOOL_PREFIXES.some((prefix) => value.startsWith(prefix))) {
+      return true;
+    }
+  }
+
+  return null;
 }
 
 export function getMCPToolSafety(tool: MCPToolSchema): MCPToolInfo["safety"] {

src/agent/mcp/tool-utils.ts

@@ -1,6 +1,8 @@
 // ── MCP integration tests ────────────────────────────────────────────
 
 import { describe, it, expect, beforeEach, vi } from "vitest";
+import { readFileSync } from "node:fs";
+import { join } from "node:path";
 import {
   parseMCPConfig,
   computeMCPConfigHash,
@@ -730,7 +732,6 @@ import {
   hasCachedTokens,
 } from "../src/agent/mcp/auth/token-cache.js";
 import { mkdirSync, writeFileSync, existsSync, rmSync } from "node:fs";
-import { join } from "node:path";
 import { homedir } from "node:os";
 import { tmpdir } from "node:os";
 
@@ -1050,6 +1051,55 @@ describe("MCP tool utility helpers", () => {
     expect(isReadOnlyMCPTool(tools[1])).toBe(true);
     expect(isReadOnlyMCPTool(tools[2])).toBe(false);
   });
+
+  it("keeps the module_info functionName schema valid for array inputs", () => {
+    const source = readFileSync(
+      join(import.meta.dirname, "..", "src", "agent", "module-info-schema.ts"),
+      "utf-8",
+    );
+
+    expect(source).toContain("anyOf: [");
+    expect(source).toContain('type: "array", items: { type: "string" }');
+  });
+
+  it("treats Power BI-style list operations as read-only from the request payload", () => {
+    const powerBiTool: MCPToolSchema = {
+      name: "connection_operations",
+      originalName: "connection_operations",
+      description: "Connect to Power BI models",
+      inputSchema: {
+        type: "object",
+        properties: {
+          operation: { type: "string" },
+        },
+      },
+    } as MCPToolSchema;
+
+    expect(
+      isReadOnlyMCPTool(powerBiTool, { operation: "ListLocalInstances" }),
+    ).toBe(true);
+    expect(isReadOnlyMCPTool(powerBiTool, { operation: "CreateModel" })).toBe(
+      false,
+    );
+  });
+
+  it("keeps write-prefixed tool names as write operations even with read-like payload hints", () => {
+    const deleteTool: MCPToolSchema = {
+      name: "delete_model",
+      originalName: "delete_model",
+      description: "Delete a Power BI model",
+      inputSchema: {
+        type: "object",
+        properties: {
+          operation: { type: "string" },
+        },
+      },
+    } as MCPToolSchema;
+
+    expect(
+      isReadOnlyMCPTool(deleteTool, { operation: "ListLocalInstances" }),
+    ).toBe(false);
+  });
 });
 
 describe("normaliseToolResult", () => {