fix(doctor): skip pluggable provider profiles when a dedicated check exists (#22346)#22529
Closed
wesleysimplicio wants to merge 2 commits into
Closed
Conversation
…exists (NousResearch#22346) Problem ------- `hermes doctor` ran two health checks for Anthropic: a dedicated one with the correct `x-api-key` + `anthropic-version` headers, and a generic Bearer-auth one driven by the pluggable `ProviderProfile` for "anthropic". The generic check called `https://api.anthropic.com/v1/models` with `Authorization: Bearer ...`, which Anthropic answers with HTTP 404, producing a noisy duplicate warning even when the dedicated check passed. Root cause ---------- `hermes_cli/doctor.py:_build_apikey_providers_list` deduplicated profiles against a `_known_canonical` set built from the static list (Z.AI/GLM, Kimi, DeepSeek, …). Providers with their own dedicated check above the generic loop (Anthropic, OpenRouter, Bedrock) were not in that set, so their profiles were appended and ran a second, broken check. Fix --- Add `{"anthropic", "openrouter", "bedrock"}` to the skip set, and also skip profiles whose aliases match any of those names (e.g. `claude`, `claude-oauth` → anthropic). Tests ----- tests/hermes_cli/test_doctor_dedicated_provider_skip.py: - test_build_apikey_providers_list_skips_dedicated_check_providers: asserts the assembled list does not contain anthropic, openrouter, or bedrock entries. - test_build_apikey_providers_list_includes_non_dedicated_providers: sanity guard that legitimate providers (DeepSeek, Z.AI/GLM) survive. Both confirmed via stash-verify (fail pre-fix with anthropic/openrouter leaking, pass post-fix). Fixes NousResearch#22346
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reduces noise in hermes doctor by preventing the generic “API-key providers” Bearer-auth health-check loop from running for providers that already have dedicated checks with custom auth/header requirements (notably Anthropic, plus OpenRouter and Bedrock).
Changes:
- Extend
_build_apikey_providers_list()to treat{"anthropic", "openrouter", "bedrock"}as “known” so their pluggableProviderProfiles are not appended to the generic API-key provider list. - Add an additional skip condition intended to skip profiles based on their aliases.
- Add regression tests asserting that dedicated-check providers don’t appear in the generic provider list and that non-dedicated providers remain.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
hermes_cli/doctor.py |
Skips pluggable provider profiles for providers that already have dedicated doctor checks. |
tests/hermes_cli/test_doctor_dedicated_provider_skip.py |
Adds regression tests around _build_apikey_providers_list() contents. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
260
to
+264
| _label = _pp.display_name or _pp.name | ||
| if _label in _known_names or _pp.name in _known_canonical: | ||
| continue | ||
| if any(_alias in _dedicated_canonical for _alias in (_pp.aliases or ())): | ||
| continue |
Comment on lines
+35
to
+37
| assert not any("bedrock" in name for name in names), ( | ||
| f"Bedrock uses AWS SDK creds, not Bearer auth; generic loop must skip. " | ||
| f"Got: {sorted(names)}" |
Contributor
|
Merged via salvage PR #22801. salvage cherry-picked your commits; authorship preserved. Thanks for the contribution! |
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
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.
Problem
hermes doctorruns two health checks for Anthropic: a dedicated one with the correctx-api-key+anthropic-versionheaders, and a genericAuthorization: Bearer ...one driven by the pluggableProviderProfilefor "anthropic". The generic check hitshttps://api.anthropic.com/v1/modelswith the wrong auth and Anthropic returns HTTP 404, producing a noisy duplicate warning even when the dedicated check passed.Root cause
hermes_cli/doctor.py:_build_apikey_providers_listdeduplicates profiles against_known_canonical, but that set only covers the static list (Z.AI/GLM, Kimi, DeepSeek, …). Providers that already have a dedicated check above the generic loop (Anthropic, OpenRouter, Bedrock) were not in it, so their profiles were appended and ran a second, broken check.Fix
Add
{"anthropic", "openrouter", "bedrock"}to the skip set, and also skip profiles whosealiasesmatch any of those names (soclaude/claude-oauth→ anthropic).Tests
tests/hermes_cli/test_doctor_dedicated_provider_skip.py:test_build_apikey_providers_list_skips_dedicated_check_providers— asserts assembled list excludes anthropic, openrouter, bedrock.test_build_apikey_providers_list_includes_non_dedicated_providers— sanity that DeepSeek and Z.AI/GLM survive.Both confirmed via stash-verify (fail pre-fix with anthropic/openrouter leaking, pass post-fix).
Fixes #22346