Skip to content

Commit fe2a098

Browse files
committed
Eject: block ejecting a volume mid-transfer
- Each copy/move/delete records the volume IDs it touches (source and destination); the backend derives a "busy volumes" set as their union (minus `root`) and pushes membership changes via a `volumes-busy-changed` event. Panic-safe: the existing `OperationStateGuard` clears the mark on unwind. - `eject_volume` refuses a busy volume (the real safety net, since the picker disable is only UX), so a disconnect can't truncate an in-flight file. `get_busy_volume_ids` bootstraps the frontend set. - The picker's header/row eject buttons and right-click item disable with a "Can't eject while operations are in progress on this device" tooltip; the native breadcrumb menu renders the item disabled with a ` (busy)` suffix. - New `volume-busy-store.svelte.ts` (subscribe + bootstrap). Covers MTP/SMB/USB/DMG: a local→USB copy goes through the both-local branch, which now also marks the ejectable destination busy. - Tests: 4 Rust (membership, root-exclusion, overlap-until-last-op, panic-unwind) + 4 frontend store.
1 parent 9f5afdf commit fe2a098

26 files changed

Lines changed: 621 additions & 89 deletions

apps/desktop/src-tauri/src/commands/CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ immediately to business-logic modules. No significant logic lives here.
1414
| `volumes_linux.rs` | Volume management (Linux) | Same interface as `volumes.rs`, delegates to `volumes_linux` module |
1515
| `mtp.rs` | MTP devices | Full MTP command surface (connect, disconnect, list, download, upload, delete, rename, move, scan) |
1616
| `network.rs` | SMB/network shares | Discovery, share listing, keychain, mounting, direct-connection upgrade, in-place reconnect (`reconnect_smb_volume`: backend single-flighted via `Volume::attempt_reconnect`), per-volume disconnect (`disconnect_smb_volume`: macOS shells out to `diskutil unmount`, Linux drops the smb2 session). Lazy-startup hooks: `ensure_network_discovery_started` (idempotent: kicks off mDNS + manual-server load + smb-mount upgrade on first user network action) and `set_network_enabled` (live-applies the `network.enabled` toggle: stops mDNS and clears discovered hosts when off). Upgrade business logic (address resolution, credential lookup, smb2 connection) lives in `network::smb_upgrade`; commands here are thin wrappers. |
17-
| `eject.rs` | Volume eject | `eject_volume(volume_id)`: dispatches by kind. MTP → `mtp::connection_manager().disconnect`; SMB → `diskutil unmount` (FSEvents handles smb2 teardown via `on_unmount`); physical/DMG → `diskutil eject`. Pure `decide_eject_action` (unit-tested) keeps the dispatch logic separate from the impure shell-out. |
17+
| `eject.rs` | Volume eject | `eject_volume(volume_id)`: dispatches by kind. MTP → `mtp::connection_manager().disconnect`; SMB → `diskutil unmount` (FSEvents handles smb2 teardown via `on_unmount`); physical/DMG → `diskutil eject`. Pure `decide_eject_action` (unit-tested) keeps the dispatch logic separate from the impure shell-out. Guards against ejecting a volume with an in-flight write op (`busy_volume_ids().contains(...)` → error) so a transfer can't be truncated. `get_busy_volume_ids()`: bootstrap for the picker's busy set (see `write_operations/CLAUDE.md` § "Busy-volumes set"). |
1818
| `font_metrics.rs` | Font metrics cache | `store_font_metrics`, `has_font_metrics` |
1919
| `icons.rs` | File icons | `get_icons`, `get_custom_folder_icon_ids` (visible-range custom-folder detection), `refresh_directory_icons`, cache clear |
2020
| `rename.rs` | Rename / trash | `move_to_trash` (delegates to `write_operations::trash::move_to_trash_sync`), `check_rename_permission`, `check_rename_validity`, `rename_file`. `rename_file` calls `notify_mutation` after success to update the listing cache (both local and volume-aware paths). |

apps/desktop/src-tauri/src/commands/eject.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,17 @@ pub fn decide_eject_action(ctx: &EjectContext) -> Result<EjectAction, EjectDecis
109109
pub async fn eject_volume(volume_id: String) -> Result<(), IpcError> {
110110
use crate::file_system::get_volume_manager;
111111

112+
// Safety gate: never tear down a volume while a write op is reading from or
113+
// writing to it. The picker disables Eject for busy volumes, so reaching
114+
// here means a race (or an MCP / automation caller); refuse rather than
115+
// disconnect mid-transfer and risk a truncated file. See the volume picker's
116+
// `volumes-busy-changed` wiring.
117+
if crate::file_system::busy_volume_ids().contains(&volume_id) {
118+
return Err(IpcError::from_err(
119+
"operations are in progress on this device. Eject again once they finish",
120+
));
121+
}
122+
112123
// MTP volumes use ID format `{device_id}:{storage_id}` and aren't
113124
// registered in VolumeManager; check the live MTP device list first.
114125
let is_mtp = is_mtp_volume_id(&volume_id).await;
@@ -148,6 +159,16 @@ pub async fn eject_volume(volume_id: String) -> Result<(), IpcError> {
148159
}
149160
}
150161

162+
/// Returns the IDs of volumes that currently have a write op (copy / move /
163+
/// delete) reading from or writing to them. The volume picker bootstraps its
164+
/// busy set from this once on startup, then keeps it live via the
165+
/// `volumes-busy-changed` event. Used to disable Eject for a busy device.
166+
#[tauri::command]
167+
#[specta::specta]
168+
pub fn get_busy_volume_ids() -> Vec<String> {
169+
crate::file_system::busy_volume_ids()
170+
}
171+
151172
/// MTP volume IDs are shaped `{device_id}:{storage_id}` (see
152173
/// `commands/volumes.rs::append_mtp_volumes`). Confirm against the live device
153174
/// list so we don't false-positive on any future ID containing a colon.

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

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,17 @@ pub async fn copy_between_volumes(
3939
let dest_path = PathBuf::from(dest_path);
4040
let config = config.unwrap_or_default();
4141

42-
ops_copy_between_volumes(app, source_volume, source_paths, dest_volume, dest_path, config).await
42+
ops_copy_between_volumes(
43+
app,
44+
source_volume_id,
45+
source_volume,
46+
source_paths,
47+
dest_volume_id,
48+
dest_volume,
49+
dest_path,
50+
config,
51+
)
52+
.await
4353
}
4454

4555
/// Unified move across volume types. Same events as `copy_between_volumes`.
@@ -73,7 +83,17 @@ pub async fn move_between_volumes(
7383
let dest_path = PathBuf::from(dest_path);
7484
let config = config.unwrap_or_default();
7585

76-
ops_move_between_volumes(app, source_volume, source_paths, dest_volume, dest_path, config).await
86+
ops_move_between_volumes(
87+
app,
88+
source_volume_id,
89+
source_volume,
90+
source_paths,
91+
dest_volume_id,
92+
dest_volume,
93+
dest_path,
94+
config,
95+
)
96+
.await
7797
}
7898

7999
/// Pre-flight scan: total count/bytes, available space, conflicts. Doesn't copy anything.

apps/desktop/src-tauri/src/commands/file_system/write_ops.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,10 @@ pub async fn copy_files(
186186
let destination = PathBuf::from(expand_tilde(&destination));
187187
let config = config.unwrap_or_default();
188188

189-
ops_copy_files_start(app, sources, destination, config).await
189+
// The unified transfer dialog routes every cross-device copy through
190+
// `copy_between_volumes`; this plain command is the same-`root` local path,
191+
// so no ejectable volume is involved (empty busy set).
192+
ops_copy_files_start(app, sources, destination, config, vec![]).await
190193
}
191194

192195
/// Uses rename() for same-filesystem (instant), copy+delete for cross-filesystem.
@@ -203,7 +206,9 @@ pub async fn move_files(
203206
let destination = PathBuf::from(expand_tilde(&destination));
204207
let config = config.unwrap_or_default();
205208

206-
ops_move_files_start(app, sources, destination, config).await
209+
// Same-`root` local move (the FE uses `move_between_volumes` whenever the
210+
// source and destination volumes differ), so no ejectable volume here.
211+
ops_move_files_start(app, sources, destination, config, vec![]).await
207212
}
208213

209214
/// Recursively deletes files and directories. Same events as `copy_files`.

apps/desktop/src-tauri/src/commands/ui.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,13 @@ pub fn show_breadcrumb_context_menu<R: Runtime>(
127127
) -> Result<(), String> {
128128
let app = window.app_handle();
129129
let accelerator = frontend_shortcut_to_accelerator(&shortcut).unwrap_or_default();
130-
let menu =
131-
build_breadcrumb_context_menu(app, &accelerator, eject_volume_name.as_deref()).map_err(|e| e.to_string())?;
130+
// Disable the eject item while a write op touches this volume (the picker's
131+
// inline eject button is disabled the same way).
132+
let eject_busy = eject_volume_id
133+
.as_ref()
134+
.is_some_and(|id| crate::file_system::busy_volume_ids().contains(id));
135+
let menu = build_breadcrumb_context_menu(app, &accelerator, eject_volume_name.as_deref(), eject_busy)
136+
.map_err(|e| e.to_string())?;
132137

133138
// Stash eject target so on_menu_event can read it back when the user clicks
134139
// the "Eject (name)" item. If only one of the two args is present, treat as no

apps/desktop/src-tauri/src/file_system/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@ pub(crate) use watcher::compute_diff;
6363
// Re-export write operation types
6464
pub use write_operations::{
6565
OperationStatus, OperationSummary, WriteOperationConfig, WriteOperationError, WriteOperationStartResult,
66-
cancel_all_write_operations, cancel_write_operation, copy_files_start, delete_files_start, get_operation_status,
67-
list_active_operations, move_files_start, trash_files_start,
66+
busy_volume_ids, cancel_all_write_operations, cancel_write_operation, copy_files_start, delete_files_start,
67+
get_operation_status, init_busy_volume_emitter, list_active_operations, move_files_start, trash_files_start,
6868
};
6969
// Re-export volume copy types and functions
7070
pub use write_operations::{

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,13 +153,23 @@ See also: [`apps/desktop/src-tauri/src/downloads/CLAUDE.md`](../../downloads/CLA
153153
| `write-cancelled` | Operation cancelled (includes `rolled_back` flag) |
154154
| `write-error` | Operation failed. Carries `error: WriteOperationError` (typed) plus `friendly: FriendlyError` (rendered title/explanation/suggestion + category) populated by `WriteErrorEvent::new` via `friendly_from_write_error`. The FE renders the `friendly` payload directly in `TransferErrorDialog` and applies category-based colors. |
155155
| `write-settled` | Emitted once per op after the spawned background task fully returns. See [Settle contract](#settle-contract). |
156+
| `volumes-busy-changed` | The set of volume IDs with an in-flight op changed (an op started or finished). Payload is `string[]`. See [Busy-volumes set](#busy-volumes-set). |
156157
| `write-source-item-done` | All files for a top-level source item processed (for gradual deselection) |
157158
| `dry-run-complete` | `config.dry_run == true` (returns `DryRunResult`) |
158159
| `scan-preview-progress` | During `start_scan_preview` |
159160
| `scan-preview-complete` | Preview scan finished |
160161
| `scan-preview-error` | Preview scan failed |
161162
| `scan-preview-cancelled` | Preview scan cancelled |
162163

164+
## Busy-volumes set
165+
166+
Drives "disable Eject while an op reads from / writes to this device" so a disconnect can't truncate an in-flight file. Lives in `state.rs`.
167+
168+
- Each op records the volume IDs it touches via `register_operation_status(op_id, type, volume_ids)`. Source **and** destination go in (a download from a phone is as corruptible as an upload to it). `OperationStateGuard`'s `Drop` clears them on unregister, so a panicking op can't leave a volume stuck busy.
169+
- The busy set is the union of every active op's `volume_ids`, minus `root` (never ejectable). `recompute_and_emit_busy_volumes` fires `volumes-busy-changed` only when membership changes — progress ticks don't churn it. Membership-by-union means two concurrent transfers to one device keep it busy until both finish, with no manual refcount.
170+
- **Where `volume_ids` come from**: the cross-volume transfer entry points (`copy_between_volumes`, `move_between_volumes`, `move_within_same_volume`) and the volume-aware `delete_files_start` carry the IDs; `copy_files_start` / `move_files_start` take a `volume_ids` param so the both-local branch of `copy_between_volumes` (which is how a local→USB / DMG copy lands) still marks the ejectable destination. The plain `copy_files` / `move_files` / `trash` commands pass `vec![]` — the unified transfer dialog only routes through them for same-`root` ops, where no ejectable volume is involved.
171+
- **Consumers**: `busy_volume_ids()` backs the `get_busy_volume_ids` bootstrap command, the `eject_volume` server-side guard (refuses a busy volume — the real safety net, since the picker's disable is only UX), and the native breadcrumb-menu builder (renders the Eject item disabled with a ` (busy)` suffix). The frontend `volume-busy-store.svelte.ts` subscribes to `volumes-busy-changed` and exposes `isVolumeBusy(id)` to disable the picker's eject controls. `init_busy_volume_emitter(app)` wires the emitter at startup (`lib.rs`).
172+
163173
## Settle contract
164174

165175
`write-settled` fires exactly once per operation, after the spawned background task has fully torn down — including in-flight USB / network teardown that may briefly outlive the `write-cancelled` emit. The FE uses it to gate the "Cancelling…" dialog close so the user can't dispatch a new op against a still-tearing-down volume (the wedge mode that cancel propagation already shortens but doesn't eliminate).

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

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@ use trash::trash_files_with_progress;
5858
// Re-export public types
5959
pub use scan_preview::{cancel_scan_preview, get_scan_preview_totals, start_scan_preview};
6060
pub use state::{
61-
cancel_all_write_operations, cancel_write_operation, get_operation_status, list_active_operations,
62-
resolve_write_conflict,
61+
busy_volume_ids, cancel_all_write_operations, cancel_write_operation, get_operation_status,
62+
init_busy_volume_emitter, list_active_operations, resolve_write_conflict,
6363
};
6464
#[allow(unused_imports, reason = "Public API re-exports for consumers of this module")]
6565
pub use types::{
@@ -109,6 +109,7 @@ async fn start_write_operation<F>(
109109
app: tauri::AppHandle,
110110
operation_type: WriteOperationType,
111111
progress_interval_ms: u64,
112+
volume_ids: Vec<String>,
112113
handler: F,
113114
) -> Result<WriteOperationStartResult, WriteOperationError>
114115
where
@@ -120,7 +121,7 @@ where
120121
if let Ok(mut cache) = WRITE_OPERATION_STATE.write() {
121122
cache.insert(operation_id.clone(), Arc::clone(&state));
122123
}
123-
register_operation_status(&operation_id, operation_type);
124+
register_operation_status(&operation_id, operation_type, volume_ids);
124125

125126
let operation_id_for_spawn = operation_id.clone();
126127

@@ -185,11 +186,16 @@ where
185186
}
186187

187188
/// Starts a copy operation in the background.
189+
///
190+
/// `volume_ids` lists the volumes this copy touches (source + destination), so
191+
/// an ejectable USB / DMG / SMB volume is marked busy while the copy runs. Pass
192+
/// an empty `Vec` for a same-`root` local copy (root is never ejectable).
188193
pub async fn copy_files_start(
189194
app: tauri::AppHandle,
190195
sources: Vec<PathBuf>,
191196
destination: PathBuf,
192197
config: WriteOperationConfig,
198+
volume_ids: Vec<String>,
193199
) -> Result<WriteOperationStartResult, WriteOperationError> {
194200
log::info!(
195201
"copy_files_start: sources={:?}, destination={:?}, dry_run={}",
@@ -202,6 +208,7 @@ pub async fn copy_files_start(
202208
app,
203209
WriteOperationType::Copy,
204210
config.progress_interval_ms,
211+
volume_ids,
205212
move |app, op_id, state| {
206213
validate_sources(&sources)?;
207214
validate_destination(&destination)?;
@@ -224,6 +231,7 @@ pub async fn move_files_start(
224231
sources: Vec<PathBuf>,
225232
destination: PathBuf,
226233
config: WriteOperationConfig,
234+
volume_ids: Vec<String>,
227235
) -> Result<WriteOperationStartResult, WriteOperationError> {
228236
log::info!(
229237
"move_files_start: sources={:?}, destination={:?}, dry_run={}",
@@ -236,6 +244,7 @@ pub async fn move_files_start(
236244
app,
237245
WriteOperationType::Move,
238246
config.progress_interval_ms,
247+
volume_ids,
239248
move |app, op_id, state| {
240249
validate_sources(&sources)?;
241250
validate_destination(&destination)?;
@@ -278,7 +287,7 @@ pub async fn delete_files_start(
278287
if let Ok(mut cache) = WRITE_OPERATION_STATE.write() {
279288
cache.insert(operation_id.clone(), Arc::clone(&state));
280289
}
281-
register_operation_status(&operation_id, WriteOperationType::Delete);
290+
register_operation_status(&operation_id, WriteOperationType::Delete, vec![volume_id_str.clone()]);
282291

283292
let operation_id_for_spawn = operation_id.clone();
284293
tokio::spawn(async move {
@@ -351,10 +360,12 @@ pub async fn delete_files_start(
351360
operation_type: WriteOperationType::Delete,
352361
})
353362
} else {
363+
// Local same-`root` delete: no ejectable volume involved.
354364
start_write_operation(
355365
app,
356366
WriteOperationType::Delete,
357367
config.progress_interval_ms,
368+
vec![],
358369
move |app, op_id, state| {
359370
validate_sources(&sources)?;
360371
delete_files_with_progress(&app, &op_id, &state, &sources, &config)
@@ -377,10 +388,12 @@ pub async fn trash_files_start(
377388
) -> Result<WriteOperationStartResult, WriteOperationError> {
378389
log::info!("trash_files_start: sources={:?}", sources);
379390

391+
// Trash always targets the local macOS Trash; no ejectable volume involved.
380392
start_write_operation(
381393
app,
382394
WriteOperationType::Trash,
383395
config.progress_interval_ms,
396+
vec![],
384397
move |app, op_id, state| {
385398
validate_sources(&sources)?;
386399
let events: Arc<dyn types::OperationEventSink> = Arc::new(types::TauriEventSink::new(app));

0 commit comments

Comments
 (0)