fix(mcp): log actionable OAuth discovery failures for misconfigured server urls - #34225
Conversation
…erver urls A typo'd MCP server url failed OAuth endpoint discovery silently: every failure died at debug level, the config loader warned nothing, and the /authorize 400 blamed "servers with no url" even when a url was set. _descovery_metadata now records each attempt's outcome and, when a total failure would leave the server's flow without a needed endpoint, logs one warning with the trail (urls origin-only, exception text url-stripped). Both server loaders warn which endpoints stayed unresolved for the server's flow (client_credentials never needs authorization_url, OBO needs only token_url) with the remedies; this replaces the DB path's reason-less warning and closes the config path's no-warning gap. The authorize/token/register 400 details branch on server shape via one shared helper and point at the proxy logs. _redact_mcp_resource_url moves to oauth_utils.py so the manager can import it without a cycle. Resolves LIT-4658
Greptile SummaryThis PR makes failed MCP OAuth discovery easier to diagnose. The main changes are:
Confidence Score: 5/5This looks safe to merge.
|
| Filename | Overview |
|---|---|
| litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py | Adds server-specific details for unresolved authorization and token endpoints without exposing configured values. |
| litellm/proxy/_experimental/mcp_server/mcp_server_manager.py | Adds redacted discovery-attempt warnings and flow-aware unresolved-endpoint warnings for both server loaders. |
| litellm/proxy/_experimental/mcp_server/oauth_utils.py | Centralizes MCP URL redaction and catches malformed ports during lazy URL parsing. |
| litellm/proxy/_experimental/mcp_server/server.py | Uses the shared MCP URL-redaction helper. |
| tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py | Covers endpoint errors for URL-based, issuer-based, and URL-less server configurations. |
| tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server.py | Adds a direct malformed-port test for the shared URL-redaction helper. |
| tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py | Covers sanitized discovery warnings and endpoint requirements across configuration and database loading. |
Reviews (2): Last reviewed commit: "fix(mcp): keep url redaction total when ..." | Re-trigger Greptile
urlsplit validates the port lazily, so a non-numeric port raised ValueError out of _redact_mcp_resource_url after the urlsplit try had already passed; the server loaders now call the helper while warning about typo'd urls, which would have turned the warning into a load failure. Resolve hostname and port inside the guard and pin the malformed-port case in the redaction test
|
CI status: 79/80 checks green. The one red, "All Other Providers / Run tests", is a pre-existing flake unrelated to this diff. It fails only on |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit df75d29. Configure here.
TLDR
Problem this solves:
How it solves it:
Relevant issues
Linear ticket
Resolves LIT-4658
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Note: the lint-e2e-basedpyright gate is red on the unmodified PR base (8 pre-existing errors in tests/e2e/load/test_session_anomaly.py from the staging tip merge #34166); this diff touches nothing under tests/e2e and every gate it does touch is green
Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Live proxy on 127.0.0.1:4658, no DB, config with three oauth2 servers: a typo'd host (DNS cannot resolve), a typo'd path on a host with no OAuth discovery (api.github.com/mcp), and a working control (mcp.linear.app/sse)
Before, at the PR base 6375923 (stock litellm_internal_staging): startup logs contain zero warning lines about either broken server, and the authorize wall misdirects the operator
After, at cfbcef3: the same startup logs the reason per server, urls reduced to origin
and the authorize wall now names the likely cause
Negative control on the same run: the correctly configured server discovers Linear's IdP and keeps redirecting, with no warnings logged for it
Type
🐛 Bug Fix
Changes
_descovery_metadatagainswarn_when_no_metadata: an all-empty discovery result logs one warning listing each step's outcome (initial GET status or exception class, well-known lookups, origin fallback), with urls reduced to origin and exception text stripped of embedded urls_warn_oauth_endpoints_unresolvednaming the server, the endpoints its flow needs that stayed unresolved (client_credentials never needs authorization_url, OBO needs only token_url), and the remedies; this replaces the DB path's reason-less warning and closes the config path's no-warning gap_endpoint_not_configured_detailthat branches on server shape: anchored issuer, url set (discovery failed, check logs), or genuinely url-less (wording unchanged)_redact_mcp_resource_urlmoves from server.py to oauth_utils.py so the manager can import it without a cycle; behavior unchanged, server.py imports it from the new homeTwo things a reviewer may ask about. The DB path's old "yielded no metadata" warning also fired when manual endpoints were fully set and only scopes failed to discover; that case is deliberately silent now because scope-less metadata is normal for many servers and nothing actionable follows, and scopes never trigger the new warning on their own. The loader warning fires at build time while last-known-good carry-forward can still restore endpoints afterwards, so it says "until they resolve" rather than promising a 400
Final Attestation
Note
Low Risk
Observability and operator-facing error text only; discovery logic and HTTP status codes are unchanged, with broad test coverage for logging and API details.
Overview
MCP OAuth misconfiguration is now visible at default log level, and authorize/token/register 400 responses match how the server is set up instead of always blaming url-less servers.
On discovery,
_descovery_metadatacan emit one WARNING with a per-step attempt trail (HTTP status, well-known lookups, origin fallback), using redacted origins and sanitized exception text. Config and DB server builds call_warn_oauth_endpoints_unresolvedwhen flows still lack needed endpoints after manual config—scoped by flow (client_credentialsskipsauthorization_url; OBO only needstoken_url).Missing authorization / token URLs on discoverable endpoints now use
_endpoint_not_configured_detail: anchored issuer failure, url present but discovery failed (check logs), or genuinely no url._redact_mcp_resource_urlmoves tooauth_utils.py(shared by manager logging); malformed ports no longer raise from lazyurlsplitvalidation.Reviewed by Cursor Bugbot for commit df75d29. Bugbot is set up for automated code reviews on this repo. Configure here.