feat(telemetry): onboarding-funnel events + nth_repo_added cohort (#1591)

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Brennan Benson
2026-05-08 10:50:43 -07:00
committed by GitHub
co-authored by Orca
parent 20f950de3d
commit 002e2acc38
16 changed files with 652 additions and 29 deletions
+8
View File
@@ -16,6 +16,7 @@ import { closeAllWatchers } from './ipc/filesystem-watcher'
import { registerCoreHandlers } from './ipc/register-core-handlers'
import { registerMobileHandlers } from './ipc/mobile'
import { initTelemetry, shutdownTelemetry, trackAppOpenedOnce } from './telemetry/client'
import { initCohortClassifier } from './telemetry/cohort-classifier'
import { resolveConsent } from './telemetry/consent'
import { triggerStartupNotificationRegistration } from './ipc/notifications'
import { OrcaRuntimeService } from './runtime/orca-runtime'
@@ -399,6 +400,13 @@ app.whenReady().then(async () => {
// the Store reference, seeds common props, and resets per-session burst
// caps. Actual transport initialization is still gated by both flags.
initTelemetry(store)
// Why: cohort-classifier reads the repo count synchronously at every emit
// for cohort-extended events. The Store has been sync-loaded above, and
// this init runs before any IPC handler is registered and before any
// window loads — so the classifier is hydrated before any `track()` call,
// regardless of whether it originates from the renderer, an IPC handler,
// or `trackAppOpenedOnce` / `did-finish-load`.
initCohortClassifier(store)
stats = new StatsCollector()
claudeUsage = new ClaudeUsageStore(store)
codexUsage = new CodexUsageStore(store)
+5 -2
View File
@@ -28,6 +28,7 @@ import { applyTerminalAttributionEnv } from '../attribution/terminal-attribution
import { registerPty, unregisterPty } from '../memory/pty-registry'
import { track } from '../telemetry/client'
import { classifyError } from '../telemetry/classify-error'
import { getCohortAtEmit } from '../telemetry/cohort-classifier'
import {
agentKindSchema,
launchSourceSchema,
@@ -945,7 +946,8 @@ export function registerPtyHandlers(
const classified = classifyError(err)
track('agent_error', {
agent_kind: errorAgentKind,
error_class: classified.error_class
error_class: classified.error_class,
...getCohortAtEmit()
})
}
throw err
@@ -1101,7 +1103,8 @@ export function registerPtyHandlers(
track('agent_started', {
agent_kind: agentKindParse.data,
launch_source: launchSourceParse.data,
request_kind: requestKindParse.data
request_kind: requestKindParse.data,
...getCohortAtEmit()
})
}
}
+7 -1
View File
@@ -30,6 +30,7 @@ import { getSshGitProvider } from '../providers/ssh-git-dispatch'
import { getActiveMultiplexer } from './ssh'
import { normalizeSparseDirectories } from './sparse-checkout-directories'
import { track } from '../telemetry/client'
import { getCohortAtEmit } from '../telemetry/cohort-classifier'
import type { RepoMethod } from '../../shared/telemetry-events'
// Why: `method` answers "which entry point did the user take?", not "what did
@@ -46,7 +47,11 @@ function emitRepoAdded(method: RepoMethod, alreadyExisted: boolean): void {
if (alreadyExisted) {
return
}
track('repo_added', { method })
// Why: cohort must read AFTER `store.addRepo()` lands so the just-added
// repo is counted — every call site below already emits post-addRepo, so
// `getCohortAtEmit()` here returns the user's Nth `repo_added` as `N`.
// See docs/onboarding-funnel-cohort-addendum.md §Read-vs-write ordering.
track('repo_added', { method, ...getCohortAtEmit() })
}
// Why: module-scoped so the abort handle survives window re-creation on macOS.
@@ -391,6 +396,7 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v
// other invocation is using it. Leaking a freshly-made empty folder on
// a rare race is strictly safer than deleting a directory the winning
// call (and the user) now owns.
emitRepoAdded('folder_picker', true)
return { repo: raceWinner }
}
+55 -6
View File
@@ -16,13 +16,15 @@ const {
trackMock,
setOptInMock,
persistBannerAcknowledgeMock,
consumeConsentMutationTokenMock
consumeConsentMutationTokenMock,
getCohortAtEmitMock
} = vi.hoisted(() => ({
handleMock: vi.fn(),
trackMock: vi.fn(),
setOptInMock: vi.fn(),
persistBannerAcknowledgeMock: vi.fn(),
consumeConsentMutationTokenMock: vi.fn()
consumeConsentMutationTokenMock: vi.fn(),
getCohortAtEmitMock: vi.fn()
}))
vi.mock('electron', () => ({ ipcMain: { handle: handleMock } }))
@@ -34,6 +36,9 @@ vi.mock('../telemetry/client', () => ({
vi.mock('../telemetry/burst-cap', () => ({
consumeConsentMutationToken: consumeConsentMutationTokenMock
}))
vi.mock('../telemetry/cohort-classifier', () => ({
getCohortAtEmit: getCohortAtEmitMock
}))
import { _resetStoreForTests, registerTelemetryHandlers } from './telemetry'
@@ -82,6 +87,8 @@ describe('telemetry IPC handlers', () => {
persistBannerAcknowledgeMock.mockReset()
consumeConsentMutationTokenMock.mockReset()
consumeConsentMutationTokenMock.mockReturnValue(true)
getCohortAtEmitMock.mockReset()
getCohortAtEmitMock.mockReturnValue({ nth_repo_added: 0 })
_resetStoreForTests()
})
afterEach(() => {
@@ -102,12 +109,53 @@ describe('telemetry IPC handlers', () => {
// ── telemetry:track ──────────────────────────────────────────────────
it('forwards a well-typed track call to track()', () => {
it('forwards a well-typed track call to track() and injects cohort for COHORT_EXTENDED events', () => {
registerWith({ installId: 'x', existedBeforeTelemetryRelease: false, optedIn: true })
getCohortAtEmitMock.mockReturnValue({ nth_repo_added: 2 })
const handler = handlers.get('telemetry:track')!
handler({}, 'app_opened', {})
expect(trackMock).toHaveBeenCalledTimes(1)
expect(trackMock).toHaveBeenCalledWith('app_opened', {})
expect(trackMock).toHaveBeenCalledWith('app_opened', { nth_repo_added: 2 })
})
// The IPC handler's selectivity is load-bearing: schemas are `.strict()`,
// so injecting `nth_repo_added` on a non-cohort event would silently
// drop the entire event at the validator. Events outside `COHORT_EXTENDED`
// must reach `track()` unmodified.
it('does NOT inject cohort on events outside COHORT_EXTENDED', () => {
registerWith({ installId: 'x', existedBeforeTelemetryRelease: false, optedIn: true })
const handler = handlers.get('telemetry:track')!
handler({}, 'settings_changed', { setting_key: 'editorAutoSave', value_kind: 'bool' })
expect(trackMock).toHaveBeenCalledTimes(1)
expect(trackMock).toHaveBeenCalledWith('settings_changed', {
setting_key: 'editorAutoSave',
value_kind: 'bool'
})
expect(getCohortAtEmitMock).not.toHaveBeenCalled()
})
// The renderer-only Setup-step events fire from React `onClick` and
// depend on the IPC handler injecting cohort — call sites stay
// synchronous and pass only their own props.
it('injects cohort for add_repo_setup_step_action (renderer-only event)', () => {
registerWith({ installId: 'x', existedBeforeTelemetryRelease: false, optedIn: true })
getCohortAtEmitMock.mockReturnValue({ nth_repo_added: 1 })
const handler = handlers.get('telemetry:track')!
handler({}, 'add_repo_setup_step_action', { action: 'skip' })
expect(trackMock).toHaveBeenCalledWith('add_repo_setup_step_action', {
action: 'skip',
nth_repo_added: 1
})
})
// Fail-soft: a degraded classifier returns `{ nth_repo_added: undefined }`.
// The schemas declare the field optional, so the event still validates.
it('forwards undefined cohort when the classifier returns undefined', () => {
registerWith({ installId: 'x', existedBeforeTelemetryRelease: false, optedIn: true })
getCohortAtEmitMock.mockReturnValue({ nth_repo_added: undefined })
const handler = handlers.get('telemetry:track')!
handler({}, 'app_opened', {})
expect(trackMock).toHaveBeenCalledWith('app_opened', { nth_repo_added: undefined })
})
it('drops track calls with a non-string name', () => {
@@ -129,12 +177,13 @@ describe('telemetry IPC handlers', () => {
it('treats null/undefined props as an empty object', () => {
registerWith({ installId: 'x', existedBeforeTelemetryRelease: false, optedIn: true })
getCohortAtEmitMock.mockReturnValue({ nth_repo_added: 0 })
const handler = handlers.get('telemetry:track')!
handler({}, 'app_opened', null)
handler({}, 'app_opened', undefined)
expect(trackMock).toHaveBeenCalledTimes(2)
expect(trackMock).toHaveBeenNthCalledWith(1, 'app_opened', {})
expect(trackMock).toHaveBeenNthCalledWith(2, 'app_opened', {})
expect(trackMock).toHaveBeenNthCalledWith(1, 'app_opened', { nth_repo_added: 0 })
expect(trackMock).toHaveBeenNthCalledWith(2, 'app_opened', { nth_repo_added: 0 })
})
// ── telemetry:setOptIn — input narrowing ─────────────────────────────
+15 -1
View File
@@ -38,8 +38,10 @@
import { ipcMain } from 'electron'
import { consumeConsentMutationToken } from '../telemetry/burst-cap'
import { persistBannerAcknowledgeWithoutEmitting, setOptIn, track } from '../telemetry/client'
import { getCohortAtEmit } from '../telemetry/cohort-classifier'
import { resolveConsent, type ConsentState } from '../telemetry/consent'
import type { Store } from '../persistence'
import { isCohortExtendedEvent } from '../../shared/telemetry-events'
import type { EventName, EventProps } from '../../shared/telemetry-events'
import type { OptInVia } from '../../shared/telemetry-events'
@@ -114,12 +116,24 @@ export function registerTelemetryHandlers(store: Store): void {
if (props !== null && props !== undefined && typeof props !== 'object') {
return
}
// Inject cohort here, at the IPC entry, only for events whose schemas
// declare `nth_repo_added` (see `COHORT_EXTENDED` in telemetry-events.ts).
// The selectivity is load-bearing: schemas are `.strict()`, so adding
// `nth_repo_added` to an event that does not declare it would fail Zod
// validation and silently drop the entire event. The renderer call sites
// stay synchronous (matching the existing fire-and-forget shape) and
// avoid an extra IPC round-trip to fetch cohort.
const eventName = name as EventName
const baseProps = (props ?? {}) as Record<string, unknown>
const finalProps = isCohortExtendedEvent(eventName)
? { ...baseProps, ...getCohortAtEmit() }
: baseProps
// The casts to `EventName` / `EventProps<EventName>` here are
// pass-through only — this file does NOT pretend the renderer's
// name/props are type-safe. The validator inside `track()` is the
// single enforcement point at runtime; these casts only feed the
// typed channel that the validator will re-check.
track(name as EventName, (props ?? {}) as EventProps<EventName>)
track(eventName, finalProps as EventProps<EventName>)
})
ipcMain.handle('telemetry:setOptIn', (_event, optedIn: unknown): Promise<void> | void => {
@@ -0,0 +1,76 @@
// Spot-check the classifier against the load-bearing throws in
// worktree-remote.ts. Not exhaustive — substring widening still requires
// explicit review per the schema-evolution doctrine in
// telemetry-events.ts. Add fixtures here when adding new buckets or when
// a renamed throw site needs regression coverage.
import { describe, expect, it } from 'vitest'
import { classifyWorkspaceCreateError } from './workspace-create-error-classifier'
describe('classifyWorkspaceCreateError', () => {
it('buckets the missing-base-ref throw as base_ref_missing', () => {
const err = new Error(
'Could not resolve a default base ref for this repo. Pick a base branch explicitly and try again.'
)
expect(classifyWorkspaceCreateError(err)).toBe('base_ref_missing')
})
it('buckets a branch-already-exists throw as path_collision', () => {
const err = new Error('Branch "feature/foo" already exists. Pick a different worktree name.')
expect(classifyWorkspaceCreateError(err)).toBe('path_collision')
})
it('buckets the suffix-exhaustion throw as path_collision', () => {
const err = new Error(
'Could not find an available worktree name for "feature". Pick a different worktree name.'
)
expect(classifyWorkspaceCreateError(err)).toBe('path_collision')
})
it('buckets a branch-already-exists-locally throw as path_collision', () => {
const err = new Error(
'Branch "feature/foo" already exists locally. Pick a different worktree name.'
)
expect(classifyWorkspaceCreateError(err)).toBe('path_collision')
})
it('buckets an existing-PR collision throw as path_collision', () => {
const err = new Error(
'Branch "feature/foo" already has PR #42. Pick a different worktree name.'
)
expect(classifyWorkspaceCreateError(err)).toBe('path_collision')
})
it('buckets the post-create listing-miss throw as git_failed', () => {
const err = new Error('Worktree created but not found in listing')
expect(classifyWorkspaceCreateError(err)).toBe('git_failed')
})
it('buckets EACCES errors as permission_denied', () => {
const err = Object.assign(new Error("EACCES: permission denied, mkdir '/tmp/x'"), {
code: 'EACCES'
})
expect(classifyWorkspaceCreateError(err)).toBe('permission_denied')
})
it('buckets generic git errors as git_failed', () => {
const err = new Error('fatal: not a git repository')
expect(classifyWorkspaceCreateError(err)).toBe('git_failed')
})
it('falls through to unknown for unrecognised errors', () => {
const err = new Error('something completely unexpected')
expect(classifyWorkspaceCreateError(err)).toBe('unknown')
})
it('falls through to unknown for SSH-precondition errors', () => {
const err = new Error('SSH connection is not available. Please reconnect and try again.')
expect(classifyWorkspaceCreateError(err)).toBe('unknown')
})
it('handles non-Error values without throwing', () => {
expect(classifyWorkspaceCreateError('a bare string')).toBe('unknown')
expect(classifyWorkspaceCreateError(undefined)).toBe('unknown')
expect(classifyWorkspaceCreateError(null)).toBe('unknown')
})
})
@@ -0,0 +1,53 @@
// Bucket errors thrown by `createLocalWorktree` / `createRemoteWorktree` into
// the `workspace_create_failed` event's `error_class` enum.
//
// Why: the throw sites are bare `throw new Error('...')` calls in
// worktree-remote.ts, some of which interpolate user-controlled content
// (branch names, paths). The classifier reads `error.message` to bucket, but
// the matched strings never cross the wire — only the enum value does. The
// schema discipline at telemetry-events.ts §"Properties to keep off these
// events" is what makes that safe.
//
// Substring set is intentionally narrow: per the schema-evolution doctrine,
// widening this match set later requires explicit review, not silent regex
// expansion. Anything we cannot confidently bucket falls through to `unknown`.
import type { WorkspaceCreateErrorClass } from '../../shared/telemetry-events'
export function classifyWorkspaceCreateError(error: unknown): WorkspaceCreateErrorClass {
// Why: throw sites mix capitalization ('Worktree created...' vs lowercased
// git messages); normalize once so all anchors below can be lowercase literals.
const text = (error instanceof Error ? error.message : '').toLowerCase()
if (text.includes('could not resolve a default base ref')) {
return 'base_ref_missing'
}
if (
text.includes('already exists locally') ||
text.includes('already exists on a remote') ||
text.includes('already exists. pick') ||
text.includes('already has pr') ||
text.includes('could not find an available worktree name')
) {
return 'path_collision'
}
if (text.includes('eacces') || text.includes('eperm') || text.includes('permission denied')) {
return 'permission_denied'
}
// Why: anchors are intentionally specific to true git failures from
// worktree-remote.ts. Bare 'git ' / 'worktree' would mis-bucket SSH-relay
// and sparse-checkout validation errors (which the design doc routes to
// 'unknown'); 'created but not found in listing' covers the formerly
// miscased 'Worktree created but not found in listing' throw.
// SSH-precondition failures (e.g. 'no git provider', 'SSH connection is
// not available') deliberately fall through to 'unknown' — they're not
// git failures and bucketing them here would mask connectivity issues.
if (
text.includes('fatal:') ||
text.includes('git worktree') ||
text.includes('created but not found in listing')
) {
return 'git_failed'
}
return 'unknown'
}
+25 -7
View File
@@ -45,7 +45,9 @@ import { killAllProcessesForWorktree } from '../runtime/worktree-teardown'
import { getLocalPtyProvider } from './pty'
import { removeWorktreeSymlinks } from './worktree-symlinks'
import { track } from '../telemetry/client'
import { getCohortAtEmit } from '../telemetry/cohort-classifier'
import { workspaceSourceSchema, type WorkspaceSource } from '../../shared/telemetry-events'
import { classifyWorkspaceCreateError } from './workspace-create-error-classifier'
// Why: worktrees discovered on disk (not created via Orca's UI) have no
// persisted WorktreeMeta, so mergeWorktree falls back to `lastActivityAt: 0`.
@@ -229,10 +231,27 @@ export function registerWorktreeHandlers(
throw new Error('Folder mode does not support creating worktrees.')
}
// Remote repos route all git operations through the relay
const result = repo.connectionId
? await createRemoteWorktree(args, repo, store, mainWindow)
: await createLocalWorktree(args, repo, store, mainWindow, runtime)
const sourceParse = workspaceSourceSchema.safeParse(args.telemetrySource)
const source: WorkspaceSource = sourceParse.success ? sourceParse.data : 'unknown'
let result: CreateWorktreeResult
try {
// Why: only wrap the helpers themselves. The pre-validation throws
// above (`Repo not found`, `Folder mode does not support creating
// worktrees`) signal IPC-shape bugs, not the user-visible
// git/filesystem failures the funnel cares about — bucketing them
// into `unknown` would pollute the failure taxonomy.
result = repo.connectionId
? await createRemoteWorktree(args, repo, store, mainWindow)
: await createLocalWorktree(args, repo, store, mainWindow, runtime)
} catch (error) {
track('workspace_create_failed', {
source,
error_class: classifyWorkspaceCreateError(error),
...getCohortAtEmit()
})
throw error
}
// Why: emit `workspace_created` only after the underlying create has
// resolved (the helpers throw on failure, so reaching this line means
@@ -242,11 +261,10 @@ export function registerWorktreeHandlers(
// baseBranch; an unspecified baseBranch means "branch from default
// HEAD", which is the not-from-existing-branch case. We never send
// the branch name itself.
const sourceParse = workspaceSourceSchema.safeParse(args.telemetrySource)
const source: WorkspaceSource = sourceParse.success ? sourceParse.data : 'unknown'
track('workspace_created', {
source,
from_existing_branch: typeof args.baseBranch === 'string' && args.baseBranch.length > 0
from_existing_branch: typeof args.baseBranch === 'string' && args.baseBranch.length > 0,
...getCohortAtEmit()
})
return result
+10
View File
@@ -470,6 +470,16 @@ export class Store {
return this.state.repos.map((repo) => this.hydrateRepo(repo))
}
/**
* O(1) read of the persisted repo count. Use this when you only need the
* count (e.g. cohort-classifier) — `getRepos()` hydrates each repo and
* may run a synchronous git subprocess via `getGitUsername()`, which is
* wasteful when the caller only reads `.length`.
*/
getRepoCount(): number {
return this.state.repos.length
}
getRepo(id: string): Repo | undefined {
const repo = this.state.repos.find((r) => r.id === id)
return repo ? this.hydrateRepo(repo) : undefined
+5 -1
View File
@@ -33,6 +33,7 @@ import { PostHog } from 'posthog-node'
import type { CommonProps, EventName, EventProps, OptInVia } from '../../shared/telemetry-events'
import type { Store } from '../persistence'
import { consumeBurstToken, resetBurstCapsForSession } from './burst-cap'
import { getCohortAtEmit } from './cohort-classifier'
import { resolveConsent, type ConsentState } from './consent'
import { commonPropsSchema, validate } from './validator'
@@ -436,7 +437,10 @@ export function trackAppOpenedOnce(): void {
return
}
appOpenedTrackedThisSession = true
track('app_opened', {})
// Why: `nth_repo_added: 0` on `app_opened` is the canonical session-zero
// / pre-repo cohort signal — a user who has launched but never added a
// repo. See docs/onboarding-funnel-cohort-addendum.md.
track('app_opened', { ...getCohortAtEmit() })
}
export async function shutdownTelemetry(): Promise<void> {
@@ -0,0 +1,101 @@
// Pins the cohort-classifier contract: synchronous read of
// `store.getRepoCount()`, fail-soft to `undefined` on any failure mode,
// at most one warn per session. See docs/onboarding-funnel-cohort-addendum.md.
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import type { Repo } from '../../shared/types'
import type { Store } from '../persistence'
import {
_resetSessionWarnFlagForTests,
_setStoreForTests,
getCohortAtEmit,
initCohortClassifier
} from './cohort-classifier'
function makeFakeStore(getRepos: () => Repo[]): Store {
// Both reads delegate to the same `getRepos` thunk so existing tests stay
// meaningful: a throw or a list change is observed identically through
// either accessor.
return {
getRepos: vi.fn(getRepos),
getRepoCount: vi.fn(() => getRepos().length)
} as unknown as Store
}
function makeRepos(n: number): Repo[] {
return Array.from({ length: n }, (_, i) => ({ id: `repo-${i}` }) as unknown as Repo)
}
describe('cohort-classifier', () => {
beforeEach(() => {
vi.spyOn(console, 'warn').mockImplementation(() => {})
_setStoreForTests(null)
_resetSessionWarnFlagForTests()
})
afterEach(() => {
vi.restoreAllMocks()
_setStoreForTests(null)
})
it('returns the current repo count', () => {
initCohortClassifier(makeFakeStore(() => makeRepos(3)))
expect(getCohortAtEmit()).toEqual({ nth_repo_added: 3 })
})
// The session-zero / pre-repo cohort signal: a launch with no repos must
// emit `0`, not `undefined`. Filtering `nth_repo_added = 0` on
// `app_opened` is the canonical way to isolate the pre-repo cohort.
it('returns 0 (not undefined) for an empty repo list', () => {
initCohortClassifier(makeFakeStore(() => []))
expect(getCohortAtEmit()).toEqual({ nth_repo_added: 0 })
})
it('returns undefined when the store is not initialized', () => {
expect(getCohortAtEmit()).toEqual({ nth_repo_added: undefined })
})
it('returns undefined and never throws when getRepos throws', () => {
initCohortClassifier(
makeFakeStore(() => {
throw new Error('disk fault')
})
)
expect(() => getCohortAtEmit()).not.toThrow()
expect(getCohortAtEmit()).toEqual({ nth_repo_added: undefined })
})
// Why: a degraded boot should not flood stderr; the warn-once flag is the
// breadcrumb mechanism for "why is some chunk of last week's data
// missing nth_repo_added?" without burning logs on every emit.
it('warns at most once per session even across many degraded calls', () => {
initCohortClassifier(
makeFakeStore(() => {
throw new Error('disk fault')
})
)
const warnSpy = console.warn as unknown as ReturnType<typeof vi.spyOn>
for (let i = 0; i < 50; i++) {
getCohortAtEmit()
}
expect(warnSpy).toHaveBeenCalledTimes(1)
})
// The session-warn flag resets on initCohortClassifier so a fresh
// process gets a fresh breadcrumb budget.
it('resets the warn flag when reinitialized', () => {
initCohortClassifier(
makeFakeStore(() => {
throw new Error('first')
})
)
getCohortAtEmit()
initCohortClassifier(
makeFakeStore(() => {
throw new Error('second')
})
)
getCohortAtEmit()
const warnSpy = console.warn as unknown as ReturnType<typeof vi.spyOn>
expect(warnSpy).toHaveBeenCalledTimes(2)
})
})
+64
View File
@@ -0,0 +1,64 @@
// Single source of truth for cohort state attached to telemetry events.
// See docs/onboarding-funnel-cohort-addendum.md.
//
// `nth_repo_added` is the count of repos the user has at the moment the
// event fires — read from `store.getRepoCount()`. The rule is single
// and consistent: the value reflects current store state at emit time.
// On `repo_added` the read happens *after* `store.addRepo` lands, so the
// user's Nth repo addition emits `N` (the just-landed write is included).
// On every other event, the same read returns whatever the count is —
// including `0` for a brand-new user on `app_opened` who has never added
// a repo. That `0` is the canonical session-zero / pre-repo cohort signal,
// not a sentinel and not "undefined."
//
// Failure mode: this module never throws. On any read error or
// store-not-yet-initialized condition, `getCohortAtEmit` returns
// `{ nth_repo_added: undefined }`. The schemas declare the field
// `.optional()`, so an event with an undefined cohort still validates and
// emits — it just lands without the cohort property. This preserves the
// telemetry rule "must never crash the app."
import type { Store } from '../persistence'
let storeRef: Store | null = null
// Session-scoped flag analogous to `appOpenedTrackedThisSession` in
// `client.ts`: emit one debug breadcrumb per session if the classifier
// has to fail soft, so missing cohort data has a corresponding log line
// without flooding stderr.
let warnedThisSession = false
export function initCohortClassifier(store: Store): void {
storeRef = store
warnedThisSession = false
}
export function getCohortAtEmit(): { nth_repo_added: number | undefined } {
if (!storeRef) {
warnOnce('store not initialized')
return { nth_repo_added: undefined }
}
try {
const length = storeRef.getRepoCount()
return { nth_repo_added: length }
} catch (err) {
warnOnce(err instanceof Error ? err.message : String(err))
return { nth_repo_added: undefined }
}
}
function warnOnce(reason: string): void {
if (warnedThisSession) {
return
}
warnedThisSession = true
console.warn('[telemetry-cohort] classifier returned undefined', { reason })
}
export function _setStoreForTests(store: Store | null): void {
storeRef = store
}
export function _resetSessionWarnFlagForTests(): void {
warnedThisSession = false
}
+48
View File
@@ -102,6 +102,54 @@ describe('validate', () => {
expect(result.ok).toBe(false)
})
// ── Cohort property (nth_repo_added) ────────────────────────────────
// Pin the schema contract from
// docs/onboarding-funnel-cohort-addendum.md. The field is optional so a
// classifier degraded-mode `undefined` still validates; rejected shapes
// (negative, non-integer, string) must drop.
it('accepts app_opened with nth_repo_added=0 (the session-zero cohort signal)', () => {
const result = validate('app_opened', { nth_repo_added: 0 })
expect(result.ok).toBe(true)
})
it('accepts repo_added with nth_repo_added=1', () => {
const result = validate('repo_added', { method: 'folder_picker', nth_repo_added: 1 })
expect(result.ok).toBe(true)
})
it('accepts events without nth_repo_added (classifier degraded mode)', () => {
const result = validate('agent_started', {
agent_kind: 'claude-code',
launch_source: 'command_palette',
request_kind: 'new'
})
expect(result.ok).toBe(true)
})
it('rejects negative nth_repo_added', () => {
const result = validate('app_opened', { nth_repo_added: -1 } as never)
expect(result.ok).toBe(false)
})
it('rejects non-integer nth_repo_added', () => {
const result = validate('app_opened', { nth_repo_added: 1.5 } as never)
expect(result.ok).toBe(false)
})
it('rejects nth_repo_added on a non-cohort event (settings_changed)', () => {
// The IPC handler relies on this rejection: schemas are `.strict()`,
// so injecting `nth_repo_added` on an event whose schema does not
// declare it must drop the entire event. The selectivity guard in
// `telemetry:track` is what prevents that from happening in practice.
const result = validate('settings_changed', {
setting_key: 'editorAutoSave',
value_kind: 'bool',
nth_repo_added: 1
} as never)
expect(result.ok).toBe(false)
})
// Rate-limit: at most one warn per event name per 60s. We cannot easily
// control Date.now() without mocking time, so the coarse assertion is
// that repeat-dropping the same event name does not emit a warn on every
@@ -11,6 +11,7 @@ import {
} from '@/components/ui/dialog'
import { Button } from '@/components/ui/button'
import { activateAndRevealWorktree } from '@/lib/worktree-activation'
import { track } from '@/lib/telemetry'
import { RemoteStep, CloneStep, useRemoteRepo } from './AddRepoSteps'
import { CreateStep, useCreateRepo } from './AddRepoCreateStep'
import { SetupStep } from './AddRepoSetupStep'
@@ -193,6 +194,7 @@ const AddRepoDialog = React.memo(function AddRepoDialog() {
const handleOpenWorktree = useCallback(
(worktree: Worktree) => {
track('add_repo_setup_step_action', { action: 'open_existing' })
activateAndRevealWorktree(worktree.id)
closeModal()
},
@@ -200,6 +202,8 @@ const AddRepoDialog = React.memo(function AddRepoDialog() {
)
const handleCreateWorktree = useCallback(() => {
// Why: Setup-step "Create" affordance — fires on click intent, not on IPC arrival, mirroring the other 4 actions in this dialog.
track('add_repo_setup_step_action', { action: 'create_worktree' })
// Why: small delay so the Add Project dialog close animation finishes before
// the composer modal takes focus; otherwise the dialog teardown can steal
// the first focus frame from the composer's prompt textarea.
@@ -210,6 +214,7 @@ const AddRepoDialog = React.memo(function AddRepoDialog() {
}, [closeModal, openModal, repoId])
const handleConfigureRepo = useCallback(() => {
track('add_repo_setup_step_action', { action: 'configure' })
closeModal()
openSettingsTarget({ pane: 'repo', repoId })
openSettingsPage()
@@ -218,11 +223,33 @@ const AddRepoDialog = React.memo(function AddRepoDialog() {
// Why: handleBack reuses resetState which already aborts clones and resets all fields.
const handleBack = resetState
const handleSkip = useCallback(() => {
track('add_repo_setup_step_action', { action: 'skip' })
closeModal()
resetState()
}, [closeModal, resetState])
// Why: only the Setup step's "Add another project" back arrow counts as a
// funnel event — the in-flight Back arrows on clone/remote/create are not
// a Setup-step affordance. Keeping the emit scoped to this handler avoids
// also tagging mid-clone backs.
const handleSetupStepBack = useCallback(() => {
track('add_repo_setup_step_action', { action: 'back' })
handleBack()
}, [handleBack])
return (
<Dialog
open={isOpen}
onOpenChange={(open) => {
if (!open) {
// Why: Radix only fires onOpenChange for internal triggers (X icon, ESC,
// outside-click), so this branch only runs for implicit closes — explicit
// Skip is handled on its own renderer-side click handler. Implicit closes
// on the Setup step are funnel-equivalent to Skip.
if (step === 'setup') {
track('add_repo_setup_step_action', { action: 'skip' })
}
closeModal()
resetState()
}
@@ -243,7 +270,7 @@ const AddRepoDialog = React.memo(function AddRepoDialog() {
{step === 'setup' && (
<button
className="absolute left-6 inline-flex items-center gap-1 text-xs text-muted-foreground hover:text-foreground transition-colors cursor-pointer"
onClick={handleBack}
onClick={handleSetupStepBack}
>
<ArrowLeft className="size-3" />
Add another project
@@ -396,10 +423,7 @@ const AddRepoDialog = React.memo(function AddRepoDialog() {
onOpenWorktree={handleOpenWorktree}
onCreateWorktree={handleCreateWorktree}
onConfigureRepo={handleConfigureRepo}
onSkip={() => {
closeModal()
resetState()
}}
onSkip={handleSkip}
/>
)}
</DialogContent>
+64
View File
@@ -5,6 +5,7 @@
import { describe, expect, it } from 'vitest'
import {
addRepoSetupStepActionSchema,
AGENT_KIND_VALUES,
agentKindSchema,
commonPropsSchema,
@@ -110,6 +111,69 @@ describe('agent_started schema', () => {
})
})
describe('add_repo_setup_step_action schema', () => {
it('accepts every Setup-step action declared in the schema', () => {
for (const action of addRepoSetupStepActionSchema.options) {
const parsed = eventSchemas.add_repo_setup_step_action.safeParse({ action })
expect(parsed.success).toBe(true)
}
})
it('rejects unknown action enum values', () => {
const parsed = eventSchemas.add_repo_setup_step_action.safeParse({
action: 'export_to_pdf'
})
expect(parsed.success).toBe(false)
})
it('rejects extra keys via .strict()', () => {
const parsed = eventSchemas.add_repo_setup_step_action.safeParse({
action: 'skip',
repo_name: 'orca' // raw repo names are UGC — must not cross the wire
})
expect(parsed.success).toBe(false)
})
})
describe('workspace_create_failed schema', () => {
it('accepts a valid payload', () => {
const parsed = eventSchemas.workspace_create_failed.safeParse({
source: 'sidebar',
error_class: 'git_failed'
})
expect(parsed.success).toBe(true)
})
it('rejects unknown error_class values', () => {
const parsed = eventSchemas.workspace_create_failed.safeParse({
source: 'sidebar',
error_class: 'cosmic_ray'
})
expect(parsed.success).toBe(false)
})
// Core invariant mirroring agent_error: raw error strings never cross the
// wire. If this test ever flips, the failure-rate lane is leaking UGC —
// revert the offending schema change.
it('rejects error_message via .strict()', () => {
const parsed = eventSchemas.workspace_create_failed.safeParse({
source: 'sidebar',
error_class: 'git_failed',
error_message: 'fatal: cannot create work tree at /Users/alice/secret'
})
expect(parsed.success).toBe(false)
})
it('rejects error_stack via .strict()', () => {
const parsed = eventSchemas.workspace_create_failed.safeParse({
source: 'sidebar',
error_class: 'git_failed',
error_stack: 'Error: cannot create work tree\n at /Users/alice/...'
})
expect(parsed.success).toBe(false)
})
})
describe('settings_changed schema', () => {
it('accepts whitelisted setting keys', () => {
for (const key of SETTINGS_CHANGED_WHITELIST) {
+87 -6
View File
@@ -73,6 +73,33 @@ export type ErrorClass = z.infer<typeof errorClassSchema>
export const repoMethodSchema = z.enum(['folder_picker', 'clone_url', 'drag_drop'])
export type RepoMethod = z.infer<typeof repoMethodSchema>
// Five Setup-step affordances the user can pick after `repo_added` fires (see
// AddRepoSetupStep). One enum because every value lives on the same screen and
// the funnel question is "which one did they pick" — adding a sixth value
// later is additive-safe per the schema-evolution doctrine below.
export const addRepoSetupStepActionSchema = z.enum([
'create_worktree',
'configure',
'skip',
'open_existing',
'back'
])
export type AddRepoSetupStepAction = z.infer<typeof addRepoSetupStepActionSchema>
// Deliberately a separate enum from `errorClassSchema` (PTY-spawn taxonomy):
// different domain — this one buckets git/filesystem failures thrown by
// `createLocalWorktree` / `createRemoteWorktree`. Merging the two would lock
// both domains to the union forever, which the schema-evolution comment
// below warns against.
export const workspaceCreateErrorClassSchema = z.enum([
'git_failed',
'path_collision',
'permission_denied',
'base_ref_missing',
'unknown'
])
export type WorkspaceCreateErrorClass = z.infer<typeof workspaceCreateErrorClassSchema>
export const workspaceSourceSchema = z.enum([
'command_palette',
'sidebar',
@@ -145,14 +172,25 @@ export type SettingsChangedKey = z.infer<typeof settingsChangedKeySchema>
// unknown keys at parse time. This is the runtime counterpart to the
// compile-time "unions of string literals, no raw `string`" rule.
const emptySchema = z.object({}).strict()
// Cohort signal — see docs/onboarding-funnel-cohort-addendum.md. One integer
// shared across the events listed in `COHORT_EXTENDED` below: the count of
// repos the user has at emit time, read from `store.getRepos().length`.
// `.int().nonnegative()` constrains malformed values to the floor;
// `.optional()` lets the classifier's fail-soft fallback (returning
// `undefined`) validate cleanly so a read error never crashes a track call.
const nthRepoAddedSchema = z.number().int().nonnegative().optional()
const repoAddedSchema = z.object({ method: repoMethodSchema }).strict()
const appOpenedSchema = z.object({ nth_repo_added: nthRepoAddedSchema }).strict()
const repoAddedSchema = z
.object({ method: repoMethodSchema, nth_repo_added: nthRepoAddedSchema })
.strict()
const workspaceCreatedSchema = z
.object({
source: workspaceSourceSchema,
from_existing_branch: z.boolean()
from_existing_branch: z.boolean(),
nth_repo_added: nthRepoAddedSchema
})
.strict()
@@ -160,7 +198,8 @@ const agentStartedSchema = z
.object({
agent_kind: agentKindSchema,
launch_source: launchSourceSchema,
request_kind: requestKindSchema
request_kind: requestKindSchema,
nth_repo_added: nthRepoAddedSchema
})
.strict()
@@ -172,7 +211,8 @@ const agentStartedSchema = z
const agentErrorSchema = z
.object({
error_class: errorClassSchema,
agent_kind: agentKindSchema
agent_kind: agentKindSchema,
nth_repo_added: nthRepoAddedSchema
})
.strict()
@@ -186,6 +226,22 @@ const settingsChangedSchema = z
const telemetryOptedInSchema = z.object({ via: optInViaSchema }).strict()
const telemetryOptedOutSchema = z.object({ via: optInViaSchema }).strict()
const addRepoSetupStepActionEventSchema = z
.object({ action: addRepoSetupStepActionSchema, nth_repo_added: nthRepoAddedSchema })
.strict()
// Why: same enum-only discipline as `agent_error` — `.strict()` rejects raw
// error strings if a future call site tries to attach `error_message` /
// `error_stack`. The classifier in worktrees.ts reads `error.message` to
// bucket into the enum, but those strings never cross the wire.
const workspaceCreateFailedSchema = z
.object({
source: workspaceSourceSchema,
error_class: workspaceCreateErrorClassSchema,
nth_repo_added: nthRepoAddedSchema
})
.strict()
// ── Event registry: the one record the validator consumes ───────────────
//
// The validator does `eventSchemas[name].safeParse(props)`. `EventMap` is
@@ -200,10 +256,12 @@ const telemetryOptedOutSchema = z.object({ via: optInViaSchema }).strict()
// change silently blends pre- and post-change rows under one event name,
// which cannot be unmixed after the fact.
export const eventSchemas = {
app_opened: emptySchema,
app_opened: appOpenedSchema,
repo_added: repoAddedSchema,
add_repo_setup_step_action: addRepoSetupStepActionEventSchema,
workspace_created: workspaceCreatedSchema,
workspace_create_failed: workspaceCreateFailedSchema,
agent_started: agentStartedSchema,
agent_error: agentErrorSchema,
@@ -218,6 +276,29 @@ export type EventMap = { [N in keyof typeof eventSchemas]: z.infer<(typeof event
export type EventName = keyof EventMap
export type EventProps<N extends EventName> = EventMap[N]
// Events whose schemas declare `nth_repo_added`. Derived from `eventSchemas`
// at module load by probing each schema's `.shape` — there is no parallel
// hand-maintained list to drift out of sync. The IPC `telemetry:track`
// handler injects the cohort property only when the incoming event name is
// in this set: the schemas are `.strict()`, so injecting `nth_repo_added`
// on an event whose schema does not declare it would fail validation and
// silently drop the entire event.
//
// Schema-additions checklist for adding a new cohort-extended event:
// add `nth_repo_added: nthRepoAddedSchema` to the event's schema above.
// That is the *only* step — this set updates automatically.
const COHORT_EXTENDED_SET: ReadonlySet<EventName> = new Set(
(Object.entries(eventSchemas) as [EventName, z.ZodObject<z.ZodRawShape>][])
.filter(([, schema]) => 'nth_repo_added' in schema.shape)
.map(([name]) => name)
)
export const COHORT_EXTENDED: readonly EventName[] = Array.from(COHORT_EXTENDED_SET)
export type CohortExtendedEvent = EventName
export function isCohortExtendedEvent(name: EventName): name is CohortExtendedEvent {
return COHORT_EXTENDED_SET.has(name)
}
// Common props attached by the client — declared here so the validator knows
// which keys to allow on every outgoing event.
//