From b66c98fd72dd2cba58a82ad08c07064fb127bfb2 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sun, 18 Oct 2026 15:48:42 -0700 Subject: [PATCH] update prompt logic (#30444) --- src/main/star-nag/agent-value-moment.ts | 22 +++--- .../star-nag/service-prompt-moments.test.ts | 20 +++++- .../star-nag/service-threshold-prompt.test.ts | 69 ++++++++++++++++++- src/main/star-nag/threshold-trigger.ts | 11 +-- src/shared/persisted-ui-state-types.ts | 6 +- 5 files changed, 98 insertions(+), 30 deletions(-) diff --git a/src/main/star-nag/agent-value-moment.ts b/src/main/star-nag/agent-value-moment.ts index ad785341a01..7c721903bfd 100644 --- a/src/main/star-nag/agent-value-moment.ts +++ b/src/main/star-nag/agent-value-moment.ts @@ -36,9 +36,8 @@ export class StarNagAgentValueMoment { this.deps.isCooldownActive(ui.starNagDeferredUntil) || this.deps.isPromptVisible() ) { - // Why: each app version gets at most one completion-moment attempt, even if - // an existing prompt/cooldown blocks the extra agent-finished trigger. - this.consumeVersion() + // Existing prompts and cooldowns also spend the one-time completion ask. + this.consumeMoment() return { status: 'skipped' } } this.deps.setEvaluating(true) @@ -54,9 +53,7 @@ export class StarNagAgentValueMoment { if (starred) { this.deps.trackAlreadyStarredSuppressed() this.deps.markCompleted() - // Why: already-starred users should not be rechecked on every agent - // completion after this version has been resolved. - this.consumeVersion() + this.consumeMoment() return { status: 'skipped' } } this.pendingMode = 'gh' @@ -78,17 +75,14 @@ export class StarNagAgentValueMoment { this.deps.isPromptVisible() || this.deps.isEvaluating() ) { - // Why: the prepared moment can go stale before display; consuming prevents - // repeated prompts from the same app-version completion moment. - this.consumeVersion() + // A prepared moment can go stale while eligibility is being checked. + this.consumeMoment() this.pendingMode = null return } const delivered = this.deps.broadcastShow(mode) if (delivered || this.deps.store.getUI().starNagCompleted) { - // Why: once a prompt is delivered or completion wins the race, this app - // version's agent-value moment has been spent. - this.consumeVersion() + this.consumeMoment() } if (delivered) { this.pendingMode = null @@ -99,11 +93,11 @@ export class StarNagAgentValueMoment { this.pendingMode = null } - private consumeVersion(): void { + private consumeMoment(): void { this.deps.store.updateUI({ starNagAgentValueMomentAppVersion: app.getVersion() }) } private wasConsumed(): boolean { - return this.deps.store.getUI().starNagAgentValueMomentAppVersion === app.getVersion() + return Boolean(this.deps.store.getUI().starNagAgentValueMomentAppVersion) } } diff --git a/src/main/star-nag/service-prompt-moments.test.ts b/src/main/star-nag/service-prompt-moments.test.ts index 9fd8e858637..c127f0c2491 100644 --- a/src/main/star-nag/service-prompt-moments.test.ts +++ b/src/main/star-nag/service-prompt-moments.test.ts @@ -59,7 +59,7 @@ describe('StarNagService', () => { consoleInfoMock.mockRestore() }) - it('shows agent value moment prompts once per app version after eligibility passes', async () => { + it('does not rearm agent value moment prompts after an app update', async () => { const window = createWindow() browserWindowMock.getAllWindows.mockReturnValue([window]) const { service, ui } = createHarness() @@ -71,6 +71,8 @@ describe('StarNagService', () => { }) await getIpcHandler('star-nag:showAgentValueMoment')() getIpcHandler('star-nag:dismiss')() + ui.starNagDeferredUntil = null + mocks.appMock.getVersion.mockReturnValue('1.2.4') await expect(getIpcHandler('star-nag:agentValueMoment')()).resolves.toEqual({ status: 'skipped' }) @@ -87,7 +89,7 @@ describe('StarNagService', () => { ) }) - it('consumes agent value moment for cooldown suppression without showing later in the same version', async () => { + it('keeps cooldown-suppressed agent value moments consumed after an update', async () => { const window = createWindow() browserWindowMock.getAllWindows.mockReturnValue([window]) const { service, ui } = createHarness({ @@ -99,6 +101,7 @@ describe('StarNagService', () => { status: 'skipped' }) ui.starNagDeferredUntil = null + mocks.appMock.getVersion.mockReturnValue('1.2.4') await expect(getIpcHandler('star-nag:agentValueMoment')()).resolves.toEqual({ status: 'skipped' }) @@ -107,6 +110,19 @@ describe('StarNagService', () => { expect(ui.starNagAgentValueMomentAppVersion).toBe('1.2.3') }) + it('honors an agent value moment consumed by an older app version on startup', async () => { + const { service, store } = createHarness({ starNagAgentValueMomentAppVersion: '1.2.2' }) + service.registerIpcHandlers() + + await expect(getIpcHandler('star-nag:agentValueMoment')()).resolves.toEqual({ + status: 'skipped' + }) + await getIpcHandler('star-nag:showAgentValueMoment')() + + expect(checkOrcaStarredMock).not.toHaveBeenCalled() + expect(store.updateUI).not.toHaveBeenCalled() + }) + it('does not consume agent value moment when no window can receive the card', async () => { const { service, ui } = createHarness() diff --git a/src/main/star-nag/service-threshold-prompt.test.ts b/src/main/star-nag/service-threshold-prompt.test.ts index c39b424de66..e9e2bec9b8c 100644 --- a/src/main/star-nag/service-threshold-prompt.test.ts +++ b/src/main/star-nag/service-threshold-prompt.test.ts @@ -59,6 +59,55 @@ describe('StarNagService', () => { consoleInfoMock.mockRestore() }) + it('preserves the counter and dismissal backoff after an app update', async () => { + const window = createWindow() + browserWindowMock.getAllWindows.mockReturnValue([window]) + const { service, emitAgentStarted, store, ui } = createHarness({ + starNagAppVersion: '1.2.2', + starNagBaselineAgents: 10, + starNagNextThreshold: 70, + starNagDeferredUntil: 1 + }) + + service.start() + expect(store.updateUI).not.toHaveBeenCalled() + expect(ui.starNagDeferredUntil).toBe(1) + emitAgentStarted(79) + await flushAsyncWork() + expect(window.webContents.send).not.toHaveBeenCalled() + + emitAgentStarted(80) + await flushAsyncWork() + expect(window.webContents.send).toHaveBeenCalledTimes(1) + expect(trackMock).toHaveBeenCalledWith( + 'star_nag_outcome', + expect.objectContaining({ outcome: 'shown', threshold: 70, agents_since_baseline: 70 }) + ) + }) + + it.each([undefined, null])('initializes a missing baseline (%s) only once', async (baseline) => { + const window = createWindow() + browserWindowMock.getAllWindows.mockReturnValue([window]) + const { service, emitAgentStarted, ui } = createHarness({ + starNagAppVersion: undefined, + starNagBaselineAgents: baseline, + starNagNextThreshold: undefined + }) + + service.start() + expect(ui.starNagBaselineAgents).toBe(45) + expect(ui.starNagNextThreshold).toBe(STAR_NAG_INITIAL_THRESHOLD) + service.stop() + mocks.appMock.getVersion.mockReturnValue('1.2.4') + service.start() + emitAgentStarted(79) + await flushAsyncWork() + expect(window.webContents.send).not.toHaveBeenCalled() + emitAgentStarted(80) + await flushAsyncWork() + expect(window.webContents.send).toHaveBeenCalledTimes(1) + }) + it('logs a threshold exposure exactly once while the card remains visible', async () => { const window = createWindow() browserWindowMock.getAllWindows.mockReturnValue([window]) @@ -182,18 +231,32 @@ describe('StarNagService', () => { expect(ui.starNagDeferredUntil).toBeGreaterThan(Date.now()) }) - it('does not show threshold prompts while the persisted cooldown is active', async () => { + it('preserves an active cooldown and backoff after an app update', async () => { const window = createWindow() browserWindowMock.getAllWindows.mockReturnValue([window]) - const { service, emitAgentStarted } = createHarness({ + const { service, emitAgentStarted, store, ui } = createHarness({ + starNagAppVersion: '1.2.2', + starNagNextThreshold: 70, starNagDeferredUntil: Date.now() + 3 * 24 * 60 * 60 * 1000 }) service.start() - emitAgentStarted(45) + emitAgentStarted(80) await flushAsyncWork() + expect(checkOrcaStarredMock).not.toHaveBeenCalled() expect(window.webContents.send).not.toHaveBeenCalled() expect(trackMock).not.toHaveBeenCalled() + expect(store.updateUI).not.toHaveBeenCalled() + + ui.starNagDeferredUntil = Date.now() - 1 + emitAgentStarted(81) + await flushAsyncWork() + + expect(window.webContents.send).toHaveBeenCalledTimes(1) + expect(window.webContents.send).toHaveBeenCalledWith('star-nag:show', { + mode: 'gh', + surface: 'card' + }) }) }) diff --git a/src/main/star-nag/threshold-trigger.ts b/src/main/star-nag/threshold-trigger.ts index bb487c7ab64..42f4401a103 100644 --- a/src/main/star-nag/threshold-trigger.ts +++ b/src/main/star-nag/threshold-trigger.ts @@ -1,4 +1,3 @@ -import { app } from 'electron' import { STAR_NAG_INITIAL_THRESHOLD } from '../../shared/constants' import type { Store } from '../persistence' import type { StatsCollector } from '../stats/collector' @@ -14,16 +13,12 @@ type ThresholdPromptInput = { export function ensureStarNagBaseline(store: Store, stats: StatsCollector): void { const ui = store.getUI() - const currentVersion = app.getVersion() - if (ui.starNagAppVersion === currentVersion && ui.starNagBaselineAgents != null) { + if (ui.starNagBaselineAgents != null) { return } - // Why: after an update, completed users stay suppressed but everyone else - // gets a fresh countdown from the current agent total. store.updateUI({ - starNagAppVersion: currentVersion, starNagBaselineAgents: stats.getTotalAgentsSpawned(), - starNagNextThreshold: STAR_NAG_INITIAL_THRESHOLD + starNagNextThreshold: ui.starNagNextThreshold ?? STAR_NAG_INITIAL_THRESHOLD }) } @@ -35,7 +30,7 @@ export function shouldShowStarNagThresholdPrompt(input: ThresholdPromptInput): b if (ui.starNagCompleted || input.isCooldownActive(ui.starNagDeferredUntil)) { return false } - if (ui.starNagAppVersion !== app.getVersion()) { + if (ui.starNagBaselineAgents == null) { ensureStarNagBaseline(input.store, input.stats) return false } diff --git a/src/shared/persisted-ui-state-types.ts b/src/shared/persisted-ui-state-types.ts index 60d90556201..f0f7c852213 100644 --- a/src/shared/persisted-ui-state-types.ts +++ b/src/shared/persisted-ui-state-types.ts @@ -215,9 +215,9 @@ export type PersistedUIState = { _jiraIssueWorktreeCardPropertyDefaulted?: boolean /** One-shot backfill flag for 'host', which became a toggleable property after earlier profiles were already stamped. */ _hostWorktreeCardPropertyDefaulted?: boolean - /** totalAgentsSpawned snapshot at first sighting of the current app version, so the nag counts agents since last update (not from zero). */ + /** Agent total when the star countdown began or was deferred; preserved across app updates. */ starNagBaselineAgents?: number | null - /** App version that set the current baseline; a version change re-captures the baseline on next spawn, restarting the nag countdown. */ + /** Legacy baseline version retained for older builds; no longer resets the countdown. */ starNagAppVersion?: string | null /** Next agents-since-baseline threshold that fires the star-nag; starts at 35, doubles per dismissal without starring. */ starNagNextThreshold?: number @@ -225,7 +225,7 @@ export type PersistedUIState = { starNagCompleted?: boolean /** Timestamp until which nonterminal dismissals suppress threshold prompts (force-show bypasses for dev/testing). */ starNagDeferredUntil?: number | null - /** App version that consumed the first value-moment ask; main-owned so remote/web clients can't spoof the once-per-version cap. */ + /** Version that consumed the one-time value-moment ask; any saved version suppresses future attempts. Main-owned. */ starNagAgentValueMomentAppVersion?: string | null trustedOrcaHooks?: PersistedTrustedOrcaHooks setupScriptPromptDismissedRepoIds?: string[]