From 0d606f2f3002d76238384072270c2cf0f0b48930 Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 10 Jul 2026 01:21:33 +0000 Subject: [PATCH] fix(cli): preserved mcp tools with tools filter Interactive sessions defer MCP discovery, so CLI --tools produced an initial built-in-only active set and later MCP refreshes respected that filtered set. Force-activate deferred MCP tools when MCP discovery mode is disabled, matching the blocking startup path while leaving discovery-mode selection intact. Fixes #5013 --- packages/coding-agent/CHANGELOG.md | 4 ++ packages/coding-agent/src/sdk.ts | 14 ++---- .../coding-agent/src/session/agent-session.ts | 12 +++--- .../test/sdk-mcp-instructions.test.ts | 43 ++++++++++++++++++- 4 files changed, 55 insertions(+), 18 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 6c5d526e2..0d8d21bfb 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `--tools` filtering in interactive sessions disabling deferred MCP tools; MCP tools discovered from configured servers now stay active when the flag limits only built-in tools. ([#5013](https://github.com/can1357/oh-my-pi/issues/5013)) + ## [16.3.15] - 2026-07-09 ### Changed diff --git a/packages/coding-agent/src/sdk.ts b/packages/coding-agent/src/sdk.ts index 0d27eac07..fa87bf6c3 100644 --- a/packages/coding-agent/src/sdk.ts +++ b/packages/coding-agent/src/sdk.ts @@ -1763,7 +1763,7 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} // Discovery flipped on mid-flight: route the explicit request // through discovery-aware activation so selection persists. await liveSession.activateDiscoveredMCPTools(activation.explicitlyRequestedMCPToolNames); - } else if (!discoveryEnabled) { + } else if (!discoveryEnabled && !activateAll) { await liveSession.setActiveToolsByName([ ...liveSession.getActiveToolNames(), ...activation.explicitlyRequestedMCPToolNames, @@ -3055,16 +3055,8 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} try { await session.refreshMCPTools( tools, - deferMCPDiscoveryForUI && !mcpDiscoveryEnabled && options.toolNames === undefined - ? { activateAll: true } - : undefined, + deferMCPDiscoveryForUI && !mcpDiscoveryEnabled ? { activateAll: true } : undefined, ); - if (deferMCPDiscoveryForUI && !mcpDiscoveryEnabled && explicitlyRequestedMCPToolNames.length > 0) { - await session.setActiveToolsByName([ - ...session.getActiveToolNames(), - ...explicitlyRequestedMCPToolNames, - ]); - } } catch (error) { logger.warn("MCP tool refresh failed", { error: error instanceof Error ? error.message : String(error), @@ -3106,7 +3098,7 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} startDeferredMCPDiscovery?.(session, { mcpDiscoveryEnabled, explicitlyRequestedMCPToolNames, - activateAllMCPTools: !mcpDiscoveryEnabled && options.toolNames === undefined, + activateAllMCPTools: !mcpDiscoveryEnabled, }); return { diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 3b4274e5f..31cca6cfd 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -6593,8 +6593,8 @@ export class AgentSession { * * @param mcpTools The new MCP tools to register. * @param options.activateAll When true, force-activates every newly registered MCP tool - * regardless of prior selection state. Used when an ACP client provisions MCP servers - * for a session where MCP discovery is disabled. + * regardless of prior selection state. Used when MCP discovery is disabled and tools + * arrive after initial session activation. */ async refreshMCPTools(mcpTools: CustomTool[], options?: { activateAll?: boolean }): Promise { const previousSelectedMCPToolNames = this.getSelectedMCPToolNames(); @@ -6639,10 +6639,10 @@ export class AgentSession { if (options?.activateAll) { // Force-activate every newly registered MCP tool. This path is used - // when an ACP client provisions MCP servers for a session where MCP - // discovery is disabled — without it, getSelectedMCPToolNames() - // returns only already-active tools (circular deadlock: tools can - // only become active if they're already active). + // when MCP discovery is disabled and tools arrive after initial + // activation — without it, getSelectedMCPToolNames() returns only + // already-active tools (circular deadlock: tools can only become + // active if they're already active). const newMcpNames = mcpTools.map(t => t.name); const nextActive = [...new Set([...this.#getActiveNonMCPToolNames(), ...newMcpNames])]; await this.#applyActiveToolsByName(nextActive, { previousSelectedMCPToolNames }); diff --git a/packages/coding-agent/test/sdk-mcp-instructions.test.ts b/packages/coding-agent/test/sdk-mcp-instructions.test.ts index 686351288..042afb217 100644 --- a/packages/coding-agent/test/sdk-mcp-instructions.test.ts +++ b/packages/coding-agent/test/sdk-mcp-instructions.test.ts @@ -9,7 +9,7 @@ import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { createAgentSession } from "@oh-my-pi/pi-coding-agent/sdk"; import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager"; import { removeSyncWithRetries, Snowflake } from "@oh-my-pi/pi-utils"; -import { SERVER_INSTRUCTIONS } from "./fixtures/instructions-mcp"; +import { SERVER_INSTRUCTIONS, TOOL_NAME } from "./fixtures/instructions-mcp"; // Contract: a deferred interactive (`hasUI`) session runs MCP discovery off the // first-paint path, so an MCP server's `instructions` are not available when the @@ -19,6 +19,7 @@ import { SERVER_INSTRUCTIONS } from "./fixtures/instructions-mcp"; // guard: a prior version gated instruction inclusion on `!deferMCPDiscoveryForUI`, // which dropped server instructions permanently for every UI session. const FIXTURE_PATH = path.join(import.meta.dir, "fixtures", "instructions-mcp.ts"); +const MCP_TOOL_NAME = `mcp__instr_${TOOL_NAME}`; describe("createAgentSession MCP server instructions (deferred UI)", () => { let registryDir: string; @@ -113,4 +114,44 @@ describe("createAgentSession MCP server instructions (deferred UI)", () => { await session.dispose(); } }, 20_000); + + it("keeps MCP tools active after deferred discovery when CLI tool filtering names only built-ins", async () => { + const { session } = await createAgentSession({ + cwd: tempDir, + agentDir: tempDir, + modelRegistry, + sessionManager: SessionManager.inMemory(), + settings: Settings.isolated({}), + model: getBundledModel("openai", "gpt-4o-mini"), + disableExtensionDiscovery: true, + skills: [], + contextFiles: [], + promptTemplates: [], + slashCommands: [], + enableLsp: false, + skipPythonPreflight: true, + enableMCP: true, + hasUI: true, + toolNames: ["read"], + }); + try { + expect(session.getActiveToolNames()).toContain("read"); + + // Deferred MCP discovery is fire-and-forget and exposes no promise or + // event; fake timers cannot drive the real subprocess handshake, so we + // poll the live active-tool state and exit as soon as the fixture tool + // appears. + const deadline = Date.now() + 12_000; + let activeToolNames = session.getActiveToolNames(); + while (!activeToolNames.includes(MCP_TOOL_NAME) && Date.now() < deadline) { + await Bun.sleep(50); + activeToolNames = session.getActiveToolNames(); + } + + expect(activeToolNames).toContain("read"); + expect(activeToolNames).toContain(MCP_TOOL_NAME); + } finally { + await session.dispose(); + } + }, 20_000); });