docs(xai-oauth): confirm token propagation is correct, add E2E regression tests (#29344)#30992
Open
bradhallett wants to merge 1 commit into
Open
Conversation
…sion tests (NousResearch#29344) SmelterLabs suspected a second-layer bug beyond the classifier fix (cc93053): _swap_credential might update the credential pool entry but NOT propagate the new token into the live OpenAI SDK client / HTTP transport, leaving the next request on the stale token. Full trace of the propagation chain for non-Anthropic providers (xai-oauth): _swap_credential(entry) → self._client_kwargs["api_key"] = runtime_key (run_agent.py:2910) → self._client_kwargs["base_url"] = runtime_base (run_agent.py:2911) → self._replace_primary_openai_client( reason="credential_rotation") (run_agent.py:2913) → self.client = OpenAI(**self._client_kwargs) (line ~2501) On the next API call, _create_request_openai_client copies _client_kwargs (line ~2574) into the fresh per-request client — the refreshed token flows through automatically. No propagation gap exists. The stale-token issue reported in NousResearch#29344 was entirely at the classifier layer: _is_entitlement_failure and the xai-oauth catch-all in recover_with_credential_pool both misclassified bad-credentials 403s as entitlement, blocking the refresh path before _swap_credential ever ran. Commits cc93053 and b5ea6a5 fixed that layer. This commit: * Adds a detailed docstring to _swap_credential documenting the propagation chain and the NousResearch#29344 investigation conclusion. * Adds 10 regression tests in test_xai_oauth_token_propagation.py that exercise the full path: stale token → 403 → recovery → _swap_credential (NOT stubbed) → verify new token in _client_kwargs, api_key, and the per-request client created by _create_request_openai_client.
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.
Summary
Follow-up to #29344 (commits cc93053, b5ea6a5). SmelterLabs suspected a second-layer bug:
_swap_credentialmight update the credential pool entry without propagating the refreshed token into the live OpenAI SDK client.After a full trace of the propagation chain, token propagation is NOT broken:
On the next API call,
_create_request_openai_clientdoesdict(self._client_kwargs)→ the new key flows into the per-request client. The stale-token issue was entirely at the classifier layer, fixed by the parent commits.Changes
run_agent.py: Added docstring to_swap_credentialdocumenting the propagation chain and confirming no gap existstests/run_agent/test_xai_oauth_token_propagation.py(new): 10 E2E regression tests exercising the full path — stale token → 403 → recovery →_swap_credential(NOT stubbed) → verify new token propagated to_client_kwargs,api_key, and per-request clientTest plan
Refs #29344