From 5bb428287875e50c5b7d2a956f3df6eb39664d29 Mon Sep 17 00:00:00 2001 From: Mathews-Tom Date: Fri, 24 Jul 2026 06:01:08 +0530 Subject: [PATCH] feat(coding-agent): autocomplete MCP server names in /mcp subcommands Adds an exported collectMcpServerNames() helper (used by both /mcp list and the new completer) and a runtime-bound getArgumentCompletions for /mcp that resolves server names after enable/disable/test/remove/ reconnect/reauth/unauth, filtered by the typed prefix. Subcommands with a different argument shape (add, smithery-search, ...) keep returning null, and subcommand-name completion is unaffected. Closes #5654. --- packages/coding-agent/CHANGELOG.md | 1 + .../controllers/mcp-command-controller.ts | 36 ++++- .../src/slash-commands/builtin-registry.ts | 47 ++++++- .../test/mcp-name-autocomplete.test.ts | 128 ++++++++++++++++++ 4 files changed, 210 insertions(+), 2 deletions(-) create mode 100644 packages/coding-agent/test/mcp-name-autocomplete.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 1dc69f44b..8775619af 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -19,6 +19,7 @@ - Added a toggle-list editor in `/settings` for array-of-enum settings: `providers.webSearchOrder` and `providers.imageOrder` (ordered — Enter/Space toggles, ←/→ nudges, 1-9 splices the hovered provider into that position) and `providers.webSearchExclude` now appear under Providers → Services instead of being config-file only. - Added `models.yml` Bedrock Converse prompt-cache capability overrides for bundled and opaque inference profiles. - Documented Vibe mode (`/vibe`) in `docs/vibe-mode.md` and the `/fresh` provider-stream reset in the session-operations doc, and linked both from the README's new "Session controls" section ([#6440](https://github.com/can1357/oh-my-pi/issues/6440)). +- Added server-name autocomplete for `/mcp enable`, `disable`, `test`, `remove`, `reconnect`, `reauth`, and `unauth`, sourced from configured and runtime-discovered MCP servers ([#5654](https://github.com/can1357/oh-my-pi/issues/5654)). ### Changed 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 928282fba..99cf15cc3 100644 --- a/packages/coding-agent/src/modes/controllers/mcp-command-controller.ts +++ b/packages/coding-agent/src/modes/controllers/mcp-command-controller.ts @@ -246,6 +246,39 @@ type MCPSearchParsed = { error?: string; }; +/** + * Collect the de-duplicated union of every MCP server name we know about: + * user config, project config, and any runtime-discovered servers not + * already present in either config (`ctx.mcpManager.getAllServerNames()` + * covers connections, pending connections, and discovered-but-not-yet- + * connected sources). Disabled servers stay in this list — 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 is the single source of truth for "every known server name": both + * `MCPCommandController#handleList()` and the `/mcp` slash-command argument + * completer (server-name autocomplete for `enable`/`disable`/`test`/etc.) + * call this instead of re-deriving the union themselves. + */ +export async function collectMcpServerNames(ctx: InteractiveModeContext): Promise { + const cwd = getProjectDir(); + const [userConfig, projectConfig] = await Promise.all([ + readMCPConfigFile(getMCPConfigPath("user", cwd)), + readMCPConfigFile(getMCPConfigPath("project", cwd)), + ]); + + const names = new Set([ + ...Object.keys(userConfig.mcpServers ?? {}), + ...Object.keys(projectConfig.mcpServers ?? {}), + ]); + if (ctx.mcpManager) { + for (const name of ctx.mcpManager.getAllServerNames()) { + names.add(name); + } + } + return [...names].sort((a, b) => a.localeCompare(b, undefined, { sensitivity: "base" })); +} + export class MCPCommandController { constructor(private ctx: InteractiveModeContext) {} @@ -1274,7 +1307,8 @@ export class MCPCommandController { const disabledServerNames = new Set(await readDisabledServers(userPath)); const discoveredServers: { name: string; source: SourceMeta }[] = []; if (this.ctx.mcpManager) { - for (const name of this.ctx.mcpManager.getAllServerNames()) { + const allServerNames = await collectMcpServerNames(this.ctx); + for (const name of allServerNames) { if (configServerNames.has(name)) continue; if (disabledServerNames.has(name)) continue; const source = this.ctx.mcpManager.getSource(name); diff --git a/packages/coding-agent/src/slash-commands/builtin-registry.ts b/packages/coding-agent/src/slash-commands/builtin-registry.ts index 349bd27a9..61f6937f9 100644 --- a/packages/coding-agent/src/slash-commands/builtin-registry.ts +++ b/packages/coding-agent/src/slash-commands/builtin-registry.ts @@ -28,6 +28,7 @@ import { } from "../extensibility/plugins/marketplace"; import { resolveMemoryBackend } from "../memory-backend"; import { runPauseScreen } from "../modes/components/pause-screen"; +import { collectMcpServerNames } from "../modes/controllers/mcp-command-controller"; import { describeLoopLimitRuntime } from "../modes/loop-limit"; import { theme } from "../modes/theme/theme"; import type { InteractiveModeContext } from "../modes/types"; @@ -2436,6 +2437,47 @@ 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", +]); + +/** + * Build getArgumentCompletions for /mcp. Delegates to the generic + * declarative subcommand completer while the subcommand name itself is + * still being typed, then switches to MCP server-name completion (sourced + * from {@link collectMcpServerNames}) once a recognized server-name + * subcommand (enable/disable/test/remove/reconnect/reauth/unauth) is + * followed by a space. Subcommands with a different argument shape + * (add, smithery-search, ...) get no argument completion. + */ +function buildMcpArgumentCompletions( + subcommands: SubcommandDef[], + runtime: TuiSlashCommandRuntime, +): (argumentPrefix: string) => Promise { + const genericCompletions = buildArgumentCompletions(subcommands); + return async (argumentPrefix: string) => { + const spaceIndex = argumentPrefix.indexOf(" "); + if (spaceIndex === -1) return genericCompletions(argumentPrefix); + + const rawSubcommand = argumentPrefix.slice(0, spaceIndex); + if (!MCP_SERVER_NAME_SUBCOMMANDS.has(rawSubcommand.toLowerCase())) return null; + + const namePrefix = argumentPrefix.slice(spaceIndex + 1).toLowerCase(); + const serverNames = await collectMcpServerNames(runtime.ctx); + const matches: AutocompleteItem[] = serverNames + .filter(name => name.toLowerCase().startsWith(namePrefix)) + .map(name => ({ value: `${rawSubcommand} ${name} `, label: name })); + return matches.length > 0 ? matches : null; + }; +} + /** * Build getInlineHint from declarative subcommand definitions. * Shows remaining completion + usage as dim ghost text after cursor. @@ -2599,7 +2641,10 @@ function materializeTuiBuiltinSlashCommand( ): TuiBuiltinSlashCommand { const materialized: TuiBuiltinSlashCommand = { ...cmd }; if (cmd.subcommands) { - materialized.getArgumentCompletions = buildArgumentCompletions(cmd.subcommands); + materialized.getArgumentCompletions = + cmd.name === "mcp" && runtime + ? buildMcpArgumentCompletions(cmd.subcommands, runtime) + : buildArgumentCompletions(cmd.subcommands); materialized.getInlineHint = buildSubcommandInlineHint(cmd.subcommands); } else if (cmd.name === "move") { materialized.getArgumentCompletions = buildDirectoryArgumentCompletions(); diff --git a/packages/coding-agent/test/mcp-name-autocomplete.test.ts b/packages/coding-agent/test/mcp-name-autocomplete.test.ts new file mode 100644 index 000000000..51a21903a --- /dev/null +++ b/packages/coding-agent/test/mcp-name-autocomplete.test.ts @@ -0,0 +1,128 @@ +import { afterEach, 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 { collectMcpServerNames } from "@oh-my-pi/pi-coding-agent/modes/controllers/mcp-command-controller"; +import type { InteractiveModeContext } from "@oh-my-pi/pi-coding-agent/modes/types"; +import { buildTuiBuiltinSlashCommands } from "@oh-my-pi/pi-coding-agent/slash-commands/builtin-registry"; +import type { TuiSlashCommandRuntime } from "@oh-my-pi/pi-coding-agent/slash-commands/types"; +import { + getConfigRootDir, + getMCPConfigPath, + getProjectDir, + removeWithRetries, + 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; +} + +async function writeConfig( + scope: "user" | "project", + cwd: string, + servers: Record, +): Promise { + await Bun.write(getMCPConfigPath(scope, cwd), `${JSON.stringify({ mcpServers: servers }, null, 2)}\n`); +} + +/** Fake ctx carrying only the mcpManager surface `collectMcpServerNames` reads. */ +function createFakeCtx(discoveredNames: string[]) { + const mcpManager = { + getAllServerNames: vi.fn((): string[] => discoveredNames), + getSource: vi.fn((): SourceMeta | undefined => undefined), + getConnectionStatus: vi.fn(() => "connected" as const), + }; + const ctx = { mcpManager } as never as InteractiveModeContext; + return { ctx, mcpManager }; +} + +describe("MCP server-name autocomplete", () => { + let projectDir = ""; + let agentDir = ""; + + beforeEach(async () => { + projectDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-mcp-autocomplete-project-")); + agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-mcp-autocomplete-agent-")); + setProjectDir(projectDir); + setAgentDir(agentDir); + }); + + afterEach(async () => { + vi.restoreAllMocks(); + setProjectDir(originalProjectDir); + restoreAgentDir(); + await removeWithRetries(projectDir); + await removeWithRetries(agentDir); + }); + + test("collectMcpServerNames returns the deduplicated union of config and discovered names, including disabled ones", async () => { + await writeConfig("user", projectDir, { + "user-enabled": { type: "stdio", command: "user-one" }, + "user-disabled": { type: "stdio", command: "user-two", enabled: false }, + }); + await writeConfig("project", projectDir, { + "project-server": { type: "stdio", command: "project-one" }, + }); + // "project-server" is discovered too (already in config), "runtime-discovered" is new. + const { ctx } = createFakeCtx(["project-server", "runtime-discovered"]); + + const names = await collectMcpServerNames(ctx); + + expect(names).toEqual(["project-server", "runtime-discovered", "user-disabled", "user-enabled"]); + }); + + test("/mcp getArgumentCompletions resolves known server names after a server-name subcommand, filtered by prefix", async () => { + await writeConfig("user", projectDir, { + "my-server": { type: "stdio", command: "one" }, + "my-other": { type: "stdio", command: "two" }, + "other-server": { type: "stdio", command: "three" }, + }); + 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"); + + const unfiltered = await mcp.getArgumentCompletions("enable "); + expect(unfiltered?.map(item => item.label).sort()).toEqual(["my-other", "my-server", "other-server"]); + + const filtered = await mcp.getArgumentCompletions("enable my-s"); + expect(filtered?.map(item => item.label)).toEqual(["my-server"]); + expect(filtered?.[0]?.value).toBe("enable my-server "); + }); + + 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([]); + 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("add ")).toBeNull(); + }); + + test("/mcp getArgumentCompletions still completes subcommand names while the subcommand is being typed", async () => { + 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"); + + const matches = await mcp.getArgumentCompletions("en"); + expect(matches?.map(item => item.label)).toEqual(["enable"]); + }); +});