Skip to content

feat(auth): add credential pool switch command - #45513

Closed
hanzckernel wants to merge 2 commits into
NousResearch:mainfrom
hanzckernel:han/auth-switch-credential
Closed

feat(auth): add credential pool switch command#45513
hanzckernel wants to merge 2 commits into
NousResearch:mainfrom
hanzckernel:han/auth-switch-credential

Conversation

@hanzckernel

Copy link
Copy Markdown
Contributor

Summary

Adds a focused hermes auth switch <provider> <target> command for manually choosing which credential in a same-provider credential pool should be tried first.

The command:

  • accepts the same target forms as hermes auth remove: 1-based index, credential id, or exact label
  • persists the selected credential as priority 0 instead of adding a second active-credential state
  • warns when the configured strategy (least_used, round_robin, or random) may choose a different credential later
  • updates CLI help and credential-pool docs

For openai-codex, switching an OAuth credential also keeps the singleton Codex auth block aligned with the selected pool entry, because some Codex paths still read that singleton state. That path is intentionally guarded so it:

  • promotes the selected entry to the singleton-backed device_code source
  • demotes the previous device_code entry to manual:device_code so the old account is preserved
  • updates pool + singleton state under one auth-store lock
  • preserves active_provider
  • refuses Codex OAuth entries that lack required refresh material, avoiding silent credential loss

Issues / relation to existing work

Fixes #22407.
Fixes #37224.

Related: #22916, #42798, #43747.

This is a smaller, refreshed alternative to stale PR #17527. It keeps the public surface to one command plus docs/tests, and adds extra Codex regression coverage around singleton/pool drift and independent account preservation.

Verification

  • scripts/run_tests.sh tests/hermes_cli/test_auth_commands.py tests/hermes_cli/test_auth_codex_provider.py
    • 84 tests passed
  • python -m py_compile agent/credential_pool.py hermes_cli/auth_commands.py hermes_cli/subcommands/auth.py hermes_cli/_parser.py
  • git diff --check
  • Manual smoke:
    • python -m hermes_cli.main auth switch openrouter second against a temp HERMES_HOME reordered the pool to ['second', 'first']
    • python -m hermes_cli.main auth switch openai-codex account-B against a temp HERMES_HOME promoted account-B to device_code, demoted account-A to manual:device_code, and kept both credentials after load_pool('openai-codex')

Review

Independent review was run after implementation. Initial review found Codex singleton/pool edge cases; those were fixed with additional regression tests. Final focused review passed with no remaining blockers.

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard area/auth Authentication, OAuth, credential pools labels Jun 13, 2026
@liuhao1024

Copy link
Copy Markdown
Contributor

Verification: clean security/reliability review

Reviewed the auth switch command implementation:

  1. Singleton sync for openai-codex — The _switch_codex_pool_and_singleton function correctly atomically promotes the selected OAuth entry to source="device_code" while demoting the previous singleton. The access_token/refresh_token are read from the on-disk pool entry (not the in-memory matched object), which correctly handles the case where another process refreshed the tokens between load and switch.
  2. Refresh token guard — If the selected OAuth entry lacks a refresh_token, the command raises SystemExit with a clear recovery instruction (hermes auth add openai-codex). This prevents promoting a half-configured credential.
  3. Priority reorderingactivate_index uses enumerate(ordered) to assign sequential priorities after reordering, ensuring no priority gaps or collisions.
  4. UX signals — The command warns when the selected credential is exhausted/dead and when the pool uses a non-fill_first strategy (where later requests may select a different credential).

No issues found. LGTM.

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Approved

New CLI command hermes auth switch that moves a pooled credential to the first position in the pool, making it the active credential. Also adds a CredentialPool.activate_index() method.

Design: The approach (reordering the entries list, then re-prioritizing from 0) is sound — it's essentially a list move-to-front operation. Persisting after reordering ensures durability. CLI help text is updated, and the implementation correctly uses the pool lock during the operation.

Code Quality: Clean. The reordering uses enumerate for priority assignment, which is idiomatic. The return of self._entries[0] (the moved credential) is useful for CLI feedback.


Reviewed by Hermes Agent (cron batch)

@hanzckernel

Copy link
Copy Markdown
Contributor Author

Small follow-up after the review: this keeps the core auth switch behavior unchanged, but exposes it through the bare interactive hermes auth menu as well.

The new commit adds:

  • Switch active credential for a provider to the interactive auth menu
  • an interactive switch flow that reuses auth_switch_command
  • a regression test for the interactive menu surface
  • matching credential-pools docs menu numbering

Local focused check:
python -m pytest tests/hermes_cli/test_auth_commands.py -k 'auth_switch or interactive_auth_menu_exposes_switch' -o addopts= -q
→ 6 passed, 53 deselected

@eonewg

eonewg commented Jul 14, 2026

Copy link
Copy Markdown

I tested this PR locally because I’m building a standalone /cred plugin on top of Hermes credential pools. The narrow disk-level primitive is useful, and the focused switch tests passed in my checkout.

A few findings that may help the rebase/review:

  • The branch is now substantially behind main and conflicts around agent/credential_pool.py; current write_credential_pool also has concurrent-add merge semantics that should be preserved during rebase.
  • Plain switch correctly preserves exhausted/dead state. It would be useful to add an explicit regression test for that and for ambiguous duplicate labels.
  • A target-scoped reset is still missing. Today the remediation is provider-wide hermes auth reset <provider>, which clears unrelated credentials too. A follow-up auth reset <provider> [target] or auth switch ... --reset would make manual recovery safer.
  • Long-running Gateway sessions can still retain the previous credential when _session_model_overrides[session_key].api_key was populated by /model. Reordering the on-disk pool does not refresh that pinned per-session key, so docs should at least say that an existing session may need /new/restart. A full live switch needs a core/public session rebind path; plugin command handlers currently do not have enough stable runtime control. PR feat(plugins): propagate session context to plugin hooks #42416 is relevant for passing session context, but a public credential-refresh/invalidation callback would still be needed.
  • For Codex OAuth, a resolved target with missing access-token material currently falls through to the misleading “No credential matching” message. Distinguishing sync/material failure from target resolution would improve diagnostics.

Local evidence: focused switch selection/health/ambiguity tests passed; the two full-file failures on the old PR base were existing Codex pool-only rate-limit tests already addressed on newer main, not switch regressions.

I’m avoiding a competing auth-switch PR because this one already covers the core CLI primitive.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused CLI primitive. Current main still lacks an auth switch dispatch path (hermes_cli/auth_commands.py:756-780), so one-time pool reordering remains useful.

Problems

  • Fixes #37224 overstates the scope. That issue requests a durable credential pin, while this PR explicitly allows round_robin, random, and least_used to select another entry. Current selection confirms those strategies do not use priority as their primary decision (agent/credential_pool.py:1534-1558). Please either implement that pin contract or remove the closing linkage.
  • The docs call the selected entry “the next choice,” but a live Gateway /model override reapplies its cached api_key (gateway/run.py:15975-15991). The disk reorder therefore does not necessarily affect an existing session.
  • The Codex helper has a targeted missing-refresh-token error, but an entry missing access_token falls through to the generic “No credential matching” message.

Suggested changes

  • Document that existing Gateway sessions may require /new or restart.
  • Add the access-token diagnostic and regression coverage, plus ambiguity and health-state preservation coverage.
  • Preserve the current concurrent-add merge behavior in write_credential_pool() (hermes_cli/auth.py:1369-1417) while salvaging the stale branch.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
@eonewg

eonewg commented Jul 14, 2026

Copy link
Copy Markdown

Followed up on the sweeper review in a rebased branch:

  • rebased the PR commits onto current main (46e87b14f)
  • added a targeted missing-access_token diagnostic for Codex OAuth switching
  • documented that non-fill_first strategies may choose another credential and that Gateway sessions with a cached /model API key may need /new or a restart
  • added regression coverage for ambiguous labels, health-state preservation, missing access-token material, and preservation of credentials added concurrently on disk
  • retained current write_credential_pool() concurrent-add merge behavior through the rebase

Branch: https://github.com/eonewg/hermes-agent/tree/review/pr-45513-followup
Follow-up commit: eonewg@c19c2b6e7

Verification:

  • python -m pytest tests/hermes_cli/test_auth_commands.py tests/hermes_cli/test_auth_codex_provider.py -o addopts= -q → 94 passed
  • python -m pytest tests/agent/test_credential_pool.py tests/gateway/test_session_model_override_credential_pool.py -o addopts= -q → 90 passed
  • python -m py_compile agent/credential_pool.py hermes_cli/auth_commands.py hermes_cli/subcommands/auth.py hermes_cli/_parser.py
  • git diff --check

One PR-metadata change remains for the author: please remove Fixes #37224 from the PR body. This implementation provides one-time pool reordering rather than the durable pin contract requested there; Fixes #22407 still matches the focused CLI primitive.

@eonewg

eonewg commented Jul 19, 2026

Copy link
Copy Markdown

Opened #67285 as the current-main salvage/replacement for this PR. It preserves @hanzckernel’s authorship on the original two commits, carries forward the review fixes from review/pr-45513-followup, and is rebased cleanly onto f099b469d. Focused CI-style verification is 186/186 tests passing, plus Ruff, Windows-footgun, bytecode, diff, and CLI smoke checks. This is a continuation of the implementation here rather than a separate competing design; maintainers can route review to #67285 and close whichever PR they prefer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add CLI command to select active pooled credential [Feature]: Allow manually selecting the active credential within a provider credential pool

6 participants