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
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ insta = { version = "1", features = ["json"] }
tempfile = "3"
pretty_assertions = "1"
reqwest = { version = "0.12", default-features = false, features = ["stream", "json"] }
tokio = { version = "1", features = ["test-util"] }

[lints.rust]
unsafe_code = "warn"
Expand Down
2 changes: 1 addition & 1 deletion agentflare-workspace-hack/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ serde_json = { version = "1", features = ["alloc", "preserve_order", "raw_value"
simd-adler32 = { version = "0.3", default-features = false, features = ["std"] }
similar = { version = "2", features = ["inline"] }
smallvec = { version = "1", default-features = false, features = ["const_new"] }
tokio = { version = "1", features = ["fs", "io-std", "io-util", "macros", "net", "process", "rt-multi-thread", "sync", "time"] }
tokio = { version = "1", features = ["fs", "io-std", "io-util", "macros", "net", "process", "rt-multi-thread", "test-util"] }
tokio-stream = { version = "0.1", features = ["sync"] }
tower = { version = "0.5", default-features = false, features = ["log", "make", "util"] }
tracing = { version = "0.1", features = ["log"] }
Expand Down
53 changes: 53 additions & 0 deletions src/daemon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,22 @@ impl BinarySnapshot {
}
}

/// Polls `snapshot.is_stale()` every `interval`, returning as soon as it's
/// `true`. Split out from `dashboard::server`'s `spawn_binary_staleness_watchdog`
/// so the polling *task* itself -- not just `is_stale()`'s comparison logic
/// in isolation -- has test coverage (item #107: the watchdog silently never
/// fired in a live daemon despite `is_stale()`'s own unit tests passing,
/// which unit tests on `is_stale()` alone couldn't have caught).
pub async fn wait_for_stale(snapshot: &BinarySnapshot, interval: Duration) {
let mut ticker = tokio::time::interval(interval);
loop {
ticker.tick().await;
if snapshot.is_stale() {
return;
}
}
}

/// Spawns a replacement daemon from the (now-updated) binary at
/// `snapshot`'s path and exits this process. Cleans up this process's own
/// pid/lock files *first* so the replacement's own `is_daemon_running()`
Expand Down Expand Up @@ -349,4 +365,41 @@ mod binary_snapshot_tests {

assert!(snapshot.is_stale());
}

// Regression coverage for item #107: `is_stale()` itself was already
// correct (the three tests above), but the live daemon's watchdog task
// never fired anyway. This exercises `wait_for_stale`'s actual polling
// loop -- not just the comparison it polls -- to catch that class of bug.
#[tokio::test(start_paused = true)]
async fn wait_for_stale_polls_until_the_file_on_disk_changes() {
let dir = tempfile::tempdir().unwrap();
let path = dir.path().join("agentflare");
std::fs::write(&path, b"v1").unwrap();
let snapshot = snapshot_at(&path);
let interval = std::time::Duration::from_secs(60);

let handle = tokio::spawn(async move {
super::wait_for_stale(&snapshot, interval).await;
});

// `interval`'s first tick fires immediately; let that first (not
// stale) check happen before advancing the clock.
tokio::task::yield_now().await;
assert!(!handle.is_finished());

tokio::time::advance(interval / 2).await;
assert!(!handle.is_finished(), "fired before the file changed");

let staged = dir.path().join("agentflare.new");
std::fs::write(&staged, b"v2-longer-content").unwrap();
std::fs::rename(&staged, &path).unwrap();

tokio::time::advance(interval).await;
tokio::task::yield_now().await;

assert!(
handle.is_finished(),
"wait_for_stale never returned after the on-disk binary changed"
);
}
}
34 changes: 21 additions & 13 deletions src/dashboard/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -682,22 +682,30 @@ fn work_max_concurrency() -> usize {
/// no-op rather than a hard failure.
fn spawn_binary_staleness_watchdog(snapshot: Option<crate::daemon::BinarySnapshot>) {
let Some(snapshot) = snapshot else {
// `BinarySnapshot::capture()`'s doc comment already explains why this
// is a no-op rather than a hard failure -- but a no-op that never
// logs anything is indistinguishable from "armed and nothing's
// stale yet" from the outside (item #107), so say so explicitly.
eprintln!(
"agentflare-daemon: binary staleness watchdog disabled -- couldn't resolve the \
on-disk binary this process was launched from at startup"
);
return;
};
eprintln!(
"agentflare-daemon: binary staleness watchdog armed for {}, checking every {}s",
snapshot.path().display(),
BINARY_STALENESS_CHECK_INTERVAL.as_secs()
);
tokio::spawn(async move {
let mut ticker = tokio::time::interval(BINARY_STALENESS_CHECK_INTERVAL);
loop {
ticker.tick().await;
if snapshot.is_stale() {
eprintln!(
"agentflare-daemon: on-disk binary at {} changed since this daemon \
started -- restarting to pick up the new build instead of running \
stale in-process job logic",
snapshot.path().display()
);
crate::daemon::respawn_from_stale(&snapshot);
}
}
crate::daemon::wait_for_stale(&snapshot, BINARY_STALENESS_CHECK_INTERVAL).await;
eprintln!(
"agentflare-daemon: on-disk binary at {} changed since this daemon \
started -- restarting to pick up the new build instead of running \
stale in-process job logic",
snapshot.path().display()
);
crate::daemon::respawn_from_stale(&snapshot);
});
}

Expand Down
Loading