From 6dfa5b9a5273b406eb5b429c55884b2b59cd0c29 Mon Sep 17 00:00:00 2001 From: Mahdi Hedhli <16087011+MahdiHedhli@users.noreply.github.com> Date: Mon, 3 Aug 2026 20:56:36 -0400 Subject: [PATCH 1/2] fix(acp): report a turn that produced no assistant text as `empty`, not `ok` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A CLI runtime that refuses to start a turn generally ends its ACP session cleanly without emitting any `agent_message_chunk`. Expired or missing credentials, an untrusted working directory, and a missing provider config all behave this way. That is protocol-legal, so the harness sees `StopReason::EndTurn`, records `outcome="ok"`, posts nothing, and logs nothing above DEBUG. The agent appears healthy and idle while every turn is silently lost. Observed with codex-acp: with a working directory outside a trusted project the CLI refuses ("Not inside a trusted directory"), the turn returns in ~4s having emitted nothing, and the harness reports success. With the identical config inside a trusted repository the same prompt takes ~70s and replies. Both were logged as `outcome="ok"`, which is the only reason this took so long to find. Track whether a turn emitted non-whitespace assistant text, label such turns `empty`, and log them at WARN rather than DEBUG — the default deployment level is exactly where this needs to surface. Whitespace-only chunks do not count, so a runtime that emits a stray newline before refusing still reports `empty`. No protocol change, no behaviour change for turns that produce output. Signed-off-by: Mahdi Hedhli <16087011+MahdiHedhli@users.noreply.github.com> --- crates/buzz-acp/src/acp.rs | 60 ++++++++++++++++++++++++++++++++++++++ crates/buzz-acp/src/lib.rs | 31 ++++++++++++++++---- 2 files changed, 86 insertions(+), 5 deletions(-) diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index 700d5e8dcf..a9a528ef82 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -137,6 +137,11 @@ fn build_initialize_params() -> serde_json::Value { /// One `AcpClient` per agent process. Multiple sessions can be created on the /// same client via repeated calls to [`session_new`](AcpClient::session_new). pub struct AcpClient { + /// Whether the in-flight turn has emitted any `agent_message_chunk` text. + /// Reset at each `session/prompt`. A turn that ends cleanly having emitted + /// nothing is indistinguishable from a successful turn unless it is tracked + /// here — see `last_turn_emitted_text`. + turn_emitted_text: bool, /// The agent child process (kept alive to prevent zombie). child: Child, /// Write end of the agent's stdin pipe. @@ -535,6 +540,7 @@ impl AcpClient { .ok_or_else(|| AcpError::Protocol("failed to open agent stdout".into()))?; Ok(Self { + turn_emitted_text: false, child, stdin, reader: FramedRead::new(stdout, LinesCodec::new_with_max_length(MAX_LINE_SIZE)), @@ -761,6 +767,18 @@ impl AcpClient { /// Used for slash-command pass-through: ACP connectors detect commands via /// the **first** block's text starting with `/`, so the harness sends /// `["/cmd args", ""]` instead of one wrapped block. + /// Whether the most recent turn emitted any non-whitespace assistant text. + /// + /// A CLI runtime that refuses to start — lost or expired credentials, an + /// untrusted working directory, a missing provider config — commonly exits + /// its turn cleanly without emitting anything. That is protocol-legal, so + /// the harness sees `StopReason::EndTurn` and reports a successful turn + /// while nothing is posted and no error is logged. Callers use this to tell + /// "worked, said nothing" apart from "never ran". + pub fn last_turn_emitted_text(&self) -> bool { + self.turn_emitted_text + } + pub async fn session_prompt_blocks_with_idle_timeout( &mut self, session_id: &str, @@ -771,6 +789,9 @@ impl AcpClient { let params = build_prompt_params(session_id, prompt_blocks); let hard_deadline = tokio::time::Instant::now() + max_duration; self.current_hard_deadline = Some(hard_deadline); + // Per-turn, not per-session: a runtime that refuses one prompt may serve + // the next, so staleness here would mask a recurring refusal. + self.turn_emitted_text = false; // Mark the usage tracker as in-flight for this turn BEFORE sending the // prompt so that any setup notifications recorded earlier are not @@ -1731,6 +1752,9 @@ impl AcpClient { match update_type { "agent_message_chunk" => { if let Some(text) = update["content"]["text"].as_str() { + if !text.trim().is_empty() { + self.turn_emitted_text = true; + } tracing::info!(target: "acp::stream", "{text}"); } false @@ -3592,6 +3616,42 @@ mod tests { assert_eq!(client.active_run_id(), Some("run-abc-123")); } + /// A runtime that refuses to start its turn (expired credentials, untrusted + /// working directory, missing provider config) ends the session cleanly with + /// no `agent_message_chunk`. That must remain distinguishable from a turn + /// that actually produced output, otherwise the harness reports success and + /// posts nothing. + #[tokio::test] + async fn turn_emitted_text_tracks_assistant_output() { + let mut client = spawn_inert_client().await; + assert!( + !client.last_turn_emitted_text(), + "a fresh client has emitted nothing" + ); + + // Whitespace-only chunks are not output — a runtime that emits a stray + // newline before refusing must still count as empty. + let blank = serde_json::json!({ + "params": {"update": {"sessionUpdate": "agent_message_chunk", + "content": {"text": " \n"}}} + }); + let _ = client.handle_session_update(&blank); + assert!( + !client.last_turn_emitted_text(), + "whitespace-only output must not count as assistant text" + ); + + let real = serde_json::json!({ + "params": {"update": {"sessionUpdate": "agent_message_chunk", + "content": {"text": "PONG"}}} + }); + let _ = client.handle_session_update(&real); + assert!( + client.last_turn_emitted_text(), + "non-whitespace assistant text must be recorded" + ); + } + #[tokio::test] async fn active_run_id_clears_on_null() { let mut client = spawn_inert_client().await; diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 811253e4ac..b56f961590 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -3301,7 +3301,14 @@ fn handle_prompt_result( result.agent.state.invalidate_channel(ch); } + // A turn that ends cleanly having emitted no assistant text is reported + // separately from a normal success. Runtimes that refuse to start — expired + // credentials, an untrusted working directory, a missing provider config — + // end the session protocol-legally with no output, which is otherwise + // indistinguishable from a healthy turn and leaves the agent looking idle. + let emitted_text = result.agent.acp.last_turn_emitted_text(); let outcome_label = match &result.outcome { + PromptOutcome::Ok(_) if !emitted_text => "empty", PromptOutcome::Ok(_) => "ok", PromptOutcome::Error(_) => "error", PromptOutcome::Timeout(TimeoutKind::Idle) => "idle_timeout", @@ -3351,11 +3358,25 @@ fn handle_prompt_result( match result.outcome { // Successful prompt — return agent to pool. PromptOutcome::Ok(_) => { - tracing::debug!( - agent = agent_index, - outcome = outcome_label, - "agent_returned" - ); + if emitted_text { + tracing::debug!( + agent = agent_index, + outcome = outcome_label, + "agent_returned" + ); + } else { + // WARN, not DEBUG: this is the only signal that a runtime ran + // and produced nothing. At DEBUG it is invisible on a default + // deployment, which is exactly when it needs to be seen. + tracing::warn!( + agent = agent_index, + outcome = outcome_label, + configured_model = %harness_configured_model, + pid = harness_pid, + "agent_returned — turn produced no assistant text; check the \ + runtime's credentials, working directory, and provider config" + ); + } pool.return_agent(result.agent); } // Fatal outcomes: the agent subprocess is dead or poisoned — respawn it. From 7cab7e4553c40ea25eee4e3aac6da2bf95f3e699 Mon Sep 17 00:00:00 2001 From: Mahdi Hedhli <16087011+MahdiHedhli@users.noreply.github.com> Date: Tue, 4 Aug 2026 09:10:28 -0400 Subject: [PATCH 2/2] fix(acp): restore the prompt-block method's rustdoc The new getter was inserted between `session_prompt_blocks_with_idle_timeout` and its documentation, so rustdoc attached that description to the getter and left the prompt-block method undocumented. Move the getter and its own doc comment above the block instead. No behaviour change. Signed-off-by: Mahdi Hedhli <16087011+MahdiHedhli@users.noreply.github.com> --- crates/buzz-acp/src/acp.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index a9a528ef82..6b7edc66b5 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -761,12 +761,6 @@ impl AcpClient { .await } - /// Like [`session_prompt_with_idle_timeout`](Self::session_prompt_with_idle_timeout), - /// but sends each entry in `prompt_blocks` as a separate text content block. - /// - /// Used for slash-command pass-through: ACP connectors detect commands via - /// the **first** block's text starting with `/`, so the harness sends - /// `["/cmd args", ""]` instead of one wrapped block. /// Whether the most recent turn emitted any non-whitespace assistant text. /// /// A CLI runtime that refuses to start — lost or expired credentials, an @@ -779,6 +773,12 @@ impl AcpClient { self.turn_emitted_text } + /// Like [`session_prompt_with_idle_timeout`](Self::session_prompt_with_idle_timeout), + /// but sends each entry in `prompt_blocks` as a separate text content block. + /// + /// Used for slash-command pass-through: ACP connectors detect commands via + /// the **first** block's text starting with `/`, so the harness sends + /// `["/cmd args", ""]` instead of one wrapped block. pub async fn session_prompt_blocks_with_idle_timeout( &mut self, session_id: &str,