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
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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<void> {
|
||||
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 });
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user