fix(auth): quarantine invalid xAI OAuth state into its source store, not the active profile store - #90156
Open
johnmwhitman wants to merge 1 commit into
Open
Conversation
…not the active profile store When a terminal xAI OAuth refresh failure fires in a profile-mode process, the quarantine path loads provider state (which may resolve from the global-root store via the NousResearch#18594 fallback) but then persists the emptied state into the ACTIVE profile's store. That creates a shadowing providers.xai-oauth stub in the profile store which permanently hides the root grant from that profile (the NousResearch#74339 shape) — the profile reads its own empty stub forever instead of falling back to root. Fix: resolve where the state actually came from with _load_provider_state_with_source and persist the quarantined state back to that source store (root when root-resolved) via _persist_provider_state_to_store; only fall back to the active store when the state was genuinely profile-local. Regression test: profile store empty + root store holds the grant -> terminal refresh failure -> quarantine lands in ROOT, and no providers.xai-oauth stub appears in the profile store. Verified red-on-revert against the unpatched tree. Companion to NousResearch#81383 (cross-profile refresh serialization) — that PR fixes the rotation race; this fixes the quarantine-side stub that the race's failures leave behind.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When a terminal xAI OAuth refresh failure fires in a profile-mode process, the quarantine path loads the provider state — which may have been resolved from the global-root store via the #18594 read-only fallback — but then persists the emptied/quarantined state into the active profile's store.
That write creates a shadowing
providers.xai-oauthstub in the profile store (the #74339 shape). From then on the profile reads its own empty stub forever instead of falling back to the root grant: the lane looks permanently logged-out even after the root grant is repaired or re-issued.Observed in a production multi-profile deployment (10 profile homes sharing one root grant): every terminal refresh failure left another profile blinded, and re-granting at root did not recover the affected profiles until the stubs were manually deleted.
Fix
Resolve where the state actually came from with
_load_provider_state_with_source, and persist the quarantined state back to that source store (the global root when root-resolved) via_persist_provider_state_to_store. Only fall back to the active store when the state was genuinely profile-local. This mirrors the write-through discipline the success path already has (#43589 / #74339).Test
tests/test_xai_quarantine_source_store.py: hermetic HERMES_HOME profile layout, profile store empty, root store holds the grant → force a terminal refresh failure → assert the quarantine landed in the root store and noproviders.xai-oauthstub appears in the profile store. Verified red-on-revert against the unpatched tree.Relationship to #81383
Companion fix: #81383 serializes refreshes across profiles (preventing the rotation race that causes many of these terminal failures — independently confirmed there). This PR fixes the quarantine-side stub that each failure leaves behind. They compose; neither depends on the other.