Fix upstream deletes across independent peer revision spaces - #185
github-actions[bot] wants to merge 1 commit into
Conversation
Gardener-Operation: op_5e746d3660443aa9d21345f7e9a13aa41dc4605e4a9a3d8b3210451bd0331dd8:bc1b93cc3ac5de32532ffba0acf0db9b7c51d98bbdbe7fb2097be51daad0fade
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| } | ||
| const row = db.one<{ rev: number }>("SELECT rev FROM vfs_nodes WHERE inode = ?", live.inode); | ||
| return row !== undefined && row.rev > entry.rev; | ||
| return row !== undefined && row.rev > readWatermark(db, "pushRev", options.backend); |
There was a problem hiding this comment.
馃敶 Remote deletes resurrect pulled files
If a remote file is deleted before its earlier pull is echoed, tombstoneIsStale retains the pulled copy. The next push recreates the deleted file remotely.
Learn more
A pull applies a remote file through applyChanges, giving its local inode a fresh revision without advancing the push watermark. If the remote deletes that file before an echo push, the new guard mistakes the pulled copy for an unpushed local edit and skips the tombstone. The pull checkpoints past the deletion, and pushBlocks later exports the retained file as a live change. The remote consequently gets its deleted file back.
Example: The remote creates /x; the host pulls it at local rev 1 with pushRev 0. The remote deletes /x before the host's next push. The host ignores the delete, checkpoints it, then pushes /x back.
Recommended fix: Distinguish local authored changes from revisions minted by upstream apply when guarding pulls, or otherwise track the provenance of versions that have not yet been echoed. Ensure a later remote delete wins over an earlier pulled file while locally authored, unpushed edits remain protected.
Was this helpful? React with 馃憤 or 馃憥 to provide feedback.
| const row = db.one<{ rev: number }>("SELECT rev FROM vfs_nodes WHERE inode = ?", live.inode); | ||
| return row !== undefined && row.rev > entry.rev; | ||
| return row !== undefined && row.rev > readWatermark(db, "pushRev", options.backend); |
There was a problem hiding this comment.
馃敶 Directory deletes discard unpushed child edits
When a child changes beneath a previously pushed directory, tombstoneIsStale checks only the directory's old revision. A remote directory delete recursively removes the unpushed child edit.
Learn more
The guard compares the directory inode's revision against pushRev, but changing a child does not update that directory inode. Once the directory passes the guard, rm deletes its entire subtree, including children that were edited locally after the last push. The pull then checkpoints the remote tombstone, so those child changes cannot be sent upstream.
Example: Push /project and /project/draft at rev 2. Edit /project/draft locally at rev 3, without pushing it. A remote delete of /project passes because /project remains at rev 1 or 2 and pushRev is 2, destroying the draft.
Recommended fix: For a directory tombstone, inspect descendant live revisions or track dirty subtrees for the selected backend before calling recursive rm. Protect unpushed descendants without treating a clean directory as dirty.
Was this helpful? React with 馃憤 or 馃憥 to provide feedback.
| } | ||
| const row = db.one<{ rev: number }>("SELECT rev FROM vfs_nodes WHERE inode = ?", live.inode); | ||
| return row !== undefined && row.rev > entry.rev; | ||
| return row !== undefined && row.rev > readWatermark(db, "pushRev", options.backend); |
There was a problem hiding this comment.
馃敶 Partial pushes lose unshipped same-revision changes
After a partial same-revision push, tombstoneIsStale treats every path at pushRev as shipped. A remote delete can remove a later, unshipped path before its push.
Learn more
A directory rename stamps all moved nodes with one shared revision in stampRenamedSubtree. A bounded push block can checkpoint partway through that revision, recording both its rev and its last path. readWatermark(..., 'pushRev') discards the path component, so the guard treats later, still-unshipped paths at the same revision as clean. An incoming delete can erase one of those pending changes.
Example: Rename a directory with 3,000 files at rev 40 and ship only the first 2,000 paths, recording {rev:40,path:'/new/f1999'}. If the remote deletes /new/f2999 before the next push, the host accepts that deletion even though /new/f2999 was never shipped.
Recommended fix: Use readPushCursor and compare both revision and path for a live entry rather than comparing against the scalar push watermark. Account for paths whose inode revision was stamped by a rename.
Was this helpful? React with 馃憤 or 馃憥 to provide feedback.
Summary
Addresses #167. Requested by #167 (comment).
The existing tombstone guard compares a local node revision with the sender's independent revision counter. Four new integration regressions fail against the original implementation: entry and pack pulls after a high-revision host push, and entry and pack pushes into a higher-revision receiver.
Validation
No real container was required or exercised. Pulls conservatively retain local versions not yet pushed to the selected backend.