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.
This commit is contained in:
Armin Ronacher 2026-10-30 14:15:25 +01:00
parent 0441483926
commit be15fe3c17
4 changed files with 60 additions and 10 deletions

View file

@ -38,6 +38,7 @@
- Fixed `pi -e npm:<pkg>` 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:<repo>@<branch>` 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

View file

@ -325,8 +325,8 @@ These commands work outside a session, so agents can run them through `bash`. Se
| `pi mcp add <server> [options] --url <url>` | Add or replace a streamable HTTP server; `--header KEY=VALUE` (repeatable), `--bearer-token-env-var <NAME>` (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 <server>` | 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 <server> [--timeout <seconds>]` | 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 <server>` | Delete the stored OAuth credentials of a server |
| `pi mcp login <server> [-l] [--timeout <seconds>]` | 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 <server> [-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 <mode>` (see [Exposure](mcp.md#control-tool-exposure)) and `--description <text>` and does not connect; run `pi mcp list` to check the server.

View file

@ -35,8 +35,8 @@ const HELP = `${chalk.bold("Usage:")}
${APP_NAME} mcp add <server> [options] --url <url>
${APP_NAME} mcp remove <server> [-l]
${APP_NAME} mcp list [--json]
${APP_NAME} mcp login <server> [--timeout <seconds>]
${APP_NAME} mcp logout <server>
${APP_NAME} mcp login <server> [-l] [--timeout <seconds>]
${APP_NAME} mcp logout <server> [-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 <server> Sign in through the browser
logout <server> 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<string, string | true>();
const lists = new Map<string, string[]>();
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} <server>\n${HELP_HINT}`);
error(`Usage: ${APP_NAME} mcp ${command} <server> [-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 {};

View file

@ -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", "."],