Skip to content

fix(auth): canonicalize inherited Codex refresh state - #64572

Open
Thomisso wants to merge 1 commit into
NousResearch:mainfrom
Thomisso:fix/codex-canonical-profile-auth
Open

fix(auth): canonicalize inherited Codex refresh state#64572
Thomisso wants to merge 1 commit into
NousResearch:mainfrom
Thomisso:fix/codex-canonical-profile-auth

Conversation

@Thomisso

Copy link
Copy Markdown

What does this PR do?

Makes the default/root Hermes auth store the canonical source for inherited openai-codex OAuth state across named profiles.

Codex refresh tokens rotate and are single-use. A named profile could previously read an inherited root grant, refresh it while locking or persisting the active profile store, and leave root or another in-memory pool snapshot holding stale state. That can cause refresh_token_reused, recreate local profile shadows, or roll a separately refreshed pool row backward.

This ports and completes the canonical-root direction proposed in #28277 on current main, including the credential-pool path requested by its review. It complements #41705 without scanning sibling profile stores, and extends the root write-through landed in #52760 with canonical ownership, locking, and stale-snapshot-safe persistence.

Related Issue

Fixes #6653

Related: #28277, #41705, #52760

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✅ Tests (adding or improving test coverage)

Changes Made

  • hermes_cli/auth.py
    • preserve the exact auth-store source used by inherited Codex singleton and pool reads
    • lock and persist refreshes against that canonical source path
    • make nested file-lock reentrancy path-scoped so profile and root locks are never conflated
    • add source-aware credential-pool reads/writes
    • perform a baseline-aware three-way field merge under the canonical lock, preserving independent on-disk token rotations while applying local status/priority changes and explicit removals
    • allow direct Codex resolution to consume root pool-only credentials
  • agent/credential_pool.py
    • carry the canonical Codex auth source path through pool lifetime
    • re-read the exact pool row after taking the canonical lock before spending a rotating refresh token
    • persist refresh, status, quarantine, and removal changes to the canonical store without creating profile-local shadows
    • adopt the merged canonical rows after writes so subsequent mutations use a current baseline
  • Regression tests cover inherited singleton and pool refresh, malformed-token quarantine, same-row and different-row stale snapshots, concurrent rotation plus removal/reprioritization, root pool-only resolution, path-scoped locks, and profile-shadow prevention.

How to Test

python -m pytest tests/hermes_cli/test_auth*.py tests/agent/test_credential_pool*.py -q -o 'addopts='
# 449 passed

python -m ruff check hermes_cli/auth.py agent/credential_pool.py \
  tests/hermes_cli/test_auth_profile_fallback.py \
  tests/hermes_cli/test_auth_codex_provider.py \
  tests/agent/test_credential_pool_oauth_writethrough.py
# All checks passed

python scripts/check-windows-footguns.py hermes_cli/auth.py agent/credential_pool.py \
  tests/hermes_cli/test_auth_profile_fallback.py \
  tests/hermes_cli/test_auth_codex_provider.py \
  tests/agent/test_credential_pool_oauth_writethrough.py
# No Windows footguns found

git diff --check origin/main...HEAD

A live Linux/ARM64 multi-profile deployment was also migrated to one root Codex pool. Five independent profile invocations returned successful Codex responses, named-profile stores remained shadow-free after the calls, and all persistent gateways remained healthy.

The full repository suite was attempted on the ARM64 Raspberry Pi host but was stopped at 16% after roughly nine minutes because it was not practical to complete there. CI should provide the complete repository result; the entire affected auth and credential-pool suite passed.

Checklist

Code

  • I've read the Contributing Guide
  • My commit message follows Conventional Commits
  • I searched open and closed related PRs/issues
  • My PR contains only related changes
  • I've run the entire pytest tests/ -q suite locally (affected suite: 449 passed; full suite deferred to CI)
  • I've added regression tests
  • Tested on Linux ARM64 (Raspberry Pi)

Documentation & Housekeeping

  • Documentation update: N/A; internal auth persistence semantics, no new user-facing configuration
  • cli-config.yaml.example: N/A; no config keys added
  • CONTRIBUTING.md / AGENTS.md: N/A; no contributor workflow change
  • Cross-platform impact considered; platform-neutral file/path logic and Windows-footgun checks pass
  • Tool descriptions/schemas: N/A

Design lineage

Credit to #28277 for proposing the canonical-root Codex store direction. This PR is a current-main completion of that approach across both singleton and credential-pool paths, incorporating the maintainer review request to retain current pool behavior and avoid sibling-store freshness heuristics.

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard provider/openai OpenAI / Codex Responses API area/auth Authentication, OAuth, credential pools P2 Medium — degraded but workaround exists sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data codex labels Jul 14, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for carrying the canonical-source approach through both Codex singleton and credential-pool paths.

Current main still has the reported ownership gap: inherited singleton resolution reads through _load_provider_state() after locking the active store (hermes_cli/auth.py:3238-3243), while _save_codex_tokens() persists through that active store (hermes_cli/auth.py:3386-3407). The pool path likewise falls back to root (hermes_cli/auth.py:1361-1366) but persists to the active store (hermes_cli/auth.py:1389-1417); the existing regression even asserts the resulting profile copy at tests/agent/test_credential_pool_oauth_writethrough.py:103-106.

The PR preserves the accepted root-fallback model while making Codex ownership explicit (hermes_cli/auth.py:1381-1431 on PR head), targets writes and locks to that source, and adds coverage for root-only refresh, pool-row reload, removal, and stale-snapshot merging. The PR base is present locally, and current post-base auth changes are limited to documentation commit 5fc2d9e64, so this appears suitable for straightforward salvage.

Automated hermes-sweeper review.

@teknium1 teknium1 added 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 16, 2026
@solovision24

Copy link
Copy Markdown

Halo validation: PR #64572 is still OPEN and CONFLICTING with main. I attempted the supported GraphQL updatePullRequestBranch(REBASE) mutation, but the authenticated account solovision24 is forbidden from updating Thomisso's cross-repository branch (maintainerCanModify is true, but this token lacks that permission). Please rebase fix/codex-canonical-profile-auth onto current main, then I can rerun the focused auth/pool suites and close out the Kanban task.

@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Thirty-seven PRs address or reference this Codex OAuth/auth-store complex. Their diffs span CLI-token isolation and recovery, profile/global pool fallback, rotating-token serialization and root write-through, sibling-profile recovery, usage lookup, diagnostics, and adjacent credential-pool concurrency fixes; the remaining #6653 ownership gap is most comprehensively targeted by #64572.

Related pull requests

Duplicates

#15173 and #17480 share the /usage/pool fallback path; #28277, #34121, #39352, #43415, #47221, #47233, and #48364 are shared/canonical Codex-store variants; #48416, #49032, #49127, #50632, #51753, and #52760 are the root write-through chain; #39796 and #41705 are sibling-copy recovery variants. #11512 is contained in #15104, #65844 is contained in #70147, and #34820's narrow refresh-lock portion is contained in #56233.

Suggested consolidation

Author action on #64572: rebase onto current main and rerun the focused singleton/profile/pool ownership suites; this follows its recorded #6653 best-fix status and keep_open/high-salvage review without recommending a merge. Keep #34141 open as the focused #34143 salvage and keep #39796 only pending exact-head comparison with rebased #64572; close #28277, #38273, #41704, #41705, and #49170 as superseded by that canonical-source cut if their distinct reviewed concerns are covered, and close #39222 because its contributor review documents the opposite profile-boundary decision.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I6653(["issue #6653 (open)"])
    I34143(["issue #34143 (open)"])
    P64572["PR #64572 (open)"]
    P64572 -->|best fix| I6653
    P64572 -->|fixes| I34143
    class I6653 open
    class I34143 open
    class P64572 open
    class P64572 best
    class P64572 target
    click I6653 "https://github.com/NousResearch/hermes-agent/issues/6653"
    click I34143 "https://github.com/NousResearch/hermes-agent/issues/34143"
    click P64572 "https://github.com/NousResearch/hermes-agent/pull/64572"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 37 pull requests and 6 issues in this complex. Each diff was read against this issue; Assessment working set: 749 kB of PR diffs, 106 kB of issue/PR text, 51 kB of discussion (65 comments), 52 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

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 codex comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists provider/openai OpenAI / Codex Responses API 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/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] openai-codex reauthentication loop when switching back from local models across Hermes profiles

5 participants