diff --git a/apps/marketing/src/components/tweet.tsx b/apps/marketing/src/components/tweet.tsx index 021e69924..617e3c573 100644 --- a/apps/marketing/src/components/tweet.tsx +++ b/apps/marketing/src/components/tweet.tsx @@ -1,9 +1,9 @@ -import { Tweet } from 'react-tweet' +import { Tweet } from "react-tweet"; export function TweetEmbed({ id }: { id: string }) { return (
- ) + ); } diff --git a/packages/plugins/graphql/src/sdk/plugin.test.ts b/packages/plugins/graphql/src/sdk/plugin.test.ts index cd0816ba5..64c8fecb8 100644 --- a/packages/plugins/graphql/src/sdk/plugin.test.ts +++ b/packages/plugins/graphql/src/sdk/plugin.test.ts @@ -1084,3 +1084,51 @@ describe("graphqlPlugin", () => { }), ); }); + +describe("graphqlPlugin detect URL-token fallback", () => { + // Port 1 connection-refuses immediately, so introspection always + // fails and the URL-token fallback is the only thing that can + // produce a candidate. + it.effect("returns low-confidence candidate when path has /graphql segment", () => + Effect.gen(function* () { + const executor = yield* createExecutor( + makeTestConfig({ plugins: [graphqlPlugin()] as const }), + ); + const results = yield* executor.sources.detect("http://127.0.0.1:1/api/graphql"); + const gql = results.find((r) => r.kind === "graphql"); + expect(gql).toBeDefined(); + expect(gql?.confidence).toBe("low"); + }), + ); + + it.effect("matches graphql on hostname label", () => + Effect.gen(function* () { + const executor = yield* createExecutor( + makeTestConfig({ plugins: [graphqlPlugin()] as const }), + ); + const results = yield* executor.sources.detect("http://graphql.127.0.0.1.nip.io:1/"); + const gql = results.find((r) => r.kind === "graphql"); + expect(gql?.confidence).toBe("low"); + }), + ); + + it.effect("does not match graphql as a substring", () => + Effect.gen(function* () { + const executor = yield* createExecutor( + makeTestConfig({ plugins: [graphqlPlugin()] as const }), + ); + const results = yield* executor.sources.detect("http://127.0.0.1:1/graphqlite"); + expect(results.find((r) => r.kind === "graphql")).toBeUndefined(); + }), + ); + + it.effect("returns null when no token match and introspection fails", () => + Effect.gen(function* () { + const executor = yield* createExecutor( + makeTestConfig({ plugins: [graphqlPlugin()] as const }), + ); + const results = yield* executor.sources.detect("http://127.0.0.1:1/api/v1"); + expect(results.find((r) => r.kind === "graphql")).toBeUndefined(); + }), + ); +}); diff --git a/packages/plugins/graphql/src/sdk/plugin.ts b/packages/plugins/graphql/src/sdk/plugin.ts index 516a4a43a..47d09af02 100644 --- a/packages/plugins/graphql/src/sdk/plugin.ts +++ b/packages/plugins/graphql/src/sdk/plugin.ts @@ -128,6 +128,16 @@ export interface GraphqlUpdateSourceInput { // Helpers // --------------------------------------------------------------------------- +/** Match `token` as a separator-bounded run inside a URL hostname or path, + * used as a low-confidence detection hint when introspection fails. + * Boundary chars are everything non-alphanumeric, so `/api/graphql`, + * `graphql.example.com`, `graphql-api`, and `graphql_v2` all match while + * `graphqlserver` and `/graphqlite` do not. */ +const urlMatchesToken = (url: URL, token: string): boolean => { + const re = new RegExp(`(?:^|[^a-z0-9])${token}(?:$|[^a-z0-9])`, "i"); + return re.test(url.hostname) || re.test(url.pathname); +}; + /** Derive a namespace from an endpoint URL */ const namespaceFromEndpoint = (endpoint: string): string => { // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: URL construction throws; this helper intentionally falls back to the stable default namespace @@ -1144,16 +1154,34 @@ export const graphqlPlugin = definePlugin((options?: GraphqlPluginOptions) => { Effect.catch(() => Effect.succeed(false)), ); - if (!ok) return null; - const name = namespaceFromEndpoint(trimmed); - return new SourceDetectionResult({ - kind: "graphql", - confidence: "high", - endpoint: trimmed, - name, - namespace: name, - }); + + if (ok) { + return new SourceDetectionResult({ + kind: "graphql", + confidence: "high", + endpoint: trimmed, + name, + namespace: name, + }); + } + + // Low-confidence URL-token fallback. Introspection can fail for + // many reasons (auth, CORS, the endpoint disabled introspection + // in production, transport errors). When the URL itself + // strongly implies GraphQL, surface a candidate so the user + // can still pick it from the detect dropdown. + if (urlMatchesToken(parsed.value, "graphql")) { + return new SourceDetectionResult({ + kind: "graphql", + confidence: "low", + endpoint: trimmed, + name, + namespace: name, + }); + } + + return null; }), routes: () => GraphqlGroup, diff --git a/packages/plugins/mcp/src/react/AddMcpSource.tsx b/packages/plugins/mcp/src/react/AddMcpSource.tsx index 392591999..9fee4cb2d 100644 --- a/packages/plugins/mcp/src/react/AddMcpSource.tsx +++ b/packages/plugins/mcp/src/react/AddMcpSource.tsx @@ -298,6 +298,7 @@ export default function AddMcpSource(props: { const oauth = useOAuthPopupFlow({ popupName: "mcp-oauth", popupBlockedMessage: "OAuth popup was blocked", + detectPopupClosed: false, startErrorMessage: "Failed to start OAuth", }); diff --git a/packages/plugins/mcp/src/react/McpSignInButton.tsx b/packages/plugins/mcp/src/react/McpSignInButton.tsx index 6d01e04f9..2c220ce59 100644 --- a/packages/plugins/mcp/src/react/McpSignInButton.tsx +++ b/packages/plugins/mcp/src/react/McpSignInButton.tsx @@ -72,6 +72,7 @@ export default function McpSignInButton(props: { sourceId: string }) { bindings ?? [], )} isConnected={isConnected} + detectPopupClosed={false} onConnected={async (nextConnectionId) => { await setBinding({ params: { scopeId: userScopeId }, diff --git a/packages/plugins/mcp/src/sdk/plugin.test.ts b/packages/plugins/mcp/src/sdk/plugin.test.ts index 5869ecf0d..10586683d 100644 --- a/packages/plugins/mcp/src/sdk/plugin.test.ts +++ b/packages/plugins/mcp/src/sdk/plugin.test.ts @@ -17,7 +17,7 @@ import { type SecretProvider, } from "@executor-js/sdk"; -import { mcpPlugin } from "./plugin"; +import { mcpPlugin, userFacingProbeMessage } from "./plugin"; import { MCP_OAUTH_CONNECTION_SLOT } from "./types"; import { extractManifestFromListToolsResult, deriveMcpNamespace, joinToolPath } from "./manifest"; import { serveMcpServer } from "../testing"; @@ -863,3 +863,89 @@ describe("MCP destructiveHint → requiresApproval", () => { }), ); }); + +describe("userFacingProbeMessage", () => { + it("turns auth-required into a credentials-asking message", () => { + const message = userFacingProbeMessage({ + kind: "not-mcp", + category: "auth-required", + reason: "401 without Bearer WWW-Authenticate — not an MCP auth challenge", + }); + expect(message).toMatch(/requires authentication/i); + expect(message).toMatch(/credentials/i); + }); + + it("turns wrong-shape into a 'not an MCP server' message", () => { + const message = userFacingProbeMessage({ + kind: "not-mcp", + category: "wrong-shape", + reason: "2xx POST body is not a JSON-RPC envelope", + }); + expect(message).toMatch(/doesn't appear to host an MCP server/i); + }); + + it("turns unreachable into a connectivity message", () => { + const message = userFacingProbeMessage({ + kind: "unreachable", + reason: "ECONNREFUSED", + }); + expect(message).toMatch(/couldn't reach/i); + }); + + it("never surfaces the raw probe reason verbatim", () => { + const reasons = [ + "401 without Bearer WWW-Authenticate — not an MCP auth challenge", + "2xx POST body is not a JSON-RPC envelope", + "GET response is not an SSE stream", + "unexpected status 418 for initialize", + ] as const; + for (const reason of reasons) { + const auth = userFacingProbeMessage({ kind: "not-mcp", category: "auth-required", reason }); + const wrong = userFacingProbeMessage({ kind: "not-mcp", category: "wrong-shape", reason }); + expect(auth).not.toContain(reason); + expect(wrong).not.toContain(reason); + } + }); +}); + +describe("mcpPlugin detect URL-token fallback", () => { + // Port 1 connection-refuses immediately, so wire-shape detection + // returns `unreachable` and the URL-token fallback is the only thing + // that can produce a candidate. + it.effect("returns low-confidence candidate when path has /mcp segment", () => + Effect.gen(function* () { + const executor = yield* createExecutor(makeTestConfig({ plugins: [mcpPlugin()] as const })); + const results = yield* executor.sources.detect("http://127.0.0.1:1/api/mcp"); + const mcp = results.find((r) => r.kind === "mcp"); + expect(mcp).toBeDefined(); + expect(mcp?.confidence).toBe("low"); + }), + ); + + it.effect("matches mcp on hostname label", () => + Effect.gen(function* () { + const executor = yield* createExecutor(makeTestConfig({ plugins: [mcpPlugin()] as const })); + const results = yield* executor.sources.detect("http://mcp.127.0.0.1.nip.io:1/"); + const mcp = results.find((r) => r.kind === "mcp"); + expect(mcp?.confidence).toBe("low"); + }), + ); + + it.effect("does not match mcp as a substring", () => + Effect.gen(function* () { + const executor = yield* createExecutor(makeTestConfig({ plugins: [mcpPlugin()] as const })); + // `/mcpstore` is a substring containing `mcp` but `mcp` is not a + // separator-bounded run, so the URL-token fallback must not fire. + const results = yield* executor.sources.detect("http://127.0.0.1:1/mcpstore"); + expect(results.find((r) => r.kind === "mcp")).toBeUndefined(); + }), + ); + + it.effect("returns null when no token match and no wire-shape match", () => + Effect.gen(function* () { + const executor = yield* createExecutor(makeTestConfig({ plugins: [mcpPlugin()] as const })); + const results = yield* executor.sources.detect("http://127.0.0.1:1/api/v1"); + expect(results.find((r) => r.kind === "mcp")).toBeUndefined(); + }), + ); +}); diff --git a/packages/plugins/mcp/src/sdk/plugin.ts b/packages/plugins/mcp/src/sdk/plugin.ts index 77168c25d..07ba4d1f0 100644 --- a/packages/plugins/mcp/src/sdk/plugin.ts +++ b/packages/plugins/mcp/src/sdk/plugin.ts @@ -42,7 +42,7 @@ import { discoverTools } from "./discover"; import { McpConnectionError, McpInvocationError, McpToolDiscoveryError } from "./errors"; import { invokeMcpTool } from "./invoke"; import { deriveMcpNamespace, type McpToolManifest, type McpToolManifestEntry } from "./manifest"; -import { probeMcpEndpointShape } from "./probe-shape"; +import { probeMcpEndpointShape, type McpShapeProbeResult } from "./probe-shape"; import { MCP_HEADER_AUTH_SLOT, MCP_OAUTH_CLIENT_ID_SLOT, @@ -195,6 +195,34 @@ const toBinding = (entry: McpToolManifestEntry): McpToolBinding => const MCP_PLUGIN_ID = "mcp"; +/** Match `token` as a separator-bounded run inside a URL hostname or path, + * used as a low-confidence detection hint when wire-shape detection fails. + * Boundary chars are everything non-alphanumeric, so `/api/mcp`, + * `mcp.example.com`, `mcp-server`, and `mcp_v1` all match while + * `mcphost.com` and `/mcpstore` do not. */ +const urlMatchesToken = (url: URL, token: string): boolean => { + const re = new RegExp(`(?:^|[^a-z0-9])${token}(?:$|[^a-z0-9])`, "i"); + return re.test(url.hostname) || re.test(url.pathname); +}; + +/** Translate a non-MCP probe outcome into a message a user can act on. + * The technical `reason` (`401 without Bearer WWW-Authenticate — not an + * MCP auth challenge`, etc.) stays in telemetry via the probe span; the + * user gets a sentence pointing at their next step. Exported for tests. */ +export const userFacingProbeMessage = ( + shape: Extract, +): string => { + if (shape.kind === "unreachable") { + return "Couldn't reach this URL. Check the address, your network, and that the server is running."; + } + switch (shape.category) { + case "auth-required": + return "This server requires authentication. Add credentials (Authorization header, query parameter, or API key) below and retry."; + case "wrong-shape": + return "This URL doesn't appear to host an MCP server. Double-check the address, including the path."; + } +}; + const scopeRanks = (ctx: PluginCtx): ReadonlyMap => new Map(ctx.scopes.map((scope, index) => [String(scope.id), index])); @@ -1042,10 +1070,7 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => { if (shape.kind !== "mcp") { return yield* new McpConnectionError({ transport: "remote", - message: - shape.kind === "not-mcp" - ? `Endpoint does not look like an MCP server: ${shape.reason}` - : `Could not reach endpoint: ${shape.reason}`, + message: userFacingProbeMessage(shape), }); } @@ -1075,7 +1100,8 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => { return yield* new McpConnectionError({ transport: "remote", - message: "MCP server requires authentication but OAuth discovery failed", + message: + "This server requires authentication, but OAuth metadata wasn't found. Add credentials (Authorization header, query parameter, or API key) below and retry.", }); }).pipe( Effect.withSpan("mcp.plugin.probe_endpoint", { @@ -1662,30 +1688,41 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => { }); } - // host publishes RFC 9728 + 8414 metadata) would be classified - // as MCP whenever the cross-plugin detector fans out to us. + // The shape probe inspects the JSON-RPC `initialize` response + // and only classifies as MCP when the wire shape is + // unambiguous (2xx + JSON-RPC body, 2xx SSE, or 401 + Bearer + + // JSON-RPC error envelope). That body-shape gate is what + // separates real MCP servers — including those that + // authenticate with static API keys and publish no OAuth + // metadata — from unrelated OAuth-protected services whose + // host happens to expose RFC 9728/8414 documents. const shape = yield* probeMcpEndpointShape(trimmed, { httpClientLayer }); - if (shape.kind !== "mcp") return null; - - // Confirm OAuth metadata is actually reachable. The shape - // probe already found a Bearer challenge; the core OAuth - // service's probe verifies the AS metadata resolves so we - // don't classify endpoints that challenge but have no - // discovery. - const probeOk = yield* ctx.oauth.probe({ endpoint: trimmed }).pipe( - Effect.map(() => true), - Effect.catch(() => Effect.succeed(false)), - Effect.withSpan("mcp.plugin.probe_oauth"), - ); - if (!probeOk) return null; + if (shape.kind === "mcp") { + return new SourceDetectionResult({ + kind: "mcp", + confidence: "high", + endpoint: trimmed, + name, + namespace, + }); + } - return new SourceDetectionResult({ - kind: "mcp", - confidence: "high", - endpoint: trimmed, - name, - namespace, - }); + // Low-confidence URL-token fallback. When wire-shape detection + // can't confirm MCP (server unreachable, behind unusual auth, + // returns a non-canonical body, etc.) but the URL itself is a + // strong hint, surface a candidate so the user can still pick + // it from the detect dropdown rather than getting nothing. + if (urlMatchesToken(parsed.value, "mcp")) { + return new SourceDetectionResult({ + kind: "mcp", + confidence: "low", + endpoint: trimmed, + name, + namespace, + }); + } + + return null; }).pipe( Effect.catch(() => Effect.succeed(null)), Effect.withSpan("mcp.plugin.detect", { diff --git a/packages/plugins/mcp/src/sdk/probe-shape.test.ts b/packages/plugins/mcp/src/sdk/probe-shape.test.ts index 2bf27e486..fd4d9abd4 100644 --- a/packages/plugins/mcp/src/sdk/probe-shape.test.ts +++ b/packages/plugins/mcp/src/sdk/probe-shape.test.ts @@ -70,16 +70,45 @@ describe("probeMcpEndpointShape", () => { ), ); - it.effect("classifies 401 with Bearer WWW-Authenticate as MCP+OAuth", () => + it.effect("classifies 401 with Bearer + JSON-RPC error envelope as MCP+auth", () => withServer( () => - HttpServerResponse.empty({ - status: 401, - headers: { - "www-authenticate": - 'Bearer resource_metadata="https://mcp.example/.well-known/oauth-protected-resource"', + HttpServerResponse.jsonUnsafe( + { + jsonrpc: "2.0", + id: null, + error: { code: -32000, message: "Unauthorized" }, + }, + { + status: 401, + headers: { + "www-authenticate": + 'Bearer resource_metadata="https://mcp.example/.well-known/oauth-protected-resource"', + }, }, + ), + (endpoint) => + Effect.gen(function* () { + const result = yield* probeMcpEndpointShape(endpoint); + expect(result).toEqual({ kind: "mcp", requiresAuth: true }); }), + ), + ); + + // cubic.dev/api/mcp shape: bare `Bearer` challenge, no resource_metadata. + // The JSON-RPC error body is what tells us this is MCP rather than some + // other OAuth/API-key protected service. + it.effect("classifies 401 with bare Bearer + JSON-RPC error as MCP+auth", () => + withServer( + () => + HttpServerResponse.jsonUnsafe( + { + jsonrpc: "2.0", + id: null, + error: { code: -32000, message: "Unauthorized: Valid API key required." }, + }, + { status: 401, headers: { "www-authenticate": "Bearer" } }, + ), (endpoint) => Effect.gen(function* () { const result = yield* probeMcpEndpointShape(endpoint); @@ -88,13 +117,46 @@ describe("probeMcpEndpointShape", () => { ), ); - it.effect("rejects 401 without WWW-Authenticate as non-MCP", () => + it.effect("rejects 401 without WWW-Authenticate as auth-required", () => withServer( () => HttpServerResponse.text("nope", { status: 401 }), (endpoint) => Effect.gen(function* () { const result = yield* probeMcpEndpointShape(endpoint); - expect(result.kind).toBe("not-mcp"); + expect(result).toMatchObject({ kind: "not-mcp", category: "auth-required" }); + }), + ), + ); + + it.effect("rejects 401 + Bearer with empty body as auth-required", () => + withServer( + () => + HttpServerResponse.empty({ + status: 401, + headers: { "www-authenticate": "Bearer" }, + }), + (endpoint) => + Effect.gen(function* () { + const result = yield* probeMcpEndpointShape(endpoint); + expect(result).toMatchObject({ kind: "not-mcp", category: "auth-required" }); + }), + ), + ); + + // Railway-style: OAuth-protected GraphQL endpoint that returns a Bearer + // challenge but a non-JSON-RPC error envelope. Must NOT be classified as + // MCP — otherwise we misclassify any OAuth-protected non-MCP service. + it.effect("rejects 401 + Bearer with GraphQL-shape body as auth-required", () => + withServer( + () => + HttpServerResponse.jsonUnsafe( + { errors: [{ message: "Unauthorized" }] }, + { status: 401, headers: { "www-authenticate": "Bearer" } }, + ), + (endpoint) => + Effect.gen(function* () { + const result = yield* probeMcpEndpointShape(endpoint); + expect(result).toMatchObject({ kind: "not-mcp", category: "auth-required" }); }), ), ); @@ -105,13 +167,20 @@ describe("probeMcpEndpointShape", () => { if (request.method === "POST") { return HttpServerResponse.empty({ status: 405 }); } - return HttpServerResponse.empty({ - status: 401, - headers: { - "www-authenticate": - 'Bearer resource_metadata="https://mcp.example/.well-known/oauth-protected-resource"', + return HttpServerResponse.jsonUnsafe( + { + jsonrpc: "2.0", + id: null, + error: { code: -32000, message: "Unauthorized" }, }, - }); + { + status: 401, + headers: { + "www-authenticate": + 'Bearer resource_metadata="https://mcp.example/.well-known/oauth-protected-resource"', + }, + }, + ); }, (endpoint) => Effect.gen(function* () { @@ -140,7 +209,32 @@ describe("probeMcpEndpointShape", () => { ), ); - it.effect("rejects 400 GraphQL-shape responses as non-MCP", () => + it.effect("rejects 2xx with non-JSON-RPC JSON body as wrong-shape", () => + withServer( + () => HttpServerResponse.jsonUnsafe({ ok: true, data: { id: "x" } }), + (endpoint) => + Effect.gen(function* () { + const result = yield* probeMcpEndpointShape(endpoint); + expect(result).toMatchObject({ kind: "not-mcp", category: "wrong-shape" }); + }), + ), + ); + + it.effect("rejects 2xx with HTML body as wrong-shape", () => + withServer( + () => + HttpServerResponse.text("", { + contentType: "text/html", + }), + (endpoint) => + Effect.gen(function* () { + const result = yield* probeMcpEndpointShape(endpoint); + expect(result).toMatchObject({ kind: "not-mcp", category: "wrong-shape" }); + }), + ), + ); + + it.effect("rejects 400 GraphQL-shape responses as wrong-shape", () => withServer( () => HttpServerResponse.jsonUnsafe( @@ -150,18 +244,18 @@ describe("probeMcpEndpointShape", () => { (endpoint) => Effect.gen(function* () { const result = yield* probeMcpEndpointShape(endpoint); - expect(result.kind).toBe("not-mcp"); + expect(result).toMatchObject({ kind: "not-mcp", category: "wrong-shape" }); }), ), ); - it.effect("rejects 404 as non-MCP", () => + it.effect("rejects 404 as wrong-shape", () => withServer( () => HttpServerResponse.empty({ status: 404 }), (endpoint) => Effect.gen(function* () { const result = yield* probeMcpEndpointShape(endpoint); - expect(result.kind).toBe("not-mcp"); + expect(result).toMatchObject({ kind: "not-mcp", category: "wrong-shape" }); }), ), ); diff --git a/packages/plugins/mcp/src/sdk/probe-shape.ts b/packages/plugins/mcp/src/sdk/probe-shape.ts index 79869151c..51888cd84 100644 --- a/packages/plugins/mcp/src/sdk/probe-shape.ts +++ b/packages/plugins/mcp/src/sdk/probe-shape.ts @@ -6,27 +6,31 @@ // // `discoverTools` (via the MCP SDK's StreamableHTTP / SSE transport) // fails for every non-MCP endpoint too — 200-with-HTML, 400-GraphQL, -// 404, etc. all surface as the same opaque transport error. When -// `discoverTools` fails, plugin.ts falls through to -// `startMcpOAuthAuthorization`, which succeeds against *any* URL whose -// origin publishes OAuth 2.0 Protected Resource + Authorization Server -// Metadata (RFC 9728 + RFC 8414) — that's what the MCP SDK's `auth()` -// consumes. Plenty of non-MCP APIs (Railway's `backboard.railway.com/ -// graphql/v2`, anything backed by a standards-compliant OAuth AS) -// publish that metadata, so the fall-through misclassifies them. +// 404, etc. all surface as the same opaque transport error. We need +// our own classifier that distinguishes "real MCP" from +// "OAuth-protected non-MCP service" without relying on RFC 9728/8414 +// metadata, since (a) plenty of non-MCP APIs publish that metadata, +// and (b) plenty of real MCP servers authenticate with static API +// keys and publish no OAuth metadata at all (e.g. cubic.dev). // -// The MCP authorization spec (`modelcontextprotocol.io/specification/ -// draft/basic/authorization`) mandates the handshake that distinguishes -// a real MCP-requires-OAuth endpoint from the general case: +// The probe issues an unauth JSON-RPC `initialize` POST and accepts +// only the wire shapes a real MCP server can return: // -// - On an unauthenticated request the server MUST respond `401` and -// include `WWW-Authenticate: Bearer` with a `resource_metadata=` -// attribute pointing at its RFC 9728 document. +// - 2xx with `Content-Type: text/event-stream` — streamable HTTP +// transport, body is an SSE stream we don't consume. +// - 2xx with `Content-Type: application/json` whose body parses as a +// JSON-RPC 2.0 envelope (`{jsonrpc:"2.0", result|error|method,...}`). +// - 401 with `WWW-Authenticate: Bearer` AND a JSON-RPC error envelope +// in the body. The body shape is what separates a real MCP server +// from an unrelated OAuth-protected API: GraphQL/REST/HTML 401s +// don't shape themselves as JSON-RPC. // -// This module issues an unauth JSON-RPC `initialize` POST and checks -// exactly that shape. That's enough to separate "MCP server that needs -// OAuth" from "non-MCP service whose host happens to publish OAuth -// metadata". It's a single `fetch`, no MCP-SDK session state, no OAuth +// When POST returns 404/405/406/415 we retry with GET + `Accept: +// text/event-stream` to support legacy SSE-only servers; that path +// only accepts 2xx with `text/event-stream` or the same 401+Bearer +// shape. +// +// One `fetch` (occasionally two), no MCP-SDK session state, no OAuth // round-trip, no DCR — every non-MCP endpoint exits here. // --------------------------------------------------------------------------- @@ -68,6 +72,24 @@ class ProbeTransportError extends Data.TaggedError("ProbeTransportError")<{ readonly cause: unknown; }> {} +/** Quick check that a body parses as a JSON-RPC 2.0 envelope. The MCP wire + * protocol is JSON-RPC 2.0, so a real MCP server's response to `initialize` + * (whether 2xx with the result, or a 401 error envelope) carries this + * shape. Non-MCP services don't — GraphQL APIs return `{errors:[...]}`, + * REST APIs return their own envelope, marketing pages return HTML. */ +const decodeJsonString = Schema.decodeUnknownOption(Schema.fromJsonString(Schema.Unknown)); + +const isJsonRpcEnvelope = (body: string): boolean => { + if (!body) return false; + const parsed = decodeJsonString(body); + if (Option.isNone(parsed)) return false; + const value = parsed.value; + if (typeof value !== "object" || value === null || Array.isArray(value)) return false; + const obj = value as Record; + if (obj.jsonrpc !== "2.0") return false; + return "result" in obj || "error" in obj || "method" in obj; +}; + const ErrorMessageShape = Schema.Struct({ message: Schema.String }); const decodeErrorMessageShape = Schema.decodeUnknownOption(ErrorMessageShape); @@ -84,13 +106,31 @@ const reasonFromBoundaryCause = (cause: unknown): string => { return "fetch failed"; }; +/** Why the probe rejected an endpoint as not-MCP. + * + * - `auth-required` — server returned 401. We don't know for sure it's + * an MCP server (no spec-compliant Bearer challenge or the body + * isn't JSON-RPC), but the right next step for the user is the same + * either way: provide credentials and retry. This is what + * misclassifies real MCP servers like cubic.dev (no + * resource_metadata) or ref.tools (no WWW-Authenticate at all) + * without the URL-token fallback at the detect layer. + * - `wrong-shape` — endpoint responded but with a body or status that + * doesn't match any MCP shape (200 HTML, 400 GraphQL, 404 from a + * static host, etc.). User action: this URL probably isn't MCP. */ +export type McpProbeRejectCategory = "auth-required" | "wrong-shape"; + export type McpShapeProbeResult = /** Server answered initialize successfully — either a 2xx with a * JSON-RPC payload, or a 401 + WWW-Authenticate: Bearer (RFC 6750 * challenge) that the MCP auth spec requires. */ | { readonly kind: "mcp"; readonly requiresAuth: boolean } /** Endpoint is reachable but the response does not look like MCP. */ - | { readonly kind: "not-mcp"; readonly reason: string } + | { + readonly kind: "not-mcp"; + readonly reason: string; + readonly category: McpProbeRejectCategory; + } /** Transport-level failure (DNS, TLS, timeout, abort, ...). */ | { readonly kind: "unreachable"; readonly reason: string }; @@ -121,36 +161,77 @@ export const probeMcpEndpointShape = ( const timeoutMs = options.timeoutMs ?? 8_000; const outcome = yield* Effect.gen(function* () { const client = yield* HttpClient.HttpClient; + + const readBody = (response: { + readonly text: Effect.Effect; + }): Effect.Effect => + response.text.pipe( + Effect.timeout(Duration.millis(timeoutMs)), + Effect.catch(() => Effect.succeed("")), + ); + const classify = ( - response: { readonly status: number; readonly headers: Readonly> }, + response: { + readonly status: number; + readonly headers: Readonly>; + readonly text: Effect.Effect; + }, method: "GET" | "POST", - ) => { - if (response.status === 401) { - const wwwAuth = readHeader(response.headers, "www-authenticate"); - if (wwwAuth && /^\s*bearer\b/i.test(wwwAuth)) { + ): Effect.Effect => + Effect.gen(function* () { + const contentType = readHeader(response.headers, "content-type") ?? ""; + const isSse = /^\s*text\/event-stream\b/i.test(contentType); + + if (response.status === 401) { + const wwwAuth = readHeader(response.headers, "www-authenticate"); + if (!wwwAuth || !/^\s*bearer\b/i.test(wwwAuth)) { + return { + kind: "not-mcp", + category: "auth-required", + reason: "401 without Bearer WWW-Authenticate — not an MCP auth challenge", + } as const; + } + // SSE responses can't carry a JSON-RPC error envelope; accept the + // Bearer challenge alone in that case (rare but spec-permissible). + if (isSse) return { kind: "mcp", requiresAuth: true } as const; + const body = yield* readBody(response); + if (!isJsonRpcEnvelope(body)) { + return { + kind: "not-mcp", + category: "auth-required", + reason: "401 + Bearer but body is not a JSON-RPC envelope", + } as const; + } return { kind: "mcp", requiresAuth: true } as const; } - return { - kind: "not-mcp", - reason: "401 without Bearer WWW-Authenticate — not an MCP auth challenge", - } as const; - } - if (response.status >= 200 && response.status < 300) { - if (method === "GET") { - const contentType = readHeader(response.headers, "content-type") ?? ""; - if (!/^\s*text\/event-stream\b/i.test(contentType)) { + if (response.status >= 200 && response.status < 300) { + if (method === "GET") { + if (!isSse) { + return { + kind: "not-mcp", + category: "wrong-shape", + reason: "GET response is not an SSE stream", + } as const; + } + return { kind: "mcp", requiresAuth: false } as const; + } + // POST 2xx: SSE body is opaque to us; otherwise require a + // JSON-RPC envelope so we don't accept HTML/REST 200 responses. + if (isSse) return { kind: "mcp", requiresAuth: false } as const; + const body = yield* readBody(response); + if (!isJsonRpcEnvelope(body)) { return { kind: "not-mcp", - reason: "GET response is not an SSE stream", + category: "wrong-shape", + reason: "2xx POST body is not a JSON-RPC envelope", } as const; } + return { kind: "mcp", requiresAuth: false } as const; } - return { kind: "mcp", requiresAuth: false } as const; - } - return null; - }; + return null; + }); const url = new URL(endpoint); for (const [key, value] of Object.entries(options.queryParams ?? {})) { @@ -170,7 +251,7 @@ export const probeMcpEndpointShape = ( .execute(postRequest) .pipe(Effect.timeout(Duration.millis(timeoutMs))); - const postResult = classify(postResponse, "POST"); + const postResult = yield* classify(postResponse, "POST"); if (postResult) return postResult; if ([404, 405, 406, 415].includes(postResponse.status)) { @@ -183,12 +264,13 @@ export const probeMcpEndpointShape = ( const getResponse = yield* client .execute(getRequest) .pipe(Effect.timeout(Duration.millis(timeoutMs))); - const getResult = classify(getResponse, "GET"); + const getResult = yield* classify(getResponse, "GET"); if (getResult) return getResult; } return { kind: "not-mcp", + category: "wrong-shape", reason: `unexpected status ${postResponse.status} for initialize`, } as const; }).pipe( diff --git a/packages/react/src/api/oauth-popup.test.ts b/packages/react/src/api/oauth-popup.test.ts index 9fd6d9c10..396d1b276 100644 --- a/packages/react/src/api/oauth-popup.test.ts +++ b/packages/react/src/api/oauth-popup.test.ts @@ -171,4 +171,60 @@ describe("openOAuthPopup", () => { expect(opened).toBe("about:blank"); expect(popup.location.href).toBe("http://example.com/authorize"); }); + + it("can disable popup.closed polling for providers with strict opener policies", () => { + let closedCalled = false; + let intervalStarted = false; + const popup: FakePopup = { closed: true, close: () => {}, location: { href: "" } }; + const previousWindow = globalThis.window; + const previousSetInterval = globalThis.setInterval; + const fakeWindow: OAuthPopupTestWindow = { + screenX: 0, + screenY: 0, + outerWidth: 1200, + outerHeight: 900, + location: { origin: "https://app.example" }, + addEventListener: () => {}, + removeEventListener: () => {}, + open: () => popup, + }; + Object.defineProperty(globalThis, "window", { + configurable: true, + value: fakeWindow, + writable: true, + }); + Object.defineProperty(globalThis, "setInterval", { + configurable: true, + value: () => { + intervalStarted = true; + return 1; + }, + writable: true, + }); + + const teardown = openOAuthPopup({ + url: "https://auth.example/authorize", + popupName: "oauth", + channelName: "oauth-channel", + closedPollMs: null, + onResult: () => {}, + onClosed: () => { + closedCalled = true; + }, + }); + + teardown(); + Object.defineProperty(globalThis, "window", { + configurable: true, + value: previousWindow, + writable: true, + }); + Object.defineProperty(globalThis, "setInterval", { + configurable: true, + value: previousSetInterval, + writable: true, + }); + expect(intervalStarted).toBe(false); + expect(closedCalled).toBe(false); + }); }); diff --git a/packages/react/src/api/oauth-popup.ts b/packages/react/src/api/oauth-popup.ts index ab3c9c744..f0dd85751 100644 --- a/packages/react/src/api/oauth-popup.ts +++ b/packages/react/src/api/oauth-popup.ts @@ -43,8 +43,8 @@ export type OpenOAuthPopupInput = { readonly onClosed?: () => void; readonly width?: number; readonly height?: number; - /** How often to poll `popup.closed`. Default 500ms. */ - readonly closedPollMs?: number; + /** How often to poll `popup.closed`. Default 500ms. Set to null to disable. */ + readonly closedPollMs?: number | null; }; export type ReservedOAuthPopup = { @@ -91,7 +91,8 @@ export const reserveOAuthPopup = (input: { * * Settles exactly once via one of three paths: * 1. `onResult` — popup posted a message back (success or error) - * 2. `onClosed` — user closed the popup without completing the flow + * 2. `onClosed` — user closed the popup without completing the flow, + * unless `closedPollMs` is null * 3. teardown called — caller cancelled programmatically * * If the popup is blocked (`window.open` returns null), invokes @@ -175,23 +176,25 @@ export const openOAuthPopup = (input: OpenOAuthPopupInput): (() => return () => {}; } - // Poll for manual popup close. We only settle via onClosed if no - // message-based result has arrived; onResult settles first and - // stops the poll before we see the close. - const pollMs = input.closedPollMs ?? 500; - pollHandle = setInterval(() => { - let isClosed = false; - // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: browser popup.closed can throw while navigating cross-origin - try { - isClosed = popup.closed; - } catch { - // Cross-origin access can throw during navigation; treat as open. - } - if (isClosed && !settled) { - settle(); - input.onClosed?.(); - } - }, pollMs); + // Some providers use COOP headers that can make a live cross-origin + // popup look closed to the opener. Callers can disable polling and rely + // on the explicit cancel path plus BroadcastChannel completion. + const pollMs = input.closedPollMs === undefined ? 500 : input.closedPollMs; + if (pollMs !== null) { + pollHandle = setInterval(() => { + let isClosed = false; + // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: browser popup.closed can throw while navigating cross-origin + try { + isClosed = popup.closed; + } catch { + // Cross-origin access can throw during navigation; treat as open. + } + if (isClosed && !settled) { + settle(); + input.onClosed?.(); + } + }, pollMs); + } return () => { if (settled) return; diff --git a/packages/react/src/plugins/oauth-sign-in.tsx b/packages/react/src/plugins/oauth-sign-in.tsx index 6e82a47cf..2cb2db1c2 100644 --- a/packages/react/src/plugins/oauth-sign-in.tsx +++ b/packages/react/src/plugins/oauth-sign-in.tsx @@ -87,10 +87,12 @@ export function useOAuthPopupFlow< readonly noAuthorizationUrlMessage?: string; readonly popupBlockedMessage?: string; readonly popupClosedMessage?: string; + readonly detectPopupClosed?: boolean; readonly startErrorMessage?: string; }) { const { callbackPath, + detectPopupClosed = true, noAuthorizationUrlMessage, popupBlockedMessage, popupClosedMessage, @@ -197,6 +199,7 @@ export function useOAuthPopupFlow< channelName: OAUTH_POPUP_MESSAGE_TYPE, expectedSessionId: response.sessionId, reservedPopup, + closedPollMs: detectPopupClosed ? undefined : null, onResult: async (result: OAuthPopupResult) => { cleanupRef.current = null; sessionRef.current = null; @@ -230,7 +233,9 @@ export function useOAuthPopupFlow< onClosed: () => { cleanupRef.current = null; sessionRef.current = null; - cancelSession(response.sessionId, input.tokenScope); + // `popup.closed` is advisory: COOP redirects can make a live popup + // appear closed to the opener. Keep server OAuth state alive for a + // callback or TTL cleanup; only explicit cancel deletes the session. const message = popupClosedMessage ?? "Sign-in cancelled - popup was closed before completing the flow."; @@ -252,6 +257,7 @@ export function useOAuthPopupFlow< [ cancel, cancelSession, + detectPopupClosed, noAuthorizationUrlMessage, popupBlockedMessage, popupClosedMessage, @@ -343,11 +349,13 @@ export function SourceOAuthSignInButton(props: { readonly queryParams?: Record; readonly isConnected: boolean; readonly onConnected: (connectionId: ConnectionId) => void | Promise; + readonly detectPopupClosed?: boolean; readonly reconnectingLabel?: string; readonly signingInLabel?: string; }) { const { connectionId, + detectPopupClosed, endpoint, fallbackNamespace, headers, @@ -364,6 +372,7 @@ export function SourceOAuthSignInButton(props: { } = props; const oauth = useOAuthPopupFlow({ popupName, + detectPopupClosed, }); const handleSignIn = useCallback(async () => {