mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
feat(telemetry): track agent_hook_install_failed per agent (#1668)
* feat(telemetry): track agent_hook_install_failed per agent Replaces the closure-style installer loop in `src/main/index.ts` with a labelled `runManagedHookInstallers` so each catch can attribute the failure to its agent. Adds the `agent_hook_install_failed` event + `hookInstallAgentSchema` enum (claude/codex/gemini/cursor) and a unit test pinning fail-open semantics, label routing, and the 200-char error_message truncation. Co-authored-by: Orca <help@stably.ai> * fix(telemetry): harden agent-hook installer fail-open - describeError always returns a string (JSON.stringify can return literal undefined for throw undefined / Symbol / function, which would crash the catch handler before track fires) - wrap track() in inner try/catch so a telemetry-side throw can't abort the installer loop - dedupe AGENT_HOOK_TARGETS into one tuple in agent-hook-types so the IPC AgentHookTarget type and hookInstallAgentSchema can't drift - regression tests for object/undefined throws and track-throws Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -0,0 +1,155 @@
|
||||
// Pins the contract between `runManagedHookInstallers` and the
|
||||
// `agent_hook_install_failed` telemetry event: each catch must fire `track`
|
||||
// with the correct agent label and a truncated error_message, and one
|
||||
// installer's failure must not stop the others (fail-open semantics).
|
||||
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { trackMock } = vi.hoisted(() => ({ trackMock: vi.fn() }))
|
||||
|
||||
vi.mock('../telemetry/client', () => ({ track: trackMock }))
|
||||
|
||||
import { runManagedHookInstallers } from './install-telemetry'
|
||||
|
||||
describe('runManagedHookInstallers', () => {
|
||||
beforeEach(() => {
|
||||
trackMock.mockReset()
|
||||
vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
})
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
it('runs every installer when none throw and never calls track', () => {
|
||||
const claude = vi.fn()
|
||||
const codex = vi.fn()
|
||||
runManagedHookInstallers([
|
||||
['claude', claude],
|
||||
['codex', codex]
|
||||
])
|
||||
expect(claude).toHaveBeenCalledTimes(1)
|
||||
expect(codex).toHaveBeenCalledTimes(1)
|
||||
expect(trackMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('fires agent_hook_install_failed with the correct agent label when an installer throws', () => {
|
||||
runManagedHookInstallers([
|
||||
[
|
||||
'codex',
|
||||
() => {
|
||||
throw new Error('codex config malformed')
|
||||
}
|
||||
]
|
||||
])
|
||||
|
||||
expect(trackMock).toHaveBeenCalledTimes(1)
|
||||
expect(trackMock).toHaveBeenCalledWith('agent_hook_install_failed', {
|
||||
agent: 'codex',
|
||||
error_message: 'codex config malformed'
|
||||
})
|
||||
})
|
||||
|
||||
it('continues running later installers after an earlier one throws (fail-open)', () => {
|
||||
const codex = vi.fn()
|
||||
const gemini = vi.fn()
|
||||
runManagedHookInstallers([
|
||||
[
|
||||
'claude',
|
||||
() => {
|
||||
throw new Error('claude failed')
|
||||
}
|
||||
],
|
||||
['codex', codex],
|
||||
['gemini', gemini]
|
||||
])
|
||||
expect(codex).toHaveBeenCalledTimes(1)
|
||||
expect(gemini).toHaveBeenCalledTimes(1)
|
||||
expect(trackMock).toHaveBeenCalledTimes(1)
|
||||
expect(trackMock).toHaveBeenCalledWith(
|
||||
'agent_hook_install_failed',
|
||||
expect.objectContaining({ agent: 'claude' })
|
||||
)
|
||||
})
|
||||
|
||||
it('truncates error_message to 200 chars', () => {
|
||||
const longMessage = 'x'.repeat(500)
|
||||
runManagedHookInstallers([
|
||||
[
|
||||
'gemini',
|
||||
() => {
|
||||
throw new Error(longMessage)
|
||||
}
|
||||
]
|
||||
])
|
||||
expect(trackMock).toHaveBeenCalledTimes(1)
|
||||
const [, props] = trackMock.mock.calls[0] as [string, { error_message: string }]
|
||||
expect(props.error_message.length).toBe(200)
|
||||
})
|
||||
|
||||
it('handles non-Error throws', () => {
|
||||
runManagedHookInstallers([
|
||||
[
|
||||
'cursor',
|
||||
() => {
|
||||
throw 'cursor string failure'
|
||||
}
|
||||
]
|
||||
])
|
||||
expect(trackMock).toHaveBeenCalledWith('agent_hook_install_failed', {
|
||||
agent: 'cursor',
|
||||
error_message: 'cursor string failure'
|
||||
})
|
||||
})
|
||||
|
||||
it('serializes thrown objects through JSON.stringify', () => {
|
||||
runManagedHookInstallers([
|
||||
[
|
||||
'cursor',
|
||||
() => {
|
||||
throw { code: 'EACCES', path: '/tmp' }
|
||||
}
|
||||
]
|
||||
])
|
||||
expect(trackMock).toHaveBeenCalledTimes(1)
|
||||
expect(trackMock).toHaveBeenCalledWith('agent_hook_install_failed', {
|
||||
agent: 'cursor',
|
||||
error_message: '{"code":"EACCES","path":"/tmp"}'
|
||||
})
|
||||
})
|
||||
|
||||
it('does not throw when an installer throws undefined (regression for JSON.stringify undefined return)', () => {
|
||||
expect(() =>
|
||||
runManagedHookInstallers([
|
||||
[
|
||||
'cursor',
|
||||
() => {
|
||||
throw undefined
|
||||
}
|
||||
]
|
||||
])
|
||||
).not.toThrow()
|
||||
expect(trackMock).toHaveBeenCalledTimes(1)
|
||||
const [eventName, props] = trackMock.mock.calls[0] as [string, { error_message: string }]
|
||||
expect(eventName).toBe('agent_hook_install_failed')
|
||||
expect(typeof props.error_message).toBe('string')
|
||||
})
|
||||
|
||||
it('continues running later installers when track itself throws (telemetry must not break fail-open)', () => {
|
||||
const codex = vi.fn()
|
||||
trackMock.mockImplementationOnce(() => {
|
||||
throw new Error('telemetry blew up')
|
||||
})
|
||||
expect(() =>
|
||||
runManagedHookInstallers([
|
||||
[
|
||||
'claude',
|
||||
() => {
|
||||
throw new Error('claude failed')
|
||||
}
|
||||
],
|
||||
['codex', codex]
|
||||
])
|
||||
).not.toThrow()
|
||||
expect(codex).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,53 @@
|
||||
// Per-agent managed-hook installer with fail-open semantics and PostHog
|
||||
// attribution. Lifted out of `src/main/index.ts` so the loop is unit-testable
|
||||
// without standing up the full Electron startup graph — the catch site needs
|
||||
// the agent label to fire `agent_hook_install_failed`, and the previous
|
||||
// closure-style loop lost it.
|
||||
|
||||
import type { HookInstallAgent } from '../../shared/telemetry-events'
|
||||
import { track } from '../telemetry/client'
|
||||
|
||||
// Why: install errors are about config-file shape (malformed JSON, ACL
|
||||
// denial), not user content — but messages can include paths or stack
|
||||
// fragments. The 200-char cap matches `agentHookInstallFailedSchema.error_message`
|
||||
// in `src/shared/telemetry-events.ts`; the validator drops overlength values,
|
||||
// so truncation must happen here at the call site.
|
||||
const ERROR_MESSAGE_MAX_LEN = 200
|
||||
|
||||
export type ManagedHookInstaller = readonly [HookInstallAgent, () => void]
|
||||
|
||||
function describeError(error: unknown): string {
|
||||
if (error instanceof Error) {
|
||||
return error.message
|
||||
}
|
||||
if (typeof error === 'string') {
|
||||
return error
|
||||
}
|
||||
try {
|
||||
const json = JSON.stringify(error)
|
||||
return typeof json === 'string' ? json : String(error)
|
||||
} catch {
|
||||
return String(error)
|
||||
}
|
||||
}
|
||||
|
||||
export function runManagedHookInstallers(installers: readonly ManagedHookInstaller[]): void {
|
||||
for (const [agent, install] of installers) {
|
||||
try {
|
||||
install()
|
||||
} catch (error) {
|
||||
console.error(`[agent-hooks] Failed to install ${agent} managed hooks:`, error)
|
||||
// Why: telemetry must not break fail-open. A throw inside `track` (e.g.
|
||||
// a corrupted settings store the resolveConsent path reads from) would
|
||||
// otherwise abort the for-loop and skip later agents' installers.
|
||||
try {
|
||||
track('agent_hook_install_failed', {
|
||||
agent,
|
||||
error_message: describeError(error).slice(0, ERROR_MESSAGE_MAX_LEN)
|
||||
})
|
||||
} catch (telemetryError) {
|
||||
console.error('[agent-hooks] Failed to record install-failure telemetry:', telemetryError)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
+9
-16
@@ -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 { runManagedHookInstallers } from './agent-hooks/install-telemetry'
|
||||
import { initCohortClassifier } from './telemetry/cohort-classifier'
|
||||
import { initOnboardingCohortClassifier } from './telemetry/onboarding-cohort-classifier'
|
||||
import { resolveConsent } from './telemetry/consent'
|
||||
@@ -488,22 +489,14 @@ app.whenReady().then(async () => {
|
||||
// Why: managed hook installation mutates user-global agent config. Each
|
||||
// installer runs inside its own try/catch so a malformed local config
|
||||
// (e.g. corrupted ~/.claude/settings.json) cannot brick Orca startup.
|
||||
for (const installManagedHooks of [
|
||||
() => claudeHookService.install(),
|
||||
() => codexHookService.install(),
|
||||
() => geminiHookService.install()
|
||||
]) {
|
||||
try {
|
||||
installManagedHooks()
|
||||
} catch (error) {
|
||||
console.error('[agent-hooks] Failed to install managed hooks:', error)
|
||||
}
|
||||
}
|
||||
try {
|
||||
cursorHookService.install()
|
||||
} catch (error) {
|
||||
console.error('[agent-hooks] Failed to install Cursor managed hooks:', error)
|
||||
}
|
||||
// The agent label travels with each installer so the catch can attribute
|
||||
// the failure in the `agent_hook_install_failed` telemetry event.
|
||||
runManagedHookInstallers([
|
||||
['claude', () => claudeHookService.install()],
|
||||
['codex', () => codexHookService.install()],
|
||||
['gemini', () => geminiHookService.install()],
|
||||
['cursor', () => cursorHookService.install()]
|
||||
])
|
||||
|
||||
registerAppMenu({
|
||||
onCheckForUpdates: (options) => checkForUpdatesFromMenu(options),
|
||||
|
||||
@@ -4,7 +4,8 @@
|
||||
// gemini/cursor/hook-service.ts). Lives in `shared/` to keep a single
|
||||
// source of truth for the version string and status contract.
|
||||
|
||||
export type AgentHookTarget = 'claude' | 'codex' | 'gemini' | 'cursor'
|
||||
export const AGENT_HOOK_TARGETS = ['claude', 'codex', 'gemini', 'cursor'] as const
|
||||
export type AgentHookTarget = (typeof AGENT_HOOK_TARGETS)[number]
|
||||
|
||||
export type AgentHookInstallState = 'installed' | 'not_installed' | 'partial' | 'error'
|
||||
|
||||
|
||||
@@ -15,6 +15,7 @@
|
||||
|
||||
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'
|
||||
|
||||
@@ -250,6 +251,27 @@ const workspaceCreateFailedSchema = z
|
||||
})
|
||||
.strict()
|
||||
|
||||
// Managed-hook installer per-agent label. Distinct from `AGENT_KIND_VALUES`:
|
||||
// hook installation only targets these four agents and the labels here match
|
||||
// the `*HookService.install()` call sites in `src/main/index.ts`. `claude`
|
||||
// (not `claude-code`) is intentional — the failure is about Claude Code's
|
||||
// `~/.claude/settings.json`, not the broader product taxonomy. Sourced from
|
||||
// `AGENT_HOOK_TARGETS` so the wire enum and the IPC `AgentHookTarget` type
|
||||
// cannot drift if a fifth hook-install agent is added.
|
||||
export const hookInstallAgentSchema = z.enum(AGENT_HOOK_TARGETS)
|
||||
export type HookInstallAgent = z.infer<typeof hookInstallAgentSchema>
|
||||
|
||||
// Why: install failures are config-file-shape errors (malformed JSON, missing
|
||||
// keys, ACL denials on `~/.claude` etc.) — not user content. The 200-char
|
||||
// cap is the truncation contract; callers must truncate before calling
|
||||
// `track`, and the validator will drop overlength strings via `.max(200)`.
|
||||
const agentHookInstallFailedSchema = z
|
||||
.object({
|
||||
agent: hookInstallAgentSchema,
|
||||
error_message: z.string().max(200)
|
||||
})
|
||||
.strict()
|
||||
|
||||
// ── Onboarding ──────────────────────────────────────────────────────────
|
||||
//
|
||||
// Closed enums only — no raw paths, repo names, clone URLs, or error
|
||||
@@ -461,6 +483,7 @@ export const eventSchemas = {
|
||||
|
||||
agent_started: agentStartedSchema,
|
||||
agent_error: agentErrorSchema,
|
||||
agent_hook_install_failed: agentHookInstallFailedSchema,
|
||||
|
||||
settings_changed: settingsChangedSchema,
|
||||
|
||||
@@ -528,12 +551,11 @@ type _CohortExtendedRoster =
|
||||
type _DerivedCohortExtendedEvents = {
|
||||
[N in EventName]: 'nth_repo_added' extends keyof EventMap[N] ? N : never
|
||||
}[EventName]
|
||||
type _CohortExtendedRosterSync =
|
||||
_CohortExtendedRoster extends _DerivedCohortExtendedEvents
|
||||
? _DerivedCohortExtendedEvents extends _CohortExtendedRoster
|
||||
? true
|
||||
: never
|
||||
type _CohortExtendedRosterSync = _CohortExtendedRoster extends _DerivedCohortExtendedEvents
|
||||
? _DerivedCohortExtendedEvents extends _CohortExtendedRoster
|
||||
? true
|
||||
: never
|
||||
: never
|
||||
const _cohortExtendedRosterSyncCheck: _CohortExtendedRosterSync = true
|
||||
void _cohortExtendedRosterSyncCheck
|
||||
|
||||
@@ -577,12 +599,11 @@ type _OnboardingCohortRoster =
|
||||
type _DerivedOnboardingCohortEvents = {
|
||||
[N in EventName]: 'cohort' extends keyof EventMap[N] ? N : never
|
||||
}[EventName]
|
||||
type _OnboardingCohortRosterSync =
|
||||
_OnboardingCohortRoster extends _DerivedOnboardingCohortEvents
|
||||
? _DerivedOnboardingCohortEvents extends _OnboardingCohortRoster
|
||||
? true
|
||||
: never
|
||||
type _OnboardingCohortRosterSync = _OnboardingCohortRoster extends _DerivedOnboardingCohortEvents
|
||||
? _DerivedOnboardingCohortEvents extends _OnboardingCohortRoster
|
||||
? true
|
||||
: never
|
||||
: never
|
||||
const _onboardingCohortRosterSyncCheck: _OnboardingCohortRosterSync = true
|
||||
void _onboardingCohortRosterSyncCheck
|
||||
|
||||
|
||||
Reference in New Issue
Block a user