fix(coding-agent): filter disabled MCP completions
(cherry picked from commit 33886ad9691cf4d330cf1a645e19cae7681c8e94)
This commit is contained in:
@@ -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 <name>` 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<string[]> {
|
||||
let userConfig: MCPConfigFile;
|
||||
let projectConfig: MCPConfigFile;
|
||||
@@ -295,11 +291,17 @@ export async function collectMcpServerNames(
|
||||
]);
|
||||
}
|
||||
|
||||
const names = new Set<string>([
|
||||
...Object.keys(userConfig.mcpServers ?? {}),
|
||||
...Object.keys(projectConfig.mcpServers ?? {}),
|
||||
...(includeDisabled ? (userConfig.disabledServers ?? []) : []),
|
||||
]);
|
||||
const names = new Set<string>(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);
|
||||
|
||||
@@ -2717,27 +2717,32 @@ function buildArgumentCompletions(subcommands: SubcommandDef[]): (prefix: string
|
||||
}
|
||||
|
||||
/** /mcp subcommands whose argument is a server name (per their `usage: "<name>..."`). */
|
||||
const MCP_SERVER_NAME_SUBCOMMANDS: ReadonlySet<string> = new Set([
|
||||
"enable",
|
||||
"disable",
|
||||
"test",
|
||||
"remove",
|
||||
"reconnect",
|
||||
"reauth",
|
||||
"unauth",
|
||||
]);
|
||||
const MCP_SERVER_NAME_SUBCOMMANDS: Readonly<Record<string, true>> = {
|
||||
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<Record<string, true>> = {
|
||||
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<string> = new Set(["enable", "disable"]);
|
||||
const MCP_DISABLED_CONFIG_ELIGIBLE_SUBCOMMANDS: Readonly<Record<string, true>> = {
|
||||
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 });
|
||||
|
||||
@@ -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([]);
|
||||
|
||||
Reference in New Issue
Block a user