test: MockProvider rejects transcripts a real provider would

src/provider/mock.rs

@@ -32,15 +32,95 @@ pub struct MockToolCall {
 /// Mock LLM provider for tests. Supply a sequence of responses.
 pub struct MockProvider {
     responses: std::sync::Mutex<Vec<MockResponse>>,
+    /// Whether to reject a request whose transcript a real provider would.
+    ///
+    /// On by default, and that default is the point. This type used to discard
+    /// `StreamConfig` entirely, so 600+ tests drove the agent loop without one
+    /// of them noticing when it built a message sequence every provider
+    /// rejects. Loop detection shipped exactly that bug to a release branch:
+    /// a steering message injected between an assistant's `tool_use` blocks and
+    /// their `tool_result`s, which poisons the agent for every later prompt.
+    ///
+    /// Turning this off is legitimate for a test that deliberately constructs a
+    /// malformed history — say the compaction orphan-handling paths — but it
+    /// should be deliberate and commented.
+    validate_transcript: bool,
+}
+
+/// Reject a message sequence a real provider would reject.
+///
+/// The invariant every provider enforces: an assistant's `tool_use` blocks must
+/// be answered by their `tool_result`s before anything else intervenes.
+/// Anthropic returns "tool_use ids were found without tool_result blocks
+/// immediately after"; OpenAI returns the equivalent about tool messages. The
+/// crate treats this as sacred elsewhere — `context.rs` calls an unanswered
+/// call "an orphan every provider rejects", and `llm_compaction.rs` has a
+/// dedicated test for it — but nothing enforced it on the loop's own output.
+///
+/// Returns the reason a provider would give, or `None` if the transcript is
+/// well-formed.
+fn transcript_violation(messages: &[Message]) -> Option<String> {
+    let mut pending: Vec<String> = Vec::new();
+    for (i, msg) in messages.iter().enumerate() {
+        match msg {
+            Message::Assistant { content, .. } => {
+                if !pending.is_empty() {
+                    return Some(format!(
+                        "assistant message at [{i}] while tool calls {pending:?} are still \
+                         unanswered"
+                    ));
+                }
+                pending = content
+                    .iter()
+                    .filter_map(|c| match c {
+                        Content::ToolCall { id, .. } => Some(id.clone()),
+                        _ => None,
+                    })
+                    .collect();
+            }
+            Message::ToolResult { tool_call_id, .. } => {
+                match pending.iter().position(|p| p == tool_call_id) {
+                    Some(at) => {
+                        pending.remove(at);
+                    }
+                    None => {
+                        return Some(format!(
+                            "tool_result at [{i}] for {tool_call_id:?} answers no open tool call"
+                        ))
+                    }
+                }
+            }
+            Message::User { .. } => {
+                if !pending.is_empty() {
+                    return Some(format!(
+                        "a user message at [{i}] lands between an assistant's tool_use and its \
+                         tool_result — {pending:?} still unanswered"
+                    ));
+                }
+            }
+        }
+    }
+    None
 }
 
 impl MockProvider {
     pub fn new(responses: Vec<MockResponse>) -> Self {
         Self {
             responses: std::sync::Mutex::new(responses),
+            validate_transcript: true,
         }
     }
 
+    /// Accept message sequences a real provider would reject.
+    ///
+    /// For tests that construct a malformed history *on purpose* — orphan
+    /// handling, compaction edge cases, recovery paths. Say why at the call
+    /// site; the default exists because the alternative let a real bug through.
+    pub fn without_transcript_validation(mut self) -> Self {
+        self.validate_transcript = false;
+        self
+    }
+
     /// Convenience: provider that always returns the same text
     pub fn text(text: impl Into<String>) -> Self {
         Self::new(vec![MockResponse::Text(text.into())])
@@ -61,10 +141,27 @@ impl MockProvider {
 impl StreamProvider for MockProvider {
     async fn stream(
         &self,
-        _config: StreamConfig,
+        config: StreamConfig,
         tx: mpsc::UnboundedSender<StreamEvent>,
         cancel: tokio_util::sync::CancellationToken,
     ) -> Result<Message, ProviderError> {
+        // A mock that accepts anything tests the loop against a provider that
+        // does not exist. Panic rather than return an error: a malformed
+        // transcript is a defect in the code under test, and the loop is
+        // designed to survive provider *errors*, so returning one would be
+        // swallowed by the very retry path that should never see this.
+        if self.validate_transcript {
+            if let Some(why) = transcript_violation(&config.messages) {
+                panic!(
+                    "MockProvider received a transcript a real provider would reject: {why}.\n\n\
+                     This is the shape that poisons an agent — the history is kept, so every \
+                     later prompt fails too. If the malformed sequence is deliberate, use \
+                     `MockProvider::without_transcript_validation()` and say why.\n\n\
+                     Messages: {:#?}",
+                    config.messages
+                );
+            }
+        }
         let response = {
             let mut responses = self.responses.lock().unwrap();
             if responses.is_empty() {

tests/agent_loop_test.rs

@@ -230,6 +230,22 @@ async fn test_continue_from_tool_result() {
         system_prompt: "test".into(),
         messages: vec![
             AgentMessage::Llm(Message::user("do something")),
+            // The assistant turn that *made* the call. Without it the
+            // tool_result answers nothing, which is a history no provider
+            // accepts — and which therefore cannot arise in production, so
+            // asserting resume behaviour on it proved nothing about the real
+            // path. MockProvider now rejects it.
+            AgentMessage::Llm(Message::assistant(
+                vec![Content::tool_call(
+                    "tc-1",
+                    "test_tool",
+                    serde_json::json!({}),
+                )],
+                StopReason::ToolUse,
+                "mock",
+                "mock",
+                Usage::default(),
+            )),
             AgentMessage::Llm(Message::ToolResult {
                 tool_call_id: "tc-1".into(),
                 tool_name: "test_tool".into(),