From ca54eeee0e693bc08cc5f290114474cfb1cc41f0 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 7 Oct 2026 13:02:27 -0700 Subject: [PATCH 01/14] fix(billing): converge Stripe cancel/contact syncs and guard webhook echoes over unsynced DB changes --- .../api/v1/admin/subscriptions/[id]/route.ts | 9 +- .../lib/admin/subscription-lifecycle.test.ts | 10 + apps/sim/lib/admin/subscription-lifecycle.ts | 26 +- apps/sim/lib/auth/auth.ts | 2 + .../billing/organizations/lock-order.test.ts | 2 + .../lib/billing/organizations/membership.ts | 10 +- .../pause-pro-for-coverage.test.ts | 30 +- .../organizations/provision-seat.test.ts | 40 +- .../billing/organizations/provision-seat.ts | 15 +- .../lib/billing/organizations/seats.test.ts | 15 +- apps/sim/lib/billing/organizations/seats.ts | 16 +- .../sim/lib/billing/webhooks/outbox-events.ts | 22 +- .../lib/billing/webhooks/outbox-handlers.ts | 232 +++++---- .../stripe-sync-convergence.integration.ts | 444 ++++++++++++++++++ .../lib/billing/webhooks/subscription-sync.ts | 224 +++++++++ .../mocks/billing-subscription-sync.mock.ts | 43 ++ packages/testing/src/mocks/index.ts | 9 + packages/testing/src/mocks/stripe.mock.ts | 279 +++++++++++ 18 files changed, 1272 insertions(+), 156 deletions(-) create mode 100644 apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts create mode 100644 apps/sim/lib/billing/webhooks/subscription-sync.ts create mode 100644 packages/testing/src/mocks/billing-subscription-sync.mock.ts diff --git a/apps/sim/app/api/v1/admin/subscriptions/[id]/route.ts b/apps/sim/app/api/v1/admin/subscriptions/[id]/route.ts index ffc61afe9ba..0e88a8b1a3d 100644 --- a/apps/sim/app/api/v1/admin/subscriptions/[id]/route.ts +++ b/apps/sim/app/api/v1/admin/subscriptions/[id]/route.ts @@ -33,8 +33,7 @@ import { } from '@/lib/api/contracts/v1/admin' import { parseRequest } from '@/lib/api/server' import { requireStripeClient } from '@/lib/billing/stripe-client' -import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' -import { enqueueOutboxEvent } from '@/lib/core/outbox/service' +import { enqueueCancelAtPeriodEndSync } from '@/lib/billing/webhooks/subscription-sync' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { withAdminAuthParams } from '@/app/api/v1/admin/middleware' import { @@ -113,15 +112,17 @@ export const DELETE = withRouteHandler( } if (atPeriodEnd) { + const stripeSubscriptionId = existing.stripeSubscriptionId await db.transaction(async (tx) => { await tx .update(subscription) .set({ cancelAtPeriodEnd: true }) .where(eq(subscription.id, subscriptionId)) - await enqueueOutboxEvent(tx, OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, { - stripeSubscriptionId: existing.stripeSubscriptionId, + await enqueueCancelAtPeriodEndSync(tx, { + stripeSubscriptionId, subscriptionId: existing.id, + cancelAtPeriodEnd: true, reason: reason ?? 'admin-cancel-at-period-end', }) }) diff --git a/apps/sim/lib/admin/subscription-lifecycle.test.ts b/apps/sim/lib/admin/subscription-lifecycle.test.ts index 89a7f37e4b6..23ab4c42cab 100644 --- a/apps/sim/lib/admin/subscription-lifecycle.test.ts +++ b/apps/sim/lib/admin/subscription-lifecycle.test.ts @@ -2,6 +2,10 @@ import { outboxEvent, subscription } from '@sim/db/schema' import { dbChainMockFns, queueTableRows, resetDbChainMock } from '@sim/testing' import { auditMock, auditMockFns } from '@sim/testing/mocks/audit.mock' import { billingOutboxHandlersMock } from '@sim/testing/mocks/billing-outbox-handlers.mock' +import { + billingSubscriptionSyncMock, + billingSubscriptionSyncMockFns, +} from '@sim/testing/mocks/billing-subscription-sync.mock' import { organizationMembershipMock } from '@sim/testing/mocks/organization-membership.mock' import { outboxServiceMock, outboxServiceMockFns } from '@sim/testing/mocks/outbox-service.mock' import { stripeClientMock } from '@sim/testing/mocks/stripe.mock' @@ -24,6 +28,7 @@ vi.mock('@/lib/billing/organizations/membership', () => organizationMembershipMo vi.mock('@/lib/billing/stripe-client', () => stripeClientMock) vi.mock('@/lib/billing/webhooks/outbox-handlers', () => billingOutboxHandlersMock) vi.mock('@/lib/core/outbox/service', () => outboxServiceMock) +vi.mock('@/lib/billing/webhooks/subscription-sync', () => billingSubscriptionSyncMock) import { refundDashboardSubscriptionPayment, @@ -119,6 +124,11 @@ describe('admin subscription cancellation', () => { expect.objectContaining({ status: 'pending', attempts: 0, lastError: null }) ) expect(dbChainMockFns.set).toHaveBeenCalledWith({ cancelAtPeriodEnd: true }) + expect(billingSubscriptionSyncMockFns.mockRecommitCancelAtPeriodEndSync).toHaveBeenCalledWith( + expect.anything(), + 'outbox-1', + true + ) expect(result).toMatchObject({ operationId: '67e55044-10b1-426f-9247-bb680e5fe0c8', status: 'pending', diff --git a/apps/sim/lib/admin/subscription-lifecycle.ts b/apps/sim/lib/admin/subscription-lifecycle.ts index 72335997f92..44177c17ca9 100644 --- a/apps/sim/lib/admin/subscription-lifecycle.ts +++ b/apps/sim/lib/admin/subscription-lifecycle.ts @@ -8,6 +8,10 @@ import { acquireOrganizationMutationLock } from '@/lib/billing/organizations/mem import { requireStripeClient } from '@/lib/billing/stripe-client' import { ENTITLED_SUBSCRIPTION_STATUSES } from '@/lib/billing/subscriptions/utils' import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' +import { + enqueueCancelAtPeriodEndSync, + recommitCancelAtPeriodEndSync, +} from '@/lib/billing/webhooks/subscription-sync' import { enqueueOutboxEvent } from '@/lib/core/outbox/service' const RECENT_INVOICE_LIMIT = 12 @@ -301,6 +305,7 @@ export async function requestDashboardSubscriptionCancellation({ if (!restoredSubscription) { throw new Error('Cancellation subscription no longer exists') } + await recommitCancelAtPeriodEndSync(tx, existingOperation.id, true) } await tx .update(outboxEvent) @@ -384,18 +389,15 @@ export async function requestDashboardSubscriptionCancellation({ .set({ cancelAtPeriodEnd: true }) .where(eq(subscription.id, subscriptionRow.id)) } - const eventId = await enqueueOutboxEvent( - tx, - OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, - { - operationId, - organizationId, - subscriptionId: subscriptionRow.id, - stripeSubscriptionId: subscriptionRow.stripeSubscriptionId, - reason: normalizedReason, - requestedBy: actor, - } - ) + const eventId = await enqueueCancelAtPeriodEndSync(tx, { + operationId, + organizationId, + subscriptionId: subscriptionRow.id, + stripeSubscriptionId: subscriptionRow.stripeSubscriptionId, + cancelAtPeriodEnd: true, + reason: normalizedReason, + requestedBy: actor, + }) return { operationId, outboxEventId: eventId, diff --git a/apps/sim/lib/auth/auth.ts b/apps/sim/lib/auth/auth.ts index 65ac5d554e0..cfaf5fe4ec0 100644 --- a/apps/sim/lib/auth/auth.ts +++ b/apps/sim/lib/auth/auth.ts @@ -103,6 +103,7 @@ import { handleSubscriptionCreated, handleSubscriptionDeleted, } from '@/lib/billing/webhooks/subscription' +import { reconcileSubscriptionSyncFromStripe } from '@/lib/billing/webhooks/subscription-sync' import { handleSubscriptionUsageUpdate } from '@/lib/billing/webhooks/subscription-usage' import { env } from '@/lib/core/config/env' import { @@ -1727,6 +1728,7 @@ export const auth = betterAuth({ case 'customer.subscription.created': case 'customer.subscription.updated': { await handleManualEnterpriseSubscription(event) + await reconcileSubscriptionSyncFromStripe(event) await handleSubscriptionUsageUpdate(event) break } diff --git a/apps/sim/lib/billing/organizations/lock-order.test.ts b/apps/sim/lib/billing/organizations/lock-order.test.ts index 108bb594f63..b6234a37a9a 100644 --- a/apps/sim/lib/billing/organizations/lock-order.test.ts +++ b/apps/sim/lib/billing/organizations/lock-order.test.ts @@ -17,6 +17,7 @@ import { workspace, } from '@sim/db/schema' import { dbChainMockFns, resetDbChainMock } from '@sim/testing' +import { billingSubscriptionSyncMock } from '@sim/testing/mocks/billing-subscription-sync.mock' import { outboxServiceMock } from '@sim/testing/mocks/outbox-service.mock' import { beforeEach, describe, expect, it, vi } from 'vitest' @@ -42,6 +43,7 @@ import type { DbOrTx } from '@/lib/db/types' import { attachOwnedWorkspacesToOrganizationTx } from '@/lib/workspaces/organization-workspaces' vi.mock('@/lib/core/outbox/service', () => outboxServiceMock) +vi.mock('@/lib/billing/webhooks/subscription-sync', () => billingSubscriptionSyncMock) /** * A superset row that satisfies every read in the join path: a paid org sub, a diff --git a/apps/sim/lib/billing/organizations/membership.ts b/apps/sim/lib/billing/organizations/membership.ts index e93f4bcd0e2..e78c74ddfe0 100644 --- a/apps/sim/lib/billing/organizations/membership.ts +++ b/apps/sim/lib/billing/organizations/membership.ts @@ -48,6 +48,7 @@ import { import { toDecimal, toNumber } from '@/lib/billing/utils/decimal' import { validateSeatAvailability } from '@/lib/billing/validation/seat-management' import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' +import { enqueueCancelAtPeriodEndSync } from '@/lib/billing/webhooks/subscription-sync' import { isBillingEnabled } from '@/lib/core/config/env-flags' import { OrchestrationError } from '@/lib/core/orchestration/types' import { enqueueOutboxEvent } from '@/lib/core/outbox/service' @@ -288,9 +289,10 @@ export async function restoreUserProSubscription(userId: string): Promise ({ @@ -8,11 +12,12 @@ vi.mock('@/lib/billing/storage/payer-transfer', () => ({ changeWorkspaceStoragePayersInTx: vi.fn(), })) vi.mock('@/lib/core/outbox/service', () => outboxServiceMock) +vi.mock('@/lib/billing/webhooks/subscription-sync', () => billingSubscriptionSyncMock) import { pauseProSubscriptionForOrgCoverage } from '@/lib/billing/organizations/membership' -import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' -const mockEnqueueOutboxEvent = outboxServiceMockFns.mockEnqueueOutboxEvent +const mockEnqueueCancelAtPeriodEndSync = + billingSubscriptionSyncMockFns.mockEnqueueCancelAtPeriodEndSync const ACTIVE_PERSONAL_PRO = { id: 'sub-personal', @@ -68,15 +73,12 @@ describe('pauseProSubscriptionForOrgCoverage', () => { organizationId: 'org-1', }) expect(dbChainMockFns.set).toHaveBeenCalledWith({ cancelAtPeriodEnd: true }) - expect(mockEnqueueOutboxEvent).toHaveBeenCalledWith( - expect.anything(), - OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, - { - stripeSubscriptionId: 'stripe-sub-personal', - subscriptionId: 'sub-personal', - reason: 'covered-by-organization', - } - ) + expect(mockEnqueueCancelAtPeriodEndSync).toHaveBeenCalledWith(expect.anything(), { + stripeSubscriptionId: 'stripe-sub-personal', + subscriptionId: 'sub-personal', + cancelAtPeriodEnd: true, + reason: 'covered-by-organization', + }) }) it('reports covered even when no entitled personal Pro row exists', async () => { @@ -94,7 +96,7 @@ describe('pauseProSubscriptionForOrgCoverage', () => { organizationId: 'org-1', }) expect(dbChainMockFns.update).not.toHaveBeenCalled() - expect(mockEnqueueOutboxEvent).not.toHaveBeenCalled() + expect(mockEnqueueCancelAtPeriodEndSync).not.toHaveBeenCalled() }) it('reports covered without pausing again when the personal Pro is already pausing', async () => { @@ -113,6 +115,6 @@ describe('pauseProSubscriptionForOrgCoverage', () => { organizationId: 'org-1', }) expect(dbChainMockFns.update).not.toHaveBeenCalled() - expect(mockEnqueueOutboxEvent).not.toHaveBeenCalled() + expect(mockEnqueueCancelAtPeriodEndSync).not.toHaveBeenCalled() }) }) diff --git a/apps/sim/lib/billing/organizations/provision-seat.test.ts b/apps/sim/lib/billing/organizations/provision-seat.test.ts index 6a38cc50eb6..a30f896d796 100644 --- a/apps/sim/lib/billing/organizations/provision-seat.test.ts +++ b/apps/sim/lib/billing/organizations/provision-seat.test.ts @@ -1,12 +1,16 @@ import { billingCoreMock, billingCoreMockFns } from '@sim/testing/mocks/billing-core.mock' import { billingOutboxHandlersMock } from '@sim/testing/mocks/billing-outbox-handlers.mock' import { billingPlanMock, billingPlanMockFns } from '@sim/testing/mocks/billing-plan.mock' +import { + billingSubscriptionSyncMock, + billingSubscriptionSyncMockFns, +} from '@sim/testing/mocks/billing-subscription-sync.mock' import { dbChainMockFns, resetDbChainMock } from '@sim/testing/mocks/database.mock' import { organizationMembershipMock, organizationMembershipMockFns, } from '@sim/testing/mocks/organization-membership.mock' -import { outboxServiceMock, outboxServiceMockFns } from '@sim/testing/mocks/outbox-service.mock' +import { outboxServiceMock } from '@sim/testing/mocks/outbox-service.mock' import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' const { @@ -41,6 +45,8 @@ vi.mock('@/lib/billing/plans', () => ({ vi.mock('@/lib/core/outbox/service', () => outboxServiceMock) +vi.mock('@/lib/billing/webhooks/subscription-sync', () => billingSubscriptionSyncMock) + vi.mock('@/lib/billing/webhooks/outbox-handlers', () => billingOutboxHandlersMock) import { ensureTeamOrganizationForAcceptance } from '@/lib/billing/organizations/provision-seat' @@ -49,7 +55,8 @@ const { mockAcquireOrganizationMutationLock } = organizationMembershipMockFns const mockGetOrganizationSubscription = billingCoreMockFns.mockGetOrganizationSubscription const mockGetHighestPriorityPersonalSubscription = billingPlanMockFns.mockGetHighestPriorityPersonalSubscription -const enqueueMock = outboxServiceMockFns.mockEnqueueOutboxEvent +const { mockEnqueueSubscriptionSeatsSync, mockEnqueueCancelAtPeriodEndSync } = + billingSubscriptionSyncMockFns function testExecutor(onUpdate: () => void = () => {}) { return { @@ -57,7 +64,12 @@ function testExecutor(onUpdate: () => void = () => {}) { set: (values: Record) => { onUpdate() updateCalls.value.push(values) - return { where: () => Promise.resolve([]) } + return { + where: () => + Object.assign(Promise.resolve([]), { + returning: () => Promise.resolve([{ seats: 1 }]), + }), + } }, }), } as never @@ -113,10 +125,9 @@ describe('ensureTeamOrganizationForAcceptance', () => { }) expect(updateCalls.value).toContainEqual(expect.objectContaining({ plan: 'team_6000' })) // The Pro→Team price migration is durably enqueued at conversion time. - expect(enqueueMock).toHaveBeenCalledWith( + expect(mockEnqueueSubscriptionSeatsSync).toHaveBeenCalledWith( executor, - 'stripe.sync-subscription-seats', - expect.objectContaining({ subscriptionId: 'sub-pro' }) + expect.objectContaining({ subscriptionId: 'sub-pro', seats: 1 }) ) expect(mockGetOrganizationSubscription).toHaveBeenCalledWith( 'org-1', @@ -203,19 +214,14 @@ describe('ensureTeamOrganizationForAcceptance', () => { expect.objectContaining({ plan: 'team_6000', referenceId: 'owner-1' }) ) // The plan change enqueues the price seat-sync... - expect(enqueueMock).toHaveBeenCalledWith( + expect(mockEnqueueSubscriptionSeatsSync).toHaveBeenCalledWith( executor, - 'stripe.sync-subscription-seats', - expect.objectContaining({ subscriptionId: 'sub-pro' }) + expect.objectContaining({ subscriptionId: 'sub-pro', seats: 1 }) ) expect(dbChainMockFns.transaction).not.toHaveBeenCalled() expect(lockOrder).toEqual(['organization', 'subscription']) // ...but with no scheduled cancellation there is no cancel-sync event. - expect(enqueueMock).not.toHaveBeenCalledWith( - expect.anything(), - 'stripe.sync-cancel-at-period-end', - expect.anything() - ) + expect(mockEnqueueCancelAtPeriodEndSync).not.toHaveBeenCalled() }) it('blocks personal Pro conversion when the reused organization has unresolved Enterprise', async () => { @@ -243,7 +249,8 @@ describe('ensureTeamOrganizationForAcceptance', () => { }) ).rejects.toThrow('Enterprise issuance is unfinished') expect(updateCalls.value).toHaveLength(0) - expect(enqueueMock).not.toHaveBeenCalled() + expect(mockEnqueueSubscriptionSeatsSync).not.toHaveBeenCalled() + expect(mockEnqueueCancelAtPeriodEndSync).not.toHaveBeenCalled() }) it('provisions an org for a legacy personal-scoped Team subscription without a plan change', async () => { @@ -271,7 +278,8 @@ describe('ensureTeamOrganizationForAcceptance', () => { expect.objectContaining({ plan: 'team', referenceId: 'owner-1' }) ) // No plan change and no scheduled cancellation: nothing to push to Stripe. - expect(enqueueMock).not.toHaveBeenCalled() + expect(mockEnqueueSubscriptionSeatsSync).not.toHaveBeenCalled() + expect(mockEnqueueCancelAtPeriodEndSync).not.toHaveBeenCalled() }) it('returns upgrade-required (no downgrade) when no eligible Team tier exists', async () => { diff --git a/apps/sim/lib/billing/organizations/provision-seat.ts b/apps/sim/lib/billing/organizations/provision-seat.ts index 0d5e55d79c5..5ef2cb4152d 100644 --- a/apps/sim/lib/billing/organizations/provision-seat.ts +++ b/apps/sim/lib/billing/organizations/provision-seat.ts @@ -16,8 +16,10 @@ import { } from '@/lib/billing/plan-helpers' import { getPlanByName } from '@/lib/billing/plans' import { hasUsableSubscriptionStatus } from '@/lib/billing/subscriptions/utils' -import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' -import { enqueueOutboxEvent } from '@/lib/core/outbox/service' +import { + enqueueCancelAtPeriodEndSync, + enqueueSubscriptionSeatsSync, +} from '@/lib/billing/webhooks/subscription-sync' import type { DbOrTx, DbTransaction } from '@/lib/db/types' const logger = createLogger('ProvisionSeat') @@ -248,22 +250,25 @@ async function activateTeamSubscription( Boolean(sub.cancelAtPeriodEnd) && Boolean(sub.stripeSubscriptionId) const apply = async (tx: DbOrTx) => { - await tx + const [activated] = await tx .update(subscriptionTable) .set({ plan: targetPlan, cancelAtPeriodEnd: false }) .where(eq(subscriptionTable.id, sub.id)) + .returning({ seats: subscriptionTable.seats }) if (planChanged) { - await enqueueOutboxEvent(tx, OUTBOX_EVENT_TYPES.STRIPE_SYNC_SUBSCRIPTION_SEATS, { + await enqueueSubscriptionSeatsSync(tx, { subscriptionId: sub.id, + seats: activated?.seats ?? 1, reason: 'pro-to-team-conversion', }) } if (shouldClearCancellation) { - await enqueueOutboxEvent(tx, OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, { + await enqueueCancelAtPeriodEndSync(tx, { stripeSubscriptionId: sub.stripeSubscriptionId as string, subscriptionId: sub.id, + cancelAtPeriodEnd: false, reason: 'pro-to-team-conversion', }) } diff --git a/apps/sim/lib/billing/organizations/seats.test.ts b/apps/sim/lib/billing/organizations/seats.test.ts index d76bf2a02f9..4fda5643dde 100644 --- a/apps/sim/lib/billing/organizations/seats.test.ts +++ b/apps/sim/lib/billing/organizations/seats.test.ts @@ -8,7 +8,10 @@ import { setEnvFlags, } from '@sim/testing' import { billingOutboxHandlersMock } from '@sim/testing/mocks/billing-outbox-handlers.mock' -import { outboxServiceMock, outboxServiceMockFns } from '@sim/testing/mocks/outbox-service.mock' +import { + billingSubscriptionSyncMock, + billingSubscriptionSyncMockFns, +} from '@sim/testing/mocks/billing-subscription-sync.mock' import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' const { mockSyncSubscriptionUsageLimits } = vi.hoisted(() => ({ @@ -19,7 +22,7 @@ vi.mock('@/lib/billing/organization', () => ({ syncSubscriptionUsageLimits: mockSyncSubscriptionUsageLimits, })) -vi.mock('@/lib/core/outbox/service', () => outboxServiceMock) +vi.mock('@/lib/billing/webhooks/subscription-sync', () => billingSubscriptionSyncMock) vi.mock('@/lib/billing/webhooks/outbox-handlers', () => billingOutboxHandlersMock) @@ -27,7 +30,7 @@ vi.mock('@sim/audit', () => auditMock) import { reconcileOrganizationSeats } from '@/lib/billing/organizations/seats' -const enqueueMock = outboxServiceMockFns.mockEnqueueOutboxEvent +const enqueueMock = billingSubscriptionSyncMockFns.mockEnqueueSubscriptionSeatsSync const teamSub = { id: 'sub-1', @@ -48,7 +51,6 @@ afterAll(resetEnvFlagsMock) describe('reconcileOrganizationSeats', () => { beforeEach(() => { resetDbChainMock() - enqueueMock.mockResolvedValue('evt-1') setEnvFlags({ isBillingEnabled: true }) }) @@ -69,11 +71,12 @@ describe('reconcileOrganizationSeats', () => { previousSeats: 1, seats: 2, reason: undefined, - outboxEventId: 'evt-1', + outboxEventId: 'subscription-seats-sync-event', }) expect(dbChainMockFns.set).toHaveBeenCalledWith({ seats: 2 }) - expect(enqueueMock).toHaveBeenCalledWith(expect.anything(), 'stripe.sync-subscription-seats', { + expect(enqueueMock).toHaveBeenCalledWith(expect.anything(), { subscriptionId: 'sub-1', + seats: 2, reason: 'member-accepted-invite', }) expect(mockSyncSubscriptionUsageLimits).toHaveBeenCalledWith( diff --git a/apps/sim/lib/billing/organizations/seats.ts b/apps/sim/lib/billing/organizations/seats.ts index 7b2d02e70d6..b50e38c80eb 100644 --- a/apps/sim/lib/billing/organizations/seats.ts +++ b/apps/sim/lib/billing/organizations/seats.ts @@ -6,9 +6,8 @@ import { and, count, desc, eq, inArray } from 'drizzle-orm' import { syncSubscriptionUsageLimits } from '@/lib/billing/organization' import { isTeam } from '@/lib/billing/plan-helpers' import { ENTITLED_SUBSCRIPTION_STATUSES } from '@/lib/billing/subscriptions/utils' -import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' +import { enqueueSubscriptionSeatsSync } from '@/lib/billing/webhooks/subscription-sync' import { isBillingEnabled } from '@/lib/core/config/env-flags' -import { enqueueOutboxEvent } from '@/lib/core/outbox/service' import { captureServerEvent } from '@/lib/posthog/server' const logger = createLogger('OrganizationSeats') @@ -112,14 +111,11 @@ export async function reconcileOrganizationSeats({ .set({ seats: targetSeats }) .where(eq(subscription.id, orgSubscription.id)) - const outboxEventId = await enqueueOutboxEvent( - tx, - OUTBOX_EVENT_TYPES.STRIPE_SYNC_SUBSCRIPTION_SEATS, - { - subscriptionId: orgSubscription.id, - reason, - } - ) + const outboxEventId = await enqueueSubscriptionSeatsSync(tx, { + subscriptionId: orgSubscription.id, + seats: targetSeats, + reason, + }) return { kind: 'changed', diff --git a/apps/sim/lib/billing/webhooks/outbox-events.ts b/apps/sim/lib/billing/webhooks/outbox-events.ts index 5a7b53307c5..0cc6bf3946a 100644 --- a/apps/sim/lib/billing/webhooks/outbox-events.ts +++ b/apps/sim/lib/billing/webhooks/outbox-events.ts @@ -1,19 +1,27 @@ export const OUTBOX_EVENT_TYPES = { /** * Sync a subscription's `cancel_at_period_end` flag from our DB to - * Stripe. The handler reads the current DB value at processing time - * — so rapid cancel→uncancel→cancel sequences always converge on - * the last-committed DB state regardless of outbox ordering. Callers - * enqueue this event after every DB change to `cancelAtPeriodEnd`. + * Stripe. Enqueue through `enqueueCancelAtPeriodEndSync` in the same + * transaction as every DB change to `cancelAtPeriodEnd`. + * + * Guarantee: once every in-flight event for a subscription completes, + * Stripe holds the last value committed to the DB. Each handler pushes + * the row's current value (not its payload's) and re-reads the row + * after its Stripe write, retrying while the value moved, so racing + * events converge even when an earlier request lands in Stripe last. + * While an event is in flight, `reconcileSubscriptionSyncFromStripe` + * keeps webhook echoes and stale snapshots from overwriting the + * committed value; a change made in Stripe itself wins over it. */ STRIPE_SYNC_CANCEL_AT_PERIOD_END: 'stripe.sync-cancel-at-period-end', /** Cancel in Stripe; the verified deletion webhook remains the only DB entitlement authority. */ STRIPE_CANCEL_SUBSCRIPTION_IMMEDIATELY: 'stripe.cancel-subscription-immediately', /** * Sync a Team subscription's price and seat quantity from our DB to - * Stripe. The handler reads the current DB plan + seats at processing - * time and reconciles the Stripe item's price (e.g. after a Pro→Team - * conversion) and quantity, charging the proration via `always_invoice`. + * Stripe. Enqueue through `enqueueSubscriptionSeatsSync`. The handler + * reads the current DB plan + seats at processing time and reconciles + * the Stripe item's price (e.g. after a Pro→Team conversion) and + * quantity, charging the proration via `always_invoice`. * A failed charge surfaces through Stripe dunning and the existing * billing-blocked system, never under the synchronous accept path. */ diff --git a/apps/sim/lib/billing/webhooks/outbox-handlers.ts b/apps/sim/lib/billing/webhooks/outbox-handlers.ts index 1d3aa61109e..50e06bd6b40 100644 --- a/apps/sim/lib/billing/webhooks/outbox-handlers.ts +++ b/apps/sim/lib/billing/webhooks/outbox-handlers.ts @@ -3,6 +3,7 @@ import { db } from '@sim/db' import { member, subscription as subscriptionTable, user } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' +import { generateShortId } from '@sim/utils/id' import { and, eq } from 'drizzle-orm' import { isTeam } from '@/lib/billing/plan-helpers' import { getPlanByName } from '@/lib/billing/plans' @@ -10,23 +11,15 @@ import { requireStripeClient } from '@/lib/billing/stripe-client' import { resolveDefaultPaymentMethod } from '@/lib/billing/stripe-payment-method' import { hasPaidSubscriptionStatus } from '@/lib/billing/subscriptions/utils' import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' +import { + type CancelAtPeriodEndSyncPayload, + cancelAtPeriodEndSyncIdempotencyKey, + type SubscriptionSeatsSyncPayload, +} from '@/lib/billing/webhooks/subscription-sync' import type { OutboxHandler } from '@/lib/core/outbox/service' const logger = createLogger('BillingOutboxHandlers') -interface StripeSyncCancelAtPeriodEndPayload { - stripeSubscriptionId: string - /** The DB subscription row id — also our source-of-truth pointer. */ - subscriptionId: string - /** Optional: reason this was enqueued — e.g. 'member-joined-paid-org'. */ - reason?: string - /** Correlates Enterprise-issuance follow-up work for Admin progress/retry. */ - sourceOperationId?: string - operationId?: string - organizationId?: string - requestedBy?: { id: string | null; name: string; email: string | null } -} - interface StripeCancelSubscriptionImmediatelyPayload { stripeSubscriptionId: string subscriptionId: string @@ -62,12 +55,6 @@ async function recordAdminCancellationAudit(params: { }) } -interface StripeSyncSubscriptionSeatsPayload { - /** The DB subscription row id — the handler reads current seats from this row. */ - subscriptionId: string - reason?: string -} - interface StripeSyncCustomerContactPayload { /** The DB subscription row id — handler resolves current owner/contact at processing time. */ subscriptionId: string @@ -102,42 +89,74 @@ async function getSubscriptionSeatSyncState(subscriptionId: string) { return row ?? null } -const stripeSyncCancelAtPeriodEnd: OutboxHandler = async ( +async function readCancelAtPeriodEnd(subscriptionId: string): Promise { + const [row] = await db + .select({ cancelAtPeriodEnd: subscriptionTable.cancelAtPeriodEnd }) + .from(subscriptionTable) + .where(eq(subscriptionTable.id, subscriptionId)) + .limit(1) + return row ? Boolean(row.cancelAtPeriodEnd) : null +} + +/** + * Pushes the row's current value, never the payload's: racing events for one subscription each + * converge on the last committed value. Stripe is read first and written only when it differs, + * and the row is re-read after the write so a value committed while this event's request was in + * flight is pushed too, even when an earlier event's request lands in Stripe after a newer one. + */ +const stripeSyncCancelAtPeriodEnd: OutboxHandler = async ( payload, ctx ) => { await recordAdminCancellationAudit({ ...payload, timing: 'period_end' }) - // Read the DB value at processing time (not at enqueue time). This - // makes the handler idempotent across racing enqueues: multiple - // events for the same subscription all push whatever the DB - // currently says, converging on the last committed value. - const rows = await db - .select({ cancelAtPeriodEnd: subscriptionTable.cancelAtPeriodEnd }) - .from(subscriptionTable) - .where(eq(subscriptionTable.id, payload.subscriptionId)) - .limit(1) + const stripe = requireStripeClient() + const maxSyncAttempts = 2 - if (rows.length === 0) { - logger.warn('Subscription not found when syncing cancel_at_period_end', { + for (let attempt = 1; attempt <= maxSyncAttempts; attempt++) { + const desiredValue = await readCancelAtPeriodEnd(payload.subscriptionId) + if (desiredValue === null) { + logger.warn('Subscription not found when syncing cancel_at_period_end', { + eventId: ctx.eventId, + subscriptionId: payload.subscriptionId, + }) + return + } + + const stripeSubscription = await stripe.subscriptions.retrieve(payload.stripeSubscriptionId) + const needsUpdate = stripeSubscription.cancel_at_period_end !== desiredValue + if (needsUpdate) { + await stripe.subscriptions.update( + payload.stripeSubscriptionId, + { cancel_at_period_end: desiredValue }, + { idempotencyKey: cancelAtPeriodEndSyncIdempotencyKey(ctx.eventId) } + ) + } + + const latestValue = await readCancelAtPeriodEnd(payload.subscriptionId) + if (latestValue !== desiredValue) { + logger.info('cancel_at_period_end changed during Stripe sync; retrying latest value', { + eventId: ctx.eventId, + subscriptionId: payload.subscriptionId, + stripeSubscriptionId: payload.stripeSubscriptionId, + attemptedValue: desiredValue, + latestValue, + attempt, + }) + continue + } + + logger.info('Synced cancel_at_period_end from DB to Stripe', { + eventId: ctx.eventId, + stripeSubscriptionId: payload.stripeSubscriptionId, subscriptionId: payload.subscriptionId, + desiredValue, + alreadySynced: !needsUpdate, + reason: payload.reason, }) return } - const desiredValue = Boolean(rows[0].cancelAtPeriodEnd) - const stripe = requireStripeClient() - await stripe.subscriptions.update( - payload.stripeSubscriptionId, - { cancel_at_period_end: desiredValue }, - { idempotencyKey: `outbox:${ctx.eventId}` } - ) - logger.info('Synced cancel_at_period_end from DB to Stripe', { - eventId: ctx.eventId, - stripeSubscriptionId: payload.stripeSubscriptionId, - subscriptionId: payload.subscriptionId, - desiredValue, - reason: payload.reason, - }) + throw new Error(`cancel_at_period_end changed while syncing ${payload.subscriptionId}`) } const stripeCancelSubscriptionImmediately: OutboxHandler< @@ -160,7 +179,7 @@ const stripeCancelSubscriptionImmediately: OutboxHandler< }) } -const stripeSyncSubscriptionSeats: OutboxHandler = async ( +const stripeSyncSubscriptionSeats: OutboxHandler = async ( payload, ctx ) => { @@ -369,33 +388,31 @@ const stripeThresholdOverageInvoice: OutboxHandler = async ( - payload, - ctx -) => { +type CustomerContactState = + | { status: 'ready'; stripeCustomerId: string; email: string; name: string } + | { status: 'skipped'; reason: string; organizationId?: string } + +async function readCustomerContact(subscriptionId: string): Promise { const [subscriptionRow] = await db .select({ referenceId: subscriptionTable.referenceId, stripeCustomerId: subscriptionTable.stripeCustomerId, }) .from(subscriptionTable) - .where(eq(subscriptionTable.id, payload.subscriptionId)) + .where(eq(subscriptionTable.id, subscriptionId)) .limit(1) if (!subscriptionRow) { - logger.warn('Subscription not found when syncing Stripe customer contact', { - eventId: ctx.eventId, - subscriptionId: payload.subscriptionId, - }) - return + return { + status: 'skipped', + reason: 'Subscription not found when syncing Stripe customer contact', + } } - if (!subscriptionRow.stripeCustomerId) { - logger.warn('Subscription has no Stripe customer id when syncing contact', { - eventId: ctx.eventId, - subscriptionId: payload.subscriptionId, - }) - return + return { + status: 'skipped', + reason: 'Subscription has no Stripe customer id when syncing contact', + } } const [owner] = await db @@ -409,29 +426,86 @@ const stripeSyncCustomerContact: OutboxHandler .limit(1) if (!owner) { - logger.warn('Organization owner not found when syncing Stripe customer contact', { + return { + status: 'skipped', + reason: 'Organization owner not found when syncing Stripe customer contact', + organizationId: subscriptionRow.referenceId, + } + } + + return { + status: 'ready', + stripeCustomerId: subscriptionRow.stripeCustomerId, + email: owner.email, + name: owner.name, + } +} + +/** + * Pushes the organization owner's current contact, re-reading it after the write so an + * ownership change committed while this event's request was in flight is pushed too. + */ +const stripeSyncCustomerContact: OutboxHandler = async ( + payload, + ctx +) => { + const stripe = requireStripeClient() + const maxSyncAttempts = 2 + + for (let attempt = 1; attempt <= maxSyncAttempts; attempt++) { + const contact = await readCustomerContact(payload.subscriptionId) + if (contact.status === 'skipped') { + logger.warn(contact.reason, { + eventId: ctx.eventId, + subscriptionId: payload.subscriptionId, + ...(contact.organizationId ? { organizationId: contact.organizationId } : {}), + }) + return + } + + const customer = await stripe.customers.retrieve(contact.stripeCustomerId) + if (customer.deleted) { + throw new Error(`Stripe customer ${contact.stripeCustomerId} is deleted`) + } + const needsUpdate = + customer.email !== contact.email || Boolean(contact.name && customer.name !== contact.name) + if (needsUpdate) { + await stripe.customers.update( + contact.stripeCustomerId, + { + email: contact.email, + ...(contact.name ? { name: contact.name } : {}), + }, + { idempotencyKey: `outbox:${ctx.eventId}:${generateShortId()}` } + ) + } + + const latest = await readCustomerContact(payload.subscriptionId) + if ( + latest.status !== 'ready' || + latest.stripeCustomerId !== contact.stripeCustomerId || + latest.email !== contact.email || + latest.name !== contact.name + ) { + logger.info('Stripe customer contact changed during sync; retrying latest value', { + eventId: ctx.eventId, + subscriptionId: payload.subscriptionId, + attempt, + }) + continue + } + + logger.info('Synced Stripe customer contact', { eventId: ctx.eventId, + stripeCustomerId: contact.stripeCustomerId, subscriptionId: payload.subscriptionId, - organizationId: subscriptionRow.referenceId, + alreadySynced: !needsUpdate, + reason: payload.reason, }) return } - const stripe = requireStripeClient() - await stripe.customers.update( - subscriptionRow.stripeCustomerId, - { - email: owner.email, - ...(owner.name ? { name: owner.name } : {}), - }, - { idempotencyKey: `outbox:${ctx.eventId}` } - ) - logger.info('Synced Stripe customer contact', { - eventId: ctx.eventId, - stripeCustomerId: subscriptionRow.stripeCustomerId, - subscriptionId: payload.subscriptionId, - reason: payload.reason, - }) + throw new Error(`Stripe customer contact changed while syncing ${payload.subscriptionId}`) } export const billingOutboxHandlers = { diff --git a/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts b/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts new file mode 100644 index 00000000000..ede3b58165d --- /dev/null +++ b/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts @@ -0,0 +1,444 @@ +/** + * DB → Stripe sync convergence against real PostgreSQL, the real outbox worker path, and the + * real Better Auth Stripe webhook endpoint (plugin write first, then Sim's callbacks), with an + * in-memory Stripe that applies requests in the order the test releases them. + */ + +import { stripe as stripePlugin } from '@better-auth/stripe' +import * as schema from '@sim/db/schema' +import { member, organization, outboxEvent, subscription, user } from '@sim/db/schema' +import { readTestDatabaseUrl } from '@sim/db/testing/test-infrastructure' +import { withUtcTimestamps } from '@sim/db/timestamps' +import { envFlagsMock, resetEnvFlagsMock, setEnvFlags } from '@sim/testing/mocks/env-flags.mock' +import { + createInMemoryStripe, + type InMemoryStripe, + stripeClientMock, +} from '@sim/testing/mocks/stripe.mock' +import { generateId } from '@sim/utils/id' +import { type BetterAuthOptions, betterAuth } from 'better-auth' +import { and, asc, eq, sql } from 'drizzle-orm' +import { drizzle, type PostgresJsDatabase } from 'drizzle-orm/postgres-js' +import postgres from 'postgres' +import type Stripe from 'stripe' +import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' + +const database = vi.hoisted(() => ({ + current: undefined as PostgresJsDatabase | undefined, +})) + +vi.mock('@sim/db', () => ({ + get db() { + if (!database.current) throw new Error('Stripe sync test database is not initialized') + return database.current + }, +})) +vi.mock('@/lib/billing/stripe-client', () => stripeClientMock) +vi.mock('@/lib/core/config/env-flags', () => envFlagsMock) + +import { createSimAuthAdapter } from '@/lib/auth/sim-auth-adapter' +import { + pauseProSubscriptionForOrgCoverage, + restoreUserProSubscription, +} from '@/lib/billing/organizations/membership' +import { reconcileOrganizationSeats } from '@/lib/billing/organizations/seats' +import { isTeam } from '@/lib/billing/plan-helpers' +import { syncSeatsFromStripeQuantity } from '@/lib/billing/validation/seat-management' +import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' +import { billingOutboxHandlers } from '@/lib/billing/webhooks/outbox-handlers' +import { reconcileSubscriptionSyncFromStripe } from '@/lib/billing/webhooks/subscription-sync' +import { enqueueOutboxEvent, processOutboxEventById } from '@/lib/core/outbox/service' + +const schemaName = `stripe_sync_${generateId().replaceAll('-', '')}` +const connection = postgres( + readTestDatabaseUrl(), + withUtcTimestamps({ + max: 6, + prepare: false, + fetch_types: false, + connection: { search_path: schemaName }, + onnotice: () => {}, + }) +) +const testDatabase = drizzle(connection, { schema }) + +let stripe: InMemoryStripe + +/** + * Sim's Stripe plugin callbacks from `lib/auth/auth.ts`, reduced to the parts that touch the + * synced fields: the seat sync in `onSubscriptionUpdate` and the reconcile step in `onEvent`. + */ +function createWebhookEndpoint() { + const auth = betterAuth({ + baseURL: 'http://localhost:3000', + secret: 'isolated-integration-fixture-secret-not-a-real-credential', + database: (options: BetterAuthOptions) => createSimAuthAdapter(options, testDatabase), + plugins: [ + stripePlugin({ + stripeClient: stripe.client, + stripeWebhookSecret: 'whsec_fixture', + subscription: { + enabled: true, + plans: [], + onSubscriptionUpdate: async ({ event, subscription: updated }) => { + const stripeSubscription = event.data.object as Stripe.Subscription + if (!isTeam(updated.plan)) return + await syncSeatsFromStripeQuantity( + updated.id, + updated.seats ?? null, + stripeSubscription.items?.data?.[0]?.quantity || 1 + ) + }, + }, + onEvent: reconcileSubscriptionSyncFromStripe, + }), + ], + }) + + return async function deliver(event: Stripe.Event) { + const response = await auth.handler( + new Request('http://localhost:3000/api/auth/stripe/webhook', { + method: 'POST', + headers: { 'content-type': 'application/json', 'stripe-signature': 't=1,v1=fixture' }, + body: JSON.stringify(event), + }) + ) + expect(response.status).toBe(200) + } +} + +let deliver: ReturnType + +beforeAll(async () => { + await connection`CREATE SCHEMA ${connection(schemaName)}` + for (const table of ['subscription', 'outbox_event', 'member', 'user', 'organization']) { + await connection.unsafe(`CREATE TABLE "${table}" (LIKE public."${table}" INCLUDING ALL)`) + } + database.current = testDatabase +}) + +beforeEach(() => { + stripe = createInMemoryStripe() + stripeClientMock.requireStripeClient.mockReturnValue(stripe.client) + deliver = createWebhookEndpoint() +}) + +afterAll(async () => { + resetEnvFlagsMock() + try { + await connection`DROP SCHEMA ${connection(schemaName)} CASCADE` + } finally { + await connection.end() + database.current = undefined + } +}) + +async function createUser(label: string) { + const id = generateId() + const now = new Date() + await testDatabase.insert(user).values({ + id, + name: label, + email: `${label}-${id}@example.com`, + emailVerified: true, + createdAt: now, + updatedAt: now, + }) + return { id, email: `${label}-${id}@example.com`, name: label } +} + +async function createOrganizationWithPlan(plan: 'team', seats = 1) { + const organizationId = generateId() + await testDatabase + .insert(organization) + .values({ id: organizationId, name: 'Org', slug: organizationId }) + const subscriptionId = generateId() + const stripeSubscriptionId = `sub_${subscriptionId}` + const stripeCustomerId = `cus_${subscriptionId}` + await testDatabase.insert(subscription).values({ + id: subscriptionId, + plan, + referenceId: organizationId, + status: 'active', + seats, + stripeSubscriptionId, + stripeCustomerId, + cancelAtPeriodEnd: false, + }) + stripe.addSubscription({ id: stripeSubscriptionId, customer: stripeCustomerId, quantity: seats }) + return { organizationId, subscriptionId, stripeSubscriptionId, stripeCustomerId } +} + +async function addMember(organizationId: string, userId: string, role = 'member') { + await testDatabase + .insert(member) + .values({ id: generateId(), organizationId, userId, role, createdAt: new Date() }) +} + +/** A user on personal Pro, synced with Stripe, who belongs to a paid Team organization. */ +async function createProUserInPaidOrganization() { + const proUser = await createUser('pro') + const subscriptionId = generateId() + const stripeSubscriptionId = `sub_${subscriptionId}` + await testDatabase.insert(subscription).values({ + id: subscriptionId, + plan: 'pro', + referenceId: proUser.id, + status: 'active', + seats: 1, + stripeSubscriptionId, + stripeCustomerId: `cus_${subscriptionId}`, + cancelAtPeriodEnd: false, + }) + stripe.addSubscription({ id: stripeSubscriptionId, customer: `cus_${subscriptionId}` }) + const paidOrganization = await createOrganizationWithPlan('team') + await addMember(paidOrganization.organizationId, proUser.id) + return { userId: proUser.id, subscriptionId, stripeSubscriptionId, paidOrganization } +} + +async function leaveOrganization(userId: string, organizationId: string) { + await testDatabase + .delete(member) + .where(and(eq(member.userId, userId), eq(member.organizationId, organizationId))) +} + +async function outboxEventIds(eventType: string, subscriptionId: string) { + const rows = await testDatabase + .select({ id: outboxEvent.id }) + .from(outboxEvent) + .where( + and( + eq(outboxEvent.eventType, eventType), + sql`${outboxEvent.payload} ->> 'subscriptionId' = ${subscriptionId}` + ) + ) + .orderBy(asc(outboxEvent.createdAt), asc(outboxEvent.id)) + return rows.map((row) => row.id) +} + +async function latestOutboxEventId(eventType: string, subscriptionId: string) { + const ids = await outboxEventIds(eventType, subscriptionId) + const id = ids.at(-1) + if (!id) throw new Error(`No ${eventType} event for ${subscriptionId}`) + return id +} + +function processEvent(eventId: string) { + return processOutboxEventById(eventId, billingOutboxHandlers) +} + +async function makeDue(eventId: string) { + await testDatabase + .update(outboxEvent) + .set({ availableAt: new Date() }) + .where(eq(outboxEvent.id, eventId)) +} + +async function storedSubscription(subscriptionId: string) { + const [row] = await testDatabase + .select({ cancelAtPeriodEnd: subscription.cancelAtPeriodEnd, seats: subscription.seats }) + .from(subscription) + .where(eq(subscription.id, subscriptionId)) + if (!row) throw new Error(`Subscription ${subscriptionId} not found`) + return row +} + +describe('cancel_at_period_end sync', () => { + it('pushes the latest value when an earlier sync lands in Stripe after a newer one', async () => { + const pro = await createProUserInPaidOrganization() + await pauseProSubscriptionForOrgCoverage(pro.userId) + const pauseSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + + const gate = stripe.holdNextUpdate('subscriptions') + const pausing = processEvent(pauseSync) + await gate.reached + + await leaveOrganization(pro.userId, pro.paidOrganization.organizationId) + await restoreUserProSubscription(pro.userId) + const restoreSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + await expect(processEvent(restoreSync)).resolves.toBe('completed') + + gate.release() + await pausing + + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(false) + expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(false) + + for (const event of stripe.events) await deliver(event) + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(false) + }) + + it('keeps the latest committed value when the echo of an earlier sync arrives mid-sync', async () => { + const pro = await createProUserInPaidOrganization() + await pauseProSubscriptionForOrgCoverage(pro.userId) + const pauseSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + + const stalePush = stripe.holdNextUpdate('subscriptions') + const pausing = processEvent(pauseSync) + await stalePush.reached + await leaveOrganization(pro.userId, pro.paidOrganization.organizationId) + await restoreUserProSubscription(pro.userId) + + const correctingPush = stripe.holdNextUpdate('subscriptions') + stalePush.release() + await correctingPush.reached + expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(true) + await deliver(stripe.events.at(-1) as Stripe.Event) + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(false) + + correctingPush.release() + await expect(pausing).resolves.toBe('completed') + const restoreSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + await expect(processEvent(restoreSync)).resolves.toBe('completed') + expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(false) + }) + + it('retries after a request reached Stripe but failed on the client and the value then changed', async () => { + const pro = await createProUserInPaidOrganization() + await pauseProSubscriptionForOrgCoverage(pro.userId) + const pauseSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + + stripe.failNextUpdateAfterApplying('subscriptions') + await expect(processEvent(pauseSync)).resolves.toBe('pending') + expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(true) + + await leaveOrganization(pro.userId, pro.paidOrganization.organizationId) + await restoreUserProSubscription(pro.userId) + await makeDue(pauseSync) + + await expect(processEvent(pauseSync)).resolves.toBe('completed') + expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(false) + }) + + it('keeps a change that has not reached Stripe when an unrelated subscription update arrives', async () => { + const pro = await createProUserInPaidOrganization() + await pauseProSubscriptionForOrgCoverage(pro.userId) + const pauseSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + + const renewal = stripe.updateOutsideSim(pro.stripeSubscriptionId, { + metadata: { renewedAt: 'period-2' }, + }) + expect(renewal.cancel_at_period_end).toBe(false) + await deliver(stripe.events.at(-1) as Stripe.Event) + + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(true) + await expect(processEvent(pauseSync)).resolves.toBe('completed') + expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(true) + }) + + it('keeps the live Stripe value when its webhooks arrive out of order', async () => { + const pro = await createProUserInPaidOrganization() + stripe.updateOutsideSim(pro.stripeSubscriptionId, { cancel_at_period_end: true }) + stripe.updateOutsideSim(pro.stripeSubscriptionId, { cancel_at_period_end: false }) + const [cancelled, renewed] = stripe.events + + await deliver(renewed) + await deliver(cancelled) + + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(false) + }) + + it('lets a change made in Stripe while a sync is pending win over the pending value', async () => { + const pro = await createProUserInPaidOrganization() + await pauseProSubscriptionForOrgCoverage(pro.userId) + const pauseSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + + stripe.updateOutsideSim(pro.stripeSubscriptionId, { cancel_at_period_end: true }) + stripe.updateOutsideSim(pro.stripeSubscriptionId, { cancel_at_period_end: false }) + for (const event of stripe.events) await deliver(event) + + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(false) + await expect(processEvent(pauseSync)).resolves.toBe('completed') + expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(false) + }) +}) + +describe('customer contact sync', () => { + it('pushes the current owner when an earlier sync lands in Stripe after a newer one', async () => { + const [first, second, third] = await Promise.all([ + createUser('first-owner'), + createUser('second-owner'), + createUser('third-owner'), + ]) + const org = await createOrganizationWithPlan('team') + stripe.addCustomer({ id: org.stripeCustomerId, email: first.email, name: first.name }) + await addMember(org.organizationId, first.id, 'owner') + await addMember(org.organizationId, second.id) + await addMember(org.organizationId, third.id) + + async function transferOwnership(from: string, to: string) { + await testDatabase.transaction(async (tx) => { + await tx.update(member).set({ role: 'admin' }).where(eq(member.userId, from)) + await tx.update(member).set({ role: 'owner' }).where(eq(member.userId, to)) + await enqueueOutboxEvent(tx, OUTBOX_EVENT_TYPES.STRIPE_SYNC_CUSTOMER_CONTACT, { + subscriptionId: org.subscriptionId, + reason: 'ownership-transfer', + }) + }) + return latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CUSTOMER_CONTACT, + org.subscriptionId + ) + } + + const firstSync = await transferOwnership(first.id, second.id) + const gate = stripe.holdNextUpdate('customers') + const firstSyncRun = processEvent(firstSync) + await gate.reached + + const secondSync = await transferOwnership(second.id, third.id) + await expect(processEvent(secondSync)).resolves.toBe('completed') + gate.release() + await firstSyncRun + + expect(stripe.customer(org.stripeCustomerId)).toMatchObject({ + email: third.email, + name: third.name, + }) + }) +}) + +describe('Team seat sync', () => { + beforeAll(() => setEnvFlags({ isBillingEnabled: true })) + + it('keeps a seat change that has not reached Stripe when a stale subscription update arrives', async () => { + const [owner, joiner] = await Promise.all([createUser('owner'), createUser('joiner')]) + const org = await createOrganizationWithPlan('team', 1) + await addMember(org.organizationId, owner.id, 'owner') + await addMember(org.organizationId, joiner.id) + await reconcileOrganizationSeats({ organizationId: org.organizationId, reason: 'member-added' }) + expect((await storedSubscription(org.subscriptionId)).seats).toBe(2) + const seatSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_SUBSCRIPTION_SEATS, + org.subscriptionId + ) + + stripe.updateOutsideSim(org.stripeSubscriptionId, { metadata: { renewedAt: 'period-2' } }) + await deliver(stripe.events.at(-1) as Stripe.Event) + + expect((await storedSubscription(org.subscriptionId)).seats).toBe(2) + await expect(processEvent(seatSync)).resolves.toBe('completed') + expect(stripe.subscription(org.stripeSubscriptionId).items.data[0].quantity).toBe(2) + }) +}) diff --git a/apps/sim/lib/billing/webhooks/subscription-sync.ts b/apps/sim/lib/billing/webhooks/subscription-sync.ts new file mode 100644 index 00000000000..6c545aafa7b --- /dev/null +++ b/apps/sim/lib/billing/webhooks/subscription-sync.ts @@ -0,0 +1,224 @@ +import { db } from '@sim/db' +import { outboxEvent, subscription } from '@sim/db/schema' +import { createLogger } from '@sim/logger' +import { generateShortId } from '@sim/utils/id' +import { toRecord } from '@sim/utils/object' +import { and, eq, inArray, isNotNull, sql } from 'drizzle-orm' +import type Stripe from 'stripe' +import { requireStripeClient } from '@/lib/billing/stripe-client' +import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' +import { enqueueOutboxEvent, patchOutboxEventPayload } from '@/lib/core/outbox/service' +import type { DbOrTx } from '@/lib/db/types' + +const logger = createLogger('BillingSubscriptionSync') + +/** + * Prefix of the idempotency key on every `cancel_at_period_end` write the sync handler sends. + * Stripe copies the key onto the resulting event's `request.idempotency_key`, which is how a + * webhook is recognized as the echo of Sim's own sync rather than a change made in Stripe. + */ +const CANCEL_AT_PERIOD_END_SYNC_KEY_PREFIX = 'outbox-sync-cancel-at-period-end:' + +/** + * The value Sim committed for a synced field, recorded on the sync event. `requestedAt` is the + * database clock read while the enqueuing transaction held the subscription row lock, so among + * a subscription's in-flight events the latest `requestedAt` is the latest committed value. + * Transaction start time (`created_at`) cannot order them: a transaction that began earlier + * can take the row lock later. + */ +interface SyncIntent { + requestedAt: string +} + +export interface CancelAtPeriodEndSyncPayload extends Partial { + stripeSubscriptionId: string + /** The DB subscription row id; the handler pushes this row's current value. */ + subscriptionId: string + /** The value committed with this event. Absent on events enqueued before it was recorded. */ + cancelAtPeriodEnd?: boolean + /** Reason this was enqueued, e.g. 'joined-paid-org'. */ + reason?: string + /** Correlates Enterprise-issuance follow-up work for Admin progress/retry. */ + sourceOperationId?: string + operationId?: string + organizationId?: string + requestedBy?: { id: string | null; name: string; email: string | null } +} + +export interface SubscriptionSeatsSyncPayload extends Partial { + /** The DB subscription row id; the handler pushes this row's current plan and seats. */ + subscriptionId: string + /** The seat count committed with this event. Absent on events enqueued before it was recorded. */ + seats?: number + reason?: string +} + +async function readDatabaseClock(executor: DbOrTx): Promise { + const [row] = await executor.execute<{ requestedAt: string }>( + sql`select to_json(clock_timestamp()) #>> '{}' as "requestedAt"` + ) + if (!row) throw new Error('Database clock read returned no row') + return row.requestedAt +} + +/** + * Enqueue the Stripe sync for a `cancelAtPeriodEnd` value written in this transaction. The + * caller must hold the subscription row lock (`FOR UPDATE`, or the `UPDATE` itself). + */ +export async function enqueueCancelAtPeriodEndSync( + tx: DbOrTx, + payload: Omit & { + cancelAtPeriodEnd: boolean + } +): Promise { + const intent: CancelAtPeriodEndSyncPayload = { + ...payload, + requestedAt: await readDatabaseClock(tx), + } + return enqueueOutboxEvent(tx, OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, intent) +} + +/** + * Re-records the committed value on an existing cancel-sync event that is being retried, so it + * orders after every event enqueued since. The caller must hold the subscription row lock. + */ +export async function recommitCancelAtPeriodEndSync( + tx: DbOrTx, + eventId: string, + cancelAtPeriodEnd: boolean +): Promise { + const patch: Pick = { + cancelAtPeriodEnd, + requestedAt: await readDatabaseClock(tx), + } + await patchOutboxEventPayload(tx, eventId, patch) +} + +/** + * Enqueue the Stripe sync for a Team subscription's plan and `seats` as written in this + * transaction. The caller must hold the subscription row lock. + */ +export async function enqueueSubscriptionSeatsSync( + tx: DbOrTx, + payload: Omit & { seats: number } +): Promise { + const intent: SubscriptionSeatsSyncPayload = { + ...payload, + requestedAt: await readDatabaseClock(tx), + } + return enqueueOutboxEvent(tx, OUTBOX_EVENT_TYPES.STRIPE_SYNC_SUBSCRIPTION_SEATS, intent) +} + +/** A fresh key per Stripe write: the SDK reuses it across its own network retries of that call. */ +export function cancelAtPeriodEndSyncIdempotencyKey(eventId: string): string { + return `${CANCEL_AT_PERIOD_END_SYNC_KEY_PREFIX}${eventId}:${generateShortId()}` +} + +/** The value of the most recently committed in-flight sync event, or undefined. */ +async function readInflightIntent( + tx: DbOrTx, + eventType: string, + subscriptionId: string +): Promise | undefined> { + const [latest] = await tx + .select({ payload: outboxEvent.payload }) + .from(outboxEvent) + .where( + and( + eq(outboxEvent.eventType, eventType), + inArray(outboxEvent.status, ['pending', 'processing']), + sql`${outboxEvent.payload} ->> 'subscriptionId' = ${subscriptionId}`, + isNotNull(sql`${outboxEvent.payload} ->> 'requestedAt'`) + ) + ) + .orderBy(sql`(${outboxEvent.payload} ->> 'requestedAt')::timestamptz desc`) + .limit(1) + return latest ? toRecord(latest.payload) : undefined +} + +/** True when the event records a `cancel_at_period_end` change made in Stripe, not by Sim's sync. */ +function isCancellationChangedInStripe(event: Stripe.Event): boolean { + const previousAttributes = toRecord(event.data.previous_attributes) + if (!('cancel_at_period_end' in previousAttributes)) return false + const idempotencyKey = event.request?.idempotency_key + return !idempotencyKey?.startsWith(CANCEL_AT_PERIOD_END_SYNC_KEY_PREFIX) +} + +/** + * Reconciles the Sim-owned subscription fields after the Better Auth Stripe plugin has copied a + * `customer.subscription.updated` payload into the row. The plugin writes `cancelAtPeriodEnd` + * and `seats` unconditionally, so a delayed, out-of-order, or unrelated event would otherwise + * overwrite a value Sim committed but has not yet pushed, and the pending sync would then push + * the overwritten value back to Stripe. + * + * Under the subscription row lock (the lock every enqueuing writer holds): + * - `cancelAtPeriodEnd`: while a cancel sync is in flight, Sim's latest committed value wins over + * snapshots and over echoes of Sim's own earlier writes. A change made in Stripe itself (customer + * portal, dashboard, Better Auth's cancel/restore endpoints) is newer than anything pending and + * wins, as does Stripe whenever no sync is in flight. Stripe's value is read live, never taken + * from the event, so out-of-order delivery cannot regress it. + * - `seats`: Team seats follow the member count Sim maintains; while a seat sync is in flight its + * latest committed value wins. With none in flight the plugin's write stands, as before. + * + * Only the DB row is written: nothing is enqueued and nothing is sent to Stripe, so this cannot + * trigger another webhook. The in-flight sync pushes the restored value. + */ +export async function reconcileSubscriptionSyncFromStripe(event: Stripe.Event): Promise { + if (event.type !== 'customer.subscription.updated') return + const stripeSubscriptionId = event.data.object.id + + const [row] = await db + .select({ id: subscription.id }) + .from(subscription) + .where(eq(subscription.stripeSubscriptionId, stripeSubscriptionId)) + .limit(1) + if (!row) return + + const liveSubscription = await requireStripeClient().subscriptions.retrieve(stripeSubscriptionId) + const changedInStripe = isCancellationChangedInStripe(event) + + await db.transaction(async (tx) => { + const [current] = await tx + .select({ cancelAtPeriodEnd: subscription.cancelAtPeriodEnd, seats: subscription.seats }) + .from(subscription) + .where(eq(subscription.id, row.id)) + .for('update') + .limit(1) + if (!current) return + + const cancelIntent = changedInStripe + ? undefined + : (await readInflightIntent(tx, OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, row.id)) + ?.cancelAtPeriodEnd + const seatsIntent = ( + await readInflightIntent(tx, OUTBOX_EVENT_TYPES.STRIPE_SYNC_SUBSCRIPTION_SEATS, row.id) + )?.seats + + const cancelAtPeriodEnd = + typeof cancelIntent === 'boolean' + ? cancelIntent + : Boolean(liveSubscription.cancel_at_period_end) + const seats = typeof seatsIntent === 'number' ? seatsIntent : current.seats + + if (Boolean(current.cancelAtPeriodEnd) === cancelAtPeriodEnd && current.seats === seats) { + return + } + + await tx + .update(subscription) + .set({ cancelAtPeriodEnd, seats }) + .where(eq(subscription.id, row.id)) + + logger.info('Reconciled Sim-owned subscription fields after a Stripe webhook', { + eventId: event.id, + subscriptionId: row.id, + stripeSubscriptionId, + cancelAtPeriodEnd: { + stored: current.cancelAtPeriodEnd, + reconciled: cancelAtPeriodEnd, + source: typeof cancelIntent === 'boolean' ? 'pending-sync' : 'stripe', + }, + seats: { stored: current.seats, reconciled: seats }, + }) + }) +} diff --git a/packages/testing/src/mocks/billing-subscription-sync.mock.ts b/packages/testing/src/mocks/billing-subscription-sync.mock.ts new file mode 100644 index 00000000000..8eb1b310365 --- /dev/null +++ b/packages/testing/src/mocks/billing-subscription-sync.mock.ts @@ -0,0 +1,43 @@ +import { vi } from 'vitest' + +/** + * Controllable mock functions for `@/lib/billing/webhooks/subscription-sync`. The enqueue + * functions resolve to a fixed event id; drive them with `mockResolvedValueOnce`. + * + * @example + * ```ts + * import { billingSubscriptionSyncMockFns } from '@sim/testing/mocks/billing-subscription-sync.mock' + * + * expect(billingSubscriptionSyncMockFns.mockEnqueueCancelAtPeriodEndSync).toHaveBeenCalledWith( + * expect.anything(), + * expect.objectContaining({ cancelAtPeriodEnd: true }) + * ) + * ``` + */ +export const billingSubscriptionSyncMockFns = { + mockEnqueueCancelAtPeriodEndSync: vi.fn(async () => 'cancel-at-period-end-sync-event'), + mockRecommitCancelAtPeriodEndSync: vi.fn(async () => undefined), + mockEnqueueSubscriptionSeatsSync: vi.fn(async () => 'subscription-seats-sync-event'), + mockCancelAtPeriodEndSyncIdempotencyKey: vi.fn( + (eventId: string) => `outbox-sync-cancel-at-period-end:${eventId}:key` + ), + mockReconcileSubscriptionSyncFromStripe: vi.fn(async () => undefined), +} + +/** + * Static mock module for `@/lib/billing/webhooks/subscription-sync`. + * + * @example + * ```ts + * vi.mock('@/lib/billing/webhooks/subscription-sync', () => billingSubscriptionSyncMock) + * ``` + */ +export const billingSubscriptionSyncMock = { + enqueueCancelAtPeriodEndSync: billingSubscriptionSyncMockFns.mockEnqueueCancelAtPeriodEndSync, + recommitCancelAtPeriodEndSync: billingSubscriptionSyncMockFns.mockRecommitCancelAtPeriodEndSync, + enqueueSubscriptionSeatsSync: billingSubscriptionSyncMockFns.mockEnqueueSubscriptionSeatsSync, + cancelAtPeriodEndSyncIdempotencyKey: + billingSubscriptionSyncMockFns.mockCancelAtPeriodEndSyncIdempotencyKey, + reconcileSubscriptionSyncFromStripe: + billingSubscriptionSyncMockFns.mockReconcileSubscriptionSyncFromStripe, +} diff --git a/packages/testing/src/mocks/index.ts b/packages/testing/src/mocks/index.ts index 4332a76b106..13ff671430b 100644 --- a/packages/testing/src/mocks/index.ts +++ b/packages/testing/src/mocks/index.ts @@ -127,6 +127,10 @@ export { billingSubscriptionMock, billingSubscriptionMockFns, } from './billing-subscription.mock' +export { + billingSubscriptionSyncMock, + billingSubscriptionSyncMockFns, +} from './billing-subscription-sync.mock' export { billingSubscriptionUtilsMock, billingSubscriptionUtilsMockFns, @@ -746,7 +750,12 @@ export { storageServiceMockFns, } from './storage-service.mock' export { + createInMemoryStripe, createMockStripeEvent, + type InMemoryStripe, + type InMemoryStripeCustomer, + type InMemoryStripeRequestGate, + type InMemoryStripeSubscription, stripeClientMock, stripePaymentMethodMock, } from './stripe.mock' diff --git a/packages/testing/src/mocks/stripe.mock.ts b/packages/testing/src/mocks/stripe.mock.ts index 5babfd73b6f..a6d05601d19 100644 --- a/packages/testing/src/mocks/stripe.mock.ts +++ b/packages/testing/src/mocks/stripe.mock.ts @@ -55,3 +55,282 @@ export function createMockStripeEvent( ...overrides, } as Stripe.Event } + +/** A Stripe subscription as the in-memory fake stores it: the fields billing code reads. */ +export interface InMemoryStripeSubscription { + id: string + object: 'subscription' + customer: string + status: Stripe.Subscription.Status + cancel_at_period_end: boolean + cancel_at: number | null + canceled_at: number | null + ended_at: number | null + trial_start: number | null + trial_end: number | null + schedule: string | null + metadata: Record + items: { + object: 'list' + data: Array<{ + id: string + quantity: number + current_period_start: number + current_period_end: number + price: { id: string; recurring: { interval: 'month' | 'year' } } + }> + } +} + +/** A Stripe customer as the in-memory fake stores it. */ +export interface InMemoryStripeCustomer { + id: string + object: 'customer' + email: string | null + name: string | null +} + +/** A request the fake parked on arrival, before Stripe processes it. */ +export interface InMemoryStripeRequestGate { + /** Resolves once the parked request has reached the fake. */ + reached: Promise + /** Lets the parked request proceed to Stripe's processing. */ + release(): void +} + +type UpdatableResource = 'subscriptions' | 'customers' + +interface SubscriptionUpdateParams { + cancel_at_period_end?: boolean + metadata?: Record + items?: Array<{ id: string; quantity?: number; price?: string }> +} + +interface CustomerUpdateParams { + email?: string + name?: string +} + +/** + * An in-memory Stripe account for integration tests that need Stripe to behave like Stripe: + * `retrieve` returns current state, `update` applies params and emits the + * `customer.subscription.updated` event Stripe would send (with `previous_attributes` and the + * originating `request.idempotency_key`), and a reused idempotency key replays the first + * response, or rejects when its parameters differ. A test can park the next update on a gate to + * control the order requests land in, or make the next update apply and then fail on the + * client, as a dropped connection does. + * + * @example + * ```ts + * const stripe = createInMemoryStripe() + * stripeClientMock.requireStripeClient.mockReturnValue(stripe.client) + * const gate = stripe.holdNextUpdate('subscriptions') + * ``` + */ +export function createInMemoryStripe() { + const subscriptions = new Map() + const customers = new Map() + const idempotentResults = new Map() + const gates = new Map< + UpdatableResource, + Array<{ reached: () => void; released: Promise }> + >() + const failuresAfterApply = new Map() + const events: Stripe.Event[] = [] + let sequence = 0 + + function nextId(prefix: string) { + sequence += 1 + return `${prefix}_${sequence}` + } + + function requireSubscription(id: string) { + const subscription = subscriptions.get(id) + if (!subscription) throw new Error(`No such subscription: '${id}'`) + return subscription + } + + function requireCustomer(id: string) { + const customer = customers.get(id) + if (!customer) throw new Error(`No such customer: '${id}'`) + return customer + } + + function applySubscriptionUpdate( + id: string, + params: SubscriptionUpdateParams, + idempotencyKey: string | null + ) { + const current = requireSubscription(id) + const next = structuredClone(current) + const previousAttributes: Record = {} + if ( + params.cancel_at_period_end !== undefined && + params.cancel_at_period_end !== current.cancel_at_period_end + ) { + previousAttributes.cancel_at_period_end = current.cancel_at_period_end + next.cancel_at_period_end = params.cancel_at_period_end + } + if (params.metadata) { + previousAttributes.metadata = current.metadata + next.metadata = { ...current.metadata, ...params.metadata } + } + for (const item of params.items ?? []) { + const target = next.items.data.find((existing) => existing.id === item.id) + if (!target) throw new Error(`No such subscription item: '${item.id}'`) + if (item.quantity !== undefined) target.quantity = item.quantity + if (item.price !== undefined) target.price = { ...target.price, id: item.price } + } + if (JSON.stringify(next.items) !== JSON.stringify(current.items)) { + previousAttributes.items = current.items + } + subscriptions.set(id, next) + if (Object.keys(previousAttributes).length > 0) { + events.push({ + id: nextId('evt'), + object: 'event', + api_version: '2025-08-27.basil', + created: sequence, + livemode: false, + pending_webhooks: 1, + request: { id: nextId('req'), idempotency_key: idempotencyKey }, + type: 'customer.subscription.updated', + data: { + object: structuredClone(next) as unknown as Stripe.Subscription, + previous_attributes: previousAttributes as Partial, + }, + } as Stripe.Event) + } + return structuredClone(next) + } + + async function update( + resource: UpdatableResource, + id: string, + params: unknown, + options: { idempotencyKey?: string } | undefined, + apply: () => T + ): Promise { + const gate = gates.get(resource)?.shift() + if (gate) { + gate.reached() + await gate.released + } + + const idempotencyKey = options?.idempotencyKey + const fingerprint = JSON.stringify([resource, id, params]) + if (idempotencyKey) { + const previous = idempotentResults.get(idempotencyKey) + if (previous && previous.fingerprint !== fingerprint) { + throw Object.assign( + new Error( + 'Keys for idempotent requests can only be used with the same parameters they were first used with.' + ), + { type: 'StripeIdempotencyError' } + ) + } + if (previous) return structuredClone(previous.result) as T + } + + const result = apply() + if (idempotencyKey) idempotentResults.set(idempotencyKey, { fingerprint, result }) + const failure = failuresAfterApply.get(resource)?.shift() + if (failure) throw failure + return structuredClone(result) + } + + const client = { + subscriptions: { + retrieve: async (id: string) => structuredClone(requireSubscription(id)), + update: ( + id: string, + params: SubscriptionUpdateParams, + options?: { idempotencyKey?: string } + ) => + update('subscriptions', id, params, options, () => + applySubscriptionUpdate(id, params, options?.idempotencyKey ?? null) + ), + }, + customers: { + retrieve: async (id: string) => structuredClone(requireCustomer(id)), + update: (id: string, params: CustomerUpdateParams, options?: { idempotencyKey?: string }) => + update('customers', id, params, options, () => { + const next = { ...requireCustomer(id), ...params } + customers.set(id, next) + return structuredClone(next) + }), + }, + webhooks: { + constructEventAsync: async (payload: string) => JSON.parse(payload) as Stripe.Event, + }, + } + + return { + /** Pass to `stripeClientMock.requireStripeClient` or to the Better Auth Stripe plugin. */ + client: client as unknown as Stripe, + /** Every `customer.subscription.updated` event Stripe emitted, in the order it applied them. */ + events, + addSubscription( + subscription: Pick & + Partial> & { + quantity?: number + } + ) { + const now = Math.floor(Date.now() / 1000) + subscriptions.set(subscription.id, { + id: subscription.id, + object: 'subscription', + customer: subscription.customer, + status: subscription.status ?? 'active', + cancel_at_period_end: subscription.cancel_at_period_end ?? false, + cancel_at: null, + canceled_at: null, + ended_at: null, + trial_start: null, + trial_end: null, + schedule: null, + metadata: {}, + items: { + object: 'list', + data: [ + { + id: `si_${subscription.id}`, + quantity: subscription.quantity ?? 1, + current_period_start: now, + current_period_end: now + 30 * 24 * 60 * 60, + price: { id: `price_${subscription.id}`, recurring: { interval: 'month' } }, + }, + ], + }, + }) + }, + addCustomer(customer: Pick) { + customers.set(customer.id, { ...customer, object: 'customer' }) + }, + subscription: (id: string) => structuredClone(requireSubscription(id)), + customer: (id: string) => structuredClone(requireCustomer(id)), + /** Applies a change made outside Sim (dashboard, customer portal) and emits its event. */ + updateOutsideSim(id: string, params: SubscriptionUpdateParams) { + return applySubscriptionUpdate(id, params, null) + }, + /** Parks the next update to `resource` until the returned gate is released. */ + holdNextUpdate(resource: UpdatableResource): InMemoryStripeRequestGate { + let reached: () => void = () => {} + const arrival = new Promise((resolve) => { + reached = resolve + }) + let release: () => void = () => {} + const released = new Promise((resolve) => { + release = resolve + }) + gates.set(resource, [...(gates.get(resource) ?? []), { reached, released }]) + return { reached: arrival, release } + }, + /** Makes the next update to `resource` apply in Stripe, then fail on the client. */ + failNextUpdateAfterApplying(resource: UpdatableResource, error = new Error('socket hang up')) { + failuresAfterApply.set(resource, [...(failuresAfterApply.get(resource) ?? []), error]) + }, + } +} + +export type InMemoryStripe = ReturnType From db9069f74de342c380e971562f4a46eaf7b66db0 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 7 Oct 2026 13:21:22 -0700 Subject: [PATCH 02/14] fix(billing): record each committed sync value on every in-flight event so no stale intent can resurface --- .../api/v1/admin/outbox/[id]/requeue/route.ts | 54 ++- .../lib/admin/subscription-lifecycle.test.ts | 6 +- apps/sim/lib/admin/subscription-lifecycle.ts | 10 +- .../sim/lib/billing/webhooks/outbox-events.ts | 15 +- .../lib/billing/webhooks/outbox-handlers.ts | 22 +- .../stripe-sync-convergence.integration.ts | 200 +++++++++- .../lib/billing/webhooks/subscription-sync.ts | 345 ++++++++++++------ apps/sim/lib/core/outbox/service.ts | 68 +++- .../mocks/billing-subscription-sync.mock.ts | 11 +- packages/testing/src/mocks/index.ts | 3 - .../testing/src/mocks/outbox-service.mock.ts | 4 + packages/testing/src/mocks/stripe.mock.ts | 22 +- 12 files changed, 564 insertions(+), 196 deletions(-) diff --git a/apps/sim/app/api/v1/admin/outbox/[id]/requeue/route.ts b/apps/sim/app/api/v1/admin/outbox/[id]/requeue/route.ts index 944c0b94036..2e0c74ca6d8 100644 --- a/apps/sim/app/api/v1/admin/outbox/[id]/requeue/route.ts +++ b/apps/sim/app/api/v1/admin/outbox/[id]/requeue/route.ts @@ -2,6 +2,7 @@ import { db } from '@sim/db' import { outboxEvent } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { toError } from '@sim/utils/errors' +import { toRecord } from '@sim/utils/object' import { and, eq, sql } from 'drizzle-orm' import { NextResponse } from 'next/server' import { adminV1RequeueOutboxEventContract } from '@/lib/api/contracts/v1/admin' @@ -11,6 +12,10 @@ import { ENTERPRISE_METADATA_SYNC_EVENT_TYPE, ENTERPRISE_PROVISION_EVENT_TYPE, } from '@/lib/billing/enterprise-outbox-events' +import { + isSubscriptionSyncEventType, + recommitSubscriptionSync, +} from '@/lib/billing/webhooks/subscription-sync' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { withAdminAuthParams } from '@/app/api/v1/admin/middleware' @@ -28,7 +33,9 @@ const invalidOutboxEventResponse = (message: string) => * will retry it. Resets `attempts`, `lastError`, and `availableAt` so * the next poll picks it up. Only dead-lettered events can be * requeued — completed/pending/processing rows are rejected to avoid - * operator errors. + * operator errors. A Stripe subscription sync is re-committed with the + * subscription's current value, so the retry never revives the value it + * failed with. */ export const POST = withRouteHandler( withAdminAuthParams<{ id: string }>(async (request, context) => { @@ -61,23 +68,34 @@ export const POST = withRouteHandler( const deliveryRevision = metadataIntent?.success ? metadataIntent.data.deliveryRevision + 1 : null - const result = await db - .update(outboxEvent) - .set({ - status: 'pending', - attempts: 0, - lastError: null, - availableAt: new Date(), - lockedAt: null, - processedAt: null, - ...(deliveryRevision === null - ? {} - : { - payload: sql`(${outboxEvent.payload}::jsonb || ${JSON.stringify({ deliveryRevision })}::jsonb)::json`, - }), - }) - .where(and(eq(outboxEvent.id, id), eq(outboxEvent.status, 'dead_letter'))) - .returning({ id: outboxEvent.id, eventType: outboxEvent.eventType }) + const result = await db.transaction(async (tx) => { + const requeued = await tx + .update(outboxEvent) + .set({ + status: 'pending', + attempts: 0, + lastError: null, + availableAt: new Date(), + lockedAt: null, + processedAt: null, + ...(deliveryRevision === null + ? {} + : { + payload: sql`(${outboxEvent.payload}::jsonb || ${JSON.stringify({ deliveryRevision })}::jsonb)::json`, + }), + }) + .where(and(eq(outboxEvent.id, id), eq(outboxEvent.status, 'dead_letter'))) + .returning({ id: outboxEvent.id, eventType: outboxEvent.eventType }) + const subscriptionId = toRecord(existing?.payload).subscriptionId + if ( + requeued.length > 0 && + isSubscriptionSyncEventType(requeued[0].eventType) && + typeof subscriptionId === 'string' + ) { + await recommitSubscriptionSync(tx, requeued[0].eventType, subscriptionId) + } + return requeued + }) if (result.length === 0) { return NextResponse.json( diff --git a/apps/sim/lib/admin/subscription-lifecycle.test.ts b/apps/sim/lib/admin/subscription-lifecycle.test.ts index 23ab4c42cab..6f9a3d2e4c1 100644 --- a/apps/sim/lib/admin/subscription-lifecycle.test.ts +++ b/apps/sim/lib/admin/subscription-lifecycle.test.ts @@ -124,10 +124,10 @@ describe('admin subscription cancellation', () => { expect.objectContaining({ status: 'pending', attempts: 0, lastError: null }) ) expect(dbChainMockFns.set).toHaveBeenCalledWith({ cancelAtPeriodEnd: true }) - expect(billingSubscriptionSyncMockFns.mockRecommitCancelAtPeriodEndSync).toHaveBeenCalledWith( + expect(billingSubscriptionSyncMockFns.mockRecommitSubscriptionSync).toHaveBeenCalledWith( expect.anything(), - 'outbox-1', - true + 'stripe.sync-cancel-at-period-end', + 'sub-row-1' ) expect(result).toMatchObject({ operationId: '67e55044-10b1-426f-9247-bb680e5fe0c8', diff --git a/apps/sim/lib/admin/subscription-lifecycle.ts b/apps/sim/lib/admin/subscription-lifecycle.ts index 44177c17ca9..abacaa25fb0 100644 --- a/apps/sim/lib/admin/subscription-lifecycle.ts +++ b/apps/sim/lib/admin/subscription-lifecycle.ts @@ -10,7 +10,7 @@ import { ENTITLED_SUBSCRIPTION_STATUSES } from '@/lib/billing/subscriptions/util import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' import { enqueueCancelAtPeriodEndSync, - recommitCancelAtPeriodEndSync, + recommitSubscriptionSync, } from '@/lib/billing/webhooks/subscription-sync' import { enqueueOutboxEvent } from '@/lib/core/outbox/service' @@ -305,7 +305,6 @@ export async function requestDashboardSubscriptionCancellation({ if (!restoredSubscription) { throw new Error('Cancellation subscription no longer exists') } - await recommitCancelAtPeriodEndSync(tx, existingOperation.id, true) } await tx .update(outboxEvent) @@ -320,6 +319,13 @@ export async function requestDashboardSubscriptionCancellation({ .where( and(eq(outboxEvent.id, existingOperation.id), eq(outboxEvent.status, 'dead_letter')) ) + if (existingOperation.eventType === OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END) { + await recommitSubscriptionSync( + tx, + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + existingOperation.subscriptionId + ) + } return { operationId, outboxEventId: existingOperation.id, diff --git a/apps/sim/lib/billing/webhooks/outbox-events.ts b/apps/sim/lib/billing/webhooks/outbox-events.ts index 0cc6bf3946a..7b027d32c4e 100644 --- a/apps/sim/lib/billing/webhooks/outbox-events.ts +++ b/apps/sim/lib/billing/webhooks/outbox-events.ts @@ -1,18 +1,5 @@ export const OUTBOX_EVENT_TYPES = { - /** - * Sync a subscription's `cancel_at_period_end` flag from our DB to - * Stripe. Enqueue through `enqueueCancelAtPeriodEndSync` in the same - * transaction as every DB change to `cancelAtPeriodEnd`. - * - * Guarantee: once every in-flight event for a subscription completes, - * Stripe holds the last value committed to the DB. Each handler pushes - * the row's current value (not its payload's) and re-reads the row - * after its Stripe write, retrying while the value moved, so racing - * events converge even when an earlier request lands in Stripe last. - * While an event is in flight, `reconcileSubscriptionSyncFromStripe` - * keeps webhook echoes and stale snapshots from overwriting the - * committed value; a change made in Stripe itself wins over it. - */ + /** Sync `cancelAtPeriodEnd` from our DB to Stripe; enqueue via `enqueueCancelAtPeriodEndSync`. */ STRIPE_SYNC_CANCEL_AT_PERIOD_END: 'stripe.sync-cancel-at-period-end', /** Cancel in Stripe; the verified deletion webhook remains the only DB entitlement authority. */ STRIPE_CANCEL_SUBSCRIPTION_IMMEDIATELY: 'stripe.cancel-subscription-immediately', diff --git a/apps/sim/lib/billing/webhooks/outbox-handlers.ts b/apps/sim/lib/billing/webhooks/outbox-handlers.ts index 50e06bd6b40..de6ad594620 100644 --- a/apps/sim/lib/billing/webhooks/outbox-handlers.ts +++ b/apps/sim/lib/billing/webhooks/outbox-handlers.ts @@ -20,6 +20,17 @@ import type { OutboxHandler } from '@/lib/core/outbox/service' const logger = createLogger('BillingOutboxHandlers') +/** + * Passes a DB→Stripe sync handler makes before throwing for an outbox retry: each pass re-reads + * the row after its Stripe write and goes again when the value moved in the meantime. + */ +const MAX_SYNC_ATTEMPTS = 2 + +/** A fresh key per Stripe write: the SDK reuses it across its own network retries of that call. */ +function customerContactSyncIdempotencyKey(eventId: string): string { + return `outbox:${eventId}:${generateShortId()}` +} + interface StripeCancelSubscriptionImmediatelyPayload { stripeSubscriptionId: string subscriptionId: string @@ -110,9 +121,8 @@ const stripeSyncCancelAtPeriodEnd: OutboxHandler = ) => { await recordAdminCancellationAudit({ ...payload, timing: 'period_end' }) const stripe = requireStripeClient() - const maxSyncAttempts = 2 - for (let attempt = 1; attempt <= maxSyncAttempts; attempt++) { + for (let attempt = 1; attempt <= MAX_SYNC_ATTEMPTS; attempt++) { const desiredValue = await readCancelAtPeriodEnd(payload.subscriptionId) if (desiredValue === null) { logger.warn('Subscription not found when syncing cancel_at_period_end', { @@ -184,9 +194,8 @@ const stripeSyncSubscriptionSeats: OutboxHandler = ctx ) => { const stripe = requireStripeClient() - const maxSyncAttempts = 2 - for (let attempt = 1; attempt <= maxSyncAttempts; attempt++) { + for (let attempt = 1; attempt <= MAX_SYNC_ATTEMPTS; attempt++) { const row = await getSubscriptionSeatSyncState(payload.subscriptionId) if (!row) { logger.warn('Subscription not found when syncing seats', { @@ -450,9 +459,8 @@ const stripeSyncCustomerContact: OutboxHandler ctx ) => { const stripe = requireStripeClient() - const maxSyncAttempts = 2 - for (let attempt = 1; attempt <= maxSyncAttempts; attempt++) { + for (let attempt = 1; attempt <= MAX_SYNC_ATTEMPTS; attempt++) { const contact = await readCustomerContact(payload.subscriptionId) if (contact.status === 'skipped') { logger.warn(contact.reason, { @@ -476,7 +484,7 @@ const stripeSyncCustomerContact: OutboxHandler email: contact.email, ...(contact.name ? { name: contact.name } : {}), }, - { idempotencyKey: `outbox:${ctx.eventId}:${generateShortId()}` } + { idempotencyKey: customerContactSyncIdempotencyKey(ctx.eventId) } ) } diff --git a/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts b/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts index ede3b58165d..1cb9ed3e5fa 100644 --- a/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts +++ b/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts @@ -17,12 +17,19 @@ import { } from '@sim/testing/mocks/stripe.mock' import { generateId } from '@sim/utils/id' import { type BetterAuthOptions, betterAuth } from 'better-auth' -import { and, asc, eq, sql } from 'drizzle-orm' +import { and, desc, eq, sql } from 'drizzle-orm' import { drizzle, type PostgresJsDatabase } from 'drizzle-orm/postgres-js' +import { NextRequest } from 'next/server' import postgres from 'postgres' import type Stripe from 'stripe' import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' +const ADMIN_API_KEY = vi.hoisted(() => { + const key = 'integration-fixture-admin-key' + process.env.ADMIN_API_KEY = key + return key +}) + const database = vi.hoisted(() => ({ current: undefined as PostgresJsDatabase | undefined, })) @@ -48,6 +55,7 @@ import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' import { billingOutboxHandlers } from '@/lib/billing/webhooks/outbox-handlers' import { reconcileSubscriptionSyncFromStripe } from '@/lib/billing/webhooks/subscription-sync' import { enqueueOutboxEvent, processOutboxEventById } from '@/lib/core/outbox/service' +import { POST as requeueOutboxEvent } from '@/app/api/v1/admin/outbox/[id]/requeue/route' const schemaName = `stripe_sync_${generateId().replaceAll('-', '')}` const connection = postgres( @@ -202,8 +210,8 @@ async function leaveOrganization(userId: string, organizationId: string) { .where(and(eq(member.userId, userId), eq(member.organizationId, organizationId))) } -async function outboxEventIds(eventType: string, subscriptionId: string) { - const rows = await testDatabase +async function latestOutboxEventId(eventType: string, subscriptionId: string) { + const [latest] = await testDatabase .select({ id: outboxEvent.id }) .from(outboxEvent) .where( @@ -212,15 +220,10 @@ async function outboxEventIds(eventType: string, subscriptionId: string) { sql`${outboxEvent.payload} ->> 'subscriptionId' = ${subscriptionId}` ) ) - .orderBy(asc(outboxEvent.createdAt), asc(outboxEvent.id)) - return rows.map((row) => row.id) -} - -async function latestOutboxEventId(eventType: string, subscriptionId: string) { - const ids = await outboxEventIds(eventType, subscriptionId) - const id = ids.at(-1) - if (!id) throw new Error(`No ${eventType} event for ${subscriptionId}`) - return id + .orderBy(desc(outboxEvent.createdAt), desc(outboxEvent.id)) + .limit(1) + if (!latest) throw new Error(`No ${eventType} event for ${subscriptionId}`) + return latest.id } function processEvent(eventId: string) { @@ -234,6 +237,30 @@ async function makeDue(eventId: string) { .where(eq(outboxEvent.id, eventId)) } +/** Runs the event's last attempt with Stripe unavailable, so it dead-letters without applying. */ +async function deadLetter(eventId: string) { + await testDatabase.update(outboxEvent).set({ maxAttempts: 1 }).where(eq(outboxEvent.id, eventId)) + stripe.failNextRequest('subscriptions.update') + await expect(processEvent(eventId)).resolves.toBe('dead_letter') +} + +async function requeueFromAdminApi(eventId: string) { + const response = await requeueOutboxEvent( + new NextRequest(`http://localhost:3000/api/v1/admin/outbox/${eventId}/requeue`, { + method: 'POST', + headers: { 'x-admin-key': ADMIN_API_KEY }, + }), + { params: Promise.resolve({ id: eventId }) } + ) + expect(response.status).toBe(200) +} + +/** Delivers a subscription update Stripe makes on its own, e.g. a renewal. */ +async function deliverUnrelatedUpdate(stripeSubscriptionId: string) { + stripe.updateOutsideSim(stripeSubscriptionId, { metadata: { renewedAt: generateId() } }) + await deliver(stripe.events.at(-1) as Stripe.Event) +} + async function storedSubscription(subscriptionId: string) { const [row] = await testDatabase .select({ cancelAtPeriodEnd: subscription.cancelAtPeriodEnd, seats: subscription.seats }) @@ -372,6 +399,126 @@ describe('cancel_at_period_end sync', () => { await expect(processEvent(pauseSync)).resolves.toBe('completed') expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(false) }) + it('keeps a renewal made in Stripe after later updates while an earlier sync retries', async () => { + const pro = await createProUserInPaidOrganization() + await pauseProSubscriptionForOrgCoverage(pro.userId) + const pauseSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + stripe.failNextUpdateAfterApplying('subscriptions') + await expect(processEvent(pauseSync)).resolves.toBe('pending') + await deliver(stripe.events.at(-1) as Stripe.Event) + + stripe.updateOutsideSim(pro.stripeSubscriptionId, { cancel_at_period_end: false }) + await deliver(stripe.events.at(-1) as Stripe.Event) + await deliverUnrelatedUpdate(pro.stripeSubscriptionId) + + await makeDue(pauseSync) + await expect(processEvent(pauseSync)).resolves.toBe('completed') + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(false) + expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(false) + }) + + it('keeps a cancel-then-renew made in Stripe after a later update while a sync is pending', async () => { + const pro = await createProUserInPaidOrganization() + await pauseProSubscriptionForOrgCoverage(pro.userId) + const pauseSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + + stripe.updateOutsideSim(pro.stripeSubscriptionId, { cancel_at_period_end: true }) + await deliver(stripe.events.at(-1) as Stripe.Event) + stripe.updateOutsideSim(pro.stripeSubscriptionId, { cancel_at_period_end: false }) + await deliver(stripe.events.at(-1) as Stripe.Event) + await deliverUnrelatedUpdate(pro.stripeSubscriptionId) + + await expect(processEvent(pauseSync)).resolves.toBe('completed') + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(false) + expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(false) + }) + + it('does not restore an older value while its slow sync is still running', async () => { + const pro = await createProUserInPaidOrganization() + await pauseProSubscriptionForOrgCoverage(pro.userId) + const pauseSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + const slowPush = stripe.holdNextUpdate('subscriptions') + const pausing = processEvent(pauseSync) + await slowPush.reached + + await leaveOrganization(pro.userId, pro.paidOrganization.organizationId) + await restoreUserProSubscription(pro.userId) + const restoreSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + await expect(processEvent(restoreSync)).resolves.toBe('completed') + await deliverUnrelatedUpdate(pro.stripeSubscriptionId) + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(false) + + slowPush.release() + await expect(pausing).resolves.toBe('completed') + expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(false) + }) + + it('does not revive an older value when its dead-lettered sync is requeued', async () => { + const pro = await createProUserInPaidOrganization() + await pauseProSubscriptionForOrgCoverage(pro.userId) + const pauseSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + await deadLetter(pauseSync) + + await leaveOrganization(pro.userId, pro.paidOrganization.organizationId) + await restoreUserProSubscription(pro.userId) + const restoreSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + await expect(processEvent(restoreSync)).resolves.toBe('completed') + + await requeueFromAdminApi(pauseSync) + await deliverUnrelatedUpdate(pro.stripeSubscriptionId) + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(false) + await expect(processEvent(pauseSync)).resolves.toBe('completed') + expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(false) + }) + + it('leaves the field alone while a sync enqueued by an older deploy is in flight', async () => { + const pro = await createProUserInPaidOrganization() + await pauseProSubscriptionForOrgCoverage(pro.userId) + await leaveOrganization(pro.userId, pro.paidOrganization.organizationId) + await testDatabase.transaction(async (tx) => { + await tx + .update(subscription) + .set({ cancelAtPeriodEnd: false }) + .where(eq(subscription.id, pro.subscriptionId)) + await enqueueOutboxEvent(tx, OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, { + stripeSubscriptionId: pro.stripeSubscriptionId, + subscriptionId: pro.subscriptionId, + reason: 'member-left-paid-org', + }) + }) + + await deliverUnrelatedUpdate(pro.stripeSubscriptionId) + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(false) + }) + + it('restores a pending value without reading Stripe', async () => { + const pro = await createProUserInPaidOrganization() + await pauseProSubscriptionForOrgCoverage(pro.userId) + + stripe.updateOutsideSim(pro.stripeSubscriptionId, { metadata: { renewedAt: 'period-2' } }) + stripe.failNextRequest('subscriptions.retrieve') + await deliver(stripe.events.at(-1) as Stripe.Event) + + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(true) + }) }) describe('customer contact sync', () => { @@ -441,4 +588,33 @@ describe('Team seat sync', () => { await expect(processEvent(seatSync)).resolves.toBe('completed') expect(stripe.subscription(org.stripeSubscriptionId).items.data[0].quantity).toBe(2) }) + it('does not revive an older seat count when its dead-lettered sync is requeued', async () => { + const [owner, joiner] = await Promise.all([createUser('owner'), createUser('joiner')]) + const org = await createOrganizationWithPlan('team', 1) + await addMember(org.organizationId, owner.id, 'owner') + await addMember(org.organizationId, joiner.id) + await reconcileOrganizationSeats({ organizationId: org.organizationId, reason: 'member-added' }) + const growSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_SUBSCRIPTION_SEATS, + org.subscriptionId + ) + await deadLetter(growSync) + + await leaveOrganization(joiner.id, org.organizationId) + await reconcileOrganizationSeats({ + organizationId: org.organizationId, + reason: 'member-removed', + }) + const shrinkSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_SUBSCRIPTION_SEATS, + org.subscriptionId + ) + await expect(processEvent(shrinkSync)).resolves.toBe('completed') + + await requeueFromAdminApi(growSync) + await deliverUnrelatedUpdate(org.stripeSubscriptionId) + expect((await storedSubscription(org.subscriptionId)).seats).toBe(1) + await expect(processEvent(growSync)).resolves.toBe('completed') + expect(stripe.subscription(org.stripeSubscriptionId).items.data[0].quantity).toBe(1) + }) }) diff --git a/apps/sim/lib/billing/webhooks/subscription-sync.ts b/apps/sim/lib/billing/webhooks/subscription-sync.ts index 6c545aafa7b..85d02c333e1 100644 --- a/apps/sim/lib/billing/webhooks/subscription-sync.ts +++ b/apps/sim/lib/billing/webhooks/subscription-sync.ts @@ -1,13 +1,17 @@ import { db } from '@sim/db' -import { outboxEvent, subscription } from '@sim/db/schema' +import { subscription } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { generateShortId } from '@sim/utils/id' import { toRecord } from '@sim/utils/object' -import { and, eq, inArray, isNotNull, sql } from 'drizzle-orm' +import { eq, sql } from 'drizzle-orm' import type Stripe from 'stripe' import { requireStripeClient } from '@/lib/billing/stripe-client' import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' -import { enqueueOutboxEvent, patchOutboxEventPayload } from '@/lib/core/outbox/service' +import { + enqueueOutboxEvent, + listInflightOutboxEvents, + patchInflightOutboxEvents, +} from '@/lib/core/outbox/service' import type { DbOrTx } from '@/lib/db/types' const logger = createLogger('BillingSubscriptionSync') @@ -19,23 +23,17 @@ const logger = createLogger('BillingSubscriptionSync') */ const CANCEL_AT_PERIOD_END_SYNC_KEY_PREFIX = 'outbox-sync-cancel-at-period-end:' -/** - * The value Sim committed for a synced field, recorded on the sync event. `requestedAt` is the - * database clock read while the enqueuing transaction held the subscription row lock, so among - * a subscription's in-flight events the latest `requestedAt` is the latest committed value. - * Transaction start time (`created_at`) cannot order them: a transaction that began earlier - * can take the row lock later. - */ -interface SyncIntent { - requestedAt: string -} +const CANCEL_SYNC = OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END +const SEATS_SYNC = OUTBOX_EVENT_TYPES.STRIPE_SYNC_SUBSCRIPTION_SEATS -export interface CancelAtPeriodEndSyncPayload extends Partial { +export interface CancelAtPeriodEndSyncPayload { stripeSubscriptionId: string /** The DB subscription row id; the handler pushes this row's current value. */ subscriptionId: string - /** The value committed with this event. Absent on events enqueued before it was recorded. */ + /** The latest committed value. Absent on events enqueued before it was recorded. */ cancelAtPeriodEnd?: boolean + /** When `cancelAtPeriodEnd` was committed; see `withCommittedAt`. */ + committedAt?: number /** Reason this was enqueued, e.g. 'joined-paid-org'. */ reason?: string /** Correlates Enterprise-issuance follow-up work for Admin progress/retry. */ @@ -45,53 +43,79 @@ export interface CancelAtPeriodEndSyncPayload extends Partial { requestedBy?: { id: string | null; name: string; email: string | null } } -export interface SubscriptionSeatsSyncPayload extends Partial { +export interface SubscriptionSeatsSyncPayload { /** The DB subscription row id; the handler pushes this row's current plan and seats. */ subscriptionId: string - /** The seat count committed with this event. Absent on events enqueued before it was recorded. */ + /** The latest committed seat count. Absent on events enqueued before it was recorded. */ seats?: number + /** When `seats` was committed; see `withCommittedAt`. */ + committedAt?: number reason?: string } -async function readDatabaseClock(executor: DbOrTx): Promise { - const [row] = await executor.execute<{ requestedAt: string }>( - sql`select to_json(clock_timestamp()) #>> '{}' as "requestedAt"` +type SubscriptionSyncEventType = typeof CANCEL_SYNC | typeof SEATS_SYNC +type SyncIntentFields = { cancelAtPeriodEnd: boolean } | { seats: number } + +export function isSubscriptionSyncEventType( + eventType: string +): eventType is SubscriptionSyncEventType { + return eventType === CANCEL_SYNC || eventType === SEATS_SYNC +} + +function subscriptionSubject(subscriptionId: string) { + return { payloadKey: 'subscriptionId', payloadValue: subscriptionId } +} + +/** + * Stamps `fields` with the database clock in microseconds since the epoch. Read while the + * caller holds the subscription row lock, so a later stamp is a later committed value. The + * transaction start time (`created_at`) cannot order them: a transaction that began earlier + * can take the row lock later. + */ +async function withCommittedAt( + tx: DbOrTx, + fields: T +): Promise { + const [row] = await tx.execute<{ committedAt: string }>( + sql`select (extract(epoch from clock_timestamp()) * 1000000)::bigint::text as "committedAt"` ) if (!row) throw new Error('Database clock read returned no row') - return row.requestedAt + return { ...fields, committedAt: Number(row.committedAt) } } /** - * Enqueue the Stripe sync for a `cancelAtPeriodEnd` value written in this transaction. The - * caller must hold the subscription row lock (`FOR UPDATE`, or the `UPDATE` itself). + * Records `fields` as the subscription's latest committed value for `eventType` and writes it + * onto every in-flight event of that type, so no pending, retrying, or reaped event still + * carries an older value for the webhook reconcile to restore. The caller must hold the + * subscription row lock (`FOR UPDATE`, or the `UPDATE` itself). */ -export async function enqueueCancelAtPeriodEndSync( +async function commitIntent( tx: DbOrTx, - payload: Omit & { - cancelAtPeriodEnd: boolean - } -): Promise { - const intent: CancelAtPeriodEndSyncPayload = { - ...payload, - requestedAt: await readDatabaseClock(tx), - } - return enqueueOutboxEvent(tx, OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, intent) + eventType: SubscriptionSyncEventType, + subscriptionId: string, + fields: T +): Promise { + const committed = await withCommittedAt(tx, fields) + await patchInflightOutboxEvents(tx, eventType, subscriptionSubject(subscriptionId), committed) + return committed } /** - * Re-records the committed value on an existing cancel-sync event that is being retried, so it - * orders after every event enqueued since. The caller must hold the subscription row lock. + * Enqueue the Stripe sync for a `cancelAtPeriodEnd` value written in this transaction. The + * caller must hold the subscription row lock. Once every in-flight event for the subscription + * completes, Stripe holds the last committed value. */ -export async function recommitCancelAtPeriodEndSync( +export async function enqueueCancelAtPeriodEndSync( tx: DbOrTx, - eventId: string, - cancelAtPeriodEnd: boolean -): Promise { - const patch: Pick = { - cancelAtPeriodEnd, - requestedAt: await readDatabaseClock(tx), + payload: Omit & { + cancelAtPeriodEnd: boolean } - await patchOutboxEventPayload(tx, eventId, patch) +): Promise { + const committed = await commitIntent(tx, CANCEL_SYNC, payload.subscriptionId, { + cancelAtPeriodEnd: payload.cancelAtPeriodEnd, + }) + const intent: CancelAtPeriodEndSyncPayload = { ...payload, ...committed } + return enqueueOutboxEvent(tx, CANCEL_SYNC, intent) } /** @@ -100,13 +124,41 @@ export async function recommitCancelAtPeriodEndSync( */ export async function enqueueSubscriptionSeatsSync( tx: DbOrTx, - payload: Omit & { seats: number } + payload: Omit & { seats: number } ): Promise { - const intent: SubscriptionSeatsSyncPayload = { - ...payload, - requestedAt: await readDatabaseClock(tx), - } - return enqueueOutboxEvent(tx, OUTBOX_EVENT_TYPES.STRIPE_SYNC_SUBSCRIPTION_SEATS, intent) + const committed = await commitIntent(tx, SEATS_SYNC, payload.subscriptionId, { + seats: payload.seats, + }) + const intent: SubscriptionSeatsSyncPayload = { ...payload, ...committed } + return enqueueOutboxEvent(tx, SEATS_SYNC, intent) +} + +/** + * Re-commits the subscription's current DB value onto its in-flight sync events, for a + * dead-lettered event that was just reset to `pending`: the retry then carries the latest value + * rather than the one it failed with. Takes the subscription row lock itself. + */ +export async function recommitSubscriptionSync( + tx: DbOrTx, + eventType: SubscriptionSyncEventType, + subscriptionId: string +): Promise { + const [current] = await tx + .select({ cancelAtPeriodEnd: subscription.cancelAtPeriodEnd, seats: subscription.seats }) + .from(subscription) + .where(eq(subscription.id, subscriptionId)) + .for('update') + .limit(1) + if (!current) return + + await commitIntent( + tx, + eventType, + subscriptionId, + eventType === CANCEL_SYNC + ? { cancelAtPeriodEnd: Boolean(current.cancelAtPeriodEnd) } + : { seats: current.seats ?? 1 } + ) } /** A fresh key per Stripe write: the SDK reuses it across its own network retries of that call. */ @@ -114,26 +166,45 @@ export function cancelAtPeriodEndSyncIdempotencyKey(eventId: string): string { return `${CANCEL_AT_PERIOD_END_SYNC_KEY_PREFIX}${eventId}:${generateShortId()}` } -/** The value of the most recently committed in-flight sync event, or undefined. */ -async function readInflightIntent( - tx: DbOrTx, - eventType: string, - subscriptionId: string -): Promise | undefined> { - const [latest] = await tx - .select({ payload: outboxEvent.payload }) - .from(outboxEvent) - .where( - and( - eq(outboxEvent.eventType, eventType), - inArray(outboxEvent.status, ['pending', 'processing']), - sql`${outboxEvent.payload} ->> 'subscriptionId' = ${subscriptionId}`, - isNotNull(sql`${outboxEvent.payload} ->> 'requestedAt'`) - ) - ) - .orderBy(sql`(${outboxEvent.payload} ->> 'requestedAt')::timestamptz desc`) - .limit(1) - return latest ? toRecord(latest.payload) : undefined +/** + * What a sync type's in-flight events say about its field: nothing in flight, the latest + * committed value, or `legacy` when an event predates recorded values (enqueued by an older + * deploy) so the committed value is unknown. + */ +type InflightIntent = { status: 'none' } | { status: 'legacy' } | { status: 'value'; value: T } + +function latestIntent( + events: { eventType: string; payload: unknown }[], + eventType: SubscriptionSyncEventType, + readValue: (payload: Record) => T | undefined +): InflightIntent { + let latest: { committedAt: number; value: T } | undefined + for (const event of events) { + if (event.eventType !== eventType) continue + const payload = toRecord(event.payload) + const value = readValue(payload) + if (typeof payload.committedAt !== 'number' || value === undefined) return { status: 'legacy' } + if (!latest || payload.committedAt > latest.committedAt) { + latest = { committedAt: payload.committedAt, value } + } + } + return latest ? { status: 'value', value: latest.value } : { status: 'none' } +} + +async function readInflightIntents(executor: DbOrTx, subscriptionId: string) { + const events = await listInflightOutboxEvents( + executor, + [CANCEL_SYNC, SEATS_SYNC], + subscriptionSubject(subscriptionId) + ) + return { + cancelAtPeriodEnd: latestIntent(events, CANCEL_SYNC, (payload) => + typeof payload.cancelAtPeriodEnd === 'boolean' ? payload.cancelAtPeriodEnd : undefined + ), + seats: latestIntent(events, SEATS_SYNC, (payload) => + typeof payload.seats === 'number' ? payload.seats : undefined + ), + } } /** True when the event records a `cancel_at_period_end` change made in Stripe, not by Sim's sync. */ @@ -144,6 +215,22 @@ function isCancellationChangedInStripe(event: Stripe.Event): boolean { return !idempotencyKey?.startsWith(CANCEL_AT_PERIOD_END_SYNC_KEY_PREFIX) } +type CancelAtPeriodEndSource = + | { source: 'unchanged' } + | { source: 'pending-sync'; value: boolean } + | { source: 'stripe' } + +function cancelAtPeriodEndSource( + intent: InflightIntent, + changedInStripe: boolean +): CancelAtPeriodEndSource { + if (intent.status === 'legacy') return { source: 'unchanged' } + if (intent.status === 'value' && !changedInStripe) { + return { source: 'pending-sync', value: intent.value } + } + return { source: 'stripe' } +} + /** * Reconciles the Sim-owned subscription fields after the Better Auth Stripe plugin has copied a * `customer.subscription.updated` payload into the row. The plugin writes `cancelAtPeriodEnd` @@ -151,17 +238,17 @@ function isCancellationChangedInStripe(event: Stripe.Event): boolean { * overwrite a value Sim committed but has not yet pushed, and the pending sync would then push * the overwritten value back to Stripe. * - * Under the subscription row lock (the lock every enqueuing writer holds): - * - `cancelAtPeriodEnd`: while a cancel sync is in flight, Sim's latest committed value wins over - * snapshots and over echoes of Sim's own earlier writes. A change made in Stripe itself (customer - * portal, dashboard, Better Auth's cancel/restore endpoints) is newer than anything pending and - * wins, as does Stripe whenever no sync is in flight. Stripe's value is read live, never taken - * from the event, so out-of-order delivery cannot regress it. - * - `seats`: Team seats follow the member count Sim maintains; while a seat sync is in flight its - * latest committed value wins. With none in flight the plugin's write stands, as before. + * Decided under the subscription row lock that every committing writer holds: + * - `cancelAtPeriodEnd`: while a cancel sync is in flight, its committed value wins over + * snapshots and over echoes of Sim's own writes. A change made in Stripe itself (customer + * portal, dashboard, Better Auth's cancel/restore endpoints) is newer and wins, and is + * committed onto the in-flight events so none can later restore the value it replaced. With + * no sync in flight Stripe wins, read live so out-of-order delivery cannot regress it. + * - `seats`: Team seats are Sim-owned; while a seat sync is in flight its committed value wins. + * - A field with an in-flight event from an older deploy is left as the plugin wrote it. * - * Only the DB row is written: nothing is enqueued and nothing is sent to Stripe, so this cannot - * trigger another webhook. The in-flight sync pushes the restored value. + * Stripe is read only when its value decides, and never under the lock. Only the DB row and + * in-flight payloads are written, so this cannot trigger another webhook. */ export async function reconcileSubscriptionSyncFromStripe(event: Stripe.Event): Promise { if (event.type !== 'customer.subscription.updated') return @@ -174,51 +261,67 @@ export async function reconcileSubscriptionSyncFromStripe(event: Stripe.Event): .limit(1) if (!row) return - const liveSubscription = await requireStripeClient().subscriptions.retrieve(stripeSubscriptionId) const changedInStripe = isCancellationChangedInStripe(event) + let liveCancelAtPeriodEnd: boolean | undefined - await db.transaction(async (tx) => { - const [current] = await tx - .select({ cancelAtPeriodEnd: subscription.cancelAtPeriodEnd, seats: subscription.seats }) - .from(subscription) - .where(eq(subscription.id, row.id)) - .for('update') - .limit(1) - if (!current) return - - const cancelIntent = changedInStripe - ? undefined - : (await readInflightIntent(tx, OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, row.id)) - ?.cancelAtPeriodEnd - const seatsIntent = ( - await readInflightIntent(tx, OUTBOX_EVENT_TYPES.STRIPE_SYNC_SUBSCRIPTION_SEATS, row.id) - )?.seats - - const cancelAtPeriodEnd = - typeof cancelIntent === 'boolean' - ? cancelIntent - : Boolean(liveSubscription.cancel_at_period_end) - const seats = typeof seatsIntent === 'number' ? seatsIntent : current.seats - - if (Boolean(current.cancelAtPeriodEnd) === cancelAtPeriodEnd && current.seats === seats) { - return + for (let pass = 1; pass <= 2; pass++) { + if (liveCancelAtPeriodEnd === undefined) { + const needsStripe = + pass > 1 || + cancelAtPeriodEndSource( + (await readInflightIntents(db, row.id)).cancelAtPeriodEnd, + changedInStripe + ).source === 'stripe' + if (needsStripe) { + const live = await requireStripeClient().subscriptions.retrieve(stripeSubscriptionId) + liveCancelAtPeriodEnd = Boolean(live.cancel_at_period_end) + } } - await tx - .update(subscription) - .set({ cancelAtPeriodEnd, seats }) - .where(eq(subscription.id, row.id)) - - logger.info('Reconciled Sim-owned subscription fields after a Stripe webhook', { - eventId: event.id, - subscriptionId: row.id, - stripeSubscriptionId, - cancelAtPeriodEnd: { - stored: current.cancelAtPeriodEnd, - reconciled: cancelAtPeriodEnd, - source: typeof cancelIntent === 'boolean' ? 'pending-sync' : 'stripe', - }, - seats: { stored: current.seats, reconciled: seats }, + const reconciled = await db.transaction(async (tx) => { + const [current] = await tx + .select({ cancelAtPeriodEnd: subscription.cancelAtPeriodEnd, seats: subscription.seats }) + .from(subscription) + .where(eq(subscription.id, row.id)) + .for('update') + .limit(1) + if (!current) return true + + const intents = await readInflightIntents(tx, row.id) + const cancel = cancelAtPeriodEndSource(intents.cancelAtPeriodEnd, changedInStripe) + let cancelAtPeriodEnd = Boolean(current.cancelAtPeriodEnd) + if (cancel.source === 'pending-sync') { + cancelAtPeriodEnd = cancel.value + } else if (cancel.source === 'stripe') { + if (liveCancelAtPeriodEnd === undefined) return false + cancelAtPeriodEnd = liveCancelAtPeriodEnd + if (intents.cancelAtPeriodEnd.status === 'value') { + await commitIntent(tx, CANCEL_SYNC, row.id, { cancelAtPeriodEnd }) + } + } + const seats = intents.seats.status === 'value' ? intents.seats.value : current.seats + + if (Boolean(current.cancelAtPeriodEnd) === cancelAtPeriodEnd && current.seats === seats) { + return true + } + await tx + .update(subscription) + .set({ cancelAtPeriodEnd, seats }) + .where(eq(subscription.id, row.id)) + + logger.info('Reconciled Sim-owned subscription fields after a Stripe webhook', { + eventId: event.id, + subscriptionId: row.id, + stripeSubscriptionId, + cancelAtPeriodEnd: { + stored: current.cancelAtPeriodEnd, + reconciled: cancelAtPeriodEnd, + source: cancel.source, + }, + seats: { stored: current.seats, reconciled: seats }, + }) + return true }) - }) + if (reconciled) return + } } diff --git a/apps/sim/lib/core/outbox/service.ts b/apps/sim/lib/core/outbox/service.ts index 2990714e39c..4f746d861f5 100644 --- a/apps/sim/lib/core/outbox/service.ts +++ b/apps/sim/lib/core/outbox/service.ts @@ -406,6 +406,60 @@ export async function findDeadLetteredEvents( .limit(DEAD_LETTER_SCAN_LIMIT) } +/** Statuses of an event whose side effect may still run. */ +const INFLIGHT_OUTBOX_STATUSES = ['pending', 'processing'] as const + +/** Identifies the subject of an event by one scalar field of its JSON payload. */ +export interface OutboxPayloadSubject { + payloadKey: string + payloadValue: string +} + +function inflightForSubject(eventTypes: readonly string[], subject: OutboxPayloadSubject) { + return and( + inArray(outboxEvent.eventType, [...eventTypes]), + inArray(outboxEvent.status, [...INFLIGHT_OUTBOX_STATUSES]), + sql`${outboxEvent.payload} ->> ${subject.payloadKey} = ${subject.payloadValue}` + ) +} + +/** + * The `pending` or `processing` events of the given types for one subject. Pass the caller's + * transaction to read under its locks. + */ +export async function listInflightOutboxEvents( + executor: Pick, + eventTypes: readonly string[], + subject: OutboxPayloadSubject, + limit?: number +): Promise<{ id: string; eventType: string; payload: unknown }[]> { + const query = executor + .select({ id: outboxEvent.id, eventType: outboxEvent.eventType, payload: outboxEvent.payload }) + .from(outboxEvent) + .where(inflightForSubject(eventTypes, subject)) + return limit === undefined ? query : query.limit(limit) +} + +/** + * Shallow-merges `patch` into the payload of every `pending` or `processing` event of the type + * for one subject. Callers serialize writers for the subject with their domain lock. + */ +export async function patchInflightOutboxEvents( + executor: Pick, + eventType: string, + subject: OutboxPayloadSubject, + patch: Record +): Promise { + const patched = await executor + .update(outboxEvent) + .set({ + payload: sql`(coalesce(${outboxEvent.payload}::jsonb, '{}'::jsonb) || ${JSON.stringify(patch)}::jsonb)::json`, + }) + .where(inflightForSubject([eventType], subject)) + .returning({ id: outboxEvent.id }) + return patched.length +} + /** * True when an event of the given type whose JSON payload has * `payload->>payloadKey === payloadValue` is still `pending` or `processing`. @@ -417,18 +471,8 @@ export async function hasInflightOutboxEvent( payloadKey: string, payloadValue: string ): Promise { - const [row] = await db - .select({ id: outboxEvent.id }) - .from(outboxEvent) - .where( - and( - eq(outboxEvent.eventType, eventType), - inArray(outboxEvent.status, ['pending', 'processing']), - sql`${outboxEvent.payload} ->> ${payloadKey} = ${payloadValue}` - ) - ) - .limit(1) - return Boolean(row) + const events = await listInflightOutboxEvents(db, [eventType], { payloadKey, payloadValue }, 1) + return events.length > 0 } /** diff --git a/packages/testing/src/mocks/billing-subscription-sync.mock.ts b/packages/testing/src/mocks/billing-subscription-sync.mock.ts index 8eb1b310365..6d5fc87c5bd 100644 --- a/packages/testing/src/mocks/billing-subscription-sync.mock.ts +++ b/packages/testing/src/mocks/billing-subscription-sync.mock.ts @@ -3,6 +3,7 @@ import { vi } from 'vitest' /** * Controllable mock functions for `@/lib/billing/webhooks/subscription-sync`. The enqueue * functions resolve to a fixed event id; drive them with `mockResolvedValueOnce`. + * `mockIsSubscriptionSyncEventType` keeps the real logic. * * @example * ```ts @@ -16,7 +17,12 @@ import { vi } from 'vitest' */ export const billingSubscriptionSyncMockFns = { mockEnqueueCancelAtPeriodEndSync: vi.fn(async () => 'cancel-at-period-end-sync-event'), - mockRecommitCancelAtPeriodEndSync: vi.fn(async () => undefined), + mockRecommitSubscriptionSync: vi.fn(async () => undefined), + mockIsSubscriptionSyncEventType: vi.fn( + (eventType: string) => + eventType === 'stripe.sync-cancel-at-period-end' || + eventType === 'stripe.sync-subscription-seats' + ), mockEnqueueSubscriptionSeatsSync: vi.fn(async () => 'subscription-seats-sync-event'), mockCancelAtPeriodEndSyncIdempotencyKey: vi.fn( (eventId: string) => `outbox-sync-cancel-at-period-end:${eventId}:key` @@ -34,7 +40,8 @@ export const billingSubscriptionSyncMockFns = { */ export const billingSubscriptionSyncMock = { enqueueCancelAtPeriodEndSync: billingSubscriptionSyncMockFns.mockEnqueueCancelAtPeriodEndSync, - recommitCancelAtPeriodEndSync: billingSubscriptionSyncMockFns.mockRecommitCancelAtPeriodEndSync, + recommitSubscriptionSync: billingSubscriptionSyncMockFns.mockRecommitSubscriptionSync, + isSubscriptionSyncEventType: billingSubscriptionSyncMockFns.mockIsSubscriptionSyncEventType, enqueueSubscriptionSeatsSync: billingSubscriptionSyncMockFns.mockEnqueueSubscriptionSeatsSync, cancelAtPeriodEndSyncIdempotencyKey: billingSubscriptionSyncMockFns.mockCancelAtPeriodEndSyncIdempotencyKey, diff --git a/packages/testing/src/mocks/index.ts b/packages/testing/src/mocks/index.ts index 13ff671430b..f8dd5d0d97a 100644 --- a/packages/testing/src/mocks/index.ts +++ b/packages/testing/src/mocks/index.ts @@ -753,9 +753,6 @@ export { createInMemoryStripe, createMockStripeEvent, type InMemoryStripe, - type InMemoryStripeCustomer, - type InMemoryStripeRequestGate, - type InMemoryStripeSubscription, stripeClientMock, stripePaymentMethodMock, } from './stripe.mock' diff --git a/packages/testing/src/mocks/outbox-service.mock.ts b/packages/testing/src/mocks/outbox-service.mock.ts index 15f80918643..60bfa822fa7 100644 --- a/packages/testing/src/mocks/outbox-service.mock.ts +++ b/packages/testing/src/mocks/outbox-service.mock.ts @@ -68,6 +68,8 @@ export const outboxServiceMockFns = { ) }), mockFindDeadLetteredEvents: vi.fn(), + mockListInflightOutboxEvents: vi.fn(), + mockPatchInflightOutboxEvents: vi.fn(), mockHasInflightOutboxEvent: vi.fn(), mockHasDueOutboxWork: vi.fn(), mockProcessOutboxEvents: vi.fn(), @@ -97,6 +99,8 @@ export const outboxServiceMock = { outboxEventHasSourceOperationId: outboxServiceMockFns.mockOutboxEventHasSourceOperationId, outboxPayloadHasSourceOperationId: outboxServiceMockFns.mockOutboxPayloadHasSourceOperationId, findDeadLetteredEvents: outboxServiceMockFns.mockFindDeadLetteredEvents, + listInflightOutboxEvents: outboxServiceMockFns.mockListInflightOutboxEvents, + patchInflightOutboxEvents: outboxServiceMockFns.mockPatchInflightOutboxEvents, hasInflightOutboxEvent: outboxServiceMockFns.mockHasInflightOutboxEvent, hasDueOutboxWork: outboxServiceMockFns.mockHasDueOutboxWork, processOutboxEvents: outboxServiceMockFns.mockProcessOutboxEvents, diff --git a/packages/testing/src/mocks/stripe.mock.ts b/packages/testing/src/mocks/stripe.mock.ts index a6d05601d19..5fce08c41d9 100644 --- a/packages/testing/src/mocks/stripe.mock.ts +++ b/packages/testing/src/mocks/stripe.mock.ts @@ -99,6 +99,7 @@ export interface InMemoryStripeRequestGate { } type UpdatableResource = 'subscriptions' | 'customers' +type StripeOperation = `${UpdatableResource}.${'retrieve' | 'update'}` interface SubscriptionUpdateParams { cancel_at_period_end?: boolean @@ -136,6 +137,7 @@ export function createInMemoryStripe() { Array<{ reached: () => void; released: Promise }> >() const failuresAfterApply = new Map() + const failuresOnArrival = new Map() const events: Stripe.Event[] = [] let sequence = 0 @@ -204,6 +206,11 @@ export function createInMemoryStripe() { return structuredClone(next) } + function rejectIfFailing(operation: StripeOperation) { + const failure = failuresOnArrival.get(operation)?.shift() + if (failure) throw failure + } + async function update( resource: UpdatableResource, id: string, @@ -211,6 +218,7 @@ export function createInMemoryStripe() { options: { idempotencyKey?: string } | undefined, apply: () => T ): Promise { + rejectIfFailing(`${resource}.update`) const gate = gates.get(resource)?.shift() if (gate) { gate.reached() @@ -241,7 +249,10 @@ export function createInMemoryStripe() { const client = { subscriptions: { - retrieve: async (id: string) => structuredClone(requireSubscription(id)), + retrieve: async (id: string) => { + rejectIfFailing('subscriptions.retrieve') + return structuredClone(requireSubscription(id)) + }, update: ( id: string, params: SubscriptionUpdateParams, @@ -252,7 +263,10 @@ export function createInMemoryStripe() { ), }, customers: { - retrieve: async (id: string) => structuredClone(requireCustomer(id)), + retrieve: async (id: string) => { + rejectIfFailing('customers.retrieve') + return structuredClone(requireCustomer(id)) + }, update: (id: string, params: CustomerUpdateParams, options?: { idempotencyKey?: string }) => update('customers', id, params, options, () => { const next = { ...requireCustomer(id), ...params } @@ -327,6 +341,10 @@ export function createInMemoryStripe() { return { reached: arrival, release } }, /** Makes the next update to `resource` apply in Stripe, then fail on the client. */ + /** Makes the next call to `operation` fail before Stripe processes it, as an outage does. */ + failNextRequest(operation: StripeOperation, error = new Error('Stripe is unavailable')) { + failuresOnArrival.set(operation, [...(failuresOnArrival.get(operation) ?? []), error]) + }, failNextUpdateAfterApplying(resource: UpdatableResource, error = new Error('socket hang up')) { failuresAfterApply.set(resource, [...(failuresAfterApply.get(resource) ?? []), error]) }, From ee87f9750edabe3aee64f1bf065ceb344a07a329 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 7 Oct 2026 13:38:26 -0700 Subject: [PATCH 03/14] fix(billing): close remaining stale-intent paths (enterprise retry, legacy echoes, customer restore) --- apps/sim/lib/auth/auth.ts | 15 +++- .../lib/billing/enterprise-provisioning.ts | 8 ++ .../stripe-sync-convergence.integration.ts | 88 ++++++++++++++++++- .../lib/billing/webhooks/subscription-sync.ts | 73 +++++++++++++-- apps/sim/lib/core/outbox/service.ts | 31 ++++++- .../mocks/billing-subscription-sync.mock.ts | 3 + .../testing/src/mocks/outbox-service.mock.ts | 2 + 7 files changed, 208 insertions(+), 12 deletions(-) diff --git a/apps/sim/lib/auth/auth.ts b/apps/sim/lib/auth/auth.ts index cfaf5fe4ec0..35f31d9013a 100644 --- a/apps/sim/lib/auth/auth.ts +++ b/apps/sim/lib/auth/auth.ts @@ -103,7 +103,10 @@ import { handleSubscriptionCreated, handleSubscriptionDeleted, } from '@/lib/billing/webhooks/subscription' -import { reconcileSubscriptionSyncFromStripe } from '@/lib/billing/webhooks/subscription-sync' +import { + commitCustomerRestoredSubscription, + reconcileSubscriptionSyncFromStripe, +} from '@/lib/billing/webhooks/subscription-sync' import { handleSubscriptionUsageUpdate } from '@/lib/billing/webhooks/subscription-usage' import { env } from '@/lib/core/config/env' import { @@ -1101,6 +1104,16 @@ export const auth = betterAuth({ return }), after: createAuthMiddleware(async (ctx) => { + if (isBillingEnabled && ctx.path === '/subscription/restore') { + try { + await commitCustomerRestoredSubscription(ctx.context.returned) + } catch (error) { + logger.error('Failed to record a restored subscription as the committed value', { + error, + }) + } + } + if (isBillingEnabled && ctx.path === '/subscription/upgrade') { const checkoutContext = ctx as typeof ctx & { billingCheckoutAdmissionClaim?: CheckoutAdmissionClaim diff --git a/apps/sim/lib/billing/enterprise-provisioning.ts b/apps/sim/lib/billing/enterprise-provisioning.ts index bb1d5dce165..7438e286c28 100644 --- a/apps/sim/lib/billing/enterprise-provisioning.ts +++ b/apps/sim/lib/billing/enterprise-provisioning.ts @@ -77,6 +77,7 @@ import { TERMINAL_SUBSCRIPTION_STATUSES } from '@/lib/billing/subscriptions/util import { countPendingSeatInvitations } from '@/lib/billing/validation/seat-management' import { withEnterpriseReconciliationLease } from '@/lib/billing/webhooks/enterprise-reconciliation-lease' import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' +import { recommitSubscriptionSync } from '@/lib/billing/webhooks/subscription-sync' import { env } from '@/lib/core/config/env' import { continueOutboxHandler, @@ -2135,6 +2136,13 @@ export async function retryEnterpriseFollowUpJob( processedAt: null, }) .where(eq(outboxEvent.id, jobEventId)) + if (detail.kind === 'personal_subscription_cancellation') { + await recommitSubscriptionSync( + tx, + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + detail.subjectId + ) + } return true }) diff --git a/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts b/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts index 1cb9ed3e5fa..992898e17e1 100644 --- a/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts +++ b/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts @@ -53,7 +53,10 @@ import { isTeam } from '@/lib/billing/plan-helpers' import { syncSeatsFromStripeQuantity } from '@/lib/billing/validation/seat-management' import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' import { billingOutboxHandlers } from '@/lib/billing/webhooks/outbox-handlers' -import { reconcileSubscriptionSyncFromStripe } from '@/lib/billing/webhooks/subscription-sync' +import { + commitCustomerRestoredSubscription, + reconcileSubscriptionSyncFromStripe, +} from '@/lib/billing/webhooks/subscription-sync' import { enqueueOutboxEvent, processOutboxEventById } from '@/lib/core/outbox/service' import { POST as requeueOutboxEvent } from '@/app/api/v1/admin/outbox/[id]/requeue/route' @@ -519,6 +522,89 @@ describe('cancel_at_period_end sync', () => { expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(true) }) + + it('does not revive an older value when a retry path resets its sync without re-committing', async () => { + const pro = await createProUserInPaidOrganization() + await pauseProSubscriptionForOrgCoverage(pro.userId) + const pauseSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + await deadLetter(pauseSync) + await leaveOrganization(pro.userId, pro.paidOrganization.organizationId) + await restoreUserProSubscription(pro.userId) + const restoreSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + await expect(processEvent(restoreSync)).resolves.toBe('completed') + + await testDatabase + .update(outboxEvent) + .set({ + status: 'pending', + attempts: 0, + lastError: null, + availableAt: new Date(), + lockedAt: null, + processedAt: null, + }) + .where(eq(outboxEvent.id, pauseSync)) + await deliverUnrelatedUpdate(pro.stripeSubscriptionId) + + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(false) + await expect(processEvent(pauseSync)).resolves.toBe('completed') + expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(false) + }) + + it("treats a write from an older deploy's sync handler as Sim's own echo", async () => { + const pro = await createProUserInPaidOrganization() + await pauseProSubscriptionForOrgCoverage(pro.userId) + const pauseSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + await leaveOrganization(pro.userId, pro.paidOrganization.organizationId) + await restoreUserProSubscription(pro.userId) + + await stripe.client.subscriptions.update( + pro.stripeSubscriptionId, + { cancel_at_period_end: true }, + { idempotencyKey: `outbox:${pauseSync}` } + ) + await deliver(stripe.events.at(-1) as Stripe.Event) + + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(false) + }) + + it("keeps a customer's restore made while an earlier sync is retrying", async () => { + const pro = await createProUserInPaidOrganization() + await pauseProSubscriptionForOrgCoverage(pro.userId) + const pauseSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + stripe.failNextUpdateAfterApplying('subscriptions') + await expect(processEvent(pauseSync)).resolves.toBe('pending') + + const restored = await stripe.client.subscriptions.update(pro.stripeSubscriptionId, { + cancel_at_period_end: false, + }) + const restoreEvent = stripe.events.at(-1) as Stripe.Event + await testDatabase + .update(subscription) + .set({ cancelAtPeriodEnd: false, cancelAt: null, canceledAt: null }) + .where(eq(subscription.id, pro.subscriptionId)) + await commitCustomerRestoredSubscription(restored) + + await deliverUnrelatedUpdate(pro.stripeSubscriptionId) + await makeDue(pauseSync) + await expect(processEvent(pauseSync)).resolves.toBe('completed') + await deliver(restoreEvent) + + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(false) + expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(false) + }) }) describe('customer contact sync', () => { diff --git a/apps/sim/lib/billing/webhooks/subscription-sync.ts b/apps/sim/lib/billing/webhooks/subscription-sync.ts index 85d02c333e1..b514a6ca23a 100644 --- a/apps/sim/lib/billing/webhooks/subscription-sync.ts +++ b/apps/sim/lib/billing/webhooks/subscription-sync.ts @@ -10,6 +10,7 @@ import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' import { enqueueOutboxEvent, listInflightOutboxEvents, + maxSettledOutboxPayloadNumber, patchInflightOutboxEvents, } from '@/lib/core/outbox/service' import type { DbOrTx } from '@/lib/db/types' @@ -22,6 +23,8 @@ const logger = createLogger('BillingSubscriptionSync') * webhook is recognized as the echo of Sim's own sync rather than a change made in Stripe. */ const CANCEL_AT_PERIOD_END_SYNC_KEY_PREFIX = 'outbox-sync-cancel-at-period-end:' +/** The key prefix the cancel-sync handler used before the one above; old pods still send it mid-rollout. */ +const LEGACY_SYNC_KEY_PREFIX = 'outbox:' const CANCEL_SYNC = OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END const SEATS_SYNC = OUTBOX_EVENT_TYPES.STRIPE_SYNC_SUBSCRIPTION_SEATS @@ -161,20 +164,62 @@ export async function recommitSubscriptionSync( ) } +/** + * Records a customer's restore through Better Auth's `/subscription/restore` as Sim's latest + * committed `cancelAtPeriodEnd`. That endpoint updates Stripe and then writes the row directly, + * so a cancel sync still in flight would otherwise carry the cancellation the customer just + * undid, and a webhook processed before the restore's own could restore it for that sync to + * push. Enqueuing re-pushes `false` even if such a sync already ran. Called from the endpoint's + * `after` hook with the Stripe subscription it returned. + */ +export async function commitCustomerRestoredSubscription(restored: unknown): Promise { + const stripeSubscription = toRecord(restored) + const stripeSubscriptionId = stripeSubscription.id + if ( + typeof stripeSubscriptionId !== 'string' || + stripeSubscription.cancel_at_period_end !== false + ) { + return + } + + await db.transaction(async (tx) => { + const [row] = await tx + .select({ id: subscription.id }) + .from(subscription) + .where(eq(subscription.stripeSubscriptionId, stripeSubscriptionId)) + .for('update') + .limit(1) + if (!row) return + + await tx + .update(subscription) + .set({ cancelAtPeriodEnd: false }) + .where(eq(subscription.id, row.id)) + await enqueueCancelAtPeriodEndSync(tx, { + stripeSubscriptionId, + subscriptionId: row.id, + cancelAtPeriodEnd: false, + reason: 'customer-restored', + }) + }) +} + /** A fresh key per Stripe write: the SDK reuses it across its own network retries of that call. */ export function cancelAtPeriodEndSyncIdempotencyKey(eventId: string): string { return `${CANCEL_AT_PERIOD_END_SYNC_KEY_PREFIX}${eventId}:${generateShortId()}` } /** - * What a sync type's in-flight events say about its field: nothing in flight, the latest + * What a sync type's in-flight events say about its field: no applicable value, the latest * committed value, or `legacy` when an event predates recorded values (enqueued by an older - * deploy) so the committed value is unknown. + * deploy) so the committed value is unknown. An in-flight value older than one a settled event + * already carried was revived by a retry path that did not re-commit, and does not apply. */ type InflightIntent = { status: 'none' } | { status: 'legacy' } | { status: 'value'; value: T } function latestIntent( events: { eventType: string; payload: unknown }[], + settledCommittedAt: Map, eventType: SubscriptionSyncEventType, readValue: (payload: Record) => T | undefined ): InflightIntent { @@ -188,20 +233,27 @@ function latestIntent( latest = { committedAt: payload.committedAt, value } } } - return latest ? { status: 'value', value: latest.value } : { status: 'none' } + if (!latest || latest.committedAt < (settledCommittedAt.get(eventType) ?? 0)) { + return { status: 'none' } + } + return { status: 'value', value: latest.value } } async function readInflightIntents(executor: DbOrTx, subscriptionId: string) { - const events = await listInflightOutboxEvents( + const eventTypes = [CANCEL_SYNC, SEATS_SYNC] + const subject = subscriptionSubject(subscriptionId) + const events = await listInflightOutboxEvents(executor, eventTypes, subject) + const settledCommittedAt = await maxSettledOutboxPayloadNumber( executor, - [CANCEL_SYNC, SEATS_SYNC], - subscriptionSubject(subscriptionId) + eventTypes, + subject, + 'committedAt' ) return { - cancelAtPeriodEnd: latestIntent(events, CANCEL_SYNC, (payload) => + cancelAtPeriodEnd: latestIntent(events, settledCommittedAt, CANCEL_SYNC, (payload) => typeof payload.cancelAtPeriodEnd === 'boolean' ? payload.cancelAtPeriodEnd : undefined ), - seats: latestIntent(events, SEATS_SYNC, (payload) => + seats: latestIntent(events, settledCommittedAt, SEATS_SYNC, (payload) => typeof payload.seats === 'number' ? payload.seats : undefined ), } @@ -212,7 +264,10 @@ function isCancellationChangedInStripe(event: Stripe.Event): boolean { const previousAttributes = toRecord(event.data.previous_attributes) if (!('cancel_at_period_end' in previousAttributes)) return false const idempotencyKey = event.request?.idempotency_key - return !idempotencyKey?.startsWith(CANCEL_AT_PERIOD_END_SYNC_KEY_PREFIX) + const issuedBySimSync = + idempotencyKey?.startsWith(CANCEL_AT_PERIOD_END_SYNC_KEY_PREFIX) || + idempotencyKey?.startsWith(LEGACY_SYNC_KEY_PREFIX) + return !issuedBySimSync } type CancelAtPeriodEndSource = diff --git a/apps/sim/lib/core/outbox/service.ts b/apps/sim/lib/core/outbox/service.ts index 4f746d861f5..5eb5235ad49 100644 --- a/apps/sim/lib/core/outbox/service.ts +++ b/apps/sim/lib/core/outbox/service.ts @@ -4,7 +4,7 @@ import { createLogger } from '@sim/logger' import { describeError, toError } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' import { truncate } from '@sim/utils/string' -import { and, asc, desc, eq, inArray, lte, sql } from 'drizzle-orm' +import { and, asc, desc, eq, inArray, lte, notInArray, sql } from 'drizzle-orm' import { dueOutboxWorkQuery, isStuckProcessing, @@ -440,6 +440,35 @@ export async function listInflightOutboxEvents( return limit === undefined ? query : query.limit(limit) } +/** + * For each event type, the largest numeric `payloadKey` among the subject's settled + * (`completed` or `dead_letter`) events. Types with no such value are absent from the map. + */ +export async function maxSettledOutboxPayloadNumber( + executor: Pick, + eventTypes: readonly string[], + subject: OutboxPayloadSubject, + payloadKey: string +): Promise> { + const rows = await executor + .select({ + eventType: outboxEvent.eventType, + value: sql`max((${outboxEvent.payload} ->> ${payloadKey})::numeric)::text`, + }) + .from(outboxEvent) + .where( + and( + inArray(outboxEvent.eventType, [...eventTypes]), + notInArray(outboxEvent.status, [...INFLIGHT_OUTBOX_STATUSES]), + sql`${outboxEvent.payload} ->> ${subject.payloadKey} = ${subject.payloadValue}` + ) + ) + .groupBy(outboxEvent.eventType) + return new Map( + rows.flatMap((row) => (row.value === null ? [] : [[row.eventType, Number(row.value)]])) + ) +} + /** * Shallow-merges `patch` into the payload of every `pending` or `processing` event of the type * for one subject. Callers serialize writers for the subject with their domain lock. diff --git a/packages/testing/src/mocks/billing-subscription-sync.mock.ts b/packages/testing/src/mocks/billing-subscription-sync.mock.ts index 6d5fc87c5bd..27ba4076f5d 100644 --- a/packages/testing/src/mocks/billing-subscription-sync.mock.ts +++ b/packages/testing/src/mocks/billing-subscription-sync.mock.ts @@ -28,6 +28,7 @@ export const billingSubscriptionSyncMockFns = { (eventId: string) => `outbox-sync-cancel-at-period-end:${eventId}:key` ), mockReconcileSubscriptionSyncFromStripe: vi.fn(async () => undefined), + mockCommitCustomerRestoredSubscription: vi.fn(async () => undefined), } /** @@ -47,4 +48,6 @@ export const billingSubscriptionSyncMock = { billingSubscriptionSyncMockFns.mockCancelAtPeriodEndSyncIdempotencyKey, reconcileSubscriptionSyncFromStripe: billingSubscriptionSyncMockFns.mockReconcileSubscriptionSyncFromStripe, + commitCustomerRestoredSubscription: + billingSubscriptionSyncMockFns.mockCommitCustomerRestoredSubscription, } diff --git a/packages/testing/src/mocks/outbox-service.mock.ts b/packages/testing/src/mocks/outbox-service.mock.ts index 60bfa822fa7..9fdfb6a474a 100644 --- a/packages/testing/src/mocks/outbox-service.mock.ts +++ b/packages/testing/src/mocks/outbox-service.mock.ts @@ -70,6 +70,7 @@ export const outboxServiceMockFns = { mockFindDeadLetteredEvents: vi.fn(), mockListInflightOutboxEvents: vi.fn(), mockPatchInflightOutboxEvents: vi.fn(), + mockMaxSettledOutboxPayloadNumber: vi.fn(), mockHasInflightOutboxEvent: vi.fn(), mockHasDueOutboxWork: vi.fn(), mockProcessOutboxEvents: vi.fn(), @@ -101,6 +102,7 @@ export const outboxServiceMock = { findDeadLetteredEvents: outboxServiceMockFns.mockFindDeadLetteredEvents, listInflightOutboxEvents: outboxServiceMockFns.mockListInflightOutboxEvents, patchInflightOutboxEvents: outboxServiceMockFns.mockPatchInflightOutboxEvents, + maxSettledOutboxPayloadNumber: outboxServiceMockFns.mockMaxSettledOutboxPayloadNumber, hasInflightOutboxEvent: outboxServiceMockFns.mockHasInflightOutboxEvent, hasDueOutboxWork: outboxServiceMockFns.mockHasDueOutboxWork, processOutboxEvents: outboxServiceMockFns.mockProcessOutboxEvents, From 3873afddcf70e869fdad4f04aab35698c34cc30c Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 7 Oct 2026 13:53:35 -0700 Subject: [PATCH 04/14] fix(billing): record Team activation's cleared cancellation under the row lock; drop the settled-event scan --- .../organizations/provision-seat.test.ts | 22 ++- .../billing/organizations/provision-seat.ts | 63 ++++---- .../stripe-sync-convergence.integration.ts | 153 +++++++++++++++++- .../lib/billing/webhooks/subscription-sync.ts | 75 +++++---- apps/sim/lib/core/outbox/service.ts | 78 ++++----- .../mocks/billing-subscription-sync.mock.ts | 2 + .../testing/src/mocks/outbox-service.mock.ts | 10 +- 7 files changed, 284 insertions(+), 119 deletions(-) diff --git a/apps/sim/lib/billing/organizations/provision-seat.test.ts b/apps/sim/lib/billing/organizations/provision-seat.test.ts index a30f896d796..cb42956831a 100644 --- a/apps/sim/lib/billing/organizations/provision-seat.test.ts +++ b/apps/sim/lib/billing/organizations/provision-seat.test.ts @@ -58,18 +58,24 @@ const mockGetHighestPriorityPersonalSubscription = const { mockEnqueueSubscriptionSeatsSync, mockEnqueueCancelAtPeriodEndSync } = billingSubscriptionSyncMockFns -function testExecutor(onUpdate: () => void = () => {}) { +/** The subscription row as the activation re-reads it under its lock. */ +function testExecutor(onSubscriptionLock: () => void = () => {}) { + const lockedRow = { cancelAtPeriodEnd: false, seats: 1 } return { + select: () => ({ + from: () => ({ + where: () => ({ + for: () => { + onSubscriptionLock() + return { limit: () => Promise.resolve([lockedRow]) } + }, + }), + }), + }), update: () => ({ set: (values: Record) => { - onUpdate() updateCalls.value.push(values) - return { - where: () => - Object.assign(Promise.resolve([]), { - returning: () => Promise.resolve([{ seats: 1 }]), - }), - } + return { where: () => Promise.resolve([]) } }, }), } as never diff --git a/apps/sim/lib/billing/organizations/provision-seat.ts b/apps/sim/lib/billing/organizations/provision-seat.ts index 5ef2cb4152d..b397dcd0a8e 100644 --- a/apps/sim/lib/billing/organizations/provision-seat.ts +++ b/apps/sim/lib/billing/organizations/provision-seat.ts @@ -19,6 +19,7 @@ import { hasUsableSubscriptionStatus } from '@/lib/billing/subscriptions/utils' import { enqueueCancelAtPeriodEndSync, enqueueSubscriptionSeatsSync, + recordCancelAtPeriodEnd, } from '@/lib/billing/webhooks/subscription-sync' import type { DbOrTx, DbTransaction } from '@/lib/db/types' @@ -238,43 +239,49 @@ async function convertPersonalSubscriptionToTeam( * the post-join seat reconcile is skipped or fails. Any scheduled cancellation * is cleared (DB + Stripe) so a freshly-activated Team is not left scheduled to * cancel, including the legacy personal-scoped Team case where the plan is - * unchanged. + * unchanged. The row is read under its lock, so a cancellation committed after + * the caller's earlier read is still cleared and recorded. */ async function activateTeamSubscription( - sub: { id: string; cancelAtPeriodEnd?: boolean | null; stripeSubscriptionId: string | null }, + sub: { id: string; stripeSubscriptionId: string | null }, targetPlan: string, { planChanged }: { planChanged: boolean }, - executor: DbOrTx + tx: DbOrTx ): Promise { - const shouldClearCancellation = - Boolean(sub.cancelAtPeriodEnd) && Boolean(sub.stripeSubscriptionId) - - const apply = async (tx: DbOrTx) => { - const [activated] = await tx - .update(subscriptionTable) - .set({ plan: targetPlan, cancelAtPeriodEnd: false }) - .where(eq(subscriptionTable.id, sub.id)) - .returning({ seats: subscriptionTable.seats }) + const [locked] = await tx + .select({ + cancelAtPeriodEnd: subscriptionTable.cancelAtPeriodEnd, + seats: subscriptionTable.seats, + }) + .from(subscriptionTable) + .where(eq(subscriptionTable.id, sub.id)) + .for('update') + .limit(1) - if (planChanged) { - await enqueueSubscriptionSeatsSync(tx, { - subscriptionId: sub.id, - seats: activated?.seats ?? 1, - reason: 'pro-to-team-conversion', - }) - } + await tx + .update(subscriptionTable) + .set({ plan: targetPlan, cancelAtPeriodEnd: false }) + .where(eq(subscriptionTable.id, sub.id)) - if (shouldClearCancellation) { - await enqueueCancelAtPeriodEndSync(tx, { - stripeSubscriptionId: sub.stripeSubscriptionId as string, - subscriptionId: sub.id, - cancelAtPeriodEnd: false, - reason: 'pro-to-team-conversion', - }) - } + if (planChanged) { + await enqueueSubscriptionSeatsSync(tx, { + subscriptionId: sub.id, + seats: locked?.seats ?? 1, + reason: 'pro-to-team-conversion', + }) } - await apply(executor) + if (!sub.stripeSubscriptionId) return + if (locked?.cancelAtPeriodEnd) { + await enqueueCancelAtPeriodEndSync(tx, { + stripeSubscriptionId: sub.stripeSubscriptionId, + subscriptionId: sub.id, + cancelAtPeriodEnd: false, + reason: 'pro-to-team-conversion', + }) + } else { + await recordCancelAtPeriodEnd(tx, sub.id, false) + } } /** diff --git a/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts b/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts index 992898e17e1..050402a2f2a 100644 --- a/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts +++ b/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts @@ -48,6 +48,7 @@ import { pauseProSubscriptionForOrgCoverage, restoreUserProSubscription, } from '@/lib/billing/organizations/membership' +import { ensureTeamOrganizationForAcceptance } from '@/lib/billing/organizations/provision-seat' import { reconcileOrganizationSeats } from '@/lib/billing/organizations/seats' import { isTeam } from '@/lib/billing/plan-helpers' import { syncSeatsFromStripeQuantity } from '@/lib/billing/validation/seat-management' @@ -55,6 +56,7 @@ import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' import { billingOutboxHandlers } from '@/lib/billing/webhooks/outbox-handlers' import { commitCustomerRestoredSubscription, + enqueueCancelAtPeriodEndSync, reconcileSubscriptionSyncFromStripe, } from '@/lib/billing/webhooks/subscription-sync' import { enqueueOutboxEvent, processOutboxEventById } from '@/lib/core/outbox/service' @@ -122,7 +124,15 @@ let deliver: ReturnType beforeAll(async () => { await connection`CREATE SCHEMA ${connection(schemaName)}` - for (const table of ['subscription', 'outbox_event', 'member', 'user', 'organization']) { + for (const table of [ + 'subscription', + 'outbox_event', + 'member', + 'user', + 'organization', + 'workspace', + 'permissions', + ]) { await connection.unsafe(`CREATE TABLE "${table}" (LIKE public."${table}" INCLUDING ALL)`) } database.current = testDatabase @@ -607,6 +617,77 @@ describe('cancel_at_period_end sync', () => { }) }) +describe('Team activation', () => { + /** Resolves once another backend is blocked on a lock, i.e. the racing transaction is parked. */ + async function untilAnotherTransactionWaitsOnALock() { + for (let attempt = 0; attempt < 200; attempt++) { + const [row] = await connection<{ waiting: number }[]>` + select count(*)::int as waiting from pg_stat_activity + where datname = current_database() and wait_event_type = 'Lock'` + if (row.waiting > 0) return + await new Promise((resolve) => setImmediate(resolve)) + } + throw new Error('The racing transaction never waited on a lock') + } + + it('records the cleared cancellation when a cancel is committed while it activates Team', async () => { + const owner = await createUser('owner') + const subscriptionId = generateId() + const stripeSubscriptionId = `sub_${subscriptionId}` + await testDatabase.insert(subscription).values({ + id: subscriptionId, + plan: 'team', + referenceId: owner.id, + status: 'active', + seats: 1, + stripeSubscriptionId, + stripeCustomerId: `cus_${subscriptionId}`, + cancelAtPeriodEnd: false, + }) + stripe.addSubscription({ id: stripeSubscriptionId, customer: `cus_${subscriptionId}` }) + + let releaseCancel: () => void = () => {} + const cancelHeld = new Promise((resolve) => { + releaseCancel = resolve + }) + const cancelling = testDatabase.transaction(async (tx) => { + await tx + .update(subscription) + .set({ cancelAtPeriodEnd: true }) + .where(eq(subscription.id, subscriptionId)) + await enqueueCancelAtPeriodEndSync(tx, { + stripeSubscriptionId, + subscriptionId, + cancelAtPeriodEnd: true, + reason: 'admin-cancel-at-period-end', + }) + await cancelHeld + }) + const activating = testDatabase.transaction((tx) => + ensureTeamOrganizationForAcceptance({ + billingOwnerUserId: owner.id, + workspaceOrganizationId: null, + executor: tx, + workspaceIdsToAttach: [], + }) + ) + await untilAnotherTransactionWaitsOnALock() + releaseCancel() + await cancelling + await expect(activating).resolves.toMatchObject({ success: true }) + expect((await storedSubscription(subscriptionId)).cancelAtPeriodEnd).toBe(false) + + await deliverUnrelatedUpdate(stripeSubscriptionId) + expect((await storedSubscription(subscriptionId)).cancelAtPeriodEnd).toBe(false) + const cancelSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + subscriptionId + ) + await expect(processEvent(cancelSync)).resolves.toBe('completed') + expect(stripe.subscription(stripeSubscriptionId).cancel_at_period_end).toBe(false) + }) +}) + describe('customer contact sync', () => { it('pushes the current owner when an earlier sync lands in Stripe after a newer one', async () => { const [first, second, third] = await Promise.all([ @@ -704,3 +785,73 @@ describe('Team seat sync', () => { expect(stripe.subscription(org.stripeSubscriptionId).items.data[0].quantity).toBe(1) }) }) + +describe('webhook reconcile cost', () => { + interface QueryPlan { + 'Node Type': string + 'Relation Name'?: string + 'Shared Hit Blocks': number + 'Shared Read Blocks': number + Plans?: QueryPlan[] + } + const planNodes = (plan: QueryPlan): QueryPlan[] => [ + plan, + ...(plan.Plans ?? []).flatMap(planNodes), + ] + + it('reads only the syncs that can still run, however many have completed', async () => { + const pro = await createProUserInPaidOrganization() + await pauseProSubscriptionForOrgCoverage(pro.userId) + await connection` + INSERT INTO outbox_event (id, event_type, payload, status, available_at, created_at, processed_at) + SELECT ${generateId()} || ':' || n, ${OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END}, + json_build_object( + 'subscriptionId', ${pro.subscriptionId}::text, + 'cancelAtPeriodEnd', n % 2 = 0, + 'committedAt', n + ), + 'completed', now(), now(), now() + FROM generate_series(1, 20000) AS n` + await connection`ANALYZE outbox_event` + + const issued: { query: string; parameters: unknown[] }[] = [] + const traced = postgres( + readTestDatabaseUrl(), + withUtcTimestamps({ + max: 2, + prepare: false, + fetch_types: false, + connection: { search_path: schemaName }, + onnotice: () => {}, + debug: (_connection: number, query: string, parameters: unknown[]) => { + issued.push({ query, parameters }) + }, + }) + ) + database.current = drizzle(traced, { schema }) + try { + await deliverUnrelatedUpdate(pro.stripeSubscriptionId) + } finally { + database.current = testDatabase + await traced.end() + } + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(true) + + const outboxReads = issued.filter(({ query }) => /from "outbox_event"/i.test(query)) + expect(outboxReads.length).toBeGreaterThan(0) + for (const { query, parameters } of outboxReads) { + const [explained] = await connection.unsafe( + `EXPLAIN (ANALYZE, BUFFERS, FORMAT JSON) ${query}`, + parameters as never[] + ) + const plan = (explained['QUERY PLAN'] as { Plan: QueryPlan }[])[0].Plan + const nodes = planNodes(plan) + expect( + nodes.some( + (node) => node['Node Type'] === 'Seq Scan' && node['Relation Name'] === 'outbox_event' + ) + ).toBe(false) + expect(plan['Shared Hit Blocks'] + plan['Shared Read Blocks']).toBeLessThan(100) + } + }) +}) diff --git a/apps/sim/lib/billing/webhooks/subscription-sync.ts b/apps/sim/lib/billing/webhooks/subscription-sync.ts index b514a6ca23a..8995791ab03 100644 --- a/apps/sim/lib/billing/webhooks/subscription-sync.ts +++ b/apps/sim/lib/billing/webhooks/subscription-sync.ts @@ -9,9 +9,8 @@ import { requireStripeClient } from '@/lib/billing/stripe-client' import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' import { enqueueOutboxEvent, - listInflightOutboxEvents, - maxSettledOutboxPayloadNumber, - patchInflightOutboxEvents, + listRetryableOutboxEvents, + patchRetryableOutboxEvents, } from '@/lib/core/outbox/service' import type { DbOrTx } from '@/lib/db/types' @@ -88,9 +87,10 @@ async function withCommittedAt( /** * Records `fields` as the subscription's latest committed value for `eventType` and writes it - * onto every in-flight event of that type, so no pending, retrying, or reaped event still - * carries an older value for the webhook reconcile to restore. The caller must hold the - * subscription row lock (`FOR UPDATE`, or the `UPDATE` itself). + * onto every event of that type that can still run: pending, processing, or dead-lettered (each + * operator retry path resets dead letters to pending). No event that can run again ever carries + * an older value for the webhook reconcile to restore. The caller must hold the subscription row + * lock (`FOR UPDATE`, or the `UPDATE` itself). */ async function commitIntent( tx: DbOrTx, @@ -99,7 +99,7 @@ async function commitIntent( fields: T ): Promise { const committed = await withCommittedAt(tx, fields) - await patchInflightOutboxEvents(tx, eventType, subscriptionSubject(subscriptionId), committed) + await patchRetryableOutboxEvents(tx, eventType, subscriptionSubject(subscriptionId), committed) return committed } @@ -136,6 +136,19 @@ export async function enqueueSubscriptionSeatsSync( return enqueueOutboxEvent(tx, SEATS_SYNC, intent) } +/** + * Records a `cancelAtPeriodEnd` value written in this transaction without enqueuing a sync, for + * a writer that left the row's value unchanged: no sync that can still run keeps an older value. + * The caller must hold the subscription row lock. + */ +export async function recordCancelAtPeriodEnd( + tx: DbOrTx, + subscriptionId: string, + cancelAtPeriodEnd: boolean +): Promise { + await commitIntent(tx, CANCEL_SYNC, subscriptionId, { cancelAtPeriodEnd }) +} + /** * Re-commits the subscription's current DB value onto its in-flight sync events, for a * dead-lettered event that was just reset to `pending`: the retry then carries the latest value @@ -210,22 +223,22 @@ export function cancelAtPeriodEndSyncIdempotencyKey(eventId: string): string { } /** - * What a sync type's in-flight events say about its field: no applicable value, the latest + * What a sync type's in-flight events say about its field: nothing in flight, the latest * committed value, or `legacy` when an event predates recorded values (enqueued by an older - * deploy) so the committed value is unknown. An in-flight value older than one a settled event - * already carried was revived by a retry path that did not re-commit, and does not apply. + * deploy) so the committed value is unknown. */ type InflightIntent = { status: 'none' } | { status: 'legacy' } | { status: 'value'; value: T } +const INFLIGHT_STATUSES = new Set(['pending', 'processing']) + function latestIntent( - events: { eventType: string; payload: unknown }[], - settledCommittedAt: Map, + events: { eventType: string; status: string; payload: unknown }[], eventType: SubscriptionSyncEventType, readValue: (payload: Record) => T | undefined ): InflightIntent { let latest: { committedAt: number; value: T } | undefined for (const event of events) { - if (event.eventType !== eventType) continue + if (event.eventType !== eventType || !INFLIGHT_STATUSES.has(event.status)) continue const payload = toRecord(event.payload) const value = readValue(payload) if (typeof payload.committedAt !== 'number' || value === undefined) return { status: 'legacy' } @@ -233,29 +246,29 @@ function latestIntent( latest = { committedAt: payload.committedAt, value } } } - if (!latest || latest.committedAt < (settledCommittedAt.get(eventType) ?? 0)) { - return { status: 'none' } - } - return { status: 'value', value: latest.value } + return latest ? { status: 'value', value: latest.value } : { status: 'none' } } -async function readInflightIntents(executor: DbOrTx, subscriptionId: string) { - const eventTypes = [CANCEL_SYNC, SEATS_SYNC] - const subject = subscriptionSubject(subscriptionId) - const events = await listInflightOutboxEvents(executor, eventTypes, subject) - const settledCommittedAt = await maxSettledOutboxPayloadNumber( +/** One indexed read of the subscription's sync events that can still run. */ +async function readSyncIntents(executor: DbOrTx, subscriptionId: string) { + const events = await listRetryableOutboxEvents( executor, - eventTypes, - subject, - 'committedAt' + [CANCEL_SYNC, SEATS_SYNC], + subscriptionSubject(subscriptionId) ) return { - cancelAtPeriodEnd: latestIntent(events, settledCommittedAt, CANCEL_SYNC, (payload) => + cancelAtPeriodEnd: latestIntent(events, CANCEL_SYNC, (payload) => typeof payload.cancelAtPeriodEnd === 'boolean' ? payload.cancelAtPeriodEnd : undefined ), - seats: latestIntent(events, settledCommittedAt, SEATS_SYNC, (payload) => + seats: latestIntent(events, SEATS_SYNC, (payload) => typeof payload.seats === 'number' ? payload.seats : undefined ), + /** Whether a cancel sync that can still run carries a value other than `value`. */ + cancelSyncCarriesOtherThan: (value: boolean) => + events.some( + (event) => + event.eventType === CANCEL_SYNC && toRecord(event.payload).cancelAtPeriodEnd !== value + ), } } @@ -297,7 +310,7 @@ function cancelAtPeriodEndSource( * - `cancelAtPeriodEnd`: while a cancel sync is in flight, its committed value wins over * snapshots and over echoes of Sim's own writes. A change made in Stripe itself (customer * portal, dashboard, Better Auth's cancel/restore endpoints) is newer and wins, and is - * committed onto the in-flight events so none can later restore the value it replaced. With + * committed onto every sync that can still run so none can later restore the value it replaced. With * no sync in flight Stripe wins, read live so out-of-order delivery cannot regress it. * - `seats`: Team seats are Sim-owned; while a seat sync is in flight its committed value wins. * - A field with an in-flight event from an older deploy is left as the plugin wrote it. @@ -324,7 +337,7 @@ export async function reconcileSubscriptionSyncFromStripe(event: Stripe.Event): const needsStripe = pass > 1 || cancelAtPeriodEndSource( - (await readInflightIntents(db, row.id)).cancelAtPeriodEnd, + (await readSyncIntents(db, row.id)).cancelAtPeriodEnd, changedInStripe ).source === 'stripe' if (needsStripe) { @@ -342,7 +355,7 @@ export async function reconcileSubscriptionSyncFromStripe(event: Stripe.Event): .limit(1) if (!current) return true - const intents = await readInflightIntents(tx, row.id) + const intents = await readSyncIntents(tx, row.id) const cancel = cancelAtPeriodEndSource(intents.cancelAtPeriodEnd, changedInStripe) let cancelAtPeriodEnd = Boolean(current.cancelAtPeriodEnd) if (cancel.source === 'pending-sync') { @@ -350,7 +363,7 @@ export async function reconcileSubscriptionSyncFromStripe(event: Stripe.Event): } else if (cancel.source === 'stripe') { if (liveCancelAtPeriodEnd === undefined) return false cancelAtPeriodEnd = liveCancelAtPeriodEnd - if (intents.cancelAtPeriodEnd.status === 'value') { + if (intents.cancelSyncCarriesOtherThan(cancelAtPeriodEnd)) { await commitIntent(tx, CANCEL_SYNC, row.id, { cancelAtPeriodEnd }) } } diff --git a/apps/sim/lib/core/outbox/service.ts b/apps/sim/lib/core/outbox/service.ts index 5eb5235ad49..09e9de357a2 100644 --- a/apps/sim/lib/core/outbox/service.ts +++ b/apps/sim/lib/core/outbox/service.ts @@ -4,7 +4,7 @@ import { createLogger } from '@sim/logger' import { describeError, toError } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' import { truncate } from '@sim/utils/string' -import { and, asc, desc, eq, inArray, lte, notInArray, sql } from 'drizzle-orm' +import { and, asc, desc, eq, inArray, lte, sql } from 'drizzle-orm' import { dueOutboxWorkQuery, isStuckProcessing, @@ -408,6 +408,11 @@ export async function findDeadLetteredEvents( /** Statuses of an event whose side effect may still run. */ const INFLIGHT_OUTBOX_STATUSES = ['pending', 'processing'] as const +/** + * Statuses an event can still run from: in flight, or dead-lettered, which every operator retry + * path resets to `pending`. A `completed` event never runs again. + */ +const RETRYABLE_OUTBOX_STATUSES = [...INFLIGHT_OUTBOX_STATUSES, 'dead_letter'] as const /** Identifies the subject of an event by one scalar field of its JSON payload. */ export interface OutboxPayloadSubject { @@ -415,65 +420,44 @@ export interface OutboxPayloadSubject { payloadValue: string } -function inflightForSubject(eventTypes: readonly string[], subject: OutboxPayloadSubject) { +function eventsForSubject( + eventTypes: readonly string[], + subject: OutboxPayloadSubject, + statuses: readonly string[] +) { return and( inArray(outboxEvent.eventType, [...eventTypes]), - inArray(outboxEvent.status, [...INFLIGHT_OUTBOX_STATUSES]), + inArray(outboxEvent.status, [...statuses]), sql`${outboxEvent.payload} ->> ${subject.payloadKey} = ${subject.payloadValue}` ) } /** - * The `pending` or `processing` events of the given types for one subject. Pass the caller's - * transaction to read under its locks. - */ -export async function listInflightOutboxEvents( - executor: Pick, - eventTypes: readonly string[], - subject: OutboxPayloadSubject, - limit?: number -): Promise<{ id: string; eventType: string; payload: unknown }[]> { - const query = executor - .select({ id: outboxEvent.id, eventType: outboxEvent.eventType, payload: outboxEvent.payload }) - .from(outboxEvent) - .where(inflightForSubject(eventTypes, subject)) - return limit === undefined ? query : query.limit(limit) -} - -/** - * For each event type, the largest numeric `payloadKey` among the subject's settled - * (`completed` or `dead_letter`) events. Types with no such value are absent from the map. + * The `pending`, `processing`, or `dead_letter` events of the given types for one subject. Pass + * the caller's transaction to read under its locks. */ -export async function maxSettledOutboxPayloadNumber( +export async function listRetryableOutboxEvents( executor: Pick, eventTypes: readonly string[], - subject: OutboxPayloadSubject, - payloadKey: string -): Promise> { - const rows = await executor + subject: OutboxPayloadSubject +): Promise<{ id: string; eventType: string; status: string; payload: unknown }[]> { + return executor .select({ + id: outboxEvent.id, eventType: outboxEvent.eventType, - value: sql`max((${outboxEvent.payload} ->> ${payloadKey})::numeric)::text`, + status: outboxEvent.status, + payload: outboxEvent.payload, }) .from(outboxEvent) - .where( - and( - inArray(outboxEvent.eventType, [...eventTypes]), - notInArray(outboxEvent.status, [...INFLIGHT_OUTBOX_STATUSES]), - sql`${outboxEvent.payload} ->> ${subject.payloadKey} = ${subject.payloadValue}` - ) - ) - .groupBy(outboxEvent.eventType) - return new Map( - rows.flatMap((row) => (row.value === null ? [] : [[row.eventType, Number(row.value)]])) - ) + .where(eventsForSubject(eventTypes, subject, RETRYABLE_OUTBOX_STATUSES)) } /** - * Shallow-merges `patch` into the payload of every `pending` or `processing` event of the type - * for one subject. Callers serialize writers for the subject with their domain lock. + * Shallow-merges `patch` into the payload of every `pending`, `processing`, or `dead_letter` + * event of the type for one subject. Callers serialize writers for the subject with their + * domain lock. */ -export async function patchInflightOutboxEvents( +export async function patchRetryableOutboxEvents( executor: Pick, eventType: string, subject: OutboxPayloadSubject, @@ -484,7 +468,7 @@ export async function patchInflightOutboxEvents( .set({ payload: sql`(coalesce(${outboxEvent.payload}::jsonb, '{}'::jsonb) || ${JSON.stringify(patch)}::jsonb)::json`, }) - .where(inflightForSubject([eventType], subject)) + .where(eventsForSubject([eventType], subject, RETRYABLE_OUTBOX_STATUSES)) .returning({ id: outboxEvent.id }) return patched.length } @@ -500,8 +484,12 @@ export async function hasInflightOutboxEvent( payloadKey: string, payloadValue: string ): Promise { - const events = await listInflightOutboxEvents(db, [eventType], { payloadKey, payloadValue }, 1) - return events.length > 0 + const [row] = await db + .select({ id: outboxEvent.id }) + .from(outboxEvent) + .where(eventsForSubject([eventType], { payloadKey, payloadValue }, INFLIGHT_OUTBOX_STATUSES)) + .limit(1) + return Boolean(row) } /** diff --git a/packages/testing/src/mocks/billing-subscription-sync.mock.ts b/packages/testing/src/mocks/billing-subscription-sync.mock.ts index 27ba4076f5d..c1475f05578 100644 --- a/packages/testing/src/mocks/billing-subscription-sync.mock.ts +++ b/packages/testing/src/mocks/billing-subscription-sync.mock.ts @@ -18,6 +18,7 @@ import { vi } from 'vitest' export const billingSubscriptionSyncMockFns = { mockEnqueueCancelAtPeriodEndSync: vi.fn(async () => 'cancel-at-period-end-sync-event'), mockRecommitSubscriptionSync: vi.fn(async () => undefined), + mockRecordCancelAtPeriodEnd: vi.fn(async () => undefined), mockIsSubscriptionSyncEventType: vi.fn( (eventType: string) => eventType === 'stripe.sync-cancel-at-period-end' || @@ -42,6 +43,7 @@ export const billingSubscriptionSyncMockFns = { export const billingSubscriptionSyncMock = { enqueueCancelAtPeriodEndSync: billingSubscriptionSyncMockFns.mockEnqueueCancelAtPeriodEndSync, recommitSubscriptionSync: billingSubscriptionSyncMockFns.mockRecommitSubscriptionSync, + recordCancelAtPeriodEnd: billingSubscriptionSyncMockFns.mockRecordCancelAtPeriodEnd, isSubscriptionSyncEventType: billingSubscriptionSyncMockFns.mockIsSubscriptionSyncEventType, enqueueSubscriptionSeatsSync: billingSubscriptionSyncMockFns.mockEnqueueSubscriptionSeatsSync, cancelAtPeriodEndSyncIdempotencyKey: diff --git a/packages/testing/src/mocks/outbox-service.mock.ts b/packages/testing/src/mocks/outbox-service.mock.ts index 9fdfb6a474a..543459c2218 100644 --- a/packages/testing/src/mocks/outbox-service.mock.ts +++ b/packages/testing/src/mocks/outbox-service.mock.ts @@ -68,9 +68,8 @@ export const outboxServiceMockFns = { ) }), mockFindDeadLetteredEvents: vi.fn(), - mockListInflightOutboxEvents: vi.fn(), - mockPatchInflightOutboxEvents: vi.fn(), - mockMaxSettledOutboxPayloadNumber: vi.fn(), + mockListRetryableOutboxEvents: vi.fn(), + mockPatchRetryableOutboxEvents: vi.fn(), mockHasInflightOutboxEvent: vi.fn(), mockHasDueOutboxWork: vi.fn(), mockProcessOutboxEvents: vi.fn(), @@ -100,9 +99,8 @@ export const outboxServiceMock = { outboxEventHasSourceOperationId: outboxServiceMockFns.mockOutboxEventHasSourceOperationId, outboxPayloadHasSourceOperationId: outboxServiceMockFns.mockOutboxPayloadHasSourceOperationId, findDeadLetteredEvents: outboxServiceMockFns.mockFindDeadLetteredEvents, - listInflightOutboxEvents: outboxServiceMockFns.mockListInflightOutboxEvents, - patchInflightOutboxEvents: outboxServiceMockFns.mockPatchInflightOutboxEvents, - maxSettledOutboxPayloadNumber: outboxServiceMockFns.mockMaxSettledOutboxPayloadNumber, + listRetryableOutboxEvents: outboxServiceMockFns.mockListRetryableOutboxEvents, + patchRetryableOutboxEvents: outboxServiceMockFns.mockPatchRetryableOutboxEvents, hasInflightOutboxEvent: outboxServiceMockFns.mockHasInflightOutboxEvent, hasDueOutboxWork: outboxServiceMockFns.mockHasDueOutboxWork, processOutboxEvents: outboxServiceMockFns.mockProcessOutboxEvents, From c53a94f6b5422f92c651eaa3404c38bf94d4d69d Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 7 Oct 2026 14:08:28 -0700 Subject: [PATCH 05/14] fix(billing): lock the subscription before the outbox row in every sync retry path --- .../api/v1/admin/outbox/[id]/requeue/route.ts | 24 ++-- .../lib/admin/subscription-lifecycle.test.ts | 7 ++ apps/sim/lib/admin/subscription-lifecycle.ts | 22 +++- .../lib/billing/enterprise-provisioning.ts | 12 +- .../stripe-sync-convergence.integration.ts | 109 ++++++++++++++++-- .../lib/billing/webhooks/subscription-sync.ts | 34 +++++- apps/sim/lib/core/outbox/service.ts | 2 +- .../mocks/billing-subscription-sync.mock.ts | 2 + .../testing/src/mocks/outbox-service.mock.ts | 3 +- 9 files changed, 185 insertions(+), 30 deletions(-) diff --git a/apps/sim/app/api/v1/admin/outbox/[id]/requeue/route.ts b/apps/sim/app/api/v1/admin/outbox/[id]/requeue/route.ts index 2e0c74ca6d8..08ba707cba2 100644 --- a/apps/sim/app/api/v1/admin/outbox/[id]/requeue/route.ts +++ b/apps/sim/app/api/v1/admin/outbox/[id]/requeue/route.ts @@ -14,6 +14,7 @@ import { } from '@/lib/billing/enterprise-outbox-events' import { isSubscriptionSyncEventType, + lockSubscriptionForSyncRetry, recommitSubscriptionSync, } from '@/lib/billing/webhooks/subscription-sync' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' @@ -68,7 +69,17 @@ export const POST = withRouteHandler( const deliveryRevision = metadataIntent?.success ? metadataIntent.data.deliveryRevision + 1 : null + const subscriptionId = toRecord(existing?.payload).subscriptionId + const subscriptionSync = + existing && + isSubscriptionSyncEventType(existing.eventType) && + typeof subscriptionId === 'string' + ? { eventType: existing.eventType, subscriptionId } + : null const result = await db.transaction(async (tx) => { + if (subscriptionSync) { + await lockSubscriptionForSyncRetry(tx, subscriptionSync.subscriptionId) + } const requeued = await tx .update(outboxEvent) .set({ @@ -86,13 +97,12 @@ export const POST = withRouteHandler( }) .where(and(eq(outboxEvent.id, id), eq(outboxEvent.status, 'dead_letter'))) .returning({ id: outboxEvent.id, eventType: outboxEvent.eventType }) - const subscriptionId = toRecord(existing?.payload).subscriptionId - if ( - requeued.length > 0 && - isSubscriptionSyncEventType(requeued[0].eventType) && - typeof subscriptionId === 'string' - ) { - await recommitSubscriptionSync(tx, requeued[0].eventType, subscriptionId) + if (subscriptionSync && requeued.length > 0) { + await recommitSubscriptionSync( + tx, + subscriptionSync.eventType, + subscriptionSync.subscriptionId + ) } return requeued }) diff --git a/apps/sim/lib/admin/subscription-lifecycle.test.ts b/apps/sim/lib/admin/subscription-lifecycle.test.ts index 6f9a3d2e4c1..f86d82a387e 100644 --- a/apps/sim/lib/admin/subscription-lifecycle.test.ts +++ b/apps/sim/lib/admin/subscription-lifecycle.test.ts @@ -103,6 +103,7 @@ describe('admin subscription cancellation', () => { it('requeues the same dead-lettered period-end cancellation operation', async () => { dbChainMockFns.returning.mockResolvedValueOnce([{ id: activeSubscription.id }]) + queueTableRows(outboxEvent, [{ subscriptionId: 'sub-row-1' }]) queueTableRows(outboxEvent, [ { id: 'outbox-1', @@ -124,6 +125,10 @@ describe('admin subscription cancellation', () => { expect.objectContaining({ status: 'pending', attempts: 0, lastError: null }) ) expect(dbChainMockFns.set).toHaveBeenCalledWith({ cancelAtPeriodEnd: true }) + expect(billingSubscriptionSyncMockFns.mockLockSubscriptionForSyncRetry).toHaveBeenCalledWith( + expect.anything(), + 'sub-row-1' + ) expect(billingSubscriptionSyncMockFns.mockRecommitSubscriptionSync).toHaveBeenCalledWith( expect.anything(), 'stripe.sync-cancel-at-period-end', @@ -136,6 +141,7 @@ describe('admin subscription cancellation', () => { }) it('replays an immediate cancellation after the webhook removed active entitlement', async () => { + queueTableRows(outboxEvent, []) queueTableRows(outboxEvent, [ { id: 'outbox-1', @@ -159,6 +165,7 @@ describe('admin subscription cancellation', () => { }) it('rejects reuse of a cancellation operation id with different timing', async () => { + queueTableRows(outboxEvent, []) queueTableRows(outboxEvent, [ { id: 'outbox-1', diff --git a/apps/sim/lib/admin/subscription-lifecycle.ts b/apps/sim/lib/admin/subscription-lifecycle.ts index abacaa25fb0..6bd34a029e1 100644 --- a/apps/sim/lib/admin/subscription-lifecycle.ts +++ b/apps/sim/lib/admin/subscription-lifecycle.ts @@ -10,6 +10,7 @@ import { ENTITLED_SUBSCRIPTION_STATUSES } from '@/lib/billing/subscriptions/util import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' import { enqueueCancelAtPeriodEndSync, + lockSubscriptionForSyncRetry, recommitSubscriptionSync, } from '@/lib/billing/webhooks/subscription-sync' import { enqueueOutboxEvent } from '@/lib/core/outbox/service' @@ -261,6 +262,24 @@ export async function requestDashboardSubscriptionCancellation({ : 'admin-dashboard-cancel-at-period-end') const cancellation = await db.transaction(async (tx) => { await acquireOrganizationMutationLock(tx, organizationId) + const isThisOperation = and( + sql`${outboxEvent.payload} ->> 'operationId' = ${operationId}`, + sql`${outboxEvent.payload} ->> 'organizationId' = ${organizationId}` + ) + + const [retriedSync] = await tx + .select({ subscriptionId: sql`${outboxEvent.payload} ->> 'subscriptionId'` }) + .from(outboxEvent) + .where( + and( + eq(outboxEvent.eventType, OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END), + isThisOperation + ) + ) + .limit(1) + if (retriedSync?.subscriptionId) { + await lockSubscriptionForSyncRetry(tx, retriedSync.subscriptionId) + } const [existingOperation] = await tx .select({ @@ -277,8 +296,7 @@ export async function requestDashboardSubscriptionCancellation({ OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, OUTBOX_EVENT_TYPES.STRIPE_CANCEL_SUBSCRIPTION_IMMEDIATELY, ]), - sql`${outboxEvent.payload} ->> 'operationId' = ${operationId}`, - sql`${outboxEvent.payload} ->> 'organizationId' = ${organizationId}` + isThisOperation ) ) .for('update') diff --git a/apps/sim/lib/billing/enterprise-provisioning.ts b/apps/sim/lib/billing/enterprise-provisioning.ts index 7438e286c28..cc3962f5a79 100644 --- a/apps/sim/lib/billing/enterprise-provisioning.ts +++ b/apps/sim/lib/billing/enterprise-provisioning.ts @@ -77,7 +77,10 @@ import { TERMINAL_SUBSCRIPTION_STATUSES } from '@/lib/billing/subscriptions/util import { countPendingSeatInvitations } from '@/lib/billing/validation/seat-management' import { withEnterpriseReconciliationLease } from '@/lib/billing/webhooks/enterprise-reconciliation-lease' import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' -import { recommitSubscriptionSync } from '@/lib/billing/webhooks/subscription-sync' +import { + lockSubscriptionForSyncRetry, + recommitSubscriptionSync, +} from '@/lib/billing/webhooks/subscription-sync' import { env } from '@/lib/core/config/env' import { continueOutboxHandler, @@ -2104,6 +2107,9 @@ export async function retryEnterpriseFollowUpJob( const retried = await db.transaction(async (tx) => { await acquireOrganizationMutationLock(tx, operationPayload.request.organizationId) + if (snapshotDetail.kind === 'personal_subscription_cancellation') { + await lockSubscriptionForSyncRetry(tx, snapshotDetail.subjectId) + } const [row] = await tx .select({ status: outboxEvent.status, @@ -2120,7 +2126,9 @@ export async function retryEnterpriseFollowUpJob( !detail || !getEnterpriseFollowUpOperationIds(row.eventType, row.payload).includes(operationId) || (detail.kind === 'member_reconciliation' && - detail.subjectId !== operationPayload.request.organizationId) + detail.subjectId !== operationPayload.request.organizationId) || + detail.kind !== snapshotDetail.kind || + detail.subjectId !== snapshotDetail.subjectId ) { throw new EnterpriseProvisioningError('Enterprise follow-up job not found') } diff --git a/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts b/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts index 050402a2f2a..3a4249157d6 100644 --- a/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts +++ b/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts @@ -43,6 +43,7 @@ vi.mock('@sim/db', () => ({ vi.mock('@/lib/billing/stripe-client', () => stripeClientMock) vi.mock('@/lib/core/config/env-flags', () => envFlagsMock) +import { requestDashboardSubscriptionCancellation } from '@/lib/admin/subscription-lifecycle' import { createSimAuthAdapter } from '@/lib/auth/sim-auth-adapter' import { pauseProSubscriptionForOrgCoverage, @@ -132,6 +133,7 @@ beforeAll(async () => { 'organization', 'workspace', 'permissions', + 'audit_log', ]) { await connection.unsafe(`CREATE TABLE "${table}" (LIKE public."${table}" INCLUDING ALL)`) } @@ -283,6 +285,18 @@ async function storedSubscription(subscriptionId: string) { return row } +/** Resolves once another backend is blocked on a lock, i.e. the racing transaction is parked. */ +async function untilAnotherTransactionWaitsOnALock() { + for (let attempt = 0; attempt < 200; attempt++) { + const [row] = await connection<{ waiting: number }[]>` + select count(*)::int as waiting from pg_stat_activity + where datname = current_database() and wait_event_type = 'Lock'` + if (row.waiting > 0) return + await new Promise((resolve) => setImmediate(resolve)) + } + throw new Error('The racing transaction never waited on a lock') +} + describe('cancel_at_period_end sync', () => { it('pushes the latest value when an earlier sync lands in Stripe after a newer one', async () => { const pro = await createProUserInPaidOrganization() @@ -618,18 +632,6 @@ describe('cancel_at_period_end sync', () => { }) describe('Team activation', () => { - /** Resolves once another backend is blocked on a lock, i.e. the racing transaction is parked. */ - async function untilAnotherTransactionWaitsOnALock() { - for (let attempt = 0; attempt < 200; attempt++) { - const [row] = await connection<{ waiting: number }[]>` - select count(*)::int as waiting from pg_stat_activity - where datname = current_database() and wait_event_type = 'Lock'` - if (row.waiting > 0) return - await new Promise((resolve) => setImmediate(resolve)) - } - throw new Error('The racing transaction never waited on a lock') - } - it('records the cleared cancellation when a cancel is committed while it activates Team', async () => { const owner = await createUser('owner') const subscriptionId = generateId() @@ -688,6 +690,89 @@ describe('Team activation', () => { }) }) +describe('operator retry', () => { + it('requeues a dead-lettered sync while a writer commits a new value for the subscription', async () => { + const pro = await createProUserInPaidOrganization() + await pauseProSubscriptionForOrgCoverage(pro.userId) + const pauseSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + await deadLetter(pauseSync) + + let releaseWriter: () => void = () => {} + const writerHeld = new Promise((resolve) => { + releaseWriter = resolve + }) + const writing = testDatabase.transaction(async (tx) => { + await tx + .update(subscription) + .set({ cancelAtPeriodEnd: false }) + .where(eq(subscription.id, pro.subscriptionId)) + await writerHeld + await enqueueCancelAtPeriodEndSync(tx, { + stripeSubscriptionId: pro.stripeSubscriptionId, + subscriptionId: pro.subscriptionId, + cancelAtPeriodEnd: false, + reason: 'member-left-paid-org', + }) + }) + const requeuing = requeueFromAdminApi(pauseSync) + await untilAnotherTransactionWaitsOnALock() + releaseWriter() + + await expect(Promise.all([writing, requeuing])).resolves.toBeDefined() + await deliverUnrelatedUpdate(pro.stripeSubscriptionId) + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(false) + }) + + it('retries a dead-lettered dashboard cancellation while a writer commits a new value', async () => { + const org = await createOrganizationWithPlan('team') + const operationId = generateId() + const actor = { id: null, name: 'Admin', email: null } + await requestDashboardSubscriptionCancellation({ + organizationId: org.organizationId, + operationId, + timing: 'period_end', + actor, + }) + const cancelSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + org.subscriptionId + ) + await deadLetter(cancelSync) + + let releaseWriter: () => void = () => {} + const writerHeld = new Promise((resolve) => { + releaseWriter = resolve + }) + const writing = testDatabase.transaction(async (tx) => { + await tx + .update(subscription) + .set({ cancelAtPeriodEnd: false }) + .where(eq(subscription.id, org.subscriptionId)) + await writerHeld + await enqueueCancelAtPeriodEndSync(tx, { + stripeSubscriptionId: org.stripeSubscriptionId, + subscriptionId: org.subscriptionId, + cancelAtPeriodEnd: false, + reason: 'pro-to-team-conversion', + }) + }) + const retrying = requestDashboardSubscriptionCancellation({ + organizationId: org.organizationId, + operationId, + timing: 'period_end', + actor, + }) + await untilAnotherTransactionWaitsOnALock() + releaseWriter() + + await expect(Promise.all([writing, retrying])).resolves.toBeDefined() + expect((await storedSubscription(org.subscriptionId)).cancelAtPeriodEnd).toBe(true) + }) +}) + describe('customer contact sync', () => { it('pushes the current owner when an earlier sync lands in Stripe after a newer one', async () => { const [first, second, third] = await Promise.all([ diff --git a/apps/sim/lib/billing/webhooks/subscription-sync.ts b/apps/sim/lib/billing/webhooks/subscription-sync.ts index 8995791ab03..219e5526f8e 100644 --- a/apps/sim/lib/billing/webhooks/subscription-sync.ts +++ b/apps/sim/lib/billing/webhooks/subscription-sync.ts @@ -9,6 +9,7 @@ import { requireStripeClient } from '@/lib/billing/stripe-client' import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' import { enqueueOutboxEvent, + INFLIGHT_OUTBOX_STATUSES, listRetryableOutboxEvents, patchRetryableOutboxEvents, } from '@/lib/core/outbox/service' @@ -90,7 +91,8 @@ async function withCommittedAt( * onto every event of that type that can still run: pending, processing, or dead-lettered (each * operator retry path resets dead letters to pending). No event that can run again ever carries * an older value for the webhook reconcile to restore. The caller must hold the subscription row - * lock (`FOR UPDATE`, or the `UPDATE` itself). + * lock (`FOR UPDATE`, or the `UPDATE` itself), per the lock order on + * {@link lockSubscriptionForSyncRetry}. */ async function commitIntent( tx: DbOrTx, @@ -150,9 +152,32 @@ export async function recordCancelAtPeriodEnd( } /** - * Re-commits the subscription's current DB value onto its in-flight sync events, for a + * Takes the subscription row lock for an operator retry of one of its sync events; call it + * before touching the event, then {@link recommitSubscriptionSync} after resetting it. + * + * Lock order for every writer of a subscription's synced fields and their outbox events: + * organization mutation lock (where taken) → subscription row → outbox rows. Committing a value + * rewrites the subscription's retryable sync events, dead letters included, so a retry that + * locked a dead-lettered event before the subscription would deadlock against any concurrent + * writer. + */ +export async function lockSubscriptionForSyncRetry( + tx: DbOrTx, + subscriptionId: string +): Promise { + await tx + .select({ id: subscription.id }) + .from(subscription) + .where(eq(subscription.id, subscriptionId)) + .for('update') + .limit(1) +} + +/** + * Re-commits the subscription's current DB value onto its sync events that can still run, for a * dead-lettered event that was just reset to `pending`: the retry then carries the latest value - * rather than the one it failed with. Takes the subscription row lock itself. + * rather than the one it failed with. The caller holds the lock from + * {@link lockSubscriptionForSyncRetry}, taken before the reset. */ export async function recommitSubscriptionSync( tx: DbOrTx, @@ -163,7 +188,6 @@ export async function recommitSubscriptionSync( .select({ cancelAtPeriodEnd: subscription.cancelAtPeriodEnd, seats: subscription.seats }) .from(subscription) .where(eq(subscription.id, subscriptionId)) - .for('update') .limit(1) if (!current) return @@ -229,7 +253,7 @@ export function cancelAtPeriodEndSyncIdempotencyKey(eventId: string): string { */ type InflightIntent = { status: 'none' } | { status: 'legacy' } | { status: 'value'; value: T } -const INFLIGHT_STATUSES = new Set(['pending', 'processing']) +const INFLIGHT_STATUSES: ReadonlySet = new Set(INFLIGHT_OUTBOX_STATUSES) function latestIntent( events: { eventType: string; status: string; payload: unknown }[], diff --git a/apps/sim/lib/core/outbox/service.ts b/apps/sim/lib/core/outbox/service.ts index 09e9de357a2..2182db74187 100644 --- a/apps/sim/lib/core/outbox/service.ts +++ b/apps/sim/lib/core/outbox/service.ts @@ -407,7 +407,7 @@ export async function findDeadLetteredEvents( } /** Statuses of an event whose side effect may still run. */ -const INFLIGHT_OUTBOX_STATUSES = ['pending', 'processing'] as const +export const INFLIGHT_OUTBOX_STATUSES = ['pending', 'processing'] as const /** * Statuses an event can still run from: in flight, or dead-lettered, which every operator retry * path resets to `pending`. A `completed` event never runs again. diff --git a/packages/testing/src/mocks/billing-subscription-sync.mock.ts b/packages/testing/src/mocks/billing-subscription-sync.mock.ts index c1475f05578..47270c8b63b 100644 --- a/packages/testing/src/mocks/billing-subscription-sync.mock.ts +++ b/packages/testing/src/mocks/billing-subscription-sync.mock.ts @@ -17,6 +17,7 @@ import { vi } from 'vitest' */ export const billingSubscriptionSyncMockFns = { mockEnqueueCancelAtPeriodEndSync: vi.fn(async () => 'cancel-at-period-end-sync-event'), + mockLockSubscriptionForSyncRetry: vi.fn(async () => undefined), mockRecommitSubscriptionSync: vi.fn(async () => undefined), mockRecordCancelAtPeriodEnd: vi.fn(async () => undefined), mockIsSubscriptionSyncEventType: vi.fn( @@ -42,6 +43,7 @@ export const billingSubscriptionSyncMockFns = { */ export const billingSubscriptionSyncMock = { enqueueCancelAtPeriodEndSync: billingSubscriptionSyncMockFns.mockEnqueueCancelAtPeriodEndSync, + lockSubscriptionForSyncRetry: billingSubscriptionSyncMockFns.mockLockSubscriptionForSyncRetry, recommitSubscriptionSync: billingSubscriptionSyncMockFns.mockRecommitSubscriptionSync, recordCancelAtPeriodEnd: billingSubscriptionSyncMockFns.mockRecordCancelAtPeriodEnd, isSubscriptionSyncEventType: billingSubscriptionSyncMockFns.mockIsSubscriptionSyncEventType, diff --git a/packages/testing/src/mocks/outbox-service.mock.ts b/packages/testing/src/mocks/outbox-service.mock.ts index 543459c2218..09fa2708011 100644 --- a/packages/testing/src/mocks/outbox-service.mock.ts +++ b/packages/testing/src/mocks/outbox-service.mock.ts @@ -78,7 +78,7 @@ export const outboxServiceMockFns = { /** * Static mock module for `@/lib/core/outbox/service`. Covers every runtime export; - * `MAX_BULK_ENQUEUE_EVENTS` carries the real value. + * `MAX_BULK_ENQUEUE_EVENTS` and `INFLIGHT_OUTBOX_STATUSES` carry the real values. * * @example * ```ts @@ -87,6 +87,7 @@ export const outboxServiceMockFns = { */ export const outboxServiceMock = { MAX_BULK_ENQUEUE_EVENTS: 1_000, + INFLIGHT_OUTBOX_STATUSES: ['pending', 'processing'] as const, deferOutboxHandler: outboxServiceMockFns.mockDeferOutboxHandler, continueOutboxHandler: outboxServiceMockFns.mockContinueOutboxHandler, withOutboxHandlerTimeout: outboxServiceMockFns.mockWithOutboxHandlerTimeout, From 822dc1b94ee4f52c4213070de095c008a18e0a1c Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 7 Oct 2026 14:37:21 -0700 Subject: [PATCH 06/14] fix(billing): push each sync's recorded value and reject Stripe reads older than a newer Sim commit --- .../lib/admin/subscription-lifecycle.test.ts | 14 +- apps/sim/lib/auth/auth.ts | 12 +- .../pause-pro-for-coverage.test.ts | 18 +- .../organizations/provision-seat.test.ts | 24 +- .../lib/billing/organizations/seats.test.ts | 16 +- .../lib/billing/webhooks/outbox-handlers.ts | 37 +++- .../stripe-sync-convergence.integration.ts | 207 +++++++++++++++--- .../lib/billing/webhooks/subscription-sync.ts | 88 ++++++-- apps/sim/lib/core/outbox/service.ts | 10 + .../mocks/billing-subscription-sync.mock.ts | 7 +- .../testing/src/mocks/outbox-service.mock.ts | 2 + packages/testing/src/mocks/stripe.mock.ts | 48 ++-- 12 files changed, 320 insertions(+), 163 deletions(-) diff --git a/apps/sim/lib/admin/subscription-lifecycle.test.ts b/apps/sim/lib/admin/subscription-lifecycle.test.ts index f86d82a387e..dc990730e2c 100644 --- a/apps/sim/lib/admin/subscription-lifecycle.test.ts +++ b/apps/sim/lib/admin/subscription-lifecycle.test.ts @@ -2,10 +2,7 @@ import { outboxEvent, subscription } from '@sim/db/schema' import { dbChainMockFns, queueTableRows, resetDbChainMock } from '@sim/testing' import { auditMock, auditMockFns } from '@sim/testing/mocks/audit.mock' import { billingOutboxHandlersMock } from '@sim/testing/mocks/billing-outbox-handlers.mock' -import { - billingSubscriptionSyncMock, - billingSubscriptionSyncMockFns, -} from '@sim/testing/mocks/billing-subscription-sync.mock' +import { billingSubscriptionSyncMock } from '@sim/testing/mocks/billing-subscription-sync.mock' import { organizationMembershipMock } from '@sim/testing/mocks/organization-membership.mock' import { outboxServiceMock, outboxServiceMockFns } from '@sim/testing/mocks/outbox-service.mock' import { stripeClientMock } from '@sim/testing/mocks/stripe.mock' @@ -125,15 +122,6 @@ describe('admin subscription cancellation', () => { expect.objectContaining({ status: 'pending', attempts: 0, lastError: null }) ) expect(dbChainMockFns.set).toHaveBeenCalledWith({ cancelAtPeriodEnd: true }) - expect(billingSubscriptionSyncMockFns.mockLockSubscriptionForSyncRetry).toHaveBeenCalledWith( - expect.anything(), - 'sub-row-1' - ) - expect(billingSubscriptionSyncMockFns.mockRecommitSubscriptionSync).toHaveBeenCalledWith( - expect.anything(), - 'stripe.sync-cancel-at-period-end', - 'sub-row-1' - ) expect(result).toMatchObject({ operationId: '67e55044-10b1-426f-9247-bb680e5fe0c8', status: 'pending', diff --git a/apps/sim/lib/auth/auth.ts b/apps/sim/lib/auth/auth.ts index 35f31d9013a..4b317187d5a 100644 --- a/apps/sim/lib/auth/auth.ts +++ b/apps/sim/lib/auth/auth.ts @@ -104,8 +104,8 @@ import { handleSubscriptionDeleted, } from '@/lib/billing/webhooks/subscription' import { - commitCustomerRestoredSubscription, reconcileSubscriptionSyncFromStripe, + recordCustomerRestoreAfterHook, } from '@/lib/billing/webhooks/subscription-sync' import { handleSubscriptionUsageUpdate } from '@/lib/billing/webhooks/subscription-usage' import { env } from '@/lib/core/config/env' @@ -1104,15 +1104,7 @@ export const auth = betterAuth({ return }), after: createAuthMiddleware(async (ctx) => { - if (isBillingEnabled && ctx.path === '/subscription/restore') { - try { - await commitCustomerRestoredSubscription(ctx.context.returned) - } catch (error) { - logger.error('Failed to record a restored subscription as the committed value', { - error, - }) - } - } + if (isBillingEnabled) await recordCustomerRestoreAfterHook(ctx) if (isBillingEnabled && ctx.path === '/subscription/upgrade') { const checkoutContext = ctx as typeof ctx & { diff --git a/apps/sim/lib/billing/organizations/pause-pro-for-coverage.test.ts b/apps/sim/lib/billing/organizations/pause-pro-for-coverage.test.ts index 751881b2d4f..b12bb0e3688 100644 --- a/apps/sim/lib/billing/organizations/pause-pro-for-coverage.test.ts +++ b/apps/sim/lib/billing/organizations/pause-pro-for-coverage.test.ts @@ -1,8 +1,5 @@ import { dbChainMockFns, resetDbChainMock } from '@sim/testing' -import { - billingSubscriptionSyncMock, - billingSubscriptionSyncMockFns, -} from '@sim/testing/mocks/billing-subscription-sync.mock' +import { billingSubscriptionSyncMock } from '@sim/testing/mocks/billing-subscription-sync.mock' import { outboxServiceMock } from '@sim/testing/mocks/outbox-service.mock' import { beforeEach, describe, expect, it, vi } from 'vitest' @@ -16,9 +13,6 @@ vi.mock('@/lib/billing/webhooks/subscription-sync', () => billingSubscriptionSyn import { pauseProSubscriptionForOrgCoverage } from '@/lib/billing/organizations/membership' -const mockEnqueueCancelAtPeriodEndSync = - billingSubscriptionSyncMockFns.mockEnqueueCancelAtPeriodEndSync - const ACTIVE_PERSONAL_PRO = { id: 'sub-personal', plan: 'pro_6000', @@ -55,7 +49,7 @@ describe('pauseProSubscriptionForOrgCoverage', () => { resetDbChainMock() }) - it('pauses the personal Pro and queues the Stripe sync when an entitled paid org covers the user', async () => { + it('pauses the personal Pro when an entitled paid org covers the user', async () => { queueWhereResponses([ [{ organizationId: 'org-1' }], [{ plan: 'team_6000', referenceId: 'org-1' }], @@ -73,12 +67,6 @@ describe('pauseProSubscriptionForOrgCoverage', () => { organizationId: 'org-1', }) expect(dbChainMockFns.set).toHaveBeenCalledWith({ cancelAtPeriodEnd: true }) - expect(mockEnqueueCancelAtPeriodEndSync).toHaveBeenCalledWith(expect.anything(), { - stripeSubscriptionId: 'stripe-sub-personal', - subscriptionId: 'sub-personal', - cancelAtPeriodEnd: true, - reason: 'covered-by-organization', - }) }) it('reports covered even when no entitled personal Pro row exists', async () => { @@ -96,7 +84,6 @@ describe('pauseProSubscriptionForOrgCoverage', () => { organizationId: 'org-1', }) expect(dbChainMockFns.update).not.toHaveBeenCalled() - expect(mockEnqueueCancelAtPeriodEndSync).not.toHaveBeenCalled() }) it('reports covered without pausing again when the personal Pro is already pausing', async () => { @@ -115,6 +102,5 @@ describe('pauseProSubscriptionForOrgCoverage', () => { organizationId: 'org-1', }) expect(dbChainMockFns.update).not.toHaveBeenCalled() - expect(mockEnqueueCancelAtPeriodEndSync).not.toHaveBeenCalled() }) }) diff --git a/apps/sim/lib/billing/organizations/provision-seat.test.ts b/apps/sim/lib/billing/organizations/provision-seat.test.ts index cb42956831a..9be5b3e5eab 100644 --- a/apps/sim/lib/billing/organizations/provision-seat.test.ts +++ b/apps/sim/lib/billing/organizations/provision-seat.test.ts @@ -1,10 +1,7 @@ import { billingCoreMock, billingCoreMockFns } from '@sim/testing/mocks/billing-core.mock' import { billingOutboxHandlersMock } from '@sim/testing/mocks/billing-outbox-handlers.mock' import { billingPlanMock, billingPlanMockFns } from '@sim/testing/mocks/billing-plan.mock' -import { - billingSubscriptionSyncMock, - billingSubscriptionSyncMockFns, -} from '@sim/testing/mocks/billing-subscription-sync.mock' +import { billingSubscriptionSyncMock } from '@sim/testing/mocks/billing-subscription-sync.mock' import { dbChainMockFns, resetDbChainMock } from '@sim/testing/mocks/database.mock' import { organizationMembershipMock, @@ -55,8 +52,6 @@ const { mockAcquireOrganizationMutationLock } = organizationMembershipMockFns const mockGetOrganizationSubscription = billingCoreMockFns.mockGetOrganizationSubscription const mockGetHighestPriorityPersonalSubscription = billingPlanMockFns.mockGetHighestPriorityPersonalSubscription -const { mockEnqueueSubscriptionSeatsSync, mockEnqueueCancelAtPeriodEndSync } = - billingSubscriptionSyncMockFns /** The subscription row as the activation re-reads it under its lock. */ function testExecutor(onSubscriptionLock: () => void = () => {}) { @@ -130,11 +125,6 @@ describe('ensureTeamOrganizationForAcceptance', () => { }, }) expect(updateCalls.value).toContainEqual(expect.objectContaining({ plan: 'team_6000' })) - // The Pro→Team price migration is durably enqueued at conversion time. - expect(mockEnqueueSubscriptionSeatsSync).toHaveBeenCalledWith( - executor, - expect.objectContaining({ subscriptionId: 'sub-pro', seats: 1 }) - ) expect(mockGetOrganizationSubscription).toHaveBeenCalledWith( 'org-1', expect.objectContaining({ executor }) @@ -219,15 +209,8 @@ describe('ensureTeamOrganizationForAcceptance', () => { executor, expect.objectContaining({ plan: 'team_6000', referenceId: 'owner-1' }) ) - // The plan change enqueues the price seat-sync... - expect(mockEnqueueSubscriptionSeatsSync).toHaveBeenCalledWith( - executor, - expect.objectContaining({ subscriptionId: 'sub-pro', seats: 1 }) - ) expect(dbChainMockFns.transaction).not.toHaveBeenCalled() expect(lockOrder).toEqual(['organization', 'subscription']) - // ...but with no scheduled cancellation there is no cancel-sync event. - expect(mockEnqueueCancelAtPeriodEndSync).not.toHaveBeenCalled() }) it('blocks personal Pro conversion when the reused organization has unresolved Enterprise', async () => { @@ -255,8 +238,6 @@ describe('ensureTeamOrganizationForAcceptance', () => { }) ).rejects.toThrow('Enterprise issuance is unfinished') expect(updateCalls.value).toHaveLength(0) - expect(mockEnqueueSubscriptionSeatsSync).not.toHaveBeenCalled() - expect(mockEnqueueCancelAtPeriodEndSync).not.toHaveBeenCalled() }) it('provisions an org for a legacy personal-scoped Team subscription without a plan change', async () => { @@ -283,9 +264,6 @@ describe('ensureTeamOrganizationForAcceptance', () => { expect.anything(), expect.objectContaining({ plan: 'team', referenceId: 'owner-1' }) ) - // No plan change and no scheduled cancellation: nothing to push to Stripe. - expect(mockEnqueueSubscriptionSeatsSync).not.toHaveBeenCalled() - expect(mockEnqueueCancelAtPeriodEndSync).not.toHaveBeenCalled() }) it('returns upgrade-required (no downgrade) when no eligible Team tier exists', async () => { diff --git a/apps/sim/lib/billing/organizations/seats.test.ts b/apps/sim/lib/billing/organizations/seats.test.ts index 4fda5643dde..51756140367 100644 --- a/apps/sim/lib/billing/organizations/seats.test.ts +++ b/apps/sim/lib/billing/organizations/seats.test.ts @@ -8,10 +8,7 @@ import { setEnvFlags, } from '@sim/testing' import { billingOutboxHandlersMock } from '@sim/testing/mocks/billing-outbox-handlers.mock' -import { - billingSubscriptionSyncMock, - billingSubscriptionSyncMockFns, -} from '@sim/testing/mocks/billing-subscription-sync.mock' +import { billingSubscriptionSyncMock } from '@sim/testing/mocks/billing-subscription-sync.mock' import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' const { mockSyncSubscriptionUsageLimits } = vi.hoisted(() => ({ @@ -30,8 +27,6 @@ vi.mock('@sim/audit', () => auditMock) import { reconcileOrganizationSeats } from '@/lib/billing/organizations/seats' -const enqueueMock = billingSubscriptionSyncMockFns.mockEnqueueSubscriptionSeatsSync - const teamSub = { id: 'sub-1', plan: 'team_6000', @@ -58,7 +53,7 @@ describe('reconcileOrganizationSeats', () => { resetDbChainMock() }) - it('grows seats to the member count and enqueues a Stripe sync', async () => { + it('grows seats to the member count', async () => { queueReconcileReads([teamSub], [{ value: 2 }]) const result = await reconcileOrganizationSeats({ @@ -74,11 +69,6 @@ describe('reconcileOrganizationSeats', () => { outboxEventId: 'subscription-seats-sync-event', }) expect(dbChainMockFns.set).toHaveBeenCalledWith({ seats: 2 }) - expect(enqueueMock).toHaveBeenCalledWith(expect.anything(), { - subscriptionId: 'sub-1', - seats: 2, - reason: 'member-accepted-invite', - }) expect(mockSyncSubscriptionUsageLimits).toHaveBeenCalledWith( expect.objectContaining({ id: 'sub-1', referenceId: 'org-1', seats: 2 }) ) @@ -94,7 +84,6 @@ describe('reconcileOrganizationSeats', () => { expect(result.changed).toBe(true) expect(dbChainMockFns.set).toHaveBeenCalledWith({ seats: 2 }) - expect(enqueueMock).toHaveBeenCalledOnce() }) it('still records the seat audit when the post-commit usage-limit sync fails', async () => { @@ -129,7 +118,6 @@ describe('reconcileOrganizationSeats', () => { expect(result.changed).toBe(true) expect(result.seats).toBe(2) expect(dbChainMockFns.set).toHaveBeenCalledWith({ seats: 2 }) - expect(enqueueMock).toHaveBeenCalled() }) it('never drops below one seat', async () => { diff --git a/apps/sim/lib/billing/webhooks/outbox-handlers.ts b/apps/sim/lib/billing/webhooks/outbox-handlers.ts index de6ad594620..0be81303a80 100644 --- a/apps/sim/lib/billing/webhooks/outbox-handlers.ts +++ b/apps/sim/lib/billing/webhooks/outbox-handlers.ts @@ -14,6 +14,7 @@ import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' import { type CancelAtPeriodEndSyncPayload, cancelAtPeriodEndSyncIdempotencyKey, + readRecordedSyncValue, type SubscriptionSeatsSyncPayload, } from '@/lib/billing/webhooks/subscription-sync' import type { OutboxHandler } from '@/lib/core/outbox/service' @@ -100,20 +101,32 @@ async function getSubscriptionSeatSyncState(subscriptionId: string) { return row ?? null } -async function readCancelAtPeriodEnd(subscriptionId: string): Promise { +/** + * The value this sync should push: the one recorded on its own event, re-read now, which every + * commit keeps current on each sync that can still run. The subscription row is not used for + * this, because the Stripe plugin can overwrite it with a stale webhook payload before the + * reconcile step restores it. An event enqueued before values were recorded falls back to the + * row. Null when the subscription no longer exists. + */ +async function readDesiredCancelAtPeriodEnd( + eventId: string, + subscriptionId: string +): Promise { const [row] = await db .select({ cancelAtPeriodEnd: subscriptionTable.cancelAtPeriodEnd }) .from(subscriptionTable) .where(eq(subscriptionTable.id, subscriptionId)) .limit(1) - return row ? Boolean(row.cancelAtPeriodEnd) : null + if (!row) return null + return (await readRecordedSyncValue(eventId))?.cancelAtPeriodEnd ?? Boolean(row.cancelAtPeriodEnd) } /** - * Pushes the row's current value, never the payload's: racing events for one subscription each - * converge on the last committed value. Stripe is read first and written only when it differs, - * and the row is re-read after the write so a value committed while this event's request was in - * flight is pushed too, even when an earlier event's request lands in Stripe after a newer one. + * Pushes the latest committed value (see `readDesiredCancelAtPeriodEnd`), never the claim-time + * payload: racing events for one subscription each converge on it. Stripe is read first and + * written only when it differs, and the value is re-read after the write so one committed while + * this event's request was in flight is pushed too, even when an earlier event's request lands + * in Stripe after a newer one. */ const stripeSyncCancelAtPeriodEnd: OutboxHandler = async ( payload, @@ -123,7 +136,7 @@ const stripeSyncCancelAtPeriodEnd: OutboxHandler = const stripe = requireStripeClient() for (let attempt = 1; attempt <= MAX_SYNC_ATTEMPTS; attempt++) { - const desiredValue = await readCancelAtPeriodEnd(payload.subscriptionId) + const desiredValue = await readDesiredCancelAtPeriodEnd(ctx.eventId, payload.subscriptionId) if (desiredValue === null) { logger.warn('Subscription not found when syncing cancel_at_period_end', { eventId: ctx.eventId, @@ -142,7 +155,7 @@ const stripeSyncCancelAtPeriodEnd: OutboxHandler = ) } - const latestValue = await readCancelAtPeriodEnd(payload.subscriptionId) + const latestValue = await readDesiredCancelAtPeriodEnd(ctx.eventId, payload.subscriptionId) if (latestValue !== desiredValue) { logger.info('cancel_at_period_end changed during Stripe sync; retrying latest value', { eventId: ctx.eventId, @@ -189,6 +202,10 @@ const stripeCancelSubscriptionImmediately: OutboxHandler< }) } +/** + * Pushes the seat count recorded on its own event, re-read each pass (falling back to the row + * for an event enqueued before values were recorded), for the same reason as the cancel sync. + */ const stripeSyncSubscriptionSeats: OutboxHandler = async ( payload, ctx @@ -231,7 +248,7 @@ const stripeSyncSubscriptionSeats: OutboxHandler = return } - const desiredSeats = row.seats || 1 + const desiredSeats = (await readRecordedSyncValue(ctx.eventId))?.seats ?? (row.seats || 1) const stripeSubscription = await stripe.subscriptions.retrieve(row.stripeSubscriptionId) if (!hasPaidSubscriptionStatus(stripeSubscription.status)) { @@ -281,7 +298,7 @@ const stripeSyncSubscriptionSeats: OutboxHandler = } const latest = await getSubscriptionSeatSyncState(payload.subscriptionId) - const latestSeats = latest?.seats || 1 + const latestSeats = (await readRecordedSyncValue(ctx.eventId))?.seats ?? (latest?.seats || 1) if (latestSeats !== desiredSeats) { logger.info('Subscription seats changed during Stripe sync; retrying latest value', { eventId: ctx.eventId, diff --git a/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts b/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts index 3a4249157d6..57a556299b8 100644 --- a/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts +++ b/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts @@ -17,6 +17,7 @@ import { } from '@sim/testing/mocks/stripe.mock' import { generateId } from '@sim/utils/id' import { type BetterAuthOptions, betterAuth } from 'better-auth' +import { createAuthMiddleware } from 'better-auth/api' import { and, desc, eq, sql } from 'drizzle-orm' import { drizzle, type PostgresJsDatabase } from 'drizzle-orm/postgres-js' import { NextRequest } from 'next/server' @@ -56,9 +57,9 @@ import { syncSeatsFromStripeQuantity } from '@/lib/billing/validation/seat-manag import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' import { billingOutboxHandlers } from '@/lib/billing/webhooks/outbox-handlers' import { - commitCustomerRestoredSubscription, enqueueCancelAtPeriodEndSync, reconcileSubscriptionSyncFromStripe, + recordCustomerRestoreAfterHook, } from '@/lib/billing/webhooks/subscription-sync' import { enqueueOutboxEvent, processOutboxEventById } from '@/lib/core/outbox/service' import { POST as requeueOutboxEvent } from '@/app/api/v1/admin/outbox/[id]/requeue/route' @@ -78,15 +79,21 @@ const testDatabase = drizzle(connection, { schema }) let stripe: InMemoryStripe +/** Runs between the plugin's write and Sim's reconcile step, to place work in that window. */ +let beforeReconcile: (() => Promise) | undefined + /** - * Sim's Stripe plugin callbacks from `lib/auth/auth.ts`, reduced to the parts that touch the - * synced fields: the seat sync in `onSubscriptionUpdate` and the reconcile step in `onEvent`. + * Sim's Better Auth Stripe wiring from `lib/auth/auth.ts`, reduced to the parts that touch the + * synced fields: the seat sync in `onSubscriptionUpdate`, the reconcile step in `onEvent`, and + * the restore step in the `after` hook. */ -function createWebhookEndpoint() { - const auth = betterAuth({ +function createTestAuth() { + return betterAuth({ baseURL: 'http://localhost:3000', secret: 'isolated-integration-fixture-secret-not-a-real-credential', database: (options: BetterAuthOptions) => createSimAuthAdapter(options, testDatabase), + emailAndPassword: { enabled: true }, + hooks: { after: createAuthMiddleware(recordCustomerRestoreAfterHook) }, plugins: [ stripePlugin({ stripeClient: stripe.client, @@ -104,24 +111,27 @@ function createWebhookEndpoint() { ) }, }, - onEvent: reconcileSubscriptionSyncFromStripe, + onEvent: async (event) => { + await beforeReconcile?.() + await reconcileSubscriptionSyncFromStripe(event) + }, }), ], }) - - return async function deliver(event: Stripe.Event) { - const response = await auth.handler( - new Request('http://localhost:3000/api/auth/stripe/webhook', { - method: 'POST', - headers: { 'content-type': 'application/json', 'stripe-signature': 't=1,v1=fixture' }, - body: JSON.stringify(event), - }) - ) - expect(response.status).toBe(200) - } } -let deliver: ReturnType +let auth: ReturnType + +async function deliver(event: Stripe.Event) { + const response = await auth.handler( + new Request('http://localhost:3000/api/auth/stripe/webhook', { + method: 'POST', + headers: { 'content-type': 'application/json', 'stripe-signature': 't=1,v1=fixture' }, + body: JSON.stringify(event), + }) + ) + expect(response.status).toBe(200) +} beforeAll(async () => { await connection`CREATE SCHEMA ${connection(schemaName)}` @@ -134,6 +144,9 @@ beforeAll(async () => { 'workspace', 'permissions', 'audit_log', + 'session', + 'account', + 'verification', ]) { await connection.unsafe(`CREATE TABLE "${table}" (LIKE public."${table}" INCLUDING ALL)`) } @@ -143,7 +156,8 @@ beforeAll(async () => { beforeEach(() => { stripe = createInMemoryStripe() stripeClientMock.requireStripeClient.mockReturnValue(stripe.client) - deliver = createWebhookEndpoint() + beforeReconcile = undefined + auth = createTestAuth() }) afterAll(async () => { @@ -199,8 +213,8 @@ async function addMember(organizationId: string, userId: string, role = 'member' } /** A user on personal Pro, synced with Stripe, who belongs to a paid Team organization. */ -async function createProUserInPaidOrganization() { - const proUser = await createUser('pro') +async function createProUserInPaidOrganization(existingUserId?: string) { + const proUser = existingUserId ? { id: existingUserId } : await createUser('pro') const subscriptionId = generateId() const stripeSubscriptionId = `sub_${subscriptionId}` await testDatabase.insert(subscription).values({ @@ -276,6 +290,45 @@ async function deliverUnrelatedUpdate(stripeSubscriptionId: string) { await deliver(stripe.events.at(-1) as Stripe.Event) } +async function signUp(label: string) { + const email = `${label}-${generateId()}@example.com` + const response = await auth.api.signUpEmail({ + body: { email, password: 'integration-fixture-password', name: label }, + asResponse: true, + }) + expect(response.status).toBe(200) + const { user: created } = (await response.json()) as { user: { id: string } } + const cookie = response.headers + .getSetCookie() + .map((header) => header.split(';')[0]) + .join('; ') + return { userId: created.id, cookie } +} + +function restoreSubscription(cookie: string) { + return auth.handler( + new Request('http://localhost:3000/api/auth/subscription/restore', { + method: 'POST', + headers: { 'content-type': 'application/json', cookie, origin: 'http://localhost:3000' }, + body: '{}', + }) + ) +} + +async function cancelValuesOfRetryableSyncs(subscriptionId: string) { + const rows = await testDatabase + .select({ payload: outboxEvent.payload }) + .from(outboxEvent) + .where( + and( + eq(outboxEvent.eventType, OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END), + sql`${outboxEvent.status} in ('pending', 'processing', 'dead_letter')`, + sql`${outboxEvent.payload} ->> 'subscriptionId' = ${subscriptionId}` + ) + ) + return rows.map((row) => (row.payload as { cancelAtPeriodEnd?: boolean }).cancelAtPeriodEnd) +} + async function storedSubscription(subscriptionId: string) { const [row] = await testDatabase .select({ cancelAtPeriodEnd: subscription.cancelAtPeriodEnd, seats: subscription.seats }) @@ -306,7 +359,7 @@ describe('cancel_at_period_end sync', () => { pro.subscriptionId ) - const gate = stripe.holdNextUpdate('subscriptions') + const gate = stripe.holdNextRequest('subscriptions.update') const pausing = processEvent(pauseSync) await gate.reached @@ -336,13 +389,13 @@ describe('cancel_at_period_end sync', () => { pro.subscriptionId ) - const stalePush = stripe.holdNextUpdate('subscriptions') + const stalePush = stripe.holdNextRequest('subscriptions.update') const pausing = processEvent(pauseSync) await stalePush.reached await leaveOrganization(pro.userId, pro.paidOrganization.organizationId) await restoreUserProSubscription(pro.userId) - const correctingPush = stripe.holdNextUpdate('subscriptions') + const correctingPush = stripe.holdNextRequest('subscriptions.update') stalePush.release() await correctingPush.reached expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(true) @@ -473,7 +526,7 @@ describe('cancel_at_period_end sync', () => { OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, pro.subscriptionId ) - const slowPush = stripe.holdNextUpdate('subscriptions') + const slowPush = stripe.holdNextRequest('subscriptions.update') const pausing = processEvent(pauseSync) await slowPush.reached @@ -547,6 +600,61 @@ describe('cancel_at_period_end sync', () => { expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(true) }) + it('pushes the committed value when its sync runs between the plugin write and the reconcile', async () => { + const pro = await createProUserInPaidOrganization() + await pauseProSubscriptionForOrgCoverage(pro.userId) + const pauseSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + + beforeReconcile = async () => { + beforeReconcile = undefined + await expect(processEvent(pauseSync)).resolves.toBe('completed') + } + await deliverUnrelatedUpdate(pro.stripeSubscriptionId) + + expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(true) + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(true) + }) + + it('keeps a value Sim committed after the reconcile read Stripe', async () => { + const pro = await createProUserInPaidOrganization() + await pauseProSubscriptionForOrgCoverage(pro.userId) + const pauseSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + await expect(processEvent(pauseSync)).resolves.toBe('completed') + + stripe.updateOutsideSim(pro.stripeSubscriptionId, { cancel_at_period_end: false }) + const liveRead = stripe.holdNextRequest('subscriptions.retrieve') + const delivering = deliver(stripe.events.at(-1) as Stripe.Event) + await liveRead.reached + await testDatabase.transaction(async (tx) => { + await tx + .update(subscription) + .set({ cancelAtPeriodEnd: true }) + .where(eq(subscription.id, pro.subscriptionId)) + await enqueueCancelAtPeriodEndSync(tx, { + stripeSubscriptionId: pro.stripeSubscriptionId, + subscriptionId: pro.subscriptionId, + cancelAtPeriodEnd: true, + reason: 'admin-cancel-at-period-end', + }) + }) + liveRead.release() + await delivering + + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(true) + const adminSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + await expect(processEvent(adminSync)).resolves.toBe('completed') + expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(true) + }) + it('does not revive an older value when a retry path resets its sync without re-committing', async () => { const pro = await createProUserInPaidOrganization() await pauseProSubscriptionForOrgCoverage(pro.userId) @@ -601,8 +709,9 @@ describe('cancel_at_period_end sync', () => { expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(false) }) - it("keeps a customer's restore made while an earlier sync is retrying", async () => { - const pro = await createProUserInPaidOrganization() + it("records a customer's restore through the real endpoint while an earlier sync is retrying", async () => { + const customer = await signUp('restorer') + const pro = await createProUserInPaidOrganization(customer.userId) await pauseProSubscriptionForOrgCoverage(pro.userId) const pauseSync = await latestOutboxEventId( OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, @@ -611,15 +720,12 @@ describe('cancel_at_period_end sync', () => { stripe.failNextUpdateAfterApplying('subscriptions') await expect(processEvent(pauseSync)).resolves.toBe('pending') - const restored = await stripe.client.subscriptions.update(pro.stripeSubscriptionId, { - cancel_at_period_end: false, - }) + expect((await restoreSubscription(customer.cookie)).status).toBe(200) const restoreEvent = stripe.events.at(-1) as Stripe.Event - await testDatabase - .update(subscription) - .set({ cancelAtPeriodEnd: false, cancelAt: null, canceledAt: null }) - .where(eq(subscription.id, pro.subscriptionId)) - await commitCustomerRestoredSubscription(restored) + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(false) + expect(new Set(await cancelValuesOfRetryableSyncs(pro.subscriptionId))).toEqual( + new Set([false]) + ) await deliverUnrelatedUpdate(pro.stripeSubscriptionId) await makeDue(pauseSync) @@ -629,6 +735,15 @@ describe('cancel_at_period_end sync', () => { expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(false) expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(false) }) + + it('records nothing when the restore endpoint refuses the request', async () => { + const customer = await signUp('not-cancelling') + const pro = await createProUserInPaidOrganization(customer.userId) + + expect((await restoreSubscription(customer.cookie)).status).toBe(400) + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(false) + expect(await cancelValuesOfRetryableSyncs(pro.subscriptionId)).toEqual([]) + }) }) describe('Team activation', () => { @@ -802,7 +917,7 @@ describe('customer contact sync', () => { } const firstSync = await transferOwnership(first.id, second.id) - const gate = stripe.holdNextUpdate('customers') + const gate = stripe.holdNextRequest('customers.update') const firstSyncRun = processEvent(firstSync) await gate.reached @@ -840,6 +955,26 @@ describe('Team seat sync', () => { await expect(processEvent(seatSync)).resolves.toBe('completed') expect(stripe.subscription(org.stripeSubscriptionId).items.data[0].quantity).toBe(2) }) + it('pushes the committed seats when its sync runs between the plugin write and the reconcile', async () => { + const [owner, joiner] = await Promise.all([createUser('owner'), createUser('joiner')]) + const org = await createOrganizationWithPlan('team', 1) + await addMember(org.organizationId, owner.id, 'owner') + await addMember(org.organizationId, joiner.id) + await reconcileOrganizationSeats({ organizationId: org.organizationId, reason: 'member-added' }) + const seatSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_SUBSCRIPTION_SEATS, + org.subscriptionId + ) + + beforeReconcile = async () => { + beforeReconcile = undefined + await expect(processEvent(seatSync)).resolves.toBe('completed') + } + await deliverUnrelatedUpdate(org.stripeSubscriptionId) + + expect(stripe.subscription(org.stripeSubscriptionId).items.data[0].quantity).toBe(2) + }) + it('does not revive an older seat count when its dead-lettered sync is requeued', async () => { const [owner, joiner] = await Promise.all([createUser('owner'), createUser('joiner')]) const org = await createOrganizationWithPlan('team', 1) diff --git a/apps/sim/lib/billing/webhooks/subscription-sync.ts b/apps/sim/lib/billing/webhooks/subscription-sync.ts index 219e5526f8e..2ae2d830eb4 100644 --- a/apps/sim/lib/billing/webhooks/subscription-sync.ts +++ b/apps/sim/lib/billing/webhooks/subscription-sync.ts @@ -12,6 +12,7 @@ import { INFLIGHT_OUTBOX_STATUSES, listRetryableOutboxEvents, patchRetryableOutboxEvents, + readOutboxEventPayload, } from '@/lib/core/outbox/service' import type { DbOrTx } from '@/lib/db/types' @@ -79,11 +80,16 @@ async function withCommittedAt( tx: DbOrTx, fields: T ): Promise { - const [row] = await tx.execute<{ committedAt: string }>( - sql`select (extract(epoch from clock_timestamp()) * 1000000)::bigint::text as "committedAt"` + return { ...fields, committedAt: await readDatabaseClock(tx) } +} + +/** The clock `committedAt` is stamped from, in microseconds since the epoch. */ +async function readDatabaseClock(executor: DbOrTx): Promise { + const [row] = await executor.execute<{ now: string }>( + sql`select (extract(epoch from clock_timestamp()) * 1000000)::bigint::text as "now"` ) if (!row) throw new Error('Database clock read returned no row') - return { ...fields, committedAt: Number(row.committedAt) } + return Number(row.now) } /** @@ -206,10 +212,9 @@ export async function recommitSubscriptionSync( * committed `cancelAtPeriodEnd`. That endpoint updates Stripe and then writes the row directly, * so a cancel sync still in flight would otherwise carry the cancellation the customer just * undid, and a webhook processed before the restore's own could restore it for that sync to - * push. Enqueuing re-pushes `false` even if such a sync already ran. Called from the endpoint's - * `after` hook with the Stripe subscription it returned. + * push. Enqueuing re-pushes `false` even if such a sync already ran. */ -export async function commitCustomerRestoredSubscription(restored: unknown): Promise { +async function commitCustomerRestoredSubscription(restored: unknown): Promise { const stripeSubscription = toRecord(restored) const stripeSubscriptionId = stripeSubscription.id if ( @@ -241,6 +246,41 @@ export async function commitCustomerRestoredSubscription(restored: unknown): Pro }) } +/** + * The Better Auth `after` hook step for `/subscription/restore`: records the restored value from + * the Stripe subscription the endpoint returned. A failure is logged rather than failing a + * restore that already reached Stripe and the row; the reconcile step then still treats the + * restore's own webhook as a change made in Stripe. + */ +export async function recordCustomerRestoreAfterHook(ctx: { + path: string + context: { returned?: unknown } +}): Promise { + if (ctx.path !== '/subscription/restore') return + try { + await commitCustomerRestoredSubscription(ctx.context.returned) + } catch (error) { + logger.error('Failed to record a restored subscription as the committed value', { error }) + } +} + +/** + * The value recorded on a sync event as of now, not as of its claim: every commit rewrites it on + * each sync that can still run. Undefined for an event enqueued before values were recorded. + */ +export async function readRecordedSyncValue( + eventId: string +): Promise<{ cancelAtPeriodEnd?: boolean; seats?: number } | undefined> { + const payload = toRecord(await readOutboxEventPayload(eventId)) + if (typeof payload.committedAt !== 'number') return undefined + return { + ...(typeof payload.cancelAtPeriodEnd === 'boolean' + ? { cancelAtPeriodEnd: payload.cancelAtPeriodEnd } + : {}), + ...(typeof payload.seats === 'number' ? { seats: payload.seats } : {}), + } +} + /** A fresh key per Stripe write: the SDK reuses it across its own network retries of that call. */ export function cancelAtPeriodEndSyncIdempotencyKey(eventId: string): string { return `${CANCEL_AT_PERIOD_END_SYNC_KEY_PREFIX}${eventId}:${generateShortId()}` @@ -251,7 +291,10 @@ export function cancelAtPeriodEndSyncIdempotencyKey(eventId: string): string { * committed value, or `legacy` when an event predates recorded values (enqueued by an older * deploy) so the committed value is unknown. */ -type InflightIntent = { status: 'none' } | { status: 'legacy' } | { status: 'value'; value: T } +type InflightIntent = + | { status: 'none' } + | { status: 'legacy' } + | { status: 'value'; value: T; committedAt: number } const INFLIGHT_STATUSES: ReadonlySet = new Set(INFLIGHT_OUTBOX_STATUSES) @@ -270,7 +313,7 @@ function latestIntent( latest = { committedAt: payload.committedAt, value } } } - return latest ? { status: 'value', value: latest.value } : { status: 'none' } + return latest ? { status: 'value', ...latest } : { status: 'none' } } /** One indexed read of the subscription's sync events that can still run. */ @@ -312,12 +355,21 @@ type CancelAtPeriodEndSource = | { source: 'pending-sync'; value: boolean } | { source: 'stripe' } +/** + * A Stripe-side change wins over a pending value unless the pending value was committed after + * Stripe was read (`liveReadAt`, on the same database clock): that read predates Sim's newer + * commit, so applying it would roll the newer value back. + */ function cancelAtPeriodEndSource( intent: InflightIntent, - changedInStripe: boolean + changedInStripe: boolean, + liveReadAt?: number ): CancelAtPeriodEndSource { if (intent.status === 'legacy') return { source: 'unchanged' } - if (intent.status === 'value' && !changedInStripe) { + if ( + intent.status === 'value' && + (!changedInStripe || (liveReadAt !== undefined && intent.committedAt > liveReadAt)) + ) { return { source: 'pending-sync', value: intent.value } } return { source: 'stripe' } @@ -333,14 +385,18 @@ function cancelAtPeriodEndSource( * Decided under the subscription row lock that every committing writer holds: * - `cancelAtPeriodEnd`: while a cancel sync is in flight, its committed value wins over * snapshots and over echoes of Sim's own writes. A change made in Stripe itself (customer - * portal, dashboard, Better Auth's cancel/restore endpoints) is newer and wins, and is - * committed onto every sync that can still run so none can later restore the value it replaced. With - * no sync in flight Stripe wins, read live so out-of-order delivery cannot regress it. + * portal, dashboard, Better Auth's cancel/restore endpoints) wins and is committed onto every + * sync that can still run, unless Sim committed a newer value after Stripe was read. With no + * sync in flight Stripe wins, read live so out-of-order delivery cannot regress it. + * - Precedence across the two systems is arrival order, not wall-clock order: a Stripe-side + * change whose webhook is processed after a Sim commit wins even if the customer made it + * earlier. Stripe's `event.created` is not compared with the database clock, because skew + * between them could override a genuinely newer customer action. * - `seats`: Team seats are Sim-owned; while a seat sync is in flight its committed value wins. * - A field with an in-flight event from an older deploy is left as the plugin wrote it. * * Stripe is read only when its value decides, and never under the lock. Only the DB row and - * in-flight payloads are written, so this cannot trigger another webhook. + * sync payloads are written, so this cannot trigger another webhook. */ export async function reconcileSubscriptionSyncFromStripe(event: Stripe.Event): Promise { if (event.type !== 'customer.subscription.updated') return @@ -355,6 +411,7 @@ export async function reconcileSubscriptionSyncFromStripe(event: Stripe.Event): const changedInStripe = isCancellationChangedInStripe(event) let liveCancelAtPeriodEnd: boolean | undefined + let liveReadAt: number | undefined for (let pass = 1; pass <= 2; pass++) { if (liveCancelAtPeriodEnd === undefined) { @@ -365,6 +422,7 @@ export async function reconcileSubscriptionSyncFromStripe(event: Stripe.Event): changedInStripe ).source === 'stripe' if (needsStripe) { + liveReadAt = await readDatabaseClock(db) const live = await requireStripeClient().subscriptions.retrieve(stripeSubscriptionId) liveCancelAtPeriodEnd = Boolean(live.cancel_at_period_end) } @@ -380,7 +438,7 @@ export async function reconcileSubscriptionSyncFromStripe(event: Stripe.Event): if (!current) return true const intents = await readSyncIntents(tx, row.id) - const cancel = cancelAtPeriodEndSource(intents.cancelAtPeriodEnd, changedInStripe) + const cancel = cancelAtPeriodEndSource(intents.cancelAtPeriodEnd, changedInStripe, liveReadAt) let cancelAtPeriodEnd = Boolean(current.cancelAtPeriodEnd) if (cancel.source === 'pending-sync') { cancelAtPeriodEnd = cancel.value diff --git a/apps/sim/lib/core/outbox/service.ts b/apps/sim/lib/core/outbox/service.ts index 2182db74187..78650a2b710 100644 --- a/apps/sim/lib/core/outbox/service.ts +++ b/apps/sim/lib/core/outbox/service.ts @@ -406,6 +406,16 @@ export async function findDeadLetteredEvents( .limit(DEAD_LETTER_SCAN_LIMIT) } +/** The event's current payload, read fresh rather than from the copy its handler was claimed with. */ +export async function readOutboxEventPayload(eventId: string): Promise { + const [row] = await db + .select({ payload: outboxEvent.payload }) + .from(outboxEvent) + .where(eq(outboxEvent.id, eventId)) + .limit(1) + return row?.payload +} + /** Statuses of an event whose side effect may still run. */ export const INFLIGHT_OUTBOX_STATUSES = ['pending', 'processing'] as const /** diff --git a/packages/testing/src/mocks/billing-subscription-sync.mock.ts b/packages/testing/src/mocks/billing-subscription-sync.mock.ts index 47270c8b63b..52b958a31f2 100644 --- a/packages/testing/src/mocks/billing-subscription-sync.mock.ts +++ b/packages/testing/src/mocks/billing-subscription-sync.mock.ts @@ -30,7 +30,8 @@ export const billingSubscriptionSyncMockFns = { (eventId: string) => `outbox-sync-cancel-at-period-end:${eventId}:key` ), mockReconcileSubscriptionSyncFromStripe: vi.fn(async () => undefined), - mockCommitCustomerRestoredSubscription: vi.fn(async () => undefined), + mockRecordCustomerRestoreAfterHook: vi.fn(async () => undefined), + mockReadRecordedSyncValue: vi.fn(async () => undefined), } /** @@ -52,6 +53,6 @@ export const billingSubscriptionSyncMock = { billingSubscriptionSyncMockFns.mockCancelAtPeriodEndSyncIdempotencyKey, reconcileSubscriptionSyncFromStripe: billingSubscriptionSyncMockFns.mockReconcileSubscriptionSyncFromStripe, - commitCustomerRestoredSubscription: - billingSubscriptionSyncMockFns.mockCommitCustomerRestoredSubscription, + recordCustomerRestoreAfterHook: billingSubscriptionSyncMockFns.mockRecordCustomerRestoreAfterHook, + readRecordedSyncValue: billingSubscriptionSyncMockFns.mockReadRecordedSyncValue, } diff --git a/packages/testing/src/mocks/outbox-service.mock.ts b/packages/testing/src/mocks/outbox-service.mock.ts index 09fa2708011..4875502f550 100644 --- a/packages/testing/src/mocks/outbox-service.mock.ts +++ b/packages/testing/src/mocks/outbox-service.mock.ts @@ -69,6 +69,7 @@ export const outboxServiceMockFns = { }), mockFindDeadLetteredEvents: vi.fn(), mockListRetryableOutboxEvents: vi.fn(), + mockReadOutboxEventPayload: vi.fn(), mockPatchRetryableOutboxEvents: vi.fn(), mockHasInflightOutboxEvent: vi.fn(), mockHasDueOutboxWork: vi.fn(), @@ -101,6 +102,7 @@ export const outboxServiceMock = { outboxPayloadHasSourceOperationId: outboxServiceMockFns.mockOutboxPayloadHasSourceOperationId, findDeadLetteredEvents: outboxServiceMockFns.mockFindDeadLetteredEvents, listRetryableOutboxEvents: outboxServiceMockFns.mockListRetryableOutboxEvents, + readOutboxEventPayload: outboxServiceMockFns.mockReadOutboxEventPayload, patchRetryableOutboxEvents: outboxServiceMockFns.mockPatchRetryableOutboxEvents, hasInflightOutboxEvent: outboxServiceMockFns.mockHasInflightOutboxEvent, hasDueOutboxWork: outboxServiceMockFns.mockHasDueOutboxWork, diff --git a/packages/testing/src/mocks/stripe.mock.ts b/packages/testing/src/mocks/stripe.mock.ts index 5fce08c41d9..85bef873e2f 100644 --- a/packages/testing/src/mocks/stripe.mock.ts +++ b/packages/testing/src/mocks/stripe.mock.ts @@ -117,25 +117,22 @@ interface CustomerUpdateParams { * `retrieve` returns current state, `update` applies params and emits the * `customer.subscription.updated` event Stripe would send (with `previous_attributes` and the * originating `request.idempotency_key`), and a reused idempotency key replays the first - * response, or rejects when its parameters differ. A test can park the next update on a gate to - * control the order requests land in, or make the next update apply and then fail on the + * response, or rejects when its parameters differ. A test can park the next request on a gate to + * control the order requests land in (a parked retrieve answers with the state it arrived to), or make the next update apply and then fail on the * client, as a dropped connection does. * * @example * ```ts * const stripe = createInMemoryStripe() * stripeClientMock.requireStripeClient.mockReturnValue(stripe.client) - * const gate = stripe.holdNextUpdate('subscriptions') + * const gate = stripe.holdNextRequest('subscriptions.update') * ``` */ export function createInMemoryStripe() { const subscriptions = new Map() const customers = new Map() const idempotentResults = new Map() - const gates = new Map< - UpdatableResource, - Array<{ reached: () => void; released: Promise }> - >() + const gates = new Map void; released: Promise }>>() const failuresAfterApply = new Map() const failuresOnArrival = new Map() const events: Stripe.Event[] = [] @@ -211,6 +208,21 @@ export function createInMemoryStripe() { if (failure) throw failure } + async function waitAtGate(operation: StripeOperation) { + const gate = gates.get(operation)?.shift() + if (gate) { + gate.reached() + await gate.released + } + } + + async function retrieve(operation: StripeOperation, read: () => T): Promise { + rejectIfFailing(operation) + const snapshot = structuredClone(read()) + await waitAtGate(operation) + return snapshot + } + async function update( resource: UpdatableResource, id: string, @@ -219,11 +231,7 @@ export function createInMemoryStripe() { apply: () => T ): Promise { rejectIfFailing(`${resource}.update`) - const gate = gates.get(resource)?.shift() - if (gate) { - gate.reached() - await gate.released - } + await waitAtGate(`${resource}.update`) const idempotencyKey = options?.idempotencyKey const fingerprint = JSON.stringify([resource, id, params]) @@ -249,10 +257,7 @@ export function createInMemoryStripe() { const client = { subscriptions: { - retrieve: async (id: string) => { - rejectIfFailing('subscriptions.retrieve') - return structuredClone(requireSubscription(id)) - }, + retrieve: (id: string) => retrieve('subscriptions.retrieve', () => requireSubscription(id)), update: ( id: string, params: SubscriptionUpdateParams, @@ -263,10 +268,7 @@ export function createInMemoryStripe() { ), }, customers: { - retrieve: async (id: string) => { - rejectIfFailing('customers.retrieve') - return structuredClone(requireCustomer(id)) - }, + retrieve: (id: string) => retrieve('customers.retrieve', () => requireCustomer(id)), update: (id: string, params: CustomerUpdateParams, options?: { idempotencyKey?: string }) => update('customers', id, params, options, () => { const next = { ...requireCustomer(id), ...params } @@ -327,8 +329,8 @@ export function createInMemoryStripe() { updateOutsideSim(id: string, params: SubscriptionUpdateParams) { return applySubscriptionUpdate(id, params, null) }, - /** Parks the next update to `resource` until the returned gate is released. */ - holdNextUpdate(resource: UpdatableResource): InMemoryStripeRequestGate { + /** Parks the next call to `operation` until the returned gate is released. */ + holdNextRequest(operation: StripeOperation): InMemoryStripeRequestGate { let reached: () => void = () => {} const arrival = new Promise((resolve) => { reached = resolve @@ -337,7 +339,7 @@ export function createInMemoryStripe() { const released = new Promise((resolve) => { release = resolve }) - gates.set(resource, [...(gates.get(resource) ?? []), { reached, released }]) + gates.set(operation, [...(gates.get(operation) ?? []), { reached, released }]) return { reached: arrival, release } }, /** Makes the next update to `resource` apply in Stripe, then fail on the client. */ From c79bc166f8fe055e40c13b27fa06032bf1a39162 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 7 Oct 2026 14:45:53 -0700 Subject: [PATCH 07/14] fix(billing): order Stripe-accepted reconcile values by when Stripe was read --- .../stripe-sync-convergence.integration.ts | 32 +++++++++++++++++++ .../lib/billing/webhooks/subscription-sync.ts | 14 ++++++-- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts b/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts index 57a556299b8..6b89670b94e 100644 --- a/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts +++ b/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts @@ -618,6 +618,38 @@ describe('cancel_at_period_end sync', () => { expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(true) }) + it('orders concurrent reconciles of Stripe-side changes by when each read Stripe', async () => { + const pro = await createProUserInPaidOrganization() + await pauseProSubscriptionForOrgCoverage(pro.userId) + const pauseSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + stripe.failNextUpdateAfterApplying('subscriptions') + await expect(processEvent(pauseSync)).resolves.toBe('pending') + + stripe.updateOutsideSim(pro.stripeSubscriptionId, { cancel_at_period_end: false }) + const renewal = stripe.events.at(-1) as Stripe.Event + const firstRead = stripe.holdNextRequest('subscriptions.retrieve') + const secondRead = stripe.holdNextRequest('subscriptions.retrieve') + const reconcilingRenewal = deliver(renewal) + await firstRead.reached + + stripe.updateOutsideSim(pro.stripeSubscriptionId, { cancel_at_period_end: true }) + const reconcilingCancel = deliver(stripe.events.at(-1) as Stripe.Event) + await secondRead.reached + + firstRead.release() + await reconcilingRenewal + secondRead.release() + await reconcilingCancel + + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(true) + await makeDue(pauseSync) + await expect(processEvent(pauseSync)).resolves.toBe('completed') + expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(true) + }) + it('keeps a value Sim committed after the reconcile read Stripe', async () => { const pro = await createProUserInPaidOrganization() await pauseProSubscriptionForOrgCoverage(pro.userId) diff --git a/apps/sim/lib/billing/webhooks/subscription-sync.ts b/apps/sim/lib/billing/webhooks/subscription-sync.ts index 2ae2d830eb4..ee0f55dee66 100644 --- a/apps/sim/lib/billing/webhooks/subscription-sync.ts +++ b/apps/sim/lib/billing/webhooks/subscription-sync.ts @@ -99,14 +99,22 @@ async function readDatabaseClock(executor: DbOrTx): Promise { * an older value for the webhook reconcile to restore. The caller must hold the subscription row * lock (`FOR UPDATE`, or the `UPDATE` itself), per the lock order on * {@link lockSubscriptionForSyncRetry}. + * + * A Sim commit is stamped with the clock under that lock. A value taken from Stripe passes + * `observedAt`, the clock read just before Stripe was read, so it orders by when it was observed: + * a slower reconcile of an earlier Stripe read cannot outrank a later one. */ async function commitIntent( tx: DbOrTx, eventType: SubscriptionSyncEventType, subscriptionId: string, - fields: T + fields: T, + observedAt?: number ): Promise { - const committed = await withCommittedAt(tx, fields) + const committed = + observedAt === undefined + ? await withCommittedAt(tx, fields) + : { ...fields, committedAt: observedAt } await patchRetryableOutboxEvents(tx, eventType, subscriptionSubject(subscriptionId), committed) return committed } @@ -446,7 +454,7 @@ export async function reconcileSubscriptionSyncFromStripe(event: Stripe.Event): if (liveCancelAtPeriodEnd === undefined) return false cancelAtPeriodEnd = liveCancelAtPeriodEnd if (intents.cancelSyncCarriesOtherThan(cancelAtPeriodEnd)) { - await commitIntent(tx, CANCEL_SYNC, row.id, { cancelAtPeriodEnd }) + await commitIntent(tx, CANCEL_SYNC, row.id, { cancelAtPeriodEnd }, liveReadAt) } } const seats = intents.seats.status === 'value' ? intents.seats.value : current.seats From 62a94f138b0282bd027f0e1127032411568c040b Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 7 Oct 2026 15:16:32 -0700 Subject: [PATCH 08/14] fix(billing): recommit the in-flight value, include the plan in seat convergence, bound the reconcile's outbox reads --- apps/sim/lib/admin/subscription-lifecycle.ts | 8 +- .../organizations/provision-seat.test.ts | 2 +- .../billing/organizations/provision-seat.ts | 9 +- .../lib/billing/webhooks/outbox-handlers.ts | 8 +- .../stripe-sync-convergence.integration.ts | 306 +++++++++++++----- .../lib/billing/webhooks/subscription-sync.ts | 89 ++--- apps/sim/lib/core/outbox/service.ts | 50 +-- .../testing/src/mocks/outbox-service.mock.ts | 7 +- packages/testing/src/mocks/stripe.mock.ts | 23 +- 9 files changed, 340 insertions(+), 162 deletions(-) diff --git a/apps/sim/lib/admin/subscription-lifecycle.ts b/apps/sim/lib/admin/subscription-lifecycle.ts index 6bd34a029e1..406eb5fadf0 100644 --- a/apps/sim/lib/admin/subscription-lifecycle.ts +++ b/apps/sim/lib/admin/subscription-lifecycle.ts @@ -11,7 +11,7 @@ import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' import { enqueueCancelAtPeriodEndSync, lockSubscriptionForSyncRetry, - recommitSubscriptionSync, + recordCancelAtPeriodEnd, } from '@/lib/billing/webhooks/subscription-sync' import { enqueueOutboxEvent } from '@/lib/core/outbox/service' @@ -338,11 +338,7 @@ export async function requestDashboardSubscriptionCancellation({ and(eq(outboxEvent.id, existingOperation.id), eq(outboxEvent.status, 'dead_letter')) ) if (existingOperation.eventType === OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END) { - await recommitSubscriptionSync( - tx, - OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, - existingOperation.subscriptionId - ) + await recordCancelAtPeriodEnd(tx, existingOperation.subscriptionId, true) } return { operationId, diff --git a/apps/sim/lib/billing/organizations/provision-seat.test.ts b/apps/sim/lib/billing/organizations/provision-seat.test.ts index 9be5b3e5eab..2129fb485f6 100644 --- a/apps/sim/lib/billing/organizations/provision-seat.test.ts +++ b/apps/sim/lib/billing/organizations/provision-seat.test.ts @@ -55,7 +55,7 @@ const mockGetHighestPriorityPersonalSubscription = /** The subscription row as the activation re-reads it under its lock. */ function testExecutor(onSubscriptionLock: () => void = () => {}) { - const lockedRow = { cancelAtPeriodEnd: false, seats: 1 } + const lockedRow = { cancelAtPeriodEnd: false, seats: 1, stripeSubscriptionId: 'stripe_sub' } return { select: () => ({ from: () => ({ diff --git a/apps/sim/lib/billing/organizations/provision-seat.ts b/apps/sim/lib/billing/organizations/provision-seat.ts index b397dcd0a8e..dd170264614 100644 --- a/apps/sim/lib/billing/organizations/provision-seat.ts +++ b/apps/sim/lib/billing/organizations/provision-seat.ts @@ -243,7 +243,7 @@ async function convertPersonalSubscriptionToTeam( * the caller's earlier read is still cleared and recorded. */ async function activateTeamSubscription( - sub: { id: string; stripeSubscriptionId: string | null }, + sub: { id: string }, targetPlan: string, { planChanged }: { planChanged: boolean }, tx: DbOrTx @@ -252,6 +252,7 @@ async function activateTeamSubscription( .select({ cancelAtPeriodEnd: subscriptionTable.cancelAtPeriodEnd, seats: subscriptionTable.seats, + stripeSubscriptionId: subscriptionTable.stripeSubscriptionId, }) .from(subscriptionTable) .where(eq(subscriptionTable.id, sub.id)) @@ -271,10 +272,10 @@ async function activateTeamSubscription( }) } - if (!sub.stripeSubscriptionId) return - if (locked?.cancelAtPeriodEnd) { + if (!locked?.stripeSubscriptionId) return + if (locked.cancelAtPeriodEnd) { await enqueueCancelAtPeriodEndSync(tx, { - stripeSubscriptionId: sub.stripeSubscriptionId, + stripeSubscriptionId: locked.stripeSubscriptionId, subscriptionId: sub.id, cancelAtPeriodEnd: false, reason: 'pro-to-team-conversion', diff --git a/apps/sim/lib/billing/webhooks/outbox-handlers.ts b/apps/sim/lib/billing/webhooks/outbox-handlers.ts index 0be81303a80..b71ef41d974 100644 --- a/apps/sim/lib/billing/webhooks/outbox-handlers.ts +++ b/apps/sim/lib/billing/webhooks/outbox-handlers.ts @@ -299,11 +299,13 @@ const stripeSyncSubscriptionSeats: OutboxHandler = const latest = await getSubscriptionSeatSyncState(payload.subscriptionId) const latestSeats = (await readRecordedSyncValue(ctx.eventId))?.seats ?? (latest?.seats || 1) - if (latestSeats !== desiredSeats) { - logger.info('Subscription seats changed during Stripe sync; retrying latest value', { + if (latestSeats !== desiredSeats || latest?.plan !== row.plan) { + logger.info('Subscription plan or seats changed during Stripe sync; retrying latest', { eventId: ctx.eventId, subscriptionId: payload.subscriptionId, stripeSubscriptionId: row.stripeSubscriptionId, + attemptedPlan: row.plan, + latestPlan: latest?.plan, attemptedSeats: desiredSeats, latestSeats, attempt, @@ -323,7 +325,7 @@ const stripeSyncSubscriptionSeats: OutboxHandler = return } - throw new Error(`Subscription seats changed while syncing ${payload.subscriptionId}`) + throw new Error(`Subscription plan or seats changed while syncing ${payload.subscriptionId}`) } const stripeThresholdOverageInvoice: OutboxHandler = async ( diff --git a/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts b/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts index 6b89670b94e..c54216de3b9 100644 --- a/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts +++ b/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts @@ -28,6 +28,8 @@ import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vites const ADMIN_API_KEY = vi.hoisted(() => { const key = 'integration-fixture-admin-key' process.env.ADMIN_API_KEY = key + process.env.STRIPE_PRICE_TEAM_25_MO = 'price_team_pro_tier_month' + process.env.STRIPE_PRICE_TEAM_100_MO = 'price_team_max_tier_month' return key }) @@ -46,6 +48,7 @@ vi.mock('@/lib/core/config/env-flags', () => envFlagsMock) import { requestDashboardSubscriptionCancellation } from '@/lib/admin/subscription-lifecycle' import { createSimAuthAdapter } from '@/lib/auth/sim-auth-adapter' +import { CREDIT_TIERS } from '@/lib/billing/constants' import { pauseProSubscriptionForOrgCoverage, restoreUserProSubscription, @@ -58,6 +61,7 @@ import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' import { billingOutboxHandlers } from '@/lib/billing/webhooks/outbox-handlers' import { enqueueCancelAtPeriodEndSync, + enqueueSubscriptionSeatsSync, reconcileSubscriptionSyncFromStripe, recordCustomerRestoreAfterHook, } from '@/lib/billing/webhooks/subscription-sync' @@ -339,15 +343,43 @@ async function storedSubscription(subscriptionId: string) { } /** Resolves once another backend is blocked on a lock, i.e. the racing transaction is parked. */ -async function untilAnotherTransactionWaitsOnALock() { - for (let attempt = 0; attempt < 200; attempt++) { - const [row] = await connection<{ waiting: number }[]>` - select count(*)::int as waiting from pg_stat_activity - where datname = current_database() and wait_event_type = 'Lock'` - if (row.waiting > 0) return - await new Promise((resolve) => setImmediate(resolve)) +type TestTransaction = Parameters[0]>[0] + +/** + * Starts a transaction that takes its locks in `holdLocks`, then parks until released and runs + * `finish`. `untilBlocking` resolves once another backend is waiting on one of its locks. + */ +function startParkedTransaction( + holdLocks: (tx: TestTransaction) => Promise, + finish: (tx: TestTransaction) => Promise = async () => {} +) { + let release: () => void = () => {} + const released = new Promise((resolve) => { + release = resolve + }) + let reportPid: (pid: number) => void = () => {} + const holderPid = new Promise((resolve) => { + reportPid = resolve + }) + const done = testDatabase.transaction(async (tx) => { + await holdLocks(tx) + const [row] = await tx.execute<{ pid: number }>(sql`select pg_backend_pid() as pid`) + reportPid(row.pid) + await released + await finish(tx) + }) + async function untilBlocking() { + const pid = await holderPid + for (let attempt = 0; attempt < 200; attempt++) { + const [row] = await connection<{ blocked: number }[]>` + select count(*)::int as blocked from pg_stat_activity + where ${pid}::int = any(pg_blocking_pids(pid))` + if (row.blocked > 0) return + await new Promise((resolve) => setImmediate(resolve)) + } + throw new Error('No transaction ever waited on the parked one') } - throw new Error('The racing transaction never waited on a lock') + return { done, release, untilBlocking } } describe('cancel_at_period_end sync', () => { @@ -463,6 +495,30 @@ describe('cancel_at_period_end sync', () => { expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(false) }) + it('treats clearing a scheduled cancel_at in Stripe as a change made in Stripe', async () => { + const pro = await createProUserInPaidOrganization() + const scheduledEnd = Math.floor(Date.now() / 1000) + 7 * 24 * 60 * 60 + stripe.updateOutsideSim(pro.stripeSubscriptionId, { cancel_at: scheduledEnd }) + await deliver(stripe.events.at(-1) as Stripe.Event) + await pauseProSubscriptionForOrgCoverage(pro.userId) + const pauseSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + + stripe.updateOutsideSim(pro.stripeSubscriptionId, { cancel_at: '' }) + const restore = stripe.events.at(-1) as Stripe.Event + expect(Object.keys(restore.data.previous_attributes ?? {})).toEqual(['cancel_at']) + await deliver(restore) + + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(false) + expect(new Set(await cancelValuesOfRetryableSyncs(pro.subscriptionId))).toEqual( + new Set([false]) + ) + await expect(processEvent(pauseSync)).resolves.toBe('completed') + expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(false) + }) + it('lets a change made in Stripe while a sync is pending win over the pending value', async () => { const pro = await createProUserInPaidOrganization() await pauseProSubscriptionForOrgCoverage(pro.userId) @@ -687,6 +743,44 @@ describe('cancel_at_period_end sync', () => { expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(true) }) + it('requeues with the pending value when the plugin has overwritten the row', async () => { + const pro = await createProUserInPaidOrganization() + await pauseProSubscriptionForOrgCoverage(pro.userId) + const pauseSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + await deadLetter(pauseSync) + await testDatabase.transaction(async (tx) => { + await tx + .select({ id: subscription.id }) + .from(subscription) + .where(eq(subscription.id, pro.subscriptionId)) + .for('update') + await enqueueCancelAtPeriodEndSync(tx, { + stripeSubscriptionId: pro.stripeSubscriptionId, + subscriptionId: pro.subscriptionId, + cancelAtPeriodEnd: true, + reason: 'admin-cancel-at-period-end', + }) + }) + const adminSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + + beforeReconcile = async () => { + beforeReconcile = undefined + await requeueFromAdminApi(pauseSync) + } + await deliverUnrelatedUpdate(pro.stripeSubscriptionId) + + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(true) + await expect(processEvent(adminSync)).resolves.toBe('completed') + await expect(processEvent(pauseSync)).resolves.toBe('completed') + expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(true) + }) + it('does not revive an older value when a retry path resets its sync without re-committing', async () => { const pro = await createProUserInPaidOrganization() await pauseProSubscriptionForOrgCoverage(pro.userId) @@ -795,11 +889,7 @@ describe('Team activation', () => { }) stripe.addSubscription({ id: stripeSubscriptionId, customer: `cus_${subscriptionId}` }) - let releaseCancel: () => void = () => {} - const cancelHeld = new Promise((resolve) => { - releaseCancel = resolve - }) - const cancelling = testDatabase.transaction(async (tx) => { + const cancelling = startParkedTransaction(async (tx) => { await tx .update(subscription) .set({ cancelAtPeriodEnd: true }) @@ -810,7 +900,6 @@ describe('Team activation', () => { cancelAtPeriodEnd: true, reason: 'admin-cancel-at-period-end', }) - await cancelHeld }) const activating = testDatabase.transaction((tx) => ensureTeamOrganizationForAcceptance({ @@ -820,9 +909,9 @@ describe('Team activation', () => { workspaceIdsToAttach: [], }) ) - await untilAnotherTransactionWaitsOnALock() - releaseCancel() - await cancelling + await cancelling.untilBlocking() + cancelling.release() + await cancelling.done await expect(activating).resolves.toMatchObject({ success: true }) expect((await storedSubscription(subscriptionId)).cancelAtPeriodEnd).toBe(false) @@ -847,28 +936,27 @@ describe('operator retry', () => { ) await deadLetter(pauseSync) - let releaseWriter: () => void = () => {} - const writerHeld = new Promise((resolve) => { - releaseWriter = resolve - }) - const writing = testDatabase.transaction(async (tx) => { - await tx - .update(subscription) - .set({ cancelAtPeriodEnd: false }) - .where(eq(subscription.id, pro.subscriptionId)) - await writerHeld - await enqueueCancelAtPeriodEndSync(tx, { - stripeSubscriptionId: pro.stripeSubscriptionId, - subscriptionId: pro.subscriptionId, - cancelAtPeriodEnd: false, - reason: 'member-left-paid-org', - }) - }) + const writing = startParkedTransaction( + async (tx) => { + await tx + .update(subscription) + .set({ cancelAtPeriodEnd: false }) + .where(eq(subscription.id, pro.subscriptionId)) + }, + async (tx) => { + await enqueueCancelAtPeriodEndSync(tx, { + stripeSubscriptionId: pro.stripeSubscriptionId, + subscriptionId: pro.subscriptionId, + cancelAtPeriodEnd: false, + reason: 'member-left-paid-org', + }) + } + ) const requeuing = requeueFromAdminApi(pauseSync) - await untilAnotherTransactionWaitsOnALock() - releaseWriter() + await writing.untilBlocking() + writing.release() - await expect(Promise.all([writing, requeuing])).resolves.toBeDefined() + await expect(Promise.all([writing.done, requeuing])).resolves.toBeDefined() await deliverUnrelatedUpdate(pro.stripeSubscriptionId) expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(false) }) @@ -889,33 +977,32 @@ describe('operator retry', () => { ) await deadLetter(cancelSync) - let releaseWriter: () => void = () => {} - const writerHeld = new Promise((resolve) => { - releaseWriter = resolve - }) - const writing = testDatabase.transaction(async (tx) => { - await tx - .update(subscription) - .set({ cancelAtPeriodEnd: false }) - .where(eq(subscription.id, org.subscriptionId)) - await writerHeld - await enqueueCancelAtPeriodEndSync(tx, { - stripeSubscriptionId: org.stripeSubscriptionId, - subscriptionId: org.subscriptionId, - cancelAtPeriodEnd: false, - reason: 'pro-to-team-conversion', - }) - }) + const writing = startParkedTransaction( + async (tx) => { + await tx + .update(subscription) + .set({ cancelAtPeriodEnd: false }) + .where(eq(subscription.id, org.subscriptionId)) + }, + async (tx) => { + await enqueueCancelAtPeriodEndSync(tx, { + stripeSubscriptionId: org.stripeSubscriptionId, + subscriptionId: org.subscriptionId, + cancelAtPeriodEnd: false, + reason: 'pro-to-team-conversion', + }) + } + ) const retrying = requestDashboardSubscriptionCancellation({ organizationId: org.organizationId, operationId, timing: 'period_end', actor, }) - await untilAnotherTransactionWaitsOnALock() - releaseWriter() + await writing.untilBlocking() + writing.release() - await expect(Promise.all([writing, retrying])).resolves.toBeDefined() + await expect(Promise.all([writing.done, retrying])).resolves.toBeDefined() expect((await storedSubscription(org.subscriptionId)).cancelAtPeriodEnd).toBe(true) }) }) @@ -1007,6 +1094,42 @@ describe('Team seat sync', () => { expect(stripe.subscription(org.stripeSubscriptionId).items.data[0].quantity).toBe(2) }) + it('pushes the latest plan when an earlier seat sync lands in Stripe after a newer one', async () => { + const [smallTeam, largeTeam] = CREDIT_TIERS.map((tier) => `team_${tier.credits}`) + const org = await createOrganizationWithPlan('team', 1) + await testDatabase + .update(subscription) + .set({ plan: smallTeam }) + .where(eq(subscription.id, org.subscriptionId)) + async function commitPlan(plan: string) { + await testDatabase.transaction(async (tx) => { + await tx.update(subscription).set({ plan }).where(eq(subscription.id, org.subscriptionId)) + await enqueueSubscriptionSeatsSync(tx, { + subscriptionId: org.subscriptionId, + seats: 1, + reason: 'plan-change', + }) + }) + return latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_SUBSCRIPTION_SEATS, + org.subscriptionId + ) + } + + const smallSync = await commitPlan(smallTeam) + const stalePush = stripe.holdNextRequest('subscriptions.update') + const pushingSmall = processEvent(smallSync) + await stalePush.reached + const largeSync = await commitPlan(largeTeam) + await expect(processEvent(largeSync)).resolves.toBe('completed') + stalePush.release() + await expect(pushingSmall).resolves.toBe('completed') + + expect(stripe.subscription(org.stripeSubscriptionId).items.data[0].price.id).toBe( + 'price_team_max_tier_month' + ) + }) + it('does not revive an older seat count when its dead-lettered sync is requeued', async () => { const [owner, joiner] = await Promise.all([createUser('owner'), createUser('joiner')]) const org = await createOrganizationWithPlan('team', 1) @@ -1044,26 +1167,52 @@ describe('webhook reconcile cost', () => { 'Relation Name'?: string 'Shared Hit Blocks': number 'Shared Read Blocks': number + 'Actual Rows': number Plans?: QueryPlan[] } + + /** EXPLAIN ANALYZE inside a rolled-back transaction, so a measured UPDATE changes nothing. */ + async function explainWithoutEffects(query: string, parameters: unknown[]) { + const rollback = new Error('rollback') + let plan: QueryPlan | undefined + await connection + .begin(async (sql) => { + const [explained] = await sql.unsafe( + `EXPLAIN (ANALYZE, BUFFERS, FORMAT JSON) ${query}`, + parameters as never[] + ) + plan = (explained['QUERY PLAN'] as { Plan: QueryPlan }[])[0].Plan + throw rollback + }) + .catch((error: unknown) => { + if (error !== rollback) throw error + }) + if (!plan) throw new Error(`No plan for ${query}`) + return plan + } const planNodes = (plan: QueryPlan): QueryPlan[] => [ plan, ...(plan.Plans ?? []).flatMap(planNodes), ] - it('reads only the syncs that can still run, however many have completed', async () => { + it('reads only in-flight syncs, however many have completed or dead-lettered', async () => { const pro = await createProUserInPaidOrganization() await pauseProSubscriptionForOrgCoverage(pro.userId) - await connection` - INSERT INTO outbox_event (id, event_type, payload, status, available_at, created_at, processed_at) - SELECT ${generateId()} || ':' || n, ${OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END}, - json_build_object( - 'subscriptionId', ${pro.subscriptionId}::text, - 'cancelAtPeriodEnd', n % 2 = 0, - 'committedAt', n - ), - 'completed', now(), now(), now() - FROM generate_series(1, 20000) AS n` + for (const [status, count] of [ + ['completed', 20000], + ['dead_letter', 50], + ] as const) { + await connection` + INSERT INTO outbox_event (id, event_type, payload, status, available_at, created_at, processed_at) + SELECT ${generateId()} || ':' || n, ${OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END}, + json_build_object( + 'subscriptionId', ${pro.subscriptionId}::text, + 'cancelAtPeriodEnd', false, + 'committedAt', n + ), + ${status}, now(), now(), now() + FROM generate_series(1, ${count}::integer) AS n` + } await connection`ANALYZE outbox_event` const issued: { query: string; parameters: unknown[] }[] = [] @@ -1083,27 +1232,26 @@ describe('webhook reconcile cost', () => { database.current = drizzle(traced, { schema }) try { await deliverUnrelatedUpdate(pro.stripeSubscriptionId) + stripe.updateOutsideSim(pro.stripeSubscriptionId, { cancel_at_period_end: true }) + await deliver(stripe.events.at(-1) as Stripe.Event) } finally { database.current = testDatabase await traced.end() } expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(true) + expect(new Set(await cancelValuesOfRetryableSyncs(pro.subscriptionId))).toEqual(new Set([true])) - const outboxReads = issued.filter(({ query }) => /from "outbox_event"/i.test(query)) - expect(outboxReads.length).toBeGreaterThan(0) - for (const { query, parameters } of outboxReads) { - const [explained] = await connection.unsafe( - `EXPLAIN (ANALYZE, BUFFERS, FORMAT JSON) ${query}`, - parameters as never[] - ) - const plan = (explained['QUERY PLAN'] as { Plan: QueryPlan }[])[0].Plan - const nodes = planNodes(plan) + const outboxQueries = issued.filter(({ query }) => /"outbox_event"/i.test(query)) + expect(outboxQueries.some(({ query }) => /^update/i.test(query))).toBe(true) + for (const { query, parameters } of outboxQueries) { + const plan = await explainWithoutEffects(query, parameters) expect( - nodes.some( + planNodes(plan).some( (node) => node['Node Type'] === 'Seq Scan' && node['Relation Name'] === 'outbox_event' ) ).toBe(false) - expect(plan['Shared Hit Blocks'] + plan['Shared Read Blocks']).toBeLessThan(100) + expect(plan['Shared Hit Blocks'] + plan['Shared Read Blocks']).toBeLessThan(200) + if (/^select/i.test(query)) expect(plan['Actual Rows']).toBeLessThanOrEqual(1) } }) }) diff --git a/apps/sim/lib/billing/webhooks/subscription-sync.ts b/apps/sim/lib/billing/webhooks/subscription-sync.ts index ee0f55dee66..e783529f6d8 100644 --- a/apps/sim/lib/billing/webhooks/subscription-sync.ts +++ b/apps/sim/lib/billing/webhooks/subscription-sync.ts @@ -9,8 +9,7 @@ import { requireStripeClient } from '@/lib/billing/stripe-client' import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' import { enqueueOutboxEvent, - INFLIGHT_OUTBOX_STATUSES, - listRetryableOutboxEvents, + listInflightOutboxEvents, patchRetryableOutboxEvents, readOutboxEventPayload, } from '@/lib/core/outbox/service' @@ -100,9 +99,10 @@ async function readDatabaseClock(executor: DbOrTx): Promise { * lock (`FOR UPDATE`, or the `UPDATE` itself), per the lock order on * {@link lockSubscriptionForSyncRetry}. * - * A Sim commit is stamped with the clock under that lock. A value taken from Stripe passes - * `observedAt`, the clock read just before Stripe was read, so it orders by when it was observed: - * a slower reconcile of an earlier Stripe read cannot outrank a later one. + * A Sim commit is stamped with the clock under that lock and rewrites every such event. A value + * taken from Stripe passes `observedAt`, the clock read just before Stripe was read, so it orders + * by when it was observed (a slower reconcile of an earlier Stripe read cannot outrank a later + * one), and rewrites only the events that do not already carry it. */ async function commitIntent( tx: DbOrTx, @@ -115,7 +115,13 @@ async function commitIntent( observedAt === undefined ? await withCommittedAt(tx, fields) : { ...fields, committedAt: observedAt } - await patchRetryableOutboxEvents(tx, eventType, subscriptionSubject(subscriptionId), committed) + await patchRetryableOutboxEvents( + tx, + eventType, + subscriptionSubject(subscriptionId), + committed, + observedAt === undefined ? undefined : fields + ) return committed } @@ -153,8 +159,9 @@ export async function enqueueSubscriptionSeatsSync( } /** - * Records a `cancelAtPeriodEnd` value written in this transaction without enqueuing a sync, for - * a writer that left the row's value unchanged: no sync that can still run keeps an older value. + * Records a `cancelAtPeriodEnd` value written in this transaction without enqueuing a sync, for a + * writer whose value an existing sync will push or Stripe already holds. It is written onto every + * sync that can still run (including one just reset to pending), so none keeps an older value. * The caller must hold the subscription row lock. */ export async function recordCancelAtPeriodEnd( @@ -188,9 +195,11 @@ export async function lockSubscriptionForSyncRetry( } /** - * Re-commits the subscription's current DB value onto its sync events that can still run, for a - * dead-lettered event that was just reset to `pending`: the retry then carries the latest value - * rather than the one it failed with. The caller holds the lock from + * Re-commits the latest committed value onto the subscription's sync events that can still run, + * for a dead-lettered event that was just reset to `pending`: the retry then carries the latest + * value rather than the one it failed with. That is the newest in-flight value; the row is only a + * fallback when nothing in flight records one, because until the reconcile step runs the row can + * hold the Stripe plugin's stale webhook payload. The caller holds the lock from * {@link lockSubscriptionForSyncRetry}, taken before the reset. */ export async function recommitSubscriptionSync( @@ -205,14 +214,19 @@ export async function recommitSubscriptionSync( .limit(1) if (!current) return - await commitIntent( - tx, - eventType, - subscriptionId, - eventType === CANCEL_SYNC - ? { cancelAtPeriodEnd: Boolean(current.cancelAtPeriodEnd) } - : { seats: current.seats ?? 1 } - ) + const pending = await readSyncIntents(tx, subscriptionId) + if (eventType === CANCEL_SYNC) { + const intent = pending.cancelAtPeriodEnd + await commitIntent(tx, eventType, subscriptionId, { + cancelAtPeriodEnd: + intent.status === 'value' ? intent.value : Boolean(current.cancelAtPeriodEnd), + }) + return + } + const intent = pending.seats + await commitIntent(tx, eventType, subscriptionId, { + seats: intent.status === 'value' ? intent.value : (current.seats ?? 1), + }) } /** @@ -304,16 +318,14 @@ type InflightIntent = | { status: 'legacy' } | { status: 'value'; value: T; committedAt: number } -const INFLIGHT_STATUSES: ReadonlySet = new Set(INFLIGHT_OUTBOX_STATUSES) - function latestIntent( - events: { eventType: string; status: string; payload: unknown }[], + events: { eventType: string; payload: unknown }[], eventType: SubscriptionSyncEventType, readValue: (payload: Record) => T | undefined ): InflightIntent { let latest: { committedAt: number; value: T } | undefined for (const event of events) { - if (event.eventType !== eventType || !INFLIGHT_STATUSES.has(event.status)) continue + if (event.eventType !== eventType) continue const payload = toRecord(event.payload) const value = readValue(payload) if (typeof payload.committedAt !== 'number' || value === undefined) return { status: 'legacy' } @@ -324,9 +336,13 @@ function latestIntent( return latest ? { status: 'value', ...latest } : { status: 'none' } } -/** One indexed read of the subscription's sync events that can still run. */ +/** + * One indexed read of the subscription's in-flight syncs. Dead letters are not intents: they are + * failed syncs awaiting an operator, kept current by every commit so a retry pushes the latest + * value, but never a reason to override Stripe. + */ async function readSyncIntents(executor: DbOrTx, subscriptionId: string) { - const events = await listRetryableOutboxEvents( + const events = await listInflightOutboxEvents( executor, [CANCEL_SYNC, SEATS_SYNC], subscriptionSubject(subscriptionId) @@ -338,19 +354,19 @@ async function readSyncIntents(executor: DbOrTx, subscriptionId: string) { seats: latestIntent(events, SEATS_SYNC, (payload) => typeof payload.seats === 'number' ? payload.seats : undefined ), - /** Whether a cancel sync that can still run carries a value other than `value`. */ - cancelSyncCarriesOtherThan: (value: boolean) => - events.some( - (event) => - event.eventType === CANCEL_SYNC && toRecord(event.payload).cancelAtPeriodEnd !== value - ), } } -/** True when the event records a `cancel_at_period_end` change made in Stripe, not by Sim's sync. */ +/** + * True when the event records a cancellation change made in Stripe, not by Sim's sync. A change + * to `cancel_at` counts too: Better Auth's restore clears `cancel_at` when it is set, and Stripe + * may then list only `cancel_at` among the previous attributes. + */ function isCancellationChangedInStripe(event: Stripe.Event): boolean { const previousAttributes = toRecord(event.data.previous_attributes) - if (!('cancel_at_period_end' in previousAttributes)) return false + if (!('cancel_at_period_end' in previousAttributes) && !('cancel_at' in previousAttributes)) { + return false + } const idempotencyKey = event.request?.idempotency_key const issuedBySimSync = idempotencyKey?.startsWith(CANCEL_AT_PERIOD_END_SYNC_KEY_PREFIX) || @@ -393,7 +409,8 @@ function cancelAtPeriodEndSource( * Decided under the subscription row lock that every committing writer holds: * - `cancelAtPeriodEnd`: while a cancel sync is in flight, its committed value wins over * snapshots and over echoes of Sim's own writes. A change made in Stripe itself (customer - * portal, dashboard, Better Auth's cancel/restore endpoints) wins and is committed onto every + * portal, dashboard, Better Auth's cancel/restore endpoints), recognised by a non-Sim request + * changing `cancel_at_period_end` or `cancel_at`, wins and is committed onto every * sync that can still run, unless Sim committed a newer value after Stripe was read. With no * sync in flight Stripe wins, read live so out-of-order delivery cannot regress it. * - Precedence across the two systems is arrival order, not wall-clock order: a Stripe-side @@ -453,9 +470,7 @@ export async function reconcileSubscriptionSyncFromStripe(event: Stripe.Event): } else if (cancel.source === 'stripe') { if (liveCancelAtPeriodEnd === undefined) return false cancelAtPeriodEnd = liveCancelAtPeriodEnd - if (intents.cancelSyncCarriesOtherThan(cancelAtPeriodEnd)) { - await commitIntent(tx, CANCEL_SYNC, row.id, { cancelAtPeriodEnd }, liveReadAt) - } + await commitIntent(tx, CANCEL_SYNC, row.id, { cancelAtPeriodEnd }, liveReadAt) } const seats = intents.seats.status === 'value' ? intents.seats.value : current.seats diff --git a/apps/sim/lib/core/outbox/service.ts b/apps/sim/lib/core/outbox/service.ts index 78650a2b710..f732a2d165b 100644 --- a/apps/sim/lib/core/outbox/service.ts +++ b/apps/sim/lib/core/outbox/service.ts @@ -417,7 +417,7 @@ export async function readOutboxEventPayload(eventId: string): Promise } /** Statuses of an event whose side effect may still run. */ -export const INFLIGHT_OUTBOX_STATUSES = ['pending', 'processing'] as const +const INFLIGHT_OUTBOX_STATUSES = ['pending', 'processing'] as const /** * Statuses an event can still run from: in flight, or dead-lettered, which every operator retry * path resets to `pending`. A `completed` event never runs again. @@ -443,42 +443,48 @@ function eventsForSubject( } /** - * The `pending`, `processing`, or `dead_letter` events of the given types for one subject. Pass - * the caller's transaction to read under its locks. + * The `pending` or `processing` events of the given types for one subject. Pass the caller's + * transaction to read under its locks. */ -export async function listRetryableOutboxEvents( +export async function listInflightOutboxEvents( executor: Pick, eventTypes: readonly string[], - subject: OutboxPayloadSubject -): Promise<{ id: string; eventType: string; status: string; payload: unknown }[]> { - return executor - .select({ - id: outboxEvent.id, - eventType: outboxEvent.eventType, - status: outboxEvent.status, - payload: outboxEvent.payload, - }) + subject: OutboxPayloadSubject, + limit?: number +): Promise<{ id: string; eventType: string; payload: unknown }[]> { + const query = executor + .select({ id: outboxEvent.id, eventType: outboxEvent.eventType, payload: outboxEvent.payload }) .from(outboxEvent) - .where(eventsForSubject(eventTypes, subject, RETRYABLE_OUTBOX_STATUSES)) + .where(eventsForSubject(eventTypes, subject, INFLIGHT_OUTBOX_STATUSES)) + return limit === undefined ? query : query.limit(limit) } /** * Shallow-merges `patch` into the payload of every `pending`, `processing`, or `dead_letter` - * event of the type for one subject. Callers serialize writers for the subject with their - * domain lock. + * event of the type for one subject, skipping events whose payload already contains + * `unlessPayloadContains`. One UPDATE; nothing is read into memory. Callers serialize writers + * for the subject with their domain lock. */ export async function patchRetryableOutboxEvents( executor: Pick, eventType: string, subject: OutboxPayloadSubject, - patch: Record + patch: Record, + unlessPayloadContains?: Record ): Promise { const patched = await executor .update(outboxEvent) .set({ payload: sql`(coalesce(${outboxEvent.payload}::jsonb, '{}'::jsonb) || ${JSON.stringify(patch)}::jsonb)::json`, }) - .where(eventsForSubject([eventType], subject, RETRYABLE_OUTBOX_STATUSES)) + .where( + and( + eventsForSubject([eventType], subject, RETRYABLE_OUTBOX_STATUSES), + unlessPayloadContains + ? sql`not (${outboxEvent.payload}::jsonb @> ${JSON.stringify(unlessPayloadContains)}::jsonb)` + : undefined + ) + ) .returning({ id: outboxEvent.id }) return patched.length } @@ -494,12 +500,8 @@ export async function hasInflightOutboxEvent( payloadKey: string, payloadValue: string ): Promise { - const [row] = await db - .select({ id: outboxEvent.id }) - .from(outboxEvent) - .where(eventsForSubject([eventType], { payloadKey, payloadValue }, INFLIGHT_OUTBOX_STATUSES)) - .limit(1) - return Boolean(row) + const events = await listInflightOutboxEvents(db, [eventType], { payloadKey, payloadValue }, 1) + return events.length > 0 } /** diff --git a/packages/testing/src/mocks/outbox-service.mock.ts b/packages/testing/src/mocks/outbox-service.mock.ts index 4875502f550..1744ecfb0bd 100644 --- a/packages/testing/src/mocks/outbox-service.mock.ts +++ b/packages/testing/src/mocks/outbox-service.mock.ts @@ -68,7 +68,7 @@ export const outboxServiceMockFns = { ) }), mockFindDeadLetteredEvents: vi.fn(), - mockListRetryableOutboxEvents: vi.fn(), + mockListInflightOutboxEvents: vi.fn(), mockReadOutboxEventPayload: vi.fn(), mockPatchRetryableOutboxEvents: vi.fn(), mockHasInflightOutboxEvent: vi.fn(), @@ -79,7 +79,7 @@ export const outboxServiceMockFns = { /** * Static mock module for `@/lib/core/outbox/service`. Covers every runtime export; - * `MAX_BULK_ENQUEUE_EVENTS` and `INFLIGHT_OUTBOX_STATUSES` carry the real values. + * `MAX_BULK_ENQUEUE_EVENTS` carries the real value. * * @example * ```ts @@ -88,7 +88,6 @@ export const outboxServiceMockFns = { */ export const outboxServiceMock = { MAX_BULK_ENQUEUE_EVENTS: 1_000, - INFLIGHT_OUTBOX_STATUSES: ['pending', 'processing'] as const, deferOutboxHandler: outboxServiceMockFns.mockDeferOutboxHandler, continueOutboxHandler: outboxServiceMockFns.mockContinueOutboxHandler, withOutboxHandlerTimeout: outboxServiceMockFns.mockWithOutboxHandlerTimeout, @@ -101,7 +100,7 @@ export const outboxServiceMock = { outboxEventHasSourceOperationId: outboxServiceMockFns.mockOutboxEventHasSourceOperationId, outboxPayloadHasSourceOperationId: outboxServiceMockFns.mockOutboxPayloadHasSourceOperationId, findDeadLetteredEvents: outboxServiceMockFns.mockFindDeadLetteredEvents, - listRetryableOutboxEvents: outboxServiceMockFns.mockListRetryableOutboxEvents, + listInflightOutboxEvents: outboxServiceMockFns.mockListInflightOutboxEvents, readOutboxEventPayload: outboxServiceMockFns.mockReadOutboxEventPayload, patchRetryableOutboxEvents: outboxServiceMockFns.mockPatchRetryableOutboxEvents, hasInflightOutboxEvent: outboxServiceMockFns.mockHasInflightOutboxEvent, diff --git a/packages/testing/src/mocks/stripe.mock.ts b/packages/testing/src/mocks/stripe.mock.ts index 85bef873e2f..4f39b6ceb34 100644 --- a/packages/testing/src/mocks/stripe.mock.ts +++ b/packages/testing/src/mocks/stripe.mock.ts @@ -103,6 +103,8 @@ type StripeOperation = `${UpdatableResource}.${'retrieve' | 'update'}` interface SubscriptionUpdateParams { cancel_at_period_end?: boolean + /** A Unix timestamp schedules the cancellation; `''` clears it. Only `cancel_at` changes. */ + cancel_at?: number | '' metadata?: Record items?: Array<{ id: string; quantity?: number; price?: string }> } @@ -170,6 +172,13 @@ export function createInMemoryStripe() { previousAttributes.cancel_at_period_end = current.cancel_at_period_end next.cancel_at_period_end = params.cancel_at_period_end } + if (params.cancel_at !== undefined) { + const cancelAt = params.cancel_at === '' ? null : params.cancel_at + if (cancelAt !== current.cancel_at) { + previousAttributes.cancel_at = current.cancel_at + next.cancel_at = cancelAt + } + } if (params.metadata) { previousAttributes.metadata = current.metadata next.metadata = { ...current.metadata, ...params.metadata } @@ -288,8 +297,11 @@ export function createInMemoryStripe() { events, addSubscription( subscription: Pick & - Partial> & { + Partial< + Pick + > & { quantity?: number + priceId?: string } ) { const now = Math.floor(Date.now() / 1000) @@ -299,7 +311,7 @@ export function createInMemoryStripe() { customer: subscription.customer, status: subscription.status ?? 'active', cancel_at_period_end: subscription.cancel_at_period_end ?? false, - cancel_at: null, + cancel_at: subscription.cancel_at ?? null, canceled_at: null, ended_at: null, trial_start: null, @@ -314,7 +326,10 @@ export function createInMemoryStripe() { quantity: subscription.quantity ?? 1, current_period_start: now, current_period_end: now + 30 * 24 * 60 * 60, - price: { id: `price_${subscription.id}`, recurring: { interval: 'month' } }, + price: { + id: subscription.priceId ?? `price_${subscription.id}`, + recurring: { interval: 'month' }, + }, }, ], }, @@ -342,11 +357,11 @@ export function createInMemoryStripe() { gates.set(operation, [...(gates.get(operation) ?? []), { reached, released }]) return { reached: arrival, release } }, - /** Makes the next update to `resource` apply in Stripe, then fail on the client. */ /** Makes the next call to `operation` fail before Stripe processes it, as an outage does. */ failNextRequest(operation: StripeOperation, error = new Error('Stripe is unavailable')) { failuresOnArrival.set(operation, [...(failuresOnArrival.get(operation) ?? []), error]) }, + /** Makes the next update to `resource` apply in Stripe, then fail on the client. */ failNextUpdateAfterApplying(resource: UpdatableResource, error = new Error('socket hang up')) { failuresAfterApply.set(resource, [...(failuresAfterApply.get(resource) ?? []), error]) }, From 681e2c5dfe8952342fc24fb0441f09dfd241832d Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 7 Oct 2026 15:28:52 -0700 Subject: [PATCH 09/14] fix(billing): stamp every sync a later Stripe read confirms; ignore a moved cancel_at --- .../stripe-sync-convergence.integration.ts | 60 +++++++++++++++++++ .../lib/billing/webhooks/subscription-sync.ts | 29 +++++---- apps/sim/lib/core/outbox/service.ts | 13 ++-- 3 files changed, 84 insertions(+), 18 deletions(-) diff --git a/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts b/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts index c54216de3b9..782b762ac3f 100644 --- a/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts +++ b/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts @@ -519,6 +519,66 @@ describe('cancel_at_period_end sync', () => { expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(false) }) + it('records a later Stripe read even when every sync already carries its value', async () => { + const pro = await createProUserInPaidOrganization() + await pauseProSubscriptionForOrgCoverage(pro.userId) + const pauseSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + stripe.failNextUpdateAfterApplying('subscriptions') + await expect(processEvent(pauseSync)).resolves.toBe('pending') + + stripe.updateOutsideSim(pro.stripeSubscriptionId, { cancel_at_period_end: false }) + const restoreRead = stripe.holdNextRequest('subscriptions.retrieve') + const reconcilingRestore = deliver(stripe.events.at(-1) as Stripe.Event) + await restoreRead.reached + + stripe.updateOutsideSim(pro.stripeSubscriptionId, { cancel_at_period_end: true }) + const cancelRead = stripe.holdNextRequest('subscriptions.retrieve') + const reconcilingCancel = deliver(stripe.events.at(-1) as Stripe.Event) + await cancelRead.reached + + cancelRead.release() + await reconcilingCancel + restoreRead.release() + await reconcilingRestore + + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(true) + await makeDue(pauseSync) + await expect(processEvent(pauseSync)).resolves.toBe('completed') + expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(true) + }) + + it('keeps a pending value when an unrelated Stripe update moves the cancellation date', async () => { + const pro = await createProUserInPaidOrganization() + await pauseProSubscriptionForOrgCoverage(pro.userId) + const pauseSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + await expect(processEvent(pauseSync)).resolves.toBe('completed') + const periodEnd = Math.floor(Date.now() / 1000) + 30 * 24 * 60 * 60 + stripe.updateOutsideSim(pro.stripeSubscriptionId, { cancel_at: periodEnd }) + await deliver(stripe.events.at(-1) as Stripe.Event) + + await leaveOrganization(pro.userId, pro.paidOrganization.organizationId) + await restoreUserProSubscription(pro.userId) + const restoreSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + + stripe.updateOutsideSim(pro.stripeSubscriptionId, { cancel_at: periodEnd + 335 * 24 * 60 * 60 }) + const moved = stripe.events.at(-1) as Stripe.Event + expect(Object.keys(moved.data.previous_attributes ?? {})).toEqual(['cancel_at']) + await deliver(moved) + + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(false) + await expect(processEvent(restoreSync)).resolves.toBe('completed') + expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(false) + }) + it('lets a change made in Stripe while a sync is pending win over the pending value', async () => { const pro = await createProUserInPaidOrganization() await pauseProSubscriptionForOrgCoverage(pro.userId) diff --git a/apps/sim/lib/billing/webhooks/subscription-sync.ts b/apps/sim/lib/billing/webhooks/subscription-sync.ts index e783529f6d8..26787b587e5 100644 --- a/apps/sim/lib/billing/webhooks/subscription-sync.ts +++ b/apps/sim/lib/billing/webhooks/subscription-sync.ts @@ -101,8 +101,9 @@ async function readDatabaseClock(executor: DbOrTx): Promise { * * A Sim commit is stamped with the clock under that lock and rewrites every such event. A value * taken from Stripe passes `observedAt`, the clock read just before Stripe was read, so it orders - * by when it was observed (a slower reconcile of an earlier Stripe read cannot outrank a later - * one), and rewrites only the events that do not already carry it. + * by when it was observed: it rewrites (value and stamp) only the events last stamped before that + * observation, including ones already holding the value, so a slower reconcile of an earlier + * Stripe read cannot outrank a later one and never overwrites a newer commit. */ async function commitIntent( tx: DbOrTx, @@ -120,7 +121,7 @@ async function commitIntent( eventType, subscriptionSubject(subscriptionId), committed, - observedAt === undefined ? undefined : fields + observedAt === undefined ? undefined : 'committedAt' ) return committed } @@ -358,15 +359,18 @@ async function readSyncIntents(executor: DbOrTx, subscriptionId: string) { } /** - * True when the event records a cancellation change made in Stripe, not by Sim's sync. A change - * to `cancel_at` counts too: Better Auth's restore clears `cancel_at` when it is set, and Stripe - * may then list only `cancel_at` among the previous attributes. + * True when the event records a cancellation change made in Stripe, not by Sim's sync. A + * `cancel_at` that was set or cleared counts too: Better Auth's restore clears `cancel_at` when it + * is set, and Stripe may then list only `cancel_at` among the previous attributes. A `cancel_at` + * that only moved (e.g. a billing-interval switch on a subscription already ending) is not a + * cancellation change. */ function isCancellationChangedInStripe(event: Stripe.Event): boolean { const previousAttributes = toRecord(event.data.previous_attributes) - if (!('cancel_at_period_end' in previousAttributes) && !('cancel_at' in previousAttributes)) { - return false - } + const scheduledOrCleared = + 'cancel_at' in previousAttributes && + (previousAttributes.cancel_at == null) !== (toRecord(event.data.object).cancel_at == null) + if (!('cancel_at_period_end' in previousAttributes) && !scheduledOrCleared) return false const idempotencyKey = event.request?.idempotency_key const issuedBySimSync = idempotencyKey?.startsWith(CANCEL_AT_PERIOD_END_SYNC_KEY_PREFIX) || @@ -410,9 +414,10 @@ function cancelAtPeriodEndSource( * - `cancelAtPeriodEnd`: while a cancel sync is in flight, its committed value wins over * snapshots and over echoes of Sim's own writes. A change made in Stripe itself (customer * portal, dashboard, Better Auth's cancel/restore endpoints), recognised by a non-Sim request - * changing `cancel_at_period_end` or `cancel_at`, wins and is committed onto every - * sync that can still run, unless Sim committed a newer value after Stripe was read. With no - * sync in flight Stripe wins, read live so out-of-order delivery cannot regress it. + * changing `cancel_at_period_end` or setting or clearing `cancel_at`, wins and is committed + * onto every sync that can still run, unless Sim committed a newer value after Stripe was + * read. With no sync in flight Stripe wins, read live so out-of-order delivery cannot regress + * it. * - Precedence across the two systems is arrival order, not wall-clock order: a Stripe-side * change whose webhook is processed after a Sim commit wins even if the customer made it * earlier. Stripe's `event.created` is not compared with the database clock, because skew diff --git a/apps/sim/lib/core/outbox/service.ts b/apps/sim/lib/core/outbox/service.ts index f732a2d165b..eed0495eafa 100644 --- a/apps/sim/lib/core/outbox/service.ts +++ b/apps/sim/lib/core/outbox/service.ts @@ -461,16 +461,17 @@ export async function listInflightOutboxEvents( /** * Shallow-merges `patch` into the payload of every `pending`, `processing`, or `dead_letter` - * event of the type for one subject, skipping events whose payload already contains - * `unlessPayloadContains`. One UPDATE; nothing is read into memory. Callers serialize writers - * for the subject with their domain lock. + * event of the type for one subject. With `onlyIfOlderThanPatch`, naming a numeric payload key + * that `patch` sets, an event whose own value for that key is already at least the patch's is + * left alone, so a stale writer never overwrites a newer one. One UPDATE; nothing is read into + * memory. Callers serialize writers for the subject with their domain lock. */ export async function patchRetryableOutboxEvents( executor: Pick, eventType: string, subject: OutboxPayloadSubject, patch: Record, - unlessPayloadContains?: Record + onlyIfOlderThanPatch?: string ): Promise { const patched = await executor .update(outboxEvent) @@ -480,8 +481,8 @@ export async function patchRetryableOutboxEvents( .where( and( eventsForSubject([eventType], subject, RETRYABLE_OUTBOX_STATUSES), - unlessPayloadContains - ? sql`not (${outboxEvent.payload}::jsonb @> ${JSON.stringify(unlessPayloadContains)}::jsonb)` + onlyIfOlderThanPatch + ? sql`coalesce((${outboxEvent.payload} ->> ${onlyIfOlderThanPatch})::numeric, -1) < ${String(patch[onlyIfOlderThanPatch])}::numeric` : undefined ) ) From dc34ffa24338c7789a12c82338e41e97b6f4ac31 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 7 Oct 2026 15:55:18 -0700 Subject: [PATCH 10/14] fix(billing): a fresh idempotency key per seat write so a returning value is applied, not replayed --- .../lib/billing/webhooks/outbox-handlers.ts | 11 ++-- .../stripe-sync-convergence.integration.ts | 59 +++++++++++++++++-- packages/testing/src/mocks/stripe.mock.ts | 7 ++- 3 files changed, 65 insertions(+), 12 deletions(-) diff --git a/apps/sim/lib/billing/webhooks/outbox-handlers.ts b/apps/sim/lib/billing/webhooks/outbox-handlers.ts index b71ef41d974..6c71a48589a 100644 --- a/apps/sim/lib/billing/webhooks/outbox-handlers.ts +++ b/apps/sim/lib/billing/webhooks/outbox-handlers.ts @@ -27,8 +27,11 @@ const logger = createLogger('BillingOutboxHandlers') */ const MAX_SYNC_ATTEMPTS = 2 -/** A fresh key per Stripe write: the SDK reuses it across its own network retries of that call. */ -function customerContactSyncIdempotencyKey(eventId: string): string { +/** + * A fresh key per Stripe write: the SDK reuses it across its own network retries of that call. + * A key derived from the pushed value would be replayed, unapplied, once that value comes back. + */ +function syncWriteIdempotencyKey(eventId: string): string { return `outbox:${eventId}:${generateShortId()}` } @@ -293,7 +296,7 @@ const stripeSyncSubscriptionSeats: OutboxHandler = ], proration_behavior: 'always_invoice', }, - { idempotencyKey: `outbox:${ctx.eventId}:${row.plan}:${desiredSeats}` } + { idempotencyKey: syncWriteIdempotencyKey(ctx.eventId) } ) } @@ -503,7 +506,7 @@ const stripeSyncCustomerContact: OutboxHandler email: contact.email, ...(contact.name ? { name: contact.name } : {}), }, - { idempotencyKey: customerContactSyncIdempotencyKey(ctx.eventId) } + { idempotencyKey: syncWriteIdempotencyKey(ctx.eventId) } ) } diff --git a/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts b/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts index 782b762ac3f..da4e0d3e154 100644 --- a/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts +++ b/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts @@ -25,12 +25,23 @@ import postgres from 'postgres' import type Stripe from 'stripe' import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' -const ADMIN_API_KEY = vi.hoisted(() => { - const key = 'integration-fixture-admin-key' - process.env.ADMIN_API_KEY = key - process.env.STRIPE_PRICE_TEAM_25_MO = 'price_team_pro_tier_month' - process.env.STRIPE_PRICE_TEAM_100_MO = 'price_team_max_tier_month' - return key +const { ADMIN_API_KEY, restoreEnvironment } = vi.hoisted(() => { + const fixture: Record = { + ADMIN_API_KEY: 'integration-fixture-admin-key', + STRIPE_PRICE_TEAM_25_MO: 'price_team_pro_tier_month', + STRIPE_PRICE_TEAM_100_MO: 'price_team_max_tier_month', + } + const previous = Object.fromEntries(Object.keys(fixture).map((key) => [key, process.env[key]])) + Object.assign(process.env, fixture) + return { + ADMIN_API_KEY: fixture.ADMIN_API_KEY, + restoreEnvironment() { + for (const [key, value] of Object.entries(previous)) { + if (value === undefined) delete process.env[key] + else process.env[key] = value + } + }, + } }) const database = vi.hoisted(() => ({ @@ -166,6 +177,7 @@ beforeEach(() => { afterAll(async () => { resetEnvFlagsMock() + restoreEnvironment() try { await connection`DROP SCHEMA ${connection(schemaName)} CASCADE` } finally { @@ -1190,6 +1202,41 @@ describe('Team seat sync', () => { ) }) + it('pushes a seat count that changes away and back while its sync retries', async () => { + const org = await createOrganizationWithPlan('team', 1) + async function commitSeats(seats: number) { + await testDatabase.transaction(async (tx) => { + await tx.update(subscription).set({ seats }).where(eq(subscription.id, org.subscriptionId)) + await enqueueSubscriptionSeatsSync(tx, { + subscriptionId: org.subscriptionId, + seats, + reason: 'member-change', + }) + }) + return latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_SUBSCRIPTION_SEATS, + org.subscriptionId + ) + } + + const seatSync = await commitSeats(2) + const firstPush = stripe.holdNextRequest('subscriptions.update') + const syncing = processEvent(seatSync) + await firstPush.reached + await commitSeats(3) + const secondPush = stripe.holdNextRequest('subscriptions.update') + firstPush.release() + await secondPush.reached + await commitSeats(2) + secondPush.release() + await expect(syncing).resolves.toBe('pending') + expect(stripe.subscription(org.stripeSubscriptionId).items.data[0].quantity).toBe(3) + + await makeDue(seatSync) + await expect(processEvent(seatSync)).resolves.toBe('completed') + expect(stripe.subscription(org.stripeSubscriptionId).items.data[0].quantity).toBe(2) + }) + it('does not revive an older seat count when its dead-lettered sync is requeued', async () => { const [owner, joiner] = await Promise.all([createUser('owner'), createUser('joiner')]) const org = await createOrganizationWithPlan('team', 1) diff --git a/packages/testing/src/mocks/stripe.mock.ts b/packages/testing/src/mocks/stripe.mock.ts index 4f39b6ceb34..6dfdd11cf2d 100644 --- a/packages/testing/src/mocks/stripe.mock.ts +++ b/packages/testing/src/mocks/stripe.mock.ts @@ -180,8 +180,11 @@ export function createInMemoryStripe() { } } if (params.metadata) { - previousAttributes.metadata = current.metadata - next.metadata = { ...current.metadata, ...params.metadata } + const metadata = { ...current.metadata, ...params.metadata } + if (JSON.stringify(metadata) !== JSON.stringify(current.metadata)) { + previousAttributes.metadata = current.metadata + next.metadata = metadata + } } for (const item of params.items ?? []) { const target = next.items.data.find((existing) => existing.id === item.id) From fa5e995bf3760cd264601c0755c3a2fba9a094d2 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 7 Oct 2026 16:23:19 -0700 Subject: [PATCH 11/14] fix(billing): compare membership-driven seat and cancel changes against the committed value, not a possibly-stale row --- .../lib/billing/organizations/membership.ts | 32 +++++++++++-- .../billing/organizations/provision-seat.ts | 6 ++- apps/sim/lib/billing/organizations/seats.ts | 11 ++++- .../stripe-sync-convergence.integration.ts | 46 +++++++++++++++++++ .../lib/billing/webhooks/subscription-sync.ts | 37 ++++++++++++--- .../mocks/billing-subscription-sync.mock.ts | 11 ++++- 6 files changed, 128 insertions(+), 15 deletions(-) diff --git a/apps/sim/lib/billing/organizations/membership.ts b/apps/sim/lib/billing/organizations/membership.ts index e78c74ddfe0..bd9ba1b061b 100644 --- a/apps/sim/lib/billing/organizations/membership.ts +++ b/apps/sim/lib/billing/organizations/membership.ts @@ -48,7 +48,10 @@ import { import { toDecimal, toNumber } from '@/lib/billing/utils/decimal' import { validateSeatAvailability } from '@/lib/billing/validation/seat-management' import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' -import { enqueueCancelAtPeriodEndSync } from '@/lib/billing/webhooks/subscription-sync' +import { + enqueueCancelAtPeriodEndSync, + readCommittedCancelAtPeriodEnd, +} from '@/lib/billing/webhooks/subscription-sync' import { isBillingEnabled } from '@/lib/core/config/env-flags' import { OrchestrationError } from '@/lib/core/orchestration/types' import { enqueueOutboxEvent } from '@/lib/core/outbox/service' @@ -259,7 +262,13 @@ export async function restoreUserProSubscription(userId: string): Promise { expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(true) }) + it('restores the personal Pro when its member leaves while the plugin has overwritten the row', async () => { + const pro = await createProUserInPaidOrganization() + await pauseProSubscriptionForOrgCoverage(pro.userId) + const pauseSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + + beforeReconcile = async () => { + beforeReconcile = undefined + await leaveOrganization(pro.userId, pro.paidOrganization.organizationId) + await restoreUserProSubscription(pro.userId) + } + await deliverUnrelatedUpdate(pro.stripeSubscriptionId) + + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(false) + await expect(processEvent(pauseSync)).resolves.toBe('completed') + expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(false) + }) + it('does not revive an older value when a retry path resets its sync without re-committing', async () => { const pro = await createProUserInPaidOrganization() await pauseProSubscriptionForOrgCoverage(pro.userId) @@ -1237,6 +1257,32 @@ describe('Team seat sync', () => { expect(stripe.subscription(org.stripeSubscriptionId).items.data[0].quantity).toBe(2) }) + it('drops a seat when a member leaves while the plugin has overwritten the row', async () => { + const [owner, joiner] = await Promise.all([createUser('owner'), createUser('joiner')]) + const org = await createOrganizationWithPlan('team', 1) + await addMember(org.organizationId, owner.id, 'owner') + await addMember(org.organizationId, joiner.id) + await reconcileOrganizationSeats({ organizationId: org.organizationId, reason: 'member-added' }) + const growSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_SUBSCRIPTION_SEATS, + org.subscriptionId + ) + + beforeReconcile = async () => { + beforeReconcile = undefined + await leaveOrganization(joiner.id, org.organizationId) + await reconcileOrganizationSeats({ + organizationId: org.organizationId, + reason: 'member-removed', + }) + } + await deliverUnrelatedUpdate(org.stripeSubscriptionId) + + expect((await storedSubscription(org.subscriptionId)).seats).toBe(1) + await expect(processEvent(growSync)).resolves.toBe('completed') + expect(stripe.subscription(org.stripeSubscriptionId).items.data[0].quantity).toBe(1) + }) + it('does not revive an older seat count when its dead-lettered sync is requeued', async () => { const [owner, joiner] = await Promise.all([createUser('owner'), createUser('joiner')]) const org = await createOrganizationWithPlan('team', 1) diff --git a/apps/sim/lib/billing/webhooks/subscription-sync.ts b/apps/sim/lib/billing/webhooks/subscription-sync.ts index 26787b587e5..b8dae4db133 100644 --- a/apps/sim/lib/billing/webhooks/subscription-sync.ts +++ b/apps/sim/lib/billing/webhooks/subscription-sync.ts @@ -215,18 +215,18 @@ export async function recommitSubscriptionSync( .limit(1) if (!current) return - const pending = await readSyncIntents(tx, subscriptionId) if (eventType === CANCEL_SYNC) { - const intent = pending.cancelAtPeriodEnd await commitIntent(tx, eventType, subscriptionId, { - cancelAtPeriodEnd: - intent.status === 'value' ? intent.value : Boolean(current.cancelAtPeriodEnd), + cancelAtPeriodEnd: await readCommittedCancelAtPeriodEnd( + tx, + subscriptionId, + Boolean(current.cancelAtPeriodEnd) + ), }) return } - const intent = pending.seats await commitIntent(tx, eventType, subscriptionId, { - seats: intent.status === 'value' ? intent.value : (current.seats ?? 1), + seats: await readCommittedSeats(tx, subscriptionId, current.seats ?? 1), }) } @@ -304,6 +304,31 @@ export async function readRecordedSyncValue( } } +/** + * The subscription's latest committed `cancelAtPeriodEnd`: the newest value an in-flight sync + * records, else `stored` (the row). Until the reconcile step runs, the row can hold the Stripe + * plugin's stale webhook payload, so a writer deciding whether a change is needed compares + * against this, never the row alone. The caller holds the subscription row lock. + */ +export async function readCommittedCancelAtPeriodEnd( + tx: DbOrTx, + subscriptionId: string, + stored: boolean +): Promise { + const intent = (await readSyncIntents(tx, subscriptionId)).cancelAtPeriodEnd + return intent.status === 'value' ? intent.value : stored +} + +/** The seat-count counterpart of {@link readCommittedCancelAtPeriodEnd}. */ +export async function readCommittedSeats( + tx: DbOrTx, + subscriptionId: string, + stored: number +): Promise { + const intent = (await readSyncIntents(tx, subscriptionId)).seats + return intent.status === 'value' ? intent.value : stored +} + /** A fresh key per Stripe write: the SDK reuses it across its own network retries of that call. */ export function cancelAtPeriodEndSyncIdempotencyKey(eventId: string): string { return `${CANCEL_AT_PERIOD_END_SYNC_KEY_PREFIX}${eventId}:${generateShortId()}` diff --git a/packages/testing/src/mocks/billing-subscription-sync.mock.ts b/packages/testing/src/mocks/billing-subscription-sync.mock.ts index 52b958a31f2..596d5226ede 100644 --- a/packages/testing/src/mocks/billing-subscription-sync.mock.ts +++ b/packages/testing/src/mocks/billing-subscription-sync.mock.ts @@ -3,7 +3,8 @@ import { vi } from 'vitest' /** * Controllable mock functions for `@/lib/billing/webhooks/subscription-sync`. The enqueue * functions resolve to a fixed event id; drive them with `mockResolvedValueOnce`. - * `mockIsSubscriptionSyncEventType` keeps the real logic. + * `mockIsSubscriptionSyncEventType` keeps the real logic; the `mockReadCommitted*` readers + * return the stored value they are given, as when nothing is in flight. * * @example * ```ts @@ -32,6 +33,12 @@ export const billingSubscriptionSyncMockFns = { mockReconcileSubscriptionSyncFromStripe: vi.fn(async () => undefined), mockRecordCustomerRestoreAfterHook: vi.fn(async () => undefined), mockReadRecordedSyncValue: vi.fn(async () => undefined), + mockReadCommittedCancelAtPeriodEnd: vi.fn( + async (_tx: unknown, _subscriptionId: string, stored: boolean) => stored + ), + mockReadCommittedSeats: vi.fn( + async (_tx: unknown, _subscriptionId: string, stored: number) => stored + ), } /** @@ -55,4 +62,6 @@ export const billingSubscriptionSyncMock = { billingSubscriptionSyncMockFns.mockReconcileSubscriptionSyncFromStripe, recordCustomerRestoreAfterHook: billingSubscriptionSyncMockFns.mockRecordCustomerRestoreAfterHook, readRecordedSyncValue: billingSubscriptionSyncMockFns.mockReadRecordedSyncValue, + readCommittedCancelAtPeriodEnd: billingSubscriptionSyncMockFns.mockReadCommittedCancelAtPeriodEnd, + readCommittedSeats: billingSubscriptionSyncMockFns.mockReadCommittedSeats, } From 4f1c786d4506102055ffa9a7e33e2d26aa48b1fa Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 7 Oct 2026 16:31:35 -0700 Subject: [PATCH 12/14] fix(billing): a membership writer skips only when both the row and the committed value already match --- .../lib/billing/organizations/membership.ts | 28 +++++++++++-------- .../billing/organizations/provision-seat.ts | 4 +-- apps/sim/lib/billing/organizations/seats.ts | 2 +- .../stripe-sync-convergence.integration.ts | 27 ++++++++++++++++++ .../lib/billing/webhooks/subscription-sync.ts | 27 +++++++++++++++--- .../mocks/billing-subscription-sync.mock.ts | 11 ++++---- 6 files changed, 76 insertions(+), 23 deletions(-) diff --git a/apps/sim/lib/billing/organizations/membership.ts b/apps/sim/lib/billing/organizations/membership.ts index bd9ba1b061b..c7a023759ac 100644 --- a/apps/sim/lib/billing/organizations/membership.ts +++ b/apps/sim/lib/billing/organizations/membership.ts @@ -50,7 +50,7 @@ import { validateSeatAvailability } from '@/lib/billing/validation/seat-manageme import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-events' import { enqueueCancelAtPeriodEndSync, - readCommittedCancelAtPeriodEnd, + isCancelAtPeriodEndSettled, } from '@/lib/billing/webhooks/subscription-sync' import { isBillingEnabled } from '@/lib/core/config/env-flags' import { OrchestrationError } from '@/lib/core/orchestration/types' @@ -263,12 +263,16 @@ export async function restoreUserProSubscription(userId: string): Promise { expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(true) }) + it('keeps a pause committed after the reconcile read a customer restore from Stripe', async () => { + const pro = await createProUserInPaidOrganization() + await pauseProSubscriptionForOrgCoverage(pro.userId) + const pauseSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + stripe.failNextUpdateAfterApplying('subscriptions') + await expect(processEvent(pauseSync)).resolves.toBe('pending') + + stripe.updateOutsideSim(pro.stripeSubscriptionId, { cancel_at_period_end: false }) + const liveRead = stripe.holdNextRequest('subscriptions.retrieve') + const reconcilingRestore = deliver(stripe.events.at(-1) as Stripe.Event) + await liveRead.reached + await pauseProSubscriptionForOrgCoverage(pro.userId) + liveRead.release() + await reconcilingRestore + + expect((await storedSubscription(pro.subscriptionId)).cancelAtPeriodEnd).toBe(true) + const latestSync = await latestOutboxEventId( + OUTBOX_EVENT_TYPES.STRIPE_SYNC_CANCEL_AT_PERIOD_END, + pro.subscriptionId + ) + await expect(processEvent(latestSync)).resolves.toBe('completed') + expect(stripe.subscription(pro.stripeSubscriptionId).cancel_at_period_end).toBe(true) + }) + it('restores the personal Pro when its member leaves while the plugin has overwritten the row', async () => { const pro = await createProUserInPaidOrganization() await pauseProSubscriptionForOrgCoverage(pro.userId) diff --git a/apps/sim/lib/billing/webhooks/subscription-sync.ts b/apps/sim/lib/billing/webhooks/subscription-sync.ts index b8dae4db133..86d2f2d87f7 100644 --- a/apps/sim/lib/billing/webhooks/subscription-sync.ts +++ b/apps/sim/lib/billing/webhooks/subscription-sync.ts @@ -310,7 +310,7 @@ export async function readRecordedSyncValue( * plugin's stale webhook payload, so a writer deciding whether a change is needed compares * against this, never the row alone. The caller holds the subscription row lock. */ -export async function readCommittedCancelAtPeriodEnd( +async function readCommittedCancelAtPeriodEnd( tx: DbOrTx, subscriptionId: string, stored: boolean @@ -319,6 +319,24 @@ export async function readCommittedCancelAtPeriodEnd( return intent.status === 'value' ? intent.value : stored } +/** + * True when both the row and the latest committed `cancelAtPeriodEnd` already hold `desired`, so + * a writer has nothing to record. A writer that sees either one differ writes the row and + * commits: a redundant sync of the same value is harmless, while skipping on the committed value + * alone could let an older Stripe read, accepted after this writer, override it. + */ +export async function isCancelAtPeriodEndSettled( + tx: DbOrTx, + subscriptionId: string, + stored: boolean, + desired: boolean +): Promise { + return ( + stored === desired && + (await readCommittedCancelAtPeriodEnd(tx, subscriptionId, stored)) === desired + ) +} + /** The seat-count counterpart of {@link readCommittedCancelAtPeriodEnd}. */ export async function readCommittedSeats( tx: DbOrTx, @@ -363,9 +381,10 @@ function latestIntent( } /** - * One indexed read of the subscription's in-flight syncs. Dead letters are not intents: they are - * failed syncs awaiting an operator, kept current by every commit so a retry pushes the latest - * value, but never a reason to override Stripe. + * One read of the subscription's in-flight syncs, through the status index and bounded by the + * in-flight backlog. Dead letters are not intents: they are failed syncs awaiting an operator, + * kept current by every commit so a retry pushes the latest value, but never a reason to override + * Stripe. */ async function readSyncIntents(executor: DbOrTx, subscriptionId: string) { const events = await listInflightOutboxEvents( diff --git a/packages/testing/src/mocks/billing-subscription-sync.mock.ts b/packages/testing/src/mocks/billing-subscription-sync.mock.ts index 596d5226ede..492ce5f4a49 100644 --- a/packages/testing/src/mocks/billing-subscription-sync.mock.ts +++ b/packages/testing/src/mocks/billing-subscription-sync.mock.ts @@ -3,8 +3,8 @@ import { vi } from 'vitest' /** * Controllable mock functions for `@/lib/billing/webhooks/subscription-sync`. The enqueue * functions resolve to a fixed event id; drive them with `mockResolvedValueOnce`. - * `mockIsSubscriptionSyncEventType` keeps the real logic; the `mockReadCommitted*` readers - * return the stored value they are given, as when nothing is in flight. + * `mockIsSubscriptionSyncEventType` keeps the real logic; `mockReadCommittedSeats` and + * `mockIsCancelAtPeriodEndSettled` answer from the stored value, as when nothing is in flight. * * @example * ```ts @@ -33,8 +33,9 @@ export const billingSubscriptionSyncMockFns = { mockReconcileSubscriptionSyncFromStripe: vi.fn(async () => undefined), mockRecordCustomerRestoreAfterHook: vi.fn(async () => undefined), mockReadRecordedSyncValue: vi.fn(async () => undefined), - mockReadCommittedCancelAtPeriodEnd: vi.fn( - async (_tx: unknown, _subscriptionId: string, stored: boolean) => stored + mockIsCancelAtPeriodEndSettled: vi.fn( + async (_tx: unknown, _subscriptionId: string, stored: boolean, desired: boolean) => + stored === desired ), mockReadCommittedSeats: vi.fn( async (_tx: unknown, _subscriptionId: string, stored: number) => stored @@ -62,6 +63,6 @@ export const billingSubscriptionSyncMock = { billingSubscriptionSyncMockFns.mockReconcileSubscriptionSyncFromStripe, recordCustomerRestoreAfterHook: billingSubscriptionSyncMockFns.mockRecordCustomerRestoreAfterHook, readRecordedSyncValue: billingSubscriptionSyncMockFns.mockReadRecordedSyncValue, - readCommittedCancelAtPeriodEnd: billingSubscriptionSyncMockFns.mockReadCommittedCancelAtPeriodEnd, + isCancelAtPeriodEndSettled: billingSubscriptionSyncMockFns.mockIsCancelAtPeriodEndSettled, readCommittedSeats: billingSubscriptionSyncMockFns.mockReadCommittedSeats, } From c9db5e84a69858bef7091292b242111dca4764cf Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 7 Oct 2026 16:54:08 -0700 Subject: [PATCH 13/14] fix(billing): a seat-row repair records no seat-change audit; deterministic latest-sync test helper --- apps/sim/lib/billing/organizations/seats.ts | 14 ++++++++++++++ .../stripe-sync-convergence.integration.ts | 6 +++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/apps/sim/lib/billing/organizations/seats.ts b/apps/sim/lib/billing/organizations/seats.ts index 013ec281fe0..7e7b804cae2 100644 --- a/apps/sim/lib/billing/organizations/seats.ts +++ b/apps/sim/lib/billing/organizations/seats.ts @@ -16,6 +16,11 @@ import { captureServerEvent } from '@/lib/posthog/server' const logger = createLogger('OrganizationSeats') export interface ReconcileOrganizationSeatsResult { + /** + * True only when the seat count changed. Repairing a row the Stripe plugin left stale, back to + * the committed count, still rewrites the row and re-records the sync (`outboxEventId` is set) + * but reports false and records no seat audit or analytics event. + */ changed: boolean previousSeats?: number seats?: number @@ -171,6 +176,15 @@ export async function reconcileOrganizationSeats({ outboxEventId: outcome.outboxEventId, }) + if (outcome.seats === outcome.previousSeats) { + return { + changed: false, + previousSeats: outcome.previousSeats, + seats: outcome.seats, + outboxEventId: outcome.outboxEventId, + } + } + const increased = outcome.seats > outcome.previousSeats if (actorId) { recordAudit({ diff --git a/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts b/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts index 7e1bad915a0..d7bb535b1d6 100644 --- a/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts +++ b/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts @@ -265,7 +265,11 @@ async function latestOutboxEventId(eventType: string, subscriptionId: string) { sql`${outboxEvent.payload} ->> 'subscriptionId' = ${subscriptionId}` ) ) - .orderBy(desc(outboxEvent.createdAt), desc(outboxEvent.id)) + .orderBy( + desc(outboxEvent.createdAt), + sql`(${outboxEvent.payload} ->> 'committedAt')::numeric desc nulls last`, + desc(outboxEvent.id) + ) .limit(1) if (!latest) throw new Error(`No ${eventType} event for ${subscriptionId}`) return latest.id From 9cccfb443111a857e6eaf41b401e3b12815c2f0c Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 7 Oct 2026 17:14:23 -0700 Subject: [PATCH 14/14] test(billing): latest-sync helper fails loudly on an enqueue-time tie --- .../stripe-sync-convergence.integration.ts | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts b/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts index d7bb535b1d6..6e7777e611a 100644 --- a/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts +++ b/apps/sim/lib/billing/webhooks/stripe-sync-convergence.integration.ts @@ -255,9 +255,16 @@ async function leaveOrganization(userId: string, organizationId: string) { .where(and(eq(member.userId, userId), eq(member.organizationId, organizationId))) } +/** + * The sync event enqueued last for a subscription. `created_at` is the enqueuing transaction's + * start time, compared at microsecond precision, so it orders events from different + * transactions. No test enqueues two of one type for one subscription in a single transaction, + * and a tie fails loudly rather than being broken arbitrarily: `outbox_event` has no per-insert + * sequence to break it with. + */ async function latestOutboxEventId(eventType: string, subscriptionId: string) { - const [latest] = await testDatabase - .select({ id: outboxEvent.id }) + const [latest, previous] = await testDatabase + .select({ id: outboxEvent.id, createdAt: sql`${outboxEvent.createdAt}::text` }) .from(outboxEvent) .where( and( @@ -265,13 +272,12 @@ async function latestOutboxEventId(eventType: string, subscriptionId: string) { sql`${outboxEvent.payload} ->> 'subscriptionId' = ${subscriptionId}` ) ) - .orderBy( - desc(outboxEvent.createdAt), - sql`(${outboxEvent.payload} ->> 'committedAt')::numeric desc nulls last`, - desc(outboxEvent.id) - ) - .limit(1) + .orderBy(desc(outboxEvent.createdAt)) + .limit(2) if (!latest) throw new Error(`No ${eventType} event for ${subscriptionId}`) + if (previous?.createdAt === latest.createdAt) { + throw new Error(`Two ${eventType} events for ${subscriptionId} share one enqueue time`) + } return latest.id }