fix(mcp): trigger OAuth dance inside startAuth to get redirect URL#28740
Open
achetronic wants to merge 1 commit into
Open
fix(mcp): trigger OAuth dance inside startAuth to get redirect URL#28740achetronic wants to merge 1 commit into
achetronic wants to merge 1 commit into
Conversation
The MCP SDK's connect() resolves before initiating OAuth, which made startAuth() return with authorizationUrl="" and authenticate() then called defs() on an unauthenticated client, failing with "Failed to get tools" before the browser was ever opened. Force a listTools() right after connect() so the SDK runs the auth dance and onRedirect populates capturedUrl in time.
Contributor
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
github-actions
Bot
added
needs:issue
needs:compliance
Contributor
|
The following comment was made by an LLM, it may be inaccurate: Based on the search results, I found a potentially related PR: Related PR:
Additionally, there are other OAuth/MCP-related PRs in the codebase:
However, #26236 is the most similar in addressing the core issue of forcing OAuth flow at the right time. If that PR was merged and this issue persists, it may be a more specific edge case (with Keycloak dynamic client registration). |
github-actions
Bot
removed
needs:compliance
Contributor
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Issue for this PR
Closes #28741
Type of change
What does this PR do?
opencode mcp auth <server>against any OAuth-protected MCP server fails right away withFailed to get tools. The browser never opens, the callback server stays idle. Reproduced onv1.15.6with a Keycloak backend using dynamic client registration.The MCP SDK's
connect()resolves before it actually starts the OAuth flow — that only kicks in on the first real request. SostartAuth()thinks everything is fine and returnsauthorizationUrl="", andauthenticate()ends up callingdefs()→listTools()on a client that isn't authenticated yet. By the time the SDK gets around to firingonRedirectand fillingcapturedUrl, it's too late:defs()has already swallowed theUnauthorizedErrorand reported failure.This PR makes
startAuth()itself calllistTools()right afterconnect(). That forces the SDK to do the OAuth dance there and then, socapturedUrlis ready by the time we return.authenticate()sees a real authorization URL, opens the browser, and waits for the callback like it's supposed to.How did you verify your code works?
Ran the patched build end-to-end against my own Keycloak-backed MCP server:
rm -f ~/.local/share/opencode/mcp-auth.jsonto start cleanbun run --conditions=browser ./src/index.ts --log-level DEBUG --print-logs mcp auth myserverhttp://127.0.0.1:19876/mcp/oauth/callback?code=..., opencode picks up the code, exchanges it for tokens and stores them.Also confirmed
bun run typecheckpasses clean.Screenshots / recordings
N/A (CLI flow, no UI changes).
Checklist