diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index b3986fdb9b8..ea32489bd3f 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -2987,6 +2987,7 @@ async fn tokio_main() -> Result<()> { &respawn_tx, &mut respawn_tasks, observer.clone(), + Some(&ctx.rest_client), ) == LoopAction::Exit { break; @@ -3011,6 +3012,7 @@ async fn tokio_main() -> Result<()> { &respawn_tx, &mut respawn_tasks, observer.clone(), + Some(&ctx.rest_client), ); if pool.live_count() == 0 && !any_respawn_in_flight(&crash_history) { tracing::error!("all agents dead — exiting"); @@ -3654,6 +3656,15 @@ fn is_auth_error(error: &acp::AcpError) -> bool { message.contains("Re-authenticate") || message.contains("API Error: 401") } +/// The retry-exhausted notice. Single source for the sentence so the +/// retries-exhausted and panic dead-letter paths cannot drift apart. +fn retry_exhausted_notice(reason: &str) -> String { + format!( + "⚠️ I couldn't process the last request after multiple retries ({reason}). \ + Please re-send if it's still needed." + ) +} + /// Spawn a task that posts a user-visible failure notice to the relay. /// /// Shared by the hard-cap immediate dead-letter path and the retries-exhausted @@ -3820,9 +3831,7 @@ fn handle_prompt_result( PromptOutcome::Error(e) => format!("{e}"), _ => "repeated failures".to_string(), }; - let content = format!( - "⚠️ I couldn't process the last request after multiple retries ({reason}). Please re-send if it's still needed." - ); + let content = retry_exhausted_notice(&reason); spawn_failure_notice(rest_client, &dead, content); } } else { @@ -4079,6 +4088,7 @@ fn recover_panicked_agent( respawn_tx: &mpsc::Sender, respawn_tasks: &mut tokio::task::JoinSet<()>, observer: Option, + rest_client: Option<&relay::RestClient>, ) { let task_id = join_error.id(); let Some(meta) = pool.task_map_mut().remove(&task_id) else { @@ -4091,10 +4101,20 @@ fn recover_panicked_agent( if let Some(batch) = meta.recoverable_batch { if let Some(ch) = meta.channel_id { if !removed_channels.contains(&ch) { - // Dead-letter on exhaustion is logged inside requeue(); a - // panic path has no outcome to report, so no notice here. - let _ = queue.requeue(batch); - tracing::warn!("requeued batch for panicked agent {i}"); + if let Some(dead) = queue.requeue(batch) { + // Retry budget exhausted: the events are discarded here, + // so without a notice the channel just goes quiet. Report + // it the same way handle_prompt_result does. + tracing::warn!( + channel_id = %ch, + events = dead.events.len(), + "dead-lettered batch for panicked agent {i}" + ); + let content = retry_exhausted_notice("the agent crashed"); + spawn_failure_notice(rest_client, &dead, content); + } else { + tracing::warn!("requeued batch for panicked agent {i}"); + } } else { tracing::debug!( channel_id = %ch, @@ -4177,6 +4197,7 @@ fn drain_ready_join_results( respawn_tx: &mpsc::Sender, respawn_tasks: &mut tokio::task::JoinSet<()>, observer: Option, + rest_client: Option<&relay::RestClient>, ) -> LoopAction { while let Some(Some(join_result)) = pool.join_set.join_next().now_or_never() { if let Err(join_error) = join_result { @@ -4193,6 +4214,7 @@ fn drain_ready_join_results( respawn_tx, respawn_tasks, observer.clone(), + rest_client, ); if pool.live_count() == 0 && !any_respawn_in_flight(crash_history) { return LoopAction::Exit; @@ -6727,9 +6749,17 @@ mod error_outcome_emission_tests { AgentPool, OwnedAgent, PromptOutcome, PromptResult, PromptSource, TimeoutKind, }; use crate::queue::{BatchEvent, FlushBatch}; - use nostr::{EventBuilder, Keys, Kind}; + use nostr::{Event, EventBuilder, Keys, Kind, Tag}; use std::collections::HashSet; + #[test] + fn retry_exhausted_notice_matches_existing_template() { + assert_eq!( + retry_exhausted_notice("the agent crashed"), + "⚠️ I couldn't process the last request after multiple retries (the agent crashed). Please re-send if it's still needed." + ); + } + fn test_config() -> Config { Config { keys: nostr::Keys::generate(), @@ -7204,6 +7234,7 @@ mod error_outcome_emission_tests { &respawn_tx, &mut respawn_tasks, Some(observer.clone()), + None, ); let panic = observer @@ -7218,6 +7249,302 @@ mod error_outcome_emission_tests { assert_eq!(panic.turn_id.as_deref(), Some("panic-turn-id")); } + async fn read_submitted_event(listener: &tokio::net::TcpListener) -> (String, Event) { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + let (mut socket, _) = listener.accept().await.expect("accept request"); + let mut request = Vec::new(); + let mut buffer = [0u8; 4096]; + let header_end = loop { + let read = socket.read(&mut buffer).await.expect("read request"); + assert_ne!(read, 0, "request ended before its headers completed"); + request.extend_from_slice(&buffer[..read]); + if let Some(offset) = request.windows(4).position(|bytes| bytes == b"\r\n\r\n") { + break offset + 4; + } + }; + + let headers = std::str::from_utf8(&request[..header_end]).expect("UTF-8 request headers"); + let request_line = headers.lines().next().unwrap_or_default().to_string(); + let content_length = headers + .lines() + .filter_map(|line| line.split_once(':')) + .find(|(name, _)| name.eq_ignore_ascii_case("content-length")) + .and_then(|(_, value)| value.trim().parse::().ok()) + .expect("Content-Length header"); + + while request.len() < header_end + content_length { + let read = socket.read(&mut buffer).await.expect("read request body"); + assert_ne!(read, 0, "request ended before its body completed"); + request.extend_from_slice(&buffer[..read]); + } + let event = serde_json::from_slice(&request[header_end..header_end + content_length]) + .expect("POST /events body must be a Nostr event"); + + socket + .write_all( + b"HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: 2\r\nConnection: close\r\n\r\n{}", + ) + .await + .expect("complete relay response"); + (request_line, event) + } + + async fn test_rest_client() -> (tokio::net::TcpListener, relay::RestClient) { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let rest = relay::RestClient { + http: reqwest::Client::new(), + base_url: format!("http://{}", listener.local_addr().unwrap()), + keys: Keys::generate(), + auth_tag_json: None, + }; + (listener, rest) + } + + async fn assert_no_submitted_event(listener: &tokio::net::TcpListener, message: &str) { + assert!( + tokio::time::timeout(Duration::from_millis(250), listener.accept()) + .await + .is_err(), + "{message}" + ); + } + + fn threaded_batch(channel_id: Uuid) -> (FlushBatch, nostr::EventId, nostr::EventId) { + let keys = Keys::generate(); + let root = EventBuilder::new(Kind::Custom(9), "root") + .sign_with_keys(&keys) + .unwrap(); + let parent = EventBuilder::new(Kind::Custom(9), "parent") + .sign_with_keys(&keys) + .unwrap(); + let root_hex = root.id.to_hex(); + let parent_hex = parent.id.to_hex(); + let event = EventBuilder::new(Kind::Custom(9), "trigger") + .tags([ + Tag::parse(["h", &channel_id.to_string()]).unwrap(), + Tag::parse(["e", &root_hex, "", "root"]).unwrap(), + Tag::parse(["e", &parent_hex, "", "reply"]).unwrap(), + ]) + .sign_with_keys(&keys) + .unwrap(); + ( + FlushBatch { + channel_id, + events: vec![BatchEvent { + event, + prompt_tag: "test".into(), + received_at: std::time::Instant::now(), + }], + cancelled_events: Vec::new(), + cancel_reason: None, + }, + root.id, + parent.id, + ) + } + + async fn drain_panicked_batch( + queue: &mut EventQueue, + channel_id: Uuid, + batch: Option, + removed_channels: &HashSet, + rest: &relay::RestClient, + ) { + let mut pool = AgentPool::from_slots(vec![]); + let handle = pool + .join_set + .spawn(async { panic!("simulated agent panic") }); + let task_id = handle.id(); + pool.task_map_mut().insert( + task_id, + crate::pool::TaskMeta { + agent_index: 0, + channel_id: Some(channel_id), + turn_id: "dead-letter-turn".to_string(), + recoverable_batch: batch, + control_tx: None, + steer_tx: None, + successful_steer_deliveries: HashSet::new(), + }, + ); + + let config = test_config(); + let mut heartbeat_in_flight = false; + let mut typing_channels = HashMap::new(); + let mut crash_history = vec![SlotCircuit { + crash_times: Vec::new(), + open_until: None, + respawn_in_flight: false, + }]; + let (respawn_tx, _respawn_rx) = mpsc::channel(8); + let mut respawn_tasks = tokio::task::JoinSet::new(); + let observer = ObserverHandle::in_process(); + + tokio::time::timeout(Duration::from_secs(5), async { + while pool.task_map().contains_key(&task_id) { + drain_ready_join_results( + &mut pool, + queue, + &config, + &mut heartbeat_in_flight, + removed_channels, + &mut typing_channels, + &mut crash_history, + &respawn_tx, + &mut respawn_tasks, + Some(observer.clone()), + Some(rest), + ); + tokio::task::yield_now().await; + } + }) + .await + .expect("production join-drain path must consume the panic"); + + assert!( + observer + .snapshot() + .iter() + .any(|event| event.kind == "agent_panic"), + "the production drain must observe a genuine task panic" + ); + } + + /// A panicking agent whose batch has already burned its retry budget must + /// tell the channel, the same way `handle_prompt_result` does. Before this + /// was wired up the panic path discarded the dead-lettered batch silently + /// and the channel simply went quiet. + #[tokio::test] + async fn panic_dead_letter_posts_a_failure_notice() { + let (listener, rest) = test_rest_client().await; + + let channel_id = Uuid::new_v4(); + let mut queue = EventQueue::new(config::DedupMode::Queue); + // Burn the retry budget so the next requeue dead-letters. + for _ in 0..queue::MAX_RETRIES { + assert!(queue.requeue(empty_batch(channel_id)).is_none()); + } + let (batch, root_id, parent_id) = threaded_batch(channel_id); + + drain_panicked_batch(&mut queue, channel_id, Some(batch), &HashSet::new(), &rest).await; + + let (request_line, event) = + tokio::time::timeout(Duration::from_secs(5), read_submitted_event(&listener)) + .await + .expect("dead-lettering a panicked batch must post a failure notice"); + assert_eq!(request_line, "POST /events HTTP/1.1"); + event + .verify() + .expect("failure notice signature must verify"); + assert_eq!(event.pubkey, rest.keys.public_key()); + assert_eq!(event.kind, Kind::Custom(9)); + assert_eq!( + event.content, + "⚠️ I couldn't process the last request after multiple retries (the agent crashed). \ + Please re-send if it's still needed." + ); + + let h_tags: Vec<_> = event + .tags + .iter() + .filter(|tag| tag.as_slice().first().map(String::as_str) == Some("h")) + .map(|tag| tag.as_slice().to_vec()) + .collect(); + assert_eq!( + h_tags, + vec![vec!["h".to_string(), channel_id.to_string()]], + "notice must target exactly the dead-lettered channel" + ); + + let e_tags: Vec<_> = event + .tags + .iter() + .filter(|tag| tag.as_slice().first().map(String::as_str) == Some("e")) + .map(|tag| tag.as_slice().to_vec()) + .collect(); + assert_eq!( + e_tags, + vec![ + vec![ + "e".to_string(), + root_id.to_hex(), + String::new(), + "root".to_string(), + ], + vec![ + "e".to_string(), + parent_id.to_hex(), + String::new(), + "reply".to_string(), + ], + ], + "notice must remain in the triggering thread" + ); + + assert_no_submitted_event( + &listener, + "panic recovery must submit exactly one failure notice", + ) + .await; + } + + #[tokio::test] + async fn panic_retry_before_dead_letter_posts_no_failure_notice() { + let (listener, rest) = test_rest_client().await; + let channel_id = Uuid::new_v4(); + let mut queue = EventQueue::new(config::DedupMode::Queue); + + drain_panicked_batch( + &mut queue, + channel_id, + Some(empty_batch(channel_id)), + &HashSet::new(), + &rest, + ) + .await; + + assert_no_submitted_event( + &listener, + "a retryable panic must not post a terminal failure notice", + ) + .await; + } + + #[tokio::test] + async fn panic_for_removed_channel_posts_no_failure_notice() { + let (listener, rest) = test_rest_client().await; + let channel_id = Uuid::new_v4(); + let mut queue = EventQueue::new(config::DedupMode::Queue); + for _ in 0..queue::MAX_RETRIES { + assert!(queue.requeue(empty_batch(channel_id)).is_none()); + } + + drain_panicked_batch( + &mut queue, + channel_id, + Some(empty_batch(channel_id)), + &HashSet::from([channel_id]), + &rest, + ) + .await; + + assert_no_submitted_event( + &listener, + "a removed channel must never receive a failure notice", + ) + .await; + } + + fn empty_batch(channel_id: Uuid) -> queue::FlushBatch { + queue::FlushBatch { + channel_id, + events: Vec::new(), + cancelled_events: Vec::new(), + cancel_reason: None, + } + } + #[tokio::test] async fn idle_timeout_emits_exactly_one_feed_event() { assert_eq!(