Skip to content

Commit e7f657d

Browse files
committed
Bugfix: Per-iter Skip on volume copy now credits byte progress
`ConflictDecision::Skip` carried no payload, so the async driver's per-iter Skip arm bumped `files_done` by 1 but `bytes_done` stayed at 0. Visible as: file counter moved forward while the size bar froze at 0% for the whole operation when conflicts resolved via `apply_to_all` (the mid-operation dialog choice) rather than the pre-flight `preKnownConflicts` bulk-skip. - `ConflictDecision::Skip { bytes_accounted: u64 }` carries the source's byte size; driver's Skip arm credits both axes. - `volume_copy`'s resolver looks up `source_hints` and returns the file's size (0 for dirs by convention — recursive total isn't tracked there, that's the separate residual gotcha in CLAUDE.md). - `volume_move`'s two resolvers pass 0 (move has no pre-flight byte scan; consistent with the move bulk-skip prelude that always credits 0 bytes). - Driver test `async_driver_progress_accounting_sums_correctly` previously locked in the buggy "conflict-skip has no bytes" expectation; updated to assert the correct 350 = 200 bulk-skip + 50 per-iter-skip + 100 transferred. - New Playwright spec `MTP-to-local copy with apply-to-all Skip credits byte progress` pins the FE-visible behavior: subscribe to `write-conflict` + `write-complete`, default conflict policy (`stop`), answer the first conflict via `resolve_write_conflict(opId, Skip, applyToAll=true)`, then assert `WriteCompleteEvent.bytes_processed === sum(source file sizes)`.
1 parent 71bbca6 commit e7f657d

5 files changed

Lines changed: 158 additions & 14 deletions

File tree

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -508,9 +508,14 @@ pub(super) struct ConflictDecisionInput<'a> {
508508
/// Conflict-resolution outcome the resolver hands back to the driver.
509509
#[derive(Debug)]
510510
pub(super) enum ConflictDecision {
511-
/// Skip this source. Driver bumps counters via skip accounting and
512-
/// continues. The `transfer_one` closure is NOT invoked.
513-
Skip,
511+
/// Skip this source. Driver bumps counters via skip accounting (both
512+
/// `files_done += 1` and `bytes_done += bytes_accounted`) and continues.
513+
/// The `transfer_one` closure is NOT invoked. `bytes_accounted` is the
514+
/// source's byte size from the caller's pre-flight scan (volume copy
515+
/// looks it up in `source_hints`; volume move has no scan and passes 0).
516+
/// Without it, the size progress bar would stay at 0 % while the file
517+
/// counter moved forward on Skip-All operations.
518+
Skip { bytes_accounted: u64 },
514519
/// Proceed with the given (possibly rewritten) destination path. Driver
515520
/// calls `transfer_one` with `dest_path = Some(this)`.
516521
Proceed { dest_path: PathBuf },
@@ -671,11 +676,12 @@ where
671676
.await;
672677

673678
match decision {
674-
Ok(ConflictDecision::Skip) => {
679+
Ok(ConflictDecision::Skip { bytes_accounted }) => {
675680
// Per-iter skip accounting: bump counters and emit
676681
// throttled progress so the bar reflects the skip
677682
// immediately.
678683
files_done += 1;
684+
bytes_done += bytes_accounted;
679685
if last_progress_time.elapsed() >= progress_interval {
680686
last_progress_time = Instant::now();
681687
emit_progress_and_status(

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

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -699,7 +699,7 @@ async fn async_driver_does_not_invoke_closure_when_conflict_resolved_as_skip() {
699699
assert_eq!(input.source_path, Path::new("/a.txt"));
700700
assert_eq!(input.initial_dest_path, Path::new("/dest/a.txt"));
701701
assert_eq!(input.dest_size_hint, Some(50));
702-
Ok(ConflictDecision::Skip)
702+
Ok(ConflictDecision::Skip { bytes_accounted: 0 })
703703
})
704704
},
705705
|ctx: TransferContext<'_>| -> TransferFut<'_> {
@@ -1049,9 +1049,9 @@ async fn async_driver_apply_to_all_resolver_decision_persists_across_sources() {
10491049
// Closure latches Skip on first call; from then on, returns Skip.
10501050
let mut guard = latched.lock().unwrap();
10511051
if guard.is_none() {
1052-
*guard = Some(ConflictDecision::Skip);
1052+
*guard = Some(ConflictDecision::Skip { bytes_accounted: 0 });
10531053
}
1054-
Ok(ConflictDecision::Skip)
1054+
Ok(ConflictDecision::Skip { bytes_accounted: 0 })
10551055
})
10561056
},
10571057
|ctx: TransferContext<'_>| -> TransferFut<'_> {
@@ -1105,7 +1105,9 @@ async fn async_driver_progress_accounting_sums_correctly() {
11051105
let conflict = p == Path::new("/dest/conflict");
11061106
Box::pin(async move { if conflict { Some(50) } else { None } })
11071107
},
1108-
|_i: ConflictDecisionInput<'_>| -> ResolveFut<'_> { Box::pin(async { Ok(ConflictDecision::Skip) }) },
1108+
|_i: ConflictDecisionInput<'_>| -> ResolveFut<'_> {
1109+
Box::pin(async { Ok(ConflictDecision::Skip { bytes_accounted: 50 }) })
1110+
},
11091111
|_ctx: TransferContext<'_>| -> TransferFut<'_> {
11101112
Box::pin(async { Ok(TransferOutcome::Transferred { bytes: 100 }) })
11111113
},
@@ -1115,8 +1117,8 @@ async fn async_driver_progress_accounting_sums_correctly() {
11151117
assert!(matches!(outcome.intent, PostLoopIntent::Completed));
11161118
assert_eq!(outcome.files_done, 4, "2 bulk + 1 conflict-skip + 1 transferred");
11171119
assert_eq!(
1118-
outcome.bytes_done, 300,
1119-
"200 bulk-skipped + 0 (conflict-skip has no bytes) + 100 transferred"
1120+
outcome.bytes_done, 350,
1121+
"200 bulk-skipped + 50 (conflict-skip's `bytes_accounted`) + 100 transferred"
11201122
);
11211123
uninstall_state(&op_id);
11221124
unregister_operation_status(&op_id);
@@ -1143,7 +1145,9 @@ async fn async_driver_status_cache_matches_emitted_progress() {
11431145
&HashSet::new(),
11441146
&copy_config(),
11451147
|_p: &Path| -> FetchFut<'_> { Box::pin(async { Some(50) }) }, // conflict
1146-
|_i: ConflictDecisionInput<'_>| -> ResolveFut<'_> { Box::pin(async { Ok(ConflictDecision::Skip) }) },
1148+
|_i: ConflictDecisionInput<'_>| -> ResolveFut<'_> {
1149+
Box::pin(async { Ok(ConflictDecision::Skip { bytes_accounted: 0 }) })
1150+
},
11471151
|_ctx: TransferContext<'_>| -> TransferFut<'_> {
11481152
Box::pin(async { panic!("closure should not fire under Skip") })
11491153
},

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1011,7 +1011,14 @@ pub(crate) async fn copy_volumes_with_progress(
10111011
"copy_volumes_with_progress: skipping {} due to conflict resolution",
10121012
source_path_owned.display()
10131013
);
1014-
ConflictDecision::Skip
1014+
// Credit the source's byte size so the size
1015+
// progress bar matches the file counter when
1016+
// every source is skipped. Dirs report 0 in
1017+
// `source_hints` by convention (the recursive
1018+
// total isn't tracked there); per-file skips
1019+
// credit the real size.
1020+
let bytes_accounted = source_hint.map(|h| h.size).unwrap_or(0);
1021+
ConflictDecision::Skip { bytes_accounted }
10151022
}
10161023
Some(dest_path) => ConflictDecision::Proceed { dest_path },
10171024
})

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,10 @@ pub(super) async fn move_volumes_with_progress(
354354
"move_between_volumes: skipping {} due to conflict resolution",
355355
source_path_owned.display()
356356
);
357-
ConflictDecision::Skip
357+
// Move has no pre-flight byte scan, so skip-bytes
358+
// stays at 0 (consistent with the move bulk-skip
359+
// prelude that always credits 0 bytes).
360+
ConflictDecision::Skip { bytes_accounted: 0 }
358361
}
359362
Some(dest_path) => ConflictDecision::Proceed { dest_path },
360363
})
@@ -747,7 +750,9 @@ pub(super) async fn move_within_same_volume_with_progress(
747750
"move_within_same_volume: skipping {} due to conflict resolution",
748751
source_path_owned.display()
749752
);
750-
ConflictDecision::Skip
753+
// Move has no pre-flight byte scan, so skip-bytes
754+
// stays at 0.
755+
ConflictDecision::Skip { bytes_accounted: 0 }
751756
}
752757
Some(dest_path) => ConflictDecision::Proceed { dest_path },
753758
})

apps/desktop/test/e2e-playwright/mtp-conflicts.spec.ts

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,3 +339,125 @@ test.describe('MTP same-volume move conflicts', () => {
339339
expect(fs.existsSync(path.join(MTP_FIXTURE_ROOT, 'internal', 'Documents', 'report.txt'))).toBe(true)
340340
})
341341
})
342+
343+
// ── Cross-volume COPY conflicts (MTP -> local) ─────────────────────────────
344+
345+
test.describe('MTP cross-volume copy conflicts', () => {
346+
test('MTP-to-local copy with apply-to-all Skip credits byte progress', async ({ tauriPage }) => {
347+
// Regression test for: per-iter Skip in the async transfer driver bumps
348+
// `filesDone` but not `bytesDone`. User-visible symptom: the file counter
349+
// moves forward while the size counter stays at 0 % on Skip-All copies
350+
// where conflicts resolve via `apply_to_all` (the mid-operation conflict
351+
// dialog), not via the FE's pre-flight `preKnownConflicts` bulk-skip.
352+
//
353+
// Setup: MTP `Documents/` has `report.txt` + `notes.txt`. Pre-populate
354+
// matching files at local `right/` so both source files conflict at dest.
355+
// Trigger MTP -> local copy with the DEFAULT (`stop`) conflict policy so
356+
// `preKnownConflicts` stays empty. Intercept the first `write-conflict`
357+
// event and resolve it via `resolve_write_conflict(opId, Skip, true)` so
358+
// `apply_to_all` latches and subsequent conflicts auto-resolve via the
359+
// driver's per-iter Skip arm. Assert the final `write-progress` event has
360+
// both axes landed at total.
361+
const fixtureRoot = getFixtureRoot()
362+
fs.writeFileSync(path.join(fixtureRoot, 'right', 'report.txt'), 'local-report')
363+
fs.writeFileSync(path.join(fixtureRoot, 'right', 'notes.txt'), 'local-notes')
364+
365+
await ensureAppReady(tauriPage)
366+
await mcpSelectVolume('left', INTERNAL_STORAGE)
367+
const mtpPath = await getMtpVolumePath(INTERNAL_STORAGE)
368+
await mcpNavToPath('left', `${mtpPath}/Documents`)
369+
await mcpAwaitItem('left', 'report.txt')
370+
await mcpAwaitItem('left', 'notes.txt')
371+
372+
// Subscribe to write-conflict (for the apply-to-all answer) and
373+
// write-complete (carries the authoritative `bytes_processed` /
374+
// `files_processed`) BEFORE triggering the copy. write-progress is
375+
// throttled to 200 ms and 2 fast in-memory Skips don't reliably emit a
376+
// post-iter event — write-complete is the source of truth.
377+
await tauriPage.evaluate(`(async function() {
378+
window.__skipBytesTestConflicts = [];
379+
window.__skipBytesTestComplete = null;
380+
const conflictHandler = (event) => { window.__skipBytesTestConflicts.push(event.payload); };
381+
const completeHandler = (event) => { window.__skipBytesTestComplete = event.payload; };
382+
const conflictHandlerId = window.__TAURI_INTERNALS__.transformCallback(conflictHandler);
383+
const completeHandlerId = window.__TAURI_INTERNALS__.transformCallback(completeHandler);
384+
window.__skipBytesTestConflictId = await window.__TAURI_INTERNALS__.invoke('plugin:event|listen', {
385+
event: 'write-conflict',
386+
target: { kind: 'Any' },
387+
handler: conflictHandlerId,
388+
});
389+
window.__skipBytesTestCompleteId = await window.__TAURI_INTERNALS__.invoke('plugin:event|listen', {
390+
event: 'write-complete',
391+
target: { kind: 'Any' },
392+
handler: completeHandlerId,
393+
});
394+
})()`)
395+
396+
try {
397+
// Select both files in the MTP pane.
398+
await tauriPage.keyboard.press('Meta+A')
399+
await dispatchMenuCommand(tauriPage, 'file.copy')
400+
401+
await tauriPage.waitForSelector(TRANSFER_DIALOG, 10000)
402+
await waitForConflictPolicy(tauriPage)
403+
// Leave the dropdown at its default ('stop') so preKnownConflicts stays
404+
// empty and conflicts surface per-iter via write-conflict.
405+
await clickTransferStart(tauriPage)
406+
407+
// Wait for the first write-conflict event, then resolve via IPC.
408+
await pollUntil(
409+
tauriPage,
410+
async () => tauriPage.evaluate<boolean>(`(window.__skipBytesTestConflicts ?? []).length > 0`),
411+
10000,
412+
50,
413+
)
414+
const firstConflict = await tauriPage.evaluate<{ operationId: string }>(`window.__skipBytesTestConflicts[0]`)
415+
expect(firstConflict.operationId).toBeTruthy()
416+
await tauriPage.evaluate(`window.__TAURI_INTERNALS__.invoke('resolve_write_conflict', {
417+
operationId: '${firstConflict.operationId}',
418+
resolution: 'skip',
419+
applyToAll: true,
420+
})`)
421+
422+
await waitForDialogsToClose(tauriPage, 30000)
423+
424+
// Dest content unchanged (both files were skipped).
425+
expect(fs.readFileSync(path.join(fixtureRoot, 'right', 'report.txt'), 'utf-8')).toBe('local-report')
426+
expect(fs.readFileSync(path.join(fixtureRoot, 'right', 'notes.txt'), 'utf-8')).toBe('local-notes')
427+
428+
// Source files still on MTP (skip is non-destructive for copy).
429+
expect(fs.existsSync(path.join(MTP_FIXTURE_ROOT, 'internal', 'Documents', 'report.txt'))).toBe(true)
430+
expect(fs.existsSync(path.join(MTP_FIXTURE_ROOT, 'internal', 'Documents', 'notes.txt'))).toBe(true)
431+
432+
// write-complete carries the authoritative final tallies. With the bug,
433+
// every per-iter Skip bumps `filesProcessed` by 1 but `bytesProcessed`
434+
// stays at 0, so the dialog's size bar reads 0 % at the end.
435+
const completed = await tauriPage.evaluate<{
436+
filesProcessed: number
437+
bytesProcessed: number
438+
} | null>(`window.__skipBytesTestComplete`)
439+
const expectedBytes =
440+
fs.statSync(path.join(MTP_FIXTURE_ROOT, 'internal', 'Documents', 'report.txt')).size +
441+
fs.statSync(path.join(MTP_FIXTURE_ROOT, 'internal', 'Documents', 'notes.txt')).size
442+
443+
expect(completed).not.toBeNull()
444+
expect(completed!.filesProcessed).toBe(2)
445+
expect(completed!.bytesProcessed).toBe(expectedBytes)
446+
} finally {
447+
await tauriPage.evaluate(`(async function() {
448+
const conflictId = window.__skipBytesTestConflictId;
449+
const completeId = window.__skipBytesTestCompleteId;
450+
if (conflictId !== undefined) {
451+
await window.__TAURI_INTERNALS__.invoke('plugin:event|unlisten', { event: 'write-conflict', eventId: conflictId });
452+
}
453+
if (completeId !== undefined) {
454+
await window.__TAURI_INTERNALS__.invoke('plugin:event|unlisten', { event: 'write-complete', eventId: completeId });
455+
}
456+
delete window.__skipBytesTestConflicts;
457+
delete window.__skipBytesTestComplete;
458+
delete window.__skipBytesTestConflictId;
459+
delete window.__skipBytesTestCompleteId;
460+
})()`)
461+
}
462+
})
463+
})

0 commit comments

Comments
 (0)