fix(persistence): never overwrite an unreadable data.json or write during window unload

src/main.activateView.test.ts

@@ -251,6 +251,13 @@ describe("PodNotes onload wiring (#55)", () => {
 			registerView: vi.fn(),
 			registerObsidianProtocolHandler: vi.fn(),
 			registerEvent: vi.fn(),
+			registerDomEvent: vi.fn(),
+			pendingSave: null,
+			pendingSaveWaiters: [],
+			saveScheduled: false,
+			saveChain: Promise.resolve(),
+			writeRefusal: null,
+			writeInFlight: null,
 			mediaSessionActions: [],
 			storeUnsubscribers: [],
 			views: new Set(),
@@ -277,6 +284,89 @@ describe("PodNotes onload wiring (#55)", () => {
 		return { commands, ribbonCalls, activateSpy, plugin };
 	}
 
+	it("starts no save once the window fires pagehide", async () => {
+		const { plugin } = await loadPlugin({ schemaVersion: 2 });
+		const domEvents = vi.mocked(plugin.registerDomEvent).mock.calls as unknown as Array<
+			[EventTarget, string, () => void]
+		>;
+		const onPageHide = domEvents.find(
+			([target, type]) => target === window && type === "pagehide",
+		)?.[2];
+		const saveData = vi.mocked(plugin.saveData);
+		saveData.mockClear();
+
+		expect(onPageHide).toBeDefined();
+		onPageHide?.();
+
+		await expect(plugin.saveSettingsStrict()).rejects.toThrow(/window unloads/);
+		expect(saveData).not.toHaveBeenCalled();
+	});
+
+	function quitHandler(
+		plugin: PodNotes,
+	): (tasks: { addPromise: (p: Promise<void>) => void }) => void {
+		const calls = vi.mocked(plugin.app.workspace.on).mock.calls as unknown as Array<
+			[string, (tasks: { addPromise: (p: Promise<void>) => void }) => void]
+		>;
+		const handler = calls.find(([name]) => name === "quit")?.[1];
+		expect(handler).toBeDefined();
+		return handler!;
+	}
+
+	it("adds no quit task when no save is in flight, and saves nothing after quit", async () => {
+		const { plugin } = await loadPlugin({ schemaVersion: 2 });
+		const saveData = vi.mocked(plugin.saveData);
+		saveData.mockClear();
+		const addPromise = vi.fn();
+
+		quitHandler(plugin)({ addPromise });
+
+		expect(addPromise).not.toHaveBeenCalled();
+		await expect(plugin.saveSettingsStrict()).rejects.toThrow(/closes/);
+		expect(saveData).not.toHaveBeenCalled();
+	});
+
+	it("makes quit wait for the save in flight", async () => {
+		const { plugin } = await loadPlugin({ schemaVersion: 2 });
+		let finishWrite = () => {};
+		vi.mocked(plugin.saveData).mockImplementation(
+			() => new Promise<void>((resolve) => (finishWrite = resolve)),
+		);
+		const addPromise = vi.fn();
+
+		void plugin.saveSettings();
+		await vi.waitFor(() => expect(plugin.saveData).toHaveBeenCalled());
+		quitHandler(plugin)({ addPromise });
+
+		expect(addPromise).toHaveBeenCalledTimes(1);
+		const quitTask: Promise<void> = addPromise.mock.calls[0][0];
+		let quitTaskDone = false;
+		void quitTask.then(() => (quitTaskDone = true));
+		await new Promise((resolve) => setTimeout(resolve, 0));
+		expect(quitTaskDone).toBe(false);
+
+		finishWrite();
+		await quitTask;
+		expect(quitTaskDone).toBe(true);
+	});
+
+	it("gives quit a task that resolves even when the save in flight fails", async () => {
+		const { plugin } = await loadPlugin({ schemaVersion: 2 });
+		let failWrite = (_error: Error) => {};
+		vi.mocked(plugin.saveData).mockImplementation(
+			() => new Promise<void>((_resolve, reject) => (failWrite = reject)),
+		);
+		vi.spyOn(console, "error").mockImplementation(() => undefined);
+		const addPromise = vi.fn();
+
+		void plugin.saveSettings();
+		await vi.waitFor(() => expect(plugin.saveData).toHaveBeenCalled());
+		quitHandler(plugin)({ addPromise });
+		failWrite(new Error("disk full"));
+
+		await expect(addPromise.mock.calls[0][0]).resolves.toBeUndefined();
+	});
+
 	it("registers Show PodNotes as an always-available callback, not a leaf-gated checkCallback", async () => {
 		const { commands } = await loadPlugin();
 

src/main.persistence.test.ts

@@ -17,11 +17,16 @@ function memorySecretStorage(initial: Record<string, string> = {}) {
 
 function makePlugin(
 	saveData: (data: unknown) => Promise<void> = vi.fn().mockResolvedValue(undefined),
+	{ dataFileExists = false } = {},
 ): PodNotes {
 	const { storage } = memorySecretStorage();
 	const plugin = Object.create(PodNotes.prototype) as PodNotes;
 	Object.assign(plugin, {
-		app: { secretStorage: storage },
+		app: {
+			secretStorage: storage,
+			vault: { adapter: { exists: vi.fn().mockResolvedValue(dataFileExists) } },
+		},
+		manifest: { dir: ".obsidian/plugins/podnotes" },
 		credentials: new CredentialRepository(storage),
 		isReady: true,
 		settings: structuredClone(DEFAULT_SETTINGS),
@@ -237,6 +242,59 @@ describe("PodNotes persistence integration", () => {
 		);
 	});
 
+	it("leaves a data.json it cannot read unchanged and refuses every save", async () => {
+		const saveData = vi.fn().mockResolvedValue(undefined);
+		const plugin = makePlugin(saveData, { dataFileExists: true });
+		const privateFeedUrl = "podnotes-feed-url-00000000-0000-4000-8000-000000000000";
+		const { storage, values } = memorySecretStorage({
+			[privateFeedUrl]: "https://user:token@example.com/feed.xml",
+		});
+		Object.assign(plugin, {
+			app: { ...plugin.app, secretStorage: storage },
+			loadData: vi.fn().mockResolvedValue(undefined),
+		});
+		vi.spyOn(console, "error").mockImplementation(() => undefined);
+
+		await plugin.loadSettings();
+		await plugin.saveSettings();
+
+		await expect(plugin.saveSettingsStrict()).rejects.toThrow(/could not read/);
+		expect(saveData).not.toHaveBeenCalled();
+		expect(plugin.settings).toEqual(DEFAULT_SETTINGS);
+		expect(values.get(privateFeedUrl)).toBe("https://user:token@example.com/feed.xml");
+	});
+
+	it("starts no write once the window unloads and rejects the queued strict save", async () => {
+		const resolvers: Array<() => void> = [];
+		const saveData = vi.fn(() => new Promise<void>((resolve) => resolvers.push(resolve)));
+		const plugin = makePlugin(saveData);
+		vi.spyOn(console, "error").mockImplementation(() => undefined);
+
+		void plugin.saveSettings();
+		await vi.waitFor(() => expect(saveData).toHaveBeenCalledTimes(1));
+		const queued = plugin.saveSettingsStrict();
+		Object.assign(plugin, { writeRefusal: "unloading" });
+		const late = plugin.saveSettingsStrict();
+		resolvers[0]();
+
+		await expect(queued).rejects.toThrow("unloading");
+		await expect(late).rejects.toThrow("unloading");
+		expect(saveData).toHaveBeenCalledTimes(1);
+	});
+
+	it("starts no migration write once the window unloads", async () => {
+		const saveData = vi.fn().mockResolvedValue(undefined);
+		const plugin = makePlugin(saveData);
+		Object.assign(plugin, {
+			loadData: vi.fn().mockResolvedValue({ defaultVolume: 0.4 }),
+			writeRefusal: "unloading",
+		});
+		vi.spyOn(console, "error").mockImplementation(() => undefined);
+
+		await expect(plugin.loadSettings()).rejects.toThrow("unloading");
+		expect(saveData).not.toHaveBeenCalled();
+	});
+
 	it("does not rewrite data that is already schema v2", async () => {
 		const saveData = vi.fn().mockResolvedValue(undefined);
 		const plugin = makePlugin(saveData);

src/main.ts

@@ -16,10 +16,10 @@ import {
 } from "src/store";
 import { bindStoresToSettings } from "src/store/persistence";
 import { registerCommands } from "src/commands";
-import { Notice, Platform, Plugin, type Editor, type WorkspaceLeaf } from "obsidian";
+import { normalizePath, Notice, Platform, Plugin, type Editor, type WorkspaceLeaf } from "obsidian";
 import { API } from "src/API/API";
 import type { IAPI } from "src/API/IAPI";
-import { VIEW_TYPE } from "src/constants";
+import { DEFAULT_SETTINGS, VIEW_TYPE } from "src/constants";
 import { PodNotesSettingsTab } from "src/ui/settings/PodNotesSettingsTab";
 import { MainView } from "src/ui/PodcastView";
 import type { IPodNotesSettings } from "./types/IPodNotesSettings";
@@ -94,6 +94,13 @@ export default class PodNotes extends Plugin implements IPodNotes {
 	private isUnloaded = false;
 	private podcastViewMountEnabled = true;
 	private isReady = false;
+	// Why data.json must not be written, or null while writes are allowed. Set when the
+	// file exists but could not be read (a sync client's half-written copy, or one a
+	// reload cut off), and when the window starts unloading or Obsidian quits: a write
+	// started then is cut off after it truncates the file, which leaves data.json empty.
+	private writeRefusal: string | null = null;
+	// The data.json write in progress, settled either way, or null. Writes never overlap.
+	private writeInFlight: Promise<void> | null = null;
 	private pendingSave: IPodNotesSettings | null = null;
 	private pendingSaveWaiters: SaveWaiter[] = [];
 	private saveScheduled = false;
@@ -105,6 +112,18 @@ export default class PodNotes extends Plugin implements IPodNotes {
 	override async onload() {
 		this.isUnloaded = false;
 		this.podcastViewMountEnabled = !this.isMobileRuntime();
+		this.registerDomEvent(window, "pagehide", () => {
+			this.writeRefusal = "PodNotes does not start a save while the window unloads.";
+		});
+		// Obsidian closes the window once its quit tasks settle, so start no write after
+		// quit and let a write in flight finish first. A reload fires no quit; pagehide
+		// covers it. A rejected quit task would stall the close until Obsidian forces it.
+		this.registerEvent(
+			this.app.workspace.on("quit", (tasks) => {
+				this.writeRefusal = "PodNotes does not start a save while Obsidian closes.";
+				if (this.writeInFlight) tasks.addPromise(this.writeInFlight);
+			}),
+		);
 		plugin.set(this);
 		this.credentials = new CredentialRepository(this.app.secretStorage);
 		this.feedUrls = new FeedUrlRepository(this.app.secretStorage);
@@ -439,7 +458,24 @@ export default class PodNotes extends Plugin implements IPodNotes {
 
 	async loadSettings() {
 		try {
-			const decoded = decodePodNotesData(await this.loadData());
+			const data: unknown = await this.loadData();
+			// loadData resolves nullish both for a missing file and for one it could
+			// not read or parse, so tell them apart by whether the file exists.
+			if (data == null && (await this.app.vault.adapter.exists(this.dataFilePath()))) {
+				this.writeRefusal = "PodNotes does not save over a data.json it could not read.";
+				this.settings = structuredClone(DEFAULT_SETTINGS);
+				this.persistenceUnknownFields = {};
+				new Notice(
+					`PodNotes could not read ${this.dataFilePath()}, so it left the file unchanged and started with default settings. Nothing is saved until you restore or delete that file and restart Obsidian.`,
+					0,
+				);
+				console.error(
+					`PodNotes could not read ${this.dataFilePath()} and will not save over it`,
+				);
+				return;
+			}
+
+			const decoded = decodePodNotesData(data);
 			let settings = decoded.settings;
 			this.credentials ??= new CredentialRepository(this.app.secretStorage);
 
@@ -458,7 +494,7 @@ export default class PodNotes extends Plugin implements IPodNotes {
 							? this.credentials.storeValues(decoded.legacySecrets)
 							: {};
 					settings = { ...settings, ...references };
-					await this.saveData(encodePodNotesData(settings, decoded.unknownFields));
+					await this.writeDataFile(encodePodNotesData(settings, decoded.unknownFields));
 				} catch (error) {
 					throw new PodNotesSchemaMigrationError(
 						error,
@@ -491,7 +527,7 @@ export default class PodNotes extends Plugin implements IPodNotes {
 					feedMigration.replacements,
 				);
 				evictCachedFeedUrls([...feedMigration.replacements.keys()]);
-				await this.saveData(encodePodNotesData(settings, decoded.unknownFields));
+				await this.writeDataFile(encodePodNotesData(settings, decoded.unknownFields));
 				const count = feedMigration.replacements.size;
 				new Notice(
 					`Moved ${count} private feed ${count === 1 ? "URL" : "URLs"} into Obsidian SecretStorage.`,
@@ -539,6 +575,11 @@ export default class PodNotes extends Plugin implements IPodNotes {
 
 	private requestSettingsSave(): Promise<void> {
 		if (!this.isReady) return Promise.resolve();
+		if (this.writeRefusal) {
+			const refused = Promise.reject<void>(new Error(this.writeRefusal));
+			void refused.catch(() => undefined);
+			return refused;
+		}
 
 		try {
 			this.pendingSave = this.cloneSettings();
@@ -575,7 +616,7 @@ export default class PodNotes extends Plugin implements IPodNotes {
 				this.pendingSaveWaiters = [];
 
 				try {
-					await this.saveData(
+					await this.writeDataFile(
 						encodePodNotesData(snapshot, this.persistenceUnknownFields),
 					);
 					for (const waiter of waiters) waiter.resolve();
@@ -592,6 +633,22 @@ export default class PodNotes extends Plugin implements IPodNotes {
 		}
 	}
 
+	// Every data.json write goes through here, so no path can write while writes are refused.
+	private async writeDataFile(data: unknown): Promise<void> {
+		if (this.writeRefusal) throw new Error(this.writeRefusal);
+		const write = this.saveData(data);
+		this.writeInFlight = write.catch(() => undefined);
+		try {
+			await write;
+		} finally {
+			this.writeInFlight = null;
+		}
+	}
+
+	private dataFilePath(): string {
+		return normalizePath(`${this.manifest.dir}/data.json`);
+	}
+
 	private cloneSettings(): IPodNotesSettings {
 		// structuredClone is available in Obsidian's Electron runtime; fallback for safety.
 		if (typeof structuredClone === "function") {