diff --git a/crates/bashkit/src/builtins/walk_cache.rs b/crates/bashkit/src/builtins/walk_cache.rs index 4675c550f..804bf75a5 100644 --- a/crates/bashkit/src/builtins/walk_cache.rs +++ b/crates/bashkit/src/builtins/walk_cache.rs @@ -138,6 +138,30 @@ mod tests { assert_eq!(cache.held(), 0); } + /// The default walk visits directories in path order, the order tree, + /// ls -R and find consume them, so a batch cut short by `max_entries` + /// holds the next listings the caller needs instead of arbitrary ones. + #[tokio::test] + async fn default_walk_visits_directories_in_path_order() { + let fs = InMemoryFs::new(); + for d in ["b", "a", "c", "a/y", "a/x", "c/z"] { + fs.mkdir(&PathBuf::from(format!("/t/{d}")), true) + .await + .unwrap(); + } + let got: Vec = fs + .walk(Path::new("/t"), WalkOptions::new(None)) + .await + .unwrap() + .into_iter() + .map(|l| l.path.display().to_string()) + .collect(); + assert_eq!( + got, + ["/t", "/t/a", "/t/a/x", "/t/a/y", "/t/b", "/t/c", "/t/c/z"] + ); + } + #[tokio::test] async fn missing_directory_reports_read_dir_error() { let fs = tree(1, 1).await; diff --git a/crates/bashkit/src/fs/traits.rs b/crates/bashkit/src/fs/traits.rs index 8826588b7..f1aec93e7 100644 --- a/crates/bashkit/src/fs/traits.rs +++ b/crates/bashkit/src/fs/traits.rs @@ -539,12 +539,19 @@ pub async fn walk_with_read_dir( }; let child_depth = depth + 1; if opts.max_depth.is_none_or(|m| child_depth < m) { - for entry in &entries { - if entry.metadata.file_type.is_dir() - && !(opts.skip_hidden && entry.name.starts_with('.')) - { - pending.push((super::vfs_join(&path, &entry.name), child_depth, None)); - } + // Visit children in name order (pushed reversed, popped first), + // the order tree/ls -R/find consume them: a batch cut short by + // `max_entries` then holds the next listings the caller needs. + let mut dirs: Vec<&str> = entries + .iter() + .filter(|e| { + e.metadata.file_type.is_dir() && !(opts.skip_hidden && e.name.starts_with('.')) + }) + .map(|e| e.name.as_str()) + .collect(); + dirs.sort_unstable_by(|a, b| b.cmp(a)); + for name in dirs { + pending.push((super::vfs_join(&path, name), child_depth, None)); } } held = held.saturating_add(entries.len()); diff --git a/crates/bashkit/tests/integration/remote_fs_round_trip_tests.rs b/crates/bashkit/tests/integration/remote_fs_round_trip_tests.rs index 71330e12e..442183941 100644 --- a/crates/bashkit/tests/integration/remote_fs_round_trip_tests.rs +++ b/crates/bashkit/tests/integration/remote_fs_round_trip_tests.rs @@ -545,8 +545,10 @@ async fn tree_prefetch_stays_within_entry_budget() { let r = bash.exec("tree tree").await.unwrap(); assert_ne!(r.exit_code, 0, "expected the entry budget to stop tree"); let c = fs.counts(); - // 50 entries is a handful of directories; never the whole tree. - assert!(c.read_dir < 15, "{c}"); + // 50 entries is a handful of directories. The walk prefetches in path + // order, the order tree prints, so it reads exactly the listings tree + // shows (before, arbitrary order read 8-15 depending on hashing). + assert!(c.read_dir <= 6, "{c}"); } #[tokio::test] diff --git a/knowledge/foundations/vfs.md b/knowledge/foundations/vfs.md index c626cff08..583c3e0a9 100644 --- a/knowledge/foundations/vfs.md +++ b/knowledge/foundations/vfs.md @@ -417,9 +417,11 @@ and measured by `benches/remote_fs.rs` over the bashkit checkout: the listings of `root` and the directories below it in one call, as a bounded batch: it stops once the answer holds `max_entries` entries (default 10k; at most one listing over, never a split listing). The - default is one `read_dir` per directory, depth first, reading a directory - only when popped (`walk_with_read_dir`); bulk backends override it with a - prefix query and a row limit. Wrappers forward it when they map the subtree one-to-one + default is one `read_dir` per directory, depth first in path order (the + order tree/ls -R/find consume, so a cut-short batch holds the next listings + they need), reading a directory only when popped (`walk_with_read_dir`); + bulk backends override it with a prefix query and a row limit, and should + answer in path order too. Wrappers forward it when they map the subtree one-to-one (`FollowFs` rebases a linked root, `MountableFs` when no mount lies below the root, `RootFs` outside system trees, `ReadOnlyFs`, per-execution wrappers); otherwise they fall back to the default. Callers use it only when they would diff --git a/knowledge/log.md b/knowledge/log.md index 8fd61c21e..d04376d2e 100644 --- a/knowledge/log.md +++ b/knowledge/log.md @@ -2,6 +2,7 @@ ## 2026-11-03 +* **Walk order**: the default `walk` visits directories in path order. It used `read_dir` order (hash order for `InMemoryFs`), so a batch cut short by `max_entries` prefetched listings `tree` never printed and `tree_prefetch_stays_within_entry_budget` read 8-15 directories run to run. See [Virtual Filesystem](foundations/vfs.md#remote-backends-round-trips). * **Large-tree stress tests**: `tests/large_tree_stress_tests.rs` measures heap peaks of tree walkers over a 200k-file procedural tree (2M with `BASHKIT_STRESS_FILES`). It found `rg` collecting every candidate path before reading (~70 MB for 200k files); its walk now streams in path order. See [Testing](operations/testing.md). * **ls -R / tree bulk walk**: `ls -R` and `tree` list through bounded `walk` batches (`WalkCache`) like `find`/`du`. New `WalkOptions::skip_hidden` keeps the default walk out of dot directories they do not print; `tree` caps batches at its remaining entry budget and keeps `read_dir` under `-I`. See [Virtual Filesystem](foundations/vfs.md#remote-backends-round-trips).