Skip to content

fix(grok): classify missing billing methods by RPC code - #3947

Merged
steipete merged 1 commit into
mainfrom
triage/20260921-grok-local
Sep 24, 2026
Merged

steipete merged 1 commit into
mainfrom
triage/20260921-grok-local

Conversation

@steipete

Copy link
Copy Markdown
Owner

Grok discarded JSON-RPC error codes and recognized a missing billing method only from its English message. A valid -32601 with different wording therefore bypassed the team identity fallback and its local token summary; a different code saying Method not found incorrectly entered that fallback. Preserve the code and classify on -32601, retaining the original diagnostic message and leaving local transport errors unclassified.

The new stub-CLI cases fail on unchanged production code and pass with this change, including publication of 42 local tokens. The existing personal Auto RPC-to-proxy-to-Usage & Spend/share test already passes on main. Usage JSON intentionally omits live-only costUsage; this PR does not claim to fix the separate account-specific dashboard loss reported in #3716. Current main already handles #1805's weekly billing periods, including near-reset labeling and measured monthly windows.

Validation:

  • Before the fix: 184 tests in 15 suites, 182 passed, one failed with six assertions in the two new scenarios, one opt-in live test skipped. The existing RPC-to-share test passed.
  • After the fix: Test run with 238 tests in 19 suites passed (237 passed, one opt-in live test skipped, zero failures), covering Grok, RPC teardown/timeouts, bounded child processes, and provider architecture.
  • Final formatted regression: all 11 parameterized cases passed (one test, zero failures).
  • make check passed: 0/2,617 files require formatting; SwiftLint found zero violations in 2,616 files. One unrelated synthetic packaging receipt test failed under load, then passed in isolation and in the complete gate; one test-line wrapping violation was corrected. git diff --check passed.
  • Independent Codex autoreview: scoped-clean through P2.
  • Installed signed release 0.65.0 (154): both config show --json and usage --provider grok --json timed out after 90 seconds (exit 124, no JSON). Live reproduction and Grok's configured state remain unverified. No app relaunch, login, or settings/account change.
  • Production diff: 2 files changed, 5 insertions, 6 deletions; net −1. No provider registration or settings changes.

Exact regression commands (the first intentionally records the pre-fix failures):

CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 CODEXBAR_ALLOW_TEST_KEYCHAIN_ACCESS=0 CODEXBAR_TEST_CODEX_FILE_ISOLATION=1 CODEXBAR_TEST_SESSION_FILE_ISOLATION=1 CODEXBAR_LIVE_GROK_CATALOG_PROOF=0 swift test --jobs 2 --no-parallel --filter 'Grok(AccountContext|Auth|BillingResponse|CreditsProxyFetcher|FailedBillingWork|LocalSessionScanner|MenuCardModel|Plan|RemainingResetsFetcher|RemainingResetsRouting|SettingsReader|TokenSnapshotProjection|WebBillingFetcher|XAISpendCatalog|ZeroUsage)Tests'
CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 CODEXBAR_ALLOW_TEST_KEYCHAIN_ACCESS=0 CODEXBAR_TEST_CODEX_FILE_ISOLATION=1 CODEXBAR_TEST_SESSION_FILE_ISOLATION=1 CODEXBAR_LIVE_GROK_CATALOG_PROOF=0 swift test --jobs 2 --no-parallel --filter 'Grok(AccountContext|Auth|BillingResponse|CreditsProxyFetcher|FailedBillingWork|LocalSessionScanner|MenuCardModel|Plan|RemainingResetsFetcher|RemainingResetsRouting|SettingsReader|TokenSnapshotProjection|WebBillingFetcher|XAISpendCatalog|ZeroUsage)Tests|RPCChildProcessTeardownTests|RPCRequestTimeoutTests|BoundedChildProcessProofTests|ProviderArchitectureGatekeeperTests'
CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 CODEXBAR_ALLOW_TEST_KEYCHAIN_ACCESS=0 CODEXBAR_TEST_CODEX_FILE_ISOLATION=1 CODEXBAR_TEST_SESSION_FILE_ISOLATION=1 CODEXBAR_LIVE_GROK_CATALOG_PROOF=0 swift test --jobs 2 --no-parallel --filter GrokFailedBillingWorkTests
make check
git diff --check

Refs #3716
Refs #1805
Refs #3805

@clawsweeper

clawsweeper Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 24, 2026
@clawsweeper

clawsweeper Bot commented Sep 24, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed September 24, 2026, 6:04 AM ET / 10:04 UTC.

ClawSweeper review

What this changes

The branch preserves Grok JSON-RPC error codes and uses the missing-method code to decide when a team account retains identity and local token history after billing fails.

Merge readiness

✅ Ready for maintainer review

The Grok fallback defect is still present on current main and in v0.65.0. This owner-authored PR provides a narrow, source-supported repair; it should remain open for normal review. The separate dashboard omission in #3716 remains unresolved.

Priority: P2
Reviewed head: 2eb5b37b28c0b6326c081e008998fad97d434cf0

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A small, coherent fix has focused subprocess regression coverage and no identified blocking patch defect.
Proof confidence 🌊 off-meta tidepool Not applicable: The owner-authored PR is exempt from the external contributor proof gate. Its reported after-fix run uses the production Grok RPC subprocess against a synthetic CLI and checks retained local tokens; live Grok confirmation remains unverified after the installed-release commands timed out. No stored-data contract changes.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The owner-authored PR is exempt from the external contributor proof gate. Its reported after-fix run uses the production Grok RPC subprocess against a synthetic CLI and checks retained local tokens; live Grok confirmation remains unverified after the installed-release commands timed out. No stored-data contract changes.
Evidence reviewed 7 items Introduced production path: The branch reads the JSON-RPC error code, retains the diagnostic message, and classifies only code -32601 as an unavailable billing method.
Fallback boundary: The fallback still requires an attempted billing call and a non-expired team credential; successful fallback scans local history before constructing the usage snapshot.
Focused regression coverage: The stub CLI cases distinguish alternate wording with code -32601 from misleading wording with another code, and check that the accepted snapshot carries 42 local tokens.
Findings None None.
Security None None.

How this fits together

CodexBar's Grok provider asks the Grok CLI for billing data and turns the result into usage shown in the menu and spend views. When CLI billing fails, the provider decides whether to retain a team identity snapshot or continue to web billing.

flowchart LR
A[Grok CLI] --> B[Billing RPC reply]
B --> C[Error code check]
C --> D[Team credential check]
D --> E[Local token scan]
E --> F[Usage snapshot]
C --> G[Web billing fallback]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

None.

Technical review

Best possible solution:

Land the coded-error classification with its team, personal, and transport regression coverage; investigate the reported account-specific dashboard loss separately.

Do we have a high-confidence way to reproduce the issue?

Yes: the source and stub-CLI cases define a deterministic missing-method response whose alternate wording fails on current main. This read-only review did not execute the tests or verify the behavior against a live Grok account.

Is this the best way to solve the issue?

Yes: preserving the RPC code and classifying -32601 at the existing fallback decision is a narrow repair, while keeping unrelated RPC and transport errors outside that fallback.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 01c81fa1352a.

Labels

Label changes:

  • add P2: The repair addresses a bounded Grok team-account fallback failure without evidence of broader app failure.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The owner-authored PR is exempt from the external contributor proof gate. Its reported after-fix run uses the production Grok RPC subprocess against a synthetic CLI and checks retained local tokens; live Grok confirmation remains unverified after the installed-release commands timed out. No stored-data contract changes.

Label justifications:

  • P2: The repair addresses a bounded Grok team-account fallback failure without evidence of broader app failure.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The owner-authored PR is exempt from the external contributor proof gate. Its reported after-fix run uses the production Grok RPC subprocess against a synthetic CLI and checks retained local tokens; live Grok confirmation remains unverified after the installed-release commands timed out. No stored-data contract changes.

Evidence

What I checked:

Likely related people:

  • unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)
  • unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)
  • unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@steipete
steipete merged commit 371f65b into main Sep 24, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant