fix(#1220): remove default 180s timeout from bash run — no timeout unless explicitly set
This commit is contained in:
parent
4fbe9e4927
commit
280ae23c0c
3 changed files with 46 additions and 29 deletions
|
|
@ -138,8 +138,9 @@ fn render_bash_status(id: &str, resp: Result<DaemonResponse>) -> String {
|
||||||
struct BashRunArgs {
|
struct BashRunArgs {
|
||||||
/// Shell command to run (passed to `sh -c`).
|
/// Shell command to run (passed to `sh -c`).
|
||||||
cmd: String,
|
cmd: String,
|
||||||
/// Timeout in seconds. Defaults to 180. Task is killed and marked
|
/// Timeout in seconds. Defaults to `None` (no timeout) — task runs until
|
||||||
/// `timed_out` when the limit is exceeded.
|
/// natural exit. Pass an explicit value to kill the task after N seconds
|
||||||
|
/// and mark it `timed_out`.
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
timeout_secs: Option<u64>,
|
timeout_secs: Option<u64>,
|
||||||
/// Optional inline wait: `run` polls for up to `wait_seconds`
|
/// Optional inline wait: `run` polls for up to `wait_seconds`
|
||||||
|
|
@ -177,12 +178,14 @@ impl BashMcp {
|
||||||
do NOT wait inline. When the command finishes, the harness fires a wake with \
|
do NOT wait inline. When the command finishes, the harness fires a wake with \
|
||||||
`from: \"bash-task-<id>\"` and the exit code + last stdout lines in the body; \
|
`from: \"bash-task-<id>\"` and the exit code + last stdout lines in the body; \
|
||||||
handle it on a future turn. Use `status` to poll the task status within \
|
handle it on a future turn. Use `status` to poll the task status within \
|
||||||
the same turn if needed. `timeout_secs` defaults to 180. Pass `wait_seconds` \
|
the same turn if needed. `timeout_secs` defaults to `None` (no timeout) — \
|
||||||
(capped at 30) to wait inline for fast commands: when the task finishes within \
|
task runs until natural exit; pass an explicit value to kill after N seconds. \
|
||||||
the window the full status is returned immediately and no wake is fired; when \
|
Pass `wait_seconds` (capped at 30) to wait inline for fast commands: when the \
|
||||||
the timeout expires the task keeps running and the normal `task started: id=<id>` \
|
task finishes within the window the full status is returned immediately and no \
|
||||||
response is returned. `wait_seconds` defaults to 3; pass `wait_seconds: 0` to \
|
wake is fired; when the timeout expires the task keeps running and the normal \
|
||||||
disable inline waiting and always get the immediate response."
|
`task started: id=<id>` response is returned. `wait_seconds` defaults to 3; \
|
||||||
|
pass `wait_seconds: 0` to disable inline waiting and always get the immediate \
|
||||||
|
response."
|
||||||
)]
|
)]
|
||||||
async fn run(&self, Parameters(args): Parameters<BashRunArgs>) -> String {
|
async fn run(&self, Parameters(args): Parameters<BashRunArgs>) -> String {
|
||||||
let req = DaemonRequest::BashRun {
|
let req = DaemonRequest::BashRun {
|
||||||
|
|
|
||||||
|
|
@ -25,7 +25,11 @@ pub enum TaskStatus {
|
||||||
pub struct TaskFile {
|
pub struct TaskFile {
|
||||||
pub id: String,
|
pub id: String,
|
||||||
pub cmd: String,
|
pub cmd: String,
|
||||||
pub timeout_secs: u64,
|
/// Kill timeout in seconds. `None` means no timeout — task runs until
|
||||||
|
/// natural exit. Old task files with a numeric value are still readable
|
||||||
|
/// (serde coerces `u64` → `Some(u64)` is handled by the caller).
|
||||||
|
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||||
|
pub timeout_secs: Option<u64>,
|
||||||
pub status: TaskStatus,
|
pub status: TaskStatus,
|
||||||
pub created_at: i64,
|
pub created_at: i64,
|
||||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||||
|
|
|
||||||
|
|
@ -33,9 +33,6 @@ const POLL_INTERVAL: Duration = Duration::from_millis(200);
|
||||||
/// Full output always lives in the `.out`/`.err` files.
|
/// Full output always lives in the `.out`/`.err` files.
|
||||||
pub const SUMMARY_BYTES: usize = 4096;
|
pub const SUMMARY_BYTES: usize = 4096;
|
||||||
|
|
||||||
/// Default task timeout.
|
|
||||||
pub const DEFAULT_TIMEOUT_SECS: u64 = 180;
|
|
||||||
|
|
||||||
/// Maximum inline wait (cap on `wait_seconds`).
|
/// Maximum inline wait (cap on `wait_seconds`).
|
||||||
pub const MAX_WAIT_SECS: u64 = 30;
|
pub const MAX_WAIT_SECS: u64 = 30;
|
||||||
|
|
||||||
|
|
@ -136,7 +133,7 @@ pub fn submit_task(cmd: String, timeout_secs: Option<u64>) -> Result<String> {
|
||||||
let task = TaskFile {
|
let task = TaskFile {
|
||||||
id: id.clone(),
|
id: id.clone(),
|
||||||
cmd,
|
cmd,
|
||||||
timeout_secs: timeout_secs.unwrap_or(DEFAULT_TIMEOUT_SECS),
|
timeout_secs,
|
||||||
status: TaskStatus::Pending,
|
status: TaskStatus::Pending,
|
||||||
created_at: now_unix(),
|
created_at: now_unix(),
|
||||||
started_at: None,
|
started_at: None,
|
||||||
|
|
@ -308,9 +305,8 @@ async fn run_task(mut task: TaskFile, socket: &Path) {
|
||||||
|
|
||||||
let out_path = paths::task_out(&id);
|
let out_path = paths::task_out(&id);
|
||||||
let err_path = paths::task_err(&id);
|
let err_path = paths::task_err(&id);
|
||||||
let timeout = Duration::from_secs(task.timeout_secs);
|
|
||||||
|
|
||||||
let (timed_out, exit_code) = match exec_cmd(&task.cmd, &out_path, &err_path, timeout).await {
|
let (timed_out, exit_code) = match exec_cmd(&task.cmd, &out_path, &err_path, task.timeout_secs).await {
|
||||||
Ok((code, false)) => (false, Some(code)),
|
Ok((code, false)) => (false, Some(code)),
|
||||||
Ok((_, true)) => {
|
Ok((_, true)) => {
|
||||||
tracing::warn!(id = %id, "bash_runner: task timed out");
|
tracing::warn!(id = %id, "bash_runner: task timed out");
|
||||||
|
|
@ -341,7 +337,8 @@ async fn run_task(mut task: TaskFile, socket: &Path) {
|
||||||
refresh_loose_ends();
|
refresh_loose_ends();
|
||||||
|
|
||||||
let summary = if timed_out {
|
let summary = if timed_out {
|
||||||
format!("timed out after {}s", task.timeout_secs)
|
let secs = task.timeout_secs.unwrap_or(0);
|
||||||
|
format!("timed out after {secs}s")
|
||||||
} else {
|
} else {
|
||||||
format!("exit={}", exit_code.unwrap_or(-1))
|
format!("exit={}", exit_code.unwrap_or(-1))
|
||||||
};
|
};
|
||||||
|
|
@ -351,11 +348,12 @@ async fn run_task(mut task: TaskFile, socket: &Path) {
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Run `sh -c cmd`, streaming output to files. Returns `(exit_code, timed_out)`.
|
/// Run `sh -c cmd`, streaming output to files. Returns `(exit_code, timed_out)`.
|
||||||
|
/// `timeout_secs = None` means no timeout — run until natural exit.
|
||||||
async fn exec_cmd(
|
async fn exec_cmd(
|
||||||
cmd: &str,
|
cmd: &str,
|
||||||
out_path: &Path,
|
out_path: &Path,
|
||||||
err_path: &Path,
|
err_path: &Path,
|
||||||
timeout: Duration,
|
timeout_secs: Option<u64>,
|
||||||
) -> Result<(i32, bool)> {
|
) -> Result<(i32, bool)> {
|
||||||
use tokio::process::Command;
|
use tokio::process::Command;
|
||||||
// SAFETY: `nice` is async-signal-safe and modifies only the calling
|
// SAFETY: `nice` is async-signal-safe and modifies only the calling
|
||||||
|
|
@ -389,19 +387,31 @@ async fn exec_cmd(
|
||||||
err_path,
|
err_path,
|
||||||
));
|
));
|
||||||
|
|
||||||
match tokio::time::timeout(timeout, child.wait()).await {
|
if let Some(secs) = timeout_secs {
|
||||||
Ok(Ok(status)) => {
|
match tokio::time::timeout(Duration::from_secs(secs), child.wait()).await {
|
||||||
let _ = copy_out.await;
|
Ok(Ok(status)) => {
|
||||||
let _ = copy_err.await;
|
let _ = copy_out.await;
|
||||||
Ok((status.code().unwrap_or(-1), false))
|
let _ = copy_err.await;
|
||||||
|
Ok((status.code().unwrap_or(-1), false))
|
||||||
|
}
|
||||||
|
Ok(Err(e)) => Err(e.into()),
|
||||||
|
Err(_elapsed) => {
|
||||||
|
let _ = child.kill().await;
|
||||||
|
let _ = child.wait().await;
|
||||||
|
let _ = copy_out.await;
|
||||||
|
let _ = copy_err.await;
|
||||||
|
Ok((-1, true))
|
||||||
|
}
|
||||||
}
|
}
|
||||||
Ok(Err(e)) => Err(e.into()),
|
} else {
|
||||||
Err(_elapsed) => {
|
// No timeout — wait for natural exit.
|
||||||
let _ = child.kill().await;
|
match child.wait().await {
|
||||||
let _ = child.wait().await;
|
Ok(status) => {
|
||||||
let _ = copy_out.await;
|
let _ = copy_out.await;
|
||||||
let _ = copy_err.await;
|
let _ = copy_err.await;
|
||||||
Ok((-1, true))
|
Ok((status.code().unwrap_or(-1), false))
|
||||||
|
}
|
||||||
|
Err(e) => Err(e.into()),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue