Replace a damaged catalog on the server instead of pinning it there

The catalog sync refuses to upload when it cannot read the server's copy,
because the upload is a read-modify-write and writing blind would discard
another device's collections. That is the right rule for a timeout, a
dropped connection or a newer schema — the remote is fine, only our view
of it failed.

A file SQLite calls malformed is not that. No device will ever read it
again, so refusing to write over it preserves nothing — and every client
declines in turn, pinning the damaged file in place for good. Collections
and people then stop crossing between devices on all of them at once,
each logging "catalog not pushed" on every pass. This library did exactly
that from 2026-09-07, on the desktop and on a freshly installed phone
alike, while 32 collections sat undelivered.

Now a copy that arrived whole and still will not open is set aside under
a dated name and replaced by ours. Whole is checked against the size the
server advertises: a truncated download will not open either, and on a
phone that is the far likelier story, so anything short — or any size the
listing cannot confirm — is treated as the transport failure it is and
the server's copy is left alone. A placeholder's size is not trusted for
the comparison, since it means nothing.

The report says when this happened, and the log line calls it "pushed
over a damaged copy" rather than folding it into an ordinary push: it is
the one push that discarded something.

ui/dr-ui/src/derived_sync.rs

@@ -52,6 +52,13 @@ pub struct SyncReport {
     pub thumbnails_adopted: usize,
     pub catalog_uploaded: bool,
     pub catalog_merged: bool,
+    /// The copy on the server was damaged beyond reading, and ours replaced it.
+    ///
+    /// Counted because it is the one outcome here that *destroys* something. A
+    /// sync that silently overwrote a peer's collections would be the bug this
+    /// whole path is written to avoid, so when it does happen the report has to
+    /// be able to say so rather than looking like an ordinary push.
+    pub catalog_replaced: bool,
     pub collections_gained: usize,
     /// Membership rows the merge brought in (FR-CAT-7).
     ///
@@ -88,6 +95,7 @@ impl SyncReport {
             || self.shards_downloaded > 0
             || self.catalog_uploaded
             || self.catalog_merged
+            || self.catalog_replaced
             || self.face_shards_uploaded > 0
             || self.face_shards_downloaded > 0
             || self.place_adopted
@@ -636,6 +644,22 @@ async fn sync_catalog(
                         report.collections_gained = merge.inserted + merge.updated;
                         report.members_gained = merge.members_added;
                     }
+                    // TRACES: FR-NC-9
+                    // Damaged beyond reading, and the whole file arrived. See
+                    // [`replace_corrupt_remote`] for why this one failure is
+                    // the exception to "never write over what you could not
+                    // read": there is nothing left in it to preserve, and
+                    // refusing for ever is what pinned a damaged file in place
+                    // on every device for a week.
+                    Err(dr_catalog::CatalogError::Corrupt { detail }) => {
+                        let _ = std::fs::remove_file(&downloaded);
+                        if !replace_corrupt_remote(backend, base, remote_name, &bytes, &detail)
+                            .await
+                        {
+                            return Ok(());
+                        }
+                        report.catalog_replaced = true;
+                    }
                     // Unreadable is not the same as absent: it may be a newer
                     // format, or a torn upload. Ours must not go over it.
                     Err(e) => {
@@ -851,6 +875,117 @@ async fn read_derived(
     }
 }
 
+/// TRACES: FR-NC-9
+/// Set a damaged remote catalog aside so ours can replace it.
+///
+/// # Why this is allowed to destroy something
+///
+/// Everything else in [`sync_catalog`] refuses to upload when it could not read
+/// the server's copy, because the upload is a read-modify-write and writing
+/// blind discards another device's collections. That rule is right for every
+/// failure it was written for — a timeout, a dropped connection, a newer schema
+/// — because in all of them the remote is *fine* and only our view of it
+/// failed.
+///
+/// A file SQLite calls malformed is not that. It is not a view that failed; it
+/// is a file whose contents no device can ever read again. Refusing to write
+/// over it preserves nothing, and every client then declines in turn: the
+/// damaged copy is pinned in place for ever, and collections and people stop
+/// crossing between devices silently, on all of them at once. That is not
+/// theoretical — it is what this library did from 2026-09-07, on the desktop
+/// and on a freshly installed phone alike, both logging "catalog not pushed"
+/// on every pass for a week while 32 collections sat undelivered.
+///
+/// # What makes it safe
+///
+/// **The whole file has to have arrived.** A truncated download is also
+/// unreadable, and it is the far more likely story on a phone — this library's
+/// logs are full of aborted bodies and DNS failures. So the size the server
+/// advertises is compared against what we actually received, and anything short
+/// is treated as the transport failure it is. Only a complete file that still
+/// will not open is judged damaged.
+///
+/// **Nothing is deleted.** The damaged bytes are uploaded beside the catalog
+/// under a dated name first, and the replacement only proceeds once that has
+/// landed. If some later build learns to salvage collections out of a damaged
+/// catalog, the file is still there to salvage them from.
+///
+/// Returns whether the caller may now push over the remote.
+async fn replace_corrupt_remote(
+    backend: &dyn RemoteBackend,
+    base: &RemotePath,
+    remote_name: &str,
+    bytes: &[u8],
+    detail: &str,
+) -> bool {
+    let advertised = match remote_size(backend, base, remote_name).await {
+        Some(n) => n,
+        None => {
+            log::warn!(
+                "not pushing the catalog: the copy on the server will not open ({detail}), \
+                 but its size could not be confirmed, so it may simply have arrived short"
+            );
+            return false;
+        }
+    };
+
+    if advertised != bytes.len() as u64 {
+        log::warn!(
+            "not pushing the catalog: the copy on the server will not open ({detail}), \
+             but only {} of {advertised} bytes arrived — that is a truncated download, \
+             not a damaged file, so the server's copy is left alone",
+            bytes.len()
+        );
+        return false;
+    }
+
+    let stamp = std::time::SystemTime::now()
+        .duration_since(std::time::UNIX_EPOCH)
+        .map(|d| d.as_secs())
+        .unwrap_or(0);
+    let aside_name = format!("catalog.corrupt-{stamp}.sqlite");
+    let aside = RemotePath::new(format!("{}/{aside_name}", base.as_str()));
+
+    if let Err(e) = backend.put(&aside, bytes.to_vec(), None).await {
+        log::warn!(
+            "not pushing the catalog: the copy on the server is damaged ({detail}), \
+             but it could not be set aside as {aside_name} ({e}), and it will not be \
+             overwritten until a copy of it is safe"
+        );
+        return false;
+    }
+
+    log::warn!(
+        "the catalog on the server is damaged ({detail}) and all {advertised} bytes of it \
+         arrived, so no device can read it. Kept as {aside_name}; replacing it with this \
+         device's copy, which is what lets collections and people sync again"
+    );
+    true
+}
+
+/// The size the server says an entry in the derived folder has.
+///
+/// One listing of a folder that holds a handful of files, rather than a HEAD
+/// the backend trait does not offer. `None` covers "the listing failed", "it is
+/// not there", and "the size it reports means nothing" alike, and the caller
+/// treats all of them as not knowing — which is the answer that declines to
+/// overwrite.
+///
+/// A placeholder is excluded rather than trusted: `RemoteEntry::size` is
+/// explicitly not meaningful when `materialised` is false — a suffix-mode stub
+/// is one byte and carries no record of what it stands for — so comparing a
+/// download against it would be comparing against nothing.
+async fn remote_size(backend: &dyn RemoteBackend, base: &RemotePath, name: &str) -> Option<u64> {
+    backend
+        .list(base, None)
+        .await
+        .ok()?
+        .into_iter()
+        .find(|e| e.kind == dr_sync::EntryKind::File && e.path.name() == name)
+        .filter(|e| e.materialised)
+        .map(|e| e.size)
+}
+
 fn shard_name(client: &str, id: u32) -> String {
     format!("shard-{client}-{id:04}.sqlite")
 }
@@ -979,6 +1114,10 @@ mod derived_guard_tests {
         /// record went up rather than only that one did.
         last_put: Arc<std::sync::Mutex<Vec<u8>>>,
         caps: dr_sync::Capabilities,
+        /// The size `list` claims `catalog.sqlite` has, if it lists it at all.
+        /// This is what says whether a body that will not open is a damaged
+        /// file or merely a short download.
+        advertise: Option<u64>,
     }
 
     impl Fussy {
@@ -1000,6 +1139,7 @@ mod derived_guard_tests {
                     puts: puts.clone(),
                     last_put: last_put.clone(),
                     caps: dr_sync::Capabilities::minimal(),
+                    advertise: None,
                 },
                 puts,
                 last_put,
@@ -1017,10 +1157,23 @@ mod derived_guard_tests {
         }
         async fn list(
             &self,
-            _dir: &RemotePath,
+            dir: &RemotePath,
             _since: Option<&dr_sync::Validator>,
         ) -> Result<Vec<dr_sync::RemoteEntry>, RemoteError> {
-            Ok(Vec::new())
+            let Some(size) = self.advertise else {
+                return Ok(Vec::new());
+            };
+            let path = RemotePath::new(format!("{}/catalog.sqlite", dir.as_str()));
+            Ok(vec![dr_sync::RemoteEntry {
+                id: RemoteId::Path(path.clone()),
+                path,
+                kind: dr_sync::EntryKind::File,
+                validator: dr_sync::Validator::new("v"),
+                size,
+                modified: None,
+                has_preview: false,
+                materialised: true,
+            }])
         }
         async fn dir_validator(
             &self,
@@ -1135,6 +1288,70 @@ mod derived_guard_tests {
         assert!(report.catalog_uploaded);
     }
 
+    // --- a damaged copy on the server (FR-NC-9) --------------------------
+    //
+    // The one exception to everything above. A file SQLite calls malformed is
+    // not a read that failed; it is a file no device will ever read again, and
+    // leaving it alone pins it there for every client at once.
+
+    /// A body that is definitely not a SQLite database, with a chosen size the
+    /// listing will or will not agree with.
+    async fn corrupt_remote(
+        body_len: usize,
+        advertised: Option<u64>,
+        name: &str,
+    ) -> (usize, SyncReport, Vec<u8>) {
+        let (catalog_path, scratch) = fixture(name);
+        let (mut backend, puts, last) = Fussy::serving(None, vec![0xAB; body_len]);
+        backend.advertise = advertised;
+        let mut report = SyncReport::default();
+        sync_catalog(
+            &backend,
+            &RemotePath::new(".darkroom-derived"),
+            &catalog_path,
+            &scratch,
+            &mut report,
+        )
+        .await
+        .unwrap();
+        let sent = last.lock().unwrap().clone();
+        (puts.load(Ordering::SeqCst), report, sent)
+    }
+
+    #[tokio::test]
+    async fn a_damaged_catalog_that_arrived_whole_is_set_aside_and_replaced() {
+        // The deadlock this exists to break: every device downloads the same
+        // unreadable file, every device declines to overwrite it, and
+        // collections and people stop crossing between devices for ever.
+        let (puts, report, _) = corrupt_remote(64, Some(64), "corrupt-whole").await;
+        assert_eq!(puts, 2, "the damaged copy is kept, then ours goes over it");
+        assert!(report.catalog_replaced, "and the report says what happened");
+        assert!(report.catalog_uploaded);
+        assert!(!report.catalog_merged, "there was nothing to merge");
+    }
+
+    #[tokio::test]
+    async fn a_short_download_is_a_truncated_transfer_not_a_damaged_file() {
+        // The failure that matters most to get right. A phone on a flaky link
+        // aborts bodies constantly, and a partial download will not open
+        // either — treating that as damage would let one bad connection
+        // destroy a catalog the server was holding perfectly well.
+        let (puts, report, _) = corrupt_remote(64, Some(4096), "corrupt-short").await;
+        assert_eq!(puts, 0, "nothing is written over a copy that arrived short");
+        assert!(!report.catalog_replaced);
+        assert!(!report.catalog_uploaded);
+    }
+
+    #[tokio::test]
+    async fn a_size_the_server_will_not_confirm_leaves_the_copy_alone() {
+        // Not knowing is not the same as knowing it is whole. Without a size
+        // to compare against there is no way to tell damage from truncation,
+        // and the answer to "I cannot tell" has to stay "do not overwrite".
+        let (puts, report, _) = corrupt_remote(64, None, "corrupt-unconfirmed").await;
+        assert_eq!(puts, 0);
+        assert!(!report.catalog_replaced);
+    }
+
     // --- the place (FR-UI-8) ---------------------------------------------
     //
     // The same "do not write over what you could not read" rule as above, for a

ui/dr-ui/src/library_ui.rs

@@ -4286,10 +4286,14 @@ fn start_derived_sync(window: &AppWindow, ctl: &Rc<LibraryController>) {
                             report.shards_uploaded,
                             report.shards_downloaded,
                             report.thumbnails_adopted,
-                            if report.catalog_uploaded {
+                            // Replacing a damaged copy is said apart from an
-                                "pushed"
+                            // ordinary push: it is the one push that discarded
-                            } else {
+                            // something, and a log line that called it "pushed"
-                                "not pushed"
+                            // would hide the only moment worth going back to.
+                            match (report.catalog_uploaded, report.catalog_replaced) {
+                                (true, true) => "pushed over a damaged copy",
+                                (true, false) => "pushed",
+                                _ => "not pushed",
                             },
                             if report.collections_gained > 0 {
                                 format!(", {} collection(s) gained", report.collections_gained)