diff --git a/src/openhuman/agent/harness/session/turn/core.rs b/src/openhuman/agent/harness/session/turn/core.rs index 261f54391a..7807a0c99e 100644 --- a/src/openhuman/agent/harness/session/turn/core.rs +++ b/src/openhuman/agent/harness/session/turn/core.rs @@ -957,10 +957,11 @@ impl Agent { // cycle. Fold the extra call's usage into the turn accounting. let base = self.tool_dispatcher.to_provider_messages(&self.history); let (summary, summary_usage) = self - .summarize_iteration_checkpoint( + .summarize_turn_wrapup( &base, effective_model, outcome.model_calls as u32 + 1, + super::super::turn_checkpoint::MAX_ITER_CHECKPOINT_INSTRUCTION, ) .await; if let Some(u) = summary_usage { @@ -991,6 +992,61 @@ impl Agent { iteration: outcome.model_calls, }, )); + } else if outcome.text.trim().is_empty() { + // #4093: the loop ran tool calls (tool_calls > 0, so the branch + // above did not fire) and then yielded a terminating response with + // no final text — the turn did work but would otherwise end + // silently, leaving the user with nothing. Enforce the + // "must produce a final response" terminal step: re-prompt the + // model (tools disabled) for a closing summary of what it did, + // falling back to a deterministic summary of the tool calls so the + // synthesized message is never itself empty. Fold the extra call's + // usage into the turn accounting, exactly like the cap path above. + let base = self.tool_dispatcher.to_provider_messages(&self.history); + let (summary, summary_usage) = self + .summarize_turn_wrapup( + &base, + effective_model, + outcome.model_calls as u32 + 1, + super::super::turn_checkpoint::FINAL_ANSWER_INSTRUCTION, + ) + .await; + if let Some(u) = summary_usage { + input_tokens += u.input_tokens; + output_tokens += u.output_tokens; + cached_input_tokens += u.cached_input_tokens; + charged_amount_usd += u.charged_amount_usd; + } + let final_answer = if summary.trim().is_empty() { + super::super::turn_checkpoint::build_deterministic_final_summary( + &tool_records_from_conversation(&outcome.conversation, &outcome.tool_outcomes), + ) + } else { + summary + }; + log::info!( + "[agent_loop] turn produced no final text after {} tool call(s); synthesized a closing summary ({} chars) — #4093", + outcome.tool_calls, + final_answer.chars().count() + ); + // The empty terminal assistant response was already folded into + // `self.history` via `outcome.conversation` above (an empty + // `Chat(assistant(""))` — see `messages_to_conversation`). Drop that + // blank turn before appending the synthesized answer so the + // transcript and the next prompt don't carry a dangling empty + // assistant message immediately before the real reply (Codex review). + if matches!( + self.history.last(), + Some(ConversationMessage::Chat(msg)) + if msg.role == "assistant" && msg.content.trim().is_empty() + ) { + self.history.pop(); + } + self.history + .push(ConversationMessage::Chat(ChatMessage::assistant( + final_answer.clone(), + ))); + final_answer } else { outcome.text.clone() }; diff --git a/src/openhuman/agent/harness/session/turn/session_io.rs b/src/openhuman/agent/harness/session/turn/session_io.rs index 0e6953f8f6..4a6f1e9f68 100644 --- a/src/openhuman/agent/harness/session/turn/session_io.rs +++ b/src/openhuman/agent/harness/session/turn/session_io.rs @@ -1,7 +1,6 @@ //! Session persistence: transcript loading, checkpointing, and background tasks. use super::super::transcript; -use super::super::turn_checkpoint::MAX_ITER_CHECKPOINT_INSTRUCTION; use super::super::types::Agent; use crate::openhuman::agent::harness; use crate::openhuman::agent::progress::AgentProgress; @@ -63,27 +62,30 @@ impl Agent { } } - /// Ask the provider for a resumable checkpoint summary when a turn - /// hits the tool-call iteration cap, with native tools **disabled** so - /// the model returns prose rather than another tool call. Streams text - /// deltas to the progress sink (when attached) so the checkpoint + /// Ask the provider for a short wrap-up message with native tools + /// **disabled** so the model returns prose rather than another tool call. + /// Streams text deltas to the progress sink (when attached) so the summary /// appears in the UI like any other reply. /// + /// `instruction` is the synthetic user turn that steers the wrap-up — the + /// tool-call-cap checkpoint (`MAX_ITER_CHECKPOINT_INSTRUCTION`) or the + /// no-final-answer close (`FINAL_ANSWER_INSTRUCTION`, issue #4093). + /// /// Returns the summary text (empty when the provider call fails or - /// yields nothing — the caller then falls back to - /// [`build_deterministic_checkpoint`] so the thread is never left on an - /// unterminated tool cycle, bug-report-2026-05-26 A1) **paired with the - /// provider usage** for this extra call, so the caller can fold it into - /// the turn's cumulative token/cost accounting instead of silently - /// dropping it. - pub(super) async fn summarize_iteration_checkpoint( + /// yields nothing — the caller then falls back to a deterministic builder + /// so the turn is never left without a well-formed assistant message, + /// bug-report-2026-05-26 A1 / issue #4093) **paired with the provider + /// usage** for this extra call, so the caller can fold it into the turn's + /// cumulative token/cost accounting instead of silently dropping it. + pub(super) async fn summarize_turn_wrapup( &self, base_messages: &[ChatMessage], effective_model: &str, iteration_for_stream: u32, + instruction: &str, ) -> (String, Option) { let mut messages = base_messages.to_vec(); - messages.push(ChatMessage::user(MAX_ITER_CHECKPOINT_INSTRUCTION)); + messages.push(ChatMessage::user(instruction)); // Mirror the main loop's streaming sink so the checkpoint renders // incrementally. Only text deltas are relevant here (tools are diff --git a/src/openhuman/agent/harness/session/turn_checkpoint.rs b/src/openhuman/agent/harness/session/turn_checkpoint.rs index 110fa71939..db7a770ecc 100644 --- a/src/openhuman/agent/harness/session/turn_checkpoint.rs +++ b/src/openhuman/agent/harness/session/turn_checkpoint.rs @@ -74,3 +74,32 @@ pub(super) fn build_deterministic_checkpoint( ); out } + +/// Instruction appended (as a synthetic user turn) when a turn finished its +/// tool work but the model produced **no final answer** — it yielded a +/// terminating response with empty text after running tools (issue #4093). +/// Native tools are disabled for this call so the model wraps up in prose +/// instead of requesting more tools. +pub(super) const FINAL_ANSWER_INSTRUCTION: &str = "\ +You have finished using tools for this turn but have not yet written a reply to the user. \ +Do not call any more tools. Write a short, self-contained final message that summarises what you did and \ +what you found or accomplished, grounded in the tool results above. If nothing conclusive resulted, say so plainly."; + +/// Build a deterministic final answer from this turn's tool-call records. +/// Used as the guaranteed non-empty fallback when a turn ran tools but the +/// model produced no closing message and the re-prompt for one also came +/// back empty — so a turn that did work can never end silently (issue #4093). +/// Distinct from [`build_deterministic_checkpoint`]: the turn did NOT hit the +/// iteration cap, so this reads as a completed summary, not a paused one. +pub(super) fn build_deterministic_final_summary(records: &[ToolCallRecord]) -> String { + if records.is_empty() { + return "I finished this turn but produced no result to report.".to_string(); + } + let mut out = String::from("Here's a summary of what I did this turn:\n\n"); + for r in records { + let status = if r.success { "ok" } else { "failed" }; + out.push_str(&format!("- `{}` — {}\n", r.name, status)); + } + out.push_str("\nLet me know if you'd like me to go further."); + out +} diff --git a/src/openhuman/agent/harness/session/turn_tests.rs b/src/openhuman/agent/harness/session/turn_tests.rs index 693efe34d7..0c6b06f82f 100644 --- a/src/openhuman/agent/harness/session/turn_tests.rs +++ b/src/openhuman/agent/harness/session/turn_tests.rs @@ -1038,6 +1038,165 @@ async fn turn_checkpoint_falls_back_to_deterministic_summary_when_model_summary_ ); } +#[tokio::test] +async fn turn_synthesizes_final_answer_when_tool_turn_yields_no_text() { + // #4093: the model runs a tool and then yields a terminating response with + // NO text and NO further tool calls — the turn did work but would end + // silently. Because the cap was not hit, this is not a checkpoint case; the + // harness must enforce the "must produce a final response" terminal step by + // re-prompting the model (tools disabled) for a closing summary and + // returning that instead of a blank reply. + let provider: Arc = Arc::new(SequenceProvider { + responses: AsyncMutex::new(vec![ + // Tool iteration (well under the cap). + Ok(ChatResponse { + text: Some("{\"name\":\"echo\",\"arguments\":{}}".into()), + tool_calls: vec![], + usage: None, + reasoning_content: None, + }), + // Terminal response with no text and no tool calls — the silent end. + Ok(ChatResponse { + text: Some(String::new()), + tool_calls: vec![], + usage: None, + reasoning_content: None, + }), + // The harness's forced final-answer re-prompt (tools disabled). + Ok(ChatResponse { + text: Some("All done — I ran echo and it succeeded.".into()), + tool_calls: vec![], + usage: None, + reasoning_content: None, + }), + ]), + requests: AsyncMutex::new(Vec::new()), + }); + let mut agent = make_agent_with_builder( + provider, + vec![Box::new(EchoTool)], + Box::new(FixedMemoryLoader { + context: String::new(), + }), + vec![], + crate::openhuman::config::AgentConfig { + max_tool_iterations: 5, + ..crate::openhuman::config::AgentConfig::default() + }, + crate::openhuman::config::ContextConfig::default(), + ); + + let reply = agent + .turn("hello") + .await + .expect("a tool-only turn with no final text should synthesize one, not error"); + assert!( + !reply.trim().is_empty(), + "turn must never end with an empty final message (#4093), got: {reply:?}" + ); + assert!( + reply.contains("I ran echo"), + "the synthesized final message should be the model's wrap-up, got: {reply}" + ); + // The transcript must end on the assistant's final message, not a dangling + // tool cycle. + assert!( + matches!( + agent.history.last(), + Some(ConversationMessage::Chat(msg)) + if msg.role == "assistant" && !msg.content.trim().is_empty() + ), + "history should end on a non-empty assistant message, got: {:?}", + agent.history.last() + ); + // ...and the blank terminal assistant response (folded in from the turn + // outcome) must have been dropped, not left dangling before the synthesized + // answer (Codex review). + assert!( + !agent.history.iter().any(|m| matches!( + m, + ConversationMessage::Chat(msg) + if msg.role == "assistant" && msg.content.trim().is_empty() + )), + "no blank assistant turn should remain in history, got: {:?}", + agent.history + ); +} + +#[tokio::test] +async fn turn_final_answer_falls_back_to_deterministic_summary_when_reprompt_empty() { + // #4093 safety net: the tool ran, the model yielded no final text, and the + // forced final-answer re-prompt ALSO came back empty. The harness must fall + // back to a deterministic summary of the tool calls so the turn is never + // blank — and, unlike the cap path, it must read as a completed summary + // rather than a paused "tool-call limit" checkpoint. + let provider: Arc = Arc::new(SequenceProvider { + responses: AsyncMutex::new(vec![ + Ok(ChatResponse { + text: Some("{\"name\":\"echo\",\"arguments\":{}}".into()), + tool_calls: vec![], + usage: None, + reasoning_content: None, + }), + Ok(ChatResponse { + text: Some(String::new()), + tool_calls: vec![], + usage: None, + reasoning_content: None, + }), + // Re-prompt for a final answer also returns empty. + Ok(ChatResponse { + text: Some(String::new()), + tool_calls: vec![], + usage: None, + reasoning_content: None, + }), + ]), + requests: AsyncMutex::new(Vec::new()), + }); + let mut agent = make_agent_with_builder( + provider, + vec![Box::new(EchoTool)], + Box::new(FixedMemoryLoader { + context: String::new(), + }), + vec![], + crate::openhuman::config::AgentConfig { + max_tool_iterations: 5, + ..crate::openhuman::config::AgentConfig::default() + }, + crate::openhuman::config::ContextConfig::default(), + ); + + let reply = agent + .turn("hello") + .await + .expect("empty final re-prompt should fall back deterministically, not error"); + assert!( + !reply.trim().is_empty(), + "deterministic fallback must be non-empty (#4093), got: {reply:?}" + ); + assert!( + reply.contains("echo"), + "fallback should list the tool that ran, got: {reply}" + ); + assert!( + !reply.contains("tool-call limit"), + "a non-capped turn must not claim it hit the tool-call limit, got: {reply}" + ); + // The blank terminal assistant response must not linger before the + // deterministic summary (Codex review). + assert!( + !agent.history.iter().any(|m| matches!( + m, + ConversationMessage::Chat(msg) + if msg.role == "assistant" && msg.content.trim().is_empty() + )), + "no blank assistant turn should remain in history, got: {:?}", + agent.history + ); +} + #[tokio::test] async fn turn_checkpoint_usage_is_folded_into_transcript_accounting() { // The extra checkpoint provider call costs tokens; those must land in