diff --git a/src/main/persistence.test.ts b/src/main/persistence.test.ts index 8574c871e..f9a701dbf 100644 --- a/src/main/persistence.test.ts +++ b/src/main/persistence.test.ts @@ -25,7 +25,7 @@ import type { } from '../shared/types' import { isTerminalLeafId, makePaneKey } from '../shared/stable-pane-id' import { MAX_BROWSER_HISTORY_ENTRIES } from '../shared/workspace-session-browser-history' -import { ONBOARDING_FLOW_VERSION } from '../shared/constants' +import { ONBOARDING_FINAL_STEP, ONBOARDING_FLOW_VERSION } from '../shared/constants' // Shared mutable state so the electron mock can reference a per-test directory const testState = { dir: '' } @@ -286,6 +286,173 @@ describe('Store', () => { expect(ui.lastActiveRepoId).toBeNull() expect(ui.dismissedUpdateVersion).toBeNull() expect(ui.lastUpdateCheckAt).toBeNull() + expect(ui.setupGuideSidebarDismissed).toBe(false) + }) + + it('hides the setup guide sidebar entry for existing users backfilled as completed', async () => { + writeDataFile({ + schemaVersion: 1, + ui: {} + }) + + const store = await createStore() + const onboarding = store.getOnboarding() + + expect(onboarding.closedAt).not.toBeNull() + expect(onboarding.outcome).toBe('completed') + expect(onboarding.lastCompletedStep).toBe(ONBOARDING_FINAL_STEP) + expect(store.getUI().setupGuideSidebarDismissed).toBe(true) + }) + + it('persists the existing-user onboarding backfill back to disk', async () => { + // Why: the upgrade-cohort backfill is derived at load; this asserts the + // backfilled onboarding+gate state round-trips through a write intact (the + // load-time scheduleSave that triggers it without a manual flush is wired + // via loadNeedsSave at the no-onboarding-block branch). + writeDataFile({ + schemaVersion: 1, + ui: {} + }) + + const store = await createStore() + store.flush() + const persisted = readDataFile() as { + onboarding?: { closedAt: number | null; outcome: string | null; lastCompletedStep: number } + ui?: { setupGuideSidebarDismissed?: boolean } + } + + expect(persisted.onboarding?.closedAt).not.toBeNull() + expect(persisted.onboarding?.outcome).toBe('completed') + expect(persisted.onboarding?.lastCompletedStep).toBe(ONBOARDING_FINAL_STEP) + expect(persisted.ui?.setupGuideSidebarDismissed).toBe(true) + }) + + it('keeps the setup guide sidebar entry available while onboarding is open', async () => { + writeDataFile({ + onboarding: { + flowVersion: ONBOARDING_FLOW_VERSION, + closedAt: null, + outcome: null, + lastCompletedStep: -1, + checklist: {} + }, + ui: {} + }) + + const store = await createStore() + + expect(store.getOnboarding().closedAt).toBeNull() + expect(store.getUI().setupGuideSidebarDismissed).toBe(false) + }) + + it('treats persisted false setup guide sidebar dismissal as stale once onboarding is closed', async () => { + writeDataFile({ + onboarding: { + flowVersion: ONBOARDING_FLOW_VERSION, + closedAt: 123, + outcome: 'dismissed', + lastCompletedStep: 2, + checklist: {} + }, + ui: { + setupGuideSidebarDismissed: false + } + }) + + const store = await createStore() + + expect(store.getUI().setupGuideSidebarDismissed).toBe(true) + }) + + it('keeps malformed completed onboarding closed for the setup guide sidebar gate', async () => { + writeDataFile({ + onboarding: { + flowVersion: ONBOARDING_FLOW_VERSION, + closedAt: 'yesterday', + outcome: 'completed', + lastCompletedStep: ONBOARDING_FINAL_STEP, + checklist: {} + }, + ui: { + setupGuideSidebarDismissed: false + } + }) + + const store = await createStore() + const onboarding = store.getOnboarding() + + expect(onboarding.closedAt).not.toBeNull() + expect(onboarding.outcome).toBe('completed') + expect(onboarding.lastCompletedStep).toBe(ONBOARDING_FINAL_STEP) + expect(store.getUI().setupGuideSidebarDismissed).toBe(true) + }) + + it('does not reopen the setup guide sidebar when closed onboarding has a null timestamp', async () => { + writeDataFile({ + onboarding: { + flowVersion: ONBOARDING_FLOW_VERSION, + closedAt: null, + outcome: 'dismissed', + lastCompletedStep: 1, + checklist: {} + }, + ui: {} + }) + + const store = await createStore() + + expect(store.getOnboarding().closedAt).not.toBeNull() + expect(store.getUI().setupGuideSidebarDismissed).toBe(true) + }) + + it('recovers a close timestamp when closed onboarding omits the closedAt key', async () => { + // Why: a persisted block missing `closedAt` entirely (vs an explicit null) + // must still stay closed via outcome recovery, guarding the + // `'closedAt' in raw` sanitizer branch separately from the null case. + writeDataFile({ + onboarding: { + flowVersion: ONBOARDING_FLOW_VERSION, + outcome: 'completed', + lastCompletedStep: ONBOARDING_FINAL_STEP, + checklist: {} + }, + ui: {} + }) + + const store = await createStore() + + expect(store.getOnboarding().closedAt).not.toBeNull() + expect(store.getUI().setupGuideSidebarDismissed).toBe(true) + }) + + it('does not mutate gate fields for a consistent closed-onboarding existing user', async () => { + // Why: the gate must be idempotent. A user already persisted as + // closed+completed must round-trip unchanged — the backfill path must not + // fire and stomp the real closedAt with a fresh Date.now() each launch. + const consistent = { + onboarding: { + flowVersion: ONBOARDING_FLOW_VERSION, + closedAt: 123, + outcome: 'completed', + lastCompletedStep: ONBOARDING_FINAL_STEP, + checklist: {} + }, + ui: { + setupGuideSidebarDismissed: true + } + } + writeDataFile(consistent) + + const store = await createStore() + expect(store.getUI().setupGuideSidebarDismissed).toBe(true) + + store.flush() + const persisted = readDataFile() as typeof consistent + + // Flushing the loaded state preserves the persisted gate fields verbatim. + expect(persisted.onboarding.closedAt).toBe(123) + expect(persisted.onboarding.outcome).toBe('completed') + expect(persisted.ui.setupGuideSidebarDismissed).toBe(true) }) it.each([ diff --git a/src/main/persistence.ts b/src/main/persistence.ts index a5193095f..4f2cc2d7a 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -583,6 +583,60 @@ export function sanitizeOnboardingUpdate( return out } +function normalizeLoadedOnboardingState( + input: unknown, + defaults: OnboardingState +): OnboardingState { + // Why: if we successfully parsed an existing orca-data.json that lacks an + // onboarding block, this is an upgrade-cohort user — backfill as completed + // (not dismissed) so they don't get dropped into the wizard regardless of + // whether they currently have repos, SSH targets, or just non-default + // settings. Analytics still distinguish this from users who explicitly + // bailed mid-funnel. + if (!input) { + return { + ...defaults, + closedAt: Date.now(), + outcome: 'completed', + lastCompletedStep: ONBOARDING_FINAL_STEP + } + } + // Why: validate every persisted onboarding key explicitly via the shared + // sanitizer instead of spreading raw values. A type-flipped field on disk + // (string where number expected, unknown checklist key) is dropped or + // coerced to the default rather than poisoning in-memory state. + const sanitized = sanitizeOnboardingUpdate(input, { + migrateLegacyProgress: true + }) + // Why: a persisted completed/dismissed outcome means the user left + // onboarding. Recover from a bad/missing/null closedAt instead of reopening + // the new-user sidebar checklist. + const recoveredClosedAt = + typeof sanitized.closedAt === 'number' + ? sanitized.closedAt + : sanitized.outcome !== null && sanitized.outcome !== undefined + ? Date.now() + : sanitized.closedAt + return { + ...defaults, + ...sanitized, + closedAt: recoveredClosedAt ?? defaults.closedAt, + checklist: { + ...defaults.checklist, + ...sanitized.checklist + } + } +} + +function resolveSetupGuideSidebarDismissedOnLoad( + persistedDismissed: unknown, + onboarding: OnboardingState +): boolean { + // Why: the sidebar checklist is a new-user prompt. Once onboarding is + // closed, persisted false is just the old default value, not a user opt-in. + return onboarding.closedAt !== null || persistedDismissed === true +} + // Why: read a settings field that was removed from the GlobalSettings type // but still round-trips on disk via the ...parsed.settings spread. One-shot // use only — for the inline-agents default-on migration's Case B discriminator. @@ -1755,6 +1809,13 @@ export class Store { if (!visibleTaskProvidersDefaultedForJira) { this.loadNeedsSave = true } + const normalizedOnboarding = normalizeLoadedOnboardingState( + parsed.onboarding, + defaults.onboarding + ) + if (!parsed.onboarding) { + this.loadNeedsSave = true + } result = { ...defaults, ...parsed, @@ -1935,6 +1996,16 @@ export class Store { ) { this.loadNeedsSave = true } + const setupGuideSidebarDismissed = resolveSetupGuideSidebarDismissedOnLoad( + parsed.ui?.setupGuideSidebarDismissed, + normalizedOnboarding + ) + if ( + parsed.ui?.setupGuideSidebarDismissed !== setupGuideSidebarDismissed && + (setupGuideSidebarDismissed || parsed.ui?.setupGuideSidebarDismissed !== undefined) + ) { + this.loadNeedsSave = true + } return { ...defaults.ui, ...parsed.ui, @@ -1942,6 +2013,7 @@ export class Store { // when no explicit persisted chrome preference exists yet. rightSidebarOpen, rightSidebarTab: normalizeRightSidebarTab(parsed.ui?.rightSidebarTab), + setupGuideSidebarDismissed, sortBy: migrate ? ('smart' as const) : sort, showDotfilesByWorktree: normalizeShowDotfilesByWorktree( parsed.ui?.showDotfilesByWorktree @@ -1995,38 +2067,7 @@ export class Store { ), automations: Array.isArray(parsed.automations) ? parsed.automations : [], automationRuns: Array.isArray(parsed.automationRuns) ? parsed.automationRuns : [], - onboarding: (() => { - // Why: if we successfully parsed an existing orca-data.json that - // lacks an onboarding block, this is an upgrade-cohort user — - // backfill as completed (not dismissed) so they don't get dropped - // into the wizard regardless of whether they currently have repos, - // SSH targets, or just non-default settings. Analytics still - // distinguish this from users who explicitly bailed mid-funnel. - if (!parsed.onboarding) { - return { - ...defaults.onboarding, - closedAt: Date.now(), - outcome: 'completed' as const, - lastCompletedStep: ONBOARDING_FINAL_STEP - } - } - // Why: validate every persisted onboarding key explicitly via the - // shared sanitizer instead of spreading raw values. A type-flipped - // field on disk (string where number expected, unknown checklist - // key) is dropped or coerced to the default rather than poisoning - // in-memory state. - const sanitized = sanitizeOnboardingUpdate(parsed.onboarding, { - migrateLegacyProgress: true - }) - return { - ...defaults.onboarding, - ...sanitized, - checklist: { - ...defaults.onboarding.checklist, - ...sanitized.checklist - } - } - })() + onboarding: normalizedOnboarding } } } catch (err) {