fix(migrations): preserve migrated settings state

src/main.ts

@@ -53,7 +53,7 @@ export default class QuickAdd extends Plugin {
 		QuickAdd.instance = this;
 
 		await this.loadSettings();
-		settingsStore.setState(this.settings);
+		settingsStore.replaceState(this.settings);
 		this.unsubscribeSettingsStore = settingsStore.subscribe((settings) => {
 			this.settings = settings;
 			void this.saveSettings();

src/migrations/migrate.test.ts

@@ -1,4 +1,7 @@
-import { describe, it, expect, beforeEach, vi } from "vitest";
+import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
+import { DEFAULT_SETTINGS } from "src/settings";
+import { settingsStore } from "src/settingsStore";
+import migrate from "./migrate";
 
 // Mock the logger to avoid test output noise
 vi.mock("src/logger/logManager", () => ({
@@ -11,8 +14,13 @@ vi.mock("src/logger/logManager", () => ({
 describe("Migration Re-entrance Safety", () => {
 	let mockPlugin: any;
 	let mockSettings: any;
+	let unsubscribe: (() => void) | undefined;
 
 	beforeEach(() => {
+		settingsStore.replaceState(structuredClone(DEFAULT_SETTINGS));
+		unsubscribe?.();
+		unsubscribe = undefined;
+
 		// Reset settings with minimal structure needed for migration tests
 		mockSettings = {
 			choices: [],
@@ -25,6 +33,12 @@ describe("Migration Re-entrance Safety", () => {
 		};
 	});
 
+	afterEach(() => {
+		unsubscribe?.();
+		unsubscribe = undefined;
+		settingsStore.replaceState(structuredClone(DEFAULT_SETTINGS));
+	});
+
 	describe("Migration safety patterns", () => {
 		it("should verify migrations are tracked correctly", async () => {
 			// Create a simple mock migration function
@@ -142,6 +156,130 @@ describe("Migration Re-entrance Safety", () => {
 	});
 
 	describe("Specific migration interaction patterns", () => {
+		it("does not let a later store-backed migration restore stale choices", async () => {
+			const legacyTemplateChoice = {
+				id: "daily-note",
+				name: "Open Daily Note",
+				type: "Template",
+				setFileExistsBehavior: true,
+				fileExistsMode: "Nothing",
+			};
+			const loadedSettings = {
+				...structuredClone(DEFAULT_SETTINGS),
+				choices: [legacyTemplateChoice],
+				ai: {
+					...structuredClone(DEFAULT_SETTINGS.ai),
+					providers: [
+						{
+							name: "Provider without model source",
+							type: "openai",
+							apiKey: "",
+							defaultModel: "",
+							models: [],
+						},
+					],
+				},
+				migrations: {
+					useQuickAddTemplateFolder: true,
+					incrementFileNameSettingMoveToDefaultBehavior: true,
+					consolidateFileExistsBehavior: false,
+					repairTemplateFileExistsBehavior: true,
+					mutualExclusionInsertAfterAndWriteToBottomOfFile: true,
+					setVersionAfterUpdateModalRelease: true,
+					addDefaultAIProviders: true,
+					removeMacroIndirection: true,
+					migrateFileOpeningSettings: true,
+					backfillFileOpeningDefaults: true,
+					setProviderModelDiscoveryMode: false,
+					migrateProviderApiKeysToSecretStorage: true,
+				},
+			};
+
+			settingsStore.replaceState(loadedSettings as any);
+			mockPlugin = {
+				manifest: { version: "2.12.2" },
+				settings: structuredClone(loadedSettings),
+				saveSettings: vi.fn(),
+			};
+			unsubscribe = settingsStore.subscribe((settings) => {
+				mockPlugin.settings = settings;
+				void mockPlugin.saveSettings();
+			});
+
+			await migrate(mockPlugin);
+
+			expect(mockPlugin.settings.choices[0]).toMatchObject({
+				fileExistsBehavior: { kind: "apply", mode: "doNothing" },
+			});
+			expect(mockPlugin.settings.choices[0]).not.toHaveProperty(
+				"setFileExistsBehavior",
+			);
+			expect(mockPlugin.settings.choices[0]).not.toHaveProperty(
+				"fileExistsMode",
+			);
+			expect(
+				mockPlugin.settings.migrations.consolidateFileExistsBehavior,
+			).toBe(true);
+			expect(
+				mockPlugin.settings.migrations.setProviderModelDiscoveryMode,
+			).toBe(true);
+		});
+
+		it("repairs stale legacy choices when the earlier consolidation is marked complete", async () => {
+			const loadedSettings = {
+				...structuredClone(DEFAULT_SETTINGS),
+				choices: [
+					{
+						id: "daily-note",
+						name: "Open Daily Note",
+						type: "Template",
+						setFileExistsBehavior: true,
+						fileExistsMode: "Nothing",
+					},
+				],
+				migrations: {
+					useQuickAddTemplateFolder: true,
+					incrementFileNameSettingMoveToDefaultBehavior: true,
+					consolidateFileExistsBehavior: true,
+					repairTemplateFileExistsBehavior: false,
+					mutualExclusionInsertAfterAndWriteToBottomOfFile: true,
+					setVersionAfterUpdateModalRelease: true,
+					addDefaultAIProviders: true,
+					removeMacroIndirection: true,
+					migrateFileOpeningSettings: true,
+					backfillFileOpeningDefaults: true,
+					setProviderModelDiscoveryMode: true,
+					migrateProviderApiKeysToSecretStorage: true,
+				},
+			};
+
+			settingsStore.replaceState(loadedSettings as any);
+			mockPlugin = {
+				manifest: { version: "2.12.2" },
+				settings: structuredClone(loadedSettings),
+				saveSettings: vi.fn(),
+			};
+			unsubscribe = settingsStore.subscribe((settings) => {
+				mockPlugin.settings = settings;
+				void mockPlugin.saveSettings();
+			});
+
+			await migrate(mockPlugin);
+
+			expect(mockPlugin.settings.choices[0]).toMatchObject({
+				fileExistsBehavior: { kind: "apply", mode: "doNothing" },
+			});
+			expect(mockPlugin.settings.choices[0]).not.toHaveProperty(
+				"setFileExistsBehavior",
+			);
+			expect(mockPlugin.settings.choices[0]).not.toHaveProperty(
+				"fileExistsMode",
+			);
+			expect(
+				mockPlugin.settings.migrations.repairTemplateFileExistsBehavior,
+			).toBe(true);
+		});
+
 		it("should handle macro-related migration sequence", async () => {
 			// This test verifies the specific pattern we're concerned about:
 			// One migration embeds macros, another removes macro references

src/migrations/migrate.ts

@@ -4,6 +4,7 @@ import type { Migrations } from "./Migrations";
 import useQuickAddTemplateFolder from "./useQuickAddTemplateFolder";
 import incrementFileNameSettingMoveToDefaultBehavior from "./incrementFileNameSettingMoveToDefaultBehavior";
 import consolidateFileExistsBehavior from "./consolidateFileExistsBehavior";
+import repairTemplateFileExistsBehavior from "./repairTemplateFileExistsBehavior";
 import mutualExclusionInsertAfterAndWriteToBottomOfFile from "./mutualExclusionInsertAfterAndWriteToBottomOfFile";
 import setVersionAfterUpdateModalRelease from "./setVersionAfterUpdateModalRelease";
 import addDefaultAIProviders from "./addDefaultAIProviders";
@@ -13,11 +14,13 @@ import backfillFileOpeningDefaults from "./backfillFileOpeningDefaults";
 import setProviderModelDiscoveryMode from "./setProviderModelDiscoveryMode";
 import { deepClone } from "src/utils/deepClone";
 import migrateProviderApiKeysToSecretStorage from "./migrateProviderApiKeysToSecretStorage";
+import { settingsStore } from "src/settingsStore";
 
 const migrations: Migrations = {
 	useQuickAddTemplateFolder,
 	incrementFileNameSettingMoveToDefaultBehavior,
 	consolidateFileExistsBehavior,
+	repairTemplateFileExistsBehavior,
 	mutualExclusionInsertAfterAndWriteToBottomOfFile,
 	setVersionAfterUpdateModalRelease,
 	addDefaultAIProviders,
@@ -39,18 +42,26 @@ async function migrate(plugin: QuickAdd): Promise<void> {
 		return;
 	}
 
+	settingsStore.replaceState(deepClone(plugin.settings));
+
 	// Could batch-run with Promise.all, but we want to log each migration as it runs.
 	for (const migration of migrationsToRun as (keyof Migrations)[]) {
 		log.logMessage(
 			`Running migration ${migration}: ${migrations[migration].description}`
 		);
 
 		const backup = deepClone(plugin.settings);
+		const storeBeforeMigration = settingsStore.getState();
 
 		try {
 			await migrations[migration].migrate(plugin);
 
+			if (settingsStore.getState() !== storeBeforeMigration) {
+				plugin.settings = deepClone(settingsStore.getState());
+			}
+
 			plugin.settings.migrations[migration] = true;
+			settingsStore.replaceState(deepClone(plugin.settings));
 
 			log.logMessage(`Migration ${migration} successful.`);
 		} catch (error) {
@@ -59,6 +70,7 @@ async function migrate(plugin: QuickAdd): Promise<void> {
 			);
 
 			plugin.settings = backup;
+			settingsStore.replaceState(deepClone(plugin.settings));
 		}
 	}
 

src/migrations/repairTemplateFileExistsBehavior.ts

@@ -0,0 +1,10 @@
+import consolidateFileExistsBehavior from "./consolidateFileExistsBehavior";
+import type { Migration } from "./Migrations";
+
+const repairTemplateFileExistsBehavior: Migration = {
+	description:
+		"Repair template file collision settings that may have been left in legacy format",
+	migrate: consolidateFileExistsBehavior.migrate,
+};
+
+export default repairTemplateFileExistsBehavior;

src/settings.ts

@@ -43,6 +43,7 @@ export interface QuickAddSettings {
 		useQuickAddTemplateFolder: boolean;
 		incrementFileNameSettingMoveToDefaultBehavior: boolean;
 		consolidateFileExistsBehavior: boolean;
+		repairTemplateFileExistsBehavior: boolean;
 		mutualExclusionInsertAfterAndWriteToBottomOfFile: boolean;
 		setVersionAfterUpdateModalRelease: boolean;
 		addDefaultAIProviders: boolean;
@@ -86,6 +87,7 @@ export const DEFAULT_SETTINGS: QuickAddSettings = {
 		useQuickAddTemplateFolder: false,
 		incrementFileNameSettingMoveToDefaultBehavior: false,
 		consolidateFileExistsBehavior: false,
+		repairTemplateFileExistsBehavior: false,
 		mutualExclusionInsertAfterAndWriteToBottomOfFile: false,
 		setVersionAfterUpdateModalRelease: false,
 		addDefaultAIProviders: false,

src/settingsStore.ts

@@ -14,6 +14,7 @@ export const settingsStore = (() => {
 
 	return {
 		getState,
+		replaceState: (state: SettingsState) => setState(state, true),
 		setState,
 		subscribe,
 

tests/e2e/file-exists-behavior.test.ts

@@ -364,3 +364,45 @@ describe("migration: consolidateFileExistsBehavior", () => {
 		expect(data.migrations.consolidateFileExistsBehavior).toBe(true);
 	});
 });
+
+describe("migration: repairTemplateFileExistsBehavior", () => {
+	beforeAll(async () => {
+		const root = sandbox.root;
+		const migrationChoice = (id: string) => templateChoice(id, `${root}/repair-test`);
+
+		await qa.data<QuickAddData>().patch((data) => {
+			clearTestChoices(data);
+
+			data.choices.push(
+				{ ...migrationChoice("__qa-test-r1-stale-nothing"), setFileExistsBehavior: true, fileExistsMode: "Nothing" },
+				{ ...migrationChoice("__qa-test-r2-already"), fileExistsBehavior: { kind: "apply", mode: "overwrite" } },
+			);
+
+			data.migrations.consolidateFileExistsBehavior = true;
+			data.migrations.repairTemplateFileExistsBehavior = false;
+		});
+
+		await qa.reload();
+		await qa.waitForData<QuickAddData>(
+			(data) => data.migrations.repairTemplateFileExistsBehavior === true,
+			{ timeoutMs: 10_000, intervalMs: 300 },
+		);
+	}, 15_000);
+
+	async function readChoice(id: string) {
+		const data = await qa.data<QuickAddData>().read();
+		return findChoice(data, id);
+	}
+
+	it("repairs stale legacy do-nothing behavior", async () => {
+		const choice = await readChoice("__qa-test-r1-stale-nothing");
+		expect(choice?.fileExistsBehavior).toEqual(behavior.doNothing);
+		expect(choice).not.toHaveProperty("setFileExistsBehavior");
+		expect(choice).not.toHaveProperty("fileExistsMode");
+	});
+
+	it("preserves already-migrated behavior", async () => {
+		const choice = await readChoice("__qa-test-r2-already");
+		expect(choice?.fileExistsBehavior).toEqual(behavior.overwrite);
+	});
+});