[codex] Improve TUI insufficient credit recovery#724
Merged
rahulkarajgikar merged 8 commits intotruffle-ai:mainfrom Apr 14, 2026
Merged
[codex] Improve TUI insufficient credit recovery#724rahulkarajgikar merged 8 commits intotruffle-ai:mainfrom
rahulkarajgikar merged 8 commits intotruffle-ai:mainfrom
Conversation
# Conflicts: # packages/tui/src/services/processStream.test.ts
|
@rahulkarajgikar is attempting to deploy a commit to the Shaunak's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an interactive billing recovery flow: new CLI/TUI billing API client methods, auth helpers and browser-launch wrapper, TUI host/runtime wiring, state/types/overlay for insufficient-credits, a multi-step checkout overlay, and integrations to surface the overlay on LLM insufficient-credits errors. Changes
Sequence DiagramsequenceDiagram
participant User as User (TUI)
participant TUI as InsufficientCreditsOverlay
participant Host as TUI Host / Runtime Services
participant API as Dexto API
participant Browser as Browser
User->>TUI: Overlay shown (insufficient credits)
TUI->>TUI: Show amount selector
User->>TUI: Select top-up amount
TUI->>Host: createBillingCheckoutForCurrentLogin(creditsUsd)
Host->>API: POST /api/billing/checkout-session
API-->>Host: { checkoutUrl, checkoutSessionId }
Host-->>TUI: Checkout session returned
TUI->>Host: openDextoBillingPage(checkoutUrl)
Host->>Browser: open(checkoutUrl)
Browser-->>User: Payment page opened
User->>Browser: Completes payment
User->>TUI: Return to CLI
TUI->>Host: getBillingBalanceForCurrentLogin()
Host->>API: GET /api/billing/balance
API-->>Host: { creditsUsd }
Host-->>TUI: Updated balance
TUI->>User: Transition to success and close
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
packages/cli/src/cli/auth/api-client.ts (1)
671-710: Align new billing methods with repo options/logging conventions.The new APIs reintroduce optional/defaulted options objects and unstructured error logs. Please keep required options objects and structured logger context in these new methods.
♻️ Suggested refactor
+interface GetBillingBalanceOptions { + signal: AbortSignal | null; +} + +interface CreateBillingCheckoutSessionOptions { + creditsUsd: number; + returnUrl: string | null; + signal: AbortSignal | null; +} + - async getBillingBalance( - authToken: string, - options: RequestOptions = {} - ): Promise<BillingBalanceResponse> { + async getBillingBalance( + authToken: string, + options: GetBillingBalanceOptions + ): Promise<BillingBalanceResponse> { @@ - signal: this.createRequestSignal(options.signal), + signal: this.createRequestSignal(options.signal ?? undefined), @@ - logger.error(`Error fetching billing balance: ${error}`); + logger.error('Error fetching billing balance', { + error: error instanceof Error ? error.message : String(error), + }); throw error; } } @@ - options: { - creditsUsd: number; - returnUrl?: string | undefined; - signal?: AbortSignal | undefined; - } + options: CreateBillingCheckoutSessionOptions @@ - ...(options.returnUrl ? { return_url: options.returnUrl } : {}), + ...(options.returnUrl !== null ? { return_url: options.returnUrl } : {}), @@ - logger.error(`Error creating billing checkout session: ${error}`); + logger.error('Error creating billing checkout session', { + error: error instanceof Error ? error.message : String(error), + creditsUsd: options.creditsUsd, + }); throw error; } }As per coding guidelines, “Avoid optional parameters…prefer single required function signatures and explicit options objects with internal defaults” and “Prefer structured logging with logger.info('Message', { contextKey: value }).”
Also applies to: 695-698, 704-710, 737-740
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/cli/auth/api-client.ts` around lines 671 - 710, The new billing methods (getBillingBalance and createBillingCheckoutSession) currently accept optional/defaulted options and log errors with unstructured strings; change their signatures to require an explicit options object (remove defaulting to {} and make the parameter non-optional) and initialize any internal defaults inside the function body, and replace unstructured logger calls with structured logging (e.g., logger.debug('Fetching billing balance', { authTokenMask: mask(authToken), ...options }) and logger.error('Error fetching billing balance', { error, status: response?.status, rawText })) so logs include context; update all similar occurrences in this file (see getBillingBalance, createBillingCheckoutSession and the related error handling blocks around the response.ok checks) to follow the same pattern.packages/cli/src/cli/auth/billing.ts (1)
36-37: Prefer an options object foropenDextoBillingPage.
openDextoBillingPage(url = DEXTO_CREDITS_URL)adds another optional positional parameter to the public billing API, and the TUI host wrapper now has to mirror that shape. Please switch this to a required options object with the default applied internally.As per coding guidelines, "Avoid optional parameters, overload signatures, and fallback union types; prefer single required function signatures and explicit options objects with internal defaults."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/cli/auth/billing.ts` around lines 36 - 37, Change the public API of openDextoBillingPage to accept a single required options object (e.g., options: { url?: string }) instead of an optional positional url parameter; inside the function apply the default DEXTO_CREDITS_URL when options.url is undefined and call openBrowserUrl with that resolved URL. Update any callers (including the TUI host wrapper) to pass an options object rather than a bare string, and keep the function name openDextoBillingPage and the use of openBrowserUrl and DEXTO_CREDITS_URL intact so callers are easy to locate and update.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/cli/utils/config-validation.test.ts`:
- Around line 51-64: The test mock currently overrides provider requirement
helpers (requiresApiKey and requiresBaseURL), hiding the codex-specific path;
restore the real behavior by not stubbing requiresApiKey/requiresBaseURL (or
mirror their real implementations) and only stub side effects like logger and
resolveApiKeyForProvider/getPrimaryApiKeyEnvVar as needed so the codex bypass
path is exercised; locate the vi.mock block and remove or replace the mocked
requiresApiKey/requiresBaseURL with the actual helper implementations (or call
through to the real module) while keeping logger and resolveApiKeyForProvider
mocked.
- Around line 33-49: The mocked AgentConfigSchema uses z.object() for the root
and nested objects (llm and permissions) but is not strict; update the mock so
each z.object(...) call for AgentConfigSchema, the llm object, and the
permissions object is converted to a strict schema (i.e., use .strict() on those
z.object(...) results) so unknown fields are rejected—look for the
AgentConfigSchema declaration and the nested llm and permissions z.object
definitions to apply the change.
In `@packages/tui/src/components/overlays/InsufficientCreditsOverlay.tsx`:
- Around line 246-273: When openDextoBillingPage() fails (e.g., in
headless/SSH/CI), surface the created billing/checkout URL in the overlay so
users can copy it instead of being stuck; update the catch blocks around
openDextoBillingPage() (and the similar block at the other occurrence) to
extract the URL returned/available from openDextoBillingPage() or its error,
call setOpenedTarget with that URL (e.g., setOpenedTarget({ kind: 'url', url }))
and setErrorMessage/getStatusMessage appropriately so the overlay shows the
checkout link and still sets step via setStep('waiting') and clears loading via
setIsLoading(false) using the existing isActiveRef.current checks.
- Around line 196-229: The current "balanceIncreased" logic can mark
small/negative-to-less-negative changes as recovery; change the gate so success
is only when the new balance is positive. Update the check around
getBillingBalanceForCurrentLogin()/nextBalanceUsd so that you only call
setStep('success')/setSelectedIndex(0)/setStatusMessage(...) when nextBalanceUsd
!== null AND nextBalanceUsd > 0 AND it is strictly greater than the previous
balance (use balanceUsd as previous); keep the existing null handling
(nextBalanceUsd === null => "Balance is still unavailable") and the fallback
waiting/status messages unchanged.
---
Nitpick comments:
In `@packages/cli/src/cli/auth/api-client.ts`:
- Around line 671-710: The new billing methods (getBillingBalance and
createBillingCheckoutSession) currently accept optional/defaulted options and
log errors with unstructured strings; change their signatures to require an
explicit options object (remove defaulting to {} and make the parameter
non-optional) and initialize any internal defaults inside the function body, and
replace unstructured logger calls with structured logging (e.g.,
logger.debug('Fetching billing balance', { authTokenMask: mask(authToken),
...options }) and logger.error('Error fetching billing balance', { error,
status: response?.status, rawText })) so logs include context; update all
similar occurrences in this file (see getBillingBalance,
createBillingCheckoutSession and the related error handling blocks around the
response.ok checks) to follow the same pattern.
In `@packages/cli/src/cli/auth/billing.ts`:
- Around line 36-37: Change the public API of openDextoBillingPage to accept a
single required options object (e.g., options: { url?: string }) instead of an
optional positional url parameter; inside the function apply the default
DEXTO_CREDITS_URL when options.url is undefined and call openBrowserUrl with
that resolved URL. Update any callers (including the TUI host wrapper) to pass
an options object rather than a bare string, and keep the function name
openDextoBillingPage and the use of openBrowserUrl and DEXTO_CREDITS_URL intact
so callers are easy to locate and update.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 813b6809-2370-4520-96e1-d0df8aa0226d
📒 Files selected for processing (22)
.changeset/ink-billing-recovery.mdpackages/cli/src/cli/auth/api-client.test.tspackages/cli/src/cli/auth/api-client.tspackages/cli/src/cli/auth/billing.tspackages/cli/src/cli/auth/browser-launch.tspackages/cli/src/cli/auth/index.tspackages/cli/src/cli/auth/service.tspackages/cli/src/cli/commands/billing/status.tspackages/cli/src/cli/commands/setup.test.tspackages/cli/src/cli/commands/setup.tspackages/cli/src/cli/modes/cli.test.tspackages/cli/src/cli/modes/cli.tspackages/cli/src/cli/utils/config-validation.test.tspackages/tui/src/components/overlays/InsufficientCreditsOverlay.tsxpackages/tui/src/containers/OverlayContainer.tsxpackages/tui/src/hooks/useAgentEvents.tspackages/tui/src/hooks/useCLIState.tspackages/tui/src/host/index.tspackages/tui/src/services/processStream.test.tspackages/tui/src/services/processStream.tspackages/tui/src/state/initialState.tspackages/tui/src/state/types.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/core/src/llm/executor/stream-processor.test.ts (1)
1243-1284:⚠️ Potential issue | 🟡 MinorAdd the
tool-call→ fatalerrorregression as well.These assertions lock the new throw/no-
llm:errorbehavior, but the risky path is a fatal stream error after a tool call has already been persisted. A test that emitstool-calland thenerrorwould catch regressions in the tool-use/tool-result pairing invariant.Based on learnings: When fixing bugs, add regression coverage where feasible.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/llm/executor/stream-processor.test.ts` around lines 1243 - 1284, Add a regression test that emits a tool-call event followed by a fatal error to ensure a persisted tool call doesn't break the fatal-error handling: using createMocks(), instantiate StreamProcessor and call processor.process(() => createMockStream(events) as never) where events = [{ type: 'tool-call', ...toolCallPayload }, { type: 'error', error: new Error('...') }] and assert the promise rejects with that Error (or matches its message for non-Error) and that mocks.emittedEvents still contains the tool-call emission (e.g. find by name or payload) while ensuring no 'llm:error' event was emitted; place it alongside the existing tests that call StreamProcessor.process and reuse createMocks/createMockStream to mirror the other error tests.packages/core/src/llm/executor/stream-processor.ts (1)
541-547:⚠️ Potential issue | 🟠 MajorPersist pending tool results before bailing out on fatal stream errors.
If the SDK emits
type: 'error'after atool-callhas already been persisted, this immediate throw leavespendingToolCallsuntouched. That produces history with atool_usebut no matchingtool_result, which is exactly what the abort andtool-errorpaths already defend against.💡 Suggested direction
case 'error': { const err = event.error instanceof Error ? event.error : new Error(String(event.error)); - throw err; + await this.persistFailedToolResults( + err.message + ); + throw err; }Then mirror
persistCancelledToolResults()with a helper that records a synthetic failedtool_resultfor every entry still inpendingToolCalls.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/llm/executor/stream-processor.ts` around lines 541 - 547, When handling the 'error' event in stream-processor.ts, before throwing the error, iterate over the pendingToolCalls map and persist synthetic failed tool_result entries (mirroring the behavior of persistCancelledToolResults()) so every recorded tool_use has a corresponding tool_result; add a helper (e.g., persistFailedPendingToolResults or extend persistCancelledToolResults) that creates and stores a failed tool_result for each key in pendingToolCalls, clears pendingToolCalls, and then rethrows the original Error (constructed as currently done) to preserve existing abort/tool-error defenses.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/core/src/llm/executor/stream-processor.test.ts`:
- Around line 1243-1284: Add a regression test that emits a tool-call event
followed by a fatal error to ensure a persisted tool call doesn't break the
fatal-error handling: using createMocks(), instantiate StreamProcessor and call
processor.process(() => createMockStream(events) as never) where events = [{
type: 'tool-call', ...toolCallPayload }, { type: 'error', error: new
Error('...') }] and assert the promise rejects with that Error (or matches its
message for non-Error) and that mocks.emittedEvents still contains the tool-call
emission (e.g. find by name or payload) while ensuring no 'llm:error' event was
emitted; place it alongside the existing tests that call StreamProcessor.process
and reuse createMocks/createMockStream to mirror the other error tests.
In `@packages/core/src/llm/executor/stream-processor.ts`:
- Around line 541-547: When handling the 'error' event in stream-processor.ts,
before throwing the error, iterate over the pendingToolCalls map and persist
synthetic failed tool_result entries (mirroring the behavior of
persistCancelledToolResults()) so every recorded tool_use has a corresponding
tool_result; add a helper (e.g., persistFailedPendingToolResults or extend
persistCancelledToolResults) that creates and stores a failed tool_result for
each key in pendingToolCalls, clears pendingToolCalls, and then rethrows the
original Error (constructed as currently done) to preserve existing
abort/tool-error defenses.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 09d8c7c4-87ea-49fd-98c6-8e579375b1ae
📒 Files selected for processing (6)
packages/core/src/agent/DextoAgent.lifecycle.test.tspackages/core/src/agent/DextoAgent.tspackages/core/src/llm/executor/stream-processor.test.tspackages/core/src/llm/executor/stream-processor.tspackages/tui/src/services/processStream.test.tspackages/tui/src/services/processStream.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/tui/src/services/processStream.test.ts
- packages/tui/src/services/processStream.ts
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli/src/cli/auth/login-persistence.ts (1)
62-84:⚠️ Potential issue | 🟠 MajorDon't downgrade a preserved managed key to
hasDextoApiKey: falseon transient ensure failures.Lines 66-74 already write
preservedDextoApiKeyinto auth beforeensureDextoApiKeyForAuthToken()runs. If that ensure step later returnsnullbecause validation/provisioning had a transient failure, Lines 80-84 still reporthasDextoApiKey: falseeven though auth now contains the same-user provisioned key.OverlayContaineruses that flag to skip the post-login Dexto Nova refresh, so relogin can leave the active billing-backed session stale even after the key was preserved. Please fall back to the preserved key metadata here, or make the ensure path distinguish transport failures from “no usable key”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/cli/auth/login-persistence.ts` around lines 62 - 84, The returned metadata currently overwrites preserved managed-key info when ensureDextoApiKeyForAuthToken() fails (returns null) causing hasDextoApiKey to become false; change the return assembly to prefer ensured values but fall back to preservedDextoApiKey (from getPreservedDextoApiKey/loadAuth/storeAuth) so keyId and hasDextoApiKey reflect the preserved key when ensure is transiently failing—i.e., compute keyId = ensured?.keyId ?? preservedDextoApiKey?.keyId and hasDextoApiKey = Boolean(ensured?.dextoApiKey ?? preservedDextoApiKey?.dextoApiKey).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/cli/auth/api-client.ts`:
- Around line 703-724: The createBillingCheckoutSession function currently
forwards any creditsUsd to the platform and can send 0, negative or non-finite
values; add a client-side guard at the top of createBillingCheckoutSession to
validate options.creditsUsd is a finite number > 0 and throw a clear Error (or
reject) with a descriptive message if not; keep the rest of the behavior
(logging, calling getPlatformUrl, fetch, and using createRequestSignal)
unchanged so invalid amounts never reach the billing API and callers receive a
fast, explicit failure.
In `@packages/tui/src/utils/dexto-auth-refresh.ts`:
- Around line 29-35: The code is creating a session-specific override by calling
switchLLM whenever getCurrentLLMConfig(sessionId) resolves to 'dexto-nova' even
if the session is only inheriting the global config; instead, detect whether the
session actually has an explicit LL M override (e.g., check for an override flag
or use an API like a session-specific override getter rather than
getCurrentLLMConfig), and only call
agent.switchLLM(buildDextoNovaRefreshUpdate(...), sessionId) when that explicit
override exists; otherwise implement a refresh path that re-reads the env-backed
DEXTO API key / global model (for example call a global refresh function or
update the global provider config) so you do not create a per-session
override—reference getCurrentLLMConfig, switchLLM, buildDextoNovaRefreshUpdate,
sessionId, and llmOverride/OverlayContainer when making the change.
---
Outside diff comments:
In `@packages/cli/src/cli/auth/login-persistence.ts`:
- Around line 62-84: The returned metadata currently overwrites preserved
managed-key info when ensureDextoApiKeyForAuthToken() fails (returns null)
causing hasDextoApiKey to become false; change the return assembly to prefer
ensured values but fall back to preservedDextoApiKey (from
getPreservedDextoApiKey/loadAuth/storeAuth) so keyId and hasDextoApiKey reflect
the preserved key when ensure is transiently failing—i.e., compute keyId =
ensured?.keyId ?? preservedDextoApiKey?.keyId and hasDextoApiKey =
Boolean(ensured?.dextoApiKey ?? preservedDextoApiKey?.dextoApiKey).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 73d8bed3-1d45-430f-b598-0c63a25b0157
📒 Files selected for processing (9)
packages/cli/src/cli/auth/api-client.test.tspackages/cli/src/cli/auth/api-client.tspackages/cli/src/cli/auth/dexto-api-key.tspackages/cli/src/cli/auth/login-persistence.test.tspackages/cli/src/cli/auth/login-persistence.tspackages/cli/src/cli/commands/auth/login.tspackages/tui/src/containers/OverlayContainer.tsxpackages/tui/src/utils/dexto-auth-refresh.test.tspackages/tui/src/utils/dexto-auth-refresh.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/cli/src/cli/auth/api-client.test.ts
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/tui/src/components/overlays/InsufficientCreditsOverlay.tsx (2)
205-217:⚠️ Potential issue | 🟠 MajorTighten the recovery gate to require a positive balance.
The current
balanceIncreasedlogic marks any increase as success, including transitions like-$1.00 → -$0.10or-$0.04 → $0.00. In both cases the balance is still non-positive and the user cannot use the service. The success gate should additionally requirenextBalanceUsd > 0.🐛 Proposed fix to require positive balance for success
const balanceIncreased = nextBalanceUsd !== null && + nextBalanceUsd > 0 && ((previousBalanceUsd === null && nextBalanceUsd > 0) || - (previousBalanceUsd !== null && nextBalanceUsd > previousBalanceUsd)); + (previousBalanceUsd !== null && nextBalanceUsd > previousBalanceUsd));Or simplified:
-const balanceIncreased = - nextBalanceUsd !== null && - ((previousBalanceUsd === null && nextBalanceUsd > 0) || - (previousBalanceUsd !== null && nextBalanceUsd > previousBalanceUsd)); +const balanceIncreased = + nextBalanceUsd !== null && + nextBalanceUsd > 0 && + (previousBalanceUsd === null || nextBalanceUsd > previousBalanceUsd);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tui/src/components/overlays/InsufficientCreditsOverlay.tsx` around lines 205 - 217, The current balanceIncreased check (using previousBalanceUsd and nextBalanceUsd) treats any increase as recovery; change it so recovery requires nextBalanceUsd > 0 as well. Update the condition that computes balanceIncreased in InsufficientCreditsOverlay (the variables previousBalanceUsd and nextBalanceUsd) to only return true when nextBalanceUsd !== null AND nextBalanceUsd > 0 in addition to the existing increase logic, so that setStep('success'), setSelectedIndex(0) and setStatusMessage(...) only run when the user has a positive balance.
300-310:⚠️ Potential issue | 🟠 MajorSurface the checkout URL when browser launch fails.
In headless, SSH, or CI environments,
openDextoBillingPage()throws but the checkout session was already created successfully. The user sees "Checkout was created successfully" but cannot access the URL. The "Reopen checkout" option will also fail in the same environment. Consider including the URL in the status message so users can copy it manually.🛠️ Proposed fix to include the checkout URL
} catch (error) { if (!isActiveRef.current) { return; } setStep('waiting'); setSelectedIndex(0); setErrorMessage(getErrorMessage(error)); setStatusMessage( - 'Checkout was created successfully. Open it again once your browser is ready.' + `Checkout was created successfully but browser could not open. URL: ${checkout.checkoutUrl}` ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tui/src/components/overlays/InsufficientCreditsOverlay.tsx` around lines 300 - 310, When openDextoBillingPage() throws after a successful checkout/session creation, update the catch block in InsufficientCreditsOverlay.tsx to include the checkout URL in the user-facing status message so users can copy it manually; keep the existing isActiveRef.current guard and existing state updates (setStep, setSelectedIndex, setErrorMessage) but change the setStatusMessage call to append the URL (use the created session object or variable like checkoutUrl, checkoutSession.url, or similar from the surrounding code) and format it clearly (e.g., "Checkout created: <URL> — open manually"). Ensure the code still handles missing URL gracefully by falling back to the current message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/tui/src/components/overlays/InsufficientCreditsOverlay.tsx`:
- Around line 205-217: The current balanceIncreased check (using
previousBalanceUsd and nextBalanceUsd) treats any increase as recovery; change
it so recovery requires nextBalanceUsd > 0 as well. Update the condition that
computes balanceIncreased in InsufficientCreditsOverlay (the variables
previousBalanceUsd and nextBalanceUsd) to only return true when nextBalanceUsd
!== null AND nextBalanceUsd > 0 in addition to the existing increase logic, so
that setStep('success'), setSelectedIndex(0) and setStatusMessage(...) only run
when the user has a positive balance.
- Around line 300-310: When openDextoBillingPage() throws after a successful
checkout/session creation, update the catch block in
InsufficientCreditsOverlay.tsx to include the checkout URL in the user-facing
status message so users can copy it manually; keep the existing
isActiveRef.current guard and existing state updates (setStep, setSelectedIndex,
setErrorMessage) but change the setStatusMessage call to append the URL (use the
created session object or variable like checkoutUrl, checkoutSession.url, or
similar from the surrounding code) and format it clearly (e.g., "Checkout
created: <URL> — open manually"). Ensure the code still handles missing URL
gracefully by falling back to the current message.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 317da08e-773e-483a-8ddc-0250eea311fa
📒 Files selected for processing (2)
packages/tui/src/components/overlays/InsufficientCreditsOverlay.tsxpackages/tui/src/components/overlays/custom-model-wizard/provider-config.ts
✅ Files skipped from review due to trivial changes (1)
- packages/tui/src/components/overlays/custom-model-wizard/provider-config.ts
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/tui/src/components/overlays/InsufficientCreditsOverlay.tsx (1)
25-49: Consider using distinct IDs for top-up options.All three top-up amounts share
id: 'top-up', relying onamountUsdfor differentiation. While this works, distinct IDs (e.g.,'top-up-25','top-up-10','top-up-50') would make the intent clearer and simplify future maintenance or analytics tracking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tui/src/components/overlays/InsufficientCreditsOverlay.tsx` around lines 25 - 49, TOP_UP_OPTIONS currently reuses the same id ('top-up') for multiple ActionOption entries which can cause ambiguity; update the TOP_UP_OPTIONS array so each option has a unique id (e.g., 'top-up-25', 'top-up-10', 'top-up-50') while keeping label/amountUsd/description the same, and ensure any consumer of ActionOption that relies on id (selection handlers, analytics, keys) is updated to reference the new distinct ids.packages/tui/src/containers/OverlayContainer.tsx (1)
916-946: Note: ThenextBalanceUsdnumber branch appears unused.
handleInsufficientCreditsResolvedhandles bothnulland numeric balance values, butInsufficientCreditsOverlaycurrently only callsonResolved(null)(line 239). The numeric branch at lines 940-941 is dead code unless balance refresh is re-added later.This is fine if intentional future-proofing, but could be simplified to only handle
nullif that feature isn't planned.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tui/src/containers/OverlayContainer.tsx` around lines 916 - 946, The numeric branch in handleInsufficientCreditsResolved (nextBalanceUsd !== null) is never used because InsufficientCreditsOverlay only calls onResolved(null); remove the unused numeric branch or align the overlay to pass a refreshed balance. Specifically, either simplify handleInsufficientCreditsResolved to always handle null-only messages (remove the nextBalanceUsd parameter and formatting) or update InsufficientCreditsOverlay to call onResolved(nextBalanceUsd) when a balance refresh occurs; refer to handleInsufficientCreditsResolved, InsufficientCreditsOverlay, onResolved, and nextBalanceUsd to locate and apply the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/tui/src/components/overlays/InsufficientCreditsOverlay.tsx`:
- Around line 25-49: TOP_UP_OPTIONS currently reuses the same id ('top-up') for
multiple ActionOption entries which can cause ambiguity; update the
TOP_UP_OPTIONS array so each option has a unique id (e.g., 'top-up-25',
'top-up-10', 'top-up-50') while keeping label/amountUsd/description the same,
and ensure any consumer of ActionOption that relies on id (selection handlers,
analytics, keys) is updated to reference the new distinct ids.
In `@packages/tui/src/containers/OverlayContainer.tsx`:
- Around line 916-946: The numeric branch in handleInsufficientCreditsResolved
(nextBalanceUsd !== null) is never used because InsufficientCreditsOverlay only
calls onResolved(null); remove the unused numeric branch or align the overlay to
pass a refreshed balance. Specifically, either simplify
handleInsufficientCreditsResolved to always handle null-only messages (remove
the nextBalanceUsd parameter and formatting) or update
InsufficientCreditsOverlay to call onResolved(nextBalanceUsd) when a balance
refresh occurs; refer to handleInsufficientCreditsResolved,
InsufficientCreditsOverlay, onResolved, and nextBalanceUsd to locate and apply
the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0b57757f-5993-41e4-a013-374555166b8a
📒 Files selected for processing (9)
packages/cli/src/cli/auth/billing.test.tspackages/cli/src/cli/auth/billing.tspackages/cli/src/cli/auth/constants.test.tspackages/cli/src/cli/auth/constants.tspackages/cli/src/cli/auth/index.tspackages/cli/src/cli/modes/cli.tspackages/tui/src/components/overlays/InsufficientCreditsOverlay.tsxpackages/tui/src/containers/OverlayContainer.tsxpackages/tui/src/host/index.ts
✅ Files skipped from review due to trivial changes (2)
- packages/cli/src/cli/auth/constants.test.ts
- packages/cli/src/cli/auth/billing.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/cli/src/cli/modes/cli.ts
- packages/cli/src/cli/auth/index.ts
- packages/cli/src/cli/auth/billing.ts
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
What changed
maininto this branch and kept the upstream stream/usage changes greenWhy it changed
When a Dexto Nova user ran out of balance, the TUI still surfaced a raw error and made top-up recovery manual.
User impact
Root cause
The recovery UI was only keyed off the structured insufficient-credits error code. Real runs could still surface a plain provider-style error message like
Insufficient credits. Balance: $-0.04. Please top up ..., which bypassed the overlay and fell back to the old raw transcript error.Validation
scripts/quality-checks.shSummary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests