WIP: Changes before auto-review fixes

Co-authored-by: Orca <help@stably.ai>
2026-10-13 08:02:52 +00:00 · 2026-05-07 11:28:16 -07:00
parent b5d2b288b6
commit d3fd5120dd
8 changed files with 493 additions and 7 deletions
@@ -0,0 +1,83 @@
+# Review Context
+
+## Branch Info
+
+- Base: origin/main (b5d2b288 — branch is at HEAD of main, all changes are uncommitted)
+- Current: brennanb2025/posthog-checkin
+- All "diff" is uncommitted changes (no commits yet on top of main)
+
+## Changed Files Summary
+
+- M src/main/ipc/worktrees.ts
+- M src/renderer/src/components/sidebar/AddRepoDialog.tsx
+- M src/shared/telemetry-events.test.ts
+- M src/shared/telemetry-events.ts
+- A docs/onboarding-funnel-telemetry.md (untracked)
+- A src/main/ipc/worktree-create-error-class.ts (untracked)
+- A src/main/ipc/worktree-create-error-class.test.ts (untracked)
+
+## Changed Line Ranges (PR Scope)
+
+<!-- In scope: issues on these lines OR caused by these changes. Out of scope: unrelated pre-existing issues -->
+
+| File | Changed Lines |
+|------|---------------|
+| src/main/ipc/worktrees.ts | 43 (added import), 162-189 (added try/catch + telemetry), 198 (removed line) |
+| src/renderer/src/components/sidebar/AddRepoDialog.tsx | 14 (import), 197, 215, 224-232 (handleSetupStepBack), 258 (onClick rebind), 412 (skip onClick) |
+| src/shared/telemetry-events.ts | 76-102 (new schemas), 217-231 (event schemas), 249, 251 (registry entries) |
+| src/shared/telemetry-events.test.ts | 109-162 (new test blocks) |
+| src/main/ipc/worktree-create-error-class.ts | ALL (new file, 1-44) |
+| src/main/ipc/worktree-create-error-class.test.ts | ALL (new file, 1-51) |
+| docs/onboarding-funnel-telemetry.md | ALL (new file) |
+
+## Review Standards Reference
+
+- Follow /review-code standards
+- Focus on: correctness, security, performance, maintainability
+- Priority levels: Critical > High > Medium > Low
+
+## File Categories
+
+### Backend/IPC
+- src/main/ipc/worktrees.ts
+- src/main/ipc/worktree-create-error-class.ts
+- src/main/ipc/worktree-create-error-class.test.ts
+
+### Frontend/UI
+- src/renderer/src/components/sidebar/AddRepoDialog.tsx
+
+### Utility/Common
+- src/shared/telemetry-events.ts
+- src/shared/telemetry-events.test.ts
+
+### Docs (skipped from code review; reviewed only as context)
+- docs/onboarding-funnel-telemetry.md
+
+## Skipped Issues (Do Not Re-validate)
+
+- worktree-create-error-class.ts:17-20 | Low | Hypothetical Buffer-stderr handling. Current throws all use bare Error('...') without .stderr; existing code does not need this defensive code per "don't add validation for scenarios that can't happen". | Buffer stderr handling missing
+- worktrees.ts:168 | Low | Optional comment about future single-caller assumption. Not a correctness issue. | Comment about future caller of worktrees:create
+- AddRepoDialog.tsx:411-414 | Low | Inline arrow onSkip is purely consistency nit; no functional impact. | Inline onSkip arrow not memoized
+- AddRepoDialog.tsx:411-414, :197, :215, :228-231 | Low | Double-click guards: track is fire-and-forget; button unmounts on close; impact bounded; precedent (appOpenedTrackedThisSession) is only for useEffect-driven emits. | Skip/Open/Configure/Back double-click guards
+- telemetry-events.test.ts:115 | Low | Test loop type (use addRepoSetupStepActionSchema.options); maintainability nit only — test still runtime-fails on typo. | Test loop should use schema.options
+- telemetry-events.test.ts:137-165 | Low | Missing source enum rejection test; redundant with workspace_created coverage of same schema. | Missing source enum test
+- telemetry-events.ts:87 | Low | Unused export AddRepoSetupStepAction; consistent with peer-type exports (RepoMethod, OptInVia). No fix needed per agent. | Unused type export
+- worktrees.ts:168 | Low | Optional comment about emit placement vs pre-validation throws. | Optional emit-placement comment
+
+## Issues Validated as False Positive
+
+- worktrees.ts:182 (Codex): track() in catch wrapping concern. track() does not throw — `validate()` returns a result object, posthog.capture is internally guarded, all early-returns inside track() prevent throws. Existing pattern across the codebase wraps no track() call. FP.
+
+## Issues to Fix (Phase 3)
+
+1. **High** worktree-create-error-class.ts:18-43 — Case-sensitivity gap: `text.includes('worktree')` is lowercase but real throw site at worktree-remote.ts:203,441 is `'Worktree created but not found in listing'` (capital W). Found by: Claude (×2). Fix: lowercase-normalize the text once before substring checks.
+
+2. **Medium** worktree-create-error-class.ts:40 — Overly broad `'git '` and `'worktree'` substrings cause SSH relay errors (worktree-remote.ts:190, "The SSH relay has not registered the worktree path yet") to bucket into `git_failed` instead of `unknown`. Found by: Claude (×2). Fix: replace broad anchors with specific markers (`'fatal:'`, `'git worktree'`, `'created but not found'`, `'no git provider'`).
+
+3. **Medium** worktree-create-error-class.test.ts:1-4 — Header comment over-promises: claims to "fixture the actual throw sites" but only covers 3 of ~10 throw sites in worktree-remote.ts. Found by: Claude. Fix: adjust the comment to honestly describe spot-coverage of load-bearing throws.
+
+## Iteration State
+
+Current iteration: 1
+Last completed phase: Validation
+Files fixed this iteration: []
@@ -0,0 +1,174 @@
+# Onboarding Funnel Telemetry
+
+## Goal
+
+Identify where new users drop off between launching Orca and getting an agent running, with enough granularity to fix it. Today the funnel is `app_opened → repo_added → workspace_created → agent_started`, which collapses ~10 distinct UI surfaces into 4 visible signals. About 60% of users who add a repo never create a workspace (PostHog funnel: `<insight ID / query name TBD>`, measured `<window TBD>` — citation pending), and we have no data on which of the intermediate surfaces is responsible.
+
+This document proposes the minimum set of events that, together with what already exists, gives the funnel no invisible drop-off points.
+
+## System context
+
+Every existing `track()` call site lives in main. New events that originate in the renderer cross the IPC boundary through a bridge that already ships: preload `telemetryTrack` (`src/preload/index.ts:803-815`) → IPC `telemetry:track` handler (`src/main/ipc/telemetry.ts:102-123`) → existing `track()`, with the typed renderer wrapper at `src/renderer/src/lib/telemetry.ts:50-65`. The main-side handler is the single point where the consent gate, burst-cap, and Zod validator run (`client.ts:256-310`).
+
+```
+┌──────────┐  onClick / mount    ┌──────────────────┐  telemetry:track   ┌───────────────────┐  capture()  ┌─────────┐
+│ Renderer │ ──────────────────► │ preload bridge   │ ─────────────────► │ main: track()     │ ──────────► │ PostHog │
+│ (React)  │                     │ window.api.tele- │                    │  consent + cap +  │             │         │
+│          │                     │ metry.track      │                    │  Zod validate     │             │         │
+└──────────┘                     └──────────────────┘                    └───────────────────┘             └─────────┘
+```
+
+The renderer is untrusted relative to telemetry payloads. Names and properties crossing the bridge are revalidated by the same Zod schema used for in-process emits; malformed entries are dropped silently to match the existing validator behavior.
+
+## Non-goals
+
+- Not adding per-keystroke instrumentation. Composer-form abandonment is captured by "composer opened, no workspace_created."
+- Not tracking which agent or repo a user picked at this layer — that lives on `workspace_created` / `agent_started` properties already.
+- Not changing the privacy contract. Every new event respects the consent gate at `client.ts:283-286`, attaches no PII, and routes through the same validator.
+
+## The path being measured
+
+For a new user with no repos, the observable journey is:
+
+```
+1.  app launches                                                    [client.ts:434, app_opened]
+2.  Landing screen                                                  [Landing.tsx]
+3.  click "Add Project" → AddRepoDialog opens
+4.  pick entry method: Browse / Clone / Remote / Create             [AddRepoDialog.tsx:31, step state]
+5.  sub-step (clone/remote/create) succeeds OR errors
+6.  AddRepoDialog Setup step shown                                  [AddRepoSetupStep.tsx]
+        repo_added fires here                                       [repos.ts:49]
+7.  Setup step action: Create worktree / Open existing / Configure / Skip / Back
+8.  NewWorkspaceComposer opens                                      [NewWorkspaceComposerCard.tsx]
+9.  user fills form, submits
+10. worktrees:create IPC succeeds OR errors                         [worktrees.ts:150]
+        workspace_created fires on success                          [worktrees.ts:176]
+11. terminal opens, agent process spawns
+        agent_started fires                                         [pty.ts:1084]
+```
+
+Steps 1, 6, 10, 11 emit events today. Steps 2-5, 7-9 are invisible.
+
+## Proposed events
+
+Maximum-value-first. Two events ship now because they each close a question that is currently impossible to answer; the rest are deferred behind a single generic `funnel_step` event so the registry doesn't grow one entry per UI surface. "Load-bearing" means the event closes an invisible-today question; "high"/"medium" calibrate marginal value once the load-bearing pair is live.
+
+### First ship
+
+#### 1. `add_repo_setup_step_action` — **load-bearing**
+
+**Fires:** when the user picks an action from `AddRepoSetupStep` (or backs out via the dialog's "Add another project" arrow). The five actions split across two emit sites:
+
+| Action | Has main-process IPC seam? | Emit site |
+|---|---|---|
+| `create_worktree` | yes (`worktrees:create`) | main-side, at the IPC handler entry |
+| `open_existing` | no (renderer-only `activateAndRevealWorktree`, `AddRepoDialog.tsx:194-200`) | renderer-side `onClick`, via the IPC bridge |
+| `configure` | no (opens a renderer-only modal) | renderer-side `onClick`, via the IPC bridge |
+| `skip` | no (pure UI dismiss) | renderer-side `onClick`, via the IPC bridge |
+| `back` | no (returns to entry-method picker, `AddRepoDialog.tsx:243-251`) | renderer-side `onClick`, via the IPC bridge |
+
+Only `create_worktree` has a main-process seam; the other four are pure-UI transitions and emit from the renderer because there is nowhere else to put them. This is a deliberate departure from "all telemetry lives in main" — requiring a main-side trace for Skip/Configure/Back/open_existing would mean inventing IPC calls that exist only to carry telemetry.
+
+**Properties:** `action: 'create_worktree' | 'configure' | 'skip' | 'open_existing' | 'back'`
+**Tells us:** what fraction of users who add a repo immediately bail vs continue to workspace creation. This is almost certainly the biggest drop in the existing `repo_added → workspace_created` step (the ~60% drop cited in Goal). Without this, that drop is a single black box; with it, we can split "user hit Skip" (a UX problem we can fix) from "user opened the composer and abandoned mid-form" (a different UX problem) from "user opened composer and the create call failed" (a bug).
+**Strength:** high. The action enum has five discrete values that map 1:1 to affordances on a single screen. No interpretation needed.
+**Cost:** ~7 lines across the five call sites. The renderer→main bridge already exists (see "System context") — no new infra.
+
+**Caveat:** folder-mode (non-git) repos auto-close the dialog at `AddRepoDialog.tsx:131-134` and never reach the Setup step, so they emit `repo_added` with no follow-on `add_repo_setup_step_action`. Funnel queries on `repo_added → add_repo_setup_step_action` should expect a legitimate drop here, not a failure to instrument.
+
+#### 2. `workspace_create_failed` — **load-bearing**
+
+**Fires:** when `worktrees:create` throws inside `createLocalWorktree` / `createRemoteWorktree`.
+**Properties:** `source: WorkspaceSource`, `error_class: 'git_failed' | 'path_collision' | 'permission_denied' | 'base_ref_missing' | 'unknown'`.
+**Tells us:** how many of the missing `workspace_created` events are user abandonment vs system failure. Today, the comment at `worktrees.ts:166-173` explicitly notes that `workspace_created` only fires after the helpers resolve — a thrown helper is silent. That silence is fine for the success-rate denominator on creation, but it leaves the failure rate completely unobservable.
+**Strength:** high. Errors thrown by a single function with a known taxonomy. Caveat: if a future refactor adds a new throw site, the event needs to be wired there too — same maintenance burden as `agent_error`.
+
+The `error_class` values are intentionally a separate enum from `agent_error.error_class` (`['binary_not_found', 'unknown']`). Different domain — `agent_error` covers PTY-spawn failures, this covers git/filesystem failures during worktree creation. The schema-evolution comment at `telemetry-events.ts:68-69` warns against speculatively widening enums across domains; merging them would lock both domains to the union forever.
+
+**Cost:** wrap *only* the `await createLocalWorktree(...)` / `await createRemoteWorktree(...)` calls in a try/catch (`worktrees.ts:162-164`), classify the error, fire the event, rethrow. ~10 lines. The pre-validation throws above those calls (`Repo not found`, `Folder mode does not support creating worktrees` at `worktrees.ts:154-159`) are intentionally NOT instrumented — they signal IPC-shape bugs, not the user-visible git/filesystem failures the funnel cares about, and would otherwise bucket into `unknown` and pollute the failure taxonomy.
+
+### The renderer→main bridge
+
+Event #1 is the first *funnel* event to fire from a React `onClick`, but the bridge it rides on already exists and is reused as-is — no new infra:
+
+- **Preload exposure: `window.api.telemetryTrack(name, props)`** at `src/preload/index.ts:803-815`, mapped to the `telemetry:track` IPC channel.
+- **Main-side handler at `src/main/ipc/telemetry.ts:102-123`** is a thin shim that narrows `name`/`props` at the boundary then calls the existing `track()`. All three guards — consent gate (`client.ts:283-286`), burst-cap, Zod validator (`client.ts:256-310`) — apply unchanged. The bridge is not a parallel pipeline; it is an entry point into the same one.
+- **Typed renderer wrapper at `src/renderer/src/lib/telemetry.ts:50-65`** gives call sites `EventMap`-based type safety while keeping the preload surface deliberately loose (the validator is the single enforcement point at runtime).
+- **The renderer is untrusted relative to telemetry payloads.** Names and props are revalidated by the registry's `.strict()` Zod schema; unknown event names and unknown/extra properties are dropped silently, matching how the in-process validator already behaves.
+
+The bridge is also reused for any future renderer-originated event, including the deferred `funnel_step` cases below.
+
+### Deferred to `funnel_step`
+
+Events #3-#6 in earlier drafts each had a distinct event name (`add_repo_method_selected`, `add_repo_failed`, `add_repo_dialog_opened`, `workspace_composer_opened`). Shipping four named events for four UI surfaces grows the registry linearly with the funnel and creates four enum-design decisions we'd have to get right up front. Instead, when any of these prove necessary after observing #1+#2 data, fold them into a single generic event:
+
+```
+funnel_step
+  funnel:       'onboarding_v1'
+  step:         enum  // e.g. 'add_repo_dialog_opened',
+                      //      'add_repo_method_selected',
+                      //      'add_repo_failed',
+                      //      'workspace_composer_opened'
+  source?:      enum  // surface-specific, e.g. 'landing' | 'sidebar' | 'composer_no_repo'
+  action?:      enum  // surface-specific, e.g. method picked
+  error_class?: enum  // surface-specific failure taxonomy
+```
+
+Adding a new `step` value is additive-safe per the schema-evolution doctrine in `telemetry-events.ts`. New funnels get a new `funnel` value; the same event carries them.
+
+Sketches of what each deferred case would look like as a `step`:
+
+- `step: 'add_repo_dialog_opened'`, `source: 'landing' | 'sidebar' | 'composer_no_repo'` — splits the longest invisible gap (`app_opened → repo_added`) into "never opened the dialog" vs "opened and abandoned." Diminishing returns once #1+#2 are live.
+- `step: 'add_repo_method_selected'`, `action: 'folder_picker' | 'clone_url' | 'drag_drop' | ...` — visibility into which entry methods users *try* before any succeed. The `action` enum must be unified with `repo_added.method` (`'folder_picker' | 'clone_url' | 'drag_drop'` per `telemetry-events.ts:73`) or explicitly extended with documented superset semantics; otherwise the join `repo_added{method=X} / funnel_step{action=X}` silently breaks.
+- `step: 'add_repo_failed'`, `action: <method>`, `error_class: 'user_cancelled' | 'clone_network_error' | 'git_init_failed' | ...` — paired with the previous step to compute per-method success rates with cancels broken out from infra errors. The `error_class` design is the load-bearing part; conflating cancels with errors muddies both metrics.
+- `step: 'workspace_composer_opened'`, `source: WorkspaceSource` — composer-mid-form abandonment. Mostly redundant with `add_repo_setup_step_action.action='create_worktree'` once #1 ships.
+
+For any "X opened" step (dialog/composer), the firing site is a `useEffect` on a modal's open state, which double-fires under React StrictMode and on remounts. Use a session-scoped dedup analogous to `appOpenedTrackedThisSession` in `client.ts:98` to avoid inflating the numerator.
+
+## What already exists and should not be re-added
+
+- `app_opened` is the session heartbeat. Don't add another.
+- `repo_added` already carries `method` and dedupes re-adds (`repos.ts:41-49`).
+- `workspace_created` already carries `source` and `from_existing_branch`.
+- `agent_started` already carries `agent_kind`, `launch_source`, `request_kind`.
+
+## Properties to keep off these events
+
+In line with the existing schema discipline at `telemetry-events.ts:142-148` (every event uses `.strict()` Zod schemas):
+
+- No file paths, repo URLs, or display names.
+- No clone URLs (already excluded from `repo_added`).
+- No raw error messages — only an `error_class` enum value. The classifier for `workspace_create_failed` reads `error.message` strings (the throw sites in `worktree-remote.ts` are bare `throw new Error('...')`, some interpolating user-controlled content like branch names) to bucket into the enum, but those strings never cross the wire. Widening `error_class` later requires explicit review of the substring match set, not silent regex expansion.
+- No partial form contents (workspace name, branch name, etc.).
+- `value_kind` only on settings-style events, never the value itself.
+
+## Rollout
+
+**Phase 1 (this PR): #1 and #2.** Combined cost ~17 lines across the five Setup-step call sites and the `worktrees:create` try/catch. The renderer→main bridge already ships (see "System context"); no new IPC plumbing. Closes the two largest unknowns: Setup-step drop-off and creation failure rate. Both metrics are immediately actionable — Setup-step splits the ~60% drop into UX vs bug; failed-create gives us the failure-rate denominator we lack today.
+
+**Phase 2 (only if Phase 1 data shows residual unknowns): `funnel_step`.** Land the generic event with whichever `step` values the Phase 1 data actually demands, in priority order:
+1. `add_repo_dialog_opened` if `app_opened → repo_added` is still a black box.
+2. `add_repo_method_selected` + `add_repo_failed` together — `method_selected` without `failed` produces a misleading conversion rate.
+3. `workspace_composer_opened` last; mostly redundant with #1.
+
+Defer indefinitely if Phase 1 answers the questions on its own.
+
+## What Phase 1 gives us
+
+```
+app_opened                                                      (existing)
+  ↓
+repo_added                                                      (existing)
+  ↓
+add_repo_setup_step_action                                      [#1]
+  ↓ (action = create_worktree)
+workspace_created  ──or──►  workspace_create_failed             (existing / [#2])
+  ↓
+agent_started                                                   (existing)
+```
+
+The two transitions that were previously single black boxes (`repo_added → workspace_created` and "missing `workspace_created` events") now have measurable numerators and denominators. Remaining invisible gaps are the pre-`repo_added` surfaces, which Phase 2 addresses if needed.
+
+## Open questions
+
+- **Should we add a `landing_cta_clicked` step under `funnel_step`?** Only if Phase 1 plus a `step: 'add_repo_dialog_opened'` reveals that most `app_opened` sessions never open the AddRepo dialog. Defer.
@@ -0,0 +1,51 @@
+// Pin the classifier's substring set: widening it later requires explicit
+// review per the schema-evolution doctrine in telemetry-events.ts. These
+// cases fixture the actual throw sites in worktree-remote.ts so a refactor
+// that renames the user-facing error strings shows up here as a regression.
+
+import { describe, expect, it } from 'vitest'
+import { classifyWorkspaceCreateError } from './worktree-create-error-class'
+
+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 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('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,44 @@
+// 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 {
+  const message = error instanceof Error ? error.message : ''
+  const stderr = error instanceof Error ? (error as { stderr?: unknown }).stderr : ''
+  const text = `${message} ${typeof stderr === 'string' ? stderr : ''}`
+
+  if (text.includes('Could not resolve a default base ref')) {
+    return 'base_ref_missing'
+  }
+  if (
+    text.includes('already exists') ||
+    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') ||
+    text.includes('Permission denied')
+  ) {
+    return 'permission_denied'
+  }
+  if (text.includes('fatal:') || text.includes('git ') || text.includes('worktree')) {
+    return 'git_failed'
+  }
+  return 'unknown'
+}
@@ -40,6 +40,7 @@ import { getLocalPtyProvider } from './pty'
 import { removeWorktreeSymlinks } from './worktree-symlinks'
 import { track } from '../telemetry/client'
 import { workspaceSourceSchema, type WorkspaceSource } from '../../shared/telemetry-events'
+import { classifyWorkspaceCreateError } from './worktree-create-error-class'

 // Why: worktrees discovered on disk (not created via Orca's UI) have no
 // persisted WorktreeMeta, so mergeWorktree falls back to `lastActivityAt: 0`.
@@ -158,10 +159,34 @@ 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)
+      // Why: the only Setup-step affordance that has a main-process seam to
+      // ride. The other four (skip/configure/open_existing/back) are pure-UI
+      // transitions and emit from the renderer (see AddRepoDialog.tsx).
+      // Firing here covers the load-bearing case where the create request
+      // actually landed — a renderer-side emit on the SetupStep "Create"
+      // click would also count abandoned composer sessions.
+      track('add_repo_setup_step_action', { action: 'create_worktree' })
+
+      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)
+        })
+        throw error
+      }

      // Why: emit `workspace_created` only after the underlying create has
      // resolved (the helpers throw on failure, so reaching this line means
@@ -171,8 +196,6 @@ 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
@@ -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()
    },
@@ -210,6 +212,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,6 +221,15 @@ const AddRepoDialog = React.memo(function AddRepoDialog() {
  // Why: handleBack reuses resetState which already aborts clones and resets all fields.
  const handleBack = 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}
@@ -243,7 +255,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
@@ -397,6 +409,7 @@ const AddRepoDialog = React.memo(function AddRepoDialog() {
            onCreateWorktree={handleCreateWorktree}
            onConfigureRepo={handleConfigureRepo}
            onSkip={() => {
+              track('add_repo_setup_step_action', { action: 'skip' })
              closeModal()
              resetState()
            }}
@@ -110,6 +110,60 @@ describe('agent_started schema', () => {
  })
 })

+describe('add_repo_setup_step_action schema', () => {
+  it('accepts each of the five Setup-step actions', () => {
+    for (const action of ['create_worktree', 'configure', 'skip', 'open_existing', 'back']) {
+      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)
+  })
+})
+
 describe('settings_changed schema', () => {
  it('accepts whitelisted setting keys', () => {
    for (const key of SETTINGS_CHANGED_WHITELIST) {
@@ -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',
@@ -187,6 +214,21 @@ const settingsChangedSchema = z
 const telemetryOptedInSchema = z.object({ via: optInViaSchema }).strict()
 const telemetryOptedOutSchema = z.object({ via: optInViaSchema }).strict()

+const addRepoSetupStepActionEventSchema = z
+  .object({ action: addRepoSetupStepActionSchema })
+  .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
+  })
+  .strict()
+
 // ── Event registry: the one record the validator consumes ───────────────
 //
 // The validator does `eventSchemas[name].safeParse(props)`. `EventMap` is
@@ -204,7 +246,9 @@ export const eventSchemas = {
  app_opened: emptySchema,

  repo_added: repoAddedSchema,
+  add_repo_setup_step_action: addRepoSetupStepActionEventSchema,
  workspace_created: workspaceCreatedSchema,
+  workspace_create_failed: workspaceCreateFailedSchema,

  agent_started: agentStartedSchema,
  agent_error: agentErrorSchema,