fix(core): surface OAuth authorization for SSE MCP servers on 401 - #13050
Merged
Conversation
A 401 from an SSE MCP server never persisted authorizationRequired: the fetch-boundary UnauthorizedError was consumed by EventSource and re-thrown as a status-less SseError, so the instanceof check routed it to markConnectionError and hosts never offered the OAuth connect action. Give the SSE stream request a raw fetch so a 401 fails the connection with the SDK's typed SseError(401), and recognize 401s across transports with a single isMcpUnauthorizedError predicate at every detection site.
Contributor
Greptile SummaryThe PR preserves HTTP 401 status information from legacy SSE MCP connections so OAuth-required state can be surfaced consistently.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| sdk/packages/core/src/extensions/mcp/oauth.ts | Preserves SSE stream response status and centralizes recognition of unauthorized transport errors. |
| sdk/packages/core/src/extensions/mcp/client.ts | Uses the shared unauthorized predicate when persisting connection and operation failures. |
| sdk/packages/core/src/extensions/mcp/oauth.test.ts | Covers unauthorized error shapes and verifies transport-specific handling of passive 401 responses. |
| sdk/packages/core/src/extensions/mcp/client-url.test.ts | Verifies authorization-required persistence for SSE and streamable HTTP while preserving ordinary SSE errors. |
Sequence Diagram
sequenceDiagram
participant Client as MCP Client
participant Transport as SSE Transport
participant Server as MCP Server
participant State as OAuth Status Store
Client->>Transport: Connect
Transport->>Server: SSE stream request
Server-->>Transport: HTTP 401
Transport-->>Client: SseError(code: 401)
Client->>Client: isMcpUnauthorizedError(error)
Client->>State: markAuthorizationRequired(message)
Reviews (2): Last reviewed commit: "Merge branch 'main' into saoudrizwan/mcp..." | Re-trigger Greptile
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issue
Found during desktop v0.0.10 regression testing (MCP OAuth shipped in #12983 / #12984).
Description
Enabling a remote MCP server that uses the SSE (legacy) transport, which is also what a bare
{"url": ...}config entry resolves to, never surfaced the OAuth flow. The connection probe hit the server's 401, but the toggle silently snapped back off with no banner, no error, and no way to authorize.Root cause: for passive connections
createMcpSdkTransportthrows a typedUnauthorizedErrorat the fetch boundary. The streamable HTTP transport propagates that rejection, but the SSE transport runs the stream request insideEventSource, which consumes the thrown error and re-emits it as a status-lessSseError("SSE error: MCP server requires authorization",code: undefined). Theerror instanceof UnauthorizedErrorchecks inclient.tsthen routed the failure tomarkConnectionErrorinstead ofmarkAuthorizationRequired, soauthorizationRequiredwas never persisted and hosts (like the desktop MCP settings view) never rendered the Connect action.The fix keeps 401 handling typed end to end:
createMcpSdkTransportnow passes the SSE transport a raw stream fetch viaeventSourceInit.fetch, so a 401 on the stream request fails the connection with the SDK's ownSseErrorcarryingcode: 401(this also avoids EventSource's reconnect scheduling on thrown fetch errors). The throwing wrapper stays in place for streamable HTTP and for SSE message POSTs, where the typedUnauthorizedErrorpropagates correctly.isMcpUnauthorizedErrorpredicate recognizes a 401 in both shapes (UnauthorizedError, orSseErrorwithcode === 401) and replaces the fourinstanceof UnauthorizedErrorchecks inclient.tsconnect/operation error handling and the check inauthorizeMcpServerOAuth.Test Procedure
SSEClientTransportbuilt bycreateMcpSdkTransportwith an injected fetch returning 401 rejects with anSseErrorthe predicate recognizes; a streamable HTTP transport still rejects with the typedUnauthorizedError; and client-level tests assertauthorizationRequiredis persisted for SSE 401s and typed 401s, while a non-401SseErrorstays a plain connection error.bunx vitest run src/extensions/mcpinsdk/packages/core: 8 files, 70 tests, all passing.tsc --noEmitclean. Full@cline/core test:unit: 1758 passed; the 2 failures are the documented cloud-VM gitinsteadOfartifact and an unrelatedcheckpoint-restorehook timeout that passes in isolation.tauri dev): configuredlinearas a bare-URL server (https://mcp.linear.app/mcp, resolves to SSE legacy) and toggled it on in Settings -> MCP. The probe now persistsauthorizationRequired, the amber "OAuth authorization required" banner with Connect appears, Connect opens Linear's real authorization page in the browser and switches the banner to "Waiting for OAuth authorization" with Cancel, and Cancel reverts it. Before the fix this exact flow ended with the toggle silently flipping back off.Type of Change
Pre-flight Checklist
bun test) and code is formatted and linted (bun run format && bun run lint)Screenshots
SSE (legacy) server toggled on after the fix, probe detects the 401 and offers Connect:
Connect opens the browser authorization flow and shows a cancellable waiting state:
Cancel reverts to the authorization-required state:
Additional Notes
The desktop app needed no changes: its MCP view already renders the banner from
oauthStatus.authorizationRequired, which now gets persisted for SSE servers too.