Skip to content

fix(grok): treat expired credentials as missing in fetch#976

Merged
steipete merged 4 commits into
steipete:mainfrom
taibaran:grok-expired-credentials
May 16, 2026
Merged

fix(grok): treat expired credentials as missing in fetch#976
steipete merged 4 commits into
steipete:mainfrom
taibaran:grok-expired-credentials

Conversation

@taibaran
Copy link
Copy Markdown
Contributor

Summary

Follow-up to the Codex bot's P1 review on #965 (commit cbd30a4e). Grok session tokens expire after ~7 days, so this hits real users on a predictable cadence.

The bug

GrokStatusProbe.fetch decides whether to surface the RPC auth error using:

if billing == nil, credentials == nil {
    throw rpcError ?? GrokRPCError.notAuthenticated
}

That check is permissive: an auth.json whose expires_at is already in the past still loads successfully (isExpired is computed but never consulted), so credentials != nil and the auth error is silently swallowed. The UI then renders a "successful" snapshot with stale email/team and no grok login hint while billing keeps 401-ing in the background.

The fix

Treat expired records the same as missing credentials when deciding whether to swallow the RPC error, and drop them from the rendered snapshot so the UI doesn't surface stale identity for an inactive session:

let activeCredentials = credentials.flatMap { $0.isExpired ? nil : $0 }
if billing == nil, activeCredentials == nil {
    throw rpcError ?? GrokRPCError.notAuthenticated
}

Behaviour matrix after this change:

expires_at Old behaviour New behaviour
missing rendered as fresh rendered as fresh (unchanged — pre-expires_at shape stays supported)
in the future rendered as fresh rendered as fresh (unchanged)
in the past rendered with stale data, RPC error swallowed RPC error raised, snapshot omits stale identity

Tests

Adds isExpired reflects past expires_at covering past, future, and missing expires_at. All 11 Grok tests pass under Xcode 26.5.

Test plan

  • make test passes
  • With a freshly-logged-in grok login session, Grok shows identity as before
  • After rm ~/.grok/auth.json (or letting it expire), the provider surfaces the Not authenticated to Grok. Run \grok login`.` hint instead of rendering empty

taibaran added 2 commits May 16, 2026 10:45
Follow-up to the Codex P1 review on steipete#965 (`cbd30a4e`).

Grok session tokens expire after ~7 days. The auth-error suppression in
`GrokStatusProbe.fetch` previously only checked `credentials == nil`, so
an `auth.json` whose `expires_at` was already in the past still counted
as "renderable" and masked the RPC auth failure. The probe would return
a successful snapshot carrying stale identity and no `grok login` hint
while billing silently 401s — users had to inspect logs to figure out
they were logged out.

Treat expired records the same as missing credentials when deciding
whether to swallow the RPC error, and drop them from the rendered
snapshot so the UI doesn't surface a stale email/team for an inactive
session. Adds a unit test covering past/future/missing `expires_at`.
CI on steipete#976 surfaced a hardcoded provider-order list in
SettingsStoreTests.swift:1161 that wasn't updated when grok was added
in steipete#965. The Grok-only test filter I ran locally before steipete#965 skipped
this file, so the breakage only became visible on the next post-merge
CI run.

Append .grok to the expected ordering so the suite passes. All 2565
tests pass under Xcode 26.5 (51 in CodexBarTests + plenty more).
@taibaran
Copy link
Copy Markdown
Contributor Author

CI was failing on SettingsStoreTests / "provider order persists and appends new providers" because the hardcoded expected list in SettingsStoreTests.swift:1161 was never updated when grok was added in #965 — my Grok-only swift test --filter Grok skipped this file so the regression became visible only on the next post-merge CI run.

Pushed 64585525 to append .grok to the fixture. Full suite (swift test, all 2565 tests) passes locally under Xcode 26.5.

@steipete steipete merged commit 567ac94 into steipete:main May 16, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants