diff --git a/extensions/gentle-agents.ts b/extensions/gentle-agents.ts index 870d2bfc0..11ef4b495 100644 --- a/extensions/gentle-agents.ts +++ b/extensions/gentle-agents.ts @@ -1666,6 +1666,9 @@ export default function gentleAgents(pi: ExtensionAPI, env: NodeJS.ProcessEnv = const parentRepositoryIdentity = resolveCanonicalGitRepositoryIdentitySync(parentWorktreeRoot); const childEnv = { ...deps.env }; if (foreign) for (const key of inheritedUnsafeGitEnvironmentKeys(childEnv)) delete childEnv[key]; + const activeMcpTools = typeof pi.getAllTools === "function" + ? pi.getAllTools().map((tool) => tool.name).filter((name) => name.startsWith("mcp__")) + : undefined; const request: TaskRequest = { agent, prompt, @@ -1674,6 +1677,7 @@ export default function gentleAgents(pi: ExtensionAPI, env: NodeJS.ProcessEnv = mode, cwd: target ?? parentWorktreeRoot, parentSessionId, + ...(activeMcpTools && activeMcpTools.length > 0 ? { mcpTools: activeMcpTools } : {}), ...(admittedModel === undefined ? {} : { beforeSpawn: () => { if (!current()) throw new Error("Writer session changed before spawn."); registry.validate(originalCwd); diff --git a/lib/agents-runner.ts b/lib/agents-runner.ts index 7b28b0077..dc35f6aa9 100644 --- a/lib/agents-runner.ts +++ b/lib/agents-runner.ts @@ -129,6 +129,8 @@ export interface TaskRequest { sessionDir: string; resumeSessionPath: string | undefined; env: NodeJS.ProcessEnv; + // Active parent MCP tools for dynamic runtime capability expansion (#1686). + mcpTools?: readonly string[] | string[]; // Host-provided only: the launcher's package injection signal (#1690) or // the curated fallback. Never derived from tool input or agent definitions. extensionPaths?: string[]; @@ -270,9 +272,40 @@ export function formatChildExit(code: number | null | undefined, signal?: string const hostProcess: ProcessControl = { platform: process.platform, kill: (pid, signal) => process.kill(pid, signal) }; +/** + * Expands declared agent tools for Pi 1.0 native MCP execution (#1686). + * When `mcp` is declared, it is treated as a runtime capability sentinel: + * 1. Omit the retired literal `mcp` token. + * 2. Add `codemode` and `tool_search` to enable Pi 1.0 native MCP execution and discovery. + * 3. Add all active parent MCP tools (`mcp__*`). + * When a scoped prefix like `mcp__` is declared, it expands to `codemode`, + * `tool_search`, and matching `mcp____*` tools only when matching active tools exist. + * If no matching tools exist for that server prefix, discovery tools are omitted. + * Agents without `mcp` retain strict isolation with no MCP tools added. + */ +export function expandChildTools(tools: readonly string[], activeMcpTools: readonly string[] = []): string[] { + if (tools.length === 0) return []; + const expanded: string[] = []; + for (const tool of tools) { + if (tool === "mcp") { + expanded.push("codemode", "tool_search", ...activeMcpTools); + } else if (/^mcp__[a-zA-Z0-9_-]+$/.test(tool)) { + const prefix = `${tool}__`; + const matched = activeMcpTools.filter((t) => t.startsWith(prefix)); + if (matched.length > 0) { + expanded.push("codemode", "tool_search", ...matched); + } + } else { + expanded.push(tool); + } + } + return [...new Set([...expanded, PARENT_NOTIFICATION_TOOL])]; +} + // The --tools value, or undefined when Pi keeps its default tools. function requestedTools(request: TaskRequest): string | undefined { - const tools = request.agent.tools.length > 0 ? [...new Set([...request.agent.tools, PARENT_NOTIFICATION_TOOL])] : DEFAULT_TOOLS; + const rawTools = request.agent.tools; + const tools = rawTools.length > 0 ? expandChildTools(rawTools, request.mcpTools) : DEFAULT_TOOLS; return tools.length > 0 ? tools.join(",") : undefined; } diff --git a/odd/tasks/fix-1686-child-agents-native-mcp-allowlist.md b/odd/tasks/fix-1686-child-agents-native-mcp-allowlist.md new file mode 100644 index 000000000..32f5a9acf --- /dev/null +++ b/odd/tasks/fix-1686-child-agents-native-mcp-allowlist.md @@ -0,0 +1,25 @@ +# Dynamic MCP capability expansion for subagents on Pi 1.0 (#1686) + +## Objective and scope +Restore MCP server access for Gentle child agents running on Pi 1.0 native MCP. Because `pi-mcp-adapter` was retired and `--tools` in Pi 1.0 is a strict filter without glob support, child agents with `tools: ... mcp` lose access to all MCP tools. +Treat `mcp` in agent frontmatter as a runtime capability sentinel: +1. Dynamically expand `mcp` into `codemode`, `tool_search`, and all active `mcp__*` tools from the parent session (`request.mcpTools`). +2. Drop the retired literal `"mcp"` token from the child's `--tools` CLI arguments. +3. Support server-scoped tokens (`mcp__`) expanding to tools matching `mcp____*`. +4. Preserve strict isolation for agents without `mcp` in their frontmatter. + +## Completed tasks +- [x] T1: Strict TDD test reproducing lack of MCP expansion in `childArguments`. +- [x] T2: Implement `expandChildTools` in `lib/agents-runner.ts` and wire it into `childArguments`. +- [x] T3: Add `mcpTools` to `TaskRequest` in `lib/agents-runner.ts` and populate it from `pi.getAllTools()` in `extensions/gentle-agents.ts`. +- [x] T4: Verify with unit tests in `tests/agents-runner.test.ts` and `tests/gentle-agents.test.ts`, plus typecheck and package checks. +- [x] T5: Code review follow-up: omit `codemode` and `tool_search` when scoped `mcp__` matches no active tools, and verify generic `mcp` expansion with empty tool list. + +## Evidence +Base: upstream/main at cf3012f7. Branch: fix/1686-child-agents-native-mcp-allowlist. +- TDD RED: 2 failed / 1 passed in `tests/agents-runner.test.ts`. +- TDD GREEN: 89/89 passed in `tests/agents-runner.test.ts`. +- Agents extension suite: 190/190 passed in `tests/gentle-agents.test.ts`. +- Review follow-up: 91/91 passed in `tests/agents-runner.test.ts` (TDD RED: 1 failed / 90 passed, TDD GREEN: 91/91 passed). +- Typecheck: 186 recorded baseline diagnostics, 0 regressions, 12 improved. +- Package integrity: 155 files, 69 exact byte-pinned artifacts checked. diff --git a/tests/agents-runner.test.ts b/tests/agents-runner.test.ts index 82564c6d8..4a003eedd 100644 --- a/tests/agents-runner.test.ts +++ b/tests/agents-runner.test.ts @@ -731,6 +731,98 @@ test("childArguments grants every child the notification-only parent message too assert.equal(args[args.indexOf("--tools") + 1], "read,grep,subagent_parent_message"); }); +test("childArguments dynamically expands mcp sentinel into codemode, tool_search, and active mcp tools (#1686)", () => { + const activeMcpTools = [ + "mcp__context7__resolve_library_id", + "mcp__context7__get_docs", + "mcp__notion__search", + ]; + const req = request({ + agent: { ...explorer, tools: ["read", "grep", "mcp"] }, + mcpTools: activeMcpTools, + }); + const args = childArguments(req); + const toolsArg = args[args.indexOf("--tools") + 1]; + const tools = toolsArg.split(","); + + assert.ok(!tools.includes("mcp"), "retired mcp token must be omitted from --tools"); + assert.ok(tools.includes("codemode"), "codemode must be present when mcp is declared"); + assert.ok(tools.includes("tool_search"), "tool_search must be present when mcp is declared"); + for (const mcpTool of activeMcpTools) { + assert.ok(tools.includes(mcpTool), `${mcpTool} must be included in --tools`); + } + assert.ok(tools.includes("read") && tools.includes("grep") && tools.includes("subagent_parent_message")); +}); + +test("childArguments expands scoped mcp__ tokens to matching active tools (#1686)", () => { + const activeMcpTools = [ + "mcp__context7__resolve_library_id", + "mcp__context7__get_docs", + "mcp__notion__search", + ]; + const req = request({ + agent: { ...explorer, tools: ["read", "mcp__context7"] }, + mcpTools: activeMcpTools, + }); + const args = childArguments(req); + const toolsArg = args[args.indexOf("--tools") + 1]; + const tools = toolsArg.split(","); + + assert.ok(!tools.includes("mcp__context7"), "server prefix token must be expanded"); + assert.ok(tools.includes("codemode") && tools.includes("tool_search")); + assert.ok(tools.includes("mcp__context7__resolve_library_id")); + assert.ok(tools.includes("mcp__context7__get_docs")); + assert.ok(!tools.includes("mcp__notion__search"), "unscoped servers must not be included"); +}); + +test("childArguments preserves strict tool isolation when agent omits mcp (#1686)", () => { + const activeMcpTools = ["mcp__context7__resolve_library_id"]; + const req = request({ + agent: { ...explorer, tools: ["read", "bash"] }, + mcpTools: activeMcpTools, + }); + const args = childArguments(req); + const toolsArg = args[args.indexOf("--tools") + 1]; + const tools = toolsArg.split(","); + + assert.ok(!tools.includes("codemode")); + assert.ok(!tools.includes("tool_search")); + assert.ok(!tools.includes("mcp__context7__resolve_library_id")); + assert.deepEqual(tools, ["read", "bash", "subagent_parent_message"]); +}); + +test("childArguments retains codemode and tool_search when mcp is declared even with no active MCP tools (#1686)", () => { + const req = request({ + agent: { ...explorer, tools: ["read", "mcp"] }, + mcpTools: [], + }); + const args = childArguments(req); + const toolsArg = args[args.indexOf("--tools") + 1]; + const tools = toolsArg.split(","); + + assert.ok(!tools.includes("mcp")); + assert.ok(tools.includes("codemode")); + assert.ok(tools.includes("tool_search")); + assert.deepEqual(tools, ["read", "codemode", "tool_search", "subagent_parent_message"]); +}); + +test("childArguments omits codemode and tool_search when scoped mcp__ has no matching tools (#1686)", () => { + const activeMcpTools = ["mcp__notion__search"]; + const req = request({ + agent: { ...explorer, tools: ["read", "mcp__context7"] }, + mcpTools: activeMcpTools, + }); + const args = childArguments(req); + const toolsArg = args[args.indexOf("--tools") + 1]; + const tools = toolsArg.split(","); + + assert.ok(!tools.includes("mcp__context7"), "server prefix token must be omitted"); + assert.ok(!tools.includes("codemode"), "codemode must be omitted when no scoped tools match"); + assert.ok(!tools.includes("tool_search"), "tool_search must be omitted when no scoped tools match"); + assert.ok(!tools.includes("mcp__notion__search"), "unscoped servers must not be included"); + assert.deepEqual(tools, ["read", "subagent_parent_message"]); +}); + // #1690: a forwarded launcher takeover keeps --no-extensions ahead of every // --extension and leaves the rest of the launch unchanged. test("childArguments forwards a takeover set with --no-extensions first", async () => {