Fix deletions

CLAUDE.md

@@ -7,16 +7,15 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
 VaultLink is a self-hosted Obsidian file-sync system. Two halves of one repo:
 
 - `sync-server/` — Rust (axum + sqlx/SQLite). Source of truth for vault state, broadcasts changes via WebSocket.
-- `frontend/` — npm workspaces. The sync engine (`sync-client`) is consumed by an Obsidian plugin, a standalone CLI, a fuzz E2E harness, a scripted determinism harness, and a history UI.
+- `frontend/` — npm workspaces. The sync engine (`sync-client`) is consumed by an Obsidian plugin, a standalone CLI, a fuzz E2E harness, and a scripted determinism harness.
 
-The HTTP/WS API types are generated from Rust (`ts-rs`) and mirrored into the TS workspaces. **Never hand-edit files in `frontend/sync-client/src/services/types/` or `frontend/history-ui/src/lib/types/`** — run `scripts/update-api-types.sh` after changing anything Serde-derived in the server.
+The HTTP/WS API types are generated from Rust (`ts-rs`) and mirrored into the TS workspaces. **Never hand-edit files in `frontend/sync-client/src/services/types/`** — run `scripts/update-api-types.sh` after changing anything Serde-derived in the server.
 
 ### Frontend workspaces
 
 - `sync-client` — the sync engine; published to consumers via `dist/`. All other TS workspaces depend on it via `file:../sync-client`.
 - `obsidian-plugin` — Obsidian plugin built from `sync-client`.
 - `local-client-cli` — same engine wrapped as a standalone CLI.
-- `history-ui` — vault-history web UI.
 - `test-client` — fuzz E2E harness (random ops across N processes).
 - `deterministic-tests` — scripted multi-client tests with an in-memory FS, run against a real server.
 
@@ -67,7 +66,7 @@ Frontend dev (sync-client + obsidian-plugin watch in parallel):
 cd frontend && npm install && npm run dev
 ```
 
-Regenerate TS bindings from Rust types (touches `frontend/{sync-client,history-ui}/src/.../types/`):
+Regenerate TS bindings from Rust types (touches `frontend/sync-client/src/services/types/`):
 
 ```sh
 scripts/update-api-types.sh
@@ -119,7 +118,7 @@ Local FS events from the watcher update `localPath` synchronously at enqueue tim
 
 **Watermark.** `lastSeenUpdateId` uses a `MinCovered` (a contiguous-prefix tracker over a stream of integers): we only advance the published min when the next consecutive id has been processed, so out-of-order RemoteChange ids don't fool the WebSocket handshake into requesting a too-recent catch-up.
 
-**Server catch-up.** The server's WS handshake replays events newer than the client's `last_seen_vault_update_id` from the `latest_document_versions` view (one row per doc, the latest). On those replayed rows `is_new_file` means _new to this client_ (`creation_vault_update_id > last_seen_vault_update_id`), not "this row is the doc's first version" — necessary because the catch-up only carries the latest version; if a doc was created and updated past the watermark, the client never sees its create otherwise.
+**Server catch-up.** The server's WS handshake replays events newer than the client's `last_seen_vault_update_id`, computed as the latest version per document as of the cursor. The catch-up only carries each doc's *latest* version, not its full history. The client treats any RemoteChange whose `documentId` it has no record of as a fresh create and downloads the bytes.
 
 ## Edge-case patterns the sync engine has to survive
 
@@ -135,8 +134,6 @@ The two-loop split defuses most of the old race catalogue (slot-collision stashe
 
 **Watermark advancement is load-bearing both ways.** Branches that _skip_ a remote event without advancing `lastSeenUpdateId` create permanent gaps that re-deliver forever. Branches that _advance_ without applying the content lose data: the server has no further event to re-deliver, the catch-up only carries the latest version, and any state in between is gone. Don't advance unless the event was actually applied (or deliberately discarded after weighing both halves).
 
-**`isNewFile` semantics differ between catch-up and real-time.** On WS handshake replay it means _new to this client_ (`creation_vault_update_id > last_seen_vault_update_id`); on real-time broadcasts it means _this version is the create_ (`creation_vault_update_id == vault_update_id`). A handler that decides based on one interpretation will be wrong on the other channel; reasoning about fetch-and-treat-as-new vs. ignore needs to know which channel delivered the event.
-
 **Pause / disable-sync mid-flight** is the one race the new model doesn't structurally fix. An HTTP that committed server-side but whose response was discarded leaves the server holding a doc the client has no record of. Resume → offline scan → server-side dedupe handles it (the server merges the duplicate create into the existing doc), but if the merge produces a deconflict, the client picks up an extra file. Out of scope for the two-loop split.
 
 **Cycle reconciliation uses in-memory content swap.** When the move graph contains a cycle, the reconciler reads every file in the cycle into memory and writes each back to its new slot, with no tmp files. A write-ahead marker at `.vaultlink/swap-<uuid>.json` lists each leg; on startup the reconciler reads the marker, hashes each `from` to determine which legs ran, and replays the rest. The `.vaultlink/**` glob is hard-coded as an internal ignore pattern so swap markers don't get sync'd.

frontend/deterministic-tests/README.md

@@ -97,9 +97,10 @@ s.assertContains("path", "a", "b");         // all substrings present in file
 s.assertContainsAny("path", "a", "b"); // at least one substring present
 s.assertAnyFileContains("text"); // substring present in some file
 s.assertNoFileContains("text"); // substring absent from every file
-s.assertSubstringCount("path", "x", 3);     // substring appears exactly N times
 s.assertContentInAtMostOneFile("text"); // no duplicate content
-s.ifFileExists("path", (s) => { /* … */ }); // conditional block
+s.ifFileExists("path", (s) => {
+  /* … */
+}); // conditional block
 s.getContent("path"); // raw content (or "" if missing)
 ```
 

frontend/deterministic-tests/src/cli.ts

@@ -42,7 +42,7 @@ function testUsesPauseServer(test: TestDefinition): boolean {
  */
 function findProjectRoot(): string {
     let dir = path.dirname(__filename);
-    const root = path.parse(dir).root;
+    const { root } = path.parse(dir);
     while (dir !== root) {
         if (
             fs.existsSync(path.join(dir, "sync-server")) &&

frontend/deterministic-tests/src/deterministic-agent.ts

@@ -59,6 +59,26 @@ export class DeterministicAgent extends debugging.InMemoryFileSystem {
         this.data.settings = { ...initialSettings };
     }
 
+    private static isCreateDocumentRequest(
+        input: RequestInfo | URL,
+        init: RequestInit | undefined
+    ): boolean {
+        const method =
+            init?.method ??
+            (typeof Request !== "undefined" && input instanceof Request
+                ? input.method
+                : "GET");
+        if (method.toUpperCase() !== "POST") {
+            return false;
+        }
+
+        const url =
+            input instanceof URL
+                ? input
+                : new URL(typeof input === "string" ? input : input.url);
+        return /\/documents\/?$/.test(url.pathname);
+    }
+
     public async init(
         fetchImplementation: typeof globalThis.fetch
     ): Promise<void> {
@@ -118,13 +138,12 @@ export class DeterministicAgent extends debugging.InMemoryFileSystem {
             this.nextCreateResponseDrop === undefined,
             `Client ${this.clientId} already has a create response drop armed`
         );
-        let resolveDropped!: () => void;
+        const resolvers = Promise.withResolvers<undefined>();
-        const dropped = new Promise<void>((resolve) => {
-            resolveDropped = resolve;
-        });
         this.nextCreateResponseDrop = {
-            dropped,
+            dropped: resolvers.promise as Promise<void>,
-            resolveDropped
+            resolveDropped: (): void => {
+                resolvers.resolve(undefined);
+            }
         };
         this.log("Armed next create response drop");
     }
@@ -155,9 +174,7 @@ export class DeterministicAgent extends debugging.InMemoryFileSystem {
         await withTimeout(
             new Promise<void>((resolve) => {
                 const unsubscribe = this.client.onSyncHistoryUpdated.add(() => {
-                    const entry = this.client
+                    const entry = this.client.getHistoryEntries().find(matches);
-                        .getHistoryEntries()
-                        .find(matches);
                     if (entry === undefined) {
                         return;
                     }
@@ -304,11 +321,8 @@ export class DeterministicAgent extends debugging.InMemoryFileSystem {
             });
         }
 
-        const nextWriteRename = this.nextWriteRename;
+        const { nextWriteRename } = this;
-        if (
+        if (nextWriteRename?.oldPath === path) {
-            nextWriteRename !== undefined &&
-            nextWriteRename.oldPath === path
-        ) {
             this.nextWriteRename = undefined;
             await super.rename(
                 nextWriteRename.oldPath,
@@ -460,24 +474,4 @@ export class DeterministicAgent extends debugging.InMemoryFileSystem {
             return response;
         };
     }
-
-    private static isCreateDocumentRequest(
-        input: RequestInfo | URL,
-        init: RequestInit | undefined
-    ): boolean {
-        const method =
-            init?.method ??
-            (typeof Request !== "undefined" && input instanceof Request
-                ? input.method
-                : "GET");
-        if (method.toUpperCase() !== "POST") {
-            return false;
-        }
-
-        const url =
-            input instanceof URL
-                ? input
-                : new URL(typeof input === "string" ? input : input.url);
-        return /\/documents\/?$/.test(url.pathname);
-    }
 }

frontend/deterministic-tests/src/server-control.ts

@@ -46,7 +46,7 @@ export class ServerControl {
         // Retry on bind failure: findFreePort closes its probe before we
         // spawn, so under heavy parallelism another process can grab the
         // same port. Each attempt picks a fresh port.
-        let lastError: unknown;
+        let lastError: unknown = undefined;
         for (let attempt = 1; attempt <= SERVER_START_MAX_ATTEMPTS; attempt++) {
             try {
                 await this.startOnce();
@@ -65,69 +65,6 @@ export class ServerControl {
         );
     }
 
-    private async startOnce(): Promise<void> {
-        const reservation = await findFreePort();
-        this._port = reservation.port;
-        const tmpBase = os.tmpdir();
-        this.tempDir = fs.mkdtempSync(path.join(tmpBase, "vault-link-test-"));
-        const tempConfigPath = path.join(this.tempDir, "config.yml");
-        const dbDir = path.join(this.tempDir, "databases");
-
-        this.writeConfigFile(tempConfigPath, dbDir);
-
-        this.logger.info(
-            `Starting server: ${this.serverPath} (port ${this._port})`
-        );
-
-        // Release the port reservation right before spawning to minimize
-        // the TOCTOU window between port discovery and server binding.
-        reservation.release();
-
-        this.process = spawn(this.serverPath, [tempConfigPath], {
-            stdio: ["ignore", "pipe", "pipe"],
-            detached: false
-        });
-
-        this.process.stdout?.on("data", (data: Buffer) => {
-            this.logger.info(`[SERVER] ${data.toString().trim()}`);
-        });
-
-        this.process.stderr?.on("data", (data: Buffer) => {
-            this.logger.info(`[SERVER] ${data.toString().trim()}`);
-        });
-
-        this.process.on("error", (err) => {
-            this.logger.error(`[SERVER] Process error: ${err.message}`);
-        });
-
-        const currentProcess = this.process;
-        currentProcess.on("exit", (code, signal) => {
-            this.logger.info(
-                `Server exited with code ${code}, signal ${signal}`
-            );
-            // Only clear state if this handler is for the current process.
-            // A fast stop→start cycle could create a new process before this
-            // handler fires — clearing state here would corrupt the new one.
-            if (this.process === currentProcess) {
-                this.process = null;
-                this._isPaused = false;
-            }
-        });
-
-        try {
-            await this.waitForReady();
-        } catch (error) {
-            // Kill the spawned process if it failed to become ready,
-            // preventing a zombie process from lingering.
-            try {
-                await this.stop();
-            } catch {
-                // Best-effort cleanup
-            }
-            throw error;
-        }
-    }
-
     public async waitForReady(
         maxAttempts: number = SERVER_READY_MAX_ATTEMPTS
     ): Promise<void> {
@@ -239,8 +176,7 @@ export class ServerControl {
     public isRunning(): boolean {
         const proc = this.process;
         return (
-            proc !== null &&
+            proc?.pid !== undefined &&
-            proc.pid !== undefined &&
             proc.exitCode === null &&
             proc.signalCode === null
         );
@@ -269,6 +205,69 @@ export class ServerControl {
         }
     }
 
+    private async startOnce(): Promise<void> {
+        const reservation = await findFreePort();
+        this._port = reservation.port;
+        const tmpBase = os.tmpdir();
+        this.tempDir = fs.mkdtempSync(path.join(tmpBase, "vault-link-test-"));
+        const tempConfigPath = path.join(this.tempDir, "config.yml");
+        const dbDir = path.join(this.tempDir, "databases");
+
+        this.writeConfigFile(tempConfigPath, dbDir);
+
+        this.logger.info(
+            `Starting server: ${this.serverPath} (port ${this._port})`
+        );
+
+        // Release the port reservation right before spawning to minimize
+        // the TOCTOU window between port discovery and server binding.
+        reservation.release();
+
+        this.process = spawn(this.serverPath, [tempConfigPath], {
+            stdio: ["ignore", "pipe", "pipe"],
+            detached: false
+        });
+
+        this.process.stdout?.on("data", (data: Buffer) => {
+            this.logger.info(`[SERVER] ${data.toString().trim()}`);
+        });
+
+        this.process.stderr?.on("data", (data: Buffer) => {
+            this.logger.info(`[SERVER] ${data.toString().trim()}`);
+        });
+
+        this.process.on("error", (err) => {
+            this.logger.error(`[SERVER] Process error: ${err.message}`);
+        });
+
+        const currentProcess = this.process;
+        currentProcess.on("exit", (code, signal) => {
+            this.logger.info(
+                `Server exited with code ${code}, signal ${signal}`
+            );
+            // Only clear state if this handler is for the current process.
+            // A fast stop→start cycle could create a new process before this
+            // handler fires — clearing state here would corrupt the new one.
+            if (this.process === currentProcess) {
+                this.process = null;
+                this._isPaused = false;
+            }
+        });
+
+        try {
+            await this.waitForReady();
+        } catch (error) {
+            // Kill the spawned process if it failed to become ready,
+            // preventing a zombie process from lingering.
+            try {
+                await this.stop();
+            } catch {
+                // Best-effort cleanup
+            }
+            throw error;
+        }
+    }
+
     private writeConfigFile(destPath: string, dbDir: string): void {
         // Assumes config-e2e.yml has exactly one 2-space-indented `port:` and
         // one `databases_directory_path:` (under `server:` and `database:`

frontend/deterministic-tests/src/test-registry.ts

@@ -103,6 +103,7 @@ import { renamedPendingCreateReusedPathThenDeleteTest } from "./tests/renamed-pe
 import { renamePendingCreateOntoPendingDeletePathTest } from "./tests/rename-pending-create-onto-pending-delete-path.test";
 import { remoteQuickWriteRenameBeforeRecordTest } from "./tests/remote-quick-write-rename-before-record.test";
 import { selfMergePendingRenameAliasesSecondCreateTest } from "./tests/self-merge-pending-rename-aliases-second-create.test";
+import { disableMidCreateThenDeleteTest } from "./tests/disable-mid-create-then-delete.test";
 
 export const TESTS: Partial<Record<string, TestDefinition>> = {
     "rename-create-conflict": renameCreateConflictTest,
@@ -239,5 +240,6 @@ export const TESTS: Partial<Record<string, TestDefinition>> = {
     "remote-quick-write-rename-before-record":
         remoteQuickWriteRenameBeforeRecordTest,
     "self-merge-pending-rename-aliases-second-create":
-        selfMergePendingRenameAliasesSecondCreateTest
+        selfMergePendingRenameAliasesSecondCreateTest,
+    "disable-mid-create-then-delete": disableMidCreateThenDeleteTest
 };

frontend/deterministic-tests/src/test-runner.ts

@@ -1,7 +1,7 @@
 import type { TestDefinition, TestResult, TestStep } from "./test-definition";
 import { DeterministicAgent } from "./deterministic-agent";
 import type { ServerControl } from "./server-control";
-import type { SyncSettings, Logger } from "sync-client";
+import { SyncType, type SyncSettings, type Logger } from "sync-client";
 import { assert } from "./utils/assert";
 import { AssertableState } from "./utils/assertable-state";
 import { sleep } from "./utils/sleep";
@@ -188,9 +188,11 @@ export class TestRunner {
                 const agent = this.getAgent(step.client);
                 const historySeen = agent.waitForHistoryEntry(
                     (entry) =>
-                        entry.details.type === step.syncType &&
+                        entry.details.type === SyncType[step.syncType] &&
                         entry.details.relativePath === step.path,
-                    () => this.serverControl.pause()
+                    () => {
+                        this.serverControl.pause();
+                    }
                 );
                 this.serverControl.resume();
                 await historySeen;

frontend/deterministic-tests/src/tests/catchup-create-and-update-not-skipped.test.ts

@@ -5,14 +5,12 @@ export const catchupCreateAndUpdateNotSkippedTest: TestDefinition = {
     description:
         "Client 1 disconnects (sync disabled). Client 0 creates a doc and " +
         "then updates it. When Client 1 reconnects, the server's catch-up " +
-        "stream sends only the doc's *latest* version (the update), not the " +
+        "stream sends only the doc's *latest* version (the update), not " +
-        "full history. Pre-fix the wire's `is_new_file` was set to " +
+        "the full history. Client 1 must still pick up the doc — any handler " +
-        "`creation == latest_version`, so the catch-up flagged the doc as " +
+        "that gates the create-on-untracked path on a server-supplied " +
-        "non-new even though Client 1 had never seen its creation. Client " +
+        "'is this the first version' flag would drop it (the latest version " +
-        "1's `processRemoteChange` then dropped it as a 'stale RemoteChange " +
+        "is not the create), silently leaking the doc. The client treats " +
-        "for untracked, non-new document' and the doc was silently lost. " +
+        "every untracked-doc RemoteChange as a fresh create.",
-        "Post-fix `is_new_file` in the catch-up stream means 'new relative " +
-        "to the recipient's watermark' (`creation > last_seen_vault_update_id`).",
     clients: 2,
     steps: [
         { type: "enable-sync", client: 0 },
@@ -36,7 +34,7 @@ export const catchupCreateAndUpdateNotSkippedTest: TestDefinition = {
 
         // Client 0 updates the doc (vault_update_id v_X > v_C). The
         // server's `latest_document_versions` view now returns the
-        // *update* row — its `creation_vault_update_id != vault_update_id`.
+        // *update* row — the create row is no longer the latest.
         {
             type: "update",
             client: 0,
@@ -46,10 +44,9 @@ export const catchupCreateAndUpdateNotSkippedTest: TestDefinition = {
         { type: "sync", client: 0 },
 
         // Client 1 reconnects. Server's catch-up replays docs with
-        // `vault_update_id > last_seen`. For doc.md it sends v_X with
+        // `vault_update_id > last_seen`. For doc.md it sends v_X; Client
-        // `is_new_file` derived from `creation_vault_update_id >
+        // 1 has no record of the doc, so it treats the RemoteChange as a
-        // last_seen_vault_update_id` (post-fix) — so Client 1 treats it
+        // fresh create and downloads the latest content.
-        // as a fresh create and downloads the latest content.
         { type: "enable-sync", client: 1 },
         { type: "barrier" },
 

frontend/deterministic-tests/src/tests/concurrent-rename-and-create-at-target-create-first.test.ts

@@ -1,7 +1,8 @@
 import type { AssertableState } from "../utils/assertable-state";
 import type { TestDefinition } from "../test-definition";
 
-export const concurrentRenameAndCreateAtTargetCreateFirstTest: TestDefinition = {
+export const concurrentRenameAndCreateAtTargetCreateFirstTest: TestDefinition =
+    {
         description:
             "One client renames X to Y while another creates a new file at Y, " +
             "both offline. After syncing, Y should contain merged content from " +

frontend/deterministic-tests/src/tests/concurrent-rename-and-create-at-target-rename-first.test.ts

@@ -1,7 +1,8 @@
 import type { AssertableState } from "../utils/assertable-state";
 import type { TestDefinition } from "../test-definition";
 
-export const concurrentRenameAndCreateAtTargetRenameFirstTest: TestDefinition = {
+export const concurrentRenameAndCreateAtTargetRenameFirstTest: TestDefinition =
+    {
         description:
             "One client renames X to Y while another creates a new file at Y, " +
             "both offline. We can't merge the create because it would result in a cycle",

frontend/deterministic-tests/src/tests/disable-mid-create-then-delete.test.ts

@@ -0,0 +1,56 @@
+import type { AssertableState } from "../utils/assertable-state";
+import type { TestDefinition } from "../test-definition";
+
+export const disableMidCreateThenDeleteTest: TestDefinition = {
+    description:
+        "Reproduces a fuzz failure where one client's create-then-delete-then-disable-sync " +
+        "sequence loses the file: the create commits server-side, the response is " +
+        "lost (sync reset), the local file is deleted, then sync is re-enabled. The " +
+        "catch-up replay should redeliver the create so both clients converge to " +
+        "having the file (the delete never reached the server because its docId " +
+        "Promise was rejected when the queue cleared).",
+    clients: 2,
+    steps: [
+        // Client 0 is online (the witness); client 1 starts disabled.
+        { type: "enable-sync", client: 0 },
+
+        // Client 1 creates the file while offline so the LocalCreate is queued.
+        { type: "create", client: 1, path: "file-32.md", content: "hello" },
+
+        // Arm the drop so client 1's create POST commits server-side but the
+        // response is replaced with SyncResetError (matches the fuzz scenario
+        // where sync was disabled mid-flight).
+        { type: "drop-next-create-response", client: 1 },
+
+        // Enable sync on client 1: offline scan picks up file-32, drain fires
+        // POST /documents, server commits, broadcast goes out, response is
+        // dropped on the client. SyncResetError exits the drain leaving the
+        // create event still in the queue.
+        { type: "enable-sync", client: 1 },
+        { type: "wait-for-dropped-create-response", client: 1 },
+
+        // The user then deletes the file locally and toggles sync off/on
+        // (the same flow the fuzz harness used). The disable's pause()
+        // does not clear the queue, but the re-enable runs an offline
+        // scan that calls clearPending() — wiping the dangling LocalCreate
+        // and any LocalDelete behind it. The local disk is empty, so
+        // nothing is enqueued.
+        { type: "delete", client: 1, path: "file-32.md" },
+        { type: "disable-sync", client: 1 },
+        { type: "enable-sync", client: 1 },
+
+        // Catch-up on the new WS connection should deliver file-32 (vault
+        // update id 1) since client 1's lastSeenUpdateId is still 0.
+        { type: "barrier" },
+
+        {
+            type: "assert-consistent",
+            verify: (s: AssertableState): void => {
+                s.assertFileExists("file-32.md").assertContent(
+                    "file-32.md",
+                    "hello"
+                );
+            }
+        }
+    ]
+};

frontend/deterministic-tests/src/utils/assertable-state.ts

@@ -106,22 +106,6 @@ export class AssertableState {
         return this;
     }
 
-    public assertSubstringCount(
-        path: string,
-        substring: string,
-        expected: number
-    ): this {
-        this.assertFileExists(path);
-        const content = this.files.get(path) ?? "";
-        const actual = content.split(substring).length - 1;
-        if (actual !== expected) {
-            throw new Error(
-                `Expected "${substring}" to appear ${expected} time(s) in "${path}", found ${actual}. Content: "${content}"`
-            );
-        }
-        return this;
-    }
-
     public assertContentInAtMostOneFile(substring: string): this {
         const matches = Array.from(this.files.entries()).filter(([, content]) =>
             content.includes(substring)
@@ -143,8 +127,4 @@ export class AssertableState {
         }
         return this;
     }
-
-    public getContent(path: string): string {
-        return this.files.get(path) ?? "";
-    }
 }

frontend/local-client-cli/src/args.test.ts

@@ -169,7 +169,6 @@ test("parseArgs - parse ERROR log level", () => {
     assert.equal(args.logLevel, LogLevel.ERROR);
 });
 
-
 test("parseArgs - reads required options from environment variables", () => {
     process.env.VAULTLINK_LOCAL_PATH = "/env/path";
     process.env.VAULTLINK_REMOTE_URI = "https://env.example.com";

frontend/local-client-cli/src/cli.ts

@@ -1,11 +1,10 @@
 import * as path from "path";
 import * as fs from "fs/promises";
 import * as fsSync from "fs";
-import type { NetworkConnectionStatus } from "sync-client";
+import type { NetworkConnectionStatus, Logger } from "sync-client";
 import {
     SyncClient,
     DEFAULT_SETTINGS,
-    Logger,
     LogLevel,
     LogLine,
     type SyncSettings,

frontend/obsidian-plugin/src/obsidian-file-system.ts

@@ -139,10 +139,6 @@ export class ObsidianFileSystemOperations implements FileSystemOperations {
         return (await this.statFile(path)).size;
     }
 
-    public async getModificationTime(path: RelativePath): Promise<Date> {
-        return new Date((await this.statFile(path)).mtime);
-    }
-
     public async exists(path: RelativePath): Promise<boolean> {
         return this.vault.adapter.exists(normalizePath(path));
     }

frontend/sync-client/src/file-operations/file-operations.test.ts

@@ -7,7 +7,6 @@ import { assertSetContainsExactly } from "../utils/assert-set-contains-exactly";
 import type { FileSystemOperations } from "./filesystem-operations";
 import type { TextWithCursors } from "reconcile-text";
 import type { ServerConfig, ServerConfigData } from "../services/server-config";
-import { ExpectedFsEvents } from "../sync-operations/expected-fs-events";
 import { FileAlreadyExistsError } from "../errors/file-already-exists-error";
 
 class MockServerConfig implements Pick<ServerConfig, "getConfig"> {
@@ -72,8 +71,7 @@ function makeOps(): {
     const ops = new FileOperations(
         new Logger(),
         fs,
-        new MockServerConfig() as ServerConfig, // eslint-disable-line @typescript-eslint/no-unsafe-type-assertion
+        new MockServerConfig() as ServerConfig // eslint-disable-line @typescript-eslint/no-unsafe-type-assertion
-        new ExpectedFsEvents()
     );
     return { fs, ops };
 }
@@ -85,7 +83,7 @@ describe("File operations", () => {
         const result = await ops.create("a", new Uint8Array());
 
         assertSetContainsExactly(fs.names, "a");
-        assert.equal(result.actualPath, "a");
+        assert.equal(result, "a");
     });
 
     it("create throws FileAlreadyExistsError when the path is occupied", async () => {
@@ -109,7 +107,7 @@ describe("File operations", () => {
 
         const result = await ops.move("a", "b");
         assertSetContainsExactly(fs.names, "b");
-        assert.equal(result.actualPath, "b");
+        assert.equal(result, "b");
     });
 
     it("move with same source and target is a no-op", async () => {
@@ -119,7 +117,7 @@ describe("File operations", () => {
         const result = await ops.move("a", "a");
 
         assertSetContainsExactly(fs.names, "a");
-        assert.equal(result.actualPath, "a");
+        assert.equal(result, "a");
     });
 
     it("move throws FileAlreadyExistsError when the target is occupied", async () => {

frontend/sync-client/src/file-operations/file-operations.ts

@@ -9,17 +9,6 @@ import { isBinary } from "../utils/is-binary";
 import type { ServerConfig } from "../services/server-config";
 import { FileNotFoundError } from "../errors/file-not-found-error";
 import { FileAlreadyExistsError } from "../errors/file-already-exists-error";
-import type { ExpectedFsEvents } from "../sync-operations/expected-fs-events";
-
-/**
- * Outcome of a `move`/`create`. `actualPath` is where the file ended up;
- * with the conflict-path machinery removed it is always equal to the
- * requested path. The shape is preserved so callers don't all need to
- * change.
- */
-export interface FileOpResult {
-    actualPath: RelativePath;
-}
 
 export class FileOperations {
     private readonly fs: SafeFileSystemOperations;
@@ -28,7 +17,6 @@ export class FileOperations {
         private readonly logger: Logger,
         fs: FileSystemOperations,
         private readonly serverConfig: ServerConfig,
-        private readonly expectedFsEvents: ExpectedFsEvents,
         private readonly nativeLineEndings = "\n"
     ) {
         this.fs = new SafeFileSystemOperations(fs, logger);
@@ -68,7 +56,7 @@ export class FileOperations {
     public async create(
         path: RelativePath,
         newContent: Uint8Array
-    ): Promise<FileOpResult> {
+    ): Promise<RelativePath> {
         if (await this.fs.exists(path)) {
             throw new FileAlreadyExistsError(
                 `Refusing to create '${path}': file already exists`,
@@ -77,14 +65,8 @@ export class FileOperations {
         }
         await this.createParentDirectories(path);
 
-        this.expectedFsEvents.expectCreate(path);
-        try {
         await this.fs.write(path, this.toNativeLineEndings(newContent));
-        } catch (e) {
+        return path;
-            this.expectedFsEvents.unexpectCreate(path);
-            throw e;
-        }
-        return { actualPath: path };
     }
 
     /**
@@ -105,12 +87,6 @@ export class FileOperations {
             return;
         }
 
-        // Single-source the expectation registration: register exactly once
-        // per call, and unexpect from the catch if the underlying fs op
-        // throws (FileNotFoundError or otherwise). The previous shape
-        // registered inside each branch and let the catch swallow
-        // FileNotFoundError, leaking the expectation into the map.
-        this.expectedFsEvents.expectUpdate(path);
         try {
             if (
                 !isFileTypeMergable(
@@ -175,7 +151,6 @@ export class FileOperations {
                 }
             );
         } catch (e) {
-            this.expectedFsEvents.unexpectUpdate(path);
             if (e instanceof FileNotFoundError) {
                 this.logger.debug(
                     `File ${path} disappeared during write; not recreating`
@@ -188,13 +163,7 @@ export class FileOperations {
 
     public async delete(path: RelativePath): Promise<void> {
         if (await this.exists(path)) {
-            this.expectedFsEvents.expectDelete(path);
-            try {
             await this.fs.delete(path);
-            } catch (e) {
-                this.expectedFsEvents.unexpectDelete(path);
-                throw e;
-            }
             await this.deletingEmptyParentDirectoriesOfDeletedFile(path);
         } else {
             this.logger.debug(`No need to delete '${path}', it doesn't exist`);
@@ -220,9 +189,9 @@ export class FileOperations {
     public async move(
         oldPath: RelativePath,
         newPath: RelativePath
-    ): Promise<FileOpResult> {
+    ): Promise<RelativePath> {
         if (oldPath === newPath) {
-            return { actualPath: oldPath };
+            return oldPath;
         }
 
         if (await this.fs.exists(newPath)) {
@@ -233,15 +202,9 @@ export class FileOperations {
         }
         await this.createParentDirectories(newPath);
 
-        this.expectedFsEvents.expectRename(oldPath, newPath);
-        try {
         await this.fs.rename(oldPath, newPath);
-        } catch (e) {
-            this.expectedFsEvents.unexpectRename(oldPath, newPath);
-            throw e;
-        }
         await this.deletingEmptyParentDirectoriesOfDeletedFile(oldPath);
-        return { actualPath: newPath };
+        return newPath;
     }
 
     private async deletingEmptyParentDirectoriesOfDeletedFile(

frontend/sync-client/src/services/sync-service.ts

@@ -13,8 +13,6 @@ import { HttpClientError } from "../errors/http-client-error";
 import type { SerializedError } from "./types/SerializedError";
 import type { DocumentVersionWithoutContent } from "./types/DocumentVersionWithoutContent";
 import type { DocumentUpdateResponse } from "./types/DocumentUpdateResponse";
-import type { DocumentVersion } from "./types/DocumentVersion";
-import type { FetchLatestDocumentsResponse } from "./types/FetchLatestDocumentsResponse";
 import type { PingResponse } from "./types/PingResponse";
 import type { UpdateTextDocumentVersion } from "./types/UpdateTextDocumentVersion";
 import { buildVaultUrl } from "./build-vault-url";
@@ -272,32 +270,6 @@ export class SyncService {
         });
     }
 
-    public async get({
-        documentId
-    }: {
-        documentId: DocumentId;
-    }): Promise<DocumentVersion> {
-        return this.retryForever(async () => {
-            this.logger.debug(`Getting document with id ${documentId}`);
-
-            const response = await this.client(
-                this.getUrl(`/documents/${documentId}`),
-                {
-                    headers: this.getDefaultHeaders()
-                }
-            );
-
-            await SyncService.throwIfNotOk(response, "get document");
-
-            const result: DocumentVersion =
-                (await response.json()) as DocumentVersion; // eslint-disable-line @typescript-eslint/no-unsafe-type-assertion
-
-            this.logger.debug(`Got document ${JSON.stringify(result)}`);
-
-            return result;
-        });
-    }
-
     public async getDocumentVersionContent({
         documentId,
         vaultUpdateId
@@ -332,36 +304,6 @@ export class SyncService {
         });
     }
 
-    public async getAll(
-        since?: VaultUpdateId
-    ): Promise<FetchLatestDocumentsResponse> {
-        return this.retryForever(async () => {
-            this.logger.debug(
-                "Getting all documents" +
-                    (since != null ? ` since ${since}` : "")
-            );
-
-            const url = new URL(this.getUrl("/documents"));
-            if (since !== undefined) {
-                url.searchParams.append("since_update_id", since.toString());
-            }
-            const response = await this.client(url.toString(), {
-                headers: this.getDefaultHeaders()
-            });
-
-            await SyncService.throwIfNotOk(response, "get documents");
-
-            const result: FetchLatestDocumentsResponse =
-                (await response.json()) as FetchLatestDocumentsResponse; // eslint-disable-line @typescript-eslint/no-unsafe-type-assertion
-
-            this.logger.debug(
-                `Got ${result.latestDocuments.length} document metadata`
-            );
-
-            return result;
-        });
-    }
-
     public async ping(): Promise<PingResponse> {
         this.logger.debug("Pinging server");
         const response = await this.pingClient(this.getUrl("/ping"), {

frontend/sync-client/src/services/types/DocumentVersionWithoutContent.ts

@@ -9,8 +9,4 @@ export interface DocumentVersionWithoutContent {
     userId: string;
     deviceId: string;
     contentSize: number;
-    /**
-     * True iff this is the first version of the document
-     */
-    isNewFile: boolean;
 }

frontend/sync-client/src/services/types/FetchLatestDocumentsResponse.ts

@@ -1,13 +0,0 @@
-// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
-import type { DocumentVersionWithoutContent } from "./DocumentVersionWithoutContent";
-
-/**
- * Response to a fetch latest documents request.
- */
-export interface FetchLatestDocumentsResponse {
-    latestDocuments: DocumentVersionWithoutContent[];
-    /**
-     * The update ID of the latest document in the response.
-     */
-    lastUpdateId: bigint;
-}

frontend/sync-client/src/sync-client.ts

@@ -30,7 +30,6 @@ import { setUpTelemetry } from "./utils/set-up-telemetry";
 import { ServerConfig } from "./services/server-config";
 import type { EventListeners } from "./utils/data-structures/event-listeners";
 import { Lock } from "./utils/data-structures/locks";
-import { ExpectedFsEvents } from "./sync-operations/expected-fs-events";
 
 export class SyncClient {
     private hasFinishedOfflineSync = false;
@@ -55,14 +54,7 @@ export class SyncClient {
         private readonly fileChangeNotifier: FileChangeNotifier,
         private readonly contentCache: FixedSizeDocumentCache,
         private readonly serverConfig: ServerConfig,
-        private readonly syncService: SyncService,
+        private readonly syncService: SyncService
-        private readonly expectedFsEvents: ExpectedFsEvents,
-        private readonly persistence: PersistenceProvider<
-            Partial<{
-                settings: Partial<SyncSettings>;
-                database: Partial<StoredSyncState>;
-            }>
-        >
     ) {}
 
     public get syncedDocumentCount(): number {
@@ -172,7 +164,7 @@ export class SyncClient {
         // new deviceId, the server-side query would miss, and the
         // pending-but-lost create would deconflict instead of
         // binding to the doc its content was already absorbed into.
-        let deviceId = state.deviceId;
+        let { deviceId } = state;
         if (deviceId === undefined) {
             deviceId = createClientId();
             state = { ...state, deviceId };
@@ -215,13 +207,10 @@ export class SyncClient {
 
         const serverConfig = new ServerConfig(syncService, settings);
 
-        const expectedFsEvents = new ExpectedFsEvents();
-
         const fileOperations = new FileOperations(
             logger,
             fs,
             serverConfig,
-            expectedFsEvents,
             nativeLineEndings
         );
 
@@ -268,9 +257,7 @@ export class SyncClient {
             fileChangeNotifier,
             contentCache,
             serverConfig,
-            syncService,
+            syncService
-            expectedFsEvents,
-            persistence
         );
 
         logger.info("SyncClient created successfully");
@@ -322,26 +309,6 @@ export class SyncClient {
         }
     }
 
-    /**
-     * Reload settings from disk overriding current in-memory settings.
-     * Missing values will be filled in from DEFAULT_SETTINGS rather than
-     * retaining current in-memory settings.
-     */
-    public async reloadSettings(): Promise<void> {
-        this.checkIfDestroyed("reloadSettings");
-
-        const state = (await this.persistence.load()) ?? {
-            settings: undefined
-        };
-
-        const settings = {
-            ...DEFAULT_SETTINGS,
-            ...(state.settings ?? {})
-        };
-
-        await this.setSettings(settings);
-    }
-
     public async checkConnection(): Promise<NetworkConnectionStatus> {
         this.checkIfDestroyed("checkConnection");
 
@@ -405,10 +372,6 @@ export class SyncClient {
         this.checkIfDestroyed("syncLocallyCreatedFile");
 
         this.fileChangeNotifier.notifyOfFileChange(relativePath); // this is for updating cursors
-        if (this.expectedFsEvents.matchCreate(relativePath)) {
-            return;
-        }
-
         this.syncer.syncLocallyCreatedFile(relativePath);
     }
 
@@ -422,10 +385,6 @@ export class SyncClient {
         this.checkIfDestroyed("syncLocallyUpdatedFile");
 
         this.fileChangeNotifier.notifyOfFileChange(relativePath); // this is for updating cursors
-        if (this.expectedFsEvents.matchUpdate(relativePath, oldPath)) {
-            return;
-        }
-
         this.syncer.syncLocallyUpdatedFile({
             oldPath,
             relativePath
@@ -436,10 +395,6 @@ export class SyncClient {
         this.checkIfDestroyed("syncLocallyDeletedFile");
 
         this.fileChangeNotifier.notifyOfFileChange(relativePath); // this is for updating cursors
-        if (this.expectedFsEvents.matchDelete(relativePath)) {
-            return;
-        }
-
         this.syncer.syncLocallyDeletedFile(relativePath);
     }
 
@@ -567,10 +522,6 @@ export class SyncClient {
         // paused (offline edits, deletes, renames) wouldn't be detected, and
         // an incoming remote update would silently overwrite them.
         this.syncer.clearOfflineScanGate();
-        // Drop any expected fs events that were registered but never matched
-        // (e.g. an op aborted by SyncResetError). Otherwise a real user edit
-        // at the same path after re-enable would be swallowed.
-        this.expectedFsEvents.clear();
     }
 
     private resetInMemoryState(): void {

frontend/sync-client/src/sync-operations/expected-fs-events.ts

@@ -1,138 +0,0 @@
-import type { RelativePath } from "./types";
-
-/**
- * Counter-based registry of filesystem events the syncer is about to
- * cause. The syncer's own writes/renames/deletes go through
- * `FileOperations`, which calls into the host filesystem; the host then
- * fires watcher events that come back through `SyncClient.syncLocallyXxx`.
- * Without filtering, those echo events would be re-uploaded to the server
- * and broadcast back, producing an unbounded loop.
- *
- * The fix: every fs call in `FileOperations` registers the event it is
- * about to provoke; the matching `syncLocallyXxx` handler consumes it.
- * User-initiated edits never register, so they pass through unchanged.
- *
- * Counts are per (kind, path) so back-to-back syncer ops on the same path
- * (e.g. apply remote update then re-apply during convergence) match
- * one-for-one. If the watcher never fires for a registered op (e.g. the
- * fs throws before notifying), the entry is left behind; `clear()` is
- * called on pause/destroy to drop those before they collide with a real
- * user event later.
- */
-export class ExpectedFsEvents {
-    private readonly creates = new Map<RelativePath, number>();
-    private readonly updates = new Map<RelativePath, number>();
-    private readonly deletes = new Map<RelativePath, number>();
-    // Renames are keyed by `JSON.stringify({oldPath, newPath})` so the
-    // delimiter cannot occur inside either path.
-    private readonly renames = new Map<RelativePath, number>();
-
-    private static renameKey(
-        oldPath: RelativePath,
-        newPath: RelativePath
-    ): string {
-        return JSON.stringify({ oldPath, newPath });
-    }
-
-    public expectCreate(path: RelativePath): void {
-        this.bump(this.creates, path);
-    }
-
-    public expectUpdate(path: RelativePath): void {
-        this.bump(this.updates, path);
-    }
-
-    public expectDelete(path: RelativePath): void {
-        this.bump(this.deletes, path);
-    }
-
-    public expectRename(oldPath: RelativePath, newPath: RelativePath): void {
-        this.bump(this.renames, ExpectedFsEvents.renameKey(oldPath, newPath));
-    }
-
-    /**
-     * Cancel a previously-registered expectation when the fs op that registered
-     * it failed before any watcher event could fire. Without this, a leaked
-     * expectation silently swallows the next genuine user event at the same
-     * path (or, for renames, the same `oldPath → newPath` pair).
-     *
-     * Floored at zero: if the watcher *did* fire (op partially completed) and
-     * already consumed the entry, the unexpect is a no-op. The fallback is
-     * acceptable — at worst we re-upload a real edit we'd otherwise filter.
-     */
-    public unexpectCreate(path: RelativePath): void {
-        this.decrement(this.creates, path);
-    }
-
-    public unexpectUpdate(path: RelativePath): void {
-        this.decrement(this.updates, path);
-    }
-
-    public unexpectDelete(path: RelativePath): void {
-        this.decrement(this.deletes, path);
-    }
-
-    public unexpectRename(oldPath: RelativePath, newPath: RelativePath): void {
-        this.decrement(
-            this.renames,
-            ExpectedFsEvents.renameKey(oldPath, newPath)
-        );
-    }
-
-    public matchCreate(path: RelativePath): boolean {
-        return this.consume(this.creates, path);
-    }
-
-    public matchUpdate(
-        path: RelativePath,
-        oldPath: RelativePath | undefined
-    ): boolean {
-        if (oldPath !== undefined) {
-            return this.consume(
-                this.renames,
-                ExpectedFsEvents.renameKey(oldPath, path)
-            );
-        }
-        return this.consume(this.updates, path);
-    }
-
-    public matchDelete(path: RelativePath): boolean {
-        return this.consume(this.deletes, path);
-    }
-
-    public clear(): void {
-        this.creates.clear();
-        this.updates.clear();
-        this.deletes.clear();
-        this.renames.clear();
-    }
-
-    private bump(map: Map<RelativePath, number>, key: RelativePath): void {
-        map.set(key, (map.get(key) ?? 0) + 1);
-    }
-
-    private consume(
-        map: Map<RelativePath, number>,
-        key: RelativePath
-    ): boolean {
-        const count = map.get(key) ?? 0;
-        if (count === 0) {
-            return false;
-        }
-        if (count === 1) {
-            map.delete(key);
-        } else {
-            map.set(key, count - 1);
-        }
-        return true;
-    }
-
-    private decrement(map: Map<RelativePath, number>, key: RelativePath): void {
-        const count = map.get(key) ?? 0;
-        if (count <= 1) {
-            map.delete(key);
-        } else {
-            map.set(key, count - 1);
-        }
-    }
-}

frontend/sync-client/src/sync-operations/offline-change-detector.test.ts

@@ -2,7 +2,10 @@ import { describe, it } from "node:test";
 import assert from "node:assert";
 import { Logger } from "../tracing/logger";
 import { Settings } from "../persistence/settings";
-import { STORED_STATE_SCHEMA_VERSION, SyncEventQueue } from "./sync-event-queue";
+import {
+    STORED_STATE_SCHEMA_VERSION,
+    SyncEventQueue
+} from "./sync-event-queue";
 import { scheduleOfflineChanges } from "./offline-change-detector";
 import type { FileOperations } from "../file-operations/file-operations";
 import type { RelativePath } from "./types";
@@ -22,19 +25,20 @@ const makeQueue = async (): Promise<SyncEventQueue> => {
     );
 };
 
-const makeOperations = (
+const makeOperations = (files: Record<string, Uint8Array>): FileOperations => {
-    files: Record<string, Uint8Array>
+    const map = new Map<RelativePath, Uint8Array>(Object.entries(files));
-): FileOperations => {
+    const partial: Partial<FileOperations> = {
-    return {
+        listFilesRecursively: async () => [...map.keys()],
-        listFilesRecursively: async () => Object.keys(files),
         read: async (path: RelativePath) => {
-            const data = files[path];
+            const data = map.get(path);
             if (data === undefined) {
                 throw new Error(`File not found: ${path}`);
             }
             return data;
         }
-    } as unknown as FileOperations;
+    };
+    // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
+    return partial as FileOperations;
 };
 
 describe("scheduleOfflineChanges", () => {
@@ -70,7 +74,8 @@ describe("scheduleOfflineChanges", () => {
             operations,
             queue,
             (path) => enqueued.push({ kind: "create", path }),
-            (args) => enqueued.push({ kind: "update", path: args.relativePath }),
+            (args) =>
+                enqueued.push({ kind: "update", path: args.relativePath }),
             (path) => enqueued.push({ kind: "delete", path })
         );
 
@@ -109,13 +114,12 @@ describe("scheduleOfflineChanges", () => {
             operations,
             queue,
             (path) => enqueued.push({ kind: "create", path }),
-            (args) => enqueued.push({ kind: "update", path: args.relativePath }),
+            (args) =>
+                enqueued.push({ kind: "update", path: args.relativePath }),
             (path) => enqueued.push({ kind: "delete", path })
         );
 
-        assert.deepStrictEqual(enqueued, [
+        assert.deepStrictEqual(enqueued, [{ kind: "update", path: "doc.md" }]);
-            { kind: "update", path: "doc.md" }
-        ]);
     });
 
     it("schedules a delete for a settled record whose local file is missing", async () => {
@@ -136,13 +140,12 @@ describe("scheduleOfflineChanges", () => {
             operations,
             queue,
             (path) => enqueued.push({ kind: "create", path }),
-            (args) => enqueued.push({ kind: "update", path: args.relativePath }),
+            (args) =>
+                enqueued.push({ kind: "update", path: args.relativePath }),
             (path) => enqueued.push({ kind: "delete", path })
         );
 
-        assert.deepStrictEqual(enqueued, [
+        assert.deepStrictEqual(enqueued, [{ kind: "delete", path: "gone.md" }]);
-            { kind: "delete", path: "gone.md" }
-        ]);
     });
 
     it("detects an offline rename when an untracked file matches a deleted record's content hash", async () => {

frontend/sync-client/src/sync-operations/offline-change-detector.ts

@@ -7,6 +7,24 @@ import type { SyncEventQueue } from "./sync-event-queue";
 import { removeFromArray } from "../utils/remove-from-array";
 import { FileNotFoundError } from "../errors/file-not-found-error";
 
+async function readOrUndefined(
+    operations: FileOperations,
+    path: RelativePath,
+    logger: Logger
+): Promise<Uint8Array | undefined> {
+    try {
+        return await operations.read(path);
+    } catch (e) {
+        if (e instanceof FileNotFoundError) {
+            logger.debug(
+                `File ${path} disappeared before offline-scan could read it; skipping`
+            );
+            return undefined;
+        }
+        throw e;
+    }
+}
+
 /**
  * Scans the local filesystem and the document database to determine
  * which files were created, updated, moved, or deleted while the
@@ -85,19 +103,11 @@ export async function scheduleOfflineChanges(
     // the whole scan; nothing to sync for a file that's already gone.
     const disappearedPaths = new Set<RelativePath>();
     for (const path of locallyPossibleCreatedFiles) {
-        let content: Uint8Array;
+        const content = await readOrUndefined(operations, path, logger);
-        try {
+        if (content === undefined) {
-            content = await operations.read(path);
-        } catch (e) {
-            if (e instanceof FileNotFoundError) {
-                logger.debug(
-                    `File ${path} disappeared before offline-scan could read it; skipping`
-                );
             disappearedPaths.add(path);
             continue;
         }
-            throw e;
-        }
         const contentHash = await hash(content);
 
         const matchingDeletedFile = await findMatchingFile(
@@ -148,8 +158,7 @@ export async function scheduleOfflineChanges(
     for (const path of syncedLocalFiles) {
         const record = allDocuments.get(path);
         if (
-            record !== undefined &&
+            record?.localPath !== undefined &&
-            record.localPath !== undefined &&
             record.localPath !== record.remoteRelativePath &&
             !allLocalFiles.has(record.remoteRelativePath) &&
             queue.byLocalPath.get(record.remoteRelativePath) === undefined

frontend/sync-client/src/sync-operations/reconciler.test.ts

@@ -2,7 +2,10 @@ import { describe, it } from "node:test";
 import assert from "node:assert";
 import { Logger, LogLevel } from "../tracing/logger";
 import { Settings } from "../persistence/settings";
-import { STORED_STATE_SCHEMA_VERSION, SyncEventQueue } from "./sync-event-queue";
+import {
+    STORED_STATE_SCHEMA_VERSION,
+    SyncEventQueue
+} from "./sync-event-queue";
 import { Reconciler } from "./reconciler";
 import { SyncResetError } from "../errors/sync-reset-error";
 import type { FileOperations } from "../file-operations/file-operations";
@@ -32,18 +35,22 @@ describe("Reconciler", () => {
             localPath: undefined
         });
 
-        const operations = {
+        const operationsPartial: Partial<FileOperations> = {
             exists: async () => false,
             create: async () => {
                 assert.fail("reset-interrupted placement should not write");
             }
-        } as unknown as FileOperations;
+        };
+        // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
+        const operations = operationsPartial as FileOperations;
 
-        const syncService = {
+        const syncServicePartial: Partial<SyncService> = {
             getDocumentVersionContent: async () => {
                 throw new SyncResetError();
             }
-        } as unknown as SyncService;
+        };
+        // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
+        const syncService = syncServicePartial as SyncService;
 
         const reconciler = new Reconciler(
             logger,

frontend/sync-client/src/sync-operations/reconciler.ts

@@ -306,9 +306,65 @@ export class Reconciler {
             }
         }
 
+        // Re-check ownership after the content fetch. A user rename or
+        // other interleaved op may have placed bytes / claimed the slot
+        // during the await. Without this, `upsertRecord` below would
+        // displace the new owner (clearing its `localPath`) and the
+        // following `operations.create` would then throw
+        // `FileAlreadyExistsError`, leaving the displaced record
+        // placement-pending with its bytes orphaned on disk.
+        try {
+            if (await this.operations.exists(target)) {
+                this.logger.debug(
+                    `Reconciler: cannot place ${record.documentId} at ${target} ` +
+                        `— slot newly occupied on disk after fetch; will retry next pass`
+                );
+                return;
+            }
+        } catch (e) {
+            this.logger.error(
+                `Reconciler: existence check failed for ${target}: ${String(e)}`
+            );
+            return;
+        }
+        if (this.queue.byLocalPath.get(target) !== undefined) {
+            this.logger.debug(
+                `Reconciler: cannot place ${record.documentId} at ${target} ` +
+                    `— slot newly tracked by another record after fetch; will retry next pass`
+            );
+            return;
+        }
+
+        // Install the slot *before* the disk write so the watcher's
+        // create echo, when it arrives, sees `byLocalPath[target]` set
+        // and the queue's enqueue-time echo guard drops it. Also pre-
+        // populates `remoteHash` so any downstream operation that
+        // compares against it (e.g. `processLocalUpdate`'s hashChanged
+        // skip) sees the right value. Mirrors the ordering in
+        // `processRemoteCreateForNewDocument`'s quick-write branch.
+        const contentHash = await hash(content);
+        try {
+            await this.queue.upsertRecord({
+                documentId: record.documentId,
+                parentVersionId: record.parentVersionId,
+                remoteRelativePath: record.remoteRelativePath,
+                remoteHash: contentHash,
+                localPath: target
+            });
+        } catch (e) {
+            this.logger.error(
+                `Reconciler: upsertRecord before create failed for ${record.documentId}: ${String(e)}`
+            );
+            return;
+        }
+
         try {
             await this.operations.create(target, content);
         } catch (e) {
+            // Roll back the slot claim so a later pass can retry or
+            // re-resolve. Without this, the record looks placed but the
+            // bytes never made it to disk.
+            await this.queue.setLocalPath(record.documentId, undefined);
             if (e instanceof FileNotFoundError) {
                 this.logger.debug(
                     `Reconciler: create at ${target} hit FileNotFound (likely parent ` +
@@ -329,14 +385,6 @@ export class Reconciler {
             return;
         }
 
-        try {
-            await this.queue.setLocalPath(record.documentId, target);
-        } catch (e) {
-            this.logger.error(
-                `Reconciler: setLocalPath after create failed for ${record.documentId}: ${String(e)}`
-            );
-            return;
-        }
         this.pendingPlacementContent.delete(record.documentId);
         this.logger.debug(
             `Reconciler: placed ${record.documentId} at ${target}`
@@ -663,8 +711,10 @@ export class Reconciler {
         // We pass the freshly-read pre-write content as
         // `expectedContent` so the 3-way merge inside `operations.write`
         // becomes a clean overwrite (no concurrent edits to merge with).
-        // `operations.write` registers `expectUpdate` itself, so the
+        // Each leg's echo modify event is harmless: the wire-loop's
-        // watcher swallows each leg's modify event.
+        // `processLocalUpdate` re-hashes the file and compares to
+        // `record.remoteHash`, which was set to match the just-written
+        // bytes — so the echo is dropped as a no-op.
         const writtenLegs: SwapLeg[] = [];
         for (const leg of legs) {
             const newBytes = contentByDocId.get(leg.documentId);

frontend/sync-client/src/sync-operations/sync-event-queue.test.ts

@@ -66,7 +66,6 @@ function fakeRemoteVersion(
         userId: "user",
         deviceId: "device",
         contentSize: 100,
-        isNewFile: true,
         ...overrides
     };
 }
@@ -248,36 +247,33 @@ describe("SyncEventQueue", () => {
         assert.strictEqual(second.isUserRename, true);
     });
 
-    it("settled record owns a path over a stale pending create", async () => {
+    it("drops LocalCreate echoes for paths already tracked", async () => {
+        // The syncer's own remote-create writes (quick-write +
+        // reconciler placements) upsert the record at `localPath`
+        // before calling `operations.create`. The watcher echo then
+        // re-enters as a LocalCreate at the same path — it must be
+        // dropped here, otherwise the wire-loop would POST a duplicate
+        // and the server would deconflict it into a phantom file.
         const queue = createQueue();
         await queue.upsertRecord(fakeRecord("A", { localPath: "b.md" }));
 
         await queue.enqueue({ type: SyncEventType.LocalCreate, path: "b.md" });
-        await queue.enqueue({
+
-            type: SyncEventType.LocalUpdate,
+        assert.strictEqual(await queue.next(), undefined);
-            path: "c.md",
-            oldPath: "b.md"
     });
 
-        const aRecord = queue.getDocumentByDocumentId("A");
+    it("admits LocalCreate when the prior owner is pending server delete", async () => {
-        assert.strictEqual(aRecord?.localPath, "c.md");
+        // A user create at a path whose previous doc is in the
-        assert.strictEqual(
+        // HTTP-acked-but-WS-pending window is genuine — propagate it.
-            queue.getRecordByLocalPath("b.md" as RelativePath),
+        const queue = createQueue();
-            undefined
+        await queue.upsertRecord(fakeRecord("A", { localPath: "b.md" }));
-        );
+        queue.markServerDeletePending("A");
-        assert.strictEqual(
+
-            queue.getRecordByLocalPath("c.md" as RelativePath)?.documentId,
+        await queue.enqueue({ type: SyncEventType.LocalCreate, path: "b.md" });
-            "A"
-        );
 
         const create = await queue.next();
         assert.strictEqual(create?.type, SyncEventType.LocalCreate);
         assert.strictEqual(create.path, "b.md");
-
-        const update = await queue.next();
-        assert.strictEqual(update?.type, SyncEventType.LocalUpdate);
-        assert.strictEqual(update.documentId, "A");
-        assert.strictEqual(update.path, "c.md");
     });
 
     it("byLocalPath stays consistent across upsertRecord, setLocalPath, and rename", async () => {
@@ -307,7 +303,10 @@ describe("SyncEventQueue", () => {
             queue.byLocalPath.get("renamed.md" as RelativePath),
             undefined
         );
-        assert.strictEqual(queue.getDocumentByDocumentId("A")?.localPath, "a.md");
+        assert.strictEqual(
+            queue.getDocumentByDocumentId("A")?.localPath,
+            "a.md"
+        );
 
         // setLocalPath does re-key — it's the explicit path-mutation API.
         await queue.setLocalPath("A", "later.md" as RelativePath);

frontend/sync-client/src/sync-operations/sync-event-queue.ts

@@ -94,6 +94,16 @@ export class SyncEventQueue {
     // `clearAllState` / schema-version-mismatch reset.
     private readonly _pendingServerDeletes = new Set<DocumentId>();
 
+    // DocIds we've seen deleted in this session. `removeDocumentById`
+    // adds here so that any stale `RemoteChange` for that doc that
+    // arrives later (e.g. an older vuid buffered in the network-chaos
+    // jitter pipeline, or a re-enqueue that landed after the delete's
+    // `purgeRemoteChangesForDocumentId`) is recognised in
+    // `processRemoteChange` and skipped instead of falling through to
+    // `processRemoteCreateForNewDocument` and resurrecting the doc
+    // with pre-delete bytes. Cleared on `clearAllState`.
+    private readonly _deletedDocumentIds = new Set<DocumentId>();
+
     public constructor(
         private readonly settings: Settings,
         private readonly logger: Logger,
@@ -220,9 +230,7 @@ export class SyncEventQueue {
      * path) still fires when neither side holds a record for the
      * collision target.
      */
-    public lastSeenUpdateIdForCreate(
+    public lastSeenUpdateIdForCreate(requestPath: RelativePath): VaultUpdateId {
-        requestPath: RelativePath
-    ): VaultUpdateId {
         let watermark = this._lastSeenUpdateId.min;
         for (const record of this.byDocId.values()) {
             if (
@@ -269,6 +277,16 @@ export class SyncEventQueue {
         }
 
         if (input.type === SyncEventType.LocalCreate) {
+            const owner = this._byLocalPath.get(path);
+            if (
+                owner !== undefined &&
+                !this.hasPendingServerDelete(owner.documentId)
+            ) {
+                this.logger.debug(
+                    `Ignoring LocalCreate echo at ${path}: slot is already tracked by ${owner.documentId}`
+                );
+                return;
+            }
             this.events.push({
                 type: SyncEventType.LocalCreate,
                 path,
@@ -324,7 +342,7 @@ export class SyncEventQueue {
                 !pendingCreate.isProcessing
             ) {
                 this.cancelPendingCreate(pendingCreate);
-                if (recordIsDeleting && record !== undefined) {
+                if (recordIsDeleting) {
                     // A stale deleting record was still claiming this path.
                     // The not-yet-started create/delete pair collapsed to
                     // nothing, and the disk file is gone, so clear the stale
@@ -343,11 +361,11 @@ export class SyncEventQueue {
                 path: lookupPath
             });
             this.notifyPendingUpdateCountChanged();
-            if (recordOwnsLookupPath && record !== undefined) {
+            if (recordOwnsLookupPath) {
                 // The file is gone from disk; clear the doc's localPath so the
                 // Reconciler doesn't try to operate on a vacated slot.
                 await this.setLocalPath(record.documentId, undefined);
-            } else if (recordIsDeleting && record !== undefined) {
+            } else if (recordIsDeleting) {
                 // A stale deleting record was still claiming this path while a
                 // newer pending create owned the actual disk file. Drop the
                 // stale claim now that the file is gone.
@@ -597,6 +615,15 @@ export class SyncEventQueue {
     }
 
     public async removeDocumentById(documentId: DocumentId): Promise<void> {
+        // Record the tombstone unconditionally: `processRemoteChange`
+        // checks it to drop late RemoteChanges that would otherwise
+        // resurrect the doc via `processRemoteCreateForNewDocument`.
+        // Purging the queue (below) only catches events that are
+        // already enqueued; events that arrive after this point (e.g.
+        // a stale broadcast buffered in the network-chaos jitter
+        // pipeline, or a re-enqueue that lands after this purge) need
+        // the tombstone to be skipped.
+        this._deletedDocumentIds.add(documentId);
         const record = this.byDocId.get(documentId);
         if (record === undefined) {
             // Still clear any deletion-pending mark and purge stale
@@ -620,13 +647,16 @@ export class SyncEventQueue {
         // in the queue ahead of it. Once those drain and the doc is
         // removed, a still-pending RemoteChange for an earlier version
         // would be processed by `processRemoteCreateForNewDocument` (the
-        // doc is now untracked, and catch-up's `isNewFile=true` semantics
+        // doc is now untracked), resurrecting the doc on disk with stale
-        // qualify it as a fresh create), resurrecting the doc on disk
+        // bytes that disagree with every other agent.
-        // with stale bytes that disagree with every other agent.
         this.purgeRemoteChangesForDocumentId(documentId);
         return this.save();
     }
 
+    public hasBeenDeleted(documentId: DocumentId): boolean {
+        return this._deletedDocumentIds.has(documentId);
+    }
+
     /**
      * Mark a doc as "HTTP DELETE has been acked by the server but the
      * WebSocket receipt that would call `removeDocumentById` hasn't arrived
@@ -648,14 +678,6 @@ export class SyncEventQueue {
         return this.byDocId.get(target);
     }
 
-    public getDocumentByDocumentIdOrFail(target: DocumentId): DocumentRecord {
-        const result = this.getDocumentByDocumentId(target);
-        if (!result) {
-            throw new Error(`No document found with id ${target}`);
-        }
-        return result;
-    }
-
     public getRecordByLocalPath(
         path: RelativePath
     ): DocumentRecord | undefined {
@@ -740,6 +762,7 @@ export class SyncEventQueue {
         this.byDocId.clear();
         this._byLocalPath.clear();
         this._pendingServerDeletes.clear();
+        this._deletedDocumentIds.clear();
         this._lastSeenUpdateId.reset();
         await this.save();
     }
@@ -814,6 +837,7 @@ export class SyncEventQueue {
                 event.path === path &&
                 event.documentId !== promise
             ) {
+                // eslint-disable-next-line no-restricted-syntax -- splice-by-index here is a reorder, not an item removal
                 this.events.splice(i, 1);
                 this.events.splice(createIndex, 0, event);
                 createIndex++;
@@ -866,6 +890,7 @@ export class SyncEventQueue {
                 typeof event.documentId === "string" &&
                 blockingDocIds.has(event.documentId)
             ) {
+                // eslint-disable-next-line no-restricted-syntax -- splice-by-index here is a reorder, not an item removal
                 this.events.splice(i, 1);
                 this.events.splice(createIndex, 0, event);
                 createIndex++;
@@ -907,8 +932,8 @@ export class SyncEventQueue {
             this._byLocalPath.delete(previousLocalPath);
         }
         record.localPath = newLocalPath;
-        let displacedRecord: DocumentRecord | undefined;
+        let displacedRecord: DocumentRecord | undefined = undefined;
-        let displacedOldPath: RelativePath | undefined;
+        let displacedOldPath: RelativePath | undefined = undefined;
         if (newLocalPath !== undefined) {
             const displaced = this._byLocalPath.get(newLocalPath);
             if (displaced !== undefined && displaced !== record) {

frontend/sync-client/src/sync-operations/syncer.ts

@@ -592,10 +592,28 @@ export class Syncer {
     ): Promise<void> {
         const documentId = await event.documentId;
         const record = this.queue.getDocumentByDocumentId(documentId);
-        if (
+        if (record === undefined) {
-            record?.localPath !== undefined &&
+            // The doc is no longer tracked. Typical cause: a remote delete
-            record.localPath !== event.path
+            // arrived first and `processRemoteDelete` already ran
-        ) {
+            // `removeDocumentById`, but `operations.delete` fired a
+            // watcher echo that landed in the queue as a stale LocalDelete.
+            // Without this skip we'd re-send DELETE to the server for a
+            // doc that's already gone.
+            this.logger.debug(
+                `Skipping local-delete for ${documentId} — doc no longer tracked`
+            );
+            return;
+        }
+        if (this.queue.hasPendingServerDelete(documentId)) {
+            // We already initiated the server delete; nothing more to do.
+            // Reaches here when a LocalDelete echo lands behind the
+            // already-queued LocalDelete that drove the server delete.
+            this.logger.debug(
+                `Skipping local-delete for ${documentId} — server delete already pending`
+            );
+            return;
+        }
+        if (record.localPath !== undefined && record.localPath !== event.path) {
             this.logger.debug(
                 `Skipping local-delete for ${documentId} at ${event.path}: ` +
                     `record now owns ${record.localPath}`
@@ -703,8 +721,7 @@ export class Syncer {
         if (response.isDeleted) {
             await this.processRemoteDelete(record.localPath, {
                 ...response,
-                contentSize: 0,
+                contentSize: 0
-                isNewFile: false
             });
             return;
         }
@@ -859,10 +876,19 @@ export class Syncer {
             return this.processRemoteUpdate(trackedRecord, remoteVersion);
         }
 
-        if (!remoteVersion.isNewFile) {
+        // Tombstoned: we removed this doc in this session via
+        // `removeDocumentById` (either WS delete receipt or PUT response
+        // with `isDeleted=true`). A late RemoteChange for the same doc
+        // can still reach us — buffered in the network-chaos jitter
+        // pipeline, or re-enqueued after the delete's purge — and
+        // without this gate `processRemoteCreateForNewDocument` would
+        // happily fetch pre-delete bytes and resurrect the doc, blocking
+        // any other doc whose `remoteRelativePath` happens to be the
+        // same slot.
+        if (this.queue.hasBeenDeleted(remoteVersion.documentId)) {
             this.queue.lastSeenUpdateId = remoteVersion.vaultUpdateId;
             this.logger.debug(
-                `Ignoring stale RemoteChange for untracked, non-new document ${remoteVersion.documentId}`
+                `Discarding stale remote update for tombstoned ${remoteVersion.documentId} at ${remoteVersion.relativePath}`
             );
             return;
         }
@@ -1103,7 +1129,7 @@ export class Syncer {
                         remoteHash,
                         localPath: target
                     });
-                    const result = await this.operations.create(
+                    const createdPath = await this.operations.create(
                         target,
                         remoteContent
                     );
@@ -1112,7 +1138,7 @@ export class Syncer {
                     );
                     localPath =
                         liveRecord === undefined
-                            ? result.actualPath
+                            ? createdPath
                             : liveRecord.localPath;
                     await this.updateCache(
                         remoteVersion.vaultUpdateId,

frontend/sync-client/src/tracing/logger.ts

@@ -54,11 +54,6 @@ export class Logger {
         );
     }
 
-    public reset(): void {
-        this.messages.length = 0;
-        this.debug("Logger has been reset");
-    }
-
     private pushMessage(message: string, level: LogLevel): void {
         const logLine = new LogLine(level, message);
         this.messages.push(logLine);

frontend/sync-client/src/utils/data-structures/locks.ts

@@ -92,10 +92,6 @@ export class Locks<T> {
         this.waiters.clear();
     }
 
-    public isLocked(key: T): boolean {
-        return this.locked.has(key);
-    }
-
     /**
      * Attempts to acquire a lock immediately without waiting.
      * Must call `unlock()` if successful.

frontend/test-client/src/agent/mock-agent.ts

@@ -58,7 +58,8 @@ export class MockAgent extends MockClient {
         // (e.g. `initial-1.md → initial-1 (2).md` after a same-path
         // collision) lands at a path the touch-list never knew about,
         // and an offline rename against that path strands the file.
-        this.client.onDocumentPathChanged.add((_documentId, oldPath, newPath) => {
+        this.client.onDocumentPathChanged.add(
+            (_documentId, oldPath, newPath) => {
                 if (oldPath !== undefined && newPath !== undefined) {
                     if (this.doNotTouchWhileOffline.includes(oldPath)) {
                         this.doNotTouchWhileOffline.push(newPath);
@@ -67,7 +68,8 @@ export class MockAgent extends MockClient {
                         this.doNotRenameWhileOffline.push(newPath);
                     }
                 }
-        });
+            }
+        );
 
         this.client.logger.onLogEmitted.add((logLine: LogLine) => {
             const state = this.client.getSettings().isSyncEnabled

frontend/test-client/src/cli.ts

@@ -7,7 +7,7 @@ import { randomCasing } from "./utils/random-casing";
 import { TimeoutError } from "./utils/with-timeout";
 import { TestErrorTracker } from "./utils/test-error-tracker";
 
-const TEST_ITERATIONS = 5;
+const TEST_ITERATIONS = 50;
 const MAX_INITIAL_DOCS = 10;
 
 // Simulate async file access by injecting waiting time before returning from file operations.

scripts/check.sh

@@ -21,9 +21,10 @@ cargo test --verbose
 
 if [[ "$FIX_MODE" == true ]]; then
     cargo clippy --all-targets --all-features --fix --allow-dirty --allow-staged
+    cargo clippy --all-targets --all-features -- -D warnings
     cargo fmt --all
 else
-    cargo clippy --all-targets --all-features
+    cargo clippy --all-targets --all-features -- -D warnings
     cargo fmt --all -- --check
 fi
 

scripts/e2e.sh

@@ -91,25 +91,10 @@ print_failed_log() {
     return 1
 }
 
-E2E_TIMEOUT=${2:-3600}
+echo "Monitoring $process_count processes"
-start_time=$(date +%s)
-echo "Monitoring $process_count processes (timeout: ${E2E_TIMEOUT}s)"
 
 # Monitor processes
 while true; do
-    # Script-level timeout to prevent indefinite hangs
-    current_time=$(date +%s)
-    elapsed=$((current_time - start_time))
-    if [ $elapsed -ge $E2E_TIMEOUT ]; then
-        echo "E2E timeout reached (${E2E_TIMEOUT}s). Killing remaining processes."
-        for pid in "${pids[@]}"; do
-            if [ -n "$pid" ]; then
-                kill $pid 2>/dev/null || true
-            fi
-        done
-        exit 1
-    fi
-
     if print_failed_log; then
         # Kill remaining processes
         for pid in "${pids[@]}"; do

scripts/update-api-types.sh

@@ -8,11 +8,9 @@ cd sync-server
 cargo test export_bindings
 cd -
 
-# Both target directories contain only generated bindings — wipe and copy
+# Wipe and copy generated bindings into the consuming workspace
 rm -f frontend/sync-client/src/services/types/*.ts
-rm -f frontend/history-ui/src/lib/types/*.ts
 cp -r sync-server/bindings/* frontend/sync-client/src/services/types/
-cp -r sync-server/bindings/* frontend/history-ui/src/lib/types/
 
 cd frontend
 npm run lint

sync-server/Cargo.lock

@@ -2181,7 +2181,6 @@ dependencies = [
  "log",
  "rand 0.9.0",
  "reconcile-text",
- "regex",
  "sanitize-filename",
  "serde",
  "serde_json",

sync-server/Cargo.toml

@@ -26,7 +26,6 @@ sqlx = { version = "0.8.6", features = ["sqlite", "runtime-tokio", "uuid", "chro
 chrono = { version = "0.4.41", features = ["serde"] }
 rand = "0.9.0"
 sanitize-filename = "0.6.0"
-regex = "1.12.2"
 clap = { version = "4.5.38", features = ["derive"] }
 futures = "0.3.31"
 serde_yaml = "0.9.34"
@@ -49,15 +48,19 @@ rust_2018_idioms = { level = "warn", priority = -1 }
 missing_debug_implementations = "warn"
 
 [lints.clippy]
+arithmetic_side_effects = "deny"
 await_holding_lock = "warn"
 dbg_macro = "warn"
-empty_enum = "warn"
+disallowed_macros = { level = "deny", priority = 1 }
+empty_enums = "warn"
 enum_glob_use = "warn"
+expect_used = "deny"
 exit = "warn"
 filter_map_next = "warn"
 fn_params_excessive_bools = "warn"
 if_let_mutex = "warn"
 imprecise_flops = "warn"
+indexing_slicing = "deny"
 inefficient_to_string = "warn"
 linkedlist = "warn"
 lossy_float_literal = "warn"
@@ -67,13 +70,19 @@ mem_forget = "warn"
 needless_borrow = "warn"
 needless_continue = "warn"
 option_option = "warn"
+panic = "deny"
+panic_in_result_fn = "deny"
 rest_pat_in_fully_bound_structs = "warn"
 str_to_string = "warn"
 suboptimal_flops = "warn"
-todo = "warn"
+todo = "deny"
 uninlined_format_args = "warn"
+unimplemented = "deny"
+unreachable = "deny"
 unnested_or_patterns = "warn"
 unused_self = "warn"
+unwrap_in_result = "deny"
+unwrap_used = "deny"
 verbose_file_reads = "warn"
 
 large_stack_arrays = { level = "allow", priority = 1 } # https://github.com/rust-lang/rust-clippy/issues/13774
@@ -87,7 +96,7 @@ single_call_fn = { level = "allow", priority = 1 }
 similar_names = { level = "allow", priority = 1 }
 missing_docs_in_private_items = { level = "allow", priority = 1 }
 
-pedantic = { level = "warn", priority = 0 }
+pedantic = { level = "warn", priority = -1 }
 
 [package.metadata.cargo-machete]
 ignored = ["humantime-serde"]  # only used in serde macro

sync-server/build.rs

@@ -1,3 +1,4 @@
+// generated by `sqlx migrate build-script`
 fn main() {
     // trigger recompilation when a new migration is added
     println!("cargo:rerun-if-changed=migrations");

sync-server/clippy.toml

@@ -0,0 +1,3 @@
+disallowed-macros = [
+    { path = "std::eprintln", reason = "use log::info! or log::warn! instead" },
+]

sync-server/src/app_state/cursors.rs

@@ -7,16 +7,16 @@ use super::{
     database::models::{DeviceId, VaultId},
     websocket::{
         broadcasts::Broadcasts,
-        models::{
+        models::{ClientCursors, CursorPositionFromServer, WebSocketServerMessage},
-            ClientCursors, CursorPositionFromServer, WebSocketServerMessage,
-            WebSocketServerMessageWithOrigin,
-        },
     },
 };
 use crate::{
     app_state::websocket::models::DocumentWithCursors, config::database_config::DatabaseConfig,
+    errors::SyncServerError,
 };
 
+const CURSOR_CLEANUP_INTERVAL: Duration = Duration::from_secs(1);
+
 #[derive(Clone, Debug)]
 pub struct Cursors {
     config: DatabaseConfig,
@@ -39,7 +39,7 @@ impl Cursors {
         user_name: String,
         device_id: &DeviceId,
         document_to_cursors: Vec<DocumentWithCursors>,
-    ) {
+    ) -> Result<(), SyncServerError> {
         let mut vault_to_cursors = self.vault_to_cursors.lock().await;
 
         let all_device_cursors = vault_to_cursors
@@ -54,7 +54,7 @@ impl Cursors {
         }));
 
         drop(vault_to_cursors); // Explicitly drop the lock before broadcasting to avoid deadlock
-        self.broadcast_cursors_for_vault(&vault_id).await;
+        self.broadcast_cursors_for_vault(&vault_id).await
     }
 
     pub async fn get_cursors(&self, vault_id: &VaultId) -> Vec<ClientCursors> {
@@ -75,16 +75,18 @@ impl Cursors {
         tokio::spawn(async move {
             loop {
                 tokio::select! {
-                    () = tokio::time::sleep(Duration::from_secs(1)) => {
+                    () = tokio::time::sleep(CURSOR_CLEANUP_INTERVAL) => {
-                        self.remove_expired_cursors().await;
+                        self.remove_expired_cursors().await?;
                     }
                     Ok(()) = shutdown.changed() => break,
                 }
             }
+
+            Ok::<(), SyncServerError>(())
         });
     }
 
-    async fn remove_expired_cursors(&self) {
+    async fn remove_expired_cursors(&self) -> Result<(), SyncServerError> {
         let changed_vaults: Vec<VaultId> = {
             let mut vault_to_cursors = self.vault_to_cursors.lock().await;
 
@@ -104,11 +106,13 @@ impl Cursors {
         };
 
         for vault_id in &changed_vaults {
-            self.broadcast_cursors_for_vault(vault_id).await;
+            self.broadcast_cursors_for_vault(vault_id).await?;
-        }
         }
 
-    async fn broadcast_cursors_for_vault(&self, vault_id: &VaultId) {
+        Ok(())
+    }
+
+    async fn broadcast_cursors_for_vault(&self, vault_id: &VaultId) -> Result<(), SyncServerError> {
         let client_cursors: Vec<ClientCursors> = {
             let vault_to_cursors = self.vault_to_cursors.lock().await;
             vault_to_cursors
@@ -118,16 +122,18 @@ impl Cursors {
         };
 
         self.broadcasts.send_document_update(
-            vault_id.clone(),
+            vault_id,
-            WebSocketServerMessageWithOrigin::new(WebSocketServerMessage::CursorPositions(
+            WebSocketServerMessage::CursorPositions(CursorPositionFromServer {
-                CursorPositionFromServer {
                 clients: client_cursors,
-                },
+            }),
-            )),
+        )
-        );
     }
 
-    pub async fn remove_cursors_of_device(&self, vault_id: &VaultId, device_id: &DeviceId) {
+    pub async fn remove_cursors_of_device(
+        &self,
+        vault_id: &VaultId,
+        device_id: &DeviceId,
+    ) -> Result<(), SyncServerError> {
         let changed = {
             let mut vault_to_cursors = self.vault_to_cursors.lock().await;
 
@@ -145,8 +151,9 @@ impl Cursors {
         };
 
         if changed {
-            self.broadcast_cursors_for_vault(vault_id).await;
+            self.broadcast_cursors_for_vault(vault_id).await?;
         }
+        Ok(())
     }
 }
 

sync-server/src/app_state/database.rs

@@ -12,6 +12,8 @@ use models::{
 };
 use sqlx::{ConnectOptions, Connection, sqlite::SqliteConnectOptions, types::chrono::Utc};
 
+use crate::errors::{SyncServerError, database_error, server_error};
+
 pub mod models;
 
 /// Sentinel error indicating the `SQLite` database is busy (`SQLITE_BUSY`).
@@ -20,6 +22,24 @@ pub mod models;
 #[error("Database is busy")]
 pub struct WriteBusyError;
 
+/// Detects whether a `sqlx::Error` indicates the database is currently
+/// unavailable for a retryable reason: a `SQLITE_BUSY` from the engine, or
+/// a `PoolTimedOut` from our short acquire timeout. Both should surface as
+/// 429 so the client retries instead of treating it as a server fault.
+pub fn is_sqlite_busy_error(err: &sqlx::Error) -> bool {
+    match err {
+        sqlx::Error::Database(db_err) => {
+            // SQLITE_BUSY base code is 5. Extended codes share base 5.
+            let busy_by_code = db_err
+                .code()
+                .is_some_and(|c| c.parse::<u32>().is_ok_and(|n| n & 0xFF == 5));
+            busy_by_code || db_err.message().contains("database is locked")
+        }
+        sqlx::Error::PoolTimedOut => true,
+        _ => false,
+    }
+}
+
 use sqlx::{
     Pool, Sqlite, pool::PoolConnection, sqlite::SqliteConnection, sqlite::SqlitePoolOptions,
 };
@@ -29,10 +49,14 @@ use uuid::fmt::Hyphenated;
 
 use super::websocket::{
     broadcasts::Broadcasts,
-    models::{WebSocketServerMessage, WebSocketServerMessageWithOrigin, WebSocketVaultUpdate},
+    models::{WebSocketServerMessage, WebSocketVaultUpdate},
 };
 use crate::config::database_config::DatabaseConfig;
-use crate::consts::IDLE_POOL_TIMEOUT;
+use crate::consts::{IDLE_POOL_TIMEOUT, POOL_ACQUIRE_TIMEOUT};
+
+fn duration_millis_u64(duration: Duration) -> u64 {
+    u64::try_from(duration.as_millis()).unwrap_or(u64::MAX)
+}
 
 /// Holds separate reader and writer pools for a single vault.
 /// The writer pool has exactly 1 connection so writes never compete
@@ -83,22 +107,16 @@ impl WriteTransaction {
         pool: &Pool<Sqlite>,
         write_guard: tokio::sync::OwnedMutexGuard<()>,
     ) -> Result<Self> {
-        let mut conn = pool
+        let mut conn = match pool.acquire().await {
-            .acquire()
+            Ok(conn) => conn,
-            .await
+            Err(e) if is_sqlite_busy_error(&e) => return Err(WriteBusyError.into()),
-            .context("Cannot acquire connection for write transaction")?;
+            Err(e) => {
-        if let Err(e) = sqlx::query("BEGIN IMMEDIATE").execute(&mut *conn).await {
+                return Err(anyhow::Error::from(e)
-            let is_busy = match &e {
+                    .context("Cannot acquire connection for write transaction"));
-                sqlx::Error::Database(db_err) => {
-                    // SQLITE_BUSY base code is 5. Extended codes share base 5.
-                    let busy_by_code = db_err
-                        .code()
-                        .is_some_and(|c| c.parse::<u32>().is_ok_and(|n| n & 0xFF == 5));
-                    busy_by_code || db_err.message().contains("database is locked")
             }
-                _ => false,
         };
-            if is_busy {
+        if let Err(e) = sqlx::query("BEGIN IMMEDIATE").execute(&mut *conn).await {
+            if is_sqlite_busy_error(&e) {
                 return Err(WriteBusyError.into());
             }
             return Err(e).context("Cannot begin immediate transaction");
@@ -109,25 +127,33 @@ impl WriteTransaction {
         })
     }
 
-    pub async fn commit(mut self) -> Result<()> {
+    pub async fn commit(mut self) -> Result<(), SyncServerError> {
         if let Some(mut conn) = self.conn.take() {
             sqlx::query("COMMIT")
                 .execute(&mut *conn)
                 .await
-                .context("Failed to commit transaction")?;
+                .context("Failed to commit transaction")
+                .map_err(database_error)?;
         }
         Ok(())
     }
 
-    pub async fn rollback(mut self) -> Result<()> {
+    pub async fn rollback(mut self) -> Result<(), SyncServerError> {
         if let Some(mut conn) = self.conn.take() {
             sqlx::query("ROLLBACK")
                 .execute(&mut *conn)
                 .await
-                .context("Failed to rollback transaction")?;
+                .context("Failed to rollback transaction")
+                .map_err(database_error)?;
         }
         Ok(())
     }
+
+    pub fn connection_mut(&mut self) -> Result<&mut SqliteConnection> {
+        self.conn
+            .as_deref_mut()
+            .context("WriteTransaction already consumed")
+    }
 }
 
 impl Drop for WriteTransaction {
@@ -143,25 +169,6 @@ impl Drop for WriteTransaction {
     }
 }
 
-impl std::ops::Deref for WriteTransaction {
-    type Target = SqliteConnection;
-    fn deref(&self) -> &Self::Target {
-        self.conn
-            .as_ref()
-            .expect("BUG: WriteTransaction dereferenced after being consumed")
-            .deref()
-    }
-}
-
-impl std::ops::DerefMut for WriteTransaction {
-    fn deref_mut(&mut self) -> &mut Self::Target {
-        self.conn
-            .as_mut()
-            .expect("BUG: WriteTransaction dereferenced after being consumed")
-            .deref_mut()
-    }
-}
-
 /// Ensure the connection has no leftover open transaction (e.g. from a
 /// `WriteTransaction` that was dropped without commit/rollback). ROLLBACK
 /// is a harmless no-op if no transaction is active.
@@ -182,7 +189,7 @@ fn rollback_before_acquire(
 
 impl Database {
     fn now_ms(&self) -> u64 {
-        self.epoch.elapsed().as_millis() as u64
+        duration_millis_u64(self.epoch.elapsed())
     }
 
     pub async fn try_new(
@@ -274,10 +281,14 @@ impl Database {
         drop(init_conn);
 
         // Per-connection PRAGMAs shared by both reader and writer pools.
-        // journal_mode = WAL is a no-op on an already-WAL database.
+        // Database-level PRAGMAs (auto_vacuum, journal_mode) are deliberately
+        // omitted here: they require a write lock to verify or set, so issuing
+        // them on every new pool connection blocks behind any in-flight writer
+        // and can fail with SQLITE_BUSY just to open a connection. The init
+        // connection above set them once; the WAL mode persists in the database
+        // header, so subsequent opens pick it up automatically.
         let base_options = SqliteConnectOptions::new()
             .filename(file_name.clone())
-            .journal_mode(sqlx::sqlite::SqliteJournalMode::Wal)
             .busy_timeout(Duration::from_secs(30))
             .log_slow_statements(log::LevelFilter::Warn, Duration::from_secs(30))
             // In WAL mode, NORMAL is safe: data survives OS crashes, only the
@@ -301,6 +312,7 @@ impl Database {
         // Reader pool: multiple connections for concurrent reads.
         let reader = SqlitePoolOptions::new()
             .max_connections(config.max_connections_per_vault)
+            .acquire_timeout(POOL_ACQUIRE_TIMEOUT)
             .acquire_slow_threshold(Duration::from_secs(30))
             // Disabled: the health-check query is subject to busy_timeout
             // and blocks all connection checkouts when a write is active,
@@ -318,6 +330,7 @@ impl Database {
         // reader pool ensures writes never compete with reads for pool slots.
         let writer = SqlitePoolOptions::new()
             .max_connections(1)
+            .acquire_timeout(POOL_ACQUIRE_TIMEOUT)
             .acquire_slow_threshold(Duration::from_secs(30))
             .test_before_acquire(false)
             .before_acquire(rollback_before_acquire)
@@ -384,7 +397,10 @@ impl Database {
         Ok(self.get_vault_pools(vault).await?.reader)
     }
 
-    pub async fn create_write_transaction(&self, vault: &VaultId) -> Result<WriteTransaction> {
+    pub async fn create_write_transaction(
+        &self,
+        vault: &VaultId,
+    ) -> Result<WriteTransaction, SyncServerError> {
         let write_lock = {
             let mut locks = self.write_locks.lock().await;
             locks
@@ -393,8 +409,10 @@ impl Database {
                 .clone()
         };
         let write_guard = write_lock.lock_owned().await;
-        let pools = self.get_vault_pools(vault).await?;
+        let pools = self.get_vault_pools(vault).await.map_err(database_error)?;
-        WriteTransaction::new(&pools.writer, write_guard).await
+        WriteTransaction::new(&pools.writer, write_guard)
+            .await
+            .map_err(database_error)
     }
 
     /// Return the latest state of all documents in the vault, optionally
@@ -406,7 +424,7 @@ impl Database {
         vault: &VaultId,
         up_to_vault_update_id: Option<VaultUpdateId>,
         connection: Option<&mut SqliteConnection>,
-    ) -> Result<Vec<DocumentVersionWithoutContent>> {
+    ) -> Result<Vec<DocumentVersionWithoutContent>, SyncServerError> {
         // `i64::MAX` makes the upper bound a no-op for callers that don't
         // care about an exact snapshot (they pass `None`).
         let upper = up_to_vault_update_id.unwrap_or(i64::MAX);
@@ -414,7 +432,6 @@ impl Database {
             r#"
             select
                 vault_update_id,
-                creation_vault_update_id,
                 document_id as "document_id: Hyphenated",
                 relative_path,
                 updated_date as "updated_date: chrono::DateTime<Utc>",
@@ -433,7 +450,12 @@ impl Database {
             query.fetch_all(&mut *conn).await
         } else {
             query
-                .fetch_all(&self.get_connection_pool(vault).await?)
+                .fetch_all(
+                    &self
+                        .get_connection_pool(vault)
+                        .await
+                        .map_err(database_error)?,
+                )
                 .await
         }
         .context("Cannot fetch latest documents")
@@ -448,10 +470,10 @@ impl Database {
                     user_id: row.user_id,
                     device_id: row.device_id,
                     content_size: row.content_size.unwrap_or(0),
-                    is_new_file: row.creation_vault_update_id == row.vault_update_id,
                 })
                 .collect()
         })
+        .map_err(database_error)
     }
 
     /// Return the latest state of all documents (including deleted) in the
@@ -465,7 +487,7 @@ impl Database {
         vault_update_id: VaultUpdateId,
         up_to_vault_update_id: Option<VaultUpdateId>,
         connection: Option<&mut SqliteConnection>,
-    ) -> Result<Vec<DocumentVersionWithoutContent>> {
+    ) -> Result<Vec<DocumentVersionWithoutContent>, SyncServerError> {
         // `i64::MAX` makes the upper bound a no-op for callers that don't
         // care about an exact snapshot (they pass `None`).
         let upper = up_to_vault_update_id.unwrap_or(i64::MAX);
@@ -475,19 +497,14 @@ impl Database {
         // cursor capture (under broadcast send-lock) and this query
         // (which runs after drop-lock) would expose a `vault_update_id
         // > cursor` row that the cursor filter then drops, removing
-        // the doc from the catch-up entirely. The post-cursor live
+        // the doc from the catch-up entirely. Computing the snapshot
-        // broadcast then carries `is_new_file = false` (per real-time
+        // from the documents table directly with the upper bound
-        // semantics it's an update of a previously-existing version),
+        // applied at the GROUP BY layer keeps the catch-up
-        // and the receiving client — which has no record of the doc —
+        // self-contained at exactly the cursor.
-        // ignores it as stale, stranding the doc forever. Computing
-        // the snapshot from the documents table directly with the
-        // upper bound applied at the GROUP BY layer keeps the
-        // catch-up self-contained at exactly the cursor.
         let query = sqlx::query!(
             r#"
             select
                 d.vault_update_id,
-                d.creation_vault_update_id,
                 d.document_id as "document_id: Hyphenated",
                 d.relative_path,
                 d.updated_date as "updated_date: chrono::DateTime<Utc>",
@@ -515,7 +532,12 @@ impl Database {
             query.fetch_all(&mut *conn).await
         } else {
             query
-                .fetch_all(&self.get_connection_pool(vault).await?)
+                .fetch_all(
+                    &self
+                        .get_connection_pool(vault)
+                        .await
+                        .map_err(database_error)?,
+                )
                 .await
         }
         .with_context(|| {
@@ -532,27 +554,17 @@ impl Database {
                     user_id: row.user_id,
                     device_id: row.device_id,
                     content_size: row.content_size.unwrap_or(0),
-                    // For catch-up streams, "new file" means "new to this
-                    // recipient" — the doc was created past the recipient's
-                    // watermark. The catch-up only carries the doc's
-                    // *latest* version (not its full history), so using
-                    // `creation == latest` instead would mis-flag every
-                    // doc that was created and then updated before the
-                    // client reconnected, and the client's
-                    // `processRemoteChange` would drop it as "stale
-                    // RemoteChange for untracked, non-new document",
-                    // silently leaking docs to clients catching up.
-                    is_new_file: row.creation_vault_update_id > vault_update_id,
                 })
                 .collect()
         })
+        .map_err(database_error)
     }
 
     pub async fn get_max_update_id_in_vault(
         &self,
         vault: &VaultId,
         connection: Option<&mut SqliteConnection>,
-    ) -> Result<i64> {
+    ) -> Result<i64, SyncServerError> {
         let query = sqlx::query!(
             r#"
             select coalesce(max(vault_update_id), 0) as max_vault_update_id
@@ -564,11 +576,17 @@ impl Database {
             query.fetch_one(&mut *conn).await
         } else {
             query
-                .fetch_one(&self.get_connection_pool(vault).await?)
+                .fetch_one(
+                    &self
+                        .get_connection_pool(vault)
+                        .await
+                        .map_err(database_error)?,
+                )
                 .await
         }
         .map(|row| row.max_vault_update_id)
         .context("Cannot fetch max update id in vault")
+        .map_err(database_error)
     }
 
     pub async fn get_latest_non_deleted_document_by_path(
@@ -576,7 +594,7 @@ impl Database {
         vault: &VaultId,
         relative_path: &str,
         connection: Option<&mut SqliteConnection>,
-    ) -> Result<Option<StoredDocumentVersion>> {
+    ) -> Result<Option<StoredDocumentVersion>, SyncServerError> {
         let query = sqlx::query_as!(
             StoredDocumentVersion,
             r#"
@@ -605,10 +623,16 @@ impl Database {
             query.fetch_optional(&mut *conn).await
         } else {
             query
-                .fetch_optional(&self.get_connection_pool(vault).await?)
+                .fetch_optional(
+                    &self
+                        .get_connection_pool(vault)
+                        .await
+                        .map_err(database_error)?,
+                )
                 .await
         }
         .context("Cannot fetch latest document version")
+        .map_err(database_error)
     }
 
     /// Find a doc whose CREATE was authored by this device with
@@ -638,7 +662,7 @@ impl Database {
         last_seen_vault_update_id: VaultUpdateId,
         content: &[u8],
         connection: Option<&mut SqliteConnection>,
-    ) -> Result<Option<StoredDocumentVersion>> {
+    ) -> Result<Option<StoredDocumentVersion>, SyncServerError> {
         let query = sqlx::query_as!(
             StoredDocumentVersion,
             r#"
@@ -673,10 +697,16 @@ impl Database {
             query.fetch_optional(&mut *conn).await
         } else {
             query
-                .fetch_optional(&self.get_connection_pool(vault).await?)
+                .fetch_optional(
+                    &self
+                        .get_connection_pool(vault)
+                        .await
+                        .map_err(database_error)?,
+                )
                 .await
         }
         .context("Cannot fetch lost-create candidate")
+        .map_err(database_error)
     }
 
     pub async fn get_latest_document(
@@ -684,7 +714,7 @@ impl Database {
         vault: &VaultId,
         document_id: &DocumentId,
         connection: Option<&mut SqliteConnection>,
-    ) -> Result<Option<StoredDocumentVersion>> {
+    ) -> Result<Option<StoredDocumentVersion>, SyncServerError> {
         let document_id = document_id.as_hyphenated();
         let query = sqlx::query_as!(
             StoredDocumentVersion,
@@ -710,10 +740,16 @@ impl Database {
             query.fetch_optional(&mut *conn).await
         } else {
             query
-                .fetch_optional(&self.get_connection_pool(vault).await?)
+                .fetch_optional(
+                    &self
+                        .get_connection_pool(vault)
+                        .await
+                        .map_err(database_error)?,
+                )
                 .await
         }
         .context("Cannot fetch latest document version")
+        .map_err(database_error)
     }
 
     pub async fn get_document_version(
@@ -721,7 +757,7 @@ impl Database {
         vault: &VaultId,
         vault_update_id: VaultUpdateId,
         connection: Option<&mut SqliteConnection>,
-    ) -> Result<Option<StoredDocumentVersion>> {
+    ) -> Result<Option<StoredDocumentVersion>, SyncServerError> {
         let query = sqlx::query_as!(
             StoredDocumentVersion,
             r#"
@@ -745,10 +781,16 @@ impl Database {
             query.fetch_optional(&mut *conn).await
         } else {
             query
-                .fetch_optional(&self.get_connection_pool(vault).await?)
+                .fetch_optional(
+                    &self
+                        .get_connection_pool(vault)
+                        .await
+                        .map_err(database_error)?,
+                )
                 .await
         }
         .context("Cannot fetch document version")
+        .map_err(database_error)
     }
 
     // inserting the document must be the last step of the transaction
@@ -757,7 +799,7 @@ impl Database {
         vault_id: &VaultId,
         version: &StoredDocumentVersion,
         mut transaction: WriteTransaction,
-    ) -> Result<()> {
+    ) -> Result<(), SyncServerError> {
         let document_id = version.document_id.as_hyphenated();
         let query = sqlx::query!(
             r#"
@@ -793,32 +835,26 @@ impl Database {
         let _send_guard = self.broadcasts.acquire_send_lock(vault_id).await;
 
         query
-            .execute(&mut *transaction)
+            .execute(transaction.connection_mut().map_err(server_error)?)
             .await
-            .context("Cannot insert document version")?;
+            .context("Cannot insert document version")
+            .map_err(database_error)?;
 
-        transaction
+        transaction.commit().await?;
-            .commit()
-            .await
-            .context("Failed to commit transaction")?;
 
-        // For non-delete writes the originating device already has
+        // Broadcast every commit to every connected client, including
-        // authoritative state from its HTTP response, so we tag the
+        // the originator. The HTTP response is the originator's normal
-        // broadcast with `origin_device_id` and the send task in
+        // path to learn its own update, but if the response is lost
-        // `websocket.rs` filters it out for that device. Deletes are
+        // (sync reset, dropped TCP) the broadcast is the only remaining
-        // delivered to *every* connected client including the author —
+        // delivery channel — and the client-side `parentVersionId`
-        // the originator only removes the document from its sync queue
+        // dedup absorbs the redundant message when the response made it
-        // once it receives this receipt.
+        // through.
-        let envelope = WebSocketServerMessage::VaultUpdate(WebSocketVaultUpdate {
+        self.broadcasts.send_document_update(
+            vault_id,
+            WebSocketServerMessage::VaultUpdate(WebSocketVaultUpdate {
                 document: version.clone().into(),
-        });
+            }),
-        let with_origin = if version.is_deleted {
+        )?;
-            WebSocketServerMessageWithOrigin::new(envelope)
-        } else {
-            WebSocketServerMessageWithOrigin::with_origin(version.device_id.clone(), envelope)
-        };
-        self.broadcasts
-            .send_document_update(vault_id.clone(), with_origin);
 
         Ok(())
     }
@@ -831,7 +867,7 @@ impl Database {
         let idle_pools: Vec<(VaultId, Arc<VaultPool>)> = {
             let mut pools = self.connection_pools.lock().await;
             let now_ms = self.now_ms();
-            let idle_threshold_ms = IDLE_POOL_TIMEOUT.as_millis() as u64;
+            let idle_threshold_ms = duration_millis_u64(IDLE_POOL_TIMEOUT);
 
             let vaults_to_remove: Vec<VaultId> = pools
                 .iter()

sync-server/src/app_state/database/models.rs

@@ -46,14 +46,10 @@ pub struct DocumentVersionWithoutContent {
 
     #[ts(type = "number")]
     pub content_size: u64,
-
-    /// True iff this is the first version of the document
-    pub is_new_file: bool,
 }
 
 impl From<StoredDocumentVersion> for DocumentVersionWithoutContent {
     fn from(value: StoredDocumentVersion) -> Self {
-        let is_new_file = value.creation_vault_update_id == value.vault_update_id;
         Self {
             vault_update_id: value.vault_update_id,
             document_id: value.document_id,
@@ -63,7 +59,6 @@ impl From<StoredDocumentVersion> for DocumentVersionWithoutContent {
             user_id: value.user_id,
             device_id: value.device_id,
             content_size: value.content.len() as u64,
-            is_new_file,
         }
     }
 }
@@ -83,7 +78,6 @@ pub struct DocumentVersion {
     pub device_id: DeviceId,
 }
 
-
 impl From<StoredDocumentVersion> for DocumentVersion {
     fn from(value: StoredDocumentVersion) -> Self {
         Self {

sync-server/src/app_state/websocket/broadcasts.rs

@@ -3,11 +3,15 @@ use std::{
     sync::{Arc, Mutex as StdMutex},
 };
 
-use log::{debug, info, warn};
+use log::{debug, warn};
 use tokio::sync::{Mutex, broadcast};
 
-use super::models::{WebSocketServerMessage, WebSocketServerMessageWithOrigin};
+use super::models::WebSocketServerMessage;
-use crate::{app_state::database::models::VaultId, config::server_config::ServerConfig};
+use crate::{
+    app_state::database::models::VaultId,
+    config::server_config::ServerConfig,
+    errors::{SyncServerError, client_error, server_error},
+};
 
 #[derive(Debug, Clone)]
 pub struct Broadcasts {
@@ -17,11 +21,11 @@ pub struct Broadcasts {
     // this non-async lets `send_document_update` run without an `.await`,
     // so an axum handler that is cancelled between `transaction.commit()`
     // and the broadcast can never drop the notification mid-flight.
-    tx: Arc<StdMutex<HashMap<VaultId, broadcast::Sender<WebSocketServerMessageWithOrigin>>>>,
+    tx: Arc<StdMutex<HashMap<VaultId, broadcast::Sender<WebSocketServerMessage>>>>,
     send_locks: Arc<Mutex<HashMap<VaultId, Arc<tokio::sync::Mutex<()>>>>>,
 }
 
-type TxMap = HashMap<VaultId, broadcast::Sender<WebSocketServerMessageWithOrigin>>;
+type TxMap = HashMap<VaultId, broadcast::Sender<WebSocketServerMessage>>;
 
 impl Broadcasts {
     pub fn new(server_config: &ServerConfig) -> Self {
@@ -60,88 +64,64 @@ impl Broadcasts {
 
     pub fn get_receiver(
         &self,
-        vault: VaultId,
+        vault: &VaultId,
         max_clients: usize,
-    ) -> Result<broadcast::Receiver<WebSocketServerMessageWithOrigin>, crate::errors::SyncServerError>
+    ) -> Result<broadcast::Receiver<WebSocketServerMessage>, SyncServerError> {
-    {
         let mut tx_map = self
             .tx
             .lock()
-            .expect("broadcasts.tx mutex poisoned — a previous holder panicked");
+            .map_err(|_| server_error(anyhow::anyhow!("broadcasts.tx mutex poisoned")))?;
 
-        let count_before_prune = tx_map
+        Self::prune_inactive_vaults(&mut tx_map);
-            .get(&vault)
-            .map_or(0, tokio::sync::broadcast::Sender::receiver_count);
-        let pruned = Self::prune_inactive_vaults(&mut tx_map);
-        let pruned_self = pruned.contains(&vault);
 
         let sender = tx_map
-            .entry(vault.clone())
+            .entry(vault.to_owned())
             .or_insert_with(|| broadcast::channel(self.broadcast_channel_capacity).0);
 
         // Hold the lock across the count check *and* the subscribe so the
         // `max_clients` cap is atomic: two concurrent callers can't both
         // observe `receiver_count() < max_clients` and both subscribe.
         if sender.receiver_count() >= max_clients {
-            return Err(crate::errors::client_error(anyhow::anyhow!(
+            return Err(client_error(anyhow::anyhow!(
                 "Vault has reached the maximum number of clients ({max_clients})"
             )));
         }
 
         let receiver = sender.subscribe();
-        let count_after = sender.receiver_count();
-        info!(
-            "[BCAST] get_receiver vault={vault} count_before_prune={count_before_prune} pruned_self={pruned_self} pruned_total={} count_after_subscribe={count_after}",
-            pruned.len()
-        );
         Ok(receiver)
     }
 
     /// Notify all clients (who are subscribed to the vault) about an update.
     /// Synchronous: safe to invoke from a handler between `commit()` and
     /// function return without worrying about task cancellation dropping
-    /// the broadcast mid-flight. Failures are logged, never propagated.
+    /// the broadcast mid-flight. Mutex poison is returned; send failures
-    pub fn send_document_update(&self, vault: VaultId, document: WebSocketServerMessageWithOrigin) {
+    /// are logged because they can happen when receivers disconnect.
-        let vault_update_id = match &document.message {
+    pub fn send_document_update(
-            WebSocketServerMessage::VaultUpdate(u) => Some(u.document.vault_update_id),
+        &self,
-            WebSocketServerMessage::CursorPositions(_) => None,
+        vault: &str,
-        };
+        document: WebSocketServerMessage,
-        let is_deleted = match &document.message {
+    ) -> Result<(), SyncServerError> {
-            WebSocketServerMessage::VaultUpdate(u) => Some(u.document.is_deleted),
+        let mut tx_map = self.tx.lock().map_err(|_| {
-            WebSocketServerMessage::CursorPositions(_) => None,
+            server_error(anyhow::anyhow!(
-        };
+                "broadcasts.tx mutex poisoned; skipping document update broadcast"
-        let mut tx_map = self
+            ))
-            .tx
+        })?;
-            .lock()
+        Self::prune_inactive_vaults(&mut tx_map);
-            .expect("broadcasts.tx mutex poisoned — a previous holder panicked");
-        let count_before_prune = tx_map
-            .get(&vault)
-            .map_or(0, tokio::sync::broadcast::Sender::receiver_count);
-        let pruned = Self::prune_inactive_vaults(&mut tx_map);
-        let pruned_self = pruned.contains(&vault);
 
         let sender = tx_map
-            .entry(vault.clone())
+            .entry(vault.to_owned())
             .or_insert_with(|| broadcast::channel(self.broadcast_channel_capacity).0);
 
         let count_before_send = sender.receiver_count();
 
         if count_before_send == 0 {
-            info!(
-                "[BCAST] send_document_update vault={vault} vuid={vault_update_id:?} is_deleted={is_deleted:?} count_before_prune={count_before_prune} pruned_self={pruned_self} count_before_send=0 SKIPPED"
-            );
             debug!("Skipping broadcast, no clients connected for vault `{vault}`");
-            return;
+            return Ok(());
         }
 
-        let send_result = sender.send(document);
+        if let Err(e) = sender.send(document) {
-        match &send_result {
+            warn!("Failed to send document update broadcast: {e}");
-            Ok(n) => info!(
+        }
-                "[BCAST] send_document_update vault={vault} vuid={vault_update_id:?} is_deleted={is_deleted:?} count_before_prune={count_before_prune} pruned_self={pruned_self} count_before_send={count_before_send} SENT delivered_to={n}"
+        Ok(())
-            ),
-            Err(e) => warn!(
-                "[BCAST] send_document_update vault={vault} vuid={vault_update_id:?} is_deleted={is_deleted:?} count_before_prune={count_before_prune} pruned_self={pruned_self} count_before_send={count_before_send} FAILED err={e}"
-            ),
-        }
     }
 }

sync-server/src/app_state/websocket/models.rs

@@ -58,15 +58,6 @@ pub struct CursorPositionFromServer {
     pub clients: Vec<ClientCursors>,
 }
 
-// One committed version. Non-delete updates are broadcast to every
-// connected client *except* the device that authored them — that
-// device already has the new state via its HTTP response. Deletes are
-// broadcast to every client including the author: the author keeps
-// the document in its sync queue until this receipt arrives so a late
-// remote update can't sneak in between the HTTP response and the
-// queue cleanup. The server also emits these one-at-a-time to catch
-// up a freshly-connected client on versions committed while it was
-// offline, in ascending `vault_update_id` order.
 #[derive(TS, Serialize, Clone, Debug)]
 #[serde(rename_all = "camelCase")]
 pub struct WebSocketVaultUpdate {
@@ -88,29 +79,3 @@ pub enum WebSocketServerMessage {
     VaultUpdate(WebSocketVaultUpdate),
     CursorPositions(CursorPositionFromServer),
 }
-
-/// Broadcast envelope carrying the message plus the device that produced
-/// it. The per-recipient send task compares `origin_device_id` against
-/// its own device id to fill in `originates_from_self` before the message
-/// is serialized on the wire.
-#[derive(Clone, Debug)]
-pub struct WebSocketServerMessageWithOrigin {
-    pub origin_device_id: Option<DeviceId>,
-    pub message: WebSocketServerMessage,
-}
-
-impl WebSocketServerMessageWithOrigin {
-    pub fn new(message: WebSocketServerMessage) -> Self {
-        Self {
-            origin_device_id: None,
-            message,
-        }
-    }
-
-    pub fn with_origin(origin_device_id: DeviceId, message: WebSocketServerMessage) -> Self {
-        Self {
-            origin_device_id: Some(origin_device_id),
-            message,
-        }
-    }
-}

sync-server/src/app_state/websocket/utils.rs

@@ -10,7 +10,7 @@ use crate::{
     },
     config::user_config::User,
     errors::{SyncServerError, client_error, server_error, unauthenticated_error},
-    server::auth::auth,
+    server::auth::authenticate_for_vault,
 };
 
 pub struct AuthenticatedWebSocketHandshake {
@@ -30,7 +30,7 @@ pub fn get_authenticated_handshake(
 
         match message {
             WebSocketClientMessage::Handshake(handshake) => {
-                let user = auth(state, handshake.token.trim(), vault_id)?;
+                let user = authenticate_for_vault(state, handshake.token.trim(), vault_id)?;
                 Ok(AuthenticatedWebSocketHandshake { handshake, user })
             }
             WebSocketClientMessage::CursorPositions(_) => Err(unauthenticated_error(
@@ -51,6 +51,9 @@ pub fn get_authenticated_handshake(
 /// vault send lock; commits past the cursor are then delivered solely
 /// through the broadcast channel (filtered by the same cursor on the
 /// receive side), so every committed update is delivered exactly once.
+/// We could've used a read transaction but that would've meant all other
+/// clients would need to wait for the new client to catch up before
+/// sending any updates.
 pub async fn get_unseen_documents(
     state: &AppState,
     vault_id: &VaultId,
@@ -62,13 +65,11 @@ pub async fn get_unseen_documents(
             .database
             .get_latest_documents_since(vault_id, update_id, Some(up_to_vault_update_id), None)
             .await
-            .map_err(server_error)
     } else {
         state
             .database
             .get_latest_documents(vault_id, Some(up_to_vault_update_id), None)
             .await
-            .map_err(server_error)
     }
 }
 

sync-server/src/cli/color_when.rs

@@ -23,9 +23,10 @@ impl ColorWhen {
 
 impl std::fmt::Display for ColorWhen {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        self.to_possible_value()
+        f.write_str(match self {
-            .expect("no values are skipped")
+            Self::Always => "always",
-            .get_name()
+            Self::Auto => "auto",
-            .fmt(f)
+            Self::Never => "never",
+        })
     }
 }

sync-server/src/config/server_config.rs

@@ -71,6 +71,10 @@ impl ServerConfig {
             self.max_pending_websocket_connections > 0,
             "max_pending_websocket_connections must be greater than 0"
         );
+        ensure!(
+            self.rate_limit_per_user_per_second != Some(0),
+            "rate_limit_per_user_per_second must be greater than 0 when set (use null to disable rate limiting)"
+        );
 
         Ok(())
     }

sync-server/src/config/user_config.rs

@@ -20,15 +20,7 @@ where
     let mut user_token_map = BiHashMap::new();
     for user in &users {
         if let Some(existing_name) = user_token_map.get_by_right(&user.token) {
-            let redacted = if user.token.len() > 6 {
+            let redacted = redact_token(&user.token);
-                format!(
-                    "{}...{}",
-                    &user.token[..3],
-                    &user.token[user.token.len() - 3..]
-                )
-            } else {
-                "***".to_owned()
-            };
             return Err(D::Error::custom(format!(
                 "Duplicate user token found: `{redacted}` for users `{}` and `{}`. User tokens \
                 must be unique.",
@@ -49,6 +41,23 @@ where
     Ok(users)
 }
 
+fn redact_token(token: &str) -> String {
+    if token.chars().count() <= 6 {
+        return "***".to_owned();
+    }
+
+    let prefix = token.chars().take(3).collect::<String>();
+    let suffix = token
+        .chars()
+        .rev()
+        .take(3)
+        .collect::<Vec<_>>()
+        .into_iter()
+        .rev()
+        .collect::<String>();
+    format!("{prefix}...{suffix}")
+}
+
 impl UserConfig {
     pub fn get_user(&self, token: &str) -> Option<&User> {
         self.user_configs

sync-server/src/consts.rs

@@ -21,6 +21,10 @@ pub const DEFAULT_MAX_PENDING_WS_CONNECTIONS: usize = 128;
 pub const DEFAULT_LOG_DIRECTORY: &str = "logs";
 pub const DEFAULT_LOG_ROTATION_INTERVAL: Duration = Duration::from_hours(24);
 pub const IDLE_POOL_TIMEOUT: Duration = Duration::from_mins(5);
+
+/// Fail fast on pool acquire so a transiently locked database surfaces as
+/// a 429 in seconds, not after a 30s `busy_timeout`. Callers retry.
+pub const POOL_ACQUIRE_TIMEOUT: Duration = Duration::from_secs(5);
 pub const GRACEFUL_SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(10);
 pub const HANDSHAKE_TIMEOUT: Duration = Duration::from_secs(10);
 

sync-server/src/errors.rs

@@ -79,10 +79,7 @@ impl IntoResponse for SyncServerError {
             Self::InitError(_) | Self::ServerError(_) => {
                 error!("{serialized}");
             }
-            Self::ClientError(_) | Self::NotFound(_) => {
+            Self::ClientError(_) | Self::NotFound(_) | Self::TooManyRequests(_) => {
-                warn!("{serialized}");
-            }
-            Self::TooManyRequests(_) => {
                 warn!("{serialized}");
             }
             Self::Unauthenticated(_) | Self::PermissionDeniedError(_) => {}
@@ -166,15 +163,26 @@ pub fn too_many_requests_error(error: anyhow::Error) -> SyncServerError {
     SyncServerError::TooManyRequests(error)
 }
 
-/// Maps a `create_write_transaction` error to 429 if the database is busy,
+/// Maps a database-operation error to 429 if the database is busy or the
-/// or 500 for all other failures.
+/// pool acquire timed out (both retryable), or 500 for all other failures.
-pub fn write_transaction_error(error: anyhow::Error) -> SyncServerError {
+pub fn database_error(error: anyhow::Error) -> SyncServerError {
-    if error
+    if is_database_busy(&error) {
-        .downcast_ref::<crate::app_state::database::WriteBusyError>()
-        .is_some()
-    {
         too_many_requests_error(error)
     } else {
         server_error(error)
     }
 }
+
+fn is_database_busy(error: &anyhow::Error) -> bool {
+    if error
+        .downcast_ref::<crate::app_state::database::WriteBusyError>()
+        .is_some()
+    {
+        return true;
+    }
+    error.chain().any(|cause| {
+        cause
+            .downcast_ref::<sqlx::Error>()
+            .is_some_and(crate::app_state::database::is_sqlite_busy_error)
+    })
+}

sync-server/src/main.rs

@@ -1,3 +1,19 @@
+#![cfg_attr(
+    test,
+    allow(
+        clippy::arithmetic_side_effects,
+        clippy::expect_used,
+        clippy::indexing_slicing,
+        clippy::panic,
+        clippy::panic_in_result_fn,
+        clippy::todo,
+        clippy::unimplemented,
+        clippy::unreachable,
+        clippy::unwrap_in_result,
+        clippy::unwrap_used
+    )
+)]
+
 mod app_state;
 mod cli;
 mod config;
@@ -14,7 +30,7 @@ use cli::args::Args;
 use config::Config;
 use consts::DEFAULT_CONFIG_PATH;
 use errors::{SyncServerError, init_error};
-use log::info;
+use log::{error, info, warn};
 use server::create_server;
 use tracing_appender::non_blocking::WorkerGuard;
 use tracing_subscriber::{EnvFilter, fmt::format, layer::SubscriberExt, util::SubscriberInitExt};
@@ -36,30 +52,63 @@ async fn main() -> ExitCode {
         .map_err(init_error)
     {
         Ok(config) => config,
-        Err(e) => {
+        Err(error) => {
-            eprintln!("{}", e.serialize());
+            return exit_with_startup_error(&args, &error);
-            return ExitCode::FAILURE;
         }
     };
 
-    let result = async {
+    if let Err(error) = config.validate().map_err(init_error) {
-        config.validate().map_err(init_error)?;
+        return exit_with_startup_error(&args, &error);
-        // Hold the non-blocking writer guards until shutdown so the
-        // dedicated writer threads stay alive and flush queued log lines.
-        let _log_guards = set_up_logging(&args, &config.logging)?;
-        start_server(config).await
     }
-    .await;
 
-    match result {
+    // Hold the non-blocking writer guards until shutdown so the dedicated
+    // writer threads stay alive and flush queued log lines.
+    let _log_guards = match set_up_logging(&args, &config.logging) {
+        Ok(log_guards) => log_guards,
+        Err(error) => {
+            return exit_with_startup_error(&args, &error);
+        }
+    };
+
+    match start_server(config).await {
         Ok(()) => ExitCode::SUCCESS,
-        Err(e) => {
+        Err(error) => {
-            eprintln!("{}", e.serialize());
+            let serialized = error.serialize();
+            warn!("{serialized}");
             ExitCode::FAILURE
         }
     }
 }
 
+fn exit_with_startup_error(args: &Args, err: &SyncServerError) -> ExitCode {
+    let _ = set_up_stderr_logging(args);
+
+    let serialized = err.serialize();
+    error!("{serialized}");
+
+    ExitCode::FAILURE
+}
+
+fn set_up_stderr_logging(args: &Args) -> Result<(), SyncServerError> {
+    let env_filter = EnvFilter::builder()
+        .with_default_directive(tracing::Level::WARN.into())
+        .from_env()
+        .context("Failed to create logging env filter")
+        .map_err(init_error)?;
+
+    let stderr_layer = tracing_subscriber::fmt::layer()
+        .with_ansi(args.color.use_colors())
+        .with_writer(std::io::stderr)
+        .event_format(format().compact());
+
+    tracing_subscriber::registry()
+        .with(env_filter)
+        .with(stderr_layer)
+        .try_init()
+        .context("Failed to initialise fallback tracing")
+        .map_err(init_error)
+}
+
 fn set_up_logging(
     args: &Args,
     logging_config: &config::logging_config::LoggingConfig,

sync-server/src/server.rs

@@ -4,8 +4,6 @@ mod delete_document;
 mod device_id_header;
 mod fetch_document_version;
 mod fetch_document_version_content;
-mod fetch_latest_document_version;
-mod fetch_latest_documents;
 mod index;
 mod ping;
 mod rate_limit;
@@ -14,13 +12,14 @@ mod responses;
 mod update_document;
 mod websocket;
 
-use anyhow::{Context as _, Result};
+use anyhow::{Context as _, Result, anyhow};
 use auth::auth_middleware;
 use axum::{
     Router,
     extract::{DefaultBodyLimit, Request},
     http::{self, HeaderValue, Method},
     middleware,
+    response::IntoResponse,
     routing::{IntoMakeService, delete, get, post, put},
 };
 use device_id_header::DEVICE_ID_HEADER_NAME;
@@ -42,6 +41,7 @@ use crate::{
     app_state::AppState,
     config::{Config, server_config::ServerConfig},
     consts::GRACEFUL_SHUTDOWN_TIMEOUT,
+    errors::not_found_error,
 };
 
 pub async fn create_server(config: Config) -> Result<()> {
@@ -71,7 +71,13 @@ pub async fn create_server(config: Config) -> Result<()> {
     let app = app
         .layer(DefaultBodyLimit::disable())
         .layer(RequestBodyLimitLayer::new(
-            app_state.config.server.max_body_size_mb * 1024 * 1024,
+            app_state
+                .config
+                .server
+                .max_body_size_mb
+                .checked_mul(1024)
+                .and_then(|kb| kb.checked_mul(1024))
+                .context("max_body_size_mb is too large")?,
         ))
         .layer(TimeoutLayer::new(server_config.response_timeout))
         .layer(cors_layer)
@@ -95,6 +101,7 @@ pub async fn create_server(config: Config) -> Result<()> {
                 .on_failure(DefaultOnFailure::new().level(Level::ERROR)),
         )
         .with_state(app_state.clone())
+        .fallback(handle_404)
         .into_make_service();
 
     start_server(app, &server_config, app_state).await
@@ -103,7 +110,7 @@ pub async fn create_server(config: Config) -> Result<()> {
 fn build_cors_layer(server_config: &ServerConfig) -> Result<CorsLayer> {
     let origins = &server_config.allowed_origins;
 
-    let cors = if origins.len() == 1 && origins[0] == "*" {
+    let cors = if origins.len() == 1 && origins.first().is_some_and(|origin| origin == "*") {
         info!("CORS: allowing all origins");
         let header: HeaderValue = "*"
             .parse()
@@ -131,18 +138,10 @@ fn build_cors_layer(server_config: &ServerConfig) -> Result<CorsLayer> {
 
 fn get_authed_routes(app_state: AppState) -> Router<AppState> {
     Router::new()
-        .route(
-            "/vaults/:vault_id/documents",
-            get(fetch_latest_documents::fetch_latest_documents),
-        )
         .route(
             "/vaults/:vault_id/documents",
             post(create_document::create_document),
         )
-        .route(
-            "/vaults/:vault_id/documents/:document_id",
-            get(fetch_latest_document_version::fetch_latest_document_version),
-        )
         .route(
             "/vaults/:vault_id/documents/:document_id/binary",
             put(update_document::update_binary),
@@ -233,3 +232,7 @@ async fn shutdown_signal() {
         () = terminate => {},
     }
 }
+
+async fn handle_404() -> impl IntoResponse {
+    not_found_error(anyhow!("Endpoint not found"))
+}

sync-server/src/server/auth.rs

@@ -34,7 +34,7 @@ pub async fn auth_middleware(
             .ok_or_else(|| unauthenticated_error(anyhow::anyhow!("Missing vault_id")))?,
     );
 
-    let user = auth(&state, token, &vault_id)?;
+    let user = authenticate_for_vault(&state, token, &vault_id)?;
 
     req.extensions_mut().insert(user);
 
@@ -50,7 +50,11 @@ pub fn authenticate(state: &AppState, token: &str) -> Result<User, SyncServerErr
         .ok_or_else(|| unauthenticated_error(anyhow::anyhow!("Invalid token")))
 }
 
-pub fn auth(state: &AppState, token: &str, vault_id: &VaultId) -> Result<User, SyncServerError> {
+pub fn authenticate_for_vault(
+    state: &AppState,
+    token: &str,
+    vault_id: &VaultId,
+) -> Result<User, SyncServerError> {
     let user = authenticate(state, token)?;
 
     if match user.vault_access {

sync-server/src/server/create_document.rs

@@ -14,7 +14,7 @@ use crate::{
         database::models::{StoredDocumentVersion, VaultId},
     },
     config::user_config::User,
-    errors::{SyncServerError, client_error, server_error, write_transaction_error},
+    errors::{SyncServerError, client_error, server_error},
     server::{responses::DocumentUpdateResponse, update_document},
     utils::{
         find_first_available_path::find_first_available_path, is_binary::is_binary,
@@ -49,8 +49,7 @@ pub async fn create_document(
     let mut transaction = state
         .database
         .create_write_transaction(&vault_id)
-        .await
+        .await?;
-        .map_err(write_transaction_error)?;
 
     let sanitized_relative_path = sanitize_path(&request.relative_path).map_err(client_error)?;
     let new_content = request.content.contents.to_vec();
@@ -60,10 +59,9 @@ pub async fn create_document(
         .get_latest_non_deleted_document_by_path(
             &vault_id,
             &sanitized_relative_path,
-            Some(&mut *transaction),
+            Some(transaction.connection_mut().map_err(server_error)?),
         )
-        .await
+        .await?;
-        .map_err(server_error)?;
 
     if let Some(latest_version) = latest_version {
         // Only merge with an existing document the client couldn't have
@@ -129,16 +127,13 @@ pub async fn create_document(
             &device_id.0,
             request.last_seen_vault_update_id,
             &new_content,
-            Some(&mut *transaction),
+            Some(transaction.connection_mut().map_err(server_error)?),
         )
-        .await
+        .await?
-        .map_err(server_error)?
     {
         info!(
             "Lost-create recovery: binding retry at `{sanitized_relative_path}` to existing doc {} (was at `{}`) in vault `{vault_id}` for device `{}`",
-            lost_create.document_id,
+            lost_create.document_id, lost_create.relative_path, device_id.0
-            lost_create.relative_path,
-            device_id.0
         );
         return update_document::update_document(
             &sanitized_relative_path,
@@ -159,9 +154,11 @@ pub async fn create_document(
 
     let last_update_id = state
         .database
-        .get_max_update_id_in_vault(&vault_id, Some(&mut *transaction))
+        .get_max_update_id_in_vault(
-        .await
+            &vault_id,
-        .map_err(server_error)?;
+            Some(transaction.connection_mut().map_err(server_error)?),
+        )
+        .await?;
 
     let deduped_path = find_first_available_path(
         &vault_id,
@@ -169,8 +166,7 @@ pub async fn create_document(
         &state.database,
         &mut transaction,
     )
-    .await
+    .await?;
-    .map_err(server_error)?;
 
     if deduped_path != sanitized_relative_path {
         info!(
@@ -178,7 +174,9 @@ pub async fn create_document(
         );
     }
 
-    let new_vault_update_id = last_update_id + 1;
+    let new_vault_update_id = last_update_id
+        .checked_add(1)
+        .ok_or_else(|| server_error(anyhow::anyhow!("Vault update id overflow")))?;
     let new_version = StoredDocumentVersion {
         vault_update_id: new_vault_update_id,
         creation_vault_update_id: new_vault_update_id,
@@ -195,8 +193,7 @@ pub async fn create_document(
     state
         .database
         .insert_document_version(&vault_id, &new_version, transaction)
-        .await
+        .await?;
-        .map_err(server_error)?;
 
     Ok(Json(DocumentUpdateResponse::FastForwardUpdate(
         new_version.into(),

sync-server/src/server/delete_document.rs

@@ -1,4 +1,4 @@
-use anyhow::{Context, anyhow};
+use anyhow::anyhow;
 use axum::{
     Extension, Json,
     extract::{Path, State},
@@ -16,7 +16,7 @@ use crate::{
         },
     },
     config::user_config::User,
-    errors::{SyncServerError, not_found_error, server_error, write_transaction_error},
+    errors::{SyncServerError, not_found_error, server_error},
     utils::normalize::normalize,
 };
 
@@ -43,44 +43,42 @@ pub async fn delete_document(
     let mut transaction = state
         .database
         .create_write_transaction(&vault_id)
-        .await
+        .await?;
-        .map_err(write_transaction_error)?;
 
     let last_update_id = state
         .database
-        .get_max_update_id_in_vault(&vault_id, Some(&mut transaction))
+        .get_max_update_id_in_vault(
-        .await
+            &vault_id,
-        .map_err(server_error)?;
+            Some(transaction.connection_mut().map_err(server_error)?),
+        )
+        .await?;
 
     let latest_version = state
         .database
-        .get_latest_document(&vault_id, &document_id, Some(&mut transaction))
+        .get_latest_document(
-        .await
+            &vault_id,
-        .map_err(server_error)?;
+            &document_id,
+            Some(transaction.connection_mut().map_err(server_error)?),
+        )
+        .await?;
 
     let Some(latest_version) = latest_version else {
-        transaction
+        transaction.rollback().await?;
-            .rollback()
-            .await
-            .context("Failed to roll back transaction")
-            .map_err(server_error)?;
         return Err(not_found_error(anyhow!(
             "Document `{document_id}` not found in vault `{vault_id}`"
         )));
     };
 
     if latest_version.is_deleted {
-        transaction
+        transaction.rollback().await?;
-            .rollback()
-            .await
-            .context("Failed to roll back transaction")
-            .map_err(server_error)?;
 
         info!("Document `{document_id}` has already been deleted",);
         return Ok(Json(latest_version.into()));
     }
 
-    let new_vault_update_id = last_update_id + 1;
+    let new_vault_update_id = last_update_id
+        .checked_add(1)
+        .ok_or_else(|| server_error(anyhow!("Vault update id overflow")))?;
     let latest_relative_path = latest_version.relative_path;
     let latest_content = latest_version.content;
     let creation_vault_update_id = latest_version.creation_vault_update_id;
@@ -101,8 +99,7 @@ pub async fn delete_document(
     state
         .database
         .insert_document_version(&vault_id, &new_version, transaction)
-        .await
+        .await?;
-        .map_err(server_error)?;
 
     Ok(Json(new_version.into()))
 }

sync-server/src/server/fetch_document_version.rs

@@ -11,7 +11,7 @@ use crate::{
         AppState,
         database::models::{DocumentId, DocumentVersion, VaultId, VaultUpdateId},
     },
-    errors::{SyncServerError, client_error, not_found_error, server_error},
+    errors::{SyncServerError, not_found_error},
     utils::normalize::normalize,
 };
 
@@ -40,8 +40,7 @@ pub async fn fetch_document_version(
     let result = state
         .database
         .get_document_version(&vault_id, vault_update_id, None)
-        .await
+        .await?
-        .map_err(server_error)?
         .map_or_else(
             || {
                 Err(not_found_error(anyhow!(
@@ -52,7 +51,7 @@ pub async fn fetch_document_version(
         )?;
 
     if result.document_id != document_id {
-        return Err(client_error(anyhow!(
+        return Err(not_found_error(anyhow!(
             "Document with document id `{document_id}` does not have a version with id \
             `{vault_update_id}`",
         )));

sync-server/src/server/fetch_document_version_content.rs

@@ -11,7 +11,7 @@ use crate::{
         AppState,
         database::models::{DocumentId, VaultId, VaultUpdateId},
     },
-    errors::{SyncServerError, client_error, not_found_error, server_error},
+    errors::{SyncServerError, not_found_error},
     utils::normalize::normalize,
 };
 
@@ -40,8 +40,7 @@ pub async fn fetch_document_version_content(
     let result = state
         .database
         .get_document_version(&vault_id, vault_update_id, None)
-        .await
+        .await?
-        .map_err(server_error)?
         .map_or_else(
             || {
                 Err(not_found_error(anyhow!(
@@ -52,7 +51,7 @@ pub async fn fetch_document_version_content(
         )?;
 
     if result.document_id != document_id {
-        return Err(client_error(anyhow!(
+        return Err(not_found_error(anyhow!(
             "Document with document id `{document_id}` does not have a version with id \
             `{vault_update_id}`",
         )));

sync-server/src/server/fetch_latest_document_version.rs

@@ -1,51 +0,0 @@
-use anyhow::anyhow;
-use axum::{
-    Json,
-    extract::{Path, State},
-};
-use log::debug;
-use serde::Deserialize;
-
-use crate::{
-    app_state::{
-        AppState,
-        database::models::{DocumentId, DocumentVersion, VaultId},
-    },
-    errors::{SyncServerError, not_found_error, server_error},
-    utils::normalize::normalize,
-};
-
-#[derive(Deserialize)]
-pub struct FetchLatestDocumentVersionPathParams {
-    #[serde(deserialize_with = "normalize")]
-    vault_id: VaultId,
-
-    document_id: DocumentId,
-}
-
-#[axum::debug_handler]
-pub async fn fetch_latest_document_version(
-    Path(FetchLatestDocumentVersionPathParams {
-        vault_id,
-        document_id,
-    }): Path<FetchLatestDocumentVersionPathParams>,
-    State(state): State<AppState>,
-) -> Result<Json<DocumentVersion>, SyncServerError> {
-    debug!("Fetching latest document version for document `{document_id}` in vault `{vault_id}`");
-
-    let latest_version = state
-        .database
-        .get_latest_document(&vault_id, &document_id, None)
-        .await
-        .map_err(server_error)?
-        .map_or_else(
-            || {
-                Err(not_found_error(anyhow!(
-                    "Document with id `{document_id}` not found",
-                )))
-            },
-            Ok,
-        )?;
-
-    Ok(Json(latest_version.into()))
-}

sync-server/src/server/fetch_latest_documents.rs

@@ -1,59 +0,0 @@
-use axum::{
-    Json,
-    extract::{Path, Query, State},
-};
-use log::debug;
-use serde::Deserialize;
-
-use super::responses::FetchLatestDocumentsResponse;
-use crate::{
-    app_state::{
-        AppState,
-        database::models::{VaultId, VaultUpdateId},
-    },
-    errors::{SyncServerError, server_error},
-    utils::normalize::normalize,
-};
-
-#[derive(Deserialize)]
-pub struct FetchLatestDocumentsPathParams {
-    #[serde(deserialize_with = "normalize")]
-    vault_id: VaultId,
-}
-
-#[derive(Deserialize)]
-pub struct QueryParams {
-    since_update_id: Option<VaultUpdateId>,
-}
-
-#[axum::debug_handler]
-pub async fn fetch_latest_documents(
-    Path(FetchLatestDocumentsPathParams { vault_id }): Path<FetchLatestDocumentsPathParams>,
-    Query(QueryParams { since_update_id }): Query<QueryParams>,
-    State(state): State<AppState>,
-) -> Result<Json<FetchLatestDocumentsResponse>, SyncServerError> {
-    debug!("Fetching latest documents in vault `{vault_id}` since update ID `{since_update_id:?}`");
-
-    let documents = if let Some(since_update_id) = since_update_id {
-        state
-            .database
-            .get_latest_documents_since(&vault_id, since_update_id, None, None)
-            .await
-            .map_err(server_error)
-    } else {
-        state
-            .database
-            .get_latest_documents(&vault_id, None, None)
-            .await
-            .map_err(server_error)
-    }?;
-
-    Ok(Json(FetchLatestDocumentsResponse {
-        last_update_id: documents
-            .iter()
-            .map(|doc| doc.vault_update_id)
-            .max()
-            .unwrap_or(since_update_id.unwrap_or(0)),
-        latest_documents: documents,
-    }))
-}

sync-server/src/server/ping.rs

@@ -9,7 +9,7 @@ use axum_extra::{
 use log::debug;
 use serde::Deserialize;
 
-use super::{auth::auth, responses::PingResponse};
+use super::{auth::authenticate_for_vault, responses::PingResponse};
 use crate::{
     app_state::{AppState, database::models::VaultId},
     consts::SUPPORTED_API_VERSION,
@@ -31,8 +31,9 @@ pub async fn ping(
 ) -> Result<Json<PingResponse>, SyncServerError> {
     debug!("Pinging vault `{vault_id}`");
 
-    let is_authenticated = maybe_auth_header
+    let is_authenticated = maybe_auth_header.is_some_and(|auth_header| {
-        .is_some_and(|auth_header| auth(&state, auth_header.token(), &vault_id).is_ok());
+        authenticate_for_vault(&state, auth_header.token(), &vault_id).is_ok()
+    });
 
     Ok(Json(PingResponse {
         server_version: env!("CARGO_PKG_VERSION").to_owned(),

sync-server/src/server/rate_limit.rs

@@ -32,26 +32,23 @@ struct BucketState {
 
 impl RateLimiter {
     /// Create a new per-user rate limiter.
-    ///
-    /// # Panics
-    ///
-    /// Panics if `max_per_second` is 0.
     pub fn new(max_per_second: u64) -> Self {
-        assert!(
-            max_per_second > 0,
-            "max_per_second must be > 0 (set rate_limit_per_user_per_second to null in config to disable)"
-        );
-
         Self {
             max_per_second,
             buckets: Arc::new(Mutex::new(HashMap::new())),
         }
     }
 
-    fn get_or_create_bucket(&self, token: &str) -> Arc<TokenBucket> {
+    fn get_or_create_bucket(
-        self.buckets
+        &self,
+        token: &str,
+    ) -> std::result::Result<Arc<TokenBucket>, StatusCode> {
+        let mut buckets = self
+            .buckets
             .lock()
-            .expect("rate limiter lock poisoned")
+            .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
+
+        Ok(buckets
             .entry(token.to_owned())
             .or_insert_with(|| {
                 Arc::new(TokenBucket {
@@ -62,23 +59,26 @@ impl RateLimiter {
                     max_tokens: self.max_per_second,
                 })
             })
-            .clone()
+            .clone())
     }
 }
 
 impl TokenBucket {
-    fn try_acquire(&self) -> bool {
+    fn try_acquire(&self) -> std::result::Result<bool, StatusCode> {
-        let mut state = self.state.lock().expect("token bucket lock poisoned");
+        let mut state = self
+            .state
+            .lock()
+            .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
         let now = Instant::now();
         if now.duration_since(state.last_refill).as_secs() >= 1 {
             state.tokens = self.max_tokens;
             state.last_refill = now;
         }
         if state.tokens > 0 {
-            state.tokens -= 1;
+            state.tokens = state.tokens.saturating_sub(1);
-            true
+            Ok(true)
         } else {
-            false
+            Ok(false)
         }
     }
 }
@@ -88,13 +88,13 @@ pub async fn rate_limit_middleware(
     auth_header: Option<TypedHeader<Authorization<Bearer>>>,
     req: Request,
     next: Next,
-) -> Result<Response, StatusCode> {
+) -> std::result::Result<Response, StatusCode> {
     let Some(TypedHeader(auth)) = auth_header else {
         return Ok(next.run(req).await);
     };
 
-    let bucket = limiter.get_or_create_bucket(auth.token());
+    let bucket = limiter.get_or_create_bucket(auth.token())?;
-    if bucket.try_acquire() {
+    if bucket.try_acquire()? {
         Ok(next.run(req).await)
     } else {
         Err(StatusCode::TOO_MANY_REQUESTS)

sync-server/src/server/responses.rs

@@ -1,9 +1,7 @@
 use serde::{self, Serialize};
 use ts_rs::TS;
 
-use crate::app_state::database::models::{
+use crate::app_state::database::models::{DocumentVersion, DocumentVersionWithoutContent};
-    DocumentVersion, DocumentVersionWithoutContent, VaultUpdateId,
-};
 
 /// Response to a ping request.
 #[derive(TS, Debug, Clone, Serialize)]
@@ -25,17 +23,6 @@ pub struct PingResponse {
     pub supported_api_version: u32,
 }
 
-/// Response to a fetch latest documents request.
-#[derive(TS, Debug, Clone, Serialize)]
-#[serde(rename_all = "camelCase")]
-#[ts(export)]
-pub struct FetchLatestDocumentsResponse {
-    pub latest_documents: Vec<DocumentVersionWithoutContent>,
-
-    /// The update ID of the latest document in the response.
-    pub last_update_id: VaultUpdateId,
-}
-
 /// Response to a create/update document request.
 #[derive(TS, Debug, Clone, Serialize)]
 #[serde(tag = "type")]

sync-server/src/server/update_document.rs

@@ -22,12 +22,10 @@ use crate::{
         },
     },
     config::user_config::User,
-    errors::{
+    errors::{SyncServerError, client_error, not_found_error, server_error},
-        SyncServerError, client_error, not_found_error, server_error, write_transaction_error,
-    },
     server::requests::UpdateBinaryDocumentVersion,
     utils::{
-        find_first_available_path::find_first_available_path, is_binary::is_binary,
+        find_first_available_path::find_first_available_path, is_binary::as_non_binary_text,
         is_file_type_mergable::is_file_type_mergable, normalize::normalize,
         sanitize_path::sanitize_path,
     },
@@ -58,8 +56,7 @@ pub async fn update_binary(
     let transaction = state
         .database
         .create_write_transaction(&vault_id)
-        .await
+        .await?;
-        .map_err(write_transaction_error)?;
 
     update_document(
         &parent_document.relative_path,
@@ -104,8 +101,7 @@ pub async fn update_text(
     let transaction = state
         .database
         .create_write_transaction(&vault_id)
-        .await
+        .await?;
-        .map_err(write_transaction_error)?;
 
     update_document(
         &parent_document.relative_path,
@@ -131,8 +127,7 @@ async fn get_parent_document(
     let parent = state
         .database
         .get_document_version(vault_id, parent_version_id, None)
-        .await
+        .await?
-        .map_err(server_error)?
         .map_or_else(
             || {
                 Err(not_found_error(anyhow!(
@@ -173,15 +168,20 @@ pub async fn update_document(
 
     let last_update_id = state
         .database
-        .get_max_update_id_in_vault(&vault_id, Some(&mut transaction))
+        .get_max_update_id_in_vault(
-        .await
+            &vault_id,
-        .map_err(server_error)?;
+            Some(transaction.connection_mut().map_err(server_error)?),
+        )
+        .await?;
 
     let latest_version = state
         .database
-        .get_latest_document(&vault_id, &document_id, Some(&mut transaction))
+        .get_latest_document(
-        .await
+            &vault_id,
-        .map_err(server_error)?
+            &document_id,
+            Some(transaction.connection_mut().map_err(server_error)?),
+        )
+        .await?
         .map_or_else(
             || {
                 Err(not_found_error(anyhow!(
@@ -192,11 +192,7 @@ pub async fn update_document(
         )?;
 
     if latest_version.is_deleted {
-        transaction
+        transaction.rollback().await?;
-            .rollback()
-            .await
-            .context("Failed to roll back transaction")
-            .map_err(server_error)?;
 
         info!("Document `{document_id}` has been deleted, ignoring update to it",);
         return Ok(Json(DocumentUpdateResponse::FastForwardUpdate(
@@ -214,48 +210,42 @@ pub async fn update_document(
         info!(
             "Document content is the same as the latest version for `{document_id}`, skipping update"
         );
-        transaction
+        transaction.rollback().await?;
-            .rollback()
-            .await
-            .context("Failed to roll back transaction")
-            .map_err(server_error)?;
 
         return Ok(Json(DocumentUpdateResponse::FastForwardUpdate(
             latest_version.into(),
         )));
     }
 
-    // For mergability, use whichever path the new version will live at — the
+    // For mergability, use whichever path the new version will live at:
-    // requested rename target if the client sent one, otherwise the existing
+    // - the requested rename target if the client sent one
-    // server-side path.
+    // - otherwise the existing server-side path.
     let mergable_check_path = sanitized_relative_path
         .as_deref()
         .unwrap_or(&latest_version.relative_path);
-    let are_all_participants_mergable = is_file_type_mergable(
+
+    let mergeable_texts = if is_file_type_mergable(
         mergable_check_path,
         &state.config.server.mergeable_file_extensions,
-    ) && !is_binary(&parent_content)
+    ) {
-        && !is_binary(&latest_version.content)
+        as_non_binary_texts(&parent_content, &latest_version.content, &content)
-        && !is_binary(&content);
+    } else {
+        None
+    };
+    let are_all_participants_mergable = mergeable_texts.is_some();
 
-    let (merged_content, is_different_from_request_content) = if are_all_participants_mergable {
+    let (merged_content, is_same_as_request) =
+        if let Some((parent_text, latest_text, new_text)) = mergeable_texts {
             info!("Merging changes for document `{document_id}` in vault `{vault_id}`");
-        let parent_text = str::from_utf8(&parent_content)
+
-            .context("Parent document content is not valid UTF-8")
-            .map_err(client_error)?;
-        let latest_text = str::from_utf8(&latest_version.content)
-            .context("Latest version content is not valid UTF-8")
-            .map_err(client_error)?;
-        let new_text = str::from_utf8(&content)
-            .context("New content is not valid UTF-8")
-            .map_err(client_error)?;
             let parent_owned = parent_text.to_owned();
             let latest_owned = latest_text.to_owned();
             let new_owned = new_text.to_owned();
             let content_clone = content.clone();
 
-        let (merged, is_different) = tokio::task::spawn_blocking(move || {
+            let merged = tokio::task::spawn_blocking(move || {
-            let merged = reconcile(
+                
+                reconcile(
                     &parent_owned,
                     &latest_owned.into(),
                     &new_owned.into(),
@@ -263,26 +253,20 @@ pub async fn update_document(
                 )
                 .apply()
                 .text()
-            .into_bytes();
+                .into_bytes()
-            let is_different = merged != content_clone;
-            (merged, is_different)
             })
             .await
             .map_err(|e| server_error(anyhow::anyhow!("Reconcile task failed: {e}")))?;
 
-        (merged, is_different)
+            let is_same = merged == content_clone;
+            (merged, is_same)
         } else {
-        (content, false) // false means that the client doesn't need to refetch the file as we can ensure the remote and local versions are the same as LWW is the merging method for binary files
+            (content, true) // true means that the client doesn't need to refetch the file as we can ensure the remote and local versions are the same as LWW is the merging method for binary files
         };
 
-    // Rename resolution: only apply the client's rename if (a) the client
+    // First rename wins: apply the client's rename only if the doc's path
-    // requested one (`sanitized_relative_path` is `Some`) and (b) the
+    // hasn't changed since its parent version. Content from both clients
-    // document's path hasn't changed since this client's parent version.
+    // still merges via the 3-way reconcile above
-    // If the parent and latest paths differ, another client already renamed
-    // the document — keep the latest path (first rename wins). Content
-    // changes from both clients are still merged correctly via the 3-way
-    // reconcile above, independent of which rename wins. A missing
-    // relative_path means "keep current path" (content-only edit).
     let new_relative_path = match sanitized_relative_path.as_deref() {
         Some(requested)
             if parent_relative_path == latest_version.relative_path
@@ -290,8 +274,7 @@ pub async fn update_document(
         {
             let new_path =
                 find_first_available_path(&vault_id, requested, &state.database, &mut transaction)
-                    .await
+                    .await?;
-                    .map_err(server_error)?;
 
             if new_path != requested {
                 info!(
@@ -306,7 +289,9 @@ pub async fn update_document(
 
     let new_version = StoredDocumentVersion {
         document_id,
-        vault_update_id: last_update_id + 1,
+        vault_update_id: last_update_id
+            .checked_add(1)
+            .ok_or_else(|| server_error(anyhow!("Vault update id overflow")))?,
         creation_vault_update_id: latest_version.creation_vault_update_id,
         relative_path: new_relative_path,
         content: merged_content,
@@ -314,18 +299,29 @@ pub async fn update_document(
         is_deleted: false,
         user_id: user.name,
         device_id: device_id.0,
-        has_been_merged: are_all_participants_mergable && is_different_from_request_content,
+        has_been_merged: are_all_participants_mergable && !is_same_as_request,
     };
 
     state
         .database
         .insert_document_version(&vault_id, &new_version, transaction)
-        .await
+        .await?;
-        .map_err(server_error)?;
 
-    Ok(Json(if is_different_from_request_content {
+    Ok(Json(if is_same_as_request {
-        DocumentUpdateResponse::MergingUpdate(new_version.into())
-    } else {
         DocumentUpdateResponse::FastForwardUpdate(new_version.into())
+    } else {
+        DocumentUpdateResponse::MergingUpdate(new_version.into())
     }))
 }
+
+fn as_non_binary_texts<'a>(
+    parent_content: &'a [u8],
+    latest_content: &'a [u8],
+    new_content: &'a [u8],
+) -> Option<(&'a str, &'a str, &'a str)> {
+    Some((
+        as_non_binary_text(parent_content)?,
+        as_non_binary_text(latest_content)?,
+        as_non_binary_text(new_content)?,
+    ))
+}

sync-server/src/server/websocket.rs

@@ -136,8 +136,7 @@ async fn websocket(
     //    catch-up and in a contended-then-released broadcast is
     //    delivered exactly once (via the catch-up).
     let send_guard = state.broadcasts.acquire_send_lock(&vault_id).await;
-    let mut broadcast_receiver = match state.broadcasts.get_receiver(vault_id.clone(), max_clients)
+    let mut broadcast_receiver = match state.broadcasts.get_receiver(&vault_id, max_clients) {
-    {
         Ok(receiver) => receiver,
         Err(err) => {
             drop(send_guard);
@@ -163,8 +162,7 @@ async fn websocket(
     let cursor = state
         .database
         .get_max_update_id_in_vault(&vault_id, None)
-        .await
+        .await?;
-        .map_err(server_error)?;
     drop(send_guard);
 
     // Catch-up on versions committed while this client was offline,
@@ -209,14 +207,24 @@ async fn websocket(
         loop {
             match broadcast_receiver.recv().await {
                 Ok(update) => {
-                    // Drop messages this device authored because the HTTP
+                    // Always deliver vault updates to the originating
-                    // response already carried authoritative state back.
+                    // device too. The HTTP response is the *normal* path
-                    // Delete broadcasts are sent without an origin so the
+                    // for the originator to learn its own update, and
-                    // author also receives them — that's the receipt the
+                    // the client-side wire loop dedupes redundant
-                    // client needs to drop the doc from its sync queue.
+                    // broadcasts via the `parentVersionId` check. But
-                    if Some(&device_id) == update.origin_device_id.as_ref() {
+                    // when the response is lost mid-flight (sync reset,
-                        continue;
+                    // pause/resume, dropped TCP) the originator has no
-                    }
+                    // record of the doc; if the broadcast is also
+                    // suppressed AND the next handshake's cursor was
+                    // captured before the commit (cursor < vuid), the
+                    // doc falls through both delivery paths and is
+                    // stranded forever on the originator. Letting the
+                    // self-broadcast through closes that window — the
+                    // message processes as a remote create on the
+                    // originator's reconnected WS and the file is
+                    // restored. (Cursor messages still get the inner
+                    // self-filter below; we drop our own cursor entries
+                    // from the `clients` payload.)
 
                     // Filter out vault updates already covered by the
                     // catch-up snapshot. The handshake atomically
@@ -229,13 +237,13 @@ async fn websocket(
                     // Cursor messages aren't versioned and are always
                     // forwarded.
                     if let WebSocketServerMessage::VaultUpdate(WebSocketVaultUpdate { document }) =
-                        &update.message
+                        &update
                         && document.vault_update_id <= cursor
                     {
                         continue;
                     }
 
-                    let message = match update.message {
+                    let message = match update {
                         WebSocketServerMessage::CursorPositions(CursorPositionFromServer {
                             clients,
                         }) => WebSocketServerMessage::CursorPositions(CursorPositionFromServer {
@@ -244,7 +252,7 @@ async fn websocket(
                                 .filter(|client| client.device_id != device_id)
                                 .collect(),
                         }),
-                        WebSocketServerMessage::VaultUpdate(_) => update.message,
+                        update @ WebSocketServerMessage::VaultUpdate(_) => update,
                     };
 
                     send_update_over_websocket(&message, &mut sender).await?;
@@ -307,7 +315,7 @@ async fn websocket(
                                     &device_id,
                                     docs,
                                 )
-                                .await;
+                                .await?;
                         }
                     }
                 }
@@ -352,7 +360,7 @@ async fn websocket(
     state
         .cursors
         .remove_cursors_of_device(&vault_id, &authed_handshake.handshake.device_id)
-        .await;
+        .await?;
 
     match &result {
         Ok(()) => {

sync-server/src/utils/dedup_paths.rs

@@ -1,10 +1,3 @@
-use std::sync::LazyLock;
-
-use regex::Regex;
-
-static DEDUP_SUFFIX_REGEX: LazyLock<Regex> =
-    LazyLock::new(|| Regex::new(r" \((\d+)\)$").expect("invalid regex"));
-
 pub fn dedup_paths(path: &str) -> impl Iterator<Item = String> {
     let mut path_parts = path.split('/').collect::<Vec<_>>();
     let file_name = path_parts
@@ -24,29 +17,19 @@ pub fn dedup_paths(path: &str) -> impl Iterator<Item = String> {
     let (stem, extension) = if is_simple_dotfile {
         (file_name.clone(), String::new())
     } else {
-        // Regular file or dotfile with extension
+        match file_name.rsplit_once('.') {
-        let name_parts = file_name.rsplitn(2, '.').collect::<Vec<_>>();
+            Some((stem, extension)) => (stem.to_owned(), format!(".{extension}")),
-        let mut reverse_parts = name_parts.into_iter().rev();
+            None => (file_name.clone(), String::new()),
-        match (reverse_parts.next(), reverse_parts.next()) {
-            (Some(stem), maybe_extension) => (
-                stem.to_owned(),
-                maybe_extension
-                    .map(|ext| format!(".{ext}"))
-                    .unwrap_or_default(),
-            ),
-            _ => unreachable!("Path must have at least one part"),
         }
     };
 
-    let start_number = DEDUP_SUFFIX_REGEX
+    let (clean_stem, start_number) = strip_dedup_suffix(&stem);
-        .captures(&stem)
+    let clean_stem = clean_stem.to_owned();
-        .and_then(|caps| caps.get(1))
-        .and_then(|m| m.as_str().parse::<u32>().ok())
-        .unwrap_or(0);
 
-    let clean_stem = DEDUP_SUFFIX_REGEX.replace(&stem, "").to_string();
+    std::iter::successors(Some(start_number), |dedup_number| {
-
+        dedup_number.checked_add(1)
-    (start_number..).map(move |dedup_number| {
+    })
+    .map(move |dedup_number| {
         if dedup_number == 0 {
             format!("{directory}{clean_stem}{extension}")
         } else {
@@ -55,6 +38,20 @@ pub fn dedup_paths(path: &str) -> impl Iterator<Item = String> {
     })
 }
 
+fn strip_dedup_suffix(stem: &str) -> (&str, u64) {
+    let Some(without_closing_paren) = stem.strip_suffix(')') else {
+        return (stem, 0);
+    };
+    let Some((clean_stem, number)) = without_closing_paren.rsplit_once(" (") else {
+        return (stem, 0);
+    };
+    if number.is_empty() || !number.chars().all(|c| c.is_ascii_digit()) {
+        return (stem, 0);
+    }
+
+    (clean_stem, number.parse::<u64>().unwrap_or(0))
+}
+
 #[cfg(test)]
 mod test {
     use super::*;
@@ -103,7 +100,7 @@ mod test {
     }
 
     #[test]
-    fn test_regex_capturing_group() {
+    fn test_dedup_suffix_parsing() {
         // Single digit in parentheses
         let mut deduped = dedup_paths("document (5).md");
         assert_eq!(deduped.next(), Some("document (5).md".to_owned()));

sync-server/src/utils/find_first_available_path.rs

@@ -1,20 +1,24 @@
-use crate::app_state::database::models::VaultId;
+use crate::app_state::database::{WriteTransaction, models::VaultId};
+use crate::errors::{SyncServerError, server_error};
 use crate::utils::dedup_paths::dedup_paths;
-use anyhow::Result;
+use anyhow::anyhow;
 use log::{debug, info};
-use sqlx::sqlite::SqliteConnection;
 
 pub async fn find_first_available_path(
     vault_id: &VaultId,
     sanitized_relative_path: &str,
     database: &crate::app_state::database::Database,
-    connection: &mut SqliteConnection,
+    transaction: &mut WriteTransaction,
-) -> Result<String> {
+) -> Result<String, SyncServerError> {
     info!("Finding first available path for `{sanitized_relative_path}` in vault `{vault_id}`");
     for candidate in dedup_paths(sanitized_relative_path) {
         debug!("Checking candidate path for deconflicting names: `{candidate}`");
         if database
-            .get_latest_non_deleted_document_by_path(vault_id, &candidate, Some(connection))
+            .get_latest_non_deleted_document_by_path(
+                vault_id,
+                &candidate,
+                Some(transaction.connection_mut().map_err(server_error)?),
+            )
             .await?
             .is_none()
         {
@@ -27,5 +31,7 @@ pub async fn find_first_available_path(
         );
     }
 
-    unreachable!("dedup_paths produces infinite paths");
+    Err(server_error(anyhow!(
+        "No available path candidates produced for `{sanitized_relative_path}` in vault `{vault_id}`"
+    )))
 }

sync-server/src/utils/is_binary.rs

@@ -1,16 +1,22 @@
-/// Heuristically determine if the given data is a binary or a text file's
+/// Return the given data as UTF-8 text if it is not considered binary.
-/// content.
 ///
 /// Only text inputs can be reconciled using the crate's functions.
 #[must_use]
-pub fn is_binary(data: &[u8]) -> bool {
+pub fn as_non_binary_text(data: &[u8]) -> Option<&str> {
     if data.contains(&0) {
         // Even though the NUL character is valid in UTF-8, it's highly suspicious in
         // human-readable text.
-        return true;
+        return None;
     }
 
-    std::str::from_utf8(data).is_err()
+    std::str::from_utf8(data).ok()
+}
+
+/// Heuristically determine if the given data is a binary or a text file's
+/// content.
+#[must_use]
+pub fn is_binary(data: &[u8]) -> bool {
+    as_non_binary_text(data).is_none()
 }
 
 #[cfg(test)]
@@ -23,4 +29,11 @@ mod tests {
         assert!(is_binary(&[0, 12]));
         assert!(!is_binary(b"hello"));
     }
+
+    #[test]
+    fn test_as_non_binary_text() {
+        assert_eq!(as_non_binary_text(b"hello"), Some("hello"));
+        assert_eq!(as_non_binary_text(&[0, 12]), None);
+        assert_eq!(as_non_binary_text(&[0xff]), None);
+    }
 }

sync-server/src/utils/rotating_file_writer.rs

@@ -2,11 +2,12 @@ use std::{
     fs::{self, OpenOptions},
     io::{self, Write},
     path::{Path, PathBuf},
-    sync::{Arc, Mutex},
+    sync::{Arc, Mutex, MutexGuard},
     time::{Duration, SystemTime, UNIX_EPOCH},
 };
 
 use chrono::NaiveDateTime;
+use log::warn;
 use tracing_subscriber::fmt::MakeWriter;
 
 #[derive(Clone)]
@@ -51,14 +52,14 @@ impl RotatingFileWriter {
     /// Parse timestamp from log filename and return as `SystemTime`
     fn parse_log_timestamp(filename: &str, file_prefix: &str) -> Option<SystemTime> {
         // Expected format: {prefix}.{timestamp}.log where timestamp is %Y-%m-%d_%H-%M-%S
-        let prefix_len = file_prefix.len() + 1; // +1 for the dot
+        let prefix_len = file_prefix.len().checked_add(1)?; // +1 for the dot
         let timestamp_str = filename.get(prefix_len..filename.len().checked_sub(4)?)?;
 
         let dt = NaiveDateTime::parse_from_str(timestamp_str, "%Y-%m-%d_%H-%M-%S").ok()?;
         let timestamp = dt.and_utc();
         let secs: u64 = timestamp.timestamp().try_into().ok()?;
 
-        Some(UNIX_EPOCH + Duration::from_secs(secs))
+        UNIX_EPOCH.checked_add(Duration::from_secs(secs))
     }
 
     fn find_latest_log_file(directory: &Path, file_prefix: &str) -> Option<String> {
@@ -85,7 +86,9 @@ impl RotatingFileWriter {
         Self::find_latest_log_file(directory, file_prefix)
             .and_then(|filename| Self::parse_log_timestamp(&filename, file_prefix))
             .map_or_else(SystemTime::now, |last_rotation| {
-                last_rotation + rotation_duration
+                last_rotation
+                    .checked_add(rotation_duration)
+                    .unwrap_or_else(SystemTime::now)
             })
     }
 
@@ -93,6 +96,17 @@ impl RotatingFileWriter {
         SystemTime::now() >= inner.next_rotation_time
     }
 
+    fn lock_inner(&self) -> MutexGuard<'_, RotatingFileWriterInner> {
+        match self.inner.lock() {
+            Ok(inner) => inner,
+            Err(poisoned) => {
+                warn!("RotatingFileWriter mutex was poisoned, recovering");
+                self.inner.clear_poison();
+                poisoned.into_inner()
+            }
+        }
+    }
+
     fn open_or_create_log_file(inner: &mut RotatingFileWriterInner) -> io::Result<()> {
         // If we haven't reached rotation time and there's an existing log file, reuse it
         if !Self::should_rotate(inner)
@@ -124,7 +138,9 @@ impl RotatingFileWriter {
             .open(&filepath)?;
 
         inner.current_file = Some(file);
-        inner.next_rotation_time = SystemTime::now() + inner.rotation_duration;
+        inner.next_rotation_time = SystemTime::now()
+            .checked_add(inner.rotation_duration)
+            .unwrap_or_else(SystemTime::now);
 
         Ok(())
     }
@@ -132,10 +148,7 @@ impl RotatingFileWriter {
 
 impl Write for RotatingFileWriter {
     fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
-        let mut inner = self.inner.lock().unwrap_or_else(|poisoned| {
+        let mut inner = self.lock_inner();
-            eprintln!("RotatingFileWriter mutex was poisoned, recovering");
-            poisoned.into_inner()
-        });
 
         // Reset file handle after poison recovery so the next branch
         // re-opens a valid file rather than writing to a potentially
@@ -154,10 +167,7 @@ impl Write for RotatingFileWriter {
     }
 
     fn flush(&mut self) -> io::Result<()> {
-        let mut inner = self.inner.lock().unwrap_or_else(|poisoned| {
+        let mut inner = self.lock_inner();
-            eprintln!("RotatingFileWriter mutex was poisoned, recovering");
-            poisoned.into_inner()
-        });
         if let Some(ref mut file) = inner.current_file {
             file.flush()
         } else {