feat: multiple accounts#10
Merged
Merged
Conversation
Why: - Make multi-account quota state visible and refreshable without changing the active model. What: - Group extension config and caches under ~/.pi/grok-cli with verified atomic migration. - Add provider-keyed quota caching and in-dialog refresh for authenticated accounts. - Reuse the lowest free provider alias while reserving deferred active removals. Validation: - bun run check (467 tests)
Why: - Keep Grok Build reasoning summaries and replay data without sending unsupported effort controls. What: - Distinguish reasoning support from configurable effort support. - Preserve and deduplicate encrypted reasoning capture for capable models. - Correct the bundled Grok Build context window to 500,000 tokens. Validation: - bun test tests/payload/sanitize.test.ts tests/models/catalog.test.ts - bun run check
Why: - Avoid retrying recently exhausted accounts across adjacent requests while preserving sticky normal routing. What: - Keep exact-402 accounts cooling down for five minutes within the Pi session and clear health on session start or successful relogin. - Rank eligible fallbacks by fresh cached tightest-window quota without fetching billing data. - Preserve circular slots for stale, missing, invalid, or tied quota candidates. Validation: - bun run check (470 tests) - autoreview --mode local
Why: - Give users a local web UI to add, switch, rename, log in/out, and monitor quota across Grok accounts instead of TUI-only flows. What: - Loopback-only server gated by a capability cookie, CSRF header, host check, and strict security headers; browser navigations get an HTML error page instead of raw JSON. - Account grid with quota meters, login progress, and semantic status colors (green ok / amber pending / red error); cyan is reserved for interactive elements. - Add-account modal opens the login popup with the final OAuth ticket URL, so embedded browsers that cannot script an about:blank popup (e.g. WKWebView) still complete login. - Fix danger-button hover contrast where the ghost danger rule bled into the primary variant. Validation: - bun run lint:ci - bun run typecheck - bun run knip - AGENT=1 bun run test (484 passed) Note: depends on the uncommitted account-manager changes in src/provider/accounts.ts; this does not build standalone without them.
Why: - Expose the local account dashboard through the accounts command and share one account-mutation path between the TUI and the dashboard server, completing the dashboard work started in cbc9e03. What: - Extract an AccountManager in accounts.ts that serializes mutations and owns add, rename, activate, login, logout, remove, refresh, and snapshot; the TUI selector and dashboard server both go through it. - Add the /grok-cli-accounts gui subcommand, reject unknown arguments, and close the dashboard on session shutdown. - Adopt pi 0.80.9 APIs: ProviderConfig['oauth'], getProviderAuthStatus, and ModelRuntime login/logout instead of direct authStorage access. - Restyle the dashboard with the Linear-inspired dark theme (indigo accent, inverted primary buttons, subtle inset highlights). - Document the dashboard in README and SECURITY; ignore local agent tooling artifacts in .gitignore. Validation: - bun run check (typecheck, knip, biome lint, 484 tests passed)
Why: - The dashboard went blank on server disconnect, hid login errors behind a "no quota" hint, and the [hidden] utility lost to component display rules. What: - Keep the last good state on screen when the server is unreachable, retry polling every 5s, re-render on recovery, and toast the offline transition only once. - Show a "Logging in…" pill for pending logins, prefer the error text over the no-quota hint, and add empty and loading grid placeholders. - Double the [hidden] selector so it beats component display rules, close the dialog on backdrop click, and label it for assistive tech. Validation: - bun run check (typecheck, knip, biome lint, 484 tests passed)
Why:
- Dashboard copy mixed technical strings with terse labels, leaving
states ambiguous ("usable" vs logged in, "Token instructions") and
server errors opaque ("Dashboard request validation failed.").
What:
- Rewrite user-facing dashboard copy: button labels, dialog messages,
empty states, quota labels, and the stats summary.
- Rewrite server error messages to state what happened and how to
recover; fold the 401 reopen hint into the message itself.
- Rename the environment-account status to "Logged in (environment)".
Validation:
- bun run check (typecheck, knip, biome lint, 484 tests passed)
Why: - Proximity hover expressed as scale did not read on the dense, uniform card grid; cards have no room to grow. What: - Replace scale/brightness proximity with a 1px spotlight ring masked to the card border band, positioned and faded via --mx/--my/--glow custom properties. - The light follows the cursor across cards (220px falloff) without touching layout; far-card dimming removed. - Gated on fine pointers and prefers-reduced-motion; resets when the pointer leaves the grid. Validation: - bun run check - Browser-verified against a stubbed /api/state: falloff gradient, pointer-leave reset, reduced-motion no-op
Why: - Turn the accounts surface into an impressive, informational single-page console where the backdrop itself reports fleet state. What: - WebGL2 domain-warped FBM shader field driven by live state: aggregate quota burn sets energy and palette (indigo → teal → amber), errored or offline states bleed ember through the warp, sync activity shimmers, and the pointer stirs the flow; eased uniforms, half-res rendering, pause on hidden tab, still frame under reduced motion, CSS aurora fallback. - Quota meters replaced by conic-gradient ring gauges on a registered @Property, swept via WAAPI on mount and on poll-driven value changes. - Modern CSS throughout: linear() spring easings, @starting-style and allow-discrete dialog transitions, corner-shape squircles, OKLCH relative colors, container queries, scroll-driven appbar progress hairline, and :has() state signals (offline desaturation, dialog field dim, pending login ring). - Structural mutations (add/remove/switch/login) morph via the View Transitions API with per-card view-transition-name; polling, typing, and reduced-motion renders stay plain. - All existing behavior preserved: polling, login code flow, toasts, modal, offline retry, cursor spotlight. Validation: - bun run check (biome, typecheck, knip, jscpd, vitest coverage) - Playwright: desktop/mobile/dialog/reduced-motion/offline/recovery, zero console errors
Why:
- Icon-only refresh button read as page reload, gauges framed quota as
consumption instead of remaining balance, and free-plan accounts were
shown a weekly row that could never resolve.
What:
- Label the refresh button ("Refresh quotas") and pulse gauges plus a
per-card "Refreshing…" line while a quota refresh is in flight
- Flip gauges to percent-remaining with meter role; monthly side text
shows "N left"; header stats reduced to account count
- Add plan tiers from the monthly credit cap (free, SuperGrok Lite at
4,000, SuperGrok at 20,000, Heavy above) shown as an accent-tinted
pill; free-plan cards render 0% gauges with "Not available"
- Rename account status "Logged in" to "Authenticated"; label input
gets an "Optional" placeholder and the add dialog confirms with "Add"
Validation:
- bun run check
Why: - Separate `/grok-cli-vision:on` and `:off` commands were redundant for a boolean setting. What: - Replace both with `/grok-cli-vision`, which flips enabled state and notifies ON/OFF - Update README and register tests to match the three-command surface Validation: - Not run (doc + command registration change only; tests updated in lockstep)
Why: - "dashboard" was vague about where the GUI appears; the command description also omitted the gui subcommand entirely. What: - Mention gui in the /grok-cli-accounts command description. - Refer to the GUI as the "browser UI" in the description and README. Validation: - bun run check
Why: - Final polish pass surfaced measured WCAG AA contrast failures, motion-rule violations, and geometry drift from the design system. What: - Add --ok-text/--red-text text-on-tint tokens; fix ACTIVE badge (4.04:1), error panel and danger hovers (3.69:1), and login-panel link (4.01:1). - Replace overshoot spring with ease-out-expo on card-enter, dialog, toast. - Align geometry with the system: 10px cards, 6px login panel, 80% appbar scrim, ghost borders at 8% rest / 14% hover; drop scroll-progress hairline. - Add :active press state to buttons; wrap card meta rows against overflow; remove unused interpolate-size. Validation: - bun run check (lint, typecheck, knip, tests) - Browser-verified every account state plus empty, offline, dialog, toast, mobile, and keyboard focus against a mock state server.
Why: - A design audit found WCAG AA contrast failures in the offline state, long labels overflowing card bounds and scrolling the page, and no audible feedback for login progress or account actions. What: - Constrain card titles with min-width: 0 so long labels ellipsize inside the card; the offline filter now desaturates without brightness dimming, keeping dim text at 5.12:1. - Announce login progress through a persistent #sr-status live region and confirm switch, rename, remove, and login outcomes with toasts; card titles are h2s, the appbar separator is aria-hidden, and hidden toasts clear their text. - Gate the state field to its documented 30fps cadence and size it via ResizeObserver; component rules derive status hues from tokens with relative color syntax instead of duplicated raw values. Validation: - bun run check (487 tests, typecheck, knip, jscpd, coverage) - Live browser verification of overflow, contrast, draw cadence, toast clearing, and the Chromium accessibility tree
Why: - Review found malformed reasoning payloads, cross-session dashboard cookie collisions, stale quota resurrection, invalid two-digit account routes, and inaccurate security wording. What: - Normalize reasoning content while preserving valid replay fields. - Isolate dashboard cookies and accept every valid numbered account alias. - Serialize quota refreshes with account mutations so removed aliases cannot regain cached usage. - Document manual authorization-code handling accurately and add focused regressions. Validation: - bun run test -- tests/payload/sanitize.test.ts tests/provider/dashboard.test.ts tests/provider/accounts.test.ts (65 tests) - bun run check (491 tests, typecheck, knip, jscpd, coverage) - npm pack --dry-run --json --ignore-scripts - autoreview --mode local (clean)
Why: - A documentation audit found inaccurate OAuth recovery guidance, incomplete security boundaries, and advanced configuration missing from the published package. What: - Distinguish callback bind failures from manual callback recovery. - Document persisted base URLs, OS-permitted tool reach, and OAuth, image, Imagine, and quota data flows. - Publish CONFIGURATION.md and lock the corrected package inventory. Validation: - bun run check (491 tests, typecheck, knip, jscpd, coverage) - npm pack --dry-run --json --ignore-scripts - git diff --check
Why: - The README introduction needed stronger visual identity and a clearer summary of the current account experience. What: - Add the branded Pi Grok CLI header image. - Reorder feature highlights and describe browser account management and exhaustion rotation. - Keep the image repository-only; the npm package inventory remains unchanged. Validation: - bun run check (491 tests, typecheck, knip, jscpd, coverage) - git diff --cached --check
📝 WalkthroughWalkthroughThis PR adds multi-account Grok CLI support with persisted account configuration, quota caching, automatic exhaustion rotation, a local account dashboard, consolidated storage migration, shared token resolution, reasoning payload normalization, and updated vision, Imagine, OAuth, documentation, and tests. ChangesGrok CLI platform
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant PiExtension
participant Dashboard
participant AccountManager
participant OAuthProvider
participant QuotaCache
User->>PiExtension: Open account management
PiExtension->>Dashboard: Start local dashboard
User->>Dashboard: Add or log in to account
Dashboard->>OAuthProvider: Start OAuth interaction
OAuthProvider-->>Dashboard: Return credentials
Dashboard->>AccountManager: Register authenticated account
AccountManager->>QuotaCache: Refresh and persist quota
Dashboard-->>User: Render account and quota state
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis PR adds multi-account support for the Grok CLI provider. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (2): Last reviewed commit: "fix(dashboard): show free quota as unava..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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: 54cc8476f2
ℹ️ 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".
Comment on lines
+611
to
+614
| const modelId = isGrokCliProvider(ctx.model?.provider) ? ctx.model?.id : DEFAULT_GROK_MODEL; | ||
| const model = | ||
| ctx.modelRegistry.find(provider, modelId ?? DEFAULT_GROK_MODEL) ?? | ||
| ctx.modelRegistry.find(provider, DEFAULT_GROK_MODEL); |
There was a problem hiding this comment.
When PI_GROK_CLI_MODELS is set to a catalog that omits grok-build (the documented filter/reorder override), selecting an authenticated account while the current model is non-Grok cannot activate that account: modelId is forced to grok-build, both lookups miss, and the command/dashboard reports the account unavailable even though other models are registered for that provider. Please fall back to one of the provider's registered models before failing.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (4)
src/payload/sanitize.ts (1)
267-272: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the new
elsebranches.Use independent guarded statements for content assignment, effort removal, and empty reasoning cleanup. This keeps the central sanitization path aligned with repository style without changing behavior.
As per coding guidelines, “Avoid
elsestatements. Prefer early returns.”Also applies to: 323-335
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/payload/sanitize.ts` around lines 267 - 272, In the reasoning sanitization logic around normalizeReasoningContent and the corresponding path near the additional referenced section, remove the new else branches and use independent guarded statements for content assignment, effort removal, and empty reasoning cleanup. Preserve the existing behavior while following the repository’s no-else style.Source: Coding guidelines
src/provider/register.ts (1)
7-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse inferred callback types and functional iteration.
Proposed refactor
-import { type GrokCliModelConfig, resolveModels } from '../models/catalog.js'; +import { resolveModels } from '../models/catalog.js'; ... - models: models.map((m: GrokCliModelConfig) => ({ + models: models.map((m) => ({ ... - for (const account of loadConfig().config.accounts.items) registerAccount(account); + loadConfig().config.accounts.items.forEach(registerAccount);As per coding guidelines, “Rely on type inference when possible” and “Prefer functional array methods over for loops.”
Also applies to: 61-61, 79-79
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/provider/register.ts` at line 7, Refactor the affected code in the provider registration flow, including the import-related usage and the locations around lines 61 and 79, to rely on inferred callback parameter types instead of explicit annotations where TypeScript can infer them. Replace imperative for-loop iteration with appropriate functional array methods while preserving the existing registration behavior.Source: Coding guidelines
src/provider/rotation.ts (1)
184-195: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the final
elsewith an early return.Proposed refactor
if ( authenticated.every( (account) => exhausted.has(account.provider) || isRecentlyExhausted(account.provider, now), ) ) { ctx.ui.notify('Grok CLI: all logged-in accounts are exhausted.', 'warning'); - } else { - ctx.ui.notify( - 'Grok CLI: no other logged-in account is available for automatic rotation.', - 'warning', - ); + clearChain(); + return; } + + ctx.ui.notify( + 'Grok CLI: no other logged-in account is available for automatic rotation.', + 'warning', + ); clearChain();As per coding guidelines, “Avoid
elsestatements. Prefer early returns.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/provider/rotation.ts` around lines 184 - 195, Replace the final else branch in the account-exhaustion notification logic with an early return after notifying that no other logged-in account is available. Keep the all-exhausted notification path unchanged, and ensure the surrounding flow returns immediately before reaching the former fallback notification.Source: Coding guidelines
tests/provider/dashboard.test.ts (1)
362-445: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftReplace source-string assertions with behavioral dashboard tests.
These checks can pass while focus restoration, live-region announcements, toast timing, or rendering behavior is broken. Exercise the served client in a browser-capable DOM harness and assert the observable outcomes.
As per coding guidelines,
**/*.{test,spec}.{ts,tsx}should “Test actual implementation, do not duplicate logic into tests.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/provider/dashboard.test.ts` around lines 362 - 445, Replace the source-string assertions in the dashboard tests with behavioral tests that execute the served page and app.js in a browser-capable DOM harness. Use observable interactions and outcomes to verify DOM safety, accessibility roles and focus/code preservation, live-region announcements, one-time entrance motion, hover-paused toast dismissal, CSS containment/contrast, account-operation confirmations, and state-field throttling/resizing; retain only assertions that validate runtime behavior rather than implementation text.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/config.ts`:
- Around line 470-471: Update the migration flow around migrateStoredFile and
the subsequent saveConfig/semantic verification so the legacy configuration
remains available until those operations succeed. Defer deletion of the source
file until the verification steps corresponding to lines 486-490 complete
successfully, while preserving the existing warning behavior and migration
result handling.
In `@src/provider/accounts.ts`:
- Around line 504-512: Update refresh and refreshOne to perform all network
fetches before calling mutate, then enqueue only a revalidated cache commit that
verifies the account is still present and authenticated before applying results.
Preserve the existing mutation serialization for add, switch, logout, and remove
so refreshes cannot block those operations or restore accounts that were removed
or logged out.
In `@src/provider/dashboard/app.css`:
- Around line 32-36: Update the CSS declarations near the --ui and --mono
variables and the additionally referenced ranges to satisfy Stylelint: quote all
named font-family values, remove deprecated clip declarations, and add the
required declaration spacing. Preserve the existing styling and values apart
from these lint fixes.
In `@src/provider/quotaCache.ts`:
- Around line 85-100: Update updateQuotaCache so the queued next operation runs
after previous settles successfully or with an error, rather than only after
fulfillment. Preserve the existing cache loading, update, atomic write, queue
tracking, and cleanup behavior.
In `@src/provider/rotation.ts`:
- Around line 133-174: Reload the configuration after the awaited
pi.setModel(model) call and use that refreshed configuration for failedLabel,
selected account lookup, selectedProvider assignment, and saveConfig. Do not
save the stale config captured before the await, so concurrent dashboard account
changes remain intact.
In `@src/provider/usage.ts`:
- Line 38: Update the fetchBillingUsage call in the usage command to enforce the
same 30-second timeout used by account refresh requests. Pass the timeout
through the existing request configuration mechanism without changing the
surrounding usage handling.
In `@tests/storage.test.ts`:
- Around line 73-91: Update the test around migrateLegacyConfig to make
directory creation fail deterministically: replace the chmodSync permission
manipulation with creation of a regular file at getGrokCliDirectory(), ensuring
child-path creation consistently fails across users and platforms while
preserving the existing fallback assertions and cleanup.
---
Nitpick comments:
In `@src/payload/sanitize.ts`:
- Around line 267-272: In the reasoning sanitization logic around
normalizeReasoningContent and the corresponding path near the additional
referenced section, remove the new else branches and use independent guarded
statements for content assignment, effort removal, and empty reasoning cleanup.
Preserve the existing behavior while following the repository’s no-else style.
In `@src/provider/register.ts`:
- Line 7: Refactor the affected code in the provider registration flow,
including the import-related usage and the locations around lines 61 and 79, to
rely on inferred callback parameter types instead of explicit annotations where
TypeScript can infer them. Replace imperative for-loop iteration with
appropriate functional array methods while preserving the existing registration
behavior.
In `@src/provider/rotation.ts`:
- Around line 184-195: Replace the final else branch in the account-exhaustion
notification logic with an early return after notifying that no other logged-in
account is available. Keep the all-exhausted notification path unchanged, and
ensure the surrounding flow returns immediately before reaching the former
fallback notification.
In `@tests/provider/dashboard.test.ts`:
- Around line 362-445: Replace the source-string assertions in the dashboard
tests with behavioral tests that execute the served page and app.js in a
browser-capable DOM harness. Use observable interactions and outcomes to verify
DOM safety, accessibility roles and focus/code preservation, live-region
announcements, one-time entrance motion, hover-paused toast dismissal, CSS
containment/contrast, account-operation confirmations, and state-field
throttling/resizing; retain only assertions that validate runtime behavior
rather than implementation text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e8e07be8-af12-46f8-9a17-605ac2f805b9
⛔ Files ignored due to path filters (2)
.github/assets/pi-grok-cli.pngis excluded by!**/*.pngbun.lockis excluded by!**/*.lock
📒 Files selected for processing (46)
.gitignoreCONFIGURATION.mdREADME.mdSECURITY.mdpackage.jsonsrc/auth/grokCredentials.tssrc/auth/oauth.tssrc/config.tssrc/imagine/auth.tssrc/models/catalog.tssrc/payload/sanitize.tssrc/provider/accounts.tssrc/provider/billing.tssrc/provider/dashboard/app.csssrc/provider/dashboard/app.jssrc/provider/dashboard/index.htmlsrc/provider/dashboard/server.tssrc/provider/quotaCache.tssrc/provider/register.tssrc/provider/rotation.tssrc/provider/toolScope.tssrc/provider/usage.tssrc/storage.tssrc/vision/cache.tssrc/vision/describe.tssrc/vision/register.tstests/auth/grokCredentials.test.tstests/auth/oauth.test.tstests/config.test.tstests/imagine/config.test.tstests/imagine/register.test.tstests/models/catalog.test.tstests/payload/sanitize.test.tstests/provider/accounts.test.tstests/provider/dashboard.test.tstests/provider/package.test.tstests/provider/quotaCache.test.tstests/provider/register.test.tstests/provider/rotation.test.tstests/provider/toolScope.integration.test.tstests/provider/toolScope.test.tstests/storage.test.tstests/vision/config.test.tstests/vision/describe.test.tstests/vision/helpers.tstests/vision/register.test.ts
💤 Files with no reviewable changes (2)
- tests/auth/grokCredentials.test.ts
- src/auth/grokCredentials.ts
Why: - Overlapping account, model, quota, replay, and dashboard lifecycle work could restore stale state, emit invalid payloads, or block shutdown. What: - Serialize model persistence without re-entrant waits and guard quota writes by account generation. - Keep only valid reasoning content discriminators. - Bound incomplete dashboard bodies and close active connections during shutdown. Validation: - bun run check (495 tests, typecheck, knip, jscpd, coverage) - autoreview --mode local (clean)
Why: - Migration, refresh, and rotation races could discard legacy or concurrent state, while dashboard tests relied on source-string checks. What: - Defer legacy cleanup and revalidate asynchronous account and quota commits. - Preserve concurrent rotation changes and align timeout, style, sanitization, and registration behavior. - Execute dashboard client behavior in a browser-capable DOM and cover the affected concurrency paths. Validation: - bun run check (38 test files, 499 tests)
Why: - Free-plan accounts can include weekly billing data, which made the dashboard show 100% remaining with a reset date. What: - Make the free-plan state override weekly payload values for both quota metadata and percentage. - Add a browser-behavior regression covering monthly and weekly free-plan gauges. Validation: - bun run check (38 test files, 500 tests)
Owner
Author
|
@greptileai review |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/provider/dashboard.test.ts`:
- Around line 824-830: Increase the outer Promise.race watchdog delays in the
socket data and close checks around the response and connectionClosed flows from
100 ms to a CI-safe margin of about one second. Preserve the existing 20 ms
server timeout and the current timeout fallback results.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 28655703-c85a-4acb-8a5e-f31b9bf60f01
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
package.jsonsrc/config.tssrc/payload/sanitize.tssrc/provider/accounts.tssrc/provider/dashboard/app.csssrc/provider/dashboard/app.jssrc/provider/dashboard/server.tssrc/provider/quotaCache.tssrc/provider/register.tssrc/provider/rotation.tssrc/provider/usage.tssrc/storage.tstests/payload/sanitize.test.tstests/provider/accounts.test.tstests/provider/dashboard.test.tstests/provider/quotaCache.test.tstests/provider/register.test.tstests/provider/rotation.test.tstests/storage.test.ts
🚧 Files skipped from review as they are similar to previous changes (16)
- src/provider/quotaCache.ts
- package.json
- src/provider/usage.ts
- src/storage.ts
- tests/provider/quotaCache.test.ts
- src/provider/rotation.ts
- src/payload/sanitize.ts
- tests/provider/accounts.test.ts
- src/provider/dashboard/app.css
- src/provider/accounts.ts
- src/config.ts
- tests/provider/register.test.ts
- tests/payload/sanitize.test.ts
- src/provider/dashboard/app.js
- src/provider/register.ts
- tests/provider/rotation.test.ts
Comment on lines
+824
to
+830
| const response = await Promise.race([ | ||
| once(socket, 'data').then(([data]) => data.toString()), | ||
| new Promise<string>((resolve) => setTimeout(() => resolve(''), 100)), | ||
| ]); | ||
| const connectionClosed = await Promise.race([ | ||
| once(socket, 'close').then(() => true), | ||
| new Promise<boolean>((resolve) => setTimeout(() => resolve(false), 100)), |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Use a CI-safe watchdog for real socket operations.
These 100 ms deadlines can fail intermittently under event-loop or runner load despite correct server behavior. Keep the 20 ms server timeout, but give the outer test watchdog a more generous margin such as one second.
Also applies to: 843-846
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/provider/dashboard.test.ts` around lines 824 - 830, Increase the outer
Promise.race watchdog delays in the socket data and close checks around the
response and connectionClosed flows from 100 ms to a CI-safe margin of about one
second. Preserve the existing 20 ms server timeout and the current timeout
fallback results.
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.
Summary by CodeRabbit