diff --git a/.github/failure-classes/FC-early-terminal-strips-paid-period.json b/.github/failure-classes/FC-early-terminal-strips-paid-period.json new file mode 100644 index 00000000000..8a07b902f76 --- /dev/null +++ b/.github/failure-classes/FC-early-terminal-strips-paid-period.json @@ -0,0 +1,15 @@ +{ + "schema_version": 1, + "id": "FC-early-terminal-strips-paid-period", + "violated_contract": "A subscription that reaches its terminal Stripe status early — the portal's cancel-immediately mode fires customer.subscription.deleted/updated with status canceled while current_period_end is still in the future — must not be projected as an immediate Basic downgrade. The customer paid through current_period_end, and persisting Basic makes the stored row permanently hide the paid-period grace get_user_valid_subscription already implements.", + "canonical_prevention": "When projecting an inactive snapshot on the subscription-event path, keep the resolvable paid plan with status active and cancel_at_period_end true while current_period_end is in the future, and only for status canceled; the grace candidate must still route through the stale-downgrade shield so a newer stored paid subscription or an adoptable active replacement wins, and unresolvable price IDs, unpaid/incomplete_expired and past-period-end cancellations still downgrade to Basic immediately. Pin the boundaries (future vs past period end, unknown price, unpaid, superseded subscription id, replacement adoption) with behavioral tests executed against the extracted builder.", + "canonical_prevention_artifact": [ + "backend/tests/unit/test_stripe_webhook_behavioral.py" + ], + "evidence_prs": [], + "scope_hints": [ + "backend/routers/payment.py", + "backend/utils/subscription_state_machine.py" + ], + "status": "open" +} diff --git a/backend/routers/payment.py b/backend/routers/payment.py index dfa470ea2cb..3f9316a073c 100644 --- a/backend/routers/payment.py +++ b/backend/routers/payment.py @@ -227,10 +227,44 @@ class AvailablePlansResponse(BaseModel): plans: List[PricingOption] -def _build_subscription_from_stripe_object(stripe_sub: dict) -> Subscription | None: +def _build_subscription_from_stripe_object( + stripe_sub: dict, grant_cancel_grace: bool = False, now: int | None = None +) -> Subscription | None: """Builds a Subscription object from a Stripe Subscription object.""" - # Inactive snapshots always project Basic, regardless of the price binding. - if stripe_sub['status'] not in ('active', 'trialing'): + stripe_status = stripe_sub['status'] + + # For inactive subscriptions (canceled, unpaid, etc.), always downgrade to Basic + # regardless of price ID — ensures deleted/canceled users don't keep paid access. + # Exception (subscription-event paths only): an immediately-canceled subscription + # (e.g. the customer portal's "cancel immediately" mode) whose paid period has not + # ended yet. The customer paid through current_period_end, so paid access must + # survive the early terminal event. The candidate still routes through the + # stale-downgrade shield in the webhook handler, so a newer stored paid + # subscription or an adopted replacement always wins over this grace. + if stripe_status not in ('active', 'trialing'): + period_end = stripe_sub.get('current_period_end') + if ( + grant_cancel_grace + and stripe_status == 'canceled' + and period_end + and period_end > (now if now is not None else int(time.time())) + ): + items = stripe_sub.get('items', {}).get('data', []) + price_id = items[0]['price']['id'] if items else None + try: + paid_plan = get_plan_type_from_price_id(price_id) if price_id else None + except ValueError: + paid_plan = None + if paid_plan is not None: + return Subscription( + plan=paid_plan, + status=SubscriptionStatus.active, + current_period_end=period_end, + current_period_start=stripe_sub.get('current_period_start'), + stripe_subscription_id=stripe_sub['id'], + cancel_at_period_end=True, + limits=get_plan_limits(paid_plan), + ) return project_subscription(stripe_sub, None, get_basic_plan_limits()) # Active subscriptions: resolve plan from price ID @@ -1045,7 +1079,10 @@ async def _handle_stripe_event(event: dict, background_tasks: BackgroundTasks | uid = user['uid'] if uid: - new_subscription = _build_subscription_from_stripe_object(subscription_obj) + now = int(time.time()) + new_subscription = _build_subscription_from_stripe_object( + subscription_obj, grant_cancel_grace=True, now=now + ) if new_subscription: # Resolve prior entitlement before reconciliation; emit only # after the authoritative write so stale deletion is not churn. @@ -1076,10 +1113,13 @@ async def _handle_stripe_event(event: dict, background_tasks: BackgroundTasks | # all customer/storage IO remains in this adapter. current_subscription = None active_paid = None - if not is_paid_plan(new_subscription.plan): + # A paid-through canceled snapshot (grace) is downgrade-shaped for + # the stale shield: a newer stored paid sub or an adoptable active + # paid sub must still win over the grace candidate. + if not is_paid_plan(new_subscription.plan) or subscription_obj.get('status') == 'canceled': event_sub_id = subscription_obj.get('id') current_subscription = await run_blocking(db_executor, users_db.get_existing_user_subscription, uid) - if preserves_current_paid_subscription(current_subscription, event_sub_id, int(time.time())): + if preserves_current_paid_subscription(current_subscription, event_sub_id, now): logger.info( f"Ignoring downgrade from {event['type']} (sub {event_sub_id}) for user {uid}: " f"stored paid sub {current_subscription.stripe_subscription_id} is still valid." @@ -1091,21 +1131,57 @@ async def _handle_stripe_event(event: dict, background_tasks: BackgroundTasks | f"Ignoring downgrade from {event['type']} (sub {event_sub_id}) for user {uid}: " f"a different active paid sub {active_paid.stripe_subscription_id} exists." ) + decision_candidate = new_subscription + # Paid-through grace applies only when the canceled event belongs + # to the user's current subscription (the portal cancel flow). A + # canceled snapshot for a different, superseded subscription id + # keeps main's downgrade semantics: project Basic and let the + # stale guard / replacement adoption decide. + grace_candidate = ( + subscription_obj.get('status') == 'canceled' + and is_paid_plan(new_subscription.plan) + and ( + current_subscription is None + or current_subscription.stripe_subscription_id == subscription_obj.get('id') + ) + ) + if ( + subscription_obj.get('status') == 'canceled' + and is_paid_plan(new_subscription.plan) + and not grace_candidate + ): + decision_candidate = project_subscription(subscription_obj, None, get_basic_plan_limits()) decision = transition( None, ResolvedStripeEvent( event['type'], subscription_obj.get('id'), - new_subscription, - int(time.time()), + decision_candidate, + now, active_paid, billing_owner_confirmed, + grace_paid=grace_candidate, ), ) if decision.next_state is None: raise ValueError('Subscription transition requires a projected state') new_subscription = decision.next_state adopted_active_paid = decision.adopted_active_paid + if adopted_active_paid: + logger.info( + f"Early-cancel paid-through grace for user {uid}: adopted replacement paid sub " + f"{new_subscription.stripe_subscription_id} over canceled event sub " + f"{subscription_obj.get('id')}." + ) + elif ( + is_paid_plan(new_subscription.plan) + and decision.outcome != 'preserved_current' + and subscription_obj.get('status') == 'canceled' + ): + logger.info( + f"Early-cancel paid-through grace for user {uid}: keeping plan until period end " + f"for canceled sub {subscription_obj.get('id')}." + ) try: if 'unlock_conversations' in decision.effects: # An adopted replacement keeps its own customer identity; diff --git a/backend/tests/unit/test_stripe_event_ledger.py b/backend/tests/unit/test_stripe_event_ledger.py index f702600aec0..d8b22c2f5bc 100644 --- a/backend/tests/unit/test_stripe_event_ledger.py +++ b/backend/tests/unit/test_stripe_event_ledger.py @@ -312,9 +312,9 @@ def test_real_webhook_unknown_price_is_200_and_same_id_reenters(lane, monkeypatc builds = [] build = payment._build_subscription_from_stripe_object - def tracking(obj): + def tracking(obj, **kwargs): builds.append(obj['id']) - return build(obj) + return build(obj, **kwargs) monkeypatch.setattr(payment, '_build_subscription_from_stripe_object', tracking) assert run_lifecycle(lane) == {'status': 'success'} diff --git a/backend/tests/unit/test_stripe_webhook_behavioral.py b/backend/tests/unit/test_stripe_webhook_behavioral.py index 487937a911d..178056a6ac6 100644 --- a/backend/tests/unit/test_stripe_webhook_behavioral.py +++ b/backend/tests/unit/test_stripe_webhook_behavioral.py @@ -10,6 +10,7 @@ import os import re import sys +import time from pathlib import Path from unittest.mock import MagicMock from unittest.mock import MagicMock as _MagicMock, patch @@ -41,6 +42,7 @@ def _get_build_subscription_fn(): 'Dict': Dict, 'Optional': Optional, 'cast': _cast, + 'time': __import__('time'), 'project_subscription': __import__( 'utils.subscription_state_machine', fromlist=['project_subscription'] ).project_subscription, @@ -158,6 +160,47 @@ def test_canceled_ignores_cancel_at_period_end(self): assert result is not None assert result.cancel_at_period_end is False + def test_immediately_canceled_subscription_keeps_paid_plan_until_period_end(self): + """A canceled-now event with a future current_period_end must not strip + access the customer already paid for (portal "cancel immediately").""" + future = int(time.time()) + 86_400 * 20 + sub = _make_stripe_sub(status='canceled', price_id='price_known') + sub['current_period_end'] = future + result = self.fn(sub, grant_cancel_grace=True) + assert result is not None + assert result.plan == PlanType.unlimited + assert result.status == SubscriptionStatus.active + assert result.cancel_at_period_end is True + assert result.current_period_end == future + + def test_canceled_paid_through_keeps_basic_without_grant(self): + """Schedule/checkout paths keep main's downgrade: no grant, no grace.""" + sub = _make_stripe_sub(status='canceled', price_id='price_known') + sub['current_period_end'] = int(time.time()) + 86_400 * 20 + result = self.fn(sub) + assert result is not None + assert result.plan == PlanType.basic + assert result.cancel_at_period_end is False + + def test_immediately_canceled_with_unresolvable_price_downgrades_to_basic(self): + """Paid-through preservation cannot invent a plan; unknown price stays Basic.""" + self.ns['get_plan_type_from_price_id'] = MagicMock(side_effect=ValueError("Unknown")) + future = int(time.time()) + 86_400 * 20 + sub = _make_stripe_sub(status='canceled', price_id='price_unknown') + sub['current_period_end'] = future + result = self.fn(sub, grant_cancel_grace=True) + assert result is not None + assert result.plan == PlanType.basic + + def test_unpaid_with_future_period_end_still_downgrades_to_basic(self): + """Grace is limited to canceled; unpaid/incomplete_expired keep Basic.""" + future = int(time.time()) + 86_400 * 20 + sub = _make_stripe_sub(status='unpaid', price_id='price_known') + sub['current_period_end'] = future + result = self.fn(sub, grant_cancel_grace=True) + assert result is not None + assert result.plan == PlanType.basic + def test_subscription_includes_stripe_id(self): """Result always includes the Stripe subscription ID.""" result = self.fn(_make_stripe_sub()) diff --git a/backend/tests/unit/test_subscription_ingestion_characterization.py b/backend/tests/unit/test_subscription_ingestion_characterization.py index 32db0397c4d..b48610deeb9 100644 --- a/backend/tests/unit/test_subscription_ingestion_characterization.py +++ b/backend/tests/unit/test_subscription_ingestion_characterization.py @@ -121,20 +121,24 @@ def test_every_lifecycle_status_and_paid_plan_preserves_bytes_and_effect_order(l lane.event = event(kind, status, plan) assert run(lane) == {'status': 'success'} paid = status in ('active', 'trialing') + # Portal "cancel immediately" fires canceled while the period is still live; + # the paid-through grace (FC-early-terminal-strips-paid-period) keeps the paid + # plan until period end instead of downgrading to Basic on the spot. + keeps_paid = paid or status == 'canceled' expected = Subscription( - plan=plan if paid else PlanType.basic, + plan=plan if keeps_paid else PlanType.basic, status=SubscriptionStatus.active, stripe_subscription_id='sub-current', current_period_start=NOW - 100, current_period_end=NOW + 100, - cancel_at_period_end=paid, - limits=PlanLimits(transcription_seconds=123 if paid else 7), + cancel_at_period_end=keeps_paid, + limits=PlanLimits(transcription_seconds=123 if keeps_paid else 7), ) # Includes every model field and default, especially current_price_id=None. assert wire(lane.current.model_dump()) == wire(expected.model_dump()) assert [name for name, _ in lane.calls] == ( ['customer', 'conversations', 'memories', 'actions', 'write', 'telemetry', 'credits', 'trial_cache', 'fair_use'] - if paid + if keeps_paid else ['write', 'telemetry', 'credits', 'trial_cache'] ) diff --git a/backend/utils/subscription_state_machine.py b/backend/utils/subscription_state_machine.py index e3f2f205630..335a25b5302 100644 --- a/backend/utils/subscription_state_machine.py +++ b/backend/utils/subscription_state_machine.py @@ -69,6 +69,10 @@ class ResolvedStripeEvent: active_paid: Subscription | None = None billing_owner_confirmed: bool = False schedule_status: str | None = None + # Candidate projects a paid plan only via the early-terminal grace (a + # canceled snapshot paid through its period end). Stale-downgrade protection + # must still see it as a downgrade-shaped candidate. + grace_paid: bool = False @dataclass(frozen=True) @@ -110,7 +114,10 @@ def transition(current: Subscription | None, event: ResolvedStripeEvent) -> Tran return Transition(current) candidate = event.candidate adopted = False - if candidate.plan not in PAID_PLAN_TYPES: + # A grace candidate (canceled but paid through period end) keeps paid access, + # yet a newer stored paid sub or a different active paid sub still wins: the + # event may belong to an old subscription redelivered after a replacement. + if candidate.plan not in PAID_PLAN_TYPES or event.grace_paid: if preserves_current_paid_subscription(current, event.subscription_id, event.now): return Transition(current, outcome='preserved_current') if event.active_paid and event.active_paid.stripe_subscription_id != event.subscription_id: