Skip to content

fix: implement saveDiscoveryState/discoveryState on NodeOAuthClientProvider#236

Closed
mano1233 wants to merge 1 commit into
geelen:mainfrom
mano1233:fix/discovery-state-propagation
Closed

fix: implement saveDiscoveryState/discoveryState on NodeOAuthClientProvider#236
mano1233 wants to merge 1 commit into
geelen:mainfrom
mano1233:fix/discovery-state-propagation

Conversation

@mano1233

Copy link
Copy Markdown

Summary

Implement saveDiscoveryState() / discoveryState() on NodeOAuthClientProvider to fix OAuth metadata loss in proxy mode.

Root cause

In proxy mode, connectToRemoteServer(null, ...) creates a test StreamableHTTPClientTransport that correctly discovers _resourceMetadataUrl via POST → 401 → WWW-Authenticate header. But the main transport never gets this URL, so finishAuth() sends the token exchange to the MCP server URL instead of the authorization server — producing the "Invalid api path" error.

Why the try/finally approach from #231 is fragile

It accesses transport._resourceMetadataUrl directly, which is a private property on StreamableHTTPClientTransport. This couples mcp-remote to SDK internals that could change without notice.

Cleaner fix — SDK v1.27.0+ already solved this

PR modelcontextprotocol/typescript-sdk#1527 (merged Feb 13) added saveDiscoveryState() and discoveryState() to the OAuthClientProvider interface. When the test transport's auth() call discovers the authorization server, it calls provider.saveDiscoveryState(). When the main transport's finishAuth() calls auth(), it checks provider.discoveryState() first — retrieving the cached metadata instead of re-discovering.

Since both transports share the same authProvider instance, the discovery state propagates correctly without touching any private fields.

Changes

  • Updated @modelcontextprotocol/sdk from 1.25.31.27.1
  • Implemented saveDiscoveryState() / discoveryState() on NodeOAuthClientProvider with in-memory + disk persistence (discovery_state.json)
  • Added 'discovery' scope to invalidateCredentials() for re-discovery on auth failures

Test plan

  • pnpm run build — passes
  • pnpm run test:unit — 103/103 tests pass
  • Tested against: AWS Bedrock AgentCore + Okta OAuth provider. With the methods → works. Without the methods (same SDK version) → fails with "Invalid api path".

Fixes #231

…ovider

- Update @modelcontextprotocol/sdk from 1.25.3 to 1.27.1
- Implement saveDiscoveryState() / discoveryState() with in-memory + disk
  persistence (discovery_state.json)
- Add 'discovery' scope to invalidateCredentials() for re-discovery on
  auth failures

Fixes geelen/mcp-remote#231
@halllo

halllo commented Mar 21, 2026

Copy link
Copy Markdown

I proposed a solution that reuses the same transport and therefore does not need saveDiscoveryState: #167

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.

_resourceMetadataUrl not propagated from test transport to main transport when client=null

2 participants