From 6f336acff9724db49913f02ba2735e116fc033e5 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 1 Jun 2026 15:54:08 -0700 Subject: [PATCH] Instrument star nag exposure logs (#4409) Co-authored-by: Orca --- src/main/star-nag/service.test.ts | 443 ++++++++++++++++++++++++++++++ src/main/star-nag/service.ts | 89 +++++- 2 files changed, 522 insertions(+), 10 deletions(-) create mode 100644 src/main/star-nag/service.test.ts diff --git a/src/main/star-nag/service.test.ts b/src/main/star-nag/service.test.ts new file mode 100644 index 000000000..f46cf64c6 --- /dev/null +++ b/src/main/star-nag/service.test.ts @@ -0,0 +1,443 @@ +/* eslint-disable max-lines -- Why: StarNagService tests share one mocked +Electron/IPC harness; splitting the narrow service suite would duplicate setup +and make the prompt-session edge cases harder to compare. */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { STAR_NAG_INITIAL_THRESHOLD } from '../../shared/constants' +import type { PersistedUIState } from '../../shared/types' +import type { Store } from '../persistence' +import type { StatsCollector } from '../stats/collector' +import { StarNagService } from './service' + +type TestWindow = { + isDestroyed: () => boolean + webContents: { send: ReturnType } +} + +const { appMock, browserWindowMock, checkOrcaStarredMock, ipcMainHandleMock } = vi.hoisted(() => ({ + appMock: { + getVersion: vi.fn(() => '1.2.3') + }, + browserWindowMock: { + getAllWindows: vi.fn<() => TestWindow[]>(() => []) + }, + checkOrcaStarredMock: vi.fn(), + ipcMainHandleMock: vi.fn() +})) + +vi.mock('electron', () => ({ + app: appMock, + BrowserWindow: browserWindowMock, + ipcMain: { + handle: ipcMainHandleMock + } +})) + +vi.mock('../github/client', () => ({ + checkOrcaStarred: checkOrcaStarredMock +})) + +type AgentStartedListener = (totalAgentsSpawned: number) => void +type IpcHandler = () => unknown + +type TestHarness = { + service: StarNagService + store: Store + ui: PersistedUIState + emitAgentStarted: (totalAgentsSpawned: number) => void +} + +function createWindow(): TestWindow { + return { + isDestroyed: () => false, + webContents: { + send: vi.fn() + } + } +} + +function createHarness(initialUI: Partial = {}): TestHarness { + let totalAgentsSpawned = 45 + const listeners: AgentStartedListener[] = [] + const ui = { + starNagAppVersion: '1.2.3', + starNagBaselineAgents: 10, + starNagNextThreshold: STAR_NAG_INITIAL_THRESHOLD, + ...initialUI + } as PersistedUIState + const store = { + getUI: vi.fn(() => ui), + updateUI: vi.fn((updates: Partial) => { + Object.assign(ui, updates) + }) + } as unknown as Store + const stats = { + onAgentStarted: vi.fn((listener: AgentStartedListener) => { + listeners.push(listener) + return () => { + const index = listeners.indexOf(listener) + if (index !== -1) { + listeners.splice(index, 1) + } + } + }), + getTotalAgentsSpawned: vi.fn(() => totalAgentsSpawned) + } as unknown as StatsCollector + + return { + service: new StarNagService(store, stats), + store, + ui, + emitAgentStarted: (nextTotal: number) => { + totalAgentsSpawned = nextTotal + for (const listener of listeners) { + listener(nextTotal) + } + } + } +} + +function createDeferred(): { + promise: Promise + resolve: (value: T) => void +} { + let resolve!: (value: T) => void + const promise = new Promise((innerResolve) => { + resolve = innerResolve + }) + return { promise, resolve } +} + +function getIpcHandler(channel: string): IpcHandler { + const call = ipcMainHandleMock.mock.calls.find( + ([registeredChannel]) => registeredChannel === channel + ) + if (!call) { + throw new Error(`missing IPC handler for ${channel}`) + } + return call[1] as IpcHandler +} + +async function flushAsyncWork(): Promise { + await new Promise((resolve) => setImmediate(resolve)) +} + +describe('StarNagService', () => { + let consoleInfoMock: ReturnType + + beforeEach(() => { + appMock.getVersion.mockReset() + appMock.getVersion.mockReturnValue('1.2.3') + browserWindowMock.getAllWindows.mockReset() + browserWindowMock.getAllWindows.mockReturnValue([]) + checkOrcaStarredMock.mockReset() + checkOrcaStarredMock.mockResolvedValue(false) + ipcMainHandleMock.mockReset() + consoleInfoMock = vi.spyOn(console, 'info').mockImplementation(() => undefined) + }) + + afterEach(() => { + consoleInfoMock.mockRestore() + }) + + it('logs a threshold exposure exactly once while the card remains visible', async () => { + const window = createWindow() + browserWindowMock.getAllWindows.mockReturnValue([window]) + const { service, emitAgentStarted } = createHarness() + + service.start() + emitAgentStarted(45) + await flushAsyncWork() + emitAgentStarted(46) + + expect(window.webContents.send).toHaveBeenCalledTimes(1) + expect(window.webContents.send).toHaveBeenCalledWith('star-nag:show') + expect(consoleInfoMock).toHaveBeenCalledTimes(1) + expect(consoleInfoMock).toHaveBeenCalledWith({ + event: 'star_nag_shown', + app_version: '1.2.3', + threshold: STAR_NAG_INITIAL_THRESHOLD, + agents_since_baseline: 35, + source: 'threshold' + }) + }) + + it.each([null, true])( + 'does not log a threshold exposure when checkOrcaStarred returns %s', + async (result) => { + const window = createWindow() + browserWindowMock.getAllWindows.mockReturnValue([window]) + checkOrcaStarredMock.mockResolvedValue(result) + const { service, emitAgentStarted } = createHarness() + + service.start() + emitAgentStarted(45) + await flushAsyncWork() + + expect(window.webContents.send).not.toHaveBeenCalled() + expect(consoleInfoMock).not.toHaveBeenCalled() + } + ) + + it('does not block a later real prompt after crossing the threshold with no window', async () => { + const { service, emitAgentStarted } = createHarness() + + service.start() + emitAgentStarted(45) + await flushAsyncWork() + + expect(consoleInfoMock).not.toHaveBeenCalled() + + const window = createWindow() + browserWindowMock.getAllWindows.mockReturnValue([window]) + emitAgentStarted(46) + await flushAsyncWork() + + expect(window.webContents.send).toHaveBeenCalledWith('star-nag:show') + expect(consoleInfoMock).toHaveBeenCalledWith({ + event: 'star_nag_shown', + app_version: '1.2.3', + threshold: STAR_NAG_INITIAL_THRESHOLD, + agents_since_baseline: 36, + source: 'threshold' + }) + }) + + it('logs dismissal with doubled next_threshold and advances backoff for the active session', async () => { + const window = createWindow() + browserWindowMock.getAllWindows.mockReturnValue([window]) + const { service, emitAgentStarted, ui } = createHarness() + + service.start() + service.registerIpcHandlers() + emitAgentStarted(45) + await flushAsyncWork() + getIpcHandler('star-nag:dismiss')() + + expect(consoleInfoMock).toHaveBeenLastCalledWith({ + event: 'star_nag_dismissed', + app_version: '1.2.3', + threshold: STAR_NAG_INITIAL_THRESHOLD, + agents_since_baseline: 35, + source: 'threshold', + next_threshold: STAR_NAG_INITIAL_THRESHOLD * 2 + }) + expect(ui.starNagNextThreshold).toBe(STAR_NAG_INITIAL_THRESHOLD * 2) + expect(ui.starNagBaselineAgents).toBe(45) + }) + + it('keeps the force_show source through exposure and dismissal', () => { + const window = createWindow() + browserWindowMock.getAllWindows.mockReturnValue([window]) + const { service, ui } = createHarness() + + service.registerIpcHandlers() + getIpcHandler('star-nag:forceShow')() + getIpcHandler('star-nag:dismiss')() + + expect(consoleInfoMock).toHaveBeenNthCalledWith(1, { + event: 'star_nag_shown', + app_version: '1.2.3', + threshold: STAR_NAG_INITIAL_THRESHOLD, + agents_since_baseline: 35, + source: 'force_show' + }) + expect(consoleInfoMock).toHaveBeenNthCalledWith(2, { + event: 'star_nag_dismissed', + app_version: '1.2.3', + threshold: STAR_NAG_INITIAL_THRESHOLD, + agents_since_baseline: 35, + source: 'force_show', + next_threshold: STAR_NAG_INITIAL_THRESHOLD * 2 + }) + expect(ui.starNagNextThreshold).toBe(STAR_NAG_INITIAL_THRESHOLD * 2) + }) + + it('does not log or block a later force_show when no window exists', () => { + const { service } = createHarness() + + service.registerIpcHandlers() + const forceShow = getIpcHandler('star-nag:forceShow') + forceShow() + + expect(consoleInfoMock).not.toHaveBeenCalled() + + const window = createWindow() + browserWindowMock.getAllWindows.mockReturnValue([window]) + forceShow() + + expect(window.webContents.send).toHaveBeenCalledWith('star-nag:show') + expect(consoleInfoMock).toHaveBeenCalledWith({ + event: 'star_nag_shown', + app_version: '1.2.3', + threshold: STAR_NAG_INITIAL_THRESHOLD, + agents_since_baseline: 35, + source: 'force_show' + }) + }) + + it('keeps threshold source when force_show is requested during a successful threshold evaluation', async () => { + const window = createWindow() + browserWindowMock.getAllWindows.mockReturnValue([window]) + const deferredStarCheck = createDeferred() + checkOrcaStarredMock.mockReturnValue(deferredStarCheck.promise) + const { service, emitAgentStarted } = createHarness() + + service.start() + service.registerIpcHandlers() + emitAgentStarted(45) + getIpcHandler('star-nag:forceShow')() + + expect(window.webContents.send).not.toHaveBeenCalled() + expect(consoleInfoMock).not.toHaveBeenCalled() + + deferredStarCheck.resolve(false) + await flushAsyncWork() + + expect(window.webContents.send).toHaveBeenCalledTimes(1) + expect(consoleInfoMock).toHaveBeenCalledTimes(1) + expect(consoleInfoMock).toHaveBeenCalledWith({ + event: 'star_nag_shown', + app_version: '1.2.3', + threshold: STAR_NAG_INITIAL_THRESHOLD, + agents_since_baseline: 35, + source: 'threshold' + }) + }) + + it('does not replay a stale queued force_show after threshold delivery wins', async () => { + const window = createWindow() + browserWindowMock.getAllWindows.mockReturnValue([window]) + const firstStarCheck = createDeferred() + checkOrcaStarredMock.mockReturnValueOnce(firstStarCheck.promise).mockResolvedValue(null) + const { service, emitAgentStarted } = createHarness() + + service.start() + service.registerIpcHandlers() + emitAgentStarted(45) + getIpcHandler('star-nag:forceShow')() + + firstStarCheck.resolve(false) + await flushAsyncWork() + getIpcHandler('star-nag:dismiss')() + + emitAgentStarted(115) + await flushAsyncWork() + + expect(window.webContents.send).toHaveBeenCalledTimes(1) + expect(consoleInfoMock.mock.calls).toEqual([ + [ + { + event: 'star_nag_shown', + app_version: '1.2.3', + threshold: STAR_NAG_INITIAL_THRESHOLD, + agents_since_baseline: 35, + source: 'threshold' + } + ], + [ + { + event: 'star_nag_dismissed', + app_version: '1.2.3', + threshold: STAR_NAG_INITIAL_THRESHOLD, + agents_since_baseline: 35, + source: 'threshold', + next_threshold: STAR_NAG_INITIAL_THRESHOLD * 2 + } + ] + ]) + }) + + it('does not show after completion wins an in-flight threshold evaluation', async () => { + const window = createWindow() + browserWindowMock.getAllWindows.mockReturnValue([window]) + const deferredStarCheck = createDeferred() + checkOrcaStarredMock.mockReturnValue(deferredStarCheck.promise) + const { service, emitAgentStarted, ui } = createHarness() + + service.start() + service.registerIpcHandlers() + emitAgentStarted(45) + getIpcHandler('star-nag:forceShow')() + getIpcHandler('star-nag:complete')() + + deferredStarCheck.resolve(false) + await flushAsyncWork() + + expect(ui.starNagCompleted).toBe(true) + expect(window.webContents.send).not.toHaveBeenCalled() + expect(consoleInfoMock).not.toHaveBeenCalled() + }) + + it('replays force_show after an in-flight threshold evaluation exits without showing', async () => { + const window = createWindow() + browserWindowMock.getAllWindows.mockReturnValue([window]) + const deferredStarCheck = createDeferred() + checkOrcaStarredMock.mockReturnValue(deferredStarCheck.promise) + const { service, emitAgentStarted } = createHarness() + + service.start() + service.registerIpcHandlers() + emitAgentStarted(45) + getIpcHandler('star-nag:forceShow')() + + expect(window.webContents.send).not.toHaveBeenCalled() + expect(consoleInfoMock).not.toHaveBeenCalled() + + deferredStarCheck.resolve(null) + await flushAsyncWork() + + expect(window.webContents.send).toHaveBeenCalledTimes(1) + expect(consoleInfoMock).toHaveBeenCalledTimes(1) + expect(consoleInfoMock).toHaveBeenCalledWith({ + event: 'star_nag_shown', + app_version: '1.2.3', + threshold: STAR_NAG_INITIAL_THRESHOLD, + agents_since_baseline: 35, + source: 'force_show' + }) + }) + + it('ignores stray and duplicate dismissals without logging or advancing backoff', () => { + const window = createWindow() + browserWindowMock.getAllWindows.mockReturnValue([window]) + const { service, store, ui } = createHarness() + + service.registerIpcHandlers() + const dismiss = getIpcHandler('star-nag:dismiss') + dismiss() + + expect(consoleInfoMock).not.toHaveBeenCalled() + expect(store.updateUI).not.toHaveBeenCalled() + expect(ui.starNagNextThreshold).toBe(STAR_NAG_INITIAL_THRESHOLD) + + getIpcHandler('star-nag:forceShow')() + dismiss() + dismiss() + + const dismissedLogs = consoleInfoMock.mock.calls.filter( + ([payload]) => (payload as { event?: string }).event === 'star_nag_dismissed' + ) + expect(dismissedLogs).toHaveLength(1) + expect(ui.starNagNextThreshold).toBe(STAR_NAG_INITIAL_THRESHOLD * 2) + }) + + it('marks completion without adding duplicate success logging', () => { + const window = createWindow() + browserWindowMock.getAllWindows.mockReturnValue([window]) + const { service, ui } = createHarness() + + service.registerIpcHandlers() + getIpcHandler('star-nag:forceShow')() + getIpcHandler('star-nag:complete')() + + expect(ui.starNagCompleted).toBe(true) + expect(consoleInfoMock).toHaveBeenCalledTimes(1) + expect(consoleInfoMock).toHaveBeenCalledWith({ + event: 'star_nag_shown', + app_version: '1.2.3', + threshold: STAR_NAG_INITIAL_THRESHOLD, + agents_since_baseline: 35, + source: 'force_show' + }) + }) +}) diff --git a/src/main/star-nag/service.ts b/src/main/star-nag/service.ts index dad3c29cb..ca16b6565 100644 --- a/src/main/star-nag/service.ts +++ b/src/main/star-nag/service.ts @@ -4,6 +4,12 @@ import { checkOrcaStarred } from '../github/client' import type { Store } from '../persistence' import type { StatsCollector } from '../stats/collector' +type StarNagPromptSource = 'threshold' | 'force_show' + +type StarNagPromptSession = { + source: StarNagPromptSource +} + /** * Service that decides when to prompt the user with the "star Orca on GitHub" * notification. Counts agents spawned since the current app version was first @@ -28,6 +34,10 @@ export class StarNagService { // tiny window between crossing the threshold and the first gh check // resolving. private evaluating = false + private pendingForceShow = false + // Why: dismissal backoff should only apply to a prompt that was actually + // delivered, and the dismissal payload needs the delivered prompt source. + private promptSession: StarNagPromptSession | null = null constructor(store: Store, stats: StatsCollector) { this.store = store @@ -99,10 +109,10 @@ export class StarNagService { if (sinceBaseline < threshold) { return } - void this.maybeShow() + void this.maybeShow('threshold') } - private async maybeShow(): Promise { + private async maybeShow(source: StarNagPromptSource): Promise { if (this.promptVisible || this.evaluating) { return } @@ -124,19 +134,62 @@ export class StarNagService { this.markCompleted() return } - this.promptVisible = true - this.broadcastShow() + if (this.store.getUI().starNagCompleted) { + this.pendingForceShow = false + return + } + if (this.promptVisible) { + return + } + this.broadcastShow(source) } finally { this.evaluating = false + this.flushPendingForceShow() } } - private broadcastShow(): void { - const win = BrowserWindow.getAllWindows().find((w) => !w.isDestroyed()) - if (!win) { + private flushPendingForceShow(): void { + if (!this.pendingForceShow || this.evaluating) { return } + this.pendingForceShow = false + if (this.promptVisible) { + return + } + this.broadcastShow('force_show') + } + + private broadcastShow(source: StarNagPromptSource): boolean { + const win = BrowserWindow.getAllWindows().find((w) => !w.isDestroyed()) + if (!win) { + this.promptVisible = false + this.promptSession = null + return false + } win.webContents.send('star-nag:show') + this.promptVisible = true + this.promptSession = { source } + this.logConsoleEvent('star_nag_shown', source) + return true + } + + private logConsoleEvent( + event: 'star_nag_shown' | 'star_nag_dismissed', + source: StarNagPromptSource, + nextThreshold?: number + ): void { + const ui = this.store.getUI() + const threshold = ui.starNagNextThreshold ?? STAR_NAG_INITIAL_THRESHOLD + const agentsSinceBaseline = this.stats.getTotalAgentsSpawned() - (ui.starNagBaselineAgents ?? 0) + + console.info({ + event, + app_version: app.getVersion(), + threshold, + agents_since_baseline: agentsSinceBaseline, + source, + ...(nextThreshold === undefined ? {} : { next_threshold: nextThreshold }) + }) } // ── Public actions (invoked from IPC) ───────────────────────────── @@ -149,24 +202,40 @@ export class StarNagService { * more, etc. */ private dismiss(): void { + const session = this.promptSession + if (!session) { + this.promptVisible = false + return + } const ui = this.store.getUI() const threshold = ui.starNagNextThreshold ?? STAR_NAG_INITIAL_THRESHOLD + const nextThreshold = threshold * 2 + this.logConsoleEvent('star_nag_dismissed', session.source, nextThreshold) this.store.updateUI({ - starNagNextThreshold: threshold * 2, + starNagNextThreshold: nextThreshold, starNagBaselineAgents: this.stats.getTotalAgentsSpawned() }) this.promptVisible = false + this.promptSession = null } /** User successfully starred → never nag again. */ private markCompleted(): void { this.store.updateUI({ starNagCompleted: true }) this.promptVisible = false + this.promptSession = null + this.pendingForceShow = false } /** Dev-only entry point: skip all gating and fire the notification. */ private forceShow(): void { - this.promptVisible = true - this.broadcastShow() + if (this.promptVisible) { + return + } + if (this.evaluating) { + this.pendingForceShow = true + return + } + this.broadcastShow('force_show') } }