diff --git a/src/main/ipc/github.test.ts b/src/main/ipc/github.test.ts index 23c6afaa6..c527c805e 100644 --- a/src/main/ipc/github.test.ts +++ b/src/main/ipc/github.test.ts @@ -8,6 +8,10 @@ const { listWorkItemsMock, getAuthenticatedViewerMock, mergePRMock, + checkOrcaStarredMock, + starOrcaMock, + trackMock, + getCohortAtEmitMock, getAllWebContentsMock } = vi.hoisted(() => ({ handleMock: vi.fn(), @@ -17,6 +21,10 @@ const { listWorkItemsMock: vi.fn(), getAuthenticatedViewerMock: vi.fn(), mergePRMock: vi.fn(), + checkOrcaStarredMock: vi.fn(), + starOrcaMock: vi.fn(), + trackMock: vi.fn(), + getCohortAtEmitMock: vi.fn(), getAllWebContentsMock: vi.fn() })) @@ -35,7 +43,17 @@ vi.mock('../github/client', () => ({ listIssues: listIssuesMock, listWorkItems: listWorkItemsMock, getAuthenticatedViewer: getAuthenticatedViewerMock, - mergePR: mergePRMock + mergePR: mergePRMock, + checkOrcaStarred: checkOrcaStarredMock, + starOrca: starOrcaMock +})) + +vi.mock('../telemetry/client', () => ({ + track: trackMock +})) + +vi.mock('../telemetry/cohort-classifier', () => ({ + getCohortAtEmit: getCohortAtEmitMock })) import { registerGitHubHandlers } from './github' @@ -70,6 +88,11 @@ describe('registerGitHubHandlers', () => { listWorkItemsMock.mockReset() getAuthenticatedViewerMock.mockReset() mergePRMock.mockReset() + checkOrcaStarredMock.mockReset() + starOrcaMock.mockReset() + trackMock.mockReset() + getCohortAtEmitMock.mockReset() + getCohortAtEmitMock.mockReturnValue({ nth_repo_added: undefined }) getAllWebContentsMock.mockReset() getAllWebContentsMock.mockReturnValue([]) for (const key of Object.keys(handlers)) { @@ -270,4 +293,73 @@ describe('registerGitHubHandlers', () => { }) expect(getAuthenticatedViewerMock).toHaveBeenCalled() }) + + it('emits app_starred_orca once after a successful star with cohort context', async () => { + starOrcaMock.mockResolvedValue(true) + getCohortAtEmitMock.mockReturnValue({ nth_repo_added: 3 }) + + registerGitHubHandlers(store as never, stats as never) + + await expect(handlers['gh:starOrca'](null, 'settings')).resolves.toBe(true) + + expect(starOrcaMock).toHaveBeenCalledTimes(1) + expect(getCohortAtEmitMock).toHaveBeenCalledTimes(1) + expect(trackMock).toHaveBeenCalledTimes(1) + expect(trackMock).toHaveBeenCalledWith('app_starred_orca', { + source: 'settings', + nth_repo_added: 3 + }) + }) + + it('accepts every app star source for success telemetry', async () => { + starOrcaMock.mockResolvedValue(true) + + registerGitHubHandlers(store as never, stats as never) + + for (const source of ['star_nag', 'settings', 'landing'] as const) { + await expect(handlers['gh:starOrca'](null, source)).resolves.toBe(true) + } + + expect(trackMock).toHaveBeenCalledTimes(3) + expect(trackMock.mock.calls.map(([, props]) => props)).toEqual([ + { source: 'star_nag', nth_repo_added: undefined }, + { source: 'settings', nth_repo_added: undefined }, + { source: 'landing', nth_repo_added: undefined } + ]) + }) + + it('does not emit app_starred_orca when the star action returns false', async () => { + starOrcaMock.mockResolvedValue(false) + + registerGitHubHandlers(store as never, stats as never) + + await expect(handlers['gh:starOrca'](null, 'landing')).resolves.toBe(false) + + expect(starOrcaMock).toHaveBeenCalledTimes(1) + expect(trackMock).not.toHaveBeenCalled() + expect(getCohortAtEmitMock).not.toHaveBeenCalled() + }) + + it('does not emit app_starred_orca when the star action throws', async () => { + starOrcaMock.mockRejectedValue(new Error('gh failed')) + + registerGitHubHandlers(store as never, stats as never) + + await expect(handlers['gh:starOrca'](null, 'star_nag')).rejects.toThrow('gh failed') + + expect(trackMock).not.toHaveBeenCalled() + expect(getCohortAtEmitMock).not.toHaveBeenCalled() + }) + + it('preserves star result but skips telemetry for an invalid IPC source', async () => { + starOrcaMock.mockResolvedValue(true) + + registerGitHubHandlers(store as never, stats as never) + + await expect(handlers['gh:starOrca'](null, 'github_website')).resolves.toBe(true) + + expect(starOrcaMock).toHaveBeenCalledTimes(1) + expect(trackMock).not.toHaveBeenCalled() + expect(getCohortAtEmitMock).not.toHaveBeenCalled() + }) }) diff --git a/src/main/ipc/github.ts b/src/main/ipc/github.ts index 332cecb7d..4eb3ab744 100644 --- a/src/main/ipc/github.ts +++ b/src/main/ipc/github.ts @@ -92,6 +92,9 @@ import type { UpdateProjectItemFieldArgs, UpdatePullRequestBySlugArgs } from '../../shared/github-project-types' +import { appStarSourceSchema } from '../../shared/gh-star-source' +import { track } from '../telemetry/client' +import { getCohortAtEmit } from '../telemetry/cohort-classifier' // Why: notify every renderer (each window has its own SWR cache instance) // that a work item was mutated locally so they can drop their cached entry @@ -844,7 +847,19 @@ export function registerGitHubHandlers(store: Store, stats: StatsCollector): voi // Star operations target the Orca repo itself — no repoPath validation needed ipcMain.handle('gh:viewer', () => getAuthenticatedViewer()) ipcMain.handle('gh:checkOrcaStarred', () => checkOrcaStarred()) - ipcMain.handle('gh:starOrca', () => starOrca()) + ipcMain.handle('gh:starOrca', async (_event, source: unknown) => { + const sourceParse = appStarSourceSchema.safeParse(source) + const starred = await starOrca() + if (starred && sourceParse.success) { + // Why: this main-owned event bypasses renderer telemetry IPC, so cohort + // context must be attached here on the successful star path. + track('app_starred_orca', { + source: sourceParse.data, + ...getCohortAtEmit() + }) + } + return starred + }) // Why: `rate_limit` is exempt from GitHub's rate-limit accounting, so // polling is cheap. A 30s in-process cache still avoids the gh subprocess diff --git a/src/main/ipc/telemetry.test.ts b/src/main/ipc/telemetry.test.ts index 3499ac466..4d02a8d1d 100644 --- a/src/main/ipc/telemetry.test.ts +++ b/src/main/ipc/telemetry.test.ts @@ -156,6 +156,14 @@ describe('telemetry IPC handlers', () => { }) }) + it('drops main-owned events from renderer telemetry IPC', () => { + registerWith({ installId: 'x', existedBeforeTelemetryRelease: false, optedIn: true }) + const handler = handlers.get('telemetry:track')! + handler({}, 'app_starred_orca', { source: 'settings' }) + expect(trackMock).not.toHaveBeenCalled() + expect(getCohortAtEmitMock).not.toHaveBeenCalled() + }) + it('injects cohort for setup script prompt events', () => { registerWith({ installId: 'x', existedBeforeTelemetryRelease: false, optedIn: true }) getCohortAtEmitMock.mockReturnValue({ nth_repo_added: 3 }) diff --git a/src/main/ipc/telemetry.ts b/src/main/ipc/telemetry.ts index 29d57536d..c5fe79b83 100644 --- a/src/main/ipc/telemetry.ts +++ b/src/main/ipc/telemetry.ts @@ -53,6 +53,8 @@ import type { OptInVia } from '../../shared/telemetry-events' // mirrors how other core-handlers accept the store explicitly. let storeRef: Store | null = null +const MAIN_OWNED_TELEMETRY_EVENTS = new Set(['app_starred_orca']) + /** * Derive the `via` discriminator for a `telemetry:setOptIn` call from * main-owned state. Called BEFORE any state mutation so the cohort + opt-in @@ -117,6 +119,13 @@ export function registerTelemetryHandlers(store: Store): void { if (props !== null && props !== undefined && typeof props !== 'object') { return } + const eventName = name as EventName + // Why: some event schemas are registered for main-owned emissions only. + // Letting renderer IPC emit them would let compromised content spoof + // product outcomes that must be tied to a successful main-side action. + if (MAIN_OWNED_TELEMETRY_EVENTS.has(eventName)) { + return + } // Inject cohort here, at the IPC entry, only for events whose schemas // declare `nth_repo_added` (see `COHORT_EXTENDED` in telemetry-events.ts). // The selectivity is load-bearing: schemas are `.strict()`, so adding @@ -130,7 +139,6 @@ export function registerTelemetryHandlers(store: Store): void { // The two injection sets are disjoint by construction today — no schema // declares both `nth_repo_added` and `cohort` — but combining them via // spread keeps that an additive change rather than a structural one. - const eventName = name as EventName const baseProps = (props ?? {}) as Record const withRepoCohort = isCohortExtendedEvent(eventName) ? { ...baseProps, ...getCohortAtEmit() } diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 4a3c56dac..c9fc9ca9b 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -272,6 +272,7 @@ import type { } from '../shared/opencode-usage-types' import type { TelemetryConsentState } from '../shared/telemetry-consent-types' import type { AgentKind, LaunchSource, RequestKind } from '../shared/telemetry-events' +import type { AppStarSource } from '../shared/gh-star-source' import type { RemoteWorkspaceChangedEvent, RemoteWorkspaceConnectedClient, @@ -1072,7 +1073,7 @@ export type PreloadApi = { }) => void ) => () => void checkOrcaStarred: () => Promise - starOrca: () => Promise + starOrca: (source: AppStarSource) => Promise /** * GitHub API rate-limit snapshot. Does NOT consume quota (the * `rate_limit` endpoint is exempt). Cached 30s server-side — pass diff --git a/src/preload/index.ts b/src/preload/index.ts index 9a0e8a515..1b52641f5 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -120,6 +120,7 @@ import type { import type { TelemetryConsentState } from '../shared/telemetry-consent-types' import type { RefreshAgentsResult } from './api-types' import type { AgentKind, LaunchSource, RequestKind } from '../shared/telemetry-events' +import type { AppStarSource } from '../shared/gh-star-source' import type { Automation, AutomationCreateInput, @@ -1164,7 +1165,8 @@ const api = { }, checkOrcaStarred: (): Promise => ipcRenderer.invoke('gh:checkOrcaStarred'), - starOrca: (): Promise => ipcRenderer.invoke('gh:starOrca'), + starOrca: (source: AppStarSource): Promise => + ipcRenderer.invoke('gh:starOrca', source), // Why: rate_limit is exempt from rate-limit accounting, but we still pass // `force` through so callers can bust the 30s in-process cache after a diff --git a/src/renderer/src/components/Landing.tsx b/src/renderer/src/components/Landing.tsx index 3d0ac6513..a2b9652ba 100644 --- a/src/renderer/src/components/Landing.tsx +++ b/src/renderer/src/components/Landing.tsx @@ -104,7 +104,7 @@ function GitHubStarButton({ hasRepos }: { hasRepos: boolean }): React.JSX.Elemen return } setState('starred') // optimistic - const ok = await window.api.gh.starOrca() + const ok = await window.api.gh.starOrca('landing') if (!ok) { setState('not-starred') return diff --git a/src/renderer/src/components/StarNagCard.tsx b/src/renderer/src/components/StarNagCard.tsx index cc8ee1f8c..43f78576b 100644 --- a/src/renderer/src/components/StarNagCard.tsx +++ b/src/renderer/src/components/StarNagCard.tsx @@ -66,7 +66,7 @@ export function StarNagCard(): React.JSX.Element | null { } setBusy(true) setError(false) - const ok = await window.api.gh.starOrca() + const ok = await window.api.gh.starOrca('star_nag') setBusy(false) if (!ok) { setError(true) diff --git a/src/renderer/src/components/settings/GeneralPane.tsx b/src/renderer/src/components/settings/GeneralPane.tsx index d16a2b3f9..2937d2391 100644 --- a/src/renderer/src/components/settings/GeneralPane.tsx +++ b/src/renderer/src/components/settings/GeneralPane.tsx @@ -153,7 +153,7 @@ export function GeneralPane({ settings, updateSettings }: GeneralPaneProps): Rea return } setStarState('starring') - const ok = await window.api.gh.starOrca() + const ok = await window.api.gh.starOrca('settings') if (!ok) { setStarState('error') return diff --git a/src/shared/gh-star-source.ts b/src/shared/gh-star-source.ts new file mode 100644 index 000000000..4e11266af --- /dev/null +++ b/src/shared/gh-star-source.ts @@ -0,0 +1,8 @@ +import { z } from 'zod' + +export const APP_STAR_SOURCE_VALUES = ['star_nag', 'settings', 'landing'] as const + +// Why: renderer-originated IPC is untrusted, so main validates against this +// closed enum before attaching source context to successful star telemetry. +export const appStarSourceSchema = z.enum(APP_STAR_SOURCE_VALUES) +export type AppStarSource = z.infer diff --git a/src/shared/telemetry-events.test.ts b/src/shared/telemetry-events.test.ts index 37972dd91..29e917216 100644 --- a/src/shared/telemetry-events.test.ts +++ b/src/shared/telemetry-events.test.ts @@ -14,6 +14,39 @@ import { SETTINGS_CHANGED_WHITELIST, settingsChangedKeySchema } from './telemetry-events' +import { appStarSourceSchema } from './gh-star-source' + +describe('app_starred_orca schema', () => { + it('accepts every declared app star source', () => { + for (const source of appStarSourceSchema.options) { + const parsed = eventSchemas.app_starred_orca.safeParse({ source }) + expect(parsed.success).toBe(true) + } + }) + + it('accepts cohort context on successful app star telemetry', () => { + const parsed = eventSchemas.app_starred_orca.safeParse({ + source: 'settings', + nth_repo_added: 2 + }) + expect(parsed.success).toBe(true) + }) + + it('rejects unknown app star source values', () => { + const parsed = eventSchemas.app_starred_orca.safeParse({ + source: 'github_website' + }) + expect(parsed.success).toBe(false) + }) + + it('rejects extra keys via .strict()', () => { + const parsed = eventSchemas.app_starred_orca.safeParse({ + source: 'landing', + repo: 'stablyai/orca' + }) + expect(parsed.success).toBe(false) + }) +}) describe('agent_error schema', () => { it('round-trips a minimal {error_class, agent_kind} payload', () => { diff --git a/src/shared/telemetry-events.ts b/src/shared/telemetry-events.ts index c3439c4d5..fb62716ce 100644 --- a/src/shared/telemetry-events.ts +++ b/src/shared/telemetry-events.ts @@ -18,6 +18,7 @@ import { FEATURE_WALL_MAX_DWELL_MS } from './feature-wall-telemetry' import { FEATURE_WALL_EXIT_ACTIONS, FEATURE_WALL_TOUR_DEPTH_STEPS } from './feature-wall-tour-depth' import { SETUP_SCRIPT_IMPORT_PROVIDERS } from './setup-script-import-providers' import { WORKSPACE_SOURCE_VALUES, type WorkspaceSource } from './workspace-source' +import { appStarSourceSchema } from './gh-star-source' import { NESTED_REPO_COUNT_BUCKETS, NESTED_REPO_IMPORT_ACTIONS, @@ -267,6 +268,13 @@ const repoAddedSchema = z .object({ method: repoMethodSchema, nth_repo_added: nthRepoAddedSchema }) .strict() +const appStarredOrcaSchema = z + .object({ + source: appStarSourceSchema, + nth_repo_added: nthRepoAddedSchema + }) + .strict() + const workspaceCreatedSchema = z .object({ source: workspaceSourceSchema, @@ -1014,6 +1022,7 @@ const onboardingFeatureSetupTerminalInteractedSchema = z // which cannot be unmixed after the fact. export const eventSchemas = { app_opened: appOpenedSchema, + app_starred_orca: appStarredOrcaSchema, repo_added: repoAddedSchema, add_repo_setup_step_action: addRepoSetupStepActionEventSchema, @@ -1107,6 +1116,7 @@ export const COHORT_EXTENDED: readonly EventName[] = Array.from(COHORT_EXTENDED_ // injection set against silent schema drift. type _CohortExtendedRoster = | 'app_opened' + | 'app_starred_orca' | 'repo_added' | 'add_repo_setup_step_action' | 'add_repo_existing_workspaces_detected'