Skip to content

fix(server): report actionable Grok usage-limit probe failures - #17062

Closed
jieyuexing wants to merge 2 commits into
pingdotgg:mainfrom
jieyuexing:fix/grok-usage-error-notices
Closed

jieyuexing wants to merge 2 commits into
pingdotgg:mainfrom
jieyuexing:fix/grok-usage-error-notices

Conversation

@jieyuexing

Copy link
Copy Markdown

Problem

Every failed Grok usage probe currently displays “Grok could not read usage limits.” A network timeout, a rejected login, an unreadable credential file, and an invalid billing response therefore look identical, even though they require different recovery steps.

Change

Return bounded diagnostic messages for credential-read errors, request timeouts, transport failures, invalid responses, and HTTP failures. Only 401 recommends reconnecting; 403 does not claim that the login has expired. Keep the account and the existing unavailable reason, without returning response bodies, tokens, or raw exception details.

Scope and approval

This is a focused diagnostic fix at the existing Grok usage-probe boundary. It changes the text of already-reported failures and does not introduce a setting, retry policy, authentication flow, or wire-schema change.

Verification

  • vp test run apps/server/src/provider/GrokProvider.test.ts: 34 passed on this branch.
  • HTTP fixtures cover 401, 403, 429, 503, another status (418), malformed JSON, and invalid schema data. Credential and transport failures retain safe messages without leaking private response text or causes.
  • The malformed-JSON regression initially exposed the wrong connection-failure message; the fix recognizes the HTTP client's decode/empty-body errors as invalid responses.
  • Timeout classification uses Effect 4's actual TimeoutError tag. The existing ten-second timer itself was not exercised.
  • Targeted lint, formatting, and the integrated server TypeScript check passed. Production desktop/account state was not changed.

Implemented with GPT-6 Astra via Codex in T3 Code.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 8, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 8, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at e3b533d

Macroscope's review found this PR approvable — This is a focused Grok diagnostics fix that preserves the existing usage-limit contract and successful behavior while distinguishing credential, HTTP, timeout, transport, and malformed-response failures with sanitized messages. The production change is self-contained and covered by expanded tests, with no product-default, schema, authentication-flow, or billing-decision changes.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 31784e80-c2d0-4bde-84ab-bbe1241923f0

📥 Commits

Reviewing files that changed from the base of the PR and between 15852fb and e3b533d.


📒 Files selected for processing (3)
  • apps/server/src/provider/GrokProvider.test.ts
  • packages/provider-grok/src/server/usage.test.ts
  • packages/provider-grok/src/server/usageLimits.ts

💤 Files with no reviewable changes (1)
  • apps/server/src/provider/GrokProvider.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.



📝 Walkthrough

Walkthrough

Grok usage-limit failures now return classified messages for credential, HTTP status, response decoding, and transport errors. Tests cover these failures, missing and malformed credentials, and retention of the account email when billing probes fail.

Changes

Grok Usage-Limit Failure Handling

Layer / File(s) Summary
Map usage-limit failures
packages/provider-grok/src/server/usageLimits.ts, packages/provider-grok/src/server/usage.test.ts, apps/server/src/provider/GrokProvider.test.ts
The usage-limit code returns specific messages for credential-read failures, HTTP statuses, response errors, and transport errors. Tests check the results and account-email retention. Related tests were removed from GrokProvider.test.ts.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge


Merge Risk: ⚪ Minimal · up to e3b53

The changed failure messages have no identified merge-blocking issue. The Grok usage-limit probe is ready to merge after normal checks.

Architecture Summary

Architecture risk: 🟡 Medium · up to e3b53

The change affects 2 systems.

Changed systems: packages/provider-grok, apps/server

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — packages/provider-grok (library) was modified; 2 changed files map to changed impact.
  • observed — apps/server (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in apps/server/src/provider/GrokProvider.test.ts: Removed a test that expected missing credentials to produce an unsupported result and malformed credentials to produce a sanitized probeFailed result.
  • observed — Modified behavior in apps/server/src/provider/GrokProvider.test.ts: Removed a test that expected HTTP errors and malformed billing responses to retain the account email while returning no usage windows and a sanitized probeFailed result.
  • observed — Modified behavior in packages/provider-grok/src/server/usage.test.ts: Added imports for Effect, filesystem and HTTP test services, and readGrokAccount.
  • observed — Modified behavior in packages/provider-grok/src/server/usage.test.ts: Added a test expecting a missing login to be unsupported and two malformed credential inputs to produce the same sanitized probeFailed message. The HTTP client is configured to fail if requested.

Reliability and maintainability

  • inferred — Risk-relevant change factors for packages/provider-grok: blast_radius_1; direct_dependents_1

Pre-merge checks | Passed 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description check Passed The description covers the problem, implementation, scope rationale, and focused verification. It identifies the changed failure classes, safety constraints, test results, limitations, and agent detai…
Title check Passed The title is concise, conventional, and accurately describes the main change: reporting actionable Grok usage-limit probe failures.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

jieyuexing and others added 2 commits October 11, 2026 14:22
Co-Authored-By: Codex (gpt-6-astra) <noreply@openai.com>
Co-Authored-By: Codex (gpt-6-astra) <noreply@openai.com>
@jieyuexing
jieyuexing force-pushed the fix/grok-usage-error-notices branch from 15852fb to e3b533d Compare October 11, 2026 06:37
@jieyuexing

Copy link
Copy Markdown
Author

Note

Drafted by GPT-6 Astra in Codex and Claude Opus 5.5 in Claude Code (both running inside T3 Code); reviewed and posted by @jieyuexing

Rebased onto current main (5047ee7). #17357 moved the Grok usage code into packages/provider-grok, so the fix now lives in packages/provider-grok/src/server/usageLimits.ts and the probe-failure tests moved into that package's usage.test.ts. The two server tests that asserted the generic message moved with them.

  • On main without the fix, 3 tests fail with the generic "Grok could not read usage limits." message.
  • With the fix, usage.test.ts passes (13 tests) and apps/server/src/provider/GrokProvider.test.ts passes (31). Targeted lint, fmt and the provider-grok typecheck are clean.

Not covered: no live Grok billing request.

@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 11, 2026 — with ChatGPT Codex Connector
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 11, 2026 06:45

Dismissing prior approval to re-evaluate e3b533d

@maria-rcks

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

Thanks for improving the Grok diagnostics. The patch classifies existing failure messages while leaving credential reads and usage-probe outcomes unchanged. Right now we are only taking focused bug fixes from outside contributors because of our limited review capacity. If the generic message blocks recovery, please open an issue with that reproduction so we can agree on a focused fix for reconsideration.

@maria-rcks maria-rcks closed this Oct 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants