bash-mcp: kill of a pending task reports failure when the task file write fails
A failed write left the task Pending on disk, so the poll loop still started it while the kill reply said it was cancelled.
This commit is contained in:
parent
7294707d7d
commit
5cec29ff49
2 changed files with 37 additions and 2 deletions
|
|
@ -256,7 +256,8 @@ impl BashMcp {
|
|||
let (killed, was_running) = runner::kill_task(&args.id, args.force);
|
||||
if !killed {
|
||||
return format!(
|
||||
"no running or pending task with id `{}` (already finished or unknown)",
|
||||
"no running or pending task with id `{}` could be killed (already finished, unknown, \
|
||||
or its pending file could not be updated — see the daemon log)",
|
||||
args.id
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -416,7 +416,10 @@ pub fn kill_task(id: &str, force: bool) -> (bool, bool) {
|
|||
{
|
||||
task.status = TaskStatus::Killed;
|
||||
task.completed_at = Some(chrono::Utc::now());
|
||||
let _ = write_task(&task);
|
||||
if let Err(e) = write_task(&task) {
|
||||
tracing::warn!(id, error = ?e, "kill: could not mark pending task killed");
|
||||
return (false, false);
|
||||
}
|
||||
// A Pending task never reached Running, so it has no keyed "active"
|
||||
// todo to clear and (like the old code) fires no completion signal —
|
||||
// the caller that killed it already knows.
|
||||
|
|
@ -1174,4 +1177,35 @@ mod tests {
|
|||
.join("never-created-subdir");
|
||||
assert_eq!(sweep_old_tasks(&missing, SystemTime::now()), 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn kill_pending_reports_failure_when_the_write_fails() {
|
||||
use super::{kill_task, read_task, write_task};
|
||||
use crate::protocol::{TaskFile, TaskStatus};
|
||||
with_harness_dir(|| {
|
||||
let harness = tempfile::tempdir().unwrap();
|
||||
// SAFETY: serialised by the lock in `with_harness_dir`, which
|
||||
// restores the previous value when the closure returns.
|
||||
unsafe { std::env::set_var("HYPERHIVE_HARNESS_DIR", harness.path()) };
|
||||
std::fs::create_dir_all(crate::paths::tasks_dir()).unwrap();
|
||||
let task = TaskFile {
|
||||
id: "t1".to_owned(),
|
||||
cmd: "echo hi".to_owned(),
|
||||
timeout_secs: None,
|
||||
status: TaskStatus::Pending,
|
||||
created_at: chrono::Utc::now(),
|
||||
started_at: None,
|
||||
completed_at: None,
|
||||
exit_code: None,
|
||||
stdout_tail: None,
|
||||
stderr_tail: None,
|
||||
};
|
||||
write_task(&task).unwrap();
|
||||
// A directory at the temp path makes `write_task` fail even as root.
|
||||
std::fs::create_dir(crate::paths::task_json("t1").with_extension("json.tmp")).unwrap();
|
||||
|
||||
assert_eq!(kill_task("t1", false), (false, false));
|
||||
assert_eq!(read_task("t1").unwrap().status, TaskStatus::Pending);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue