feat(core): support pre-registered OAuth clients for remote MCP - #12983
Conversation
|
JetBrains Plugin tests failed ❌ The changes in this PR are causing test failures in the JetBrains plugin integration. Please review and fix your changes to ensure they work properly with:
Please check the workflow logs for specific test failure details and update your PR accordingly. Branch: |
Greptile SummaryThe PR adds pre-registered OAuth-client support for remote MCP servers across core, CLI, and the desktop example.
Confidence Score: 4/5The PR is not safe to merge until first-time authorization with a pre-registered OAuth client can persist its PKCE verifier. The MCP SDK saves the PKCE verifier before token exchange, but the provider currently requires matching client information to already be persisted, so a newly configured pre-registered client fails before the browser authorization step. Files Needing Attention: sdk/packages/core/src/extensions/mcp/oauth.ts
|
| Filename | Overview |
|---|---|
| sdk/packages/core/src/extensions/mcp/oauth.ts | Adds client-bound, concurrency-safe OAuth persistence, but the initial PKCE verifier write rejects a newly configured pre-registered client before authorization can begin. |
| sdk/packages/core/src/extensions/mcp/config-loader.ts | Adds OAuth-client parsing, guarded state mutation, per-server resolution, and OAuth status projection. |
| apps/cli/src/wizards/mcp/settings.ts | Persists configured OAuth clients and clears old OAuth state when client credentials change. |
| apps/examples/desktop-app/sidecar/commands.ts | Adds explicit desktop OAuth authorization, cancellation, passive probing, and safer MCP settings updates. |
| apps/examples/desktop-app/webview/components/views/settings/mcp-view.tsx | Surfaces OAuth status and explicit connect/cancel controls for remote MCP servers. |
Sequence Diagram
sequenceDiagram
participant Host
participant Core as MCP OAuth Provider
participant SDK as MCP SDK
participant Browser
Host->>Core: authorize configured server
Core->>SDK: connect with pre-registered client
SDK->>Core: saveCodeVerifier
Core-->>SDK: McpOAuthClientChangedError
Note over SDK,Browser: Browser authorization never opens
Prompt To Fix All With AI
### Issue 1
sdk/packages/core/src/extensions/mcp/oauth.ts:270-274
**Initial PKCE verifier is rejected**
When a pre-registered client starts its first OAuth authorization with no persisted `clientInformation`, `saveCodeVerifier` compares the empty state against the configured client and throws `McpOAuthClientChangedError`, preventing the browser authorization flow from starting.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (9): Last reviewed commit: "Merge branch 'main' into bee/mcp-oauth" | Re-trigger Greptile
|
Status review — two things here: 1. The CodeQL / github-advanced-security alert is stale and should be dismissed. It fired against the intermediate commit 2. One wiring gap I'd like addressed (or explained) before merge: Minor: Also — has the full browser round-trip been run against a real pre-registered client (e.g. GitHub's remote MCP)? The test procedure in the description only shows |
|
Reviewed the full diff at eb57dc2 (all 7 commits), with focus on the desktop wiring. Overall this is in good shape: the architecture split between passive connection and explicit browser authorization is the right design, and the later commits addressed the earlier review threads properly. Findings below, roughly by severity. What holds together well
Issues1. One malformed settings entry now blanks the entire desktop MCP list (moderate)
2. Editing an enabled remote server can silently disable it (moderate)
3. Connect then Cancel flips an enabled server to disabled (minor, possibly intended)
4. mcp-remote rewrite drops
5. Shared Removing the fixed 6. Stale error banner on disabled servers (nit) The card shows Verification
|
|
Reviewed and end-to-end tested this PR against a fake GitHub-style remote MCP server: an OAuth authorization server with no What was tested (CLI wizard + real browser flow):
One gap found — already fixed by the newer commits on this branch: at the initial revision ( All Minor notes (non-blocking):
|
| assertOAuthClientUnchanged( | ||
| options.serverName, | ||
| current, | ||
| clientInformation, | ||
| ); |
There was a problem hiding this comment.
Initial PKCE verifier is rejected
When a pre-registered client starts its first OAuth authorization with no persisted clientInformation, saveCodeVerifier compares the empty state against the configured client and throws McpOAuthClientChangedError, preventing the browser authorization flow from starting.
Knowledge Base Used: SDK Core (@cline/core)
Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk/packages/core/src/extensions/mcp/oauth.ts
Line: 270-274
Comment:
**Initial PKCE verifier is rejected**
When a pre-registered client starts its first OAuth authorization with no persisted `clientInformation`, `saveCodeVerifier` compares the empty state against the configured client and throws `McpOAuthClientChangedError`, preventing the browser authorization flow from starting.
**Knowledge Base Used:** [SDK Core (`@cline/core`)](https://app.greptile.com/cline-org-2/-/custom-context/knowledge-base/cline/cline/-/docs/sdk-core.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Port from cline/cline#12983 (the 'invalidate tokens when OAuth client changes' invariant): tokens are minted for a specific client_id, so after a user edits oauth.client_id / oauth.client_secret in config.yaml the old tokens can only fail with invalid_client. Pre-registered clients are deliberately exempt from the invalid_client auto-poison path, so the stale tokens wedged every request until ~/.hermes/mcp-tokens/<server>.* was wiped by hand. _maybe_preregister_client() now compares the on-disk client.json identity against the incoming config identity before overwriting it and discards tokens.json + meta.json on a mismatch (with a log line pointing at hermes mcp login). Unchanged identity is a strict no-op. Proven live on main with an isolated-HERMES_HOME E2E probe; regression tests sabotage-verified (fail without the wiring line).
Port from cline/cline#12983 (the 'invalidate tokens when OAuth client changes' invariant): tokens are minted for a specific client_id, so after a user edits oauth.client_id / oauth.client_secret in config.yaml the old tokens can only fail with invalid_client. Pre-registered clients are deliberately exempt from the invalid_client auto-poison path, so the stale tokens wedged every request until ~/.hermes/mcp-tokens/<server>.* was wiped by hand. _maybe_preregister_client() now compares the on-disk client.json identity against the incoming config identity before overwriting it and discards tokens.json + meta.json on a mismatch (with a log line pointing at hermes mcp login). Unchanged identity is a strict no-op. Proven live on main with an isolated-HERMES_HOME E2E probe; regression tests sabotage-verified (fail without the wiring line). (cherry picked from commit 3eac116b9d56dbcb7ed2a8624fcf28477d12f280)
Port from cline/cline#12983 (the 'invalidate tokens when OAuth client changes' invariant): tokens are minted for a specific client_id, so after a user edits oauth.client_id / oauth.client_secret in config.yaml the old tokens can only fail with invalid_client. Pre-registered clients are deliberately exempt from the invalid_client auto-poison path, so the stale tokens wedged every request until ~/.hermes/mcp-tokens/<server>.* was wiped by hand. _maybe_preregister_client() now compares the on-disk client.json identity against the incoming config identity before overwriting it and discards tokens.json + meta.json on a mismatch (with a log line pointing at hermes mcp login). Unchanged identity is a strict no-op. Proven live on main with an isolated-HERMES_HOME E2E probe; regression tests sabotage-verified (fail without the wiring line).
Related Issue
Issue: CLIENTS-74
Description
Enable MCP servers that do not support OAuth Dynamic Client Registration, such as GitHub’s remote MCP server, to authenticate using pre-registered OAuth client credentials.
Problem
The MCP OAuth flow assumed that authorization servers support Dynamic Client Registration. Servers without a registration endpoint failed before browser authorization with:
Incompatible auth server: does not support dynamic client registrationOAuth flow in Desktop are not wired up.
Root cause
The SDK only loaded dynamically registered client information from mutable OAuth state. There was no configuration interface for supplying an existing OAuth client ID and secret.
Fix
oauthClientMCP configuration containing a client ID and optional client secret.@cline/core.This allows services such as GitHub to use their pre-registered OAuth application while preserving the existing flow for servers that support dynamic registration.
Test Procedure
bun run cli mcp install github --transport http https://api.githubcopilot.com/mcp/
Type of Change
Pre-flight Checklist
bun test) and code is formatted and linted (bun run format && bun run lint)Screenshots
Connect should open the oauth flow in browser
Additional Notes