Skip to content

Commit 7ecf9d3

Browse files
vdavidvdavid
committed
Fix: route dir-into-dir cross-volume conflicts through resolve_volume_conflict
User report: after moving folder A → NAS, then copying it back, then moving back to NAS again — the third move silently merged into the existing folder instead of prompting "Overwrite/Skip/Rename" like every other conflict path. Root cause: `move_between_volumes`, `move_within_same_volume`, and the two loops in `copy_volumes_with_progress` all had a special-case `if source_is_dir && dest_is_dir { merge silently }` that bypassed `resolve_volume_conflict`. The pre-flight FE conflict scan ("Found 1 conflicts at destination") had no effect for dir-vs-dir because the backend never asked for a resolution. Removed all four bypasses. Every conflict now routes through the resolver, respecting `config.conflict_resolution`: - Stop → emits `write-conflict`, awaits user pick (the case the user wanted) - Skip → skips the whole tree (continue) - Overwrite → tries to delete dest first; for non-empty dirs the delete fails benignly (Volume::delete contract) and the recursive copy proceeds, so same-named files get overwritten and the rest of the dest tree stays. Effectively a "merge with overwrite-on-file-conflict" — same end-state as the previous silent merge, but only when the user picks it. - Rename → the resolver finds a unique name, source lands cleanly under it FE timing fix in `TransferDialog.svelte`: `handleConfirm` now awaits the in-flight conflict scan before passing `conflictPolicy` out. Prevents a fast Enter from committing with `conflicts: []` even when a check was about to find one. The captured promise is stored in `conflictCheckPromise` $state for visibility. Out of scope: the `get_file_at: index out of bounds — frontend/backend index mismatch` line in user's logs is pre-existing diagnostic (commit 14ebdc1, 2026-04-26) — stale FE selection after move, not caused by this branch. Logged as a separate follow-up. Tests: 149 write_operations tests pass. Full check suite next.
1 parent 64eb64d commit 7ecf9d3

3 files changed

Lines changed: 149 additions & 171 deletions

File tree

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

Lines changed: 57 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -517,49 +517,36 @@ pub(crate) async fn copy_volumes_with_progress(
517517
dest_path.to_path_buf()
518518
};
519519
if let Ok(dest_meta) = dest_volume.get_metadata(&dest_item_path).await {
520-
// Reuse the per-source hint from the scan instead of
521-
// re-statting. If the hint is missing (e.g., cached preview
522-
// branch that didn't populate it), default to false —
523-
// behaves as if the source is a file (worst case: one extra
524-
// conflict prompt vs. silent merge for dir-over-dir).
520+
// Reuse the per-source hint from the scan instead of re-statting.
525521
let source_is_dir = source_hints.get(source_path).map(|h| h.is_directory).unwrap_or(false);
526-
let dest_is_dir = dest_meta.is_directory;
527-
if source_is_dir && dest_is_dir {
528-
log::debug!(
529-
"copy_volumes_with_progress: merging directories {} -> {}",
530-
source_path.display(),
531-
dest_item_path.display()
532-
);
533-
} else {
534-
log::debug!(
535-
"copy_volumes_with_progress: conflict detected at {} (source_is_dir={}, dest_is_dir={})",
536-
dest_item_path.display(),
537-
source_is_dir,
538-
dest_is_dir
539-
);
540-
let resolved = resolve_volume_conflict(
541-
&source_volume,
542-
source_path,
543-
&dest_volume,
544-
&dest_item_path,
545-
config,
546-
events,
547-
operation_id,
548-
state,
549-
&mut apply_to_all_resolution,
550-
)
551-
.await
552-
.map_err(WriteFailure::synthetic)?;
553-
match resolved {
554-
None => {
555-
log::debug!(
556-
"copy_volumes_with_progress: skipping {} due to conflict resolution",
557-
source_path.display()
558-
);
559-
continue;
560-
}
561-
Some(p) => dest_item_path = p,
522+
log::debug!(
523+
"copy_volumes_with_progress: conflict detected at {} (source_is_dir={}, dest_is_dir={})",
524+
dest_item_path.display(),
525+
source_is_dir,
526+
dest_meta.is_directory,
527+
);
528+
let resolved = resolve_volume_conflict(
529+
&source_volume,
530+
source_path,
531+
&dest_volume,
532+
&dest_item_path,
533+
config,
534+
events,
535+
operation_id,
536+
state,
537+
&mut apply_to_all_resolution,
538+
)
539+
.await
540+
.map_err(WriteFailure::synthetic)?;
541+
match resolved {
542+
None => {
543+
log::debug!(
544+
"copy_volumes_with_progress: skipping {} due to conflict resolution",
545+
source_path.display()
546+
);
547+
continue;
562548
}
549+
Some(p) => dest_item_path = p,
563550
}
564551
}
565552

@@ -711,47 +698,36 @@ pub(crate) async fn copy_volumes_with_progress(
711698
};
712699

713700
if let Ok(dest_meta) = dest_volume.get_metadata(&dest_item_path).await {
714-
// Reuse the per-source hint from the scan; see note in the
715-
// concurrent path for the fallback.
716701
let source_is_dir = source_hints.get(source_path).map(|h| h.is_directory).unwrap_or(false);
717-
let dest_is_dir = dest_meta.is_directory;
718-
if source_is_dir && dest_is_dir {
719-
log::debug!(
720-
"copy_volumes_with_progress: merging directories {} -> {}",
721-
source_path.display(),
722-
dest_item_path.display()
723-
);
724-
} else {
725-
log::debug!(
726-
"copy_volumes_with_progress: conflict detected at {} (source_is_dir={}, dest_is_dir={})",
727-
dest_item_path.display(),
728-
source_is_dir,
729-
dest_is_dir
730-
);
731-
let resolved = resolve_volume_conflict(
732-
&source_volume,
733-
source_path,
734-
&dest_volume,
735-
&dest_item_path,
736-
config,
737-
events,
738-
operation_id,
739-
state,
740-
&mut apply_to_all_resolution,
741-
)
742-
.await
743-
.map_err(WriteFailure::synthetic)?;
744-
match resolved {
745-
None => {
746-
log::debug!(
747-
"copy_volumes_with_progress: skipping {} due to conflict resolution",
748-
source_path.display()
749-
);
750-
continue;
751-
}
752-
Some(resolved_path) => {
753-
dest_item_path = resolved_path;
754-
}
702+
log::debug!(
703+
"copy_volumes_with_progress: conflict detected at {} (source_is_dir={}, dest_is_dir={})",
704+
dest_item_path.display(),
705+
source_is_dir,
706+
dest_meta.is_directory,
707+
);
708+
let resolved = resolve_volume_conflict(
709+
&source_volume,
710+
source_path,
711+
&dest_volume,
712+
&dest_item_path,
713+
config,
714+
events,
715+
operation_id,
716+
state,
717+
&mut apply_to_all_resolution,
718+
)
719+
.await
720+
.map_err(WriteFailure::synthetic)?;
721+
match resolved {
722+
None => {
723+
log::debug!(
724+
"copy_volumes_with_progress: skipping {} due to conflict resolution",
725+
source_path.display()
726+
);
727+
continue;
728+
}
729+
Some(resolved_path) => {
730+
dest_item_path = resolved_path;
755731
}
756732
}
757733
}

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

Lines changed: 72 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -128,52 +128,47 @@ pub async fn move_between_volumes(
128128
// re-statted inside `copy_single_path`.
129129
let source_is_dir = source_volume.is_directory(source_path).await.unwrap_or(false);
130130

131-
// Check for conflict: does destination already exist?
131+
// Check for conflict: does destination already exist? Route every conflict
132+
// (file-vs-file, dir-vs-dir, file-vs-dir, dir-vs-file) through
133+
// `resolve_volume_conflict` so the user's chosen `conflict_resolution`
134+
// applies. For dir-vs-dir, picking Overwrite merges (the existing
135+
// contents stay; same-named files inside get overwritten by the recursive
136+
// copy); Skip skips the whole tree; Rename lands the source under a unique
137+
// name; Stop emits a `write-conflict` event and awaits the user's pick.
132138
if let Ok(dest_meta) = dest_volume.get_metadata(&dest_item).await {
133-
let dest_is_dir = dest_meta.is_directory;
134-
135-
if source_is_dir && dest_is_dir {
136-
// Both are directories — merge, not a conflict
137-
log::debug!(
138-
"move_between_volumes: merging directories {} -> {}",
139-
source_path.display(),
140-
dest_item.display()
141-
);
142-
} else {
143-
log::debug!(
144-
"move_between_volumes: conflict detected at {} (source_is_dir={}, dest_is_dir={})",
145-
dest_item.display(),
146-
source_is_dir,
147-
dest_is_dir
148-
);
149-
150-
let events = TauriEventSink::new(app.clone());
151-
let resolved = resolve_volume_conflict(
152-
&source_volume,
153-
source_path,
154-
&dest_volume,
155-
&dest_item,
156-
&config,
157-
&events,
158-
&operation_id_for_spawn,
159-
&state,
160-
&mut apply_to_all_resolution,
161-
)
162-
.await
163-
.map_err(WriteFailure::synthetic)?;
164-
165-
match resolved {
166-
None => {
167-
// Skip — don't copy and don't delete source
168-
log::debug!(
169-
"move_between_volumes: skipping {} due to conflict resolution",
170-
source_path.display()
171-
);
172-
continue;
173-
}
174-
Some(resolved_path) => {
175-
dest_item = resolved_path;
176-
}
139+
log::debug!(
140+
"move_between_volumes: conflict detected at {} (source_is_dir={}, dest_is_dir={})",
141+
dest_item.display(),
142+
source_is_dir,
143+
dest_meta.is_directory,
144+
);
145+
146+
let events = TauriEventSink::new(app.clone());
147+
let resolved = resolve_volume_conflict(
148+
&source_volume,
149+
source_path,
150+
&dest_volume,
151+
&dest_item,
152+
&config,
153+
&events,
154+
&operation_id_for_spawn,
155+
&state,
156+
&mut apply_to_all_resolution,
157+
)
158+
.await
159+
.map_err(WriteFailure::synthetic)?;
160+
161+
match resolved {
162+
None => {
163+
// Skip — don't copy and don't delete source
164+
log::debug!(
165+
"move_between_volumes: skipping {} due to conflict resolution",
166+
source_path.display()
167+
);
168+
continue;
169+
}
170+
Some(resolved_path) => {
171+
dest_item = resolved_path;
177172
}
178173
}
179174
}
@@ -378,49 +373,38 @@ async fn move_within_same_volume(
378373
// Check for conflict: does destination already exist?
379374
if let Ok(dest_meta) = volume.get_metadata(&dest_item).await {
380375
let source_is_dir = volume.is_directory(source_path).await.unwrap_or(false);
381-
let dest_is_dir = dest_meta.is_directory;
382-
383-
if source_is_dir && dest_is_dir {
384-
// Both are directories — merge, not a conflict
385-
log::debug!(
386-
"move_within_same_volume: merging directories {} -> {}",
387-
source_path.display(),
388-
dest_item.display()
389-
);
390-
} else {
391-
log::debug!(
392-
"move_within_same_volume: conflict detected at {} (source_is_dir={}, dest_is_dir={})",
393-
dest_item.display(),
394-
source_is_dir,
395-
dest_is_dir
396-
);
397-
398-
let events = TauriEventSink::new(app.clone());
399-
let resolved = resolve_volume_conflict(
400-
&volume,
401-
source_path,
402-
&volume,
403-
&dest_item,
404-
&config,
405-
&events,
406-
&operation_id_for_spawn,
407-
&state,
408-
&mut apply_to_all_resolution,
409-
)
410-
.await?;
411-
412-
match resolved {
413-
None => {
414-
// Skip — don't move this file
415-
log::debug!(
416-
"move_within_same_volume: skipping {} due to conflict resolution",
417-
source_path.display()
418-
);
419-
continue;
420-
}
421-
Some(resolved_path) => {
422-
dest_item = resolved_path;
423-
}
376+
log::debug!(
377+
"move_within_same_volume: conflict detected at {} (source_is_dir={}, dest_is_dir={})",
378+
dest_item.display(),
379+
source_is_dir,
380+
dest_meta.is_directory,
381+
);
382+
383+
let events = TauriEventSink::new(app.clone());
384+
let resolved = resolve_volume_conflict(
385+
&volume,
386+
source_path,
387+
&volume,
388+
&dest_item,
389+
&config,
390+
&events,
391+
&operation_id_for_spawn,
392+
&state,
393+
&mut apply_to_all_resolution,
394+
)
395+
.await?;
396+
397+
match resolved {
398+
None => {
399+
// Skip — don't move this file
400+
log::debug!(
401+
"move_within_same_volume: skipping {} due to conflict resolution",
402+
source_path.display()
403+
);
404+
continue;
405+
}
406+
Some(resolved_path) => {
407+
dest_item = resolved_path;
424408
}
425409
}
426410
}

‎apps/desktop/src/lib/file-operations/transfer/TransferDialog.svelte‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,16 @@
220220
unlisteners = []
221221
}
222222
223+
/**
224+
* Pending conflict check, captured so `handleConfirm` can await it. Without this,
225+
* a fast confirm (Enter pressed before the check finishes) sends the operation
226+
* with `conflicts: []` even when conflicts exist. The FE never displays the count
227+
* + radio policy section, and the backend can't help if the user picked
228+
* `overwrite_all` blindly. We resolve this by gating Confirm on the check
229+
* completing — see `handleConfirm`.
230+
*/
231+
let conflictCheckPromise: Promise<void> | null = $state(null)
232+
223233
/** Checks for conflicts at the destination. */
224234
async function checkConflicts() {
225235
if (destroyed || isCheckingConflicts || conflictCheckComplete) return
@@ -285,7 +295,7 @@
285295
isScanning = false
286296
scanComplete = true
287297
// After source scan completes, check for conflicts
288-
void checkConflicts()
298+
conflictCheckPromise = checkConflicts()
289299
}),
290300
)
291301
unlisteners.push(
@@ -321,7 +331,7 @@
321331
// when the copy starts and reads the cached scan result).
322332
isScanning = false
323333
scanComplete = true
324-
void checkConflicts()
334+
conflictCheckPromise = checkConflicts()
325335
}
326336
}
327337
}
@@ -363,6 +373,14 @@
363373
// with the IPC and leaves the progress dialog with a null previewId that it
364374
// cannot recover from once scan events have already been emitted.
365375
await scanStarted
376+
// Also wait for the conflict scan if it's still running. Without this, a fast
377+
// confirm sends `conflicts: []` to the backend even when conflicts exist —
378+
// the user never sees the radio policy section, and the operation runs with
379+
// whatever default `conflictPolicy` was set ("stop" by default, so it'd still
380+
// prompt per-file via the backend, but only because of the default).
381+
if (conflictCheckPromise) {
382+
await conflictCheckPromise
383+
}
366384
onConfirm(editedPath, selectedVolumeId, previewId, conflictPolicy, activeOperationType, isScanning)
367385
}
368386

0 commit comments

Comments
 (0)
⚡