diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 17f446908..22d4c9b1d 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,11 +2,17 @@ ## [Unreleased] +### Added + +- Added `*` patterns to `--tools` and `--exclude-tools`, for example `--tools read,codemode,'mcp__radius__*'` +- Added `--no-mcp` to disable the built-in MCP support for one run + ### Fixed - Fixed syntax highlighting losing colors after the first line of multiline strings and comments in fenced code blocks ([#10143](https://github.com/earendil-works/pi/issues/10143)) - Fixed codemode scripts not receiving images from `read`: `tools.read()` now resolves to an image block for image files, which `image()` shows ([#10251](https://github.com/earendil-works/pi/issues/10251)) - Fixed MCP OAuth sign-in failing with `invalid_redirect_uri` on servers with OpenID Connect client registration, such as `mcp.modem.dev`: pi now registers as a native client ([#10493](https://github.com/earendil-works/pi/issues/10493)) +- Fixed `--tools` removing MCP tools, which left `pi --tools codemode` without any MCP servers. `--tools` now keeps MCP tools unless an entry starts with `mcp__` ## [1.0.3] - 2026-10-05 diff --git a/packages/coding-agent/docs/cli.md b/packages/coding-agent/docs/cli.md index fc095e78d..4550a3feb 100644 --- a/packages/coding-agent/docs/cli.md +++ b/packages/coding-agent/docs/cli.md @@ -117,16 +117,26 @@ pi --tools read,grep,find,ls --print "Review this project" See [Settings](settings.md#tools) for configuring the default tool selection. - `-t`, `--tools `
- Replaces the default selection with a comma-separated allowlist of built-in, extension, or custom tools. + Replaces the default selection with a comma-separated allowlist of built-in, extension, or custom tools. Entries are tool names or patterns where `*` matches any characters. MCP tools are kept unless an entry starts with `mcp__` (see [MCP tools](#mcp-tools)). - `-xt`, `--exclude-tools `
- Disables comma-separated tool names after all other selection options. + Disables comma-separated tool names or patterns after all other selection options, MCP tools included. - `-nbt`, `--no-builtin-tools`
Disables default built-in tools while retaining extension and custom tools. - `-nt`, `--no-tools`
- Starts with all built-in, extension, and custom tools disabled. + Starts with all built-in, extension, custom, and MCP tools disabled. Default enabled tools are `read`, `bash`, `edit`, and `write`, unless `defaultTools` changes them. `--tools` replaces the whole selection, so name every tool you want; `defaultTools` also accepts `+name` and `-name` to change the defaults instead. + + +`--tools` selects the tools declared to the model. It does not remove MCP tools, whose reach is set by their [exposure](mcp.md#control-tool-exposure): `pi --tools read,codemode` keeps every MCP tool callable from codemode scripts. An MCP tool that no entry names or matches is never declared directly, whatever its exposure; only `tool_search`, if listed, can load it. Once an entry starts with `mcp__`, `--tools` filters MCP tools too, so this keeps only the tools of the `radius` server: + +```sh +pi --tools read,bash,codemode,'mcp__radius__*' +``` + +The MCP resource tools (`list_mcp_resources`, `list_mcp_resource_templates`, `read_mcp_resource`) count as MCP tools. To remove MCP tools, use `--exclude-tools 'mcp__*'` or [`--no-mcp`](#resource-options). + | Built-in | Purpose | |---|---| | `read` | Read text files and supported images | @@ -185,6 +195,8 @@ See [Configuration](configuration.md) for conventional directories and project t Loads an extension file or directory, or a built-in extension such as `builtin:mcp`, and is repeatable. - `-ne`, `--no-extensions`
Disables discovered, configured, and built-in extensions. Explicit `-e` paths still load, so `pi -ne -e builtin:mcp` keeps only the built-in MCP support. +- `--no-mcp`
+ Disables the built-in MCP support for this run: no servers connect, and there are no MCP tools or `/mcp`. It does not affect an extension that replaces the built-in MCP support. - `--skill `
Loads a skill file or directory and is repeatable. - `-ns`, `--no-skills`
diff --git a/packages/coding-agent/docs/mcp.md b/packages/coding-agent/docs/mcp.md index 0d0116141..c6e160444 100644 --- a/packages/coding-agent/docs/mcp.md +++ b/packages/coding-agent/docs/mcp.md @@ -225,6 +225,8 @@ Pi activates `codemode` when a server with `codemode` exposure connects. It acti Tools with `codemode` or `deferred` exposure can be reached through either indirect mechanism: codemode scripts can call them, and `tool_search` can load them. Codemode calls do not depend on the active tool set, so they remain available after `/tree`, resume, and fork. Tools loaded by `tool_search` are recorded in the transcript and remain declared on that branch. +`--tools` does not remove MCP tools unless one of its entries starts with `mcp__`; `pi --tools read,codemode,'mcp__radius__*'` keeps only the tools of `radius`. `--exclude-tools` accepts the same patterns, and `--no-mcp` disables MCP for one run (see [Tools](cli.md#mcp-tools)). + To keep `codemode` active without MCP servers, add `"defaultTools": ["+codemode"]` to [settings](settings.md#tools). To prevent automatic codemode activation, set `"autoEnableCodemode": false` beside `mcpServers`. A project value overrides the user-level value. Pi warns once when neither `codemode` nor `tool_search` is active and non-direct tools cannot be called. Text results over 20 KB reach the model with their middle removed around a `…N chars truncated…` marker. The full text is saved to a temporary file named in the result. Codemode scripts receive the complete result and can reduce it before returning output to the model. @@ -261,7 +263,7 @@ Changes to enabled state or exposure apply only to the current session. A file-c ### Replace the built-in MCP support -An installed extension that registers `/mcp`, such as `pi-mcp-adapter`, replaces the built-in MCP support for sessions. Pi then does not read `mcp.json` or connect its servers in a session, and `/mcp` belongs to the extension. Remove the extension to restore the built-in behavior. To disable built-in MCP support without a replacement, disable `mcp` under Built-in in `pi config`, or set `"extensions": ["-builtin:mcp"]` in [settings](settings.md#resources). +An installed extension that registers `/mcp`, such as `pi-mcp-adapter`, replaces the built-in MCP support for sessions. Pi then does not read `mcp.json` or connect its servers in a session, and `/mcp` belongs to the extension. Remove the extension to restore the built-in behavior. To disable built-in MCP support without a replacement, disable `mcp` under Built-in in `pi config`, or set `"extensions": ["-builtin:mcp"]` in [settings](settings.md#resources). `--no-mcp` disables it for one run. An extension that registers `codemode` or `tool_search` similarly replaces the built-in tool with that name. Shell-level `pi mcp` commands always use the built-in implementation. diff --git a/packages/coding-agent/src/cli/args.ts b/packages/coding-agent/src/cli/args.ts index 9461c3e21..c994db011 100644 --- a/packages/coding-agent/src/cli/args.ts +++ b/packages/coding-agent/src/cli/args.ts @@ -35,6 +35,7 @@ export interface Args { noBuiltinTools?: boolean; extensions?: string[]; noExtensions?: boolean; + noMcp?: boolean; print?: boolean; export?: string; noSkills?: boolean; @@ -181,6 +182,8 @@ export function parseArgs(args: string[]): Args { result.extensions.push(args[++i]); } else if (arg === "--no-extensions" || arg === "-ne") { result.noExtensions = true; + } else if (arg === "--no-mcp") { + result.noMcp = true; } else if (arg === "--skill" && i + 1 < args.length) { result.skills = result.skills ?? []; result.skills.push(args[++i]); @@ -308,13 +311,14 @@ ${chalk.bold("Options:")} Supports globs (anthropic/*, *sonnet*) and fuzzy matching --no-tools, -nt Disable all tools by default (built-in and extension) --no-builtin-tools, -nbt Disable built-in tools by default but keep extension/custom tools enabled - --tools, -t Comma-separated allowlist of tool names to enable - Applies to built-in, extension, and custom tools - --exclude-tools, -xt Comma-separated denylist of tool names to disable - Applies to built-in, extension, and custom tools + --tools, -t Comma-separated allowlist of tool names or patterns (*) to enable + Keeps MCP tools unless an entry starts with mcp__ + --exclude-tools, -xt Comma-separated denylist of tool names or patterns (*) to disable + Applies to all tools, MCP tools included --thinking Set thinking level: off, minimal, low, medium, high, xhigh, max --extension, -e Load an extension file or builtin: (can be used multiple times) --no-extensions, -ne Disable extension discovery and built-in extensions (explicit -e paths still work) + --no-mcp Disable built-in MCP support: no servers connect and no MCP tools --skill Load a skill file or directory (can be used multiple times) --no-skills, -ns Disable skills discovery and loading --prompt-template Load a prompt template file or directory (can be used multiple times) @@ -391,6 +395,9 @@ ${chalk.bold("Examples:")} # Read-only mode (no file modifications possible) ${APP_NAME} --tools read,grep,find,ls -p "Review the code in src/" + # Codemode with only the tools of one MCP server + ${APP_NAME} --tools read,bash,codemode,'mcp__radius__*' + # Disable one tool while keeping the rest available ${APP_NAME} --exclude-tools ask_question diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index f641d6ec9..6263a8860 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -113,6 +113,7 @@ import { wrapRegisteredTools, } from "./extensions/index.ts"; import { emitSessionShutdownEvent } from "./extensions/runner.ts"; +import { createToolNameMatcher, isMcpToolName } from "./mcp-servers.ts"; import { type BashExecutionMessage, type CustomMessage, convertToLlm } from "./messages.ts"; import { ModelRegistry } from "./model-registry.ts"; import type { ModelRuntime } from "./model-runtime.ts"; @@ -265,9 +266,14 @@ export interface AgentSessionConfig { * tools newly added to the setting. Tools removed from it stay active. */ usesDefaultTools?: boolean; - /** Optional allowlist of tool names. When provided, only these tool names are exposed. */ + /** + * Optional allowlist of tool names or patterns (`*` matches any characters). When provided, only + * matching tools are exposed. A non-empty list without `mcp__` entries also keeps MCP tools + * registered for codemode and tool_search; only tool_search can declare them. An empty list + * exposes no tools. + */ allowedToolNames?: string[]; - /** Optional denylist of tool names. When provided, these tool names are not exposed. */ + /** Optional denylist of tool names or patterns. When provided, matching tools are not exposed. */ excludedToolNames?: string[]; /** * Override base tools (useful for custom runtimes). @@ -430,8 +436,14 @@ export class AgentSession { */ private _pendingToolNames = new Set(); private _usesDefaultTools: boolean; - private _allowedToolNames?: Set; - private _excludedToolNames?: Set; + /** Matches the `--tools` entries: tool names or patterns. */ + private _allowedTools?: (name: string) => boolean; + /** + * Whether the allowlist filters MCP tools: it is empty (`--no-tools`) or names an MCP tool + * (`mcp__*`). Otherwise it keeps MCP tools registered for codemode and tool_search. + */ + private _allowlistFiltersMcp = false; + private _excludedTools?: (name: string) => boolean; private _baseToolsOverride?: Record; private _sessionStartEvent: SessionStartEvent; private _extensionUIContext?: ExtensionUIContext; @@ -475,8 +487,12 @@ export class AgentSession { this._extensionRunnerRef = config.extensionRunnerRef; this._initialActiveToolNames = config.initialActiveToolNames; this._usesDefaultTools = config.usesDefaultTools ?? false; - this._allowedToolNames = config.allowedToolNames ? new Set(config.allowedToolNames) : undefined; - this._excludedToolNames = config.excludedToolNames ? new Set(config.excludedToolNames) : undefined; + if (config.allowedToolNames) { + this._allowedTools = createToolNameMatcher(config.allowedToolNames); + this._allowlistFiltersMcp = + config.allowedToolNames.length === 0 || config.allowedToolNames.some((entry) => entry.startsWith("mcp__")); + } + this._excludedTools = config.excludedToolNames ? createToolNameMatcher(config.excludedToolNames) : undefined; this._baseToolsOverride = config.baseToolsOverride; this._sessionStartEvent = config.sessionStartEvent ?? { type: "session_start", reason: "startup" }; @@ -1500,8 +1516,25 @@ export class AgentSession { this._rebuildSystemPrompt(tools.map((tool) => tool.name)); } + /** + * Whether `--tools` and `--exclude-tools` keep the tool registered. MCP tools stay registered + * unless the allowlist filters them (see `_allowlistFiltersMcp`). + */ private _isAllowedTool(name: string): boolean { - return (!this._allowedToolNames || this._allowedToolNames.has(name)) && !this._excludedToolNames?.has(name); + if (this._excludedTools?.(name)) return false; + if (!this._allowedTools || this._allowedTools(name)) return true; + return !this._allowlistFiltersMcp && isMcpToolName(name); + } + + /** + * Whether the tool may be active, which declares it to the model. MCP tools the allowlist keeps + * without matching them are only for codemode and tool_search: they may be declared only when + * tool_search can load them (non-`direct` exposure and tool_search registered). This also applies + * to tools restored from the transcript or set by extensions. + */ + private _isActivatable(name: string): boolean { + if (!this._allowedTools || this._allowedTools(name) || !isMcpToolName(name)) return true; + return this._getToolExposure(name) !== "direct" && this._toolRegistry.has("tool_search"); } private _getToolExposure(name: string): ToolExposure { @@ -1528,7 +1561,7 @@ export class AgentSession { private _applyToolLoadout(toolNames: string[]): AgentTool[] { const tools = [...new Set(toolNames)].flatMap((name) => { const tool = this._toolRegistry.get(name); - return tool && this._getToolExposure(name) !== "hidden" ? [tool] : []; + return tool && this._getToolExposure(name) !== "hidden" && this._isActivatable(name) ? [tool] : []; }); const hooks = tools.flatMap((tool) => { const entry = this._toolDefinitions.get(tool.name); @@ -3452,7 +3485,7 @@ export class AgentSession { [...this._toolRegistry.keys()].filter((name) => this._isActivatedOnRegistration(name)), ); const previousActiveToolNames = this.getActiveToolNames(); - const allowedToolNames = this._allowedToolNames; + const allowedTools = this._allowedTools; const registeredTools = this._extensionRunner.getAllRegisteredTools(); const allCustomTools = [ @@ -3520,10 +3553,11 @@ export class AgentSession { options?.activeToolNames ? [...options.activeToolNames] : [...previousActiveToolNames] ).filter((name) => this._isAllowedTool(name)); - if (allowedToolNames) { + if (allowedTools) { for (const toolName of this._toolRegistry.keys()) { - // Naming a tool activates it even when it is not active by default. - if (allowedToolNames.has(toolName) && this._isDeclarable(toolName)) { + // Naming or matching a tool activates it even when it is not active by default. MCP tools + // kept registered without being named stay inactive. + if (allowedTools(toolName) && this._isDeclarable(toolName)) { nextActiveToolNames.push(toolName); } } diff --git a/packages/coding-agent/src/core/mcp-servers.ts b/packages/coding-agent/src/core/mcp-servers.ts index 14e4e14dc..1f6414928 100644 --- a/packages/coding-agent/src/core/mcp-servers.ts +++ b/packages/coding-agent/src/core/mcp-servers.ts @@ -5,6 +5,41 @@ * extension that handles `mcp_servers_change`) connects them next to the servers from `mcp.json`. */ +/** + * Regular expression for a tool name pattern where `*` matches any characters, as `toolExposure`, + * `--tools`, and `--exclude-tools` accept them. + */ +function toolPatternRegExp(pattern: string): RegExp { + const source = pattern + .split("*") + .map((part) => part.replace(/[.+?^${}()|[\]\\]/g, "\\$&")) + .join(".*"); + return new RegExp(`^${source}$`); +} + +/** Whether a tool name matches any of the entries, each an exact name or a pattern. */ +export function createToolNameMatcher(entries: readonly string[]): (name: string) => boolean { + const names = new Set(entries.filter((entry) => !entry.includes("*"))); + const patterns = entries.filter((entry) => entry.includes("*")).map(toolPatternRegExp); + return (name) => names.has(name) || patterns.some((pattern) => pattern.test(name)); +} + +/** MCP resource tools, which reach every server with resources. */ +export const LIST_MCP_RESOURCES_TOOL = "list_mcp_resources"; +export const LIST_MCP_RESOURCE_TEMPLATES_TOOL = "list_mcp_resource_templates"; +export const READ_MCP_RESOURCE_TOOL = "read_mcp_resource"; + +const MCP_RESOURCE_TOOLS: ReadonlySet = new Set([ + LIST_MCP_RESOURCES_TOOL, + LIST_MCP_RESOURCE_TEMPLATES_TOOL, + READ_MCP_RESOURCE_TOOL, +]); + +/** Whether a tool comes from MCP: a server tool (`mcp____`) or a resource tool. */ +export function isMcpToolName(name: string): boolean { + return name.startsWith("mcp__") || MCP_RESOURCE_TOOLS.has(name); +} + /** * - `codemode`: tools are callable from codemode scripts but neither declared to the model nor * listed in the codemode description, which lists only the server's namespace. Scripts find them @@ -195,14 +230,6 @@ function resolveExposureAliases(value: Record): Record part.replace(/[.+?^${}()|[\]\\]/g, "\\$&")) - .join(".*"); - return new RegExp(`^${source}$`); -} - /** Exposure of one tool of a server: its `toolExposure` entry, else the server's `exposure`. */ export function getMcpToolExposure(config: McpServerConfig, toolName: string): McpExposure { const overrides = config.toolExposure ?? {}; diff --git a/packages/coding-agent/src/core/resource-loader.ts b/packages/coding-agent/src/core/resource-loader.ts index fa877d83c..b45b21299 100644 --- a/packages/coding-agent/src/core/resource-loader.ts +++ b/packages/coding-agent/src/core/resource-loader.ts @@ -280,6 +280,8 @@ export interface DefaultResourceLoaderOptions { additionalThemePaths?: string[]; extensionFactories?: InlineExtension[]; noExtensions?: boolean; + /** Built-in extensions not to load, by name (such as `mcp`), even when settings or `-e` enable them. */ + disabledBuiltinExtensions?: string[]; noSkills?: boolean; noPromptTemplates?: boolean; noThemes?: boolean; @@ -319,6 +321,7 @@ export class DefaultResourceLoader implements ResourceLoader { private extensionFactories: InlineExtension[]; private builtinExtensions: Map; private noExtensions: boolean; + private disabledBuiltinExtensions: Set; private noSkills: boolean; private noPromptTemplates: boolean; private noThemes: boolean; @@ -384,6 +387,7 @@ export class DefaultResourceLoader implements ResourceLoader { this.additionalPromptTemplatePaths = options.additionalPromptTemplatePaths ?? []; this.additionalThemePaths = options.additionalThemePaths ?? []; this.noExtensions = options.noExtensions ?? false; + this.disabledBuiltinExtensions = new Set(options.disabledBuiltinExtensions); this.noSkills = options.noSkills ?? false; this.noPromptTemplates = options.noPromptTemplates ?? false; this.noThemes = options.noThemes ?? false; @@ -566,9 +570,13 @@ export class DefaultResourceLoader implements ResourceLoader { const cliEnabledPrompts = getEnabledPaths(cliExtensionPaths.prompts); const cliEnabledThemes = getEnabledPaths(cliExtensionPaths.themes); - const extensionPaths = this.noExtensions - ? cliEnabledExtensions - : this.mergePaths(cliEnabledExtensions, enabledExtensions); + const extensionPaths = ( + this.noExtensions ? cliEnabledExtensions : this.mergePaths(cliEnabledExtensions, enabledExtensions) + ).filter( + (path) => + !path.startsWith(BUILTIN_PATH_PREFIX) || + !this.disabledBuiltinExtensions.has(path.slice(BUILTIN_PATH_PREFIX.length)), + ); const packageWarnings = collectExtensionPackageWarnings(extensionPaths, metadataByPath); const extensionsResult = await this.loadFinalExtensionSet(extensionPaths, preTrustExtensions); diff --git a/packages/coding-agent/src/core/sdk.ts b/packages/coding-agent/src/core/sdk.ts index 035304a2c..09bb92fa0 100644 --- a/packages/coding-agent/src/core/sdk.ts +++ b/packages/coding-agent/src/core/sdk.ts @@ -9,6 +9,7 @@ import { formatNoModelsAvailableMessage } from "./auth-guidance.ts"; import { CacheWarmer } from "./cache-warmer.ts"; import { DEFAULT_THINKING_LEVEL } from "./defaults.ts"; import type { ExtensionRunner, LoadExtensionsResult, SessionStartEvent, ToolDefinition } from "./extensions/index.ts"; +import { createToolNameMatcher } from "./mcp-servers.ts"; import { convertToLlm } from "./messages.ts"; import { findInitialModel } from "./model-resolver.ts"; import { ModelRuntime } from "./model-runtime.ts"; @@ -63,16 +64,21 @@ export interface CreateAgentSessionOptions { */ noTools?: "all" | "builtin"; /** - * Optional allowlist of tool names. + * Optional allowlist of tool names or patterns, where `*` matches any characters. * * When omitted, pi uses the resolved `defaultTools` setting for the initial * selection when configured. Otherwise it enables the default built-in tools * (read, bash, edit, write). Extension/custom tools remain enabled unless - * `noTools` changes that default. When provided, only the listed tool names are - * enabled. + * `noTools` changes that default. When provided, only matching tools are + * enabled. MCP tools stay registered for codemode and tool search unless an + * entry starts with `mcp__`; then only matching MCP tools are kept. An empty + * list, like `noTools: "all"`, disables MCP tools too. */ tools?: string[]; - /** Optional denylist of tool names to disable. Applies after `tools` when both are provided. */ + /** + * Optional denylist of tool names or patterns to disable. Applies after `tools` when both are + * provided, MCP tools included. + */ excludeTools?: string[]; /** Custom tools to register (in addition to built-in tools). */ customTools?: ToolDefinition[]; @@ -264,10 +270,10 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} const configuredDefaultToolNames = settingsManager.getDefaultTools(); const allowedToolNames = options.tools ?? (options.noTools === "all" ? [] : undefined); const excludedToolNames = options.excludeTools; - const excludedToolNameSet = excludedToolNames ? new Set(excludedToolNames) : undefined; + const isExcludedTool = excludedToolNames ? createToolNameMatcher(excludedToolNames) : undefined; const initialActiveToolNames = ( options.tools ?? (options.noTools ? [] : (configuredDefaultToolNames ?? DEFAULT_TOOL_NAMES)) - ).filter((name) => !excludedToolNameSet?.has(name)); + ).filter((name) => !isExcludedTool?.(name)); // Create convertToLlm wrapper that filters images if blockImages is enabled (defense-in-depth) const convertToLlmWithBlockImages = (messages: AgentMessage[]): Message[] => { diff --git a/packages/coding-agent/src/extensions/mcp/resources.ts b/packages/coding-agent/src/extensions/mcp/resources.ts index 81002d5b2..e0e21efeb 100644 --- a/packages/coding-agent/src/extensions/mcp/resources.ts +++ b/packages/coding-agent/src/extensions/mcp/resources.ts @@ -22,18 +22,15 @@ import type { } from "@earendil-works/pi-mcp"; import type { TSchema } from "typebox"; import type { ToolAnnotations, ToolDefinition } from "../../core/extensions/types.ts"; -import type { McpExposure } from "./config.ts"; import { - limitMcpContent, - type McpToolDetails, + LIST_MCP_RESOURCE_TEMPLATES_TOOL, + LIST_MCP_RESOURCES_TOOL, READ_MCP_RESOURCE_TOOL, - toModelContent, - toToolExposure, -} from "./tools.ts"; +} from "../../core/mcp-servers.ts"; +import type { McpExposure } from "./config.ts"; +import { limitMcpContent, type McpToolDetails, toModelContent, toToolExposure } from "./tools.ts"; -export const LIST_MCP_RESOURCES_TOOL = "list_mcp_resources"; -export const LIST_MCP_RESOURCE_TEMPLATES_TOOL = "list_mcp_resource_templates"; -export { READ_MCP_RESOURCE_TOOL }; +export { LIST_MCP_RESOURCE_TEMPLATES_TOOL, LIST_MCP_RESOURCES_TOOL, READ_MCP_RESOURCE_TOOL }; /** A connected server that offers resources. */ export interface McpResourceServer { diff --git a/packages/coding-agent/src/extensions/mcp/tools.ts b/packages/coding-agent/src/extensions/mcp/tools.ts index 90699f9c0..ca4da96b0 100644 --- a/packages/coding-agent/src/extensions/mcp/tools.ts +++ b/packages/coding-agent/src/extensions/mcp/tools.ts @@ -29,6 +29,7 @@ import type { ToolNamespace, ToolRenderers, } from "../../core/extensions/types.ts"; +import { READ_MCP_RESOURCE_TOOL } from "../../core/mcp-servers.ts"; import { formatToolCallWithArgs, getTextOutput, replaceTabs } from "../../core/tools/render-utils.ts"; import { formatSize, truncateMiddle } from "../../core/tools/truncate.ts"; import { keyHint } from "../../modes/interactive/components/keybinding-hints.ts"; @@ -51,7 +52,7 @@ export const MCP_OUTPUT_MAX_BYTES = 20 * 1024; /** Visual (wrapped) result lines shown before the output is expanded. */ const OUTPUT_PREVIEW_LINES = 5; /** Tool that reads the resources named by resource links. */ -export const READ_MCP_RESOURCE_TOOL = "read_mcp_resource"; +export { READ_MCP_RESOURCE_TOOL }; export interface McpToolDetails { server: string; diff --git a/packages/coding-agent/src/main.ts b/packages/coding-agent/src/main.ts index d9c6ce33c..dea9e1501 100644 --- a/packages/coding-agent/src/main.ts +++ b/packages/coding-agent/src/main.ts @@ -782,6 +782,7 @@ export async function main(args: string[], options?: MainOptions) { additionalPromptTemplatePaths: resolvedPromptTemplatePaths, additionalThemePaths: resolvedThemePaths, noExtensions: parsed.noExtensions, + disabledBuiltinExtensions: parsed.noMcp ? ["mcp"] : undefined, noSkills: parsed.noSkills, noPromptTemplates: parsed.noPromptTemplates, noThemes: parsed.noThemes, diff --git a/packages/coding-agent/test/args.test.ts b/packages/coding-agent/test/args.test.ts index a6af05c49..a09334821 100644 --- a/packages/coding-agent/test/args.test.ts +++ b/packages/coding-agent/test/args.test.ts @@ -282,6 +282,14 @@ describe("parseArgs", () => { }); }); + describe("--no-mcp flag", () => { + test("parses --no-mcp flag", () => { + const result = parseArgs(["--no-mcp"]); + expect(result.noMcp).toBe(true); + expect(result.unknownFlags.size).toBe(0); + }); + }); + describe("--skill flag", () => { test("parses single --skill", () => { const result = parseArgs(["--skill", "./skill-dir"]); diff --git a/packages/coding-agent/test/resource-loader.test.ts b/packages/coding-agent/test/resource-loader.test.ts index d996f4b10..3c2a7be87 100644 --- a/packages/coding-agent/test/resource-loader.test.ts +++ b/packages/coding-agent/test/resource-loader.test.ts @@ -1110,6 +1110,27 @@ export default function(pi: ExtensionAPI) { expect(loaded).toEqual(["llama"]); }); + it("should skip disabledBuiltinExtensions even when settings or -e enable them", async () => { + mkdirSync(join(cwd, ".pi"), { recursive: true }); + writeFileSync(join(cwd, ".pi", "settings.json"), JSON.stringify({ extensions: ["+builtin:mcp"] })); + const loaded: string[] = []; + const loader = new DefaultResourceLoader({ + cwd, + agentDir, + disabledBuiltinExtensions: ["mcp"], + additionalExtensionPaths: ["builtin:mcp"], + extensionFactories: [ + { name: "mcp", builtin: true, factory: () => void loaded.push("mcp") }, + { name: "llama", builtin: true, factory: () => void loaded.push("llama") }, + ], + }); + await loader.reload({ resolveProjectTrust: async () => true }); + + expect(loader.getExtensions().extensions.map((extension) => extension.path)).toEqual(["builtin:llama"]); + expect(loader.getExtensions().errors).toEqual([]); + expect(loaded).toEqual(["llama"]); + }); + it("should load built-in extensions after file extensions with and without trust resolution", async () => { const userExtDir = join(agentDir, "extensions"); mkdirSync(userExtDir, { recursive: true }); diff --git a/packages/coding-agent/test/suite/agent-session-mcp.test.ts b/packages/coding-agent/test/suite/agent-session-mcp.test.ts index 6fe69e24a..f30137dc9 100644 --- a/packages/coding-agent/test/suite/agent-session-mcp.test.ts +++ b/packages/coding-agent/test/suite/agent-session-mcp.test.ts @@ -130,6 +130,13 @@ describe("AgentSession MCP integration", () => { options: { autoEnableCodemode?: boolean; builtInTools?: string[]; + /** `--tools` and `--exclude-tools`. */ + allowedToolNames?: string[]; + excludedToolNames?: string[]; + /** Session to continue. */ + sessionManager?: SessionManager; + /** Whether to wait for `mcp__docs__search` to register. Default: true. */ + waitForTools?: boolean; extensionFactories?: ExtensionFactory[]; toolExposure?: Record; resources?: boolean; @@ -140,6 +147,10 @@ describe("AgentSession MCP integration", () => { ) { const { builtInTools, + allowedToolNames, + excludedToolNames, + sessionManager, + waitForTools = true, extensionFactories = [], toolExposure, resources, @@ -164,7 +175,10 @@ describe("AgentSession MCP integration", () => { // `builtInTools` are the built-in tools active at the start. The MCP extension activates codemode // or tool_search. const harness = await createHarness({ - initialActiveToolNames: builtInTools ?? [], + initialActiveToolNames: allowedToolNames ?? builtInTools ?? [], + allowedToolNames, + excludedToolNames, + sessionManager, extensionFactories: [ ...extensionFactories, createCodemodeExtension(), @@ -185,9 +199,11 @@ describe("AgentSession MCP integration", () => { uiContext: createTestUiContext({ notify: (message) => notifications.push(message) }), }); // The first prompt waits only for servers with direct tools; wait for the others here. - await vi.waitFor(() => - expect(harness.session.getAllTools().some((tool) => tool.name === "mcp__docs__search")).toBe(true), - ); + if (waitForTools) { + await vi.waitFor(() => + expect(harness.session.getAllTools().some((tool) => tool.name === "mcp__docs__search")).toBe(true), + ); + } return { harness, calls, servers, notifications }; } @@ -212,6 +228,100 @@ describe("AgentSession MCP integration", () => { return harness.session.getCallableToolNames(); } + it("keeps MCP tools when --tools names no MCP tool", async () => { + for (const exposure of ["codemode", "deferred"] as const) { + const { harness } = await setup(exposure, undefined, { + allowedToolNames: ["read", "codemode"], + resources: true, + }); + expect(harness.session.getActiveToolNames()).toEqual(["read", "codemode"]); + expect(nestedToolNames(harness)).toEqual( + expect.arrayContaining(["mcp__docs__search", "mcp__docs__fail", "list_mcp_resources", "read_mcp_resource"]), + ); + expect(harness.session.getAllTools().map((tool) => tool.name)).not.toContain("bash"); + } + + // A direct MCP tool stays registered but is declared only when --tools names it. + const { harness } = await setup("codemode", undefined, { + allowedToolNames: ["codemode"], + toolExposure: { fail: "direct" }, + }); + await vi.waitFor(() => + expect(harness.session.getAllTools().map((tool) => tool.name)).toContain("mcp__docs__fail"), + ); + expect(harness.session.getActiveToolNames()).toEqual(["codemode"]); + }); + + it("removes MCP tools with --no-tools", async () => { + // The first prompt waits for servers with direct tools, so their tools would be registered by then. + const { harness, servers } = await setup("direct", undefined, { allowedToolNames: [], waitForTools: false }); + harness.setResponses([fauxAssistantMessage("done")]); + await harness.session.prompt("go"); + + expect(servers).toHaveLength(1); + expect(harness.session.getAllTools()).toEqual([]); + expect(harness.session.getActiveToolNames()).toEqual([]); + }); + + it("does not declare unnamed MCP tools restored from the transcript", async () => { + const first = await setup("direct"); + first.harness.setResponses([fauxAssistantMessage("one"), fauxAssistantMessage("two")]); + await first.harness.session.prompt("first"); + await first.harness.session.prompt("second"); + expect(declaredToolNames(first.harness)).toContain("mcp__docs__search"); + + // Like `pi --tools read,codemode -c` followed by /tree. + const second = await setup("direct", undefined, { + allowedToolNames: ["read", "codemode"], + sessionManager: first.harness.sessionManager, + }); + const firstAssistant = second.harness.sessionManager + .getBranch() + .find((entry) => entry.type === "message" && entry.message.role === "assistant"); + if (!firstAssistant) throw new Error("No assistant entry"); + await second.harness.session.navigateTree(firstAssistant.id); + + // The transcript's loadout is restored without the MCP tools it declared. + expect(second.harness.session.getActiveToolNames()).toEqual([]); + expect(second.harness.session.getAllTools().map((tool) => tool.name)).toContain("mcp__docs__search"); + }); + + it("lets tool_search declare unnamed MCP tools when --tools names it", async () => { + const { harness } = await setup("deferred", undefined, { allowedToolNames: ["tool_search"] }); + harness.setResponses([ + fauxAssistantMessage([fauxToolCall("tool_search", { query: "search the docs", limit: 1 })], { + stopReason: "toolUse", + }), + fauxAssistantMessage("loaded"), + ]); + await harness.session.prompt("load"); + + expect(harness.session.getActiveToolNames()).toEqual(["tool_search", "mcp__docs__search"]); + }); + + it("filters MCP tools by the mcp__ entries of --tools", async () => { + const { harness } = await setup("codemode", undefined, { + allowedToolNames: ["codemode", "mcp__docs__s*"], + resources: true, + toolExposure: { shot: "direct" }, + }); + await vi.waitFor(() => expect(harness.session.getActiveToolNames()).toEqual(["codemode", "mcp__docs__shot"])); + const registered = harness.session.getAllTools().map((tool) => tool.name); + expect(registered).toEqual(expect.arrayContaining(["mcp__docs__search", "mcp__docs__shot"])); + expect(registered).not.toContain("mcp__docs__fail"); + expect(registered).not.toContain("list_mcp_resources"); + }); + + it("removes MCP tools matching --exclude-tools patterns", async () => { + const { harness } = await setup("codemode", undefined, { + allowedToolNames: ["codemode"], + excludedToolNames: ["mcp__docs__f*"], + }); + const registered = harness.session.getAllTools().map((tool) => tool.name); + expect(registered).toContain("mcp__docs__search"); + expect(registered).not.toContain("mcp__docs__fail"); + }); + it("exposes codemode-only MCP tools through codemode and hides them from the model", async () => { const { harness, calls } = await setup("codemode"); const searchName = createMcpToolName("docs", "search"); diff --git a/packages/coding-agent/test/suite/regressions/5109-exclude-tools.test.ts b/packages/coding-agent/test/suite/regressions/5109-exclude-tools.test.ts index 0e7454162..bb7a27f0d 100644 --- a/packages/coding-agent/test/suite/regressions/5109-exclude-tools.test.ts +++ b/packages/coding-agent/test/suite/regressions/5109-exclude-tools.test.ts @@ -78,4 +78,22 @@ describe("regression #5109: exclude tools", () => { harness.cleanup(); } }); + + it("matches allowlist and denylist patterns", async () => { + const harness = await createHarness({ + allowedToolNames: ["*_tool", "ask_*", "re*"], + excludedToolNames: ["ask*"], + initialActiveToolNames: ["*_tool", "ask_*", "re*"], + extensionFactories, + }); + try { + await harness.session.bindExtensions({}); + + // Patterns that match a tool activate it like its name. + expect(toolNames(harness.session.getAllTools())).toEqual(["dynamic_tool", "read"]); + expect(harness.session.getActiveToolNames().sort()).toEqual(["dynamic_tool", "read"]); + } finally { + harness.cleanup(); + } + }); });