Skip to content

Commit 0ac5d05

Browse files
vdavidvdavid
committed
Copy: Visible rollback with progress bars
- Replaced `cancelled: AtomicBool` + `skip_rollback: AtomicBool` with `OperationIntent` enum (`Running` → `RollingBack` → `Stopped`) backed by `AtomicU8` - `cancel_write_operation` does validated state transitions; `cancel_all_write_operations` transitions to `Stopped` (no silent background rollback on teardown) - Rollback runs synchronously on the same `spawn_blocking` thread with progress events — no more fire-and-forget `rollback_in_background` - Backend emits decreasing `files_done`/`bytes_done` during rollback so the frontend's progress bars count backwards from the cancellation point - Frontend reuses the same progress view for rollback — no separate UI, no flicker. Rollback button shows "Rolling back..." disabled, Cancel stays active to stop the rollback - Removed `rollback_in_background()` from `CopyTransaction` - Added `RollingBack` variant to `WriteOperationPhase` (backend + frontend) - Updated ~50 cancellation check sites across 14 files to use `is_cancelled(&state.intent)`
1 parent f8dfefb commit 0ac5d05

20 files changed

Lines changed: 458 additions & 278 deletions

File tree

‎apps/desktop/src-tauri/src/file_system/write_operations/CLAUDE.md‎

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ Frontend
4343
→ execute phase: per-file copy/delete
4444
→ throttled write-progress events (200ms default)
4545
→ success: CopyTransaction::commit(), emit write-complete
46-
→ cancel: CopyTransaction::rollback_in_background(), emit write-cancelled
46+
→ cancel (Stopped): CopyTransaction::commit(), emit write-cancelled (rolled_back: false)
47+
→ cancel (RollingBack): rollback_with_progress() → emit write-progress (phase: rolling_back) → emit write-cancelled
4748
→ error: CopyTransaction::rollback(), emit write-error
4849
→ safety net: start_write_operation emits write-error for unhandled handler errors
4950
→ state removed from both caches
@@ -53,14 +54,18 @@ Frontend
5354

5455
**All blocking work in `spawn_blocking`.** Never call blocking I/O on the async executor.
5556

56-
**Two-layer cancellation.** `AtomicBool` for fast in-loop checks. `run_cancellable` wraps blocking operations (e.g.,
57+
**`OperationIntent` state machine.** Replaces the old `cancelled: AtomicBool` + `skip_rollback: AtomicBool` pair with a
58+
single `AtomicU8`-backed enum: `Running → RollingBack` (user clicks Rollback), `Running → Stopped` (user clicks Cancel
59+
or teardown), `RollingBack → Stopped` (user cancels the rollback). `Stopped` is terminal. The `is_cancelled()` helper
60+
returns true for both `RollingBack` and `Stopped`, so the 40+ cancellation check sites just call `is_cancelled(&state.intent)`.
61+
62+
**Two-layer cancellation.** `AtomicU8` for fast in-loop checks. `run_cancellable` wraps blocking operations (e.g.,
5763
network-mount copies that may block indefinitely) in a separate thread, polling the flag every 100ms via `mpsc::channel`.
5864

59-
**`CopyTransaction` rollback: sync vs async.** Two rollback paths: `rollback()` (synchronous, for error paths where
60-
cleanup must complete before the error event) and `rollback_in_background()` (fire-and-forget on a detached thread, for
61-
user-initiated cancel where the UI must respond instantly). `rollback_in_background` sets `committed = true` to prevent
62-
`Drop` from triggering a synchronous double-rollback, then moves the file/dir lists into the detached thread. The
63-
synchronous `rollback()` and auto-rollback-on-panic via `Drop` remain unchanged. Delete operations are not rollbackable.
65+
**`CopyTransaction` rollback: sync with progress.** `rollback()` (synchronous, for error paths) and tracked
66+
`rollback_with_progress()` in `copy.rs` (for user-initiated rollback — emits `write-progress` events with
67+
`phase: RollingBack`, checks for `Stopped` between file deletions so the user can cancel the rollback). Auto-rollback
68+
via `Drop` remains as a panic safety net. Delete operations are not rollbackable.
6469

6570
**Symlinks never dereferenced.** All stat calls use `symlink_metadata`. Symlink loop detection uses a `HashSet<PathBuf>`
6671
of canonicalized paths.
@@ -72,7 +77,10 @@ rename temp → dest, delete backup. The original is intact until step 3 complet
7277
Frontend calls `resolve_write_conflict(operation_id, resolution, apply_to_all)` which stores a `ConflictResolutionResponse`
7378
and notifies the condvar. `cancel_write_operation` also notifies the condvar to unblock.
7479

75-
**`skip_rollback` is stored inverted.** `cancel_write_operation(rollback: bool)` stores `!rollback` in `skip_rollback`.
80+
**`cancel_write_operation` does state transitions.** `rollback=true` → `Running → RollingBack`, `rollback=false` →
81+
`Running → Stopped` or `RollingBack → Stopped`. First caller's decision wins — subsequent calls with different intent
82+
are no-ops (unless transitioning from `RollingBack → Stopped`). `cancel_all_write_operations` always transitions to
83+
`Stopped` (teardown should never silently roll back without visual feedback).
7684

7785
**Scan preview caching.** `start_scan_preview` runs a background scan, caches the result in `SCAN_PREVIEW_RESULTS`. The
7886
actual `copy_files_start` can consume the cache via `preview_id` in `WriteOperationConfig`, skipping a redundant scan.
@@ -121,9 +129,9 @@ the progress dialog and offer cancel, even if a network mount is stalled.
121129
emits `write-error` as a safety net, and `Err(join_error)` handles panics. Double-emit is harmless because the
122130
frontend's `handleError` removes all listeners on first receipt.
123131

124-
**Background cleanup is best-effort.** `rollback_in_background`, `remove_file_in_background`, and
125-
`remove_dir_all_in_background` run on detached threads. If the network mount disconnects or the app exits, partial
126-
files or staging directories may remain on disk. These use the `.cmdr-` prefix, so they're recognizable.
132+
**Background cleanup is best-effort.** `remove_file_in_background` and `remove_dir_all_in_background` run on detached
133+
threads (used for temp/backup file cleanup, not for user-visible rollback). If the network mount disconnects or the app
134+
exits, partial files or staging directories may remain on disk. These use the `.cmdr-` prefix, so they're recognizable.
127135

128136
**`volume_copy` path is incomplete.** The three `volume_*` files are Phase 5 work, but are publicly re-exported from `mod.rs` and at least partially wired up.
129137

‎apps/desktop/src-tauri/src/file_system/write_operations/chunked_copy.rs‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@
88
use std::io::{Read, Write};
99
use std::path::Path;
1010
use std::sync::Arc;
11-
use std::sync::atomic::{AtomicBool, Ordering};
11+
use std::sync::atomic::AtomicU8;
12+
#[cfg(test)]
13+
use std::sync::atomic::Ordering;
1214

1315
use super::types::WriteOperationError;
1416

@@ -59,7 +61,7 @@ pub fn is_network_filesystem(path: &Path) -> bool {
5961
pub fn chunked_copy_with_metadata(
6062
source: &Path,
6163
dest: &Path,
62-
cancelled: &Arc<AtomicBool>,
64+
cancelled: &Arc<AtomicU8>,
6365
progress_callback: Option<ChunkedCopyProgressFn>,
6466
) -> Result<u64, WriteOperationError> {
6567
log::info!(
@@ -98,7 +100,7 @@ pub fn chunked_copy_with_metadata(
98100
fn copy_data_chunked(
99101
source: &Path,
100102
dest: &Path,
101-
cancelled: &Arc<AtomicBool>,
103+
cancelled: &Arc<AtomicU8>,
102104
source_size: u64,
103105
progress_callback: Option<ChunkedCopyProgressFn>,
104106
) -> Result<u64, WriteOperationError> {
@@ -117,7 +119,7 @@ fn copy_data_chunked(
117119

118120
loop {
119121
// Check cancellation BEFORE each read
120-
if cancelled.load(Ordering::Relaxed) {
122+
if super::state::is_cancelled(cancelled) {
121123
log::info!(
122124
"chunked_copy: cancellation detected after {} bytes, cleaning up",
123125
total_bytes
@@ -312,7 +314,7 @@ mod tests {
312314

313315
fs::write(&src, "Hello, chunked copy!").unwrap();
314316

315-
let cancelled = Arc::new(AtomicBool::new(false));
317+
let cancelled = Arc::new(AtomicU8::new(0));
316318
let result = chunked_copy_with_metadata(&src, &dst, &cancelled, None);
317319

318320
assert!(result.is_ok());
@@ -334,7 +336,7 @@ mod tests {
334336
fs::write(&src, &large_content).unwrap();
335337

336338
// Pre-cancelled
337-
let cancelled = Arc::new(AtomicBool::new(true));
339+
let cancelled = Arc::new(AtomicU8::new(2));
338340
let result = chunked_copy_with_metadata(&src, &dst, &cancelled, None);
339341

340342
assert!(matches!(result, Err(WriteOperationError::Cancelled { .. })));
@@ -355,7 +357,7 @@ mod tests {
355357
fs::write(&src, "#!/bin/bash").unwrap();
356358
fs::set_permissions(&src, fs::Permissions::from_mode(0o755)).unwrap();
357359

358-
let cancelled = Arc::new(AtomicBool::new(false));
360+
let cancelled = Arc::new(AtomicU8::new(0));
359361
let result = chunked_copy_with_metadata(&src, &dst, &cancelled, None);
360362

361363
assert!(result.is_ok());
@@ -373,7 +375,7 @@ mod tests {
373375

374376
fs::write(&src, "").unwrap();
375377

376-
let cancelled = Arc::new(AtomicBool::new(false));
378+
let cancelled = Arc::new(AtomicU8::new(0));
377379
let result = chunked_copy_with_metadata(&src, &dst, &cancelled, None);
378380

379381
assert!(result.is_ok());
@@ -397,7 +399,7 @@ mod tests {
397399
let expected_size = large_content.len() as u64;
398400
fs::write(&src, &large_content).unwrap();
399401

400-
let cancelled = Arc::new(AtomicBool::new(false));
402+
let cancelled = Arc::new(AtomicU8::new(0));
401403
let callback_count = Arc::new(AtomicU64::new(0));
402404
let last_bytes = Arc::new(AtomicU64::new(0));
403405

0 commit comments

Comments
 (0)