feat: Dev Plans API, Stripe integration, and Code dashboard UI - #1449
Conversation
![terragon-labs[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
- Introduce Dev Plans API routes to manage subscriptions: subscribe, cancel, resume, change tier, and status. - Implement Dev Plans logic in Stripe webhook handling with custom transaction types. - Add personal organization concept for Dev Plan users, with restrictions on team and pro features. - Extend environment variables for Dev Plan Stripe price IDs and credits multiplier. - Update UI with landing page, dashboard, login, and signup to support Dev Plans. - Integrate PostHog events for Dev Plan lifecycle tracking. - Add new TypeScript types, OpenAPI schemas, and frontend utilities for Dev Plans. - Prevent deletion and team management for personal orgs tied to Dev Plans. - Maintain separate handling of Dev Plan subscriptions from Pro subscriptions. This provides a subscription system tailored for individual developers with monthly credit plans. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR implements a comprehensive Dev Plans subscription system with a dedicated Code app for developers. The implementation includes backend API endpoints for managing personal organizations and dev plan subscriptions, Stripe integration for billing, database schema changes to support dev plan features, and a complete Next.js frontend with authentication, dashboard UI, and plan management capabilities.
Changes:
- Added Dev Plans API with endpoints for personal org management, subscription, cancellation, resumption, tier changes, and status queries
- Extended organization schema with dev plan fields (isPersonal, devPlan, credits tracking, billing cycle, and cancellation status)
- Implemented Stripe checkout flow for dev plans with metadata tracking and webhook handling for subscription lifecycle events
- Created Code app (Next.js) with authentication, landing page, dashboard with plan management UI, and shared UI components
- Updated worker to prioritize dev plan credits over regular credits for cost deduction
Reviewed changes
Copilot reviewed 40 out of 50 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/api/src/routes/dev-plans.ts | New API endpoints for dev plan subscription management |
| packages/db/src/schema.ts | Added dev plan fields to organization table |
| packages/db/src/types.ts | Extended Organization type with devPlan field |
| apps/worker/src/worker.ts | Updated credit deduction logic to use dev plan credits first |
| apps/gateway/src/chat/chat.ts | Added dev plan credit checks before request processing |
| apps/code/* | New Next.js app with auth, dashboard, and plan management UI |
| apps/api/src/routes/organization.ts | Filtered personal orgs from regular UI, blocked deletion |
| apps/api/src/routes/subscriptions.ts | Blocked Pro plan for personal orgs |
| apps/api/src/routes/team.ts | Blocked team management for personal orgs |
| .env.example | Added dev plan environment variables |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } catch { | ||
| console.error(`Server API error for ${method} ${path}`); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
The error is being caught but not logged. The error details should be logged to help with debugging. Change to catch (error) and log the error object: console.error(\Server API error for ${method} ${path}`, error);`
| return config; | ||
| }, | ||
| typescript: { | ||
| ignoreBuildErrors: true, |
There was a problem hiding this comment.
Setting ignoreBuildErrors: true bypasses TypeScript type checking during builds, which can mask type errors and lead to runtime issues. This should be removed or set to false to enforce type safety.
| ignoreBuildErrors: true, | |
| ignoreBuildErrors: false, |
| ], | ||
| allow_promotion_codes: true, | ||
| success_url: `${process.env.CODE_URL || "http://localhost:3007"}/dashboard?success=true`, | ||
| cancel_url: `${process.env.CODE_URL || "http://localhost:3007"}/dashboard/plans?canceled=true`, |
There was a problem hiding this comment.
The query parameter canceled uses one 'l' spelling, while devPlanCancelled in the schema uses two 'l's. Consider using consistent spelling throughout the codebase. Use either 'canceled' (US English) or 'cancelled' (UK English) consistently.
| cancel_url: `${process.env.CODE_URL || "http://localhost:3007"}/dashboard/plans?canceled=true`, | |
| cancel_url: `${process.env.CODE_URL || "http://localhost:3007"}/dashboard/plans?cancelled=true`, |
| if (data) { | ||
| posthog.identify(data.user.id, { | ||
| email: data.user.email, | ||
| name: data.user.name, | ||
| }); | ||
| } |
There was a problem hiding this comment.
The PostHog identify call runs on every render when data changes, even if the user hasn't changed. This should be wrapped in a useEffect with proper dependencies to avoid unnecessary re-identification calls.
| {}, | ||
| { | ||
| enabled: !!user, | ||
| refetchInterval: 5000, |
There was a problem hiding this comment.
Polling every 5 seconds (5000ms) may be excessive for dev plan status updates, which typically only change during user actions (subscribe, cancel, etc.). Consider using a longer interval (e.g., 30000ms) or removing automatic polling in favor of manual refetch after mutations.
| refetchInterval: 5000, | |
| refetchInterval: 30000, |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (6)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a Dev Plans subscription feature (Lite/Pro/Max) with Stripe Checkout/webhook integration, DB schema and migration updates, API routes for plan lifecycle, frontend "code" app with dashboard/auth, shared icon components, and gateway/worker changes to account for dev-plan credits and personal-org guards. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Frontend as Code UI
participant API as Dev Plans API
participant Stripe as Stripe
participant Webhook as API Stripe Webhook
participant DB as Database
User->>Frontend: Click "Subscribe" (choose tier)
Frontend->>API: POST /dev-plans/subscribe (tier)
API->>Stripe: Create Checkout Session (price id, metadata)
Stripe-->>API: Session info
API-->>Frontend: Checkout URL
Frontend->>Stripe: Redirect user to Checkout
User->>Stripe: Complete payment
Stripe->>Webhook: checkout.session.completed
Webhook->>DB: create/ensure personal org, set devPlan, credits limit, billing cycle, store subscription id
Webhook->>DB: insert dev_plan_start transaction, generate invoice
Webhook-->>Stripe: 200 OK
User->>Frontend: View Dashboard
Frontend->>API: GET /dev-plans/status
API->>DB: fetch org dev-plan & credits
API-->>Frontend: plan status, credits usage, API key
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/api/src/routes/subscriptions.ts (2)
232-234: Arbitrary sleep is fragile.Using
setTimeoutwith a fixed 5-second delay to wait for webhook processing is unreliable. If webhooks are slow, the response may return before state is updated; if fast, it adds unnecessary latency. Consider polling for state change or returning immediately with a note that state will update asynchronously.
325-327: Same arbitrary sleep pattern.Same concern as the cancel flow - the 5-second sleep for webhook processing is unreliable.
apps/api/src/routes/organization.ts (1)
128-137: Potential API schema drift: returning fullorganizationrows while OpenAPI schema omits new fields.
getOrganizationsreturnsuo.organizationobjects directly, butorganizationSchemadoes not include fields likeisPersonal/devPlan*. Clients will receive extra fields not reflected in the contract. Either:
- explicitly select/pick only
organizationSchemafields beforec.json, or- extend
organizationSchemato include the new fields you intend to expose.apps/api/src/stripe.ts (1)
77-82: Avoidanytype for organization.The
organization: anyreturn type violates the coding guideline to avoidany. Use the proper organization type from the schema.🐛 Proposed fix
+import type { InferSelectModel } from "drizzle-orm"; + async function resolveOrganizationFromStripeEvent(eventData: { metadata?: { organizationId?: string }; customer?: string; subscription?: string; lines?: { data?: Array<{ metadata?: { organizationId?: string } }> }; -}): Promise<{ organizationId: string; organization: any } | null> { +}): Promise<{ organizationId: string; organization: InferSelectModel<typeof tables.organization> } | null> {Based on coding guidelines,
anyshould not be used unless absolutely necessary.
🤖 Fix all issues with AI agents
In `@apps/api/src/routes/dev-plans.ts`:
- Around line 441-463: The resume handler contains a blocking 3-second sleep
after calling stripe.subscriptions.update which should be removed (same fix as
the cancel handler); delete the Promise/setTimeout block following
stripe.subscriptions.update in the resume route so the handler returns
immediately after the update (references: stripe.subscriptions.update and
personalOrg.devPlanStripeSubscriptionId), leaving the existing error handling
and response intact.
- Around line 16-27: DEV_PLAN_PRICES (and its associated type DevPlanTier and
usage in getDevPlanCreditsLimit) are duplicated from stripe.ts; extract the
pricing constants into a shared module (e.g., export const DEV_PLAN_PRICES and
export type DevPlanTier from a new shared file) and update both
apps/api/src/routes/dev-plans.ts and stripe.ts to import them instead of
redefining; ensure getDevPlanCreditsLimit uses the imported DEV_PLAN_PRICES and
DevPlanTier and preserve the existing multiplier logic.
- Around line 354-376: Remove the blocking sleep after calling
stripe.subscriptions.update: delete the new Promise with setTimeout and return
success immediately once
stripe.subscriptions.update(personalOrg.devPlanStripeSubscriptionId, {
cancel_at_period_end: true }) resolves; keep the existing try/catch and error
logging (logger.error and HTTPException) but do not wait for webhooks in the
request handler—relying on the dashboard polling /status is sufficient.
- Around line 548-560: The code assumes subscription.items.data[0] exists; add a
defensive null/length check before using it (e.g., verify subscription.items and
subscription.items.data are present and length > 0), extract the first item id
into a variable (like firstItemId) and if missing return/throw a clear error or
log and skip the update; update the call in the stripe.subscriptions.update
block to use that validated firstItemId and ensure metadata update still occurs
only after the check.
In `@apps/api/src/routes/organization.ts`:
- Around line 512-518: The deletion guard currently checks
organization?.isPersonal but does not enforce owner-only deletes; update the
delete flow to verify userOrganization.role === "owner" before performing the
soft-delete. In the route handling deletion (where userOrganization and
organization?.isPersonal are checked), add a check that throws new
HTTPException(403, { message: "Only organization owners can delete
organizations." }) when userOrganization.role !== "owner", placing it prior to
the actual soft-delete logic so non-owners cannot trigger deletion.
In `@apps/api/src/stripe.ts`:
- Around line 20-32: DEV_PLAN_PRICES, DevPlanTier, and getDevPlanCreditsLimit
are duplicated and should be centralized: extract these three symbols into a
shared module (e.g., apps/api/src/utils/dev-plan-config.ts or a package-level
module), export DEV_PLAN_PRICES, DevPlanTier, and getDevPlanCreditsLimit from
that module, and update apps/api/src/stripe.ts and
apps/api/src/routes/dev-plans.ts to import them instead of redefining; ensure
the exported getDevPlanCreditsLimit still reads DEV_PLAN_CREDITS_MULTIPLIER from
process.env exactly as before to preserve behavior.
In `@apps/code/.gitignore`:
- Around line 34-35: The .gitignore pattern `.env*` is too broad and will ignore
important example files like `.env.example` that should be committed. Replace
this wildcard with a more specific pattern that ignores only actual environment
variable files, such as `.env` and `.env.local`, while explicitly excluding
example or template files. Adjust the .gitignore so it ignores `.env` files
except those meant as examples or templates.
In `@apps/code/public/favicon/site.webmanifest`:
- Around line 1-3: The manifest's "name" and "short_name" fields are empty; set
them to meaningful, user-facing strings (e.g., full app name for "name" and a
shorter label for "short_name") so the PWA shows a proper title when installed
or added to home screen; update the "name" and "short_name" keys in
site.webmanifest (the manifest JSON) with appropriate localized values if needed
and ensure they are non-empty and trimmed.
In `@apps/code/src/app/login/page.tsx`:
- Around line 77-119: The onSubmit function currently shows duplicate error
toasts because signIn.email's onError callback already displays a toast and the
subsequent if (error) block also shows one; remove the redundant error-handling
block (the if (error) {...} after the signIn.email call) so errors are handled
only in the onError callback, or alternatively remove the onError callback and
rely solely on the returned error — update either signIn.email usage or the if
(error) block accordingly (modify symbols: onSubmit, signIn.email, onError, and
the if (error) block).
In `@apps/code/src/app/page.tsx`:
- Around line 84-86: Confirm whether a free trial is actually implemented; if
not, update the CTA text in apps/code/src/app/page.tsx where the Link and Button
render (e.g., the Link href="/signup" wrapping <Button size="lg">Start Free
Trial</Button> and the other occurrences around lines 202-204) to an accurate
label such as "Get Started" or "Subscribe Now" and ensure the copy is consistent
across all CTA instances; if a free trial does exist, keep the labels but add a
brief qualifier or link to the trial terms in the same components to avoid
misleading users.
In `@apps/code/src/app/signup/page.tsx`:
- Around line 85-93: The code is sending PII (email and name) to PostHog via
posthog.identify and posthog.capture; remove ctx.data.user.email,
ctx.data.user.name, values.email and values.name from those calls and only
identify the user by their unique ID (e.g., posthog.identify(ctx.data.user.id)
with no PII properties) and send non-PII event properties like selectedPlan; if
a user-readable name is required for product features, persist it server-side or
use a hashed/consented value instead, and ensure any intentional PII collection
is documented in the privacy policy.
- Line 12: Update the Zod import to use the project convention by replacing the
non-standard import specifier "zod/v3" with "zod" (i.e., import { z } from
"zod";) in the file that defines the signup page (where the symbol z is
imported); ensure any other imports in this file that reference "zod/v3" are
similarly updated to "zod" for consistency.
In `@apps/code/src/components/ui/form.tsx`:
- Around line 47-57: The null-check for fieldContext is after its usage in
useFormField, causing a crash when fieldContext is falsy; move the guard so the
check for fieldContext (the React.useContext(FormFieldContext) result) occurs
before any access to fieldContext.name or before calling
useFormState/getFieldState with fieldContext.name; ensure the early throw in
useFormField runs immediately after obtaining fieldContext and before computing
formState or calling getFieldState so all uses of fieldContext.name are safe.
In `@apps/code/src/hooks/useUser.ts`:
- Around line 38-50: The useEffect block starting with useEffect(() => { if
(!data?.user || isLoading) { return; } ... }, [data?.user, isLoading, router,
pathname]) is dead code because it contains no side-effects or logic after its
guards; remove this entire useEffect to clean up the hook (delete the effect
that references data?.user, isLoading, router, and pathname). If redirect or
auth-handling behavior was intended, re-implement it explicitly in a new effect
or function instead of leaving this no-op effect.
In `@apps/code/src/lib/server-api.ts`:
- Around line 49-53: The fetchServerData function uses options?: any which
violates the no-any guideline; replace any with the proper OpenAPI request
options type (e.g., the request/options type exported by your openapi-fetch
client or generator) and import that type into this module, then update the
signature of fetchServerData(method: "GET" | "POST" | "PUT" | "DELETE" |
"PATCH", path: keyof paths, options?: OpenApiRequestOptions): Promise<T | null>;
if needed create method-specific overloads or a union of option types for
different HTTP verbs to tighten typing further.
♻️ Duplicate comments (5)
apps/code/next.config.ts (1)
18-20: RemoveignoreBuildErrors: trueto enforce type safety.Setting
ignoreBuildErrors: truebypasses TypeScript type checking during builds, allowing type errors to slip into production and potentially cause runtime failures. This undermines the value of TypeScript in the codebase.Proposed fix
typescript: { - ignoreBuildErrors: true, + ignoreBuildErrors: false, },apps/code/src/lib/server-api.ts (1)
85-88: Log the error object for debugging.The caught error is discarded, making it harder to diagnose issues. Capture and log the error details.
Proposed fix
- } catch { - console.error(`Server API error for ${method} ${path}`); + } catch (error) { + console.error(`Server API error for ${method} ${path}`, error); return null; }apps/code/src/hooks/useUser.ts (1)
31-36: Move PostHog identify call into auseEffect.The
posthog.identifycall runs on every render whendatachanges. This should be wrapped in auseEffectto avoid unnecessary re-identification calls and to follow React's rules for side effects.Proposed fix
+ useEffect(() => { + if (data?.user) { + posthog.identify(data.user.id, { + email: data.user.email, + name: data.user.name, + }); + } + }, [data?.user?.id, data?.user?.email, data?.user?.name, posthog]); + - if (data) { - posthog.identify(data.user.id, { - email: data.user.email, - name: data.user.name, - }); - }apps/code/src/app/dashboard/page.tsx (1)
62-70: Polling interval is excessive for dev plan status.Dev plan status changes only occur during explicit user actions (subscribe, cancel, resume). This comment duplicates a prior review.
apps/api/src/routes/dev-plans.ts (1)
263-263: Spelling inconsistency already flagged.The
canceledvscancelledspelling inconsistency was noted in a prior review.
🧹 Nitpick comments (17)
apps/code/package.json (2)
35-35: Movebabel-plugin-react-compilerto devDependencies.This is a build-time compiler plugin and should not be in runtime dependencies. Placing it in
dependenciesunnecessarily includes it in production bundles.♻️ Suggested fix
Move to
devDependencies:"dependencies": { ... - "babel-plugin-react-compiler": "1.0.0", ... }, "devDependencies": { + "babel-plugin-react-compiler": "1.0.0", ... }
18-53: Consider standardizing version pinning strategy.The dependencies mix exact versions (e.g.,
"next": "16.1.1") with caret ranges (e.g.,"clsx": "^2.1.1"). For reproducible builds, consider using a consistent pinning strategy across all dependencies, or rely on a lockfile to enforce determinism..env.unified.example (1)
18-25: Consider alphabetical ordering for consistency.
CODE_URLcould be placed beforeDOCS_URLandPLAYGROUND_URLto maintain alphabetical ordering.📋 Suggested reordering
UI_URL=http://localhost:3002 -PLAYGROUND_URL=http://localhost:3003 CODE_URL=http://localhost:3007 DOCS_URL=http://localhost:3005 +PLAYGROUND_URL=http://localhost:3003 ADMIN_URL=http://localhost:3006apps/gateway/src/lib/rate-limit.spec.ts (1)
81-114: Consider extracting a mock organization factory to reduce duplication.The mock organization object is repeated 6 times across test cases with only minor variations (primarily
creditsvalue). A factory function would reduce boilerplate and make test maintenance easier.♻️ Suggested factory function
function createMockOrganization(overrides: Partial<typeof baseOrg> = {}) { return { id: "org-1", createdAt: new Date(), updatedAt: new Date(), name: "Test Org", billingEmail: "test@example.com", billingCompany: null, billingAddress: null, billingTaxId: null, billingNotes: null, stripeCustomerId: null, stripeSubscriptionId: null, credits: "0", autoTopUpEnabled: false, autoTopUpThreshold: "10", autoTopUpAmount: "10", plan: "free" as const, planExpiresAt: null, subscriptionCancelled: false, trialStartDate: null, trialEndDate: null, isTrialActive: false, retentionLevel: "retain" as const, status: "active" as const, referralEarnings: "0", isPersonal: false, devPlan: "none" as const, devPlanCreditsUsed: "0", devPlanCreditsLimit: "0", devPlanBillingCycleStart: null, devPlanStripeSubscriptionId: null, devPlanCancelled: false, devPlanExpiresAt: null, ...overrides, }; } // Usage in tests: vi.mocked(cdb.query.organization.findFirst).mockResolvedValue( createMockOrganization({ credits: "10.50" }) );apps/code/src/hooks/useUser.ts (1)
69-77: Remove redundantoptionsfrom dependency array.The dependency array includes
options?.redirectTo,options?.redirectWhen, andoptions. The fulloptionsobject is redundant since its properties are already listed individually, and including it may cause unnecessary re-runs if the object reference changes.Proposed fix
}, [ data?.user, isLoading, error, router, options?.redirectTo, options?.redirectWhen, - options, ]);apps/code/src/app/signup/page.tsx (3)
60-62: Guard against PostHog not being initialized.When
posthogKeyis not configured,PostHogProvideris not rendered, andusePostHog()may return an uninitialized instance. Callingposthog.capture()in this case could fail silently or throw. Add a guard check.Proposed fix
useEffect(() => { - posthog.capture("page_viewed_signup", { plan: selectedPlan }); + posthog?.capture("page_viewed_signup", { plan: selectedPlan }); }, [posthog, selectedPlan]);
100-118: Duplicate error handling.The
onErrorcallback (lines 100-107) and theif (error)check (lines 111-118) both display error toasts. IfsignUp.emailtriggersonErrorand also returns an error object, the user may see duplicate toasts. Consider consolidating error handling.Proposed fix - remove duplicate
const { error } = await signUp.email( { name: values.name, email: values.email, password: values.password, }, { onSuccess: (ctx) => { queryClient.clear(); - posthog.identify(ctx.data.user.id, { - email: ctx.data.user.email, - name: ctx.data.user.name, - }); - posthog.capture("user_signed_up", { - email: values.email, - name: values.name, + posthog?.identify(ctx.data.user.id); + posthog?.capture("user_signed_up", { plan: selectedPlan, }); toast.success("Account created", { description: "Please check your email to verify your account before signing in.", }); router.push(returnUrl); }, - onError: (ctx) => { - toast.error(ctx.error.message || "Failed to sign up", { - style: { - backgroundColor: "var(--destructive)", - color: "var(--destructive-foreground)", - }, - }); - }, }, ); if (error) { toast.error(error.message || "Failed to sign up", { style: { backgroundColor: "var(--destructive)", color: "var(--destructive-foreground)", }, }); }
35-43: Login link does not preserve returnUrl.When users click "Already have an account? Sign in", the
returnUrlandplanquery params are lost. Consider forwarding them to maintain the intended flow.Proposed fix
- <p className="px-8 text-center text-sm text-muted-foreground"> - <Link - href="/login" - className="hover:text-primary underline underline-offset-4" - > - Already have an account? Sign in - </Link> - </p> + <p className="px-8 text-center text-sm text-muted-foreground"> + <Link + href={`/login?returnUrl=${encodeURIComponent(returnUrl)}${selectedPlan ? `&plan=${selectedPlan}` : ""}`} + className="hover:text-primary underline underline-offset-4" + > + Already have an account? Sign in + </Link> + </p>Also applies to: 194-199
apps/code/src/components/providers.tsx (1)
36-40: PostHog options object recreated on every render.The
posthogOptionsobject is created fresh on each render, which could cause unnecessary re-renders ofPostHogProvider. Consider memoizing it.Proposed fix
- const posthogOptions: Partial<PostHogConfig> | undefined = { - api_host: config.posthogHost, - capture_pageview: "history_change", - autocapture: true, - }; + const posthogOptions = useMemo<Partial<PostHogConfig>>( + () => ({ + api_host: config.posthogHost, + capture_pageview: "history_change", + autocapture: true, + }), + [config.posthogHost], + );apps/code/src/components/ui/form.tsx (1)
29-31: Empty object context defaults may cause subtle runtime issues.Using
{} as FormFieldContextValueand{} as FormItemContextValueas default values means accessing properties likenameoridoutside a provider will returnundefinedrather than throwing. This could lead to hard-to-debug issues. Consider usingnullwith proper type guards.Also applies to: 74-76
apps/api/src/routes/team.ts (1)
319-325: Consider extracting the personal org guard to reduce repetition.The same guard logic appears in
addMember,updateMember, andremoveMember. You could extract this into a reusable helper.♻️ Optional: Extract helper function
function assertNotPersonalOrg(organization: { isPersonal?: boolean } | null) { if (organization?.isPersonal) { throw new HTTPException(403, { message: "Team management is not available for personal organizations. Please create a regular organization to invite team members.", }); } }Then use
assertNotPersonalOrg(userOrganization.organization)in each handler.Also applies to: 470-476
apps/code/src/app/page.tsx (1)
6-49: Plan data should stay in sync with backend pricing configuration.The hardcoded plan prices and credits match the backend configuration (
DEV_PLAN_PRICESandDEV_PLAN_CREDITS_MULTIPLIER=3). However, if backend pricing changes, this frontend data could drift.Consider fetching plan data from an API endpoint or sharing a constants file to ensure consistency.
apps/api/src/routes/organization.ts (1)
64-91: Keep transaction type enums in sync with the DB/source-of-truth.You’re extending
transactionSchema.typewithdev_plan_*values; consider importing a shared enum (or generating from DB schema/types) to prevent future drift between API OpenAPI schema and persisted values.apps/code/src/app/layout.tsx (1)
22-47: Avoid hardcoding the canonical site URL inmetadataBase(staging/preview correctness).Consider deriving
metadataBase(andopenGraph.url) from config/env so previews don’t publish production canonical URLs.apps/worker/src/worker.ts (1)
653-705: Dev plan eligibility + N+1 query risk inside the transaction.
- Eligibility: this logic only checks
org.devPlan !== "none". IfdevPlanExpiresAt/devPlanCancelledcan be out-of-sync, the worker may incorrectly spend dev plan credits. Either enforce the invariant at write-time (best) or gate here (e.g.,devPlanExpiresAt > now && !devPlanCancelled).- Perf:
await tx.query.organization.findFirst(...)in a loop can become N+1. Consider preloading all org rows fororgCosts.keys()viainArrayonce.apps/gateway/src/chat/chat.ts (1)
1788-1809: Stabilize renewal-date formatting and de-duplicate the “available credits” logic.
new Date(...).toLocaleDateString()is environment-dependent; prefer a stable format (e.g. ISO date) for error messages.- The
regularCredits + (devPlanLimit - devPlanUsed)logic is repeated in 3 places; consider a small helper to compute{ totalAvailableCredits, devPlanCreditsRemaining, renewalDate }once and reuse.- If
devPlanExpiresAt/devPlanCancelledcan be out-of-sync, consider gating dev plan credits on validity here (or ensure invariants upstream).Also applies to: 1895-1917, 1946-1959
apps/code/src/app/dashboard/page.tsx (1)
23-46: Hardcoded plan credits may drift from server configuration.The credits values (87, 237, 447) are derived from
price * 3(the default multiplier), but ifDEV_PLAN_CREDITS_MULTIPLIERis changed server-side, this UI will display incorrect values. Consider fetching plan details from an API endpoint or deriving credits from a shared constant.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (10)
apps/code/public/favicon/android-chrome-192x192.pngis excluded by!**/*.pngapps/code/public/favicon/android-chrome-512x512.pngis excluded by!**/*.pngapps/code/public/favicon/apple-touch-icon.pngis excluded by!**/*.pngapps/code/public/favicon/favicon-16x16.pngis excluded by!**/*.pngapps/code/public/favicon/favicon-32x32.pngis excluded by!**/*.pngapps/code/public/favicon/favicon.icois excluded by!**/*.icoapps/code/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsapps/playground/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsapps/ui/src/lib/api/v1.d.tsis excluded by!**/v1.d.tspnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (40)
.env.example.env.unified.exampleapps/api/src/routes/dev-plans.tsapps/api/src/routes/index.tsapps/api/src/routes/organization.tsapps/api/src/routes/subscriptions.tsapps/api/src/routes/team.tsapps/api/src/stripe.tsapps/code/.gitignoreapps/code/components.jsonapps/code/eslint.config.mjsapps/code/next.config.tsapps/code/package.jsonapps/code/postcss.config.mjsapps/code/public/favicon/site.webmanifestapps/code/src/app/dashboard/page.tsxapps/code/src/app/globals.cssapps/code/src/app/layout.tsxapps/code/src/app/login/page.tsxapps/code/src/app/page.tsxapps/code/src/app/signup/page.tsxapps/code/src/components/providers.tsxapps/code/src/components/ui/button.tsxapps/code/src/components/ui/form.tsxapps/code/src/components/ui/input.tsxapps/code/src/components/ui/label.tsxapps/code/src/components/ui/sonner.tsxapps/code/src/hooks/useUser.tsapps/code/src/lib/auth-client.tsapps/code/src/lib/config-server.tsapps/code/src/lib/config.tsxapps/code/src/lib/fetch-client.tsapps/code/src/lib/server-api.tsapps/code/src/lib/utils.tsapps/code/tsconfig.jsonapps/gateway/src/chat/chat.tsapps/gateway/src/lib/rate-limit.spec.tsapps/worker/src/worker.tspackages/db/src/schema.tspackages/db/src/types.ts
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never useanyoras anyunless absolutely necessary in TypeScript code
For database reads: Usedb().query.<table>.findMany()ordb().query.<table>.findFirst()
Files:
apps/code/src/components/ui/input.tsxapps/code/src/components/ui/sonner.tsxapps/code/src/components/providers.tsxapps/code/src/lib/fetch-client.tsapps/code/src/lib/auth-client.tsapps/gateway/src/lib/rate-limit.spec.tsapps/code/src/lib/utils.tsapps/code/src/hooks/useUser.tsapps/code/src/app/layout.tsxapps/code/src/lib/config-server.tsapps/code/src/components/ui/label.tsxapps/code/next.config.tsapps/gateway/src/chat/chat.tsapps/api/src/routes/team.tsapps/code/src/app/dashboard/page.tsxapps/worker/src/worker.tspackages/db/src/schema.tspackages/db/src/types.tsapps/api/src/stripe.tsapps/api/src/routes/dev-plans.tsapps/code/src/lib/config.tsxapps/api/src/routes/subscriptions.tsapps/code/src/app/signup/page.tsxapps/code/src/lib/server-api.tsapps/code/src/app/page.tsxapps/code/src/components/ui/button.tsxapps/code/src/app/login/page.tsxapps/api/src/routes/organization.tsapps/api/src/routes/index.tsapps/code/src/components/ui/form.tsx
**/*.{ts,tsx,js,jsx,json,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use tabs for indentation
Files:
apps/code/src/components/ui/input.tsxapps/code/src/components/ui/sonner.tsxapps/code/src/components/providers.tsxapps/code/src/lib/fetch-client.tsapps/code/src/lib/auth-client.tsapps/gateway/src/lib/rate-limit.spec.tsapps/code/src/lib/utils.tsapps/code/src/hooks/useUser.tsapps/code/package.jsonapps/code/src/app/layout.tsxapps/code/src/lib/config-server.tsapps/code/tsconfig.jsonapps/code/src/components/ui/label.tsxapps/code/next.config.tsapps/gateway/src/chat/chat.tsapps/api/src/routes/team.tsapps/code/src/app/dashboard/page.tsxapps/worker/src/worker.tspackages/db/src/schema.tspackages/db/src/types.tsapps/api/src/stripe.tsapps/api/src/routes/dev-plans.tsapps/code/src/lib/config.tsxapps/api/src/routes/subscriptions.tsapps/code/components.jsonapps/code/src/app/signup/page.tsxapps/code/src/lib/server-api.tsapps/code/src/app/page.tsxapps/code/src/components/ui/button.tsxapps/code/src/app/login/page.tsxapps/api/src/routes/organization.tsapps/api/src/routes/index.tsapps/code/src/components/ui/form.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Always use top-levelimport, never use require or dynamic imports
No unnecessary code comments
Files:
apps/code/src/components/ui/input.tsxapps/code/src/components/ui/sonner.tsxapps/code/src/components/providers.tsxapps/code/src/lib/fetch-client.tsapps/code/src/lib/auth-client.tsapps/gateway/src/lib/rate-limit.spec.tsapps/code/src/lib/utils.tsapps/code/src/hooks/useUser.tsapps/code/src/app/layout.tsxapps/code/src/lib/config-server.tsapps/code/src/components/ui/label.tsxapps/code/next.config.tsapps/gateway/src/chat/chat.tsapps/api/src/routes/team.tsapps/code/src/app/dashboard/page.tsxapps/worker/src/worker.tspackages/db/src/schema.tspackages/db/src/types.tsapps/api/src/stripe.tsapps/api/src/routes/dev-plans.tsapps/code/src/lib/config.tsxapps/api/src/routes/subscriptions.tsapps/code/src/app/signup/page.tsxapps/code/src/lib/server-api.tsapps/code/src/app/page.tsxapps/code/src/components/ui/button.tsxapps/code/src/app/login/page.tsxapps/api/src/routes/organization.tsapps/api/src/routes/index.tsapps/code/src/components/ui/form.tsx
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Always use top-level
import, never use require or dynamic imports
Files:
apps/code/src/components/ui/input.tsxapps/code/src/components/ui/sonner.tsxapps/code/src/components/providers.tsxapps/code/src/lib/fetch-client.tsapps/code/src/lib/auth-client.tsapps/gateway/src/lib/rate-limit.spec.tsapps/code/src/lib/utils.tsapps/code/src/hooks/useUser.tsapps/code/src/app/layout.tsxapps/code/src/lib/config-server.tsapps/code/src/components/ui/label.tsxapps/code/next.config.tsapps/gateway/src/chat/chat.tsapps/api/src/routes/team.tsapps/code/src/app/dashboard/page.tsxapps/worker/src/worker.tspackages/db/src/schema.tspackages/db/src/types.tsapps/api/src/stripe.tsapps/api/src/routes/dev-plans.tsapps/code/src/lib/config.tsxapps/api/src/routes/subscriptions.tsapps/code/src/app/signup/page.tsxapps/code/src/lib/server-api.tsapps/code/src/app/page.tsxapps/code/src/components/ui/button.tsxapps/code/src/app/login/page.tsxapps/api/src/routes/organization.tsapps/api/src/routes/index.tsapps/code/src/components/ui/form.tsx
**/*.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Unit tests should use
*.spec.tsfile naming convention
Files:
apps/gateway/src/lib/rate-limit.spec.ts
apps/{gateway,api}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Hono framework with Zod validation and OpenAPI documentation for backend APIs
Files:
apps/gateway/src/lib/rate-limit.spec.tsapps/gateway/src/chat/chat.tsapps/api/src/routes/team.tsapps/api/src/stripe.tsapps/api/src/routes/dev-plans.tsapps/api/src/routes/subscriptions.tsapps/api/src/routes/organization.tsapps/api/src/routes/index.ts
{apps/api,apps/gateway,packages/db}/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
{apps/api,apps/gateway,packages/db}/**/*.ts: Use Drizzle ORM with latest object syntax for database operations
For database reads: Usedb().query.<table>.findMany()ordb().query.<table>.findFirst()
Files:
apps/gateway/src/lib/rate-limit.spec.tsapps/gateway/src/chat/chat.tsapps/api/src/routes/team.tspackages/db/src/schema.tspackages/db/src/types.tsapps/api/src/stripe.tsapps/api/src/routes/dev-plans.tsapps/api/src/routes/subscriptions.tsapps/api/src/routes/organization.tsapps/api/src/routes/index.ts
apps/{gateway,api}/src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
apps/{gateway,api}/src/**/*.ts: Use Hono for backend framework in Gateway and API services
Use Zod schemas for validation in Hono services
Files:
apps/gateway/src/lib/rate-limit.spec.tsapps/gateway/src/chat/chat.tsapps/api/src/routes/team.tsapps/api/src/stripe.tsapps/api/src/routes/dev-plans.tsapps/api/src/routes/subscriptions.tsapps/api/src/routes/organization.tsapps/api/src/routes/index.ts
packages/db/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Drizzle ORM with latest object syntax for database operations
Files:
packages/db/src/schema.tspackages/db/src/types.ts
🧠 Learnings (6)
📚 Learning: 2025-12-03T12:42:14.219Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T12:42:14.219Z
Learning: Applies to apps/{ui,playground}/**/*.{ts,tsx} : Use Next.js App Router with React Server Components for frontend development
Applied to files:
apps/code/src/hooks/useUser.tsapps/code/package.jsonapps/code/src/app/layout.tsxapps/code/src/app/dashboard/page.tsxapps/code/src/lib/config.tsxapps/code/src/lib/server-api.tsapps/code/src/app/page.tsxapps/code/src/app/login/page.tsxapps/api/src/routes/index.ts
📚 Learning: 2025-12-03T12:42:26.162Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T12:42:26.162Z
Learning: Applies to apps/{ui,playground,docs}/**/*.{ts,tsx} : Use `next/link` for links and `next/navigation`'s router for programmatic navigation
Applied to files:
apps/code/src/app/layout.tsxapps/code/src/app/dashboard/page.tsxapps/code/src/app/page.tsxapps/code/src/app/login/page.tsxapps/api/src/routes/index.ts
📚 Learning: 2025-12-03T12:42:14.219Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T12:42:14.219Z
Learning: Applies to apps/{ui,playground}/**/*.{tsx} : Use `next/link` for links and `next/navigation`'s router for programmatic navigation
Applied to files:
apps/code/src/app/layout.tsxapps/code/src/app/page.tsxapps/code/src/app/login/page.tsx
📚 Learning: 2025-12-03T12:42:14.219Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T12:42:14.219Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : No unnecessary code comments
Applied to files:
apps/code/tsconfig.json
📚 Learning: 2025-12-03T12:42:14.219Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T12:42:14.219Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Always use top-level `import`, never use require or dynamic imports
Applied to files:
apps/code/tsconfig.json
📚 Learning: 2025-12-03T12:42:14.219Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T12:42:14.219Z
Learning: Applies to **/*.{ts,tsx} : Never use `any` or `as any` unless absolutely necessary in TypeScript code
Applied to files:
apps/code/next.config.ts
🧬 Code graph analysis (18)
apps/code/src/components/ui/input.tsx (1)
apps/code/src/lib/utils.ts (1)
cn(4-6)
apps/code/src/lib/fetch-client.ts (1)
apps/code/src/lib/config.tsx (1)
useAppConfig(25-31)
apps/code/src/lib/auth-client.ts (1)
apps/code/src/lib/config.tsx (1)
useAppConfig(25-31)
apps/code/src/lib/utils.ts (1)
apps/docs/lib/cn.ts (1)
twMerge(1-1)
apps/code/src/hooks/useUser.ts (3)
apps/api/src/posthog.ts (1)
posthog(3-6)apps/code/src/lib/fetch-client.ts (1)
useApi(20-26)packages/logger/src/index.ts (1)
error(153-160)
apps/code/src/components/ui/label.tsx (2)
apps/code/src/lib/utils.ts (1)
cn(4-6)apps/admin/src/components/ui/label.tsx (1)
Label(8-22)
apps/gateway/src/chat/chat.ts (2)
packages/db/src/schema.ts (1)
organization(109-160)packages/models/src/models.ts (1)
ModelDefinition(181-241)
apps/code/src/app/dashboard/page.tsx (7)
apps/api/src/posthog.ts (1)
posthog(3-6)apps/code/src/lib/auth-client.ts (1)
useAuth(18-31)apps/code/src/lib/fetch-client.ts (1)
useApi(20-26)apps/code/src/hooks/useUser.ts (1)
useUser(14-85)packages/db/src/schema.ts (1)
user(41-53)apps/ui/src/lib/components/use-toast.ts (1)
toast(194-194)apps/code/src/components/ui/button.tsx (1)
Button(60-60)
apps/worker/src/worker.ts (1)
packages/db/src/schema.ts (1)
organization(109-160)
apps/api/src/stripe.ts (3)
packages/db/src/schema.ts (2)
organization(109-160)transaction(189-233)apps/api/src/utils/invoice.ts (1)
generateAndEmailInvoice(192-283)apps/api/src/utils/email.ts (2)
sendTransactionalEmail(38-105)generateSubscriptionCancelledEmailHtml(251-330)
apps/api/src/routes/dev-plans.ts (4)
packages/db/src/db.ts (1)
db(21-25)packages/db/src/index.ts (1)
tables(14-16)apps/api/src/stripe.ts (1)
ensureStripeCustomer(34-71)apps/api/src/routes/payments.ts (1)
stripe(13-18)
apps/api/src/routes/subscriptions.ts (1)
packages/db/src/schema.ts (1)
organization(109-160)
apps/code/src/lib/server-api.ts (1)
apps/code/src/lib/config-server.ts (1)
getConfig(12-24)
apps/code/src/app/page.tsx (1)
apps/code/src/components/ui/button.tsx (1)
Button(60-60)
apps/code/src/components/ui/button.tsx (1)
apps/code/src/lib/utils.ts (1)
cn(4-6)
apps/code/src/app/login/page.tsx (3)
apps/api/src/posthog.ts (1)
posthog(3-6)apps/code/src/lib/auth-client.ts (1)
useAuth(18-31)apps/code/src/hooks/useUser.ts (1)
useUser(14-85)
apps/api/src/routes/index.ts (1)
apps/api/src/routes/dev-plans.ts (1)
devPlans(14-14)
apps/code/src/components/ui/form.tsx (2)
apps/code/src/lib/utils.ts (1)
cn(4-6)apps/code/src/components/ui/label.tsx (1)
Label(24-24)
🪛 dotenv-linter (4.0.0)
.env.unified.example
[warning] 20-20: [UnorderedKey] The CODE_URL key should go before the PLAYGROUND_URL key
(UnorderedKey)
[warning] 133-133: [UnorderedKey] The STRIPE_DEV_PLAN_MAX_PRICE_ID key should go before the STRIPE_DEV_PLAN_PRO_PRICE_ID key
(UnorderedKey)
.env.example
[warning] 59-59: [UnorderedKey] The CODE_URL key should go before the PLAYGROUND_URL key
(UnorderedKey)
[warning] 199-199: [UnorderedKey] The STRIPE_DEV_PLAN_MAX_PRICE_ID key should go before the STRIPE_DEV_PLAN_PRO_PRICE_ID key
(UnorderedKey)
🔇 Additional comments (46)
apps/code/tsconfig.json (1)
1-35: LGTM!The TypeScript configuration is well-structured for a Next.js application:
- Appropriate compiler options for Next.js (
moduleResolution: "bundler",jsx: "preserve",strict: true)- Correctly includes Next.js generated type definitions (
.next/types/**/*.ts)- Path alias
@/*properly configured for cleaner imports- Incremental builds enabled for faster compilation
apps/code/eslint.config.mjs (1)
1-17: LGTM!The ESLint flat config structure is correct, and the composition of base + React configs is appropriate. Disabling
react/no-unescaped-entitiesis reasonable to reduce noise from apostrophes in JSX text. The ignore pattern for**/v1.d.tscorrectly excludes auto-generated type definitions.One optional consideration:
no-console: "off"allows console statements globally. If you'd prefer to catch accidental debug logs in production while still permitting warnings/errors, you could use"no-console": ["warn", { "allow": ["warn", "error"] }]instead—but this is fine as-is for a developer-facing dashboard.apps/code/public/favicon/site.webmanifest (1)
4-15: Icon files exist and are correctly referenced.The manifest references to
android-chrome-192x192.pngandandroid-chrome-512x512.pngare valid—both files are present inapps/code/public/favicon/.apps/code/package.json (3)
1-9: LGTM!Package metadata is appropriate for a private monorepo application. The
filesarray correctly scopes the publishable artifacts.
10-17: LGTM!Scripts are well-structured. The build script's defensive
test -d public &&check before copying is a good pattern.
67-67: Remove unusedzoddependency from devDependencies.
zodis listed indevDependenciesbut is not used anywhere in the codebase and is not installed in the dependency tree. If it's not required for development or type generation, remove it to keep dependencies clean.Likely an incorrect or invalid review comment.
apps/code/.gitignore (2)
1-42: Overall structure looks good.The gitignore patterns are well-organized and cover all the standard Next.js, TypeScript, and Node.js artifacts. The Yarn 2+ configuration with selective allowances for patches, plugins, releases, and versions follows best practices.
16-19: No action needed. The/.next-dev/directory is intentionally configured innext.config.tsto serve as the build output directory during development mode (distDir: process.env.NODE_ENV === "development" ? ".next-dev" : ".next"), and the corresponding.gitignoreentry is correct.Likely an incorrect or invalid review comment.
apps/code/src/lib/utils.ts (1)
1-6: LGTM!Clean implementation of the standard
cnutility for composing Tailwind classes. The combination ofclsxfor conditional class handling andtwMergefor deduplication is the recommended pattern for shadcn/ui components.apps/code/src/components/ui/label.tsx (1)
1-24: LGTM!Well-structured Label component that aligns with the existing pattern in
apps/admin/src/components/ui/label.tsx. The use ofdata-slot="label", consistent disabled state styling, and proper TypeScript typing are all good practices..env.unified.example (1)
130-136: Env vars are well documented.The dev plan configuration is clearly explained. Minor note: alphabetically,
STRIPE_DEV_PLAN_MAX_PRICE_IDwould come beforeSTRIPE_DEV_PLAN_PRO_PRICE_ID, but this follows the logical tier ordering (lite → pro → max) which is arguably more intuitive.apps/code/components.json (1)
1-22: LGTM!Well-configured shadcn/ui setup. The empty
tailwind.configis appropriate for Tailwind v4's CSS-first configuration approach. Aliases are properly mapped and consistent with the app structure.apps/gateway/src/lib/rate-limit.spec.ts (1)
106-114: Remove this suggestion -devPlandoes not affect rate limiting behavior.The rate limiting logic depends only on whether the model is free, the organization's credit balance, and the model's
rateLimitKind. ThedevPlanfield is not referenced in the implementation and therefore does not impact rate limits.Likely an incorrect or invalid review comment.
apps/code/src/components/ui/input.tsx (1)
1-21: LGTM!Clean implementation following React 19 patterns where
refis a regular prop viaComponentProps. The Tailwind styling is comprehensive with proper state handling (focus-visible, aria-invalid, disabled) and dark mode support.apps/code/src/components/ui/sonner.tsx (1)
1-27: LGTM!The implementation correctly integrates Sonner with next-themes and follows the established pattern from
apps/ui/src/lib/components/sonner.tsx. Theas React.CSSPropertiescast is necessary for CSS custom properties.apps/code/next.config.ts (1)
5-17: Configuration looks reasonable for monorepo standalone deployment.A minor note:
productionBrowserSourceMaps: trueexposes source code to end users in production, which may have IP or security implications. Verify this is intentional for the Code app's use case.apps/code/src/lib/server-api.ts (2)
8-27: LGTM!The cookie forwarding logic correctly handles both secure and non-secure session tokens, prioritizing the
__Secure-prefixed cookie when available.
29-47: LGTM!The path type helpers provide good compile-time safety by constraining paths to those supporting each HTTP method. The use of
anyhere is at the type level for property existence checks, which is acceptable.apps/code/src/lib/config-server.ts (1)
1-24: LGTM!Well-structured configuration module with appropriate typing and sensible defaults for local development. The fallback of
apiBackendUrltoapiUrlis a good pattern for environments where they're the same.apps/code/src/lib/config.tsx (1)
1-31: LGTM!Clean implementation of the AppConfig context pattern. The provider and hook follow React best practices with proper null safety check and a descriptive error message.
apps/code/src/lib/fetch-client.ts (1)
1-26: LGTM!Well-structured hooks with proper memoization. The
credentials: "include"setting is appropriate for cookie-based authentication across origins.apps/code/src/lib/auth-client.ts (1)
1-31: LGTM!Good implementation with proper memoization. The pattern of exposing
useSessionthrough the returned object follows the better-auth library's intended usage pattern.apps/code/src/components/ui/button.tsx (1)
1-60: LGTM!Well-structured Button component following shadcn/ui conventions. The CVA configuration provides comprehensive variants and sizes, and the
asChildpattern with Radix Slot is correctly implemented for composition flexibility.apps/code/src/hooks/useUser.ts (1)
87-97: LGTM!The
useUpdateUserhook is well-structured with proper cache invalidation on success.apps/code/src/components/providers.tsx (1)
42-68: LGTM!The provider composition is well-structured with conditional PostHog rendering and dev-only React Query devtools. The nesting order is appropriate.
apps/code/src/components/ui/form.tsx (2)
110-127: LGTM!FormControl properly sets up ARIA attributes for accessibility, binding
aria-describedbyandaria-invalidbased on form state.
142-160: LGTM!FormMessage correctly handles both error messages and custom children, with proper null return when no content exists.
apps/api/src/routes/subscriptions.ts (1)
88-94: LGTM!The guard correctly blocks Pro subscriptions for personal organizations, directing users to the appropriate Dev Plans flow. The placement before existing checks is appropriate.
apps/api/src/routes/index.ts (1)
9-9: LGTM!The devPlans route is properly imported and registered following existing patterns. The placement after subscriptions is logical given the related functionality.
Also applies to: 58-58
apps/code/postcss.config.mjs (1)
1-5: LGTM!Clean PostCSS configuration for Tailwind CSS v4. The
@tailwindcss/postcssplugin is the correct approach for v4's CSS-first configuration..env.example (1)
196-202: LGTM!Well-documented environment variables for Dev Plans. The grouping keeps related Stripe price IDs together, and the credits multiplier default of 3 aligns with the PR description (credits = price × 3).
apps/api/src/routes/team.ts (1)
172-178: LGTM!The guard correctly blocks team management for personal organizations before checking other permissions. The error message helpfully guides users to create a regular organization.
apps/code/src/app/globals.css (3)
1-5: LGTM!Correct Tailwind CSS v4 setup with
@import "tailwindcss"and the@custom-variant darkdirective for class-based dark mode toggling.
6-44: LGTM!The
@theme inlineblock correctly maps CSS custom properties to Tailwind's design token system, enabling utilities likebg-background,text-foreground, etc. The radius calculations provide consistent scaling.
46-113: LGTM!Well-structured theme using
oklch()color space for better perceptual uniformity. Both light (:root) and dark (.dark) themes are comprehensive with consistent token coverage.apps/code/src/app/page.tsx (1)
51-222: LGTM on overall structure.Clean landing page implementation using Next.js App Router conventions. The component correctly uses
next/linkfor navigation (per project guidelines), imports the sharedButtoncomponent, and renders responsive plan cards with appropriate styling for the popular plan.apps/code/src/app/layout.tsx (1)
49-59: Providers wiring viagetConfig()in RootLayout looks clean.Nice separation: server config resolved once at the layout and passed into a single Providers entrypoint.
packages/db/src/types.ts (2)
79-87: OrganizationdevPlantyping matches the DB enum; looks good.The
Omit<..., "devPlan"> & { devPlan: ... }pattern keeps the enum narrow and consistent.
107-126: SerializedOrganization timestamp serialization for dev plan fields is consistent.Adding
devPlanBillingCycleStart/devPlanExpiresAtasstring | nullin the serialized shape matches the existing serialization strategy for timestamps.packages/db/src/schema.ts (2)
147-159: LGTM! Dev plan fields are well-structured.The new organization fields follow existing patterns with appropriate defaults and constraints. The
devPlanStripeSubscriptionIdunique constraint prevents duplicate subscription associations.
208-213: LGTM! Transaction types cover the complete dev plan lifecycle.The new transaction types (
dev_plan_start,dev_plan_upgrade,dev_plan_downgrade,dev_plan_cancel,dev_plan_end,dev_plan_renewal) properly mirror the existing subscription event types and will enable comprehensive audit trails.apps/code/src/app/dashboard/page.tsx (1)
77-98: LGTM! Subscribe flow handles loading state and errors appropriately.The mutation correctly manages the subscribing state, validates the checkout URL, and captures analytics before redirecting.
apps/code/src/app/login/page.tsx (1)
34-42: LGTM! Good open redirect protection.
getSafeRedirectUrlproperly validates that the redirect URL is a relative path (starts with/but not//), preventing open redirect vulnerabilities.apps/api/src/stripe.ts (2)
293-388: LGTM! Dev plan checkout session handling is comprehensive.The handler correctly:
- Activates the dev plan and sets credits limits
- Creates transaction records with duplicate prevention
- Generates and emails invoices with proper error suppression
- Tracks events in PostHog
1028-1057: LGTM! Dev plan renewal correctly resets credits.The renewal handler properly resets
devPlanCreditsUsedto "0" and updatesdevPlanBillingCycleStart, ensuring users get their full credit allocation each billing cycle.apps/api/src/routes/dev-plans.ts (1)
66-142: LGTM! Personal org retrieval/creation logic is solid.The handler correctly finds existing personal orgs or creates new ones with the necessary associations (user-org relationship, default project). The response properly serializes all dev plan fields.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| try { | ||
| await stripe.subscriptions.update(personalOrg.devPlanStripeSubscriptionId, { | ||
| cancel_at_period_end: true, | ||
| }); | ||
|
|
||
| // Wait for webhook to process | ||
| await new Promise((resolve) => { | ||
| setTimeout(resolve, 3000); | ||
| }); | ||
|
|
||
| return c.json({ | ||
| success: true, | ||
| }); | ||
| } catch (error) { | ||
| logger.error( | ||
| "Stripe dev plan cancellation error", | ||
| error instanceof Error ? error : new Error(String(error)), | ||
| ); | ||
| throw new HTTPException(500, { | ||
| message: "Failed to cancel dev plan subscription", | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Avoid blocking sleep to wait for webhooks.
The 3-second setTimeout blocks the request thread and provides no guarantee the webhook has been processed. Webhooks may arrive later, or not at all due to network issues.
Consider returning success immediately after the Stripe API call succeeds, and let the UI poll for the updated status (which it already does via /status endpoint).
🐛 Proposed fix: Remove the sleep
try {
await stripe.subscriptions.update(personalOrg.devPlanStripeSubscriptionId, {
cancel_at_period_end: true,
});
- // Wait for webhook to process
- await new Promise((resolve) => {
- setTimeout(resolve, 3000);
- });
-
return c.json({
success: true,
});
} catch (error) {The dashboard already polls /status every 5 seconds, so the UI will reflect the cancellation state once the webhook updates the database.
🤖 Prompt for AI Agents
In `@apps/api/src/routes/dev-plans.ts` around lines 354 - 376, Remove the blocking
sleep after calling stripe.subscriptions.update: delete the new Promise with
setTimeout and return success immediately once
stripe.subscriptions.update(personalOrg.devPlanStripeSubscriptionId, {
cancel_at_period_end: true }) resolves; keep the existing try/catch and error
logging (logger.error and HTTPException) but do not wait for webhooks in the
request handler—relying on the dashboard polling /status is sufficient.
|
|
||
| await stripe.subscriptions.update(personalOrg.devPlanStripeSubscriptionId, { | ||
| cancel_at_period_end: false, | ||
| }); | ||
|
|
||
| // Wait for webhook to process | ||
| await new Promise((resolve) => { | ||
| setTimeout(resolve, 3000); | ||
| }); | ||
|
|
||
| return c.json({ | ||
| success: true, | ||
| }); | ||
| } catch (error) { | ||
| logger.error( | ||
| "Stripe dev plan resume error", | ||
| error instanceof Error ? error : new Error(String(error)), | ||
| ); | ||
| throw new HTTPException(500, { | ||
| message: "Failed to resume dev plan subscription", | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Same blocking sleep issue in resume handler.
Apply the same fix as the cancel handler - remove the 3-second sleep.
🤖 Prompt for AI Agents
In `@apps/api/src/routes/dev-plans.ts` around lines 441 - 463, The resume handler
contains a blocking 3-second sleep after calling stripe.subscriptions.update
which should be removed (same fix as the cancel handler); delete the
Promise/setTimeout block following stripe.subscriptions.update in the resume
route so the handler returns immediately after the update (references:
stripe.subscriptions.update and personalOrg.devPlanStripeSubscriptionId),
leaving the existing error handling and response intact.
| await stripe.subscriptions.update(personalOrg.devPlanStripeSubscriptionId, { | ||
| items: [ | ||
| { | ||
| id: subscription.items.data[0].id, | ||
| price: newPriceId, | ||
| }, | ||
| ], | ||
| proration_behavior: "create_prorations", | ||
| metadata: { | ||
| ...subscription.metadata, | ||
| devPlan: newTier, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Add null check for subscription items.
subscription.items.data[0] assumes the subscription has at least one item. While this should always be true for valid subscriptions, a defensive check prevents potential runtime errors.
🐛 Proposed fix
const subscription = await stripe.subscriptions.retrieve(
personalOrg.devPlanStripeSubscriptionId,
);
+ if (!subscription.items.data[0]) {
+ throw new HTTPException(500, {
+ message: "Invalid subscription: no items found",
+ });
+ }
+
// Update subscription with new tier
await stripe.subscriptions.update(personalOrg.devPlanStripeSubscriptionId, {🤖 Prompt for AI Agents
In `@apps/api/src/routes/dev-plans.ts` around lines 548 - 560, The code assumes
subscription.items.data[0] exists; add a defensive null/length check before
using it (e.g., verify subscription.items and subscription.items.data are
present and length > 0), extract the first item id into a variable (like
firstItemId) and if missing return/throw a clear error or log and skip the
update; update the call in the stripe.subscriptions.update block to use that
validated firstItemId and ensure metadata update still occurs only after the
check.
| posthog.identify(ctx.data.user.id, { | ||
| email: ctx.data.user.email, | ||
| name: ctx.data.user.name, | ||
| }); | ||
| posthog.capture("user_signed_up", { | ||
| email: values.email, | ||
| name: values.name, | ||
| plan: selectedPlan, | ||
| }); |
There was a problem hiding this comment.
PII logged to analytics.
Sending email and name to PostHog may have privacy/compliance implications (GDPR, CCPA). Consider whether this is intentional and documented in your privacy policy. If only tracking is needed, the user ID alone should suffice.
Proposed fix - remove PII from event
- posthog.identify(ctx.data.user.id, {
- email: ctx.data.user.email,
- name: ctx.data.user.name,
- });
- posthog.capture("user_signed_up", {
- email: values.email,
- name: values.name,
- plan: selectedPlan,
- });
+ posthog?.identify(ctx.data.user.id);
+ posthog?.capture("user_signed_up", {
+ plan: selectedPlan,
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| posthog.identify(ctx.data.user.id, { | |
| email: ctx.data.user.email, | |
| name: ctx.data.user.name, | |
| }); | |
| posthog.capture("user_signed_up", { | |
| email: values.email, | |
| name: values.name, | |
| plan: selectedPlan, | |
| }); | |
| posthog?.identify(ctx.data.user.id); | |
| posthog?.capture("user_signed_up", { | |
| plan: selectedPlan, | |
| }); |
🤖 Prompt for AI Agents
In `@apps/code/src/app/signup/page.tsx` around lines 85 - 93, The code is sending
PII (email and name) to PostHog via posthog.identify and posthog.capture; remove
ctx.data.user.email, ctx.data.user.name, values.email and values.name from those
calls and only identify the user by their unique ID (e.g.,
posthog.identify(ctx.data.user.id) with no PII properties) and send non-PII
event properties like selectedPlan; if a user-readable name is required for
product features, persist it server-side or use a hashed/consented value
instead, and ensure any intentional PII collection is documented in the privacy
policy.
| const useFormField = () => { | ||
| const fieldContext = React.useContext(FormFieldContext); | ||
| const itemContext = React.useContext(FormItemContext); | ||
| const { getFieldState } = useFormContext(); | ||
| const formState = useFormState({ name: fieldContext.name }); | ||
| const fieldState = getFieldState(fieldContext.name, formState); | ||
|
|
||
| if (!fieldContext) { | ||
| throw new Error("useFormField should be used within <FormField>"); | ||
| } | ||
|
|
There was a problem hiding this comment.
Incorrect guard check ordering in useFormField.
The null check for fieldContext at line 54 happens after fieldContext.name is already used at line 51. This means if fieldContext is falsy, the code will error before reaching the guard. Move the check before usage.
Proposed fix
const useFormField = () => {
const fieldContext = React.useContext(FormFieldContext);
const itemContext = React.useContext(FormItemContext);
+
+ if (!fieldContext.name) {
+ throw new Error("useFormField should be used within <FormField>");
+ }
+
const { getFieldState } = useFormContext();
const formState = useFormState({ name: fieldContext.name });
const fieldState = getFieldState(fieldContext.name, formState);
- if (!fieldContext) {
- throw new Error("useFormField should be used within <FormField>");
- }
-
const { id } = itemContext;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const useFormField = () => { | |
| const fieldContext = React.useContext(FormFieldContext); | |
| const itemContext = React.useContext(FormItemContext); | |
| const { getFieldState } = useFormContext(); | |
| const formState = useFormState({ name: fieldContext.name }); | |
| const fieldState = getFieldState(fieldContext.name, formState); | |
| if (!fieldContext) { | |
| throw new Error("useFormField should be used within <FormField>"); | |
| } | |
| const useFormField = () => { | |
| const fieldContext = React.useContext(FormFieldContext); | |
| const itemContext = React.useContext(FormItemContext); | |
| if (!fieldContext) { | |
| throw new Error("useFormField should be used within <FormField>"); | |
| } | |
| const { getFieldState } = useFormContext(); | |
| const formState = useFormState({ name: fieldContext.name }); | |
| const fieldState = getFieldState(fieldContext.name, formState); | |
| const { id } = itemContext; |
🤖 Prompt for AI Agents
In `@apps/code/src/components/ui/form.tsx` around lines 47 - 57, The null-check
for fieldContext is after its usage in useFormField, causing a crash when
fieldContext is falsy; move the guard so the check for fieldContext (the
React.useContext(FormFieldContext) result) occurs before any access to
fieldContext.name or before calling useFormState/getFieldState with
fieldContext.name; ensure the early throw in useFormField runs immediately after
obtaining fieldContext and before computing formState or calling getFieldState
so all uses of fieldContext.name are safe.
| useEffect(() => { | ||
| if (!data?.user || isLoading) { | ||
| return; | ||
| } | ||
|
|
||
| const currentPath = pathname; | ||
| const isAuthPage = ["/login", "/signup"].includes(currentPath); | ||
| const isLandingPage = currentPath === "/"; | ||
|
|
||
| if (isAuthPage || isLandingPage) { | ||
| return; | ||
| } | ||
| }, [data?.user, isLoading, router, pathname]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove dead code: this useEffect performs no action.
This effect checks conditions for auth/landing pages but never executes any logic—it just returns early. If redirect prevention logic was intended here, it's incomplete. Otherwise, this should be removed.
Proposed fix: remove the dead effect
- useEffect(() => {
- if (!data?.user || isLoading) {
- return;
- }
-
- const currentPath = pathname;
- const isAuthPage = ["/login", "/signup"].includes(currentPath);
- const isLandingPage = currentPath === "/";
-
- if (isAuthPage || isLandingPage) {
- return;
- }
- }, [data?.user, isLoading, router, pathname]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| if (!data?.user || isLoading) { | |
| return; | |
| } | |
| const currentPath = pathname; | |
| const isAuthPage = ["/login", "/signup"].includes(currentPath); | |
| const isLandingPage = currentPath === "/"; | |
| if (isAuthPage || isLandingPage) { | |
| return; | |
| } | |
| }, [data?.user, isLoading, router, pathname]); |
🤖 Prompt for AI Agents
In `@apps/code/src/hooks/useUser.ts` around lines 38 - 50, The useEffect block
starting with useEffect(() => { if (!data?.user || isLoading) { return; } ... },
[data?.user, isLoading, router, pathname]) is dead code because it contains no
side-effects or logic after its guards; remove this entire useEffect to clean up
the hook (delete the effect that references data?.user, isLoading, router, and
pathname). If redirect or auth-handling behavior was intended, re-implement it
explicitly in a new effect or function instead of leaving this no-op effect.
| export async function fetchServerData<T>( | ||
| method: "GET" | "POST" | "PUT" | "DELETE" | "PATCH", | ||
| path: keyof paths, | ||
| options?: any, | ||
| ): Promise<T | null> { |
There was a problem hiding this comment.
Avoid any for the options parameter.
Per coding guidelines, any should not be used unless absolutely necessary. The options parameter can be typed more strictly using the openapi-fetch request options types.
Suggested improvement
+import type { FetchOptions } from "openapi-fetch";
+
export async function fetchServerData<T>(
method: "GET" | "POST" | "PUT" | "DELETE" | "PATCH",
path: keyof paths,
- options?: any,
+ options?: FetchOptions<unknown>,
): Promise<T | null> {Alternatively, if more precise typing is needed per method, consider method-specific overloads or a union type based on the path parameter.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function fetchServerData<T>( | |
| method: "GET" | "POST" | "PUT" | "DELETE" | "PATCH", | |
| path: keyof paths, | |
| options?: any, | |
| ): Promise<T | null> { | |
| import type { FetchOptions } from "openapi-fetch"; | |
| export async function fetchServerData<T>( | |
| method: "GET" | "POST" | "PUT" | "DELETE" | "PATCH", | |
| path: keyof paths, | |
| options?: FetchOptions<unknown>, | |
| ): Promise<T | null> { |
🤖 Prompt for AI Agents
In `@apps/code/src/lib/server-api.ts` around lines 49 - 53, The fetchServerData
function uses options?: any which violates the no-any guideline; replace any
with the proper OpenAPI request options type (e.g., the request/options type
exported by your openapi-fetch client or generator) and import that type into
this module, then update the signature of fetchServerData(method: "GET" | "POST"
| "PUT" | "DELETE" | "PATCH", path: keyof paths, options?:
OpenApiRequestOptions): Promise<T | null>; if needed create method-specific
overloads or a union of option types for different HTTP verbs to tighten typing
further.
| // Dev plan credits are deducted first, then regular credits | ||
| const referralEarnings = new Map<string, Decimal>(); | ||
|
|
||
| for (const [orgId, totalCost] of orgCosts.entries()) { | ||
| if (totalCost.greaterThan(0)) { | ||
| const costNumber = totalCost.toNumber(); | ||
| await tx | ||
| .update(organization) | ||
| .set({ | ||
| credits: sql`${organization.credits} - ${costNumber}`, | ||
| }) | ||
| .where(eq(organization.id, orgId)); | ||
| let remainingCost = totalCost; | ||
|
|
||
| logger.debug( | ||
| `Deducted ${costNumber} credits from organization ${orgId}`, | ||
| ); | ||
| // Fetch the organization to check for dev plan | ||
| const org = await tx.query.organization.findFirst({ | ||
| where: { id: { eq: orgId } }, | ||
| }); | ||
|
|
||
| // First, try to deduct from dev plan credits if available | ||
| if (org && org.devPlan !== "none") { | ||
| const devPlanCreditsLimit = new Decimal( | ||
| org.devPlanCreditsLimit || "0", | ||
| ); | ||
| const devPlanCreditsUsed = new Decimal( | ||
| org.devPlanCreditsUsed || "0", | ||
| ); | ||
| const devPlanRemaining = | ||
| devPlanCreditsLimit.minus(devPlanCreditsUsed); | ||
|
|
||
| if (devPlanRemaining.greaterThan(0)) { | ||
| const deductFromDevPlan = Decimal.min( | ||
| remainingCost, | ||
| devPlanRemaining, | ||
| ); | ||
| const deductNumber = deductFromDevPlan.toNumber(); | ||
|
|
||
| await tx | ||
| .update(organization) | ||
| .set({ | ||
| devPlanCreditsUsed: sql`${organization.devPlanCreditsUsed} + ${deductNumber}`, | ||
| }) | ||
| .where(eq(organization.id, orgId)); | ||
|
|
||
| logger.debug( | ||
| `Deducted ${deductNumber} dev plan credits from organization ${orgId}`, | ||
| ); | ||
|
|
||
| remainingCost = remainingCost.minus(deductFromDevPlan); | ||
| } | ||
| } | ||
|
|
||
| // Deduct any remaining cost from regular credits | ||
| if (remainingCost.greaterThan(0)) { | ||
| const costNumber = remainingCost.toNumber(); | ||
| await tx | ||
| .update(organization) | ||
| .set({ | ||
| credits: sql`${organization.credits} - ${costNumber}`, | ||
| }) | ||
| .where(eq(organization.id, orgId)); | ||
|
|
||
| logger.debug( | ||
| `Deducted ${costNumber} regular credits from organization ${orgId}`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Credit precision risk: avoid Decimal -> number for balance updates.
toNumber() can introduce floating rounding (especially with fractional costs). Prefer updating using string/Decimal values so Postgres numeric math stays exact.
Proposed fix (keep arithmetic in numeric/string form)
- const deductNumber = deductFromDevPlan.toNumber();
+ const deductValue = deductFromDevPlan.toString();
await tx
.update(organization)
.set({
- devPlanCreditsUsed: sql`${organization.devPlanCreditsUsed} + ${deductNumber}`,
+ devPlanCreditsUsed: sql`${organization.devPlanCreditsUsed} + ${deductValue}`,
})
.where(eq(organization.id, orgId));
...
- const costNumber = remainingCost.toNumber();
+ const costValue = remainingCost.toString();
await tx
.update(organization)
.set({
- credits: sql`${organization.credits} - ${costNumber}`,
+ credits: sql`${organization.credits} - ${costValue}`,
})
.where(eq(organization.id, orgId));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Dev plan credits are deducted first, then regular credits | |
| const referralEarnings = new Map<string, Decimal>(); | |
| for (const [orgId, totalCost] of orgCosts.entries()) { | |
| if (totalCost.greaterThan(0)) { | |
| const costNumber = totalCost.toNumber(); | |
| await tx | |
| .update(organization) | |
| .set({ | |
| credits: sql`${organization.credits} - ${costNumber}`, | |
| }) | |
| .where(eq(organization.id, orgId)); | |
| let remainingCost = totalCost; | |
| logger.debug( | |
| `Deducted ${costNumber} credits from organization ${orgId}`, | |
| ); | |
| // Fetch the organization to check for dev plan | |
| const org = await tx.query.organization.findFirst({ | |
| where: { id: { eq: orgId } }, | |
| }); | |
| // First, try to deduct from dev plan credits if available | |
| if (org && org.devPlan !== "none") { | |
| const devPlanCreditsLimit = new Decimal( | |
| org.devPlanCreditsLimit || "0", | |
| ); | |
| const devPlanCreditsUsed = new Decimal( | |
| org.devPlanCreditsUsed || "0", | |
| ); | |
| const devPlanRemaining = | |
| devPlanCreditsLimit.minus(devPlanCreditsUsed); | |
| if (devPlanRemaining.greaterThan(0)) { | |
| const deductFromDevPlan = Decimal.min( | |
| remainingCost, | |
| devPlanRemaining, | |
| ); | |
| const deductNumber = deductFromDevPlan.toNumber(); | |
| await tx | |
| .update(organization) | |
| .set({ | |
| devPlanCreditsUsed: sql`${organization.devPlanCreditsUsed} + ${deductNumber}`, | |
| }) | |
| .where(eq(organization.id, orgId)); | |
| logger.debug( | |
| `Deducted ${deductNumber} dev plan credits from organization ${orgId}`, | |
| ); | |
| remainingCost = remainingCost.minus(deductFromDevPlan); | |
| } | |
| } | |
| // Deduct any remaining cost from regular credits | |
| if (remainingCost.greaterThan(0)) { | |
| const costNumber = remainingCost.toNumber(); | |
| await tx | |
| .update(organization) | |
| .set({ | |
| credits: sql`${organization.credits} - ${costNumber}`, | |
| }) | |
| .where(eq(organization.id, orgId)); | |
| logger.debug( | |
| `Deducted ${costNumber} regular credits from organization ${orgId}`, | |
| ); | |
| } | |
| // Dev plan credits are deducted first, then regular credits | |
| const referralEarnings = new Map<string, Decimal>(); | |
| for (const [orgId, totalCost] of orgCosts.entries()) { | |
| if (totalCost.greaterThan(0)) { | |
| let remainingCost = totalCost; | |
| // Fetch the organization to check for dev plan | |
| const org = await tx.query.organization.findFirst({ | |
| where: { id: { eq: orgId } }, | |
| }); | |
| // First, try to deduct from dev plan credits if available | |
| if (org && org.devPlan !== "none") { | |
| const devPlanCreditsLimit = new Decimal( | |
| org.devPlanCreditsLimit || "0", | |
| ); | |
| const devPlanCreditsUsed = new Decimal( | |
| org.devPlanCreditsUsed || "0", | |
| ); | |
| const devPlanRemaining = | |
| devPlanCreditsLimit.minus(devPlanCreditsUsed); | |
| if (devPlanRemaining.greaterThan(0)) { | |
| const deductFromDevPlan = Decimal.min( | |
| remainingCost, | |
| devPlanRemaining, | |
| ); | |
| const deductValue = deductFromDevPlan.toString(); | |
| await tx | |
| .update(organization) | |
| .set({ | |
| devPlanCreditsUsed: sql`${organization.devPlanCreditsUsed} + ${deductValue}`, | |
| }) | |
| .where(eq(organization.id, orgId)); | |
| logger.debug( | |
| `Deducted ${deductValue} dev plan credits from organization ${orgId}`, | |
| ); | |
| remainingCost = remainingCost.minus(deductFromDevPlan); | |
| } | |
| } | |
| // Deduct any remaining cost from regular credits | |
| if (remainingCost.greaterThan(0)) { | |
| const costValue = remainingCost.toString(); | |
| await tx | |
| .update(organization) | |
| .set({ | |
| credits: sql`${organization.credits} - ${costValue}`, | |
| }) | |
| .where(eq(organization.id, orgId)); | |
| logger.debug( | |
| `Deducted ${costValue} regular credits from organization ${orgId}`, | |
| ); | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/api/src/routes/dev-plans.ts`:
- Around line 204-228: The org creation block duplicates getPersonalOrg’s
missing transaction handling; extract a shared helper
getOrCreatePersonalOrg(user, db) that runs inside a single transaction (use
db.transaction or the project’s transaction helper) and performs the insert into
tables.organization, tables.userOrganization and tables.project together,
returning the created organization; replace the inline creation code and any
other usages (including where personalOrg is set) with calls to
getOrCreatePersonalOrg to ensure atomicity and avoid partial state on failures.
♻️ Duplicate comments (8)
apps/code/src/app/page.tsx (1)
84-86: "Free Trial" messaging may be inaccurate.The CTAs reference "Start Free Trial" but the PR objectives don't mention a free trial feature. If there's no actual free trial implementation, update the copy to match the actual flow (e.g., "Get Started").
Also applies to lines 202-204.
apps/code/src/app/dashboard/page.tsx (1)
62-70: Consider reducing polling frequency.Polling every 5 seconds may be excessive for dev plan status, which typically only changes during user actions. Consider increasing to 30 seconds or using manual refetch after mutations.
apps/api/src/stripe.ts (1)
20-32: Duplicate pricing constants - already flagged.This duplication has been identified in a previous review. Extract
DEV_PLAN_PRICES,DevPlanTier, andgetDevPlanCreditsLimitto a shared module to avoid drift betweenstripe.tsanddev-plans.ts.apps/api/src/routes/dev-plans.ts (5)
16-27: Duplicate pricing constants - already flagged.Extract these to a shared module as noted in the previous review.
359-362: Remove blocking sleep - already flagged.The 3-second sleep blocks the request thread and doesn't guarantee webhook processing. The UI already polls
/status.
446-449: Remove blocking sleep - already flagged.Same issue as the cancel handler.
547-560: Add null check for subscription items - already flagged.
564-566: Type assertion may fail for "none" plan - already flagged.
🧹 Nitpick comments (5)
apps/code/src/app/page.tsx (1)
6-49: Consider extracting shared plan definitions.The
plansarray is duplicated in bothpage.tsxanddashboard/page.tsx. Extract this to a shared constants file to maintain consistency and reduce maintenance burden.♻️ Suggested extraction
Create a shared file, e.g.,
apps/code/src/lib/plans.ts:export const plans = [ { name: "Lite", price: 29, credits: 87, description: "For small dev tasks and getting started", features: [ "$87 in monthly credits", "Access to all LLM models", "Claude, GPT-4, and more", "Credits reset monthly", ], tier: "lite" as const, }, // ... Pro and Max ] as const; export type PlanTier = (typeof plans)[number]["tier"];Then import in both pages:
import { plans } from "@/lib/plans";apps/code/src/app/dashboard/page.tsx (2)
93-94: Empty catch blocks lose error context.The catch blocks discard error information which makes debugging harder. Consider logging the error or including it in the toast message.
♻️ Proposed fix
- } catch { + } catch (error) { + console.error("Failed to start subscription:", error); toast.error("Failed to start subscription"); } finally {Apply similar pattern to
handleCancel(line 109),handleResume(line 122), andhandleChangeTier(line 141).
23-46: Duplicate plan definitions.Same
plansarray exists inpage.tsx. See earlier comment about extracting to a shared module.apps/api/src/routes/dev-plans.ts (1)
568-582: Consider adding error handling for DB update after Stripe change.If the Stripe subscription update succeeds (lines 548-560) but the database update fails (lines 568-574), there's a temporary inconsistency. While webhooks will eventually sync the state, consider wrapping the DB operations in a try-catch with logging, or relying entirely on webhooks for state sync instead of optimistic local updates.
apps/api/src/stripe.ts (1)
1351-1356: Use dev-plan-specific cancellation email copy.The dev plan cancellation sends
generateSubscriptionCancelledEmailHtml, which contains hardcoded Pro subscription messaging: "Your Pro subscription for ${organizationName} has been cancelled and your organization has been downgraded to the free plan." This template doesn't apply to dev plans.Create a dev-plan-specific template or modify the existing one to accept a plan parameter for appropriate user messaging.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/code/src/lib/api/v1.d.tsis excluded by!**/v1.d.ts
📒 Files selected for processing (10)
.env.example.env.unified.exampleapps/api/src/index.tsapps/api/src/routes/dev-plans.tsapps/api/src/stripe.tsapps/code/package.jsonapps/code/src/app/dashboard/page.tsxapps/code/src/app/page.tsxapps/gateway/src/app.tspackage.json
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/code/package.json
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never useanyoras anyunless absolutely necessary in TypeScript code
For database reads: Usedb().query.<table>.findMany()ordb().query.<table>.findFirst()
Files:
apps/api/src/index.tsapps/api/src/routes/dev-plans.tsapps/gateway/src/app.tsapps/code/src/app/page.tsxapps/code/src/app/dashboard/page.tsxapps/api/src/stripe.ts
**/*.{ts,tsx,js,jsx,json,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use tabs for indentation
Files:
apps/api/src/index.tsapps/api/src/routes/dev-plans.tsapps/gateway/src/app.tsapps/code/src/app/page.tsxpackage.jsonapps/code/src/app/dashboard/page.tsxapps/api/src/stripe.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Always use top-levelimport, never use require or dynamic imports
No unnecessary code comments
Files:
apps/api/src/index.tsapps/api/src/routes/dev-plans.tsapps/gateway/src/app.tsapps/code/src/app/page.tsxapps/code/src/app/dashboard/page.tsxapps/api/src/stripe.ts
apps/{gateway,api}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Hono framework with Zod validation and OpenAPI documentation for backend APIs
Files:
apps/api/src/index.tsapps/api/src/routes/dev-plans.tsapps/gateway/src/app.tsapps/api/src/stripe.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Always use top-level
import, never use require or dynamic imports
Files:
apps/api/src/index.tsapps/api/src/routes/dev-plans.tsapps/gateway/src/app.tsapps/code/src/app/page.tsxapps/code/src/app/dashboard/page.tsxapps/api/src/stripe.ts
{apps/api,apps/gateway,packages/db}/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
{apps/api,apps/gateway,packages/db}/**/*.ts: Use Drizzle ORM with latest object syntax for database operations
For database reads: Usedb().query.<table>.findMany()ordb().query.<table>.findFirst()
Files:
apps/api/src/index.tsapps/api/src/routes/dev-plans.tsapps/gateway/src/app.tsapps/api/src/stripe.ts
apps/{gateway,api}/src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
apps/{gateway,api}/src/**/*.ts: Use Hono for backend framework in Gateway and API services
Use Zod schemas for validation in Hono services
Files:
apps/api/src/index.tsapps/api/src/routes/dev-plans.tsapps/gateway/src/app.tsapps/api/src/stripe.ts
{package.json,pnpm-lock.yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
pnpmfor package management, never use npm or yarn
Files:
package.json
🧠 Learnings (5)
📚 Learning: 2025-12-03T12:42:26.162Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T12:42:26.162Z
Learning: Applies to apps/{gateway,api}/src/**/*.ts : Use Hono for backend framework in Gateway and API services
Applied to files:
apps/gateway/src/app.ts
📚 Learning: 2025-12-03T12:42:26.162Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T12:42:26.162Z
Learning: Applies to apps/{ui,playground,docs}/**/*.{ts,tsx} : Use `next/link` for links and `next/navigation`'s router for programmatic navigation
Applied to files:
apps/code/src/app/page.tsxapps/code/src/app/dashboard/page.tsx
📚 Learning: 2025-12-03T12:42:14.219Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T12:42:14.219Z
Learning: Applies to apps/{ui,playground}/**/*.{ts,tsx} : Use Next.js App Router with React Server Components for frontend development
Applied to files:
apps/code/src/app/page.tsxapps/code/src/app/dashboard/page.tsx
📚 Learning: 2025-12-03T12:42:14.219Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T12:42:14.219Z
Learning: Applies to apps/{ui,playground}/**/*.{tsx} : Use `next/link` for links and `next/navigation`'s router for programmatic navigation
Applied to files:
apps/code/src/app/page.tsx
📚 Learning: 2025-12-03T12:42:14.219Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T12:42:14.219Z
Learning: Run `pnpm build` to ensure production builds work after code changes
Applied to files:
package.json
🧬 Code graph analysis (3)
apps/code/src/app/page.tsx (1)
apps/code/src/components/ui/button.tsx (1)
Button(60-60)
apps/code/src/app/dashboard/page.tsx (5)
apps/api/src/posthog.ts (1)
posthog(3-6)apps/code/src/lib/auth-client.ts (1)
useAuth(18-31)apps/code/src/lib/fetch-client.ts (1)
useApi(20-26)apps/code/src/hooks/useUser.ts (1)
useUser(14-85)apps/ui/src/lib/components/use-toast.ts (1)
toast(194-194)
apps/api/src/stripe.ts (6)
apps/api/src/routes/organization.ts (1)
organization(11-11)packages/db/src/schema.ts (2)
organization(109-160)transaction(189-233)packages/db/src/index.ts (1)
tables(14-16)apps/api/src/utils/invoice.ts (1)
generateAndEmailInvoice(192-283)apps/api/src/posthog.ts (1)
posthog(3-6)apps/api/src/utils/email.ts (2)
sendTransactionalEmail(38-105)generateSubscriptionCancelledEmailHtml(251-330)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 59-59: [UnorderedKey] The CODE_URL key should go before the PLAYGROUND_URL key
(UnorderedKey)
[warning] 199-199: [UnorderedKey] The STRIPE_DEV_PLAN_MAX_PRICE_ID key should go before the STRIPE_DEV_PLAN_PRO_PRICE_ID key
(UnorderedKey)
.env.unified.example
[warning] 20-20: [UnorderedKey] The CODE_URL key should go before the PLAYGROUND_URL key
(UnorderedKey)
[warning] 133-133: [UnorderedKey] The STRIPE_DEV_PLAN_MAX_PRICE_ID key should go before the STRIPE_DEV_PLAN_PRO_PRICE_ID key
(UnorderedKey)
🔇 Additional comments (14)
package.json (1)
12-12: LGTM!The addition of
--filter=!codecorrectly excludes the newcodeapp from core builds, consistent with how other frontend apps (ui,docs,playground,admin) are already excluded.apps/api/src/index.ts (1)
60-61: LGTM — minor clarification on port 3005.Port 3004 aligns with the new Dev Plans "code" app mentioned in the PR. Could you clarify what service is expected to run on port 3005? Adding a brief comment or documenting the port assignments would help maintainability.
apps/gateway/src/app.ts (1)
76-76: LGTM!The addition of
http://localhost:3004to the CORS origins is consistent with the newCODE_URLenvironment variable and enables the new Code app to communicate with the gateway..env.unified.example (2)
20-20: LGTM!The
CODE_URLaddition is consistent with the new Code app at port 3004.
129-136: LGTM!The Dev Plans Stripe configuration variables are well-documented with clear comments explaining their purpose and default values.
.env.example (2)
59-59: LGTM!Consistent with
.env.unified.example.
196-202: LGTM!Dev Plans configuration is consistent with
.env.unified.exampleand well-documented.apps/code/src/app/page.tsx (1)
51-222: LGTM on component structure.The landing page is well-structured with clear sections, uses
next/linkfor navigation per coding guidelines, and the pricing grid renders correctly with conditional styling for the popular plan.apps/code/src/app/dashboard/page.tsx (2)
161-167: LGTM on credit calculations.The credit usage logic correctly handles edge cases with
Math.max(0, ...)and safe percentage calculation whencreditsLimitis 0.
48-60: LGTM on authentication flow.The
useUserhook withredirectToandredirectWhenoptions properly handles unauthenticated users, and the component correctly usesnext/navigationfor programmatic navigation. Based on learnings, this follows the recommended pattern.apps/api/src/stripe.ts (3)
1019-1074: Dev plan renewal logic looks correct.The renewal detection via
devPlanStripeSubscriptionIdmatch anddevPlan !== "none"check is sound. The credit reset and transaction recording follow the expected pattern.
1185-1243: Dev plan subscription update handling looks good.The dual detection mechanism (metadata or subscription ID match) is robust. Transaction records for cancellation events are properly created.
315-323: The Drizzle query syntax used is correct and consistent with the codebase. The{ fieldName: { eq: value } }pattern is the established standard throughout the repository (used in stripe.ts, worker.ts, organization.ts, and payments.ts) and aligns with the "latest object syntax" requirement from the coding guidelines. No refactoring needed.apps/api/src/routes/dev-plans.ts (1)
598-680: Status endpoint implementation looks good.Auth check, null handling for missing personal org, and credit calculations are all correct.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…s-subscription-ime9td # Conflicts: # apps/gateway/src/chat/chat.ts # apps/gateway/src/lib/rate-limit.spec.ts # packages/db/src/schema.ts # packages/db/src/types.ts
- Auto-create API key when user has active dev plan - Show API key in dashboard with copy/show/hide controls - Link to integration guides from dashboard - Change usage display from $ amounts to percentage - Remove free trial mentions (direct paid subscription) - Remove credits terminology from UI copy - Update metadata descriptions 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
steebchen
changed the title
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.env.example (1)
56-64: Add CODE_URL origin to ORIGIN_URLS (likely required for local dev).
CODE_URLishttp://localhost:3004, butORIGIN_URLSstill only includeshttp://localhost:3002, which can break CORS/auth callbacks when the code app calls the API.Proposed fix
ORIGIN_URLS=http://localhost:3002 +# ORIGIN_URLS=http://localhost:3002,http://localhost:3004 +ORIGIN_URLS=http://localhost:3002,http://localhost:3004apps/gateway/src/chat/chat.ts (1)
1854-1875: Use the same “free model” predicate as other branches (isModelTrulyFree).This branch checks
!(modelInfo as ModelDefinition).free, while the hybrid branch usesisModelTrulyFree(...). Aligning avoids inconsistent credit enforcement across modes.
🤖 Fix all issues with AI agents
In `@apps/api/src/routes/dev-plans.ts`:
- Around line 706-725: The GET /status handler currently performs a DB write by
calling getOrCreatePersonalOrgApiKey when personalOrg.devPlan !== "none" and
uses db.query.project.findFirst without ordering, causing nondeterministic
project selection; change the handler to remove side effects (do not create API
keys on GET) and instead only read existing keys, moving creation to an explicit
POST endpoint (or subscription/webhook flow) that calls
getOrCreatePersonalOrgApiKey, and make project lookup deterministic by replacing
db.query.project.findFirst with a deterministic query (e.g., findFirst with an
orderBy createdAt or lookup by a “Default Project” name/flag) so the code that
currently references personalOrg.devPlan, db.query.project.findFirst, and
getOrCreatePersonalOrgApiKey no longer performs writes on GET and selects a
predictable project.
In `@apps/api/src/stripe.ts`:
- Around line 348-353: The invoice line item description in the lineItems array
mixes currency and credit units; update the description construction (where
description is built using devPlanTier and creditsLimit) to remove the dollar
sign and clearly indicate credits (e.g., "Dev Plan ${devPlanTier.toUpperCase()}
(${creditsLimit} credits included)") so it doesn't read like a dollar amount;
ensure only the description string is changed and leave amount calculation
(amount: (session.amount_total || 0) / 100) untouched.
- Around line 281-304: The code unsafely casts metadata.devPlan to DevPlanTier;
validate metadata.devPlan against the allowed DevPlanTier values before using
it. In the block that sets isDevPlan and devPlanTier, replace the unchecked cast
by checking metadata?.devPlan is one of the known tiers (e.g., via a Set/map or
a type-guard function isValidDevPlanTier(value): value is DevPlanTier) and only
assign devPlanTier when valid; if invalid, log/warn and skip calling
getDevPlanCreditsLimit and the DB update that writes devPlanCreditsLimit/related
fields. Ensure getDevPlanCreditsLimit is called only with a validated
devPlanTier and that you do not persist NaN or unexpected values.
- Around line 1431-1437: The cancellation email send isn't protected and can
surface transient failures to Stripe; wrap the await sendTransactionalEmail call
(the call using sendTransactionalEmail with
generateSubscriptionCancelledEmailHtml and organization.billingEmail) in a
try/catch inside the webhook handler so any errors are caught, logged via
processLogger or similar, and not re-thrown — do not change the handler's
success flow or return value so Stripe sees the webhook as handled even if email
sending fails.
- Around line 1108-1113: Validate organization.devPlan before casting to
DevPlanTier: inside the isDevPlanRenewal block, check that organization.devPlan
is a string and that organization.devPlan in DEV_PLAN_PRICES (and explicitly
exclude "none") before calling getDevPlanCreditsLimit; if the check fails,
handle defensively by logging a warning/error and either set creditsLimit to a
safe default (e.g., 0) or abort the renewal flow to avoid computing NaN. Update
the code paths that use creditsLimit (the existing getDevPlanCreditsLimit call
and subsequent logic) to use the validated value or early-return on invalid
devPlan.
In `@apps/code/src/app/dashboard/page.tsx`:
- Around line 253-255: The remaining-percentage display can go negative when
usagePercentage > 100; update the JSX that renders "{(100 -
usagePercentage).toFixed(0)}% remaining this cycle" to clamp the computed value
to a minimum of 0 (e.g., compute remaining = Math.max(0, 100 - usagePercentage)
and render remaining.toFixed(0)), referring to the usagePercentage variable and
the paragraph that outputs the remaining percentage.
- Around line 157-162: The handleCopyApiKey handler calls
navigator.clipboard.writeText without error handling; wrap the await call in a
try-catch inside handleCopyApiKey (keeping the existing devPlanStatus?.apiKey
guard), toast.success on success and on failure catch the error, log it
(console.error or process logger) and call toast.error with a clear message like
"Failed to copy API key" (optionally include error.message); ensure the function
remains async and does not allow unhandled rejections.
♻️ Duplicate comments (8)
apps/worker/src/worker.ts (1)
690-719: Credit precision risk: avoidDecimal -> numberfor balance updates.Converting
DecimaltonumberviatoNumber()can introduce floating-point rounding errors for fractional costs. Use.toString()instead and let Postgres handle the numeric arithmetic precisely.Proposed fix (keep arithmetic in numeric/string form)
- const deductNumber = deductFromDevPlan.toNumber(); + const deductValue = deductFromDevPlan.toString(); await tx .update(organization) .set({ - devPlanCreditsUsed: sql`${organization.devPlanCreditsUsed} + ${deductNumber}`, + devPlanCreditsUsed: sql`${organization.devPlanCreditsUsed} + ${deductValue}::numeric`, }) .where(eq(organization.id, orgId)); ... - const costNumber = remainingCost.toNumber(); + const costValue = remainingCost.toString(); await tx .update(organization) .set({ - credits: sql`${organization.credits} - ${costNumber}`, + credits: sql`${organization.credits} - ${costValue}::numeric`, }) .where(eq(organization.id, orgId));apps/api/src/stripe.ts (1)
20-32: Centralize dev-plan pricing and verify max price (179).Same duplication/drift risk as
apps/api/src/routes/dev-plans.ts.apps/api/src/routes/dev-plans.ts (5)
16-23: Confirm dev-plan pricing (max=179) and centralize constants to avoid drift.This hardcodes
max: 179, which appears to contradict the PR objective description (and the same constants are duplicated inapps/api/src/stripe.ts). At minimum, verify the intended price and moveDEV_PLAN_PRICES/DevPlanTier/getDevPlanCreditsLimitto a shared module.#!/bin/bash set -euo pipefail # Find all dev plan pricing definitions/usages to ensure no drift. rg -n --hidden --glob '!.git/**' 'DEV_PLAN_PRICES|DevPlanTier|getDevPlanCreditsLimit|DEV_PLAN_CREDITS_MULTIPLIER' .Also applies to: 61-64
143-165: Wrap personal-org + membership + default-project creation in a DB transaction.These multi-step inserts aren’t atomic; a partial failure can leave an orphaned org (or org without a default project).
Also applies to: 241-266
299-315: Keep cancel_url query param spelling consistent (canceled vs cancelled).
cancel_urluses?canceled=true, but the codebase usesdevPlanCancelledelsewhere. Aligning spelling avoids brittle UI parsing / confusion.
391-404: Remove blocking sleep waiting for webhooks in cancel/resume.Sleeping in the request handler is unreliable and ties up capacity; the dashboard already polls
/status, so return immediately after Stripe update.Proposed fix
await stripe.subscriptions.update(personalOrg.devPlanStripeSubscriptionId, { cancel_at_period_end: true, }); - // Wait for webhook to process - await new Promise((resolve) => { - setTimeout(resolve, 3000); - }); - return c.json({ success: true, });await stripe.subscriptions.update(personalOrg.devPlanStripeSubscriptionId, { cancel_at_period_end: false, }); - // Wait for webhook to process - await new Promise((resolve) => { - setTimeout(resolve, 3000); - }); - return c.json({ success: true, });Also applies to: 468-491
579-604: Guard subscription.items access + validate currentTier instead of casting.
subscription.items.data[0].idcan throw if items are empty, andpersonalOrg.devPlan as DevPlanTieris fragile if it’s"none"or unexpected.Proposed fix
const subscription = await stripe.subscriptions.retrieve( personalOrg.devPlanStripeSubscriptionId, ); + const firstItem = subscription.items.data[0]; + if (!firstItem) { + throw new HTTPException(500, { message: "Invalid subscription: no items found" }); + } + + const currentTier = personalOrg.devPlan; + if (currentTier === "none" || !(currentTier in DEV_PLAN_PRICES)) { + throw new HTTPException(400, { message: "Current plan is not a valid dev plan tier" }); + } + // Update subscription with new tier await stripe.subscriptions.update(personalOrg.devPlanStripeSubscriptionId, { items: [ { - id: subscription.items.data[0].id, + id: firstItem.id, price: newPriceId, }, ], proration_behavior: "create_prorations", metadata: { ...subscription.metadata, devPlan: newTier, }, }); // Update local database immediately const newCreditsLimit = getDevPlanCreditsLimit(newTier); const isUpgrade = - DEV_PLAN_PRICES[newTier] > - DEV_PLAN_PRICES[personalOrg.devPlan as DevPlanTier]; + DEV_PLAN_PRICES[newTier] > DEV_PLAN_PRICES[currentTier];apps/code/src/app/dashboard/page.tsx (1)
66-74: Consider reducing polling frequency.A 5-second polling interval for dev plan status is aggressive given that status typically only changes during explicit user actions. Consider increasing to 30 seconds or removing automatic polling in favor of refetching after mutations complete.
🧹 Nitpick comments (7)
apps/worker/src/worker.ts (1)
669-672: Consider batching organization fetches to avoid N+1 queries.The organization query runs inside the loop over
orgCosts, which can have up toBATCH_SIZE(100) entries. Pre-fetching all relevant organizations in a single query would reduce database round-trips.Proposed optimization
+ // Pre-fetch all organizations that have costs to deduct + const orgIds = Array.from(orgCosts.keys()); + const orgsData = await tx.query.organization.findMany({ + where: { id: { in: orgIds } }, + }); + const orgsMap = new Map(orgsData.map((o) => [o.id, o])); + for (const [orgId, totalCost] of orgCosts.entries()) { if (totalCost.greaterThan(0)) { let remainingCost = totalCost; - // Fetch the organization to check for dev plan - const org = await tx.query.organization.findFirst({ - where: { id: { eq: orgId } }, - }); + const org = orgsMap.get(orgId); // First, try to deduct from dev plan credits if availableapps/code/src/app/layout.tsx (1)
17-20: Consider addingdisplay: "swap"for consistency.The
Interfont is configured withdisplay: "swap"butGeist_Monois not. Adding it would ensure consistent font-loading behavior and prevent invisible text during load.Suggested change
const geistMono = Geist_Mono({ variable: "--font-mono", subsets: ["latin"], + display: "swap", });.env.example (1)
59-59: Fix dotenv-linter UnorderedKey warnings (optional).This is low-impact, but it’ll keep
.env.exampleconsistent with the linter expectations.Also applies to: 201-208
apps/api/src/routes/dev-plans.ts (1)
24-59: Avoid returning inactive/deleted API keys from getOrCreatePersonalOrgApiKey.Right now
status != "deleted"can returninactivekeys and hand them out as the “Dev Plan API Key”. Consider filtering tostatus == "active"and (if you want to scope)description == "Dev Plan API Key"to avoid exposing unrelated keys.apps/gateway/src/chat/chat.ts (1)
1854-1871: Consider extracting totalAvailableCredits computation (and avoid float drift).The same “regular + dev plan remaining” math is duplicated 3 times and uses
parseFloaton decimal strings. A small helper (and/orDecimal) would reduce repetition and edge-case drift.Also applies to: 1961-1981, 2015-2028
apps/code/src/app/dashboard/page.tsx (2)
97-98: Consider logging errors for debugging.The caught errors are discarded, which makes debugging production issues difficult. Consider logging to console or capturing in PostHog for observability.
- } catch { + } catch (error) { + console.error("Subscribe failed:", error); toast.error("Failed to start subscription");
294-299: Add aria-label for accessibility.The show/hide toggle button lacks an accessible label for screen readers.
<Button variant="outline" onClick={() => setShowApiKey(!showApiKey)} + aria-label={showApiKey ? "Hide API key" : "Show API key"} > {showApiKey ? "Hide" : "Show"} </Button>
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
apps/admin/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsapps/code/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsapps/playground/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsapps/ui/src/lib/api/v1.d.tsis excluded by!**/v1.d.ts
📒 Files selected for processing (12)
.env.exampleapps/api/src/routes/dev-plans.tsapps/api/src/routes/subscriptions.tsapps/api/src/stripe.tsapps/code/src/app/dashboard/page.tsxapps/code/src/app/layout.tsxapps/code/src/app/page.tsxapps/gateway/src/chat/chat.tsapps/gateway/src/lib/rate-limit.spec.tsapps/worker/src/worker.tspackages/db/src/schema.tspackages/db/src/types.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/db/src/schema.ts
- packages/db/src/types.ts
- apps/api/src/routes/subscriptions.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never useanyoras anyunless absolutely necessary in TypeScript code
For database reads: Usedb().query.<table>.findMany()ordb().query.<table>.findFirst()
Files:
apps/worker/src/worker.tsapps/gateway/src/lib/rate-limit.spec.tsapps/gateway/src/chat/chat.tsapps/api/src/routes/dev-plans.tsapps/api/src/stripe.tsapps/code/src/app/layout.tsxapps/code/src/app/dashboard/page.tsxapps/code/src/app/page.tsx
**/*.{ts,tsx,js,jsx,json,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use tabs for indentation
Files:
apps/worker/src/worker.tsapps/gateway/src/lib/rate-limit.spec.tsapps/gateway/src/chat/chat.tsapps/api/src/routes/dev-plans.tsapps/api/src/stripe.tsapps/code/src/app/layout.tsxapps/code/src/app/dashboard/page.tsxapps/code/src/app/page.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Always use top-levelimport, never use require or dynamic imports
No unnecessary code comments
Files:
apps/worker/src/worker.tsapps/gateway/src/lib/rate-limit.spec.tsapps/gateway/src/chat/chat.tsapps/api/src/routes/dev-plans.tsapps/api/src/stripe.tsapps/code/src/app/layout.tsxapps/code/src/app/dashboard/page.tsxapps/code/src/app/page.tsx
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Always use top-level
import, never use require or dynamic imports
Files:
apps/worker/src/worker.tsapps/gateway/src/lib/rate-limit.spec.tsapps/gateway/src/chat/chat.tsapps/api/src/routes/dev-plans.tsapps/api/src/stripe.tsapps/code/src/app/layout.tsxapps/code/src/app/dashboard/page.tsxapps/code/src/app/page.tsx
**/*.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Unit tests should use
*.spec.tsfile naming convention
Files:
apps/gateway/src/lib/rate-limit.spec.ts
apps/{gateway,api}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Hono framework with Zod validation and OpenAPI documentation for backend APIs
Files:
apps/gateway/src/lib/rate-limit.spec.tsapps/gateway/src/chat/chat.tsapps/api/src/routes/dev-plans.tsapps/api/src/stripe.ts
{apps/api,apps/gateway,packages/db}/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
{apps/api,apps/gateway,packages/db}/**/*.ts: Use Drizzle ORM with latest object syntax for database operations
For database reads: Usedb().query.<table>.findMany()ordb().query.<table>.findFirst()
Files:
apps/gateway/src/lib/rate-limit.spec.tsapps/gateway/src/chat/chat.tsapps/api/src/routes/dev-plans.tsapps/api/src/stripe.ts
apps/{gateway,api}/src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
apps/{gateway,api}/src/**/*.ts: Use Hono for backend framework in Gateway and API services
Use Zod schemas for validation in Hono services
Files:
apps/gateway/src/lib/rate-limit.spec.tsapps/gateway/src/chat/chat.tsapps/api/src/routes/dev-plans.tsapps/api/src/stripe.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T12:42:14.219Z
Learning: Applies to apps/{ui,playground}/**/*.{ts,tsx} : Use Next.js App Router with React Server Components for frontend development
📚 Learning: 2025-12-03T12:42:14.219Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T12:42:14.219Z
Learning: Applies to apps/{ui,playground}/**/*.{ts,tsx} : Use Next.js App Router with React Server Components for frontend development
Applied to files:
apps/code/src/app/layout.tsxapps/code/src/app/dashboard/page.tsxapps/code/src/app/page.tsx
📚 Learning: 2025-12-03T12:42:26.162Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T12:42:26.162Z
Learning: Applies to apps/{ui,playground,docs}/**/*.{ts,tsx} : Use `next/link` for links and `next/navigation`'s router for programmatic navigation
Applied to files:
apps/code/src/app/layout.tsxapps/code/src/app/dashboard/page.tsxapps/code/src/app/page.tsx
📚 Learning: 2025-12-03T12:42:14.219Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T12:42:14.219Z
Learning: Applies to apps/{ui,playground}/**/*.{tsx} : Use `next/link` for links and `next/navigation`'s router for programmatic navigation
Applied to files:
apps/code/src/app/layout.tsxapps/code/src/app/page.tsx
🧬 Code graph analysis (6)
apps/worker/src/worker.ts (1)
packages/db/src/schema.ts (1)
organization(109-162)
apps/gateway/src/chat/chat.ts (2)
packages/db/src/schema.ts (1)
organization(109-162)packages/models/src/models.ts (1)
ModelDefinition(181-241)
apps/api/src/routes/dev-plans.ts (6)
packages/db/src/db.ts (1)
db(21-25)packages/db/src/schema.ts (4)
shortid(39-39)user(41-53)apiKey(291-318)project(264-289)packages/db/src/index.ts (1)
tables(14-16)apps/api/src/stripe.ts (1)
ensureStripeCustomer(34-71)apps/api/src/routes/payments.ts (1)
stripe(13-18)packages/logger/src/index.ts (2)
error(153-160)logger(181-181)
apps/code/src/app/layout.tsx (3)
apps/code/postcss.config.mjs (1)
config(1-3)apps/code/src/lib/config-server.ts (1)
getConfig(12-24)apps/code/src/components/providers.tsx (1)
Providers(21-69)
apps/code/src/app/dashboard/page.tsx (4)
apps/code/src/lib/auth-client.ts (1)
useAuth(18-31)apps/code/src/lib/config.tsx (1)
useAppConfig(25-31)apps/code/src/lib/fetch-client.ts (1)
useApi(20-26)apps/code/src/hooks/useUser.ts (1)
useUser(14-85)
apps/code/src/app/page.tsx (1)
apps/code/src/components/ui/button.tsx (1)
Button(60-60)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 59-59: [UnorderedKey] The CODE_URL key should go before the PLAYGROUND_URL key
(UnorderedKey)
[warning] 204-204: [UnorderedKey] The STRIPE_DEV_PLAN_MAX_PRICE_ID key should go before the STRIPE_DEV_PLAN_PRO_PRICE_ID key
(UnorderedKey)
🔇 Additional comments (16)
apps/worker/src/worker.ts (1)
662-667: LGTM!The introduction of
remainingCostand the updated comments clearly document the two-step deduction flow (dev plan credits first, then regular credits). Referral earnings correctly remain based on total cost, ensuring referrers receive consistent earnings regardless of how the referred org's costs are split.Also applies to: 722-723
apps/code/src/app/layout.tsx (4)
1-9: LGTM!Imports are well-organized with type imports separated, using top-level imports and path aliases consistently.
22-22: LGTM!Using
force-dynamicis appropriate here sincegetConfig()reads environment variables that need to be evaluated at runtime.
24-47: LGTM!Metadata is comprehensive with proper
metadataBaseconfiguration for OG image resolution. The versioned query params on assets are a good cache-busting strategy.
49-59: LGTM!The layout follows Next.js App Router conventions correctly. The
suppressHydrationWarningis appropriate given theThemeProviderinProvidersthat may cause attribute mismatches during hydration.apps/code/src/app/page.tsx (7)
1-5: LGTM!Imports are properly structured using top-level imports,
next/linkfor navigation, and lucide-react icons. This follows the coding guidelines and retrieved learnings for Next.js App Router patterns.
45-63: LGTM!Header navigation properly uses
next/linkfor client-side navigation. The CTA copy has been updated from "Free Trial" to "Get Started," addressing the earlier review feedback about potentially misleading messaging.
65-87: LGTM!Hero section is well-structured with clear messaging. The anchor link to
#pricingproperly connects to the pricing section'sidattribute on line 124.
89-122: LGTM!Features section accurately reflects the "3x your subscription price" multiplier mentioned in the PR objectives (
DEV_PLAN_CREDITS_MULTIPLIERdefault 3). Clean grid layout with good semantic structure.
134-177: LGTM!Pricing cards are well-implemented with proper key usage (
plan.tier), conditional styling for the popular plan, and query parameter passing to signup. The pattern of passing?plan=${plan.tier}enables pre-selection of the chosen plan on the signup page.
182-209: LGTM!CTA section maintains consistent messaging. The footer's dynamic year (
new Date().getFullYear()) is appropriate since this is a React Server Component, avoiding client-side re-evaluation concerns.
31-42: No action required—prices are already consistent.The Max plan is priced at $179 in both the frontend code (line 33) and backend configuration (stripe.ts and dev-plans.ts). There is no discrepancy to address.
Likely an incorrect or invalid review comment.
apps/gateway/src/lib/rate-limit.spec.ts (1)
81-116: Test fixture updates look consistent with the new org schema.Good expansion of the mocked organization shape to include dev-plan fields.
Also applies to: 135-170, 186-221, 304-339
apps/code/src/app/dashboard/page.tsx (3)
42-47: Verify Max plan price: PR description says $149, code shows $179.The PR objectives state
DEV_PLAN_PRICES (lite: 29, pro: 79, max: 149 as described), but the Max plan here is priced at $179. Please confirm which price is correct.
179-198: LGTM!The layout structure, header navigation with
next/link, and responsive design are well-implemented. The loading state handling provides good user feedback.
359-420: LGTM!The "No Active Plan" view with the subscription grid is well-structured. The "Most Popular" badge positioning and plan cards provide clear visual hierarchy for users to make subscription decisions.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| // Get API key if user has an active dev plan | ||
| let apiKey: string | null = null; | ||
| if (personalOrg.devPlan !== "none") { | ||
| // Find the default project for this org | ||
| const project = await db.query.project.findFirst({ | ||
| where: { | ||
| organizationId: { | ||
| eq: personalOrg.id, | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| if (project) { | ||
| apiKey = await getOrCreatePersonalOrgApiKey( | ||
| personalOrg.id, | ||
| project.id, | ||
| user.id, | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Avoid side effects in GET /status and make project selection deterministic.
/status auto-creates an API key (DB write) and findFirst for the project has no ordering, so which project gets an API key is nondeterministic. Consider:
- move API key creation to an explicit POST (or create on subscribe/webhook), and
- fetch the intended “Default Project” deterministically (e.g., by name or createdAt order).
Also applies to: 727-741
🤖 Prompt for AI Agents
In `@apps/api/src/routes/dev-plans.ts` around lines 706 - 725, The GET /status
handler currently performs a DB write by calling getOrCreatePersonalOrgApiKey
when personalOrg.devPlan !== "none" and uses db.query.project.findFirst without
ordering, causing nondeterministic project selection; change the handler to
remove side effects (do not create API keys on GET) and instead only read
existing keys, moving creation to an explicit POST endpoint (or
subscription/webhook flow) that calls getOrCreatePersonalOrgApiKey, and make
project lookup deterministic by replacing db.query.project.findFirst with a
deterministic query (e.g., findFirst with an orderBy createdAt or lookup by a
“Default Project” name/flag) so the code that currently references
personalOrg.devPlan, db.query.project.findFirst, and
getOrCreatePersonalOrgApiKey no longer performs writes on GET and selects a
predictable project.
| // Check if this is a dev plan subscription | ||
| const isDevPlan = metadata?.subscriptionType === "dev_plan"; | ||
| const devPlanTier = metadata?.devPlan as DevPlanTier | undefined; | ||
|
|
||
| logger.info( | ||
| `Found organization: ${organization.name} (${organization.id}), current plan: ${organization.plan}`, | ||
| `Found organization: ${organization.name} (${organization.id}), current plan: ${organization.plan}, isDevPlan: ${isDevPlan}`, | ||
| ); | ||
|
|
||
| // Update organization with subscription ID and upgrade to pro plan | ||
| try { | ||
| const subscriptionId = | ||
| typeof subscription === "string" ? subscription : subscription?.id; | ||
|
|
||
| const result = await db | ||
| .update(tables.organization) | ||
| .set({ | ||
| plan: "pro", | ||
| stripeSubscriptionId: subscriptionId, | ||
| subscriptionCancelled: false, | ||
| }) | ||
| .where(eq(tables.organization.id, organizationId)) | ||
| .returning(); | ||
| if (isDevPlan && devPlanTier) { | ||
| // Handle dev plan subscription | ||
| const creditsLimit = getDevPlanCreditsLimit(devPlanTier); | ||
|
|
||
| await db | ||
| .update(tables.organization) | ||
| .set({ | ||
| devPlan: devPlanTier, | ||
| devPlanCreditsLimit: creditsLimit.toString(), | ||
| devPlanCreditsUsed: "0", | ||
| devPlanBillingCycleStart: new Date(), | ||
| devPlanStripeSubscriptionId: subscriptionId, | ||
| devPlanCancelled: false, | ||
| }) | ||
| .where(eq(tables.organization.id, organizationId)); |
There was a problem hiding this comment.
Validate metadata.devPlan before using it (don’t cast untrusted strings).
const devPlanTier = metadata?.devPlan as DevPlanTier can turn an unexpected value into NaN credits and persist bad state. Validate metadata.devPlan is one of the known tiers before calling getDevPlanCreditsLimit.
🤖 Prompt for AI Agents
In `@apps/api/src/stripe.ts` around lines 281 - 304, The code unsafely casts
metadata.devPlan to DevPlanTier; validate metadata.devPlan against the allowed
DevPlanTier values before using it. In the block that sets isDevPlan and
devPlanTier, replace the unchecked cast by checking metadata?.devPlan is one of
the known tiers (e.g., via a Set/map or a type-guard function
isValidDevPlanTier(value): value is DevPlanTier) and only assign devPlanTier
when valid; if invalid, log/warn and skip calling getDevPlanCreditsLimit and the
DB update that writes devPlanCreditsLimit/related fields. Ensure
getDevPlanCreditsLimit is called only with a validated devPlanTier and that you
do not persist NaN or unexpected values.
| lineItems: [ | ||
| { | ||
| description: `Dev Plan ${devPlanTier.toUpperCase()} ($${creditsLimit} credits included)`, | ||
| amount: (session.amount_total || 0) / 100, | ||
| }, | ||
| ], |
There was a problem hiding this comment.
Invoice line item wording mixes currency and credits.
Dev Plan ... ($${creditsLimit} credits included) looks like dollars but it’s a credit limit; consider removing $ to avoid confusing invoices.
🤖 Prompt for AI Agents
In `@apps/api/src/stripe.ts` around lines 348 - 353, The invoice line item
description in the lineItems array mixes currency and credit units; update the
description construction (where description is built using devPlanTier and
creditsLimit) to remove the dollar sign and clearly indicate credits (e.g., "Dev
Plan ${devPlanTier.toUpperCase()} (${creditsLimit} credits included)") so it
doesn't read like a dollar amount; ensure only the description string is changed
and leave amount calculation (amount: (session.amount_total || 0) / 100)
untouched.
| if (isDevPlanRenewal) { | ||
| // Handle dev plan renewal - reset credits | ||
| const creditsLimit = getDevPlanCreditsLimit( | ||
| organization.devPlan as DevPlanTier, | ||
| ); | ||
|
|
There was a problem hiding this comment.
Defensively validate organization.devPlan before casting to DevPlanTier.
Even if “should not happen”, a DB inconsistency would currently compute NaN credits on renewal. Validate organization.devPlan in DEV_PLAN_PRICES (and handle "none").
🤖 Prompt for AI Agents
In `@apps/api/src/stripe.ts` around lines 1108 - 1113, Validate
organization.devPlan before casting to DevPlanTier: inside the isDevPlanRenewal
block, check that organization.devPlan is a string and that organization.devPlan
in DEV_PLAN_PRICES (and explicitly exclude "none") before calling
getDevPlanCreditsLimit; if the check fails, handle defensively by logging a
warning/error and either set creditsLimit to a safe default (e.g., 0) or abort
the renewal flow to avoid computing NaN. Update the code paths that use
creditsLimit (the existing getDevPlanCreditsLimit call and subsequent logic) to
use the validated value or early-return on invalid devPlan.
| // Send dev plan cancelled email | ||
| await sendTransactionalEmail({ | ||
| to: organization.billingEmail, | ||
| subject: "Your LLMGateway Dev Plan Has Been Cancelled", | ||
| html: generateSubscriptionCancelledEmailHtml(organization.name), | ||
| }); | ||
|
|
There was a problem hiding this comment.
Suppress email failures in webhook handlers to avoid repeated Stripe retries.
Unlike the dev-plan checkout invoice email, this cancellation email isn’t guarded; a transient email failure can cause Stripe to retry the webhook and re-run side effects.
🤖 Prompt for AI Agents
In `@apps/api/src/stripe.ts` around lines 1431 - 1437, The cancellation email send
isn't protected and can surface transient failures to Stripe; wrap the await
sendTransactionalEmail call (the call using sendTransactionalEmail with
generateSubscriptionCancelledEmailHtml and organization.billingEmail) in a
try/catch inside the webhook handler so any errors are caught, logged via
processLogger or similar, and not re-thrown — do not change the handler's
success flow or return value so Stripe sees the webhook as handled even if email
sending fails.
| const handleCopyApiKey = async () => { | ||
| if (devPlanStatus?.apiKey) { | ||
| await navigator.clipboard.writeText(devPlanStatus.apiKey); | ||
| toast.success("API key copied to clipboard"); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Add error handling for clipboard API.
navigator.clipboard.writeText can throw if permissions are denied or the API is unavailable. Wrap in try-catch to prevent unhandled rejections.
Suggested fix
const handleCopyApiKey = async () => {
if (devPlanStatus?.apiKey) {
- await navigator.clipboard.writeText(devPlanStatus.apiKey);
- toast.success("API key copied to clipboard");
+ try {
+ await navigator.clipboard.writeText(devPlanStatus.apiKey);
+ toast.success("API key copied to clipboard");
+ } catch {
+ toast.error("Failed to copy API key");
+ }
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleCopyApiKey = async () => { | |
| if (devPlanStatus?.apiKey) { | |
| await navigator.clipboard.writeText(devPlanStatus.apiKey); | |
| toast.success("API key copied to clipboard"); | |
| } | |
| }; | |
| const handleCopyApiKey = async () => { | |
| if (devPlanStatus?.apiKey) { | |
| try { | |
| await navigator.clipboard.writeText(devPlanStatus.apiKey); | |
| toast.success("API key copied to clipboard"); | |
| } catch { | |
| toast.error("Failed to copy API key"); | |
| } | |
| } | |
| }; |
🤖 Prompt for AI Agents
In `@apps/code/src/app/dashboard/page.tsx` around lines 157 - 162, The
handleCopyApiKey handler calls navigator.clipboard.writeText without error
handling; wrap the await call in a try-catch inside handleCopyApiKey (keeping
the existing devPlanStatus?.apiKey guard), toast.success on success and on
failure catch the error, log it (console.error or process logger) and call
toast.error with a clear message like "Failed to copy API key" (optionally
include error.message); ensure the function remains async and does not allow
unhandled rejections.
Personal orgs should always have plan='free' since they use the separate devPlan field for tracking coding plan subscriptions. This adds a safeguard in the checkout webhook handler. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/stripe.ts (1)
1556-1565:handleSubscriptionCreateddoesn't distinguish dev plan subscriptions.This handler unconditionally sets
plan: "pro"andstripeSubscriptionIdfor all subscriptions. For dev plan subscriptions, this will incorrectly overwrite the organization state set byhandleCheckoutSessionCompleted, assigningplan: "pro"when it should remain"free"(dev plans use separatedevPlan*fields).🛠️ Suggested fix
async function handleSubscriptionCreated( event: Stripe.CustomerSubscriptionCreatedEvent, ) { const subscription = event.data.object; const { customer, metadata } = subscription; logger.info( `Processing subscription created for customer: ${customer}, subscription: ${subscription.id}`, ); const result = await resolveOrganizationFromStripeEvent({ metadata: metadata as { organizationId?: string } | undefined, customer: typeof customer === "string" ? customer : customer?.id, subscription: subscription.id, }); if (!result) { logger.error( `Organization not found for customer: ${customer}, subscription: ${subscription.id}`, ); return; } const { organizationId, organization } = result; logger.info( `Found organization: ${organization.name} (${organization.id}) for subscription creation`, ); + // Skip for dev plan subscriptions - they are handled by handleCheckoutSessionCompleted + const isDevPlan = metadata?.subscriptionType === "dev_plan"; + if (isDevPlan) { + logger.info( + `Skipping subscription.created for dev plan subscription ${subscription.id} - handled by checkout.session.completed`, + ); + return; + } + + // Skip for personal orgs - they should use devPlan, not pro plan + if (organization.isPersonal) { + logger.warn( + `Skipping subscription.created for personal org ${organizationId} - personal orgs should use devPlan`, + ); + return; + } + try { await db .update(tables.organization) .set({ plan: "pro", stripeSubscriptionId: subscription.id, subscriptionCancelled: false, })
🤖 Fix all issues with AI agents
In `@apps/api/src/stripe.ts`:
- Around line 1490-1495: Wrap the call to sendTransactionalEmail (the one using
organization.billingEmail and
generateSubscriptionCancelledEmailHtml(organization.name)) in a try/catch so
email-send failures don’t bubble up and trigger Stripe webhook retries; on error
catch and log the failure (include the organization id/name and the error) but
do not rethrow—allow the webhook handler to complete successfully.
- Around line 1122-1133: The dev plan renewal branch currently always inserts a
new transaction record (tables.transaction) using stripeInvoiceId = invoice.id
and type "dev_plan_renewal", which can create duplicates on webhook retries;
mirror the pro plan branch by first querying for an existing transaction with
stripeInvoiceId === invoice.id (and optionally type === "dev_plan_renewal")
using the same db/query helper, and only perform the insert if no existing
record is found—if found, skip the insert or update status as appropriate to
maintain idempotency.
- Around line 1163-1178: The pro plan branch inserts a subscription_start
transaction unconditionally, causing duplicates on webhook retries; before
inserting in the else branch, query tables.transaction for an existing record
with stripeInvoiceId === invoice.id (and same organizationId) like
handleCheckoutSessionCompleted does, and if one exists skip the insert (or
return early), otherwise proceed to insert the transaction with the same fields
(organizationId, type: "subscription_start", amount, currency, status,
stripePaymentIntentId, stripeInvoiceId, description).
♻️ Duplicate comments (5)
apps/api/src/stripe.ts (5)
20-32: Extract shared dev plan pricing constants to avoid duplication.This code duplicates
DEV_PLAN_PRICES,DevPlanTier, andgetDevPlanCreditsLimitwhich also exist inapps/api/src/routes/dev-plans.ts. Consider extracting to a shared module to prevent drift when prices change.
281-283: Validatemetadata.devPlanbefore using it.The cast
metadata?.devPlan as DevPlanTiercan produce an invalid tier if Stripe metadata contains an unexpected value. While line 290 checksdevPlanTieris truthy, it doesn't validate it's a known tier, which would causegetDevPlanCreditsLimitto returnNaN.🛠️ Suggested validation
// Check if this is a dev plan subscription const isDevPlan = metadata?.subscriptionType === "dev_plan"; - const devPlanTier = metadata?.devPlan as DevPlanTier | undefined; + const rawDevPlan = metadata?.devPlan; + const devPlanTier: DevPlanTier | undefined = + rawDevPlan && rawDevPlan in DEV_PLAN_PRICES + ? (rawDevPlan as DevPlanTier) + : undefined;
348-353: Invoice line item wording mixes currency and credits.The description
($${creditsLimit} credits included)appears to show a dollar amount butcreditsLimitis a credit count. Remove the$to avoid confusing invoices.🔧 Suggested fix
- description: `Dev Plan ${devPlanTier.toUpperCase()} ($${creditsLimit} credits included)`, + description: `Dev Plan ${devPlanTier.toUpperCase()} (${creditsLimit} credits included)`,
1116-1121: Defensively validateorganization.devPlanbefore casting toDevPlanTier.If
organization.devPlanis"none"or an unexpected value due to database inconsistency,getDevPlanCreditsLimitwill computeNaNcredits. Validate before calling.🛠️ Suggested validation
if (isDevPlanRenewal) { // Handle dev plan renewal - reset credits + const currentDevPlan = organization.devPlan; + if (!currentDevPlan || !(currentDevPlan in DEV_PLAN_PRICES)) { + logger.error( + `Invalid devPlan "${currentDevPlan}" for organization ${organizationId} during renewal`, + ); + return; + } const creditsLimit = getDevPlanCreditsLimit( - organization.devPlan as DevPlanTier, + currentDevPlan as DevPlanTier, );
1439-1444: Wrap email send in try/catch to avoid webhook retry loops.Unlike the invoice email in
handleCheckoutSessionCompleted(lines 356-361), thissendTransactionalEmailcall is unguarded. A transient email failure will cause the webhook to fail and Stripe to retry, potentially re-running database side effects.🛠️ Suggested fix
// Send dev plan cancelled email + try { await sendTransactionalEmail({ to: organization.billingEmail, subject: "Your LLMGateway Dev Plan Has Been Cancelled", html: generateSubscriptionCancelledEmailHtml(organization.name), }); + } catch (e) { + logger.error( + "Dev plan cancellation email failed; suppressing webhook failure", + e as Error, + ); + }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/src/stripe.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never useanyoras anyunless absolutely necessary in TypeScript code
For database reads: Usedb().query.<table>.findMany()ordb().query.<table>.findFirst()
Files:
apps/api/src/stripe.ts
**/*.{ts,tsx,js,jsx,json,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use tabs for indentation
Files:
apps/api/src/stripe.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Always use top-levelimport, never use require or dynamic imports
No unnecessary code comments
Files:
apps/api/src/stripe.ts
apps/{gateway,api}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Hono framework with Zod validation and OpenAPI documentation for backend APIs
Files:
apps/api/src/stripe.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Always use top-level
import, never use require or dynamic imports
Files:
apps/api/src/stripe.ts
{apps/api,apps/gateway,packages/db}/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
{apps/api,apps/gateway,packages/db}/**/*.ts: Use Drizzle ORM with latest object syntax for database operations
For database reads: Usedb().query.<table>.findMany()ordb().query.<table>.findFirst()
Files:
apps/api/src/stripe.ts
apps/{gateway,api}/src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
apps/{gateway,api}/src/**/*.ts: Use Hono for backend framework in Gateway and API services
Use Zod schemas for validation in Hono services
Files:
apps/api/src/stripe.ts
🧬 Code graph analysis (1)
apps/api/src/stripe.ts (4)
packages/db/src/schema.ts (2)
organization(109-162)transaction(191-235)apps/api/src/utils/invoice.ts (1)
generateAndEmailInvoice(192-283)apps/api/src/posthog.ts (1)
posthog(3-6)apps/api/src/utils/email.ts (1)
sendTransactionalEmail(38-105)
⏰ 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). (17)
- GitHub Check: build-unified (linux/amd64)
- GitHub Check: build-split (gateway, linux/amd64)
- GitHub Check: build-split (ui, linux/amd64)
- GitHub Check: build-split (api, linux/amd64)
- GitHub Check: build-split (playground, linux/amd64)
- GitHub Check: build-split (admin, linux/amd64)
- GitHub Check: e2e-shards (5)
- GitHub Check: e2e-shards (1)
- GitHub Check: e2e-shards (4)
- GitHub Check: e2e-shards (3)
- GitHub Check: e2e-shards (2)
- GitHub Check: autofix
- GitHub Check: generate / run
- GitHub Check: test / run
- GitHub Check: lint / run
- GitHub Check: build / run
- GitHub Check: title
🔇 Additional comments (1)
apps/api/src/stripe.ts (1)
386-394: LGTM!Good safeguard to prevent personal orgs from being assigned the pro plan—aligns with the PR objective that personal orgs should use the
devPlanfield instead.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| // Create transaction record for dev plan renewal | ||
| await db.insert(tables.transaction).values({ | ||
| organizationId, | ||
| type: "subscription_start", | ||
| type: "dev_plan_renewal", | ||
| amount: (invoice.amount_paid / 100).toString(), | ||
| creditAmount: creditsLimit.toString(), | ||
| currency: invoice.currency.toUpperCase(), | ||
| status: "completed", | ||
| stripePaymentIntentId: (invoice as any).payment_intent, | ||
| stripeInvoiceId: invoice.id, | ||
| description: "Pro subscription started", | ||
| }) | ||
| .returning(); | ||
| description: `Dev Plan ${organization.devPlan?.toUpperCase()} renewed`, | ||
| }); |
There was a problem hiding this comment.
Missing duplicate transaction check for dev plan renewal.
Similar to the pro plan branch, the dev plan renewal inserts a transaction without checking if one already exists for this stripeInvoiceId. Webhook retries could create duplicate dev_plan_renewal transactions.
🛠️ Suggested fix
+ // Check for existing transaction to avoid duplicates on webhook retry
+ const existingTransaction = await db.query.transaction.findFirst({
+ where: {
+ stripeInvoiceId: {
+ eq: invoice.id,
+ },
+ },
+ });
+
+ if (existingTransaction) {
+ logger.info(
+ "Dev plan renewal transaction already exists for invoice; skipping",
+ { stripeInvoiceId: invoice.id },
+ );
+ return;
+ }
+
// Create transaction record for dev plan renewal
await db.insert(tables.transaction).values({📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Create transaction record for dev plan renewal | |
| await db.insert(tables.transaction).values({ | |
| organizationId, | |
| type: "subscription_start", | |
| type: "dev_plan_renewal", | |
| amount: (invoice.amount_paid / 100).toString(), | |
| creditAmount: creditsLimit.toString(), | |
| currency: invoice.currency.toUpperCase(), | |
| status: "completed", | |
| stripePaymentIntentId: (invoice as any).payment_intent, | |
| stripeInvoiceId: invoice.id, | |
| description: "Pro subscription started", | |
| }) | |
| .returning(); | |
| description: `Dev Plan ${organization.devPlan?.toUpperCase()} renewed`, | |
| }); | |
| // Check for existing transaction to avoid duplicates on webhook retry | |
| const existingTransaction = await db.query.transaction.findFirst({ | |
| where: { | |
| stripeInvoiceId: { | |
| eq: invoice.id, | |
| }, | |
| }, | |
| }); | |
| if (existingTransaction) { | |
| logger.info( | |
| "Dev plan renewal transaction already exists for invoice; skipping", | |
| { stripeInvoiceId: invoice.id }, | |
| ); | |
| return; | |
| } | |
| // Create transaction record for dev plan renewal | |
| await db.insert(tables.transaction).values({ | |
| organizationId, | |
| type: "dev_plan_renewal", | |
| amount: (invoice.amount_paid / 100).toString(), | |
| creditAmount: creditsLimit.toString(), | |
| currency: invoice.currency.toUpperCase(), | |
| status: "completed", | |
| stripePaymentIntentId: (invoice as any).payment_intent, | |
| stripeInvoiceId: invoice.id, | |
| description: `Dev Plan ${organization.devPlan?.toUpperCase()} renewed`, | |
| }); |
🤖 Prompt for AI Agents
In `@apps/api/src/stripe.ts` around lines 1122 - 1133, The dev plan renewal branch
currently always inserts a new transaction record (tables.transaction) using
stripeInvoiceId = invoice.id and type "dev_plan_renewal", which can create
duplicates on webhook retries; mirror the pro plan branch by first querying for
an existing transaction with stripeInvoiceId === invoice.id (and optionally type
=== "dev_plan_renewal") using the same db/query helper, and only perform the
insert if no existing record is found—if found, skip the insert or update status
as appropriate to maintain idempotency.
| } else { | ||
| // Handle regular pro plan subscription | ||
| // Create transaction record for subscription start | ||
| const [transaction] = await db | ||
| .insert(tables.transaction) | ||
| .values({ | ||
| organizationId, | ||
| type: "subscription_start", | ||
| amount: (invoice.amount_paid / 100).toString(), | ||
| currency: invoice.currency.toUpperCase(), | ||
| status: "completed", | ||
| stripePaymentIntentId: (invoice as any).payment_intent, | ||
| stripeInvoiceId: invoice.id, | ||
| description: "Pro subscription started", | ||
| }) | ||
| .returning(); |
There was a problem hiding this comment.
Missing duplicate transaction check for invoice payment.
Unlike handleCheckoutSessionCompleted (lines 411-420) which checks for existing transactions by stripeInvoiceId before inserting, this pro plan branch inserts unconditionally. If Stripe retries the webhook, duplicate subscription_start transactions will be created.
🛠️ Suggested fix
+ // Check for existing transaction to avoid duplicates on webhook retry
+ const existingTransaction = await db.query.transaction.findFirst({
+ where: {
+ stripeInvoiceId: {
+ eq: invoice.id,
+ },
+ },
+ });
+
+ if (existingTransaction) {
+ logger.info(
+ "Subscription transaction already exists for invoice; skipping duplicate insert",
+ { stripeInvoiceId: invoice.id },
+ );
+ return;
+ }
+
// Create transaction record for subscription start
const [transaction] = await db
.insert(tables.transaction)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } else { | |
| // Handle regular pro plan subscription | |
| // Create transaction record for subscription start | |
| const [transaction] = await db | |
| .insert(tables.transaction) | |
| .values({ | |
| organizationId, | |
| type: "subscription_start", | |
| amount: (invoice.amount_paid / 100).toString(), | |
| currency: invoice.currency.toUpperCase(), | |
| status: "completed", | |
| stripePaymentIntentId: (invoice as any).payment_intent, | |
| stripeInvoiceId: invoice.id, | |
| description: "Pro subscription started", | |
| }) | |
| .returning(); | |
| } else { | |
| // Handle regular pro plan subscription | |
| // Check for existing transaction to avoid duplicates on webhook retry | |
| const existingTransaction = await db.query.transaction.findFirst({ | |
| where: { | |
| stripeInvoiceId: { | |
| eq: invoice.id, | |
| }, | |
| }, | |
| }); | |
| if (existingTransaction) { | |
| logger.info( | |
| "Subscription transaction already exists for invoice; skipping duplicate insert", | |
| { stripeInvoiceId: invoice.id }, | |
| ); | |
| return; | |
| } | |
| // Create transaction record for subscription start | |
| const [transaction] = await db | |
| .insert(tables.transaction) | |
| .values({ | |
| organizationId, | |
| type: "subscription_start", | |
| amount: (invoice.amount_paid / 100).toString(), | |
| currency: invoice.currency.toUpperCase(), | |
| status: "completed", | |
| stripePaymentIntentId: (invoice as any).payment_intent, | |
| stripeInvoiceId: invoice.id, | |
| description: "Pro subscription started", | |
| }) | |
| .returning(); |
🤖 Prompt for AI Agents
In `@apps/api/src/stripe.ts` around lines 1163 - 1178, The pro plan branch inserts
a subscription_start transaction unconditionally, causing duplicates on webhook
retries; before inserting in the else branch, query tables.transaction for an
existing record with stripeInvoiceId === invoice.id (and same organizationId)
like handleCheckoutSessionCompleted does, and if one exists skip the insert (or
return early), otherwise proceed to insert the transaction with the same fields
(organizationId, type: "subscription_start", amount, currency, status,
stripePaymentIntentId, stripeInvoiceId, description).
| // Send subscription cancelled email | ||
| await sendTransactionalEmail({ | ||
| to: organization.billingEmail, | ||
| subject: "Your LLMGateway Subscription Has Been Cancelled", | ||
| html: generateSubscriptionCancelledEmailHtml(organization.name), | ||
| }); |
There was a problem hiding this comment.
Wrap email send in try/catch to avoid webhook retry loops.
Same issue as the dev plan cancellation email—transient email failures will cause Stripe webhook retries.
🛠️ Suggested fix
// Send subscription cancelled email
+ try {
await sendTransactionalEmail({
to: organization.billingEmail,
subject: "Your LLMGateway Subscription Has Been Cancelled",
html: generateSubscriptionCancelledEmailHtml(organization.name),
});
+ } catch (e) {
+ logger.error(
+ "Subscription cancellation email failed; suppressing webhook failure",
+ e as Error,
+ );
+ }🤖 Prompt for AI Agents
In `@apps/api/src/stripe.ts` around lines 1490 - 1495, Wrap the call to
sendTransactionalEmail (the one using organization.billingEmail and
generateSubscriptionCancelledEmailHtml(organization.name)) in a try/catch so
email-send failures don’t bubble up and trigger Stripe webhook retries; on error
catch and log the failure (include the organization id/name and the error) but
do not rethrow—allow the webhook handler to complete successfully.
…s-subscription-ime9td
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/worker/src/worker.ts`:
- Around line 679-715: The current read-then-write on
organization.devPlanCreditsUsed (via tx.query.organization.findFirst and then
tx.update(organization).set(...)) has a TOCTOU race; replace it with a single
atomic update so deduction and bounds-check happen in the DB. Implement an
atomic UPDATE on organization that sets devPlanCreditsUsed = devPlanCreditsUsed
+ LEAST(:remainingCost, GREATEST(devPlanCreditsLimit - devPlanCreditsUsed, 0))
(or equivalent SQL using LEAST/GREATEST) and add a WHERE clause for id = :orgId
and devPlan != 'none' and devPlanCreditsUsed < devPlanCreditsLimit; use the
number of affected rows and the computed new/old values returned (or a SELECT
... RETURNING) to determine how much was actually deducted and update
remainingCost accordingly, removing the separate tx.query.organization.findFirst
and the two-step calculation.
♻️ Duplicate comments (2)
apps/worker/src/worker.ts (2)
700-706: Credit precision risk: avoidDecimal -> numberfor balance updates.This concern was raised in a previous review but remains unaddressed.
toNumber()can introduce floating-point rounding errors, especially with fractional costs. Prefer keeping arithmetic in string/Decimal form for Postgres numeric math.♻️ Proposed fix (keep arithmetic in numeric/string form)
- const deductNumber = deductFromDevPlan.toNumber(); + const deductValue = deductFromDevPlan.toString(); await tx .update(organization) .set({ - devPlanCreditsUsed: sql`${organization.devPlanCreditsUsed} + ${deductNumber}`, + devPlanCreditsUsed: sql`${organization.devPlanCreditsUsed} + ${deductValue}::numeric`, }) .where(eq(organization.id, orgId));
717-730: Same precision concern for regular credits deduction.Consistent with the dev plan deduction, use
.toString()instead of.toNumber()for the regular credits update to maintain numeric precision.♻️ Proposed fix
if (remainingCost.greaterThan(0)) { - const costNumber = remainingCost.toNumber(); + const costValue = remainingCost.toString(); await tx .update(organization) .set({ - credits: sql`${organization.credits} - ${costNumber}`, + credits: sql`${organization.credits} - ${costValue}::numeric`, }) .where(eq(organization.id, orgId)); logger.debug( - `Deducted ${costNumber} regular credits from organization ${orgId}`, + `Deducted ${costValue} regular credits from organization ${orgId}`, ); }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/worker/src/worker.tspackages/db/migrations/1768492522_chilly_makkari.sqlpackages/db/migrations/meta/1768492522_snapshot.jsonpackages/db/migrations/meta/_journal.jsonpackages/db/src/schema.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/db/src/schema.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx,json,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use tabs for indentation
Files:
packages/db/migrations/meta/_journal.jsonapps/worker/src/worker.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never useanyoras anyunless absolutely necessary in TypeScript code
For database reads: Usedb().query.<table>.findMany()ordb().query.<table>.findFirst()
Files:
apps/worker/src/worker.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Always use top-levelimport, never use require or dynamic imports
No unnecessary code comments
Files:
apps/worker/src/worker.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Always use top-level
import, never use require or dynamic imports
Files:
apps/worker/src/worker.ts
🧬 Code graph analysis (1)
apps/worker/src/worker.ts (1)
packages/db/src/schema.ts (1)
organization(109-162)
🔇 Additional comments (3)
packages/db/migrations/meta/_journal.json (1)
571-578: LGTM!The journal entry correctly references the new migration with the proper sequential index and matching tag.
packages/db/migrations/1768492522_chilly_makkari.sql (1)
1-9: LGTM! Migration is well-structured and backward compatible.The schema additions are sensible:
- Appropriate use of
numericfor credit fields (decimal precision)- UNIQUE constraint on
dev_plan_stripe_subscription_idcorrectly allows multiple NULLs- Reasonable defaults ensure backward compatibility for existing rows
- Timestamp columns align with existing schema pattern (plain
timestampwithout timezone is used consistently across the codebase)apps/worker/src/worker.ts (1)
732-733: LGTM!The comment correctly documents that referral earnings are calculated from the total cost (sum of both dev plan and regular credit deductions), which aligns with the implementation using
totalCoston line 741.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| // Fetch the organization to check for dev plan | ||
| const org = await tx.query.organization.findFirst({ | ||
| where: { id: { eq: orgId } }, | ||
| }); | ||
|
|
||
| // First, try to deduct from dev plan credits if available | ||
| if (org && org.devPlan !== "none") { | ||
| const devPlanCreditsLimit = new Decimal( | ||
| org.devPlanCreditsLimit || "0", | ||
| ); | ||
| const devPlanCreditsUsed = new Decimal( | ||
| org.devPlanCreditsUsed || "0", | ||
| ); | ||
| const devPlanRemaining = | ||
| devPlanCreditsLimit.minus(devPlanCreditsUsed); | ||
|
|
||
| if (devPlanRemaining.greaterThan(0)) { | ||
| const deductFromDevPlan = Decimal.min( | ||
| remainingCost, | ||
| devPlanRemaining, | ||
| ); | ||
| const deductNumber = deductFromDevPlan.toNumber(); | ||
|
|
||
| await tx | ||
| .update(organization) | ||
| .set({ | ||
| devPlanCreditsUsed: sql`${organization.devPlanCreditsUsed} + ${deductNumber}`, | ||
| }) | ||
| .where(eq(organization.id, orgId)); | ||
|
|
||
| logger.debug( | ||
| `Deducted ${deductNumber} dev plan credits from organization ${orgId}`, | ||
| ); | ||
|
|
||
| remainingCost = remainingCost.minus(deductFromDevPlan); | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential race condition on dev plan credits.
The pattern of reading devPlanCreditsUsed, calculating available credits, then updating introduces a TOCTOU (time-of-check to time-of-use) race. If two workers process logs for the same organization concurrently, both may read the same devPlanCreditsUsed value, calculate the full remaining budget is available, and both attempt to deduct—potentially over-spending the dev plan credit limit.
Consider adding a FOR UPDATE lock when fetching the organization, or using a single atomic SQL statement that performs the min calculation and update together.
🔒 Option 1: Add row-level lock when fetching organization
// Fetch the organization to check for dev plan
- const org = await tx.query.organization.findFirst({
- where: { id: { eq: orgId } },
- });
+ const [org] = await tx
+ .select()
+ .from(organization)
+ .where(eq(organization.id, orgId))
+ .for("update");🔒 Option 2: Atomic update with LEAST/GREATEST in SQL
+ // Atomically deduct from dev plan credits, capping at available balance
+ if (org && org.devPlan !== "none") {
+ const costValue = remainingCost.toString();
+ const result = await tx
+ .update(organization)
+ .set({
+ devPlanCreditsUsed: sql`${organization.devPlanCreditsUsed} + LEAST(
+ ${costValue}::numeric,
+ GREATEST(${organization.devPlanCreditsLimit} - ${organization.devPlanCreditsUsed}, 0)
+ )`,
+ })
+ .where(eq(organization.id, orgId))
+ .returning({
+ deducted: sql<string>`LEAST(
+ ${costValue}::numeric,
+ GREATEST(${organization.devPlanCreditsLimit} - ${organization.devPlanCreditsUsed}, 0)
+ )`,
+ });
+ // Use result[0].deducted to update remainingCost
+ }🤖 Prompt for AI Agents
In `@apps/worker/src/worker.ts` around lines 679 - 715, The current
read-then-write on organization.devPlanCreditsUsed (via
tx.query.organization.findFirst and then tx.update(organization).set(...)) has a
TOCTOU race; replace it with a single atomic update so deduction and
bounds-check happen in the DB. Implement an atomic UPDATE on organization that
sets devPlanCreditsUsed = devPlanCreditsUsed + LEAST(:remainingCost,
GREATEST(devPlanCreditsLimit - devPlanCreditsUsed, 0)) (or equivalent SQL using
LEAST/GREATEST) and add a WHERE clause for id = :orgId and devPlan != 'none' and
devPlanCreditsUsed < devPlanCreditsLimit; use the number of affected rows and
the computed new/old values returned (or a SELECT ... RETURNING) to determine
how much was actually deducted and update remainingCost accordingly, removing
the separate tx.query.organization.findFirst and the two-step calculation.
…s-subscription-ime9td
Prevent negative percentage display when usage exceeds 100%. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add code builder and runtime stages to split.dockerfile - Add code port 3004 to unified.dockerfile - Add code program to supervisord.conf - Add code service to docker-compose files (split and unified) - Add code to images.yml workflow matrix - Add code endpoint to test-split-docker.sh and test-unified-docker.sh - Add code app to .github/start.sh smoke test 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@infra/docker-compose.unified.local.yml`:
- Line 13: Update the ORIGIN_URLS default in docker-compose.unified.local.yml to
include the code service on port 3004 so CORS defaults match the hardcoded
fallbacks in apps/gateway/src/app.ts and apps/api/src/index.ts; specifically
modify the ORIGIN_URLS environment variable default (symbol: ORIGIN_URLS) to
include http://localhost:3004 alongside the existing entries so that overridden
ORIGIN_URLS values won’t omit the code origin.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.github/start.sh.github/test-split-docker.sh.github/test-unified-docker.sh.github/workflows/images.ymlapps/code/src/app/dashboard/page.tsxinfra/docker-compose.split.local.ymlinfra/docker-compose.split.ymlinfra/docker-compose.unified.local.ymlinfra/split.dockerfileinfra/supervisord.confinfra/unified.dockerfile
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/code/src/app/dashboard/page.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-03T12:42:26.162Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T12:42:26.162Z
Learning: Run `pnpm build` to ensure production builds work
Applied to files:
infra/split.dockerfile
📚 Learning: 2025-12-03T12:42:14.219Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T12:42:14.219Z
Learning: Run `pnpm build` to ensure production builds work after code changes
Applied to files:
infra/split.dockerfile
🪛 Hadolint (2.14.0)
infra/split.dockerfile
[error] 120-120: COPY with more than 2 arguments requires the last argument to end with /
(DL3021)
[error] 121-121: COPY with more than 2 arguments requires the last argument to end with /
(DL3021)
⏰ 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). (19)
- GitHub Check: build-split (admin, linux/amd64)
- GitHub Check: build-split (ui, linux/amd64)
- GitHub Check: build-split (code, linux/amd64)
- GitHub Check: build-split (api, linux/amd64)
- GitHub Check: build-split (gateway, linux/amd64)
- GitHub Check: build-split (docs, linux/amd64)
- GitHub Check: build-split (worker, linux/amd64)
- GitHub Check: build-split (playground, linux/amd64)
- GitHub Check: build-unified (linux/amd64)
- GitHub Check: test / run
- GitHub Check: build / run
- GitHub Check: lint / run
- GitHub Check: generate / run
- GitHub Check: e2e-shards (5)
- GitHub Check: e2e-shards (4)
- GitHub Check: e2e-shards (3)
- GitHub Check: e2e-shards (2)
- GitHub Check: e2e-shards (1)
- GitHub Check: autofix
🔇 Additional comments (11)
infra/unified.dockerfile (1)
123-123: LGTM!The addition of port 3004 for the Code service is consistent with the other infrastructure changes in this PR and follows the existing port numbering scheme.
.github/start.sh (1)
11-11: LGTM!The "code" app entry and its port mapping (3004) are correctly added and consistent with the infrastructure changes across this PR.
Also applies to: 19-19
.github/test-unified-docker.sh (1)
25-25: LGTM!The Code service endpoint is correctly configured for health checks and integration testing, following the same pattern as other services.
.github/workflows/images.yml (1)
59-59: LGTM!The "code" app is consistently added to all three CI jobs (build-split, merge-split, test-split), ensuring complete CI coverage for the new Code service image. The
split.dockerfilecontains the requiredcodetarget stage at line 264, confirming the build will succeed.Also applies to: 130-130, 221-221
infra/supervisord.conf (1)
76-86: LGTM!The new
[program:code]section follows the established pattern for other Next.js apps (ui, playground, docs, admin) with consistent configuration for autostart, autorestart, logging, and environment variables. Port 3004 aligns with the other docker-compose configurations in this PR.infra/split.dockerfile (2)
117-124: LGTM! Code builder stage follows established patterns.The code-builder stage is consistent with other builder stages (api-builder, ui-builder, etc.).
Regarding the Hadolint warnings (DL3021) on lines 120-121: these are false positives. The
--parentsflag changesCOPYsemantics to preserve directory structure, making the destination.valid. The same pattern is used throughout this Dockerfile for all other builder stages (e.g., lines 57-58, 66-67, 75-76).
263-278: LGTM!The code runtime stage mirrors the structure of other Next.js app runtime stages (ui, playground, docs, admin) with proper standalone output copying, port exposure, and environment configuration.
infra/docker-compose.split.yml (1)
209-237: LGTM!The new code service follows the established pattern for frontend services in this compose file. The configuration is consistent with similar services (ui, playground, docs) including healthcheck, networking, and environment variables.
.github/test-split-docker.sh (2)
21-21: LGTM!The code endpoint addition is consistent with the port configuration (3004) used across all other infrastructure files in this PR.
169-171: LGTM!The code service override follows the same pattern as other services in the heredoc.
infra/docker-compose.split.local.yml (1)
232-263: LGTM!The code service configuration for local development is well-structured and consistent with other frontend services in this file. The build target
codecorrectly references the runtime stage defined insplit.dockerfile, and the environment variables align with the production compose file.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| ports: | ||
| - "3002:3002" # UI | ||
| - "3003:3003" # Playground | ||
| - "3004:3004" # Code |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the docker-compose file to understand the context
cat -n infra/docker-compose.unified.local.yml | head -50Repository: theopenco/llmgateway
Length of output: 2230
🏁 Script executed:
# Search for ORIGIN_URLS and CORS-related configurations
rg -n "ORIGIN_URLS|allowedOrigins|cors" --type yaml --type ts --type json -C 2 | head -80Repository: theopenco/llmgateway
Length of output: 4424
🏁 Script executed:
# Check if port 3004 is mentioned anywhere else in the codebase
rg -n "3004" --type yaml --type ts --type json -g '!node_modules' | head -40Repository: theopenco/llmgateway
Length of output: 989
Add http://localhost:3004 to ORIGIN_URLS for consistency.
The port mapping and CODE_URL follow the established patterns. However, the ORIGIN_URLS default on line 28 does not include http://localhost:3004, even though the code's CORS middleware (in apps/gateway/src/app.ts and apps/api/src/index.ts) includes it in hardcoded fallback defaults. This inconsistency could cause CORS failures if ORIGIN_URLS is explicitly overridden without including port 3004.
Update line 28 to: - ORIGIN_URLS=${ORIGIN_URLS:-http://localhost:3002,http://localhost:3003,http://localhost:3004,http://localhost:4002}
🤖 Prompt for AI Agents
In `@infra/docker-compose.unified.local.yml` at line 13, Update the ORIGIN_URLS
default in docker-compose.unified.local.yml to include the code service on port
3004 so CORS defaults match the hardcoded fallbacks in apps/gateway/src/app.ts
and apps/api/src/index.ts; specifically modify the ORIGIN_URLS environment
variable default (symbol: ORIGIN_URLS) to include http://localhost:3004
alongside the existing entries so that overridden ORIGIN_URLS values won’t omit
the code origin.
…ion-ime9td' into terragon/add-dev-plans-subscription-ime9td
- Move DEV_PLAN_PRICES, DevPlanTier, and getDevPlanCreditsLimit to @llmgateway/shared for reuse across api and stripe modules - Add getOrCreatePersonalOrg helper with transaction atomicity to prevent partial state on org creation failures - Simplify dev-plans.ts handlers by using shared helpers 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
Changes
Backend API
Stripe & Payments
Database / Models
API/UI Routing
Frontend (Code app) – Dev Plans UI scaffolding
UI / UX
Admin / Security tweaks
Code App UI Artifacts
How to set up / migrate
Testing plan
Notes
🌿 Generated by Terry
📎 Task: https://www.terragonlabs.com/task/63361f9f-9494-46d5-b6f3-1b2a453ac902
Summary by CodeRabbit
New Features
Behavior Changes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.