Skip to content

Commit c8f879a

Browse files
committed
fix(billing): roll forward-moved periods, lock only past period end, skip ended admissions
- Roll a cumulative charge into the payer's current period whenever that period starts after the latest row's, so an anchor reset or resync inside the old period never tops up a closed period. - Share-lock the payer's subscription row only once the latest row's period has ended; before that no close can be due, and the lock would starve the rollover update for a busy payer. - Mid-run usage checks report unknown (continue) once the run's admitted period has ended, instead of judging the old period against its allowance. - Refuse request keys containing "@" when period rows are in play, so they cannot collide with another request's period rows. - Document the mixed-version and rollback window: code that predates period rows can double-count a run's post-rollover spend only between a rollover and that period's close (at least an hour); the exposure is cents to dollars.
1 parent ac64709 commit c8f879a

7 files changed

Lines changed: 153 additions & 39 deletions

File tree

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

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -852,6 +852,10 @@ describe('POST /api/billing/update-cost — workspaceId attribution', () => {
852852

853853
describe('POST /api/billing/update-cost — mid-run usage gate', () => {
854854
let callbackSequence = 0
855+
const CURRENT_ATTRIBUTION = {
856+
...ATTRIBUTION,
857+
billingPeriod: { start: '2026-07-01T00:00:00.000Z', end: '2099-01-01T00:00:00.000Z' },
858+
}
855859

856860
function attributedCallback() {
857861
callbackSequence += 1
@@ -881,7 +885,7 @@ describe('POST /api/billing/update-cost — mid-run usage gate', () => {
881885
mockCheckInternalApiKey.mockReturnValue({ success: true })
882886
mockRecordCumulativeUsage.mockResolvedValue({ billed: true, delta: 0.5, total: 0.5 })
883887
mockCheckAndBillPayerOverageThreshold.mockResolvedValue(undefined)
884-
mockRequireBillingAttributionHeader.mockReturnValue(ATTRIBUTION)
888+
mockRequireBillingAttributionHeader.mockReturnValue(CURRENT_ATTRIBUTION)
885889
mockToBillingContext.mockReturnValue({
886890
billingEntity: { type: 'organization', id: 'org-1' },
887891
billingPeriod: {
@@ -906,12 +910,12 @@ describe('POST /api/billing/update-cost — mid-run usage gate', () => {
906910
message: expect.stringContaining('usage limit'),
907911
},
908912
})
909-
expect(mockCheckAttributedUsageLimits).toHaveBeenCalledWith(ATTRIBUTION)
913+
expect(mockCheckAttributedUsageLimits).toHaveBeenCalledWith(CURRENT_ATTRIBUTION)
910914
})
911915

912916
it('offers a paid organization payer the increase-limit card', async () => {
913917
mockRequireBillingAttributionHeader.mockReturnValue({
914-
...ATTRIBUTION,
918+
...CURRENT_ATTRIBUTION,
915919
payerSubscription: { id: 'sub-1', plan: 'team', status: 'active', seats: 4 },
916920
})
917921
mockCheckAttributedUsageLimits.mockResolvedValue({ isExceeded: true, scope: 'payer' })
@@ -971,7 +975,7 @@ describe('POST /api/billing/update-cost — mid-run usage gate', () => {
971975
beforeEach(() => {
972976
mockCheckAttributedUsageLimits.mockResolvedValue({ isExceeded: false })
973977
mockRequireBillingAttributionHeader.mockReturnValue({
974-
...ATTRIBUTION,
978+
...CURRENT_ATTRIBUTION,
975979
payerSubscription: PAYER_SUBSCRIPTION,
976980
})
977981
})
@@ -1061,6 +1065,16 @@ describe('POST /api/billing/update-cost — mid-run usage gate', () => {
10611065
})
10621066
})
10631067

1068+
it('does not judge a run against an admitted period that has already ended', async () => {
1069+
mockRequireBillingAttributionHeader.mockReturnValue(ATTRIBUTION)
1070+
mockCheckAttributedUsageLimits.mockResolvedValue({ isExceeded: true, scope: 'payer' })
1071+
1072+
const body = await (await POST(attributedCallback())).json()
1073+
1074+
expect(body.usageExceeded).toBe(false)
1075+
expect(mockCheckAttributedUsageLimits).not.toHaveBeenCalled()
1076+
})
1077+
10641078
it('keeps a recorded charge successful when the gate read fails', async () => {
10651079
mockCheckAttributedUsageLimits.mockRejectedValue(new Error('ledger read timed out'))
10661080

‎apps/sim/app/api/copilot/api-keys/validate/route.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ const ATTRIBUTION = {
6363
billingEntity: { type: 'organization' as const, id: 'org-1' },
6464
billingPeriod: {
6565
start: '2026-07-01T00:00:00.000Z',
66-
end: '2026-08-01T00:00:00.000Z',
66+
end: '2099-01-01T00:00:00.000Z',
6767
source: 'reporting' as const,
6868
},
6969
payerSubscription: null,

‎apps/sim/lib/billing/core/mid-run-usage.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@ const logger = createLogger('MidRunUsage')
1414
* upgrade card.
1515
* - `blocked`: the account is blocked (payment failed, dispute); the run is refused as a blocked
1616
* account, never with the upgrade card.
17-
* - `unknown`: the gate could not read usage. Admission fails closed on this, but a run already
18-
* under way continues: a database blip must not end a paying user's long run, and the next
19-
* step or re-check reads again.
17+
* - `unknown`: the gate could not read usage, or the run's admitted period has ended. Admission
18+
* fails closed on an unreadable ledger, but a run already under way continues: a database
19+
* blip must not end a paying user's long run, and the next step or re-check reads again.
2020
*/
2121
export type MidRunUsageVerdict =
2222
| { status: 'within' }
@@ -27,6 +27,12 @@ export type MidRunUsageVerdict =
2727
export async function readMidRunUsageVerdict(
2828
attribution: BillingAttributionSnapshot
2929
): Promise<MidRunUsageVerdict> {
30+
// The gate judges the admitted snapshot's period. Once that period has ended it would keep
31+
// counting the old period against the old allowance, so a run just past a reset is not judged
32+
// until its next admission reads the new one.
33+
if (Date.now() >= new Date(attribution.billingPeriod.end).getTime()) {
34+
return { status: 'unknown' }
35+
}
3036
let usage: AttributedUsageLimitsResult
3137
try {
3238
usage = await checkExecutionUsageLimits(attribution)

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

Lines changed: 69 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -314,38 +314,52 @@ describe('Cumulative billing with PostgreSQL', () => {
314314
)
315315

316316
describe('a request that outlives its billing period', () => {
317+
// Past periods: the old period's row is written under the subscription lock only once
318+
// that period has ended.
317319
const periods = [
318-
new Date('2026-09-01T00:00:00.000Z'),
319-
new Date('2026-10-01T00:00:00.000Z'),
320-
new Date('2026-11-01T00:00:00.000Z'),
321-
new Date('2026-12-01T00:00:00.000Z'),
320+
new Date('2025-09-01T00:00:00.000Z'),
321+
new Date('2025-10-01T00:00:00.000Z'),
322+
new Date('2025-11-01T00:00:00.000Z'),
323+
new Date('2025-12-01T00:00:00.000Z'),
322324
]
323325
const payer = { type: 'organization', id: 'payer' } as const
324326

325-
async function setSubscriptionPeriod(index: number) {
327+
async function setSubscriptionWindow(start: Date, end: Date) {
326328
await connection`
327329
insert into subscription (id, period_start, period_end)
328-
values ('sub-1', ${periods[index].toISOString()}::timestamptz at time zone 'UTC', ${periods[index + 1].toISOString()}::timestamptz at time zone 'UTC')
330+
values ('sub-1', ${start.toISOString()}::timestamptz at time zone 'UTC', ${end.toISOString()}::timestamptz at time zone 'UTC')
329331
on conflict (id) do update
330332
set period_start = excluded.period_start, period_end = excluded.period_end
331333
`
332334
}
333335

334-
function charge(cost: number) {
335-
return recordCumulativeUsage({ ...usage(cost), payerSubscriptionId: 'sub-1' })
336+
async function setSubscriptionPeriod(index: number) {
337+
await setSubscriptionWindow(periods[index], periods[index + 1])
338+
}
339+
340+
function charge(cost: number, frozen = { start: periods[0], end: periods[1] }) {
341+
return recordCumulativeUsage({
342+
...usage(cost),
343+
billingPeriod: frozen,
344+
payerSubscriptionId: 'sub-1',
345+
})
336346
}
337347

338348
/** What the cycle close invoices for one period: the ledger rows stamped with it. */
339-
async function stampedTotal(index: number) {
349+
async function stampedWindowTotal(from: Date, to: Date) {
340350
const byUser = await getStampedPeriodRangeUsageCostByUser(
341351
payer,
342-
{ from: periods[index], to: periods[index + 1] },
352+
{ from, to },
343353
undefined,
344354
database
345355
)
346356
return [...byUser.values()].reduce((total, cost) => total + cost, 0)
347357
}
348358

359+
function stampedTotal(index: number) {
360+
return stampedWindowTotal(periods[index], periods[index + 1])
361+
}
362+
349363
it('invoices a charge that spans a period close exactly once in total', async () => {
350364
await setSubscriptionPeriod(0)
351365
expect(await charge(0.4)).toMatchObject({ billed: true, total: 0.4 })
@@ -410,5 +424,50 @@ describe('Cumulative billing with PostgreSQL', () => {
410424
}
411425
expect(await stampedTotal(0)).toBeCloseTo(0.6, 9)
412426
})
427+
428+
it('rolls into a period whose start moved forward before the old period ended', async () => {
429+
await setSubscriptionPeriod(0)
430+
await charge(0.4)
431+
const resetStart = new Date('2025-09-15T00:00:00.000Z')
432+
const resetEnd = new Date('2025-10-15T00:00:00.000Z')
433+
await setSubscriptionWindow(resetStart, resetEnd)
434+
435+
expect(await charge(1)).toMatchObject({
436+
billed: true,
437+
billingPeriod: { start: resetStart, end: resetEnd },
438+
})
439+
expect(await stampedTotal(0)).toBeCloseTo(0.4, 9)
440+
expect(await stampedWindowTotal(resetStart, resetEnd)).toBeCloseTo(0.6, 9)
441+
})
442+
443+
it('does not hold the rollover while the latest period is still running', async () => {
444+
const start = new Date(Date.now() - 24 * 60 * 60 * 1000)
445+
const end = new Date(Date.now() + 30 * 24 * 60 * 60 * 1000)
446+
await setSubscriptionWindow(start, end)
447+
await charge(0.4, { start, end })
448+
const pause = pauseNextTransaction()
449+
const inFlight = charge(0.6, { start, end })
450+
try {
451+
await pause.reached.promise
452+
await connection.begin(async (tx) => {
453+
await tx`select set_config('lock_timeout', '300ms', true)`
454+
await tx`update subscription set period_end = period_end where id = 'sub-1'`
455+
})
456+
} finally {
457+
pause.release.resolve()
458+
await inFlight
459+
}
460+
})
461+
462+
it('refuses a request key that could collide with its period rows', async () => {
463+
await setSubscriptionPeriod(0)
464+
await expect(
465+
recordCumulativeUsage({
466+
...usage(0.4, 'update-cost:request@1'),
467+
payerSubscriptionId: 'sub-1',
468+
})
469+
).rejects.toThrow('@')
470+
expect(await ledgerRows()).toEqual([])
471+
})
413472
})
414473
})

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

Lines changed: 39 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,13 @@ export interface RecordCumulativeUsageParams {
591591
* recorded in a new row stamped with the subscription's current period, so a request that
592592
* outlives its billing period is invoiced by the period it was spent in rather than topping up
593593
* a period that has already been closed. Omit it for reporting-window and free payers.
594+
*
595+
* Mixed versions: code that predates period rows reads only the request key. If such code
596+
* (during a deploy, or after a rollback) handles a later callback for a request that already
597+
* has period rows, it re-adds those rows' amount to the first row. That only double-counts
598+
* when it lands between the rollover and that period's close, which waits at least an hour,
599+
* and only for runs spanning a rollover; the exposure is one run's post-rollover spend, cents
600+
* to dollars.
594601
*/
595602
payerSubscriptionId?: string
596603
}
@@ -612,7 +619,10 @@ export interface RecordCumulativeUsageResult {
612619
*/
613620
const MAX_CUMULATIVE_PERIOD_ROWS = 12
614621

615-
/** The ledger key of the `index`-th period a cumulative request rolled into; 0 is the request key. */
622+
/**
623+
* The ledger key of the `index`-th period a cumulative request rolled into; 0 is the request key.
624+
* A request key may not itself contain `@`, so these keys never collide with another request's.
625+
*/
616626
function cumulativePeriodEventKey(eventKey: string, index: number): string {
617627
return index === 0 ? eventKey : `${eventKey}@${index}`
618628
}
@@ -774,6 +784,9 @@ export async function recordCumulativeUsage(
774784
}
775785

776786
const billingContext = await resolveBillingContext(userId, billingEntity, billingPeriod)
787+
if (payerSubscriptionId && eventKey.includes('@')) {
788+
throw new Error(`Cumulative usage event "${eventKey}" must not contain "@"`)
789+
}
777790

778791
const startedAt = Date.now()
779792
let stage: CumulativeUsageStage = 'pool'
@@ -808,20 +821,6 @@ export async function recordCumulativeUsage(
808821
await acquireAdvisoryXactLock(tx, 'usage_log_event', eventKey)
809822

810823
enterStage('read')
811-
// The payer's current period, share-locked so a rollover of the subscription waits for
812-
// this top-up to commit: whatever a close later sums for the old period is final.
813-
const [currentPeriod] = payerSubscriptionId
814-
? await tx
815-
.select({
816-
start: subscriptionTable.periodStart,
817-
end: subscriptionTable.periodEnd,
818-
})
819-
.from(subscriptionTable)
820-
.where(eq(subscriptionTable.id, payerSubscriptionId))
821-
.for('share')
822-
.limit(1)
823-
: []
824-
825824
const rows = await tx
826825
.select({
827826
id: usageLog.id,
@@ -881,12 +880,34 @@ export async function recordCumulativeUsage(
881880
return { billed: false, delta: 0, total: recorded, billingPeriod: latestPeriod }
882881
}
883882

884-
// Only ever forward: a subscription period that is not past the latest row keeps topping
885-
// up that row, whatever the wall clock or a replayed webhook says.
883+
// The payer's current period. Once the latest row's period has ended, the read is
884+
// share-locked so a rollover of the subscription waits for this top-up to commit, and
885+
// whatever the close later sums for the old period is final. Before that no close can be
886+
// due, and locking every callback would starve the rollover UPDATE for a busy payer.
887+
const periodQuery = payerSubscriptionId
888+
? tx
889+
.select({
890+
start: subscriptionTable.periodStart,
891+
end: subscriptionTable.periodEnd,
892+
})
893+
.from(subscriptionTable)
894+
.where(eq(subscriptionTable.id, payerSubscriptionId))
895+
.limit(1)
896+
: null
897+
const [currentPeriod] = periodQuery
898+
? Date.now() >= latestPeriod.end.getTime()
899+
? await periodQuery.for('share')
900+
: await periodQuery
901+
: []
902+
903+
// Only ever forward: a subscription period that does not start after the latest row's
904+
// keeps topping up that row, whatever the wall clock or a replayed webhook says. A start
905+
// that moved forward inside the old period (anchor reset, resync) still rolls, so the old
906+
// period's close is never topped up after the fact.
886907
const rolledPeriod =
887908
currentPeriod?.start &&
888909
currentPeriod.end &&
889-
currentPeriod.start.getTime() >= latestPeriod.end.getTime()
910+
currentPeriod.start.getTime() > latestPeriod.start.getTime()
890911
? { start: currentPeriod.start, end: currentPeriod.end }
891912
: null
892913
if (rolledPeriod && latest && chain.length >= MAX_CUMULATIVE_PERIOD_ROWS) {

‎apps/sim/lib/mothership/request/lifecycle/admission.test.ts‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ const attribution = {
2222
organizationId: 'original-org',
2323
billedAccountUserId: 'original-owner',
2424
billingEntity: { type: 'organization' as const, id: 'original-org' },
25-
billingPeriod: { start: '2026-09-01T00:00:00.000Z', end: '2026-10-01T00:00:00.000Z' },
25+
billingPeriod: { start: '2026-09-01T00:00:00.000Z', end: '2099-01-01T00:00:00.000Z' },
2626
payerSubscription: null,
2727
}
2828
const context = {
@@ -121,4 +121,18 @@ describe('continuation admission', () => {
121121
await expect(refusal).rejects.toBeInstanceOf(OrchestrationError)
122122
await expect(refusal).rejects.toThrow('blocked')
123123
})
124+
it('lets a leg run once its admitted period has ended instead of judging the old period', async () => {
125+
mockCheckExecutionUsageLimits.mockResolvedValue({ isExceeded: true, scope: 'payer' })
126+
127+
await expect(
128+
authorizeLifecycleContinuation({
129+
...context,
130+
billingAttribution: {
131+
...attribution,
132+
billingPeriod: { start: '2026-07-01T00:00:00.000Z', end: '2026-08-01T00:00:00.000Z' },
133+
},
134+
})
135+
).resolves.toBeUndefined()
136+
expect(mockCheckExecutionUsageLimits).not.toHaveBeenCalled()
137+
})
124138
})

‎apps/sim/lib/mothership/request/lifecycle/run.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2343,7 +2343,7 @@ describe('runCopilotLifecycle', () => {
23432343
organizationId: 'org-1',
23442344
billedAccountUserId: 'original-owner',
23452345
billingEntity: { type: 'organization' as const, id: 'org-1' },
2346-
billingPeriod: { start: '2026-07-01T00:00:00.000Z', end: '2026-08-01T00:00:00.000Z' },
2346+
billingPeriod: { start: '2026-07-01T00:00:00.000Z', end: '2099-01-01T00:00:00.000Z' },
23472347
payerSubscription: null,
23482348
}
23492349
setEnvFlags({ isHosted: true })
@@ -2430,7 +2430,7 @@ describe('runCopilotLifecycle', () => {
24302430
organizationId: 'org-1',
24312431
billedAccountUserId: 'user-1',
24322432
billingEntity: { type: 'organization' as const, id: 'org-1' },
2433-
billingPeriod: { start: '2026-07-01T00:00:00.000Z', end: '2026-08-01T00:00:00.000Z' },
2433+
billingPeriod: { start: '2026-07-01T00:00:00.000Z', end: '2099-01-01T00:00:00.000Z' },
24342434
payerSubscription: null,
24352435
}
24362436
setEnvFlags({ isHosted: true })

0 commit comments

Comments
 (0)