Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
160 changes: 159 additions & 1 deletion src/sync/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1556,6 +1556,23 @@ fn scan_into_node_observed(
observed: &mut HashMap<String, ContentHash>,
cache: &mut HashMap<String, ScanCacheEntry>,
) -> Result<bool> {
// A ROOT THAT IS NOT THERE IS NOT A FOLDER SOMEBODY EMPTIED.
//
// `scan_folder` returns an empty result for a missing root. Every tracked
// path would then be in the observed set and absent from the scan, which is
// the same shape as a local delete, so a delete-propagating policy would
// tombstone the entire entry and send that to every peer.
//
// A root vanishes for reasons that are not a deletion: an unmounted volume,
// a directory renamed while a pass runs, a mount that is not ready at boot.
// The blast radius is the whole entry rather than one path, so this is the
// one place where doing nothing is clearly right. Wait until we can see it.
//
// Deleting the CONTENTS of a folder still propagates normally, because the
// root survives that and the scan runs.
if !root.exists() {
return Ok(false);
}
let scanned = scan_folder(root, entry, cache)?;
// Refresh the cache from what this scan actually saw, so the next scan of an
// untouched file is free. Rebuilt rather than merged, so a vanished path
Expand Down Expand Up @@ -1625,10 +1642,33 @@ fn scan_into_node_observed(

let now = now_secs();
for path in previous.keys() {
if !current.contains_key(path) && node.local_remove(path, policy, now) {
if current.contains_key(path) {
continue;
}
// A PATH THE ENTRY NO LONGER SELECTS HAS NOT BEEN DELETED. It has left
// this entry's scope, and the two are indistinguishable from here: both
// look like "in my records, absent from my scan".
//
// Treating the first as the second removed thirteen live files from
// three machines on 2026-08-25. `plans/**` was taken out of an entry's
// include, which left its paths recorded but unscannable, and the next
// release turned delete propagation on. Every one of those files was
// recoverable from git, which is the only reason it was recoverable.
//
// Narrowing an include is a scope change. Nobody deleted anything.
if !entry.includes(path) {
continue;
}
if node.local_remove(path, policy, now) {
changed = true;
}
}
// NOT DONE HERE: dropping the excluded paths from the manifest outright.
// It is the tidier half of the rule and it needs its own change, because a
// peer whose config still selects the path would send it back on every
// reconcile and this side would drop it again, writing the whole index each
// time. Fixing a delete by inventing a write loop is not a fix. Filtering on
// adopt is the likelier answer and it changes what crosses the wire.
*observed = current;
Ok(changed)
}
Expand Down Expand Up @@ -3391,6 +3431,124 @@ mod tests {
assert_eq!(paths, vec!["agent.toml".to_string()]);
}

/// A WATCHED FOLDER THAT IS NOT THERE IS NOT A FOLDER SOMEBODY EMPTIED.
///
/// `scan_folder` returns an EMPTY result when the root does not exist. Every
/// tracked path is then in the observed set and absent from the scan, which
/// is the same shape as a local delete, so a delete-propagating policy
/// tombstones the lot and sends that to every peer.
///
/// The root can vanish for reasons that are not a deletion: an unmounted
/// volume, a directory renamed while a pass runs, a mount that is not ready
/// yet at boot. None of those mean the user deleted their files, and the
/// blast radius is the whole entry rather than one path.
///
/// Prefer the annoying failure. A folder we cannot see is a folder we cannot
/// see, and the honest answer is to do nothing until we can.
#[test]
fn a_vanished_root_is_not_a_mass_delete() {
let dir = tempfile::tempdir().unwrap();
let root = dir.path().join("watched");
std::fs::create_dir_all(&root).unwrap();
for name in ["a.md", "b.md", "c.md"] {
std::fs::write(root.join(name), b"payload").unwrap();
}
let entry = entry_with_policy("sync", &root, SyncPolicy::Bus);
let rules = entry.policy.rules();
assert!(
rules.propagate_deletes,
"this test is about a policy that propagates deletes"
);

let mut node = SyncNode::new(Author([1; 32]));
let mut observed = HashMap::new();
let mut cache = HashMap::new();
scan_into_node_observed(&mut node, &root, &entry, rules, &mut observed, &mut cache)
.unwrap();
assert_eq!(
node.manifest().present_paths().count(),
3,
"the fixture never recorded the files, so losing them below proves nothing"
);

// The volume goes away. Nobody deleted anything.
std::fs::remove_dir_all(&root).unwrap();
scan_into_node_observed(&mut node, &root, &entry, rules, &mut observed, &mut cache)
.unwrap();

assert_eq!(
node.manifest().present_paths().count(),
3,
"a root that vanished was read as a delete of every file in the entry, \
and that tombstone set goes to every peer"
);
}

/// Turning delete propagation ON must not delete a file that is simply
/// there. A delete pending for an unknown length of time is not a delete
/// anybody asked for today.
///
/// HONESTY ABOUT THIS TEST: it passes before the fix as well as after, and I
/// am keeping it anyway. It was written to reproduce a proposed cause of the
/// 2026-08-25 file loss, that enabling propagation replayed a backlog of old
/// deletes. IT DOES NOT REPRODUCE, because that was not the cause. The cause
/// was a path removed from an entry's include, which left it recorded but
/// unscannable, and `a_path_dropped_from_include_is_forgotten_not_deleted`
/// in tests/folder_sync.rs is the test that DOES reproduce it.
///
/// A test that passes before and after is still worth having: it proves the
/// switch itself is not the dangerous part, so nobody has to wonder again.
#[test]
fn enabling_delete_propagation_does_not_delete_what_is_still_there() {
let dir = tempfile::tempdir().unwrap();
let root = dir.path();
std::fs::write(root.join("a.md"), b"alpha").unwrap();
std::fs::write(root.join("b.md"), b"beta").unwrap();
let entry = entry_with_policy("sync", root, SyncPolicy::Catalog);
let quiet = PolicyRules {
propagate_deletes: false,
sweep_tombstones: false,
};
let loud = PolicyRules {
propagate_deletes: true,
sweep_tombstones: false,
};

let mut node = SyncNode::new(Author([1; 32]));
let mut observed = HashMap::new();
let mut cache = HashMap::new();
scan_into_node_observed(&mut node, root, &entry, quiet, &mut observed, &mut cache)
.unwrap();
assert_eq!(
node.manifest().present_paths().count(),
2,
"the fixture never recorded both files, so the switch below proves nothing"
);

// The switch. Same disk, same records, nobody deleted anything.
scan_into_node_observed(&mut node, root, &entry, loud, &mut observed, &mut cache)
.unwrap();
let protected = observed.clone();
materialize_tracked(
&mut node,
root,
loud,
&protected,
&mut observed,
&cache,
None,
)
.unwrap();

assert!(root.join("a.md").exists(), "enabling propagation deleted a.md");
assert!(root.join("b.md").exists(), "enabling propagation deleted b.md");
assert_eq!(
node.manifest().present_paths().count(),
2,
"enabling propagation tombstoned a file nobody deleted"
);
}

/// The bookkeeping files are rewritten WHOLE every time they are written,
/// and they are fabric's own index rather than anyone's data. Indenting
/// them for a reader who does not exist cost 62% of every byte.
Expand Down
58 changes: 58 additions & 0 deletions tests/folder_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,3 +336,61 @@ async fn catalog_delete_while_peer_away_does_not_resurrect_on_return() -> Result
node_a.shutdown().await?;
Ok(())
}

/// THE INCIDENT OF 2026-08-25, AS A TEST.
///
/// Removing a path from an entry's `include` leaves whatever the manifest
/// already recorded for it. Under a policy that does not propagate deletes that
/// is inert. Under one that does, the entry sees a path that is Present in its
/// manifest and absent from its scan, which is exactly what a local delete looks
/// like, so it tombstones the path and DELETES THE REAL FILE.
///
/// That is not hypothetical. It removed thirteen live plan files from three
/// machines. Every one was recoverable from git, which is the only reason it was
/// a bad night rather than a disaster.
///
/// A path that leaves an entry's include set must be FORGOTTEN, not deleted.
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
async fn a_path_dropped_from_include_is_forgotten_not_deleted() -> Result<()> {
let _guard = FOLDER_SYNC_LOCK.lock().await;
let a_dir = TempDir::new()?;
let a_home = FabricHome::new(a_dir.path());
let a_folder = a_dir.path().join("shared");
std::fs::create_dir_all(a_folder.join("plans"))?;
std::fs::create_dir_all(a_folder.join("keep"))?;
write_sync_with_include(a_dir.path(), &a_folder, "catalog", "**");

let node_a = FabricNode::start(a_home.clone()).await?;

let doomed = a_folder.join("plans/live-work.md");
let kept = a_folder.join("keep/other.md");
std::fs::write(&doomed, b"work someone is going to ship")?;
std::fs::write(&kept, b"still included")?;
reload_sync(&a_home).await?;
assert!(
wait_for_file(&doomed, b"work someone is going to ship").await,
"POSITIVE CONTROL FAILED: the entry never recorded the file, so dropping \
it from the include afterwards would prove nothing"
);

// Now narrow the include so plans/ is no longer this entry's business. The
// file itself is untouched on disk and nobody asked for it to be deleted.
write_sync_with_include(a_dir.path(), &a_folder, "catalog", "keep/**");
reload_sync(&a_home).await?;

assert_stays_missing(&a_folder.join("never-existed.md"), "sanity").await;
node_a.shutdown().await?;

assert!(
doomed.exists(),
"DROPPING A PATH FROM THE INCLUDE DELETED THE FILE. A path that leaves an \
entry is not a file anybody deleted."
);
assert_eq!(
std::fs::read(&doomed)?,
b"work someone is going to ship",
"the excluded file survived but its content changed"
);
assert!(kept.exists(), "an included file was lost too");
Ok(())
}
Loading