diff --git a/crates/pdfs-core/src/db/mod.rs b/crates/pdfs-core/src/db/mod.rs index 611e0e3..1752e27 100644 --- a/crates/pdfs-core/src/db/mod.rs +++ b/crates/pdfs-core/src/db/mod.rs @@ -216,18 +216,34 @@ impl Db { let conn = match Self::open_configured(path) { Ok(conn) => conn, Err(e) if is_corrupt(&e) => { - // Every byte in here is derived from the account or from the - // content cache directory, so the recovery is to start over — - // loudly, and keeping the damaged file for a post-mortem rather - // than deleting evidence. + // Most of what is in here is derived from the account and is + // fetched again. The queue is not: it holds writes accepted + // here that Drive does not have yet, and the mirror baselines + // and pins are this machine's alone. So the damaged file is + // moved aside, kept as evidence, and those tables are copied + // out of it into the new one as far as SQLite can still read + // them (`docs/BUGS.md` B216). tracing::error!( db = %path.display(), error = %e, - "cache database is corrupt — moving it aside and rebuilding \ - (metadata will be re-fetched; no user data is stored here)" + "cache database is corrupt; moving it aside, rebuilding, and \ + copying the upload queue, sync baselines and pins out of it" ); - quarantine_corrupt_db(path)?; - Self::open_configured(path)? + let damaged = quarantine_corrupt_db(path)?; + let conn = Self::open_configured(path)?; + for (table, result) in salvage(&conn, &damaged) { + match result { + Ok(rows) => { + tracing::warn!(table, rows, "salvaged from the damaged database") + } + Err(error) => tracing::error!( + table, + %error, + "could not salvage from the damaged database" + ), + } + } + conn } Err(e) => return Err(e), }; @@ -428,12 +444,96 @@ fn is_corrupt(e: &Error) -> bool { ) } -/// Move a damaged database (and its WAL sidecars) out of the way so the next -/// open starts from empty. +/// The tables [`salvage`] copies out of a damaged database, in order, each +/// with the rows it keeps. /// -/// Renamed rather than deleted: this is a cache, but it is also the only -/// evidence of what went wrong, and it costs one file the user can remove. -fn quarantine_corrupt_db(path: &Path) -> Result<()> { +/// The queue goes with the node rows it names by local id: a create made here +/// has no other record of its node, and a real node keeps its local id when its +/// folder is listed again. Those rows come back unlisted, so their folders are +/// fetched again rather than trusted. Claims are cleared by [`Db::open`] as +/// after any other crash. +const SALVAGE: &[(&str, &str)] = &[ + ( + "nodes", + "lid IN (SELECT lid FROM old.pending_op WHERE lid IS NOT NULL + UNION SELECT parent_lid FROM old.pending_op WHERE parent_lid IS NOT NULL) + OR uid LIKE 'local~%'", + ), + ("pending_op", "1"), + ("sync_folder", "1"), + ("sync_entry", "1"), + ("pins", "1"), + ("own_sealed_rev", "1"), +]; + +/// Copy what only this machine has out of the damaged database at `damaged` +/// into the fresh one `conn` just created. Returns how each table went. +/// +/// Each table is copied in its own transaction, so a table SQLite cannot read +/// to the end is left out whole rather than half copied, and does not stop +/// the others. Only the columns both files have are copied, which covers a +/// file that broke in the middle of an upgrade. The damaged file is opened read +/// only and left as it was. +fn salvage(conn: &Connection, damaged: &Path) -> Vec<(&'static str, Result)> { + let uri = format!( + "file:{}?mode=ro", + damaged + .to_string_lossy() + .replace('%', "%25") + .replace('?', "%3f") + .replace('#', "%23") + ); + if let Err(e) = conn.execute("ATTACH DATABASE ?1 AS old", [&uri]) { + let e = Error::Db(e); + tracing::error!(error = %e, "could not open the damaged database to salvage it"); + return Vec::new(); + } + let results = SALVAGE + .iter() + .map(|&(table, rows)| (table, salvage_table(conn, table, rows))) + .collect(); + let _ = conn.execute("UPDATE nodes SET listed = 0", []); + if let Err(e) = conn.execute("DETACH DATABASE old", []) { + tracing::warn!(error = %e, "could not detach the damaged database"); + } + results +} + +fn salvage_table(conn: &Connection, table: &str, rows: &str) -> Result { + let columns = |schema: &str| -> Result> { + let mut stmt = conn.prepare(&format!("PRAGMA {schema}.table_info({table})"))?; + let names = stmt + .query_map([], |r| r.get::<_, String>(1))? + .collect::>>()?; + Ok(names) + }; + let new = columns("main")?; + let shared: Vec = columns("old")? + .into_iter() + .filter(|c| new.contains(c)) + .map(|c| format!("\"{c}\"")) + .collect(); + if shared.is_empty() { + return Ok(0); + } + let shared = shared.join(", "); + let tx = conn.unchecked_transaction()?; + let copied = tx.execute( + &format!( + "INSERT OR IGNORE INTO main.{table} ({shared}) SELECT {shared} FROM old.{table} WHERE {rows}" + ), + [], + )?; + tx.commit()?; + Ok(copied) +} + +/// Move a damaged database (and its WAL sidecars) out of the way so the next +/// open starts from empty. Returns where the database itself went. +/// +/// Renamed rather than deleted: it is the only evidence of what went wrong, +/// and [`salvage`] reads what only this machine has back out of it. +fn quarantine_corrupt_db(path: &Path) -> Result { let stamp = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .map(|d| d.as_secs()) @@ -457,7 +557,9 @@ fn quarantine_corrupt_db(path: &Path) -> Result<()> { Err(e) => tracing::warn!(file = %from.display(), error = %e, "could not quarantine"), } } - Ok(()) + let mut kept = path.as_os_str().to_owned(); + kept.push(format!(".corrupt-{stamp}")); + Ok(PathBuf::from(kept)) } #[cfg(test)] diff --git a/crates/pdfs-core/src/db/tests.rs b/crates/pdfs-core/src/db/tests.rs index 324378e..8be06bc 100644 --- a/crates/pdfs-core/src/db/tests.rs +++ b/crates/pdfs-core/src/db/tests.rs @@ -1283,6 +1283,89 @@ fn a_corrupt_database_is_moved_aside_and_rebuilt() { let _ = std::fs::remove_file(path.with_extension("db.lock")); } +/// A rebuilt database must not lose what only this machine has: writes Drive +/// has not received, the mirror baselines and the pins (`docs/BUGS.md` B216). +#[test] +fn a_rebuild_keeps_the_queue_baselines_and_pins_of_the_damaged_file() { + let id = now_test_id(); + let damaged = + std::env::temp_dir().join(format!("pdfs-salvage-old-{}-{id}.db", std::process::id())); + let fresh = + std::env::temp_dir().join(format!("pdfs-salvage-new-{}-{id}.db", std::process::id())); + + let db = Db::open(&damaged).unwrap(); + db.upsert_node(&folder("root", None, "My Files")).unwrap(); + db.upsert_node(&file("listed", "root", "listed.txt", 1)) + .unwrap(); + db.upsert_node(&file("queued", "root", "queued.txt", 2)) + .unwrap(); + db.set_listed(&uid("root"), true).unwrap(); + db.enqueue_op(&PendingOp { + id: 0, + kind: OP_REVISION.to_string(), + uid: uid("queued").to_string(), + parent_uid: None, + name: None, + blob_path: Some("/staging/queued".into()), + meta_json: None, + created_at: 1, + attempts: 3, + last_error: None, + next_attempt_at: 0, + }) + .unwrap(); + let folder_id = db + .sync_folder_add("/home/me/Docs", "vol~docs", "share") + .unwrap(); + db.sync_entry_upsert( + folder_id, + &StoredSyncEntry { + rel_path: "a.txt".into(), + remote_uid: Some("vol~a".into()), + local_mtime: 5, + local_mtime_ns: None, + local_size: 6, + remote_rev: Some("7".into()), + remote_hash: Some("8".into()), + }, + ) + .unwrap(); + db.pin_add("vol~pinned", "/pinned", true).unwrap(); + drop(db); + + let db = Db::open(&fresh).unwrap(); + let results = salvage(&db.conn.lock(), &damaged); + assert!(results.iter().all(|(_, r)| r.is_ok()), "{results:?}"); + + let ops = db.pending_ops().unwrap(); + assert_eq!(ops.len(), 1); + assert_eq!(ops[0].uid, uid("queued").to_string()); + assert_eq!(ops[0].blob_path.as_deref(), Some("/staging/queued")); + // The op's node came with it; the rest of the tree is fetched again. + assert!( + db.node_by_uid(&uid("queued").to_string()) + .unwrap() + .is_some() + ); + assert!( + db.node_by_uid(&uid("listed").to_string()) + .unwrap() + .is_none() + ); + assert_eq!(db.sync_entries(folder_id).unwrap().len(), 1); + assert!(db.is_pinned("vol~pinned").unwrap()); + drop(db); + + for path in [&damaged, &fresh] { + for suffix in ["", "-wal", "-shm"] { + let mut file = path.as_os_str().to_owned(); + file.push(suffix); + let _ = std::fs::remove_file(std::path::PathBuf::from(file)); + } + let _ = std::fs::remove_file(path.with_extension("db.lock")); + } +} + fn local(path: &str, name: &str, is_dir: bool) -> LocalEntry { LocalEntry { path: path.into(), diff --git a/docs/BUGS.md b/docs/BUGS.md index 3794d18..201193b 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -100,6 +100,7 @@ a live account since. | Entry | Title | |---|---| +| [B216](#b216--a-damaged-database-is-rebuilt-without-the-writes-still-queued-in-it) | A damaged database is rebuilt without the writes still queued in it | | [B215](#b215--a-remote-name-holding--or--becomes-a-path-outside-the-sync-folder) | A remote name holding `/` or `..` becomes a path outside the sync folder | | [B214](#b214--deleting-a-mirror-folder-here-trashes-another-devices-new-files-in-it) | Deleting a mirror folder here trashes another device's new files in it | | [B213](#b213--a-file-edited-during-a-mirror-pass-is-deleted-because-drive-deleted-it-earlier) | A file edited during a mirror pass is deleted because Drive deleted it earlier | @@ -318,6 +319,32 @@ a live account since. Newest first. +### B216 — A damaged database is rebuilt without the writes still queued in it + +**Status:** Fixed, not verified live +**Found:** 2026-10-16, architecture audit (`FINDINGS.md` 1.7). +**Where:** `Db::open`, `salvage` and `quarantine_corrupt_db` in `crates/pdfs-core/src/db/mod.rs` + +**Repro:** Damage `cache.db` so that SQLite reports `SQLITE_CORRUPT` or `SQLITE_NOTADB` at open, +with writes still queued. + +**Cause.** Opening a damaged database moved it aside and started an empty one, logging that +"no user data is stored here". The same file holds the upload queue (`pending_op`), the mirror +baselines (`sync_entry`, `sync_folder`), the pins and the record of revisions this machine sealed. +Every queued create, rename and trash was dropped without a word. The staged blobs lost their ops, +and those of files made here could never be queued again. + +**Fix.** After the rebuild, `salvage` attaches the damaged file read only and copies those tables +back, each in its own transaction and with only the columns both files have. The queue keeps the +node rows it names by local id, so files made here keep their identity; those rows come back +unlisted, so their folders are fetched again. A table SQLite cannot read to the end is left out +and logged as an error. + +**Test:** `db::tests::a_rebuild_keeps_the_queue_baselines_and_pins_of_the_damaged_file`, +`db::tests::a_corrupt_database_is_moved_aside_and_rebuilt`. + +--- + ### B215 — A remote name holding `/` or `..` becomes a path outside the sync folder **Status:** Fixed, not verified live