Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions crates/buzz-acp/src/acp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)),
Expand Down Expand Up @@ -755,6 +761,18 @@ impl AcpClient {
.await
}

/// Whether the most recent turn emitted any non-whitespace assistant text.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restore the prompt-block method's documentation

Move the new getter and its documentation outside the existing rustdoc block: as written, Rust associates the preceding “Like session_prompt_with_idle_timeout … separate text content block” description with last_turn_emitted_text, while session_prompt_blocks_with_idle_timeout loses its documentation entirely. This produces misleading generated API docs and leaves a public method undocumented.

AGENTS.md reference: AGENTS.md:L113-L116

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7cab7e4 — good catch, and it was a real docs regression rather than a style nit.

The getter was inserted between session_prompt_blocks_with_idle_timeout and its rustdoc, so the "Like session_prompt_with_idle_timeout…" description bound to the getter and the prompt-block method was left undocumented. Moved the getter and its own doc comment above the block, so each description now attaches to the item it describes.

No behaviour change; cargo test -p buzz-acp --lib still 669 passing and cargo fmt --check clean.

///
/// 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
}

/// 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.
///
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
31 changes: 26 additions & 5 deletions crates/buzz-acp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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.
Expand Down