fix(grok): treat expired credentials as missing in fetch#976
Merged
Conversation
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).
Contributor
Author
|
CI was failing on Pushed |
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 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.fetchdecides whether to surface the RPC auth error using:That check is permissive: an
auth.jsonwhoseexpires_atis already in the past still loads successfully (isExpiredis computed but never consulted), socredentials != niland the auth error is silently swallowed. The UI then renders a "successful" snapshot with stale email/team and nogrok loginhint 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:
Behaviour matrix after this change:
expires_atexpires_atshape stays supported)Tests
Adds
isExpired reflects past expires_atcovering past, future, and missingexpires_at. All 11 Grok tests pass under Xcode 26.5.Test plan
make testpassesgrok loginsession, Grok shows identity as beforerm ~/.grok/auth.json(or letting it expire), the provider surfaces theNot authenticated to Grok. Run \grok login`.` hint instead of rendering empty