diff --git a/crates/goose/src/conversation/mod.rs b/crates/goose/src/conversation/mod.rs index 24ed6fbefc..2b65bbf792 100644 --- a/crates/goose/src/conversation/mod.rs +++ b/crates/goose/src/conversation/mod.rs @@ -58,10 +58,16 @@ impl Conversation { Some(MessageContent::Thinking(ref mut last)), Some(MessageContent::Thinking(new)), ) if message.content.len() == 1 - && (new.signature.is_empty() - || last.signature.is_empty() - || new.signature == last.signature) => + && (last.signature.is_empty() || new.signature == last.signature) => { + // Merge cases: + // - `last` is still unsigned (block in progress) — append + // and adopt `new.signature` if it's the closing delta. + // - signatures match — same block continuing. + // An unsigned delta arriving after a signed block belongs + // to the next block (signature-at-end streams emit the + // first text of block N+1 before its signature), so the + // outer match arm falls through to push it separately. last.thinking.push_str(&new.thinking); if !new.signature.is_empty() { last.signature = new.signature.clone(); @@ -1294,10 +1300,12 @@ mod tests { } #[test] - fn test_push_thinking_adopts_latest_signature() { + fn test_push_thinking_adopts_signature_on_closing_delta() { use crate::conversation::message::MessageContent; let mut conv = Conversation::empty(); + // Streamed shape for one signed block: text deltas accumulate while + // unsigned; the closing delta carries the signature. conv.push( Message::assistant() .with_thinking("a", "") @@ -1308,24 +1316,57 @@ mod tests { .with_thinking("b", "sig1") .with_id("turn-1"), ); - conv.push( - Message::assistant() - .with_thinking("c", "") - .with_id("turn-1"), - ); let content = &conv.messages()[0].content; assert_eq!(content.len(), 1); match &content[0] { MessageContent::Thinking(t) => { - assert_eq!(t.thinking, "abc"); - // Non-empty signature wins; later empty signature does not clobber. + assert_eq!(t.thinking, "ab"); assert_eq!(t.signature, "sig1"); } other => panic!("expected Thinking, got {:?}", other), } } + #[test] + fn test_push_unsigned_thinking_after_signed_starts_new_block() { + use crate::conversation::message::MessageContent; + + let mut conv = Conversation::empty(); + conv.push( + Message::assistant() + .with_thinking("first body", "sig1") + .with_id("turn-1"), + ); + // A second thinking block begins; in signature-at-end streams the + // first text arrives before the block's signature, so the new + // unsigned delta must NOT be appended to the already-signed block — + // otherwise the closing signature later would replay text under + // the wrong signature. + conv.push( + Message::assistant() + .with_thinking("second body start", "") + .with_id("turn-1"), + ); + + let content = &conv.messages()[0].content; + assert_eq!( + content.len(), + 2, + "unsigned delta must not merge into a signed block: {:?}", + content + ); + match (&content[0], &content[1]) { + (MessageContent::Thinking(a), MessageContent::Thinking(b)) => { + assert_eq!(a.thinking, "first body"); + assert_eq!(a.signature, "sig1"); + assert_eq!(b.thinking, "second body start"); + assert_eq!(b.signature, ""); + } + other => panic!("unexpected content shape: {:?}", other), + } + } + #[test] fn test_push_keeps_distinct_signed_thinking_blocks_separate() { use crate::conversation::message::MessageContent;