diff --git a/src/renderer/src/components/settings/SettingsSidebar.test.tsx b/src/renderer/src/components/settings/SettingsSidebar.test.tsx index 0b22d4865..ddcec2cf6 100644 --- a/src/renderer/src/components/settings/SettingsSidebar.test.tsx +++ b/src/renderer/src/components/settings/SettingsSidebar.test.tsx @@ -1,18 +1,39 @@ import { renderToStaticMarkup } from 'react-dom/server' import { Bot, Mic, Network } from 'lucide-react' -import { describe, expect, it, vi } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' import { SettingsSidebar } from './SettingsSidebar' import { TooltipProvider } from '../ui/tooltip' +import type { SettingsSetupGuideProgress } from './settings-setup-guide-progress' + +const mocks = vi.hoisted(() => ({ + useSettingsSetupGuideProgress: vi.fn() +})) vi.mock('@/hooks/useShortcutLabel', () => ({ useShortcutLabel: () => '⌘F' })) -function renderSidebar(): string { +vi.mock('./settings-setup-guide-progress', () => ({ + useSettingsSetupGuideProgress: mocks.useSettingsSetupGuideProgress +})) + +function makeSetupGuideProgress( + overrides: Partial = {} +): SettingsSetupGuideProgress { + return { + ready: true, + doneCount: 5, + total: 8, + firstIncompleteStepId: 'agent-capabilities', + ...overrides + } +} + +function renderSidebar(activeSectionId = 'orchestration'): string { return renderToStaticMarkup( { + beforeEach(() => { + mocks.useSettingsSetupGuideProgress.mockReset() + mocks.useSettingsSetupGuideProgress.mockReturnValue(makeSetupGuideProgress()) + }) + it('renders install state labels separately from static badges', () => { const markup = renderSidebar() @@ -69,4 +95,42 @@ describe('SettingsSidebar', () => { expect(markup).toContain('Installed') expect(markup).toContain('Optional') }) + + it('does not render the setup guide row before progress readiness settles', () => { + mocks.useSettingsSetupGuideProgress.mockReturnValue( + makeSetupGuideProgress({ + ready: false, + doneCount: 7, + firstIncompleteStepId: 'setup-script' + }) + ) + + expect(renderSidebar()).not.toContain('Onboarding checklist') + }) + + it('renders incomplete setup progress with the full checklist total', () => { + const markup = renderSidebar() + + expect(markup).toContain('Onboarding checklist') + expect(markup).toContain('Onboarding checklist, 5 of 8 done. Show setup guide.') + expect(markup).toContain('5 of 8 setup steps complete') + }) + + it('does not render the setup guide row after every checklist step is complete', () => { + mocks.useSettingsSetupGuideProgress.mockReturnValue( + makeSetupGuideProgress({ + doneCount: 8, + firstIncompleteStepId: null + }) + ) + + expect(renderSidebar()).not.toContain('Onboarding checklist') + }) + + it('keeps the setup guide row available from Settings when incomplete', () => { + const markup = renderSidebar('setup-guide') + + expect(markup).toContain('aria-current="page"') + expect(markup).toContain('Onboarding checklist') + }) }) diff --git a/src/renderer/src/components/settings/SettingsSidebar.tsx b/src/renderer/src/components/settings/SettingsSidebar.tsx index f3118acc5..66d02a23a 100644 --- a/src/renderer/src/components/settings/SettingsSidebar.tsx +++ b/src/renderer/src/components/settings/SettingsSidebar.tsx @@ -116,7 +116,10 @@ export function SettingsSidebar({ }: SettingsSidebarProps): React.JSX.Element { const setupGuideProgress = useSettingsSetupGuideProgress(true) const setupActive = activeSectionId === 'setup-guide' - const showSetupGuideTopRow = setupGuideProgress.doneCount < setupGuideProgress.total + // Why: "Hide from sidebar" only hides the top-left app sidebar prompt; + // Settings should remain a stable place to reopen the checklist. + const showSetupGuideTopRow = + setupGuideProgress.ready && setupGuideProgress.doneCount < setupGuideProgress.total const searchShortcutHint = useShortcutLabel('settings.search') const navItemClassName = (isActive: boolean): string => cn( diff --git a/src/renderer/src/components/settings/settings-setup-guide-progress-hook.test.tsx b/src/renderer/src/components/settings/settings-setup-guide-progress-hook.test.tsx new file mode 100644 index 000000000..cfd79db87 --- /dev/null +++ b/src/renderer/src/components/settings/settings-setup-guide-progress-hook.test.tsx @@ -0,0 +1,44 @@ +import { renderToStaticMarkup } from 'react-dom/server' +import { describe, expect, it, vi } from 'vitest' +import type { FeatureWallSetupProgress } from '../feature-wall/feature-wall-setup-progress' +import { useSettingsSetupGuideProgress } from './settings-setup-guide-progress' + +const mocks = vi.hoisted(() => ({ + useSetupGuideProgress: vi.fn() +})) + +vi.mock('../setup-guide/use-setup-guide-progress', () => ({ + useSetupGuideProgress: mocks.useSetupGuideProgress +})) + +function makeProgress(): FeatureWallSetupProgress { + return { + ready: true, + stepDone: { + 'default-agent': true, + 'add-two-repos': false, + notifications: true, + 'split-terminal': true, + 'two-worktrees': true, + 'task-sources': true, + 'agent-capabilities': false, + 'setup-script': false + }, + coreDoneCount: 5, + coreTotal: 8 + } +} + +function SettingsProgressProbe(): React.JSX.Element { + const progress = useSettingsSetupGuideProgress(true) + return {`${progress.doneCount}/${progress.total}`} +} + +describe('useSettingsSetupGuideProgress', () => { + it('uses the same setup progress path as the main sidebar', () => { + mocks.useSetupGuideProgress.mockReturnValue(makeProgress()) + + expect(renderToStaticMarkup()).toContain('5/8') + expect(mocks.useSetupGuideProgress).toHaveBeenCalledWith(true, false, false) + }) +}) diff --git a/src/renderer/src/components/settings/settings-setup-guide-progress.test.ts b/src/renderer/src/components/settings/settings-setup-guide-progress.test.ts index ff461f823..daea6d99b 100644 --- a/src/renderer/src/components/settings/settings-setup-guide-progress.test.ts +++ b/src/renderer/src/components/settings/settings-setup-guide-progress.test.ts @@ -1,61 +1,64 @@ import { describe, expect, it } from 'vitest' -import type { FeatureWallSetupStepId } from '../../../../shared/feature-wall-setup-steps' import { - getSettingsSetupGuideProgress, - SETTINGS_SETUP_GUIDE_STEP_IDS -} from './settings-setup-guide-progress' + FEATURE_WALL_SETUP_STEPS, + type FeatureWallSetupStepId +} from '../../../../shared/feature-wall-setup-steps' +import { getSettingsSetupGuideProgress } from './settings-setup-guide-progress' describe('settings setup guide progress', () => { - it('tracks the five-step settings checklist', () => { - expect(SETTINGS_SETUP_GUIDE_STEP_IDS).toEqual([ - 'split-terminal', - 'two-worktrees', - 'notifications', - 'default-agent', - 'task-sources' - ]) - }) + it('tracks the full setup checklist total', () => { + const progress = getSettingsSetupGuideProgress({ + ready: true, + stepDone: {} + }) - it('returns a 3/5 progress label source and first incomplete step', () => { - const stepDone = { - 'split-terminal': true, - 'two-worktrees': true, - notifications: true - } satisfies Partial> - - expect(getSettingsSetupGuideProgress(stepDone)).toEqual({ - doneCount: 3, - total: 5, - firstIncompleteStepId: 'default-agent' + expect(progress).toEqual({ + ready: true, + doneCount: 0, + total: FEATURE_WALL_SETUP_STEPS.length, + firstIncompleteStepId: 'split-terminal' }) }) - it('ignores setup-guide tasks that are not part of the settings checklist', () => { + it('does not mark Settings complete when only the old five-step subset is done', () => { const stepDone = { 'split-terminal': true, 'two-worktrees': true, notifications: true, - 'setup-script': true, - 'add-two-repos': true, - 'agent-capabilities': true + 'default-agent': true, + 'task-sources': true } satisfies Partial> - expect(getSettingsSetupGuideProgress(stepDone)).toEqual({ - doneCount: 3, - total: 5, - firstIncompleteStepId: 'default-agent' + expect(getSettingsSetupGuideProgress({ ready: true, stepDone })).toEqual({ + ready: true, + doneCount: 5, + total: FEATURE_WALL_SETUP_STEPS.length, + firstIncompleteStepId: 'agent-capabilities' }) }) - it('marks the settings checklist complete when all five settings steps are done', () => { + it('marks Settings complete when every setup guide step is done', () => { const stepDone = Object.fromEntries( - SETTINGS_SETUP_GUIDE_STEP_IDS.map((stepId) => [stepId, true]) + FEATURE_WALL_SETUP_STEPS.map((step) => [step.id, true]) ) as Record - expect(getSettingsSetupGuideProgress(stepDone)).toEqual({ - doneCount: 5, - total: 5, + expect(getSettingsSetupGuideProgress({ ready: true, stepDone })).toEqual({ + ready: true, + doneCount: FEATURE_WALL_SETUP_STEPS.length, + total: FEATURE_WALL_SETUP_STEPS.length, firstIncompleteStepId: null }) }) + + it('preserves progress readiness for the sidebar row gate', () => { + const progress = getSettingsSetupGuideProgress({ + ready: false, + stepDone: { + 'split-terminal': true + } + }) + + expect(progress.ready).toBe(false) + expect(progress.doneCount).toBe(1) + }) }) diff --git a/src/renderer/src/components/settings/settings-setup-guide-progress.ts b/src/renderer/src/components/settings/settings-setup-guide-progress.ts index 7a46772aa..0cb50944e 100644 --- a/src/renderer/src/components/settings/settings-setup-guide-progress.ts +++ b/src/renderer/src/components/settings/settings-setup-guide-progress.ts @@ -1,98 +1,40 @@ -import { useEffect, useMemo } from 'react' -import type { FeatureWallSetupStepId } from '../../../../shared/feature-wall-setup-steps' -import { useAppStore } from '@/store' -import { getFeatureWallSetupProgress } from '../feature-wall/feature-wall-setup-progress' - -export const SETTINGS_SETUP_GUIDE_STEP_IDS = [ - 'split-terminal', - 'two-worktrees', - 'notifications', - 'default-agent', - 'task-sources' -] as const satisfies readonly FeatureWallSetupStepId[] +import { useMemo } from 'react' +import { + FEATURE_WALL_SETUP_STEPS, + getFirstIncompleteFeatureWallSetupStepId, + type FeatureWallSetupStepId +} from '../../../../shared/feature-wall-setup-steps' +import { useSetupGuideProgress } from '../setup-guide/use-setup-guide-progress' export type SettingsSetupGuideProgress = { + ready: boolean doneCount: number total: number firstIncompleteStepId: FeatureWallSetupStepId | null } -export function getSettingsSetupGuideProgress( +export function getSettingsSetupGuideProgress(progress: { + ready: boolean stepDone: Partial> -): SettingsSetupGuideProgress { - const doneCount = SETTINGS_SETUP_GUIDE_STEP_IDS.filter((stepId) => stepDone[stepId]).length +}): SettingsSetupGuideProgress { + const doneCount = FEATURE_WALL_SETUP_STEPS.filter((step) => progress.stepDone[step.id]).length const firstIncompleteStepId = - SETTINGS_SETUP_GUIDE_STEP_IDS.find((stepId) => !stepDone[stepId]) ?? null + doneCount === FEATURE_WALL_SETUP_STEPS.length + ? null + : getFirstIncompleteFeatureWallSetupStepId(progress.stepDone) return { + ready: progress.ready, doneCount, - total: SETTINGS_SETUP_GUIDE_STEP_IDS.length, + total: FEATURE_WALL_SETUP_STEPS.length, firstIncompleteStepId } } export function useSettingsSetupGuideProgress( - shouldRefreshTaskSourceState: boolean + shouldRefreshCoreState: boolean ): SettingsSetupGuideProgress { - const settings = useAppStore((s) => s.settings) - const featureInteractions = useAppStore((s) => s.featureInteractions) - const worktreesByRepo = useAppStore((s) => s.worktreesByRepo) - const tabsByWorktree = useAppStore((s) => s.tabsByWorktree) - const terminalLayoutsByTabId = useAppStore((s) => s.terminalLayoutsByTabId) - const preflightStatus = useAppStore((s) => s.preflightStatus) - const preflightStatusChecked = useAppStore((s) => s.preflightStatusChecked) - const refreshPreflightStatus = useAppStore((s) => s.refreshPreflightStatus) - const linearStatus = useAppStore((s) => s.linearStatus) - const linearStatusChecked = useAppStore((s) => s.linearStatusChecked) - const checkLinearConnection = useAppStore((s) => s.checkLinearConnection) + const fullProgress = useSetupGuideProgress(shouldRefreshCoreState, false, false) - useEffect(() => { - if (!shouldRefreshTaskSourceState) { - return - } - if (!preflightStatusChecked) { - void refreshPreflightStatus() - } - if (!linearStatusChecked) { - void checkLinearConnection() - } - }, [ - checkLinearConnection, - linearStatusChecked, - preflightStatusChecked, - refreshPreflightStatus, - shouldRefreshTaskSourceState - ]) - - const hasConnectedTaskSource = - (preflightStatus?.gh.installed === true && preflightStatus.gh.authenticated === true) || - (preflightStatus?.glab?.installed === true && preflightStatus.glab.authenticated === true) || - linearStatus.connected === true - - return useMemo(() => { - // Why: Settings renders only five steps, so avoid the full setup guide - // probes for setup scripts and agent skills, especially over SSH. - const fullProgress = getFeatureWallSetupProgress({ - settings, - featureInteractions, - hasConnectedTaskSource, - browserUseSkillInstalled: false, - computerUseSkillInstalled: false, - computerUsePermissionsReady: false, - orchestrationSkillInstalled: false, - gitRepoCount: 0, - worktreesByRepo, - tabsByWorktree, - terminalLayoutsByTabId, - hasSetupScript: false - }) - return getSettingsSetupGuideProgress(fullProgress.stepDone) - }, [ - featureInteractions, - hasConnectedTaskSource, - settings, - tabsByWorktree, - terminalLayoutsByTabId, - worktreesByRepo - ]) + return useMemo(() => getSettingsSetupGuideProgress(fullProgress), [fullProgress]) } diff --git a/src/renderer/src/components/setup-guide/use-setup-guide-progress.ts b/src/renderer/src/components/setup-guide/use-setup-guide-progress.ts index 5f5741bb7..518df6e17 100644 --- a/src/renderer/src/components/setup-guide/use-setup-guide-progress.ts +++ b/src/renderer/src/components/setup-guide/use-setup-guide-progress.ts @@ -1,5 +1,5 @@ /* oxlint-disable react-doctor/no-adjust-state-on-prop-change -- Why: setup-guide readiness is driven by bounded IPC probes and browser focus events; the state cannot be derived synchronously from render inputs. */ -import { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { useCallback, useEffect, useMemo, useRef, useState, useSyncExternalStore } from 'react' import { useAppStore } from '@/store' import { isGitRepoKind } from '../../../../shared/repo-kind' import { checkRuntimeHooks } from '@/runtime/runtime-hooks-client' @@ -28,6 +28,34 @@ import { const SETUP_SCRIPT_PROBE_SETTLE_TIMEOUT_MS = 15_000 +const setupScriptProbeCacheListeners = new Set<() => void>() +let setupScriptProbeCache = INITIAL_SETUP_SCRIPT_PROBE_STATE + +function readSetupScriptProbeCache(): SetupScriptProbeState { + return setupScriptProbeCache +} + +function subscribeSetupScriptProbeCache(listener: () => void): () => void { + setupScriptProbeCacheListeners.add(listener) + return () => { + setupScriptProbeCacheListeners.delete(listener) + } +} + +function setSetupScriptProbeCache(next: SetupScriptProbeState): void { + if ( + setupScriptProbeCache.signature === next.signature && + setupScriptProbeCache.ready === next.ready && + setupScriptProbeCache.hasSetupScript === next.hasSetupScript + ) { + return + } + setupScriptProbeCache = next + for (const listener of setupScriptProbeCacheListeners) { + listener() + } +} + export function useSetupGuideProgress( shouldRefreshCoreState: boolean, orchestrationSkillInstalled: boolean, @@ -46,8 +74,10 @@ export function useSetupGuideProgress( const checkLinearConnection = useAppStore((s) => s.checkLinearConnection) const repos = useAppStore((s) => s.repos) const activeRepoId = useAppStore((s) => s.activeRepoId) - const [setupScriptProbe, setSetupScriptProbe] = useState( - INITIAL_SETUP_SCRIPT_PROBE_STATE + const setupScriptProbe = useSyncExternalStore( + subscribeSetupScriptProbeCache, + readSetupScriptProbeCache, + readSetupScriptProbeCache ) const [computerUsePermissionsReady, setComputerUsePermissionsReady] = useState(false) const [computerUsePermissionStatusChecked, setComputerUsePermissionStatusChecked] = @@ -116,14 +146,14 @@ export function useSetupGuideProgress( // visibility readiness so a wedged read cannot hide the checklist forever. const timeoutId = window.setTimeout(() => { if (activeSetupScriptProbeSignatureRef.current === signature) { - setSetupScriptProbe({ signature, ready: true, hasSetupScript: false }) + setSetupScriptProbeCache({ signature, ready: true, hasSetupScript: false }) } }, SETUP_SCRIPT_PROBE_SETTLE_TIMEOUT_MS) const settle = (hasSetupScript: boolean): void => { window.clearTimeout(timeoutId) if (activeSetupScriptProbeSignatureRef.current === signature) { - setSetupScriptProbe({ signature, ready: true, hasSetupScript }) + setSetupScriptProbeCache({ signature, ready: true, hasSetupScript }) } }