Skip to content

fix(credential-pool): isolate manual Codex accounts - #92198

Open
yuzilongleif-collab wants to merge 2 commits into
NousResearch:mainfrom
yuzilongleif-collab:fix/codex-independent-pool-sync-20260822
Open

fix(credential-pool): isolate manual Codex accounts#92198
yuzilongleif-collab wants to merge 2 commits into
NousResearch:mainfrom
yuzilongleif-collab:fix/codex-independent-pool-sync-20260822

Conversation

@yuzilongleif-collab

@yuzilongleif-collab yuzilongleif-collab commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Restrict runtime Codex auth-store resync to the singleton-seeded device_code entry. Independent manual:device_code pool credentials no longer adopt the active singleton's token pair.

Root cause

Commit 7380b48589 widened _sync_codex_entry_from_auth_store() to every manual:device_code entry so a refresh-token-only singleton recovery could reach manually added credentials. That source string also identifies independent accounts created by hermes auth add openai-codex.

During a manual account's refresh path, _refresh_entry() calls _sync_codex_entry_from_auth_store() first. The broadened guard therefore copies providers.openai-codex.tokens into the independent entry and persists it before refresh. Repeating this across the pool collapses distinct labels onto the currently active account.

This regresses the independent-account invariant fixed by #39236 / #42316. The maintainer review on #41704 also states that singleton auth-store sync must remain restricted to device_code; manual-account healing needs an identity-matched path instead of source-only singleton adoption.

Change

  • agent/credential_pool.py: accept only entry.source == "device_code" in _sync_codex_entry_from_auth_store().
  • tests/agent/test_credential_pool.py: add paired coverage proving an independent manual:device_code entry is not overwritten, while the seeded device_code entry still adopts a rotated refresh token when the singleton has no access token.

The refresh-token-only adoption added for #70097 remains unchanged for the singleton-seeded device_code entry.

Proof of detection

The manual-account isolation test is RED on the base and GREEN here. The seeded refresh-token-only recovery test is intentionally GREEN on both base and head: it is characterization coverage that prevents the fix from swinging too far and deleting the original #70097 recovery.

After the added symmetric coverage:

75 passed in 8.10s

Command:

python -m pytest -q \
  tests/agent/test_credential_pool.py \
  tests/hermes_cli/test_auth_codex_provider.py \
  tests/agent/test_credential_pool_oauth_writethrough.py

git diff --check also passes.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint provider/openai OpenAI / Codex Responses API area/auth Authentication, OAuth, credential pools sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Aug 22, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

Correctly scoped revert of an over-broad widening: 7380b48589 extended _sync_codex_entry_from_auth_store() to every manual:device_code entry to serve one recovery case (refresh-token-only singleton), but that source string also identifies independent accounts created via hermes auth add openai-codex — so a manual account's refresh path could adopt the active singleton's token pair, silently replacing accounts. Narrowing back to the seeded device_code entry restores the shadowing semantics (singleton shadows only its seeded slot) while keeping that original recovery working; the regression test drives the real pool against a temp auth store and asserts the manual entry's tokens survive.

One verification worth stating in the description:

  1. The original motivating case for 7380b48 (singleton whose refresh token was recovered from the CLI auth store) should get an explicit test here too — narrowing fixes the over-reach, but pinning both behaviors is what prevents the next swing of the pendulum from re-breaking either side.

Cover the refresh-token-only singleton recovery preserved by NousResearch#92198 alongside the manual-account isolation regression. This characterization is green on both the PR base and head and prevents a future fix from narrowing away the seeded device_code recovery path.
@yuzilongleif-collab

Copy link
Copy Markdown
Contributor Author

Thanks — added the symmetric pin in acefdba518.

The new test covers the original #70097 shape directly: the seeded device_code entry has a stale access/refresh pair, while the singleton has only a rotated refresh token. It asserts that sync preserves the existing access token, adopts the rotated refresh token, clears exhausted/error state, and persists the same result.

This is intentionally a characterization test: it is green on both PR base a4f16e3fef and current head. The manual-account isolation test remains the RED→GREEN detector. The affected suite is now 75 passed.

A separate gap remains for cross-process resync of independent manual:device_code rows by entry identity; that needs its own scoped follow-up rather than widening singleton adoption again.

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 P3 Low — cosmetic, nice to have provider/openai OpenAI / Codex Responses API sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants