From ac48563cdbc76f8d473fa23d520cefc883a6110f Mon Sep 17 00:00:00 2001 From: Wout Stiens <71498452+StiensWout@users.noreply.github.com> Date: Sat, 10 Oct 2026 16:40:58 +0200 Subject: [PATCH] fix(pi): allow known read-only T3 tools without approval --- .../src/server/mcpExtensionSource.test.ts | 79 +++++++++++++++++-- .../src/server/mcpExtensionSource.ts | 18 ++++- .../src/server/mcpInjection.test.ts | 7 ++ .../provider-pi/src/server/mcpInjection.ts | 5 ++ 4 files changed, 102 insertions(+), 7 deletions(-) diff --git a/packages/provider-pi/src/server/mcpExtensionSource.test.ts b/packages/provider-pi/src/server/mcpExtensionSource.test.ts index 25b040c2c002..f5fc5ec99f6d 100644 --- a/packages/provider-pi/src/server/mcpExtensionSource.test.ts +++ b/packages/provider-pi/src/server/mcpExtensionSource.test.ts @@ -74,12 +74,14 @@ async function loadMcpBridge( readonly toolSearchAvailable?: boolean; readonly toolSearchDisabled?: boolean; readonly allowsTool?: (name: string) => boolean; + readonly runtimeMode?: string; } = {}, ) { const handlers = new Map(); const tools: RegisteredTool[] = []; const requests: Array<{ readonly method: string; readonly params?: unknown }> = []; let activeTools = ["read"]; + let bridgeSourcePath = "/fixture/pi-t3-extension.ts"; const transports: Array<{ readonly url: string; readonly authorization: string; @@ -87,9 +89,17 @@ async function loadMcpBridge( }> = []; const servers: Array<{ readonly name: string; readonly config: Record }> = []; const catalog = [ - { name: "orchestrator_capabilities", description: "Discover available providers and models." }, + { + name: "orchestrator_capabilities", + description: "Discover available providers and models.", + annotations: { readOnlyHint: true }, + }, { name: "delegate_task", description: "Delegate work to another agent." }, - { name: "task_status", description: "Check delegated work." }, + { + name: "task_status", + description: "Check delegated work.", + annotations: { readOnlyHint: false }, + }, { name: "preview_snapshot", description: "Inspect the collaborative browser." }, ].map((tool) => ({ ...tool, inputSchema: { type: "object", properties: {} } })); const source = NodeModule.stripTypeScriptTypes( @@ -100,9 +110,15 @@ async function loadMcpBridge( ); await NodeVM.runInNewContext(`${source}\nt3McpExtension(pi)`, { process: { - env: { T3_MCP_URL: "http://fixture.invalid/mcp", T3_MCP_BEARER_TOKEN: "fixture-token" }, + env: { + T3_MCP_URL: "http://fixture.invalid/mcp", + T3_MCP_BEARER_TOKEN: "fixture-token", + T3_PI_MCP_EXTENSION_PATH: "/fixture/pi-t3-extension.ts", + T3_PI_RUNTIME_MODE: options.runtimeMode, + }, }, AbortSignal, + NodePath, Type: { Unsafe: (schema: unknown) => schema }, fetch: async ( url: string, @@ -137,12 +153,14 @@ async function loadMcpBridge( setActiveTools: (names: string[]) => { activeTools = names.filter((name) => options.allowsTool?.(name) ?? true); }, - getAllTools: () => - options.toolSearchAvailable && + getAllTools: () => [ + ...tools.map((tool) => ({ name: tool.name, sourceInfo: { path: bridgeSourcePath } })), + ...(options.toolSearchAvailable && !options.toolSearchDisabled && (options.allowsTool?.("tool_search") ?? true) ? [{ name: "tool_search", sourceInfo: { path: "builtin:tool-search" } }] - : [], + : []), + ], ...(options.modern ? { registerMcpServer: (name: string, config: Record) => @@ -155,6 +173,9 @@ async function loadMcpBridge( return { handlers, tools, + setBridgeSourcePath: (path: string) => { + bridgeSourcePath = path; + }, requests, servers, transports, @@ -301,6 +322,52 @@ describe("Pi MCP tool exposure", () => { }); describe("Pi tool discovery permissions", () => { + it.each(["approval-required", "auto-accept-edits", "auto"])( + "allows annotated T3 reads in %s while gating mutations and replacements", + async (runtimeMode) => { + const bridge = await loadMcpBridge({ modern: true, runtimeMode }); + const hook = bridge.handlers.get("tool_call") as unknown as ( + event: { toolName: string; input: unknown }, + ctx: { ui: { confirm: (title: string) => Promise } }, + ) => Promise<{ block: true; reason: string } | undefined>; + const confirmations: string[] = []; + const ctx = { + ui: { + confirm: async (title: string) => { + confirmations.push(title); + return false; + }, + }, + }; + for (const prefix of ["mcp__t3-code__", "mcp__t3_code__"]) { + assert.isUndefined( + await hook({ toolName: prefix + "orchestrator_capabilities", input: {} }, ctx), + ); + assert.equal( + (await hook({ toolName: prefix + "delegate_task", input: {} }, ctx))?.block, + true, + ); + // task_status acknowledges result delivery, and is intentionally not read-only. + assert.equal( + (await hook({ toolName: prefix + "task_status", input: {} }, ctx))?.block, + true, + ); + } + assert.equal(confirmations.length, 4); + bridge.setBridgeSourcePath("/user/extensions/replacement.ts"); + assert.equal( + (await hook({ toolName: "mcp__t3-code__orchestrator_capabilities", input: {} }, ctx)) + ?.block, + true, + ); + assert.equal( + (await hook({ toolName: "mcp__other__orchestrator_capabilities", input: {} }, ctx))?.block, + true, + ); + assert.equal(confirmations.length, 6); + }, + ); + it("allows discovery without confirmation and still gates the discovered tool", async () => { type ToolCallHook = ( event: { toolName: string; input: unknown }, diff --git a/packages/provider-pi/src/server/mcpExtensionSource.ts b/packages/provider-pi/src/server/mcpExtensionSource.ts index 8279a515ed65..6c7806fb5108 100644 --- a/packages/provider-pi/src/server/mcpExtensionSource.ts +++ b/packages/provider-pi/src/server/mcpExtensionSource.ts @@ -15,6 +15,7 @@ export const PI_T3_MCP_EXTENSION_FILENAME = "pi-t3-mcp-extension.ts"; export const T3_MCP_URL_ENV = "T3_MCP_URL"; export const T3_MCP_BEARER_ENV = "T3_MCP_BEARER_TOKEN"; export const T3_PI_RUNTIME_MODE_ENV = "T3_PI_RUNTIME_MODE"; +export const T3_PI_MCP_EXTENSION_PATH_ENV = "T3_PI_MCP_EXTENSION_PATH"; /** * Pi tools whose confirmations the bridge raises as file-change approvals. @@ -31,6 +32,7 @@ import { Type } from "typebox"; const URL_ENV = ${JSON.stringify(T3_MCP_URL_ENV)}; const TOKEN_ENV = ${JSON.stringify(T3_MCP_BEARER_ENV)}; const RUNTIME_MODE_ENV = ${JSON.stringify(T3_PI_RUNTIME_MODE_ENV)}; +const EXTENSION_PATH_ENV = ${JSON.stringify(T3_PI_MCP_EXTENSION_PATH_ENV)}; const ORCHESTRATION_INSTRUCTIONS = ${JSON.stringify(T3_CODE_ORCHESTRATION_INSTRUCTIONS.trim())}; const PROTOCOL = "2025-06-18"; const READ_ONLY_TOOLS = new Set(["read", "grep", "find", "ls"]); @@ -48,6 +50,7 @@ type McpTool = { readonly name: string; readonly description?: string; readonly inputSchema?: Record; + readonly annotations?: { readonly readOnlyHint?: boolean }; }; function env(name: string): string | undefined { @@ -274,10 +277,21 @@ export default async function t3McpExtension(pi: ExtensionAPI) { typeof pi.getAllTools === "function" && pi.getAllTools().some((tool) => tool.name === "tool_search" && tool.sourceInfo?.path === "builtin:tool-search"); + const readOnlyMcpTools = new Set(); + const isReadOnlyMcpTool = (name: string) => { + const extensionPath = env(EXTENSION_PATH_ENV); + if (extensionPath === undefined || !readOnlyMcpTools.has(name) || typeof pi.getAllTools !== "function") return false; + // A user extension may own the same name. Only our registered HTTP bridge + // may inherit the canonical T3 server's read-only annotation. + return pi.getAllTools().some((tool) => tool.name === name && + typeof tool.sourceInfo?.path === "string" && + NodePath.resolve(tool.sourceInfo.path) === NodePath.resolve(extensionPath)); + }; + pi.on("tool_call", async (event, ctx) => { const mode = runtimeMode(); if (mode === "full-access") return; - if (event.toolName === "tool_search" ? hasBuiltinToolSearch() : READ_ONLY_TOOLS.has(event.toolName)) { + if ((event.toolName === "tool_search" ? hasBuiltinToolSearch() : READ_ONLY_TOOLS.has(event.toolName)) || isReadOnlyMcpTool(event.toolName)) { return; } if (mode === "auto-accept-edits" && FILE_CHANGE_TOOLS.has(event.toolName)) { @@ -318,9 +332,11 @@ export default async function t3McpExtension(pi: ExtensionAPI) { // Preserve public names for saved loadouts and tool selectors. Hidden // canonical names reserve ownership against Pi's configured MCP servers. const prefixes = supportsExposure ? ["mcp__t3-code__", "mcp__t3_code__"] : ["mcp__t3-code__"]; + readOnlyMcpTools.clear(); for (const tool of catalog) { const name = tool.name; for (const prefix of prefixes) { + if (tool.annotations?.readOnlyHint === true) readOnlyMcpTools.add(\`\${prefix}\${name}\`); const exposure = prefix === "mcp__t3_code__" ? "hidden" : deferOptionalTools && !directTools.has(name) ? "deferred" : "direct"; pi.registerTool({ diff --git a/packages/provider-pi/src/server/mcpInjection.test.ts b/packages/provider-pi/src/server/mcpInjection.test.ts index 45f8781a5635..50d21afd32b4 100644 --- a/packages/provider-pi/src/server/mcpInjection.test.ts +++ b/packages/provider-pi/src/server/mcpInjection.test.ts @@ -9,6 +9,7 @@ import { T3_MCP_BEARER_ENV, T3_MCP_URL_ENV, T3_PI_RUNTIME_MODE_ENV, + T3_PI_MCP_EXTENSION_PATH_ENV, } from "./mcpExtensionSource.ts"; import { buildPiRpcLaunch, @@ -65,12 +66,14 @@ describe("pi T3 MCP injection", () => { assert.equal(launch.env[T3_MCP_URL_ENV], "http://127.0.0.1:43123/mcp"); assert.equal(launch.env[T3_MCP_BEARER_ENV], "secret-pi-token"); assert.equal(launch.env[T3_PI_RUNTIME_MODE_ENV], "approval-required"); + assert.equal(launch.env[T3_PI_MCP_EXTENSION_PATH_ENV], "/tmp/cache/pi-t3-mcp-extension.ts"); const permissionOnly = buildPiRpcLaunch({ launchArgs: [], environment: { [T3_MCP_URL_ENV]: "http://127.0.0.1:9999/stale", [T3_MCP_BEARER_ENV]: "stale-token", + [T3_PI_MCP_EXTENSION_PATH_ENV]: "/stale/extension.ts", }, mcpSession: undefined, extensionPath: "/tmp/cache/pi-t3-mcp-extension.ts", @@ -86,6 +89,10 @@ describe("pi T3 MCP injection", () => { assert.isUndefined(permissionOnly.env[T3_MCP_URL_ENV]); assert.isUndefined(permissionOnly.env[T3_MCP_BEARER_ENV]); assert.equal(permissionOnly.env[T3_PI_RUNTIME_MODE_ENV], "auto-accept-edits"); + assert.equal( + permissionOnly.env[T3_PI_MCP_EXTENSION_PATH_ENV], + "/tmp/cache/pi-t3-mcp-extension.ts", + ); }); it("falls back to Pi's first supported mode for legacy auto threads", () => { diff --git a/packages/provider-pi/src/server/mcpInjection.ts b/packages/provider-pi/src/server/mcpInjection.ts index 25e1d3ddbb98..260b37642e91 100644 --- a/packages/provider-pi/src/server/mcpInjection.ts +++ b/packages/provider-pi/src/server/mcpInjection.ts @@ -9,6 +9,7 @@ import { T3_MCP_BEARER_ENV, T3_MCP_URL_ENV, T3_PI_RUNTIME_MODE_ENV, + T3_PI_MCP_EXTENSION_PATH_ENV, } from "./mcpExtensionSource.ts"; const RESERVED_PI_LAUNCH_ARGUMENTS = new Set([ @@ -291,11 +292,15 @@ export function buildPiRpcLaunch(input: { // credentials inherited from the server or a parent provider process. delete environment[T3_MCP_URL_ENV]; delete environment[T3_MCP_BEARER_ENV]; + delete environment[T3_PI_MCP_EXTENSION_PATH_ENV]; return { args, env: { ...environment, + ...(hasT3Extension && input.extensionPath !== undefined + ? { [T3_PI_MCP_EXTENSION_PATH_ENV]: input.extensionPath } + : {}), ...(hasT3Extension && input.runtimeMode !== undefined ? { [T3_PI_RUNTIME_MODE_ENV]: