Merge pull request #364 from RhysSullivan/rs/mcp-defer-oauth-signin

mcp: allow saving OAuth2 source before sign-in

Author: RhysSullivan

packages/plugins/mcp/src/react/AddMcpSource.tsx

@@ -378,12 +378,16 @@ export default function AddMcpSource(props: {
   const remoteHeadersComplete = remoteHeaders.every(
     (header) => header.name.trim() && header.value.trim(),
   );
+  // OAuth is "ready to save" even without tokens — the source is stored
+  // with a stable connectionId pointer, and each user completes their
+  // own sign-in via McpSignInButton on the source detail page (per-user
+  // scope shadowing means each user's tokens land at their own scope).
   const authReady =
     remoteAuthMode === "none"
       ? canUseNone
       : remoteAuthMode === "header"
         ? headerAuthComplete
-        : tokens !== null;
+        : true;
   const canAdd = Boolean(probe) && authReady && remoteHeadersComplete && !isAdding && !isOAuthBusy;
   // Probe failures are shown inline on the URL field; other failures
   // (OAuth start, add source) render in the bottom error block.
@@ -493,6 +497,17 @@ export default function AddMcpSource(props: {
     if (!probe) return;
     dispatch({ type: "add-start" });
     const headerAuth = remoteAuthHeaders[0];
+    // For oauth2 sources saved without completing the flow, use the
+    // same stable connectionId the handleOAuth path would have used.
+    // This pins the source's auth pointer, so when a per-user sign-in
+    // runs later (via McpSignInButton) it mints the connection at the
+    // user scope against the same id — innermost-wins shadowing then
+    // resolves tokens per-user at invoke time.
+    const deferredOAuthConnectionId = mcpOAuthConnectionId(
+      slugifyNamespace(remoteIdentity.namespace) ||
+        slugifyNamespace(probe.namespace ?? "") ||
+        "mcp",
+    );
     const auth =
       remoteAuthMode === "header" && headerAuth?.secretId
         ? {
@@ -501,10 +516,10 @@ export default function AddMcpSource(props: {
             secretId: headerAuth.secretId,
             ...(headerAuth.prefix ? { prefix: headerAuth.prefix } : {}),
           }
-        : remoteAuthMode === "oauth2" && tokens
+        : remoteAuthMode === "oauth2"
           ? {
               kind: "oauth2" as const,
-              connectionId: tokens.connectionId,
+              connectionId: tokens?.connectionId ?? deferredOAuthConnectionId,
             }
           : { kind: "none" as const };
     const headers = Object.fromEntries(
@@ -788,9 +803,15 @@ export default function AddMcpSource(props: {
               {remoteAuthMode === "oauth2" && (
                 <>
                   {!tokens && state.step === "probed" && (
-                    <Button onClick={handleOAuth}  variant="outline">
-                      Sign in
-                    </Button>
+                    <div className="flex flex-col gap-2">
+                      <Button onClick={handleOAuth} variant="outline">
+                        Sign in
+                      </Button>
+                      <p className="text-[11px] text-muted-foreground">
+                        Optional — you can save the source now and each user can sign
+                        in from the source detail page later.
+                      </p>
+                    </div>
                   )}
 
                   {!tokens && state.step === "oauth-starting" && (

packages/plugins/mcp/src/sdk/plugin.test.ts

@@ -650,8 +650,15 @@ export const mcpPlugin = definePlugin(
             const namespace = normalizeNamespace(config);
             const sd = toStoredSourceData(config);
 
-            // Resolve auth (may fail if stdio gate is off)
-            const ci = yield* resolveConnectorInput(sd, ctx, allowStdio).pipe(
+            // Stdio sources are gated — a resolver failure there is a
+            // config error the admin must fix before the source makes
+            // sense to persist at all. For remote sources we defer the
+            // resolver failure: auth might not be ready yet (oauth2
+            // connection awaiting per-user sign-in, header secret
+            // awaiting upload) but the source row should still land so
+            // it shows up in the list and exposes a Sign-in affordance.
+            const resolved = yield* resolveConnectorInput(sd, ctx, allowStdio).pipe(
+              Effect.either,
               Effect.withSpan("mcp.plugin.resolve_connector", {
                 attributes: {
                   "mcp.source.namespace": namespace,
@@ -660,20 +667,26 @@ export const mcpPlugin = definePlugin(
               }),
             );
 
-            const connector = createMcpConnector(ci);
-            // Try discovery. If it fails (auth, network, bad spec), we still
-            // want the source to land in the catalog so users see it in
-            // their list and can retry via refresh. The error still
-            // propagates to the caller so boot-time sync logs the reason.
-            const discovery = yield* discoverTools(connector).pipe(
-              Effect.mapError((err) =>
-                mcpDiscoveryError(`MCP discovery failed: ${err.message}`),
-              ),
-              Effect.either,
-              Effect.withSpan("mcp.plugin.discover_tools", {
-                attributes: { "mcp.source.namespace": namespace },
-              }),
-            );
+            if (resolved._tag === "Left" && sd.transport === "stdio") {
+              return yield* Effect.fail(resolved.left);
+            }
+
+            // Try discovery only if we have a live connector input.
+            // Otherwise fall straight through to the persist step with
+            // an empty manifest and surface the resolver failure to
+            // the caller at the end.
+            const discovery =
+              resolved._tag === "Right"
+                ? yield* discoverTools(createMcpConnector(resolved.right)).pipe(
+                    Effect.mapError((err) =>
+                      mcpDiscoveryError(`MCP discovery failed: ${err.message}`),
+                    ),
+                    Effect.either,
+                    Effect.withSpan("mcp.plugin.discover_tools", {
+                      attributes: { "mcp.source.namespace": namespace },
+                    }),
+                  )
+                : ({ _tag: "Left", left: resolved.left } as const);
             const manifest =
               discovery._tag === "Right"
                 ? discovery.right