authorgravatar for 71498452+StiensWout@users.noreply.github.comWout Stiens <71498452+StiensWout@users.noreply.github.com> 2026-10-11 01:19:04+02:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-10-10 18:19:04-05:00
log5f7294d452a5c6d6e0b7bd2c992478f830fbe212
treea0c5716de675b87944442d54d48640f0901a14cc
parent953edeec9dcddd7afc582926eb1b65cfd82dc784
signature Signed by PGP key B5690EEEBB952194

fix(pi): allow known read-only T3 tools without approval (#17852)

Co-authored-by: Yash Singh <saiansh2525@gmail.com>

5 files changed, 103 insertions(+), 7 deletions(-)

packages/provider-pi/src/server/mcpBridge.testkit.ts+28-6
...@@ -1,5 +1,6 @@...@@ -1,5 +1,6 @@
1// @effect-diagnostics nodeBuiltinImport:off -- Executes the shipped Pi extension at its native JavaScript boundary.1// @effect-diagnostics nodeBuiltinImport:off -- Executes the shipped Pi extension at its native JavaScript boundary.
2import * as NodeModule from "node:module";2import * as NodeModule from "node:module";
3import * as NodePath from "node:path";
3import * as NodeVM from "node:vm";4import * as NodeVM from "node:vm";
4import { PI_T3_MCP_EXTENSION_SOURCE } from "./mcpExtensionSource.ts";5import { PI_T3_MCP_EXTENSION_SOURCE } from "./mcpExtensionSource.ts";
56
...@@ -39,12 +40,14 @@ export async function loadMcpBridge(...@@ -39,12 +40,14 @@ export async function loadMcpBridge(
39 readonly toolSearchAvailable?: boolean;40 readonly toolSearchAvailable?: boolean;
40 readonly toolSearchDisabled?: boolean;41 readonly toolSearchDisabled?: boolean;
41 readonly allowsTool?: (name: string) => boolean;42 readonly allowsTool?: (name: string) => boolean;
43 readonly runtimeMode?: string;
42 } = {},44 } = {},
43) {45) {
44 const handlers = new Map<string, AgentStartHook>();46 const handlers = new Map<string, AgentStartHook>();
45 const tools: RegisteredTool[] = [];47 const tools: RegisteredTool[] = [];
46 const requests: Array<{ readonly method: string; readonly params?: unknown }> = [];48 const requests: Array<{ readonly method: string; readonly params?: unknown }> = [];
47 let activeTools = ["read"];49 let activeTools = ["read"];
50 let bridgeSourcePath = "/fixture/pi-t3-extension.ts";
48 const transports: Array<{51 const transports: Array<{
49 readonly url: string;52 readonly url: string;
50 readonly authorization: string;53 readonly authorization: string;
...@@ -52,9 +55,17 @@ export async function loadMcpBridge(...@@ -52,9 +55,17 @@ export async function loadMcpBridge(
52 }> = [];55 }> = [];
53 const servers: Array<{ readonly name: string; readonly config: Record<string, unknown> }> = [];56 const servers: Array<{ readonly name: string; readonly config: Record<string, unknown> }> = [];
54 const catalog = [57 const catalog = [
55 { name: "orchestrator_capabilities", description: "Discover available providers and models." },58 {
59 name: "orchestrator_capabilities",
60 description: "Discover available providers and models.",
61 annotations: { readOnlyHint: true },
62 },
56 { name: "delegate_task", description: "Delegate work to another agent." },63 { name: "delegate_task", description: "Delegate work to another agent." },
57 { name: "task_status", description: "Check delegated work." },64 {
65 name: "task_status",
66 description: "Check delegated work.",
67 annotations: { readOnlyHint: false },
68 },
58 { name: "preview_snapshot", description: "Inspect the collaborative browser." },69 { name: "preview_snapshot", description: "Inspect the collaborative browser." },
59 ].map((tool) => ({70 ].map((tool) => ({
60 ...tool,71 ...tool,
...@@ -69,9 +80,15 @@ export async function loadMcpBridge(...@@ -69,9 +80,15 @@ export async function loadMcpBridge(
69 );80 );
70 await NodeVM.runInNewContext(`${source}\nt3McpExtension(pi)`, {81 await NodeVM.runInNewContext(`${source}\nt3McpExtension(pi)`, {
71 process: {82 process: {
72 env: { T3_MCP_URL: "http://fixture.invalid/mcp", T3_MCP_BEARER_TOKEN: "fixture-token" },83 env: {
84 T3_MCP_URL: "http://fixture.invalid/mcp",
85 T3_MCP_BEARER_TOKEN: "fixture-token",
86 T3_PI_MCP_EXTENSION_PATH: "/fixture/pi-t3-extension.ts",
87 T3_PI_RUNTIME_MODE: options.runtimeMode,
88 },
73 },89 },
74 AbortSignal,90 AbortSignal,
91 NodePath,
75 Type: { Unsafe: (schema: unknown) => schema },92 Type: { Unsafe: (schema: unknown) => schema },
76 fetch: async (93 fetch: async (
77 url: string,94 url: string,
...@@ -111,12 +128,14 @@ export async function loadMcpBridge(...@@ -111,12 +128,14 @@ export async function loadMcpBridge(
111 setActiveTools: (names: string[]) => {128 setActiveTools: (names: string[]) => {
112 activeTools = names.filter((name) => options.allowsTool?.(name) ?? true);129 activeTools = names.filter((name) => options.allowsTool?.(name) ?? true);
113 },130 },
114 getAllTools: () =>131 getAllTools: () => [
115 options.toolSearchAvailable &&132 ...tools.map((tool) => ({ name: tool.name, sourceInfo: { path: bridgeSourcePath } })),
133 ...(options.toolSearchAvailable &&
116 !options.toolSearchDisabled &&134 !options.toolSearchDisabled &&
117 (options.allowsTool?.("tool_search") ?? true)135 (options.allowsTool?.("tool_search") ?? true)
118 ? [{ name: "tool_search", sourceInfo: { path: "builtin:tool-search" } }]136 ? [{ name: "tool_search", sourceInfo: { path: "builtin:tool-search" } }]
119 : [],137 : []),
138 ],
120 ...(options.modern139 ...(options.modern
121 ? {140 ? {
122 registerMcpServer: (name: string, config: Record<string, unknown>) =>141 registerMcpServer: (name: string, config: Record<string, unknown>) =>
...@@ -129,6 +148,9 @@ export async function loadMcpBridge(...@@ -129,6 +148,9 @@ export async function loadMcpBridge(
129 return {148 return {
130 handlers,149 handlers,
131 tools,150 tools,
151 setBridgeSourcePath: (path: string) => {
152 bridgeSourcePath = path;
153 },
132 requests,154 requests,
133 servers,155 servers,
134 transports,156 transports,
packages/provider-pi/src/server/mcpExtensionSource.test.ts+46
...@@ -246,6 +246,52 @@ describe("Pi MCP tool exposure", () => {...@@ -246,6 +246,52 @@ describe("Pi MCP tool exposure", () => {
246});246});
247247
248describe("Pi tool discovery permissions", () => {248describe("Pi tool discovery permissions", () => {
249 it.each(["approval-required", "auto-accept-edits", "auto"])(
250 "allows annotated T3 reads in %s while gating mutations and replacements",
251 async (runtimeMode) => {
252 const bridge = await loadMcpBridge({ modern: true, runtimeMode });
253 const hook = bridge.handlers.get("tool_call") as unknown as (
254 event: { toolName: string; input: unknown },
255 ctx: { ui: { confirm: (title: string) => Promise<boolean> } },
256 ) => Promise<{ block: true; reason: string } | undefined>;
257 const confirmations: string[] = [];
258 const ctx = {
259 ui: {
260 confirm: async (title: string) => {
261 confirmations.push(title);
262 return false;
263 },
264 },
265 };
266 for (const prefix of ["mcp__t3-code__", "mcp__t3_code__"]) {
267 assert.isUndefined(
268 await hook({ toolName: prefix + "orchestrator_capabilities", input: {} }, ctx),
269 );
270 assert.equal(
271 (await hook({ toolName: prefix + "delegate_task", input: {} }, ctx))?.block,
272 true,
273 );
274 // task_status acknowledges result delivery, and is intentionally not read-only.
275 assert.equal(
276 (await hook({ toolName: prefix + "task_status", input: {} }, ctx))?.block,
277 true,
278 );
279 }
280 assert.equal(confirmations.length, 4);
281 bridge.setBridgeSourcePath("/user/extensions/replacement.ts");
282 assert.equal(
283 (await hook({ toolName: "mcp__t3-code__orchestrator_capabilities", input: {} }, ctx))
284 ?.block,
285 true,
286 );
287 assert.equal(
288 (await hook({ toolName: "mcp__other__orchestrator_capabilities", input: {} }, ctx))?.block,
289 true,
290 );
291 assert.equal(confirmations.length, 6);
292 },
293 );
294
249 it("allows discovery without confirmation and still gates the discovered tool", async () => {295 it("allows discovery without confirmation and still gates the discovered tool", async () => {
250 type ToolCallHook = (296 type ToolCallHook = (
251 event: { toolName: string; input: unknown },297 event: { toolName: string; input: unknown },
packages/provider-pi/src/server/mcpExtensionSource.ts+17-1
...@@ -15,6 +15,7 @@ export const PI_T3_MCP_EXTENSION_FILENAME = "pi-t3-mcp-extension.ts";...@@ -15,6 +15,7 @@ export const PI_T3_MCP_EXTENSION_FILENAME = "pi-t3-mcp-extension.ts";
15export const T3_MCP_URL_ENV = "T3_MCP_URL";15export const T3_MCP_URL_ENV = "T3_MCP_URL";
16export const T3_MCP_BEARER_ENV = "T3_MCP_BEARER_TOKEN";16export const T3_MCP_BEARER_ENV = "T3_MCP_BEARER_TOKEN";
17export const T3_PI_RUNTIME_MODE_ENV = "T3_PI_RUNTIME_MODE";17export const T3_PI_RUNTIME_MODE_ENV = "T3_PI_RUNTIME_MODE";
18export const T3_PI_MCP_EXTENSION_PATH_ENV = "T3_PI_MCP_EXTENSION_PATH";
1819
19/**20/**
20 * Pi tools whose confirmations the bridge raises as file-change approvals.21 * Pi tools whose confirmations the bridge raises as file-change approvals.
...@@ -31,6 +32,7 @@ import { Type } from "typebox";...@@ -31,6 +32,7 @@ import { Type } from "typebox";
31const URL_ENV = ${JSON.stringify(T3_MCP_URL_ENV)};32const URL_ENV = ${JSON.stringify(T3_MCP_URL_ENV)};
32const TOKEN_ENV = ${JSON.stringify(T3_MCP_BEARER_ENV)};33const TOKEN_ENV = ${JSON.stringify(T3_MCP_BEARER_ENV)};
33const RUNTIME_MODE_ENV = ${JSON.stringify(T3_PI_RUNTIME_MODE_ENV)};34const RUNTIME_MODE_ENV = ${JSON.stringify(T3_PI_RUNTIME_MODE_ENV)};
35const EXTENSION_PATH_ENV = ${JSON.stringify(T3_PI_MCP_EXTENSION_PATH_ENV)};
34const ORCHESTRATION_INSTRUCTIONS = ${JSON.stringify(T3_CODE_ORCHESTRATION_INSTRUCTIONS.trim())};36const ORCHESTRATION_INSTRUCTIONS = ${JSON.stringify(T3_CODE_ORCHESTRATION_INSTRUCTIONS.trim())};
35const PROTOCOL = "2025-06-18";37const PROTOCOL = "2025-06-18";
36const READ_ONLY_TOOLS = new Set(["read", "grep", "find", "ls"]);38const READ_ONLY_TOOLS = new Set(["read", "grep", "find", "ls"]);
...@@ -50,6 +52,7 @@ type McpTool = {...@@ -50,6 +52,7 @@ type McpTool = {
50 readonly name: string;52 readonly name: string;
51 readonly description?: string;53 readonly description?: string;
52 readonly inputSchema?: Record<string, unknown>;54 readonly inputSchema?: Record<string, unknown>;
55 readonly annotations?: { readonly readOnlyHint?: boolean };
53 readonly outputSchema?: Record<string, unknown>;56 readonly outputSchema?: Record<string, unknown>;
54};57};
5558
...@@ -306,10 +309,21 @@ export default async function t3McpExtension(pi: ExtensionAPI) {...@@ -306,10 +309,21 @@ export default async function t3McpExtension(pi: ExtensionAPI) {
306 typeof pi.getAllTools === "function" &&309 typeof pi.getAllTools === "function" &&
307 pi.getAllTools().some((tool) => tool.name === "tool_search" && tool.sourceInfo?.path === "builtin:tool-search");310 pi.getAllTools().some((tool) => tool.name === "tool_search" && tool.sourceInfo?.path === "builtin:tool-search");
308311
312 const readOnlyMcpTools = new Set<string>();
313 const isReadOnlyMcpTool = (name: string) => {
314 const extensionPath = env(EXTENSION_PATH_ENV);
315 if (extensionPath === undefined || !readOnlyMcpTools.has(name) || typeof pi.getAllTools !== "function") return false;
316 // A user extension may own the same name. Only our registered HTTP bridge
317 // may inherit the canonical T3 server's read-only annotation.
318 return pi.getAllTools().some((tool) => tool.name === name &&
319 typeof tool.sourceInfo?.path === "string" &&
320 NodePath.resolve(tool.sourceInfo.path) === NodePath.resolve(extensionPath));
321 };
322
309 pi.on("tool_call", async (event, ctx) => {323 pi.on("tool_call", async (event, ctx) => {
310 const mode = runtimeMode();324 const mode = runtimeMode();
311 if (mode === "full-access") return;325 if (mode === "full-access") return;
312 if (event.toolName === "tool_search" ? hasBuiltinToolSearch() : READ_ONLY_TOOLS.has(event.toolName)) {326 if ((event.toolName === "tool_search" ? hasBuiltinToolSearch() : READ_ONLY_TOOLS.has(event.toolName)) || isReadOnlyMcpTool(event.toolName)) {
313 return;327 return;
314 }328 }
315 if (mode === "auto-accept-edits" && FILE_CHANGE_TOOLS.has(event.toolName)) {329 if (mode === "auto-accept-edits" && FILE_CHANGE_TOOLS.has(event.toolName)) {
...@@ -351,9 +365,11 @@ export default async function t3McpExtension(pi: ExtensionAPI) {...@@ -351,9 +365,11 @@ export default async function t3McpExtension(pi: ExtensionAPI) {
351 // Preserve public names for saved loadouts and tool selectors. Hidden365 // Preserve public names for saved loadouts and tool selectors. Hidden
352 // canonical names reserve ownership against Pi's configured MCP servers.366 // canonical names reserve ownership against Pi's configured MCP servers.
353 const prefixes = supportsExposure ? ["mcp__t3-code__", "mcp__t3_code__"] : ["mcp__t3-code__"];367 const prefixes = supportsExposure ? ["mcp__t3-code__", "mcp__t3_code__"] : ["mcp__t3-code__"];
368 readOnlyMcpTools.clear();
354 for (const tool of catalog) {369 for (const tool of catalog) {
355 const name = tool.name;370 const name = tool.name;
356 for (const prefix of prefixes) {371 for (const prefix of prefixes) {
372 if (tool.annotations?.readOnlyHint === true) readOnlyMcpTools.add(\`\${prefix}\${name}\`);
357 const exposure = prefix === "mcp__t3_code__" ? "hidden" :373 const exposure = prefix === "mcp__t3_code__" ? "hidden" :
358 deferOptionalTools && !directTools.has(name) ? "deferred" : "direct";374 deferOptionalTools && !directTools.has(name) ? "deferred" : "direct";
359 pi.registerTool({375 pi.registerTool({
packages/provider-pi/src/server/mcpInjection.test.ts+7
...@@ -9,6 +9,7 @@ import {...@@ -9,6 +9,7 @@ import {
9 T3_MCP_BEARER_ENV,9 T3_MCP_BEARER_ENV,
10 T3_MCP_URL_ENV,10 T3_MCP_URL_ENV,
11 T3_PI_RUNTIME_MODE_ENV,11 T3_PI_RUNTIME_MODE_ENV,
12 T3_PI_MCP_EXTENSION_PATH_ENV,
12} from "./mcpExtensionSource.ts";13} from "./mcpExtensionSource.ts";
13import {14import {
14 buildPiRpcLaunch,15 buildPiRpcLaunch,
...@@ -65,12 +66,14 @@ describe("pi T3 MCP injection", () => {...@@ -65,12 +66,14 @@ describe("pi T3 MCP injection", () => {
65 assert.equal(launch.env[T3_MCP_URL_ENV], "http://127.0.0.1:43123/mcp");66 assert.equal(launch.env[T3_MCP_URL_ENV], "http://127.0.0.1:43123/mcp");
66 assert.equal(launch.env[T3_MCP_BEARER_ENV], "secret-pi-token");67 assert.equal(launch.env[T3_MCP_BEARER_ENV], "secret-pi-token");
67 assert.equal(launch.env[T3_PI_RUNTIME_MODE_ENV], "approval-required");68 assert.equal(launch.env[T3_PI_RUNTIME_MODE_ENV], "approval-required");
69 assert.equal(launch.env[T3_PI_MCP_EXTENSION_PATH_ENV], "/tmp/cache/pi-t3-mcp-extension.ts");
6870
69 const permissionOnly = buildPiRpcLaunch({71 const permissionOnly = buildPiRpcLaunch({
70 launchArgs: [],72 launchArgs: [],
71 environment: {73 environment: {
72 [T3_MCP_URL_ENV]: "http://127.0.0.1:9999/stale",74 [T3_MCP_URL_ENV]: "http://127.0.0.1:9999/stale",
73 [T3_MCP_BEARER_ENV]: "stale-token",75 [T3_MCP_BEARER_ENV]: "stale-token",
76 [T3_PI_MCP_EXTENSION_PATH_ENV]: "/stale/extension.ts",
74 },77 },
75 mcpSession: undefined,78 mcpSession: undefined,
76 extensionPath: "/tmp/cache/pi-t3-mcp-extension.ts",79 extensionPath: "/tmp/cache/pi-t3-mcp-extension.ts",
...@@ -86,6 +89,10 @@ describe("pi T3 MCP injection", () => {...@@ -86,6 +89,10 @@ describe("pi T3 MCP injection", () => {
86 assert.isUndefined(permissionOnly.env[T3_MCP_URL_ENV]);89 assert.isUndefined(permissionOnly.env[T3_MCP_URL_ENV]);
87 assert.isUndefined(permissionOnly.env[T3_MCP_BEARER_ENV]);90 assert.isUndefined(permissionOnly.env[T3_MCP_BEARER_ENV]);
88 assert.equal(permissionOnly.env[T3_PI_RUNTIME_MODE_ENV], "auto-accept-edits");91 assert.equal(permissionOnly.env[T3_PI_RUNTIME_MODE_ENV], "auto-accept-edits");
92 assert.equal(
93 permissionOnly.env[T3_PI_MCP_EXTENSION_PATH_ENV],
94 "/tmp/cache/pi-t3-mcp-extension.ts",
95 );
89 });96 });
9097
91 it("falls back to Pi's first supported mode for legacy auto threads", () => {98 it("falls back to Pi's first supported mode for legacy auto threads", () => {
packages/provider-pi/src/server/mcpInjection.ts+5
...@@ -9,6 +9,7 @@ import {...@@ -9,6 +9,7 @@ import {
9 T3_MCP_BEARER_ENV,9 T3_MCP_BEARER_ENV,
10 T3_MCP_URL_ENV,10 T3_MCP_URL_ENV,
11 T3_PI_RUNTIME_MODE_ENV,11 T3_PI_RUNTIME_MODE_ENV,
12 T3_PI_MCP_EXTENSION_PATH_ENV,
12} from "./mcpExtensionSource.ts";13} from "./mcpExtensionSource.ts";
1314
14const RESERVED_PI_LAUNCH_ARGUMENTS = new Set([15const RESERVED_PI_LAUNCH_ARGUMENTS = new Set([
...@@ -291,11 +292,15 @@ export function buildPiRpcLaunch(input: {...@@ -291,11 +292,15 @@ export function buildPiRpcLaunch(input: {
291 // credentials inherited from the server or a parent provider process.292 // credentials inherited from the server or a parent provider process.
292 delete environment[T3_MCP_URL_ENV];293 delete environment[T3_MCP_URL_ENV];
293 delete environment[T3_MCP_BEARER_ENV];294 delete environment[T3_MCP_BEARER_ENV];
295 delete environment[T3_PI_MCP_EXTENSION_PATH_ENV];
294296
295 return {297 return {
296 args,298 args,
297 env: {299 env: {
298 ...environment,300 ...environment,
301 ...(hasT3Extension && input.extensionPath !== undefined
302 ? { [T3_PI_MCP_EXTENSION_PATH_ENV]: input.extensionPath }
303 : {}),
299 ...(hasT3Extension && input.runtimeMode !== undefined304 ...(hasT3Extension && input.runtimeMode !== undefined
300 ? {305 ? {
301 [T3_PI_RUNTIME_MODE_ENV]:306 [T3_PI_RUNTIME_MODE_ENV]: