Skip to content

fix(auth): resolve xAI OAuth credentials across profiles - #38440

Closed
capt-marbles wants to merge 1 commit into
NousResearch:mainfrom
capt-marbles:fix/xai-oauth-profile-resolution
Closed

fix(auth): resolve xAI OAuth credentials across profiles#38440
capt-marbles wants to merge 1 commit into
NousResearch:mainfrom
capt-marbles:fix/xai-oauth-profile-resolution

Conversation

@capt-marbles

Copy link
Copy Markdown
Contributor

Summary

  • Resolve xAI OAuth tokens from the local credential pool when provider-local state is present but empty/stale.
  • Fall back to the global/root auth store so profile/cron contexts can use freshly re-authenticated xAI OAuth credentials.
  • Add regression coverage for credential-pool fallback, global-store fallback, and the still-broken/no-usable-token path.

Why

After re-authentication, default/root Hermes could successfully call xAI, but profile/cron contexts could still fail if their provider-local providers.xai-oauth.tokens state was empty or stale. _read_xai_oauth_tokens() treated that empty singleton as authoritative and raised xai_auth_missing_access_token instead of checking the credential pool or root auth store.

This makes the post-auth failure mode explicit: a user can have valid xAI OAuth credentials available, while cron still behaves as if auth is missing.

Test Plan

  • RED before fix: python -m pytest tests/hermes_cli/test_xai_oauth_profile_auth.py -q -o 'addopts=' produced 2 failed, 1 passed; both failures raised xai_auth_missing_access_token when usable fallback credentials existed.
  • GREEN after fix: python -m pytest tests/hermes_cli/test_xai_oauth_profile_auth.py -q -o 'addopts='3 passed.
  • Existing provider suite: python -m pytest tests/hermes_cli/test_auth_xai_oauth_provider.py -q -o 'addopts='82 passed.
  • Compile check: python -m py_compile hermes_cli/auth.py tests/hermes_cli/test_xai_oauth_profile_auth.py.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard area/auth Authentication, OAuth, credential pools provider/xai xAI (Grok) labels Jun 3, 2026
@liuhao1024

Copy link
Copy Markdown
Contributor

Verified the credential pool + global store fallback chain. Three things worth noting:

  1. _xai_oauth_state_from_store correctly checks credential pool entries when provider state has empty tokens — this handles the profile sync scenario where the pool has fresh tokens but providers.xai-oauth.tokens is stale or empty.

  2. The explicit _xai_oauth_state_from_store(_load_global_auth_store()) fallback is necessary because _load_provider_state's built-in global fallback only checks providers.xai-oauth, not credential_pool.xai-oauth. Without this, a global credential pool entry wouldn't be resolved from a profile context.

  3. The three tests cover the key scenarios: pool fallback (profile provider empty, pool has tokens), global fallback (profile empty, global has tokens), and error path (all empty → AuthError).

@capt-marbles
capt-marbles force-pushed the fix/xai-oauth-profile-resolution branch from dacfed8 to 2fd04f7 Compare June 4, 2026 13:34
@capt-marbles

Copy link
Copy Markdown
Contributor Author

Rebased onto current main and re-ran the targeted regression coverage after the maintainer verification above.

Current head: 2fd04f7a0551f7336ff722e55fa2f925455b719f

Validation after rebase:

uv run --extra dev pytest \
  tests/hermes_cli/test_xai_oauth_profile_auth.py \
  tests/hermes_cli/test_auth_xai_oauth_provider.py \
  -q
# 85 passed

python3 -m py_compile \
  hermes_cli/auth.py \
  tests/hermes_cli/test_xai_oauth_profile_auth.py \
  tests/hermes_cli/test_auth_xai_oauth_provider.py

git diff --check

No conflict/merge cleanup needed after the rebase. This is still the same narrow fix: profile-local xAI provider state can be empty while the credential pool or root/global auth store has valid OAuth tokens, and genuinely empty auth still raises AuthError.

@capt-marbles
capt-marbles force-pushed the fix/xai-oauth-profile-resolution branch from 2fd04f7 to 8d8b9f5 Compare June 10, 2026 11:53
@capt-marbles

Copy link
Copy Markdown
Contributor Author

Rebased onto current main and re-ran the targeted xAI OAuth regression coverage.

Current head: 8d8b9f50e486fbb77e20799ce0b49d23ac853e88

Validation after rebase:

uv run --extra dev pytest \
  tests/hermes_cli/test_xai_oauth_profile_auth.py \
  tests/hermes_cli/test_auth_xai_oauth_provider.py \
  -q
# 85 passed in 2.98s

python3 -m py_compile \
  hermes_cli/auth.py \
  tests/hermes_cli/test_xai_oauth_profile_auth.py \
  tests/hermes_cli/test_auth_xai_oauth_provider.py

git diff --check

No conflicts on this rebase. The PR is still the same narrow fix: profile-local xAI provider state can be empty while the credential pool or root/global auth store has valid OAuth tokens, and genuinely empty auth still raises AuthError.

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Thanks for this, @capt-marbles — your fix was correct and is now on main.

Salvaged your commit verbatim (authorship preserved) in #46614 and paired it with the missing write-through half: because xAI rotates the refresh token on every refresh, the read-fallback alone left the global root holding a revoked token whenever a profile refreshed a grant it had read from root — the multi-profile revocation cascade in #43589. #46614 writes the rotated chain back to root in that case, so the cross-profile grant stays coherent in both directions.

Closing this in favor of #46614 (merged as 8844e09). Your commit lands on main credited to you. Appreciate the clean diff and regression tests.

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/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have provider/xai xAI (Grok) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants