From c18d400bdf2b603c042e7abc001e9fe007966cd3 Mon Sep 17 00:00:00 2001 From: roboomp Date: Sat, 20 Jun 2026 23:58:03 +0000 Subject: [PATCH] fix(cli): scoped mcp toggles to one server Updated /mcp enable and /mcp disable so they connect or disconnect only the named server instead of reloading every MCP server in the session. Added regression coverage for both toggle directions and updated the coding-agent changelog. Fixes #3157 --- packages/coding-agent/CHANGELOG.md | 4 + .../controllers/mcp-command-controller.ts | 52 +++++-- .../test/mcp-command-toggle.test.ts | 136 ++++++++++++++++++ 3 files changed, 180 insertions(+), 12 deletions(-) create mode 100644 packages/coding-agent/test/mcp-command-toggle.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 65b2ee52b..2c63b4920 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `/mcp enable` and `/mcp disable` reconnecting unrelated MCP servers by scoping toggle reconnect/disconnect work to the named server. ([#3157](https://github.com/can1357/oh-my-pi/issues/3157)) + ## [16.1.8] - 2026-06-20 ### Added diff --git a/packages/coding-agent/src/modes/controllers/mcp-command-controller.ts b/packages/coding-agent/src/modes/controllers/mcp-command-controller.ts index 9b32edae8..7dbd2a686 100644 --- a/packages/coding-agent/src/modes/controllers/mcp-command-controller.ts +++ b/packages/coding-agent/src/modes/controllers/mcp-command-controller.ts @@ -8,7 +8,7 @@ import { type Component, replaceTabs, Spacer, Text } from "@oh-my-pi/pi-tui"; import { getMCPConfigPath, getProjectDir } from "@oh-my-pi/pi-utils"; import type { SourceMeta } from "../../capability/types"; import { expandEnvVarsDeep } from "../../discovery/helpers"; -import { analyzeAuthError, discoverOAuthEndpoints, MCPManager } from "../../mcp"; +import { analyzeAuthError, discoverOAuthEndpoints, loadAllMCPConfigs, MCPManager } from "../../mcp"; import { connectToServer, disconnectServer, listTools } from "../../mcp/client"; import { addMCPServer, @@ -1383,7 +1383,7 @@ export class MCPCommandController { } await setServerDisabled(userConfigPath, name, !enabled); if (enabled) { - await this.#reloadMCP(); + await this.#connectEnabledMCPServer(name); const state = await this.#waitForServerConnectionWithAnimation(name); const status = state === "connected" @@ -1419,7 +1419,12 @@ export class MCPCommandController { const updated: MCPServerConfig = { ...found.config, enabled }; await updateMCPServer(found.filePath, name, updated); - await this.#reloadMCP(); + if (enabled) { + await this.#connectEnabledMCPServer(name); + } else { + await this.ctx.mcpManager?.disconnectServer(name); + await this.ctx.session.refreshMCPTools(this.ctx.mcpManager?.getTools() ?? []); + } let status = ""; if (enabled) { @@ -1671,6 +1676,37 @@ export class MCPCommandController { } } + async #connectEnabledMCPServer(name: string): Promise { + if (!this.ctx.mcpManager) { + return; + } + + const { configs, sources } = await loadAllMCPConfigs(getProjectDir()); + const config = configs[name]; + if (!config) { + await this.ctx.session.refreshMCPTools(this.ctx.mcpManager.getTools()); + return; + } + + const source = sources[name]; + const result = await this.ctx.mcpManager.connectServers({ [name]: config }, source ? { [name]: source } : {}); + await this.ctx.session.refreshMCPTools(this.ctx.mcpManager.getTools()); + this.#showMCPConnectionErrors(result.errors); + } + + #showMCPConnectionErrors(errors: Map): void { + if (errors.size === 0) { + return; + } + + const errorLines = ["", theme.fg("warning", "Some servers failed to connect:"), ""]; + for (const [serverName, error] of errors.entries()) { + errorLines.push(` ${serverName}: ${error}`); + } + errorLines.push(""); + this.#showMessage(errorLines.join("\n")); + } + /** * Reload MCP manager with new configs */ @@ -1686,15 +1722,7 @@ export class MCPCommandController { const result = await this.ctx.mcpManager.discoverAndConnect(); await this.ctx.session.refreshMCPTools(this.ctx.mcpManager.getTools()); - // Show any connection errors - if (result.errors.size > 0) { - const errorLines = ["", theme.fg("warning", "Some servers failed to connect:"), ""]; - for (const [serverName, error] of result.errors.entries()) { - errorLines.push(` ${serverName}: ${error}`); - } - errorLines.push(""); - this.#showMessage(errorLines.join("\n")); - } + this.#showMCPConnectionErrors(result.errors); } /** diff --git a/packages/coding-agent/test/mcp-command-toggle.test.ts b/packages/coding-agent/test/mcp-command-toggle.test.ts new file mode 100644 index 000000000..5f940ab5f --- /dev/null +++ b/packages/coding-agent/test/mcp-command-toggle.test.ts @@ -0,0 +1,136 @@ +import { afterEach, beforeAll, beforeEach, describe, expect, test, vi } from "bun:test"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import type { SourceMeta } from "@oh-my-pi/pi-coding-agent/capability/types"; +import type { MCPServerConfig } from "@oh-my-pi/pi-coding-agent/mcp/types"; +import { MCPCommandController } from "@oh-my-pi/pi-coding-agent/modes/controllers/mcp-command-controller"; +import { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; +import { getConfigRootDir, getMCPConfigPath, getProjectDir, setAgentDir, setProjectDir } from "@oh-my-pi/pi-utils"; + +const originalProjectDir = getProjectDir(); +const originalAgentDir = process.env.PI_CODING_AGENT_DIR; +const fallbackAgentDir = path.join(getConfigRootDir(), "agent"); + +function restoreAgentDir(): void { + if (originalAgentDir) { + setAgentDir(originalAgentDir); + process.env.PI_CODING_AGENT_DIR = originalAgentDir; + Bun.env.PI_CODING_AGENT_DIR = originalAgentDir; + return; + } + setAgentDir(fallbackAgentDir); + delete process.env.PI_CODING_AGENT_DIR; + delete Bun.env.PI_CODING_AGENT_DIR; +} + +function createController() { + const refreshMCPTools = vi.fn(async () => {}); + const mcpManager = { + disconnectAll: vi.fn(async () => {}), + discoverAndConnect: vi.fn(async () => ({ errors: new Map() })), + disconnectServer: vi.fn(async () => {}), + connectServers: vi.fn( + async (_configs: Record, _sources: Record) => ({ + errors: new Map(), + connectedServers: [], + tools: [], + exaApiKeys: [], + }), + ), + getTools: vi.fn(() => []), + waitForConnection: vi.fn(async () => ({})), + getConnectionStatus: vi.fn(() => "connected"), + getSource: vi.fn(() => undefined), + }; + const controller = new MCPCommandController({ + chatContainer: { addChild: vi.fn() }, + present: vi.fn(), + ui: { requestRender: vi.fn() }, + editor: {}, + showError: vi.fn(), + showStatus: vi.fn(), + oauthManualInput: { + hasPending: vi.fn(() => false), + pendingProviderId: undefined, + tryClaimInput: vi.fn(), + }, + session: { + refreshMCPTools, + modelRegistry: { authStorage: undefined }, + }, + mcpManager, + } as never); + + return { controller, mcpManager, refreshMCPTools }; +} + +async function writeProjectConfig(projectDir: string, servers: Record): Promise { + await Bun.write( + getMCPConfigPath("project", projectDir), + `${JSON.stringify( + { + mcpServers: servers, + }, + null, + 2, + )}\n`, + ); +} + +describe("/mcp enable and disable", () => { + let projectDir = ""; + let agentDir = ""; + + beforeAll(() => { + initTheme(); + }); + + beforeEach(async () => { + projectDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-mcp-toggle-project-")); + agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-mcp-toggle-agent-")); + setProjectDir(projectDir); + setAgentDir(agentDir); + }); + + afterEach(async () => { + vi.restoreAllMocks(); + setProjectDir(originalProjectDir); + restoreAgentDir(); + await fs.rm(projectDir, { recursive: true, force: true }); + await fs.rm(agentDir, { recursive: true, force: true }); + }); + + test("disabling one configured server does not reload other MCP servers", async () => { + await writeProjectConfig(projectDir, { + mcp1: { type: "stdio", command: "mcp-one" }, + mcp2: { type: "stdio", command: "mcp-two" }, + }); + const { controller, mcpManager, refreshMCPTools } = createController(); + + await controller.handle("/mcp disable mcp1"); + + expect(mcpManager.disconnectServer).toHaveBeenCalledWith("mcp1"); + expect(refreshMCPTools).toHaveBeenCalledWith([]); + expect(mcpManager.disconnectAll).not.toHaveBeenCalled(); + expect(mcpManager.discoverAndConnect).not.toHaveBeenCalled(); + expect(mcpManager.connectServers).not.toHaveBeenCalled(); + }); + + test("enabling one configured server connects only that MCP server", async () => { + await writeProjectConfig(projectDir, { + mcp1: { type: "stdio", command: "mcp-one", enabled: false }, + mcp2: { type: "stdio", command: "mcp-two" }, + }); + const { controller, mcpManager } = createController(); + + await controller.handle("/mcp enable mcp1"); + + expect(mcpManager.disconnectAll).not.toHaveBeenCalled(); + expect(mcpManager.discoverAndConnect).not.toHaveBeenCalled(); + expect(mcpManager.connectServers).toHaveBeenCalledTimes(1); + const [configs] = mcpManager.connectServers.mock.calls[0]!; + expect(Object.keys(configs)).toEqual(["mcp1"]); + expect(configs.mcp1).toEqual({ type: "stdio", command: "mcp-one", enabled: true }); + }); +});