fix(grok): preserve billing semantics in nonempty requests - #3336
fix(grok): preserve billing semantics in nonempty requests#3336CharlieLZ wants to merge 6 commits into
Conversation
…usage grok.com's billing API now rejects the empty GetGrokCreditsConfig request (grpc-status 13, "Missing request message."), so the Grok usage fetch always failed and the card never showed a percentage. Send the request with usage_period_type: WEEKLY explicitly. SuperGrok Heavy weekly-limit frames carry a per-period limit but no used amount, so the previous parse rule classified them as unknown usage and the adoption guard rejected the value as fabricated 0%. A limit-declared period is a fully determined zero-usage response on the wire, so publish it as 0% and let the web billing candidate fill the usage bar.
|
🦞👀 Pull request received. I will update this pull request when review starts. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
clawsweeper
Bot
added
merge-risk: 🚨 availability
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb12ee75e4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let hasDefinedPeriodLimit = scan.varintFields.contains { field in | ||
| (field.path == [1, 4, 2] || field.path == [1, 5, 2]) | ||
| && field.value > 0 && field.value < 1_000_000_000_000 |
There was a problem hiding this comment.
For any period-only response whose start or end has a nonzero fractional second, this condition mistakes the Timestamp.nanos value for a billing limit: the same parser already treats [1,5,1] as the reset's epoch seconds, and the new fixture's [1,5,2] is 56,343,000, the corresponding nanoseconds field. Such a response is therefore marked as having a wire-published 0%, allowing resolvingUnknownUsage to replace genuinely unknown proxy usage with a fabricated zero; use an actual limit/usage field rather than the second member of these timestamp messages.
Useful? React with 👍 / 👎.
| request.httpMethod = "POST" | ||
| request.timeoutInterval = Self.requestTimeoutSeconds | ||
| request.httpBody = Data([0x00, 0x00, 0x00, 0x00, 0x00]) | ||
| request.httpBody = Self.weeklyCreditsConfigRequest |
There was a problem hiding this comment.
When the grok.com request stalls in a direct cookie/bearer gRPC path or after the proxy fails, replacing these assignments also removes request.timeoutInterval = Self.requestTimeoutSeconds; the request now inherits the transport default instead of the intended 15 seconds. This also makes the existing request.timeoutInterval == 15 assertion fail and leaves requestTimeoutSeconds unused, so retain the timeout assignment alongside the new body.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| @Test | ||
| func `parses live weekly limit billing response as zero percent`() throws { |
There was a problem hiding this comment.
Inserting this method immediately after the existing @Test causes that annotation to attach to the new fixture test, leaving the following parses omitted zero percent with current billing period method unannotated. Swift Testing therefore no longer discovers or executes that existing regression test; add a separate @Test before the displaced method.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
Codex review: needs real behavior proof before merge. Reviewed September 1, 2026, 1:29 AM ET / 05:29 UTC. ClawSweeper reviewWhat this changesThis PR sends Grok’s billing fallback a nonempty protobuf request that explicitly preserves legacy monthly-usage semantics, with regression tests for request bytes and unknown usage enrichment. Merge readiness⛔ Blocked until real behavior proof from a real setup is added - 3 items remain The implementation is focused and resolves all previously raised code defects, but the intended Grok endpoint recovery remains unproven on an affected account. Keep this PR open for real post-change behavior proof before merge. Priority: P2 Review scores
Verification
How this fits togetherCodexBar’s Grok provider combines a CLI proxy result with a best-effort grok.com billing fallback to produce usage, reset, and plan data for the menu bar. This change alters the fallback request bytes before its parsed result can enrich an otherwise unknown proxy usage value. flowchart LR
A[Grok credentials] --> B[Usage fetch strategy]
B --> C[CLI proxy billing]
B --> D[grok.com billing fallback]
D --> E[Protobuf response parser]
C --> F[Usage enrichment]
E --> F
F --> G[Menu bar usage display]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Merge after a redacted affected-account trace shows the corrected request receives a successful billing response while unknown proxy usage remains unknown when no wire percentage is returned. Do we have a high-confidence way to reproduce the issue? No: source and stubs establish the changed request and fallback behavior, but the supplied evidence lacks a real affected-account request and observed post-change response. Is this the best way to solve the issue? Yes, conditionally: explicit proto3 false is the narrowest semantics-preserving way to make the request nonempty, provided an affected-account trace confirms the endpoint accepts it. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against a140553c9ef7. LabelsLabel justifications:
EvidenceWhat 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 (6 earlier review cycles)
|
Use the verified exclude_legacy_monthly_usage boolean default instead of treating it as a weekly selector. Keep inferred usage out of proxy enrichment, retain the timeout, and restore parser regression coverage. Live endpoint recovery remains to be confirmed. Co-authored-by: charlielz <1540060247@qq.com>
steipete
changed the title
clawsweeper
Bot
added
status: 📣 needs proof
…hed 0% A unified-billing account with onDemandCap 0 / onDemandUsed 0 has no credit usage at all; the grok.com Usage page shows 0% used for the weekly period. The proxy omits creditUsagePercent entirely for this shape, so the fetch pipeline reported unknown usage and the Grok menu card showed the unavailable-usage diagnostic. Treat the 0/0 unified-billing payload as a wire-published zero, keeping the legacy period-only unknown-usage semantics for everything else.
Zero on-demand credits do not establish included subscription usage. Preserve the existing unknown-usage enrichment path and retain the new proxy fixtures as negative regression coverage. Co-authored-by: charlielz <1540060247@qq.com>
Match the expected reset with the fractional timestamp in the captured proxy payload.
Assert the observable fallback contract instead of the provenance flag, which is not meaningful for a missing percentage.
2 participants
Summary
Addresses the reported
grpc-status 13/Missing request message.rejection by sending a nonemptyGetGrokCreditsConfigRequest, while preserving the default billing semantics.The current official Grok descriptor defines request field 1 as
bool exclude_legacy_monthly_usage, not a weekly-period enum. The web client requests{}. This patch explicitly encodes false (00 00 00 00 02 08 00) so the message is nonempty without excluding legacy monthly usage. Decoding withprotocconfirms empty and0800both mean false, whereas the original0802means true.The maintainer repair also restores the 15-second timeout and the displaced
@Test. Response parsing is unchanged from main. The captured frame's nonzero timestamp nanoseconds are not usage limits, so its inferred zero must not replace an unknown proxy percentage. The fixture now covers that boundary through the real parser, proxy enrichment, and usage snapshot; explicit wire zero/nonzero percentages still work.The later unified-billing
0/0inference has also been removed. Its fixtures remain as regression coverage: absent subscription usage stays unknown, and real fallback usage can enrich it without changing the authoritative reset date. Zero on-demand spending does not establish zero subscription usage.Thanks @CharlieLZ for the report, original request change, and captured regression fixture.
Protocol evidence
Public assets checked September 1, 2026 UTC; downloaded JavaScript was inspected, not executed.
Verification and merge gate
10cefdc9c4cbf470ce882d27ef3cc2432b450b69: 81 tests across two suites, with stubbed HTTP and synthetic credentials. The same compiler was used through SwiftPM's native backend after Xcode's SDK stat-cache tool stalled before compilation.make checkpasses with zero lint violations across 2,079 Swift files.10cefdc9c4cbf470ce882d27ef3cc2432b450b69, including both macOS test shards and all three Linux builds.StatusMenuTeststimeout, after two timing-sensitive failures recovered on retry. It is not a green full-suite result. The independently diagnosed Sakana cancellation-test race is addressed separately in #3344.Leave unmerged until the corrected
0800request is confirmed against the affected account. No authenticated provider probe, real cookie import, Keychain access, or installed-app relaunch was performed for this repair. The original reported 0% result was produced by the discarded parser heuristic and is not proof of published usage or of this corrected request's live recovery. Unknown percentages intentionally remain unknown.