fix: route gtm through telemetry entrypoint#8354
Conversation
📝 WalkthroughWalkthroughAdds a registry-based telemetry system (Mixpanel + GTM), cloud-only init, GTM runtime/types and remote-config flag, pageview/auth/checkout tracking and attribution, TelemetryRegistry and GTM provider, checkout-attribution util and tests, subscription checkout telemetry hooks/tests, ComfyWorkflow extraction, and a CI dist telemetry scan workflow. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App Startup
participant Main as src/main.ts
participant Init as initTelemetry()
participant Index as telemetry index
participant Registry as TelemetryRegistry
participant Mixpanel as MixpanelProvider
participant GTM as GtmTelemetryProvider
App->>Main: boot (cloud build)
Main->>Init: dynamic import & call initTelemetry()
activate Init
Init->>Registry: create TelemetryRegistry
Registry->>Mixpanel: registerProvider(Mixpanel)
Registry->>GTM: registerProvider(GTM)
GTM->>GTM: initialize() (ensure window.dataLayer, inject GTM script)
Registry-->>Init: registry ready
Init->>Index: setTelemetryRegistry(registry)
deactivate Init
sequenceDiagram
participant User as User
participant Router as src/router.ts
participant Telemetry as TelemetryRegistry
participant GTM as GtmTelemetryProvider
participant Mixpanel as MixpanelProvider
participant DataLayer as window.dataLayer
User->>Router: navigate
Router->>Telemetry: trackPageView(title, path)
activate Telemetry
Telemetry->>GTM: trackPageView(...)
GTM->>DataLayer: push page_view event
Telemetry->>Mixpanel: trackPageView(...)
deactivate Telemetry
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
88677aa to
563cdbf
Compare
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 02/05/2026, 06:56:12 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Tests:
|
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 22.4 kB (baseline 22.5 kB) • 🟢 -104 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 852 kB (baseline 840 kB) • 🔴 +12.3 kBGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 69 kB (baseline 69 kB) • 🔴 +42 BTop-level views, pages, and routed surfaces
Status: 11 added / 11 removed Panels & Settings — 410 kB (baseline 410 kB) • 🔴 +81 BConfiguration panels, inspectors, and settings screens
Status: 23 added / 24 removed User & Accounts — 16.1 kB (baseline 16 kB) • 🔴 +80 BAuthentication, profile, and account management bundles
Status: 7 added / 7 removed Editors & Dialogs — 3.57 kB (baseline 3.47 kB) • 🔴 +104 BModals, dialogs, drawers, and in-app editors
Status: 2 added / 2 removed UI Components — 38 kB (baseline 37.8 kB) • 🔴 +205 BReusable component library chunks
Status: 10 added / 10 removed Data & Services — 2.06 MB (baseline 2.1 MB) • 🟢 -39.9 kBStores, services, APIs, and repositories
Status: 15 added / 13 removed Utilities & Hooks — 237 kB (baseline 234 kB) • 🔴 +2.7 kBHelpers, composables, and utility bundles
Status: 20 added / 19 removed Vendor & Third-Party — 8.77 MB (baseline 9.37 MB) • 🟢 -601 kBExternal libraries and shared vendor chunks
Status: 8 added / 8 removed Other — 7.1 MB (baseline 7.08 MB) • 🔴 +12.8 kBBundles that do not match a named category
Status: 125 added / 123 removed |
There was a problem hiding this comment.
Pull request overview
This PR consolidates Google Tag Manager (GTM) initialization and data-layer event tracking behind the telemetry entrypoint to enable tree-shaking of GTM code in non-cloud builds. The implementation uses compile-time constants (__DISTRIBUTION__) and dynamic imports to ensure GTM dependencies are excluded from OSS builds while maintaining full functionality in cloud builds.
Changes:
- Created new
src/platform/telemetry/gtm.tsmodule with GTM initialization and event pushing logic - Updated
src/platform/telemetry/index.tsto exposeinitGtm()andpushDataLayerEvent()functions that dynamically import GTM only in cloud builds - Added subscription purchase tracking with localStorage-based pending purchase tracker
- Added sign-up tracking with SHA-256 hashed user IDs for email, Google, and GitHub authentication methods
- Added page view tracking on route navigation
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/platform/telemetry/index.ts |
Added GTM module loading with dynamic imports and cloud build guards |
src/platform/telemetry/gtm.ts |
New file containing GTM initialization and dataLayer event pushing logic |
src/main.ts |
Added GTM initialization call in cloud builds |
src/router.ts |
Added page view tracking on route changes |
src/stores/firebaseAuthStore.ts |
Added sign-up tracking with hashed user IDs |
src/platform/cloud/subscription/utils/subscriptionPurchaseTracker.ts |
New utility for tracking pending subscription purchases via localStorage |
src/platform/cloud/subscription/utils/subscriptionCheckoutUtil.ts |
Added call to start subscription purchase tracking |
src/platform/cloud/subscription/composables/useSubscription.ts |
Added purchase event tracking when subscription completes |
src/platform/cloud/subscription/composables/useSubscription.test.ts |
Added test coverage for purchase tracking and updated test structure with effectScope |
global.d.ts |
Added TypeScript definition for window.dataLayer |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/platform/cloud/subscription/composables/useSubscription.test.ts (1)
101-130: Potential test isolation issue:window.dataLayernot cleaned up between tests.The
mockPushDataLayerEventimplementation (lines 115-118) pushes towindow.dataLayer, and line 248 initializes it to an empty array. However, there's no cleanup ofwindow.dataLayerinafterEach, which could cause state leakage between tests.Also,
scope?.stop()on line 107 inbeforeEachis redundant sinceafterEachalready handles scope cleanup.🧹 Proposed fix: Add dataLayer cleanup and remove redundant stop
afterEach(() => { scope?.stop() scope = undefined + window.dataLayer = undefined }) beforeEach(() => { - scope?.stop() scope = effectScope() vi.clearAllMocks()
🤖 Fix all issues with AI agents
In `@src/platform/cloud/subscription/composables/useSubscription.ts`:
- Around line 242-247: The call to trackSubscriptionPurchase is synchronous
(returns void) but is being awaited; remove the unnecessary await in the try
block (replace "await trackSubscriptionPurchase(statusData)" with
"trackSubscriptionPurchase(statusData)") so the code correctly reflects
synchronous behavior while preserving the existing try/catch error handling
around the trackSubscriptionPurchase invocation.
In `@src/platform/cloud/subscription/utils/subscriptionPurchaseTracker.ts`:
- Around line 12-13: Replace the hardcoded VALID_TIERS and VALID_CYCLES arrays
in subscriptionPurchaseTracker.ts by importing exported constants from the
source-of-truth files: add export const TIER_KEYS: TierKey[] = [...] to
tierPricing.ts and export const BILLING_CYCLES: BillingCycle[] = [...] to
subscriptionTierRank.ts, then remove the local definitions of VALID_TIERS and
VALID_CYCLES and import TIER_KEYS and BILLING_CYCLES into
subscriptionPurchaseTracker.ts (use these imported symbols wherever VALID_TIERS
and VALID_CYCLES were used, preserving types and behavior).
In `@src/platform/telemetry/gtm.ts`:
- Around line 8-37: Change initGtm from a void function to return the
initialization Promise so callers can await GTM readiness: have initGtm return
initPromise (Promise<void>) when created or return a resolved Promise if already
initialized; reference the existing symbols initGtm, initPromise, isInitialized
and finalize to locate the logic that creates and resolves the promise. Also
adjust the error handler on the script element (the listener that currently
calls finalize on 'error') to still resolve for telemetry resilience but emit a
development-only warning (e.g., when NODE_ENV !== 'production' or a dev flag) so
load failures are visible during debugging.
src/platform/cloud/subscription/utils/subscriptionPurchaseTracker.ts
Outdated
Show resolved
Hide resolved
src/platform/telemetry/gtm.ts
Outdated
| export function initGtm(): void { | ||
| if (!isCloud || typeof window === 'undefined') return | ||
| if (typeof document === 'undefined') return | ||
| if (isInitialized) return | ||
|
|
||
| if (!initPromise) { | ||
| initPromise = new Promise((resolve) => { | ||
| const dataLayer = window.dataLayer ?? (window.dataLayer = []) | ||
| dataLayer.push({ | ||
| 'gtm.start': Date.now(), | ||
| event: 'gtm.js' | ||
| }) | ||
|
|
||
| const script = document.createElement('script') | ||
| script.async = true | ||
| script.src = `https://www.googletagmanager.com/gtm.js?id=${GTM_CONTAINER_ID}` | ||
|
|
||
| const finalize = () => { | ||
| isInitialized = true | ||
| resolve() | ||
| } | ||
|
|
||
| script.addEventListener('load', finalize, { once: true }) | ||
| script.addEventListener('error', finalize, { once: true }) | ||
| document.head?.appendChild(script) | ||
| }) | ||
| } | ||
|
|
||
| void initPromise | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
initGtm returns void but internal promise is never exposed to callers.
The function creates initPromise but the void initPromise on line 36 is a no-op that simply suppresses the floating-promise lint. Callers cannot await GTM initialization completion. If callers need to wait for GTM to be ready before pushing events, consider returning the promise.
Additionally, script load errors are silently swallowed (line 31 resolves on error). This is acceptable for telemetry but consider logging a warning in development for debuggability.
♻️ Optional: Return promise for callers who need to await
-export function initGtm(): void {
+export function initGtm(): Promise<void> {
if (!isCloud || typeof window === 'undefined') return
+ return Promise.resolve()
if (typeof document === 'undefined') return
+ return Promise.resolve()
if (isInitialized) return
+ return Promise.resolve()
if (!initPromise) {
initPromise = new Promise((resolve) => {
// ... existing code ...
})
}
- void initPromise
+ return initPromise
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function initGtm(): void { | |
| if (!isCloud || typeof window === 'undefined') return | |
| if (typeof document === 'undefined') return | |
| if (isInitialized) return | |
| if (!initPromise) { | |
| initPromise = new Promise((resolve) => { | |
| const dataLayer = window.dataLayer ?? (window.dataLayer = []) | |
| dataLayer.push({ | |
| 'gtm.start': Date.now(), | |
| event: 'gtm.js' | |
| }) | |
| const script = document.createElement('script') | |
| script.async = true | |
| script.src = `https://www.googletagmanager.com/gtm.js?id=${GTM_CONTAINER_ID}` | |
| const finalize = () => { | |
| isInitialized = true | |
| resolve() | |
| } | |
| script.addEventListener('load', finalize, { once: true }) | |
| script.addEventListener('error', finalize, { once: true }) | |
| document.head?.appendChild(script) | |
| }) | |
| } | |
| void initPromise | |
| } | |
| export function initGtm(): Promise<void> { | |
| if (!isCloud || typeof window === 'undefined') return Promise.resolve() | |
| if (typeof document === 'undefined') return Promise.resolve() | |
| if (isInitialized) return Promise.resolve() | |
| if (!initPromise) { | |
| initPromise = new Promise((resolve) => { | |
| const dataLayer = window.dataLayer ?? (window.dataLayer = []) | |
| dataLayer.push({ | |
| 'gtm.start': Date.now(), | |
| event: 'gtm.js' | |
| }) | |
| const script = document.createElement('script') | |
| script.async = true | |
| script.src = `https://www.googletagmanager.com/gtm.js?id=${GTM_CONTAINER_ID}` | |
| const finalize = () => { | |
| isInitialized = true | |
| resolve() | |
| } | |
| script.addEventListener('load', finalize, { once: true }) | |
| script.addEventListener('error', finalize, { once: true }) | |
| document.head?.appendChild(script) | |
| }) | |
| } | |
| return initPromise | |
| } |
🤖 Prompt for AI Agents
In `@src/platform/telemetry/gtm.ts` around lines 8 - 37, Change initGtm from a
void function to return the initialization Promise so callers can await GTM
readiness: have initGtm return initPromise (Promise<void>) when created or
return a resolved Promise if already initialized; reference the existing symbols
initGtm, initPromise, isInitialized and finalize to locate the logic that
creates and resolves the promise. Also adjust the error handler on the script
element (the listener that currently calls finalize on 'error') to still resolve
for telemetry resilience but emit a development-only warning (e.g., when
NODE_ENV !== 'production' or a dev flag) so load failures are visible during
debugging.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2db667bde4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/platform/cloud/subscription/utils/subscriptionPurchaseTracker.ts
Outdated
Show resolved
Hide resolved
|
The dist telemetry sniffing code can be made more strict by modifying the regex. |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/platform/telemetry/types.ts (1)
470-493:⚠️ Potential issue | 🟡 MinorMissing
PageViewMetadataandBeginCheckoutMetadatain union type.The
TelemetryEventPropertiesunion type does not include the newly addedPageViewMetadataandBeginCheckoutMetadatatypes. If this union is used for type-checking event properties elsewhere, it should include these new metadata types for completeness.🔧 Proposed fix
export type TelemetryEventProperties = | AuthMetadata | SurveyResponses | TemplateMetadata | ExecutionContext | RunButtonProperties | ExecutionErrorMetadata | ExecutionSuccessMetadata | CreditTopupMetadata | WorkflowImportMetadata | TemplateLibraryMetadata | TemplateLibraryClosedMetadata | PageVisibilityMetadata | TabCountMetadata | NodeSearchMetadata | NodeSearchResultMetadata | TemplateFilterMetadata | SettingChangedMetadata | UiButtonClickMetadata | HelpCenterOpenedMetadata | HelpResourceClickedMetadata | HelpCenterClosedMetadata | WorkflowCreatedMetadata | EnterLinearMetadata + | PageViewMetadata + | BeginCheckoutMetadata
🧹 Nitpick comments (1)
src/platform/cloud/subscription/components/PricingTable.test.ts (1)
205-233: Consider adding telemetry assertion for new subscriber flow.This test verifies new subscribers go through the checkout flow but doesn't assert on
trackBeginCheckout. SincesubscriptionCheckoutUtil.tsnow tracks checkout for new subscribers, consider adding an assertion to verify telemetry is called withcheckout_type: 'new'.💡 Suggested addition
expect(windowOpenSpy).toHaveBeenCalledWith( 'https://checkout.stripe.com/test', '_blank' ) + + expect(mockTrackBeginCheckout).toHaveBeenCalledWith({ + user_id: 'user-123', + tier: expect.any(String), + cycle: 'yearly', + checkout_type: 'new' + }) windowOpenSpy.mockRestore()
simula-r
left a comment
There was a problem hiding this comment.
LGTM. Nice job on this.
PR changed direction, last review was from Jake
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
## Summary Enhances the existing CI telemetry scan workflow to also detect Mixpanel code in dist files, ensuring it's properly tree-shaken from OSS builds. ## Context - Extends existing `ci-dist-telemetry-scan.yaml` (added in PR #8354) - Based on analysis in closed PR #6777 (split into focused PRs) - Complements GTM detection already in place - Part of comprehensive OSS compliance effort ## Implementation - Adds separate Mixpanel check step with specific patterns: - `mixpanel.init` - `mixpanel.identify` - `MixpanelTelemetryProvider` - `mp.comfy.org` - `mixpanel-browser` - `mixpanel.track(` - Separates GTM and Mixpanel checks for clarity - Adds `DISTRIBUTION=localhost` env var to build step - Excludes source maps from scanning ## Related - Supersedes part of closed PR #6777 - Complements existing GTM check from PR #8354 - Related to PR #8623 (GTM-focused, may be redundant) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8826-Add-Mixpanel-detection-to-telemetry-tree-shaking-validation-3056d73d36508153bab5f55d4bb17658) by [Unito](https://www.unito.io) --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
## Summary Enhances the existing CI telemetry scan workflow to also detect Mixpanel code in dist files, ensuring it's properly tree-shaken from OSS builds. ## Context - Extends existing `ci-dist-telemetry-scan.yaml` (added in PR #8354) - Based on analysis in closed PR #6777 (split into focused PRs) - Complements GTM detection already in place - Part of comprehensive OSS compliance effort ## Implementation - Adds separate Mixpanel check step with specific patterns: - `mixpanel.init` - `mixpanel.identify` - `MixpanelTelemetryProvider` - `mp.comfy.org` - `mixpanel-browser` - `mixpanel.track(` - Separates GTM and Mixpanel checks for clarity - Adds `DISTRIBUTION=localhost` env var to build step - Excludes source maps from scanning ## Related - Supersedes part of closed PR #6777 - Complements existing GTM check from PR #8354 - Related to PR #8623 (GTM-focused, may be redundant) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8826-Add-Mixpanel-detection-to-telemetry-tree-shaking-validation-3056d73d36508153bab5f55d4bb17658) by [Unito](https://www.unito.io) --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
## Summary Adds CI workflow to validate OSS build compliance by checking for proprietary fonts and non-approved dependency licenses. ## Context - Part of comprehensive OSS compliance effort (split from closed PR #6777) - Uses simple bash/grep approach following proven #8623 pattern - Complements telemetry checking in PR #8826 and existing #8354 ## Implementation ### Font Validation - Scans dist/ for proprietary ABCROM fonts (.woff, .woff2, .ttf, .otf) - Fails if any ABCROM fonts found in OSS builds - Provides clear fix instructions ### License Validation - Uses `license-checker` npm package - Validates all production dependencies - Only allows OSI-approved licenses: - MIT, Apache-2.0, BSD-2-Clause, BSD-3-Clause, ISC - 0BSD, BlueOak-1.0.0, Python-2.0, CC0-1.0 - Unlicense, CC-BY-4.0, CC-BY-3.0 - Common dual-license combinations ### Workflow Details - Two separate jobs for parallel execution - Runs on PRs and pushes to main/dev - Builds with `DISTRIBUTION=localhost` for OSS mode - Clear error messages with remediation steps ## Testing - [ ] Font check passes on current main (no ABCROM fonts in dist) - [ ] License check passes on current main (all approved licenses) - [ ] Intentional violation testing ## Related - Supersedes remaining parts of closed PR #6777 - Complements PR #8826 (Mixpanel telemetry) - Follows pattern from PR #8623 (simple bash/grep) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8828-Add-CI-validation-for-OSS-assets-fonts-and-licenses-3056d73d3650812390d5d91ca2f319fc) by [Unito](https://www.unito.io) --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: bymyself <cbyrne@comfy.org>

Wire checkout attribution into GTM events and checkout POST payloads.
This updates the cloud telemetry flow so the backend team can correlate checkout events without relying on frontend cookie parsing. We now surface GA4 identity via a GTM-provided global and include attribution on both
begin_checkouttelemetry and the checkout POST body. The backend should continue to derive the Firebase UID from the auth header; the checkout POST body does not include a user ID.GTM events pushed (unchanged list, updated payloads):
page_view(page title/location/referrer as before)sign_up/loginbegin_checkoutnow includes:user_id,tier,cycle,checkout_type,previous_tier(if change flow)ga_client_id,ga_session_id,ga_session_numbergclid,gbraid,wbraidBackend-facing change:
POST /customers/cloud-subscription-checkout/:tiernow includes a JSON body with attribution fields only:ga_client_id,ga_session_id,ga_session_numbergclid,gbraid,wbraidRequired GTM setup:
window.__ga_identity__via a GTM Custom HTML tag (after GA4/Google tag) with{ client_id, session_id, session_number }. The frontend reads this to populate the GA fields.Screenshots (if applicable)
Manual Testing:




To manually test, you will need to override api/features in devtools to also return this:
┆Issue is synchronized with this Notion page by Unito
Summary by CodeRabbit
New Features
Chores
Tests
CI