diff --git a/agent/credential_pool.py b/agent/credential_pool.py index 53cc31daf6d..04b22c76a68 100644 --- a/agent/credential_pool.py +++ b/agent/credential_pool.py @@ -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" diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index 954a948f0e3..668200a0a38 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -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.`` 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 diff --git a/hermes_cli/auth_commands.py b/hermes_cli/auth_commands.py index ff03e84408a..f1f87c7703c 100644 --- a/hermes_cli/auth_commands.py +++ b/hermes_cli/auth_commands.py @@ -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": diff --git a/scripts/release.py b/scripts/release.py index 12ad6ed0937..e53c380a2aa 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -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", diff --git a/tests/hermes_cli/test_auth_commands.py b/tests/hermes_cli/test_auth_commands.py index b53e73737ed..1723c11e32c 100644 --- a/tests/hermes_cli/test_auth_commands.py +++ b/tests/hermes_cli/test_auth_commands.py @@ -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"