From 91f9744d3a1286c3edf25111f6e0c515fa0ffae0 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 10 Oct 2026 17:36:49 -0700 Subject: [PATCH] fix(settings): return current preferences after slow updates (#27199) * fix(settings): return current preferences after reconciliation * test(settings): run SQLite reply regressions in Node --- config/scripts/vitest-node-runtime-files.mjs | 1 + .../ipc/settings-delayed-response.test.ts | 157 ++++++++++++++++++ src/main/ipc/settings.ts | 2 +- 3 files changed, 159 insertions(+), 1 deletion(-) create mode 100644 src/main/ipc/settings-delayed-response.test.ts diff --git a/config/scripts/vitest-node-runtime-files.mjs b/config/scripts/vitest-node-runtime-files.mjs index 36763a4a3a9..fb184c456bb 100644 --- a/config/scripts/vitest-node-runtime-files.mjs +++ b/config/scripts/vitest-node-runtime-files.mjs @@ -86,6 +86,7 @@ export const NODE_RUNTIME_INCLUDE = [ 'src/relay/relay-primary-channel.test.ts', 'src/renderer/src/lib/monaco-languages/monarch-upstream-mdx-recursion.test.ts', 'src/main/ipc/orca-profiles-switch-persistence.test.ts', + 'src/main/ipc/settings-delayed-response.test.ts', 'src/main/runtime/structured-agent-session-startup-tab-restore.test.ts', 'src/shared/remote-runtime-shared-control-connection.test.ts', 'src/main/runtime/relay/relay-control-client.test.ts', diff --git a/src/main/ipc/settings-delayed-response.test.ts b/src/main/ipc/settings-delayed-response.test.ts new file mode 100644 index 00000000000..f1d79107913 --- /dev/null +++ b/src/main/ipc/settings-delayed-response.test.ts @@ -0,0 +1,157 @@ +import { beforeEach, expect, it, vi } from 'vitest' +import { + createWorkerMaintenanceFixture, + maintenanceBarrier +} from '../persistence/loading-store/profile-state-maintenance-fixture' + +const { + handleMock, + setMainUiLanguageMock, + applyElectronProxySettingsMock, + applyAgentStatusHooksEnabledMock +} = vi.hoisted(() => ({ + handleMock: vi.fn(), + setMainUiLanguageMock: vi.fn(), + applyElectronProxySettingsMock: vi.fn(), + applyAgentStatusHooksEnabledMock: vi.fn() +})) + +vi.mock('electron', () => ({ + app: { getPath: vi.fn(() => '/test/user-data') }, + BrowserWindow: { getAllWindows: vi.fn(() => []) }, + ipcMain: { handle: handleMock, on: vi.fn() }, + nativeTheme: { themeSource: 'system' } +})) +vi.mock('../telemetry/client', () => ({ track: vi.fn() })) +vi.mock('../telemetry/cohort-classifier', () => ({ + getCohortAtEmit: () => ({ nth_repo_added: 2 }) +})) +vi.mock('../ssh/ssh-config-parser', () => ({ + loadUserSshConfig: () => ({ hosts: [] }), + sshConfigHostsToTargets: () => [] +})) +vi.mock('../ghostty/index', () => ({ previewGhosttyImport: vi.fn() })) +vi.mock('../warp-themes', () => ({ previewWarpThemeImport: vi.fn() })) +vi.mock('../network/proxy-settings', () => ({ + applyElectronProxySettings: applyElectronProxySettingsMock +})) +vi.mock('../browser/browser-session-proxy', () => ({ + applyBrowserSessionProxies: vi.fn().mockResolvedValue(undefined) +})) +vi.mock('../browser/browser-session-registry', () => ({ + browserSessionRegistry: { listProfiles: vi.fn(() => []) } +})) +vi.mock('../app-icon', () => ({ applyAppIcon: vi.fn() })) +vi.mock('../ai-vault-search/session-search-enablement', () => ({ + applySessionSearchSettingsChange: vi.fn() +})) +vi.mock('../agent-hooks/managed-agent-hook-controls', () => ({ + applyAgentStatusHooksEnabled: applyAgentStatusHooksEnabledMock +})) +vi.mock('../worktree-root-preparation', () => ({ + prepareLocalWorktreeRootsForRepos: vi.fn().mockResolvedValue(undefined) +})) +vi.mock('../menu/register-app-menu', () => ({ rebuildAppMenu: vi.fn() })) +vi.mock('../i18n/main-i18n', () => ({ setMainUiLanguage: setMainUiLanguageMock })) + +import { registerSettingsHandlers } from './settings' + +beforeEach(() => { + handleMock.mockClear() + setMainUiLanguageMock.mockReset().mockResolvedValue('en') + applyElectronProxySettingsMock.mockReset().mockResolvedValue({ source: 'settings' }) + applyAgentStatusHooksEnabledMock.mockReset().mockResolvedValue([]) +}) + +const event = { sender: { id: 1 } } + +function settingsWriter() { + const handler = handleMock.mock.calls.find(([channel]) => channel === 'settings:set')?.[1] + if (typeof handler !== 'function') { + throw new Error('Missing settings:set handler') + } + return handler +} + +it('returns current settings when a language change finishes after a newer theme write', async () => { + const { store, readState } = await createWorkerMaintenanceFixture() + store.updateSettings({ theme: 'system', uiLanguage: 'en' }) + await store.flushPendingOrThrowAsync() + registerSettingsHandlers(store) + const write = settingsWriter() + const started = maintenanceBarrier() + const release = maintenanceBarrier() + setMainUiLanguageMock.mockImplementationOnce(async () => { + started.resolve() + await release.promise + return 'ko' + }) + + const first = write(event, { theme: 'dark', uiLanguage: 'ko' }) + await started.promise + await write(event, { theme: 'light' }) + release.resolve() + const reply = await first + await store.flushPendingOrThrowAsync() + + expect(store.getSettings().theme).toBe('light') + expect(readState().settings.theme).toBe('light') + expect(reply.theme).toBe('light') +}) + +it('returns current settings when hook reconciliation finishes after a newer theme write', async () => { + const { store, readState } = await createWorkerMaintenanceFixture() + store.updateSettings({ theme: 'system', agentStatusHooksEnabled: false }) + await store.flushPendingOrThrowAsync() + registerSettingsHandlers(store) + const write = settingsWriter() + const started = maintenanceBarrier() + const release = maintenanceBarrier() + applyAgentStatusHooksEnabledMock.mockImplementationOnce(async () => { + started.resolve() + await release.promise + return [] + }) + + const first = write(event, { theme: 'dark', agentStatusHooksEnabled: true }) + await started.promise + await write(event, { theme: 'light' }) + release.resolve() + const reply = await first + await store.flushPendingOrThrowAsync() + + expect(store.getSettings().theme).toBe('light') + expect(readState().settings.theme).toBe('light') + expect(reply.theme).toBe('light') + expect(applyAgentStatusHooksEnabledMock).toHaveBeenCalledWith( + true, + expect.objectContaining({ theme: 'dark' }), + expect.any(Object) + ) +}) + +it('returns current settings when proxy reconciliation finishes after a newer theme write', async () => { + const { store, readState } = await createWorkerMaintenanceFixture() + store.updateSettings({ theme: 'system', httpProxyUrl: '' }) + await store.flushPendingOrThrowAsync() + registerSettingsHandlers(store) + const write = settingsWriter() + const started = maintenanceBarrier() + const release = maintenanceBarrier() + applyElectronProxySettingsMock.mockImplementationOnce(async () => { + started.resolve() + await release.promise + return { source: 'settings' } + }) + + const first = write(event, { theme: 'dark', httpProxyUrl: 'http://proxy.example:8080' }) + await started.promise + await write(event, { theme: 'light' }) + release.resolve() + const reply = await first + await store.flushPendingOrThrowAsync() + + expect(store.getSettings().theme).toBe('light') + expect(readState().settings.theme).toBe('light') + expect(reply.theme).toBe('light') +}) diff --git a/src/main/ipc/settings.ts b/src/main/ipc/settings.ts index 1200a1803d7..cd9fd873e0a 100644 --- a/src/main/ipc/settings.ts +++ b/src/main/ipc/settings.ts @@ -302,7 +302,7 @@ export function registerSettingsHandlers( }) } - return result + return store.getSettings() }) ipcMain.handle(