diff --git a/.gitignore b/.gitignore index 70d09f319..8187eebb8 100644 --- a/.gitignore +++ b/.gitignore @@ -30,3 +30,7 @@ subagents/ # Working notes and design spikes — local only, never committed docs/plans/ + +# Scratch repro tests — never committed +__repro/ +**/__repro/ diff --git a/e2e/subagent-permission.test.ts b/e2e/subagent-permission.test.ts index a3d858c66..bfdaec4ef 100644 --- a/e2e/subagent-permission.test.ts +++ b/e2e/subagent-permission.test.ts @@ -21,6 +21,7 @@ import type { MCPClient } from "../src/mcp/client.js"; import { getSubAgentIdentity } from "../src/subagent/identity-context.js"; import { createSubAgentSessionStore } from "../src/subagent/session-store.js"; import { workerPermissionGate } from "../src/permission/reactor-authorize.js"; +import { getProcessWorkerGrantStore } from "../src/permission/worker-grant.js"; import { gateAgentTools } from "../src/plugins/permission-plugin.js"; const report = @@ -69,6 +70,12 @@ async function withWorker( }), }; try { + // Every probe reuses worker id "worker" in a fresh tmpdir, but denied-call + // grant envelopes live in a process-shared store keyed by that session id. + // A prior probe's pending envelope (same tool + empty args, other cwd) + // would veto this probe's call via the retry-from-another-directory + // blocker, so each probe starts from a clean store. + getProcessWorkerGrantStore().clear(); await withMockedModuleDuring( import.meta.resolve("../src/session/assemble-runtime.js"), (real: typeof import("../src/session/assemble-runtime.js")) => ({ diff --git a/src/agent/mcp-promote-on-execute.test.ts b/src/agent/mcp-promote-on-execute.test.ts new file mode 100644 index 000000000..34f2de904 --- /dev/null +++ b/src/agent/mcp-promote-on-execute.test.ts @@ -0,0 +1,182 @@ +/** + * CL-9704: Linear MCP discovery-to-invocation on the primary session. + * + * Regression lock for "discoverable but not callable": tool_search finds + * `mcp__linear__*`, and promote-on-execute must then commit a callable + * schema for exactly the called name and dispatch it — list_teams first, + * then save_issue. Search alone never promotes (the wire stays + * built-ins-only until a call), and promoting one name never implies its + * siblings: the primary session mounts MCP tools on demand, mirroring the + * worker requires_tools gate. + */ + +import { afterEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { + installMcpConnectMock, + linearHttpMcpServer, + mcpTestPermissionGate, +} from "../../testkit/mcp-connect-mock.js"; + +const linearTools = [ + { + name: "list_teams", + description: "List Linear teams", + inputSchema: { type: "object", properties: {}, required: [] }, + }, + { + name: "save_issue", + description: "Save a Linear issue", + inputSchema: { + type: "object", + properties: { + title: { type: "string" }, + teamId: { type: "string" }, + }, + required: ["title"], + }, + }, +]; + +const mock = await installMcpConnectMock( + import.meta.resolve("../mcp/client.js"), + { initialTools: linearTools, toolCallResult: "linear-ok" }, +); + +const { createAgentToolset } = await import("./tools.js"); +const { createToolIndex, createToolSearchTool } = + await import("./tool-search.js"); +const { createAdvertisedToolset } = + await import("../session/assemble-runtime.js"); + +const dirs: string[] = []; + +function tempDir(prefix: string): string { + const dir = mkdtempSync(join(tmpdir(), prefix)); + dirs.push(dir); + return dir; +} + +afterEach(() => { + mock.reset(); + for (const dir of dirs.splice(0)) { + rmSync(dir, { recursive: true, force: true }); + } +}); + +describe("CL-9704 linear MCP discovery-to-invocation", () => { + test("tool_search finds list_teams, promote-on-execute makes list_teams then save_issue callable", async () => { + const toolset = await createAgentToolset({ + cwd: tempDir("corbits-cl9704-"), + permissionGate: mcpTestPermissionGate(), + onOperatorGate: async () => ({ kind: "cancel" }), + mcpServers: [linearHttpMcpServer], + }); + try { + await toolset.connectMCP({ + interactiveAuth: false, + onStatus: () => undefined, + onToolsChanged: () => undefined, + }); + + const registered = toolset.dynamicRunner + .currentDefinitions() + .map((d) => d.name); + expect(registered).toContain("mcp__linear__list_teams"); + expect(registered).toContain("mcp__linear__save_issue"); + + // Primary-session promote-on-execute wiring (mirrors + // tui/runner/session.ts + tui/runner/exit.ts): the call gate keys off + // the advertised set, and the promoter declares exactly the called + // name, committing its schema onto the next infer's wire. + const advertised = createAdvertisedToolset({ + sessionMode: "orchestrator", + toolAvailability: { languageServerAvailable: false }, + getProvider: () => ({ providerName: "test", model: "test" }), + }); + toolset.dynamicRunner.setCallGate( + (name) => advertised.isAdvertised(name), + { isActivated: (name) => advertised.activated.has(name) }, + ); + let wire: string[] = []; + toolset.setToolPromoter((names) => { + advertised.activated.activate(names); + if (advertised.flushPromotions()) { + wire = advertised + .computeAdvertised(toolset.dynamicRunner.currentDefinitions()) + .map((d) => d.name); + } + }); + const wireSchemas = (): Map => + new Map( + advertised + .computeAdvertised(toolset.dynamicRunner.currentDefinitions()) + .map((d) => [d.name, d.inputSchema]), + ); + + const index = createToolIndex(() => + toolset.dynamicRunner.currentDefinitions(), + ); + const search = createToolSearchTool({ + search: (query, limit) => index.search(query, limit), + lookup: (name) => + toolset.dynamicRunner + .currentDefinitions() + .find((d) => d.name === name), + }); + if (search.kind !== "string") throw new Error("expected string tool"); + + // Discovery: the card names list_teams with its description. + const card = await search.handler( + { query: "linear teams" }, + new AbortController().signal, + ); + expect(card).toContain("mcp__linear__list_teams"); + expect(card).toContain("List Linear teams"); + + // Search alone promotes nothing: no Linear schema on the wire. + expect(wire).not.toContain("mcp__linear__list_teams"); + expect(wire).not.toContain("mcp__linear__save_issue"); + + const run = (name: string, args: Record = {}) => + toolset.dynamicRunner.run( + { id: `call-${name}`, name, arguments: args }, + new AbortController().signal, + ); + + // Invocation 1: list_teams dispatches and promotes only itself. + const listed = await run("mcp__linear__list_teams"); + expect(listed.isError).toBeUndefined(); + expect(listed.content).toContain("linear-ok"); + expect(wire).toContain("mcp__linear__list_teams"); + expect(wire).not.toContain("mcp__linear__save_issue"); + expect(wireSchemas().get("mcp__linear__list_teams")).toEqual( + linearTools[0]?.inputSchema, + ); + + // Invocation 2: save_issue dispatches with its args after discovery. + const saved = await run("mcp__linear__save_issue", { + title: "hello", + teamId: "t1", + }); + expect(saved.isError).toBeUndefined(); + expect(saved.content).toContain("linear-ok"); + expect(wire).toEqual( + expect.arrayContaining([ + "mcp__linear__list_teams", + "mcp__linear__save_issue", + ]), + ); + expect(mock.calls.map((c) => c.toolName)).toEqual([ + "list_teams", + "save_issue", + ]); + expect(mock.calls[1]?.args).toEqual({ title: "hello", teamId: "t1" }); + } finally { + await toolset.dispose(); + } + }); +}); diff --git a/src/subagent/agent-fleet-requires-tools.test.ts b/src/subagent/agent-fleet-requires-tools.test.ts index bec4f93c3..139d62945 100644 --- a/src/subagent/agent-fleet-requires-tools.test.ts +++ b/src/subagent/agent-fleet-requires-tools.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from "bun:test"; +import { stringTool, type AgentTool } from "@intx/agent"; import { createFleetMailbox, createSpawnAgentTool, @@ -38,6 +39,13 @@ const FULL_MOUNT_PROFILE: AgentProfile = { systemPromptRole: "You do anything.", }; +// BUILD_TOOLS-like envelope: built-ins only, no MCP names. +const BUILD_LIKE_PROFILE: AgentProfile = { + id: "build-worker", + systemPromptRole: "You build.", + capabilities: { mode: "allow", tools: ["read_file", "run_shell"] }, +}; + function makeDeps( run: (params: RunSubAgentParams) => Promise, capturedTelemetry: TelemetryEvent[], @@ -300,6 +308,156 @@ describe("spawn_agent requires_tools preflight", () => { expect(seenTier).toBe("leaf"); }); + test("mounted mcp__linear__ tool passes dispatch preflight under a built-ins-only allowlist", async () => { + const telemetry: TelemetryEvent[] = []; + let runCalled = false; + let seenRequires: readonly string[] | undefined; + const linearTool: AgentTool = stringTool({ + definition: { + name: "mcp__linear__list_teams", + description: "Inherited Linear tool", + inputSchema: {}, + }, + handler: async () => "teams", + }); + const base = makeDeps(async (params) => { + runCalled = true; + seenRequires = params.requiresTools; + return { report: "done" }; + }, telemetry); + const deps: AgentFleetDeps = { + ...base, + profiles: [BUILD_LIKE_PROFILE], + inheritMcpTools: () => [linearTool], + }; + const spawn = createSpawnAgentTool(deps); + + const result = await callSpawn(spawn, { + description: "linear job", + prompt: "list teams", + agent: "build-worker", + requires_tools: ["mcp__linear__list_teams"], + }); + + expect(result.isError).not.toBe(true); + expect(runCalled).toBe(true); + const body = JSON.parse(result.content) as { agent_id: string }; + expect(deps.sessions.get(body.agent_id)?.requiresTools).toEqual([ + "mcp__linear__list_teams", + ]); + expect(seenRequires).toEqual(["mcp__linear__list_teams"]); + }); + + test("unmounted mcp__linear__ tool rejects dispatch preflight as unknown_tool with no run", async () => { + const telemetry: TelemetryEvent[] = []; + let runCalled = false; + const base = makeDeps(async () => { + runCalled = true; + return { report: "done" }; + }, telemetry); + const deps: AgentFleetDeps = { + ...base, + profiles: [BUILD_LIKE_PROFILE], + }; + const spawn = createSpawnAgentTool(deps); + + const result = await callSpawn(spawn, { + description: "linear job", + prompt: "list teams", + agent: "build-worker", + requires_tools: ["mcp__linear__list_teams"], + }); + + expect(result.isError).toBe(true); + expect(result.content).toContain('unknown tool "mcp__linear__list_teams"'); + expect(result.content).not.toContain("Did you mean"); + expect(runCalled).toBe(false); + expect(deps.sessions.list()).toEqual([]); + expect(telemetry).toEqual([]); + }); + + test("requires_tools stamps only the requested live tool, never the inherited set", async () => { + const telemetry: TelemetryEvent[] = []; + let runCalled = false; + let seenRequires: readonly string[] | undefined; + const mcpTool = (name: string): AgentTool => + stringTool({ + definition: { + name, + description: `Inherited ${name}`, + inputSchema: {}, + }, + handler: async () => name, + }); + const base = makeDeps(async (params) => { + runCalled = true; + seenRequires = params.requiresTools; + return { report: "done" }; + }, telemetry); + const deps: AgentFleetDeps = { + ...base, + profiles: [BUILD_LIKE_PROFILE], + inheritMcpTools: () => [ + mcpTool("mcp__linear__list_teams"), + mcpTool("mcp__linear__create_issue"), + ], + }; + const spawn = createSpawnAgentTool(deps); + + const result = await callSpawn(spawn, { + description: "linear job", + prompt: "list teams", + agent: "build-worker", + requires_tools: ["mcp__linear__list_teams"], + }); + + // On-demand: dispatch stamps exactly the requested live tool — the + // inherited sibling mounts only under its own stamp (run.ts drops it). + expect(result.isError).not.toBe(true); + expect(runCalled).toBe(true); + expect(seenRequires).toEqual(["mcp__linear__list_teams"]); + }); + + test("requires_tools naming a live and an unmounted mcp__ tool rejects the unmounted one with no run", async () => { + const telemetry: TelemetryEvent[] = []; + let runCalled = false; + const base = makeDeps(async () => { + runCalled = true; + return { report: "done" }; + }, telemetry); + const deps: AgentFleetDeps = { + ...base, + profiles: [BUILD_LIKE_PROFILE], + inheritMcpTools: () => [ + stringTool({ + definition: { + name: "mcp__linear__list_teams", + description: "Inherited Linear tool", + inputSchema: {}, + }, + handler: async () => "teams", + }), + ], + }; + const spawn = createSpawnAgentTool(deps); + + const result = await callSpawn(spawn, { + description: "linear job", + prompt: "list teams and file", + agent: "build-worker", + requires_tools: ["mcp__linear__list_teams", "mcp__linear__create_issue"], + }); + + expect(result.isError).toBe(true); + expect(result.content).toContain( + 'unknown tool "mcp__linear__create_issue"', + ); + expect(result.content).not.toContain("Did you mean"); + expect(runCalled).toBe(false); + expect(deps.sessions.list()).toEqual([]); + expect(telemetry).toEqual([]); + }); + test("whitespace-only requires_tools rejects fail-closed with no session, telemetry, or run", async () => { for (const requiresTools of [[" "], ["read_file", " "]]) { const telemetry: TelemetryEvent[] = []; diff --git a/src/subagent/agent-fleet.ts b/src/subagent/agent-fleet.ts index d11bb2c3d..a73c357a3 100644 --- a/src/subagent/agent-fleet.ts +++ b/src/subagent/agent-fleet.ts @@ -127,6 +127,8 @@ import { type InterventionSink, } from "./intervention-log.js"; import { takeAndProjectMailboxRecord } from "./fleet-dry-drive.js"; +import { canonicalToolName } from "../agent/canonical-tool-name.js"; +import { isMcpToolName } from "../mcp/tool-name.js"; const log = getLogger([LOG_NAMESPACE_ROOT, "subagent", "agent-fleet"]); @@ -1115,12 +1117,25 @@ export function createSpawnAgentTool(deps: AgentFleetDeps): AgentTool { } const requiresToolsRaw = rawEntries.map((t) => t.trim()); if (requiresToolsRaw.length > 0) { + // Live inherited-MCP set: the worker mount carries a requested live + // tool on demand (run.ts retains stamped inherited MCP tools, and + // retains the inherited set unstamped only when no narrower + // constraint applies), so preflight validates `mcp__*` requirements + // against this set instead of the built-in catalog. Gating through + // the worker gate keeps the names to what this dispatch may + // actually mount. + const availableMcpTools = ( + deps.inheritMcpTools?.(deps.permissionGate) ?? [] + ) + .map((tool) => canonicalToolName(tool.definition.name)) + .filter((name) => isMcpToolName(name)); const preflight = preflightCapabilities({ required: requiresToolsRaw, ...(resolved.capabilities !== undefined ? { resolvedFilter: resolved.capabilities } : {}), knownEngines: DEFAULT_KNOWN_ENGINES, + availableMcpTools, agentLabel: resolved.agentLabel, }); if (!preflight.ok) { diff --git a/src/subagent/capability-preflight.test.ts b/src/subagent/capability-preflight.test.ts index e157ef2a4..8ec91d62d 100644 --- a/src/subagent/capability-preflight.test.ts +++ b/src/subagent/capability-preflight.test.ts @@ -25,15 +25,24 @@ function preflight( required: readonly string[], resolvedFilter?: CapabilityFilter, knownEngines: readonly string[] = DEFAULT_KNOWN_ENGINES, + availableMcpTools: readonly string[] = [], ) { return preflightCapabilities({ required, ...(resolvedFilter !== undefined ? { resolvedFilter } : {}), knownEngines, + availableMcpTools, agentLabel: "test-worker", }); } +// A BUILD_TOOLS-like allowlist: built-ins only, no MCP names — the shape +// that used to strip inherited Linear tools at both layers. +const ALLOW_BUILD_NO_MCP: CapabilityFilter = { + mode: "allow", + tools: ["read_file", "run_shell"], +}; + describe("preflightCapabilities", () => { test("allow filter mounting the tool passes and returns canonical names", () => { const result = preflight(["run_shell"], ALLOW_SHELL); @@ -125,6 +134,72 @@ describe("preflightCapabilities", () => { expect(alternatives).toEqual([...alternatives].sort()); expect(alternatives).not.toContain("dispatch"); }); + + test("mounted mcp__linear__ tool passes preflight under a built-ins-only allowlist", () => { + const result = preflight( + ["mcp__linear__list_teams"], + ALLOW_BUILD_NO_MCP, + DEFAULT_KNOWN_ENGINES, + ["mcp__linear__list_teams"], + ); + expect(result).toEqual({ + ok: true, + canonical: ["mcp__linear__list_teams"], + }); + }); + + test("unmounted mcp__linear__ tool rejects unknown_tool when no server is mounted", () => { + const result = preflight(["mcp__linear__list_teams"], ALLOW_BUILD_NO_MCP); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("expected rejection"); + expect(result.unavailable.code).toBe("unknown_tool"); + expect(result.unavailable.tool).toBe("mcp__linear__list_teams"); + expect(result.unavailable.suggestion).toBeUndefined(); + }); + + test("mcp__ tool from an unmounted server rejects even when other servers are mounted", () => { + const result = preflight( + ["mcp__nope__frobnicate"], + ALLOW_BUILD_NO_MCP, + DEFAULT_KNOWN_ENGINES, + ["mcp__linear__list_teams"], + ); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("expected rejection"); + expect(result.unavailable.code).toBe("unknown_tool"); + expect(result.unavailable.suggestion).toBeUndefined(); + }); + + test("preflight grants only the named requirement, never sibling live tools", () => { + const result = preflight( + ["mcp__linear__list_teams"], + ALLOW_BUILD_NO_MCP, + DEFAULT_KNOWN_ENGINES, + ["mcp__linear__list_teams", "mcp__linear__create_issue"], + ); + // On-demand: the stamp covers exactly the requested tool — the live + // sibling is not implied and mounts only under its own stamp. + expect(result).toEqual({ + ok: true, + canonical: ["mcp__linear__list_teams"], + }); + }); + + test("explicit exclude naming a live MCP tool rejects permission_static", () => { + const result = preflightCapabilities({ + required: ["mcp__linear__list_teams"], + resolvedFilter: { + mode: "exclude", + tools: ["mcp__linear__list_teams"], + }, + knownEngines: DEFAULT_KNOWN_ENGINES, + availableMcpTools: ["mcp__linear__list_teams"], + agentLabel: "test-worker", + }); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("expected rejection"); + expect(result.unavailable.code).toBe("permission_static"); + }); }); describe("formatCapabilityUnavailable", () => { diff --git a/src/subagent/capability-preflight.ts b/src/subagent/capability-preflight.ts index 1c9a09196..d99700dcb 100644 --- a/src/subagent/capability-preflight.ts +++ b/src/subagent/capability-preflight.ts @@ -25,6 +25,7 @@ import { packageToCapabilities, } from "../agent/directors/registry.js"; import type { CapabilityFilter } from "../agent/profile-types.js"; +import { isMcpToolName } from "../mcp/tool-name.js"; export type CapabilityUnavailableCode = | "missing_tool" @@ -44,6 +45,16 @@ export interface PreflightCapabilitiesInput { * test-only seam for simulating an incomplete runtime. */ knownEngines: readonly string[]; + /** + * Canonical ids of the live inherited-MCP tools the parent session mounted + * (`mcp____`). An `mcp__*` requirement present here passes the + * known-engine and allowlist checks — the worker mount carries it on + * demand (run.ts retains only requested inherited MCP tools), so presence + * here proves the worker mounts it. An `mcp__*` name absent + * from this set still rejects as `unknown_tool`. Fail-closed: availability + * is never inferred from the name shape alone. + */ + availableMcpTools?: readonly string[] | undefined; /** Worker label for messages (director id or profile id). */ agentLabel: string; } @@ -162,6 +173,10 @@ function levenshtein(a: string, b: string): number { } function nearestToolName(raw: string): string | undefined { + // MCP names (`mcp____`) live outside the built-in catalog — + // a typo hint against built-ins would mislead, so MCP-shaped input never + // gets a suggestion. + if (isMcpToolName(raw.trim())) return undefined; const lower = raw.toLowerCase(); let best: string | undefined; let bestDistance = Number.MAX_SAFE_INTEGER; @@ -221,6 +236,9 @@ export function preflightCapabilities( ): CapabilityPreflightResult { const { resolvedFilter, knownEngines } = input; const known = new Set(knownEngines.map((name) => canonicalToolName(name))); + const liveMcp = new Set( + (input.availableMcpTools ?? []).map((name) => canonicalToolName(name)), + ); const allow = resolvedFilter?.mode === "allow" ? new Set(resolvedFilter.tools.map((name) => canonicalToolName(name))) @@ -239,7 +257,12 @@ export function preflightCapabilities( return { ok: false, unavailable: { code: "unknown_tool", tool: raw } }; } const engine = canonicalToolName(trimmed); - if (!catalog.has(engine)) { + // A live inherited-MCP tool passes the catalog check: presence in the + // live set proves the worker mounts it on demand (run.ts retains only + // requested inherited MCP tools). Shape alone proves nothing — an + // `mcp__*` name outside the live set still rejects below. + const isLiveMcp = isMcpToolName(engine) && liveMcp.has(engine); + if (!catalog.has(engine) && !isLiveMcp) { const suggestion = nearestToolName(trimmed); return { ok: false, @@ -250,13 +273,13 @@ export function preflightCapabilities( }, }; } - if (!known.has(engine)) { + if (!isLiveMcp && !known.has(engine)) { return { ok: false, unavailable: { code: "missing_binary", tool: engine }, }; } - if (!postFilter.has(engine)) { + if (!postFilter.has(engine) && !isLiveMcp) { if (allow !== undefined && !allow.has(engine)) { return { ok: false, @@ -277,6 +300,18 @@ export function preflightCapabilities( }, }; } + } else if (isLiveMcp && deny !== undefined && deny.has(engine)) { + // An explicit exclude naming a live MCP tool still withholds it — + // run.ts strips named tools in exclude mode, so the mount would drop + // it and the requirement must reject here, not as a stale snapshot. + return { + ok: false, + unavailable: { + code: "permission_static", + tool: engine, + alternatives: rerouteAlternatives(engine), + }, + }; } if (!seen.has(engine)) { seen.add(engine); diff --git a/src/subagent/run-requires-tools.test.ts b/src/subagent/run-requires-tools.test.ts index d7311da75..981b73a47 100644 --- a/src/subagent/run-requires-tools.test.ts +++ b/src/subagent/run-requires-tools.test.ts @@ -14,9 +14,12 @@ import { tmpdir } from "node:os"; import { mkdtemp } from "node:fs/promises"; import { join } from "node:path"; +import { stringTool } from "@intx/agent"; import { createPermissionGate } from "../permission/gate.js"; -import { runSubAgent } from "./run.js"; +import { applyCapabilityFilter, runSubAgent } from "./run.js"; import type { RunSubAgentParams } from "./types.js"; +import type { AgentTool } from "@intx/agent"; +import type { CapabilityFilter } from "../agent/profiles.js"; const testPermissionGate = createPermissionGate({ approvals: [], @@ -186,4 +189,244 @@ describe("runSubAgent requires_tools mount echo", () => { expect(error).toBeInstanceOf(Error); expect((error as Error).message).not.toContain("stale_snapshot"); }, 15_000); + + test("requested inherited mcp__linear__ tool survives a built-ins-only allowlist", async () => { + const cwd = await tmpCwd(); + const error = await withFailingInference(async (baseURL) => { + try { + await runSubAgent({ + ...baseParams(cwd, baseURL), + capabilities: { mode: "allow", tools: ["read_file", "run_shell"] }, + inheritMcpTools: () => [ + stringTool({ + definition: { + name: "mcp__linear__list_teams", + description: "Inherited Linear tool", + inputSchema: {}, + }, + handler: async () => "teams", + }), + ], + requiresTools: ["mcp__linear__list_teams"], + }); + } catch (err) { + return err; + } + throw new Error("runSubAgent did not throw"); + }); + + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).not.toContain("stale_snapshot"); + }, 15_000); + + test("unmounted mcp__ requirement still throws stale_snapshot setup_error", async () => { + const cwd = await tmpCwd(); + const error = await withFailingInference(async (baseURL) => { + try { + await runSubAgent({ + ...baseParams(cwd, baseURL), + capabilities: { mode: "allow", tools: ["read_file", "run_shell"] }, + requiresTools: ["mcp__linear__list_teams"], + }); + } catch (err) { + return err; + } + throw new Error("runSubAgent did not throw"); + }); + + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toContain("stale_snapshot"); + expect((error as Error).message).toContain("mcp__linear__list_teams"); + }, 15_000); + + test("requested live mcp__ tool mounts while an inherited sibling stays unmounted", async () => { + const cwd = await tmpCwd(); + const error = await withFailingInference(async (baseURL) => { + try { + await runSubAgent({ + ...baseParams(cwd, baseURL), + capabilities: { mode: "allow", tools: ["read_file", "run_shell"] }, + inheritMcpTools: () => [ + mcpTool("mcp__linear__list_teams"), + mcpTool("mcp__linear__create_issue"), + ], + requiresTools: ["mcp__linear__list_teams"], + }); + } catch (err) { + return err; + } + throw new Error("runSubAgent did not throw"); + }); + + // The stamped requirement mounted (no stale_snapshot); the sibling's + // absence is pinned by the applyCapabilityFilter unit tests below. + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).not.toContain("stale_snapshot"); + }, 15_000); + + test("exclude naming a requested live mcp__ tool throws stale_snapshot setup_error", async () => { + const cwd = await tmpCwd(); + const error = await withFailingInference(async (baseURL) => { + try { + await runSubAgent({ + ...baseParams(cwd, baseURL), + capabilities: { + mode: "exclude", + tools: ["mcp__linear__list_teams"], + }, + inheritMcpTools: () => [mcpTool("mcp__linear__list_teams")], + requiresTools: ["mcp__linear__list_teams"], + }); + } catch (err) { + return err; + } + throw new Error("runSubAgent did not throw"); + }); + + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toContain("stale_snapshot"); + expect((error as Error).message).toContain("mcp__linear__list_teams"); + }, 15_000); +}); + +function mcpTool(name: string): AgentTool { + return stringTool({ + definition: { + name, + description: `Inherited ${name}`, + inputSchema: {}, + }, + handler: async () => name, + }); +} + +function builtinTool(name: string): AgentTool { + return stringTool({ + definition: { + name, + description: `Built-in ${name}`, + inputSchema: {}, + }, + handler: async () => name, + }); +} + +function filteredNames( + tools: AgentTool[], + capabilities: CapabilityFilter | undefined, + requiresTools?: readonly string[], + inheritedMcpTools?: readonly string[], +): string[] { + return applyCapabilityFilter( + tools, + capabilities, + requiresTools, + inheritedMcpTools, + ).map((tool) => tool.definition.name); +} + +describe("applyCapabilityFilter on-demand MCP mounting", () => { + const allowBuild: CapabilityFilter = { + mode: "allow", + tools: ["read_file", "run_shell"], + }; + const tools: AgentTool[] = [ + builtinTool("read_file"), + builtinTool("run_shell"), + mcpTool("mcp__linear__list_teams"), + mcpTool("mcp__linear__create_issue"), + ]; + + test("allowlist mounts only the requested MCP tool, never the inherited set", () => { + expect( + filteredNames(tools, allowBuild, ["mcp__linear__list_teams"]), + ).toEqual(["read_file", "run_shell", "mcp__linear__list_teams"]); + }); + + test("allowlist with no requires_tools mounts no MCP tools", () => { + expect(filteredNames(tools, allowBuild)).toEqual([ + "read_file", + "run_shell", + ]); + }); + + test("allowlist naming an MCP tool mounts it without a requires_tools stamp", () => { + const allowWithMcp: CapabilityFilter = { + mode: "allow", + tools: ["read_file", "mcp__linear__create_issue"], + }; + expect(filteredNames(tools, allowWithMcp)).toEqual([ + "read_file", + "mcp__linear__create_issue", + ]); + }); + + test("exclude keeps a requested live MCP tool unless named explicitly", () => { + const excludeOther: CapabilityFilter = { + mode: "exclude", + tools: ["run_shell"], + }; + expect( + filteredNames(tools, excludeOther, ["mcp__linear__list_teams"]), + ).toEqual(["read_file", "mcp__linear__list_teams"]); + }); + + test("exclude naming a requested live MCP tool withholds it", () => { + const excludeMcp: CapabilityFilter = { + mode: "exclude", + tools: ["mcp__linear__list_teams"], + }; + expect( + filteredNames(tools, excludeMcp, ["mcp__linear__list_teams"]), + ).toEqual(["read_file", "run_shell"]); + }); + + test("exclude with no requires_tools mounts no MCP tools", () => { + const excludeOther: CapabilityFilter = { + mode: "exclude", + tools: ["run_shell"], + }; + expect(filteredNames(tools, excludeOther)).toEqual(["read_file"]); + }); + + test("full mount mounts only the requested MCP tool", () => { + expect( + filteredNames(tools, undefined, ["mcp__linear__list_teams"]), + ).toEqual(["read_file", "run_shell", "mcp__linear__list_teams"]); + }); + + test("full mount with no requires_tools mounts no MCP tools", () => { + expect(filteredNames(tools, undefined)).toEqual(["read_file", "run_shell"]); + }); + + test("full mount retains the inherited MCP set without a stamp", () => { + expect( + filteredNames(tools, undefined, undefined, [ + "mcp__linear__list_teams", + "mcp__linear__create_issue", + ]), + ).toEqual([ + "read_file", + "run_shell", + "mcp__linear__list_teams", + "mcp__linear__create_issue", + ]); + }); + + test("full mount drops an mcp__ name outside the inherited set", () => { + expect( + filteredNames(tools, undefined, undefined, ["mcp__linear__list_teams"]), + ).toEqual(["read_file", "run_shell", "mcp__linear__list_teams"]); + }); + + test("full mount with a stamp mounts only the stamped inherited tool", () => { + expect( + filteredNames( + tools, + undefined, + ["mcp__linear__list_teams"], + ["mcp__linear__list_teams", "mcp__linear__create_issue"], + ), + ).toEqual(["read_file", "run_shell", "mcp__linear__list_teams"]); + }); }); diff --git a/src/subagent/run.ts b/src/subagent/run.ts index 3d8db699e..c9e2cf6d5 100644 --- a/src/subagent/run.ts +++ b/src/subagent/run.ts @@ -67,6 +67,7 @@ import { } from "./intervention-log.js"; import { normalizeToolDefinitionsForProvider } from "../agent/tool-schema-normalize.js"; import { canonicalToolName } from "../agent/canonical-tool-name.js"; +import { isMcpToolName } from "../mcp/tool-name.js"; import { createApplyPatchTool } from "../agent/apply-patch-tool.js"; import { createWorktreeRootsProvider } from "../permission/worktree-roots.js"; import { @@ -378,19 +379,65 @@ export function coreSubAgentWebTools( ); } -function applyCapabilityFilter( +/** + * Capability allowlist/exclude over the assembled worker tool set, with + * on-demand inherited-MCP mounting (CL-9476 follow-up). + * + * Inherited MCP tools (`mcp____`) mount only when the dispatch + * requested them. An explicit stamp always counts: a `requiresTools` entry + * (the requires_tools gate, validated pre-spawn against the live inherited + * set) or an allowlist naming in `capabilities.tools`. Inheritance itself + * also counts: names in `inheritedMcpTools` — the live set returned by + * `inheritMcpTools` for this worker — mount without an extra stamp when no + * narrower constraint applies (no `capabilities`, no `requiresTools`), so + * the inherit-by-default contract holds and the worker gate (parent grant) + * governs execution. An explicit stamp narrows: with `requiresTools` + * present only stamped MCP names mount, even inherited siblings. An + * `mcp__*` name outside both sets never mounts. An explicit exclude still + * withholds a requested live MCP tool (surfacing downstream as a stale + * snapshot, normally pre-empted by the dispatch preflight). + */ +export function applyCapabilityFilter( tools: AgentTool[], - capabilities: CapabilityFilter, + capabilities: CapabilityFilter | undefined, + requiresTools?: readonly string[] | undefined, + inheritedMcpTools?: readonly string[] | undefined, ): AgentTool[] { + const required = new Set( + (requiresTools ?? []).map((name) => canonicalToolName(name)), + ); + const inherited = new Set( + (inheritedMcpTools ?? []).map((name) => canonicalToolName(name)), + ); + if (capabilities === undefined) { + if (required.size === 0) { + return tools.filter((t) => { + const name = canonicalToolName(t.definition.name); + if (!isMcpToolName(name)) return true; + return inherited.has(name); + }); + } + return tools.filter((t) => { + const name = canonicalToolName(t.definition.name); + if (isMcpToolName(name)) return required.has(name); + return true; + }); + } const engines = new Set( capabilities.tools.map((name) => canonicalToolName(name)), ); if (capabilities.mode === "exclude") { - return tools.filter( - (t) => !engines.has(canonicalToolName(t.definition.name)), - ); + return tools.filter((t) => { + const name = canonicalToolName(t.definition.name); + if (isMcpToolName(name)) return required.has(name) && !engines.has(name); + return !engines.has(name); + }); } - return tools.filter((t) => engines.has(canonicalToolName(t.definition.name))); + return tools.filter((t) => { + const name = canonicalToolName(t.definition.name); + if (isMcpToolName(name)) return required.has(name) || engines.has(name); + return engines.has(name); + }); } export interface SubAgentRunController { @@ -801,9 +848,14 @@ async function runSubAgentInner( ), ]; - if (params.capabilities !== undefined) { - tools = applyCapabilityFilter(tools, params.capabilities); - } + tools = applyCapabilityFilter( + tools, + params.capabilities, + params.requiresTools, + inherited + .map((tool) => canonicalToolName(tool.definition.name)) + .filter((name) => isMcpToolName(name)), + ); if ( toolProfileForModel(params.provider) === "gpt" && tools.some((t) =>