diff --git a/README.md b/README.md index 6a013c24..d1e6687e 100644 --- a/README.md +++ b/README.md @@ -122,7 +122,9 @@ These move: check each project's current docs before relying on a cell. exclude rules, and a configurable symlink policy, and that skips excluded directories instead of descending them. - Live exclusion preview that re-classifies the folder tree as you edit a rule, - from an in-memory tree rather than a fresh walk of the disk. + from an in-memory tree rather than a fresh walk of the disk, with a + per-folder file-count and size rollup so an exclude rule's actual weight - + or what re-including it would cost - is visible before you commit to it. - Exclusion rules that take effect immediately, even mid-backup: saving a new rule stops the running backup from uploading anything it newly excludes at the next file boundary (the file already in flight still finishes cleanly), and @@ -192,6 +194,11 @@ These move: check each project's current docs before relying on a cell. plain local / removable folder (USB drive, external disk, NAS share) - all behind one pluggable backend trait, so adding the next one is a new backend crate plus a factory arm rather than a fork of every call site. +- Destination folder picker with client-side sort (name or last-modified) and + type-to-filter, a "New folder" button on every browsable destination, and + inline rename on Google Drive and SFTP (S3's key-prefix "folders" have no + separate identity to rename, so that control is disabled there with an + explanation instead of hidden outright). - Scheduled integrity scrub: on top of the local re-hash and the remote-existence audit above, a rolling background pass re-checks each already-backed-up object's size, and its content checksum where the diff --git a/crates/driven-backend/src/lib.rs b/crates/driven-backend/src/lib.rs index 66b08ed0..759b0f22 100644 --- a/crates/driven-backend/src/lib.rs +++ b/crates/driven-backend/src/lib.rs @@ -128,6 +128,9 @@ pub struct BackendDescriptor { /// file, so a point-in-time restore returns the older bytes rather than /// today's (`BackendKind::supports_version_history`; issue #220). pub supports_version_history: bool, + /// Whether the destination picker's inline rename affordance applies to + /// this backend's folder rows (`BackendKind::supports_rename`; issue #307). + pub supports_rename: bool, } /// Every destination this build can construct, in picker order. The first entry @@ -142,6 +145,7 @@ pub fn descriptors() -> Vec { uses_oauth: kind.uses_oauth(), supports_folder_picker: kind.supports_folder_picker(), supports_version_history: kind.supports_version_history(), + supports_rename: kind.supports_rename(), }) .collect() } @@ -551,6 +555,7 @@ mod tests { desc.supports_version_history, kind.supports_version_history() ); + assert_eq!(desc.supports_rename, kind.supports_rename()); } assert_eq!(d[0].kind, BackendKind::default()); } diff --git a/crates/driven-core/src/types.rs b/crates/driven-core/src/types.rs index f9fe4d6a..520d082e 100644 --- a/crates/driven-core/src/types.rs +++ b/crates/driven-core/src/types.rs @@ -1253,6 +1253,13 @@ pub enum ErrorCode { /// (the identity marker, then the write/remove round trip), since the /// remedy is identical either way. SftpRootNotWritable, + /// `remote.rename_unsupported` - the destination picker's inline rename + /// (issue #307) was called against a backend whose `RemoteStore` has no + /// rename primitive (S3, whose "folders" are key prefixes with no + /// separate identity to rename). The picker UI hides the affordance for + /// such backends already (`BackendKind::supports_rename`); reaching this + /// code means a stale client called it anyway. + RemoteRenameUnsupported, } impl ErrorCode { @@ -1311,6 +1318,7 @@ impl ErrorCode { ErrorCode::SftpRootNotADirectory => "sftp.root_not_a_directory", ErrorCode::SftpDestMarkerMismatch => "sftp.dest_marker_mismatch", ErrorCode::SftpRootNotWritable => "sftp.root_not_writable", + ErrorCode::RemoteRenameUnsupported => "remote.rename_unsupported", } } @@ -1373,6 +1381,7 @@ impl ErrorCode { "sftp.root_not_a_directory" => ErrorCode::SftpRootNotADirectory, "sftp.dest_marker_mismatch" => ErrorCode::SftpDestMarkerMismatch, "sftp.root_not_writable" => ErrorCode::SftpRootNotWritable, + "remote.rename_unsupported" => ErrorCode::RemoteRenameUnsupported, _ => return None, }) } diff --git a/crates/driven-drive/src/fake/mod.rs b/crates/driven-drive/src/fake/mod.rs index 8669451e..8b6a114c 100644 --- a/crates/driven-drive/src/fake/mod.rs +++ b/crates/driven-drive/src/fake/mod.rs @@ -1110,6 +1110,34 @@ impl RemoteStore for InMemoryRemoteStore { .collect()) } + /// Renames a folder in place, mirroring `GoogleDriveStore::rename_folder` + /// (issue #307): only `name` changes, id and `parent_id` are untouched. + async fn rename_folder( + &self, + folder_id: &str, + new_name: &str, + drive_context: &DriveContext, + ) -> anyhow::Result { + self.record_context(drive_context); + self.check_request_faults(RequestKind::WriteTarget).await?; + let mut guard = self.inner.lock(); + if !guard + .objects + .get(folder_id) + .is_some_and(FileEntry::is_folder) + { + anyhow::bail!("fake: no folder with file_id {folder_id}"); + } + let new_now = guard.tick(); + let entry = guard + .objects + .get_mut(folder_id) + .expect("presence just checked above"); + entry.name = new_name.to_string(); + entry.modified_time_ms = new_now; + Ok(entry.to_remote_entry()) + } + async fn create( &self, parent_id: &str, diff --git a/crates/driven-drive/src/google/mod.rs b/crates/driven-drive/src/google/mod.rs index a7330a9e..dd8fc1fc 100644 --- a/crates/driven-drive/src/google/mod.rs +++ b/crates/driven-drive/src/google/mod.rs @@ -1108,6 +1108,33 @@ impl RemoteStore for GoogleDriveStore { self.list_query(&q, drive_context).await } + /// Renames a folder via `PATCH /files/{id}` with just the `name` field - + /// the id and `parents` are untouched (issue #307). Unlike `create_folder` + /// this is a PATCH-by-id, which is idempotent, so it is safe to blind-retry + /// through the normal `send_json` path. + async fn rename_folder( + &self, + folder_id: &str, + new_name: &str, + _drive_context: &DriveContext, + ) -> anyhow::Result { + let body = json_body(&serde_json::json!({ "name": new_name }))?; + let file: DriveFile = self + .send_json(|token| { + self.http + .patch(format!("{DRIVE_API_BASE}/files/{folder_id}")) + .query(&[("fields", pagination::FILE_FIELDS), SUPPORTS_ALL_DRIVES]) + .bearer_auth(token) + .header(reqwest::header::CONTENT_TYPE, "application/json") + .body(body.clone()) + }) + .await + // The target is `folder_id`; a 404 / unclassified 403 is a + // dest-folder condition, not a transient (mirrors `create_folder`). + .map_err(map_parent_write_error)?; + Ok(file.into_remote_entry()) + } + async fn create( &self, parent_id: &str, diff --git a/crates/driven-drive/tests/fake_contract.rs b/crates/driven-drive/tests/fake_contract.rs index d84be446..f69390c6 100644 --- a/crates/driven-drive/tests/fake_contract.rs +++ b/crates/driven-drive/tests/fake_contract.rs @@ -1004,3 +1004,69 @@ async fn fake_with_fileid_recycle_reuses_trashed_id() { "exactly one live object now holds the recycled id" ); } + +// --------------------------------------------------------------------------- +// rename_folder (issue #307 - the destination picker's inline rename). +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn fake_rename_folder_changes_only_the_name() { + let store = fake(); + let root = store.root_id().to_string(); + let folder = store + .ensure_folder(&root, "Old machines", &DriveContext::MyDrive) + .await + .expect("ensure_folder"); + + let renamed = store + .rename_folder(&folder.id, "Archive", &DriveContext::MyDrive) + .await + .expect("rename_folder"); + + assert_eq!(renamed.id, folder.id, "id is unchanged by a rename"); + assert_eq!(renamed.name, "Archive"); + assert_eq!(renamed.parents, folder.parents, "location is unchanged"); + + // ...and list_folder agrees. + let listing = store + .list_folder(&root, &DriveContext::MyDrive) + .await + .expect("list root"); + let entry = listing + .iter() + .find(|e| e.id == folder.id) + .expect("the renamed folder is still listed under the same id"); + assert_eq!(entry.name, "Archive"); +} + +#[tokio::test] +async fn fake_rename_folder_rejects_an_unknown_id() { + let store = fake(); + let err = store + .rename_folder("does-not-exist", "New name", &DriveContext::MyDrive) + .await + .expect_err("renaming an unknown id must Err"); + assert!(err.to_string().contains("does-not-exist")); +} + +#[tokio::test] +async fn fake_rename_folder_refuses_to_rename_a_file() { + let store = fake(); + let root = store.root_id().to_string(); + let file = store + .create( + &root, + "a.txt", + "text/plain", + UploadBody::Bytes(Bytes::from_static(b"x")), + props(&[]), + ) + .await + .expect("create"); + + let err = store + .rename_folder(&file.id, "b.txt", &DriveContext::MyDrive) + .await + .expect_err("renaming a FILE via rename_folder must Err"); + assert!(err.to_string().contains(&file.id)); +} diff --git a/crates/driven-remote/src/backend.rs b/crates/driven-remote/src/backend.rs index c765dea0..7ae345cf 100644 --- a/crates/driven-remote/src/backend.rs +++ b/crates/driven-remote/src/backend.rs @@ -122,6 +122,33 @@ impl BackendKind { } } + /// Whether this backend's destination picker can offer an inline RENAME + /// on a folder row (issue #307). Mirrors + /// [`crate::RemoteStore::rename_folder`]'s implementations one-for-one - + /// this is the flag the picker UI reads to + /// decide whether to show the affordance at all, so it must never say + /// `true` for a backend whose store still falls through to the trait's + /// unsupported default. + pub const fn supports_rename(self) -> bool { + match self { + // `files.update` with just a `name` patch - id and parents + // untouched. + BackendKind::GoogleDrive => true, + // S3 "folders" are key prefixes with no separate identity to + // rename; doing so for real would mean copying every object under + // the prefix to a new key and deleting the old ones, a bulk + // operation the picker's single-row rename does not attempt. + BackendKind::S3 => false, + // The destination folder has no browsable tree at all + // (`supports_folder_picker` is false), so there is no row to + // rename. + BackendKind::LocalFolder => false, + // An SFTP RENAME of the directory, with its sidecar (if any) + // moved alongside it. + BackendKind::Sftp => true, + } + } + /// Whether this backend can honour per-source VERSION HISTORY: keeping the /// bytes of a superseded file so a point-in-time restore ("restore this /// source's files as they were on an earlier date") really returns the older diff --git a/crates/driven-remote/src/remote_store.rs b/crates/driven-remote/src/remote_store.rs index 6f7a42e6..c468d342 100644 --- a/crates/driven-remote/src/remote_store.rs +++ b/crates/driven-remote/src/remote_store.rs @@ -294,6 +294,37 @@ pub trait RemoteStore: Send + Sync { drive_context: &DriveContext, ) -> anyhow::Result>; + /// Renames a folder IN PLACE - its LOCATION (parent) never changes, only + /// its display name (issue #307, the destination picker's inline rename). + /// + /// The returned [`RemoteEntry::id`] is the folder's id to use from now on. + /// For Drive this is always the SAME id the caller passed in (a rename is + /// a pure metadata patch by opaque file id). For SFTP it can DIFFER: an + /// SFTP id is built from encoded path components, so a rename that + /// actually changes the encoded name moves the real directory and mints a + /// new id at the new path - the caller MUST replace any id it was holding + /// with the one this call returns, never assume the input `folder_id` + /// still resolves. + /// + /// The default degrades to an explicit, stably-coded "unsupported" error + /// rather than a silent no-op: [`BackendKind::supports_rename`] is what + /// tells the picker UI whether to offer the affordance at all, so reaching + /// this default means a stale client called it anyway and must be told + /// clearly, not left to wonder why nothing changed. S3 "folders" are key + /// prefixes with no separate identity to rename (renaming one would mean + /// copying every object under the prefix to a new key and deleting the + /// old ones - a bulk operation the picker's inline affordance does not + /// attempt), so it keeps this default. + async fn rename_folder( + &self, + folder_id: &str, + new_name: &str, + drive_context: &DriveContext, + ) -> anyhow::Result { + let _ = (folder_id, new_name, drive_context); + anyhow::bail!("remote.rename_unsupported: this destination cannot rename folders") + } + /// Enumerates the Shared Drives the authenticated account can access /// (Drive `drives.list`), for the destination picker to show Shared Drive /// roots beside My Drive (issue #7). diff --git a/crates/driven-sftp/src/store.rs b/crates/driven-sftp/src/store.rs index 74115777..b121b01b 100644 --- a/crates/driven-sftp/src/store.rs +++ b/crates/driven-sftp/src/store.rs @@ -2009,6 +2009,82 @@ impl RemoteStore for SftpStore { .collect() } + /// Renames a folder in place via SFTP `RENAME`, moving its sidecar (if it + /// has one) alongside it (issue #307). + /// + /// Unlike Drive, the on-disk stored name is DERIVED from the display name + /// (`names::encode`), so a rename that actually changes the encoded form + /// must move the real directory - a metadata-only patch would leave the + /// picker showing one name while every other SFTP client (and Driven + /// itself, on the next `list_folder`) sees another. `encode` is + /// identity-preserving for an ordinary name (it only escapes reserved + /// characters, a trailing dot/space, or a sidecar-suffix collision), so + /// this is the common case for any name change worth renaming to. Only + /// when the encoded form comes out UNCHANGED - retyping the same name, or + /// the astronomically rare case of two different over-length names + /// truncating to the same digest - is nothing moved and only the + /// sidecar's display name is rewritten. + /// + /// This is a single top-level rename of a folder the user is organizing in + /// the destination picker, not a source-mirrored path, so it does not go + /// through `resolve_stored_name`'s concurrent-upload collision-claim + /// machinery - there is nothing else racing to create a sibling under the + /// same name while this runs. + async fn rename_folder( + &self, + folder_id: &str, + new_name: &str, + _drive_context: &DriveContext, + ) -> anyhow::Result { + let channel = self.channel().await?; + let sftp = channel.sftp(); + self.guard_root(sftp).await?; + + let dir_id = folder_prefix(folder_id); + let old_stored = base_name(&dir_id).to_string(); + let parent_id = parent_of(&dir_id); + let parent_path = self.remote_path(&parent_id)?; + let new_stored = names::encode(new_name)?; + + // Carry forward any app_properties the existing sidecar recorded (a + // rename must not silently drop Driven's own identity stamp on a + // folder it created). + let existing_props = Self::read_sidecar(sftp, &parent_path, &old_stored) + .await? + .map(|s| s.props) + .unwrap_or_default(); + + if new_stored != old_stored { + let old_path = self.remote_path(&dir_id)?; + let new_path = join_remote(&parent_path, &new_stored); + if Self::stat_kind(sftp, &new_path).await?.is_some() { + anyhow::bail!( + "internal.invalid_input: a folder or file named {new_name:?} already exists here" + ); + } + sftp.rename(old_path, new_path) + .await + .map_err(|e| sftp_op_error(&format!("rename folder to {new_name:?}"), e))?; + Self::remove_sidecar(sftp, &parent_path, &old_stored).await?; + } + + let sidecar = Sidecar { + version: 1, + kind: EntryKind::Dir, + name: new_name.to_string(), + stored: new_stored.clone(), + size: None, + md5: None, + mime: Some(FOLDER_MIME.to_string()), + modified_ms: now_ms(), + props: existing_props, + }; + Self::write_sidecar(&channel, self.write_deadline, &parent_path, &sidecar).await?; + + let new_id = crate::store::folder_id(&parent_id, &new_stored); + self.entry_for(sftp, &new_id).await + } + /// Write a new object at `/`. /// /// Unlike Drive, a filesystem cannot hold two files of one name in one @@ -2858,6 +2934,110 @@ mod tests { assert_eq!(listed[0].mime_type, FOLDER_MIME); } + #[tokio::test] + async fn rename_folder_moves_the_real_directory_and_its_sidecar() { + let server = TestSftpServer::spawn().await.unwrap(); + let store = store_for(&server); + + let folder = store + .ensure_folder("", "Old machines", &DriveContext::MyDrive) + .await + .expect("ensure_folder"); + let entry = store + .create( + &folder.id, + "notes.txt", + "text/plain", + body(b"hi"), + HashMap::new(), + ) + .await + .expect("create inside the folder"); + + let renamed = store + .rename_folder(&folder.id, "Archive", &DriveContext::MyDrive) + .await + .expect("rename_folder"); + + assert_eq!(renamed.name, "Archive"); + assert!( + server.root().join("Archive").is_dir(), + "the real directory moved" + ); + assert!( + !server.root().join("Old machines").exists(), + "nothing is left at the old path" + ); + + // Listed from the parent under the NEW name, and its own child (the + // file created before the rename) is still reachable at the new id. + let listed = store + .list_folder("", &DriveContext::MyDrive) + .await + .expect("list root"); + let names: Vec<&str> = listed.iter().map(|e| e.name.as_str()).collect(); + assert_eq!(names, vec!["Archive"], "{names:?}"); + let new_folder_id = listed[0].id.clone(); + + let inner = store + .list_folder(&new_folder_id, &DriveContext::MyDrive) + .await + .expect("list the renamed folder"); + assert_eq!(inner.len(), 1); + assert_eq!(inner[0].name, "notes.txt"); + // Unlike Drive's opaque ids, an SFTP id is PATH-based, so moving the + // parent directory necessarily changes the child's id too - it is + // still reachable, just at the new path. + assert_eq!(inner[0].id, "Archive/notes.txt"); + assert_ne!(inner[0].id, entry.id, "the old id no longer resolves"); + } + + #[tokio::test] + async fn rename_folder_refuses_to_clobber_an_existing_name() { + let server = TestSftpServer::spawn().await.unwrap(); + let store = store_for(&server); + + store + .ensure_folder("", "Alpha", &DriveContext::MyDrive) + .await + .expect("ensure_folder alpha"); + let beta = store + .ensure_folder("", "Beta", &DriveContext::MyDrive) + .await + .expect("ensure_folder beta"); + + let err = store + .rename_folder(&beta.id, "Alpha", &DriveContext::MyDrive) + .await + .expect_err("renaming onto an existing name must Err"); + assert!(err.to_string().contains("internal.invalid_input")); + // Both directories are exactly as they were. + assert!(server.root().join("Alpha").is_dir()); + assert!(server.root().join("Beta").is_dir()); + } + + #[tokio::test] + async fn rename_folder_to_the_same_name_only_rewrites_the_sidecar() { + // Retyping the same name is the ordinary case where the ENCODED form + // comes out unchanged: nothing needs to move on disk, just the + // sidecar's display name (a no-op in substance, but must not error). + let server = TestSftpServer::spawn().await.unwrap(); + let store = store_for(&server); + + let folder = store + .ensure_folder("", "docs", &DriveContext::MyDrive) + .await + .expect("ensure_folder"); + + let renamed = store + .rename_folder(&folder.id, "docs", &DriveContext::MyDrive) + .await + .expect("rename_folder to the same encoded name"); + assert_eq!(renamed.id, folder.id, "the id (and on-disk path) is stable"); + assert_eq!(renamed.name, "docs"); + assert!(server.root().join("docs").is_dir()); + } + #[tokio::test] async fn update_rewrites_in_place_and_carries_the_identity_stamp_forward() { let server = TestSftpServer::spawn().await.unwrap(); diff --git a/src-tauri/src/commands/dtos.rs b/src-tauri/src/commands/dtos.rs index f07a996c..ac980701 100644 --- a/src-tauri/src/commands/dtos.rs +++ b/src-tauri/src/commands/dtos.rs @@ -447,6 +447,13 @@ pub struct DriveFolderEntry { /// picker can badge it and know selecting it targets the drive root. #[serde(default)] pub is_shared_drive: bool, + /// Last-modified time as Unix epoch ms, where the backend provides one + /// (issue #306: the picker's Modified sort/column). `None` for a backend + /// whose "folders" carry no timestamp of their own (S3 key prefixes) - the + /// UI falls back to hiding the column value / excluding the row from a + /// Modified sort rather than showing a fabricated date. + #[serde(default)] + pub modified_time: Option, } /// The result of `pick_drive_folder` (SPEC s11.2 `DriveFolderListing`): the @@ -539,6 +546,28 @@ pub struct ExclusionPreviewNode { pub included: bool, /// File size in bytes; always 0 for a directory. pub size: u64, + /// Directories only (issue #305): the number of FILES discovered so far + /// anywhere beneath this directory, regardless of their own individual + /// verdict. Always 0 for a file. + /// + /// For a directory the walk DESCENDS into, this SETTLES upward as + /// descendants stream in - the value in an early batch is a partial count, + /// and a later batch re-emits the same node (same `path`) with the + /// updated total once more of its subtree has been classified. For a + /// directory the walk PRUNES (excluded with no reachable negation, so it + /// is never descended - see `exclusion_stream`'s pruning rule), this is + /// filled by one lightweight recursive disk count (no classification, just + /// `read_dir` + `metadata`) at the moment the directory is streamed, so it + /// already carries its final answer - "what would be freed" if the rule + /// stays. + #[serde(default)] + pub file_count: u64, + /// Directories only: the total bytes of every file discovered so far + /// beneath this directory (see `file_count` for the settling / pruned-dir + /// rules, which apply identically here). Always equal to `size` for a + /// file, so the frontend can treat this as "the rollup" uniformly. + #[serde(default)] + pub byte_size: u64, } /// One streamed batch of the exclusion preview (`exclusion_preview:batch`). @@ -563,6 +592,12 @@ pub struct ExclusionPreviewBatch { pub excluded_count: u64, /// Total bytes of the included files so far (exact, never truncated). pub included_bytes: u64, + /// Total bytes of the EXCLUDED files so far (exact, never truncated; + /// issue #305 summary line - "N would be freed"). Mirrors + /// `included_bytes`, which the one-shot `preview_exclusions` already had; + /// the streaming path only gained the excluded side with the rollups. + #[serde(default)] + pub excluded_bytes: u64, /// `true` once the streamed node cap was hit: the TREE stops growing but the /// counts above stay live and exact. pub truncated: bool, @@ -582,6 +617,10 @@ pub struct ExclusionPreviewDone { pub excluded_count: u64, /// Final total bytes of the included files. pub included_bytes: u64, + /// Final total bytes of the EXCLUDED files - issue #305's "would be + /// freed" figure. + #[serde(default)] + pub excluded_bytes: u64, /// `true` if the streamed TREE was capped (the counts are still exact). pub truncated: bool, /// `true` when the walk stopped early because it was cancelled (superseded diff --git a/src-tauri/src/commands/exclusion_stream.rs b/src-tauri/src/commands/exclusion_stream.rs index a876ec8d..753e23f5 100644 --- a/src-tauri/src/commands/exclusion_stream.rs +++ b/src-tauri/src/commands/exclusion_stream.rs @@ -69,7 +69,7 @@ //! updating to the exact end of the pass - the summary line stays truthful even //! when the tree is a partial view. -use std::collections::VecDeque; +use std::collections::{HashMap, HashSet, VecDeque}; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; @@ -279,6 +279,143 @@ impl Default for StreamConfig { } } +/// The live per-directory rollup a batch can settle over time (issue #305): +/// the total FILE COUNT and BYTE SIZE discovered so far anywhere beneath one +/// directory, regardless of the individual verdict of what is under it - the +/// same "how big is this folder" answer a file browser would give, which is +/// exactly the number the exclusions browser needs to show what an exclude +/// rule actually costs (or, for an excluded folder, what it would free). +/// +/// Tracked only for directories that were actually STREAMED as nodes (see +/// [`stream_classify_tree`]'s per-entry cap check) - a directory nobody has a +/// row for has nothing to update. +struct RollupAcc { + /// The directory's own include/exclude verdict, carried so a settled + /// update can be re-emitted as a full [`ExclusionPreviewNode`] without + /// looking the verdict back up. + included: bool, + file_count: u64, + byte_size: u64, +} + +/// Recursively counts files and sums bytes under `path` WITHOUT classifying +/// anything. +/// +/// Used only for a directory [`stream_classify_tree`] PRUNES (excluded, with +/// no reachable negation - see its module docs), so that row's rollup can +/// still answer "what would be freed" without descending it for real +/// classification. Cheaper than the classification pass it stands in for: no +/// matcher calls, and the subtree is walked and discarded in one call rather +/// than growing the BFS queue or streaming node by node. +/// +/// Follows the same walk policy as [`read_dir_entries`]: symlinks are never +/// followed, and an unreadable directory is skipped (logged at debug) rather +/// than failing the whole preview. The cancel flag is polled periodically so +/// an enormous excluded tree - the exact case pruning exists to avoid paying +/// for - cannot hang a preview that has already been asked to stop. +/// +/// Not cache-backed (unlike the classification pass): a rule edit that keeps +/// this same directory pruned re-walks its subtree from disk every time. That +/// is an accepted cost for a feature whose whole point is to show the size of +/// a subtree the classification pass deliberately never reads. +fn count_pruned_subtree(path: &Path, cancel: &AtomicBool) -> (u64, u64) { + let mut file_count: u64 = 0; + let mut byte_size: u64 = 0; + let mut stack: Vec = vec![path.to_path_buf()]; + let mut polled: usize = 0; + while let Some(dir) = stack.pop() { + polled += 1; + if polled % CANCEL_POLL_ENTRIES == 0 && cancel.load(Ordering::SeqCst) { + return (file_count, byte_size); + } + let Ok(read) = std::fs::read_dir(&dir) else { + tracing::debug!(target: TARGET, dir = %dir.display(), "pruned-subtree count: skipping unreadable directory"); + continue; + }; + for entry in read.flatten() { + polled += 1; + if polled % CANCEL_POLL_ENTRIES == 0 && cancel.load(Ordering::SeqCst) { + return (file_count, byte_size); + } + let Ok(file_type) = entry.file_type() else { + continue; + }; + if file_type.is_symlink() { + continue; + } + if file_type.is_dir() { + stack.push(entry.path()); + } else if file_type.is_file() { + file_count += 1; + byte_size = + byte_size.saturating_add(entry.metadata().map(|m| m.len()).unwrap_or(0)); + } + } + } + (file_count, byte_size) +} + +/// Adds `(file_count_delta, byte_delta)` to the rollup of EVERY ancestor +/// directory of `rel_str` that is currently tracked in `rollups` (see +/// [`RollupAcc`]'s doc for why an untracked ancestor is silently skipped +/// rather than an error), marking each one dirty so the next flush re-emits +/// its settled node. Walks the full ancestor chain, not just the immediate +/// parent, by repeatedly trimming the last `/`-separated component. +fn bump_ancestors( + rollups: &mut HashMap, + dirty: &mut HashSet, + rel_str: &str, + file_count_delta: u64, + byte_delta: u64, +) { + let mut cur = rel_str; + while let Some((parent, _)) = cur.rsplit_once('/') { + if let Some(acc) = rollups.get_mut(parent) { + acc.file_count += file_count_delta; + acc.byte_size = acc.byte_size.saturating_add(byte_delta); + dirty.insert(parent.to_string()); + } + cur = parent; + } +} + +/// Applies every dirty rollup to `pending`, clearing the dirty set. Called +/// right before a batch flushes, so a rollup update rides the SAME cadence as +/// new nodes rather than needing its own timer. +/// +/// A directory whose ORIGINAL node has not been flushed out of `pending` yet +/// (it streamed and settled within the same batch - the common case for a +/// small tree, or any directory near the end of a walk) is updated IN PLACE, +/// so the wire never carries two entries for one path where one would do. A +/// directory flushed in an EARLIER batch instead gets a fresh +/// [`ExclusionPreviewNode`] re-emission with the same `path` - the webview's +/// `upsert` updates that row in place rather than duplicating it, which is +/// the whole mechanism that lets a rollup "settle" visibly on a large tree. +fn drain_dirty_rollups( + rollups: &HashMap, + dirty: &mut HashSet, + pending: &mut Vec, +) { + for path in dirty.drain() { + let Some(acc) = rollups.get(&path) else { + continue; + }; + if let Some(existing) = pending.iter_mut().find(|n| n.path == path) { + existing.file_count = acc.file_count; + existing.byte_size = acc.byte_size; + continue; + } + pending.push(ExclusionPreviewNode { + path, + is_dir: true, + included: acc.included, + size: 0, + file_count: acc.file_count, + byte_size: acc.byte_size, + }); + } +} + /// A directory waiting to be classified, in BFS order. struct QueuedDir { /// Path relative to the source root; empty for the root itself. @@ -373,9 +510,20 @@ pub fn stream_classify_tree( let mut included_count: u64 = 0; let mut excluded_count: u64 = 0; let mut included_bytes: u64 = 0; + // Issue #305 summary line: the excluded-side counterpart of + // `included_bytes` - "N would be freed". + let mut excluded_bytes: u64 = 0; let mut streamed_nodes: usize = 0; let mut truncated = false; + // Issue #305 per-folder rollups: tracked only for directories that were + // actually streamed (see the per-entry cap check below), bumped for every + // classified entry regardless of the cap (like `included_count` / + // `excluded_count`, the rollup a tracked ancestor sees stays exact), and + // re-emitted through `dirty_rollups` at the next flush once they change. + let mut rollups: HashMap = HashMap::new(); + let mut dirty_rollups: HashSet = HashSet::new(); + let mut pending: Vec = Vec::with_capacity(cfg.batch_max_nodes); let mut last_flush = Instant::now(); @@ -435,20 +583,56 @@ pub fn stream_classify_tree( // `rel`. let rel_str = rel.to_string_lossy().replace('\\', "/"); + // Issue #305: this node's OWN rollup. Stays 0/0 for a file (its + // `size` already carries the weight) and for a directory the walk + // is about to DESCEND (its rollup starts empty and settles from + // its streamed descendants via `bump_ancestors`); a directory the + // walk PRUNES gets its final answer right now, from one + // lightweight recursive disk count. + let mut node_file_count: u64 = 0; + let mut node_byte_size: u64 = 0; + if is_dir { // Descend unless this dir is excluded AND pruning it is safe - // i.e. no `!`-rule anywhere could re-include something beneath // THIS directory (the scanner's own per-directory rule). - if included || matcher.negations_could_match_under(&rel) { - if let Some(decision) = child_state { - queue.push_back(QueuedDir { rel, decision }); + let will_descend = (included || matcher.negations_could_match_under(&rel)) + && child_state.is_some(); + if will_descend { + if streamed_nodes < cfg.node_stream_cap { + rollups.insert( + rel_str.clone(), + RollupAcc { + included, + file_count: 0, + byte_size: 0, + }, + ); } + queue.push_back(QueuedDir { + rel, + decision: child_state.expect("will_descend checked child_state.is_some()"), + }); + } else { + let (sub_count, sub_bytes) = count_pruned_subtree(&root.join(&rel), cancel); + node_file_count = sub_count; + node_byte_size = sub_bytes; + bump_ancestors( + &mut rollups, + &mut dirty_rollups, + &rel_str, + sub_count, + sub_bytes, + ); } } else if included { included_count += 1; included_bytes = included_bytes.saturating_add(size); + bump_ancestors(&mut rollups, &mut dirty_rollups, &rel_str, 1, size); } else { excluded_count += 1; + excluded_bytes = excluded_bytes.saturating_add(size); + bump_ancestors(&mut rollups, &mut dirty_rollups, &rel_str, 1, size); } // Past the cap the tree stops growing but the counts above keep @@ -461,6 +645,8 @@ pub fn stream_classify_tree( is_dir, included, size, + file_count: node_file_count, + byte_size: node_byte_size, }); } else if !truncated { truncated = true; @@ -470,12 +656,14 @@ pub fn stream_classify_tree( if pending.len() >= cfg.batch_max_nodes || last_flush.elapsed() >= cfg.batch_max_interval { + drain_dirty_rollups(&rollups, &mut dirty_rollups, &mut pending); emit(ExclusionPreviewBatch { preview_id: preview_id.to_string(), nodes: std::mem::take(&mut pending), included_count, excluded_count, included_bytes, + excluded_bytes, truncated, }); pending.reserve(cfg.batch_max_nodes); @@ -493,8 +681,11 @@ pub fn stream_classify_tree( } }; - // Final partial batch (also the ONLY batch for a small tree). Skipped when - // empty so a finished pass does not emit a redundant no-op event. + // Final partial batch (also the ONLY batch for a small tree). Drains any + // rollup still dirty so every directory's settled total reaches the + // webview even when the pass finished inside one batch. Skipped when both + // are empty so a finished pass does not emit a redundant no-op event. + drain_dirty_rollups(&rollups, &mut dirty_rollups, &mut pending); if !pending.is_empty() { emit(ExclusionPreviewBatch { preview_id: preview_id.to_string(), @@ -502,6 +693,7 @@ pub fn stream_classify_tree( included_count, excluded_count, included_bytes, + excluded_bytes, truncated, }); } @@ -511,6 +703,7 @@ pub fn stream_classify_tree( included_count, excluded_count, included_bytes, + excluded_bytes, truncated, cancelled, } @@ -787,6 +980,7 @@ mod tests { assert_eq!(done.included_count, 2, "keep.txt + docs/inner.txt"); assert_eq!(done.excluded_count, 2, "drop.log + docs/inner.log"); assert_eq!(done.included_bytes, 8, "5 + 3 bytes of included files"); + assert_eq!(done.excluded_bytes, 3, "2 + 1 bytes of excluded files"); assert!(!done.truncated); assert!(!done.cancelled); assert_eq!(done.preview_id, "gen-1"); @@ -1607,4 +1801,156 @@ mod tests { "the editor closed, so the tree is released" ); } + + // --------------------------------------------------------------------- + // Per-folder rollups (issue #305) + // --------------------------------------------------------------------- + + /// The LAST-emitted node for `path` across every batch - a directory's + /// rollup can be re-emitted as it settles, so the last one is its final + /// answer. + fn last_node<'a>( + batches: &'a [ExclusionPreviewBatch], + path: &str, + ) -> Option<&'a ExclusionPreviewNode> { + batches + .iter() + .flat_map(|b| b.nodes.iter()) + .rfind(|n| n.path == path) + } + + #[test] + fn a_descended_directory_rollup_settles_to_the_total_of_everything_beneath_it() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + write(&root.join("docs/a.txt"), "abcde"); + write(&root.join("docs/b.txt"), "xy"); + write(&root.join("docs/sub/c.txt"), "zzz"); + + let source = source_at(root, &[], &[]); + let (batches, _done) = run(root, &source, &StreamConfig::default()); + + let docs = last_node(&batches, "docs").expect("docs streamed"); + assert!(docs.is_dir); + assert_eq!(docs.file_count, 3, "a.txt + b.txt + sub/c.txt"); + assert_eq!(docs.byte_size, 10, "5 + 2 + 3 bytes"); + + let sub = last_node(&batches, "docs/sub").expect("docs/sub streamed"); + assert_eq!(sub.file_count, 1); + assert_eq!(sub.byte_size, 3); + } + + #[test] + fn a_pruned_excluded_directory_rollup_is_final_on_its_own_first_batch() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + write(&root.join("node_modules/pkg/index.js"), "abcdefgh"); + write(&root.join("node_modules/pkg/readme.md"), "xy"); + write(&root.join("src/app.js"), "z"); + + let source = source_at(root, &[], &["/node_modules/"]); + let (batches, done) = run(root, &source, &StreamConfig::default()); + + // node_modules itself is streamed (excluded, one node) but never + // descended - its children never stream as their own nodes. + assert!( + !batches + .iter() + .flat_map(|b| b.nodes.iter()) + .any(|n| n.path.starts_with("node_modules/")), + "a pruned directory streams no children" + ); + + let first_batch_with_nm = batches + .iter() + .find(|b| b.nodes.iter().any(|n| n.path == "node_modules")) + .expect("some batch streamed node_modules"); + let first_seen = first_batch_with_nm + .nodes + .iter() + .find(|n| n.path == "node_modules") + .unwrap(); + assert!(!first_seen.included); + // Final on the FIRST (only) batch that streamed it - a pruned + // directory needs no settling, unlike a descended one. + assert_eq!( + first_seen.file_count, 2, + "index.js + readme.md, counted off disk" + ); + assert_eq!(first_seen.byte_size, 10, "8 + 2 bytes"); + + assert_eq!(done.included_count, 1, "only src/app.js"); + assert_eq!( + done.excluded_count, 0, + "node_modules' children were never visited, so they are not in the exact count either" + ); + } + + #[test] + fn a_rollup_propagates_to_every_ancestor_not_just_the_immediate_parent() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + write(&root.join("a/b/c/leaf.txt"), "abcd"); + + let source = source_at(root, &[], &[]); + let (batches, _done) = run(root, &source, &StreamConfig::default()); + + for path in ["a", "a/b", "a/b/c"] { + let node = last_node(&batches, path).unwrap_or_else(|| panic!("{path} streamed")); + assert_eq!(node.file_count, 1, "{path}"); + assert_eq!(node.byte_size, 4, "{path}"); + } + } + + #[test] + fn a_descended_but_excluded_directory_rollup_counts_everything_beneath_it_either_way() { + // A directory can be excluded yet still DESCENDED when a negation + // could reach inside it (see the pruning-exception test above); its + // rollup counts every file beneath it regardless of each file's OWN + // individual verdict - "how big is this folder", not "how much of it + // is included". + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + write(&root.join("vendor/lib.js"), "x"); + write(&root.join("vendor/keep.js"), "yy"); + + let source = source_at(root, &["/vendor/keep.js"], &["/vendor/"]); + let (batches, _done) = run(root, &source, &StreamConfig::default()); + + let vendor = last_node(&batches, "vendor").expect("vendor streamed"); + assert!(!vendor.included); + assert_eq!(vendor.file_count, 2, "lib.js + keep.js, included or not"); + assert_eq!(vendor.byte_size, 3, "1 + 2 bytes"); + } + + #[test] + fn a_directory_past_the_node_cap_is_never_seeded_and_bumps_nothing() { + // A directory the cap kept off the wire has no row on the webview, so + // seeding (and later bumping) its rollup would be dead weight held + // for the life of the pass. This pins that it simply never appears as + // a rollup target: the pass must not panic or misbehave when an + // ancestor is untracked. + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + for i in 0..5 { + write(&root.join(format!("d{i}/leaf.txt")), "x"); + } + + let source = source_at(root, &[], &[]); + let cfg = StreamConfig { + node_stream_cap: 2, + ..StreamConfig::default() + }; + let (batches, done) = run(root, &source, &cfg); + + assert_eq!( + done.included_count, 5, + "the exact count is unaffected by the cap" + ); + assert_eq!( + all_nodes(&batches).len(), + 2, + "the streamed tree itself stops at the cap" + ); + } } diff --git a/src-tauri/src/commands/mod.rs b/src-tauri/src/commands/mod.rs index 68c1c107..edd0399e 100644 --- a/src-tauri/src/commands/mod.rs +++ b/src-tauri/src/commands/mod.rs @@ -234,6 +234,7 @@ fn code_from_message(msg: &str) -> Option { "update.signature_invalid", "update.endpoint_unreachable", "internal.invalid_input", + "remote.rename_unsupported", ]; CANDIDATES .iter() diff --git a/src-tauri/src/commands/sources.rs b/src-tauri/src/commands/sources.rs index 5f742ea1..f2393024 100644 --- a/src-tauri/src/commands/sources.rs +++ b/src-tauri/src/commands/sources.rs @@ -943,6 +943,10 @@ pub async fn pick_drive_folder( name: e.name, drive_id: listing_drive_id.clone(), is_shared_drive: false, + // Issue #306: 0 means "unknown" to Drive/SFTP (a real folder is + // never modified at the Unix epoch); S3 always reports 0 here + // since a key prefix carries no timestamp of its own. + modified_time: (e.modified_time != 0).then_some(e.modified_time), }) .collect(); @@ -987,6 +991,8 @@ async fn shared_drive_root_entries(store: &dyn RemoteStore) -> Vec { @@ -999,6 +1005,99 @@ async fn shared_drive_root_entries(store: &dyn RemoteStore) -> Vec, + account_id: AccountId, + parent_id: String, + name: String, + drive_id: Option, +) -> CommandResult { + let account = find_account(state.state().as_ref(), account_id).await?; + let trimmed = name.trim(); + if trimmed.is_empty() { + return Err(CommandError::with_code( + ErrorCode::InvalidInput, + "folder name must not be empty", + )); + } + let ca = crate::commands::settings::load_custom_ca_config(state.state().as_ref()) + .await + .unwrap_or_default(); + let proxy = crate::commands::settings::load_proxy_config(state.state().as_ref()).await?; + let (store, _default_folder_id) = select_picker_store(state.inner(), &account, &ca, &proxy)?; + let drive_context = DriveContext::from_stored(drive_id.as_deref()); + + let entry = store + .ensure_folder(&parent_id, trimmed, &drive_context) + .await + .map_err(CommandError::from)?; + let listing_drive_id = drive_context.drive_id().map(str::to_string); + tracing::info!(target: TARGET, account_id = %account_id, parent_id = %parent_id, name = %trimmed, "picker: folder created"); + Ok(DriveFolderEntry { + id: entry.id, + name: entry.name, + drive_id: listing_drive_id, + is_shared_drive: false, + modified_time: (entry.modified_time != 0).then_some(entry.modified_time), + }) +} + +/// `rename_remote_folder(account_id, folder_id, new_name, drive_id?)` - rename +/// a folder in place for the destination picker's inline rename (issue #307). +/// Drive and SFTP only (`BackendKind::supports_rename`) - the picker UI hides +/// the affordance for S3, whose `RemoteStore::rename_folder` falls through to +/// the trait's default and rejects with `remote.rename_unsupported`; reaching +/// that here means a stale client called it anyway. +#[tauri::command] +pub async fn rename_remote_folder( + state: State<'_, AppState>, + account_id: AccountId, + folder_id: String, + new_name: String, + drive_id: Option, +) -> CommandResult { + let account = find_account(state.state().as_ref(), account_id).await?; + let trimmed = new_name.trim(); + if trimmed.is_empty() { + return Err(CommandError::with_code( + ErrorCode::InvalidInput, + "folder name must not be empty", + )); + } + let ca = crate::commands::settings::load_custom_ca_config(state.state().as_ref()) + .await + .unwrap_or_default(); + let proxy = crate::commands::settings::load_proxy_config(state.state().as_ref()).await?; + let (store, _default_folder_id) = select_picker_store(state.inner(), &account, &ca, &proxy)?; + let drive_context = DriveContext::from_stored(drive_id.as_deref()); + + let entry = store + .rename_folder(&folder_id, trimmed, &drive_context) + .await + .map_err(CommandError::from)?; + let listing_drive_id = drive_context.drive_id().map(str::to_string); + tracing::info!(target: TARGET, account_id = %account_id, folder_id = %folder_id, "picker: folder renamed"); + Ok(DriveFolderEntry { + id: entry.id, + name: entry.name, + drive_id: listing_drive_id, + is_shared_drive: false, + modified_time: (entry.modified_time != 0).then_some(entry.modified_time), + }) +} + /// `preview_exclusions(req)` - preview which files the candidate rules would /// include vs exclude (SPEC s11.2; DESIGN s8.5 step 3). /// diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 245cc586..f6b512a4 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -686,6 +686,9 @@ pub fn run() { commands::sources::update_source, commands::sources::remove_source, commands::sources::pick_drive_folder, + // Issue #307: the picker's "New folder" / inline rename. + commands::sources::create_remote_folder, + commands::sources::rename_remote_folder, // Agent QA harness: env-gated headless twin of the native folder // picker (refused unless DRIVEN_E2E_HOOKS=1; see e2e_hooks docs). commands::e2e_hooks::e2e_pick_folder, diff --git a/src-tauri/src/tray.rs b/src-tauri/src/tray.rs index b6d27c37..0ed0d755 100644 --- a/src-tauri/src/tray.rs +++ b/src-tauri/src/tray.rs @@ -804,7 +804,11 @@ fn error_code_is_network(code: ErrorCode) -> bool { | ErrorCode::SftpRootMissing | ErrorCode::SftpRootNotADirectory | ErrorCode::SftpRootNotWritable - | ErrorCode::SftpDestMarkerMismatch => false, + | ErrorCode::SftpDestMarkerMismatch + // The destination picker's inline rename is a per-backend capability + // question, not a reachability one - the server answered fine, this + // backend just has no rename primitive. + | ErrorCode::RemoteRenameUnsupported => false, } } diff --git a/ui/src/__tests__/backend-picker.test.ts b/ui/src/__tests__/backend-picker.test.ts index 5b7699de..e52b67a0 100644 --- a/ui/src/__tests__/backend-picker.test.ts +++ b/ui/src/__tests__/backend-picker.test.ts @@ -24,6 +24,7 @@ const DRIVE: BackendDto = { usesOauth: true, supportsFolderPicker: true, supportsVersionHistory: true, + supportsRename: true, isDefault: true, }; @@ -35,6 +36,7 @@ const OTHER: BackendDto = { usesOauth: false, supportsFolderPicker: false, supportsVersionHistory: false, + supportsRename: false, isDefault: false, }; diff --git a/ui/src/__tests__/drive-folder-picker.test.ts b/ui/src/__tests__/drive-folder-picker.test.ts index 4df555b2..21b21b62 100644 --- a/ui/src/__tests__/drive-folder-picker.test.ts +++ b/ui/src/__tests__/drive-folder-picker.test.ts @@ -171,3 +171,243 @@ describe("DriveFolderPicker root label is per-destination", () => { ); }); }); + +// Issue #306: client-side sort + filter, and near-fullscreen sizing. +describe("DriveFolderPicker sort and filter (issue #306)", () => { + const LISTING: DriveFolderListing = { + currentFolderId: "root", + driveId: null, + currentFolderPath: "", + folders: [ + { id: "f-b", name: "Beta", modifiedTime: 2000 }, + { id: "f-a", name: "Archive", modifiedTime: 3000 }, + { id: "f-c", name: "camelCase", modifiedTime: null }, + ], + }; + + beforeEach(() => { + invokeMock.mockReset(); + invokeMock.mockResolvedValue(LISTING); + }); + + function names(wrapper: ReturnType): string[] { + return wrapper.findAll("li").map((li) => li.find("button").text()); + } + + it("sorts by name ascending by default", async () => { + const wrapper = mountPicker(); + await flushPromises(); + expect(names(wrapper)).toEqual(["Archive", "Beta", "camelCase"]); + }); + + it("re-sorts client-side when the sort control changes, without re-fetching", async () => { + const wrapper = mountPicker(); + await flushPromises(); + invokeMock.mockClear(); + + await wrapper.get('[data-testid="drive-picker-sort"]').setValue("nameDesc"); + expect(names(wrapper)).toEqual(["camelCase", "Beta", "Archive"]); + expect(invokeMock).not.toHaveBeenCalled(); + }); + + it("sorts by modified date, folders with no timestamp sorting last either way", async () => { + const wrapper = mountPicker(); + await flushPromises(); + + await wrapper.get('[data-testid="drive-picker-sort"]').setValue("modifiedDesc"); + expect(names(wrapper)).toEqual(["Archive", "Beta", "camelCase"]); + + await wrapper.get('[data-testid="drive-picker-sort"]').setValue("modifiedAsc"); + expect(names(wrapper)).toEqual(["Beta", "Archive", "camelCase"]); + }); + + it("filters to a case-insensitive substring match against the current folder only", async () => { + const wrapper = mountPicker(); + await flushPromises(); + + await wrapper.get('[data-testid="drive-picker-filter"]').setValue("ca"); + expect(names(wrapper)).toEqual(["camelCase"]); + + await wrapper.get('[data-testid="drive-picker-filter"]').setValue("nomatch"); + expect(wrapper.findAll("li")).toHaveLength(0); + expect(wrapper.text()).toContain(i18n.global.t("drivePicker.noMatches")); + }); + + it("resets the filter and any in-progress row action on navigation", async () => { + const wrapper = mountPicker(); + await flushPromises(); + await wrapper.get('[data-testid="drive-picker-filter"]').setValue("arch"); + expect(names(wrapper)).toEqual(["Archive"]); + + invokeMock.mockResolvedValueOnce({ + currentFolderId: "f-a", + driveId: null, + currentFolderPath: "", + folders: [], + }); + await wrapper.findAll("li")[0].get("button").trigger("click"); + await flushPromises(); + + expect( + (wrapper.get('[data-testid="drive-picker-filter"]').element as HTMLInputElement).value + ).toBe(""); + }); +}); + +// Issue #307: create + rename. +describe("DriveFolderPicker create and rename (issue #307)", () => { + const LISTING: DriveFolderListing = { + currentFolderId: "root", + driveId: null, + currentFolderPath: "", + folders: [{ id: "f-1", name: "Archive", modifiedTime: 1000 }], + }; + + beforeEach(() => { + invokeMock.mockReset(); + invokeMock.mockImplementation((cmd: string) => { + if (cmd === "pick_drive_folder") return Promise.resolve(LISTING); + return Promise.resolve(undefined); + }); + }); + + it("creates a folder and appends it to the list without a re-fetch", async () => { + invokeMock.mockImplementation((cmd: string, args: Record) => { + if (cmd === "pick_drive_folder") return Promise.resolve(LISTING); + if (cmd === "create_remote_folder") { + expect(args).toEqual({ + accountId: ACCOUNT, + parentId: "root", + name: "New folder", + driveId: null, + }); + return Promise.resolve({ id: "f-new", name: "New folder", modifiedTime: 5000 }); + } + throw new Error(`unexpected command ${cmd}`); + }); + const wrapper = mountPicker(); + await flushPromises(); + + await wrapper.get('[data-testid="drive-picker-new-folder"]').trigger("click"); + await wrapper.get('[data-testid="drive-picker-create-input"]').setValue("New folder"); + await wrapper.get('[data-testid="drive-picker-create-confirm"]').trigger("click"); + await flushPromises(); + + expect(wrapper.find('[data-testid="drive-picker-create-row"]').exists()).toBe(false); + expect(wrapper.text()).toContain("New folder"); + }); + + it("shows the create error inline and keeps the row open to retry", async () => { + invokeMock.mockImplementation((cmd: string) => { + if (cmd === "pick_drive_folder") return Promise.resolve(LISTING); + if (cmd === "create_remote_folder") { + return Promise.reject({ code: "internal.invalid_input", message: "bad name" }); + } + throw new Error(`unexpected command ${cmd}`); + }); + const wrapper = mountPicker(); + await flushPromises(); + + await wrapper.get('[data-testid="drive-picker-new-folder"]').trigger("click"); + await wrapper.get('[data-testid="drive-picker-create-input"]').setValue("Bad*Name"); + await wrapper.get('[data-testid="drive-picker-create-confirm"]').trigger("click"); + await flushPromises(); + + expect(wrapper.get('[data-testid="drive-picker-create-error"]').text()).toBe( + i18n.global.t("errors.internal.invalid_input.short") + ); + // Still open, so the user can fix the name and retry. + expect(wrapper.find('[data-testid="drive-picker-create-row"]').exists()).toBe(true); + }); + + it("cancelling the new-folder row clears it without calling the backend", async () => { + const wrapper = mountPicker(); + await flushPromises(); + + await wrapper.get('[data-testid="drive-picker-new-folder"]').trigger("click"); + await wrapper.get('[data-testid="drive-picker-create-input"]').setValue("Whatever"); + await wrapper.get('[data-testid="drive-picker-create-cancel"]').trigger("click"); + + expect(wrapper.find('[data-testid="drive-picker-create-row"]').exists()).toBe(false); + expect(invokeMock).not.toHaveBeenCalledWith("create_remote_folder", expect.anything()); + }); + + it("offers rename only when supportsRename is true", async () => { + const withRename = mount(DriveFolderPicker, { + props: { accountId: ACCOUNT, backendKind: "google_drive", supportsRename: true }, + global: { plugins: [i18n] }, + }); + await flushPromises(); + expect(withRename.find('[data-testid="drive-picker-rename-f-1"]').exists()).toBe(true); + expect(withRename.find('[data-testid="drive-picker-rename-disabled"]').exists()).toBe(false); + + const withoutRename = mount(DriveFolderPicker, { + props: { accountId: ACCOUNT, backendKind: "s3", supportsRename: false }, + global: { plugins: [i18n] }, + }); + await flushPromises(); + expect(withoutRename.find('[data-testid="drive-picker-rename-f-1"]').exists()).toBe(false); + const disabled = withoutRename.get('[data-testid="drive-picker-rename-disabled"]'); + expect(disabled.attributes("title")).toBe( + i18n.global.t("drivePicker.renameUnsupportedTooltip") + ); + }); + + it("renames a folder in place, replacing the row with the backend's returned entry", async () => { + invokeMock.mockImplementation((cmd: string, args: Record) => { + if (cmd === "pick_drive_folder") return Promise.resolve(LISTING); + if (cmd === "rename_remote_folder") { + expect(args).toEqual({ + accountId: ACCOUNT, + folderId: "f-1", + newName: "Archived 2026", + driveId: null, + }); + // SFTP-shaped response: id can change on rename. + return Promise.resolve({ id: "f-1-renamed", name: "Archived 2026", modifiedTime: 9000 }); + } + throw new Error(`unexpected command ${cmd}`); + }); + const wrapper = mount(DriveFolderPicker, { + props: { accountId: ACCOUNT, backendKind: "sftp", supportsRename: true }, + global: { plugins: [i18n] }, + }); + await flushPromises(); + + await wrapper.get('[data-testid="drive-picker-rename-f-1"]').trigger("click"); + const input = wrapper.get('[data-testid="drive-picker-rename-input-f-1"]'); + expect((input.element as HTMLInputElement).value).toBe("Archive"); + await input.setValue("Archived 2026"); + await wrapper.get('[data-testid="drive-picker-rename-confirm-f-1"]').trigger("click"); + await flushPromises(); + + expect(wrapper.find('[data-testid="drive-picker-rename-input-f-1"]').exists()).toBe(false); + expect(wrapper.text()).toContain("Archived 2026"); + expect(wrapper.find('[data-testid="drive-picker-rename-f-1"]').exists()).toBe(false); + expect(wrapper.find('[data-testid="drive-picker-rename-f-1-renamed"]').exists()).toBe(true); + }); + + it("shows the rename error inline without losing the edit", async () => { + invokeMock.mockImplementation((cmd: string) => { + if (cmd === "pick_drive_folder") return Promise.resolve(LISTING); + if (cmd === "rename_remote_folder") { + return Promise.reject({ code: "remote.rename_unsupported", message: "nope" }); + } + throw new Error(`unexpected command ${cmd}`); + }); + const wrapper = mount(DriveFolderPicker, { + props: { accountId: ACCOUNT, backendKind: "s3", supportsRename: true }, + global: { plugins: [i18n] }, + }); + await flushPromises(); + + await wrapper.get('[data-testid="drive-picker-rename-f-1"]').trigger("click"); + await wrapper.get('[data-testid="drive-picker-rename-confirm-f-1"]').trigger("click"); + await flushPromises(); + + expect(wrapper.get('[data-testid="drive-picker-rename-error"]').text()).toBe( + i18n.global.t("errors.remote.rename_unsupported.short") + ); + expect(wrapper.find('[data-testid="drive-picker-rename-input-f-1"]').exists()).toBe(true); + }); +}); diff --git a/ui/src/__tests__/exclusion-preview-store.test.ts b/ui/src/__tests__/exclusion-preview-store.test.ts index 2e4925b9..4d52347a 100644 --- a/ui/src/__tests__/exclusion-preview-store.test.ts +++ b/ui/src/__tests__/exclusion-preview-store.test.ts @@ -49,8 +49,15 @@ import { } from "../stores/exclusionPreview"; import type { ExclusionPreviewBatch, ExclusionPreviewNode } from "../ipc/types"; -function node(path: string, isDir: boolean, included: boolean, size = 0): ExclusionPreviewNode { - return { path, isDir, included, size }; +function node( + path: string, + isDir: boolean, + included: boolean, + size = 0, + fileCount = 0, + byteSize = 0 +): ExclusionPreviewNode { + return { path, isDir, included, size, fileCount, byteSize }; } function batch( @@ -65,6 +72,7 @@ function batch( includedCount: files.filter((n) => n.included).length, excludedCount: files.filter((n) => !n.included).length, includedBytes: files.filter((n) => n.included).reduce((a, n) => a + n.size, 0), + excludedBytes: files.filter((n) => !n.included).reduce((a, n) => a + n.size, 0), truncated: false, ...over, }; @@ -377,6 +385,7 @@ describe("createExclusionPreview", () => { includedCount: 999, excludedCount: 999, includedBytes: 999, + excludedBytes: 0, truncated: false, cancelled: false, }); @@ -423,6 +432,7 @@ describe("createExclusionPreview", () => { includedCount: 12, excludedCount: 4, includedBytes: 4096, + excludedBytes: 0, truncated: false, cancelled: false, }); @@ -441,6 +451,7 @@ describe("createExclusionPreview", () => { includedCount: 3, excludedCount: 0, includedBytes: 1, + excludedBytes: 0, truncated: false, cancelled: true, }); @@ -537,6 +548,7 @@ describe("createExclusionPreview", () => { includedCount: 0, excludedCount: 0, includedBytes: 0, + excludedBytes: 0, truncated: false, cancelled: false, }); @@ -561,6 +573,7 @@ describe("createExclusionPreview", () => { includedCount: 0, excludedCount: 0, includedBytes: 0, + excludedBytes: 0, truncated: false, cancelled: true, }); @@ -733,4 +746,32 @@ describe("createExclusionPreview", () => { expect(preview.roots.value).toHaveLength(1); expect(preview.nodeAt("a.txt")?.included).toBe(false); }); + + // Issue #305: per-folder rollups. + it("carries a directory's file-count and byte rollup through", async () => { + const preview = await started(); + batchHandler!(batch("gen-1", [node("node_modules", true, false, 0, 31_204, 2_254_857_830)])); + preview.flush(); + const dir = preview.nodeAt("node_modules"); + expect(dir?.fileCount).toBe(31_204); + expect(dir?.byteSize).toBe(2_254_857_830); + }); + + it("settles a directory's rollup across later batches without duplicating the row", async () => { + // A pruned/excluded folder needs no settling (its rollup is final on + // arrival - see the Rust module docs), but a DESCENDED directory's + // rollup starts at 0 and the backend re-sends the SAME path with a + // growing total as its subtree streams in across batches. + const preview = await started(); + batchHandler!(batch("gen-1", [node("docs", true, true, 0, 0, 0)])); + preview.flush(); + expect(preview.nodeAt("docs")?.fileCount).toBe(0); + + batchHandler!(batch("gen-1", [node("docs", true, true, 0, 5, 500)])); + preview.flush(); + + expect(preview.roots.value).toHaveLength(1); + expect(preview.nodeAt("docs")?.fileCount).toBe(5); + expect(preview.nodeAt("docs")?.byteSize).toBe(500); + }); }); diff --git a/ui/src/__tests__/exclusion-preview-tree.test.ts b/ui/src/__tests__/exclusion-preview-tree.test.ts index af6cb7e1..fc10d8aa 100644 --- a/ui/src/__tests__/exclusion-preview-tree.test.ts +++ b/ui/src/__tests__/exclusion-preview-tree.test.ts @@ -48,8 +48,15 @@ import type { ExclusionPreviewBatch, ExclusionPreviewNode } from "../ipc/types"; const globalMountOptions = { plugins: [i18n] }; -function node(path: string, isDir: boolean, included: boolean, size = 0): ExclusionPreviewNode { - return { path, isDir, included, size }; +function node( + path: string, + isDir: boolean, + included: boolean, + size = 0, + fileCount = 0, + byteSize = 0 +): ExclusionPreviewNode { + return { path, isDir, included, size, fileCount, byteSize }; } function batch(nodes: ExclusionPreviewNode[], previewId = "gen-1"): ExclusionPreviewBatch { @@ -60,6 +67,7 @@ function batch(nodes: ExclusionPreviewNode[], previewId = "gen-1"): ExclusionPre includedCount: files.filter((n) => n.included).length, excludedCount: files.filter((n) => !n.included).length, includedBytes: files.filter((n) => n.included).reduce((a, n) => a + n.size, 0), + excludedBytes: files.filter((n) => !n.included).reduce((a, n) => a + n.size, 0), truncated: false, }; } @@ -166,6 +174,87 @@ describe("ExclusionPreviewTree", () => { expect(wrapper.find('[data-testid="preview-row-a.txt"]').exists()).toBe(true); }); + // Issue #305: per-folder rollups. + it("shows a folder row's file-count and byte rollup, right of the name", async () => { + const wrapper = await mountWithNodes([ + node("node_modules", true, false, 0, 31_204, 2_254_857_830), + ]); + const row = wrapper.get('[data-testid="preview-row-node_modules"]'); + expect(row.text()).toContain( + i18n.global.t("settings.exclusionPreview.rollup", { count: "31,204", size: "2.1 GB" }) + ); + const rollup = wrapper.get('[data-testid="preview-rollup-node_modules"]'); + expect(rollup.text()).toBe( + i18n.global.t("settings.exclusionPreview.rollup", { count: "31,204", size: "2.1 GB" }) + ); + }); + + it("settles a folder's rollup across later batches without duplicating the row", async () => { + const wrapper = await mountWithNodes([node("docs", true, true, 0, 0, 0)]); + expect(wrapper.get('[data-testid="preview-rollup-docs"]').text()).toContain("0 files"); + + batchHandler!(batch([node("docs", true, true, 0, 5, 500)])); + await settle(); + + expect(wrapper.findAll('[data-testid="preview-row-docs"]')).toHaveLength(1); + expect(wrapper.get('[data-testid="preview-rollup-docs"]').text()).toContain( + i18n.global.t("settings.exclusionPreview.rollup", { count: "5", size: "500 B" }) + ); + }); + + it("shows the would-be-freed total for excluded bytes in the summary line", async () => { + const wrapper = mount(ExclusionPreviewTree, { + global: globalMountOptions, + props: { + sourceId: "src-1", + respectGitignore: true, + includePatterns: [], + excludePatterns: [], + }, + }); + await flushPromises(); + batchHandler!({ + ...batch([node("skip.log", false, false, 1024)]), + excludedBytes: 1024, + }); + vi.advanceTimersByTime(20); + await flushPromises(); + + expect(wrapper.get('[data-testid="preview-excluded-bytes"]').text()).toBe( + i18n.global.t("settings.addSource.preview.excludedBytes", { size: "1 KB" }) + ); + }); + + it("flexes to fill its container when `fill` is set, instead of a fixed cap", async () => { + const capped = await mountWithNodes([node("a.txt", false, true)]); + expect(capped.get('[data-testid="exclusion-preview"]').classes()).not.toContain("flex-col"); + const cappedTree = capped.get('[role="tree"]').element.parentElement; + expect(cappedTree?.className).toContain("max-h-64"); + + const filled = mount(ExclusionPreviewTree, { + global: globalMountOptions, + props: { + sourceId: "src-1", + respectGitignore: true, + includePatterns: [], + excludePatterns: [], + fill: true, + }, + }); + await flushPromises(); + batchHandler!(batch([node("a.txt", false, true)])); + vi.advanceTimersByTime(20); + await flushPromises(); + + // The root becomes a flex column in fill mode... + expect(filled.get('[data-testid="exclusion-preview"]').classes()).toContain("flex-col"); + // ...and the old fixed cap on the scrollable tree body is gone, replaced + // by flex-fill sizing. + const filledTree = filled.get('[role="tree"]').element.parentElement; + expect(filledTree?.className).not.toContain("max-h-64"); + expect(filledTree?.className).toContain("flex-1"); + }); + it("renders streamed rows with live counts while the walk is in flight", async () => { const wrapper = await mountWithNodes([ node("docs", true, true), @@ -191,6 +280,7 @@ describe("ExclusionPreviewTree", () => { includedCount: 1, excludedCount: 0, includedBytes: 4, + excludedBytes: 0, truncated: false, cancelled: false, }); diff --git a/ui/src/__tests__/settings-components.test.ts b/ui/src/__tests__/settings-components.test.ts index a85fe758..d54f020b 100644 --- a/ui/src/__tests__/settings-components.test.ts +++ b/ui/src/__tests__/settings-components.test.ts @@ -1133,13 +1133,21 @@ describe("AddSourceWizard", () => { previewBatchHandler!({ previewId: "gen-1", nodes: [ - { path: "keep.txt", isDir: false, included: true, size: 4 }, - { path: "build", isDir: true, included: true, size: 0 }, - { path: "secret.env", isDir: false, included: false, size: 2 }, + { path: "keep.txt", isDir: false, included: true, size: 4, fileCount: 0, byteSize: 4 }, + { path: "build", isDir: true, included: true, size: 0, fileCount: 0, byteSize: 0 }, + { + path: "secret.env", + isDir: false, + included: false, + size: 2, + fileCount: 0, + byteSize: 2, + }, ], includedCount: 1, excludedCount: 1, includedBytes: 4, + excludedBytes: 2, truncated: false, }); await new Promise((r) => setTimeout(r, 25)); diff --git a/ui/src/__tests__/wizard-backend-destination.test.ts b/ui/src/__tests__/wizard-backend-destination.test.ts index 31505ad0..2c605c34 100644 --- a/ui/src/__tests__/wizard-backend-destination.test.ts +++ b/ui/src/__tests__/wizard-backend-destination.test.ts @@ -57,6 +57,7 @@ const BACKENDS: BackendDto[] = [ usesOauth: true, supportsFolderPicker: true, supportsVersionHistory: true, + supportsRename: true, isDefault: true, }, { @@ -64,6 +65,7 @@ const BACKENDS: BackendDto[] = [ usesOauth: false, supportsFolderPicker: true, supportsVersionHistory: false, + supportsRename: false, isDefault: false, }, { @@ -71,6 +73,7 @@ const BACKENDS: BackendDto[] = [ usesOauth: false, supportsFolderPicker: false, supportsVersionHistory: false, + supportsRename: false, isDefault: false, }, ]; diff --git a/ui/src/components/AddSourceWizard.vue b/ui/src/components/AddSourceWizard.vue index cf6c6d6d..a0972cf4 100644 --- a/ui/src/components/AddSourceWizard.vue +++ b/ui/src/components/AddSourceWizard.vue @@ -425,13 +425,29 @@ function onPhraseRevealError(code: unknown): void { revealErrorDetail.value = toErrorMessage(code); } +// Issue #306: near-fullscreen sizing for the two data-heavy steps (the +// destination-folder picker and the exclusions tree), which cramp badly into +// the wizard's normal compact width. Every OTHER step keeps today's +// max-w-lg card - a plain confirmation/toggle screen does not need the extra +// width, and widening it would just leave acres of empty space either side. +const isWideStep = computed(() => step.value === "driveFolder" || step.value === "exclusions"); +/** Whether the CURRENTLY-selected account's destination backend supports the + * picker's inline rename (issue #307; `BackendDto.supportsRename`). Defaults + * to false while `backends` is still loading, matching the fail-closed + * default the picker itself uses for an unrecognised prop. */ +const supportsRename = computed( + () => + backends.value.find((b) => b.id === selectedAccount.value?.backendKind)?.supportsRename ?? false +); + defineExpose({ start });