diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 5033453f6..f92a1e3b9 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -16,6 +16,7 @@ - Images in steering and follow-up messages, including `prompt()` while streaming, are now resized like images in idle prompts instead of being sent and saved at full size ([#11738](https://github.com/earendil-works/pi/issues/11738)) - A failed token refresh of an MCP connection no longer registers an OAuth client or replaces the PKCE verifier of a sign-in in progress, which made that sign-in fail. MCP sign-in now refreshes the stored tokens with the stored client even when the loopback callback port changed, instead of registering a new client and opening the browser. Ctrl+C in `/mcp` and the `--timeout` of `pi mcp login` now also stop a sign-in while it waits up to 25 seconds for another pi process's token refresh, and a refresh lock that fails for another reason than that wait now fails the sign-in instead of opening the browser. +- Fixed `pi mcp list` reporting MCP servers with `"auth": { "provider": "" }`, such as Radius, as needing sign-in although the provider was signed in: the shell command sent no token. It now sends the provider's `/login` token and refreshes it when it expired. For these servers the sign-in hint names `/login ` instead of `pi mcp login`, and `pi mcp login` and `pi mcp logout` point to `/login` and `/logout` instead of failing with "does not use OAuth" - Fixed RPC mode ignoring an extension's `ctx.shutdown()` while idle, for example from a timer: the process kept running until the client sent another command. pi now shuts down right away when idle, and a shutdown requested by an extension command waits until the command's `prompt` response is written ([#11599](https://github.com/earendil-works/pi/issues/11599)) - Fixed changelog notes repeating on every fresh launch when the installed package version has a prerelease suffix, such as `0.1.5-20261020.7`: the saved version was read as `0.1.0`, so the `0.1.1` to `0.1.5` notes counted as new each time. A saved version without `major.minor.patch` now shows no notes and is replaced by the installed version, instead of showing the whole changelog ([#11703](https://github.com/earendil-works/pi/issues/11703)) - Fixed RPC mode cutting off shutdown cleanup, such as a pending MCP token refresh, when stdin closed and SIGTERM arrived together, which process managers like Paseo and systemd do. Pi lost the refreshed token and the MCP server revoked the sign-in. Once a shutdown has started, a stdin EOF or a single signal no longer interrupts it; a second signal still exits at once. Print mode likewise no longer dies from a signal that arrives during its final cleanup ([#11599](https://github.com/earendil-works/pi/issues/11599)) diff --git a/packages/coding-agent/src/extensions/mcp/cli.ts b/packages/coding-agent/src/extensions/mcp/cli.ts index 81d28ad66..9dba82a84 100644 --- a/packages/coding-agent/src/extensions/mcp/cli.ts +++ b/packages/coding-agent/src/extensions/mcp/cli.ts @@ -11,6 +11,7 @@ import { createInterface } from "node:readline/promises"; import chalk from "chalk"; import { APP_NAME, CONFIG_DIR_NAME } from "../../config.ts"; import { validateMcpServerConfig } from "../../core/mcp-servers.ts"; +import { ModelRuntime } from "../../core/model-runtime.ts"; import { ProjectTrustStore } from "../../core/trust-manager.ts"; import { openBrowser } from "../../utils/open-browser.ts"; import { @@ -83,6 +84,8 @@ export interface McpCommandOptions { agentDir: string; /** Defaults to `mcp-auth.json` in the agent directory. */ credentials?: McpOAuthCredentialStore; + /** Token of a pi provider, for servers with `auth.provider`. Defaults to the agent directory's `auth.json`. */ + providerToken?: (provider: string) => Promise; /** Defaults to the platform browser. */ openUrl?: (url: string) => void; /** Defaults to console output. */ @@ -99,6 +102,8 @@ interface ServerReport { enabled: boolean; exposure: string; transport: string; + /** The pi provider whose token the server receives, from `auth.provider`. */ + authProvider?: string; state: string; tools: string[]; /** Tools whose exposure differs from the server's, from `toolExposure`. */ @@ -117,17 +122,49 @@ function describeTransport(entry: McpServerEntry): string { return "url" in config ? config.url : [config.command, ...(config.args ?? [])].join(" "); } -function createConnection(entry: McpServerEntry, options: McpCommandOptions, credentials: McpOAuthCredentialStore) { +function getAuthProvider(entry: McpServerEntry): string | undefined { + return "url" in entry.config ? entry.config.auth?.provider : undefined; +} + +function createConnection( + entry: McpServerEntry, + options: McpCommandOptions, + credentials: McpOAuthCredentialStore, + providerToken: (provider: string) => Promise, +) { return new McpServerConnection({ entry, cwd: options.cwd, createTransport: createDefaultTransport, credentials, + providerToken, log: new McpServerLog(join(options.agentDir, "mcp.log")), onTools: () => {}, }); } +/** + * Reads provider tokens from the agent directory's credentials, refreshing expired OAuth tokens like a + * session does. The model runtime is created on the first request, so servers without `auth.provider` + * do not load it. + */ +function createProviderTokenReader(agentDir: string): (provider: string) => Promise { + let runtime: Promise | undefined; + return async (provider) => { + runtime ??= ModelRuntime.create({ + authPath: join(agentDir, "auth.json"), + modelsPath: join(agentDir, "models.json"), + refreshOnCreate: false, + }); + try { + return (await (await runtime).getAuth(provider))?.auth.apiKey; + } catch { + // Missing or unrefreshable credentials: the server's 401 then reports that sign-in is needed. + return undefined; + } + }; +} + /** Short spellings of options. `-l`/`--local` match `pi install`. */ const OPTION_ALIASES = new Map([["-l", "--local"]]); @@ -207,6 +244,7 @@ export async function runMcpCommand(args: string[], options: McpCommandOptions): ? `${projectConfig} is ignored because the project is not trusted. Start ${APP_NAME} in the project to trust it.` : undefined; const credentials = options.credentials ?? new McpOAuthCredentialStore(); + const providerToken = options.providerToken ?? createProviderTokenReader(options.agentDir); switch (command) { case "list": { @@ -216,7 +254,7 @@ export async function runMcpCommand(args: string[], options: McpCommandOptions): error(`Usage: ${APP_NAME} mcp list [--json]\n${HELP_HINT}`); return 1; } - return list(loaded, parsed.values.has("json"), untrustedNote, options, credentials, log); + return list(loaded, parsed.values.has("json"), untrustedNote, options, credentials, providerToken, log); } case "login": case "logout": { @@ -244,7 +282,14 @@ export async function runMcpCommand(args: string[], options: McpCommandOptions): error(`MCP server "${name}" is defined in ${entry.source}, not in the project; omit --local.`); return 1; } - const connection = createConnection(entry, options, credentials); + const authProvider = getAuthProvider(entry); + if (authProvider) { + error( + `MCP server "${name}" uses the credentials of provider "${authProvider}". Run /${command} ${authProvider} in ${APP_NAME}.`, + ); + return 1; + } + const connection = createConnection(entry, options, credentials, providerToken); const url = connection.oauthUrl; if (!url) { error(`MCP server "${name}" does not use OAuth. Only HTTP servers without an Authorization header do.`); @@ -468,10 +513,12 @@ async function list( untrustedNote: string | undefined, options: McpCommandOptions, credentials: McpOAuthCredentialStore, + providerToken: (provider: string) => Promise, log: (line: string) => void, ): Promise { const reports = await Promise.all( loaded.servers.map(async (entry): Promise => { + const authProvider = getAuthProvider(entry); const report: ServerReport = { name: entry.name, scope: entry.scope ?? "global", @@ -480,11 +527,12 @@ async function list( enabled: entry.config.enabled !== false, exposure: entry.config.exposure ?? "codemode", transport: describeTransport(entry), + ...(authProvider ? { authProvider } : {}), state: "disabled", tools: [], }; if (!report.enabled) return report; - const connection = createConnection(entry, options, credentials); + const connection = createConnection(entry, options, credentials, providerToken); try { await connection.getClient(); } catch { @@ -528,7 +576,13 @@ async function list( log(`${report.name}: ${state} (${report.exposure}, ${report.scope})`); log(` ${report.transport}`); if (report.override) log(` project override: ${report.override}`); - if (report.state === "needs-auth") log(` sign in with: ${APP_NAME} mcp login ${report.name}`); + if (report.state === "needs-auth") { + log( + report.authProvider + ? ` sign in with: /login ${report.authProvider} in ${APP_NAME}` + : ` sign in with: ${APP_NAME} mcp login ${report.name}`, + ); + } if (report.tools.length > 0) { const tools = report.tools.map((tool) => { const exposure = report.toolExposure?.[tool]; diff --git a/packages/coding-agent/test/mcp-command.test.ts b/packages/coding-agent/test/mcp-command.test.ts index 2e9a0a1fa..5f09b37eb 100644 --- a/packages/coding-agent/test/mcp-command.test.ts +++ b/packages/coding-agent/test/mcp-command.test.ts @@ -4,6 +4,7 @@ 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"; +import { startOAuthMcpServer } from "./suite/mcp-oauth-server.ts"; const FIXTURE = resolve(import.meta.dirname, "../../mcp/test/fixtures/stdio-server.mjs"); @@ -14,7 +15,12 @@ describe("pi mcp", () => { while (dirs.length > 0) rmSync(dirs.pop() ?? "", { recursive: true, force: true }); }); - async function run(args: string[], servers: Record | undefined, dir?: string) { + async function run( + args: string[], + servers: Record | undefined, + dir?: string, + providerToken?: (provider: string) => Promise, + ) { const agentDir = dir ?? mkdtempSync(join(tmpdir(), "pi-mcp-command-")); if (!dir) dirs.push(agentDir); if (servers) writeFileSync(join(agentDir, "mcp.json"), JSON.stringify({ mcpServers: servers })); @@ -22,6 +28,7 @@ describe("pi mcp", () => { const exitCode = await runMcpCommand(args, { cwd: agentDir, agentDir, + providerToken, log: (line) => output.push(line), error: (line) => output.push(line), }); @@ -81,6 +88,36 @@ describe("pi mcp", () => { ); }); + it("sends the token of the provider named by auth.provider", async () => { + const server = await startOAuthMcpServer(); + try { + const radius = { radius: { url: server.url, auth: { provider: "radius" } } }; + const token = server.issueAccessToken(); + const requested: string[] = []; + const signedIn = await run(["list"], radius, undefined, async (provider) => { + requested.push(provider); + return token; + }); + expect(signedIn).toMatchObject({ exitCode: 0, output: expect.stringContaining("radius: connected, 1 tool") }); + expect(requested).toContain("radius"); + + // Without the provider's credentials the hint names the provider's /login, not `pi mcp login`. + const signedOut = await run(["list"], radius, undefined, async () => undefined); + expect(signedOut.exitCode).toBe(1); + expect(signedOut.output).toContain("radius: needs sign-in (codemode, global)"); + expect(signedOut.output).toContain(" sign in with: /login radius in pi"); + + for (const command of ["login", "logout"]) { + expect(await run([command, "radius"], radius)).toMatchObject({ + exitCode: 1, + output: `MCP server "radius" uses the credentials of provider "radius". Run /${command} radius in pi.`, + }); + } + } finally { + await server.close(); + } + }); + it("reports options as typed", async () => { expect((await run(["list", "-l"], servers)).output).toContain("Unknown option -l."); }); diff --git a/packages/coding-agent/test/suite/mcp-oauth-server.ts b/packages/coding-agent/test/suite/mcp-oauth-server.ts index cd2126cea..317d15cfc 100644 --- a/packages/coding-agent/test/suite/mcp-oauth-server.ts +++ b/packages/coding-agent/test/suite/mcp-oauth-server.ts @@ -171,6 +171,8 @@ export async function startOAuthMcpServer( stall, /** Requests to stalled paths, still open unless the client gave up. */ stalled, + /** A valid access token issued without a sign-in, like one of a pi provider. */ + issueAccessToken: () => issueTokens().access_token, /** Simulates access token expiry. */ expireAccessTokens: () => validTokens.clear(), close: () =>