fix(coding-agent): store MCP OAuth credentials per server name and URL

Entries sharing a URL now keep separate accounts. Credentials stored by
URL alone move to the first server that loads them.

closes #10252

packages/coding-agent/CHANGELOG.md

@@ -6,6 +6,10 @@
 
 - Added an `oauth.authServerMetadataUrl` setting for MCP servers that advertise a wrong OAuth authorization server or none. Pi uses the configured metadata document instead of discovery ([#10172](https://github.com/earendil-works/pi/issues/10172)).
 
+### Changed
+
+- MCP OAuth credentials are now stored per server name and URL, so MCP servers with the same URL can sign in with different accounts. Credentials stored by URL alone move to the first server that uses them ([#10252](https://github.com/earendil-works/pi/issues/10252)).
+
 ### Fixed
 
 - Fixed MCP OAuth sign-in accepting an authorization response whose `iss` parameter names another authorization server; the code is now rejected before it is exchanged (RFC 9207).

packages/coding-agent/docs/mcp.md

@@ -118,6 +118,8 @@ When the server rejects an unauthenticated connection, `/mcp` shows that it need
 
 Pi registers itself with the authorization server, stores tokens in `~/.pi/agent/mcp-auth.json`, and refreshes access tokens when they expire or the server rejects them. If a server later requests additional scope, Pi asks for sign-in again. Signing out deletes the stored credentials.
 
+Credentials belong to a server name and URL. Servers with the same URL under different names, such as one per account, sign in separately; servers with the same name and URL in different `mcp.json` files share one sign-in.
+
 OAuth applies to HTTP servers without an `Authorization` header. For a server that does not support dynamic client registration, configure a registered client:
 
 ```json

packages/coding-agent/src/extensions/mcp/cli.ts

@@ -236,7 +236,7 @@ export async function runMcpCommand(args: string[], options: McpCommandOptions):
 				return 1;
 			}
 			if (command === "logout") {
-				const removed = credentials.remove(url);
+				const removed = credentials.remove(name, url);
 				log(removed ? `Signed out of MCP server "${name}".` : `No stored credentials for MCP server "${name}".`);
 				return 0;
 			}
@@ -542,7 +542,7 @@ async function login(
 	try {
 		await signInMcpServer({
 			serverUrl: url,
-			store: credentials.forServer(url),
+			store: credentials.forServer(name, url),
 			settings: connection.oauthSettings(),
 			challenge: connection.challenge,
 			prompt: {

packages/coding-agent/src/extensions/mcp/index.ts

@@ -486,7 +486,7 @@ export function createMcpExtension(options: McpExtensionOptions = {}): Extension
 		const tokensAtSignIn = new Map<McpServerConnection, string>();
 		const storedTokens = (connection: McpServerConnection): string => {
 			const url = connection.oauthUrl;
-			return url && credentials ? JSON.stringify(credentials.tokens(url) ?? null) : "null";
+			return url && credentials ? JSON.stringify(credentials.tokens(connection.name, url) ?? null) : "null";
 		};
 		const onConnectionChange = (connection: McpServerConnection) => {
 			if (connection.state !== "needs-auth") tokensAtSignIn.delete(connection);
@@ -598,7 +598,7 @@ export function createMcpExtension(options: McpExtensionOptions = {}): Extension
 			try {
 				await runtime.signInMcpServer({
 					serverUrl: url,
-					store: getCredentials(runtime).forServer(url),
+					store: getCredentials(runtime).forServer(server.entry.name, url),
 					settings: connection.oauthSettings(),
 					challenge: connection.challenge,
 					prompt,
@@ -621,7 +621,7 @@ export function createMcpExtension(options: McpExtensionOptions = {}): Extension
 			const connection = server.connection;
 			const url = connection?.oauthUrl;
 			if (!connection || !url) return false;
-			const removed = getCredentials(await loadMcpRuntime()).remove(url);
+			const removed = getCredentials(await loadMcpRuntime()).remove(server.entry.name, url);
 			await connection.signOut();
 			return removed;
 		};

packages/coding-agent/src/extensions/mcp/oauth.ts

@@ -6,7 +6,7 @@
  * `McpOAuthAuthorizationRequiredError`, and the user signs in through `/mcp`, which runs
  * the authorization code flow (PKCE, dynamic client registration) against a loopback callback.
  *
- * Credentials live in `<agent-dir>/mcp-auth.json`, keyed by server URL.
+ * Credentials live in `<agent-dir>/mcp-auth.json`, keyed by server name and URL.
  */
 
 import { createHash } from "node:crypto";
@@ -31,6 +31,7 @@ import {
 import lockfile from "proper-lockfile";
 import { APP_NAME, getAgentDir } from "../../config.ts";
 import { type AuthStorageBackend, FileAuthStorageBackend } from "../../core/auth-storage.ts";
+import { mcpNamespace } from "../../core/mcp-servers.ts";
 
 const CALLBACK_HOST = "127.0.0.1";
 const CALLBACK_PATH = "/callback";
@@ -108,6 +109,19 @@ function parseStates(content: string | undefined): StoredStates {
 	return typeof parsed === "object" && parsed !== null && !Array.isArray(parsed) ? (parsed as StoredStates) : {};
 }
 
+function serializeStates(states: StoredStates): string {
+	return `${JSON.stringify(states, null, 2)}\n`;
+}
+
+/**
+ * Keys of a server's state: by name and URL, so servers sharing a URL keep separate accounts, and the
+ * legacy key by URL alone, written by older versions.
+ */
+function storeKeys(name: string, serverUrl: string): { key: string; legacyKey: string } {
+	const legacyKey = String(new URL(serverUrl));
+	return { key: `${mcpNamespace(name)}|${legacyKey}`, legacyKey };
+}
+
 export interface McpOAuthServerStore extends McpOAuthStateStore {
 	/** Run `fn` while no other process refreshes the server's tokens. */
 	withRefreshLock<T>(fn: () => Promise<T>): Promise<T>;
@@ -124,10 +138,18 @@ export class McpOAuthCredentialStore {
 		this.lockDir = backend ? lockDir : getAgentDir();
 	}
 
-	forServer(serverUrl: string): McpOAuthServerStore {
-		const key = String(new URL(serverUrl));
+	forServer(name: string, serverUrl: string): McpOAuthServerStore {
+		const { key, legacyKey } = storeKeys(name, serverUrl);
 		return {
-			load: () => this.read()[key],
+			// The first server to load legacy state takes it over; others with the same URL sign in again.
+			load: () =>
+				this.backend.withLock((current) => {
+					const states = parseStates(current);
+					if (states[key] || !states[legacyKey]) return { result: states[key] };
+					states[key] = states[legacyKey];
+					delete states[legacyKey];
+					return { result: states[key], next: serializeStates(states) };
+				}),
 			save: (state) =>
 				this.write((states) => {
 					states[key] = state;
@@ -163,30 +185,30 @@ export class McpOAuthCredentialStore {
 		}
 	}
 
-	/** The stored tokens of a server, for noticing sign-ins done by another process. */
-	tokens(serverUrl: string): McpOAuthState["tokens"] {
-		return this.read()[String(new URL(serverUrl))]?.tokens;
+	/** The stored tokens of a server, for noticing sign-ins done by another process. Does not take over legacy state. */
+	tokens(name: string, serverUrl: string): McpOAuthState["tokens"] {
+		const { key, legacyKey } = storeKeys(name, serverUrl);
+		const states = this.backend.withLock((current) => ({ result: parseStates(current) }));
+		return (states[key] ?? states[legacyKey])?.tokens;
 	}
 
-	/** Returns whether credentials were stored for the server. */
-	remove(serverUrl: string): boolean {
-		const key = String(new URL(serverUrl));
-		if (!(key in this.read())) return false;
-		this.write((states) => {
-			delete states[key];
+	/** Returns whether credentials were stored for the server. Removes legacy state the server would take over. */
+	remove(name: string, serverUrl: string): boolean {
+		const { key, legacyKey } = storeKeys(name, serverUrl);
+		return this.backend.withLock((current) => {
+			const states = parseStates(current);
+			const stored = key in states ? key : legacyKey in states ? legacyKey : undefined;
+			if (!stored) return { result: false };
+			delete states[stored];
+			return { result: true, next: serializeStates(states) };
 		});
-		return true;
-	}
-
-	private read(): StoredStates {
-		return this.backend.withLock((current) => ({ result: parseStates(current) }));
 	}
 
 	private write(update: (states: StoredStates) => void): void {
 		this.backend.withLock((current) => {
 			const states = parseStates(current);
 			update(states);
-			return { result: undefined, next: `${JSON.stringify(states, null, 2)}\n` };
+			return { result: undefined, next: serializeStates(states) };
 		});
 	}
 }

packages/coding-agent/src/extensions/mcp/runtime.ts

@@ -204,7 +204,7 @@ export class McpServerConnection implements McpToolCaller, McpResourceServer {
 		this.authProvider = url
 			? createMcpAuthProvider({
 					serverUrl: url,
-					store: options.credentials.forServer(url),
+					store: options.credentials.forServer(this.entry.name, url),
 					settings: () => this.oauthSettings(),
 					onChallenge: (challenge) => {
 						this.challenge = challenge;

packages/coding-agent/test/mcp-conformance/client.ts

@@ -151,7 +151,7 @@ async function run(serverUrl: string, scenario: string): Promise<void> {
 				log(`sign-in ${signIns}${connection.challenge?.scope ? ` (scope: ${connection.challenge.scope})` : ""}`);
 				await signInMcpServer({
 					serverUrl: url,
-					store: credentials.forServer(url),
+					store: credentials.forServer(entry.name, url),
 					settings: connection.oauthSettings(),
 					challenge: connection.challenge,
 					prompt: simulatedBrowser(),

packages/coding-agent/test/mcp-oauth-refresh.test.ts

@@ -27,7 +27,7 @@ describe("MCP OAuth refresh", () => {
 		// Stores sharing the credential file and lock directory stand in for separate pi processes.
 		const backend = new InMemoryAuthStorageBackend();
 		const process = () => {
-			const store = new McpOAuthCredentialStore(backend, lockDir).forServer(server.url);
+			const store = new McpOAuthCredentialStore(backend, lockDir).forServer("test", server.url);
 			const provider = createMcpAuthProvider({
 				serverUrl: server.url,
 				store,
@@ -95,7 +95,7 @@ describe("MCP OAuth sign-in", () => {
 		try {
 			await signInMcpServer({
 				serverUrl: server.url,
-				store: new McpOAuthCredentialStore(new InMemoryAuthStorageBackend()).forServer(server.url),
+				store: new McpOAuthCredentialStore(new InMemoryAuthStorageBackend()).forServer("test", server.url),
 				settings: settings(server.url),
 				prompt: {
 					showAuthorizationUrl: (url) => void fetch(url),
@@ -115,7 +115,7 @@ describe("MCP OAuth sign-in", () => {
 	it("keeps the granted scope when the server asks for more", async () => {
 		const server = await startOAuthMcpServer();
 		try {
-			const store = new McpOAuthCredentialStore(new InMemoryAuthStorageBackend()).forServer(server.url);
+			const store = new McpOAuthCredentialStore(new InMemoryAuthStorageBackend()).forServer("test", server.url);
 			const opened: URL[] = [];
 			const signInWith = (challenge: OAuthChallenge) =>
 				signInMcpServer({

packages/coding-agent/test/mcp-oauth-store.test.ts

@@ -0,0 +1,55 @@
+import { describe, expect, it } from "vitest";
+import { InMemoryAuthStorageBackend } from "../src/core/auth-storage.ts";
+import { McpOAuthCredentialStore } from "../src/extensions/mcp/oauth.ts";
+
+const SERVER_URL = "https://mcp.example.com/mcp";
+
+function state(accessToken: string) {
+	return { serverUrl: SERVER_URL, tokens: { access_token: accessToken, token_type: "Bearer" } };
+}
+
+function storedKeys(backend: InMemoryAuthStorageBackend): string[] {
+	return Object.keys(JSON.parse(backend.withLock((current) => ({ result: current ?? "{}" }))));
+}
+
+describe("MCP OAuth credential store", () => {
+	// https://github.com/earendil-works/pi/issues/10252
+	it("keeps separate credentials for servers sharing a SERVER_URL", async () => {
+		const store = new McpOAuthCredentialStore(new InMemoryAuthStorageBackend());
+		await store.forServer("work", SERVER_URL).save(state("work-token"));
+		await store.forServer("personal", SERVER_URL).save(state("personal-token"));
+
+		expect((await store.forServer("work", SERVER_URL).load())?.tokens?.access_token).toBe("work-token");
+		expect((await store.forServer("personal", SERVER_URL).load())?.tokens?.access_token).toBe("personal-token");
+
+		expect(store.remove("work", SERVER_URL)).toBe(true);
+		expect(await store.forServer("work", SERVER_URL).load()).toBeUndefined();
+		expect(store.tokens("personal", SERVER_URL)?.access_token).toBe("personal-token");
+	});
+
+	it("moves credentials stored by SERVER_URL to the first server that loads them", async () => {
+		const backend = new InMemoryAuthStorageBackend();
+		backend.withLock(() => ({ result: undefined, next: JSON.stringify({ [SERVER_URL]: state("legacy-token") }) }));
+		const store = new McpOAuthCredentialStore(backend);
+
+		// Reading tokens does not take the legacy state over.
+		expect(store.tokens("work", SERVER_URL)?.access_token).toBe("legacy-token");
+		expect(storedKeys(backend)).toEqual([SERVER_URL]);
+
+		expect((await store.forServer("my_work", SERVER_URL).load())?.tokens?.access_token).toBe("legacy-token");
+		// Names differing only in `-` and `_` are the same server.
+		expect(store.tokens("my-work", SERVER_URL)?.access_token).toBe("legacy-token");
+		expect(await store.forServer("personal", SERVER_URL).load()).toBeUndefined();
+		expect(storedKeys(backend)).toEqual([`mcp__my_work|${SERVER_URL}`]);
+	});
+
+	it("signs out of credentials stored by SERVER_URL", () => {
+		const backend = new InMemoryAuthStorageBackend();
+		backend.withLock(() => ({ result: undefined, next: JSON.stringify({ [SERVER_URL]: state("legacy-token") }) }));
+		const store = new McpOAuthCredentialStore(backend);
+
+		expect(store.remove("work", SERVER_URL)).toBe(true);
+		expect(storedKeys(backend)).toEqual([]);
+		expect(store.remove("work", SERVER_URL)).toBe(false);
+	});
+});