Skip to content

fix(mcp): auth-type handling, OAuth-pending UX, safe diagnostics + HTTP/2#5757

Merged
waleedlatif1 merged 6 commits into
stagingfrom
mcp-connect-fixes
Jul 18, 2026
Merged

fix(mcp): auth-type handling, OAuth-pending UX, safe diagnostics + HTTP/2#5757
waleedlatif1 merged 6 commits into
stagingfrom
mcp-connect-fixes

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

Minimal, focused set of MCP client fixes — the genuinely-good changes carved out of #5754 (whose large ordered-publication consistency layer was reverted as disproportionate; see below). Two clean commits, no fencing/CAS machinery.

  • Auth-type classification — a non-OAuth UnauthorizedError is now classified as an auth failure instead of an OAuth redirect, so static-bearer servers that merely advertise OAuth stop being diverted into the OAuth flow (fixes the class of server that couldn't connect, [BUG] MCP server "discover" step ignores configured static Authorization header, always requires OAuth #5665).
  • OAuth switch resets status — switching a server to OAuth now resets it to disconnected (it hasn't completed its auth flow), matching the create-path invariant, so it can't falsely read as connected.
  • OAuth-pending UX — the server list and refresh action surface "OAuth authorization required" instead of a generic "Not Connected".
  • DCR → 422 — OAuth servers that don't support dynamic client registration return an actionable 422.
  • Safe error diagnostics — MCP transport/connect logs redact messages/cause/session-ids via a shared getMcpSafeErrorDiagnostics.
  • HTTP/2 — the SSRF-pinned MCP fetch now negotiates h2 (MCP servers are commonly behind h2 fronts); pinning is unaffected. Default stays h1.1 for non-MCP callers.

Not in scope

This does not fix the dominant -32001 tools/list timeouts (connect-per-op with no connection reuse) — that's the next PR (turn-scoped connection reuse).

Type of Change

  • Bug fix
  • Improvement

Testing

416 MCP/security tests pass; tsc clean; biome clean; adversarial review passed.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Jul 18, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jul 18, 2026 8:05pm

Request Review

@cursor

cursor Bot commented Jul 18, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes touch OAuth/auth classification, token revocation on config updates, and outbound HTTP transport (h2 + pinning); scope is well-tested but affects connection and credential handling paths.

Overview
Improves MCP auth correctness, settings messaging, and transport hardening without changing unrelated connection-reuse behavior.

Client & OAuth flow: UnauthorizedError is only treated as “authorization required” when authType is oauth, so header/token servers no longer get pushed into OAuth. OAuth start returns 422 with guidance when the auth server lacks dynamic client registration. Server updates that flip auth type (or add OAuth client ID) reset connectionStatus to disconnected, clear lastError, revoke OAuth tokens when OAuth is turned off, and widen workspace cache invalidation (e.g. transport changes).

UX: Disconnected OAuth servers show “OAuth authorization required” in the list, detail status, and refresh action unless the refresh returned a concrete error (still Failed).

Logging & transport: Connect/transport/tools/list logs use getMcpSafeErrorDiagnostics (redacted structural fields, no messages/session IDs). The live MCP transport uses createPinnedMcpFetch with optional HTTP/2 (allowH2) while keeping IP pinning; the undici Agent is destroyed on failed connect and disconnect to avoid h2 socket leaks.

Reviewed by Cursor Bugbot for commit b856239. Configure here.

Comment thread apps/sim/lib/mcp/orchestration/server-lifecycle.ts Outdated
@greptile-apps

greptile-apps Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR improves MCP authentication, OAuth handling, diagnostics, and transport behavior. The main changes are:

  • Classify static-token authorization failures separately from OAuth redirects.
  • Reset connection state and clear OAuth credentials when authentication changes.
  • Show clearer OAuth authorization messages in server settings.
  • Return an actionable response when dynamic client registration is unsupported.
  • Redact MCP error diagnostics and omit unsafe error text.
  • Enable HTTP/2 for long-lived pinned MCP transports and close their Agents.

Confidence Score: 5/5

This looks safe to merge.

  • The reported authentication-state and OAuth-cleanup paths are addressed.
  • Pinned HTTP/2 transport resources are closed on disconnect and failed connection paths.
  • Error logs no longer include untrusted message text or session identifiers.
  • No blocking issues were found in the updated code.

Important Files Changed

Filename Overview
apps/sim/lib/mcp/orchestration/server-lifecycle.ts Resets stale connection state, cleans up OAuth credentials, and invalidates cached server state when authentication changes.
apps/sim/lib/mcp/client.ts Separates static authentication failures from OAuth redirects, sanitizes diagnostics, and closes pinned transport resources.
apps/sim/lib/mcp/pinned-fetch.ts Enables HTTP/2 for long-lived MCP transports and exposes an Agent cleanup handle.
apps/sim/lib/core/security/input-validation.server.ts Adds optional HTTP/2 negotiation while preserving pinned address resolution and the existing HTTP/1.1 default.
apps/sim/lib/mcp/error-diagnostics.ts Provides bounded and redacted structural diagnostics without credential-bearing messages or causes.

Reviews (5): Last reviewed commit: "fix(mcp): make pinned Agent teardown ide..." | Re-trigger Greptile

Comment thread apps/sim/lib/mcp/orchestration/server-lifecycle.ts
Comment thread apps/sim/lib/mcp/client.ts
Comment thread apps/sim/lib/mcp/pinned-fetch.ts
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/mcp/client.ts
Comment thread apps/sim/lib/mcp/client.ts Outdated
Comment thread apps/sim/lib/mcp/orchestration/server-lifecycle.ts
Comment thread apps/sim/lib/mcp/pinned-fetch.ts Outdated
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/mcp/orchestration/server-lifecycle.ts Outdated
Comment thread apps/sim/lib/mcp/pinned-fetch.ts Outdated
Comment thread apps/sim/lib/mcp/orchestration/server-lifecycle.ts
Comment thread apps/sim/app/api/mcp/oauth/start/route.ts Outdated
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/mcp/client.ts
MCP servers are commonly behind HTTP/2 fronts (CDNs, cloud LBs), but the
SSRF-pinned undici Agent is HTTP/1.1-only unless it opts into h2 via ALPN. Add
an allowH2 option to createPinnedFetch (default false, so LLM-provider callers
are unchanged) and centralize the MCP h2 decision in one createPinnedMcpFetch
routed through the transport, OAuth probe, and SSRF-guarded fetch. Pinning is
unaffected: the pinned lookup forces every connection to the resolved IP.
…r logging

- Classify a non-OAuth UnauthorizedError as an auth failure instead of an OAuth
  redirect, so static-bearer servers that merely advertise OAuth stop being
  diverted into the OAuth flow (fixes the class of server that couldn't connect).
- Reset a server to disconnected when it is switched to OAuth, so it can't
  falsely read as connected before completing its auth flow.
- Surface OAuth-pending state as 'OAuth authorization required' in the server
  list and refresh action instead of a generic 'Not Connected'.
- Return an actionable 422 for OAuth servers that don't support dynamic client
  registration.
- Redact error messages/cause/session-ids from MCP transport/connect logs via a
  shared getMcpSafeErrorDiagnostics helper.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit b856239. Configure here.

@waleedlatif1
waleedlatif1 merged commit 32de415 into staging Jul 18, 2026
20 checks passed
@waleedlatif1
waleedlatif1 deleted the mcp-connect-fixes branch July 18, 2026 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant