fix tests

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

@@ -0,0 +1,98 @@
+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>();
+
+    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));
+    }
+
+    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 static renameKey(
+        oldPath: RelativePath,
+        newPath: RelativePath
+    ): string {
+        return JSON.stringify({ oldPath, newPath });
+    }
+
+    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;
+    }
+}

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

@@ -169,6 +169,7 @@ export class SyncEventQueue {
             return;
         }
 
+        let needsSave = false;
         if (input.oldPath !== undefined) {
             if (pendingDocumentId !== undefined) {
                 this.updatePendingCreatePath(input.oldPath, path);
@@ -189,16 +190,25 @@ export class SyncEventQueue {
                         e.path = path;
                     }
                 }
-                await this.save();
+                needsSave = true;
             }
         }
 
+        // Push BEFORE awaiting `save()`. Callers fire `enqueue` with `void`
+        // and immediately call `ensureDraining()`, which starts a drain that
+        // synchronously shifts off the queue. If we awaited save first the
+        // shift would see the queue empty, drain would exit, and the event
+        // would never get processed until the next unrelated trigger.
         this.events.push({
             type: SyncEventType.LocalUpdate,
             documentId: (pendingDocumentId ?? documentId)!,
             path,
             originalPath: path
         });
+
+        if (needsSave) {
+            await this.save();
+        }
     }
 
     public async next(): Promise<SyncEvent | undefined> {

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

@@ -531,7 +531,21 @@ export class Syncer {
                 remoteHash
             });
         } else {
-            // The response to a create must contain the path from the create request
+            // The server may have deconflicted the path on create (e.g.
+            // another client raced us to the same path and won). Move the
+            // local file to match the server-assigned path so the queue's
+            // disk-path key, the on-disk path, and `remoteRelativePath` stay
+            // consistent. Without this, a later remote create at the
+            // originally-requested path would see a phantom local conflict
+            // and stash the new file under a `conflict-<uuid>-` path.
+            if (response.relativePath !== createEvent.path) {
+                await this.operations.move(
+                    createEvent.path,
+                    response.relativePath,
+                    MoveOnConflict.EXISTING
+                );
+                createEvent.path = response.relativePath;
+            }
             await this.queue.resolveCreate(createEvent, {
                 ...record,
                 remoteHash
@@ -637,20 +651,33 @@ export class Syncer {
                 remoteVersion.documentId
             )
         ) {
-            // no local changes
+            // no local changes — operations.move just relocated the file to
-            const currentContent = await this.operations.read(path);
+            // `actualPath`, so all subsequent reads and writes must use that
+            // path. Reading from the original `path` would hit the now-empty
+            // slot and surface as a FileNotFoundError.
+            const currentContent = await this.operations.read(actualPath);
             const remoteContent =
                 await this.syncService.getDocumentVersionContent({
                     documentId: remoteVersion.documentId,
                     vaultUpdateId: remoteVersion.vaultUpdateId
                 });
-            await this.operations.write(path, currentContent, remoteContent);
+            await this.operations.write(
+                actualPath,
+                currentContent,
+                remoteContent
+            );
 
             await this.updateCache(
                 remoteVersion.vaultUpdateId,
                 remoteContent,
-                path
+                actualPath
             );
+            await this.queue.setDocument(actualPath, {
+                ...record,
+                parentVersionId: remoteVersion.vaultUpdateId,
+                remoteRelativePath: actualPath,
+                remoteHash: await hash(remoteContent)
+            });
             this.queue.lastSeenUpdateId = remoteVersion.vaultUpdateId;
         } // else we don't need to update the content, a subsequent local update will do that
         else {