Skip to content

Commit 4e86bbc

Browse files
loganjLarry
andcommitted
fix(acp): cancel blocked task configuration and strengthen regressions
Keep launch configuration off the async worker under signals and a ten-second bound. Use the HTTP stack for memory fixtures and assert oversized file/stdin terminal errors. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson <[email protected]>
1 parent 0de8d11 commit 4e86bbc

5 files changed

Lines changed: 167 additions & 61 deletions

File tree

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/buzz-acp/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,5 +83,6 @@ evalexpr = { workspace = true }
8383
nix = { version = "0.31", default-features = false, features = ["signal", "user"] }
8484

8585
[dev-dependencies]
86+
axum = { workspace = true }
8687
tokio = { workspace = true, features = ["test-util"] }
8788
httparse = "1"

‎crates/buzz-acp/TASKS.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,11 @@ verify the document's author. File/stdin input does not need a signature.
8080
- Do not automatically publish final text. Use task reporting instructions and
8181
tools. Do not retry the task after failure, timeout, or uncertain execution.
8282

83+
Launch configuration preparation (including prompt-file reads) has a separate
84+
ten-second bound. A blocked configuration read reports `configuration_timeout`
85+
with exit 2; SIGINT/SIGTERM during that read reports `cancelled` with exit
86+
130/143 and a null `taskId`. No adapter is started during this phase.
87+
8388
The execution deadline covers adapter startup, memory loading, session setup,
8489
and the task turn. SIGINT/SIGTERM cancel the invocation. If a prompt is active,
8590
allow up to five seconds for cooperative cancellation, then kill and reap the

‎crates/buzz-acp/src/run_task.rs‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -142,12 +142,8 @@ pub(crate) async fn run() -> i32 {
142142
args.launch.heartbeat_prompt = None;
143143
args.launch.heartbeat_prompt_file = None;
144144
args.launch.turn_liveness_secs = 0;
145-
let config = match Config::from_args(args.launch) {
146-
Ok(config) if config.max_turn_duration_secs > 0 => config,
147-
_ => return emit(Terminal::new("invalid", Some("invalid_configuration")), 2),
148-
};
149145
let (signal_tx, mut signal_rx) = tokio::sync::oneshot::channel();
150-
// Register signal handlers before reading input or spawning the adapter.
146+
// Register before configuration: launch prompt files can also block on I/O.
151147
#[cfg(unix)]
152148
let signals = (
153149
tokio::signal::unix::signal(tokio::signal::unix::SignalKind::interrupt()),
@@ -166,6 +162,27 @@ pub(crate) async fn run() -> i32 {
166162
let _ = tokio::signal::ctrl_c().await;
167163
let _ = signal_tx.send(130);
168164
});
165+
// Config owns synchronous prompt-file reads. Keep them off the async worker
166+
// so signals and the preparation bound remain live, including for FIFOs.
167+
// This worker only builds configuration; it cannot spawn an agent or create
168+
// runtime resources. On cancellation/timeout the CLI's bounded runtime
169+
// shutdown and process exit also retire a still-blocked read.
170+
let configuration = tokio::task::spawn_blocking(move || Config::from_args(args.launch));
171+
let config = tokio::select! {
172+
biased;
173+
code = &mut signal_rx => return emit(Terminal::new("cancelled", None), code.unwrap_or(1)),
174+
result = tokio::time::timeout(INPUT_TIMEOUT, configuration) => match result {
175+
Ok(Ok(Ok(config))) if config.max_turn_duration_secs > 0 => config,
176+
Err(_) => {
177+
signal_task.abort();
178+
return emit(Terminal::new("invalid", Some("configuration_timeout")), 2);
179+
}
180+
_ => {
181+
signal_task.abort();
182+
return emit(Terminal::new("invalid", Some("invalid_configuration")), 2);
183+
}
184+
},
185+
};
169186
let task = tokio::select! {
170187
biased;
171188
code = &mut signal_rx => return emit(Terminal::new("cancelled", None), code.unwrap_or(1)),

‎crates/buzz-acp/tests/run_task.rs‎

Lines changed: 138 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -248,11 +248,7 @@ fn invalid_input_never_spawns() {
248248
command.args(["--task", source]);
249249
terminal(&wait(command.spawn().unwrap()), 2, "invalid");
250250
}
251-
for bytes in [
252-
b"{} {}".to_vec(),
253-
b"{".to_vec(),
254-
vec![b' '; 1024 * 1024 + 1],
255-
] {
251+
for bytes in [b"{} {}".to_vec(), b"{".to_vec()] {
256252
let mut command = f.command();
257253
command.args(["--task", "-"]);
258254
let mut child = command.spawn().unwrap();
@@ -262,6 +258,90 @@ fn invalid_input_never_spawns() {
262258
assert!(f.wire().is_empty());
263259
}
264260

261+
#[test]
262+
fn oversized_file_and_stdin_report_task_too_large_before_spawn() {
263+
let f = Fixture::new();
264+
let bytes = vec![b' '; 1024 * 1024 + 1];
265+
fs::write(f.dir.join("oversized.json"), &bytes).unwrap();
266+
for stdin in [false, true] {
267+
let mut command = f.command();
268+
command.arg("--task").arg(if stdin {
269+
PathBuf::from("-")
270+
} else {
271+
f.dir.join("oversized.json")
272+
});
273+
let mut child = command.spawn().unwrap();
274+
if stdin {
275+
child.stdin.take().unwrap().write_all(&bytes).unwrap();
276+
}
277+
let result = terminal(&wait(child), 2, "invalid");
278+
assert_eq!(result["error"], "task_too_large");
279+
assert_eq!(result["taskId"], Value::Null);
280+
assert!(f.wire().is_empty());
281+
}
282+
}
283+
284+
#[test]
285+
fn blocked_configuration_reads_are_cancellable_and_bounded() {
286+
use nix::{
287+
sys::signal::{kill, Signal},
288+
unistd::Pid,
289+
};
290+
use std::os::unix::fs::OpenOptionsExt;
291+
292+
for flag in ["--system-prompt-file", "--base-prompt-file"] {
293+
for signal in [Some(Signal::SIGINT), Some(Signal::SIGTERM), None] {
294+
let f = Fixture::new();
295+
let fifo = f.dir.join("prompt.fifo");
296+
assert!(Command::new("mkfifo")
297+
.arg(&fifo)
298+
.status()
299+
.unwrap()
300+
.success());
301+
let started = Instant::now();
302+
let child = f
303+
.command()
304+
.arg(flag)
305+
.arg(&fifo)
306+
.args(["--no-memory", "--task", "-"])
307+
.spawn()
308+
.unwrap();
309+
// A successful nonblocking writer open proves Config has opened
310+
// this FIFO for reading. Hold it without data, so read_to_string
311+
// cannot finish. No sleep-based assumption about startup readiness.
312+
let until = Instant::now() + Duration::from_secs(5);
313+
let writer = loop {
314+
match fs::OpenOptions::new()
315+
.write(true)
316+
.custom_flags(nix::libc::O_NONBLOCK)
317+
.open(&fifo)
318+
{
319+
Ok(writer) => break writer,
320+
Err(error) if error.raw_os_error() == Some(nix::libc::ENXIO) => {
321+
assert!(Instant::now() < until, "configuration never opened FIFO");
322+
std::thread::sleep(Duration::from_millis(10));
323+
}
324+
Err(error) => panic!("FIFO writer: {error}"),
325+
}
326+
};
327+
if let Some(signal) = signal {
328+
let signalled = Instant::now();
329+
kill(Pid::from_raw(child.id() as i32), signal).unwrap();
330+
let code = if signal == Signal::SIGINT { 130 } else { 143 };
331+
let result = terminal(&wait(child), code, "cancelled");
332+
assert_eq!(result["taskId"], Value::Null);
333+
assert!(signalled.elapsed() < Duration::from_secs(3));
334+
} else {
335+
let result = terminal(&wait(child), 2, "invalid");
336+
assert_eq!(result["error"], "configuration_timeout");
337+
assert!(started.elapsed() < Duration::from_secs(15));
338+
}
339+
drop(writer);
340+
assert!(f.wire().is_empty());
341+
}
342+
}
343+
}
344+
265345
#[test]
266346
fn stop_reasons_failures_and_permission_exchange_are_explicit() {
267347
for (mode, reason, code, status) in [
@@ -384,61 +464,51 @@ fn signals_cancel_startup_turn_and_pending_stdin_without_hanging() {
384464
}
385465
}
386466

387-
#[test]
388-
fn memory_is_loaded_before_session_and_opt_out_makes_no_request() {
389-
use std::{
390-
io::Read,
391-
net::TcpListener,
392-
sync::{
393-
atomic::{AtomicUsize, Ordering},
394-
Arc,
395-
},
467+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
468+
async fn memory_is_loaded_before_session_and_opt_out_makes_no_request() {
469+
use axum::{body::Bytes, http::HeaderMap, routing::post, Router};
470+
use std::sync::{
471+
atomic::{AtomicUsize, Ordering},
472+
Arc,
396473
};
474+
397475
for (body, expected) in [("[]", true), ("not-json", false)] {
398-
let listener = TcpListener::bind("127.0.0.1:0").unwrap();
399-
listener.set_nonblocking(true).unwrap();
476+
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
400477
let url = format!("ws://{}", listener.local_addr().unwrap());
401478
let calls = Arc::new(AtomicUsize::new(0));
402479
let observed = calls.clone();
403-
let done = Arc::new(std::sync::atomic::AtomicBool::new(false));
404-
let stop = done.clone();
405-
let server = std::thread::spawn(move || {
406-
while !stop.load(Ordering::SeqCst) {
407-
if let Ok((mut stream, _)) = listener.accept() {
408-
stream
409-
.set_read_timeout(Some(Duration::from_secs(2)))
410-
.unwrap();
411-
let mut request = Vec::new();
412-
let mut byte = [0];
413-
while !request.ends_with(b"\r\n\r\n") {
414-
stream.read_exact(&mut byte).unwrap();
415-
request.push(byte[0]);
480+
let unexpected = Arc::new(AtomicUsize::new(0));
481+
let rejected = unexpected.clone();
482+
// Use the real HTTP parser rather than byte-wise timed socket reads.
483+
// Idle connections and partial packets cannot block another request.
484+
let app = Router::new()
485+
.route(
486+
"/query",
487+
post(move |headers: HeaderMap, bytes: Bytes| {
488+
let observed = observed.clone();
489+
async move {
490+
assert!(headers["authorization"]
491+
.to_str()
492+
.unwrap()
493+
.starts_with("Nostr "));
494+
assert!(String::from_utf8(bytes.to_vec()).unwrap().contains("30174"));
495+
observed.fetch_add(1, Ordering::SeqCst);
496+
body
416497
}
417-
let headers = String::from_utf8(request).unwrap();
418-
assert!(
419-
headers.starts_with("POST /query "),
420-
"unexpected service access: {headers}"
421-
);
422-
assert!(headers.to_lowercase().contains("authorization: nostr "));
423-
let len: usize = headers
424-
.lines()
425-
.find_map(|s| {
426-
s.to_lowercase()
427-
.strip_prefix("content-length: ")
428-
.map(str::to_owned)
429-
})
430-
.unwrap()
431-
.parse()
432-
.unwrap();
433-
let mut bytes = vec![0; len];
434-
stream.read_exact(&mut bytes).unwrap();
435-
assert!(String::from_utf8(bytes).unwrap().contains("30174"));
436-
observed.fetch_add(1, Ordering::SeqCst);
437-
write!(stream, "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{}", body.len(), body).unwrap();
438-
} else {
439-
std::thread::sleep(Duration::from_millis(5));
440-
}
441-
}
498+
}),
499+
)
500+
.fallback(move || {
501+
rejected.fetch_add(1, Ordering::SeqCst);
502+
async { axum::http::StatusCode::NOT_FOUND }
503+
});
504+
let (stop, stopped) = tokio::sync::oneshot::channel();
505+
let mut server = tokio::spawn(async move {
506+
axum::serve(listener, app)
507+
.with_graceful_shutdown(async {
508+
let _ = stopped.await;
509+
})
510+
.await
511+
.unwrap();
442512
});
443513
for disabled in [false, true] {
444514
let f = Fixture::new();
@@ -491,7 +561,19 @@ fn memory_is_loaded_before_session_and_opt_out_makes_no_request() {
491561
expected && !disabled
492562
);
493563
}
494-
done.store(true, Ordering::SeqCst);
495-
server.join().unwrap();
564+
assert_eq!(
565+
unexpected.load(Ordering::SeqCst),
566+
0,
567+
"unexpected service access"
568+
);
569+
stop.send(()).unwrap();
570+
match tokio::time::timeout(Duration::from_secs(2), &mut server).await {
571+
Ok(result) => result.unwrap(),
572+
Err(_) => {
573+
server.abort();
574+
let _ = server.await;
575+
panic!("memory server did not drain");
576+
}
577+
}
496578
}
497579
}

0 commit comments

Comments
 (0)