From 5d2608134cb058033fa8ee734133af3f1b3a4497 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Wed, 14 Oct 2026 20:57:57 +0200 Subject: [PATCH 1/2] test(sync): cover a synced data.json that Obsidian does not report as changed Obsidian calls onExternalSettingsChange only when data.json's modified time is later than the plugin's last saveData. A sync client that keeps the time the other device saved the file, or a write that rounds to the same millisecond as a save, is never reported, so a new device keeps its empty first-run library (#477). The e2e test moves a library over data.json with an earlier modified time, the way such a sync client does, and expects the view to show it. The unit tests take a change reported as a raw vault event once the reports settle, take a change that landed while PodNotes loaded, and read nothing after unload. The own-save e2e test now also counts writes, so a check of PodNotes' own save that writes again fails it. --- src/main.activateView.test.ts | 38 +++++++++++++++++++++++ src/persistence/writer.test.ts | 28 +++++++++++++++++ tests/e2e/sync-persistence.test.ts | 50 ++++++++++++++++++++++++++++-- 3 files changed, 114 insertions(+), 2 deletions(-) diff --git a/src/main.activateView.test.ts b/src/main.activateView.test.ts index 187820f3..414451da 100644 --- a/src/main.activateView.test.ts +++ b/src/main.activateView.test.ts @@ -495,6 +495,44 @@ describe("PodNotes onload wiring (#55)", () => { expect(hide).toHaveBeenCalledTimes(1); }); + const syncedLibrary = () => + encodePodNotesData({ + ...structuredClone(DEFAULT_SETTINGS), + savedFeeds: { + Synced: { title: "Synced", url: "https://example.com/synced.xml", artworkUrl: "" }, + }, + }); + const pastReportSettle = () => new Promise((resolve) => window.setTimeout(resolve, 100)); + + it("takes a data.json change Obsidian reports only as a raw vault event", async () => { + const { plugin } = await loadPlugin({ schemaVersion: 2 }); + const listeners = vi.mocked(plugin.app.vault.on).mock.calls as unknown as Array< + [string, (path: string) => void] + >; + const report = (path: string) => { + for (const [name, listener] of listeners) if (name === "raw") listener(path); + }; + await pastReportSettle(); + const loadData = vi.mocked(plugin.loadData); + loadData.mockResolvedValue(syncedLibrary()); + const reads = loadData.mock.calls.length; + + report(".obsidian/plugins/other-plugin/data.json"); + await pastReportSettle(); + expect(loadData).toHaveBeenCalledTimes(reads); + + report(".obsidian/plugins/podnotes/data.json"); + await vi.waitFor(() => expect(Object.keys(plugin.settings.savedFeeds)).toEqual(["Synced"])); + }); + + it("takes a data.json change that landed while it loaded", async () => { + const { plugin } = await loadPlugin({ schemaVersion: 2 }); + + vi.mocked(plugin.loadData).mockResolvedValue(syncedLibrary()); + + await vi.waitFor(() => expect(Object.keys(plugin.settings.savedFeeds)).toEqual(["Synced"])); + }); + const savedEpisode: Episode = { title: "Saved Episode", streamUrl: "https://example.com/saved.mp3", diff --git a/src/persistence/writer.test.ts b/src/persistence/writer.test.ts index 8eb4b2d5..16aa5b7a 100644 --- a/src/persistence/writer.test.ts +++ b/src/persistence/writer.test.ts @@ -492,6 +492,34 @@ describe("DataFileWriter changes made elsewhere", () => { expect(events.onRefusal.mock.calls).toEqual([[UNREADABLE]]); }); + it("reads the file once the changes reported to it settle", async () => { + const { access, clock, state, writer } = setup(); + state.disk!.savedFeeds.Synced = { title: "Synced" }; + + writer.report(); + writer.report(); + expect(access.read).not.toHaveBeenCalled(); + expect(clock.pending()).toBe(1); + clock.runAll(); + await writer.flush(); + + expect(access.read).toHaveBeenCalledTimes(1); + expect(state.memory.savedFeeds.Synced).toEqual({ title: "Synced" }); + }); + + it("reads nothing for a change reported before unload", async () => { + const { access, clock, state, writer } = setup(); + state.disk!.savedFeeds.Synced = { title: "Synced" }; + + writer.report(); + await writer.unload(); + writer.report(); + clock.runAll(); + await settled(); + + expect(access.read).not.toHaveBeenCalled(); + }); + it("adopts the file when it had nothing to merge against", async () => { const { access, events, state, writer } = setup(doc(), { base: null, refusal: STARTUP }); state.memory.defaultVolume = 0.3; diff --git a/tests/e2e/sync-persistence.test.ts b/tests/e2e/sync-persistence.test.ts index e4e9fc9b..cd5cd111 100644 --- a/tests/e2e/sync-persistence.test.ts +++ b/tests/e2e/sync-persistence.test.ts @@ -1,4 +1,4 @@ -import { readFile, rm, writeFile } from "node:fs/promises"; +import { readFile, rename, rm, utimes, writeFile } from "node:fs/promises"; import type { PluginHandle } from "obsidian-e2e"; import { describe, expect, onTestFinished, test } from "vitest"; import type { Episode } from "../../src/types/Episode"; @@ -19,7 +19,7 @@ import { click } from "./journey/input"; import { ui } from "./journey/targets"; import { isShown, listedEpisodes } from "./journey/wait"; import { type FixtureFeed, pubDate, startFixtureServer } from "./journey/fixtures"; -import { seedLibrary, waitForLibrary } from "./journey/library"; +import { recordWrites, seedLibrary, waitForLibrary } from "./journey/library"; import { jsLiteral, PLUGIN_ID, RELOAD_OPTIONS, WAIT_OPTS, waitForPodNotesReady } from "./runtime"; import { assertNoNewRuntimeErrors } from "./runtime-evidence"; @@ -212,6 +212,19 @@ describe("PodNotes data.json changed elsewhere", () => { test("does not treat its own saves as external changes", async () => { const { obsidian, plugin } = getContext(); + await plugin.updateDataAndReload((data) => { + addSyncedChanges(data); + data.queue = queueOf([ + { + title: "Queued Episode", + podcastName: "Synced Podcast", + streamUrl: "https://example.com/queued.mp3", + url: "https://example.com/queued", + description: "", + content: "", + }, + ]); + }, RELOAD_OPTIONS); // The harness rewrites data.json before each test, which is a change made // elsewhere. Let Obsidian report it before counting. await obsidian.sleep(2_500); @@ -225,6 +238,7 @@ describe("PodNotes data.json changed elsewhere", () => { }; return true; })()`); + const writes = await recordWrites(obsidian); await setVolume(obsidian, 0.55); await plugin.waitForData((data) => data.defaultVolume === 0.55, WAIT_OPTS); @@ -232,6 +246,12 @@ describe("PodNotes data.json changed elsewhere", () => { await obsidian.sleep(2_500); expect(await obsidian.dev.evalJson("window.__podnotesExternalChanges")).toBe(0); + expect(await writes.stop()).toMatchObject({ + attempted: 1, + completed: 1, + failed: 0, + intact: true, + }); }); test("saves an unsaved edit and keeps the change made elsewhere when disabled during their read", async () => { @@ -380,6 +400,32 @@ describe("PodNotes data.json changed elsewhere", () => { }); }); + test("shows a library synced in with the earlier modified time it had on the other device", async () => { + const { obsidian, plugin } = getContext(); + const dataPath = await plugin.dataPath(); + const synced = `${dataPath}.synced`; + onTestFinished(() => rm(synced, { force: true })); + const library = await plugin.data().read(); + addSyncedChanges(library); + await startAsNewDevice(plugin); + await setVolume(obsidian, 0.42); + await plugin.waitForData((data) => data.defaultVolume === 0.42, WAIT_OPTS); + + // A sync client that keeps modified times writes beside the file and moves it over. + const savedOnOtherDevice = new Date(Date.now() - 60_000); + await writeFile(synced, JSON.stringify(library)); + await utimes(synced, savedOnOtherDevice, savedOnOtherDevice); + await rename(synced, dataPath); + + const disk = await plugin.waitForData( + (data) => data.defaultVolume === 0.42 && Boolean(data.savedFeeds?.[SYNCED_FEED.title]), + WAIT_OPTS, + ); + expect(disk.playedEpisodes?.["Synced Podcast::Synced Episode"]).toEqual(SYNCED_PROGRESS); + await showLibrary(obsidian); + await expect.poll(() => isShown(obsidian, ui.podcastCard("Synced Feed")), POLL).toBe(true); + }); + test("shows both names when the library saved a podcast under an earlier name than a new device", async () => { const context = getContext(); const { obsidian, plugin } = context; From a58c9c6ced581862b22aeb2b883a61f1f4fb9afc Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Wed, 14 Oct 2026 20:57:57 +0200 Subject: [PATCH 2/2] fix(sync): check data.json on every change Obsidian sees to it PodNotes learned of a data.json replaced by sync only through onExternalSettingsChange, which Obsidian skips unless the file's modified time is later than the plugin's last save. A new device that saved its first-run library before sync finished never showed the synced library: with a sync client that keeps the other device's earlier modified time always, and in the e2e journey once in about 300 runs, when the test's write rounded to the save's millisecond (#477). PodNotes now listens to the vault's raw event for data.json, which Obsidian raises for every change, PodNotes' own writes included, and checks the file 50 ms after the changes stop, so a file written in place is read whole. The writer's existing comparison with what was last loaded or written decides whether anything changed, so an own save changes nothing. One check also runs after load, for a change that landed before the listener existed. A read that finds the file unchanged skips decoding it, so the extra check costs a parse and a comparison. onExternalSettingsChange stays: Obsidian refreshes an open settings tab once it resolves. --- src/global.d.ts | 6 +++++- src/main.ts | 15 +++++++++++++++ src/persistence/writer.ts | 17 +++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/global.d.ts b/src/global.d.ts index 1fa29721..119ef79b 100644 --- a/src/global.d.ts +++ b/src/global.d.ts @@ -1,4 +1,4 @@ -import type { App } from "obsidian"; +import type { App, EventRef, Vault } from "obsidian"; interface Manifest { author: string; @@ -34,6 +34,10 @@ interface App2 extends App { open(): void; openTabById(id: string): void; }; + vault: Vault & { + /** Not in the public types: any change Obsidian sees to a path, config files included. */ + on(name: "raw", callback: (path: string) => unknown): EventRef; + }; } export default App2; diff --git a/src/main.ts b/src/main.ts index d58d4ed8..e6d37114 100644 --- a/src/main.ts +++ b/src/main.ts @@ -154,6 +154,18 @@ export default class PodNotes extends Plugin implements IPodNotes { await this.loadSettings(); this.writer = this.createWriter(MAIN_WINDOW_CLOCK); + // Obsidian calls onExternalSettingsChange only when data.json's modified time is later + // than PodNotes' last save. A sync client can keep the other device's earlier time, and + // a write right after a save can round to the same millisecond, so the writer checks + // every change Obsidian sees to the file, and once for a change made while it loaded. + const dataFile = this.dataFilePath(); + const writer = this.writer; + this.registerEvent( + this.app.vault.on("raw", (path) => { + if (path === dataFile) writer.report(); + }), + ); + writer.report(); this.setCollectionStores(); // The media lives in the main window, so playback outlasts the view and popouts. @@ -478,6 +490,7 @@ export default class PodNotes extends Plugin implements IPodNotes { return this.writer?.flush() ?? Promise.resolve(); } + // Obsidian refreshes an open settings tab once this resolves. override async onExternalSettingsChange(): Promise { if (!this.writer) return; await this.writer.onExternalChange(); @@ -515,6 +528,8 @@ export default class PodNotes extends Plugin implements IPodNotes { return { read: async (base): Promise> => { const data: unknown = await this.loadData(); + if (base && jsonValuesEqual(data, base)) + return { kind: "file", doc: structuredClone(base) }; const path = this.dataFilePath(); const unreadable: ReadOutcome = { kind: "refused", diff --git a/src/persistence/writer.ts b/src/persistence/writer.ts index b125906b..c87a75b2 100644 --- a/src/persistence/writer.ts +++ b/src/persistence/writer.ts @@ -57,6 +57,9 @@ export interface WriterEvents { // Each save rewrites the whole file, and playback progress changes every few // seconds, so a progress checkpoint starts at least this long after the last save. const CHECKPOINT_INTERVAL_MS = 10_000; +// A program writing data.json in place empties the file first. Obsidian waits as +// long before its own check. +const REPORT_SETTLE_MS = 50; /** * Writes data.json without clobbering changes another device synced in. Every @@ -76,6 +79,8 @@ export class DataFileWriter { private chain: Promise | null = null; /** A checkpoint waiting for its interval. */ private timer: number | null = null; + /** A check of the file waiting for the changes reported to it to settle. */ + private reportTimer: number | null = null; /** A save is queued and has not started, so later changes are written by it. */ private saveQueued = false; private unloaded = false; @@ -153,10 +158,22 @@ export class DataFileWriter { }); } + /** data.json changed, maybe by PodNotes itself: check it once the changes stop. */ + report(): void { + if (this.unloaded) return; + if (this.reportTimer !== null) this.clock.clearTimeout(this.reportTimer); + this.reportTimer = this.clock.setTimeout(() => { + this.reportTimer = null; + void this.onExternalChange(); + }, REPORT_SETTLE_MS); + } + /** The next instance reads the file itself; the flushed save still merges with disk. */ unload(): Promise { this.memoryAtUnload = this.file.local(); this.unloaded = true; + if (this.reportTimer !== null) this.clock.clearTimeout(this.reportTimer); + this.reportTimer = null; return this.flush(); }