diff --git a/src/settings/tab.obsidian.tsx b/src/settings/tab.obsidian.tsx index 0f458351..35c2a8b5 100644 --- a/src/settings/tab.obsidian.tsx +++ b/src/settings/tab.obsidian.tsx @@ -21,7 +21,9 @@ const SETTINGS_INTRO_TEXT = "Codex Panel stores panel preferences only. Runtime export class CodexPanelSettingTab extends PluginSettingTab { private readonly dynamicSections: SettingsDynamicSectionsController; - private lastSavedSettings: CodexPanelSettings; + private displayed = false; + private settingsShellRevision = 0; + private settingsMutationQueue: Promise = Promise.resolve(); private archivedDeleteConfirmThreadId: string | null = null; private disposeOutsidePointer: (() => void) | null = null; private readonly cancelArchivedDeleteConfirmOnOutsidePointer = (event: PointerEvent): void => { @@ -33,7 +35,7 @@ export class CodexPanelSettingTab extends PluginSettingTab { if (deleteConfirm && this.containerEl.contains(deleteConfirm)) return; } this.archivedDeleteConfirmThreadId = null; - this.renderSettingsShell(); + this.requestRender(); }; constructor( @@ -42,10 +44,9 @@ export class CodexPanelSettingTab extends PluginSettingTab { private readonly plugin: CodexPanelSettingTabHost, ) { super(app, owner); - this.lastSavedSettings = { ...plugin.settings }; this.dynamicSections = new SettingsDynamicSectionsController(plugin, { display: () => { - this.renderSettingsShell(); + this.requestRender(); }, notify: (message) => { new Notice(message); @@ -54,11 +55,13 @@ export class CodexPanelSettingTab extends PluginSettingTab { } display(): void { + this.displayed = true; this.dynamicSections.activate(); this.renderSettingsTab({ autoLoadDynamicSections: true }); } override hide(): void { + this.displayed = false; this.disposeOutsidePointer?.(); this.disposeOutsidePointer = null; this.archivedDeleteConfirmThreadId = null; @@ -82,6 +85,7 @@ export class CodexPanelSettingTab extends PluginSettingTab { renderUiRoot( this.containerEl, void this.setArchiveExportTags(value), onRestore: (threadId) => { this.archivedDeleteConfirmThreadId = null; - this.renderSettingsShell(); + this.requestRender(); void this.dynamicSections.restoreArchivedThread(threadId); }, onStartDelete: (threadId) => { this.archivedDeleteConfirmThreadId = threadId; - this.renderSettingsShell(); + this.requestRender(); }, onDelete: (threadId) => { this.archivedDeleteConfirmThreadId = null; - this.renderSettingsShell(); + this.requestRender(); void this.dynamicSections.deleteArchivedThread(threadId); }, }, @@ -200,128 +204,152 @@ export class CodexPanelSettingTab extends PluginSettingTab { this.dynamicSections.maybeAutoLoadDynamicSections(); } - private async setCodexPath(value: string): Promise { + private setCodexPath(value: string): Promise { const codexPath = value.trim() || DEFAULT_CODEX_PATH; - if (codexPath === this.plugin.settings.codexPath) return; - this.plugin.settings.codexPath = codexPath; - if (!(await this.persistSettings())) return; - this.dynamicSections.resetDynamicSectionContext(); - this.plugin.dynamicData.notifyContextChanged(); - this.plugin.refreshOpenViews(); - this.renderSettingsShell(); + return this.queueSettingsMutation( + () => { + if (codexPath === this.plugin.settings.codexPath) return false; + this.plugin.settings.codexPath = codexPath; + return true; + }, + () => { + this.dynamicSections.resetDynamicSectionContext(); + this.plugin.dynamicData.notifyContextChanged(); + this.plugin.refreshOpenViews(); + }, + ); } - private async setShowToolbar(value: boolean): Promise { - this.plugin.settings.showToolbar = value; - if (!(await this.persistSettings())) return; - this.plugin.refreshOpenViews(); - this.renderSettingsShell(); + private setShowToolbar(value: boolean): Promise { + return this.queueSettingsMutation( + () => { + this.plugin.settings.showToolbar = value; + }, + () => { + this.plugin.refreshOpenViews(); + }, + ); } - private async setSendShortcut(value: "enter" | "mod-enter"): Promise { - this.plugin.settings.sendShortcut = value; - if (!(await this.persistSettings())) return; - this.renderSettingsShell(); + private setSendShortcut(value: "enter" | "mod-enter"): Promise { + return this.queueSettingsMutation(() => { + this.plugin.settings.sendShortcut = value; + }); } - private async setScrollThreadFromComposerEdges(value: boolean): Promise { - this.plugin.settings.scrollThreadFromComposerEdges = value; - if (!(await this.persistSettings())) return; - this.renderSettingsShell(); + private setScrollThreadFromComposerEdges(value: boolean): Promise { + return this.queueSettingsMutation(() => { + this.plugin.settings.scrollThreadFromComposerEdges = value; + }); } - private async setReferenceActiveNoteOnSend(value: boolean): Promise { - this.plugin.settings.referenceActiveNoteOnSend = value; - if (!(await this.persistSettings())) return; - this.renderSettingsShell(); + private setReferenceActiveNoteOnSend(value: boolean): Promise { + return this.queueSettingsMutation(() => { + this.plugin.settings.referenceActiveNoteOnSend = value; + }); } - private async setAttachmentFolder(value: string): Promise { - this.plugin.settings.attachmentFolder = value.trim() || DEFAULT_ATTACHMENT_FOLDER; - if (!(await this.persistSettings())) return; - this.renderSettingsShell(); + private setAttachmentFolder(value: string): Promise { + return this.queueSettingsMutation(() => { + this.plugin.settings.attachmentFolder = value.trim() || DEFAULT_ATTACHMENT_FOLDER; + }); } - private async setClipFolder(value: string): Promise { - this.plugin.settings.clipFolder = value.trim() || DEFAULT_CLIP_FOLDER; - if (!(await this.persistSettings())) return; - this.renderSettingsShell(); + private setClipFolder(value: string): Promise { + return this.queueSettingsMutation(() => { + this.plugin.settings.clipFolder = value.trim() || DEFAULT_CLIP_FOLDER; + }); } - private async setClipFilenameTemplate(value: string): Promise { - this.plugin.settings.clipFilenameTemplate = value.trim() || DEFAULT_CLIP_FILENAME_TEMPLATE; - if (!(await this.persistSettings())) return; - this.renderSettingsShell(); + private setClipFilenameTemplate(value: string): Promise { + return this.queueSettingsMutation(() => { + this.plugin.settings.clipFilenameTemplate = value.trim() || DEFAULT_CLIP_FILENAME_TEMPLATE; + }); } - private async setClipTags(value: string): Promise { - this.plugin.settings.clipTags = value.trim(); - if (!(await this.persistSettings())) return; - this.renderSettingsShell(); + private setClipTags(value: string): Promise { + return this.queueSettingsMutation(() => { + this.plugin.settings.clipTags = value.trim(); + }); } - private async setArchiveExportEnabled(enabled: boolean): Promise { - this.plugin.settings.archiveExportEnabled = enabled; - if (!(await this.persistSettings())) return; - this.renderSettingsShell(); + private setArchiveExportEnabled(enabled: boolean): Promise { + return this.queueSettingsMutation(() => { + this.plugin.settings.archiveExportEnabled = enabled; + }); } - private async setArchiveExportFolderTemplate(value: string): Promise { - this.plugin.settings.archiveExportFolderTemplate = value.trim() || DEFAULT_ARCHIVE_EXPORT_FOLDER_TEMPLATE; - if (!(await this.persistSettings())) return; - this.renderSettingsShell(); + private setArchiveExportFolderTemplate(value: string): Promise { + return this.queueSettingsMutation(() => { + this.plugin.settings.archiveExportFolderTemplate = value.trim() || DEFAULT_ARCHIVE_EXPORT_FOLDER_TEMPLATE; + }); } - private async setArchiveExportFilenameTemplate(value: string): Promise { - this.plugin.settings.archiveExportFilenameTemplate = value.trim() || DEFAULT_ARCHIVE_EXPORT_FILENAME_TEMPLATE; - if (!(await this.persistSettings())) return; - this.renderSettingsShell(); + private setArchiveExportFilenameTemplate(value: string): Promise { + return this.queueSettingsMutation(() => { + this.plugin.settings.archiveExportFilenameTemplate = value.trim() || DEFAULT_ARCHIVE_EXPORT_FILENAME_TEMPLATE; + }); } - private async setArchiveExportTags(value: string): Promise { - this.plugin.settings.archiveExportTags = value.trim(); - if (!(await this.persistSettings())) return; - this.renderSettingsShell(); + private setArchiveExportTags(value: string): Promise { + return this.queueSettingsMutation(() => { + this.plugin.settings.archiveExportTags = value.trim(); + }); } - private async setThreadNamingModel(value: string | null): Promise { - this.plugin.settings.threadNamingModel = value; - if (!this.dynamicSections.namingEffortSupported(this.plugin.settings.threadNamingEffort)) { - this.plugin.settings.threadNamingEffort = null; - } - if (!(await this.persistSettings())) return; - this.renderSettingsShell(); + private setThreadNamingModel(value: string | null): Promise { + return this.queueSettingsMutation(() => { + this.plugin.settings.threadNamingModel = value; + if (!this.dynamicSections.namingEffortSupported(this.plugin.settings.threadNamingEffort)) { + this.plugin.settings.threadNamingEffort = null; + } + }); } - private async setThreadNamingEffort(value: ReasoningEffort | null): Promise { - this.plugin.settings.threadNamingEffort = value; - if (!(await this.persistSettings())) return; + private setThreadNamingEffort(value: ReasoningEffort | null): Promise { + return this.queueSettingsMutation(() => { + this.plugin.settings.threadNamingEffort = value; + }); } - private async setRewriteSelectionModel(value: string | null): Promise { - this.plugin.settings.rewriteSelectionModel = value; - if (!this.dynamicSections.rewriteSelectionEffortSupported(this.plugin.settings.rewriteSelectionEffort)) { - this.plugin.settings.rewriteSelectionEffort = null; - } - if (!(await this.persistSettings())) return; - this.renderSettingsShell(); + private setRewriteSelectionModel(value: string | null): Promise { + return this.queueSettingsMutation(() => { + this.plugin.settings.rewriteSelectionModel = value; + if (!this.dynamicSections.rewriteSelectionEffortSupported(this.plugin.settings.rewriteSelectionEffort)) { + this.plugin.settings.rewriteSelectionEffort = null; + } + }); } - private async setRewriteSelectionEffort(value: ReasoningEffort | null): Promise { - this.plugin.settings.rewriteSelectionEffort = value; - await this.persistSettings(); + private setRewriteSelectionEffort(value: ReasoningEffort | null): Promise { + return this.queueSettingsMutation(() => { + this.plugin.settings.rewriteSelectionEffort = value; + }); } - private async persistSettings(): Promise { - try { - await this.plugin.saveSettings(); - this.lastSavedSettings = { ...this.plugin.settings }; - return true; - } catch (error) { - Object.assign(this.plugin.settings, this.lastSavedSettings); - new Notice(`Failed to save Codex Panel settings: ${error instanceof Error ? error.message : String(error)}`); - this.renderSettingsShell(); - return false; - } + private queueSettingsMutation(mutate: () => boolean | undefined, onSaved?: () => void): Promise { + const operation = this.settingsMutationQueue.then(async () => { + const previousSettings: CodexPanelSettings = { ...this.plugin.settings }; + if (mutate() === false) return; + try { + await this.plugin.saveSettings(); + } catch (error) { + Object.assign(this.plugin.settings, previousSettings); + this.settingsShellRevision += 1; + new Notice(`Failed to save Codex Panel settings: ${error instanceof Error ? error.message : String(error)}`); + return; + } + onSaved?.(); + }); + const settledOperation = operation.catch(() => undefined); + this.settingsMutationQueue = settledOperation; + void settledOperation.then(() => { + if (this.settingsMutationQueue === settledOperation) this.requestRender(); + }); + return operation; + } + + private requestRender(): void { + if (this.displayed) this.renderSettingsShell(); } } diff --git a/tests/settings/settings-tab.test.ts b/tests/settings/settings-tab.test.ts index 3adbe34a..9919f4c0 100644 --- a/tests/settings/settings-tab.test.ts +++ b/tests/settings/settings-tab.test.ts @@ -124,6 +124,99 @@ describe("settings tab", () => { expect(settingDesc(tab, "Show chat toolbar")).toContain("toolbar above chat panels"); }); + it("finishes a pending settings save without remounting a hidden tab", async () => { + const save = deferred(); + const saveSettings = vi.fn(() => save.promise); + const refreshOpenViews = vi.fn(); + const tab = newSettingsTab({ saveSettings, refreshOpenViews }); + + tab.display(); + const toggle = inputForSetting(tab, "Show chat toolbar"); + if (!toggle) throw new Error("Missing toolbar visibility toggle"); + toggle.checked = false; + toggle.dispatchEvent(new Event("change")); + await Promise.resolve(); + + tab.hide(); + expect(tab.containerEl.children).toHaveLength(0); + + save.resolve(undefined); + await flushPromises(); + + expect(saveSettings).toHaveBeenCalledOnce(); + expect(refreshOpenViews).toHaveBeenCalledOnce(); + expect(tab.containerEl.children).toHaveLength(0); + }); + + it("serializes overlapping settings saves", async () => { + const firstSave = deferred(); + const saveSettings = vi.fn().mockReturnValueOnce(firstSave.promise).mockResolvedValueOnce(undefined); + const tab = newSettingsTab({ saveSettings }); + + tab.display(); + const shortcut = selectForSetting(tab, "Send shortcut"); + const toggle = inputForSetting(tab, "Reference active file on send"); + if (!shortcut || !toggle) throw new Error("Missing settings controls"); + + shortcut.value = "mod-enter"; + shortcut.dispatchEvent(new Event("change")); + toggle.checked = true; + toggle.dispatchEvent(new Event("change")); + await Promise.resolve(); + + expect(saveSettings).toHaveBeenCalledOnce(); + + firstSave.resolve(undefined); + await flushPromises(); + + expect(saveSettings).toHaveBeenCalledTimes(2); + }); + + it("keeps the latest native control value while an earlier save is pending", async () => { + const firstSave = deferred(); + const saveSettings = vi.fn().mockReturnValueOnce(firstSave.promise).mockResolvedValueOnce(undefined); + const tab = newSettingsTab({ saveSettings }); + + tab.display(); + const toggle = inputForSetting(tab, "Show chat toolbar"); + if (!toggle) throw new Error("Missing toolbar visibility toggle"); + + toggle.checked = false; + toggle.dispatchEvent(new Event("change")); + toggle.checked = true; + toggle.dispatchEvent(new Event("change")); + await Promise.resolve(); + + expect(toggle.checked).toBe(true); + expect(saveSettings).toHaveBeenCalledOnce(); + + firstSave.resolve(undefined); + await flushPromises(); + + expect(inputForSetting(tab, "Show chat toolbar")?.checked).toBe(true); + expect(saveSettings).toHaveBeenCalledTimes(2); + }); + + it("rolls back only the failed mutation after an earlier queued save succeeds", async () => { + const saveSettings = vi.fn().mockResolvedValueOnce(undefined).mockRejectedValueOnce(new Error("disk full")); + const tab = newSettingsTab({ saveSettings }); + + tab.display(); + const shortcut = selectForSetting(tab, "Send shortcut"); + const toggle = inputForSetting(tab, "Reference active file on send"); + if (!shortcut || !toggle) throw new Error("Missing settings controls"); + + shortcut.value = "mod-enter"; + shortcut.dispatchEvent(new Event("change")); + toggle.checked = true; + toggle.dispatchEvent(new Event("change")); + await flushPromises(); + + expect(selectForSetting(tab, "Send shortcut")?.value).toBe("mod-enter"); + expect(inputForSetting(tab, "Reference active file on send")?.checked).toBe(false); + expect(notices).toContain("Failed to save Codex Panel settings: disk full"); + }); + it("saves the composer line edge scroll setting", async () => { const saveSettings = vi.fn().mockResolvedValue(undefined); const tab = newSettingsTab({ saveSettings });