Skip to content

fix(billing): refuse charges into a period the terminal settlement already invoiced - #8505

Merged
waleedlatif1 merged 2 commits into
stagingfrom
fix/billing-no-topup-after-terminal-settlement
Oct 1, 2026
Merged

waleedlatif1 merged 2 commits into
stagingfrom
fix/billing-no-topup-after-terminal-settlement

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

This addresses the release review's finding "Closed periods receive later charges" (apps/sim/lib/billing/core/usage-log.ts). A cost callback that commits after a subscription's terminal settlement no longer tops up a period the final invoice has already summed.

Root cause

When a subscription is deleted, handleSubscriptionDeleted settles its terminal period straight away: it claims the period, sums overage, sends the final invoice and writes the bookkeeping. A deleted subscription's periodStart never moves again, so recordCumulativeUsage never sees a rolled period. A request that was still running therefore kept topping up the terminal period's row after the invoice had summed it, and no invoice ever read that spend. The terminal claim wrote no durable "settled" marker that a later callback could check: it only moved the close marker to periodStart, which every reader treats as "current period still open". This is pre-existing behaviour, not a regression in the release; prod tops up the frozen row the same way.

Fix

  • claimTerminalPeriod (lib/billing/cycle-close.ts) now moves the existing close marker (last_closed_period_start) to the terminal period's end (or to periodStart when there is no end). It does this under the FOR UPDATE lock it already held, and does it whether or not the marker was current.
  • recordCumulativeUsage reads that marker in its existing FOR SHARE subscription read. If the marker is at or past the end of the charge's target period, it throws CumulativeUsagePeriodClosedError. The two locks put every charge in a strict order against the settlement: a charge either commits before the claim, so the final invoice sums it, or it sees the marker and is refused.
  • update-cost maps that error to the existing non-retryable BILLING_PERIOD_ELAPSED (409) outcome, which the worker already treats as non-retryable.
  • No migration. The marker is an existing column. Every other reader compares it with >= periodStart, so a marker at periodEnd still reads as "current". Rollovers are unaffected: the sweep sets the marker to the new periodStart, which is always earlier than the end of any period a charge can target.

Behaviour changes

  • Spend reported for a deleted subscription's terminal period after its final settlement now returns 409 BILLING_PERIOD_ELAPSED (retryable: false). Before, it was silently written to a row no invoice would read.
  • Nothing changes for active subscriptions, rollovers, or callbacks that commit before the terminal claim.

Not changed

  • The "no usable current period bounds" half of the finding. A subscription row with a null periodStart never rolls, the sweep never closes it, and claimTerminalPeriod returns early for it. No invoiced period is being topped up there, so there is nothing to refuse against. Topping up the latest row is the correct behaviour for an active payer with unsynced bounds. Adding a wall-clock heuristic would refuse legitimate charges, so this PR leaves that path alone.

Test plan

  • usage-log.integration.ts: a charge that would roll into a period whose marker the terminal settlement has passed is refused, and the ledger is unchanged. Fails with the guard removed.
  • update-cost/route.integration.ts: a callback, then claimTerminalPeriod, then a later callback returns 409 BILLING_PERIOD_ELAPSED and leaves the original row's cost unchanged. Fails with the guard removed, and also fails with the claim reverted to periodStart.
  • bun run test:integration (filtered to the billing usage-log and update-cost suites, on throwaway containers)
  • Affected unit suites (lib/billing/cycle-close, lib/billing/webhooks, lib/billing/core, update-cost/route.test.ts, threshold-billing)
  • bun run lint, bun run type-check (apps/sim, packages/testing), bun run check:audits

@vercel

vercel Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
docs Ready Ready Preview Oct 1, 2026 6:29am UTC

Request Review

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 9 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@greptile-apps

greptile-apps Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Critical risk] Blocks charges after a subscription's final settlement.

The PR appears safe to merge; no outstanding findings remain.

Summary

This PR marks a deleted subscription’s terminal period as settled and refuses later cumulative charges into that period.

  • Maps the refusal to the existing non-retryable billing-period-elapsed callback outcome.
  • Adds integration coverage for post-settlement charges and both overlapping charge/claim orderings.
Diagram
sequenceDiagram
  participant Charge as Cost callback
  participant DB as Subscription row
  participant Claim as Terminal settlement
  Charge->>DB: Read period and marker FOR SHARE
  Claim->>DB: Claim terminal period FOR UPDATE
  Note over Charge,Claim: Row locks order the charge and claim
  alt Charge commits first
    Claim->>DB: Advance marker to period end
    Note over Claim: Final sum includes committed charge
  else Claim commits first
    Charge->>DB: Read settled marker
    Note over Charge: Refuse charge with billing-period-elapsed outcome
  end
Loading

Reviews (2) · Last reviewed commit: "test(billing): cover a terminal claim ov..."

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 9 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

Comment thread apps/sim/app/api/billing/update-cost/route.integration.ts
…ready invoiced

A subscription's deletion settles its terminal period at once (claim, overage,
final invoice, bookkeeping), but recordCumulativeUsage kept topping up that
period's row for a still-running request because the subscription's period
never rolls after deletion. That spend was never billed.

The terminal claim now advances the close marker to the period's end under its
FOR UPDATE lock, and recordCumulativeUsage reads the marker in its existing FOR
SHARE read: a charge whose target period ends at or before the marker throws
CumulativeUsagePeriodClosedError, which update-cost answers with the existing
non-retryable BILLING_PERIOD_ELAPSED outcome, so the worker quarantines the leg
for reconciliation instead of the spend disappearing into an invoiced period.

No migration: the marker is an existing column, and every reader treats a
marker at or past periodStart as current, so v0.9.6 behaves unchanged.
Both lock orders against real PostgreSQL: a claim waits for an in-flight
charge so the final sum includes it, and a charge that waited on an
in-flight claim is refused.
@waleedlatif1
waleedlatif1 force-pushed the fix/billing-no-topup-after-terminal-settlement branch from 8af8bf8 to 37a2eea Compare October 1, 2026 06:27
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 9 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@waleedlatif1
waleedlatif1 merged commit c57c905 into staging Oct 1, 2026
32 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/billing-no-topup-after-terminal-settlement branch October 1, 2026 06:43

This branch was successfully deployed

1 active deployment
Preview — 37a2eeab Deployed Oct 1, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant