Integrate billing to new desktop bundle #1668
Conversation
📝 WalkthroughWalkthroughIntroduces a desktop BillingProvider with pro-plan gating for AI providers, refactors Stripe webhook secret handling and related env keys, adds a web checkout route and input validation, updates many desktop AI/STT/LLM components to respect billing, and applies multiple import/order and env adjustments across web and desktop apps. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DesktopApp
participant BillingProvider
participant ReactQuery
participant Supabase
participant SettingsUI
User->>DesktopApp: Open Settings
DesktopApp->>BillingProvider: Mounts (context)
BillingProvider->>ReactQuery: fetch billings (user_id)
ReactQuery->>Supabase: query billings
Supabase-->>ReactQuery: billing data
ReactQuery-->>BillingProvider: returns data
BillingProvider-->>SettingsUI: provide isPro / billing state
SettingsUI->>SettingsUI: compute locked = provider.requiresPro && !isPro
alt locked
SettingsUI->>User: show PlanLockMessage, disable controls
else
SettingsUI->>User: allow provider selection
end
sequenceDiagram
participant User
participant WebApp
participant CheckoutRoute
participant BillingFunction
participant Stripe
User->>WebApp: Click "Upgrade to Pro"
WebApp->>CheckoutRoute: navigate /app/checkout?period=monthly
CheckoutRoute->>BillingFunction: createCheckoutSession({period})
BillingFunction->>Stripe: create checkout session with priceId
Stripe-->>BillingFunction: session URL
BillingFunction-->>CheckoutRoute: return URL
CheckoutRoute->>User: redirect to Stripe checkout
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas to focus on:
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/web/src/routes/_view/callback/auth.tsx (2)
70-84: Remove redundant redirect from useEffect.Line 72 throws a redirect inside
useEffect, which violates React's component lifecycle patterns. This can cause hydration issues and unexpected behavior. Additionally, this redirect is redundant because the web flow is already handled by thebeforeLoadhook at lines 18-27, which redirects to/appwhensearch.codeexists. ThebeforeLoadhook is the appropriate place for redirects in TanStack Router.Apply this diff to remove the redundant redirect:
useEffect(() => { - if (search.flow === "web") { - throw redirect({ to: "/app" }); - } - if ( search.flow === "desktop" && search.access_token && search.refresh_token ) { setTimeout(() => { handleDeeplink(); }, 2000); } }, [search]);
10-11: Replace token-in-URL pattern with secure OAuth flow for desktop integration.Passing access and refresh tokens as URL search parameters violates RFC 8252 (OAuth 2.0 for Native Apps). URLs are logged in browser history, sent in Referer headers, and exposed to other apps, creating information exposure vulnerabilities.
Current code (lines 40-41, 62-63) passes actual tokens via URL redirect, then extracts them for the deeplink. Instead:
- Use Authorization Code flow with PKCE; exchange the authorization code locally in the native app for tokens
- Pass only a short-lived authorization code or session identifier in the URL
- Let the native app securely exchange it for tokens using a backend endpoint
- Store tokens in platform-protected storage (OS keychain/credential vault), not URLs
This eliminates token exposure in browser history, server logs, and Referer headers.
apps/web/src/routes/webhook/stripe.ts (1)
4-78: Webhook error handler should not return 200 on verification/processing failureThe import adjustments are fine, but the POST handler currently does:
} catch (error) { console.error("[STRIPE WEBHOOK] Error processing event", error); return new Response(JSON.stringify({ received: true }), { status: 200, }); }This acknowledges the event to Stripe even when signature verification or processing fails, which can silently drop billing events (e.g., bad
STRIPE_WEBHOOK_SECRET, transient failures inprocessEvent).Consider returning a non-2xx status and an error payload instead, so Stripe can surface and/or retry failures:
- } catch (error) { - console.error("[STRIPE WEBHOOK] Error processing event", error); - return new Response(JSON.stringify({ received: true }), { - status: 200, - }); - } + } catch (error) { + console.error("[STRIPE WEBHOOK] Error processing event", error); + return new Response(JSON.stringify({ error: "Webhook error" }), { + status: 400, // or 500 if you prefer to signal server-side failure + }); + }This ensures misconfigurations or runtime issues don’t get silently acknowledged as successful deliveries.
apps/desktop/src/components/settings/ai/llm/health.tsx (1)
96-131: Add billing to useMemo dependency array.The
useMemoat line 96 usesbilling.isProandbilling.isLoading(lines 111-116) but doesn't includebillingin the dependency array at line 129. This could cause stale availability checks when billing state changes.Apply this diff:
- }, [current_llm_provider, current_llm_model, configuredProviders]); + }, [current_llm_provider, current_llm_model, configuredProviders, billing]);
🧹 Nitpick comments (6)
apps/desktop/src/components/settings/ai/llm/configure.tsx (1)
173-200: Consider reusing shared pro‑gating helper for HyprnoteHere you hard‑code
locked = providerId === "hyprnote" && !billing.isPro, while elsewhere the PR introducesllmProviderRequiresPro. For consistency and easier future changes, consider derivinglockedfrom that helper instead of special‑casingproviderId === "hyprnote".apps/desktop/src/components/settings/ai/llm/shared.tsx (1)
11-19: Provider typing and pro‑gating helper look solid; consider tighteningrequiresProThe new
Providertype andllmProviderRequiresProhelper are well‑structured and make gating explicit. Since every entry inPROVIDERScurrently setsrequiresPro, you could simplify the type torequiresPro: boolean(non‑optional) to avoid having to coalesce tofalseand to catch missing flags at compile time.Also applies to: 23-87, 89-91
apps/desktop/src/components/settings/ai/stt/configure.tsx (1)
65-68: Locked STT providers use the same disabled pattern—lock message may not be visibleAs with the LLM configure view,
AccordionItemis disabled whenlockedand thePlanLockMessageis insideAccordionContent. Depending on the accordion implementation, users may not be able to open the item and therefore never see why it’s blocked.Consider:
- Leaving the item expandable while preventing edits, or
- Moving
PlanLockMessageoutside the collapsible area whenlockedis true.This would make Pro gating clearer for STT providers as well.
Also applies to: 95-105, 113-161
apps/desktop/src/components/settings/ai/stt/shared.tsx (1)
13-22: STT provider metadata and pro‑gating helper are consistent; requiresPro can likely be non‑optionalThe new
Providershape andsttProviderRequiresProhelper mirror the LLM side well and make it easy to gate STT providers by plan.As all entries in
PROVIDERSdefinerequiresPro, you could simplify the type torequiresPro: boolean(non‑optional) to ensure every future provider explicitly opts in/out of Pro gating and to avoid the need for?? false.Also applies to: 62-130, 132-134
apps/desktop/src/components/settings/ai/llm/select.tsx (2)
106-133: Provider & model gating logic is coherent; consider deduplicatinglockedcomputation and using shared messagingThe
lockedchecks for Pro-only access on both the provider list and the model combobox are consistent and correctly gate selection based onprovider.requiresPro && !billing.isPro. However, the same computation and upgrade messaging are now implemented in two places (provider select and model select), which increases the chance of divergence later.You could:
- Centralize the “is this provider locked for this billing state?” logic in a small helper (or reuse any existing helper such as an
llmProviderRequiresPro-style utility), and- Reuse a shared “plan lock” UI component/message (if available elsewhere in the settings UI) so copy and styling stay aligned across all gated surfaces.
This would keep the Pro gating behavior DRY and easier to evolve if the rules or wording change.
Also applies to: 147-175
193-257: Avoid recomputing mapping on every billing context change; depend only onbilling.isPro
useConfiguredMappingcurrently pulls in the entirebillingcontext object as a dependency to theuseMemo, even though the mapping only depends onbilling.isPro. This means any change to loading/fetching flags will cause the mapping to recompute and all consumers to re-render, even when Pro status hasn’t changed.You can tighten the dependency to the scalar you actually use:
- const billing = useBillingAccess(); + const { isPro } = useBillingAccess(); … - if (provider.requiresPro && !billing.isPro) { + if (provider.requiresPro && !isPro) { return [provider.id, null]; } … - }, [configuredProviders, auth, billing]); + }, [configuredProviders, auth, isPro]);This keeps behavior identical while reducing unnecessary recomputation and rerenders.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/web/src/routeTree.gen.tsis excluded by!**/*.gen.ts
📒 Files selected for processing (53)
.prettierrc(1 hunks)Taskfile.yaml(2 hunks)apps/api/.env.sample(1 hunks)apps/api/package.json(1 hunks)apps/api/src/env.ts(1 hunks)apps/api/src/stripe.ts(1 hunks)apps/desktop/package.json(1 hunks)apps/desktop/src/billing.tsx(1 hunks)apps/desktop/src/components/settings/account.tsx(2 hunks)apps/desktop/src/components/settings/ai/index.tsx(0 hunks)apps/desktop/src/components/settings/ai/llm/configure.tsx(5 hunks)apps/desktop/src/components/settings/ai/llm/health.tsx(3 hunks)apps/desktop/src/components/settings/ai/llm/select.tsx(8 hunks)apps/desktop/src/components/settings/ai/llm/shared.tsx(8 hunks)apps/desktop/src/components/settings/ai/shared/index.tsx(1 hunks)apps/desktop/src/components/settings/ai/stt/configure.tsx(7 hunks)apps/desktop/src/components/settings/ai/stt/health.tsx(3 hunks)apps/desktop/src/components/settings/ai/stt/select.tsx(5 hunks)apps/desktop/src/components/settings/ai/stt/shared.tsx(7 hunks)apps/desktop/src/env.ts(1 hunks)apps/desktop/src/hooks/useBilling.ts(0 hunks)apps/desktop/src/routes/__root.tsx(2 hunks)apps/web/.env.sample(0 hunks)apps/web/package.json(1 hunks)apps/web/src/env.ts(0 hunks)apps/web/src/functions/auth.ts(1 hunks)apps/web/src/functions/billing.ts(2 hunks)apps/web/src/functions/nango.ts(1 hunks)apps/web/src/functions/stripe.ts(1 hunks)apps/web/src/functions/supabase.ts(1 hunks)apps/web/src/middleware/drizzle.ts(1 hunks)apps/web/src/middleware/nango.ts(1 hunks)apps/web/src/middleware/supabase.ts(1 hunks)apps/web/src/routes/__root.tsx(1 hunks)apps/web/src/routes/_view/app/account.tsx(3 hunks)apps/web/src/routes/_view/app/checkout.tsx(1 hunks)apps/web/src/routes/_view/blog/$slug.tsx(1 hunks)apps/web/src/routes/_view/callback/auth.tsx(1 hunks)apps/web/src/routes/_view/docs/-components.tsx(1 hunks)apps/web/src/routes/_view/download/index.tsx(1 hunks)apps/web/src/routes/_view/enterprise.tsx(1 hunks)apps/web/src/routes/_view/index.tsx(1 hunks)apps/web/src/routes/_view/pricing.tsx(1 hunks)apps/web/src/routes/_view/product/ai-assistant.tsx(1 hunks)apps/web/src/routes/_view/product/ai-notetaking.tsx(1 hunks)apps/web/src/routes/_view/product/api.tsx(1 hunks)apps/web/src/routes/_view/product/mini-apps.tsx(1 hunks)apps/web/src/routes/_view/product/notepad.tsx(1 hunks)apps/web/src/routes/_view/product/workflows.tsx(1 hunks)apps/web/src/routes/_view/route.tsx(1 hunks)apps/web/src/routes/_view/templates.tsx(1 hunks)apps/web/src/routes/auth.tsx(1 hunks)apps/web/src/routes/webhook/stripe.ts(1 hunks)
💤 Files with no reviewable changes (4)
- apps/web/src/env.ts
- apps/desktop/src/hooks/useBilling.ts
- apps/desktop/src/components/settings/ai/index.tsx
- apps/web/.env.sample
🧰 Additional context used
🧬 Code graph analysis (14)
apps/web/src/routes/_view/app/checkout.tsx (2)
apps/web/src/routes/_view/app/account.tsx (1)
Route(11-14)apps/web/src/functions/billing.ts (1)
createCheckoutSession(12-66)
apps/desktop/src/components/settings/ai/stt/health.tsx (2)
apps/desktop/src/billing.tsx (1)
useBillingAccess(108-116)apps/desktop/src/components/settings/ai/stt/shared.tsx (1)
sttProviderRequiresPro(132-134)
apps/desktop/src/components/settings/ai/llm/select.tsx (2)
apps/desktop/src/billing.tsx (1)
useBillingAccess(108-116)apps/desktop/src/components/settings/ai/llm/shared.tsx (1)
PROVIDERS(23-87)
apps/desktop/src/components/settings/ai/llm/configure.tsx (2)
apps/desktop/src/billing.tsx (1)
useBillingAccess(108-116)apps/desktop/src/components/settings/ai/shared/index.tsx (3)
useProvider(57-69)PlanLockMessage(124-130)FormField(71-122)
apps/desktop/src/components/settings/ai/llm/shared.tsx (1)
apps/desktop/src/components/settings/ai/stt/shared.tsx (2)
ProviderId(24-24)PROVIDERS(62-130)
apps/web/src/functions/billing.ts (2)
crates/aec/src/lib.rs (1)
data(51-53)apps/api/src/stripe.ts (1)
stripe(6-8)
apps/desktop/src/billing.tsx (1)
apps/desktop/src/auth.tsx (1)
useAuth(184-192)
apps/desktop/src/components/settings/ai/stt/select.tsx (1)
apps/desktop/src/billing.tsx (1)
useBillingAccess(108-116)
apps/desktop/src/components/settings/account.tsx (3)
apps/desktop/src/env.ts (1)
env(4-14)apps/desktop/src/auth.tsx (1)
useAuth(184-192)apps/desktop/src/billing.tsx (2)
useBillingAccess(108-116)BillingAccess(31-31)
apps/desktop/src/components/settings/ai/llm/health.tsx (2)
apps/desktop/src/billing.tsx (1)
useBillingAccess(108-116)apps/desktop/src/components/settings/ai/llm/shared.tsx (1)
llmProviderRequiresPro(89-91)
apps/desktop/src/routes/__root.tsx (1)
apps/desktop/src/billing.tsx (1)
BillingProvider(35-106)
apps/desktop/src/components/settings/ai/stt/configure.tsx (2)
apps/desktop/src/billing.tsx (1)
useBillingAccess(108-116)apps/desktop/src/components/settings/ai/shared/index.tsx (3)
useProvider(57-69)PlanLockMessage(124-130)FormField(71-122)
apps/desktop/src/components/settings/ai/stt/shared.tsx (1)
apps/desktop/src/components/settings/ai/llm/shared.tsx (2)
ProviderId(21-21)PROVIDERS(23-87)
apps/api/src/stripe.ts (1)
apps/api/src/env.ts (1)
env(4-20)
🪛 dotenv-linter (4.0.0)
apps/api/.env.sample
[warning] 2-2: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci (macos, macos-14)
- GitHub Check: fmt
🔇 Additional comments (43)
apps/web/src/routes/_view/templates.tsx (1)
8-10: Import reorganization aligns with Prettier config updates.The DownloadButton and SlashSeparator imports follow the new @/ alias grouping pattern, properly separated from external packages. Both components are used in the component (lines 141, 176, and 189), so no unused imports. The change is well-organized and consistent with the PR's import cleanup work across web routes.
apps/web/src/routes/_view/download/index.tsx (1)
6-7: LGTM! Import ordering standardization.The import reordering aligns with the project's formatting standards and has no functional impact.
apps/web/src/routes/_view/blog/$slug.tsx (1)
9-10: LGTM! Import ordering standardization.The import reordering aligns with the project's formatting standards and has no functional impact.
apps/web/src/functions/nango.ts (1)
4-4: LGTM! Import ordering standardization.The import reordering aligns with the project's formatting standards and has no functional impact.
apps/desktop/package.json (1)
10-13: LGTM! Improved developer experience.Adding the
--silentflag toenv-cmdreduces log noise during development and build processes.apps/web/src/routes/_view/pricing.tsx (1)
6-7: LGTM! Import ordering standardization.The import reordering aligns with the project's formatting standards and has no functional impact.
apps/web/src/routes/_view/enterprise.tsx (1)
6-6: LGTM! Import ordering standardization.The import reordering aligns with the project's formatting standards and has no functional impact.
apps/web/src/middleware/nango.ts (1)
4-4: LGTM! Import ordering standardization.The import reordering aligns with the project's formatting standards and has no functional impact.
apps/api/.env.sample (1)
2-2: Environment variable rename verified as complete and consistent.Verification confirms the rename from
STRIPE_WEBHOOK_SIGNING_SECRETtoSTRIPE_WEBHOOK_SECREThas been applied uniformly throughout the codebase:
- No remnants of the old variable name remain
- New variable is properly defined in both web and API environment schemas (env.ts)
- Actual usage correctly updated in Stripe integration files
- No deployment, CI/CD, or Docker configuration files reference the old variable
- Environment generation (Taskfile.yaml) uses the new variable name
All concerns raised in the review have been addressed.
apps/api/src/stripe.ts (2)
34-34: Good addition for debugging.Adding error logging improves troubleshooting webhook verification failures. This complements the existing error message extraction on line 35.
26-26: Code change is correct and consistent.Verification confirms the rename from
STRIPE_WEBHOOK_SIGNING_SECRETtoSTRIPE_WEBHOOK_SECREThas been applied consistently across all files. No stale references to the old variable name remain, and all usages (environment schemas in both apps, webhook handlers, and task configuration) properly reference the new variable name.apps/api/package.json (1)
6-6: LGTM! Reduced verbosity during development.The
--silentflag suppresses env-cmd output, reducing console noise. This aligns with similar changes across other packages in the PR..prettierrc (1)
9-9: LGTM! Import order configuration aligned with codebase aliases.The new
@/import path pattern groups these imports logically between@hyprpackages and relative imports, supporting the import standardization evident throughout this PR.apps/web/src/routes/_view/docs/-components.tsx (1)
18-19: LGTM! Import reordering aligns with Prettier configuration.The
CtaCardimport has been repositioned to follow the new@/import grouping pattern. No functional changes.apps/web/package.json (1)
6-6: LGTM! Consistent silent flag usage across packages.Adding
--silenttoenv-cmdreduces development console noise. This change is consistent with the same pattern applied to other packages in this PR.apps/web/src/routes/_view/index.tsx (1)
1-9: LGTM! Import reorganization consistent with Prettier configuration.Imports have been reordered to align with the new
@/import grouping rules. No functional changes.apps/desktop/src/env.ts (1)
7-7: LGTM! Default URL improves local development experience.Changing from
.optional()to.default("http://localhost:3000")provides a sensible fallback for local development and eliminates the possibility of undefined values.Ensure that production and staging environments explicitly set
VITE_APP_URLto their respective domains, as the localhost default should only be used during local development.apps/web/src/functions/billing.ts (2)
8-10: LGTM! Input validation ensures type safety.The zod schema properly constrains the period to only valid values ("monthly" or "yearly"), preventing invalid inputs from reaching the handler.
47-50: Environment variables are properly defined and validated.Verification confirms both
STRIPE_MONTHLY_PRICE_IDandSTRIPE_YEARLY_PRICE_IDare defined in the environment schema (apps/web/src/env.ts, lines 14-15) with proper validation requiring non-empty strings. The billing.ts implementation correctly uses these variables.Taskfile.yaml (2)
247-254: LGTM! Simplified Stripe webhook secret handling.The new approach directly retrieves the secret using
stripe listen --print-secretand includes proper validation to ensure the secret is non-empty before writing. This is cleaner than the previous FIFO-based extraction.
241-241: LGTM! Streamlined cleanup and webhook listener invocation.The cleanup now only removes the generated env file (no FIFO cleanup needed), and the webhook listener starts directly after the secret is written. This simplifies the flow and reduces complexity.
Also applies to: 309-311
apps/api/src/env.ts (1)
14-14: All environment file references have been successfully updated.The verification confirms the Stripe webhook secret rename is complete with no issues:
- Zero remaining references to
STRIPE_WEBHOOK_SIGNING_SECRET- All new
STRIPE_WEBHOOK_SECRETreferences correctly placed in.env.samplefiles, environment schemas (apps/api/src/env.ts,apps/web/src/env.ts), and webhook implementations (apps/api/src/stripe.ts,apps/web/src/routes/webhook/stripe.ts)- Both
apps/apiandapps/webconsistently updatedapps/web/src/routes/_view/product/ai-notetaking.tsx (1)
7-8: MockWindow & SlashSeparator imports match usageBoth components are used later in the file and the
@/components/...paths look consistent with the rest of the codebase. No behavioral changes introduced here.apps/web/src/routes/_view/product/workflows.tsx (1)
6-6: SharedImageimport is consistent with usageThe new
Imageimport aligns with how this component renders icons throughout the file and keeps image behavior consistent. No issues from this change.apps/web/src/routes/_view/product/ai-assistant.tsx (1)
6-6: SlashSeparator import aligns with component usage
SlashSeparatoris used multiple times in this route; importing it from@/components/slash-separatoris correct and purely structural.apps/web/src/functions/stripe.ts (1)
4-9: Server-only Stripe client factory looks correctUsing
createServerOnlyFnaround the Stripe client instantiation keepsenv.STRIPE_SECRET_KEYserver-only while providing a simplegetStripeClient()helper for server code. The explicitapiVersionis also good for stability.apps/web/src/routes/_view/product/notepad.tsx (1)
6-6: SlashSeparator import is consistent and necessary
SlashSeparatoris used multiple times in this component; importing it from@/components/slash-separatoris correct and keeps imports consistent with other routes.apps/web/src/routes/_view/product/api.tsx (1)
5-5: MockWindow import matches the hero preview usageThe
MockWindowimport from@/components/mock-windowcorrectly backs the terminal-style preview in the hero. No behavioral changes beyond wiring the existing component.apps/web/src/routes/__root.tsx (1)
9-12: Alias imports for core root wiring look goodImporting
NotFoundDocument,fetchUser, andappCssvia the@/aliases matches how they’re used in the route config (beforeLoad,notFoundComponent, andlinks). No behavioral change from these import adjustments.apps/web/src/routes/_view/product/mini-apps.tsx (1)
6-7: LGTM! Clean addition of visual separator.The SlashSeparator import and its usage throughout the component provide consistent section separation in the mini-apps layout.
apps/desktop/src/components/settings/ai/shared/index.tsx (1)
124-130: LGTM! Shared component for Pro-gated features.The PlanLockMessage component provides a consistent UI pattern for displaying upgrade prompts across AI provider settings. The amber color scheme appropriately conveys a locked/gated state.
apps/desktop/src/routes/__root.tsx (1)
29-34: LGTM! Proper provider nesting order.The BillingProvider is correctly positioned between AuthProvider and the application content, ensuring billing context depends on authentication state and is available to all child components including devtools.
apps/desktop/src/components/settings/ai/llm/health.tsx (1)
111-118: LGTM! Proper Pro-gating for LLM providers.The billing-based access control correctly blocks providers that require Pro, with a loading-aware message that provides good user feedback.
apps/desktop/src/components/settings/ai/stt/health.tsx (1)
92-99: LGTM! Consistent Pro-gating pattern for STT providers.The billing-based access control follows the same pattern as LLM providers, with appropriate loading state handling and clear upgrade messaging.
apps/web/src/routes/_view/app/account.tsx (2)
73-80: LGTM! Cleaner checkout flow for free users.The migration from mutation-based upgrades to a dedicated checkout route simplifies the account page logic and centralizes billing flow.
98-104: Trial functionality remains active—review comment misidentified the change scope.The lines 98-104 modify only the "free" plan button, not trial functionality. The component maintains distinct branches for four plan states: "free", "trial", "trial_over", and "pro". Trial users continue to see "Manage Billing" (lines 83–92), while only free-tier users see "Upgrade to Pro" linking to checkout. Stripe webhooks still handle trial events ("customer.subscription.trial_will_end"), and the FAQ continues mentioning the 14-day free trial. The button change reflects a free-to-paid upgrade path, not trial removal.
Likely an incorrect or invalid review comment.
apps/desktop/src/components/settings/ai/stt/select.tsx (2)
102-132: LGTM! Clear Pro-gating UI with excellent user feedback.The provider selection properly gates Pro-required providers with:
- Visual Pro badge for identification
- Disabled state when locked
- Clear "Upgrade to Pro" messaging
The implementation provides excellent UX for gated features.
230-232: LGTM! Correct data-level gating for Pro providers.The useConfiguredMapping hook properly filters out Pro-required providers when the user doesn't have Pro access, ensuring the data layer enforces the same constraints as the UI.
apps/desktop/src/components/settings/ai/llm/configure.tsx (1)
50-52: Locked providers are disabled but the lock message may never be visible
AccordionItemis markeddisabled={locked}, and the lock message (PlanLockMessage) lives insideAccordionContent. If the accordion implementation prevents opening disabled items, users will only see a dimmed, non‑clickable card without any explanation.You may want to:
- Keep the trigger enabled but block configuration actions, or
- Render the
PlanLockMessageoutside the collapsible content so it’s always visible whenlockedis true.This will make the Pro gating clearer without relying on accordion behavior.
Also applies to: 90-101, 107-158
apps/desktop/src/components/settings/ai/stt/configure.tsx (1)
242-261: HyprProviderCloudRow text gating is clear and non‑breakingUsing
billing.isProsolely to switch between “For Pro Users” and “Coming soon” while keeping the button disabled is a safe way to surface future Pro‑only functionality without introducing new flows yet. This looks good and should be easy to extend once the feature is live.apps/desktop/src/components/settings/account.tsx (2)
13-53: Auth and billing wiring for SettingsAccount looks correctUsing
useAuthanduseBillingAccessto gate the whole view, with a clear unauthenticated sign‑in card and authenticated account/billing containers, is a clean refactor. The URL construction viaWEB_APP_BASE_URLand the callbacks for account/checkout navigation are straightforward and easy to reason about.
259-277: PLANS andformatSubscriptionStatusutilities are simple and readableThe
PLANSmap andformatSubscriptionStatushelper nicely encapsulate display details away from the main component. This should make future plan tweaks or status formatting changes easy to implement without touching the billing logic.apps/desktop/src/billing.tsx (1)
118-136:computeIsPrologic matches typical Stripe subscription modelingThe
computeIsProhelper correctly checks both subscription status ("active"or"trialing") and whether any subscription item’s product matchesenv.VITE_PRO_PRODUCT_ID, handling both string IDs and expanded product objects. This is a solid, explicit definition of “Pro” and looks good as a central place to evolve the rules if needed.
| export function BillingProvider({ children }: { children: ReactNode }) { | ||
| const auth = useAuth(); | ||
|
|
||
| const { | ||
| data: queryData, | ||
| isLoading, | ||
| isPending, | ||
| isFetching, | ||
| isRefetching, | ||
| isError, | ||
| error, | ||
| refetch, | ||
| } = useQuery({ | ||
| enabled: !!auth?.supabase && !!auth?.session?.user?.id, | ||
| queryKey: ["billing", auth?.session?.user?.id], | ||
| queryFn: async (): Promise<BillingData> => { | ||
| if (!auth?.supabase || !auth?.session?.user?.id) { | ||
| return null; | ||
| } | ||
|
|
||
| const { data, error } = await auth.supabase | ||
| .from("billings") | ||
| .select("*") | ||
| .eq("user_id", auth.session.user.id) | ||
| .maybeSingle(); | ||
|
|
||
| if (error) { | ||
| throw error; | ||
| } | ||
|
|
||
| if (!data) { | ||
| return null; | ||
| } | ||
|
|
||
| const billing = data as BillingRow; | ||
| return { | ||
| ...billing, | ||
| isPro: computeIsPro(billing.stripe_subscription), | ||
| }; | ||
| }, | ||
| }); | ||
|
|
||
| const data = queryData ?? null; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
isPro can stay truthy after logout because cached query data is still used
Right now data is derived purely from queryData:
const {
data: queryData,
…
} = useQuery({ enabled: !!auth?.supabase && !!auth?.session?.user?.id, … });
const data = queryData ?? null;When the user logs out, enabled becomes false, but React Query keeps the last successful data in cache and continues to expose it. That means data (and therefore isPro) can still reflect the previous user’s subscription even when auth.session is no longer present, which can make Pro gating appear “stuck” in a Pro state post-logout.
A minimal fix is to explicitly null out billing when there’s no authenticated user:
- const data = queryData ?? null;
+ const hasUser = !!auth?.session?.user?.id;
+ const data = hasUser ? queryData ?? null : null;
…
- isPro: !!data?.isPro,
+ isPro: hasUser && !!data?.isPro,and add auth?.session?.user?.id (or hasUser) to the useMemo dependency list so the value recomputes when auth changes.
This keeps cached data useful while avoiding leaking subscription state across logouts.
Also applies to: 79-101
🌐 Web query:
In @tanstack/react-query v5, what happens to `data` returned by `useQuery` when `enabled` switches from `true` to `false`? Does it keep the last successful result, or does it reset to `undefined`?
💡 Result:
It keeps the last successful result (cached data). When enabled is false a query with cached data is initialized in success/isSuccess and returns that cached data; if there's no cache it will start idle/pending with data undefined. [1][2]
isPro and billing data persist after logout due to React Query caching
When enabled becomes false on logout, React Query keeps the last successful cached data. This means isPro continues to reflect the previous user's subscription state even after auth.session is cleared—a privacy and gating issue.
Fix by explicitly nulling data when unauthenticated:
- const data = queryData ?? null;
+ const hasUser = !!auth?.session?.user?.id;
+ const data = hasUser ? queryData ?? null : null;Also update the context value to gate isPro by auth state:
- isPro: !!data?.isPro,
+ isPro: hasUser && !!data?.isPro,And add auth?.session?.user?.id to the context useMemo dependency list to recompute when auth changes.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/desktop/src/billing.tsx around lines 35 to 78, React Query keeps the
last cached billing data when enabled flips to false on logout, so isPro and
billing persist after unauthentication; to fix, ensure the local data is
explicitly set to null when auth or auth.session?.user?.id is missing (e.g., set
const data = auth?.session?.user?.id ? queryData ?? null : null), update the
context value so isPro is only true when auth.session?.user exists (gate isPro
by auth state), and add auth?.session?.user?.id to the useMemo dependency list
so the context recomputes when the authenticated user changes.
| function SettingsBilling({ | ||
| billing, | ||
| onManage, | ||
| onUpgrade, | ||
| }: { | ||
| plan: BillingPlan; | ||
| currentPlan: PlanId; | ||
| onChangePlan: (plan: PlanId) => void; | ||
| onContactSales: () => void; | ||
| className?: string; | ||
| removeBorder?: boolean; | ||
| billing: BillingAccess; | ||
| onManage: () => void; | ||
| onUpgrade: () => void; | ||
| }) { | ||
| const isCurrent = plan.id === currentPlan; | ||
| if (billing.isPending && !billing.data) { | ||
| return ( | ||
| <div className="text-sm text-neutral-600">Loading billing details...</div> | ||
| ); | ||
| } | ||
|
|
||
| const billingData = billing.data; | ||
| const planId: PlanId = billingData?.isPro ? "pro" : "free"; | ||
| const plan = PLANS[planId]; | ||
| const hasStripeCustomer = !!billingData?.stripe_customer; | ||
| const subscriptionStatus = billingData?.stripe_subscription?.status; | ||
| const showErrorBanner = billing.isError; | ||
| const errorMessage = | ||
| billing.error instanceof Error | ||
| ? billing.error.message | ||
| : "Unable to load billing details."; | ||
|
|
||
| return ( | ||
| <div | ||
| className={cn([ | ||
| "h-full p-8 flex flex-col justify-center gap-6 bg-white", | ||
| !removeBorder && "border border-neutral-200 rounded-lg", | ||
| className, | ||
| ])} | ||
| > | ||
| <div className="flex flex-col gap-4"> | ||
| <div className="space-y-2"> | ||
| <h3 className="text-lg font-semibold">{plan.name}</h3> | ||
| <p className="text-sm text-neutral-600">{plan.description}</p> | ||
| <div className="space-y-4"> | ||
| {showErrorBanner && ( | ||
| <div className="rounded-md border border-red-200 bg-red-50 p-3 text-sm text-red-700 flex flex-col gap-2 sm:flex-row sm:items-center sm:justify-between"> | ||
| <span>{errorMessage}</span> | ||
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| onClick={() => billing.refetch()} | ||
| disabled={billing.isRefetching} | ||
| > | ||
| Retry | ||
| </Button> | ||
| </div> | ||
| )} | ||
|
|
||
| <ul className="space-y-3"> | ||
| {plan.features.map((feature) => ( | ||
| <li | ||
| key={feature} | ||
| className="flex items-start gap-2 text-sm text-neutral-700" | ||
| > | ||
| <Check size={16} className="mt-0.5 text-emerald-500 shrink-0" /> | ||
| <span>{feature}</span> | ||
| </li> | ||
| ))} | ||
| </ul> | ||
|
|
||
| <p className="text-xl font-semibold"> | ||
| {plan.price}{" "} | ||
| {plan.priceSuffix && ( | ||
| <span className="text-sm text-neutral-500 font-light"> | ||
| {plan.priceSuffix} | ||
| </span> | ||
| )} | ||
| </p> | ||
| <div className="flex flex-col gap-4 sm:flex-row sm:items-start sm:justify-between"> | ||
| <div className="flex-1"> | ||
| <p className="text-xs uppercase text-neutral-500">Active plan</p> | ||
| <p className="text-lg font-semibold text-neutral-900">{plan.name}</p> | ||
| <p className="text-sm text-neutral-600">{plan.description}</p> | ||
| {subscriptionStatus && planId === "pro" ? ( | ||
| <p className="text-xs text-neutral-500"> | ||
| Subscription status:{" "} | ||
| {formatSubscriptionStatus(subscriptionStatus)} | ||
| </p> | ||
| ) : null} | ||
| </div> | ||
|
|
||
| <PlanActions | ||
| planId={plan.id} | ||
| currentPlan={currentPlan} | ||
| onChangePlan={onChangePlan} | ||
| onContactSales={onContactSales} | ||
| isCurrent={isCurrent} | ||
| /> | ||
| <div className="sm:w-auto"> | ||
| <PlanActions | ||
| planId={planId} | ||
| hasStripeCustomer={hasStripeCustomer} | ||
| onManage={onManage} | ||
| onUpgrade={onUpgrade} | ||
| /> | ||
| </div> | ||
| </div> | ||
|
|
||
| <ul className="space-y-2"> | ||
| {plan.features.map((feature) => ( | ||
| <li | ||
| key={feature} | ||
| className="flex items-start gap-2 text-sm text-neutral-700" | ||
| > | ||
| <Check size={16} className="mt-0.5 text-emerald-500 shrink-0" /> | ||
| <span>{feature}</span> | ||
| </li> | ||
| ))} | ||
| </ul> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
SettingsBilling handles loading and error states well; check pro-without-stripe edge case
The billing component correctly covers:
- Initial loading (
isPending && !data), - Error banner with retry,
- Plan derivation from
billing.data?.isPro, - Stripe subscription status display for Pro.
One edge case: if billingData.isPro is true but stripe_customer is missing, planId will be "pro" but hasStripeCustomer will be false, causing PlanActions to render “Upgrade to Pro”. If that situation is possible in your backend, you may want a distinct “Contact support” or “Manage via web” path instead of suggesting an upgrade again.
🤖 Prompt for AI Agents
In apps/desktop/src/components/settings/account.tsx around lines 144-222, the
component can mark a user as "pro" (planId === "pro") while hasStripeCustomer is
false, which makes PlanActions incorrectly show an "Upgrade to Pro" path for
already-pro users; update the render logic so when billingData?.isPro is true
but billingData?.stripe_customer is falsy you pass a distinct prop or render a
different action (e.g., show "Manage via web" or "Contact support" and
disable/hide the Upgrade action) — specifically detect if billingData?.isPro &&
!billingData?.stripe_customer and either call PlanActions with a new flag like
isProWithoutStripe or bypass PlanActions to render a support/manage button that
invokes onManage or onUpgrade appropriately, and update PlanActions to handle
that new flag (or consume the adjusted props) so users who are pro but lack a
stripe customer aren’t prompted to upgrade again.
| const billing = useBillingAccess(); | ||
| const [provider, setProvider] = useProvider(config.id); | ||
| const locked = config.requiresPro && !billing.isPro; | ||
|
|
There was a problem hiding this comment.
Confirm whether any STT providers should actually be Pro‑only
locked is computed as config.requiresPro && !billing.isPro, but in stt/shared.tsx all providers currently have requiresPro: false. That means the new gating path is effectively inactive for STT providers today.
If any STT providers are meant to be Pro‑only, ensure their requiresPro flags (and any UI badges) are updated to match the business rules.
🤖 Prompt for AI Agents
In apps/desktop/src/components/settings/ai/stt/configure.tsx around lines 65–68,
the gating flag `locked` is computed but all STT providers currently have
`requiresPro: false` in stt/shared.tsx, so the gate is inactive; confirm which
providers should be Pro‑only, then update each provider's requiresPro boolean in
apps/desktop/src/components/settings/ai/stt/shared.tsx accordingly and add or
update any UI badges/labels in the STT provider list to reflect Pro‑only status
so the `locked` calculation and visual state match the business rules.
| export const Route = createFileRoute("/_view/app/checkout")({ | ||
| validateSearch, | ||
| beforeLoad: async ({ search }) => { | ||
| const { url } = await createCheckoutSession({ | ||
| data: { period: search.period }, | ||
| }); | ||
|
|
||
| if (url) { | ||
| throw redirect({ href: url }); | ||
| } | ||
|
|
||
| throw redirect({ to: "/app/account" }); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
Verify error handling for checkout session creation.
The route correctly validates search params and initiates checkout flow. However, there's no explicit error handling if createCheckoutSession fails. Errors will bubble to the error boundary, but consider whether users need a more graceful fallback.
Verify that the error boundary at the root route provides appropriate feedback for checkout failures:
🏁 Script executed:
#!/bin/bash
# Description: Check error boundary implementation in root route
# Look for ErrorComponent implementation
rg -A 10 "ErrorComponent" apps/web/src/routes/__root.tsxLength of output: 0
🏁 Script executed:
#!/bin/bash
# Check if __root.tsx exists and inspect its content structure
fd "__root.tsx" apps/web/src/routes --type f -x cat {}Length of output: 2195
🏁 Script executed:
#!/bin/bash
# Search for error component/boundary with broader patterns
rg -i "error" apps/web/src/routes/__root.tsx -A 5 -B 5Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search across all route files for ErrorComponent or error handling patterns
rg -i "ErrorComponent|errorComponent" apps/web/src/routes/ -lLength of output: 0
🏁 Script executed:
#!/bin/bash
# Search for errorComponent definitions anywhere in codebase
rg "errorComponent" apps/web/src --type tsx --type ts -B 2 -A 2Length of output: 27
🏁 Script executed:
#!/bin/bash
# Check parent route files for checkout (check _view/app route)
fd -e tsx -e ts "apps/web/src/routes/**" | grep -E "(\/_view|/app)" | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look for RouteConfig or any error handling patterns in checkout vicinity
cat apps/web/src/routes/_view/app/checkout.tsxLength of output: 582
🏁 Script executed:
#!/bin/bash
# Search for errorComponent with correct syntax (no type flag)
rg "errorComponent" apps/web/src -B 2 -A 2Length of output: 0
🏁 Script executed:
#!/bin/bash
# Find all route files to understand parent route structure
find apps/web/src/routes -type f \( -name "*.tsx" -o -name "*.ts" \) | sortLength of output: 2011
🏁 Script executed:
#!/bin/bash
# Check for _view route file
cat apps/web/src/routes/_view.tsx 2>/dev/null || cat apps/web/src/routes/_view/app.tsx 2>/dev/null || echo "Files not found"Length of output: 15
🏁 Script executed:
#!/bin/bash
# Check parent routes for error handling
cat apps/web/src/routes/_view/route.tsxLength of output: 24946
🏁 Script executed:
#!/bin/bash
# Check app parent route
cat apps/web/src/routes/_view/app/route.tsxLength of output: 235
Add error handling to checkout route and error boundary to root route.
Verification confirms there is no error boundary in the application. The root route at apps/web/src/routes/__root.tsx lacks an errorComponent property, and the checkout route at apps/web/src/routes/_view/app/checkout.tsx has no error handling for createCheckoutSession failures. When the function fails, the error bubbles unhandled with no UI feedback to users.
Required fixes:
- Add
try-catchin checkoutbeforeLoadto handlecreateCheckoutSessionerrors gracefully (e.g., redirect to/app/accountwith error message or default error page) - Add
errorComponentto root route to provide fallback UI for unhandled route errors
🤖 Prompt for AI Agents
In apps/web/src/routes/_view/app/checkout.tsx around lines 10 to 23 and in
apps/web/src/routes/__root.tsx (root route), add error handling and a root error
boundary: wrap the createCheckoutSession call in beforeLoad with a try/catch
that catches any error, logs it (optional), and performs a safe redirect to
"/app/account" (or to a dedicated error page) passing an error indicator (e.g.,
query param like ?error=checkout_failed) so the UI can show a message; then
update the root route definition in __root.tsx to include an errorComponent
property pointing to a simple fallback React component that renders a
user-friendly error UI (title, message, optional retry/home links) so unhandled
route errors render that component instead of crashing the app.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/desktop/src/components/settings/ai/llm/health.tsx (1)
96-122: Add billing dependencies to the availability memo.
useAvailabilityreadsbilling.isProandbilling.isLoading, but the memo only depends on provider/model. React will keep returning a stale result after billing state changes (e.g., upgrading to Pro still shows the upgrade gating until you change providers). Add the billing flags to the dependency array so the memo re-renders when access changes.- }, [current_llm_provider, current_llm_model, configuredProviders]); + }, [ + current_llm_provider, + current_llm_model, + configuredProviders, + billing.isPro, + billing.isLoading, + ]);apps/desktop/src/components/settings/ai/stt/health.tsx (1)
20-50: Cloud detection likely too broad; hyprnote non‑cloud models skip local health handling
isCloudis currentlycurrent_stt_provider === "hyprnote" || current_stt_model === "cloud". That means any hyprnote model (including localam-*/Quantized*) is treated as “cloud”, souseConnectionHealthreturns based only onconnand never inspects the local STT server status.In
apps/desktop/src/config/registry.ts, the STT side effects still start the local server whenprovider === "hyprnote"andmodelis not"cloud"and matches the local prefixes, which implies those combinations are still local-mode.To keep health in sync with the registry behavior,
isCloudshould probably only be true for the explicit"cloud"model (and optionally only when paired with the hyprnote provider), so local hyprnote models continue to use thelocalstatus path.A minimal fix:
- const isCloud = - current_stt_provider === "hyprnote" || current_stt_model === "cloud"; + const isCloud = + current_stt_provider === "hyprnote" && current_stt_model === "cloud";This keeps cloud-only handling for the new cloud mode while preserving the existing local health behavior for non‑cloud hyprnote models.
♻️ Duplicate comments (1)
apps/desktop/src/billing.tsx (1)
43-86: Cached billing data persists after logout—privacy & gating issueThe issue flagged in the previous review remains unaddressed. When the user logs out,
enabledbecomesfalse, but React Query retains the last successfulqueryDatain cache. Line 85 assignsdata = queryData ?? nullwithout gating on auth state, soisProcontinues to reflect the previous user's subscription even afterauth.sessionis cleared.Apply this diff to explicitly null out data when unauthenticated:
- const data = queryData ?? null; + const hasUser = !!auth?.session?.user?.id; + const data = hasUser ? queryData ?? null : null;Also update the context value (line 94) to gate
isPro:- isPro: !!data?.isPro, + isPro: hasUser && !!data?.isPro,And add
auth?.session?.user?.idto theuseMemodependency array (lines 104-114) so the context recomputes on auth changes.
🧹 Nitpick comments (6)
apps/desktop/src/components/settings/ai/stt/health.tsx (1)
62-103: Billing gating for STT availability looks correct; consider softening behavior while loadingThe
sttProviderRequiresPro+billing.isProgate is wired correctly and short‑circuits availability with clear messages, including a special"Checking plan access..."string whilebilling.isLoadingis true.One trade‑off: while loading, Pro‑required providers are effectively locked even for Pro users until
isProflips true. If that brief lock is undesirable, you could gate on something like!billing.isLoading && !billing.isProinstead, or expose a separateisReadyflag from billing.apps/desktop/src/components/settings/ai/llm/configure.tsx (1)
51-59: Locked providers usedisabledAccordionItems; verify content is still reachableBoth
NonHyprProviderCardandHyprProviderCardcompute alockedflag and passdisabled={locked}to<AccordionItem>, while the locked UI (PlanLockMessage / upgrade messaging) lives inside<AccordionContent>.Depending on how
@hypr/ui’s Accordion handlesdisabled, this may prevent users from expanding a locked item at all, meaning the PlanLockMessage and upgrade affordance are never seen. The trigger is visually dimmed, but there might be no way to discover why the provider is unavailable.A safer pattern is often to leave the item expandable and only gate the interactive form:
- Keep the
cursor-not-allowed opacity-30styling on the trigger to convey lock.- Remove
disabled={locked}fromAccordionItem.- Continue to render
PlanLockMessageinstead of the form (or in addition, for Hyprnote), as you already do.Also applies to: 91-162, 174-205
apps/desktop/src/components/settings/ai/llm/select.tsx (1)
188-257: useConfiguredMapping cleanly centralizes provider availability (auth + billing + config)The mapping function correctly short‑circuits to
nullin all cases where model listing should be disabled:
- Pro‑required providers when
!billing.isPro.- Hyprnote when there is no
auth?.session.- Any provider missing
base_urlor requiredapi_key.Otherwise, it builds an appropriate
listModelsFuncper provider.If you find the Pro‑lock condition creeping into more places, you could consider extracting a small helper (e.g.,
isProviderLocked(provider, billing)) to reuse between here and the UI components, but it’s not strictly necessary given current usage.apps/desktop/src/config/registry.ts (1)
90-124: Local STT start condition matches new “cloud” model semantics; consider de‑duplicating logicThe updated side effects for
current_stt_providerandcurrent_stt_model:
- Safely treat both values as
string | undefined.- Only call
localSttCommands.startServerwhen:
- provider is
"hyprnote",- model is set,
- model is not
"cloud", and- model starts with
"am-"or"Quantized".That aligns well with the new “cloud” model and prevents accidental
startServercalls for unsupported strings.The conditional block is duplicated in both sideEffect handlers, though. If you find these conditions evolving (e.g., more local models), consider extracting a small helper like
maybeStartLocalSttForConfig(provider, model)to keep them in sync and reduce maintenance overhead.apps/desktop/src/billing.tsx (2)
87-89: Remove unusedauthdependency from upgradeToPro callbackThe
authvariable is listed in the dependency array but is never used within theupgradeToProcallback body. This causes unnecessary re-creation of the callback wheneverauthchanges.Apply this diff:
const upgradeToPro = useCallback(() => { openUrl(`${env.VITE_APP_URL}/app/checkout?period=monthly`); - }, [auth]); + }, []);
132-150: Guard against undefinedVITE_PRO_PRODUCT_IDfor clearer failure modeThe function relies on
env.VITE_PRO_PRODUCT_IDto determine Pro status, but that environment variable is optional (perenv.ts). If it's undefined, the product comparison (lines 145-146) will always fail silently, causing all users to appear non-Pro.Adding an explicit guard makes the failure mode clear and prevents subtle bugs:
function computeIsPro( subscription: Stripe.Subscription | null | undefined, ): boolean { if (!subscription) { return false; } + + if (!env.VITE_PRO_PRODUCT_ID) { + console.warn('VITE_PRO_PRODUCT_ID is not configured; Pro status unavailable'); + return false; + } const hasValidStatus = ["active", "trialing"].includes(subscription.status);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
.github/workflows/desktop_cd.yaml(1 hunks)apps/api/src/billing.ts(1 hunks)apps/api/src/deepgram.ts(1 hunks)apps/desktop/src/billing.tsx(1 hunks)apps/desktop/src/components/settings/ai/llm/configure.tsx(5 hunks)apps/desktop/src/components/settings/ai/llm/health.tsx(3 hunks)apps/desktop/src/components/settings/ai/llm/select.tsx(9 hunks)apps/desktop/src/components/settings/ai/stt/configure.tsx(7 hunks)apps/desktop/src/components/settings/ai/stt/health.tsx(4 hunks)apps/desktop/src/components/settings/ai/stt/select.tsx(6 hunks)apps/desktop/src/components/settings/ai/stt/shared.tsx(8 hunks)apps/desktop/src/config/registry.ts(2 hunks)apps/desktop/src/env.ts(1 hunks)apps/desktop/src/hooks/useLLMConnection.ts(1 hunks)apps/desktop/src/hooks/useSTTConnection.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/components/settings/ai/stt/configure.tsx
- apps/desktop/src/components/settings/ai/stt/select.tsx
🧰 Additional context used
🧬 Code graph analysis (8)
apps/desktop/src/components/settings/ai/llm/select.tsx (2)
apps/desktop/src/billing.tsx (1)
useBillingAccess(122-130)apps/desktop/src/components/settings/ai/llm/shared.tsx (1)
PROVIDERS(23-87)
apps/desktop/src/components/settings/ai/llm/configure.tsx (3)
apps/desktop/src/billing.tsx (1)
useBillingAccess(122-130)apps/desktop/src/components/settings/ai/shared/index.tsx (4)
useProvider(57-69)PlanLockMessage(124-130)FormField(71-122)StyledStreamdown(40-55)packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/settings/ai/stt/shared.tsx (1)
apps/desktop/src/components/settings/ai/llm/shared.tsx (2)
ProviderId(21-21)PROVIDERS(23-87)
apps/desktop/src/components/settings/ai/stt/health.tsx (3)
apps/desktop/src/config/use-config.ts (1)
useConfigValues(42-72)apps/desktop/src/billing.tsx (1)
useBillingAccess(122-130)apps/desktop/src/components/settings/ai/stt/shared.tsx (1)
sttProviderRequiresPro(137-139)
apps/desktop/src/components/settings/ai/llm/health.tsx (2)
apps/desktop/src/billing.tsx (1)
useBillingAccess(122-130)apps/desktop/src/components/settings/ai/llm/shared.tsx (1)
llmProviderRequiresPro(89-91)
apps/desktop/src/hooks/useLLMConnection.ts (1)
apps/desktop/src/env.ts (1)
env(4-15)
apps/desktop/src/billing.tsx (2)
apps/desktop/src/auth.tsx (1)
useAuth(184-192)apps/desktop/src/env.ts (1)
env(4-15)
apps/desktop/src/hooks/useSTTConnection.ts (2)
apps/desktop/src/auth.tsx (1)
useAuth(184-192)apps/desktop/src/env.ts (1)
env(4-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: fmt
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (7)
apps/api/src/deepgram.ts (1)
367-369: Good fix: usingsetprevents duplicate parameters.The change from
appendtosetensures that duplicate query parameter keys are replaced rather than accumulated, which is the correct behavior for most API parameters.apps/desktop/src/components/settings/ai/llm/configure.tsx (1)
207-237: ProviderContext Pro‑aware messaging for Hyprnote reads cleanlyThe updated
ProviderContextcleanly adds billing‑aware messaging:
- Uses
useBillingAccessto pullisProandupgradeToPro.- For hyprnote when not Pro, wraps the standard
StyledStreamdowncontent with an inline “Upgrade to Pro” button.- Falls back to the previous content rendering for all other providers or when already Pro.
This keeps the contextual help strings centralized and provides a clear CTA without complicating the calling components.
apps/desktop/src/components/settings/ai/llm/select.tsx (1)
30-186: Provider/model selection Pro gating looks consistent and ergonomicWithin
SelectProviderAndModel:
- The provider field listener that sets model to
"Auto"for"hyprnote"and clears it otherwise keeps the pair in a valid state.- Provider
SelectItems are correctly disabled when either not configured (!configuredProviders[provider.id]) or Pro‑locked (provider.requiresPro && !billing.isPro), and show succinct upgrade messaging when locked.- The model field reuses the same lock condition and prevents listing or selecting models when the provider is unavailable, while adding a clear “Upgrade to Pro to pick … models.” hint.
Behavior matches the PROVIDERS metadata and the Pro expectations from
configure.tsx, and the UX should be predictable.apps/desktop/src/components/settings/ai/stt/shared.tsx (1)
13-22: STT Provider metadata and Pro‑gating helper are well‑structured; note gating is currently a no‑opThe new
Providertype,PROVIDERSdefinition,"cloud"model, andsttProviderRequiresProhelper are all wired coherently:
satisfies readonly Provider[]locks in the provider shape and improves type‑safety.displayModelIdhandling of"cloud"→"Cloud"keeps the UI readable.sttProviderRequiresProcleanly abstracts therequiresProflag with a safe default.With the current data, every STT provider has
requiresPro: false, so the billing gate instt/health.tsxwill never trigger yet. That’s fine if STT should remain free for now; just be aware that flipping any provider torequiresPro: truelater will immediately activate the gate in the health/availability flow.Also applies to: 38-41, 66-135, 137-139
apps/desktop/src/billing.tsx (3)
1-39: LGTM!The imports and type definitions are clean and well-structured. The type hierarchy (BillingRow → BillingData → BillingContextValue) clearly models the data flow from database through enrichment to context API.
122-130: LGTM!The
useBillingAccesshook follows the standard context consumer pattern and matches the style used inuseAuth. Clear error messaging ensures developers are alerted if the hook is used outside the provider.
132-150: No compatibility issues found—Stripe types are correctThe Stripe SDK version installed is ^19.3.0, and subscription.status values "active" and "trialing" are both valid in Stripe v19. The subscription structure being accessed (items.data, price.product) aligns with the standard Stripe subscription object. The code is compatible with the installed Stripe version.
| target.searchParams.append(key, value); | ||
| target.searchParams.set(key, value); | ||
| }); | ||
| target.searchParams.set("model", "nova-3-general"); |
There was a problem hiding this comment.
🧩 Analysis chain
Verify if hardcoded model restriction is intentional.
This change forces all Deepgram requests to use the "nova-3-general" model, overwriting any client-specified model parameter. Given the PR's billing integration focus, this may be intentional plan gating. However, consider:
- Should different billing plans allow different models?
- Should the model be configurable via environment variable or per-request logic?
- Consider extracting the magic string to a named constant (e.g.,
DEFAULT_DEEPGRAM_MODEL).
Run the following script to check if there's any client code that attempts to specify a model parameter:
🏁 Script executed:
#!/bin/bash
# Description: Search for code that sets Deepgram model parameters in client code
# Search for model parameter usage in desktop and web apps
rg -n --type=ts --type=tsx 'model.*nova|deepgram.*model|searchParams.*model' apps/desktop/ apps/web/ -A 2 -B 2Length of output: 27
🏁 Script executed:
#!/bin/bash
# Description: Search for Deepgram model parameter usage in client code (fixed)
# Search for model parameter usage in desktop and web apps - using valid rg type
rg -n --type=ts 'model.*nova|deepgram.*model|searchParams.*model' apps/desktop/ apps/web/ -A 2 -B 2Length of output: 1350
🏁 Script executed:
#!/bin/bash
# Description: Examine the deepgram.ts file context around line 370
# Show context around line 370 in deepgram.ts
sed -n '350,390p' apps/api/src/deepgram.tsLength of output: 1097
🏁 Script executed:
#!/bin/bash
# Description: Search for plan-based or billing logic related to model restrictions
# Search for plan/billing related logic in the codebase
rg -n --type=ts 'plan|billing|pro.*plan|subscription' apps/api/src/ -C 3 | head -100Length of output: 3296
Remove the hardcoded model override or implement proper plan-based gating logic.
The buildDeepgramUrl function at line 370 hardcodes nova-3-general for all requests, overwriting any model parameter sent by the client. This breaks existing client functionality:
- The desktop app explicitly supports model selection (nova-3-general, nova-2-general, etc.) with language variants
- The API copies incoming parameters but immediately overwrites the model unconditionally
- Billing logic exists but contains no plan-based restrictions to justify this behavior
Either:
- Remove the hardcoded model to respect client-specified choices
- Implement plan-based gating (check user subscription, allow different models per plan tier)
- Extract to a configurable constant with documented reasoning
🤖 Prompt for AI Agents
In apps/api/src/deepgram.ts around line 370, the code unconditionally sets
target.searchParams.set("model","nova-3-general"), which overwrites any
client-supplied model; remove that hardcoded assignment so the function
preserves an incoming model parameter (only set a default if none was provided),
or replace it with plan-based gating: fetch the caller's subscription/plan,
validate allowed models for that plan and only override or reject when the plan
disallows the requested model, or extract the model default into a named
constant/config and document why it’s used; ensure you check incoming
searchParams first, apply plan checks synchronously/async where needed, and
return a clear error when a requested model is not permitted.
No description provided.