From 4e6089268e3aebd31c1532c5d5e4878ea3ea0853 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Fri, 8 May 2026 15:41:02 -0700 Subject: [PATCH] Fix MCP OAuth and OpenAPI JSON parsing --- packages/plugins/mcp/src/api/group.ts | 1 + .../plugins/mcp/src/react/AddMcpSource.tsx | 33 ++++++++++++------- packages/plugins/mcp/src/sdk/plugin.ts | 9 +++-- .../plugins/openapi/src/sdk/parse.test.ts | 14 ++++++++ packages/plugins/openapi/src/sdk/parse.ts | 28 +++++++++++----- packages/react/src/plugins/oauth-sign-in.tsx | 17 +++++++--- 6 files changed, 75 insertions(+), 27 deletions(-) diff --git a/packages/plugins/mcp/src/api/group.ts b/packages/plugins/mcp/src/api/group.ts index 5cb72923f..ab4d5e471 100644 --- a/packages/plugins/mcp/src/api/group.ts +++ b/packages/plugins/mcp/src/api/group.ts @@ -88,6 +88,7 @@ const ProbeEndpointPayload = Schema.Struct({ const ProbeEndpointResponse = Schema.Struct({ connected: Schema.Boolean, requiresOAuth: Schema.Boolean, + supportsDynamicRegistration: Schema.Boolean, name: Schema.String, namespace: Schema.String, toolCount: Schema.NullOr(Schema.Number), diff --git a/packages/plugins/mcp/src/react/AddMcpSource.tsx b/packages/plugins/mcp/src/react/AddMcpSource.tsx index dd0fd5078..392591999 100644 --- a/packages/plugins/mcp/src/react/AddMcpSource.tsx +++ b/packages/plugins/mcp/src/react/AddMcpSource.tsx @@ -79,6 +79,7 @@ type OAuthTokens = OAuthCompletionPayload; type ProbeResult = { connected: boolean; requiresOAuth: boolean; + supportsDynamicRegistration: boolean; name: string; namespace: string; toolCount: number | null; @@ -315,7 +316,7 @@ export default function AddMcpSource(props: { const isAdding = state.step === "adding"; const isOAuthBusy = state.step === "oauth-starting" || state.step === "oauth-waiting" || oauth.busy; - const canUseNone = probe?.requiresOAuth !== true; + const canUseNone = probe?.requiresOAuth !== true || probe.supportsDynamicRegistration === false; const remoteHeadersComplete = remoteHeaders.every( (header) => header.name.trim() && header.value.trim(), ); @@ -622,7 +623,7 @@ export default function AddMcpSource(props: { Authentication tabs={ - probe.requiresOAuth + probe.requiresOAuth && probe.supportsDynamicRegistration ? [{ value: "oauth2", label: "OAuth" }] : [ { value: "none", label: "None" }, @@ -649,16 +650,24 @@ export default function AddMcpSource(props: { label="Connect via OAuth" help="Start the provider OAuth flow." > - {!tokens && state.step === "probed" && ( - - )} + {!tokens && + state.step === "probed" && + (probe.supportsDynamicRegistration ? ( + + ) : ( +
+ This server requires OAuth, but its authorization server does not support + dynamic client registration. Use request headers with a bearer token, or + save the source and connect a supported OAuth connection later. +
+ ))} {!tokens && state.step === "oauth-starting" && (
diff --git a/packages/plugins/mcp/src/sdk/plugin.ts b/packages/plugins/mcp/src/sdk/plugin.ts index 4e1fe0846..77168c25d 100644 --- a/packages/plugins/mcp/src/sdk/plugin.ts +++ b/packages/plugins/mcp/src/sdk/plugin.ts @@ -122,6 +122,7 @@ export type McpSourceConfig = McpRemoteSourceConfig | McpStdioSourceConfig; export interface McpProbeResult { readonly connected: boolean; readonly requiresOAuth: boolean; + readonly supportsDynamicRegistration: boolean; readonly name: string; readonly namespace: string; readonly toolCount: number | null; @@ -1019,6 +1020,7 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => { return { connected: true, requiresOAuth: false, + supportsDynamicRegistration: false, name: result.manifest.server?.name ?? name, namespace, toolCount: result.manifest.tools.length, @@ -1054,15 +1056,16 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => { queryParams: probeQueryParams, }) .pipe( - Effect.map(() => true), - Effect.catch(() => Effect.succeed(false)), + Effect.map((oauth) => ({ ok: true as const, oauth })), + Effect.catch(() => Effect.succeed({ ok: false as const, oauth: null })), Effect.withSpan("mcp.plugin.probe_oauth"), ); - if (probeResult) { + if (probeResult.ok) { return { connected: false, requiresOAuth: true, + supportsDynamicRegistration: probeResult.oauth.supportsDynamicRegistration, name, namespace, toolCount: null, diff --git a/packages/plugins/openapi/src/sdk/parse.test.ts b/packages/plugins/openapi/src/sdk/parse.test.ts index c48248d73..e2906bc0f 100644 --- a/packages/plugins/openapi/src/sdk/parse.test.ts +++ b/packages/plugins/openapi/src/sdk/parse.test.ts @@ -33,6 +33,20 @@ paths: {} }), ); + it.effect("falls back to YAML for flow-style YAML documents", () => + Effect.gen(function* () { + const doc = yield* parse(` +{ + openapi: 3.0.0, + info: { title: Test, version: 1.0.0 }, + paths: {} +} +`); + + expect(doc.openapi).toBe("3.0.0"); + }), + ); + it.effect("returns a stable parse error for empty documents", () => Effect.gen(function* () { const error = yield* parse("").pipe(Effect.flip); diff --git a/packages/plugins/openapi/src/sdk/parse.ts b/packages/plugins/openapi/src/sdk/parse.ts index 906be813b..69d4902cb 100644 --- a/packages/plugins/openapi/src/sdk/parse.ts +++ b/packages/plugins/openapi/src/sdk/parse.ts @@ -1,5 +1,5 @@ import type { OpenAPI, OpenAPIV3, OpenAPIV3_1 } from "openapi-types"; -import { Duration, Effect } from "effect"; +import { Duration, Effect, Schema } from "effect"; import { HttpClient, HttpClientRequest } from "effect/unstable/http"; import YAML from "yaml"; @@ -107,13 +107,14 @@ const parseTextToObject = (text: string): Effect.Effect YAML.parse(trimmed) as unknown, - catch: () => - new OpenApiParseError({ - message: "Failed to parse OpenAPI document", - }), - }); + const parsed = yield* parseJsonLike(trimmed).pipe( + Effect.mapError( + () => + new OpenApiParseError({ + message: "Failed to parse OpenAPI document", + }), + ), + ); if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) { return yield* new OpenApiParseError({ @@ -123,3 +124,14 @@ const parseTextToObject = (text: string): Effect.Effect => { + const parseYaml = Effect.try({ + try: () => YAML.parse(text) as unknown, + catch: () => "YamlParseFailed" as const, + }); + if (!text.startsWith("{") && !text.startsWith("[")) return parseYaml; + return parseJsonText(text).pipe(Effect.catch(() => parseYaml)); +}; diff --git a/packages/react/src/plugins/oauth-sign-in.tsx b/packages/react/src/plugins/oauth-sign-in.tsx index 97bcb9666..6e82a47cf 100644 --- a/packages/react/src/plugins/oauth-sign-in.tsx +++ b/packages/react/src/plugins/oauth-sign-in.tsx @@ -5,7 +5,7 @@ import * as Effect from "effect/Effect"; import * as Exit from "effect/Exit"; import { cancelOAuth, startOAuth } from "../api/atoms"; -import { messageFromUnknown, useReportHandledError } from "../api/error-reporting"; +import { messageFromExit, messageFromUnknown, useReportHandledError } from "../api/error-reporting"; import { openOAuthPopup, reserveOAuthPopup, type OAuthPopupResult } from "../api/oauth-popup"; import { Button } from "../components/button"; import { @@ -48,6 +48,7 @@ export type OAuthAuthorizationStartResult = { class OAuthAuthorizationStartError extends Data.TaggedError("OAuthAuthorizationStartError")<{ readonly cause: unknown; + readonly message: string; }> {} export type StartOAuthAuthorizationInput = { @@ -153,11 +154,15 @@ export function useOAuthPopupFlow< const startExit = await Effect.runPromiseExit( Effect.tryPromise({ try: input.run, - catch: (cause) => new OAuthAuthorizationStartError({ cause }), + catch: (cause) => + new OAuthAuthorizationStartError({ + cause, + message: messageFromUnknown(cause, startErrorMessage ?? "Failed to start sign-in"), + }), }), ); if (Exit.isFailure(startExit)) { - const message = startErrorMessage ?? "Failed to start sign-in"; + const message = messageFromExit(startExit, startErrorMessage ?? "Failed to start sign-in"); reportHandledError(startExit.cause, { surface: "oauth", action: "start", @@ -278,7 +283,11 @@ export function useOAuthPopupFlow< }).then((exit) => Exit.isSuccess(exit) ? exit.value - : Effect.runPromise(Effect.fail(startErrorMessage ?? "Failed to start sign-in")), + : Effect.runPromise( + Effect.fail({ + message: messageFromExit(exit, startErrorMessage ?? "Failed to start sign-in"), + }), + ), ), }); },