feat: unified base-fee + per-module overage timer model (supersedes #45, #46, #47)#48
Merged
Merged
Conversation
Add a post-hoc TRANSPARENCY flag for off-session charges that ALREADY succeeded, once they cross a per-account size threshold — so customers aren't surprised by a large automatic debit. This changes NO charging behaviour; it is complementary to (and distinct from) the spend-ceiling gate, which SKIPS a charge before it fires. - migration 031: accounts.auto_collect_threshold_micros (BIGINT NULL, NULL = $100 default) + invoices.is_large_auto_collect (BOOLEAN NOT NULL DEFAULT false). - collection.DefaultAutoCollectThresholdMicros ($100) + ResolveAutoCollectThreshold + IsLargeAutoCollect (strict-> : exactly-at threshold is NOT flagged, matching ExceedsSpendCeiling's dollar-cap reading). - Server-computed at charge time (threshold resolved when the charge fires, not later) in BOTH off-session charge legs: RunBillingCycle (arrears + advance base) and RegisterApp proration. - Wired through InvoiceMirror/UpsertInvoice and exposed on the wire as InvoiceRow.is_large_auto_collect (additive) for the web billing page. - Tests: collection flag logic (below/above/at/override/NULL) + cycle wiring (large flags, small doesn't, per-account override respected). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A newly created app is no longer charged its creation-period base synchronously in RegisterApp. It enters a GraceDays (=3) grace window and is charged only once it has SURVIVED it, so an app soft-deleted within grace is NEVER billed. A survivor pays the SAME creation-period proration (identical ProratedBaseMicros math, anchored to the TRUE created_at) — grace delays WHEN the charge fires, never WHAT it covers. - RegisterApp now ONLY mirrors the ms_billing.apps row (account, created_at, module_count). It never calls Stripe. Response fields kept for wire back-compat (ProrationCents always 0; ProrationInvoiceID echoes the armed guard on a post-sweep retry). - New Service.ChargeCreationProration(appID): the extracted charge leg (same Stripe plumbing, micros→cents boundary, invoice mirror, and migration-028 base snapshot the old inline RegisterApp charge used), keyed by app id with the same one-shot proration_invoice_id guard. - New Service.SweepCreationProrations(at): lists apps past grace (created_at <= at − GraceDays, guard unarmed, deleted_at IS NULL) and charges each. Wired into cmd/billing-cycle alongside the per-account boundary loop (both transports). - Race safety: ChargeProrationLocked runs the not-deleted re-check, the Stripe charge, and the guard-arm inside ONE transaction holding a SELECT ... FOR UPDATE row lock, so a concurrent soft-delete and the charge are mutually exclusive (no refund path, D1e). Whichever commits first wins; the loser no-ops. - Delayed billing correctly still charges an app whose creation period has since closed: that period is billed by NO other leg (the boundary advance leg only ever bills an app's SUBSEQUENT periods), so it never double-bills. - Migration 029: partial index apps_pending_proration_idx on (created_at) WHERE deleted_at IS NULL AND proration_invoice_id IS NULL — the sweep's work-list stays a fraction of the roster. - GraceDays const in internal/account/usage/bill.go alongside the pricing tier consts. Tests: RegisterApp mirror-only (never charges); sweep grace-holds / deleted-within-grace-never-charged / survivor-charged-once-across-reruns; proration $ math unchanged (200¢, 1300¢ overage, full-base-on-boundary); gates + idempotency; integration (build-tagged) for the AppsPendingProration filter and the FOR UPDATE ChargeProrationLocked semantics. make test / go vet / go build / gofmt clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move module overage from PER-APP to a single ACCOUNT-WIDE POOL of 5 included modules, per the owner spec 2026-07-05 (a deliberate reversal of the base-fee v1 per-app tier). - Schema (030): accounts.overage_since (one grace timer per account) + account_overage_snapshots (per-(account, period) charge ledger, the double-charge guard mirroring app_base_snapshots). - Pricing: per-app base is now FLAT $20; overage = $3 x max(0, SUM(live-app module_count) - 5) at the account level (usage.AccountOverageMicros / ProratedOverageMicros). - Timer: RegisterApp / SyncAppModules recompute the pool after every module_count write and arm/clear accounts.overage_since (first-cross-wins / idempotent clear, no refund on drop). - Mid-period grace charge: a new cmd/billing-cycle sweep charges the pooled overage prorated from grace-end to the period end once the 3-day grace window elapses (a deliberate mid-period charge; base-fee mid-period behavior is unchanged). Deterministic per-(account, period) Stripe idem keys + the snapshot ledger make it money-safe. - Boundary advance leg: each app now contributes only its flat base; a SEPARATE pooled-overage term bills the closing period ONCE, skipped when the sweep already billed it (snapshot guard), source='advance'. - Display: GetAccountBillResponse gains AccountOverageMicros (additive, snapshot-first else live pooled estimate); per-app base display is flat. Tests cover: pool crossing 5 arms the timer, dropping under clears it, grace holds for exactly 3 days, the mid-period charge fires once and is excluded from the boundary (no double-charge), interleaved installs sum to the account pool, and uninstall never refunds. mirrorstack-docs/db/ms_billing/ needs a companion update for the new account_overage_snapshots table + accounts.overage_since column. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Stripe only ever charges whole cents (round-half-up, cycle.centsFromMicros). IsLargeAutoCollect compared the raw pre-rounding micros amount against the threshold, so a charge strictly between the threshold and threshold+5,000 micros (half a cent) rounds DOWN to exactly the threshold's dollar amount when actually charged, yet was still flagged "large" — contradicting the "exactly at threshold is not large" contract. Round both the charged amount and the threshold to cents (the same conversion Stripe applies) before comparing, so the flag can never disagree with the money that actually moved. Corrects one existing test (TestIsLargeAutoCollect_ZeroOverrideFlagsAny...) whose old assertion baked in the same raw-micros-vs-charged-cents mismatch (a 1-micro "charge" rounds to $0.00 actually charged, so it must not be flagged over a $0 threshold either) — documented inline as a judgment call. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…legs RunBillingCycle resolved the account's auto_collect_threshold_micros near the top of the function — before the risk gate, the PM check, and both Stripe HTTP calls — while RegisterApp's proration leg already resolved it AFTER its own Stripe charge succeeded. A concurrent threshold edit landing mid-charge was therefore honored inconsistently between the two call sites, even though both are documented as resolving "at charge time". RunBillingCycle now re-resolves the threshold immediately after its Stripe charge succeeds (the same point RegisterApp already used), so a race lands identically on both legs. Also adds an end-to-end regression for the earlier IsLargeAutoCollect rounding fix, proving the concrete cents Stripe is charged (not just "no error"). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… findings) PR #47 adversarial review found the grace sweep and the boundary leg could both charge the same period's pooled overage if a crash landed between a successful Stripe call and the account_overage_snapshots write (disjoint Idempotency-Key namespaces, so Stripe never deduped it). Adds a status column ('pending'/'charged') to account_overage_snapshots: the claim is now written BEFORE either leg calls Stripe, so a crash anywhere leaves durable evidence the other leg must respect. Also fixes the boundary reclaim livelock (a reclaimed run recomputed the overage to 0 instead of reusing the frozen amount, colliding with its own stable Idempotency-Key) and threads the genuine Stripe invoice item id back from the boundary's combined charge instead of storing the idempotency-key string. Judgment call (no product decision available): pooled-overage growth after a period's grace charge now gets an incremental top-up, conservatively prorated from the sweep's own instant forward (never retroactive) — flagged in cycle/overage.go's topUpGraceOverage doc for owner review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Three confirmed, adversarially-verified findings on the creation-grace charge (PR #46, money-charging code; invariant D1d: no retroactive catch-up): 1. HIGH — ChargeCreationProration priced the ENTIRE historical creation-period window off module_count read FRESH at sweep time, days after creation. A customer installing/uninstalling modules via SyncAppModules during the mandatory grace window could retroactively move the tier applied to days that never had that module count. Fixed: migration 030 adds apps.created_module_count, frozen once at RegisterApp (InsertAppMirror stamps it from the SAME value as module_count) and never touched by any other writer. ChargeCreationProration now prices off created_module_count; module_count remains the live value the boundary advance leg (and the display estimate for future periods) uses. 2. HIGH — removing the "skip if the creation period already closed" gate reintroduced the retroactive catch-up billing D1d forbids: an app created while its account is unactivated sits correctly unbilled every sweep, but the moment the account activates months later, the next sweep charged it for a period that closed long ago. Fixed: migration 031 adds apps.proration_skipped_at, a one-shot PERMANENT marker. ChargeCreationProration now compares the account's activation anchor against the app's anchored creation-period end (not "now" — grace + ordinary sweep cadence can legitimately push a healthy, already-activated account's charge a few days past its own period end, and that must still charge normally); when the account only activated at/after the period closed, the charge is permanently skipped and the app never resurfaces on a later sweep (AppsPendingProration now also excludes proration_skipped_at). 3. MEDIUM — ChargeProrationLocked held a SELECT ... FOR UPDATE row lock across two live Stripe HTTP calls (up to ~160s), blocking any concurrent SyncAppModules/AppDeleted write to the same row. Its deferred tx.Rollback(ctx) also reused the request context, which can silently fail against an already-cancelled/expired ctx. Fixed: split into three phases — lock+read+release (a short tx), the Stripe calls OUTSIDE any lock, then persist (a second short tx). A concurrent delete that races in during the Stripe call no longer blocks, and — since the charge has already succeeded in Stripe by then — is recorded regardless (D1e already forbids refunds; the money moved). The deferred rollback now uses a short-lived detached context (context.WithoutCancel + a fresh timeout) so cleanup reaches Postgres even when the caller's context has died. Tests: unit regression tests for all three (frozen-count pricing both directions, permanent-skip vs. still-charges-when-activated-in-time, verified to fail without the fix by reverting each in isolation); DB integration tests for the SQL-layer freeze/skip semantics and the lock-not-held-across-Stripe-call + dead-context-rollback behavior (verified to fail/hang against the pre-fix code). Full migration up/down/up round-trip validated against a real Postgres. make test / go vet / go build / gofmt / integration suite (-race) all clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…migration 030→032
Resolves the merge of origin/feat/account-wide-module-overage on top of the
already-merged creation-grace stack (origin/fix/app-creation-grace-period).
Conflict resolution (both feature sets coexist):
- cmd/billing-cycle/main.go: log lines carry BOTH the creation-proration
sweep tallies AND the account-overage sweep tallies; runProrationSweep is
invoked alongside the boundary loop + overage sweep.
- cycle/apps.go RegisterApp: keeps creation-grace's no-synchronous-charge
semantics AND adds account-overage's recomputeAccountOverage timer arming.
- cycle/service_test.go: unions both branches' fake-store error hooks.
- cycle/apps_test.go: keeps mirror-only assertions; adapts the 7-module test.
Semantic reconciliation:
- cycle/proration.go: the creation proration now charges the FLAT per-app base
only (usage.BaseFeeMicros); the per-app AppBaseFeeMicros overage tier was
retired by the pooled-overage reversal. proration_test.go updated to flat-base
amounts (7 modules → 1000¢, not 1300¢); frozen created_module_count still
recorded on the snapshot for display but no longer moves the base.
Migration renumber (collision fix): both branches used slot 030. Creation-grace
keeps 029/030/031; account-overage's 030_account_wide_overage {up,down}.sql
renumbered to 032, with account-overage "migration 030" comments updated to 032.
sqlc regenerated. go build / vet / test / gofmt all clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…number migration 031→034 PR #45 (feat/auto-collect-disclosure) adds collection.IsLargeAutoCollect / AutoCollectThresholdMicros + the invoices.is_large_auto_collect mirror column (migration 034, renumbered from 031 to clear this branch's creation-grace 031_apps_proration_skipped + 032_account_wide_overage). Conflict resolution, preserving all three intents: - apps.go: kept this branch's creation-grace RegisterApp (mirror-only, no synchronous charge). #45's IsLargeAutoCollect wiring targeted the OLD synchronous-charge proration path that no longer lives in RegisterApp, so it is ported to proration.go's ChargeCreationProration callback instead (the threshold is resolved right after the Stripe call succeeds, matching the boundary leg). - models.go / *.sql.go: regenerated via sqlc from the merged migrations (032 account-wide overage + 034 auto-collect both present). - charge_test.go: the #45 RegisterApp post-charge-resolution test targeted the removed synchronous path; rewritten to drive ChargeCreationProration with the unified model's flat $20 base ($10 pre-charge vs $30 mid-charge threshold straddle), preserving the "resolved after Stripe succeeds" assertion. build / vet / test / gofmt all green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ion 033) Replace the account-wide single-timer pooled overage model (migration 032, superseded) with one independently-anchored grace timer per module INSTALL EVENT, each priced from its own install date. DESIGN.md "Base fee — v2: creation grace + per-module overage timers". Migration 033 (born clean; fully replaces 032's account-wide schema since it never shipped to main): - new table ms_billing.app_module_overage_timers (surrogate id, account_id, app_id, installed_at [FIFO key + proration anchor], grace_expires_at, removed_at, grace_charged_at, grace_resolved, grace_invoice_id, grace_invoice_item_id) + 3 partial indexes (live-FIFO, sweep work-list, LIFO removal); - up drops accounts.overage_since + account_overage_snapshots; down recreates them verbatim from 032 so up/down/up round-trips cleanly. Instance synthesis (RegisterApp / SyncAppModules, no api-platform change — the RPC still carries an integer module_count): - RegisterApp(count=K) inserts K timers anchored installed_at = created_at; - SyncAppModules N→M inserts M−N at now() (grow) or LIFO soft-removes N−M newest (shrink); app delete soft-removes all live timers. Reconcile against the LIVE timer count so both RPCs stay idempotent + self-healing on retry. Live FIFO + Leg 1 (cycle/overage.go, replaces the account-wide sweep entirely): - included-vs-over computed fresh at every grace-check via LiveModuleTimerRankBefore (order installed_at,id; first IncludedModules are included); - exploiting monotonicity, an "included" verdict is PERMANENT (grace_resolved) — never charged, never re-checked; - an "over" timer past its own grace is charged $3 prorated from installed_at's UTC day to its INSTALL period's end (install-anchored — the correction vs. the prior grace-elapse-anchored attempt; also keeps Leg 1 within the install period so scenario 6's boundary precharge can't double-bill), via a per-timer Stripe invoice (deterministic keys mod-overage-ii/inv-<timerID>), storing the REAL Stripe item id (not the idem-key string). Auto-collect disclosure flag (migration 034) resolved post-charge at this site too. charge.go boundary leg drops the pooled-overage term (base-only advance now); GetAccountBill's account-overage line shows the live steady-state estimate. cmd/billing-cycle drives SweepModuleOverage. Tests: scenario 4 exact numbers (two modules a day apart → $2.40 on June 13 and $2.30 on June 14, install-anchored proration), FIFO monotonicity / permanent inclusion, over→included flip-before-grace not charged, removed-in-grace never charged, no-PM skip+retry, unactivated never swept, LIFO shrink; plus a migration-033 integration test exercising the real timer SQL end-to-end. Full build / vet / test / gofmt green; sqlc regenerated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…scenario 3), shared auto-collect helper + scenario suite Completes the per-module overage timer model (DESIGN.md "Base fee — v2"): the mid-period grace charge (Leg 1) and the account creation grace shipped in Stage A; this adds the two remaining charge legs, unifies the disclosure flag, and switches the display to the timer table. No schema change — all new reads are window-function SELECTs over migration 033's app_module_overage_timers. Leg 2 — boundary precharge (charge.go / scenario 6): the boundary invoice now carries, alongside the closed period's usage arrears + the new period's flat advance base, a FULL $3-per-module precharge for every ONGOING over-module — a live install timer that is "over" per the live FIFO AND already charged at least once (grace_charged_at set). Counted per-row via CountOngoingOverModuleTimers (row_number() FIFO rank > IncludedModules AND grace_charged_at IS NOT NULL), pooled into the same one invoice, guarded by the same billing_run idempotency. A timer still inside its own grace (never charged) is excluded — it stays purely on Leg 1's timer and is never double-counted. New ChargeSummary.AdvanceOverageMicros. Scenario 3 — combined creation invoice (proration.go): when an app's creation grace charge fires, its co-created over-modules (installed_at == created_at, over per live FIFO, via CoCreatedOverModuleTimers) are billed as ADDITIONAL line items on the SAME CreateInvoice — one combined invoice, not two. Each is $3 prorated over the identical day-0→period-end window as the base, using the SAME per-timer idem keys Leg 1 would use (so a racing sweep can't double-charge), and marked charged in the SAME transaction that arms the app guard (ProrationCharge. TimerCharges, persisted by persistProrationCharge). cmd/billing-cycle now runs the proration sweep BEFORE the overage sweep so the combined invoice wins and Leg 1 skips the already-resolved co-created rows. Scenario 5 — one shared flagLargeAutoCollect(chargedMicros, acct) helper (charge.go) wraps collection.IsLargeAutoCollect and is called from ALL charge sites (creation/ combined, Leg 1 grace, boundary) — the disclosure flag is no longer reimplemented per leg. The combined/boundary flags reflect the FULL debit (base + every overage line). Display (accountbill.go): GetAccountBill's account-overage line reads the live install-timer count (CountLiveModuleTimersForAccount) instead of SUM(apps. module_count) — the overage model's source of truth. Removed the now-dead SumLiveModuleCount query + PooledModuleCount, and the dead ProratedOverageMicros (superseded by per-install ProratedBaseMicros) + its test. Tests: new end-to-end scenario suite (scenarios_test.go) — one test per scenario 1-6 with exact amounts/invoice-counts (scenario 3 asserts exactly ONE invoice with 3 items; scenario 6 asserts the in-grace over-module is excluded; scenario 5 asserts the flag at every site under low/high thresholds). New integration test covering the three window-function reads against real Postgres. Full build / vet (incl. -tags integration) / gofmt / unit + integration test suites green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…combined-invoice ownership, boundary reclaim freeze) Adversarial 3-lens review against docs-temp/account-billing-read/DESIGN.md surfaced three confirmed defects in the per-module-instance overage rebuild (migration 033). Each is fixed at root cause with a regression test that reproduces the exact failure scenario (all three fail without their fix). 1. [critical] Leg 1 module-overage sweep had no retroactive-catch-up guard (D1d). ChargeModuleOverage charged any past-grace "over" timer the instant the account activated, even when the timer's install-anchored period had already closed months earlier — exactly the retroactive base-fee catch-up D1d forbids and the sibling ChargeCreationProration already guards. Ported the same period-closed check (compare ActivatedAt against the install period's end): resolve the timer terminally with NO charge instead of minting a historical, never-chargeable invoice. New status period_closed. 2. [high] Scenario 3's one-invoice guarantee collapsed when the combined creation charge's persist phase failed AFTER its Stripe calls succeeded: the co-created over-module timers stayed unresolved, and the same-process overage sweep then minted a SECOND invoice for them under a disjoint idem key (mod-overage-inv-<id>), mis-attributing money Stripe already pooled onto the combined invoice. Leg 1 now DEFERS a co-created over-module whose app's creation proration is still unresolved (the combined-invoice path owns it); the proration sweep's next retry converges on the same combined invoice. New status deferred_to_combined. 3. [high] The boundary advance-base/overage were recomputed live on every billing_run reclaim with no frozen snapshot — reintroducing the retry livelock ee5043c fixed once for the account-wide model (whose account_overage_snapshots freeze migration 033 dropped). A reclaimed run keeps the same deterministic idem keys (ii-<run>/inv-<run>), so live-state drift between a crash and the retry made it re-send those keys with a different amount — a Stripe idempotency conflict that stalls the run forever. Migration 035 adds frozen_charge_cents/with_base to billing_runs, frozen BEFORE the first Stripe call and REUSED verbatim on reclaim, so every attempt sends a byte-identical request under the stable key. Full suite green (go test ./...); sqlc regenerated for migration 035. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jul 5, 2026
…eck across charge legs Two duplication findings from the /simplify pass, each independently confirmed by two review agents (altitude + reuse/simplification): - resolveChargeableCustomer (service.go): the "no usable PM -> skip" + "resolve Stripe customer, empty id is an anomaly" sequence was copy-pasted verbatim across RunBillingCycle (charge.go), ChargeCreationProration (proration.go), and ChargeModuleOverage (overage.go). Extracted to one shared helper; each call site keeps its own skip-status semantics. - periodClosedByActivation (service.go): the D1d no-retroactive-catch-up decision (anchored period containing an instant, compared against the account's activation anchor) was independently reimplemented in both ChargeCreationProration and ChargeModuleOverage, with comments in each explicitly cross-referencing the other as "the same posture." Extracted the pure decision into one helper; each leg keeps its own what-to-persist-when-closed tail. No behavior change. go build/vet/test/gofmt all clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
persistProrationCharge built its UpsertInvoiceParams without the IsLargeAutoCollect field, so every scenario-2/3 creation invoice was written with is_large_auto_collect = false in production regardless of what the charge callback computed (the unit fakes carry the struct through verbatim, which is why scenario 5a stayed green). Carry the flag through, with an integration regression test against the real pgx persist path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… never-billed) CountOngoingOverModuleTimers counted every live over-rank timer with grace_charged_at IS NOT NULL. Three defects in one predicate: - No installed_at < period_end cutoff (the advance-base leg has exactly this cutoff for exactly this reason): a module installed INSIDE the new period, already covered by its own Leg 1 grace charge, was precharged a second full $3 when a skipped_no_pm/failed boundary run was reclaimed after the grace charge fired — repro'd in review with a unit test against the PR's own fakes. - grace_charged_at as the 'ongoing' proxy permanently exempted every D1d period-closed-resolved over-module (installed pre-activation, resolved uncharged via MarkModuleTimerIncluded) from ALL boundary precharges — their $3/period overage was never billed, forever. - No grace_expires_at < period_end cutoff: prerequisite for the boundary-straddling-grace coverage rule (Leg 1 covers install through the end of the period its grace elapses into; the follow-up commit extends Leg 1 to actually charge that coverage). New predicate: live, over-rank, grace_resolved, installed_at < period_end, grace_expires_at < period_end. Unit regressions for all three + SQL-level integration coverage of each cutoff. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…a boundary Leg 1 anchored its charge strictly to the install period, with a comment deferring the next period to the boundary precharge — but the boundary for that period runs BEFORE a straddling grace elapses and (correctly) excludes the unresolved timer, so the straddled period was billed by NO leg at all: a systematic $3/module undercharge whenever a module was installed within GraceDays of a boundary. Coverage contract: every grace charge covers install day → the END of the period its grace elapses into. For a straddler that is the install period prorated + the straddled period in full; the boundary precharge picks the timer up from the FIRST boundary after its grace elapsed (grace_expires_at < period_end, previous commit), so coverage is complete and disjoint by construction. Amount stays deterministic across retries (installed_at + activation anchor are immutable), so the per-timer Stripe idem keys stay stable. Invoice mirror window extends with the amount. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… advance base The advance-base leg billed the FULL next-period base for every live app with created_at < period_end — including apps still INSIDE their 3-day creation grace at the boundary. An app created within GraceDays of its period boundary and deleted in grace (spec scenario 1: never charged) was billed a full month of base. Coverage contract (same rule as the module timers): the boundary counts an app only once its creation grace elapsed BEFORE the new period opened; the creation-proration charge in turn covers creation day → the END of the period the grace elapses into (creation period prorated + the straddled period in full, one more snapshot row for the display). A grace-deleted straddler now pays $0; a survivor pays the same total as before, on one invoice, and joins the advance leg at the NEXT boundary — matching the spec's 'join this boundary mechanism starting at the NEXT boundary after their own creation charge fires'. The co-created overage lines on the combined scenario-3 invoice extend the same way, complementing the Leg-2 grace_expires_at cutoff. AccountHasLiveApps (the no-usage boundary gate) applies the same rule so a boundary is not armed by grace-only apps. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ding-item pooling) Every charge site used CreateInvoiceItem (a floating customer-level pending item) followed by CreateInvoice with pending_invoice_items_behavior=include — which sweeps up ALL of the customer's pending items, not just the ones that leg created. Pre-PR there was effectively one item→invoice sequence per account; this PR runs several concurrently (boundary + per-timer Leg 1 + combined creation), so any crash between item-create and invoice-create leaked the orphaned item onto the NEXT leg's unrelated invoice: money collected on the wrong invoice, the crashed leg livelocked or double-charged, and a wedged combined-invoice proration. New flow at every site: CreateDraftInvoice (empty, pending_invoice_items_behavior=exclude, auto_advance=false, stamped with a deterministic ms_charge_ref metadata anchor) → CreateInvoiceItem PINNED to that draft via InvoiceItemParams.Invoice → FinalizeInvoice (auto_advance — the ONLY money-moving step), under three deterministic idem keys (inv-/ii-/fin-). A crash at any step leaves an inert draft no other invoice can sweep; the retry replays the same objects. Includes the first Stripe-failure injection tests on Leg 1 (errItem was previously never assigned by any test) proving a failed item or finalize leaves the timer unresolved and retryable through identical keys. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…reeze-or-adopt H8: the frozen-charge lookup ran AFTER the zero-skip and the prepaid/ceiling/risk/PM gates. A reclaimed run whose prior attempt had already put money through Stripe could be re-gated into skipped_prepaid/skipped_no_pm — or zero-skipped 'invoiced' when the live total collapsed — WITHOUT mirroring the charge that already fired: unmirrored money now, a fresh double-charge once the idem keys age out. The frozen lookup now runs FIRST; a frozen run's only job is to finish (replay the same Stripe objects, mirror, mark). The PM gate is deliberately bypassed on the frozen path: an idem-key replay needs no fresh authorization, and a genuinely fresh finalize with the PM removed fails loudly into the retryable 'failed' path instead of a skip. H6: FreezeBillingRunCharge was write-first-wins but the charger kept its LOCALLY computed amount — two daemons reclaiming the same run could send different bodies under the shared idem keys. The freeze now returns the SURVIVING row value and the charger adopts it. M4: the disclosure flag is computed from the amount actually sent to Stripe when it differs from the live recompute (frozen reclaim / lost freeze race), not from a live total describing a charge that never happened. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ked synthesis
Three timer-lifecycle defects from the review:
- RegisterApp retry (or billing-backfill re-registration) landing after
the app was deleted resurrected K live timers: deletion freezes
module_count and soft-removes timers, so the reconcile saw live=0
against the frozen count and re-inserted phantoms that occupied FIFO
slots and charged $3 at every boundary with no removal path. Timer
synthesis now never runs for a deleted app.
- SyncAppModules deletion is two non-transactional writes; a crash
between MarkAppDeleted and the timer soft-remove left live orphans,
and the retry guard ('req.Deleted && !app.Deleted') skipped the
soft-remove forever. The delete signal now re-fires the idempotent
soft-remove on every delivery, self-healing the crash window.
- Synthesis was an unguarded count-then-insert in a fire-and-forget-
with-retry RPC environment: two concurrent executions both read the
same live count and both inserted the full deficit (recurring phantom
charges). New ReconcileModuleTimersToTarget runs count+write in one
transaction under a per-app pg_advisory_xact_lock; both RPCs use it,
with an 8-way concurrent integration test pinning the invariant.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e-overage legs The boundary spine always gated off-session charging on the account's collection mode, but the two NEW charge legs (creation proration and per-module overage) bypassed it entirely — a prepaid-mode account (tightened after delinquency, or opted in) could still be auto-charged by them. Both legs now share one offSessionChargePermitted gate: a prepaid account is skipped TRANSIENTLY (nothing resolved, guard unarmed), and a webhook-driven relax back to arrears lets the deferred charge fire through the unchanged deterministic idem keys. The spend ceiling deliberately stays boundary-only: it caps USAGE surprises, and by the spine's own documented design the predictable base fee + overage never trip it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ime re-verification H5: every charge site's crash story rested solely on deterministic idem keys — but Stripe prunes keys at ~24h and the retry driver is a daily cron, so the first retry sat exactly on that boundary and any later one was guaranteed to mint brand-new Stripe objects and double-charge. Migration 036 adds charge-attempt markers (timers: charge_attempted_at; apps: proration_attempted_at — the boundary leg's marker is the migration-035 freeze), stamped first-write-wins BEFORE the first Stripe call. A retry that sees its marker set reconciles against Stripe via the ms_charge_ref metadata anchor (FindInvoiceByRef, Stripe Search API) before recomputing anything: a finalized invoice is adopted (mirrored + marked, zero new objects); an inert draft is completed (the deterministic line attached only if it never landed — a mismatched draft is refused loudly); nothing found means nothing moved → charge fresh. Gates never strand an attempted charge (the proration analogue of the boundary H8 rule). H9: Leg-1's retry used to recompute the FIFO verdict live, so a crash after Stripe succeeded + an earlier module's removal resolved the timer 'included' — money moved with no mirror, no mark, no disclosure. Recovery now runs BEFORE the live verdict: a rank flip can never orphan a real charge. M2: sweep candidates are re-verified (still live + unresolved) immediately before acting — the work list is read once and can be minutes stale for late candidates. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ng list length The extended migration-033 fixtures keep the 5 included timers unresolved, so the late work lists legitimately contain them too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…wave 2: D5/D10/D7) D5: the SQL advance-base grace cutoff used make_interval(days => N) — timestamptz + a day-interval shifts with the SESSION timezone across DST, while the Go legs' grace is a fixed 24h-per-day UTC window; a non-UTC session would disagree by an hour and double-bill or gap a whole period. Now hours-based. D10: all three crash-recovery arms treated any non-draft invoice found under the ms_charge_ref anchor as 'the money moved' — including VOID (support canceled the charge during an incident). Adopting a void invoice silently forgave the amount and terminally consumed the charge identity. Void now fails loudly into the auditable retried path for ops; uncollectible (charge exists, dunning) is still adopted. D7: the boundaryTotal==0 and cents==0 zero-skips marked 'invoiced' (terminal) unconditionally — under the two-daemons model a stale hasFrozen read let a zero-skip bury a concurrent reclaim's just-frozen charge forever. The terminal zero-mark is now guarded WHERE frozen_charge_cents IS NULL; losing the guard leaves the run reclaimable for the next beat to reconcile. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e_resolved (wave 2: D1) The wave-1 predicate required grace_resolved = true — but resolution is MUTABLE and set only by the grace sweeps, and cmd/billing-cycle runs the boundary spine BEFORE the sweeps in every beat. Any timer whose grace expired in the ~24h window before a boundary (and every timer whose grace expired during a skipped_no_pm/failed outage, on the reclaim) was still unresolved when its covering boundary run executed: excluded from the precharge, run marked invoiced (terminal), and Leg 1's charge — derived from immutable timestamps — stops at the boundary. The post-boundary period was billed by NOBODY, deterministically. The predicate now uses only immutable inputs (live, over-rank, installed_at < period_end, grace_expires_at < period_end), so the precharge decision is identical whenever the run or its reclaim executes; an expired-unresolved timer counted here is later charged its own disjoint install-period coverage by Leg 1 — never a double-bill. Documented residual edge (verdict-at-boundary-time, D1e no-refunds): a rank that improves over→included between the boundary and the sweep keeps the one precharge; the next boundary excludes it by rank. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ctivation period is billed (wave 2: D4) The period-closed posture resolved a pre-activation install (or app creation) terminally uncharged even when its grace straddled into the FIRST post-activation period — a period the account was fully chargeable for and in which the module/app was live. The boundary predicate excludes that period too (grace_expires_at >= its start), so it was billed by nobody. The charge shape (one shared home per leg: moduleOverageChargeShape / the proration callback) now narrows a closed-creation straddle to the straddled period billed IN FULL, with the mirror window and snapshot narrowed to match; only coverage that closed ENTIRELY before activation stays forgiven and permanently skipped. Deterministic inputs only, so fresh charges, idem replays, and recovery recompute the same shape. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ne set (wave 2: D2) The H5 recovery validated a recovered draft against a total recomputed from the LIVE co-created timer set — which can only SHRINK between the crash and the retry (an uninstall, or a rank flip via an earlier removal). Any shrink turned a COMPLETE recovered draft into the refuse-loudly branch on every subsequent sweep, forever: the guard never armed, the deferred-to-combined guard kept Leg 1 off the timers, and the boundary excluded them — the app's creation+straddle base and all its co-created overage were permanently unbilled behind a daily error livelock. Validation is now structural: base + k overage lines with len(live set) <= k <= created_module_count is a complete deterministic line set (the crashed attempt's then-correct superset stands; D1e forbids unwinding pinned lines for since-removed timers). k below the live set — a draft genuinely crashed mid-attach — and any amount fitting no k stay refused loudly for ops, since completing them past the idem-key window risks duplicate lines. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…Stripe (wave 2: D6) The attempt/freeze markers are stamped BEFORE the first Stripe call, so 'attempted' alone never means money moved — a crash or Stripe 4xx/5xx between the marker and CreateDraftInvoice leaves nothing on Stripe, yet all three legs treated the marker as license to bypass the prepaid/PM gates on the retry and then charged FRESH when the ref lookup found nothing: a prepaid-tightened or PM-removed account got a brand-new off-session debit through the 'recovery' path. The gate-bypass now keys on what the ref lookup actually finds, looked up once next to the marker read: a FINALIZED invoice → money moved → reconcile with gates bypassed (H8 unchanged); an inert DRAFT → no money moved → finalizing is a fresh debit, every gate applies (a gate skip leaves the draft inert and the run/timer/app non-terminal); NOTHING → genuinely fresh charge, every gate applies. Search-lag safety: the Search API lags ≲1min while idem keys live ~24h, so a lag-missed invoice is re-found by key replay — a false 'nothing' cannot double-charge. Boundary re-gate skips stay non-terminal and the frozen amount survives for a post-relax reclaim. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… locked transaction (wave 2: D8/D9) D8: ReconcileModuleTimersToTarget took a caller-supplied target read OUTSIDE the advisory lock — a late RegisterApp retry whose mirror read predated a concurrent SyncAppModules commit reconciled to the stale count, LIFO-shrinking away genuinely-installed modules (never billed, never re-synthesized: count and timers permanently diverged). The target, owning account, and deleted state are now read from the roster row INSIDE the locked transaction; a stale caller cannot impose anything. D9: the deleted-app resurrection guard was a read-then-act race (checked outside the lock) and the delete path took no lock at all. Deletion + timer soft-removal now commit in ONE transaction under the SAME per-app advisory lock (MarkAppDeletedAndRemoveTimers), and a deleted row reconciles to ZERO — a late synthesis retry removes orphans instead of resurrecting timers, in every interleaving. Integration test extended: concurrent reconcile invariant, stale-target immunity, locked delete, post-delete reconcile-to-zero. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…harge (wave 2: D11) Every deleted-app skip on the creation-proration leg treated 'deleted' as 'never charged' — but the spec's free-delete window is the GRACE: an app deleted after the grace elapsed survived it and owes the creation charge (grace only delays WHEN it fires). Combined with the H2 boundary exclusion (no other leg backstops the straddled period), 'delete in the grace-elapse→daily-sweep window' had become a user-timable ~$22 dodge (creation proration + the straddled month + $3 per co-created over-module), repeatable per app. The work list, the cheap early-out, and the locked re-check now skip only deletions stamped BEFORE the grace expiry (grace in hours, D5); a post-grace deletion stays pending and charges normally. Test fixtures that logically deleted 'within grace' but were clocked past it now delete with an in-grace service clock. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The unkeyed fake returned its seeded invoice for ANY ref, so the recovery tests could not detect a leg reconciling against another leg's charge identity. Seeds now register under an exact ref (setFindByRef); a wrong-ref lookup misses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ndependent count, D11 grace-relative deletes) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
I-am-nothing
added a commit
that referenced
this pull request
Jul 6, 2026
Replaces the account-wide pooled overage model with per-module-instance overage timers (migration 033), merges in the app-creation grace period and the auto-collect disclosure flag, and unifies base fee + module overage into one charge concept. Supersedes #45, #46, #47. Key mechanics: 3-day creation grace (deleted WITHIN grace = never charged; after = still owes); 5 included modules account-wide with $3/module beyond, one install-anchored timer per module instance with live FIFO re-evaluation (over->included only); coverage contract — every grace charge covers install/creation day through the END of the period its grace elapses into, boundary legs count only entities whose (immutable) install + grace-expiry predate the new period; all Stripe charges via draft->pinned-items->finalize with ms_charge_ref metadata; migration 035/036 markers + FindInvoiceByRef recovery for retries past Stripe's idempotency-key window (gates bypassed only when a finalized invoice actually exists); per-app advisory-locked timer synthesis and deletion. Migrations 029-036. Squash of the 32-commit PR branch, including two adversarial-review fix waves (23 + 12 confirmed findings fixed; full record on PR #48 and in docs-temp/pr48-review/findings.md). go build/vet/gofmt clean; full unit + integration suites green against real Postgres (migrations 001->036). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces the account-wide pooled overage model with per-module-instance overage timers, merges in the app-creation grace period and the auto-collect disclosure flag, and unifies base fee + module overage into one charge concept instead of two parallel systems. Spec:
docs-temp/account-billing-read/DESIGN.md, section "Base fee — v2: creation grace + per-module overage timers".Supersedes and should be merged instead of:
collection.IsLargeAutoCollect,AutoCollectThresholdMicros) is merged in as-is (migration renumbered 031→034 to avoid collision) and now wired into every charge site via one shared helper, not just the boundary leg.What changed (57 files, +6081/-684)
ms_billing.app_module_overage_timers(per-module install/grace/charge state), replacingaccounts.overage_since+account_overage_snapshots(032).billing_runsfrozen-charge columns (see fix feat(internal/account): billing service + webhook router #3 below).RegisterApp/SyncAppModulesmodule-count deltas (no api-platform change needed).overage.go): per-module grace charge, install-anchored proration, real Stripe item IDs.charge.go): boundary precharge for ongoing over-modules, folded into the existing arrears+base invoice.proration.go): an app created with the pool already over 5 gets ONE combined invoice (base + co-created overage lines), not two.flagLargeAutoCollecthelper called from all four charge sites.Adversarial review — 3 confirmed bugs, all fixed and independently re-verified
Each fix's regression test was verified by disabling the guard and confirming the test fails, then re-enabling and confirming it passes.
Verification
go build ./...,go vet ./...(incl.-tags integration),gofmt -l .all clean.go test ./...— 17 packages green. Migrations 001→035 apply in order against a real Postgres (testcontainers), including the new per-module FIFO SQL end-to-end.Merge instructions
Squash-merge only — the branch history includes the three superseded branches' own iteration commits plus two merge commits; squashing is what lands this as one clean commit on
main.🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
Fix wave — 2026-07-06 (review revalidation, commits
889fc93..5272214)A 74-agent adversarial review (independent spec-first re-derivation of all 6 scenarios — all matched — plus cross-dimension bug hunting) confirmed 2 critical + 10 high defects on crash/retry/concurrency edges. All are fixed on this branch, one commit per defect cluster, each with regression tests:
The coverage contract (fixes the Leg-1/Leg-2 handoff family — C1 never-billed, H1 reclaim double-charge, M1 straddle gap, H2 in-grace base precharge): every grace charge covers install/creation day → the end of the period its grace elapses into (install period prorated + straddled period in full); every boundary leg counts only entities with
grace_resolved AND installed_at < period_end AND grace_expires_at < period_end(apps: the analogous created+grace cutoff). Coverage is complete and disjoint by construction; a grace-deleted app/module pays $0.Stripe object lifecycle:
InvoiceItemParams.Invoice) → finalize, withpending_invoice_items_behavior=excludeand anms_charge_refmetadata anchor. A crashed leg's orphaned item can never be swept onto another leg's invoice; only finalize moves money.FindInvoiceByRef(Search API) — finalized invoice adopted, inert draft completed, mismatched draft refused loudly — before recomputing any live verdict, so a FIFO rank flip can never orphan moved money.Timer lifecycle (H4/H7): synthesis runs under a per-app
pg_advisory_xact_lock(ReconcileModuleTimersToTarget— 8-way concurrent integration test), never runs for deleted apps (no resurrection by late retries/backfill), and the delete signal re-fires the idempotent timer soft-remove so the crash window self-heals.Gates (H10): the creation and module-overage legs now share the prepaid-mode gate the boundary always had (transient skip; relax re-attempts through unchanged keys). H3:
persistProrationChargeno longer dropsIsLargeAutoCollect(integration test against the real pgx path). M2: sweep candidates re-verified live immediately before charging.Deferred (documented in the review report, low/medium display-or-edge items): M3 backdated
RegisterApp.created_atvs FIFO monotonicity (backfill design decision), M5GetAccountBilloverage line ignores the requested period (display-only; needs the per-timer charged-micros column the code already TODOs), L1 uninstall-after-grace-before-sweep undercharge, L2 combined-invoice flag rounding at the threshold boundary.Verification:
go build,go vet(incl.-tags integration),gofmtclean; full unit + integration suites green — 17 packages, migrations 001→036 applied in order against real Postgres.Wave 2 — delta review of the fix wave (commits
3e7879d..78016fa)A second 40-agent adversarial review over the wave-1 delta confirmed 12 defects in the fixes themselves; all are fixed:
grace_resolvedwhile the cron runs the boundary before the sweeps — timers whose grace expired in the ~24h pre-boundary window lost a full period to nobody. The predicate is now immutable-cutoffs-only (ordering-independent), with a documented verdict-at-boundary-time residual.Deferred (documented in
docs-temp/pr48-review/findings.md): D3 — a crashed-then-charged timer whose module is later removed leaves a paid-but-unmirrored invoice until webhook reconciliation (money safe, never double-charged; PR #7 webhook flow is the designed owner); D12 — display-only snapshot drift on frozen reclaims.Verification: full unit + integration suites green (17 packages, migrations 001→036 against real Postgres via testcontainers);
go vet -tags integration,gofmtclean.