feat(telemetry): instrument on_path:false triage on onboarding_agent_picked (#1674)
* feat(telemetry): instrument on_path:false triage on onboarding_agent_picked Adds path_source and path_failure_reason to onboarding_agent_picked so the ~30% on_path:false rate on dashboard 1562016 can be split between shell hydration failures and genuinely-not-on-PATH cases before picking a fix. See docs/agent-on-path-detection.md. Co-authored-by: Orca <help@stably.ai> * fix(telemetry): close PathSource compile-time-sync hole Add `_PathSourceSync` guard mirroring `_PathFailureReasonSync` so adding a new `PathSource` value to the alias without updating the schema (or vice versa) fails the build. Without it, drift would silently drop `onboarding_agent_picked` at the strict validator. Also replace stale line-number references in docs/agent-on-path-detection.md with named function/handler references that survive future edits. Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Brennan Benson
GitHub
parent
c4bb10e1f5
commit
98b15aeabc
|
|
@ -198,7 +198,8 @@ describe('preflight', () => {
|
|||
// (3) re-run `which` so newly-installed CLIs appear without a restart.
|
||||
hydrateShellPathMock.mockResolvedValueOnce({
|
||||
segments: ['/Users/test/.opencode/bin'],
|
||||
ok: true
|
||||
ok: true,
|
||||
failureReason: 'none'
|
||||
})
|
||||
mergePathSegmentsMock.mockReturnValueOnce(['/Users/test/.opencode/bin'])
|
||||
execFileAsyncMock.mockImplementation(async (command, args) => {
|
||||
|
|
@ -217,18 +218,26 @@ describe('preflight', () => {
|
|||
agents: string[]
|
||||
addedPathSegments: string[]
|
||||
shellHydrationOk: boolean
|
||||
pathSource: string
|
||||
pathFailureReason: string
|
||||
}
|
||||
|
||||
expect(result).toEqual({
|
||||
agents: ['opencode'],
|
||||
addedPathSegments: ['/Users/test/.opencode/bin'],
|
||||
shellHydrationOk: true
|
||||
shellHydrationOk: true,
|
||||
pathSource: 'shell_hydrate',
|
||||
pathFailureReason: 'none'
|
||||
})
|
||||
expect(hydrateShellPathMock).toHaveBeenCalledWith({ force: true })
|
||||
})
|
||||
|
||||
it('still re-detects when the shell spawn fails — relies on the existing PATH', async () => {
|
||||
hydrateShellPathMock.mockResolvedValueOnce({ segments: [], ok: false })
|
||||
hydrateShellPathMock.mockResolvedValueOnce({
|
||||
segments: [],
|
||||
ok: false,
|
||||
failureReason: 'timeout'
|
||||
})
|
||||
execFileAsyncMock.mockImplementation(async (command, args) => {
|
||||
if (command !== 'which') {
|
||||
throw new Error(`unexpected command ${String(command)}`)
|
||||
|
|
@ -245,13 +254,38 @@ describe('preflight', () => {
|
|||
agents: string[]
|
||||
addedPathSegments: string[]
|
||||
shellHydrationOk: boolean
|
||||
pathSource: string
|
||||
pathFailureReason: string
|
||||
}
|
||||
|
||||
expect(result.shellHydrationOk).toBe(false)
|
||||
expect(result.addedPathSegments).toEqual([])
|
||||
expect(result.agents).toEqual(['claude'])
|
||||
// Why: drives the agent_picks `on_path:false` triage in dashboard 1562016.
|
||||
// Without these fields we cannot distinguish "hydration failed" from
|
||||
// "user genuinely doesn't have the binary."
|
||||
expect(result.pathSource).toBe('sync_seed_only')
|
||||
expect(result.pathFailureReason).toBe('timeout')
|
||||
// Why: when hydration fails, we must not call merge — nothing to merge —
|
||||
// otherwise we'd log a no-op "added 0 segments" event on every refresh.
|
||||
expect(mergePathSegmentsMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it.each(['no_shell', 'spawn_error', 'empty_path'] as const)(
|
||||
'classifies pathFailureReason=%s when hydration reports it',
|
||||
async (failureReason) => {
|
||||
hydrateShellPathMock.mockResolvedValueOnce({ segments: [], ok: false, failureReason })
|
||||
execFileAsyncMock.mockRejectedValue(new Error('not found'))
|
||||
|
||||
registerPreflightHandlers()
|
||||
|
||||
const result = (await handlers['preflight:refreshAgents']()) as {
|
||||
pathSource: string
|
||||
pathFailureReason: string
|
||||
}
|
||||
|
||||
expect(result.pathSource).toBe('sync_seed_only')
|
||||
expect(result.pathFailureReason).toBe(failureReason)
|
||||
}
|
||||
)
|
||||
})
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ import { execFile } from 'child_process'
|
|||
import { promisify } from 'util'
|
||||
import path from 'path'
|
||||
import { TUI_AGENT_CONFIG } from '../../shared/tui-agent-config'
|
||||
import type { PathSource, ShellHydrationFailureReason } from '../../shared/types'
|
||||
import { hydrateShellPath, mergePathSegments } from '../startup/hydrate-shell-path'
|
||||
import { getActiveMultiplexer } from './ssh'
|
||||
|
||||
|
|
@ -69,6 +70,14 @@ export type RefreshAgentsResult = {
|
|||
addedPathSegments: string[]
|
||||
/** True when the shell spawn succeeded. False = relied on existing PATH. */
|
||||
shellHydrationOk: boolean
|
||||
/** Whether `detectInstalledAgents` ran against shell-hydrated PATH or only
|
||||
* the seed list from `patchPackagedProcessPath`. Drives the on_path:false
|
||||
* triage in tile A on dashboard 1562016. */
|
||||
pathSource: PathSource
|
||||
/** Why hydration failed (or `'none'` on success). Typed against the shared
|
||||
* alias so the IPC boundary stays in lockstep with the renderer-visible
|
||||
* enum on `onboardingAgentPickedSchema`. */
|
||||
pathFailureReason: ShellHydrationFailureReason
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -84,7 +93,9 @@ export async function refreshShellPathAndDetectAgents(): Promise<RefreshAgentsRe
|
|||
return {
|
||||
agents,
|
||||
addedPathSegments: added,
|
||||
shellHydrationOk: hydration.ok
|
||||
shellHydrationOk: hydration.ok,
|
||||
pathSource: hydration.ok ? 'shell_hydrate' : 'sync_seed_only',
|
||||
pathFailureReason: hydration.failureReason
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -2,9 +2,12 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest'
|
|||
import {
|
||||
_resetHydrateShellPathCache,
|
||||
hydrateShellPath,
|
||||
mergePathSegments
|
||||
mergePathSegments,
|
||||
type HydrationResult
|
||||
} from './hydrate-shell-path'
|
||||
|
||||
type HydrationSpawner = (shell: string) => Promise<HydrationResult>
|
||||
|
||||
describe('hydrateShellPath', () => {
|
||||
const originalPath = process.env.PATH
|
||||
|
||||
|
|
@ -28,7 +31,8 @@ describe('hydrateShellPath', () => {
|
|||
capturedShell = shell
|
||||
return {
|
||||
segments: ['/Users/tester/.opencode/bin', '/Users/tester/.cargo/bin'],
|
||||
ok: true
|
||||
ok: true,
|
||||
failureReason: 'none'
|
||||
}
|
||||
}
|
||||
})
|
||||
|
|
@ -36,13 +40,14 @@ describe('hydrateShellPath', () => {
|
|||
expect(capturedShell).toBe('/bin/zsh')
|
||||
expect(result.ok).toBe(true)
|
||||
expect(result.segments).toEqual(['/Users/tester/.opencode/bin', '/Users/tester/.cargo/bin'])
|
||||
expect(result.failureReason).toBe('none')
|
||||
})
|
||||
|
||||
it('caches the hydration result so repeated calls do not re-spawn', async () => {
|
||||
let spawnCount = 0
|
||||
const spawner = async (): Promise<{ segments: string[]; ok: boolean }> => {
|
||||
const spawner: HydrationSpawner = async () => {
|
||||
spawnCount += 1
|
||||
return { segments: ['/a'], ok: true }
|
||||
return { segments: ['/a'], ok: true, failureReason: 'none' }
|
||||
}
|
||||
|
||||
await hydrateShellPath({ shellOverride: '/bin/zsh', spawner })
|
||||
|
|
@ -54,9 +59,9 @@ describe('hydrateShellPath', () => {
|
|||
|
||||
it('re-spawns when force:true is passed — matches the Refresh button contract', async () => {
|
||||
let spawnCount = 0
|
||||
const spawner = async (): Promise<{ segments: string[]; ok: boolean }> => {
|
||||
const spawner: HydrationSpawner = async () => {
|
||||
spawnCount += 1
|
||||
return { segments: ['/a'], ok: true }
|
||||
return { segments: ['/a'], ok: true, failureReason: 'none' }
|
||||
}
|
||||
|
||||
await hydrateShellPath({ shellOverride: '/bin/zsh', spawner })
|
||||
|
|
@ -65,7 +70,7 @@ describe('hydrateShellPath', () => {
|
|||
expect(spawnCount).toBe(2)
|
||||
})
|
||||
|
||||
it('returns ok:false when no shell is available (Windows path)', async () => {
|
||||
it('returns failureReason:no_shell when no shell is available (Windows path)', async () => {
|
||||
const result = await hydrateShellPath({
|
||||
shellOverride: null,
|
||||
spawner: async () => {
|
||||
|
|
@ -73,7 +78,36 @@ describe('hydrateShellPath', () => {
|
|||
}
|
||||
})
|
||||
|
||||
expect(result).toEqual({ segments: [], ok: false })
|
||||
expect(result).toEqual({ segments: [], ok: false, failureReason: 'no_shell' })
|
||||
})
|
||||
|
||||
// Why: each failure mode tagged independently so dashboards can pick the
|
||||
// right fix (lengthen timeout vs investigate shell-invocation strategy vs
|
||||
// surface a UX error). Spawner override stands in for the four resolve
|
||||
// sites — the actual classification happens inside `spawnShellAndReadPath`,
|
||||
// covered by the existing real-shell smoke surface.
|
||||
it('propagates failureReason:timeout from the spawner', async () => {
|
||||
const result = await hydrateShellPath({
|
||||
shellOverride: '/bin/zsh',
|
||||
spawner: async () => ({ segments: [], ok: false, failureReason: 'timeout' })
|
||||
})
|
||||
expect(result).toEqual({ segments: [], ok: false, failureReason: 'timeout' })
|
||||
})
|
||||
|
||||
it('propagates failureReason:spawn_error from the spawner', async () => {
|
||||
const result = await hydrateShellPath({
|
||||
shellOverride: '/bin/zsh',
|
||||
spawner: async () => ({ segments: [], ok: false, failureReason: 'spawn_error' })
|
||||
})
|
||||
expect(result).toEqual({ segments: [], ok: false, failureReason: 'spawn_error' })
|
||||
})
|
||||
|
||||
it('propagates failureReason:empty_path from the spawner', async () => {
|
||||
const result = await hydrateShellPath({
|
||||
shellOverride: '/bin/zsh',
|
||||
spawner: async () => ({ segments: [], ok: false, failureReason: 'empty_path' })
|
||||
})
|
||||
expect(result).toEqual({ segments: [], ok: false, failureReason: 'empty_path' })
|
||||
})
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import { spawn } from 'child_process'
|
||||
import { delimiter } from 'path'
|
||||
import type { ShellHydrationFailureReason } from '../../shared/types'
|
||||
|
||||
// Why: GUI-launched Electron on macOS/Linux inherits a minimal PATH from launchd
|
||||
// that does not include dirs appended by the user's shell rc files (~/.zshrc,
|
||||
|
|
@ -20,12 +21,17 @@ const SPAWN_TIMEOUT_MS = 5000
|
|||
// files print banners or set colored prompts. Strip them before parsing.
|
||||
const ANSI_RE = /\x1b\[[0-9;?]*[A-Za-z]/g // eslint-disable-line no-control-regex
|
||||
|
||||
type HydrationResult = {
|
||||
/** PATH segments extracted from the login shell, in order, de-duplicated. */
|
||||
segments: string[]
|
||||
/** True when the shell spawn succeeded and returned a non-empty PATH. */
|
||||
ok: boolean
|
||||
}
|
||||
// Why: the discriminator lets telemetry classify *why* hydration failed, not
|
||||
// just whether it did. Five resolve sites in this file each tag the result
|
||||
// with the right reason. The shared alias keeps the enum in lockstep with the
|
||||
// telemetry schema (compile-time guard in telemetry-events.ts).
|
||||
export type HydrationResult =
|
||||
| { ok: true; segments: string[]; failureReason: 'none' }
|
||||
| {
|
||||
ok: false
|
||||
segments: []
|
||||
failureReason: Exclude<ShellHydrationFailureReason, 'none'>
|
||||
}
|
||||
|
||||
let cached: Promise<HydrationResult> | null = null
|
||||
|
||||
|
|
@ -104,7 +110,7 @@ function spawnShellAndReadPath(shell: string): Promise<HydrationResult> {
|
|||
} catch {
|
||||
// ignore
|
||||
}
|
||||
resolve({ segments: [], ok: false })
|
||||
resolve({ segments: [], ok: false, failureReason: 'timeout' })
|
||||
}, SPAWN_TIMEOUT_MS)
|
||||
|
||||
child.stdout.on('data', (chunk: Buffer) => {
|
||||
|
|
@ -117,7 +123,7 @@ function spawnShellAndReadPath(shell: string): Promise<HydrationResult> {
|
|||
}
|
||||
finished = true
|
||||
clearTimeout(timer)
|
||||
resolve({ segments: [], ok: false })
|
||||
resolve({ segments: [], ok: false, failureReason: 'spawn_error' })
|
||||
})
|
||||
|
||||
child.on('close', () => {
|
||||
|
|
@ -127,7 +133,11 @@ function spawnShellAndReadPath(shell: string): Promise<HydrationResult> {
|
|||
finished = true
|
||||
clearTimeout(timer)
|
||||
const segments = parseCapturedPath(stdout)
|
||||
resolve({ segments, ok: segments.length > 0 })
|
||||
if (segments.length === 0) {
|
||||
resolve({ segments: [], ok: false, failureReason: 'empty_path' })
|
||||
return
|
||||
}
|
||||
resolve({ segments, ok: true, failureReason: 'none' })
|
||||
})
|
||||
})
|
||||
}
|
||||
|
|
@ -154,7 +164,7 @@ export function hydrateShellPath(options: HydrateOptions = {}): Promise<Hydratio
|
|||
if (!shell) {
|
||||
// Windows uses cmd/PowerShell rather than a POSIX login shell — the
|
||||
// `patchPackagedProcessPath` static list is sufficient there.
|
||||
cached = Promise.resolve({ segments: [], ok: false })
|
||||
cached = Promise.resolve({ segments: [], ok: false, failureReason: 'no_shell' })
|
||||
return cached
|
||||
}
|
||||
cached = (options.spawner ?? spawnShellAndReadPath)(shell)
|
||||
|
|
|
|||
|
|
@ -189,6 +189,57 @@ describe('validate', () => {
|
|||
expect(result.ok).toBe(false)
|
||||
})
|
||||
|
||||
it('accepts onboarding_agent_picked with path_source and path_failure_reason', () => {
|
||||
// Why: the on_path:false triage instrumentation —
|
||||
// see docs/agent-on-path-detection.md.
|
||||
const result = validate('onboarding_agent_picked', {
|
||||
agent_kind: 'claude-code',
|
||||
on_path: false,
|
||||
detected_count: 0,
|
||||
detection_state: 'complete',
|
||||
from_collapsed_section: false,
|
||||
path_source: 'sync_seed_only',
|
||||
path_failure_reason: 'timeout'
|
||||
})
|
||||
expect(result.ok).toBe(true)
|
||||
})
|
||||
|
||||
it('accepts onboarding_agent_picked without the new optional path fields', () => {
|
||||
// Pre-deploy events validate cleanly under `.optional()`.
|
||||
const result = validate('onboarding_agent_picked', {
|
||||
agent_kind: 'codex',
|
||||
on_path: true,
|
||||
detected_count: 2,
|
||||
detection_state: 'complete',
|
||||
from_collapsed_section: false
|
||||
})
|
||||
expect(result.ok).toBe(true)
|
||||
})
|
||||
|
||||
it('rejects onboarding_agent_picked with unknown path_source', () => {
|
||||
const result = validate('onboarding_agent_picked', {
|
||||
agent_kind: 'claude-code',
|
||||
on_path: true,
|
||||
detected_count: 1,
|
||||
detection_state: 'complete',
|
||||
from_collapsed_section: false,
|
||||
path_source: 'env_path'
|
||||
} as never)
|
||||
expect(result.ok).toBe(false)
|
||||
})
|
||||
|
||||
it('rejects onboarding_agent_picked with unknown path_failure_reason', () => {
|
||||
const result = validate('onboarding_agent_picked', {
|
||||
agent_kind: 'claude-code',
|
||||
on_path: true,
|
||||
detected_count: 1,
|
||||
detection_state: 'complete',
|
||||
from_collapsed_section: false,
|
||||
path_failure_reason: 'parse_failed'
|
||||
} as never)
|
||||
expect(result.ok).toBe(false)
|
||||
})
|
||||
|
||||
it('accepts onboarding_ghostty_discovered with field_group_count_bucket', () => {
|
||||
const result = validate('onboarding_ghostty_discovered', {
|
||||
state: 'found',
|
||||
|
|
|
|||
|
|
@ -49,11 +49,13 @@ import type {
|
|||
NotificationSoundResult,
|
||||
OnboardingState,
|
||||
OrcaHooks,
|
||||
PathSource,
|
||||
PersistedUIState,
|
||||
PRCheckDetail,
|
||||
PRComment,
|
||||
PRInfo,
|
||||
Repo,
|
||||
ShellHydrationFailureReason,
|
||||
SparsePreset,
|
||||
SearchOptions,
|
||||
SearchResult,
|
||||
|
|
@ -250,6 +252,15 @@ export type RefreshAgentsResult = {
|
|||
agents: string[]
|
||||
addedPathSegments: string[]
|
||||
shellHydrationOk: boolean
|
||||
/** Why: drives the agent_picks `on_path:false` triage in dashboard 1562016
|
||||
* (insight A). `'shell_hydrate'` = detection saw the user's full shell PATH;
|
||||
* `'sync_seed_only'` = hydration failed and detection ran against the
|
||||
* seed list from `patchPackagedProcessPath`. */
|
||||
pathSource: PathSource
|
||||
/** Why: classified hydration outcome. `'none'` on success; one of the failure
|
||||
* modes when `shellHydrationOk` is false. Typed off the shared alias so
|
||||
* schema/main/preload/renderer stay in lockstep. */
|
||||
pathFailureReason: ShellHydrationFailureReason
|
||||
}
|
||||
|
||||
export type PreflightApi = {
|
||||
|
|
|
|||
|
|
@ -72,6 +72,7 @@ import type {
|
|||
} from '../shared/ssh-types'
|
||||
import type { AgentStatusState } from '../shared/agent-status-types'
|
||||
import type { TelemetryConsentState } from '../shared/telemetry-consent-types'
|
||||
import type { RefreshAgentsResult } from './api-types'
|
||||
import type { AgentKind, LaunchSource, RequestKind } from '../shared/telemetry-events'
|
||||
import {
|
||||
ORCA_EDITOR_SAVE_DIRTY_FILES_EVENT,
|
||||
|
|
@ -941,11 +942,8 @@ const api = {
|
|||
linear: { connected: boolean }
|
||||
}> => ipcRenderer.invoke('preflight:check', args),
|
||||
detectAgents: (): Promise<string[]> => ipcRenderer.invoke('preflight:detectAgents'),
|
||||
refreshAgents: (): Promise<{
|
||||
agents: string[]
|
||||
addedPathSegments: string[]
|
||||
shellHydrationOk: boolean
|
||||
}> => ipcRenderer.invoke('preflight:refreshAgents'),
|
||||
refreshAgents: (): Promise<RefreshAgentsResult> =>
|
||||
ipcRenderer.invoke('preflight:refreshAgents'),
|
||||
detectRemoteAgents: (args: { connectionId: string }): Promise<string[]> =>
|
||||
ipcRenderer.invoke('preflight:detectRemoteAgents', args)
|
||||
},
|
||||
|
|
|
|||
|
|
@ -0,0 +1,98 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
import { buildAgentPickedPayload } from './agent-picked-payload'
|
||||
|
||||
// Why: this test guards the renderer-end attachment of `path_source` and
|
||||
// `path_failure_reason` to `onboarding_agent_picked`. Without it the entire
|
||||
// instrument-first plan in docs/agent-on-path-detection.md can ship dark for
|
||||
// two weeks before a dashboard read shows the fields are null-only.
|
||||
|
||||
describe('buildAgentPickedPayload', () => {
|
||||
it('attaches path_source and path_failure_reason from the store snapshot', () => {
|
||||
const payload = buildAgentPickedPayload({
|
||||
agent: 'claude',
|
||||
detectedAgentIds: ['claude'],
|
||||
isDetecting: false,
|
||||
fromCollapsedSection: false,
|
||||
pathSource: 'sync_seed_only',
|
||||
pathFailureReason: 'timeout'
|
||||
})
|
||||
|
||||
expect(payload).toEqual({
|
||||
agent_kind: 'claude-code',
|
||||
on_path: true,
|
||||
detected_count: 1,
|
||||
detection_state: 'complete',
|
||||
from_collapsed_section: false,
|
||||
path_source: 'sync_seed_only',
|
||||
path_failure_reason: 'timeout'
|
||||
})
|
||||
})
|
||||
|
||||
it('reports on_path:false and the seed-only source for the headline triage case', () => {
|
||||
// Why: this is the dominant row the instrumentation exists to interpret —
|
||||
// a claude-code pick that read on_path:false because hydration failed,
|
||||
// not because the user is missing the binary.
|
||||
const payload = buildAgentPickedPayload({
|
||||
agent: 'claude',
|
||||
detectedAgentIds: [],
|
||||
isDetecting: false,
|
||||
fromCollapsedSection: false,
|
||||
pathSource: 'sync_seed_only',
|
||||
pathFailureReason: 'empty_path'
|
||||
})
|
||||
|
||||
expect(payload.on_path).toBe(false)
|
||||
expect(payload.path_source).toBe('sync_seed_only')
|
||||
expect(payload.path_failure_reason).toBe('empty_path')
|
||||
})
|
||||
|
||||
it('omits the path fields when the store snapshot has not resolved yet', () => {
|
||||
// Why: pre-refresh clicks must NOT emit `path_source: null`. The schema
|
||||
// declares both fields as `.optional()`, so a literal `null` would fail
|
||||
// `.strict()` validation and drop the entire event.
|
||||
const payload = buildAgentPickedPayload({
|
||||
agent: 'codex',
|
||||
detectedAgentIds: ['codex'],
|
||||
isDetecting: true,
|
||||
fromCollapsedSection: false,
|
||||
pathSource: null,
|
||||
pathFailureReason: null
|
||||
})
|
||||
|
||||
expect(payload).toEqual({
|
||||
agent_kind: 'codex',
|
||||
on_path: true,
|
||||
detected_count: 1,
|
||||
detection_state: 'pending',
|
||||
from_collapsed_section: false
|
||||
})
|
||||
expect('path_source' in payload).toBe(false)
|
||||
expect('path_failure_reason' in payload).toBe(false)
|
||||
})
|
||||
|
||||
it('reports detection_state=pending while the refresh is in flight', () => {
|
||||
const payload = buildAgentPickedPayload({
|
||||
agent: 'codex',
|
||||
detectedAgentIds: [],
|
||||
isDetecting: true,
|
||||
fromCollapsedSection: false,
|
||||
pathSource: 'shell_hydrate',
|
||||
pathFailureReason: 'none'
|
||||
})
|
||||
|
||||
expect(payload.detection_state).toBe('pending')
|
||||
})
|
||||
|
||||
it('forwards from_collapsed_section verbatim', () => {
|
||||
const payload = buildAgentPickedPayload({
|
||||
agent: 'aider',
|
||||
detectedAgentIds: [],
|
||||
isDetecting: false,
|
||||
fromCollapsedSection: true,
|
||||
pathSource: 'shell_hydrate',
|
||||
pathFailureReason: 'none'
|
||||
})
|
||||
|
||||
expect(payload.from_collapsed_section).toBe(true)
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,37 @@
|
|||
// Why: extracted as a pure helper so the renderer-end attachment guarantee
|
||||
// (use-onboarding-flow reads `pathSource` / `pathFailureReason` from the
|
||||
// store and forwards them on `onboarding_agent_picked`) is unit-testable
|
||||
// without a React rendering harness. Without this isolation, the entire
|
||||
// instrument-first plan in docs/agent-on-path-detection.md can ship dark
|
||||
// for two weeks before a dashboard read shows the fields are null-only.
|
||||
|
||||
import { tuiAgentToAgentKind } from '@/lib/telemetry'
|
||||
import type { EventProps } from '../../../../shared/telemetry-events'
|
||||
import type { PathSource, ShellHydrationFailureReason, TuiAgent } from '../../../../shared/types'
|
||||
|
||||
export type AgentPickedSnapshot = {
|
||||
agent: TuiAgent
|
||||
detectedAgentIds: readonly TuiAgent[]
|
||||
isDetecting: boolean
|
||||
fromCollapsedSection: boolean
|
||||
pathSource: PathSource | null
|
||||
pathFailureReason: ShellHydrationFailureReason | null
|
||||
}
|
||||
|
||||
export function buildAgentPickedPayload(
|
||||
snapshot: AgentPickedSnapshot
|
||||
): EventProps<'onboarding_agent_picked'> {
|
||||
return {
|
||||
agent_kind: tuiAgentToAgentKind(snapshot.agent),
|
||||
on_path: snapshot.detectedAgentIds.includes(snapshot.agent),
|
||||
detected_count: snapshot.detectedAgentIds.length,
|
||||
detection_state: snapshot.isDetecting ? 'pending' : 'complete',
|
||||
from_collapsed_section: snapshot.fromCollapsedSection,
|
||||
// Why: omit (not null) when refresh hasn't resolved yet so `.optional()`
|
||||
// validates cleanly under `.strict()` in the main-process schema.
|
||||
...(snapshot.pathSource !== null ? { path_source: snapshot.pathSource } : {}),
|
||||
...(snapshot.pathFailureReason !== null
|
||||
? { path_failure_reason: snapshot.pathFailureReason }
|
||||
: {})
|
||||
}
|
||||
}
|
||||
|
|
@ -5,7 +5,8 @@ import { AGENT_CATALOG } from '@/lib/agent-catalog'
|
|||
import { useAppStore } from '@/store'
|
||||
import { activateAndRevealWorktree } from '@/lib/worktree-activation'
|
||||
import { applyDocumentTheme } from '@/lib/document-theme'
|
||||
import { track, tuiAgentToAgentKind } from '@/lib/telemetry'
|
||||
import { track } from '@/lib/telemetry'
|
||||
import { buildAgentPickedPayload } from './agent-picked-payload'
|
||||
import { isGitRepoKind } from '../../../../shared/repo-kind'
|
||||
import type { GlobalSettings, OnboardingState, TuiAgent } from '../../../../shared/types'
|
||||
import type { NotificationDraft } from './NotificationStep'
|
||||
|
|
@ -26,6 +27,8 @@ export function useOnboardingFlow(
|
|||
const refreshDetectedAgents = useAppStore((s) => s.refreshDetectedAgents)
|
||||
const detectedAgentIds = useAppStore((s) => s.detectedAgentIds)
|
||||
const isDetectingAgents = useAppStore((s) => s.isDetectingAgents || s.isRefreshingAgents)
|
||||
const pathSource = useAppStore((s) => s.pathSource)
|
||||
const pathFailureReason = useAppStore((s) => s.pathFailureReason)
|
||||
const fetchRepos = useAppStore((s) => s.fetchRepos)
|
||||
const fetchWorktrees = useAppStore((s) => s.fetchWorktrees)
|
||||
const openModal = useAppStore((s) => s.openModal)
|
||||
|
|
@ -91,6 +94,11 @@ export function useOnboardingFlow(
|
|||
const detectedAgentIdsRef = useRef<readonly TuiAgent[]>(detectedAgentIds ?? [])
|
||||
const isDetectingRef = useRef<boolean>(isDetectingAgents)
|
||||
const selectedAgentRef = useRef(selectedAgent)
|
||||
// Why: refs let `setSelectedAgentInteractive` (a stable useCallback) read
|
||||
// the freshest hydration classification at click time. Mirrors the
|
||||
// detectedAgentIdsRef / isDetectingRef pattern.
|
||||
const pathSourceRef = useRef(pathSource)
|
||||
const pathFailureReasonRef = useRef(pathFailureReason)
|
||||
useEffect(() => {
|
||||
selectedAgentRef.current = selectedAgent
|
||||
}, [selectedAgent])
|
||||
|
|
@ -105,16 +113,20 @@ export function useOnboardingFlow(
|
|||
return
|
||||
}
|
||||
// Why: emit at click time, not at step completion, so we capture
|
||||
// mind-changes within the step. `tuiAgentToAgentKind` falls back to
|
||||
// `'other'` for any string outside the union.
|
||||
const detected = detectedAgentIdsRef.current
|
||||
track('onboarding_agent_picked', {
|
||||
agent_kind: tuiAgentToAgentKind(value),
|
||||
on_path: detected.includes(value),
|
||||
detected_count: detected.length,
|
||||
detection_state: isDetectingRef.current ? 'pending' : 'complete',
|
||||
from_collapsed_section: fromCollapsedSection
|
||||
})
|
||||
// mind-changes within the step. The payload builder is extracted so the
|
||||
// store-fields-attached invariant has unit coverage — see
|
||||
// agent-picked-payload.test.ts.
|
||||
track(
|
||||
'onboarding_agent_picked',
|
||||
buildAgentPickedPayload({
|
||||
agent: value,
|
||||
detectedAgentIds: detectedAgentIdsRef.current,
|
||||
isDetecting: isDetectingRef.current,
|
||||
fromCollapsedSection,
|
||||
pathSource: pathSourceRef.current,
|
||||
pathFailureReason: pathFailureReasonRef.current
|
||||
})
|
||||
)
|
||||
},
|
||||
[]
|
||||
)
|
||||
|
|
@ -132,6 +144,12 @@ export function useOnboardingFlow(
|
|||
useEffect(() => {
|
||||
isDetectingRef.current = isDetectingAgents
|
||||
}, [isDetectingAgents])
|
||||
useEffect(() => {
|
||||
pathSourceRef.current = pathSource
|
||||
}, [pathSource])
|
||||
useEffect(() => {
|
||||
pathFailureReasonRef.current = pathFailureReason
|
||||
}, [pathFailureReason])
|
||||
|
||||
// Why: pin start time once so onboarding_completed reports a real funnel duration.
|
||||
const startTimeRef = useRef<number>(Date.now())
|
||||
|
|
|
|||
|
|
@ -1,11 +1,17 @@
|
|||
import type { StateCreator } from 'zustand'
|
||||
import type { AppState } from '../types'
|
||||
import type { TuiAgent } from '../../../../shared/types'
|
||||
import type { PathSource, ShellHydrationFailureReason, TuiAgent } from '../../../../shared/types'
|
||||
|
||||
export type DetectedAgentsSlice = {
|
||||
detectedAgentIds: TuiAgent[] | null
|
||||
isDetectingAgents: boolean
|
||||
isRefreshingAgents: boolean
|
||||
/** Telemetry classification of the most recent refreshAgents() run. `null`
|
||||
* before the first refresh resolves. Read by the wizard at agent-pick time
|
||||
* to attach `path_source` / `path_failure_reason` to `onboarding_agent_picked`
|
||||
* — see docs/agent-on-path-detection.md. */
|
||||
pathSource: PathSource | null
|
||||
pathFailureReason: ShellHydrationFailureReason | null
|
||||
/** Runs `preflight.detectAgents` once per session. Subsequent callers reuse
|
||||
* the in-flight promise so every surface sees the same result. */
|
||||
ensureDetectedAgents: () => Promise<TuiAgent[]>
|
||||
|
|
@ -36,6 +42,8 @@ export const createDetectedAgentsSlice: StateCreator<AppState, [], [], DetectedA
|
|||
detectedAgentIds: null,
|
||||
isDetectingAgents: false,
|
||||
isRefreshingAgents: false,
|
||||
pathSource: null,
|
||||
pathFailureReason: null,
|
||||
|
||||
ensureDetectedAgents: () => {
|
||||
const existing = get().detectedAgentIds
|
||||
|
|
@ -73,7 +81,12 @@ export const createDetectedAgentsSlice: StateCreator<AppState, [], [], DetectedA
|
|||
.refreshAgents()
|
||||
.then((result) => {
|
||||
const typed = result.agents as TuiAgent[]
|
||||
set({ detectedAgentIds: typed, isRefreshingAgents: false })
|
||||
set({
|
||||
detectedAgentIds: typed,
|
||||
isRefreshingAgents: false,
|
||||
pathSource: result.pathSource,
|
||||
pathFailureReason: result.pathFailureReason
|
||||
})
|
||||
// Why: once refresh has run, treat its result as the current detection
|
||||
// snapshot so `ensureDetectedAgents` short-circuits.
|
||||
detectPromise = Promise.resolve(typed)
|
||||
|
|
|
|||
|
|
@ -17,7 +17,13 @@ import { z } from 'zod'
|
|||
|
||||
import { AGENT_HOOK_TARGETS } from './agent-hook-types'
|
||||
import { ONBOARDING_FINAL_STEP } from './constants'
|
||||
import type { DiscoveryStatusEmitted, GlobalSettings, OnboardingChecklistState } from './types'
|
||||
import type {
|
||||
DiscoveryStatusEmitted,
|
||||
GlobalSettings,
|
||||
OnboardingChecklistState,
|
||||
PathSource,
|
||||
ShellHydrationFailureReason
|
||||
} from './types'
|
||||
|
||||
// ── Shared property enums ───────────────────────────────────────────────
|
||||
|
||||
|
|
@ -398,6 +404,39 @@ const activationChecklistItemCompletedSchema = z
|
|||
})
|
||||
.strict()
|
||||
|
||||
// Why: see docs/agent-on-path-detection.md. Disambiguates `on_path: false`
|
||||
// rows on dashboard 1562016 — distinguishes shell-hydration failure (where
|
||||
// `on_path` is misleading because Orca's view of PATH is incomplete) from
|
||||
// genuinely-not-on-PATH (where the field is reporting accurately). Closed
|
||||
// enum kept in lockstep with `ShellHydrationFailureReason` via a compile-time
|
||||
// guard below.
|
||||
const pathSourceSchema = z.enum(['shell_hydrate', 'sync_seed_only'])
|
||||
const pathFailureReasonSchema = z.enum(['none', 'no_shell', 'timeout', 'spawn_error', 'empty_path'])
|
||||
|
||||
// Compile-time guard: schema enum must match `ShellHydrationFailureReason`.
|
||||
// Adding a new failure mode in `hydrate-shell-path.ts` without updating both
|
||||
// the shared alias and this schema breaks the build here. Without the guard,
|
||||
// a new enum value would ship `failureReason` strings the strict validator
|
||||
// rejects, dropping the entire `onboarding_agent_picked` event at parse time
|
||||
// and losing the `agent_kind`/`on_path` data on that pick.
|
||||
type _PathFailureReasonSync =
|
||||
z.infer<typeof pathFailureReasonSchema> extends ShellHydrationFailureReason
|
||||
? ShellHydrationFailureReason extends z.infer<typeof pathFailureReasonSchema>
|
||||
? true
|
||||
: never
|
||||
: never
|
||||
const _pathFailureReasonSyncCheck: _PathFailureReasonSync = true
|
||||
void _pathFailureReasonSyncCheck
|
||||
|
||||
type _PathSourceSync =
|
||||
z.infer<typeof pathSourceSchema> extends PathSource
|
||||
? PathSource extends z.infer<typeof pathSourceSchema>
|
||||
? true
|
||||
: never
|
||||
: never
|
||||
const _pathSourceSyncCheck: _PathSourceSync = true
|
||||
void _pathSourceSyncCheck
|
||||
|
||||
// Fired at click time from `setSelectedAgentInteractive` so we capture
|
||||
// mind-changes within the step rather than just the final pick. `agent_kind`
|
||||
// uses `tuiAgentToAgentKind` so the wire enum stays closed even when stale
|
||||
|
|
@ -416,6 +455,11 @@ const onboardingAgentPickedSchema = z
|
|||
// ("Show N more"). Signals whether users go looking for less-popular
|
||||
// agents — input for catalog ordering decisions.
|
||||
from_collapsed_section: z.boolean(),
|
||||
// Why: instrumentation for the `on_path:false` triage. `.optional()` is
|
||||
// load-bearing — events emitted before this deploy validate cleanly under
|
||||
// `.strict()`. See docs/agent-on-path-detection.md.
|
||||
path_source: pathSourceSchema.optional(),
|
||||
path_failure_reason: pathFailureReasonSchema.optional(),
|
||||
cohort: cohortSchema
|
||||
})
|
||||
.strict()
|
||||
|
|
|
|||
|
|
@ -7,6 +7,21 @@ import type { GitHubProjectSettings } from './github-project-types'
|
|||
// `WorkspaceCreateTelemetrySource` from '../../../shared/types'.
|
||||
export type { WorkspaceSource as WorkspaceCreateTelemetrySource } from './telemetry-events'
|
||||
|
||||
// ─── Shell PATH hydration ────────────────────────────────────────────
|
||||
// Why: shared so the main-side `HydrationResult` discriminator and the
|
||||
// telemetry schema in `telemetry-events.ts` stay in lockstep without
|
||||
// `src/shared/` taking a forbidden import from `src/main/`. A compile-time
|
||||
// guard in telemetry-events.ts asserts the schema enum matches this alias —
|
||||
// adding a new failure mode without updating both places fails the build.
|
||||
export type ShellHydrationFailureReason =
|
||||
| 'none'
|
||||
| 'no_shell'
|
||||
| 'timeout'
|
||||
| 'spawn_error'
|
||||
| 'empty_path'
|
||||
|
||||
export type PathSource = 'shell_hydrate' | 'sync_seed_only'
|
||||
|
||||
// ─── Repo ────────────────────────────────────────────────────────────
|
||||
export type RepoKind = 'git' | 'folder'
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue