Skip to content

Commit 0fbafeb

Browse files
committed
Bugfix: Volume copy dialog hung after cancel
- Cancelling a multi-file volume copy mid-stream (SMB, MTP) left the Copy dialog open until app restart. A per-task `VolumeError::Cancelled` lands in `copy_error` mapped to `WriteOperationError::Cancelled`, so the post-loop's `if copy_error.is_none() { emit_cancelled }` gate skipped the terminal event. The outer `copy_between_volumes` wrapper then assumed the inner had already emitted, so the FE saw no `write-cancelled` and the dialog wedged. - The post-loop now reclassifies a Cancelled-shaped `copy_error` as cancellation (`copy_error = None`) when `is_cancelled(&state.intent)` is true, restoring the `emit_cancelled` path. The synthetic `Err(Cancelled)` return still propagates so the outer wrapper continues to skip `write-error`. - Pinned with a Gotcha in `write_operations/CLAUDE.md`.
1 parent 38ebdc8 commit 0fbafeb

2 files changed

Lines changed: 19 additions & 0 deletions

File tree

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,9 @@ and `statfs.f_fstypename` for APFS. See `copy_strategy.rs` for the implementatio
318318
**Gotcha**: Recursive scan helpers that bail with `Err(Cancelled)` must NOT emit `write-cancelled` themselves; their top-level callers must.
319319
**Why**: `delete::scan_volume_recursive` checks `is_cancelled(&state.intent)` at the top of every recursion level. If it emitted `write-cancelled` at the bail site, a mid-walk cancel would fire the terminal event once per recursion frame still on the stack. So the function returns `Err(Cancelled)` silently and the caller is responsible for emitting before propagating. `delete_volume_files_with_progress_inner` uses the `emit_cancelled_if_aborted` helper at each of its three `scan_volume_recursive(...).await?` sites for exactly this. Any future recursive scan that follows the same shape (per-level cancel check) needs the same caller-side emit, otherwise the FE never sees `write-cancelled` for scan-phase cancels and the dialog closes via the M4 settle-fallback path instead of the proper cancel flow. Pinned by `delete_cancel_during_scan_emits_write_cancelled` in `delete_volume_reuse_tests.rs`.
320320

321+
**Gotcha**: In `copy_volumes_with_progress`, a per-task `VolumeError::Cancelled` populates `copy_error` and would otherwise skip the `write-cancelled` emit.
322+
**Why**: Both the concurrent path (`Some(Err((failed_dest, e)))` arm) and the serial path (`PostLoopIntent::Failed`) feed any per-task error into `copy_error` via `map_volume_error`. `VolumeError::Cancelled` → `WriteOperationError::Cancelled`, so a mid-flight cancel that surfaces through a streaming reader lands in `copy_error` as a Cancelled-shaped `WriteFailure`. The post-loop gate `if copy_error.is_none() { emit_cancelled }` then silently swallows it, the outer `copy_between_volumes` wrapper's `matches!(Cancelled)` arm assumes the inner already emitted, and the FE never sees a terminal event — Copy dialog hangs until restart. The post-loop reclassifies a Cancelled-shaped `copy_error` as cancellation (`copy_error = None`) whenever `is_cancelled(&state.intent)` is true, so the emit gate fires; the synthetic `Err(Cancelled)` at the bottom of the function still propagates so the outer wrapper continues to skip `write-error`. Repro: copy 13 SMB files concurrent path, cancel mid-stream → no `write-cancelled` on the wire pre-fix.
323+
321324
**Gotcha**: Skip-All on volume copy/move with a top-level dir conflict still skips the entire dir subtree, even after the local-FS bulk-skip fix.
322325
**Why**: `build_pre_skip_set` now excludes top-level directories so non-conflicting children inside a conflicting dir get a chance to copy. For LOCAL copy this works because `copy_files_with_progress_inner` flattens dirs to per-file `FileInfo` entries pre-loop, and per-iter conflict resolution then evaluates each child individually. For VOLUME copy/move, the driver iterates top-level paths directly, and `resolve_volume_conflict` returns `Ok(None)` (= Skip) for ANY dir-vs-dir conflict under Skip mode without recursing — so the whole subtree is still dropped. Fixing this requires teaching `resolve_volume_conflict` (or the volume-side closure) to recurse-and-merge for dir conflicts under Skip, the same way `apply_volume_conflict_resolution` already does for Overwrite. Pinned by the Playwright spec `conflict-copy.spec.ts::Copy with Skip All preserves destination files` (local-FS path, currently green) — a volume-side equivalent test would catch the residual.
323326

apps/desktop/src-tauri/src/file_system/write_operations/volume_copy.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1143,6 +1143,22 @@ pub(crate) async fn copy_volumes_with_progress(
11431143
// Post-loop: handle success, cancellation, or error
11441144
let intent = load_intent(&state.intent);
11451145

1146+
// A `VolumeError::Cancelled` from a per-task stream (concurrent path's
1147+
// `Err((dest, e))` arm, or the serial driver's `PostLoopIntent::Failed`
1148+
// arm) maps to `WriteOperationError::Cancelled` and ends up here in
1149+
// `copy_error`. That's not a transport failure: it's the cooperative
1150+
// response to the user's cancel click. Reclassify it as cancellation so
1151+
// the gate below emits `write-cancelled` instead of dropping the terminal
1152+
// event entirely and wedging the FE dialog.
1153+
if is_cancelled(&state.intent)
1154+
&& matches!(
1155+
copy_error.as_ref().map(|f| &f.error),
1156+
Some(WriteOperationError::Cancelled { .. }),
1157+
)
1158+
{
1159+
copy_error = None;
1160+
}
1161+
11461162
if copy_error.is_none() && !is_cancelled(&state.intent) {
11471163
// All files copied successfully
11481164
log::info!(

0 commit comments

Comments
 (0)