diff --git a/packages/ui/src/components/auth/SessionAuthGate.tsx b/packages/ui/src/components/auth/SessionAuthGate.tsx index 702d879b..dee8f225 100644 --- a/packages/ui/src/components/auth/SessionAuthGate.tsx +++ b/packages/ui/src/components/auth/SessionAuthGate.tsx @@ -602,7 +602,7 @@ export const SessionAuthGate: React.FC = ({ hasBootstrapResyncedRef.current = true; void (async () => { await initializeAppearancePreferences(); - await syncDesktopSettings({ adoptWorkspace: isBootstrapResync }); + await syncDesktopSettings({ bootstrap: isBootstrapResync }); if (isBootstrapResync) { await applyPersistedDirectoryPreferences(); } diff --git a/packages/ui/src/contexts/ThemeSystemContext.tsx b/packages/ui/src/contexts/ThemeSystemContext.tsx index edc7929b..c417fc24 100644 --- a/packages/ui/src/contexts/ThemeSystemContext.tsx +++ b/packages/ui/src/contexts/ThemeSystemContext.tsx @@ -34,6 +34,7 @@ import { getRuntimeKey, subscribeRuntimeEndpointChanged } from '@/lib/runtime-sw import { adoptThemePreferencesForRuntime, resolveThemePreferencesForRuntime, + resolveThemePreferencesFromSettingsSync, resolveThemePreferencesFromStorageEvent, writeThemePreferencesForRuntime, } from './theme-storage'; @@ -590,46 +591,14 @@ export function ThemeSystemProvider({ children, defaultThemeId }: ThemeSystemPro return; } const handleSettingsSynced = (event: Event) => { - const detail = (event as CustomEvent).detail?.settings; + const detail = (event as CustomEvent).detail?.settings + ? (event as CustomEvent).detail + : null; if (!detail) { return; } - setPreferences((prev) => { - let nextMode = prev.themeMode; - if (detail.useSystemTheme === true) { - nextMode = 'system'; - } else if (detail.useSystemTheme === false) { - if (detail.themeVariant === 'dark' || detail.themeVariant === 'light') { - nextMode = detail.themeVariant; - } - } - - let nextLight = prev.lightThemeId; - if (typeof detail.lightThemeId === 'string' && detail.lightThemeId.length > 0) { - nextLight = detail.lightThemeId.trim(); - } - - let nextDark = prev.darkThemeId; - if (typeof detail.darkThemeId === 'string' && detail.darkThemeId.length > 0) { - nextDark = detail.darkThemeId.trim(); - } - - const same = - nextMode === prev.themeMode && - nextLight === prev.lightThemeId && - nextDark === prev.darkThemeId; - - if (same) { - return prev; - } - - return { - themeMode: nextMode, - lightThemeId: nextLight, - darkThemeId: nextDark, - }; - }); + setPreferences((prev) => resolveThemePreferencesFromSettingsSync(detail, prev) ?? prev); }; window.addEventListener('openchamber:settings-synced', handleSettingsSynced); diff --git a/packages/ui/src/contexts/theme-storage.test.ts b/packages/ui/src/contexts/theme-storage.test.ts index ecba5ad0..1128b44c 100644 --- a/packages/ui/src/contexts/theme-storage.test.ts +++ b/packages/ui/src/contexts/theme-storage.test.ts @@ -7,6 +7,7 @@ import { getThemePreferencesStorageKey, readThemePreferencesForRuntime, resolveThemePreferencesForRuntime, + resolveThemePreferencesFromSettingsSync, resolveThemePreferencesFromStorageEvent, writeThemePreferencesForRuntime, } from './theme-storage'; @@ -288,3 +289,40 @@ describe('theme storage event resolution', () => { expect(resolveThemePreferencesFromStorageEvent(getThemePreferencesStorageKey('runtime-a'), 'runtime-a', current)).toBeNull(); }); }); + +describe('settings sync resolution', () => { + const current = { themeMode: 'system' as const, lightThemeId: 'light-theme', darkThemeId: 'dark-theme' }; + const serverTheme = { useSystemTheme: false as const, themeVariant: 'dark' as const, lightThemeId: 'server-light', darkThemeId: 'server-dark' }; + + test('a non-bootstrap sync (settings save echo) never changes preferences', () => { + expect(resolveThemePreferencesFromSettingsSync({ adoptTheme: false, settings: serverTheme }, current)).toBeNull(); + expect(resolveThemePreferencesFromSettingsSync(null, current)).toBeNull(); + }); + + test('a bootstrap sync adopts the server theme', () => { + expect(resolveThemePreferencesFromSettingsSync({ adoptTheme: true, settings: serverTheme }, current)).toEqual({ + themeMode: 'dark', + lightThemeId: 'server-light', + darkThemeId: 'server-dark', + }); + }); + + test('theme fields omitted by the server keep the current preferences (not-set is not reset-to-defaults)', () => { + expect(resolveThemePreferencesFromSettingsSync({ adoptTheme: true, settings: {} }, current)).toBeNull(); + expect( + resolveThemePreferencesFromSettingsSync({ adoptTheme: true, settings: { useSystemTheme: true } }, current), + ).toBeNull(); + expect( + resolveThemePreferencesFromSettingsSync({ adoptTheme: true, settings: { lightThemeId: 'server-light' } }, current), + ).toEqual({ themeMode: 'system', lightThemeId: 'server-light', darkThemeId: 'dark-theme' }); + }); + + test('a bootstrap sync carrying the current preferences resolves to no change', () => { + expect( + resolveThemePreferencesFromSettingsSync( + { adoptTheme: true, settings: { useSystemTheme: true, lightThemeId: 'light-theme', darkThemeId: 'dark-theme' } }, + current, + ), + ).toBeNull(); + }); +}); diff --git a/packages/ui/src/contexts/theme-storage.ts b/packages/ui/src/contexts/theme-storage.ts index 863b9efc..a7c9c1ce 100644 --- a/packages/ui/src/contexts/theme-storage.ts +++ b/packages/ui/src/contexts/theme-storage.ts @@ -1,4 +1,5 @@ import type { ThemeMode } from '@/types/theme'; +import type { DesktopSettings } from '@/lib/desktop'; import { DEFAULT_DARK_THEME_ID, DEFAULT_LIGHT_THEME_ID, getThemeById } from '@/lib/theme/themes'; import { isTransientRuntimeKey } from '@/lib/runtime-switch'; @@ -179,3 +180,48 @@ export const adoptThemePreferencesForRuntime = ( runtimeKey: string, current: StoredThemePreferences, ): StoredThemePreferences => readThemePreferencesForRuntime(runtimeKey) ?? current; + +type SettingsSyncThemePayload = Pick< + DesktopSettings, + 'useSystemTheme' | 'themeVariant' | 'lightThemeId' | 'darkThemeId' +>; + +/** + * Resolve the preferences a settings sync should apply. Returns null — meaning + * "keep current preferences" — when the sync is not bootstrap-grade (a + * mid-session save echo replays the server document, which may hold another + * window's theme, a default, or a stale value; the scoped entry and bootstrap + * syncs own this window's theme) and when the resolved values already match + * the current ones (the identity check breaks adoption loops). + * + * Theme fields absent from the server document mean "not set", not "reset to + * defaults": each one independently keeps the current preference. The payload + * is the already-sanitized settings document (sanitizeWebSettings), so fields + * present in it are domain-valid; an unknown id injected some other way fails + * theme lookup downstream and falls back cosmetically. + */ +export const resolveThemePreferencesFromSettingsSync = ( + detail: { adoptTheme: boolean; settings: SettingsSyncThemePayload } | null, + current: StoredThemePreferences, +): StoredThemePreferences | null => { + if (!detail?.adoptTheme) { + return null; + } + + const settings = detail.settings; + let themeMode = current.themeMode; + if (settings.useSystemTheme === true) { + themeMode = 'system'; + } else if (settings.useSystemTheme === false && (settings.themeVariant === 'dark' || settings.themeVariant === 'light')) { + themeMode = settings.themeVariant; + } + + const lightThemeId = settings.lightThemeId?.trim() || current.lightThemeId; + const darkThemeId = settings.darkThemeId?.trim() || current.darkThemeId; + + if (themeMode === current.themeMode && lightThemeId === current.lightThemeId && darkThemeId === current.darkThemeId) { + return null; + } + + return { themeMode, lightThemeId, darkThemeId }; +}; diff --git a/packages/ui/src/lib/persistence.test.ts b/packages/ui/src/lib/persistence.test.ts index c2dce48c..0fa1adb0 100644 --- a/packages/ui/src/lib/persistence.test.ts +++ b/packages/ui/src/lib/persistence.test.ts @@ -15,6 +15,7 @@ import { subscribeToSettingsSaveState, syncDesktopSettings, updateDesktopSettings, + type SettingsSyncedDetail, } from './persistence'; import { switchRuntimeEndpoint } from './runtime-switch'; @@ -843,6 +844,100 @@ describe('updateDesktopSettings', () => { expect(useUIStore.getState().autoSaveEnabled).toBe(true); expect(saveCalls.some((changes) => changes.autoSaveEnabled === true)).toBe(true); }); + + test('does not invent theme defaults when the authoritative snapshot omits theme fields', async () => { + getWindow(); + invalidateSettingsCache(); + registerSettingsApi( + // SAFETY: this mock echoes back exactly the partial changes it received; + // the tests below only read fields the changes actually contain. + async (changes) => ({ ...changes } as SettingsPayload), + async () => ({ + settings: { draftStartersCraftGoalAdded: true, draftStartersScheduleTaskAdded: true }, + source: 'web', + }), + ); + + const synced: SettingsSyncedDetail[] = []; + const listener = (event: Event): void => { + // SAFETY: dispatchSettingsSynced is the only emitter for this key and + // always sends a CustomEvent. + const detail = (event as CustomEvent).detail; + if (detail) synced.push(detail); + }; + window.addEventListener('openchamber:settings-synced', listener); + try { + await syncDesktopSettings(); + } finally { + window.removeEventListener('openchamber:settings-synced', listener); + } + + expect(synced.length).toBeGreaterThan(0); + const bootstrapSync = synced.find((detail) => detail.bootstrap); + expect(bootstrapSync).toBeTruthy(); + expect(bootstrapSync?.adoptTheme).toBe(true); + expect(bootstrapSync?.settings.useSystemTheme).toBe(undefined); + expect(bootstrapSync?.settings.lightThemeId).toBe(undefined); + expect(bootstrapSync?.settings.darkThemeId).toBe(undefined); + }); + + test('marks settings save echoes as non-bootstrap syncs', async () => { + getWindow(); + invalidateSettingsCache(); + registerSettingsApi( + // SAFETY: this mock echoes back exactly the partial changes it received; + // the assertions below only read fields the changes actually contain. + async (changes) => ({ ...changes } as SettingsPayload), + ); + + const synced: SettingsSyncedDetail[] = []; + const listener = (event: Event): void => { + // SAFETY: dispatchSettingsSynced is the only emitter for this key and + // always sends a CustomEvent. + const detail = (event as CustomEvent).detail; + if (detail) synced.push(detail); + }; + window.addEventListener('openchamber:settings-synced', listener); + try { + await updateDesktopSettings({ themeVariant: 'dark' }); + } finally { + window.removeEventListener('openchamber:settings-synced', listener); + } + + expect(synced.length).toBeGreaterThan(0); + expect(synced.every((detail) => detail.bootstrap === false)).toBe(true); + expect(synced.every((detail) => detail.adoptTheme === false)).toBe(true); + expect(synced.every((detail) => detail.settings.themeVariant === 'dark')).toBe(true); + }); + + test('allows a bootstrap sync to preserve the current window theme', async () => { + getWindow(); + invalidateSettingsCache(); + registerSettingsApi( + async (changes) => ({ ...changes } as SettingsPayload), + async () => ({ + settings: { activeProjectId: 'project-a', themeVariant: 'dark' }, + source: 'web', + }), + ); + + const synced: SettingsSyncedDetail[] = []; + const listener = (event: Event): void => { + const detail = (event as CustomEvent).detail; + if (detail) synced.push(detail); + }; + window.addEventListener('openchamber:settings-synced', listener); + try { + await syncDesktopSettings({ adoptTheme: false }); + } finally { + window.removeEventListener('openchamber:settings-synced', listener); + } + + const broadcastSync = synced.find((detail) => detail.bootstrap && !detail.adoptTheme); + expect(broadcastSync).toBeTruthy(); + expect(broadcastSync?.settings.activeProjectId).toBe('project-a'); + expect(broadcastSync?.settings.themeVariant).toBe('dark'); + }); }); describe('unload lifecycle flush (#2197)', () => { diff --git a/packages/ui/src/lib/persistence.ts b/packages/ui/src/lib/persistence.ts index 91870b90..c5750464 100644 --- a/packages/ui/src/lib/persistence.ts +++ b/packages/ui/src/lib/persistence.ts @@ -20,7 +20,6 @@ import { runtimeFetch } from '@/lib/runtime-fetch'; import { isCapacitorApp } from '@/lib/platform'; import { isTerminalShell } from '@/lib/terminalShell'; import { getRuntimeKey, subscribeRuntimeEndpointChanged, subscribeRuntimeEndpointWillChange } from '@/lib/runtime-switch'; -import { DEFAULT_DARK_THEME_ID, DEFAULT_LIGHT_THEME_ID } from '@/lib/theme/themes'; import { DEFAULT_OPEN_IN_APP_ID } from '@/lib/openInApps'; import { useSessionDisplayStore } from '@/stores/useSessionDisplayStore'; @@ -197,20 +196,27 @@ const persistToLocalStorage = (settings: DesktopSettings) => { export interface SettingsSyncedDetail { settings: DesktopSettings; - /** Whether listeners may adopt cross-window workspace pointers - (activeProjectId / lastDirectory). True only for a bootstrap-grade sync: - the settings document is shared by every window of this server, so a - mid-session reconciliation adopting them would hijack this window's - workspace with another window's choice. */ - adoptWorkspace: boolean; + /** Whether listeners may adopt authoritative state that this window owns a + live copy of (workspace pointers, theme). True only for a bootstrap-grade + sync: the settings document is shared by every window of this server, so + a mid-session reconciliation adopting them would hijack this window's + choices with another window's. Every settings save echoes the full + document back as a sync event with bootstrap=false — the echo itself is + not filtered; listeners gate their adoption on this flag and keep their + live state for the fields they own. */ + bootstrap: boolean; + /** Whether this sync may replace this window's theme preferences. VS Code + settings broadcasts remain bootstrap-grade for shared workspace pointers, + but must not copy one webview's theme into another webview. */ + adoptTheme: boolean; } -const dispatchSettingsSynced = (settings: DesktopSettings, adoptWorkspace: boolean): void => { +const dispatchSettingsSynced = (settings: DesktopSettings, bootstrap: boolean, adoptTheme = bootstrap): void => { if (typeof window === 'undefined') { return; } window.dispatchEvent(new CustomEvent('openchamber:settings-synced', { - detail: { settings, adoptWorkspace }, + detail: { settings, bootstrap, adoptTheme }, })); }; @@ -534,9 +540,11 @@ const materializeAuthoritativeUiSettings = (settings: DesktopSettings): DesktopS const defaults = useUIStore.getInitialState(); return { - useSystemTheme: true, - lightThemeId: DEFAULT_LIGHT_THEME_ID, - darkThemeId: DEFAULT_DARK_THEME_ID, + // Theme fields are deliberately NOT defaulted: the theme authority is the + // ThemeSystemContext (scoped per-runtime entry + bootstrap syncs). A + // server document without theme fields means "not set" — inventing + // defaults here would clobber the window's theme and write it back to the + // server. Absent fields keep the current preferences. openInAppId: DEFAULT_OPEN_IN_APP_ID, showReasoningTraces: defaults.showReasoningTraces, streamingAutoFollowEnabled: defaults.streamingAutoFollowEnabled, @@ -1896,8 +1904,9 @@ export const invalidateSettingsCache = (): void => { _settingsCache = null; }; -export const syncDesktopSettings = async (options?: { adoptWorkspace?: boolean }): Promise => { - const adoptWorkspace = options?.adoptWorkspace !== false; +export const syncDesktopSettings = async (options?: { bootstrap?: boolean; adoptTheme?: boolean }): Promise => { + const bootstrap = options?.bootstrap !== false; + const adoptTheme = options?.adoptTheme ?? bootstrap; if (typeof window === 'undefined') { return; } @@ -2026,7 +2035,7 @@ export const syncDesktopSettings = async (options?: { adoptWorkspace?: boolean } if (!isSettingsRuntimeContextCurrent(context)) return; } - dispatchSettingsSynced(authoritativeSettings, adoptWorkspace); + dispatchSettingsSynced(authoritativeSettings, bootstrap, adoptTheme); }; try { diff --git a/packages/ui/src/stores/DOCUMENTATION.md b/packages/ui/src/stores/DOCUMENTATION.md index 355db701..cf3964b0 100644 --- a/packages/ui/src/stores/DOCUMENTATION.md +++ b/packages/ui/src/stores/DOCUMENTATION.md @@ -84,7 +84,7 @@ Permission auto-accept policy is authoritative in the active Web server or VS Co Shared safe storage treats durable failures per key. A quota or access failure creates an ephemeral override or tombstone for that key without disabling reads and writes for unrelated keys; later writes retry the durable backend. Deferred adapters retain failed operations for a later flush, and malformed Zustand JSON is removed and treated as missing so hydration can recover. -Project and UI settings use successful settings synchronization as authority. Omitted fields in a complete snapshot reset to canonical client defaults, including an omitted project list becoming empty; transport or settings-load failure dispatches no synchronization event and preserves current state. Settings save responses are partial patches and must not clear unrelated in-memory preferences or local mirrors. Debounced settings writes flush best-effort on page hide, document hidden, app freeze, and unload — canceling the pending timer so the write happens exactly once — because a write lost inside the debounce window lets the stale server snapshot override the change on next startup; a hard process kill can still lose the in-flight request. The unload flush uses `keepalive: true` on the HTTP write, because a plain fetch started from `pagehide`/`beforeunload` is cancelled with the document; `navigator.sendBeacon` is not used, as it cannot carry the runtime bearer header. On Capacitor neither `pagehide` nor `beforeunload` fires when the OS suspends the app, so the flush also runs on `App.appStateChange` going inactive. +Project and UI settings use successful settings synchronization as authority. Omitted fields in a complete snapshot reset to canonical client defaults, including an omitted project list becoming empty. Theme fields are the exception: only bootstrap-grade theme adoption applies fields supplied by the server, while omitted fields preserve this window's current runtime-scoped theme and settings save echoes never adopt a theme. VS Code settings broadcasts may still adopt shared workspace pointers without replacing each webview's editor-derived theme. Transport or settings-load failure dispatches no synchronization event and preserves current state. Settings save responses are partial patches and must not clear unrelated in-memory preferences or local mirrors. Debounced settings writes flush best-effort on page hide, document hidden, app freeze, and unload — canceling the pending timer so the write happens exactly once — because a write lost inside the debounce window lets the stale server snapshot override the change on next startup; a hard process kill can still lose the in-flight request. The unload flush uses `keepalive: true` on the HTTP write, because a plain fetch started from `pagehide`/`beforeunload` is cancelled with the document; `navigator.sendBeacon` is not used, as it cannot carry the runtime bearer header. On Capacitor neither `pagehide` nor `beforeunload` fires when the OS suspends the app, so the flush also runs on `App.appStateChange` going inactive. Project ordering defaults to manual. Session display persistence v3 migrates the previously shipped `recent` project order to `manual` while preserving every other explicit sort mode. diff --git a/packages/ui/src/stores/useProjectsStore.ts b/packages/ui/src/stores/useProjectsStore.ts index 4e89833f..4a8c3495 100644 --- a/packages/ui/src/stores/useProjectsStore.ts +++ b/packages/ui/src/stores/useProjectsStore.ts @@ -1112,7 +1112,7 @@ if (typeof window !== 'undefined') { const detail = (event as CustomEvent).detail; if (detail && typeof detail === 'object' && detail.settings) { useProjectsStore.getState().synchronizeFromSettings(detail.settings, { - adoptActiveProject: detail.adoptWorkspace, + adoptActiveProject: detail.bootstrap, }); } }); diff --git a/packages/vscode/webview/main.tsx b/packages/vscode/webview/main.tsx index a49ba39f..03e8b1d1 100644 --- a/packages/vscode/webview/main.tsx +++ b/packages/vscode/webview/main.tsx @@ -1859,7 +1859,7 @@ window.addEventListener('openchamber:vscode-notification-event', (event) => { // Listen for settings sync command from extension (broadcast to all VS Code webviews) onCommand('settingsSynced', () => { import('@openchamber/ui/lib/persistence').then(({ syncDesktopSettings }) => { - void syncDesktopSettings(); + void syncDesktopSettings({ adoptTheme: false }); }); });