fix(auth): preserve independent Codex pool entries on re-auth (#39236)
The #33538 fix refreshed every credential_pool entry with source "manual:device_code" on every Codex OAuth re-auth, on the assumption that such entries were always legacy aliases of the singleton from the #33000 workaround era. That assumption is no longer true: `hermes auth add openai-codex` also produces "manual:device_code" entries for independent ChatGPT accounts, and the broad sync silently clobbered them with the latest-authenticated token pair (labels preserved, token material overwritten, status / quota readings then lie). Narrow the sync: refresh a "manual:device_code" entry only when its existing access_token matches the previous singleton access_token (true legacy alias). Entries with distinct token material represent independent accounts and are now left alone. Error markers are cleared only on entries actually rewritten, so an independent account's own 429 / 401 state survives a re-auth that targeted a different account. Tests: * New: independent acctB/acctC are not overwritten when acctA re-auths. * New: legacy singleton-alias still refreshed (preserves #33538). * New: missing previous singleton state handled (no crash, no false alias match). * New: access_token-only alias match (legacy schema without refresh_token still recognized). * New: error markers cleared only on entries actually refreshed. * Updated: existing manual-device-code sync test now covers both the legacy-alias path AND the independent-account path in one fixture. Behaviour change is zero for users with a single Codex account and zero for users whose only "manual:device_code" entry is the legacy alias of the singleton. Users with multiple independent Codex accounts added via `hermes auth add` now keep their distinct token material across re-auths. Local: 29 passed in tests/hermes_cli/test_auth_codex_provider.py, no new failures in tests/hermes_cli/ vs upstream/main baseline. Fixes #39236.
This commit is contained in:
Ted Malone
@@ -301,19 +301,23 @@ def test_save_codex_tokens_syncs_credential_pool(tmp_path, monkeypatch):
|
||||
|
||||
|
||||
def test_save_codex_tokens_syncs_manual_device_code_entries(tmp_path, monkeypatch):
|
||||
"""Re-auth must also refresh ``manual:device_code`` pool entries.
|
||||
"""Re-auth must refresh ``manual:device_code`` entries that are true
|
||||
aliases of the singleton, while leaving INDEPENDENT entries alone.
|
||||
|
||||
Regression for #33538: a user who hit #33000 before the #33164 fix landed
|
||||
would have run ``hermes auth add openai-codex`` as a workaround, leaving
|
||||
a pool entry with ``source="manual:device_code"``. On every subsequent
|
||||
re-auth via setup/model picker, the singleton-seeded ``device_code`` entry
|
||||
got refreshed but the ``manual:device_code`` entry stayed stale, recreating
|
||||
the same 401 token_invalidated symptom that #33164 was supposed to fix.
|
||||
Original regression for #33538: a user who hit #33000 before the #33164
|
||||
fix landed would have run ``hermes auth add openai-codex`` as a
|
||||
workaround, leaving a pool entry with ``source="manual:device_code"``.
|
||||
On every subsequent re-auth via setup/model picker, the singleton-seeded
|
||||
``device_code`` entry got refreshed but the ``manual:device_code`` entry
|
||||
stayed stale, recreating the same 401 token_invalidated symptom that
|
||||
#33164 was supposed to fix.
|
||||
|
||||
An interactive Codex device-code re-auth proves the user owns the ChatGPT
|
||||
account, so it is safe to refresh every device-code-backed entry in the
|
||||
pool — but NOT independent ``manual:api_key`` entries (separate accounts /
|
||||
explicit API keys).
|
||||
Narrowed for #39236: the original fix treated every ``manual:device_code``
|
||||
entry as a singleton-alias and refreshed them all, which silently
|
||||
clobbered independent accounts added via ``hermes auth add openai-codex``.
|
||||
The current behavior refreshes only entries whose access_token matches
|
||||
the *previous* singleton access_token (true legacy aliases), and leaves
|
||||
distinct-token entries alone (independent accounts).
|
||||
"""
|
||||
hermes_home = tmp_path / "hermes"
|
||||
hermes_home.mkdir(parents=True, exist_ok=True)
|
||||
@@ -335,16 +339,30 @@ def test_save_codex_tokens_syncs_manual_device_code_entries(tmp_path, monkeypatc
|
||||
"access_token": "old-at",
|
||||
"refresh_token": "old-rt",
|
||||
},
|
||||
# Legacy alias from the #33000 workaround era — its tokens
|
||||
# match the singleton, so it is a true alias and SHOULD be
|
||||
# refreshed (preserves #33538 behavior).
|
||||
{
|
||||
"id": "auth-add",
|
||||
"id": "legacy-alias",
|
||||
"source": "manual:device_code",
|
||||
"auth_type": "oauth",
|
||||
"access_token": "stale-manual-at",
|
||||
"refresh_token": "stale-manual-rt",
|
||||
"access_token": "old-at",
|
||||
"refresh_token": "old-rt",
|
||||
"last_status": "exhausted",
|
||||
"last_error_code": 401,
|
||||
"last_error_reason": "token_invalidated",
|
||||
},
|
||||
# Independent account from `hermes auth add openai-codex` —
|
||||
# its tokens are distinct from the singleton. Must NOT be
|
||||
# overwritten by a re-auth that targeted a different account
|
||||
# (#39236).
|
||||
{
|
||||
"id": "independent",
|
||||
"source": "manual:device_code",
|
||||
"auth_type": "oauth",
|
||||
"access_token": "independent-at",
|
||||
"refresh_token": "independent-rt",
|
||||
},
|
||||
{
|
||||
"id": "api-key",
|
||||
"source": "manual:api_key",
|
||||
@@ -363,18 +381,23 @@ def test_save_codex_tokens_syncs_manual_device_code_entries(tmp_path, monkeypatc
|
||||
pool = auth["credential_pool"]["openai-codex"]
|
||||
|
||||
# Singleton-seeded device_code entry: refreshed and error markers cleared.
|
||||
seeded = next(e for e in pool if e["source"] == "device_code")
|
||||
seeded = next(e for e in pool if e["id"] == "seeded")
|
||||
assert seeded["access_token"] == "fresh-at"
|
||||
assert seeded["refresh_token"] == "fresh-rt"
|
||||
|
||||
# manual:device_code entry: ALSO refreshed (the new behavior).
|
||||
manual_dc = next(e for e in pool if e["source"] == "manual:device_code")
|
||||
assert manual_dc["access_token"] == "fresh-at"
|
||||
assert manual_dc["refresh_token"] == "fresh-rt"
|
||||
assert manual_dc["last_refresh"] == "2026-05-28T00:00:00Z"
|
||||
assert manual_dc["last_status"] is None
|
||||
assert manual_dc["last_error_code"] is None
|
||||
assert manual_dc["last_error_reason"] is None
|
||||
# Legacy alias (tokens matched previous singleton): ALSO refreshed.
|
||||
legacy = next(e for e in pool if e["id"] == "legacy-alias")
|
||||
assert legacy["access_token"] == "fresh-at"
|
||||
assert legacy["refresh_token"] == "fresh-rt"
|
||||
assert legacy["last_refresh"] == "2026-05-28T00:00:00Z"
|
||||
assert legacy["last_status"] is None
|
||||
assert legacy["last_error_code"] is None
|
||||
assert legacy["last_error_reason"] is None
|
||||
|
||||
# Independent manual:device_code entry: NOT overwritten (#39236).
|
||||
independent = next(e for e in pool if e["id"] == "independent")
|
||||
assert independent["access_token"] == "independent-at"
|
||||
assert independent["refresh_token"] == "independent-rt"
|
||||
|
||||
# manual:api_key entry: untouched — independent credential.
|
||||
api_key = next(e for e in pool if e["source"] == "manual:api_key")
|
||||
@@ -382,6 +405,333 @@ def test_save_codex_tokens_syncs_manual_device_code_entries(tmp_path, monkeypatc
|
||||
assert "refresh_token" not in api_key or api_key.get("refresh_token") is None
|
||||
|
||||
|
||||
def test_save_codex_tokens_does_not_overwrite_independent_manual_entries(tmp_path, monkeypatch):
|
||||
"""Re-auth must NOT overwrite ``manual:device_code`` entries that hold
|
||||
independent token material (different OpenAI/ChatGPT accounts).
|
||||
|
||||
Regression for #39236: ``hermes auth add openai-codex`` for accounts B and C
|
||||
routes through ``_save_codex_tokens`` because the singleton path is the
|
||||
only Codex OAuth save flow. The #33538 fix refreshed every
|
||||
``manual:device_code`` entry on every re-auth, which works fine for the
|
||||
one-account/legacy-workaround case but silently overwrote distinct
|
||||
independent accounts with the latest-authenticated tokens (labels
|
||||
preserved, token material clobbered, status/quota readings then lie).
|
||||
|
||||
The safe invariant: an entry is a singleton-alias only when its current
|
||||
access_token matches the *previous* singleton access_token. Manual
|
||||
entries whose tokens never matched the singleton are independent accounts
|
||||
and must be left alone.
|
||||
"""
|
||||
hermes_home = tmp_path / "hermes"
|
||||
hermes_home.mkdir(parents=True, exist_ok=True)
|
||||
(hermes_home / "auth.json").write_text(json.dumps({
|
||||
"version": 1,
|
||||
"providers": {
|
||||
"openai-codex": {
|
||||
# Old singleton tokens — represent "account A" which the user
|
||||
# logged in with via setup originally.
|
||||
"tokens": {"access_token": "acctA-at", "refresh_token": "acctA-rt"},
|
||||
"last_refresh": "2026-01-01T00:00:00Z",
|
||||
"auth_mode": "chatgpt",
|
||||
"label": "account-A",
|
||||
},
|
||||
},
|
||||
"credential_pool": {
|
||||
"openai-codex": [
|
||||
# The seeded singleton mirror of account A.
|
||||
{
|
||||
"id": "seeded",
|
||||
"label": "account-A",
|
||||
"source": "device_code",
|
||||
"auth_type": "oauth",
|
||||
"access_token": "acctA-at",
|
||||
"refresh_token": "acctA-rt",
|
||||
},
|
||||
# Two INDEPENDENT manual entries added later via
|
||||
# ``hermes auth add openai-codex`` (account B and account C).
|
||||
# Each has its OWN distinct token material, unrelated to the
|
||||
# singleton.
|
||||
{
|
||||
"id": "acctB",
|
||||
"label": "account-B",
|
||||
"source": "manual:device_code",
|
||||
"auth_type": "oauth",
|
||||
"access_token": "acctB-at",
|
||||
"refresh_token": "acctB-rt",
|
||||
},
|
||||
{
|
||||
"id": "acctC",
|
||||
"label": "account-C",
|
||||
"source": "manual:device_code",
|
||||
"auth_type": "oauth",
|
||||
"access_token": "acctC-at",
|
||||
"refresh_token": "acctC-rt",
|
||||
},
|
||||
],
|
||||
},
|
||||
}))
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
|
||||
# User re-authenticates account A — fresh device-code login produces new
|
||||
# tokens. The legitimate update is the seeded singleton mirror; the
|
||||
# independent acctB/acctC entries must be untouched.
|
||||
_save_codex_tokens(
|
||||
{"access_token": "acctA-new-at", "refresh_token": "acctA-new-rt"},
|
||||
last_refresh="2026-06-05T00:00:00Z",
|
||||
)
|
||||
|
||||
auth = json.loads((hermes_home / "auth.json").read_text())
|
||||
pool = auth["credential_pool"]["openai-codex"]
|
||||
|
||||
# Singleton-seeded entry: refreshed (legitimate sync).
|
||||
seeded = next(e for e in pool if e["source"] == "device_code")
|
||||
assert seeded["access_token"] == "acctA-new-at"
|
||||
assert seeded["refresh_token"] == "acctA-new-rt"
|
||||
assert seeded["last_refresh"] == "2026-06-05T00:00:00Z"
|
||||
|
||||
# acctB: INDEPENDENT entry — must NOT be overwritten.
|
||||
acctB = next(e for e in pool if e["id"] == "acctB")
|
||||
assert acctB["access_token"] == "acctB-at", (
|
||||
"acctB was clobbered by acctA re-auth (#39236 regression)"
|
||||
)
|
||||
assert acctB["refresh_token"] == "acctB-rt"
|
||||
|
||||
# acctC: INDEPENDENT entry — must NOT be overwritten.
|
||||
acctC = next(e for e in pool if e["id"] == "acctC")
|
||||
assert acctC["access_token"] == "acctC-at", (
|
||||
"acctC was clobbered by acctA re-auth (#39236 regression)"
|
||||
)
|
||||
assert acctC["refresh_token"] == "acctC-rt"
|
||||
|
||||
|
||||
def test_save_codex_tokens_still_refreshes_legacy_manual_alias(tmp_path, monkeypatch):
|
||||
"""The #33538 legacy use case must keep working.
|
||||
|
||||
A user who hit #33000 before the #33164 fix landed might have run
|
||||
``hermes auth add openai-codex`` as a workaround when there was no
|
||||
singleton entry — that created a ``manual:device_code`` pool entry that
|
||||
holds the SAME token material as the (later) singleton. This entry is a
|
||||
true alias of the singleton and SHOULD still be refreshed on subsequent
|
||||
re-auths, otherwise it goes stale and recreates the #33538 symptom.
|
||||
|
||||
The distinguishing signal: a legacy alias has access_token == previous
|
||||
singleton access_token; an independent account does not.
|
||||
"""
|
||||
hermes_home = tmp_path / "hermes"
|
||||
hermes_home.mkdir(parents=True, exist_ok=True)
|
||||
(hermes_home / "auth.json").write_text(json.dumps({
|
||||
"version": 1,
|
||||
"providers": {
|
||||
"openai-codex": {
|
||||
"tokens": {"access_token": "shared-at", "refresh_token": "shared-rt"},
|
||||
"last_refresh": "2026-01-01T00:00:00Z",
|
||||
"auth_mode": "chatgpt",
|
||||
},
|
||||
},
|
||||
"credential_pool": {
|
||||
"openai-codex": [
|
||||
{
|
||||
"id": "seeded",
|
||||
"source": "device_code",
|
||||
"auth_type": "oauth",
|
||||
"access_token": "shared-at",
|
||||
"refresh_token": "shared-rt",
|
||||
},
|
||||
{
|
||||
"id": "legacy",
|
||||
"label": "legacy-alias",
|
||||
"source": "manual:device_code",
|
||||
"auth_type": "oauth",
|
||||
# Token material matches the singleton — this is a true
|
||||
# alias from the #33000 workaround era.
|
||||
"access_token": "shared-at",
|
||||
"refresh_token": "shared-rt",
|
||||
"last_status": "exhausted",
|
||||
"last_error_code": 401,
|
||||
"last_error_reason": "token_invalidated",
|
||||
},
|
||||
],
|
||||
},
|
||||
}))
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
|
||||
_save_codex_tokens(
|
||||
{"access_token": "fresh-at", "refresh_token": "fresh-rt"},
|
||||
last_refresh="2026-06-05T00:00:00Z",
|
||||
)
|
||||
|
||||
auth = json.loads((hermes_home / "auth.json").read_text())
|
||||
pool = auth["credential_pool"]["openai-codex"]
|
||||
|
||||
# Singleton: refreshed.
|
||||
seeded = next(e for e in pool if e["source"] == "device_code")
|
||||
assert seeded["access_token"] == "fresh-at"
|
||||
|
||||
# Legacy alias: still refreshed (preserves #33538 fix).
|
||||
legacy = next(e for e in pool if e["id"] == "legacy")
|
||||
assert legacy["access_token"] == "fresh-at"
|
||||
assert legacy["refresh_token"] == "fresh-rt"
|
||||
assert legacy["last_refresh"] == "2026-06-05T00:00:00Z"
|
||||
# Error markers cleared on the refreshed entry.
|
||||
assert legacy["last_status"] is None
|
||||
assert legacy["last_error_code"] is None
|
||||
assert legacy["last_error_reason"] is None
|
||||
|
||||
|
||||
def test_save_codex_tokens_handles_missing_previous_singleton_tokens(tmp_path, monkeypatch):
|
||||
"""First-ever Codex save (no prior singleton tokens) must not crash.
|
||||
|
||||
Edge case: a user has only pool entries (e.g. via direct auth.json edit
|
||||
or a partial state from a corrupted upgrade), no `providers.openai-codex.tokens`
|
||||
block at all. The previous-singleton-tokens guard must handle missing
|
||||
state gracefully — fall back to "no previous tokens", which means no
|
||||
pool entry can be a true alias and only the singleton-seeded entry gets
|
||||
written.
|
||||
"""
|
||||
hermes_home = tmp_path / "hermes"
|
||||
hermes_home.mkdir(parents=True, exist_ok=True)
|
||||
(hermes_home / "auth.json").write_text(json.dumps({
|
||||
"version": 1,
|
||||
"providers": {},
|
||||
"credential_pool": {
|
||||
"openai-codex": [
|
||||
{
|
||||
"id": "preexisting",
|
||||
"label": "pre-existing-manual",
|
||||
"source": "manual:device_code",
|
||||
"auth_type": "oauth",
|
||||
"access_token": "preexisting-at",
|
||||
"refresh_token": "preexisting-rt",
|
||||
},
|
||||
],
|
||||
},
|
||||
}))
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
|
||||
_save_codex_tokens(
|
||||
{"access_token": "first-at", "refresh_token": "first-rt"},
|
||||
last_refresh="2026-06-05T00:00:00Z",
|
||||
)
|
||||
|
||||
auth = json.loads((hermes_home / "auth.json").read_text())
|
||||
pool = auth["credential_pool"]["openai-codex"]
|
||||
# Pre-existing independent entry with no relationship to a (now-new)
|
||||
# singleton MUST be preserved.
|
||||
pre = next(e for e in pool if e["id"] == "preexisting")
|
||||
assert pre["access_token"] == "preexisting-at"
|
||||
assert pre["refresh_token"] == "preexisting-rt"
|
||||
|
||||
|
||||
def test_save_codex_tokens_alias_match_uses_access_token_only(tmp_path, monkeypatch):
|
||||
"""A manual entry counts as an alias if its access_token matches the
|
||||
previous singleton access_token, regardless of refresh_token presence.
|
||||
|
||||
Some legacy entries (older auth.json schemas, pre-refresh-token versions)
|
||||
have access_token but no refresh_token. These should still be treated as
|
||||
aliases when the access_token matches.
|
||||
"""
|
||||
hermes_home = tmp_path / "hermes"
|
||||
hermes_home.mkdir(parents=True, exist_ok=True)
|
||||
(hermes_home / "auth.json").write_text(json.dumps({
|
||||
"version": 1,
|
||||
"providers": {
|
||||
"openai-codex": {
|
||||
"tokens": {"access_token": "shared-at", "refresh_token": "shared-rt"},
|
||||
"auth_mode": "chatgpt",
|
||||
},
|
||||
},
|
||||
"credential_pool": {
|
||||
"openai-codex": [
|
||||
{
|
||||
"id": "alias-no-refresh",
|
||||
"source": "manual:device_code",
|
||||
"auth_type": "oauth",
|
||||
"access_token": "shared-at",
|
||||
# No refresh_token at all — legacy schema.
|
||||
},
|
||||
],
|
||||
},
|
||||
}))
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
|
||||
_save_codex_tokens(
|
||||
{"access_token": "new-at", "refresh_token": "new-rt"},
|
||||
last_refresh="2026-06-05T00:00:00Z",
|
||||
)
|
||||
|
||||
auth = json.loads((hermes_home / "auth.json").read_text())
|
||||
pool = auth["credential_pool"]["openai-codex"]
|
||||
alias = next(e for e in pool if e["id"] == "alias-no-refresh")
|
||||
# Treated as alias → refreshed with new tokens.
|
||||
assert alias["access_token"] == "new-at"
|
||||
assert alias["refresh_token"] == "new-rt"
|
||||
|
||||
|
||||
def test_save_codex_tokens_clears_error_markers_only_on_refreshed_entries(tmp_path, monkeypatch):
|
||||
"""Error markers must be cleared only on entries that were actually
|
||||
refreshed by this re-auth. Independent ``manual:device_code`` entries
|
||||
with their own stale-error markers must be left alone (their stale state
|
||||
is not the current re-auth's business).
|
||||
"""
|
||||
hermes_home = tmp_path / "hermes"
|
||||
hermes_home.mkdir(parents=True, exist_ok=True)
|
||||
(hermes_home / "auth.json").write_text(json.dumps({
|
||||
"version": 1,
|
||||
"providers": {
|
||||
"openai-codex": {
|
||||
"tokens": {"access_token": "acctA-at", "refresh_token": "acctA-rt"},
|
||||
"auth_mode": "chatgpt",
|
||||
},
|
||||
},
|
||||
"credential_pool": {
|
||||
"openai-codex": [
|
||||
{
|
||||
"id": "seeded",
|
||||
"source": "device_code",
|
||||
"auth_type": "oauth",
|
||||
"access_token": "acctA-at",
|
||||
"refresh_token": "acctA-rt",
|
||||
"last_status": "exhausted",
|
||||
"last_error_code": 401,
|
||||
},
|
||||
{
|
||||
"id": "acctB",
|
||||
"source": "manual:device_code",
|
||||
"auth_type": "oauth",
|
||||
"access_token": "acctB-at",
|
||||
"refresh_token": "acctB-rt",
|
||||
"last_status": "exhausted",
|
||||
"last_error_code": 429,
|
||||
"last_error_reason": "quota_exhausted",
|
||||
},
|
||||
],
|
||||
},
|
||||
}))
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
|
||||
_save_codex_tokens(
|
||||
{"access_token": "fresh-at", "refresh_token": "fresh-rt"},
|
||||
last_refresh="2026-06-05T00:00:00Z",
|
||||
)
|
||||
|
||||
auth = json.loads((hermes_home / "auth.json").read_text())
|
||||
pool = auth["credential_pool"]["openai-codex"]
|
||||
|
||||
# Singleton: refreshed AND error markers cleared.
|
||||
seeded = next(e for e in pool if e["id"] == "seeded")
|
||||
assert seeded["access_token"] == "fresh-at"
|
||||
assert seeded["last_status"] is None
|
||||
assert seeded["last_error_code"] is None
|
||||
|
||||
# Independent acctB: NOT refreshed AND error markers NOT cleared.
|
||||
# (Its 429 quota state belongs to acctB's own account, not acctA's re-auth.)
|
||||
acctB = next(e for e in pool if e["id"] == "acctB")
|
||||
assert acctB["access_token"] == "acctB-at" # not overwritten
|
||||
assert acctB["last_status"] == "exhausted" # not cleared
|
||||
assert acctB["last_error_code"] == 429
|
||||
assert acctB["last_error_reason"] == "quota_exhausted"
|
||||
|
||||
|
||||
def test_import_codex_cli_tokens(tmp_path, monkeypatch):
|
||||
codex_home = tmp_path / "codex-cli"
|
||||
codex_home.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
Reference in New Issue
Block a user