Skip to content

Commit 304aa55

Browse files
fix(search): bound glob/grep tree-walk with cooperative + watchdog timeout (#201)
A Glob over a path containing a network mount (rclone/OSS NFS) hung the agent loop forever: the single-threaded walker exhaustively traversed the whole tree for a zero-match pattern with no deadline, blocking the spawn_blocking thread that the agent loop awaits. Two-layer fix: - Inner cooperative deadline (ResourceLimits::walk_timeout, 30s) checked between walk entries; returns partial results + timed_out so the tool can tell the LLM to narrow `path`. - Outer hard floor via the runtime per-tool watchdog, now extended from Bash-only to all fs-read tools (Glob/Grep/Ls/Read/ReadPdf/ReadImage/ ReadHtml) with a typed StaleReason. The watchdog is applied at a single convergence point (execute_tool_watchdogged) that BOTH the streaming early-start path and the normal approval path route through, so a tool can never be bounded on one path and unbounded on the other. Also: parallelize the glob walker (mirrors grep's build_parallel), default follow_links to false (ripgrep parity, avoids cross-mount escape/cycles), decouple `truncated` from the timeout signal (no spurious overflow files), add a deterministic path tiebreak to glob output, dedup the timeout notice into loopal-tool-api, and clamp search `max` to >=1. Design notes under design/glob-traversal-hang/.
1 parent f9189a6 commit 304aa55

22 files changed

Lines changed: 1018 additions & 69 deletions

File tree

crates/loopal-backend/src/limits.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ pub struct ResourceLimits {
2222
pub max_fetch_bytes: usize,
2323
/// Default shell command timeout.
2424
pub default_timeout: Duration,
25+
/// Cooperative deadline checked between glob/grep walk entries: bounds
26+
/// slow-but-responsive trees and returns partial results. A syscall stuck
27+
/// on a dead mount can't be interrupted here — that case is bounded by the
28+
/// runtime per-tool watchdog instead.
29+
pub walk_timeout: Duration,
2530
/// HTTP fetch timeout.
2631
pub fetch_timeout: Duration,
2732
/// Maximum image file size in bytes.
@@ -40,6 +45,7 @@ impl Default for ResourceLimits {
4045
max_grep_matches: 500,
4146
max_fetch_bytes: 5 * 1024 * 1024, // 5 MB
4247
default_timeout: Duration::from_secs(300), // 5 min
48+
walk_timeout: Duration::from_secs(30),
4349
fetch_timeout: Duration::from_secs(30),
4450
image_max_bytes: IMAGE_MAX_BYTES,
4551
image_max_pixels: IMAGE_MAX_PIXELS,
Lines changed: 65 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
1-
//! Glob pattern search with file-type filtering and modification time.
2-
31
use std::path::Path;
4-
use std::time::UNIX_EPOCH;
2+
use std::sync::Arc;
3+
use std::sync::atomic::{AtomicBool, Ordering};
4+
use std::time::{Instant, UNIX_EPOCH};
55

66
use globset::Glob;
7+
use ignore::WalkState;
78
use loopal_error::ToolIoError;
89
use loopal_tool_api::backend_types::{GlobEntry, GlobOptions, GlobSearchResult};
910
use loopal_tool_api::save_to_overflow_file;
11+
use parking_lot::Mutex;
1012

1113
use crate::limits::ResourceLimits;
1214
use crate::search::{overflow_fmt, walker};
1315

14-
/// Execute a glob search and return matching entries.
1516
pub fn glob_search(
1617
opts: &GlobOptions,
1718
cwd: &Path,
@@ -25,50 +26,75 @@ pub fn glob_search(
2526

2627
let glob =
2728
Glob::new(&opts.pattern).map_err(|e| ToolIoError::Other(format!("invalid glob: {e}")))?;
28-
let matcher = glob.compile_matcher();
29+
let max = opts.max_results.min(limits.max_glob_results).max(1);
2930

30-
let max = opts.max_results.min(limits.max_glob_results);
31-
let Some(walker) = walker::build_walker(&search_path, opts.type_filter.as_deref()) else {
31+
let Some(w) = walker::build_walker(&search_path, opts.type_filter.as_deref()) else {
3232
return Ok(GlobSearchResult {
3333
entries: Vec::new(),
3434
truncated: false,
35+
timed_out: false,
3536
overflow_path: None,
3637
});
3738
};
3839

39-
let mut entries = Vec::new();
40-
let mut truncated = false;
41-
42-
for entry in walker.build().flatten() {
43-
if !entry.file_type().is_some_and(|ft| ft.is_file()) {
44-
continue;
45-
}
46-
let path = entry.path();
47-
let rel = match path.strip_prefix(&search_path) {
48-
Ok(r) => r,
49-
Err(_) => continue,
50-
};
51-
if !matcher.is_match(rel) {
52-
continue;
53-
}
54-
let modified_secs = entry
55-
.metadata()
56-
.ok()
57-
.and_then(|m| m.modified().ok())
58-
.and_then(|t| t.duration_since(UNIX_EPOCH).ok())
59-
.map(|d| d.as_secs());
60-
61-
entries.push(GlobEntry {
62-
path: path.to_string_lossy().into_owned(),
63-
modified_secs,
64-
});
40+
let deadline = Instant::now() + limits.walk_timeout;
41+
let done = Arc::new(AtomicBool::new(false));
42+
let timed_out = Arc::new(AtomicBool::new(false));
43+
let entries: Arc<Mutex<Vec<GlobEntry>>> = Arc::new(Mutex::new(Vec::new()));
44+
let search_path = Arc::new(search_path);
45+
let matcher = Arc::new(glob.compile_matcher());
6546

66-
if entries.len() >= max {
67-
truncated = true;
68-
break;
69-
}
70-
}
47+
w.build_parallel().run(|| {
48+
let done = Arc::clone(&done);
49+
let timed_out = Arc::clone(&timed_out);
50+
let entries = Arc::clone(&entries);
51+
let search_path = Arc::clone(&search_path);
52+
let matcher = Arc::clone(&matcher);
53+
Box::new(move |entry| {
54+
if done.load(Ordering::Relaxed) {
55+
return WalkState::Quit;
56+
}
57+
if Instant::now() >= deadline {
58+
done.store(true, Ordering::Relaxed);
59+
timed_out.store(true, Ordering::Relaxed);
60+
return WalkState::Quit;
61+
}
62+
let Ok(entry) = entry else {
63+
return WalkState::Continue;
64+
};
65+
if !entry.file_type().is_some_and(|ft| ft.is_file()) {
66+
return WalkState::Continue;
67+
}
68+
let Ok(rel) = entry.path().strip_prefix(search_path.as_path()) else {
69+
return WalkState::Continue;
70+
};
71+
if !matcher.is_match(rel) {
72+
return WalkState::Continue;
73+
}
74+
let modified_secs = entry
75+
.metadata()
76+
.ok()
77+
.and_then(|m| m.modified().ok())
78+
.and_then(|t| t.duration_since(UNIX_EPOCH).ok())
79+
.map(|d| d.as_secs());
80+
let n = {
81+
let mut guard = entries.lock();
82+
guard.push(GlobEntry {
83+
path: entry.path().to_string_lossy().into_owned(),
84+
modified_secs,
85+
});
86+
guard.len()
87+
};
88+
if n >= max {
89+
done.store(true, Ordering::Relaxed);
90+
return WalkState::Quit;
91+
}
92+
WalkState::Continue
93+
})
94+
});
7195

96+
let entries = Arc::try_unwrap(entries).unwrap().into_inner();
97+
let truncated = entries.len() >= max;
7298
let overflow_path = if truncated {
7399
Some(save_to_overflow_file(
74100
&overflow_fmt::serialize_glob_results(&entries),
@@ -81,6 +107,7 @@ pub fn glob_search(
81107
Ok(GlobSearchResult {
82108
entries,
83109
truncated,
110+
timed_out: timed_out.load(Ordering::Relaxed),
84111
overflow_path,
85112
})
86113
}

crates/loopal-backend/src/search/grep.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use std::path::Path;
22
use std::sync::Arc;
33
use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
4+
use std::time::Instant;
45

56
use globset::Glob;
67
use ignore::WalkState;
@@ -55,12 +56,14 @@ pub fn grep_search(
5556
None => None,
5657
};
5758

58-
let max = opts.max_matches.min(limits.max_grep_matches);
59+
let max = opts.max_matches.min(limits.max_grep_matches).max(1);
5960
let ctx_before = opts.context_before;
6061
let ctx_after = opts.context_after;
6162
let multiline = opts.multiline;
6263
let total = Arc::new(AtomicUsize::new(0));
6364
let done = Arc::new(AtomicBool::new(false));
65+
let timed_out = Arc::new(AtomicBool::new(false));
66+
let deadline = Instant::now() + limits.walk_timeout;
6467
let results: Arc<Mutex<Vec<FileMatchResult>>> = Arc::new(Mutex::new(Vec::new()));
6568
let search_path = Arc::new(search_path);
6669
let glob_matcher = Arc::new(glob_matcher);
@@ -74,11 +77,17 @@ pub fn grep_search(
7477
let search_path = Arc::clone(&search_path);
7578
let total = Arc::clone(&total);
7679
let done = Arc::clone(&done);
80+
let timed_out = Arc::clone(&timed_out);
7781
let results = Arc::clone(&results);
7882
Box::new(move |entry| {
7983
if done.load(Ordering::Relaxed) {
8084
return WalkState::Quit;
8185
}
86+
if Instant::now() >= deadline {
87+
done.store(true, Ordering::Relaxed);
88+
timed_out.store(true, Ordering::Relaxed);
89+
return WalkState::Quit;
90+
}
8291
let entry = match entry {
8392
Ok(e) => e,
8493
Err(_) => return WalkState::Continue,
@@ -100,9 +109,10 @@ pub fn grep_search(
100109
});
101110

102111
let file_matches = Arc::try_unwrap(results).unwrap().into_inner();
103-
let truncated = done.load(Ordering::Relaxed);
112+
let truncated = total.load(Ordering::Relaxed) >= max;
104113
Ok(GrepSearchResult {
105114
total_match_count: total.load(Ordering::Relaxed),
115+
timed_out: timed_out.load(Ordering::Relaxed),
106116
overflow_path: maybe_save_overflow(truncated, &file_matches),
107117
file_matches,
108118
})

crates/loopal-backend/src/search/grep_file.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ pub(crate) fn empty_result() -> GrepSearchResult {
4343
GrepSearchResult {
4444
file_matches: Vec::new(),
4545
total_match_count: 0,
46+
timed_out: false,
4647
overflow_path: None,
4748
}
4849
}
@@ -90,6 +91,7 @@ pub(crate) fn search_single_file(
9091
let overflow_path = maybe_save_overflow(truncated, &file_matches);
9192
Ok(GrepSearchResult {
9293
total_match_count: count,
94+
timed_out: false,
9395
file_matches,
9496
overflow_path,
9597
})

crates/loopal-backend/src/search/walker.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,15 @@ use ignore::types::TypesBuilder;
77

88
/// Build a `WalkBuilder` with shared defaults.
99
///
10-
/// * Follows symlinks.
10+
/// * Does not follow symlinks (ripgrep default) — avoids cross-mount escape and traversal cycles.
1111
/// * Respects `.gitignore` (ignore crate default).
1212
/// * Applies file-type filtering when `type_filter` is given.
1313
///
1414
/// Returns `None` when `type_filter` names an unknown file type — the
1515
/// caller should short-circuit with an empty result.
1616
pub fn build_walker(search_path: &Path, type_filter: Option<&str>) -> Option<WalkBuilder> {
1717
let mut builder = WalkBuilder::new(search_path);
18-
builder.follow_links(true);
18+
builder.follow_links(false);
1919

2020
if let Some(ty) = type_filter {
2121
let mut tb = TypesBuilder::new();

crates/loopal-backend/tests/suite.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ mod approved_paths_test;
55
mod batch_test;
66
#[path = "suite/fetch_headers_test.rs"]
77
mod fetch_headers_test;
8+
#[path = "suite/glob_parallel_test.rs"]
9+
mod glob_parallel_test;
810
#[path = "suite/image_test.rs"]
911
mod image_test;
1012
#[path = "suite/log_file_test.rs"]
@@ -15,5 +17,7 @@ mod path_approval_test;
1517
mod process_group_test;
1618
#[path = "suite/resolve_checked_test.rs"]
1719
mod resolve_checked_test;
20+
#[path = "suite/search_timeout_test.rs"]
21+
mod search_timeout_test;
1822
#[path = "suite/tmp_cleanup_test.rs"]
1923
mod tmp_cleanup_test;
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
use loopal_backend::ResourceLimits;
2+
use loopal_backend::search::glob_search;
3+
use loopal_tool_api::backend_types::GlobOptions;
4+
5+
fn glob_opts(pattern: &str) -> GlobOptions {
6+
GlobOptions {
7+
pattern: pattern.to_string(),
8+
path: None,
9+
type_filter: None,
10+
max_results: 10_000,
11+
}
12+
}
13+
14+
#[test]
15+
fn parallel_glob_finds_all_nested_matches() {
16+
let tmp = tempfile::tempdir().unwrap();
17+
std::fs::write(tmp.path().join("a.rs"), "").unwrap();
18+
let sub = tmp.path().join("sub/deep");
19+
std::fs::create_dir_all(&sub).unwrap();
20+
std::fs::write(tmp.path().join("sub/b.rs"), "").unwrap();
21+
std::fs::write(sub.join("c.rs"), "").unwrap();
22+
std::fs::write(tmp.path().join("skip.txt"), "").unwrap();
23+
24+
let res = glob_search(
25+
&glob_opts("**/*.rs"),
26+
tmp.path(),
27+
&ResourceLimits::default(),
28+
)
29+
.unwrap();
30+
31+
let paths: Vec<&str> = res.entries.iter().map(|e| e.path.as_str()).collect();
32+
assert_eq!(res.entries.len(), 3);
33+
assert!(paths.iter().any(|p| p.ends_with("a.rs")));
34+
assert!(paths.iter().any(|p| p.ends_with("b.rs")));
35+
assert!(paths.iter().any(|p| p.ends_with("c.rs")));
36+
assert!(!res.truncated);
37+
assert!(!res.timed_out);
38+
}
39+
40+
#[test]
41+
fn parallel_glob_truncates_at_max_results_with_tolerance() {
42+
let tmp = tempfile::tempdir().unwrap();
43+
for i in 0..50 {
44+
std::fs::write(tmp.path().join(format!("f{i:02}.rs")), "").unwrap();
45+
}
46+
let limits = ResourceLimits {
47+
max_glob_results: 10,
48+
..ResourceLimits::default()
49+
};
50+
51+
let res = glob_search(&glob_opts("**/*.rs"), tmp.path(), &limits).unwrap();
52+
53+
assert!(res.truncated);
54+
assert!(!res.timed_out);
55+
assert!(res.entries.len() >= 10);
56+
assert!(res.entries.len() < 50);
57+
}
58+
59+
#[cfg(unix)]
60+
#[test]
61+
fn glob_does_not_follow_symlinks() {
62+
let tmp = tempfile::tempdir().unwrap();
63+
let root = tmp.path().join("root");
64+
let outside = tmp.path().join("outside");
65+
std::fs::create_dir_all(&root).unwrap();
66+
std::fs::create_dir_all(&outside).unwrap();
67+
std::fs::write(root.join("real.rs"), "").unwrap();
68+
std::fs::write(outside.join("secret.rs"), "").unwrap();
69+
std::os::unix::fs::symlink(&outside, root.join("link")).unwrap();
70+
71+
let res = glob_search(&glob_opts("**/*.rs"), &root, &ResourceLimits::default()).unwrap();
72+
73+
let paths: Vec<&str> = res.entries.iter().map(|e| e.path.as_str()).collect();
74+
assert!(paths.iter().any(|p| p.ends_with("real.rs")));
75+
assert!(!paths.iter().any(|p| p.ends_with("secret.rs")));
76+
}

0 commit comments

Comments
 (0)