Skip to content

Commit babf00f

Browse files
dante01yoonampagent
andcommitted
fix(billing): reject stale workspace completions
Amp-Thread-ID: https://ampcode.com/threads/T-01a02a4c-0f97-734f-999f-3f04a1e74795 Co-authored-by: Amp <amp@ampcode.com>
1 parent 587cf63 commit babf00f

8 files changed

Lines changed: 221 additions & 13 deletions

src/composables/billing/useBillingContext.test.ts

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ const {
3535
mockUpdateActiveWorkspace,
3636
mockSetWorkspaceBillingRail,
3737
mockLegacyStatus,
38-
mockBillingStatus
38+
mockBillingStatus,
39+
mockActiveWorkspaceId,
40+
mockWorkspaceTransitionGeneration
3941
} = vi.hoisted(() => ({
4042
mockIsPersonal: { value: true },
4143
mockBillingRail: { value: undefined as BillingRail | undefined },
@@ -61,7 +63,9 @@ const {
6163
subscription_tier: 'PRO',
6264
subscription_duration: 'MONTHLY'
6365
} as Partial<BillingStatusResponse>
64-
}
66+
},
67+
mockActiveWorkspaceId: { value: 'personal-123' },
68+
mockWorkspaceTransitionGeneration: { value: 0 }
6569
}))
6670

6771
vi.mock('@vueuse/core', async (importOriginal) => {
@@ -90,8 +94,14 @@ vi.mock('@/platform/workspace/stores/teamWorkspaceStore', async () => {
9094
},
9195
get activeWorkspace() {
9296
return mockIsPersonal.value
93-
? { id: 'personal-123', type: 'personal' }
94-
: { id: 'team-456', type: 'team' }
97+
? { id: mockActiveWorkspaceId.value, type: 'personal' }
98+
: { id: mockActiveWorkspaceId.value, type: 'team' }
99+
},
100+
get activeWorkspaceId() {
101+
return mockActiveWorkspaceId.value
102+
},
103+
get workspaceTransitionGeneration() {
104+
return mockWorkspaceTransitionGeneration.value
95105
},
96106
get activeWorkspaceBillingRail() {
97107
return mockBillingRail.value
@@ -180,6 +190,8 @@ describe('useBillingContext', () => {
180190
remoteConfig.value = {}
181191
remoteConfigState.value = 'unloaded'
182192
mockIsPersonal.value = true
193+
mockActiveWorkspaceId.value = 'personal-123'
194+
mockWorkspaceTransitionGeneration.value = 0
183195
mockBillingRail.value = undefined
184196
mockSetWorkspaceBillingRail.mockImplementation(
185197
(_workspaceId: string, billingRail: BillingRail) => {
@@ -411,6 +423,37 @@ describe('useBillingContext', () => {
411423
expect(mockLegacyFetchBalance).not.toHaveBeenCalled()
412424
})
413425

426+
it('does not reconcile after switching away and back', async () => {
427+
remoteConfig.value = { legacy_billing_migration_enabled: true }
428+
remoteConfigState.value = 'authenticated'
429+
mockBillingRail.value = 'legacy_stripe'
430+
431+
const context = useBillingContext()
432+
await nextTick()
433+
vi.clearAllMocks()
434+
435+
let finishStatusRefresh!: (status: BillingStatusResponse) => void
436+
vi.mocked(workspaceApi.getBillingStatus).mockReturnValueOnce(
437+
new Promise((resolve) => {
438+
finishStatusRefresh = resolve
439+
})
440+
)
441+
442+
const reconciliation = context.reconcileSubscriptionSuccess()
443+
await vi.waitFor(() =>
444+
expect(workspaceApi.getBillingStatus).toHaveBeenCalledOnce()
445+
)
446+
mockActiveWorkspaceId.value = 'personal-456'
447+
mockWorkspaceTransitionGeneration.value++
448+
mockActiveWorkspaceId.value = 'personal-123'
449+
mockWorkspaceTransitionGeneration.value++
450+
finishStatusRefresh(DEFAULT_BILLING_STATUS)
451+
await reconciliation
452+
453+
expect(workspaceApi.getBillingBalance).not.toHaveBeenCalled()
454+
expect(mockLegacyFetchBalance).not.toHaveBeenCalled()
455+
})
456+
414457
it('rejects topup amounts that are not positive whole-dollar cents', async () => {
415458
const { topup } = useBillingContext()
416459
await expect(topup(550)).rejects.toThrow()

src/composables/billing/useBillingContext.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,11 +285,27 @@ function useBillingContextInternal(): BillingContext {
285285
}
286286

287287
async function reconcileSubscriptionSuccess(): Promise<void> {
288+
const workspaceId = store.activeWorkspaceId
289+
const workspaceTransitionGeneration = store.workspaceTransitionGeneration
288290
const checkout = checkoutContext.value
289291
await checkout.fetchStatus()
292+
if (
293+
workspaceId !== store.activeWorkspaceId ||
294+
workspaceTransitionGeneration !== store.workspaceTransitionGeneration
295+
) {
296+
return
297+
}
290298

291299
const account = activeContext.value
292-
if (account !== checkout) await account.fetchStatus()
300+
if (account !== checkout) {
301+
await account.fetchStatus()
302+
if (
303+
workspaceId !== store.activeWorkspaceId ||
304+
workspaceTransitionGeneration !== store.workspaceTransitionGeneration
305+
) {
306+
return
307+
}
308+
}
293309
await account.fetchBalance()
294310
}
295311

src/platform/workspace/components/TopUpCreditsDialogContentWorkspace.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ const mockPermissions = vi.hoisted(() => ({
2424
}))
2525
const mockShouldUseWorkspaceBilling = vi.hoisted(() => ({ value: true }))
2626
const mockDistributionTypes = vi.hoisted(() => ({ isCloud: true }))
27+
const mockWorkspace = vi.hoisted(() => ({
28+
activeWorkspaceId: 'workspace-1' as string | null,
29+
workspaceTransitionGeneration: 0
30+
}))
2731

2832
vi.mock('@/platform/distribution/types', () => mockDistributionTypes)
2933
const mockBillingOperationState = vi.hoisted(() => ({
@@ -59,6 +63,10 @@ vi.mock('@/platform/workspace/stores/billingOperationStore', async () => {
5963
}
6064
})
6165

66+
vi.mock('@/platform/workspace/stores/teamWorkspaceStore', () => ({
67+
useTeamWorkspaceStore: () => mockWorkspace
68+
}))
69+
6270
vi.mock('@/platform/workspace/composables/useWorkspaceUI', async () => {
6371
const { ref } = await import('vue')
6472
mockPermissions.ref = ref({ canTopUp: true })
@@ -212,6 +220,8 @@ describe('TopUpCreditsDialogContentWorkspace', () => {
212220
mockDistributionTypes.isCloud = true
213221
setCanTopUp(true)
214222
mockShouldUseWorkspaceBilling.value = true
223+
mockWorkspace.activeWorkspaceId = 'workspace-1'
224+
mockWorkspace.workspaceTransitionGeneration = 0
215225
setIsAddingCredits(false)
216226
setTopupActionOperation(undefined)
217227
mockFetchBalance.mockResolvedValue(undefined)
@@ -424,6 +434,35 @@ describe('TopUpCreditsDialogContentWorkspace', () => {
424434
})
425435
})
426436

437+
it('ignores a completed top-up after switching away and back', async () => {
438+
let resolveTopup!: (response: CreateTopupResponse) => void
439+
mockTopup.mockReturnValueOnce(
440+
new Promise((resolve) => {
441+
resolveTopup = resolve
442+
})
443+
)
444+
445+
renderDialog()
446+
await clickAddCredits()
447+
await userEvent.click(screen.getByRole('button', { name: 'Pay $50.00' }))
448+
await waitFor(() => expect(mockTopup).toHaveBeenCalledOnce())
449+
450+
mockWorkspace.activeWorkspaceId = 'workspace-2'
451+
mockWorkspace.workspaceTransitionGeneration++
452+
mockWorkspace.activeWorkspaceId = 'workspace-1'
453+
mockWorkspace.workspaceTransitionGeneration++
454+
resolveTopup(topupResponse('completed'))
455+
await nextTick()
456+
457+
expect(mockFetchBalance).not.toHaveBeenCalled()
458+
expect(mockFetchStatus).not.toHaveBeenCalled()
459+
expect(mockToastAdd).not.toHaveBeenCalledWith(
460+
expect.objectContaining({ severity: 'success' })
461+
)
462+
expect(mockCloseDialog).not.toHaveBeenCalled()
463+
expect(mockShowSettings).not.toHaveBeenCalled()
464+
})
465+
427466
it('opens Credits settings after a completed local top-up', async () => {
428467
mockDistributionTypes.isCloud = false
429468
mockTopup.mockResolvedValue(topupResponse('completed'))

src/platform/workspace/components/TopUpCreditsDialogContentWorkspace.vue

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,7 @@ import { categorizeBillingApiError } from '@/platform/telemetry/utils/billingFai
265265
import { useSettingsDialog } from '@/platform/settings/composables/useSettingsDialog'
266266
import { useWorkspaceUI } from '@/platform/workspace/composables/useWorkspaceUI'
267267
import { useBillingOperationStore } from '@/platform/workspace/stores/billingOperationStore'
268+
import { useTeamWorkspaceStore } from '@/platform/workspace/stores/teamWorkspaceStore'
268269
import { useDialogStore } from '@/stores/dialogStore'
269270
import { cn } from '@comfyorg/tailwind-utils'
270271
@@ -281,6 +282,7 @@ const { buildDocsUrl, docsPaths } = useExternalLink()
281282
const { fetchBalance, fetchStatus, topup } = useBillingContext()
282283
const { shouldUseWorkspaceBilling } = useBillingRouting()
283284
const { permissions } = useWorkspaceUI()
285+
const workspaceStore = useTeamWorkspaceStore()
284286
285287
const billingOperationStore = useBillingOperationStore()
286288
const isPolling = computed(() => billingOperationStore.isAddingCredits)
@@ -425,7 +427,19 @@ async function handleBuy() {
425427
})
426428
427429
const amountCents = payAmount.value * 100
430+
const workspaceId = workspaceStore.activeWorkspaceId
431+
const workspaceTransitionGeneration =
432+
workspaceStore.workspaceTransitionGeneration
428433
const response = await topup(amountCents)
434+
if (
435+
shouldUseWorkspaceBilling.value &&
436+
(workspaceId !== workspaceStore.activeWorkspaceId ||
437+
workspaceTransitionGeneration !==
438+
workspaceStore.workspaceTransitionGeneration)
439+
) {
440+
paymentSubmitted.value = false
441+
return
442+
}
429443
if (!response) {
430444
paymentSubmitted.value = false
431445
telemetry?.trackBillingEvent({

src/platform/workspace/composables/useSubscriptionCheckout.test.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,13 +280,20 @@ vi.mock('@/platform/workspace/stores/billingOperationStore', () => ({
280280
vi.mock('@/platform/workspace/stores/teamWorkspaceStore', async () => {
281281
const { ref } = await import('vue')
282282
const activeWorkspaceId = ref('workspace-1')
283+
const workspaceTransitionGeneration = ref(0)
283284
mockSetActiveWorkspaceIdImpl.value = (workspaceId) => {
285+
if (activeWorkspaceId.value !== workspaceId) {
286+
workspaceTransitionGeneration.value++
287+
}
284288
activeWorkspaceId.value = workspaceId
285289
}
286290
return {
287291
useTeamWorkspaceStore: () => ({
288292
get activeWorkspaceId() {
289293
return activeWorkspaceId.value
294+
},
295+
get workspaceTransitionGeneration() {
296+
return workspaceTransitionGeneration.value
290297
}
291298
})
292299
}
@@ -1340,6 +1347,43 @@ describe('useSubscriptionCheckout', () => {
13401347
)
13411348
})
13421349

1350+
it('does not show synchronous success after switching away and back', async () => {
1351+
const checkout = await setup()
1352+
await checkout.handleSubscribeTeamClick({
1353+
stop: {
1354+
id: 'team_700',
1355+
usd: 700,
1356+
credits: 147_700,
1357+
discountedUsd: 665
1358+
},
1359+
billingCycle: 'monthly'
1360+
})
1361+
let resolveSubscribe!: (response: {
1362+
status: 'subscribed'
1363+
billing_op_id: string
1364+
}) => void
1365+
mockSubscribe.mockReturnValueOnce(
1366+
new Promise((resolve) => {
1367+
resolveSubscribe = resolve
1368+
})
1369+
)
1370+
1371+
const subscription = checkout.handleTeamSubscribe()
1372+
await vi.waitFor(() => expect(mockSubscribe).toHaveBeenCalledOnce())
1373+
mockSetActiveWorkspaceId('workspace-2')
1374+
mockSetActiveWorkspaceId('workspace-1')
1375+
resolveSubscribe({
1376+
status: 'subscribed',
1377+
billing_op_id: 'op-team-1'
1378+
})
1379+
await subscription
1380+
1381+
expect(checkout.checkoutStep.value).not.toBe('success')
1382+
expect(mockTrackBillingEvent).not.toHaveBeenCalledWith(
1383+
expect.objectContaining({ stage: 'succeeded' })
1384+
)
1385+
})
1386+
13431387
it('forwards confirmReactivation true when the disclosure banner reports consent', async () => {
13441388
const checkout = await setup()
13451389
await checkout.handleSubscribeTeamClick({

src/platform/workspace/composables/useSubscriptionCheckout.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -986,6 +986,10 @@ export function useSubscriptionCheckout(
986986
billingCycle
987987
})
988988
}
989+
const workspaceIdentity = {
990+
id: workspaceStore.activeWorkspaceId,
991+
transitionGeneration: workspaceStore.workspaceTransitionGeneration
992+
}
989993
const response = await subscribe(planSlug, {
990994
teamCreditStopId: stop.id,
991995
billingCycle,
@@ -996,6 +1000,14 @@ export function useSubscriptionCheckout(
9961000
? previewData.value.proration_at
9971001
: undefined
9981002
})
1003+
if (
1004+
workspaceIdentity.id !== workspaceStore.activeWorkspaceId ||
1005+
workspaceIdentity.transitionGeneration !==
1006+
workspaceStore.workspaceTransitionGeneration
1007+
) {
1008+
activeCheckoutAttemptStartedAt = undefined
1009+
return
1010+
}
9991011

10001012
if (response) {
10011013
trackWorkspaceCheckoutStarted({

src/platform/workspace/stores/billingOperationStore.test.ts

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,37 @@ describe('billingOperationStore', () => {
368368
expect(mockSettingsDialogShow).toHaveBeenCalledWith('workspace')
369369
})
370370

371+
it('does not refresh the new workspace when switching during reconciliation', async () => {
372+
let finishStatusRefresh: () => void = () => {}
373+
mockFetchStatus.mockImplementationOnce(
374+
() =>
375+
new Promise<void>((resolve) => {
376+
finishStatusRefresh = resolve
377+
})
378+
)
379+
vi.mocked(workspaceApi.getBillingOpStatus).mockResolvedValue({
380+
id: 'op-1',
381+
status: 'succeeded',
382+
started_at: new Date().toISOString()
383+
})
384+
385+
const store = useBillingOperationStore()
386+
const terminal = store.startOperation('op-1', 'topup')
387+
await vi.advanceTimersByTimeAsync(0)
388+
expect(mockFetchStatus).toHaveBeenCalledOnce()
389+
390+
mockActiveWorkspaceId.value = 'workspace-2'
391+
finishStatusRefresh()
392+
await terminal
393+
394+
expect(mockFetchBalance).not.toHaveBeenCalled()
395+
expect(mockCloseDialog).not.toHaveBeenCalled()
396+
expect(mockSettingsDialogShow).not.toHaveBeenCalled()
397+
expect(mockToastAdd).not.toHaveBeenCalledWith(
398+
expect.objectContaining({ severity: 'success' })
399+
)
400+
})
401+
371402
it('opens Credits settings after a polled local topup succeeds', async () => {
372403
mockDistributionTypes.isCloud = false
373404
vi.mocked(workspaceApi.getBillingOpStatus).mockResolvedValue({
@@ -956,7 +987,7 @@ describe('billingOperationStore', () => {
956987
})
957988

958989
describe('polling timeout', () => {
959-
it('times out a subscription while its workspace is inactive', async () => {
990+
it('does not time out a subscription while its workspace is inactive', async () => {
960991
vi.mocked(workspaceApi.getBillingOpStatus).mockResolvedValue({
961992
id: 'op-1',
962993
status: 'pending',
@@ -969,7 +1000,10 @@ describe('billingOperationStore', () => {
9691000

9701001
await vi.advanceTimersByTimeAsync(5 * 60_000 + 8001)
9711002

972-
expect(store.getOperation('op-1')?.status).toBe('timeout')
1003+
expect(store.getOperation('op-1')?.status).toBe('pending')
1004+
expect(mockToastAdd).not.toHaveBeenCalledWith(
1005+
expect.objectContaining({ severity: 'error' })
1006+
)
9731007
})
9741008

9751009
it('restarts a subscription operation after a polling timeout', async () => {

0 commit comments

Comments
 (0)