fix(billing): stop the false unbilled-charge error on zero-cost runs - #6333
Conversation
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.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryLow Risk Overview In Tests add a shared Reviewed by Cursor Bugbot for commit 5235a4d. Configure here. |
Greptile SummaryThe PR prevents zero-target executions from emitting a false unbilled-charge error while preserving attribution validation and ledger error reporting whenever a billable target exists.
Confidence Score: 5/5The PR appears safe to merge because the new early return applies only when this path has no ledger entries to record. Nonzero workflow-owned costs still require billing attribution and retain the existing ledger-write and error-reporting flow, while externally owned Mothership costs continue through their separate attributed ledger path.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/logs/execution/logger.ts | Reorders the existing empty-target return so no-charge executions bypass irrelevant billing-attribution validation without changing nonzero ledger writes. |
| apps/sim/lib/logs/execution/logger.test.ts | Adds shared logger spies and verifies both suppression of the false error and retention of error-level reporting for actual ledger failures. |
Reviews (1): Last reviewed commit: "fix(billing): stop the false unbilled-ch..." | Re-trigger Greptile
Problem
Every usage-gated (blocked) execution emitted an ERROR-level log:
This is a 100% false positive on that path. It claims possible revenue loss on runs that never executed and have zero cost — exactly the line an engineer pages on. It misdirected a production investigation for hours. Across 5 days / 683 occurrences,
sum(totalCost) = 0andmax(totalCost) = 0.Mechanism (verified)
apps/sim/lib/execution/preprocessing.tslogPreprocessingErrorcallssession.safeCompleteWithError({ skipCost: true })and passes nobillingAttribution.skipCost: truemakeslogging-session.tsbuild an all-zerocostSummary(no base charge, no models, no charges).logger.tsrecordExecutionUsagerequired a billing context before buildingtargets[]:catch, which logs the unbilled-charge ERROR — even thoughtargets[]would have been empty andrecordUsagewould never have been called.Fix
Move the existing
if (targets.length === 0) return 0early return above the attribution requirement. Pure reordering — no new predicate.No charge can be lost by this change. When
targets.length === 0there is nothing to insert intousage_log: both the old path (throw → catch → log → return 0) and the new path (return 0) record exactly zero. The only difference is the spurious ERROR. Every path with a non-zero target still requiresbillingContextand still throws when it is missing.Genuine under-billing still alerts
The ERROR log is not silenced. Any real ledger failure — a
recordUsageinsert failure, a transaction/advisory-lock failure, or missing billing attribution on a run that does have cost — still reaches the samecatchand still logs at ERROR with the fullcostSummary. Alerting on this line remains valid; it is now specific.Tests
a structurally zero-cost run without billing context logs no unbilled-charge error— fails without the fix (1 spurious ERROR call).a genuine ledger write failure still logs the unbilled-charge error— regression guard; fails if anyone downgrades or removes the ERROR (verified by temporarily switching it towarn).The file's
@sim/loggermock is overridden locally sowithMetadata()children share one spy set, making log level assertable.Verification
bunx vitest run lib/logs/execution/logger.test.ts lib/logs/execution/logging-session.test.ts— 98 passedbunx tsc --noEmit -p apps/sim/tsconfig.json— cleanbun run lint— clean