feat(cli): add opt-out anonymous telemetry via PostHog#52
Conversation
Add anonymous usage telemetry to help improve the CLI. Uses PostHog's HTTP batch API directly (zero new dependencies) with a 5-second timeout and fail-silent behavior — telemetry never breaks the CLI. What's collected: command names, render performance (duration, fps, quality), template choices, OS/arch/Node version/CLI version. What's NOT collected: file paths, project names, video content, or any personally identifiable information. Telemetry is: - Disabled in dev mode (running via tsx) - Disabled in CI (CI=true) or via HYPERFRAMES_NO_TELEMETRY=1 - Disabled when API key is placeholder (safe to merge before key is set) - Controllable via `hyperframes telemetry [enable|disable|status]` - Disclosed on first run with clear opt-out instructions Config stored at ~/.hyperframes/config.json (0600 permissions). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Extract shared isDevMode() to utils/env.ts (was duplicated in dev.ts and client.ts) - Use ui/colors.ts instead of raw ANSI escapes in telemetry notice (respects NO_COLOR) - Derive known commands from subCommands object instead of maintaining duplicate set - Skip telemetry on --help/--version and unknown commands - Gate incrementCommandCount() behind shouldTrack() (no disk writes in CI) - Add flushSync() for process.exit() paths (beforeExit doesn't fire on explicit exit) - Remove dead trackBrowserInstall(success) param (failure path never called it) - Remove redundant isEnabled/anonymousId caching in client.ts (config.ts cache suffices) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PostHog's $ip: null property tells the server to not associate the request IP with the event. Combined with the "Discard client IP data" project setting for server-side enforcement. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| // This is a public project API key — safe to embed in client-side code. | ||
| // It only allows writing events, not reading data. | ||
| const POSTHOG_API_KEY = "phc_zjjbX0PnWxERXrMHhkEJWj9A9BhGVLRReICgsfTMmpx"; | ||
| const POSTHOG_HOST = "https://us.i.posthog.com"; |
There was a problem hiding this comment.
Claude: The API key is real (phc_zjjbX0...), but the PR description says it's a placeholder (__POSTHOG_API_KEY__). The shouldTrack() check on line 57 for the placeholder string is dead code. Either keep it as a placeholder and replace later, or remove the dead check.
| const payload = JSON.stringify({ api_key: POSTHOG_API_KEY, batch }); | ||
|
|
||
| try { | ||
| // Spawn a detached process to send the request so we don't block exit. |
There was a problem hiding this comment.
Claude: flushSync uses require("node:child_process") inside an ESM module ("type": "module" in package.json). This works at runtime because Node allows it, but it's inconsistent with the rest of the codebase which uses ESM imports. More importantly, execFileSync blocks the process — on slow networks this could add up to 5 seconds to every process.exit(). Consider spawning a detached child with { detached: true, stdio: 'ignore' } and calling unref() so the parent exits immediately.
| })); | ||
| eventQueue = []; | ||
|
|
||
| const controller = new AbortController(); |
There was a problem hiding this comment.
Claude: : null tells PostHog to not record the IP. Good. But the PR description says "Will configure PostHog to discard" — this property already does it. Update the description to match, or note that server-side IP logging should also be disabled in PostHog project settings for belt-and-suspenders.
| mkdirSync(CONFIG_DIR, { recursive: true }); | ||
| writeFileSync(CONFIG_FILE, JSON.stringify(config, null, 2) + "\n", { mode: 0o600 }); | ||
| cachedConfig = { ...config }; | ||
| } catch { |
There was a problem hiding this comment.
Claude: mode: 0o600 is good for the config file (user-only read/write). But mkdirSync on line 74 doesn't set directory permissions — it inherits the umask default (typically 0o755). For consistency, consider mkdirSync(CONFIG_DIR, { recursive: true, mode: 0o700 }) so the directory is also user-only.
| telemetryNoticeShown: false, | ||
| commandCount: 0, | ||
| }; | ||
|
|
There was a problem hiding this comment.
Claude: telemetryEnabled: true as default means new users are opted-in before seeing the notice. The notice is shown on first command run (not on install), so there's a window where telemetry fires before the user knows about it. The first-run notice in showTelemetryNotice() and the tracking in trackCommand() are called in the same block in cli.ts, but trackCommand runs before the notice is persisted. If the process crashes between track and write, the user gets tracked without seeing the notice. Consider: show and persist the notice before tracking.
|
|
||
| const commandArg = process.argv[2]; | ||
| const isHelpOrVersion = process.argv.includes("--help") || process.argv.includes("--version") || process.argv.includes("-h"); | ||
| const command = commandArg && commandArg in subCommands ? commandArg : "unknown"; |
There was a problem hiding this comment.
Claude: trackCommand fires before the command actually runs. If the command fails to import (e.g., missing dependency), we still track it as invoked. This inflates usage numbers. Consider moving the track call to after the command completes, or track it as "command_started" and add a separate "command_completed" event.
| } | ||
|
|
||
| scaffoldProject(destDir, basename(destDir), templateId, localVideoName); | ||
| trackInitTemplate(templateId); |
There was a problem hiding this comment.
Claude: trackInitTemplate is called in two places (line 384 and line 503) for the two code paths (direct template arg vs interactive prompt). If both paths somehow execute (bug), you'd double-track. Consider moving the track call to a single place after scaffoldProject — or dedup by tracking only once per invocation.
| }); | ||
| await producer.executeRenderJob(job, projectDir, outputPath); | ||
| } catch (error: unknown) { | ||
| trackRenderError({ fps: options.fps, quality: options.quality, docker: true }); |
There was a problem hiding this comment.
Claude: The error tracking fires before process.exit(1). Since process.exit triggers the synchronous flushSync in cli.ts, the error event will be flushed via execFileSync — a blocking child process spawn during an error path. If the network is down (which might be why the render failed), this adds a 5-second delay before the user sees the error exit. Consider making the error track fully fire-and-forget.
| * In dev: files are .ts (running via tsx). In production: bundled into .js by tsup. | ||
| */ | ||
| export function isDevMode(): boolean { | ||
| try { |
There was a problem hiding this comment.
Claude nit: The try/catch here is defensive but import.meta.url is always available in ESM. The only way this throws is if new URL() fails, which shouldn't happen. The catch returning false means "assume production" on failure — that's the right default for telemetry (fail-safe), but consider adding a comment explaining this is intentional.
- Run oxfmt on cli.ts and client.ts - Replace literal placeholder comparison with phc_ prefix check (TS2367: comparing two different string literals has no overlap) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- flushSync: use detached spawn + unref() instead of execFileSync, so process.exit() paths don't block up to 5s on slow networks - showTelemetryNotice: persist notice flag BEFORE printing/tracking, so users are never tracked without having seen the disclosure - Config dir: set mode 0o700 on ~/.hyperframes/ directory (was umask default) - $ip: null comment: clarify this is belt-and-suspenders with server-side discard - shouldTrack: update comment — phc_ prefix check is a safety net, not dead code - env.ts: add comment explaining try/catch fail-safe defaults to production - init.ts: consistently call trackInitTemplate after scaffoldProject in both paths Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
All review feedback from @miguel-heygen addressed in e52a19b:
|
Merge activity
|
Summary
hyperframes telemetry [enable|disable|status]command for user control~/.hyperframes/config.json— future-proofed for more settingsDesign Decisions
fetchinstead ofposthog-nodefetch. Can swap to SDK later if needed..tsextension detection (sharedutils/env.ts). Running viatsx= dev.phc_prefix checkflushSyncspawns a detached child process soprocess.exit()paths don't block.What's Collected
What's NOT Collected
$ip: nullon every event payload (client-side) + "Discard client IP data" enabled in PostHog project settings (server-side)Opt-Out Mechanisms
hyperframes telemetry disableHYPERFRAMES_NO_TELEMETRY=1DO_NOT_TRACK=1CI=true)Files Changed
New files:
packages/cli/src/telemetry/config.ts— Config read/write at~/.hyperframes/config.json(dir 0700, file 0600)packages/cli/src/telemetry/client.ts— PostHog HTTP client (queue, batch, flush, detached flushSync)packages/cli/src/telemetry/events.ts— Typed event helperspackages/cli/src/telemetry/index.ts— Barrel exportspackages/cli/src/commands/telemetry.ts—hyperframes telemetrycommandpackages/cli/src/utils/env.ts— SharedisDevMode()(extracted from dev.ts)Modified files:
packages/cli/src/cli.ts— Wire telemetry at entry point + add telemetry subcommandpackages/cli/src/commands/render.ts— Track render success/failure metricspackages/cli/src/commands/init.ts— Track template selectionpackages/cli/src/commands/browser.ts— Track browser download eventspackages/cli/src/commands/dev.ts— Use sharedisDevMode()from utils/env.tsTesting
tsc --noEmit)oxlint)oxfmt --check)hyperframes telemetry status/enable/disablecommands--help/--versiondon't trigger telemetryphc_prefix🤖 Generated with Claude Code