From 221f0b21541b6c961749867ec6f4a3d660926bd1 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sun, 11 Oct 2026 11:24:17 +0200 Subject: [PATCH 01/11] test(persistence): reproduce a fresh device's data.json wiping a synced library A device that starts with no data.json writes a defaults file at once and saves its own changes on top. When that file reaches a device holding the synced library, the library device adopts it whole, because its memory equals its base and the merge takes disk. In the other race order the new device adopts the library and drops what its user added. Also reproduces a rollback undone when an external change reports the file a failed write reached. --- src/main.persistence.test.ts | 126 +++++++++++++++++++++++++++++++++ src/persistence/writer.test.ts | 17 +++++ 2 files changed, 143 insertions(+) diff --git a/src/main.persistence.test.ts b/src/main.persistence.test.ts index 862d3074..b68e10a4 100644 --- a/src/main.persistence.test.ts +++ b/src/main.persistence.test.ts @@ -801,3 +801,129 @@ describe("PodNotes persistence integration", () => { expect(secondResolved).toBe(true); }); }); + +describe("a device that started before sync delivered its data.json", () => { + const feed = (title: string) => ({ + title, + url: `https://example.com/${title}.xml`, + artworkUrl: "", + }); + const library = JSON.parse( + JSON.stringify( + encodePodNotesData({ + ...structuredClone(DEFAULT_SETTINGS), + savedFeeds: { "Library Feed": feed("Library Feed") }, + playedEpisodes: { + "Library Feed::Episode": { + title: "Episode", + podcastName: "Library Feed", + time: 1234, + duration: 3600, + finished: false, + }, + }, + playlists: { + "Road Trip": { + icon: "list", + name: "Road Trip", + shouldEpisodeRemoveAfterPlay: false, + shouldRepeat: false, + episodes: [], + }, + }, + defaultPlaybackRate: 1.5, + }), + ), + ); + + /** PodNotes on its own vault, whose data.json a sync client replaces with another device's. */ + async function startDevice(data?: unknown) { + let disk = structuredClone(data); + const plugin = makePlugin(); + Object.assign(plugin, { + loadData: vi.fn(async () => structuredClone(disk)), + saveData: vi.fn(async (next: unknown) => { + disk = JSON.parse(JSON.stringify(next)); + }), + }); + Object.assign(plugin.app.vault.adapter, { + exists: vi.fn(async () => disk !== undefined), + read: vi.fn(async () => dataFileText(disk)), + }); + await plugin.loadSettings(); + attachDataFileWriter(plugin); + return { + plugin, + file: () => structuredClone(disk), + receive: async (file: unknown) => { + disk = structuredClone(file); + await plugin.onExternalSettingsChange(); + await plugin.flushSaves(); + }, + }; + } + + function expectBothLibraries(...devices: Array<{ plugin: PodNotes }>) { + for (const { plugin } of devices) { + expect(Object.keys(plugin.settings.savedFeeds).sort()).toEqual([ + "Library Feed", + "New Device Feed", + ]); + expect(plugin.settings.playedEpisodes["Library Feed::Episode"]?.time).toBe(1234); + expect(Object.keys(plugin.settings.playlists)).toEqual(["Road Trip"]); + expect(plugin.settings.defaultPlaybackRate).toBe(1.5); + } + } + + it("keeps both libraries when the new device's file reaches the library first", async () => { + const established = await startDevice(library); + const newDevice = await startDevice(); + newDevice.plugin.settings.savedFeeds = { "New Device Feed": feed("New Device Feed") }; + await newDevice.plugin.saveSettingsStrict(); + + await established.receive(newDevice.file()); + await newDevice.receive(established.file()); + + expectBothLibraries(established, newDevice); + expect(newDevice.file()).toEqual(established.file()); + }); + + it("keeps both libraries when the library reaches the new device first", async () => { + const established = await startDevice(library); + const newDevice = await startDevice(); + newDevice.plugin.settings.savedFeeds = { "New Device Feed": feed("New Device Feed") }; + await newDevice.plugin.saveSettingsStrict(); + + await newDevice.receive(established.file()); + await established.receive(newDevice.file()); + + expectBothLibraries(established, newDevice); + expect(newDevice.file()).toEqual(established.file()); + }); + + it("keeps the library when the file a new device wrote at install reaches it", async () => { + const established = await startDevice(library); + const newDevice = await startDevice(); + + await established.receive(newDevice.file()); + await newDevice.receive(established.file()); + + for (const { plugin } of [established, newDevice]) { + expect(Object.keys(plugin.settings.savedFeeds)).toEqual(["Library Feed"]); + expect(plugin.settings.playedEpisodes["Library Feed::Episode"]?.time).toBe(1234); + expect(plugin.settings.defaultPlaybackRate).toBe(1.5); + } + }); + + it("still takes a podcast removed on another device of the same library", async () => { + const established = await startDevice(library); + const newDevice = await startDevice(); + await newDevice.receive(established.file()); + + delete newDevice.plugin.settings.savedFeeds["Library Feed"]; + await newDevice.plugin.saveSettingsStrict(); + await established.receive(newDevice.file()); + + expect(established.plugin.settings.savedFeeds).toEqual({}); + }); +}); diff --git a/src/persistence/writer.test.ts b/src/persistence/writer.test.ts index b07e7975..b43f9208 100644 --- a/src/persistence/writer.test.ts +++ b/src/persistence/writer.test.ts @@ -249,6 +249,23 @@ describe("DataFileWriter saves", () => { expect(state.disk).toEqual(doc()); }); + it("keeps a rollback when the file a failed write reached is reported as changed", async () => { + const { access, state, writer } = setup(); + access.write.mockImplementationOnce(async (next: Doc) => { + state.disk = structuredClone(next); + throw new Error("could not verify"); + }); + state.memory.defaultVolume = 0.5; + await expect(writer.save()).rejects.toThrow("could not verify"); + + state.memory.defaultVolume = 1; + await writer.onExternalChange(); + await writer.save(); + + expect(state.memory).toEqual(doc()); + expect(state.disk).toEqual(doc()); + }); + it("keeps a change made elsewhere that a failed write carried to disk", async () => { const { access, state, writer } = setup(); access.write.mockImplementationOnce(async (next: Doc) => { From 8daf7a397a943a2a8a3e90a0528c3f0f4ba21f7c Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sun, 11 Oct 2026 11:26:28 +0200 Subject: [PATCH 02/11] test(e2e): reproduce a new device's data.json wiping the library in real Obsidian PodNotes started with no data.json writes the file a new device holds before the sync client delivers the library. Delivering it over a library loses the library; delivering the library over it loses the new device's change. --- tests/e2e/sync-persistence.test.ts | 56 +++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/tests/e2e/sync-persistence.test.ts b/tests/e2e/sync-persistence.test.ts index 6a8f544f..d28dd5bf 100644 --- a/tests/e2e/sync-persistence.test.ts +++ b/tests/e2e/sync-persistence.test.ts @@ -1,4 +1,4 @@ -import { readFile, writeFile } from "node:fs/promises"; +import { readFile, rm, writeFile } from "node:fs/promises"; import type { PluginHandle } from "obsidian-e2e"; import { describe, expect, test } from "vitest"; import type { Episode } from "../../src/types/Episode"; @@ -328,6 +328,60 @@ describe("PodNotes data.json changed elsewhere", () => { })()`); } }); + + // PodNotes started with no data.json writes one at once: the file a new device has + // before the sync client delivers the library. + async function startAsNewDevice(plugin: PluginHandle): Promise { + await plugin.data().patch((data) => data); + await rm(await plugin.dataPath()); + await plugin.reload(RELOAD_OPTIONS); + return plugin.data().read(); + } + + test("keeps its library when a new device's file replaces data.json", async () => { + const { obsidian, plugin } = getContext(); + const library = await plugin.data().read(); + addSyncedChanges(library); + const newDeviceFile = await startAsNewDevice(plugin); + await plugin.data().write(library); + await plugin.reload(RELOAD_OPTIONS); + + await plugin.data().write(newDeviceFile); + + const disk = await plugin.waitForData( + (data) => Boolean(data.savedFeeds?.[SYNCED_FEED.title]), + WAIT_OPTS, + ); + expect(disk.playedEpisodes?.["Synced Podcast::Synced Episode"]).toEqual(SYNCED_PROGRESS); + expect(await obsidian.dev.evalJson(MEMORY)).toMatchObject({ + feed: true, + playlist: true, + progress: 1234, + }); + }); + + test("keeps a new device's change when the library replaces its data.json", async () => { + const { obsidian, plugin } = getContext(); + 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); + + await plugin.data().write(library); + + 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); + expect(await obsidian.dev.evalJson(MEMORY)).toMatchObject({ + feed: true, + playlist: true, + progress: 1234, + volume: 0.42, + }); + }); }); describe("PodNotes queue identity", () => { From b31edb40070d0fdfa6f91908167c347939a42901 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sun, 11 Oct 2026 11:26:57 +0200 Subject: [PATCH 03/11] fix(persistence): keep a rollback when an external change reports the file a failed write reached A save settles a failed write whose content the file shows, so memory and base match what is on disk before it merges. A change reported elsewhere merged that file against the old base instead, so an edit made since the failed write, such as a failed import's rollback, was undone. --- src/persistence/writer.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/persistence/writer.ts b/src/persistence/writer.ts index 4b34b621..410a0f54 100644 --- a/src/persistence/writer.ts +++ b/src/persistence/writer.ts @@ -234,6 +234,9 @@ export class DataFileWriter { } // A missing file is not a request to reset everything. if (!disk || this.unloaded) return; + if (this.failedWrite && jsonValuesEqual(disk, this.failedWrite.written)) { + this.settle(this.failedWrite.local, this.failedWrite.written); + } const base = this.base; if (base && jsonValuesEqual(disk, base)) return; From ad6ed97e10c6046dab100c849d8bacb6dc76677f Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sun, 11 Oct 2026 12:16:46 +0200 Subject: [PATCH 04/11] fix(persistence): keep both libraries when a device that started before sync meets the synced data.json A device that starts with no data.json writes a library of its own. When that file reached a device holding the synced library, the merge read every difference from its base as an edit made elsewhere and took the file whole, so the library was gone on every device. In the other race order the new device dropped what its user had added. data.json now carries an optional root libraryId. A new install gets one that sorts by when the library started; a file without one belongs to the library every device shared before this change (""). A file whose libraryId differs from the base is another library: the two merge from the empty library, so every entry of both survives, and the merged file keeps the libraryId of the library started first, so its other devices go on merging three ways. Every decoded file carries a libraryId, so each save writes one and an Obsidian Sync key merge cannot pair one device's id with the other's content. An object both sides added, such as a playlist both libraries created under one name, now merges per key instead of taking one side whole. A device that could not read its file at startup starts a library of its own for the same reason. The test-only DataFileWriter.hasBase getter is gone. --- scripts/persistence-size.test.ts | 2 +- src/main.persistence.test.ts | 74 ++++++++++++++++- src/main.ts | 13 +-- src/persistence/merge.test.ts | 81 ++++++++++++++++++- src/persistence/merge.ts | 42 +++++++++- src/persistence/podNotesData.test.ts | 40 ++++++++- src/persistence/podNotesData.ts | 26 +++++- src/persistence/writer.test.ts | 4 +- src/persistence/writer.ts | 11 +-- .../PodNotesSettingsTab.import.test.ts | 6 +- 10 files changed, 266 insertions(+), 33 deletions(-) diff --git a/scripts/persistence-size.test.ts b/scripts/persistence-size.test.ts index fb0a6087..3aeffe4b 100644 --- a/scripts/persistence-size.test.ts +++ b/scripts/persistence-size.test.ts @@ -60,7 +60,7 @@ describe("persistence byte fixture", () => { expect(decoded.changed).toBe(false); expect( JSON.parse(JSON.stringify(encodePodNotesData(decoded.settings, decoded.unknownFields))), - ).toEqual(data); + ).toEqual({ ...data, libraryId: "" }); expect(Object.keys(data.savedFeeds)).toHaveLength(50); expect(Object.keys(data.playedEpisodes)).toHaveLength(2000); const downloads = Object.values(decoded.settings.downloadedEpisodes).flat(); diff --git a/src/main.persistence.test.ts b/src/main.persistence.test.ts index b68e10a4..082ef4d0 100644 --- a/src/main.persistence.test.ts +++ b/src/main.persistence.test.ts @@ -42,7 +42,7 @@ function makePlugin( credentials: new CredentialRepository(storage), isReady: true, settings: structuredClone(DEFAULT_SETTINGS), - persistenceUnknownFields: {}, + persistenceUnknownFields: { libraryId: "" }, loadData: vi.fn(async () => structuredClone(disk)), saveData: async (data: unknown) => { await saveData(data); @@ -926,4 +926,76 @@ describe("a device that started before sync delivered its data.json", () => { expect(established.plugin.settings.savedFeeds).toEqual({}); }); + + it("settles on one file when the two devices take each other's files at once", async () => { + const queued = (title: string) => ({ + title, + podcastName: "Pod", + streamUrl: `https://example.com/${title}.mp3`, + url: "", + description: "", + content: "", + }); + const established = await startDevice(library); + established.plugin.settings.queue.episodes = [queued("Library Episode")]; + await established.plugin.saveSettingsStrict(); + const newDevice = await startDevice(); + newDevice.plugin.settings.savedFeeds = { "New Device Feed": feed("New Device Feed") }; + newDevice.plugin.settings.queue.episodes = [queued("New Device Episode")]; + await newDevice.plugin.saveSettingsStrict(); + + const [libraryFile, newDeviceFile] = [established.file(), newDevice.file()]; + await established.receive(newDeviceFile); + await newDevice.receive(libraryFile); + await newDevice.receive(established.file()); + + expectBothLibraries(established, newDevice); + expect(newDevice.file()).toEqual(established.file()); + }); + + it("keeps the new device's podcast when Obsidian Sync key-merges the files on the library's device", async () => { + const established = await startDevice(library); + established.plugin.settings.defaultVolume = 0.5; + await established.plugin.saveSettingsStrict(); + const newDevice = await startDevice(); + newDevice.plugin.settings.savedFeeds = { "New Device Feed": feed("New Device Feed") }; + await newDevice.plugin.saveSettingsStrict(); + + const keyMerged = { ...(newDevice.file() as object), ...(established.file() as object) }; + await newDevice.receive(keyMerged); + await established.receive(keyMerged); + await established.receive(newDevice.file()); + await newDevice.receive(established.file()); + + expectBothLibraries(established, newDevice); + expect(newDevice.file()).toEqual(established.file()); + }); + + it("keeps the library when a device that could not read its file at start writes a new one", async () => { + const established = await startDevice(library); + const device = makePlugin(); + let text = '{"schemaVersion": 2, "savedFeeds": {"Library Fe'; + Object.assign(device, { + loadData: vi.fn(async () => (text.endsWith("}") ? JSON.parse(text) : undefined)), + saveData: vi.fn(async (next: unknown) => { + text = dataFileText(next); + }), + }); + Object.assign(device.app.vault.adapter, { + exists: vi.fn(async () => text !== ""), + read: vi.fn(async () => text), + }); + vi.spyOn(console, "error").mockImplementation(() => undefined); + await device.loadSettings(); + attachDataFileWriter(device); + + text = ""; + await device.onExternalSettingsChange(); + device.settings.defaultVolume = 0.5; + await device.saveSettingsStrict(); + await established.receive(JSON.parse(text)); + + expect(Object.keys(established.plugin.settings.savedFeeds)).toEqual(["Library Feed"]); + expect(established.plugin.settings.defaultVolume).toBe(0.5); + }); }); diff --git a/src/main.ts b/src/main.ts index 36155815..d2ab0544 100644 --- a/src/main.ts +++ b/src/main.ts @@ -38,6 +38,7 @@ import { encodePodNotesData, PODNOTES_DATA_SCHEMA_VERSION, PodNotesDataError, + toDocument, type PersistedPodNotesDataV2, } from "./persistence/podNotesData"; import { jsonValuesEqual, restoreDropped } from "./persistence/merge"; @@ -87,14 +88,6 @@ function holdsData(text: string, data: unknown): boolean { } } -/** data.json as plain JSON: what the merge compares and what saveData writes. */ -function toDocument( - settings: IPodNotesSettings, - unknownFields: Record, -): PersistedPodNotesDataV2 { - return JSON.parse(JSON.stringify(encodePodNotesData(settings, unknownFields))); -} - class PodNotesSchemaMigrationError extends Error { constructor( public readonly originalCause: unknown, @@ -496,7 +489,9 @@ export default class PodNotes extends Plugin implements IPodNotes { // not read or parse. if (data == null && !(await this.dataFileHoldsNothing())) { this.settings = structuredClone(DEFAULT_SETTINGS); - this.persistenceUnknownFields = {}; + // A new library: if the file goes away, this device's next save must not pass + // for a version of the one it could not read. + this.persistenceUnknownFields = decodePodNotesData(null).unknownFields; this.refuseSaves( "unreadable-data-file", `PodNotes could not read ${this.dataFilePath()}, so it left the file unchanged and started with default settings. Nothing is saved until the file can be read again, or until you delete it and restart Obsidian.`, diff --git a/src/persistence/merge.test.ts b/src/persistence/merge.test.ts index 6883225d..37929518 100644 --- a/src/persistence/merge.test.ts +++ b/src/persistence/merge.test.ts @@ -3,7 +3,7 @@ import { DEFAULT_SETTINGS } from "src/constants"; import type { PlayedEpisode } from "src/types/PlayedEpisode"; import type { PodcastFeed } from "src/types/PodcastFeed"; import type { PersistedEpisode, PersistedPlaylist } from "./episodeCodec"; -import { jsonValuesEqual, restoreDropped, threeWayMergeSettings } from "./merge"; +import { jsonValuesEqual, mergeDataFile, restoreDropped, threeWayMergeSettings } from "./merge"; import { encodePodNotesData, type PersistedPodNotesDataV2 } from "./podNotesData"; type Data = PersistedPodNotesDataV2; @@ -485,6 +485,85 @@ describe("threeWayMergeSettings", () => { }); }); +describe("mergeDataFile", () => { + const earlierId = "2026-10-11T09:00:00.000Z 0b9d"; + const laterId = "2026-10-11T10:00:00.000Z 41c7"; + + /** A library started on its own device: one podcast, its progress and a queued episode. */ + function library(podcastName: string, libraryId: string): Data { + const doc: Data = JSON.parse( + JSON.stringify(encodePodNotesData(structuredClone(DEFAULT_SETTINGS), { libraryId })), + ); + doc.savedFeeds = { [podcastName]: feed(podcastName) }; + doc.playedEpisodes = { + [`${podcastName}::Ep 1`]: { ...played("Ep 1", 10), podcastName }, + }; + doc.queue.episodes = [episode("Ep 1", podcastName)]; + return doc; + } + + it.each([ + ["a library started before ids", ""], + ["a library with an id", earlierId], + ])("takes a podcast another device of %s removed", (_, libraryId) => { + const base = { ...data(), libraryId }; + const disk = structuredClone(base); + delete disk.savedFeeds["Pod A"]; + + expect(mergeDataFile(base, structuredClone(base), disk).savedFeeds).toEqual({}); + }); + + it("keeps every entry of two libraries that meet, whichever device merges", () => { + const started = library("Pod A", ""); + started.defaultPlaybackRate = 1.5; + const joined = library("Pod B", earlierId); + + for (const [local, disk] of [ + [started, joined], + [joined, started], + ]) { + const merged = mergeDataFile(local, structuredClone(local), disk); + expect(Object.keys(merged.savedFeeds).sort()).toEqual(["Pod A", "Pod B"]); + expect(Object.keys(merged.playedEpisodes).sort()).toEqual([ + "Pod A::Ep 1", + "Pod B::Ep 1", + ]); + expect(merged.queue.episodes.map((entry) => entry.podcastName).sort()).toEqual([ + "Pod A", + "Pod B", + ]); + expect(merged.defaultPlaybackRate).toBe(1.5); + } + }); + + it("keeps the lineage of the library started first", () => { + const unmarked = library("Pod A", ""); + const earlier = library("Pod B", earlierId); + const later = library("Pod C", laterId); + + expect(mergeDataFile(unmarked, unmarked, earlier).libraryId).toBe(""); + expect(mergeDataFile(earlier, earlier, unmarked).libraryId).toBe(""); + expect(mergeDataFile(later, later, earlier).libraryId).toBe(earlierId); + expect(mergeDataFile(earlier, earlier, later).libraryId).toBe(earlierId); + // Memory took the unmarked library and has not saved it yet: base is the file it read. + expect(mergeDataFile(earlier, { ...earlier, libraryId: "" }, later).libraryId).toBe(""); + }); + + it("keeps the episodes of a playlist both libraries created under one name", () => { + const started = library("Pod A", ""); + const joined = library("Pod B", earlierId); + started.playlists = { "Road Trip": playlist("Road Trip", [episode("Ep 1", "Pod A")]) }; + joined.playlists = { "Road Trip": playlist("Road Trip", [episode("Ep 1", "Pod B")]) }; + + const merged = mergeDataFile(started, structuredClone(started), joined); + + expect(merged.playlists["Road Trip"].episodes.map((entry) => entry.podcastName)).toEqual([ + "Pod A", + "Pod B", + ]); + }); +}); + describe("restoreDropped", () => { it("puts back the entries the codec dropped and keeps every entry still there whole", () => { const base = data(); diff --git a/src/persistence/merge.ts b/src/persistence/merge.ts index b68a68e7..c0f90566 100644 --- a/src/persistence/merge.ts +++ b/src/persistence/merge.ts @@ -1,6 +1,8 @@ +import { DEFAULT_SETTINGS } from "src/constants"; import type { Episode } from "src/types/Episode"; import { getEpisodeKey } from "src/utility/episodeKey"; import { isPlainObject, type UnknownRecord } from "./codecUtils"; +import { toDocument } from "./podNotesData"; /** * Structural equality for JSON-like settings values. Key order does not matter; @@ -160,7 +162,7 @@ function threeWayMergeEpisodeList( * - If disk is unchanged from base, take local (preserves in-memory edits). * - If both changed a `currentEpisode`, a progress record, a saved feed or a note * mapping, take local whole. - * - If both changed the same plain object, recurse per key. + * - If both changed, or both added, the same plain object, recurse per key. * - Episode lists merge by episode key. * - Other irreducible array/leaf conflicts prefer local. */ @@ -175,11 +177,23 @@ export function threeWayMergeSettings( if (jsonValuesEqual(disk, base)) return structuredClone(local); if (isAtomicPath(path)) return mergeWhole(base, local, disk); - if (isPlainObject(base) && isPlainObject(local) && isPlainObject(disk)) { - const keys = new Set([...Object.keys(base), ...Object.keys(local), ...Object.keys(disk)]); + if ( + (base === undefined || isPlainObject(base)) && + isPlainObject(local) && + isPlainObject(disk) + ) { + const ancestor: UnknownRecord = base ?? {}; + const keys = new Set([ + ...Object.keys(ancestor), + ...Object.keys(local), + ...Object.keys(disk), + ]); const merged: UnknownRecord = {}; for (const key of keys) { - const value = threeWayMergeSettings(base[key], local[key], disk[key], [...path, key]); + const value = threeWayMergeSettings(ancestor[key], local[key], disk[key], [ + ...path, + key, + ]); // JSON has no undefined: a merged-away key must be absent, or the // written document never equals the same document read back. if (value !== undefined) merged[key] = value; @@ -202,6 +216,26 @@ export function threeWayMergeSettings( return structuredClone(local); } +/** + * Memory `local` merged with the file `disk`, given `base`, the file this device + * last loaded or wrote. A file whose `libraryId` differs from base's is another + * library, started on a device that never saw this one's file: the two merge from + * the empty library, so every entry of both survives, a preference one side left + * at its default takes the other's, and a real conflict keeps this device's. The + * result keeps the `libraryId` of the library started first, so that library's + * devices go on merging three ways. + */ +export function mergeDataFile(base: T, local: T, disk: T): T { + const lineage = (doc: T) => (doc as UnknownRecord).libraryId as string; + if (lineage(base) === lineage(disk)) return threeWayMergeSettings(base, local, disk); + + const empty = toDocument(structuredClone(DEFAULT_SETTINGS)) as T; + const merged = threeWayMergeSettings(empty, local, disk) as UnknownRecord; + // Memory, not base: memory may already hold a library that started earlier. + merged.libraryId = [lineage(local), lineage(disk)].sort()[0]; + return merged as T; +} + /** * `disk` with the entries the codec dropped while reading `raw` put back from * `base`: map entries, and episode list entries by episode key. A dropped entry diff --git a/src/persistence/podNotesData.test.ts b/src/persistence/podNotesData.test.ts index 52b811ed..71ccc191 100644 --- a/src/persistence/podNotesData.test.ts +++ b/src/persistence/podNotesData.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { DEFAULT_SETTINGS } from "src/constants"; import type DownloadedEpisode from "src/types/DownloadedEpisode"; import type { Episode } from "src/types/Episode"; @@ -79,6 +79,44 @@ describe("PodNotes data schema", () => { expect(second.settings.queue.episodes).toEqual([]); }); + it("gives each new install its own library id, sorting after earlier installs and every unmarked library", () => { + vi.spyOn(crypto, "randomUUID") + .mockReturnValueOnce("ffffffff-ffff-4fff-bfff-ffffffffffff") + .mockReturnValue("00000000-0000-4000-8000-000000000000"); + vi.setSystemTime(new Date("2026-10-11T09:00:00.000Z")); + const [earlier, sameInstant] = [decodePodNotesData(null), decodePodNotesData(undefined)]; + vi.setSystemTime(new Date("2026-10-11T10:00:00.000Z")); + const later = decodePodNotesData(null); + vi.useRealTimers(); + vi.restoreAllMocks(); + const [earlierId, sameInstantId, laterId] = [earlier, sameInstant, later].map( + ({ unknownFields }) => unknownFields.libraryId, + ); + + expect([earlierId, sameInstantId, laterId]).toEqual(Array(3).fill(expect.any(String))); + expect(sameInstantId).not.toBe(earlierId); + expect(["", earlierId, laterId].sort()).toEqual(["", earlierId, laterId]); + expect(encodePodNotesData(later.settings, later.unknownFields).libraryId).toBe(laterId); + }); + + it.each([ + ["names none", undefined, ""], + ["names one that is not text", 7, ""], + ["names one", "2026-10-11T09:00:00.000Z 0b9d", "2026-10-11T09:00:00.000Z 0b9d"], + ])( + "keeps the library id of a file that %s for the save to write back", + (_, libraryId, kept) => { + const file = JSON.parse( + JSON.stringify(encodePodNotesData(DEFAULT_SETTINGS, { libraryId })), + ); + const decoded = decodePodNotesData(file); + + expect(encodePodNotesData(decoded.settings, decoded.unknownFields).libraryId).toBe( + kept, + ); + }, + ); + it("round-trips every persisted episode container with canonical dates", () => { const settings = settingsWithEveryEpisodeContainer(); const persisted = encodePodNotesData(settings); diff --git a/src/persistence/podNotesData.ts b/src/persistence/podNotesData.ts index fa647d87..367e3fc0 100644 --- a/src/persistence/podNotesData.ts +++ b/src/persistence/podNotesData.ts @@ -42,7 +42,11 @@ export interface DecodedPodNotesData { legacySecrets: CredentialValues; /** Retired fields found in v2 must be scrubbed without importing their values. */ retiredPlaintextPresent: boolean; - /** Unknown supported-schema root fields retained so a normal save does not erase them. */ + /** + * Unknown supported-schema root fields retained so a normal save does not erase them. + * Also carries `libraryId`, the file's lineage: its string id, or "" for a library + * started before ids, which sorts before every id. + */ unknownFields: UnknownRecord; } @@ -72,8 +76,14 @@ export type PersistedPodNotesDataV2 = PersistedPodNotesSettings & * does not understand. */ export function decodePodNotesData(value: unknown): DecodedPodNotesData { - // No data.json yet is a new install, which starts from the current defaults. - if (value === null || value === undefined) value = structuredClone(DEFAULT_SETTINGS); + // No data.json yet is a new install, which starts from the current defaults as a + // new library. Ids sort by when the library started. + if (value === null || value === undefined) { + value = { + ...structuredClone(DEFAULT_SETTINGS), + libraryId: `${new Date().toISOString()} ${crypto.randomUUID()}`, + }; + } if (!isPlainObject(value)) { throw new PodNotesDataError( @@ -90,6 +100,8 @@ export function decodePodNotesData(value: unknown): DecodedPodNotesData { Object.prototype.hasOwnProperty.call(value, key), ); const unknownFields = copyUnknownRootFields(value); + const { libraryId } = unknownFields; + unknownFields.libraryId = typeof libraryId === "string" ? libraryId : ""; return { settings, @@ -133,6 +145,14 @@ export function encodePodNotesData( }; } +/** data.json as plain JSON: what the merge compares and what saveData writes. */ +export function toDocument( + settings: IPodNotesSettings, + unknownFields?: UnknownRecord, +): PersistedPodNotesDataV2 { + return JSON.parse(JSON.stringify(encodePodNotesData(settings, unknownFields))); +} + function readSchemaVersion(root: UnknownRecord): PodNotesDataSchemaVersion { if (!Object.prototype.hasOwnProperty.call(root, "schemaVersion")) return 0; diff --git a/src/persistence/writer.test.ts b/src/persistence/writer.test.ts index b43f9208..d28d6831 100644 --- a/src/persistence/writer.test.ts +++ b/src/persistence/writer.test.ts @@ -377,7 +377,6 @@ describe("DataFileWriter changes made elsewhere", () => { await writer.onExternalChange(); expect(state.memory.savedFeeds.Synced).toEqual({ title: "Synced" }); - expect(writer.hasBase).toBe(true); expect(clock.pending()).toBe(0); expect(state.writes).toHaveLength(0); }); @@ -432,7 +431,8 @@ describe("DataFileWriter changes made elsewhere", () => { await writer.onExternalChange(); expect(file.state.memory).toEqual(file.state.disk); - expect(writer.hasBase).toBe(true); + await writer.save(); + expect(file.access.write).not.toHaveBeenCalled(); }); }); diff --git a/src/persistence/writer.ts b/src/persistence/writer.ts index 410a0f54..0e4029bd 100644 --- a/src/persistence/writer.ts +++ b/src/persistence/writer.ts @@ -1,5 +1,5 @@ import { toError } from "src/utility/toError"; -import { jsonValuesEqual, threeWayMergeSettings } from "./merge"; +import { jsonValuesEqual, mergeDataFile, threeWayMergeSettings } from "./merge"; /** Timers of the main window. A popout's timers stop when the popout closes. */ export interface Clock { @@ -56,11 +56,6 @@ export class DataFileWriter { private base: Doc | null, ) {} - /** Whether a readable data.json has been loaded or written. */ - get hasBase(): boolean { - return this.base !== null; - } - /** * Save after any save already running; changes made until it starts share it. * Inside `checkpoint`, wait for the checkpoint interval instead. @@ -190,7 +185,7 @@ export class DataFileWriter { const local = this.file.local(); let toWrite = local; if (disk && base) { - toWrite = threeWayMergeSettings(base, local, disk); + toWrite = mergeDataFile(base, local, disk); } else if (!disk && this.failedWrite) { // A failed write that left nothing on disk held the changes made elsewhere: // write them again, with the edits made here since. @@ -242,7 +237,7 @@ export class DataFileWriter { if (base && jsonValuesEqual(disk, base)) return; const local = this.file.local(); - const merged = base ? threeWayMergeSettings(base, local, disk) : structuredClone(disk); + const merged = base ? mergeDataFile(base, local, disk) : structuredClone(disk); this.base = disk; if (!jsonValuesEqual(merged, local)) this.publish(merged); if (jsonValuesEqual(merged, disk)) this.cancelTimer(); diff --git a/src/ui/settings/PodNotesSettingsTab.import.test.ts b/src/ui/settings/PodNotesSettingsTab.import.test.ts index 154ea808..b511ea9e 100644 --- a/src/ui/settings/PodNotesSettingsTab.import.test.ts +++ b/src/ui/settings/PodNotesSettingsTab.import.test.ts @@ -418,7 +418,7 @@ describe("PodNotesSettingsTab settings import", () => { adoptReference: vi.fn(() => newerSecretId), }, isReady: true, - persistenceUnknownFields: {}, + persistenceUnknownFields: { libraryId: "" }, app: { vault: { adapter: { @@ -500,7 +500,7 @@ describe("PodNotesSettingsTab settings import", () => { settings: structuredClone(DEFAULT_SETTINGS), credentials: { storeValues: vi.fn(() => ({})) }, isReady: false, - persistenceUnknownFields: {}, + persistenceUnknownFields: { libraryId: "" }, app: { vault: { adapter: { @@ -584,7 +584,7 @@ describe("PodNotesSettingsTab settings import", () => { settings: structuredClone(disk), credentials: { storeValues: vi.fn(() => ({})) }, isReady: true, - persistenceUnknownFields: {}, + persistenceUnknownFields: { libraryId: "" }, app: { vault: { adapter: { From 5e6c3321f96f02132a48c615a869b59936ba9862 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sun, 11 Oct 2026 12:25:59 +0200 Subject: [PATCH 05/11] test(persistence): reproduce a removal undone by a failed write that was already settled A failed write that reached disk is settled when the file shows it, but stays recorded, so the next save settles the same snapshot again and brings back an entry removed since. --- src/persistence/writer.test.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/persistence/writer.test.ts b/src/persistence/writer.test.ts index d28d6831..11d81084 100644 --- a/src/persistence/writer.test.ts +++ b/src/persistence/writer.test.ts @@ -266,6 +266,24 @@ describe("DataFileWriter saves", () => { expect(state.disk).toEqual(doc()); }); + it("keeps a removal made after a change reported elsewhere showed a failed write reached disk", async () => { + const { access, state, writer } = setup(); + access.write.mockImplementationOnce(async (next: Doc) => { + state.disk = structuredClone(next); + throw new Error("could not verify"); + }); + state.memory.defaultVolume = 0.5; + state.disk!.savedFeeds.Synced = { title: "Synced" }; + await expect(writer.save()).rejects.toThrow("could not verify"); + + await writer.onExternalChange(); + delete state.memory.savedFeeds.Synced; + await writer.save(); + + expect(state.memory.savedFeeds.Synced).toBeUndefined(); + expect(state.disk!.savedFeeds.Synced).toBeUndefined(); + }); + it("keeps a change made elsewhere that a failed write carried to disk", async () => { const { access, state, writer } = setup(); access.write.mockImplementationOnce(async (next: Doc) => { From 23fae13b6f9aafd9506ca4cfecb0612aa3342982 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sun, 11 Oct 2026 12:26:24 +0200 Subject: [PATCH 06/11] fix(persistence): forget a failed write once the file shows it reached disk Settling a failed write whose content the file shows now also drops the record, in a save and in a change reported elsewhere alike. Kept, the record was settled again by the next save against memory from before the failure, which brought back an entry removed since. --- src/persistence/writer.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/persistence/writer.ts b/src/persistence/writer.ts index 0e4029bd..83deef31 100644 --- a/src/persistence/writer.ts +++ b/src/persistence/writer.ts @@ -177,9 +177,7 @@ export class DataFileWriter { this.lastSaveAt = this.clock.now(); const disk = await this.file.read(this.base); - if (this.failedWrite && disk && jsonValuesEqual(disk, this.failedWrite.written)) { - this.settle(this.failedWrite.local, this.failedWrite.written); - } + this.settleFailedWrite(disk); const base = this.base; // Captured after the read, so edits made while it was in flight are written too. const local = this.file.local(); @@ -218,6 +216,13 @@ export class DataFileWriter { ); } + /** A failed write the file shows on disk after all: memory and base take it, once. */ + private settleFailedWrite(disk: Doc | null): void { + if (!this.failedWrite || !disk || !jsonValuesEqual(disk, this.failedWrite.written)) return; + this.settle(this.failedWrite.local, this.failedWrite.written); + this.failedWrite = null; + } + private async applyExternalChange(): Promise { let disk: Doc | null; try { @@ -229,9 +234,7 @@ export class DataFileWriter { } // A missing file is not a request to reset everything. if (!disk || this.unloaded) return; - if (this.failedWrite && jsonValuesEqual(disk, this.failedWrite.written)) { - this.settle(this.failedWrite.local, this.failedWrite.written); - } + this.settleFailedWrite(disk); const base = this.base; if (base && jsonValuesEqual(disk, base)) return; From b17f5195f9a3d2897bf368aefd4e02c78c452951 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sun, 11 Oct 2026 12:40:42 +0200 Subject: [PATCH 07/11] fix(persistence): give every encoded data.json a libraryId The codec now writes the libraryId wherever it encodes a document, "" when the root fields have none, instead of relying on every caller to decode one first. A document built without them stays in its library, and the type says so. newLibraryId names the id a new library gets. The two-device tests assert that a device writes nothing once both agree, and the e2e helper checks that it produced a new device's file. --- src/main.persistence.test.ts | 22 ++++++++------- src/main.ts | 3 ++- src/persistence/merge.ts | 2 +- src/persistence/podNotesData.ts | 27 ++++++++++--------- .../PodNotesSettingsTab.import.test.ts | 6 ++--- tests/e2e/sync-persistence.test.ts | 6 ++++- tests/fixtures/persistence/large-v2.mjs | 3 ++- 7 files changed, 39 insertions(+), 30 deletions(-) diff --git a/src/main.persistence.test.ts b/src/main.persistence.test.ts index 082ef4d0..f24b344c 100644 --- a/src/main.persistence.test.ts +++ b/src/main.persistence.test.ts @@ -42,7 +42,7 @@ function makePlugin( credentials: new CredentialRepository(storage), isReady: true, settings: structuredClone(DEFAULT_SETTINGS), - persistenceUnknownFields: { libraryId: "" }, + persistenceUnknownFields: {}, loadData: vi.fn(async () => structuredClone(disk)), saveData: async (data: unknown) => { await saveData(data); @@ -840,12 +840,10 @@ describe("a device that started before sync delivered its data.json", () => { async function startDevice(data?: unknown) { let disk = structuredClone(data); const plugin = makePlugin(); - Object.assign(plugin, { - loadData: vi.fn(async () => structuredClone(disk)), - saveData: vi.fn(async (next: unknown) => { - disk = JSON.parse(JSON.stringify(next)); - }), + const saveData = vi.fn(async (next: unknown) => { + disk = JSON.parse(JSON.stringify(next)); }); + Object.assign(plugin, { loadData: vi.fn(async () => structuredClone(disk)), saveData }); Object.assign(plugin.app.vault.adapter, { exists: vi.fn(async () => disk !== undefined), read: vi.fn(async () => dataFileText(disk)), @@ -855,6 +853,7 @@ describe("a device that started before sync delivered its data.json", () => { return { plugin, file: () => structuredClone(disk), + writes: () => saveData.mock.calls.length, receive: async (file: unknown) => { disk = structuredClone(file); await plugin.onExternalSettingsChange(); @@ -885,7 +884,7 @@ describe("a device that started before sync delivered its data.json", () => { await newDevice.receive(established.file()); expectBothLibraries(established, newDevice); - expect(newDevice.file()).toEqual(established.file()); + expect(newDevice.plugin.settings).toEqual(established.plugin.settings); }); it("keeps both libraries when the library reaches the new device first", async () => { @@ -898,7 +897,7 @@ describe("a device that started before sync delivered its data.json", () => { await established.receive(newDevice.file()); expectBothLibraries(established, newDevice); - expect(newDevice.file()).toEqual(established.file()); + expect(newDevice.plugin.settings).toEqual(established.plugin.settings); }); it("keeps the library when the file a new device wrote at install reaches it", async () => { @@ -947,10 +946,13 @@ describe("a device that started before sync delivered its data.json", () => { const [libraryFile, newDeviceFile] = [established.file(), newDevice.file()]; await established.receive(newDeviceFile); await newDevice.receive(libraryFile); + const writes = [established.writes(), newDevice.writes()]; await newDevice.receive(established.file()); + await established.receive(newDevice.file()); expectBothLibraries(established, newDevice); - expect(newDevice.file()).toEqual(established.file()); + expect(newDevice.plugin.settings).toEqual(established.plugin.settings); + expect([established.writes(), newDevice.writes()]).toEqual(writes); }); it("keeps the new device's podcast when Obsidian Sync key-merges the files on the library's device", async () => { @@ -968,7 +970,7 @@ describe("a device that started before sync delivered its data.json", () => { await newDevice.receive(established.file()); expectBothLibraries(established, newDevice); - expect(newDevice.file()).toEqual(established.file()); + expect(newDevice.plugin.settings).toEqual(established.plugin.settings); }); it("keeps the library when a device that could not read its file at start writes a new one", async () => { diff --git a/src/main.ts b/src/main.ts index d2ab0544..31faa01f 100644 --- a/src/main.ts +++ b/src/main.ts @@ -37,6 +37,7 @@ import { decodePodNotesData, encodePodNotesData, PODNOTES_DATA_SCHEMA_VERSION, + newLibraryId, PodNotesDataError, toDocument, type PersistedPodNotesDataV2, @@ -491,7 +492,7 @@ export default class PodNotes extends Plugin implements IPodNotes { this.settings = structuredClone(DEFAULT_SETTINGS); // A new library: if the file goes away, this device's next save must not pass // for a version of the one it could not read. - this.persistenceUnknownFields = decodePodNotesData(null).unknownFields; + this.persistenceUnknownFields = { libraryId: newLibraryId() }; this.refuseSaves( "unreadable-data-file", `PodNotes could not read ${this.dataFilePath()}, so it left the file unchanged and started with default settings. Nothing is saved until the file can be read again, or until you delete it and restart Obsidian.`, diff --git a/src/persistence/merge.ts b/src/persistence/merge.ts index c0f90566..bd0d7fb7 100644 --- a/src/persistence/merge.ts +++ b/src/persistence/merge.ts @@ -229,7 +229,7 @@ export function mergeDataFile(base: T, local: T, disk: T): T { const lineage = (doc: T) => (doc as UnknownRecord).libraryId as string; if (lineage(base) === lineage(disk)) return threeWayMergeSettings(base, local, disk); - const empty = toDocument(structuredClone(DEFAULT_SETTINGS)) as T; + const empty = toDocument(DEFAULT_SETTINGS) as T; const merged = threeWayMergeSettings(empty, local, disk) as UnknownRecord; // Memory, not base: memory may already hold a library that started earlier. merged.libraryId = [lineage(local), lineage(disk)].sort()[0]; diff --git a/src/persistence/podNotesData.ts b/src/persistence/podNotesData.ts index 367e3fc0..e77bccf2 100644 --- a/src/persistence/podNotesData.ts +++ b/src/persistence/podNotesData.ts @@ -42,11 +42,7 @@ export interface DecodedPodNotesData { legacySecrets: CredentialValues; /** Retired fields found in v2 must be scrubbed without importing their values. */ retiredPlaintextPresent: boolean; - /** - * Unknown supported-schema root fields retained so a normal save does not erase them. - * Also carries `libraryId`, the file's lineage: its string id, or "" for a library - * started before ids, which sorts before every id. - */ + /** Unknown supported-schema root fields retained so a normal save does not erase them. */ unknownFields: UnknownRecord; } @@ -65,8 +61,18 @@ export type PersistedPodNotesSettings = Omit< export type PersistedPodNotesDataV2 = PersistedPodNotesSettings & UnknownRecord & { schemaVersion: typeof PODNOTES_DATA_SCHEMA_VERSION; + /** + * The library this file belongs to: the id the device that started it from nothing + * gave it, or "" for a library started before ids, which sorts before every id. + */ + libraryId: string; }; +/** A new library's `libraryId`. Ids sort by when their library started. */ +export function newLibraryId(): string { + return `${new Date().toISOString()} ${crypto.randomUUID()}`; +} + /** * Decode and validate plugin data before any store sees it. * @@ -76,13 +82,9 @@ export type PersistedPodNotesDataV2 = PersistedPodNotesSettings & * does not understand. */ export function decodePodNotesData(value: unknown): DecodedPodNotesData { - // No data.json yet is a new install, which starts from the current defaults as a - // new library. Ids sort by when the library started. + // No data.json yet is a new install: a new library, from the current defaults. if (value === null || value === undefined) { - value = { - ...structuredClone(DEFAULT_SETTINGS), - libraryId: `${new Date().toISOString()} ${crypto.randomUUID()}`, - }; + value = { ...structuredClone(DEFAULT_SETTINGS), libraryId: newLibraryId() }; } if (!isPlainObject(value)) { @@ -100,8 +102,6 @@ export function decodePodNotesData(value: unknown): DecodedPodNotesData { Object.prototype.hasOwnProperty.call(value, key), ); const unknownFields = copyUnknownRootFields(value); - const { libraryId } = unknownFields; - unknownFields.libraryId = typeof libraryId === "string" ? libraryId : ""; return { settings, @@ -132,6 +132,7 @@ export function encodePodNotesData( ...copyUnknownRootFields(unknownFields), ...validated, schemaVersion: PODNOTES_DATA_SCHEMA_VERSION, + libraryId: typeof unknownFields.libraryId === "string" ? unknownFields.libraryId : "", currentEpisode: validated.currentEpisode ? encodeEpisode(validated.currentEpisode) : undefined, diff --git a/src/ui/settings/PodNotesSettingsTab.import.test.ts b/src/ui/settings/PodNotesSettingsTab.import.test.ts index b511ea9e..154ea808 100644 --- a/src/ui/settings/PodNotesSettingsTab.import.test.ts +++ b/src/ui/settings/PodNotesSettingsTab.import.test.ts @@ -418,7 +418,7 @@ describe("PodNotesSettingsTab settings import", () => { adoptReference: vi.fn(() => newerSecretId), }, isReady: true, - persistenceUnknownFields: { libraryId: "" }, + persistenceUnknownFields: {}, app: { vault: { adapter: { @@ -500,7 +500,7 @@ describe("PodNotesSettingsTab settings import", () => { settings: structuredClone(DEFAULT_SETTINGS), credentials: { storeValues: vi.fn(() => ({})) }, isReady: false, - persistenceUnknownFields: { libraryId: "" }, + persistenceUnknownFields: {}, app: { vault: { adapter: { @@ -584,7 +584,7 @@ describe("PodNotesSettingsTab settings import", () => { settings: structuredClone(disk), credentials: { storeValues: vi.fn(() => ({})) }, isReady: true, - persistenceUnknownFields: { libraryId: "" }, + persistenceUnknownFields: {}, app: { vault: { adapter: { diff --git a/tests/e2e/sync-persistence.test.ts b/tests/e2e/sync-persistence.test.ts index d28dd5bf..cd22d8cd 100644 --- a/tests/e2e/sync-persistence.test.ts +++ b/tests/e2e/sync-persistence.test.ts @@ -332,10 +332,14 @@ describe("PodNotes data.json changed elsewhere", () => { // PodNotes started with no data.json writes one at once: the file a new device has // before the sync client delivers the library. async function startAsNewDevice(plugin: PluginHandle): Promise { + // A write through the harness first, so it restores this file after the test. await plugin.data().patch((data) => data); await rm(await plugin.dataPath()); await plugin.reload(RELOAD_OPTIONS); - return plugin.data().read(); + const file = await plugin.data().read(); + expect(file.savedFeeds).toEqual({}); + expect(file.libraryId).toMatch(/^\d{4}-/); + return file; } test("keeps its library when a new device's file replaces data.json", async () => { diff --git a/tests/fixtures/persistence/large-v2.mjs b/tests/fixtures/persistence/large-v2.mjs index 347385e9..a6d27649 100644 --- a/tests/fixtures/persistence/large-v2.mjs +++ b/tests/fixtures/persistence/large-v2.mjs @@ -6,8 +6,9 @@ import config from "../../../obsidian-e2e.config.mjs"; export function createLargeV2Fixture(includeGuid = false) { /** @type {{feed: import("../../../src/types/PodcastFeed").PodcastFeed, episodes: SavedEpisode[], guids: string[]}} */ const sample = JSON.parse(readFileSync("tests/fixtures/persistence/feed-sample.json", "utf8")); + // A file a released version wrote: it has no libraryId. const data = - /** @type {import("../../../src/persistence/podNotesData").PersistedPodNotesDataV2} */ ({ + /** @type {import("../../../src/persistence/podNotesData").PersistedPodNotesSettings & { schemaVersion: 2 }} */ ({ ...structuredClone(config.defaultData), schemaVersion: 2, }); From 6840719efb34d39cc75aa3f698981c391d95da2b Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sun, 11 Oct 2026 12:52:19 +0200 Subject: [PATCH 08/11] test(persistence): tighten the library id tests The codec cases build the file a released version wrote, without a key or with one that is not text, instead of letting the encoder fill it in first. The crossed-delivery test drops a delivery of a device's own file. The encoder notes why libraryId is not a known key. --- src/main.persistence.test.ts | 1 - src/persistence/podNotesData.test.ts | 2 +- src/persistence/podNotesData.ts | 2 ++ 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main.persistence.test.ts b/src/main.persistence.test.ts index f24b344c..3afc68bf 100644 --- a/src/main.persistence.test.ts +++ b/src/main.persistence.test.ts @@ -948,7 +948,6 @@ describe("a device that started before sync delivered its data.json", () => { await newDevice.receive(libraryFile); const writes = [established.writes(), newDevice.writes()]; await newDevice.receive(established.file()); - await established.receive(newDevice.file()); expectBothLibraries(established, newDevice); expect(newDevice.plugin.settings).toEqual(established.plugin.settings); diff --git a/src/persistence/podNotesData.test.ts b/src/persistence/podNotesData.test.ts index 71ccc191..38606445 100644 --- a/src/persistence/podNotesData.test.ts +++ b/src/persistence/podNotesData.test.ts @@ -107,7 +107,7 @@ describe("PodNotes data schema", () => { "keeps the library id of a file that %s for the save to write back", (_, libraryId, kept) => { const file = JSON.parse( - JSON.stringify(encodePodNotesData(DEFAULT_SETTINGS, { libraryId })), + JSON.stringify({ ...encodePodNotesData(DEFAULT_SETTINGS), libraryId }), ); const decoded = decodePodNotesData(file); diff --git a/src/persistence/podNotesData.ts b/src/persistence/podNotesData.ts index e77bccf2..1e319725 100644 --- a/src/persistence/podNotesData.ts +++ b/src/persistence/podNotesData.ts @@ -132,6 +132,8 @@ export function encodePodNotesData( ...copyUnknownRootFields(unknownFields), ...validated, schemaVersion: PODNOTES_DATA_SCHEMA_VERSION, + // Not a setting: it travels with the root fields that are not settings, so it is + // not a known key. libraryId: typeof unknownFields.libraryId === "string" ? unknownFields.libraryId : "", currentEpisode: validated.currentEpisode ? encodeEpisode(validated.currentEpisode) From 5c042cab34225ff97b615d16f4706a74a6f307e0 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sun, 11 Oct 2026 12:53:04 +0200 Subject: [PATCH 09/11] refactor(persistence): say what the libraryId comments mean plainly --- src/persistence/podNotesData.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/persistence/podNotesData.ts b/src/persistence/podNotesData.ts index 1e319725..22cfb92e 100644 --- a/src/persistence/podNotesData.ts +++ b/src/persistence/podNotesData.ts @@ -62,8 +62,8 @@ export type PersistedPodNotesDataV2 = PersistedPodNotesSettings & UnknownRecord & { schemaVersion: typeof PODNOTES_DATA_SCHEMA_VERSION; /** - * The library this file belongs to: the id the device that started it from nothing - * gave it, or "" for a library started before ids, which sorts before every id. + * The library this file belongs to: an id given when a device started it from + * nothing, or "" for a library started before ids, which sorts before every id. */ libraryId: string; }; @@ -132,8 +132,7 @@ export function encodePodNotesData( ...copyUnknownRootFields(unknownFields), ...validated, schemaVersion: PODNOTES_DATA_SCHEMA_VERSION, - // Not a setting: it travels with the root fields that are not settings, so it is - // not a known key. + // Read from the unknown root fields, so it must stay out of KNOWN_TOP_LEVEL_KEYS. libraryId: typeof unknownFields.libraryId === "string" ? unknownFields.libraryId : "", currentEpisode: validated.currentEpisode ? encodeEpisode(validated.currentEpisode) From 04d46dc272aaba06c957a5a7e1ef4be3ed0eafe5 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sun, 11 Oct 2026 12:58:32 +0200 Subject: [PATCH 10/11] refactor(persistence): drop comments the code can say, and name what it relies on mergeDataFile names the empty library it merges two libraries from and keeps the library that started first; the library from before ids is a named constant. The test that memory, not base, decides which library a merge keeps stands on its own. --- src/main.persistence.test.ts | 1 - src/main.ts | 2 -- src/persistence/merge.test.ts | 12 +++++++++--- src/persistence/merge.ts | 26 ++++++++++--------------- src/persistence/podNotesData.ts | 15 ++++++-------- src/persistence/writer.ts | 1 - tests/e2e/sync-persistence.test.ts | 2 -- tests/fixtures/persistence/large-v2.mjs | 1 - 8 files changed, 25 insertions(+), 35 deletions(-) diff --git a/src/main.persistence.test.ts b/src/main.persistence.test.ts index 3afc68bf..ad79197d 100644 --- a/src/main.persistence.test.ts +++ b/src/main.persistence.test.ts @@ -836,7 +836,6 @@ describe("a device that started before sync delivered its data.json", () => { ), ); - /** PodNotes on its own vault, whose data.json a sync client replaces with another device's. */ async function startDevice(data?: unknown) { let disk = structuredClone(data); const plugin = makePlugin(); diff --git a/src/main.ts b/src/main.ts index 31faa01f..15628f17 100644 --- a/src/main.ts +++ b/src/main.ts @@ -490,8 +490,6 @@ export default class PodNotes extends Plugin implements IPodNotes { // not read or parse. if (data == null && !(await this.dataFileHoldsNothing())) { this.settings = structuredClone(DEFAULT_SETTINGS); - // A new library: if the file goes away, this device's next save must not pass - // for a version of the one it could not read. this.persistenceUnknownFields = { libraryId: newLibraryId() }; this.refuseSaves( "unreadable-data-file", diff --git a/src/persistence/merge.test.ts b/src/persistence/merge.test.ts index 37929518..b7ca6b8a 100644 --- a/src/persistence/merge.test.ts +++ b/src/persistence/merge.test.ts @@ -489,7 +489,6 @@ describe("mergeDataFile", () => { const earlierId = "2026-10-11T09:00:00.000Z 0b9d"; const laterId = "2026-10-11T10:00:00.000Z 41c7"; - /** A library started on its own device: one podcast, its progress and a queued episode. */ function library(podcastName: string, libraryId: string): Data { const doc: Data = JSON.parse( JSON.stringify(encodePodNotesData(structuredClone(DEFAULT_SETTINGS), { libraryId })), @@ -545,8 +544,15 @@ describe("mergeDataFile", () => { expect(mergeDataFile(earlier, earlier, unmarked).libraryId).toBe(""); expect(mergeDataFile(later, later, earlier).libraryId).toBe(earlierId); expect(mergeDataFile(earlier, earlier, later).libraryId).toBe(earlierId); - // Memory took the unmarked library and has not saved it yet: base is the file it read. - expect(mergeDataFile(earlier, { ...earlier, libraryId: "" }, later).libraryId).toBe(""); + }); + + it("keeps the earlier library memory joined but has not saved yet when another arrives", () => { + const earlier = library("Pod B", earlierId); + const joinedUnmarked = { ...earlier, libraryId: "" }; + + expect(mergeDataFile(earlier, joinedUnmarked, library("Pod C", laterId)).libraryId).toBe( + "", + ); }); it("keeps the episodes of a playlist both libraries created under one name", () => { diff --git a/src/persistence/merge.ts b/src/persistence/merge.ts index bd0d7fb7..200b9949 100644 --- a/src/persistence/merge.ts +++ b/src/persistence/merge.ts @@ -162,7 +162,7 @@ function threeWayMergeEpisodeList( * - If disk is unchanged from base, take local (preserves in-memory edits). * - If both changed a `currentEpisode`, a progress record, a saved feed or a note * mapping, take local whole. - * - If both changed, or both added, the same plain object, recurse per key. + * - If both changed the same plain object, recurse per key. * - Episode lists merge by episode key. * - Other irreducible array/leaf conflicts prefer local. */ @@ -216,26 +216,20 @@ export function threeWayMergeSettings( return structuredClone(local); } -/** - * Memory `local` merged with the file `disk`, given `base`, the file this device - * last loaded or wrote. A file whose `libraryId` differs from base's is another - * library, started on a device that never saw this one's file: the two merge from - * the empty library, so every entry of both survives, a preference one side left - * at its default takes the other's, and a real conflict keeps this device's. The - * result keeps the `libraryId` of the library started first, so that library's - * devices go on merging three ways. - */ export function mergeDataFile(base: T, local: T, disk: T): T { - const lineage = (doc: T) => (doc as UnknownRecord).libraryId as string; - if (lineage(base) === lineage(disk)) return threeWayMergeSettings(base, local, disk); + const libraryOf = (doc: T) => (doc as UnknownRecord).libraryId as string; + if (libraryOf(base) === libraryOf(disk)) return threeWayMergeSettings(base, local, disk); - const empty = toDocument(DEFAULT_SETTINGS) as T; - const merged = threeWayMergeSettings(empty, local, disk) as UnknownRecord; - // Memory, not base: memory may already hold a library that started earlier. - merged.libraryId = [lineage(local), lineage(disk)].sort()[0]; + const emptyLibrary = toDocument(DEFAULT_SETTINGS) as T; + const merged = threeWayMergeSettings(emptyLibrary, local, disk) as UnknownRecord; + merged.libraryId = startedFirst(libraryOf(local), libraryOf(disk)); return merged as T; } +function startedFirst(library: string, other: string): string { + return library < other ? library : other; +} + /** * `disk` with the entries the codec dropped while reading `raw` put back from * `base`: map entries, and episode list entries by episode key. A dropped entry diff --git a/src/persistence/podNotesData.ts b/src/persistence/podNotesData.ts index 22cfb92e..bca9b335 100644 --- a/src/persistence/podNotesData.ts +++ b/src/persistence/podNotesData.ts @@ -61,14 +61,11 @@ export type PersistedPodNotesSettings = Omit< export type PersistedPodNotesDataV2 = PersistedPodNotesSettings & UnknownRecord & { schemaVersion: typeof PODNOTES_DATA_SCHEMA_VERSION; - /** - * The library this file belongs to: an id given when a device started it from - * nothing, or "" for a library started before ids, which sorts before every id. - */ libraryId: string; }; -/** A new library's `libraryId`. Ids sort by when their library started. */ +const LIBRARY_BEFORE_IDS = ""; + export function newLibraryId(): string { return `${new Date().toISOString()} ${crypto.randomUUID()}`; } @@ -82,7 +79,6 @@ export function newLibraryId(): string { * does not understand. */ export function decodePodNotesData(value: unknown): DecodedPodNotesData { - // No data.json yet is a new install: a new library, from the current defaults. if (value === null || value === undefined) { value = { ...structuredClone(DEFAULT_SETTINGS), libraryId: newLibraryId() }; } @@ -132,8 +128,10 @@ export function encodePodNotesData( ...copyUnknownRootFields(unknownFields), ...validated, schemaVersion: PODNOTES_DATA_SCHEMA_VERSION, - // Read from the unknown root fields, so it must stay out of KNOWN_TOP_LEVEL_KEYS. - libraryId: typeof unknownFields.libraryId === "string" ? unknownFields.libraryId : "", + libraryId: + typeof unknownFields.libraryId === "string" + ? unknownFields.libraryId + : LIBRARY_BEFORE_IDS, currentEpisode: validated.currentEpisode ? encodeEpisode(validated.currentEpisode) : undefined, @@ -147,7 +145,6 @@ export function encodePodNotesData( }; } -/** data.json as plain JSON: what the merge compares and what saveData writes. */ export function toDocument( settings: IPodNotesSettings, unknownFields?: UnknownRecord, diff --git a/src/persistence/writer.ts b/src/persistence/writer.ts index 83deef31..934fbd62 100644 --- a/src/persistence/writer.ts +++ b/src/persistence/writer.ts @@ -216,7 +216,6 @@ export class DataFileWriter { ); } - /** A failed write the file shows on disk after all: memory and base take it, once. */ private settleFailedWrite(disk: Doc | null): void { if (!this.failedWrite || !disk || !jsonValuesEqual(disk, this.failedWrite.written)) return; this.settle(this.failedWrite.local, this.failedWrite.written); diff --git a/tests/e2e/sync-persistence.test.ts b/tests/e2e/sync-persistence.test.ts index cd22d8cd..c8cb1b93 100644 --- a/tests/e2e/sync-persistence.test.ts +++ b/tests/e2e/sync-persistence.test.ts @@ -329,8 +329,6 @@ describe("PodNotes data.json changed elsewhere", () => { } }); - // PodNotes started with no data.json writes one at once: the file a new device has - // before the sync client delivers the library. async function startAsNewDevice(plugin: PluginHandle): Promise { // A write through the harness first, so it restores this file after the test. await plugin.data().patch((data) => data); diff --git a/tests/fixtures/persistence/large-v2.mjs b/tests/fixtures/persistence/large-v2.mjs index a6d27649..99b1f927 100644 --- a/tests/fixtures/persistence/large-v2.mjs +++ b/tests/fixtures/persistence/large-v2.mjs @@ -6,7 +6,6 @@ import config from "../../../obsidian-e2e.config.mjs"; export function createLargeV2Fixture(includeGuid = false) { /** @type {{feed: import("../../../src/types/PodcastFeed").PodcastFeed, episodes: SavedEpisode[], guids: string[]}} */ const sample = JSON.parse(readFileSync("tests/fixtures/persistence/feed-sample.json", "utf8")); - // A file a released version wrote: it has no libraryId. const data = /** @type {import("../../../src/persistence/podNotesData").PersistedPodNotesSettings & { schemaVersion: 2 }} */ ({ ...structuredClone(config.defaultData), From aca91f30aa1136ad4bb0b5f69fb42c669235a15d Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sun, 11 Oct 2026 13:55:58 +0200 Subject: [PATCH 11/11] test(e2e): show both names of a podcast two libraries saved under different titles The library saved a podcast under an earlier title and a new device saved its current title, both with the same feed URL. After the two libraries merge, both tiles render, each opens the feed's episodes, Settings opens, and no error is thrown. --- tests/e2e/sync-persistence.test.ts | 64 +++++++++++++++++++++++++++++- 1 file changed, 63 insertions(+), 1 deletion(-) diff --git a/tests/e2e/sync-persistence.test.ts b/tests/e2e/sync-persistence.test.ts index c8cb1b93..92b28308 100644 --- a/tests/e2e/sync-persistence.test.ts +++ b/tests/e2e/sync-persistence.test.ts @@ -1,6 +1,6 @@ import { readFile, rm, writeFile } from "node:fs/promises"; import type { PluginHandle } from "obsidian-e2e"; -import { describe, expect, test } from "vitest"; +import { describe, expect, onTestFinished, test } from "vitest"; import type { Episode } from "../../src/types/Episode"; import type { LocalEpisode } from "../../src/types/LocalEpisode"; import type { PodcastFeed } from "../../src/types/PodcastFeed"; @@ -18,6 +18,17 @@ import { waitForPlaybackState, waitForPodNotesReady, } from "./harness"; +import { + assertNoRuntimeErrors, + click, + closeSettings, + listedEpisodes, + openPodNotesSettings, + showPodNotes, + ui, +} from "./journey/actions"; +import { type FixtureFeed, pubDate, startFixtureServer } from "./journey/fixtures"; +import { seedLibrary, waitForLibrary } from "./journey/library"; // Obsidian calls Plugin#onExternalSettingsChange when data.json is newer than // the plugin's last write: Obsidian Sync, iCloud, another instance on the vault, @@ -384,6 +395,57 @@ describe("PodNotes data.json changed elsewhere", () => { volume: 0.42, }); }); + + 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; + const current: FixtureFeed = { + title: "Current Show Name", + slug: "renamed-show", + artwork: true, + episodes: [{ title: "Renamed Show Episode", seconds: 6, pubDate: pubDate(0) }], + }; + const earlier: FixtureFeed = { ...current, title: "Earlier Show Name" }; + const server = await startFixtureServer([current]); + onTestFinished(() => server.stop()); + onTestFinished(() => closeSettings(obsidian)); + await seedLibrary(context, server, { subscriptions: [earlier] }); + const library = await plugin.data().read(); + const newDeviceFile = await startAsNewDevice(plugin); + newDeviceFile.savedFeeds = { + [current.title]: { + title: current.title, + url: server.feedUrl(current), + artworkUrl: server.artworkUrl(current), + }, + }; + await plugin.data().write(library); + await plugin.reload(RELOAD_OPTIONS); + + await plugin.data().write(newDeviceFile); + + const merged = await waitForLibrary( + plugin, + (saved) => saved.subscriptions.length === 2, + "both names of the podcast saved", + ); + expect(merged.subscriptions.map((feed) => feed.url)).toEqual([ + server.feedUrl(current), + server.feedUrl(current), + ]); + await showPodNotes(obsidian); + for (const title of [earlier.title, current.title]) { + await click(obsidian, ui.podcastGridTab); + await click(obsidian, ui.podcastCard(title)); + await obsidian.waitFor(async () => (await listedEpisodes(obsidian)).length > 0, { + timeoutMs: 15_000, + message: `the episodes of ${title}`, + }); + expect(await listedEpisodes(obsidian)).toEqual(["Renamed Show Episode"]); + } + await openPodNotesSettings(obsidian); + await assertNoRuntimeErrors(obsidian); + }); }); describe("PodNotes queue identity", () => {