fix(mcp): disable interactive OAuth in embedded MCP server handler#4444
Merged
fix(mcp): disable interactive OAuth in embedded MCP server handler#4444
Conversation
The MCP server handler was calling registry.GetDefaultProvider() without explicitly setting WithInteractive(false). Because the registry provider is a singleton (sync.Once), and the MCP handler is typically the first caller during `thv serve --experimental-mcp`, it "poisoned" the shared provider with interactive=true. All subsequent callers — including the HTTP API routes that intentionally disable interactive mode — inherited this setting, causing the browser to open for OAuth on every registry access when credentials were invalid. Made-with: Cursor
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4444 +/- ##
==========================================
+ Coverage 69.48% 69.54% +0.05%
==========================================
Files 486 486
Lines 50017 50029 +12
==========================================
+ Hits 34755 34792 +37
+ Misses 12577 12551 -26
- Partials 2685 2686 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
JAORMX
approved these changes
Mar 30, 2026
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
2 participants
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.
Summary
When
thv servestarts with--experimental-mcp(always enabled by ToolHive Studio), the MCP server handler was the first code path to create the shared singletonregistry.Providerviaregistry.GetDefaultProvider(). BecauseNewRegistryProviderdefaults tointeractive: trueand the provider is guarded bysync.Once, this "poisoned" the singleton — all subsequent callers (including the HTTP API routes that explicitly passWithInteractive(false)) inherited interactive mode.The result: every time the desktop app restarted with invalid registry credentials (e.g. wrong
client_id), the browser opened automatically for an OAuth flow that would always fail.registry.WithInteractive(false)explicitly in the MCP handler, aligning it with how all otherthv servecode paths create the provider.Type of change
Test plan
Tested with ToolHive Studio connected to a
thv serveinstance with a misconfigured registry (client_idwrong). Before the fix, restarting the app opened the browser for OAuth. After the fix, the app correctly shows a registry configuration error without opening the browser.Changes
pkg/mcp/server/handler.goGetDefaultProviderWithConfigwithWithInteractive(false)instead ofGetDefaultProvider()Does this introduce a user-facing change?
Yes — when
thv serveis started with--experimental-mcpand the registry has invalid OAuth credentials, the browser no longer opens automatically for an OAuth flow that cannot succeed. Instead, the error is returned to the caller (API or MCP client) for proper handling.Special notes for reviewers
The root cause is that
registry.Provideris async.Oncesingleton. Whichever caller creates it first setsinteractivefor all subsequent users. Inthv serve, the MCP handler goroutine consistently wins the race againstlazySkillLookupand the API routes. This fix ensures the MCP handler creates the singleton withinteractive: false, which is the correct behavior for all server-side code paths.The
POST /auth/loginendpoint is unaffected because it creates its own standalone provider (not the singleton) withWithInteractive(true)explicitly.