Replace stale hidden-file conflict dialogues

src/features/HiddenFileCommon/JsonResolveModal.ts

@@ -43,11 +43,12 @@ export class JsonResolveModal extends Modal {
     }
 
     async UICallback(keepRev?: string, mergedStr?: string) {
-        if (this.callback) {
-            await this.callback(keepRev, mergedStr);
+        const callback = this.callback;
+        this.callback = undefined;
+        if (callback) {
+            await callback(keepRev, mergedStr);
         }
         this.close();
-        this.callback = undefined;
     }
 
     override onOpen() {
@@ -78,8 +79,10 @@ export class JsonResolveModal extends Modal {
         const { contentEl } = this;
         contentEl.empty();
         // contentEl.empty();
-        if (this.callback != undefined) {
-            void this.callback(undefined);
+        const callback = this.callback;
+        this.callback = undefined;
+        if (callback) {
+            void callback(undefined);
         }
         if (this.component != undefined) {
             void unmount(this.component);

src/features/HiddenFileCommon/JsonResolveModal.unit.spec.ts

@@ -0,0 +1,67 @@
+import { describe, expect, it, vi } from "vitest";
+import type { FilePath } from "@vrtmrz/livesync-commonlib/compat/common/types";
+import { sendSignal } from "@vrtmrz/livesync-commonlib/compat/common/utils";
+import { JsonResolveModal } from "./JsonResolveModal.ts";
+
+vi.mock("@/deps.ts", () => ({
+    App: class App {},
+    Modal: class Modal {
+        contentEl = { empty: vi.fn() };
+        close() {
+            (this as { onClose?: () => void }).onClose?.();
+        }
+    },
+}));
+vi.mock("./JsonResolvePane.svelte", () => ({ default: {} }));
+vi.mock("svelte", () => ({ mount: vi.fn(), unmount: vi.fn() }));
+
+describe("JsonResolveModal completion", () => {
+    it("applies a selection once when closing the dialogue", async () => {
+        const filename = ".obsidian/selected-once.json" as FilePath;
+        const callback = vi.fn(async () => {});
+        const modal = new JsonResolveModal({} as never, filename, [], callback);
+
+        await modal.UICallback("2-kept");
+
+        expect(callback).toHaveBeenCalledExactlyOnceWith("2-kept", undefined);
+        sendSignal(`cancel-internal-conflict:${filename}`);
+    });
+
+    it("ignores a late selection after cancellation", async () => {
+        const filename = ".obsidian/cancelled-once.json" as FilePath;
+        const callback = vi.fn(async () => {});
+        const modal = new JsonResolveModal({} as never, filename, [], callback);
+
+        sendSignal(`cancel-internal-conflict:${filename}`);
+        await vi.waitFor(() => expect(callback).toHaveBeenCalledOnce());
+        await modal.UICallback("2-stale");
+
+        expect(callback).toHaveBeenCalledExactlyOnceWith(undefined);
+    });
+
+    it("keeps an admitted write pending when cancellation closes its dialogue", async () => {
+        const filename = ".obsidian/writing-once.json" as FilePath;
+        let finish!: () => void;
+        const gate = new Promise<void>((resolve) => {
+            finish = resolve;
+        });
+        const callback = vi.fn(async () => await gate);
+        const modal = new JsonResolveModal({} as never, filename, [], callback);
+        const close = vi.spyOn(modal, "close");
+        let completed = false;
+        const selected = modal.UICallback("2-kept").then(() => {
+            completed = true;
+        });
+
+        try {
+            sendSignal(`cancel-internal-conflict:${filename}`);
+            await vi.waitFor(() => expect(close).toHaveBeenCalledOnce());
+
+            expect(callback).toHaveBeenCalledExactlyOnceWith("2-kept", undefined);
+            expect(completed).toBe(false);
+        } finally {
+            finish();
+            await selected;
+        }
+    });
+});

src/features/HiddenFileSync/CmdHiddenFileSync.queue.unit.spec.ts

@@ -18,6 +18,7 @@ import {
     type MetaEntry,
 } from "@vrtmrz/livesync-commonlib/compat/common/types";
 import { HiddenFileSync } from "./CmdHiddenFileSync";
+import { sendSignal, waitForSignal } from "@vrtmrz/livesync-commonlib/compat/common/utils";
 
 const queueOwners: HiddenFileSync[] = [];
 
@@ -133,6 +134,132 @@ describe("HiddenFileSync conflict queue", () => {
         }
     });
 
+    it.each(["prefixed", "unprefixed"])(
+        "cancels the older same-file dialogue and presents the latest revisions for a %s request",
+        async (form) => {
+            vi.useFakeTimers();
+            const filename = `.obsidian/replacement-${form}.json`;
+            const path = `i:${filename}` as FilePathWithPrefix;
+            const id = String(path) as DocumentID;
+            let generation = 2;
+            const localDatabase = {
+                getRaw: vi.fn(async (_id: DocumentID, options?: { revs_info?: boolean }) =>
+                    options?.revs_info
+                        ? {}
+                        : { _id: id, path, _rev: `${generation}-current`, _conflicts: [`${generation}-other`] }
+                ),
+                getDBEntry: vi.fn(
+                    async (_path: FilePathWithPrefix, options: { rev: string }) =>
+                        ({ _id: id, path, _rev: options.rev }) as LoadedEntry
+                ),
+                managers: { conflictManager: { mergeObject: vi.fn(async () => false) } },
+            };
+            const { hiddenFileSync } = createHiddenFileSync(localDatabase);
+            hiddenFileSync.path2id = async () => id;
+            const finishFirst = deferred<boolean>();
+            let cancelled = false;
+            const opened: string[][] = [];
+            hiddenFileSync.showJSONMergeDialogAndMerge = vi.fn(async (left, right) => {
+                opened.push([left._rev!, right._rev!]);
+                if (opened.length > 1) return false;
+                return await Promise.race([
+                    finishFirst.promise,
+                    waitForSignal(`cancel-internal-conflict:${filename}`).then(() => {
+                        cancelled = true;
+                        return false;
+                    }),
+                ]);
+            });
+
+            try {
+                hiddenFileSync.queueConflictCheck(path);
+                await vi.advanceTimersByTimeAsync(100);
+                expect(opened).toEqual([["2-current", "2-other"]]);
+
+                generation = 3;
+                hiddenFileSync.queueConflictCheck((form === "prefixed" ? path : filename) as FilePathWithPrefix);
+                await vi.advanceTimersByTimeAsync(100);
+
+                expect(cancelled).toBe(true);
+                expect(opened).toEqual([
+                    ["2-current", "2-other"],
+                    ["3-current", "3-other"],
+                ]);
+                expect(await hiddenFileSync.conflictResolutionProcessor.waitForAllProcessed()).toBe(true);
+                expect(hiddenFileSync.pendingConflictChecks.size).toBe(0);
+            } finally {
+                finishFirst.resolve(false);
+                sendSignal(`cancel-internal-conflict:${filename}`);
+                await vi.advanceTimersByTimeAsync(100);
+            }
+        }
+    );
+
+    it.each(["same", "different"])(
+        "closes only a dialogue for the %s file when a replicated resolution arrives",
+        async (target) => {
+            vi.useFakeTimers();
+            const filename = `.obsidian/received-${target}.json`;
+            const path = `i:${filename}` as FilePathWithPrefix;
+            const incomingPath = (target === "same" ? path : "i:.obsidian/other-resolved.json") as FilePathWithPrefix;
+            const id = String(path) as DocumentID;
+            let resolved = false;
+            const localDatabase = {
+                getRaw: vi.fn(async (_id: DocumentID, options?: { revs_info?: boolean }) =>
+                    options?.revs_info
+                        ? {}
+                        : {
+                              _id: id,
+                              path,
+                              _rev: resolved ? "3-resolved" : "2-current",
+                              _conflicts: resolved ? [] : ["2-other"],
+                          }
+                ),
+                getDBEntry: vi.fn(async () => ({ _id: id, path }) as LoadedEntry),
+                managers: { conflictManager: { mergeObject: vi.fn(async () => false) } },
+            };
+            const { hiddenFileSync } = createHiddenFileSync(localDatabase);
+            hiddenFileSync.path2id = async () => id;
+            hiddenFileSync.getPath = () => incomingPath;
+            hiddenFileSync.isTargetFile = vi.fn(async () => true);
+            hiddenFileSync.trackDatabaseFileModification = vi.fn(async () => true);
+            const finishFirst = deferred<boolean>();
+            let cancelled = false;
+            hiddenFileSync.showJSONMergeDialogAndMerge = vi.fn(
+                async () =>
+                    await Promise.race([
+                        finishFirst.promise,
+                        waitForSignal(`cancel-internal-conflict:${filename}`).then(() => {
+                            cancelled = true;
+                            return false;
+                        }),
+                    ])
+            );
+
+            try {
+                hiddenFileSync.queueConflictCheck(path);
+                await vi.advanceTimersByTimeAsync(100);
+                expect(hiddenFileSync.showJSONMergeDialogAndMerge).toHaveBeenCalledOnce();
+
+                resolved = target === "same";
+                const incoming = { _id: incomingPath, path: incomingPath, _rev: "3-received" } as LoadedEntry;
+                expect(await hiddenFileSync._anyProcessOptionalSyncFiles(incoming)).toBe(true);
+                await vi.advanceTimersByTimeAsync(100);
+
+                expect(cancelled).toBe(target === "same");
+                expect(hiddenFileSync.showJSONMergeDialogAndMerge).toHaveBeenCalledOnce();
+                expect(hiddenFileSync.pendingConflictChecks.size).toBe(target === "same" ? 0 : 1);
+                if (target === "same") {
+                    expect(await hiddenFileSync.conflictResolutionProcessor.waitForAllProcessed()).toBe(true);
+                }
+            } finally {
+                finishFirst.resolve(false);
+                sendSignal(`cancel-internal-conflict:${filename}`);
+                await vi.advanceTimersByTimeAsync(100);
+            }
+        }
+    );
+
     it("waits for a cancelled manual merge dialogue and then finishes without requeueing", async () => {
         vi.useFakeTimers();
         const path = "i:.obsidian/plugins/example/data.json" as FilePathWithPrefix;
@@ -225,7 +352,7 @@ describe("HiddenFileSync pipeline contracts", () => {
         gates.forEach((gate, index) => gate.resolve({ path: paths[index] } as MetaEntry));
         await vi.advanceTimersByTimeAsync(100);
         expect(await hiddenFileSync.conflictResolutionProcessor.waitForAllProcessed()).toBe(true);
-        expect(reads).toEqual(paths);
+        expect(reads).toEqual([...paths, paths[0]]);
         expect(hiddenFileSync.pendingConflictChecks.size).toBe(0);
     });
 
@@ -233,6 +360,8 @@ describe("HiddenFileSync pipeline contracts", () => {
         vi.useFakeTimers();
         const paths = Array.from({ length: 12 }, (_, index) => `i:.obsidian/${index}.json` as FilePathWithPrefix);
         const gates = paths.map(() => deferred<boolean>());
+        const firstDialogue = deferred<boolean>();
+        let firstCancelled = false;
         const localDatabase = {
             getRaw: vi.fn(async (id: DocumentID, options?: { revs_info?: boolean }) =>
                 options?.revs_info
@@ -252,6 +381,15 @@ describe("HiddenFileSync pipeline contracts", () => {
         const opened: FilePathWithPrefix[] = [];
         hiddenFileSync.showJSONMergeDialogAndMerge = vi.fn((doc) => {
             opened.push(doc.path);
+            if (opened.length === 1) {
+                return Promise.race([
+                    firstDialogue.promise,
+                    waitForSignal(`cancel-internal-conflict:${doc.path.slice(2)}`).then(() => {
+                        firstCancelled = true;
+                        return false;
+                    }),
+                ]);
+            }
             return gates[paths.indexOf(doc.path)].promise;
         });
         paths.forEach((path) => hiddenFileSync.queueConflictCheck(path));
@@ -260,12 +398,16 @@ describe("HiddenFileSync pipeline contracts", () => {
         expect(opened).toEqual([paths[0]]);
         paths.forEach((path) => hiddenFileSync.queueConflictCheck(path));
         expect(hiddenFileSync.pendingConflictChecks.size).toBe(12);
+        await vi.advanceTimersByTimeAsync(100);
+        expect(firstCancelled).toBe(true);
+        expect(opened).toEqual([paths[0], paths[0]]);
+        expect(localDatabase.managers.conflictManager.mergeObject).toHaveBeenCalledTimes(24);
         let completed = false;
         void hiddenFileSync.conflictResolutionProcessor.waitForAllProcessed().then(() => {
             completed = true;
         });
         for (let index = 0; index < paths.length; index++) {
-            expect(opened).toEqual(paths.slice(0, index + 1));
+            expect(opened).toEqual([paths[0], ...paths.slice(0, index + 1)]);
             expect(completed).toBe(false);
             gates[index].resolve(false);
             await vi.advanceTimersByTimeAsync(100);

src/features/HiddenFileSync/CmdHiddenFileSync.ts

@@ -741,21 +741,33 @@ Offline Changed files: ${processFiles.length}`;
 
     // --> Conflict processing
 
-    // Keep one in-flight conflict check per path so repeated sync events do not close the active merge dialogue.
-    pendingConflictChecks = new Set<FilePathWithPrefix>();
+    // Coalesce unstarted checks; retain a replacement once processing has begun.
+    pendingConflictChecks = new Map<FilePathWithPrefix, { started: boolean; recheck: boolean }>();
 
     queueConflictCheck(path: FilePathWithPrefix) {
-        if (this.pendingConflictChecks.has(path)) return;
-        this.pendingConflictChecks.add(path);
+        path = addPrefix(path, ICHeader);
+        const pending = this.pendingConflictChecks.get(path);
+        if (pending) {
+            if (pending.started) {
+                pending.recheck = true;
+                sendSignal(`cancel-internal-conflict:${stripAllPrefixes(path)}`);
+            }
+            return;
+        }
+        this.pendingConflictChecks.set(path, { started: false, recheck: false });
         if (!this.conflictResolutionProcessor.enqueue(path)) this.pendingConflictChecks.delete(path);
     }
 
     finishConflictCheck(path: FilePathWithPrefix) {
+        path = addPrefix(path, ICHeader);
+        const pending = this.pendingConflictChecks.get(path);
         this.pendingConflictChecks.delete(path);
+        if (pending?.recheck) this.queueConflictCheck(path);
     }
 
     requeueConflictCheck(path: FilePathWithPrefix) {
-        this.finishConflictCheck(path);
+        path = addPrefix(path, ICHeader);
+        this.pendingConflictChecks.delete(path);
         this.queueConflictCheck(path);
     }
 
@@ -806,10 +818,16 @@ Offline Changed files: ${processFiles.length}`;
     }
     conflictResolutionProcessor = new HiddenFileConflictQueue(
         async (path: FilePathWithPrefix) => {
+            const check = this.pendingConflictChecks.get(path);
+            if (check) check.started = true;
             try {
                 // Retrieve data
                 const id = await this.path2id(path, ICHeader);
                 const doc = await this.localDatabase.getRaw<MetaEntry>(id, { conflicts: true });
+                if (check?.recheck) {
+                    this.requeueConflictCheck(path);
+                    return [];
+                }
                 if (doc._conflicts === undefined) {
                     this.finishConflictCheck(path);
                     return [];
@@ -838,6 +856,10 @@ Offline Changed files: ${processFiles.length}`;
                         doc._rev,
                         conflictedRev
                     );
+                    if (check?.recheck) {
+                        this.requeueConflictCheck(path);
+                        return [];
+                    }
                     if (result) {
                         this._log(`Object merge:${path}`, LOG_LEVEL_INFO);
                         const filename = stripAllPrefixes(path);
@@ -861,7 +883,7 @@ Offline Changed files: ${processFiles.length}`;
                         await this.resolveByNewerEntry(id, path, doc, revA, revB);
                         return [];
                     }
-                    return [{ path, revA, revB, id, doc }];
+                    return [{ path, revA, revB, id, doc, check }];
                 }
                 // When not JSON file, resolve conflicts by choosing a newer one.
                 await this.resolveByNewerEntry(id, path, doc, revA, revB);
@@ -874,10 +896,18 @@ Offline Changed files: ${processFiles.length}`;
             }
         },
         async (result) => {
-            const { id, doc, path, revA, revB } = result;
+            const { id, doc, path, revA, revB, check } = result;
+            if (check?.recheck) {
+                this.requeueConflictCheck(path);
+                return;
+            }
             const prefixedPath = addPrefix(path, ICHeader);
             const docAMerge = await this.localDatabase.getDBEntry(prefixedPath, { rev: revA });
             const docBMerge = await this.localDatabase.getDBEntry(prefixedPath, { rev: revB });
+            if (check?.recheck) {
+                this.requeueConflictCheck(path);
+                return;
+            }
             try {
                 if (docAMerge != false && docBMerge != false) {
                     if (await this.showJSONMergeDialogAndMerge(docAMerge, docBMerge)) {
@@ -912,7 +942,7 @@ Offline Changed files: ${processFiles.length}`;
             const displayFilename = `${storeFilePath}`;
             // const path = this.prefixedConfigDir2configDir(stripAllPrefixes(docA.path)) || docA.path;
             // Cancel only when replacing an existing dialogue for the same path, not on every queue pass.
-            sendSignal(`cancel-internal-conflict:${docA.path}`);
+            sendSignal(`cancel-internal-conflict:${storageFilePath}`);
             const modal = new JsonResolveModal(this.app, storageFilePath, [docA, docB], async (keep, result) => {
                 // modal.close();
                 try {
@@ -998,6 +1028,9 @@ Offline Changed files: ${processFiles.length}`;
     async processReplicationResult(doc: LoadedEntry): Promise<boolean> {
         const info = this.getDocProps(doc);
         const path = info.path;
+        if (this.pendingConflictChecks.has(addPrefix(path, ICHeader))) {
+            this.queueConflictCheck(info.prefixedPath);
+        }
         const headerLine = `Tracking DB ${info.path} (${info.revDisplay}) :`;
         const ret = await this.trackDatabaseFileModification(path, headerLine);
         this._log(`${headerLine} Done: ${info.shortenedId})`, LOG_LEVEL_VERBOSE);