fix(auth): make the first pooled credential the active provider on every auth add branch - #82083
Open
briandevans wants to merge 1 commit into
Open
Conversation
…ery `auth add` branch
`auth_add_command` has six `pool.add_entry()` sites. Four establish an
active provider — openai-codex and xai-oauth via the guarded
`mark_provider_active_if_unset` call, qwen-oauth and minimax-oauth via
login helpers that reach `_save_provider_state` — but two do not:
* the generic API-key branch, which serves every provider, and
* the anthropic OAuth branch, whose `run_hermes_oauth_login_pure()` is
pure and persists nothing, so the pool insert is the only write.
On those two paths the credential lands in the pool and `hermes auth list`
shows it, while `hermes setup` still reports "No inference provider
configured". `_model_section_has_credentials()` in hermes_cli/setup.py
consults `get_active_provider()` or an API-key env var, and neither a
manual API key nor an OAuth login writes an env var. OAuth is the default
auth type for anthropic, so a bare `hermes auth add anthropic` hits this.
`mark_provider_active_if_unset`'s own docstring states the invariant this
restores: adding the very first credential for a provider should make it
the active provider so the setup wizard does not report "No inference
provider configured". Subsequent adds leave the user's choice untouched.
The API-key branch guards on `provider in PROVIDER_REGISTRY` because that
branch also accepts `custom:*` pool keys and `openrouter`, neither of which
is a registry member. `resolve_provider` only honours an `active_provider`
it can find in the registry, so marking a custom pool alias active would
satisfy the wizard while resolution still failed. `openrouter` already
resolves from its own credential-pool tier.
Scope: this restores the wizard/status signal. `resolve_provider`'s
active-provider fallback additionally requires
`get_auth_status(<provider>).get("logged_in")`.
`hermes_cli/web_server.py:10297` and `:12813` share this root cause and are
left for a follow-up; that file has an open PR of mine already.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a credential-activation gap in hermes auth add where two add paths (generic API-key and Anthropic OAuth) wrote credential-pool entries but failed to set active_provider, causing hermes setup / _model_section_has_credentials() to incorrectly report “No inference provider configured” even though hermes auth list showed the credential.
Changes:
- API-key add path now captures “first credential for this provider pool” and, when it’s the first and the provider is in
PROVIDER_REGISTRY, callsmark_provider_active_if_unset(provider). - Anthropic OAuth add path now mirrors the first-credential activation pattern (no registry guard needed since
anthropicis registry-backed). - Adds targeted regression tests covering both positive activation cases plus two invariants: don’t override an existing active provider, and never set
active_providerto acustom:*pool key.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
hermes_cli/auth_commands.py |
Sets active_provider on first credential add for API-key (registry-only) and Anthropic OAuth branches. |
tests/hermes_cli/test_auth_commands.py |
Adds regression tests ensuring active_provider is set for the two affected branches and remains correct in edge cases. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
What does this PR do?
hermes auth addputs a credential in the pool and then, on most branches, marks that provider active. On two branches it doesn't — so the credential lands,hermes auth listshows it, andhermes setupstill says "No inference provider configured".auth_add_commandhas sixpool.add_entry()sites. Four establish an active provider:active_providerfirst_credentialguard →mark_provider_active_if_unset_mark_qwen_oauth_active→_save_provider_state_minimax_oauth_login→_minimax_save_auth_state→_save_provider_statepersist_nous_credentials→_save_provider_stateThe two gaps are the widest-reach paths:
run_hermes_oauth_login_pure()is pure: it returns tokens and persists nothing, so the pool insert is the only write on that path. OAuth is the default auth type foranthropic(_OAUTH_CAPABLE_PROVIDERS), so a barehermes auth add anthropiclands here._model_section_has_credentials()(hermes_cli/setup.py:2445) returns True only onget_active_provider()or an API-key env var. Neither of the above sets either, so the wizard reports the provider missing.This is not a new invariant — the repo states it against itself.
mark_provider_active_if_unset's docstring (hermes_cli/auth.py:1485):And
test_auth_add_xai_oauth_sets_active_provider(tests/hermes_cli/test_auth_commands.py:328) records the same bug on the branch it was already fixed on:The
PROVIDER_REGISTRYguard on the API-key branch is load-bearingThe API-key branch also accepts
custom:*pool keys andopenrouter, neither of which is aPROVIDER_REGISTRYmember.mark_provider_active_if_unsetdoes no validation, and_model_section_has_credentialsreturns True for any truthyget_active_provider()— butresolve_provider(hermes_cli/auth.py:2118) only honours anactive_providerwhen_maybe in PROVIDER_REGISTRY.So marking a
custom:foopool active would silence the wizard while resolution still failed — strictly worse than the current behaviour. Henceif first_credential and provider in PROVIDER_REGISTRY:.openrouteris deliberately outside the registry (see theprovider != "openrouter"special case atauth_commands.py:166) and already resolves through its own credential-pool tier atauth.py:2103-2109, so skipping it is correct rather than an omission.anthropicis a static registry member, so the OAuth branch needs no registry guard.Scope of the claim
This restores the wizard/status signal —
_model_section_has_credentials().resolve_provider's active-provider fallback additionally requiresget_auth_status(<provider>).get("logged_in"); this PR does not change that, and the tests do not claim it.Known follow-up, declared
hermes_cli/web_server.py:10297(dashboard anthropic PKCE pool insert) and:12813(POST /api/credentials/pool, whose own comment says it "Mirrors thehermes auth addbehaviour in auth_commands.py") share this root cause. They are deliberately excluded here: I already have an open PR against that file (#74356), and stacking an unrelated concern onto it would make both harder to review. I'll file the mirror as a sibling PR once that one closes.Related Issue
No filed issue — found while tracing why a fresh
hermes auth add anthropicleaveshermes setupreporting no provider. The strongest precedent is that this exact fix was already shipped on the sibling branches, merged by @teknium1:fix(auth): set active_provider after hermes auth add qwen-oauthfix(auth): set active_provider after hermes auth add google-gemini-clifix(auth): use _save_xai_oauth_tokens in auth_commands to set active_providerfix(auth): keep Codex OAuth pool accounts distinct on add + re-auth (#39236)This PR completes that series on the two branches it never reached.
Type of Change
Changes Made
hermes_cli/auth_commands.py— API-key branch: capturefirst_credential = not pool.entries()before the insert, thenmark_provider_active_if_unset(provider)when it is the first credential andprovider in PROVIDER_REGISTRY.hermes_cli/auth_commands.py— anthropic OAuth branch: samefirst_credentialguard, no registry check needed (anthropicis statically in the registry).tests/hermes_cli/test_auth_commands.py— four tests, inserted next to the existing xai-oauth active-provider tests and following their template (assert on theauth.jsonpayload rather than throughget_active_provider()):test_auth_add_anthropic_oauth_sets_active_providertest_auth_add_api_key_sets_active_providertest_auth_add_does_not_steal_an_existing_active_provider— control: withactive_provideralready set toopenai-codex, neither new marking call overrides it, while both credentials still land in their pools. This is what keeps the change from degenerating into "last add wins".test_auth_add_custom_pool_api_key_leaves_active_provider_unset— pins thePROVIDER_REGISTRYguard so acustom:*pool can never be written as an unresolvableactive_provider.No import churn —
auth_modandPROVIDER_REGISTRYare already imported in this module and used at:344/:165.Sibling-site sweep
git grep -n "\.add_entry(" -- '*.py'on production code returns 8 sites: the 6 inauth_commands.pyin the table above (2 fixed here, 4 already correct) and the 2 inweb_server.pyexcluded above with reasons. No other caller ofpool.add_entryexists.How to Test
Repro on a clean profile:
Same for
hermes auth add deepseek --type api-key.Automated, and verified in both directions:
Before the production change (test file applied to clean
main) — the two positive tests fail on exactly the reported symptom, with the success message printed and noactive_providerwritten:The control and the
custom:*guard test pass in both directions by design — they are invariants, not regressions, and are there so the fix cannot be loosened later.After:
22 passedfor the whole file. Adjacent auth/provider/setup suites also green (211 passed):test_auth_xai_oauth_provider.pyandtest_auth_commands.pyare the only two test files in the repo that invokeauth_add_command, so the behavioural blast radius is fully covered.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass — ran the focused + adjacent suites listed under "How to Test" (233 tests, all green) rather than the full suite; happy to run anything wider on requestDocumentation & Housekeeping
docs/, docstrings) — N/A, no user-facing surface changes; the reasoning lives in code comments at both sitescli-config.yaml.exampleif I added/changed config keys — N/A, no config keysCONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/AHERMES_HOMErather than assuming a home layoutRelated / Positioning
Several open PRs touch
auth_add_commandin nearby regions. None addresses this defect, and none touchespool.add_entryor anything after it, so this is not a duplicate of any of them — though a three-way merge may be needed on the first two if they land first:name=kwarg inside thePooledCredential(...)constructors at both sites.--base-urlflag, changing thebase_url=kwarg on the API-key constructor.open_browser=intorun_hermes_oauth_login_pure().make_first=Trueordering on the openai-codex branch only.active_providerwhen removing the active OAuth provider; the complementary direction of the same invariant.