Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions .github/workflows/ci-dist-telemetry-scan.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
name: 'CI: Dist Telemetry Scan'

on:
pull_request:
branches-ignore: [wip/*, draft/*, temp/*]

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

permissions:
contents: read

jobs:
scan:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1

Check warning on line 19 in .github/workflows/ci-dist-telemetry-scan.yaml

View workflow job for this annotation

GitHub Actions / yaml-lint

19:73 [comments] too few spaces before comment: expected 2

- name: Install pnpm
uses: pnpm/action-setup@41ff72655975bd51cab0327fa583b6e92b6d3061 # v4.2.0

Check warning on line 22 in .github/workflows/ci-dist-telemetry-scan.yaml

View workflow job for this annotation

GitHub Actions / yaml-lint

22:74 [comments] too few spaces before comment: expected 2
with:
version: 10

- name: Use Node.js
uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0

Check warning on line 27 in .github/workflows/ci-dist-telemetry-scan.yaml

View workflow job for this annotation

GitHub Actions / yaml-lint

27:75 [comments] too few spaces before comment: expected 2
with:
node-version: 'lts/*'
cache: 'pnpm'

- name: Install dependencies
run: pnpm install --frozen-lockfile

- name: Build project
run: pnpm build

- name: Scan dist for telemetry references
run: |
set -euo pipefail
if rg --no-ignore -n \
-g '*.html' \
-g '*.js' \
-e 'Google Tag Manager' \
-e '(?i)\bgtm\.js\b' \
-e '(?i)googletagmanager\.com/gtm\.js\\?id=' \
-e '(?i)googletagmanager\.com/ns\.html\\?id=' \
dist; then
echo 'Telemetry references found in dist assets.'
exit 1
fi
echo 'No telemetry references found in dist assets.'
14 changes: 14 additions & 0 deletions global.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,14 @@ declare const __ALGOLIA_APP_ID__: string
declare const __ALGOLIA_API_KEY__: string
declare const __USE_PROD_CONFIG__: boolean

interface ImpactQueueFunction {
(...args: unknown[]): void
a?: unknown[][]
}

interface Window {
__CONFIG__: {
gtm_container_id?: string
mixpanel_token?: string
require_whitelist?: boolean
subscription_required?: boolean
Expand All @@ -30,6 +36,14 @@ interface Window {
badge?: string
}
}
__ga_identity__?: {
client_id?: string
session_id?: string
session_number?: string
}
dataLayer?: Array<Record<string, unknown>>
ire_o?: string
ire?: ImpactQueueFunction
}

interface Navigator {
Expand Down
5 changes: 4 additions & 1 deletion src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,15 @@ import { i18n } from './i18n'
* CRITICAL: Load remote config FIRST for cloud builds to ensure
* window.__CONFIG__is available for all modules during initialization
*/
import { isCloud } from '@/platform/distribution/types'
const isCloud = __DISTRIBUTION__ === 'cloud'

if (isCloud) {
const { refreshRemoteConfig } =
await import('@/platform/remoteConfig/refreshRemoteConfig')
await refreshRemoteConfig({ useAuth: false })

const { initTelemetry } = await import('@/platform/telemetry/initTelemetry')
await initTelemetry()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Initialize telemetry after Pinia is installed

Calling initTelemetry() here runs provider constructors before createPinia()/app.use(pinia), but MixpanelTelemetryProvider's loaded callback invokes useCurrentUser() (which immediately calls Pinia stores). In cloud builds with a Mixpanel token, that callback can throw "no active Pinia", causing the provider's import chain to hit its error path and disable Mixpanel tracking for the session.

Useful? React with 👍 / 👎.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I want to fix this in main, then backport that PR instead of fixing it here:

  1. It's a race/intermittent
  2. This only affects a fallback branch of Impact telemetry
    a. Per Impact's technical guide, we only need the generated ref for subscription attribution. And Pinia only provides the fallback as customerId + customerEmail
  3. I don't want to block the entirety of telemetry based on this, since the impact isn't high
  4. I don't want to muddy already reviewed code. This PR is basically a clean cherry pick, I don't want to add new code to the staging branch.

}

const ComfyUIPreset = definePreset(Aura, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ vi.mock('@/composables/useErrorHandling', () => ({

const subscriptionMocks = vi.hoisted(() => ({
isActiveSubscription: { value: false },
isInitialized: { value: true }
isInitialized: { value: true },
subscriptionStatus: { value: null }
}))

vi.mock('@/platform/cloud/subscription/composables/useSubscription', () => ({
Expand Down
62 changes: 57 additions & 5 deletions src/platform/cloud/subscription/components/PricingTable.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { createTestingPinia } from '@pinia/testing'
import { flushPromises, mount } from '@vue/test-utils'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { computed, ref } from 'vue'
import { computed, reactive, ref } from 'vue'
import { createI18n } from 'vue-i18n'

import PricingTable from '@/platform/cloud/subscription/components/PricingTable.vue'
Expand All @@ -14,15 +14,19 @@ const mockSubscriptionTier = ref<
const mockIsYearlySubscription = ref(false)
const mockAccessBillingPortal = vi.fn()
const mockReportError = vi.fn()
const mockTrackBeginCheckout = vi.fn()
const mockUserId = ref<string | undefined>('user-123')
const mockGetFirebaseAuthHeader = vi.fn(() =>
Promise.resolve({ Authorization: 'Bearer test-token' })
)
const mockGetCheckoutAttribution = vi.hoisted(() => vi.fn(() => ({})))

vi.mock('@/platform/cloud/subscription/composables/useSubscription', () => ({
useSubscription: () => ({
isActiveSubscription: computed(() => mockIsActiveSubscription.value),
subscriptionTier: computed(() => mockSubscriptionTier.value),
isYearlySubscription: computed(() => mockIsYearlySubscription.value)
isYearlySubscription: computed(() => mockIsYearlySubscription.value),
subscriptionStatus: ref(null)
})
}))

Expand Down Expand Up @@ -52,12 +56,24 @@ vi.mock('@/composables/useErrorHandling', () => ({
}))

vi.mock('@/stores/firebaseAuthStore', () => ({
useFirebaseAuthStore: () => ({
getFirebaseAuthHeader: mockGetFirebaseAuthHeader
}),
useFirebaseAuthStore: () =>
reactive({
getFirebaseAuthHeader: mockGetFirebaseAuthHeader,
userId: computed(() => mockUserId.value)
}),
FirebaseAuthStoreError: class extends Error {}
}))

vi.mock('@/platform/telemetry', () => ({
useTelemetry: () => ({
trackBeginCheckout: mockTrackBeginCheckout
})
}))

vi.mock('@/platform/telemetry/utils/checkoutAttribution', () => ({
getCheckoutAttribution: mockGetCheckoutAttribution
}))

vi.mock('@/platform/distribution/types', () => ({
isCloud: true
}))
Expand Down Expand Up @@ -126,6 +142,8 @@ describe('PricingTable', () => {
mockIsActiveSubscription.value = false
mockSubscriptionTier.value = null
mockIsYearlySubscription.value = false
mockUserId.value = 'user-123'
mockTrackBeginCheckout.mockReset()
vi.mocked(global.fetch).mockResolvedValue({
ok: true,
json: async () => ({ checkout_url: 'https://checkout.stripe.com/test' })
Expand All @@ -148,6 +166,13 @@ describe('PricingTable', () => {
await creatorButton?.trigger('click')
await flushPromises()

expect(mockTrackBeginCheckout).toHaveBeenCalledWith({
user_id: 'user-123',
tier: 'creator',
cycle: 'yearly',
checkout_type: 'change',
previous_tier: 'standard'
})
expect(mockAccessBillingPortal).toHaveBeenCalledWith('creator-yearly')
})

Expand All @@ -168,6 +193,33 @@ describe('PricingTable', () => {
expect(mockAccessBillingPortal).toHaveBeenCalledWith('pro-yearly')
})

it('should use the latest userId value when it changes after mount', async () => {
mockIsActiveSubscription.value = true
mockSubscriptionTier.value = 'STANDARD'
mockUserId.value = 'user-early'

const wrapper = createWrapper()
await flushPromises()

mockUserId.value = 'user-late'

const creatorButton = wrapper
.findAll('button')
.find((btn) => btn.text().includes('Creator'))

await creatorButton?.trigger('click')
await flushPromises()

expect(mockTrackBeginCheckout).toHaveBeenCalledTimes(1)
expect(mockTrackBeginCheckout).toHaveBeenCalledWith({
user_id: 'user-late',
tier: 'creator',
cycle: 'yearly',
checkout_type: 'change',
previous_tier: 'standard'
})
})

it('should not call accessBillingPortal when clicking current plan', async () => {
mockIsActiveSubscription.value = true
mockSubscriptionTier.value = 'CREATOR'
Expand Down
38 changes: 37 additions & 1 deletion src/platform/cloud/subscription/components/PricingTable.vue
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@

<script setup lang="ts">
import { cn } from '@comfyorg/tailwind-utils'
import { storeToRefs } from 'pinia'
import Popover from 'primevue/popover'
import SelectButton from 'primevue/selectbutton'
import type { ToggleButtonPassThroughMethodOptions } from 'primevue/togglebutton'
Expand All @@ -266,6 +267,9 @@ import { performSubscriptionCheckout } from '@/platform/cloud/subscription/utils
import { isPlanDowngrade } from '@/platform/cloud/subscription/utils/subscriptionTierRank'
import type { BillingCycle } from '@/platform/cloud/subscription/utils/subscriptionTierRank'
import { isCloud } from '@/platform/distribution/types'
import { useTelemetry } from '@/platform/telemetry'
import type { CheckoutAttributionMetadata } from '@/platform/telemetry/types'
import { useFirebaseAuthStore } from '@/stores/firebaseAuthStore'
import type { components } from '@/types/comfyRegistryTypes'

type SubscriptionTier = components['schemas']['SubscriptionTier']
Expand All @@ -277,6 +281,19 @@ const getCheckoutTier = (
billingCycle: BillingCycle
): CheckoutTier => (billingCycle === 'yearly' ? `${tierKey}-yearly` : tierKey)

const getCheckoutAttributionForCloud =
async (): Promise<CheckoutAttributionMetadata> => {
// eslint-disable-next-line no-undef
if (__DISTRIBUTION__ !== 'cloud') {
return {}
}

const { getCheckoutAttribution } =
await import('@/platform/telemetry/utils/checkoutAttribution')

return getCheckoutAttribution()
}

interface BillingCycleOption {
label: string
value: BillingCycle
Expand Down Expand Up @@ -330,6 +347,8 @@ const tiers: PricingTierConfig[] = [
const { n } = useI18n()
const { isActiveSubscription, subscriptionTier, isYearlySubscription } =
useSubscription()
const telemetry = useTelemetry()
const { userId } = storeToRefs(useFirebaseAuthStore())
const { accessBillingPortal, reportError } = useFirebaseAuthActions()
const { wrapWithErrorHandlingAsync } = useErrorHandling()

Expand Down Expand Up @@ -410,6 +429,19 @@ const handleSubscribe = wrapWithErrorHandlingAsync(

try {
if (isActiveSubscription.value) {
const checkoutAttribution = await getCheckoutAttributionForCloud()
if (userId.value) {
telemetry?.trackBeginCheckout({
user_id: userId.value,
Comment on lines 431 to +435
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getCheckoutAttributionForCloud() is awaited in the active-subscription (change) flow without any best-effort fallback. If the dynamic import/chunk load fails, this will block accessBillingPortal() and prevent users from changing plans. Match performSubscriptionCheckout by wrapping attribution collection in a try/catch and proceeding with {} when it fails.

Copilot uses AI. Check for mistakes.
tier: tierKey,
cycle: currentBillingCycle.value,
checkout_type: 'change',
...checkoutAttribution,
...(currentTierKey.value
? { previous_tier: currentTierKey.value }
: {})
})
}
// Pass the target tier to create a deep link to subscription update confirmation
const checkoutTier = getCheckoutTier(tierKey, currentBillingCycle.value)
const targetPlan = {
Expand All @@ -430,7 +462,11 @@ const handleSubscribe = wrapWithErrorHandlingAsync(
await accessBillingPortal(checkoutTier)
}
} else {
await performSubscriptionCheckout(tierKey, currentBillingCycle.value)
await performSubscriptionCheckout(
tierKey,
currentBillingCycle.value,
true
)
}
} finally {
isLoading.value = false
Expand Down
Loading
Loading