Fix mirror restore deletion semantics

README.md

@@ -76,7 +76,8 @@ The restore dialog shows backup history as a searchable file tree.
 - `Restore Mode` controls how existing local files are handled:
   - `Only new`: restore only files that do not exist locally, or files whose backup revision is newer than the local file.
   - `All`: restore selected files even when local files already exist.
-  - `All and delete extra`: restore selected files and include deletion records in the confirmation. Deleting local files from those records is not implemented yet.
+  - `All and delete extra`: restore selected files and remove local files represented by the selected deletion records. The confirmation lists both operations, and deletion starts only after the selected files have been restored successfully.
+    The confirmation reflects the paths and operations planned when it opened. DiffZip does not revalidate deletion candidates that change while the dialogue remains open; cancel and reopen the restore dialogue before proceeding if the Vault may have changed during review.
 - `Additional prefix` restores files under an extra path prefix, such as `restored/`.
   The `Restore folder` setting is used by the legacy restore commands; the current revision selector uses this prefix field instead.
 
@@ -110,7 +111,7 @@ Legacy command meanings:
 | `Legacy: Restore from backups (previous behaviour)` | Use the older prompt-based restore flow instead of the current revision selector. |
 | `Legacy: Restore from backups per folder` | Use the older folder-oriented restore flow. |
 | `Legacy: Fetch all new files from the backups` | Restore files from backup history when the local file is missing or older than the backup revision. Existing local files that are newer or identical are left alone. |
-| `Legacy: ⚠ Restore Vault from backups and delete with deletion` | Restore the vault from backup history and include deletion records in the confirmation. Deleting local files from those records is not implemented yet. |
+| `Legacy: ⚠ Restore Vault from backups and delete with deletion` | Restore the vault from backup history, then remove local files represented by applicable deletion records. The confirmation lists both operations before they begin. |
 | `Legacy: Selective Sync Remote Backup` | Open the older command entry for the current `Sync Remote Backup` workflow. |
 
 ## Settings

docs/devs.md

@@ -46,7 +46,7 @@ const action = await ui.confirmAction(
         labels: { restore: "Restore", cancel: "Cancel" },
         defaultAction: "cancel",
     },
-    "restore-files",
+    "restore-files"
 );
 ```
 
@@ -68,6 +68,10 @@ Real Modal rendering and dismissal use the local-only Obsidian harness documente
 
 ## Screen wake lock
 
-The plug-in owns one lifecycle-aware screen wake-lock manager. Differential backups, archive restore, and the Fetch and Send phases of selective sync use its closure-based runner, so normal completion, cancellation, and errors release their logical lease automatically. Overlapping and nested operations share the platform wake lock. Confirmation dialogues do not acquire a lease; restore protection starts only when archive input and Vault output begin.
+The plug-in owns one lifecycle-aware screen wake-lock manager. Differential backups, archive restore, and the Fetch and Send phases of selective sync use its closure-based runner, so normal completion, cancellation, and errors release their logical lease automatically. Overlapping and nested operations share the platform wake lock. Restore planning and execution use separate leases; confirmation dialogues do not acquire one.
 
 The Screen Wake Lock API is best effort. Consumer workflows must continue when it is unavailable or rejected, and must not rely on it for background execution. Dispose the manager when the plug-in unloads. Keep the manager injectable through the focused helper in `src/wakeLock.ts`; App-free tests use that boundary instead of constructing the Obsidian plug-in.
+
+## Restore confirmation boundary
+
+Restore confirmation deliberately presents the paths and operations planned at a point in time; it is not a transactional lock on the Vault. DiffZip does not revalidate deletion candidates after opening the confirmation dialogue. A caller that permits concurrent Vault changes must cancel the operation and prepare a new restore plan when the reviewed state may no longer be current.

main.ts

@@ -443,8 +443,9 @@ export default class DiffZipBackupPlugin extends Plugin {
             } while (hasNext);
         }
         if (files.length == 0) {
-            this.logMessage("Archived ZIP files were not found!");
-            return;
+            const message = `Archived ZIP files were not found: ${zipFile}`;
+            this.logMessage(message);
+            throw new Error(message);
         }
         const restored = [] as string[];
 
@@ -463,7 +464,9 @@ export default class DiffZipBackupPlugin extends Plugin {
                     const files = restored.slice(-5).join("\n");
                     this.logMessage(`${restored.length} files have been restored! \n${files}\n...`, "proc-zip-extract");
                 } else {
-                    this.logMessage(`Creating or Overwriting ${file} has been failed!`);
+                    const message = `Creating or overwriting ${file} failed`;
+                    this.logMessage(message);
+                    throw new Error(message);
                 }
             }
         );
@@ -473,15 +476,30 @@ export default class DiffZipBackupPlugin extends Plugin {
             this.logMessage(`Processing ${file}...`, "proc-zip-export-processing");
             const binary = await this.backups.readBinary(file);
             if (binary == null || binary === false) {
-                this.logMessage(`Could not read ${file}`);
-                return;
+                const message = `Could not read ${file}`;
+                this.logMessage(message);
+                throw new Error(message);
             }
             const chunks = pieces(new Uint8Array(binary), size);
             for (const chunk of chunks) {
                 extractor.addZippedContent(chunk);
             }
         }
         await extractor.finalise();
+        const expectedRestorePaths = hasMultipleSupplied
+            ? extractFiles.map((file) => `${restorePrefix}${file}`)
+            : [restoreAs ?? extractFiles];
+        const missingRestorePaths = expectedRestorePaths.filter((file) => !restored.includes(file));
+        if (missingRestorePaths.length > 0) {
+            const preview = missingRestorePaths.slice(0, 5).join(", ");
+            const remaining = missingRestorePaths.length - 5;
+            const noun = missingRestorePaths.length === 1 ? "file" : "files";
+            const message = `The archive did not restore ${missingRestorePaths.length} requested ${noun}: ${preview}${
+                remaining > 0 ? `, and ${remaining} more` : ""
+            }`;
+            this.logMessage(message);
+            throw new Error(message);
+        }
     }
 
     async selectAndRestore() {
@@ -618,6 +636,10 @@ export default class DiffZipBackupPlugin extends Plugin {
             }
             const zipMap = new Map<string, string[]>();
             for (const [filename, fileInfo] of fileMap) {
+                if (fileInfo.missing) {
+                    this.logWrite(`${filename}: is a deletion record. Skipping on non-destructive restoration`);
+                    continue;
+                }
                 const path = fileInfo.zipName;
                 const arr = zipMap.get(path) ?? [];
                 arr.push(filename);
@@ -629,6 +651,10 @@ export default class DiffZipBackupPlugin extends Plugin {
             // 	fileMap.set(path, zipName);
             // }
             const zipList = [...zipMap.entries()].sort((a, b) => a[0].localeCompare(b[0]));
+            if (zipList.length == 0) {
+                this.logMessage(`Nothing to restore`);
+                return;
+            }
             const filesCount = zipList.reduce((a, b) => a + b[1].length, 0);
             if (
                 (await askSelectString(
@@ -715,6 +741,28 @@ export default class DiffZipBackupPlugin extends Plugin {
         deleteMissing: boolean = false,
         fileFilter: Record<string, number> | undefined = undefined,
         prefix: string = ""
+    ): Promise<void> {
+        const { deletingFiles, processFileCount, zipFileMap } = await this.runWhileAwake("archive-restore", () =>
+            this.planVaultRestore(onlyNew, deleteMissing, fileFilter, prefix)
+        );
+        if (processFileCount == 0 && deletingFiles.length == 0) {
+            this.logMessage(`Nothing to restore`);
+            return;
+        }
+        if (
+            !(await confirmRestore(this.ui, { processFileCount, filesByZip: zipFileMap, deleteMissing, deletingFiles }))
+        ) {
+            this.logMessage(`Cancelled`);
+            return;
+        }
+        await this.runWhileAwake("archive-restore", () => this.executeVaultRestore(zipFileMap, deletingFiles, prefix));
+    }
+
+    private async planVaultRestore(
+        onlyNew: boolean,
+        deleteMissing: boolean,
+        fileFilter: Record<string, number> | undefined,
+        prefix: string
     ) {
         this.logMessage(`Checking backup information...`);
         const files = await this.loadTOC();
@@ -749,10 +797,20 @@ export default class DiffZipBackupPlugin extends Plugin {
             }
             history.sort((a, b) => new Date(b.modified).getTime() - new Date(a.modified).getTime());
             const latest = history[0];
+            const selectedRevisionMissing = latest.missing === true;
             const zipName = latest.zipName;
             const localFileName = this.vaultAccess.normalizePath(`${prefix}${filename}`);
             const localStat = await this.vaultAccess.stat(localFileName);
             if (localStat) {
+                if (selectedRevisionMissing) {
+                    if (!deleteMissing) {
+                        this.logWrite(`${filename}: is marked as missing, but existing in the vault. Skipping...`);
+                    } else {
+                        this.logWrite(`${filename}: is marked as missing. It will be deleted...`);
+                        deletingFiles.push(localFileName);
+                    }
+                    continue;
+                }
                 const content = await this.vaultAccess.readBinary(localFileName);
                 if (!content) {
                     this.logWrite(`${filename}: has been failed to read`);
@@ -763,24 +821,14 @@ export default class DiffZipBackupPlugin extends Plugin {
                     this.logWrite(`${filename}: is as same as the backup. Skipping...`);
                     continue;
                 }
-                if (fileInfo.missing) {
-                    if (!deleteMissing) {
-                        this.logWrite(`${filename}: is marked as missing, but existing in the vault. Skipping...`);
-                        continue;
-                    } else {
-                        // this.logWrite(`${filename}: is marked as missing. Deleting...`);
-                        deletingFiles.push(filename);
-                        //TODO: Delete the file
-                    }
-                }
                 const localMtime = localStat.mtime;
                 const remoteMtime = new Date(latest.modified).getTime();
                 if (onlyNew && localMtime >= remoteMtime) {
                     this.logWrite(`${filename}: Ours is newer than the backup. Skipping...`);
                     continue;
                 }
             } else {
-                if (fileInfo.missing) {
+                if (selectedRevisionMissing) {
                     this.logWrite(`${filename}: is missing and not found in the vault. Skipping...`);
                     continue;
                 }
@@ -794,21 +842,24 @@ export default class DiffZipBackupPlugin extends Plugin {
 
             // latestZipMap.set(filename, zipName);
         }
-        if (processFileCount == 0 && deletingFiles.length == 0) {
-            this.logMessage(`Nothing to restore`);
-            return;
-        }
-        if (
-            !(await confirmRestore(this.ui, { processFileCount, filesByZip: zipFileMap, deleteMissing, deletingFiles }))
-        ) {
-            this.logMessage(`Cancelled`);
-            return;
-        }
+        return { deletingFiles, processFileCount, zipFileMap };
+    }
+
+    private async executeVaultRestore(
+        zipFileMap: ReadonlyMap<string, string[]>,
+        deletingFiles: readonly string[],
+        prefix: string
+    ): Promise<void> {
         for (const [zipName, files] of zipFileMap) {
             this.logMessage(`Extracting ${zipName}...`);
-            await this.extract(zipName, files, undefined, prefix);
+            await this.extractWithoutWakeLock(zipName, files, undefined, prefix);
+        }
+        for (const filename of deletingFiles) {
+            this.logWrite(`${filename}: deleting from the vault...`);
+            if (!(await this.vaultAccess.deleteBinary(filename))) {
+                throw new Error(`Failed to delete ${filename} from the vault`);
+            }
         }
-        // console.dir(zipFileMap);
     }
     async onload() {
         this.register(() => {

package.json

@@ -12,6 +12,8 @@
         "test:ui:watch": "vitest --config vitest.config.ts",
         "check:e2e:obsidian": "tsc -p test/e2e-obsidian/tsconfig.json",
         "test:e2e:obsidian:restore-confirmation": "npm run build && tsx test/e2e-obsidian/restore-confirmation.mts",
+        "test:e2e:obsidian:legacy-folder-restore": "npm run build && tsx test/e2e-obsidian/legacy-folder-restore.mts",
+        "test:e2e:obsidian:mirror-delete-semantics": "npm run build && tsx test/e2e-obsidian/mirror-delete-semantics.mts",
         "test:e2e:obsidian:wake-lock": "npm run build && tsx test/e2e-obsidian/wake-lock.mts",
         "version": "node version-bump.mjs && git add manifest.json versions.json",
         "pretty": "npm run prettyNoWrite -- --write --log-level error",

src/RestoreView.ts

@@ -33,12 +33,12 @@ export class RestoreDialog extends Modal {
                 onApply: async (
                     selectedRevisions: Record<string, number>,
                     mode: "new" | "all" | "all-delete",
-                    prefix: string,
+                    prefix: string
                 ) => {
                     this.close();
                     const onlyNew = mode === "new";
-                    const skipDeleted = mode !== "all-delete";
-                    await this.plugin.restoreVault(onlyNew, skipDeleted, selectedRevisions, prefix);
+                    const deleteMissing = mode === "all-delete";
+                    await this.plugin.restoreVault(onlyNew, deleteMissing, selectedRevisions, prefix);
                 },
             },
         });

src/restoreConfirmation.test.ts

@@ -0,0 +1,35 @@
+import type { ConfirmActionOptions, UiInteractions } from "@vrtmrz/obsidian-plugin-kit/ui";
+import { confirmRestore, RESTORE_CONFIRMATION_INTERACTION_ID } from "./restoreConfirmation.ts";
+
+declare const Deno: {
+    test: (name: string, fn: () => void | Promise<void>) => void;
+};
+
+function assertEquals<T>(actual: T, expected: T, message: string): void {
+    if (actual !== expected) {
+        throw new Error(`${message}: expected=${String(expected)}, actual=${String(actual)}`);
+    }
+}
+
+Deno.test("restore confirmation: identifies a destructive restore in its title and action", async () => {
+    let request: { interactionId?: string; options: ConfirmActionOptions<string> } | undefined;
+    const ui = {
+        confirmAction: async (options: ConfirmActionOptions<string>, interactionId?: string) => {
+            request = { interactionId, options };
+            return "cancel";
+        },
+    } as UiInteractions;
+
+    await confirmRestore(ui, {
+        processFileCount: 1,
+        filesByZip: new Map([["backup.zip", ["restored.md"]]]),
+        deleteMissing: true,
+        deletingFiles: ["deleted.md"],
+    });
+
+    if (!request) throw new Error("The confirmation interaction was not requested");
+    assertEquals(request.interactionId, RESTORE_CONFIRMATION_INTERACTION_ID, "interaction ID");
+    assertEquals(request.options.title, "Restore and Delete Confirmation", "destructive confirmation title");
+    assertEquals(request.options.labels?.restore, "Restore and delete", "destructive confirmation action");
+    assertEquals(request.options.defaultAction, "cancel", "safe default action");
+});

src/restoreConfirmation.ts

@@ -18,7 +18,7 @@ export interface RestoreConfirmationOptions {
 /** Requests confirmation for a planned restore through the injected UI capability. */
 export async function confirmRestore(
     ui: UiInteractions,
-    { processFileCount, filesByZip, deleteMissing, deletingFiles }: RestoreConfirmationOptions,
+    { processFileCount, filesByZip, deleteMissing, deletingFiles }: RestoreConfirmationOptions
 ): Promise<boolean> {
     const detailFiles = `<details>
 
@@ -34,25 +34,25 @@ ${[...filesByZip.entries()]
 ${deletingFiles.map((file) => `- ${file}`).join("\n")}
 
 </details>`;
-    const deleteMessage =
-        deleteMissing && deletingFiles.length > 0
-            ? `And ${deletingFiles.length} files will be deleted.\n${detailDeletedFiles}\n`
-            : "";
+    const isDestructive = deleteMissing && deletingFiles.length > 0;
+    const deleteMessage = isDestructive
+        ? `And ${deletingFiles.length} files will be deleted.\n${detailDeletedFiles}\n`
+        : "";
     const message = `We have ${processFileCount} files to restore on ${filesByZip.size} ZIPs. \n${detailFiles}\n${deleteMessage}Are you sure to proceed?`;
 
     const action = await ui.confirmAction(
         {
-            title: "Restore Confirmation",
+            title: isDestructive ? "Restore and Delete Confirmation" : "Restore Confirmation",
             message,
             actions: ["restore", "cancel"] as const,
             labels: {
-                restore: "Yes, restore them!",
+                restore: isDestructive ? "Restore and delete" : "Yes, restore them!",
                 cancel: "Cancel",
             },
             defaultAction: "cancel",
             sourcePath: "/",
         },
-        RESTORE_CONFIRMATION_INTERACTION_ID,
+        RESTORE_CONFIRMATION_INTERACTION_ID
     );
     return action === "restore";
 }