From df67e25691f5eb55f087d4268761f1f05972d98e Mon Sep 17 00:00:00 2001 From: brennanb2025 Date: Thu, 28 May 2026 13:10:22 -0700 Subject: [PATCH] Fix feature interaction state writes Co-authored-by: Orca --- src/main/persistence.test.ts | 75 ++++++++++++++++- src/main/persistence.ts | 13 ++- .../dispatcher-feature-interactions.test.ts | 55 +++++++++++- src/main/runtime/rpc/dispatcher.ts | 22 +++-- .../runtime/rpc/methods/client-ui.test.ts | 15 ++++ src/main/runtime/rpc/methods/client-ui.ts | 4 +- .../components/browser-pane/BrowserPane.tsx | 83 +++++++++++++------ .../src/components/settings/AccountsPane.tsx | 4 +- .../src/runtime/runtime-rpc-client.test.ts | 66 +++++++++++++++ .../src/runtime/runtime-rpc-client.ts | 18 +++- .../src/store/slices/diffComments.test.ts | 8 ++ src/renderer/src/store/slices/diffComments.ts | 3 +- src/shared/feature-interactions.test.ts | 58 +++++++++++++ .../runtime-rpc-feature-interaction-source.ts | 25 ++++++ 14 files changed, 404 insertions(+), 45 deletions(-) create mode 100644 src/shared/runtime-rpc-feature-interaction-source.ts diff --git a/src/main/persistence.test.ts b/src/main/persistence.test.ts index b7d5b67ea11..ac6a7f57775 100644 --- a/src/main/persistence.test.ts +++ b/src/main/persistence.test.ts @@ -14,7 +14,13 @@ import { } from 'fs' import { join } from 'path' import { tmpdir } from 'os' -import type { Repo, TerminalTab, WorktreeLineage, WorkspaceSessionState } from '../shared/types' +import type { + PersistedState, + Repo, + TerminalTab, + WorktreeLineage, + WorkspaceSessionState +} from '../shared/types' import { isTerminalLeafId, makePaneKey } from '../shared/stable-pane-id' import { MAX_BROWSER_HISTORY_ENTRIES } from '../shared/workspace-session-browser-history' @@ -615,6 +621,35 @@ describe('Store', () => { expect(second.title).toBe('Nightly run 2') }) + it('records feature interactions when automations are created or manually queued', async () => { + const store = await createStore() + store.addRepo(makeRepo()) + const automation = store.createAutomation({ + name: 'Nightly', + prompt: 'Run checks', + agentId: 'claude', + projectId: 'r1', + workspaceMode: 'existing', + workspaceId: 'wt1', + timezone: 'UTC', + rrule: 'FREQ=DAILY;BYHOUR=9;BYMINUTE=0', + dtstart: new Date('2026-05-13T00:00:00Z').getTime() + }) + + store.createAutomationRun(automation, new Date('2026-05-13T09:00:00Z').getTime(), 'scheduled') + store.createAutomationRun(automation, new Date('2026-05-14T09:00:00Z').getTime(), 'manual') + + expect(store.getUI().featureInteractions?.['automation-created']?.interactionCount).toBe(1) + expect(store.getUI().featureInteractions?.['automation-run']?.interactionCount).toBe(1) + const persisted = readDataFile() as PersistedState + expect(persisted.ui?.featureInteractions?.['automation-created']).toMatchObject({ + interactionCount: 1 + }) + expect(persisted.ui?.featureInteractions?.['automation-run']).toMatchObject({ + interactionCount: 1 + }) + }) + it('snapshots automation run workspace names for deleted-workspace history', async () => { const store = await createStore() store.addRepo(makeRepo()) @@ -2124,6 +2159,44 @@ describe('Store', () => { }) }) + it('normalizes malformed persisted feature discovery state on read', async () => { + writeDataFile({ + schemaVersion: 1, + repos: [], + worktreeMeta: {}, + settings: {}, + ui: { + featureTipsSeenIds: ['voice-dictation', 'unknown-tip', 'voice-dictation'], + featureInteractions: { + tasks: { firstInteractedAt: 100 }, + automations: { firstInteractedAt: 150, interactionCount: 4 }, + browser: { firstInteractedAt: Number.NaN }, + unknown: { firstInteractedAt: 200 } + } + }, + githubCache: { pr: {}, issue: {} }, + workspaceSession: {} + }) + + const store = await createStore() + + expect(store.getUI().featureTipsSeenIds).toEqual(['voice-dictation']) + expect(store.getUI().featureInteractions).toEqual({ + tasks: { firstInteractedAt: 100, interactionCount: 1 }, + automations: { firstInteractedAt: 150, interactionCount: 4 } + }) + }) + + it('normalizes feature tip ids from direct UI writes', async () => { + const store = await createStore() + + store.updateUI({ + featureTipsSeenIds: ['voice-dictation', 'unknown-tip', 'voice-dictation'] as never + }) + + expect(store.getUI().featureTipsSeenIds).toEqual(['voice-dictation']) + }) + it('recordFeatureInteraction increments from the current persisted UI state', async () => { const store = await createStore() diff --git a/src/main/persistence.ts b/src/main/persistence.ts index 9dfdfe337c2..947cbeab4b7 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -89,6 +89,7 @@ import { normalizeFeatureInteractions, type FeatureInteractionId } from '../shared/feature-interactions' +import { normalizeFeatureTipIds } from '../shared/feature-tips' import { DEFAULT_WORKSPACE_STATUS_ID, clampWorkspaceBoardColumnWidth, @@ -2414,6 +2415,7 @@ export class Store { updatedAt: now } this.state.automations = [...(this.state.automations ?? []), automation] + this.recordFeatureInteraction('automation-created') this.flush() return automation } @@ -2512,6 +2514,9 @@ export class Store { createdAt: now } this.state.automationRuns = [...(this.state.automationRuns ?? []), run] + if (trigger === 'manual') { + this.recordFeatureInteraction('automation-run') + } this.flush() return run } @@ -2746,7 +2751,9 @@ export class Store { workspaceBoardCompact: normalizeWorkspaceBoardCompact(this.state.ui?.workspaceBoardCompact), workspaceBoardColumnWidth: clampWorkspaceBoardColumnWidth( this.state.ui?.workspaceBoardColumnWidth - ) + ), + featureTipsSeenIds: normalizeFeatureTipIds(this.state.ui?.featureTipsSeenIds), + featureInteractions: normalizeFeatureInteractions(this.state.ui?.featureInteractions) } } @@ -2781,6 +2788,10 @@ export class Store { workspaceBoardColumnWidth: clampWorkspaceBoardColumnWidth( updates.workspaceBoardColumnWidth ?? this.state.ui?.workspaceBoardColumnWidth ), + featureTipsSeenIds: + updates.featureTipsSeenIds !== undefined + ? normalizeFeatureTipIds(updates.featureTipsSeenIds) + : normalizeFeatureTipIds(this.state.ui?.featureTipsSeenIds), // Why: runtime RPCs and the renderer can both record education state. // Merge instead of replacing so a stale renderer snapshot cannot erase // runtime-only feature interactions. diff --git a/src/main/runtime/rpc/dispatcher-feature-interactions.test.ts b/src/main/runtime/rpc/dispatcher-feature-interactions.test.ts index 070468c9a13..87c127bbb1f 100644 --- a/src/main/runtime/rpc/dispatcher-feature-interactions.test.ts +++ b/src/main/runtime/rpc/dispatcher-feature-interactions.test.ts @@ -2,6 +2,10 @@ import { describe, expect, it, vi } from 'vitest' import { z } from 'zod' import type { PersistedUIState } from '../../../shared/types' import { getDefaultUIState } from '../../../shared/constants' +import { + ORCA_RUNTIME_RPC_BROWSER_UI_SOURCE, + ORCA_RUNTIME_RPC_FEATURE_INTERACTION_SOURCE_KEY +} from '../../../shared/runtime-rpc-feature-interaction-source' import { RpcDispatcher } from './dispatcher' import { defineMethod, defineStreamingMethod, type RpcRequest } from './core' import type { OrcaRuntimeService } from '../orca-runtime' @@ -43,6 +47,26 @@ const METHODS = [ params: z.object({}), handler: () => ({ clicked: true }) }), + defineMethod({ + name: 'browser.tabCreate', + params: z.object({}), + handler: () => ({ browserPageId: 'page-1' }) + }), + defineMethod({ + name: 'browser.tabShow', + params: z.object({}), + handler: () => ({ tab: { id: 'page-1' } }) + }), + defineMethod({ + name: 'browser.viewport', + params: z.object({}), + handler: () => ({ ok: true }) + }), + defineMethod({ + name: 'browser.eval', + params: z.object({}), + handler: () => ({ value: 'ok' }) + }), defineStreamingMethod({ name: 'browser.screencast', params: z.object({}), @@ -137,24 +161,28 @@ describe('RpcDispatcher feature interactions', () => { expect(runtime.recordFeatureInteraction).not.toHaveBeenCalled() }) - it('records screencast starts but not screencast cleanup', async () => { + it('records unmarked browser display RPCs as agent browser use', async () => { const runtime = makeRuntime() const dispatcher = new RpcDispatcher({ runtime, methods: METHODS }) const replies: string[] = [] + await dispatcher.dispatch(makeRequest('browser.tabCreate')) + await dispatcher.dispatch(makeRequest('browser.tabShow')) await dispatcher.dispatch(makeRequest('browser.screencast.unsubscribe')) - expect(runtime.recordFeatureInteraction).not.toHaveBeenCalled() + expect(runtime.recordFeatureInteraction).toHaveBeenCalledTimes(2) + expect(runtime.recordFeatureInteraction).toHaveBeenNthCalledWith(1, 'agent-browser-use') + expect(runtime.recordFeatureInteraction).toHaveBeenNthCalledWith(2, 'agent-browser-use') await dispatcher.dispatchStreaming(makeRequest('browser.screencast'), (response) => { replies.push(response) }) expect(replies).toHaveLength(2) + expect(runtime.recordFeatureInteraction).toHaveBeenCalledTimes(3) expect(runtime.recordFeatureInteraction).toHaveBeenLastCalledWith('agent-browser-use') - expect(runtime.recordFeatureInteraction).toHaveBeenCalledTimes(1) }) - it('records streaming runtime feature use when the stream only returns a start result', async () => { + it('records binary-only browser display streams as agent browser use', async () => { const runtime = makeRuntime() const dispatcher = new RpcDispatcher({ runtime, methods: METHODS }) const replies: string[] = [] @@ -168,6 +196,25 @@ describe('RpcDispatcher feature interactions', () => { expect(runtime.recordFeatureInteraction).toHaveBeenCalledWith('agent-browser-use') }) + it('does not record browser pane UI-originated browser RPCs as agent browser use', async () => { + const runtime = makeRuntime() + const dispatcher = new RpcDispatcher({ runtime, methods: METHODS }) + const browserPaneUiParams = { + [ORCA_RUNTIME_RPC_FEATURE_INTERACTION_SOURCE_KEY]: ORCA_RUNTIME_RPC_BROWSER_UI_SOURCE + } + + await dispatcher.dispatch(makeRequest('browser.viewport', browserPaneUiParams)) + await dispatcher.dispatch(makeRequest('browser.eval', browserPaneUiParams)) + await dispatcher.dispatchStreaming( + makeRequest('browser.screencast', browserPaneUiParams), + () => {} + ) + await dispatcher.dispatch(makeRequest('browser.click')) + + expect(runtime.recordFeatureInteraction).toHaveBeenCalledTimes(1) + expect(runtime.recordFeatureInteraction).toHaveBeenCalledWith('agent-browser-use') + }) + it('records each successful non-streaming runtime feature interaction', async () => { const runtime = makeRuntime() const dispatcher = new RpcDispatcher({ runtime, methods: METHODS }) diff --git a/src/main/runtime/rpc/dispatcher.ts b/src/main/runtime/rpc/dispatcher.ts index 2852830c5f8..add223a2d02 100644 --- a/src/main/runtime/rpc/dispatcher.ts +++ b/src/main/runtime/rpc/dispatcher.ts @@ -15,6 +15,7 @@ import { } from './core' import type { TerminalStreamFrame } from '../../../shared/terminal-stream-protocol' import type { FeatureInteractionId } from '../../../shared/feature-interactions' +import { isBrowserPaneUiRuntimeRpcParams } from '../../../shared/runtime-rpc-feature-interaction-source' import { errorResponse, mapBrowserError, mapRuntimeError, successResponse } from './errors' import { ALL_RPC_METHODS } from './methods' import type { OrcaRuntimeService } from '../orca-runtime' @@ -67,7 +68,7 @@ export class RpcDispatcher { runtime: this.runtime, signal: options?.signal }) - this.recordRuntimeFeatureInteraction(request.method, result) + this.recordRuntimeFeatureInteraction(request.method, result, undefined, request.params) return successResponse(request.id, meta, result) } catch (error) { return this.mapError(request, meta, error) @@ -118,7 +119,7 @@ export class RpcDispatcher { sendBinary: options?.sendBinary, registerBinaryStreamHandler: options?.registerBinaryStreamHandler }) - this.recordRuntimeFeatureInteraction(request.method, result) + this.recordRuntimeFeatureInteraction(request.method, result, undefined, request.params) reply(JSON.stringify(successResponse(request.id, meta, result))) } catch (error) { reply(JSON.stringify(this.mapError(request, meta, error))) @@ -131,7 +132,8 @@ export class RpcDispatcher { this.recordRuntimeFeatureInteraction( request.method, result, - recordedStreamingFeatureInteractions + recordedStreamingFeatureInteractions, + request.params ) const response = successResponse(request.id, meta, result) response.streaming = true @@ -154,7 +156,8 @@ export class RpcDispatcher { this.recordRuntimeFeatureInteraction( request.method, result, - recordedStreamingFeatureInteractions + recordedStreamingFeatureInteractions, + request.params ) } catch (error) { reply(JSON.stringify(this.mapError(request, meta, error))) @@ -200,9 +203,10 @@ export class RpcDispatcher { private recordRuntimeFeatureInteraction( method: string, result: unknown, - alreadyRecorded?: Set + alreadyRecorded?: Set, + rawParams?: unknown ): void { - const id = getRuntimeFeatureInteractionId(method, result) + const id = getRuntimeFeatureInteractionId(method, result, rawParams) if (!id) { return } @@ -220,7 +224,8 @@ export class RpcDispatcher { function getRuntimeFeatureInteractionId( method: string, - result: unknown + result: unknown, + rawParams?: unknown ): FeatureInteractionId | null { if (method === 'browser.profileImportFromBrowser') { return hasBooleanResult(result, 'ok') ? 'cookie-import' : null @@ -231,6 +236,9 @@ function getRuntimeFeatureInteractionId( if (method === 'browser.screencast.unsubscribe') { return null } + if (method.startsWith('browser.') && isBrowserPaneUiRuntimeRpcParams(rawParams)) { + return null + } if (method.startsWith('browser.') && !method.startsWith('browser.profile')) { return 'agent-browser-use' } diff --git a/src/main/runtime/rpc/methods/client-ui.test.ts b/src/main/runtime/rpc/methods/client-ui.test.ts index d090e7a11bc..48162cd9330 100644 --- a/src/main/runtime/rpc/methods/client-ui.test.ts +++ b/src/main/runtime/rpc/methods/client-ui.test.ts @@ -256,6 +256,21 @@ describe('client UI RPC methods', () => { expect(runtime.updateUIState).not.toHaveBeenCalled() }) + it('rejects unknown feature tip ids', async () => { + const runtime = { + getRuntimeId: () => 'test-runtime', + updateUIState: vi.fn() + } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: CLIENT_UI_METHODS }) + + const response = await dispatcher.dispatch( + makeRequest('ui.set', { featureTipsSeenIds: ['voice-dictation', 'unknown-tip'] }) + ) + + expect(response).toMatchObject({ ok: false, error: { code: 'invalid_argument' } }) + expect(runtime.updateUIState).not.toHaveBeenCalled() + }) + it('rejects unknown feature interaction ids for increment RPC', async () => { const runtime = { getRuntimeId: () => 'test-runtime', diff --git a/src/main/runtime/rpc/methods/client-ui.ts b/src/main/runtime/rpc/methods/client-ui.ts index 6981adf0460..2bfd8609759 100644 --- a/src/main/runtime/rpc/methods/client-ui.ts +++ b/src/main/runtime/rpc/methods/client-ui.ts @@ -3,12 +3,14 @@ import { isFeatureInteractionId, type FeatureInteractionId } from '../../../../shared/feature-interactions' +import { isFeatureTipId } from '../../../../shared/feature-tips' import { isTuiAgent } from '../../../../shared/tui-agent-config' import type { PersistedUIState } from '../../../../shared/types' import { defineMethod, type RpcMethod } from '../core' const NullableString = z.string().nullable() const StringArray = z.array(z.string()) +const FeatureTipIds = z.array(z.custom(isFeatureTipId, { message: 'Unknown feature tip id' })) const UnknownRecord = z.record(z.string(), z.unknown()) const UnknownRecordArray = z.array(UnknownRecord) const WorktreeCardProperty = z.enum([ @@ -186,7 +188,7 @@ const UiUpdate = z sidekickSize: z.number().finite().optional(), taskResumeState: TaskResumeState.optional(), workspaceCleanup: WorkspaceCleanup.optional(), - featureTipsSeenIds: StringArray.optional(), + featureTipsSeenIds: FeatureTipIds.optional(), featureInteractions: FeatureInteractions.optional() }) .strict() diff --git a/src/renderer/src/components/browser-pane/BrowserPane.tsx b/src/renderer/src/components/browser-pane/BrowserPane.tsx index 088156246bb..1d3cafa2108 100644 --- a/src/renderer/src/components/browser-pane/BrowserPane.tsx +++ b/src/renderer/src/components/browser-pane/BrowserPane.tsx @@ -138,6 +138,7 @@ import { decodeBrowserScreencastFrame, type BrowserScreencastFrameMetadata } from '../../../../shared/browser-screencast-protocol' +import { withBrowserPaneUiRuntimeRpcSource } from '../../../../shared/runtime-rpc-feature-interaction-source' import { formatByteCount, formatDownloadFinishedNotice, @@ -1078,7 +1079,7 @@ function RemoteBrowserPagePane({ deviceScaleFactor: getRemoteBrowserDeviceScaleFactor(), mobile: false }, - { timeoutMs: 15_000 } + { timeoutMs: 15_000, suppressFeatureInteraction: true } ) try { // Why: the streamed bitmap can include the host compositor surface, @@ -1091,7 +1092,7 @@ function RemoteBrowserPagePane({ page: pageId, expression: 'JSON.stringify({ width: window.innerWidth, height: window.innerHeight })' }, - { timeoutMs: 15_000 } + { timeoutMs: 15_000, suppressFeatureInteraction: true } ) remoteCssViewportSizeRef.current = readRemoteCssViewportSize(viewport) ?? size } catch { @@ -1303,7 +1304,7 @@ function RemoteBrowserPagePane({ { kind: 'environment', environmentId: removedHandle.environmentId }, 'browser.tabClose', { worktree: `id:${worktreeId}`, page: removedHandle.remotePageId }, - { timeoutMs: 15_000 } + { timeoutMs: 15_000, suppressFeatureInteraction: true } ).catch(() => {}) } }, [activeRuntimeEnvironmentId, browserTab.id, worktreeId]) @@ -1410,14 +1411,14 @@ function RemoteBrowserPagePane({ target, 'browser.tabCreate', { worktree: `id:${worktreeId}`, url: initialUrl }, - { timeoutMs: 30_000 } + { timeoutMs: 30_000, suppressFeatureInteraction: true } ) if (!isCurrentRemoteOperationToken(token)) { void callRuntimeRpc( target, 'browser.tabClose', { worktree: `id:${worktreeId}`, page: created.browserPageId }, - { timeoutMs: 15_000 } + { timeoutMs: 15_000, suppressFeatureInteraction: true } ).catch(() => {}) return null } @@ -1476,7 +1477,7 @@ function RemoteBrowserPagePane({ { kind: 'environment', environmentId: token.environmentId }, 'browser.tabShow', { worktree: `id:${worktreeId}`, page: token.remotePageId }, - { timeoutMs: 15_000 } + { timeoutMs: 15_000, suppressFeatureInteraction: true } ) return shown.tab }, @@ -1637,7 +1638,7 @@ function RemoteBrowserPagePane({ { selector: target.environmentId, method: 'browser.screencast', - params: { + params: withBrowserPaneUiRuntimeRpcSource({ worktree: `id:${worktreeId}`, page: pageId, format: 'jpeg', @@ -1648,7 +1649,7 @@ function RemoteBrowserPagePane({ viewportHeight: viewportSize?.height, deviceScaleFactor: getRemoteBrowserDeviceScaleFactor(), everyNthFrame: 2 - }, + }), timeoutMs: 15_000 }, { @@ -1910,7 +1911,7 @@ function RemoteBrowserPagePane({ : { worktree: `id:${worktreeId}`, page: pageId } const result = await callRuntimeRpc< BrowserGotoResult | BrowserBackResult | BrowserReloadResult - >(target, method, params, { timeoutMs: 30_000 }) + >(target, method, params, { timeoutMs: 30_000, suppressFeatureInteraction: true }) if (isCurrentRemoteOperationToken(pageToken)) { applyRemoteTabInfo(result) } @@ -2006,13 +2007,13 @@ function RemoteBrowserPagePane({ target, 'browser.mouseMove', { ...params, x: point.x, y: point.y }, - { timeoutMs: 15_000 } + { timeoutMs: 15_000, suppressFeatureInteraction: true } ) await callRuntimeRpc( target, 'browser.mouseDown', { ...params, button }, - { timeoutMs: 15_000 } + { timeoutMs: 15_000, suppressFeatureInteraction: true } ) } catch (error) { if (isCurrentRemoteOperationToken(operationToken)) { @@ -2053,13 +2054,13 @@ function RemoteBrowserPagePane({ target, 'browser.mouseMove', { ...params, x: point.x, y: point.y }, - { timeoutMs: 15_000 } + { timeoutMs: 15_000, suppressFeatureInteraction: true } ) await callRuntimeRpc( target, 'browser.mouseUp', { ...params, button }, - { timeoutMs: 15_000 } + { timeoutMs: 15_000, suppressFeatureInteraction: true } ) scheduleRemoteTabInfoRefresh(operationToken, 250) } catch (error) { @@ -2107,7 +2108,7 @@ function RemoteBrowserPagePane({ page: pageId, expression: buildRemoteContextMenuExpression(point.x, point.y) }, - { timeoutMs: 15_000 } + { timeoutMs: 15_000, suppressFeatureInteraction: true } ) const parsed = readRemoteContextMenuResult(result) if (parsed) { @@ -2155,7 +2156,12 @@ function RemoteBrowserPagePane({ return } try { - await callRuntimeRpc(target, 'browser.keypress', { ...params, key }, { timeoutMs: 15_000 }) + await callRuntimeRpc( + target, + 'browser.keypress', + { ...params, key }, + { timeoutMs: 15_000, suppressFeatureInteraction: true } + ) if (key === 'Enter' || key === 'Meta+r' || key === 'Control+r') { scheduleRemoteTabInfoRefresh(operationToken, 400) } @@ -2194,7 +2200,7 @@ function RemoteBrowserPagePane({ target, 'browser.mouseMove', { ...params, x: point.x, y: point.y }, - { timeoutMs: 15_000 } + { timeoutMs: 15_000, suppressFeatureInteraction: true } ) await callRuntimeRpc( target, @@ -2204,7 +2210,7 @@ function RemoteBrowserPagePane({ dx, dy }, - { timeoutMs: 15_000 } + { timeoutMs: 15_000, suppressFeatureInteraction: true } ) scheduleRemoteTabInfoRefresh(operationToken, 400) } catch (error) { @@ -2614,6 +2620,7 @@ function BrowserPagePane({ const addBrowserPageAnnotation = useAppStore((s) => s.addBrowserPageAnnotation) const deleteBrowserPageAnnotation = useAppStore((s) => s.deleteBrowserPageAnnotation) const clearBrowserPageAnnotations = useAppStore((s) => s.clearBrowserPageAnnotations) + const recordFeatureInteraction = useAppStore((s) => s.recordFeatureInteraction) const clearBrowserPageAnnotationsRef = useRef(clearBrowserPageAnnotations) clearBrowserPageAnnotationsRef.current = clearBrowserPageAnnotations const createBrowserTab = useAppStore((s) => s.createBrowserTab) @@ -3727,6 +3734,7 @@ function BrowserPagePane({ const startGrabIntent = useCallback( (nextIntent: GrabIntent): void => { + recordFeatureInteraction('browser-grab') setGrabIntent(nextIntent) if (nextIntent === 'copy') { setPendingAnnotationPayload(null) @@ -3737,7 +3745,7 @@ function BrowserPagePane({ grab.toggle() } }, - [grab, grabIntent] + [grab, grabIntent, recordFeatureInteraction] ) // CmdOrCtrl+C toggles grab mode @@ -3819,11 +3827,13 @@ function BrowserPagePane({ if (key === 'c') { const text = formatGrabPayloadAsText(payload) void window.api.ui.writeClipboardText(text) + recordFeatureInteraction('browser-grab') showGrabToast('Copied', 'success', payload) } else { const dataUrl = payload.screenshot?.dataUrl if (dataUrl?.startsWith('data:image/png;base64,')) { void window.api.ui.writeClipboardImage(dataUrl) + recordFeatureInteraction('browser-grab') showGrabToast('Screenshotted', 'success', payload) } else { showGrabToast('No screenshot available', 'error', payload) @@ -3877,7 +3887,7 @@ function BrowserPagePane({ })() } }, - [grab, grabIntent, showGrabToast] + [grab, grabIntent, recordFeatureInteraction, showGrabToast] ) useEffect(() => { @@ -3930,9 +3940,10 @@ function BrowserPagePane({ } const text = formatGrabPayloadAsText(payload) void window.api.ui.writeClipboardText(text) + recordFeatureInteraction('browser-grab') showGrabToast('Copied', 'success', payload) grab.rearm() - }, [grab, showGrabToast]) + }, [grab, recordFeatureInteraction, showGrabToast]) const handleGrabCopyScreenshot = useCallback(() => { grabMenuActionTakenRef.current = true @@ -3945,9 +3956,10 @@ function BrowserPagePane({ return } void window.api.ui.writeClipboardImage(dataUrl) + recordFeatureInteraction('browser-grab') showGrabToast('Screenshotted', 'success', payload) grab.rearm() - }, [grab, showGrabToast]) + }, [grab, recordFeatureInteraction, showGrabToast]) const handleAddBrowserAnnotation = useCallback( (comment: string, intent: BrowserAnnotationIntent): void => { @@ -3966,10 +3978,18 @@ function BrowserPagePane({ }) setPendingAnnotationPayload(null) setBrowserAnnotationTrayOpen(true) + recordFeatureInteraction('browser-annotations') showGrabToast('Annotation added', 'success', payload) grab.rearm() }, - [addBrowserPageAnnotation, browserTab.id, grab, pendingAnnotationPayload, showGrabToast] + [ + addBrowserPageAnnotation, + browserTab.id, + grab, + pendingAnnotationPayload, + recordFeatureInteraction, + showGrabToast + ] ) const handleCancelPendingBrowserAnnotation = useCallback((): void => { @@ -3984,16 +4004,29 @@ function BrowserPagePane({ return } void window.api.ui.writeClipboardText(browserAnnotationsPrompt) + recordFeatureInteraction('browser-annotations') clearTimeout(annotationCopyTimerRef.current) setBrowserAnnotationsCopied(true) annotationCopyTimerRef.current = setTimeout(() => setBrowserAnnotationsCopied(false), 1400) - }, [browserAnnotationsPrompt]) + }, [browserAnnotationsPrompt, recordFeatureInteraction]) const handleClearBrowserAnnotations = useCallback((): void => { + if (browserAnnotationsRef.current.length === 0) { + return + } clearTimeout(annotationCopyTimerRef.current) setBrowserAnnotationsCopied(false) clearBrowserPageAnnotations(browserTab.id) - }, [browserTab.id, clearBrowserPageAnnotations]) + recordFeatureInteraction('browser-annotations') + }, [browserTab.id, clearBrowserPageAnnotations, recordFeatureInteraction]) + + const handleDeleteBrowserAnnotation = useCallback( + (annotationId: string): void => { + deleteBrowserPageAnnotation(browserTab.id, annotationId) + recordFeatureInteraction('browser-annotations') + }, + [browserTab.id, deleteBrowserPageAnnotation, recordFeatureInteraction] + ) const navigateToUrl = useCallback( (url: string): void => { @@ -4827,7 +4860,7 @@ function BrowserPagePane({ size="icon-xs" variant="ghost" className="opacity-0 transition-opacity group-hover:opacity-100 focus-visible:opacity-100 group-focus-within:opacity-100" - onClick={() => deleteBrowserPageAnnotation(browserTab.id, annotation.id)} + onClick={() => handleDeleteBrowserAnnotation(annotation.id)} aria-label={`Delete annotation ${index + 1}`} > diff --git a/src/renderer/src/components/settings/AccountsPane.tsx b/src/renderer/src/components/settings/AccountsPane.tsx index 450eb9ee2ae..21518e22605 100644 --- a/src/renderer/src/components/settings/AccountsPane.tsx +++ b/src/renderer/src/components/settings/AccountsPane.tsx @@ -206,12 +206,12 @@ export function AccountsPane({ settings, updateSettings }: AccountsPaneProps): R action: typeof codexAction, operation: () => Promise ): Promise => { - recordFeatureInteraction('codex-account-switching') const previousActiveAccountId = codexAccounts.activeAccountId setCodexAction(action) try { const next = await operation() await syncCodexAccounts(next) + recordFeatureInteraction('codex-account-switching') const shouldPromptRestart = action === 'adding' || (action.startsWith('select:') && previousActiveAccountId !== next.activeAccountId) || @@ -238,12 +238,12 @@ export function AccountsPane({ settings, updateSettings }: AccountsPaneProps): R action: typeof claudeAction, operation: () => Promise ): Promise => { - recordFeatureInteraction('claude-account-switching') const previousActiveAccountId = claudeAccounts.activeAccountId setClaudeAction(action) try { const next = await operation() await syncClaudeAccounts(next) + recordFeatureInteraction('claude-account-switching') if (previousActiveAccountId !== next.activeAccountId || action === 'adding') { toast.info('Claude account updated.', { description: `${getClaudeAccountLabel(claudeAccounts, previousActiveAccountId)} → ${getClaudeAccountLabel(next, next.activeAccountId)}. Restart live Claude terminals before continuing old sessions.` diff --git a/src/renderer/src/runtime/runtime-rpc-client.test.ts b/src/renderer/src/runtime/runtime-rpc-client.test.ts index 21eec50c6c0..c5b67286b21 100644 --- a/src/renderer/src/runtime/runtime-rpc-client.test.ts +++ b/src/renderer/src/runtime/runtime-rpc-client.test.ts @@ -10,6 +10,10 @@ import { MIN_COMPATIBLE_RUNTIME_CLIENT_VERSION, RUNTIME_PROTOCOL_VERSION } from '../../../shared/protocol-version' +import { + ORCA_RUNTIME_RPC_BROWSER_UI_SOURCE, + ORCA_RUNTIME_RPC_FEATURE_INTERACTION_SOURCE_KEY +} from '../../../shared/runtime-rpc-feature-interaction-source' const runtimeCall = vi.fn() const runtimeEnvironmentCall = vi.fn() @@ -55,6 +59,31 @@ describe('runtime RPC client routing', () => { expect(runtimeEnvironmentCall).not.toHaveBeenCalled() }) + it('marks local UI-owned runtime calls so feature interaction tracking can ignore them', async () => { + runtimeCall.mockResolvedValue({ + id: 'local', + ok: true, + result: { ok: true }, + _meta: { runtimeId: 'local-runtime' } + }) + + await callRuntimeRpc( + { kind: 'local' }, + 'browser.viewport', + { page: 'page-1' }, + { suppressFeatureInteraction: true } + ) + + expect(runtimeCall).toHaveBeenCalledWith({ + method: 'browser.viewport', + params: { + page: 'page-1', + [ORCA_RUNTIME_RPC_FEATURE_INTERACTION_SOURCE_KEY]: ORCA_RUNTIME_RPC_BROWSER_UI_SOURCE + } + }) + expect(runtimeEnvironmentCall).not.toHaveBeenCalled() + }) + it('routes remote runtime calls through window.api.runtimeEnvironments.call', async () => { runtimeEnvironmentCall.mockResolvedValue({ id: 'remote', @@ -106,6 +135,43 @@ describe('runtime RPC client routing', () => { ]) }) + it('marks remote UI-owned runtime calls so feature interaction tracking can ignore them', async () => { + runtimeEnvironmentCall.mockImplementation(({ method }: { method: string }) => { + const result = + method === 'status.get' + ? { + runtimeId: 'remote-runtime', + graphStatus: 'ready', + runtimeProtocolVersion: RUNTIME_PROTOCOL_VERSION, + minCompatibleRuntimeClientVersion: MIN_COMPATIBLE_RUNTIME_CLIENT_VERSION + } + : { ok: true } + return Promise.resolve({ + id: method, + ok: true, + result, + _meta: { runtimeId: 'remote-runtime' } + }) + }) + + await callRuntimeRpc( + { kind: 'environment', environmentId: 'env-1' }, + 'browser.viewport', + { page: 'page-1' }, + { suppressFeatureInteraction: true } + ) + + expect(runtimeEnvironmentCall).toHaveBeenLastCalledWith({ + selector: 'env-1', + method: 'browser.viewport', + params: { + page: 'page-1', + [ORCA_RUNTIME_RPC_FEATURE_INTERACTION_SOURCE_KEY]: ORCA_RUNTIME_RPC_BROWSER_UI_SOURCE + }, + timeoutMs: undefined + }) + }) + it('caches successful remote compatibility checks per environment', async () => { runtimeEnvironmentCall.mockImplementation(({ method }: { method: string }) => { const result = diff --git a/src/renderer/src/runtime/runtime-rpc-client.ts b/src/renderer/src/runtime/runtime-rpc-client.ts index 46f0e9a1208..dd974e30363 100644 --- a/src/renderer/src/runtime/runtime-rpc-client.ts +++ b/src/renderer/src/runtime/runtime-rpc-client.ts @@ -1,6 +1,7 @@ import type { GlobalSettings } from '../../../shared/types' import type { RuntimeRpcFailure, RuntimeRpcResponse } from '../../../shared/runtime-rpc-envelope' import type { RuntimeStatus } from '../../../shared/runtime-types' +import { withBrowserPaneUiRuntimeRpcSource } from '../../../shared/runtime-rpc-feature-interaction-source' import { assertRuntimeStatusCompatible } from './runtime-protocol-compat' export type RuntimeClientTarget = { kind: 'local' } | { kind: 'environment'; environmentId: string } @@ -44,23 +45,34 @@ export async function callRuntimeRpc( target: RuntimeClientTarget, method: string, params?: unknown, - options: { timeoutMs?: number } = {} + options: { timeoutMs?: number; suppressFeatureInteraction?: boolean } = {} ): Promise { if (target.kind === 'environment' && method !== 'status.get') { await ensureRuntimeEnvironmentCompatible(target.environmentId, options.timeoutMs) } + const nextParams = addFeatureInteractionSource(params, options) const response = target.kind === 'local' - ? await window.api.runtime.call({ method, params }) + ? await window.api.runtime.call({ method, params: nextParams }) : await window.api.runtimeEnvironments.call({ selector: target.environmentId, method, - params, + params: nextParams, timeoutMs: options.timeoutMs }) return unwrapRuntimeRpcResult(response as RuntimeRpcResponse) } +function addFeatureInteractionSource( + params: unknown, + options: { suppressFeatureInteraction?: boolean } +): unknown { + if (!options.suppressFeatureInteraction) { + return params + } + return withBrowserPaneUiRuntimeRpcSource(params) +} + async function ensureRuntimeEnvironmentCompatible( environmentId: string, timeoutMs?: number diff --git a/src/renderer/src/store/slices/diffComments.test.ts b/src/renderer/src/store/slices/diffComments.test.ts index b757e9b714a..a7bf5b3d4f0 100644 --- a/src/renderer/src/store/slices/diffComments.test.ts +++ b/src/renderer/src/store/slices/diffComments.test.ts @@ -29,6 +29,10 @@ const runtimeEnvironmentCall = vi.fn().mockResolvedValue({ }) const runtimeEnvironmentTransportCall = vi.fn() const mockApi = { + ui: { + recordFeatureInteraction: vi.fn().mockResolvedValue({ featureInteractions: {} }), + set: vi.fn().mockResolvedValue(undefined) + }, worktrees: { list: vi.fn().mockResolvedValue([]), create: vi.fn().mockResolvedValue({}), @@ -434,6 +438,7 @@ describe('markDiffCommentsSent', () => { it('marks selected notes as sent and persists once', async () => { const store = createTestStore() + store.setState({ persistedUIReady: true }) seed(store, [ makeComment({ id: 'c1', filePath: 'src/foo.ts' }), makeComment({ id: 'c2', filePath: 'src/bar.ts' }) @@ -453,6 +458,9 @@ describe('markDiffCommentsSent', () => { diffComments: [expect.objectContaining({ id: 'c1', sentAt: 3000 }), expect.any(Object)] } }) + expect(store.getState().featureInteractions['review-notes']).toEqual( + expect.objectContaining({ interactionCount: 1 }) + ) }) it('returns success without persisting when no selected notes match', async () => { diff --git a/src/renderer/src/store/slices/diffComments.ts b/src/renderer/src/store/slices/diffComments.ts index 06a3b36330b..09c9afdc32e 100644 --- a/src/renderer/src/store/slices/diffComments.ts +++ b/src/renderer/src/store/slices/diffComments.ts @@ -341,7 +341,6 @@ export const createDiffCommentsSlice: StateCreator type ExtraFeatureInteractionId = Exclude +const REPO_ROOT = join(__dirname, '..', '..') +const SOURCE_ROOTS = ['src/main', 'src/renderer/src', 'src/preload'] +const PRODUCTION_FILE_PATTERN = /\.(ts|tsx)$/ +const TEST_FILE_PATTERN = /(?:^|\.)(test|spec)\.(ts|tsx)$/ + describe('feature interactions', () => { it('defines local interaction semantics for product education features', () => { const catalogMatchesPublicUnion: [ @@ -95,4 +102,55 @@ describe('feature interactions', () => { ) ).toBe(false) }) + + it('keeps every catalog id wired to a production writer', () => { + const productionText = collectProductionSourceText() + const missingWriters = FEATURE_INTERACTIONS.map((feature) => feature.id).filter((id) => { + const escaped = escapeRegExp(id) + const directRecord = new RegExp( + `recordFeatureInteraction(?:\\?\\.)?\\(\\s*['"]${escaped}['"]` + ) + const runtimeMappingReturn = new RegExp(`return[^\\n]*['"]${escaped}['"]`) + return !directRecord.test(productionText) && !runtimeMappingReturn.test(productionText) + }) + + expect(missingWriters).toEqual([]) + }) }) + +function collectProductionSourceText(): string { + const files = SOURCE_ROOTS.flatMap((root) => collectSourceFiles(join(REPO_ROOT, root))) + return files + .sort() + .map((file) => readFileSync(file, 'utf8')) + .join('\n') +} + +function collectSourceFiles(directory: string): string[] { + const files: string[] = [] + for (const entry of readdirSync(directory)) { + const path = join(directory, entry) + const stats = statSync(path) + if (stats.isDirectory()) { + if (entry === 'node_modules' || entry === 'dist' || entry === 'out') { + continue + } + files.push(...collectSourceFiles(path)) + continue + } + const repoRelativePath = relative(REPO_ROOT, path) + if (!PRODUCTION_FILE_PATTERN.test(entry) || TEST_FILE_PATTERN.test(entry)) { + continue + } + // Why: the catalog itself proves the id exists, not that runtime code writes it. + if (repoRelativePath === 'src/shared/feature-interactions.ts') { + continue + } + files.push(path) + } + return files +} + +function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') +} diff --git a/src/shared/runtime-rpc-feature-interaction-source.ts b/src/shared/runtime-rpc-feature-interaction-source.ts new file mode 100644 index 00000000000..ce481338885 --- /dev/null +++ b/src/shared/runtime-rpc-feature-interaction-source.ts @@ -0,0 +1,25 @@ +export const ORCA_RUNTIME_RPC_FEATURE_INTERACTION_SOURCE_KEY = '__orcaFeatureInteractionSource' + +export const ORCA_RUNTIME_RPC_BROWSER_UI_SOURCE = 'browser-pane-ui' + +export function withBrowserPaneUiRuntimeRpcSource(value: unknown): unknown { + if (value === null || typeof value !== 'object' || Array.isArray(value)) { + return { + [ORCA_RUNTIME_RPC_FEATURE_INTERACTION_SOURCE_KEY]: ORCA_RUNTIME_RPC_BROWSER_UI_SOURCE + } + } + return { + ...value, + [ORCA_RUNTIME_RPC_FEATURE_INTERACTION_SOURCE_KEY]: ORCA_RUNTIME_RPC_BROWSER_UI_SOURCE + } +} + +export function isBrowserPaneUiRuntimeRpcParams(value: unknown): boolean { + return ( + value !== null && + typeof value === 'object' && + !Array.isArray(value) && + (value as Record)[ORCA_RUNTIME_RPC_FEATURE_INTERACTION_SOURCE_KEY] === + ORCA_RUNTIME_RPC_BROWSER_UI_SOURCE + ) +}