Refuse a scan whose root has gone, instead of reporting it empty

FR-PLAT-AND-2, and a silent failure on both platforms. `dr_sync::scan`
stepped over a NotFound or PermissionDenied the way it does for a child
that vanished mid-walk -- correct for a child, wrong for the root, where
it ended the walk, returned Ok with nothing in it, and reported a
successful scan of a library that was no longer there.

A lost root is now its own error. The images under it are marked
Availability::Offline per FR-CAT-9 and no catalog row is deleted;
`library::persist` clears the mark per file as each one is listed again,
so a root that comes back needs no repair step.

Partly satisfied rather than closed, and the gap is worth stating.
The recovery half is real and reachable on Android today, because
`map_status` turns Nextcloud's 403 and 404 into it and Nextcloud is how
a phone actually gets a library in this build. The causes the
requirement names -- revocation, reinstall, a removed card -- are
properties of a persisted tree permission, and there is none: SAF does
not exist here, `SourceRef::Document` is constructed only in test
modules, and `LocalStorage` rejects the variant outright. When SAF
lands it becomes a third producer of this error and nothing above it
changes, which is why the discovery belongs in the connector.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-08-29 23:20:57 +02:00
co-authored by Claude Opus 5
parent 085ab766b3
commit 7418b040da
8 changed files with 478 additions and 38 deletions
+62 -8
View File
@@ -61,14 +61,18 @@ pub enum RemoteError {
/// connector for.
///
/// **Not a network failure and not an auth failure**, which is why it is
/// its own variant. A folder library whose directory has been unmounted,
/// or an account naming a backend a cut-down build was not compiled with,
/// produces a request that never leaves the process — reporting either as
/// `Network` would put the app into offline mode and tell the user their
/// connection is down, and reporting them as `AuthFailed` would send them
/// to re-enter a credential that is fine. The message names what is wrong
/// with the configuration, because that is the only thing that will fix
/// it.
/// its own variant. An account naming a backend a cut-down build was not
/// compiled with, or a path that would leave the library folder, produces
/// a request that never leaves the process — reporting either as `Network`
/// would put the app into offline mode and tell the user their connection
/// is down, and reporting them as `AuthFailed` would send them to re-enter
/// a credential that is fine. The message names what is wrong with the
/// configuration, because that is the only thing that will fix it.
///
/// A folder library whose directory is not there was once reported here
/// too, and is now [`RootUnavailable`](Self::RootUnavailable): it is not
/// something wrong with the configuration, it is the library being gone,
/// and only the second of those has a catalog to protect.
#[error("account misconfigured: {0}")]
Configuration(String),
@@ -105,6 +109,43 @@ pub enum RemoteError {
#[error("operation cancelled")]
Cancelled,
/// TRACES: FR-PLAT-AND-2 | FR-CAT-9
/// The library root itself could not be opened.
///
/// **The one failure that is about the library rather than about a file in
/// it**, and it is a separate variant because every other classification
/// of it is wrong in a way that costs the user something:
///
/// - As [`NotFound`](Self::NotFound) it is indistinguishable from a folder
/// deleted between listing its parent and reaching it, which the walk
/// correctly steps over — so a whole library going away is reported as a
/// successful scan that found nothing.
/// - As [`PermissionDenied`](Self::PermissionDenied) it inherits a message
/// about Nextcloud share permissions and sidecar writes, which is
/// accurate for the case it was written for and nonsense for a tree
/// grant the user revoked in system settings.
/// - As [`Network`](Self::Network) it would claim the connection is down,
/// which is a promise that waiting will fix it.
///
/// Today this is a Nextcloud root that answers 404 or 403 — deleted, or a
/// share withdrawn — or a folder library whose directory is not there. It
/// is also, exactly, the shape a revoked Android tree permission will have
/// when the Storage Access Framework connector FR-PLAT-AND-1 asks for
/// exists: the tree URI still stored, the permission behind it gone, every
/// read failing at the root and nowhere else. **That connector is not
/// built**, so no SAF grant can be lost yet; what this variant does is put
/// the recovery FR-PLAT-AND-2 requires in the one place all three causes
/// pass through, so the third needs no new handling above it.
///
/// The response is the same for all of them and is the point of the
/// variant: mark what the catalog holds as offline, keep every row, and
/// say which library and why (FR-CAT-9).
///
/// The string is the underlying failure, not a rewrite of it. What the
/// user is told is composed where the library's name is known.
#[error("the library folder could not be opened: {0}")]
RootUnavailable(String),
}
impl RemoteError {
@@ -150,6 +191,19 @@ impl RemoteError {
pub fn indicates_offline(&self) -> bool {
matches!(self, RemoteError::Network(_))
}
/// TRACES: FR-PLAT-AND-2
/// Whether the *library* is gone, as opposed to the server or one file.
///
/// Kept beside [`Self::indicates_offline`] because the two answer the same
/// shape of question and must not be confused. Both put the app into a
/// degraded mode that keeps working from the catalog, but they differ in
/// what the user is told and in what would end it: an offline library
/// comes back when the network does, and an unavailable root comes back
/// only when someone grants access again.
pub fn indicates_lost_root(&self) -> bool {
matches!(self, RemoteError::RootUnavailable(_))
}
}
#[cfg(test)]
+134
View File
@@ -171,6 +171,37 @@ where
let entries = match backend.list(&dir, None).await {
Ok(e) => e,
// TRACES: FR-PLAT-AND-2 | FR-CAT-9
// The root is the one directory the walk may not step over, and
// `depth == 0` is the only place it can be — nothing is ever
// pushed at that depth but the root itself.
//
// Below, a directory that has gone is a directory that went away
// between its parent being listed and it being reached, and
// continuing is right. At the root the identical error means the
// *library* is gone, and continuing is catastrophic in a way that
// is completely silent: the walk ends, the scan succeeds having
// found nothing, and the app reports a healthy library with no new
// images while every path in the catalog now points nowhere.
//
// Refused rather than reclassified. Only these two causes are —
// a `Network` failure at the root is still a network failure, and
// must stay one or an unplugged network cable would present itself
// as a revoked permission and offline mode would never engage.
Err(RemoteError::NotFound(_)) if depth == 0 => {
return Err(RemoteError::RootUnavailable(format!(
"{root} is no longer there"
)));
}
Err(RemoteError::PermissionDenied) if depth == 0 => {
// Deliberately not the variant's own message, which describes
// a Nextcloud share that refuses to *update* a sidecar. At the
// root nothing has been read at all.
return Err(RemoteError::RootUnavailable(format!(
"{root} can no longer be read"
)));
}
Err(RemoteError::NotFound(_)) => {
// Deleted between listing its parent and reaching it.
log::debug!("scan: {dir} vanished during the walk");
@@ -239,6 +270,20 @@ mod tests {
caps: Capabilities,
lists: RefCell<usize>,
probes: RefCell<usize>,
/// Directories whose listing fails, and how.
///
/// An absent directory is not enough to model this: the fake answers
/// an unknown path with an empty listing, which is exactly the shape
/// the walk must *not* confuse with a library that has gone away.
deny: HashMap<String, Deny>,
}
/// The two ways a real backend refuses a directory that is still named in
/// the catalog: it is not there, or it may not be read.
#[derive(Clone, Copy)]
enum Deny {
Missing,
Forbidden,
}
// The fake is single-threaded; tests never share it across threads.
@@ -307,8 +352,15 @@ mod tests {
},
lists: RefCell::new(0),
probes: RefCell::new(0),
deny: HashMap::new(),
}
}
/// Make one directory refuse to be listed.
fn denying(mut self, path: &str, how: Deny) -> Self {
self.deny.insert(path.to_string(), how);
self
}
}
#[async_trait]
@@ -325,6 +377,11 @@ mod tests {
_since: Option<&Validator>,
) -> Result<Vec<RemoteEntry>, RemoteError> {
*self.lists.borrow_mut() += 1;
match self.deny.get(dir.as_str()) {
Some(Deny::Missing) => return Err(RemoteError::NotFound(dir.to_string())),
Some(Deny::Forbidden) => return Err(RemoteError::PermissionDenied),
None => {}
}
Ok(self.tree.get(dir.as_str()).cloned().unwrap_or_default())
}
async fn dir_validator(&self, dir: &RemotePath) -> Result<Validator, RemoteError> {
@@ -404,6 +461,83 @@ mod tests {
assert_eq!(r.progress.directories_listed, 3);
}
/// TRACES: FR-PLAT-AND-2 | FR-CAT-9
#[tokio::test]
async fn a_root_that_is_gone_is_a_failure_and_not_an_empty_library() {
// The silent one. A vanished directory below the root is stepped over,
// and before this the root was stepped over on the same terms — which
// ended the walk immediately, returned `Ok` with nothing in it, and
// let the app report a successful scan of a library that no longer
// exists. Nothing in that path is ever told the library went away, so
// nothing marks it offline and nothing tells the user.
let b =
FakeBackend::sample(ChangeDetection::PropagatingEtags).denying("Photos", Deny::Missing);
let e = scan(
&b,
&RemotePath::new("Photos"),
&FormatFilter::all(),
&HashMap::new(),
|_| {},
)
.await
.expect_err("a library that is not there is not a library with no photographs");
assert!(e.indicates_lost_root(), "got {e:?}");
assert!(!e.indicates_offline(), "waiting will not bring this back");
assert!(e.to_string().contains("Photos"), "names the library: {e}");
}
/// TRACES: FR-PLAT-AND-2
#[tokio::test]
async fn a_root_that_may_not_be_read_reports_the_root_and_not_the_share_advice() {
// A Nextcloud share withdrawn, a directory the process may no longer
// read — and the shape a revoked Android tree grant will have when one
// can be held at all. Reported as plain `PermissionDenied` it would
// have carried that variant's message, which is several lines about a
// Nextcloud share refusing to *update* an existing sidecar: advice for
// a case where reads work, offered to a user whose reads have stopped
// entirely.
let b = FakeBackend::sample(ChangeDetection::PropagatingEtags)
.denying("Photos", Deny::Forbidden);
let e = scan(
&b,
&RemotePath::new("Photos"),
&FormatFilter::all(),
&HashMap::new(),
|_| {},
)
.await
.expect_err("a root that cannot be read is a failed scan");
assert!(e.indicates_lost_root(), "got {e:?}");
assert!(
!e.to_string().contains("sidecar"),
"the share-permission advice does not belong here: {e}"
);
}
/// TRACES: FR-PLAT-AND-2
#[tokio::test]
async fn a_folder_that_goes_away_below_the_root_is_still_stepped_over() {
// The other side of the split, and the reason the root is keyed on
// depth rather than on the error. A subfolder deleted between its
// parent being listed and it being reached is ordinary, and failing
// the scan over it would abandon every photograph beside it.
let b = FakeBackend::sample(ChangeDetection::PropagatingEtags)
.denying("Photos/2025", Deny::Missing);
let r = scan(
&b,
&RemotePath::new("Photos"),
&FormatFilter::all(),
&HashMap::new(),
|_| {},
)
.await
.expect("one folder going away is not the library going away");
assert_eq!(r.images.len(), 1, "2026 was still walked");
}
/// A library with a trash folder holding a soft-deleted image.
fn with_trash() -> FakeBackend {
let mut b = FakeBackend::sample(ChangeDetection::PropagatingEtags);