From 8eb14d7fbff2fe12922ac38987935d7e9c983c3a Mon Sep 17 00:00:00 2001 From: Adam Bowker Date: Wed, 26 Aug 2026 13:17:25 -0400 Subject: [PATCH 1/2] fix(desktop): record an onboarding step view once the step is settled `Onboarding step viewed` fired from a mount effect that ran before the lookups choosing the step set had answered, so every person recorded a `project-select` view including those with one project, whose step was then dropped. The self-heal that moves a person off a dropped step emitted nothing, so anyone it moved into `consent` advanced with no matching view. Record the view from one effect, once no pending lookup can still drop the step. Reset a persisted step id from a retired set at rehydration, which the old flow could otherwise show as an empty card. Co-Authored-By: Claude Opus 5 (1M context) --- .../core/src/onboarding/steps.test.ts | 29 ++++++++++++++++ .../packages/core/src/onboarding/steps.ts | 27 +++++++++++++-- .../onboarding/components/OnboardingFlow.tsx | 29 ++++++++-------- .../onboarding/hooks/useOnboardingFlow.ts | 23 ++++++------- .../onboarding/onboardingStore.test.ts | 33 ++++++++++++++----- .../features/onboarding/onboardingStore.ts | 12 +++++-- 6 files changed, 115 insertions(+), 38 deletions(-) diff --git a/products/desktop/packages/core/src/onboarding/steps.test.ts b/products/desktop/packages/core/src/onboarding/steps.test.ts index 0ba33928258d..2b8e6e33d3a6 100644 --- a/products/desktop/packages/core/src/onboarding/steps.test.ts +++ b/products/desktop/packages/core/src/onboarding/steps.test.ts @@ -9,6 +9,7 @@ import { type OnboardingStep, previousStep, stepDirection, + stepGatePending, } from "./steps"; type StepGates = Parameters[0]; @@ -76,6 +77,34 @@ describe("computeActiveSteps", () => { }); }); +describe("stepGatePending", () => { + it.each<{ step: OnboardingStep; gate: keyof StepGates }>([ + { step: "project-select", gate: "projectCount" }, + { step: "consent", gate: "consentRequired" }, + { step: "install-cli", gate: "hasGithubIntegration" }, + { step: "install-cli", gate: "cliReady" }, + ])("holds $step while $gate has not answered", ({ step, gate }) => { + const answered: StepGates = { + hasGithubIntegration: false, + cliReady: false, + projectCount: 2, + consentRequired: true, + }; + + expect(stepGatePending(step, answered)).toBe(false); + expect(stepGatePending(step, { ...answered, [gate]: undefined })).toBe( + true, + ); + }); + + it.each(["connect-github", "select-repo"])( + "never holds %s, which no gate can drop", + (step) => { + expect(stepGatePending(step, allSteps)).toBe(false); + }, + ); +}); + describe("nearestActiveStep", () => { const withoutConditionals = computeActiveSteps({ hasGithubIntegration: true, diff --git a/products/desktop/packages/core/src/onboarding/steps.ts b/products/desktop/packages/core/src/onboarding/steps.ts index 6a4f51ce3af5..72cd519a346b 100644 --- a/products/desktop/packages/core/src/onboarding/steps.ts +++ b/products/desktop/packages/core/src/onboarding/steps.ts @@ -21,7 +21,7 @@ export interface DetectedRepo { branch?: string; } -export function computeActiveSteps(options: { +export interface StepGates { /** Undefined while the integrations query is loading; the step only drops on a confirmed connection. */ hasGithubIntegration: boolean | undefined; /** Undefined while the local git and gh checks are loading. */ @@ -29,7 +29,9 @@ export function computeActiveSteps(options: { /** Undefined until the project list has loaded, so a slow list cannot skip a real choice. */ projectCount: number | undefined; consentRequired: boolean | undefined; -}): OnboardingStep[] { +} + +export function computeActiveSteps(options: StepGates): OnboardingStep[] { return ONBOARDING_STEPS.filter((step) => { if (step === "project-select" && options.projectCount === 1) return false; if (step === "consent" && options.consentRequired === false) return false; @@ -45,6 +47,27 @@ export function computeActiveSteps(options: { }); } +/** + * Whether a gate that governs `step` has not answered yet. An unanswered gate + * keeps its step in the active set, so the step is on screen but a later answer + * can still take it away. Analytics waits for this to be false, or it records a + * view for a step the person never had to complete. + */ +export function stepGatePending( + step: OnboardingStep, + options: StepGates, +): boolean { + if (step === "project-select") return options.projectCount === undefined; + if (step === "consent") return options.consentRequired === undefined; + if (step === "install-cli") { + return ( + options.hasGithubIntegration === undefined || + options.cliReady === undefined + ); + } + return false; +} + export function stepIndexOf( activeSteps: OnboardingStep[], step: OnboardingStep, diff --git a/products/desktop/packages/ui/src/features/onboarding/components/OnboardingFlow.tsx b/products/desktop/packages/ui/src/features/onboarding/components/OnboardingFlow.tsx index dc93b99e4239..2fe4f28247b0 100644 --- a/products/desktop/packages/ui/src/features/onboarding/components/OnboardingFlow.tsx +++ b/products/desktop/packages/ui/src/features/onboarding/components/OnboardingFlow.tsx @@ -28,6 +28,7 @@ import { ConnectGitHubStep } from "@posthog/ui/features/onboarding/components/Co import { InstallCliStep } from "@posthog/ui/features/onboarding/components/InstallCliStep"; import { useOnboardingFlow } from "@posthog/ui/features/onboarding/hooks/useOnboardingFlow"; import { useOnboardingStore } from "@posthog/ui/features/onboarding/onboardingStore"; +import type { OnboardingStep } from "@posthog/ui/features/onboarding/types"; import { useSettingsStore } from "@posthog/ui/features/settings/settingsStore"; import { shipIt } from "@posthog/ui/primitives/confetti"; import { FullScreenLayout } from "@posthog/ui/primitives/FullScreenLayout"; @@ -77,6 +78,7 @@ export function OnboardingFlow({ onOpenSupport }: OnboardingFlowProps) { hasGithubIntegration, consentSatisfied, consentRequirement, + currentStepPending, } = useOnboardingFlow(); const completeOnboarding = useOnboardingStore( (state) => state.completeOnboarding, @@ -156,15 +158,25 @@ export function OnboardingFlow({ onOpenSupport }: OnboardingFlowProps) { const flowStartedAtRef = useRef(Date.now()); const stepEnteredAtRef = useRef(Date.now()); - // biome-ignore lint/correctness/useExhaustiveDependencies: fires once on mount; subsequent step views fire from handleNext/handleBack useEffect(() => { track(ANALYTICS_EVENTS.ONBOARDING_STARTED); + }, []); + + const viewedStepRef = useRef(null); + // Recorded once the step can no longer be taken away, which covers the two + // paths that had no correct one: a step shown before its gate answers, and a + // step entered by the self-heal in useOnboardingFlow. + useEffect(() => { + if (currentIndex < 0 || currentStepPending) return; + if (viewedStepRef.current === currentStep) return; + viewedStepRef.current = currentStep; track(ANALYTICS_EVENTS.ONBOARDING_STEP_VIEWED, { step_id: currentStep, step_index: currentIndex, total_steps: activeSteps.length, }); - }, []); + stepEnteredAtRef.current = Date.now(); + }, [currentStep, currentIndex, currentStepPending, activeSteps.length]); useEffect(() => { const handleBeforeUnload = () => { @@ -195,17 +207,6 @@ export function OnboardingFlow({ onOpenSupport }: OnboardingFlowProps) { ); }; - const trackStepViewed = (stepIndex: number) => { - const stepId = activeSteps[stepIndex]; - if (!stepId) return; - track(ANALYTICS_EVENTS.ONBOARDING_STEP_VIEWED, { - step_id: stepId, - step_index: stepIndex, - total_steps: activeSteps.length, - }); - stepEnteredAtRef.current = Date.now(); - }; - const handleNext = (context?: StepCompletedContext) => { if ( currentStep === "consent" && @@ -218,13 +219,11 @@ export function OnboardingFlow({ onOpenSupport }: OnboardingFlowProps) { const safeContext = context && "nativeEvent" in context ? undefined : context; trackStepCompleted(safeContext); - trackStepViewed(currentIndex + 1); next(); }; const handleBack = () => { if (currentStep === "consent" && consentSubmitting) return; - trackStepViewed(currentIndex - 1); back(); }; diff --git a/products/desktop/packages/ui/src/features/onboarding/hooks/useOnboardingFlow.ts b/products/desktop/packages/ui/src/features/onboarding/hooks/useOnboardingFlow.ts index 8912317bbd25..c3f1c08c791c 100644 --- a/products/desktop/packages/ui/src/features/onboarding/hooks/useOnboardingFlow.ts +++ b/products/desktop/packages/ui/src/features/onboarding/hooks/useOnboardingFlow.ts @@ -12,6 +12,7 @@ import { nearestActiveStep, type OnboardingStep, stepDirection, + stepGatePending, } from "@posthog/core/onboarding/steps"; import { useHostTRPC, useHostTRPCClient } from "@posthog/host-router/react"; import { ANALYTICS_EVENTS } from "@posthog/shared/analytics-events"; @@ -205,25 +206,24 @@ export function useOnboardingFlow() { ); }, [consent]); + // A failed lookup keeps the step, same as an unanswered one, but it answers + // the gate. Otherwise the step shows with its view never recorded. const consentRequired = - consentRequirement?.organizationId === consent.organizationId - ? consentRequirement?.required - : undefined; + consent.status === "error" + ? true + : consentRequirement?.organizationId === consent.organizationId + ? consentRequirement?.required + : undefined; const sampledConsentRequirement = consentRequirement?.organizationId === consent.organizationId ? consentRequirement : undefined; - const activeSteps = useMemo( - () => - computeActiveSteps({ - hasGithubIntegration, - cliReady, - projectCount, - consentRequired, - }), + const stepGates = useMemo( + () => ({ hasGithubIntegration, cliReady, projectCount, consentRequired }), [hasGithubIntegration, cliReady, projectCount, consentRequired], ); + const activeSteps = useMemo(() => computeActiveSteps(stepGates), [stepGates]); useEffect(() => { if (!activeSteps.includes(currentStep)) { @@ -259,6 +259,7 @@ export function useOnboardingFlow() { return { currentStep, currentIndex, + currentStepPending: stepGatePending(currentStep, stepGates), totalSteps: activeSteps.length, activeSteps, isFirstStep, diff --git a/products/desktop/packages/ui/src/features/onboarding/onboardingStore.test.ts b/products/desktop/packages/ui/src/features/onboarding/onboardingStore.test.ts index 4464bdbbec12..883e5f97a6d0 100644 --- a/products/desktop/packages/ui/src/features/onboarding/onboardingStore.test.ts +++ b/products/desktop/packages/ui/src/features/onboarding/onboardingStore.test.ts @@ -2,14 +2,31 @@ import { describe, expect, it } from "vitest"; import { migrateOnboardingState } from "./onboardingStore"; describe("migrateOnboardingState", () => { + const persisted = (currentStep: string) => ({ + currentStep, + hasCompletedOnboarding: false, + hasShippedFirstPr: false, + selectedProjectId: 1, + }); + it("moves a persisted invite-code step forward", () => { - const migrated = migrateOnboardingState({ - currentStep: "invite-code", - hasCompletedOnboarding: false, - hasShippedFirstPr: false, - selectedProjectId: 1, - }); - - expect(migrated.currentStep).toBe("consent"); + expect(migrateOnboardingState(persisted("invite-code")).currentStep).toBe( + "consent", + ); + }); + + it.each(["welcome", "import-config"])( + "resets the retired %s step to the start of the flow", + (step) => { + expect(migrateOnboardingState(persisted(step)).currentStep).toBe( + "project-select", + ); + }, + ); + + it("leaves a step of the current flow alone", () => { + expect(migrateOnboardingState(persisted("select-repo")).currentStep).toBe( + "select-repo", + ); }); }); diff --git a/products/desktop/packages/ui/src/features/onboarding/onboardingStore.ts b/products/desktop/packages/ui/src/features/onboarding/onboardingStore.ts index 9662c369822c..150e6fbfa015 100644 --- a/products/desktop/packages/ui/src/features/onboarding/onboardingStore.ts +++ b/products/desktop/packages/ui/src/features/onboarding/onboardingStore.ts @@ -1,4 +1,7 @@ -import type { OnboardingStep } from "@posthog/ui/features/onboarding/types"; +import { + ONBOARDING_STEPS, + type OnboardingStep, +} from "@posthog/ui/features/onboarding/types"; import { logger } from "@posthog/ui/shell/logger"; import { create } from "zustand"; import { persist } from "zustand/middleware"; @@ -37,6 +40,11 @@ export function migrateOnboardingState( if ((state.currentStep as string) === "invite-code") { return { ...state, currentStep: "consent" }; } + // A step id from a retired set, for example "welcome", renders no branch in + // the flow, so the person sees an empty card until the self-heal moves them. + if (!ONBOARDING_STEPS.includes(state.currentStep)) { + return { ...state, currentStep: ONBOARDING_STEPS[0] }; + } return state; } @@ -61,7 +69,7 @@ export const useOnboardingStore = create()( }), { name: "onboarding-store", - version: 1, + version: 2, migrate: migrateOnboardingState, partialize: (state) => ({ currentStep: state.currentStep, From c4e54e9cacbf14d42b166ba7d32af2a1520ba443 Mon Sep 17 00:00:00 2001 From: Adam Bowker Date: Wed, 26 Aug 2026 13:49:38 -0400 Subject: [PATCH 2/2] fix(desktop): never record an onboarding completion without its view Two paths could still emit `Onboarding step completed` for a step that recorded no view. A failed GitHub integrations lookup left its gate unanswered for the session, holding `install-cli` pending while the step rendered and its Continue button worked. A person could also leave any pending step before its gate answered, through the button or the right-arrow hotkey. Read the query's pending state rather than its data, so a failed lookup answers the gate and keeps the step. Record the view from `handleNext` and `handleComplete` as well as the effect, so a completion never precedes one. Track step entry separately from the view, so a delayed view no longer measures `duration_seconds` from the previous step. Co-Authored-By: Claude Opus 5 (1M context) --- .../onboarding/components/OnboardingFlow.tsx | 29 ++++++++++++++----- .../onboarding/hooks/useOnboardingFlow.ts | 12 +++++--- 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/products/desktop/packages/ui/src/features/onboarding/components/OnboardingFlow.tsx b/products/desktop/packages/ui/src/features/onboarding/components/OnboardingFlow.tsx index 2fe4f28247b0..6ca5ec27d440 100644 --- a/products/desktop/packages/ui/src/features/onboarding/components/OnboardingFlow.tsx +++ b/products/desktop/packages/ui/src/features/onboarding/components/OnboardingFlow.tsx @@ -162,20 +162,31 @@ export function OnboardingFlow({ onOpenSupport }: OnboardingFlowProps) { track(ANALYTICS_EVENTS.ONBOARDING_STARTED); }, []); - const viewedStepRef = useRef(null); - // Recorded once the step can no longer be taken away, which covers the two - // paths that had no correct one: a step shown before its gate answers, and a - // step entered by the self-heal in useOnboardingFlow. + // Entry is when the person arrives on the step, which is not always when the + // view is recorded: a pending gate delays the view but not the reading. + // biome-ignore lint/correctness/useExhaustiveDependencies: currentStep is the trigger, not a value the body reads useEffect(() => { - if (currentIndex < 0 || currentStepPending) return; - if (viewedStepRef.current === currentStep) return; + stepEnteredAtRef.current = Date.now(); + }, [currentStep]); + + const viewedStepRef = useRef(null); + const recordStepViewed = () => { + if (currentIndex < 0 || viewedStepRef.current === currentStep) return; viewedStepRef.current = currentStep; track(ANALYTICS_EVENTS.ONBOARDING_STEP_VIEWED, { step_id: currentStep, step_index: currentIndex, total_steps: activeSteps.length, }); - stepEnteredAtRef.current = Date.now(); + }; + + // The ordinary path: the step settles while the person is reading it. This + // also covers a step entered by the self-heal in useOnboardingFlow, which + // reaches the person without passing through handleNext. + // biome-ignore lint/correctness/useExhaustiveDependencies: recordStepViewed reads the same values these deps carry + useEffect(() => { + if (currentStepPending) return; + recordStepViewed(); }, [currentStep, currentIndex, currentStepPending, activeSteps.length]); useEffect(() => { @@ -218,6 +229,9 @@ export function OnboardingFlow({ onOpenSupport }: OnboardingFlowProps) { // into capture properties poisons the whole analytics batch. const safeContext = context && "nativeEvent" in context ? undefined : context; + // A person can leave a step before its gate answers, which the effect above + // skips. Record the view first, so a completion never arrives without one. + recordStepViewed(); trackStepCompleted(safeContext); next(); }; @@ -237,6 +251,7 @@ export function OnboardingFlow({ onOpenSupport }: OnboardingFlowProps) { useHotkeys("left", handleBack, { enableOnFormTags: false }, [handleBack]); const handleComplete = (repoSkipped: boolean) => { + recordStepViewed(); if (repoSkipped) { track(ANALYTICS_EVENTS.ONBOARDING_STEP_SKIPPED, { step_id: currentStep, diff --git a/products/desktop/packages/ui/src/features/onboarding/hooks/useOnboardingFlow.ts b/products/desktop/packages/ui/src/features/onboarding/hooks/useOnboardingFlow.ts index c3f1c08c791c..0c3557971609 100644 --- a/products/desktop/packages/ui/src/features/onboarding/hooks/useOnboardingFlow.ts +++ b/products/desktop/packages/ui/src/features/onboarding/hooks/useOnboardingFlow.ts @@ -135,7 +135,8 @@ export function useOnboardingFlow() { ], ); - const { data: githubUserIntegrations } = useUserGithubIntegrations(); + const { data: githubUserIntegrations, isPending: githubIntegrationsPending } = + useUserGithubIntegrations(); // The install-cli step only offers git and gh, so a ready toolchain skips it. // InstallCliStep reuses these cached results when the step does render. const trpc = useHostTRPC(); @@ -166,9 +167,12 @@ export function useOnboardingFlow() { gitStatus.installed && ghStatus.installed && ghStatus.authenticated, ); }, [cliReady, localWorkspaces, gitStatus, ghStatus]); - const hasGithubIntegration = githubUserIntegrations - ? githubUserIntegrations.length > 0 - : undefined; + // Read the pending state, not the data: the query retries and then leaves + // `data` undefined, which would hold the install-cli gate open for the rest + // of the session. A failed lookup keeps the step, same as an unanswered one. + const hasGithubIntegration = githubIntegrationsPending + ? undefined + : (githubUserIntegrations?.length ?? 0) > 0; // Counted off the store rather than through useProjects, whose auto-select // effect would then run in a second place and re-clear the query cache. const orgProjectsMap = useAuthStateValue((state) => state.orgProjectsMap);