diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 6ffed67c4..a43200ec3 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -8,6 +8,10 @@ - Fixed lazy-initialized LSP servers (basedpyright/pyright, and likely gopls/rust-analyzer) hanging on the first request: the message reader matched incoming messages against pending client requests by id before checking for a `method`, so a server-originated `workspace/configuration` pull whose id collided with an in-flight request was swallowed as a bogus response, leaving the pull unanswered and the server wedged. The reader now routes any message carrying a `method` as a server request before id-matching ([#3001](https://github.com/can1357/oh-my-pi/issues/3001)) - Fixed `omp --approval-mode=yolo acp` and other global option flags placed before a subcommand being rewritten to `launch` with the subcommand swallowed as prompt text; the CLI resolver now skips leading global flags (using the launch parser's value-consumption contract) and dispatches the real subcommand with the flags applied, so ACP mode honors the configured approval policy. ([#2970](https://github.com/can1357/oh-my-pi/issues/2970)) +### 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 }); + }); +});