Skip to content

fix(auth): merge credential pool entries on persist, preserve exhaustion on sync - #7202

Closed
mvanhorn wants to merge 1 commit into
NousResearch:mainfrom
mvanhorn:fix/6907-credential-pool-persist
Closed

fix(auth): merge credential pool entries on persist, preserve exhaustion on sync#7202
mvanhorn wants to merge 1 commit into
NousResearch:mainfrom
mvanhorn:fix/6907-credential-pool-persist

Conversation

@mvanhorn

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes two concurrency bugs in the credential pool that cause credential loss and stale token injection when multiple Hermes instances share the same auth store.

  1. _persist() now does a read-merge-write by entry ID instead of replacing the entire provider's entry list. Entries added by another process (e.g., hermes auth add openai-codex --label chatgpt-2 in another terminal) are preserved across persists.

  2. _sync_codex_entry_from_cli() now skips manual-source entries entirely (they use Hermes-native tokens, not CLI tokens) and preserves exhaustion status when syncing - fresh tokens from ~/.codex/auth.json don't mean the provider rate limit has lifted.

Related Issue

Fixes #6907

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Changes Made

  • agent/credential_pool.py: Changed _persist() (line 389) from write_credential_pool() to a merge-by-ID pattern under _auth_store_lock(). Disk entries with IDs not in memory are preserved.
  • agent/credential_pool.py: Added early return in _sync_codex_entry_from_cli() for SOURCE_MANUAL entries. Changed token sync to preserve exhaustion markers (STATUS_EXHAUSTED entries keep their status).
  • tests/agent/test_credential_pool.py: Added 30 tests covering merge persistence, manual entry skip, and exhaustion preservation.

How to Test

  1. Start an interactive hermes session with openai-codex as the provider
  2. In another terminal, run hermes auth add openai-codex --label chatgpt-2
  3. In the first terminal, trigger a rate limit on the primary account
  4. Verify the chatgpt-2 credential survives and rotation works
  5. Run hermes auth list openai-codex to confirm both entries are present

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes
  • I've tested on my platform: macOS 26.3

Documentation & Housekeeping

  • N/A - no config or doc changes needed

This contribution was developed with AI assistance (Codex).

…ion on sync

Two fixes for credential pool concurrency:

1. _persist() now reads disk entries and merges by ID instead of
   replacing the entire provider list. Entries added by other processes
   (e.g., hermes auth add in another terminal) survive a persist from
   a running session.

2. _sync_codex_entry_from_cli() now skips manual-source entries and
   preserves exhaustion status when syncing tokens. Fresh tokens from
   ~/.codex/auth.json don't imply the rate limit has lifted.

Fixes NousResearch#6907
@PavelPancocha

Copy link
Copy Markdown

👍🏻 I want this!

@mvanhorn

Copy link
Copy Markdown
Contributor Author

Thanks @PavelPancocha - covers the exhaustion-on-sync case from #6907. Still waiting on a maintainer pass.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists area/auth Authentication, OAuth, credential pools comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels Apr 29, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused concurrency investigation. This is now redundant on current main.

  • hermes_cli/auth.py:1348-1396 now performs the locked read/merge/write at the shared persistence boundary, preserving concurrently added entries and honoring removed_ids; this shipped in 8b4c29f0f.
  • agent/credential_pool.py:540-545 routes pool persistence through that writer, and tests/agent/test_credential_pool.py:3050-3109 covers the stale-rotation/concurrent-entry case.
  • The Codex CLI synchronization path targeted by this PR was removed by b02833f32 / fix(codex): Hermes owns its own Codex auth; stop touching ~/.codex/auth.json #12360. Current Codex sync is scoped to Hermes-owned device_code auth state (agent/credential_pool.py:653-715), while agent/credential_pool.py:2038-2043 explicitly avoids runtime import from ~/.codex/auth.json.

This is an automated hermes-sweeper review.

@teknium1 teknium1 closed this Jul 12, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 12, 2026
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/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: credential pool _persist overwrites concurrent auth entries and _sync_codex_entry_from_cli rolls back exhaustion status

4 participants