fix(grok): prefer auth.x.ai session + mention Grok in OSC52 toast#8392
Conversation
auth.json can hold multiple issuer keys; Object.values order could pick a stale entry before the live auth.x.ai OIDC session. Prefer auth.x.ai keys and fall back only when none exist. Also list Grok in the OSC52 blocked clipboard toast so Grok TUI copy failures point at the same setting.
📝 WalkthroughWalkthroughGrok authentication session reading now maps tokenized entries, prefers fresh 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/main/rate-limits/grok-auth.ts (1)
70-78: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider narrowing
parseAuthEntry's return type to eliminate the!assertion.
sessionFromAuthEntryusesauthEntry.key!because TypeScript can't propagate the runtime validation fromparseAuthEntry. HavingparseAuthEntryreturn a type withkey: string(non-optional) would make the safety guarantee explicit and remove the assertion.♻️ Optional type narrowing
+type ValidatedGrokAuthEntry = Omit<GrokAuthEntry, 'key'> & { key: string } + -function parseAuthEntry(value: unknown): GrokAuthEntry | null { +function parseAuthEntry(value: unknown): ValidatedGrokAuthEntry | null { if (typeof value !== 'object' || value === null) { return null } const entry = value as GrokAuthEntry if (typeof entry.key !== 'string' || entry.key.length === 0) { return null } - return entry + return entry as ValidatedGrokAuthEntry } -function sessionFromAuthEntry(authEntry: GrokAuthEntry): GrokAuthSession { +function sessionFromAuthEntry(authEntry: ValidatedGrokAuthEntry): GrokAuthSession { return { - accessToken: authEntry.key!, + accessToken: authEntry.key,src/main/rate-limits/grok-auth.test.ts (1)
48-84: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding a test for the exact
https://auth.x.aikey (without::suffix).
isPreferredGrokAuthKeyhas an exact-match branch (key === PREFERRED_GROK_AUTH_ISSUER) that isn't covered by the current tests — both fixtures use::-suffixed keys. A quick test with the bare issuer key would close that gap.✅ Optional test for exact-match branch
it('selects an exact auth.x.ai key without :: suffix', async () => { vi.doMock('node:fs', () => ({ existsSync: vi.fn(() => true), readFileSync: vi.fn(() => JSON.stringify({ 'https://auth.x.ai': { key: 'exact-token', user_id: 'exact-user', email: 'exact@example.com', expires_at: '2099-01-01T00:00:00.000Z' } }) ) })) const { readGrokAuthSession } = await import('./grok-auth') expect(readGrokAuthSession()).toEqual({ status: 'ok', session: { accessToken: 'exact-token', userId: 'exact-user', email: 'exact@example.com', teamId: null, expiresAtMs: Date.parse('2099-01-01T00:00:00.000Z'), oidcClientId: null } }) })
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4c8cf3eb-7a85-4b83-b184-5c564c71e1fc
📒 Files selected for processing (6)
src/main/rate-limits/grok-auth.test.tssrc/main/rate-limits/grok-auth.tssrc/renderer/src/components/terminal-pane/osc52-clipboard-blocked-toast.test.tssrc/renderer/src/components/terminal-pane/osc52-clipboard-blocked-toast.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/zh.json
Co-authored-by: Orca <help@stably.ai>
Jinwoo-H
left a comment
Jinwoo-H
left a comment
There was a problem hiding this comment.
Thanks! I reviewed and tested this against current main.
- Hardened selection across multiple, expired, and tokenless auth.x.ai entries.
- Preserved alternate-issuer fallback only when no preferred issuer exists.
- Completed the OSC52 Grok copy in all supported locales.
- Verified with 67 focused tests, typecheck/lint, and Electron validation.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/rate-limits/grok-auth.test.ts (1)
118-140: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for expired-preferred-over-fallback behavior.
The implementation (
expiredPreferred ?? (preferredKeySeen ? null : fallback)) returns an expired preferred session instead of a fresh fallback when no fresh preferred entry exists. This behavior is described in the PR summary ("preserves expired preferred sessions over fallbacks") but has no test coverage. A regression likefallback ?? expiredPreferredwould pass all current tests while breaking this contract.🧪 Suggested companion test
it('skips an expired auth.x.ai client entry when a fresh one follows it', async () => { // ... existing test ... }) + + it('preserves an expired auth.x.ai session over a fresh alternate issuer', async () => { + vi.doMock('node:fs', () => ({ + existsSync: vi.fn(() => true), + readFileSync: vi.fn(() => + JSON.stringify({ + 'https://alternate.example.com::client': { + key: 'alt-token', + user_id: 'alt-user', + expires_at: '2099-01-01T00:00:00.000Z' + }, + 'https://auth.x.ai::client': { + key: 'expired-token', + user_id: 'expired-user', + expires_at: '2020-01-01T00:00:00.000Z' + } + }) + ) + })) + const { readGrokAuthSession } = await import('./grok-auth') + + expect(readGrokAuthSession()).toMatchObject({ + status: 'ok', + session: { accessToken: 'expired-token' } + }) + })
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0d93952f-ecdc-42f4-9fd1-00b6ed3f469f
📒 Files selected for processing (6)
src/main/rate-limits/grok-auth.test.tssrc/main/rate-limits/grok-auth.tssrc/renderer/src/components/terminal-pane/osc52-clipboard-blocked-toast.test.tssrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.json
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/rate-limits/grok-auth.ts
2 participants
Summary
Grok
auth.jsoncan contain multiple issuer keys.readGrokAuthSessionusedObject.valuesand returned the first tokenized entry, so a stale alternate issuer written earlier could win over the livehttps://auth.x.ai::...OIDC session. That makes usage/account status pick the wrong token on multi-entry files.Also updates the OSC 52 blocked-clipboard toast copy to mention Grok (alongside SSH/tmux/Neovim/fzf) so Grok TUI copy failures deep-link to the same setting.
Root cause
auth.jsonmap order is insertion order; the firstkey-bearing entry is not necessarily the live Grok CLI session.https://auth.x.ai/https://auth.x.ai::....Fix
https://auth.x.aior starts withhttps://auth.x.ai::, even when an expired preferred entry appears first.Merge order
Land Grok-compat fixes in this order:
auth.x.ai+ OSC52 Grok toast copyThis PR is independent of #8386 / #8336 / #8390 / #8391 (no shared files with those diffs). It can land as soon as it is green; the order above is for review sequencing of the Grok-compat batch only.
Test plan
pnpm exec vitest run --config config/vitest.config.ts src/main/rate-limits/grok-auth.test.ts src/main/rate-limits/grok-fetcher.test.ts src/main/grok-accounts/status.test.ts src/main/rate-limits/service.test.ts src/renderer/src/components/terminal-pane/osc52-clipboard-blocked-toast.test.tsauth.jsonsent the preferred token to a local billing stub and rendered the expected usage/account