fix: Prevent infinite hangs in coder agent and implement timeouts for LLM API calls and interactive tools
This commit is contained in:
@@ -31,7 +31,7 @@ settings:
|
|||||||
max_loop_iterations: 20
|
max_loop_iterations: 20
|
||||||
log_state_snapshots: true
|
log_state_snapshots: true
|
||||||
validate_before_run: true
|
validate_before_run: true
|
||||||
timeout: 1800
|
timeout: 14400
|
||||||
|
|
||||||
initial_state:
|
initial_state:
|
||||||
project_dir: ''
|
project_dir: ''
|
||||||
@@ -90,6 +90,7 @@ nodes:
|
|||||||
Project directory: {{project_dir}}
|
Project directory: {{project_dir}}
|
||||||
prompt: '{{initial_prompt}}'
|
prompt: '{{initial_prompt}}'
|
||||||
tools: []
|
tools: []
|
||||||
|
timeout: 300
|
||||||
output_schema:
|
output_schema:
|
||||||
type: object
|
type: object
|
||||||
properties:
|
properties:
|
||||||
@@ -254,6 +255,7 @@ nodes:
|
|||||||
- fs_patch
|
- fs_patch
|
||||||
- execute_command
|
- execute_command
|
||||||
max_iterations: 100
|
max_iterations: 100
|
||||||
|
timeout: 1800
|
||||||
state_updates:
|
state_updates:
|
||||||
last_node_output: '{{output}}'
|
last_node_output: '{{output}}'
|
||||||
fallback: end_failure
|
fallback: end_failure
|
||||||
@@ -327,6 +329,7 @@ nodes:
|
|||||||
- fs_ls
|
- fs_ls
|
||||||
- execute_command
|
- execute_command
|
||||||
max_iterations: 15
|
max_iterations: 15
|
||||||
|
timeout: 600
|
||||||
output_schema:
|
output_schema:
|
||||||
type: object
|
type: object
|
||||||
properties:
|
properties:
|
||||||
|
|||||||
@@ -292,6 +292,7 @@ clients:
|
|||||||
# extra:
|
# extra:
|
||||||
# proxy: socks5://127.0.0.1:1080 # Set proxy
|
# proxy: socks5://127.0.0.1:1080 # Set proxy
|
||||||
# connect_timeout: 10 # Set timeout in seconds for connect to api
|
# connect_timeout: 10 # Set timeout in seconds for connect to api
|
||||||
|
# read_timeout: 300 # Set timeout in seconds for a read stall (no bytes received); 0 disables (default: 300)
|
||||||
|
|
||||||
# See https://platform.openai.com/docs/quickstart
|
# See https://platform.openai.com/docs/quickstart
|
||||||
- type: openai
|
- type: openai
|
||||||
|
|||||||
@@ -57,12 +57,16 @@ pub trait Client: Sync + Send {
|
|||||||
let mut builder = ReqwestClient::builder();
|
let mut builder = ReqwestClient::builder();
|
||||||
let extra = self.extra_config();
|
let extra = self.extra_config();
|
||||||
let timeout = extra.and_then(|v| v.connect_timeout).unwrap_or(10);
|
let timeout = extra.and_then(|v| v.connect_timeout).unwrap_or(10);
|
||||||
|
let read_timeout = extra.and_then(|v| v.read_timeout).unwrap_or(300);
|
||||||
if let Some(proxy) = extra.and_then(|v| v.proxy.as_deref()) {
|
if let Some(proxy) = extra.and_then(|v| v.proxy.as_deref()) {
|
||||||
builder = set_proxy(builder, proxy)?;
|
builder = set_proxy(builder, proxy)?;
|
||||||
}
|
}
|
||||||
if let Some(user_agent) = self.app_config().user_agent.as_ref() {
|
if let Some(user_agent) = self.app_config().user_agent.as_ref() {
|
||||||
builder = builder.user_agent(user_agent);
|
builder = builder.user_agent(user_agent);
|
||||||
}
|
}
|
||||||
|
if read_timeout > 0 {
|
||||||
|
builder = builder.read_timeout(Duration::from_secs(read_timeout));
|
||||||
|
}
|
||||||
let client = builder
|
let client = builder
|
||||||
.connect_timeout(Duration::from_secs(timeout))
|
.connect_timeout(Duration::from_secs(timeout))
|
||||||
.build()
|
.build()
|
||||||
@@ -261,6 +265,7 @@ impl Default for ClientConfig {
|
|||||||
pub struct ExtraConfig {
|
pub struct ExtraConfig {
|
||||||
pub proxy: Option<String>,
|
pub proxy: Option<String>,
|
||||||
pub connect_timeout: Option<u64>,
|
pub connect_timeout: Option<u64>,
|
||||||
|
pub read_timeout: Option<u64>,
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Debug, Clone, Deserialize, Default)]
|
#[derive(Debug, Clone, Deserialize, Default)]
|
||||||
|
|||||||
@@ -430,10 +430,7 @@ mod tests {
|
|||||||
assert!(error_message.contains("test_function_loop"));
|
assert!(error_message.contains("test_function_loop"));
|
||||||
}
|
}
|
||||||
|
|
||||||
fn new_handler() -> (
|
fn new_handler() -> (SseHandler, tokio::sync::mpsc::UnboundedReceiver<SseEvent>) {
|
||||||
SseHandler,
|
|
||||||
tokio::sync::mpsc::UnboundedReceiver<SseEvent>,
|
|
||||||
) {
|
|
||||||
let (sender, receiver) = tokio::sync::mpsc::unbounded_channel();
|
let (sender, receiver) = tokio::sync::mpsc::unbounded_channel();
|
||||||
let abort_signal = crate::utils::create_abort_signal();
|
let abort_signal = crate::utils::create_abort_signal();
|
||||||
(SseHandler::new(sender, abort_signal), receiver)
|
(SseHandler::new(sender, abort_signal), receiver)
|
||||||
|
|||||||
@@ -103,9 +103,7 @@ impl McpFactory {
|
|||||||
}
|
}
|
||||||
|
|
||||||
let (auth, auth_reason) = resolve_http_auth(name, spec).await;
|
let (auth, auth_reason) = resolve_http_auth(name, spec).await;
|
||||||
let handle = spawn_mcp_server(spec, log_path, auth)
|
let handle = spawn_mcp_server(spec, log_path, auth).await.map_err(|e| {
|
||||||
.await
|
|
||||||
.map_err(|e| {
|
|
||||||
if is_auth_required_error(&e) {
|
if is_auth_required_error(&e) {
|
||||||
e.context(McpAuthRequired {
|
e.context(McpAuthRequired {
|
||||||
server: name.to_string(),
|
server: name.to_string(),
|
||||||
|
|||||||
+38
-6
@@ -29,16 +29,17 @@ use rust_embed::Embed;
|
|||||||
use serde::{Deserialize, Serialize};
|
use serde::{Deserialize, Serialize};
|
||||||
use serde_json::{Value, json};
|
use serde_json::{Value, json};
|
||||||
use skill::SKILL_FUNCTION_PREFIX;
|
use skill::SKILL_FUNCTION_PREFIX;
|
||||||
use std::collections::VecDeque;
|
|
||||||
use std::ffi::OsStr;
|
use std::ffi::OsStr;
|
||||||
use std::fs::File;
|
use std::fs::File;
|
||||||
use std::io::{Read, Write};
|
use std::io::{Read, Write};
|
||||||
use std::sync::atomic::Ordering;
|
use std::sync::atomic::Ordering;
|
||||||
|
use std::{collections::VecDeque, thread};
|
||||||
use std::{
|
use std::{
|
||||||
collections::{HashMap, HashSet},
|
collections::{HashMap, HashSet},
|
||||||
env, fs, io,
|
env, fs, io,
|
||||||
path::{Path, PathBuf},
|
path::{Path, PathBuf},
|
||||||
process::{Command, Stdio},
|
process::{Command, Stdio},
|
||||||
|
time::{Duration, Instant},
|
||||||
};
|
};
|
||||||
use strum_macros::AsRefStr;
|
use strum_macros::AsRefStr;
|
||||||
use supervisor::SUPERVISOR_FUNCTION_PREFIX;
|
use supervisor::SUPERVISOR_FUNCTION_PREFIX;
|
||||||
@@ -1456,6 +1457,7 @@ pub fn run_llm_function(
|
|||||||
let mut child = Command::new(&cmd_name)
|
let mut child = Command::new(&cmd_name)
|
||||||
.args(&cmd_args)
|
.args(&cmd_args)
|
||||||
.envs(envs)
|
.envs(envs)
|
||||||
|
.stdin(Stdio::null())
|
||||||
.stdout(Stdio::piped())
|
.stdout(Stdio::piped())
|
||||||
.stderr(Stdio::piped())
|
.stderr(Stdio::piped())
|
||||||
.spawn()
|
.spawn()
|
||||||
@@ -1464,7 +1466,7 @@ pub fn run_llm_function(
|
|||||||
let stdout = child.stdout.take().expect("Failed to capture stdout");
|
let stdout = child.stdout.take().expect("Failed to capture stdout");
|
||||||
let stderr = child.stderr.take().expect("Failed to capture stderr");
|
let stderr = child.stderr.take().expect("Failed to capture stderr");
|
||||||
|
|
||||||
let stdout_thread = std::thread::spawn(move || {
|
let stdout_thread = thread::spawn(move || {
|
||||||
let mut buffer = [0; 1024];
|
let mut buffer = [0; 1024];
|
||||||
let mut reader = stdout;
|
let mut reader = stdout;
|
||||||
let mut out = io::stdout();
|
let mut out = io::stdout();
|
||||||
@@ -1491,7 +1493,7 @@ pub fn run_llm_function(
|
|||||||
buf
|
buf
|
||||||
});
|
});
|
||||||
|
|
||||||
let stderr_thread = std::thread::spawn(move || {
|
let stderr_thread = thread::spawn(move || {
|
||||||
let mut buffer = [0; 1024];
|
let mut buffer = [0; 1024];
|
||||||
let mut reader = stderr;
|
let mut reader = stderr;
|
||||||
let mut err = io::stderr();
|
let mut err = io::stderr();
|
||||||
@@ -1518,9 +1520,39 @@ pub fn run_llm_function(
|
|||||||
buf
|
buf
|
||||||
});
|
});
|
||||||
|
|
||||||
let status = child
|
let timeout_secs = env::var("COYOTE_TOOL_TIMEOUT")
|
||||||
.wait()
|
.ok()
|
||||||
.map_err(|err| anyhow!("Unable to run {command_name}, {err}"))?;
|
.and_then(|v| v.parse::<u64>().ok())
|
||||||
|
.unwrap_or(1800);
|
||||||
|
let deadline = (timeout_secs > 0).then(|| Instant::now() + Duration::from_secs(timeout_secs));
|
||||||
|
let status = loop {
|
||||||
|
match child.try_wait() {
|
||||||
|
Ok(Some(status)) => break status,
|
||||||
|
Ok(None) => {}
|
||||||
|
Err(err) => bail!("Unable to run {command_name}, {err}"),
|
||||||
|
}
|
||||||
|
if let Some(deadline) = deadline
|
||||||
|
&& Instant::now() >= deadline
|
||||||
|
{
|
||||||
|
let _ = child.kill();
|
||||||
|
let _ = child.wait();
|
||||||
|
drop(stdout_thread);
|
||||||
|
drop(stderr_thread);
|
||||||
|
let tool_error_message = format!(
|
||||||
|
"Tool call '{command_name}' timed out after {timeout_secs}s and was killed (set COYOTE_TOOL_TIMEOUT to adjust; 0 = unlimited)"
|
||||||
|
);
|
||||||
|
eprintln!(
|
||||||
|
"{}",
|
||||||
|
muted_warning_text(&format!("⚠️ {tool_error_message} ⚠️"))
|
||||||
|
);
|
||||||
|
let error_json = json!({"tool_call_error": tool_error_message});
|
||||||
|
|
||||||
|
debug!("Tool call error: {error_json:?}");
|
||||||
|
|
||||||
|
return Ok(Some(error_json.to_string()));
|
||||||
|
}
|
||||||
|
thread::sleep(Duration::from_millis(100));
|
||||||
|
};
|
||||||
let stdout_bytes = stdout_thread.join().unwrap_or_default();
|
let stdout_bytes = stdout_thread.join().unwrap_or_default();
|
||||||
let stderr_bytes = stderr_thread.join().unwrap_or_default();
|
let stderr_bytes = stderr_thread.join().unwrap_or_default();
|
||||||
|
|
||||||
|
|||||||
+64
-1
@@ -250,7 +250,30 @@ impl GraphExecutor {
|
|||||||
branch_tasks.push(task);
|
branch_tasks.push(task);
|
||||||
}
|
}
|
||||||
|
|
||||||
let joined = join_all(branch_tasks).await;
|
let joined = match graph_timeout {
|
||||||
|
Some(t) => {
|
||||||
|
let remaining = t.saturating_sub(start.elapsed());
|
||||||
|
let abort_handles: Vec<_> = branch_tasks
|
||||||
|
.iter()
|
||||||
|
.map(|task| task.abort_handle())
|
||||||
|
.collect();
|
||||||
|
match tokio::time::timeout(remaining, join_all(branch_tasks)).await {
|
||||||
|
Ok(joined) => joined,
|
||||||
|
Err(_) => {
|
||||||
|
for handle in abort_handles {
|
||||||
|
handle.abort();
|
||||||
|
}
|
||||||
|
bail!(
|
||||||
|
"Graph '{}' timed out after {}s during super-step with frontier {:?}",
|
||||||
|
graph.name,
|
||||||
|
t.as_secs(),
|
||||||
|
sorted_frontier(&frontier)
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
None => join_all(branch_tasks).await,
|
||||||
|
};
|
||||||
|
|
||||||
let mut branch_writes: Vec<BranchWrites> = Vec::new();
|
let mut branch_writes: Vec<BranchWrites> = Vec::new();
|
||||||
let mut next_frontier: HashSet<String> = HashSet::new();
|
let mut next_frontier: HashSet<String> = HashSet::new();
|
||||||
@@ -793,4 +816,44 @@ nodes:
|
|||||||
"error should list both End nodes: {err}"
|
"error should list both End nodes: {err}"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn graph_timeout_interrupts_in_flight_super_step() {
|
||||||
|
if !cmd_available("bash") {
|
||||||
|
eprintln!("skipping: bash not available");
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
let ws = TestWorkspace::new();
|
||||||
|
ws.write_script("sleeper.sh", "#!/bin/bash\nsleep 10\necho '{}'\n");
|
||||||
|
|
||||||
|
let yaml = r#"
|
||||||
|
name: inflight_timeout_test
|
||||||
|
start: sleeper
|
||||||
|
settings:
|
||||||
|
timeout: 1
|
||||||
|
nodes:
|
||||||
|
sleeper:
|
||||||
|
type: script
|
||||||
|
script: sleeper.sh
|
||||||
|
state_updates: {}
|
||||||
|
next: done
|
||||||
|
done:
|
||||||
|
type: end
|
||||||
|
output: "done"
|
||||||
|
"#;
|
||||||
|
let graph: Graph = serde_yaml::from_str(yaml).unwrap();
|
||||||
|
let mut ctx = make_ctx();
|
||||||
|
let abort = create_abort_signal();
|
||||||
|
let result = GraphExecutor::new(graph, &ws.dir)
|
||||||
|
.execute(&mut ctx, abort)
|
||||||
|
.await;
|
||||||
|
|
||||||
|
assert!(result.is_err(), "expected in-flight timeout to error");
|
||||||
|
let err = format!("{:#}", result.unwrap_err());
|
||||||
|
assert!(
|
||||||
|
err.contains("timed out after 1s during super-step"),
|
||||||
|
"error should report during-super-step timeout: {err}"
|
||||||
|
);
|
||||||
|
assert!(err.contains("sleeper"), "error should name frontier: {err}");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -54,6 +54,7 @@ impl ScriptExecutor {
|
|||||||
let mut cmd = build_command(language, &script_path)?;
|
let mut cmd = build_command(language, &script_path)?;
|
||||||
cmd.stdout(Stdio::piped());
|
cmd.stdout(Stdio::piped());
|
||||||
cmd.stderr(Stdio::piped());
|
cmd.stderr(Stdio::piped());
|
||||||
|
cmd.kill_on_drop(true);
|
||||||
cmd.envs(&self.extra_envs);
|
cmd.envs(&self.extra_envs);
|
||||||
cmd.env("AUTO_CONFIRM", "true");
|
cmd.env("AUTO_CONFIRM", "true");
|
||||||
match &state_repr {
|
match &state_repr {
|
||||||
|
|||||||
Reference in New Issue
Block a user