From 5235a4d90b50d35f4cebe8082226d83b540a9eee Mon Sep 17 00:00:00 2001
From: Waleed Latif <walif6@gmail.com>
Date: Thu, 6 Aug 2026 11:47:51 -0700
Subject: [PATCH] fix(billing): stop the false unbilled-charge error on
 zero-cost runs

recordExecutionUsage required a billing context before it knew whether
there was anything to bill, so a usage-gated run (skipCost, no
billingContext) threw and logged 'charge may be unbilled' for a run that
never executed and had no cost. Move the no-billable-target early return
above the attribution requirement: a genuine ledger write failure still
logs at ERROR.
---
 apps/sim/lib/logs/execution/logger.test.ts | 52 ++++++++++++++++++++++
 apps/sim/lib/logs/execution/logger.ts      | 16 ++++---
 2 files changed, 62 insertions(+), 6 deletions(-)

diff --git a/apps/sim/lib/logs/execution/logger.test.ts b/apps/sim/lib/logs/execution/logger.test.ts
index 3edcbd0f9b3..6f7ea251622 100644
--- a/apps/sim/lib/logs/execution/logger.test.ts
+++ b/apps/sim/lib/logs/execution/logger.test.ts
@@ -15,6 +15,28 @@ import type { SerializableExecutionState } from '@/executor/execution/types'
 
 afterAll(resetDbChainMock)
 
+/** Flat logger whose withMetadata() children share one spy set, so log level is assertable. */
+const { mockLogger } = vi.hoisted(() => {
+  const mockLogger: Record<string, ReturnType<typeof vi.fn>> = {
+    info: vi.fn(),
+    warn: vi.fn(),
+    error: vi.fn(),
+    debug: vi.fn(),
+    trace: vi.fn(),
+    fatal: vi.fn(),
+  }
+  mockLogger.child = vi.fn(() => mockLogger)
+  mockLogger.withMetadata = vi.fn(() => mockLogger)
+  return { mockLogger }
+})
+
+vi.mock('@sim/logger', () => ({
+  createLogger: vi.fn(() => mockLogger),
+  logger: mockLogger,
+  runWithRequestContext: vi.fn(<T>(_ctx: unknown, fn: () => T): T => fn()),
+  getRequestContext: vi.fn(() => undefined),
+}))
+
 // Mock billing modules
 vi.mock('@/lib/billing/core/subscription', () => ({
   getHighestPriorityPersonalSubscription: vi.fn(() => Promise.resolve(null)),
@@ -1013,6 +1035,36 @@ describe('recordExecutionUsage boundary-delta reconciliation', () => {
     expect(recordUsage).not.toHaveBeenCalled()
   })
 
+  const unbilledErrorCalls = () =>
+    mockLogger.error.mock.calls.filter((call) =>
+      String(call[0]).includes('Failed to record execution usage to usage_log ledger')
+    )
+
+  test('a structurally zero-cost run without billing context logs no unbilled-charge error', async () => {
+    mockDb([])
+
+    const recorded = await logger.recordExecutionUsage(
+      'workflow-1',
+      costSummary({ baseExecutionCharge: 0 }),
+      'api',
+      'exec-1',
+      'user-1'
+    )
+
+    expect(recorded).toBe(0)
+    expect(recordUsage).not.toHaveBeenCalled()
+    expect(unbilledErrorCalls()).toHaveLength(0)
+  })
+
+  test('a genuine ledger write failure still logs the unbilled-charge error', async () => {
+    vi.mocked(recordUsage).mockRejectedValueOnce(new Error('ledger insert failed'))
+
+    const recorded = await run(costSummary(), [])
+
+    expect(recorded).toBe(0)
+    expect(unbilledErrorCalls()).toHaveLength(1)
+  })
+
   test('retry with everything already billed records nothing (idempotent)', async () => {
     await run(
       costSummary({
diff --git a/apps/sim/lib/logs/execution/logger.ts b/apps/sim/lib/logs/execution/logger.ts
index 821d43e8f96..ef9cf67a9df 100644
--- a/apps/sim/lib/logs/execution/logger.ts
+++ b/apps/sim/lib/logs/execution/logger.ts
@@ -1547,12 +1547,6 @@ export class ExecutionLogger implements IExecutionLoggerService {
         return 0
       }
 
-      if (workflowRecord.workspaceId && !billingContext) {
-        throw new Error('Billing attribution is required for workspace execution usage')
-      }
-      const resolvedBillingContext =
-        billingContext ?? deriveBillingContext(userId, await getHighestPrioritySubscription(userId))
-
       // Build the run's *cumulative* target ledger lines from the cost summary.
       // The usage_log is then reconciled to these targets: at each completion
       // boundary (pause or terminal) we record only the increment versus what
@@ -1615,11 +1609,21 @@ export class ExecutionLogger implements IExecutionLoggerService {
         }
       }
 
+      // Bail before requiring billing attribution: a run with no billable target
+      // (e.g. a preprocessing-gated run that never executed) writes no ledger row
+      // either way, so demanding attribution here would raise a lost-revenue
+      // error for a charge that does not exist.
       if (targets.length === 0) {
         statsLog.debug('No cost to record')
         return 0
       }
 
+      if (workflowRecord.workspaceId && !billingContext) {
+        throw new Error('Billing attribution is required for workspace execution usage')
+      }
+      const resolvedBillingContext =
+        billingContext ?? deriveBillingContext(userId, await getHighestPrioritySubscription(userId))
+
       // Matches the billedBefore key resolution (toFixed(8)): a delta below this
       // is finer than the idempotency key can distinguish across boundaries, so
       // ignoring it keeps the key and the gate consistent.