From 46e6684b7c6904fa82fb248d79efc0cd7656ce45 Mon Sep 17 00:00:00 2001 From: Sheng Kun Chang Date: Sun, 5 Jul 2026 06:17:37 +0800 Subject: [PATCH 1/2] feat: account-wide pooled module overage (migration 030) Move module overage from PER-APP to a single ACCOUNT-WIDE POOL of 5 included modules, per the owner spec 2026-07-05 (a deliberate reversal of the base-fee v1 per-app tier). - Schema (030): accounts.overage_since (one grace timer per account) + account_overage_snapshots (per-(account, period) charge ledger, the double-charge guard mirroring app_base_snapshots). - Pricing: per-app base is now FLAT $20; overage = $3 x max(0, SUM(live-app module_count) - 5) at the account level (usage.AccountOverageMicros / ProratedOverageMicros). - Timer: RegisterApp / SyncAppModules recompute the pool after every module_count write and arm/clear accounts.overage_since (first-cross-wins / idempotent clear, no refund on drop). - Mid-period grace charge: a new cmd/billing-cycle sweep charges the pooled overage prorated from grace-end to the period end once the 3-day grace window elapses (a deliberate mid-period charge; base-fee mid-period behavior is unchanged). Deterministic per-(account, period) Stripe idem keys + the snapshot ledger make it money-safe. - Boundary advance leg: each app now contributes only its flat base; a SEPARATE pooled-overage term bills the closing period ONCE, skipped when the sweep already billed it (snapshot guard), source='advance'. - Display: GetAccountBillResponse gains AccountOverageMicros (additive, snapshot-first else live pooled estimate); per-app base display is flat. Tests cover: pool crossing 5 arms the timer, dropping under clears it, grace holds for exactly 3 days, the mid-period charge fires once and is excluded from the boundary (no double-charge), interleaved installs sum to the account pool, and uninstall never refunds. mirrorstack-docs/db/ms_billing/ needs a companion update for the new account_overage_snapshots table + accounts.overage_since column. Co-Authored-By: Claude Opus 4.8 --- cmd/account-api/infra_route_test.go | 6 + cmd/billing-cycle/main.go | 55 ++- cmd/infra-egress-sync/main_test.go | 6 + internal/account/cycle/apps.go | 49 ++- internal/account/cycle/apps_test.go | 52 ++- internal/account/cycle/charge.go | 96 +++-- internal/account/cycle/overage.go | 268 ++++++++++++++ internal/account/cycle/overage_test.go | 334 ++++++++++++++++++ internal/account/cycle/service_test.go | 97 +++++ internal/account/cycle/store.go | 151 ++++++++ internal/account/cycle/types.go | 21 +- internal/account/db/models.go | 13 + internal/account/db/overage.sql.go | 201 +++++++++++ internal/account/db/queries/overage.sql | 83 +++++ internal/account/usage/accountbill.go | 46 ++- internal/account/usage/accountbill_test.go | 57 +-- internal/account/usage/basefee.go | 59 +++- internal/account/usage/basefee_test.go | 51 ++- internal/account/usage/bill.go | 56 +-- internal/account/usage/bill_test.go | 46 +-- internal/account/usage/service_test.go | 42 +++ internal/account/usage/store.go | 42 +++ internal/account/usage/types.go | 21 +- .../billing/030_account_wide_overage.down.sql | 10 + .../billing/030_account_wide_overage.up.sql | 88 +++++ 25 files changed, 1775 insertions(+), 175 deletions(-) create mode 100644 internal/account/cycle/overage.go create mode 100644 internal/account/cycle/overage_test.go create mode 100644 internal/account/db/overage.sql.go create mode 100644 internal/account/db/queries/overage.sql create mode 100644 migrations/billing/030_account_wide_overage.down.sql create mode 100644 migrations/billing/030_account_wide_overage.up.sql diff --git a/cmd/account-api/infra_route_test.go b/cmd/account-api/infra_route_test.go index f25aa33..40f705f 100644 --- a/cmd/account-api/infra_route_test.go +++ b/cmd/account-api/infra_route_test.go @@ -86,6 +86,12 @@ func (stubUsageStore) AppIDsWithUsage(context.Context, uuid.UUID, time.Time, tim func (stubUsageStore) MirroredAppIDs(context.Context, uuid.UUID, time.Time, time.Time) ([]uuid.UUID, error) { return nil, nil } +func (stubUsageStore) PooledModuleCount(context.Context, uuid.UUID) (int, error) { + return 0, nil +} +func (stubUsageStore) AccountOverageSnapshot(context.Context, uuid.UUID, time.Time) (int64, bool, error) { + return 0, false, nil +} func newRouterForTest(t *testing.T) http.Handler { t.Helper() diff --git a/cmd/billing-cycle/main.go b/cmd/billing-cycle/main.go index fc70e75..64daec6 100644 --- a/cmd/billing-cycle/main.go +++ b/cmd/billing-cycle/main.go @@ -77,7 +77,9 @@ func main() { "as_of", res.AsOf, "activated", res.Activated, "rolled_up", res.RolledUp, "processed", res.Processed, "charged", res.Charged, "skipped_no_pm", res.SkippedNoPM, "zero_arrears", res.ZeroArrears, - "already_run", res.AlreadyRun, "failed_runs", res.FailedRuns, "failed", res.Failed) + "already_run", res.AlreadyRun, "failed_runs", res.FailedRuns, "failed", res.Failed, + "overage_candidates", res.OverageCandidates, "overage_charged", res.OverageCharged, + "overage_skipped", res.OverageSkipped, "overage_failed", res.OverageFailed) if res.Failed > 0 { os.Exit(1) } @@ -107,7 +109,9 @@ func handler(svc *cycle.Service) func(context.Context, events.CloudWatchEvent) e "as_of", res.AsOf, "activated", res.Activated, "rolled_up", res.RolledUp, "processed", res.Processed, "charged", res.Charged, "skipped_no_pm", res.SkippedNoPM, "zero_arrears", res.ZeroArrears, - "already_run", res.AlreadyRun, "failed_runs", res.FailedRuns, "failed", res.Failed) + "already_run", res.AlreadyRun, "failed_runs", res.FailedRuns, "failed", res.Failed, + "overage_candidates", res.OverageCandidates, "overage_charged", res.OverageCharged, + "overage_skipped", res.OverageSkipped, "overage_failed", res.OverageFailed) // A per-account charge failure is recorded (billing_runs status='failed') // and does NOT fail the batch — the next cycle retries it. The handler // returns nil so EventBridge doesn't replay the whole batch. @@ -127,6 +131,12 @@ type cycleResult struct { AlreadyRun int FailedRuns int // per-account charge runs that ended status='failed' Failed int // errors (rollup error, charge error, or list error) + + // Mid-period account-wide overage grace sweep (migration 030). + OverageCandidates int // accounts past the grace window this sweep evaluated + OverageCharged int // accounts whose pooled overage was invoiced mid-period + OverageSkipped int // evaluated but not charged (already billed / under pool / no PM / 0 cents) + OverageFailed int // per-account overage-charge errors (counted, never abort) } // runCycle closes every card-bound account's just-ended ANCHORED period as of @@ -222,9 +232,50 @@ func runCycle(ctx context.Context, svc *cycle.Service, at time.Time) cycleResult } tally(&res, a.ID, chargeSummary) } + + // Mid-period account-wide overage grace sweep (migration 030): independent of + // the boundary close above. Every account whose pooled module overage has + // survived the grace window and whose CURRENT period has no pooled-overage + // snapshot yet is charged the prorated overage now (a deliberate mid-period + // charge). Idempotent per (account, period) via the snapshot ledger + the + // deterministic Stripe idem keys, so firing daily never double-charges. + runOverageSweep(ctx, svc, at, &res) return res } +// runOverageSweep charges the mid-period account-wide pooled overage for every +// account past the grace window as of `at`. A single account's error is logged + +// counted but never aborts the sweep (like the boundary loop). +func runOverageSweep(ctx context.Context, svc *cycle.Service, at time.Time, res *cycleResult) { + cands, err := svc.AccountsInOverageGrace(ctx, at) + if err != nil { + slog.ErrorContext(ctx, "list overage-grace accounts failed", "error", err) + res.Failed++ + return + } + res.OverageCandidates = len(cands) + for _, c := range cands { + summary, err := svc.ChargeAccountOverage(ctx, c, at) + if err != nil { + slog.ErrorContext(ctx, "account overage charge failed", "account_id", c.ID, "error", err) + res.OverageFailed++ + res.Failed++ + continue + } + if summary.Status == cycle.OverageCharged { + res.OverageCharged++ + } else { + res.OverageSkipped++ + } + slog.Info("account overage grace sweep", + "account_id", c.ID, + "status", string(summary.Status), + "over_count", summary.OverCount, + "charged_cents", summary.ChargedCents, + "stripe_invoice_id", summary.StripeInvoiceID) + } +} + // tally classifies one account's charge summary for the run totals + a // per-account info log. RunBillingCycle returns (nil, err) on a charge failure // — that path is counted in runCycle, not here — but the RunStatusFailed case is diff --git a/cmd/infra-egress-sync/main_test.go b/cmd/infra-egress-sync/main_test.go index a5bef57..cb5e1a9 100644 --- a/cmd/infra-egress-sync/main_test.go +++ b/cmd/infra-egress-sync/main_test.go @@ -128,6 +128,12 @@ func (f *fakeStore) AppIDsWithUsage(_ context.Context, _ uuid.UUID, _, _ time.Ti func (f *fakeStore) MirroredAppIDs(_ context.Context, _ uuid.UUID, _, _ time.Time) ([]uuid.UUID, error) { return nil, nil } +func (f *fakeStore) PooledModuleCount(_ context.Context, _ uuid.UUID) (int, error) { + return 0, nil +} +func (f *fakeStore) AccountOverageSnapshot(_ context.Context, _ uuid.UUID, _ time.Time) (int64, bool, error) { + return 0, false, nil +} func newSvc(store usage.Store) *usage.Service { return usage.NewService(store) } diff --git a/internal/account/cycle/apps.go b/internal/account/cycle/apps.go index 6952799..9e50677 100644 --- a/internal/account/cycle/apps.go +++ b/internal/account/cycle/apps.go @@ -98,11 +98,14 @@ type SyncAppModulesResponse struct { // PM; otherwise the row is recorded and NO invoice is created (no // retroactive catch-up on activation in v1). The one-shot guard // proration_invoice_id short-circuits a retry that already charged. -// Amount = ProratedBaseMicros(AppBaseFeeMicros(base, module_count), -// created_at, the anchored period CONTAINING created_at) — whole UTC -// days, creation day inclusive, round-half-up — converted micros → whole -// cents at the Stripe boundary like every other charge. 0 cents → row -// recorded, no invoice. +// Amount = ProratedBaseMicros(BaseFeeMicros, created_at, the anchored +// period CONTAINING created_at) — the FLAT per-app base only (module +// overage is account-wide pooled, migration 030), whole UTC days, creation +// day inclusive, round-half-up — converted micros → whole cents at the +// Stripe boundary like every other charge. 0 cents → row recorded, no +// invoice. The insert ALSO recomputes the account's pooled-overage grace +// timer (recomputeAccountOverage) so a creation that pushes the account +// over IncludedModules arms accounts.overage_since. // // INVARIANT (charge-leg ownership): the proration window is derived from // the read-back mirror row's created_at (the stable first-registration @@ -175,6 +178,16 @@ func (s *Service) RegisterApp(ctx context.Context, req RegisterAppRequest) (*Reg } resp := &RegisterAppResponse{AppID: app.AppID, AccountID: app.AccountID} + // Recompute the account-wide pooled overage timer (migration 030) after the + // insert: the new app's module_count may push the account's pool over + // IncludedModules (arming accounts.overage_since) — independent of the + // per-app creation-proration charge below, which only ever bills the FLAT + // base. Idempotent (Start/Clear are first-crossing-wins / no-op), so a retry + // re-runs it harmlessly. + if err := s.recomputeAccountOverage(ctx, app.AccountID); err != nil { + return nil, err + } + // One-shot guard: a prior attempt already charged (or a concurrent one // won). Idempotent success — NEVER a second invoice. if app.ProrationInvoiceID != "" { @@ -220,10 +233,10 @@ func (s *Service) RegisterApp(ctx context.Context, req RegisterAppRequest) (*Reg return resp, nil } - prorated := usage.ProratedBaseMicros( - usage.AppBaseFeeMicros(usage.BaseFeeMicros, app.ModuleCount), - app.CreatedAt, periodStart, periodEnd, - ) + // The creation-proration charge is the FLAT per-app base only (migration + // 030 — module overage is account-wide pooled, billed by the grace sweep / + // boundary at the account level, never folded into this per-app proration). + prorated := usage.ProratedBaseMicros(usage.BaseFeeMicros, app.CreatedAt, periodStart, periodEnd) cents, err := centsFromMicros(prorated) if err != nil { return nil, billing.Internal("micros to cents conversion failed", err) @@ -308,8 +321,14 @@ func (s *Service) RegisterApp(ctx context.Context, req RegisterAppRequest) (*Reg // — there is no future base for the tier to move); // - an unknown app_id is NOT_FOUND (the platform must RegisterApp first). // -// Count changes take effect at the NEXT boundary charge (D1b — no mid-period -// micro-invoices for module #6, no mid-period refunds for uninstalls). +// After any module_count / delete write it recomputes the account's pooled- +// overage grace timer (recomputeAccountOverage). The per-app FLAT base still +// takes effect at the NEXT boundary (no mid-period base micro-invoice / refund), +// but the ACCOUNT-WIDE pooled overage is now the DELIBERATE exception to the old +// D1b "no mid-period charges" rule: crossing the pooled IncludedModules arms a +// grace timer, and the mid-period sweep charges the pooled overage once the +// grace window elapses (migration 030). Dropping back under the pool clears the +// timer (no refund of anything already charged, D1e). func (s *Service) SyncAppModules(ctx context.Context, req SyncAppModulesRequest) (*SyncAppModulesResponse, error) { if req.AppID == uuid.Nil { return nil, billing.InvalidInput("app_id required") @@ -345,6 +364,14 @@ func (s *Service) SyncAppModules(ctx context.Context, req SyncAppModulesRequest) app.ModuleCount = *req.ModuleCount } + // Recompute the account-wide pooled-overage grace timer (migration 030) after + // the write: a count bump or delete can push the pool over IncludedModules + // (arm overage_since) or drop it back under (clear it). Idempotent, so a + // pure-delete-of-an-already-deleted-app retry re-runs it harmlessly. + if err := s.recomputeAccountOverage(ctx, app.AccountID); err != nil { + return nil, err + } + return &SyncAppModulesResponse{ AppID: app.AppID, ModuleCount: app.ModuleCount, diff --git a/internal/account/cycle/apps_test.go b/internal/account/cycle/apps_test.go index fb0195c..467869c 100644 --- a/internal/account/cycle/apps_test.go +++ b/internal/account/cycle/apps_test.go @@ -85,11 +85,13 @@ func TestRegisterApp_ChargesCreationProration(t *testing.T) { require.Equal(t, 0, snap.snap.ModuleCount) } -func TestRegisterApp_ProrationIncludesModuleOverage(t *testing.T) { - // module_count 7 at creation → base_at_creation = 20e6 + 2×3e6 = 26e6; - // 15 of 30 remaining days → 13e6 micros → 1300 cents. +func TestRegisterApp_ProrationIsFlatBaseRegardlessOfModuleCount(t *testing.T) { + // Migration 030: module overage is account-wide POOLED, so the per-app + // creation proration is the FLAT $20 base regardless of module_count — a + // 7-module app prorates EXACTLY like a 0-module app. 15 of 30 remaining days + // → 20e6 × 15/30 = 10e6 micros → 1000 cents (NOT the pre-030 26e6 → 1300). store := newFakeStore() - user, _ := registeredAccount(store) + user, acct := registeredAccount(store) sc := newFakeStripe() resp, err := appsSvc(store, sc).RegisterApp(context.Background(), cycle.RegisterAppRequest{ @@ -99,7 +101,10 @@ func TestRegisterApp_ProrationIncludesModuleOverage(t *testing.T) { CreatedAt: time.Date(2026, 6, 19, 12, 0, 0, 0, time.UTC), }) require.NoError(t, err) - require.EqualValues(t, 1300, resp.ProrationCents) + require.EqualValues(t, 1000, resp.ProrationCents, "flat base proration — overage is pooled, not per-app") + + // The 7-module create pushed the account pool over 5, arming the grace timer. + require.Contains(t, store.overageSince, acct, "crossing the pooled 5 arms overage_since") } func TestRegisterApp_DefaultsCreatedAtToNow(t *testing.T) { @@ -445,9 +450,11 @@ func seedAppCreated(store *fakeStore, accountID uuid.UUID, moduleCount int, dele return id } -func TestRunBillingCycle_InvoicesUsagePlusAdvanceBase(t *testing.T) { - // arrears 1e6 (usage) + base (20e6 flat + [20e6 + 1×3e6] for a 6-module - // app) = 44e6 total → 4400 cents on ONE invoice. +func TestRunBillingCycle_InvoicesUsagePlusAdvanceBasePlusPooledOverage(t *testing.T) { + // Migration 030: arrears 1e6 (usage) + FLAT base (2 × 20e6 = 40e6) + the + // account-wide POOLED overage (pool = 0 + 6 = 6 → 1 over → $3 = 3e6) = 44e6 + // total → 4400 cents on ONE invoice. Same total as the pre-030 per-app tier + // for this single-account case, but split base-vs-overage differently. store := newFakeStore() store.chargedTotal = 1_000_000 store.hasPM = true @@ -460,14 +467,14 @@ func TestRunBillingCycle_InvoicesUsagePlusAdvanceBase(t *testing.T) { require.NoError(t, err) require.Equal(t, cycle.RunStatusInvoiced, resp.Status) require.EqualValues(t, 1_000_000, resp.ArrearsMicros) - require.EqualValues(t, 43_000_000, resp.AdvanceBaseMicros) // 20e6 + 23e6 + require.EqualValues(t, 40_000_000, resp.AdvanceBaseMicros) // 2 × flat $20 (no per-app overage) + require.EqualValues(t, 3_000_000, resp.AccountOverageMicros) // pool 6 → 1 over → $3 require.EqualValues(t, 4_400, resp.ChargedCents) - require.Len(t, sc.itemCalls, 1, "usage + base pool into ONE line on ONE invoice") + require.Len(t, sc.itemCalls, 1, "usage + base + pooled overage pool into ONE line on ONE invoice") require.EqualValues(t, 4_400, sc.itemCalls[0].amountCfg) - // The advance leg froze one migration-028 snapshot per billed app for the - // NEW window [Jul 1, Aug 1) — count + base as invoiced, so the display can - // never drift after later syncs. + // The advance leg froze one migration-028 base snapshot per billed app for + // the NEW window [Jul 1, Aug 1) — now the FLAT base (overage is pooled). fs, ok := store.baseSnapshots[snapKey{flat, periodEnd}] require.True(t, ok) require.Equal(t, "advance", fs.source) @@ -475,8 +482,16 @@ func TestRunBillingCycle_InvoicesUsagePlusAdvanceBase(t *testing.T) { require.Equal(t, periodEnd.AddDate(0, 1, 0), fs.snap.PeriodEnd) ts, ok := store.baseSnapshots[snapKey{tiered, periodEnd}] require.True(t, ok) - require.EqualValues(t, 23_000_000, ts.snap.BaseMicros) + require.EqualValues(t, usage.BaseFeeMicros, ts.snap.BaseMicros, "per-app base is flat now") require.Equal(t, 6, ts.snap.ModuleCount) + + // And ONE account_overage_snapshots row (source='advance') for the closing + // period, so the mid-period sweep + display agree it is already billed. + ov, ok := store.overageSnaps[acctSnapKey{chargeAccount, periodStart}] + require.True(t, ok, "the boundary must freeze the pooled overage it billed") + require.Equal(t, "advance", ov.Source) + require.Equal(t, 1, ov.OverCount) + require.EqualValues(t, 3_000_000, ov.ChargedMicros) } func TestRunBillingCycle_BaseOnlyInvoiceWhenNoUsage(t *testing.T) { @@ -575,11 +590,14 @@ func TestRunBillingCycle_ExcludesAppCreatedInsideNewPeriod(t *testing.T) { require.False(t, ok) // NEXT boundary (closing [Jul 1, Aug 1)): the app now pre-exists the newer - // period and joins the advance leg — billed exactly once, never twice. + // period and joins the advance leg — billed exactly once, never twice. Both + // apps contribute the FLAT $20 base (2 × 20e6); the account pool is now 0 + 6 + // = 6 → 1 over → the pooled overage ($3) is charged at the account level. resp, err = svc.RunBillingCycle(context.Background(), chargeAccount, periodEnd, periodEnd.AddDate(0, 1, 0), 0) require.NoError(t, err) - require.EqualValues(t, usage.BaseFeeMicros+usage.AppBaseFeeMicros(usage.BaseFeeMicros, 6), resp.AdvanceBaseMicros, - "the new app joins the advance leg at the NEXT boundary") + require.EqualValues(t, 2*usage.BaseFeeMicros, resp.AdvanceBaseMicros, + "the new app joins the advance leg at the NEXT boundary (flat base)") + require.EqualValues(t, 3_000_000, resp.AccountOverageMicros, "pool 6 → 1 over → $3 pooled overage") } func TestRunBillingCycle_ReclaimedRunNoDoubleBase(t *testing.T) { diff --git a/internal/account/cycle/charge.go b/internal/account/cycle/charge.go index cde5378..a562cc4 100644 --- a/internal/account/cycle/charge.go +++ b/internal/account/cycle/charge.go @@ -31,20 +31,24 @@ import ( // LIVE ms_billing.apps rows (deleted_at IS NULL — a deleted app stops // accruing base, D1e, though its usage arrears above still bill) that // EXISTED BEFORE the new period opened (created_at < the closed window's -// period_end) of AppBaseFeeMicros = BaseFee + Overage × max(0, -// module_count − included). An app created INSIDE the new period is -// excluded — RegisterApp's creation-proration leg already charged its -// new-period base (full or prorated); it joins the advance leg at the -// NEXT boundary. module_count is snapshotted AT CHARGE TIME (D1b — -// mid-period installs / uninstalls take effect at this boundary, never -// mid-period), and each billed app-period is frozen into -// ms_billing.app_base_snapshots (migration 028) so the display always -// shows what was invoiced. The allowance nets USAGE only, never the base +// period_end) of the FLAT BaseFeeMicros (module overage is account-wide +// pooled now, migration 030 — no longer a per-app tier here). PLUS a +// SEPARATE account-level POOLED overage term for the CLOSING period ($3 × +// max(0, Σ live-app module_count − included)), charged ONCE only when the +// mid-period grace sweep did NOT already bill it (no +// account_overage_snapshots row for the period — the double-charge guard); +// when the boundary does charge it, it writes its own 'advance' snapshot. +// An app created INSIDE the new period is excluded — RegisterApp's +// creation-proration leg already charged its new-period base (full or +// prorated); it joins the advance leg at the NEXT boundary. module_count is +// snapshotted AT CHARGE TIME, and each billed app-period is frozen into +// ms_billing.app_base_snapshots (migration 028) so the display always shows +// what was invoiced. The allowance nets USAGE only, never the base/overage // (it offsets ModuleUsage+Infra in the display math too). An account with // NO mirror rows (pre-backfill) gets base 0 — exactly the pre-027 // arrears-only invoice — until the api-platform backfill populates the // roster. -// 4. arrears + base == 0 (BOTH zero) → MarkBillingRun('invoiced') with NO +// 4. arrears + base + pooled overage == 0 (ALL zero) → MarkBillingRun('invoiced') with NO // Stripe call. We NEVER auto-create a Stripe Customer with nothing to // charge (design §4 Axis 4). // 5. no usable default PM → MarkBillingRun('skipped_no_pm'). The usage is @@ -142,19 +146,44 @@ func (s *Service) RunBillingCycle(ctx context.Context, accountID uuid.UUID, peri if err != nil { return nil, billing.Internal("live app roster read failed", err) } + // Each live app contributes ONLY its FLAT base (migration 030 — module + // overage is account-wide pooled, no longer a per-app tier). The pooled + // module count for the ACCOUNT overage term below is Σ these apps' counts + // (the same roster the base bills — apps created inside the new period are + // excluded, so the closing period's pool is exact). var advanceBase int64 + var pooledModuleCount int for _, a := range apps { - advanceBase += usage.AppBaseFeeMicros(usage.BaseFeeMicros, a.ModuleCount) + advanceBase += usage.BaseFeeMicros + pooledModuleCount += a.ModuleCount } - summary := &ChargeSummary{FirstRun: true, ArrearsMicros: arrears, AdvanceBaseMicros: advanceBase} + // SEPARATE account-level POOLED overage term for the closing period, charged + // ONCE per account. If this period's pooled overage already has an + // account_overage_snapshots row (the mid-period grace sweep billed it), it is + // NOT charged again here — the ledger is the double-charge guard. Otherwise + // (grace never expired within the period, or overage started too late for the + // sweep to run) the boundary charges the FULL pooled overage for the account + // and writes its own snapshot (source='advance'), so every over-the-pool + // period is billed exactly once. + _, overageAlreadyBilled, err := s.store.AccountOverageSnapshot(ctx, accountID, periodStart) + if err != nil { + return nil, billing.Internal("account overage snapshot lookup failed", err) + } + var advanceOverage int64 + overCount := pooledModuleCount - usage.IncludedModules + if !overageAlreadyBilled && overCount > 0 { + advanceOverage = usage.AccountOverageMicros(pooledModuleCount) + } + + summary := &ChargeSummary{FirstRun: true, ArrearsMicros: arrears, AdvanceBaseMicros: advanceBase, AccountOverageMicros: advanceOverage} - // Zero-skip: only when arrears AND base are BOTH zero (empty/zero period - // with no live apps) is there nothing to invoice — mark invoiced with NO - // Stripe call, never auto-create a Customer with nothing to charge. A zero - // total can never breach a limit/ceiling, so this short-circuits ahead of - // the risk gate. - if arrears == 0 && advanceBase == 0 { + // Zero-skip: only when arrears, base AND pooled overage are ALL zero + // (empty/zero period with no live apps) is there nothing to invoice — mark + // invoiced with NO Stripe call, never auto-create a Customer with nothing to + // charge. A zero total can never breach a limit/ceiling, so this + // short-circuits ahead of the risk gate. + if arrears == 0 && advanceBase == 0 && advanceOverage == 0 { if err := s.store.MarkBillingRun(ctx, runID, RunStatusInvoiced, "", 0); err != nil { return nil, billing.Internal("mark billing run (zero arrears) failed", err) } @@ -268,9 +297,10 @@ func (s *Service) RunBillingCycle(ctx context.Context, accountID uuid.UUID, peri } // One invoice, one pooled line: closed period's netted usage arrears + the - // new period's advance base, converted micros → whole cents ONCE at the - // Stripe boundary (a single deterministic rounding point for the total). - cents, err := centsFromMicros(arrears + advanceBase) + // new period's advance base + the closing period's account-wide pooled + // overage, converted micros → whole cents ONCE at the Stripe boundary (a + // single deterministic rounding point for the total). + cents, err := centsFromMicros(arrears + advanceBase + advanceOverage) if err != nil { return nil, billing.Internal("micros to cents conversion failed", err) } @@ -278,7 +308,7 @@ func (s *Service) RunBillingCycle(ctx context.Context, accountID uuid.UUID, peri // Charge. A failure after the PM gate marks the run 'failed' (auditable) and // returns the error. - inv, err := s.charge(ctx, runID, custID, cents, advanceBase > 0) + inv, err := s.charge(ctx, runID, custID, cents, advanceBase > 0 || advanceOverage > 0) if err != nil { if markErr := s.store.MarkBillingRun(ctx, runID, RunStatusFailed, "", 0); markErr != nil { // Both failed: surface the original charge error; the failed-mark is @@ -315,12 +345,32 @@ func (s *Service) RunBillingCycle(ctx context.Context, accountID uuid.UUID, peri PeriodStart: periodEnd, // the new period opens where the closed one ends PeriodEnd: newPeriodEnd, ModuleCount: a.ModuleCount, - BaseMicros: usage.AppBaseFeeMicros(usage.BaseFeeMicros, a.ModuleCount), + BaseMicros: usage.BaseFeeMicros, // FLAT per-app base (overage is pooled, migration 030) }); err != nil { return nil, billing.Internal("advance base snapshot insert failed", err) } } + // Freeze the account-wide pooled overage this boundary billed for the CLOSING + // period (migration 030, source='advance') so the mid-period sweep + display + // agree it is already billed — the double-charge guard. Keyed by the closing + // period_start; ON CONFLICT DO NOTHING (a mid-period 'grace' row wins if the + // race ever writes both). Skipped when nothing was billed (already billed + // mid-period, or the account was under the pool). + if advanceOverage > 0 { + if err := s.store.InsertAccountOverageSnapshot(ctx, AccountOverageSnapshot{ + AccountID: accountID, + PeriodStart: periodStart, + PeriodEnd: periodEnd, + OverCount: overCount, + ChargedMicros: advanceOverage, + Source: "advance", + InvoiceItemID: invoiceItemIdemKey(runID), // the boundary's pooled item id (the run's ii- key) + }); err != nil { + return nil, billing.Internal("account overage snapshot insert failed", err) + } + } + if err := s.store.MarkBillingRun(ctx, runID, RunStatusInvoiced, inv.ID, cents); err != nil { return nil, billing.Internal("mark billing run (invoiced) failed", err) } diff --git a/internal/account/cycle/overage.go b/internal/account/cycle/overage.go new file mode 100644 index 0000000..d1ca569 --- /dev/null +++ b/internal/account/cycle/overage.go @@ -0,0 +1,268 @@ +package cycle + +// Account-wide POOLED module overage (migration 030, owner spec 2026-07-05, +// confirmed reversal of the per-app overage tier). Overage moved from PER-APP +// to a single ACCOUNT-WIDE POOL of IncludedModules: overage = $3 × max(0, +// Σ live-app module_count − IncludedModules), charged ONCE per account per +// period. This file owns: +// +// - recomputeAccountOverage: after any module_count write (RegisterApp / +// SyncAppModules) it re-derives the pool and arms/clears the account's ONE +// grace timer (accounts.overage_since); +// - the mid-period GRACE SWEEP (ChargeAccountOverage + AccountsInOverageGrace): +// when the pool has been over for the full grace window, it charges the +// pooled overage prorated from grace-end to the period end — a DELIBERATE +// mid-period charge (the D1b "no mid-period charges" rule is now stale for +// the OVERAGE leg specifically; base-fee mid-period behavior is unchanged). +// +// The boundary advance leg's pooled-overage term lives in charge.go alongside +// the base leg. Both legs guard against double-charging the same period through +// the account_overage_snapshots ledger (keyed (account_id, period_start)) plus +// the deterministic per-(account, period) Stripe Idempotency-Keys below — the +// same money-safety pattern app_base_snapshots gives the per-app base. + +import ( + "context" + "fmt" + "strconv" + "time" + + "github.com/google/uuid" + + "github.com/mirrorstack-ai/billing-engine/internal/account/billing" + "github.com/mirrorstack-ai/billing-engine/internal/account/usage" + "github.com/mirrorstack-ai/billing-engine/internal/billingperiod" +) + +// overageGraceWindow is the ONE grace timer per account: when the pooled module +// count first crosses IncludedModules the timer starts (accounts.overage_since), +// and only after this window elapses does the mid-period sweep charge the +// overage. If the pool drops back to ≤ IncludedModules before it elapses the +// timer is cleared and nothing is charged. Owner spec 2026-07-05: 3 days. +const overageGraceWindow = 3 * 24 * time.Hour + +// OverageChargeStatus is the terminal classification of a ChargeAccountOverage +// attempt, for the cron sweep's tally + logging. +type OverageChargeStatus string + +const ( + // OverageCharged: the pooled overage was invoiced for the period. + OverageCharged OverageChargeStatus = "charged" + // OverageSkippedAlreadyCharged: this period's pooled overage already has an + // account_overage_snapshots row (a prior sweep or the boundary billed it). + OverageSkippedAlreadyCharged OverageChargeStatus = "already_charged" + // OverageSkippedUnderPool: the pool dropped back to ≤ IncludedModules by the + // time the sweep ran — nothing to charge; the timer is cleared. + OverageSkippedUnderPool OverageChargeStatus = "under_pool" + // OverageSkippedGraceHolding: the grace window has not elapsed yet (defensive + // — the work-list query already excludes these). + OverageSkippedGraceHolding OverageChargeStatus = "grace_holding" + // OverageSkippedZeroCents: the prorated overage rounded to 0 cents (grace-end + // lands at/after the period end, or a sub-cent remainder) — nothing to + // invoice this period; a later period picks it up. + OverageSkippedZeroCents OverageChargeStatus = "zero_cents" + // OverageSkippedNoPM: no usable default payment method — retained, re-attempted + // on the next sweep (the SAME per-(account, period) Stripe idem key stays + // stable), never a failure. + OverageSkippedNoPM OverageChargeStatus = "skipped_no_pm" +) + +// OverageChargeSummary reports what one ChargeAccountOverage call did. +type OverageChargeSummary struct { + Status OverageChargeStatus + PeriodStart time.Time + OverCount int + ChargedCents int64 + // StripeInvoiceID is set only when Status == OverageCharged. + StripeInvoiceID string +} + +// AccountsInOverageGrace returns the accounts whose grace timer has EXPIRED as +// of `at` (overage_since <= at − overageGraceWindow) and are chargeable — the +// mid-period sweep's work list. A thin pass-through to the store. +func (s *Service) AccountsInOverageGrace(ctx context.Context, at time.Time) ([]OverageGraceCandidate, error) { + cands, err := s.store.AccountsInOverageGrace(ctx, at.Add(-overageGraceWindow)) + if err != nil { + return nil, billing.Internal("list overage-grace accounts failed", err) + } + return cands, nil +} + +// recomputeAccountOverage re-derives the account's pooled module count after a +// module_count write and arms/clears its ONE grace timer accordingly: pool > +// IncludedModules and not yet armed → stamp overage_since (StartAccountOverage +// is first-crossing-wins); pool ≤ IncludedModules → clear it (ClearAccountOverage +// is idempotent). No refund on the clear (D1e) — it only stops FUTURE accrual; +// overage already charged this period stays billed via its snapshot row. Called +// by RegisterApp (after the initial insert) and SyncAppModules (after any count +// / delete write) so the timer always reflects the live pool. +func (s *Service) recomputeAccountOverage(ctx context.Context, accountID uuid.UUID) error { + pooled, err := s.store.PooledModuleCount(ctx, accountID) + if err != nil { + return billing.Internal("pooled module count lookup failed", err) + } + if pooled > usage.IncludedModules { + if err := s.store.StartAccountOverage(ctx, accountID, s.nowFn().UTC()); err != nil { + return billing.Internal("start account overage timer failed", err) + } + return nil + } + if err := s.store.ClearAccountOverage(ctx, accountID); err != nil { + return billing.Internal("clear account overage timer failed", err) + } + return nil +} + +// ChargeAccountOverage is the mid-period grace charge for ONE account whose +// pooled overage has survived the grace window. It: +// +// 1. resolves the account's CURRENT anchored period (from activated_at) and the +// grace-end instant (overage_since + overageGraceWindow); +// 2. skips if this period's pooled overage already has a snapshot (a prior +// sweep or the boundary billed it — the double-charge guard); +// 3. reads the CURRENT pool; if it dropped back to ≤ IncludedModules, clears +// the timer and skips (no refund of anything already charged); +// 4. prices the pooled overage PRORATED from grace-end to the period end +// (ProratedOverageMicros) → whole cents at the Stripe boundary; a 0-cent +// result skips (grace ends at/after the period end); +// 5. charges via the SAME Stripe plumbing as the other legs with the +// deterministic per-(account, period) Idempotency-Keys, mirrors the invoice, +// and freezes the account_overage_snapshots row (source='grace'). +// +// Gated on a usable default PM exactly like the spine (the candidate is already +// activated). A failure after the charge leaves no snapshot; the next sweep +// re-attempts through the SAME idem key (Stripe dedupes) — retry-safe, never a +// double charge. +func (s *Service) ChargeAccountOverage(ctx context.Context, cand OverageGraceCandidate, at time.Time) (*OverageChargeSummary, error) { + if cand.ID == uuid.Nil { + return nil, billing.InvalidInput("account_id required") + } + if s.stripe == nil { + return nil, billing.Internal("ChargeAccountOverage requires a Stripe client", nil) + } + + graceEnd := cand.OverageSince.Add(overageGraceWindow) + if at.UTC().Before(graceEnd) { + // Defensive: the work-list query already excludes still-in-grace accounts. + return &OverageChargeSummary{Status: OverageSkippedGraceHolding}, nil + } + + anchorDay := billingperiod.AnchorDay(cand.ActivatedAt) + periodStart, periodEnd := billingperiod.AnchoredPeriodWindow(at.UTC(), anchorDay) + summary := &OverageChargeSummary{PeriodStart: periodStart} + + // Double-charge guard: this period's pooled overage was already billed (by a + // prior sweep run OR the boundary that closed a prior period into this one). + if _, snapped, err := s.store.AccountOverageSnapshot(ctx, cand.ID, periodStart); err != nil { + return nil, billing.Internal("account overage snapshot lookup failed", err) + } else if snapped { + summary.Status = OverageSkippedAlreadyCharged + return summary, nil + } + + pooled, err := s.store.PooledModuleCount(ctx, cand.ID) + if err != nil { + return nil, billing.Internal("pooled module count lookup failed", err) + } + overCount := pooled - usage.IncludedModules + if overCount <= 0 { + // Dropped back under the pool since the timer was read — clear the stale + // timer and charge nothing (no refund of anything already billed, D1e). + if err := s.store.ClearAccountOverage(ctx, cand.ID); err != nil { + return nil, billing.Internal("clear account overage timer failed", err) + } + summary.Status = OverageSkippedUnderPool + return summary, nil + } + summary.OverCount = overCount + + // Prorate the pooled overage from grace-end to the period end. A 0-cent + // result (grace ends at/after this period's end) means nothing to bill this + // period — a later period picks it up; leave the timer armed, no snapshot. + proratedMicros := usage.ProratedOverageMicros(usage.AccountOverageMicros(pooled), graceEnd, periodStart, periodEnd) + cents, err := centsFromMicros(proratedMicros) + if err != nil { + return nil, billing.Internal("micros to cents conversion failed", err) + } + if cents == 0 { + summary.Status = OverageSkippedZeroCents + return summary, nil + } + summary.ChargedCents = cents + + hasPM, err := s.store.HasUsableDefaultPM(ctx, cand.ID) + if err != nil { + return nil, billing.Internal("usable PM check failed", err) + } + if !hasPM { + summary.Status = OverageSkippedNoPM + return summary, nil // retained; re-attempted next sweep through the same idem key + } + custID, err := s.store.AccountStripeCustomer(ctx, cand.ID) + if err != nil { + return nil, billing.Internal("stripe customer lookup failed", err) + } + if custID == "" { + return nil, billing.Internal("account has a usable PM but no Stripe customer id", nil) + } + + desc := fmt.Sprintf("MirrorStack module overage (account pool, %d over) — account %s", overCount, cand.ID) + item, err := s.stripe.CreateInvoiceItem(ctx, custID, cents, chargeCurrency, desc, accountOverageItemIdemKey(cand.ID, periodStart)) + if err != nil { + return nil, billing.StripeError("overage invoice item failed", err) + } + inv, err := s.stripe.CreateInvoice(ctx, custID, true /* autoAdvance */, accountOverageInvoiceIdemKey(cand.ID, periodStart)) + if err != nil { + return nil, billing.StripeError("overage invoice failed", err) + } + + if err := s.store.UpsertInvoice(ctx, InvoiceMirror{ + AccountID: cand.ID, + StripeInvoiceID: inv.ID, + Status: inv.Status, + AmountDueCents: inv.AmountDue, + AmountPaidCents: inv.AmountPaid, + Currency: chargeCurrency, + // The mirrored window is the PARTIAL coverage [grace-end day, period end), + // the SAME instant ProratedOverageMicros priced, so the shown window and + // the charged amount agree by construction. + PeriodStart: usage.ProrationCoverageStart(graceEnd, periodStart), + PeriodEnd: periodEnd, + }); err != nil { + return nil, billing.Internal("invoice mirror upsert failed", err) + } + + // Freeze the ledger row keyed by the FULL anchored period_start (the display + + // double-charge identity) — source='grace', the prorated amount actually + // invoiced. ON CONFLICT DO NOTHING makes a retry idempotent. + if err := s.store.InsertAccountOverageSnapshot(ctx, AccountOverageSnapshot{ + AccountID: cand.ID, + PeriodStart: periodStart, + PeriodEnd: periodEnd, + OverCount: overCount, + ChargedMicros: proratedMicros, + Source: "grace", + InvoiceItemID: item.ID, + }); err != nil { + return nil, billing.Internal("account overage snapshot insert failed", err) + } + + summary.Status = OverageCharged + summary.StripeInvoiceID = inv.ID + return summary, nil +} + +// accountOverageItemIdemKey / accountOverageInvoiceIdemKey build the +// deterministic per-(account, period) Stripe Idempotency-Keys for the pooled +// overage charge. The (account, period_start) pair is the stable charge +// identity — each account's pooled overage bills at most once per period (the +// account_overage_snapshots guard) — so both the mid-period grace sweep and the +// boundary leg, if they ever target the SAME period, reuse the SAME Stripe +// objects and can never double-charge. Mirrors the app-ii-/app-inv- pattern. +func accountOverageItemIdemKey(accountID uuid.UUID, periodStart time.Time) string { + return "acct-overage-ii-" + accountID.String() + "-" + strconv.FormatInt(periodStart.UTC().Unix(), 10) +} + +func accountOverageInvoiceIdemKey(accountID uuid.UUID, periodStart time.Time) string { + return "acct-overage-inv-" + accountID.String() + "-" + strconv.FormatInt(periodStart.UTC().Unix(), 10) +} diff --git a/internal/account/cycle/overage_test.go b/internal/account/cycle/overage_test.go new file mode 100644 index 0000000..3bcbd56 --- /dev/null +++ b/internal/account/cycle/overage_test.go @@ -0,0 +1,334 @@ +package cycle_test + +// Account-wide POOLED module overage (migration 030): the grace-timer recompute +// (RegisterApp / SyncAppModules), the mid-period grace sweep (ChargeAccountOverage +// + AccountsInOverageGrace), and the no-double-charge interaction with the +// boundary advance leg. Reuses the in-memory fakeStore (service_test.go) + +// fakeStripe (charge_test.go). + +import ( + "context" + "testing" + "time" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + + "github.com/mirrorstack-ai/billing-engine/internal/account/cycle" + "github.com/mirrorstack-ai/billing-engine/internal/account/usage" +) + +func overageSvc(store *fakeStore, sc *fakeStripe, now time.Time) *cycle.Service { + return cycle.NewService(store, sc).WithNow(func() time.Time { return now }) +} + +// --- recompute: the timer arms / clears on pool crossings ------------------- + +func TestRecompute_PoolCrossingFiveArmsTimer(t *testing.T) { + // Two apps' installs INTERLEAVE and SUM to the account pool: app1=3 (pool 3, + // under 5 → not armed), then app2=4 (pool 7, over 5 → arms overage_since at + // the register instant). Proves the timer keys off the ACCOUNT-WIDE sum, not + // either app alone. + now := time.Date(2026, 6, 10, 12, 0, 0, 0, time.UTC) + store := newFakeStore() + user, acct := registeredAccount(store) + svc := overageSvc(store, newFakeStripe(), now) + + _, err := svc.RegisterApp(context.Background(), cycle.RegisterAppRequest{ + OwnerUserID: user, AppID: uuid.New(), ModuleCount: 3, CreatedAt: now, + }) + require.NoError(t, err) + require.NotContains(t, store.overageSince, acct, "pool 3 is under 5 → timer not armed") + + _, err = svc.RegisterApp(context.Background(), cycle.RegisterAppRequest{ + OwnerUserID: user, AppID: uuid.New(), ModuleCount: 4, CreatedAt: now, + }) + require.NoError(t, err) + require.Contains(t, store.overageSince, acct, "pool 3+4=7 crosses 5 → timer armed") + require.Equal(t, now, store.overageSince[acct], "armed at the crossing instant") +} + +func TestRecompute_FirstCrossingWinsTimerNotMoved(t *testing.T) { + // A LATER recompute that finds the pool still over must NOT move the anchor + // (first-crossing-wins) — the grace window is measured from the FIRST cross. + first := time.Date(2026, 6, 1, 9, 0, 0, 0, time.UTC) + store := newFakeStore() + user, acct := registeredAccount(store) + appID := uuid.New() + + _, err := overageSvc(store, newFakeStripe(), first).RegisterApp(context.Background(), cycle.RegisterAppRequest{ + OwnerUserID: user, AppID: appID, ModuleCount: 6, CreatedAt: first, + }) + require.NoError(t, err) + require.Equal(t, first, store.overageSince[acct]) + + // Later: bump the count further; the pool is still over, so the anchor stays. + later := first.AddDate(0, 0, 2) + _, err = overageSvc(store, newFakeStripe(), later).SyncAppModules(context.Background(), cycle.SyncAppModulesRequest{ + AppID: appID, ModuleCount: intPtr(9), + }) + require.NoError(t, err) + require.Equal(t, first, store.overageSince[acct], "still-over recompute must not move the first-cross anchor") +} + +func TestRecompute_DroppingUnderFiveClearsTimer(t *testing.T) { + // Pool over 5 arms the timer; a later uninstall that drops the pool back to + // ≤5 CLEARS it (no charge — the grace never elapsed). + now := time.Date(2026, 6, 10, 12, 0, 0, 0, time.UTC) + store := newFakeStore() + user, acct := registeredAccount(store) + svc := overageSvc(store, newFakeStripe(), now) + appID := uuid.New() + + _, err := svc.RegisterApp(context.Background(), cycle.RegisterAppRequest{ + OwnerUserID: user, AppID: appID, ModuleCount: 7, CreatedAt: now, + }) + require.NoError(t, err) + require.Contains(t, store.overageSince, acct) + + _, err = svc.SyncAppModules(context.Background(), cycle.SyncAppModulesRequest{AppID: appID, ModuleCount: intPtr(2)}) + require.NoError(t, err) + require.NotContains(t, store.overageSince, acct, "dropping to pool 2 clears the grace timer") +} + +func TestRecompute_DeleteDroppingUnderFiveClearsTimer(t *testing.T) { + // Deleting an app (not just a count sync) also drops the pool and clears the + // timer — deleted apps leave the live pool. + now := time.Date(2026, 6, 10, 12, 0, 0, 0, time.UTC) + store := newFakeStore() + user, acct := registeredAccount(store) + svc := overageSvc(store, newFakeStripe(), now) + a1, a2 := uuid.New(), uuid.New() + + _, err := svc.RegisterApp(context.Background(), cycle.RegisterAppRequest{OwnerUserID: user, AppID: a1, ModuleCount: 4, CreatedAt: now}) + require.NoError(t, err) + _, err = svc.RegisterApp(context.Background(), cycle.RegisterAppRequest{OwnerUserID: user, AppID: a2, ModuleCount: 4, CreatedAt: now}) + require.NoError(t, err) + require.Contains(t, store.overageSince, acct, "pool 8 armed") + + _, err = svc.SyncAppModules(context.Background(), cycle.SyncAppModulesRequest{AppID: a2, Deleted: true}) + require.NoError(t, err) + require.NotContains(t, store.overageSince, acct, "deleting one app drops the pool to 4 → timer cleared") +} + +// --- grace window: holds for exactly the grace period ----------------------- + +func TestAccountsInOverageGrace_HoldsForExactlyThreeDays(t *testing.T) { + // The sweep work list includes an account only once its grace timer has + // elapsed: overage_since <= at − 3d. Pin the boundary at EXACTLY 3 days. + store := newFakeStore() + acct := uuid.New() + since := time.Date(2026, 6, 1, 8, 0, 0, 0, time.UTC) + store.overageSince[acct] = since + store.activation[acct] = time.Date(2026, 3, 1, 0, 0, 0, 0, time.UTC) + + // 1s BEFORE the 3-day mark → still in grace, excluded. + cands, err := cycle.NewService(store, newFakeStripe()).AccountsInOverageGrace(context.Background(), since.Add(3*24*time.Hour-time.Second)) + require.NoError(t, err) + require.Empty(t, cands, "grace still holding just under 3 days") + + // EXACTLY 3 days → grace elapsed, included. + cands, err = cycle.NewService(store, newFakeStripe()).AccountsInOverageGrace(context.Background(), since.Add(3*24*time.Hour)) + require.NoError(t, err) + require.Len(t, cands, 1, "grace elapsed at exactly 3 days") + require.Equal(t, acct, cands[0].ID) + require.Equal(t, since, cands[0].OverageSince) +} + +func TestAccountsInOverageGrace_ExcludesUnactivated(t *testing.T) { + // An un-activated account (no card) is never charged, so it never enters the + // sweep even past the grace window. + store := newFakeStore() + acct := uuid.New() + store.overageSince[acct] = time.Date(2026, 6, 1, 0, 0, 0, 0, time.UTC) + // no activation row + + cands, err := cycle.NewService(store, newFakeStripe()).AccountsInOverageGrace(context.Background(), time.Date(2026, 7, 1, 0, 0, 0, 0, time.UTC)) + require.NoError(t, err) + require.Empty(t, cands) +} + +// --- mid-period grace charge ------------------------------------------------ + +// armedOverageAccount seeds a fully-chargeable account (activation anchor day 1, +// usable PM, Stripe customer) with `n` apps of `perApp` modules each, created +// before `createdBefore`, and its grace timer armed at `since`. Returns the +// account id. +func armedOverageAccount(store *fakeStore, n, perApp int, since, createdBefore time.Time) uuid.UUID { + acct := uuid.New() + store.activation[acct] = time.Date(2026, 3, 1, 0, 0, 0, 0, time.UTC) // anchor day 1 → calendar-month periods + store.hasPM = true + store.stripeCustomer = "cus_overage" + store.overageSince[acct] = since + for i := 0; i < n; i++ { + seedAppCreated(store, acct, perApp, false, createdBefore.AddDate(0, 0, -1)) + } + return acct +} + +func TestChargeAccountOverage_ChargesFullWhenGraceEndedBeforePeriodStart(t *testing.T) { + // Grace ended May 31 (before the current period [Jun 1, Jul 1)), so the + // pooled overage is charged in FULL for the period. Pool = 2 apps × 4 = 8 → + // 3 over → $9 → 900 cents. One invoice item with the deterministic + // per-(account, period) idem key; one 'grace' snapshot frozen. + at := time.Date(2026, 6, 15, 12, 0, 0, 0, time.UTC) + since := time.Date(2026, 5, 28, 0, 0, 0, 0, time.UTC) // graceEnd May 31 < Jun 1 + store := newFakeStore() + jun1 := time.Date(2026, 6, 1, 0, 0, 0, 0, time.UTC) + acct := armedOverageAccount(store, 2, 4, since, jun1) + sc := newFakeStripe() + + summary, err := overageSvc(store, sc, at).ChargeAccountOverage(context.Background(), cycle.OverageGraceCandidate{ + ID: acct, OverageSince: since, ActivatedAt: store.activation[acct], + }, at) + require.NoError(t, err) + require.Equal(t, cycle.OverageCharged, summary.Status) + require.Equal(t, 3, summary.OverCount) + require.EqualValues(t, 900, summary.ChargedCents) + + require.Len(t, sc.itemCalls, 1) + require.EqualValues(t, 900, sc.itemCalls[0].amountCfg) + require.Contains(t, sc.itemCalls[0].idemKey, "acct-overage-ii-"+acct.String()) + require.Len(t, sc.invoiceCalls, 1) + + snap, ok := store.overageSnaps[acctSnapKey{acct, jun1}] + require.True(t, ok, "the grace charge must freeze a snapshot for the period") + require.Equal(t, "grace", snap.Source) + require.Equal(t, 3, snap.OverCount) + require.EqualValues(t, 9_000_000, snap.ChargedMicros) +} + +func TestChargeAccountOverage_ProratesFromGraceEndMidPeriod(t *testing.T) { + // Grace ends mid-period (Jun 4 → coverage [Jun 4, Jul 1) = 27 of 30 days). + // Pool = 6 → 1 over → $3 → prorated 3e6 × 27/30 = 2_700_000 → 270 cents. + at := time.Date(2026, 6, 20, 12, 0, 0, 0, time.UTC) + since := time.Date(2026, 6, 1, 6, 0, 0, 0, time.UTC) // graceEnd Jun 4 (truncated day) + store := newFakeStore() + acct := armedOverageAccount(store, 1, 6, since, time.Date(2026, 6, 1, 0, 0, 0, 0, time.UTC)) + sc := newFakeStripe() + + summary, err := overageSvc(store, sc, at).ChargeAccountOverage(context.Background(), cycle.OverageGraceCandidate{ + ID: acct, OverageSince: since, ActivatedAt: store.activation[acct], + }, at) + require.NoError(t, err) + require.Equal(t, cycle.OverageCharged, summary.Status) + require.EqualValues(t, 270, summary.ChargedCents) +} + +func TestChargeAccountOverage_FiresOnceAndExcludedFromBoundary(t *testing.T) { + // THE double-charge invariant. The mid-period sweep charges the pooled + // overage for [Jun 1, Jul 1); a SECOND sweep is a no-op (the snapshot guards + // it); and the BOUNDARY that closes [Jun 1, Jul 1) must NOT charge the + // overage again (it sees the snapshot) — it bills only the flat advance base. + at := time.Date(2026, 6, 15, 12, 0, 0, 0, time.UTC) + since := time.Date(2026, 5, 28, 0, 0, 0, 0, time.UTC) // full overage this period + store := newFakeStore() + store.chargedTotal = 0 + jun1 := time.Date(2026, 6, 1, 0, 0, 0, 0, time.UTC) + jul1 := time.Date(2026, 7, 1, 0, 0, 0, 0, time.UTC) + acct := armedOverageAccount(store, 2, 4, since, jun1) // pool 8 → $9 overage + sc := newFakeStripe() + svc := overageSvc(store, sc, at) + cand := cycle.OverageGraceCandidate{ID: acct, OverageSince: since, ActivatedAt: store.activation[acct]} + + // Sweep #1 charges. + first, err := svc.ChargeAccountOverage(context.Background(), cand, at) + require.NoError(t, err) + require.Equal(t, cycle.OverageCharged, first.Status) + require.Len(t, sc.invoiceCalls, 1) + + // Sweep #2 (same period) is a no-op — the snapshot guards it. + second, err := svc.ChargeAccountOverage(context.Background(), cand, at.Add(time.Hour)) + require.NoError(t, err) + require.Equal(t, cycle.OverageSkippedAlreadyCharged, second.Status) + require.Len(t, sc.invoiceCalls, 1, "no second overage invoice") + + // The BOUNDARY closing [Jun 1, Jul 1): flat advance base (2 × $20 = 40e6), + // and ZERO pooled overage — the mid-period snapshot excludes it. + resp, err := svc.RunBillingCycle(context.Background(), acct, jun1, jul1, 0) + require.NoError(t, err) + require.Equal(t, cycle.RunStatusInvoiced, resp.Status) + require.EqualValues(t, 40_000_000, resp.AdvanceBaseMicros) + require.EqualValues(t, 0, resp.AccountOverageMicros, "the mid-period overage is NOT charged again at the boundary") + require.EqualValues(t, 4_000, resp.ChargedCents) // base only + + // Exactly two invoices total: the grace overage + the boundary base. + require.Len(t, sc.invoiceCalls, 2) + // The 'grace' snapshot (not 'advance') still owns the period row. + require.Equal(t, "grace", store.overageSnaps[acctSnapKey{acct, jun1}].Source) +} + +func TestRunBillingCycle_BoundaryChargesFullPooledOverageWhenNoSweep(t *testing.T) { + // When the grace sweep never billed a period (e.g. grace expired at cutover), + // the boundary charges the FULL pooled overage for the closing period and + // writes its own 'advance' snapshot. Pool = 8 → 3 over → $9 on top of the + // flat base. + store := newFakeStore() + store.chargedTotal = 0 + store.hasPM = true + store.stripeCustomer = "cus_bnd_overage" + seedApp(store, chargeAccount, 4, false) + seedApp(store, chargeAccount, 4, false) + sc := newFakeStripe() + + resp, err := chargeSvc(store, sc).RunBillingCycle(context.Background(), chargeAccount, periodStart, periodEnd, 0) + require.NoError(t, err) + require.EqualValues(t, 40_000_000, resp.AdvanceBaseMicros) + require.EqualValues(t, 9_000_000, resp.AccountOverageMicros) + require.EqualValues(t, 4_900, resp.ChargedCents) // (40e6 + 9e6) / 10_000 + + snap, ok := store.overageSnaps[acctSnapKey{chargeAccount, periodStart}] + require.True(t, ok) + require.Equal(t, "advance", snap.Source) + require.Equal(t, 3, snap.OverCount) + require.EqualValues(t, 9_000_000, snap.ChargedMicros) +} + +func TestChargeAccountOverage_UninstallDoesNotRefund(t *testing.T) { + // After the pooled overage was charged for a period, dropping back under the + // pool clears the timer but NEVER refunds the charge already taken (D1e). A + // later sweep in the SAME period finds the snapshot and no-ops; nothing + // negative is ever invoiced. + at := time.Date(2026, 6, 15, 12, 0, 0, 0, time.UTC) + since := time.Date(2026, 5, 28, 0, 0, 0, 0, time.UTC) + store := newFakeStore() + jun1 := time.Date(2026, 6, 1, 0, 0, 0, 0, time.UTC) + acct := armedOverageAccount(store, 2, 4, since, jun1) + sc := newFakeStripe() + svc := overageSvc(store, sc, at) + cand := cycle.OverageGraceCandidate{ID: acct, OverageSince: since, ActivatedAt: store.activation[acct]} + + _, err := svc.ChargeAccountOverage(context.Background(), cand, at) + require.NoError(t, err) + require.Len(t, sc.invoiceCalls, 1) + require.EqualValues(t, 9_000_000, store.overageSnaps[acctSnapKey{acct, jun1}].ChargedMicros) + + // Now every module is uninstalled down under the pool via SyncAppModules — + // the recompute CLEARS the timer, but the already-charged snapshot (the money + // taken) is untouched: no refund (D1e). + for id, app := range store.apps { + if app.AccountID == acct { + _, err := svc.SyncAppModules(context.Background(), cycle.SyncAppModulesRequest{AppID: id, ModuleCount: intPtr(0)}) + require.NoError(t, err) + } + } + require.NotContains(t, store.overageSince, acct, "dropping under the pool clears the timer") + require.EqualValues(t, 9_000_000, store.overageSnaps[acctSnapKey{acct, jun1}].ChargedMicros, + "the charged snapshot survives — uninstall never refunds") + require.Len(t, sc.invoiceCalls, 1, "no refund / negative invoice on uninstall") + + // And a re-run of the sweep in the SAME period is a no-op (snapshot guard) — + // definitely no refund/re-charge. + again, err := svc.ChargeAccountOverage(context.Background(), cand, at.Add(time.Hour)) + require.NoError(t, err) + require.Equal(t, cycle.OverageSkippedAlreadyCharged, again.Status) + require.Len(t, sc.invoiceCalls, 1) +} + +// Guard: the overage amount is exactly the pooled tier, never a per-app tier. +func TestAccountOverageMicros_IsPooledNotPerApp(t *testing.T) { + // Two apps of 4 modules each: per-app each is UNDER the old 5 tier (would be + // $0 overage each pre-030), but POOLED they are 8 → 3 over → $9. This is the + // whole point of the reversal. + require.EqualValues(t, 9_000_000, usage.AccountOverageMicros(4+4)) +} diff --git a/internal/account/cycle/service_test.go b/internal/account/cycle/service_test.go index 2026d7f..0adea43 100644 --- a/internal/account/cycle/service_test.go +++ b/internal/account/cycle/service_test.go @@ -69,6 +69,13 @@ type fakeStore struct { activation map[uuid.UUID]time.Time baseSnapshots map[snapKey]fakeBaseSnapshot + // account-wide POOLED overage state (migration 030). overageSince models + // accounts.overage_since (present → armed at that instant); overageSnaps + // models account_overage_snapshots keyed (account, period_start) like the + // PRIMARY KEY, recording the row each charge leg froze. + overageSince map[uuid.UUID]time.Time + overageSnaps map[acctSnapKey]cycle.AccountOverageSnapshot + // captured charge writes insertedRuns map[string]uuid.UUID // (account/start/end) → run id (the idempotency gate state) runStatus map[uuid.UUID]cycle.BillingRunStatus // run id → current status (models the DB row's terminal state) @@ -106,6 +113,20 @@ type fakeStore struct { errLiveCounts error // LiveAppsCreatedBefore errProrationSnap error // UpsertProrationBaseSnapshot errAdvanceSnap error // InsertAdvanceBaseSnapshot + + errPooledCount error // PooledModuleCount + errStartOverage error // StartAccountOverage + errClearOverage error // ClearAccountOverage + errOverageGrace error // AccountsInOverageGrace + errOverageSnap error // AccountOverageSnapshot + errInsertOverageSnap error // InsertAccountOverageSnapshot +} + +// acctSnapKey mirrors the account_overage_snapshots PRIMARY KEY +// (account_id, period_start). +type acctSnapKey struct { + account uuid.UUID + periodStart time.Time } // snapKey mirrors the app_base_snapshots PRIMARY KEY (app_id, period_start). @@ -145,6 +166,8 @@ func newFakeStore() *fakeStore { accountsByUser: map[uuid.UUID]uuid.UUID{}, activation: map[uuid.UUID]time.Time{}, baseSnapshots: map[snapKey]fakeBaseSnapshot{}, + overageSince: map[uuid.UUID]time.Time{}, + overageSnaps: map[acctSnapKey]cycle.AccountOverageSnapshot{}, // Default collection state: arrears mode with a high credit limit + no // spend ceiling, so the existing charge tests (which don't set risk // fields) flow through the gate to the charge path unchanged. Risk tests @@ -480,6 +503,80 @@ func (f *fakeStore) InsertAdvanceBaseSnapshot(_ context.Context, snap cycle.AppB return nil } +// --- account-wide POOLED overage fake (migration 030) ----------------------- + +// PooledModuleCount sums module_count over the account's live apps, mirroring +// the SQL SUM(module_count) WHERE account_id = ? AND deleted_at IS NULL. +func (f *fakeStore) PooledModuleCount(_ context.Context, accountID uuid.UUID) (int, error) { + if f.errPooledCount != nil { + return 0, f.errPooledCount + } + sum := 0 + for _, app := range f.apps { + if app.AccountID == accountID && !app.Deleted { + sum += app.ModuleCount + } + } + return sum, nil +} + +func (f *fakeStore) StartAccountOverage(_ context.Context, accountID uuid.UUID, since time.Time) error { + if f.errStartOverage != nil { + return f.errStartOverage + } + if _, armed := f.overageSince[accountID]; !armed { + f.overageSince[accountID] = since // first-crossing-wins (WHERE overage_since IS NULL) + } + return nil +} + +func (f *fakeStore) ClearAccountOverage(_ context.Context, accountID uuid.UUID) error { + if f.errClearOverage != nil { + return f.errClearOverage + } + delete(f.overageSince, accountID) // idempotent (WHERE overage_since IS NOT NULL) + return nil +} + +func (f *fakeStore) AccountsInOverageGrace(_ context.Context, cutoff time.Time) ([]cycle.OverageGraceCandidate, error) { + if f.errOverageGrace != nil { + return nil, f.errOverageGrace + } + out := []cycle.OverageGraceCandidate{} + for id, since := range f.overageSince { + activatedAt, activated := f.activation[id] + if !activated { + continue // activated_at IS NOT NULL gate + } + if since.After(cutoff) { + continue // grace still holding (overage_since <= cutoff) + } + out = append(out, cycle.OverageGraceCandidate{ID: id, OverageSince: since, ActivatedAt: activatedAt}) + } + return out, nil +} + +func (f *fakeStore) AccountOverageSnapshot(_ context.Context, accountID uuid.UUID, periodStart time.Time) (cycle.AccountOverageSnapshot, bool, error) { + if f.errOverageSnap != nil { + return cycle.AccountOverageSnapshot{}, false, f.errOverageSnap + } + snap, ok := f.overageSnaps[acctSnapKey{accountID, periodStart}] + return snap, ok, nil +} + +func (f *fakeStore) InsertAccountOverageSnapshot(_ context.Context, snap cycle.AccountOverageSnapshot) error { + if f.errInsertOverageSnap != nil { + return f.errInsertOverageSnap + } + // ON CONFLICT (account_id, period_start) DO NOTHING: an existing row wins. + k := acctSnapKey{snap.AccountID, snap.PeriodStart} + if _, exists := f.overageSnaps[k]; exists { + return nil + } + f.overageSnaps[k] = snap + return nil +} + // --- helpers -------------------------------------------------------------- func requireCode(t *testing.T, err error, want billing.Code) { diff --git a/internal/account/cycle/store.go b/internal/account/cycle/store.go index 3e248cd..c8efdbd 100644 --- a/internal/account/cycle/store.go +++ b/internal/account/cycle/store.go @@ -221,6 +221,71 @@ type Store interface { // proration snapshot, or a prior reclaimed attempt's own row) wins, so a // re-run never rewrites what was already recorded as billed. InsertAdvanceBaseSnapshot(ctx context.Context, snap AppBaseSnapshot) error + + // --- account-wide POOLED module overage (migration 030) ----------------- + + // PooledModuleCount returns the account-wide pooled installed-module count: + // SUM(module_count) over the account's LIVE apps. The overage timer recompute + // and the mid-period grace sweep tier on it (overage = $3 × max(0, this − + // IncludedModules)). + PooledModuleCount(ctx context.Context, accountID uuid.UUID) (int, error) + + // StartAccountOverage stamps the account's grace-timer anchor (overage_since) + // the FIRST time its pool crosses IncludedModules — WHERE overage_since IS + // NULL, so it is first-crossing-wins/idempotent (a later recompute that finds + // it already armed is a no-op). + StartAccountOverage(ctx context.Context, accountID uuid.UUID, since time.Time) error + + // ClearAccountOverage disarms the grace timer (overage_since → NULL) when the + // pool drops back to ≤ IncludedModules — WHERE overage_since IS NOT NULL, so + // it is idempotent. No refund (D1e): clearing only stops FUTURE accrual. + ClearAccountOverage(ctx context.Context, accountID uuid.UUID) error + + // AccountsInOverageGrace returns every account whose grace timer has EXPIRED + // as of cutoff (overage_since <= cutoff) and that is chargeable (activated) — + // the mid-period grace sweep's work list, with each account's overage_since + // (grace anchor) and activated_at (period anchor). + AccountsInOverageGrace(ctx context.Context, cutoff time.Time) ([]OverageGraceCandidate, error) + + // AccountOverageSnapshot reads the frozen pooled overage a charge leg billed + // for ONE (account, period) — the double-charge guard both the grace sweep + // and the boundary consult (found=true → this period's pooled overage was + // already billed, skip it). found=false → never charged. + AccountOverageSnapshot(ctx context.Context, accountID uuid.UUID, periodStart time.Time) (snap AccountOverageSnapshot, found bool, err error) + + // InsertAccountOverageSnapshot freezes what a charge leg billed the account + // for one period's pooled overage (migration 030) with ON CONFLICT + // (account_id, period_start) DO NOTHING — an existing row (a prior grace + // charge, or a reclaimed boundary attempt's own row) wins, so a re-run never + // rewrites what was already recorded as billed. + InsertAccountOverageSnapshot(ctx context.Context, snap AccountOverageSnapshot) error +} + +// OverageGraceCandidate is one account the mid-period grace sweep evaluates: its +// id plus the two anchors the sweep needs — OverageSince (the grace timer's +// start; grace ends at OverageSince + the grace window) and ActivatedAt (the +// billing-period anchor, ADR 0005, used to resolve the current window). +type OverageGraceCandidate struct { + ID uuid.UUID + OverageSince time.Time + ActivatedAt time.Time +} + +// AccountOverageSnapshot is the in-memory form of a +// ms_billing.account_overage_snapshots row (migration 030): what one charge leg +// billed one account for one period's POOLED module overage. PeriodStart is the +// display + double-charge lookup key; ChargedMicros is the exact overage the +// invoice billed (prorated for a 'grace' row, full for an 'advance' row); +// OverCount is the pooled over-count it tiered on; Source is 'grace' or +// 'advance'; InvoiceItemID is the Stripe item id (empty for a 0-cent charge). +type AccountOverageSnapshot struct { + AccountID uuid.UUID + PeriodStart time.Time + PeriodEnd time.Time + OverCount int + ChargedMicros int64 + Source string + InvoiceItemID string } // AppModuleCount pairs one live roster app with its module_count snapshot — @@ -917,6 +982,92 @@ func (s *pgxStore) InsertAdvanceBaseSnapshot(ctx context.Context, snap AppBaseSn return err } +// --- account-wide POOLED module overage (migration 030) -------------------- + +func (s *pgxStore) PooledModuleCount(ctx context.Context, accountID uuid.UUID) (int, error) { + sum, err := s.q.SumLiveModuleCount(ctx, accountID.String()) + if err != nil { + return 0, err + } + return int(sum), nil +} + +func (s *pgxStore) StartAccountOverage(ctx context.Context, accountID uuid.UUID, since time.Time) error { + // 0 rows = already armed (first-crossing-wins, WHERE overage_since IS NULL) — + // a no-op, not an error: the original anchor survives. + _, err := s.q.StartAccountOverage(ctx, db.StartAccountOverageParams{ + ID: accountID.String(), + OverageSince: pgtype.Timestamptz{Time: since, Valid: true}, + }) + return err +} + +func (s *pgxStore) ClearAccountOverage(ctx context.Context, accountID uuid.UUID) error { + // 0 rows = already clear (WHERE overage_since IS NOT NULL) — idempotent no-op. + _, err := s.q.ClearAccountOverage(ctx, accountID.String()) + return err +} + +func (s *pgxStore) AccountsInOverageGrace(ctx context.Context, cutoff time.Time) ([]OverageGraceCandidate, error) { + rows, err := s.q.AccountsInOverageGrace(ctx, pgtype.Timestamptz{Time: cutoff, Valid: true}) + if err != nil { + return nil, err + } + out := make([]OverageGraceCandidate, 0, len(rows)) + for _, r := range rows { + id, err := uuid.Parse(r.ID) + if err != nil { + return nil, err + } + // The query filters both columns NOT NULL, so a non-Valid value here is a + // driver anomaly; skip it defensively rather than anchor on the zero time. + if !r.OverageSince.Valid || !r.ActivatedAt.Valid { + continue + } + out = append(out, OverageGraceCandidate{ + ID: id, + OverageSince: r.OverageSince.Time, + ActivatedAt: r.ActivatedAt.Time, + }) + } + return out, nil +} + +func (s *pgxStore) AccountOverageSnapshot(ctx context.Context, accountID uuid.UUID, periodStart time.Time) (AccountOverageSnapshot, bool, error) { + row, err := s.q.SelectAccountOverageSnapshot(ctx, db.SelectAccountOverageSnapshotParams{ + AccountID: accountID.String(), + PeriodStart: periodStart, + }) + if errors.Is(err, pgx.ErrNoRows) { + return AccountOverageSnapshot{}, false, nil + } + if err != nil { + return AccountOverageSnapshot{}, false, err + } + return AccountOverageSnapshot{ + AccountID: accountID, + PeriodStart: periodStart, + OverCount: int(row.OverCount), + ChargedMicros: row.ChargedMicros, + Source: row.Source, + }, true, nil +} + +func (s *pgxStore) InsertAccountOverageSnapshot(ctx context.Context, snap AccountOverageSnapshot) error { + // 0 rows = ON CONFLICT DO NOTHING kept an existing row (a prior grace charge, + // or a reclaimed boundary attempt's write) — success either way. + _, err := s.q.InsertAccountOverageSnapshot(ctx, db.InsertAccountOverageSnapshotParams{ + AccountID: snap.AccountID.String(), + PeriodStart: snap.PeriodStart, + PeriodEnd: snap.PeriodEnd, + OverCount: int32(snap.OverCount), //nolint:gosec // over_count = pooled sum − IncludedModules; the pool is Σ validated module_counts (each ≤ maxModuleCount), far below int32 max + ChargedMicros: snap.ChargedMicros, + Source: snap.Source, + InvoiceItemID: pgtype.Text{String: snap.InvoiceItemID, Valid: snap.InvoiceItemID != ""}, + }) + return err +} + // parseUUIDs parses a slice of UUID-as-string account ids (the form the sqlc // NOT NULL uuid → string override yields) into uuid.UUID. func parseUUIDs(ids []string) ([]uuid.UUID, error) { diff --git a/internal/account/cycle/types.go b/internal/account/cycle/types.go index f8d0a86..40db176 100644 --- a/internal/account/cycle/types.go +++ b/internal/account/cycle/types.go @@ -205,15 +205,24 @@ type ChargeSummary struct { // allowanceMicros) the cycle computed for the CLOSED period. ArrearsMicros int64 - // AdvanceBaseMicros is the NEW period's advance base fee (base-fee v1): - // Σ over the account's live apps of BaseFee + Overage × max(0, - // module_count − IncludedModules). 0 for a pre-backfill account (no - // mirror rows). The invoice total is ArrearsMicros + AdvanceBaseMicros; - // only when BOTH are 0 is the Stripe call skipped. + // AdvanceBaseMicros is the NEW period's advance base fee: Σ over the + // account's live apps of the FLAT BaseFeeMicros (module overage is + // account-wide pooled, migration 030 — no longer a per-app tier here). 0 for + // a pre-backfill account (no mirror rows). AdvanceBaseMicros int64 + // AccountOverageMicros is the CLOSING period's account-wide POOLED module + // overage (migration 030): $3 × max(0, Σ live-app module_count − + // IncludedModules), charged ONCE at the boundary only when the mid-period + // grace sweep did NOT already bill it (no account_overage_snapshots row for + // the period). 0 when already billed mid-period or the account is under the + // pool. The invoice total is ArrearsMicros + AdvanceBaseMicros + + // AccountOverageMicros; only when ALL are 0 is the Stripe call skipped. + AccountOverageMicros int64 + // ChargedCents is the whole-cent amount sent to Stripe (micros → cents - // round-half-up over arrears + advance base). 0 when no charge happened. + // round-half-up over arrears + advance base + pooled overage). 0 when no + // charge happened. ChargedCents int64 // StripeInvoiceID is the created Stripe invoice id, empty when no charge diff --git a/internal/account/db/models.go b/internal/account/db/models.go index d9e8e21..73b077b 100644 --- a/internal/account/db/models.go +++ b/internal/account/db/models.go @@ -331,6 +331,19 @@ type MsBillingAccount struct { SpendCeilingMicros pgtype.Int8 `json:"spend_ceiling_micros"` // UTC instant the account bound its FIRST credit card (billing-account activation). Immutable, first-bind-wins; billing-period anchor day = activated_at day-of-month (ADR 0005). NULL = never activated -> skipped by cmd/billing-cycle. ActivatedAt pgtype.Timestamptz `json:"activated_at"` + // UTC instant the account-wide pooled SUM(module_count) over live apps first crossed the included 5 (account-wide overage grace anchor, owner spec 2026-07-05). NULL = not currently over the pool. Recomputed by RegisterApp / SyncAppModules; arms one 3-day grace timer per account. + OverageSince pgtype.Timestamptz `json:"overage_since"` +} + +type MsBillingAccountOverageSnapshot struct { + AccountID string `json:"account_id"` + PeriodStart time.Time `json:"period_start"` + PeriodEnd time.Time `json:"period_end"` + OverCount int32 `json:"over_count"` + ChargedMicros int64 `json:"charged_micros"` + Source string `json:"source"` + InvoiceItemID pgtype.Text `json:"invoice_item_id"` + CreatedAt time.Time `json:"created_at"` } type MsBillingAddCardRequest struct { diff --git a/internal/account/db/overage.sql.go b/internal/account/db/overage.sql.go new file mode 100644 index 0000000..4edd8f5 --- /dev/null +++ b/internal/account/db/overage.sql.go @@ -0,0 +1,201 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.30.0 +// source: overage.sql + +package db + +import ( + "context" + "time" + + "github.com/jackc/pgx/v5/pgtype" +) + +const accountsInOverageGrace = `-- name: AccountsInOverageGrace :many +SELECT id, overage_since, activated_at +FROM ms_billing.accounts +WHERE overage_since IS NOT NULL + AND overage_since <= $1 + AND activated_at IS NOT NULL +` + +type AccountsInOverageGraceRow struct { + ID string `json:"id"` + OverageSince pgtype.Timestamptz `json:"overage_since"` + ActivatedAt pgtype.Timestamptz `json:"activated_at"` +} + +// AccountsInOverageGrace returns every account whose grace timer has EXPIRED as +// of the cutoff (overage_since <= $1 = now − 3 days) and that can be charged +// (activated_at IS NOT NULL — same activation gate as the spine). The +// mid-period sweep iterates these, derives each account's current anchored +// period from activated_at, and charges the pooled overage once per period +// (guarded by account_overage_snapshots). overage_since is returned so the +// sweep prorates from grace-end (overage_since + 3d) to the period end. +func (q *Queries) AccountsInOverageGrace(ctx context.Context, overageSince pgtype.Timestamptz) ([]AccountsInOverageGraceRow, error) { + rows, err := q.db.Query(ctx, accountsInOverageGrace, overageSince) + if err != nil { + return nil, err + } + defer rows.Close() + items := []AccountsInOverageGraceRow{} + for rows.Next() { + var i AccountsInOverageGraceRow + if err := rows.Scan(&i.ID, &i.OverageSince, &i.ActivatedAt); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const clearAccountOverage = `-- name: ClearAccountOverage :execrows +UPDATE ms_billing.accounts +SET overage_since = NULL +WHERE id = $1 + AND overage_since IS NOT NULL +` + +// ClearAccountOverage disarms the timer when the pool drops back to ≤5: +// overage_since → NULL. WHERE overage_since IS NOT NULL keeps it idempotent (a +// recompute on an already-under account affects 0 rows). No refund is issued +// (D1e) — clearing only stops FUTURE accrual; overage already charged this +// period stays billed via its account_overage_snapshots row. +func (q *Queries) ClearAccountOverage(ctx context.Context, id string) (int64, error) { + result, err := q.db.Exec(ctx, clearAccountOverage, id) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + +const insertAccountOverageSnapshot = `-- name: InsertAccountOverageSnapshot :execrows +INSERT INTO ms_billing.account_overage_snapshots + (account_id, period_start, period_end, over_count, charged_micros, source, invoice_item_id) +VALUES ($1, $2, $3, $4, $5, $6, $7) +ON CONFLICT (account_id, period_start) DO NOTHING +` + +type InsertAccountOverageSnapshotParams struct { + AccountID string `json:"account_id"` + PeriodStart time.Time `json:"period_start"` + PeriodEnd time.Time `json:"period_end"` + OverCount int32 `json:"over_count"` + ChargedMicros int64 `json:"charged_micros"` + Source string `json:"source"` + InvoiceItemID pgtype.Text `json:"invoice_item_id"` +} + +// InsertAccountOverageSnapshot records what a charge leg billed the account for +// one period's pooled overage (migration 030). ON CONFLICT (account_id, +// period_start) DO NOTHING: an existing row — a prior grace charge, or a prior +// reclaimed boundary attempt's own row — wins, so a re-run never rewrites what +// was already recorded as billed. :execrows so the caller can observe the no-op, +// though both outcomes are success (the Stripe idempotency key on the same +// (account, period) already deduped the money). +func (q *Queries) InsertAccountOverageSnapshot(ctx context.Context, arg InsertAccountOverageSnapshotParams) (int64, error) { + result, err := q.db.Exec(ctx, insertAccountOverageSnapshot, + arg.AccountID, + arg.PeriodStart, + arg.PeriodEnd, + arg.OverCount, + arg.ChargedMicros, + arg.Source, + arg.InvoiceItemID, + ) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + +const selectAccountOverageSnapshot = `-- name: SelectAccountOverageSnapshot :one +SELECT over_count, charged_micros, source +FROM ms_billing.account_overage_snapshots +WHERE account_id = $1 + AND period_start = $2 +` + +type SelectAccountOverageSnapshotParams struct { + AccountID string `json:"account_id"` + PeriodStart time.Time `json:"period_start"` +} + +type SelectAccountOverageSnapshotRow struct { + OverCount int32 `json:"over_count"` + ChargedMicros int64 `json:"charged_micros"` + Source string `json:"source"` +} + +// SelectAccountOverageSnapshot reads the frozen pooled overage a charge leg +// billed for ONE (account, period) — the double-charge guard for the charge +// side (a row means "this period's pooled overage was already billed, skip it") +// AND the authoritative display value for GetAccountBill's pooled-overage line. +// Exact period_start match (both the grace sweep and the boundary leg key on the +// anchored window start); no row → never charged → the caller skips (charge +// side) or falls back to the live pooled estimate (display side). +func (q *Queries) SelectAccountOverageSnapshot(ctx context.Context, arg SelectAccountOverageSnapshotParams) (SelectAccountOverageSnapshotRow, error) { + row := q.db.QueryRow(ctx, selectAccountOverageSnapshot, arg.AccountID, arg.PeriodStart) + var i SelectAccountOverageSnapshotRow + err := row.Scan(&i.OverCount, &i.ChargedMicros, &i.Source) + return i, err +} + +const startAccountOverage = `-- name: StartAccountOverage :execrows +UPDATE ms_billing.accounts +SET overage_since = $2 +WHERE id = $1 + AND overage_since IS NULL +` + +type StartAccountOverageParams struct { + ID string `json:"id"` + OverageSince pgtype.Timestamptz `json:"overage_since"` +} + +// StartAccountOverage arms the account's grace timer: it stamps overage_since +// the FIRST time the pool crosses 5. WHERE overage_since IS NULL makes it +// first-crossing-wins (idempotent — a later recompute that finds the pool still +// over affects 0 rows and never moves the anchor), exactly like activated_at's +// first-bind-wins. :execrows so the caller can observe (and tolerate) the +// already-armed no-op. +func (q *Queries) StartAccountOverage(ctx context.Context, arg StartAccountOverageParams) (int64, error) { + result, err := q.db.Exec(ctx, startAccountOverage, arg.ID, arg.OverageSince) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + +const sumLiveModuleCount = `-- name: SumLiveModuleCount :one + +SELECT COALESCE(SUM(module_count), 0)::bigint AS pooled_count +FROM ms_billing.apps +WHERE account_id = $1 + AND deleted_at IS NULL +` + +// Queries backing the account-wide POOLED module overage (migration 030, owner +// spec 2026-07-05). The pool is SUM(module_count) over the account's LIVE apps; +// overage = $3 × max(0, pool − 5), charged once per account per period through +// the account_overage_snapshots ledger (the double-charge guard). These queries +// serve BOTH the charge side (cycle package: the recompute + the mid-period +// grace sweep + the boundary leg) and the display side (usage package: +// GetAccountBill's pooled-overage line). Money never lives on the apps mirror; +// the pool is derived on read. +// SumLiveModuleCount returns the account-wide POOLED installed-module count: +// SUM(module_count) over the account's LIVE (deleted_at IS NULL) apps. This is +// the single source of the pool both the overage timer recompute and the charge +// legs tier on: overage applies to max(0, this − IncludedModules). COALESCE to 0 +// so an account with no live apps sums to 0 (never over). ::bigint keeps the +// aggregate a non-nullable scalar. +func (q *Queries) SumLiveModuleCount(ctx context.Context, accountID string) (int64, error) { + row := q.db.QueryRow(ctx, sumLiveModuleCount, accountID) + var pooled_count int64 + err := row.Scan(&pooled_count) + return pooled_count, err +} diff --git a/internal/account/db/queries/overage.sql b/internal/account/db/queries/overage.sql new file mode 100644 index 0000000..bd63bf1 --- /dev/null +++ b/internal/account/db/queries/overage.sql @@ -0,0 +1,83 @@ +-- Queries backing the account-wide POOLED module overage (migration 030, owner +-- spec 2026-07-05). The pool is SUM(module_count) over the account's LIVE apps; +-- overage = $3 × max(0, pool − 5), charged once per account per period through +-- the account_overage_snapshots ledger (the double-charge guard). These queries +-- serve BOTH the charge side (cycle package: the recompute + the mid-period +-- grace sweep + the boundary leg) and the display side (usage package: +-- GetAccountBill's pooled-overage line). Money never lives on the apps mirror; +-- the pool is derived on read. + +-- SumLiveModuleCount returns the account-wide POOLED installed-module count: +-- SUM(module_count) over the account's LIVE (deleted_at IS NULL) apps. This is +-- the single source of the pool both the overage timer recompute and the charge +-- legs tier on: overage applies to max(0, this − IncludedModules). COALESCE to 0 +-- so an account with no live apps sums to 0 (never over). ::bigint keeps the +-- aggregate a non-nullable scalar. +-- name: SumLiveModuleCount :one +SELECT COALESCE(SUM(module_count), 0)::bigint AS pooled_count +FROM ms_billing.apps +WHERE account_id = $1 + AND deleted_at IS NULL; + +-- StartAccountOverage arms the account's grace timer: it stamps overage_since +-- the FIRST time the pool crosses 5. WHERE overage_since IS NULL makes it +-- first-crossing-wins (idempotent — a later recompute that finds the pool still +-- over affects 0 rows and never moves the anchor), exactly like activated_at's +-- first-bind-wins. :execrows so the caller can observe (and tolerate) the +-- already-armed no-op. +-- name: StartAccountOverage :execrows +UPDATE ms_billing.accounts +SET overage_since = $2 +WHERE id = $1 + AND overage_since IS NULL; + +-- ClearAccountOverage disarms the timer when the pool drops back to ≤5: +-- overage_since → NULL. WHERE overage_since IS NOT NULL keeps it idempotent (a +-- recompute on an already-under account affects 0 rows). No refund is issued +-- (D1e) — clearing only stops FUTURE accrual; overage already charged this +-- period stays billed via its account_overage_snapshots row. +-- name: ClearAccountOverage :execrows +UPDATE ms_billing.accounts +SET overage_since = NULL +WHERE id = $1 + AND overage_since IS NOT NULL; + +-- AccountsInOverageGrace returns every account whose grace timer has EXPIRED as +-- of the cutoff (overage_since <= $1 = now − 3 days) and that can be charged +-- (activated_at IS NOT NULL — same activation gate as the spine). The +-- mid-period sweep iterates these, derives each account's current anchored +-- period from activated_at, and charges the pooled overage once per period +-- (guarded by account_overage_snapshots). overage_since is returned so the +-- sweep prorates from grace-end (overage_since + 3d) to the period end. +-- name: AccountsInOverageGrace :many +SELECT id, overage_since, activated_at +FROM ms_billing.accounts +WHERE overage_since IS NOT NULL + AND overage_since <= $1 + AND activated_at IS NOT NULL; + +-- SelectAccountOverageSnapshot reads the frozen pooled overage a charge leg +-- billed for ONE (account, period) — the double-charge guard for the charge +-- side (a row means "this period's pooled overage was already billed, skip it") +-- AND the authoritative display value for GetAccountBill's pooled-overage line. +-- Exact period_start match (both the grace sweep and the boundary leg key on the +-- anchored window start); no row → never charged → the caller skips (charge +-- side) or falls back to the live pooled estimate (display side). +-- name: SelectAccountOverageSnapshot :one +SELECT over_count, charged_micros, source +FROM ms_billing.account_overage_snapshots +WHERE account_id = $1 + AND period_start = $2; + +-- InsertAccountOverageSnapshot records what a charge leg billed the account for +-- one period's pooled overage (migration 030). ON CONFLICT (account_id, +-- period_start) DO NOTHING: an existing row — a prior grace charge, or a prior +-- reclaimed boundary attempt's own row — wins, so a re-run never rewrites what +-- was already recorded as billed. :execrows so the caller can observe the no-op, +-- though both outcomes are success (the Stripe idempotency key on the same +-- (account, period) already deduped the money). +-- name: InsertAccountOverageSnapshot :execrows +INSERT INTO ms_billing.account_overage_snapshots + (account_id, period_start, period_end, over_count, charged_micros, source, invoice_item_id) +VALUES ($1, $2, $3, $4, $5, $6, $7) +ON CONFLICT (account_id, period_start) DO NOTHING; diff --git a/internal/account/usage/accountbill.go b/internal/account/usage/accountbill.go index daebdb9..6a8667d 100644 --- a/internal/account/usage/accountbill.go +++ b/internal/account/usage/accountbill.go @@ -5,6 +5,7 @@ import ( "context" "slices" "sort" + "time" "github.com/google/uuid" @@ -66,8 +67,11 @@ const ( // 5. applies the PaaS credit ONCE at the ACCOUNT level: the same ACTIVE-SaaS // gate as the per-app credit (v1 has no subscription system → always 0), // capped at ModuleUsageTotal + InfraTotal so it never eats base fees, -// 6. TotalMicros = BaseFeeTotal + ModuleUsageTotal + InfraTotal − PaasCredit -// (≥ 0 by the cap), plus the v1 plan stub with RenewsAt = the period end. +// 6. adds the account-wide POOLED module overage (migration 030) once, +// snapshot-first (the frozen charge, else a live estimate from the pool), +// 7. TotalMicros = BaseFeeTotal + ModuleUsageTotal + InfraTotal + AccountOverage +// − PaasCredit (≥ 0 by the cap), plus the v1 plan stub with RenewsAt = the +// period end. func (s *Service) GetAccountBill(ctx context.Context, req GetAccountBillRequest) (*GetAccountBillResponse, error) { if req.OwnerUserID == uuid.Nil && req.OwnerOrgID == uuid.Nil { return nil, billing.InvalidInput("owner_user_id or owner_org_id required") @@ -168,6 +172,21 @@ func (s *Service) GetAccountBill(ctx context.Context, req GetAccountBillRequest) infraTotal += parts.InfraTotalMicros } + // Account-wide POOLED module overage (migration 030), SNAPSHOT-FIRST exactly + // like the per-app base: a charged period reads the frozen + // account_overage_snapshots row (what the grace sweep or the boundary + // actually invoiced), so the displayed overage IS what was billed even after + // later SyncAppModules pool changes. An un-charged period (pre-030 history, + // an account under the pool, or an in-progress period no leg has billed yet) + // falls back to the LIVE estimate from the CURRENT pooled sum — the full + // monthly overage the account would owe at steady state (the display doesn't + // prorate the estimate; the charge legs own proration). It is an ACCOUNT + // line, never allocated back per app. + accountOverage, err := s.accountOverageMicros(ctx, accountID, periodStart) + if err != nil { + return nil, err + } + // TODO(subscription): same gate as GetAppBill — v1 has no subscription // system, so the credit is subscription-gated OFF and resolves to 0. const subscriptionActive = false @@ -185,11 +204,32 @@ func (s *Service) GetAccountBill(ctx context.Context, req GetAccountBillRequest) BaseFeeTotalMicros: baseFeeTotal, ModuleUsageTotalMicros: moduleUsageTotal, InfraTotalMicros: infraTotal, + AccountOverageMicros: accountOverage, PaasCreditMicros: paasCredit, - TotalMicros: baseFeeTotal + moduleUsageTotal + infraTotal - paasCredit, + TotalMicros: baseFeeTotal + moduleUsageTotal + infraTotal + accountOverage - paasCredit, }, nil } +// accountOverageMicros resolves the account-wide pooled overage for the display, +// snapshot-first: the frozen account_overage_snapshots charge for the period if +// a leg billed it, else the live estimate AccountOverageMicros(current pooled +// sum). The PaaS credit never offsets it (overage rides ON TOP, like the base +// fee) so it is resolved outside the credit math. +func (s *Service) accountOverageMicros(ctx context.Context, accountID uuid.UUID, periodStart time.Time) (int64, error) { + charged, snapped, err := s.store.AccountOverageSnapshot(ctx, accountID, periodStart) + if err != nil { + return 0, billing.Internal("account overage snapshot lookup failed", err) + } + if snapped { + return charged, nil + } + pooled, err := s.store.PooledModuleCount(ctx, accountID) + if err != nil { + return 0, billing.Internal("pooled module count lookup failed", err) + } + return AccountOverageMicros(pooled), nil +} + // accountPaasCreditMicros is the ACCOUNT-level PaaS credit: the same // subscription-gated pct-of-infra magnitude as GetAppBill's per-app // paasCreditMicros, computed over the ACCOUNT-WIDE infra total and applied diff --git a/internal/account/usage/accountbill_test.go b/internal/account/usage/accountbill_test.go index 6d86162..1566946 100644 --- a/internal/account/usage/accountbill_test.go +++ b/internal/account/usage/accountbill_test.go @@ -35,9 +35,11 @@ func TestGetAccountBill_AggregatesUsageMirrorAndBothApps(t *testing.T) { // half; base = legacy flat + usage-proxy overage (1 module → flat). // appB — mirror-only, ZERO usage (just created, nothing metered): must // still appear with its full base (created long before the period). - // appC — both halves: synced count 7 → base $20 + 2×$3; usage + module- - // attributed infra. - // Every per-app number is exactly what GetAppBill would return for that app. + // appC — both halves: synced count 7 → FLAT $20 base (overage is pooled, + // migration 030); usage + module-attributed infra. + // The account-wide pool is 0 (appB) + 7 (appC) = 7 → 2 over → $6 pooled + // overage on the ACCOUNT (not on any app). Every per-app number is exactly + // what GetAppBill would return for that app. store := newFakeStore() owner := uuid.New() store.accounts[owner] = uuid.New() @@ -79,26 +81,33 @@ func TestGetAccountBill_AggregatesUsageMirrorAndBothApps(t *testing.T) { require.EqualValues(t, 200, resp.Apps[1].InfraMicros) require.Equal(t, usage.BaseFeeMicros+1200, resp.Apps[1].TotalMicros) - // appC: synced count 7 → base + 2 × overage; module-attributed infra folds in. - wantBaseC := usage.BaseFeeMicros + 2*usage.ModuleOverageFeeMicros - require.Equal(t, wantBaseC, resp.Apps[2].BaseFeeMicros) + // appC: synced count 7 → FLAT base (overage is pooled, not per-app); + // module-attributed infra folds in. + require.Equal(t, usage.BaseFeeMicros, resp.Apps[2].BaseFeeMicros) require.EqualValues(t, 500, resp.Apps[2].ModuleUsageMicros) require.EqualValues(t, 24, resp.Apps[2].InfraMicros) - require.Equal(t, wantBaseC+524, resp.Apps[2].TotalMicros) + require.Equal(t, usage.BaseFeeMicros+524, resp.Apps[2].TotalMicros) - // Account totals are the column sums; credit is gated off (v1) → total is - // the plain sum and apps[].total_micros (pre-credit) reconcile exactly. - require.Equal(t, 2*usage.BaseFeeMicros+wantBaseC, resp.BaseFeeTotalMicros) + // Account totals are the column sums plus the account-wide POOLED overage. + // All three apps are flat, so BaseFeeTotal = 3 × $20. Pool 7 → 2 over → $6. + require.Equal(t, 3*usage.BaseFeeMicros, resp.BaseFeeTotalMicros) require.EqualValues(t, 1500, resp.ModuleUsageTotalMicros) require.EqualValues(t, 224, resp.InfraTotalMicros) + require.Equal(t, usage.AccountOverageMicros(7), resp.AccountOverageMicros) + require.EqualValues(t, 6_000_000, resp.AccountOverageMicros) // 2 over × $3 require.Zero(t, resp.PaasCreditMicros) - require.Equal(t, resp.BaseFeeTotalMicros+resp.ModuleUsageTotalMicros+resp.InfraTotalMicros, resp.TotalMicros) - var preCredit int64 + require.Equal(t, + resp.BaseFeeTotalMicros+resp.ModuleUsageTotalMicros+resp.InfraTotalMicros+resp.AccountOverageMicros, + resp.TotalMicros) + // apps[].total_micros are PRE-credit AND exclude the account-level pooled + // overage (it is never allocated per-app), so Σ apps == account total − + // overage + credit. + var perApp int64 for _, a := range resp.Apps { - preCredit += a.TotalMicros + perApp += a.TotalMicros } - require.Equal(t, resp.TotalMicros+resp.PaasCreditMicros, preCredit, - "Σ apps[].total (pre-credit) == account total + credit") + require.Equal(t, resp.TotalMicros-resp.AccountOverageMicros+resp.PaasCreditMicros, perApp, + "Σ apps[].total (pre-credit) == account total − pooled overage + credit") } func TestGetAccountBill_DeterministicAppOrdering(t *testing.T) { @@ -171,10 +180,11 @@ func TestGetAccountBill_MirrorLifecycleAcrossTheWindow(t *testing.T) { // --- snapshot-first base ---------------------------------------------------- func TestGetAccountBill_SnapshotBaseFlowsThroughChargedPeriod(t *testing.T) { - // The account bill inherits GetAppBill's snapshot-first base: the May - // period was CHARGED at count 5 (flat $20 snapshot), then SyncAppModules - // moved the mirror to 8. The frozen period must show the snapshot base; - // the CURRENT (un-snapshotted) period estimates from the live count 8. + // The account bill inherits GetAppBill's snapshot-first PER-APP base: the May + // period was CHARGED at a flat $20 snapshot, then SyncAppModules moved the + // mirror to 8. The per-app base is flat in both periods (overage is pooled). + // The CURRENT (un-snapshotted) period's account-wide overage estimates from + // the live pool 8 → 3 over → $9. store := newFakeStore() owner := uuid.New() store.accounts[owner] = uuid.New() @@ -182,7 +192,7 @@ func TestGetAccountBill_SnapshotBaseFlowsThroughChargedPeriod(t *testing.T) { app := seqUUID(1) store.appMirrors[app] = usage.AppMirrorInfo{ModuleCount: 8, CreatedAt: time.Date(2026, 1, 10, 0, 0, 0, 0, time.UTC)} store.baseSnapshots[baseSnapKey(app, time.Date(2026, 5, 1, 0, 0, 0, 0, time.UTC))] = usage.AppBaseSnapshotInfo{ - BaseMicros: usage.AppBaseFeeMicros(usage.BaseFeeMicros, 5), + BaseMicros: usage.BaseFeeMicros, } resp, err := newService(store).GetAccountBill(context.Background(), usage.GetAccountBillRequest{ @@ -190,13 +200,14 @@ func TestGetAccountBill_SnapshotBaseFlowsThroughChargedPeriod(t *testing.T) { }) require.NoError(t, err) require.Len(t, resp.Apps, 1) - require.Equal(t, usage.BaseFeeMicros, resp.Apps[0].BaseFeeMicros, "charged period shows the invoiced snapshot, not the resynced count") + require.Equal(t, usage.BaseFeeMicros, resp.Apps[0].BaseFeeMicros, "charged period shows the invoiced flat snapshot") resp, err = newService(store).GetAccountBill(context.Background(), usage.GetAccountBillRequest{OwnerUserID: owner}) require.NoError(t, err) require.Len(t, resp.Apps, 1) - require.Equal(t, usage.BaseFeeMicros+3*usage.ModuleOverageFeeMicros, resp.Apps[0].BaseFeeMicros, - "un-snapshotted current period estimates from the synced count 8") + require.Equal(t, usage.BaseFeeMicros, resp.Apps[0].BaseFeeMicros, "per-app base is flat regardless of count") + require.EqualValues(t, 9_000_000, resp.AccountOverageMicros, + "un-snapshotted current period estimates the pooled overage from the live pool 8 → 3 over → $9") } // --- period resolution -------------------------------------------------------- diff --git a/internal/account/usage/basefee.go b/internal/account/usage/basefee.go index da47975..718a0e7 100644 --- a/internal/account/usage/basefee.go +++ b/internal/account/usage/basefee.go @@ -2,29 +2,50 @@ package usage import "time" -// This file is the SINGLE home of the base-fee math (owner spec 2026-07-05, -// DESIGN.md "Base fee — v1 spec"). Both consumers — the display read -// (GetAppBill, this package) and the charge spine (cycle: the RegisterApp -// creation-proration charge + the boundary advance leg) — compute an app's -// per-period base through these two functions, so the bill page, the invoice, -// and the mirror can never disagree by construction. All money is integer -// micro-dollars; the arithmetic here is pure int64 (no big.Rat needed: the -// operands are bounded — see ProratedBaseMicros). +// This file is the SINGLE home of the base-fee + pooled-overage math (owner +// spec 2026-07-05, DESIGN.md "Base fee — v1 spec"; account-wide overage +// reversal, migration 030). Both consumers — the display read (GetAppBill / +// GetAccountBill, this package) and the charge spine (cycle: the RegisterApp +// creation-proration charge, the boundary advance leg, the mid-period grace +// sweep) — compute the per-app FLAT base and the account-wide pooled overage +// through these functions, so the bill page, the invoice, and the mirror can +// never disagree by construction. All money is integer micro-dollars; the +// arithmetic here is pure int64 (no big.Rat needed: the operands are bounded — +// see ProratedBaseMicros). +// +// Overage moved from PER-APP to ACCOUNT-WIDE POOLED (migration 030): the flat +// $20/app base is per-app and unchanged, but the $3/module surcharge now +// applies ONCE per account to max(0, Σ live-app module_count − IncludedModules) +// — see AccountOverageMicros. There is deliberately NO per-app overage helper +// anymore: an app's base is just the flat (plan-resolved) fee. -// AppBaseFeeMicros is an app's FULL per-period base fee: +// AccountOverageMicros is the account-wide POOLED module overage for one +// period: // -// base + ModuleOverageFeeMicros × max(0, moduleCount − IncludedModules) +// ModuleOverageFeeMicros × max(0, pooledModuleCount − IncludedModules) // -// baseFeeMicros is the plan-resolved flat fee (resolveBaseFeeMicros; the -// charge spine passes BaseFeeMicros until a plan resolver exists) so the -// plan seam stays in ONE place and this function stays pure tier math. -// A negative moduleCount cannot occur (DB CHECK module_count >= 0); the -// max(0, …) clamp still makes the function total. -func AppBaseFeeMicros(baseFeeMicros int64, moduleCount int) int64 { - if extra := moduleCount - IncludedModules; extra > 0 { - return baseFeeMicros + ModuleOverageFeeMicros*int64(extra) +// pooledModuleCount is SUM(module_count) over the account's LIVE apps (one pool +// of IncludedModules for the WHOLE account, not per app). A pooledModuleCount +// ≤ IncludedModules yields 0 (the max(0, …) clamp makes the function total; +// a negative count cannot occur — the sum of non-negative DB-CHECKed counts). +func AccountOverageMicros(pooledModuleCount int) int64 { + if extra := pooledModuleCount - IncludedModules; extra > 0 { + return ModuleOverageFeeMicros * int64(extra) } - return baseFeeMicros + return 0 +} + +// ProratedOverageMicros prorates an account-wide pooled overage amount for the +// period [periodStart, periodEnd), covering [grace-end day, periodEnd): the +// SAME day-count round-half-up math as ProratedBaseMicros (the overage amount +// is prorated exactly like a base amount), but ANCHORED on the grace-end +// instant instead of an app's creation instant. graceEnd on/before periodStart +// → the FULL overage (the account was over for the whole period); graceEnd +// on/after periodEnd → 0 (grace ends after this period — nothing to charge +// yet). Kept as a named wrapper so the semantic ("prorate FROM grace-end") is +// legible at the mid-period grace sweep's call site. +func ProratedOverageMicros(overageMicros int64, graceEnd, periodStart, periodEnd time.Time) int64 { + return ProratedBaseMicros(overageMicros, graceEnd, periodStart, periodEnd) } // ProratedBaseMicros prorates an app's per-period base fee for the period diff --git a/internal/account/usage/basefee_test.go b/internal/account/usage/basefee_test.go index 69db875..59c721f 100644 --- a/internal/account/usage/basefee_test.go +++ b/internal/account/usage/basefee_test.go @@ -13,24 +13,27 @@ func day(y int, m time.Month, d int) time.Time { return time.Date(y, m, d, 0, 0, 0, 0, time.UTC) } -// --- AppBaseFeeMicros: overage tier boundaries ------------------------------ +// --- AccountOverageMicros: account-wide POOLED overage tier boundaries ------- -func TestAppBaseFeeMicros_OverageBoundaries(t *testing.T) { - // Owner spec 2026-07-05: base + $3 × max(0, count − 5). The boundary cases - // pin the tier edges: 5 included modules cost nothing extra, the 6th costs - // exactly one overage, 0 modules never discount below the base. +func TestAccountOverageMicros_PooledBoundaries(t *testing.T) { + // Owner spec 2026-07-05 / migration 030: $3 × max(0, pooledCount − 5), where + // pooledCount is Σ live-app module_count for the WHOLE account. The boundary + // cases pin the tier edges: ≤5 pooled modules cost nothing, the 6th costs + // exactly one overage, and the overage is NOT the flat base (it is the + // account-level surcharge only). for _, tc := range []struct { - name string - count int - want int64 + name string + pooled int + want int64 }{ - {"zero modules → flat base (no negative overage)", 0, usage.BaseFeeMicros}, - {"exactly included (5) → flat base", usage.IncludedModules, usage.BaseFeeMicros}, - {"included+1 (6) → one $3 overage", usage.IncludedModules + 1, usage.BaseFeeMicros + usage.ModuleOverageFeeMicros}, - {"included+10 → ten overages", usage.IncludedModules + 10, usage.BaseFeeMicros + 10*usage.ModuleOverageFeeMicros}, + {"zero pooled → no overage", 0, 0}, + {"under pool (3) → no overage", 3, 0}, + {"exactly included (5) → no overage", usage.IncludedModules, 0}, + {"included+1 (6) → one $3 overage", usage.IncludedModules + 1, usage.ModuleOverageFeeMicros}, + {"included+10 → ten overages", usage.IncludedModules + 10, 10 * usage.ModuleOverageFeeMicros}, } { t.Run(tc.name, func(t *testing.T) { - require.Equal(t, tc.want, usage.AppBaseFeeMicros(usage.BaseFeeMicros, tc.count)) + require.Equal(t, tc.want, usage.AccountOverageMicros(tc.pooled)) }) } } @@ -124,10 +127,20 @@ func TestProratedBaseMicros_ClampedAnchorMonth(t *testing.T) { require.EqualValues(t, 19_354_839, got) } -func TestProratedBaseMicros_ProratesTheOverageToo(t *testing.T) { - // The prorated amount is the FULL app base (base + overage), not the flat - // fee: 7 modules → 26e6; half of a 30-day period (15 days) → 13e6. - appBase := usage.AppBaseFeeMicros(usage.BaseFeeMicros, 7) - got := usage.ProratedBaseMicros(appBase, day(2026, 6, 19), day(2026, 6, 4), day(2026, 7, 4)) - require.EqualValues(t, 13_000_000, got) +func TestProratedOverageMicros_ProratesPooledOverageFromGraceEnd(t *testing.T) { + // Migration 030: the mid-period sweep prorates the account-wide POOLED + // overage from grace-end to the period end with the SAME day-count math as + // ProratedBaseMicros. Pool of 7 → 2 over → $6/period; grace ends mid-period + // (Jun 19 → 15 of a 30-day [Jun 4, Jul 4) period) → half → $3. + overage := usage.AccountOverageMicros(7) + require.EqualValues(t, 6_000_000, overage) + got := usage.ProratedOverageMicros(overage, day(2026, 6, 19), day(2026, 6, 4), day(2026, 7, 4)) + require.EqualValues(t, 3_000_000, got) + + // grace-end on/before the period start → the FULL pooled overage (over the + // whole period). + require.EqualValues(t, overage, usage.ProratedOverageMicros(overage, day(2026, 6, 4), day(2026, 6, 4), day(2026, 7, 4))) + + // grace-end on/after the period end → 0 (grace ends after this period). + require.EqualValues(t, 0, usage.ProratedOverageMicros(overage, day(2026, 7, 4), day(2026, 6, 4), day(2026, 7, 4))) } diff --git a/internal/account/usage/bill.go b/internal/account/usage/bill.go index d33d262..947c20e 100644 --- a/internal/account/usage/bill.go +++ b/internal/account/usage/bill.go @@ -32,9 +32,10 @@ import ( const ( // BaseFeeMicros is 基本費用 — the fixed per-app/period platform base fee on the - // DEFAULT plan. It BUNDLES the PaaS infra credit (surfaced as PaasCreditMicros) - // and INCLUDES up to IncludedModules installed modules; each installed module - // beyond that adds ModuleOverageFeeMicros. Tunable. Default $20. + // DEFAULT plan. It BUNDLES the PaaS infra credit (surfaced as PaasCreditMicros). + // It is FLAT per app: the IncludedModules allowance + the ModuleOverageFeeMicros + // surcharge are ACCOUNT-WIDE POOLED (migration 030 — see AccountOverageMicros), + // NOT folded into this per-app fee. Tunable. Default $20. BaseFeeMicros int64 = 20_000_000 // $20.00 // ProBaseFeeMicros is the base fee on the Pro org plan. TODO(plan): wire a real @@ -43,16 +44,17 @@ const ( // value — tune with the real Pro plan. ProBaseFeeMicros int64 = 50_000_000 // $50.00 (placeholder) - // IncludedModules is how many installed modules the base fee bundles before the - // per-module surcharge kicks in. Owner spec 2026-07-05 (base-fee v1, DESIGN.md - // D1): 5 included. Tunable ("may change"); becomes plan-resolved later. + // IncludedModules is the ACCOUNT-WIDE POOL of installed modules the base fee + // bundles before the per-module surcharge kicks in (migration 030 — ONE pool + // of 5 for the whole account, summed over its live apps, NOT 5 per app). Owner + // spec 2026-07-05. Tunable ("may change"); becomes plan-resolved later. IncludedModules = 5 - // ModuleOverageFeeMicros is the surcharge added to the base fee for EACH - // installed module beyond IncludedModules. Owner spec 2026-07-05 (base-fee v1): - // $3.00/module/period. Tunable; becomes plan-resolved later. App base for a - // period = BaseFee + Overage × max(0, module_count − IncludedModules) — see - // AppBaseFeeMicros, the ONE place that formula lives. + // ModuleOverageFeeMicros is the surcharge for EACH installed module beyond the + // account-wide pooled IncludedModules. Owner spec 2026-07-05: $3.00/module/ + // period. Account overage for a period = Overage × max(0, Σ live-app + // module_count − IncludedModules) — see AccountOverageMicros, the ONE place + // that formula lives. Tunable; becomes plan-resolved later. ModuleOverageFeeMicros int64 = 3_000_000 // $3.00 // PaasCreditPct is PaaS 額度 — the percentage of the 基礎設施 InfraTotal credited @@ -133,11 +135,12 @@ func pctMicros(base int64, pct int) (int64, error) { // match), so the displayed base IS what the invoice charged even after // later SyncAppModules count changes. Only an un-snapshotted period // (pre-feature history, unactivated account, in-progress period) falls -// back to the live ESTIMATE from the mirror (migration 027): -// AppBaseFeeMicros(resolveBaseFeeMicros(plan), module_count), PRORATED via +// back to the live ESTIMATE from the mirror (migration 027): the FLAT +// plan-resolved base (resolveBaseFeeMicros(plan)), PRORATED via // ProratedBaseMicros when the app's created_at falls inside the period. -// An app ABSENT from the mirror (pre-backfill) falls back to the usage-proxy -// count below — today's behavior, until the api-platform backfill lands, +// Module overage is NOT in the per-app base anymore — it is account-wide +// pooled (migration 030, surfaced on GetAccountBill). An app ABSENT from +// the mirror (pre-backfill) falls back to the flat plan base below, // 5. computes PaaS 額度 credit = PaasCreditPct% of the infra total, but ONLY when // an active SaaS subscription earns it — v1 has no subscription system, so the // credit is subscription-gated OFF and is 0 (the wire field stays for back-compat), @@ -293,10 +296,10 @@ func (s *Service) computeAppBill(ctx context.Context, accountID uuid.UUID, found // Keep only the non-reserved rows → module usage (displayed lines); the // reserved infra.* / platform.* rows are dropped here (infra is sourced - // per-metric from the catalog below). Count DISTINCT non-reserved modules as - // the installed-module proxy for the base-fee tier. + // per-metric from the catalog below). Module overage is now account-wide + // pooled (migration 030), so the per-app base no longer needs a + // distinct-module proxy count here. moduleUsage := make([]AppMetricUsage, 0, len(lines)) - installedModules := make(map[uuid.UUID]struct{}) var moduleUsageTotal int64 for _, r := range lines { if isReservedMetric(r.Metric) { @@ -317,7 +320,6 @@ func (s *Service) computeAppBill(ctx context.Context, accountID uuid.UUID, found ChargedMicros: r.ChargedMicros, }) moduleUsageTotal += r.ChargedMicros - installedModules[r.ModuleID] = struct{}{} } // 基礎設施: source infra per-metric from the CATALOG (metric_definitions), NOT @@ -394,14 +396,16 @@ func (s *Service) computeAppBill(ctx context.Context, accountID uuid.UUID, found // period leaves that period's base spent, so it falls through below. baseFee = 0 case mirrored: - // No snapshot → estimate from the mirror's current count, the SAME - // AppBaseFeeMicros + ProratedBaseMicros math the charge legs bill. - baseFee = ProratedBaseMicros( - AppBaseFeeMicros(resolveBaseFeeMicros(plan), mirror.ModuleCount), - mirror.CreatedAt, periodStart, periodEnd, - ) + // No snapshot → estimate the FLAT per-app base from the plan (module + // overage is now account-wide pooled, migration 030 — never folded + // into the per-app base here), the SAME ProratedBaseMicros math the + // charge legs bill, prorated when created_at falls inside the period. + baseFee = ProratedBaseMicros(resolveBaseFeeMicros(plan), mirror.CreatedAt, periodStart, periodEnd) default: - baseFee = AppBaseFeeMicros(resolveBaseFeeMicros(plan), len(installedModules)) + // No mirror row (pre-backfill): the flat per-app base. The pre-030 + // usage-proxy overage is gone — overage is pooled at the account + // level, so a per-app read never estimates it. + baseFee = resolveBaseFeeMicros(plan) } } diff --git a/internal/account/usage/bill_test.go b/internal/account/usage/bill_test.go index 392ac80..1fb239a 100644 --- a/internal/account/usage/bill_test.go +++ b/internal/account/usage/bill_test.go @@ -59,8 +59,11 @@ func TestGetAppBill_BaseFeeBundlesUpToIncludedModules(t *testing.T) { require.Equal(t, usage.BaseFeeMicros, resp.BaseFeeMicros, "5 modules is within the bundle → no surcharge") } -func TestGetAppBill_BaseFeeSurchargesModulesBeyondIncluded(t *testing.T) { - // 7 distinct modules → base = BaseFeeMicros + ModuleOverageFeeMicros × (7 − 5). +func TestGetAppBill_PerAppBaseIsFlatEvenWithManyModules(t *testing.T) { + // Migration 030: module overage is ACCOUNT-WIDE POOLED, so the PER-APP base + // is the FLAT $20 regardless of how many modules the app uses — 7 distinct + // modules no longer surcharge THIS app's base (the pooled overage surfaces on + // GetAccountBill instead). store := newFakeStore() owner := uuid.New() store.accounts[owner] = uuid.New() @@ -70,8 +73,7 @@ func TestGetAppBill_BaseFeeSurchargesModulesBeyondIncluded(t *testing.T) { resp, err := newService(store).GetAppBill(context.Background(), usage.GetAppBillRequest{OwnerUserID: owner, AppID: uuid.New()}) require.NoError(t, err) - want := usage.BaseFeeMicros + usage.ModuleOverageFeeMicros*2 - require.Equal(t, want, resp.BaseFeeMicros) + require.Equal(t, usage.BaseFeeMicros, resp.BaseFeeMicros, "per-app base is flat; overage is pooled at the account") } func TestGetAppBill_ModuleCountIsDistinctModulesNotLines(t *testing.T) { @@ -518,10 +520,10 @@ func mirrorPeriod(store *fakeStore) uuid.UUID { return pid } -func TestGetAppBill_MirroredAppUsesSyncedModuleCount(t *testing.T) { - // A mirrored app's overage comes from the SYNCED module_count (7 → +2×$3), - // NOT the usage-proxy distinct-module count (only 1 module has usage here - // — the proxy would have said "flat base"). +func TestGetAppBill_MirroredAppShowsFlatBase(t *testing.T) { + // Migration 030: a mirrored app's per-app base is the FLAT $20 regardless of + // its synced module_count (7 here) — module overage is account-wide pooled, + // no longer part of the per-app base. store := newFakeStore() owner := uuid.New() store.accounts[owner] = uuid.New() @@ -535,7 +537,7 @@ func TestGetAppBill_MirroredAppUsesSyncedModuleCount(t *testing.T) { resp, err := newService(store).GetAppBill(context.Background(), usage.GetAppBillRequest{OwnerUserID: owner, AppID: app, PeriodID: pid}) require.NoError(t, err) - require.Equal(t, usage.BaseFeeMicros+2*usage.ModuleOverageFeeMicros, resp.BaseFeeMicros) + require.Equal(t, usage.BaseFeeMicros, resp.BaseFeeMicros, "per-app base is flat (overage is pooled)") } func TestGetAppBill_MirroredAppProratesCreationPeriod(t *testing.T) { @@ -602,9 +604,10 @@ func TestGetAppBill_AppDeletedDuringPeriodKeepsSpentBase(t *testing.T) { require.Equal(t, usage.BaseFeeMicros, resp.BaseFeeMicros) } -func TestGetAppBill_UnmirroredAppKeepsProxyFallback(t *testing.T) { - // No mirror row (pre-backfill) → today's behavior survives verbatim: flat - // base + the usage-proxy overage (7 distinct modules with usage → +2×$3). +func TestGetAppBill_UnmirroredAppShowsFlatBase(t *testing.T) { + // No mirror row (pre-backfill) → the FLAT per-app base. Migration 030 retired + // the pre-030 usage-proxy overage: overage is account-wide pooled, never + // estimated per app, so even 7 distinct modules with usage show flat $20 here. store := newFakeStore() owner := uuid.New() store.accounts[owner] = uuid.New() @@ -615,7 +618,7 @@ func TestGetAppBill_UnmirroredAppKeepsProxyFallback(t *testing.T) { resp, err := newService(store).GetAppBill(context.Background(), usage.GetAppBillRequest{OwnerUserID: owner, AppID: uuid.New(), PeriodID: pid}) require.NoError(t, err) - require.Equal(t, usage.BaseFeeMicros+2*usage.ModuleOverageFeeMicros, resp.BaseFeeMicros) + require.Equal(t, usage.BaseFeeMicros, resp.BaseFeeMicros) } func TestGetAppBill_InternalOnAppMirrorError(t *testing.T) { @@ -645,21 +648,22 @@ func TestGetAppBill_SnapshotFreezesChargedPeriodBaseAfterSync(t *testing.T) { CreatedAt: time.Date(2026, 1, 10, 0, 0, 0, 0, time.UTC), } store.baseSnapshots[baseSnapKey(app, time.Date(2026, 5, 1, 0, 0, 0, 0, time.UTC))] = usage.AppBaseSnapshotInfo{ - BaseMicros: usage.AppBaseFeeMicros(usage.BaseFeeMicros, 5), // what the boundary actually invoiced + BaseMicros: usage.BaseFeeMicros, // what the boundary actually invoiced (flat base) } - // The charged period displays the frozen count-5 base, NOT a recompute - // from the mirror's current count 8. + // The charged period displays the frozen snapshot base, NOT a recompute from + // the mirror's current count 8. resp, err := newService(store).GetAppBill(context.Background(), usage.GetAppBillRequest{OwnerUserID: owner, AppID: app, PeriodID: pid}) require.NoError(t, err) - require.Equal(t, usage.BaseFeeMicros, resp.BaseFeeMicros, "charged period shows what was invoiced (count 5 → flat base)") + require.Equal(t, usage.BaseFeeMicros, resp.BaseFeeMicros, "charged period shows what was invoiced (flat base)") - // The CURRENT period has no snapshot yet → live ESTIMATE from the synced - // count 8. + // The CURRENT period has no snapshot yet → live ESTIMATE, which is the FLAT + // per-app base (migration 030 — overage is pooled, not per-app), unaffected + // by the synced count 8. resp, err = newService(store).GetAppBill(context.Background(), usage.GetAppBillRequest{OwnerUserID: owner, AppID: app}) require.NoError(t, err) - require.Equal(t, usage.BaseFeeMicros+3*usage.ModuleOverageFeeMicros, resp.BaseFeeMicros, - "un-snapshotted period estimates from the current count 8") + require.Equal(t, usage.BaseFeeMicros, resp.BaseFeeMicros, + "un-snapshotted period estimates the flat per-app base regardless of count") } func TestGetAppBill_ProrationSnapshotFreezesCreationPeriodBase(t *testing.T) { diff --git a/internal/account/usage/service_test.go b/internal/account/usage/service_test.go index cc1b9de..147d7a1 100644 --- a/internal/account/usage/service_test.go +++ b/internal/account/usage/service_test.go @@ -40,6 +40,13 @@ type fakeStore struct { appMirrors map[uuid.UUID]usage.AppMirrorInfo // app_id → ms_billing.apps roster row (migration 027) baseSnapshots map[string]usage.AppBaseSnapshotInfo // app_id/period_start → charged-base snapshot (migration 028) + // account-wide POOLED overage (migration 030): PooledModuleCount sums the + // appMirrors' module_count (like the SQL over ms_billing.apps); overageSnaps + // keys account_id/period_start → the frozen pooled-overage charged_micros. + overageSnaps map[string]int64 + errPooledCount error + errOverageSnap error + // usageAppIDs is what AppIDsWithUsage enumerates (the usage half of // GetAccountBill's roster); the mirror half is DERIVED from appMirrors with // the real overlap rule (see MirroredAppIDs). @@ -121,6 +128,7 @@ func newFakeStore() *fakeStore { visibility: map[uuid.UUID]usage.Visibility{}, appMirrors: map[uuid.UUID]usage.AppMirrorInfo{}, baseSnapshots: map[string]usage.AppBaseSnapshotInfo{}, + overageSnaps: map[string]int64{}, appBillRowsByApp: map[uuid.UUID][]usage.AppMetricUsageRaw{}, appInfraBillRowsByApp: map[uuid.UUID][]usage.AppInfraUsage{}, appModuleInfraBillRowsByApp: map[uuid.UUID][]usage.AppModuleInfraUsage{}, @@ -183,6 +191,40 @@ func (f *fakeStore) AppBaseSnapshot(_ context.Context, appID uuid.UUID, periodSt return s, ok, nil } +// overageSnapKey mirrors the account_overage_snapshots PRIMARY KEY +// (account_id, period_start). +func overageSnapKey(accountID uuid.UUID, periodStart time.Time) string { + return accountID.String() + "/" + periodStart.UTC().Format(time.RFC3339Nano) +} + +// PooledModuleCount returns the account's live pooled module count (migration +// 030): Σ module_count over the non-deleted appMirrors, mirroring the SQL over +// ms_billing.apps. The single-account usage fake holds one account's roster, so +// it sums every live mirror row. +func (f *fakeStore) PooledModuleCount(_ context.Context, _ uuid.UUID) (int, error) { + if f.errPooledCount != nil { + return 0, f.errPooledCount + } + sum := 0 + for _, m := range f.appMirrors { + if !m.Deleted { + sum += m.ModuleCount + } + } + return sum, nil +} + +// AccountOverageSnapshot returns the fake migration-030 pooled-overage charge +// for one (account, period_start); absent → never charged, so GetAccountBill +// falls back to the live pooled estimate. +func (f *fakeStore) AccountOverageSnapshot(_ context.Context, accountID uuid.UUID, periodStart time.Time) (int64, bool, error) { + if f.errOverageSnap != nil { + return 0, false, f.errOverageSnap + } + m, ok := f.overageSnaps[overageSnapKey(accountID, periodStart)] + return m, ok, nil +} + // AccountAnchorDay returns the configured anchor day for an account, defaulting // to 1 (the UTC calendar month) so tests that don't set one keep the pre-anchor // window behavior. diff --git a/internal/account/usage/store.go b/internal/account/usage/store.go index 3a737d3..54d983f 100644 --- a/internal/account/usage/store.go +++ b/internal/account/usage/store.go @@ -208,6 +208,21 @@ type Store interface { // deleted BEFORE the period opened is excluded (base 0, no new usage — // residual ledger rows still enumerate through AppIDsWithUsage). MirroredAppIDs(ctx context.Context, accountID uuid.UUID, periodStart, periodEnd time.Time) ([]uuid.UUID, error) + + // PooledModuleCount returns the account-wide POOLED installed-module count: + // SUM(module_count) over the account's LIVE apps (migration 030). It is the + // live input to GetAccountBill's account-wide overage ESTIMATE when the + // current period has no frozen account_overage_snapshots row yet. + PooledModuleCount(ctx context.Context, accountID uuid.UUID) (int, error) + + // AccountOverageSnapshot reads the frozen pooled overage a charge leg billed + // for ONE (account, period) — the AUTHORITATIVE display value for + // GetAccountBill's account-wide overage line (what the grace sweep or the + // boundary actually invoiced). found=false → the period's pooled overage was + // never charged (pre-030 history, an account under the pool, or an + // in-progress period no leg has billed yet) and the caller falls back to the + // live pooled ESTIMATE. + AccountOverageSnapshot(ctx context.Context, accountID uuid.UUID, periodStart time.Time) (chargedMicros int64, found bool, err error) } // AppBaseSnapshotInfo is the display-read projection of a @@ -605,6 +620,33 @@ func (s *pgxStore) MirroredAppIDs(ctx context.Context, accountID uuid.UUID, peri return parseAppIDs(rows) } +// PooledModuleCount sums module_count over the account's live apps (migration +// 030) — the live input to GetAccountBill's account-wide overage estimate. +func (s *pgxStore) PooledModuleCount(ctx context.Context, accountID uuid.UUID) (int, error) { + sum, err := s.q.SumLiveModuleCount(ctx, accountID.String()) + if err != nil { + return 0, err + } + return int(sum), nil +} + +// AccountOverageSnapshot reads the frozen pooled overage charged for one +// (account, period) — pgx.ErrNoRows → found=false (the service falls back to +// the live pooled estimate). +func (s *pgxStore) AccountOverageSnapshot(ctx context.Context, accountID uuid.UUID, periodStart time.Time) (int64, bool, error) { + row, err := s.q.SelectAccountOverageSnapshot(ctx, db.SelectAccountOverageSnapshotParams{ + AccountID: accountID.String(), + PeriodStart: periodStart, + }) + if errors.Is(err, pgx.ErrNoRows) { + return 0, false, nil // never overage-charged → live estimate + } + if err != nil { + return 0, false, err + } + return row.ChargedMicros, true, nil +} + // parseAppIDs decodes a generated query's text app_id column into uuid.UUIDs, // shared by the two GetAccountBill enumeration reads. func parseAppIDs(rows []string) ([]uuid.UUID, error) { diff --git a/internal/account/usage/types.go b/internal/account/usage/types.go index 5b0f1f6..36c3946 100644 --- a/internal/account/usage/types.go +++ b/internal/account/usage/types.go @@ -392,9 +392,10 @@ type GetAppBillResponse struct { PeriodStart time.Time `json:"period_start"` PeriodEnd time.Time `json:"period_end"` - // BaseFeeMicros is 基本費用 — the fixed per-app/period platform fee, PLUS the - // per-module surcharge for each installed module beyond IncludedModules (see - // the bill.go consts). Bundles the PaaS infra credit surfaced below. + // BaseFeeMicros is 基本費用 — the FLAT fixed per-app/period platform fee (see + // the bill.go consts). Module overage is NO LONGER folded in here: it is + // account-wide pooled (migration 030) and surfaced on GetAccountBill's + // AccountOverageMicros. Bundles the PaaS infra credit surfaced below. BaseFeeMicros int64 `json:"base_fee_micros"` // ModuleUsage is 模組使用量 — one line per (module, metric, model, @@ -551,6 +552,16 @@ type GetAccountBillResponse struct { ModuleUsageTotalMicros int64 `json:"module_usage_total_micros"` InfraTotalMicros int64 `json:"infra_total_micros"` + // AccountOverageMicros is the account-wide POOLED module overage for the + // period (migration 030): $3 × max(0, Σ live-app module_count − + // IncludedModules), charged ONCE per account (NOT per app, NOT folded into + // any Apps[].base_fee_micros). It is snapshot-first like the per-app base: + // the current period's account_overage_snapshots row when a charge leg + // billed it (the grace sweep or the boundary), else a live estimate from the + // CURRENT pooled sum. Additive field (pre-030 consumers that ignore it read + // an unchanged response shape); included in TotalMicros below. + AccountOverageMicros int64 `json:"account_overage_micros"` + // PaasCreditMicros is the ACCOUNT-level PaaS credit, applied ONCE here // (never per-app): the same ACTIVE-SaaS-subscription gate as GetAppBill's // per-app credit (v1 has no subscription system → always 0), CAPPED at @@ -558,8 +569,8 @@ type GetAccountBillResponse struct { // the same usage-only offset posture as the charge spine's allowance. PaasCreditMicros int64 `json:"paas_credit_micros"` - // TotalMicros is 最終費用 = BaseFeeTotal + ModuleUsageTotal + InfraTotal − - // PaasCredit, ≥ 0 by the credit cap. + // TotalMicros is 最終費用 = BaseFeeTotal + ModuleUsageTotal + InfraTotal + + // AccountOverage − PaasCredit, ≥ 0 by the credit cap. TotalMicros int64 `json:"total_micros"` } diff --git a/migrations/billing/030_account_wide_overage.down.sql b/migrations/billing/030_account_wide_overage.down.sql new file mode 100644 index 0000000..7163aa7 --- /dev/null +++ b/migrations/billing/030_account_wide_overage.down.sql @@ -0,0 +1,10 @@ +-- Down for 030 — drop the account-wide pooled overage timer + snapshot ledger. +-- Rolling back reverts overage to whatever the code expects: the display read +-- treats a missing snapshot as "no pooled overage charged" and a missing +-- overage_since column as never-over. Money already charged stays in +-- invoices/billing_runs (unaffected). Drop the table first, then the column. + +DROP TABLE IF EXISTS ms_billing.account_overage_snapshots; + +ALTER TABLE ms_billing.accounts + DROP COLUMN IF EXISTS overage_since; diff --git a/migrations/billing/030_account_wide_overage.up.sql b/migrations/billing/030_account_wide_overage.up.sql new file mode 100644 index 0000000..44c3554 --- /dev/null +++ b/migrations/billing/030_account_wide_overage.up.sql @@ -0,0 +1,88 @@ +-- Migration 030 — account-wide POOLED module overage (owner spec 2026-07-05, +-- confirmed reversal of the per-app overage tier). +-- +-- Base-fee v1 shipped module overage PER-APP: each app's base was +-- BaseFee + $3 × max(0, module_count − 5) +-- with 5 included modules PER APP. This migration moves the overage to an +-- ACCOUNT-WIDE POOL: one allowance of 5 included modules for the ENTIRE +-- account, charged $3/month for each module beyond the pooled 5 across ALL of +-- the account's live (non-deleted) apps. The FLAT per-app base fee ($20/app) +-- is UNCHANGED and stays per-app — only the $3/module overage math moves from +-- per-app to pooled. +-- +-- Two schema additions carry the pool + its grace timer: +-- +-- 1. accounts.overage_since — the UTC instant the account's pooled +-- SUM(module_count) over live apps FIRST crossed the included 5. NULL +-- means the account is not currently over the pool. It arms ONE grace +-- timer per ACCOUNT (not per app, not per module): when the pool first +-- exceeds 5 the timer starts; if the pool drops back to ≤5 before it +-- expires the timer is CLEARED (overage_since → NULL, no charge); if the +-- pool is still >5 after 3 days the mid-period sweep charges the pooled +-- overage prorated from grace-end to the period end. The timer is +-- recomputed by RegisterApp / SyncAppModules after every module_count +-- write (SUM(module_count) WHERE account_id = ? AND deleted_at IS NULL). +-- +-- 2. account_overage_snapshots — the ACCOUNT-scoped analogue of +-- app_base_snapshots (migration 028): one row per (account, period) that +-- FREEZES the pooled overage a charge leg actually billed for that +-- period, so the mid-period grace charge and the boundary advance leg can +-- never double-charge the same period (both consult / write this ledger, +-- keyed (account_id, period_start), the same double-charge guard +-- app_base_snapshots is for the per-app base). source records which leg +-- billed it: 'grace' (the mid-period sweep, prorated from grace-end) or +-- 'advance' (the boundary, full pooled overage for a period no sweep +-- caught) — mirroring app_base_snapshots' 'proration' vs 'advance'. +-- +-- No refunds (D1e philosophy): removing a module that drops the pool back +-- under 5 clears overage_since (stops FUTURE accrual) but never refunds +-- overage already charged this period. +-- +-- Born clean at slot 030. Companion docs update pending in +-- mirrorstack-docs/db/ms_billing/ (tables.md#account_overage_snapshots + +-- the accounts.overage_since column). + +ALTER TABLE ms_billing.accounts + ADD COLUMN overage_since TIMESTAMPTZ NULL; + +COMMENT ON COLUMN ms_billing.accounts.overage_since IS + 'UTC instant the account-wide pooled SUM(module_count) over live apps first crossed the included 5 ' + '(account-wide overage grace anchor, owner spec 2026-07-05). NULL = not currently over the pool. ' + 'Recomputed by RegisterApp / SyncAppModules; arms one 3-day grace timer per account.'; + +CREATE TABLE IF NOT EXISTS ms_billing.account_overage_snapshots ( + -- The billing account whose pooled overage this row froze. Cascade: + -- dropping the account drops its overage charge history with it. + account_id UUID NOT NULL REFERENCES ms_billing.accounts(id) ON DELETE CASCADE, + + -- The FULL anchored billing-period window this overage charge covers. + -- period_start is the display + double-charge lookup key (exact match from + -- both the mid-period sweep and the boundary leg); a grace row keys on the + -- full window's start even though its charged_micros covers only + -- [grace-end, period_end). + period_start TIMESTAMPTZ NOT NULL, + period_end TIMESTAMPTZ NOT NULL, + + -- The pooled over-count the charge tiered on: SUM(module_count) over the + -- account's live apps MINUS the included 5, snapshotted at charge time. + over_count INT NOT NULL CHECK (over_count >= 0), + + -- The pooled overage actually billed for this account-period, integer + -- micro-dollars (NEVER float). + charged_micros BIGINT NOT NULL CHECK (charged_micros >= 0), + + -- Which leg billed it: the mid-period grace sweep ('grace', prorated from + -- grace-end) or the boundary advance leg ('advance', full pooled overage). + source TEXT NOT NULL CHECK (source IN ('grace', 'advance')), + + -- The Stripe invoice item id of the overage charge (empty/NULL for a + -- 0-cent rounded charge that recorded nothing) — audit trail only. + invoice_item_id TEXT NULL, + + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + + -- One pooled-overage charge per account-period — the "charged exactly + -- once" invariant, enforced at the ledger (the same guard shape as + -- app_base_snapshots' PRIMARY KEY (app_id, period_start)). + PRIMARY KEY (account_id, period_start) +); From ee5043ced1fdb0b5651eaf74bc66ff943d6219b2 Mon Sep 17 00:00:00 2001 From: Sheng Kun Chang Date: Sun, 5 Jul 2026 07:10:29 +0800 Subject: [PATCH 2/2] fix: account-overage cross-leg double-charge + retry livelock (review findings) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #47 adversarial review found the grace sweep and the boundary leg could both charge the same period's pooled overage if a crash landed between a successful Stripe call and the account_overage_snapshots write (disjoint Idempotency-Key namespaces, so Stripe never deduped it). Adds a status column ('pending'/'charged') to account_overage_snapshots: the claim is now written BEFORE either leg calls Stripe, so a crash anywhere leaves durable evidence the other leg must respect. Also fixes the boundary reclaim livelock (a reclaimed run recomputed the overage to 0 instead of reusing the frozen amount, colliding with its own stable Idempotency-Key) and threads the genuine Stripe invoice item id back from the boundary's combined charge instead of storing the idempotency-key string. Judgment call (no product decision available): pooled-overage growth after a period's grace charge now gets an incremental top-up, conservatively prorated from the sweep's own instant forward (never retroactive) — flagged in cycle/overage.go's topUpGraceOverage doc for owner review. Co-Authored-By: Claude Opus 4.8 --- cmd/billing-cycle/main.go | 4 +- internal/account/cycle/charge.go | 149 +++++++--- internal/account/cycle/overage.go | 277 ++++++++++++++++-- internal/account/cycle/overage_test.go | 200 +++++++++++++ internal/account/cycle/service_test.go | 49 +++- internal/account/cycle/store.go | 90 ++++-- internal/account/db/models.go | 1 + internal/account/db/overage.sql.go | 111 +++++-- internal/account/db/queries/overage.sql | 69 ++++- .../billing/030_account_wide_overage.up.sql | 16 +- 10 files changed, 833 insertions(+), 133 deletions(-) diff --git a/cmd/billing-cycle/main.go b/cmd/billing-cycle/main.go index 64daec6..63790e4 100644 --- a/cmd/billing-cycle/main.go +++ b/cmd/billing-cycle/main.go @@ -262,7 +262,9 @@ func runOverageSweep(ctx context.Context, svc *cycle.Service, at time.Time, res res.Failed++ continue } - if summary.Status == cycle.OverageCharged { + if summary.Status == cycle.OverageCharged || summary.Status == cycle.OverageToppedUp { + // A top-up (finding #3 — an ALREADY-charged period whose pool grew + // further before the period closed) is also money charged, not a skip. res.OverageCharged++ } else { res.OverageSkipped++ diff --git a/internal/account/cycle/charge.go b/internal/account/cycle/charge.go index a562cc4..6999236 100644 --- a/internal/account/cycle/charge.go +++ b/internal/account/cycle/charge.go @@ -160,20 +160,46 @@ func (s *Service) RunBillingCycle(ctx context.Context, accountID uuid.UUID, peri // SEPARATE account-level POOLED overage term for the closing period, charged // ONCE per account. If this period's pooled overage already has an - // account_overage_snapshots row (the mid-period grace sweep billed it), it is - // NOT charged again here — the ledger is the double-charge guard. Otherwise - // (grace never expired within the period, or overage started too late for the - // sweep to run) the boundary charges the FULL pooled overage for the account - // and writes its own snapshot (source='advance'), so every over-the-pool - // period is billed exactly once. - _, overageAlreadyBilled, err := s.store.AccountOverageSnapshot(ctx, accountID, periodStart) + // account_overage_snapshots row, its SOURCE decides how the boundary treats + // it — the cross-leg double-charge guard (PR #47 review): + // + // - source='grace': the mid-period sweep already CLAIMED it (pending or + // charged — a crash between Stripe succeeding and the row flipping to + // 'charged' must NOT let the boundary independently charge it too, so + // ANY row, not just a settled one, excludes it here); + // - source='advance': THIS boundary run's OWN prior attempt (a crash + // between claiming the period and MarkBillingRun succeeding reclaims the + // SAME run id) — reuse the FROZEN over_count/charged_micros so the retry + // recomputes the IDENTICAL combined total and the deterministic + // ii-/inv- Idempotency-Keys never see a different amount + // (finding #2 — the boundary retry livelock). + // + // No row yet (grace never expired within the period, or overage started too + // late for the sweep to run): compute the FULL pooled overage for the + // account; it is CLAIMED (a 'pending' row written BEFORE Stripe is called) + // further down, right before the charge — so every over-the-pool period is + // billed exactly once, crash-safe. + existingOverage, overageFound, err := s.store.AccountOverageSnapshot(ctx, accountID, periodStart) if err != nil { return nil, billing.Internal("account overage snapshot lookup failed", err) } - var advanceOverage int64 - overCount := pooledModuleCount - usage.IncludedModules - if !overageAlreadyBilled && overCount > 0 { - advanceOverage = usage.AccountOverageMicros(pooledModuleCount) + var ( + advanceOverage int64 + overCount int + overageAlreadyClaimed bool // true → no fresh claim-insert needed before charging + ) + switch { + case overageFound && existingOverage.Source == "grace": + overageAlreadyClaimed = true + case overageFound && existingOverage.Source == "advance": + advanceOverage = existingOverage.ChargedMicros + overCount = existingOverage.OverCount + overageAlreadyClaimed = true + default: + overCount = pooledModuleCount - usage.IncludedModules + if overCount > 0 { + advanceOverage = usage.AccountOverageMicros(pooledModuleCount) + } } summary := &ChargeSummary{FirstRun: true, ArrearsMicros: arrears, AdvanceBaseMicros: advanceBase, AccountOverageMicros: advanceOverage} @@ -296,6 +322,40 @@ func (s *Service) RunBillingCycle(ctx context.Context, accountID uuid.UUID, peri _, newPeriodEnd = billingperiod.AnchoredPeriodWindow(periodEnd, anchorDay) } + // CLAIM the boundary's pooled-overage line BEFORE calling Stripe (the + // crash-safe marker — cycle/overage.go's header). Skipped when there is no + // overage to charge, or it is already claimed (source='grace' owns it, or + // this run's own reclaimed retry is reusing its own prior 'advance' claim). + if advanceOverage > 0 && !overageAlreadyClaimed { + inserted, err := s.store.InsertAccountOverageSnapshot(ctx, AccountOverageSnapshot{ + AccountID: accountID, + PeriodStart: periodStart, + PeriodEnd: periodEnd, + OverCount: overCount, + ChargedMicros: advanceOverage, + Source: "advance", + Status: OverageSnapshotPending, + }) + if err != nil { + return nil, billing.Internal("account overage snapshot claim failed", err) + } + if !inserted { + // Lost the race to a concurrent claim (almost certainly the mid-period + // grace sweep) — re-read and defer to the winner: never charge an + // overage amount someone else already claimed. + winning, _, err := s.store.AccountOverageSnapshot(ctx, accountID, periodStart) + if err != nil { + return nil, billing.Internal("account overage snapshot lookup failed", err) + } + advanceOverage, overCount = 0, 0 + if winning.Source == "advance" { + advanceOverage, overCount = winning.ChargedMicros, winning.OverCount + } + overageAlreadyClaimed = true + } + } + summary.AccountOverageMicros = advanceOverage + // One invoice, one pooled line: closed period's netted usage arrears + the // new period's advance base + the closing period's account-wide pooled // overage, converted micros → whole cents ONCE at the Stripe boundary (a @@ -304,11 +364,20 @@ func (s *Service) RunBillingCycle(ctx context.Context, accountID uuid.UUID, peri if err != nil { return nil, billing.Internal("micros to cents conversion failed", err) } + if cents == 0 { + // The claim-race adjustment above dropped the combined total to zero (a + // narrow concurrent-invocation edge case) — never call Stripe for $0. + if err := s.store.MarkBillingRun(ctx, runID, RunStatusInvoiced, "", 0); err != nil { + return nil, billing.Internal("mark billing run (zero after overage claim race) failed", err) + } + summary.Status = RunStatusInvoiced + return summary, nil + } summary.ChargedCents = cents // Charge. A failure after the PM gate marks the run 'failed' (auditable) and // returns the error. - inv, err := s.charge(ctx, runID, custID, cents, advanceBase > 0 || advanceOverage > 0) + inv, item, err := s.charge(ctx, runID, custID, cents, advanceBase > 0 || advanceOverage > 0) if err != nil { if markErr := s.store.MarkBillingRun(ctx, runID, RunStatusFailed, "", 0); markErr != nil { // Both failed: surface the original charge error; the failed-mark is @@ -332,6 +401,17 @@ func (s *Service) RunBillingCycle(ctx context.Context, accountID uuid.UUID, peri return nil, billing.Internal("invoice mirror upsert failed", err) } + // Flip the overage claim to 'charged' now that Stripe confirmed it, using + // the GENUINE Stripe invoice item id `item.ID` — never the ii- + // idempotency-key string (finding #4). Covers both a fresh claim (won + // above) and a reused prior 'advance' claim (a reclaimed retry) — both + // need the row settled once this combined charge succeeds. + if advanceOverage > 0 { + if err := s.store.MarkAccountOverageSnapshotCharged(ctx, accountID, periodStart, item.ID); err != nil { + return nil, billing.Internal("account overage snapshot mark-charged failed", err) + } + } + // Freeze what this boundary actually billed per app for the NEW window // (migration 028, source='advance'): the display's authoritative base for // the period, so a later SyncAppModules can never drift the shown base @@ -351,26 +431,6 @@ func (s *Service) RunBillingCycle(ctx context.Context, accountID uuid.UUID, peri } } - // Freeze the account-wide pooled overage this boundary billed for the CLOSING - // period (migration 030, source='advance') so the mid-period sweep + display - // agree it is already billed — the double-charge guard. Keyed by the closing - // period_start; ON CONFLICT DO NOTHING (a mid-period 'grace' row wins if the - // race ever writes both). Skipped when nothing was billed (already billed - // mid-period, or the account was under the pool). - if advanceOverage > 0 { - if err := s.store.InsertAccountOverageSnapshot(ctx, AccountOverageSnapshot{ - AccountID: accountID, - PeriodStart: periodStart, - PeriodEnd: periodEnd, - OverCount: overCount, - ChargedMicros: advanceOverage, - Source: "advance", - InvoiceItemID: invoiceItemIdemKey(runID), // the boundary's pooled item id (the run's ii- key) - }); err != nil { - return nil, billing.Internal("account overage snapshot insert failed", err) - } - } - if err := s.store.MarkBillingRun(ctx, runID, RunStatusInvoiced, inv.ID, cents); err != nil { return nil, billing.Internal("mark billing run (invoiced) failed", err) } @@ -381,21 +441,26 @@ func (s *Service) RunBillingCycle(ctx context.Context, accountID uuid.UUID, peri } // charge creates the Stripe invoice item + draft invoice for the boundary -// total (usage arrears + advance base), with the two deterministic -// Idempotency-Keys (ii-, inv-) so a re-run reuses the same Stripe -// objects. withBase only widens the line DESCRIPTION when the total includes -// an advance base fee — a pure-usage invoice keeps the historical line text. -// Returns the created invoice projection (id/status/amounts) for the mirror -// upsert. -func (s *Service) charge(ctx context.Context, runID uuid.UUID, custID string, cents int64, withBase bool) (billingstripe.Invoice, error) { +// total (usage arrears + advance base + pooled overage), with the two +// deterministic Idempotency-Keys (ii-, inv-) so a re-run reuses the +// same Stripe objects. withBase only widens the line DESCRIPTION when the +// total includes an advance base fee — a pure-usage invoice keeps the +// historical line text. Returns the created invoice projection (id/status/ +// amounts) for the mirror upsert AND the created invoice item (the caller +// needs its GENUINE id to freeze into account_overage_snapshots when the +// combined line includes pooled overage — finding #4 — rather than the +// idempotency-key string the item was created with). +func (s *Service) charge(ctx context.Context, runID uuid.UUID, custID string, cents int64, withBase bool) (billingstripe.Invoice, billingstripe.InvoiceItem, error) { desc := fmt.Sprintf("MirrorStack usage — run %s", runID) if withBase { desc = fmt.Sprintf("MirrorStack usage + app base fees — run %s", runID) } - if _, err := s.stripe.CreateInvoiceItem(ctx, custID, cents, chargeCurrency, desc, invoiceItemIdemKey(runID)); err != nil { - return billingstripe.Invoice{}, err + item, err := s.stripe.CreateInvoiceItem(ctx, custID, cents, chargeCurrency, desc, invoiceItemIdemKey(runID)) + if err != nil { + return billingstripe.Invoice{}, billingstripe.InvoiceItem{}, err } - return s.stripe.CreateInvoice(ctx, custID, true /* autoAdvance */, invoiceIdemKey(runID)) + inv, err := s.stripe.CreateInvoice(ctx, custID, true /* autoAdvance */, invoiceIdemKey(runID)) + return inv, item, err } // AccountsWithUsageEvents returns the accounts with raw usage_events in the diff --git a/internal/account/cycle/overage.go b/internal/account/cycle/overage.go index d1ca569..1f1ddfc 100644 --- a/internal/account/cycle/overage.go +++ b/internal/account/cycle/overage.go @@ -20,6 +20,18 @@ package cycle // the account_overage_snapshots ledger (keyed (account_id, period_start)) plus // the deterministic per-(account, period) Stripe Idempotency-Keys below — the // same money-safety pattern app_base_snapshots gives the per-app base. +// +// CRASH-SAFETY (PR #47 review — the cross-leg double-charge finding): the +// ledger row is written as a claim BEFORE either leg calls Stripe +// (status='pending'; see migration 030), not only after Stripe succeeds. A +// crash between "Stripe succeeded" and "the row committed" used to leave NO +// row for the OTHER leg to see, so it would independently charge the SAME +// period's overage under a completely disjoint Idempotency-Key namespace — a +// real double charge. Now the claim is visible to both legs the instant it is +// written, before any money moves, so a crash anywhere in the sequence leaves +// evidence the other leg (and the same leg's own retry) can see and must +// respect: ANY row for the period — pending or charged — means "claimed, +// never independently charge it". import ( "context" @@ -65,6 +77,10 @@ const ( // on the next sweep (the SAME per-(account, period) Stripe idem key stays // stable), never a failure. OverageSkippedNoPM OverageChargeStatus = "skipped_no_pm" + // OverageToppedUp: an ALREADY-charged period's pool grew further while the + // period was still open, and the sweep charged the incremental delta + // (finding #3, a judgment call — see topUpGraceOverage's doc). + OverageToppedUp OverageChargeStatus = "topped_up" ) // OverageChargeSummary reports what one ChargeAccountOverage call did. @@ -118,21 +134,33 @@ func (s *Service) recomputeAccountOverage(ctx context.Context, accountID uuid.UU // // 1. resolves the account's CURRENT anchored period (from activated_at) and the // grace-end instant (overage_since + overageGraceWindow); -// 2. skips if this period's pooled overage already has a snapshot (a prior -// sweep or the boundary billed it — the double-charge guard); -// 3. reads the CURRENT pool; if it dropped back to ≤ IncludedModules, clears -// the timer and skips (no refund of anything already charged); -// 4. prices the pooled overage PRORATED from grace-end to the period end -// (ProratedOverageMicros) → whole cents at the Stripe boundary; a 0-cent -// result skips (grace ends at/after the period end); -// 5. charges via the SAME Stripe plumbing as the other legs with the -// deterministic per-(account, period) Idempotency-Keys, mirrors the invoice, -// and freezes the account_overage_snapshots row (source='grace'). +// 2. reads this period's account_overage_snapshots row, if any (the +// double-charge guard): +// - source='advance' (the boundary claimed it, pending or charged) → +// skip, never independently charge it; +// - source='grace', status='charged' → already settled; the only further +// work is a possible TOP-UP if the pool grew further within this SAME +// still-open period (topUpGraceOverage, finding #3); +// - source='grace', status='pending' → THIS leg's own attempt died +// between claiming the period and Stripe confirming (or before Stripe +// was ever called) — resume it, reusing the FROZEN over_count / +// charged_micros so the retry recomputes the IDENTICAL amount and the +// deterministic Idempotency-Key stays valid; +// 3. otherwise (no row): reads the CURRENT pool — ≤ IncludedModules clears the +// timer and skips (no refund of anything already charged, D1e); prices the +// pooled overage PRORATED from grace-end to the period end +// (ProratedOverageMicros) → whole cents; a 0-cent result skips (grace ends +// at/after the period end); +// 4. CLAIMS the period (a 'pending' account_overage_snapshots row) BEFORE +// calling Stripe — the crash-safe marker described in this file's header — +// then charges via the SAME Stripe plumbing as the other legs with the +// deterministic per-(account, period) Idempotency-Keys, mirrors the +// invoice, and flips the row to 'charged' with the genuine Stripe item id. // // Gated on a usable default PM exactly like the spine (the candidate is already -// activated). A failure after the charge leaves no snapshot; the next sweep -// re-attempts through the SAME idem key (Stripe dedupes) — retry-safe, never a -// double charge. +// activated). A failure after the claim leaves the row 'pending'; the next +// sweep resumes through the SAME idem key (Stripe dedupes) — retry-safe, never +// a double charge. func (s *Service) ChargeAccountOverage(ctx context.Context, cand OverageGraceCandidate, at time.Time) (*OverageChargeSummary, error) { if cand.ID == uuid.Nil { return nil, billing.InvalidInput("account_id required") @@ -149,15 +177,24 @@ func (s *Service) ChargeAccountOverage(ctx context.Context, cand OverageGraceCan anchorDay := billingperiod.AnchorDay(cand.ActivatedAt) periodStart, periodEnd := billingperiod.AnchoredPeriodWindow(at.UTC(), anchorDay) - summary := &OverageChargeSummary{PeriodStart: periodStart} - // Double-charge guard: this period's pooled overage was already billed (by a - // prior sweep run OR the boundary that closed a prior period into this one). - if _, snapped, err := s.store.AccountOverageSnapshot(ctx, cand.ID, periodStart); err != nil { + existing, found, err := s.store.AccountOverageSnapshot(ctx, cand.ID, periodStart) + if err != nil { return nil, billing.Internal("account overage snapshot lookup failed", err) - } else if snapped { - summary.Status = OverageSkippedAlreadyCharged - return summary, nil + } + if found { + if existing.Source == "advance" { + // The boundary claimed (pending or charged) this period's overage — + // the mid-period sweep must NEVER independently charge it, crash + // window or not (the cross-leg double-charge guard, PR #47 review). + return &OverageChargeSummary{Status: OverageSkippedAlreadyCharged, PeriodStart: periodStart, OverCount: existing.OverCount}, nil + } + if existing.Status == OverageSnapshotCharged { + return s.topUpGraceOverage(ctx, cand, existing, at, periodStart, periodEnd) + } + // existing.Status == pending, existing.Source == "grace": resume OUR + // own crashed attempt, reusing the frozen amount. + return s.completeGraceCharge(ctx, cand, existing.OverCount, existing.ChargedMicros, graceEnd, periodStart, periodEnd, true /* alreadyClaimed */) } pooled, err := s.store.PooledModuleCount(ctx, cand.ID) @@ -171,10 +208,8 @@ func (s *Service) ChargeAccountOverage(ctx context.Context, cand OverageGraceCan if err := s.store.ClearAccountOverage(ctx, cand.ID); err != nil { return nil, billing.Internal("clear account overage timer failed", err) } - summary.Status = OverageSkippedUnderPool - return summary, nil + return &OverageChargeSummary{Status: OverageSkippedUnderPool, PeriodStart: periodStart}, nil } - summary.OverCount = overCount // Prorate the pooled overage from grace-end to the period end. A 0-cent // result (grace ends at/after this period's end) means nothing to bill this @@ -185,8 +220,24 @@ func (s *Service) ChargeAccountOverage(ctx context.Context, cand OverageGraceCan return nil, billing.Internal("micros to cents conversion failed", err) } if cents == 0 { - summary.Status = OverageSkippedZeroCents - return summary, nil + return &OverageChargeSummary{Status: OverageSkippedZeroCents, PeriodStart: periodStart, OverCount: overCount}, nil + } + + return s.completeGraceCharge(ctx, cand, overCount, proratedMicros, graceEnd, periodStart, periodEnd, false /* alreadyClaimed */) +} + +// completeGraceCharge runs the PM/customer gate + the actual Stripe charge for +// the mid-period grace leg, given an already-resolved (overCount, +// chargedMicros) — either freshly computed (alreadyClaimed=false: this call +// still needs to CLAIM the period before Stripe) or reused from an existing +// 'pending' row (alreadyClaimed=true: THIS leg already claimed it on a prior +// attempt; resume straight to the PM/Stripe steps with the SAME frozen amount +// so the deterministic Idempotency-Key never sees a different total). +func (s *Service) completeGraceCharge(ctx context.Context, cand OverageGraceCandidate, overCount int, chargedMicros int64, graceEnd, periodStart, periodEnd time.Time, alreadyClaimed bool) (*OverageChargeSummary, error) { + summary := &OverageChargeSummary{PeriodStart: periodStart, OverCount: overCount} + cents, err := centsFromMicros(chargedMicros) + if err != nil { + return nil, billing.Internal("micros to cents conversion failed", err) } summary.ChargedCents = cents @@ -206,6 +257,28 @@ func (s *Service) ChargeAccountOverage(ctx context.Context, cand OverageGraceCan return nil, billing.Internal("account has a usable PM but no Stripe customer id", nil) } + if !alreadyClaimed { + // CLAIM the period BEFORE calling Stripe (the crash-safe marker — see + // this file's header). inserted=false means we lost a race to a + // concurrent claim (another sweep invocation, or the boundary) — defer + // to the winner instead of charging under our own stale claim. + inserted, err := s.store.InsertAccountOverageSnapshot(ctx, AccountOverageSnapshot{ + AccountID: cand.ID, + PeriodStart: periodStart, + PeriodEnd: periodEnd, + OverCount: overCount, + ChargedMicros: chargedMicros, + Source: "grace", + Status: OverageSnapshotPending, + }) + if err != nil { + return nil, billing.Internal("account overage snapshot claim failed", err) + } + if !inserted { + return s.resumeClaimedOverage(ctx, cand, graceEnd, periodStart, periodEnd) + } + } + desc := fmt.Sprintf("MirrorStack module overage (account pool, %d over) — account %s", overCount, cand.ID) item, err := s.stripe.CreateInvoiceItem(ctx, custID, cents, chargeCurrency, desc, accountOverageItemIdemKey(cand.ID, periodStart)) if err != nil { @@ -232,22 +305,148 @@ func (s *Service) ChargeAccountOverage(ctx context.Context, cand OverageGraceCan return nil, billing.Internal("invoice mirror upsert failed", err) } - // Freeze the ledger row keyed by the FULL anchored period_start (the display + - // double-charge identity) — source='grace', the prorated amount actually - // invoiced. ON CONFLICT DO NOTHING makes a retry idempotent. - if err := s.store.InsertAccountOverageSnapshot(ctx, AccountOverageSnapshot{ + // Flip the claim to 'charged' now that Stripe confirmed it, recording the + // GENUINE Stripe invoice item id (never an idempotency-key string). + if err := s.store.MarkAccountOverageSnapshotCharged(ctx, cand.ID, periodStart, item.ID); err != nil { + return nil, billing.Internal("account overage snapshot mark-charged failed", err) + } + + summary.Status = OverageCharged + summary.StripeInvoiceID = inv.ID + return summary, nil +} + +// resumeClaimedOverage handles the (rare, only-possible-under-true-concurrency) +// case where completeGraceCharge's own claim-insert LOST a race: by the time it +// tried to claim the period, some OTHER caller (a concurrent sweep invocation, +// or the boundary) had already inserted a row first. Re-reads the winning row +// and defers to it — an 'advance' winner means the boundary owns this period +// (skip); a 'grace' winner that is already 'charged' means another sweep +// invocation finished first (skip); a 'grace' winner still 'pending' means +// another invocation is mid-flight — resume IT (same frozen amount). +func (s *Service) resumeClaimedOverage(ctx context.Context, cand OverageGraceCandidate, graceEnd, periodStart, periodEnd time.Time) (*OverageChargeSummary, error) { + existing, found, err := s.store.AccountOverageSnapshot(ctx, cand.ID, periodStart) + if err != nil { + return nil, billing.Internal("account overage snapshot lookup failed", err) + } + if !found { + // The conflict that sent us here proves a row exists; a missing read + // immediately after is a driver/consistency anomaly. + return nil, billing.Internal("account overage snapshot claim lost but no row found on re-read", nil) + } + if existing.Source == "advance" || existing.Status == OverageSnapshotCharged { + return &OverageChargeSummary{Status: OverageSkippedAlreadyCharged, PeriodStart: periodStart, OverCount: existing.OverCount}, nil + } + return s.completeGraceCharge(ctx, cand, existing.OverCount, existing.ChargedMicros, graceEnd, periodStart, periodEnd, true) +} + +// topUpGraceOverage is finding #3's fix — A JUDGMENT CALL (PR #47 review): no +// product decision was available on the exact top-up policy, so this +// implements the safest technically-correct interpretation, flagged here and +// in the PR description. recomputeAccountOverage only ever arms/clears the +// grace timer; it never re-evaluates an ALREADY-CHARGED period's snapshot, so +// pooled-module growth after the period's first grace charge went permanently +// unbilled for that period (and the display, which is snapshot-first, never +// reflected it either). Because the timer stays armed until the pool drops +// back under the pool (recomputeAccountOverage), the sweep keeps re-invoking +// ChargeAccountOverage for this account on every pass — so THIS function now +// re-checks a 'charged' period's current pool against what was billed and, if +// it grew, charges the INCREMENTAL delta, conservatively prorated from THIS +// sweep's instant (`at`) to the period end — never retroactively for time +// before this sweep noticed the growth (the safest, never-overcharge +// interpretation; it may under-bill by the gap between the actual install and +// the next sweep tick, which is bounded by the sweep's cron interval). +// +// A pool that merely dropped (or stayed the same) since the last charge is +// D1e (no refund) — this only ever ADDS to what is billed, never subtracts. +// The top-up's own Idempotency-Keys are derived from the TARGET cumulative +// over-count, so a crash-and-retry of the SAME top-up (pool unchanged since) +// reuses the SAME Stripe objects; if the pool grows AGAIN before this one +// resolves, the next pass computes a NEW target and a NEW (legitimately +// different) key — never a collision with the one still in flight. +func (s *Service) topUpGraceOverage(ctx context.Context, cand OverageGraceCandidate, existing AccountOverageSnapshot, at, periodStart, periodEnd time.Time) (*OverageChargeSummary, error) { + pooled, err := s.store.PooledModuleCount(ctx, cand.ID) + if err != nil { + return nil, billing.Internal("pooled module count lookup failed", err) + } + overCount := pooled - usage.IncludedModules + if overCount <= existing.OverCount { + if overCount <= 0 { + if err := s.store.ClearAccountOverage(ctx, cand.ID); err != nil { + return nil, billing.Internal("clear account overage timer failed", err) + } + } + return &OverageChargeSummary{Status: OverageSkippedAlreadyCharged, PeriodStart: periodStart, OverCount: existing.OverCount}, nil + } + + deltaOverCount := overCount - existing.OverCount + deltaFullMicros := usage.ModuleOverageFeeMicros * int64(deltaOverCount) + deltaMicros := usage.ProratedOverageMicros(deltaFullMicros, at, periodStart, periodEnd) + deltaCents, err := centsFromMicros(deltaMicros) + if err != nil { + return nil, billing.Internal("micros to cents conversion failed", err) + } + summary := &OverageChargeSummary{PeriodStart: periodStart, OverCount: overCount} + if deltaCents == 0 { + summary.Status = OverageSkippedZeroCents + return summary, nil + } + summary.ChargedCents = deltaCents + + hasPM, err := s.store.HasUsableDefaultPM(ctx, cand.ID) + if err != nil { + return nil, billing.Internal("usable PM check failed", err) + } + if !hasPM { + summary.Status = OverageSkippedNoPM + return summary, nil // retained; re-attempted next sweep through the same idem key + } + custID, err := s.store.AccountStripeCustomer(ctx, cand.ID) + if err != nil { + return nil, billing.Internal("stripe customer lookup failed", err) + } + if custID == "" { + return nil, billing.Internal("account has a usable PM but no Stripe customer id", nil) + } + + desc := fmt.Sprintf("MirrorStack module overage top-up (account pool, %d over, +%d) — account %s", overCount, deltaOverCount, cand.ID) + item, err := s.stripe.CreateInvoiceItem(ctx, custID, deltaCents, chargeCurrency, desc, accountOverageTopUpItemIdemKey(cand.ID, periodStart, overCount)) + if err != nil { + return nil, billing.StripeError("overage top-up invoice item failed", err) + } + inv, err := s.stripe.CreateInvoice(ctx, custID, true /* autoAdvance */, accountOverageTopUpInvoiceIdemKey(cand.ID, periodStart, overCount)) + if err != nil { + return nil, billing.StripeError("overage top-up invoice failed", err) + } + + if err := s.store.UpsertInvoice(ctx, InvoiceMirror{ + AccountID: cand.ID, + StripeInvoiceID: inv.ID, + Status: inv.Status, + AmountDueCents: inv.AmountDue, + AmountPaidCents: inv.AmountPaid, + Currency: chargeCurrency, + // The incremental coverage starts at THIS sweep's instant (the + // conservative proration basis above), never retroactively. + PeriodStart: usage.ProrationCoverageStart(at, periodStart), + PeriodEnd: periodEnd, + }); err != nil { + return nil, billing.Internal("invoice mirror upsert failed", err) + } + + if err := s.store.TopUpAccountOverageSnapshot(ctx, AccountOverageSnapshot{ AccountID: cand.ID, PeriodStart: periodStart, PeriodEnd: periodEnd, OverCount: overCount, - ChargedMicros: proratedMicros, + ChargedMicros: existing.ChargedMicros + deltaMicros, // cumulative total actually billed for the period Source: "grace", InvoiceItemID: item.ID, }); err != nil { - return nil, billing.Internal("account overage snapshot insert failed", err) + return nil, billing.Internal("account overage snapshot top-up failed", err) } - summary.Status = OverageCharged + summary.Status = OverageToppedUp summary.StripeInvoiceID = inv.ID return summary, nil } @@ -266,3 +465,17 @@ func accountOverageItemIdemKey(accountID uuid.UUID, periodStart time.Time) strin func accountOverageInvoiceIdemKey(accountID uuid.UUID, periodStart time.Time) string { return "acct-overage-inv-" + accountID.String() + "-" + strconv.FormatInt(periodStart.UTC().Unix(), 10) } + +// accountOverageTopUpItemIdemKey / accountOverageTopUpInvoiceIdemKey build the +// deterministic Stripe Idempotency-Keys for an INCREMENTAL top-up charge +// (finding #3), keyed additionally by the TARGET cumulative over-count so a +// retry of the SAME top-up (pool unchanged since) reuses the SAME Stripe +// objects, while a further pool change before it resolves computes a NEW +// (legitimately distinct) key rather than colliding with the one in flight. +func accountOverageTopUpItemIdemKey(accountID uuid.UUID, periodStart time.Time, targetOverCount int) string { + return accountOverageItemIdemKey(accountID, periodStart) + "-topup-" + strconv.Itoa(targetOverCount) +} + +func accountOverageTopUpInvoiceIdemKey(accountID uuid.UUID, periodStart time.Time, targetOverCount int) string { + return accountOverageInvoiceIdemKey(accountID, periodStart) + "-topup-" + strconv.Itoa(targetOverCount) +} diff --git a/internal/account/cycle/overage_test.go b/internal/account/cycle/overage_test.go index 3bcbd56..6a3297e 100644 --- a/internal/account/cycle/overage_test.go +++ b/internal/account/cycle/overage_test.go @@ -8,6 +8,8 @@ package cycle_test import ( "context" + "errors" + "strings" "testing" "time" @@ -332,3 +334,201 @@ func TestAccountOverageMicros_IsPooledNotPerApp(t *testing.T) { // whole point of the reversal. require.EqualValues(t, 9_000_000, usage.AccountOverageMicros(4+4)) } + +// --- PR #47 review fixes: regression tests ---------------------------------- + +// Finding #1 [CRITICAL] — cross-leg double charge. Before the fix, +// ChargeAccountOverage called Stripe BEFORE writing account_overage_snapshots; +// a crash between "Stripe succeeded" and "the row committed" left NO row for +// the boundary to see, so it independently charged the FULL pooled overage +// again under a disjoint Idempotency-Key namespace — a real double charge. +// The fix claims the period (a 'pending' row) BEFORE calling Stripe, so the +// claim survives any crash after that point. This test simulates the crash by +// injecting a failure at the LAST step (flipping 'pending' → 'charged') — +// AFTER Stripe already succeeded — and proves the boundary that runs next +// still sees the claim and does NOT charge the overage a second time. +func TestChargeAccountOverage_CrashAfterStripeSuccess_BoundaryDoesNotDoubleCharge(t *testing.T) { + at := time.Date(2026, 6, 15, 12, 0, 0, 0, time.UTC) + since := time.Date(2026, 5, 28, 0, 0, 0, 0, time.UTC) // graceEnd May 31 < Jun 1 → full overage this period + store := newFakeStore() + jun1 := time.Date(2026, 6, 1, 0, 0, 0, 0, time.UTC) + jul1 := time.Date(2026, 7, 1, 0, 0, 0, 0, time.UTC) + acct := armedOverageAccount(store, 2, 4, since, jun1) // pool 8 → 3 over → $9.00 pooled overage + sc := newFakeStripe() + svc := overageSvc(store, sc, at) + cand := cycle.OverageGraceCandidate{ID: acct, OverageSince: since, ActivatedAt: store.activation[acct]} + + // Simulate the crash: Stripe already confirmed the charge (item + invoice + // calls below prove it), but the LAST write — flipping the claim row from + // 'pending' to 'charged' — fails, exactly like a Lambda dying right there. + store.errMarkOverageSnap = errors.New("boom: process died before the claim flipped to charged") + _, err := svc.ChargeAccountOverage(context.Background(), cand, at) + require.Error(t, err, "the crash surfaces as an error to the caller (the Lambda dies / retries)") + + // Stripe's charge already went through before the crash. + require.Len(t, sc.invoiceCalls, 1, "the grace leg's Stripe call already succeeded before the simulated crash") + require.EqualValues(t, 900, sc.itemCalls[0].amountCfg) + + // CRITICAL: the claim row survives the crash (it was written BEFORE Stripe, + // not after) — this is the durable evidence the OTHER leg must respect. + snap, ok := store.overageSnaps[acctSnapKey{acct, jun1}] + require.True(t, ok, "the pending claim row must survive the crash") + require.Equal(t, "grace", snap.Source) + + // The BOUNDARY now closes [Jun 1, Jul 1). WITHOUT the fix (no row would + // exist at this point), it would independently charge the FULL $9.00 + // pooled overage AGAIN under its own ii- Idempotency-Key — a real + // double charge totaling $18.00 for a $9.00 debt. WITH the fix, it must see + // the claim and charge ZERO overage. + store.errMarkOverageSnap = nil // the injected fault was specific to the grace leg's crash + resp, err := svc.RunBillingCycle(context.Background(), acct, jun1, jul1, 0) + require.NoError(t, err) + require.EqualValues(t, 0, resp.AccountOverageMicros, + "the boundary must NOT independently charge the pooled overage the grace leg already claimed, crash or not") + require.EqualValues(t, 40_000_000, resp.AdvanceBaseMicros, "the flat per-app base is unaffected") + require.EqualValues(t, 4_000, resp.ChargedCents, "base only ($40.00) — NOT $49.00, which would be the double charge") + + // Exactly TWO invoices total: the grace leg's original $9.00 overage charge + // + the boundary's base-only invoice. Never a second overage charge. + require.Len(t, sc.invoiceCalls, 2) +} + +// Finding #2 [HIGH] — boundary retry livelock. Before the fix, a reclaim of a +// 'pending' billing_run (after InsertAccountOverageSnapshot succeeded but +// MarkBillingRun crashed) recomputed advanceOverage FRESH from +// snapshot-presence, collapsing it from a real amount to $0 — a DIFFERENT +// combined total reusing the SAME deterministic Stripe Idempotency-Key, which +// a real Stripe would reject (a mismatched-parameter idempotency-key reuse), +// permanently stuck. The fix freezes the overage amount into the +// account_overage_snapshots row at the FIRST attempt and REUSES it verbatim on +// every reclaim of the same run. +func TestRunBillingCycle_ReclaimAfterMarkBillingRunFailure_KeepsStableOverageAmount(t *testing.T) { + store := newFakeStore() + store.chargedTotal = 0 + store.hasPM = true + store.stripeCustomer = "cus_reclaim_overage" + seedApp(store, chargeAccount, 4, false) + seedApp(store, chargeAccount, 4, false) // pool 8 → 3 over → $9.00 pooled overage + sc := newFakeStripe() + svc := chargeSvc(store, sc) + + // Attempt #1: the combined charge (base $40.00 + overage $9.00 = $49.00 = + // 4_900 cents) succeeds at Stripe and the overage snapshot is claimed + + // marked 'charged' — but MarkBillingRun then fails/crashes, so the run row + // stays 'pending' (non-terminal). + store.errMarkRun = errors.New("boom: process died before the run's terminal write landed") + _, err := svc.RunBillingCycle(context.Background(), chargeAccount, periodStart, periodEnd, 0) + require.Error(t, err) + require.Len(t, sc.invoiceCalls, 1) + require.EqualValues(t, 4_900, sc.itemCalls[0].amountCfg) + + snap, ok := store.overageSnaps[acctSnapKey{chargeAccount, periodStart}] + require.True(t, ok) + require.Equal(t, "advance", snap.Source) + require.Equal(t, cycle.OverageSnapshotCharged, snap.Status) + require.EqualValues(t, 9_000_000, snap.ChargedMicros) + + // Attempt #2: RECLAIM — InsertBillingRun reuses the SAME run id (the row is + // still 'pending'). WITHOUT the fix, advanceOverage recomputes to $0 (the + // snapshot "looks already billed" from the boundary's own prior write), + // giving a DIFFERENT combined total ($40.00 = 4_000 cents) reusing the SAME + // Idempotency-Key — a real Stripe rejects this and the run is stuck + // forever. WITH the fix, the overage amount is FROZEN and reused, so + // attempt #2 computes the IDENTICAL $49.00 = 4_900 cents. + store.errMarkRun = nil + resp, err := svc.RunBillingCycle(context.Background(), chargeAccount, periodStart, periodEnd, 0) + require.NoError(t, err) + require.True(t, resp.FirstRun, "the pending run is reclaimed for a fresh attempt") + require.Equal(t, cycle.RunStatusInvoiced, resp.Status) + require.EqualValues(t, 9_000_000, resp.AccountOverageMicros, + "the overage amount must stay stable across the reclaim retry, never recompute to 0") + require.EqualValues(t, 4_900, resp.ChargedCents, "the SAME combined total as attempt #1 — never 4_000 (base-only)") + require.Len(t, sc.invoiceCalls, 2, "the reclaim re-calls Stripe with the SAME idem key (safe/idempotent for the real client)") + require.EqualValues(t, 4_900, sc.itemCalls[1].amountCfg, "attempt #2's item amount matches attempt #1's exactly") + require.Equal(t, sc.itemCalls[0].idemKey, sc.itemCalls[1].idemKey, "the SAME deterministic ii- key is reused across the reclaim") + require.Len(t, store.insertedRuns, 1, "reclaim reuses the same run row, never a second one") +} + +// Finding #3 [MEDIUM, judgment call] — pooled-overage growth after the +// period's grace charge went permanently unbilled (recomputeAccountOverage +// only arms/clears the timer; it never re-evaluated an already-charged +// period). The fix charges an INCREMENTAL top-up, conservatively prorated from +// the sweep's own instant to the period end (never retroactively). +func TestChargeAccountOverage_PoolGrowthMidPeriodChargesIncrementalTopUp(t *testing.T) { + // First charge: pool 8 (2 apps × 4) → 3 over → $9.00 (900 cents), the exact + // fixture TestChargeAccountOverage_ChargesFullWhenGraceEndedBeforePeriodStart + // uses. Then a THIRD app (4 modules) installs mid-period, growing the pool + // to 12 → 7 over. A later sweep pass (Jun 20 — 11 days left of the 30-day + // [Jun 1, Jul 1) period) must charge the INCREMENTAL 4-module delta + // prorated for the remaining 11 days: 4 × $3.00 × 11/30 = $4.40 (440 + // cents) — never $0 (the pre-fix behavior, permanently unbilled) and never + // the full $12.00 (never retroactive for time before the sweep noticed). + at1 := time.Date(2026, 6, 15, 12, 0, 0, 0, time.UTC) + since := time.Date(2026, 5, 28, 0, 0, 0, 0, time.UTC) + store := newFakeStore() + jun1 := time.Date(2026, 6, 1, 0, 0, 0, 0, time.UTC) + acct := armedOverageAccount(store, 2, 4, since, jun1) + sc := newFakeStripe() + cand := cycle.OverageGraceCandidate{ID: acct, OverageSince: since, ActivatedAt: store.activation[acct]} + + first, err := overageSvc(store, sc, at1).ChargeAccountOverage(context.Background(), cand, at1) + require.NoError(t, err) + require.Equal(t, cycle.OverageCharged, first.Status) + require.EqualValues(t, 900, first.ChargedCents) + + // Pool grows mid-period: a third app (4 modules) installs, 8 → 12. + seedAppCreated(store, acct, 4, false, jun1.AddDate(0, 0, -1)) + + at2 := time.Date(2026, 6, 20, 9, 0, 0, 0, time.UTC) + second, err := overageSvc(store, sc, at2).ChargeAccountOverage(context.Background(), cand, at2) + require.NoError(t, err) + require.Equal(t, cycle.OverageToppedUp, second.Status) + require.Equal(t, 7, second.OverCount, "pool 12 − included 5 = 7 over, the NEW cumulative over-count") + require.EqualValues(t, 440, second.ChargedCents, "4 incremental modules × $3.00 × 11/30 remaining days = $4.40 — never $0, never $12.00") + + require.Len(t, sc.invoiceCalls, 2, "one invoice for the original charge, one for the top-up") + require.NotEqual(t, sc.itemCalls[0].idemKey, sc.itemCalls[1].idemKey, "the top-up uses its OWN Idempotency-Key, distinct from the original charge") + + snap := store.overageSnaps[acctSnapKey{acct, jun1}] + require.Equal(t, 7, snap.OverCount) + require.EqualValues(t, 13_400_000, snap.ChargedMicros, + "cumulative: the original $9.00 + the $4.40 top-up = $13.40 total billed for the period — the display reads this exact figure") + require.Equal(t, cycle.OverageSnapshotCharged, snap.Status) + + // A third pass with NO further pool growth is a clean no-op (no third + // charge, D1e — never re-bill what is already covered). + third, err := overageSvc(store, sc, at2.Add(time.Hour)).ChargeAccountOverage(context.Background(), cand, at2.Add(time.Hour)) + require.NoError(t, err) + require.Equal(t, cycle.OverageSkippedAlreadyCharged, third.Status) + require.Len(t, sc.invoiceCalls, 2, "no third invoice when the pool hasn't grown further") +} + +// Finding #4 [MEDIUM] — the boundary's account_overage_snapshots row +// (source='advance') stored the literal Idempotency-Key STRING ("ii-") +// as InvoiceItemID instead of the genuine Stripe invoice item id, because +// s.charge() discarded CreateInvoiceItem's return value. The fix threads the +// real item back from s.charge() and stores IT. +func TestRunBillingCycle_AdvanceOverageSnapshotStoresGenuineStripeItemID(t *testing.T) { + store := newFakeStore() + store.chargedTotal = 0 + store.hasPM = true + store.stripeCustomer = "cus_item_id" + seedApp(store, chargeAccount, 4, false) + seedApp(store, chargeAccount, 4, false) // pool 8 → 3 over → $9.00 pooled overage + sc := newFakeStripe() + + resp, err := chargeSvc(store, sc).RunBillingCycle(context.Background(), chargeAccount, periodStart, periodEnd, 0) + require.NoError(t, err) + require.EqualValues(t, 9_000_000, resp.AccountOverageMicros) + require.Len(t, sc.itemCalls, 1) + + snap, ok := store.overageSnaps[acctSnapKey{chargeAccount, periodStart}] + require.True(t, ok) + + idemKey := sc.itemCalls[0].idemKey + require.True(t, strings.HasPrefix(idemKey, "ii-"), "sanity: the item was created under the ii- idempotency key") + require.NotEqual(t, idemKey, snap.InvoiceItemID, + "the stored id must be the GENUINE Stripe invoice item id, never the ii- idempotency-key string") + require.True(t, strings.HasPrefix(snap.InvoiceItemID, "ii_test_"), + "must be the REAL Stripe invoice item id the fake client generated for this call") +} diff --git a/internal/account/cycle/service_test.go b/internal/account/cycle/service_test.go index 0adea43..cda3164 100644 --- a/internal/account/cycle/service_test.go +++ b/internal/account/cycle/service_test.go @@ -120,6 +120,14 @@ type fakeStore struct { errOverageGrace error // AccountsInOverageGrace errOverageSnap error // AccountOverageSnapshot errInsertOverageSnap error // InsertAccountOverageSnapshot + errMarkOverageSnap error // MarkAccountOverageSnapshotCharged + errTopUpOverageSnap error // TopUpAccountOverageSnapshot + + // overageClaimLoses, when > 0, makes the NEXT N InsertAccountOverageSnapshot + // calls report "lost the race" (inserted=false) WITHOUT actually writing + // anything — simulating a concurrent claim that beat this call, so a test + // can drive the claim-insert-loses-the-race path deterministically. + overageClaimLoses int } // acctSnapKey mirrors the account_overage_snapshots PRIMARY KEY @@ -564,16 +572,51 @@ func (f *fakeStore) AccountOverageSnapshot(_ context.Context, accountID uuid.UUI return snap, ok, nil } -func (f *fakeStore) InsertAccountOverageSnapshot(_ context.Context, snap cycle.AccountOverageSnapshot) error { +func (f *fakeStore) InsertAccountOverageSnapshot(_ context.Context, snap cycle.AccountOverageSnapshot) (bool, error) { if f.errInsertOverageSnap != nil { - return f.errInsertOverageSnap + return false, f.errInsertOverageSnap + } + if f.overageClaimLoses > 0 { + f.overageClaimLoses-- + return false, nil // simulate a concurrent claim winning the race } // ON CONFLICT (account_id, period_start) DO NOTHING: an existing row wins. k := acctSnapKey{snap.AccountID, snap.PeriodStart} if _, exists := f.overageSnaps[k]; exists { - return nil + return false, nil } f.overageSnaps[k] = snap + return true, nil +} + +func (f *fakeStore) MarkAccountOverageSnapshotCharged(_ context.Context, accountID uuid.UUID, periodStart time.Time, invoiceItemID string) error { + if f.errMarkOverageSnap != nil { + return f.errMarkOverageSnap + } + k := acctSnapKey{accountID, periodStart} + snap, ok := f.overageSnaps[k] + if !ok { + return nil // defensive: mirrors the DB's unconditional UPDATE affecting 0 rows + } + snap.Status = cycle.OverageSnapshotCharged + snap.InvoiceItemID = invoiceItemID + f.overageSnaps[k] = snap + return nil +} + +func (f *fakeStore) TopUpAccountOverageSnapshot(_ context.Context, snap cycle.AccountOverageSnapshot) error { + if f.errTopUpOverageSnap != nil { + return f.errTopUpOverageSnap + } + k := acctSnapKey{snap.AccountID, snap.PeriodStart} + existing, ok := f.overageSnaps[k] + if !ok || existing.Status != cycle.OverageSnapshotCharged { + return nil // mirrors the DB's WHERE status='charged' guard: a non-match is a no-op + } + existing.OverCount = snap.OverCount + existing.ChargedMicros = snap.ChargedMicros + existing.InvoiceItemID = snap.InvoiceItemID + f.overageSnaps[k] = existing return nil } diff --git a/internal/account/cycle/store.go b/internal/account/cycle/store.go index c8efdbd..549cd52 100644 --- a/internal/account/cycle/store.go +++ b/internal/account/cycle/store.go @@ -247,18 +247,34 @@ type Store interface { // (grace anchor) and activated_at (period anchor). AccountsInOverageGrace(ctx context.Context, cutoff time.Time) ([]OverageGraceCandidate, error) - // AccountOverageSnapshot reads the frozen pooled overage a charge leg billed - // for ONE (account, period) — the double-charge guard both the grace sweep - // and the boundary consult (found=true → this period's pooled overage was - // already billed, skip it). found=false → never charged. + // AccountOverageSnapshot reads the frozen pooled overage a charge leg + // claimed/billed for ONE (account, period) — the double-charge guard both + // the grace sweep and the boundary consult (found=true → this period's + // pooled overage is CLAIMED — status 'pending' or 'charged' — the OTHER leg + // must never independently charge it; the claiming leg resumes/reuses it). + // found=false → never claimed. AccountOverageSnapshot(ctx context.Context, accountID uuid.UUID, periodStart time.Time) (snap AccountOverageSnapshot, found bool, err error) - // InsertAccountOverageSnapshot freezes what a charge leg billed the account - // for one period's pooled overage (migration 030) with ON CONFLICT - // (account_id, period_start) DO NOTHING — an existing row (a prior grace - // charge, or a reclaimed boundary attempt's own row) wins, so a re-run never - // rewrites what was already recorded as billed. - InsertAccountOverageSnapshot(ctx context.Context, snap AccountOverageSnapshot) error + // InsertAccountOverageSnapshot CLAIMS one period's pooled overage charge for + // a leg — callers write status="pending" BEFORE calling Stripe (the + // crash-safe marker; see migration 030's status column doc) — with + // ON CONFLICT (account_id, period_start) DO NOTHING. inserted=false means + // the row already existed (a prior claim by this leg or the other one) and + // the caller MUST re-read AccountOverageSnapshot and defer to the winner + // rather than proceed to charge Stripe under its own claim. + InsertAccountOverageSnapshot(ctx context.Context, snap AccountOverageSnapshot) (inserted bool, err error) + + // MarkAccountOverageSnapshotCharged flips a claimed row to status="charged" + // once Stripe actually created the invoice item/invoice, recording the + // GENUINE Stripe invoice item id (never an idempotency-key string). + MarkAccountOverageSnapshotCharged(ctx context.Context, accountID uuid.UUID, periodStart time.Time, invoiceItemID string) error + + // TopUpAccountOverageSnapshot records an incremental charge against an + // already-'charged' period whose pool grew further before the period closed + // (the mid-period sweep's top-up leg, a judgment call — see + // cycle/overage.go's topUpGraceOverage doc). snap.OverCount/ChargedMicros + // are the NEW cumulative totals for the period. + TopUpAccountOverageSnapshot(ctx context.Context, snap AccountOverageSnapshot) error } // OverageGraceCandidate is one account the mid-period grace sweep evaluates: its @@ -273,11 +289,14 @@ type OverageGraceCandidate struct { // AccountOverageSnapshot is the in-memory form of a // ms_billing.account_overage_snapshots row (migration 030): what one charge leg -// billed one account for one period's POOLED module overage. PeriodStart is the -// display + double-charge lookup key; ChargedMicros is the exact overage the -// invoice billed (prorated for a 'grace' row, full for an 'advance' row); -// OverCount is the pooled over-count it tiered on; Source is 'grace' or -// 'advance'; InvoiceItemID is the Stripe item id (empty for a 0-cent charge). +// claimed/billed one account for one period's POOLED module overage. +// PeriodStart is the display + double-charge lookup key; ChargedMicros is the +// exact overage the invoice billed (prorated for a 'grace' row, full for an +// 'advance' row, or the cumulative total after a top-up); OverCount is the +// pooled over-count it tiered on; Source is 'grace' or 'advance'; Status is +// 'pending' (claimed, Stripe not yet confirmed — the crash-safe marker) or +// 'charged' (Stripe confirmed); InvoiceItemID is the genuine Stripe item id +// (empty while Status=="pending", or for a 0-cent charge). type AccountOverageSnapshot struct { AccountID uuid.UUID PeriodStart time.Time @@ -285,9 +304,16 @@ type AccountOverageSnapshot struct { OverCount int ChargedMicros int64 Source string + Status string InvoiceItemID string } +// Account overage snapshot status values (migration 030's status column). +const ( + OverageSnapshotPending = "pending" + OverageSnapshotCharged = "charged" +) + // AppModuleCount pairs one live roster app with its module_count snapshot — // one advance-base input row. The boundary leg needs the app id (not just the // count) to write the per-app-period base snapshot it bills (migration 028). @@ -1050,22 +1076,46 @@ func (s *pgxStore) AccountOverageSnapshot(ctx context.Context, accountID uuid.UU OverCount: int(row.OverCount), ChargedMicros: row.ChargedMicros, Source: row.Source, + Status: row.Status, }, true, nil } -func (s *pgxStore) InsertAccountOverageSnapshot(ctx context.Context, snap AccountOverageSnapshot) error { - // 0 rows = ON CONFLICT DO NOTHING kept an existing row (a prior grace charge, - // or a reclaimed boundary attempt's write) — success either way. - _, err := s.q.InsertAccountOverageSnapshot(ctx, db.InsertAccountOverageSnapshotParams{ +func (s *pgxStore) InsertAccountOverageSnapshot(ctx context.Context, snap AccountOverageSnapshot) (bool, error) { + // rows=0 = ON CONFLICT DO NOTHING kept an existing row (a prior claim by + // this leg or the other one) — the caller must re-read and defer to it, + // never proceed to charge Stripe under its own (lost) claim. + rows, err := s.q.InsertAccountOverageSnapshot(ctx, db.InsertAccountOverageSnapshotParams{ AccountID: snap.AccountID.String(), PeriodStart: snap.PeriodStart, PeriodEnd: snap.PeriodEnd, OverCount: int32(snap.OverCount), //nolint:gosec // over_count = pooled sum − IncludedModules; the pool is Σ validated module_counts (each ≤ maxModuleCount), far below int32 max ChargedMicros: snap.ChargedMicros, Source: snap.Source, + Status: snap.Status, + InvoiceItemID: pgtype.Text{String: snap.InvoiceItemID, Valid: snap.InvoiceItemID != ""}, + }) + if err != nil { + return false, err + } + return rows > 0, nil +} + +func (s *pgxStore) MarkAccountOverageSnapshotCharged(ctx context.Context, accountID uuid.UUID, periodStart time.Time, invoiceItemID string) error { + return s.q.MarkAccountOverageSnapshotCharged(ctx, db.MarkAccountOverageSnapshotChargedParams{ + AccountID: accountID.String(), + PeriodStart: periodStart, + InvoiceItemID: pgtype.Text{String: invoiceItemID, Valid: invoiceItemID != ""}, + }) +} + +func (s *pgxStore) TopUpAccountOverageSnapshot(ctx context.Context, snap AccountOverageSnapshot) error { + return s.q.TopUpAccountOverageSnapshot(ctx, db.TopUpAccountOverageSnapshotParams{ + AccountID: snap.AccountID.String(), + PeriodStart: snap.PeriodStart, + OverCount: int32(snap.OverCount), //nolint:gosec // over_count = pooled sum − IncludedModules; the pool is Σ validated module_counts (each ≤ maxModuleCount), far below int32 max + ChargedMicros: snap.ChargedMicros, InvoiceItemID: pgtype.Text{String: snap.InvoiceItemID, Valid: snap.InvoiceItemID != ""}, }) - return err } // parseUUIDs parses a slice of UUID-as-string account ids (the form the sqlc diff --git a/internal/account/db/models.go b/internal/account/db/models.go index 73b077b..6053948 100644 --- a/internal/account/db/models.go +++ b/internal/account/db/models.go @@ -342,6 +342,7 @@ type MsBillingAccountOverageSnapshot struct { OverCount int32 `json:"over_count"` ChargedMicros int64 `json:"charged_micros"` Source string `json:"source"` + Status string `json:"status"` InvoiceItemID pgtype.Text `json:"invoice_item_id"` CreatedAt time.Time `json:"created_at"` } diff --git a/internal/account/db/overage.sql.go b/internal/account/db/overage.sql.go index 4edd8f5..cae90b8 100644 --- a/internal/account/db/overage.sql.go +++ b/internal/account/db/overage.sql.go @@ -75,8 +75,8 @@ func (q *Queries) ClearAccountOverage(ctx context.Context, id string) (int64, er const insertAccountOverageSnapshot = `-- name: InsertAccountOverageSnapshot :execrows INSERT INTO ms_billing.account_overage_snapshots - (account_id, period_start, period_end, over_count, charged_micros, source, invoice_item_id) -VALUES ($1, $2, $3, $4, $5, $6, $7) + (account_id, period_start, period_end, over_count, charged_micros, source, status, invoice_item_id) +VALUES ($1, $2, $3, $4, $5, $6, $7, $8) ON CONFLICT (account_id, period_start) DO NOTHING ` @@ -87,16 +87,19 @@ type InsertAccountOverageSnapshotParams struct { OverCount int32 `json:"over_count"` ChargedMicros int64 `json:"charged_micros"` Source string `json:"source"` + Status string `json:"status"` InvoiceItemID pgtype.Text `json:"invoice_item_id"` } -// InsertAccountOverageSnapshot records what a charge leg billed the account for -// one period's pooled overage (migration 030). ON CONFLICT (account_id, -// period_start) DO NOTHING: an existing row — a prior grace charge, or a prior -// reclaimed boundary attempt's own row — wins, so a re-run never rewrites what -// was already recorded as billed. :execrows so the caller can observe the no-op, -// though both outcomes are success (the Stripe idempotency key on the same -// (account, period) already deduped the money). +// InsertAccountOverageSnapshot claims ONE (account, period)'s pooled overage +// charge for a leg — status='pending' is written BEFORE the leg calls Stripe +// (see migration 030's status column comment); the leg later calls +// MarkAccountOverageSnapshotCharged once Stripe actually succeeds. ON CONFLICT +// (account_id, period_start) DO NOTHING: an existing row — a prior grace claim, +// or a prior reclaimed boundary attempt's own row — wins, so a re-run never +// rewrites what was already claimed. :execrows so the caller can tell whether +// ITS insert won the race (rows=1) or lost to a concurrent claim (rows=0) and +// must re-read + defer to the winner instead of proceeding to charge Stripe. func (q *Queries) InsertAccountOverageSnapshot(ctx context.Context, arg InsertAccountOverageSnapshotParams) (int64, error) { result, err := q.db.Exec(ctx, insertAccountOverageSnapshot, arg.AccountID, @@ -105,6 +108,7 @@ func (q *Queries) InsertAccountOverageSnapshot(ctx context.Context, arg InsertAc arg.OverCount, arg.ChargedMicros, arg.Source, + arg.Status, arg.InvoiceItemID, ) if err != nil { @@ -113,8 +117,38 @@ func (q *Queries) InsertAccountOverageSnapshot(ctx context.Context, arg InsertAc return result.RowsAffected(), nil } +const markAccountOverageSnapshotCharged = `-- name: MarkAccountOverageSnapshotCharged :exec +UPDATE ms_billing.account_overage_snapshots +SET status = 'charged', + invoice_item_id = $3 +WHERE account_id = $1 + AND period_start = $2 +` + +type MarkAccountOverageSnapshotChargedParams struct { + AccountID string `json:"account_id"` + PeriodStart time.Time `json:"period_start"` + InvoiceItemID pgtype.Text `json:"invoice_item_id"` +} + +// MarkAccountOverageSnapshotCharged flips a claimed row to 'charged' once +// Stripe actually created the invoice item/invoice, recording the GENUINE +// Stripe invoice item id (finding #4 — never the idempotency-key string). +// Unconditional on status (not just WHERE status='pending'): a retry that +// re-enters this leg after already flipping the row to 'charged' (e.g. the +// Stripe call succeeded, the write here committed, but a LATER step in the +// caller failed) must still be able to re-affirm the row harmlessly — the +// caller only ever calls this with the Stripe values it just confirmed, so +// overwriting an already-'charged' row with the same (idempotent) values is +// safe by construction (deterministic per-(account,period) Idempotency-Keys +// guarantee Stripe returns the SAME object on every re-call). +func (q *Queries) MarkAccountOverageSnapshotCharged(ctx context.Context, arg MarkAccountOverageSnapshotChargedParams) error { + _, err := q.db.Exec(ctx, markAccountOverageSnapshotCharged, arg.AccountID, arg.PeriodStart, arg.InvoiceItemID) + return err +} + const selectAccountOverageSnapshot = `-- name: SelectAccountOverageSnapshot :one -SELECT over_count, charged_micros, source +SELECT over_count, charged_micros, source, status FROM ms_billing.account_overage_snapshots WHERE account_id = $1 AND period_start = $2 @@ -129,19 +163,26 @@ type SelectAccountOverageSnapshotRow struct { OverCount int32 `json:"over_count"` ChargedMicros int64 `json:"charged_micros"` Source string `json:"source"` + Status string `json:"status"` } // SelectAccountOverageSnapshot reads the frozen pooled overage a charge leg -// billed for ONE (account, period) — the double-charge guard for the charge -// side (a row means "this period's pooled overage was already billed, skip it") -// AND the authoritative display value for GetAccountBill's pooled-overage line. -// Exact period_start match (both the grace sweep and the boundary leg key on the -// anchored window start); no row → never charged → the caller skips (charge -// side) or falls back to the live pooled estimate (display side). +// claimed/billed for ONE (account, period) — the double-charge guard for the +// charge side (a row means "this period's pooled overage is claimed — pending +// or charged — skip/resume it, never independently charge it") AND the +// authoritative display value for GetAccountBill's pooled-overage line. Exact +// period_start match (both the grace sweep and the boundary leg key on the +// anchored window start); no row → never claimed → the caller charges it +// (charge side) or falls back to the live pooled estimate (display side). func (q *Queries) SelectAccountOverageSnapshot(ctx context.Context, arg SelectAccountOverageSnapshotParams) (SelectAccountOverageSnapshotRow, error) { row := q.db.QueryRow(ctx, selectAccountOverageSnapshot, arg.AccountID, arg.PeriodStart) var i SelectAccountOverageSnapshotRow - err := row.Scan(&i.OverCount, &i.ChargedMicros, &i.Source) + err := row.Scan( + &i.OverCount, + &i.ChargedMicros, + &i.Source, + &i.Status, + ) return i, err } @@ -199,3 +240,39 @@ func (q *Queries) SumLiveModuleCount(ctx context.Context, accountID string) (int err := row.Scan(&pooled_count) return pooled_count, err } + +const topUpAccountOverageSnapshot = `-- name: TopUpAccountOverageSnapshot :exec +UPDATE ms_billing.account_overage_snapshots +SET over_count = $3, + charged_micros = $4, + invoice_item_id = $5 +WHERE account_id = $1 + AND period_start = $2 + AND status = 'charged' +` + +type TopUpAccountOverageSnapshotParams struct { + AccountID string `json:"account_id"` + PeriodStart time.Time `json:"period_start"` + OverCount int32 `json:"over_count"` + ChargedMicros int64 `json:"charged_micros"` + InvoiceItemID pgtype.Text `json:"invoice_item_id"` +} + +// TopUpAccountOverageSnapshot records an INCREMENTAL charge against an already- +// 'charged' period whose pool grew further before the period closed (finding +// #3 — a judgment call, see cycle/overage.go's topUpGraceOverage doc). Only +// fires from a 'charged' row (a 'pending' row is a resume-in-progress, never a +// top-up target); over_count/charged_micros are overwritten with the NEW +// cumulative totals so the ledger + the display always show what was actually +// billed in total for the period. +func (q *Queries) TopUpAccountOverageSnapshot(ctx context.Context, arg TopUpAccountOverageSnapshotParams) error { + _, err := q.db.Exec(ctx, topUpAccountOverageSnapshot, + arg.AccountID, + arg.PeriodStart, + arg.OverCount, + arg.ChargedMicros, + arg.InvoiceItemID, + ) + return err +} diff --git a/internal/account/db/queries/overage.sql b/internal/account/db/queries/overage.sql index bd63bf1..0a321e5 100644 --- a/internal/account/db/queries/overage.sql +++ b/internal/account/db/queries/overage.sql @@ -57,27 +57,64 @@ WHERE overage_since IS NOT NULL AND activated_at IS NOT NULL; -- SelectAccountOverageSnapshot reads the frozen pooled overage a charge leg --- billed for ONE (account, period) — the double-charge guard for the charge --- side (a row means "this period's pooled overage was already billed, skip it") --- AND the authoritative display value for GetAccountBill's pooled-overage line. --- Exact period_start match (both the grace sweep and the boundary leg key on the --- anchored window start); no row → never charged → the caller skips (charge --- side) or falls back to the live pooled estimate (display side). +-- claimed/billed for ONE (account, period) — the double-charge guard for the +-- charge side (a row means "this period's pooled overage is claimed — pending +-- or charged — skip/resume it, never independently charge it") AND the +-- authoritative display value for GetAccountBill's pooled-overage line. Exact +-- period_start match (both the grace sweep and the boundary leg key on the +-- anchored window start); no row → never claimed → the caller charges it +-- (charge side) or falls back to the live pooled estimate (display side). -- name: SelectAccountOverageSnapshot :one -SELECT over_count, charged_micros, source +SELECT over_count, charged_micros, source, status FROM ms_billing.account_overage_snapshots WHERE account_id = $1 AND period_start = $2; --- InsertAccountOverageSnapshot records what a charge leg billed the account for --- one period's pooled overage (migration 030). ON CONFLICT (account_id, --- period_start) DO NOTHING: an existing row — a prior grace charge, or a prior --- reclaimed boundary attempt's own row — wins, so a re-run never rewrites what --- was already recorded as billed. :execrows so the caller can observe the no-op, --- though both outcomes are success (the Stripe idempotency key on the same --- (account, period) already deduped the money). +-- InsertAccountOverageSnapshot claims ONE (account, period)'s pooled overage +-- charge for a leg — status='pending' is written BEFORE the leg calls Stripe +-- (see migration 030's status column comment); the leg later calls +-- MarkAccountOverageSnapshotCharged once Stripe actually succeeds. ON CONFLICT +-- (account_id, period_start) DO NOTHING: an existing row — a prior grace claim, +-- or a prior reclaimed boundary attempt's own row — wins, so a re-run never +-- rewrites what was already claimed. :execrows so the caller can tell whether +-- ITS insert won the race (rows=1) or lost to a concurrent claim (rows=0) and +-- must re-read + defer to the winner instead of proceeding to charge Stripe. -- name: InsertAccountOverageSnapshot :execrows INSERT INTO ms_billing.account_overage_snapshots - (account_id, period_start, period_end, over_count, charged_micros, source, invoice_item_id) -VALUES ($1, $2, $3, $4, $5, $6, $7) + (account_id, period_start, period_end, over_count, charged_micros, source, status, invoice_item_id) +VALUES ($1, $2, $3, $4, $5, $6, $7, $8) ON CONFLICT (account_id, period_start) DO NOTHING; + +-- MarkAccountOverageSnapshotCharged flips a claimed row to 'charged' once +-- Stripe actually created the invoice item/invoice, recording the GENUINE +-- Stripe invoice item id (finding #4 — never the idempotency-key string). +-- Unconditional on status (not just WHERE status='pending'): a retry that +-- re-enters this leg after already flipping the row to 'charged' (e.g. the +-- Stripe call succeeded, the write here committed, but a LATER step in the +-- caller failed) must still be able to re-affirm the row harmlessly — the +-- caller only ever calls this with the Stripe values it just confirmed, so +-- overwriting an already-'charged' row with the same (idempotent) values is +-- safe by construction (deterministic per-(account,period) Idempotency-Keys +-- guarantee Stripe returns the SAME object on every re-call). +-- name: MarkAccountOverageSnapshotCharged :exec +UPDATE ms_billing.account_overage_snapshots +SET status = 'charged', + invoice_item_id = $3 +WHERE account_id = $1 + AND period_start = $2; + +-- TopUpAccountOverageSnapshot records an INCREMENTAL charge against an already- +-- 'charged' period whose pool grew further before the period closed (finding +-- #3 — a judgment call, see cycle/overage.go's topUpGraceOverage doc). Only +-- fires from a 'charged' row (a 'pending' row is a resume-in-progress, never a +-- top-up target); over_count/charged_micros are overwritten with the NEW +-- cumulative totals so the ledger + the display always show what was actually +-- billed in total for the period. +-- name: TopUpAccountOverageSnapshot :exec +UPDATE ms_billing.account_overage_snapshots +SET over_count = $3, + charged_micros = $4, + invoice_item_id = $5 +WHERE account_id = $1 + AND period_start = $2 + AND status = 'charged'; diff --git a/migrations/billing/030_account_wide_overage.up.sql b/migrations/billing/030_account_wide_overage.up.sql index 44c3554..266173d 100644 --- a/migrations/billing/030_account_wide_overage.up.sql +++ b/migrations/billing/030_account_wide_overage.up.sql @@ -75,8 +75,20 @@ CREATE TABLE IF NOT EXISTS ms_billing.account_overage_snapshots ( -- grace-end) or the boundary advance leg ('advance', full pooled overage). source TEXT NOT NULL CHECK (source IN ('grace', 'advance')), - -- The Stripe invoice item id of the overage charge (empty/NULL for a - -- 0-cent rounded charge that recorded nothing) — audit trail only. + -- CRASH-SAFE claim marker (PR #47 review fix — the cross-leg double-charge + -- finding). 'pending' is written BEFORE the leg calls Stripe (the row is the + -- durable "I am about to charge this period's overage" claim); 'charged' is + -- written only AFTER Stripe actually created the invoice item/invoice. The + -- OTHER leg (grace vs boundary) treats ANY row for the period — pending or + -- charged — as claimed and must never independently charge it, closing the + -- crash window where the ORIGINAL code only wrote this row AFTER Stripe + -- succeeded (a crash between the two let the other leg see "no row" and + -- double-charge under a disjoint Idempotency-Key namespace). + status TEXT NOT NULL CHECK (status IN ('pending', 'charged')), + + -- The Stripe invoice item id of the overage charge (empty/NULL while + -- status='pending', or for a 0-cent rounded charge that recorded nothing) + -- — audit trail only. invoice_item_id TEXT NULL, created_at TIMESTAMPTZ NOT NULL DEFAULT now(),