diff --git a/cmd/billing-cycle/main.go b/cmd/billing-cycle/main.go index fc70e75..c79c5fe 100644 --- a/cmd/billing-cycle/main.go +++ b/cmd/billing-cycle/main.go @@ -70,19 +70,39 @@ func main() { } // Local one-shot run: close every card-bound account's just-ended anchored - // period as of now, then exit. No HTTP listener — the dev cycle is a single - // batch invocation. - res := runCycle(context.Background(), svc, time.Now().UTC()) + // period as of now AND sweep the creation-proration grace queue, then exit. + // No HTTP listener — the dev cycle is a single batch invocation. + at := time.Now().UTC() + res := runCycle(context.Background(), svc, at) slog.Info("billing-cycle local run complete", "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) - if res.Failed > 0 { + sweepFailed := runProrationSweep(context.Background(), svc, at) + if res.Failed > 0 || sweepFailed { os.Exit(1) } } +// runProrationSweep charges the creation-period base for every app that has +// survived the grace window as of `at` (the second leg of the cycle job, +// alongside the per-account boundary loop). Reports whether any per-app charge +// failed so the caller can set a non-zero exit code; a failure is retried on the +// next sweep and never aborts the batch. Logged here so both transports share +// the shape. +func runProrationSweep(ctx context.Context, svc *cycle.Service, at time.Time) bool { + sweep, err := svc.SweepCreationProrations(ctx, at) + if err != nil { + slog.ErrorContext(ctx, "creation-proration sweep failed", "as_of", at, "error", err) + return true + } + slog.InfoContext(ctx, "creation-proration sweep complete", + "as_of", at, "pending", sweep.Pending, "charged", sweep.Charged, + "skipped", sweep.Skipped, "failed", sweep.Failed) + return sweep.Failed > 0 +} + // buildService wires the pgxpool + Stripe client into the cycle Service. The // Stripe secret is required (the charge leg cannot run without it). func buildService() *cycle.Service { @@ -108,7 +128,9 @@ func handler(svc *cycle.Service) func(context.Context, events.CloudWatchEvent) e "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) - // A per-account charge failure is recorded (billing_runs status='failed') + // Sweep the creation-proration grace queue as of the same instant. + runProrationSweep(ctx, svc, at.UTC()) + // A per-account charge failure (or a per-app proration failure) is recorded // and does NOT fail the batch — the next cycle retries it. The handler // returns nil so EventBridge doesn't replay the whole batch. return nil diff --git a/internal/account/cycle/apps.go b/internal/account/cycle/apps.go index 6952799..391b149 100644 --- a/internal/account/cycle/apps.go +++ b/internal/account/cycle/apps.go @@ -4,25 +4,21 @@ package cycle // v1, DESIGN.md "Base fee — v1 spec", owner spec 2026-07-05, D1c). Called by // api-platform's applications-service fire-and-forget (with retry) on app // create / module install / uninstall / app delete, so BOTH RPCs are -// idempotent end to end: a retry can re-register or re-sync without a second -// charge or a moved timestamp. +// idempotent end to end: a retry can re-register or re-sync without a moved +// timestamp. // -// They live in the cycle package because RegisterApp's creation-proration -// charge IS a charge-spine leg: it reuses the SAME Stripe invoice plumbing -// (CreateInvoiceItem + CreateInvoice with deterministic Idempotency-Keys), -// the SAME micros→cents boundary (centsFromMicros), and the SAME invoice -// mirror (UpsertInvoice) as RunBillingCycle — never a second Stripe path. +// Neither RPC charges Stripe. The creation-period base is charged by the grace +// sweep (proration.go) once an app survives GraceDays — see ChargeCreationProration +// for the charge leg that reuses the SAME Stripe plumbing, micros→cents boundary, +// and invoice mirror as RunBillingCycle. import ( "context" - "fmt" "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" ) // maxModuleCount bounds the installed-module count BOTH mirror RPCs accept. @@ -53,11 +49,12 @@ type RegisterAppRequest struct { CreatedAt time.Time `json:"created_at,omitempty"` } -// RegisterAppResponse reports the mirror write + what the proration leg did. -// ProrationInvoiceID is "" when no proration invoice exists (unactivated -// account, no usable PM, a creation period that already closed — the boundary -// advance leg owns every later period — or the amount rounded to 0 cents; all -// legitimate no-charge outcomes, D1d). +// RegisterAppResponse reports the mirror write. RegisterApp charges nothing +// (creation grace); the creation-proration invoice is minted later by the sweep +// (proration.go). ProrationInvoiceID is therefore "" for a fresh registration +// and carries the armed guard's invoice id only for a retry that lands AFTER the +// sweep already charged (idempotent visibility). ProrationCents stays 0 here (no +// charge happens in this RPC) — the fields are retained for wire back-compat. type RegisterAppResponse struct { AppID uuid.UUID `json:"app_id"` AccountID uuid.UUID `json:"account_id"` @@ -81,8 +78,12 @@ type SyncAppModulesResponse struct { Deleted bool `json:"deleted"` } -// RegisterApp mirrors a freshly created platform app into ms_billing.apps and -// charges the creation-period proration (owner spec 2026-07-05): +// RegisterApp mirrors a freshly created platform app into ms_billing.apps. It +// charges NOTHING (creation grace, owner spec 2026-07-05, D1e follow-up): a +// newly created app enters a grace window and is charged its creation-period +// base only later, by the sweep (ChargeCreationProration / SweepCreationProrations +// in proration.go), and only if it SURVIVES grace — so an app deleted within +// grace is never billed. RegisterApp: // // 1. resolve-or-create the owner's billing account via the SAME // advisory-locked get-or-create billing.Ensure uses (no Stripe Customer is @@ -91,41 +92,12 @@ type SyncAppModulesResponse struct { // // 2. insert the roster row idempotently (ON CONFLICT (app_id) DO NOTHING — // a retry keeps the FIRST registration's created_at / module_count, the -// stable proration anchor), then read the row back; +// stable proration anchor the sweep later prices from), then read it back. // -// 3. proration leg, gated exactly like the spine (D1d): the account must be -// ACTIVATED (activated_at set — migration 025) AND have a usable default -// 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. -// -// INVARIANT (charge-leg ownership): the proration window is derived from -// the read-back mirror row's created_at (the stable first-registration -// anchor), NEVER from "now" — and if that creation period has already -// ENDED, the proration charge is SKIPPED entirely (charge 0, guard left -// unarmed), deterministically on this and every future retry. Every -// period after the creation period belongs to the boundary advance leg -// (which bills the apps that existed before the period opened), so a -// late retry recomputing against a newer window would double-charge a -// period the boundary already billed — and v1 does no retroactive -// catch-up either way (D1d). -// -// 4. charge via the spine's plumbing with app-scoped deterministic -// Idempotency-Keys (app-ii-/app-inv- — the app id is the stable -// charge identity here, as the run id is for the boundary leg), mirror the -// invoice with the PARTIAL window [creation day, period end), write the -// per-app-period base snapshot (migration 028, source='proration' — the -// frozen display value for the creation period), then arm the one-shot -// guard. -// -// A Stripe/mirror/snapshot failure after the row insert returns an error with -// the guard UNARMED — the platform's retry re-attempts the charge through the -// same idem keys, so the failure mode is "retry-safe", never "double-charge". +// The response echoes the resolved account id and — for a retry that lands after +// the sweep already charged — the armed guard's invoice id (idempotent +// visibility). ProrationCents is never set here (no charge happens in this RPC). +// api-platform fires this fire-and-forget with retry; every step is idempotent. func (s *Service) RegisterApp(ctx context.Context, req RegisterAppRequest) (*RegisterAppResponse, error) { if req.OwnerUserID == uuid.Nil && req.OwnerOrgID == uuid.Nil { return nil, billing.InvalidInput("owner_user_id or owner_org_id required") @@ -163,9 +135,9 @@ func (s *Service) RegisterApp(ctx context.Context, req RegisterAppRequest) (*Reg return nil, billing.Internal("insert app mirror failed", err) } - // Read the row BACK: a retry must prorate from the FIRST registration's - // created_at / module_count (the insert's ON CONFLICT DO NOTHING preserved - // them), and the one-shot guard state decides whether to charge at all. + // Read the row BACK so the response reflects the FIRST registration's stable + // account + guard state (the insert's ON CONFLICT DO NOTHING preserved them): + // a retry that lands after the sweep charged echoes the armed invoice id. app, found, err := s.store.AppMirror(ctx, req.AppID) if err != nil { return nil, billing.Internal("app mirror lookup failed", err) @@ -173,129 +145,11 @@ func (s *Service) RegisterApp(ctx context.Context, req RegisterAppRequest) (*Reg if !found { return nil, billing.Internal("app mirror row missing immediately after insert", nil) } - resp := &RegisterAppResponse{AppID: app.AppID, AccountID: app.AccountID} - - // One-shot guard: a prior attempt already charged (or a concurrent one - // won). Idempotent success — NEVER a second invoice. - if app.ProrationInvoiceID != "" { - resp.ProrationInvoiceID = app.ProrationInvoiceID - return resp, nil - } - - // Activation gate (D1d): unactivated accounts are never charged — the row - // is recorded and the guard stays unarmed (no retroactive catch-up in v1). - activatedAt, activated, err := s.store.AccountActivation(ctx, app.AccountID) - if err != nil { - return nil, billing.Internal("account activation lookup failed", err) - } - if !activated { - return resp, nil - } - hasPM, err := s.store.HasUsableDefaultPM(ctx, app.AccountID) - if err != nil { - return nil, billing.Internal("usable PM check failed", err) - } - if !hasPM { - return resp, nil // same skipped_no_pm posture as the spine — row kept, no invoice - } - - // The creation period is the anchored window CONTAINING the app's - // created_at (ADR 0005 anchor from the activation day) — windowed from the - // read-back mirror row's created_at, the stable first-registration anchor, - // NEVER from now: a retry landing after the boundary must not recompute - // against the NEW period (ProratedBaseMicros would return the FULL base for - // a window the boundary advance leg already billed → double charge, and the - // legitimate creation-period proration would be lost). The proration bills - // the window's remaining whole UTC days, creation day inclusive. - // TODO(plan): plan-resolved base once a plan resolver exists (v1 = default). - periodStart, periodEnd := billingperiod.AnchoredPeriodWindow(app.CreatedAt.UTC(), billingperiod.AnchorDay(activatedAt)) - - // INVARIANT: the proration leg bills ONLY the creation period. If that - // window has already ended, skip the charge entirely — charge 0, guard - // left UNARMED — deterministically on this and every future retry. Every - // later period is owned by the boundary advance leg (it bills the apps - // that existed before the period opened), and v1 does no retroactive - // catch-up (D1d), so a late retry must never invoice anything here. - if !s.nowFn().UTC().Before(periodEnd) { - return resp, nil - } - - prorated := usage.ProratedBaseMicros( - usage.AppBaseFeeMicros(usage.BaseFeeMicros, app.ModuleCount), - app.CreatedAt, periodStart, periodEnd, - ) - cents, err := centsFromMicros(prorated) - if err != nil { - return nil, billing.Internal("micros to cents conversion failed", err) - } - if cents == 0 { - // Rounded to 0 cents — nothing to invoice; the row stands and the - // boundary leg picks the app up from the next period on. - return resp, nil - } - - if s.stripe == nil { - return nil, billing.Internal("RegisterApp proration requires a Stripe client", nil) - } - custID, err := s.store.AccountStripeCustomer(ctx, app.AccountID) - if err != nil { - return nil, billing.Internal("stripe customer lookup failed", err) - } - if custID == "" { - // A usable PM implies a Customer (same anomaly posture as the spine). - return nil, billing.Internal("account has a usable PM but no Stripe customer id", nil) - } - - desc := fmt.Sprintf("MirrorStack app base fee (prorated) — app %s", app.AppID) - if _, err := s.stripe.CreateInvoiceItem(ctx, custID, cents, chargeCurrency, desc, appProrationItemIdemKey(app.AppID)); err != nil { - return nil, billing.StripeError("proration invoice item failed", err) - } - inv, err := s.stripe.CreateInvoice(ctx, custID, true /* autoAdvance */, appProrationInvoiceIdemKey(app.AppID)) - if err != nil { - return nil, billing.StripeError("proration invoice failed", err) - } - - // Mirror with the PARTIAL window: [creation day (UTC midnight), period - // end) — the SAME coverage-start instant ProratedBaseMicros priced, so - // the mirrored window and the charged amount agree by construction. - partialStart := usage.ProrationCoverageStart(app.CreatedAt, periodStart) - if err := s.store.UpsertInvoice(ctx, InvoiceMirror{ - AccountID: app.AccountID, - StripeInvoiceID: inv.ID, - Status: inv.Status, - AmountDueCents: inv.AmountDue, - AmountPaidCents: inv.AmountPaid, - Currency: chargeCurrency, - PeriodStart: partialStart, - PeriodEnd: periodEnd, - }); err != nil { - return nil, billing.Internal("invoice mirror upsert failed", err) - } - - // Freeze what this charge actually billed (migration 028) BEFORE arming the - // guard: the snapshot is the display's authoritative base for the creation - // period, so a later SyncAppModules can never drift the shown base away - // from this invoice. Keyed by the FULL anchored period_start (the display - // identity); BaseMicros is the prorated partial-window amount. Idempotent — - // a retry that failed between here and the guard re-upserts identical - // values through the same Stripe idem keys. - if err := s.store.UpsertProrationBaseSnapshot(ctx, AppBaseSnapshot{ - AppID: app.AppID, - PeriodStart: periodStart, - PeriodEnd: periodEnd, - ModuleCount: app.ModuleCount, - BaseMicros: prorated, - }); err != nil { - return nil, billing.Internal("proration base snapshot upsert failed", err) - } - - if err := s.store.SetAppProrationInvoice(ctx, app.AppID, inv.ID); err != nil { - return nil, billing.Internal("arm proration guard failed", err) - } - - resp.ProrationInvoiceID = inv.ID - resp.ProrationCents = cents - return resp, nil + return &RegisterAppResponse{ + AppID: app.AppID, + AccountID: app.AccountID, + ProrationInvoiceID: app.ProrationInvoiceID, // "" until the sweep charges + }, nil } // SyncAppModules updates an app's roster row: a new installed-module-count @@ -351,12 +205,3 @@ func (s *Service) SyncAppModules(ctx context.Context, req SyncAppModulesRequest) Deleted: app.Deleted, }, nil } - -// appProrationItemIdemKey / appProrationInvoiceIdemKey build the deterministic -// Stripe Idempotency-Keys for the creation-proration charge. The APP id is the -// stable charge identity (each app prorates at most once — the one-shot -// proration_invoice_id guard), exactly as the RUN id is for the boundary leg's -// ii-/inv- keys: a RegisterApp retry reuses the SAME Stripe objects and can -// never double-charge even before the guard is armed. -func appProrationItemIdemKey(appID uuid.UUID) string { return "app-ii-" + appID.String() } -func appProrationInvoiceIdemKey(appID uuid.UUID) string { return "app-inv-" + appID.String() } diff --git a/internal/account/cycle/apps_test.go b/internal/account/cycle/apps_test.go index fb0195c..140940d 100644 --- a/internal/account/cycle/apps_test.go +++ b/internal/account/cycle/apps_test.go @@ -38,9 +38,13 @@ func appsSvc(store *fakeStore, sc *fakeStripe) *cycle.Service { return cycle.NewService(store, sc).WithNow(func() time.Time { return appsNow }) } -// --- RegisterApp: proration happy path -------------------------------------- +// --- RegisterApp: mirror-only (creation grace — RegisterApp never charges) --- -func TestRegisterApp_ChargesCreationProration(t *testing.T) { +func TestRegisterApp_MirrorsRowWithoutCharging(t *testing.T) { + // Creation grace: even a FULLY chargeable account (activated + PM + Stripe + // customer) is NOT charged at registration — RegisterApp only mirrors the + // row. The creation-period base is the sweep's job (see proration_test.go), + // so an app charged before it survives grace is impossible. store := newFakeStore() user, acct := registeredAccount(store) sc := newFakeStripe() @@ -49,97 +53,68 @@ func TestRegisterApp_ChargesCreationProration(t *testing.T) { resp, err := appsSvc(store, sc).RegisterApp(context.Background(), cycle.RegisterAppRequest{ OwnerUserID: user, AppID: appID, - CreatedAt: time.Date(2026, 7, 1, 8, 0, 0, 0, time.UTC), // the 1st → days 1–3 of [Jun 4, Jul 4) + ModuleCount: 3, + CreatedAt: time.Date(2026, 7, 1, 8, 0, 0, 0, time.UTC), }) require.NoError(t, err) require.Equal(t, acct, resp.AccountID) - // 20e6 × 3/30 = 2_000_000 micros → 200 cents, one item + one invoice with - // the app-scoped deterministic idem keys. - require.EqualValues(t, 200, resp.ProrationCents) - require.Len(t, sc.itemCalls, 1) - require.Len(t, sc.invoiceCalls, 1) - require.EqualValues(t, 200, sc.itemCalls[0].amountCfg) - require.Equal(t, "cus_apps_1", sc.itemCalls[0].custID) - require.Equal(t, "app-ii-"+appID.String(), sc.itemCalls[0].idemKey) - require.Equal(t, "app-inv-"+appID.String(), sc.invoiceCalls[0].idemKey) - require.True(t, sc.invoiceCalls[0].autoAdvance) - - // Mirrored with the PARTIAL window [creation day, period end) and the - // one-shot guard armed with the invoice id. - require.Equal(t, resp.ProrationInvoiceID, sc.invoiceID) - mirror := store.invoices[sc.invoiceID] - require.Equal(t, acct, mirror.AccountID) - require.Equal(t, time.Date(2026, 7, 1, 0, 0, 0, 0, time.UTC), mirror.PeriodStart) - require.Equal(t, time.Date(2026, 7, 4, 0, 0, 0, 0, time.UTC), mirror.PeriodEnd) - require.Equal(t, sc.invoiceID, store.apps[appID].ProrationInvoiceID) - - // And the migration-028 snapshot froze EXACTLY what was billed, keyed by - // the FULL anchored period start (the display identity), with the - // prorated partial-window amount. - snap, ok := store.baseSnapshots[snapKey{appID, time.Date(2026, 6, 4, 0, 0, 0, 0, time.UTC)}] - require.True(t, ok, "the proration leg must freeze its charged base") - require.Equal(t, "proration", snap.source) - require.EqualValues(t, 2_000_000, snap.snap.BaseMicros) - require.Equal(t, time.Date(2026, 7, 4, 0, 0, 0, 0, time.UTC), snap.snap.PeriodEnd) - 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. - store := newFakeStore() - user, _ := registeredAccount(store) - sc := newFakeStripe() + // NO charge of any kind, guard unarmed, no snapshot, no invoice. + require.Empty(t, resp.ProrationInvoiceID) + require.Zero(t, resp.ProrationCents) + require.Empty(t, sc.itemCalls) + require.Empty(t, sc.invoiceCalls) + require.Empty(t, store.invoices) + require.Empty(t, store.baseSnapshots) - resp, err := appsSvc(store, sc).RegisterApp(context.Background(), cycle.RegisterAppRequest{ - OwnerUserID: user, - AppID: uuid.New(), - ModuleCount: 7, - CreatedAt: time.Date(2026, 6, 19, 12, 0, 0, 0, time.UTC), - }) - require.NoError(t, err) - require.EqualValues(t, 1300, resp.ProrationCents) + // But the roster row IS recorded verbatim (created_at / module_count / account) + // — the stable anchor the sweep later prices from. + app := store.apps[appID] + require.Equal(t, acct, app.AccountID) + require.Equal(t, 3, app.ModuleCount) + require.Equal(t, time.Date(2026, 7, 1, 8, 0, 0, 0, time.UTC), app.CreatedAt) + require.Empty(t, app.ProrationInvoiceID) } func TestRegisterApp_DefaultsCreatedAtToNow(t *testing.T) { - // Zero CreatedAt → the server's now (appsNow, Jul 1) → the same 3-day - // proration as the explicit-timestamp case. + // Zero CreatedAt → the server's now (appsNow, Jul 1) is stamped on the mirror + // row (the anchor the later sweep prorates from). Still no charge here. store := newFakeStore() user, _ := registeredAccount(store) sc := newFakeStripe() + appID := uuid.New() resp, err := appsSvc(store, sc).RegisterApp(context.Background(), cycle.RegisterAppRequest{ - OwnerUserID: user, AppID: uuid.New(), + OwnerUserID: user, AppID: appID, }) require.NoError(t, err) - require.EqualValues(t, 200, resp.ProrationCents) + require.Empty(t, resp.ProrationInvoiceID) + require.Empty(t, sc.itemCalls) + require.Equal(t, appsNow, store.apps[appID].CreatedAt) } -// --- RegisterApp: idempotency ------------------------------------------------ - -func TestRegisterApp_RetryNeverChargesTwice(t *testing.T) { +func TestRegisterApp_EchoesArmedGuardOnRetry(t *testing.T) { + // A RegisterApp retry that lands AFTER the sweep already charged echoes the + // armed guard's invoice id (idempotent visibility) and still never charges. store := newFakeStore() user, _ := registeredAccount(store) sc := newFakeStripe() svc := appsSvc(store, sc) - req := cycle.RegisterAppRequest{ - OwnerUserID: user, - AppID: uuid.New(), - CreatedAt: time.Date(2026, 7, 1, 8, 0, 0, 0, time.UTC), - } + appID := uuid.New() + req := cycle.RegisterAppRequest{OwnerUserID: user, AppID: appID, CreatedAt: time.Date(2026, 7, 1, 8, 0, 0, 0, time.UTC)} - first, err := svc.RegisterApp(context.Background(), req) + _, err := svc.RegisterApp(context.Background(), req) require.NoError(t, err) - second, err := svc.RegisterApp(context.Background(), req) + // The sweep charges it (grace elapsed). + _, err = svc.SweepCreationProrations(context.Background(), time.Date(2026, 7, 5, 0, 0, 0, 0, time.UTC)) require.NoError(t, err) + armed := store.apps[appID].ProrationInvoiceID + require.NotEmpty(t, armed) - // The one-shot guard short-circuits the retry: SAME invoice id back, ONE - // Stripe item + invoice total, ONE roster row. - require.Equal(t, first.ProrationInvoiceID, second.ProrationInvoiceID) - require.Len(t, sc.itemCalls, 1, "a retry must never create a second charge") - require.Len(t, sc.invoiceCalls, 1) - require.Len(t, store.apps, 1) + resp, err := svc.RegisterApp(context.Background(), req) + require.NoError(t, err) + require.Equal(t, armed, resp.ProrationInvoiceID) + require.Len(t, sc.itemCalls, 1, "the retry must not add a second charge") } func TestRegisterApp_RetryKeepsFirstRegistrationsAnchor(t *testing.T) { @@ -165,32 +140,14 @@ func TestRegisterApp_RetryKeepsFirstRegistrationsAnchor(t *testing.T) { require.Equal(t, created, store.apps[appID].CreatedAt) } -// --- RegisterApp: activation gate (D1d) --------------------------------------- - -func TestRegisterApp_UnactivatedAccountRowButNoInvoice(t *testing.T) { +func TestRegisterApp_RecordsRowRegardlessOfAccountState(t *testing.T) { + // RegisterApp no longer gates on activation / PM (that moved to the charge + // sweep). It records the roster row unconditionally — even for an + // unactivated account with no PM — so the sweep can price it once the + // account becomes chargeable. No charge, ever, in this RPC. store := newFakeStore() user, acct := registeredAccount(store) delete(store.activation, acct) // never bound a card - sc := newFakeStripe() - appID := uuid.New() - - resp, err := appsSvc(store, sc).RegisterApp(context.Background(), cycle.RegisterAppRequest{ - OwnerUserID: user, AppID: appID, - }) - require.NoError(t, err) - - // The mirror row IS recorded (the boundary leg needs it once the account - // activates) but NO invoice exists and the guard stays unarmed. - require.Contains(t, store.apps, appID) - require.Empty(t, resp.ProrationInvoiceID) - require.Empty(t, sc.itemCalls) - require.Empty(t, sc.invoiceCalls) - require.Empty(t, store.apps[appID].ProrationInvoiceID) -} - -func TestRegisterApp_NoUsablePMRowButNoInvoice(t *testing.T) { - store := newFakeStore() - user, _ := registeredAccount(store) store.hasPM = false sc := newFakeStripe() appID := uuid.New() @@ -202,99 +159,7 @@ func TestRegisterApp_NoUsablePMRowButNoInvoice(t *testing.T) { require.Contains(t, store.apps, appID) require.Empty(t, resp.ProrationInvoiceID) require.Empty(t, sc.itemCalls) -} - -func TestRegisterApp_CreatedOnBoundaryChargesNewPeriodFullBase(t *testing.T) { - // Created exactly ON the anchor boundary instant (Jul 4 00:00): half-open - // windows put the creation in the NEW period [Jul 4, Aug 4), so the - // proration leg charges the FULL new-period base ($20 → 2000 cents). This - // is the fix direction of the charge-leg overlap review: the Jul-4 - // boundary's advance leg EXCLUDES apps with created_at ≥ Jul 4, so if - // RegisterApp skipped here the app's first period would never be - // base-charged (a revenue leak); charging it here bills it exactly once. - // (Pre-fix this recomputed 0 remaining days against the OLD window and - // charged nothing, leaving the period to the advance leg's roster scan.) - store := newFakeStore() - user, _ := registeredAccount(store) - sc := newFakeStripe() - appID := uuid.New() - - resp, err := appsSvc(store, sc).RegisterApp(context.Background(), cycle.RegisterAppRequest{ - OwnerUserID: user, - AppID: appID, - CreatedAt: time.Date(2026, 7, 4, 0, 0, 0, 0, time.UTC), - }) - require.NoError(t, err) - require.EqualValues(t, 2_000, resp.ProrationCents, "creation-day == period start → full base") - require.Len(t, sc.itemCalls, 1) - - // Mirrored with the FULL new window (partial start == period start) and - // the snapshot frozen for [Jul 4, Aug 4). - mirror := store.invoices[sc.invoiceID] - require.Equal(t, time.Date(2026, 7, 4, 0, 0, 0, 0, time.UTC), mirror.PeriodStart) - require.Equal(t, time.Date(2026, 8, 4, 0, 0, 0, 0, time.UTC), mirror.PeriodEnd) - snap, ok := store.baseSnapshots[snapKey{appID, time.Date(2026, 7, 4, 0, 0, 0, 0, time.UTC)}] - require.True(t, ok, "the proration leg must freeze the charged base") - require.Equal(t, "proration", snap.source) - require.EqualValues(t, usage.BaseFeeMicros, snap.snap.BaseMicros) -} - -func TestRegisterApp_RetryAfterCreationPeriodClosedNeverCharges(t *testing.T) { - // FINDING-1 pin: a RegisterApp retry that lands AFTER the creation period - // closed must NOT charge — the proration window is derived from the mirror - // row's created_at (May 20 → [May 4, Jun 4)), that window ended before now - // (Jul 1), and every later period belongs to the boundary advance leg - // (which already billed [Jun 4, Jul 4) for pre-existing apps). Pre-fix the - // window was recomputed from NOW, ProratedBaseMicros saw creation-day ≤ - // period start, and the retry double-charged the FULL current-period base. - store := newFakeStore() - user, _ := registeredAccount(store) - sc := newFakeStripe() - svc := appsSvc(store, sc) - appID := uuid.New() - req := cycle.RegisterAppRequest{ - OwnerUserID: user, - AppID: appID, - CreatedAt: time.Date(2026, 5, 20, 9, 0, 0, 0, time.UTC), // period [May 4, Jun 4) — long closed at appsNow (Jul 1) - } - - resp, err := svc.RegisterApp(context.Background(), req) - require.NoError(t, err) - - // Row recorded, NO charge, guard UNARMED, no snapshot. - require.Contains(t, store.apps, appID) - require.Empty(t, resp.ProrationInvoiceID) - require.Empty(t, sc.itemCalls, "a closed creation period must never be prorated late") require.Empty(t, sc.invoiceCalls) - require.Empty(t, store.apps[appID].ProrationInvoiceID, "guard stays unarmed") - require.Empty(t, store.baseSnapshots) - - // Deterministic: every further retry skips identically (never "eventually - // charges"). - resp, err = svc.RegisterApp(context.Background(), req) - require.NoError(t, err) - require.Empty(t, resp.ProrationInvoiceID) - require.Empty(t, sc.itemCalls) - require.Empty(t, store.apps[appID].ProrationInvoiceID) -} - -func TestRegisterApp_CurrentPeriodProrationUnchangedByWindowFix(t *testing.T) { - // FINDING-1 regression guard: for an app created in a period that is - // still CURRENT, deriving the window from created_at instead of now must - // change NOTHING — same window, same 3-day proration, snapshot frozen at - // the anchored period start with the prorated amount. - store := newFakeStore() - user, _ := registeredAccount(store) - sc := newFakeStripe() - - resp, err := appsSvc(store, sc).RegisterApp(context.Background(), cycle.RegisterAppRequest{ - OwnerUserID: user, - AppID: uuid.New(), - CreatedAt: time.Date(2026, 7, 1, 8, 0, 0, 0, time.UTC), // inside the current [Jun 4, Jul 4) - }) - require.NoError(t, err) - require.EqualValues(t, 200, resp.ProrationCents, "3 of 30 days of $20 → 200 cents, exactly as before the fix") - require.Len(t, sc.itemCalls, 1) } // --- RegisterApp: account resolution + validation ----------------------------- @@ -607,16 +472,19 @@ func TestRunBillingCycle_ReclaimedRunNoDoubleBase(t *testing.T) { require.Equal(t, cycle.RunStatusSkippedNoPM, first.Status) require.Empty(t, sc.invoiceCalls) - // PM bound; an app is created Jul 2 (inside the new period [Jul 1, Aug 1)) - // and RegisterApp prorates it: 30 of 31 days of $20 → 19_354_839 micros. + // PM bound; an app is created Jul 2 (inside the new period [Jul 1, Aug 1)). + // RegisterApp only mirrors it; the creation-proration charge (the leg the + // grace sweep drives) prorates 30 of 31 days of $20 → 19_354_839 micros. store.hasPM = true newApp := uuid.New() - reg, err := svc.RegisterApp(context.Background(), cycle.RegisterAppRequest{ + _, err = svc.RegisterApp(context.Background(), cycle.RegisterAppRequest{ OwnerUserID: user, AppID: newApp, CreatedAt: time.Date(2026, 7, 2, 10, 0, 0, 0, time.UTC), }) require.NoError(t, err) + reg, err := svc.ChargeCreationProration(context.Background(), newApp) + require.NoError(t, err) require.EqualValues(t, 1_935, reg.ProrationCents) require.Len(t, sc.invoiceCalls, 1) diff --git a/internal/account/cycle/migration029_integration_test.go b/internal/account/cycle/migration029_integration_test.go new file mode 100644 index 0000000..119188b --- /dev/null +++ b/internal/account/cycle/migration029_integration_test.go @@ -0,0 +1,127 @@ +//go:build integration + +package cycle_test + +import ( + "context" + "testing" + + "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/shared/testutil" +) + +// Migration 029 (apps_pending_proration_idx) — verifies the SQL-level semantics +// the unit fakes only model for the creation-grace sweep: the AppsPendingProration +// work-list filter (past grace AND live AND unarmed) and the ChargeProrationLocked +// FOR UPDATE section (deleted / already-armed short-circuits, and the atomic +// charge → mirror → snapshot → arm on a live unarmed app). + +func TestAppsPendingProration_Integration_Filter(t *testing.T) { + pool := testutil.NewTestDB(t) + store := cycle.NewStore(pool) + ctx := context.Background() + + acct := seedAccount(t, pool) + cutoff := mustTime(t, "2026-06-27T00:00:00Z") // now − GraceDays for a sweep on Jun 30 + + pending := uuid.New() // created before cutoff, live, unarmed → returned + young := uuid.New() // created after cutoff (within grace) → excluded + deleted := uuid.New() // before cutoff but soft-deleted → excluded + charged := uuid.New() // before cutoff but guard armed → excluded + require.NoError(t, store.InsertAppMirror(ctx, pending, acct, 0, mustTime(t, "2026-06-20T08:00:00Z"))) + require.NoError(t, store.InsertAppMirror(ctx, young, acct, 0, mustTime(t, "2026-06-29T08:00:00Z"))) + require.NoError(t, store.InsertAppMirror(ctx, deleted, acct, 0, mustTime(t, "2026-06-20T08:00:00Z"))) + require.NoError(t, store.InsertAppMirror(ctx, charged, acct, 0, mustTime(t, "2026-06-20T08:00:00Z"))) + require.NoError(t, store.MarkAppDeleted(ctx, deleted)) + require.NoError(t, store.SetAppProrationInvoice(ctx, charged, "in_already")) + + ids, err := store.AppsPendingProration(ctx, cutoff) + require.NoError(t, err) + require.Equal(t, []uuid.UUID{pending}, ids, "only the past-grace, live, unarmed app is pending") +} + +func TestChargeProrationLocked_Integration_Semantics(t *testing.T) { + pool := testutil.NewTestDB(t) + store := cycle.NewStore(pool) + ctx := context.Background() + + acct := seedAccount(t, pool) + periodStart := mustTime(t, "2026-06-04T00:00:00Z") + periodEnd := mustTime(t, "2026-07-04T00:00:00Z") + + // mkCharge builds the payload the service's charge callback would return. + mkCharge := func(appID uuid.UUID, invID string) *cycle.ProrationCharge { + return &cycle.ProrationCharge{ + InvoiceID: invID, + Cents: 200, + Invoice: cycle.InvoiceMirror{ + AccountID: acct, StripeInvoiceID: invID, Status: "open", + AmountDueCents: 200, Currency: "usd", + PeriodStart: mustTime(t, "2026-07-01T00:00:00Z"), PeriodEnd: periodEnd, + }, + Snapshot: cycle.AppBaseSnapshot{ + AppID: appID, PeriodStart: periodStart, PeriodEnd: periodEnd, + ModuleCount: 0, BaseMicros: 2_000_000, + }, + } + } + + // Live, unarmed app → the callback fires, and the invoice + snapshot + guard + // commit atomically. + live := uuid.New() + require.NoError(t, store.InsertAppMirror(ctx, live, acct, 0, mustTime(t, "2026-07-01T08:00:00Z"))) + called := false + outcome, invID, err := store.ChargeProrationLocked(ctx, live, func(l cycle.AppMirror) (*cycle.ProrationCharge, error) { + called = true + require.Equal(t, live, l.AppID) + return mkCharge(live, "in_live"), nil + }) + require.NoError(t, err) + require.True(t, called) + require.Equal(t, cycle.ProrationLockedCharged, outcome) + require.Equal(t, "in_live", invID) + app, _, err := store.AppMirror(ctx, live) + require.NoError(t, err) + require.Equal(t, "in_live", app.ProrationInvoiceID, "the guard is armed under the lock") + + // Re-run on the now-armed app → AlreadyCharged, callback NOT invoked, no + // second invoice. + called = false + outcome, invID, err = store.ChargeProrationLocked(ctx, live, func(cycle.AppMirror) (*cycle.ProrationCharge, error) { + called = true + return mkCharge(live, "in_second"), nil + }) + require.NoError(t, err) + require.False(t, called, "an armed guard short-circuits before the Stripe charge") + require.Equal(t, cycle.ProrationLockedAlreadyCharged, outcome) + require.Equal(t, "in_live", invID) + + // Deleted app → Deleted, callback NOT invoked (a within-grace delete is never + // charged, and a delete that won the race no-ops the charge). + deleted := uuid.New() + require.NoError(t, store.InsertAppMirror(ctx, deleted, acct, 0, mustTime(t, "2026-07-01T08:00:00Z"))) + require.NoError(t, store.MarkAppDeleted(ctx, deleted)) + called = false + outcome, _, err = store.ChargeProrationLocked(ctx, deleted, func(cycle.AppMirror) (*cycle.ProrationCharge, error) { + called = true + return mkCharge(deleted, "in_deleted"), nil + }) + require.NoError(t, err) + require.False(t, called, "a deleted app is never charged") + require.Equal(t, cycle.ProrationLockedDeleted, outcome) + + // Callback declines (0 cents) → NoCharge, guard stays unarmed, nothing persisted. + zero := uuid.New() + require.NoError(t, store.InsertAppMirror(ctx, zero, acct, 0, mustTime(t, "2026-07-01T08:00:00Z"))) + outcome, _, err = store.ChargeProrationLocked(ctx, zero, func(cycle.AppMirror) (*cycle.ProrationCharge, error) { + return nil, nil + }) + require.NoError(t, err) + require.Equal(t, cycle.ProrationLockedNoCharge, outcome) + app, _, err = store.AppMirror(ctx, zero) + require.NoError(t, err) + require.Empty(t, app.ProrationInvoiceID, "a declined charge leaves the guard unarmed") +} diff --git a/internal/account/cycle/migration030_integration_test.go b/internal/account/cycle/migration030_integration_test.go new file mode 100644 index 0000000..803ac27 --- /dev/null +++ b/internal/account/cycle/migration030_integration_test.go @@ -0,0 +1,74 @@ +//go:build integration + +package cycle_test + +import ( + "context" + "testing" + + "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/shared/testutil" +) + +// Migration 030 (apps.created_module_count) — verifies at the SQL layer (the +// unit fakes only model this in Go) that the frozen creation-time module count +// survives a live module_count write untouched: InsertAppMirror stamps BOTH +// columns from the same value, and SetAppModuleCount (SyncAppModules' writer) +// touches ONLY module_count. + +func TestCreatedModuleCount_Integration_FrozenAcrossSetAppModuleCount(t *testing.T) { + pool := testutil.NewTestDB(t) + store := cycle.NewStore(pool) + ctx := context.Background() + + acct := seedAccount(t, pool) + appID := uuid.New() + require.NoError(t, store.InsertAppMirror(ctx, appID, acct, 2, mustTime(t, "2026-07-01T08:00:00Z"))) + + app, found, err := store.AppMirror(ctx, appID) + require.NoError(t, err) + require.True(t, found) + require.Equal(t, 2, app.ModuleCount) + require.Equal(t, 2, app.CreatedModuleCount, "created_module_count starts equal to the registered count") + + // SyncAppModules' writer (module install/uninstall) — must move ONLY the + // live column. + require.NoError(t, store.SetAppModuleCount(ctx, appID, 9)) + + app, found, err = store.AppMirror(ctx, appID) + require.NoError(t, err) + require.True(t, found) + require.Equal(t, 9, app.ModuleCount, "the live count moved") + require.Equal(t, 2, app.CreatedModuleCount, "the frozen count must NOT move — no query writes it after insert") + + // A second live-count write (another install/uninstall cycle) still never + // touches the frozen column. + require.NoError(t, store.SetAppModuleCount(ctx, appID, 0)) + app, _, err = store.AppMirror(ctx, appID) + require.NoError(t, err) + require.Equal(t, 0, app.ModuleCount) + require.Equal(t, 2, app.CreatedModuleCount) +} + +func TestCreatedModuleCount_Integration_RetryKeepsFirstRegistrationsFrozenCount(t *testing.T) { + // InsertAppMirror's ON CONFLICT (app_id) DO NOTHING means a RegisterApp + // retry with a DIFFERENT module_count must not move either column — the + // FIRST registration's frozen count is the stable proration anchor. + pool := testutil.NewTestDB(t) + store := cycle.NewStore(pool) + ctx := context.Background() + + acct := seedAccount(t, pool) + appID := uuid.New() + created := mustTime(t, "2026-07-01T08:00:00Z") + require.NoError(t, store.InsertAppMirror(ctx, appID, acct, 3, created)) + require.NoError(t, store.InsertAppMirror(ctx, appID, acct, 12, created)) // retry, different count + + app, _, err := store.AppMirror(ctx, appID) + require.NoError(t, err) + require.Equal(t, 3, app.ModuleCount) + require.Equal(t, 3, app.CreatedModuleCount) +} diff --git a/internal/account/cycle/migration031_integration_test.go b/internal/account/cycle/migration031_integration_test.go new file mode 100644 index 0000000..d957940 --- /dev/null +++ b/internal/account/cycle/migration031_integration_test.go @@ -0,0 +1,78 @@ +//go:build integration + +package cycle_test + +import ( + "context" + "testing" + + "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/shared/testutil" +) + +// Migration 031 (apps.proration_skipped_at) — verifies at the SQL layer that +// the permanent-skip marker (D1d, no retroactive catch-up) is a one-shot, +// first-write-wins guard, and that AppsPendingProration's work-list filter +// excludes a permanently-skipped app forever (the same partial-index +// predicate the migration re-defines). + +func TestProrationSkipped_Integration_OneShotAndExcludedFromPending(t *testing.T) { + pool := testutil.NewTestDB(t) + store := cycle.NewStore(pool) + ctx := context.Background() + + acct := seedAccount(t, pool) + cutoff := mustTime(t, "2026-04-05T00:00:00Z") + + skipped := uuid.New() + require.NoError(t, store.InsertAppMirror(ctx, skipped, acct, 0, mustTime(t, "2026-01-01T08:00:00Z"))) + + // Past grace, unarmed, not yet skipped → pending. + ids, err := store.AppsPendingProration(ctx, cutoff) + require.NoError(t, err) + require.Equal(t, []uuid.UUID{skipped}, ids) + + // Mark permanently skipped (what ChargeCreationProration does once it + // determines the account activated after this app's period had closed). + require.NoError(t, store.SetAppProrationSkipped(ctx, skipped)) + + // Excluded from the pending work list from now on — a later sweep must + // never resurface it (it would otherwise sit pending forever, since + // proration_invoice_id stays NULL for a skipped charge). + ids, err = store.AppsPendingProration(ctx, cutoff) + require.NoError(t, err) + require.Empty(t, ids, "a permanently-skipped app is excluded from every future sweep") + + // One-shot: a second SetAppProrationSkipped call is a harmless no-op — it + // does not need to be idempotent-with-error, just idempotent-with-effect. + require.NoError(t, store.SetAppProrationSkipped(ctx, skipped)) + + app, _, err := store.AppMirror(ctx, skipped) + require.NoError(t, err) + require.True(t, app.ProrationSkipped) + require.Empty(t, app.ProrationInvoiceID, "skipped, never charged") +} + +func TestProrationSkipped_Integration_RefusesToSkipAnAlreadyChargedApp(t *testing.T) { + // Defensive belt-and-suspenders: SetAppProrationSkipped's WHERE clause also + // requires proration_invoice_id IS NULL, so it can never clobber a genuine + // charge that raced in first. + pool := testutil.NewTestDB(t) + store := cycle.NewStore(pool) + ctx := context.Background() + + acct := seedAccount(t, pool) + appID := uuid.New() + require.NoError(t, store.InsertAppMirror(ctx, appID, acct, 0, mustTime(t, "2026-01-01T08:00:00Z"))) + require.NoError(t, store.SetAppProrationInvoice(ctx, appID, "in_already_charged")) + + require.NoError(t, store.SetAppProrationSkipped(ctx, appID)) // no-op, not an error + + app, _, err := store.AppMirror(ctx, appID) + require.NoError(t, err) + require.False(t, app.ProrationSkipped, "an already-charged app must never be marked skipped") + require.Equal(t, "in_already_charged", app.ProrationInvoiceID) +} diff --git a/internal/account/cycle/proration.go b/internal/account/cycle/proration.go new file mode 100644 index 0000000..b18b00c --- /dev/null +++ b/internal/account/cycle/proration.go @@ -0,0 +1,382 @@ +package cycle + +// Creation-proration charge + sweep (creation grace, owner spec 2026-07-05, +// D1e follow-up). RegisterApp used to charge an app's creation-period base +// synchronously at creation; it no longer does (see apps.go). Instead a newly +// created app enters a GRACE window and is charged only once it has SURVIVED it: +// +// - RegisterApp mirrors the roster row (created_at, account, module_count) and +// charges NOTHING; +// - a periodic sweep (SweepCreationProrations, driven by cmd/billing-cycle) +// finds apps past grace (created_at <= now − GraceDays) that are still LIVE +// (deleted_at IS NULL) and NOT yet charged (proration_invoice_id IS NULL), +// and charges each the SAME creation-period proration RegisterApp used to — +// identical ProratedBaseMicros math, anchored to the TRUE created_at, so the +// app pays only for the days it actually existed. Grace delays WHEN the +// charge fires, never WHAT it covers. +// +// An app soft-deleted within grace is thus NEVER charged (the sweep excludes +// deleted rows), and the charge is race-safe against a concurrent delete via a +// brief FOR UPDATE row lock that is released BEFORE the Stripe call (see +// ChargeProrationLocked in store.go). +// +// D1d — no retroactive catch-up: an app whose account never activated (or had +// no usable PM) sits pending on every sweep. If the account only becomes +// chargeable AFTER the app's anchored creation period has already closed, +// charging it then would be exactly the retroactive catch-up D1d forbids — +// ChargeCreationProration detects this (activatedAt at/after the period's end) +// and PERMANENTLY skips the charge (proration_skipped_at, migration 031) +// rather than charging it or leaving it pending forever. +// +// module_count is a LIVE snapshot SyncAppModules can move at any time, +// including during grace. The creation-proration charge must never price its +// historical window off whatever module_count happens to read at sweep time — +// it prices off created_module_count (migration 030), frozen once at +// RegisterApp and never touched again. + +import ( + "context" + "fmt" + "log/slog" + "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" +) + +// ProrationStatus classifies one ChargeCreationProration outcome for the sweep's +// tally + per-app log line. Only ProrationStatusCharged mints a new invoice; the +// rest are legitimate no-charge outcomes (D1d/D1e) or the idempotent guard. +type ProrationStatus string + +const ( + // ProrationStatusCharged: the creation-proration invoice was created and the + // one-shot guard armed on THIS call. + ProrationStatusCharged ProrationStatus = "charged" + // ProrationStatusAlreadyCharged: the guard was already armed (a prior sweep, + // or a concurrent one, charged) — idempotent success, no second invoice. + ProrationStatusAlreadyCharged ProrationStatus = "already_charged" + // ProrationStatusDeleted: the app is soft-deleted (within grace, or a delete + // that won the race under the lock) → never charged (D1e, no refunds). + ProrationStatusDeleted ProrationStatus = "skipped_deleted" + // ProrationStatusUnactivated: the owner account never bound a card → never + // charged (D1d, no retroactive catch-up on later activation in v1). + ProrationStatusUnactivated ProrationStatus = "skipped_unactivated" + // ProrationStatusNoPM: activated but no usable default PM → skipped, same + // posture as the boundary spine; re-attempted on the next sweep. + ProrationStatusNoPM ProrationStatus = "skipped_no_pm" + // ProrationStatusNoCharge: the proration rounded to 0 cents (effectively + // unreachable for a real survived app whose base is ≥ $20) → nothing to + // invoice, guard left unarmed. + ProrationStatusNoCharge ProrationStatus = "no_charge" + // ProrationStatusNotFound: no roster row for the app id (never registered). + ProrationStatusNotFound ProrationStatus = "not_found" + // ProrationStatusPeriodClosed: the account only activated at/after the + // app's anchored creation period had already closed — charging it now + // would be a retroactive catch-up (D1d). PERMANENTLY skipped: the + // proration_skipped_at marker is armed so the app never resurfaces on a + // later sweep. + ProrationStatusPeriodClosed ProrationStatus = "skipped_period_closed" +) + +// ProrationResult reports what ChargeCreationProration did. ProrationInvoiceID is +// set on ProrationStatusCharged (the new invoice) and ProrationStatusAlreadyCharged +// (the pre-existing one); ProrationCents only on a fresh charge. +type ProrationResult struct { + AppID uuid.UUID + Status ProrationStatus + ProrationInvoiceID string + ProrationCents int64 +} + +// ProrationOutcome is the store's report from the locked charge section +// (ChargeProrationLocked), decided UNDER the row lock where the terminal +// deleted/guard state is authoritative. +type ProrationOutcome int + +const ( + // ProrationLockedNotFound: the row vanished between the sweep's read and the + // lock (unregistered / cascade-deleted). + ProrationLockedNotFound ProrationOutcome = iota + // ProrationLockedDeleted: deleted_at is set under the lock — a delete won. + ProrationLockedDeleted + // ProrationLockedAlreadyCharged: proration_invoice_id is set under the lock. + ProrationLockedAlreadyCharged + // ProrationLockedNoCharge: the charge callback declined (0 cents) — nothing + // persisted, guard unarmed. + ProrationLockedNoCharge + // ProrationLockedCharged: the charge fired, was mirrored + snapshotted, and + // the guard armed, all committed atomically. + ProrationLockedCharged +) + +// ProrationCharge is the persistence payload the charge callback returns from +// inside the locked transaction: the created Stripe invoice to mirror, the base +// snapshot to freeze (migration 028, source='proration'), and the invoice id +// that arms the one-shot guard. Cents is echoed to the caller. A nil return from +// the callback means "nothing to charge" (0 cents) — the store persists nothing. +type ProrationCharge struct { + InvoiceID string + Cents int64 + Invoice InvoiceMirror + Snapshot AppBaseSnapshot +} + +// ChargeCreationProration charges (once) the creation-period base proration for +// ONE app that has survived the grace window — the shared charge leg the sweep +// invokes per pending app. It is idempotent (the one-shot proration_invoice_id +// guard) and race-safe against a concurrent soft-delete (the FOR UPDATE section). +// +// The amount is the SAME as the pre-grace RegisterApp charge: +// +// ProratedBaseMicros(AppBaseFeeMicros(BaseFeeMicros, created_module_count), +// created_at, the anchored period CONTAINING created_at) +// +// anchored to the TRUE created_at (NOT now), so the app pays only for the whole +// UTC days it existed in its creation period, creation day inclusive — grace +// only delayed WHEN this fires. created_module_count (migration 030) is the +// module count FROZEN at RegisterApp time: SyncAppModules can move the LIVE +// module_count at any point during (or after) grace, but the historical +// creation-period days billed here must price at the tier that actually +// applied on those days, never whatever count happens to read at sweep time. +// +// It IS gated on whether the account only became chargeable after the +// creation period had already closed (D1d): that would be a retroactive +// catch-up charge for time the account was never eligible to be billed for, +// and is PERMANENTLY skipped rather than charged (see the period-closed check +// below). Short of that, the creation period is billed by NO other leg (the +// boundary advance leg only ever bills an app's SUBSEQUENT periods, never the +// one containing its creation), so charging it whenever the guard is unarmed +// and the period-closed check passes is correct and can never double-bill. +// +// Cheap gates that don't need the row lock (unregistered / already-charged / +// deleted / unactivated / period-closed / no-PM) short-circuit first; the +// actual charge + arm runs under the lock (ChargeProrationLocked), which +// re-verifies the deleted + guard state authoritatively. +func (s *Service) ChargeCreationProration(ctx context.Context, appID uuid.UUID) (*ProrationResult, error) { + if appID == uuid.Nil { + return nil, billing.InvalidInput("app_id required") + } + + app, found, err := s.store.AppMirror(ctx, appID) + if err != nil { + return nil, billing.Internal("app mirror lookup failed", err) + } + if !found { + return &ProrationResult{AppID: appID, Status: ProrationStatusNotFound}, nil + } + // Idempotent short-circuit: a prior (or concurrent) sweep already charged. + if app.ProrationInvoiceID != "" { + return &ProrationResult{AppID: appID, Status: ProrationStatusAlreadyCharged, ProrationInvoiceID: app.ProrationInvoiceID}, nil + } + // Permanently skipped on a prior sweep (D1d retroactive-catch-up guard, + // migration 031) — never re-evaluated. + if app.ProrationSkipped { + return &ProrationResult{AppID: appID, Status: ProrationStatusPeriodClosed}, nil + } + // Deleted within grace (or after) → never charged (D1e). The locked section + // re-checks this authoritatively; this is the cheap early-out. + if app.Deleted { + return &ProrationResult{AppID: appID, Status: ProrationStatusDeleted}, nil + } + + // Activation gate (D1d), same posture as the boundary spine: an + // unactivated account (never bound a card) is never charged. + activatedAt, activated, err := s.store.AccountActivation(ctx, app.AccountID) + if err != nil { + return nil, billing.Internal("account activation lookup failed", err) + } + if !activated { + return &ProrationResult{AppID: appID, Status: ProrationStatusUnactivated}, nil + } + + // D1d — no retroactive catch-up: derive the anchored period CONTAINING the + // app's created_at from the account's (now-known) activation anchor. If the + // account only activated AT OR AFTER that period's end, the account was + // unactivated — and therefore never chargeable — for the app's ENTIRE + // creation period; charging it now, however late the sweep runs, would be + // exactly the retroactive catch-up D1d forbids. Permanently mark it skipped + // (never re-evaluated again) rather than charge it, and rather than leaving + // it pending forever (proration_invoice_id would stay NULL, so without this + // marker AppsPendingProration would resurface it on every future sweep). + // + // This check deliberately compares against activatedAt, NOT "now": grace + + // ordinary sweep cadence can itself push the charge attempt a few days past + // this SAME periodEnd for a perfectly healthy, already-activated account + // (an app created near its period boundary) — that is expected, intended + // delayed billing (still the ONLY leg that ever bills this period), not a + // retroactive catch-up, and must still charge normally. + _, periodEnd := billingperiod.AnchoredPeriodWindow(app.CreatedAt.UTC(), billingperiod.AnchorDay(activatedAt)) + if !activatedAt.Before(periodEnd) { + if err := s.store.SetAppProrationSkipped(ctx, appID); err != nil { + return nil, billing.Internal("mark proration permanently skipped failed", err) + } + return &ProrationResult{AppID: appID, Status: ProrationStatusPeriodClosed}, nil + } + + // PM gate (D1d), same posture as the boundary spine: activated but no + // usable default PM is skipped and re-attempted next sweep (unlike the + // period-closed case above, "no PM right now" is not itself evidence the + // account was ever ineligible for this specific period, so it stays a + // transient, retried skip rather than a permanent one — see the judgment + // call noted in the PR description for the limits of this). + hasPM, err := s.store.HasUsableDefaultPM(ctx, app.AccountID) + if err != nil { + return nil, billing.Internal("usable PM check failed", err) + } + if !hasPM { + return &ProrationResult{AppID: appID, Status: ProrationStatusNoPM}, nil + } + if s.stripe == nil { + return nil, billing.Internal("ChargeCreationProration requires a Stripe client", nil) + } + custID, err := s.store.AccountStripeCustomer(ctx, app.AccountID) + if err != nil { + return nil, billing.Internal("stripe customer lookup failed", err) + } + if custID == "" { + // A usable PM implies a Customer (same anomaly posture as the spine). + return nil, billing.Internal("account has a usable PM but no Stripe customer id", nil) + } + + // The charge callback runs AFTER the row lock is released (ChargeProrationLocked, + // store.go): the not-deleted re-check happens under a brief lock, then the + // Stripe charge runs unlocked, then the guard-arm persists the result. + var cents int64 + outcome, invID, err := s.store.ChargeProrationLocked(ctx, appID, func(locked AppMirror) (*ProrationCharge, error) { + // Window = the anchored period CONTAINING the app's created_at (ADR 0005 + // anchor from the activation day). Derived from created_at, NEVER from now, + // so the amount is deterministic regardless of when the sweep fires. + periodStart, periodEnd := billingperiod.AnchoredPeriodWindow(locked.CreatedAt.UTC(), billingperiod.AnchorDay(activatedAt)) + // Priced off created_module_count — the count FROZEN at RegisterApp time + // — never the live module_count, which SyncAppModules may have moved + // during (or since) grace (finding 1: no retroactive tier drift). + prorated := usage.ProratedBaseMicros( + usage.AppBaseFeeMicros(usage.BaseFeeMicros, locked.CreatedModuleCount), + locked.CreatedAt, periodStart, periodEnd, + ) + c, err := centsFromMicros(prorated) + if err != nil { + return nil, billing.Internal("micros to cents conversion failed", err) + } + if c == 0 { + return nil, nil // rounds to 0 cents → nothing to invoice (guard stays unarmed) + } + + desc := fmt.Sprintf("MirrorStack app base fee (prorated) — app %s", locked.AppID) + if _, err := s.stripe.CreateInvoiceItem(ctx, custID, c, chargeCurrency, desc, appProrationItemIdemKey(locked.AppID)); err != nil { + return nil, billing.StripeError("proration invoice item failed", err) + } + inv, err := s.stripe.CreateInvoice(ctx, custID, true /* autoAdvance */, appProrationInvoiceIdemKey(locked.AppID)) + if err != nil { + return nil, billing.StripeError("proration invoice failed", err) + } + + // Mirror the PARTIAL window [creation day, period end) — the same coverage + // start ProratedBaseMicros priced, so mirror and amount agree by construction. + partialStart := usage.ProrationCoverageStart(locked.CreatedAt, periodStart) + cents = c + return &ProrationCharge{ + InvoiceID: inv.ID, + Cents: c, + Invoice: InvoiceMirror{ + AccountID: locked.AccountID, + StripeInvoiceID: inv.ID, + Status: inv.Status, + AmountDueCents: inv.AmountDue, + AmountPaidCents: inv.AmountPaid, + Currency: chargeCurrency, + PeriodStart: partialStart, + PeriodEnd: periodEnd, + }, + // Freeze what was billed keyed by the FULL anchored period_start (the + // display identity, migration 028); BaseMicros is the prorated amount. + Snapshot: AppBaseSnapshot{ + AppID: locked.AppID, + PeriodStart: periodStart, + PeriodEnd: periodEnd, + ModuleCount: locked.CreatedModuleCount, + BaseMicros: prorated, + }, + }, nil + }) + if err != nil { + // A billing.Error from the charge callback (Stripe / conversion) is already + // classified — surface it verbatim; anything else is a store/tx failure. + if _, ok := err.(*billing.Error); ok { + return nil, err + } + return nil, billing.Internal("locked creation-proration charge failed", err) + } + + switch outcome { + case ProrationLockedCharged: + return &ProrationResult{AppID: appID, Status: ProrationStatusCharged, ProrationInvoiceID: invID, ProrationCents: cents}, nil + case ProrationLockedAlreadyCharged: + return &ProrationResult{AppID: appID, Status: ProrationStatusAlreadyCharged, ProrationInvoiceID: invID}, nil + case ProrationLockedDeleted: + return &ProrationResult{AppID: appID, Status: ProrationStatusDeleted}, nil + case ProrationLockedNotFound: + return &ProrationResult{AppID: appID, Status: ProrationStatusNotFound}, nil + default: // ProrationLockedNoCharge + return &ProrationResult{AppID: appID, Status: ProrationStatusNoCharge}, nil + } +} + +// SweepProrationsResult tallies one SweepCreationProrations batch for the +// cmd/billing-cycle log line + exit code. +type SweepProrationsResult struct { + Pending int // apps past grace with an unarmed guard (the work list size) + Charged int // creation-proration invoices minted this sweep + Skipped int // legitimate no-charge outcomes (deleted / unactivated / no-PM / already / 0¢) + Failed int // per-app errors (charge failures) — retried next sweep +} + +// SweepCreationProrations charges the creation-period base for every app that has +// survived the grace window as of `at`: it lists the pending apps (created_at <= +// at − GraceDays, guard unarmed, not deleted) and runs ChargeCreationProration on +// each. Idempotent + resumable: an app charged on a prior sweep drops out of the +// work list (guard armed), and a per-app failure is counted but never aborts the +// batch (the next sweep retries it through the same deterministic Stripe keys). +func (s *Service) SweepCreationProrations(ctx context.Context, at time.Time) (*SweepProrationsResult, error) { + if at.IsZero() { + return nil, billing.InvalidInput("sweep instant required") + } + createdBefore := at.UTC().AddDate(0, 0, -usage.GraceDays) + appIDs, err := s.store.AppsPendingProration(ctx, createdBefore) + if err != nil { + return nil, billing.Internal("list pending prorations failed", err) + } + + res := &SweepProrationsResult{Pending: len(appIDs)} + for _, id := range appIDs { + r, err := s.ChargeCreationProration(ctx, id) + if err != nil { + slog.ErrorContext(ctx, "creation-proration charge failed", + "app_id", id, "error", err) + res.Failed++ + continue + } + if r.Status == ProrationStatusCharged { + res.Charged++ + } else { + res.Skipped++ + } + slog.InfoContext(ctx, "creation-proration", + "app_id", id, "status", string(r.Status), + "invoice_id", r.ProrationInvoiceID, "cents", r.ProrationCents) + } + return res, nil +} + +// appProrationItemIdemKey / appProrationInvoiceIdemKey build the deterministic +// Stripe Idempotency-Keys for the creation-proration charge. The APP id is the +// stable charge identity (each app prorates at most once — the one-shot +// proration_invoice_id guard), so a re-attempt (a retried sweep after a crash +// between the Stripe call and the guard-arm) reuses the SAME Stripe objects and +// can never double-charge even before the guard is armed. +func appProrationItemIdemKey(appID uuid.UUID) string { return "app-ii-" + appID.String() } +func appProrationInvoiceIdemKey(appID uuid.UUID) string { return "app-inv-" + appID.String() } diff --git a/internal/account/cycle/proration_test.go b/internal/account/cycle/proration_test.go new file mode 100644 index 0000000..e72b773 --- /dev/null +++ b/internal/account/cycle/proration_test.go @@ -0,0 +1,490 @@ +package cycle_test + +// ChargeCreationProration + SweepCreationProrations (creation grace, owner spec +// 2026-07-05). Reuses the in-memory fakeStore (service_test.go) + fakeStripe +// (charge_test.go) and the appsNow / registeredAccount helpers (apps_test.go). + +import ( + "context" + "testing" + "time" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + + "github.com/mirrorstack-ai/billing-engine/internal/account/billing" + "github.com/mirrorstack-ai/billing-engine/internal/account/cycle" + "github.com/mirrorstack-ai/billing-engine/internal/account/usage" +) + +// registerMirror seeds a roster row through RegisterApp (which only mirrors — no +// charge) so the app is owned by the fully-chargeable registeredAccount. +func registerMirror(t *testing.T, svc *cycle.Service, user, appID uuid.UUID, created time.Time, moduleCount int) { + t.Helper() + _, err := svc.RegisterApp(context.Background(), cycle.RegisterAppRequest{ + OwnerUserID: user, AppID: appID, ModuleCount: moduleCount, CreatedAt: created, + }) + require.NoError(t, err) +} + +// --- (a) grace holds: an app charged before GraceDays elapse is impossible --- + +func TestSweep_SkipsAppsWithinGrace(t *testing.T) { + store := newFakeStore() + user, _ := registeredAccount(store) + sc := newFakeStripe() + svc := appsSvc(store, sc) + appID := uuid.New() + registerMirror(t, svc, user, appID, time.Date(2026, 7, 1, 8, 0, 0, 0, time.UTC), 0) + + // Sweep only 2 days later (< GraceDays=3): the app is INSIDE the grace window + // (created_at Jul 1 > cutoff Jul 3 − 3d = Jun 30), so it is not even a + // candidate — no charge, nothing pending. + res, err := svc.SweepCreationProrations(context.Background(), time.Date(2026, 7, 3, 9, 0, 0, 0, time.UTC)) + require.NoError(t, err) + require.Equal(t, 0, res.Pending, "an app younger than GraceDays is not swept") + require.Equal(t, 0, res.Charged) + require.Empty(t, sc.itemCalls, "charging within grace is impossible") + require.Empty(t, store.apps[appID].ProrationInvoiceID) +} + +// --- (b) an app deleted within grace is NEVER charged, even by a later sweep -- + +func TestSweep_NeverChargesAppDeletedWithinGrace(t *testing.T) { + store := newFakeStore() + user, _ := registeredAccount(store) + sc := newFakeStripe() + svc := appsSvc(store, sc) + appID := uuid.New() + registerMirror(t, svc, user, appID, time.Date(2026, 7, 1, 8, 0, 0, 0, time.UTC), 0) + + // Deleted on day 0–1 (well within grace). + _, err := svc.SyncAppModules(context.Background(), cycle.SyncAppModulesRequest{AppID: appID, Deleted: true}) + require.NoError(t, err) + require.True(t, store.apps[appID].Deleted) + + // A sweep 9 days later (long past grace) must still NEVER charge it: the + // deleted row is excluded from the work list. + res, err := svc.SweepCreationProrations(context.Background(), time.Date(2026, 7, 10, 0, 0, 0, 0, time.UTC)) + require.NoError(t, err) + require.Equal(t, 0, res.Pending, "a deleted app is excluded from the sweep") + require.Empty(t, sc.itemCalls, "an app deleted within grace is never charged") + require.Empty(t, store.apps[appID].ProrationInvoiceID) +} + +// --- (c) a survivor is charged EXACTLY ONCE even if the sweep runs twice ------ + +func TestSweep_ChargesSurvivorExactlyOnceAcrossReruns(t *testing.T) { + store := newFakeStore() + user, _ := registeredAccount(store) + sc := newFakeStripe() + svc := appsSvc(store, sc) + appID := uuid.New() + registerMirror(t, svc, user, appID, time.Date(2026, 7, 1, 8, 0, 0, 0, time.UTC), 0) + + // First sweep past grace → charges the creation proration once. + first, err := svc.SweepCreationProrations(context.Background(), time.Date(2026, 7, 5, 0, 0, 0, 0, time.UTC)) + require.NoError(t, err) + require.Equal(t, 1, first.Pending) + require.Equal(t, 1, first.Charged) + require.Len(t, sc.itemCalls, 1) + require.EqualValues(t, 200, sc.itemCalls[0].amountCfg) // 3 of 30 days of $20 + armed := store.apps[appID].ProrationInvoiceID + require.NotEmpty(t, armed) + + // Second sweep (a re-fire the next day): the guard is armed, so the app is no + // longer pending and no second invoice is created. + second, err := svc.SweepCreationProrations(context.Background(), time.Date(2026, 7, 6, 0, 0, 0, 0, time.UTC)) + require.NoError(t, err) + require.Equal(t, 0, second.Pending, "an already-charged app drops out of the work list") + require.Equal(t, 0, second.Charged) + require.Len(t, sc.itemCalls, 1, "the re-fire must never charge twice") + require.Equal(t, armed, store.apps[appID].ProrationInvoiceID) +} + +// --- (d) the proration $ amount is unchanged from the pre-grace charge -------- + +func TestChargeCreationProration_AmountMatchesLegacyProration(t *testing.T) { + // The SAME numbers the pre-grace RegisterApp charge produced, now via the + // delayed charge leg: 20e6 × 3/30 = 2_000_000 micros → 200 cents, mirrored + // with the PARTIAL window [creation day, period end), snapshot frozen at the + // FULL anchored period start with the prorated amount. + store := newFakeStore() + user, acct := registeredAccount(store) + sc := newFakeStripe() + svc := appsSvc(store, sc) + appID := uuid.New() + registerMirror(t, svc, user, appID, time.Date(2026, 7, 1, 8, 0, 0, 0, time.UTC), 0) + + resp, err := svc.ChargeCreationProration(context.Background(), appID) + require.NoError(t, err) + require.Equal(t, cycle.ProrationStatusCharged, resp.Status) + require.EqualValues(t, 200, resp.ProrationCents) + + require.Len(t, sc.itemCalls, 1) + require.Len(t, sc.invoiceCalls, 1) + require.EqualValues(t, 200, sc.itemCalls[0].amountCfg) + require.Equal(t, "cus_apps_1", sc.itemCalls[0].custID) + require.Equal(t, "app-ii-"+appID.String(), sc.itemCalls[0].idemKey) + require.Equal(t, "app-inv-"+appID.String(), sc.invoiceCalls[0].idemKey) + require.True(t, sc.invoiceCalls[0].autoAdvance) + + require.Equal(t, sc.invoiceID, resp.ProrationInvoiceID) + mirror := store.invoices[sc.invoiceID] + require.Equal(t, acct, mirror.AccountID) + require.Equal(t, time.Date(2026, 7, 1, 0, 0, 0, 0, time.UTC), mirror.PeriodStart) // partial coverage start + require.Equal(t, time.Date(2026, 7, 4, 0, 0, 0, 0, time.UTC), mirror.PeriodEnd) + require.Equal(t, sc.invoiceID, store.apps[appID].ProrationInvoiceID) + + snap, ok := store.baseSnapshots[snapKey{appID, time.Date(2026, 6, 4, 0, 0, 0, 0, time.UTC)}] + require.True(t, ok, "the charge freezes its base keyed on the FULL anchored period start") + require.Equal(t, "proration", snap.source) + require.EqualValues(t, 2_000_000, snap.snap.BaseMicros) + require.Equal(t, time.Date(2026, 7, 4, 0, 0, 0, 0, time.UTC), snap.snap.PeriodEnd) + require.Equal(t, 0, snap.snap.ModuleCount) +} + +func TestChargeCreationProration_IncludesModuleOverage(t *testing.T) { + // module_count 7 → base 20e6 + 2×3e6 = 26e6; 15 of 30 remaining days → + // 13e6 micros → 1300 cents. Identical to the pre-grace math. + store := newFakeStore() + user, _ := registeredAccount(store) + sc := newFakeStripe() + svc := appsSvc(store, sc) + appID := uuid.New() + registerMirror(t, svc, user, appID, time.Date(2026, 6, 19, 12, 0, 0, 0, time.UTC), 7) + + resp, err := svc.ChargeCreationProration(context.Background(), appID) + require.NoError(t, err) + require.EqualValues(t, 1300, resp.ProrationCents) + require.Equal(t, 7, store.baseSnapshots[snapKey{appID, time.Date(2026, 6, 4, 0, 0, 0, 0, time.UTC)}].snap.ModuleCount) +} + +func TestChargeCreationProration_CreatedOnBoundaryChargesFullNewPeriodBase(t *testing.T) { + // Created exactly ON an anchor boundary (Jul 4 00:00): half-open windows put + // it at the START of the NEW period [Jul 4, Aug 4) → the FULL base ($20 → + // 2000 cents), snapshot keyed on Jul 4. Unchanged from the pre-grace charge. + store := newFakeStore() + user, _ := registeredAccount(store) + sc := newFakeStripe() + svc := appsSvc(store, sc) + appID := uuid.New() + registerMirror(t, svc, user, appID, time.Date(2026, 7, 4, 0, 0, 0, 0, time.UTC), 0) + + resp, err := svc.ChargeCreationProration(context.Background(), appID) + require.NoError(t, err) + require.EqualValues(t, 2_000, resp.ProrationCents, "creation-day == period start → full base") + mirror := store.invoices[sc.invoiceID] + require.Equal(t, time.Date(2026, 7, 4, 0, 0, 0, 0, time.UTC), mirror.PeriodStart) + require.Equal(t, time.Date(2026, 8, 4, 0, 0, 0, 0, time.UTC), mirror.PeriodEnd) + snap := store.baseSnapshots[snapKey{appID, time.Date(2026, 7, 4, 0, 0, 0, 0, time.UTC)}] + require.Equal(t, "proration", snap.source) + require.EqualValues(t, usage.BaseFeeMicros, snap.snap.BaseMicros) +} + +func TestChargeCreationProration_DelayedPastPeriodEndStillCharges(t *testing.T) { + // Delayed billing (grace point 5): the charge is anchored to the TRUE + // created_at, NOT now, so even when grace + ordinary sweep cadence pushes the + // charge attempt past the creation period's end, the creation-period + // proration STILL fires — that period is billed by NO other leg (the + // boundary advance leg only ever bills an app's SUBSEQUENT periods), so + // charging it is correct and never double-bills. This is NOT the D1d + // retroactive-catch-up case (see TestChargeCreationProration_ + // SkipsPermanentlyWhenActivatedAfterPeriodClosed below): the account here was + // ALREADY activated (May 4, registeredAccount) well before the app's period + // even opened — the period-closed check in ChargeCreationProration compares + // against activatedAt, not "now", so a healthy already-activated account is + // never penalized for a sweep that simply runs late. + // App created May 20 → period [May 4, Jun 4), long closed by the sweep in + // July: 15 of 31 days of $20 = round_half_up(9_677_419.8) → 968 cents. + store := newFakeStore() + user, _ := registeredAccount(store) + sc := newFakeStripe() + svc := appsSvc(store, sc) + appID := uuid.New() + registerMirror(t, svc, user, appID, time.Date(2026, 5, 20, 9, 0, 0, 0, time.UTC), 0) + + res, err := svc.SweepCreationProrations(context.Background(), time.Date(2026, 7, 1, 0, 0, 0, 0, time.UTC)) + require.NoError(t, err) + require.Equal(t, 1, res.Charged) + require.Len(t, sc.itemCalls, 1) + require.EqualValues(t, 968, sc.itemCalls[0].amountCfg) + snap := store.baseSnapshots[snapKey{appID, time.Date(2026, 5, 4, 0, 0, 0, 0, time.UTC)}] + require.Equal(t, "proration", snap.source) +} + +// --- ChargeCreationProration: idempotency + gates ---------------------------- + +func TestChargeCreationProration_IdempotentGuard(t *testing.T) { + store := newFakeStore() + user, _ := registeredAccount(store) + sc := newFakeStripe() + svc := appsSvc(store, sc) + appID := uuid.New() + registerMirror(t, svc, user, appID, time.Date(2026, 7, 1, 8, 0, 0, 0, time.UTC), 0) + + first, err := svc.ChargeCreationProration(context.Background(), appID) + require.NoError(t, err) + require.Equal(t, cycle.ProrationStatusCharged, first.Status) + + second, err := svc.ChargeCreationProration(context.Background(), appID) + require.NoError(t, err) + require.Equal(t, cycle.ProrationStatusAlreadyCharged, second.Status) + require.Equal(t, first.ProrationInvoiceID, second.ProrationInvoiceID) + require.Len(t, sc.itemCalls, 1, "the one-shot guard prevents a second charge") +} + +func TestChargeCreationProration_SkipsDeleted(t *testing.T) { + store := newFakeStore() + user, _ := registeredAccount(store) + sc := newFakeStripe() + svc := appsSvc(store, sc) + appID := uuid.New() + registerMirror(t, svc, user, appID, time.Date(2026, 7, 1, 8, 0, 0, 0, time.UTC), 0) + _, err := svc.SyncAppModules(context.Background(), cycle.SyncAppModulesRequest{AppID: appID, Deleted: true}) + require.NoError(t, err) + + resp, err := svc.ChargeCreationProration(context.Background(), appID) + require.NoError(t, err) + require.Equal(t, cycle.ProrationStatusDeleted, resp.Status) + require.Empty(t, sc.itemCalls) +} + +func TestChargeCreationProration_SkipsUnactivatedAndNoPM(t *testing.T) { + // Unactivated account → skipped_unactivated (D1d, no retroactive catch-up). + store := newFakeStore() + user, acct := registeredAccount(store) + delete(store.activation, acct) + sc := newFakeStripe() + svc := appsSvc(store, sc) + appID := uuid.New() + registerMirror(t, svc, user, appID, time.Date(2026, 7, 1, 8, 0, 0, 0, time.UTC), 0) + + resp, err := svc.ChargeCreationProration(context.Background(), appID) + require.NoError(t, err) + require.Equal(t, cycle.ProrationStatusUnactivated, resp.Status) + require.Empty(t, sc.itemCalls) + + // Activated but no usable PM → skipped_no_pm (re-attempted next sweep). + store.activation[acct] = time.Date(2026, 5, 4, 9, 0, 0, 0, time.UTC) + store.hasPM = false + resp, err = svc.ChargeCreationProration(context.Background(), appID) + require.NoError(t, err) + require.Equal(t, cycle.ProrationStatusNoPM, resp.Status) + require.Empty(t, sc.itemCalls) + require.Empty(t, store.apps[appID].ProrationInvoiceID) +} + +func TestChargeCreationProration_NotFound(t *testing.T) { + svc := appsSvc(newFakeStore(), newFakeStripe()) + resp, err := svc.ChargeCreationProration(context.Background(), uuid.New()) + require.NoError(t, err) + require.Equal(t, cycle.ProrationStatusNotFound, resp.Status) +} + +func TestChargeCreationProration_Validation(t *testing.T) { + svc := appsSvc(newFakeStore(), newFakeStripe()) + _, err := svc.ChargeCreationProration(context.Background(), uuid.Nil) + requireCode(t, err, billing.CodeInvalidInput) +} + +func TestSweepCreationProrations_Validation(t *testing.T) { + svc := appsSvc(newFakeStore(), newFakeStripe()) + _, err := svc.SweepCreationProrations(context.Background(), time.Time{}) + requireCode(t, err, billing.CodeInvalidInput) +} + +// --- Sweep: multiple apps, mixed states -------------------------------------- + +func TestSweep_ChargesOnlyPastGraceLiveUnchargedApps(t *testing.T) { + store := newFakeStore() + user, _ := registeredAccount(store) + sc := newFakeStripe() + svc := appsSvc(store, sc) + + past := uuid.New() // past grace, live, uncharged → charged + young := uuid.New() // within grace → skipped + gone := uuid.New() // past grace but deleted → skipped + registerMirror(t, svc, user, past, time.Date(2026, 6, 20, 8, 0, 0, 0, time.UTC), 0) + registerMirror(t, svc, user, young, time.Date(2026, 6, 29, 8, 0, 0, 0, time.UTC), 0) + registerMirror(t, svc, user, gone, time.Date(2026, 6, 20, 8, 0, 0, 0, time.UTC), 0) + _, err := svc.SyncAppModules(context.Background(), cycle.SyncAppModulesRequest{AppID: gone, Deleted: true}) + require.NoError(t, err) + + // Sweep as of Jun 30 → cutoff Jun 27: past (Jun 20) qualifies; young (Jun 29) + // is within grace; gone is deleted. + res, err := svc.SweepCreationProrations(context.Background(), time.Date(2026, 6, 30, 0, 0, 0, 0, time.UTC)) + require.NoError(t, err) + require.Equal(t, 1, res.Pending) + require.Equal(t, 1, res.Charged) + require.NotEmpty(t, store.apps[past].ProrationInvoiceID) + require.Empty(t, store.apps[young].ProrationInvoiceID) + require.Empty(t, store.apps[gone].ProrationInvoiceID) +} + +// --- FINDING 1: the creation-proration charge must price off the module count +// FROZEN at RegisterApp time, never whatever module_count SyncAppModules moves +// it to during (or after) the grace window ---------------------------------- + +func TestChargeCreationProration_PricesFrozenCountNotLiveCountAfterMidGraceInstall(t *testing.T) { + // Reproduces the exact failure scenario: an app registers with 0 modules, + // the customer installs 7 MORE modules (via SyncAppModules) DURING the + // mandatory grace window — before the sweep ever charges — and the sweep + // fires after grace elapses. Pre-fix, the charge priced off the module_count + // read FRESH at sweep time (7, live) → 20e6 + 2×3e6 = 26e6 base, 3 of 30 days + // → 2_600_000 micros → 260 cents: a HIGHER tier retroactively applied to + // days that never had 7 modules installed. Fixed: the charge prices off + // created_module_count (frozen at registration, 0) → 20e6 base, 3 of 30 days + // → 2_000_000 micros → 200 cents — identical to + // TestChargeCreationProration_AmountMatchesLegacyProration's un-synced case. + store := newFakeStore() + user, _ := registeredAccount(store) + sc := newFakeStripe() + svc := appsSvc(store, sc) + appID := uuid.New() + registerMirror(t, svc, user, appID, time.Date(2026, 7, 1, 8, 0, 0, 0, time.UTC), 0) + + // Mid-grace install: the live count jumps to 7 BEFORE the sweep ever runs. + _, err := svc.SyncAppModules(context.Background(), cycle.SyncAppModulesRequest{ + AppID: appID, ModuleCount: intPtr(7), + }) + require.NoError(t, err) + require.Equal(t, 7, store.apps[appID].ModuleCount, "the live count DID move") + require.Equal(t, 0, store.apps[appID].CreatedModuleCount, "the frozen count must NOT move") + + resp, err := svc.ChargeCreationProration(context.Background(), appID) + require.NoError(t, err) + require.Equal(t, cycle.ProrationStatusCharged, resp.Status) + require.EqualValues(t, 200, resp.ProrationCents, + "must price off the FROZEN count (0 modules → $20 base), not the live count (7 → $26 base)") + require.Len(t, sc.itemCalls, 1) + require.EqualValues(t, 200, sc.itemCalls[0].amountCfg) + + // The migration-028 snapshot must also record the FROZEN count/amount — the + // display must never show a tier that never applied to those days either. + snap := store.baseSnapshots[snapKey{appID, time.Date(2026, 6, 4, 0, 0, 0, 0, time.UTC)}] + require.Equal(t, 0, snap.snap.ModuleCount) + require.EqualValues(t, 2_000_000, snap.snap.BaseMicros) + + // The LIVE count survives untouched for the boundary advance leg's future + // periods — only the historical creation-period charge is frozen. + require.Equal(t, 7, store.apps[appID].ModuleCount) +} + +func TestChargeCreationProration_PricesFrozenCountNotLiveCountAfterMidGraceUninstall(t *testing.T) { + // The reverse direction: registers with 7 modules (overage tier), the + // customer UNINSTALLS all the way down to 0 during grace. Pre-fix this would + // retroactively price the creation days at the LOWER tier (200 cents, 0 + // modules) though they were never at 0 modules on those days. Fixed: prices + // off the frozen created_module_count (7) → 1300 cents, matching + // TestChargeCreationProration_IncludesModuleOverage's un-synced case exactly. + store := newFakeStore() + user, _ := registeredAccount(store) + sc := newFakeStripe() + svc := appsSvc(store, sc) + appID := uuid.New() + registerMirror(t, svc, user, appID, time.Date(2026, 6, 19, 12, 0, 0, 0, time.UTC), 7) + + _, err := svc.SyncAppModules(context.Background(), cycle.SyncAppModulesRequest{ + AppID: appID, ModuleCount: intPtr(0), + }) + require.NoError(t, err) + require.Equal(t, 0, store.apps[appID].ModuleCount) + require.Equal(t, 7, store.apps[appID].CreatedModuleCount, "the frozen count must NOT move") + + resp, err := svc.ChargeCreationProration(context.Background(), appID) + require.NoError(t, err) + require.EqualValues(t, 1_300, resp.ProrationCents, + "must price off the FROZEN count (7 modules → overage tier), not the live count (0 → base only)") +} + +// --- FINDING 2: no retroactive catch-up (D1d) when an account only activates +// after the app's anchored creation period already closed ------------------- + +func TestChargeCreationProration_SkipsPermanentlyWhenActivatedAfterPeriodClosed(t *testing.T) { + // Reproduces the exact failure scenario: an app is created while its account + // is unactivated. Every sweep correctly leaves it unbilled (skipped_ + // unactivated). MONTHS later the owner finally binds a card — with anchor + // day 1 (activated Apr 1), the app's anchored creation period is + // [Jan 1, Feb 1), long closed by Apr 1. Pre-fix, the very next charge + // attempt would retroactively bill that period in FULL (2000 cents on 0 + // modules, since Jan 1 == the period start): exactly the retroactive + // catch-up D1d forbids. Fixed: the charge is PERMANENTLY skipped — no + // Stripe call, ever, and the app never resurfaces on a later sweep (it + // would otherwise sit pending forever, since proration_invoice_id never + // gets set for a skipped charge). + store := newFakeStore() + user, acct := registeredAccount(store) + delete(store.activation, acct) // unactivated at creation + sc := newFakeStripe() + svc := appsSvc(store, sc) + appID := uuid.New() + created := time.Date(2026, 1, 1, 8, 0, 0, 0, time.UTC) + registerMirror(t, svc, user, appID, created, 0) + + // Past grace, still unactivated → correctly pending, no charge (D1d's + // existing unactivated gate — unchanged by this fix). + first, err := svc.ChargeCreationProration(context.Background(), appID) + require.NoError(t, err) + require.Equal(t, cycle.ProrationStatusUnactivated, first.Status) + require.Empty(t, sc.itemCalls) + + // MONTHS later: the owner binds a card (anchor day 1) and a PM. + store.activation[acct] = time.Date(2026, 4, 1, 9, 0, 0, 0, time.UTC) + store.hasPM = true + + second, err := svc.ChargeCreationProration(context.Background(), appID) + require.NoError(t, err) + require.Equal(t, cycle.ProrationStatusPeriodClosed, second.Status) + require.Empty(t, sc.itemCalls, "no Stripe call — a retroactive catch-up charge must never fire") + require.Empty(t, store.apps[appID].ProrationInvoiceID) + require.True(t, store.apps[appID].ProrationSkipped, "permanently marked so it is never re-evaluated") + + // A cheap re-evaluation (e.g. a retried RPC) short-circuits on the marker + // without even re-reading account activation — still no Stripe call. + third, err := svc.ChargeCreationProration(context.Background(), appID) + require.NoError(t, err) + require.Equal(t, cycle.ProrationStatusPeriodClosed, third.Status) + require.Empty(t, sc.itemCalls) + + // A LATER sweep must never resurface it — it would otherwise sit pending + // forever (proration_invoice_id stays NULL for a permanently-skipped app). + res, err := svc.SweepCreationProrations(context.Background(), time.Date(2026, 5, 1, 0, 0, 0, 0, time.UTC)) + require.NoError(t, err) + require.Equal(t, 0, res.Pending, "a permanently-skipped app never resurfaces on a later sweep") + require.Empty(t, sc.itemCalls) +} + +func TestChargeCreationProration_ActivatedBeforePeriodClosesStillCharges(t *testing.T) { + // Guard against an over-broad fix: an account that activates BEFORE the + // app's anchored creation period closes must charge normally — D1d only + // blocks a retroactive catch-up when the account was unactivated for the + // app's ENTIRE creation period. The anchor day is DERIVED from activatedAt + // itself (billingperiod.AnchorDay), so activating the SAME calendar day the + // app was created (the common "sign up, create an app, add a card" onboarding + // flow) anchors the period at that same day-of-month — putting its END a + // full month out, safely after activation. + store := newFakeStore() + user, acct := registeredAccount(store) + delete(store.activation, acct) + sc := newFakeStripe() + svc := appsSvc(store, sc) + appID := uuid.New() + registerMirror(t, svc, user, appID, time.Date(2026, 1, 10, 6, 0, 0, 0, time.UTC), 0) + + first, err := svc.ChargeCreationProration(context.Background(), appID) + require.NoError(t, err) + require.Equal(t, cycle.ProrationStatusUnactivated, first.Status) + + // Activates a few hours later the SAME day (anchor day 10) → period + // [Jan 10, Feb 10) — still wide open — and binds a PM. + store.activation[acct] = time.Date(2026, 1, 10, 9, 0, 0, 0, time.UTC) + store.hasPM = true + + resp, err := svc.ChargeCreationProration(context.Background(), appID) + require.NoError(t, err) + require.Equal(t, cycle.ProrationStatusCharged, resp.Status) + require.EqualValues(t, 2_000, resp.ProrationCents, "created on/after the period start → full base") + require.Len(t, sc.itemCalls, 1) + require.False(t, store.apps[appID].ProrationSkipped) +} diff --git a/internal/account/cycle/rollback_integration_test.go b/internal/account/cycle/rollback_integration_test.go new file mode 100644 index 0000000..3b89e74 --- /dev/null +++ b/internal/account/cycle/rollback_integration_test.go @@ -0,0 +1,54 @@ +//go:build integration + +package cycle + +// White-box (package cycle, not cycle_test) so this can exercise the +// unexported deferredRollback directly (finding 3, store.go): the deferred +// Rollback in ChargeProrationLocked's phases must reach Postgres even when the +// context passed to the surrounding call is already cancelled/expired (e.g. +// the enclosing Lambda invocation timed out while awaiting a stalled Stripe +// call) — reusing that dead context verbatim in the Rollback call can fail +// silently, leaving the transaction (and its row lock) open until Postgres's +// own dead-connection detection eventually reclaims it. + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/mirrorstack-ai/billing-engine/internal/shared/testutil" +) + +func TestDeferredRollback_Integration_SurvivesAlreadyCancelledContext(t *testing.T) { + pool := testutil.NewTestDB(t) + + // Sanity check: confirms the bug this test guards against is real — pgx + // refuses to run a network round-trip (including ROLLBACK) against an + // already-cancelled context, so a NAIVE `defer func() { tx.Rollback(ctx) }()` + // that reuses a dead ctx verbatim is a silent no-op. + ctx, cancel := context.WithCancel(context.Background()) + tx, err := pool.Begin(ctx) + require.NoError(t, err) + cancel() + naiveErr := tx.Rollback(ctx) + require.Error(t, naiveErr, "sanity: rolling back with an already-cancelled context fails") + // Clean up the still-open transaction/connection from the sanity check with + // a fresh context so it doesn't leak into the real assertion below. + _ = tx.Rollback(context.Background()) + + // The fix: deferredRollback is handed the SAME dead ctx but must still reach + // Postgres via its own short-lived detached context. + tx2, err := pool.Begin(context.Background()) + require.NoError(t, err) + deferredRollback(ctx, tx2) // ctx here is STILL the cancelled context from above + + // Prove the rollback actually completed and the connection is healthy again + // (a leaked/open transaction would eventually starve the pool; this bounds + // how long we're willing to wait for the pool to hand back a working conn). + require.Eventually(t, func() bool { + var one int + return pool.QueryRow(context.Background(), "SELECT 1").Scan(&one) == nil && one == 1 + }, 3*time.Second, 50*time.Millisecond, "deferredRollback must release the transaction/connection despite the dead ctx") +} diff --git a/internal/account/cycle/service_test.go b/internal/account/cycle/service_test.go index 2026d7f..d102725 100644 --- a/internal/account/cycle/service_test.go +++ b/internal/account/cycle/service_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "math" + "sort" "testing" "time" @@ -76,36 +77,39 @@ type fakeStore struct { invoices map[string]cycle.InvoiceMirror // stripe_invoice_id → mirror // injected errors - errOpen error - errRaw error - errPrice error - errUpsert error - errIncome error - errVis error - errSettle error - errInsertRun error - errTotal error - errPM error - errCustomer error - errInvoice error - errMarkRun error - errUnbilled error - errUsageEvents error - errActivated error // ActivatedAccounts - errLatestPeriod error // LatestClosedPeriodEnd - errCollection error // AccountCollection - errUpdateColl error // UpdateAccountCollection - errUnpaid error // HasUnpaidInvoice - errEnsureAcct error // EnsureAccountForUser - errActivation error // AccountActivation - errAppInsert error // InsertAppMirror - errAppMirror error // AppMirror - errSetProration error // SetAppProrationInvoice - errSetCount error // SetAppModuleCount - errMarkDeleted error // MarkAppDeleted - errLiveCounts error // LiveAppsCreatedBefore - errProrationSnap error // UpsertProrationBaseSnapshot - errAdvanceSnap error // InsertAdvanceBaseSnapshot + errOpen error + errRaw error + errPrice error + errUpsert error + errIncome error + errVis error + errSettle error + errInsertRun error + errTotal error + errPM error + errCustomer error + errInvoice error + errMarkRun error + errUnbilled error + errUsageEvents error + errActivated error // ActivatedAccounts + errLatestPeriod error // LatestClosedPeriodEnd + errCollection error // AccountCollection + errUpdateColl error // UpdateAccountCollection + errUnpaid error // HasUnpaidInvoice + errEnsureAcct error // EnsureAccountForUser + errActivation error // AccountActivation + errAppInsert error // InsertAppMirror + errAppMirror error // AppMirror + errSetProration error // SetAppProrationInvoice + errSetSkipped error // SetAppProrationSkipped + errSetCount error // SetAppModuleCount + errMarkDeleted error // MarkAppDeleted + errLiveCounts error // LiveAppsCreatedBefore + errProrationSnap error // UpsertProrationBaseSnapshot + errAdvanceSnap error // InsertAdvanceBaseSnapshot + errPendingProration error // AppsPendingProration + errChargeLocked error // ChargeProrationLocked } // snapKey mirrors the app_base_snapshots PRIMARY KEY (app_id, period_start). @@ -393,7 +397,9 @@ func (f *fakeStore) InsertAppMirror(_ context.Context, appID, accountID uuid.UUI return nil // ON CONFLICT (app_id) DO NOTHING — the FIRST registration wins } f.apps[appID] = cycle.AppMirror{ - AppID: appID, AccountID: accountID, ModuleCount: moduleCount, CreatedAt: createdAt, + AppID: appID, AccountID: accountID, ModuleCount: moduleCount, + CreatedModuleCount: moduleCount, // frozen at insert, mirroring InsertAppMirror's $3/$3 write + CreatedAt: createdAt, } return nil } @@ -417,6 +423,17 @@ func (f *fakeStore) SetAppProrationInvoice(_ context.Context, appID uuid.UUID, s return nil } +func (f *fakeStore) SetAppProrationSkipped(_ context.Context, appID uuid.UUID) error { + if f.errSetSkipped != nil { + return f.errSetSkipped + } + if app, ok := f.apps[appID]; ok && !app.ProrationSkipped && app.ProrationInvoiceID == "" { + app.ProrationSkipped = true // first-write-wins, like the WHERE … IS NULL guard + f.apps[appID] = app + } + return nil +} + func (f *fakeStore) SetAppModuleCount(_ context.Context, appID uuid.UUID, moduleCount int) error { if f.errSetCount != nil { return f.errSetCount @@ -440,6 +457,58 @@ func (f *fakeStore) MarkAppDeleted(_ context.Context, appID uuid.UUID) error { return nil } +func (f *fakeStore) AppsPendingProration(_ context.Context, createdBefore time.Time) ([]uuid.UUID, error) { + if f.errPendingProration != nil { + return nil, f.errPendingProration + } + // Mirrors the SQL: created_at <= cutoff AND proration_invoice_id IS NULL AND + // deleted_at IS NULL AND proration_skipped_at IS NULL. Sorted by created_at + // for a deterministic sweep order. + var out []uuid.UUID + for id, app := range f.apps { + if app.ProrationInvoiceID == "" && !app.Deleted && !app.ProrationSkipped && !app.CreatedAt.After(createdBefore) { + out = append(out, id) + } + } + sort.Slice(out, func(i, j int) bool { + return f.apps[out[i]].CreatedAt.Before(f.apps[out[j]].CreatedAt) + }) + return out, nil +} + +// ChargeProrationLocked models the pgxStore's FOR UPDATE-locked charge section: +// it re-checks the terminal state (the fake's in-memory row is the "locked" +// read), invokes charge only when still chargeable, and persists the invoice + +// snapshot + arms the guard on a non-nil payload — exactly what the real tx does +// atomically. +func (f *fakeStore) ChargeProrationLocked(_ context.Context, appID uuid.UUID, charge func(cycle.AppMirror) (*cycle.ProrationCharge, error)) (cycle.ProrationOutcome, string, error) { + if f.errChargeLocked != nil { + return 0, "", f.errChargeLocked + } + app, ok := f.apps[appID] + if !ok { + return cycle.ProrationLockedNotFound, "", nil + } + if app.Deleted { + return cycle.ProrationLockedDeleted, "", nil + } + if app.ProrationInvoiceID != "" { + return cycle.ProrationLockedAlreadyCharged, app.ProrationInvoiceID, nil + } + pc, err := charge(app) + if err != nil { + return 0, "", err + } + if pc == nil { + return cycle.ProrationLockedNoCharge, "", nil + } + f.invoices[pc.Invoice.StripeInvoiceID] = pc.Invoice + f.baseSnapshots[snapKey{pc.Snapshot.AppID, pc.Snapshot.PeriodStart}] = fakeBaseSnapshot{snap: pc.Snapshot, source: "proration"} + app.ProrationInvoiceID = pc.InvoiceID // first-charge-wins, like WHERE … IS NULL under the lock + f.apps[appID] = app + return cycle.ProrationLockedCharged, pc.InvoiceID, nil +} + func (f *fakeStore) LiveAppsCreatedBefore(_ context.Context, accountID uuid.UUID, createdBefore time.Time) ([]cycle.AppModuleCount, error) { if f.errLiveCounts != nil { return nil, f.errLiveCounts diff --git a/internal/account/cycle/store.go b/internal/account/cycle/store.go index 3e248cd..95be5b6 100644 --- a/internal/account/cycle/store.go +++ b/internal/account/cycle/store.go @@ -183,12 +183,51 @@ type Store interface { // deletion semantics). found=false → the app was never registered. AppMirror(ctx context.Context, appID uuid.UUID) (AppMirror, bool, error) + // AppsPendingProration returns the app ids past the creation grace window + // (created_at <= createdBefore = now − GraceDays) that are still LIVE + // (deleted_at IS NULL), NOT yet charged (proration_invoice_id IS NULL), and + // NOT permanently skipped (proration_skipped_at IS NULL, migration 031) — + // the creation-proration sweep's work list. An app deleted within grace, + // already charged, or already determined to be a would-be retroactive + // catch-up (D1d) is excluded. + AppsPendingProration(ctx context.Context, createdBefore time.Time) ([]uuid.UUID, error) + + // ChargeProrationLocked runs the creation-proration charge for ONE app. It + // briefly SELECT ... FOR UPDATE-locks the roster row to re-verify the row is + // still chargeable (deleted_at IS NULL AND proration_invoice_id IS NULL) and + // read its frozen state, then RELEASES the lock before invoking charge — + // which performs the (potentially slow) Stripe network calls OUTSIDE any + // lock or transaction — and finally persists the mirrored invoice, the base + // snapshot, and the one-shot guard in a second short transaction. The lock is + // deliberately NOT held across the Stripe call (a prior version did; it could + // block a concurrent SyncAppModules/MarkAppDeleted write for the Stripe SDK's + // full ~80s-per-call timeout): a soft-delete that commits while the charge + // callback is in flight does NOT unwind an already-succeeded Stripe charge + // (D1e already forbids refunds — the money moved), so the persist step + // writes the invoice/snapshot/guard unconditionally on success. A second, + // genuinely concurrent charge attempt for the SAME app converges on the SAME + // Stripe objects (the deterministic per-app Idempotency-Keys) and the guard's + // first-write-wins UPDATE, so this stays race-safe without a lock spanning + // both phases. charge returning (nil, nil) means "nothing to charge" (0 + // cents) → nothing is persisted. The returned invoice id is the armed (or + // pre-armed) guard's. + ChargeProrationLocked(ctx context.Context, appID uuid.UUID, charge func(locked AppMirror) (*ProrationCharge, error)) (ProrationOutcome, string, error) + // SetAppProrationInvoice arms the ONE-SHOT creation-proration guard: it // records the Stripe invoice id, first-charge-wins (UPDATE … WHERE // proration_invoice_id IS NULL). An already-armed guard is NOT an error — // the write is a no-op and the original invoice id survives. SetAppProrationInvoice(ctx context.Context, appID uuid.UUID, stripeInvoiceID string) error + // SetAppProrationSkipped arms the PERMANENT creation-proration skip marker + // (migration 031, D1d): the account only activated at/after this app's + // anchored creation period had already closed, so the app is EXCLUDED from + // every future sweep rather than left pending forever (proration_invoice_id + // stays NULL, so without this marker AppsPendingProration would resurface it + // on every sweep indefinitely). First-write-wins and a no-op if the app was + // somehow already charged in the meantime — never an error. + SetAppProrationSkipped(ctx context.Context, appID uuid.UUID) error + // SetAppModuleCount snapshots a new installed-module count. A deleted // app's count is frozen (the UPDATE's WHERE deleted_at IS NULL no-ops — // D1e: no future base, so no tier to move). @@ -249,12 +288,23 @@ type AppBaseSnapshot struct { // AppMirror is the in-memory form of a ms_billing.apps roster row (migration // 027). ProrationInvoiceID is "" while the one-shot creation-proration guard // is unarmed; DeletedAt is meaningful only when Deleted is true. +// CreatedModuleCount (migration 030) is the module count FROZEN at +// RegisterApp time — immutable, never touched by SyncAppModules — and is what +// ChargeCreationProration prices the historical creation-period window from; +// ModuleCount is the LIVE count SyncAppModules keeps current and is what the +// boundary advance leg (and the display read for all FUTURE periods) uses. +// ProrationSkipped (migration 031) is true once the app's creation-proration +// charge has been PERMANENTLY skipped as a would-be retroactive catch-up +// (D1d): the account only activated at/after the app's anchored creation +// period had already closed. type AppMirror struct { AppID uuid.UUID AccountID uuid.UUID ModuleCount int + CreatedModuleCount int CreatedAt time.Time ProrationInvoiceID string + ProrationSkipped bool Deleted bool DeletedAt time.Time } @@ -841,13 +891,176 @@ func (s *pgxStore) AppMirror(ctx context.Context, appID uuid.UUID) (AppMirror, b AppID: app, AccountID: acct, ModuleCount: int(row.ModuleCount), + CreatedModuleCount: int(row.CreatedModuleCount), CreatedAt: row.CreatedAt, ProrationInvoiceID: row.ProrationInvoiceID.String, // "" when NULL (guard unarmed) + ProrationSkipped: row.ProrationSkippedAt.Valid, Deleted: row.DeletedAt.Valid, DeletedAt: row.DeletedAt.Time, }, true, nil } +func (s *pgxStore) AppsPendingProration(ctx context.Context, createdBefore time.Time) ([]uuid.UUID, error) { + rows, err := s.q.AppsPendingProration(ctx, createdBefore) + if err != nil { + return nil, err + } + return parseUUIDs(rows) +} + +// deferredRollback rolls back tx using a short-lived DETACHED context rather +// than reusing ctx verbatim. ctx may already be cancelled or past its deadline +// by the time this runs (e.g. the surrounding Lambda invocation timed out +// while a Stripe call the caller was awaiting stalled) — Rollback on a dead +// context can fail silently, leaving the row lock / transaction open until +// Postgres's own dead-connection detection eventually reclaims it. Stripping +// cancellation (context.WithoutCancel) while keeping request-scoped values, +// then applying a fresh short timeout, lets cleanup reach Postgres either way. +func deferredRollback(ctx context.Context, tx pgx.Tx) { + rctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 5*time.Second) + defer cancel() + _ = tx.Rollback(rctx) // no-op after a successful Commit +} + +// lockAndReadChargeableApp briefly SELECT ... FOR UPDATE-locks the roster row, +// re-verifies it is still chargeable (deleted_at IS NULL AND +// proration_invoice_id IS NULL), and releases the lock (the transaction +// commits either way — there is nothing left to write once the terminal +// checks pass, so a plain commit is equivalent to and cheaper than a rollback +// here). proceed=false means the caller must return (outcome, invID, nil) +// immediately without invoking charge; proceed=true carries the locked +// snapshot (including the frozen created_module_count) charge prices from. +func (s *pgxStore) lockAndReadChargeableApp(ctx context.Context, appID uuid.UUID) (locked AppMirror, outcome ProrationOutcome, invID string, proceed bool, err error) { + tx, err := s.pool.Begin(ctx) + if err != nil { + return AppMirror{}, 0, "", false, err + } + defer deferredRollback(ctx, tx) + + qtx := s.q.WithTx(tx) + row, err := qtx.SelectAppMirrorForUpdate(ctx, appID.String()) + if errors.Is(err, pgx.ErrNoRows) { + return AppMirror{}, ProrationLockedNotFound, "", false, nil + } + if err != nil { + return AppMirror{}, 0, "", false, err + } + if row.DeletedAt.Valid { + return AppMirror{}, ProrationLockedDeleted, "", false, nil + } + if row.ProrationInvoiceID.Valid { + return AppMirror{}, ProrationLockedAlreadyCharged, row.ProrationInvoiceID.String, false, nil + } + + app, err := uuid.Parse(row.AppID) + if err != nil { + return AppMirror{}, 0, "", false, err + } + acct, err := uuid.Parse(row.AccountID) + if err != nil { + return AppMirror{}, 0, "", false, err + } + locked = AppMirror{ + AppID: app, + AccountID: acct, + ModuleCount: int(row.ModuleCount), + CreatedModuleCount: int(row.CreatedModuleCount), + CreatedAt: row.CreatedAt, + } + + if err := tx.Commit(ctx); err != nil { + return AppMirror{}, 0, "", false, err + } + return locked, 0, "", true, nil +} + +// persistProrationCharge mirrors a SUCCESSFULLY-created Stripe charge (the +// invoice, the migration-028 base snapshot, and the one-shot guard) inside one +// short transaction. Called AFTER the Stripe network call has already +// completed — the money has already moved — so this always persists on a +// non-nil pc: a concurrent soft-delete that raced in during the (now-released) +// window between the lock and this write does NOT unwind an already-succeeded +// charge (D1e forbids refunds), and a genuinely concurrent second charge +// attempt for the same app converges on identical values (the deterministic +// per-app Stripe Idempotency-Keys guarantee the SAME invoice id, and every +// write here is itself idempotent / first-write-wins). +func (s *pgxStore) persistProrationCharge(ctx context.Context, appID uuid.UUID, pc *ProrationCharge) (ProrationOutcome, string, error) { + tx, err := s.pool.Begin(ctx) + if err != nil { + return 0, "", err + } + defer deferredRollback(ctx, tx) + qtx := s.q.WithTx(tx) + + due, err := centsNumeric(pc.Invoice.AmountDueCents) + if err != nil { + return 0, "", err + } + paid, err := centsNumeric(pc.Invoice.AmountPaidCents) + if err != nil { + return 0, "", err + } + if err := qtx.UpsertInvoice(ctx, db.UpsertInvoiceParams{ + AccountID: pc.Invoice.AccountID.String(), + StripeInvoiceID: pc.Invoice.StripeInvoiceID, + Status: pc.Invoice.Status, + AmountDue: due, + AmountPaid: paid, + Currency: pc.Invoice.Currency, + PeriodStart: pgtype.Timestamptz{Time: pc.Invoice.PeriodStart, Valid: !pc.Invoice.PeriodStart.IsZero()}, + PeriodEnd: pgtype.Timestamptz{Time: pc.Invoice.PeriodEnd, Valid: !pc.Invoice.PeriodEnd.IsZero()}, + }); err != nil { + return 0, "", err + } + if err := qtx.UpsertProrationBaseSnapshot(ctx, db.UpsertProrationBaseSnapshotParams{ + AppID: pc.Snapshot.AppID.String(), + PeriodStart: pc.Snapshot.PeriodStart, + PeriodEnd: pc.Snapshot.PeriodEnd, + ModuleCount: int32(pc.Snapshot.ModuleCount), //nolint:gosec // count comes from the locked apps row, whose writers validate 0 ≤ count ≤ maxModuleCount + BaseMicros: pc.Snapshot.BaseMicros, + }); err != nil { + return 0, "", err + } + // Arm the one-shot guard. First-write-wins (WHERE proration_invoice_id IS + // NULL): a concurrent second attempt for the same app affects 0 rows here + // and keeps the winner's (identical, by construction) invoice id. + if _, err := qtx.SetAppProrationInvoice(ctx, db.SetAppProrationInvoiceParams{ + AppID: appID.String(), + ProrationInvoiceID: pgtype.Text{String: pc.InvoiceID, Valid: true}, + }); err != nil { + return 0, "", err + } + + if err := tx.Commit(ctx); err != nil { + return 0, "", err + } + return ProrationLockedCharged, pc.InvoiceID, nil +} + +func (s *pgxStore) ChargeProrationLocked(ctx context.Context, appID uuid.UUID, charge func(AppMirror) (*ProrationCharge, error)) (ProrationOutcome, string, error) { + // Phase 1: lock just long enough to read + verify chargeable state, then + // release — never held across the Stripe call below. + locked, outcome, invID, proceed, err := s.lockAndReadChargeableApp(ctx, appID) + if err != nil { + return 0, "", err + } + if !proceed { + return outcome, invID, nil + } + + // Phase 2: the Stripe network calls, OUTSIDE any lock or transaction. + pc, err := charge(locked) + if err != nil { + return 0, "", err // guard unarmed → the next sweep retries (idem keys) + } + if pc == nil { + return ProrationLockedNoCharge, "", nil // 0 cents — nothing to invoice + } + + // Phase 3: persist the successful charge. + return s.persistProrationCharge(ctx, appID, pc) +} + func (s *pgxStore) SetAppProrationInvoice(ctx context.Context, appID uuid.UUID, stripeInvoiceID string) error { // 0 rows = the guard was already armed (first-charge-wins) — not an error: // the deterministic Stripe idem keys guarantee the concurrent charger @@ -859,6 +1072,13 @@ func (s *pgxStore) SetAppProrationInvoice(ctx context.Context, appID uuid.UUID, return err } +func (s *pgxStore) SetAppProrationSkipped(ctx context.Context, appID uuid.UUID) error { + // 0 rows = already marked, or already charged in the meantime — neither is + // an error: the marker is a one-shot, first-write-wins terminal state. + _, err := s.q.SetAppProrationSkipped(ctx, appID.String()) + return err +} + func (s *pgxStore) SetAppModuleCount(ctx context.Context, appID uuid.UUID, moduleCount int) error { // 0 rows = the app is deleted (count frozen, D1e); existence was already // checked by the service via AppMirror, so this is a legitimate no-op. diff --git a/internal/account/cycle/store_prorationlock_integration_test.go b/internal/account/cycle/store_prorationlock_integration_test.go new file mode 100644 index 0000000..8c4a252 --- /dev/null +++ b/internal/account/cycle/store_prorationlock_integration_test.go @@ -0,0 +1,142 @@ +//go:build integration + +package cycle_test + +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/shared/testutil" +) + +// ChargeProrationLocked (finding 3, store.go) — verifies against a REAL +// Postgres that the row lock is released BEFORE the (simulated) Stripe network +// call, not held across it: a concurrent SyncAppModules/MarkAppDeleted write +// to the SAME row must complete promptly while the charge callback is still +// in flight, never blocking behind it. + +func mkProrationCharge(acct, appID uuid.UUID, invID string, periodEnd time.Time) *cycle.ProrationCharge { + return &cycle.ProrationCharge{ + InvoiceID: invID, + Cents: 200, + Invoice: cycle.InvoiceMirror{ + AccountID: acct, StripeInvoiceID: invID, Status: "open", + AmountDueCents: 200, Currency: "usd", + PeriodStart: periodEnd.AddDate(0, 0, -3), PeriodEnd: periodEnd, + }, + Snapshot: cycle.AppBaseSnapshot{ + AppID: appID, PeriodStart: periodEnd.AddDate(0, -1, 0), PeriodEnd: periodEnd, + ModuleCount: 0, BaseMicros: 2_000_000, + }, + } +} + +func TestChargeProrationLocked_Integration_LockNotHeldAcrossStripeCall(t *testing.T) { + pool := testutil.NewTestDB(t) + store := cycle.NewStore(pool) + ctx := context.Background() + + acct := seedAccount(t, pool) + appID := uuid.New() + require.NoError(t, store.InsertAppMirror(ctx, appID, acct, 0, mustTime(t, "2026-07-01T08:00:00Z"))) + + insideCallback := make(chan struct{}) + release := make(chan struct{}) + chargeDone := make(chan error, 1) + + go func() { + _, _, err := store.ChargeProrationLocked(ctx, appID, func(locked cycle.AppMirror) (*cycle.ProrationCharge, error) { + close(insideCallback) // signal: the phase-1 lock has already been read + released + <-release // simulate a slow Stripe HTTP call + return mkProrationCharge(acct, appID, "in_slow", mustTime(t, "2026-07-04T00:00:00Z")), nil + }) + chargeDone <- err + }() + + select { + case <-insideCallback: + case <-time.After(5 * time.Second): + t.Fatal("charge callback never started") + } + + // A concurrent write to the SAME row must complete promptly — it must NOT + // be blocked behind the "Stripe call" the callback is simulating. Under the + // pre-fix code (the lock held across the callback via one long-lived FOR + // UPDATE transaction) this write would hang until `release` is closed. + writeDone := make(chan error, 1) + go func() { writeDone <- store.SetAppModuleCount(ctx, appID, 42) }() + + select { + case err := <-writeDone: + require.NoError(t, err) + case <-time.After(2 * time.Second): + t.Fatal("SetAppModuleCount blocked behind the in-flight charge callback — the row lock is still held across it") + } + + close(release) + require.NoError(t, <-chargeDone) + + app, _, err := store.AppMirror(ctx, appID) + require.NoError(t, err) + require.Equal(t, 42, app.ModuleCount, "the concurrent write landed") + require.Equal(t, "in_slow", app.ProrationInvoiceID, "the charge still committed after release") +} + +func TestChargeProrationLocked_Integration_ConcurrentDeleteDoesNotBlockOnLock(t *testing.T) { + // Same shape as above, using MarkAppDeleted (the specific writer finding 3 + // calls out — SyncAppModules' soft-delete path) as the concurrent write. + pool := testutil.NewTestDB(t) + store := cycle.NewStore(pool) + ctx := context.Background() + + acct := seedAccount(t, pool) + appID := uuid.New() + require.NoError(t, store.InsertAppMirror(ctx, appID, acct, 0, mustTime(t, "2026-07-01T08:00:00Z"))) + + insideCallback := make(chan struct{}) + release := make(chan struct{}) + chargeDone := make(chan error, 1) + + go func() { + _, _, err := store.ChargeProrationLocked(ctx, appID, func(locked cycle.AppMirror) (*cycle.ProrationCharge, error) { + close(insideCallback) + <-release + return mkProrationCharge(acct, appID, "in_slow_del", mustTime(t, "2026-07-04T00:00:00Z")), nil + }) + chargeDone <- err + }() + + select { + case <-insideCallback: + case <-time.After(5 * time.Second): + t.Fatal("charge callback never started") + } + + deleteDone := make(chan error, 1) + go func() { deleteDone <- store.MarkAppDeleted(ctx, appID) }() + + select { + case err := <-deleteDone: + require.NoError(t, err) + case <-time.After(2 * time.Second): + t.Fatal("MarkAppDeleted blocked behind the in-flight charge callback — the row lock is still held across it") + } + + close(release) + require.NoError(t, <-chargeDone) + + // Judgment call (documented in store.go / proration.go and the PR + // description): once the Stripe charge has already succeeded, a delete that + // raced in during the (now-released) window does NOT unwind it — D1e + // already forbids refunds, and the money has already moved. The invoice / + // snapshot / guard are persisted regardless of the app's deleted_at. + app, _, err := store.AppMirror(ctx, appID) + require.NoError(t, err) + require.True(t, app.Deleted, "the delete committed") + require.Equal(t, "in_slow_del", app.ProrationInvoiceID, "the already-succeeded Stripe charge is still recorded, not silently dropped") +} diff --git a/internal/account/db/apps.sql.go b/internal/account/db/apps.sql.go index 140c140..c846c4a 100644 --- a/internal/account/db/apps.sql.go +++ b/internal/account/db/apps.sql.go @@ -12,6 +12,45 @@ import ( "github.com/jackc/pgx/v5/pgtype" ) +const appsPendingProration = `-- name: AppsPendingProration :many +SELECT app_id +FROM ms_billing.apps +WHERE created_at <= $1 + AND proration_invoice_id IS NULL + AND deleted_at IS NULL + AND proration_skipped_at IS NULL +ORDER BY created_at +` + +// AppsPendingProration is the creation-proration sweep's work list: apps that +// have survived the grace window ($1 = now() − GraceDays) and were never charged +// their creation-period base. proration_invoice_id IS NULL is the one-shot guard +// (an already-charged app drops out); deleted_at IS NULL excludes apps +// soft-deleted within grace (they are NEVER charged); proration_skipped_at IS +// NULL excludes apps permanently skipped as a would-be retroactive catch-up +// (migration 031, D1d). Ordered by created_at so the oldest pending app +// charges first. Backed by the partial index apps_pending_proration_idx +// (migrations 029/031). +func (q *Queries) AppsPendingProration(ctx context.Context, createdAt time.Time) ([]string, error) { + rows, err := q.db.Query(ctx, appsPendingProration, createdAt) + if err != nil { + return nil, err + } + defer rows.Close() + items := []string{} + for rows.Next() { + var app_id string + if err := rows.Scan(&app_id); err != nil { + return nil, err + } + items = append(items, app_id) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const insertAdvanceBaseSnapshot = `-- name: InsertAdvanceBaseSnapshot :execrows INSERT INTO ms_billing.app_base_snapshots (app_id, period_start, period_end, module_count, base_micros, source) @@ -49,8 +88,8 @@ func (q *Queries) InsertAdvanceBaseSnapshot(ctx context.Context, arg InsertAdvan const insertAppMirror = `-- name: InsertAppMirror :execrows -INSERT INTO ms_billing.apps (app_id, account_id, module_count, created_at) -VALUES ($1, $2, $3, $4) +INSERT INTO ms_billing.apps (app_id, account_id, module_count, created_module_count, created_at) +VALUES ($1, $2, $3, $3, $4) ON CONFLICT (app_id) DO NOTHING ` @@ -67,10 +106,14 @@ type InsertAppMirrorParams struct { // the installed-module-count snapshot, and the one-shot proration guard. // InsertAppMirror registers an app row idempotently: ON CONFLICT (app_id) DO // NOTHING so a RegisterApp retry (or a concurrent double-fire) never rewrites -// created_at / module_count / the proration guard of the original insert — -// the FIRST registration's values are the stable proration anchor. :execrows -// so the caller can tell a fresh insert (1) from a retry no-op (0), though -// both are success. +// created_at / module_count / created_module_count / the proration guard of +// the original insert — the FIRST registration's values are the stable +// proration anchor. created_module_count is stamped from the SAME $3 value as +// module_count and NEVER written again by any other query (migration 030) — +// it is the frozen count ChargeCreationProration prices its historical window +// from, immune to a later SyncAppModules install/uninstall during grace. +// :execrows so the caller can tell a fresh insert (1) from a retry no-op (0), +// though both are success. func (q *Queries) InsertAppMirror(ctx context.Context, arg InsertAppMirrorParams) (int64, error) { result, err := q.db.Exec(ctx, insertAppMirror, arg.AppID, @@ -234,7 +277,8 @@ func (q *Queries) SelectAppBaseSnapshot(ctx context.Context, arg SelectAppBaseSn } const selectAppMirror = `-- name: SelectAppMirror :one -SELECT app_id, account_id, module_count, created_at, proration_invoice_id, deleted_at +SELECT app_id, account_id, module_count, created_module_count, created_at, + proration_invoice_id, proration_skipped_at, deleted_at FROM ms_billing.apps WHERE app_id = $1 ` @@ -243,8 +287,10 @@ type SelectAppMirrorRow struct { AppID string `json:"app_id"` AccountID string `json:"account_id"` ModuleCount int32 `json:"module_count"` + CreatedModuleCount int32 `json:"created_module_count"` CreatedAt time.Time `json:"created_at"` ProrationInvoiceID pgtype.Text `json:"proration_invoice_id"` + ProrationSkippedAt pgtype.Timestamptz `json:"proration_skipped_at"` DeletedAt pgtype.Timestamptz `json:"deleted_at"` } @@ -258,8 +304,53 @@ func (q *Queries) SelectAppMirror(ctx context.Context, appID string) (SelectAppM &i.AppID, &i.AccountID, &i.ModuleCount, + &i.CreatedModuleCount, + &i.CreatedAt, + &i.ProrationInvoiceID, + &i.ProrationSkippedAt, + &i.DeletedAt, + ) + return i, err +} + +const selectAppMirrorForUpdate = `-- name: SelectAppMirrorForUpdate :one +SELECT app_id, account_id, module_count, created_module_count, created_at, + proration_invoice_id, proration_skipped_at, deleted_at +FROM ms_billing.apps +WHERE app_id = $1 +FOR UPDATE +` + +type SelectAppMirrorForUpdateRow struct { + AppID string `json:"app_id"` + AccountID string `json:"account_id"` + ModuleCount int32 `json:"module_count"` + CreatedModuleCount int32 `json:"created_module_count"` + CreatedAt time.Time `json:"created_at"` + ProrationInvoiceID pgtype.Text `json:"proration_invoice_id"` + ProrationSkippedAt pgtype.Timestamptz `json:"proration_skipped_at"` + DeletedAt pgtype.Timestamptz `json:"deleted_at"` +} + +// SelectAppMirrorForUpdate reads one roster row under a ROW LOCK (FOR UPDATE) — +// the creation-proration charge's race-safety primitive. The charge locks the +// row here just long enough to re-verify deleted_at IS NULL and +// proration_invoice_id IS NULL and read the frozen created_module_count, +// releasing the lock immediately after (ChargeProrationLocked runs the actual +// Stripe network call OUTSIDE this lock — see store.go); a concurrent +// SyncAppModules soft-delete (MarkAppDeleted) only ever contends for the brief +// read, never for the duration of a Stripe HTTP call. +func (q *Queries) SelectAppMirrorForUpdate(ctx context.Context, appID string) (SelectAppMirrorForUpdateRow, error) { + row := q.db.QueryRow(ctx, selectAppMirrorForUpdate, appID) + var i SelectAppMirrorForUpdateRow + err := row.Scan( + &i.AppID, + &i.AccountID, + &i.ModuleCount, + &i.CreatedModuleCount, &i.CreatedAt, &i.ProrationInvoiceID, + &i.ProrationSkippedAt, &i.DeletedAt, ) return i, err @@ -315,6 +406,29 @@ func (q *Queries) SetAppProrationInvoice(ctx context.Context, arg SetAppProratio return result.RowsAffected(), nil } +const setAppProrationSkipped = `-- name: SetAppProrationSkipped :execrows +UPDATE ms_billing.apps +SET proration_skipped_at = now() +WHERE app_id = $1 + AND proration_skipped_at IS NULL + AND proration_invoice_id IS NULL +` + +// SetAppProrationSkipped arms the PERMANENT skip marker (migration 031, D1d): +// the account only activated at/after this app's anchored creation period had +// already closed, so charging it now would be a retroactive catch-up. The +// WHERE clause makes it idempotent (a re-evaluation, or a concurrent one, +// affects 0 rows once set) and defensively refuses to mark an app that was +// somehow already charged in the meantime. :execrows so the caller can +// observe (and tolerate) the already-set / already-charged cases. +func (q *Queries) SetAppProrationSkipped(ctx context.Context, appID string) (int64, error) { + result, err := q.db.Exec(ctx, setAppProrationSkipped, appID) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + const upsertProrationBaseSnapshot = `-- name: UpsertProrationBaseSnapshot :exec INSERT INTO ms_billing.app_base_snapshots (app_id, period_start, period_end, module_count, base_micros, source) diff --git a/internal/account/db/models.go b/internal/account/db/models.go index d9e8e21..83ae068 100644 --- a/internal/account/db/models.go +++ b/internal/account/db/models.go @@ -352,6 +352,10 @@ type MsBillingApp struct { ProrationInvoiceID pgtype.Text `json:"proration_invoice_id"` DeletedAt pgtype.Timestamptz `json:"deleted_at"` UpdatedAt time.Time `json:"updated_at"` + // Module count frozen at RegisterApp time (immutable — SyncAppModules never writes this column). ChargeCreationProration prices the creation-period window off THIS value, never the live module_count. + CreatedModuleCount int32 `json:"created_module_count"` + // Set once (never unset) when ChargeCreationProration determines the account only activated at/after this app's anchored creation period had already closed — a would-be retroactive catch-up charge (D1d). The app is permanently excluded from the proration sweep from then on. + ProrationSkippedAt pgtype.Timestamptz `json:"proration_skipped_at"` } type MsBillingAppBaseSnapshot struct { diff --git a/internal/account/db/queries/apps.sql b/internal/account/db/queries/apps.sql index e827cfd..d70997d 100644 --- a/internal/account/db/queries/apps.sql +++ b/internal/account/db/queries/apps.sql @@ -5,23 +5,61 @@ -- InsertAppMirror registers an app row idempotently: ON CONFLICT (app_id) DO -- NOTHING so a RegisterApp retry (or a concurrent double-fire) never rewrites --- created_at / module_count / the proration guard of the original insert — --- the FIRST registration's values are the stable proration anchor. :execrows --- so the caller can tell a fresh insert (1) from a retry no-op (0), though --- both are success. +-- created_at / module_count / created_module_count / the proration guard of +-- the original insert — the FIRST registration's values are the stable +-- proration anchor. created_module_count is stamped from the SAME $3 value as +-- module_count and NEVER written again by any other query (migration 030) — +-- it is the frozen count ChargeCreationProration prices its historical window +-- from, immune to a later SyncAppModules install/uninstall during grace. +-- :execrows so the caller can tell a fresh insert (1) from a retry no-op (0), +-- though both are success. -- name: InsertAppMirror :execrows -INSERT INTO ms_billing.apps (app_id, account_id, module_count, created_at) -VALUES ($1, $2, $3, $4) +INSERT INTO ms_billing.apps (app_id, account_id, module_count, created_module_count, created_at) +VALUES ($1, $2, $3, $3, $4) ON CONFLICT (app_id) DO NOTHING; -- SelectAppMirror reads one roster row (deleted or not — the caller decides -- what deletion means for its path: SyncAppModules no-ops a count update, -- GetAppBill still displays the spent creation-period base). -- name: SelectAppMirror :one -SELECT app_id, account_id, module_count, created_at, proration_invoice_id, deleted_at +SELECT app_id, account_id, module_count, created_module_count, created_at, + proration_invoice_id, proration_skipped_at, deleted_at FROM ms_billing.apps WHERE app_id = $1; +-- SelectAppMirrorForUpdate reads one roster row under a ROW LOCK (FOR UPDATE) — +-- the creation-proration charge's race-safety primitive. The charge locks the +-- row here just long enough to re-verify deleted_at IS NULL and +-- proration_invoice_id IS NULL and read the frozen created_module_count, +-- releasing the lock immediately after (ChargeProrationLocked runs the actual +-- Stripe network call OUTSIDE this lock — see store.go); a concurrent +-- SyncAppModules soft-delete (MarkAppDeleted) only ever contends for the brief +-- read, never for the duration of a Stripe HTTP call. +-- name: SelectAppMirrorForUpdate :one +SELECT app_id, account_id, module_count, created_module_count, created_at, + proration_invoice_id, proration_skipped_at, deleted_at +FROM ms_billing.apps +WHERE app_id = $1 +FOR UPDATE; + +-- AppsPendingProration is the creation-proration sweep's work list: apps that +-- have survived the grace window ($1 = now() − GraceDays) and were never charged +-- their creation-period base. proration_invoice_id IS NULL is the one-shot guard +-- (an already-charged app drops out); deleted_at IS NULL excludes apps +-- soft-deleted within grace (they are NEVER charged); proration_skipped_at IS +-- NULL excludes apps permanently skipped as a would-be retroactive catch-up +-- (migration 031, D1d). Ordered by created_at so the oldest pending app +-- charges first. Backed by the partial index apps_pending_proration_idx +-- (migrations 029/031). +-- name: AppsPendingProration :many +SELECT app_id +FROM ms_billing.apps +WHERE created_at <= $1 + AND proration_invoice_id IS NULL + AND deleted_at IS NULL + AND proration_skipped_at IS NULL +ORDER BY created_at; + -- SetAppProrationInvoice arms the ONE-SHOT proration guard: it records the -- Stripe invoice id of the creation-proration charge, and the WHERE -- proration_invoice_id IS NULL makes the write first-charge-wins — a retry or @@ -33,6 +71,20 @@ SET proration_invoice_id = $2 WHERE app_id = $1 AND proration_invoice_id IS NULL; +-- SetAppProrationSkipped arms the PERMANENT skip marker (migration 031, D1d): +-- the account only activated at/after this app's anchored creation period had +-- already closed, so charging it now would be a retroactive catch-up. The +-- WHERE clause makes it idempotent (a re-evaluation, or a concurrent one, +-- affects 0 rows once set) and defensively refuses to mark an app that was +-- somehow already charged in the meantime. :execrows so the caller can +-- observe (and tolerate) the already-set / already-charged cases. +-- name: SetAppProrationSkipped :execrows +UPDATE ms_billing.apps +SET proration_skipped_at = now() +WHERE app_id = $1 + AND proration_skipped_at IS NULL + AND proration_invoice_id IS NULL; + -- SetAppModuleCount snapshots a new installed-module count (SyncAppModules). -- WHERE deleted_at IS NULL makes a count update on a deleted app a no-op -- (D1e — a deleted app accrues no future base, so its count is frozen). diff --git a/internal/account/usage/account_bill_integration_test.go b/internal/account/usage/account_bill_integration_test.go index ff58f6a..200904f 100644 --- a/internal/account/usage/account_bill_integration_test.go +++ b/internal/account/usage/account_bill_integration_test.go @@ -31,8 +31,8 @@ func seedMirrorApp(t *testing.T, pool *pgxpool.Pool, acct, app uuid.UUID, create del = deletedAt } _, err := pool.Exec(context.Background(), - `INSERT INTO ms_billing.apps (app_id, account_id, module_count, created_at, deleted_at) - VALUES ($1, $2, 0, $3, $4)`, + `INSERT INTO ms_billing.apps (app_id, account_id, module_count, created_module_count, created_at, deleted_at) + VALUES ($1, $2, 0, 0, $3, $4)`, app.String(), acct.String(), createdAt, del) require.NoError(t, err) } diff --git a/internal/account/usage/bill.go b/internal/account/usage/bill.go index d33d262..61893e3 100644 --- a/internal/account/usage/bill.go +++ b/internal/account/usage/bill.go @@ -61,6 +61,16 @@ const ( // Only reached once a subscription earns it; subscription-gated OFF in v1 (see // paasCreditMicros), so today the credit is always 0. PaasCreditPct = 30 + + // GraceDays is the creation grace window (owner spec 2026-07-05, D1e + // follow-up). A newly created app is NOT charged its creation-period base + // synchronously; a periodic sweep (cmd/billing-cycle) charges it only once it + // has SURVIVED this many whole days past created_at, so an app soft-deleted + // within the window is NEVER billed. A survivor is charged the SAME + // creation-period proration (identical ProratedBaseMicros math, anchored to + // the TRUE created_at) — grace delays WHEN the charge fires, never WHAT it + // covers. Tunable. + GraceDays = 3 ) // Plan is the account/org billing plan. v1 has NO real plan system — this is the diff --git a/migrations/billing/029_apps_proration_sweep_idx.down.sql b/migrations/billing/029_apps_proration_sweep_idx.down.sql new file mode 100644 index 0000000..4c16c2e --- /dev/null +++ b/migrations/billing/029_apps_proration_sweep_idx.down.sql @@ -0,0 +1 @@ +DROP INDEX IF EXISTS ms_billing.apps_pending_proration_idx; diff --git a/migrations/billing/029_apps_proration_sweep_idx.up.sql b/migrations/billing/029_apps_proration_sweep_idx.up.sql new file mode 100644 index 0000000..aa934b2 --- /dev/null +++ b/migrations/billing/029_apps_proration_sweep_idx.up.sql @@ -0,0 +1,24 @@ +-- Migration 029 — index for the creation-proration grace sweep. +-- +-- Creation grace (owner spec 2026-07-05, D1e follow-up): a newly created app is +-- no longer charged its creation-period base synchronously in RegisterApp. A +-- periodic sweep (cmd/billing-cycle) instead charges apps that have SURVIVED the +-- grace window, so an app soft-deleted within grace is never billed. The sweep's +-- work list is: +-- +-- SELECT app_id FROM ms_billing.apps +-- WHERE created_at <= now() - '3 days' +-- AND proration_invoice_id IS NULL -- one-shot guard: not yet charged +-- AND deleted_at IS NULL -- excludes apps deleted within grace +-- +-- A PARTIAL index on created_at, restricted to the exact NULL predicates the +-- sweep filters on, keeps the index tiny (it holds ONLY the still-pending apps — +-- every charged or deleted app drops out) and lets the sweep range-scan by +-- created_at without touching the full roster. It is preferred over a plain +-- composite (deleted_at, proration_invoice_id, created_at) because almost every +-- row is eventually charged (proration_invoice_id set) and thus excluded from +-- the partial index, so it stays a fraction of the table's size. + +CREATE INDEX IF NOT EXISTS apps_pending_proration_idx + ON ms_billing.apps (created_at) + WHERE deleted_at IS NULL AND proration_invoice_id IS NULL; diff --git a/migrations/billing/030_apps_created_module_count.down.sql b/migrations/billing/030_apps_created_module_count.down.sql new file mode 100644 index 0000000..316524c --- /dev/null +++ b/migrations/billing/030_apps_created_module_count.down.sql @@ -0,0 +1,6 @@ +-- Down for 030 — drop the frozen creation-time module-count column. Reverting +-- returns ChargeCreationProration to pricing off the live module_count (the +-- pre-030, retroactive-tier-drift posture). + +ALTER TABLE ms_billing.apps + DROP COLUMN IF EXISTS created_module_count; diff --git a/migrations/billing/030_apps_created_module_count.up.sql b/migrations/billing/030_apps_created_module_count.up.sql new file mode 100644 index 0000000..7ae2ddd --- /dev/null +++ b/migrations/billing/030_apps_created_module_count.up.sql @@ -0,0 +1,40 @@ +-- Migration 030 — freeze the module count the creation-proration charge prices +-- (review finding, creation-grace PR #46). +-- +-- ms_billing.apps.module_count is a LIVE snapshot: SyncAppModules overwrites it +-- on every install/uninstall. Under creation grace (migration 029) the +-- creation-proration charge no longer fires synchronously at RegisterApp — it +-- is delayed until the app survives GraceDays and the sweep gets to it, which +-- can be days after creation. A customer is free to install/uninstall modules +-- via SyncAppModules during that window, so by the time the sweep reads +-- module_count it may no longer be the count that actually applied on the +-- historical creation-period days being priced — the charge would retroactively +-- apply a tier that never existed on those days. +-- +-- created_module_count freezes the count AT REGISTRATION (RegisterApp's INSERT +-- — the same instant that stamps created_at) and is NEVER touched again: it is +-- absent from every SyncAppModules write path. The creation-proration charge +-- (ChargeCreationProration) prices its historical window off THIS column; +-- module_count remains exactly what it was — the LIVE count the boundary +-- advance leg (and the display read for all FUTURE periods) uses. +-- +-- Backfill: for rows that predate this migration, the live module_count is the +-- best available approximation of what applied at creation (no per-day history +-- exists to reconstruct it precisely) — acceptable because every pre-migration +-- row not yet proration-charged, if any, is still within its grace/pending +-- window and its module_count has had comparatively little time to drift. + +ALTER TABLE ms_billing.apps + ADD COLUMN created_module_count INT NULL CHECK (created_module_count >= 0); + +UPDATE ms_billing.apps + SET created_module_count = module_count + WHERE created_module_count IS NULL; + +ALTER TABLE ms_billing.apps + ALTER COLUMN created_module_count SET NOT NULL; + +COMMENT ON COLUMN ms_billing.apps.created_module_count IS + 'Module count frozen at RegisterApp time (immutable — SyncAppModules never ' + 'writes this column). ChargeCreationProration prices the creation-period ' + 'window off THIS value, never the live module_count.'; diff --git a/migrations/billing/031_apps_proration_skipped.down.sql b/migrations/billing/031_apps_proration_skipped.down.sql new file mode 100644 index 0000000..3b59e3d --- /dev/null +++ b/migrations/billing/031_apps_proration_skipped.down.sql @@ -0,0 +1,12 @@ +-- Down for 031 — drop the permanent-skip marker and restore the migration-029 +-- partial index predicate (deleted_at IS NULL AND proration_invoice_id IS +-- NULL only). + +DROP INDEX IF EXISTS ms_billing.apps_pending_proration_idx; + +CREATE INDEX IF NOT EXISTS apps_pending_proration_idx + ON ms_billing.apps (created_at) + WHERE deleted_at IS NULL AND proration_invoice_id IS NULL; + +ALTER TABLE ms_billing.apps + DROP COLUMN IF EXISTS proration_skipped_at; diff --git a/migrations/billing/031_apps_proration_skipped.up.sql b/migrations/billing/031_apps_proration_skipped.up.sql new file mode 100644 index 0000000..d2165e2 --- /dev/null +++ b/migrations/billing/031_apps_proration_skipped.up.sql @@ -0,0 +1,46 @@ +-- Migration 031 — permanent-skip marker for the creation-proration charge +-- (review finding, creation-grace PR #46, D1d). +-- +-- D1d: v1 never retroactively catches up an app's creation-period base once +-- the window it covers has already closed with the account never having been +-- chargeable during it (never activated). Pre-grace, RegisterApp charged +-- synchronously at creation, so this could only happen if activation itself +-- landed after the creation period ended — and the code skipped the charge +-- (0 cents, guard left unarmed) in exactly that case. +-- +-- Creation grace (migration 029) removed that gate outright when it moved the +-- charge into the async sweep, reasoning (wrongly, in general) that the +-- creation period is billed by no other leg so charging it whenever the guard +-- is unarmed "can never double-bill". That reasoning misses D1d: an app whose +-- account sat unactivated across the app's ENTIRE creation period and only +-- activates months later would, on the very next sweep, be retroactively +-- billed for a period it was never eligible to be charged for. +-- +-- proration_skipped_at records that this decision was made and is PERMANENT: +-- once an app's creation-proration charge is determined to be a would-be +-- retroactive catch-up (the account only activated at/after the app's +-- anchored creation period had already closed), the app is marked here and +-- dropped from the sweep's pending work list for good — never charged, and +-- never re-evaluated on every future sweep (which would otherwise happen +-- forever, since proration_invoice_id would stay NULL). +-- +-- The apps_pending_proration_idx partial index (migration 029) is redefined to +-- add the same predicate so the sweep's work-list query keeps hitting the +-- index. + +ALTER TABLE ms_billing.apps + ADD COLUMN proration_skipped_at TIMESTAMPTZ NULL; + +COMMENT ON COLUMN ms_billing.apps.proration_skipped_at IS + 'Set once (never unset) when ChargeCreationProration determines the ' + 'account only activated at/after this app''s anchored creation period had ' + 'already closed — a would-be retroactive catch-up charge (D1d). The app ' + 'is permanently excluded from the proration sweep from then on.'; + +DROP INDEX IF EXISTS ms_billing.apps_pending_proration_idx; + +CREATE INDEX IF NOT EXISTS apps_pending_proration_idx + ON ms_billing.apps (created_at) + WHERE deleted_at IS NULL + AND proration_invoice_id IS NULL + AND proration_skipped_at IS NULL;