diff --git a/desktop/electron/services/appMode.test.ts b/desktop/electron/services/appMode.test.ts index 1dde171a..9658da9f 100644 --- a/desktop/electron/services/appMode.test.ts +++ b/desktop/electron/services/appMode.test.ts @@ -117,6 +117,18 @@ describe('Electron app mode service', () => { }) }) + it('does not point existing config at a portable dir that cannot persist app-mode.json', () => { + const fakeApp = app() + const active = tempDir() + const selected = path.join(tempDir(), 'portable') + fs.mkdirSync(path.join(selected, 'app-mode.json'), { recursive: true }) + + expect(() => setAppMode(fakeApp, { mode: 'portable', portableDir: selected }, { CLAUDE_CONFIG_DIR: active })) + .toThrow() + expect(fs.existsSync(path.join(active, 'app-mode.json'))).toBe(false) + expect(fs.existsSync(path.join(fakeApp.getPath('userData'), 'app-mode.json'))).toBe(false) + }) + it('reports whether the default portable dir already has data', () => { const fakeApp = app() expect(detectPortableDir(fakeApp)).toEqual({ diff --git a/desktop/electron/services/appMode.ts b/desktop/electron/services/appMode.ts index fffdb298..e02b97d8 100644 --- a/desktop/electron/services/appMode.ts +++ b/desktop/electron/services/appMode.ts @@ -60,6 +60,20 @@ export function writeAppModeConfig(configDir: string, config: PersistedAppModeCo fs.writeFileSync(path.join(configDir, APP_MODE_FILE), JSON.stringify(config, null, 2)) } +function assertWritableDataDir(configDir: string): void { + try { + fs.mkdirSync(configDir, { recursive: true }) + const probeDir = fs.mkdtempSync(path.join(configDir, '.cc-haha-write-test-')) + try { + fs.writeFileSync(path.join(probeDir, 'probe'), '') + } finally { + fs.rmSync(probeDir, { recursive: true, force: true }) + } + } catch { + throw new Error(`Data storage directory is not writable: ${configDir}`) + } +} + export function determineStartupPortableDir( app: AppModeAppLike, env: NodeJS.ProcessEnv = process.env, @@ -129,7 +143,7 @@ export function setAppMode( if (fs.existsSync(selectedDir) && !fs.statSync(selectedDir).isDirectory()) { throw new Error(`portable config path is not a directory: ${selectedDir}`) } - fs.mkdirSync(selectedDir, { recursive: true }) + assertWritableDataDir(selectedDir) targetPortableDir = selectedDir config = { mode: 'portable', @@ -137,14 +151,15 @@ export function setAppMode( } } - writeAppModeConfig(activeConfigDir, config) - if (targetPortableDir && targetPortableDir !== activeConfigDir) { - writeAppModeConfig(targetPortableDir, config) - } - const systemConfigDir = app.getPath('userData') - if (systemConfigDir !== activeConfigDir) { - writeAppModeConfig(systemConfigDir, config) + const configDirs = [ + targetPortableDir, + activeConfigDir, + systemConfigDir, + ].filter((dir): dir is string => Boolean(dir)) + + for (const configDir of [...new Set(configDirs)]) { + writeAppModeConfig(configDir, config) } } diff --git a/desktop/electron/services/windows.test.ts b/desktop/electron/services/windows.test.ts index ea8fc240..daa43eef 100644 --- a/desktop/electron/services/windows.test.ts +++ b/desktop/electron/services/windows.test.ts @@ -1,4 +1,4 @@ -import { mkdtempSync, readFileSync, rmSync } from 'node:fs' +import { mkdirSync, mkdtempSync, readFileSync, rmSync } from 'node:fs' import path from 'node:path' import { tmpdir } from 'node:os' import { describe, expect, it, vi } from 'vitest' @@ -42,6 +42,25 @@ describe('Electron window service', () => { } }) + it('does not crash when window state cannot be written', () => { + const tmp = mkdtempSync(path.join(tmpdir(), 'electron-window-state-unwritable-')) + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => undefined) + try { + const app = fakeApp(path.join(tmp, 'user-data')) + const state = { x: 10, y: 20, width: 1280, height: 820, maximized: false } + mkdirSync(path.join(tmp, 'window-state.json')) + + expect(() => writeWindowState(app as never, state, { CLAUDE_CONFIG_DIR: tmp })).not.toThrow() + expect(consoleError).toHaveBeenCalledWith( + expect.stringContaining('[desktop] failed to write Electron window state'), + expect.any(Error), + ) + } finally { + consoleError.mockRestore() + rmSync(tmp, { recursive: true, force: true }) + } + }) + it('rejects undersized or off-screen state before restore', () => { expect(isPersistableWindowState({ x: 0, y: 0, width: 100, height: 100, maximized: false })).toBe(false) expect(hasMeaningfulIntersection( diff --git a/desktop/electron/services/windows.ts b/desktop/electron/services/windows.ts index 4f8996fc..431873ec 100644 --- a/desktop/electron/services/windows.ts +++ b/desktop/electron/services/windows.ts @@ -8,6 +8,7 @@ export const DEFAULT_WINDOW_HEIGHT = 820 export const MIN_WINDOW_WIDTH = 960 export const MIN_WINDOW_HEIGHT = 640 const MIN_VISIBLE_PIXELS = 80 +const failedWindowStateWritePaths = new Set() export type StoredWindowState = { x: number @@ -115,8 +116,16 @@ export function writeWindowState( ) { if (!isPersistableWindowState(state)) return const statePath = windowStatePath(app, env) - mkdirSync(path.dirname(statePath), { recursive: true }) - writeFileSync(statePath, `${JSON.stringify(state, null, 2)}\n`) + try { + mkdirSync(path.dirname(statePath), { recursive: true }) + writeFileSync(statePath, `${JSON.stringify(state, null, 2)}\n`) + failedWindowStateWritePaths.delete(statePath) + } catch (error) { + if (!failedWindowStateWritePaths.has(statePath)) { + failedWindowStateWritePaths.add(statePath) + console.error(`[desktop] failed to write Electron window state ${statePath}:`, error) + } + } } export function captureWindowState(window: BrowserWindow): StoredWindowState | null { diff --git a/desktop/src/stores/settingsStore.test.ts b/desktop/src/stores/settingsStore.test.ts index 452e1a52..c0fbdaa2 100644 --- a/desktop/src/stores/settingsStore.test.ts +++ b/desktop/src/stores/settingsStore.test.ts @@ -536,6 +536,30 @@ describe('settingsStore app mode', () => { }) }) + it('rolls back and surfaces app mode persistence failures', async () => { + const error = new Error('Data storage directory is not writable') + const setAppMode = vi.fn().mockRejectedValue(error) + installElectronAppModeHost({ set: setAppMode }) + + const { useSettingsStore } = await import('./settingsStore') + const prevAppMode = { + mode: 'default' as const, + portableDir: null, + defaultPortableDir: 'C:\\cc-haha\\CLAUDE_CONFIG_DIR', + activeConfigDir: null, + configDirSource: 'system' as const, + } + useSettingsStore.setState({ + appMode: prevAppMode, + appModeRequiresRestart: false, + }) + + await expect(useSettingsStore.getState().setAppMode('portable', 'D:\\blocked-data')) + .rejects.toThrow('Data storage directory is not writable') + expect(useSettingsStore.getState().appMode).toEqual(prevAppMode) + expect(useSettingsStore.getState().appModeRequiresRestart).toBe(false) + }) + it('switches app mode back to the system data source', async () => { const setAppMode = vi.fn().mockResolvedValue(undefined) installElectronAppModeHost({ set: setAppMode }) diff --git a/desktop/src/stores/settingsStore.ts b/desktop/src/stores/settingsStore.ts index aa40736a..634f20f0 100644 --- a/desktop/src/stores/settingsStore.ts +++ b/desktop/src/stores/settingsStore.ts @@ -593,8 +593,9 @@ export const useSettingsStore = create((set, get) => ({ mode, portableDir: newMode.portableDir || null, }) - } catch { + } catch (error) { set({ appMode: prev, appModeRequiresRestart: false }) + throw error } }, }))