Merge PR #6454: feat(coding-agent): autocomplete MCP server names in /mcp subcommands (@Mathews-Tom)

This commit is contained in:
can1357
2026-07-29 23:07:52 +02:00
4 changed files with 403 additions and 5 deletions
+3
View File
@@ -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
@@ -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 <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`.
*
* 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<string[]> {
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<string>([
...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);
@@ -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: "<name>..."`). */
const MCP_SERVER_NAME_SUBCOMMANDS: ReadonlySet<string> = 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<string> = 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<AutocompleteItem[] | null> {
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 <name>` 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 "<name>" not found
* in <scope> 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<AutocompleteItem[] | null> {
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();
@@ -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<string, MCPServerConfig>,
): Promise<void> {
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 ");
});
});