diff --git a/PARITY.md b/PARITY.md index 6baa922d8..86f481146 100644 --- a/PARITY.md +++ b/PARITY.md @@ -175,6 +175,13 @@ individually. The fix is entirely in the shared Rust core (`translate_event` / `process_file_event` / `scanner::scan_subtree`), so all three SDKs gain it at once with no binding changes and no public-surface change. +**Directory moved out of the root (#1109) — parity maintained, no drift.** The +watcher deletes every row whose file sat beneath a directory that leaves the +tree (a rename out of or within the root), which the OS reports as one event +naming the directory and none for its files. The fix is entirely in the shared +Rust core (`process_file_event` / `Db::files_under`), so all three SDKs gain it +at once with no binding changes and no public-surface change. + **Verbatim table columns (#361, epic #358) — parity by construction, no drift.** User tables carry exactly the columns declared in their DDL (row ownership is tracked in the internal `_dirsql_internal_rows` table, not injected columns). diff --git a/packages/python/e2e-attestations/claude-1109-watch-moved-out.json b/packages/python/e2e-attestations/claude-1109-watch-moved-out.json new file mode 100644 index 000000000..9face0c91 --- /dev/null +++ b/packages/python/e2e-attestations/claude-1109-watch-moved-out.json @@ -0,0 +1,7 @@ +{ + "command": "just test-e2e", + "ran_at": 1789595962, + "exit_code": 0, + "commit": "c645cfe8052bdca311060ca475f6ee102246650b", + "branch": "claude/1109-watch-moved-out" +} diff --git a/packages/rust/changelog.d/2026-09-16-watch-moved-out-directory.md b/packages/rust/changelog.d/2026-09-16-watch-moved-out-directory.md new file mode 100644 index 000000000..951335040 --- /dev/null +++ b/packages/rust/changelog.d/2026-09-16-watch-moved-out-directory.md @@ -0,0 +1,3 @@ +**Fixed** + +- **A directory moved out of the watched root now drops the rows its files produced.** Renaming a directory out of the tree (or to a new name within it) reaches the live watcher as one event naming the directory and none for the files beneath it, so their rows lingered until a cold rescan. The watcher now treats a deleted path as a possible directory: every row whose file sat under it is deleted, with a `Delete` event per file, the same as if each file had been removed on its own. The mirror case, a directory moved in, is #1096. (#1109) diff --git a/packages/rust/src/db.rs b/packages/rust/src/db.rs index 16d3bea1f..865c51ae4 100644 --- a/packages/rust/src/db.rs +++ b/packages/rust/src/db.rs @@ -645,6 +645,19 @@ impl Db { Ok(out) } + /// Every distinct `(table_name, file_path)` whose file sits beneath the + /// directory `dir`, ordered by table then path. + pub fn files_under(&self, dir: &str) -> Result> { + let prefix = format!("{dir}{}", std::path::MAIN_SEPARATOR); + let mut stmt = self.conn.prepare( + "SELECT DISTINCT table_name, file_path FROM _dirsql_internal_rows \ + WHERE substr(file_path, 1, length(?1)) = ?1 \ + ORDER BY table_name, file_path", + )?; + let rows = stmt.query_map([prefix], |r| Ok((r.get(0)?, r.get(1)?)))?; + Ok(rows.collect::>>()?) + } + /// Helper to execute delete_rows_by_file statements on any connection. /// Called from delete_rows_by_file() either inside a caller-supplied /// transaction or inside a transaction opened by delete_rows_by_file(). @@ -1700,6 +1713,45 @@ mod tests { assert_eq!(rows[0].2, 42, "rowid_ref must be the user-declared rowid"); } + #[test] + fn files_under_lists_each_file_beneath_the_directory_once_across_tables() { + let db = Db::new().unwrap(); + db.create_table("t1", "CREATE TABLE t1 (id TEXT)").unwrap(); + db.create_table("t2", "CREATE TABLE t2 (id TEXT)").unwrap(); + let row = HashMap::from([("id".into(), Value::Text("x".into()))]); + for (table, file, idx) in [ + ("t1", "dir/a.jsonl", 0), + ("t1", "dir/a.jsonl", 1), + ("t1", "dir/sub/b.jsonl", 0), + ("t2", "dir/c.jsonl", 0), + ("t1", "dir2/x.jsonl", 0), + ("t1", "other.jsonl", 0), + ] { + db.insert_row(table, &row, file, idx).unwrap(); + } + + assert_eq!( + db.files_under("dir").unwrap(), + vec![ + ("t1".to_string(), "dir/a.jsonl".to_string()), + ("t1".to_string(), "dir/sub/b.jsonl".to_string()), + ("t2".to_string(), "dir/c.jsonl".to_string()), + ], + "a multi-row file is listed once; `dir2/` is not beneath `dir`" + ); + assert!(db.files_under("nothing").unwrap().is_empty()); + } + + #[test] + fn files_under_surfaces_sql_failure() { + let db = Db::new().unwrap(); + db.conn + .execute("DROP TABLE _dirsql_internal_rows", []) + .unwrap(); + let err = db.files_under("dir").unwrap_err(); + assert!(matches!(err, DbError::Sqlite(_)), "got: {err:?}"); + } + #[test] fn delete_rows_by_file_removes_mapping() { let db = Db::new().unwrap(); diff --git a/packages/rust/src/lib.rs b/packages/rust/src/lib.rs index 86db85621..5062b1d68 100644 --- a/packages/rust/src/lib.rs +++ b/packages/rust/src/lib.rs @@ -557,9 +557,6 @@ impl DirSQL { // unspecified. An `on_file` failure produces an error event for that // table only; the other matching tables still process the event. let matches = self.inner.matcher.match_all(&rel_path_buf); - if matches.is_empty() { - return Vec::new(); - } let rel_path = rel_path_buf.to_string_lossy().to_string(); let mut events = Vec::new(); @@ -573,6 +570,33 @@ impl DirSQL { } } } + // The mirror of the walk above: a directory that leaves the tree + // (`rm -r`, or a rename out of or within the root) arrives as one + // event naming the directory and none for the files beneath it. It is + // already gone, so the rows are the only record of what it held. + if matches!(event, FileEvent::Deleted(_)) { + events.extend(self.delete_subtree(&rel_path)); + } + events + } + + /// Delete the rows of every file recorded beneath `rel_dir`, in every + /// table, as if each file had its own delete event. + fn delete_subtree(&self, rel_dir: &str) -> Vec { + let files = { + let db = match self.inner.db.lock() { + Ok(db) => db, + Err(e) => return vec![error_event(None, rel_dir, e.to_string())], + }; + match db.files_under(rel_dir) { + Ok(files) => files, + Err(e) => return vec![error_event(None, rel_dir, e.to_string())], + } + }; + let mut events = Vec::new(); + for (table, file_path) in files { + events.extend(self.handle_delete(&table, &file_path)); + } events } @@ -2753,6 +2777,153 @@ mod internal_tests { ); } + /// A one-table db whose fake fs stats every file in `files`, each already + /// upserted so the db holds a row per file. + fn populated_db(root: &Path, glob: &str, files: &[&str]) -> DirSQL { + let mut fake = FakeFs::default(); + for f in files { + fake.stats.insert(root.join(f), fake_stat()); + } + let db = DirSQL::with_ignore_and_fs( + root, + vec![Table::new( + "items", + "CREATE TABLE items (name TEXT)", + glob, + |path| { + vec![Row::from_iter([( + "name".to_string(), + Value::Text(path.to_string()), + )])] + }, + )], + Vec::::new(), + Arc::new(fake), + ) + .unwrap(); + for f in files { + let events = db.handle_upsert("items", &root.join(f), f); + assert!( + matches!(&events[..], [RowEvent::Insert { .. }]), + "got: {events:?}" + ); + } + db + } + + fn row_names(db: &DirSQL) -> Vec { + let mut names: Vec = db + .query("SELECT name FROM items") + .unwrap() + .into_iter() + .filter_map(|r| match r.get("name") { + Some(Value::Text(n)) => Some(n.clone()), + _ => None, + }) + .collect(); + names.sort(); + names + } + + fn deleted_paths(events: &[RowEvent]) -> Vec { + assert!( + events.iter().all(|e| matches!(e, RowEvent::Delete { .. })), + "expected only Delete events: {events:?}" + ); + let mut paths: Vec = events + .iter() + .filter_map(|e| match e { + RowEvent::Delete { file_path, .. } => Some(file_path.clone()), + _ => None, + }) + .collect(); + paths.sort(); + paths + } + + /// A deleted directory matches no file glob, yet every row recorded + /// beneath it goes, with one Delete per file. A sibling whose name merely + /// starts with the directory's (`moved2/`) is untouched. + #[test] + fn process_file_event_deleted_directory_removes_the_rows_beneath_it() { + let dir = TempDir::new().unwrap(); + let root = dir.path().to_path_buf(); + let files = ["moved/a.txt", "moved/one/deep.txt", "moved2/x.txt"]; + let db = populated_db(&root, "**/*.txt", &files); + + let events = db.process_file_event(FileEvent::Deleted(root.join("moved"))); + + assert_eq!( + deleted_paths(&events), + vec!["moved/a.txt".to_string(), "moved/one/deep.txt".to_string()] + ); + assert_eq!( + row_names(&db), + vec![root.join("moved2/x.txt").to_string_lossy().to_string()] + ); + } + + /// With a glob that also matches the directory itself (`**/*`), the + /// exact-path delete finds no row and the subtree delete still runs. + #[test] + fn process_file_event_deleted_directory_matching_a_glob_removes_the_rows_beneath_it() { + let dir = TempDir::new().unwrap(); + let root = dir.path().to_path_buf(); + let db = populated_db(&root, "**/*", &["moved/a.txt", "keep.txt"]); + + let events = db.process_file_event(FileEvent::Deleted(root.join("moved"))); + + assert_eq!(deleted_paths(&events), vec!["moved/a.txt".to_string()]); + assert_eq!( + row_names(&db), + vec![root.join("keep.txt").to_string_lossy().to_string()] + ); + } + + /// Only a *deleted* directory empties its subtree; a metadata change on + /// one says nothing about its files. + #[test] + fn process_file_event_keeps_the_rows_beneath_a_modified_directory() { + let dir = TempDir::new().unwrap(); + let root = dir.path().to_path_buf(); + let db = populated_db(&root, "**/*.txt", &["moved/a.txt"]); + + let events = db.process_file_event(FileEvent::Modified(root.join("moved"))); + + assert!(events.is_empty(), "got: {events:?}"); + assert_eq!( + row_names(&db), + vec![root.join("moved/a.txt").to_string_lossy().to_string()] + ); + } + + #[test] + fn delete_subtree_surfaces_db_poison() { + let (_dir, db, _abs, _rel) = upsert_fixture(); + poison(&db.inner.db); + let events = db.delete_subtree("moved"); + assert_single_lock_error(&events); + } + + #[test] + fn delete_subtree_surfaces_db_failure() { + let (_dir, db, _abs, _rel) = upsert_fixture(); + db.inner + .db + .lock() + .unwrap() + .conn() + .execute("DROP TABLE _dirsql_internal_rows", []) + .unwrap(); + let events = db.delete_subtree("moved"); + assert_eq!(events.len(), 1, "expected one error event: {events:?}"); + assert!( + matches!(&events[0], RowEvent::Error { table: None, file_path, error } + if file_path == Path::new("moved") && error.contains("no such table")), + "got: {events:?}" + ); + } + /// Building with a **relative** root canonicalizes `watch_root` to an /// absolute path while leaving `root` exactly as the caller supplied it, /// so `notify` never sees `.`. diff --git a/packages/rust/tests/cli_e2e.rs b/packages/rust/tests/cli_e2e.rs index 3b59ca088..0e3fdac47 100644 --- a/packages/rust/tests/cli_e2e.rs +++ b/packages/rust/tests/cli_e2e.rs @@ -396,6 +396,65 @@ fn server_indexes_a_directory_moved_into_the_root() { kill_and_wait(child); } +/// The mirror of the move-in above. inotify reports a directory renamed out +/// of the root as a single event naming the directory, with no per-child +/// events, so a live server has to drop the rows of every file that was +/// beneath it or they linger until a cold rescan. +#[test] +fn server_drops_the_rows_of_a_directory_moved_out_of_the_root() { + let root = blog_fixture(); + let outside = TempDir::new().unwrap(); + + let port = free_port(); + let child = spawn_dirsql_with_args(root.path(), port, &["-c", ".dirsql.toml"]); + wait_until_ready(port, Duration::from_secs(10)); + // Let notify finish installing its recursive inotify watches before the + // rename; otherwise the move can land before the root is watched at all. + std::thread::sleep(Duration::from_millis(300)); + + let client = Client::new(); + let url = format!("http://localhost:{port}/query"); + let basenames = |client: &Client| -> Vec { + let body: Value = client + .post(&url) + .json(&json!({"sql": "SELECT basename FROM posts ORDER BY basename"})) + .send() + .unwrap() + .json() + .unwrap(); + body.as_array() + .unwrap() + .iter() + .filter_map(|r| r.get("basename").and_then(Value::as_str)) + .map(str::to_string) + .collect() + }; + assert_eq!( + basenames(&client), + vec![ + "Hello-World.json".to_string(), + "Second-Post.json".to_string() + ], + "the initial scan must index both posts" + ); + + fs::rename(root.path().join("posts/bob"), outside.path().join("bob")).unwrap(); + + let deadline = Instant::now() + Duration::from_secs(10); + while Instant::now() < deadline && basenames(&client).iter().any(|b| b == "Second-Post.json") { + std::thread::sleep(Duration::from_millis(100)); + } + + let rows = basenames(&client); + assert_eq!( + rows, + vec!["Hello-World.json".to_string()], + "a directory moved out of the root must drop the rows its files produced; rows: {rows:?}" + ); + + kill_and_wait(child); +} + #[test] fn sigint_triggers_graceful_exit_zero() { let root = blog_fixture(); diff --git a/packages/rust/tests/watch_scan_correctness.rs b/packages/rust/tests/watch_scan_correctness.rs index 2d413e14e..60faaee31 100644 --- a/packages/rust/tests/watch_scan_correctness.rs +++ b/packages/rust/tests/watch_scan_correctness.rs @@ -1,8 +1,9 @@ //! Watch/scan correctness: a live `mkdir` under the root must not insert a //! directory row, renaming a matching file *out* of the tree must delete its -//! rows, and a populated directory renamed *into* the tree must index every -//! file it brought. All drive a **real** `notify` watcher over real temp -//! directories through the SDK public API — the core's integration tier. +//! rows, a populated directory renamed *into* the tree must index every +//! file it brought, and one renamed *out* must delete every row it held. All +//! drive a **real** `notify` watcher over real temp directories through the +//! SDK public API — the core's integration tier. use dirsql::{DirSQL, RowEvent, Table, Value}; use std::collections::HashMap; @@ -183,3 +184,112 @@ fn dir_moved_into_root_indexes_nested_files() { paths(&db) ); } + +/// The mirror of a move-in. A directory renamed out of the root arrives as +/// one event naming the directory and none for the files beneath it. The +/// directory is gone by then, so the rows are the only record of what it +/// held, and each must go with a Delete event. +#[test] +fn dir_moved_out_of_root_deletes_its_rows() { + let dir = tempfile::TempDir::new().unwrap(); + let outside = tempfile::TempDir::new().unwrap(); + fs::create_dir_all(dir.path().join("leaving/one")).unwrap(); + fs::write(dir.path().join("leaving/a.txt"), "alpha").unwrap(); + fs::write(dir.path().join("leaving/one/deep.txt"), "deep").unwrap(); + fs::write(dir.path().join("keep.txt"), "keep").unwrap(); + + let db = DirSQL::new(dir.path(), vec![files_table(dir.path())]).unwrap(); + let under_leaving = |db: &DirSQL| -> Vec { + let mut rows: Vec = paths(db) + .into_iter() + .filter(|p| p.starts_with("leaving/")) + .collect(); + rows.sort(); + rows + }; + assert_eq!( + under_leaving(&db), + vec![ + "leaving/a.txt".to_string(), + "leaving/one/deep.txt".to_string() + ], + "initial scan should index both files under leaving/" + ); + + db.start_watching().unwrap(); + std::thread::sleep(Duration::from_millis(250)); + + fs::rename(dir.path().join("leaving"), outside.path().join("leaving")).unwrap(); + + let mut deleted = Vec::new(); + let deadline = Instant::now() + Duration::from_secs(5); + while Instant::now() < deadline && !under_leaving(&db).is_empty() { + for e in db.poll_events(Duration::from_millis(200)).unwrap() { + if let RowEvent::Delete { file_path, .. } = e { + deleted.push(file_path); + } + } + } + + assert!( + under_leaving(&db).is_empty(), + "a directory moved out of the root must delete the rows its files produced; rows: {:?}", + paths(&db) + ); + assert!( + paths(&db).iter().any(|p| p == "keep.txt"), + "a file outside the moved directory must keep its row; rows: {:?}", + paths(&db) + ); + deleted.sort(); + assert_eq!( + deleted, + vec![ + "leaving/a.txt".to_string(), + "leaving/one/deep.txt".to_string() + ], + "one Delete event per file the directory held" + ); +} + +/// A rename within the root is a move-out of the old name. Only the +/// disappearing side is asserted here; the new name is indexed by the +/// move-in walk (#1096). +#[test] +fn dir_renamed_within_root_deletes_rows_under_old_name() { + let dir = tempfile::TempDir::new().unwrap(); + fs::create_dir(dir.path().join("a")).unwrap(); + fs::write(dir.path().join("a/x.txt"), "x").unwrap(); + + let db = DirSQL::new(dir.path(), vec![files_table(dir.path())]).unwrap(); + assert!( + paths(&db).iter().any(|p| p == "a/x.txt"), + "initial scan should index a/x.txt; rows: {:?}", + paths(&db) + ); + + db.start_watching().unwrap(); + std::thread::sleep(Duration::from_millis(250)); + + fs::rename(dir.path().join("a"), dir.path().join("b")).unwrap(); + + let mut saw_delete = false; + let deadline = Instant::now() + Duration::from_secs(5); + while Instant::now() < deadline && paths(&db).iter().any(|p| p.starts_with("a/")) { + for e in db.poll_events(Duration::from_millis(200)).unwrap() { + if matches!(&e, RowEvent::Delete { file_path, .. } if file_path == "a/x.txt") { + saw_delete = true; + } + } + } + + assert!( + !paths(&db).iter().any(|p| p.starts_with("a/")), + "renaming a directory within the root must delete the rows under its old name; rows: {:?}", + paths(&db) + ); + assert!( + saw_delete, + "renaming a directory within the root must emit a Delete event for each file under the old name" + ); +} diff --git a/packages/ts/e2e-attestations/claude-1109-watch-moved-out.json b/packages/ts/e2e-attestations/claude-1109-watch-moved-out.json new file mode 100644 index 000000000..7838a0e43 --- /dev/null +++ b/packages/ts/e2e-attestations/claude-1109-watch-moved-out.json @@ -0,0 +1,7 @@ +{ + "command": "pnpm test:e2e", + "ran_at": 1789596015, + "exit_code": 0, + "commit": "29d6ce890b0ce3eca1f8cea07f6503b13bc02f24", + "branch": "claude/1109-watch-moved-out" +}