diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index 0d08007ef33e5..5db9dd30610d4 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -4471,8 +4471,17 @@ def _save_xai_oauth_tokens( # grant through _load_provider_state's fallback. When such a profile # refreshes the (rotating) grant, we must write the rotated chain back # to root too, or root is left holding a revoked refresh token (#43589). - write_through_to_root = not _profile_has_own_xai_oauth_state(auth_store) - state = _load_provider_state(auth_store, "xai-oauth") or {} + # #74339: the old key-presence check (_profile_has_own_xai_oauth_state) + # decided write-through based on whether the profile had a + # providers.xai-oauth key BEFORE the save — but _store_provider_state + # unconditionally creates that key below. Use + # _load_provider_state_with_source to learn where the grant was + # resolved from and write back only to that source. + state, source_path = _load_provider_state_with_source( + auth_store, "xai-oauth" + ) + if state is None: + state = {} state["tokens"] = tokens state["last_refresh"] = last_refresh state["auth_mode"] = auth_mode @@ -4480,12 +4489,24 @@ def _save_xai_oauth_tokens( state["discovery"] = discovery if redirect_uri: state["redirect_uri"] = redirect_uri - _store_provider_state( - auth_store, "xai-oauth", state, set_active=set_active + global_root = _global_auth_file_path() + is_from_root = bool( + source_path is not None + and global_root is not None + and _same_path(source_path, global_root) ) - _save_auth_store(auth_store) - if write_through_to_root: + if is_from_root: + # Grant was resolved from root — write back to root only. + # Do NOT call _store_provider_state on the profile auth_store + # (it would create a shadowing providers.xai-oauth key that + # disables write-through on the next refresh — #74339). _write_through_xai_oauth_to_global_root(state) + else: + # Profile genuinely owns this — write to profile store. + _store_provider_state( + auth_store, "xai-oauth", state, set_active=set_active + ) + _save_auth_store(auth_store) def _xai_access_token_is_expiring(access_token: str, skew_seconds: int = 0) -> bool: diff --git a/tests/agent/test_credential_pool_oauth_writethrough.py b/tests/agent/test_credential_pool_oauth_writethrough.py index 707b502d1c647..52678adb33405 100644 --- a/tests/agent/test_credential_pool_oauth_writethrough.py +++ b/tests/agent/test_credential_pool_oauth_writethrough.py @@ -264,14 +264,6 @@ def test_write_through_fires_on_every_refresh_not_just_first( ) provider = "openai-codex" - call_count = [0] - - def counting_root_write(provider_id, state): - call_count[0] += 1 - - monkeypatch.setattr( - CP, "_write_through_provider_state_to_global_root", counting_root_write - ) # After patching A's module-level attributes, the bare-name imports in # credential_pool.py still hold references to the original functions # (``from X import Y`` creates a local binding that does not update when @@ -280,6 +272,9 @@ def test_write_through_fires_on_every_refresh_not_just_first( # are ``agent.credential_pool.__dict__`` — sees the mocked paths. monkeypatch.setattr(CP, "_global_auth_file_path", lambda: root_path) monkeypatch.setattr(CP, "_same_path", lambda a, b: a == b) + # Let _write_through_provider_state_to_global_root run for real so it + # persists the rotated token pair to the root auth.json — the test + # asserts the on-disk values after each refresh. # ---- REFRESH 1 ---- _write_store(profile_path, {"version": 1}) @@ -288,7 +283,12 @@ def test_write_through_fires_on_every_refresh_not_just_first( ) pool1 = CredentialPool(provider, [entry1]) pool1._sync_device_code_entry_to_auth_store(entry1) - assert call_count[0] == 1, "refresh 1: write-through must fire (#74339)" + + # Verify root was updated with the rotated tokens from refresh 1. + root_store = _read_store(root_path) + root_tokens = root_store["providers"]["openai-codex"]["tokens"] + assert root_tokens["access_token"] == "ac1" + assert root_tokens["refresh_token"] == "rf1" # After refresh 1 the profile should NOT have a providers.openai-codex # block (the fix skipped _store_provider_state because the grant came @@ -306,8 +306,14 @@ def test_write_through_fires_on_every_refresh_not_just_first( ) pool2 = CredentialPool(provider, [entry2]) pool2._sync_device_code_entry_to_auth_store(entry2) - assert call_count[0] == 2, ( - "refresh 2: write-through must fire even after a prior sync-back. " - "The old code self-disabled here (#74339)" - ) + + # Verify root was updated with the rotated tokens from refresh 2. + # The old key-presence check would have silently skipped this write. + root_store = _read_store(root_path) + root_tokens = root_store["providers"]["openai-codex"]["tokens"] + assert root_tokens["access_token"] == "ac2", ( + "refresh 2: root must carry the rotated token pair. " + "The old code self-disabled write-through here (#74339)" + ) + assert root_tokens["refresh_token"] == "rf2"