claude

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

@@ -30,6 +30,28 @@ export async function scheduleOfflineChanges(
     // next pass.
     const allDocuments = queue.allSettledDocuments();
 
+    // Placement-pending records (`localPath === undefined`) name a server
+    // path that the reconciler will eventually place. If the user already
+    // has a local file at that path — common after a sync-disable or
+    // reset that discarded a successful create's response, leaving the
+    // server-known doc as a placement-pending record once catch-up
+    // re-delivered it — treating it as an untracked file would
+    // re-create a duplicate doc at the server's deconflicted path. Bind
+    // each placement-pending record to its on-disk file: a same-hash
+    // file just inherits the record's localPath; a different-hash file
+    // is folded into the sync-up update flow below (an UPDATE on the
+    // existing doc rather than a fresh CREATE).
+    for (const record of queue.allRecords()) {
+        if (record.localPath !== undefined) {
+            continue;
+        }
+        if (!allLocalFiles.has(record.remoteRelativePath)) {
+            continue;
+        }
+        await queue.setLocalPath(record.documentId, record.remoteRelativePath);
+        allDocuments.set(record.remoteRelativePath, record);
+    }
+
     // A doc is "possibly deleted" only if it has no local file. Including
     // docs that still exist locally would queue a spurious delete alongside
     // the update below.

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

@@ -290,25 +290,30 @@ describe("SyncEventQueue", () => {
             "A"
         );
 
-        // upsertRecord that relocates the localPath should re-key.
+        // upsertRecord on an existing record with a non-undefined
+        // localPath does NOT rewrite localPath. The watcher path and the
+        // reconciler are the only authorities on localPath of an
+        // already-placed record; letting the wire loop re-key here would
+        // race a user rename that landed during an HTTP roundtrip.
         await queue.upsertRecord(
             fakeRecord("A", { localPath: "renamed.md" as RelativePath })
         );
         assert.strictEqual(queue.byLocalPath.size, 1);
         assert.strictEqual(
-            queue.byLocalPath.get("a.md" as RelativePath),
-            undefined
-        );
-        assert.strictEqual(
-            queue.byLocalPath.get("renamed.md" as RelativePath)?.documentId,
+            queue.byLocalPath.get("a.md" as RelativePath)?.documentId,
             "A"
         );
+        assert.strictEqual(
+            queue.byLocalPath.get("renamed.md" as RelativePath),
+            undefined
+        );
+        assert.strictEqual(queue.getDocumentByDocumentId("A")?.localPath, "a.md");
 
-        // setLocalPath should re-key.
+        // setLocalPath does re-key — it's the explicit path-mutation API.
         await queue.setLocalPath("A", "later.md" as RelativePath);
         assert.strictEqual(queue.byLocalPath.size, 1);
         assert.strictEqual(
-            queue.byLocalPath.get("renamed.md" as RelativePath),
+            queue.byLocalPath.get("a.md" as RelativePath),
             undefined
         );
         assert.strictEqual(
@@ -331,6 +336,73 @@ describe("SyncEventQueue", () => {
         );
     });
 
+    it("upsertRecord installs localPath only when the existing record has none (placement-pending → placed)", async () => {
+        const queue = createQueue();
+
+        // Same-docId-collapse shape: a placement-pending record (created
+        // earlier by a remote-create handler when the slot was occupied)
+        // gets resolved by a LocalCreate that returns the same docId.
+        // The watcher hasn't touched localPath since the record is
+        // placement-pending, so installing the now-known path is correct.
+        await queue.upsertRecord(fakeRecord("A", { localPath: undefined }));
+        assert.strictEqual(queue.byLocalPath.size, 0);
+
+        await queue.upsertRecord(
+            fakeRecord("A", { localPath: "fresh.md" as RelativePath })
+        );
+        assert.strictEqual(queue.byLocalPath.size, 1);
+        assert.strictEqual(
+            queue.byLocalPath.get("fresh.md" as RelativePath)?.documentId,
+            "A"
+        );
+        assert.strictEqual(
+            queue.getDocumentByDocumentId("A")?.localPath,
+            "fresh.md"
+        );
+    });
+
+    it("upsertRecord ignores stale localPath from the wire loop after a watcher rename", async () => {
+        const queue = createQueue();
+        await queue.upsertRecord(fakeRecord("A"));
+
+        // Watcher renames a.md -> renamed.md while the wire loop is
+        // mid-roundtrip. The wire loop captured an earlier snapshot of
+        // localPath and now tries to write it back through upsertRecord.
+        await queue.enqueue({
+            type: SyncEventType.LocalUpdate,
+            path: "renamed.md",
+            oldPath: "a.md"
+        });
+        assert.strictEqual(
+            queue.getDocumentByDocumentId("A")?.localPath,
+            "renamed.md"
+        );
+
+        await queue.upsertRecord(
+            fakeRecord("A", {
+                parentVersionId: 2,
+                remoteRelativePath: "a.md",
+                remoteHash: "hash-A-v2",
+                localPath: "a.md" as RelativePath
+            })
+        );
+
+        // The watcher's rename wins: localPath stays at renamed.md.
+        const record = queue.getDocumentByDocumentId("A");
+        assert.strictEqual(record?.localPath, "renamed.md");
+        assert.strictEqual(record.parentVersionId, 2);
+        assert.strictEqual(record.remoteRelativePath, "a.md");
+        assert.strictEqual(record.remoteHash, "hash-A-v2");
+        assert.strictEqual(
+            queue.byLocalPath.get("renamed.md" as RelativePath)?.documentId,
+            "A"
+        );
+        assert.strictEqual(
+            queue.byLocalPath.get("a.md" as RelativePath),
+            undefined
+        );
+    });
+
     it("create can be re-enqueued after being dequeued", async () => {
         const queue = createQueue();
         await queue.enqueue({ type: SyncEventType.LocalCreate, path: "a.md" });

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

@@ -471,9 +471,14 @@ export class SyncEventQueue {
      * fields on their next read — this stays load-bearing for the Syncer's
      * drain handlers, which await across HTTP roundtrips.
      *
-     * Maintains the `byLocalPath` index. If the `localPath` changes the
-     * relocation goes through `setLocalPath` (which also persists), so the
-     * caller doesn't need to call `save()` separately.
+     * For an existing record this updates the wire fields
+     * (`parentVersionId`, `remoteHash`, `remoteRelativePath`) and, only
+     * when the existing record has no local file yet
+     * (`localPath === undefined`), installs the supplied `localPath`. A
+     * non-undefined existing localPath is owned by the watcher path and
+     * the Reconciler — overwriting it from the wire loop would race a
+     * user rename that landed during an HTTP roundtrip and silently
+     * resurrect a stale slot.
      */
     public async upsertRecord(record: DocumentRecord): Promise<void> {
         const existing = this.byDocId.get(record.documentId);
@@ -498,8 +503,10 @@ export class SyncEventQueue {
             existing.parentVersionId = record.parentVersionId;
             existing.remoteHash = record.remoteHash;
             existing.remoteRelativePath = record.remoteRelativePath;
-            if (existing.localPath !== record.localPath) {
-                // setLocalPath re-keys `byLocalPath` and persists.
+            if (
+                existing.localPath === undefined &&
+                record.localPath !== undefined
+            ) {
                 return this.setLocalPath(record.documentId, record.localPath);
             }
         }
@@ -715,6 +722,7 @@ export class SyncEventQueue {
         createEvent.path = newPath;
         if (!createEvent.isProcessing) {
             this.moveBlockingDeletesBeforeCreate(createEvent, newPath);
+            this.moveBlockingRenamesBeforeCreate(createEvent, newPath);
         }
 
         for (const e of this.events) {
@@ -753,6 +761,58 @@ export class SyncEventQueue {
         }
     }
 
+    /**
+     * The `path` argument is the create's just-retargeted target. Any
+     * other tracked doc whose server-side path is still `path` (its
+     * watcher-driven local rename hasn't reached the server yet) needs
+     * its pending LocalUpdate to drain *before* this create — otherwise
+     * the create's HTTP request hits the server while the doc is still
+     * at `path` and triggers a same-path same-docId merge that
+     * silently consumes the user's "new doc" intent into the
+     * already-tracked doc. The pending LocalUpdate is the rename that
+     * moves the existing doc off `path` server-side; running it first
+     * frees the slot. Skipped when the create has already been sent —
+     * at that point the merge has already happened or hasn't, and
+     * reordering the queue can't unwind it.
+     */
+    private moveBlockingRenamesBeforeCreate(
+        createEvent: Extract<SyncEvent, { type: SyncEventType.LocalCreate }>,
+        path: RelativePath
+    ): void {
+        const blockingDocIds = new Set<DocumentId>();
+        for (const record of this.byDocId.values()) {
+            if (
+                record.remoteRelativePath === path &&
+                record.localPath !== path
+            ) {
+                blockingDocIds.add(record.documentId);
+            }
+        }
+        if (blockingDocIds.size === 0) {
+            return;
+        }
+
+        let createIndex = this.events.indexOf(createEvent);
+        if (createIndex < 0) {
+            return;
+        }
+
+        for (let i = createIndex + 1; i < this.events.length; ) {
+            const event = this.events[i];
+            if (
+                event.type === SyncEventType.LocalUpdate &&
+                typeof event.documentId === "string" &&
+                blockingDocIds.has(event.documentId)
+            ) {
+                this.events.splice(i, 1);
+                this.events.splice(createIndex, 0, event);
+                createIndex++;
+                continue;
+            }
+            i++;
+        }
+    }
+
     /**
      * Synchronous half of `setLocalPath`: mutate `record.localPath` and
      * re-key `_byLocalPath` without persisting. Used by `enqueue`'s

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

@@ -729,10 +729,11 @@ export class Syncer {
             parentVersionId: response.vaultUpdateId,
             remoteRelativePath: response.relativePath,
             remoteHash,
-            // localPath is unchanged by the wire loop; only the watcher
-            // (via the queue's enqueue rename branch) and the reconciler
-            // touch it. Read from the live record so a rename that
-            // arrived during the await is preserved.
+            // localPath is owned by the watcher and the reconciler. Pass
+            // the value we observed pre-await purely as a hint for the
+            // placement-pending → placed transition; `upsertRecord` ignores
+            // it when an existing localPath is already set, so a watcher
+            // rename that landed during the HTTP roundtrip is preserved.
             localPath: livePath
         });
         this.queue.lastSeenUpdateId = response.vaultUpdateId;
@@ -948,8 +949,10 @@ export class Syncer {
         // doc (a same-path recreate installed a new owner without
         // clearing this record's stale field — same race shape as the
         // processRemoteDelete fix above). Writing to a shadowed slot
-        // would clobber the new owner's bytes. Treat shadowed records
-        // as if they had no local file: stash for the reconciler.
+        // would clobber the new owner's bytes. Clear the stale claim now
+        // so the reconciler treats this record as placement-pending; the
+        // closing `upsertRecord` no longer touches an existing record's
+        // localPath, so the clear has to happen explicitly here.
         const claimedPath = record.localPath;
         const livePath =
             claimedPath !== undefined &&
@@ -960,8 +963,9 @@ export class Syncer {
         if (claimedPath !== undefined && livePath === undefined) {
             this.logger.debug(
                 `Remote update for ${record.documentId} at claimed ${claimedPath} ` +
-                    `but slot is shadowed; deferring write to reconciler`
+                    `but slot is shadowed; clearing stale claim and deferring to reconciler`
             );
+            await this.queue.setLocalPath(record.documentId, undefined);
         }
         if (livePath !== undefined) {
             const currentContent = await this.operations.read(livePath);