Add OAuth support for plugin MCP servers (#42804)

* Add OAuth support for plugin MCP servers

* Localize plugin uninstall cleanup warnings

Author: awlevin

ARCHITECTURE.md

@@ -38,6 +38,7 @@ This file is the cross-system architecture index. Detailed designs live in domai
 - Bundled-skill outbound API calls that require credentials use the Credential Execution Service (CES) tools (`make_authenticated_request`, `run_authenticated_command`) rather than manual token plumbing or proxied shell execution. See `assistant/docs/credential-execution-service.md`.
 - Managed shared-identity channel routing runs in a separate managed-gateway service lane from the per-assistant `gateway/` lane. The deployable managed-gateway runtime is platform-owned; this repo keeps public contracts/fixtures under `gateway-managed/`.
 - Production LLM calls go through the provider abstraction, not provider SDKs in feature code.
+- Plugin-declared remote MCP servers store OAuth records under `mcp-plugin/v1/<plugin>/<server>/<endpoint-digest>/<leaf>`, where plugin and server segments are base64url encoded and the SHA-256 endpoint digest covers the transport type plus the canonical URL without its fragment. The runtime public server id is presentation and routing state, not credential identity. Workspace MCP servers keep the existing `mcp:<serverId>:<leaf>` keys. This isolates plugin credentials by installed owner, original `mcp.json` key, and endpoint while leaving persisted workspace files unchanged. Plugin uninstall deletes the exact owner prefix when credential storage is reachable. If storage is unavailable, current `mcp.json` or install-fingerprint evidence blocks removal; without that evidence, removal succeeds with a warning because older credentials cannot be verified. An upgrade that removed both the declaration and its recorded fingerprint is indistinguishable from a plugin that never declared MCP, so historical keys may remain after offline removal.
 - The macOS and Windows Electron shells share platform-neutral window security, IPC validation, origin checks, and preload capability registration through `@vellumai/electron-desktop`, plus native helper process and JSON-RPC lifecycle through `@vellumai/native-sidecar`. Each client keeps platform lifecycle and native features in its own adapter modules under `clients/<platform>/src/`. Both preloads implement the same `VellumBridge` contract (`packages/ipc-contract`); a surface only one shell can back is optional there and documented in [`clients/windows/docs/parity-matrix.md`](clients/windows/docs/parity-matrix.md), which `clients/windows/src/preload/bridge-parity.test.ts` enforces against the macOS preload.
 - Packaged Windows startup provisions a user-scoped CLI runtime from
   `resources/cli-runtime`. Versioned installs and one fallback live under

assistant/openapi.yaml

@@ -26544,6 +26544,12 @@ paths:
                   target:
                     type: string
                     description: Absolute path that was removed on the assistant host. Useful for audit logs and confirmation toasts.
+                  warnings:
+                    description: Stable keys for non-fatal cleanup limitations that should be localized at the presentation edge.
+                    type: array
+                    items:
+                      type: string
+                      const: plugin.uninstall.mcp_oauth_credentials_unchecked
                 required:
                   - name
                   - target

assistant/src/__tests__/mcp-auth-routes.test.ts

@@ -3,11 +3,29 @@ import { beforeEach, describe, expect, mock, test } from "bun:test";
 // ── Module mocks (must precede imports) ───────────────────────────────────────
 
 const mockReloadMcpServers = mock(async () => {});
+let pluginServers: Array<{
+  id: string;
+  pluginName: string;
+  serverKey: string;
+  config: {
+    source: "plugin";
+    pluginName: string;
+    serverKey: string;
+    transport: {
+      type: "streamable-http";
+      url: string;
+    };
+  };
+}> = [];
 
 mock.module("../daemon/mcp-reload-service.js", () => ({
   reloadMcpServers: () => mockReloadMcpServers(),
 }));
 
+mock.module("../plugins/mcp-servers.js", () => ({
+  readPluginMcpServers: () => ({ servers: pluginServers, issues: [] }),
+}));
+
 const mockOrchestrateConnect = mock(
   async (_args: { serverId: string; transport: unknown }) => ({
     auth_url: "https://provider.example.com/authorize?state=abc",
@@ -55,6 +73,7 @@ describe("mcp-auth-routes", () => {
     mockGetMcpAuthState.mockClear();
     mockGetMcpAuthState.mockImplementation(() => null);
     mockReloadMcpServers.mockClear(); // ← add this line
+    pluginServers = [];
   });
 
   describe("POST internal/mcp/auth/start", () => {
@@ -68,6 +87,86 @@ describe("mcp-auth-routes", () => {
         auth_url: "https://provider.example.com/authorize?state=abc",
         state: "my-server",
       });
+      expect(mockOrchestrateConnect).toHaveBeenCalledWith(
+        expect.objectContaining({
+          serverId: "my-server",
+          credentialTarget: {
+            source: "workspace",
+            serverId: "my-server",
+          },
+        }),
+      );
+    });
+
+    test("resolves an installed plugin server by its public id", async () => {
+      pluginServers = [
+        {
+          id: "plugin-auth__remote",
+          pluginName: "plugin-auth",
+          serverKey: "remote",
+          config: {
+            source: "plugin",
+            pluginName: "plugin-auth",
+            serverKey: "remote",
+            transport: {
+              type: "streamable-http",
+              url: "https://mcp.example.com/plugin",
+            },
+          },
+        },
+      ];
+
+      const startRoute = findRoute("internal_mcp_auth_start");
+      await startRoute.handler({
+        body: { serverId: "plugin-auth__remote" },
+      });
+
+      expect(mockOrchestrateConnect).toHaveBeenCalledWith(
+        expect.objectContaining({
+          serverId: "plugin-auth__remote",
+          credentialTarget: {
+            source: "plugin",
+            pluginName: "plugin-auth",
+            serverKey: "remote",
+            transportType: "streamable-http",
+            url: "https://mcp.example.com/plugin",
+          },
+        }),
+      );
+    });
+
+    test("keeps workspace precedence when a plugin declares the same id", async () => {
+      pluginServers = [
+        {
+          id: "my-server",
+          pluginName: "my-server",
+          serverKey: "my-server",
+          config: {
+            source: "plugin",
+            pluginName: "my-server",
+            serverKey: "my-server",
+            transport: {
+              type: "streamable-http",
+              url: "https://loses.example.com/mcp",
+            },
+          },
+        },
+      ];
+
+      const startRoute = findRoute("internal_mcp_auth_start");
+      await startRoute.handler({ body: { serverId: "my-server" } });
+
+      expect(mockOrchestrateConnect).toHaveBeenCalledWith(
+        expect.objectContaining({
+          transport: expect.objectContaining({
+            url: "https://mcp.example.com",
+          }),
+          credentialTarget: {
+            source: "workspace",
+            serverId: "my-server",
+          },
+        }),
+      );
     });
 
     test("rejects unknown serverId with BadRequestError", async () => {

assistant/src/__tests__/mcp-list-plugin-servers.test.ts

@@ -8,18 +8,22 @@
  *
  * 1. A workspace entry of the same id wins. Getting precedence backwards
  *    would let a plugin redirect a server the user configured by hand.
- * 2. A plugin server is never health-checked. `McpClient.connect` resolves
- *    `mcp:<serverId>:headers` and `mcp:<serverId>:tokens` from the
- *    credential store, and a plugin controls both its server key and its
- *    URL, so probing one would send a workspace credential to an endpoint
- *    the plugin chose whenever an id happens to match a stored key.
+ * 2. A plugin server is never health-checked as a side effect of listing.
+ *    OAuth status is read from the plugin and endpoint-scoped credential
+ *    identity, while workspace static headers remain isolated.
  */
 
 import { mkdirSync, rmSync, writeFileSync } from "node:fs";
 import { join } from "node:path";
 import { beforeEach, describe, expect, jest, mock, test } from "bun:test";
 
 const getServerState = jest.fn();
+const hasMcpOAuthTokens = jest.fn(async (target: any) => {
+  if (target.source === "plugin" && target.url === "not a url") {
+    throw new TypeError("invalid URL");
+  }
+  return true;
+});
 
 mock.module("../mcp/client.js", () => ({
   McpClient: class {
@@ -45,9 +49,7 @@ mock.module("../mcp/mcp-auth-state.js", () => ({
 }));
 
 mock.module("../mcp/mcp-oauth-provider.js", () => ({
-  // Stand in for a credential store that holds tokens for every id, which
-  // is the condition under which a leak would be observable.
-  hasMcpOAuthTokens: async () => true,
+  hasMcpOAuthTokens,
   deleteMcpOAuthCredentials: async () => ({ ok: true, failedKeys: [] }),
 }));
 
@@ -130,6 +132,7 @@ async function listServers(): Promise<ListedServer[]> {
 describe("internal_mcp_list, plugin-declared servers", () => {
   beforeEach(() => {
     getServerState.mockReset();
+    hasMcpOAuthTokens.mockClear();
     rmSync(getWorkspacePluginsDir(), { recursive: true, force: true });
     mkdirSync(getWorkspacePluginsDir(), { recursive: true });
   });
@@ -198,13 +201,21 @@ describe("internal_mcp_list, plugin-declared servers", () => {
     expect(plugin.status).toBe("connected");
   });
 
-  test("plugin servers report no assistant-owned auth even when the store has some", async () => {
+  test("plugin servers report endpoint-scoped OAuth without static auth", async () => {
     writePlugin("unabyss", unabyssManifest());
 
     const plugin = (await listServers()).find((s) => s.id === "unabyss")!;
-    expect(plugin.hasOAuth).toBe(false);
+    expect(plugin.hasOAuth).toBe(true);
     expect(plugin.hasStaticAuth).toBe(false);
     expect(plugin.authType).toEqual("none");
+    expect(hasMcpOAuthTokens).toHaveBeenCalledWith(
+      expect.objectContaining({
+        source: "plugin",
+        pluginName: "unabyss",
+        serverKey: "unabyss",
+        url: "https://mcp.unabyss.com",
+      }),
+    );
   });
 
   test("workspace servers keep reporting their auth state", async () => {
@@ -245,6 +256,20 @@ describe("internal_mcp_list, plugin-declared servers", () => {
     expect(ids).toContain("from-workspace");
   });
 
+  test("an invalid plugin transport URL does not break the listing", async () => {
+    writePlugin("bad-url", {
+      mcpServers: {
+        remote: { type: "streamable-http", url: "not a url" },
+      },
+    });
+
+    const servers = await listServers();
+    expect(servers.find((s) => s.id === "bad-url__remote")?.hasOAuth).toBe(
+      false,
+    );
+    expect(servers.some((s) => s.id === "from-workspace")).toBe(true);
+  });
+
   test("no plugins installed leaves the listing unchanged", async () => {
     const servers = await listServers();
     expect(servers.every((s) => s.source === "workspace")).toBe(true);

assistant/src/__tests__/mcp-tool-annotations-risk.test.ts

@@ -32,10 +32,12 @@ const { RiskLevel } = await import("../permissions/types.js");
 type ServerSource = "workspace" | "plugin";
 
 function serverConfig(source: ServerSource) {
-  return {
+  const base = {
     transport: { type: "stdio" as const, command: "echo", args: [] },
-    source,
   };
+  return source === "workspace"
+    ? { ...base, source }
+    : { ...base, source, pluginName: "plugin", serverKey: "server" };
 }
 
 interface RiskAnnotations {

assistant/src/cli/commands/plugins.ts

@@ -53,6 +53,7 @@ import {
   PluginAlreadyInStateException,
   PluginDirectoryNotFoundError,
 } from "../lib/toggle-plugin.js";
+import type { PluginUninstallWarningKey } from "../lib/uninstall-plugin.js";
 import type { PluginUpgradeResult } from "../lib/upgrade-plugin.js";
 import { getCliLogger } from "../logger.js";
 import { PLUGINS_SEARCH_INSTALL_HINT, pluginsHelp } from "./plugins.help.js";
@@ -666,11 +667,16 @@ export function registerPluginsCommand(program: Command): void {
             // when the daemon is unreachable (a transport error carries no
             // `statusCode`); an operator can still uninstall while it's stopped,
             // and `shutdown` then runs in this process, the only one available.
-            const daemon = await cliIpcCall<{ name: string; target: string }>(
-              "plugins_uninstall",
-              { pathParams: { name } },
-            );
-            let result: { name: string; target: string };
+            const daemon = await cliIpcCall<{
+              name: string;
+              target: string;
+              warnings?: PluginUninstallWarningKey[];
+            }>("plugins_uninstall", { pathParams: { name } });
+            let result: {
+              name: string;
+              target: string;
+              warnings?: PluginUninstallWarningKey[];
+            };
             if (daemon.ok && daemon.result) {
               result = daemon.result;
             } else if (daemon.statusCode === undefined) {
@@ -693,6 +699,11 @@ export function registerPluginsCommand(program: Command): void {
             console.log(
               `Uninstalled plugin "${result.name}" from ${result.target}`,
             );
+            for (const warning of result.warnings ?? []) {
+              console.warn(
+                `Warning: ${libs.uninstall.resolvePluginUninstallWarning(warning)}`,
+              );
+            }
           } catch (err) {
             if (err instanceof libs.installGitHub.InvalidPluginNameError) {
               console.error(err.message);