Recover expired Grok sessions through the CLI owner - #3497
Draft
audreyt wants to merge 2 commits into
Draft
Conversation
Wire OAuth fetching to the shared CLI owner's ACP bearer export, then reload and validate the account before billing. Keep token exchange and auth.json writes out of CodexBar; preserve team identity and reject mismatching snapshots. Use the required _x.ai/auth/getBearerToken wire prefix and nested extension result envelope. Verified real CLI 1.0.13 token rotation, successful OAuth billing, and subsequent CLI reuse. Focused regression tests and make check pass. Draft-only: credential-owner concurrent account-change/logout coordination remains a release gate. Read-side guards do not prove cross-process persistence safety.
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
Closed
…very # Conflicts: # CHANGELOG.md
clawsweeper
Bot
added
P2
|
Codex review: needs real behavior proof before merge. Reviewed September 8, 2026, 3:29 AM ET / 07:29 UTC. ClawSweeper reviewWhat this changesThe PR adds CLI-owned renewal of expired Grok credentials before OAuth usage fetching, validates the reloaded account, removes raw RPC response logging, and adds tests and documentation. Merge readiness⛔ Blocked before merge - 5 items remain Keep open: this remains useful work absent from main, with credible live renewal evidence. The explicitly unresolved credential-owner concurrency guarantee still blocks release; no additional concrete patch defect was established. Priority: P2 Review scores
Verification
How this fits togetherCodexBar’s Grok provider reads CLI credentials to fetch billing and subscription information for its usage display. This change asks the CLI to renew expired credentials, then validates the saved result before using it. flowchart TD
A[OAuth usage request] --> B[Read CLI credentials]
B --> C{Expired credential?}
C -->|No| F[Billing and subscription requests]
C -->|Yes| D[CLI owns renewal and persistence]
D --> E{Reloaded token and account match?}
E -->|Yes| F
E -->|No| G[Reject recovery]
F --> H[Usage and account display]
Decision needed
Why: The author explicitly withholds release clearance pending private owner-side coordination; CodexBar’s read-side checks cannot settle the external process’s final-write contract. Before merge
Findings
Agent review detailsSecurityNeeds attention: Raw bearer response logging is removed, but credential-owner concurrency remains a release-blocking authority uncertainty. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Retain CLI-owned renewal and the single validated account capture, with an owner-approved concurrency contract that prevents stale credentials from surviving logout or account replacement. Do we have a high-confidence way to reproduce the issue? Not applicable as an established-behavior bug: main intentionally rejects expired OAuth credentials, and this PR adds automatic recovery. The supplied live output demonstrates the new happy path, not the unresolved concurrency case. Is this the best way to solve the issue? Yes for the ownership design: reusing the CLI’s renewal owner avoids a second credential writer. Release safety remains conditional on that owner’s concurrent logout and account-replacement guarantees. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against c3f3ea1faf70. LabelsLabel justifications:
EvidenceSecurity concerns:
What I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (1 earlier review cycle)
|
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
1 participant
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.
Status: draft — credential-owner coordination gate remains
Integrated successor to #3473. This is not the standalone refresher again: the production OAuth fetch path now performs recovery, and a real CLI-owned renewal plus billing recovery has been exercised.
Do not merge yet. Happy-path renewal and subsequent CLI reuse are proven. The credential owner's concurrent account-change/logout contract is not cleared for release. Read-side checks cannot establish persistence safety inside another process. Owner-side findings are being kept out of public reproducer material in accordance with xAI's security reporting policy.
Implementation / ownership boundary
auth.jsonwriter. CodexBar does not POST to an OIDC token endpoint or save credentials.grok agent --leader stdio, then ACP_x.ai/auth/getBearerToken. The shared owner is separate from the short-lived probe client.The read-side account/generation checks intentionally are not represented as a cross-process compare-and-swap or a guarantee about the CLI's own final write.
Real behavior proof (2026-09-08, official Grok 1.0.13 / 5e9a58528b76)
The initial live protocol probe caught a bug the original stub missed:
x.ai/auth/getBearerTokenreturns-32601;_x.ai/auth/getBearerTokensucceeds. The response also contains both the JSON-RPC result and the extension's nested result envelope. The updated regression failed with the old wire method, then passed after the correction.For the live expiry exercise, only the existing auth record's expiry timestamp was temporarily marked expired under the CLI's file lock. No refresh token was copied into a second credential store. The real CLI contacted the real token service and persisted the renewed generation. The harness was guarded to restore only the expiry of an unchanged generation; no rollback was needed.
Redacted result from the newly built
CodexBarCLI usage --provider grok --source oauth --json:{ "exit_code": 0, "elapsed_seconds": 2.776, "access_token_rotated": true, "refresh_token_rotated": true, "same_identity": true, "unexpired": true, "source": "grok-cli-proxy", "usage_present": true, "subscription_tier_present": true }A separate subsequent connection to the installed CLI exported the same persisted bearer and successfully called
_x.ai/billing:{ "bearer_matches_auth_file": true, "cli_reused_existing_token": true, "cli_billing_supported": true, "cli_billing_result_present": true }This proves real token exchange, persistence by the owner, resumed billing, and CLI reuse after a locally forced expiry timestamp. It is not a claim that the server-side JWT expired naturally during the test, or that the owner-side concurrency gate passed. No tokens, account identifiers, or private transcripts are included here.
Validation
swift test --filter 'GrokAccountContextTests|GrokSessionRecoveryTests': 11 tests passed, including the real subprocess protocol fixture, Team recovery, account-switch/removal/generation rejection, and existing account-capture cases.main(onlyCHANGELOG.mdconflicted),swift test --filter Grok: 156 tests in 15 suites passed. The newly built production CLI also returned usage and subscription tier with no error.make check: passed, including SwiftFormat and strict SwiftLint (0 violations across 2,142 Swift files).make teston the implementation commit: did not pass. The sharded run selected 1,035 selections in 87 groups and exited on a 180-second timeout inUsageStoreCachedTokenHydrationTests. The same selection also timed out at 180 seconds on unchanged base0cb8c425e, after a clean rebuild in an isolated worktree with the same test-isolation environment. This timeout therefore reproduces without the Grok changes; the full suite is not reported as green.Before marking ready