fix(harness): close empty and breaker-halted turns from tool records and check the reply

crates/openhuman-core/src/agent/harness/session/turn/core.rs

@@ -15,6 +15,35 @@ use crate::memory::MemoryCategory;
 use anyhow::Result;
 use std::hash::{Hash, Hasher};
 
+/// The outcome captured for one assistant tool call, matched by call id, tool
+/// name **and occurrence**.
+///
+/// Prompt-guided (XML / P-Format) calls carry no provider id, so every model
+/// response synthesizes `call_0`, `call_1`, … afresh (`tinyagents::model`), and a
+/// turn with several tool rounds repeats the same ids. A first-match lookup gave
+/// every later round the first round's result, so a turn's records could hide its
+/// latest failure (Codex review on #6289). The n-th call under an `(id, name)`
+/// takes the n-th outcome recorded under it: rounds run in order and ids are
+/// unique within a round, so parallel execution inside a round cannot reorder
+/// them. Keying on the name as well stops an unknown-tool call, which never
+/// reaches the capture sink, from consuming a later round's real outcome.
+fn nth_call_outcome<'a>(
+    tool_outcomes: &'a [crate::agent::tinyagents::ToolCallOutcome],
+    seen: &mut std::collections::HashMap<(String, String), usize>,
+    call_id: &str,
+    name: &str,
+) -> Option<&'a crate::agent::tinyagents::ToolCallOutcome> {
+    let occurrence = seen
+        .entry((call_id.to_string(), name.to_string()))
+        .or_insert(0);
+    let outcome = tool_outcomes
+        .iter()
+        .filter(|o| o.call_id == call_id && o.name == name)
+        .nth(*occurrence);
+    *occurrence += 1;
+    outcome
+}
+
 /// Flatten the assistant tool calls a turn produced into [`ToolCallRecord`]s for
 /// post-turn hooks + the deterministic cap checkpoint. Per-call success +
 /// sanitized output summary are recovered from the turn's captured
@@ -26,10 +55,11 @@ fn tool_records_from_conversation(
     tool_outcomes: &[crate::agent::tinyagents::ToolCallOutcome],
 ) -> Vec<hooks::ToolCallRecord> {
     let mut records = Vec::new();
+    let mut seen = std::collections::HashMap::new();
     for msg in conversation {
         if let ConversationMessage::AssistantToolCalls { tool_calls, .. } = msg {
             for call in tool_calls {
-                let outcome = tool_outcomes.iter().find(|o| o.call_id == call.id);
+                let outcome = nth_call_outcome(tool_outcomes, &mut seen, &call.id, &call.name);
                 // Default a MISSING outcome to `false` (#4467, item 7): a call
                 // with no captured outcome is a hallucinated/unknown tool the
                 // crate recovered via `ReturnToolError` without running
@@ -69,10 +99,11 @@ fn checkpoint_results_from_conversation(
     tool_outcomes: &[crate::agent::tinyagents::ToolCallOutcome],
 ) -> Vec<super::super::turn_checkpoint::CheckpointToolResult> {
     let mut results = Vec::new();
+    let mut seen = std::collections::HashMap::new();
     for msg in conversation {
         if let ConversationMessage::AssistantToolCalls { tool_calls, .. } = msg {
             for call in tool_calls {
-                let outcome = tool_outcomes.iter().find(|o| o.call_id == call.id);
+                let outcome = nth_call_outcome(tool_outcomes, &mut seen, &call.id, &call.name);
                 // Same missing-outcome rule as `tool_records_from_conversation`:
                 // a call the crate recovered without running `after_tool` never
                 // reached the capture sink, so it is reported as failed rather
@@ -214,5 +245,6 @@ include!("core_turn.rs");
 // calls, split by responsibility into real submodules. Their `impl Agent`
 // blocks complement `turn()` above.
 mod experience_context;
+mod grounded_close;
 mod harness_turn;
 mod resumed_prefix;

crates/openhuman-core/src/agent/harness/session/turn/core/grounded_close.rs

@@ -0,0 +1,123 @@
+//! Close a turn that has no usable reply of its own from its tool records: a
+//! tool turn that ended without final text (#4093, #6278) or a run the
+//! no-progress breaker halted (#6279).
+
+use crate::agent::harness::session::turn_checkpoint::{self, CloseVerdict};
+use crate::agent::harness::session::types::Agent;
+use crate::agent::messages::{ChatMessage, ConversationMessage};
+use crate::agent::tinyagents::TinyagentsTurnOutcome;
+use crate::inference::provider::UsageInfo;
+
+impl Agent {
+    /// Write the closing message for a turn whose run produced no usable reply:
+    /// the loop ended on empty text after tool work (#4093), or the breaker
+    /// halted it (`outcome.breaker_halt`, #6279).
+    ///
+    /// Three steps, each closing a way the old path shipped a non-answer:
+    ///
+    /// 1. **Grounded wrap-up.** One tools-disabled call whose instruction
+    ///    restates this turn's tool records, failures and their messages
+    ///    included, plus the breaker's stop note as input. Context middleware
+    ///    may have cleared or summarised those bodies earlier in the turn, and a
+    ///    model that cannot see a success will say it never happened (#6278).
+    /// 2. **Check.** A separate call that sees only the request, the records and
+    ///    the candidate rejects a reply that narrates intent, contradicts a
+    ///    record, or drops the failure that explains an unfinished request.
+    /// 3. **Fallback.** An empty, tool-calling, rejected or unverified close
+    ///    (the check failed or gave no verdict) is replaced by
+    ///    [`turn_checkpoint::build_deterministic_final_summary`], which quotes
+    ///    each result and the stop note.
+    ///
+    /// Accepted model text is streamed only after the check, so a rejected
+    /// reply never renders and the streamed text matches the persisted one. The
+    /// reply is pushed onto `history`, and the usage of every extra call is
+    /// returned for the caller's turn accounting.
+    pub(super) async fn close_turn_from_records(
+        &mut self,
+        outcome: &TinyagentsTurnOutcome,
+        user_message: &str,
+        effective_model: &str,
+    ) -> (String, Vec<UsageInfo>) {
+        let stop_reason = outcome.breaker_halt.as_deref();
+        let mut usage = Vec::new();
+
+        // The run folded its blank terminal assistant response into history (an
+        // empty `Chat(assistant(""))`). Drop it before the wrap-up request and
+        // before the reply is appended: strict providers reject empty content,
+        // and the transcript must not carry a dangling blank turn.
+        if self
+            .history
+            .last()
+            .is_some_and(super::is_empty_assistant_chat)
+        {
+            self.history.pop();
+        }
+
+        let results = super::checkpoint_results_from_conversation(
+            &outcome.conversation,
+            &outcome.tool_outcomes,
+        );
+        let records =
+            turn_checkpoint::render_tool_results(&results, turn_checkpoint::GROUNDING_TOTAL_CHARS);
+        let iteration = outcome.model_calls as u32 + 1;
+
+        let base = self.tool_dispatcher.to_provider_messages(&self.history);
+        let (candidate, candidate_usage) = self
+            .summarize_turn_wrapup(
+                &base,
+                effective_model,
+                iteration,
+                &turn_checkpoint::final_answer_instruction(stop_reason, &records),
+                false,
+            )
+            .await;
+        usage.extend(candidate_usage);
+
+        let verdict = if candidate.trim().is_empty() {
+            None
+        } else {
+            let prompt =
+                turn_checkpoint::close_verification_prompt(user_message, &records, &candidate);
+            let (verdict_text, verdict_usage) = self
+                .silent_completion(
+                    &[ChatMessage::user(prompt)],
+                    effective_model,
+                    "closing-message check",
+                )
+                .await;
+            usage.extend(verdict_usage);
+            Some(turn_checkpoint::parse_close_verdict(&verdict_text))
+        };
+        if verdict == Some(CloseVerdict::Unclear) {
+            log::warn!(
+                "[agent_loop] closing-message check returned no ACCEPT/REJECT verdict; using the deterministic fallback"
+            );
+        }
+
+        // Only an explicit ACCEPT ships the model's text. A failed or malformed
+        // check leaves the reply unverified, and shipping unverified closing text
+        // is the defect this path exists to stop (CodeRabbit on #6289).
+        let accepted = verdict == Some(CloseVerdict::Accept);
+        let reply = if accepted {
+            self.stream_text_continuation(&candidate, iteration).await;
+            candidate
+        } else {
+            turn_checkpoint::build_deterministic_final_summary(&results, stop_reason)
+        };
+
+        log::info!(
+            "[agent_loop] closed turn from tool records after {} tool call(s): halted={} verdict={:?} fallback={} ({} chars) — #4093 #6278 #6279",
+            outcome.tool_calls,
+            stop_reason.is_some(),
+            verdict,
+            !accepted,
+            reply.chars().count()
+        );
+
+        self.history
+            .push(ConversationMessage::Chat(ChatMessage::assistant(
+                reply.clone(),
+            )));
+        (reply, usage)
+    }
+}

crates/openhuman-core/src/agent/harness/session/turn/core/harness_turn.rs

@@ -273,6 +273,7 @@ impl Agent {
                     effective_model,
                     outcome.model_calls as u32 + 1,
                     turn_checkpoint::MAX_ITER_CHECKPOINT_INSTRUCTION,
+                    true,
                 )
                 .await;
             if let Some(u) = summary_usage {
@@ -328,61 +329,31 @@ 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,
-                    turn_checkpoint::FINAL_ANSWER_INSTRUCTION,
-                )
+        } else if outcome.breaker_halt.is_some() || outcome.text.trim().is_empty() {
+            // Two ways a turn reaches here without a reply it can ship:
+            //
+            // * #4093: the loop ran tool calls (tool_calls > 0, so the branch
+            //   above did not fire) and then ended with no final text — the
+            //   turn did work but would otherwise end silently.
+            // * #6279: the no-progress breaker halted the run. `outcome.text`
+            //   then holds the breaker's stop note, which is worded for a model
+            //   ("Report this back instead of retrying") and must not be the
+            //   user's reply.
+            //
+            // Both close from the turn's tool records: a grounded wrap-up, a
+            // check that rejects intent narration or claims the records
+            // contradict (#6278), and a deterministic fallback that quotes the
+            // failures. Fold the extra calls' usage into the turn accounting,
+            // exactly like the cap path above.
+            let (final_answer, close_usage) = self
+                .close_turn_from_records(&outcome, user_message, effective_model)
                 .await;
-            if let Some(u) = summary_usage {
+            for u in close_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() {
-                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 if outcome.early_exit_tool.is_some() {
             // Paused on `ask_user_clarification`. The run stopped right after the