From 39399622e5f40a95e8f3ced6f524bf46b0a4c1ad Mon Sep 17 00:00:00 2001 From: zipg Date: Mon, 10 Aug 2026 14:38:56 +0800 Subject: [PATCH] fix(settings): prevent startup from overwriting persisted settings --- .../__tests__/connectionStore.timeout.spec.ts | 68 ++++++++++++++ .../stores/__tests__/settingsStore.spec.ts | 69 +++++++++++++- apps/desktop/src/stores/connectionStore.ts | 3 + apps/desktop/src/stores/settingsStore.ts | 90 ++++++++++++------- 4 files changed, 196 insertions(+), 34 deletions(-) diff --git a/apps/desktop/src/stores/__tests__/connectionStore.timeout.spec.ts b/apps/desktop/src/stores/__tests__/connectionStore.timeout.spec.ts index 1912f73541..d44b990663 100644 --- a/apps/desktop/src/stores/__tests__/connectionStore.timeout.spec.ts +++ b/apps/desktop/src/stores/__tests__/connectionStore.timeout.spec.ts @@ -82,11 +82,75 @@ describe("connectionStore timeout recovery", () => { expect(store.connections[0]?.keepalive_interval_secs).toBe(30); }); + it("loads persisted editor settings before startup timeout migration", async () => { + const callOrder: string[] = []; + const loadEditorSettings = vi.fn(async () => { + callOrder.push("settings"); + return { + appLayout: "separated", + uiFontFamily: "persisted-font", + snippets: [{ id: "persisted", label: "Persisted", prefix: "persisted", body: "SELECT 42", enabled: true }], + globalConnectTimeoutSecs: 7, + connectTimeoutInheritConnectionIds: ["persisted"], + globalQueryTimeoutSecs: 12, + queryTimeoutInheritConnectionIds: ["persisted"], + timeoutInheritanceMigrationVersion: 2, + executeModeDefaultVersion: 1, + }; + }); + const loadConnections = vi.fn(async () => { + callOrder.push("connections"); + return [postgresConnection({ id: "persisted", connect_timeout_secs: 7, query_timeout_secs: 12 })]; + }); + const saveEditorSettings = vi.fn().mockResolvedValue(undefined); + vi.doMock("@/lib/backend/tauriRuntime", () => ({ isTauriRuntime: () => false })); + vi.doMock("@/lib/backend/api", () => ({ + deleteSchemaCachePrefix: vi.fn().mockResolvedValue(undefined), + loadEditorSettings, + loadConnections, + loadPinnedTreeNodeIds: vi.fn().mockResolvedValue([]), + loadSidebarLayout: vi.fn().mockResolvedValue(null), + loadTunnelProfiles: vi.fn().mockResolvedValue([]), + saveConnections: vi.fn().mockResolvedValue(undefined), + saveEditorSettings, + saveSidebarLayout: vi.fn().mockResolvedValue(undefined), + })); + + const { useConnectionStore } = await import("@/stores/connectionStore"); + await useConnectionStore().initFromDisk(); + + expect(callOrder).toEqual(["settings", "connections"]); + expect(saveEditorSettings).toHaveBeenCalledWith( + expect.objectContaining({ + appLayout: "separated", + uiFontFamily: "persisted-font", + snippets: [expect.objectContaining({ id: "persisted", body: "SELECT 42" })], + }), + ); + }); + + it("does not load connections when persisted editor settings cannot be read", async () => { + const loadConnections = vi.fn().mockResolvedValue([postgresConnection()]); + const saveEditorSettings = vi.fn().mockResolvedValue(undefined); + vi.doMock("@/lib/backend/tauriRuntime", () => ({ isTauriRuntime: () => false })); + vi.doMock("@/lib/backend/api", () => ({ + loadEditorSettings: vi.fn().mockRejectedValue(new Error("settings unavailable")), + loadConnections, + saveEditorSettings, + })); + + const { useConnectionStore } = await import("@/stores/connectionStore"); + await expect(useConnectionStore().initFromDisk()).rejects.toThrow("settings unavailable"); + expect(loadConnections).not.toHaveBeenCalled(); + expect(saveEditorSettings).not.toHaveBeenCalled(); + }); + it("migrates legacy timeout defaults to global inheritance and preserves custom overrides", async () => { const saveConnections = vi.fn().mockResolvedValue(undefined); vi.doMock("@/lib/backend/tauriRuntime", () => ({ isTauriRuntime: () => false })); vi.doMock("@/lib/backend/api", () => ({ deleteSchemaCachePrefix: vi.fn().mockResolvedValue(undefined), + loadEditorSettings: vi.fn().mockResolvedValue(null), loadConnections: vi .fn() .mockResolvedValue([postgresConnection({ id: "default", connect_timeout_secs: 10, query_timeout_secs: 30 }), postgresConnection({ id: "custom", connect_timeout_secs: 45, query_timeout_secs: 300 }), postgresConnection({ id: "inherited", connect_timeout_secs: 60, query_timeout_secs: 60 })]), @@ -130,6 +194,7 @@ describe("connectionStore timeout recovery", () => { vi.doMock("@/lib/backend/tauriRuntime", () => ({ isTauriRuntime: () => false })); vi.doMock("@/lib/backend/api", () => ({ deleteSchemaCachePrefix: vi.fn().mockResolvedValue(undefined), + loadEditorSettings: vi.fn().mockResolvedValue(null), loadConnections: vi.fn().mockResolvedValue([postgresConnection({ id: "local", connect_timeout_secs: 10, query_timeout_secs: 30 })]), loadPinnedTreeNodeIds: vi.fn().mockResolvedValue([]), loadSidebarLayout: vi.fn().mockResolvedValue(null), @@ -164,6 +229,7 @@ describe("connectionStore timeout recovery", () => { vi.doMock("@/lib/backend/tauriRuntime", () => ({ isTauriRuntime: () => false })); vi.doMock("@/lib/backend/api", () => ({ deleteSchemaCachePrefix: vi.fn().mockResolvedValue(undefined), + loadEditorSettings: vi.fn().mockResolvedValue(null), loadConnections: vi.fn().mockResolvedValue([postgresConnection({ id: "inherited", connect_timeout_secs: 7, query_timeout_secs: 12 })]), loadPinnedTreeNodeIds: vi.fn().mockResolvedValue([]), loadSidebarLayout: vi.fn().mockResolvedValue(null), @@ -194,6 +260,7 @@ describe("connectionStore timeout recovery", () => { vi.doMock("@/lib/backend/tauriRuntime", () => ({ isTauriRuntime: () => false })); vi.doMock("@/lib/backend/api", () => ({ deleteSchemaCachePrefix: vi.fn().mockResolvedValue(undefined), + loadEditorSettings: vi.fn().mockResolvedValue(null), loadConnections: vi.fn().mockResolvedValue([postgresConnection({ id: "inherited", connect_timeout_secs: 20, query_timeout_secs: 45 })]), loadPinnedTreeNodeIds: vi.fn().mockResolvedValue([]), loadSidebarLayout: vi.fn().mockResolvedValue(null), @@ -235,6 +302,7 @@ describe("connectionStore timeout recovery", () => { vi.doMock("@/lib/backend/configCrypto", () => ({ encryptConfig })); vi.doMock("@/lib/backend/api", () => ({ deleteSchemaCachePrefix: vi.fn().mockResolvedValue(undefined), + loadEditorSettings: vi.fn().mockResolvedValue(null), loadConnections: vi.fn().mockResolvedValue([postgresConnection({ id: "inherited", connect_timeout_secs: 99, connect_timeout_inherit: true, query_timeout_secs: 99, query_timeout_inherit: true })]), loadPinnedTreeNodeIds: vi.fn().mockResolvedValue([]), loadSidebarLayout: vi.fn().mockResolvedValue(null), diff --git a/apps/desktop/src/stores/__tests__/settingsStore.spec.ts b/apps/desktop/src/stores/__tests__/settingsStore.spec.ts index 66e7d5e99e..9f2aafc848 100644 --- a/apps/desktop/src/stores/__tests__/settingsStore.spec.ts +++ b/apps/desktop/src/stores/__tests__/settingsStore.spec.ts @@ -1,7 +1,7 @@ import { createPinia, setActivePinia } from "pinia"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { isProxy } from "vue"; -import { EXECUTE_MODE_CURRENT_DEFAULT_VERSION, enforceRightSidebarPanelExclusivity, normalizeAiConfig, normalizeDesktopSettings, normalizeEditorSettings, normalizeMcpGlobalPolicy, type RightSidebarPanelState, transitionRightSidebarPanels } from "@/stores/settingsStore"; +import { DEFAULT_EDITOR_SETTINGS, EXECUTE_MODE_CURRENT_DEFAULT_VERSION, enforceRightSidebarPanelExclusivity, normalizeAiConfig, normalizeDesktopSettings, normalizeEditorSettings, normalizeMcpGlobalPolicy, type RightSidebarPanelState, transitionRightSidebarPanels } from "@/stores/settingsStore"; import type { AiConfigItem } from "@/types/ai"; describe("normalizeEditorSettings", () => { @@ -561,6 +561,8 @@ describe("settingsStore persisted settings initialization", () => { const store = useSettingsStore(); await expect(store.initEditorSettings()).rejects.toThrow("storage temporarily unavailable"); + store.updateEditorSettings({ appLayout: "separated" }); + expect(saveEditorSettings).not.toHaveBeenCalled(); await store.initEditorSettings(); expect(store.isEditorSettingsLoaded).toBe(true); @@ -569,8 +571,61 @@ describe("settingsStore persisted settings initialization", () => { theme: "xcode-dark", executeMode: "all", updateNotificationsEnabled: false, + appLayout: "separated", + }); + expect(saveEditorSettings).toHaveBeenCalledWith(expect.objectContaining({ fontSize: 17, theme: "xcode-dark", appLayout: "separated" })); + }); + + it("shares concurrent initialization and applies startup changes after saved settings load", async () => { + let resolveLoad!: (value: unknown) => void; + const loadEditorSettings = vi.fn( + () => + new Promise((resolve) => { + resolveLoad = resolve; + }), + ); + const saveEditorSettings = vi.fn().mockResolvedValue(undefined); + vi.doMock("@/lib/backend/api", () => ({ loadEditorSettings, saveEditorSettings })); + + const { useSettingsStore } = await import("@/stores/settingsStore"); + const store = useSettingsStore(); + const firstInitialization = store.initEditorSettings(); + const secondInitialization = store.initEditorSettings(); + + store.updateEditorSettings({ + appLayout: "separated", + tabLayout: "wrap", + uiFontFamily: "pre-load default snapshot", }); expect(saveEditorSettings).not.toHaveBeenCalled(); + expect(loadEditorSettings).toHaveBeenCalledOnce(); + + resolveLoad({ + appLayout: "classic", + tabLayout: "scroll", + uiFontFamily: "persisted font", + toolbarItems: { ...DEFAULT_EDITOR_SETTINGS.toolbarItems, history: false }, + snippets: [{ id: "persisted", label: "Persisted", prefix: "persisted", body: "SELECT 42", enabled: true }], + executeModeDefaultVersion: EXECUTE_MODE_CURRENT_DEFAULT_VERSION, + }); + await Promise.all([firstInitialization, secondInitialization]); + + expect(store.editorSettings).toMatchObject({ + appLayout: "separated", + tabLayout: "wrap", + uiFontFamily: "pre-load default snapshot", + toolbarItems: expect.objectContaining({ history: false }), + snippets: [expect.objectContaining({ id: "persisted", body: "SELECT 42" })], + }); + expect(saveEditorSettings).toHaveBeenCalledWith( + expect.objectContaining({ + appLayout: "separated", + tabLayout: "wrap", + uiFontFamily: "pre-load default snapshot", + toolbarItems: expect.objectContaining({ history: false }), + snippets: [expect.objectContaining({ id: "persisted", body: "SELECT 42" })], + }), + ); }); }); @@ -581,11 +636,13 @@ describe("settingsStore sidebar connection sort persistence", () => { }); it("persists the selected alphabetical sort mode", async () => { + const loadEditorSettings = vi.fn().mockResolvedValue(null); const saveEditorSettings = vi.fn().mockResolvedValue(undefined); - vi.doMock("@/lib/backend/api", () => ({ saveEditorSettings })); + vi.doMock("@/lib/backend/api", () => ({ loadEditorSettings, saveEditorSettings })); const { useSettingsStore } = await import("@/stores/settingsStore"); const store = useSettingsStore(); + await store.initEditorSettings(); store.updateEditorSettings({ sidebarConnectionSortMode: "desc" }); expect(store.editorSettings.sidebarConnectionSortMode).toBe("desc"); @@ -597,11 +654,13 @@ describe("settingsStore sidebar connection sort persistence", () => { }); it("persists the retained result run display mode", async () => { + const loadEditorSettings = vi.fn().mockResolvedValue(null); const saveEditorSettings = vi.fn().mockResolvedValue(undefined); - vi.doMock("@/lib/backend/api", () => ({ saveEditorSettings })); + vi.doMock("@/lib/backend/api", () => ({ loadEditorSettings, saveEditorSettings })); const { useSettingsStore } = await import("@/stores/settingsStore"); const store = useSettingsStore(); + await store.initEditorSettings(); store.updateEditorSettings({ resultRunDisplayMode: "list" }); expect(store.editorSettings.resultRunDisplayMode).toBe("list"); @@ -617,11 +676,13 @@ describe("settingsStore regular expression match limit persistence", () => { }); it("normalizes and persists the configured match limit", async () => { + const loadEditorSettings = vi.fn().mockResolvedValue(null); const saveEditorSettings = vi.fn().mockResolvedValue(undefined); - vi.doMock("@/lib/backend/api", () => ({ saveEditorSettings })); + vi.doMock("@/lib/backend/api", () => ({ loadEditorSettings, saveEditorSettings })); const { useSettingsStore } = await import("@/stores/settingsStore"); const store = useSettingsStore(); + await store.initEditorSettings(); store.updateEditorSettings({ regexMaxMatchCount: 2500.4 }); expect(store.editorSettings.regexMaxMatchCount).toBe(2500); diff --git a/apps/desktop/src/stores/connectionStore.ts b/apps/desktop/src/stores/connectionStore.ts index bbe1de8270..bb4b04369f 100644 --- a/apps/desktop/src/stores/connectionStore.ts +++ b/apps/desktop/src/stores/connectionStore.ts @@ -7018,6 +7018,9 @@ export const useConnectionStore = defineStore("connection", () => { } async function initFromDisk() { + // Connection normalization and timeout migration depend on persisted global + // settings. Startup helpers may initialize connections before App.initApp(). + await settingsStore.initEditorSettings(); if (!initFromDiskPromise) { initFromDiskPromise = (async () => { const [pinnedOrder, saved] = await Promise.all([loadPinnedTreeNodeOrder(), api.loadConnections(), tunnelProfileStore.init()]); diff --git a/apps/desktop/src/stores/settingsStore.ts b/apps/desktop/src/stores/settingsStore.ts index feeecaa07b..cd0866408c 100644 --- a/apps/desktop/src/stores/settingsStore.ts +++ b/apps/desktop/src/stores/settingsStore.ts @@ -1159,6 +1159,10 @@ function editorSettingsSnapshot(settings: EditorSettings): EditorSettings { return JSON.parse(JSON.stringify(settings)) as EditorSettings; } +function editorSettingsPatchSnapshot(settings: Partial): Partial { + return JSON.parse(JSON.stringify(settings)) as Partial; +} + function saveEditorSettings(settings: EditorSettings) { void api.saveEditorSettings(editorSettingsSnapshot(settings)).catch(() => {}); } @@ -1184,6 +1188,8 @@ export const useSettingsStore = defineStore("settings", () => { const isDesktopSettingsLoaded = ref(false); const isMcpGlobalPolicyLoaded = ref(false); const isEditorSettingsLoaded = ref(false); + let initEditorSettingsPromise: Promise | null = null; + let pendingEditorSettingsPatches: Partial[] = []; let pendingAiChatSelection: AiChatSelectionState | null = null; let aiChatSelectionSaveRunning = false; @@ -1201,39 +1207,54 @@ export const useSettingsStore = defineStore("settings", () => { if (settingsNavigationRequest.value?.id === id) settingsNavigationRequest.value = null; } + function completeEditorSettingsInitialization() { + const pendingPatches = pendingEditorSettingsPatches; + pendingEditorSettingsPatches = []; + for (const patch of pendingPatches) applyEditorSettingsPatch(patch); + isEditorSettingsLoaded.value = true; + if (pendingPatches.length) saveEditorSettings(editorSettings.value); + } + async function initEditorSettings() { if (isEditorSettingsLoaded.value) return; - // A read failure is not the same as an empty settings record. Keeping the - // store unloaded prevents startup migrations from persisting defaults over - // settings that are temporarily unavailable. - const saved = await api.loadEditorSettings(); - if (saved && typeof saved === "object" && !Array.isArray(saved)) { - const savedSettings = saved as Partial; - const normalized = normalizeEditorSettings(savedSettings); - editorSettings.value = normalized; - const needsExecuteModeDefaultMigration = typeof savedSettings.executeModeDefaultVersion !== "number" || savedSettings.executeModeDefaultVersion < EXECUTE_MODE_CURRENT_DEFAULT_VERSION; - const savedUpdateDownloadSource = (saved as { updateDownloadSource?: unknown }).updateDownloadSource; - if (savedUpdateDownloadSource === "atomgit" || needsExecuteModeDefaultMigration) { - // Persist one-time migrations so removed or unsafe defaults cannot reappear. - await api.saveEditorSettings(normalized).catch(() => {}); - } - isEditorSettingsLoaded.value = true; - return; - } + if (!initEditorSettingsPromise) { + initEditorSettingsPromise = (async () => { + // A read failure is not the same as an empty settings record. Keeping the + // store unloaded prevents startup migrations from persisting defaults over + // settings that are temporarily unavailable. + const saved = await api.loadEditorSettings(); + if (saved && typeof saved === "object" && !Array.isArray(saved)) { + const savedSettings = saved as Partial; + const normalized = normalizeEditorSettings(savedSettings); + editorSettings.value = normalized; + const needsExecuteModeDefaultMigration = typeof savedSettings.executeModeDefaultVersion !== "number" || savedSettings.executeModeDefaultVersion < EXECUTE_MODE_CURRENT_DEFAULT_VERSION; + const savedUpdateDownloadSource = (saved as { updateDownloadSource?: unknown }).updateDownloadSource; + if (savedUpdateDownloadSource === "atomgit" || needsExecuteModeDefaultMigration) { + // Persist one-time migrations so removed or unsafe defaults cannot reappear. + await api.saveEditorSettings(normalized).catch(() => {}); + } + completeEditorSettingsInitialization(); + return; + } - const legacy = loadLegacyEditorSettings(); - if (legacy) { - editorSettings.value = legacy; - try { - await api.saveEditorSettings(legacy); - // Existing desktop users keep settings in localStorage; remove them only - // after the async store has accepted the migrated value. - clearLegacyEditorSettings(); - } catch { - /* keep legacy values for a later migration attempt */ - } + const legacy = loadLegacyEditorSettings(); + if (legacy) { + editorSettings.value = legacy; + try { + await api.saveEditorSettings(legacy); + // Existing desktop users keep settings in localStorage; remove them only + // after the async store has accepted the migrated value. + clearLegacyEditorSettings(); + } catch { + /* keep legacy values for a later migration attempt */ + } + } + completeEditorSettingsInitialization(); + })().finally(() => { + initEditorSettingsPromise = null; + }); } - isEditorSettingsLoaded.value = true; + await initEditorSettingsPromise; } async function initDesktopSettings() { @@ -1477,7 +1498,7 @@ export const useSettingsStore = defineStore("settings", () => { return !!config.endpoint && !!activeModel.value!.modelId && (!preset.requiresApiKey || !!config.apiKey); }); - function updateEditorSettings(partial: Partial) { + function applyEditorSettingsPatch(partial: Partial) { if (partial.fontFamily !== undefined) editorSettings.value.fontFamily = normalizeFontFamily(partial.fontFamily, DEFAULT_EDITOR_SETTINGS.fontFamily); if (partial.fontSize !== undefined) editorSettings.value.fontSize = partial.fontSize; if (partial.uiFontFamily !== undefined) editorSettings.value.uiFontFamily = normalizeFontFamily(partial.uiFontFamily, DEFAULT_EDITOR_SETTINGS.uiFontFamily); @@ -1605,10 +1626,19 @@ export const useSettingsStore = defineStore("settings", () => { if (partial.continueOnErrorOnBatch !== undefined) editorSettings.value.continueOnErrorOnBatch = partial.continueOnErrorOnBatch === true; if (partial.clickTableNavigationTarget !== undefined) editorSettings.value.clickTableNavigationTarget = normalizeClickTableNavigationTarget(partial.clickTableNavigationTarget); if (partial.completionTriggerMode !== undefined) editorSettings.value.completionTriggerMode = normalizeCompletionTriggerMode(partial.completionTriggerMode); + } + + function updateEditorSettings(partial: Partial) { + applyEditorSettingsPatch(partial); + if (!isEditorSettingsLoaded.value) { + pendingEditorSettingsPatches.push(editorSettingsPatchSnapshot(partial)); + return; + } saveEditorSettings(editorSettings.value); } async function persistEditorSettings(): Promise { + await initEditorSettings(); await api.saveEditorSettings(editorSettingsSnapshot(editorSettings.value)); }