Skip to content

Prevent legacy conflict copies from reappearing on disk - #80

Open
mrprince88 wants to merge 1 commit into
kavinsood:mainfrom
mrprince88:fix/legacy-conflict-writeback
Open

mrprince88 wants to merge 1 commit into
kavinsood:mainfrom
mrprince88:fix/legacy-conflict-writeback

Conversation

@mrprince88

Copy link
Copy Markdown

Summary

YAOS already excludes generated Markdown conflict copies from new uploads, but copies written by older clients can remain in the shared CRDT. Reconciliation and remote writeback can recreate a copy after the user deletes it locally, or overwrite a local safety copy.

This change keeps those legacy CRDT entries for recovery while excluding their paths from reconciliation disk create/update plans and from all disk writeback, including forced writes. It also recognizes uppercase .MD conflict copies. Ordinary notes continue to sync normally.

Validation

  • New regression reproduces creation, forced recreation, and overwrite of legacy conflict copies.
  • Focused suites passed: legacy-conflict-writeback, markdown-conflict-artifact, disk-mirror-observer, no-event-reconcile-admission, and reconciliation-safety-brake.
  • npm run typecheck:tests, npm run build, and npm run guard:production-bundles passed.

The production main.js is minified by the existing build process and is intentionally not committed because generated bundles are excluded from source control.

@mrayhanfadil

Copy link
Copy Markdown

I tested this against the bug in #78. In my vault, 13 legacy conflict copies were still in the CRDT and came back after every delete. My root-cause comment has the details.

  • On main (2174eb8), the new legacy-conflict-writeback suite fails 8 checks. On this branch it passes, and so do markdown-conflict-artifact, disk-mirror-observer, no-event-reconcile-admission and reconciliation-safety-brake (Node 22).
  • The two gates cover the writeback paths I traced. That includes the main one: diskPresentPaths excludes artifacts, so on main a deleted artifact lands in createdOnDisk on every reconcile and flushWrite recreates it.

One gap this PR doesn't cover is the editor binding. resolveBindingTarget (editorBinding.ts#L1410-L1434) binds to the existing Y.Text for the path, or calls vaultSync.ensureFile(file.path, …) if there isn't one. Neither step checks isMarkdownConflictArtifactPath. That has two effects:

  • Opening a conflict copy in the editor still admits it to the CRDT. That's how new artifacts keep getting in even after 614dd7e. With this PR they'd stay hidden on other devices, but they'd keep piling up in the room.
  • Opening a legacy copy binds the editor to the shared text. Remote edits would then reach the file through y-codemirror and Obsidian's save, bypassing the flushWrite gate.

One possible fix is an early return in resolveBindingTarget:

if (isMarkdownConflictArtifactPath(file.path)) return null;

repair() and heal() treat a null target as a failure unless isHardTombstonedPath is true, so artifacts probably need the same "intentionally unbound" treatment there, to avoid repeated rebind attempts. Happy to open that as a follow-up PR on top of this one if it helps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants