Cloud plan handing #769
Cloud plan handing #769
Conversation
yujonglee
force-pushed
the
cloud-plan-handle
branch
from
aefe0d7 to
80d604d
Compare
|
WalkthroughThis update introduces a comprehensive "membership" feature across the Rust backend, Tauri desktop app, and plugin ecosystem. It adds new crates for membership interfaces and plugins, centralizes environment/configuration management, enhances Stripe billing integration, and refactors subscription and billing logic. The desktop UI is updated to reflect dynamic subscription states, and obsolete billing-related UI and localization entries are removed. Permission schemas and plugin bindings are expanded to support the new functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DesktopApp
participant MembershipPlugin
participant Backend
participant Stripe
User->>DesktopApp: Clicks "Upgrade to Pro"
DesktopApp->>MembershipPlugin: commands.refresh()
MembershipPlugin->>Backend: GET /api/web/checkout
Backend->>Stripe: Create Checkout Session
Stripe-->>Backend: Returns session URL
Backend-->>MembershipPlugin: Returns session URL
MembershipPlugin-->>DesktopApp: Returns session URL
DesktopApp->>User: Opens Stripe Checkout in browser
Note over User,DesktopApp: After payment...
Stripe->>Backend: Sends webhook event
Backend->>Database: Update billing/subscription
Backend->>MembershipPlugin: (on next refresh) Returns updated subscription
MembershipPlugin-->>DesktopApp: Updates subscription state
DesktopApp->>User: Shows Pro mode enabled
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🔭 Outside diff range comments (6)
crates/db-admin/src/billings_migration.sql (1)
1-8: 💡 Verification agent❓ Verification inconclusive
Ensure data migration strategy for existing billing records.
This schema change replaces
organization_idwithaccount_id, which is a breaking change. Ensure that:
- Existing billing records are migrated properly
- The migration is coordinated with corresponding code changes
- A rollback plan exists if needed
Verify if there's a data migration script for existing records:
🏁 Script executed:
#!/bin/bash # Look for migration scripts that handle data transformation fd -e sql -e rs | xargs rg -l "UPDATE.*billings.*organization_id|INSERT.*billings.*SELECT" rg "migration.*billing" --type rust -A 5Length of output: 135
Missing data migration for existing billing records
I didn’t find any script that migrates existing
organization_idvalues into the newaccount_idcolumn. Please ensure:
- A data-migration step in
crates/db-admin/src/billings_migration.sql(or a separate migration) that updates all existing rows to setaccount_idbased on the previousorganization_id.- Application code changes are coordinated so that new writes/read use
account_idconsistently.- A clear rollback plan for both schema and data migration in case of issues.
apps/desktop/src/contexts/hypr.tsx (1)
18-33: 🛠️ Refactor suggestionConsider handling subscription query errors consistently.
The subscription query lacks the same error handling as the other queries. If the subscription fetch fails, it could cause unexpected behavior in components relying on subscription data.
Consider adding error handling for the subscription query:
if (userId.status === "pending" || onboardingSessionId.status === "pending") { return null; } - if (userId.status === "error" || onboardingSessionId.status === "error") { - console.error(userId.error, onboardingSessionId.error); + if (userId.status === "error" || onboardingSessionId.status === "error" || subscription.status === "error") { + console.error(userId.error, onboardingSessionId.error, subscription.error); return null; }crates/db-admin/src/billings_ops.rs (1)
26-59: 🛠️ Refactor suggestionReview JSON serialization error handling in create_billing method.
The method handles most errors properly, but the JSON serialization could fail and cause panics.
- libsql::Value::Text(serde_json::to_string(&stripe_subscription).unwrap()), - libsql::Value::Text(serde_json::to_string(&stripe_customer).unwrap()), + libsql::Value::Text(serde_json::to_string(&stripe_subscription) + .map_err(|e| crate::Error::Serialization(e.to_string()))?), + libsql::Value::Text(serde_json::to_string(&stripe_customer) + .map_err(|e| crate::Error::Serialization(e.to_string()))?),apps/app/server/src/main.rs (3)
49-57: 🛠️ Refactor suggestion
env::load()panics on mis-configuration – surface a clear error instead
env::load()internally callsenvy::from_env::<ENV>().unwrap()which will abort the whole process if an expected variable is missing or malformed.
For a production server, fail-fast is good, but panicking produces an opaque stack-trace and bypasses Sentry logging.Consider returning a
Result<ENV, envy::Error>and printing a concise, user-friendly message before exiting:- let config = env::load(); + let config = env::load().unwrap_or_else(|e| { + eprintln!("Configuration error: {e}"); + std::process::exit(1); + });
109-128:⚠️ Potential issueUnrecoverable
unwrap()when connecting to the admin DB
generate_db_token(...).await.unwrap()and the subsequentDatabaseBuilder::build().await.unwrap()will crash the whole service on transient Turso outages.Prefer graceful shutdown or retry logic:
- let db_token = turso.generate_db_token(&db_name).await.unwrap(); + let db_token = turso + .generate_db_token(&db_name) + .await + .map_err(|e| { + tracing::error!("turso_token_generation_failed: {e}"); + Error::Internal("unable_to_connect_database") + })?;This converts the panic into a logged error and allows upstream error handling.
280-288: 🛠️ Refactor suggestionSwallowing server / worker errors
tokio::join!(http, monitor)assigns to_resultand discards it. If either future returnsErr, the error is silently ignored.Recommend propagating or logging:
- let _result = tokio::join!(http, monitor); + match tokio::join!(http, monitor) { + (Err(http_err), _) => tracing::error!("http_server_error: {http_err}"), + (_, Err(worker_err)) => tracing::error!("worker_monitor_error: {worker_err}"), + _ => {} + }
♻️ Duplicate comments (2)
crates/db-admin/src/billings_ops.rs (2)
55-55: Consistent error handling needed for deserialization.Same issue as the get method - using
.unwrap()for deserialization could cause panics.Apply the same error handling improvement as suggested for line 20-21.
83-83: Consistent error handling needed for remaining unwrap calls.The
update_stripe_customerandupdate_stripe_subscriptionmethods also use.unwrap()for deserialization.Apply the same error handling improvement pattern to these methods as well.
Also applies to: 114-114
🧹 Nitpick comments (27)
plugins/membership/.gitignore (2)
1-9: Ignore IDE and system files comprehensively
The existing entries catch many IDE configs and OS artifacts. Consider ignoring your entire.vscode/directory (not justsettings.json) to cover user-specific settings, and you might fold.DS_Storeand.Thumbs.dbinto a single, case-insensitive pattern for consistency.Example:
-.vscode/settings.json +.vscode/ - .DS_Store - .Thumbs.db +*.[dD][sS]_[sS]tore +*.Thumbs.db
10-17: Normalize directory patterns and include missing lock files
Directory names likedist-jsanddistlack trailing slashes—adding them prevents accidentally ignoring files with similar names. Also, if your JS workflow ever uses pnpm, includepnpm-lock.yaml.- debug.log - package-lock.json - yarn.lock - /tauri - /target - Cargo.lock - node_modules/ - dist-js - dist +debug.log +/package-lock.json +/yarn.lock +/pnpm-lock.yaml +/.tauri/ +/target/ +Cargo.lock +node_modules/ +/dist-js/ +/dist/apps/app/server/src/env.rs (1)
8-31: Well-structured configuration with comprehensive coverage.The
ENVstruct provides good coverage of all necessary configuration values. The use ofserde::Deserializewithenvyprovides type safety and clear environment variable mapping.Consider adding documentation comments for sensitive or less obvious configuration fields:
#[derive(Debug, serde::Deserialize)] pub struct ENV { + /// Sentry DSN for error reporting pub sentry_dsn: String, + /// Turso database API key pub turso_api_key: String, // ... other fields could benefit from similar documentationplugins/membership/permissions/autogenerated/reference.md (1)
5-5: Fix markdown heading structure and formatting.The static analysis correctly identifies markdown formatting issues that should be addressed.
Apply these fixes to improve markdown compliance:
-#### This default permission set includes the following: +### This default permission set includes the followingThe heading level should be h3 (###) instead of h4 (####), and the trailing colon should be removed.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
5-5: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4(MD001, heading-increment)
5-5: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
apps/desktop/src-tauri/src/lib.rs (1)
2-2: Consider graceful error handling for missing environment variables.The centralized configuration loading is a good improvement. However, the
env::load()function uses.unwrap()which will panic if required environment variables are missing.Consider updating the
env::load()function to provide more graceful error handling:pub fn load() -> ENV { - envy::from_env::<ENV>().unwrap() + envy::from_env::<ENV>().unwrap_or_else(|e| { + eprintln!("Failed to load environment variables: {}", e); + std::process::exit(1); + }) }Also applies to: 19-19
apps/desktop/src-tauri/src/env.rs (1)
9-11: Consider adding .env.local loading for development consistency.The load function is simple and clean. However, the server version in
apps/app/server/src/env.rsalso loads from.env.localin debug mode, which this desktop version doesn't.Consider adding
.env.localloading for development consistency:pub fn load() -> ENV { + #[cfg(debug_assertions)] + { + if let Err(e) = dotenv::from_filename(".env.local") { + // Silently ignore if .env.local doesn't exist + } + } envy::from_env::<ENV>().unwrap() }This would allow developers to use
.env.localfiles consistently across desktop and server environments.apps/app/server/src/web/connect.rs (2)
46-46: Consider making trial period configurable.The 7-day trial period is hard-coded. Consider making this configurable through environment variables or application configuration.
- stripe_ops::create_subscription_with_trial(&state.stripe, customer.id.clone(), 7) + stripe_ops::create_subscription_with_trial(&state.stripe, customer.id.clone(), state.config.trial_period_days)
44-54: Consider wrapping Stripe operations in a database transaction.The current implementation could leave the system in an inconsistent state if Stripe operations succeed but database operations fail. Consider wrapping the entire account creation process in a database transaction.
apps/app/server/openapi.gen.json (1)
56-67: Consider enhancing the checkout endpoint specification.The new
/api/web/checkoutendpoint appears functional but could benefit from additional specification details:
- Response schema: Consider defining a more specific response format or documenting what plain text content is returned
- Error responses: Add specifications for error cases (4xx, 5xx)
- Parameters: If the endpoint accepts query parameters or requires authentication, these should be documented
- Description: Add a description field explaining the endpoint's purpose
This would improve API documentation and client integration.
apps/app/server/src/state.rs (1)
23-23: Consider security implications for webhook signing secret.The addition of
stripe_webhook_signing_secrettoAppStateis correct for webhook validation. Ensure this secret is:
- Loaded securely from environment variables
- Not logged in debug output
- Properly validated before use
plugins/membership/src/lib.rs (2)
12-20: Consider adding documentation for the Specta builder.The Specta builder configuration is correct, but consider adding documentation:
+/// Creates a Specta builder with membership plugin commands and error handling fn make_specta_builder<R: tauri::Runtime>() -> tauri_specta::Builder<R> {The error handling mode
Throwis appropriate for frontend integration where exceptions should be caught and handled by the UI.
47-58: Good integration test coverage.The test verifies:
- Plugin can be added to a Tauri app
- Integration with tauri_plugin_store
- Basic app creation succeeds
Consider adding a test that verifies the commands can be invoked successfully.
apps/app/server/src/native/subscription.rs (1)
7-26: Comprehensive subscription mapping with room for simplification.The Stripe to internal subscription status mapping is complete and correct. However, the price ID extraction can be simplified for better readability.
Consider simplifying the nested Option handling:
- price_id: s - .items - .data - .first() - .map(|item| item.price.as_ref().map(|p| p.id.to_string())) - .flatten(), + price_id: s + .items + .data + .first() + .and_then(|item| item.price.as_ref()) + .map(|p| p.id.to_string()),apps/app/server/src/stripe/webhook.rs (1)
99-129: Consider adding concurrency limits and timeout handling.The background task approach is good for non-blocking webhook responses. However, consider implementing the suggested concurrency limit from the TODO comment to prevent resource exhaustion during high webhook volume.
// TODO: do this with background worker with concurrency limit + // Consider using a semaphore to limit concurrent webhook processing + static WEBHOOK_SEMAPHORE: tokio::sync::Semaphore = tokio::sync::Semaphore::const_new(10);Additionally, consider adding timeout handling for the Stripe API calls to prevent hanging tasks.
apps/desktop/src/contexts/hypr.tsx (2)
52-52: Make isPro computation more defensive.The current implementation could throw an error if
subscription.datais undefined. Consider making it more defensive.- isPro: subscription.data?.status === "active" || subscription.data?.status === "trialing", + isPro: subscription.data?.status === "active" || subscription.data?.status === "trialing" || false,Or even better:
- isPro: subscription.data?.status === "active" || subscription.data?.status === "trialing", + isPro: ["active", "trialing"].includes(subscription.data?.status || ""),
30-30: Consider caching strategy for subscription refresh.Using
membershipCommands.refresh()on every context initialization makes an API call each time. Consider usingmembershipCommands.getSubscription()first and only refresh if needed, or implement a smarter caching strategy.{ queryKey: ["subscription"], queryFn: async () => { // Try getting cached subscription first const cached = await membershipCommands.getSubscription(); if (cached) { return cached; } // Only refresh if no cached data return membershipCommands.refresh(); }, },plugins/membership/Cargo.toml (2)
1-9: Consider adding missing package metadata fields.The package configuration is functional but missing some important metadata fields that would be helpful for documentation and discoverability.
[package] name = "tauri-plugin-membership" version = "0.1.0" authors = ["You"] edition = "2021" exclude = ["./js"] links = "tauri-plugin-membership" -description = "" +description = "Tauri plugin for managing user membership and subscription data" +license = "MIT OR Apache-2.0" +repository = "https://github.com/fastrepl/hyprnote"
4-4: Update placeholder author information.The author field contains placeholder text that should be updated to reflect the actual maintainers.
-authors = ["You"] +authors = ["Fastrepl Team <[email protected]>"]apps/desktop/src/components/left-sidebar/top-area/settings-button.tsx (2)
80-104: Consider extracting the background pattern to a reusable constant.Both
ProModeandLocalModecomponents use the same complex SVG background pattern. This creates code duplication and makes maintenance harder.+const GRID_BACKGROUND_PATTERN = "bg-[url('data:image/svg+xml;base64,PHN2ZyB3aWR0aD0iMjAwIiBoZWlnaHQ9IjIwMCIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIj48ZGVmcz48cGF0dGVybiBpZD0iZ3JpZCIgd2lkdGg9IjIwIiBoZWlnaHQ9IjIwIiBwYXR0ZXJuVW5pdHM9InVzZXJTcGFjZU9uVXNlIj48cGF0aCBkPSJNIDIwIDAgTCAwIDAgTCAwIDIwIiBmaWxsPSJub25lIiBzdHJva2U9InJnYmEoMjU1LDI1NSwyNTUsMC4xNSkiIHN0cm9rZS13aWR0aD0iMS41Ii8+PC9wYXR0ZXJuPjwvZGVmcz48cmVjdCB3aWR0aD0iMTAwJSIgaGVpZ2h0PSIxMDAlIiBmaWxsPSJ1cmwoI2dyaWQpIi8+PC9zdmc+')] opacity-70"; function ProMode({ onClick }: { onClick: () => void }) { return ( <div className={cn([ "px-2 py-3 bg-gradient-to-r rounded-t-md relative overflow-hidden cursor-pointer", "from-blue-500 to-blue-600 hover:from-blue-600 hover:to-blue-700", ])} onClick={onClick} > - <div className="absolute inset-0 bg-[url('data:image/svg+xml;base64,PHN2ZyB3aWR0aD0iMjAwIiBoZWlnaHQ9IjIwMCIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIj48ZGVmcz48cGF0dGVybiBpZD0iZ3JpZCIgd2lkdGg9IjIwIiBoZWlnaHQ9IjIwIiBwYXR0ZXJuVW5pdHM9InVzZXJTcGFjZU9uVXNlIj48cGF0aCBkPSJNIDIwIDAgTCAwIDAgTCAwIDIwIiBmaWxsPSJub25lIiBzdHJva2U9InJnYmEoMjU1LDI1NSwyNTUsMC4xNSkiIHN0cm9rZS13aWR0aD0iMS41Ii8+PC9wYXR0ZXJuPjwvZGVmcz48cmVjdCB3aWR0aD0iMTAwJSIgaGVpZ2h0PSIxMDAlIiBmaWxsPSJ1cmwoI2dyaWQpIi8+PC9zdmc+')] opacity-70"> + <div className={`absolute inset-0 ${GRID_BACKGROUND_PATTERN}`}>
92-92: Consider accessibility improvements for animated icons.The
animate-pulseclass might cause issues for users with vestibular disorders. Consider addingprefers-reduced-motionsupport.- <TrainFrontIcon className="size-8 animate-pulse" /> + <TrainFrontIcon className="size-8 animate-pulse motion-reduce:animate-none" />- <CpuIcon className="size-8 animate-pulse" /> + <CpuIcon className="size-8 animate-pulse motion-reduce:animate-none" />Also applies to: 118-118
apps/app/server/src/stripe/ops.rs (1)
72-79: Remove dead code or document why it's kept.The
trial_settings_for_checkoutfunction is marked with#[allow(dead_code)]but might be needed for future functionality.Consider either:
- Removing it if not needed
- Adding a comment explaining why it's kept for future use
- Implementing the checkout with trial functionality if it's planned
+// Reserved for future checkout sessions with trial periods #[allow(dead_code)] fn trial_settings_for_checkout() -> CreateCheckoutSessionSubscriptionDataTrialSettings {plugins/membership/permissions/schemas/schema.json (2)
94-99: Minor wording & typo inPermissiondescription
"Descriptions of explicit privileges of commands."reads awkwardly and uses the plural “Descriptions” for a singular item.
Consider:- "description": "Descriptions of explicit privileges of commands.\n\nIt can enable commands to be accessible in the frontend of the application.\n\nIf the scope is defined it can be used to fine grain control the access of individual or multiple commands.", + "description": "Description of explicit command privileges.\n\nThis permission enables specific commands in the frontend. If a scope is provided, it offers fine-grained access control over one or multiple commands.",
120-139: Schema looseness – allowlisting unknown keysNone of the object definitions (
Permission,PermissionSet,DefaultPermission, etc.) specify"additionalProperties": falseWithout this, any unknown field will silently validate, weakening contract enforcement. Explicitly disallowing extra properties prevents configuration mistakes from going unnoticed.
apps/app/server/src/main.rs (1)
175-180: Middleware order: ensureClerkLayerexecutes beforeattach_user_from_clerkCurrently:
attach_user_from_clerk- outer
ClerkLayerAxum applies outer layers first, so
ClerkLayerwill indeed precedeattach_user_from_clerk. Good.
However, the intent is not obvious when reading the two declarations separately. A short inline comment would save future maintainers from mis-ordering layers.// ClerkLayer must wrap router so Extension<ClerkJwt> is available to inner middlewareapps/desktop/src/routes/app.plans.tsx (3)
194-306: High duplication across the three render helpers
RenderInactive,RenderActiveWithTrial, andRenderActiveWithoutTrialrepeat ~90 lines each, differing only in:• extra
subscriptionInfonode
• which plan is markedisActive
• copy insidesubscriptionInfoExtract the common grid into a single
<PlansGrid variant="inactive" | "trial" | "active" subscription={subscription}/>component. This will:
- Reduce maintenance overhead when features/copy change.
- Prevent divergent UI bugs between variants.
- Simplify adding new plan tiers in the future.
Happy to sketch the refactor if useful.
35-47:subscriptionInfotype leaks presentation into data layerPassing a
ReactNodeup toPricingCardforces callers to construct JSX, coupling what we want to show with how we show it.
Instead, accept structured data ({ label: string; date?: Date; }) and letPricingCardrender the markup. This keeps the parent components data-centric and improves testability.
63-75: Add explicitalttext orrole="presentation"for background imagesScreen readers may announce the image URLs because they’re injected via inline style. Either:
<div role="presentation" aria-hidden="true" … />or move them to CSS with
.bg-local-cardutility classes to keep markup semantically clean.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
Cargo.lockis excluded by!**/*.lockpackages/client/generated/@tanstack/react-query.gen.tsis excluded by!**/generated/**packages/client/generated/sdk.gen.tsis excluded by!**/generated/**packages/client/generated/types.gen.tsis excluded by!**/generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (67)
Cargo.toml(3 hunks)Taskfile.yaml(1 hunks)apps/app/server/Cargo.toml(2 hunks)apps/app/server/openapi.gen.json(1 hunks)apps/app/server/src/env.rs(1 hunks)apps/app/server/src/main.rs(8 hunks)apps/app/server/src/middleware.rs(2 hunks)apps/app/server/src/nango.rs(1 hunks)apps/app/server/src/native/subscription.rs(1 hunks)apps/app/server/src/state.rs(2 hunks)apps/app/server/src/stripe/mod.rs(1 hunks)apps/app/server/src/stripe/ops.rs(1 hunks)apps/app/server/src/stripe/webhook.rs(4 hunks)apps/app/server/src/types.rs(0 hunks)apps/app/server/src/web/checkout.rs(1 hunks)apps/app/server/src/web/connect.rs(3 hunks)apps/app/server/src/web/mod.rs(1 hunks)apps/app/src/routeTree.gen.ts(13 hunks)apps/app/src/routes/checkout.success.tsx(1 hunks)apps/desktop/package.json(1 hunks)apps/desktop/src-tauri/Cargo.toml(2 hunks)apps/desktop/src-tauri/capabilities/default.json(1 hunks)apps/desktop/src-tauri/src/env.rs(1 hunks)apps/desktop/src-tauri/src/lib.rs(5 hunks)apps/desktop/src/components/left-sidebar/top-area/settings-button.tsx(4 hunks)apps/desktop/src/components/settings/views/billing.tsx(0 hunks)apps/desktop/src/components/settings/views/index.ts(0 hunks)apps/desktop/src/components/settings/views/lab.tsx(0 hunks)apps/desktop/src/components/welcome-modal/index.tsx(3 hunks)apps/desktop/src/contexts/hypr.tsx(4 hunks)apps/desktop/src/contexts/index.ts(0 hunks)apps/desktop/src/contexts/login-modal.tsx(0 hunks)apps/desktop/src/locales/en/messages.po(23 hunks)apps/desktop/src/locales/ko/messages.po(23 hunks)apps/desktop/src/routes/app.plans.tsx(6 hunks)apps/docs/data/i18n.json(1 hunks)crates/db-admin/src/billings_migration.sql(1 hunks)crates/db-admin/src/billings_ops.rs(1 hunks)crates/db-admin/src/billings_types.rs(2 hunks)plugins/analytics/src/lib.rs(2 hunks)plugins/auth/js/bindings.gen.ts(1 hunks)plugins/auth/src/store.rs(1 hunks)plugins/connector/Cargo.toml(1 hunks)plugins/connector/src/ext.rs(2 hunks)plugins/membership-interface/Cargo.toml(1 hunks)plugins/membership-interface/src/lib.rs(1 hunks)plugins/membership/.gitignore(1 hunks)plugins/membership/Cargo.toml(1 hunks)plugins/membership/build.rs(1 hunks)plugins/membership/js/bindings.gen.ts(1 hunks)plugins/membership/js/index.ts(1 hunks)plugins/membership/package.json(1 hunks)plugins/membership/permissions/autogenerated/commands/get_subscription.toml(1 hunks)plugins/membership/permissions/autogenerated/commands/refresh.toml(1 hunks)plugins/membership/permissions/autogenerated/reference.md(1 hunks)plugins/membership/permissions/default.toml(1 hunks)plugins/membership/permissions/schemas/schema.json(1 hunks)plugins/membership/src/commands.rs(1 hunks)plugins/membership/src/error.rs(1 hunks)plugins/membership/src/ext.rs(1 hunks)plugins/membership/src/lib.rs(1 hunks)plugins/membership/src/store.rs(1 hunks)plugins/membership/tsconfig.json(1 hunks)plugins/store2/permissions/autogenerated/commands/ping.toml(0 hunks)plugins/store2/permissions/autogenerated/reference.md(1 hunks)plugins/store2/permissions/default.toml(1 hunks)plugins/store2/permissions/schemas/schema.json(1 hunks)
💤 Files with no reviewable changes (7)
- apps/desktop/src/contexts/index.ts
- apps/desktop/src/components/settings/views/index.ts
- plugins/store2/permissions/autogenerated/commands/ping.toml
- apps/app/server/src/types.rs
- apps/desktop/src/contexts/login-modal.tsx
- apps/desktop/src/components/settings/views/billing.tsx
- apps/desktop/src/components/settings/views/lab.tsx
🧰 Additional context used
🧬 Code Graph Analysis (11)
apps/desktop/src-tauri/src/env.rs (1)
apps/app/server/src/env.rs (1)
load(1-6)
apps/desktop/src-tauri/src/lib.rs (3)
apps/desktop/src-tauri/src/env.rs (1)
load(9-11)plugins/membership/src/lib.rs (1)
init(22-28)plugins/analytics/src/lib.rs (1)
init(27-38)
plugins/membership/src/store.rs (1)
plugins/membership/js/bindings.gen.ts (1)
Subscription(28-28)
plugins/membership/src/commands.rs (2)
plugins/membership/js/bindings.gen.ts (2)
Subscription(28-28)refresh(10-12)plugins/membership/src/ext.rs (4)
refresh(9-9)refresh(25-38)get_subscription(8-8)get_subscription(17-23)
apps/desktop/src/components/welcome-modal/index.tsx (1)
apps/desktop/src/client.ts (1)
baseUrl(10-12)
apps/app/server/src/env.rs (1)
apps/desktop/src-tauri/src/env.rs (1)
load(9-11)
plugins/membership-interface/src/lib.rs (1)
plugins/membership/js/bindings.gen.ts (2)
Subscription(28-28)SubscriptionStatus(29-29)
apps/desktop/src/components/left-sidebar/top-area/settings-button.tsx (2)
apps/desktop/src/contexts/hypr.tsx (1)
useHypr(62-68)packages/ui/src/lib/utils.ts (1)
cn(4-6)
crates/db-admin/src/billings_ops.rs (2)
crates/db-core/src/lib.rs (1)
conn(17-22)crates/db-admin/src/billings_types.rs (1)
from_row(15-28)
plugins/connector/src/ext.rs (2)
plugins/auth/js/bindings.gen.ts (1)
StoreKey(54-54)plugins/flags/js/bindings.gen.ts (1)
StoreKey(31-31)
apps/app/server/src/main.rs (7)
apps/app/server/src/env.rs (1)
load(1-6)apps/app/server/src/nango.rs (1)
handler(10-45)apps/app/server/src/stripe/webhook.rs (1)
handler(46-137)apps/app/server/src/web/checkout.rs (1)
handler(10-54)apps/app/server/src/web/connect.rs (1)
handler(11-135)apps/app/server/src/state.rs (5)
from_ref(61-68)from_ref(72-77)from_ref(81-87)from_ref(91-95)from_ref(99-106)apps/app/server/src/middleware.rs (2)
attach_user_from_clerk(58-91)verify_api_key(15-55)
🪛 markdownlint-cli2 (0.17.2)
plugins/membership/permissions/autogenerated/reference.md
5-5: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
5-5: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🪛 GitHub Actions: .github/workflows/desktop_ci.yaml
apps/desktop/src/components/welcome-modal/index.tsx
[error] 88-88: TypeScript error TS6133: '_handleStartCloud' is declared but its value is never read.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: ci
🔇 Additional comments (76)
plugins/store2/permissions/schemas/schema.json (1)
369-374: PermissionKind default entry updated correctly
Thedefaultconstant now enumerates the six explicitallow-*permissions and omitsallow-ping/deny-ping, matching the schema and default config.plugins/store2/permissions/autogenerated/reference.md (1)
7-12: Default permission documentation updated
The bulleted list accurately reflects the six granularstore2permissions. Removing thepingentries aligns with the updated schema and default settings.plugins/store2/permissions/default.toml (1)
3-10: Default TOML permissions updated
The default permissions array now contains the six specificallow-*entries and excludesping, keeping it in sync with both the schema and documentation.plugins/auth/src/store.rs (1)
13-16: LGTM! Consistent implementation following established patterns.The new
Planvariant is properly implemented with all required serialization annotations that match the existingUserIdandAccountIdvariants. The naming convention "auth-plan" aligns with the established pattern.apps/desktop/src/locales/ko/messages.po (1)
273-284: Appropriate localization cleanup aligned with UI refactoring.The obsolescence of these Korean translation entries (lines 273-284 and others throughout the file) correctly reflects the removal of lab and billing-related UI components mentioned in the AI summary. The
#~prefix properly marks these strings as obsolete.plugins/analytics/src/lib.rs (2)
62-66: Test helper appropriately updated.The test helper function correctly provides the required API key parameter to maintain test functionality.
27-27:✅ Verification successful
Good refactoring to centralize configuration management.
The change from compile-time environment variable access to runtime parameter passing aligns well with the centralized configuration approach mentioned in the AI summary.
Verify that all callers of the
initfunction have been updated to pass the API key parameter:
🏁 Script executed:
#!/bin/bash # Search for all calls to analytics plugin init function rg -A 3 -B 1 "analytics.*init|init.*analytics"Length of output: 481
Analytics
initcallers updated correctlyConfirmed that
tauri_plugin_analytics::initis only invoked inapps/desktop/src-tauri/src/lib.rsand it’s passedconfig.posthog_api_key. No other calls were found—no further changes needed.apps/app/server/src/env.rs (1)
33-35: Clean default value implementation.The default port function is simple and clear. The choice of "1234" as the default port is consistent with the development setup mentioned in the AI summary.
apps/app/server/src/web/mod.rs (1)
1-1: Addcheckoutmodule declaration
Introduces thecheckoutsubmodule underwebfor routing Stripe checkout endpoints. Ensure thatcheckout.rsexists and exports the expected handler functions.apps/app/server/Cargo.toml (2)
12-12: Includehypr-membership-interfaceas a workspace dependency
Adds the membership interface crate to support the new plugin system. Confirm that it's correctly listed in the root workspace and resolves without version conflicts.
53-53: Addenvyfor environment variable deserialization
Introducesenvyto centralize configuration parsing via structs—aligns with the newenv.rsmodule. Ensure no duplicatedotenvvs.envyoverlap.plugins/membership/permissions/default.toml (1)
1-3: Define default membership plugin permissions
Establishes the baseline commands (allow-refresh,allow-get-subscription) for the membership plugin. Verify these keys match the actual permission checks in the plugin code.apps/desktop/package.json (1)
31-31: Add@hypr/plugin-membershipdependency
Integrates the membership plugin into the desktop app’s workspace dependencies. Don’t forget to runpnpm installand update any lock files.Taskfile.yaml (1)
13-13: Update Stripe webhook forwarding port
Aligns thestripe listenforward-to URL with the new default server port (1234). Confirm that the application’s server is configured to listen on this port for webhooks.plugins/connector/Cargo.toml (1)
17-17: Approved: Dependency reorder and feature addition fortauri
Thetauridependency has been moved within the[dependencies]section and now includes thetestfeature as intended. Cargo treats dependency order as non-semantic, so this change is safe.plugins/membership/package.json (1)
1-11: Approve new plugin manifest
Thepackage.jsoncorrectly declares the private plugin, sets the entry point, and includes thecodegenscript. The dependency on@tauri-apps/api@^2.5.0aligns with the rest of the Tauri ecosystem.apps/desktop/src-tauri/Cargo.toml (1)
38-38: Approve: Added membership plugin
Includingtauri-plugin-membershipas a workspace dependency enables the new membership commands in the desktop app.plugins/membership/js/index.ts (1)
1-1: Approve: Re-exporting bindings
This module cleanly re-exports all generated bindings, providing a single entry point for consumers.apps/docs/data/i18n.json (1)
4-5: Approve: Updated translation entry counts
Thetotalandmissingfields have been updated to 266 for both languages, reflecting the newly added membership and subscription keys.Also applies to: 9-10
plugins/membership/tsconfig.json (1)
1-5: LGTM! Standard TypeScript configuration.The configuration properly extends the base TypeScript config and includes the appropriate files for the membership plugin's TypeScript bindings.
plugins/membership-interface/Cargo.toml (1)
1-9: LGTM! Well-structured Cargo manifest.The dependencies are appropriate for a membership interface crate - schemars for schema generation, serde for serialization, and specta for TypeScript bindings. Using workspace dependencies ensures version consistency across the project.
apps/app/server/src/stripe/mod.rs (1)
1-2: LGTM! Clean module organization.The separation of Stripe operations and webhook handling into distinct modules follows good Rust practices for code organization.
plugins/auth/js/bindings.gen.ts (1)
54-54: LGTM! Necessary addition for membership feature.The addition of "auth-plan" to the StoreKey type properly supports the new membership functionality by allowing plan information to be stored and retrieved from the auth store.
plugins/membership/build.rs (1)
1-5: LGTM! Standard Tauri plugin build script.The build script correctly follows Tauri plugin conventions with appropriate command definitions for membership functionality.
plugins/membership/permissions/autogenerated/commands/refresh.toml (1)
1-13: Auto-generated permission file looks correct.The permission structure follows Tauri conventions with appropriate allow/deny configurations for the refresh command. Since this is auto-generated (line 1), any manual modifications would be overwritten.
apps/desktop/src-tauri/capabilities/default.json (1)
51-51: Good addition of membership plugin permission.The "membership:default" permission correctly integrates the new membership plugin into the desktop app capabilities.
plugins/membership/permissions/autogenerated/commands/get_subscription.toml (1)
1-13: Consistent auto-generated permission structure.The permission file correctly mirrors the refresh command structure with appropriate allow/deny configurations for get_subscription. The consistency across command permission files is good practice.
apps/app/server/src/middleware.rs (2)
53-53: LGTM: Billing data added to request extensions.This enables downstream handlers to access billing information for cloud plan enforcement, which aligns well with the PR objectives.
45-50:❌ Incorrect review comment
Consider handling optional billing data more gracefully.
The billing retrieval will fail the entire request if no billing record exists for an account. For new accounts or free-tier users, this might not be appropriate behavior.
Consider making billing data optional to handle accounts without billing records:
- let billing = state - .admin_db - .get_billing_by_account_id(&account.id) - .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + let billing = state + .admin_db + .get_billing_by_account_id(&account.id) + .await + .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;Verify if billing records are expected to exist for all accounts:
🏁 Script executed:
#!/bin/bash # Check if billing creation is mandatory for all accounts ast-grep --pattern 'create_billing($_)' rg -A 5 "create_billing|insert.*billing" --type rustLength of output: 1312
🏁 Script executed:
#!/bin/bash # Verify the signature of get_billing_by_account_id to confirm its return type rg -n "pub async fn get_billing_by_account_id" -A3 crates/db-admin/src/billings_ops.rsLength of output: 243
Ignore optional billing suggestion—get_billing_by_account_id already returns an Option
The call
let billing = state .admin_db .get_billing_by_account_id(&account.id) .await .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;yields a
Result<Option<Billing>, Error>, so missing records map toNonerather than an error. No change is needed to “make billing data optional.”Likely an incorrect or invalid review comment.
plugins/membership/permissions/autogenerated/reference.md (1)
7-8: LGTM: Permission definitions are well-structured.The permission table clearly defines the available permissions for the membership plugin, covering both allow and deny variants for the
get_subscriptionandrefreshcommands.Also applies to: 22-22, 35-35, 48-48, 61-61
apps/app/server/src/nango.rs (1)
7-7: LGTM: Clean state refactoring to WebhookState.The change to use
WebhookStateinstead ofAppStateis a good refactoring that provides webhook handlers with only the necessary state components, improving modularity and reducing coupling.Also applies to: 11-11
crates/db-admin/src/billings_types.rs (1)
15-18: LGTM! Method correctly updated to match field rename.The
from_rowmethod update correctly reflects the field rename fromorganization_idtoaccount_id. The database column parsing is consistent with the struct definition.apps/desktop/src-tauri/src/lib.rs (3)
33-33: LGTM! Runtime configuration is more flexible than compile-time conditionals.Using
config.sentry_dsnfrom environment variables is an improvement over compile-time DSN selection. This allows the same binary to work across different environments.
80-80: LGTM! Analytics plugin correctly receives API key from configuration.The analytics plugin update is consistent with the centralized configuration loading and matches the expected plugin interface that requires an API key parameter.
58-58: LGTM! Membership plugin correctly integrated into the plugin chain.The new membership plugin is properly added to the Tauri builder chain and follows the same initialization pattern as other plugins.
plugins/membership/src/error.rs (2)
3-9: LGTM! Well-designed error enum following Rust best practices.The error enum design is excellent:
- Uses
thiserror::Errorfor automatic error trait implementation- Transparent error propagation with
#[error(transparent)]- Automatic conversions with
#[from]attributes- Covers the two main error sources: HTTP requests and store operations
11-18: LGTM! Appropriate serialization for Tauri error handling.The
Serializeimplementation correctly converts errors to string representation, which is perfect for sending error information across the Tauri bridge to the frontend.apps/desktop/src-tauri/src/env.rs (1)
1-7: LGTM! Clean environment configuration struct with development-friendly defaults.The
ENVstruct design is excellent:
- Conditional default values in debug mode make development easier
- Clear field names that match their usage
- Proper derives for debugging and deserialization
plugins/membership/src/store.rs (1)
1-8: LGTM! Clean and proper store key implementation.The implementation correctly follows Tauri plugin store patterns with appropriate trait derivations for serialization, hashing, and display functionality. The
ScopedStoreKeyimplementation enables proper scoped storage for subscription data.plugins/connector/src/ext.rs (2)
172-177: LGTM! Consistent plan checking logic.The
get_stt_connectionfunction uses identical plan checking logic toget_llm_connection, ensuring consistency across the connector plugin.
109-114:✅ Verification successful
Verify plan value consistency across the codebase.
The transition from feature flags to plan-based access control looks correct. The logic properly fallbacks to "free" plan and compares against "pro" for cloud access.
Please verify that plan values are consistent across the codebase:
🏁 Script executed:
#!/bin/bash # Description: Check for plan value usage consistency across the codebase # Expected: All plan checks should use consistent string values echo "=== Searching for plan value references ===" rg -A 3 -B 3 '"pro"' --type rust echo echo "=== Searching for plan enum/constant definitions ===" rg -A 5 -B 2 'enum.*Plan|const.*PLAN|Plan.*=' --type rust echo echo "=== Searching for other plan comparisons ===" rg -A 3 -B 3 'plan.*==' --type rustLength of output: 4347
Plan value consistency verified
I searched the entire codebase and found only the two occurrences in plugins/connector/src/ext.rs comparing against
"pro"(with a"free"fallback). There are no other plan-related constants, enums, or comparisons elsewhere. The plan strings are used consistently—no changes needed.Cargo.toml (3)
64-64: LGTM! Appropriate membership interface dependency.The addition of
hypr-membership-interfacefollows the established workspace pattern for plugin interfaces.
86-86: LGTM! Consistent plugin dependency addition.The
tauri-plugin-membershipdependency is properly added alongside other Tauri plugins in the workspace.
118-118: LGTM! Standard environment variable handling dependency.The
envycrate is a well-established choice for environment variable deserialization and version 0.4 is appropriate.apps/app/server/src/web/connect.rs (1)
20-29: LGTM! Improved performance by avoiding unnecessary clones.Direct access to
state.admin_dbis more efficient than cloning the database handle for these read operations.apps/desktop/src/locales/en/messages.po (1)
273-284: LGTM: Localization updates align with feature changes.The localization changes properly reflect the UI updates for cloud plan handling:
- Obsolete billing-related entries (lines 273-284) correspond to removed UI components
- New "Pro mode"/"Local mode" entries (lines 703-705, 862-865) support the membership feature
- "or, just use it locally" entry (lines 833-836) reinforces the local/cloud distinction
The translation entries are consistent and properly formatted.
Also applies to: 703-705, 833-836, 862-865
apps/app/server/src/state.rs (1)
26-32: Well-designed modular state structure.The
WebhookStatestruct and itsFromRefimplementation provide good separation of concerns:
- Contains only the necessary components for webhook processing
- Clean conversion from
AppStatevia theFromReftrait- Improves modularity by not exposing unnecessary state to webhook handlers
This follows Axum's best practices for state management.
Also applies to: 60-69
plugins/membership/src/lib.rs (3)
1-9: Well-structured plugin module organization.The module structure is clean and follows Rust conventions:
- Clear separation of concerns (commands, error, ext, store)
- Public re-exports provide a clean API surface
- Module organization supports maintainability
22-28: Clean plugin initialization following Tauri patterns.The plugin initialization correctly:
- Uses the Specta builder for type-safe command handling
- Follows standard Tauri plugin builder patterns
- Provides a generic runtime parameter for flexibility
34-45: Comprehensive TypeScript export test.The test properly configures TypeScript export with:
- Appropriate header and formatting
- BigInt number behavior for compatibility
- Correct output path for generated bindings
This ensures type safety between Rust and TypeScript.
apps/desktop/src/components/welcome-modal/index.tsx (2)
75-86: OAuth URL construction looks correct.The OAuth URL construction properly includes necessary parameters (client ID, fingerprint flag, port) and uses the baseUrl from the client configuration. The query setup follows good practices with proper URL construction.
43-44: Verify simplified authentication flow aligns with requirements.The authentication success handling was simplified from a complex flow that included database setup and modal navigation to just showing a success message. Ensure this change aligns with the intended user experience, especially given the PR objectives mention that "onboarding modal should redirect users to the connect route."
This change may not fulfill the stated objective about redirecting to the connect route. Consider whether additional navigation logic is needed after successful authentication.
apps/app/server/src/web/checkout.rs (2)
32-36: Good error handling for database operations.The billing retrieval properly handles potential database errors and maps them to appropriate HTTP status codes.
38-47: Stripe customer creation logic is well-structured.The code properly handles both existing and new customer scenarios, with appropriate error handling for Stripe operations.
plugins/membership-interface/src/lib.rs (2)
1-9: Excellent use of macro to reduce boilerplate.The
common_derivesmacro effectively reduces repetitive derive statements across the subscription data structures, improving maintainability.
11-32: Well-designed subscription data structures.The
Subscriptionstruct andSubscriptionStatusenum are appropriately designed for subscription management. The snake_case serialization aligns with the TypeScript bindings and follows common API conventions.apps/app/server/src/native/subscription.rs (1)
8-17: Complete and accurate status mapping.The mapping between Stripe's
SubscriptionStatusand the internalSubscriptionStatusenum covers all possible values and maintains semantic consistency.plugins/membership/src/commands.rs (1)
1-19: LGTM! Clean and well-structured Tauri commands.The commands are properly implemented following Tauri plugin patterns. The delegation to extension trait methods is clean, and the annotations are correct for both Tauri integration and type generation.
The error handling converts all errors to strings, which is appropriate for the Tauri command interface and provides consistent error messaging to the frontend.
apps/app/server/src/stripe/webhook.rs (2)
11-11: Good refactoring to use focused state struct.The change from generic
AppStatetoWebhookStateimproves code organization by providing only the necessary components for webhook processing.
16-42: LGTM! Proper webhook signature validation.The
FromRequestimplementation correctly validates Stripe webhook signatures using the signing secret from the state. The error handling appropriately returnsBAD_REQUESTfor invalid signatures.apps/desktop/src/contexts/hypr.tsx (2)
6-6: Good integration of membership plugin.The import and type usage properly integrates the new membership plugin functionality.
11-13: Well-designed context interface extension.The addition of optional
subscriptionandisProfields appropriately extends the context interface for subscription-aware functionality.plugins/membership/src/ext.rs (4)
6-10: Well-designed trait interface.The trait design is clean and follows Rust patterns well. The use of
impl Futurereturn types is appropriate for async methods.
12-16: LGTM! Clean store access implementation.The scoped store implementation properly encapsulates membership data storage.
17-23: Good caching implementation.The get_subscription method properly retrieves cached subscription data from the store.
25-39:❓ Verification inconclusive
Critical security and configuration concerns.
The refresh method has several issues that need attention:
- Unauthenticated requests: The HTTP requests lack authentication, which could expose subscription data or allow unauthorized access.
- Hardcoded URLs: API endpoints should be configurable rather than hardcoded.
- Environment detection: Using
cfg!(debug_assertions)for URL selection isn't ideal for production deployments.Consider this refactor:
async fn refresh(&self) -> Result<Subscription, crate::Error> { - let url = if cfg!(debug_assertions) { - "http://localhost:1234/api/desktop/subscription" - } else { - "https://app.hypr.com/api/desktop/subscription" - }; - - let resp = reqwest::get(url).await?; + let url = std::env::var("MEMBERSHIP_API_URL") + .unwrap_or_else(|_| "https://app.hypr.com/api/desktop/subscription".to_string()); + + // Add authentication token if needed + let client = reqwest::Client::new(); + let resp = client + .get(&url) + .timeout(std::time::Duration::from_secs(30)) + .send() + .await?; let data: Subscription = resp.json().await?;Please verify if authentication is required for this endpoint:
🏁 Script executed:
#!/bin/bash # Check if the subscription endpoint requires authentication echo "Checking API endpoint authentication requirements..." curl -I https://app.hypr.com/api/desktop/subscription echo "Status code should indicate if authentication is required (401/403 vs 200)"Length of output: 613
Enforce configurable, authenticated requests in
refreshThe current
refreshimplementation uses hardcoded URLs, unauthenticated requests, andcfg!(debug_assertions)for env detection. To lock this down:
- Read the API URL from an environment variable (e.g.
MEMBERSHIP_API_URL), defaulting to production.- Read an authentication token (e.g.
MEMBERSHIP_API_TOKEN) from env and include it viabearer_auth.- Use a shared
reqwest::Clientwith a reasonable timeout.- Remove
cfg!(debug_assertions)branching in favor of explicit configuration.Proposed diff:
async fn refresh(&self) -> Result<Subscription, crate::Error> { - let url = if cfg!(debug_assertions) { - "http://localhost:1234/api/desktop/subscription" - } else { - "https://app.hypr.com/api/desktop/subscription" - }; - let resp = reqwest::get(url).await?; + let url = std::env::var("MEMBERSHIP_API_URL") + .unwrap_or_else(|_| "https://app.hypr.com/api/desktop/subscription".to_string()); + let token = std::env::var("MEMBERSHIP_API_TOKEN") + .expect("MEMBERSHIP_API_TOKEN must be set"); + let client = reqwest::Client::new(); + let resp = client + .get(&url) + .bearer_auth(token) + .timeout(std::time::Duration::from_secs(30)) + .send() + .await?; let data: Subscription = resp.json().await?; self.membership_store() .set(crate::StoreKey::Subscription, data.clone())?; Ok(data) }Note: the sandbox couldn’t resolve
app.hypr.com, so please verify in your environment whether the/api/desktop/subscriptionendpoint requires authentication and that the token flows correctly.crates/db-admin/src/billings_ops.rs (1)
4-24: LGTM! Well-implemented query method with proper error handling.The
get_billing_by_account_idmethod correctly uses parameterized queries to prevent SQL injection and handles the optional result appropriately.apps/desktop/src/components/left-sidebar/top-area/settings-button.tsx (2)
21-21: LGTM! Clean destructuring of subscription state.Good use of the
useHyprhook to access theisProflag for conditional rendering.
55-55: Excellent conditional rendering implementation.The conditional rendering based on subscription status provides a clear visual distinction between Pro and Local modes, enhancing the user experience.
apps/app/server/src/stripe/ops.rs (2)
12-18: LGTM! Clean helper function for line items.The
get_line_itemsfunction provides a clean abstraction for creating checkout line items with proper defaults.
37-52: Good implementation of subscription with trial.The subscription creation with trial settings properly configures the trial behavior according to Stripe best practices.
plugins/membership/js/bindings.gen.ts (1)
4-16: Generated file – no manual changes recommended
tauri-spectaregenerates this file on every build. Any manual edits will be lost and could break type-safety guarantees.
Unless you intend to patch the generator, consider leaving this file untouched.apps/app/src/routeTree.gen.ts (1)
13-47: Route-tree artefact – safe to skipThis file is fully code-generated by TanStack Router. Manual edits are discouraged; instead update the actual route component (
checkout.success.tsx) and re-run generation if anything needs changing.apps/app/server/src/main.rs (1)
238-245: Static directory fallback: potential 404 loopIf
config.app_static_diris mis-configured or missingindex.html, every unmatched request becomes a 404 served by the same fallback, which can create noisy logs.Add an existence check at startup:
if !static_dir.join("index.html").exists() { panic!("app_static_dir does not contain index.html: {:?}", static_dir); }Or log a warning and continue.
apps/desktop/src/routes/app.plans.tsx (2)
100-104:ProgressiveBlurz-index sits below overlay badgesBecause the blur container uses
z-5(see previous note) and the badges usez-10, the blur might overlap incorrectly when Tailwind purges the unknown class. Confirm visual stacking once the z-index is fixed.
219-240:price_idmight not exist onSubscriptiontype
isActive={!!subscription.price_id}assumes the backend surfacesprice_id. If the generatedSubscriptiontype doesn’t include it, the code will compile only because ofanyorunknownescape hatches.
Ensure the membership plugin’s TS bindings expose the field or switch to Stripe’sitems.data[0].price.idpath.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (3)
apps/desktop/src/routes/app.settings.tsx (2)
48-49:⚠️ Potential issueRemove dead code references to removed tabs.
The code still contains references to "billing" and "lab" tabs in the UI logic, but these tabs are no longer functional since:
- The
Labcomponent is not imported- Neither "lab" nor "billing" tabs are rendered in the main content area (lines 115-121)
This creates inconsistent behavior where users might be able to navigate to these tabs but see no content.
Remove the dead code references:
case "billing": - return t`Billing`; case "team": return t`Team`;: tab.name === "ai" ? <Trans>AI</Trans> - : tab.name === "lab" - ? <Trans>Lab</Trans> : tab.name === "feedback" ? <Trans>Feedback</Trans>Also applies to: 90-91
69-97: 💡 Verification agent🧩 Analysis chain
Verify TABS configuration excludes removed functionality.
Ensure that the
TABSconfiguration no longer includes "lab" or "billing" entries to prevent users from navigating to non-functional tabs.
🏁 Script executed:
#!/bin/bash # Description: Check if TABS still contains removed lab or billing entries # Expected: TABS should not contain lab or billing entries echo "Checking TABS configuration:" rg -A 20 "export.*TABS" apps/desktop/src/components/settings/components/types.ts echo -e "\nChecking for any remaining lab/billing references in settings:" rg -i "(lab|billing)" apps/desktop/src/components/settings/ --type ts --type tsxLength of output: 735
🏁 Script executed:
#!/bin/bash echo "Searching for 'billing' references in settings components and routes:" rg -i "billing" apps/desktop/src/components/settings rg -i "billing" apps/desktop/src/routes/app.settings.tsxLength of output: 313
Remove outdated “lab” and “billing” tabs
The
TABSarray and the tab‐label switch still reference non‐functional entries:• In
apps/desktop/src/components/settings/components/types.tsexport const TABS: { name: Tab; icon: LucideIcon }[] = [ { name: "general", icon: Settings }, { name: "calendar", icon: Calendar }, { name: "ai", icon: Sparkles }, { name: "notifications", icon: Bell }, { name: "sound", icon: Volume2 }, { name: "lab", icon: FlaskConical }, // ← remove this line { name: "feedback", icon: MessageSquare }, ];• In
apps/desktop/src/routes/app.settings.tsxswitch (tab.name) { … case "billing": // ← remove this case return t`Billing`; … }Please delete both the
"lab"entry fromTABSand the"billing"case in your label/render logic to prevent users from accessing non‐functional tabs.apps/desktop/src/components/welcome-modal/index.tsx (1)
47-53:⚠️ Potential issueRemove unreachable code or implement proper conditional logic.
The hardcoded
truecondition makes theModelSelectionViewbranch unreachable and triggers a static analysis error. This appears to be incomplete implementation.Either remove the unreachable code or implement the proper condition:
- {true - ? <WelcomeView /> + <WelcomeView /> - : ( - <ModelSelectionView - onContinue={handleModelSelected} - /> - )}Or if model selection should be conditionally shown, implement the proper state logic:
+ const [showModelSelection, setShowModelSelection] = useState(false); + // ... in JSX - {true + {!showModelSelection ? <WelcomeView /> : ( <ModelSelectionView onContinue={handleModelSelected} /> )}🧰 Tools
🪛 Biome (1.9.4)
[error] 47-47: Unexpected constant condition.
(lint/correctness/noConstantCondition)
♻️ Duplicate comments (2)
apps/desktop/src/routes/app.plans.tsx (2)
23-27: Consider hardening subscription state branching to handle all possible statuses explicitlyThe current logic still uses a simple array check which silently treats any unexpected Stripe status as inactive without feedback or telemetry, as highlighted in previous reviews.
156-159: External link opens in Tauri webview instead of system browserThe anchor tag with
target="_blank"will open a new webview window in Tauri rather than the system browser, which can break auth flows and leave orphan windows, as noted in previous reviews.
🧹 Nitpick comments (2)
apps/desktop/src/components/welcome-modal/index.tsx (1)
19-21: Consider removing unused mutation if model selection is no longer needed.The
selectSTTModelmutation is defined but may be unused if theModelSelectionViewis being removed entirely. This could lead to unnecessary complexity.If model selection functionality is no longer part of the welcome flow, consider removing this unused mutation to clean up the code.
apps/desktop/src/components/welcome-modal/welcome-view.tsx (1)
23-37: Enhance authentication error handling with more specific error messages.The current error handling shows generic messages for authentication success/error, but could be more informative for debugging and user experience.
Consider enhancing error handling to provide more specific feedback:
.listen(({ payload }) => { if (payload === "success") { - message("Successfully authenticated!"); + message("Successfully authenticated! Redirecting to your account..."); return; } if (payload.error) { - message("Error occurred while authenticating!"); + message(`Authentication failed: ${payload.error}`); return; } })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
Cargo.toml(3 hunks)apps/app/server/src/env.rs(1 hunks)apps/desktop/src/components/welcome-modal/index.tsx(2 hunks)apps/desktop/src/components/welcome-modal/rating-display.tsx(1 hunks)apps/desktop/src/components/welcome-modal/welcome-view.tsx(2 hunks)apps/desktop/src/routes/app.plans.tsx(5 hunks)apps/desktop/src/routes/app.settings.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/desktop/src/components/welcome-modal/rating-display.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- Cargo.toml
- apps/app/server/src/env.rs
🧰 Additional context used
🧬 Code Graph Analysis (2)
apps/desktop/src/components/welcome-modal/welcome-view.tsx (1)
apps/desktop/src/client.ts (1)
baseUrl(10-12)
apps/desktop/src/components/welcome-modal/index.tsx (1)
apps/desktop/src/components/welcome-modal/welcome-view.tsx (1)
WelcomeView(12-93)
🪛 Biome (1.9.4)
apps/desktop/src/components/welcome-modal/index.tsx
[error] 47-47: Unexpected constant condition.
(lint/correctness/noConstantCondition)
🔇 Additional comments (8)
apps/desktop/src/routes/app.plans.tsx (5)
194-249: Well-structured component for active subscriptions without trialThe
RenderActiveWithoutTrialcomponent properly handles the display of billing information and correctly determines which plan is active based on the presence ofsubscription.price_id. The date formatting and conditional rendering logic are implemented correctly.
251-306: Clean implementation for trial subscription stateThe
RenderActiveWithTrialcomponent effectively differentiates trial subscriptions by displaying the trial end date with appropriate styling. The logic correctly handles the trial end date conversion and formatting.
308-354: Appropriate fallback for inactive subscription statesThe
RenderInactivecomponent provides a sensible default view when no active subscription is present, correctly showing the Local plan as active and the Pro plan as available for upgrade.
45-46: Enhanced PricingCard with subscription state awarenessThe addition of
isActiveandsubscriptionInfoprops to the PricingCard component enables dynamic rendering based on subscription status, which is a good architectural improvement for displaying contextual information.Also applies to: 57-58
15-32: Well-organized subscription state routing logicThe main Component function effectively routes between different render states based on subscription status and trial information. The early return pattern for null subscriptions and the conditional rendering based on trial_end provides clear separation of concerns.
apps/desktop/src/routes/app.settings.tsx (1)
8-8: LGTM! Lab component removal aligns with membership integration.The removal of the
Labcomponent import is consistent with the broader refactoring to integrate membership features elsewhere in the codebase.apps/desktop/src/components/welcome-modal/welcome-view.tsx (2)
16-46: OAuth server lifecycle management looks solid with proper cleanup.The useEffect properly manages the OAuth server lifecycle with appropriate cleanup functions. The implementation correctly handles:
- Asynchronous server startup
- Event listener registration
- Cleanup on unmount
- Error boundaries for authentication events
54-56: Verify security implications of UUID and port in URL parameters.The implementation includes sensitive information (OAuth port and client UUID) in URL parameters. Ensure this aligns with security best practices.
Please verify that exposing the OAuth server port and client UUID in URL parameters doesn't create security vulnerabilities, especially in the context of cloud plan handling and subscription management.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Removals
Documentation