Harden the backup tree's atomic copy (closes #22)

The backup copy now fsyncs its temp file before the rename and the
directory after it, using the download writer's new fsyncPath helper.
Each backup run deletes .quak-backup-*.tmp files whose process is no
longer running, leaving those of a concurrent backup alone. The rename
sites and the README backup layout state that a symlink at the
destination is replaced and the new file takes the temp file's
permissions, and the README names the temp files. Adds tests for a
missing and an unwritable destination directory for downloadFile and
downloadThumbnail.

Model: opus-5-5

README.md

@@ -490,6 +490,16 @@ appears in. On subsequent runs, existing originals are skipped. If a download
 fails, the error is logged and the backup continues with the next file. The exit
 code is non-zero if any files failed.
 
+Each original is copied to a temporary file named
+`.quak-backup-<fileID>.<ext>-<pid>-<random>.tmp` in the same directory, synced
+to disk, and renamed into place, so an original is either complete or absent,
+even after a power cut. A run that is killed can leave one of these temporary
+files behind; the next backup deletes those whose process is no longer running.
+Downloads and the content cache use the same scheme with `.quak-<random>.tmp`
+names. The rename replaces whatever was at the destination rather than writing
+through it: a symlink there is replaced, not followed, and the new file has the
+temporary file's permissions, not those of the file it replaced.
+
 ## TODO
 
 - [x] Retry policy: no retry on 4xx, exponential backoff on 5xx and network

TODO.md

@@ -18,6 +18,22 @@ Tag v1.0.0.
 
 # Completed Steps
 
+- 2026-09-23: Hardened the backup tree's atomic copy (issue 22). `copyAtomic`
+  fsyncs its temp file before the rename and the directory after it, through the
+  download writer's `fsyncPath`; each backup run deletes `.quak-backup-*.tmp`
+  files whose process is no longer running. The README backup layout names the
+  temp files and states that the rename replaces a symlink and takes the temp
+  file's permissions. Added tests for a missing and an unwritable destination
+  directory for `downloadFile` and `downloadThumbnail`.
+- 2026-09-22: Stopped `make test` collecting tests from checkouts nested under
+  `.claude/` (issue 25). vitest ignores `.gitignore` when finding tests, so a
+  nested checkout ran the whole suite again; `vitest.config.ts` now adds
+  `.claude/**` to vitest's default excludes, and
+  `test/packaging/nested-checkout.test.ts` plants a nested checkout in a temp
+  directory and fails if vitest would collect it.
+- 2026-09-22: Dropped the deprecated `@types/libsodium-wrappers-sumo` stub from
+  `devDependencies` (issue 27). It shipped no declarations; the types come from
+  `libsodium-wrappers-sumo` itself. `yarn.lock` regenerated by `yarn remove`.
 - 2026-09-22: Hardened the client session lifecycle (issue 10).
   `Client.fromJSON` checks every snapshot field and each key's decoded length
   and names the bad field; `toJSON` reads the token through

package.json

@@ -29,7 +29,6 @@
     },
     "devDependencies": {
         "@eslint/js": "9.38.0",
-        "@types/libsodium-wrappers-sumo": "0.8.2",
         "@types/node": "22.18.13",
         "eslint": "9.38.0",
         "prettier": "3.8.1",

src/backup.ts

@@ -29,19 +29,20 @@
 // rather than counted forever, which would poison a scheduled backup's exit code.
 
 import {
-    copyFileSync,
     lstatSync,
     mkdirSync,
+    readdirSync,
     readFileSync,
     readlinkSync,
-    renameSync,
     rmSync,
     statSync,
     symlinkSync,
     writeFileSync,
 } from "node:fs";
+import { copyFile, rename, rm } from "node:fs/promises";
 import { basename, dirname, join, relative } from "node:path";
 
+import { fsyncPath } from "./download/index.js";
 import { safeExtension, sanitizeFileName } from "./filename.js";
 import type { Collection, EnteFile } from "./model/types.js";
 
@@ -154,8 +155,12 @@ const errorMessage = (err: unknown): string =>
     err instanceof Error ? err.message : String(err);
 
 // Copy bytes into `dest` via a temp file in the same directory plus rename, so
-// `dest` appears only once it is whole ("present means complete").
+// `dest` appears only once it is whole ("present means complete"). As in the
-const copyAtomic = (src: string, dest: string): void => {
+// download writer, the temp file is fsynced before the rename and the directory
+// after it, so a power cut cannot leave a correctly named but short original.
+// The temp name carries this process's ID so a later run can tell a leftover
+// from a copy still in progress (see `removeLeftoverTempFiles`).
+const copyAtomic = async (src: string, dest: string): Promise<void> => {
     if (src === dest) return;
     const tmp = join(
         dirname(dest),
@@ -164,10 +169,45 @@ const copyAtomic = (src: string, dest: string): void => {
             .slice(2)}.tmp`,
     );
     try {
-        copyFileSync(src, tmp);
+        await copyFile(src, tmp);
-        renameSync(tmp, dest);
+        await fsyncPath(tmp);
+        // `rename` replaces the destination's directory entry: an existing
+        // symlink at `dest` is replaced, not followed, and the new file has
+        // the temp file's permissions (copied from `src`).
+        await rename(tmp, dest);
+        await fsyncPath(dirname(dest));
     } finally {
-        rmSync(tmp, { force: true });
+        await rm(tmp, { force: true });
+    }
+};
+
+// A process-ID check: signal 0 delivers nothing and only reports whether the
+// process exists. EPERM means it exists but belongs to another user.
+const isRunning = (pid: number): boolean => {
+    try {
+        process.kill(pid, 0);
+        return true;
+    } catch (err) {
+        return (err as NodeJS.ErrnoException).code === "EPERM";
+    }
+};
+
+// Delete the temp files `copyAtomic` leaves behind when a backup is killed
+// before its rename. Only files whose process is no longer running are
+// removed, so a backup running at the same time keeps its own. A reused
+// process ID can only keep a leftover a while longer, never remove a live one.
+const removeLeftoverTempFiles = (dir: string): void => {
+    let names: string[];
+    try {
+        names = readdirSync(dir);
+    } catch {
+        return;
+    }
+    for (const name of names) {
+        const match = /^\.quak-backup-.*-(\d+)-[0-9a-z]*\.tmp$/.exec(name);
+        if (match && !isRunning(Number(match[1]))) {
+            rmSync(join(dir, name), { force: true });
+        }
     }
 };
 
@@ -254,6 +294,8 @@ export const runBackup = async (
     mkdirSync(originalsDir, { recursive: true });
     mkdirSync(collectionsDir, { recursive: true });
     if (includeThumbnails) mkdirSync(thumbnailsDir, { recursive: true });
+    removeLeftoverTempFiles(originalsDir);
+    removeLeftoverTempFiles(thumbnailsDir);
 
     const ledgerPath = join(downloadDirectory, "failures.json");
     const ledger = loadLedger(ledgerPath);
@@ -321,7 +363,7 @@ export const runBackup = async (
             try {
                 log(`Fetching original ${file.metadata.title} (${fileID})...`);
                 const { path } = await lib.original(fileID);
-                copyAtomic(path, dest);
+                await copyAtomic(path, dest);
                 downloaded++;
             } catch (err) {
                 log(
@@ -342,7 +384,7 @@ export const runBackup = async (
             if (isPresent(dest)) continue;
             try {
                 const { path } = await lib.thumbnail(fileID);
-                copyAtomic(path, dest);
+                await copyAtomic(path, dest);
             } catch (err) {
                 recordFailure(
                     file,

src/download/index.ts

@@ -158,6 +158,18 @@ const streamDecrypt = async (
     return totalPlain;
 };
 
+// Fsync a file or a directory, so its contents (for a directory, its entries)
+// are on stable storage. Exported for the backup tree's copy, which needs the
+// same durability as the writer below.
+export const fsyncPath = async (path: string): Promise<void> => {
+    const handle = await open(path, "r");
+    try {
+        await handle.sync();
+    } finally {
+        await handle.close();
+    }
+};
+
 // Stage a write to `destination` atomically and durably, then rename it into
 // place. `fill` writes the contents into the open temp file handle — either the
 // whole buffer at once (`writeAtomic`) or chunk by chunk as they decrypt
@@ -193,16 +205,15 @@ const stageAtomic = async (
         } finally {
             await handle.close();
         }
+        // `rename` replaces the destination's directory entry rather than
+        // writing through it: an existing symlink at `destination` is
+        // replaced, not followed, and the new file has the temp file's
+        // permissions, not those of the file it replaced.
         await rename(tmpPath, destination);
         // Fsync the directory so the rename itself survives a crash: renaming
         // over a synced temp file still leaves the new directory entry in the
         // page cache until the directory is synced.
-        const dirHandle = await open(dir, "r");
+        await fsyncPath(dir);
-        try {
-            await dirHandle.sync();
-        } finally {
-            await dirHandle.close();
-        }
     } catch (err) {
         // Best-effort cleanup. A failure to remove the temporary file must
         // never replace the error that actually explains what went wrong.

test/cli/backup.test.ts

@@ -42,15 +42,44 @@ import {
     rmSync,
     writeFileSync,
 } from "node:fs";
+import { spawnSync } from "node:child_process";
 import { join } from "node:path";
 import { tmpdir } from "node:os";
-import { describe, it, expect, beforeEach, afterEach } from "vitest";
+import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
 
 import { Library } from "../../src/library/index.js";
 import type { ContentSource } from "../../src/library/content.js";
 import type { CollectionsPage, FilesPage } from "../../src/client.js";
 import type { Collection, EnteFile } from "../../src/model/types.js";
 
+// `open` and `rename` are wrapped to record, in order, every fsync and rename,
+// so a test can pin the sequence "fsync the temp file, rename, fsync the
+// directory" that makes a copied original survive a power cut. `vi.hoisted`
+// because `vi.mock` factories run before module-level constants exist.
+const fsEvents = vi.hoisted(() => [] as string[]);
+
+vi.mock("node:fs/promises", async (importOriginal) => {
+    const actual = await importOriginal<typeof import("node:fs/promises")>();
+    return {
+        ...actual,
+        open: async (
+            ...args: Parameters<typeof actual.open>
+        ): Promise<Awaited<ReturnType<typeof actual.open>>> => {
+            const handle = await actual.open(...args);
+            const realSync = handle.sync.bind(handle);
+            handle.sync = async (): Promise<void> => {
+                fsEvents.push(`sync:${String(args[0])}`);
+                await realSync();
+            };
+            return handle;
+        },
+        rename: async (from: string, to: string): Promise<void> => {
+            fsEvents.push(`rename:${to}`);
+            await actual.rename(from, to);
+        },
+    };
+});
+
 const USER_ID = 42;
 
 // Decrypted-byte length each stub original writes, keyed by fileID.
@@ -530,4 +559,66 @@ describe("lib.backup", () => {
         expect(readLedger(outDir).files["101"]!.attempts).toBe(1);
         lib.close();
     });
+
+    it("fsyncs a copied original before the rename and its directory after", async () => {
+        const lib = await openLibrary(stubSource());
+        const outDir = join(root, "backup");
+        const originals = join(outDir, "originals");
+        const dest = join(originals, "100.jpg");
+        fsEvents.length = 0;
+
+        await lib.backup({ downloadDirectory: outDir });
+
+        const at = fsEvents.indexOf(`rename:${dest}`);
+        expect(at).toBeGreaterThan(0);
+        expect(fsEvents[at - 1]).toMatch(
+            /^sync:.*\/\.quak-backup-100\.jpg-\d+-[0-9a-z]*\.tmp$/,
+        );
+        expect(fsEvents[at + 1]).toBe(`sync:${originals}`);
+        lib.close();
+    });
+
+    it("removes temp files left by a killed backup but not those of one still running", async () => {
+        const outDir = join(root, "backup");
+        const originals = join(outDir, "originals");
+        mkdirSync(originals, { recursive: true });
+        // A child that has already exited: its process ID is not running.
+        const exitedPID = spawnSync(process.execPath, ["-e", ""]).pid;
+        const leftover = `.quak-backup-100.jpg-${exitedPID}-abc123.tmp`;
+        // This test's own process stands in for a backup running at the same
+        // time.
+        const inProgress = `.quak-backup-101.jpg-${process.pid}-def456.tmp`;
+        writeFileSync(join(originals, leftover), "partial");
+        writeFileSync(join(originals, inProgress), "partial");
+        const lib = await openLibrary(stubSource());
+
+        await lib.backup({ downloadDirectory: outDir });
+
+        const names = readdirSync(originals);
+        expect(names).not.toContain(leftover);
+        expect(names).toContain(inProgress);
+        lib.close();
+    });
+
+    it("removes leftover temp files in thumbnails/ but not those of a backup still running", async () => {
+        const outDir = join(root, "backup");
+        const thumbnails = join(outDir, "thumbnails");
+        mkdirSync(thumbnails, { recursive: true });
+        const exitedPID = spawnSync(process.execPath, ["-e", ""]).pid;
+        const leftover = `.quak-backup-100.jpg-${exitedPID}-abc123.tmp`;
+        const inProgress = `.quak-backup-101.jpg-${process.pid}-def456.tmp`;
+        writeFileSync(join(thumbnails, leftover), "partial");
+        writeFileSync(join(thumbnails, inProgress), "partial");
+        const lib = await openLibrary(stubSource());
+
+        await lib.backup({
+            downloadDirectory: outDir,
+            includeThumbnails: true,
+        });
+
+        const names = readdirSync(thumbnails);
+        expect(names).not.toContain(leftover);
+        expect(names).toContain(inProgress);
+        lib.close();
+    });
 });

test/download/download.test.ts

@@ -48,6 +48,7 @@
  */
 
 import {
+    chmodSync,
     existsSync,
     mkdirSync,
     readdirSync,
@@ -989,6 +990,51 @@ describe.each(entryPoints)(
             expect(readFileSync(outPath)).toEqual(Buffer.from(existing));
             expect(readdirSync(dir)).toEqual(["rename-fails.bin"]);
         });
+
+        it("fails without creating anything when the destination directory does not exist", async () => {
+            const key = sodium.crypto_secretstream_xchacha20poly1305_keygen();
+            const { header, ciphertext } = encryptFileBody(
+                patternBytes(64, 33),
+                key,
+            );
+            const { api, file } = fixtureFor(key, header, ciphertext);
+            const dir = freshDir();
+            const outPath = join(dir, "missing", "never.bin");
+
+            await expect(download(api, file, outPath)).rejects.toMatchObject({
+                code: "ENOENT",
+            });
+
+            // The missing directory is not created on the caller's behalf.
+            expect(readdirSync(dir)).toEqual([]);
+        });
+
+        // Root ignores directory permissions, so this cannot fail as root
+        // (the Docker test image runs as root).
+        it.skipIf(process.getuid?.() === 0)(
+            "fails without creating anything when the destination directory is not writable",
+            async () => {
+                const key =
+                    sodium.crypto_secretstream_xchacha20poly1305_keygen();
+                const { header, ciphertext } = encryptFileBody(
+                    patternBytes(64, 34),
+                    key,
+                );
+                const { api, file } = fixtureFor(key, header, ciphertext);
+                const dir = freshDir();
+                const outPath = join(dir, "never.bin");
+                chmodSync(dir, 0o500);
+                try {
+                    await expect(
+                        download(api, file, outPath),
+                    ).rejects.toMatchObject({ code: "EACCES" });
+                } finally {
+                    chmodSync(dir, 0o700);
+                }
+
+                expect(readdirSync(dir)).toEqual([]);
+            },
+        );
     },
 );
 

test/packaging/nested-checkout.test.ts

@@ -0,0 +1,50 @@
+// A checkout nested under `.claude/` must not add its tests to this suite.
+// The test plants one in a temporary directory next to a real test file and
+// asks vitest, with this repo's config, which test files it would run.
+import { afterEach, describe, expect, it } from "vitest";
+import { execFileSync } from "node:child_process";
+import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { fileURLToPath } from "node:url";
+
+const repoRoot = fileURLToPath(new URL("../../", import.meta.url));
+
+let root = "";
+
+afterEach(() => {
+    rmSync(root, { recursive: true, force: true });
+});
+
+const writeTest = (path: string): void => {
+    mkdirSync(join(root, path, ".."), { recursive: true });
+    writeFileSync(
+        join(root, path),
+        'import { it } from "vitest";\nit("runs", () => {});\n',
+    );
+};
+
+describe("vitest.config.ts", () => {
+    it("does not collect tests from a checkout nested under .claude/", () => {
+        root = mkdtempSync(join(tmpdir(), "quak-nested-checkout-"));
+        writeTest("test/real.test.ts");
+        writeTest(".claude/worktrees/other/test/real.test.ts");
+
+        const output = execFileSync(
+            process.execPath,
+            [
+                join(repoRoot, "node_modules/vitest/vitest.mjs"),
+                "list",
+                "--filesOnly",
+                "--config",
+                join(repoRoot, "vitest.config.ts"),
+                "--root",
+                root,
+            ],
+            { cwd: root, encoding: "utf-8" },
+        );
+
+        const files = output.split("\n").filter((line) => line !== "");
+        expect(files).toEqual(["test/real.test.ts"]);
+    });
+});

vitest.config.ts

@@ -0,0 +1,10 @@
+import { configDefaults, defineConfig } from "vitest/config";
+
+// vitest does not read .gitignore when looking for tests. A checkout nested
+// under .claude/ has its own test/ tree, and without this exclude the suite
+// runs once per nested checkout and still reports success.
+export default defineConfig({
+    test: {
+        exclude: [...configDefaults.exclude, ".claude/**"],
+    },
+});

yarn.lock

@@ -528,13 +528,6 @@
   resolved "https://registry.yarnpkg.com/@types/json-schema/-/json-schema-7.0.15.tgz#596a1747233694d50f6ad8a7869fcb6f56cf5841"
   integrity sha512-5+fP8P8MFNC+AyZCDxrB2pkZFPGzqQWUzpSeuuVLvm8VMcorNYavBqoFcxK8bQz4Qsbn4oUEEem4wDLfcysGHA==
 
-"@types/libsodium-wrappers-sumo@0.8.2":
-  version "0.8.2"
-  resolved "https://registry.yarnpkg.com/@types/libsodium-wrappers-sumo/-/libsodium-wrappers-sumo-0.8.2.tgz#488e8747fbb982fe901020b5afeaddfa63da6830"
-  integrity sha512-uFOBpg/r21hExVlh2ty8YpDfSR+Yy3Jn8XS4+SSjitbhTxdYq+pBz/49XRxyUFe8SzqujHf/Wu0/O4d+FUtNfQ==
-  dependencies:
-    libsodium-wrappers-sumo "*"
-
 "@types/node@22.18.13":
   version "22.18.13"
   resolved "https://registry.yarnpkg.com/@types/node/-/node-22.18.13.tgz#a037c4f474b860be660e05dbe92a9ef945472e28"
@@ -1249,7 +1242,7 @@ libsodium-sumo@^0.8.0:
   resolved "https://registry.yarnpkg.com/libsodium-sumo/-/libsodium-sumo-0.8.4.tgz#6d4687781fa0ad398af14a7df872d5c27cf8cd31"
   integrity sha512-TMtHShQfVVsaxDygyapvUC3o7YsPgXa/hRWeIgzyFz6w5k/1hirGptCxp1U7XwW3rCskaTTYKgV10v86UiGgNw==
 
-libsodium-wrappers-sumo@*, libsodium-wrappers-sumo@0.8.4:
+libsodium-wrappers-sumo@0.8.4:
   version "0.8.4"
   resolved "https://registry.yarnpkg.com/libsodium-wrappers-sumo/-/libsodium-wrappers-sumo-0.8.4.tgz#6656a3e7e0551ecce08ddee4bfb501a092eac6fa"
   integrity sha512-ql7hcgulKZ3ekfa2DGAogcCKsWU0diA/0nArz1CFzh93WQdb46/Kj18ka/Hifq6uA3Ush34Pc6vU/6HXeRwUkg==