diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index d97a1f633..e7596b47e 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -238,6 +238,9 @@ - Fixed spilled tool-output artifact descriptors leaking on error/abort paths. `OutputSink.dump()` was the only path that closed the spill `Bun.FileSink`, but the bash and Python executors re-throw on failure and their `finally` blocks never closed the sink, so a large-output command that errored leaked the artifact descriptor until an unrelated read (e.g. a `SKILL.md` load) hit `EMFILE`. `OutputSink` now exposes an idempotent `dispose()` that closes the sink exactly once, wired into every executor's `finally` ([#6463](https://github.com/can1357/oh-my-pi/issues/6463)). - Fixed the first submitted prompt stalling while the local tiny-title worker started: the interactive submit handler now paints the pending user row before starting title generation, and startup prewarms an idle, unref'd worker so the first submit reuses a live subprocess instead of paying spawn latency ahead of the first frame ([#6462](https://github.com/can1357/oh-my-pi/issues/6462)). - Fixed legacy Pi extensions failing validation when importing the upstream `keyText` keybinding helper ([#6470](https://github.com/can1357/oh-my-pi/issues/6470)). +### Added + +- 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)). ## [17.1.0] - 2026-07-24 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..fb6241592 100644 --- a/packages/coding-agent/src/modes/controllers/mcp-command-controller.ts +++ b/packages/coding-agent/src/modes/controllers/mcp-command-controller.ts @@ -48,7 +48,13 @@ import { searchSmitheryRegistry, toConfigName, } from "../../mcp/smithery-registry"; -import type { MCPAuthChallenge, MCPAuthConfig, MCPServerConfig, MCPServerConnection } from "../../mcp/types"; +import type { + MCPAuthChallenge, + MCPAuthConfig, + MCPConfigFile, + MCPServerConfig, + MCPServerConnection, +} from "../../mcp/types"; import { shortenPath } from "../../tools/render-utils"; import { urlHyperlinkAlways } from "../../tui"; import { copyToClipboard } from "../../utils/clipboard"; @@ -246,6 +252,62 @@ 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). + * + * 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`. + * + * 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. + * + * `preloaded` lets a caller that already read both config files (e.g. + * `#handleList()`) pass them in and skip the redundant re-read. + */ +export async function collectMcpServerNames( + ctx: InteractiveModeContext, + preloaded?: { userConfig: MCPConfigFile; projectConfig: MCPConfigFile }, + includeDisabled = true, +): Promise { + let userConfig: MCPConfigFile; + let projectConfig: MCPConfigFile; + if (preloaded) { + ({ userConfig, projectConfig } = preloaded); + } else { + const cwd = getProjectDir(); + [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 ?? {}), + ...(includeDisabled ? (userConfig.disabledServers ?? []) : []), + ]); + 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) {} @@ -1271,10 +1333,11 @@ export class MCPCommandController { // Collect runtime-discovered servers not in config files const configServerNames = new Set([...userServers, ...projectServers]); - const disabledServerNames = new Set(await readDisabledServers(userPath)); + const disabledServerNames = new Set(userConfig.disabledServers ?? []); const discoveredServers: { name: string; source: SourceMeta }[] = []; if (this.ctx.mcpManager) { - for (const name of this.ctx.mcpManager.getAllServerNames()) { + const allServerNames = await collectMcpServerNames(this.ctx, { userConfig, projectConfig }); + 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 b55cbd0c0..80235d96f 100644 --- a/packages/coding-agent/src/slash-commands/builtin-registry.ts +++ b/packages/coding-agent/src/slash-commands/builtin-registry.ts @@ -3,7 +3,7 @@ import * as os from "node:os"; import * as path from "node:path"; import { getOAuthProviders } from "@oh-my-pi/pi-ai/oauth"; import { type AutocompleteItem, Spacer } from "@oh-my-pi/pi-tui"; -import { APP_NAME, getProjectDir, setProjectDir } from "@oh-my-pi/pi-utils"; +import { APP_NAME, getMCPConfigPath, getProjectDir, logger, setProjectDir } from "@oh-my-pi/pi-utils"; import { reset as resetCapabilities } from "../capability"; import { COLLAB_GUEST_ALLOWED_COMMANDS, CollabGuestLink } from "../collab/guest"; import { CollabHost } from "../collab/host"; @@ -31,8 +31,10 @@ import { getPluginsCacheDir, MarketplaceManager, } from "../extensibility/plugins/marketplace"; +import { readMCPConfigFile } from "../mcp/config-writer"; 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"; @@ -2714,6 +2716,119 @@ 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", +]); + +/** + * `/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. + */ +const MCP_DISABLED_ONLY_ELIGIBLE_SUBCOMMANDS: ReadonlySet = new Set(["enable", "disable"]); + +/** + * 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. `remove` gets its own scope-aware completions (see + * {@link buildMcpRemoveCompletions}) since — unlike the others — + * it only ever succeeds against a config-file entry. 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); + const lowerSubcommand = rawSubcommand.toLowerCase(); + if (!MCP_SERVER_NAME_SUBCOMMANDS.has(lowerSubcommand)) return null; + + const namePrefix = argumentPrefix.slice(spaceIndex + 1).toLowerCase(); + if (lowerSubcommand === "remove") { + return await buildMcpRemoveCompletions(rawSubcommand, namePrefix); + } + + let serverNames: string[]; + try { + serverNames = await collectMcpServerNames( + runtime.ctx, + undefined, + MCP_DISABLED_ONLY_ELIGIBLE_SUBCOMMANDS.has(lowerSubcommand), + ); + } catch (error) { + logger.warn("MCP server-name autocomplete failed to read config", { error }); + return null; + } + const matches: AutocompleteItem[] = serverNames + .filter(name => name.toLowerCase().startsWith(namePrefix)) + .map(name => ({ value: `${rawSubcommand} ${name} `, label: name })); + return matches.length > 0 ? matches : null; + }; +} + +/** + * Build `/mcp remove ` completions. Unlike the other server-name + * subcommands, `#handleRemove` only ever succeeds against a config-file + * `mcpServers` entry in the target scope (project by default, user with an + * explicit `--scope user`) — a purely runtime-discovered server has no + * config entry to remove and always fails with `Server "" not found + * in config.`. Completions are therefore restricted to config-file + * names, and a name that exists only in the user config is completed with + * `--scope user` appended so the inserted command is directly executable. + */ +async function buildMcpRemoveCompletions( + rawSubcommand: string, + namePrefix: string, +): Promise { + const cwd = getProjectDir(); + let projectNames: string[]; + let userNames: string[]; + try { + const [projectConfig, userConfig] = await Promise.all([ + readMCPConfigFile(getMCPConfigPath("project", cwd)), + readMCPConfigFile(getMCPConfigPath("user", cwd)), + ]); + projectNames = Object.keys(projectConfig.mcpServers ?? {}); + userNames = Object.keys(userConfig.mcpServers ?? {}); + } catch (error) { + logger.warn("MCP remove autocomplete failed to read config", { error }); + return null; + } + + const projectNameSet = new Set(projectNames); + const allNames = new Set([...projectNames, ...userNames]); + const matches: AutocompleteItem[] = [...allNames] + .filter(name => name.toLowerCase().startsWith(namePrefix)) + .map(name => + projectNameSet.has(name) + ? { value: `${rawSubcommand} ${name} `, label: name } + : { value: `${rawSubcommand} ${name} --scope user `, label: `${name} (user)` }, + ) + .sort((a, b) => a.label.localeCompare(b.label, undefined, { sensitivity: "base" })); + return matches.length > 0 ? matches : null; +} + /** * Build getInlineHint from declarative subcommand definitions. * Shows remaining completion + usage as dim ghost text after cursor. @@ -2877,7 +2992,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..953c159e5 --- /dev/null +++ b/packages/coding-agent/test/mcp-name-autocomplete.test.ts @@ -0,0 +1,214 @@ +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("collectMcpServerNames includes a discovered server disabled via disabledServers, even once dropped from mcpManager", async () => { + // A third-party-discovered server that was `/mcp disable`d: recorded in the user + // config's top-level `disabledServers` list, absent from `mcpServers`, and no + // longer reported by the manager (loadAllMCPConfigs filters disabled sources out). + await Bun.write( + getMCPConfigPath("user", projectDir), + `${JSON.stringify({ mcpServers: {}, disabledServers: ["discovered-disabled"] }, null, 2)}\n`, + ); + await writeConfig("project", projectDir, {}); + const { ctx } = createFakeCtx([]); + + const names = await collectMcpServerNames(ctx); + + expect(names).toEqual(["discovered-disabled"]); + }); + + test("collectMcpServerNames accepts preloaded configs and skips re-reading them from disk", async () => { + await writeConfig("user", projectDir, { "user-server": { type: "stdio", command: "one" } }); + await writeConfig("project", projectDir, { "project-server": { type: "stdio", command: "two" } }); + const { ctx } = createFakeCtx(["runtime-discovered"]); + + const names = await collectMcpServerNames(ctx, { + userConfig: { mcpServers: { "override-server": { type: "stdio", command: "override" } } }, + projectConfig: { mcpServers: {} }, + }); + + // Reflects the preloaded configs, not what's actually on disk for "user"/"project". + expect(names).toEqual(["override-server", "runtime-discovered"]); + }); + + 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 offers a disabled-only discovered name for enable/disable but not test/reconnect/reauth/unauth", async () => { + // "discovered-disabled" is a third-party server that was /mcp disable'd: + // present only in userConfig.disabledServers, absent from mcpServers, and + // no longer reported by the manager (loadAllMCPConfigs drops disabled + // sources). #resolveServerForAuth/reconnectServer can't resolve it, so + // test/reconnect/reauth/unauth must not suggest it. + await Bun.write( + getMCPConfigPath("user", projectDir), + `${JSON.stringify({ mcpServers: {}, disabledServers: ["discovered-disabled"] }, null, 2)}\n`, + ); + await writeConfig("project", projectDir, {}); + 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(["discovered-disabled"]); + expect((await mcp.getArgumentCompletions("disable "))?.map(item => item.label)).toEqual(["discovered-disabled"]); + expect(await mcp.getArgumentCompletions("test ")).toBeNull(); + expect(await mcp.getArgumentCompletions("reconnect ")).toBeNull(); + expect(await mcp.getArgumentCompletions("reauth ")).toBeNull(); + expect(await mcp.getArgumentCompletions("unauth ")).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([]); + 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"]); + }); + + test("/mcp getArgumentCompletions returns null instead of throwing when a config file is malformed", async () => { + // Malformed JSON makes readMCPConfigFile's JSON.parse throw (ENOENT is the only + // error it swallows), which must not escape the autocomplete provider. + await Bun.write(getMCPConfigPath("user", projectDir), "{ not valid json"); + 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"); + + await expect(mcp.getArgumentCompletions("enable ")).resolves.toBeNull(); + }); + + test("/mcp getArgumentCompletions for remove only offers config-file names, tagging user-only ones with --scope user", async () => { + await writeConfig("user", projectDir, { "user-only": { type: "stdio", command: "one" } }); + await writeConfig("project", projectDir, { "project-only": { type: "stdio", command: "two" } }); + // A purely runtime-discovered server (no config entry in either scope) has + // nothing for /mcp remove to delete and must not be offered. + const { ctx } = createFakeCtx(["discovered-only"]); + 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("remove "); + expect(matches?.map(item => item.label)).toEqual(["project-only", "user-only (user)"]); + + const projectMatch = matches?.find(item => item.label === "project-only"); + expect(projectMatch?.value).toBe("remove project-only "); + + const userMatch = matches?.find(item => item.label === "user-only (user)"); + expect(userMatch?.value).toBe("remove user-only --scope user "); + }); +});