@sigitcodereview / sigit / commits / 1bbe991

Let background polls wait instead of tripping the repeat guard

Watching a slow background command (a release workflow, say) meant calling command_output with the same task id again and again. The third identical call hit the repeated-tool-call guard, which refused to run it and dropped tools for the rest of the turn, so the model ended the turn without a result. command_output now long-polls: it waits up to wait_seconds (default 10, max 30) for new output or an exit before it returns. The repeat guard also exempts it, since polling the same task is the whole point of the tool. MAX_TOOL_ROUNDS still caps a model that polls forever. Co-Authored-By: siGit Code v1.5.11 <noreply@sigit.si> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Seto Elkahfi committed Sep 30, 2026 at 07:09 UTC 1bbe9913cdad0fb7aabd81445b3260b7e090967e

3 files changed +137 -9

src/main.rs

+5 -1

@@ -2332,7 +2332,11 @@ impl SiGitAgent {

2332 .entry(signature)

2333 .and_modify(|count| *count += 1)

2334 .or_insert(1);

2335 - let repeated = *repeat_count >= 3;

2335 + // Repeated status checks are expected for a live background

2336 + // command. command_output long-polls and the overall round cap

2337 + // still bounds a confused model without cutting off a real

2338 + // build or release while it is in progress.

2339 + let repeated = *repeat_count >= 3 && tc.name != "command_output";

2340 if repeated {

2341 force_text = true;

2342 log::warn!(

src/tools.rs

+75 -4

@@ -390,9 +390,10 @@ pub fn all_tools() -> Vec<AgentTool> {

390 name: "command_output",

391 description: "Get the output a background task (started with run_command's \

392 run_in_background) has produced since your last check, plus its \

393 - status: still running, or exited with an exit code. Poll this \

394 - periodically to follow builds, test suites, and servers. Between \

395 - polls output is buffered up to 50 000 bytes per task; older \

393 + status: still running, or exited with an exit code. By default the \

394 + call waits up to 10 seconds for new output or completion, so call it \

395 + again when the task is still running rather than ending the turn. \

396 + Between polls output is buffered up to 50 000 bytes per task; older \

397 output beyond that is dropped and the truncation is noted.",

398 parameters_schema: json!({

399 "type": "object",

@@ -400,6 +401,12 @@ pub fn all_tools() -> Vec<AgentTool> {

401 "task_id": {

402 "type": "integer",

403 "description": "Id of the background task, as returned by run_command with run_in_background."

404 + },

405 + "wait_seconds": {

406 + "type": "integer",

407 + "minimum": 0,

408 + "maximum": 30,

409 + "description": "Seconds to wait for new output or task completion before returning. Defaults to 10; use 0 for an immediate status check."

410 }

411 },

412 "required": ["task_id"],

@@ -458,6 +465,7 @@ async fn execute_tool_impl(name: &str, arguments: &str) -> String {

465 match name {

466 TASK_TOOL_NAME => exec_task(arguments).await,

467 WEB_SEARCH_TOOL_NAME => exec_web_search(arguments).await,

468 + "command_output" => exec_command_output_wait(arguments).await,

469 // Tools discovered from MCP servers are namespaced `mcp__<server>__<tool>`

470 // and forwarded to the owning server.

471 _ if crate::mcp::is_mcp_tool(name) => crate::mcp::call_tool(name, arguments).await,

@@ -515,7 +523,6 @@ fn execute_sync_tool(name: &str, arguments: &str, owner: Option<&str>) -> String

523 "remember" => exec_remember(arguments),

524 "delete_file" => exec_delete_file(arguments),

525 "run_command" => exec_run_command(arguments, owner),

518 - "command_output" => exec_command_output(arguments, owner),

526 "kill_command" => exec_kill_command(arguments, owner),

527 "skill" => crate::skills::activate_skill(arguments),

528 _ => format!("Unknown tool: {name}"),

@@ -2526,6 +2533,7 @@ fn exec_command_output(arguments: &str, owner: Option<&str>) -> String {

2533 // the final output through the pipes before draining the buffer.

2534 std::thread::sleep(std::time::Duration::from_millis(100));

2535 }

2536 +

2537 let (new_output, dropped) = drain_task_output(task);

2538

2539 let command = &task.command;

@@ -2548,6 +2556,42 @@ fn exec_command_output(arguments: &str, owner: Option<&str>) -> String {

2556 }

2557 }

2558

2559 +/// Long-poll a background task without occupying Tokio's blocking pool while

2560 +/// nothing changes. This keeps model tool rounds from being consumed by a

2561 +/// tight status-check loop.

2562 +async fn exec_command_output_wait(arguments: &str) -> String {

2563 + const DEFAULT_WAIT_SECONDS: u64 = 10;

2564 + const MAX_WAIT_SECONDS: u64 = 30;

2565 + const POLL_INTERVAL: std::time::Duration = std::time::Duration::from_millis(200);

2566 +

2567 + let wait_seconds = serde_json::from_str::<Value>(arguments)

2568 + .ok()

2569 + .and_then(|args| args.get("wait_seconds").and_then(Value::as_u64))

2570 + .unwrap_or(DEFAULT_WAIT_SECONDS)

2571 + .min(MAX_WAIT_SECONDS);

2572 + let owner = active_session();

2573 + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(wait_seconds);

2574 +

2575 + loop {

2576 + let arguments = arguments.to_owned();

2577 + let owner = owner.clone();

2578 + let output =

2579 + tokio::task::spawn_blocking(move || exec_command_output(&arguments, owner.as_deref()))

2580 + .await

2581 + .unwrap_or_else(|err| format!("Error: command_output task failed: {err}"));

2582 +

2583 + if wait_seconds == 0

2584 + || !output.contains(" is still running.")

2585 + || !output.contains(" No new output since the last check.")

2586 + || tokio::time::Instant::now() >= deadline

2587 + {

2588 + return output;

2589 + }

2590 +

2591 + tokio::time::sleep(POLL_INTERVAL).await;

2592 + }

2593 +}

2594 +

2595 /// `kill_command` tool: stop a background task and report its output tail.

2596 fn exec_kill_command(arguments: &str, owner: Option<&str>) -> String {

2597 let task_id = match parse_task_id(arguments) {

@@ -3783,6 +3827,33 @@ mod tests {

3827 assert!(combined.contains("done"), "got: {combined}");

3828 }

3829

3830 + #[tokio::test]

3831 + async fn test_command_output_waits_for_background_completion() {

3832 + #[cfg(unix)]

3833 + let command = "sleep 1; echo done";

3834 + #[cfg(windows)]

3835 + let command = "ping -n 2 127.0.0.1 > nul&& echo done";

3836 +

3837 + let args = serde_json::json!({

3838 + "command": command,

3839 + "cwd": std::env::temp_dir(),

3840 + "run_in_background": true

3841 + })

3842 + .to_string();

3843 + let result = exec_run_command(&args, None);

3844 + let task_id = background_task_id(&result);

3845 +

3846 + let poll_args = serde_json::json!({

3847 + "task_id": task_id,

3848 + "wait_seconds": 5

3849 + })

3850 + .to_string();

3851 + let poll = exec_command_output_wait(&poll_args).await;

3852 +

3853 + assert!(poll.contains("exited with code 0"), "got: {poll}");

3854 + assert!(poll.contains("done"), "got: {poll}");

3855 + }

3856 +

3857 #[test]

3858 fn test_kill_command_stops_background_task() {

3859 #[cfg(unix)]

tests/acp_permissions.rs

+57 -4

@@ -1038,11 +1038,11 @@ fn an_unparseable_tool_call_is_hidden_and_retried() {

1038 /// response without being executed or recorded as orphaned history entries.

1039 #[test]

1040 fn forced_text_suppresses_glm_check_status_tool_markup() {

1041 - let repeated_arguments = json!({"task_id": 1}).to_string();

1041 + let repeated_arguments = json!({"path": "missing.txt"}).to_string();

1042 let endpoint = start_fake_endpoint(vec![

1043 - sse_tool_call("call_1", "command_output", &repeated_arguments),

1044 - sse_tool_call("call_2", "command_output", &repeated_arguments),

1045 - sse_tool_call("call_3", "command_output", &repeated_arguments),

1043 + sse_tool_call("call_1", "read_file", &repeated_arguments),

1044 + sse_tool_call("call_2", "read_file", &repeated_arguments),

1045 + sse_tool_call("call_3", "read_file", &repeated_arguments),

1046 sse_body(&[

1047 json!({"choices": [{"delta": {"content": "Build is still running. <tool_call>command_output CheckStatus=true_or_poll_"}}]}),

1048 json!({"choices": [{"delta": {"content": "again_with_different_params</arg_value><arg_key>task_id</arg_key><arg_value>1</arg_value></tool_call>"}}]}),

@@ -1227,6 +1227,59 @@ fn a_kimi_k3_tool_call_emitted_as_text_is_executed_rather_than_rendered() {

1227 let _ = std::fs::remove_dir_all(&scratch);

1228 }

1229

1230 +#[test]

1231 +fn repeated_command_output_calls_remain_available() {

1232 + let repeated_arguments = json!({"task_id": 999}).to_string();

1233 + let endpoint = start_fake_endpoint(vec![

1234 + sse_tool_call("call_1", "command_output", &repeated_arguments),

1235 + sse_tool_call("call_2", "command_output", &repeated_arguments),

1236 + sse_tool_call("call_3", "command_output", &repeated_arguments),

1237 + sse_text("The background task finished."),

1238 + ]);

1239 +

1240 + let scratch =

1241 + std::env::temp_dir().join(format!("sigit_acp_repeated_poll_{}", std::process::id()));

1242 + let config_dir = scratch.join("config");

1243 + let cwd = scratch.join("cwd");

1244 + std::fs::create_dir_all(&config_dir).unwrap();

1245 + std::fs::create_dir_all(&cwd).unwrap();

1246 +

1247 + let mut agent = spawn_agent(endpoint.port, &config_dir);

1248 + let id = agent.request(

1249 + "initialize",

1250 + json!({"protocolVersion": 1, "clientCapabilities": {}}),

1251 + );

1252 + agent.wait_for_response(id);

1253 + let id = agent.request("session/new", json!({"cwd": cwd, "mcpServers": []}));

1254 + let session_id = agent.wait_for_response(id)["result"]["sessionId"]

1255 + .as_str()

1256 + .expect("session id")

1257 + .to_string();

1258 +

1259 + let prompt_id = agent.request(

1260 + "session/prompt",

1261 + json!({

1262 + "sessionId": session_id,

1263 + "prompt": [{"type": "text", "text": "wait for the background task"}],

1264 + }),

1265 + );

1266 + let (response, rendered) = agent.wait_for_prompt(prompt_id);

1267 + assert_eq!(response["result"]["stopReason"], "end_turn");

1268 + assert!(rendered.contains("finished"), "got: {rendered:?}");

1269 +

1270 + let requests = endpoint.requests.lock().unwrap();

1271 + assert_eq!(requests.len(), 4);

1272 + assert!(

1273 + requests[3].get("tools").is_some(),

1274 + "command_output repetition must not force tools off: {:?}",

1275 + requests[3]

1276 + );

1277 + drop(requests);

1278 +

1279 + drop(agent);

1280 + let _ = std::fs::remove_dir_all(&scratch);

1281 +}

1282 +

1283 #[test]

1284 fn write_todos_reaches_the_client_as_an_acp_plan() {

1285 let endpoint = start_fake_endpoint(vec![