Skip to content

Commit f4fd25e

Browse files
authored
fix(billing): answer an in-flight retry at once, and tell Chat what provenance withheld (#9094)
* fix(billing): answer an in-flight retry at once, and tell Chat what provenance withheld A cost callback retried while an earlier attempt of the same request was still committing (a commit can wait on a synchronous standby while holding the request's ledger lock) waited out lock_timeout on a pooled connection and failed with a 500. The ledger lock is now taken without waiting: a held lock answers 503 BILLING_REQUEST_IN_FLIGHT (Retry-After) at once, and the caller resends later; the monotonic top-up applies the higher cumulative once. Provenance refusals are policy, not faults: each gate admission now reports its refusals as one warning with counts and a bounded sample instead of one error per item, and the table envelope's unknown-row line is a warning. Chat's CLI result now notes how many items of each kind a gate withheld, on stderr, so the model can tell a withheld item from an absent one. Callers outside Chat never classify content and see no change. Also corrects two comments: the worker's callback timeout is 15 s, and no transaction setting bounds a commit waiting on a standby. * test(billing): model the ledger try-lock in the unit tx mock; pass the route context The usage-log unit mock answered every statement with undefined, which the try-lock cannot read; it now answers whether the lock was taken, with a test that a held lock refuses before any read or write. The new route test passes the route context like the typed handler expects, and the refusal reason type stays module-private. * fix(mothership): word the withheld note for every refusal, and describe the rows that read as unknown A gate refuses either the item (unknown provenance) or only the requested view (an excerpt or raw bytes redaction cannot cover), so the note no longer says another read cannot return it. Untracked legacy rows read as exact-empty; only tracked rows with a stale, missing or malformed sidecar read as unknown. * fix(provenance): keep refused ids paired with their cause, name sandbox files by hash, and tell Chat why each item was withheld Refusal reports are grouped by surface, cause and owning record (document, table or chat), each line with top-level surface, cause and refusedCount and the refused ids (up to 200, flagged when truncated), so per-cause queries and id-to-cause pairing survive aggregation. A sandbox file is attributed by a chat-scoped hash of its path rather than the path, which carries the user's file name. A streamed sandbox image declares its view opaque, so its refusal reads as opaque content rather than a missing registry; the use case now requires every caller to declare the view. Gate verdicts carry the refusal reason, and Chat's CLI note says per reason what a read can do next: unknown provenance will not return on a retry, recorded secrets can be read as text for a redacted copy, and provenance that could not be verified may be readable later. Chunk write refusals name their document as the parent. * fix(billing): an in-flight refusal is not a slow ledger transaction The usage ledger's slow-transaction warning fired for every in-flight refusal (stage lock, not succeeded), although nothing waited and the route already logs the refusal once at INFO. It now skips refusals. The span keeps the existing internal_error outcome, since the closed outcome set is generated from the legacy contracts; the response code and the INFO line tell it apart. * fix(provenance): keep this change to the refusal logs and note it introduced Drops the sandbox attribution id, the streamed-image view and the owning-record ids from this change; they belong to the provenance owner's work and fix behavior that predates it. Refusal lines stay one per surface and cause with top-level surface, cause and refusedCount and every refused id as each line names it today, and the note keeps its per-reason wording. * fix(provenance): give every refusal line a refusedCount, and say what tells an in-flight 503 apart A single refusal now carries refusedCount 1 like a grouped one, so summing refusedCount by cause covers both. The route comment now says the span is the same as the retryable settlement 503's and that the body code and the INFO line distinguish them.
1 parent debdd0e commit f4fd25e

22 files changed

Lines changed: 572 additions & 78 deletions

‎apps/sim/app/api/billing/update-cost/route.test.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { readFileSync } from 'node:fs'
22
import { createMockRequest, resetEnvFlagsMock, setEnvFlags } from '@sim/testing'
3+
import { createRouteContext } from '@sim/testing/helpers/http'
34
import {
45
billingAttributionMock,
56
billingAttributionMockFns,
@@ -9,6 +10,7 @@ import { billingPlanMock, billingPlanMockFns } from '@sim/testing/mocks/billing-
910
import {
1011
billingUsageLogMock,
1112
billingUsageLogMockFns,
13+
MockCumulativeUsageInFlightError,
1214
} from '@sim/testing/mocks/billing-usage-log.mock'
1315
import {
1416
billingUsageMonitorMock,
@@ -18,6 +20,7 @@ import { copilotHttpMock, copilotHttpMockFns } from '@sim/testing/mocks/copilot-
1820
import { mothershipOtelMock } from '@sim/testing/mocks/mothership-otel.mock'
1921
import { sleep } from '@sim/utils/helpers'
2022
import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
23+
import { BillingInFlightCode } from '@/lib/mothership/generated/billing'
2124

2225
const {
2326
mockCheckAndBillOverageThreshold,
@@ -748,6 +751,42 @@ describe('POST /api/billing/update-cost — workspaceId attribution', () => {
748751
})
749752
})
750753

754+
it('answers a retryable in-flight 503 while another attempt of the request is recorded', async () => {
755+
const billingRequestId = '0190c03f-9f7d-4b79-8b58-e7f779fd29e1'
756+
mockRecordCumulativeUsage.mockRejectedValueOnce(
757+
new MockCumulativeUsageInFlightError(`update-cost:${billingRequestId}`)
758+
)
759+
760+
const res = await POST(
761+
createMockRequest(
762+
'POST',
763+
{
764+
userId: 'user-1',
765+
cost: 0.5,
766+
model: 'claude-opus-4.8',
767+
source: 'workspace-chat',
768+
idempotencyKey: billingRequestId,
769+
},
770+
{
771+
'x-api-key': 'internal',
772+
'x-sim-billing-protocol': 'direct-v1',
773+
'x-sim-billing-request-id': billingRequestId,
774+
'x-sim-billing-account-decision': 'serialized-account-decision',
775+
}
776+
),
777+
createRouteContext({})
778+
)
779+
780+
expect(res.status).toBe(503)
781+
expect(res.headers.get('retry-after')).toBe('5')
782+
await expect(res.json()).resolves.toMatchObject({
783+
success: false,
784+
code: BillingInFlightCode,
785+
retryable: true,
786+
})
787+
expect(mockCheckAndBillPayerOverageThreshold).not.toHaveBeenCalled()
788+
})
789+
751790
it('retries modern settlement on a duplicate cumulative callback before returning 409', async () => {
752791
const billingRequestId = '0190c03f-9f7d-4b79-8b58-e7f779fd29e1'
753792
mockRecordCumulativeUsage

‎apps/sim/app/api/billing/update-cost/route.ts‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
import {
3131
type CumulativeUsageContextField,
3232
CumulativeUsageContextMismatchError,
33+
CumulativeUsageInFlightError,
3334
CumulativeUsagePeriodClosedError,
3435
recordCumulativeUsage,
3536
} from '@/lib/billing/core/usage-log'
@@ -43,6 +44,7 @@ import { isBillingEnabled, isHosted } from '@/lib/core/config/env-flags'
4344
import { withinDeadline } from '@/lib/core/utils/deadline'
4445
import { generateRequestId } from '@/lib/core/utils/request'
4546
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
47+
import { BillingInFlightCode } from '@/lib/mothership/generated/billing'
4648
import { BILLING_CALLBACK_OUTCOME } from '@/lib/mothership/generated/billing-protocol-v1'
4749
import { BillingRouteOutcome } from '@/lib/mothership/generated/trace-attribute-values-v1'
4850
import { TraceAttr } from '@/lib/mothership/generated/trace-attributes-v1'
@@ -53,7 +55,7 @@ import { withIncomingGoSpan } from '@/lib/mothership/request/otel'
5355
const logger = createLogger('BillingUpdateCostAPI')
5456
/**
5557
* How long a cost callback waits on the payer's standing. The worker gives up on the whole
56-
* callback after 5 s, and a cold gate read can wait on the ledger far longer; past this the
58+
* callback after 15 s, and a cold gate read can wait on the ledger far longer; past this the
5759
* callback answers not-exceeded. The abandoned read keeps running and caches its admission, and
5860
* the next step or re-check reads a refusal again.
5961
*/
@@ -510,6 +512,32 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
510512
)
511513
}
512514

515+
/**
516+
* Another attempt of this request is still being recorded, so this one's outcome is unknown:
517+
* the caller resends later, and the monotonic top-up applies the higher cumulative once.
518+
*/
519+
if (error instanceof CumulativeUsageInFlightError) {
520+
logger.info(`[${requestId}] Cost update already in flight for this request`, {
521+
eventKey: error.eventKey,
522+
duration,
523+
})
524+
// The closed outcome set has no in-flight value, so the span matches the retryable settlement
525+
// 503; the body's BILLING_REQUEST_IN_FLIGHT code and the INFO line above tell them apart.
526+
span.setAttribute(TraceAttr.BillingOutcome, BillingRouteOutcome.InternalError)
527+
span.setAttribute(TraceAttr.HttpStatusCode, 503)
528+
span.setAttribute(TraceAttr.BillingDurationMs, duration)
529+
return NextResponse.json(
530+
{
531+
success: false,
532+
code: BillingInFlightCode,
533+
error: 'Billing request already in flight',
534+
retryable: true,
535+
requestId,
536+
},
537+
{ status: 503, headers: { 'Retry-After': '5' } }
538+
)
539+
}
540+
513541
const pgCode = getPostgresErrorCode(error)
514542
const pgConstraint = getPostgresConstraintName(error)
515543
const reconciliationOutcome =

‎apps/sim/lib/billing/core/usage-log.integration.ts‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ vi.mock('@/lib/billing/subscriptions/utils', async (importOriginal) => ({
2828

2929
import {
3030
CumulativeUsageContextMismatchError,
31+
CumulativeUsageInFlightError,
3132
CumulativeUsagePeriodClosedError,
3233
getBillingPeriodUsageCost,
3334
getBillingPeriodUsageCostByUser,
@@ -92,6 +93,18 @@ function usage(cost: number, eventKey = 'update-cost:shared-request'): RecordCum
9293
}
9394
}
9495

96+
/** Resends a callback refused as in flight, the way the worker's replay does, until it is recorded. */
97+
async function untilRecorded(params: RecordCumulativeUsageParams) {
98+
for (let attempt = 0; ; attempt++) {
99+
try {
100+
return await recordCumulativeUsage(params)
101+
} catch (error) {
102+
if (!(error instanceof CumulativeUsageInFlightError) || attempt >= 200) throw error
103+
await sleep(50)
104+
}
105+
}
106+
}
107+
95108
async function ledgerRows() {
96109
return connection<{ event_key: string; cost: string }[]>`
97110
select event_key, cost from usage_log order by event_key
@@ -199,13 +212,15 @@ describe('Cumulative billing with PostgreSQL', () => {
199212
}
200213
)
201214

202-
it('reproduces a retry timing out behind an idle holder when only lock waits are bounded', async () => {
215+
it('refuses a retry behind a holder of the same request at once, without waiting on its lock', async () => {
203216
const pause = pauseNextTransaction(true)
204217
const holder = recordCumulativeUsage(usage(0.4))
205218
try {
206219
await pause.reached.promise
220+
const startedAt = performance.now()
207221
const failure = await recordCumulativeUsage(usage(0.8)).catch((error: unknown) => error)
208-
expect(getPostgresErrorCode(failure)).toBe('55P03')
222+
expect(failure).toBeInstanceOf(CumulativeUsageInFlightError)
223+
expect(performance.now() - startedAt).toBeLessThan(1_000)
209224
expect(await ledgerRows()).toEqual([])
210225
} finally {
211226
pause.release.resolve()
@@ -223,8 +238,8 @@ describe('Cumulative billing with PostgreSQL', () => {
223238
try {
224239
await pause.reached.promise
225240
const failure = await recordCumulativeUsage(usage(0.8)).catch((error: unknown) => error)
226-
expect(getPostgresErrorCode(failure)).toBe('55P03')
227-
const recovered = await recordCumulativeUsage(usage(0.8))
241+
expect(failure).toBeInstanceOf(CumulativeUsageInFlightError)
242+
const recovered = await untilRecorded(usage(0.8))
228243
expect(recovered.billed).toBe(true)
229244
expect(recovered.delta).toBeCloseTo(0.8 - initial, 9)
230245
expect(recovered.total).toBe(0.8)
@@ -249,7 +264,7 @@ describe('Cumulative billing with PostgreSQL', () => {
249264

250265
it('converges concurrent out-of-order callbacks and independent events to their exact totals', async () => {
251266
const costs = [0.4, 0.1, 0.8, 0.3, 0.8, 0.6]
252-
const results = await Promise.all(costs.map((cost) => recordCumulativeUsage(usage(cost))))
267+
const results = await Promise.all(costs.map((cost) => untilRecorded(usage(cost))))
253268
expect(results.reduce((total, result) => total + result.delta, 0)).toBeCloseTo(0.8, 9)
254269
await Promise.all(
255270
Array.from({ length: 32 }, (_, index) =>

‎apps/sim/lib/billing/core/usage-log.test.ts‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
billingSubscriptionUtilsMock,
66
billingSubscriptionUtilsMockFns,
77
} from '@sim/testing/mocks/billing-subscription-utils.mock'
8+
import { getMockLogger } from '@sim/testing/mocks/logger.mock'
89
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
910

1011
const {
@@ -31,6 +32,7 @@ import { USAGE_LEDGER_STATEMENT_TIMEOUT_MS } from '@/lib/billing/constants'
3132
import {
3233
CUMULATIVE_COST_EPSILON,
3334
CumulativeUsageContextMismatchError,
35+
CumulativeUsageInFlightError,
3436
getBillingPeriodUsageCost,
3537
getBillingPeriodUsageCostByUser,
3638
getBillingPeriodUsageCostWithSourceSubset,
@@ -245,7 +247,8 @@ describe('recordCumulativeUsage', () => {
245247
mockIsOrgScopedSubscription.mockReturnValue(true)
246248
})
247249

248-
const setupTx = (existingRow: Partial<typeof defaultExistingRow> | null) => {
250+
/** `locked`: another attempt of the request holds its ledger lock. */
251+
const setupTx = (existingRow: Partial<typeof defaultExistingRow> | null, locked = false) => {
249252
const resolvedExistingRow = existingRow ? { ...defaultExistingRow, ...existingRow } : null
250253
const limit = vi.fn().mockResolvedValue(resolvedExistingRow ? [resolvedExistingRow] : [])
251254
const where = vi.fn().mockReturnValue({ limit })
@@ -255,7 +258,8 @@ describe('recordCumulativeUsage', () => {
255258
const updateSet = vi.fn().mockReturnValue({ where: updateWhere })
256259
mockUpdate.mockReturnValue({ set: updateSet })
257260
const tx = {
258-
execute: vi.fn().mockResolvedValue(undefined),
261+
// The configure statement's result is unused; the try-lock reads `acquired`.
262+
execute: vi.fn().mockResolvedValue([{ acquired: !locked }]),
259263
select,
260264
update: mockUpdate,
261265
insert: mockInsert, // recordUsage(tx) reuses the shared insert chain
@@ -290,6 +294,27 @@ describe('recordCumulativeUsage', () => {
290294
})
291295
})
292296

297+
it('refuses at once, writing nothing, while another attempt of the request holds its lock', async () => {
298+
const { select } = setupTx(null, true)
299+
await expect(
300+
recordCumulativeUsage({
301+
userId: 'user-1',
302+
source: 'workspace-chat',
303+
model: 'claude-opus-4.8',
304+
cost: 0.4662453,
305+
eventKey: 'update-cost:msg-1-billing',
306+
})
307+
).rejects.toBeInstanceOf(CumulativeUsageInFlightError)
308+
expect(select).not.toHaveBeenCalled()
309+
expect(mockInsert).not.toHaveBeenCalled()
310+
expect(mockUpdate).not.toHaveBeenCalled()
311+
// The route logs the refusal once; it is not a transaction that failed to complete.
312+
expect(getMockLogger('UsageLog').warn).not.toHaveBeenCalledWith(
313+
'Cumulative usage transaction did not complete promptly',
314+
expect.anything()
315+
)
316+
})
317+
293318
it('tops up to the higher cumulative and bills only the delta', async () => {
294319
const { updateSet } = setupTx({ id: 'row-1', cost: '0.3474447' })
295320
const result = await recordCumulativeUsage({

‎apps/sim/lib/billing/core/usage-log.ts‎

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import { isOrgScopedSubscription } from '@/lib/billing/subscriptions/utils'
3030
import type { InternalUsageLogSource } from '@/lib/billing/usage-sources'
3131
import { asOrchestrationError, OrchestrationError } from '@/lib/core/orchestration/types'
3232
import { HttpError } from '@/lib/core/utils/http-error'
33-
import { acquireAdvisoryXactLock } from '@/lib/db/advisory-locks'
33+
import { tryAcquireAdvisoryXactLock } from '@/lib/db/advisory-locks'
3434
import type { DbClient, DbOrTx } from '@/lib/db/types'
3535

3636
const logger = createLogger('UsageLog')
@@ -700,6 +700,18 @@ export class CumulativeUsagePeriodClosedError extends Error {
700700
}
701701
}
702702

703+
/**
704+
* Another attempt of the same request holds its ledger lock, so this one cannot know whether that
705+
* attempt's charge will land. Waiting would only hold a connection for an answer the caller gets by
706+
* retrying later: the monotonic top-up applies whichever cumulative arrives once.
707+
*/
708+
export class CumulativeUsageInFlightError extends Error {
709+
constructor(readonly eventKey: string) {
710+
super(`Cumulative usage event "${eventKey}" is already being recorded`)
711+
this.name = 'CumulativeUsageInFlightError'
712+
}
713+
}
714+
703715
interface CumulativeUsageLedgerBinding {
704716
userId: string
705717
workspaceId: string | null
@@ -755,11 +767,13 @@ function assertCumulativeUsageLedgerBinding(
755767
}
756768

757769
/**
758-
* PostgreSQL 17+ bounds the entire transaction below the callback's five-second
759-
* deadline. Older supported servers instead bound each idle interval between
760-
* statements, alongside the per-statement budget. Both policies release an idle
761-
* lock holder without waiting for its application process to resume; only the
762-
* newer policy also limits total elapsed transaction time.
770+
* PostgreSQL 17+ bounds the transaction's statements and the idle time between
771+
* them; older supported servers bound each idle interval instead, alongside the
772+
* per-statement budget. Both release an idle lock holder without waiting for its
773+
* application process to resume. Neither bounds COMMIT: a commit waiting for a
774+
* synchronous standby keeps its locks, including the request's ledger lock, until
775+
* the standby confirms, however long that takes. `lock_timeout` bounds the row
776+
* locks taken under the ledger lock; the ledger lock itself is never waited for.
763777
*/
764778
const CUMULATIVE_FLUSH_TRANSACTION_TIMEOUT_MS = 4_000
765779
const CUMULATIVE_FLUSH_STATEMENT_TIMEOUT_MS = 3_500
@@ -778,10 +792,12 @@ type CumulativeUsageStage = 'pool' | 'configure' | 'lock' | 'read' | 'write' | '
778792
* (no two flushes can both believe they are first and clobber each other).
779793
* An existing row must match the incoming actor, workspace, payer, and billing
780794
* period before either a duplicate no-op or a top-up is accepted.
781-
* The billing context is resolved BEFORE the transaction and the lock wait is
782-
* bounded by `lock_timeout`. A server-enforced transaction deadline, or idle
783-
* transaction deadline on older PostgreSQL, releases a stalled holder. The
784-
* critical section uses one SELECT plus one INSERT/UPDATE on a single connection.
795+
* The billing context is resolved BEFORE the transaction. The lock is taken
796+
* without waiting: a held lock means another attempt of the same request is
797+
* still being recorded (possibly committing behind a synchronous standby), and
798+
* this attempt throws {@link CumulativeUsageInFlightError} instead of holding a
799+
* connection while it waits. The critical section uses one SELECT plus one
800+
* INSERT/UPDATE on a single connection.
785801
*
786802
* Because every leg flushes its cumulative and this converges to the max,
787803
* there is no under-billing if the request recovers after a partial flush, no
@@ -816,6 +832,7 @@ export async function recordCumulativeUsage(
816832
const stageDurationsMs: Partial<Record<CumulativeUsageStage, number>> = {}
817833
let succeeded = false
818834
let pgCode: string | undefined
835+
let inFlight = false
819836
let toppedUpRowCreatedAt: Date | undefined
820837
const enterStage = (nextStage: CumulativeUsageStage) => {
821838
const now = Date.now()
@@ -841,7 +858,9 @@ export async function recordCumulativeUsage(
841858
set_config('lock_timeout', ${`${CUMULATIVE_FLUSH_LOCK_TIMEOUT_MS}ms`}, true)
842859
`)
843860
enterStage('lock')
844-
await acquireAdvisoryXactLock(tx, 'usage_log_event', eventKey)
861+
if (!(await tryAcquireAdvisoryXactLock(tx, 'usage_log_event', eventKey))) {
862+
throw new CumulativeUsageInFlightError(eventKey)
863+
}
845864

846865
enterStage('read')
847866
const rows = await tx
@@ -997,12 +1016,14 @@ export async function recordCumulativeUsage(
9971016
return result
9981017
} catch (error) {
9991018
pgCode = getPostgresErrorCode(error)
1019+
// Another attempt holding the lock is an answer, not a slow transaction; the route logs it.
1020+
if (error instanceof CumulativeUsageInFlightError) inFlight = true
10001021
throw error
10011022
} finally {
10021023
const now = Date.now()
10031024
stageDurationsMs[stage] = now - stageStartedAt
10041025
const durationMs = now - startedAt
1005-
if (!succeeded || durationMs >= CUMULATIVE_FLUSH_SLOW_MS) {
1026+
if (!inFlight && (!succeeded || durationMs >= CUMULATIVE_FLUSH_SLOW_MS)) {
10061027
logger.warn('Cumulative usage transaction did not complete promptly', {
10071028
eventKey,
10081029
succeeded,

0 commit comments

Comments
 (0)