diff --git a/packages/provider-pi/src/server/mcpBridge.testkit.ts b/packages/provider-pi/src/server/mcpBridge.testkit.ts index 6061c2a3b556b97731a4d331ca83ec8841801ca5..afaa4888b320a3e0bce8ccc80af4989bb320f8e8 100644 --- a/packages/provider-pi/src/server/mcpBridge.testkit.ts +++ b/packages/provider-pi/src/server/mcpBridge.testkit.ts @@ -1,5 +1,6 @@ // @effect-diagnostics nodeBuiltinImport:off -- Executes the shipped Pi extension at its native JavaScript boundary. import * as NodeModule from "node:module"; +import * as NodePath from "node:path"; import * as NodeVM from "node:vm"; import { PI_T3_MCP_EXTENSION_SOURCE } from "./mcpExtensionSource.ts"; @@ -39,12 +40,14 @@ export 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; @@ -52,9 +55,17 @@ export 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, @@ -69,9 +80,15 @@ export 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, @@ -111,12 +128,14 @@ export 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) => @@ -129,6 +148,9 @@ export async function loadMcpBridge( return { handlers, tools, + setBridgeSourcePath: (path: string) => { + bridgeSourcePath = path; + }, requests, servers, transports, diff --git a/packages/provider-pi/src/server/mcpExtensionSource.test.ts b/packages/provider-pi/src/server/mcpExtensionSource.test.ts index e75eb2e0e81c0918d9d6feeff48596b740b7d5df..b8857d3c629cb1a9a5aa8f348cf5832bc34f7f8c 100644 --- a/packages/provider-pi/src/server/mcpExtensionSource.test.ts +++ b/packages/provider-pi/src/server/mcpExtensionSource.test.ts @@ -246,6 +246,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 feb091c51bd723260e1e74830a52b4f99116671a..7071b1ae6209f3d100d9807e45d529d7c6a9c80f 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"]); @@ -50,6 +52,7 @@ type McpTool = { readonly name: string; readonly description?: string; readonly inputSchema?: Record; + readonly annotations?: { readonly readOnlyHint?: boolean }; readonly outputSchema?: Record; }; @@ -306,10 +309,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)) { @@ -351,9 +365,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 45f8781a563554510ea0a89e627770c303d1d3d1..50d21afd32b4bdf83527e7079ec6e57312429c75 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 25e1d3ddbb98cc5168e7400413162e0b59cbb7f3..260b37642e91ad22242daae90e4a52b649d90e72 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]: