mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-09 08:21:50 +00:00
fix(auth): add Codex OAuth accounts as distinct pool entries
hermes auth add openai-codex now creates an independent manual:device_code pool entry per account instead of routing through the singleton _save_codex_tokens save path, which collapsed every added account into the latest login (the second add overwrote the first account's singleton-mirrored device_code entry). This is the add-path half of #39236; PR #39243 (already on this branch) fixes the re-auth half. manual:device_code entries refresh from their own token pair (_sync_codex_entry_from_auth_store only adopts the singleton for source=="device_code"), so they need no providers.openai-codex shadow. Adding the first credential marks openai-codex active (the singleton path did this implicitly) so the setup wizard's get_active_provider() check still passes; subsequent adds leave the active provider untouched. Adds SOURCE_MANUAL_DEVICE_CODE constant and a regression test that two distinct accounts keep distinct token pairs. Updates two existing add tests to the pool-only behavior. Co-authored-by: glesperance <info@glesperance.com>
This commit is contained in:
parent
761b744abb
commit
c78b3e1d3c
5 changed files with 130 additions and 12 deletions
|
|
@ -91,6 +91,7 @@ AUTH_TYPE_OAUTH = "oauth"
|
|||
AUTH_TYPE_API_KEY = "api_key"
|
||||
|
||||
SOURCE_MANUAL = "manual"
|
||||
SOURCE_MANUAL_DEVICE_CODE = f"{SOURCE_MANUAL}:device_code"
|
||||
|
||||
STRATEGY_FILL_FIRST = "fill_first"
|
||||
STRATEGY_ROUND_ROBIN = "round_robin"
|
||||
|
|
|
|||
|
|
@ -1182,6 +1182,24 @@ def _store_provider_state(
|
|||
auth_store["active_provider"] = provider_id
|
||||
|
||||
|
||||
def mark_provider_active_if_unset(provider_id: str) -> None:
|
||||
"""Set ``active_provider`` to *provider_id* only when none is set yet.
|
||||
|
||||
Used by ``hermes auth add`` OAuth paths that create credential-pool
|
||||
entries directly (no singleton ``providers.<id>`` block). Adding the
|
||||
very first credential for a provider should make it the active provider
|
||||
so the setup wizard's ``_model_section_has_credentials()`` check (which
|
||||
consults ``get_active_provider()``) does not report "No inference
|
||||
provider configured". Subsequent adds for an already-active setup leave
|
||||
the user's chosen active provider untouched.
|
||||
"""
|
||||
with _auth_store_lock():
|
||||
auth_store = _load_auth_store()
|
||||
if not (auth_store.get("active_provider") or "").strip():
|
||||
auth_store["active_provider"] = provider_id
|
||||
_save_auth_store(auth_store)
|
||||
|
||||
|
||||
def is_known_auth_provider(provider_id: str) -> bool:
|
||||
normalized = (provider_id or "").strip().lower()
|
||||
return normalized in PROVIDER_REGISTRY or normalized in SERVICE_PROVIDER_NAMES
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ from agent.credential_pool import (
|
|||
AUTH_TYPE_OAUTH,
|
||||
CUSTOM_POOL_PREFIX,
|
||||
SOURCE_MANUAL,
|
||||
SOURCE_MANUAL_DEVICE_CODE,
|
||||
STATUS_EXHAUSTED,
|
||||
STRATEGY_FILL_FIRST,
|
||||
STRATEGY_ROUND_ROBIN,
|
||||
|
|
@ -312,15 +313,35 @@ def auth_add_command(args) -> None:
|
|||
creds["tokens"]["access_token"],
|
||||
_oauth_default_label(provider, len(pool.entries()) + 1),
|
||||
)
|
||||
auth_mod._save_codex_tokens(
|
||||
creds["tokens"],
|
||||
last_refresh=creds.get("last_refresh"),
|
||||
# Add a distinct, self-contained pool entry per account (matching the
|
||||
# xai-oauth / google-gemini-cli / qwen-oauth patterns) instead of
|
||||
# routing through the singleton ``_save_codex_tokens`` save path.
|
||||
# The singleton round-trip collapsed every added account into the
|
||||
# latest login: a second ``hermes auth add openai-codex`` overwrote
|
||||
# the first account's singleton-mirrored ``device_code`` entry rather
|
||||
# than creating an independent one (#39236). ``manual:device_code``
|
||||
# entries refresh from their own token pair, so they need no singleton
|
||||
# shadow.
|
||||
entry = PooledCredential(
|
||||
provider=provider,
|
||||
id=uuid.uuid4().hex[:6],
|
||||
label=label,
|
||||
auth_type=AUTH_TYPE_OAUTH,
|
||||
priority=0,
|
||||
source=SOURCE_MANUAL_DEVICE_CODE,
|
||||
access_token=creds["tokens"]["access_token"],
|
||||
refresh_token=creds["tokens"].get("refresh_token"),
|
||||
base_url=creds.get("base_url"),
|
||||
last_refresh=creds.get("last_refresh"),
|
||||
)
|
||||
pool = load_pool(provider)
|
||||
entry = next((item for item in pool.entries() if item.source == "device_code"), None)
|
||||
shown_label = entry.label if entry is not None else label
|
||||
print(f'Saved {provider} OAuth device-code credentials: "{shown_label}"')
|
||||
first_credential = not pool.entries()
|
||||
pool.add_entry(entry)
|
||||
# Adding the first Codex credential should make it the active provider
|
||||
# (the old singleton save path did this implicitly via
|
||||
# _save_provider_state). Subsequent adds leave the active provider as-is.
|
||||
if first_credential:
|
||||
auth_mod.mark_provider_active_if_unset(provider)
|
||||
print(f'Added {provider} OAuth credential #{len(pool.entries())}: "{entry.label}"')
|
||||
return
|
||||
|
||||
if provider == "xai-oauth":
|
||||
|
|
|
|||
|
|
@ -66,6 +66,7 @@ AUTHOR_MAP = {
|
|||
"129007007+HeLLGURD@users.noreply.github.com": "HeLLGURD",
|
||||
"290859878+synapsesx@users.noreply.github.com": "synapsesx",
|
||||
"dirtyren@users.noreply.github.com": "dirtyren",
|
||||
"ted.malone@outlook.com": "temalo",
|
||||
"adityamalik2833@gmail.com": "alarcritty",
|
||||
"islam666@users.noreply.github.com": "islam666",
|
||||
"25539605+lsaether@users.noreply.github.com": "lsaether",
|
||||
|
|
|
|||
|
|
@ -397,15 +397,92 @@ def test_auth_add_codex_oauth_persists_pool_entry(tmp_path, monkeypatch):
|
|||
|
||||
payload = json.loads((tmp_path / "hermes" / "auth.json").read_text())
|
||||
entries = payload["credential_pool"]["openai-codex"]
|
||||
entry = next(item for item in entries if item["source"] == "device_code")
|
||||
# The add path now creates a distinct, self-contained ``manual:device_code``
|
||||
# pool entry per account instead of routing through the singleton save path
|
||||
# (which collapsed multiple accounts into the latest login — #39236).
|
||||
entry = next(item for item in entries if item["source"] == "manual:device_code")
|
||||
assert payload["active_provider"] == "openai-codex"
|
||||
assert payload["providers"]["openai-codex"]["tokens"]["access_token"] == token
|
||||
# No singleton ``providers.openai-codex`` block is written by the add path.
|
||||
assert "openai-codex" not in payload.get("providers", {})
|
||||
assert entry["label"] == "codex@example.com"
|
||||
assert entry["source"] == "device_code"
|
||||
assert entry["source"] == "manual:device_code"
|
||||
assert entry["access_token"] == token
|
||||
assert entry["refresh_token"] == "refresh-token"
|
||||
assert entry["base_url"] == "https://chatgpt.com/backend-api/codex"
|
||||
|
||||
|
||||
def test_auth_add_codex_oauth_keeps_distinct_pool_accounts(tmp_path, monkeypatch):
|
||||
"""Two ``hermes auth add openai-codex`` runs for different ChatGPT
|
||||
accounts must produce two independent pool entries with distinct tokens.
|
||||
|
||||
Regression for #39236: the add path used to route through the singleton
|
||||
``_save_codex_tokens`` save, so the second login overwrote the first
|
||||
account's singleton-mirrored ``device_code`` entry instead of adding a
|
||||
second independent one. ``hermes auth list`` showed two labels sharing
|
||||
one token pair, and rotation silently always used the latest account.
|
||||
"""
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes"))
|
||||
_write_auth_store(tmp_path, {"version": 1, "providers": {}})
|
||||
first_token = _jwt_with_email("first-codex@example.com")
|
||||
second_token = _jwt_with_email("second-codex@example.com")
|
||||
logins = iter(
|
||||
[
|
||||
{
|
||||
"tokens": {
|
||||
"access_token": first_token,
|
||||
"refresh_token": "first-refresh-token",
|
||||
},
|
||||
"base_url": "https://chatgpt.com/backend-api/codex",
|
||||
"last_refresh": "2026-03-23T10:00:00Z",
|
||||
},
|
||||
{
|
||||
"tokens": {
|
||||
"access_token": second_token,
|
||||
"refresh_token": "second-refresh-token",
|
||||
},
|
||||
"base_url": "https://chatgpt.com/backend-api/codex",
|
||||
"last_refresh": "2026-03-23T10:05:00Z",
|
||||
},
|
||||
]
|
||||
)
|
||||
monkeypatch.setattr("hermes_cli.auth._codex_device_code_login", lambda: next(logins))
|
||||
|
||||
from hermes_cli.auth_commands import auth_add_command
|
||||
from agent.credential_pool import load_pool
|
||||
|
||||
class _Args:
|
||||
provider = "openai-codex"
|
||||
auth_type = "oauth"
|
||||
api_key = None
|
||||
label = None
|
||||
|
||||
auth_add_command(_Args())
|
||||
auth_add_command(_Args())
|
||||
|
||||
pool = load_pool("openai-codex")
|
||||
entries = pool.entries()
|
||||
|
||||
assert [entry.source for entry in entries] == [
|
||||
"manual:device_code",
|
||||
"manual:device_code",
|
||||
]
|
||||
assert [entry.label for entry in entries] == [
|
||||
"first-codex@example.com",
|
||||
"second-codex@example.com",
|
||||
]
|
||||
assert [entry.access_token for entry in entries] == [first_token, second_token]
|
||||
assert [entry.refresh_token for entry in entries] == [
|
||||
"first-refresh-token",
|
||||
"second-refresh-token",
|
||||
]
|
||||
|
||||
payload = json.loads((tmp_path / "hermes" / "auth.json").read_text())
|
||||
# No singleton block — the add path is now pool-only.
|
||||
assert "openai-codex" not in payload.get("providers", {})
|
||||
# First add activated the provider; second add left it as-is.
|
||||
assert payload["active_provider"] == "openai-codex"
|
||||
|
||||
|
||||
def test_auth_add_xai_oauth_sets_active_provider(tmp_path, monkeypatch):
|
||||
"""hermes auth add xai-oauth must write providers singleton and set active_provider.
|
||||
|
||||
|
|
@ -1313,9 +1390,9 @@ def test_auth_add_codex_clears_suppression_marker(tmp_path, monkeypatch):
|
|||
payload = json.loads((hermes_home / "auth.json").read_text())
|
||||
# Suppression marker must be cleared
|
||||
assert "openai-codex" not in payload.get("suppressed_sources", {})
|
||||
# New pool entry must be present
|
||||
# New pool entry must be present (distinct manual:device_code entry — #39236)
|
||||
entries = payload["credential_pool"]["openai-codex"]
|
||||
assert any(e["source"] == "device_code" for e in entries)
|
||||
assert any(e["source"] == "manual:device_code" for e in entries)
|
||||
assert payload["active_provider"] == "openai-codex"
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue