hive-bash-mcp: sweep harness/bash-tasks/ of files older than 30d at startup
fixes #2855
This commit is contained in:
parent
a2d40dc1bd
commit
4555ab4f10
3 changed files with 181 additions and 0 deletions
1
Cargo.lock
generated
1
Cargo.lock
generated
|
|
@ -1723,6 +1723,7 @@ dependencies = [
|
||||||
"schemars",
|
"schemars",
|
||||||
"serde",
|
"serde",
|
||||||
"serde_json",
|
"serde_json",
|
||||||
|
"tempfile",
|
||||||
"tokio",
|
"tokio",
|
||||||
"tracing",
|
"tracing",
|
||||||
"tracing-subscriber",
|
"tracing-subscriber",
|
||||||
|
|
|
||||||
|
|
@ -26,6 +26,9 @@ tokio.workspace = true
|
||||||
tracing.workspace = true
|
tracing.workspace = true
|
||||||
tracing-subscriber.workspace = true
|
tracing-subscriber.workspace = true
|
||||||
|
|
||||||
|
[dev-dependencies]
|
||||||
|
tempfile = "3"
|
||||||
|
|
||||||
# `hive-bash-daemon` — long-running per-agent bash task runner. Spawns
|
# `hive-bash-daemon` — long-running per-agent bash task runner. Spawns
|
||||||
# `sh -c` subprocesses, monitors completion, writes task state files
|
# `sh -c` subprocesses, monitors completion, writes task state files
|
||||||
# under harness/bash-tasks/, and serves the MCP tools (`run`/`status`/
|
# under harness/bash-tasks/, and serves the MCP tools (`run`/`status`/
|
||||||
|
|
|
||||||
|
|
@ -18,6 +18,12 @@
|
||||||
//! (the process died with the previous daemon). The todo is still updated to
|
//! (the process died with the previous daemon). The todo is still updated to
|
||||||
//! its interrupted-done summary so the agent is not silently blocked.
|
//! its interrupted-done summary so the agent is not silently blocked.
|
||||||
//!
|
//!
|
||||||
|
//! At daemon startup, before that interrupted-marking pass, any file under
|
||||||
|
//! `tasks_dir()` older than 30 days is deleted outright — see
|
||||||
|
//! [`sweep_old_tasks`]. Nothing swept this directory before, and it had
|
||||||
|
//! grown without bound as a result, repeatedly holding stale credentials
|
||||||
|
//! in captured output.
|
||||||
|
//!
|
||||||
//! The runner kills the child process on timeout — `tokio::process::Child::drop()`
|
//! The runner kills the child process on timeout — `tokio::process::Child::drop()`
|
||||||
//! does not kill children, so we explicitly call `child.kill().await`.
|
//! does not kill children, so we explicitly call `child.kill().await`.
|
||||||
|
|
||||||
|
|
@ -436,6 +442,10 @@ async fn run_loop(socket: PathBuf) {
|
||||||
if let Err(e) = std::fs::create_dir_all(paths::tasks_dir()) {
|
if let Err(e) = std::fs::create_dir_all(paths::tasks_dir()) {
|
||||||
tracing::warn!(error = ?e, "bash_runner: create tasks dir failed");
|
tracing::warn!(error = ?e, "bash_runner: create tasks dir failed");
|
||||||
}
|
}
|
||||||
|
let removed = sweep_old_tasks(&paths::tasks_dir(), SystemTime::now() - RETENTION);
|
||||||
|
if removed > 0 {
|
||||||
|
tracing::info!(removed, "bash_runner: swept old task files at startup");
|
||||||
|
}
|
||||||
mark_interrupted(&socket).await;
|
mark_interrupted(&socket).await;
|
||||||
|
|
||||||
let claimed: Arc<Mutex<HashSet<String>>> = Arc::new(Mutex::new(HashSet::new()));
|
let claimed: Arc<Mutex<HashSet<String>>> = Arc::new(Mutex::new(HashSet::new()));
|
||||||
|
|
@ -446,6 +456,58 @@ async fn run_loop(socket: PathBuf) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// How long a task's files are kept before [`sweep_old_tasks`] removes
|
||||||
|
/// them. `harness/bash-tasks/` grew without bound (and had repeatedly
|
||||||
|
/// held stale credentials in captured output) because nothing ever swept
|
||||||
|
/// it. Mara: "build automatic cleanup at daemon start to clear older
|
||||||
|
/// than 30d" — a flat age cutoff, no count-based retention knob.
|
||||||
|
const RETENTION: Duration = Duration::from_hours(720);
|
||||||
|
|
||||||
|
/// Delete every regular file directly under `dir` whose mtime is older
|
||||||
|
/// than `cutoff`. Runs once, at daemon startup (`run_loop`), before
|
||||||
|
/// [`mark_interrupted`] — an ancient still-`running` task is swept away
|
||||||
|
/// outright rather than given a fresh "interrupted" todo nobody's going
|
||||||
|
/// to act on 30 days later.
|
||||||
|
///
|
||||||
|
/// Whole-directory, extension-agnostic rather than `.json`-id-driven like
|
||||||
|
/// [`mark_interrupted`]/[`poll_once`]: a task's `.json`/`.out`/`.err`
|
||||||
|
/// triple is written together at completion (the same instant, well
|
||||||
|
/// inside the 30-day granularity this cares about), so sweeping
|
||||||
|
/// file-by-file on its own mtime can't split a still-referenced group —
|
||||||
|
/// and this shape also reaps orphaned `.out`/`.err` (a crash between
|
||||||
|
/// writing output and the final `.json` write) and stray `.json.tmp`
|
||||||
|
/// scratch files (`write_task`'s tmp+rename can leave one behind on a
|
||||||
|
/// crash mid-rename) that an id-driven walk would miss entirely.
|
||||||
|
/// Directories are skipped (`tasks_dir()` holds none today, but a future
|
||||||
|
/// entry shouldn't be treated as sweepable file content). Best-effort: a
|
||||||
|
/// per-entry `remove_file` failure is logged and the pass continues
|
||||||
|
/// rather than aborting on the first one. Returns the count actually
|
||||||
|
/// removed.
|
||||||
|
fn sweep_old_tasks(dir: &Path, cutoff: SystemTime) -> usize {
|
||||||
|
let Ok(rd) = std::fs::read_dir(dir) else {
|
||||||
|
return 0;
|
||||||
|
};
|
||||||
|
let mut removed = 0;
|
||||||
|
for entry in rd.flatten() {
|
||||||
|
let Ok(meta) = entry.metadata() else { continue };
|
||||||
|
if !meta.is_file() {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
let Ok(mtime) = meta.modified() else { continue };
|
||||||
|
if mtime >= cutoff {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
let path = entry.path();
|
||||||
|
match std::fs::remove_file(&path) {
|
||||||
|
Ok(()) => removed += 1,
|
||||||
|
Err(e) => {
|
||||||
|
tracing::warn!(path = %path.display(), error = ?e, "bash_runner: sweep remove failed");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
removed
|
||||||
|
}
|
||||||
|
|
||||||
/// On boot, flip any `running` tasks to `interrupted` and fire a wake.
|
/// On boot, flip any `running` tasks to `interrupted` and fire a wake.
|
||||||
async fn mark_interrupted(socket: &Path) {
|
async fn mark_interrupted(socket: &Path) {
|
||||||
let Ok(rd) = std::fs::read_dir(paths::tasks_dir()) else {
|
let Ok(rd) = std::fs::read_dir(paths::tasks_dir()) else {
|
||||||
|
|
@ -967,4 +1029,119 @@ mod tests {
|
||||||
assert!(!body.contains(".out"), "no stdout ⇒ no stdout pointer");
|
assert!(!body.contains(".out"), "no stdout ⇒ no stdout pointer");
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// sweep_old_tasks: a flat, whole-directory age sweep.
|
||||||
|
// `cutoff` is passed in explicitly (mirroring `hive-agent`'s
|
||||||
|
// `has_fresh_credentials(dir, since, ..)` pattern) so these don't need
|
||||||
|
// to fake a real 30-day-old file — `set_modified` moves a fresh
|
||||||
|
// tempfile's mtime directly, no sleeping required.
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn sweep_removes_only_files_older_than_cutoff() {
|
||||||
|
use super::sweep_old_tasks;
|
||||||
|
use std::time::{Duration, SystemTime};
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let old = dir.path().join("old.json");
|
||||||
|
let fresh = dir.path().join("fresh.json");
|
||||||
|
std::fs::write(&old, b"{}").unwrap();
|
||||||
|
std::fs::write(&fresh, b"{}").unwrap();
|
||||||
|
let now = SystemTime::now();
|
||||||
|
std::fs::File::open(&old)
|
||||||
|
.unwrap()
|
||||||
|
.set_modified(now - Duration::from_hours(744)) // 31 days
|
||||||
|
.unwrap();
|
||||||
|
std::fs::File::open(&fresh)
|
||||||
|
.unwrap()
|
||||||
|
.set_modified(now)
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
let removed = sweep_old_tasks(dir.path(), now - Duration::from_hours(720)); // 30 days
|
||||||
|
|
||||||
|
assert_eq!(removed, 1);
|
||||||
|
assert!(!old.exists(), "the 31-day-old file must be gone");
|
||||||
|
assert!(fresh.exists(), "today's file must survive");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn sweep_keeps_a_file_exactly_at_the_cutoff() {
|
||||||
|
// Boundary: `mtime >= cutoff` is kept, matching `sweep_old_tasks`'s
|
||||||
|
// doc — a task's mtime is never used to justify deleting it on the
|
||||||
|
// very instant it crosses the line, only once it's strictly older.
|
||||||
|
use super::sweep_old_tasks;
|
||||||
|
use std::time::SystemTime;
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let path = dir.path().join("boundary.json");
|
||||||
|
std::fs::write(&path, b"{}").unwrap();
|
||||||
|
let cutoff = SystemTime::now();
|
||||||
|
std::fs::File::open(&path)
|
||||||
|
.unwrap()
|
||||||
|
.set_modified(cutoff)
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
let removed = sweep_old_tasks(dir.path(), cutoff);
|
||||||
|
|
||||||
|
assert_eq!(removed, 0);
|
||||||
|
assert!(path.exists());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn sweep_is_extension_agnostic_and_reaps_orphans_and_tmp_files() {
|
||||||
|
// Whole-directory by design (see the doc comment): an orphaned
|
||||||
|
// `.out` with no `.json` (crash mid-task) and a stray `.json.tmp`
|
||||||
|
// (crash mid-rename in `write_task`) are exactly the garbage an
|
||||||
|
// id-driven walk like `mark_interrupted`'s would miss.
|
||||||
|
use super::sweep_old_tasks;
|
||||||
|
use std::time::{Duration, SystemTime};
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let orphan_out = dir.path().join("orphan.out");
|
||||||
|
let stray_tmp = dir.path().join("abc123.json.tmp");
|
||||||
|
std::fs::write(&orphan_out, b"stdout").unwrap();
|
||||||
|
std::fs::write(&stray_tmp, b"{}").unwrap();
|
||||||
|
let past = SystemTime::now() - Duration::from_hours(1440); // 60 days
|
||||||
|
std::fs::File::open(&orphan_out)
|
||||||
|
.unwrap()
|
||||||
|
.set_modified(past)
|
||||||
|
.unwrap();
|
||||||
|
std::fs::File::open(&stray_tmp)
|
||||||
|
.unwrap()
|
||||||
|
.set_modified(past)
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
let removed = sweep_old_tasks(dir.path(), SystemTime::now());
|
||||||
|
|
||||||
|
assert_eq!(removed, 2);
|
||||||
|
assert!(!orphan_out.exists());
|
||||||
|
assert!(!stray_tmp.exists());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn sweep_skips_subdirectories() {
|
||||||
|
// Defensive: `tasks_dir()` holds no directories today, but a
|
||||||
|
// future entry must not be treated as sweepable file content
|
||||||
|
// (`remove_file` on a directory just errors and is swallowed —
|
||||||
|
// this pins the *intent*, not merely today's accidental safety).
|
||||||
|
use super::sweep_old_tasks;
|
||||||
|
use std::time::{Duration, SystemTime};
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let sub = dir.path().join("a-subdir");
|
||||||
|
std::fs::create_dir(&sub).unwrap();
|
||||||
|
|
||||||
|
let removed = sweep_old_tasks(dir.path(), SystemTime::now() + Duration::from_secs(1));
|
||||||
|
|
||||||
|
assert_eq!(removed, 0);
|
||||||
|
assert!(sub.exists());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn sweep_missing_dir_is_a_noop() {
|
||||||
|
// Defensive, same shape as `has_fresh_credentials_missing_dir_is_false`
|
||||||
|
// in `hive-agent`: a not-yet-created tasks dir must not panic.
|
||||||
|
use super::sweep_old_tasks;
|
||||||
|
use std::time::SystemTime;
|
||||||
|
let missing = tempfile::tempdir()
|
||||||
|
.unwrap()
|
||||||
|
.path()
|
||||||
|
.join("never-created-subdir");
|
||||||
|
assert_eq!(sweep_old_tasks(&missing, SystemTime::now()), 0);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue