oauth client example: skip DCR if MCP_CLIENT_ID is set #549
Conversation
If the user has set the MCP_CLIENT_ID environmental variable then they don't want Dynamic Client Registration and it should be skipped.
WalkthroughIntroduces a conditional check before registering the OAuth client in examples/oauth_client/main.go. The client is registered only if oauthHandler.GetClientID() returns an empty string. All other OAuth flow steps remain unchanged. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
examples/oauth_client/main.go (2)
132-137: Add lightweight logs to clarify whether DCR is performed or skippedImproves observability and helps users understand why registration ran or was skipped.
Apply this diff:
- if oauthHandler.GetClientID() == "" { - err = oauthHandler.RegisterClient(context.Background(), "mcp-go-oauth-example") + if oauthHandler.GetClientID() == "" { + log.Printf("No client ID configured; attempting dynamic client registration...") + err = oauthHandler.RegisterClient(context.Background(), "mcp-go-oauth-example") if err != nil { log.Fatalf("Failed to register client: %v", err) } + } else { + log.Printf("Using configured client ID %q; skipping dynamic client registration.", oauthHandler.GetClientID()) }
28-36: Sanitize env vars to avoid accidental whitespace causing false “configured” stateIf MCP_CLIENT_ID or MCP_CLIENT_SECRET contain only whitespace,
GetClientID()may be non-empty and skip DCR, leading to confusing failures later. Trim the env vars at source.Apply this diff:
- ClientID: os.Getenv("MCP_CLIENT_ID"), - ClientSecret: os.Getenv("MCP_CLIENT_SECRET"), + ClientID: strings.TrimSpace(os.Getenv("MCP_CLIENT_ID")), + ClientSecret: strings.TrimSpace(os.Getenv("MCP_CLIENT_SECRET")),Also add the import:
import ( "context" "fmt" "log" "net/http" "os" "os/exec" "runtime" + "strings"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
examples/oauth_client/main.go(1 hunks)
🔇 Additional comments (1)
examples/oauth_client/main.go (1)
132-137: LGTM: Conditional DCR guard matches the PR objective and preserves expected behaviorChecking
oauthHandler.GetClientID()before callingRegisterClientcorrectly skips dynamic client registration when a client ID is provided (e.g., via MCP_CLIENT_ID). This aligns with the PR intent without altering the rest of the OAuth flow.
Description
If the user has set the MCP_CLIENT_ID environmental variable then they don't want Dynamic Client Registration and it should be skipped.
Type of Change
Checklist
MCP Spec Compliance
Additional Information
Summary by CodeRabbit