History

mcp: allow saving oauth2 source before sign-in, defer per-user auth

Selecting the OAuth auth method when adding an MCP source no longer forces the admin to complete the authorization flow before Save. The source is persisted with `{kind: "oauth2", connectionId}` pointing at the stable `mcp-oauth2-${namespace}` id, and each user later mints their own backing connection via McpSignInButton on the source detail page (per-scope shadowing resolves tokens per-user at invoke time). On the plugin side, remote addSource now captures resolver failures (e.g. no connection minted yet, missing header secret) the same way it already handles discovery failures: the source row still lands in the catalog so it shows up in the list and exposes a Sign-in affordance, while the underlying error still surfaces to the caller for logging. Stdio behaviour is unchanged — a stdio resolver failure is a config error the admin must fix first.

Rhys Sullivan committed 6 months ago

Commit 5dc4aa81 parent e9fad473 files · +55 −21Browse files

Unified · Split

3 files
  1. packages/plugins/mcp/src/react/AddMcpSource.tsx +27 −6
  2. packages/plugins/mcp/src/sdk/plugin.test.ts +0 −0
  3. packages/plugins/mcp/src/sdk/plugin.ts +29 −16
packages/plugins/mcp/src/react/AddMcpSource.tsx+27−6
@@ -378,12 +378,16 @@ export default function AddMcpSource(props: { · 377 unmodified lines
378378 const remoteHeadersComplete = remoteHeaders.every(
379379 (header) => header.name.trim() && header.value.trim(),
380380 );
381+ // OAuth is "ready to save" even without tokens — the source is stored
382+ // with a stable connectionId pointer, and each user completes their
383+ // own sign-in via McpSignInButton on the source detail page (per-user
384+ // scope shadowing means each user's tokens land at their own scope).
381385 const authReady =
382386 remoteAuthMode === "none"
383387 ? canUseNone
384388 : remoteAuthMode === "header"
385389 ? headerAuthComplete
386− : tokens !== null;
390+ : true;
387391 const canAdd = Boolean(probe) && authReady && remoteHeadersComplete && !isAdding && !isOAuthBusy;
388392 // Probe failures are shown inline on the URL field; other failures
389393 // (OAuth start, add source) render in the bottom error block.
@@ -493,6 +497,17 @@ export default function AddMcpSource(props: { · 103 unmodified lines
493497 if (!probe) return;
494498 dispatch({ type: "add-start" });
495499 const headerAuth = remoteAuthHeaders[0];
500+ // For oauth2 sources saved without completing the flow, use the
501+ // same stable connectionId the handleOAuth path would have used.
502+ // This pins the source's auth pointer, so when a per-user sign-in
503+ // runs later (via McpSignInButton) it mints the connection at the
504+ // user scope against the same id — innermost-wins shadowing then
505+ // resolves tokens per-user at invoke time.
506+ const deferredOAuthConnectionId = mcpOAuthConnectionId(
507+ slugifyNamespace(remoteIdentity.namespace) ||
508+ slugifyNamespace(probe.namespace ?? "") ||
509+ "mcp",
510+ );
496511 const auth =
497512 remoteAuthMode === "header" && headerAuth?.secretId
498513 ? {
@@ -501,10 +516,10 @@ export default function AddMcpSource(props: { · 2 unmodified lines
501516 secretId: headerAuth.secretId,
502517 ...(headerAuth.prefix ? { prefix: headerAuth.prefix } : {}),
503518 }
504− : remoteAuthMode === "oauth2" && tokens
519+ : remoteAuthMode === "oauth2"
505520 ? {
506521 kind: "oauth2" as const,
507− connectionId: tokens.connectionId,
522+ connectionId: tokens?.connectionId ?? deferredOAuthConnectionId,
508523 }
509524 : { kind: "none" as const };
510525 const headers = Object.fromEntries(
@@ -788,9 +803,15 @@ export default function AddMcpSource(props: { · 277 unmodified lines
788803 {remoteAuthMode === "oauth2" && (
789804 <>
790805 {!tokens && state.step === "probed" && (
791− <Button onClick={handleOAuth} variant="outline">
792− Sign in
793− </Button>
806+ <div className="flex flex-col gap-2">
807+ <Button onClick={handleOAuth} variant="outline">
808+ Sign in
809+ </Button>
810+ <p className="text-[11px] text-muted-foreground">
811+ Optional — you can save the source now and each user can sign
812+ in from the source detail page later.
813+ </p>
814+ </div>
794815 )}
795816
796817 {!tokens && state.step === "oauth-starting" && (
packages/plugins/mcp/src/sdk/plugin.test.ts+0−0

Binary file not shown.

packages/plugins/mcp/src/sdk/plugin.ts+29−16
@@ -650,8 +650,15 @@ export const mcpPlugin = definePlugin( · 649 unmodified lines
650650 const namespace = normalizeNamespace(config);
651651 const sd = toStoredSourceData(config);
652652
653− // Resolve auth (may fail if stdio gate is off)
654− const ci = yield* resolveConnectorInput(sd, ctx, allowStdio).pipe(
653+ // Stdio sources are gated — a resolver failure there is a
654+ // config error the admin must fix before the source makes
655+ // sense to persist at all. For remote sources we defer the
656+ // resolver failure: auth might not be ready yet (oauth2
657+ // connection awaiting per-user sign-in, header secret
658+ // awaiting upload) but the source row should still land so
659+ // it shows up in the list and exposes a Sign-in affordance.
660+ const resolved = yield* resolveConnectorInput(sd, ctx, allowStdio).pipe(
661+ Effect.either,
655662 Effect.withSpan("mcp.plugin.resolve_connector", {
656663 attributes: {
657664 "mcp.source.namespace": namespace,
@@ -660,20 +667,26 @@ export const mcpPlugin = definePlugin( · 2 unmodified lines
660667 }),
661668 );
662669
663− const connector = createMcpConnector(ci);
664− // Try discovery. If it fails (auth, network, bad spec), we still
665− // want the source to land in the catalog so users see it in
666− // their list and can retry via refresh. The error still
667− // propagates to the caller so boot-time sync logs the reason.
668− const discovery = yield* discoverTools(connector).pipe(
669− Effect.mapError((err) =>
670− mcpDiscoveryError(`MCP discovery failed: ${err.message}`),
671− ),
672− Effect.either,
673− Effect.withSpan("mcp.plugin.discover_tools", {
674− attributes: { "mcp.source.namespace": namespace },
675− }),
676− );
670+ if (resolved._tag === "Left" && sd.transport === "stdio") {
671+ return yield* Effect.fail(resolved.left);
672+ }
673+
674+ // Try discovery only if we have a live connector input.
675+ // Otherwise fall straight through to the persist step with
676+ // an empty manifest and surface the resolver failure to
677+ // the caller at the end.
678+ const discovery =
679+ resolved._tag === "Right"
680+ ? yield* discoverTools(createMcpConnector(resolved.right)).pipe(
681+ Effect.mapError((err) =>
682+ mcpDiscoveryError(`MCP discovery failed: ${err.message}`),
683+ ),
684+ Effect.either,
685+ Effect.withSpan("mcp.plugin.discover_tools", {
686+ attributes: { "mcp.source.namespace": namespace },
687+ }),
688+ )
689+ : ({ _tag: "Left", left: resolved.left } as const);
677690 const manifest =
678691 discovery._tag === "Right"
679692 ? discovery.right