eleven / ocho Public

server: checkout receipts must not advance the subscription ordering watermark (#846)

* server: checkout receipts must not advance the subscription ordering watermark

Stripe stamps checkout.session.completed AFTER customer.subscription.created
but can deliver it first. The receipt (which carries no period end) claimed
the per-sub ext_event_ts watermark, so the later-delivered created event —
the only one with current_period_end — was dropped as stale, leaving
paid_until stuck at 0: a blank renewal date, and a row the silent-lapse
backstop (paid_until + grace <= now) can never catch.

Found live on the first sandbox E2E run of paid access levels (#703).
Checkout receipts now fold without advancing the watermark (EventTS 0 +
MAX() in setEntitlement so a late receipt can't wind it backwards), and a
regression test pins the checkout-first order.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* server: reset the watermark when a level switch moves a new sub onto a row

High-effort review of this diff confirmed a regression in the blanket MAX():
a level switch folds the moved subscription into a destination row that may
carry a DIFFERENT sub's higher watermark; keeping it would drop the moved
sub's later, lower-stamped events (its cancel would never fold). The
watermark's domain is one subscription: MAX within the same sub, reset on a
sub change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Paul Campbell 3 months ago 6a455eb4e1a9d4c811739dcd6947a7c8281e5bc2 parent dc6c93e

3 files changed, +123 −4

db.go +13 −2
subscriptions.go +13 −2
subscriptions_test.go +97 −0

diff --git a/db.go b/db.go

index 6795fed..95cf953 100644

--- a/db.go

+++ b/db.go

@@ -2374,9 +2374,20 @@ func (db *DB) setEntitlement(e Entitlement) error {

return err

}

// No source= here: preserve an existing admin/invite grant (see doc above).

+ // The ordering watermark's domain is ONE subscription: within the same sub,

+ // MAX() so a checkout receipt (EventTS 0 — see applySubEvent) folded after a

+ // real lifecycle event can't wind it backwards; when the row's subscription

+ // CHANGES (a level switch moved a different sub onto this row), the old

+ // sub's watermark is meaningless — keep the incoming event's stamp, or the

+ // moved sub's later, lower-stamped events would be dropped as stale.

if _, err := tx.Exec(

- "UPDATE user_tiers SET status=?, paid_until=?, grace_deadline=?, ext_sub=?, ext_customer=?, ext_event_ts=? WHERE tier_id=? AND user_id=?",

- e.Status, e.PaidUntil, e.GraceDeadline, nullable(e.ExtSub), nullable(e.ExtCustomer), e.EventTS, e.TierID, e.UserID); err != nil {

+ `UPDATE user_tiers SET status=?, paid_until=?, grace_deadline=?,

+ ext_event_ts = CASE WHEN COALESCE(ext_sub,'') = COALESCE(?,'') THEN MAX(ext_event_ts, ?) ELSE ? END,

+ ext_sub=?, ext_customer=?

+ WHERE tier_id=? AND user_id=?`,

+ e.Status, e.PaidUntil, e.GraceDeadline,

+ nullable(e.ExtSub), e.EventTS, e.EventTS,

+ nullable(e.ExtSub), nullable(e.ExtCustomer), e.TierID, e.UserID); err != nil {

return err

}

return tx.Commit()

diff --git a/subscriptions.go b/subscriptions.go

index 372226e..d7c6bb0 100644

--- a/subscriptions.go

+++ b/subscriptions.go

@@ -60,6 +60,9 @@ type SubEvent struct {

AllowCreate bool // a genuine "new subscription" signal (checkout completed,

// or subscription.created) — the ONLY kind that may CREATE a

// membership, so a bare update/delete can't resurrect a revoked one

+ Checkout bool // a checkout receipt: a creation signal carrying NO lifecycle

+ // data (no period end, no cancel state) — must never advance the

+ // per-sub ordering watermark (see applySubEvent)

}

// CheckoutReq is what a member's subscribe needs from the provider.

@@ -185,7 +188,15 @@ func applySubEvent(db *DB, ev SubEvent, tierID, userID string, now int64) (activ

e.GraceDeadline = 0

active = true

}

- e.EventTS = ev.Created

+ // A checkout receipt must not advance the per-sub ordering watermark: Stripe

+ // stamps checkout.session.completed AFTER customer.subscription.created but

+ // can deliver it FIRST — if the receipt claimed the watermark, the created

+ // event (the only one carrying the real period end) would be dropped as

+ // stale and paid_until stuck at 0 (blank renewal date, and the silent-lapse

+ // backstop in entitlementsDueForRevoke never applies).

+ if !ev.Checkout {

+ e.EventTS = ev.Created

+ }

if err := db.setEntitlement(e); err != nil {

return false, err

}

@@ -435,7 +446,7 @@ func (p *stripeConnect) ParseWebhook(sig string, body []byte) ([]SubEvent, error

return []SubEvent{{

ExtSub: o.Subscription, ExtCustomer: o.Customer,

UserRef: o.Metadata["user_ref"], TierRef: o.Metadata["tier_ref"],

- Status: "active", Created: ev.Created, AllowCreate: true,

+ Status: "active", Created: ev.Created, AllowCreate: true, Checkout: true,

}}, nil

case strings.HasPrefix(ev.Type, "customer.subscription."):

var o struct {

diff --git a/subscriptions_test.go b/subscriptions_test.go

index 16620a4..3d8817d 100644

--- a/subscriptions_test.go

+++ b/subscriptions_test.go

@@ -387,3 +387,100 @@ func TestBillingSweepBackstop(t *testing.T) {

t.Fatalf("healthy sub wrongly revoked")

}

+// TestCheckoutFirstDeliveryKeepsPeriodEnd pins the delivery-order race found

+// live on the first real sandbox run: Stripe stamps checkout.session.completed

+// one second AFTER customer.subscription.created but delivered it FIRST. The

+// checkout receipt (which carries no period end) must not claim the ordering

+// watermark, or the created event — the only one with current_period_end — is

+// dropped as stale and paid_until sticks at 0.

+func TestCheckoutFirstDeliveryKeepsPeriodEnd(t *testing.T) {

+ _, srv := newTestServer(t)

+ tid, uid := seedTierAndMember(t, srv, "Gold", "cara")

+ now := int64(2_000_000)

+ month := int64(30 * 24 * 3600)

+ // Delivered first: the checkout receipt, stamped now (later than the

+ // subscription.created it reports on).

+ if err := srv.applyWebhookEvent(context.Background(), SubEvent{

+ ExtSub: "subX", ExtCustomer: "cusX", UserRef: uid, TierRef: tid,

+ Status: "active", Created: now, AllowCreate: true, Checkout: true,

+ }, now); err != nil {

+ t.Fatalf("checkout event: %v", err)

+ }

+ // Delivered second: subscription.created, stamped a second EARLIER, carrying

+ // the real period end. It must still fold.

+ if err := srv.applyWebhookEvent(context.Background(), SubEvent{

+ ExtSub: "subX", ExtCustomer: "cusX", UserRef: uid, TierRef: tid,

+ Status: "active", CurrentPeriodEnd: now + month, Created: now - 1, AllowCreate: true,

+ }, now); err != nil {

+ t.Fatalf("created event: %v", err)

+ }

+ e, ok := srv.db.memberEntitlement(tid, uid)

+ if !ok || e.PaidUntil != now+month {

+ t.Fatalf("paid_until = %d, want %d (subscription.created dropped by the watermark?)", e.PaidUntil, now+month)

+ }

+ // The watermark still guards real lifecycle reordering: a stale pre-dating

+ // "canceled" must be ignored.

+ if err := srv.applyWebhookEvent(context.Background(), SubEvent{

+ ExtSub: "subX", UserRef: uid, TierRef: tid,

+ Status: "canceled", Deleted: true, Created: now - 2,

+ }, now); err != nil {

+ t.Fatalf("stale delete: %v", err)

+ }

+ if e, ok = srv.db.memberEntitlement(tid, uid); !ok || e.Status != "active" {

+ t.Fatalf("status = %q after stale delete, want active", e.Status)

+ }

+ // And the reverse order stays correct: a late checkout receipt neither

+ // zeroes paid_until nor winds the watermark back.

+ if err := srv.applyWebhookEvent(context.Background(), SubEvent{

+ ExtSub: "subX", ExtCustomer: "cusX", UserRef: uid, TierRef: tid,

+ Status: "active", Created: now + 1, AllowCreate: true, Checkout: true,

+ }, now); err != nil {

+ t.Fatalf("late checkout: %v", err)

+ }

+ if e, ok = srv.db.memberEntitlement(tid, uid); !ok || e.PaidUntil != now+month || e.EventTS != now-1 {

+ t.Fatalf("after late checkout: paid_until=%d eventTS=%d, want %d/%d", e.PaidUntil, e.EventTS, now+month, now-1)

+ }

+// TestLevelSwitchResetsWatermark pins the review finding on the MAX() rewrite:

+// when a level switch moves a subscription onto a row that already carried a

+// DIFFERENT subscription's higher watermark, the watermark must reset to the

+// incoming event's stamp — a blanket MAX() would keep the stale-high mark and

+// drop the moved sub's later, lower-stamped lifecycle events (a cancel would

+// never fold; the member would keep access past period end).

+func TestLevelSwitchResetsWatermark(t *testing.T) {

+ _, srv := newTestServer(t)

+ tidGold, uid := seedTierAndMember(t, srv, "Gold", "dana")

+ tidSilver, err := srv.db.createTier("Silver")

+ if err != nil {

+ t.Fatalf("createTier: %v", err)

+ }

+ now := int64(3_000_000)

+ must := func(err error) {

+ t.Helper()

+ if err != nil {

+ t.Fatalf("applyWebhookEvent: %v", err)

+ }

+ // Member holds Silver via subB, whose watermark sits HIGH (an event stamped

+ // now+5 was delivered early), and Gold via subA (stamped now).

+ must(srv.applyWebhookEvent(context.Background(), SubEvent{ExtSub: "subB", ExtCustomer: "c", UserRef: uid, TierRef: tidSilver,

+ Status: "active", CurrentPeriodEnd: now + 1000, AllowCreate: true, Created: now + 5}, now))

+ must(srv.applyWebhookEvent(context.Background(), SubEvent{ExtSub: "subA", ExtCustomer: "c", UserRef: uid, TierRef: tidGold,

+ Status: "active", CurrentPeriodEnd: now + 1000, AllowCreate: true, Created: now}, now))

+ // Level switch: subA's metadata now points at Silver; its update (stamped

+ // now) moves it onto the Silver row, displacing subB.

+ must(srv.applyWebhookEvent(context.Background(), SubEvent{ExtSub: "subA", ExtCustomer: "c", UserRef: uid, TierRef: tidSilver,

+ Status: "active", CurrentPeriodEnd: now + 2000, Created: now}, now))

+ // The member cancels subA. The delete is stamped now+2 — LOWER than subB's

+ // leftover now+5 — and must still fold and revoke.

+ must(srv.applyWebhookEvent(context.Background(), SubEvent{ExtSub: "subA", UserRef: uid, TierRef: tidSilver,

+ Status: "canceled", Deleted: true, Created: now + 2}, now+2))

+ if e, ok := srv.db.memberEntitlement(tidSilver, uid); ok && e.Status == "active" {

+ t.Fatalf("subA's cancel was dropped by the displaced sub's stale watermark (status=%q)", e.Status)

+ }