From 3bd04589f9d65698541cd0571fcb6e481723c7a1 Mon Sep 17 00:00:00 2001 From: GCWing Date: Sat, 10 Oct 2026 10:21:00 +0800 Subject: [PATCH 1/2] fix(settings): stabilize loading and isolate warm snapshots --- src/web-ui/src/app/scenes/settings/AGENTS.md | 8 ++ .../app/scenes/settings/SettingsNav.test.tsx | 33 ++++++ .../src/app/scenes/settings/SettingsNav.tsx | 39 +++++-- .../app/scenes/settings/SettingsScene.scss | 51 --------- .../scenes/settings/SettingsScene.test.tsx | 66 ++++++++++- .../src/app/scenes/settings/SettingsScene.tsx | 108 ++++++++---------- .../pages/ai/DefaultHarnessSection.tsx | 11 +- .../pages/ai/ExecutionSettingsPage.tsx | 24 ++-- .../pages/ai/MemorySettingsSection.tsx | 14 ++- .../ai/ModelSettingsPage.loading.test.tsx | 16 +++ .../settings/pages/ai/ModelSettingsPage.tsx | 48 ++++---- .../pages/ai/PermissionsSettingsPage.tsx | 14 ++- .../settings/pages/ai/SessionTitleSection.tsx | 21 ++-- .../pages/application/GeneralSettingsPage.tsx | 30 ++--- .../pages/application/PetSettingsSection.tsx | 22 ++-- .../TextSelectionSettingsSection.test.tsx | 2 +- .../application/VoiceSettingsSection.tsx | 21 +++- .../data/ArchivedSessionsSettingsPage.tsx | 3 +- .../pages/data/DiagnosticsSettingsPage.tsx | 37 ++++-- .../data/UsageStatisticsSettingsPage.test.tsx | 25 ++++ .../data/UsageStatisticsSettingsPage.tsx | 34 ++++-- .../development/EditorSettingsPage.test.tsx | 2 +- .../pages/development/EditorSettingsPage.tsx | 14 ++- .../GitCommitSettingsSection.test.tsx | 2 +- .../development/GitCommitSettingsSection.tsx | 9 +- .../development/TerminalSettingsPage.tsx | 29 +++-- .../development/WorkspaceSearchSection.tsx | 7 +- .../WorktreeSettingsSection.test.tsx | 9 ++ .../development/WorktreeSettingsSection.tsx | 19 +-- .../RuntimeSettings.presentation.test.ts | 2 +- .../pages/shared/RuntimeSettings.test.tsx | 4 +- .../pages/tools/HooksSettingsSection.test.tsx | 2 +- .../pages/tools/HooksSettingsSection.tsx | 15 ++- .../tools/QuickActionsSettingsSection.tsx | 9 +- .../pages/tools/WebSearchSettingsPage.tsx | 7 +- .../scenes/settings/settingsDataPreload.ts | 36 ++++++ .../scenes/settings/settingsRegistry.test.ts | 32 +++++- .../app/scenes/settings/settingsRegistry.ts | 34 +++++- .../useSettingsScrollRestoration.test.tsx | 72 ++++++++++++ .../settings/useSettingsScrollRestoration.ts | 61 ++++++++++ .../selection/FlowChatSelectionBar.test.tsx | 2 + .../infrastructure/api/service-api/MCPAPI.ts | 14 ++- .../src/infrastructure/config/AGENTS.md | 9 ++ .../config/components/McpToolsConfig.test.tsx | 22 ++++ .../config/components/McpToolsConfig.tsx | 50 +++++--- .../components/UsageActivityHeatmap.tsx | 32 ++++-- .../common/ConfigLoadingState.test.tsx | 37 ++++++ .../components/common/ConfigLoadingState.tsx | 30 +++-- .../components/common/ConfigPageHeader.scss | 27 +---- .../components/common/ConfigPageLayout.scss | 21 ++-- .../components/common/ConfigPageState.scss | 62 ++++++++-- .../common/config-page-layout.tokens.scss | 11 ++ .../hooks/useAIExperienceSettings.test.tsx | 83 ++++++++++++++ .../config/hooks/useAIExperienceSettings.ts | 60 +++++----- .../config/hooks/useConfigSeed.test.tsx | 47 ++++++++ .../config/hooks/useConfigSeed.ts | 17 +++ .../useSelectionToolbarPreference.test.tsx | 2 + .../hooks/useSelectionToolbarPreference.ts | 5 +- .../AIExperienceConfigService.test.ts | 26 +++++ .../services/AIExperienceConfigService.ts | 36 +++++- .../config/services/ConfigManager.test.ts | 45 ++++++++ .../config/services/ConfigManager.ts | 55 +++++++++ .../services/PermissionConfigService.test.ts | 6 + .../services/PermissionConfigService.ts | 3 +- .../config/services/SettingsReadCache.test.ts | 65 +++++++++++ .../config/services/SettingsReadCache.ts | 39 +++++++ .../src/infrastructure/config/types/index.ts | 3 + 67 files changed, 1403 insertions(+), 398 deletions(-) create mode 100644 src/web-ui/src/app/scenes/settings/settingsDataPreload.ts create mode 100644 src/web-ui/src/app/scenes/settings/useSettingsScrollRestoration.test.tsx create mode 100644 src/web-ui/src/app/scenes/settings/useSettingsScrollRestoration.ts create mode 100644 src/web-ui/src/infrastructure/config/components/common/ConfigLoadingState.test.tsx create mode 100644 src/web-ui/src/infrastructure/config/hooks/useAIExperienceSettings.test.tsx create mode 100644 src/web-ui/src/infrastructure/config/hooks/useConfigSeed.test.tsx create mode 100644 src/web-ui/src/infrastructure/config/hooks/useConfigSeed.ts create mode 100644 src/web-ui/src/infrastructure/config/services/SettingsReadCache.test.ts create mode 100644 src/web-ui/src/infrastructure/config/services/SettingsReadCache.ts diff --git a/src/web-ui/src/app/scenes/settings/AGENTS.md b/src/web-ui/src/app/scenes/settings/AGENTS.md index da81df569f..6a29e939be 100644 --- a/src/web-ui/src/app/scenes/settings/AGENTS.md +++ b/src/web-ui/src/app/scenes/settings/AGENTS.md @@ -26,6 +26,14 @@ Follow `src/web-ui/AGENTS.md` and the settings control sizing rules in rename generated protocol IDs to match sidebar labels. - Moving a setting must preserve its execution host, remote capability gates, unsupported states, persistence keys, and draft registration. +- Page preparation must retain the scene frame and use `SettingsPage` for its + fallback. First-load placeholders use `ConfigLoadingState` inside the content + frame; refreshes retain successful content. Seed editable forms only from a + complete `ConfigManager` snapshot, never from default values after a failed + read. Do not hydrate over edits or keep inactive page effects alive for caching. +- Runtime snapshots and scroll positions belong to the current device activation. + Background responses must not repopulate a previous activation; section links + take precedence over restored scroll positions. ## Focused verification diff --git a/src/web-ui/src/app/scenes/settings/SettingsNav.test.tsx b/src/web-ui/src/app/scenes/settings/SettingsNav.test.tsx index a5434e6484..295efbbc5d 100644 --- a/src/web-ui/src/app/scenes/settings/SettingsNav.test.tsx +++ b/src/web-ui/src/app/scenes/settings/SettingsNav.test.tsx @@ -46,6 +46,7 @@ vi.mock('@/shared/utils/motionPreference', () => ({ })); import SettingsNav from './SettingsNav'; +import { preloadSettingsPage } from './settingsRegistry'; import { useSettingsStore } from './settingsStore'; import { registerSettingsDraft, @@ -58,6 +59,7 @@ describe('SettingsNav shared component composition', () => { beforeEach(() => { vi.useFakeTimers(); + vi.mocked(preloadSettingsPage).mockReset().mockResolvedValue(undefined); resetSettingsDraftRegistryForTests(); useSettingsStore.setState(useSettingsStore.getInitialState()); container = document.createElement('div'); @@ -183,4 +185,35 @@ describe('SettingsNav shared component composition', () => { expect(useSettingsStore.getState().activePageId).toBe('application.pet'); expect(useSettingsStore.getState().activeSectionId).toBeNull(); }); + it('marks a slow destination pending before showing its loading page', async () => { + vi.mocked(preloadSettingsPage).mockReturnValue(new Promise(() => {})); + const item = container.querySelector('[data-settings-page="application.appearance"]') as HTMLButtonElement; + await act(async () => item.click()); + expect(item.getAttribute('aria-busy')).toBe('true'); + expect(useSettingsStore.getState().activePageId).toBe('application.general'); + await act(async () => vi.advanceTimersByTimeAsync(180)); + expect(useSettingsStore.getState().activePageId).toBe('application.appearance'); + expect(item.hasAttribute('aria-busy')).toBe(false); + }); + + it('ignores an earlier click when its preload completes after the latest selection', async () => { + let finishOld!: () => void; + vi.mocked(preloadSettingsPage).mockImplementation(pageId => pageId === 'application.appearance' + ? new Promise(resolve => { finishOld = resolve; }) : Promise.resolve()); + await act(async () => (container.querySelector('[data-settings-page="application.appearance"]') as HTMLButtonElement).click()); + await act(async () => (container.querySelector('[data-settings-page="application.pet"]') as HTMLButtonElement).click()); + await act(async () => finishOld()); + await act(async () => vi.advanceTimersByTimeAsync(180)); + expect(useSettingsStore.getState().activePageId).toBe('application.pet'); + }); + + it('does not override a newer programmatic destination with a pending click', async () => { + let finish!: () => void; + vi.mocked(preloadSettingsPage).mockReturnValue(new Promise(resolve => { finish = resolve; })); + await act(async () => (container.querySelector('[data-settings-page="application.appearance"]') as HTMLButtonElement).click()); + await act(async () => useSettingsStore.getState().openPage('application.pet')); + await act(async () => finish()); + expect(useSettingsStore.getState().activePageId).toBe('application.pet'); + }); + }); diff --git a/src/web-ui/src/app/scenes/settings/SettingsNav.tsx b/src/web-ui/src/app/scenes/settings/SettingsNav.tsx index 8a03a68c85..20201a606f 100644 --- a/src/web-ui/src/app/scenes/settings/SettingsNav.tsx +++ b/src/web-ui/src/app/scenes/settings/SettingsNav.tsx @@ -1,4 +1,5 @@ import { useSettingsDraftSnapshot } from '@/infrastructure/config/settingsDraftRegistry'; +import { getActiveSurfaceScope } from '@/infrastructure/peer-device/deviceSurface'; import { useI18n } from '@/infrastructure/i18n/hooks/useI18n'; import { getInteractionMotion } from '@/shared/utils/motionPreference'; import { @@ -11,10 +12,10 @@ import { NavigationPanelSection, OverflowText, SearchField, + Spinner, } from '@openbitfun/ui'; import type { i18n as I18nApi } from 'i18next'; import React, { - startTransition, useCallback, useEffect, useMemo, @@ -138,6 +139,13 @@ const SettingsNav: React.FC = () => { const searchInputRef = useRef(null); const resultsRef = useRef(null); const activationRequestRef = useRef(0); + const [pendingPageId, setPendingPageId] = useState(null); + const activationTimerRef = useRef(); + + useEffect(() => () => { + activationRequestRef.current += 1; + window.clearTimeout(activationTimerRef.current); + }, []); useEffect(() => { const timer = window.setTimeout(() => setSearchQuery(draftQuery), SEARCH_DEBOUNCE_MS); @@ -186,14 +194,27 @@ const SettingsNav: React.FC = () => { const activate = useCallback((destination: SettingsDestination, clear = false) => { const requestId = ++activationRequestRef.current; + const scope = getActiveSurfaceScope(); + const navigationRequestId = useSettingsStore.getState().navigationRequestId; const motion = getInteractionMotion(); + window.clearTimeout(activationTimerRef.current); + setPendingPageId(destination.pageId); + let committed = false; const commit = () => { - if (requestId !== activationRequestRef.current) return; - startTransition(() => { - openDestination(destination, motion); - if (clear) clearSearch(); - }); + if (committed || requestId !== activationRequestRef.current) return; + if (!scope.isCurrent() || useSettingsStore.getState().navigationRequestId !== navigationRequestId) { + window.clearTimeout(activationTimerRef.current); + setPendingPageId(null); + return; + } + committed = true; + window.clearTimeout(activationTimerRef.current); + setPendingPageId(null); + openDestination(destination, motion); + if (clear) clearSearch(); }; + // Keep the current page during fast preparation; slow loads get a titled skeleton. + activationTimerRef.current = window.setTimeout(commit, 180); void preloadSettingsPage(destination.pageId).then(commit, commit); }, [clearSearch, openDestination]); @@ -301,6 +322,7 @@ const SettingsNav: React.FC = () => { id={`settings-nav-result-${index}`} role="option" aria-selected={active} + aria-busy={pendingPageId === row.destination.pageId || undefined} selected={active} data-openbitfun-component="settings-nav" data-openbitfun-part="searchResult" @@ -325,7 +347,7 @@ const SettingsNav: React.FC = () => { {highlightFirstMatch(row.description, searchQuery)} - {dirtyMarker(row.destination.pageId)} + {pendingPageId === row.destination.pageId ? : dirtyMarker(row.destination.pageId)} ); })} @@ -362,12 +384,13 @@ const SettingsNav: React.FC = () => { data-openbitfun-state={activePageId === page.id ? 'active' : undefined} className="openbitfun-settings-nav__item" selected={activePageId === page.id} + aria-busy={pendingPageId === page.id || undefined} onClick={() => activate({ pageId: page.id })} onPointerEnter={() => preload(page.id)} onFocus={() => preload(page.id)} > {t(page.labelKey)} - {dirtyMarker(page.id)} + {pendingPageId === page.id ? : dirtyMarker(page.id)} ))} diff --git a/src/web-ui/src/app/scenes/settings/SettingsScene.scss b/src/web-ui/src/app/scenes/settings/SettingsScene.scss index 6b3837e45c..2a5340504c 100644 --- a/src/web-ui/src/app/scenes/settings/SettingsScene.scss +++ b/src/web-ui/src/app/scenes/settings/SettingsScene.scss @@ -9,57 +9,6 @@ height: 100%; overflow: hidden; - &__loading { - flex: 1; - padding: var(--openbitfun-space-6); - } - - &__loading-line, - &__loading-block { - border-radius: var(--openbitfun-radius-md); - background: var(--openbitfun-color-action-quiet-hover); - opacity: 0.65; - } - - &__loading-line { - height: 14px; - max-width: 360px; - margin-bottom: var(--openbitfun-space-3); - - &--title { - height: 20px; - max-width: 220px; - margin-bottom: var(--openbitfun-space-5); - } - } - - &__loading-block { - height: 160px; - max-width: 720px; - margin-top: var(--openbitfun-space-6); - } - - &__content-stack { - flex: 1; - min-width: 0; - min-height: 0; - overflow: hidden; - } - - &__content-transition { - flex: 1 1 auto; - width: 100%; - height: 100%; - min-width: 0; - min-height: 0; - overflow: hidden; - - > .openbitfun-view-transition-boundary__view { - width: 100%; - height: 100%; - } - } - &__content-wrapper { display: flex; flex-direction: column; diff --git a/src/web-ui/src/app/scenes/settings/SettingsScene.test.tsx b/src/web-ui/src/app/scenes/settings/SettingsScene.test.tsx index 21cc93460b..6f1a15b9aa 100644 --- a/src/web-ui/src/app/scenes/settings/SettingsScene.test.tsx +++ b/src/web-ui/src/app/scenes/settings/SettingsScene.test.tsx @@ -4,6 +4,19 @@ import React, { act } from 'react'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { createRoot, type Root } from 'react-dom/client'; +const preparation = vi.hoisted(() => ({ + ready: vi.fn(() => true), + preload: vi.fn(async (_pageId: string) => undefined), + loading: vi.fn(), +})); +vi.mock('./pages/shared/SettingsPage', () => ({ + SettingsPage: ({ pageId, children }: { pageId: string; children: React.ReactNode }) =>

{pageId}

{children}
, +})); +vi.mock('@/infrastructure/config/components/common', () => ({ + ConfigLoadingState: () => { preparation.loading(); return
; }, + ConfigRetryState: ({ onRetry }: { onRetry: () => void }) => , +})); + vi.mock('./settingsRegistry', () => { const pages = { 'application.general': { @@ -33,12 +46,13 @@ vi.mock('./settingsRegistry', () => { DEFAULT_SETTINGS_PAGE_ID: 'application.general', getSettingsPageManifest: (pageId: keyof typeof pages) => pages[pageId] ?? pages['application.general'], isSettingsPageId: (value: string) => value in pages, - isSettingsPageReady: () => true, - preloadSettingsPage: vi.fn(async () => undefined), + isSettingsPageReady: preparation.ready, + preloadSettingsPage: preparation.preload, }; }); import SettingsScene from './SettingsScene'; +import { activateSurface } from '@/infrastructure/peer-device/deviceSurface'; import { useSettingsStore } from './settingsStore'; import { registerSettingsDraft, @@ -50,6 +64,9 @@ describe('SettingsScene canonical page routing', () => { let root: Root; beforeEach(() => { + preparation.ready.mockReset().mockReturnValue(true); + preparation.preload.mockReset().mockResolvedValue(undefined); + preparation.loading.mockClear(); resetSettingsDraftRegistryForTests(); container = document.createElement('div'); document.body.appendChild(container); @@ -135,4 +152,49 @@ describe('SettingsScene canonical page routing', () => { expect(save).toHaveBeenCalledOnce(); expect(useSettingsStore.getState().activePageId).toBe('application.appearance'); }); + it('never mounts loading UI while switching between prepared pages', async () => { + await act(async () => root.render()); + const frame = container.querySelector('.openbitfun-settings-scene__content-wrapper'); + await act(async () => useSettingsStore.getState().openPage('application.appearance', 'pointer')); + await act(async () => useSettingsStore.getState().openPage('application.general', 'pointer')); + expect(preparation.loading).not.toHaveBeenCalled(); + expect(container.querySelector('.openbitfun-settings-scene__content-wrapper')).toBe(frame); + expect(container.querySelectorAll('[data-testid="settings-scene-content"]')).toHaveLength(1); + }); + + it('keeps the latest destination when cold imports complete out of order', async () => { + let finishAppearance!: () => void; + let finishAutomation!: () => void; + preparation.ready.mockReturnValue(false); + preparation.preload.mockImplementation(pageId => new Promise(resolve => { + if (pageId === 'application.appearance') finishAppearance = resolve; + else finishAutomation = resolve; + })); + useSettingsStore.getState().openPage('application.appearance'); + await act(async () => root.render()); + await act(async () => useSettingsStore.getState().openPage('tools.automation')); + await act(async () => finishAppearance()); + expect(container.querySelector('h2')?.textContent).toBe('tools.automation'); + expect(container.querySelector('[data-testid="appearance-page"]')).toBeNull(); + await act(async () => finishAutomation()); + expect(container.querySelector('[data-testid="automation-page"]')).not.toBeNull(); + }); + + it('shows an in-place retry when preparation fails and recovers without leaving settings', async () => { + preparation.ready.mockReturnValue(false); + preparation.preload.mockRejectedValueOnce(new Error('offline')); + await act(async () => root.render()); + expect(container.querySelector('[data-testid="retry-page"]')).not.toBeNull(); + expect(container.querySelector('[data-testid="general-page"]')).toBeNull(); + await act(async () => (container.querySelector('[data-testid="retry-page"]') as HTMLButtonElement).click()); + expect(container.querySelector('[data-testid="general-page"]')).not.toBeNull(); + }); + + it('replaces page-local state synchronously on device activation', async () => { + await act(async () => root.render()); + const oldPage = container.querySelector('[data-testid="general-page"]'); + await act(async () => activateSurface('peer-settings-test')); + expect(container.querySelector('[data-testid="general-page"]')).not.toBe(oldPage); + }); + }); diff --git a/src/web-ui/src/app/scenes/settings/SettingsScene.tsx b/src/web-ui/src/app/scenes/settings/SettingsScene.tsx index b862b13950..be7b313c1b 100644 --- a/src/web-ui/src/app/scenes/settings/SettingsScene.tsx +++ b/src/web-ui/src/app/scenes/settings/SettingsScene.tsx @@ -1,4 +1,3 @@ -import { NavigationTransitionBoundary } from '@/app/navigation/NavigationTransitionBoundary'; import { cancelPendingSettingsNavigation, discardAndContinueSettingsNavigation, @@ -6,7 +5,11 @@ import { useSettingsDraftSnapshot, } from '@/infrastructure/config/settingsDraftRegistry'; import { ConfirmDialog } from '@openbitfun/ui'; -import React, { Suspense, useEffect, useLayoutEffect, useRef, useState } from 'react'; +import React, { Suspense, useEffect, useRef, useState, useSyncExternalStore } from 'react'; +import { getActiveSurfaceScope, onSurfaceActivated } from '@/infrastructure/peer-device/deviceSurface'; +import { ConfigLoadingState, ConfigRetryState } from '@/infrastructure/config/components/common'; +import { SettingsPage } from './pages/shared/SettingsPage'; +import { useSettingsScrollRestoration } from './useSettingsScrollRestoration'; import { useTranslation } from 'react-i18next'; import { getSettingsPageManifest, @@ -17,20 +20,12 @@ import './SettingsScene.scss'; import { useSettingsStore } from './settingsStore'; import type { SettingsPageId } from './settingsTypes'; -function SettingsSceneLoading() { +function SettingsSceneLoading({ pageId }: { pageId: SettingsPageId }) { + const { t } = useTranslation('common'); return ( -