fix(billing): drop priorUsage re-arm to make dedup a single atomic claim (no duplicate-email race)

Author: waleedlatif1

apps/sim/lib/billing/core/limit-notifications.test.ts

@@ -118,28 +118,12 @@ describe('maybeSendLimitThresholdEmail', () => {
     expect(sendEmailSpy).not.toHaveBeenCalled()
   })
 
-  it('re-arms then claims when a single jump crosses from below the band past 80% (priorUsage)', async () => {
-    // prior 50% (re-arm band) → current 90%: re-arm (update) + claim (update) = 2 updates, then send.
-    await maybeSendLimitThresholdEmail({
-      ...baseUserParams,
-      currentUsage: 4.5,
-      limit: 5,
-      priorUsage: 2.5,
-    })
-    expect(dbUpdateSpy).toHaveBeenCalledTimes(2)
-    expect(sendEmailSpy).toHaveBeenCalledTimes(1)
-    expect(renderMock).toHaveBeenCalledWith(expect.objectContaining({ kind: 'warning' }))
-  })
-
-  it('does not re-arm when prior usage was already in-band (no spurious reset)', async () => {
-    // prior 85% and current 90%: both >= re-arm band → claim only, no re-arm update.
-    await maybeSendLimitThresholdEmail({
-      ...baseUserParams,
-      currentUsage: 4.5,
-      limit: 5,
-      priorUsage: 4.25,
-    })
+  it('claims without re-arming on a crossing (re-arm and claim are mutually exclusive)', async () => {
+    // 90% crossing: a single claim update, no re-arm — so there is no re-arm/claim
+    // interleaving for a concurrent caller to exploit into a duplicate email.
+    await maybeSendLimitThresholdEmail({ ...baseUserParams, currentUsage: 4.5, limit: 5 })
     expect(dbUpdateSpy).toHaveBeenCalledTimes(1)
+    expect(mockClaim).toHaveBeenCalledTimes(1)
     expect(sendEmailSpy).toHaveBeenCalledTimes(1)
   })
 
@@ -185,18 +169,6 @@ describe('maybeSendLimitThresholdEmail', () => {
     expect(sendEmailSpy).not.toHaveBeenCalled()
   })
 
-  it('re-arms then sends on wipe-then-rebuild (priorUsage 0)', async () => {
-    // prior 0% (empty table) → current 90%: re-arm + claim + send.
-    await maybeSendLimitThresholdEmail({
-      ...baseUserParams,
-      currentUsage: 4.5,
-      limit: 5,
-      priorUsage: 0,
-    })
-    expect(dbUpdateSpy).toHaveBeenCalledTimes(2)
-    expect(sendEmailSpy).toHaveBeenCalledTimes(1)
-  })
-
   it('skips when the limit is non-positive', async () => {
     await maybeSendLimitThresholdEmail({ ...baseUserParams, currentUsage: 4, limit: 0 })
     expect(dbUpdateSpy).not.toHaveBeenCalled()

apps/sim/lib/billing/core/limit-notifications.ts

@@ -172,13 +172,6 @@ export async function maybeSendLimitThresholdEmail(params: {
   usageLabel: string
   /** Pre-formatted limit for the email body, e.g. "5 GB", "10 seats". */
   limitLabel: string
-  /**
-   * Usage immediately BEFORE the mutation, when known (e.g. the pre-insert row
-   * count). Lets a single large change that jumps from below the re-arm band
-   * past a threshold still re-arm before claiming, so the re-warning isn't
-   * suppressed. Omit when only the post-mutation usage is observable.
-   */
-  priorUsage?: number
   /**
    * When true, only the re-arm is evaluated and no email is ever sent. Used by
    * usage-decrease paths (e.g. a storage shrink) where usage can still be above
@@ -198,17 +191,15 @@ export async function maybeSendLimitThresholdEmail(params: {
 
     const { category, scope } = params
     const percent = Math.max(0, (params.currentUsage / params.limit) * 100)
-    const priorPercent =
-      params.priorUsage != null && params.priorUsage >= 0
-        ? (params.priorUsage / params.limit) * 100
-        : percent
     const desired = thresholdFor(percent)
 
     const stateId = scope === 'user' ? params.userId : params.organizationId
     if (!stateId) return
 
-    // Re-arm if usage is (or just was) back in the low band, so a fresh climb re-notifies.
-    if (Math.min(percent, priorPercent) < REARM_BELOW) {
+    // Re-arm when current usage is back in the low band, so a fresh climb re-notifies.
+    // Re-arm and claim are mutually exclusive (re-arm only when desired === 0), which
+    // keeps the dedup a single atomic claim with no re-arm/claim interleaving race.
+    if (percent < REARM_BELOW) {
       await rearmThreshold(scope, stateId, category)
     }
 
@@ -289,8 +280,6 @@ export async function maybeNotifyLimit(params: {
   limit: number
   usageLabel: string
   limitLabel: string
-  /** Usage before the mutation, when known — see {@link maybeSendLimitThresholdEmail}. */
-  priorUsage?: number
   /** Re-arm only, never send — for usage-decrease callers. See {@link maybeSendLimitThresholdEmail}. */
   rearmOnly?: boolean
 }): Promise<void> {
@@ -318,7 +307,6 @@ export async function maybeNotifyLimit(params: {
       limit: params.limit,
       usageLabel: params.usageLabel,
       limitLabel: params.limitLabel,
-      priorUsage: params.priorUsage,
       rearmOnly: params.rearmOnly,
       userId: params.billedUserId,
       userEmail,

apps/sim/lib/table/billing.ts

@@ -23,7 +23,6 @@ const TABLE_ROW_NOTIFY_PERCENT = 80
  */
 async function maybeNotifyTableRowLimit(
   workspaceId: string,
-  currentRowCount: number,
   projectedRowCount: number,
   limit: number
 ): Promise<void> {
@@ -39,7 +38,6 @@ async function maybeNotifyTableRowLimit(
       limit,
       usageLabel: `${projectedRowCount.toLocaleString('en-US')} rows`,
       limitLabel: `${limit.toLocaleString('en-US')} rows`,
-      priorUsage: currentRowCount,
     })
   } catch (error) {
     logger.error('Error evaluating table row-limit notification:', error)
@@ -51,7 +49,13 @@ async function maybeNotifyTableRowLimit(
  * gated so only near-limit writes pay the cost. Shared by every insert path
  * ({@link assertRowCapacity} and the transactional upsert/import branches that
  * check capacity with {@link wouldExceedRowLimit} instead). Pass the pre-insert
- * `currentRowCount` so a delete-then-insert jump re-arms correctly.
+ * `currentRowCount` and `addedRows` so the projected count drives the notify gate.
+ *
+ * Re-arm (allowing a fresh warning) happens on any insert that lands the table
+ * back below the re-arm band. Deleting straight below the band and then jumping
+ * back over a threshold in a single insert (with no intermediate write) is a
+ * best-effort gap — the 100% reached email and first-time crossings are
+ * unaffected. This keeps the dedup a single atomic claim with no re-arm/claim race.
  */
 export function notifyTableRowUsage(params: {
   workspaceId: string
@@ -62,12 +66,7 @@ export function notifyTableRowUsage(params: {
   if (params.limit <= 0) return
   const projected = params.currentRowCount + params.addedRows
   if ((projected / params.limit) * 100 >= TABLE_ROW_NOTIFY_PERCENT) {
-    void maybeNotifyTableRowLimit(
-      params.workspaceId,
-      params.currentRowCount,
-      projected,
-      params.limit
-    )
+    void maybeNotifyTableRowLimit(params.workspaceId, projected, params.limit)
   }
 }