From 2be93e7c8409d72cb6902fea7edfa548cb800d2b Mon Sep 17 00:00:00 2001 From: can1357 Date: Wed, 29 Jul 2026 22:53:12 +0200 Subject: [PATCH] fix(coding-agent): filter disabled MCP completions (cherry picked from commit 33886ad9691cf4d330cf1a645e19cae7681c8e94) --- .../controllers/mcp-command-controller.ts | 34 +++++++------- .../src/slash-commands/builtin-registry.ts | 47 ++++++++++--------- .../test/mcp-name-autocomplete.test.ts | 17 +++++++ 3 files changed, 61 insertions(+), 37 deletions(-) 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 fb6241592..f32e58133 100644 --- a/packages/coding-agent/src/modes/controllers/mcp-command-controller.ts +++ b/packages/coding-agent/src/modes/controllers/mcp-command-controller.ts @@ -259,16 +259,11 @@ type MCPSearchParsed = { * covers connections, pending connections, and discovered-but-not-yet- * connected sources). * - * When `includeDisabled` is true (the default), disabled-server entries - * are unioned in too — disabling a server only flips its config `enabled` - * flag, it doesn't remove the config entry, so a disabled server is still - * a valid `/mcp enable ` target. This also covers a discovered - * (non-config) server that was disabled: `loadAllMCPConfigs` filters it - * out of `getAllServerNames()`, but its name survives in - * `userConfig.disabledServers`. Callers whose target operation needs a - * live connection or config entry (`/mcp test`/`reconnect`/`reauth`/ - * `unauth`) — which a disabled-only name can never satisfy — must pass - * `includeDisabled: false`. + * `includeDisabledOnly` controls names found only in + * `userConfig.disabledServers`, while `includeDisabledConfigured` controls + * config entries whose `enabled` flag is false. Both default to true because + * callers such as `/mcp list` need the complete union. Autocomplete callers + * must disable the categories their target operation cannot accept. * * This is the single source of truth for "every known server name": both * `MCPCommandController#handleList()` and the `/mcp` slash-command argument @@ -281,7 +276,8 @@ type MCPSearchParsed = { export async function collectMcpServerNames( ctx: InteractiveModeContext, preloaded?: { userConfig: MCPConfigFile; projectConfig: MCPConfigFile }, - includeDisabled = true, + includeDisabledOnly = true, + includeDisabledConfigured = true, ): Promise { let userConfig: MCPConfigFile; let projectConfig: MCPConfigFile; @@ -295,11 +291,17 @@ export async function collectMcpServerNames( ]); } - const names = new Set([ - ...Object.keys(userConfig.mcpServers ?? {}), - ...Object.keys(projectConfig.mcpServers ?? {}), - ...(includeDisabled ? (userConfig.disabledServers ?? []) : []), - ]); + const names = new Set(includeDisabledOnly ? (userConfig.disabledServers ?? []) : []); + const addConfiguredNames = (config: MCPConfigFile): void => { + const servers = config.mcpServers; + if (!servers) return; + for (const name in servers) { + const server = servers[name]; + if (server && (includeDisabledConfigured || server.enabled !== false)) names.add(name); + } + }; + addConfiguredNames(userConfig); + addConfiguredNames(projectConfig); if (ctx.mcpManager) { for (const name of ctx.mcpManager.getAllServerNames()) { names.add(name); diff --git a/packages/coding-agent/src/slash-commands/builtin-registry.ts b/packages/coding-agent/src/slash-commands/builtin-registry.ts index 80235d96f..044e77b11 100644 --- a/packages/coding-agent/src/slash-commands/builtin-registry.ts +++ b/packages/coding-agent/src/slash-commands/builtin-registry.ts @@ -2717,27 +2717,32 @@ function buildArgumentCompletions(subcommands: SubcommandDef[]): (prefix: string } /** /mcp subcommands whose argument is a server name (per their `usage: "..."`). */ -const MCP_SERVER_NAME_SUBCOMMANDS: ReadonlySet = new Set([ - "enable", - "disable", - "test", - "remove", - "reconnect", - "reauth", - "unauth", -]); +const MCP_SERVER_NAME_SUBCOMMANDS: Readonly> = { + enable: true, + disable: true, + test: true, + remove: true, + reconnect: true, + reauth: true, + unauth: true, +}; + +/** Subcommands that accept names found only in `userConfig.disabledServers`. */ +const MCP_DISABLED_ONLY_ELIGIBLE_SUBCOMMANDS: Readonly> = { + enable: true, + disable: true, +}; /** - * `/mcp` subcommands where a discovered server disabled via `/mcp disable` - * (name only in `userConfig.disabledServers`, dropped from - * `mcpManager.getAllServerNames()` by `loadAllMCPConfigs`) is still a valid - * completion target: `enable` is the primary re-enable path, and offering - * it for `disable` is a harmless no-op (`#handleSetEnabled` reports - * "already disabled"). The rest (`test`/`reconnect`/`reauth`/`unauth`) need - * a live connection or config entry that a disabled-only name never has — - * `#resolveServerForAuth`/`reconnectServer` would report it as not found. + * Subcommands that accept configured servers whose `enabled` flag is false. + * `unauth` can clear persisted credentials without connecting; test, + * reconnect, and reauth explicitly require an enabled server. */ -const MCP_DISABLED_ONLY_ELIGIBLE_SUBCOMMANDS: ReadonlySet = new Set(["enable", "disable"]); +const MCP_DISABLED_CONFIG_ELIGIBLE_SUBCOMMANDS: Readonly> = { + enable: true, + disable: true, + unauth: true, +}; /** * Build getArgumentCompletions for /mcp. Delegates to the generic @@ -2762,8 +2767,7 @@ function buildMcpArgumentCompletions( const rawSubcommand = argumentPrefix.slice(0, spaceIndex); const lowerSubcommand = rawSubcommand.toLowerCase(); - if (!MCP_SERVER_NAME_SUBCOMMANDS.has(lowerSubcommand)) return null; - + if (MCP_SERVER_NAME_SUBCOMMANDS[lowerSubcommand] !== true) return null; const namePrefix = argumentPrefix.slice(spaceIndex + 1).toLowerCase(); if (lowerSubcommand === "remove") { return await buildMcpRemoveCompletions(rawSubcommand, namePrefix); @@ -2774,7 +2778,8 @@ function buildMcpArgumentCompletions( serverNames = await collectMcpServerNames( runtime.ctx, undefined, - MCP_DISABLED_ONLY_ELIGIBLE_SUBCOMMANDS.has(lowerSubcommand), + MCP_DISABLED_ONLY_ELIGIBLE_SUBCOMMANDS[lowerSubcommand] === true, + MCP_DISABLED_CONFIG_ELIGIBLE_SUBCOMMANDS[lowerSubcommand] === true, ); } catch (error) { logger.warn("MCP server-name autocomplete failed to read config", { error }); diff --git a/packages/coding-agent/test/mcp-name-autocomplete.test.ts b/packages/coding-agent/test/mcp-name-autocomplete.test.ts index 953c159e5..e8245b109 100644 --- a/packages/coding-agent/test/mcp-name-autocomplete.test.ts +++ b/packages/coding-agent/test/mcp-name-autocomplete.test.ts @@ -160,6 +160,23 @@ describe("MCP server-name autocomplete", () => { expect(await mcp.getArgumentCompletions("unauth ")).toBeNull(); }); + test("/mcp only offers disabled configured servers to subcommands that can accept them", async () => { + await writeConfig("user", projectDir, { + disabled: { type: "stdio", command: "disabled", enabled: false }, + }); + const { ctx } = createFakeCtx([]); + const runtime: TuiSlashCommandRuntime = { ctx }; + const mcp = buildTuiBuiltinSlashCommands(runtime).find(c => c.name === "mcp"); + if (!mcp?.getArgumentCompletions) throw new Error("expected /mcp command with getArgumentCompletions"); + + expect((await mcp.getArgumentCompletions("enable "))?.map(item => item.label)).toEqual(["disabled"]); + expect((await mcp.getArgumentCompletions("disable "))?.map(item => item.label)).toEqual(["disabled"]); + expect((await mcp.getArgumentCompletions("unauth "))?.map(item => item.label)).toEqual(["disabled"]); + expect(await mcp.getArgumentCompletions("test ")).toBeNull(); + expect(await mcp.getArgumentCompletions("reconnect ")).toBeNull(); + expect(await mcp.getArgumentCompletions("reauth ")).toBeNull(); + }); + test("/mcp getArgumentCompletions returns null for subcommands that don't take a server name", async () => { await writeConfig("user", projectDir, { "my-server": { type: "stdio", command: "one" } }); const { ctx } = createFakeCtx([]);