-
Notifications
You must be signed in to change notification settings - Fork 3.7k
fix(mcp): Upgrade SDK and add callbackPort/callbackPath config #5940
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
christso
wants to merge
13
commits into
sst:dev
Choose a base branch
from
EntityProcess:feat/oauth-public-client-fallback
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
fix(mcp): Upgrade SDK and add callbackPort/callbackPath config #5940
christso
wants to merge
13
commits into
sst:dev
from
EntityProcess:feat/oauth-public-client-fallback
+184
−35
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
Future Enhancement Note: Dynamic port allocation (binding to port |
- Change config from `redirectUri` string to separate `callbackPort` and `callbackPath` options - Support dynamic port allocation with `callbackPort: 0` - Default behavior unchanged (port 19876) - Add tests for ensureRunning behavior 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
377f4ab to
6dcd821
Compare
Remove all DCR (Dynamic Client Registration) fallback code that was incorrectly kept during the rebase conflict resolution. The commit "remove connected_public_client" had already removed this feature. Removed: - connected_public_client status from Status schema - usedPublicClient references from authenticate function - DCR tracking in auth.ts (dcrFailed, usedPublicClient fields) - DCR helper functions (markDcrFailed, clearDcrFailed, markUsedPublicClient) - UI handling for connected_public_client in mcp.ts and sidebar.tsx 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use explicit check for "failed" status instead of fallback cast. Add exhaustive else clause to satisfy TypeScript. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Fix dynamic port handling when callbackPort is 0 (dynamic allocation) - Start callback server to get actual port instead of hardcoding fallback 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
7fd33af to
260374d
Compare
Revert to simpler else clause with type cast. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
319518a to
88a3615
Compare
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
3eef5d6 to
01fb104
Compare
- Single check with early return for "already configured" case - Clearer flow: check running → check port in use → start server - Renamed variable to requestedPath for consistency 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #5766
Summary
callbackPortandcallbackPathfor OAuth callbackcallbackPort: 0Problem
Wrong authorization URL: MCP SDK 1.15.1 incorrectly constructs the authorization URL by appending
/authorizeto the issuer URL instead of usingauthorization_endpointfrom OAuth metadata.No callback config: Users cannot configure the OAuth callback URL to match what's registered on their OAuth server.
SSH port forwarding: Users running opencode over SSH may only be able to forward specific ports, requiring explicit port configuration.
Solution
Upgrade MCP SDK to 1.25.1 which correctly reads
authorization_endpointfrom OAuth discovery metadata.Add callback config options:
callbackPort: OAuth callback server port (default: 19876, set to 0 for dynamic)callbackPath: OAuth callback path (default: /mcp/oauth/callback)User Configuration
For dynamic port (OS assigns available port, similar to VSCode's approach):
Test plan
callbackPort: 0ensureRunningbehaviormcp debugcommand to use actual allocated port🤖 Generated with Claude Code