Skip to content

Commit 866d3f0

Browse files
icecrasher321claude
andcommitted
fix(billing): reject lagging terminal claims and thread billingInterval into deletion bookkeeping
claimTerminalPeriod no longer advances the marker over an unclosed elapsed period: a lagging marker is reported without a write so the deletion handler can run the elapsed close once more (healing a rollover that committed between close and claim) and only then seal the marker explicitly, with an error log, when the period is genuinely unclosable. Sealing preserves the in-flight-sweep abort guarantee. Deletion bookkeeping now passes the subscription's billingInterval through, so an enterprise reporting subscription whose interval lives on the row column (not metadata) still resolves as reporting-anchored and keeps its bookkeeping no-op, matching every other resolver call site. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 482413e commit 866d3f0

3 files changed

Lines changed: 66 additions & 18 deletions

File tree

apps/sim/lib/billing/cycle-close.test.ts

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -550,7 +550,7 @@ describe('claimTerminalPeriod', () => {
550550
dbChainMockFns.returning.mockResolvedValue([{ id: 'sub-1' }])
551551
})
552552

553-
it('claims the marker from the fresh row period and returns it for settlement', async () => {
553+
it('returns the fresh period without rewriting a current marker', async () => {
554554
queueTableRows(schemaMock.subscription, [
555555
{
556556
periodStart: PERIOD_START,
@@ -563,13 +563,10 @@ describe('claimTerminalPeriod', () => {
563563

564564
expect(terminal.periodStart).toEqual(PERIOD_START)
565565
expect(terminal.markerWasCurrent).toBe(true)
566-
const markerSet = dbChainMockFns.set.mock.calls.find(
567-
(call) => (call[0] as Record<string, unknown>).lastClosedPeriodStart instanceof Date
568-
)
569-
expect(markerSet).toBeDefined()
566+
expect(dbChainMockFns.set).not.toHaveBeenCalled()
570567
})
571568

572-
it('reports a lagging marker so the terminal settlement ignores the stale tracker', async () => {
569+
it('reports a lagging marker without jumping it, so the caller can close and re-claim', async () => {
573570
queueTableRows(schemaMock.subscription, [
574571
{
575572
periodStart: PERIOD_START,
@@ -581,6 +578,25 @@ describe('claimTerminalPeriod', () => {
581578
const terminal = await claimTerminalPeriod('sub-1')
582579

583580
expect(terminal.markerWasCurrent).toBe(false)
581+
expect(dbChainMockFns.set).not.toHaveBeenCalled()
582+
})
583+
584+
it('seals a lagging marker on request, forgiving the unclosed period loudly', async () => {
585+
queueTableRows(schemaMock.subscription, [
586+
{
587+
periodStart: PERIOD_START,
588+
periodEnd: new Date('2026-09-01T00:00:00.000Z'),
589+
lastClosedPeriodStart: PREV_PERIOD_START,
590+
},
591+
])
592+
593+
const terminal = await claimTerminalPeriod('sub-1', { sealLagging: true })
594+
595+
expect(terminal.markerWasCurrent).toBe(false)
596+
const markerSet = dbChainMockFns.set.mock.calls.find(
597+
(call) => (call[0] as Record<string, unknown>).lastClosedPeriodStart instanceof Date
598+
)
599+
expect(markerSet).toBeDefined()
584600
})
585601

586602
it('returns nulls without claiming when the subscription has no period', async () => {

apps/sim/lib/billing/cycle-close.ts

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -197,14 +197,25 @@ export async function closeElapsedPeriodBeforeDeletion(subscriptionId: string):
197197
* the same period. Call `closeElapsedPeriodBeforeDeletion` first so a lagging
198198
* elapsed period is settled rather than jumped. Returns the fresh period
199199
* bounds for the deletion flow to settle against, plus `markerWasCurrent`:
200-
* whether the close marker had already caught up to the terminal period
201-
* before this claim. The `billedOverageThisPeriod` tracker only ever holds
202-
* collections for the period that began at the marker (the threshold gate
203-
* blocks settlement whenever the marker lags), so the terminal settlement
204-
* must ignore the tracker when the marker was still lagging — its contents
205-
* belong to a forgiven elapsed period, never to the terminal window.
200+
* whether the close marker had already caught up to the terminal period.
201+
* The `billedOverageThisPeriod` tracker only ever holds collections for the
202+
* period that began at the marker (the threshold gate blocks settlement
203+
* whenever the marker lags), so the terminal settlement must ignore the
204+
* tracker when the marker was still lagging — its contents belong to a
205+
* forgiven elapsed period, never to the terminal window.
206+
*
207+
* A lagging marker means an elapsed period is still unclosed — either the
208+
* preceding close deferred, or a rollover committed between that close and
209+
* this claim. By default the claim then leaves the marker untouched so the
210+
* caller can run the close again and re-claim; `sealLagging` advances the
211+
* marker over the unclosed period anyway (logging the forgiveness), which
212+
* also guarantees an in-flight sweep that selected this subscription before
213+
* its status changed aborts its own conflicting close.
206214
*/
207-
export async function claimTerminalPeriod(subscriptionId: string): Promise<{
215+
export async function claimTerminalPeriod(
216+
subscriptionId: string,
217+
options: { sealLagging?: boolean } = {}
218+
): Promise<{
208219
periodStart: Date | null
209220
periodEnd: Date | null
210221
markerWasCurrent: boolean
@@ -229,7 +240,17 @@ export async function claimTerminalPeriod(subscriptionId: string): Promise<{
229240
const markerWasCurrent =
230241
!!row.lastClosedPeriodStart &&
231242
row.lastClosedPeriodStart.getTime() >= row.periodStart.getTime()
232-
await claimCloseMarker(tx, subscriptionId, row.periodStart)
243+
if (!markerWasCurrent && options.sealLagging) {
244+
logger.error(
245+
'Sealing an unclosed elapsed period at terminal claim; residual overage forgiven',
246+
{
247+
subscriptionId,
248+
marker: row.lastClosedPeriodStart?.toISOString() ?? null,
249+
periodStart: row.periodStart.toISOString(),
250+
}
251+
)
252+
await claimCloseMarker(tx, subscriptionId, row.periodStart)
253+
}
233254
return { periodStart: row.periodStart, periodEnd: row.periodEnd, markerWasCurrent }
234255
})
235256
}
@@ -713,6 +734,7 @@ export async function writeFinalPeriodBookkeeping(sub: {
713734
id: string
714735
plan: string | null
715736
referenceId: string
737+
billingInterval?: string | null
716738
periodStart?: Date | null
717739
periodEnd?: Date | null
718740
metadata?: unknown

apps/sim/lib/billing/webhooks/subscription.ts

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,7 @@ export async function handleSubscriptionDeleted(
260260
referenceId: string
261261
stripeSubscriptionId: string | null
262262
seats?: number | null
263+
billingInterval?: string | null
263264
periodStart?: Date | null
264265
periodEnd?: Date | null
265266
metadata?: unknown
@@ -293,10 +294,17 @@ export async function handleSubscriptionDeleted(
293294

294295
// Then claim the terminal period BEFORE computing or charging: this
295296
// reads the row's fresh period (webhook payloads can be stale across
296-
// a rollover) and serializes with the cycle-close sweep — an
297-
// in-flight close fails its guarded marker claim and rolls back, so
298-
// both paths can never bill the same period.
299-
const terminal = await claimTerminalPeriod(subscription.id)
297+
// a rollover) and serializes with the cycle-close sweep. A lagging
298+
// marker here means the close above deferred OR a rollover committed
299+
// in between — run the close once more (it settles a freshly elapsed
300+
// period; a deferred close defers again, loudly), then seal so the
301+
// marker cannot be raced indefinitely and an in-flight sweep aborts
302+
// its conflicting close.
303+
let terminal = await claimTerminalPeriod(subscription.id)
304+
if (!terminal.markerWasCurrent) {
305+
await closeElapsedPeriodBeforeDeletion(subscription.id)
306+
terminal = await claimTerminalPeriod(subscription.id, { sealLagging: true })
307+
}
300308
const settlementPeriod = {
301309
periodStart: terminal.periodStart ?? subscription.periodStart ?? null,
302310
periodEnd: terminal.periodEnd ?? subscription.periodEnd ?? null,
@@ -313,6 +321,7 @@ export async function handleSubscriptionDeleted(
313321
id: subscription.id,
314322
plan: subscription.plan,
315323
referenceId: subscription.referenceId,
324+
billingInterval: subscription.billingInterval ?? null,
316325
...settlementPeriod,
317326
metadata: subscription.metadata,
318327
})
@@ -442,6 +451,7 @@ export async function handleSubscriptionDeleted(
442451
id: subscription.id,
443452
plan: subscription.plan,
444453
referenceId: subscription.referenceId,
454+
billingInterval: subscription.billingInterval ?? null,
445455
...settlementPeriod,
446456
metadata: subscription.metadata,
447457
})

0 commit comments

Comments
 (0)