From be15fe3c178e75235345f8baead6f7337c2935d5 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Fri, 30 Oct 2026 14:15:25 +0100 Subject: [PATCH] fix(coding-agent): accept -l in pi mcp login and logout Report unknown options as typed and suggest a configured server for a mistyped name. --- packages/coding-agent/CHANGELOG.md | 1 + packages/coding-agent/docs/cli.md | 4 +- .../coding-agent/src/extensions/mcp/cli.ts | 40 +++++++++++++++---- .../coding-agent/test/mcp-command.test.ts | 25 ++++++++++++ 4 files changed, 60 insertions(+), 10 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 17a9683fd..9337ac979 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -38,6 +38,7 @@ - Fixed `pi -e npm:` staying on the version from the first download. On each start and `/reload`, pi now reinstalls an `-e` npm package without an exact version when the registry has a newer matching version. `pi -e git:@` likewise stayed on its first checkout and is now fetched and reset to the branch; a commit-hash ref that is already checked out is not fetched. Offline mode and failed checks keep the cached copy ([#11375](https://github.com/earendil-works/pi/issues/11375), [#11590](https://github.com/earendil-works/pi/pull/11590)) - Fixed Windows paths being compared case-sensitively. When the working directory and home directory differed only in drive-letter casing (`c:\Users\me` vs `C:\Users\me`), skills in `~/.agents/skills` loaded twice, as project and user skills, and were reported as name collisions. The same mismatch also re-asked for saved project trust, hid sessions in a custom session directory from `--continue` and `/resume`, and stopped `pi remove` from matching configured local packages ([#10488](https://github.com/earendil-works/pi/issues/10488)) - Fixed paths next to the home directory being shown with a broken `~` prefix, for example `/home/me2/project` shown as `~2/project` when home is `/home/me` +- Fixed `pi mcp login -l` and `pi mcp logout -l` failing with `Unknown option --local.` although `pi mcp add` and `pi mcp remove` accept `-l`. With `-l`, they now require the server to be a project server. Unknown options are now reported as typed, and a mistyped server name gets a suggestion, for example `Did you mean "gurker"?` ## [1.4.0] - 2026-10-21 diff --git a/packages/coding-agent/docs/cli.md b/packages/coding-agent/docs/cli.md index 376a83ab2..bb40e27a8 100644 --- a/packages/coding-agent/docs/cli.md +++ b/packages/coding-agent/docs/cli.md @@ -325,8 +325,8 @@ These commands work outside a session, so agents can run them through `bash`. Se | `pi mcp add [options] --url ` | Add or replace a streamable HTTP server; `--header KEY=VALUE` (repeatable), `--bearer-token-env-var ` (sends `Authorization: Bearer ${NAME}`), `--oauth-client-id`, `--oauth-client-secret`, `--oauth-callback-port`, `--oauth-client-name`, and `--oauth-client-registration` (`auto`, `cimd`, or `dcr`) configure authentication | | `pi mcp remove ` | Remove a server from `mcp.json`; stored OAuth credentials are kept | | `pi mcp list [--json]` | Connect to every enabled server and print its state, tools, and errors; exit with `1` when a config entry is invalid or an enabled server is not connected | -| `pi mcp login [--timeout ]` | Sign in to an OAuth server: open the authorization page and wait for the browser; a terminal also accepts the pasted redirect URL. `--timeout` limits the whole sign-in, including requests to the authorization server (default 300 seconds) | -| `pi mcp logout ` | Delete the stored OAuth credentials of a server | +| `pi mcp login [-l] [--timeout ]` | Sign in to an OAuth server: open the authorization page and wait for the browser; a terminal also accepts the pasted redirect URL. `--timeout` limits the whole sign-in, including requests to the authorization server (default 300 seconds). `-l` requires the server to be defined in the project's `.pi/mcp.json` | +| `pi mcp logout [-l]` | Delete the stored OAuth credentials of a server | `add` and `remove` change `~/.pi/agent/mcp.json`, or `.pi/mcp.json` in the current directory with `--local` (`-l`). `add` also takes `--exposure ` (see [Exposure](mcp.md#control-tool-exposure)) and `--description ` and does not connect; run `pi mcp list` to check the server. diff --git a/packages/coding-agent/src/extensions/mcp/cli.ts b/packages/coding-agent/src/extensions/mcp/cli.ts index 2a55f0b6a..81d28ad66 100644 --- a/packages/coding-agent/src/extensions/mcp/cli.ts +++ b/packages/coding-agent/src/extensions/mcp/cli.ts @@ -35,8 +35,8 @@ const HELP = `${chalk.bold("Usage:")} ${APP_NAME} mcp add [options] --url ${APP_NAME} mcp remove [-l] ${APP_NAME} mcp list [--json] - ${APP_NAME} mcp login [--timeout ] - ${APP_NAME} mcp logout + ${APP_NAME} mcp login [-l] [--timeout ] + ${APP_NAME} mcp logout [-l] Configure and check MCP servers and sign in to OAuth servers without starting a session. Reads ~/${CONFIG_DIR_NAME}/agent/mcp.json and, in trusted projects, ${CONFIG_DIR_NAME}/mcp.json. @@ -48,7 +48,7 @@ Commands: login Sign in through the browser logout Delete the stored OAuth credentials -Options for add and remove: +Options for add, remove, login, and logout: -l, --local Use ${CONFIG_DIR_NAME}/mcp.json in the current project instead of the global file Options for add: @@ -153,7 +153,8 @@ function parseOptions( const values = new Map(); const lists = new Map(); for (let index = 0; index < args.length; index++) { - const arg = OPTION_ALIASES.get(args[index]) ?? args[index]; + const typed = args[index]; + const arg = OPTION_ALIASES.get(typed) ?? typed; if (arg === "--" || positional.length >= maxPositionals) { positional.push(...args.slice(arg === "--" ? index + 1 : index)); break; @@ -165,7 +166,7 @@ function parseOptions( const name = arg.slice(2); const kind = known[name]; if (!kind) { - error(`Unknown option ${arg}.\n${HELP_HINT}`); + error(`Unknown option ${typed}.\n${HELP_HINT}`); return undefined; } if (kind === "flag") { @@ -219,20 +220,30 @@ export async function runMcpCommand(args: string[], options: McpCommandOptions): } case "login": case "logout": { - const parsed = parseOptions(rest, command === "login" ? { timeout: "value" } : {}, error); + const parsed = parseOptions( + rest, + command === "login" ? { local: "flag", timeout: "value" } : { local: "flag" }, + error, + ); if (!parsed) return 1; const [name, ...extra] = parsed.positional; if (!name || extra.length > 0) { - error(`Usage: ${APP_NAME} mcp ${command} \n${HELP_HINT}`); + error(`Usage: ${APP_NAME} mcp ${command} [-l]\n${HELP_HINT}`); return 1; } const entry = loaded.servers.find((server) => server.name === name); if (!entry) { + const names = loaded.servers.map((server) => server.name); + const suggestion = names.find((candidate) => editDistance(candidate, name) <= 2); error( - `No MCP server named "${name}".${untrustedNote ? ` ${untrustedNote}` : ""} Configured: ${loaded.servers.map((server) => server.name).join(", ") || "none"}.`, + `No MCP server named "${name}".${suggestion ? ` Did you mean "${suggestion}"?` : ""}${untrustedNote ? ` ${untrustedNote}` : ""} Configured: ${names.join(", ") || "none"}.`, ); return 1; } + if (parsed.values.has("local") && entry.scope !== "project") { + error(`MCP server "${name}" is defined in ${entry.source}, not in the project; omit --local.`); + return 1; + } const connection = createConnection(entry, options, credentials); const url = connection.oauthUrl; if (!url) { @@ -261,6 +272,19 @@ export async function runMcpCommand(args: string[], options: McpCommandOptions): } } +/** Levenshtein distance, to suggest a configured server name for a mistyped one. */ +function editDistance(a: string, b: string): number { + let previous = Array.from({ length: b.length + 1 }, (_, index) => index); + for (let i = 1; i <= a.length; i++) { + const current = [i]; + for (let j = 1; j <= b.length; j++) { + current[j] = Math.min(previous[j] + 1, current[j - 1] + 1, previous[j - 1] + (a[i - 1] === b[j - 1] ? 0 : 1)); + } + previous = current; + } + return previous[b.length]; +} + /** Parse `KEY=VALUE` pairs of a repeatable option into a record. */ function parsePairs(option: string, pairs: string[] | undefined, error: (line: string) => void) { if (!pairs) return {}; diff --git a/packages/coding-agent/test/mcp-command.test.ts b/packages/coding-agent/test/mcp-command.test.ts index d4cef236b..2e9a0a1fa 100644 --- a/packages/coding-agent/test/mcp-command.test.ts +++ b/packages/coding-agent/test/mcp-command.test.ts @@ -2,6 +2,7 @@ import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "no import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; +import { ProjectTrustStore } from "../src/core/trust-manager.ts"; import { runMcpCommand } from "../src/extensions/mcp/cli.ts"; const FIXTURE = resolve(import.meta.dirname, "../../mcp/test/fixtures/stdio-server.mjs"); @@ -74,6 +75,30 @@ describe("pi mcp", () => { expect((await run(["frobnicate"], servers)).exitCode).toBe(1); }); + it("suggests a configured server for a mistyped name", async () => { + expect((await run(["login", "fixtures"], servers)).output).toBe( + 'No MCP server named "fixtures". Did you mean "fixture"? Configured: fixture, broken, parked.', + ); + }); + + it("reports options as typed", async () => { + expect((await run(["list", "-l"], servers)).output).toContain("Unknown option -l."); + }); + + it("restricts login and logout to project servers with --local", async () => { + const { agentDir } = await run(["add", "-l", "local", "--", "node", "server.js"], servers); + new ProjectTrustStore(agentDir).set(agentDir, true); + + expect(await run(["logout", "-l", "local"], undefined, agentDir)).toMatchObject({ + exitCode: 1, + output: 'MCP server "local" does not use OAuth. Only HTTP servers without an Authorization header do.', + }); + expect(await run(["login", "-l", "fixture"], undefined, agentDir)).toMatchObject({ + exitCode: 1, + output: `MCP server "fixture" is defined in ${join(agentDir, "mcp.json")}, not in the project; omit --local.`, + }); + }); + it("adds stdio servers and passes options after the command through", async () => { const added = await run( ["add", "--env", "A=1", "--env", "B=x=y", "files", "--", "npx", "-y", "server", "--root", "."],