From e0851ec7223af8866e63cb4deed6fbbcc601c85b Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Fri, 8 May 2026 14:17:50 -0700 Subject: [PATCH] feat: onboarding flow for new users (#1596) * wip * WIP: Changes before auto-review fixes Co-authored-by: Orca * WIP: Changes before auto-review fixes Co-authored-by: Orca * WIP: Changes before auto-review fixes Co-authored-by: Orca * fix: address auto-review findings (iteration 1) Co-authored-by: Orca * fix: address auto-review findings (iteration 2) Co-authored-by: Orca * fix: archive review context and improve agent detection on wizard mount Co-authored-by: Orca * fix: address CI lint failures and split use-onboarding-flow.ts Co-authored-by: Orca * fix: mock ./onboarding in register-core-handlers test Co-authored-by: Orca * fix: also toggle light class on documentElement so onboarding e2e theme wait resolves The onboarding e2e calls waitForFunction(() => classList.contains('dark') || classList.contains('light')) before snapshotting the starting theme. applyDocumentTheme only toggled 'dark', so on a host that resolves system to light the wait timed out (CI Linux headless). Toggle 'light' as the inverse class so consumers can observe the resolved theme symmetrically; Tailwind keys only on 'dark' so styling is unchanged. Co-authored-by: Orca * fix: add braces to Landing menu close-on-outside-click handler oxlint config requires braces for all if statements. Co-authored-by: Orca * chore: trigger CI Co-authored-by: Orca --------- Co-authored-by: Orca --- config/scripts/dev-fresh-profile.sh | 43 ++ src/main/index.ts | 10 +- src/main/ipc/notifications.ts | 22 + src/main/ipc/onboarding.ts | 16 + src/main/ipc/register-core-handlers.test.ts | 10 +- src/main/ipc/register-core-handlers.ts | 2 + src/main/persistence.ts | 135 ++++++- src/preload/api-types.ts | 15 + src/preload/index.ts | 15 + src/renderer/src/App.tsx | 39 +- src/renderer/src/components/Landing.tsx | 73 +++- .../src/components/onboarding/AgentStep.tsx | 152 +++++++ .../onboarding/NotificationStep.tsx | 76 ++++ .../components/onboarding/OnboardingFlow.tsx | 213 ++++++++++ .../src/components/onboarding/RepoStep.tsx | 104 +++++ .../src/components/onboarding/ThemeStep.tsx | 328 +++++++++++++++ .../onboarding/should-show-onboarding.ts | 7 + .../use-onboarding-flow-persistence.ts | 180 +++++++++ .../onboarding/use-onboarding-flow-types.ts | 13 + .../onboarding/use-onboarding-flow.ts | 347 ++++++++++++++++ src/renderer/src/hooks/useComposerState.ts | 8 +- src/renderer/src/lib/document-theme.ts | 3 + src/renderer/src/lib/editable-target.ts | 24 ++ src/shared/constants.ts | 31 +- src/shared/telemetry-events.ts | 97 ++++- src/shared/types.ts | 36 ++ tests/e2e/helpers/orca-app.ts | 40 ++ tests/e2e/onboarding.spec.ts | 377 ++++++++++++++++++ 28 files changed, 2365 insertions(+), 51 deletions(-) create mode 100755 config/scripts/dev-fresh-profile.sh create mode 100644 src/main/ipc/onboarding.ts create mode 100644 src/renderer/src/components/onboarding/AgentStep.tsx create mode 100644 src/renderer/src/components/onboarding/NotificationStep.tsx create mode 100644 src/renderer/src/components/onboarding/OnboardingFlow.tsx create mode 100644 src/renderer/src/components/onboarding/RepoStep.tsx create mode 100644 src/renderer/src/components/onboarding/ThemeStep.tsx create mode 100644 src/renderer/src/components/onboarding/should-show-onboarding.ts create mode 100644 src/renderer/src/components/onboarding/use-onboarding-flow-persistence.ts create mode 100644 src/renderer/src/components/onboarding/use-onboarding-flow-types.ts create mode 100644 src/renderer/src/components/onboarding/use-onboarding-flow.ts create mode 100644 src/renderer/src/lib/editable-target.ts create mode 100644 tests/e2e/onboarding.spec.ts diff --git a/config/scripts/dev-fresh-profile.sh b/config/scripts/dev-fresh-profile.sh new file mode 100755 index 000000000..3579c840f --- /dev/null +++ b/config/scripts/dev-fresh-profile.sh @@ -0,0 +1,43 @@ +#!/usr/bin/env bash +# Launch `pn dev` with a fresh, isolated userData profile so the app behaves +# like a first-time install (onboarding overlay paints, no persisted repos, +# no saved sessions). Your real `orca-dev` profile is left untouched. +# +# Usage: +# ./config/scripts/dev-fresh-profile.sh # ephemeral temp profile, deleted on exit +# ./config/scripts/dev-fresh-profile.sh --keep # keep the profile dir after exit +# ORCA_FRESH_PROFILE_DIR=/some/path ./config/scripts/dev-fresh-profile.sh # use a fixed dir +set -euo pipefail + +KEEP=0 +for arg in "$@"; do + case "$arg" in + --keep) KEEP=1 ;; + -h|--help) + sed -n '2,9p' "$0" | sed 's/^# \{0,1\}//' + exit 0 ;; + *) + echo "Unknown argument: $arg" >&2 + echo "Usage: $0 [--keep] [--help]" >&2 + exit 2 ;; + esac +done + +PROFILE_DIR="${ORCA_FRESH_PROFILE_DIR:-$(mktemp -d "${TMPDIR:-/tmp}/orca-fresh-profile.XXXXXXXX")}" +mkdir -p "$PROFILE_DIR" + +cleanup() { + if [[ "$KEEP" -eq 0 && -z "${ORCA_FRESH_PROFILE_DIR:-}" ]]; then + # Guard rm -rf against accidental empty/unrelated PROFILE_DIR. + [[ -n "${PROFILE_DIR:-}" && -d "$PROFILE_DIR" && "$PROFILE_DIR" == */orca-fresh-profile* ]] || return 0 + rm -rf "$PROFILE_DIR" + echo "[dev-fresh-profile] removed $PROFILE_DIR" + else + echo "[dev-fresh-profile] kept $PROFILE_DIR" + fi +} +trap cleanup EXIT + +echo "[dev-fresh-profile] using userData=$PROFILE_DIR" +# Don't exec — we need the EXIT trap to fire so the temp profile gets cleaned up. +ORCA_DEV_USER_DATA_PATH="$PROFILE_DIR" pnpm dev diff --git a/src/main/index.ts b/src/main/index.ts index fb583c98c..8c1f4c7e2 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -614,7 +614,15 @@ app.whenReady().then(async () => { // dialog either doesn't appear or gets immediately covered by the maximized // window, making it impossible for the user to click "Allow". win.once('show', () => { - triggerStartupNotificationRegistration(store!) + // Why: store can be null if init failed earlier; bail rather than risk a + // throw inside an Electron event listener. + if (!store) { + return + } + const onboarding = store.getOnboarding() + if (onboarding.closedAt !== null) { + triggerStartupNotificationRegistration(store) + } }) app.on('activate', () => { diff --git a/src/main/ipc/notifications.ts b/src/main/ipc/notifications.ts index 8ec5752ee..e67b3fa08 100644 --- a/src/main/ipc/notifications.ts +++ b/src/main/ipc/notifications.ts @@ -5,6 +5,7 @@ import type { Store } from '../persistence' import type { NotificationDispatchRequest, NotificationDispatchResult, + NotificationPermissionStatusResult, NotificationSoundDataResult } from '../../shared/types' import type { OrcaRuntimeService } from '../runtime/orca-runtime' @@ -31,6 +32,8 @@ export function registerNotificationHandlers(store: Store, runtime?: OrcaRuntime const recentNotifications = new Map() ipcMain.removeHandler('notifications:openSystemSettings') + ipcMain.removeHandler('notifications:getPermissionStatus') + ipcMain.removeHandler('notifications:requestPermission') ipcMain.handle('notifications:openSystemSettings', (): void => { if (process.platform === 'darwin') { // Deep-link into the macOS Notifications settings pane. @@ -40,6 +43,25 @@ export function registerNotificationHandlers(store: Store, runtime?: OrcaRuntime } }) + // Why: Electron's main-process `Notification` class exposes no synchronous + // way to read macOS auth status — the renderer-side `Notification.permission` + // does not exist here. We expose what we can reliably observe: whether the + // platform supports notifications and whether we've already kicked off the + // first-permission prompt. A 'denied' OS result is invisible to us; the + // dispatch path simply won't deliver in that case, which the user can + // diagnose via the System Settings deep-link. + const getPermissionStatus = (): NotificationPermissionStatusResult => ({ + supported: Notification.isSupported(), + platform: process.platform, + requested: store.getUI().notificationPermissionRequested === true + }) + + ipcMain.handle('notifications:getPermissionStatus', getPermissionStatus) + ipcMain.handle('notifications:requestPermission', (): NotificationPermissionStatusResult => { + triggerStartupNotificationRegistration(store) + return getPermissionStatus() + }) + ipcMain.removeHandler('notifications:dispatch') ipcMain.handle( 'notifications:dispatch', diff --git a/src/main/ipc/onboarding.ts b/src/main/ipc/onboarding.ts new file mode 100644 index 000000000..c5449d6a8 --- /dev/null +++ b/src/main/ipc/onboarding.ts @@ -0,0 +1,16 @@ +import { ipcMain } from 'electron' +import { sanitizeOnboardingUpdate, type Store } from '../persistence' +import type { OnboardingState } from '../../shared/types' + +export function registerOnboardingHandlers(store: Store): void { + ipcMain.removeHandler('onboarding:get') + ipcMain.removeHandler('onboarding:update') + + ipcMain.handle('onboarding:get', (): OnboardingState => store.getOnboarding()) + // Why: never trust renderer input — a compromised/buggy caller could send + // unknown keys or wrong-typed values that would poison persisted state. + // Run every update through the shared whitelist sanitizer. + ipcMain.handle('onboarding:update', (_event, updates: unknown): OnboardingState => { + return store.updateOnboarding(sanitizeOnboardingUpdate(updates)) + }) +} diff --git a/src/main/ipc/register-core-handlers.test.ts b/src/main/ipc/register-core-handlers.test.ts index 3002d53d5..40840246e 100644 --- a/src/main/ipc/register-core-handlers.test.ts +++ b/src/main/ipc/register-core-handlers.test.ts @@ -32,7 +32,8 @@ const { registerFilesystemWatcherHandlersMock, registerAppHandlersMock, registerLinearHandlersMock, - registerExportHandlersMock + registerExportHandlersMock, + registerOnboardingHandlersMock } = vi.hoisted(() => ({ registerCliHandlersMock: vi.fn(), registerPreflightHandlersMock: vi.fn(), @@ -65,7 +66,12 @@ const { registerFilesystemWatcherHandlersMock: vi.fn(), registerAppHandlersMock: vi.fn(), registerLinearHandlersMock: vi.fn(), - registerExportHandlersMock: vi.fn() + registerExportHandlersMock: vi.fn(), + registerOnboardingHandlersMock: vi.fn() +})) + +vi.mock('./onboarding', () => ({ + registerOnboardingHandlers: registerOnboardingHandlersMock })) vi.mock('./cli', () => ({ diff --git a/src/main/ipc/register-core-handlers.ts b/src/main/ipc/register-core-handlers.ts index 4fa4d2237..b69896f3b 100644 --- a/src/main/ipc/register-core-handlers.ts +++ b/src/main/ipc/register-core-handlers.ts @@ -17,6 +17,7 @@ import { registerMemoryHandlers } from './memory' import { registerRateLimitHandlers } from './rate-limits' import { registerRuntimeHandlers } from './runtime' import { registerNotificationHandlers } from './notifications' +import { registerOnboardingHandlers } from './onboarding' import { registerDeveloperPermissionHandlers } from './developer-permissions' import { setTrustedBrowserRendererWebContentsId, setAgentBrowserBridgeRef } from './browser' import { registerSessionHandlers } from './session' @@ -83,6 +84,7 @@ export function registerCoreHandlers( registerStatsHandlers(stats) registerMemoryHandlers(store) registerNotificationHandlers(store, runtime) + registerOnboardingHandlers(store) registerDeveloperPermissionHandlers() registerSettingsHandlers(store) registerTelemetryHandlers(store) diff --git a/src/main/persistence.ts b/src/main/persistence.ts index 3cd7dc81b..20d486f66 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -12,7 +12,10 @@ import type { Repo, SparsePreset, WorktreeMeta, - GlobalSettings + GlobalSettings, + OnboardingChecklistState, + OnboardingOutcome, + OnboardingState } from '../shared/types' import type { SshTarget } from '../shared/ssh-types' import { isFolderRepo } from '../shared/repo-kind' @@ -20,9 +23,11 @@ import { getGitUsername } from './git/repo' import { getDefaultPersistedState, getDefaultNotificationSettings, + getDefaultOnboardingState, getDefaultUIState, getDefaultRepoHookSettings, - getDefaultWorkspaceSession + getDefaultWorkspaceSession, + ONBOARDING_FINAL_STEP } from '../shared/constants' import { parseWorkspaceSession } from '../shared/workspace-session-schema' @@ -103,6 +108,68 @@ function normalizeSshTarget(t: SshTarget): SshTarget { return { ...t, configHost: t.configHost ?? t.label ?? t.host } } +// Why: shared by load-time merge and the IPC update handler so the same +// strict whitelist guards every entry into onboarding state — arbitrary +// renderer/disk input cannot inject unknown keys or wrong-typed values. +// Returns only validated fields; unknown keys are dropped silently. +// Why: returns Partial<...> with a partial checklist so the IPC update path +// merges over current state without wiping previously-true keys. Invalid +// top-level fields are OMITTED (not coerced to fallbacks) so partial updates +// don't clobber valid persisted state; the load-path caller spreads defaults. +export function sanitizeOnboardingUpdate( + input: unknown +): Partial> & { checklist?: Partial } { + if (!input || typeof input !== 'object' || Array.isArray(input)) { + return {} + } + const raw = input as Record + const out: Partial> & { + checklist?: Partial + } = {} + + if ('closedAt' in raw) { + if (typeof raw.closedAt === 'number') { + out.closedAt = raw.closedAt + } else if (raw.closedAt === null) { + out.closedAt = null + } + // else: omit — preserve existing persisted value on merge. + } + if ('outcome' in raw) { + const v = raw.outcome + if (v === 'completed' || v === 'dismissed') { + out.outcome = v as OnboardingOutcome + } else if (v === null) { + out.outcome = null + } + // else: omit. + } + if ('lastCompletedStep' in raw) { + const v = raw.lastCompletedStep + if (typeof v === 'number' && Number.isInteger(v) && v >= -1 && v <= ONBOARDING_FINAL_STEP) { + out.lastCompletedStep = v + } + // else: omit. + } + if ('checklist' in raw) { + const rawChecklist = raw.checklist + if (rawChecklist && typeof rawChecklist === 'object' && !Array.isArray(rawChecklist)) { + // Why: copy ONLY caller-sent boolean keys so partial updates (e.g. + // `{ addedRepo: true }`) don't reset other checklist items to false. + const defaults = getDefaultOnboardingState().checklist + const rc = rawChecklist as Record + const checklist: Partial = {} + for (const key of Object.keys(defaults) as (keyof OnboardingChecklistState)[]) { + if (key in rc && typeof rc[key] === 'boolean') { + checklist[key] = rc[key] as boolean + } + } + out.checklist = checklist + } + } + return out +} + // 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. @@ -284,7 +351,37 @@ export class Store { } return { ...defaults.workspaceSession, ...result.value } })(), - sshTargets: (parsed.sshTargets ?? []).map(normalizeSshTarget) + sshTargets: (parsed.sshTargets ?? []).map(normalizeSshTarget), + 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) + return { + ...defaults.onboarding, + ...sanitized, + checklist: { + ...defaults.onboarding.checklist, + ...sanitized.checklist + } + } + })() } } } catch (err) { @@ -671,6 +768,38 @@ export class Store { this.scheduleSave() } + // ── Onboarding ──────────────────────────────────────────────────── + + getOnboarding(): PersistedState['onboarding'] { + const defaults = getDefaultOnboardingState() + return { + ...defaults, + ...this.state.onboarding, + checklist: { + ...defaults.checklist, + ...this.state.onboarding?.checklist + } + } + } + + updateOnboarding( + updates: Partial> & { + checklist?: Partial + } + ): PersistedState['onboarding'] { + const current = this.getOnboarding() + this.state.onboarding = { + ...current, + ...updates, + checklist: { + ...current.checklist, + ...updates.checklist + } + } + this.scheduleSave() + return this.getOnboarding() + } + // ── GitHub Cache ────────────────────────────────────────────────── getGitHubCache(): PersistedState['githubCache'] { diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 9e623d1ca..e7b29851c 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -45,7 +45,9 @@ import type { GetRateLimitResult, NotificationDispatchRequest, NotificationDispatchResult, + NotificationPermissionStatusResult, NotificationSoundResult, + OnboardingState, OrcaHooks, PersistedUIState, PRCheckDetail, @@ -750,8 +752,21 @@ export type PreloadApi = { notifications: { dispatch: (args: NotificationDispatchRequest) => Promise openSystemSettings: () => Promise + getPermissionStatus: () => Promise + requestPermission: () => Promise playSound: (options?: { force?: boolean }) => Promise } + onboarding: { + get: () => Promise + // Why: main-process `updateOnboarding` merges checklist field-by-field, so + // callers can pass a partial checklist (e.g. just `{ addedRepo: true }`) + // without re-supplying every flag. + update: ( + updates: Partial> & { + checklist?: Partial + } + ) => Promise + } developerPermissions: { getStatus: () => Promise request: (args: { id: DeveloperPermissionId }) => Promise diff --git a/src/preload/index.ts b/src/preload/index.ts index 03d61ebe3..8bf96d538 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -21,9 +21,11 @@ import type { ListWorkItemsResult, MemorySnapshot, NotificationDispatchResult, + NotificationPermissionStatusResult, NotificationSoundDataResult, NotificationSoundPathResult, NotificationSoundResult, + OnboardingState, SearchResult } from '../shared/types' import type { RuntimeStatus, RuntimeSyncWindowGraph } from '../shared/runtime-types' @@ -910,6 +912,10 @@ const api = { dispatch: (args: Record): Promise => ipcRenderer.invoke('notifications:dispatch', args), openSystemSettings: (): Promise => ipcRenderer.invoke('notifications:openSystemSettings'), + getPermissionStatus: (): Promise => + ipcRenderer.invoke('notifications:getPermissionStatus'), + requestPermission: (): Promise => + ipcRenderer.invoke('notifications:requestPermission'), playSound: async (options?: { force?: boolean }): Promise => { try { // Why: drop replays while the sound is still ringing. The "test" @@ -971,6 +977,15 @@ const api = { } }, + onboarding: { + get: (): Promise => ipcRenderer.invoke('onboarding:get'), + update: ( + updates: Partial> & { + checklist?: Partial + } + ): Promise => ipcRenderer.invoke('onboarding:update', updates) + }, + developerPermissions: { getStatus: (): Promise => ipcRenderer.invoke('developerPermissions:getStatus'), request: (args: { id: string }): Promise => diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index b18483f74..d0d41a339 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -31,6 +31,7 @@ import { UpdateCard } from './components/UpdateCard' import { StarNagCard } from './components/StarNagCard' import { TelemetryFirstLaunchSurface } from './components/TelemetryFirstLaunchSurface' import { ZoomOverlay } from './components/ZoomOverlay' +import { shouldShowOnboarding } from './components/onboarding/should-show-onboarding' import { SshPassphraseDialog } from './components/settings/SshPassphraseDialog' import { useGitStatusPolling } from './components/right-sidebar/useGitStatusPolling' import { useEditorExternalWatch } from './hooks/useEditorExternalWatch' @@ -45,12 +46,14 @@ import { buildWorkspaceSessionPayload } from './lib/workspace-session' import { countWorkingAgents, getWorkingAgentsPerWorktree } from './lib/agent-status' import { activateAndRevealWorktree } from './lib/worktree-activation' import { applyDocumentTheme } from './lib/document-theme' +import { isEditableTarget } from './lib/editable-target' import { Popover, PopoverTrigger, PopoverContent } from '@/components/ui/popover' import { findWorktreeById, getRepoIdFromWorktreeId } from '@/store/slices/worktree-helpers' import { canGoBackWorktreeHistory, canGoForwardWorktreeHistory } from '@/store/slices/worktree-nav-history' +import type { OnboardingState } from '../../shared/types' const isMac = navigator.userAgent.includes('Mac') const isWindows = !isMac && navigator.userAgent.includes('Windows') @@ -117,28 +120,10 @@ const NewWorkspaceComposerModal = lazy(() => import('./components/NewWorkspaceCo // Why: lazy-loaded so the WebP asset + overlay module aren't fetched unless // the user opts into the experimental flag. const PetOverlay = lazy(() => import('./components/pet/PetOverlay')) - -function isEditableTarget(target: EventTarget | null): boolean { - if (!(target instanceof HTMLElement)) { - return false - } - - // xterm.js focuses a hidden