sync-bak/sync-tmp: each suffix gets one consistent meaning

Conceptual fix: before this change, `.sync-bak` was ambiguous — it
meant "old bytes backed up" in atomicWriteFile's 5-step pull-replace
protocol AND "new bytes staged" in ConflictStore.create's 3-step
sibling registration. The same suffix encoding opposite semantics
made the recovery sweep harder to reason about and trapped readers
who assumed "bak" implied a backup of something pre-existing.

After:
- `.sync-tmp` = NEW bytes staged for a target.
  Ambiguous between two callsites; the recovery sweep dispatches
  by ownership via ConflictStore.getBySibling(finalPath):
    - record exists → forward-finalize (Path B sibling registration)
    - no record → drop as transient (Path A pull-replace artefact)
- `.sync-bak` = OLD bytes backed up before an overwrite.
  Produced only by atomicWriteFile; ConflictStore never writes
  `.sync-bak` after this change. Recovery is snapshot-based with no
  ownership dispatch needed — simpler code path.

Changes:
- ConflictStore.create now stages at `<sibling>.sync-tmp.<ext>`
  (was `.sync-bak.<ext>`). 3-step protocol unchanged otherwise.
- AtomicWriteRecovery.sweep moves the ownership-dispatch block
  from the `.sync-bak` loop into the `.sync-tmp` loop.
  `.sync-bak` handling simplifies to snapshot-based recovery only.
- File-level comments in atomic-write.ts + conflict-store.ts
  updated to describe the new contract.
- atomic-write.test.ts: N9/N9b renamed (.sync-bak → .sync-tmp in
  titles + stagingPathFor() arg). Existing orphan-tmp test retitled
  for clarity. New N9c pins the dispatch: a Path A transient
  .sync-tmp must drop even when ConflictStore is in the recovery
  constructor, as long as no record names its finalPath.
- conflict-store.test.ts: 2 comments + 1 test title updated.

Migration: at rest there are no `.sync-bak` staging files for
ConflictStore-owned siblings (Step 3 promotes them immediately on
success), so existing installations migrate transparently. The only
edge case — an upgrade landing exactly during a mid-Step-3 crash
window — is benign because the conservative-restore branch in the
`.sync-bak` handler would still rename the leftover into place,
which is the same outcome the new code would produce via the
.sync-tmp branch.

Tests: 526/526 unit pass (was 525, +1 for N9c). Build clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: dbdeveloper

src/sync2/atomic-write.ts

@@ -205,36 +205,41 @@ interface ConflictStoreLike {
 }
 
 // Crash-recovery sweep for `atomicWriteFile` AND for ConflictStore's
-// vault-level `.sync-bak` sibling staging. Runs on plugin onload
+// vault-level `.sync-tmp` sibling staging. Runs on plugin onload
 // BEFORE the engine starts touching the vault — walks the tree for
 // any `.sync-tmp` / `.sync-bak` leftovers and reconciles them
 // against the snapshot + conflict stores.
 //
-// `.sync-tmp` files: always safe to drop (transient write artifact).
+// Each suffix now has ONE consistent meaning (see PSEUDO-MERGE-MODE.md
+// §9 for the rationale):
 //
-// `.sync-bak` files: dispatch by ownership. If `conflictStore.
-// getBySibling(finalPath)` returns a record, we treat the staging
-// as a Stage 13 conflict-sibling stage (PSEUDO-MERGE-MODE.md §"Recovery
-// sweep на onload — vault-level `.sync-bak` sweep"):
+//   `.sync-tmp` = NEW bytes staged for a target (existing or new).
+//      Ambiguous between two callsites; dispatch by ownership via
+//      conflictStore.getBySibling(finalPath):
+//        record exists, finalPath exists                    → drop tmp
+//                                                            [Step 3 done,
+//                                                             orphan cleanup]
+//        record exists, finalPath missing, SHA matches      → rename(tmp →
+//                                                             finalPath)
+//                                                            [resume Step 3]
+//        record exists, finalPath missing, SHA mismatches   → drop tmp
+//                                                            [data integrity
+//                                                             > resolution;
+//                                                             record dropped
+//                                                             on next drain
+//                                                             Phase B]
+//        no record (Path A transient new bytes)              → drop tmp
 //
-//   finalPath exists                         → delete bak [Step 3 done,
-//                                              orphan cleanup]
-//   finalPath missing, SHA(bak) === theirsBlobSha → rename(bak → finalPath)
-//                                              [resume Step 3]
-//   finalPath missing, SHA(bak) ≠ theirsBlobSha   → delete bak (data
-//                                              integrity > resolution
-//                                              completeness; record
-//                                              gets dropped on next
-//                                              drain Phase B)
-//
-// Otherwise (no record owns finalPath) the sweep falls through to the
-// existing snapshot-based atomicWriteFile recovery semantics:
-//
-//   finalPath missing                        → rename(bak → finalPath)
-//                                              [restore]
-//   finalPath exists, snapshot.remoteSha === SHA(file) → delete bak
-//                                              [cleanup race]
-//   mismatch / no snapshot                   → restore bak
+//   `.sync-bak` = OLD bytes backed up before an overwrite.
+//      Only produced by atomicWriteFile (Path A); ConflictStore never
+//      writes `.sync-bak`. Recovery is snapshot-based, no ownership
+//      dispatch needed:
+//        finalPath missing                                  → rename(bak
+//                                                             → finalPath)
+//                                                            [restore]
+//        finalPath exists, snapshot.remoteSha === SHA(file) → delete bak
+//                                                            [cleanup race]
+//        mismatch / no snapshot                             → restore bak
 //
 // Returns counts so main.ts can log / surface what was recovered.
 export class AtomicWriteRecovery {
@@ -250,49 +255,51 @@ export class AtomicWriteRecovery {
 
     const { syncTmps, syncBaks } = await this.findCandidates();
 
-    // 1. .sync-tmp: always safe to drop. Either the write was
-    // interrupted before promotion, or someone left stale staging.
-    for (const { stagingPath: tmpPath } of syncTmps) {
+    // 1. .sync-tmp: forward-direction staging. Dispatch by ownership.
+    // Path B (ConflictStore.create) → resume Step 3 by renaming to the
+    // final sibling path if SHA matches the record's theirsBlobSha.
+    // Path A (atomicWriteFile transient) → drop (next sync repeats).
+    for (const { stagingPath: tmpPath, finalPath: originalPath } of syncTmps) {
       try {
-        await this.vault.adapter.remove(tmpPath);
-        cleaned++;
-      } catch {
-        // Ignore individual failures; sweep is best-effort.
-      }
-    }
-
-    // 2. .sync-bak: state-driven recovery. Dispatch by ownership.
-    for (const { stagingPath: bakPath, finalPath: originalPath } of syncBaks) {
-      try {
-        // Stage 13: ConflictStore owns siblings — record-bound
-        // recovery uses theirsBlobSha as the integrity witness.
         const conflictRecord = this.conflictStore?.getBySibling(originalPath);
         if (conflictRecord !== undefined) {
           const fileExists = await this.vault.adapter.exists(originalPath);
           if (fileExists) {
             // Step 3 completed at some point; the staging is stale.
-            await this.vault.adapter.remove(bakPath);
+            await this.vault.adapter.remove(tmpPath);
             cleaned++;
             continue;
           }
-          const bytes = await this.vault.adapter.readBinary(bakPath);
+          const bytes = await this.vault.adapter.readBinary(tmpPath);
           const sha = await calculateGitBlobSHA(bytes);
           if (sha === conflictRecord.theirsBlobSha) {
             // Resume the interrupted Step 3.
-            await this.vault.adapter.rename(bakPath, originalPath);
+            await this.vault.adapter.rename(tmpPath, originalPath);
             restored++;
           } else {
             // SHA mismatch — disk corruption or a stale staging from
             // some unrelated path that happens to collide. Drop it;
             // the next drain Phase B drops the record on the missing
             // sibling.
-            await this.vault.adapter.remove(bakPath);
+            await this.vault.adapter.remove(tmpPath);
             cleaned++;
           }
           continue;
         }
+        // No ConflictStore record → Path A transient. Always safe to
+        // drop; next sync repeats the operation if still needed.
+        await this.vault.adapter.remove(tmpPath);
+        cleaned++;
+      } catch {
+        // Ignore individual failures; sweep is best-effort.
+      }
+    }
 
-        // Fall-through: atomicWriteFile-style snapshot-based recovery.
+    // 2. .sync-bak: rollback backups, snapshot-based recovery.
+    // Produced only by atomicWriteFile (Path A); ConflictStore never
+    // writes .sync-bak. No ownership dispatch needed.
+    for (const { stagingPath: bakPath, finalPath: originalPath } of syncBaks) {
+      try {
         const fileExists = await this.vault.adapter.exists(originalPath);
         if (!fileExists) {
           // Crash between step 2 and step 3: only backup survived.

src/sync2/conflict-store.ts

@@ -17,26 +17,26 @@ import { stagingPathFor } from "./atomic-write";
 //   <configDir>/plugins/<self>/.conflicts/
 //     <recordId>/
 //       meta.json          ← ConflictRecord JSON (atomically written)
-//       sibling-content.bin ← raw bytes of the theirs side; permanent
-//                             backup (re-used by per-crash-window
-//                             recovery sweep + as authoritative
-//                             content if the user externally deleted
-//                             the vault sibling)
 //
 // The sibling file itself lives in the vault next to the original:
 //
 //   <basename>.conflict-from-<deviceLabel>-<isoTs>.<ext>
+//
+// During create(), the sibling is staged at vault level as a
+// `.sync-tmp` pre-suffix file (e.g., `note.conflict-from-Phone-...sync-tmp.md`)
+// and atomically renamed to its final name. Crash-recovery is handled
+// by AtomicWriteRecovery.sweep() via ownership dispatch on .sync-tmp.
+// See PSEUDO-MERGE-MODE.md §9 for the full protocol.
 
 const CONFLICTS_DIRNAME = ".conflicts";
 const META_FILE = "meta.json";
 const META_TMP_FILE = "meta.json.tmp";
 // Legacy: pre-Stage-13 ConflictStore wrote a `sibling-content.bin`
-// backup file inside each `<recordDir>` for crash recovery. Stage 13
-// uses vault-level `.sync-bak` staging instead (see create() below)
-// and load() no longer consults the backup at all. Old install dirs
-// may still carry a `sibling-content.bin` artifact; it's harmless
-// dead weight and gets removed by `delete()` along with the rest of
-// the recordDir.
+// backup file inside each `<recordDir>` for crash recovery. Removed
+// when staging moved to vault-level `.sync-tmp` (see create() below).
+// Old install dirs may still carry a `sibling-content.bin` artifact;
+// it's harmless dead weight and gets removed by `delete()` along with
+// the rest of the recordDir.
 
 // Two kinds: modify-vs-modify (both sides edited) and delete-vs-modify
 // (local deleted, remote modified). The third theoretical kind —
@@ -258,14 +258,17 @@ export default class ConflictStore {
     const siblingSha = orphan?.sha ?? (await calculateGitBlobSHA(args.theirsContent));
 
     const recordDir = `${this.conflictsRoot}/${id}`;
-    const stagingPath = stagingPathFor(siblingPath, "bak");
-
-    // Stage 13 (Decision #29): vault-level `.sync-bak` staging
-    // replaces the legacy `<recordDir>/sibling-content.bin` backup.
-    // 3-step protocol per PSEUDO-MERGE-MODE.md §"3-step atomic
-    // create protocol":
+    const stagingPath = stagingPathFor(siblingPath, "tmp");
+
+    // Vault-level `.sync-tmp` staging (Decision #29; suffix corrected
+    // post-Stage-13 to match semantics — see PSEUDO-MERGE-MODE.md §9).
+    // `.sync-tmp` carries NEW bytes destined for a not-yet-existing
+    // target; `.sync-bak` is reserved for backups of files that
+    // already existed (atomicWriteFile's rollback target). Sibling
+    // registration writes a brand-new file → tmp, not bak.
     //
-    //   1. writeBinary(<sibling>.sync-bak.<ext>, theirsContent)
+    // 3-step protocol:
+    //   1. writeBinary(<sibling>.sync-tmp.<ext>, theirsContent)
     //   2. atomic write meta.json (via persistRecord — tmp + rename)
     //   3. atomic rename staging → siblingPath
     //

tests/sync2/atomic-write.test.ts

@@ -31,13 +31,18 @@ import { calculateGitBlobSHA } from "../../src/utils";
 //   4. afterCommit()  ← snapshot.recordSync, typically
 //   5. remove(<path>.sync-bak)
 //
-// Crash-recovery sweep:
-//   *.sync-tmp                             → delete (junk)
-//   *.sync-bak (no <file>)                 → restore from .sync-bak
-//   *.sync-bak (with <file>):
-//     file SHA matches snapshot.remoteSha  → delete .sync-bak [cleanup race]
-//     mismatch                             → restore from .sync-bak
-//     no snapshot entry                    → restore from .sync-bak (conservative)
+// Crash-recovery sweep (post-Stage-13, suffix semantics corrected):
+//   *.sync-tmp:                           dispatch by ownership via
+//                                         ConflictStore.getBySibling
+//     no record (Path A transient)        → delete (junk)
+//     record + finalPath exists           → delete (Step 3 done, stale)
+//     record + finalPath missing, SHA ok  → rename .sync-tmp → finalPath
+//     record + finalPath missing, SHA bad → delete (record drops later)
+//   *.sync-bak (Path A only, no dispatch):
+//     no <file>                           → restore from .sync-bak
+//     <file> + SHA == snapshot.remoteSha  → delete .sync-bak [cleanup race]
+//     <file> + SHA mismatch               → restore from .sync-bak
+//     <file>, no snapshot entry           → restore from .sync-bak (conservative)
 
 function fixture(): {
   root: string;
@@ -176,7 +181,7 @@ describe("AtomicWriteRecovery.sweep", () => {
     f.cleanup();
   });
 
-  it("orphan .sync-tmp: deleted on sweep (transient write artifact)", async () => {
+  it("orphan .sync-tmp without ConflictStore in scope: dropped (Path A transient)", async () => {
     fs.writeFileSync(path.join(f.root, "x.sync-tmp.md"), "partial");
     const recovery = new AtomicWriteRecovery(
       f.vault as unknown as import("obsidian").Vault,
@@ -385,7 +390,7 @@ describe("AtomicWriteRecovery SHA-verify (Stage 13) — Phase 4 wires sweep agai
     f.cleanup();
   });
 
-  it("N9: sweep finds .sync-bak with SHA matching record.theirsBlobSha → rename to finalPath", async () => {
+  it("N9: sweep finds .sync-tmp with SHA matching record.theirsBlobSha → rename to finalPath", async () => {
     const { default: ConflictStore } = await import(
       "../../src/sync2/conflict-store"
     );
@@ -414,7 +419,7 @@ describe("AtomicWriteRecovery SHA-verify (Stage 13) — Phase 4 wires sweep agai
     });
     // Synthesize the mid-Step-3 crash state.
     const siblingAbs = path.join(f.root, rec.siblingPath);
-    const stagingAbs = path.join(f.root, stagingPathFor(rec.siblingPath, "bak"));
+    const stagingAbs = path.join(f.root, stagingPathFor(rec.siblingPath, "tmp"));
     fs.renameSync(siblingAbs, stagingAbs);
     expect(fs.existsSync(siblingAbs)).toBe(false);
     expect(fs.existsSync(stagingAbs)).toBe(true);
@@ -431,7 +436,7 @@ describe("AtomicWriteRecovery SHA-verify (Stage 13) — Phase 4 wires sweep agai
     expect(fs.readFileSync(siblingAbs, "utf8")).toBe("theirs content\n");
   });
 
-  it("N9b: sweep finds .sync-bak with SHA NOT matching record.theirsBlobSha → drop, leave record for drain Phase B", async () => {
+  it("N9b: sweep finds .sync-tmp with SHA NOT matching record.theirsBlobSha → drop, leave record for drain Phase B", async () => {
     const { default: ConflictStore } = await import(
       "../../src/sync2/conflict-store"
     );
@@ -458,7 +463,7 @@ describe("AtomicWriteRecovery SHA-verify (Stage 13) — Phase 4 wires sweep agai
     // content (SHA differs from record.theirsBlobSha — disk
     // corruption / race / unrelated staging collision).
     const siblingAbs = path.join(f.root, rec.siblingPath);
-    const stagingAbs = path.join(f.root, stagingPathFor(rec.siblingPath, "bak"));
+    const stagingAbs = path.join(f.root, stagingPathFor(rec.siblingPath, "tmp"));
     fs.unlinkSync(siblingAbs);
     fs.writeFileSync(stagingAbs, "corrupted bytes");
 
@@ -478,4 +483,39 @@ describe("AtomicWriteRecovery SHA-verify (Stage 13) — Phase 4 wires sweep agai
     expect(fs.existsSync(siblingAbs)).toBe(false);
     expect(conflictStore.get(rec.id)).toBeDefined();
   });
+
+  it("N9c: .sync-tmp at a path with no ConflictStore record → dropped as Path A transient (even when conflictStore is in scope)", async () => {
+    // Pin the dispatch: presence of conflictStore in the recovery
+    // constructor must NOT cause a Path A transient .sync-tmp (one
+    // whose finalPath is just an ordinary user file, not a sibling)
+    // to be treated as a forward-finalize candidate.
+    const { default: ConflictStore } = await import(
+      "../../src/sync2/conflict-store"
+    );
+    const conflictStore = new ConflictStore({
+      vault: f.vault as unknown as import("obsidian").Vault,
+      configDir: ".obsidian",
+      selfPluginId: "github-easy-sync",
+    });
+    await conflictStore.load();
+    // Place a .sync-tmp at a path that ConflictStore knows nothing
+    // about — e.g., from a crashed atomicWriteFile pull-replace.
+    fs.mkdirSync(path.join(f.root, "Notes"), { recursive: true });
+    fs.writeFileSync(path.join(f.root, "Notes/regular.sync-tmp.md"), "partial");
+
+    const recovery = new AtomicWriteRecovery(
+      f.vault as unknown as import("obsidian").Vault,
+      f.store,
+      conflictStore,
+    );
+    const result = await recovery.sweep();
+    expect(result.cleaned).toBe(1);
+    expect(result.restored).toBe(0);
+    expect(
+      fs.existsSync(path.join(f.root, "Notes/regular.sync-tmp.md")),
+    ).toBe(false);
+    // The "final" path (Notes/regular.md) was never created — drop is
+    // correct because the bytes are transient, not destined.
+    expect(fs.existsSync(path.join(f.root, "Notes/regular.md"))).toBe(false);
+  });
 });

tests/sync2/conflict-store.test.ts

@@ -181,8 +181,9 @@ describe("ConflictStore", () => {
       // meta.json persisted; tmp atomic-write artifact cleaned.
       expect(fs.existsSync(path.join(dir, "meta.json"))).toBe(true);
       expect(fs.existsSync(path.join(dir, "meta.json.tmp"))).toBe(false);
-      // Stage 13: sibling staging lives in the vault as a `.sync-bak`
-      // pre-suffix file, then atomically renamed to the final
+      // Sibling staging lives in the vault as a `.sync-tmp` pre-suffix
+      // file (NEW bytes destined for a brand-new sibling — semantically
+      // a "tmp", not a "bak"), then atomically renamed to the final
       // siblingPath. No legacy `sibling-content.bin` backup inside
       // the recordDir.
       expect(fs.existsSync(path.join(dir, "sibling-content.bin"))).toBe(false);
@@ -360,7 +361,7 @@ describe("ConflictStore", () => {
       // from meta.json; the missing sibling becomes a drain Phase B
       // drop signal. (Pre-Stage-13 also had a `sibling-content.bin`
       // backup file inside recordDir; that artifact is gone now —
-      // vault-level `.sync-bak` staging is the new mechanism and
+      // vault-level `.sync-tmp` staging is the new mechanism and
       // it's already been renamed to the final siblingPath by the
       // time create() returns.)
       await f.store.load();