fix(execution): run rate-limit gate only after ban/usage pass

The rate-limit gate is not read-only — checkRateLimitWithSubscription consumes
a token — so running it in parallel with the read-only gates debited rate-limit
quota for requests that the ban (403) or usage (402) gates reject, which the
original sequential flow never did.

Move the rate-limit gate to run sequentially after the ban and usage gates pass,
preserving the read-only gates' parallelism (ban + subscription + usage) and the
exact ban -> usage -> rate precedence. Add regression tests asserting the rate
limiter is not consumed when an earlier gate rejects, and is consumed once when
they pass.

Caught by Cursor Bugbot review.

Author: waleedlatif1

apps/sim/lib/execution/preprocessing.test.ts

@@ -229,6 +229,40 @@ describe('preprocessExecution ban gate', () => {
     })
   })
 
+  it('does not debit rate-limit quota when the ban gate rejects', async () => {
+    // The rate-limit gate consumes a token, so it must not run for a request
+    // an earlier gate (ban) already rejects.
+    mockGetActivelyBannedUserIds.mockResolvedValue(['billed-account-1'])
+
+    const result = await preprocessExecution({ ...baseOptions, checkRateLimit: true })
+
+    expect(result).toMatchObject({ success: false, error: { statusCode: 403 } })
+    expect(mockCheckRateLimit).not.toHaveBeenCalled()
+  })
+
+  it('does not debit rate-limit quota when the usage gate rejects', async () => {
+    vi.mocked(checkServerSideUsageLimits).mockResolvedValue({
+      isExceeded: true,
+      currentUsage: 20,
+      limit: 10,
+      message: 'Usage limit exceeded. Please upgrade your plan to continue.',
+    } as any)
+
+    const result = await preprocessExecution({ ...baseOptions, checkRateLimit: true })
+
+    expect(result).toMatchObject({ success: false, error: { statusCode: 402 } })
+    expect(mockCheckRateLimit).not.toHaveBeenCalled()
+  })
+
+  it('consumes the rate-limit gate exactly once when the ban and usage gates pass', async () => {
+    mockCheckRateLimit.mockResolvedValue({ allowed: true, remaining: 5, resetAt: new Date() })
+
+    const result = await preprocessExecution({ ...baseOptions, checkRateLimit: true })
+
+    expect(result.success).toBe(true)
+    expect(mockCheckRateLimit).toHaveBeenCalledTimes(1)
+  })
+
   it('checks the billing actor, caller-provided userId, and workflow owner in one call', async () => {
     const result = await preprocessExecution(baseOptions)
 

apps/sim/lib/execution/preprocessing.ts

@@ -573,11 +573,14 @@ export async function preprocessExecution(
   let rateLimitInfo: { allowed: boolean; remaining: number; resetAt: Date } | undefined
 
   /**
-   * STEP 6: rate-limit gate. Returns the failure outcome, or `null` on
-   * pass/skip. On a non-error outcome it populates `rateLimitInfo` for the
-   * success result, matching the sequential version.
+   * STEP 6: rate-limit gate. Unlike the other gates this one is NOT read-only —
+   * `checkRateLimitWithSubscription` consumes a token — so it is invoked
+   * sequentially only after the ban and usage gates pass, matching the original
+   * order. Running it eagerly or in parallel would debit rate-limit quota for
+   * requests that ban or usage rejects. Returns the failure outcome, or `null`
+   * on pass/skip; on a non-error outcome it populates `rateLimitInfo`.
    */
-  const rateLimitTask = (async (): Promise<GateFailure | null> => {
+  const runRateLimitGate = async (): Promise<GateFailure | null> => {
     if (!checkRateLimit) return null
     try {
       const rateLimiter = new RateLimiter()
@@ -646,21 +649,31 @@ export async function preprocessExecution(
         },
       }
     }
-  })()
+  }
 
-  const [usageResult, rateLimitFailure] = await Promise.all([usageCheckTask, rateLimitTask])
-  const usageFailure = usageResult.failure
+  const usageResult = await usageCheckTask
   const usageSnapshot = usageResult.snapshot
 
-  // Evaluate outcomes in fixed precedence order — ban (403) → usage (402) →
-  // rate limit (429) — so the response matches the sequential version exactly,
-  // independent of which gate's promise settled first.
-  const gateFailure = banFailure ?? usageFailure ?? rateLimitFailure
-  if (gateFailure) {
-    if (gateFailure.recordError) {
-      await recordPreprocessingError(gateFailure.recordError)
+  // The read-only gates (ban, subscription, usage) ran concurrently; evaluate
+  // them in fixed precedence order — ban (403) → usage (402) — so the response
+  // matches the sequential version regardless of which settled first.
+  const readGateFailure = banFailure ?? usageResult.failure
+  if (readGateFailure) {
+    if (readGateFailure.recordError) {
+      await recordPreprocessingError(readGateFailure.recordError)
+    }
+    return readGateFailure.response
+  }
+
+  // Rate limiting (429) runs only after ban and usage pass, because it debits a
+  // token — matching the original sequential order so rejected requests never
+  // consume quota.
+  const rateLimitFailure = await runRateLimitGate()
+  if (rateLimitFailure) {
+    if (rateLimitFailure.recordError) {
+      await recordPreprocessingError(rateLimitFailure.recordError)
     }
-    return gateFailure.response
+    return rateLimitFailure.response
   }
 
   /**