WIP: Changes before auto-review fixes

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
brennanb2025
2026-05-05 18:09:55 -07:00
co-authored by Orca
parent 148f33c470
commit 9b489c2bcb
10 changed files with 962 additions and 4 deletions
+75
View File
@@ -0,0 +1,75 @@
# Review Context
## Branch Info
- Base: origin/main (merge-base 148f33c47062562d0678ec8972ed20edf08b1ec9)
- Current: brennanb2025/orca-cli-telemetry
## Changed Files Summary
- M config/tsconfig.cli.json
- M src/cli/dispatch.ts
- A src/cli/telemetry.test.ts
- A src/cli/telemetry.ts
- M src/main/runtime/rpc/methods/index.ts
- A src/main/runtime/rpc/methods/telemetry.test.ts
- A src/main/runtime/rpc/methods/telemetry.ts
- M src/shared/telemetry-events.test.ts
- M src/shared/telemetry-events.ts
## Changed Line Ranges (PR Scope)
| File | Changed Lines |
| --------------------------------------------- | ------------------------------ |
| config/tsconfig.cli.json | 3-8 |
| src/cli/dispatch.ts | 3, 62-74 |
| src/cli/telemetry.test.ts | 1-181 (new file) |
| src/cli/telemetry.ts | 1-344 (new file) |
| src/main/runtime/rpc/methods/index.ts | 11, 25-26 |
| src/main/runtime/rpc/methods/telemetry.test.ts| 1-131 (new file) |
| src/main/runtime/rpc/methods/telemetry.ts | 1-82 (new file) |
| src/shared/telemetry-events.test.ts | 10-11, 15, 194-272 |
| src/shared/telemetry-events.ts | 113-133, 211-220, 246-260 |
## Review Standards Reference
- Follow /review-code standards
- Focus on: correctness, security, performance, maintainability
- Priority levels: Critical > High > Medium > Low
## File Categories
### Electron/Main
- src/main/runtime/rpc/methods/index.ts
- src/main/runtime/rpc/methods/telemetry.ts
- src/main/runtime/rpc/methods/telemetry.test.ts
### Config/Build
- config/tsconfig.cli.json
### Utility/Common
- src/cli/dispatch.ts
- src/cli/telemetry.ts
- src/cli/telemetry.test.ts
- src/shared/telemetry-events.ts
- src/shared/telemetry-events.test.ts
## Skipped Issues (Do Not Re-validate)
- [src/main/runtime/rpc/methods/telemetry.ts:44] | Low | Cosmetic — per-event `.strict()` schemas catch non-object props at the same boundary | params.props z.unknown() doesn't enforce object shape at params layer
- [src/main/runtime/rpc/methods/telemetry.ts:42] | Low | Stylistic — sibling RPC files use the same transform-then-pipe pattern | "Missing event name" error message is misleading for non-string name values
- [config/tsconfig.cli.json:7] | Low | No-harm pre-emptive include; potentially needed for future telemetry shared-code work | `../src/types/**/*` include not currently referenced
- [src/cli/telemetry.ts:283-344] | Low | Significant test infra (tmp socket server) for code with extensive load-bearing comments | defaultSender socket-lifecycle has no integration test
- [src/cli/telemetry.ts:9-10] | Low | Documented design choice — "best-effort, fire-and-forget" per design doc | unref + likely `process.exit` means short commands drop events
- [src/cli/telemetry.ts:220-227] | Low | Documented design choice — "first-write-wins" per design doc | first-failure-wins biases dashboard
- [src/main/runtime/rpc/methods/telemetry.ts:18-19] | Low | Architectural refactor — out of scope for this PR | direct import of `client` instead of routing through `OrcaRuntimeService`
- [src/main/runtime/rpc/methods/telemetry.ts:58-60] | Low | Threat-model says no wire leak; main-side debug log is judgment call | no observability hook for rejected CLI event names
- [src/cli/telemetry.ts:186-204] | Low | Stylistic — `_*ForTests` exports are an existing project pattern | test seams in production module
- [src/cli/telemetry.ts:294-302] | Low | Belt-and-braces is documented; structure works correctly | createConnection sync-throw try/catch awkwardness
- [src/cli/dispatch.ts:3] | Low | Stylistic only | `./telemetry` import not grouped with handler imports
## Iteration State
Current iteration: 1
Last completed phase: Validation
Files fixed this iteration: []
+6 -1
View File
@@ -1,6 +1,11 @@
{
"extends": "@electron-toolkit/tsconfig/tsconfig.node.json",
"include": ["../src/cli/**/*", "../src/shared/**/*", "../src/main/runtime/runtime-metadata.ts"],
"include": [
"../src/cli/**/*",
"../src/shared/**/*",
"../src/main/runtime/runtime-metadata.ts",
"../src/types/**/*"
],
"compilerOptions": {
"composite": true,
"module": "CommonJS",
+14 -1
View File
@@ -1,5 +1,6 @@
import type { RuntimeClient } from './runtime-client'
import { RuntimeClientError } from './runtime-client'
import { recordCliFeatureUsed } from './telemetry'
import { CORE_HANDLERS } from './handlers/core'
import { REPO_HANDLERS } from './handlers/repo'
import { WORKTREE_HANDLERS } from './handlers/worktree'
@@ -58,5 +59,17 @@ export async function dispatch(commandPath: string[], ctx: HandlerContext): Prom
if (!handler) {
throw new RuntimeClientError('invalid_argument', `Unknown command: ${commandPath.join(' ')}`)
}
await handler(ctx)
// Why: telemetry is fire-at-action — emit on the first call in a feature
// group with that call's exit_status, then never again for this process.
// The recording call is non-blocking and silent on failure, so it cannot
// observably affect the command. We record `failure` if the handler
// throws and `success` otherwise, then re-throw to preserve the original
// error path. No buffering, no process-exit flush.
try {
await handler(ctx)
} catch (error) {
recordCliFeatureUsed(commandPath, 'failure')
throw error
}
recordCliFeatureUsed(commandPath, 'success')
}
+181
View File
@@ -0,0 +1,181 @@
// Dispatch-side dedupe and feature-group-mapping coverage. The wire shape
// (what props flow to the runtime RPC method) is the contract we're
// pinning here — `recordCliFeatureUsed` is the boundary the rest of the
// CLI talks to, and a regression in the dedupe logic would either flood
// PostHog (no dedupe) or starve adoption signal (over-dedupe).
//
// Tests intentionally substitute a fake sender via `_setTelemetrySenderForTests`
// rather than letting a socket open against a runtime that may or may not
// be running — the fire-and-forget transport's design (unref, swallow
// errors) makes it impossible to observe failures otherwise.
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import {
_getEmittedGroupsForTests,
_resetEmittedGroupsForTests,
_setTelemetrySenderForTests,
recordCliFeatureUsed,
resolveFeatureGroup
} from './telemetry'
import type { RuntimeMetadata } from '../shared/runtime-bootstrap'
import type * as MetadataModule from './runtime/metadata'
// Stand in for a metadata file on disk. The sender shim never reads from
// disk in tests, but `tryReadMetadata` does — see the `vi.mock` below.
const FAKE_METADATA: RuntimeMetadata = {
runtimeId: 'test-runtime',
pid: 0,
authToken: 'test-token',
startedAt: 0,
transports: [{ kind: 'unix', endpoint: '/tmp/orca-test.sock' }]
}
vi.mock('./runtime/metadata', async (importOriginal) => {
const actual = (await importOriginal()) as typeof MetadataModule
return {
...actual,
tryReadMetadata: () => FAKE_METADATA,
getDefaultUserDataPath: () => '/tmp/orca-test-userdata'
}
})
type SenderCall = {
endpoint: string
body: { method: string; params: unknown }
}
describe('resolveFeatureGroup', () => {
it('maps representative commands to their declared groups', () => {
// One per group — protects the design-doc grouping table from drift.
expect(resolveFeatureGroup(['snapshot'])).toBe('browser_observation')
expect(resolveFeatureGroup(['click'])).toBe('browser_interaction')
expect(resolveFeatureGroup(['goto'])).toBe('browser_navigation')
expect(resolveFeatureGroup(['cookie', 'set'])).toBe('browser_config')
expect(resolveFeatureGroup(['terminal', 'create'])).toBe('terminal_orchestration')
expect(resolveFeatureGroup(['terminal', 'send'])).toBe('terminal_io')
expect(resolveFeatureGroup(['worktree', 'create'])).toBe('worktree_orchestration')
expect(resolveFeatureGroup(['orchestration', 'run'])).toBe('orchestration_coordinator')
expect(resolveFeatureGroup(['orchestration', 'check'])).toBe('orchestration_messaging')
expect(resolveFeatureGroup(['status'])).toBe('discovery')
})
it('returns null for unmapped commands (soft no-op)', () => {
expect(resolveFeatureGroup(['definitely-not-a-real-command'])).toBeNull()
expect(resolveFeatureGroup([])).toBeNull()
})
})
describe('recordCliFeatureUsed', () => {
let senderCalls: SenderCall[]
beforeEach(() => {
_resetEmittedGroupsForTests()
senderCalls = []
_setTelemetrySenderForTests(async (_metadata, endpoint, body) => {
senderCalls.push({ endpoint, body })
})
})
afterEach(() => {
_setTelemetrySenderForTests(null)
_resetEmittedGroupsForTests()
})
// Drains microtasks so the fire-and-forget `void send(...).catch(...)`
// resolves before assertions. `recordCliFeatureUsed` deliberately does
// not return a Promise so dispatch never blocks on telemetry.
async function flush(): Promise<void> {
await new Promise<void>((resolve) => setImmediate(resolve))
}
it('emits exactly one event for the first call in a group', async () => {
recordCliFeatureUsed(['snapshot'], 'success')
await flush()
expect(senderCalls).toHaveLength(1)
expect(senderCalls[0].body.method).toBe('telemetry.captureCliEvent')
expect(senderCalls[0].body.params).toEqual({
name: 'cli_feature_used',
props: { feature_group: 'browser_observation', exit_status: 'success' }
})
})
// The cap-saving design: a single agent loop that calls `snapshot` 200
// times must produce exactly one event. If this regresses, the PostHog
// bill regresses with it.
it('dedupes subsequent calls in the same group within one process', async () => {
for (let i = 0; i < 50; i++) {
recordCliFeatureUsed(['snapshot'], 'success')
}
await flush()
expect(senderCalls).toHaveLength(1)
})
// First-write-wins on exit_status: a later failure in the same group
// does not retransmit. Adopting "any failure flips the bit" would mean
// buffering and a process-exit flush, which the design explicitly
// rejected.
it('first-write-wins on exit_status (later failure does not re-emit)', async () => {
recordCliFeatureUsed(['snapshot'], 'success')
recordCliFeatureUsed(['screenshot'], 'failure') // same group: browser_observation
await flush()
expect(senderCalls).toHaveLength(1)
expect(senderCalls[0].body.params).toEqual({
name: 'cli_feature_used',
props: { feature_group: 'browser_observation', exit_status: 'success' }
})
})
it('emits separate events for distinct groups', async () => {
recordCliFeatureUsed(['snapshot'], 'success')
recordCliFeatureUsed(['click'], 'success')
recordCliFeatureUsed(['goto'], 'failure')
await flush()
expect(senderCalls).toHaveLength(3)
const groups = senderCalls.map((c) => {
const params = c.body.params as { props: { feature_group: string } }
return params.props.feature_group
})
expect(groups.sort()).toEqual(['browser_interaction', 'browser_navigation', 'browser_observation'])
})
it('records exit_status: failure when the first call fails', async () => {
recordCliFeatureUsed(['snapshot'], 'failure')
await flush()
expect(senderCalls).toHaveLength(1)
expect(senderCalls[0].body.params).toEqual({
name: 'cli_feature_used',
props: { feature_group: 'browser_observation', exit_status: 'failure' }
})
})
it('is a no-op for unmapped commands and does not consume the dedupe slot', async () => {
recordCliFeatureUsed(['unknown-command'], 'success')
await flush()
expect(senderCalls).toHaveLength(0)
expect(_getEmittedGroupsForTests().size).toBe(0)
})
// Belt-and-braces: the dedupe set must be consulted BEFORE the send is
// dispatched. If a sender failure could un-mark the group, retries on a
// broken socket would amplify into one connect attempt per command —
// exactly the "telemetry never affects user-visible behavior" violation
// the design rules out.
it('does not retry on sender failure', async () => {
const failingSender = vi.fn().mockRejectedValue(new Error('connection refused'))
_setTelemetrySenderForTests(failingSender)
recordCliFeatureUsed(['snapshot'], 'success')
recordCliFeatureUsed(['snapshot'], 'success')
recordCliFeatureUsed(['screenshot'], 'success')
await flush()
expect(failingSender).toHaveBeenCalledTimes(1)
})
// The function returns synchronously (`void`) — the design forbids
// dispatch from awaiting telemetry so a stuck socket can never delay
// the user-visible command.
it('returns synchronously without awaiting the send', () => {
const result = recordCliFeatureUsed(['snapshot'], 'success')
expect(result).toBeUndefined()
})
})
+344
View File
@@ -0,0 +1,344 @@
// Dispatch-side CLI telemetry. One job: emit `cli_feature_used` at most once
// per (feature_group, CLI process) and never observably affect command
// behavior. See docs/cli-telemetry-design.md for the full design — the
// constraints below are load-bearing, not stylistic preferences.
//
// Constraints (each maps to a comment further down):
// - never write to stdout/stderr (preserves --json purity, no
// UnhandledPromiseRejectionWarning leaks)
// - never block process exit (unref the socket + any pending timeout, no
// `await` from the call site)
// - never throw out of the dispatch path on telemetry failure
// - dedupe per process via an in-memory Set
//
// Communication path: CLI → runtime RPC `telemetry.captureCliEvent` → main
// `track()`. If the app isn't running, metadata is missing, or the auth
// token is stale, we silently swallow — the CLI is not the consent
// authority and not the place to log telemetry health.
import { createConnection } from 'net'
import { randomUUID } from 'crypto'
import { findTransport, type RuntimeMetadata } from '../shared/runtime-bootstrap'
import { getDefaultUserDataPath, tryReadMetadata } from './runtime/metadata'
import type { CliFeatureGroup } from '../shared/telemetry-events'
// Map from `commandPath` (the same `commandPath.join(' ')` key dispatch uses)
// to a feature group. Kept exhaustive for the commands shipped today;
// unmapped commands return `null` and emit no event. Adding a new command
// without a mapping is a soft no-op — telemetry will silently miss it
// rather than mis-bucket — which is the right tradeoff for a coarse
// adoption signal.
//
// Groupings mirror docs/cli-telemetry-design.md §Events. If you change the
// grouping for a command, change it there too — dashboards built on the
// original grouping will silently mix old + new rows.
const COMMAND_TO_FEATURE_GROUP: Record<string, CliFeatureGroup> = {
// worktree_orchestration: lifecycle of worktrees and base-ref repo state
'worktree create': 'worktree_orchestration',
'worktree rm': 'worktree_orchestration',
'worktree set': 'worktree_orchestration',
'repo add': 'worktree_orchestration',
'repo set-base-ref': 'worktree_orchestration',
// terminal_orchestration: terminal lifecycle + focus
'terminal create': 'terminal_orchestration',
'terminal split': 'terminal_orchestration',
'terminal close': 'terminal_orchestration',
'terminal stop': 'terminal_orchestration',
'terminal rename': 'terminal_orchestration',
'terminal switch': 'terminal_orchestration',
'terminal focus': 'terminal_orchestration',
// terminal_io: data flow into/out of an existing terminal
'terminal read': 'terminal_io',
'terminal send': 'terminal_io',
'terminal wait': 'terminal_io',
// browser_navigation: things that change "where" the page is
goto: 'browser_navigation',
back: 'browser_navigation',
forward: 'browser_navigation',
reload: 'browser_navigation',
'tab create': 'browser_navigation',
'tab close': 'browser_navigation',
'tab switch': 'browser_navigation',
// browser_observation: read-only inspection
snapshot: 'browser_observation',
screenshot: 'browser_observation',
'full-screenshot': 'browser_observation',
eval: 'browser_observation',
wait: 'browser_observation',
pdf: 'browser_observation',
get: 'browser_observation',
is: 'browser_observation',
find: 'browser_observation',
// browser_interaction: mutate the page
click: 'browser_interaction',
dblclick: 'browser_interaction',
fill: 'browser_interaction',
type: 'browser_interaction',
select: 'browser_interaction',
check: 'browser_interaction',
uncheck: 'browser_interaction',
focus: 'browser_interaction',
clear: 'browser_interaction',
'select-all': 'browser_interaction',
keypress: 'browser_interaction',
hover: 'browser_interaction',
drag: 'browser_interaction',
upload: 'browser_interaction',
scroll: 'browser_interaction',
scrollintoview: 'browser_interaction',
inserttext: 'browser_interaction',
download: 'browser_interaction',
highlight: 'browser_interaction',
'mouse move': 'browser_interaction',
'mouse down': 'browser_interaction',
'mouse up': 'browser_interaction',
'mouse wheel': 'browser_interaction',
// browser_config: viewport/device/cookies/storage/intercept/capture
'set device': 'browser_config',
'set offline': 'browser_config',
'set headers': 'browser_config',
'set credentials': 'browser_config',
'set media': 'browser_config',
'clipboard read': 'browser_config',
'clipboard write': 'browser_config',
'dialog accept': 'browser_config',
'dialog dismiss': 'browser_config',
'cookie get': 'browser_config',
'cookie set': 'browser_config',
'cookie delete': 'browser_config',
'storage local get': 'browser_config',
'storage local set': 'browser_config',
'storage local clear': 'browser_config',
'storage session get': 'browser_config',
'storage session set': 'browser_config',
'storage session clear': 'browser_config',
'intercept enable': 'browser_config',
'intercept disable': 'browser_config',
'intercept list': 'browser_config',
'capture start': 'browser_config',
'capture stop': 'browser_config',
'tab profile list': 'browser_config',
'tab profile create': 'browser_config',
'tab profile delete': 'browser_config',
'tab profile set': 'browser_config',
'tab profile show': 'browser_config',
'tab profile use-default': 'browser_config',
'tab profile clone': 'browser_config',
// orchestration_coordinator: agents driving other agents
'orchestration run': 'orchestration_coordinator',
'orchestration run-stop': 'orchestration_coordinator',
'orchestration task-create': 'orchestration_coordinator',
'orchestration dispatch': 'orchestration_coordinator',
'orchestration ask': 'orchestration_coordinator',
'orchestration gate-create': 'orchestration_coordinator',
'orchestration gate-resolve': 'orchestration_coordinator',
'orchestration reset': 'orchestration_coordinator',
// orchestration_messaging: inbox and message-mutation surfaces
'orchestration check': 'orchestration_messaging',
'orchestration inbox': 'orchestration_messaging',
'orchestration task-list': 'orchestration_messaging',
'orchestration task-update': 'orchestration_messaging',
'orchestration send': 'orchestration_messaging',
'orchestration reply': 'orchestration_messaging',
'orchestration dispatch-show': 'orchestration_messaging',
'orchestration gate-list': 'orchestration_messaging',
// discovery: status / list / show / current / search
status: 'discovery',
open: 'discovery',
'worktree list': 'discovery',
'worktree ps': 'discovery',
'worktree show': 'discovery',
'worktree current': 'discovery',
'terminal list': 'discovery',
'terminal show': 'discovery',
'tab list': 'discovery',
'tab show': 'discovery',
'tab current': 'discovery',
'repo list': 'discovery',
'repo show': 'discovery',
'repo search-refs': 'discovery'
}
export function resolveFeatureGroup(commandPath: readonly string[]): CliFeatureGroup | null {
return COMMAND_TO_FEATURE_GROUP[commandPath.join(' ')] ?? null
}
// Per-process dedupe set. The "first-write-wins on exit_status" rule lives
// here: once a group is in this Set, any later call in the same process —
// success or failure — never re-emits. See docs/cli-telemetry-design.md
// §Events for why.
const emittedGroups = new Set<CliFeatureGroup>()
// Test seam for the network layer. Production uses `defaultSender` below.
// Tests substitute a fake to assert what would be sent without opening a
// socket. Kept separate from `_resetEmittedGroupsForTests` so a test can
// reset dedupe state without rewiring the sender.
type Sender = (
metadata: RuntimeMetadata,
endpoint: string,
body: { method: string; params: unknown }
) => Promise<void>
let senderOverride: Sender | null = null
export function _setTelemetrySenderForTests(sender: Sender | null): void {
senderOverride = sender
}
export function _resetEmittedGroupsForTests(): void {
emittedGroups.clear()
}
export function _getEmittedGroupsForTests(): ReadonlySet<CliFeatureGroup> {
return emittedGroups
}
/**
* Record that `commandPath` ran with `exitStatus`, and emit a
* `cli_feature_used` event if this is the first call in the group for this
* process. Fire-and-forget — this function never throws, never blocks, and
* never logs.
*/
export function recordCliFeatureUsed(
commandPath: readonly string[],
exitStatus: 'success' | 'failure'
): void {
const group = resolveFeatureGroup(commandPath)
if (group === null) {
return
}
if (emittedGroups.has(group)) {
return
}
// Mark BEFORE attempting the send. If the send fails, we still treat the
// group as "reported for this process" — retrying on every subsequent call
// would amplify a broken-RPC scenario into one failed connect per command,
// which is the opposite of "telemetry never affects user-visible behavior."
emittedGroups.add(group)
// `void` + `.catch` together cover the two ways a stray rejection could
// become user-visible: an `await`-less promise without a `.catch` would
// surface as `UnhandledPromiseRejectionWarning` to stderr, breaking
// `--json` purity. Belt-and-braces because the implementation is supposed
// to swallow internally, but a future refactor mustn't be able to leak.
void sendCliFeatureUsed(group, exitStatus).catch(() => {
// Swallow. This is the entire failure-handling story.
})
}
async function sendCliFeatureUsed(
group: CliFeatureGroup,
exitStatus: 'success' | 'failure'
): Promise<void> {
// Resolve userDataPath at send time so `ORCA_USER_DATA_PATH` is honored
// identically to a regular RPC call.
let userDataPath: string
try {
userDataPath = getDefaultUserDataPath()
} catch {
return
}
const metadata = tryReadMetadata(userDataPath)
if (!metadata) {
return
}
const transport = findTransport(metadata, 'unix', 'named-pipe')
if (!transport) {
return
}
const send = senderOverride ?? defaultSender
await send(metadata, transport.endpoint, {
method: 'telemetry.captureCliEvent',
params: {
name: 'cli_feature_used',
props: { feature_group: group, exit_status: exitStatus }
}
})
}
// A purpose-built minimal RPC sender for telemetry. It diverges from
// `runtime/transport.ts` in two important ways:
// 1. The socket is `unref()`ed so a still-in-flight send never holds the
// Node process open after the user-visible command has finished. A
// short-lived `orca worktree list` exits as soon as `printResult`
// returns, not when telemetry round-trips.
// 2. The timeout is `unref()`ed for the same reason. Without this, a
// pending timer would keep the event loop alive even after the socket
// is unref'd.
// We do not parse / validate the response — the CLI doesn't care whether
// main accepted, dropped at the consent gate, or rejected at the validator.
// The contract is: post the event and walk away.
const TELEMETRY_RPC_TIMEOUT_MS = 2_000
const defaultSender: Sender = async (metadata, endpoint, body) => {
await new Promise<void>((resolve) => {
let settled = false
const settle = (): void => {
if (settled) {
return
}
settled = true
resolve()
}
let socket: ReturnType<typeof createConnection>
try {
socket = createConnection(endpoint)
} catch {
// `createConnection` typically does not throw synchronously, but
// belt-and-braces — a malformed endpoint string would surface here.
settle()
return
}
socket.unref()
// Discard whatever the server sends back. We only care about the
// connect → write → end lifecycle for unrefing purposes; reading the
// response is incidental and any read errors are themselves a "swallow"
// case.
socket.on('error', settle)
socket.on('close', settle)
socket.on('end', settle)
// Drain incoming data so the kernel buffer doesn't sit around.
socket.resume()
const timeout = setTimeout(() => {
socket.destroy()
settle()
}, TELEMETRY_RPC_TIMEOUT_MS)
timeout.unref()
socket.on('connect', () => {
try {
socket.write(
`${JSON.stringify({
id: randomUUID(),
authToken: metadata.authToken,
method: body.method,
params: body.params
})}\n`,
() => {
// Half-close: signals "no more requests on this socket" so main
// can release its handler slot promptly. The server's reply (if
// any) is what triggers `end`/`close` above and resolves us.
socket.end()
}
)
} catch {
socket.destroy()
settle()
}
})
})
}
+3 -1
View File
@@ -8,6 +8,7 @@ import { BROWSER_EXTRA_METHODS } from './browser-extras'
import { ORCHESTRATION_METHODS } from './orchestration'
import { NOTIFICATION_METHODS } from './notifications'
import { STATS_METHODS } from './stats'
import { TELEMETRY_METHODS } from './telemetry'
// Why: a flat manifest keeps registration order explicit and provides one
// grep-point for "what methods does the RPC server expose?" — useful when
@@ -21,5 +22,6 @@ export const ALL_RPC_METHODS: readonly RpcAnyMethod[] = [
...BROWSER_EXTRA_METHODS,
...ORCHESTRATION_METHODS,
...NOTIFICATION_METHODS,
...STATS_METHODS
...STATS_METHODS,
...TELEMETRY_METHODS
]
@@ -0,0 +1,131 @@
// Coverage for the CLI-facing telemetry RPC method. Two invariants the
// system depends on:
// 1. A CLI-originated payload that names a non-CLI event is dropped
// silently — the CLI must not be a privilege-escalation surface for
// main-owned events (`agent_started`, `telemetry_opted_in`, etc.).
// 2. A schema-invalid payload is dropped without throwing — the CLI
// gets back `{}` and never learns the request was rejected, so
// telemetry health cannot leak through the wire shape.
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { TELEMETRY_METHODS } from './telemetry'
import { buildRegistry, type RpcContext } from '../core'
import { OrcaRuntimeService } from '../../orca-runtime'
const trackMock = vi.fn()
vi.mock('../../../telemetry/client', () => ({
track: (name: string, props: unknown) => trackMock(name, props)
}))
describe('telemetry RPC methods', () => {
let ctx: RpcContext
beforeEach(() => {
trackMock.mockReset()
ctx = { runtime: new OrcaRuntimeService() }
})
afterEach(() => {
trackMock.mockReset()
})
function findMethod(name: string) {
const method = TELEMETRY_METHODS.find((m) => m.name === name)
if (!method) {
throw new Error(`Method not found: ${name}`)
}
return method
}
async function call(name: string, params: Record<string, unknown>) {
const method = findMethod(name)
const parsed = method.params ? method.params.parse(params) : undefined
return method.handler(parsed, ctx)
}
it('registers telemetry.captureCliEvent', () => {
const registry = buildRegistry([...TELEMETRY_METHODS])
expect(registry.has('telemetry.captureCliEvent')).toBe(true)
})
it('forwards a valid cli_feature_used payload to track()', async () => {
await call('telemetry.captureCliEvent', {
name: 'cli_feature_used',
props: { feature_group: 'browser_observation', exit_status: 'success' }
})
expect(trackMock).toHaveBeenCalledTimes(1)
expect(trackMock).toHaveBeenCalledWith('cli_feature_used', {
feature_group: 'browser_observation',
exit_status: 'success'
})
})
// The narrow at the boundary: a compromised or buggy CLI must not be
// able to emit main-owned events. Without this drop, the CLI's auth
// token would let it ship `agent_started` / `telemetry_opted_in` /
// anything else in the registry.
it('drops non-CLI event names without calling track()', async () => {
await call('telemetry.captureCliEvent', {
name: 'agent_started',
props: { agent_kind: 'claude-code', launch_source: 'sidebar', request_kind: 'new' }
})
await call('telemetry.captureCliEvent', {
name: 'telemetry_opted_in',
props: { via: 'first_launch_banner' }
})
await call('telemetry.captureCliEvent', {
name: 'app_opened',
props: {}
})
expect(trackMock).not.toHaveBeenCalled()
})
it('drops schema-invalid props without calling track()', async () => {
// Unknown feature_group
await call('telemetry.captureCliEvent', {
name: 'cli_feature_used',
props: { feature_group: 'made_up_group', exit_status: 'success' }
})
// Missing exit_status
await call('telemetry.captureCliEvent', {
name: 'cli_feature_used',
props: { feature_group: 'discovery' }
})
// Extra key (would leak free-form content if forwarded)
await call('telemetry.captureCliEvent', {
name: 'cli_feature_used',
props: {
feature_group: 'browser_navigation',
exit_status: 'failure',
error_message: 'ENOENT'
}
})
expect(trackMock).not.toHaveBeenCalled()
})
it('rejects an empty event name at the params boundary', () => {
const method = findMethod('telemetry.captureCliEvent')
expect(() =>
method.params!.parse({
name: '',
props: { feature_group: 'discovery', exit_status: 'success' }
})
).toThrow(/Missing event name/)
})
it('returns an empty object on success (not raw track() output)', async () => {
const result = await call('telemetry.captureCliEvent', {
name: 'cli_feature_used',
props: { feature_group: 'discovery', exit_status: 'success' }
})
expect(result).toEqual({})
})
it('returns an empty object on a rejected (non-CLI) event', async () => {
const result = await call('telemetry.captureCliEvent', {
name: 'agent_started',
props: { agent_kind: 'claude-code', launch_source: 'sidebar', request_kind: 'new' }
})
expect(result).toEqual({})
})
})
+82
View File
@@ -0,0 +1,82 @@
// CLI-side telemetry funnels through this RPC method into main's `track()`.
// The CLI process never talks to PostHog directly — there is one write key,
// one consent gate, one validator, one wire identity (install_id, app_version,
// session_id, …) attached on the main side. This file is the narrow boundary
// where a CLI-originated event becomes a typed `track()` call.
//
// Threat model: the CLI is a separate process and the RPC socket is
// auth-token gated, but we still narrow the incoming `name` to a CLI subset
// (`isCliEventName`). A buggy or compromised CLI must not be able to spoof
// `agent_started` / `telemetry_opted_in` / etc. by posting them through this
// channel — those are main-owned events.
//
// Not in `src/main/ipc/telemetry.ts`: that file is renderer-only (Electron
// `ipcMain.handle()`). The CLI talks to main over the UNIX-socket / named-
// pipe runtime RPC dispatcher, not Electron IPC.
import { z } from 'zod'
import { defineMethod, type RpcMethod } from '../core'
import { track } from '../../../telemetry/client'
import {
cliFeatureGroupSchema,
isCliEventName,
type CliEventName,
type EventProps
} from '../../../../shared/telemetry-events'
// Per-CLI-event property schemas. Kept here (not in `telemetry-events.ts`)
// because they describe what the RPC method accepts on the wire — the
// authoritative event-shape validator inside `track()` re-checks these
// against `eventSchemas[name]`. This is the boundary type-narrow; that is
// the wire-contract enforcement.
const CliFeatureUsedProps = z
.object({
feature_group: cliFeatureGroupSchema,
exit_status: z.enum(['success', 'failure'])
})
.strict()
const CaptureCliEventParams = z.object({
name: z
.unknown()
.transform((value) => (typeof value === 'string' ? value : ''))
.pipe(z.string().min(1, 'Missing event name')),
props: z.unknown()
})
export const TELEMETRY_METHODS: RpcMethod[] = [
defineMethod({
name: 'telemetry.captureCliEvent',
params: CaptureCliEventParams,
handler: (params) => {
// Narrow `name` to the CLI subset before any further work. A non-CLI
// event name is silently dropped — the CLI must never learn whether
// a name was rejected (that would be a side-channel for probing the
// server's allow-list), and the validator inside `track()` would
// drop it anyway. Returning `{}` keeps the wire shape consistent
// with other `defineMethod` handlers.
if (!isCliEventName(params.name)) {
return {}
}
const name: CliEventName = params.name
// Per-event prop narrowing. Only `cli_feature_used` exists today.
// Adding a new CLI event means adding a branch here AND a schema in
// `eventSchemas`; the type system makes the omission a `tsc` error
// because `name` is the discriminated `CliEventName` union.
switch (name) {
case 'cli_feature_used': {
const parsed = CliFeatureUsedProps.safeParse(params.props)
if (!parsed.success) {
return {}
}
// The cast is the boundary handoff: `track()` re-validates
// against `eventSchemas.cli_feature_used` (the single source of
// truth) so this assignment is "I have already checked the
// shape, hand it to the central validator."
track(name, parsed.data satisfies EventProps<'cli_feature_used'>)
return {}
}
}
}
})
]
+82
View File
@@ -7,9 +7,12 @@ import { describe, expect, it } from 'vitest'
import {
AGENT_KIND_VALUES,
agentKindSchema,
CLI_EVENT_NAMES,
CLI_FEATURE_GROUP_VALUES,
commonPropsSchema,
errorClassSchema,
eventSchemas,
isCliEventName,
SETTINGS_CHANGED_WHITELIST,
settingsChangedKeySchema
} from './telemetry-events'
@@ -188,6 +191,85 @@ describe('commonPropsSchema', () => {
})
})
describe('cli_feature_used schema', () => {
it('round-trips a minimal valid payload', () => {
const parsed = eventSchemas.cli_feature_used.safeParse({
feature_group: 'browser_observation',
exit_status: 'success'
})
expect(parsed.success).toBe(true)
})
it('accepts every declared feature_group value', () => {
for (const group of CLI_FEATURE_GROUP_VALUES) {
const parsed = eventSchemas.cli_feature_used.safeParse({
feature_group: group,
exit_status: 'success'
})
expect(parsed.success).toBe(true)
}
})
it('rejects unknown feature_group values', () => {
const parsed = eventSchemas.cli_feature_used.safeParse({
feature_group: 'made_up_group',
exit_status: 'success'
})
expect(parsed.success).toBe(false)
})
it('rejects unknown exit_status values', () => {
const parsed = eventSchemas.cli_feature_used.safeParse({
feature_group: 'discovery',
exit_status: 'partial'
})
expect(parsed.success).toBe(false)
})
// The whole point of the wire-shape constraint: the CLI never transmits a
// raw command name, URL, error message, or path. `.strict()` enforces this
// at the schema level — any extra key drops the event.
it('rejects extra keys via .strict() (no command name leak, no error_message)', () => {
const withCommand = eventSchemas.cli_feature_used.safeParse({
feature_group: 'browser_navigation',
exit_status: 'success',
command: 'goto'
})
expect(withCommand.success).toBe(false)
const withErrorMessage = eventSchemas.cli_feature_used.safeParse({
feature_group: 'browser_navigation',
exit_status: 'failure',
error_message: 'ENOENT: no such file'
})
expect(withErrorMessage.success).toBe(false)
})
})
describe('isCliEventName', () => {
it('accepts every CLI_EVENT_NAMES entry', () => {
for (const name of CLI_EVENT_NAMES) {
expect(isCliEventName(name)).toBe(true)
}
})
// The narrow exists so a CLI process cannot spoof main-owned events
// (`agent_started`, `telemetry_opted_in`, etc.) via the runtime RPC
// method. If these ever pass, `methods/telemetry.ts` would be a
// privilege-escalation surface.
it('rejects main-owned event names', () => {
expect(isCliEventName('app_opened')).toBe(false)
expect(isCliEventName('agent_started')).toBe(false)
expect(isCliEventName('telemetry_opted_in')).toBe(false)
expect(isCliEventName('settings_changed')).toBe(false)
})
it('rejects unknown event names', () => {
expect(isCliEventName('made_up_event')).toBe(false)
expect(isCliEventName('')).toBe(false)
})
})
describe('exported enum schemas', () => {
it('agentKindSchema accepts the known product IDs', () => {
for (const kind of AGENT_KIND_VALUES) {
+44 -1
View File
@@ -110,6 +110,27 @@ export type RequestKind = z.infer<typeof requestKindSchema>
export const optInViaSchema = z.enum(['first_launch_banner', 'settings'])
export type OptInVia = z.infer<typeof optInViaSchema>
// CLI feature groups for `cli_feature_used`. Coarse buckets — finer
// per-command granularity is intentionally deferred (see
// docs/cli-telemetry-design.md §"What we explicitly do not ship"). The CLI
// process dedupes per `(feature_group, process)`, so a single agent loop
// that snapshots the page hundreds of times produces exactly one
// `browser_observation` event for that whole process.
export const CLI_FEATURE_GROUP_VALUES = [
'worktree_orchestration',
'terminal_orchestration',
'terminal_io',
'browser_navigation',
'browser_observation',
'browser_interaction',
'browser_config',
'orchestration_coordinator',
'orchestration_messaging',
'discovery'
] as const
export const cliFeatureGroupSchema = z.enum(CLI_FEATURE_GROUP_VALUES)
export type CliFeatureGroup = z.infer<typeof cliFeatureGroupSchema>
// Whitelist of settings whose `setting_key` may be emitted on
// `settings_changed`. If a setting isn't in this list, we do not emit.
//
@@ -187,6 +208,16 @@ const settingsChangedSchema = z
const telemetryOptedInSchema = z.object({ via: optInViaSchema }).strict()
const telemetryOptedOutSchema = z.object({ via: optInViaSchema }).strict()
// First-write-wins per `(feature_group, CLI process)`: emitted on the first
// call to a group, carrying that call's `exit_status`; subsequent calls in
// the same process never re-emit. See docs/cli-telemetry-design.md §Events.
const cliFeatureUsedSchema = z
.object({
feature_group: cliFeatureGroupSchema,
exit_status: z.enum(['success', 'failure'])
})
.strict()
// ── Event registry: the one record the validator consumes ───────────────
//
// The validator does `eventSchemas[name].safeParse(props)`. `EventMap` is
@@ -212,9 +243,21 @@ export const eventSchemas = {
settings_changed: settingsChangedSchema,
telemetry_opted_in: telemetryOptedInSchema,
telemetry_opted_out: telemetryOptedOutSchema
telemetry_opted_out: telemetryOptedOutSchema,
cli_feature_used: cliFeatureUsedSchema
} as const
// CLI-eligible event subset: the runtime RPC method narrows the incoming
// `name` to this set so the CLI process can never emit non-CLI events
// (e.g. `agent_started`) by spoofing the wire. Keep aligned with the
// `cli_*` prefix convention.
export const CLI_EVENT_NAMES = ['cli_feature_used'] as const
export type CliEventName = (typeof CLI_EVENT_NAMES)[number]
export function isCliEventName(name: string): name is CliEventName {
return (CLI_EVENT_NAMES as readonly string[]).includes(name)
}
export type EventMap = { [N in keyof typeof eventSchemas]: z.infer<(typeof eventSchemas)[N]> }
export type EventName = keyof EventMap
export type EventProps<N extends EventName> = EventMap[N]