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
49 changes: 44 additions & 5 deletions crates/buzz-acp/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ use crate::filter::SubscriptionRule;
///
/// Sized for slow turns where the agent may go silent on its outer ACP channel
/// while running long sub-tools (e.g. a buzz-agent running another agent, or
/// codex/claude doing multi-minute single tool calls). 900s gives 300s of
/// breathing room above the 600s max shell timeout, so legitimate long-running
/// codex/claude doing multi-minute single tool calls). 1500s gives 300s of
/// breathing room above the 1200s max shell timeout, so legitimate long-running
/// tool calls don't race the idle deadline.
/// Override via `--idle-timeout` / `BUZZ_ACP_IDLE_TIMEOUT`.
pub(crate) const DEFAULT_IDLE_TIMEOUT_SECS: u64 = 900;
pub(crate) const DEFAULT_IDLE_TIMEOUT_SECS: u64 = 1_500;

/// Default absolute wall-clock cap per agent turn (2 hours).
/// Override via `--max-turn-duration` / `BUZZ_ACP_MAX_TURN_DURATION`.
Expand Down Expand Up @@ -2757,9 +2757,9 @@ channels = "ALL"
// ── Idle timeout constant + guard (PR #935) ───────────────────────────────

#[test]
fn default_idle_timeout_is_900_seconds() {
fn default_idle_timeout_is_1500_seconds() {
// Lock the constant value so accidental changes are caught.
assert_eq!(DEFAULT_IDLE_TIMEOUT_SECS, 900);
assert_eq!(DEFAULT_IDLE_TIMEOUT_SECS, 1_500);
}

#[test]
Expand All @@ -2779,6 +2779,45 @@ channels = "ALL"
}
}

#[test]
fn budget_ordering_invariant_shell_cap_plus_headroom_fits_within_idle_timeout() {
// Asserts the three-layer budget relationship introduced in PR #7185:
// buzz-dev-mcp MAX_TIMEOUT_MS (1 200 000 ms = 1 200s)
// ≤ buzz-agent BUZZ_AGENT_TOOL_TIMEOUT_SECS default (1 260s)
// < buzz-acp DEFAULT_IDLE_TIMEOUT_SECS (1 500s)
//
// The idle deadline must strictly outlast the agent tool timeout so a
// legitimately long-running tool call is killed by buzz-agent first (at
// 1 260s) rather than the ACP idle watchdog. The 240s gap gives the agent
// time to handle the timeout, emit a response, and reset the idle clock
// before the ACP connection dies.
//
// If any of these constants change the compiler catches the inversion here.
// Cross-crate constants are mirrored as literals; grep for PR #7185 to
// find the authoritative source if you need to update them.
const SHELL_CAP_MS: u64 = 1_200_000; // buzz-dev-mcp MAX_TIMEOUT_MS
const SHELL_CAP_SECS: u64 = SHELL_CAP_MS / 1_000;
const AGENT_TOOL_TIMEOUT_SECS: u64 = 1_260; // buzz-agent BUZZ_AGENT_TOOL_TIMEOUT_SECS default

const {
// Shell cap must not exceed the agent's per-tool-call timeout.
assert!(
SHELL_CAP_SECS <= AGENT_TOOL_TIMEOUT_SECS,
"shell cap must be <= agent tool timeout"
);
// Agent tool timeout must be strictly less than the ACP idle deadline.
assert!(
AGENT_TOOL_TIMEOUT_SECS < DEFAULT_IDLE_TIMEOUT_SECS,
"agent tool timeout must be < ACP idle timeout"
);
// ACP idle timeout must remain below the max turn duration.
assert!(
DEFAULT_IDLE_TIMEOUT_SECS < DEFAULT_MAX_TURN_DURATION_SECS,
"ACP idle timeout must be < max turn duration"
);
}
}

// --- BUZZ_ACP_ALLOWED_RESPOND_TO gate ---

fn parse_allowed_respond_to(raw: &[&str]) -> Result<HashSet<RespondTo>, ConfigError> {
Expand Down
4 changes: 2 additions & 2 deletions crates/buzz-agent/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ Everything is environment variables. No flags, no config files. (We are a subpro
| `BUZZ_AGENT_MAX_CONTEXT_TOKENS` | `200000` | Provider context window used by the handoff gate. |
| `BUZZ_AGENT_MAX_HANDOFFS` | `10` | Max context handoffs per session before falling back to truncation. |
| `BUZZ_AGENT_LLM_TIMEOUT_SECS` | `240` | Max seconds with no response bytes before abandoning an LLM call (per-read inactivity, not wall-clock). |
| `BUZZ_AGENT_TOOL_TIMEOUT_SECS` | `660` | Per-tool call timeout in seconds |
| `BUZZ_AGENT_TOOL_TIMEOUT_SECS` | `1260` | Per-tool call timeout in seconds |
| `BUZZ_AGENT_MAX_PARALLEL_TOOLS` | `8` | Max concurrent tool calls per turn (1 = sequential) |
| `BUZZ_AGENT_MAX_SESSIONS` | unlimited | Max concurrent ACP sessions. Sessions are cheap; default has no cap. |
| `BUZZ_AGENT_MAX_LINE_BYTES` | `4194304` | 4 MiB. Hard cap on inbound JSON-RPC frames. |
Expand Down Expand Up @@ -326,7 +326,7 @@ The trust boundary is **the operator who launched the agent**. The harness, MCP
| Tool calls per turn | 64 | `MAX_TOOL_CALLS_PER_TURN` |
| Loop rounds | 0 (unlimited) | `BUZZ_AGENT_MAX_ROUNDS` |
| LLM read inactivity timeout | 240 s | `BUZZ_AGENT_LLM_TIMEOUT_SECS` |
| Tool call timeout | 660 s | `BUZZ_AGENT_TOOL_TIMEOUT_SECS` |
| Tool call timeout | 1260 s | `BUZZ_AGENT_TOOL_TIMEOUT_SECS` |

## What This Is NOT

Expand Down
22 changes: 21 additions & 1 deletion crates/buzz-agent/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -709,7 +709,7 @@ impl Config {
max_output_tokens: parse_env("BUZZ_AGENT_MAX_OUTPUT_TOKENS", 65_536)?,
max_token_recoveries: parse_env("BUZZ_AGENT_MAX_TOKEN_RECOVERIES", 3u32)?,
llm_timeout: Duration::from_secs(parse_env("BUZZ_AGENT_LLM_TIMEOUT_SECS", 240)?),
tool_timeout: Duration::from_secs(parse_env("BUZZ_AGENT_TOOL_TIMEOUT_SECS", 660)?),
tool_timeout: Duration::from_secs(parse_env("BUZZ_AGENT_TOOL_TIMEOUT_SECS", 1_260)?),
mcp_init_timeout: Duration::from_secs(parse_env(
"BUZZ_AGENT_MCP_INIT_TIMEOUT_SECS",
30,
Expand Down Expand Up @@ -2462,4 +2462,24 @@ mod tests {
assert_eq!(pricing_authority("https://api.databricks.com/v1"), None);
assert_eq!(pricing_authority("https://custom.llm.corp/v1"), None);
}

#[test]
fn default_tool_timeout_is_1260_seconds() {
// Lock the production default so accidental regressions are caught.
// This value must remain >= buzz-dev-mcp's MAX_TIMEOUT_MS (1_200s) to
// give every shell(timeout_ms=1_200_000) call time to complete before
// buzz-agent kills the MCP server. See PR #7185 for the full budget chain.
//
// 1_260s is the literal default passed to parse_env in Config::from_env().
// Update here if and only if you update that literal; the test name makes
// "grep for old value" reliable.
const DEFAULT_TOOL_TIMEOUT_SECS: u64 = 1_260;
const {
// Shell cap (1_200_000 ms = 1_200s) must fit inside the agent timeout.
assert!(
1_200u64 <= DEFAULT_TOOL_TIMEOUT_SECS,
"agent tool timeout must be >= dev-mcp shell cap (1200s)"
);
}
}
}
2 changes: 1 addition & 1 deletion crates/buzz-dev-mcp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ impl DevMcp {

#[tool(
name = "shell",
description = "Run a shell command (bash by default; set `BUZZ_SHELL` to use cmd, PowerShell, or another shell). Ephemeral process per call. Output tail-truncated to ~8KB for the LLM; full output (first 10MB) saved to artifact file. timeout_ms defaults to 120000 (2 min) if omitted; capped at 600000 (10 min). For long-running commands (git push with hooks, cargo build, test suites), use 300000+. On PATH: rg (prefer over grep; flags: -n -i -l -g <glob> -C <n> --files), tree (flags: -d <depth>; shows line counts), and buzz (Buzz relay CLI — run buzz --help for commands)."
description = "Run a shell command (bash by default; set `BUZZ_SHELL` to use cmd, PowerShell, or another shell). Ephemeral process per call. Output tail-truncated to ~8KB for the LLM; full output (first 10MB) saved to artifact file. timeout_ms defaults to 120000 (2 min) if omitted; capped at 1,200,000 (20 min). For long-running commands (git push with hooks, cargo build, test suites), use 300000+. On PATH: rg (prefer over grep; flags: -n -i -l -g <glob> -C <n> --files), tree (flags: -d <depth>; shows line counts), and buzz (Buzz relay CLI — run buzz --help for commands)."
)]
async fn shell(
&self,
Expand Down
22 changes: 16 additions & 6 deletions crates/buzz-dev-mcp/src/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use tokio::process::Command;
use tokio_util::sync::CancellationToken;

const DEFAULT_TIMEOUT_MS: u64 = 120_000;
const MAX_TIMEOUT_MS: u64 = 600_000;
const MAX_TIMEOUT_MS: u64 = 1_200_000;
const MAX_COMMAND_BYTES: usize = 1_000_000;
const CAPTURE_CAP: usize = 10 * 1024 * 1024;
const MAX_BYTES: usize = 50 * 1024;
Expand Down Expand Up @@ -121,12 +121,16 @@ pub struct ShellParams {
pub command: String,
#[serde(default)]
pub workdir: Option<String>,
/// Defaults to 120000 ms (2 min) if omitted; capped at 600000 ms (10 min).
/// Defaults to 120000 ms (2 min) if omitted; capped at 1,200,000 ms (20 min).
/// For long-running commands (git push with hooks, cargo build, test suites), use 300000+.
#[serde(default)]
pub timeout_ms: Option<u64>,
}

fn effective_timeout_ms(requested: Option<u64>) -> u64 {
requested.unwrap_or(DEFAULT_TIMEOUT_MS).min(MAX_TIMEOUT_MS)
}

pub async fn run(
state: &SharedState,
p: ShellParams,
Expand All @@ -138,10 +142,7 @@ pub async fn run(
None,
));
}
let timeout_ms = p
.timeout_ms
.unwrap_or(DEFAULT_TIMEOUT_MS)
.min(MAX_TIMEOUT_MS);
let timeout_ms = effective_timeout_ms(p.timeout_ms);
let workdir: PathBuf = p
.workdir
.as_deref()
Expand Down Expand Up @@ -1002,6 +1003,15 @@ mod tests {
serde_json::from_str(&text).expect("json")
}

#[test]
fn timeout_bounds_preserve_default_and_cap_requests_at_twenty_minutes() {
assert_eq!(effective_timeout_ms(None), 120_000);
assert_eq!(effective_timeout_ms(Some(120_000)), 120_000);
assert_eq!(effective_timeout_ms(Some(1_200_000)), 1_200_000);
assert_eq!(effective_timeout_ms(Some(1_200_001)), 1_200_000);
assert_eq!(effective_timeout_ms(Some(u64::MAX)), 1_200_000);
}

#[tokio::test(flavor = "current_thread")]
async fn basic_echo() {
let dir = tempdir().expect("tempdir");
Expand Down
Loading