Better sync: defer hot paths, rebase before park by usergenic · Pull Request #19 · usergenic/mrplex

usergenic · 2026-08-31T05:50:18Z

Summary

Adds docs/better-sync.plan and archives completed design docs into docs/archive/.
Implements inbound rebase-before-park: when local bytes are dirty and remote has advanced, sync tries docs.put onto remote current before creating a conflict sibling.
Adds mtime settle gating on both directions (--settle): hot files are skipped on push/feed; inbound work is held in an in-memory deferred map and retried on later polls (cursor still advances).
Introduces shared converge.ts / hot-path.ts helpers used by feed, reconcile, push, and the daemon.

Test plan

npx vitest run test/sync-*.test.ts (81 tests)
Manual: run mrplex sync <vault> --settle 3000 while editing a note; confirm no sibling appears mid-edit and convergence happens after save settles
Manual: two-writer scenario — local edit + concurrent remote change should rebase in place when puts succeed, sibling only on true divergence

Made with Cursor

Co-authored-by: Cursor <cursoragent@cursor.com>

Inbound feed and reconcile now try optimistic rebase onto remote current before parking conflict siblings, and the daemon holds hot-file work in memory with settle-based mtime gating on both push and feed directions. Co-authored-by: Cursor <cursoragent@cursor.com>

usergenic

Code Review

Verdict: APPROVE WITH RESERVATIONS

Well-structured decomposition of the sync convergence logic into shared helpers with a clean decision/effect split; the settle gate and deferred map are the right abstractions. A few control-flow gaps deserve attention before merge.

0 critical · 3 important · 2 minor

usergenic · 2026-08-31T06:14:08Z

+  if (result === "parked" || decision.action === "noop") {
+    // noop here would mean ignored; occupied diverge shouldn't noop.
+    if (result === "parked") {
+      return { path, verdict: "conflict", detail: "rebase failed; parked remote sibling" };
+    }
+  }
+  if (decision.action === "rebase" || result === "applied") {
+    return { path, verdict: "rebase", detail: "local bytes put onto remote current" };
+  }
+  if (decision.action === "adopt") {
+    return { path, verdict: "adopt", detail: "content matches remote; provenance repaired" };
+  }
+  return { path, verdict: "conflict", detail: "could not converge occupied path" };


(important) convergeOccupied returns incorrect "conflict" verdict when decideInbound returns noop

If decideInbound returns noop (e.g. the divergence resolves between the index scan and this call, or the file becomes $sync: ignore in a race), effectInbound returns noop, and the function falls through all conditions to line 368, returning { verdict: "conflict", detail: "could not converge occupied path" }. This misreports a benign noop as a conflict. Consider handling the result === "noop" case explicitly — either return { verdict: "clean" } or at minimum log it separately so it does not show up as a conflict in the reconcile report.

Fixed in 93ec11e — noop now returns clean/ignored instead of conflict.

usergenic · 2026-08-31T06:14:08Z

+    try {
+      remote = await client.docs.get_version(repo, entry.ref.version_id);
+    } catch {
+      return { done: true, result: "noop" };
+    }


(important) Bare catch permanently drops deferred entries on transient errors

This catch block swallows all errors — network timeouts, auth failures, server errors — not just doc_not_found. The resulting { done: true, result: "noop" } tells the daemon to permanently remove the deferred entry, silently abandoning convergence for that path. Consider only catching KernelError with doc_not_found code (consistent with the pattern at lines 269-274 above) and re-throwing everything else so transient failures trigger the daemon's existing defer retry error log+continue rather than a silent drop.

Fixed in 93ec11e — only doc_not_found is caught; transient errors propagate to defer retry.

usergenic · 2026-08-31T06:14:08Z

    if (err instanceof KernelError && err.code === "create_conflict") {
-      // A doc appeared at this path since the index scan; treat the occupied
-      // path as a conflict rather than clobbering it.
-      const current = (err.data as { current_version_id?: string }).current_version_id;
-      if (current) await parkConflict(client, store, repo, path, current);
+      // A doc appeared at this path since the index scan; rebase or park.
+      await convergeOccupied(client, store, repo, path);
      return;


(important) pushCreate discards the SyncAction returned by convergeOccupied

convergeOccupied returns a SyncAction indicating whether it rebased, adopted, or parked a sibling, but pushCreate is void and discards it. The caller at line 246 then unconditionally reports { verdict: "push", detail: "local creation" } regardless of the actual outcome. This means the reconcile report will say "push" when the real action was a rebase or conflict park, which can mislead operators reviewing sync behavior.

Fixed in 93ec11e — pushCreate returns SyncAction; callers propagate converge outcomes.

usergenic · 2026-08-31T06:14:08Z

+            park = await client.docs.get(repo, path);
+          } catch {
+            // Keep get_version result.
+          }


(minor) Bare catch in rebase fallback swallows non-retrieval errors

This catch-all is meant to handle the case where docs.get fails because the doc was deleted or moved, falling back to the get_version result. But it also swallows network errors, auth failures, etc. — parking a potentially stale version instead of surfacing the problem. Consider narrowing to KernelError with doc_not_found for consistency with the error-handling pattern elsewhere in this file.

Fixed in 93ec11e — docs.get fallback only catches doc_not_found.

usergenic · 2026-08-31T06:14:08Z

+    // Fast path: bytes already match — may still need provenance repair.
    if (intr.computed_hash === ref.content_hash) {
-      // Bytes already present (our own push echoing back, or a vault copy).
-      map?.set(ref.path, { version_id: ref.version_id, content_hash: ref.content_hash });
-      // Repair provenance only if the embedded version lags.
-      if (intr.version === ref.version_id) return false;
-      if (versionAtOrAhead(intr.version, ref.version_id)) return false;
-      const v = await client.docs.get_version(repo, ref.version_id);
-      await store.write(ref.path, renderMaterialized(v), { preserveMtime: true });
-      return true;
-    }
-    // Local is at or ahead of this ref (echo of our push, or a replay of an
-    // older version). Never clobber and never park a sibling of something we
-    // already have — including when the user has typed more since (dirty).
-    if (versionAtOrAhead(intr.version, ref.version_id)) return false;
-    if (isDirty(intr)) {
-      // A local edit collides with a *newer* incoming version → conflict, not
-      // overwrite. Park the remote as an ignored sibling; keep local bytes.
-      const v = await client.docs.get_version(repo, ref.version_id);
-      await store.write(withVersionSuffix(ref.path, ref.version_id), renderIgnoredSibling(v));
-      log(`feed conflict\t${ref.path}`);
-      return true;
+      opts.map?.set(ref.path, { version_id: ref.version_id, content_hash: ref.content_hash });
+      if (intr.version === ref.version_id) return "noop";
+      if (versionAtOrAhead(intr.version, ref.version_id)) return "noop";
+      const hot = await pathIsHot(store, ref.path, settleMs, nowMs);
+      const v = await client.docs.get_version(opts.repo, ref.version_id);
+      const decision = decideInbound({
+        localText: existing,
+        remote: v,
+        hot,
+        canDefer,
+      });
+      const result = await effectInbound(client, store, opts.repo, ref.path, decision, {
+        deferred: opts.deferred,
+        ref,
+        map: opts.map,
+        nowMs,
+      });
+      if (result === "deferred") return "deferred";
+      if (result === "noop") return "noop";
+      log(`feed adopt\t${ref.path}`);
+      return "applied";
    }


(minor) Hash-equal fast path re-parses intrinsics via decideInbound

When computed_hash === ref.content_hash and provenance needs repair, the code fetches the full version and routes through decideInbound, which re-reads intrinsics and re-checks the hash condition that was already confirmed at line 154. This is correct but redundant on the echo-suppression hot path (every feed poll). A direct materializeAt with preserveMtime (the adopt action) would be equivalent and skip the re-parse, since the only possible decideInbound outcome here is adopt or defer.

Fixed in 93ec11e — cold hash-equal path uses materializeAt directly; hot still defers.

Return accurate SyncActions from pushCreate, map noop convergence to clean, narrow deferred/rebase catch blocks to doc_not_found, and streamline the hash-equal feed adopt path. Co-authored-by: Cursor <cursoragent@cursor.com>

usergenic · 2026-08-31T06:20:15Z

Review follow-up pushed in 93ec11e:

Thread	Action
`convergeOccupied` noop → conflict	Fixed — noop returns `clean` ("already converged") or `ignored`
Deferred bare `catch` drops entries	Fixed — only `doc_not_found` swallowed; transient errors keep deferred entry
`pushCreate` discards `SyncAction`	Fixed — returns `SyncAction`; callers propagate; resurrect keeps correct verdict
Rebase park bare `catch`	Fixed — `docs.get` fallback only catches `doc_not_found`
Hash-equal redundant `decideInbound`	Fixed — cold path uses direct `materializeAt`; hot path still defers

CI typecheck failure is pre-existing on main (test/kernel-suite.ts LinkSyntaxes), not introduced by this PR.

Matches the rename case so chokidar/debounce latency in a busy vitest worker does not flake at the default 8s deadline. Co-authored-by: Cursor <cursoragent@cursor.com>

usergenic and others added 2 commits August 30, 2026 19:52

Add better-sync plan and archive completed sync design docs.

48807e9

Co-authored-by: Cursor <cursoragent@cursor.com>

usergenic commented Aug 31, 2026

View reviewed changes

Fix review feedback on reconcile verdicts and error handling.

93ec11e

Return accurate SyncActions from pushCreate, map noop convergence to clean, narrow deferred/rebase catch blocks to doc_not_found, and streamline the hash-equal feed adopt path. Co-authored-by: Cursor <cursoragent@cursor.com>

Give watcher push daemon test 20s waitFor headroom under load.

8a26e4d

Matches the rename case so chokidar/debounce latency in a busy vitest worker does not flake at the default 8s deadline. Co-authored-by: Cursor <cursoragent@cursor.com>

usergenic merged commit 5fe8342 into main Aug 31, 2026
0 of 5 checks passed

usergenic deleted the better-sync branch August 31, 2026 06:39

Navigation Menu

Better sync: defer hot paths, rebase before park - #19

Better sync: defer hot paths, rebase before park#19
usergenic merged 4 commits into
mainfrom
better-sync

usergenic commented Aug 31, 2026

Uh oh!

usergenic left a comment

Uh oh!

usergenic Aug 31, 2026

Uh oh!

usergenic Aug 31, 2026

Uh oh!

usergenic Aug 31, 2026

Uh oh!

usergenic Aug 31, 2026

Uh oh!

usergenic Aug 31, 2026

Uh oh!

usergenic Aug 31, 2026

Uh oh!

usergenic Aug 31, 2026

Uh oh!

usergenic Aug 31, 2026

Uh oh!

usergenic Aug 31, 2026

Uh oh!

usergenic Aug 31, 2026

Uh oh!

usergenic commented Aug 31, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

usergenic commented Aug 31, 2026

Summary

Test plan

Uh oh!

usergenic left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

usergenic commented Aug 31, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant