Skip to content

Commit 11bcf5d

Browse files
fix(watch): bound sidecar delivery watches away from Resource payloads
The DING sidecar and the codex/opencode delivery pumps recursively watched each agent dir. A recursive registration eagerly allocates one inotify watch per directory BEFORE any callback filtering, and agent dirs contain `resources/` payload trees — the same unbounded-traversal class the supervisor hit in #314, one level down. - watch_delivery_inputs now makes two shallow subscriptions: a non-recursive watch on the agent dir (status file) plus a recursive watch on resources/inbox, which is st2-owned and bounded by delivery traffic rather than payloads. - run_ding anchors its watch on the inbox itself and creates it up front, instead of falling back to a recursive walk over the whole agent dir when the inbox did not exist yet. - All remaining recursive registrations construct their watcher with follow_symlinks(false): a symlink must never carry traversal outside the watched boundary. Closes #314 (sidecar instance). Co-authored-by: schickling-assistant <schickling-assistant@users.noreply.github.com> agent-identity: unknown agent-persona: generalist agent-supervisor: unavailable agent-tool: OMP agent-tool-version: 18.0.3 agent-runtime: OMP 18.0.3 tooling-profile: dotfiles@f33cd9c-dirty
1 parent dd0fcc7 commit 11bcf5d

2 files changed

Lines changed: 68 additions & 10 deletions

File tree

src/ding/mod.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -951,13 +951,13 @@ pub fn run_ding(
951951
stop: &AtomicBool,
952952
) -> anyhow::Result<()> {
953953
// Arm the watcher before seeding. The timer remains the correctness fallback if watching fails.
954+
// The subscription is the inbox itself, never a recursive walk over the agent dir: Resource
955+
// payload trees live under `resources/` beside the inbox, and eager registration would pay
956+
// one inotify watch per payload directory before any filtering ever ran. The inbox is
957+
// created up front so the watch anchors on it directly (senders create it on demand anyway).
958+
let _ = std::fs::create_dir_all(inbox_dir);
954959
let (tx, rx) = channel::<()>();
955-
let watch_at = if inbox_dir.exists() {
956-
inbox_dir
957-
} else {
958-
inbox_dir.parent().unwrap_or(inbox_dir)
959-
};
960-
let _watcher = crate::watch::watch_recursive_mutations(watch_at, tx);
960+
let _watcher = crate::watch::watch_recursive_mutations(inbox_dir, tx);
961961

962962
let mut seen = HashSet::new();
963963
let backlog = new_arrivals(inbox_dir, &mut seen);

src/watch.rs

Lines changed: 62 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,15 @@ use notify::event::{ModifyKind, RenameMode};
1414
use notify::{Event, EventKind, RecommendedWatcher, RecursiveMode, Watcher};
1515

1616
/// Watch a directory recursively, forwarding only events that can change reconciled state.
17+
///
18+
/// Recursive registration eagerly walks the tree (one inotify watch per directory), so this is
19+
/// only sound over st2-owned, payload-free directories. Symlink following is disabled: a link
20+
/// must never carry traversal outside the watched boundary.
1721
pub(crate) fn watch_recursive_mutations(
1822
dir: &Path,
1923
tx: Sender<()>,
2024
) -> Option<notify::RecommendedWatcher> {
21-
let mut watcher = notify::recommended_watcher(move |result: notify::Result<Event>| {
25+
let mut watcher = recommended_watcher(move |result: notify::Result<Event>| {
2226
if result.is_ok_and(|event| is_mutation(&event)) {
2327
let _ = tx.send(());
2428
}
@@ -37,23 +41,43 @@ pub(crate) fn watch_delivery_inputs(
3741
tx: Sender<()>,
3842
) -> Option<notify::RecommendedWatcher> {
3943
let inbox = agent_dir.join("resources").join("inbox");
44+
let inbox_for_callback = inbox.clone();
4045
let status = agent_dir.join("status");
41-
let mut watcher = notify::recommended_watcher(move |result: notify::Result<Event>| {
46+
let mut watcher = recommended_watcher(move |result: notify::Result<Event>| {
4247
if result.is_ok_and(|event| {
4348
is_mutation(&event)
4449
&& event
4550
.paths
4651
.iter()
47-
.any(|path| path.starts_with(&inbox) || *path == status)
52+
.any(|path| path.starts_with(&inbox_for_callback) || *path == status)
4853
}) {
4954
let _ = tx.send(());
5055
}
5156
})
5257
.ok()?;
53-
watcher.watch(agent_dir, RecursiveMode::Recursive).ok()?;
58+
// Two shallow subscriptions instead of one recursive walk over the agent dir: Resource
59+
// payload trees live beside the inbox under `resources/`, and a recursive subscription pays
60+
// one inotify watch per payload directory before any filtering ever runs. The agent-dir watch
61+
// stays shallow because everything beside `status` is runtime state; the inbox itself is
62+
// st2-owned and message-flat, so its recursion is bounded by delivery traffic, not payloads.
63+
let inbox_for_watch = inbox;
64+
watcher.watch(agent_dir, RecursiveMode::NonRecursive).ok()?;
65+
watcher
66+
.watch(&inbox_for_watch, RecursiveMode::Recursive)
67+
.ok()?;
5468
Some(watcher)
5569
}
5670

71+
/// A [`notify::RecommendedWatcher`] that never follows symlinks while walking.
72+
fn recommended_watcher(
73+
handler: impl FnMut(notify::Result<Event>) + Send + 'static,
74+
) -> notify::Result<notify::RecommendedWatcher> {
75+
notify::RecommendedWatcher::new(
76+
handler,
77+
notify::Config::default().with_follow_symlinks(false),
78+
)
79+
}
80+
5781
/// A shallow subscription over the directories that can contain declarations.
5882
///
5983
/// `notify` implements a recursive Linux watch by eagerly walking the entire tree and allocating
@@ -336,6 +360,40 @@ mod tests {
336360
.expect("status rename must wake");
337361
}
338362

363+
#[cfg(target_os = "linux")]
364+
#[test]
365+
fn delivery_watch_is_bounded_by_inputs_not_payload_depth() {
366+
use std::sync::mpsc::channel;
367+
368+
fn inotify_watch_count() -> usize {
369+
std::fs::read_dir("/proc/self/fdinfo")
370+
.unwrap()
371+
.flatten()
372+
.map(|entry| std::fs::read_to_string(entry.path()).unwrap_or_default())
373+
.filter(|content| content.contains("inotify wd:"))
374+
.map(|content| content.lines().count())
375+
.sum()
376+
}
377+
378+
let dir = tempfile::tempdir().unwrap();
379+
let agent_dir = dir.path();
380+
let payload = agent_dir.join("resources/worktree/node_modules");
381+
for i in 0..300 {
382+
std::fs::create_dir_all(payload.join(format!("pkg-{i}/dist/sub"))).unwrap();
383+
}
384+
std::fs::create_dir_all(agent_dir.join("resources/inbox")).unwrap();
385+
386+
let before = inotify_watch_count();
387+
let (tx, _rx) = channel();
388+
let _watcher = watch_delivery_inputs(agent_dir, tx).expect("start delivery watcher");
389+
let delta = inotify_watch_count() - before;
390+
assert!(
391+
delta < 32,
392+
"payload depth must not drive watch allocation: {delta} watches for a \
393+
900-directory payload tree"
394+
);
395+
}
396+
339397
#[cfg(target_os = "linux")]
340398
#[test]
341399
fn linux_reads_are_silent_but_real_mutations_wake() {

0 commit comments

Comments
 (0)