Merge remote-tracking branch 'origin/farm/7a22e7a4/mcp-toggle-reconnects-others'

2026-06-21 02:18:29 +02:00
parent 0f976084b8 c18d400bdf
commit e31bfa4b96
3 changed files with 180 additions and 12 deletions
@@ -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
@@ -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<void> {
+		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<string, string>): 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);
 	}

 	/**
@@ -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<string, string>() })),
+		disconnectServer: vi.fn(async () => {}),
+		connectServers: vi.fn(
+			async (_configs: Record<string, MCPServerConfig>, _sources: Record<string, SourceMeta>) => ({
+				errors: new Map<string, string>(),
+				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<string, MCPServerConfig>): Promise<void> {
+	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 });
+	});
+});