Sync → Manifest can no longer record content that was never uploaded

src/sync/plan.test.ts

@@ -1,7 +1,7 @@
 import assert from "node:assert/strict";
 import { test } from "node:test";
 import { empty, file, snapshot } from "./fake.ts";
-import { conflictCopyPath, MANIFEST_KEY, planSync } from "./plan.ts";
+import { conflictCopyPath, MANIFEST_KEY, manifestAfterSync, planSync } from "./plan.ts";
 
 test("planSync: a path only changed locally is pushed", () => {
   const previous = empty;
@@ -89,6 +89,87 @@ test("planSync: the manifest's own path is never turned into an action", () => {
   assert.deepEqual(planSync(previous, local, remote), []);
 });
 
+test("manifestAfterSync: a push records the local snapshot's entry", () => {
+  const local = snapshot(file("a.md", "h2"));
+  const remote = snapshot(file("a.md", "h1"), file("b.md", "h3"));
+
+  const result = manifestAfterSync(local, remote, [{ kind: "push", path: "a.md" }], 1);
+
+  assert.deepEqual(result, snapshot(file("a.md", "h2"), file("b.md", "h3")));
+});
+
+test("manifestAfterSync: a pushDelete removes the entry", () => {
+  const local = empty;
+  const remote = snapshot(file("a.md", "h1"), file("b.md", "h3"));
+
+  const result = manifestAfterSync(local, remote, [{ kind: "pushDelete", path: "a.md" }], 1);
+
+  assert.deepEqual(result, snapshot(file("b.md", "h3")));
+});
+
+test("manifestAfterSync: pull and pullDelete leave the bucket, and so the manifest, untouched", () => {
+  const local = empty;
+  const remote = snapshot(file("a.md", "h1"));
+
+  const result = manifestAfterSync(
+    local,
+    remote,
+    [
+      { kind: "pull", path: "a.md" },
+      { kind: "pullDelete", path: "b.md" },
+    ],
+    1,
+  );
+
+  assert.deepEqual(result, remote);
+});
+
+test("manifestAfterSync: a content conflict keeps the remote entry and adds the pushed copy", () => {
+  const now = Date.parse("2026-07-14T10:00:00.000Z");
+  const local = snapshot(file("a.md", "h2"));
+  const remote = snapshot(file("a.md", "h3"));
+
+  const result = manifestAfterSync(
+    local,
+    remote,
+    [{ kind: "conflict", path: "a.md", deletedSide: "none" }],
+    now,
+  );
+
+  const copyPath = "a (conflicted copy 2026-07-14T10-00-00-000Z).md";
+  assert.deepEqual(result, snapshot(file("a.md", "h3"), { ...file("a.md", "h2"), path: copyPath }));
+});
+
+test("manifestAfterSync: a remote deletion conflict records only the pushed copy", () => {
+  const now = Date.parse("2026-07-14T10:00:00.000Z");
+  const local = snapshot(file("a.md", "h2"));
+  const remote = empty;
+
+  const result = manifestAfterSync(
+    local,
+    remote,
+    [{ kind: "conflict", path: "a.md", deletedSide: "remote" }],
+    now,
+  );
+
+  const copyPath = "a (conflicted copy 2026-07-14T10-00-00-000Z).md";
+  assert.deepEqual(result, snapshot({ ...file("a.md", "h2"), path: copyPath }));
+});
+
+test("manifestAfterSync: a local deletion conflict pushes nothing, the remote entry stands", () => {
+  const local = empty;
+  const remote = snapshot(file("a.md", "h2"));
+
+  const result = manifestAfterSync(
+    local,
+    remote,
+    [{ kind: "conflict", path: "a.md", deletedSide: "local" }],
+    1,
+  );
+
+  assert.deepEqual(result, remote);
+});
+
 test("conflictCopyPath: keeps the extension", () => {
   assert.equal(
     conflictCopyPath("notes/todo.md", Date.parse("2026-07-14T10:00:00.000Z")),

src/sync/plan.ts

@@ -29,6 +29,57 @@ export function conflictCopyPath(path: string, now: number): string {
   return `${path.slice(0, lastDot)} (conflicted copy ${stamp})${path.slice(lastDot)}`;
 }
 
+// manifestAfterSync returns the snapshot of what the bucket holds once every action in the plan
+// has succeeded: remote as it was read, minus pushed deletions, plus pushed files and conflict
+// copies recorded at the local snapshot's entry. It is computed from the plan rather than
+// re-snapshotted from disk so the manifest can never record content the bucket does not have
+// (#84): a file that changed while the plan ran keeps its bucket entry, and the next sync sees
+// the drift as a local change and pushes it. The one race left, a push whose bytes drifted past
+// the local snapshot before they were read, only ever understates the bucket, and the next pass
+// simply pushes again.
+export function manifestAfterSync(
+  local: Snapshot,
+  remote: Snapshot,
+  actions: SyncAction[],
+  now: number,
+): Snapshot {
+  const files = byPath(remote.files);
+  const localByPath = byPath(local.files);
+
+  for (const action of actions) {
+    // pull and pullDelete only change the local vault; the bucket is untouched.
+    if (action.kind === "pull" || action.kind === "pullDelete") {
+      continue;
+    }
+    if (action.kind === "pushDelete") {
+      files.delete(action.path);
+      continue;
+    }
+    if (action.kind === "push") {
+      // A push is only ever planned for a file present in the local snapshot, so the guard is
+      // narrowing, not a real branch; a miss would mean planSync broke that invariant.
+      const pushed = localByPath.get(action.path);
+      if (pushed !== undefined) {
+        files.set(action.path, pushed);
+      }
+      continue;
+    }
+    // conflict: a local deletion pushes nothing, the remote entry stands as is. The other two
+    // sides push the local edit under its conflict copy name; the original path is already
+    // correct in remote (present for deletedSide "none", absent for "remote").
+    if (action.deletedSide === "local") {
+      continue;
+    }
+    const copied = localByPath.get(action.path);
+    if (copied !== undefined) {
+      const copyPath = conflictCopyPath(action.path, now);
+      files.set(copyPath, { ...copied, path: copyPath });
+    }
+  }
+
+  return { files: [...files.values()] };
+}
+
 // planSync compares what changed locally since the last successful sync against what changed
 // remotely since that same sync, and decides what to push, what to pull, and what's a genuine
 // conflict: a path that changed on both sides to different content. previous is the snapshot

src/sync/sync.test.ts

@@ -3,7 +3,7 @@ import { test } from "node:test";
 import type { Snapshot } from "../vault/vault.ts";
 import { empty, fakeLocalWriter, fakeReader, fakeStorage, file, snapshot } from "./fake.ts";
 import { MANIFEST_KEY } from "./plan.ts";
-import { readRemoteManifest, syncOnce } from "./sync.ts";
+import { adoptLiveStats, readRemoteManifest, syncOnce } from "./sync.ts";
 
 test("readRemoteManifest: a 404 is treated as an empty snapshot", async () => {
   const { storage } = fakeStorage();
@@ -164,6 +164,55 @@ test("syncOnce: a manifest overwritten by another device mid sync fails the pass
   assert.equal(files.get("a.md"), "xy");
 });
 
+test("syncOnce: a file changed mid sync is not recorded in the manifest and is pushed next pass", async () => {
+  // Reproduces #84. The vault is in sync (a.md, unchanged), and b.md is new locally, so the pass
+  // pushes b.md. While that push is in flight the user edits a.md and creates c.md. Before the
+  // fix the manifest was a re-snapshot of the disk taken after the plan ran, so it recorded both
+  // with content the bucket never received; neither then ever uploaded (state.json already agreed
+  // with the manifest), and another device could push the stale bucket copy of a.md back over the
+  // edit. The manifest must instead keep claiming only what the bucket holds, leaving both files
+  // as local changes for the next pass to push.
+  const ancestor = snapshot(file("a.md", "h1"));
+  const { storage, objects } = fakeStorage({ [MANIFEST_KEY]: JSON.stringify(ancestor) });
+  // a.md matches the ancestor's size and mtime so takeSnapshot reuses its hash and sees no local
+  // change there; b.md is the new local file whose push is the mid sync moment to interleave on.
+  const readerFiles: Record<string, string> = { "a.md": "xy", "b.md": "beta" };
+  const reader = fakeReader(readerFiles);
+  const { writer } = fakeLocalWriter();
+  const inner = storage.putObject;
+  let edited = false;
+  storage.putObject = async (key, body, condition) => {
+    if (key === "b.md" && !edited) {
+      edited = true;
+      readerFiles["a.md"] = "edited mid sync";
+      readerFiles["c.md"] = "created mid sync";
+    }
+    return inner(key, body, condition);
+  };
+
+  const outcome = await syncOnce(ancestor, reader, writer, storage, 1);
+
+  assert.ok(outcome.ok);
+  // The manifest still records a.md as the bucket knows it, and doesn't know c.md at all: neither
+  // file's new content ever reached the bucket.
+  const manifestBody = objects.get(MANIFEST_KEY);
+  assert.ok(manifestBody !== undefined);
+  const manifest = JSON.parse(manifestBody) as Snapshot;
+  const paths = manifest.files.map((f) => f.path);
+  assert.deepEqual(paths.sort(), ["a.md", "b.md"]);
+  assert.deepEqual(
+    manifest.files.filter((f) => f.path === "a.md"),
+    [file("a.md", "h1")],
+  );
+  assert.equal(objects.has("c.md"), false);
+
+  // The next pass sees both as plain local changes and pushes them.
+  const retry = await syncOnce(outcome.snapshot, reader, writer, storage, 1);
+  assert.equal(retry.ok, true);
+  assert.equal(objects.get("a.md"), "edited mid sync");
+  assert.equal(objects.get("c.md"), "created mid sync");
+});
+
 test("syncOnce: two first syncs racing for an empty bucket, the loser fails instead of clobbering", async () => {
   // Both devices see no manifest and plan a first sync. The other device's manifest lands while
   // this one is mid pass; the "ifAbsent" conditional upload must lose rather than overwrite it.
@@ -188,3 +237,29 @@ test("syncOnce: two first syncs racing for an empty bucket, the loser fails inst
   assert.equal(objects.get(MANIFEST_KEY), otherManifest);
   assert.equal(files.get("a.md"), "alpha");
 });
+
+test("adoptLiveStats: an entry whose content matches the live vault adopts the live stats", () => {
+  const manifest = snapshot({ path: "a.md", size: 2, mtime: 5, hash: "h1" });
+  const live = snapshot({ path: "a.md", size: 2, mtime: 9, hash: "h1" });
+
+  assert.deepEqual(adoptLiveStats(manifest, live), live);
+});
+
+test("adoptLiveStats: a mid sync edit keeps the manifest's entry, so the next diff sees it", () => {
+  const manifest = snapshot(file("a.md", "h1"));
+  const live = snapshot({ path: "a.md", size: 7, mtime: 9, hash: "h2" });
+
+  assert.deepEqual(adoptLiveStats(manifest, live), manifest);
+});
+
+test("adoptLiveStats: a mid sync deletion keeps the manifest's entry, so the next diff sees it", () => {
+  const manifest = snapshot(file("a.md", "h1"));
+
+  assert.deepEqual(adoptLiveStats(manifest, empty), manifest);
+});
+
+test("adoptLiveStats: a mid sync creation is never added to the manifest", () => {
+  const live = snapshot(file("c.md", "h9"));
+
+  assert.deepEqual(adoptLiveStats(empty, live), empty);
+});

src/sync/sync.ts

@@ -1,7 +1,14 @@
 import type { PutCondition, StorageClient } from "../storage/storage.ts";
-import { isSnapshot, type Reader, type Snapshot, takeSnapshot } from "../vault/vault.ts";
+import {
+  byPath,
+  type FileState,
+  isSnapshot,
+  type Reader,
+  type Snapshot,
+  takeSnapshot,
+} from "../vault/vault.ts";
 import { executeSyncPlan, type LocalWriter, type SyncFailure } from "./execute.ts";
-import { MANIFEST_KEY, planSync } from "./plan.ts";
+import { MANIFEST_KEY, manifestAfterSync, planSync } from "./plan.ts";
 
 // SyncOutcome is the result of a single sync pass. On success it carries the new snapshot to
 // persist as the next sync's starting point and how many actions were applied; on failure it
@@ -10,6 +17,27 @@ export type SyncOutcome =
   | { ok: true; snapshot: Snapshot; changeCount: number }
   | { ok: false; message: string; failures: SyncFailure[] };
 
+// adoptLiveStats returns manifest with each entry swapped for the live vault's entry at the same
+// path wherever the content hashes match, so state.json carries local size and mtime and the next
+// snapshot can stat-skip the rehash. An entry whose live content differs (a mid sync edit) and a
+// live file the manifest doesn't know (a mid sync creation) both keep the manifest's view, so the
+// next sync's diff picks them up as local changes. Exported for its tests; syncOnce is the only
+// production caller.
+export function adoptLiveStats(manifest: Snapshot, live: Snapshot): Snapshot {
+  const liveByPath = byPath(live.files);
+  const files: FileState[] = [];
+  for (const entry of manifest.files) {
+    const liveEntry = liveByPath.get(entry.path);
+    if (liveEntry !== undefined && liveEntry.hash === entry.hash) {
+      files.push(liveEntry);
+      continue;
+    }
+    files.push(entry);
+  }
+
+  return { files };
+}
+
 // readRemoteManifest fetches and parses the remote manifest. A confirmed 404 means no manifest
 // has ever been written, the safe assumption for a first sync against an empty bucket, so that's
 // treated as an empty snapshot flagged firstSync. Any other failure (network, auth, a real 5xx)
@@ -63,8 +91,8 @@ export async function readRemoteManifest(
 
 // syncOnce runs one full sync pass over the injected local vault (reader/localWriter) and remote
 // bucket (storage): it snapshots the local vault against previous (the last synced snapshot),
-// reads the remote manifest, plans and executes the reconciliation, then re-snapshots and uploads
-// the manifest so it always matches what is really on disk. previous is passed in and the new
+// reads the remote manifest, plans and executes the reconciliation, then uploads a manifest
+// reflecting what the bucket now actually holds. previous is passed in and the new
 // snapshot returned rather than read or written internally, so the caller owns persistence (the
 // plugin through state.json, tests through their own store) and this stays pure over its inputs.
 // now is injected so a conflict copy's name is deterministic under test.
@@ -98,10 +126,14 @@ export async function syncOnce(
     return { ok: false, message: `${failures.length} file(s) failed to sync`, failures };
   }
 
-  // Re-snapshot rather than hand merging local with the plan's outcome: this is the only way to
-  // be sure the manifest we upload matches what's really on disk after every pull, delete, and
-  // conflict rename just applied.
-  const final = await takeSnapshot(reader, local);
+  // The manifest is derived from what the plan just did to the bucket, never from a fresh disk
+  // snapshot: a file edited while the plan ran would land in a re-snapshot claiming content the
+  // bucket never received, the edit would then never upload (state.json already agrees with the
+  // manifest), and another device could later push the stale bucket copy back over it (#84). The
+  // re-snapshot here only refreshes stats, so a mid sync edit keeps its bucket entry and reads as
+  // a local change on the next pass.
+  const manifest = manifestAfterSync(local, remote.snapshot, actions, now);
+  const final = adoptLiveStats(manifest, await takeSnapshot(reader, local));
   const manifestBody = new TextEncoder().encode(JSON.stringify(final));
 
   // The upload is conditional on the remote manifest still being exactly what this pass read at