fix(auth): apply same source-path write-through fix to non-pool xAI path
_save_xai_oauth_tokens had the identical self-sealing bug as _sync_device_code_entry_to_auth_store: key-presence check before _store_provider_state, which unconditionally creates the key. Use _load_provider_state_with_source to decide write-through from the actual grant source, not key presence. Also update regression test per review: use the real _write_through_provider_state_to_global_root helper and assert rotated token pair values in the root store after each refresh instead of just counting mock calls.
This commit is contained in:
parent
4e6299af48
commit
35b44e0d5f
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue