Skip to content

Remove the probe-era auth-challenge workaround and detect the SDK's typed 401/403 #1807

Description

@cliffhall

Follow-up from #1805 / PR #1806.

Remove the client-side workaround that compensates for an SDK probe-classification gap. The gap is closed — the SDK fix shipped in @modelcontextprotocol/client@2.0.0, which is in the tree as of #1989. This is ordinary local cleanup, ready to pick up.

Background — why the workaround exists

Era auto/modern sends the SDK's server/discover negotiation probe before initialize, so authorization first surfaces at the probe. On the direct transport path (CLI/TUI, and any path with no stored tokens and therefore no authProvider), a 401 reached the SDK as a raw SdkHttpError. The old classifyHttpError only looked for a JSON-RPC error body — it ignored the HTTP status — so it verdicted "not a modern server". In pin mode (protocolEra: "modern") that verdict was rethrown with the 401 discarded entirely: no status, not even a cause.

#1806 worked around it by enabling interceptAuthChallenges for the probing eras even with no stored tokens, so the 401 became a typed AuthChallengeError that survives the probe as data.cause.

What the SDK now does

typescript-sdk#2564 rewrote the classification, covering both of the original asks:

  • 401/403 are auth outcomes, never era evidence. classifyHttpError gets explicit rows ahead of the JSON-RPC body parse: a 401/403 rejects as a typed SdkHttpError with code ClientHttpAuthentication / ClientHttpForbidden. The codes are deliberately not EraNegotiationFailed, so an auth wall cannot enter era-recovery flows keyed on that code.
  • The status is no longer dropped. The error carries status, statusText, and the response text in data. 5xx is split out as well (as EraNegotiationFailed), leaving the legacy-fallback set equal to the spec-licensed one — the 4xx a deployed 2025 server answers a request it does not recognize.
  • Auth errors propagate unchanged. A new internal auth-seam stamp (Symbol.for('mcp.authSeamEscape')) marks errors escaping the transport's auth boundaries, and the probe routes stamped errors to auth-required with the original object intact — identity, instanceof, and cause chain preserved.

The work

  1. Delete the || this.probesProtocolEra() clause in core/mcp/inspectorClient.ts, so interceptAuthChallenges is enabled only when there is actually an authProvider. Trim the long comment block above it down to what still applies.

  2. Teach isUnauthorizedError (core/auth/utils.ts) to recognize the new error. This is the part that is easy to miss, and it is load-bearing:

    • SdkHttpError carries the status at err.data.status, not err.status.
    • err.code is SdkErrorCode.ClientHttpAuthentication, not 401.
    • The message reads Version negotiation failed: the server requires authorization (HTTP 401), which the existing \bfailed\b[^\n]*\(401\) pattern does not match — (HTTP 401) is not (401).
    • There is no cause chain to walk.

    So with the intercept removed and the detector untouched, a probe 401 would fail to start OAuth recovery. Both changes land together or neither does.

  3. Keep findNestedAuthError() in core/auth/challenge.ts — it is the permanent fix and is deliberately not gated on the SDK error code or message, so it keeps working across SDK rewording.

Bonus: an accepted side effect goes away

Turning the intercept on with no stored tokens meant parseAuthChallengeFromResponse treated 403 as a challenge too, so a probe answered 403 for a non-auth reason (a gateway rejecting the unknown server/discover method, say) started OAuth discovery instead of letting auto fall back to legacy. That was documented as a known, accepted cost of the workaround. Removing the clause removes it.

Acceptance criteria

  • The || this.probesProtocolEra() clause is gone; interceptAuthChallenges follows authProvider alone.
  • isUnauthorizedError recognizes SdkHttpError with data.status of 401/403, with tests covering the real shape the SDK produces (status, code, and message all as emitted — not a hand-rolled stand-in).
  • Connect to an OAuth-protected server on era auto and modern, with no stored tokens, on CLI/TUI (direct transport) and web: OAuth recovery starts in every combination.
  • A probe answered 403 for a non-auth reason falls back to legacy under auto rather than starting OAuth discovery.
  • npm run ci passes.

Activity

  1. self-assigned this
    on Jul 27, 2026
  2. cliffhall commented on Jul 27, 2026

    @cliffhall
    MemberAuthor

    Filed upstream: modelcontextprotocol/typescript-sdk#2561 — "server/discover probe classifier discards HTTP 401/403, and pin mode drops the cause".

    Covers both asks from this issue, with the beta.5 source excerpts for classifyHttpError and the pin-mode throw site, plus a note on the normalizeReply → classifyNetworkError path (typed auth errors land at data.cause, not cause, since SdkError's third parameter is data).

    Once ask (1) lands upstream, the probesProtocolEra() / interceptAuthChallenges workaround in core/mcp/inspectorClient.ts can be narrowed or removed. The findNestedAuthError() unwrap in core/auth/challenge.ts is not gated on the SDK's error code or message, so it stays correct either way and can be left in place.

  3. changed the title [-]Upstream SDK: probe classifier should treat 401/403 as auth-required, and pin mode must not drop the status[/-] [+][Blocked upstream] SDK probe classifier should treat 401/403 as auth-required, and pin mode must not drop the status[/+] on Jul 27, 2026
  4. added this to the v2.2.0 milestone on Jul 28, 2026
  5. removed their assignment
    on Aug 1, 2026
  6. cliffhall commented on Aug 1, 2026

    @cliffhall
    MemberAuthor

    Triage: Priority = Low

    Scored 5/16 with the priority rubric added to AGENTS.md in #1891. This card is already approved and sitting in Todo — only Priority was set here, its Status is unchanged.

    Axis Score Reasoning
    Severity / impact 2/5 A real misclassification — a 401/403 at the discover probe should read as auth-required — but the Inspector already ships a working client-side compensation in PR #1806, so users aren't currently affected.
    Urgency / staleness 2/5 Blocked on typescript-sdk#2561; no in-repo work is possible until that lands, and the compensation holds in the meantime.
    Signal bonuses +1 milestone (v2.2.0) (+1)
    Total 5 Lands in the Low band (5 or below).

    Related issues. These look connected and may be worth reading together:

    Scores are a starting point, not a verdict — the rubric is meant to be overruled when it's plainly wrong, provided the reason is written down.

  7. added
    authIssues and PRs related to authorization
    bugSomething isn't working
    on Aug 5, 2026
  8. modified the milestones: v2.2.0, v2.4.0 on Aug 11, 2026
  9. cliffhall commented on Aug 12, 2026

    @cliffhall
    MemberAuthor

    Unblocked upstream. The SDK fix landed as typescript-sdk#2564 — classifyHttpError now gets explicit rows ahead of the JSON-RPC body parse, rejecting a probe 401/403 as a typed SdkHttpError (ClientHttpAuthentication / ClientHttpForbidden) carrying the status, reason phrase, and response text, instead of falling into the conservative legacy fallback. 5xx is split out too (as EraNegotiationFailed), so the remaining legacy set is exactly the spec-licensed one.

    It ships in 2.0.0, which #1988 / #1989 brings in. That PR deliberately does not touch this — the directAuthRecovery clause still sets interceptAuthChallenges, so the challenge is typed before the classifier sees it and connect behavior is unchanged by the bump. This issue keeps ownership of removing the || this.probesProtocolEra() workaround, as its in-code comment instructs.

    One thing to account for when doing it: SdkHttpError carries the status at err.data.status, not err.status, and its message reads Version negotiation failed: the server requires authorization (HTTP 401). isUnauthorizedError in core/auth/utils.ts checks err.status / err.code === 401 and matches \bfailed\b[^\n]*\(401\) — (HTTP 401) does not match that pattern, and there is no cause chain to walk. So dropping the intercept without also teaching the detector about data.status would leave a 401 probe failing to start OAuth recovery.

  10. changed the title [-][Blocked upstream] SDK probe classifier should treat 401/403 as auth-required, and pin mode must not drop the status[/-] [+]SDK probe classifier should treat 401/403 as auth-required, and pin mode must not drop the status[/+] on Aug 12, 2026
  11. modified the milestones: v2.4.0, v2.3.0 on Aug 12, 2026
  12. changed the title [-]SDK probe classifier should treat 401/403 as auth-required, and pin mode must not drop the status[/-] [+]Remove the probe-era auth-challenge workaround and detect the SDK's typed 401/403[/+] on Aug 12, 2026
  13. added a commit that references this issue on Aug 12, 2026
    3e76498
  14. self-assigned this
    on Aug 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

authIssues and PRs related to authorizationbugSomething isn't workingv2Issues and PRs for v2

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions