mcp: surface auth-gated failures

[gjermundgaraba]

I noticed after trying to add the official excalidraw mcp server that a basic 401 error (non-compliant mcp error (no Bearer WWW-Authenticate, no RFC 9728 metadata)), would not let me add the server at all and progress to the steps needed to configure the auth headers.

The fix applied here attempts to solve this by handling non-mcp-spec compliant auth failures the same way as compliant ones.

If you don't want to handle non-mcp-spec compliant auth failures like this, feel free to disregard the PR.

Additional note, during testing and review of the changes, I also found another issue that is folded in here for tool invocation: call-time 401/403 responses (and OAuth reauthorization) are now classified as actionable auth failures ("re-authenticate / update the connection") rather than generic tool errors, while ensuring raw upstream error bodies are never leaked into user-facing messages.

[greptile-apps]

Greptile Summary

This PR improves how auth-gated MCP connection failures surface to users across two distinct scenarios: during server probing (non-spec-compliant 401 with no Bearer challenge now routes to the auth-method editor instead of a dead-end error), and during tool invocation (call-time 401/403 HTTP responses are now classified as actionable auth failures rather than generic errors, and OAuth reauth signals propagate cleanly).

• McpOAuthReauthorizationRequired is introduced as a Data.TaggedError to thread OAuth reauth signals through the connection and invocation layers without leaking upstream error bodies; the SSE fallback in createMcpConnector is updated to skip the SSE retry on this error.
• McpInvocationError is changed from Schema.TaggedErrorClass to Data.TaggedError (internal-only; removed from the barrel export), gains an optional status field sourced exclusively from StreamableHTTPError.code or the SSE POST-error prefix, and is caught in plugin.ts to route 401/403 to mcpInvocationAuthFailure while re-raising other statuses.
• New unit tests in invoke.test.ts and plugin.test.ts cover status extraction, info-leak prevention, and the non-spec-compliant 401 probe path; a new selfhost e2e test guards the "no dead-end" flow end-to-end.

Confidence Score: 5/5

Safe to merge; changes are well-scoped, thoroughly tested, and the new auth-failure paths are all guarded by unit tests with explicit do-not-leak sentinels.

The probe and invocation changes are independently gated: the not-mcp/auth-required early return can't be triggered for a reachable, spec-compliant MCP server (those produce kind: mcp), and the McpInvocationError 401/403 branch only fires when the HTTP transport layer specifically surfaces those codes. The SSE status extraction has a documented graceful-degradation path (regex mismatch → undefined → generic error, no crash). Tests cover the sanitization invariants, the transport-fallback skip for OAuth reauth, and the end-to-end browser flow.

No files require special attention, though errors.ts is worth a second look because McpInvocationError lost its Schema.TaggedErrorClass HTTP API annotation — the previous review thread has the details.

Important Files Changed

Filename	Overview
packages/plugins/mcp/src/sdk/errors.ts	McpInvocationError changed from Schema.TaggedErrorClass to Data.TaggedError (loses httpApiStatus annotation) and gains an optional status field; McpOAuthReauthorizationRequired added as a new Data.TaggedError; both are now internal-only.
packages/plugins/mcp/src/sdk/connection.ts	connectionFailure helper added to detect McpOAuthReauthorizationRequired in the catch block; auto-transport fallback updated to propagate OAuth reauth instead of retrying SSE; McpConnector type extended with the new error union.
packages/plugins/mcp/src/sdk/invoke.ts	Adds HTTP status extraction from StreamableHTTPError and the SSE POST-error prefix message, sanitizes upstream bodies from McpInvocationError, and propagates McpOAuthReauthorizationRequired from callTool rejections.
packages/plugins/mcp/src/sdk/plugin.ts	probeEndpoint now treats not-mcp/auth-required as requiresAuthentication:true instead of a hard stop; invokeTool pipe gains catchTags for McpOAuthReauthorizationRequired and McpInvocationError with 401/403 routing; userFacingProbeMessage simplified to remove the auth-required branch.
packages/plugins/mcp/src/sdk/index.ts	McpInvocationError removed from the barrel export to reflect its internal-only status; McpOAuthReauthorizationRequired intentionally kept private.
packages/plugins/mcp/src/sdk/invoke.test.ts	New test file; covers status extraction, info-leak prevention (do-not-leak sentinel), OAuth reauth propagation through both fake connector and real end-to-end server paths.
packages/plugins/mcp/src/sdk/plugin.test.ts	Adds fixtures and tests for call-time 401/403 auth failures, non-auth 500 wrapping, JSON-RPC code vs HTTP status distinction, and the non-spec-compliant 401 probe path.
e2e/selfhost/mcp-auth-required-add.test.ts	New selfhost-only e2e regression guard; verifies that a non-spec-compliant 401 server routes the UI to the auth-method editor, seeds a detected Bearer-header method, and allows adding the source and connecting via API key.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant UI
    participant Plugin as plugin.ts
    participant Probe as probeMcpEndpointShape
    participant Connector as createMcpConnector
    participant SDK as MCP SDK Client

    Note over UI, SDK: Probe flow (add-MCP)
    UI->>Plugin: probeEndpoint(url)
    Plugin->>Connector: discoverTools
    Connector->>SDK: client.connect()
    SDK-->>Connector: reject
    Plugin->>Probe: probeMcpEndpointShape(url)

    alt "shape = unreachable"
        Plugin-->>UI: McpConnectionError (hard stop)
    else "shape = not-mcp / wrong-shape"
        Plugin-->>UI: McpConnectionError (hard stop)
    else "shape = not-mcp / auth-required (NEW)"
        Plugin-->>UI: requiresAuthentication:true, requiresOAuth:false
        Note over UI: Auth-method editor rendered
    else "shape = mcp"
        Plugin-->>UI: requiresOAuth:true
    end

    Note over UI, SDK: Tool invocation flow
    UI->>Plugin: invokeTool(credential, toolName, args)
    Plugin->>Connector: createMcpConnector(headers/token)
    Connector->>SDK: client.connect()

    alt connect OAuth reauth
        SDK-->>Plugin: McpOAuthReauthorizationRequired
        Plugin-->>UI: authToolFailure(oauth_reauth_required)
    else connect failure
        SDK-->>Plugin: McpConnectionError
        Plugin-->>UI: authToolFailure(connection_rejected)
    else connect success
        Plugin->>SDK: client.callTool(name, args)
        alt HTTP 401 or 403
            SDK-->>Plugin: StreamableHTTPError or SSE POST error
            Plugin-->>UI: authToolFailure(connection_rejected, status)
        else OAuth reauth
            SDK-->>Plugin: McpOAuthReauthorizationRequired
            Plugin-->>UI: authToolFailure(oauth_reauth_required)
        else other error
            SDK-->>Plugin: McpInvocationError
            Plugin-->>UI: ToolInvocationError
        else success
            SDK-->>Plugin: tool result
            Plugin-->>UI: ToolResult.ok
        end
    end

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant UI
    participant Plugin as plugin.ts
    participant Probe as probeMcpEndpointShape
    participant Connector as createMcpConnector
    participant SDK as MCP SDK Client

    Note over UI, SDK: Probe flow (add-MCP)
    UI->>Plugin: probeEndpoint(url)
    Plugin->>Connector: discoverTools
    Connector->>SDK: client.connect()
    SDK-->>Connector: reject
    Plugin->>Probe: probeMcpEndpointShape(url)

    alt "shape = unreachable"
        Plugin-->>UI: McpConnectionError (hard stop)
    else "shape = not-mcp / wrong-shape"
        Plugin-->>UI: McpConnectionError (hard stop)
    else "shape = not-mcp / auth-required (NEW)"
        Plugin-->>UI: requiresAuthentication:true, requiresOAuth:false
        Note over UI: Auth-method editor rendered
    else "shape = mcp"
        Plugin-->>UI: requiresOAuth:true
    end

    Note over UI, SDK: Tool invocation flow
    UI->>Plugin: invokeTool(credential, toolName, args)
    Plugin->>Connector: createMcpConnector(headers/token)
    Connector->>SDK: client.connect()

    alt connect OAuth reauth
        SDK-->>Plugin: McpOAuthReauthorizationRequired
        Plugin-->>UI: authToolFailure(oauth_reauth_required)
    else connect failure
        SDK-->>Plugin: McpConnectionError
        Plugin-->>UI: authToolFailure(connection_rejected)
    else connect success
        Plugin->>SDK: client.callTool(name, args)
        alt HTTP 401 or 403
            SDK-->>Plugin: StreamableHTTPError or SSE POST error
            Plugin-->>UI: authToolFailure(connection_rejected, status)
        else OAuth reauth
            SDK-->>Plugin: McpOAuthReauthorizationRequired
            Plugin-->>UI: authToolFailure(oauth_reauth_required)
        else other error
            SDK-->>Plugin: McpInvocationError
            Plugin-->>UI: ToolInvocationError
        else success
            SDK-->>Plugin: tool result
            Plugin-->>UI: ToolResult.ok
        end
    end

Loading

_{Reviews (2): Last reviewed commit: "pr fixes" | Re-trigger Greptile}

[greptile-apps]

McpInvocationError is still exported from index.ts but has been changed from Schema.TaggedErrorClass (which carries httpApiStatus: 400 and is compatible with .addError() on HTTP API groups) to Data.TaggedError (no schema annotations). Any downstream consumer that used the old class as a typed HTTP API error would get a silent runtime change — the httpApiStatus property no longer exists and the error can no longer be added to an HTTP API route with addError. Consider also explicitly documenting whether McpOAuthReauthorizationRequired is intentionally absent from the barrel export so the package surface is clear and consistent.

[RhysSullivan]

thanks!