fix(buzz-dev-mcp): configurable shell-timeout ceiling; report the clamp - #4819
fix(buzz-dev-mcp): configurable shell-timeout ceiling; report the clamp#4819alanshurafa wants to merge 1 commit into
Conversation
Every shell call was capped at 600s by an unconfigurable .min(), and the clamp was invisible: a caller that asked for 3600s was silently given 600s, and on expiry the process-group kill took the child's own output with it, so the agent saw a truncated log rather than a timeout. Four consecutive real agent runs died this way at exactly 600s and the agent diagnosed its model backend as broken (block#4638). - The ceiling is now BUZZ_DEV_MCP_MAX_TIMEOUT_MS (default unchanged at 600000 ms), resolved once at startup; unparseable or zero values fall back to the default rather than lowering the ceiling. - The clamp is reported in-band via the result's existing notes array (requested vs ceiling, naming the env var), and expiry adds a note naming the effective timeout, so a killed job is attributable without guessing from a truncated stream. timed_out/exit_code 124 semantics are unchanged; notes are additive. - The ShellParams schema doc no longer hard-codes the cap. - buzz-acp's DEFAULT_IDLE_TIMEOUT_SECS comment now states the pairing: operators raising the shell ceiling past 900s must raise BUZZ_ACP_IDLE_TIMEOUT too, or long tool calls die there instead (the knob already exists; see also BUZZ_ACP_MAX_TURN_DURATION past 7200s). Fixes block#4638 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Agent57 <agent57@shurafa.com>
|
@block/buzz-oss-team — could a maintainer please approve the workflow runs for this fork PR and do a code-owner review? The reachable checks (Semgrep OSS, zizmor, DCO) should pass; branch is based on 28ae6cd (buzz-dev-mcp unchanged vs current main). Thanks! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9e6be1865
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| session_dir, | ||
| bootstrap_instructions, | ||
| resolved_shell, | ||
| max_timeout_ms: resolve_max_timeout_ms(std::env::var(MAX_TIMEOUT_ENV).ok()), |
There was a problem hiding this comment.
Pass the ceiling env through buzz-agent
When buzz-dev-mcp is launched as a tool server behind buzz-agent, this env lookup never sees an operator-set BUZZ_DEV_MCP_MAX_TIMEOUT_MS: buzz-agent::spawn_one calls env_clear() and only restores names in PASSTHROUGH_ENV plus the per-server spec env, and BUZZ_DEV_MCP_MAX_TIMEOUT_MS is not in that allowlist (crates/buzz-agent/src/mcp.rs:39-94, :742-758). In the managed agent path, setting the new knob on the harness/agent therefore silently falls back to 600000 ms, so the reported fix does not work for the primary dev-MCP integration unless this env var is forwarded into the MCP subprocess.
Useful? React with 👍 / 👎.
| let requested_timeout_ms = p.timeout_ms.unwrap_or(DEFAULT_TIMEOUT_MS); | ||
| let timeout_ms = requested_timeout_ms.min(state.max_timeout_ms); |
There was a problem hiding this comment.
Keep buzz-agent's tool timeout above the shell ceiling
If the new ceiling is successfully raised above 660000 ms and the MCP server is used through buzz-agent, long shell calls still cannot reach that ceiling because every MCP tool call is wrapped in tokio::time::timeout(self.cfg.tool_timeout, ...), whose default is BUZZ_AGENT_TOOL_TIMEOUT_SECS=660 (crates/buzz-agent/src/config.rs:844, crates/buzz-agent/src/agent.rs:654-680). In that common path a 20-minute shell request is accepted here, but buzz-agent kills the MCP server at 11 minutes before buzz-dev-mcp can return the new timeout/clamp notes, so raising this ceiling needs to also raise or document/configure the agent tool timeout.
Useful? React with 👍 / 👎.
Summary
buzz-dev-mcpcapped every shell call at 600 s via an unconfigurable.min(MAX_TIMEOUT_MS), and the clamp was invisible twice over: a caller asking for 3600 s was silently given 600 s (timeout_msis an accepted tool parameter, so callers reasonably believe it was honored), and on expiry the process-group kill takes the child's own output buffer with it, so what the agent sees is a truncated log — not a timeout. The reporter lost four consecutive real agent runs to this, each dying at exactly 600 s with a 15-byte log, and the agent diagnosed its model backend as broken.This implements both of the issue's asks:
BUZZ_DEV_MCP_MAX_TIMEOUT_MS(default unchanged, 600 000 ms), resolved once atSharedStateconstruction. Unparseable or zero values fall back to the built-in default rather than lowering the ceiling — a misconfigured knob must never make every command die instantly (unit-tested).notes: []array (additive — no schema change,timed_out/exit_code: 124semantics untouched): a request above the ceiling gets "requested … exceeds the … ceiling; clamped (set BUZZ_DEV_MCP_MAX_TIMEOUT_MS to raise the ceiling)", and expiry adds "process group killed at the {N} ms timeout", so a killed job is attributable from inside the result.Also updated: the
ShellParamsschema doc-comment no longer hard-codes "capped at 600000 ms", and — one comment line inbuzz-acp—DEFAULT_IDLE_TIMEOUT_SECS's doc now states the coupling this issue's scenario runs into next: the 900 s ACP idle timer is sized to the default shell ceiling, so operators raising the ceiling past ~900 s must raiseBUZZ_ACP_IDLE_TIMEOUTto match (the knob already exists; past 7200 s,BUZZ_ACP_MAX_TURN_DURATIONas well). Without that pairing the reporter's agent jobs would next die at 900 s. Related: #935 raised the idle default for exactly the adjacent reason; the issue itself draws the distinction.Related issue
Fixes #4638. Duplicate search at filing (2026-08-04): no open or closed PR addresses the shell-timeout ceiling (nearest match #4261 is buzz-acp
idle_timeout, a different knob in a different crate).Testing
cargo test -p buzz-dev-mcp --lib -- --test-threads=1— 121 passed, 0 failed (new: env-resolver cases incl. zero/garbage/empty fallbacks; clamp note asserted end-to-end with a lowered ceiling; the existingtimeout_firestest now also asserts the kill-attribution note).cargo clippy --workspace --all-targets -- -D warnings,cargo fmt --all -- --check— clean.buzz-dev-mcpis unchanged between that and current main.