Review fixes

Author: jahooma

packages/billing/src/subscription-webhooks.ts

@@ -13,6 +13,17 @@ import type { Logger } from '@codebuff/common/types/contracts/logger'
 import type { PlanConfig } from '@codebuff/common/constants/subscription-plans'
 import type Stripe from 'stripe'
 
+type SubscriptionStatus = (typeof schema.subscriptionStatusEnum.enumValues)[number]
+
+/**
+ * Maps a Stripe subscription status to our local enum.
+ */
+function mapStripeStatus(status: Stripe.Subscription.Status): SubscriptionStatus {
+  if (status === 'past_due') return 'past_due'
+  if (status === 'canceled') return 'canceled'
+  return 'active'
+}
+
 /**
  * Looks up a user ID by Stripe customer ID.
  */
@@ -137,7 +148,7 @@ export async function handleSubscriptionInvoicePaid(params: {
       target: schema.subscription.stripe_subscription_id,
       set: {
         status: 'active',
-        user_id: userId,
+        ...(userId ? { user_id: userId } : {}),
         stripe_price_id: priceId,
         plan_name: plan.name,
         billing_period_start: new Date(
@@ -250,6 +261,8 @@ export async function handleSubscriptionUpdated(params: {
       : stripeSubscription.customer.id
   const userId = await getUserIdByCustomerId(customerId)
 
+  const status = mapStripeStatus(stripeSubscription.status)
+
   // Upsert — webhook ordering is not guaranteed by Stripe, so this event
   // may arrive before invoice.paid creates the row.
   await db
@@ -260,6 +273,7 @@ export async function handleSubscriptionUpdated(params: {
       user_id: userId,
       stripe_price_id: priceId,
       plan_name: planName,
+      status,
       cancel_at_period_end: stripeSubscription.cancel_at_period_end,
       billing_period_start: new Date(
         stripeSubscription.current_period_start * 1000,
@@ -271,9 +285,10 @@ export async function handleSubscriptionUpdated(params: {
     .onConflictDoUpdate({
       target: schema.subscription.stripe_subscription_id,
       set: {
-        user_id: userId,
+        ...(userId ? { user_id: userId } : {}),
         stripe_price_id: priceId,
         plan_name: planName,
+        status,
         cancel_at_period_end: stripeSubscription.cancel_at_period_end,
         billing_period_start: new Date(
           stripeSubscription.current_period_start * 1000,

packages/billing/src/subscription.ts

@@ -537,26 +537,26 @@ export async function handleSubscribe(params: {
   logger: Logger
 }): Promise<void> {
   const { userId, stripeSubscription, logger } = params
-
-  // Idempotency: skip if this subscription was already processed
-  const existing = await db
-    .select({ stripe_subscription_id: schema.subscription.stripe_subscription_id })
-    .from(schema.subscription)
-    .where(eq(schema.subscription.stripe_subscription_id, stripeSubscription.id))
-    .limit(1)
-
-  if (existing.length > 0) {
-    logger.info(
-      { userId, subscriptionId: stripeSubscription.id },
-      'Subscription already processed — skipping handleSubscribe',
-    )
-    return
-  }
-
   const newResetDate = new Date(stripeSubscription.current_period_end * 1000)
 
   await withAdvisoryLockTransaction({
     callback: async (tx) => {
+      // Idempotency: skip if this subscription was already processed
+      // Must be inside the lock to prevent TOCTOU races on concurrent webhooks
+      const existing = await tx
+        .select({ stripe_subscription_id: schema.subscription.stripe_subscription_id })
+        .from(schema.subscription)
+        .where(eq(schema.subscription.stripe_subscription_id, stripeSubscription.id))
+        .limit(1)
+
+      if (existing.length > 0) {
+        logger.info(
+          { userId, subscriptionId: stripeSubscription.id },
+          'Subscription already processed — skipping handleSubscribe',
+        )
+        return
+      }
+
       // Move next_quota_reset and bump subscription_count
       await tx
         .update(schema.user)
@@ -652,7 +652,7 @@ async function migrateUnusedCredits(params: {
   }
 
   // Create a single migration grant preserving the total
-  const operationId = `migration-${userId}-${Date.now()}`
+  const operationId = `migration-${userId}-${crypto.randomUUID()}`
   await tx
     .insert(schema.creditLedger)
     .values({

web/src/app/api/stripe/webhook/route.ts

@@ -251,9 +251,9 @@ async function handleSubscriptionEvent(subscription: Stripe.Subscription) {
   )
 
   if (!organizationId) {
-    logger.warn(
+    logger.debug(
       { subscriptionId: subscription.id },
-      'Subscription event received without organization_id in metadata',
+      'Subscription event received without organization_id in metadata (user subscription)',
     )
     return
   }