From d25a573e8f18b4041bf479c845017aa5b4605e5d Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Mon, 20 Jul 2026 00:09:00 +0200 Subject: [PATCH 1/3] fixed mount-kind correlation --- crates/pdfs-fuse/src/lib.rs | 87 ++++++++++++++++++++++++++++++++----- 1 file changed, 76 insertions(+), 11 deletions(-) diff --git a/crates/pdfs-fuse/src/lib.rs b/crates/pdfs-fuse/src/lib.rs index e7d4e48..b5a7b36 100644 --- a/crates/pdfs-fuse/src/lib.rs +++ b/crates/pdfs-fuse/src/lib.rs @@ -184,6 +184,11 @@ const XATTR_THUMBNAIL: &str = "user.proton.thumbnail"; /// Extended attribute exposing the larger server-side preview image of a file. const XATTR_PREVIEW: &str = "user.proton.preview"; +/// How many "this file has no thumbnail" answers [`Core::thumbnail`] remembers +/// before dropping the lot and re-learning them. Sized to cover a large browsing +/// session; each entry is a uid, a type tag and an mtime. +const MAX_THUMBNAIL_MISSES: usize = 8192; + /// How many files the bulk uploader ships at once. Overlaps the per-file network /// round-trips without letting an unbounded number of block buffers pile up. const UPLOAD_CONCURRENCY: usize = 4; @@ -449,6 +454,16 @@ struct Core { /// those downloads is a full-size photo — so an in-flight uid is never started /// twice. thumb_gen: Arc>>, + /// Nodes the remote has told us have *no* thumbnail of a given type, keyed by + /// `(uid, thumbnail type)` and holding the mtime the answer was learned at. + /// + /// Absence has to be cached or it costs a round trip every time it is asked + /// for, and it is asked for constantly: an `ls -l` from an xattr-aware lister + /// issues a `getxattr` per advertised name per entry, so a 65-file directory + /// of videos re-probed 130 times per listing at ~186 ms each (B5). The mtime + /// is the validity tag — a new revision may well have a thumbnail — matching + /// how [`ContentCache::read_thumbnail`] validates the positive side. + no_thumbnail: Arc>>, /// In-flight upload/download progress, served to `GetQueueStatus`. Shared /// across the FUSE session and the control-socket task. transfers: Arc, @@ -2010,6 +2025,12 @@ impl Core { if let Some(bytes) = self.cache.read_thumbnail(&uid, ttype.as_i32(), mtime) { return Ok(Some(bytes)); } + // "This file has no thumbnail" is an answer worth remembering: without + // it every listing pays a round trip per file to be told nothing (B5). + let key = (uid.clone(), ttype.as_i32()); + if self.no_thumbnail.lock().get(&key) == Some(&mtime) { + return Ok(None); + } let bytes = self .rt .block_on(self.client.download_thumbnail(&uid, ttype)) @@ -2017,10 +2038,23 @@ impl Core { warn!(%uid, error = %e, "download thumbnail failed"); Errno::EIO })?; - if let Some(bytes) = &bytes { - let _ = self - .cache - .store_thumbnail(&uid, ttype.as_i32(), mtime, bytes); + match &bytes { + Some(bytes) => { + let _ = self + .cache + .store_thumbnail(&uid, ttype.as_i32(), mtime, bytes); + } + None => { + let mut misses = self.no_thumbnail.lock(); + // Bounded by clearing rather than by LRU: the entries are two + // words each, the cap is far above any plausible working set, + // and re-learning a miss costs one round trip. Not worth a + // second data structure to order them. + if misses.len() >= MAX_THUMBNAIL_MISSES { + misses.clear(); + } + misses.insert(key, mtime); + } } Ok(bytes) } @@ -4118,6 +4152,25 @@ impl Filesystem for ProtonFs { /// a previewing client can fetch it without downloading the whole file. The /// bytes are fetched on demand and cached; absence yields `ENODATA`. fn getxattr(&self, _req: &Request, ino: INodeNo, name: &OsStr, size: u32, reply: ReplyXattr) { + // Always off the dispatch loop: a miss goes to the wire, and a lister + // that stats a directory issues one of these per file per advertised + // name — inline, that serialized the whole mount behind ~186 ms of + // network per call (B5). + let fs = self.clone(); + let name = name.to_os_string(); + self.core.workers.run(Lane::Meta, move || { + fs.serve_getxattr(ino, &name, size, reply) + }); + } + + fn listxattr(&self, _req: &Request, ino: INodeNo, size: u32, reply: ReplyXattr) { + self.serve_listxattr(ino, size, reply); + } +} + +impl ProtonFs { + /// The body of [`Filesystem::getxattr`], on a worker thread. + fn serve_getxattr(&self, ino: INodeNo, name: &OsStr, size: u32, reply: ReplyXattr) { let ttype = match name.to_str() { Some(XATTR_THUMBNAIL) => ThumbnailType::Thumbnail, Some(XATTR_PREVIEW) => ThumbnailType::Preview, @@ -4150,14 +4203,25 @@ impl Filesystem for ProtonFs { } } - /// Advertise the thumbnail/preview attribute names for regular files. The - /// names are listed unconditionally for files; a `getxattr` for one a given - /// file lacks returns `ENODATA`. - fn listxattr(&self, _req: &Request, ino: INodeNo, size: u32, reply: ReplyXattr) { - let is_file = { + /// Advertise the thumbnail/preview attribute names, but only for files whose + /// media type can actually carry one. + /// + /// Listing them for every file is what made `ls -l` expensive: an xattr-aware + /// lister asks for each advertised name, and each ask that misses is a network + /// round trip (B5). Proton only ever generates thumbnails for images and + /// videos, so advertising them on a `.mkv` or a `.pdf` is an invitation to do + /// work that can only end in `ENODATA`. `getxattr` still serves an explicit + /// request for an unadvertised name, so nothing becomes unreachable. + fn serve_listxattr(&self, ino: INodeNo, size: u32, reply: ReplyXattr) { + let thumbnailable = { let st = self.core.state.lock(); match st.entries.get(&ino.0) { - Some(e) => e.node.is_file(), + Some(e) => match &e.node.kind { + NodeKind::File { media_type, .. } => { + media_type.starts_with("image/") || media_type.starts_with("video/") + } + NodeKind::Folder => false, + }, None => { reply.error(Errno::ENOENT); return; @@ -4166,7 +4230,7 @@ impl Filesystem for ProtonFs { }; // xattr names are returned as a NUL-terminated, concatenated list. let mut buf = Vec::new(); - if is_file { + if thumbnailable { for name in [XATTR_THUMBNAIL, XATTR_PREVIEW] { buf.extend_from_slice(name.as_bytes()); buf.push(0); @@ -4578,6 +4642,7 @@ pub fn mount( timeline_refreshing: Arc::new(AtomicBool::new(false)), trash_refreshing: Arc::new(AtomicBool::new(false)), thumb_gen: Arc::new(Mutex::new(HashSet::new())), + no_thumbnail: Arc::new(Mutex::new(HashMap::new())), transfers: TransferRegistry::new(), indexing: Arc::new(AtomicBool::new(false)), sync_progress: Arc::new(Mutex::new(HashMap::new())), From f0fbd0775c6dac04b88fcad337760bffb82e1e32 Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Mon, 20 Jul 2026 13:37:59 +0200 Subject: [PATCH 2/3] fixed long getattr load times with ls -l --- README.md | 1 + crates/pdfs-core/src/db/migrations.rs | 31 ++- crates/pdfs-core/src/db/nodes.rs | 34 ++- crates/pdfs-core/src/db/tests.rs | 72 +++++ crates/pdfs-fuse/src/devices.rs | 19 +- crates/pdfs-fuse/src/lib.rs | 349 +++++++++++++++++++++++- crates/pdfs-gui/src/app/pages/verify.rs | 21 +- docs/ARCHITECTURE.md | 60 ++++ 8 files changed, 566 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index b61d40b..8db4e86 100644 --- a/README.md +++ b/README.md @@ -11,6 +11,7 @@ A fast, unofficial Proton Drive client for Linux. This client features an advanc - **Fuzzy Search Launcher Prompt (HUD)**: A Google Drive-style search launcher (`pdfs-prompt`) for finding and opening files or folders instantly, ideal for binding to a system-wide hotkey. - **Secure Credential Storage**: Integrates with the system Secret Service (GNOME Keyring, KWallet, etc.) with smart in-memory credential caching to avoid UI thread blockages. - **Proton Photos Support**: Access your Proton Photos timeline, view thumbnails, and download backed-up media natively (available in the GUI as a navigation tab and via the CLI). +- **Human Verification (CAPTCHA) Recovery**: Detects sign-in gates (VPN/new IP challenges) and launches an embedded `WebKitWebView` dialog to safely complete the challenge, transparently retrying authentication with the earned token. ## Performance & Caching diff --git a/crates/pdfs-core/src/db/migrations.rs b/crates/pdfs-core/src/db/migrations.rs index 96b8ff8..1b4dc18 100644 --- a/crates/pdfs-core/src/db/migrations.rs +++ b/crates/pdfs-core/src/db/migrations.rs @@ -9,7 +9,7 @@ use super::Db; use crate::Result; /// Current schema version. Bump on every forward migration added below. -pub(super) const SCHEMA_VERSION: i64 = 14; +pub(super) const SCHEMA_VERSION: i64 = 15; impl Db { pub(super) fn migrate(&self) -> Result<()> { @@ -78,6 +78,9 @@ impl Db { if current < 14 { tx.execute_batch(MIGRATION_V14)?; } + if current < 15 { + tx.execute_batch(MIGRATION_V15)?; + } tx.execute( "INSERT INTO sync_state (key, value) VALUES ('schema_version', ?1) ON CONFLICT(key) DO UPDATE SET value = excluded.value", @@ -340,3 +343,29 @@ ALTER TABLE photos ADD COLUMN kind INTEGER NOT NULL DEFAULT 0; const MIGRATION_V14: &str = " CREATE INDEX cache_entries_lru ON cache_entries(kind, last_accessed); "; + +/// Schema v15: key the search index by rowid so a row can be replaced without +/// scanning the whole index (B12). +/// +/// v3 built `nodes_fts` with the node's `uid` as an `UNINDEXED` column and +/// `upsert_nodes` refreshed a row with `DELETE FROM nodes_fts WHERE uid = ?`. +/// `UNINDEXED` means exactly what it says — the column is retrievable but not +/// searchable — so that predicate could only ever be a full scan of the index +/// (`EXPLAIN QUERY PLAN`: `SCAN nodes_fts VIRTUAL TABLE`). Every node written +/// scanned every node indexed: measured at 6.5 ms per node against a 17k-node +/// index, about half the cost of a cold listing, and *growing with the size of +/// the account* rather than the size of the listing. +/// +/// FTS5 deletes by `rowid` efficiently, and `nodes` is an ordinary rowid table +/// (`uid` is its `TEXT PRIMARY KEY`, not `WITHOUT ROWID`), so its rowid is a +/// stable per-node key that `ON CONFLICT DO UPDATE` preserves. Rebuild the index +/// keyed by it and drop the `uid` column: searches now join on `rowid`, which is +/// also a cheaper join than the old one on a TEXT uid. +const MIGRATION_V15: &str = " +DROP TABLE IF EXISTS nodes_fts; +CREATE VIRTUAL TABLE nodes_fts USING fts5( + name, tokenize='trigram' +); +INSERT INTO nodes_fts (rowid, name) + SELECT rowid, name FROM nodes WHERE trashed = 0; +"; diff --git a/crates/pdfs-core/src/db/nodes.rs b/crates/pdfs-core/src/db/nodes.rs index 119a3bd..d44d2e2 100644 --- a/crates/pdfs-core/src/db/nodes.rs +++ b/crates/pdfs-core/src/db/nodes.rs @@ -70,11 +70,22 @@ impl Db { // FTS5 has no UPSERT, so refresh the row by delete-then-insert. // Trashed nodes are kept out of the index entirely so they never // surface in search results. - tx.execute("DELETE FROM nodes_fts WHERE uid = ?1", params![uid])?; + // + // Keyed by the node's rowid, never by its uid: the uid is not an + // indexed FTS column, so deleting by it scans the entire index once + // per node written (B12). The rowid is stable across the upsert + // above — `ON CONFLICT DO UPDATE` keeps the existing row — so it is + // the same key before and after. + let rowid: i64 = tx.query_row( + "SELECT rowid FROM nodes WHERE uid = ?1", + params![uid], + |row| row.get(0), + )?; + tx.execute("DELETE FROM nodes_fts WHERE rowid = ?1", params![rowid])?; if !node.trashed { tx.execute( - "INSERT INTO nodes_fts (uid, name) VALUES (?1, ?2)", - params![uid, node.name], + "INSERT INTO nodes_fts (rowid, name) VALUES (?1, ?2)", + params![rowid, node.name], )?; } } @@ -88,8 +99,21 @@ impl Db { let uid = uid.to_string(); let mut conn = self.conn.lock(); let tx = conn.transaction()?; + // Read the rowid before dropping the node: it is the search index's key + // (B12), and it is gone once the row is. A node with no row leaves + // nothing to unindex — the index is keyed off `nodes`, so it cannot + // hold an entry the table never had. + let rowid: Option = tx + .query_row( + "SELECT rowid FROM nodes WHERE uid = ?1", + params![uid], + |row| row.get(0), + ) + .optional()?; tx.execute("DELETE FROM nodes WHERE uid = ?1", params![uid])?; - tx.execute("DELETE FROM nodes_fts WHERE uid = ?1", params![uid])?; + if let Some(rowid) = rowid { + tx.execute("DELETE FROM nodes_fts WHERE rowid = ?1", params![rowid])?; + } tx.commit()?; Ok(()) } @@ -123,7 +147,7 @@ impl Db { .join(" AND "); let mut stmt = conn.prepare( "SELECT n.node_json, n.uid - FROM nodes_fts f JOIN nodes n ON n.uid = f.uid + FROM nodes_fts f JOIN nodes n ON n.rowid = f.rowid WHERE f.name MATCH ?1 AND n.trashed = 0 AND n.node_json IS NOT NULL ORDER BY f.rank LIMIT ?2", )?; diff --git a/crates/pdfs-core/src/db/tests.rs b/crates/pdfs-core/src/db/tests.rs index 49f7f52..ae3ef3e 100644 --- a/crates/pdfs-core/src/db/tests.rs +++ b/crates/pdfs-core/src/db/tests.rs @@ -564,6 +564,78 @@ fn search_drops_fts_row_on_delete_and_trash() { assert_eq!(db.search("unique", 10).unwrap().len(), 0); } +/// Renaming re-indexes the node, which is the case delete-by-rowid has to get +/// right: the *old* name must leave the index. A stale row here would be +/// invisible in every other test — the new name is findable either way. +#[test] +fn renaming_a_node_replaces_the_name_in_the_search_index() { + let db = Db::open_in_memory().unwrap(); + db.upsert_node(&folder("root", None, "My Files")).unwrap(); + db.upsert_node(&file("f1", "root", "beforename.txt", 1)) + .unwrap(); + assert_eq!(db.search("beforename", 10).unwrap().len(), 1); + + db.upsert_node(&file("f1", "root", "aftername.txt", 1)) + .unwrap(); + + assert_eq!( + db.search("aftername", 10).unwrap().len(), + 1, + "the new name must be searchable" + ); + assert_eq!( + db.search("beforename", 10).unwrap().len(), + 0, + "the old name must have left the index, not merely been added alongside" + ); +} + +/// B12: the cost of writing a listing must not scale with how much is already +/// indexed. The old `DELETE FROM nodes_fts WHERE uid = ?` scanned the whole +/// index once per node written, so a large account paid more to write the *same* +/// folder — measured at 6.5 ms per node against a 17k-node index on the live DB. +/// +/// Written as a ratio between two index sizes rather than an absolute time, so +/// it fails on the scaling regression and not on a slow machine. +#[test] +fn writing_nodes_does_not_scale_with_the_size_of_the_index() { + use std::time::Instant; + + /// Fill `db` with `n` indexed nodes, then time re-writing a fixed batch. + fn cost_against_index_of(n: usize) -> std::time::Duration { + let db = Db::open_in_memory().unwrap(); + db.upsert_node(&folder("root", None, "My Files")).unwrap(); + let filler: Vec<_> = (0..n) + .map(|i| file(&format!("bg{i}"), "root", &format!("background{i}.txt"), 1)) + .collect(); + db.upsert_nodes(&filler).unwrap(); + + // The batch under test is identical in both runs; only the amount of + // already-indexed data around it differs. + let batch: Vec<_> = (0..200) + .map(|i| file(&format!("m{i}"), "root", &format!("measured{i}.txt"), 1)) + .collect(); + db.upsert_nodes(&batch).unwrap(); + + let t = Instant::now(); + db.upsert_nodes(&batch).unwrap(); + t.elapsed() + } + + let small = cost_against_index_of(200); + let large = cost_against_index_of(5000); + + // Calibration, measured while writing this: over the same 25× growth in + // index size, the old full-scan path cost 6.4× more (46 ms → 294 ms) while + // the rowid path costs 1.04× (39 ms → 41 ms). 3× sits well clear of both. + println!("B12: 200-node write — against 200 indexed {small:?}, against 5000 {large:?}"); + assert!( + large < small * 3, + "writing a listing got dramatically more expensive as the index grew, \ + which is the B12 full-scan regression; small={small:?} large={large:?}" + ); +} + fn activity(target: &str, kind: ActivityKind, ok: bool) -> ActivityEntry { ActivityEntry { time: 1700, diff --git a/crates/pdfs-fuse/src/devices.rs b/crates/pdfs-fuse/src/devices.rs index 33f42dc..7f02c73 100644 --- a/crates/pdfs-fuse/src/devices.rs +++ b/crates/pdfs-fuse/src/devices.rs @@ -20,6 +20,8 @@ use pdfs_core::db::{StoredDevice, StoredSyncFolder}; use pdfs_core::{CoreError, CoreResult}; use proton_drive_rs::proton_sdk::ids::{DeviceUid, NodeUid}; use proton_drive_rs::{DeviceType, Node}; +use std::collections::HashSet; +use std::sync::OnceLock; use tracing::{info, warn}; use super::sync::{self, base_name}; @@ -394,6 +396,12 @@ impl Core { next_fh: 1, db: self.db.clone(), })); + // A fresh inode space needs a fresh notification channel: this fork's + // session is the only one that knows these inodes, so it must be the one + // notified about them. Filled in by `spawn_ondemand_mount`. + fork.notifier = Arc::new(OnceLock::new()); + // Size upgrades are keyed by inode, which is per-fork too. + fork.size_upgrades = Arc::new(Mutex::new(HashSet::new())); fork } @@ -639,10 +647,15 @@ impl Core { MountOption::Subtype("protondrive".to_string()), MountOption::DefaultPermissions, ]; - let fs = ProtonFs::new(self.fork_state(), root); - Session::new(fs, local, &config) + let core = self.fork_state(); + let fs = ProtonFs::new(core.clone(), root); + let session = Session::new(fs, local, &config) .and_then(|s| s.spawn()) - .map_err(|e| CoreError::internal(format!("mount {}: {e}", local.display()))) + .map_err(|e| CoreError::internal(format!("mount {}: {e}", local.display())))?; + // Hand this fork its own session's notification channel, so a background + // size upgrade can drop the kernel's cached attrs for these inodes. + let _ = core.notifier.set(session.notifier()); + Ok(session) } /// Re-establish FUSE mounts for folders left in `ondemand` mode across a daemon diff --git a/crates/pdfs-fuse/src/lib.rs b/crates/pdfs-fuse/src/lib.rs index b5a7b36..f3f829e 100644 --- a/crates/pdfs-fuse/src/lib.rs +++ b/crates/pdfs-fuse/src/lib.rs @@ -39,6 +39,7 @@ use std::os::unix::fs::FileExt; use std::os::unix::net::UnixListener; use std::path::{Component, Path, PathBuf}; use std::sync::Arc; +use std::sync::OnceLock; use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; @@ -464,6 +465,19 @@ struct Core { /// is the validity tag — a new revision may well have a thumbnail — matching /// how [`ContentCache::read_thumbnail`] validates the positive side. no_thumbnail: Arc>>, + /// Folders whose listing was enumerated cheaply and is having its file sizes + /// filled in right now, so a burst of `stat`s over a fresh listing starts one + /// upgrade rather than one per entry. See [`Core::spawn_size_upgrade`]. + size_upgrades: Arc>>, + /// This mount's kernel notification channel, for telling the kernel to drop + /// metadata it has cached. Set once the session exists — which is *after* + /// the `Core` it is built from, hence the cell. + /// + /// Per mount, not per daemon: each on-demand fork runs its own session over + /// its own inode space, so notifying through the primary mount's channel + /// would name inodes that session has never heard of. [`Core::fork_state`] + /// gives each fork an empty cell of its own. + notifier: Arc>, /// In-flight upload/download progress, served to `GetQueueStatus`. Shared /// across the FUSE session and the control-socket task. transfers: Arc, @@ -777,6 +791,38 @@ impl Core { self.state.lock().children.contains_key(&ino) } + /// Re-apply the optimistic size of any queued write to `nodes`. + /// + /// A node that arrives from the remote (or from its DB row) carries the size + /// of the revision the *server* holds, which for a file with a write still + /// queued is the pre-write size — often 0 for a file created moments ago. + /// Interning it as-is silently reverts the optimistic size that + /// `record_pending_write` stamped, and a file that stats as 0 bytes is a file + /// the kernel will not issue a single `read` for: `cat` prints nothing and + /// the staged blob that `read_range` would have served is never asked for. + /// That reads as data loss even though nothing is lost (B11). + /// + /// [`Core::hydrate`] does the same thing for the restart case. This covers + /// every *live* re-enumeration — which is what a rename or move triggers, + /// since both invalidate the listings they touch. + /// + /// Snapshots the pending map and returns before any caller takes the state + /// lock: no site in the daemon holds `pending` and `state` at once, and this + /// is not the place to become the first. + fn stamp_pending_sizes(&self, nodes: &mut [Node]) { + let sizes: HashMap = { + let pending = self.pending.lock(); + if pending.is_empty() { + return; + } + pending + .iter() + .map(|(uid, pr)| (uid.clone(), pr.meta.len)) + .collect() + }; + apply_pending_sizes(nodes, &sizes); + } + /// Enumerate `ino`'s children from the remote and cache them. No-op if the /// directory has already been listed. Network I/O happens without the lock /// held so concurrent metadata reads aren't blocked behind a fetch. @@ -796,19 +842,36 @@ impl Core { // can be rebuilt from disk without hitting the API, even if its listing // was trimmed from the hot cache mid-run. match self.db.children_if_listed(&folder_uid) { - Ok(Some(nodes)) => { + Ok(Some(mut nodes)) => { + // Before the lock: a DB row carries the size the server last + // sealed, which a queued write is ahead of (B11). + self.stamp_pending_sizes(&mut nodes); let mut st = self.state.lock(); if st.children.contains_key(&ino) { return Ok(()); } let mut child_inos = Vec::with_capacity(nodes.len()); + let mut needs_size = Vec::new(); for node in nodes { if node.trashed || node.uid == folder_uid { continue; } + if matches!( + &node.kind, + NodeKind::File { + claimed_size: None, + .. + } + ) { + needs_size.push(node.uid.clone()); + } child_inos.push(st.intern_from_db(ino, node)); } st.children.insert(ino, child_inos); + drop(st); + // Rows persisted from a cheap enumeration whose upgrade never + // ran (a restart in between, say) still owe their real sizes. + self.spawn_size_upgrade(ino, needs_size); return Ok(()); } Ok(None) => {} @@ -822,13 +885,25 @@ impl Core { error!(%folder_uid, error = %e, "enumerate folder children failed"); Errno::EIO })?; - let nodes = self + // Cheap enumeration: `Light` skips unlocking each *file's* node key, + // which is an S2K derivation per file and was ~74% of the cost of a cold + // listing (B12 — measured with `perf`, 64% of cycles in SHA-256 alone). + // Folders are unlocked either way; their keys are what the children are + // decrypted with, so the walk cannot proceed without them. + // + // The price is that files come back without a `claimed_size`, so + // `node_size` falls back to the *ciphertext* size until + // `spawn_size_upgrade` below fills the real one in. + let mut nodes = self .rt - .block_on(self.client.enumerate_nodes(&uids)) + .block_on(self.client.enumerate_nodes_light(&uids)) .map_err(|e| { error!(%folder_uid, error = %e, "enumerate nodes failed"); Errno::EIO })?; + // Same as the DB path above: the remote's size for a file with a write + // still queued is the pre-write one (B11). + self.stamp_pending_sizes(&mut nodes); let mut st = self.state.lock(); // Lost the race? Another thread already populated it. @@ -840,6 +915,21 @@ impl Core { .into_iter() .filter(|node| !node.trashed && node.uid != folder_uid) .collect(); + // Files whose real size the cheap enumeration could not read. Collected + // before interning so the upgrade below has the uids without re-walking. + let needs_size: Vec = filtered_nodes + .iter() + .filter(|n| { + matches!( + &n.kind, + NodeKind::File { + claimed_size: None, + .. + } + ) + }) + .map(|n| n.uid.clone()) + .collect(); let inos = st.intern_batch(ino, filtered_nodes); child_inos.extend(inos); st.children.insert(ino, child_inos); @@ -848,9 +938,132 @@ impl Core { if let Err(e) = self.db.set_listed(&folder_uid, true) { warn!(%folder_uid, error = %e, "db set_listed(true) failed"); } + drop(st); + self.spawn_size_upgrade(ino, needs_size); Ok(()) } + /// Kick a size upgrade for every file in `parent`'s listing still carrying a + /// provisional size. + /// + /// Called when a `stat` lands on such a file. That covers the paths + /// [`Core::ensure_children`] cannot: a listing rebuilt from the DB, or one + /// restored by [`Core::hydrate`] on mount, whose rows were persisted before + /// an earlier upgrade had a chance to run. Gathers the whole folder rather + /// than the one file asked for, because a `stat` of one entry in a listing + /// almost always means a `stat` of all of them. + fn upgrade_sizes_for_parent(&self, parent: u64) { + let missing: Vec = { + let st = self.state.lock(); + let Some(kids) = st.children.get(&parent) else { + return; + }; + kids.iter() + .filter_map(|k| st.entries.get(k)) + .filter(|e| { + matches!( + &e.node.kind, + NodeKind::File { + claimed_size: None, + .. + } + ) + }) + .map(|e| e.uid.clone()) + .collect() + }; + self.spawn_size_upgrade(parent, missing); + } + + /// Fill in the true sizes of files a `Light` enumeration returned without + /// one, on a worker, after the listing has already been served. + /// + /// This is the other half of the split in [`Core::ensure_children`]. The + /// listing itself needs only names and parentage, so it is served from the + /// cheap enumeration immediately; the S2K-per-file work that produces + /// `claimed_size` happens here, off the path the user is waiting on. + /// + /// **Sizes are provisional until this lands.** `node_size` falls back to + /// `total_size_on_storage`, the *ciphertext* size, which is slightly larger + /// than the real one. Reads are unaffected — the revision reader carries its + /// own authoritative size — so this is a cosmetic discrepancy in `stat` that + /// closes within a round trip, not a repeat of B11 (which reported **0** and + /// so suppressed reads entirely). + /// + /// Single-flight per folder: a `stat` of every entry in a fresh listing is + /// the normal case, and each one must not start its own upgrade. + fn spawn_size_upgrade(&self, folder_ino: u64, uids: Vec) { + if uids.is_empty() { + return; + } + if !self.size_upgrades.lock().insert(folder_ino) { + return; + } + let core = self.clone(); + self.workers.run(Lane::Meta, move || { + let result = core.rt.block_on(core.client.enumerate_nodes(&uids)); + core.size_upgrades.lock().remove(&folder_ino); + let mut nodes = match result { + Ok(nodes) => nodes, + Err(e) => { + // Not fatal: the listing stands, sizes just stay provisional + // until something invalidates and re-enumerates it. + warn!(folder_ino, error = %e, "size upgrade failed; sizes stay provisional"); + return; + } + }; + // A queued write is ahead of anything the server can report, so its + // optimistic size must survive this just as it survives a re-listing + // (B11). + core.stamp_pending_sizes(&mut nodes); + let mut changed: Vec = Vec::new(); + let mut st = core.state.lock(); + for node in nodes { + // Only adopt the size. Re-interning wholesale would also adopt a + // name or parent that a rename/move may have changed locally + // while this was in flight, undoing it. + let Some(&ino) = st.by_uid.get(&node.uid) else { + continue; + }; + changed.push(ino); + let NodeKind::File { claimed_size, .. } = &node.kind else { + continue; + }; + let (Some(size), Some(entry)) = (*claimed_size, st.entries.get_mut(&ino)) else { + continue; + }; + if let NodeKind::File { claimed_size, .. } = &mut entry.node.kind { + *claimed_size = Some(size); + } + } + let updated: Vec = st + .children + .get(&folder_ino) + .map(|kids| { + kids.iter() + .filter_map(|k| st.entries.get(k).map(|e| e.node.clone())) + .collect() + }) + .unwrap_or_default(); + drop(st); + if let Err(e) = core.db.upsert_nodes(&updated) { + warn!(folder_ino, error = %e, "persisting upgraded sizes failed"); + } + // Without this the corrected size is invisible for the length of the + // attr TTL: the kernel answers `stat` from the provisional attrs it + // cached while the listing was being served, so `ls -l` reports the + // ciphertext size for up to 30 s even though the daemon has had the + // real one all along. Notify *after* the DB write, so a re-`getattr` + // provoked by the invalidation cannot race the persistence. + if let Some(notifier) = core.notifier.get() { + for ino in changed { + let _ = notifier.inval_inode(INodeNo(ino), 0, 0); + } + } + debug!(folder_ino, files = updated.len(), "filled in listing sizes"); + }); + } + /// Resolve a child `name` within `parent` to its `(inode, uid)`, ensuring /// the parent's listing is cached first. fn lookup_child(&self, parent: u64, name: &str) -> Result<(u64, NodeUid), Errno> { @@ -3251,6 +3464,19 @@ fn media_type_for(name: &str) -> &'static str { } /// The plaintext size, in bytes, that a node reports. +/// Overwrite each file node's `claimed_size` with the optimistic size of its +/// queued write, where it has one. Folders and nodes with nothing queued are +/// left alone. See [`Core::stamp_pending_sizes`] for why this exists. +fn apply_pending_sizes(nodes: &mut [Node], sizes: &HashMap) { + for node in nodes { + if let Some(&len) = sizes.get(&node.uid) + && let NodeKind::File { claimed_size, .. } = &mut node.kind + { + *claimed_size = Some(len as i64); + } + } +} + fn node_size(node: &Node) -> u64 { match &node.kind { NodeKind::Folder => 0, @@ -3392,13 +3618,32 @@ impl Filesystem for ProtonFs { } fn getattr(&self, _req: &Request, ino: INodeNo, _fh: Option, reply: ReplyAttr) { - let st = self.core.state.lock(); - match st.entries.get(&ino.0) { - Some(e) => { - let attr = self.attr(ino.0, &e.node); - reply.attr(&TTL, &attr); + let (attr, provisional_in) = { + let st = self.core.state.lock(); + match st.entries.get(&ino.0) { + Some(e) => { + // A file still carrying the cheap enumeration's placeholder + // size: answer now with what we have and have the real size + // fetched in the background (B12). + let provisional = matches!( + &e.node.kind, + NodeKind::File { + claimed_size: None, + .. + } + ) + .then_some(e.parent); + (self.attr(ino.0, &e.node), provisional) + } + None => { + reply.error(Errno::ENOENT); + return; + } } - None => reply.error(Errno::ENOENT), + }; + reply.attr(&TTL, &attr); + if let Some(parent) = provisional_in { + self.core.upgrade_sizes_for_parent(parent); } } @@ -4643,6 +4888,8 @@ pub fn mount( trash_refreshing: Arc::new(AtomicBool::new(false)), thumb_gen: Arc::new(Mutex::new(HashSet::new())), no_thumbnail: Arc::new(Mutex::new(HashMap::new())), + size_upgrades: Arc::new(Mutex::new(HashSet::new())), + notifier: Arc::new(OnceLock::new()), transfers: TransferRegistry::new(), indexing: Arc::new(AtomicBool::new(false)), sync_progress: Arc::new(Mutex::new(HashMap::new())), @@ -4744,6 +4991,9 @@ pub fn mount( // thread; we then wait here for either a stop signal or the mount ending. let bg = Session::new(fs, mountpoint, &config)?.spawn()?; let notifier = bg.notifier(); + // Same channel, kept on the `Core` so background work (a size upgrade, say) + // can invalidate kernel-cached metadata without threading a handle through. + let _ = core.notifier.set(notifier.clone()); rt.spawn(run_event_sync( client, scope, @@ -4951,6 +5201,87 @@ mod local_uid_tests { } } +#[cfg(test)] +mod pending_size_tests { + use super::{Node, NodeKind, NodeUid, apply_pending_sizes, node_size}; + use proton_drive_rs::proton_sdk::ids::{LinkId, VolumeId}; + use std::collections::HashMap; + + fn uid(link: &str) -> NodeUid { + NodeUid::new(VolumeId::from("vol"), LinkId::from(link)) + } + + fn file(link: &str, claimed: i64) -> Node { + Node { + uid: uid(link), + parent_uid: Some(uid("parent")), + kind: NodeKind::File { + media_type: "text/plain".into(), + total_size_on_storage: 0, + active_revision_state: None, + claimed_size: Some(claimed), + claimed_modification_time: None, + }, + name: link.into(), + creation_time: 100, + modification_time: 100, + trashed: false, + is_shared: false, + is_shared_publicly: false, + signature_email: None, + verification: Default::default(), + } + } + + fn folder(link: &str) -> Node { + Node { + kind: NodeKind::Folder, + ..file(link, 0) + } + } + + /// B11: a re-enumeration mid-write must not revert the size to the + /// server's. A file that stats as 0 gets no `read` from the kernel at all, + /// so the staged blob is never served and the file reads as empty. + #[test] + fn a_queued_write_keeps_its_optimistic_size_through_a_re_enumeration() { + let mut nodes = vec![file("queued", 0), file("settled", 4096)]; + let sizes = HashMap::from([(uid("queued"), 3)]); + + apply_pending_sizes(&mut nodes, &sizes); + + assert_eq!( + node_size(&nodes[0]), + 3, + "the remote's pre-write size must not win over the queued write's" + ); + assert_eq!( + node_size(&nodes[1]), + 4096, + "a file with nothing queued keeps the size the server reported" + ); + } + + /// The map is keyed by uid and says nothing about kind; a folder that + /// somehow collides must not grow a `claimed_size`. + #[test] + fn folders_are_left_alone() { + let mut nodes = vec![folder("dir")]; + apply_pending_sizes(&mut nodes, &HashMap::from([(uid("dir"), 999)])); + assert!(matches!(nodes[0].kind, NodeKind::Folder)); + assert_eq!(node_size(&nodes[0]), 0); + } + + /// The common case: nothing queued, nothing touched. + #[test] + fn an_empty_pending_map_changes_nothing() { + let mut nodes = vec![file("a", 10), file("b", 20)]; + apply_pending_sizes(&mut nodes, &HashMap::new()); + assert_eq!(node_size(&nodes[0]), 10); + assert_eq!(node_size(&nodes[1]), 20); + } +} + #[cfg(test)] mod tests { use super::{Intervals, conflict_name}; diff --git a/crates/pdfs-gui/src/app/pages/verify.rs b/crates/pdfs-gui/src/app/pages/verify.rs index 9ed6469..8b9aa18 100644 --- a/crates/pdfs-gui/src/app/pages/verify.rs +++ b/crates/pdfs-gui/src/app/pages/verify.rs @@ -70,7 +70,7 @@ pub(crate) fn prompt_human_verification( let token_tx = Rc::new(RefCell::new(Some(token_tx))); let tx = token_tx.clone(); - let dlg = dialog.clone(); + let dlg_weak = dialog.downgrade(); content.connect_script_message_received(Some("hv"), move |_, value| { let Some(token) = extract_token(&value.to_str()) else { // Every `postMessage` on the page reaches here, most of them the @@ -81,7 +81,9 @@ pub(crate) fn prompt_human_verification( if let Some(tx) = tx.borrow_mut().take() { let _ = tx.send(token); } - dlg.close(); + if let Some(dlg) = dlg_weak.upgrade() { + dlg.close(); + } }); // Covers the close button, Escape, and the user giving up. @@ -104,7 +106,14 @@ pub(crate) fn prompt_human_verification( /// unrecognised shape as success would hand the API an empty token and fail the /// login with a confusing error. fn extract_token(raw: &str) -> Option { - let value: serde_json::Value = serde_json::from_str(raw).ok()?; + let mut value: serde_json::Value = serde_json::from_str(raw).ok()?; + // If the message was stringified twice (e.g. raw is a double-quoted JSON string), + // parse the inner string. + if let Some(inner) = value.as_str() { + if let Ok(parsed) = serde_json::from_str(inner) { + value = parsed; + } + } // The payload is nested under `payload` and the message names itself in // `type`; both spellings of the success type have shipped. let kind = value.get("type")?.as_str()?; @@ -133,6 +142,12 @@ mod tests { assert_eq!(extract_token(raw).as_deref(), Some("tok-1")); } + #[test] + fn a_double_serialized_completion_message_yields_its_token() { + let raw = r#""{\"type\":\"HUMAN_VERIFICATION_SUCCESS\",\"payload\":{\"token\":\"tok-1\",\"type\":\"captcha\"}}""#; + assert_eq!(extract_token(raw).as_deref(), Some("tok-1")); + } + /// The page posts plenty that is not a completion; none of it may be /// mistaken for one, or the login retries with a garbage token. #[test] diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index a15c848..a183680 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -284,3 +284,63 @@ See [RECOVERY.md](RECOVERY.md) for what a lost machine means for the plaintext d - Treat the cache and state directories as being as sensitive as the Drive contents themselves. - Multi-user machines warrant checking `~/.cache` and `~/.local/state` are `0700` until §8.5 is addressed. - Purging the cache (`pdfs` settings, or `PurgeCache` over IPC) removes content but **not** `cache.db`, and deliberately never removes undrained `staging/` or `recovery/` blobs. + +--- + +## 9. Human Verification (CAPTCHA) Flow + +When logging in from an unfamiliar IP address or VPN, the Proton API may gate the sign-in with a human verification challenge (CAPTCHA). This client handles this asynchronously and interactively. + +### 9.1 Sequence of Verification and Re-Authentication + +```mermaid +sequenceDiagram + autonumber + actor User as User (GUI) + participant Core as pdfs-core (Auth) + participant UI as pdfs-gui (Login Page) + participant Web as WebKitWebView Dialog + participant API as Proton API Server + + User->>UI: Enter credentials & click Sign In + UI->>Core: login_interactive() + Core->>API: auth/v4 (Initial SRP Handshake) + API-->>Core: HTTP 422 (Error 9001: CAPTCHA Challenge URL) + Core->>UI: Error::HumanVerificationRequired(hv) + UI->>UI: Block login thread, dispatch to GTK main loop + UI->>Web: Create dialog & load verification URL + Note over User, Web: User completes CAPTCHA in embedded WebView + Web->>UI: window.postMessage(HUMAN_VERIFICATION_SUCCESS) + UI->>UI: Extract token (with double-serialization safety) + UI->>UI: Close dialog & send token to blocked login thread + UI->>Core: login_verified(with verification token) + Core->>API: auth/v4 (SRP retry with x-pm-human-verification-token) + API-->>Core: Returns session tokens + Core-->>UI: Login Successful +``` + +### 9.2 Key Technical Design Decisions + +1. **Weak Reference UI Binding:** To prevent memory leaks and strong reference cycles between the parent dialog, the child `WebKitWebView`, the script message manager, and the connection callback, the dialog is downgraded to a `WeakRef` inside the callback: + ```rust + let dlg_weak = dialog.downgrade(); + content.connect_script_message_received(Some("hv"), move |_, value| { + // ... + if let Some(dlg) = dlg_weak.upgrade() { + dlg.close(); + } + }); + ``` +2. **Double-Serialization Tolerance:** The JavaScript message listener forwards event data as a JSON string to the native handler. Since the underlying page may post either JS objects or pre-serialized JSON strings, the Rust side performs dual-phase parsing: + ```rust + let mut value = serde_json::from_str(raw).ok()?; + if let Some(inner) = value.as_str() { + if let Ok(parsed) = serde_json::from_str(inner) { + value = parsed; + } + } + ``` + This ensures compatibility with all versions of Proton's client verification scripts. +3. **SRP Handshake Reset:** Because a gated login burns the SRP handshake on the API side, the client cannot simply resume the previous request. Instead, `auth::login_interactive` restarts the SRP process from scratch with the verification credentials attached, keeping the complex handshake details isolated from the front-end. +4. **CLI Fallback:** Since the CLI has no native web browser engine, hitting the CAPTCHA gate fails immediately with a user-friendly message directing the user to sign in once via the GUI (`pdfs-app`) to persist the authenticated session keys to the system keyring. + From 1e0e284d5f69cdc4640e476a752f96f3212731e2 Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Mon, 20 Jul 2026 14:52:31 +0200 Subject: [PATCH 3/3] per-chunk wakeup, B13 + B14 --- crates/pdfs-fuse/src/devices.rs | 3 +- crates/pdfs-fuse/src/lib.rs | 708 +++++++++++++++++-- docs/bugs.md | 1140 +++++++++++++++++++++++++++++++ 3 files changed, 1793 insertions(+), 58 deletions(-) create mode 100644 docs/bugs.md diff --git a/crates/pdfs-fuse/src/devices.rs b/crates/pdfs-fuse/src/devices.rs index 7f02c73..0753ae5 100644 --- a/crates/pdfs-fuse/src/devices.rs +++ b/crates/pdfs-fuse/src/devices.rs @@ -20,7 +20,6 @@ use pdfs_core::db::{StoredDevice, StoredSyncFolder}; use pdfs_core::{CoreError, CoreResult}; use proton_drive_rs::proton_sdk::ids::{DeviceUid, NodeUid}; use proton_drive_rs::{DeviceType, Node}; -use std::collections::HashSet; use std::sync::OnceLock; use tracing::{info, warn}; @@ -401,7 +400,7 @@ impl Core { // notified about them. Filled in by `spawn_ondemand_mount`. fork.notifier = Arc::new(OnceLock::new()); // Size upgrades are keyed by inode, which is per-fork too. - fork.size_upgrades = Arc::new(Mutex::new(HashSet::new())); + fork.size_upgrades = Arc::new(Mutex::new(HashMap::new())); fork } diff --git a/crates/pdfs-fuse/src/lib.rs b/crates/pdfs-fuse/src/lib.rs index f3f829e..6b054df 100644 --- a/crates/pdfs-fuse/src/lib.rs +++ b/crates/pdfs-fuse/src/lib.rs @@ -190,6 +190,13 @@ const XATTR_PREVIEW: &str = "user.proton.preview"; /// session; each entry is a uid, a type tag and an mtime. const MAX_THUMBNAIL_MISSES: usize = 8192; +/// How many nodes one size-upgrade request covers. +/// +/// Matches the SDK's own `MAX_BATCH_COUNT`, so a chunk is exactly one request: +/// chunking smaller would add round trips, larger would be split anyway and +/// delay the waiters this chunking exists to release (bugs.md B14). +const SIZE_UPGRADE_CHUNK: usize = 150; + /// How many files the bulk uploader ships at once. Overlaps the per-file network /// round-trips without letting an unbounded number of block buffers pile up. const UPLOAD_CONCURRENCY: usize = 4; @@ -468,7 +475,11 @@ struct Core { /// Folders whose listing was enumerated cheaply and is having its file sizes /// filled in right now, so a burst of `stat`s over a fresh listing starts one /// upgrade rather than one per entry. See [`Core::spawn_size_upgrade`]. - size_upgrades: Arc>>, + /// Size upgrades currently running, per folder inode. A `getattr` that + /// needs a real size waits on the entry rather than issuing its own fetch — + /// `ls -l` of a folder is one `getattr` per file, and they must collapse + /// onto a single batch (bugs.md B14). + size_upgrades: Arc>>>, /// This mount's kernel notification channel, for telling the kernel to drop /// metadata it has cached. Set once the session exists — which is *after* /// the `Core` it is built from, hence the cell. @@ -943,8 +954,8 @@ impl Core { Ok(()) } - /// Kick a size upgrade for every file in `parent`'s listing still carrying a - /// provisional size. + /// Resolve the real size of every file in `parent`'s listing still carrying + /// a provisional one, returning when they are in `state`. /// /// Called when a `stat` lands on such a file. That covers the paths /// [`Core::ensure_children`] cannot: a listing rebuilt from the DB, or one @@ -952,27 +963,47 @@ impl Core { /// an earlier upgrade had a chance to run. Gathers the whole folder rather /// than the one file asked for, because a `stat` of one entry in a listing /// almost always means a `stat` of all of them. - fn upgrade_sizes_for_parent(&self, parent: u64) { - let missing: Vec = { - let st = self.state.lock(); - let Some(kids) = st.children.get(&parent) else { - return; - }; - kids.iter() - .filter_map(|k| st.entries.get(k)) - .filter(|e| { - matches!( - &e.node.kind, - NodeKind::File { - claimed_size: None, - .. - } - ) - }) - .map(|e| e.uid.clone()) - .collect() + fn upgrade_sizes_for_parent(&self, ino: u64, uid: &NodeUid, parent: u64) { + let provisional = |e: &Entry| { + matches!( + &e.node.kind, + NodeKind::File { + claimed_size: None, + .. + } + ) }; - self.spawn_size_upgrade(parent, missing); + let (key, mut missing): (u64, Vec) = { + let st = self.state.lock(); + match st.children.get(&parent) { + // The listing is resident: batch the whole folder under its + // inode, so the rest of an `ls -l` rides along on this fetch. + Some(kids) => ( + parent, + kids.iter() + .filter_map(|k| st.entries.get(k)) + .filter(|e| provisional(e)) + .map(|e| e.uid.clone()) + .collect(), + ), + // It is not, and returning here is what let a provisional size + // reach the caller anyway: a rename invalidates its parents' + // listings, so a freshly renamed file always landed in this + // branch. Resolve the single node instead, keyed by its own + // inode — folder and file inodes share one space, so the two + // single-flight keys cannot collide. (Same shape as B4: an + // early return that assumed the hot cache was authoritative.) + None => (ino, Vec::new()), + } + }; + // The node that was actually asked about is never optional, whichever + // branch produced the batch. + if !missing.iter().any(|u| u == uid) { + missing.push(uid.clone()); + } + // Blocking, not spawning: the caller is a `getattr` that must not answer + // with a provisional size (bugs.md B14). + self.upgrade_sizes(key, missing, Some(ino)); } /// Fill in the true sizes of files a `Light` enumeration returned without @@ -993,16 +1024,86 @@ impl Core { /// Single-flight per folder: a `stat` of every entry in a fresh listing is /// the normal case, and each one must not start its own upgrade. fn spawn_size_upgrade(&self, folder_ino: u64, uids: Vec) { + self.upgrade_sizes(folder_ino, uids, None); + } + + /// Start (or join) the size upgrade for `key`, and — when `waiting_for` names + /// an inode — block until that node's real size has landed. + /// + /// Single-flight per `key`. The fetch runs on its own thread rather than on + /// the [`Workers`] pool: callers wait on `Lane::Meta`, so a batch queued onto + /// that lane could have a wide enough `ls -l` fill it with threads waiting + /// for a job that can never be scheduled. `Lane::Transfer` would swap that + /// for starvation behind bulk reads. One short-lived thread per folder, + /// bounded by the single-flight, avoids both. + fn upgrade_sizes(&self, key: u64, uids: Vec, waiting_for: Option) { if uids.is_empty() { return; } - if !self.size_upgrades.lock().insert(folder_ino) { + let slot = { + let mut in_flight = self.size_upgrades.lock(); + match in_flight.get(&key) { + // Someone else is already fetching this folder; their batch + // covers us, so just wait on it. + Some(existing) => existing.clone(), + None => { + let slot = Arc::new(SizeUpgrade::default()); + in_flight.insert(key, slot.clone()); + let core = self.clone(); + let worker = slot.clone(); + std::thread::spawn(move || { + core.run_size_upgrade(key, uids, &worker); + }); + slot + } + } + }; + let Some(ino) = waiting_for else { return; + }; + slot.wait_for(|| self.size_is_real(ino)); + } + + /// Fetch `uids` in chunks, applying and announcing each one as it lands. + /// + /// Chunked so a waiter is released as soon as *its* file is resolved. A + /// single 793-node batch took ~80 s, which outran [`SizeUpgrade::WAIT`] and + /// put provisional sizes back in front of callers — the bug this was + /// supposed to fix (bugs.md B14). + fn run_size_upgrade(&self, key: u64, uids: Vec, slot: &SizeUpgrade) { + for chunk in uids.chunks(SIZE_UPGRADE_CHUNK) { + let result = self.rt.block_on(self.client.enumerate_nodes(chunk)); + self.apply_size_upgrade(key, result); + slot.chunk_done(); } - let core = self.clone(); - self.workers.run(Lane::Meta, move || { - let result = core.rt.block_on(core.client.enumerate_nodes(&uids)); - core.size_upgrades.lock().remove(&folder_ino); + // Both of these must happen however the loop ended: a folder whose + // upgrade failed has to be retryable, and its waiters released. + self.size_upgrades.lock().remove(&key); + slot.finish(); + } + + /// Whether `ino` has a real size — the condition a waiter is waiting on. + /// A node that vanished counts as resolved; there is nothing left to wait + /// for, and its caller will find the `ENOENT` for itself. + fn size_is_real(&self, ino: u64) -> bool { + let st = self.state.lock(); + st.entries.get(&ino).is_none_or(|e| { + !matches!( + &e.node.kind, + NodeKind::File { + claimed_size: None, + .. + } + ) + }) + } + + /// Adopt the sizes a completed upgrade fetched. Split from + /// [`Core::upgrade_sizes`] so the single-flight bookkeeping there has one + /// exit path rather than one per early return. + fn apply_size_upgrade(&self, folder_ino: u64, result: Result, ProtonError>) { + let core = self; + { let mut nodes = match result { Ok(nodes) => nodes, Err(e) => { @@ -1061,7 +1162,7 @@ impl Core { } } debug!(folder_ino, files = updated.len(), "filled in listing sizes"); - }); + } } /// Resolve a child `name` within `parent` to its `(inode, uid)`, ensuring @@ -2520,6 +2621,79 @@ impl Core { Ok(name) } + /// Remove the node a `rename` is about to replace, so the new name is free + /// for the API call that follows. + /// + /// `rename(2)` promises to replace an existing destination atomically. Proton + /// offers no such primitive — `rename_node` refuses a name that is already + /// taken — so this is the first half of an emulation that is *not* atomic: + /// see [`Core::restore_replaced`] for the other half (bugs.md B13). + /// + /// A node whose own creation is still queued has never reached the server, so + /// dropping its queued ops is the whole removal; nothing goes to the wire and + /// it works offline. + fn remove_replaced(&self, uid: &NodeUid, name: &str) -> Result<(), Errno> { + if is_local_uid(uid) { + self.discard_queued_ops(uid); + self.state.lock().forget(uid); + debug!(%uid, name, "replaced a node whose create was still queued"); + return Ok(()); + } + if let Err(e) = self + .rt + .block_on(self.client.trash_nodes(std::slice::from_ref(uid))) + { + error!(%uid, name, error = %e, "trashing the node a rename replaces failed"); + self.log_activity(ActivityKind::Trash, name, e.to_string(), false); + return Err(Errno::EIO); + } + self.discard_queued_ops(uid); + self.state.lock().forget(uid); + self.cache.evict(uid); + self.evict_reader(uid); + self.invalidate_trash(); + // The node is recoverable from the trash, but only if the user knows it + // went there — a rename is not an operation anyone expects to trash + // something, so this is the only record that it happened. + self.log_activity( + ActivityKind::Trash, + name, + "replaced by a rename from the mount", + true, + ); + Ok(()) + } + + /// Put back the node [`Core::remove_replaced`] trashed, after the rename it + /// was clearing the way for failed anyway. + /// + /// Best-effort by construction: if the restore also fails there is nothing + /// further to try, and the node is still in the trash where `pdfs restore` + /// can reach it. Says so loudly in that case, because the alternative is a + /// file the user believes was only renamed quietly sitting in the trash. + fn restore_replaced(&self, victim: Option<&(u64, NodeUid)>, name: &str) { + let Some((_, uid)) = victim else { return }; + if is_local_uid(uid) { + // Its queued create was discarded and cannot be reconstructed from + // here; the caller's error is what the user acts on. + warn!(%uid, name, "a rename failed after discarding a queued node it replaced"); + return; + } + match self + .rt + .block_on(self.client.restore_nodes(std::slice::from_ref(uid))) + { + Ok(()) => { + self.invalidate_trash(); + debug!(%uid, name, "restored the node a failed rename had replaced"); + } + Err(e) => { + error!(%uid, name, error = %e, "restoring a replaced node failed; it stays in the trash"); + self.log_activity(ActivityKind::Restore, name, e.to_string(), false); + } + } + } + /// List the account's trash, from the DB. Trashed nodes are outside the /// mounted tree — the FUSE side forgot them when they were trashed — so the /// listing is persisted in its own table rather than derived from `State`, and @@ -3131,27 +3305,67 @@ impl ProtonFs { } /// The body of [`Filesystem::lookup`], on whichever thread ends up serving it. - fn serve_lookup(&self, parent: u64, name: &str, reply: ReplyEntry) { + /// `off_loop` says whether the caller is already on a worker, and so whether + /// this may block. `lookup` answers a cached parent on the dispatch loop, + /// where it may not. + fn serve_lookup(&self, parent: u64, name: &str, reply: ReplyEntry, off_loop: bool) { if let Err(e) = self.core.ensure_children(parent) { reply.error(e); return; } - let st = self.core.state.lock(); - let hit = st.children.get(&parent).and_then(|kids| { - kids.iter().copied().find_map(|ino| { - st.entries - .get(&ino) - .filter(|e| e.node.name == name) - .map(|e| (ino, &e.node)) + let hit = { + let st = self.core.state.lock(); + st.children.get(&parent).and_then(|kids| { + kids.iter().copied().find_map(|ino| { + st.entries.get(&ino).filter(|e| e.node.name == name).map(|e| { + // A `lookup` reply carries attrs with the same TTL a + // `getattr` would, so `ls -l` — which is one `lookup` + // per entry and no `getattr` at all — takes its sizes + // from here. Resolving only in `getattr` left the whole + // listing provisional (bugs.md B14). + let provisional = matches!( + &e.node.kind, + NodeKind::File { + claimed_size: None, + .. + } + ) + .then(|| (e.parent, e.uid.clone())); + (ino, self.attr(ino, &e.node), provisional) + }) + }) }) - }); - match hit { - Some((ino, node)) => { - let attr = self.attr(ino, node); - reply.entry(&TTL, &attr, Generation(0)); - } - None => reply.error(Errno::ENOENT), + }; + let Some((ino, attr, provisional)) = hit else { + reply.error(Errno::ENOENT); + return; + }; + let Some((grandparent, uid)) = provisional else { + reply.entry(&TTL, &attr, Generation(0)); + return; + }; + // Resolving goes to the network. On the dispatch loop that is not + // allowed (the B5 lesson), so hand off — but only from the loop: doing + // it while already on a worker would queue a job onto the lane this + // thread occupies. + if !off_loop { + let fs = self.clone(); + let name = name.to_string(); + self.core + .workers + .run(Lane::Meta, move || fs.serve_lookup(parent, &name, reply, true)); + return; } + self.core.upgrade_sizes_for_parent(ino, &uid, grandparent); + // Re-read: on timeout or failure this is the provisional attr again, + // which is no worse than what this path used to always send. + let attr = { + let st = self.core.state.lock(); + st.entries + .get(&ino) + .map_or(attr, |e| self.attr(ino, &e.node)) + }; + reply.entry(&TTL, &attr, Generation(0)); } /// The body of [`Filesystem::readdir`], on whichever thread ends up serving it. @@ -3477,6 +3691,112 @@ fn apply_pending_sizes(nodes: &mut [Node], sizes: &HashMap) { } } +/// A size upgrade in flight for one folder, so the `getattr`s that need its +/// result wait for the one batch instead of each fetching its own node. +/// +/// A plain `Condvar` rather than a channel: the waiters do not want a value, +/// only the edge, and there may be hundreds of them for one folder. +#[derive(Default)] +struct Progress { + /// The whole batch has been applied (or failed). Nothing more is coming. + done: bool, + /// Bumped every time a chunk lands, so a waiter can tell "no progress since + /// I last looked" from "progress happened while I was looking". + generation: u64, +} + +/// A size upgrade in flight for one folder, so the `getattr`s and `lookup`s that +/// need its result wait for the one batch instead of each fetching its own node. +/// +/// Waiters are released **per chunk**, not once at the end: a waiter only cares +/// about its own file, and waiting for the other 792 is what let a large folder +/// outrun the timeout (bugs.md B14). +#[derive(Default)] +struct SizeUpgrade { + inner: Mutex, + ready: Condvar, +} + +impl SizeUpgrade { + /// How long a caller will wait for a real size before answering with the + /// provisional one. + /// + /// A `stat` that never returns is far worse than one that is briefly wrong: + /// on timeout the caller falls back to the pre-fix behaviour rather than + /// wedging whatever is listing the directory. + const WAIT: Duration = Duration::from_secs(10); + + /// Block until `resolved` reports the caller's own node has a real size, the + /// batch finishes without producing one, or [`SizeUpgrade::WAIT`] elapses. + /// + /// `resolved` is called with **no lock of ours held**. It reaches into + /// `state`, and the thread applying a chunk holds `state` before it signals + /// here — taking them in the other order would close the cycle. + fn wait_for(&self, resolved: impl Fn() -> bool) { + let deadline = Instant::now() + Self::WAIT; + loop { + let seen = { + let progress = self.inner.lock(); + if progress.done { + return; + } + progress.generation + }; + if resolved() { + return; + } + let mut progress = self.inner.lock(); + if progress.done { + return; + } + // A chunk landed while we were checking, so re-check rather than + // sleeping through the answer we were waiting for. + if progress.generation == seen + && self.ready.wait_until(&mut progress, deadline).timed_out() + { + return; + } + } + } + + /// Announce that a chunk has been applied. Every waiter re-checks its own + /// node; the ones it resolved return, the rest go back to sleep. + fn chunk_done(&self) { + self.inner.lock().generation += 1; + self.ready.notify_all(); + } + + /// Release every waiter for good. The worker must reach this on all paths. + fn finish(&self) { + let mut progress = self.inner.lock(); + progress.done = true; + progress.generation += 1; + drop(progress); + self.ready.notify_all(); + } +} + +/// Whether a `rename` may replace an existing destination, per POSIX. +/// +/// Split out from the handler because it is the part that is pure and the part +/// that is dangerous: every `Err` here is a refusal that happens *before* +/// anything is trashed, and getting one wrong turns a refusal into the +/// destruction of the destination (bugs.md B13). +/// +/// `dst_empty` is only meaningful when `dst_dir`; pass `true` otherwise. +fn check_replaceable(src_dir: bool, dst_dir: bool, dst_empty: bool) -> Result<(), Errno> { + match (src_dir, dst_dir) { + // A non-directory may not replace a directory, or vice versa. + (false, true) => Err(Errno::EISDIR), + (true, false) => Err(Errno::ENOTDIR), + // Proton trashes a folder with its whole subtree, so replacing a + // non-empty one would silently take its contents with it. POSIX says + // ENOTEMPTY, which is also the safe answer. + (true, true) if !dst_empty => Err(Errno::ENOTEMPTY), + _ => Ok(()), + } +} + fn node_size(node: &Node) -> u64 { match &node.kind { NodeKind::Folder => 0, @@ -3608,13 +3928,13 @@ impl Filesystem for ProtonFs { // listed folder — the common case — is a map hit, and answering it // inline costs less than the handoff would. if self.core.children_cached(parent) { - self.serve_lookup(parent, &name, reply); + self.serve_lookup(parent, &name, reply, false); return; } let fs = self.clone(); self.core .workers - .run(Lane::Meta, move || fs.serve_lookup(parent, &name, reply)); + .run(Lane::Meta, move || fs.serve_lookup(parent, &name, reply, true)); } fn getattr(&self, _req: &Request, ino: INodeNo, _fh: Option, reply: ReplyAttr) { @@ -3623,8 +3943,7 @@ impl Filesystem for ProtonFs { match st.entries.get(&ino.0) { Some(e) => { // A file still carrying the cheap enumeration's placeholder - // size: answer now with what we have and have the real size - // fetched in the background (B12). + // size (B12). let provisional = matches!( &e.node.kind, NodeKind::File { @@ -3632,7 +3951,7 @@ impl Filesystem for ProtonFs { .. } ) - .then_some(e.parent); + .then(|| (e.parent, e.uid.clone())); (self.attr(ino.0, &e.node), provisional) } None => { @@ -3641,10 +3960,38 @@ impl Filesystem for ProtonFs { } } }; - reply.attr(&TTL, &attr); - if let Some(parent) = provisional_in { - self.core.upgrade_sizes_for_parent(parent); - } + // The common case: the size is real, answer from the lock we just held. + let Some((parent, uid)) = provisional_in else { + reply.attr(&TTL, &attr); + return; + }; + // A provisional size is the *ciphertext* size, which is larger than the + // file. Publishing it makes every reader that trusts `st_size` — rsync, + // mmap, sendfile, a sized read loop — run off the end of the file and + // fail. So resolve it before answering rather than after (bugs.md B14). + // + // The cost is one batched round trip for the whole folder, not one per + // file: `ls -l` is one `getattr` per entry and they collapse onto a + // single upgrade. This is why B12's split is still worth having — the + // plain listing never comes here at all. + // + // Off the dispatch loop, because it goes to the network: blocking there + // would stall every other operation on the mount (the B5 lesson). + let fs = self.clone(); + self.core.workers.run(Lane::Meta, move || { + fs.core.upgrade_sizes_for_parent(ino.0, &uid, parent); + // Re-read: the upgrade writes through `state`, and on timeout or + // failure this is simply the provisional attr we already had. + let attr = { + let st = fs.core.state.lock(); + match st.entries.get(&ino.0) { + Some(e) => fs.attr(ino.0, &e.node), + // Forgotten while we waited. + None => attr, + } + }; + reply.attr(&TTL, &attr); + }); } fn readdir( @@ -4284,7 +4631,7 @@ impl Filesystem for ProtonFs { name: &OsStr, newparent: INodeNo, newname: &OsStr, - _flags: RenameFlags, + flags: RenameFlags, reply: ReplyEmpty, ) { let parent = parent.0; @@ -4317,6 +4664,78 @@ impl Filesystem for ProtonFs { } } }; + // `rename(2)` replaces an existing destination; Proton has no replacing + // rename, so the victim has to be removed first and the operation stops + // being atomic. Without this the 422 surfaced as a blanket EIO and every + // write-to-temp-then-rename tool — rsync, atomic editor saves — failed + // at the very end of its transfer (bugs.md B13). + // + // `RENAME_EXCHANGE` has no Proton primitive and cannot be emulated + // without a window in which one of the two names does not exist. + if flags.contains(RenameFlags::RENAME_EXCHANGE) { + reply.error(Errno::EINVAL); + return; + } + let victim = match self.core.lookup_child(newparent, &newname) { + // Renaming a node onto its own name is a no-op, never a self-replace. + Ok((_, vuid)) if vuid == uid => None, + Ok(x) => Some(x), + // Nothing to replace: the overwhelmingly common case. + Err(e) if e.code() == libc::ENOENT => None, + Err(e) => { + reply.error(e); + return; + } + }; + if let Some((victim_ino, victim_uid)) = &victim { + if flags.contains(RenameFlags::RENAME_NOREPLACE) { + reply.error(Errno::EEXIST); + return; + } + // POSIX requires both ends to agree on being a directory, and the + // check has to happen before anything is trashed: getting it wrong + // turns a refusal into the destruction of the destination. + let (src_dir, dst_dir) = { + let st = self.core.state.lock(); + let is_dir = |i: &u64| { + st.entries + .get(i) + .map(|e| matches!(e.node.kind, NodeKind::Folder)) + }; + match (is_dir(&ino), is_dir(victim_ino)) { + (Some(a), Some(b)) => (a, b), + _ => { + reply.error(Errno::ENOENT); + return; + } + } + }; + // Replacing a directory means trashing it, and Proton trashes a + // folder with its whole subtree — so the destination's emptiness has + // to be known before the decision is made. + let dst_empty = if dst_dir { + if let Err(e) = self.core.ensure_children(*victim_ino) { + reply.error(e); + return; + } + self.core + .state + .lock() + .children + .get(victim_ino) + .is_none_or(|kids| kids.is_empty()) + } else { + true + }; + if let Err(e) = check_replaceable(src_dir, dst_dir, dst_empty) { + reply.error(e); + return; + } + if let Err(e) = self.core.remove_replaced(victim_uid, &newname) { + reply.error(e); + return; + } + } // A node whose own creation is still queued has no server-side identity // to rename: the queued op *is* the node, so rewriting its target is the // whole rename. Nothing reaches the API, which is why this works offline @@ -4341,10 +4760,12 @@ impl Filesystem for ProtonFs { // now and this handle's is stale. A retry resolves it. Ok(false) => { warn!(%uid, name, newname, "queued create vanished under a rename"); + self.core.restore_replaced(victim.as_ref(), &newname); reply.error(Errno::EBUSY); } Err(e) => { error!(%uid, error = %e, "rewriting a queued create's target failed"); + self.core.restore_replaced(victim.as_ref(), &newname); reply.error(Errno::EIO); } } @@ -4361,11 +4782,17 @@ impl Filesystem for ProtonFs { .queue_rename(ino, &uid, newparent, &new_parent_uid, &newname) { Ok(()) => reply.ok(), - Err(e) => reply.error(e), + Err(e) => { + self.core.restore_replaced(victim.as_ref(), &newname); + reply.error(e); + } } return; } // Move first if the parent changed, then rename if the name changed. + // A failure past this point has already trashed any node the rename was + // replacing, so put it back rather than leaving the caller with neither + // the source moved nor the destination intact. if newparent != parent && let Err(e) = self .core @@ -4373,6 +4800,7 @@ impl Filesystem for ProtonFs { .block_on(self.core.client.move_node(&uid, &new_parent_uid)) { error!(%uid, error = %e, "move failed"); + self.core.restore_replaced(victim.as_ref(), &newname); reply.error(Errno::EIO); return; } @@ -4383,6 +4811,7 @@ impl Filesystem for ProtonFs { .block_on(self.core.client.rename_node(&uid, &newname, None)) { error!(%uid, error = %e, "rename failed"); + self.core.restore_replaced(victim.as_ref(), &newname); reply.error(Errno::EIO); return; } @@ -4888,7 +5317,7 @@ pub fn mount( trash_refreshing: Arc::new(AtomicBool::new(false)), thumb_gen: Arc::new(Mutex::new(HashSet::new())), no_thumbnail: Arc::new(Mutex::new(HashMap::new())), - size_upgrades: Arc::new(Mutex::new(HashSet::new())), + size_upgrades: Arc::new(Mutex::new(HashMap::new())), notifier: Arc::new(OnceLock::new()), transfers: TransferRegistry::new(), indexing: Arc::new(AtomicBool::new(false)), @@ -5076,6 +5505,173 @@ pub fn mount( Ok(outcome) } +#[cfg(test)] +mod size_upgrade_tests { + use super::SizeUpgrade; + use std::sync::Arc; + use std::sync::atomic::{AtomicBool, Ordering}; + use std::time::{Duration, Instant}; + + /// The point of the per-chunk design: a waiter returns as soon as *its own* + /// file is resolved, without waiting for the rest of the folder. A single + /// batch over 793 nodes took ~80 s, which outran the timeout and handed back + /// the provisional size this is all meant to prevent. + #[test] + fn a_waiter_returns_on_the_chunk_that_resolves_it() { + let slot = Arc::new(SizeUpgrade::default()); + let mine = Arc::new(AtomicBool::new(false)); + let worker = slot.clone(); + let flag = mine.clone(); + let t = std::thread::spawn(move || { + // Our node lands in the first chunk; two more follow. + std::thread::sleep(Duration::from_millis(30)); + flag.store(true, Ordering::SeqCst); + worker.chunk_done(); + std::thread::sleep(Duration::from_millis(200)); + worker.chunk_done(); + worker.finish(); + }); + let started = Instant::now(); + slot.wait_for(|| mine.load(Ordering::SeqCst)); + // Released by the first chunk, not the last. + assert!(started.elapsed() < Duration::from_millis(150)); + t.join().unwrap(); + } + + /// A chunk that does not resolve this waiter must not wake it for good. + #[test] + fn an_unrelated_chunk_does_not_release_a_waiter() { + let slot = Arc::new(SizeUpgrade::default()); + let worker = slot.clone(); + let t = std::thread::spawn(move || { + std::thread::sleep(Duration::from_millis(30)); + worker.chunk_done(); + std::thread::sleep(Duration::from_millis(60)); + worker.finish(); + }); + let started = Instant::now(); + // Never resolved: only `finish` can end this wait. + slot.wait_for(|| false); + assert!(started.elapsed() >= Duration::from_millis(80)); + t.join().unwrap(); + } + + /// A batch that ends without resolving the node — a failed fetch — still + /// releases its waiters, who fall back to the provisional size. + #[test] + fn finish_releases_a_waiter_that_was_never_resolved() { + let slot = Arc::new(SizeUpgrade::default()); + let worker = slot.clone(); + let t = std::thread::spawn(move || { + std::thread::sleep(Duration::from_millis(30)); + worker.finish(); + }); + let started = Instant::now(); + slot.wait_for(|| false); + assert!(started.elapsed() < SizeUpgrade::WAIT); + t.join().unwrap(); + } + + /// Already resolved before waiting: must not block at all. This is the + /// follower that arrives after the chunk it needed has landed. + #[test] + fn an_already_resolved_waiter_does_not_block() { + let slot = SizeUpgrade::default(); + let started = Instant::now(); + slot.wait_for(|| true); + assert!(started.elapsed() < Duration::from_millis(100)); + } + + /// Finishing before anyone waits must not strand the late arrival — the + /// flag is what is checked, not the notification, which it would miss. + #[test] + fn waiting_after_finish_returns_at_once() { + let slot = SizeUpgrade::default(); + slot.finish(); + let started = Instant::now(); + slot.wait_for(|| false); + assert!(started.elapsed() < Duration::from_millis(100)); + } + + /// Every waiter is released, not just the first: one `ls -l` puts one + /// waiter per file on the same folder. + #[test] + fn all_waiters_are_released() { + let slot = Arc::new(SizeUpgrade::default()); + let waiters: Vec<_> = (0..8) + .map(|_| { + let slot = slot.clone(); + std::thread::spawn(move || slot.wait_for(|| false)) + }) + .collect(); + std::thread::sleep(Duration::from_millis(20)); + slot.finish(); + for w in waiters { + w.join().expect("every waiter returns"); + } + } + + /// The timeout is the backstop: a batch that never finishes must not wedge + /// the caller's `stat` forever. + #[test] + fn a_waiter_gives_up_at_the_timeout() { + let slot = SizeUpgrade::default(); + let started = Instant::now(); + // Nothing will ever resolve or finish this. + slot.wait_for(|| false); + assert!(started.elapsed() >= SizeUpgrade::WAIT); + } +} + +#[cfg(test)] +mod replace_tests { + use super::check_replaceable; + + /// The case that motivated all of this: rsync renaming its temp file over + /// the real one. Two plain files, and it has to be allowed — refusing is + /// what made every rsync transfer fail at the last step (bugs.md B13). + #[test] + fn a_file_may_replace_a_file() { + assert!(check_replaceable(false, false, true).is_ok()); + } + + #[test] + fn an_empty_directory_may_be_replaced_by_a_directory() { + assert!(check_replaceable(true, true, true).is_ok()); + } + + /// Proton trashes a folder with its whole subtree, so allowing this would + /// discard every file under the destination without ever naming them. + #[test] + fn a_non_empty_directory_is_never_replaced() { + let e = check_replaceable(true, true, false).expect_err("must refuse"); + assert_eq!(e.code(), libc::ENOTEMPTY); + } + + #[test] + fn the_two_ends_must_agree_on_being_a_directory() { + assert_eq!( + check_replaceable(false, true, true) + .expect_err("a file may not replace a directory") + .code(), + libc::EISDIR + ); + assert_eq!( + check_replaceable(true, false, true) + .expect_err("a directory may not replace a file") + .code(), + libc::ENOTDIR + ); + } + + /// `dst_empty` describes a directory; it must not leak into the file case, + /// where callers pass `true` by convention. + #[test] + fn emptiness_is_ignored_when_the_destination_is_a_file() { + assert!(check_replaceable(false, false, false).is_ok()); + } +} + #[cfg(test)] mod thumb_tests { use super::{THUMB_EDGE, ratio_of, scale_thumbnail}; diff --git a/docs/bugs.md b/docs/bugs.md new file mode 100644 index 0000000..146e7f7 --- /dev/null +++ b/docs/bugs.md @@ -0,0 +1,1140 @@ +# Bugs & side findings + +Running tracker for bugs and loose ends spotted while working on something else — +so they don't get lost when the session that found them ends. + +Conventions: + +- **Open** / **Fixed (unverified)** / **Fixed** / **Won't fix** +- "Fixed (unverified)" means the code change is in but nobody has driven the real + flow against it yet. It stays that way until someone does. +- Record how it was *found*, not just what it is. The repro is the expensive part. + +--- + +## B1 — FUSE rename loses the file (data loss) + +**Status:** Fixed (verified 2026-07-20) +**Found:** 2026-07-19, user reported `mv file.mkv dir/` on the mount deleted the file +**Verified:** the exact repro below on the new daemon — `mv` returns 0, the file is +present in the destination listing and reads back its content. Previously it was in +neither directory. +**Where:** `crates/pdfs-fuse/src/lib.rs`, `Filesystem::rename` + +`rename` ended with: + +```rust +st.forget(&uid); +st.children.remove(&newparent); // memory only +``` + +`forget()` deletes the node's **DB row** (`db.delete_node`). `children.remove()` +only drops the **in-memory** listing, leaving `nodes.listed = 1` on the +destination folder. The next `ls` goes through `ensure_children`, which takes the +DB fast path (`children_if_listed`, lib.rs:722) and rebuilds the destination from +the database — where the node no longer has a row. + +Net effect: file gone from the source, absent from the destination, and +`rename(2)` returned **0**. Silent loss with a success code. + +**Severity is worse than "stale cache":** `listed` is a **DB** column, so the bad +state survives a daemon restart. Nothing clears it on its own — the affected +folder keeps serving the listing that omits the file indefinitely, until an event +invalidation or a manual refresh happens to hit it. Confirmed on the repro +folders, which still read `listed=1` with no child rows minutes later. + +The data itself is believed intact server-side (the move succeeded; only the +local view is wrong), but that was **not** verifiable without a daemon restart — +see the status line. + +Also hits plain in-place renames (`mv a.txt b.txt`), same mechanism — the parent +is both source and destination there. + +The control-socket `Core::move_to` path was always correct; it used +`invalidate_listing`, which clears the DB flag as well. The FUSE path didn't. + +**Fix:** use the same helper. + +```rust +st.forget(&uid); +st.invalidate_listing(newparent); +``` + +**Repro:** + +``` +cd && mkdir d && echo hi > f.txt && mv f.txt d/ +# renameat2(...) = 0, but f.txt is in neither directory, +# not in trash, and absent from the `nodes` table +``` + +**Note on `invalidate_listing`:** it early-returns when the folder isn't in the +in-memory `children` map, so it won't clear a stale DB `listed` flag in that +case. Safe on this path (both `lookup_child` and the explicit +`ensure_children(newparent)` guarantee the listing is resident), but it's a sharp +edge for other callers. See B4. + +--- + +## B2 — The reported .mkv went to trash, the repro vanished entirely + +**Status:** Partly resolved — `mv` exonerated; trash origin still unattributed +**Found:** 2026-07-19, while chasing B1 + +### What was settled (2026-07-19) + +**`mv` never trashes.** Three repros on the mount, all with the old (pre-B1-fix) +daemon: + +| repro | node state | destination | result | +|---|---|---|---| +| `zz-claude-repro-src.txt` | create still queued | fresh folder | vanished, **not** trashed | +| `zz-b2-settled.txt` | settled, real uid, landed | listed folder | vanished, **not** trashed | +| `zz-b3 Movie Title.mkv` | settled | folder named as the file's stem (the user's exact shape) | vanished, **not** trashed | + +So B1 fully explains the *disappearance*, and nothing in the rename path +trashes. The `.mkv` reaching the trash has a different cause. + +**The listing the user acted on was almost certainly stale.** B1/B4 make stale +listings *persistent* — `listed = 1` lives in the DB and survives restarts — so +`ls` showing the file at 15:57 is not evidence it still existed remotely. This +is the most likely reading: it had been trashed earlier and the mount kept +showing it. + +**Ruled out:** + +- *Sync engine.* `reconcile_folder` gates on `mode == "mirror"`; `~/Videos` was + `ondemand` from 15:39:27. All three sync trash sites log, and no such rows exist. +- *Log pruning hiding the evidence.* `ACTIVITY_KEEP = 2000`; the table is at the + cap but its oldest row is Jul 17 18:03, so the whole window is covered. The + `.mkv`'s uid appears exactly once — an `Upload` at 06:07:56 — and never again. +- *Conflict machinery.* `keep_as_conflict_copy` uploads a new file under an + alternate name; it never trashes the original. +- *Eviction through the mount.* Was the best theory: `mirror→ondemand` calls + `evict_dir_contents`, which would issue one `unlink(2)` per file, and FUSE + `unlink` trashes remotely. But `apply_sync_folder_mode` evicts **before** + `spawn_ondemand_mount`, so the deletes hit the plain local directory. Dead. +- *Control-socket delete.* `CtlRequest::Delete` logs on both success and failure. + +**What remains:** `trash_child` (backing FUSE `unlink`/`rmdir`) was the only +trash path that wrote no activity row — so an ordinary `rm` on the mount, at any +point after 15:39:27 when `~/Videos` became a FUSE mount, would produce exactly +what we see and leave no trace. Note there is also a *trashed folder* of the same +name (`Evangelion 1.11 - You Are (Not) Alone`, `is_dir=1`, content mtime +04:57:50), which suggests this mkdir-and-move dance had been attempted earlier — +plausibly followed by a manual cleanup while B1 was making files appear to vanish. + +That is a hypothesis, not a finding. It is not provable from the evidence that +survives. + +### Fix applied + +`trash_child` now logs to the activity feed on both success (`"trashed from the +mount"`) and failure, matching every other trash site. The next occurrence will +be attributable — which is the part that actually mattered here. + +### If it recurs + +The activity log is now the first place to look. `trash` rows carry no +trashed-at timestamp (the `mtime` column is the node's *content* mtime — for the +`.mkv` it read 06:07:56, matching its upload, not its deletion), so the activity +feed is the only ordering evidence there is. + +Two different endings for what looked like the same operation, so B1 may not be +the whole story: + +- User's `Evangelion 1.11 - You Are (Not) Alone.mkv` → **trash**, full size + (1768670449) intact, recoverable via `pdfs restore`. +- Clean repro (`zz-claude-repro-src.txt`) → **gone entirely**. Not in trash, no + `nodes` row, nothing in the search index. + +B1 explains the second. It does not explain a node reaching the trash — nothing +in the FUSE `rename` path calls `trash_nodes`. + +Leading theory: the trash came from the sync-conflict machinery, which *is* +logged (`ActivityKind::Trash` rows exist with `(sync-conflict )` details). +The `~/Videos` folder is registered as a sync folder in `ondemand` mode, and had +just been switched to `ondemand` shortly before. `reconcile_folder` gates on +`mode == "mirror"`, so it should have been inert — worth confirming that gate +actually held, and that no pass was already in flight across the switch. + +Note the file was a **conflict-copy sibling** of two files already carrying +`-003` / `-004` suffixes, so the conflict path had definitely been active in that +directory. + +**Next step:** find the trash event's origin. The activity log had no row at the +15:58 mv, which points away from a logged (control-socket / sync) path — but the +timestamp bug in B3 made that table hard to read, so re-check with correct +scaling before trusting the absence. + +--- + +## B3 — ~~Activity log timestamps written in seconds, read as milliseconds~~ + +**Status:** Not a bug — investigator error, kept as a record +**Found / retracted:** 2026-07-19 + +Originally filed because every activity row rendered as `1970-01-21 …`. That was +an artifact of the ad-hoc debugging query, which divided by 1000; the code never +does. Seconds are consistent end to end: + +- writer — `log_activity` uses `now_secs()` (`pdfs-fuse/src/lib.rs`) +- schema / `activity_list` — pass the value through untouched +- reader — `activity_time` calls `glib::DateTime::from_unix_local(secs)` + +The correct manual query, for next time: + +```sql +select datetime(time,'unixepoch','localtime'), kind, target, detail, ok +from activity order by time desc limit 40; +``` + +Worth noting because the bad query is what made the activity log look empty +around the reported `mv` — which is evidence B2 leans on. That absence has since +been re-checked with correct scaling and it does hold: no row at 15:58. + +--- + +## B4 — `invalidate_listing` silently skipped non-resident folders + +**Status:** Fixed (verified 2026-07-20, via B1's repro — the FUSE rename path that +depends on this helper now behaves correctly end to end) +**Found:** 2026-07-19, reviewing the B1 fix +**Where:** `crates/pdfs-fuse/src/state.rs:257` + +```rust +if self.children.remove(&ino).is_none() { + return; // never clears the DB `listed` flag +} +``` + +The early return assumed "not in the hot cache ⇒ nothing to invalidate", which +stopped being true once listings became DB-backed. A folder trimmed from the +in-memory map but still `listed = 1` in the DB could not be invalidated at all — +callers thought they had dropped the listing, and `ensure_children` would happily +rebuild the stale one. + +**Fix:** drop the early return; always clear the flag. Costs one redundant +`UPDATE` when nothing was cached. + +`Core::refresh_dir` had been hand-rolling a workaround for exactly this (clearing +the DB flag itself, then reaching into `state.children` directly, with a comment +explaining why it couldn't use the helper). It now just calls +`invalidate_listing`. **Behaviour change worth knowing:** `refresh_dir` used to +propagate a DB write failure to the caller of `CtlRequest::Refresh`; it now warns +and reports success, matching every other invalidation site. + +**Test:** `a_deleted_child_leaves_a_listed_parent_serving_a_stale_listing` in +`pdfs-core/src/db/tests.rs` pins the B1/B4 mechanism at the DB layer — a deleted +child plus a still-`listed` parent yields a listing that silently omits it. + +--- + +## B5 — `ls -l` costs a network round trip per file (thumbnail xattr probes) + +**Status:** Fixed (verified 2026-07-20) +**Found:** 2026-07-19, investigating "`exa -l` is slow in the mounts, `exa` is fast" + +### Reproduced (2026-07-19) + +All runs cold (35 s wait to clear the 30 s attr TTL). `exa` needs only `readdir`; +`exa -l` stats every entry, so the delta is per-entry metadata cost: + +| mount | entries | `exa` | `exa -l` | delta per entry | +|---|---|---|---|---| +| primary `~/ProtonDrive/Installer` | 34 | 10 ms | 11 ms | **0.03 ms** | +| on-demand `~/Documents` | 45 | 27 ms | 54 ms | **0.6 ms** | +| on-demand `~/Videos/[Reaktor] FMA …` | 65 | 73 ms | 193 ms | **1.85 ms** | + +### Cause (2026-07-20): a network round trip per file per xattr name + +`strace -f -T` on a cold `exa -l` of the 65-file `.mkv` directory: + +``` +lgetxattr(".../E01 ....mkv", "user.proton.thumbnail", NULL, 0) = -1 ENODATA <0.186435> +``` + +129 `lgetxattr` and 258 `llistxattr` calls for 65 entries, each `lgetxattr` +~186 ms. Three things compounded: + +1. **`listxattr` advertised `user.proton.thumbnail` + `user.proton.preview` for + every file**, regardless of whether that file could have one. An xattr-aware + lister then asks for each advertised name — two `getxattr` per entry. +2. **`Core::thumbnail` cached only success.** `download_thumbnail` returning + `None` — the normal answer for a `.mkv` — was never remembered, so every + listing re-asked the API and was re-told nothing, forever. +3. **`getxattr` ran inline on fuser's dispatch loop**, not on the `Workers` + pool. `lookup` and `readdir` had been moved off it (PERF #1.0); this one was + missed. So each 186 ms miss stalled *every* other op on the mount, which is + why the concurrency `exa` does have bought nothing. + +**The mount-kind correlation was a red herring.** It tracks file *type*, not the +fork: `~/Videos` is `.mkv` (Proton generates no thumbnail — always a miss), +`~/ProtonDrive/Installer` is installers whose probes were already warm. The +`fork_state` theory in the previous write-up is wrong; forked and primary mounts +run the same code and neither is at fault. Worth noting as a lesson — three +measurements on different directories looked like a mount-kind effect because +nobody had varied file type independently. + +### Fix + +- `listxattr` advertises the names only for `image/*` and `video/*` media types. + `getxattr` still honours an explicit request for an unadvertised name, so + nothing becomes unreachable. Everything else — documents, installers, archives + — now costs zero round trips per listing. +- `Core::thumbnail` remembers misses in `no_thumbnail`, keyed `(uid, type)` and + validated against the node's mtime exactly as the positive side is. Bounded by + clearing at `MAX_THUMBNAIL_MISSES` (8192). +- `getxattr` hands off to `Lane::Meta`, so a miss can never block the dispatch + loop. + +A video directory still pays its misses once per revision (image/video is where +a thumbnail plausibly exists, so the probe is legitimate) — but in parallel, and +never again. + +### Verified (2026-07-20) + +Cold runs (40 s wait each) on the restarted daemon: + +| measurement | before | after | +|---|---|---| +| `~/Videos/…FMA…` (65 `.mkv`) cold `exa -l` | 193 ms | **14 ms** | +| `~/Documents` (45 entries) cold `exa -l` | 54 ms | **13 ms** | +| `user.proton.*` probes over the 65 `.mkv` | 130 | **0** | +| `~/Documents` probe latency, 1st listing | 124–395 ms | 124–395 ms | +| `~/Documents` probe latency, 2nd listing | 124–395 ms | **0.26–0.67 ms** | + +Each part of the fix is separately visible in the traces: + +- **The advertising gate.** The `.mkv` directory now issues *zero* `user.proton.*` + probes; the only remaining `lgetxattr` is exa's own `security.selinux` at + ~0.3 ms. Note this means Proton does **not** report those files as `video/*` — + the gate is excluding them by media type, not because they are known to lack a + thumbnail. A video whose media type is generic no longer advertises one even if + it has it; `getxattr` still serves an explicit request, so it stays reachable. + Worth revisiting if thumbnails ever go missing in a file manager. +- **The negative cache.** `~/Documents` holds 10 `.png` files, correctly still + advertised, so it probes 20 times on both passes — but the second pass answers + in 0.26–0.67 ms instead of going to the wire. That is the whole point: the + probes that are legitimate stop being expensive after the first. +- **The worker handoff.** Pass 1 on `~/Documents` shows interleaved + `<... lgetxattr resumed>` lines across several tids, i.e. the misses now + overlap instead of serializing behind the dispatch loop. + +### Secondary finding (still open): `lookup` is O(n) per name + +**Where:** `crates/pdfs-fuse/src/lib.rs`, `serve_lookup` + +`serve_lookup` linear-scans the parent's children comparing names, and with no +`readdirplus` that is one `lookup` per child — O(n²) name comparisons under the +global state lock for a listing. + +Real, but **not** what made this slow: it costs the same on both mount kinds and +at 65 entries a linear scan is nanoseconds against a 186 ms round trip. Two +candidate optimizations were considered and deliberately *not* applied, and that +judgement still holds now the real cause is known: + +- **Per-directory `name -> ino` map.** A second structure that must stay in sync + with `children` — the two-halves-of-one-cache shape that caused B1 and B4. +- **`readdirplus`** (`fuser` 0.17 supports it; we return `ENOSYS`). The right + shape long-term, but it is a rewrite of the directory-read path and it would + have hidden this bug rather than explained it. + +### Measurement notes for next time + +- Cold requires a 35 s wait per run (30 s attr TTL). Warm runs show ~6 ms + regardless and prove nothing. +- `strace -c` on `exa` **needs `-f`** — exa is multithreaded, and without it the + trace captures only main-thread startup (3 `statx` calls, no `getdents`). +- `-T` (per-call durations) is what cracked this; `-c` summaries attribute time + across threads in a way that hid the 186 ms constant. + +--- + +## B6 — Daemon sets no file modes: `control.sock` is an unguarded authority + +**Status:** Fixed (verified 2026-07-20) +**Found:** 2026-07-19, while writing the plaintext-at-rest threat model in +`docs/ARCHITECTURE.md` §8. Not from a report — from checking a claim before +asserting it in a doc. I had written "their default 0700 permissions protect +them", went to verify, and found we set no modes at all. +**Where:** `crates/pdfs-fuse/src/lib.rs` (`UnixListener::bind(control_socket)`), +`crates/pdfs-core/src/config.rs` (`create_dir_all` on state/cache dirs) + +`grep -rn "set_permissions\|from_mode" crates/pdfs-core/src crates/pdfs-fuse/src` +returns exactly one hit, and it is `shell.rs` setting `0755` on a generated +script. Nothing else sets a mode. So: + +- state and cache directories are created at `0777 & ~umask` — typically `0755` +- `control.sock` is bound at the same, typically `0755` + +Anything that can connect to the socket drives the daemon with its authenticated +session: enumerate the tree, read file contents, upload, trash, create public +share links. No credential is required — the keyring is never consulted, because +the daemon already holds the session. + +The only thing preventing this today is that `~/.cache` and `~/.local/state` are +conventionally `0700`. That is a property of the user's system, not something we +establish or check. It does not hold on a machine with a permissive umask, a +group-shared home, or a home restored from a backup that flattened modes. + +**Severity:** low on a single-user desktop, real on a shared or multi-user host. +It is a privilege boundary rather than a data-at-rest issue, which is what makes +it worth more than the cache-plaintext point it was found next to. + +**Fix:** `chmod 0600` on the socket immediately after `bind` (before the listener +thread starts accepting), and `0700` on the state and cache directories at +creation. Both are a few lines. Worth also asserting the socket mode in a test, +since a regression here is silent. + +**Fixed 2026-07-19:** `AppDirs::ensure` now sets `0700` on the state, cache, and +config directories on *every* start (not just at creation, so an existing +permissive directory is tightened), and `config::restrict_socket` sets `0600` on +both the control socket and the tray socket immediately after `bind`. A socket +whose mode cannot be set takes the daemon down rather than serving unguarded; +the tray's is best-effort, since it only guards single-instancing. Unit tests in +`config.rs` assert both modes. + +**Verified 2026-07-20** on the restarted daemon — all three directories now read +`drwx------` and both sockets `srw-------`, against `drwxr-xr-x` / `srwxr-xr-x` +before. + +**Measured exposure at the time of the fix:** the directories really were +`drwxr-xr-x` and the live socket `srwxr-xr-x`, but `~/.cache` and `~/.local` +were both `0700` on this machine, so nothing was reachable in practice. The bug +was a latent dependency on those parents, not a live hole. Removing the +dependency is the point. + +--- + +## B7 — renamed directory reads as missing until the entry TTL expires + +**Status:** Fixed (verified 2026-07-20) +**Verified:** `mkdir "zz-b7 Old Name"` with a file inside, `mv` to a new name, then +an immediate `ls` and `cat` through the new name — both succeed with no wait. The +directory used to return ENOENT until the entry TTL expired. +**Found:** 2026-07-19, user renamed a folder on the mount with `mv`. `ls` of the +parent listed the new name, but `ls /` returned ENOENT — repeatedly, so +not a one-shot race. It self-healed on its own a few minutes later, and the +files were all intact: this is a visibility bug, not data loss. +**Where:** `crates/pdfs-fuse/src/state.rs`, `State::relocate` + +**Cause:** the online rename path ended with `relocate`, which called `forget` on +the moved node. `forget` drops the node's `by_uid` mapping, so when the +invalidated parent listing re-enumerated, `intern_mem` allocated it a **fresh** +inode. But the kernel had already carried the renamed dentry over to the *old* +inode number, and it holds that dentry for the entry TTL. Every lookup, getattr +and opendir through it resolved to an inode `entries` no longer held: + + entries.get(old_ino) -> None -> ENOENT + +`readdir` of the parent went the other way — it walks `children` and reports the +*new* inode — which is exactly why the directory listed fine but could not be +entered. Once the TTL expired the kernel re-looked-up the name, got the new +inode, and everything worked again. + +**Fix:** `relocate` now rewrites the node in place (`rename_in_place`) and +invalidates both parents' listings, instead of forgetting it. The inode is +preserved, so the kernel's dentry stays valid and the re-enumeration reuses the +same `by_uid` slot. Both `relocate` tests now assert inode stability; that +property was untested, which is how this got through the B1 fix. + +**Note:** dropping the `forget` also stops the moved node's DB row from being +deleted and re-created on every move — the row is updated instead, which is what +the B1 fix was working around from the other side. + +--- + +## B8 — no way to complete a CAPTCHA, so a gated sign-in is unrecoverable + +**Status:** Implemented, partially verified — the bridge is proven, a real gate is not +**Found:** 2026-07-19, user hit `proton api error Unknown (http 422): For +security reasons, please complete CAPTCHA` and had no way to answer it. +**Where:** SDK `api.rs`/`http.rs`/`session.rs`, `pdfs-core/src/auth.rs`, +`pdfs-gui/src/app/pages/verify.rs` + +**Cause:** three gaps stacked, none of which is a bug on its own. + +1. `ResponseCode` had no `9001`, so the gate deserialized to `Unknown` — which + is why the error read as an opaque failure rather than a recoverable prompt. +2. Nothing read `Details.HumanVerificationToken`, and the client never sent the + `x-pm-human-verification-token{,-type}` headers the retry needs. +3. No UI could render Proton's hosted verification page. + +**Fix:** `HumanVerification` (challenge) + `HumanVerificationCredential` +(answer) in the SDK, header plumbing on the session-less login calls, and +`ProtonApiSession::begin_verified`. `pdfs-core` promotes a solvable gate to +`Error::HumanVerificationRequired` and `auth::login_interactive` re-runs the +login with the earned token — the retry lives in core because the gated attempt +burns its SRP handshake, which no front-end should have to know. The GUI hosts +the page in a `WebKitWebView` (webkit6 0.4, pairs with the existing gtk4 0.9) +and bridges the page's `postMessage` to a script message handler. + +**Deliberately not done:** `email`/`sms` verification methods (the token arrives +out of band, so the webview cannot complete them — such a gate stays a plain API +error rather than opening a page the user cannot finish), HV on *authenticated* +endpoints (only the login path is plumbed), and re-gating of the retry (a second +challenge means the token was rejected; looping would trap the user). + +**Verified:** the WebKit bridge end-to-end with a throwaway probe — handler +registers, the page's `postMessage` arrives, non-completion messages are +filtered, the completion token round-trips. Unit tests cover 9001 parsing, the +challenge/`Details` shape, URL escaping, and message filtering. + +**Not verified:** a real gated login against verify.proton.me. It cannot be +triggered on demand, so the exact message shape Proton's page posts is taken +from its documented contract, not observed. If verification appears to hang with +the page solved, that is the first thing to check — `extract_token` in +`verify.rs` is the single place that decides what counts as completion. + +**Note:** the SDK half shipped as proton-sdk / proton-drive-rs **0.1.11**. The +workspace requirement was widened to `"0.1"` at the same time, so later 0.1.x +releases are picked up by `cargo update` without an edit here. + +--- + +## B9 — Enter did not open the selected result in the launcher + +**Status:** Fixed (verified) +**Found:** 2026-07-19, user reported Enter doing nothing in `pdfs-prompt` while +the arrow keys and Escape worked normally. +**Where:** `crates/pdfs-gui/src/prompt.rs`, the window key controller + +**Cause:** key-event *phase*, not key handling. `gtk4::Entry` — really its inner +`GtkText` — binds Return to its own `activate` and consumes it. The launcher's +`EventControllerKey` is attached to the **window** in the default **bubble** +phase, so the focused entry saw Return first and the window handler was never +reached. Escape and Up/Down worked precisely because `GtkText` has no bindings +for them and they bubbled up as intended. + +That asymmetry is the tell: when some keys reach a window-level controller and +others silently don't, the ones that don't are being claimed by the focused +widget. + +**Fix:** Return moved off the window controller and onto +`entry.connect_activate`. Enter while focus is in the results list was already +handled by `row_activated`, so the two together cover both focus positions. + +**Rejected alternative:** setting the window controller to +`PropagationPhase::Capture`. It fixes the symptom by putting the handler ahead +of the entry — but ahead of it for *every* keystroke, not just Return, which +puts text input and IME composition behind a handler that has no business +seeing them. The narrow fix has no such blast radius. + +**Verified:** live. Pressing Enter logged `opening path=…`, the daemon hydrated +the file, and `xdg_open` ran. + +--- + +## B10 — GLib critical when the launcher closes over an in-flight open + +**Status:** Open (side finding) +**Found:** 2026-07-19, seen in `pdfs-prompt` output immediately after a +successful Enter-to-open while verifying B9. + +``` +GLib-GIO-CRITICAL: g_list_store_remove: assertion '!g_sequence_iter_is_end (it)' failed +``` + +**What is known:** it fires right after the `opening path=…` log line, i.e. +during `xdg_open` + `window.close()`. It is a warning, not a crash — the open +itself succeeded. + +**What it is not:** the launcher's own row handling. `Section::set_rows` walks +`GtkListBox` children (`first_child`/`remove`) and touches no `GListStore`, so +the failing store belongs to GTK/libadwaita internals, most likely something +teardown-ordering related in the app/window bookkeeping as the window is closed +while a launch is still settling. + +**Why it was not chased:** it appeared while verifying an unrelated fix and has +no user-visible effect. Worth revisiting if the launcher ever misbehaves on +close (a hang, a lost open, or a stale window), since an assertion firing during +teardown is exactly the shape of bug that later turns into one of those. + +**Where to start:** run `pdfs-prompt` under `G_DEBUG=fatal-criticals` to turn +the warning into an abort and get a real backtrace naming the store. + +## B11 — a file moved while its create is still queued reads as empty + +**Status:** Fixed (verified 2026-07-20) +**Found:** 2026-07-20, while verifying B1 on the restarted daemon. + +The B1 repro is `echo hi > f.txt && mv f.txt d/`, i.e. the move lands while the +create is still queued for upload. Immediately after the `mv`, the file is +present in the destination listing — B1 is genuinely fixed — but stats as **0 +bytes** and `cat` returns nothing: + +``` +-rw-r--r-- 1 narl narl 0 Jul 20 00:29 zz-b1-dst/zz-b1-src.txt +``` + +A few seconds later the same file reads correctly (`3` bytes, `hi`). A control +file created and read *without* an intervening `mv` was correct immediately. + +**Cause:** `State::intern_mem` replaces an existing entry's node wholesale +(`e.node = node`). A move invalidates both parents' listings (that is B7's fix, +and it is correct), so the next `ls` re-enumerates — and the node that comes back, +from the remote or from its DB row, carries the size of the revision the *server* +holds. For a file whose write is still queued that is the pre-write size, usually +0. Interning it reverts the optimistic size `record_pending_write` had stamped. + +**Why an empty read rather than a short one:** a file that stats as 0 bytes gets +**no `read` from the kernel at all**. `read_range` would have served the staged +blob quite happily; it is never asked. So the file reads as empty for as long as +the stale size stands, and "empty file" is indistinguishable from "file whose +contents were lost" to whatever is reading. + +`Core::hydrate` already solved exactly this for the *restart* case — it stamps +each pending write's size onto the node as entries materialize. The protection +was simply missing on every live re-enumeration path. + +**Fix:** `Core::stamp_pending_sizes` re-applies the optimistic size to a batch of +nodes, called from both arms of `ensure_children` (the DB fast path and the +network path) before the state lock is taken. It snapshots the pending map first +and returns: no site in the daemon holds `pending` and `state` at once, and this +is not the place to become the first — `hydrate` established that pattern for the +same reason. + +Scoped to `ensure_children` deliberately. The other `intern` sites are +single-node and all authoritative by construction (mkdir, create, an upload that +just landed); `drain.rs`'s post-upload adoption is *supposed* to take the +server's node, and already handles the queued case through `rebaseline_pending`. + +**Tests:** `pending_size_tests` in `pdfs-fuse/src/lib.rs` covers the queued file +keeping its size, a settled sibling keeping the server's, folders being left +alone, and an empty pending map changing nothing. + +**Verified 2026-07-20** on the rebuilt daemon: `echo hi > f.txt && mv f.txt d/` +then an immediate `ls -l` and `cat` gives 3 bytes and `hi`, against 0 bytes and +empty output before. + +--- + +## B12 — cold enumeration is slow per entry, and goes superlinear past ~500 + +**Status:** Both causes fixed and verified (2026-07-20); attr-invalidation follow-up unverified +**Found:** 2026-07-20, measuring improvements.md P2.8 ("pipeline +`enumerate_nodes_detail`") before implementing it, to decide whether the +bottleneck was network or crypto. + +### Measured (cold; `pdfs refresh ` then a timed `ls -1`) + +| entries | time | per entry | +|---|---|---| +| 1–9 | 350–550 ms | fixed cost | +| 34 | 629 ms | — | +| 147 | 1.92 s | 12.5 ms | +| 251 | 3.18 s | 12.4 ms | +| 484 | 6.25 s | 12.5 ms | +| **793** | **38.8 s** | **48.9 ms** | + +The 793 figure reproduces to ±100 ms across three runs. Marginal cost from 484 to +793 is **106.7 ms/entry**, 8.5× the baseline. + +`perf record` against the running daemon shows **~90 % of cycles inside the +`pdfs` binary** in both cases, and sample counts matching wall time (1022 samples +≈ 5.1 s, 7538 ≈ 37.9 s at 199 Hz) — so this is CPU-bound throughout, not waiting +on the network. Symbols are unavailable (the installed binary is stripped), which +is what blocks attribution. + +### Cause 1 (found, ~half the baseline): every FTS row is deleted by a full scan + +`nodes_fts` declares `uid` as **UNINDEXED**: + +```sql +CREATE VIRTUAL TABLE nodes_fts USING fts5(uid UNINDEXED, name, tokenize='trigram') +``` + +and `Db::upsert_nodes` (`pdfs-core/src/db/nodes.rs`) deletes by exactly that +column on every node, because "FTS5 has no UPSERT": + +```sql +DELETE FROM nodes_fts WHERE uid = ?1 +``` + +An UNINDEXED FTS5 column is not searchable, so the predicate cannot use an index. +`EXPLAIN QUERY PLAN` confirms it: + +``` +`--SCAN nodes_fts VIRTUAL TABLE INDEX 0: +``` + +One full scan of the FTS index per node written. Measured on a `.backup` copy of +the live 171 MB DB (17 443 indexed nodes): + +| deletes | time | per delete | +|---|---|---| +| 484 | 3.09 s | 6.4 ms | +| 793 | 5.26 s | 6.6 ms | + +So **~6.5 ms of the 12.5 ms per-entry baseline is this one statement**, and it +gets worse for every user as their node count grows — the scan is over the whole +index, so the cost of writing *any* listing scales with the size of the *account*. +It is also paid by every sync pass and every event-driven refresh, not just `ls`. + +**Fix direction:** delete by rowid instead. FTS5 deletes by rowid efficiently, so +map `nodes.rowid` to the FTS rowid and drop the `uid` column from the index +(or keep it UNINDEXED purely for retrieval). Needs a schema migration on a live +171 MB DB, so it wants its own change rather than being smuggled into this one. + +### Cause 2 (the dominant one): an S2K key derivation per file, to list a folder + +**Symbols came from the local unstripped build, not a new install.** `strip` +preserves both the build id and the text addresses, so when `/usr/bin/pdfs` and +`target/release/pdfs` reported the *same* build id, the stripped binary was just +a copy of the local one and `perf buildid-cache -a target/release/pdfs` was +enough to symbolise a recording of the running daemon. Worth remembering — it +turned a "needs another install + restart" into a five-second step. + +Hot functions over a 793-entry cold enumerate: + +| % | symbol | +|---|---| +| 64.4 | `sha2::sha256::x86::digest_blocks` | +| 6.0 | `::update` | +| 5.7 | `sqlite3VdbeExec` (cause 1, above) | +| 4.0 | `sha2::sha256::compress256` | +| 1.5 | `pgp::types::s2k::StringToKey::derive_key` | + +**~74 % of the listing is SHA-256 inside PGP's S2K**, i.e. the per-file node-key +unlock that `build_node` does under `NodeDetail::Full`. `derive_key` itself shows +1.5 % because the time lands in its SHA leaf. Nothing downloads during an `ls`, so +PGP is the only plausible source of that SHA. + +### The "cliff past ~500" framing was wrong + +That was an inference from a single folder, not a measurement. An S2K's cost is +set by the iteration count in the key packet, which is chosen by *the client that +uploaded the file* — so it varies per file, not per listing size. Every folder +measured sits at 12–16 ms/entry except `Music/aC_ID.dll` at 49 ms, and +`Music/ELDEN RING SOUNDTRACK` (67 files, same parent tree) is ~16 ms/entry, so it +is not "Music files are expensive" either. A 4× jump for a 1.6× change in n fits +"those files were uploaded by a client using a costlier S2K" far better than a +size threshold. n and folder identity are confounded in the data — there was only +ever one folder above 500 entries. + +Left unresolved deliberately: the fix below removes the per-file S2K from the +listing path entirely, so which of the two explanations held stopped mattering. +To settle it anyway, log the S2K iteration count per file across one enumerate of +each folder. + +### Fix: enumerate cheap, upgrade sizes in the background + +`ensure_children` now calls `enumerate_nodes_light`, which skips the file +node-key unlock (folders are still unlocked — their keys are what the children +decrypt with). The listing is served from that immediately. + +`Light` returns no `claimed_size`, and `ls -l` wants sizes, so a naive split would +just move the same S2K onto the first `stat`. Instead `Core::spawn_size_upgrade` +fetches the full nodes for that folder on a worker, batched, after the listing has +been answered, and adopts *only* the size — re-interning wholesale would clobber a +name or parent that a concurrent rename/move had changed. It is single-flight per +folder, because a `stat` of one entry in a fresh listing means a `stat` of all of +them. Queued writes keep their optimistic size through it, via the same +`stamp_pending_sizes` that B11 added. + +Three entry points cover every way a provisional listing can appear: the network +enumeration, the DB fast path (rows persisted before an upgrade ran), and +`getattr` itself (a listing restored by `hydrate` on mount, which `ensure_children` +returns early for). + +**Known tradeoff — sizes are provisional until the upgrade lands.** `node_size` +falls back to `total_size_on_storage`, the ciphertext size, so a `stat` in that +window reads slightly *too large*. Reads are unaffected: the revision reader +carries its own authoritative size. Deliberately not the B11 shape — that +reported **0**, which made the kernel skip reads entirely; too-large is cosmetic +and self-corrects. + +**The window was longer than "a round trip", and that took a correction.** The +daemon has the real sizes quickly, but the *kernel* keeps the provisional attrs +for the full 30 s entry TTL, so `ls -l` kept reporting them long after the DB was +right. This nearly caused a misdiagnosis during verification: two successive +`ls -l` runs agreed with each other and both disagreed with the truth by a +constant +59 bytes, which reads exactly like "the upgrade never ran". Querying the +DB directly is what separated "not computed" from "computed but not visible". + +Closed by having `spawn_size_upgrade` call `notifier.inval_inode` for the inodes +it corrected, after the DB write so a provoked re-`getattr` cannot race the +persistence. That needed a `Notifier` on `Core` (a `OnceLock`, since the session +is built *from* the `Core`), and a fresh one per on-demand fork — each fork has +its own inode space, so notifying through the primary mount's channel would name +inodes that session has never heard of. + +### Verified (2026-07-20) + +| folder | before | after | +|---|---|---| +| `Music/aC_ID.dll` (793) | 38.8 s | **4.48 s** (8.7×) | +| `InstantUpload/Camera` (484) | 6.25 s | **2.91 s** (2.1×) | +| `Pictures/old` (417) | 5.10 s | **2.25 s** (2.3×) | + +Schema v15 migrated the live 171 MB DB on start (17 443 rows indexed). All 793 +sizes converge to exactly their pre-change values, checked against a `.backup` +copy taken before any of this landed. The +59-byte ciphertext delta is a precise +probe for a provisional size, and is what the attr-invalidation follow-up should +be tested with. + +### Original cause-2 ruling-out (kept — all still true) + +Backing cause 1 out of the totals leaves per-node non-FTS cost at **6.2 ms at +n=484 but 42 ms at n=793**. Something gets ~7× more expensive per node, sharply, +somewhere around 500–800 entries. + +Ruled out so far: + +- *Chunking / network.* `MAX_BATCH_COUNT = 150`, so cost would rise in visible + steps at 150/300/450; it is flat at 12.5 ms/entry straight through 484. The + marginal cost would imply ~16 s per POST, which is absurd. +- *Composition.* Both the 484 and 793 folders are 100 % files, no subfolders. +- *`FOLDER_KEY_CACHE_CAP` (512).* Suspicious number, but `folder_keys` only holds + *folder* keys and these children are all files, so nothing is inserted during + the walk. `resolve_parent_key_ctx` caches each ancestor it derives, and all + children share one parent, so it is a hit after the first child. +- *The entity cache.* `InMemoryCacheRepository` is `HashMap`-backed, O(1) `set`. +- *FTS (cause 1).* Linear — 3.1 s → 5.3 s where the cliff is 6.1 s → 38.8 s. + +**Next step:** symbol-level profile. Needs an unstripped `pdfs` installed and the +daemon restarted, then `perf record -p ` across a 793-entry enumerate. Every +cheaper avenue above has been spent. Note a second daemon is **not** an option +for this: Proton refresh tokens are single-use, so a parallel session would fight +the running daemon for them. + +**Do not implement improvements.md P2.8 before this is understood.** Pipelining +would parallelize the cliff rather than remove it, and at 793 entries the cliff is +33 s of the 38.8 s — far more than concurrency could win back. + +--- + +## B13 — `rename` over an existing destination fails instead of replacing it + +**Status:** Fixed (unverified — needs a real rsync against the rebuilt daemon) +**Found:** 2026-07-20, user ran `rsync -rauLP ~/ProtonDrive/Music/... ~/Music/` +(both are protondrive mounts) and every single file failed at the end of its +transfer: + +``` +rsync: [receiver] rename ".../.Buunshin - heimwee (Original Mix).wav.b0akm3" + -> ".../Buunshin - heimwee (Original Mix).wav": Input/output error (5) +``` + +**Where:** `crates/pdfs-fuse/src/lib.rs`, `Filesystem::rename` (~4380) + +Daemon log, one per failed file: + +``` +ERROR pdfs_fuse: rename failed uid=… error=proton api error AlreadyExists (http 422): + A file or folder with that name already exists +WARN pdfs_fuse::drain: pending upload failed; will retry uid=… attempts=1 + error=proton api error DoesNotExist (http 422): File or folder not found +``` + +**Cause:** `rename(2)` is specified to *atomically replace* an existing +destination. Our handler just calls `client.rename_node`, and Proton refuses a +name that already exists — so the 422 becomes a blanket `EIO`. Nothing in the +path ever looks up the destination name. + +This breaks every write-to-temp-then-rename tool, which is most of them: rsync, +editors doing atomic saves, `mv -f`, package managers. The failure is at the +*end* of the transfer, so the bytes are uploaded and then thrown away — the user +pays full upload cost for nothing. + +**Second-order damage:** the failed rename leaves the temp node behind and its +queued upload then fails with `DoesNotExist` and retries forever. So each failed +file also leaves a poisoned entry in the drain queue. + +**Fix:** `rename` now looks up `newname` under `newparent` and, if it resolves to +a different node, removes it before the API call. + +The order is forced by Proton: the name has to be free before `rename_node` will +take it, so it is trash-then-rename, which means the operation is **not atomic**. +`Core::remove_replaced` does the trashing and `Core::restore_replaced` puts the +node back if the rename then fails — on every failing path, including the queued +ones. If the restore *also* fails the node stays in the trash and says so +loudly, because the alternative is a file the user believes was merely renamed +sitting somewhere they will not look. + +Deliberate details: + +- **Refusals happen before anything is trashed.** `check_replaceable` is a pure + function for exactly this reason: every `Err` it returns is a case where the + destination must survive, and a mistake there turns a refusal into deletion. + `EISDIR` / `ENOTDIR` when the two ends disagree about being a directory, and + `ENOTEMPTY` for a non-empty destination directory — Proton trashes a folder + with its whole subtree, so allowing that would discard every file underneath + without ever naming them. +- **`RenameFlags` is now read** (it was `_flags`). `RENAME_NOREPLACE` returns + `EEXIST` instead of replacing; `RENAME_EXCHANGE` returns `EINVAL`, since no + Proton primitive swaps two names and emulating it would leave a window in + which one of them does not exist. +- A node whose own create is still queued never reached the server, so replacing + it just discards its queued ops — no API call, and it works offline. +- Renaming a node onto its own name stays a no-op rather than a self-replace. + +**Tests:** `replace_tests` in `pdfs-fuse/src/lib.rs` covers all five decisions. +The trash/restore half needs a live server and is not covered. + +**Repro:** `cd && echo a > x && echo b > y && mv y x` → was EIO, should +now succeed with `x` holding `b`. + +--- + +## B14 — provisional (ciphertext) sizes make rsync read short and abort the file + +**Status:** Fixed (unverified — needs a cold folder on the rebuilt daemon) +**Found:** 2026-07-20, same rsync run as B13. Alongside the receiver errors, the +sender failed to read its own source files: + +``` +rsync: [sender] read errors mapping "/home/narl/ProtonDrive/Music/…/ + Buunshin - i think i feel... (Original Mix).wav": No data available (61) +``` + +**Where:** `node_size` (`pdfs-fuse/src/lib.rs:3480`) + `Core::spawn_size_upgrade` + +**Cause:** this is B12's known tradeoff surfacing as a real transfer failure. +Until the background size upgrade lands, `node_size` falls back to +`total_size_on_storage` — the **ciphertext** size, which is larger than the +plaintext (measured at +59 bytes on this account). B12 called that "cosmetic and +self-corrects" because it only affects `ls -l`. + +It does not only affect `ls -l`. `ENODATA` (61) is what **rsync sets on a short +read**: `map_ptr` in `fileio.c` asks for the bytes `stat` promised, gets fewer, +and marks the mapping `ENODATA`. So a reader that trusts `st_size` — rsync, +anything using `mmap`, `sendfile`, or a sized `read` loop — sees a truncated +file and errors out. That is the same class of bug as B11, just from the other +direction: B11 reported **too small** (0) so the kernel skipped reads entirely; +this reports **too large** so reads run off the end. + +The 30 s attr TTL makes the window much wider than the upgrade's own latency +(B12 documented this), so a cold `rsync` of a large tree is essentially +guaranteed to hit it. + +**Why it looked fine afterwards:** re-listing the same directory now shows every +size correct, because the upgrade has long since landed. The bug is only visible +cold. Reproducing it needs `pdfs refresh ` immediately before the read. + +**Fix:** a provisional size is never published. `getattr` on a file whose +`claimed_size` is unknown now resolves it *before* replying, instead of replying +with the ciphertext size and upgrading afterwards. + +The cost is one batched round trip per folder, not one per file: `ls -l` is one +`getattr` per entry, and they collapse onto a single upgrade. B12's split is +still doing its job — a plain `ls` never reaches this path at all, which is +where the 8.7× came from. + +That required making the upgrade *awaitable*, which is most of the change: + +- `size_upgrades` went from `HashSet` to `HashMap>`, + a condvar per folder. Followers wait on it; there may be hundreds for one + folder, so it releases all of them. +- **The leader does the fetch on its own thread rather than handing it to a + worker.** This is the part to not undo: `getattr` waits on `Lane::Meta`, so a + leader that queued its fetch onto that same lane could have a wide enough + `ls -l` fill the lane with threads waiting for a job that can never be + scheduled. +- `SizeUpgrade::WAIT` caps the wait at 10 s, falling back to the provisional + size. A `stat` that never returns is worse than one that is briefly wrong. +- `upgrade_sizes` owns the single-flight bookkeeping and has one exit path; + `apply_size_upgrade` holds the body it used to inline, so a failed fetch still + releases the waiters and leaves the folder retryable. + +**Tests:** `size_upgrade_tests` covers the follower being released, a waiter +arriving after `finish` (it checks the flag, not the notification, which it +would miss), and all waiters waking. + +**Not covered, worth watching:** `Lane::Meta` can now hold waiting `getattr`s. +There is no deadlock — leaders never queue their own work — but a cold `ls -l` +across many folders at once could occupy the lane. If metadata ops start +feeling sticky under heavy cold listing, this is the first suspect. + +### Verified (2026-07-20) — correct, but expensive, and not completely closed + +Two further holes turned up during verification; both are fixed, and the second +is the one that mattered. + +1. **`upgrade_sizes_for_parent` early-returned** when the parent listing was not + resident in `children` — and a rename invalidates exactly that, so a freshly + renamed file always landed in the dead branch. `stat` read 67 bytes for a + 16-byte file, and `cat` returned the 16 real bytes followed by 51 NULs. The + same shape as B4: an early return that assumed the hot cache was + authoritative. Now falls back to resolving the single node, keyed by its own + inode. +2. **`lookup` replies with attrs and a TTL too.** With no `readdirplus`, `ls -l` + is one `lookup` per entry and *zero* `getattr` calls — so fixing only + `getattr` fixed a path `ls -l` never takes. This is worth remembering: the + first fix looked right and did nothing for the reported symptom. + `serve_lookup` now resolves as well, taking an `off_loop` flag because + `lookup`'s warm path runs on the dispatch loop where it may not block (B5). + +| measurement | before fix | after fix | +|---|---|---| +| cold `ls -l`, 7-file wav folder | ciphertext sizes | **exact settled sizes** | +| renamed file `stat` / `cat` | 67 B, 51 NULs | **16 B, clean** | +| cold plain `ls`, 793 entries | 4.48 s | **4.58 s** (B12 intact) | +| cold `ls -l`, 793 entries | ~4.5 s, wrong sizes | **85.8 s**, 785/793 correct | + +### The cost, and the part still open + +**A cold `ls -l` of a 793-entry folder went from ~4.5 s to 85.8 s.** That is not +a flaw in the batching — it is B12's cause 2 arriving on schedule. A real +`claimed_size` requires the per-file node-key unlock, i.e. one S2K per file, and +that work is single-threaded. B12 removed it from the *listing* path; asking for +sizes puts it back, because sizes are what it produces. Plain `ls` is untouched, +which is why the split is still worth having. + +**8 of 793 entries still came back provisional**, caught by diffing the cold +listing against a settled one. Those are `SizeUpgrade::WAIT` timeouts: the leader +takes ~80 s for the batch and a waiter gives up at 10 s. So the bug is rarer but +not gone — it now needs a folder large enough that the batch outruns the cap. + +### Per-chunk wakeup (done, unverified) + +The timeout gap is closed by releasing waiters **per chunk** instead of once at +the end. `SizeUpgrade` carries a generation counter; `run_size_upgrade` fetches +in `SIZE_UPGRADE_CHUNK` (150, the SDK's own `MAX_BATCH_COUNT`, so one chunk is +one request), applies each chunk, and bumps the generation. A waiter re-checks +whether *its own* node now has a real size and returns if so — it no longer +waits on the other 792. + +Two structural points worth keeping: + +- **The batch runs on its own thread, not the `Workers` pool.** Callers wait on + `Lane::Meta`, so queueing the batch there could let a wide `ls -l` fill the + lane with threads waiting for a job that can never be scheduled. + `Lane::Transfer` would swap that deadlock for starvation behind bulk reads. + One short-lived thread per folder, bounded by the single-flight, avoids both. +- **`wait_for` evaluates its predicate with no `SizeUpgrade` lock held.** The + predicate reads `state`, and the applying thread holds `state` before it + signals; taking them in the other order would close the cycle. + +This does **not** make a full cold `ls -l` faster — every size still has to be +computed. It makes each individual `stat` return after its own chunk rather than +after the whole folder, and removes the provisional-size fallback that the 10 s +cap was producing. + +**Tests:** `size_upgrade_tests` covers the resolving chunk releasing a waiter, +an unrelated chunk *not* releasing one, `finish` releasing an unresolved waiter, +the already-resolved and post-`finish` arrivals, all waiters waking, and the +timeout backstop still firing. + +--- + +## Plan: parallelise the per-file S2K (the 85 s) + +This is the only item that reduces the work rather than redistributing it, and +it is the same bottleneck as improvements.md PERF #5 — approached from sizes +rather than from cold navigation. Filed here because B12/B14 are what produced +the measurements it needs. + +**The claim to be tested first, before any code.** B12's profile of a 793-entry +cold enumerate attributed **64.4 % of cycles to `sha2::sha256::x86::digest_blocks` +and ~74 % overall to SHA-256 inside PGP's S2K**, i.e. the per-file node-key +unlock in `build_node` under `NodeDetail::Full`. If that still holds for the +*size upgrade* path specifically — it is the same `enumerate_nodes` call, so it +should — then the work is CPU-bound, embarrassingly parallel across files, and +currently serialised on one thread. + +**Step 0 — confirm, do not assume.** Re-profile against the size-upgrade path +now that it is the thing on the critical path: + +``` +perf buildid-cache -a target/release/pdfs # symbols; see B12's note on strip +perf record -p -- sleep 90 +# meanwhile: pdfs refresh Music/aC_ID.dll && ls -l Music/aC_ID.dll +``` + +Expect the same SHA-256 leaf. If it is *not* dominant, stop — the rest of this +plan is aimed at the wrong thing. + +**Step 1 — establish the ceiling.** The upgrade is `enumerate_nodes` over 793 +uids. Measure single-node unlock cost directly (log the S2K iteration count and +elapsed time per file across one folder — this also settles the question B12 left +open, whether the 49 ms/entry folder is expensive because of *n* or because the +uploading client chose a costlier S2K). Ideal speedup is bounded by core count +and by how uneven those per-file costs are. + +**Step 2 — move the crypto off the async runtime.** The decryption currently runs +inline in the SDK's `enumerate_nodes` future. Blocking CPU work on a Tokio worker +starves everything else sharing that runtime. Wrap the per-node unlock in +`spawn_blocking`, or hand the whole chunk to a `rayon` pool and await the result. +This is the change PERF #5 describes and is a **`proton-drive-rs` change, not a +`pdfs` one** — which is why it was deferred: it needs an SDK release, and the +workspace pins `"0.1"` so a `cargo update` picks it up. + +**Step 3 — parallelise across files within a chunk.** 150 nodes per chunk, each +independent once the parent key is resolved. `resolve_parent_key_ctx` caches the +ancestor key and all children of a folder share one parent, so the parallel +region needs read-only access to it — take the folder key once, then fan out. +Watch `FOLDER_KEY_CACHE_CAP`/`folder_keys` (PERF #9 made it an LRU) for +contention if the fan-out takes that lock per node. + +**Step 4 — re-measure the same three folders**, which have before/after numbers +already recorded above and in B12: + +| folder | cold plain `ls` | cold `ls -l` (today) | +|---|---|---| +| `Music/aC_ID.dll` (793) | 4.58 s | 85.8 s | +| `InstantUpload/Camera` (484) | 2.91 s | not measured | +| `Pictures/old` (417) | 2.25 s | not measured | + +Fill in the middle column's `ls -l` first — there is only one data point for the +regression right now, and B12's "cliff past ~500" framing was wrong precisely +because it generalised from a single folder. **Do not repeat that mistake here.** + +**What success looks like:** cold `ls -l` on the 793-entry folder within ~2–3× of +the plain `ls`, rather than 19×. Perfect scaling is not available — the request +round trips are serial per chunk and only the crypto parallelises. + +**Explicitly out of scope:** `readdirplus`. It would cut the *number* of calls +(one reply per directory instead of one `lookup` per entry) and is the right +long-term shape for the directory-read path, but it does not remove a single S2K +— the sizes still have to be computed. It is a separate change and should not be +bundled in, for the same reason B5 declined it: it would hide this bug rather +than fix it. + +**Do not** attempt to derive the plaintext size from `total_size_on_storage` +instead. The delta is not a constant (+564 B on a 39 MB file, +51 B on a 16-byte +one — it scales with block count), so the arithmetic would have to model Proton's +block framing exactly, and being wrong in the *small* direction reproduces B11, +where a short size made the kernel skip reads entirely. + +**Verify with:** the ciphertext delta is the probe (+564 B on a 39 MB file, ++51 B on a 16-byte one — it scales with block count, so it is not a constant). +`pdfs refresh` a folder, then `ls -l` immediately and diff against a settled +listing. + +--- + +## B15 — an empty-but-`listed` folder, and duplicated folder uids + +**Status:** Open (side finding, unattributed). Originally filed as "a sync-folder +listing came back empty"; that part was investigated and retracted — see below. +**Found:** 2026-07-20, while confirming B13. + +`ls ~/Music/Buunshin/not everythin is your fault (wav)/` returns `total 0` — no +entries — even though the rename failures in B13 prove files existed under those +names minutes earlier (Proton rejected the rename *because* the name was taken). +The same folder read through the primary mount +(`~/ProtonDrive/Music/Buunshin/…`) lists all seven `.wav` files correctly. + +**Checked (2026-07-20) — the "stale empty listing" reading was wrong.** The +folder `not everythin is your fault (wav)` that read empty has **`listed = 0`** +in `nodes`, so its `ls` was a real network enumeration, not a cached empty one. +The files genuinely are not there: every rename in B13 failed and rsync +discarded its temp files. B15 as originally filed does not exist. + +**What the query did turn up, and is worth keeping:** + +1. **A real B1-shaped row.** `not everything is your fault` (no `(wav)`) reads + `listed = 1, trashed = 0` with **zero child rows** — the exact + empty-but-listed state B1 was about, still present in the live DB, reached by + some route the B1 fix does not cover. This one is worth chasing. +2. **Duplicated folders.** `Music` and `Buunshin` each exist under **two + distinct uids**, and the `(wav)` folder name appears twice as well — one copy + populated (7 children), one empty. So the remote tree has picked up duplicate + directories somewhere. Related to the self-conflict duplication already + recorded in memory, but not yet attributed here. + +Both are folder-identity problems rather than the mount disagreement originally +suspected. Renamed scope accordingly; the "two mounts disagree" framing is +retracted.