fix: undo brings a move back across filesystems by nixfred · Pull Request #86 · thisisgm/flea

nixfred · 2026-09-08T19:28:40Z

The bug

Move a file from a real disk onto a USB stick, another partition or tmpfs, then press undo. The reversal answered {"t":"error","where":"rename",...,"msg":"Invalid cross-device link (os error 18)"} and the file stayed where it was moved to. Because Journal::undo pops the entry before reversing it, the entry was also spent: the next undo reached an older operation.

The forward path already knows a cross-filesystem move is a copy and a remove (copyfile.rs's move_any on EXDEV). The reverse path goes through renamecompat::rename_path, whose copy fallback only fired for an rclone EINVAL or a WebDAV EIO.

The fix

needs_copy_fallback answers true for EXDEV, so the reversal takes the same copy_then_remove arm the two measured mounts use: exclusive create, partial cleanup on a failed copy, and the rename-kept rule when the source will not go. docs/protocol.md's undo section says a move that crossed filesystems copies back the way it went out.

Tests

Unit test pins the errno in renamecompat.rs, alongside the existing WebDAV and rclone predicate tests.
tests/ops.sh gains a scenario that moves a file and a nested tree from the fixture root onto a /dev/shm tmpfs root and undoes them, using the same self-marked tmpfs root pattern tests/drag.sh R7 uses, with the device check that proves the two really are different filesystems. On the unfixed binary the scenario fails four checks: the undo answers a rename error and both sources stay gone.
cargo test: 442 passed, zero warnings in debug and release.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AnB5YmkdfX8Yri7gSGuAWf

Summary by CodeRabbit

Bug Fixes
- Cross-filesystem moves now complete successfully by copying items to the destination and removing the originals.
- Undoing a cross-filesystem move now restores the item to its original location and removes the destination copy.
Tests
- Added coverage for cross-filesystem file and directory moves, including undo behavior and cleanup.

The journal reverses a move through rename_path, whose copy fallback fired only for an rclone EINVAL or a WebDAV EIO, so undoing a move onto a USB stick, another partition or tmpfs answered "Invalid cross-device link" and, because the entry was already popped, the next undo reached an older operation. copyfile.rs's move_any copies and removes on EXDEV going out; rename_path now takes the same copy_then_remove arm on EXDEV coming back, which keeps the exclusive create, the partial cleanup and the rename-kept rule the two measured mounts already use. The errno is pinned in a unit test, and tests/ops.sh moves a file and a tree from the fixture root onto a /dev/shm tmpfs and undoes them, the pair drag.sh R7 already relies on; on the old code the undo answered a rename error and both sources stayed gone. LR-T: cargo test (442 passed); ./tests/ops.sh (all checks passed, and its new scenario fails on the unfixed binary) LR-D: a move to another filesystem could not be undone and the failed undo spent the journal entry LR-K: src/backend/renamecompat.rs; src/backend/undo.rs; tests/ops.sh; docs/protocol.md Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AnB5YmkdfX8Yri7gSGuAWf

coderabbitai · 2026-09-08T19:29:34Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 8bbb4b6d-da79-4cc6-a5c0-6eb162c3e701

📥 Commits

Reviewing files that changed from the base of the PR and between a41d34c and 2ad41e5.

📒 Files selected for processing (3)

docs/protocol.md
src/backend/renamecompat.rs
tests/ops.sh

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.

📝 Walkthrough

Walkthrough

The rename compatibility logic now treats EXDEV as a copy-and-remove fallback. Unit and end-to-end tests cover cross-filesystem file and directory moves, undo restoration, cleanup, and protocol documentation.

Changes

Cross-filesystem move handling

Layer / File(s)	Summary
EXDEV fallback detection `src/backend/renamecompat.rs`	The module defines `EXDEV`, selects the copy fallback for cross-filesystem rename errors, and tests file and directory cases.
Cross-filesystem move and undo validation `tests/ops.sh`, `docs/protocol.md`	The end-to-end test verifies cross-filesystem moves and undo restoration. Cleanup removes the temporary fixture safely. The protocol documents copy-before-remove behavior.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 2ad41

Cross-filesystem moves and their undo operations now use the established copy-and-remove behavior, restoring moved files and directories without leaving destination copies. The change is ready to merge.

Suggested reviewers: thisisgm

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: fixing undo for moves across filesystems.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1 unsupported.)

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

Eleven fixes over 0.1.6, each on its own branch and open as a PR against upstream (thisisgm#86 to thisisgm#96), merged here so a build carries them together. Cargo.toml, Cargo.lock and the source PKGBUILD move together, as for 0.1.6. LR-T: cargo test (448 passed, 0 warnings debug and release); ./tests/run-all.sh (23 run, the 4 failures are this box's: expect not installed, media fixture not built); ./tools/flea-qmllint-gate; ./tools/flea-file-budget (0 over cap) LR-D: the 0.1.7 release of the fork with the eleven bug-hunt fixes LR-K: Cargo.toml; Cargo.lock; PKGBUILD Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AnB5YmkdfX8Yri7gSGuAWf

Upstream moved 188 commits and two releases while these sat as open PRs (thisisgm#86 to thisisgm#96). Seven files conflicted and all seven are resolved toward upstream's newer shape: - Cargo.toml, Cargo.lock and PKGBUILD take 0.2.1, so the fork carries upstream's version and not the fork-local 0.1.7. - renamecompat.rs is taken wholesale: upstream fixed the cross-device undo itself (thisisgm#86), inline in rename_path rather than in the fallback predicate, so that fix is superseded and its duplicate const would no longer compile. The tests/ops.sh scenario stays, because it drives the real reversal across tmpfs and a disk rather than the predicate. - trash.rs folds the already-gone partition into upstream's new trash_checked, keeping its validate_sources calls (thisisgm#88). - Ops.js startRename takes upstream's every-view signature and guards the row it was actually asked for; Filter.cursorShown generalises to rowShown(pane, row) so an explicit menu row is checked too (thisisgm#92). - Tabs.js keeps the search re-list and the listRequests selection guard on top of upstream's Startup.newTabPath destination (thisisgm#91, thisisgm#93, thisisgm#94). LR-T: ./tests/js.sh (3,106 checks, 0 failed) LR-D: the fork was two releases behind while its ten fixes were still unmerged upstream LR-K: src/backend/trash.rs; ui/js/Tabs.js; ui/js/Ops.js; ui/js/Filter.js; src/backend/renamecompat.rs Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AnB5YmkdfX8Yri7gSGuAWf

thisisgm · 2026-09-19T21:23:11Z

Superseded by 5c3b594 (v0.2.0): undo already uses the exclusive-copy fallback for cross-device renames; this PR was not merged.

thisisgm closed this Sep 19, 2026

Navigation Menu

fix: undo brings a move back across filesystems - #86

fix: undo brings a move back across filesystems#86
nixfred wants to merge 1 commit into
thisisgm:mainfrom
nixfred:fix/undo-cross-device-move

nixfred commented Sep 8, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Sep 8, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

thisisgm commented Sep 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

nixfred commented Sep 8, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The bug

The fix

Tests

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

thisisgm commented Sep 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nixfred commented Sep 8, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 8, 2026 •

edited

Loading