Skip to content

docs(xai-oauth): confirm token propagation is correct, add E2E regression tests (#29344)#30992

Open
bradhallett wants to merge 1 commit into
NousResearch:mainfrom
bradhallett:fix/xai-oauth-token-propagation-29344
Open

docs(xai-oauth): confirm token propagation is correct, add E2E regression tests (#29344)#30992
bradhallett wants to merge 1 commit into
NousResearch:mainfrom
bradhallett:fix/xai-oauth-token-propagation-29344

Conversation

@bradhallett

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #29344 (commits cc93053, b5ea6a5). SmelterLabs suspected a second-layer bug: _swap_credential might 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:

_swap_credential(entry)
  → self._client_kwargs["api_key"]  = new_key
  → self._client_kwargs["base_url"] = new_url
  → self._replace_primary_openai_client(reason="credential_rotation")
      → self.client = OpenAI(**self._client_kwargs)

On the next API call, _create_request_openai_client does dict(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_credential documenting the propagation chain and confirming no gap exists
  • tests/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 client

Test plan

pytest tests/run_agent/test_xai_oauth_token_propagation.py -v   # 10 passed
pytest tests/agent/test_codex_xai_oauth_recovery.py -v          # 37 passed (existing)

Refs #29344

…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.
@alt-glitch alt-glitch added type/test Test coverage or test infrastructure comp/agent Core agent loop, run_agent.py, prompt builder provider/xai xAI (Grok) area/auth Authentication, OAuth, credential pools P3 Low — cosmetic, nice to have labels May 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/agent Core agent loop, run_agent.py, prompt builder P3 Low — cosmetic, nice to have provider/xai xAI (Grok) type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants