From 258d84ad7926250714f684f4d2c1748c7fee8fe5 Mon Sep 17 00:00:00 2001 From: Josh Sherman Date: Tue, 1 Sep 2026 15:11:03 -0400 Subject: [PATCH 01/12] feat(posthog-integration): detect existing PostHog, surface the other tricks (#1066) Co-authored-by: Claude --- AGENTS.md | 4 +- bin.ts | 35 +---- src/__tests__/programs-cli.test.ts | 25 ++++ src/commands/index.ts | 44 ++++++ .../posthog-integration-detect.test.ts | 71 ++++++++++ .../__tests__/program-registry.test.ts | 57 +++++++- src/lib/programs/events-audit/index.ts | 1 - .../programs/posthog-integration/detect.ts | 23 +++ src/lib/programs/posthog-integration/index.ts | 1 - src/lib/programs/program-registry.ts | 14 ++ src/lib/programs/program-step.ts | 1 + src/lib/runners/run-wizard.ts | 15 +- src/lib/wizard-session.ts | 4 + .../posthog-integration-intro.test.ts | 132 ++++++++++++++++++ src/ui/tui/__tests__/store.test.ts | 95 +++++++++++++ src/ui/tui/posthog-integration-intro.ts | 50 +++++++ src/ui/tui/router.ts | 6 + .../screens/PostHogIntegrationIntroScreen.tsx | 66 +++++---- src/ui/tui/store.ts | 23 +++ src/utils/__tests__/analytics.test.ts | 13 +- src/utils/analytics.ts | 1 + 21 files changed, 614 insertions(+), 67 deletions(-) create mode 100644 src/commands/index.ts create mode 100644 src/lib/programs/__tests__/posthog-integration-detect.test.ts create mode 100644 src/ui/tui/__tests__/posthog-integration-intro.test.ts create mode 100644 src/ui/tui/posthog-integration-intro.ts diff --git a/AGENTS.md b/AGENTS.md index 9538219a4..fc5d172d7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -83,7 +83,9 @@ confuse it with the top-level `wizard skill` command. ### Where the surface is defined (source of truth) -- **Registration:** [`bin.ts`](bin.ts) — the `.use()` chain wires each command. +- **Registration:** [`src/commands/index.ts`](src/commands/index.ts) — + `ALL_COMMANDS` is the list [`bin.ts`](bin.ts) registers. Add a new command + there and nothing else needs touching. - **Command shape:** [`src/commands/command.ts`](src/commands/command.ts) — the `Command` interface every command implements. - **Flat native commands** (e.g. `revenue-analytics`, `upload-source-maps`) are diff --git a/bin.ts b/bin.ts index f8580dcff..ba1e83c68 100644 --- a/bin.ts +++ b/bin.ts @@ -46,22 +46,7 @@ if (process.env.NODE_ENV === 'test') { } import { Wizard } from './src/wizard'; -import { basicIntegrationCommand } from './src/commands/basic-integration'; -import { mcpCommand } from './src/commands/mcp'; -import { mcpAnalyticsCommand } from './src/commands/mcp-analytics'; -import { replayVisionCommand } from './src/commands/replay-vision'; -import { aiObservabilityCommand } from './src/commands/ai-observability'; -import { metricsCommand } from './src/commands/metrics'; -import { auditCommand } from './src/commands/audit'; -import { doctorCommand } from './src/commands/doctor'; -import { migrateCommand } from './src/commands/migrate'; -import { revenueCommand } from './src/commands/revenue'; -import { warehouseCommand } from './src/commands/warehouse'; -import { selfDrivingCommand } from './src/commands/self-driving'; -import { slackCommand } from './src/commands/slack'; -import { uploadSourcemapsCommand } from './src/commands/upload-sourcemaps'; -import { skillCommand } from './src/commands/skill'; -import { cliCommand } from './src/commands/cli'; +import { ALL_COMMANDS } from './src/commands'; import { recoverOrphanedSettingsBackups } from './src/lib/agent/claude-settings'; // Heal any .claude/settings backup a previous interrupted run left orphaned, @@ -79,20 +64,4 @@ function resolveInstallDir(): string { return process.env.POSTHOG_WIZARD_INSTALL_DIR ?? process.cwd(); } -Wizard.use(basicIntegrationCommand) - .use(mcpCommand) - .use(mcpAnalyticsCommand) - .use(replayVisionCommand) - .use(aiObservabilityCommand) - .use(metricsCommand) - .use(cliCommand) - .use(auditCommand) - .use(doctorCommand) - .use(migrateCommand) - .use(revenueCommand) - .use(warehouseCommand) - .use(selfDrivingCommand) - .use(slackCommand) - .use(uploadSourcemapsCommand) - .use(skillCommand) - .init(); +Wizard.use(...ALL_COMMANDS).init(); diff --git a/src/__tests__/programs-cli.test.ts b/src/__tests__/programs-cli.test.ts index 70c9bd9b9..63d299fc6 100644 --- a/src/__tests__/programs-cli.test.ts +++ b/src/__tests__/programs-cli.test.ts @@ -35,6 +35,9 @@ import { fetchSkillMenu, type CliEntry } from '@lib/wizard-tools'; import { auditConfig } from '@lib/programs/audit/index'; import { webAnalyticsDoctorConfig } from '@lib/programs/web-analytics-doctor/index'; import { parseCommand } from './helpers/parse-command.no-jest'; +import { ALL_COMMANDS } from '../commands'; +import { commandKeys } from '../commands/command'; +import { getSubcommandPrograms } from '@lib/programs/program-registry'; const mockFetchSkillMenu = fetchSkillMenu as MockedFunction< typeof fetchSkillMenu @@ -57,6 +60,28 @@ function mockMenu(cliEntries: CliEntry[]): void { mockFetchSkillMenu.mockResolvedValue({ categories: {}, cliEntries }); } +// A program's `command` field says what it calls itself, not whether the CLI +// can run it. Retiring or moving a command leaves that field behind, and the +// program keeps advertising a word nobody can type. +describe('advertised commands', () => { + const registered = new Set( + ALL_COMMANDS.flatMap((command) => commandKeys(command.name)), + ); + + test('every advertised command is one the CLI registers', () => { + // A nested program is reached through its parent, so that's the word + // yargs knows — `audit`, not `web-analytics`. + const wordFor = (program: { parentCommand?: string; command: string }) => + program.parentCommand ?? program.command; + + const unrunnable = getSubcommandPrograms() + .filter((program) => !registered.has(wordFor(program))) + .map((program) => `${program.id} advertises "${wordFor(program)}"`); + + expect(unrunnable).toEqual([]); + }); +}); + describe('top-level command shapes', () => { beforeEach(() => { vi.clearAllMocks(); diff --git a/src/commands/index.ts b/src/commands/index.ts new file mode 100644 index 000000000..832ff3c16 --- /dev/null +++ b/src/commands/index.ts @@ -0,0 +1,44 @@ +/** + * Every command the CLI registers, in the order `--help` lists them. + * + * Kept here rather than inline in bin.ts so registration and the tests that + * check it read from the same list — a program can otherwise advertise a + * command that no longer exists, and nothing catches it. + */ + +import type { Command } from './command'; +import { basicIntegrationCommand } from './basic-integration'; +import { mcpCommand } from './mcp'; +import { mcpAnalyticsCommand } from './mcp-analytics'; +import { replayVisionCommand } from './replay-vision'; +import { aiObservabilityCommand } from './ai-observability'; +import { metricsCommand } from './metrics'; +import { cliCommand } from './cli'; +import { auditCommand } from './audit'; +import { doctorCommand } from './doctor'; +import { migrateCommand } from './migrate'; +import { revenueCommand } from './revenue'; +import { warehouseCommand } from './warehouse'; +import { selfDrivingCommand } from './self-driving'; +import { slackCommand } from './slack'; +import { uploadSourcemapsCommand } from './upload-sourcemaps'; +import { skillCommand } from './skill'; + +export const ALL_COMMANDS: readonly Command[] = [ + basicIntegrationCommand, + mcpCommand, + mcpAnalyticsCommand, + replayVisionCommand, + aiObservabilityCommand, + metricsCommand, + cliCommand, + auditCommand, + doctorCommand, + migrateCommand, + revenueCommand, + warehouseCommand, + selfDrivingCommand, + slackCommand, + uploadSourcemapsCommand, + skillCommand, +]; diff --git a/src/lib/programs/__tests__/posthog-integration-detect.test.ts b/src/lib/programs/__tests__/posthog-integration-detect.test.ts new file mode 100644 index 000000000..7249ee81e --- /dev/null +++ b/src/lib/programs/__tests__/posthog-integration-detect.test.ts @@ -0,0 +1,71 @@ +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; +import { detectExistingPostHog } from '@lib/programs/posthog-integration/detect'; + +function makeTmpDir(): string { + return fs.mkdtempSync(path.join(os.tmpdir(), 'ph-detect-')); +} + +function writePackageJson( + dir: string, + pkg: { + dependencies?: Record; + devDependencies?: Record; + } = {}, +): void { + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify(pkg)); +} + +describe('detectExistingPosthog', () => { + let tmpDir: string; + let setPosthogSdkDetected: ReturnType; + + beforeEach(() => { + tmpDir = makeTmpDir(); + setPosthogSdkDetected = vi.fn(); + }); + + afterEach(() => fs.rmSync(tmpDir, { recursive: true, force: true })); + + const run = (dir: string) => + detectExistingPostHog({ setPosthogSdkDetected }, dir); + + it('reports false when no package.json exists', () => { + run(tmpDir); + expect(setPosthogSdkDetected).toHaveBeenCalledWith(false); + }); + + it('reports false when dependencies have no PostHog SDK', () => { + writePackageJson(tmpDir, { dependencies: { react: '^19.0.0' } }); + run(tmpDir); + expect(setPosthogSdkDetected).toHaveBeenCalledWith(false); + }); + + it('reports true for posthog-js in dependencies', () => { + writePackageJson(tmpDir, { dependencies: { 'posthog-js': '^1.0.0' } }); + run(tmpDir); + expect(setPosthogSdkDetected).toHaveBeenCalledWith(true); + }); + + it('reports true for posthog-node in devDependencies', () => { + writePackageJson(tmpDir, { devDependencies: { 'posthog-node': '^4.0.0' } }); + run(tmpDir); + expect(setPosthogSdkDetected).toHaveBeenCalledWith(true); + }); + + it('reports true for a PostHog SDK in a nested monorepo package', () => { + writePackageJson(tmpDir, { dependencies: {} }); + writePackageJson(path.join(tmpDir, 'apps', 'web'), { + dependencies: { 'posthog-js': '^1.0.0' }, + }); + run(tmpDir); + expect(setPosthogSdkDetected).toHaveBeenCalledWith(true); + }); + + it('does not throw and reports false for an invalid install dir', () => { + expect(() => run('/nonexistent/path')).not.toThrow(); + expect(setPosthogSdkDetected).toHaveBeenCalledWith(false); + }); +}); diff --git a/src/lib/programs/__tests__/program-registry.test.ts b/src/lib/programs/__tests__/program-registry.test.ts index 4a1baa37a..14e7c0a0b 100644 --- a/src/lib/programs/__tests__/program-registry.test.ts +++ b/src/lib/programs/__tests__/program-registry.test.ts @@ -1,6 +1,8 @@ import { PROGRAM_REGISTRY, agentSkillConfig, + getCommandPath, + getLaunchablePrograms, getProgramConfig, getSubcommandPrograms, } from '@lib/programs/program-registry'; @@ -34,7 +36,6 @@ describe('getSubcommandPrograms', () => { const subcommands = getSubcommandPrograms(); const commands = subcommands.map((c) => c.command); - expect(commands).toContain('integrate'); expect(commands).toContain('revenue-analytics'); for (const config of subcommands) { expect(config.command).toBeTruthy(); @@ -42,6 +43,60 @@ describe('getSubcommandPrograms', () => { }); }); +// What a user types to reach the program. A nested program's own `command` is +// only half of that, so anything telling a user how to run one has to join it +// to the parent. +describe('getCommandPath', () => { + const subcommand = (id: string) => + getSubcommandPrograms().find((config) => config.id === id)!; + + it('reaches a nested program through its parent', () => { + expect(getCommandPath(subcommand('web-analytics-doctor'))).toBe( + 'audit web-analytics', + ); + }); + + it('leaves a top-level program alone', () => { + expect(getCommandPath(subcommand('revenue-analytics-setup'))).toBe( + 'revenue-analytics', + ); + }); +}); + +// The programs the intro can hand off to in-session. A family parent isn't one +// of them — typing `wizard audit` opens a picker rather than running anything, +// so there's nothing to hand off to. Its leaves are still fair game. +describe('getLaunchablePrograms', () => { + const ids = () => getLaunchablePrograms().map((config) => config.id); + + it('skips a family parent', () => { + expect(ids()).not.toContain('audit'); + }); + + it('keeps the leaves under that family', () => { + expect(ids()).toContain('web-analytics-doctor'); + }); + + it('keeps the flat programs', () => { + expect(ids()).toContain('revenue-analytics-setup'); + expect(ids()).toContain('metrics'); + }); + + // Derived from who claims whom as a parent, so the next family drops out on + // its own instead of waiting for someone to remember this list. + it('drops nothing but parents', () => { + const parents = new Set( + getSubcommandPrograms().map((config) => config.parentCommand), + ); + const dropped = getSubcommandPrograms() + .filter((config) => !ids().includes(config.id)) + .map((config) => config.command); + + expect(dropped).not.toEqual([]); + expect(dropped.every((command) => parents.has(command))).toBe(true); + }); +}); + describe('parentCommand nesting', () => { it('nests web-analytics-doctor under the audit command', () => { const webAnalytics = getProgramConfig('web-analytics-doctor'); diff --git a/src/lib/programs/events-audit/index.ts b/src/lib/programs/events-audit/index.ts index f6937b2b5..ae1d8f0fd 100644 --- a/src/lib/programs/events-audit/index.ts +++ b/src/lib/programs/events-audit/index.ts @@ -20,7 +20,6 @@ export { SETUP_REPORT_FILE }; const DOCS_URL = 'https://posthog.com/docs/product-analytics/best-practices'; export const eventsAuditConfig: ProgramConfig = { - command: 'events-audit', description: 'Audit PostHog event tracking in this project', id: 'events-audit', skillId: 'events-audit', diff --git a/src/lib/programs/posthog-integration/detect.ts b/src/lib/programs/posthog-integration/detect.ts index 9404b02d0..93e701ba7 100644 --- a/src/lib/programs/posthog-integration/detect.ts +++ b/src/lib/programs/posthog-integration/detect.ts @@ -20,6 +20,7 @@ import { import { analytics } from '@utils/analytics'; import { detectWarehouseSources } from '@lib/warehouse-sources/detect'; import { DETECTED_WAREHOUSE_SOURCES_KEY } from '@lib/programs/warehouse-source/detect'; +import { findPackageJsons } from '@lib/programs/shared/package-scanning'; export async function detectPostHogIntegration( ctx: ProgramReadyContext, @@ -73,6 +74,7 @@ export async function detectPostHogIntegration( } detectWarehouseSourcesForSuggestion(ctx, installDir); + detectExistingPostHog(ctx, installDir); ctx.setDetectionComplete(); } @@ -127,3 +129,24 @@ function detectWarehouseSourcesForSuggestion( ); } } + +/** + * Scan for existing PostHog SDKs in the project. Dependency-level signal only, + * not a verified (or complete) install. Best-effort: scan failure reports + * false rather than breaking the detection step. + */ +export function detectExistingPostHog( + ctx: Pick, + installDir: string, +): void { + try { + const pkgJsons = findPackageJsons(installDir); + ctx.setPosthogSdkDetected(pkgJsons.some((p) => p.posthogSdks.length > 0)); + } catch (error) { + analytics.captureException( + error instanceof Error ? error : new Error(String(error)), + { step: 'detectExistingPosthog' }, + ); + ctx.setPosthogSdkDetected(false); + } +} diff --git a/src/lib/programs/posthog-integration/index.ts b/src/lib/programs/posthog-integration/index.ts index d6a447ecb..2b4b5e66f 100644 --- a/src/lib/programs/posthog-integration/index.ts +++ b/src/lib/programs/posthog-integration/index.ts @@ -180,7 +180,6 @@ export const SETUP_REPORT_FILE = 'posthog-setup-report.md'; export { EVENT_PLAN_FILE } from './constants.js'; export const posthogIntegrationConfig: ProgramConfig = { - command: 'integrate', description: 'Set up PostHog SDK integration', id: 'posthog-integration', agentFlow: 'integration-v2', diff --git a/src/lib/programs/program-registry.ts b/src/lib/programs/program-registry.ts index 2633c295e..a81214e15 100644 --- a/src/lib/programs/program-registry.ts +++ b/src/lib/programs/program-registry.ts @@ -135,3 +135,17 @@ export function getSubcommandPrograms(): SubcommandProgram[] { (c): c is SubcommandProgram => c.command != null, ); } + +/** What a user types to reach the program. Nested ones go through its parent. */ +export function getCommandPath(config: SubcommandProgram): string { + return config.parentCommand + ? `${config.parentCommand} ${config.command}` + : config.command; +} + +/** Programs the intro can launch, Family parents open a picker instead. */ +export function getLaunchablePrograms(): SubcommandProgram[] { + const all = getSubcommandPrograms(); + const parents = new Set(all.map((config) => config.parentCommand)); + return all.filter((config) => !parents.has(config.command)); +} diff --git a/src/lib/programs/program-step.ts b/src/lib/programs/program-step.ts index 53460b7f7..064bbb2d9 100644 --- a/src/lib/programs/program-step.ts +++ b/src/lib/programs/program-step.ts @@ -59,6 +59,7 @@ export interface ProgramReadyContext { }) => void; readonly addDiscoveredFeature: (feature: DiscoveredFeature) => void; readonly setDetectionComplete: () => void; + readonly setPosthogSdkDetected: (detected: boolean) => void; } export interface ProgramStep { diff --git a/src/lib/runners/run-wizard.ts b/src/lib/runners/run-wizard.ts index 809e7b30f..35703afb5 100644 --- a/src/lib/runners/run-wizard.ts +++ b/src/lib/runners/run-wizard.ts @@ -2,6 +2,7 @@ import { VERSION } from '@lib/version'; import { logToFile, getLogFilePath } from '@utils/debug'; import { runAgent } from '@lib/agent/agent-runner'; import { authenticate } from '@lib/agent/runner/shared/authenticate'; +import { getProgramConfig } from '@lib/programs/program-registry'; import type { ProgramConfig } from '@lib/programs/program-step'; import type { Harness, Sequence } from '@lib/constants'; import type { startTUI as StartTUIFn } from '@ui/tui/start-tui'; @@ -182,10 +183,16 @@ export function runWizard( process.on('SIGINT', onSignal); process.on('SIGTERM', onSignal); - await activeTui.store.runReadyHooks(); - // Settle the pre-run screens. `integration-check` is a no-op gate for - // programs without it. - await activeTui.store.getGate('intro'); + for (;;) { + await activeTui.store.runReadyHooks(); + // Settle the pre-run screens. `integration-check` is a no-op gate for + // programs without it. + await activeTui.store.getGate('intro'); + + const active = activeTui.store.router.activeProgram; + if (active === config.id) break; + config = getProgramConfig(active); + } await activeTui.store.getGate('integration-check'); await activeTui.store.getGate('health-check'); diff --git a/src/lib/wizard-session.ts b/src/lib/wizard-session.ts index 729106999..cfa5eea05 100644 --- a/src/lib/wizard-session.ts +++ b/src/lib/wizard-session.ts @@ -255,6 +255,9 @@ export interface WizardSession { /** Human-readable label for the detected framework variant (e.g., "Django with Wagtail CMS") */ detectedFrameworkLabel: string | null; + /** Existing PostHog detected in the project (set during detect). Signal, not proof. Currently a dependency-level check */ + posthogSdkDetected: boolean; + /** True once framework detection has run (whether it found something or not) */ detectionComplete: boolean; @@ -446,6 +449,7 @@ export function buildSession(args: { frameworkContext: {}, typescript: false, detectedFrameworkLabel: null, + posthogSdkDetected: false, detectionComplete: false, unsupportedVersion: null, diff --git a/src/ui/tui/__tests__/posthog-integration-intro.test.ts b/src/ui/tui/__tests__/posthog-integration-intro.test.ts new file mode 100644 index 000000000..b8882607e --- /dev/null +++ b/src/ui/tui/__tests__/posthog-integration-intro.test.ts @@ -0,0 +1,132 @@ +/** + * Intro-screen copy and menu decisions, extracted from + * PostHogIntegrationIntroScreen so they can be asserted without a render — + * vitest aliases `ink` to no-op stubs suite-wide (vitest.config.ts), so the + * screen itself draws nothing here. + * + * These assert the wiring, not the wording: which headline each detection state + * resolves to, and what order the menu offers. Copy edits land in one place and + * don't drag the test with them — menu order is pinned on `value` (stable + * identifiers), and the only label asserted is the one that actually branches. + */ + +import { + CONTINUE_ANYWAY_LABEL, + CONTINUE_LABEL, + DEFAULT_HEADLINE, + DETECTED_HEADLINE, + introHeadline, + introMenuOptions, +} from '@ui/tui/posthog-integration-intro'; + +const valuesFor = (args: { + view?: 'default' | 'more-info' | 'privacy' | 'commands'; + showContinue?: boolean; + posthogSdkDetected?: boolean; +}): string[] => + ( + introMenuOptions({ + view: args.view ?? 'default', + showContinue: args.showContinue ?? true, + posthogSdkDetected: args.posthogSdkDetected ?? false, + }) ?? [] + ).map((option) => option.value); + +describe('introHeadline', () => { + it('leads with the detection when PostHog is already installed', () => { + expect(introHeadline(true)).toBe(DETECTED_HEADLINE); + }); + + it('keeps the default headline for a clean project', () => { + expect(introHeadline(false)).toBe(DEFAULT_HEADLINE); + }); + + // Without literals on either side, both assertions above pass vacuously if + // the two headlines ever collapse to the same string. + it('resolves the two states to different copy', () => { + expect(DETECTED_HEADLINE).not.toBe(DEFAULT_HEADLINE); + }); +}); + +describe('introMenuOptions', () => { + describe('an install we already detected', () => { + // The ask: exploring the other commands outranks re-running an + // integration the project may not need. + it('offers the tricks before continuing', () => { + expect(valuesFor({ posthogSdkDetected: true })).toEqual([ + 'commands', + 'continue', + 'framework', + 'more-info', + 'cancel', + ]); + }); + + it('hedges the continue label', () => { + const options = introMenuOptions({ + view: 'default', + showContinue: true, + posthogSdkDetected: true, + }); + expect(options?.find((o) => o.value === 'continue')?.label).toBe( + CONTINUE_ANYWAY_LABEL, + ); + }); + }); + + describe('a clean project', () => { + it('is untouched by any of this', () => { + expect(valuesFor({ posthogSdkDetected: false })).toEqual([ + 'continue', + 'framework', + 'more-info', + 'cancel', + ]); + }); + + it('continues without the hedge', () => { + const options = introMenuOptions({ + view: 'default', + showContinue: true, + posthogSdkDetected: false, + }); + expect(options?.find((o) => o.value === 'continue')?.label).toBe( + CONTINUE_LABEL, + ); + }); + }); + + // Same reasoning as the headline pair: with neither side a literal, both + // label assertions pass vacuously if the two ever collapse. + it('distinguishes the two continue labels', () => { + expect(CONTINUE_ANYWAY_LABEL).not.toBe(CONTINUE_LABEL); + }); + + describe('the sub-views', () => { + it('gives the tricks view a way back', () => { + expect(valuesFor({ view: 'commands', posthogSdkDetected: true })).toEqual( + ['back'], + ); + }); + + it('gives the privacy view a way back', () => { + expect(valuesFor({ view: 'privacy' })).toEqual(['back']); + }); + + it('reaches privacy from more info', () => { + expect(valuesFor({ view: 'more-info' })).toEqual(['back', 'privacy']); + }); + }); + + // Detecting, picking a framework, and the unsupported-version prompt all + // clear showContinue — the screen owns the interaction, so no menu renders. + it('renders no menu when there is nothing to continue to', () => { + expect( + introMenuOptions({ + view: 'default', + showContinue: false, + posthogSdkDetected: true, + }), + ).toBeNull(); + }); +}); diff --git a/src/ui/tui/__tests__/store.test.ts b/src/ui/tui/__tests__/store.test.ts index 8643a030f..41716c2b6 100644 --- a/src/ui/tui/__tests__/store.test.ts +++ b/src/ui/tui/__tests__/store.test.ts @@ -18,6 +18,7 @@ import { buildSession } from '@lib/wizard-session'; import { HostResolution } from '@lib/host-resolution'; import { Integration } from '@lib/constants'; import { analytics } from '@utils/analytics'; +import { getProgramConfig } from '@lib/programs/program-registry'; vi.mock('../../../utils/analytics.js', () => ({ analytics: { @@ -93,6 +94,93 @@ describe('WizardStore', () => { expect(store.getVersion()).toBe(0); expect(store.getSnapshot()).toBe(0); }); + + // Picking one of the wizard's other commands from the intro runs it in + // this session instead of making the user quit and type it. Nothing has + // happened yet at that point — no auth, no agent, no files touched — so + // the switch only has to repoint the router and start the new program's + // journey from its own first screen. + describe('switchProgram', () => { + it('makes the chosen program the active one', () => { + const store = createStore(); + store.switchProgram(Program.Metrics); + expect(store.router.activeProgram).toBe(Program.Metrics); + }); + + it('routes to the new program instead of finishing the old one', () => { + const store = createStore(); + store.switchProgram(Program.Metrics); + expect(store.router.resolve(store.session)).toBe(ScreenId.MetricsIntro); + }); + + // The old intro is behind us, but every program gates its intro on the + // same `setupConfirmed` flag — leaving it set marks the new program's + // intro complete before the user has seen it. + it('does not carry the old confirmation into the new intro', () => { + const store = createStore(); + store.completeSetup(); + expect(store.session.setupConfirmed).toBe(true); + + store.switchProgram(Program.Metrics); + + expect(store.session.setupConfirmed).toBe(false); + expect(store.router.resolve(store.session)).toBe(ScreenId.MetricsIntro); + }); + + // bin.ts parks on these gates. They resolved for the program we left, so + // reusing them would run the new program past its own screens. + it('reopens the gates for the new program', async () => { + const store = createStore(); + const before = store.getGate('intro'); + store.completeSetup(); + await expect(before).resolves.toBeUndefined(); + + store.switchProgram(Program.Metrics); + + const after = store.getGate('intro'); + expect(after).not.toBe(before); + await expect( + Promise.race([after, Promise.resolve('pending')]), + ).resolves.toBe('pending'); + }); + + // The runner is parked on the old program's intro gate at the moment of + // the switch. Dropping that promise without resolving it strands the + // runner — no error, no new program, just a wizard that stops. + it('releases callers parked on the old gates', async () => { + const store = createStore(); + const parked = store.getGate('intro'); + + store.switchProgram(Program.Metrics); + + await expect(parked).resolves.toBeUndefined(); + }); + + it('reports screens under the new program', () => { + const store = createStore(); + store.switchProgram(Program.Metrics); + expect(store.analyticsProgramId).toBe(Program.Metrics); + }); + + it('follows the new program for label and skill', () => { + const store = createStore(); + store.switchProgram(Program.Metrics); + expect(store.session.programLabel).toBe(Program.Metrics); + expect(store.session.skillId).toBe( + getProgramConfig(Program.Metrics).skillId ?? null, + ); + }); + + // Selecting the program already running should cost the user nothing — + // in particular it must not throw away a confirmation they just gave. + it('leaves the session alone when the program is unchanged', () => { + const store = createStore(); + store.completeSetup(); + store.switchProgram(Program.PostHogIntegration); + expect(store.session.setupConfirmed).toBe(true); + expect(store.router.activeProgram).toBe(Program.PostHogIntegration); + }); + }); }); // ── Change notification ────────────────────────────────────────── @@ -215,6 +303,13 @@ describe('WizardStore', () => { expect(store.session.detectionComplete).toBe(true); }); + it('setPosthogSdkDetected stores the verdict', () => { + const store = createStore(); + expect(store.session.posthogSdkDetected).toBe(false); + store.setPosthogSdkDetected(true); + expect(store.session.posthogSdkDetected).toBe(true); + }); + it('setDetectedFramework sets the label', () => { const store = createStore(); store.setDetectedFramework('Django'); diff --git a/src/ui/tui/posthog-integration-intro.ts b/src/ui/tui/posthog-integration-intro.ts new file mode 100644 index 000000000..395aac38f --- /dev/null +++ b/src/ui/tui/posthog-integration-intro.ts @@ -0,0 +1,50 @@ +export type IntroMenuView = 'default' | 'more-info' | 'privacy' | 'commands'; + +export const CONTINUE_LABEL = 'Continue'; +export const CONTINUE_ANYWAY_LABEL = 'Continue anyway'; + +export const DEFAULT_HEADLINE = "Let's do two hours of work in eight minutes."; +export const DETECTED_HEADLINE = + 'Looks like you already have PostHog installed.'; + +export function introHeadline(posthogSdkDetected: boolean): string { + return posthogSdkDetected ? DETECTED_HEADLINE : DEFAULT_HEADLINE; +} + +export function introMenuOptions({ + view, + showContinue, + posthogSdkDetected, +}: { + view: IntroMenuView; + showContinue: boolean; + posthogSdkDetected: boolean; +}): { label: string; value: string }[] | null { + if (view === 'more-info') { + return [ + { label: 'Back', value: 'back' }, + { label: 'Privacy & data usage', value: 'privacy' }, + ]; + } + + if (view === 'privacy' || view === 'commands') { + return [{ label: 'Back', value: 'back' }]; + } + + if (showContinue) { + return [ + ...(posthogSdkDetected + ? [{ label: 'Explore wizard tricks', value: 'commands' }] + : []), + { + label: posthogSdkDetected ? CONTINUE_ANYWAY_LABEL : CONTINUE_LABEL, + value: 'continue', + }, + { label: 'Change framework', value: 'framework' }, + { label: 'More info', value: 'more-info' }, + { label: 'Cancel', value: 'cancel' }, + ]; + } + + return null; +} diff --git a/src/ui/tui/router.ts b/src/ui/tui/router.ts index c9b887072..60a6669ff 100644 --- a/src/ui/tui/router.ts +++ b/src/ui/tui/router.ts @@ -50,8 +50,14 @@ export class WizardRouter { private overlays: Overlay[] = []; constructor(programId: ProgramId = Program.PostHogIntegration) { + this.setProgram(programId); + } + + /** Point the router at a different program. */ + setProgram(programId: ProgramId): void { this.programId = programId; this.sequence = PROGRAM_SEQUENCES[programId]; + this.overlays = []; } /** diff --git a/src/ui/tui/screens/PostHogIntegrationIntroScreen.tsx b/src/ui/tui/screens/PostHogIntegrationIntroScreen.tsx index 6781f4047..72b19c10b 100644 --- a/src/ui/tui/screens/PostHogIntegrationIntroScreen.tsx +++ b/src/ui/tui/screens/PostHogIntegrationIntroScreen.tsx @@ -13,13 +13,20 @@ import type { ReactNode } from 'react'; import { useState, useSyncExternalStore } from 'react'; import type { WizardStore } from '@ui/tui/store'; import { Integration } from '@lib/constants'; +import { + getCommandPath, + getLaunchablePrograms, +} from '@lib/programs/program-registry'; import { PickerMenu, LoadingBox } from '@ui/tui/primitives/index'; import { IntroScreenLayout, type DetectionRow } from './IntroScreenLayout.js'; import { SkillSourceInfo, useSkillEntry } from './SkillSourceInfo.js'; import { PrivacyPanel } from '@ui/tui/components/PrivacyPanel'; import { analytics } from '@utils/analytics'; - -type View = 'default' | 'more-info' | 'privacy'; +import type { IntroMenuView } from '@ui/tui/posthog-integration-intro'; +import { + introHeadline, + introMenuOptions, +} from '@ui/tui/posthog-integration-intro'; /** Framework picker shown when auto-detection fails. */ const FrameworkPicker = ({ @@ -67,7 +74,7 @@ export const PostHogIntegrationIntroScreen = ({ const [pickingFramework, setPickingFramework] = useState(false); const [manuallySelected, setManuallySelected] = useState(false); - const [view, setView] = useState('default'); + const [view, setView] = useState('default'); const { session } = store; const config = session.frameworkConfig; @@ -158,15 +165,27 @@ export const PostHogIntegrationIntroScreen = ({ ); + } else if (view === 'commands') { + body = ( + ({ + label: `${getCommandPath(program).padEnd(21)}${program.description}`, + value: program.id, + }))} + onSelect={(value) => { + const id = Array.isArray(value) ? value[0] : value; + store.switchProgram(id); + }} + /> + ); } else if (view === 'privacy') { body = ; } else if (showContinue) { body = ( - <> - - Let's do two hours of work in eight minutes. - - + + {introHeadline(session.posthogSdkDetected)} + ); } @@ -187,6 +206,13 @@ export const PostHogIntegrationIntroScreen = ({ }); } + if (session.posthogSdkDetected) { + detectionRows.push({ + label: 'PostHog', + value: 'detected in package.json', + }); + } + // ── Children (between rows and menu) ─────────────────────────────── let bodyChildren: ReactNode = null; @@ -229,23 +255,11 @@ export const PostHogIntegrationIntroScreen = ({ // ── Menu ─────────────────────────────────────────────────────────── - let menuOptions: { label: string; value: string }[] | null = null; - - if (view === 'more-info') { - menuOptions = [ - { label: 'Back', value: 'back' }, - { label: 'Privacy & data usage', value: 'privacy' }, - ]; - } else if (view === 'privacy') { - menuOptions = [{ label: 'Back', value: 'back' }]; - } else if (showContinue) { - menuOptions = [ - { label: 'Continue', value: 'continue' }, - { label: 'Change framework', value: 'framework' }, - { label: 'More info', value: 'more-info' }, - { label: 'Cancel', value: 'cancel' }, - ]; - } + const menuOptions = introMenuOptions({ + view, + showContinue, + posthogSdkDetected: session.posthogSdkDetected, + }); const handleSelect = (value: string) => { analytics.wizardCapture('intro menu selected', { value, view }); @@ -256,6 +270,8 @@ export const PostHogIntegrationIntroScreen = ({ setManuallySelected(true); } else if (value === 'more-info') { setView('more-info'); + } else if (value === 'commands') { + setView('commands'); } else if (value === 'privacy') { setView('privacy'); } else if (value === 'back') { diff --git a/src/ui/tui/store.ts b/src/ui/tui/store.ts index eeb818d27..13c5c8ffa 100644 --- a/src/ui/tui/store.ts +++ b/src/ui/tui/store.ts @@ -301,6 +301,7 @@ export class WizardStore { setFrameworkContext: (k, v) => this.setFrameworkContext(k, v), setFrameworkConfig: (i, c) => this.setFrameworkConfig(i, c), setDetectedFramework: (l) => this.setDetectedFramework(l), + setPosthogSdkDetected: (d) => this.setPosthogSdkDetected(d), setSkillId: (id) => this.setSkillId(id), setUnsupportedVersion: (info) => this.setUnsupportedVersion(info), addDiscoveredFeature: (f) => this.addDiscoveredFeature(f), @@ -484,6 +485,11 @@ export class WizardStore { this.emitChange(); } + setPosthogSdkDetected(detected: boolean): void { + this.$session.setKey('posthogSdkDetected', detected); + this.emitChange(); + } + setSkillId(skillId: string | null): void { this.$session.setKey('skillId', skillId); this.emitChange(); @@ -869,6 +875,23 @@ export class WizardStore { this.emitChange(); } + switchProgram(program: ProgramId): void { + if (program === this.router.activeProgram) return; + + // Flush unresolved promises so the wizard can advance + for (const gate of this._gates.values()) gate.resolve(); + this._gates.clear(); + + this.router.setProgram(program); + this._initFromProgram(program); + + const config = getProgramConfig(program); + this.$session.setKey('setupConfirmed', false); + this.$session.setKey('programLabel', config.id); + this.$session.setKey('skillId', config.skillId ?? null); + this.emitChange(); + } + // ── Derived state ─────────────────────────────────────────────── /** diff --git a/src/utils/__tests__/analytics.test.ts b/src/utils/__tests__/analytics.test.ts index c8943957b..da8c28b81 100644 --- a/src/utils/__tests__/analytics.test.ts +++ b/src/utils/__tests__/analytics.test.ts @@ -1,4 +1,5 @@ -import { Analytics, groupsFromUser } from '@utils/analytics'; +import { Analytics, groupsFromUser, sessionProperties } from '@utils/analytics'; +import { buildSession } from '@lib/wizard-session'; import { PostHog } from 'posthog-node'; import { v4 as uuidv4 } from 'uuid'; import { ANALYTICS_TEAM_TAG, WIZARD_FLAG_KEYS } from '@lib/constants'; @@ -632,6 +633,16 @@ describe('Analytics', () => { }); }); + describe('sessionProperties', () => { + it('includes the posthog_sdk_detected verdict', () => { + const session = buildSession({}); + expect(sessionProperties(session).posthog_sdk_detected).toBe(false); + + session.posthogSdkDetected = true; + expect(sessionProperties(session).posthog_sdk_detected).toBe(true); + }); + }); + describe('groupsFromUser', () => { const userWith = (overrides: Partial): ApiUser => ({ diff --git a/src/utils/analytics.ts b/src/utils/analytics.ts index 450e03b0e..66b8e8372 100644 --- a/src/utils/analytics.ts +++ b/src/utils/analytics.ts @@ -46,6 +46,7 @@ export function sessionProperties( discovered_features: session.discoveredFeatures, additional_features: session.additionalFeatureQueue, run_phase: session.runPhase, + posthog_sdk_detected: session.posthogSdkDetected, }; } From d793c47ac922110a82ea4b50639140d8a9dc274a Mon Sep 17 00:00:00 2001 From: "Vincent (Wen Yu) Ge" Date: Tue, 1 Sep 2026 15:21:57 -0400 Subject: [PATCH 02/12] feat(posthog-integration): say what the wizard can do instead when PostHog is found MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The detected headline stated the detection and stopped there, leaving the re-run user a fact and no reason to pick either option. It now names three things worth doing on a project that already has PostHog, and says plainly that continuing may overwrite work — which is the fear the original issue quotes, in the words the reporter used. Two paragraphs rather than one line, so `introHeadline` returns `string[]` and the screen renders them in the same 64-column block `more-info` uses. A single line still centers; a wrapped block aligns left, since centered prose reads ragged. The "PostHog ✔" row's label is padded to the width the layout hardcodes for its own rows. Unpadded, its tick sat two columns left of the three above it. Generated-By: PostHog Desktop Task-Id: 5a773d7d-c2dc-444b-a8dd-6eea3ebcadc3 --- .../posthog-integration-intro.test.ts | 15 +++++++++---- src/ui/tui/posthog-integration-intro.ts | 19 ++++++++++------ .../screens/PostHogIntegrationIntroScreen.tsx | 22 +++++++++++++++---- 3 files changed, 41 insertions(+), 15 deletions(-) diff --git a/src/ui/tui/__tests__/posthog-integration-intro.test.ts b/src/ui/tui/__tests__/posthog-integration-intro.test.ts index 296b8adec..06e632a66 100644 --- a/src/ui/tui/__tests__/posthog-integration-intro.test.ts +++ b/src/ui/tui/__tests__/posthog-integration-intro.test.ts @@ -34,17 +34,24 @@ const valuesFor = (args: { describe('introHeadline', () => { it('leads with the detection when PostHog is already installed', () => { - expect(introHeadline(true)).toBe(DETECTED_HEADLINE); + expect(introHeadline(true)).toEqual(DETECTED_HEADLINE); }); it('keeps the default headline for a clean project', () => { - expect(introHeadline(false)).toBe(DEFAULT_HEADLINE); + expect(introHeadline(false)).toEqual([DEFAULT_HEADLINE]); }); // Without literals on either side, both assertions above pass vacuously if - // the two headlines ever collapse to the same string. + // the two headlines ever collapse to the same copy. it('resolves the two states to different copy', () => { - expect(DETECTED_HEADLINE).not.toBe(DEFAULT_HEADLINE); + expect(DETECTED_HEADLINE).not.toEqual([DEFAULT_HEADLINE]); + }); + + // The screen centers a single line and left-aligns a block, so the count is + // a layout decision rather than an incidental shape. + it('keeps the clean project to one line and says more on a detection', () => { + expect(introHeadline(false)).toHaveLength(1); + expect(introHeadline(true).length).toBeGreaterThan(1); }); }); diff --git a/src/ui/tui/posthog-integration-intro.ts b/src/ui/tui/posthog-integration-intro.ts index de757328c..1a94a0f3c 100644 --- a/src/ui/tui/posthog-integration-intro.ts +++ b/src/ui/tui/posthog-integration-intro.ts @@ -4,16 +4,21 @@ export type IntroMenuView = 'default' | 'more-info' | 'commands'; export const CONTINUE_LABEL = 'Continue'; export const CONTINUE_ANYWAY_LABEL = 'Continue anyway'; -// Shorter than the "wizard tricks" this was written as: IntroScreenLayout -// renders the menu in a 24-column box, and the label past that wraps mid-word. -export const COMMANDS_LABEL = 'Explore tricks'; +// Two words shorter than the prose calls it: IntroScreenLayout renders the +// menu in a 24-column box, and a label past that wraps mid-word. +export const COMMANDS_LABEL = 'Explore spell book'; export const DEFAULT_HEADLINE = "Let's do two hours of work in eight minutes."; -export const DETECTED_HEADLINE = - 'Looks like you already have PostHog installed.'; +export const DETECTED_HEADLINE = [ + 'It looks like PostHog is already installed. The Wizard has many tricks ' + + 'up its sleeve, like auditing, uploading source maps, or making your ' + + 'product self-drive.', + 'You can still rerun the command, but it might overwrite some of your work.', +]; -export function introHeadline(posthogSdkDetected: boolean): string { - return posthogSdkDetected ? DETECTED_HEADLINE : DEFAULT_HEADLINE; +/** Paragraphs, so the detected state can say more than the clean one. */ +export function introHeadline(posthogSdkDetected: boolean): string[] { + return posthogSdkDetected ? DETECTED_HEADLINE : [DEFAULT_HEADLINE]; } export function introMenuOptions({ diff --git a/src/ui/tui/screens/PostHogIntegrationIntroScreen.tsx b/src/ui/tui/screens/PostHogIntegrationIntroScreen.tsx index 796244645..9fc03b89f 100644 --- a/src/ui/tui/screens/PostHogIntegrationIntroScreen.tsx +++ b/src/ui/tui/screens/PostHogIntegrationIntroScreen.tsx @@ -216,7 +216,7 @@ export const PostHogIntegrationIntroScreen = ({ } else if (view === 'commands') { body = ( ({ label: `${getCommandPath(program).padEnd(21)}${program.description}`, value: program.id, @@ -228,9 +228,21 @@ export const PostHogIntegrationIntroScreen = ({ /> ); } else if (showContinue) { + const paragraphs = introHeadline(session.posthogSdkDetected); body = ( - - {introHeadline(session.posthogSdkDetected)} + 1 ? undefined : 'center'} + > + {paragraphs.map((paragraph, i) => ( + + {paragraph} + + ))} ); } @@ -254,7 +266,9 @@ export const PostHogIntegrationIntroScreen = ({ if (session.posthogSdkDetected) { detectionRows.push({ - label: 'PostHog', + // Padded to the width the layout hardcodes for its own rows, so the + // ticks line up in one column. + label: 'PostHog ', value: 'detected in package.json', }); } From e5ebb22c0c14e34e98a20d479c16cd9569d1b5d9 Mon Sep 17 00:00:00 2001 From: "Vincent (Wen Yu) Ge" Date: Tue, 1 Sep 2026 15:22:03 -0400 Subject: [PATCH 03/12] feat(programs): lead the intro's command list with the three the copy names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The list sits directly under a sentence promising auditing, source maps and self-drive, but rendered in `PROGRAM_REGISTRY` order — so it opened with revenue-analytics, and whatever program was appended most recently would keep displacing what the copy had just promised. Featured ids are an explicit list rather than a reordering of the registry: the registry drives CLI dispatch, and this is a presentation concern. The sort is stable, so everything unfeatured keeps registry order behind them and a new program lands at the bottom. Generated-By: PostHog Desktop Task-Id: 5a773d7d-c2dc-444b-a8dd-6eea3ebcadc3 --- .../__tests__/program-registry.test.ts | 20 +++++++++++++++++++ src/lib/programs/program-registry.ts | 19 +++++++++++++++++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/lib/programs/__tests__/program-registry.test.ts b/src/lib/programs/__tests__/program-registry.test.ts index 14e7c0a0b..bdc696f0c 100644 --- a/src/lib/programs/__tests__/program-registry.test.ts +++ b/src/lib/programs/__tests__/program-registry.test.ts @@ -82,6 +82,26 @@ describe('getLaunchablePrograms', () => { expect(ids()).toContain('metrics'); }); + // The intro copy names auditing, source maps and self-drive, and the list + // sits directly under that sentence. Registry order alone would put whatever + // was appended most recently in front of them. + it('leads with the three the intro copy names', () => { + expect(ids().slice(0, 3)).toEqual([ + 'web-analytics-doctor', + 'error-tracking-upload-source-maps', + 'self-driving', + ]); + }); + + it('leaves everything else in registry order', () => { + const rest = ids().slice(3); + const registryOrder = getSubcommandPrograms() + .map((config) => config.id) + .filter((id) => rest.includes(id)); + + expect(rest).toEqual(registryOrder); + }); + // Derived from who claims whom as a parent, so the next family drops out on // its own instead of waiting for someone to remember this list. it('drops nothing but parents', () => { diff --git a/src/lib/programs/program-registry.ts b/src/lib/programs/program-registry.ts index a81214e15..014def7d2 100644 --- a/src/lib/programs/program-registry.ts +++ b/src/lib/programs/program-registry.ts @@ -143,9 +143,26 @@ export function getCommandPath(config: SubcommandProgram): string { : config.command; } +/** + * Leads the intro's list, because the intro copy names these three. A new + * program lands at the bottom rather than displacing what the copy promised. + */ +const FEATURED_LAUNCHABLE_PROGRAMS = [ + 'web-analytics-doctor', + 'error-tracking-upload-source-maps', + 'self-driving', +]; + /** Programs the intro can launch, Family parents open a picker instead. */ export function getLaunchablePrograms(): SubcommandProgram[] { const all = getSubcommandPrograms(); const parents = new Set(all.map((config) => config.parentCommand)); - return all.filter((config) => !parents.has(config.command)); + const launchable = all.filter((config) => !parents.has(config.command)); + + const rank = (config: SubcommandProgram): number => { + const i = FEATURED_LAUNCHABLE_PROGRAMS.indexOf(config.id); + return i === -1 ? FEATURED_LAUNCHABLE_PROGRAMS.length : i; + }; + // Sort is stable, so everything unfeatured keeps registry order behind them. + return launchable.slice().sort((a, b) => rank(a) - rank(b)); } From d2087d49899e1cc1147e141ed409d3124ebacbf4 Mon Sep 17 00:00:00 2001 From: "Vincent (Wen Yu) Ge" Date: Tue, 1 Sep 2026 15:50:23 -0400 Subject: [PATCH 04/12] fix(tui): stop the command list and the menu below it sharing one cursor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `useKeyBindings` gives every mounted component every keystroke — there is no focus scope. The command list is a picker in the intro's `body` slot, so the `Back` menu the layout rendered underneath it was a second picker: one arrow key moved both cursors, and the screen showed two focus markers. That view now renders without a menu and binds Esc to go back, which is how the framework picker has always worked — it clears `showContinue` while it's up so nothing else is navigable. Esc registers only for that view, so no other view advertises a hint it doesn't need. Selecting a command also captures `intro menu selected` now, the same event the menu rows fire. Which trick a re-run user picks is the measure of whether offering them beat re-integrating, and the picker was the one path that reported nothing. Generated-By: PostHog Desktop Task-Id: 5a773d7d-c2dc-444b-a8dd-6eea3ebcadc3 --- .../posthog-integration-intro.test.ts | 15 +++++++++---- src/ui/tui/posthog-integration-intro.ts | 12 ++++++++--- .../screens/PostHogIntegrationIntroScreen.tsx | 21 +++++++++++++++++++ 3 files changed, 41 insertions(+), 7 deletions(-) diff --git a/src/ui/tui/__tests__/posthog-integration-intro.test.ts b/src/ui/tui/__tests__/posthog-integration-intro.test.ts index 06e632a66..57c77d611 100644 --- a/src/ui/tui/__tests__/posthog-integration-intro.test.ts +++ b/src/ui/tui/__tests__/posthog-integration-intro.test.ts @@ -110,10 +110,17 @@ describe('introMenuOptions', () => { }); describe('the sub-views', () => { - it('gives the tricks view a way back', () => { - expect(valuesFor({ view: 'commands', posthogSdkDetected: true })).toEqual( - ['back'], - ); + // Nothing scopes arrow keys to one picker. The command list is a picker in + // the body slot, so a menu here would move both cursors at once — the bug + // this replaced. The screen binds Esc for that view instead. + it('renders no menu under the command list', () => { + expect( + introMenuOptions({ + view: 'commands', + showContinue: true, + posthogSdkDetected: true, + }), + ).toBeNull(); }); // IntroScreenLayout appends the disclosure row to every intro menu, so diff --git a/src/ui/tui/posthog-integration-intro.ts b/src/ui/tui/posthog-integration-intro.ts index 1a94a0f3c..dff472455 100644 --- a/src/ui/tui/posthog-integration-intro.ts +++ b/src/ui/tui/posthog-integration-intro.ts @@ -30,9 +30,15 @@ export function introMenuOptions({ showContinue: boolean; posthogSdkDetected: boolean; }): PickerOption[] | null { - // No route to the disclosure panel from either sub-view: it has its own - // top-level row, which IntroScreenLayout appends to every intro menu. - if (view === 'more-info' || view === 'commands') { + // Nothing scopes arrow keys to one picker, so a menu under the command list + // would move both cursors at once. That view puts a picker in the body slot + // and owns the interaction; Esc is its way back. Same shape as the framework + // picker, which clears the menu while it's up. + if (view === 'commands') return null; + + // No route to the disclosure panel from here: it has its own top-level row, + // which IntroScreenLayout appends to every intro menu. + if (view === 'more-info') { return [{ label: 'Back', value: 'back' }]; } diff --git a/src/ui/tui/screens/PostHogIntegrationIntroScreen.tsx b/src/ui/tui/screens/PostHogIntegrationIntroScreen.tsx index 9fc03b89f..94b63a737 100644 --- a/src/ui/tui/screens/PostHogIntegrationIntroScreen.tsx +++ b/src/ui/tui/screens/PostHogIntegrationIntroScreen.tsx @@ -25,6 +25,7 @@ import { import { IntroScreenLayout, type DetectionRow } from './IntroScreenLayout.js'; import { SkillSourceInfo, useSkillEntry } from './SkillSourceInfo.js'; import { ScanConsent } from '@lib/wizard-session'; +import { KeyMatch, useKeyBindings } from '@ui/tui/hooks/useKeyBindings'; import { Icons } from '@ui/tui/styles'; import { analytics } from '@utils/analytics'; import { PRIVACY_PANEL_LABEL } from '@ui/tui/components/PrivacyPanel'; @@ -145,6 +146,23 @@ export const PostHogIntegrationIntroScreen = ({ view === 'default' && !unsupported; + // The command list is the only view whose body is itself a picker, so it + // renders without a menu beneath it and needs its own way out. Empty + // bindings elsewhere keep the hint off every view that has a Back row. + useKeyBindings( + 'posthog-integration-intro', + view === 'commands' + ? [ + { + match: KeyMatch.Escape, + label: 'esc', + action: 'back', + handler: () => setView('default'), + }, + ] + : [], + ); + // ── Title ────────────────────────────────────────────────────────── const title = detecting ? 'PostHog Wizard starting up' : 'PostHog Wizard 🦔'; @@ -223,6 +241,9 @@ export const PostHogIntegrationIntroScreen = ({ }))} onSelect={(value) => { const id = Array.isArray(value) ? value[0] : value; + // Same event as the menu rows: which trick a re-run user picks is + // the measure of whether offering them beat re-integrating. + analytics.wizardCapture('intro menu selected', { value: id, view }); store.switchProgram(id); }} /> From eecee848b833367e1de1c246473d4cddb3632904 Mon Sep 17 00:00:00 2001 From: "Vincent (Wen Yu) Ge" Date: Tue, 1 Sep 2026 15:55:17 -0400 Subject: [PATCH 05/12] fix(programs): shorten three descriptions so the intro's command list centers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The intro renders each program as "" on one line inside a block it centers. A row wider than the terminal stops the block from centering at all — it pins to the left edge while the title above it stays centered, which is what made the list look misaligned rather than merely wide. Three rows were over an 80-column terminal: doctor at 94, warehouse at 85, revenue-analytics at 79. The other eight already fit. What pushed those three over was a trailing parenthetical of examples, or a clause restating what the command name says — so the short forms are better `--help` lines too, which is the other place these strings render. A test pins every launchable row inside the 80-column budget, so the next long description fails there rather than silently un-centering the list. Generated-By: PostHog Desktop Task-Id: 5a773d7d-c2dc-444b-a8dd-6eea3ebcadc3 --- .../programs/__tests__/program-registry.test.ts | 17 +++++++++++++++++ src/lib/programs/posthog-doctor/index.ts | 3 +-- src/lib/programs/revenue-analytics/index.ts | 2 +- src/lib/programs/warehouse-source/index.ts | 3 +-- 4 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/lib/programs/__tests__/program-registry.test.ts b/src/lib/programs/__tests__/program-registry.test.ts index bdc696f0c..449c6c69e 100644 --- a/src/lib/programs/__tests__/program-registry.test.ts +++ b/src/lib/programs/__tests__/program-registry.test.ts @@ -82,6 +82,23 @@ describe('getLaunchablePrograms', () => { expect(ids()).toContain('metrics'); }); + // The intro renders these as "" on one line, in a + // block it centers. A row past the terminal's width stops the block from + // centering at all — it pins to the left edge, which is how three overlong + // descriptions made the whole list look misaligned. 80 columns less the + // command column and the focus marker is what a row has to live within. + it('keeps every row inside an 80-column terminal', () => { + const COMMAND_COLUMN = 21; + const MARKER_PREFIX = 2; + const BUDGET = 80 - COMMAND_COLUMN - MARKER_PREFIX; + + const tooLong = getLaunchablePrograms() + .filter((config) => config.description.length > BUDGET) + .map((config) => `${config.id} (${config.description.length})`); + + expect(tooLong).toEqual([]); + }); + // The intro copy names auditing, source maps and self-drive, and the list // sits directly under that sentence. Registry order alone would put whatever // was appended most recently in front of them. diff --git a/src/lib/programs/posthog-doctor/index.ts b/src/lib/programs/posthog-doctor/index.ts index 828b6fb4b..8ea70acf1 100644 --- a/src/lib/programs/posthog-doctor/index.ts +++ b/src/lib/programs/posthog-doctor/index.ts @@ -4,8 +4,7 @@ import { POSTHOG_DOCTOR_PROGRAM } from './steps.js'; export const posthogDoctorConfig: ProgramConfig = { command: 'doctor', - description: - 'Diagnose your PostHog project for configuration issues and setup warnings', + description: 'Diagnose your PostHog project setup', id: 'posthog-doctor', requiresAi: false, steps: POSTHOG_DOCTOR_PROGRAM, diff --git a/src/lib/programs/revenue-analytics/index.ts b/src/lib/programs/revenue-analytics/index.ts index 27a30ffa2..08b0f48a9 100644 --- a/src/lib/programs/revenue-analytics/index.ts +++ b/src/lib/programs/revenue-analytics/index.ts @@ -6,7 +6,7 @@ import { getContentBlocks } from './content/index.js'; export const revenueAnalyticsConfig: ProgramConfig = { command: 'revenue-analytics', - description: 'Set up PostHog revenue analytics (e.g. Stripe integration)', + description: 'Set up PostHog revenue analytics', id: 'revenue-analytics-setup', skillId: 'revenue-analytics-setup', steps: REVENUE_ANALYTICS_PROGRAM, diff --git a/src/lib/programs/warehouse-source/index.ts b/src/lib/programs/warehouse-source/index.ts index 8667a6b7e..c09883e72 100644 --- a/src/lib/programs/warehouse-source/index.ts +++ b/src/lib/programs/warehouse-source/index.ts @@ -42,8 +42,7 @@ function buildPrompt(session: WizardSession): string { export const warehouseSourceConfig: ProgramConfig = { command: 'warehouse', - description: - 'Detect and connect a data warehouse source (Postgres, Stripe, …)', + description: 'Detect and connect a data warehouse source', id: 'warehouse-source', skillId: 'data-warehouse-source-setup', steps: WAREHOUSE_SOURCE_PROGRAM, From 6e760dd0d3470f95cd53a27702c9087b395f275f Mon Sep 17 00:00:00 2001 From: "Vincent (Wen Yu) Ge" Date: Tue, 1 Sep 2026 16:02:06 -0400 Subject: [PATCH 06/12] fix(tui): align a picker's filter row with its message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PromptLabel carries a leading space; the filter row below it was flush left, so on every filterable picker with a message the two header lines sat one column apart. Indenting the filter row is the narrow fix: PromptLabel is also used by ConfirmationInput and GroupedPickerMenu, which have no marker column to hang, so changing it there would move more than this. The marker column still hangs one column left of both, with option labels one right — the header lines now agree with each other rather than each landing on its own column. Generated-By: PostHog Desktop Task-Id: 5a773d7d-c2dc-444b-a8dd-6eea3ebcadc3 --- src/ui/tui/primitives/PickerMenu.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ui/tui/primitives/PickerMenu.tsx b/src/ui/tui/primitives/PickerMenu.tsx index d9b43260c..399e4e228 100644 --- a/src/ui/tui/primitives/PickerMenu.tsx +++ b/src/ui/tui/primitives/PickerMenu.tsx @@ -294,7 +294,9 @@ const FilterRow = ({ shown: number; total: number; }) => ( - + // Indented to sit under PromptLabel, which carries a leading space of its + // own. Flush left, this row hangs one column left of the message above it. + {filter ? `Filter: ${filter} (${shown} of ${total})` From 0b83da63b651d6865ab6e6c620f88f94db83271b Mon Sep 17 00:00:00 2001 From: "Vincent (Wen Yu) Ge" Date: Tue, 1 Sep 2026 21:38:08 -0400 Subject: [PATCH 07/12] refactor(tui): fix detection-row alignment at the layout, and trim the noise MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit pass over this branch's own commits. The detection block hardcoded its label padding three separate ways — `Directory` bare at 9, `Program` at 7 plus two literal spaces, `Skill` at 5 plus five — so a row label of any other length put its tick in its own column. `PostHog ` with trailing spaces was a fourth copy of that, working around the block rather than fixing it. One `DetectionLine` now pads to the widest label a screen passes, which also lines up `PostHog SDK` on the revenue intro, wrong since before this branch. Everything else here is subtraction: the featured-program ordering reads as "these, then the rest" rather than a rank function fed to a stable sort; COMMANDS_LABEL is inlined at its one use; two tests that could not fail are gone (one asserted two constants differ, one restated a sibling); the label column test no longer walks a view that renders no menu; and the comments that narrated their own code are cut to the one line that says why. Generated-By: PostHog Desktop Task-Id: 5a773d7d-c2dc-444b-a8dd-6eea3ebcadc3 --- .../__tests__/program-registry.test.ts | 19 +---- src/lib/programs/program-registry.ts | 19 +++-- .../posthog-integration-intro.test.ts | 11 +-- src/ui/tui/posthog-integration-intro.ts | 13 +--- src/ui/tui/primitives/PickerMenu.tsx | 3 +- src/ui/tui/screens/IntroScreenLayout.tsx | 72 ++++++++++++------- .../screens/PostHogIntegrationIntroScreen.tsx | 13 +--- .../__tests__/IntroScreenLayout.test.ts | 20 +++++- .../PostHogIntegrationIntroScreen.test.ts | 13 ++-- 9 files changed, 88 insertions(+), 95 deletions(-) diff --git a/src/lib/programs/__tests__/program-registry.test.ts b/src/lib/programs/__tests__/program-registry.test.ts index 449c6c69e..612d86d3f 100644 --- a/src/lib/programs/__tests__/program-registry.test.ts +++ b/src/lib/programs/__tests__/program-registry.test.ts @@ -82,11 +82,7 @@ describe('getLaunchablePrograms', () => { expect(ids()).toContain('metrics'); }); - // The intro renders these as "" on one line, in a - // block it centers. A row past the terminal's width stops the block from - // centering at all — it pins to the left edge, which is how three overlong - // descriptions made the whole list look misaligned. 80 columns less the - // command column and the focus marker is what a row has to live within. + // A row wider than the terminal stops the whole block from centering. it('keeps every row inside an 80-column terminal', () => { const COMMAND_COLUMN = 21; const MARKER_PREFIX = 2; @@ -99,9 +95,7 @@ describe('getLaunchablePrograms', () => { expect(tooLong).toEqual([]); }); - // The intro copy names auditing, source maps and self-drive, and the list - // sits directly under that sentence. Registry order alone would put whatever - // was appended most recently in front of them. + // The intro copy promises these three by name, directly above the list. it('leads with the three the intro copy names', () => { expect(ids().slice(0, 3)).toEqual([ 'web-analytics-doctor', @@ -110,15 +104,6 @@ describe('getLaunchablePrograms', () => { ]); }); - it('leaves everything else in registry order', () => { - const rest = ids().slice(3); - const registryOrder = getSubcommandPrograms() - .map((config) => config.id) - .filter((id) => rest.includes(id)); - - expect(rest).toEqual(registryOrder); - }); - // Derived from who claims whom as a parent, so the next family drops out on // its own instead of waiting for someone to remember this list. it('drops nothing but parents', () => { diff --git a/src/lib/programs/program-registry.ts b/src/lib/programs/program-registry.ts index 014def7d2..0330bb78d 100644 --- a/src/lib/programs/program-registry.ts +++ b/src/lib/programs/program-registry.ts @@ -143,10 +143,7 @@ export function getCommandPath(config: SubcommandProgram): string { : config.command; } -/** - * Leads the intro's list, because the intro copy names these three. A new - * program lands at the bottom rather than displacing what the copy promised. - */ +/** Leads the intro's list, in this order, because the intro copy names them. */ const FEATURED_LAUNCHABLE_PROGRAMS = [ 'web-analytics-doctor', 'error-tracking-upload-source-maps', @@ -158,11 +155,13 @@ export function getLaunchablePrograms(): SubcommandProgram[] { const all = getSubcommandPrograms(); const parents = new Set(all.map((config) => config.parentCommand)); const launchable = all.filter((config) => !parents.has(config.command)); + const featured = (config: SubcommandProgram) => + FEATURED_LAUNCHABLE_PROGRAMS.includes(config.id); - const rank = (config: SubcommandProgram): number => { - const i = FEATURED_LAUNCHABLE_PROGRAMS.indexOf(config.id); - return i === -1 ? FEATURED_LAUNCHABLE_PROGRAMS.length : i; - }; - // Sort is stable, so everything unfeatured keeps registry order behind them. - return launchable.slice().sort((a, b) => rank(a) - rank(b)); + return [ + ...FEATURED_LAUNCHABLE_PROGRAMS.map((id) => + launchable.find((config) => config.id === id), + ).filter((config) => config != null), + ...launchable.filter((config) => !featured(config)), + ]; } diff --git a/src/ui/tui/__tests__/posthog-integration-intro.test.ts b/src/ui/tui/__tests__/posthog-integration-intro.test.ts index 57c77d611..c20ad2213 100644 --- a/src/ui/tui/__tests__/posthog-integration-intro.test.ts +++ b/src/ui/tui/__tests__/posthog-integration-intro.test.ts @@ -46,13 +46,6 @@ describe('introHeadline', () => { it('resolves the two states to different copy', () => { expect(DETECTED_HEADLINE).not.toEqual([DEFAULT_HEADLINE]); }); - - // The screen centers a single line and left-aligns a block, so the count is - // a layout decision rather than an incidental shape. - it('keeps the clean project to one line and says more on a detection', () => { - expect(introHeadline(false)).toHaveLength(1); - expect(introHeadline(true).length).toBeGreaterThan(1); - }); }); describe('introMenuOptions', () => { @@ -110,9 +103,7 @@ describe('introMenuOptions', () => { }); describe('the sub-views', () => { - // Nothing scopes arrow keys to one picker. The command list is a picker in - // the body slot, so a menu here would move both cursors at once — the bug - // this replaced. The screen binds Esc for that view instead. + // A menu here would fight the body's picker for the arrow keys. it('renders no menu under the command list', () => { expect( introMenuOptions({ diff --git a/src/ui/tui/posthog-integration-intro.ts b/src/ui/tui/posthog-integration-intro.ts index dff472455..650cbf007 100644 --- a/src/ui/tui/posthog-integration-intro.ts +++ b/src/ui/tui/posthog-integration-intro.ts @@ -4,9 +4,6 @@ export type IntroMenuView = 'default' | 'more-info' | 'commands'; export const CONTINUE_LABEL = 'Continue'; export const CONTINUE_ANYWAY_LABEL = 'Continue anyway'; -// Two words shorter than the prose calls it: IntroScreenLayout renders the -// menu in a 24-column box, and a label past that wraps mid-word. -export const COMMANDS_LABEL = 'Explore spell book'; export const DEFAULT_HEADLINE = "Let's do two hours of work in eight minutes."; export const DETECTED_HEADLINE = [ @@ -16,7 +13,6 @@ export const DETECTED_HEADLINE = [ 'You can still rerun the command, but it might overwrite some of your work.', ]; -/** Paragraphs, so the detected state can say more than the clean one. */ export function introHeadline(posthogSdkDetected: boolean): string[] { return posthogSdkDetected ? DETECTED_HEADLINE : [DEFAULT_HEADLINE]; } @@ -30,14 +26,9 @@ export function introMenuOptions({ showContinue: boolean; posthogSdkDetected: boolean; }): PickerOption[] | null { - // Nothing scopes arrow keys to one picker, so a menu under the command list - // would move both cursors at once. That view puts a picker in the body slot - // and owns the interaction; Esc is its way back. Same shape as the framework - // picker, which clears the menu while it's up. + // Its body is a picker, and a second menu here would move both cursors. if (view === 'commands') return null; - // No route to the disclosure panel from here: it has its own top-level row, - // which IntroScreenLayout appends to every intro menu. if (view === 'more-info') { return [{ label: 'Back', value: 'back' }]; } @@ -45,7 +36,7 @@ export function introMenuOptions({ if (showContinue) { return [ ...(posthogSdkDetected - ? [{ label: COMMANDS_LABEL, value: 'commands' }] + ? [{ label: 'Explore spell book', value: 'commands' }] : []), { label: posthogSdkDetected ? CONTINUE_ANYWAY_LABEL : CONTINUE_LABEL, diff --git a/src/ui/tui/primitives/PickerMenu.tsx b/src/ui/tui/primitives/PickerMenu.tsx index 7ba7fcbab..701103b4b 100644 --- a/src/ui/tui/primitives/PickerMenu.tsx +++ b/src/ui/tui/primitives/PickerMenu.tsx @@ -294,8 +294,7 @@ const FilterRow = ({ shown: number; total: number; }) => ( - // Indented to sit under PromptLabel, which carries a leading space of its - // own. Flush left, this row hangs one column left of the message above it. + // Indented to sit under PromptLabel, which carries a leading space. {filter diff --git a/src/ui/tui/screens/IntroScreenLayout.tsx b/src/ui/tui/screens/IntroScreenLayout.tsx index 349816817..67b0aecb7 100644 --- a/src/ui/tui/screens/IntroScreenLayout.tsx +++ b/src/ui/tui/screens/IntroScreenLayout.tsx @@ -98,6 +98,32 @@ interface IntroScreenLayoutProps { errorView?: ReactNode; } +/** + * Pads to the widest label a screen actually passes, so the ticks land in one + * column. Exported pure: this has silently misaligned twice. + */ +export function detectionLabelWidth(rows?: DetectionRow[]): number { + return Math.max( + 'Directory'.length, + ...(rows ?? []).map((row) => row.label.length), + ); +} + +const DetectionLine = ({ + label, + width, + children, +}: { + label: string; + width: number; + children: ReactNode; +}) => ( + + {label.padEnd(width)} {'✔'}{' '} + {children} + +); + const WizardTitle = ({ title }: { title: string }) => ( {'\u2588'} @@ -191,6 +217,8 @@ export const IntroScreenLayout = ({ privacyOptions, }); + const labelWidth = detectionLabelWidth(detectionRows); + const handleSelect = (value: string) => { if (value === 'privacy') return setShowingPrivacy(true); if (value === 'privacy-back') return setShowingPrivacy(false); @@ -249,40 +277,32 @@ export const IntroScreenLayout = ({ {showDetection && !showingPrivacy && ( - - - Directory {'\u2714'}{' '} - - - {'/'} - {path.basename(installDir)} - - + + {'/'} + {path.basename(installDir)} + {detectionRows?.map((row) => ( - - - {row.label} {'\u2714'}{' '} - - - {row.value} - {row.suffix ? ` ${row.suffix}` : ''} - - + + {row.value} + {row.suffix ? ` ${row.suffix}` : ''} + ))} {programLabel && ( - - Program{' '} - {'\u2714'} {programLabel} - + + {programLabel} + )} {programLabel === 'agent-skill' && skillId && ( - - Skill{' '} - {'\u2714'} {skillId} - + + {skillId} + )} )} diff --git a/src/ui/tui/screens/PostHogIntegrationIntroScreen.tsx b/src/ui/tui/screens/PostHogIntegrationIntroScreen.tsx index 94b63a737..3da96b496 100644 --- a/src/ui/tui/screens/PostHogIntegrationIntroScreen.tsx +++ b/src/ui/tui/screens/PostHogIntegrationIntroScreen.tsx @@ -146,9 +146,7 @@ export const PostHogIntegrationIntroScreen = ({ view === 'default' && !unsupported; - // The command list is the only view whose body is itself a picker, so it - // renders without a menu beneath it and needs its own way out. Empty - // bindings elsewhere keep the hint off every view that has a Back row. + // The only view with no menu to carry a Back row, so Esc is its way out. useKeyBindings( 'posthog-integration-intro', view === 'commands' @@ -241,8 +239,6 @@ export const PostHogIntegrationIntroScreen = ({ }))} onSelect={(value) => { const id = Array.isArray(value) ? value[0] : value; - // Same event as the menu rows: which trick a re-run user picks is - // the measure of whether offering them beat re-integrating. analytics.wizardCapture('intro menu selected', { value: id, view }); store.switchProgram(id); }} @@ -255,8 +251,7 @@ export const PostHogIntegrationIntroScreen = ({ flexDirection="column" width={64} flexShrink={0} - // One line stays centered as it always was. A wrapped block reads as - // ragged centered, so it aligns left inside the same width. + // A wrapped block reads as ragged centered; one line always centered. alignItems={paragraphs.length > 1 ? undefined : 'center'} > {paragraphs.map((paragraph, i) => ( @@ -287,9 +282,7 @@ export const PostHogIntegrationIntroScreen = ({ if (session.posthogSdkDetected) { detectionRows.push({ - // Padded to the width the layout hardcodes for its own rows, so the - // ticks line up in one column. - label: 'PostHog ', + label: 'PostHog', value: 'detected in package.json', }); } diff --git a/src/ui/tui/screens/__tests__/IntroScreenLayout.test.ts b/src/ui/tui/screens/__tests__/IntroScreenLayout.test.ts index 6f6d7a2d7..01507c4cc 100644 --- a/src/ui/tui/screens/__tests__/IntroScreenLayout.test.ts +++ b/src/ui/tui/screens/__tests__/IntroScreenLayout.test.ts @@ -5,7 +5,10 @@ * nesting under More info. */ -import { buildIntroMenu } from '@ui/tui/screens/IntroScreenLayout'; +import { + buildIntroMenu, + detectionLabelWidth, +} from '@ui/tui/screens/IntroScreenLayout'; import { PRIVACY_PANEL_LABEL } from '@ui/tui/components/PrivacyPanel'; const values = (options: ReturnType) => @@ -90,3 +93,18 @@ describe('buildIntroMenu', () => { expect(plain?.at(-1)?.icon).toBeUndefined(); }); }); + +// Ticks used to sit in whatever column each label happened to end at. +describe('detectionLabelWidth', () => { + it('holds the Directory column when rows are shorter', () => { + expect(detectionLabelWidth([{ label: 'PostHog', value: 'yes' }])).toBe( + 'Directory'.length, + ); + }); + + it('widens for a row label longer than any the layout owns', () => { + expect(detectionLabelWidth([{ label: 'PostHog SDK', value: 'x' }])).toBe( + 'PostHog SDK'.length, + ); + }); +}); diff --git a/src/ui/tui/screens/__tests__/PostHogIntegrationIntroScreen.test.ts b/src/ui/tui/screens/__tests__/PostHogIntegrationIntroScreen.test.ts index 3434944cd..3821cd9b4 100644 --- a/src/ui/tui/screens/__tests__/PostHogIntegrationIntroScreen.test.ts +++ b/src/ui/tui/screens/__tests__/PostHogIntegrationIntroScreen.test.ts @@ -15,18 +15,15 @@ const MENU_BOX_WIDTH = 24; const ICON_ROW_PREFIX_WIDTH = 4; const MAX_MENU_LABEL_LENGTH = MENU_BOX_WIDTH - ICON_ROW_PREFIX_WIDTH; -const menuFor = ( - view: 'default' | 'more-info' | 'commands', - posthogSdkDetected: boolean, -) => introMenuOptions({ view, showContinue: true, posthogSdkDetected }) ?? []; +const menuFor = (view: 'default' | 'more-info', posthogSdkDetected: boolean) => + introMenuOptions({ view, showContinue: true, posthogSdkDetected }) ?? []; -// Both detection states, because only one of them offers the spell book row -// and the label that hedges Continue. +// Both detection states: only one offers the spell book row and the hedged +// Continue label. const EVERY_LABEL = [ ...menuFor('default', false), ...menuFor('default', true), ...menuFor('more-info', true), - ...menuFor('commands', true), ...sharingOptions(true), ].map((o) => o.label); @@ -41,7 +38,7 @@ describe('PostHogIntegrationIntroScreen menu labels', () => { it('leaves the disclosure row to the layout', () => { // IntroScreenLayout appends it to every intro menu — see its own test. // A copy here would drift, which is how the panel got five names. - for (const view of ['default', 'more-info', 'commands'] as const) { + for (const view of ['default', 'more-info'] as const) { expect(menuFor(view, true).map((o) => o.value)).not.toContain('privacy'); } }); From 54b5f9981e718d55ed74e9d26eb5a61ca84667d9 Mon Sep 17 00:00:00 2001 From: "Vincent (Wen Yu) Ge" Date: Tue, 1 Sep 2026 21:50:08 -0400 Subject: [PATCH 08/12] feat(intro): curate the command list, and drop the ALL_COMMANDS indirection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The intro's list was derived — every CLI program that wasn't a family parent, in registry order. That rule picked the wrong things. It dropped `audit`, the one command a project with PostHog already installed most wants, purely because `audit web-analytics` claims it as a parent; the leaf survived and the parent didn't, so the menu advertised one narrow audit and hid the comprehensive one. `audit` is a real program with its own steps and run, so nothing stopped the intro handing off to it. It's now an explicit ordered list. A program earns a row by being worth a re-run user's time, which no field on the config can tell us: `migrate` and `audit web-analytics` are out, `audit` is in, and the order is chosen rather than inherited from PROGRAM_REGISTRY. Five descriptions reworded to match — they render in `--help` too, where they read better short. Also reverts the ALL_COMMANDS list added earlier on this branch. bin.ts is back to main's `use()` chain, where the commands are visible at the point they're registered. Its only other consumer was a test asserting the registry agreed with that list; the two dead `command` fields it was written to catch are still deleted, which was the actual fix. Generated-By: PostHog Desktop Task-Id: 5a773d7d-c2dc-444b-a8dd-6eea3ebcadc3 --- AGENTS.md | 5 +- bin.ts | 35 +++++++++++- src/__tests__/programs-cli.test.ts | 25 --------- src/commands/index.ts | 44 --------------- .../__tests__/program-registry.test.ts | 53 +++++-------------- src/lib/programs/audit/index.ts | 3 +- src/lib/programs/mcp-analytics/index.ts | 2 +- src/lib/programs/program-registry.ts | 37 +++++++------ src/lib/programs/replay-vision/index.ts | 2 +- src/lib/programs/revenue-analytics/index.ts | 2 +- src/lib/programs/warehouse-source/index.ts | 2 +- src/ui/tui/posthog-integration-intro.ts | 2 +- 12 files changed, 75 insertions(+), 137 deletions(-) delete mode 100644 src/commands/index.ts diff --git a/AGENTS.md b/AGENTS.md index fc5d172d7..226c6c924 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -83,9 +83,8 @@ confuse it with the top-level `wizard skill` command. ### Where the surface is defined (source of truth) -- **Registration:** [`src/commands/index.ts`](src/commands/index.ts) — - `ALL_COMMANDS` is the list [`bin.ts`](bin.ts) registers. Add a new command - there and nothing else needs touching. +- **Registration:** the `Wizard.use(...)` chain at the bottom of + [`bin.ts`](bin.ts). Add a new command there and nothing else needs touching. - **Command shape:** [`src/commands/command.ts`](src/commands/command.ts) — the `Command` interface every command implements. - **Flat native commands** (e.g. `revenue-analytics`, `upload-source-maps`) are diff --git a/bin.ts b/bin.ts index b7b56291f..f4bb1bd58 100644 --- a/bin.ts +++ b/bin.ts @@ -52,7 +52,22 @@ if (process.env.NODE_ENV === 'test') { } import { Wizard } from './src/wizard'; -import { ALL_COMMANDS } from './src/commands'; +import { basicIntegrationCommand } from './src/commands/basic-integration'; +import { mcpCommand } from './src/commands/mcp'; +import { mcpAnalyticsCommand } from './src/commands/mcp-analytics'; +import { replayVisionCommand } from './src/commands/replay-vision'; +import { aiObservabilityCommand } from './src/commands/ai-observability'; +import { metricsCommand } from './src/commands/metrics'; +import { auditCommand } from './src/commands/audit'; +import { doctorCommand } from './src/commands/doctor'; +import { migrateCommand } from './src/commands/migrate'; +import { revenueCommand } from './src/commands/revenue'; +import { warehouseCommand } from './src/commands/warehouse'; +import { selfDrivingCommand } from './src/commands/self-driving'; +import { slackCommand } from './src/commands/slack'; +import { uploadSourcemapsCommand } from './src/commands/upload-sourcemaps'; +import { skillCommand } from './src/commands/skill'; +import { cliCommand } from './src/commands/cli'; import { recoverOrphanedSettingsBackups } from './src/lib/agent/claude-settings'; // Heal any .claude/settings backup a previous interrupted run left orphaned, @@ -70,4 +85,20 @@ function resolveInstallDir(): string { return process.env.POSTHOG_WIZARD_INSTALL_DIR ?? process.cwd(); } -Wizard.use(...ALL_COMMANDS).init(); +Wizard.use(basicIntegrationCommand) + .use(mcpCommand) + .use(mcpAnalyticsCommand) + .use(replayVisionCommand) + .use(aiObservabilityCommand) + .use(metricsCommand) + .use(cliCommand) + .use(auditCommand) + .use(doctorCommand) + .use(migrateCommand) + .use(revenueCommand) + .use(warehouseCommand) + .use(selfDrivingCommand) + .use(slackCommand) + .use(uploadSourcemapsCommand) + .use(skillCommand) + .init(); diff --git a/src/__tests__/programs-cli.test.ts b/src/__tests__/programs-cli.test.ts index a7774d27d..7b891e1ca 100644 --- a/src/__tests__/programs-cli.test.ts +++ b/src/__tests__/programs-cli.test.ts @@ -35,9 +35,6 @@ import { fetchSkillMenu, type CliEntry } from '@lib/wizard-tools'; import { auditConfig } from '@lib/programs/audit/index'; import { webAnalyticsDoctorConfig } from '@lib/programs/web-analytics-doctor/index'; import { parseCommand } from './helpers/parse-command.no-jest'; -import { ALL_COMMANDS } from '../commands'; -import { commandKeys } from '../commands/command'; -import { getSubcommandPrograms } from '@lib/programs/program-registry'; const mockFetchSkillMenu = fetchSkillMenu as MockedFunction< typeof fetchSkillMenu @@ -60,28 +57,6 @@ function mockMenu(cliEntries: CliEntry[]): void { mockFetchSkillMenu.mockResolvedValue({ categories: {}, cliEntries }); } -// A program's `command` field says what it calls itself, not whether the CLI -// can run it. Retiring or moving a command leaves that field behind, and the -// program keeps advertising a word nobody can type. -describe('advertised commands', () => { - const registered = new Set( - ALL_COMMANDS.flatMap((command) => commandKeys(command.name)), - ); - - test('every advertised command is one the CLI registers', () => { - // A nested program is reached through its parent, so that's the word - // yargs knows — `audit`, not `web-analytics`. - const wordFor = (program: { parentCommand?: string; command: string }) => - program.parentCommand ?? program.command; - - const unrunnable = getSubcommandPrograms() - .filter((program) => !registered.has(wordFor(program))) - .map((program) => `${program.id} advertises "${wordFor(program)}"`); - - expect(unrunnable).toEqual([]); - }); -}); - describe('top-level command shapes', () => { beforeEach(() => { vi.clearAllMocks(); diff --git a/src/commands/index.ts b/src/commands/index.ts deleted file mode 100644 index 832ff3c16..000000000 --- a/src/commands/index.ts +++ /dev/null @@ -1,44 +0,0 @@ -/** - * Every command the CLI registers, in the order `--help` lists them. - * - * Kept here rather than inline in bin.ts so registration and the tests that - * check it read from the same list — a program can otherwise advertise a - * command that no longer exists, and nothing catches it. - */ - -import type { Command } from './command'; -import { basicIntegrationCommand } from './basic-integration'; -import { mcpCommand } from './mcp'; -import { mcpAnalyticsCommand } from './mcp-analytics'; -import { replayVisionCommand } from './replay-vision'; -import { aiObservabilityCommand } from './ai-observability'; -import { metricsCommand } from './metrics'; -import { cliCommand } from './cli'; -import { auditCommand } from './audit'; -import { doctorCommand } from './doctor'; -import { migrateCommand } from './migrate'; -import { revenueCommand } from './revenue'; -import { warehouseCommand } from './warehouse'; -import { selfDrivingCommand } from './self-driving'; -import { slackCommand } from './slack'; -import { uploadSourcemapsCommand } from './upload-sourcemaps'; -import { skillCommand } from './skill'; - -export const ALL_COMMANDS: readonly Command[] = [ - basicIntegrationCommand, - mcpCommand, - mcpAnalyticsCommand, - replayVisionCommand, - aiObservabilityCommand, - metricsCommand, - cliCommand, - auditCommand, - doctorCommand, - migrateCommand, - revenueCommand, - warehouseCommand, - selfDrivingCommand, - slackCommand, - uploadSourcemapsCommand, - skillCommand, -]; diff --git a/src/lib/programs/__tests__/program-registry.test.ts b/src/lib/programs/__tests__/program-registry.test.ts index 612d86d3f..c1c4f9ab2 100644 --- a/src/lib/programs/__tests__/program-registry.test.ts +++ b/src/lib/programs/__tests__/program-registry.test.ts @@ -63,23 +63,21 @@ describe('getCommandPath', () => { }); }); -// The programs the intro can hand off to in-session. A family parent isn't one -// of them — typing `wizard audit` opens a picker rather than running anything, -// so there's nothing to hand off to. Its leaves are still fair game. describe('getLaunchablePrograms', () => { - const ids = () => getLaunchablePrograms().map((config) => config.id); - - it('skips a family parent', () => { - expect(ids()).not.toContain('audit'); - }); - - it('keeps the leaves under that family', () => { - expect(ids()).toContain('web-analytics-doctor'); - }); - - it('keeps the flat programs', () => { - expect(ids()).toContain('revenue-analytics-setup'); - expect(ids()).toContain('metrics'); + // The list is curated, so an id that stops matching drops its row in silence. + it("offers the intro's programs, in order, all resolving", () => { + expect(getLaunchablePrograms().map((config) => config.id)).toEqual([ + 'self-driving', + 'error-tracking-upload-source-maps', + 'warehouse-source', + 'audit', + 'posthog-doctor', + 'mcp-analytics', + 'replay-vision', + 'ai-observability', + 'metrics', + 'revenue-analytics-setup', + ]); }); // A row wider than the terminal stops the whole block from centering. @@ -94,29 +92,6 @@ describe('getLaunchablePrograms', () => { expect(tooLong).toEqual([]); }); - - // The intro copy promises these three by name, directly above the list. - it('leads with the three the intro copy names', () => { - expect(ids().slice(0, 3)).toEqual([ - 'web-analytics-doctor', - 'error-tracking-upload-source-maps', - 'self-driving', - ]); - }); - - // Derived from who claims whom as a parent, so the next family drops out on - // its own instead of waiting for someone to remember this list. - it('drops nothing but parents', () => { - const parents = new Set( - getSubcommandPrograms().map((config) => config.parentCommand), - ); - const dropped = getSubcommandPrograms() - .filter((config) => !ids().includes(config.id)) - .map((config) => config.command); - - expect(dropped).not.toEqual([]); - expect(dropped.every((command) => parents.has(command))).toBe(true); - }); }); describe('parentCommand nesting', () => { diff --git a/src/lib/programs/audit/index.ts b/src/lib/programs/audit/index.ts index 63c41032a..ce784d82b 100644 --- a/src/lib/programs/audit/index.ts +++ b/src/lib/programs/audit/index.ts @@ -36,8 +36,7 @@ const baseConfig = createSkillProgram({ skillId: 'audit', command: 'audit', id: 'audit', - description: - 'Audit an existing PostHog integration for correctness and best practices', + description: 'Audit and improve your PostHog setup', integrationLabel: 'audit', customPrompt: 'Run a comprehensive audit of the existing PostHog integration. Follow the skill program steps in order. Do not modify any project files — only create the final audit report.', diff --git a/src/lib/programs/mcp-analytics/index.ts b/src/lib/programs/mcp-analytics/index.ts index d5ed1812c..f9199d76e 100644 --- a/src/lib/programs/mcp-analytics/index.ts +++ b/src/lib/programs/mcp-analytics/index.ts @@ -58,7 +58,7 @@ export const mcpAnalyticsConfig = createSkillProgram({ skillId: 'mcp-analytics', command: 'mcp-analytics', id: 'mcp-analytics', - description: 'Add PostHog MCP analytics to your MCP server', + description: 'Add PostHog MCP Analytics to your MCP server', integrationLabel: 'mcp-analytics', customPrompt: "Instrument this project's MCP server with PostHog MCP analytics. Run the " + diff --git a/src/lib/programs/program-registry.ts b/src/lib/programs/program-registry.ts index 0330bb78d..957653745 100644 --- a/src/lib/programs/program-registry.ts +++ b/src/lib/programs/program-registry.ts @@ -143,25 +143,28 @@ export function getCommandPath(config: SubcommandProgram): string { : config.command; } -/** Leads the intro's list, in this order, because the intro copy names them. */ -const FEATURED_LAUNCHABLE_PROGRAMS = [ - 'web-analytics-doctor', - 'error-tracking-upload-source-maps', +/** + * What the intro offers a project that already has PostHog, in the order it + * lists them. Curated, not derived: a program earns a row by being worth a + * re-run user's time, which no field on the config can tell us. + */ +const INTRO_PROGRAMS = [ 'self-driving', + 'error-tracking-upload-source-maps', + 'warehouse-source', + 'audit', + 'posthog-doctor', + 'mcp-analytics', + 'replay-vision', + 'ai-observability', + 'metrics', + 'revenue-analytics-setup', ]; -/** Programs the intro can launch, Family parents open a picker instead. */ +/** The programs the intro can hand off to, in the order it lists them. */ export function getLaunchablePrograms(): SubcommandProgram[] { - const all = getSubcommandPrograms(); - const parents = new Set(all.map((config) => config.parentCommand)); - const launchable = all.filter((config) => !parents.has(config.command)); - const featured = (config: SubcommandProgram) => - FEATURED_LAUNCHABLE_PROGRAMS.includes(config.id); - - return [ - ...FEATURED_LAUNCHABLE_PROGRAMS.map((id) => - launchable.find((config) => config.id === id), - ).filter((config) => config != null), - ...launchable.filter((config) => !featured(config)), - ]; + const byId = new Map(getSubcommandPrograms().map((c) => [c.id, c])); + return INTRO_PROGRAMS.map((id) => byId.get(id)).filter( + (config): config is SubcommandProgram => config != null, + ); } diff --git a/src/lib/programs/replay-vision/index.ts b/src/lib/programs/replay-vision/index.ts index 6c1cca047..6688cc070 100644 --- a/src/lib/programs/replay-vision/index.ts +++ b/src/lib/programs/replay-vision/index.ts @@ -119,7 +119,7 @@ const base = createSkillProgram({ skillId: 'replay-vision-setup', command: 'replay-vision', id: 'replay-vision', - description: 'Set up PostHog Replay vision scanners for your product', + description: 'Set up PostHog Replay Vision scanners for your product', integrationLabel: 'replay-vision', customPrompt: 'Set up PostHog Replay vision. Run the `replay-vision` skill end-to-end: ' + diff --git a/src/lib/programs/revenue-analytics/index.ts b/src/lib/programs/revenue-analytics/index.ts index 08b0f48a9..f75778bd1 100644 --- a/src/lib/programs/revenue-analytics/index.ts +++ b/src/lib/programs/revenue-analytics/index.ts @@ -6,7 +6,7 @@ import { getContentBlocks } from './content/index.js'; export const revenueAnalyticsConfig: ProgramConfig = { command: 'revenue-analytics', - description: 'Set up PostHog revenue analytics', + description: 'Set up PostHog for Revenue Analytics', id: 'revenue-analytics-setup', skillId: 'revenue-analytics-setup', steps: REVENUE_ANALYTICS_PROGRAM, diff --git a/src/lib/programs/warehouse-source/index.ts b/src/lib/programs/warehouse-source/index.ts index c09883e72..8163e4589 100644 --- a/src/lib/programs/warehouse-source/index.ts +++ b/src/lib/programs/warehouse-source/index.ts @@ -42,7 +42,7 @@ function buildPrompt(session: WizardSession): string { export const warehouseSourceConfig: ProgramConfig = { command: 'warehouse', - description: 'Detect and connect a data warehouse source', + description: 'Detect and connect Data Warehouse sources', id: 'warehouse-source', skillId: 'data-warehouse-source-setup', steps: WAREHOUSE_SOURCE_PROGRAM, diff --git a/src/ui/tui/posthog-integration-intro.ts b/src/ui/tui/posthog-integration-intro.ts index 650cbf007..15bc8b75a 100644 --- a/src/ui/tui/posthog-integration-intro.ts +++ b/src/ui/tui/posthog-integration-intro.ts @@ -10,7 +10,7 @@ export const DETECTED_HEADLINE = [ 'It looks like PostHog is already installed. The Wizard has many tricks ' + 'up its sleeve, like auditing, uploading source maps, or making your ' + 'product self-drive.', - 'You can still rerun the command, but it might overwrite some of your work.', + 'You can still rerun the installation, but it might overwrite some of your work.', ]; export function introHeadline(posthogSdkDetected: boolean): string[] { From 7a5899ac9c597bb6391e4a0abe91fb7c572d06b3 Mon Sep 17 00:00:00 2001 From: "Vincent (Wen Yu) Ge" Date: Tue, 1 Sep 2026 22:10:40 -0400 Subject: [PATCH 09/12] refactor: cut the comment slop and the changes that earned nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AGENTS.md goes back to main's one line — bin.ts is main's again, so the note describing it was both longer and wrong. Every comment this branch added is one line now. The ones that went were narrating their own code or restating a test's name; what's left is the constraint the code can't show: a column width, a leading space in a sibling component, why a view renders no menu. Also drops a `marginY` on the picker's filter row that reached the branch through a merge rather than a commit, and was never anything this PR needed. Generated-By: PostHog Desktop Task-Id: 5a773d7d-c2dc-444b-a8dd-6eea3ebcadc3 --- AGENTS.md | 3 +-- .../__tests__/program-registry.test.ts | 4 +-- .../programs/posthog-integration/detect.ts | 6 +---- src/lib/programs/program-registry.ts | 6 +---- src/lib/runners/run-wizard.ts | 3 +-- src/lib/wizard-session.ts | 2 +- .../posthog-integration-intro.test.ts | 27 ++++--------------- src/ui/tui/__tests__/store.test.ts | 20 ++++---------- src/ui/tui/primitives/PickerMenu.tsx | 2 +- src/ui/tui/screens/IntroScreenLayout.tsx | 5 +--- .../PostHogIntegrationIntroScreen.test.ts | 3 +-- 11 files changed, 19 insertions(+), 62 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 226c6c924..9538219a4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -83,8 +83,7 @@ confuse it with the top-level `wizard skill` command. ### Where the surface is defined (source of truth) -- **Registration:** the `Wizard.use(...)` chain at the bottom of - [`bin.ts`](bin.ts). Add a new command there and nothing else needs touching. +- **Registration:** [`bin.ts`](bin.ts) — the `.use()` chain wires each command. - **Command shape:** [`src/commands/command.ts`](src/commands/command.ts) — the `Command` interface every command implements. - **Flat native commands** (e.g. `revenue-analytics`, `upload-source-maps`) are diff --git a/src/lib/programs/__tests__/program-registry.test.ts b/src/lib/programs/__tests__/program-registry.test.ts index c1c4f9ab2..90f3e472d 100644 --- a/src/lib/programs/__tests__/program-registry.test.ts +++ b/src/lib/programs/__tests__/program-registry.test.ts @@ -43,9 +43,7 @@ describe('getSubcommandPrograms', () => { }); }); -// What a user types to reach the program. A nested program's own `command` is -// only half of that, so anything telling a user how to run one has to join it -// to the parent. +// A nested program is only reachable through its parent's word. describe('getCommandPath', () => { const subcommand = (id: string) => getSubcommandPrograms().find((config) => config.id === id)!; diff --git a/src/lib/programs/posthog-integration/detect.ts b/src/lib/programs/posthog-integration/detect.ts index 4d1cf2568..f27faa6a9 100644 --- a/src/lib/programs/posthog-integration/detect.ts +++ b/src/lib/programs/posthog-integration/detect.ts @@ -254,11 +254,7 @@ export function reportWarehouseSourcesDetected( return true; } -/** - * Scan for existing PostHog SDKs in the project. Dependency-level signal only, - * not a verified (or complete) install. Best-effort: scan failure reports - * false rather than breaking the detection step. - */ +/** Dependency-level signal, not a verified install. A failed scan reports false. */ export function detectExistingPostHog( ctx: Pick, installDir: string, diff --git a/src/lib/programs/program-registry.ts b/src/lib/programs/program-registry.ts index 957653745..56389efc2 100644 --- a/src/lib/programs/program-registry.ts +++ b/src/lib/programs/program-registry.ts @@ -143,11 +143,7 @@ export function getCommandPath(config: SubcommandProgram): string { : config.command; } -/** - * What the intro offers a project that already has PostHog, in the order it - * lists them. Curated, not derived: a program earns a row by being worth a - * re-run user's time, which no field on the config can tell us. - */ +/** What the intro offers, in order. Curated: no config field ranks these. */ const INTRO_PROGRAMS = [ 'self-driving', 'error-tracking-upload-source-maps', diff --git a/src/lib/runners/run-wizard.ts b/src/lib/runners/run-wizard.ts index bae2b69b7..714617802 100644 --- a/src/lib/runners/run-wizard.ts +++ b/src/lib/runners/run-wizard.ts @@ -189,8 +189,7 @@ export function runWizard( for (;;) { await activeTui.store.runReadyHooks(); - // Settle the pre-run screens. `integration-check` is a no-op gate for - // programs without it. + // Settle the pre-run screens; `integration-check` is a no-op gate here. await activeTui.store.getGate('intro'); const active = activeTui.store.router.activeProgram; diff --git a/src/lib/wizard-session.ts b/src/lib/wizard-session.ts index 4c37e3430..7908d8a16 100644 --- a/src/lib/wizard-session.ts +++ b/src/lib/wizard-session.ts @@ -300,7 +300,7 @@ export interface WizardSession { /** Human-readable label for the detected framework variant (e.g., "Django with Wagtail CMS") */ detectedFrameworkLabel: string | null; - /** Existing PostHog detected in the project (set during detect). Signal, not proof. Currently a dependency-level check */ + /** PostHog found in the project's dependencies. A signal, not a verified install. */ posthogSdkDetected: boolean; /** True once framework detection has run (whether it found something or not) */ diff --git a/src/ui/tui/__tests__/posthog-integration-intro.test.ts b/src/ui/tui/__tests__/posthog-integration-intro.test.ts index c20ad2213..c098d5ab2 100644 --- a/src/ui/tui/__tests__/posthog-integration-intro.test.ts +++ b/src/ui/tui/__tests__/posthog-integration-intro.test.ts @@ -1,15 +1,3 @@ -/** - * Intro-screen copy and menu decisions, extracted from - * PostHogIntegrationIntroScreen so they can be asserted without a render — - * vitest aliases `ink` to no-op stubs suite-wide (vitest.config.ts), so the - * screen itself draws nothing here. - * - * These assert the wiring, not the wording: which headline each detection state - * resolves to, and what order the menu offers. Copy edits land in one place and - * don't drag the test with them — menu order is pinned on `value` (stable - * identifiers), and the only label asserted is the one that actually branches. - */ - import { CONTINUE_ANYWAY_LABEL, CONTINUE_LABEL, @@ -41,8 +29,7 @@ describe('introHeadline', () => { expect(introHeadline(false)).toEqual([DEFAULT_HEADLINE]); }); - // Without literals on either side, both assertions above pass vacuously if - // the two headlines ever collapse to the same copy. + // Both assertions above pass vacuously if the two ever collapse. it('resolves the two states to different copy', () => { expect(DETECTED_HEADLINE).not.toEqual([DEFAULT_HEADLINE]); }); @@ -50,8 +37,7 @@ describe('introHeadline', () => { describe('introMenuOptions', () => { describe('an install we already detected', () => { - // The ask: exploring the other commands outranks re-running an - // integration the project may not need. + // Exploring outranks re-running an integration the project may not need. it('offers the tricks before continuing', () => { expect(valuesFor({ posthogSdkDetected: true })).toEqual([ 'commands', @@ -96,8 +82,7 @@ describe('introMenuOptions', () => { }); }); - // Same reasoning as the headline pair: with neither side a literal, both - // label assertions pass vacuously if the two ever collapse. + // Same vacuous-pass risk as the headline pair. it('distinguishes the two continue labels', () => { expect(CONTINUE_ANYWAY_LABEL).not.toBe(CONTINUE_LABEL); }); @@ -114,15 +99,13 @@ describe('introMenuOptions', () => { ).toBeNull(); }); - // IntroScreenLayout appends the disclosure row to every intro menu, so - // neither sub-view carries its own route to it. + // IntroScreenLayout appends the disclosure row, so no view carries one. it('gives the more-info view a way back and nothing else', () => { expect(valuesFor({ view: 'more-info' })).toEqual(['back']); }); }); - // Detecting, picking a framework, and the unsupported-version prompt all - // clear showContinue — the screen owns the interaction, so no menu renders. + // Detecting, framework-picking and unsupported all clear showContinue. it('renders no menu when there is nothing to continue to', () => { expect( introMenuOptions({ diff --git a/src/ui/tui/__tests__/store.test.ts b/src/ui/tui/__tests__/store.test.ts index ea9e30c84..1fb2f6254 100644 --- a/src/ui/tui/__tests__/store.test.ts +++ b/src/ui/tui/__tests__/store.test.ts @@ -95,11 +95,7 @@ describe('WizardStore', () => { expect(store.getSnapshot()).toBe(0); }); - // Picking one of the wizard's other commands from the intro runs it in - // this session instead of making the user quit and type it. Nothing has - // happened yet at that point — no auth, no agent, no files touched — so - // the switch only has to repoint the router and start the new program's - // journey from its own first screen. + // Runs another command in this session; nothing has happened yet to unwind. describe('switchProgram', () => { it('makes the chosen program the active one', () => { const store = createStore(); @@ -113,9 +109,7 @@ describe('WizardStore', () => { expect(store.router.resolve(store.session)).toBe(ScreenId.MetricsIntro); }); - // The old intro is behind us, but every program gates its intro on the - // same `setupConfirmed` flag — leaving it set marks the new program's - // intro complete before the user has seen it. + // Every program gates its intro on the same flag, so a stale one skips it. it('does not carry the old confirmation into the new intro', () => { const store = createStore(); store.completeSetup(); @@ -127,8 +121,7 @@ describe('WizardStore', () => { expect(store.router.resolve(store.session)).toBe(ScreenId.MetricsIntro); }); - // bin.ts parks on these gates. They resolved for the program we left, so - // reusing them would run the new program past its own screens. + // Already resolved for the program we left, so reusing them skips screens. it('reopens the gates for the new program', async () => { const store = createStore(); const before = store.getGate('intro'); @@ -144,9 +137,7 @@ describe('WizardStore', () => { ).resolves.toBe('pending'); }); - // The runner is parked on the old program's intro gate at the moment of - // the switch. Dropping that promise without resolving it strands the - // runner — no error, no new program, just a wizard that stops. + // Dropping the promise the runner is parked on strands it, silently. it('releases callers parked on the old gates', async () => { const store = createStore(); const parked = store.getGate('intro'); @@ -171,8 +162,7 @@ describe('WizardStore', () => { ); }); - // Selecting the program already running should cost the user nothing — - // in particular it must not throw away a confirmation they just gave. + // Re-selecting the running program must not discard a fresh confirmation. it('leaves the session alone when the program is unchanged', () => { const store = createStore(); store.completeSetup(); diff --git a/src/ui/tui/primitives/PickerMenu.tsx b/src/ui/tui/primitives/PickerMenu.tsx index 701103b4b..33436be9f 100644 --- a/src/ui/tui/primitives/PickerMenu.tsx +++ b/src/ui/tui/primitives/PickerMenu.tsx @@ -295,7 +295,7 @@ const FilterRow = ({ total: number; }) => ( // Indented to sit under PromptLabel, which carries a leading space. - + {filter ? `Filter: ${filter} (${shown} of ${total})` diff --git a/src/ui/tui/screens/IntroScreenLayout.tsx b/src/ui/tui/screens/IntroScreenLayout.tsx index 67b0aecb7..7e66cb37c 100644 --- a/src/ui/tui/screens/IntroScreenLayout.tsx +++ b/src/ui/tui/screens/IntroScreenLayout.tsx @@ -98,10 +98,7 @@ interface IntroScreenLayoutProps { errorView?: ReactNode; } -/** - * Pads to the widest label a screen actually passes, so the ticks land in one - * column. Exported pure: this has silently misaligned twice. - */ +/** Pads to the widest label passed, so every tick lands in one column. */ export function detectionLabelWidth(rows?: DetectionRow[]): number { return Math.max( 'Directory'.length, diff --git a/src/ui/tui/screens/__tests__/PostHogIntegrationIntroScreen.test.ts b/src/ui/tui/screens/__tests__/PostHogIntegrationIntroScreen.test.ts index 3821cd9b4..a2def44d8 100644 --- a/src/ui/tui/screens/__tests__/PostHogIntegrationIntroScreen.test.ts +++ b/src/ui/tui/screens/__tests__/PostHogIntegrationIntroScreen.test.ts @@ -18,8 +18,7 @@ const MAX_MENU_LABEL_LENGTH = MENU_BOX_WIDTH - ICON_ROW_PREFIX_WIDTH; const menuFor = (view: 'default' | 'more-info', posthogSdkDetected: boolean) => introMenuOptions({ view, showContinue: true, posthogSdkDetected }) ?? []; -// Both detection states: only one offers the spell book row and the hedged -// Continue label. +// Both detection states: only one offers the spell book and hedged Continue. const EVERY_LABEL = [ ...menuFor('default', false), ...menuFor('default', true), From 29fd1dd83ea4d887406d0920993f3b6ca5106110 Mon Sep 17 00:00:00 2001 From: "Vincent (Wen Yu) Ge" Date: Wed, 2 Sep 2026 19:12:33 -0400 Subject: [PATCH 10/12] fix(analytics): hold the SDK verdict behind the same scan consent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `posthog_sdk_detected` is a package.json scan result, but it rode along on every screen-transition and setup event regardless of `scanConsent` — so a repo with a PostHog dependency reported that fact before the user had answered, and after they answered "don't share". Route it through wizard-session like `discovered_features`: the property is absent unless consent is granted, and absent stays unambiguous against a `false` verdict. Co-Authored-By: Claude Opus 5 --- src/__tests__/mcp-cli.test.ts | 1 + src/lib/wizard-session.ts | 7 +++++++ src/utils/__tests__/analytics.test.ts | 19 ++++++++++++++++++- src/utils/analytics.ts | 6 +++++- 4 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/__tests__/mcp-cli.test.ts b/src/__tests__/mcp-cli.test.ts index f604c2fe2..e9b41f8b3 100644 --- a/src/__tests__/mcp-cli.test.ts +++ b/src/__tests__/mcp-cli.test.ts @@ -17,6 +17,7 @@ vi.mock('@lib/wizard-session', () => ({ // suite's assertions, stubbed only so the mocked module still satisfies // the real module's exports. reportableDiscoveredFeatures: () => undefined, + reportablePosthogSdkDetected: () => undefined, })); vi.mock('@ui/tui/start-tui', () => ({ startTUI: mockStartTUIMcp, diff --git a/src/lib/wizard-session.ts b/src/lib/wizard-session.ts index 7908d8a16..5c428c3ae 100644 --- a/src/lib/wizard-session.ts +++ b/src/lib/wizard-session.ts @@ -583,3 +583,10 @@ export function reportableDiscoveredFeatures( ): DiscoveredFeature[] | undefined { return mayReportScanResults(session) ? session.discoveredFeatures : undefined; } + +/** Also a scan result, so it travels under the same consent as the rest. */ +export function reportablePosthogSdkDetected( + session: WizardSession, +): boolean | undefined { + return mayReportScanResults(session) ? session.posthogSdkDetected : undefined; +} diff --git a/src/utils/__tests__/analytics.test.ts b/src/utils/__tests__/analytics.test.ts index 189a8d159..347424ec1 100644 --- a/src/utils/__tests__/analytics.test.ts +++ b/src/utils/__tests__/analytics.test.ts @@ -639,13 +639,30 @@ describe('Analytics', () => { }); describe('sessionProperties', () => { - it('includes the posthog_sdk_detected verdict', () => { + it('includes the posthog_sdk_detected verdict once sharing is granted', () => { const session = buildSession({}); + session.scanConsent = ScanConsent.Granted; expect(sessionProperties(session).posthog_sdk_detected).toBe(false); session.posthogSdkDetected = true; expect(sessionProperties(session).posthog_sdk_detected).toBe(true); }); + + // It is a package.json scan result, so it waits on the same consent. + it('omits the verdict while consent is undecided or declined', () => { + const session = buildSession({}); + session.posthogSdkDetected = true; + + expect(session.scanConsent).toBe(ScanConsent.Undecided); + expect(sessionProperties(session)).not.toHaveProperty( + 'posthog_sdk_detected', + ); + + session.scanConsent = ScanConsent.Declined; + expect(sessionProperties(session)).not.toHaveProperty( + 'posthog_sdk_detected', + ); + }); }); describe('groupsFromUser', () => { diff --git a/src/utils/analytics.ts b/src/utils/analytics.ts index f09935103..d6500a015 100644 --- a/src/utils/analytics.ts +++ b/src/utils/analytics.ts @@ -7,6 +7,7 @@ import { } from '@lib/constants'; import { reportableDiscoveredFeatures, + reportablePosthogSdkDetected, type WizardSession, } from '@lib/wizard-session'; import type { ApiUser } from '@lib/api'; @@ -46,6 +47,7 @@ export function sessionProperties( // might be absent. An absent key is unambiguous, while an empty array // would read as "we looked and found nothing" instead of "not reported". const discoveredFeatures = reportableDiscoveredFeatures(session); + const posthogSdkDetected = reportablePosthogSdkDetected(session); return { integration: session.integration, @@ -58,7 +60,9 @@ export function sessionProperties( scan_consent: session.scanConsent, additional_features: session.additionalFeatureQueue, run_phase: session.runPhase, - posthog_sdk_detected: session.posthogSdkDetected, + ...(posthogSdkDetected !== undefined + ? { posthog_sdk_detected: posthogSdkDetected } + : {}), }; } From 8157ff1335c2492175df358cf4c83d796aa7b269 Mon Sep 17 00:00:00 2001 From: "Vincent (Wen Yu) Ge" Date: Wed, 2 Sep 2026 19:12:38 -0400 Subject: [PATCH 11/12] fix(tui): retag the run when the intro switches programs start-tui stamps `program_id` once at launch. A user who leaves the integration intro for another command kept reporting under `posthog-integration` for the rest of the run, so the switch was invisible in analytics. Re-stamp the tag inside switchProgram, next to the router and session updates that already follow the new program. Co-Authored-By: Claude Opus 5 --- src/ui/tui/__tests__/store.test.ts | 11 +++++++++++ src/ui/tui/store.ts | 3 +++ 2 files changed, 14 insertions(+) diff --git a/src/ui/tui/__tests__/store.test.ts b/src/ui/tui/__tests__/store.test.ts index 1fb2f6254..ee8201a7d 100644 --- a/src/ui/tui/__tests__/store.test.ts +++ b/src/ui/tui/__tests__/store.test.ts @@ -153,6 +153,17 @@ describe('WizardStore', () => { expect(store.analyticsProgramId).toBe(Program.Metrics); }); + // The run-level tag is stamped once at launch, so events after the + // switch would otherwise still carry the program the run started as. + it('retags the run with the new program', () => { + const store = createStore(); + store.switchProgram(Program.Metrics); + expect(analytics.setTag).toHaveBeenCalledWith( + 'program_id', + Program.Metrics, + ); + }); + it('follows the new program for label and skill', () => { const store = createStore(); store.switchProgram(Program.Metrics); diff --git a/src/ui/tui/store.ts b/src/ui/tui/store.ts index 4b89e1d63..b548d37ee 100644 --- a/src/ui/tui/store.ts +++ b/src/ui/tui/store.ts @@ -951,6 +951,9 @@ export class WizardStore { this.router.setProgram(program); this._initFromProgram(program); + // start-tui stamps this once at launch; without it here every event + // after the switch still reports under the program the run started as. + analytics.setTag('program_id', program); const config = getProgramConfig(program); this.$session.setKey('setupConfirmed', false); From ec7f7baabc7846c669ef65145cdf61c58c57256e Mon Sep 17 00:00:00 2001 From: "Vincent (Wen Yu) Ge" Date: Wed, 2 Sep 2026 19:21:20 -0400 Subject: [PATCH 12/12] fix(task-stream): open the stream on the program the run settles into MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TaskStreamPush bakes its program id, session id, and event-plan path in at construction, and it was constructed before the intro's program switch could happen — so a user who left the integration intro for another command had the whole run pushed to the web app under `posthog-integration`, with the event plan watcher tailing the wrong file. Build and attach it after the switch loop instead; nothing before that point produces a task to push. The signal handler stays registered ahead of the loop, so Ctrl-C on the intro still restores the terminal and runs the cleanups — it now tolerates a stream that does not exist yet rather than reporting a run that never started. Also note on eventsAuditConfig that `wizard audit events` resolves to the context-mill `audit-events` skill, not to it — dropping its command word in this branch retires a path the family already replaced. Co-Authored-By: Claude Opus 5 --- src/lib/programs/events-audit/index.ts | 6 +++ src/lib/runners/run-wizard.ts | 70 +++++++++++++++----------- 2 files changed, 48 insertions(+), 28 deletions(-) diff --git a/src/lib/programs/events-audit/index.ts b/src/lib/programs/events-audit/index.ts index ae1d8f0fd..b80fad1ee 100644 --- a/src/lib/programs/events-audit/index.ts +++ b/src/lib/programs/events-audit/index.ts @@ -19,6 +19,12 @@ export { SETUP_REPORT_FILE }; const DOCS_URL = 'https://posthog.com/docs/product-analytics/best-practices'; +/** + * No CLI word of its own since the audit family took over: `wizard audit + * events` is the live path, and it resolves to the context-mill `audit-events` + * skill (whose id AuditRunScreen keys its slides on), not to this config. + * Registered so its id stays resolvable; nothing dispatches to it today. + */ export const eventsAuditConfig: ProgramConfig = { description: 'Audit PostHog event tracking in this project', id: 'events-audit', diff --git a/src/lib/runners/run-wizard.ts b/src/lib/runners/run-wizard.ts index 714617802..3ac193049 100644 --- a/src/lib/runners/run-wizard.ts +++ b/src/lib/runners/run-wizard.ts @@ -138,27 +138,11 @@ export function runWizard( activeTui.store.session = session; - const taskStreamEnabled = !session.noTelemetry; - taskStream = new TaskStreamPush({ - store: activeTui.store, - programId: config.id, - destinations: [ - new PostHogDestination({ - getCredentials: () => activeTui.store.session.credentials, - onError: (err) => logToFile('[task-stream-push]', err.message), - }), - ], - eventPlanPath: config.eventPlanFile - ? join(session.installDir, config.eventPlanFile) - : undefined, - enabled: taskStreamEnabled, - }); - const activeStream = taskStream; - activeStream.attach(); - // Flush a terminal-phase push on Ctrl-C so the web app sees the // run ended in error rather than hanging on the last "running" - // snapshot. + // snapshot. Registered before the stream exists: Ctrl-C on the intro + // must still restore the terminal and run the cleanups, and there is + // no run to report yet. let signalled = false; onSignal = (): void => { if (signalled || exitInProgress) return; @@ -170,19 +154,25 @@ export function runWizard( if (activeTui.store.session.runPhase === RunPhase.Running) { activeTui.store.setRunPhase(RunPhase.Error); } - void activeStream + const teardown = (): void => { + try { + activeTui.unmount(); + } catch { + // terminal may already be torn down + } + process.exit(130); + }; + const stream = taskStream; + if (!stream) { + teardown(); + return; + } + void stream .shutdown(2000) .catch((e) => logToFile('[run-wizard] task stream shutdown error on signal:', e), ) - .finally(() => { - try { - activeTui.unmount(); - } catch { - // terminal may already be torn down - } - process.exit(130); - }); + .finally(teardown); }; process.on('SIGINT', onSignal); process.on('SIGTERM', onSignal); @@ -196,6 +186,30 @@ export function runWizard( if (active === config.id) break; config = getProgramConfig(active); } + + // After the switch loop, not before: the stream bakes its program id, + // session id, and event-plan path in at construction, so a stream built + // for the launch program would report the whole run under a program the + // user left on the intro screen. Nothing before this point produces a + // task to push. + const taskStreamEnabled = !session.noTelemetry; + const activeStream = new TaskStreamPush({ + store: activeTui.store, + programId: config.id, + destinations: [ + new PostHogDestination({ + getCredentials: () => activeTui.store.session.credentials, + onError: (err) => logToFile('[task-stream-push]', err.message), + }), + ], + eventPlanPath: config.eventPlanFile + ? join(session.installDir, config.eventPlanFile) + : undefined, + enabled: taskStreamEnabled, + }); + taskStream = activeStream; + activeStream.attach(); + await activeTui.store.getGate('integration-check'); await activeTui.store.getGate('health-check');