diff --git a/README.md b/README.md index d5138e66..42e1052c 100644 --- a/README.md +++ b/README.md @@ -1679,6 +1679,68 @@ moshcode skill list --json Each row reports `installed` and `supported` separately, so an installed engine without that integration primitive remains visible rather than looking absent. +### Managing a registered server + +Registering a server is a fan-out; so is everything you do to it afterwards. + +```sh +moshcode mcp add # interactive wizard +moshcode mcp add sentry --url https://mcp.sentry.dev/mcp --token env:SENTRY_TOKEN +moshcode mcp add tools --engines claude,codex -- npx -y my-mcp-server +moshcode mcp remove sentry +moshcode mcp reauth sentry # each engine's own OAuth login +moshcode mcp unauth sentry +moshcode mcp reconnect --all +moshcode mcp list --servers # what moshcode registered +``` + +**Scope here is two axes, not one.** `--engine-scope user|project` picks the +config file each engine writes. `--engines claude,codex` picks which of the six +engines get the server at all. Both default to the widest useful answer: user +scope, every installed MCP-capable engine. `--scope` is deliberately neither of +them, because on `/mcp answer` it already means the OAuth permissions of a +shared session. + +**`--token env:VAR` is the form worth using.** A literal token is in your shell +history before moshcode sees it and in every engine's config afterwards. The +`env:` form is read from the environment at registration time, and it is what +`~/.moshcode/mcp.json` records. That file keeps header *names* and the variable +a value came from, never a value. + +**`disable` and `enable` are a round trip, not a live toggle.** No engine +moshcode drives has an enable or disable command, and moshcode does not edit +their config files. So `disable` takes the server out of every engine and keeps +its spec; `enable` registers exactly that spec again. + +What a given engine cannot do is reported rather than faked. OpenCode has no +`mcp remove`. Codex has no project scope. Only the Gemini family has +`mcp reconnect`. Gemini and Qwen authorize from inside their own session. Kimi +and omp run MCP servers perfectly well and have no scriptable `mcp` subcommand +for moshcode to drive. Each of those prints the reason and what to type instead. + +### Testing a server before you trust it + +`test`, `resources`, `prompts` and `notifications` talk *to* a server rather +than about it, and they run through [mcpjam](#mcp-server-testing), which +moshcode already installs. They take a registered name, a catalog name, or a +bare URL. The last one matters, because "does this thing work" is a question +you ask before deciding to register it. + +```sh +moshcode install mcpjam +moshcode mcp test https://mcp.sentry.dev/mcp +moshcode mcp resources sentry --json +moshcode mcp prompts sentry +moshcode mcp notifications sentry # what it declares +moshcode mcp notifications sentry --listen --for 30000 +moshcode mcp catalog search postgres # the public MCP directories +``` + +`catalog search` is the generalized form of a registry search: it sweeps the +scraped MCP directories through mcpjam rather than adding a vendor-specific +verb, and the registry API key stays mcpjam's to hold rather than being copied +into a second place. + ### Connect ChatGPT, Claude, or Chovy to this session Start the interactive pit, then mint a short-lived remote MCP URL for its live diff --git a/src/cli-schema.mjs b/src/cli-schema.mjs index 626e2ffd..9ca262ed 100644 --- a/src/cli-schema.mjs +++ b/src/cli-schema.mjs @@ -401,9 +401,15 @@ export const CORE_CLI_COMMANDS = [ synopsis: [["moshcode mcp [args…]", ""]], verbs: "MCP_VERBS", examples: [ - ["moshcode mcp list --json", "support and install status"], - ["moshcode mcp install https://mcp.example.com", "a remote server"], - ], + ["moshcode mcp list --json", "registered servers + engine support"], + ["moshcode mcp install https://mcp.example.com", "a remote server, everywhere"], + ["moshcode mcp add tools --engines claude,codex -- npx -y srv", "two engines only"], + ["moshcode mcp test sentry", "does it actually answer?"], + ["moshcode mcp catalog search postgres", "the public MCP directories"], + ], + note: "moshcode drives each engine's own `mcp` commands and never writes their config files, so a " + + "verb an engine lacks is skipped with the reason rather than faked. The verbs that talk TO a " + + "server run through mcpjam.", seeAlso: ["skill", "engines"], }, { @@ -1122,30 +1128,161 @@ export const MCP_VERBS = [ ], flags: [ ["--name ", "override the derived server name", ""], + ["--url ", "the remote server, spelled explicitly", ""], ["-t, --transport ", "http | sse | stdio", "inferred from the target"], + ["--token ", "Authorization: Bearer …; env:VAR keeps it out of history", ""], + ["--engine-scope ", "user | project: the config file each engine writes", "user"], + ["--engines ", "only these engines", "every MCP-capable engine"], ["-e, --env K=V", "repeatable", ""], ["-H, --header 'K: V'", "repeatable", ""], ["--", "everything after this is the server's argv", ""], ], + note: "scope here is two axes, not one. --engine-scope picks the config file inside each engine; " + + "--engines picks which of the six engines get the server at all. `--scope` is deliberately NOT " + + "either of them: on `mcp answer` it already means the OAuth permissions of a shared session.", }, { name: "add", - description: "register a named MCP server", + description: "register a named MCP server, or run the wizard", acceptsServerSpec: true, - synopsis: [["moshcode mcp add --name ", ""]], + synopsis: [ + ["moshcode mcp add", "interactive wizard"], + ["moshcode mcp add --url [-t http|sse]", "remote server"], + ["moshcode mcp add -- ", "local stdio server"], + ], + flags: [ + ["--url ", "the remote server, spelled explicitly", ""], + ["-t, --transport ", "http | sse | stdio", "inferred from the target"], + ["--token ", "Authorization: Bearer …; env:VAR keeps it out of history", ""], + ["--engine-scope ", "user | project", "user"], + ["--engines ", "only these engines", "every MCP-capable engine"], + ], }, { name: "bridge", description: "serve moshcode's verbs over MCP, on stdio", synopsis: [["moshcode mcp bridge", "speaks MCP on stdin/stdout; register it with any engine"]], }, - { name: "catalog", description: "show known MCP servers", synopsis: [["moshcode mcp catalog", ""]] }, { - name: "list", - description: "show MCP support and install status", - synopsis: [["moshcode mcp list [--json]", ""]], + name: "remove", + takesServerName: true, + description: "deregister a server from every engine", + synopsis: [["moshcode mcp remove [--engine-scope user|project] [--engines a,b]", ""]], + note: "OpenCode and privacycode have no `mcp remove`; they are skipped with the file to edit. " + + "Codex has no project scope.", + }, + { + name: "enable", + takesServerName: true, + description: "re-register a server moshcode disabled", + synopsis: [["moshcode mcp enable ", ""]], + note: "no engine moshcode drives has an enable/disable verb, and moshcode will not edit their " + + "config files to fake one. So disable deregisters the server everywhere and keeps its spec in " + + "~/.moshcode/mcp.json, and enable registers exactly that spec again. A credential is never " + + "kept, so a token-authenticated server needs --token again on the way back.", + }, + { + name: "disable", + takesServerName: true, + description: "deregister a server, keeping its spec for enable", + synopsis: [["moshcode mcp disable ", ""]], + seeAlso: ["mcp"], + }, + { + name: "test", + takesServerName: true, + description: "connect to a server and report what it serves", + synopsis: [["moshcode mcp test [--json]", ""]], + flags: [["--json", "machine-readable", ""]], + note: "runs through mcpjam, which moshcode already installs (`/install mcpjam`). " + + "Takes a registered name, a catalog name, or a bare URL you have not registered yet.", + }, + { + name: "reauth", + takesServerName: true, + description: "run each engine's own OAuth login for a server", + synopsis: [["moshcode mcp reauth ", ""]], + note: "drives `claude mcp login`, `codex mcp login` and `opencode mcp auth` in turn. Each runs the " + + "MCP spec's OAuth 2.1 flow (authorization code + PKCE) and keeps its own rotating refresh " + + "token; moshcode mints nothing and stores nothing. Gemini and Qwen authorize from inside " + + "the session, so they are skipped with the words to type there.", + }, + { + name: "unauth", + takesServerName: true, + description: "clear a server's stored OAuth credentials", + synopsis: [["moshcode mcp unauth ", ""]], + }, + { + name: "reconnect", + takesServerName: true, + description: "redial a server in the engines that can", + synopsis: [ + ["moshcode mcp reconnect ", ""], + ["moshcode mcp reconnect --all", "every configured server"], + ], + flags: [["-a, --all", "reconnect every server", ""]], + note: "only the Gemini family has `mcp reconnect`. Claude Code, Codex and OpenCode dial their " + + "servers when a session starts, so they are skipped with what to do instead.", + }, + { + name: "resources", + takesServerName: true, + description: "list the resources a server exposes", + synopsis: [["moshcode mcp resources [--json]", ""]], flags: [["--json", "machine-readable", ""]], }, + { + name: "prompts", + takesServerName: true, + description: "list the prompts a server exposes", + synopsis: [["moshcode mcp prompts [--json]", ""]], + flags: [["--json", "machine-readable", ""]], + }, + { + name: "notifications", + takesServerName: true, + description: "what a server can notify about, and watch it", + synopsis: [ + ["moshcode mcp notifications ", "the capabilities it declares"], + ["moshcode mcp notifications --listen --for 30000", "stream them"], + ], + flags: [ + ["--listen", "stream notifications instead of reading capabilities", ""], + ["--for ", "stop listening after this long", "until Ctrl-C"], + ["--json", "machine-readable", ""], + ], + }, + { + name: "catalog", + description: "show known MCP servers, or search the public directories", + synopsis: [ + ["moshcode mcp catalog", "moshcode's own curated list"], + ["moshcode mcp catalog search [--limit 1-100]", "the public MCP directories"], + ], + flags: [ + ["--limit <1-100>", "results per page", ""], + ["--source ", "one directory instead of all of them", "all"], + ["--json", "machine-readable", ""], + ], + note: "search runs through mcpjam's registry, which sweeps the scraped MCP directories " + + "(Smithery among them). There is no Smithery-specific verb and no second API key here on " + + "purpose: the key is mcpjam's, mcpjam already stores it, and a copy in moshcode would be a " + + "second place to leak it from.", + }, + { + name: "list", + description: "show registered servers, plus MCP support per engine", + synopsis: [ + ["moshcode mcp list [--json]", "engine support (--json is the engine array)"], + ["moshcode mcp list --servers [--json]", "just the servers moshcode registered"], + ], + flags: [ + ["--servers", "only the servers, not the engine matrix", ""], + ["--json", "machine-readable", ""], + ], + }, + { name: "help", description: "show this help", synopsis: [["moshcode mcp help", ""]] }, ]; export const SKILL_VERBS = [ diff --git a/src/completion.mjs b/src/completion.mjs index f6c098fa..74f7f289 100644 --- a/src/completion.mjs +++ b/src/completion.mjs @@ -53,6 +53,11 @@ export function completionModel() { ]), mcp: uniqueEntries(MCP_VERBS), mcpServerSpecs: uniqueEntries(MCP_VERBS.filter(({ acceptsServerSpec }) => acceptsServerSpec)), + // The verbs that take an already-registered server NAME rather than a whole + // spec. Their flag list below is the union of what the group accepts. + // Offering `--listen` after `mcp remove` is a wasted keystroke; offering + // nothing after `mcp test` is a wasted feature. + mcpServerVerbs: uniqueEntries(MCP_VERBS.filter(({ takesServerName }) => takesServerName)), skills: uniqueEntries(SKILL_VERBS), trade: uniqueEntries(TRADE_VERBS), tradeOrderOptions: uniqueEntries([ @@ -129,6 +134,7 @@ ${powershellEntries("MoshcodeCompletionUninstall", model.uninstall)} ${powershellEntries("MoshcodeCompletionUpgrade", model.upgrade)} ${powershellEntries("MoshcodeCompletionMcp", model.mcp)} ${powershellEntries("MoshcodeCompletionMcpServerSpecs", model.mcpServerSpecs)} +${powershellEntries("MoshcodeCompletionMcpServerVerbs", model.mcpServerVerbs)} ${powershellEntries("MoshcodeCompletionSkills", model.skills)} ${powershellEntries("MoshcodeCompletionTrade", model.trade)} ${powershellEntries("MoshcodeCompletionTradeOrderOptions", model.tradeOrderOptions)} @@ -143,7 +149,8 @@ ${powershellEntries("MoshcodeCompletionConsole", optionEntries("serve --url", "c ${powershellEntries("MoshcodeCompletionConsoleServe", optionEntries("--port --ttyd --bind", "console serve option"))} ${powershellEntries("MoshcodeCompletionTemplate", optionEntries("list install", "template command"))} ${powershellEntries("MoshcodeCompletionTemplateInstall", optionEntries("--into --force --dry-run", "template install option"))} -${powershellEntries("MoshcodeCompletionMcpOptions", optionEntries("--name --transport -t --env -e --header -H", "MCP option"))} +${powershellEntries("MoshcodeCompletionMcpOptions", optionEntries("--name --url --transport -t --token --engine-scope --engines --env -e --header -H", "MCP option"))} +${powershellEntries("MoshcodeCompletionMcpNameOptions", optionEntries("--engine-scope --engines --json --listen --for --all", "MCP option"))} ${powershellEntries("MoshcodeCompletionSkillOptions", optionEntries("--name", "skill option"))} Register-ArgumentCompleter -Native -CommandName moshcode -ScriptBlock { @@ -196,6 +203,8 @@ Register-ArgumentCompleter -Native -CommandName moshcode -ScriptBlock { $choices = $script:MoshcodeCompletionJson } elseif ($script:MoshcodeCompletionMcpServerSpecs.Name -contains $nested -and $wordToComplete.StartsWith('-')) { $choices = $script:MoshcodeCompletionMcpOptions + } elseif ($script:MoshcodeCompletionMcpServerVerbs.Name -contains $nested -and $wordToComplete.StartsWith('-')) { + $choices = $script:MoshcodeCompletionMcpNameOptions } } { $_ -in @('skill', 'skills') } { @@ -299,7 +308,9 @@ _moshcode_completion() { elif [[ "$nested" == "list" && "$cur" == -* ]]; then choices="--json" elif ${shellMatches("nested", model.mcpServerSpecs)} && [[ "$cur" == -* ]]; then - choices="--name --transport -t --env -e --header -H --" + choices="--name --url --transport -t --token --engine-scope --engines --env -e --header -H --" + elif ${shellMatches("nested", model.mcpServerVerbs)} && [[ "$cur" == -* ]]; then + choices="--engine-scope --engines --json --listen --for --all" fi ;; skill|skills) @@ -434,10 +445,12 @@ _moshcode() { _values "mcp list option" --json elif ${shellMatches("{words[3]}", model.mcpServerSpecs)}; then if [[ "$PREFIX" == -* ]]; then - _values "mcp option" --name --transport -t --env -e --header -H -- + _values "mcp option" --name --url --transport -t --token --engine-scope --engines --env -e --header -H -- else _files fi + elif ${shellMatches("{words[3]}", model.mcpServerVerbs)} && [[ "$PREFIX" == -* ]]; then + _values "mcp option" --engine-scope --engines --json --listen --for --all fi ;; skill|skills) @@ -579,6 +592,16 @@ complete -c moshcode -n '${nestedCondition("mcp", model.mcpServerSpecs)}' -l nam complete -c moshcode -n '${nestedCondition("mcp", model.mcpServerSpecs)}' -l transport -s t -r -d 'MCP transport' complete -c moshcode -n '${nestedCondition("mcp", model.mcpServerSpecs)}' -l env -s e -r -d 'environment KEY=VALUE' complete -c moshcode -n '${nestedCondition("mcp", model.mcpServerSpecs)}' -l header -s H -r -d 'HTTP Name: Value header' +complete -c moshcode -n '${nestedCondition("mcp", model.mcpServerSpecs)}' -l url -r -d 'remote server URL' +complete -c moshcode -n '${nestedCondition("mcp", model.mcpServerSpecs)}' -l token -r -d 'bearer token, or env:VAR' +complete -c moshcode -n '${nestedCondition("mcp", model.mcpServerSpecs)}' -l engine-scope -r -d 'user or project' +complete -c moshcode -n '${nestedCondition("mcp", model.mcpServerSpecs)}' -l engines -r -d 'only these engines' +complete -c moshcode -n '${nestedCondition("mcp", model.mcpServerVerbs)}' -l engine-scope -r -d 'user or project' +complete -c moshcode -n '${nestedCondition("mcp", model.mcpServerVerbs)}' -l engines -r -d 'only these engines' +complete -c moshcode -n '${nestedCondition("mcp", model.mcpServerVerbs)}' -l json -d 'print JSON' +complete -c moshcode -n '${nestedCondition("mcp", model.mcpServerVerbs)}' -l listen -d 'stream notifications' +complete -c moshcode -n '${nestedCondition("mcp", model.mcpServerVerbs)}' -l for -r -d 'stop listening after N ms' +complete -c moshcode -n '${nestedCondition("mcp", model.mcpServerVerbs)}' -l all -s a -d 'every configured server' complete -c moshcode -n '${nestedCondition("skill", model.skillSources)}; or ${nestedCondition("skills", model.skillSources)}' -l name -r -d 'installed skill name' `; } diff --git a/src/integrations.mjs b/src/integrations.mjs index e5d6b877..a26c79f3 100644 --- a/src/integrations.mjs +++ b/src/integrations.mjs @@ -1,10 +1,17 @@ // `/mcp` and `/skill` command flows, shared by the TUI and the CLI. Each parses // a canonical spec, plans the per-engine fan-out, runs it, and prints a // per-engine summary. See prd/0003. -import { ENGINES, isInstalled } from "./engines.mjs"; +import { ENGINES, isInstalled, runCmd } from "./engines.mjs"; import { - MCP_ENGINES, deriveName, isRemoteTarget, planMcpAdd, runMcpAdd, + MCP_ENGINES, MCP_SCOPES, deriveName, isRemoteTarget, planMcpAdd, planMcpVerb, + resolveMcpEngines, runMcpAdd, runMcpVerb, } from "./mcp.mjs"; +import { + PROBE_VERBS, catalogSearchArgs, MCPJAM, mcpjamArgs, mcpjamInstalled, missingProbeTool, +} from "./mcp-probe.mjs"; +import { + forgetServer, getServer, listServers, recordServer, setServerEnabled, +} from "./mcp-registry.mjs"; import { SKILL_ENGINES, planSkillInstall, runSkillInstall, skillName, } from "./skills.mjs"; @@ -14,6 +21,7 @@ import { } from "./plugins.mjs"; import { catalogList, resolveCatalog } from "./mcp-catalog.mjs"; import { MCP_VERBS, PLUGIN_VERBS, SKILL_VERBS } from "./cli-schema.mjs"; +import { findCommand, renderCommand } from "./help.mjs"; import { connectMcp, createMcpShare, ensureMcpCredentials, listMcpShares, revokeMcpShare } from "./mcp-share.mjs"; import { acid, ash, bone, ok, err, info } from "./ui.mjs"; @@ -35,13 +43,104 @@ function flagValue(rest, index, flag) { return { value }; } +/** + * `--scope` means OAuth permissions on `/mcp answer` and nothing else. + * + * Someone reading a parity list elsewhere will type `--scope project` at these + * verbs, and the two axes of moshcode's scope model are `--engine-scope` and + * `--engines` (see MCP_SCOPES). Failing with the name of the flag they wanted + * costs one line and saves the reader a trip through help. + */ +const SCOPE_COLLISION = "`--scope` is the OAuth permissions of a /mcp answer share, not a config scope." + + " For the config file each engine writes, use --engine-scope user|project." + + " For which engines get the server, use --engines claude,codex"; + +/** Verbs that act on one already-registered server by name, through the engines. */ +const MANAGE_VERBS = ["remove", "enable", "disable", "reauth", "unauth", "reconnect"]; + +/** + * The flags every name-addressed verb shares: which engines, which config file + * inside them, and whether the caller wants JSON back. + */ +function parseManageFlags(rest) { + const parsed = { json: false, engineScope: undefined, engines: null, name: undefined, extra: [] }; + for (let i = 0; i < rest.length; i++) { + const t = rest[i]; + if (t === "--json") { parsed.json = true; continue; } + if (t === "--all" || t === "-a") { parsed.all = true; continue; } + if (t === "--scope") return { error: SCOPE_COLLISION }; + if (t === "--engine-scope" || t === "--engines") { + const next = flagValue(rest, i, t); + if (next.error) return next; + if (t === "--engine-scope") { + if (!MCP_SCOPES.includes(next.value)) { + return { error: `--engine-scope must be ${MCP_SCOPES.join(" or ")}` }; + } + parsed.engineScope = next.value; + } else { + const resolved = resolveMcpEngines(next.value); + if (resolved.error) return resolved; + parsed.engines = resolved.engines; + } + i++; + continue; + } + if (String(t).startsWith("-")) return { error: `unknown mcp flag "${t}"` }; + if (parsed.name === undefined) parsed.name = t; + else parsed.extra.push(t); + } + return parsed; +} + /** Parse `/mcp` tokens (after the `mcp` word) into { list } | { spec } | { error }. */ export function parseMcp(tokens) { const verb = tokens[0]; - if (!verb || verb === "list") return { list: true, json: tokens.slice(1).includes("--json") }; - if (verb === "catalog") return { showCatalog: true }; + if (!verb || verb === "list") { + const flags = tokens.slice(1); + // `--servers` narrows the listing to the servers moshcode registered. + // Deliberately a narrowing flag rather than a new shape for `--json`: that + // already returns an ARRAY of engine capability rows, `skill list --json` + // returns the same array, and wrapping it in an object to make room would + // break every reader of both for a listing that has its own flag anyway. + return { + list: true, + json: flags.includes("--json"), + ...(flags.includes("--servers") ? { servers: true } : {}), + }; + } + if (verb === "help") return { help: true }; + if (verb === "catalog") { + // `catalog` with no argument is still the local list. `catalog search …` is + // the generalized form of the Smithery verbs; see catalogSearchArgs. + if (tokens[1] !== "search") { + if (tokens.length > 1) return { error: `unknown mcp catalog verb "${tokens[1]}". try search` }; + return { showCatalog: true }; + } + const search = { query: "", limit: null, source: null, json: false }; + for (let i = 2; i < tokens.length; i++) { + const t = tokens[i]; + if (t === "--json") { search.json = true; continue; } + if (t === "--limit" || t === "--source") { + const next = flagValue(tokens, i, t); + if (next.error) return next; + if (t === "--limit") { + const n = Number(next.value); + if (!Number.isInteger(n) || n < 1 || n > 100) return { error: "--limit must be a whole number from 1 to 100" }; + search.limit = n; + } else search.source = next.value; + i++; + continue; + } + if (String(t).startsWith("-")) return { error: `unknown mcp catalog search flag "${t}"` }; + search.query = search.query ? `${search.query} ${t}` : t; + } + if (!search.query) return { error: "mcp catalog search needs a keyword" }; + return { catalogSearch: search }; + } // The house rule from PRD 0018 R12: a CLI ships an MCP bridge. It takes no - // arguments because it is a transport, not a verb with options. + // arguments because it is a transport, not a verb with options. Note which + // direction it points: `bridge` makes MOSHCODE an MCP server, where every + // verb below acts on somebody else's. if (verb === "bridge") { if (tokens.length > 1) return { error: "mcp bridge takes no arguments" }; return { bridge: true }; @@ -72,6 +171,63 @@ export function parseMcp(tokens) { if (!tokens[1] || tokens.length > 2) return { error: "mcp revoke requires exactly one share id" }; return { remote: { action: "revoke", shareId: tokens[1] } }; } + + // The verbs that talk TO a server rather than about it. They take a + // registered name, a catalog name, or a bare URL. A URL because the most + // common moment to ask "does this thing work" is before deciding to register + // it, which is the whole point of having the probe. + if (PROBE_VERBS.includes(verb)) { + const probe = { verb, json: false, listen: false, durationMs: null }; + const rest = tokens.slice(1); + for (let i = 0; i < rest.length; i++) { + const t = rest[i]; + if (t === "--json") { probe.json = true; continue; } + if (t === "--listen") { probe.listen = true; continue; } + if (t === "--scope") return { error: SCOPE_COLLISION }; + if (t === "--for") { + const next = flagValue(rest, i, t); + if (next.error) return next; + const ms = Number(next.value); + if (!Number.isInteger(ms) || ms < 1) return { error: "--for must be a whole number of milliseconds" }; + probe.durationMs = ms; + i++; + continue; + } + if (String(t).startsWith("-")) return { error: `unknown mcp ${verb} flag "${t}"` }; + if (!probe.name) probe.name = t; + else return { error: `mcp ${verb} takes one server, not "${t}"` }; + } + if (!probe.name) return { error: `mcp ${verb} needs a server name or URL` }; + // --for only means anything while something is streaming, and silently + // ignoring it is how someone waits ten minutes for a capability dump. + if (probe.durationMs && !probe.listen) return { error: "--for only applies with --listen" }; + return { probe }; + } + + if (MANAGE_VERBS.includes(verb)) { + const flags = parseManageFlags(tokens.slice(1)); + if (flags.error) return flags; + if (flags.extra.length) return { error: `mcp ${verb} takes one server, not "${flags.extra[0]}"` }; + // `reconnect --all` is the one name-less case: the Gemini family's own + // reconnect takes `--all`, and "redial everything" is what people actually + // want after a laptop wakes up. + // The wrong-flag error comes first on purpose: `mcp remove --all` with the + // name check leading answers "needs a server name", which reads as though + // --all were fine and only the name were missing. + if (flags.all && verb !== "reconnect") return { error: "--all only applies to mcp reconnect" }; + const wantsAll = verb === "reconnect" && flags.all; + if (!flags.name && !wantsAll) return { error: `mcp ${verb} needs a server name` }; + return { + manage: { + verb, + name: flags.name, + all: Boolean(wantsAll), + engineScope: flags.engineScope, + engines: flags.engines, + }, + }; + } + const verbSchema = MCP_VERBS.find(({ name }) => name === verb); if (!verbSchema?.acceptsServerSpec) { const choices = MCP_VERBS.map(({ name }) => name); @@ -79,15 +235,33 @@ export function parseMcp(tokens) { } const rest = tokens.slice(1); - let name, transport, cmdParts = null; + // `mcp add` with nothing after it is the wizard, not an error. The parity + // surface asks for an interactive wizard and this is where it announces + // itself; mcpCommand decides whether a terminal is actually there to answer. + if (verb === "add" && !rest.length) return { wizard: true }; + + let name, transport, url, engineScope, engines = null, auth = null, cmdParts = null; const env = [], headers = [], positional = []; for (let i = 0; i < rest.length; i++) { const t = rest[i]; if (t === "--") { cmdParts = rest.slice(i + 1); break; } - else if (t === "--name") { + else if (t === "--scope") return { error: SCOPE_COLLISION }; + else if (t === "--name" || t === "--url" || t === "--engine-scope" || t === "--engines") { const next = flagValue(rest, i, t); if (next.error) return next; - name = next.value; i++; + if (t === "--name") name = next.value; + else if (t === "--url") url = next.value; + else if (t === "--engine-scope") { + if (!MCP_SCOPES.includes(next.value)) { + return { error: `--engine-scope must be ${MCP_SCOPES.join(" or ")}` }; + } + engineScope = next.value; + } else { + const resolved = resolveMcpEngines(next.value); + if (resolved.error) return resolved; + engines = resolved.engines; + } + i++; } else if (t === "-t" || t === "--transport") { const next = flagValue(rest, i, t); @@ -104,12 +278,45 @@ export function parseMcp(tokens) { if (next.error) return next; headers.push(next.value); i++; } + // `--token` is sugar for the Authorization header every hosted MCP server + // wants, and `env:NAME` is the form worth using. A literal token typed here + // is in the shell history before moshcode sees it and in six engine configs + // afterwards; `--token env:SENTRY_TOKEN` is neither, and it is the form the + // record in ~/.moshcode/mcp.json can keep without keeping a secret. + else if (t === "--token") { + const next = flagValue(rest, i, t); + if (next.error) return next; + const fromEnv = /^env:(.+)$/.exec(next.value); + if (fromEnv) { + const variable = fromEnv[1]; + const value = process.env[variable]; + if (!value) return { error: `--token env:${variable}: ${variable} is not set in this environment` }; + headers.push(`Authorization: Bearer ${value}`); + auth = { header: "Authorization", from: `env:${variable}` }; + } else { + headers.push(`Authorization: Bearer ${next.value}`); + auth = { header: "Authorization", from: "inline" }; + } + i++; + } else positional.push(t); } + if (transport !== undefined && !["http", "sse", "stdio"].includes(transport)) { + return { error: "-t/--transport must be http, sse, or stdio" }; + } + if (verb === "add") name = name || positional.shift(); let target, args = []; - if (cmdParts) { target = cmdParts[0]; args = cmdParts.slice(1); } + // `--url` is the explicit spelling of the remote target, and it wins over a + // positional so `mcp add sentry --url https://…` reads the way it looks. A + // URL and a `-- ` together is a contradiction rather than a + // precedence question, so it is refused instead of silently resolved. + if (url && cmdParts) { + return { error: "--url and `-- ` describe two different servers. pick one" }; + } + if (url) { target = url; args = []; } + else if (cmdParts) { target = cmdParts[0]; args = cmdParts.slice(1); } else { target = positional[0]; args = positional.slice(1); } // A bare known name is enough: `mcp add porkbun` fills the command in from @@ -169,8 +376,15 @@ export function parseMcp(tokens) { if (headers.some((header) => headerName(header) === "")) { return { error: "mcp --header requires a non-empty header name" }; } + // The spec stays exactly the canonical five fields every engine builder + // reads. The new axes ride beside it rather than inside it: `engineScope` and + // `engines` are about the fan-out, not about the server, and folding them in + // would put fan-out policy into the thing that gets recorded and re-used. return { spec: { name, target, args, transport, env, headers }, + ...(engineScope ? { engineScope } : {}), + ...(engines ? { engines } : {}), + ...(auth ? { auth } : {}), ...(catalog ? { catalog } : {}), }; } @@ -205,6 +419,90 @@ export function printMcpCatalog() { console.log(catalogList()); } +/** + * Print the servers moshcode registered, above the engine matrix. + * + * Its own printer rather than a section inside `printMcpTargets`, because that + * one answers "which engines can take a server" and this one answers "which + * servers did I register". Two questions, two blocks, and the matrix's own + * tests keep asserting exactly what they always asserted. + */ +export function printMcpServers() { + const servers = listServers(); + if (!servers.length) { + console.log(ash(" no servers registered through moshcode yet. /mcp install ")); + console.log(""); + return; + } + console.log(bone(" servers") + ash(" registered by moshcode; each engine's own config is the truth")); + const width = Math.max(10, ...servers.map((s) => s.name.length)); + for (const server of servers) { + const dot = server.enabled === false ? DOT.missing : DOT.installed; + const where = server.engines ? server.engines.join(",") : "all engines"; + const scope = server.engineScope || "user"; + const target = [server.target, ...(server.args || [])].join(" "); + console.log(` ${dot} ${bone(server.name.padEnd(width))} ${ash(target)}`); + console.log(` ${" ".repeat(width)} ${ash(`${scope} scope · ${where}${server.enabled === false ? " · disabled" : ""}`)}`); + } + console.log(""); +} + +/** + * Turn a name into something a probe can dial. + * + * Three sources, most specific first: what moshcode registered, what the + * catalog knows, and a bare URL typed on the spot. The last one matters most. + * "Does this server work" is a question people ask BEFORE registering it, and a + * probe that only accepted registered names would be useless at exactly the + * moment it is wanted. + */ +export function resolveServerSpec(token) { + if (!token) return null; + const recorded = getServer(token); + if (recorded) { + // The record keeps header NAMES, never values (see mcp-registry.mjs), so a + // header-authenticated server is rebuilt here only when the value came from + // somewhere nameable. `unauthenticated` tells the caller to say so rather + // than letting a 401 read as a broken server. + const headers = []; + let unauthenticated = false; + if (recorded.auth) { + const variable = /^env:(.+)$/.exec(recorded.auth.from || "")?.[1]; + const value = variable ? process.env[variable] : null; + if (value) headers.push(`${recorded.auth.header}: Bearer ${value}`); + else unauthenticated = true; + } else if (recorded.headers?.length) unauthenticated = true; + return { + source: "registry", + unauthenticated, + spec: { + name: recorded.name, + target: recorded.target, + args: recorded.args || [], + transport: recorded.transport || undefined, + env: [], + headers, + }, + }; + } + const catalog = resolveCatalog(token); + if (catalog) { + return { + source: "catalog", + unauthenticated: false, + spec: { name: catalog.key, target: catalog.target, args: catalog.args, transport: undefined, env: [], headers: [] }, + }; + } + if (isRemoteTarget(token)) { + return { + source: "url", + unauthenticated: false, + spec: { name: deriveName(token), target: token, args: [], transport: undefined, env: [], headers: [] }, + }; + } + return null; +} + /** Print the MCP support matrix + install status. */ export function printMcpTargets(json = false) { const targets = mcpTargetStatus(); @@ -231,12 +529,20 @@ export function printSkillTargets(json = false) { } } +// The past tenses that mean "this engine did the thing". Listed rather than +// inferred so a new verb has to choose its word here and cannot land in the +// `skipped` branch by spelling its status something nobody expected. +const DID_IT = new Set([ + "added", "installed", "removed", "authorized", "cleared", "reconnected", "enabled", "disabled", +]); + function summarize(results) { for (const r of results) { - if (r.status === "added" || r.status === "installed" || r.status === "removed") console.log(line(r.key, ok(r.status))); + if (DID_IT.has(r.status)) console.log(line(r.key, ok(r.status))); // Nothing to do and nothing wrong: grey, like the other "we didn't act" // rows, rather than the green of a change we actually made. else if (r.status === "already") console.log(line(r.key, ash("already registered"))); + else if (r.status === "missing") console.log(line(r.key, ash("not registered here"))); else if (r.status === "failed") console.log(line(r.key, err(`failed${r.code != null ? ` (code ${r.code})` : r.signal ? ` (${r.signal})` : ""}`))); else if (r.status === "not-installed") console.log(line(r.key, ash("not installed — /install " + r.key))); else console.log(line(r.key, ash(`skipped — ${r.reason}`))); @@ -255,12 +561,295 @@ function summarize(results) { */ const anyFailed = (results) => results.some((r) => r.status === "failed"); +/** + * Register one canonical spec across the chosen engines and say what happened. + * + * Its own function because three callers reach it now: `mcp install`, `mcp add` + * (flags or wizard) and `mcp enable`, and the notes it prints afterwards are + * the ones people actually read. `engineScope` and `engines` are the two axes + * of moshcode's scope model; both default to the widest useful answer. + */ +export async function fanOutAdd(spec, { + run, installedSet, engineScope = "user", engines = null, auth = null, catalog = null, +} = {}) { + const where = engines ? engines.join(", ") : "MCP engines"; + console.log(info(`registering ${bone(spec.name)} → ${ash(spec.target)} across ${where}…`)); + const plan = planMcpVerb("add", { ...spec, scope: engineScope }, { installedSet, engines }); + const results = await runMcpAdd(plan, run ? { run } : {}); + summarize(results); + // Credentials are named, never registered: an API key copied into five + // engines' config files is five places to leak it from and five to rotate. + const missing = (catalog?.env || []).filter((k) => !process.env[k]); + if (missing.length) { + console.log(ash(` note: ${spec.name} needs ${missing.join(" and ")} in the environment.`)); + } + // The catalog's own note and docs are printed whenever the catalog was used, + // not only when a variable is missing. A server whose credential is a header + // rather than an environment variable — or one that needs none at all to be + // useful — has nothing in `env`, and hanging its note off that check is what + // made the note invisible for exactly the servers it was written for. + if (catalog?.note) console.log(ash(` note: ${catalog.note}`)); + if (catalog?.docs) console.log(ash(` ${catalog.docs}`)); + if (spec.headers.length || /^https?:/i.test(spec.target)) { + console.log(ash(" note: OAuth/HTTP servers may still need per-engine auth. `/mcp reauth " + spec.name + "` drives it.")); + } + // A token typed on the command line is in the shell history before moshcode + // ever sees it and in every engine's config afterwards. moshcode has no vault + // to put it in (its own credentials are a 0600 dotfile), so the honest move is + // to say what just happened and name the form that avoids it. + if (auth?.from === "inline") { + console.log(ash(" note: that token is now in each engine's config, and in this shell's history.")); + console.log(ash(" `--token env:VAR` reads it from the environment instead, and is what gets recorded.")); + } + // Record only what actually landed somewhere. A spec that every engine + // skipped is not registered, and putting it in the list would make `/mcp + // list` claim a server this box does not have. + if (results.some((r) => r.status === "added" || r.status === "already")) { + recordServer(spec.name, { ...spec, auth }, { engineScope, engines }); + } + return anyFailed(results) ? 1 : 0; +} + +/* ------------------------------------------------------ the probe verbs */ + +/** + * `mcp test|resources|prompts|notifications `: hand it to mcpjam. + * + * The house rule is reuse before building, and TOOLS.mcpjam already states the + * split this function implements: moshcode registers a server across engines, + * mcpjam tells you whether the server is worth registering. What is left here + * is the part mcpjam cannot know: which server "sentry" means on this box. + */ +export async function runProbe(probe, { run = runCmd, probeInstalled = mcpjamInstalled } = {}) { + const resolved = resolveServerSpec(probe.name); + if (!resolved) { + console.log(err(`unknown server "${probe.name}". /mcp list shows the registered ones, or pass a URL`)); + return 1; + } + if (!probeInstalled()) { + const [headline, ...rest] = missingProbeTool(probe.verb); + console.log(err(headline)); + for (const extra of rest) console.log(ash(` ${extra}`)); + return 1; + } + if (resolved.unauthenticated) { + // The record keeps header names, never values. Saying so up front is the + // difference between "this server is broken" and "this probe is anonymous". + console.log(ash(` note: ${resolved.spec.name} authenticates with a header moshcode does not store`)); + console.log(ash(" this probe runs unauthenticated; register it with --token env:VAR to reuse the credential")); + } + const argv = mcpjamArgs(probe.verb, resolved.spec, probe); + console.log(info(`${probe.verb} ${bone(resolved.spec.name)} ${ash(`via ${MCPJAM.bin} (${resolved.source})`)}`)); + const result = await run(MCPJAM.bin, argv, { capture: false }); + return result?.ok && result.code === 0 ? 0 : 1; +} + +/** `mcp catalog search `: the local catalog, widened to the directories. */ +export async function runCatalogSearch(search, { run = runCmd, probeInstalled = mcpjamInstalled } = {}) { + // The local catalog is two curated entries and it is the better answer when + // it has one, so it is checked first and for free. + const local = resolveCatalog(search.query); + if (local) { + console.log(ok(`${bone(local.key)} ${ash("is in moshcode's own catalog")}`)); + console.log(` ${ash(local.desc)}`); + console.log(` ${acid(`/mcp install ${local.key}`)}`); + console.log(""); + } + if (!probeInstalled()) { + const [headline, ...rest] = missingProbeTool("catalog search"); + console.log(local ? info(headline) : err(headline)); + for (const extra of rest) console.log(ash(` ${extra}`)); + return local ? 0 : 1; + } + console.log(info(`searching the MCP directories for ${bone(search.query)} ${ash(`via ${MCPJAM.bin}`)}`)); + const result = await run(MCPJAM.bin, catalogSearchArgs(search.query, search), { capture: false }); + return result?.ok && result.code === 0 ? 0 : 1; +} + +/* ---------------------------------------------------- the name-addressed verbs */ + +// What each fan-out verb is called when it worked, and whether a non-zero exit +// that says "no such server" is a failure or just nothing to do. +const MANAGE_PLANS = { + remove: { verb: "remove", done: "removed", missing: true, capture: true }, + // reauth opens a browser and asks questions. Captured stdio swallows the + // prompt, so this one inherits the terminal and gives up the "already" + // detection that capturing buys. That is the right trade for a flow whose + // whole job is to talk to the person sitting there. + reauth: { verb: "reauth", done: "authorized", missing: false, capture: false }, + unauth: { verb: "unauth", done: "cleared", missing: true, capture: true }, + reconnect: { verb: "reconnect", done: "reconnected", missing: true, capture: true }, +}; + +/** + * `mcp remove|enable|disable|reauth|unauth|reconnect `. + * + * enable/disable are the two that needed a decision. No engine moshcode drives + * has an enable or disable verb. Not Claude Code, not Codex, not the Gemini + * family, not OpenCode. And moshcode will not edit their config files to fake + * one (prd/0003 rules that out, and it is the rule that keeps this whole + * command honest). So disable means what a wrapper can actually deliver: take + * the server out of every engine, and keep its spec here so `enable` can put + * exactly the same one back. It is a real round trip, not a flag nobody reads, + * and the help text says exactly that so nobody expects a live toggle. + */ +export async function runManage(manage, { run, installedSet } = {}) { + const { verb, name } = manage; + const recorded = name ? getServer(name) : null; + // What the server was registered WITH is the right default for acting on it + // again. Without this, `mcp add x --engines claude` followed by `mcp remove x` + // sends a removal to all six engines. Five never had it, and two belong to + // somebody who never asked moshcode to touch them. An + // explicit flag still wins, because the record is moshcode's memory and not + // an authority over what the user just typed. + const engineScope = manage.engineScope || recorded?.engineScope; + const engines = manage.engines || recorded?.engines || null; + const where = engines ? engines.join(", ") : "MCP engines"; + + if (verb === "disable" || verb === "enable") { + if (verb === "enable") { + if (!recorded) { + console.log(err(`nothing recorded for "${name}". moshcode can only re-enable a server it registered`)); + console.log(ash(" /mcp list shows them; /mcp install registers a new one")); + return 1; + } + const spec = { + name, target: recorded.target, args: recorded.args || [], transport: recorded.transport || undefined, + env: [], headers: [], + }; + console.log(info(`re-registering ${bone(name)} → ${ash(spec.target)} across ${where}…`)); + const results = await runMcpVerb( + planMcpVerb("add", { ...spec, scope: engineScope || "user" }, { installedSet, engines }), + { ...(run ? { run } : {}), done: "enabled", already: true }, + ); + summarize(results); + if (recorded.headers?.length || recorded.auth) { + console.log(ash(" note: the credential was never stored here. re-run /mcp install with --token to restore it")); + } + setServerEnabled(name, true); + return anyFailed(results) ? 1 : 0; + } + console.log(info(`deregistering ${bone(name)} from ${where}; moshcode keeps the spec…`)); + const results = await runMcpVerb( + planMcpVerb("remove", { name, scope: engineScope }, { installedSet, engines }), + { ...(run ? { run } : {}), done: "disabled", missing: true }, + ); + summarize(results); + if (recorded) setServerEnabled(name, false); + else console.log(ash(` note: ${name} was not in moshcode's record, so /mcp enable ${name} has nothing to restore`)); + return anyFailed(results) ? 1 : 0; + } + + const plan = MANAGE_PLANS[verb]; + const spec = { name, scope: engineScope, all: manage.all }; + const what = manage.all ? "every server" : bone(name); + console.log(info(`${verb} ${what} across ${where}…`)); + const results = await runMcpVerb( + planMcpVerb(plan.verb, spec, { installedSet, engines }), + { ...(run ? { run } : {}), done: plan.done, missing: plan.missing, capture: plan.capture }, + ); + summarize(results); + // Removing it from the engines and leaving it in moshcode's own list would + // make `/mcp list` lie about the box it is printed on. + if (verb === "remove") forgetServer(name); + if (verb === "reauth") { + console.log(ash(" each engine ran its own OAuth 2.1 flow (auth code + PKCE) and keeps its own refresh token")); + console.log(ash(" moshcode stored nothing. /mcp unauth " + name + " clears them again")); + } + return anyFailed(results) ? 1 : 0; +} + +/* ------------------------------------------------------------- the wizard */ + +/** One prompt, defaulting when the answer is empty. Injectable so tests never block. */ +async function promptLine(question, fallback = "") { + const { createInterface } = await import("node:readline/promises"); + const rl = createInterface({ input: process.stdin, output: process.stdout }); + try { + const answer = (await rl.question(question)).trim(); + return answer || fallback; + } finally { rl.close(); } +} + +/** + * `mcp add` with no arguments. + * + * The flag form is the one worth learning and the wizard prints it at the end + * rather than hiding it, so the second time somebody registers a server they do + * not need this. Refusing outright without a terminal matters more than it + * looks: `mcp add` in a script would otherwise hang forever on a read that + * nothing is going to answer. + */ +export async function runAddWizard({ run, installedSet, ask = promptLine, isTty = () => process.stdin.isTTY } = {}) { + if (!isTty()) { + console.log(err("`mcp add` with no arguments opens a wizard, and there is no terminal here")); + console.log(ash(" non-interactively: mcp add --url [--transport http|sse] [--token env:VAR]")); + console.log(ash(" or: mcp add -- ")); + return 1; + } + console.log(bone(" add an MCP server") + ash(" blank answers take the default in brackets")); + const name = await ask(" name: "); + if (!name) { console.log(err("a server needs a name")); return 1; } + const target = await ask(" url, or a command to run: "); + if (!target) { console.log(err("a server needs a URL or a command")); return 1; } + const remote = isRemoteTarget(target); + const transport = remote ? (await ask(" transport [http]: ", "http")) : undefined; + if (transport && !["http", "sse"].includes(transport)) { + console.log(err("transport must be http or sse")); + return 1; + } + const tokenVar = remote ? await ask(" auth token from which env var (blank for none): ") : ""; + const engineScope = (await ask(" config scope in each engine [user]: ", "user")).toLowerCase(); + if (!MCP_SCOPES.includes(engineScope)) { console.log(err(`scope must be ${MCP_SCOPES.join(" or ")}`)); return 1; } + const enginesAnswer = await ask(" engines [all]: ", "all"); + const engines = enginesAnswer === "all" ? null : resolveMcpEngines(enginesAnswer).engines; + if (enginesAnswer !== "all" && !engines) { console.log(err(`unknown engine in "${enginesAnswer}"`)); return 1; } + + const [command, ...args] = remote ? [target] : target.split(/\s+/); + const headers = []; + let auth = null; + if (tokenVar) { + const value = process.env[tokenVar]; + if (!value) { console.log(err(`${tokenVar} is not set in this environment`)); return 1; } + headers.push(`Authorization: Bearer ${value}`); + auth = { header: "Authorization", from: `env:${tokenVar}` }; + } + const spec = { name, target: command, args, transport, env: [], headers }; + // The flag form, printed before anything is written: the next server this + // person registers should not need the wizard at all. + const flags = [ + `moshcode mcp add ${name}`, + remote ? `--url ${target}` : "", + transport && transport !== "http" ? `--transport ${transport}` : "", + tokenVar ? `--token env:${tokenVar}` : "", + engineScope !== "user" ? `--engine-scope ${engineScope}` : "", + engines ? `--engines ${engines.join(",")}` : "", + remote ? "" : `-- ${target}`, + ].filter(Boolean).join(" "); + console.log(ash(` next time: ${flags}`)); + return fanOutAdd(spec, { run, installedSet, engineScope, engines, auth, catalog: null }); +} + /** Run `/mcp …`. `tokens` are the words after `mcp`. `run`/`installedSet` are injectable for tests. */ export async function mcpCommand(tokens, { run, installedSet, sessionId, ensureSession, sharingDisabled = false, fetchImpl, credentials, login, + probeInstalled, ask, isTty, } = {}) { const parsed = parseMcp(tokens); - if (parsed.list) { printMcpTargets(parsed.json); return 0; } + if (parsed.list) { + if (parsed.servers) { + if (parsed.json) console.log(JSON.stringify(listServers(), null, 2)); + else printMcpServers(); + return 0; + } + // `--json` keeps returning exactly the engine capability array it always + // did. The servers block is for the human view, where two questions + // ("what did I register" and "what can take it") belong on one screen. + if (!parsed.json) printMcpServers(); + printMcpTargets(parsed.json); + return 0; + } + if (parsed.help) { console.log(renderCommand(findCommand("mcp"))); return 0; } if (parsed.showCatalog) { printMcpCatalog(); return 0; } if (parsed.bridge) { const { serveBridge } = await import("./mcp.mjs"); @@ -268,6 +857,10 @@ export async function mcpCommand(tokens, { return serveBridge({ version: moshcodeVersion() || "" }); } if (parsed.error) { console.log(err(parsed.error)); return 1; } + if (parsed.catalogSearch) return runCatalogSearch(parsed.catalogSearch, { run, probeInstalled }); + if (parsed.wizard) return runAddWizard({ run, installedSet, ask, isTty }); + if (parsed.probe) return runProbe(parsed.probe, { run, probeInstalled }); + if (parsed.manage) return runManage(parsed.manage, { run, installedSet }); if (parsed.remote) { const options = { fetchImpl, credentials, login }; try { @@ -324,27 +917,10 @@ export async function mcpCommand(tokens, { } } - const { spec } = parsed; - console.log(info(`registering ${bone(spec.name)} → ${ash(spec.target)} across MCP engines…`)); - const results = await runMcpAdd(planMcpAdd(spec, { installedSet }), run ? { run } : {}); - summarize(results); - // Credentials are named, never registered: an API key copied into five - // engines' config files is five places to leak it from and five to rotate. - const missing = (parsed.catalog?.env || []).filter((k) => !process.env[k]); - if (missing.length) { - console.log(ash(` note: ${spec.name} needs ${missing.join(" and ")} in the environment.`)); - } - // The catalog's own note and docs are printed whenever the catalog was used, - // not only when a variable is missing. A server whose credential is a header - // rather than an environment variable — or one that needs none at all to be - // useful — has nothing in `env`, and hanging its note off that check is what - // made the note invisible for exactly the servers it was written for. - if (parsed.catalog?.note) console.log(ash(` note: ${parsed.catalog.note}`)); - if (parsed.catalog?.docs) console.log(ash(` ${parsed.catalog.docs}`)); - if (spec.headers.length || /^https?:/i.test(spec.target)) { - console.log(ash(" note: OAuth/HTTP servers may still need per-engine auth (e.g. `opencode mcp auth`, `codex mcp login`).")); - } - return anyFailed(results) ? 1 : 0; + return fanOutAdd(parsed.spec, { + run, installedSet, engineScope: parsed.engineScope, engines: parsed.engines, + auth: parsed.auth, catalog: parsed.catalog, + }); } /** Run `/skill …`. `tokens` are the words after `skill`. `run`/`installedSet` are injectable for tests. */ diff --git a/src/mcp-probe.mjs b/src/mcp-probe.mjs new file mode 100644 index 00000000..bc4ae10c --- /dev/null +++ b/src/mcp-probe.mjs @@ -0,0 +1,128 @@ +// The half of `/mcp` that talks to a server instead of registering one. +// +// moshcode does not implement an MCP client and is not going to. It already +// installs one: `mcpjam` is in TOOLS, and its entry there says what the split +// is meant to be. "The companion to `moshcode mcp`: that registers a server +// across engines, this one tells you whether the server is actually worth +// registering". So `test`, `resources`, `prompts` and `notifications` build an +// mcpjam command line out of the canonical spec and hand over. Nothing here +// speaks JSON-RPC, opens a socket, or holds a protocol version that will be +// wrong in six weeks. +// +// The reuse is more than a shortcut. mcpjam's flag surface is uniform across +// its subcommands: the same --transport/--url/--header/--command/--args/-e on +// every one of them. That is why one builder covers four verbs. The engines +// moshcode registers into have six different dialects; the thing it *probes* +// with has one. +import { TOOLS } from "./tools.mjs"; +import { isInstalled } from "./engines.mjs"; +import { isRemoteTarget } from "./mcp.mjs"; + +export const MCPJAM = TOOLS.mcpjam; + +/** Is the probe tool on this box? Injectable for tests. */ +export function mcpjamInstalled(probe = isInstalled) { + return probe(MCPJAM.bin, MCPJAM.binDirs); +} + +/** + * The mcpjam flags that describe WHICH server to talk to. + * + * mcpjam takes "http" or "stdio" and resolves SSE underneath; moshcode's spec + * may say "sse" because that is a word the engines accept. Collapsing it to + * http here rather than passing it through keeps the failure honest: an + * unsupported --transport value would come back as a usage error about a flag + * the user never typed. + */ +export function mcpjamTargetArgs(spec) { + const argv = []; + if (isRemoteTarget(spec.target)) { + argv.push("--transport", "http", "--url", spec.target); + for (const header of spec.headers || []) argv.push("--header", header); + } else { + argv.push("--transport", "stdio", "--command", spec.target); + if (spec.args?.length) argv.push("--args", ...spec.args); + } + for (const [key, value] of spec.env || []) argv.push("-e", `${key}=${value}`); + return argv; +} + +// Which mcpjam subcommand answers each moshcode verb. +// +// `notifications` is the one that needed a judgement. mcpjam can stream a live +// subscription (`subscriptions listen`), which blocks until Ctrl-C, and it can +// read the capabilities a server declares, which returns. A verb whose default +// hangs the terminal is a verb people run once, so the default is the reading +// and `--listen` is the stream. +const PROBES = { + test: () => ["server", "info"], + resources: () => ["resources", "list"], + prompts: () => ["prompts", "list"], + notifications: ({ listen = false, durationMs = null } = {}) => ( + listen + ? ["subscriptions", "listen", "--list-changed", ...(durationMs ? ["--duration-ms", String(durationMs)] : [])] + : ["server", "capabilities"] + ), +}; + +export const PROBE_VERBS = Object.keys(PROBES); + +/** + * The full mcpjam argv for one probe verb against one server spec. + * + * Program-level flags go before the subcommand. mcpjam declares --format on the + * program rather than on each command, and a parser that accepts it in both + * places today is not a promise it will tomorrow. + */ +export function mcpjamArgs(verb, spec, options = {}) { + const probe = PROBES[verb]; + if (!probe) throw new Error(`no mcpjam probe for "${verb}"`); + return [ + ...(options.json ? ["--format", "json"] : []), + ...probe(options), + ...mcpjamTargetArgs(spec), + ]; +} + +/** + * `mcp catalog search `: the generic form of the Smithery verbs. + * + * The parity list asked for `smithery-search`, `smithery-login` and + * `smithery-logout`: three top-level verbs branded with one registry's name. + * moshcode already has a catalog, and a second parallel registry concept beside + * it is how you end up explaining to someone why `mcp catalog` and + * `mcp smithery-search` disagree about what exists. So searching is a verb of + * the catalog moshcode already has, and the registry behind it is an + * implementation detail. It is mcpjam's `registry search`, which sweeps the + * scraped MCP directories (Smithery among them) rather than one vendor's. + * + * That also disposes of the login pair. A registry API key is mcpjam's + * credential, mcpjam already stores it (MCPJAM_API_KEY, `mcpjam cloud login`), + * and moshcode caching a second copy would be a second place to leak it from + * and a second one to rotate. Same rule the catalog states about `env`. + * When the search needs a key, mcpjam says so in its own words. + */ +export function catalogSearchArgs(query, { limit = null, source = null, json = false } = {}) { + return [ + ...(json ? ["--format", "json"] : []), + "registry", "search", query, + ...(source ? ["--source", source] : []), + ...(limit ? ["--limit", String(limit)] : []), + ]; +} + +/** + * What to print when mcpjam is not installed. + * + * Kept as data rather than a console.log so the caller owns the colours and a + * test can assert the words without capturing stdout. "Degrade gracefully with + * a clear message" means naming the command that fixes it: `/install mcpjam` + * already exists and already does the right thing. + */ +export function missingProbeTool(verb) { + return [ + `\`mcp ${verb}\` talks to the server through mcpjam, which is not installed`, + "install it with `moshcode install mcpjam` (or `/install mcpjam` in the pit), then run this again", + `mcpjam is a tool moshcode already knows about: ${MCPJAM.desc}`, + ]; +} diff --git a/src/mcp-registry.mjs b/src/mcp-registry.mjs new file mode 100644 index 00000000..9fa96bbf --- /dev/null +++ b/src/mcp-registry.mjs @@ -0,0 +1,165 @@ +// What moshcode registered, so a NAME is enough to act on a server again. +// +// This file exists because of a gap the fan-out left behind. `mcp install` +// hands six engines a canonical spec and then forgets it, which is fine while +// the only verb is "register". The moment a verb takes a name there is nothing +// to resolve it against: test this one, list its resources, disable it, bring +// it back. Reading it back out of six config formats was the other +// option and it is the one prd/0003 explicitly rules out: moshcode drives each +// engine's own CLI and never parses or writes its config. +// +// So: ~/.moshcode/mcp.json, moshcode's own record of what moshcode did. Two +// things it is NOT, both worth saying out loud because the file looks like it +// might be either. +// +// It is not the source of truth. Each engine's config is. A server someone +// added with `claude mcp add` directly is real, works fine, and is not in here, +// which is why every name-addressed verb falls through to the catalog and to a +// bare URL before it gives up. +// +// It is not a credential store. A header value is a credential; it goes to the +// engines' own configs, which the user has already chosen to trust with it, and +// it never lands here. What this file keeps is the header NAME and, when the +// value came from the environment, the variable it came from. Enough to build +// the same request again, and nothing anyone can read a secret out of. Same rule +// src/mcp-catalog.mjs states for `env` and src/payments.mjs states for gateway +// keys. The file is still 0600, because the list of servers you talk to is +// nobody else's business on a shared box. +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const FILE_MODE = 0o600; +const DIR_MODE = 0o700; + +/** Current on-disk shape. Bumped only when a migration is actually needed. */ +export const SCHEMA_VERSION = 1; + +const EMPTY = { version: SCHEMA_VERSION, servers: {} }; + +/** + * Where the record lives. + * + * Deliberately not under `MOSHCODE_HOME`: that variable means the directory + * moshcode is *installed* in, so honouring it here would file a server list + * inside the package. Same reasoning and same directory as src/aliases.mjs and + * src/business-store.mjs. `MOSHCODE_MCP_FILE` moves it for tests. + */ +export function registryFile() { + return process.env.MOSHCODE_MCP_FILE || path.join(os.homedir(), ".moshcode", "mcp.json"); +} + +/** + * Read the record, or an empty one. + * + * Every failure reads as "nothing registered yet": missing, unreadable, + * truncated by a crash mid-write, or hand-edited into something that is not an + * object. These reads sit on command paths a person is waiting on, and a + * SyntaxError thrown at somebody who typed `/mcp list` tells them nothing they + * can act on, while an empty list does. + */ +export function loadRegistry() { + let parsed; + try { parsed = JSON.parse(fs.readFileSync(registryFile(), "utf8")); } + catch { return structuredClone(EMPTY); } + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return structuredClone(EMPTY); + const servers = parsed.servers && typeof parsed.servers === "object" && !Array.isArray(parsed.servers) + ? parsed.servers + : {}; + return { version: SCHEMA_VERSION, servers }; +} + +/** + * Write it back atomically. + * + * Rename-over rather than write-in-place: two pits are a normal way to use + * moshcode, and `/mcp disable a` in one while `/mcp install b` runs in the + * other must not be able to leave the file half-written. + */ +function save(data) { + const file = registryFile(); + fs.mkdirSync(path.dirname(file), { recursive: true, mode: DIR_MODE }); + const tmp = `${file}.${process.pid}.tmp`; + fs.writeFileSync(tmp, `${JSON.stringify(data, null, 2)}\n`, { mode: FILE_MODE }); + fs.renameSync(tmp, file); +} + +/** + * Strip a spec down to what is safe to keep. + * + * Headers become names. `Authorization: Bearer sk-live-…` is the single most + * likely thing to be typed at this command and the single worst thing to write + * to disk, so the value is dropped here rather than anywhere further out, where + * a new call site could forget. `auth` records where a value came from when it + * came from somewhere nameable (an environment variable), so `mcp test` can + * rebuild the same request without ever having stored the secret. + */ +export function redactSpec(spec) { + return { + target: spec.target, + args: [...(spec.args || [])], + transport: spec.transport || null, + // Environment PAIRS are values too. The names are the useful half and the + // only half that is safe, exactly as the catalog already treats `env`. + env: (spec.env || []).map(([key]) => key), + headers: (spec.headers || []).map((h) => String(h).split(":")[0].trim()).filter(Boolean), + ...(spec.auth ? { auth: { header: spec.auth.header, from: spec.auth.from } } : {}), + }; +} + +/** Record a server moshcode just registered. Merges over any earlier entry. */ +export function recordServer(name, spec, { engineScope = "user", engines = null } = {}) { + const data = loadRegistry(); + const previous = data.servers[name] || {}; + data.servers[name] = { + ...previous, + ...redactSpec(spec), + engineScope, + engines, + enabled: true, + registeredAt: new Date().toISOString(), + }; + save(data); + return data.servers[name]; +} + +/** Forget a server entirely. Returns whether there was one. */ +export function forgetServer(name) { + const data = loadRegistry(); + if (!Object.hasOwn(data.servers, name)) return false; + delete data.servers[name]; + save(data); + return true; +} + +/** + * Mark a server enabled or disabled, keeping its spec either way. + * + * The spec surviving a disable is the whole point: `disable` deregisters the + * server from every engine, and without a kept spec `enable` would have nothing + * to register again and the word would be a one-way door. + */ +export function setServerEnabled(name, enabled) { + const data = loadRegistry(); + const entry = data.servers[name]; + if (!entry) return null; + entry.enabled = Boolean(enabled); + entry.enabledAt = new Date().toISOString(); + save(data); + return entry; +} + +/** One server by name, or null. Own properties only. */ +export function getServer(name) { + if (!name) return null; + const { servers } = loadRegistry(); + // `servers` comes from JSON.parse, so `constructor` and `__proto__` would + // otherwise resolve to something off Object.prototype with no target. + return Object.hasOwn(servers, name) ? { name, ...servers[name] } : null; +} + +/** Every recorded server, name-sorted, for `/mcp list`. */ +export function listServers() { + const { servers } = loadRegistry(); + return Object.keys(servers).sort().map((name) => ({ name, ...servers[name] })); +} diff --git a/src/mcp.mjs b/src/mcp.mjs index f272d4aa..61ffa713 100644 --- a/src/mcp.mjs +++ b/src/mcp.mjs @@ -51,18 +51,20 @@ function headerToEq(header) { * Build one engine's native `mcp add` argv for a canonical server spec, or a * skip reason when the engine can't express it. * - * spec: { name, target, args?, transport?, env?: [[k,v]], headers?: ["Key: Value"] } + * spec: { name, target, args?, transport?, scope?, env?: [[k,v]], headers?: ["Key: Value"] } * `target` is a URL (remote) or a stdio command; `args` are stdio command args. + * `scope` is "user" (the default everywhere) or "project"; see MCP_SCOPES for + * why moshcode's scope is two axes and this is only one of them. * Returns { argv } or { skip }. */ export function mcpAddArgs(key, spec) { - const { name, target, args = [], env = [], headers = [] } = spec; + const { name, target, args = [], env = [], headers = [], scope = "user" } = spec; const remote = isRemoteTarget(target); const transport = spec.transport || (remote ? "http" : "stdio"); switch (key) { case "claude": { - const argv = ["mcp", "add", "-s", "user"]; + const argv = ["mcp", "add", "-s", scope]; if (remote) argv.push("-t", transport); for (const [k, v] of env) argv.push("-e", `${k}=${v}`); for (const h of headers) argv.push("-H", h); @@ -76,7 +78,7 @@ export function mcpAddArgs(key, spec) { // builder rather than getting a copy, so the two can only drift on purpose. case "gemini": case "qwen": { - const argv = ["mcp", "add", "-s", "user"]; + const argv = ["mcp", "add", "-s", scope]; if (remote) argv.push("-t", transport); for (const [k, v] of env) argv.push("-e", `${k}=${v}`); for (const h of headers) argv.push("-H", h); @@ -94,10 +96,21 @@ export function mcpAddArgs(key, spec) { // and a more useful one than the blanket "no MCP support", which would // read as "kimi cannot do MCP at all". return { skip: "no scriptable `mcp add` — add it in kimi with /mcp-config, or in ~/.kimi-code/mcp.json" }; + case "omp": + // omp has the richest MCP surface of any engine here: /mcp add, test, + // enable, disable, reauth, reload, resources, prompts, the lot. Every one + // of them is a TUI slash command that writes .omp/mcp.json or + // ~/.omp/agent/mcp.json itself. Its shell CLI has no `mcp` subcommand at + // all (omp.sh/docs/cli lists thirty-odd others and not that one). So this + // is kimi's situation, not aider's, and the generic "no MCP support" was + // a claim about the engine that happens to be false: it supports MCP + // better than most, and moshcode simply has no command to drive. + return { skip: "no scriptable `mcp` subcommand. add it in omp with /mcp add, or in .omp/mcp.json" }; case "codex": { if (headers.length) { return { skip: "Codex supports only a bearer-token env var, not literal headers" }; } + if (scope === "project") return { skip: NO_PROJECT_SCOPE.codex }; const argv = ["mcp", "add", name]; for (const [k, v] of env) argv.push("--env", `${k}=${v}`); if (remote) argv.push("--url", target); @@ -111,6 +124,7 @@ export function mcpAddArgs(key, spec) { if (!remote) { return { skip: `${key === "privacycode" ? "privacycode" : "OpenCode"} CLI adds only remote (--url) servers non-interactively` }; } + if (scope === "project") return { skip: NO_PROJECT_SCOPE[key] }; const argv = ["mcp", "add", name, "--url", target]; for (const [k, v] of env) argv.push("--env", `${k}=${v}`); for (const h of headers) argv.push("--header", headerToEq(h)); @@ -133,13 +147,8 @@ export function mcpAddArgs(key, spec) { * capability set (it is what the matrix splits "supported" on); it is only the * iteration that widens. Supported engines keep their existing order. */ -export function planMcpAdd(spec, { installedSet } = {}) { - const rest = Object.keys(ENGINES).filter((key) => !MCP_ENGINES.includes(key)); - return [...MCP_ENGINES, ...rest].map((key) => { - const bin = ENGINES[key].bin; - const installed = installedSet ? installedSet.has(key) : isInstalled(bin, ENGINES[key].binDirs); - return { key, bin, installed, ...mcpAddArgs(key, spec) }; - }); +export function planMcpAdd(spec, options = {}) { + return planMcpVerb("add", spec, options); } /** @@ -165,15 +174,278 @@ export function alreadyRegistered(r) { * results [{ key, status: "added"|"already"|"skipped"|"failed"|"not-installed", reason? }]. * `run` is injectable for tests; defaults to the real spawner. */ -export async function runMcpAdd(plan, { run = runCmd } = {}) { +export async function runMcpAdd(plan, options = {}) { + // capture so a non-zero exit can be read for "already exists" rather than + // reported as a failure; the child's output still reaches the terminal. + return runMcpVerb(plan, { ...options, done: "added", already: true }); +} + +/* ------------------------------------------------------------------ scope */ + +// moshcode's scope carries two axes, not one. +// +// Every other tool's `--scope project|user` answers one question: which config +// file. That is only half the question here, because moshcode registers a +// server in SIX engines at once, and the other half is which engines. Reducing +// scope to project|user would quietly throw away the thing moshcode exists to +// do, so the two axes are two flags: +// +// --engine-scope user|project which config file each engine writes +// --engines claude,codex which engines are written to at all +// +// Both default to the widest useful answer: user scope, every installed +// MCP-capable engine. The short form still behaves the way it does everywhere +// else. +// +// The first flag is NOT called `--scope`, and that is not a style choice. +// `/mcp answer --scope sessions:read,sessions:write` already means the OAuth +// permission scope of a shared session, and one word meaning two things across +// sibling verbs of the same command is how a person hands a session write +// access while trying to pick a config file. `--engine-scope` says which scope +// it is in the name, and `parseMcp` catches a bare `--scope` on these verbs and +// points at it rather than failing silently. +export const MCP_SCOPES = ["user", "project"]; + +// Claude Code also has a `local` scope (this project, this machine, private to +// you). It is deliberately not in MCP_SCOPES: no other engine has it, so +// `--engine-scope local` would be a flag that silently means "claude only", and +// moshcode already spells that `--engines claude`. + +/** Which engines a `--engines a,b` selection resolves to. Returns { engines } or { error }. */ +export function resolveMcpEngines(selection) { + if (!selection) return { engines: null }; // null means "every engine" + const asked = String(selection).split(",").map((s) => s.trim().toLowerCase()).filter(Boolean); + if (!asked.length) return { error: "--engines needs at least one engine name" }; + const unknown = asked.filter((key) => !Object.hasOwn(ENGINES, key)); + if (unknown.length) { + return { error: `unknown engine ${unknown.map((u) => `"${u}"`).join(", ")}. try ${MCP_ENGINES.join(", ")}` }; + } + return { engines: asked }; +} + +// An engine that keeps MCP servers in exactly one place cannot honour `--scope +// project`. Stated per engine rather than as one shared string, because the +// reasons differ and R6 asks for a reason the reader can act on. +const NO_PROJECT_SCOPE = { + codex: "Codex keeps MCP servers in ~/.codex/config.toml only, so there is no project scope", + opencode: "OpenCode's `mcp add` takes no scope flag. it writes its own config, not a project .mcp.json", + privacycode: "privacycode's `mcp add` takes no scope flag. it writes its own config, not a project .mcp.json", +}; + +// Kimi Code runs MCP servers and simply has no scriptable subcommand for any of +// this. One string for the five new verbs so they cannot drift into five +// different stories about the same engine. `mcpAddArgs` keeps its own, longer +// version because it can name the exact command that is missing. +const KIMI_SKIP = "no scriptable `mcp` subcommand. use kimi's /mcp-config, or ~/.kimi-code/mcp.json"; + +// Same shape, different engine. omp does all of this from its own TUI and edits +// its own config file; nothing about it is reachable from a shell. See the +// longer note in `mcpAddArgs`. +const OMP_SKIP = "no scriptable `mcp` subcommand. omp does this from its TUI (/mcp) and its own config"; + +/* ---------------------------------------------------------------- removal */ + +/** + * One engine's native `mcp remove` argv, or a skip reason. + * + * spec: { name, scope? }. Same shape as `mcpAddArgs` so the fan-out planner can + * take either builder and the summary reads the same way either way. + */ +export function mcpRemoveArgs(key, spec) { + const { name, scope } = spec; + switch (key) { + // Claude, Gemini and Qwen all spell it `mcp remove [-s scope] `. Qwen + // is a Gemini CLI fork and kept the flag; Claude arrived at the same shape + // on its own. They share the branch so a future divergence has to be + // written down rather than discovered by a user. + case "claude": + case "gemini": + case "qwen": { + const argv = ["mcp", "remove"]; + if (scope) argv.push("-s", scope); + argv.push(name); + return { argv }; + } + case "codex": + if (scope === "project") return { skip: NO_PROJECT_SCOPE.codex }; + return { argv: ["mcp", "remove", name] }; + // OpenCode registers servers and never un-registers them: `opencode mcp` + // offers add, list, auth, logout and debug, and nothing that deletes. The + // reason points at the file rather than shrugging, because deleting the + // entry by hand is a real answer and the user should not have to go hunting + // for it. + case "opencode": + case "privacycode": + return { skip: `${key} has no \`mcp remove\`. delete the entry from its config (\`${key} mcp list\` names it)` }; + case "kimi": + return { skip: KIMI_SKIP }; + case "omp": + return { skip: OMP_SKIP }; + default: + return { skip: "no MCP support" }; + } +} + +/* ------------------------------------------------------------------ oauth */ + +/** + * One engine's native "authorize me against this server" argv, or a skip reason. + * + * moshcode mints no tokens and stores none. Every engine below runs the MCP + * spec's own OAuth 2.1 flow against the server: authorization code with PKCE, + * and a refresh token the engine rotates itself. Each keeps the result in its + * own credential store. So `reauth` hands the terminal to that flow once per + * engine and `unauth` clears what it left behind. There is no moshcode-side + * copy of a token to leak, and nothing on this path is ever OAuth 1.0a. + * + * Gemini and Qwen are the gap: both authorize an OAuth MCP server from inside + * the session rather than from their CLI, so both are skipped with the words + * the user would have to type instead. + */ +export function mcpAuthArgs(key, spec) { + const { name } = spec; + switch (key) { + case "claude": + case "codex": + return { argv: ["mcp", "login", name] }; + case "opencode": + case "privacycode": + return { argv: ["mcp", "auth", name] }; + case "gemini": + case "qwen": + return { skip: `${key} authorizes from inside the session. run \`${key}\` and use /mcp auth ${name}` }; + case "kimi": + return { skip: KIMI_SKIP }; + case "omp": + return { skip: OMP_SKIP }; + default: + return { skip: "no MCP support" }; + } +} + +/** One engine's native "forget this server's OAuth credentials" argv, or a skip reason. */ +export function mcpUnauthArgs(key, spec) { + const { name } = spec; + switch (key) { + case "claude": + case "codex": + case "opencode": + case "privacycode": + return { argv: ["mcp", "logout", name] }; + case "gemini": + case "qwen": + return { skip: `${key} clears credentials from inside the session. run \`${key}\` and use /mcp` }; + case "kimi": + return { skip: KIMI_SKIP }; + case "omp": + return { skip: OMP_SKIP }; + default: + return { skip: "no MCP support" }; + } +} + +/* -------------------------------------------------------------- reconnect */ + +/** + * One engine's native `mcp reconnect` argv, or a skip reason. + * + * Only the Gemini family has this: `gemini mcp reconnect [name] [-a]` drops the + * engine's live client for a server and dials it again. Nothing equivalent + * exists on Claude Code, Codex or OpenCode, and moshcode holds no MCP client of + * its own, so those engines get the thing that does work instead of a green + * tick over a command that changed nothing. + */ +export function mcpReconnectArgs(key, spec) { + const { name, all = false } = spec; + switch (key) { + case "gemini": + case "qwen": + return { argv: all ? ["mcp", "reconnect", "--all"] : ["mcp", "reconnect", name] }; + case "claude": + return { skip: "Claude Code reconnects from inside the session. run `claude` and use /mcp" }; + case "codex": + return { skip: "Codex has no `mcp reconnect`. it dials each server when a session starts" }; + case "opencode": + case "privacycode": + return { skip: `${key} has no \`mcp reconnect\`. \`${key} mcp debug ${name}\` inspects the connection` }; + case "kimi": + return { skip: KIMI_SKIP }; + case "omp": + return { skip: OMP_SKIP }; + default: + return { skip: "no MCP support" }; + } +} + +/* ------------------------------------------------------------ the fan-out */ + +/** The argv builder behind each fan-out verb. */ +export const MCP_VERB_BUILDERS = { + add: mcpAddArgs, + remove: mcpRemoveArgs, + reauth: mcpAuthArgs, + unauth: mcpUnauthArgs, + reconnect: mcpReconnectArgs, +}; + +/** + * Plan any fan-out verb: one entry per engine with its native argv or skip + * reason, annotated with install status. The generalization of `planMcpAdd`, + * which now delegates here so the six verbs cannot drift on engine coverage. + * + * `engines` narrows the fan-out to a chosen set. That is the second axis of + * moshcode's scope model. Narrowing DROPS the other engines rather than listing them as + * skipped: an engine the user deliberately left out is not a surprise that + * needs explaining, and four "skipped" rows under a command that asked for one + * engine bury the row that matters. R6's no-silent-omission rule is about + * engines moshcode chose to leave out, not engines the user did. + */ +export function planMcpVerb(verb, spec, { installedSet, engines = null } = {}) { + const build = MCP_VERB_BUILDERS[verb]; + if (!build) throw new Error(`no MCP fan-out builder for "${verb}"`); + const rest = Object.keys(ENGINES).filter((key) => !MCP_ENGINES.includes(key)); + const all = [...MCP_ENGINES, ...rest]; + const keys = engines ? all.filter((key) => engines.includes(key)) : all; + return keys.map((key) => { + const bin = ENGINES[key].bin; + const installed = installedSet ? installedSet.has(key) : isInstalled(bin, ENGINES[key].binDirs); + return { key, bin, installed, ...build(key, spec) }; + }); +} + +/** + * Did this engine exit non-zero only because the server was not there to + * remove? The mirror of `alreadyRegistered`, for the same reason: removing a + * server that two of six engines never had is the normal shape of `mcp remove`, + * and painting those two rows red teaches the reader to ignore the colour. + */ +const NOT_FOUND_RE = /no (?:such|mcp) server|not found|does not exist|is not (?:configured|registered)|no server (?:named|found)/i; +export function notRegistered(r) { + return NOT_FOUND_RE.test(String(r?.output ?? "")); +} + +/** + * Execute any fan-out plan. Returns + * [{ key, status: |"already"|"missing"|"skipped"|"failed"|"not-installed" }]. + * + * `done` is the verb's own past tense, so the summary says what happened rather + * than "ok". `capture` is false for the interactive verbs: `reauth` opens a + * browser and asks questions, and captured stdio swallows the prompt. + */ +export async function runMcpVerb(plan, { + run = runCmd, done = "done", capture = true, already = false, missing = false, +} = {}) { const results = []; for (const item of plan) { if (item.skip) { results.push({ key: item.key, status: "skipped", reason: item.skip }); continue; } if (!item.installed) { results.push({ key: item.key, status: "not-installed" }); continue; } - // capture so a non-zero exit can be read for "already exists" rather than - // reported as a failure; the child's output still reaches the terminal. - const r = await run(item.bin, item.argv, { capture: true }); - const status = ranOk(r) ? "added" : alreadyRegistered(r) ? "already" : "failed"; + const r = await run(item.bin, item.argv, { capture }); + let status = done; + if (!ranOk(r)) { + if (already && alreadyRegistered(r)) status = "already"; + else if (missing && notRegistered(r)) status = "missing"; + else status = "failed"; + } results.push({ key: item.key, status, code: r.code, signal: r.signal ?? null }); } return results; diff --git a/test/help.test.mjs b/test/help.test.mjs index b5dc07f2..b30682c9 100644 --- a/test/help.test.mjs +++ b/test/help.test.mjs @@ -135,9 +135,14 @@ test("a bad sub-verb prints that command's usage, not the whole wall", async () assert.equal(code, 1); assert.equal(stdout, ""); assert.match(stderr, /has no verb "nonsense"/); - // A ceiling, not a target: the whole wall is 127 lines. It moves when a verb - // is added, which is why it is loose rather than exact. - assert.ok(stderr.split("\n").length <= 30, "usage block should be the command's own, and short"); + // A ceiling, not a target: the whole wall is 127 lines. It moved from 24 to + // 30 when `bridge` landed and again here, because `mcp` now has twenty-odd + // verbs and listing them IS the usage block doing its job. The number keeps + // drifting up because it was carrying the whole claim on its own, so the real + // guard is the line below: the wall's closing line, which no command's own + // usage ever prints. + assert.ok(stderr.split("\n").length <= 45, "usage block should be the command's own, and short"); + assert.doesNotMatch(stderr, /^engines are installed and driven by moshcode/m, "that is the wall"); }); test("an unknown help topic suggests rather than dumping", async () => { diff --git a/test/mcp-catalog.test.mjs b/test/mcp-catalog.test.mjs index af8738ca..5dee9a07 100644 --- a/test/mcp-catalog.test.mjs +++ b/test/mcp-catalog.test.mjs @@ -73,10 +73,16 @@ test("credentials are never written into the spec", () => { test("`mcp catalog` is its own verb, and unknown verbs mention it", () => { assert.deepEqual(parseMcp(["catalog"]), { showCatalog: true }); - // The tail of the list, so adding a verb before it does not fail this. The - // claim is that an unknown verb names the real ones, not that the roster is - // frozen. - assert.match(parseMcp(["bogus"]).error, /install, add, bridge, catalog, or list/); + // Named one at a time rather than as a substring of the whole list. Pinning + // the tail was already the second attempt at surviving a growing roster and + // it only moved the problem: `catalog` and `list` are no longer last, so the + // pattern broke again the moment ten verbs landed after them. What this test + // actually claims is that an unknown verb names the real ones. + const { error } = parseMcp(["bogus"]); + assert.match(error, /unknown mcp verb/); + for (const verb of ["install", "add", "bridge", "catalog", "list"]) { + assert.ok(error.includes(verb), `expected the error to offer ${verb}`); + } }); test("the catalog listing names every entry", () => { diff --git a/test/mcp-stray-flag.test.mjs b/test/mcp-stray-flag.test.mjs index 3639c916..83b9b719 100644 --- a/test/mcp-stray-flag.test.mjs +++ b/test/mcp-stray-flag.test.mjs @@ -20,9 +20,22 @@ test("an engine-native scope flag is rejected, not turned into the server name", }); test("a long unsupported flag is rejected too", () => { - const { error, spec } = parseMcp(["add", "--scope", "user", "https://mcp.example.com/mcp"]); + const { error, spec } = parseMcp(["add", "--namespace", "user", "https://mcp.example.com/mcp"]); assert.equal(spec, undefined); - assert.match(error, /unknown mcp flag "--scope"/); + assert.match(error, /unknown mcp flag "--namespace"/); +}); + +// `--scope` used to be one of these, and now it is worse than unknown: it is a +// flag that exists on a SIBLING verb meaning something else entirely. `/mcp +// answer --scope sessions:write` grants a shared session write access, so +// someone reaching for a config scope here is one flag away from a permission +// they did not intend. It gets its own message rather than the generic one. +test("--scope is refused by name, and points at the two flags that exist", () => { + const { error, spec } = parseMcp(["add", "--scope", "project", "https://mcp.example.com/mcp"]); + assert.equal(spec, undefined); + assert.match(error, /--engine-scope/); + assert.match(error, /--engines/); + assert.match(error, /OAuth permissions/); }); test("a misspelled supported flag is rejected rather than silently accepted", () => { @@ -37,8 +50,8 @@ test("a stray flag in the command position is rejected", () => { }); test("install reports the stray flag, not a misleading missing-name error", () => { - const { error } = parseMcp(["install", "--scope", "user", "https://mcp.example.com/mcp"]); - assert.match(error, /unknown mcp flag "--scope"/); + const { error } = parseMcp(["install", "--namespace", "user", "https://mcp.example.com/mcp"]); + assert.match(error, /unknown mcp flag "--namespace"/); assert.doesNotMatch(error, /explicit --name/); }); @@ -58,8 +71,8 @@ test("no stray flag ever reaches an engine's native argv", () => { }); test("a stray flag cannot smuggle a bogus name past the catalog lookup", () => { - const { error } = parseMcp(["add", "--scope", "porkbun"]); - assert.match(error, /unknown mcp flag "--scope"/); + const { error } = parseMcp(["add", "--namespace", "porkbun"]); + assert.match(error, /unknown mcp flag "--namespace"/); }); // ---------- controls: the opposite direction ---------- diff --git a/test/mcp-surface.test.mjs b/test/mcp-surface.test.mjs new file mode 100644 index 00000000..3dd30d10 --- /dev/null +++ b/test/mcp-surface.test.mjs @@ -0,0 +1,562 @@ +// The verbs beyond `install`/`add`: remove, enable, disable, test, reauth, +// unauth, reconnect, resources, prompts, notifications, catalog search. +// +// Two standards carry over from test/mcp-add-fanout.test.mjs and are asserted +// here for every new fan-out verb rather than only for `add`: the plan must +// cover EVERY engine, and an engine that cannot do the thing must say why. A +// verb that quietly drops four engines looks like it worked everywhere. +// +// The probe verbs are the other half. They delegate to mcpjam rather than +// speaking MCP, so what is testable is the argv they build and the message they +// print when mcpjam is not installed — both without a network or a subprocess. +import assert from "node:assert/strict"; +import { mkdtempSync, readFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import test from "node:test"; + +import { ENGINES } from "../src/engines.mjs"; +import { + MCP_ENGINES, MCP_SCOPES, mcpAddArgs, mcpAuthArgs, mcpReconnectArgs, mcpRemoveArgs, + mcpUnauthArgs, notRegistered, planMcpVerb, resolveMcpEngines, runMcpVerb, +} from "../src/mcp.mjs"; +import { catalogSearchArgs, mcpjamArgs, mcpjamTargetArgs, missingProbeTool } from "../src/mcp-probe.mjs"; +import { + forgetServer, getServer, listServers, recordServer, redactSpec, registryFile, setServerEnabled, +} from "../src/mcp-registry.mjs"; +import { + parseMcp, resolveServerSpec, runAddWizard, runManage, runProbe, +} from "../src/integrations.mjs"; + +const byKey = (items) => Object.fromEntries(items.map((i) => [i.key, i])); +const NO_MCP = Object.keys(ENGINES).filter((key) => !MCP_ENGINES.includes(key)); + +/** Point the registry at a throwaway file. The real one belongs to the operator. */ +function isolateRegistry() { + process.env.MOSHCODE_MCP_FILE = path.join(mkdtempSync(path.join(tmpdir(), "moshcode-mcp-reg-")), "mcp.json"); +} +isolateRegistry(); + +/* ------------------------------------------------- scope: the two axes */ + +test("--engine-scope reaches the engines that have a scope flag", () => { + const spec = { name: "s", target: "https://x.dev/mcp", env: [], headers: [], scope: "project" }; + for (const key of ["claude", "gemini", "qwen"]) { + assert.deepEqual(mcpAddArgs(key, spec).argv.slice(0, 4), ["mcp", "add", "-s", "project"]); + } +}); + +test("an engine with one config location is skipped for project scope, with a reason", () => { + const spec = { name: "s", target: "https://x.dev/mcp", env: [], headers: [], scope: "project" }; + for (const key of ["codex", "opencode", "privacycode"]) { + const { skip, argv } = mcpAddArgs(key, spec); + assert.equal(argv, undefined, `${key} must not be handed a scope it cannot express`); + assert.match(skip, /scope/i, `${key} should say why`); + } +}); + +test("user scope is still the default, byte for byte", () => { + const spec = { name: "s", target: "https://x.dev/mcp", env: [], headers: [] }; + assert.deepEqual(mcpAddArgs("claude", spec).argv.slice(0, 4), ["mcp", "add", "-s", "user"]); + assert.deepEqual(mcpAddArgs("codex", spec).argv, ["mcp", "add", "s", "--url", "https://x.dev/mcp"]); +}); + +test("--engines narrows the fan-out to exactly what was asked for", () => { + const spec = { name: "s", target: "https://x.dev/mcp", env: [], headers: [] }; + const keys = planMcpVerb("add", spec, { installedSet: new Set(), engines: ["claude", "codex"] }) + .map((p) => p.key); + assert.deepEqual(keys, ["claude", "codex"]); +}); + +test("resolveMcpEngines rejects a name that is not an engine", () => { + assert.deepEqual(resolveMcpEngines("claude,codex").engines, ["claude", "codex"]); + assert.equal(resolveMcpEngines(null).engines, null); + assert.match(resolveMcpEngines("clawed").error, /unknown engine "clawed"/); + assert.match(resolveMcpEngines(",,").error, /at least one engine/); + // A plain object literal: these are truthy keys and are not engines. + assert.match(resolveMcpEngines("constructor").error, /unknown engine/); +}); + +/* ------------------------------ every new verb covers every engine (R6) */ + +for (const [verb, build] of [ + ["remove", mcpRemoveArgs], + ["reauth", mcpAuthArgs], + ["unauth", mcpUnauthArgs], + ["reconnect", mcpReconnectArgs], +]) { + test(`${verb} plans every engine, never a subset`, () => { + const keys = planMcpVerb(verb, { name: "s" }, { installedSet: new Set() }).map((p) => p.key); + assert.deepEqual([...keys].sort(), Object.keys(ENGINES).sort()); + }); + + test(`every engine ${verb} cannot do carries a stated reason`, () => { + for (const key of Object.keys(ENGINES)) { + const { argv, skip } = build(key, { name: "s" }); + assert.ok(argv || skip, `${key} returned neither an argv nor a reason for ${verb}`); + if (!argv) assert.ok(skip.length > 4, `${key}'s ${verb} reason is not a sentence`); + else assert.equal(skip, undefined, "a planned engine must not also carry a skip"); + } + }); + + test(`${verb} skips every engine with no MCP support at all`, () => { + for (const key of NO_MCP) assert.ok(build(key, { name: "s" }).skip, `${key} should be skipped`); + }); +} + +test("remove speaks each engine's own dialect", () => { + assert.deepEqual(mcpRemoveArgs("claude", { name: "s" }).argv, ["mcp", "remove", "s"]); + assert.deepEqual(mcpRemoveArgs("claude", { name: "s", scope: "project" }).argv, ["mcp", "remove", "-s", "project", "s"]); + assert.deepEqual(mcpRemoveArgs("qwen", { name: "s" }).argv, ["mcp", "remove", "s"]); + assert.deepEqual(mcpRemoveArgs("codex", { name: "s" }).argv, ["mcp", "remove", "s"]); + // OpenCode's `mcp` has add, list, auth, logout and debug, and nothing that + // deletes — so the reason names the file rather than shrugging. + assert.match(mcpRemoveArgs("opencode", { name: "s" }).skip, /no `mcp remove`/); +}); + +test("reauth and unauth use each engine's own word for it", () => { + assert.deepEqual(mcpAuthArgs("claude", { name: "s" }).argv, ["mcp", "login", "s"]); + assert.deepEqual(mcpAuthArgs("codex", { name: "s" }).argv, ["mcp", "login", "s"]); + assert.deepEqual(mcpAuthArgs("opencode", { name: "s" }).argv, ["mcp", "auth", "s"]); + assert.deepEqual(mcpUnauthArgs("opencode", { name: "s" }).argv, ["mcp", "logout", "s"]); + // The Gemini family authorizes from inside the session, so the skip has to + // tell the reader where to go rather than only that it did not happen. + assert.match(mcpAuthArgs("gemini", { name: "s" }).skip, /\/mcp auth s/); +}); + +test("reconnect is the Gemini family only, and --all is its own argv", () => { + assert.deepEqual(mcpReconnectArgs("qwen", { name: "s" }).argv, ["mcp", "reconnect", "s"]); + assert.deepEqual(mcpReconnectArgs("gemini", { all: true }).argv, ["mcp", "reconnect", "--all"]); + for (const key of ["claude", "codex", "opencode"]) { + assert.ok(mcpReconnectArgs(key, { name: "s" }).skip, `${key} has no mcp reconnect`); + } +}); + +test("omp is skipped for a stated reason, not the blanket no-MCP-support line", () => { + // omp supports MCP better than most engines here. What it has no scriptable + // command for is the part moshcode drives, and the row must say that rather + // than send somebody looking for an engine they already have. + for (const build of [mcpAddArgs, mcpRemoveArgs, mcpAuthArgs, mcpUnauthArgs, mcpReconnectArgs]) { + const { skip } = build("omp", { name: "s", target: "https://x.dev/mcp", env: [], headers: [] }); + assert.ok(skip, "omp cannot be planned"); + assert.notEqual(skip, "no MCP support"); + assert.match(skip, /omp|TUI/); + } +}); + +/* ------------------------------------------------------- the fan-out runner */ + +test("a remove that finds nothing is grey, not a failure", async () => { + const plan = planMcpVerb("remove", { name: "s" }, { installedSet: new Set(["claude", "codex"]) }); + const results = byKey(await runMcpVerb(plan, { + done: "removed", + missing: true, + run: async (bin) => (bin === "claude" + ? { ok: true, code: 0 } + : { ok: false, code: 1, output: "No such server: s" }), + })); + assert.equal(results.claude.status, "removed"); + assert.equal(results.codex.status, "missing"); +}); + +test("a genuine failure is still a failure", async () => { + const plan = planMcpVerb("remove", { name: "s" }, { installedSet: new Set(["claude"]) }); + const results = byKey(await runMcpVerb(plan, { + done: "removed", missing: true, run: async () => ({ ok: false, code: 1, output: "permission denied" }), + })); + assert.equal(results.claude.status, "failed"); +}); + +test("notRegistered reads the engine's own words, and nothing else", () => { + assert.equal(notRegistered({ output: "Error: no such server \"x\"" }), true); + assert.equal(notRegistered({ output: "x is not configured" }), true); + assert.equal(notRegistered({ output: "EACCES: permission denied" }), false); + assert.equal(notRegistered({}), false); +}); + +test("reauth inherits the terminal rather than capturing it", async () => { + // Captured stdio swallows an OAuth prompt, and the flow's whole job is to + // talk to the person sitting there. + const seen = []; + const plan = planMcpVerb("reauth", { name: "s" }, { installedSet: new Set(["claude"]) }); + await runMcpVerb(plan, { + done: "authorized", capture: false, + run: async (bin, argv, opts) => { seen.push(opts); return { ok: true, code: 0 }; }, + }); + assert.deepEqual(seen, [{ capture: false }]); +}); + +test("planMcpVerb refuses a verb it has no builder for", () => { + assert.throws(() => planMcpVerb("nonsense", { name: "s" }), /no MCP fan-out builder/); +}); + +/* --------------------------------------------------------------- parsing */ + +test("--scope is refused on the spec verbs and on the name verbs alike", () => { + for (const tokens of [["add", "--scope", "project", "https://x.dev/mcp"], ["remove", "s", "--scope", "project"]]) { + const { error } = parseMcp(tokens); + assert.match(error, /OAuth permissions/); + assert.match(error, /--engine-scope/); + } +}); + +test("--engine-scope only takes the scopes every engine could mean", () => { + assert.equal(parseMcp(["add", "s", "https://x.dev/mcp", "--engine-scope", "project"]).engineScope, "project"); + assert.match(parseMcp(["add", "s", "https://x.dev/mcp", "--engine-scope", "local"]).error, /must be user or project/); + assert.deepEqual(MCP_SCOPES, ["user", "project"]); +}); + +test("--engines rides beside the spec rather than inside it", () => { + const parsed = parseMcp(["add", "s", "https://x.dev/mcp", "--engines", "claude,codex"]); + assert.deepEqual(parsed.engines, ["claude", "codex"]); + // The spec is what gets spliced into every engine's argv and recorded; a + // fan-out policy field in there would be registered along with the server. + assert.deepEqual(Object.keys(parsed.spec).sort(), ["args", "env", "headers", "name", "target", "transport"]); +}); + +test("--url is the explicit spelling of a remote target", () => { + const { spec } = parseMcp(["add", "sentry", "--url", "https://mcp.sentry.dev/mcp"]); + assert.equal(spec.target, "https://mcp.sentry.dev/mcp"); + assert.deepEqual(spec.args, []); +}); + +test("--url and a `--` command together are refused, not silently resolved", () => { + const { error } = parseMcp(["add", "s", "--url", "https://x.dev/mcp", "--", "npx", "srv"]); + assert.match(error, /pick one/); +}); + +test("a transport nobody implements is rejected at the door", () => { + assert.match(parseMcp(["add", "s", "https://x.dev/mcp", "-t", "carrier-pigeon"]).error, /http, sse, or stdio/); +}); + +test("--token env:VAR never puts the literal on the command line", () => { + process.env.MOSHCODE_TEST_MCP_TOKEN = "sekret"; + const parsed = parseMcp(["add", "s", "https://x.dev/mcp", "--token", "env:MOSHCODE_TEST_MCP_TOKEN"]); + assert.deepEqual(parsed.spec.headers, ["Authorization: Bearer sekret"]); + assert.deepEqual(parsed.auth, { header: "Authorization", from: "env:MOSHCODE_TEST_MCP_TOKEN" }); + delete process.env.MOSHCODE_TEST_MCP_TOKEN; +}); + +test("--token env:VAR fails loudly when the variable is empty", () => { + const { error } = parseMcp(["add", "s", "https://x.dev/mcp", "--token", "env:MOSHCODE_TEST_MISSING"]); + assert.match(error, /MOSHCODE_TEST_MISSING is not set/); +}); + +test("a literal --token still works, and is marked as the form it is", () => { + const parsed = parseMcp(["add", "s", "https://x.dev/mcp", "--token", "abc123"]); + assert.deepEqual(parsed.spec.headers, ["Authorization: Bearer abc123"]); + assert.equal(parsed.auth.from, "inline"); +}); + +test("the probe verbs take one server and reject a second", () => { + assert.deepEqual(parseMcp(["test", "sentry"]).probe.name, "sentry"); + assert.match(parseMcp(["test"]).error, /needs a server name or URL/); + assert.match(parseMcp(["test", "a", "b"]).error, /takes one server/); +}); + +test("--for without --listen is an error rather than a silent wait", () => { + assert.match(parseMcp(["notifications", "s", "--for", "5000"]).error, /only applies with --listen/); + const { probe } = parseMcp(["notifications", "s", "--listen", "--for", "5000"]); + assert.equal(probe.durationMs, 5000); +}); + +test("reconnect --all is the one name-less manage verb", () => { + assert.equal(parseMcp(["reconnect", "--all"]).manage.all, true); + assert.match(parseMcp(["remove", "--all"]).error, /only applies to mcp reconnect/); + assert.match(parseMcp(["remove"]).error, /needs a server name/); +}); + +test("catalog search bounds its own limit", () => { + assert.equal(parseMcp(["catalog", "search", "postgres"]).catalogSearch.query, "postgres"); + assert.equal(parseMcp(["catalog", "search", "postgres", "--limit", "5"]).catalogSearch.limit, 5); + assert.match(parseMcp(["catalog", "search", "x", "--limit", "0"]).error, /1 to 100/); + assert.match(parseMcp(["catalog", "search", "x", "--limit", "101"]).error, /1 to 100/); + assert.match(parseMcp(["catalog", "search"]).error, /needs a keyword/); + assert.match(parseMcp(["catalog", "nonsense"]).error, /try search/); +}); + +test("`mcp add` with nothing after it asks for the wizard, not for a name", () => { + assert.deepEqual(parseMcp(["add"]), { wizard: true }); +}); + +test("`mcp list --servers` narrows without changing the old shape", () => { + assert.deepEqual(parseMcp(["list"]), { list: true, json: false }); + assert.deepEqual(parseMcp(["list", "--json"]), { list: true, json: true }); + assert.deepEqual(parseMcp(["list", "--servers"]), { list: true, json: false, servers: true }); +}); + +/* ---------------------------------------------------------- the mcpjam argv */ + +test("a remote spec becomes mcpjam's http flags", () => { + const argv = mcpjamTargetArgs({ target: "https://x.dev/mcp", headers: ["Authorization: Bearer z"], env: [] }); + assert.deepEqual(argv, ["--transport", "http", "--url", "https://x.dev/mcp", "--header", "Authorization: Bearer z"]); +}); + +test("a stdio spec becomes mcpjam's command flags", () => { + const argv = mcpjamTargetArgs({ target: "npx", args: ["-y", "srv"], env: [["K", "v"]] }); + assert.deepEqual(argv, ["--transport", "stdio", "--command", "npx", "--args", "-y", "srv", "-e", "K=v"]); +}); + +test("each probe verb maps to the mcpjam subcommand that answers it", () => { + const spec = { target: "https://x.dev/mcp", headers: [], env: [] }; + assert.deepEqual(mcpjamArgs("test", spec).slice(0, 2), ["server", "info"]); + assert.deepEqual(mcpjamArgs("resources", spec).slice(0, 2), ["resources", "list"]); + assert.deepEqual(mcpjamArgs("prompts", spec).slice(0, 2), ["prompts", "list"]); + // The default reads what the server declares and returns; --listen streams. + assert.deepEqual(mcpjamArgs("notifications", spec).slice(0, 2), ["server", "capabilities"]); + assert.deepEqual(mcpjamArgs("notifications", spec, { listen: true }).slice(0, 3), + ["subscriptions", "listen", "--list-changed"]); + assert.ok(mcpjamArgs("notifications", spec, { listen: true, durationMs: 50 }).includes("--duration-ms")); +}); + +test("--format json is a program-level flag and leads the argv", () => { + const argv = mcpjamArgs("test", { target: "https://x.dev/mcp", headers: [], env: [] }, { json: true }); + assert.deepEqual(argv.slice(0, 2), ["--format", "json"]); +}); + +test("catalog search builds a registry search, with no moshcode-side API key", () => { + const argv = catalogSearchArgs("postgres", { limit: 5 }); + assert.deepEqual(argv, ["registry", "search", "postgres", "--limit", "5"]); + assert.ok(!argv.includes("--api-key"), "the key is mcpjam's to hold, not moshcode's to copy"); +}); + +test("mcpjamArgs refuses a verb it has no probe for", () => { + assert.throws(() => mcpjamArgs("nonsense", { target: "x" }), /no mcpjam probe/); +}); + +test("the missing-tool message names the command that fixes it", () => { + const lines = missingProbeTool("test").join(" "); + assert.match(lines, /moshcode install mcpjam/); + assert.match(lines, /not installed/); +}); + +test("a probe with mcpjam missing explains itself and never spawns anything", async () => { + const spawned = []; + const code = await runProbe({ verb: "test", name: "https://x.dev/mcp" }, { + probeInstalled: () => false, + run: async (...args) => { spawned.push(args); return { ok: true, code: 0 }; }, + }); + assert.equal(code, 1); + assert.deepEqual(spawned, [], "nothing to run when the runner is not there"); +}); + +test("a probe against an unknown name fails before it reaches mcpjam", async () => { + const spawned = []; + const code = await runProbe({ verb: "test", name: "not-a-server" }, { + probeInstalled: () => true, + run: async (...args) => { spawned.push(args); return { ok: true, code: 0 }; }, + }); + assert.equal(code, 1); + assert.deepEqual(spawned, []); +}); + +test("a probe takes a bare URL, because that is when people ask", async () => { + const spawned = []; + const code = await runProbe({ verb: "test", name: "https://mcp.sentry.dev/mcp" }, { + probeInstalled: () => true, + run: async (bin, argv) => { spawned.push([bin, argv]); return { ok: true, code: 0 }; }, + }); + assert.equal(code, 0); + assert.equal(spawned.length, 1); + assert.equal(spawned[0][0], "mcpjam"); + assert.ok(spawned[0][1].includes("https://mcp.sentry.dev/mcp")); +}); + +/* -------------------------------------------------------------- the wizard */ + +test("the wizard refuses to run where nothing can answer it", async () => { + // `mcp add` in a script would otherwise block forever on a read. + const calls = []; + const code = await runAddWizard({ + isTty: () => false, + ask: async () => { calls.push("asked"); return "x"; }, + run: async () => ({ ok: true, code: 0 }), + }); + assert.equal(code, 1); + assert.deepEqual(calls, [], "it must not prompt into the void"); +}); + +test("the wizard's answers become the same spec the flags would have built", async () => { + isolateRegistry(); + const answers = ["wizard", "https://wiz.example.com/mcp", "http", "", "user", "claude"]; + const calls = []; + const code = await runAddWizard({ + isTty: () => true, + ask: async (_q, fallback = "") => (answers.shift() ?? fallback) || fallback, + run: async (bin, argv) => { calls.push([bin, ...argv]); return { ok: true, code: 0 }; }, + installedSet: new Set(["claude"]), + }); + assert.equal(code, 0); + assert.deepEqual(calls, [ + ["claude", "mcp", "add", "-s", "user", "-t", "http", "wizard", "https://wiz.example.com/mcp"], + ]); + assert.equal(getServer("wizard").target, "https://wiz.example.com/mcp"); +}); + +/* -------------------------------------------------------------- the record */ + +test("redactSpec keeps header names and drops every value", () => { + const redacted = redactSpec({ + target: "https://x.dev/mcp", + args: [], + env: [["PORKBUN_API_KEY", "pk-live-secret"]], + headers: ["Authorization: Bearer sk-live-secret"], + auth: { header: "Authorization", from: "env:TOKEN" }, + }); + const serialized = JSON.stringify(redacted); + assert.deepEqual(redacted.headers, ["Authorization"]); + assert.deepEqual(redacted.env, ["PORKBUN_API_KEY"]); + assert.ok(!serialized.includes("sk-live-secret"), "a bearer token must never reach the file"); + assert.ok(!serialized.includes("pk-live-secret"), "an API key value must never reach the file"); +}); + +test("a recorded server round-trips, and the file on disk holds no secret", () => { + isolateRegistry(); + recordServer("sentry", { + name: "sentry", target: "https://mcp.sentry.dev/mcp", args: [], env: [], transport: "http", + headers: ["Authorization: Bearer sk-live-nope"], + auth: { header: "Authorization", from: "env:SENTRY_TOKEN" }, + }, { engineScope: "user", engines: ["claude"] }); + + const back = getServer("sentry"); + assert.equal(back.target, "https://mcp.sentry.dev/mcp"); + assert.deepEqual(back.engines, ["claude"]); + assert.equal(back.enabled, true); + assert.ok(!readFileSync(registryFile(), "utf8").includes("sk-live-nope")); +}); + +test("getServer does not resolve off Object.prototype", () => { + isolateRegistry(); + recordServer("real", { name: "real", target: "npx", args: [], env: [], headers: [] }); + for (const bogus of ["constructor", "__proto__", "toString", ""]) { + assert.equal(getServer(bogus), null, `${bogus} is not a server`); + } +}); + +test("disable keeps the spec so enable has something to restore", () => { + isolateRegistry(); + recordServer("keepme", { name: "keepme", target: "npx", args: ["-y", "srv"], env: [], headers: [] }); + setServerEnabled("keepme", false); + const off = getServer("keepme"); + assert.equal(off.enabled, false); + assert.deepEqual(off.args, ["-y", "srv"], "a disabled server that forgot its spec is a one-way door"); + setServerEnabled("keepme", true); + assert.equal(getServer("keepme").enabled, true); + assert.equal(setServerEnabled("nope", false), null); +}); + +test("forgetServer is idempotent and listing is name-sorted", () => { + isolateRegistry(); + recordServer("b", { name: "b", target: "npx", args: [], env: [], headers: [] }); + recordServer("a", { name: "a", target: "npx", args: [], env: [], headers: [] }); + assert.deepEqual(listServers().map((s) => s.name), ["a", "b"]); + assert.equal(forgetServer("a"), true); + assert.equal(forgetServer("a"), false); + assert.deepEqual(listServers().map((s) => s.name), ["b"]); +}); + +test("an unreadable or hand-mangled record reads as nothing registered", () => { + process.env.MOSHCODE_MCP_FILE = path.join(mkdtempSync(path.join(tmpdir(), "moshcode-mcp-gone-")), "absent.json"); + assert.deepEqual(listServers(), []); +}); + +/* ----------------------------------------------------------- resolution */ + +test("a registered server resolves by name, and says when its credential is gone", () => { + isolateRegistry(); + recordServer("withauth", { + name: "withauth", target: "https://x.dev/mcp", args: [], env: [], headers: ["Authorization: Bearer x"], + auth: { header: "Authorization", from: "env:MOSHCODE_TEST_ABSENT" }, + }); + const resolved = resolveServerSpec("withauth"); + assert.equal(resolved.source, "registry"); + assert.deepEqual(resolved.spec.headers, [], "a value that was never stored cannot be rebuilt"); + assert.equal(resolved.unauthenticated, true); +}); + +test("a registered server rebuilds its header when the variable is still set", () => { + isolateRegistry(); + process.env.MOSHCODE_TEST_PRESENT = "zzz"; + recordServer("live", { + name: "live", target: "https://x.dev/mcp", args: [], env: [], headers: ["Authorization: Bearer zzz"], + auth: { header: "Authorization", from: "env:MOSHCODE_TEST_PRESENT" }, + }); + const resolved = resolveServerSpec("live"); + assert.deepEqual(resolved.spec.headers, ["Authorization: Bearer zzz"]); + assert.equal(resolved.unauthenticated, false); + delete process.env.MOSHCODE_TEST_PRESENT; +}); + +test("resolution falls through the record, the catalog, then a bare URL", () => { + isolateRegistry(); + assert.equal(resolveServerSpec("porkbun").source, "catalog"); + assert.equal(resolveServerSpec("https://x.dev/mcp").source, "url"); + assert.equal(resolveServerSpec("nothing-like-this"), null); + assert.equal(resolveServerSpec(""), null); +}); + +/* --------------------------------------------------- disable is a round trip */ + +test("disable deregisters everywhere and enable puts the same spec back", async () => { + isolateRegistry(); + recordServer("roundtrip", { name: "roundtrip", target: "npx", args: ["-y", "srv"], env: [], headers: [] }); + + const calls = []; + const run = async (bin, argv) => { calls.push([bin, ...argv]); return { ok: true, code: 0 }; }; + const installedSet = new Set(["claude"]); + + assert.equal(await runManage({ verb: "disable", name: "roundtrip" }, { run, installedSet }), 0); + // The scope comes back off the record, so the removal targets the exact place + // the registration wrote rather than "wherever claude happens to find it". + assert.deepEqual(calls.at(-1), ["claude", "mcp", "remove", "-s", "user", "roundtrip"]); + assert.equal(getServer("roundtrip").enabled, false, "the spec must survive the disable"); + + assert.equal(await runManage({ verb: "enable", name: "roundtrip" }, { run, installedSet }), 0); + assert.deepEqual(calls.at(-1), ["claude", "mcp", "add", "-s", "user", "roundtrip", "--", "npx", "-y", "srv"]); + assert.equal(getServer("roundtrip").enabled, true); +}); + +test("enable refuses a server moshcode never registered", async () => { + isolateRegistry(); + const calls = []; + const code = await runManage({ verb: "enable", name: "ghost" }, { + run: async (...a) => { calls.push(a); return { ok: true, code: 0 }; }, + installedSet: new Set(["claude"]), + }); + assert.equal(code, 1); + assert.deepEqual(calls, [], "there is no spec to register"); +}); + +test("acting on a server again reaches only the engines it was registered into", async () => { + // `mcp add x --engines claude` then `mcp remove x` used to fan the removal + // out to all six: five that never had it, and two belonging to somebody who + // never asked moshcode to touch them. + isolateRegistry(); + recordServer("narrow", { name: "narrow", target: "npx", args: [], env: [], headers: [] }, + { engineScope: "project", engines: ["claude"] }); + const calls = []; + const run = async (bin, argv) => { calls.push([bin, ...argv]); return { ok: true, code: 0 }; }; + await runManage({ verb: "remove", name: "narrow" }, { run, installedSet: new Set(Object.keys(ENGINES)) }); + assert.deepEqual(calls, [["claude", "mcp", "remove", "-s", "project", "narrow"]]); +}); + +test("an explicit --engines still beats what was recorded", async () => { + isolateRegistry(); + recordServer("narrow2", { name: "narrow2", target: "npx", args: [], env: [], headers: [] }, + { engineScope: "user", engines: ["claude"] }); + const calls = []; + const run = async (bin, argv) => { calls.push([bin, ...argv]); return { ok: true, code: 0 }; }; + await runManage({ verb: "remove", name: "narrow2", engines: ["codex"] }, { + run, installedSet: new Set(Object.keys(ENGINES)), + }); + assert.deepEqual(calls, [["codex", "mcp", "remove", "narrow2"]]); +}); + +test("remove drops the record too, so the listing cannot lie", async () => { + isolateRegistry(); + recordServer("gone", { name: "gone", target: "npx", args: [], env: [], headers: [] }); + await runManage({ verb: "remove", name: "gone" }, { + run: async () => ({ ok: true, code: 0 }), installedSet: new Set(["claude"]), + }); + assert.equal(getServer("gone"), null); +}); diff --git a/test/mcp.test.mjs b/test/mcp.test.mjs index 6d725e98..eaf53d04 100644 --- a/test/mcp.test.mjs +++ b/test/mcp.test.mjs @@ -139,7 +139,17 @@ function run(args, { binDir, env = {} }) { return new Promise((resolve, reject) => { const child = spawn(process.execPath, [BIN, ...args], { stdio: ["ignore", "ignore", "ignore"], - env: { ...process.env, ...env, PATH: `${binDir}${path.delimiter}${process.env.PATH || ""}` }, + env: { + ...process.env, + // A successful fan-out records the server in moshcode's own list. Left + // to the default that is ~/.moshcode/mcp.json, the operator's real + // one, so the suite would file a fictional Sentry server on the box + // that ran it. Same reason the MOSHCODE_ENGINE_BIN_ guard at the top of + // this file exists. + MOSHCODE_MCP_FILE: path.join(tempDir("moshcode-mcp-registry-"), "mcp.json"), + ...env, + PATH: `${binDir}${path.delimiter}${process.env.PATH || ""}`, + }, }); child.on("error", reject); child.on("exit", (code) => resolve({ code }));