feat: add PostHog event tracking across apps#476
Conversation
Add custom event tracking for key user actions: - Marketing: download_clicked, waitlist_clicked - Desktop: desktop_opened, auth_started, auth_completed, workspace_created/opened/closed/deleted, terminal_opened - Fix user identity to use database user ID consistently across all apps (was using Clerk ID in web/admin which differed from desktop) Also adds posthog-node for reliable server-side tracking in desktop main process. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
WalkthroughIntegrates PostHog analytics across the application by adding a desktop PostHog client, replacing Clerk-based user identification with TRPC queries in PostHogUserIdentifier components, introducing analytics tracking for workspace and terminal lifecycle events, and configuring required environment variables. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
1-27: Consider extracting shared logic.This component is nearly identical to
apps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx. If maintaining these in sync becomes burdensome, consider extracting the shared logic into a utility or shared package.apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (1)
1079-1083: Consider addingwas_existingproperty for consistency.The
workspace_openedevent for worktree workspaces is missing thewas_existingproperty that's present in the branch workspace version (line 286). For worktrees, this would always befalsesinceopenWorktreethrows if the workspace already exists (line 1014), but including it explicitly maintains consistency across event schemas.Proposed change for consistency
track("workspace_opened", { workspace_id: workspace.id, project_id: project.id, type: "worktree", + was_existing: false, });apps/desktop/src/main/lib/analytics/index.ts (1)
8-21: Consider adding initialization guard for concurrent calls.The
getClient()function could create multiple PostHog instances if called concurrently before the first initialization completes. While unlikely in practice (desktop app initialization is mostly sequential), adding a guard would make the code more robust.Proposed fix to prevent race conditions
let client: PostHog | null = null; +let clientInitializing = false; let cachedUserId: string | null = null; function getClient(): PostHog | null { if (!env.NEXT_PUBLIC_POSTHOG_KEY) { return null; } - if (!client) { + if (!client && !clientInitializing) { + clientInitializing = true; client = new PostHog(env.NEXT_PUBLIC_POSTHOG_KEY, { host: env.NEXT_PUBLIC_POSTHOG_HOST, flushAt: 1, // Send events immediately for desktop app flushInterval: 0, }); + clientInitializing = false; } return client; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
apps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/package.jsonapps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/desktop/src/main/env.main.tsapps/desktop/src/main/index.tsapps/desktop/src/main/lib/analytics/index.tsapps/desktop/src/main/lib/terminal/manager.test.tsapps/desktop/src/main/lib/terminal/manager.tsapps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/renderer/screens/sign-in/index.tsxapps/desktop/test-setup.tsapps/marketing/src/app/components/DownloadButton/DownloadButton.tsxapps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/main/index.tsapps/desktop/src/main/lib/terminal/manager.tsapps/desktop/src/main/lib/terminal/manager.test.tsapps/marketing/src/app/components/DownloadButton/DownloadButton.tsxapps/desktop/src/main/lib/analytics/index.tsapps/desktop/src/lib/trpc/routers/auth/index.tsapps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/main/env.main.tsapps/desktop/test-setup.tsapps/desktop/src/renderer/screens/sign-in/index.tsxapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix
Files:
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/main/index.tsapps/desktop/src/main/lib/terminal/manager.tsapps/desktop/src/main/lib/terminal/manager.test.tsapps/marketing/src/app/components/DownloadButton/DownloadButton.tsxapps/desktop/src/main/lib/analytics/index.tsapps/desktop/src/lib/trpc/routers/auth/index.tsapps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/main/env.main.tsapps/desktop/test-setup.tsapps/desktop/src/renderer/screens/sign-in/index.tsxapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx
**/{components,features}/**/[!.]*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Organize project structure with one folder per component: ComponentName/ComponentName.tsx with index.ts barrel export
Files:
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/marketing/src/app/components/DownloadButton/DownloadButton.tsxapps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx
**/{components,features}/**/*.{ts,tsx,test.ts,test.tsx,stories.tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Files:
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/marketing/src/app/components/DownloadButton/DownloadButton.tsxapps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)
Files:
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/renderer/screens/sign-in/index.tsx
apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Files:
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/main/index.tsapps/desktop/src/main/lib/terminal/manager.tsapps/desktop/src/main/lib/terminal/manager.test.tsapps/desktop/src/main/lib/analytics/index.tsapps/desktop/src/main/env.main.tsapps/desktop/src/renderer/screens/sign-in/index.tsx
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use React + TailwindCSS v4 + shadcn/ui for UI development
Files:
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/marketing/src/app/components/DownloadButton/DownloadButton.tsxapps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/renderer/screens/sign-in/index.tsxapps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx
**/{components,features}/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/{components,features}/**/*.tsx: Nest components in parent's components/ folder if used only once, promote to highest shared parent's components/ if used 2+ times
Use one component per file - do not combine multiple components in a single file
Files:
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/marketing/src/app/components/DownloadButton/DownloadButton.tsxapps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/main/index.tsapps/desktop/src/main/lib/terminal/manager.tsapps/desktop/src/main/lib/terminal/manager.test.tsapps/desktop/src/main/lib/analytics/index.tsapps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/main/env.main.tsapps/desktop/test-setup.tsapps/desktop/src/renderer/screens/sign-in/index.tsxapps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts
apps/desktop/src/main/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Accept object parameters in IPC handlers - do not use positional parameters in ipcMain.handle()
Files:
apps/desktop/src/main/index.tsapps/desktop/src/main/lib/terminal/manager.tsapps/desktop/src/main/lib/terminal/manager.test.tsapps/desktop/src/main/lib/analytics/index.tsapps/desktop/src/main/env.main.ts
apps/desktop/src/main/index.ts
📄 CodeRabbit inference engine (AGENTS.md)
Load environment variables in src/main/index.ts and electron.vite.config.ts with override: true before other imports
Files:
apps/desktop/src/main/index.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Place tests co-located with components using .test.ts or .test.tsx naming convention
Files:
apps/desktop/src/main/lib/terminal/manager.test.ts
🧠 Learnings (7)
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
Applied to files:
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/renderer/screens/sign-in/index.tsx
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to apps/desktop/src/main/**/*.ts : Accept object parameters in IPC handlers - do not use positional parameters in ipcMain.handle()
Applied to files:
apps/desktop/src/main/lib/terminal/manager.ts
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to **/*.test.{ts,tsx} : Place tests co-located with components using .test.ts or .test.tsx naming convention
Applied to files:
apps/desktop/src/main/lib/terminal/manager.test.ts
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to **/*.{tsx,css} : Use React + TailwindCSS v4 + shadcn/ui for UI development
Applied to files:
apps/desktop/package.json
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from `trpc/server/observable` instead of async generators, as the library explicitly checks `isObservable(result)` and throws an error otherwise
Applied to files:
apps/desktop/src/lib/trpc/routers/auth/index.ts
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`
Applied to files:
apps/desktop/src/lib/trpc/routers/auth/index.ts
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to apps/desktop/src/main/index.ts : Load environment variables in src/main/index.ts and electron.vite.config.ts with override: true before other imports
Applied to files:
apps/desktop/src/main/env.main.ts
🧬 Code graph analysis (9)
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
apps/desktop/src/renderer/lib/posthog.ts (1)
posthog(28-28)
apps/desktop/src/main/index.ts (2)
apps/desktop/src/main/lib/analytics/index.ts (1)
track(45-66)apps/desktop/src/main/lib/terminal/manager.ts (1)
terminalManager(432-432)
apps/desktop/src/main/lib/terminal/manager.ts (1)
apps/desktop/src/main/lib/analytics/index.ts (1)
track(45-66)
apps/marketing/src/app/components/DownloadButton/DownloadButton.tsx (1)
apps/desktop/src/renderer/lib/posthog.ts (1)
posthog(28-28)
apps/desktop/src/main/lib/analytics/index.ts (2)
apps/desktop/src/main/env.main.ts (1)
env(12-41)apps/desktop/src/main/lib/api-client.ts (1)
apiClient(11-27)
apps/desktop/src/lib/trpc/routers/auth/index.ts (1)
apps/desktop/src/main/lib/analytics/index.ts (1)
clearUserCache(37-39)
apps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
PostHogUserIdentifier(6-18)
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (1)
apps/desktop/src/main/lib/analytics/index.ts (1)
track(45-66)
apps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (2)
apps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
PostHogUserIdentifier(9-27)apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
PostHogUserIdentifier(6-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Deploy Web
- GitHub Check: Deploy API
- GitHub Check: Deploy Admin
- GitHub Check: Deploy Docs
- GitHub Check: Deploy Marketing
- GitHub Check: Lint
- GitHub Check: Build
🔇 Additional comments (21)
apps/desktop/src/renderer/screens/sign-in/index.tsx (2)
18-21: LGTM! Auth event tracking is well-placed.Capturing "auth_started" with the provider before triggering the mutation is appropriate. The event will fire exactly once per sign-in attempt and provides useful context with the provider parameter.
14-16:andapps/desktop/package.json (1)
82-82: LGTM!Adding
posthog-nodefor server-side analytics in the Electron main process is the correct approach. This enables reliable event tracking from the main process whereposthog-js(browser-based) cannot be used directly.apps/marketing/src/app/components/DownloadButton/DownloadButton.tsx (2)
24-27: LGTM!The tracking is placed before the action (opening URL), ensuring the event is captured even if the user navigates away. This is a reliable pattern for analytics.
81-84: LGTM!Good pattern: capturing the event before invoking the callback ensures analytics are recorded regardless of callback behavior.
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
6-17: LGTM!The implementation correctly uses
isSuccessto differentiate between "query still loading" and "query completed with no user data". This ensuresposthog.reset()is only called after confirming the user is actually unauthenticated, not during initial load.apps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
9-27: LGTM!The implementation correctly:
- Conditionally enables the query only when
isSignedInis true (avoiding unnecessary requests)- Uses strict equality (
isSignedIn === false) to prevent reset during initial undefined state- Sources identity from the TRPC user query (database ID) instead of Clerk, ensuring consistency across platforms as per the PR objectives
The pattern matches the admin implementation, maintaining consistency across apps.
apps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
9-27: LGTM!The implementation is consistent with the web app's
PostHogUserIdentifier, correctly sourcing user identity from TRPC to use the database user ID.apps/desktop/src/main/env.main.ts (1)
21-22: LGTM! Environment configuration for PostHog is correct.The optional
NEXT_PUBLIC_POSTHOG_KEYallows analytics to be disabled when not configured, and the default host value ensures the app works out-of-the-box when a key is provided.Also applies to: 34-35
apps/desktop/src/main/lib/terminal/manager.test.ts (1)
14-15: LGTM! Test setup reordering.The test directory initialization has been moved to improve organization. No functional changes to the test logic.
apps/desktop/src/lib/trpc/routers/auth/index.ts (1)
3-3: LGTM! User cache cleared on sign out.The
clearUserCache()call correctly clears the cached user identity after sign out, ensuring subsequent events won't be attributed to the signed-out user.Also applies to: 57-57
apps/desktop/src/main/lib/terminal/manager.ts (1)
2-2: LGTM! Terminal opened event tracked correctly.The
terminal_openedevent is emitted after successful session creation with relevant context (workspace_id, pane_id). The fire-and-forget pattern is appropriate for telemetry.Also applies to: 62-62, 79-80
apps/desktop/src/main/index.ts (2)
6-6: LGTM! Auth completion tracked.The
auth_completedevent is tracked after successful authentication, providing visibility into the auth flow.Also applies to: 52-52
143-145: LGTM! Parallel cleanup improves shutdown performance.Running terminal cleanup and analytics shutdown in parallel with
Promise.allreduces app quit time without introducing risk since the operations are independent.apps/desktop/test-setup.ts (1)
88-90: LGTM! Test mocks complete.The electron shell and analytics mocks provide the necessary test infrastructure for components that use these APIs, preventing tests from making real external calls.
Also applies to: 93-101
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (3)
170-175: LGTM! Workspace creation tracked with full context.The
workspace_createdevent includes comprehensive properties (workspace_id, project_id, branch, base_branch) for detailed telemetry analysis.
282-287: LGTM! Branch workspace opened tracked correctly.The event includes the
was_existingflag to distinguish between creating a new workspace and reactivating an existing one.
790-790: LGTM! Workspace lifecycle events tracked.Both
workspace_deletedandworkspace_closedevents are tracked after successful operations, providing visibility into workspace management.Also applies to: 1136-1136
apps/desktop/src/main/lib/analytics/index.ts (3)
23-32: LGTM! User ID caching with graceful error handling.The async fetch with caching pattern is appropriate for desktop analytics. Swallowing errors ensures tracking failures don't impact the user experience.
45-66: LGTM! Fire-and-forget tracking pattern.The implementation correctly follows the fire-and-forget pattern for analytics, ensuring tracking never blocks the UI or impacts app functionality. The automatic addition of
app_nameandplatformprovides consistent context across all events.
71-73: LGTM! Graceful shutdown handling.The optional chaining safely handles the case where PostHog was never initialized.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 Preview resources have been processed for cleanup |
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
The biggest risk is analytics correctness/reliability: several critical events can be silently dropped due to track() requiring user.me to succeed (notably auth_completed) and errors being fully swallowed in main/lib/analytics. Desktop’s desktop_opened is currently tied to the sign-in screen, which will undercount real launches. In web/admin, the move to user.me is good, but the identify/reset logic should be keyed to query completion to avoid subtle session-transition edge cases. Finally, workspace event payloads should be normalized across code paths to keep PostHog schemas consistent.
Additional notes (5)
- Readability |
apps/desktop/src/main/index.ts:49-54
track("auth_completed")fires immediately afterhandleAuthCallbackresolves, but there’s no guarantee the user profile (user.me) is available yet formain/lib/analyticsto attach adistinctId. As implemented,track()silently drops the event ifgetUserId()returnsnull.
That means a meaningful portion of auth_completed events could be lost (especially on slow networks / first app launch) and you would never know because errors are swallowed.
- Maintainability |
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts:167-179
track("workspace_created")is emitted afterloadSetupConfig(project.mainRepoPath). IfloadSetupConfigthrows (or future changes add failure paths after the workspace is created but before returning), you’ll miss the creation event even though the workspace exists.
For analytics correctness, events that describe side effects (workspace created) should be emitted immediately after the side effect is committed (DB write), not after unrelated work.
- Maintainability |
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts:279-279
track("workspace_opened")in the branch workspace flow includeswas_existing, which is good, but the worktree flow’sworkspace_openedevent does not. That makes the event schema inconsistent for the same event name and complicates analysis downstream.
Similarly, workspace_closed and workspace_deleted only include workspace_id, which may be insufficient for segmentation (project, workspace type, etc.) if you need it later.
- Maintainability |
apps/desktop/src/main/lib/terminal/manager.ts:59-59
terminal_openedis fired indoCreateSessionand includesworkspace_id, but the previous code path didn’t require thatworkspaceIdis present. IfworkspaceIdcan be missing/empty in some flows (e.g., ad-hoc terminals), you’ll either sendworkspace_id: undefinedor lose the ability to attribute sessions.
Given the rest of your desktop analytics is workspace-centric, consider either enforcing that workspaceId is always present for session creation, or intentionally omitting it when unavailable.
- Maintainability |
apps/marketing/src/app/components/DownloadButton/DownloadButton.tsx:23-28
The marketingDownloadButtonnow importsposthog-jsdirectly and captures events. This assumes PostHog is always initialized in this bundle. If initialization is conditional (env-based) or delayed,posthog.capturecan become a no-op or behave unexpectedly. Also, if this component is used before PostHog init finishes, events might be lost.
Since this PR adds many events, a centralized wrapper (that checks init and can buffer) reduces future drift across apps.
Summary of changes
What changed
Cross-app user identity normalization
- Updated Admin and Web
PostHogUserIdentifiercomponents to identify users via the backendtrpc.user.me(@tanstack/react-query+useTRPC) instead of using Clerk’suser.id. - Updated Desktop Renderer
PostHogUserIdentifierto reset PostHog only when theuser.mequery is successful and returns no user.
Desktop analytics instrumentation
- Added a new main-process analytics module at
apps/desktop/src/main/lib/analytics/index.tsusingposthog-nodeto reliably capture events server-side. - Added
posthog-nodedependency to the desktop app. - Instrumented Desktop events:
auth_completedafter OAuth callback processing inapps/desktop/src/main/index.ts- Workspace lifecycle events (
workspace_created,workspace_opened,workspace_closed,workspace_deleted) inapps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts terminal_openedwhen creating terminal sessions inapps/desktop/src/main/lib/terminal/manager.ts
- Added desktop env vars
NEXT_PUBLIC_POSTHOG_KEY(optional) andNEXT_PUBLIC_POSTHOG_HOST(defaulted) toapps/desktop/src/main/env.main.ts. - Ensured analytics cache is cleared on desktop sign out via
clearUserCache().
Marketing analytics
- Added
download_clickedandwaitlist_clickedPostHog captures toapps/marketing/src/app/components/DownloadButton/DownloadButton.tsx.
Test support
- Extended
apps/desktop/test-setup.tsto mockelectron.shell.openExternalandmain/lib/analyticsfor tests.
| export function PostHogUserIdentifier() { | ||
| const { user, isLoaded } = useUser(); | ||
| const { isSignedIn } = useUser(); | ||
| const trpc = useTRPC(); | ||
|
|
||
| useEffect(() => { | ||
| if (!isLoaded) return; | ||
| const { data: user } = useQuery({ | ||
| ...trpc.user.me.queryOptions(), | ||
| enabled: isSignedIn, | ||
| }); | ||
|
|
||
| useEffect(() => { | ||
| if (user) { | ||
| posthog.identify(user.id, { | ||
| email: user.primaryEmailAddress?.emailAddress, | ||
| name: user.fullName, | ||
| }); | ||
| } else { | ||
| posthog.identify(user.id, { email: user.email, name: user.name }); | ||
| } else if (isSignedIn === false) { | ||
| posthog.reset(); | ||
| } | ||
| }, [user, isLoaded]); | ||
| }, [user, isSignedIn]); |
There was a problem hiding this comment.
posthog.reset() is gated on isSignedIn === false, but not on the me query being finished. If the query is still in-flight and user is undefined, this effect currently does nothing (good), but when isSignedIn flips to false you reset immediately even if there’s still cached React Query data for user.me (or a stale user value). Consider resetting based on query state (e.g., isFetched/isSuccess) to avoid edge cases where you briefly identify with a stale user after sign-out or during session transitions.
Suggestion
Consider explicitly using the React Query status to drive reset/identify:
- Destructure
isSuccess/isFetched(orstatus) fromuseQuery. - Only call
posthog.identifywhenisSuccess && user. - Only call
posthog.resetwhenisSuccess && !useror whenisSignedIn === falseand the query has settled.
Example:
const { data: user, isSuccess } = useQuery({
...trpc.user.me.queryOptions(),
enabled: isSignedIn,
});
useEffect(() => {
if (!isSuccess) return;
if (user) posthog.identify(user.id, { email: user.email, name: user.name });
else posthog.reset();
}, [isSuccess, user]);Reply with "@CharlieHelps yes please" if you’d like me to add a commit implementing this change.
| export function PostHogUserIdentifier() { | ||
| const { user, isLoaded } = useUser(); | ||
| const { isSignedIn } = useUser(); | ||
| const trpc = useTRPC(); | ||
|
|
||
| useEffect(() => { | ||
| if (!isLoaded) return; | ||
| const { data: user } = useQuery({ | ||
| ...trpc.user.me.queryOptions(), | ||
| enabled: isSignedIn, | ||
| }); | ||
|
|
||
| useEffect(() => { | ||
| if (user) { | ||
| posthog.identify(user.id, { | ||
| email: user.primaryEmailAddress?.emailAddress, | ||
| name: user.fullName, | ||
| }); | ||
| } else { | ||
| posthog.identify(user.id, { email: user.email, name: user.name }); | ||
| } else if (isSignedIn === false) { | ||
| posthog.reset(); | ||
| } | ||
| }, [user, isLoaded]); | ||
| }, [user, isSignedIn]); |
There was a problem hiding this comment.
Same concern as in admin: the reset behavior depends on isSignedIn === false rather than the user.me query lifecycle. Without keying off query completion, you can get subtle timing issues during sign-out/session refresh where PostHog identity is not deterministically cleared based on the authoritative backend state.
Suggestion
Align reset/identify logic to the React Query result status (e.g., isSuccess/isFetched) rather than isSignedIn alone, so the component behavior is driven by the same authoritative source (user.me) you introduced.
Reply with "@CharlieHelps yes please" if you’d like me to add a commit applying the same pattern as suggested for admin.
| import { env } from "main/env.main"; | ||
| import { apiClient } from "main/lib/api-client"; | ||
| import { PostHog } from "posthog-node"; | ||
|
|
||
| let client: PostHog | null = null; | ||
| let cachedUserId: string | null = null; | ||
|
|
||
| function getClient(): PostHog | null { | ||
| if (!env.NEXT_PUBLIC_POSTHOG_KEY) { | ||
| return null; | ||
| } | ||
|
|
||
| if (!client) { | ||
| client = new PostHog(env.NEXT_PUBLIC_POSTHOG_KEY, { | ||
| host: env.NEXT_PUBLIC_POSTHOG_HOST, | ||
| flushAt: 1, // Send events immediately for desktop app | ||
| flushInterval: 0, | ||
| }); | ||
| } | ||
| return client; | ||
| } | ||
|
|
||
| async function getUserId(): Promise<string | null> { | ||
| if (cachedUserId) return cachedUserId; | ||
| try { | ||
| const user = await apiClient.user.me.query(); | ||
| cachedUserId = user?.id ?? null; | ||
| return cachedUserId; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Clear cached user ID (call on sign out) | ||
| */ | ||
| export function clearUserCache(): void { | ||
| cachedUserId = null; | ||
| } | ||
|
|
||
| /** | ||
| * Track an event with the current user's ID as distinct_id. | ||
| * Fire-and-forget - errors are silently ignored. | ||
| */ | ||
| export function track( | ||
| event: string, | ||
| properties?: Record<string, unknown>, | ||
| ): void { | ||
| const posthog = getClient(); | ||
| if (!posthog) return; | ||
|
|
||
| getUserId() | ||
| .then((userId) => { | ||
| if (!userId) return; | ||
| posthog.capture({ | ||
| distinctId: userId, | ||
| event, | ||
| properties: { | ||
| ...properties, | ||
| app_name: "desktop", | ||
| platform: process.platform, | ||
| }, | ||
| }); | ||
| }) | ||
| .catch(() => {}); | ||
| } | ||
|
|
||
| /** | ||
| * Shutdown PostHog client (call on app quit) | ||
| */ | ||
| export async function shutdown(): Promise<void> { | ||
| await client?.shutdown(); | ||
| } |
There was a problem hiding this comment.
The analytics module drops events whenever getUserId() fails, and it never flushes/awaits delivery for important lifecycle events. With flushAt: 1 and flushInterval: 0 you are aiming for immediate delivery, but without awaiting capture / explicit flush, events can still be lost on shutdown.
Also, swallowing all errors (catch(() => {})) removes any ability to detect a misconfiguration (bad key/host) in production logs.
Suggestion
Improve reliability/observability while keeping it low-noise:
- For shutdown, explicitly
await client?.flush()beforeshutdown()(or ensureshutdown()flushes in yourposthog-nodeversion). - For
track, consider capturing events even without a user via an app-scoped distinct id (e.g., a persisted installation id) so you don’t lose pre-auth events. - Replace the blanket
catch(() => {})with a very low-volume logger gated byNODE_ENV !== 'test'and maybe a simple once-per-session guard.
Example:
let warned = false;
...
.catch((err) => {
if (!warned && process.env.NODE_ENV !== 'test') {
warned = true;
console.warn('[analytics] capture failed', err);
}
});Reply with "@CharlieHelps yes please" if you’d like me to add a commit implementing (1) and a minimal (3).
| if (!client) { | ||
| client = new PostHog(env.NEXT_PUBLIC_POSTHOG_KEY, { | ||
| host: env.NEXT_PUBLIC_POSTHOG_HOST, | ||
| flushAt: 1, // Send events immediately for desktop app | ||
| flushInterval: 0, | ||
| }); | ||
| } |
There was a problem hiding this comment.
posthog-node is configured with flushAt: 1 and flushInterval: 0, which can create very chatty network usage (each event becomes its own request) and may impact performance/battery on laptops, especially given workspace/terminal events can be frequent. Also, shutdown() only runs on before-quit; crashes/forced exits will drop events anyway, so paying the cost of per-event flush may not be worth it.
Suggestion
Use a small batch/interval to reduce overhead (e.g., flushAt: 5 and flushInterval: 5000) and rely on shutdown() to flush on graceful exit. Optionally call client?.flush() after high-value events (auth completed) if needed.
Reply with "@CharlieHelps yes please" if you'd like me to add a commit adjusting the PostHog client config and adding an optional flush() helper for critical moments.
| // Load setup configuration from the main repo (where .superset/config.json lives) | ||
| const setupConfig = loadSetupConfig(project.mainRepoPath); | ||
|
|
||
| track("workspace_created", { | ||
| workspace_id: workspace.id, | ||
| project_id: project.id, | ||
| branch: branch, | ||
| base_branch: targetBranch, | ||
| }); | ||
|
|
There was a problem hiding this comment.
track("workspace_created"/"workspace_opened") is called unconditionally after workspace creation/open. If any downstream logic throws after the DB update but before returning (e.g., config loading, other steps not shown), you may emit events for operations that ultimately fail from the user's perspective. This can pollute funnels and make metrics hard to trust.
Suggestion
Consider tracking only after the full mutation has succeeded (i.e., immediately before returning, after all possible failure points), or wrap the mutation body in a try/catch and emit separate failure events (e.g. workspace_create_failed) when exceptions occur.
Example pattern:
- Do all work
return result- In a final step right before
return, calltrack(...)
Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this refactor across the workspace mutations.
| // Track terminal opened (only fires once per session creation) | ||
| track("terminal_opened", { workspace_id: workspaceId, pane_id: paneId }); | ||
|
|
There was a problem hiding this comment.
track("terminal_opened") runs for every session creation, including potentially during recovery flows depending on how createSession/pendingSessions behave. If recovered sessions are common, this may overcount. Also, workspace_id comes from params; ensure it’s always present/meaningful for analytics (otherwise you'll get a lot of null/undefined).
Suggestion
Gate the event to truly-new sessions only (e.g., if (!session.wasRecovered) track(...)) or include was_recovered: session.wasRecovered so analysis can filter accurately.
Reply with "@CharlieHelps yes please" if you'd like me to add a commit that includes was_recovered (or guards tracking) here.
| export function SignInScreen() { | ||
| const signInMutation = trpc.auth.signIn.useMutation(); | ||
|
|
||
| const signIn = (provider: AuthProvider) => | ||
| useEffect(() => { | ||
| posthog.capture("desktop_opened"); | ||
| }, []); | ||
|
|
||
| const signIn = (provider: AuthProvider) => { | ||
| posthog.capture("auth_started", { provider }); | ||
| signInMutation.mutate({ provider }); | ||
| }; | ||
|
|
There was a problem hiding this comment.
desktop_opened is being captured in the Sign In screen mount effect. That likely undercounts launches where the user is already authenticated and never hits this screen, and over-couples an app-lifecycle event to a specific UI route.
Since you already introduced a main-process analytics module, app-open should be tracked from the main process (e.g., when app.whenReady() resolves), potentially with a separate renderer event for “sign_in_screen_viewed” if you want it.
Suggestion
Move desktop_opened to the main process (e.g., right after the app is ready / window created) using main/lib/analytics.track, and replace the renderer event with a screen-specific event if desired.
Example main process:
import { track } from './lib/analytics';
...
await app.whenReady();
track('desktop_opened');Reply with "@CharlieHelps yes please" if you’d like me to add a commit moving the event and adjusting naming.
1 participant
Summary
posthog-nodefor reliable server-side tracking in desktop main processEvents Added
download_clickedwaitlist_clickeddesktop_openedauth_startedauth_completedworkspace_createdworkspace_openedworkspace_closedworkspace_deletedterminal_openedTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.