diff --git a/apps/server/src/routes/settings/routes/update-global.ts b/apps/server/src/routes/settings/routes/update-global.ts index 6072f237..aafbc5b1 100644 --- a/apps/server/src/routes/settings/routes/update-global.ts +++ b/apps/server/src/routes/settings/routes/update-global.ts @@ -11,7 +11,7 @@ import type { Request, Response } from 'express'; import type { SettingsService } from '../../../services/settings-service.js'; import type { GlobalSettings } from '../../../types/settings.js'; -import { getErrorMessage, logError } from '../common.js'; +import { getErrorMessage, logError, logger } from '../common.js'; /** * Create handler factory for PUT /api/settings/global @@ -32,6 +32,18 @@ export function createUpdateGlobalHandler(settingsService: SettingsService) { return; } + // Minimal debug logging to help diagnose accidental wipes. + if ('projects' in updates || 'theme' in updates || 'localStorageMigrated' in updates) { + const projectsLen = Array.isArray((updates as any).projects) + ? (updates as any).projects.length + : undefined; + logger.info( + `Update global settings request: projects=${projectsLen ?? 'n/a'}, theme=${ + (updates as any).theme ?? 'n/a' + }, localStorageMigrated=${(updates as any).localStorageMigrated ?? 'n/a'}` + ); + } + const settings = await settingsService.updateGlobalSettings(updates); res.json({ diff --git a/apps/server/src/services/settings-service.ts b/apps/server/src/services/settings-service.ts index eb7cd0be..15a27b7b 100644 --- a/apps/server/src/services/settings-service.ts +++ b/apps/server/src/services/settings-service.ts @@ -266,25 +266,80 @@ export class SettingsService { const settingsPath = getGlobalSettingsPath(this.dataDir); const current = await this.getGlobalSettings(); + + // Guard against destructive "empty array/object" overwrites. + // During auth transitions, the UI can briefly have default/empty state and accidentally + // sync it, wiping persisted settings (especially `projects`). + const sanitizedUpdates: Partial = { ...updates }; + let attemptedProjectWipe = false; + + const ignoreEmptyArrayOverwrite = (key: K): void => { + const nextVal = sanitizedUpdates[key] as unknown; + const curVal = current[key] as unknown; + if ( + Array.isArray(nextVal) && + nextVal.length === 0 && + Array.isArray(curVal) && + curVal.length > 0 + ) { + delete sanitizedUpdates[key]; + } + }; + + const currentProjectsLen = Array.isArray(current.projects) ? current.projects.length : 0; + if ( + Array.isArray(sanitizedUpdates.projects) && + sanitizedUpdates.projects.length === 0 && + currentProjectsLen > 0 + ) { + attemptedProjectWipe = true; + delete sanitizedUpdates.projects; + } + + ignoreEmptyArrayOverwrite('trashedProjects'); + ignoreEmptyArrayOverwrite('projectHistory'); + ignoreEmptyArrayOverwrite('recentFolders'); + ignoreEmptyArrayOverwrite('aiProfiles'); + ignoreEmptyArrayOverwrite('mcpServers'); + ignoreEmptyArrayOverwrite('enabledCursorModels'); + ignoreEmptyArrayOverwrite('enabledCodexModels'); + + // Empty object overwrite guard + if ( + sanitizedUpdates.lastSelectedSessionByProject && + typeof sanitizedUpdates.lastSelectedSessionByProject === 'object' && + !Array.isArray(sanitizedUpdates.lastSelectedSessionByProject) && + Object.keys(sanitizedUpdates.lastSelectedSessionByProject).length === 0 && + current.lastSelectedSessionByProject && + Object.keys(current.lastSelectedSessionByProject).length > 0 + ) { + delete sanitizedUpdates.lastSelectedSessionByProject; + } + + // If a request attempted to wipe projects, also ignore theme changes in that same request. + if (attemptedProjectWipe) { + delete sanitizedUpdates.theme; + } + const updated: GlobalSettings = { ...current, - ...updates, + ...sanitizedUpdates, version: SETTINGS_VERSION, }; // Deep merge keyboard shortcuts if provided - if (updates.keyboardShortcuts) { + if (sanitizedUpdates.keyboardShortcuts) { updated.keyboardShortcuts = { ...current.keyboardShortcuts, - ...updates.keyboardShortcuts, + ...sanitizedUpdates.keyboardShortcuts, }; } // Deep merge phaseModels if provided - if (updates.phaseModels) { + if (sanitizedUpdates.phaseModels) { updated.phaseModels = { ...current.phaseModels, - ...updates.phaseModels, + ...sanitizedUpdates.phaseModels, }; } diff --git a/apps/server/tests/unit/services/settings-service.test.ts b/apps/server/tests/unit/services/settings-service.test.ts index ff09b817..3a0c6d77 100644 --- a/apps/server/tests/unit/services/settings-service.test.ts +++ b/apps/server/tests/unit/services/settings-service.test.ts @@ -144,6 +144,33 @@ describe('settings-service.ts', () => { expect(updated.keyboardShortcuts.agent).toBe(DEFAULT_GLOBAL_SETTINGS.keyboardShortcuts.agent); }); + it('should not overwrite non-empty projects with an empty array (data loss guard)', async () => { + const initial: GlobalSettings = { + ...DEFAULT_GLOBAL_SETTINGS, + theme: 'solarized' as GlobalSettings['theme'], + projects: [ + { + id: 'proj1', + name: 'Project 1', + path: '/tmp/project-1', + lastOpened: new Date().toISOString(), + }, + ] as any, + }; + const settingsPath = path.join(testDataDir, 'settings.json'); + await fs.writeFile(settingsPath, JSON.stringify(initial, null, 2)); + + const updated = await settingsService.updateGlobalSettings({ + projects: [], + theme: 'light', + } as any); + + expect(updated.projects.length).toBe(1); + expect((updated.projects as any)[0]?.id).toBe('proj1'); + // Theme should be preserved in the same request if it attempted to wipe projects + expect(updated.theme).toBe('solarized'); + }); + it('should create data directory if it does not exist', async () => { const newDataDir = path.join(os.tmpdir(), `new-data-dir-${Date.now()}`); const newService = new SettingsService(newDataDir); diff --git a/apps/ui/src/hooks/use-settings-migration.ts b/apps/ui/src/hooks/use-settings-migration.ts index 75f191f8..5939f645 100644 --- a/apps/ui/src/hooks/use-settings-migration.ts +++ b/apps/ui/src/hooks/use-settings-migration.ts @@ -94,6 +94,17 @@ export function waitForMigrationComplete(): Promise { return migrationCompletePromise; } +/** + * Reset migration state when auth is lost (logout/session expired). + * This ensures that on re-login, the sync hook properly waits for + * fresh settings hydration before starting to sync. + */ +export function resetMigrationState(): void { + migrationCompleted = false; + migrationCompletePromise = null; + migrationCompleteResolve = null; +} + /** * Parse localStorage data into settings object */ diff --git a/apps/ui/src/hooks/use-settings-sync.ts b/apps/ui/src/hooks/use-settings-sync.ts index 0f645703..e7c4c406 100644 --- a/apps/ui/src/hooks/use-settings-sync.ts +++ b/apps/ui/src/hooks/use-settings-sync.ts @@ -18,7 +18,7 @@ import { setItem } from '@/lib/storage'; import { useAppStore, type ThemeMode, THEME_STORAGE_KEY } from '@/store/app-store'; import { useSetupStore } from '@/store/setup-store'; import { useAuthStore } from '@/store/auth-store'; -import { waitForMigrationComplete } from './use-settings-migration'; +import { waitForMigrationComplete, resetMigrationState } from './use-settings-migration'; import type { GlobalSettings } from '@automaker/types'; const logger = createLogger('SettingsSync'); @@ -98,9 +98,35 @@ export function useSettingsSync(): SettingsSyncState { const lastSyncedRef = useRef(''); const isInitializedRef = useRef(false); + // If auth is lost (logout / session expired), immediately stop syncing and + // reset initialization so we can safely re-init after the next login. + useEffect(() => { + if (!authChecked) return; + + if (!isAuthenticated) { + if (syncTimeoutRef.current) { + clearTimeout(syncTimeoutRef.current); + syncTimeoutRef.current = null; + } + lastSyncedRef.current = ''; + isInitializedRef.current = false; + + // Reset migration state so next login properly waits for fresh hydration + resetMigrationState(); + + setState({ loaded: false, error: null, syncing: false }); + } + }, [authChecked, isAuthenticated]); + // Debounced sync function const syncToServer = useCallback(async () => { try { + // Never sync when not authenticated (prevents overwriting server settings during logout/login transitions) + const auth = useAuthStore.getState(); + if (!auth.authChecked || !auth.isAuthenticated) { + return; + } + setState((s) => ({ ...s, syncing: true })); const api = getHttpApiClient(); const appState = useAppStore.getState(); @@ -215,7 +241,7 @@ export function useSettingsSync(): SettingsSyncState { // Subscribe to store changes and sync to server useEffect(() => { - if (!state.loaded) return; + if (!state.loaded || !authChecked || !isAuthenticated) return; // Subscribe to app store changes const unsubscribeApp = useAppStore.subscribe((newState, prevState) => { @@ -272,11 +298,11 @@ export function useSettingsSync(): SettingsSyncState { clearTimeout(syncTimeoutRef.current); } }; - }, [state.loaded, scheduleSyncToServer, syncNow]); + }, [state.loaded, authChecked, isAuthenticated, scheduleSyncToServer, syncNow]); // Best-effort flush on tab close / backgrounding useEffect(() => { - if (!state.loaded) return; + if (!state.loaded || !authChecked || !isAuthenticated) return; const handleBeforeUnload = () => { // Fire-and-forget; may not complete in all browsers, but helps in Electron/webview @@ -296,7 +322,7 @@ export function useSettingsSync(): SettingsSyncState { window.removeEventListener('beforeunload', handleBeforeUnload); document.removeEventListener('visibilitychange', handleVisibilityChange); }; - }, [state.loaded, syncNow]); + }, [state.loaded, authChecked, isAuthenticated, syncNow]); return state; } diff --git a/apps/ui/src/store/app-store.ts b/apps/ui/src/store/app-store.ts index 8bd5063c..250451e9 100644 --- a/apps/ui/src/store/app-store.ts +++ b/apps/ui/src/store/app-store.ts @@ -81,9 +81,23 @@ export const THEME_STORAGE_KEY = 'automaker:theme'; */ export function getStoredTheme(): ThemeMode | null { const stored = getItem(THEME_STORAGE_KEY); - if (stored) { - return stored as ThemeMode; + if (stored) return stored as ThemeMode; + + // Backwards compatibility: older versions stored theme inside the Zustand persist blob. + // We intentionally keep reading it as a fallback so users don't get a "default theme flash" + // on login/logged-out pages if THEME_STORAGE_KEY hasn't been written yet. + try { + const legacy = getItem('automaker-storage'); + if (!legacy) return null; + const parsed = JSON.parse(legacy) as { state?: { theme?: unknown } } | { theme?: unknown }; + const theme = (parsed as any)?.state?.theme ?? (parsed as any)?.theme; + if (typeof theme === 'string' && theme.length > 0) { + return theme as ThemeMode; + } + } catch { + // Ignore legacy parse errors } + return null; } diff --git a/apps/ui/tests/settings/settings-startup-sync-race.spec.ts b/apps/ui/tests/settings/settings-startup-sync-race.spec.ts index b9c51cc6..2cf43d44 100644 --- a/apps/ui/tests/settings/settings-startup-sync-race.spec.ts +++ b/apps/ui/tests/settings/settings-startup-sync-race.spec.ts @@ -104,4 +104,36 @@ test.describe('Settings startup sync race', () => { expect(settingsAfterHydration.projects?.length).toBeGreaterThan(0); expect(settingsAfterHydration.projects?.[0]?.path).toBe(FIXTURE_PROJECT_PATH); }); + + test('does not wipe projects during logout transition', async ({ page }) => { + // Ensure authenticated and app is loaded at least to welcome/board. + await authenticateForTests(page); + await page.goto('/'); + await page + .locator('[data-testid="welcome-view"], [data-testid="board-view"]') + .first() + .waitFor({ state: 'visible', timeout: 30000 }); + + // Confirm settings.json currently has projects (precondition). + const beforeLogout = JSON.parse(fs.readFileSync(SETTINGS_PATH, 'utf-8')) as { + projects?: Array; + }; + expect(beforeLogout.projects?.length).toBeGreaterThan(0); + + // Navigate to settings and click logout. + await page.goto('/settings'); + await page.locator('[data-testid="logout-button"]').click(); + + // Ensure we landed on logged-out or login (either is acceptable). + await page + .locator('text=You’ve been logged out, text=Authentication Required') + .first() + .waitFor({ state: 'visible', timeout: 30000 }); + + // The server settings file should still have projects after logout. + const afterLogout = JSON.parse(fs.readFileSync(SETTINGS_PATH, 'utf-8')) as { + projects?: Array; + }; + expect(afterLogout.projects?.length).toBeGreaterThan(0); + }); });