fix(mcp): require ready connection after remote MCP OAuth callback

[kentcdodds]

Summary

Bernardo Munz (moonbe77@gmail.com) emailed about connecting his Recipe Keeper MCP (https://polite-ladybug-723.convex.site/mcp, Clerk OAuth). After Clerk authorization succeeded, Kody left the server in Authorization required with no Authorize link, no error, and no authenticated request reaching Recipe Keeper. Reconnect did the same.

Root cause: the Agents SDK authSuccess path clears the stored auth_url and can leave the live connection in authenticating without a usable auth URL. Kody treated SDK authSuccess as final and redirected with ?auth=success.

Fix

• Report OAuth callback success only when the hub connection is ready
• Detect stuck authenticating + missing authUrl, invalidate unusable tokens, and reconnect to mint a fresh auth URL (callback + reconnect)
• Redirect failed post-auth outcomes to the server detail page with a concrete error
• Show a recovery message in the account UI when that stuck state appears
• Document the ready-only success contract

Test plan

• Unit tests for OAuth outcome resolution (ready vs stuck authenticating vs SDK failure)
• Account MCP OAuth callback handler redirects incomplete auth to the server detail with auth=error
• After deploy: Bernardo removes/re-adds Recipe Keeper (or Reconnect), completes Clerk auth, confirms tools appear / authenticated requests reach Convex

System recap — extends existing primitives (medium risk)

Classification

extends — changes MCP client OAuth callback success semantics and reconnect recovery for stuck auth state.

Primitives touched

Primitive	ID	Group	Classification
MCP client servers	mcp-client-servers	assistant	extends
Browser app (Remix 3)	app-ui	surfaces	extends

What changed

• Hub handleOAuthCallback no longer trusts Agents SDK authSuccess alone; success requires ready
• Stuck authenticating without authUrl recovers by invalidating tokens and reconnecting
• Account MCP servers UI explains the stuck state and callback errors land on the server detail page

Risk / invariants

• Per-user isolation unchanged (hub DO still keyed by userId)
• Medium risk: OAuth callback UX now fails closed when connection does not become ready (correct, but users who previously saw a false success banner will see an error + recovery path)

Docs

• docs/contributing/architecture/mcp-client-servers.md — ready-only callback success + reconnect recovery

flowchart LR
  A[Clerk OAuth redirect] --> B[Hub handleOAuthCallback]
  B --> C{SDK authSuccess?}
  C -->|no| F[auth=error]
  C -->|yes| D[establishConnection]
  D --> E{state ready?}
  E -->|yes| G[auth=success]
  E -->|authenticating no authUrl| H[invalidate tokens + reconnect]
  H --> E
  E -->|still not ready| F

Loading

  

Summary by CodeRabbit

• Bug Fixes
  • MCP server OAuth now marks authorization as successful only after the server connection reaches ready (not just when the SDK reports auth success).
  • Added clearer UI and redirects when auth completes but no authorization link is available, including a specific reason.
  • Improved recovery for MCP connections stuck in authenticating without an auth URL by invalidating unusable credentials and retrying.
  • Ensured success/error redirects reliably target the correct server page when a server ID is present.
• Documentation
  • Clarified OAuth callback success criteria, stuck-state handling, and supported redirect targets.
• Tests
  • Added unit coverage for OAuth callback outcome resolution, including missing authorization-link scenarios.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: faf779af-82e1-4ed2-b84e-faa9b2ea24d5

📥 Commits

Reviewing files that changed from the base of the PR and between 2a9367f and b8c8b3e.

📒 Files selected for processing (1)

• docs/contributing/architecture/mcp-client-servers.md

🚧 Files skipped from review as they are similar to previous changes (1)

• docs/contributing/architecture/mcp-client-servers.md

---

📝 Walkthrough

Walkthrough

OAuth callback success now requires an MCP connection to reach ready. Stuck authenticating connections without an authorization URL trigger credential invalidation and reconnection, with server-specific error redirects and corresponding UI guidance.

Changes

MCP OAuth outcome handling

Layer / File(s)	Summary
OAuth outcome resolution and validation packages/worker/src/mcp-client/oauth-callback-outcome.ts, packages/worker/src/mcp-client/oauth-callback-outcome.node.test.ts	Adds helpers that classify callback outcomes, require ready for success, describe incomplete connections, and detect missing authorization links with unit coverage.
Hub callback recovery packages/worker/src/mcp-client/hub.ts	Resolves callback outcomes from connection state and recovers stuck authenticating connections by invalidating tokens, reconnecting, and rediscovering tools.
Server-scoped callback feedback and UI states packages/worker/src/app/handlers/account-mcp-servers.ts, packages/worker/src/app/handlers/account-mcp-servers.node.test.ts, packages/worker/client/routes/account-mcp-servers.tsx, docs/contributing/architecture/mcp-client-servers.md	Preserves server-specific redirect targets for failed callbacks, adds coverage for missing authorization links, updates the UI recovery message, and documents the revised OAuth flow.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant AccountMcpServers
  participant McpClientHubBase
  participant MCPServer
  participant AccountRoute
  User->>AccountMcpServers: Complete OAuth authorization
  AccountMcpServers->>McpClientHubBase: handleOAuthCallback
  McpClientHubBase->>MCPServer: Establish connection
  MCPServer-->>McpClientHubBase: Connection state
  McpClientHubBase-->>AccountMcpServers: Success or error outcome
  AccountMcpServers->>AccountRoute: Redirect with auth result and server id
  AccountRoute-->>User: Show ready state or recovery guidance

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: OAuth callback success now requires the MCP connection to reach ready.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch cursor/fix-mcp-oauth-callback-stuck-b21d

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

🔎 Preview deployed: https://kody-pr-966.kody-a99.workers.dev

Worker: kody-pr-966
D1: kody-pr-966-db
KV: kody-pr-966-oauth-kv

Mocks:

• cloudflare: https://kody-pr-966-mock-cloudflare.kody-a99.workers.dev/__mocks (kody-pr-966-mock-cloudflare)

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/contributing/architecture/mcp-client-servers.md`:
- Around line 51-52: Update the callback flow description to document redirects
to the server-scoped `/account/mcp-servers/:serverId` route for both successful
and failed callbacks, replacing the list-route-only wording while preserving the
existing feedback query parameters.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9031cfbf-6d31-42bb-a9aa-3a486af9da04

📥 Commits

Reviewing files that changed from the base of the PR and between c6686d6 and 63c6585.

📒 Files selected for processing (7)

• docs/contributing/architecture/mcp-client-servers.md
• packages/worker/client/routes/account-mcp-servers.tsx
• packages/worker/src/app/handlers/account-mcp-servers.node.test.ts
• packages/worker/src/app/handlers/account-mcp-servers.ts
• packages/worker/src/mcp-client/hub.ts
• packages/worker/src/mcp-client/oauth-callback-outcome.node.test.ts
• packages/worker/src/mcp-client/oauth-callback-outcome.ts