fix(grok): Prefer a live credential over a stale keyring entry - #37
Merged
Conversation
load_grok() read the OS keyring before ~/.grok/auth.json and returned the first entry it found, so an expired cairn-code token shadowed the fresh one the Grok CLI maintains and every request went out with a dead bearer. The refresh probe could not recover from it either: it refreshes the CLI's file, nothing rewrites the keyring copy, so the probe ran on every call and changed nothing. Collect the candidates with the canonical file first and take the first one that is still live, rather than the first one that exists. Order alone would fix Grok while breaking the mirror case Antigravity documents, where the keyring holds the live token and the file is the stale artifact. cef4016 introduced the keyring-first ordering for both Claude and Grok, and 6f07976 corrected only Claude. Refs #36 Claude-Session: https://claude.ai/code/session_01PvhTkPF3fvjPUh5ao9MeWL
used_percent was initialised to 0.0 and only overwritten on success, so a billing request that never landed rendered as "0% used" and looked like a healthy, idle week. Carry it as an Option and report "usage unavailable" when the call does not answer. Absence of creditUsagePercent inside a successful response stays a real zero: the payload is protobuf JSON, which omits any field still holding its default, so a period with no spend legitimately returns without the field. Refs #36 Claude-Session: https://claude.ai/code/session_01PvhTkPF3fvjPUh5ao9MeWL
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe change updates Grok credential selection to evaluate all supported stores and improves billing usage reporting. Fresh credentials now take precedence over stale entries. Failed billing requests now display unavailable usage instead of zero. ChangesGrok handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Poem
✨ Finishing Touches📝 Generate docstrings
Comment |
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
Grok reported
HTTP 401and rendered as degraded with no usage windows, on amachine where the Grok CLI was signed in and working.
load_grok()read the OS keyring before~/.grok/auth.jsonand returned thefirst entry it found regardless of freshness, so an expired
cairn-codeoauth:xaitoken shadowed the live one the CLI maintains. The refresh probecould not rescue it:
grok modelsrefreshes the CLI's file, nothing rewritesthe keyring copy, so the probe fired on every call, refreshed a file that was
then ignored, and handed back the same dead token.
This is a regression. Before
cef4016("Load Grok and Claude tokens fromcairn-code keyring entries"),
load_grok()read only the file. That commit putthe keyring ahead of the file for both Claude and Grok;
6f07976("Handlenullable Claude quota fields and prioritize canonical credentials file") fixed
it one commit later for Claude alone.
The second commit closes a smaller gap in the same path: a billing call that
never landed was indistinguishable from a window that had gone unspent.
Changes
src/credentials/mod.rs:load_grok()now collects every candidate with thecanonical file first and picks the first still-live one via
pick_live(),rather than the first that merely exists. A plain order swap would fix Grok
while breaking the mirror case Antigravity documents, where the keyring holds
the live token and the file is the stale artifact.
src/fetch.rs:used_percentbecomes anOption, reported asusage unavailablewhen billing does not answer. An absentcreditUsagePercentinside a successful response stays a real zero, sincethe payload is protobuf JSON and omits fields still holding their default.
Test plan
cargo fmt --checkRUSTFLAGS="-Dwarnings" cargo clippy --all-targetscargo test(160 pass, up from 155)change in place:
a_stale_keyring_entry_never_shadows_a_live_credentialfails withleft: "expired-keyring-copy", right: "refreshed-cli-token"grok_billing_that_never_answers_reports_unavailable_rather_than_zerofailswith
left: "0% used", right: "usage unavailable"limits status -p grok --jsonreturned"ErrorMessage": "Grok user API returned HTTP 401"with an emptyWindowsarray before, and a healthy
Weeklywindow with a reset countdown after.billing: fetched credits configlog records, which show the field presentwith real values for the previous billing period and absent in the new one.
Fixes #36