Sync → Interrupted pulls no longer leave torn files#118
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #88
Problem
Changes
Why
Greptile Summary
This PR fixes the torn-file corruption described in #88 by staging every pulled write to a hidden dot-prefixed sibling (
.geode-tmp) and renaming it into place, so the destination holds either its full previous content or the complete new content — never partial bytes. The integration-testwalkhelper is updated to skip all dot-prefixed entries, matching Obsidian's own hidden-file exclusion so staged files cannot surface in a snapshot.writeThroughTempwrites to the temp sibling and renames atomically; a rename failure is caught and, if the destination still exists, delegated toreplaceViaAsidewhich moves the destination aside first so a second failure restores the original rather than deleting it.hiddenSiblingPathuses a deterministic naming scheme so any leftover from a previous crash is reclaimed by the next write to the same path, not accumulated.writeBinary.Confidence Score: 5/5
Safe to merge — the staging logic is correct for every tested failure mode and the previous review concern about destination loss on a non-destination-existence rename error is now addressed by the restore path in
replaceViaAside.All three files are narrowly scoped to the write-through-temp path. The logic correctly preserves the destination in every crash window: interrupted
writeBinary(destination untouched), successful rename (destination fully replaced), and a rename that fails for a non-destination reason (destination restored via aside). The test suite exercises each of these branches and the assertions match the implementation.No files require special attention.
Important Files Changed
writeThroughTemp,hiddenSiblingPath, andreplaceViaAsideto stage pulls via a hidden sibling file and rename into place; all failure paths restore the destination correctly.walkskip from the literal.obsidianname to any dot-prefixed entry, hiding staged temp files from the integration-test vault snapshot.Reviews (2): Last reviewed commit: "Address comment" | Re-trigger Greptile