fix(health): probe test_connection with the credential the request names - #39801
Conversation
/health/test_connection matches the request's model string against the configured deployments and merges the match's litellm_params underneath the request. A request that named a stored credential but no key of its own still satisfied the "request sets no connection fields" test, so it inherited the matched deployment's api_key and api_base, and load_credentials_from_list then skipped the named credential because api_key was already set. A wildcard route covering the model is enough to match, so the Add Model page's Test Connect probed with an unrelated deployment's key while echoing back the credential that was selected. Naming a credential the configuration does not name now withholds the configuration's credential fields, the same set already withheld from a request that supplies its own endpoint. Naming no credential still inherits them, as documented.
…itellm_health_test_connection_credential
Merging this PR will not alter performance
Comparing |
Greptile SummaryThis PR makes connection tests honor the stored credential named by the request instead of inheriting credentials and endpoint settings from an unrelated model match. Requests without a named credential retain existing configuration inheritance, and deployment-by-ID probes preserve their configured endpoint. Regression tests cover wildcard matches, credential-provided endpoints, provider defaults, blank names, and existing deployments Confidence Score: 5/5The PR appears safe to merge because the credential inheritance fix is focused, preserves the unnamed-credential path, and has direct regression coverage No actionable new failures remain. The previous compatibility and test-comment threads were manually resolved, and the latest changes remove the redundant test commentary without changing behavior
|
| Filename | Overview |
|---|---|
| litellm/proxy/health_endpoints/_health_endpoints.py | Adds credential-aware configuration inheritance so connection probes use the request’s selected stored credential without borrowing unrelated deployment connection fields |
| tests/test_litellm/proxy/health_endpoints/test_health_endpoints.py | Adds focused regression coverage for named credentials, wildcard deployments, blank names, provider defaults, and deployment-by-ID behavior, while removing redundant commentary |
Reviews (2): Last reviewed commit: "test(health): drop test docstrings that ..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
@greptileai review Generated by Claude Code |
…l args The connection-test regressions patched litellm.ahealth_check and read the params handed to it. Driving the endpoint through the app with respx faking the upstream instead lets the real credential resolution run, so the tests assert the key and host that actually go out, which is what the bug was about. It also drops three of the five patched proxy internals; the two that are left are proxy-global wiring with no injection seam, the same ones the image_edit connection test already has to reach for.
2849aee
into
litellm_internal_staging
TLDR
Problem this solves:
How it solves it:
User Flow
Before: an admin adding a new xAI model with a saved credential gets an auth error from Test Connect, even though the same model works once saved
grok-4as the modelAfter: the same Test Connect passes, using the credential that was picked
grok-4as the modelRelevant issues
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
Shared setup, both sides. A stand-in upstream on port 54711 and another on port 54712, each answering a chat completion and logging the
Authorizationheader it received. xAI points at the first one (XAI_API_BASEis set to the 54711 one), so the second is only ever reached by inheriting the wildcard'sapi_base.proxy_config.yaml, one wildcard route standing in for any deployment that matches the model string being added:Store the credential the Add Model page will select:
Before (205a5e9)
Credential selected for a new model
No credential selected
After (5661b20)
Credential selected for a new model
The borrowed
api_baseis gone from the echoed params.No credential selected
Type
🐛 Bug Fix
Caveats (if any)
Medium
allow_client_side_credentialsLow
Final Attestation