diff --git a/crates/goose/src/agents/reply_parts.rs b/crates/goose/src/agents/reply_parts.rs index a6f34002307d..0eb5aa824996 100644 --- a/crates/goose/src/agents/reply_parts.rs +++ b/crates/goose/src/agents/reply_parts.rs @@ -182,11 +182,17 @@ impl Agent { ) -> Result { let config = provider.get_model_config(); + let filtered_messages: Vec = messages + .iter() + .filter(|m| m.is_agent_visible()) + .map(|m| m.agent_visible_content()) + .collect(); + // Convert tool messages to text if toolshim is enabled let messages_for_provider = if config.toolshim { - convert_tool_messages_to_text(messages) + convert_tool_messages_to_text(&filtered_messages) } else { - Conversation::new_unvalidated(messages.to_vec()) + Conversation::new_unvalidated(filtered_messages) }; // Clone owned data to move into the async stream diff --git a/crates/goose/src/providers/chatgpt_codex.rs b/crates/goose/src/providers/chatgpt_codex.rs index 196c8b2d26d1..53491d3c96dc 100644 --- a/crates/goose/src/providers/chatgpt_codex.rs +++ b/crates/goose/src/providers/chatgpt_codex.rs @@ -78,9 +78,8 @@ static CHATGPT_CODEX_AUTH_STATE: LazyLock> = fn build_input_items(messages: &[Message]) -> Result> { let mut items = Vec::new(); - for message in messages.iter().filter(|m| m.is_agent_visible()) { - let filtered = message.agent_visible_content(); - let role = match filtered.role { + for message in messages { + let role = match message.role { Role::User => Some("user"), Role::Assistant => Some("assistant"), }; @@ -96,7 +95,7 @@ fn build_input_items(messages: &[Message]) -> Result> { } }; - for content in &filtered.content { + for content in &message.content { match content { MessageContent::Text(text) => { if !text.text.is_empty() { diff --git a/crates/goose/src/providers/cursor_agent.rs b/crates/goose/src/providers/cursor_agent.rs index 829bd173f99c..6b31e2b47c48 100644 --- a/crates/goose/src/providers/cursor_agent.rs +++ b/crates/goose/src/providers/cursor_agent.rs @@ -64,15 +64,14 @@ impl CursorAgentProvider { full_prompt.push_str("\n\n"); // Add conversation history - for message in messages.iter().filter(|m| m.is_agent_visible()) { - let filtered = message.agent_visible_content(); - let role_prefix = match filtered.role { + for message in messages { + let role_prefix = match message.role { Role::User => "Human: ", Role::Assistant => "Assistant: ", }; full_prompt.push_str(role_prefix); - for content in &filtered.content { + for content in &message.content { match content { MessageContent::Text(text_content) => { full_prompt.push_str(&text_content.text); diff --git a/crates/goose/src/providers/formats/anthropic.rs b/crates/goose/src/providers/formats/anthropic.rs index ea6743c9841e..6b20e93662b1 100644 --- a/crates/goose/src/providers/formats/anthropic.rs +++ b/crates/goose/src/providers/formats/anthropic.rs @@ -34,13 +34,7 @@ const DATA_FIELD: &str = "data"; pub fn format_messages(messages: &[Message]) -> Vec { let mut anthropic_messages = Vec::new(); - let filtered_messages: Vec = messages - .iter() - .filter(|m| m.is_agent_visible()) - .map(|m| m.agent_visible_content()) - .collect(); - - for message in &filtered_messages { + for message in messages { let role = match message.role { Role::User => USER_ROLE, Role::Assistant => ASSISTANT_ROLE, diff --git a/crates/goose/src/providers/formats/bedrock.rs b/crates/goose/src/providers/formats/bedrock.rs index 51df50c1edf8..dc37e006b8a8 100644 --- a/crates/goose/src/providers/formats/bedrock.rs +++ b/crates/goose/src/providers/formats/bedrock.rs @@ -18,12 +18,10 @@ use super::super::base::Usage; use crate::conversation::message::{Message, MessageContent}; pub fn to_bedrock_message(message: &Message) -> Result { - let filtered = message.agent_visible_content(); - bedrock::Message::builder() - .role(to_bedrock_role(&filtered.role)) + .role(to_bedrock_role(&message.role)) .set_content(Some( - filtered + message .content .iter() .map(to_bedrock_message_content) diff --git a/crates/goose/src/providers/formats/databricks.rs b/crates/goose/src/providers/formats/databricks.rs index 4f9581b57dfa..a55d26f72f4d 100644 --- a/crates/goose/src/providers/formats/databricks.rs +++ b/crates/goose/src/providers/formats/databricks.rs @@ -106,11 +106,10 @@ fn format_tool_response( /// even though the message structure is otherwise following openai, the enum switches this fn format_messages(messages: &[Message], image_format: &ImageFormat) -> Vec { let mut result = Vec::new(); - for message in messages.iter().filter(|m| m.is_agent_visible()) { - let filtered = message.agent_visible_content(); + for message in messages { let mut converted = DatabricksMessage { content: Value::Null, - role: match filtered.role { + role: match message.role { Role::User => "user".to_string(), Role::Assistant => "assistant".to_string(), }, @@ -122,7 +121,7 @@ fn format_messages(messages: &[Message], image_format: &ImageFormat) -> Vec { if !text.text.is_empty() { diff --git a/crates/goose/src/providers/formats/google.rs b/crates/goose/src/providers/formats/google.rs index 32f7545421ca..f216b0d33ea7 100644 --- a/crates/goose/src/providers/formats/google.rs +++ b/crates/goose/src/providers/formats/google.rs @@ -105,104 +105,75 @@ pub fn format_messages(messages: &[Message]) -> Vec { parts.push(json!({"text":format!("Error: {}", e)})); } }, - MessageContent::ToolResponse(response) => { - match &response.tool_result { - Ok(result) => { - // Send only contents with no audience or with Assistant in the audience - let abridged: Vec<_> = result - .content - .iter() - .filter(|content| { - content.audience().is_none_or(|audience| { - audience.contains(&Role::Assistant) - }) - }) - .map(|content| content.raw.clone()) - .collect(); - - let mut tool_content = Vec::new(); - for content in abridged { - match content { - RawContent::Image(image) => { - parts.push(json!({ - "inline_data": { - "mime_type": image.mime_type, - "data": image.data, - } - })); - } - _ => { - tool_content.push(content.no_annotation()); - } + MessageContent::ToolResponse(response) => match &response.tool_result { + Ok(result) => { + let mut tool_content = Vec::new(); + for content in result.content.iter().map(|c| c.raw.clone()) { + match content { + RawContent::Image(image) => { + parts.push(json!({ + "inline_data": { + "mime_type": image.mime_type, + "data": image.data, + } + })); } - } - let mut text = tool_content - .iter() - .filter_map(|c| match c.deref() { - RawContent::Text(t) => Some(t.text.clone()), - RawContent::Resource(raw_embedded_resource) => Some( - raw_embedded_resource - .clone() - .no_annotation() - .get_text(), - ), - _ => None, - }) - .collect::>() - .join("\n"); - - if text.is_empty() { - text = "Tool call is done.".to_string(); - } - let mut part = Map::new(); - let mut function_response = Map::new(); - function_response.insert("name".to_string(), json!(response.id)); - function_response.insert( - "response".to_string(), - json!({"content": {"text": text}}), - ); - part.insert( - "functionResponse".to_string(), - json!(function_response), - ); - if include_signature { - if let Some(signature) = - get_thought_signature(&response.metadata) - { - part.insert( - THOUGHT_SIGNATURE_KEY.to_string(), - json!(signature), - ); + _ => { + tool_content.push(content.no_annotation()); } } - parts.push(json!(part)); } - Err(e) => { - let mut part = Map::new(); - let mut function_response = Map::new(); - function_response.insert("name".to_string(), json!(response.id)); - function_response.insert( - "response".to_string(), - json!({"content": {"text": format!("Error: {}", e)}}), - ); - part.insert( - "functionResponse".to_string(), - json!(function_response), - ); - if include_signature { - if let Some(signature) = - get_thought_signature(&response.metadata) - { - part.insert( - THOUGHT_SIGNATURE_KEY.to_string(), - json!(signature), - ); - } + let mut text = tool_content + .iter() + .filter_map(|c| match c.deref() { + RawContent::Text(t) => Some(t.text.clone()), + RawContent::Resource(raw_embedded_resource) => Some( + raw_embedded_resource.clone().no_annotation().get_text(), + ), + _ => None, + }) + .collect::>() + .join("\n"); + + if text.is_empty() { + text = "Tool call is done.".to_string(); + } + let mut part = Map::new(); + let mut function_response = Map::new(); + function_response.insert("name".to_string(), json!(response.id)); + function_response + .insert("response".to_string(), json!({"content": {"text": text}})); + part.insert("functionResponse".to_string(), json!(function_response)); + if include_signature { + if let Some(signature) = get_thought_signature(&response.metadata) { + part.insert( + THOUGHT_SIGNATURE_KEY.to_string(), + json!(signature), + ); } - parts.push(json!(part)); } + parts.push(json!(part)); } - } + Err(e) => { + let mut part = Map::new(); + let mut function_response = Map::new(); + function_response.insert("name".to_string(), json!(response.id)); + function_response.insert( + "response".to_string(), + json!({"content": {"text": format!("Error: {}", e)}}), + ); + part.insert("functionResponse".to_string(), json!(function_response)); + if include_signature { + if let Some(signature) = get_thought_signature(&response.metadata) { + part.insert( + THOUGHT_SIGNATURE_KEY.to_string(), + json!(signature), + ); + } + } + parts.push(json!(part)); + } + }, MessageContent::Thinking(thinking) => { let mut part = Map::new(); part.insert("text".to_string(), json!(thinking.thinking)); diff --git a/crates/goose/src/providers/formats/openai.rs b/crates/goose/src/providers/formats/openai.rs index fffbe1c93c78..fd2dbd6f6608 100644 --- a/crates/goose/src/providers/formats/openai.rs +++ b/crates/goose/src/providers/formats/openai.rs @@ -59,17 +59,16 @@ struct StreamingChunk { pub fn format_messages(messages: &[Message], image_format: &ImageFormat) -> Vec { let mut messages_spec = Vec::new(); - for message in messages.iter().filter(|m| m.is_agent_visible()) { - let filtered = message.agent_visible_content(); + for message in messages { let mut converted = json!({ - "role": filtered.role + "role": message.role }); let mut output = Vec::new(); let mut content_array = Vec::new(); let mut text_array = Vec::new(); - for content in &filtered.content { + for content in &message.content { match content { MessageContent::Text(text) => { if !text.text.is_empty() { @@ -136,13 +135,11 @@ pub fn format_messages(messages: &[Message], image_format: &ImageFormat) -> Vec< MessageContent::ToolResponse(response) => { match &response.tool_result { Ok(result) => { - let abridged: Vec<_> = result.content.to_vec(); - // Process all content, replacing images with placeholder text let mut tool_content = Vec::new(); let mut image_messages = Vec::new(); - for content in abridged { + for content in result.content.iter() { match content.deref() { RawContent::Image(image) => { // Add placeholder text in the tool response @@ -164,7 +161,7 @@ pub fn format_messages(messages: &[Message], image_format: &ImageFormat) -> Vec< tool_content.push(Content::text(text)); } _ => { - tool_content.push(content); + tool_content.push(content.clone()); } } } diff --git a/crates/goose/src/providers/formats/openai_responses.rs b/crates/goose/src/providers/formats/openai_responses.rs index 59d85b0b9d0a..f4ca0af6a267 100644 --- a/crates/goose/src/providers/formats/openai_responses.rs +++ b/crates/goose/src/providers/formats/openai_responses.rs @@ -306,9 +306,8 @@ fn add_function_calls(input_items: &mut Vec, messages: &[Message]) { } fn add_function_call_outputs(input_items: &mut Vec, messages: &[Message]) { - for message in messages.iter().filter(|m| m.is_agent_visible()) { - let filtered = message.agent_visible_content(); - for content in &filtered.content { + for message in messages { + for content in &message.content { if let MessageContent::ToolResponse(response) = content { match &response.tool_result { Ok(contents) => { diff --git a/crates/goose/src/providers/formats/snowflake.rs b/crates/goose/src/providers/formats/snowflake.rs index 7896515011f0..ac3cd191ff20 100644 --- a/crates/goose/src/providers/formats/snowflake.rs +++ b/crates/goose/src/providers/formats/snowflake.rs @@ -12,13 +12,7 @@ use std::collections::HashSet; pub fn format_messages(messages: &[Message]) -> Vec { let mut snowflake_messages = Vec::new(); - let filtered_messages: Vec = messages - .iter() - .filter(|m| m.is_agent_visible()) - .map(|m| m.agent_visible_content()) - .collect(); - - for message in &filtered_messages { + for message in messages { let role = match message.role { Role::User => "user", Role::Assistant => "assistant", diff --git a/crates/goose/src/providers/toolshim.rs b/crates/goose/src/providers/toolshim.rs index dc03f377b0ca..a5815e98519c 100644 --- a/crates/goose/src/providers/toolshim.rs +++ b/crates/goose/src/providers/toolshim.rs @@ -321,11 +321,10 @@ pub fn convert_tool_messages_to_text(messages: &[Message]) -> Conversation { let converted_messages: Vec = messages .iter() .map(|message| { - let filtered = message.agent_visible_content(); let mut new_content = Vec::new(); let mut has_tool_content = false; - for content in &filtered.content { + for content in &message.content { match content { MessageContent::ToolRequest(req) => { has_tool_content = true; @@ -370,9 +369,9 @@ pub fn convert_tool_messages_to_text(messages: &[Message]) -> Conversation { } if has_tool_content { - Message::new(filtered.role.clone(), filtered.created, new_content) + Message::new(message.role.clone(), message.created, new_content) } else { - filtered + message.clone() } }) .collect(); diff --git a/crates/goose/tests/compaction.rs b/crates/goose/tests/compaction.rs index 4081db928ff7..59d7f923a40d 100644 --- a/crates/goose/tests/compaction.rs +++ b/crates/goose/tests/compaction.rs @@ -684,26 +684,26 @@ async fn test_context_limit_recovery_compaction() -> Result<()> { ); // Check the final token state after recovery - // Note: The current session state reflects the compaction operation, - // as the agent records compaction metrics before retrying + // Note: The session state reflects the RETRY call (after compaction), + // which only sees agent-visible messages (summary + continuation + user message) let final_input = updated_session.input_tokens.unwrap(); let final_output = updated_session.output_tokens; let final_total = updated_session.total_tokens.unwrap(); - // After compaction during recovery, the session shows the compaction tokens - // Input: system (6000) + long_tool_call messages (~15,400) + new message (100) = ~21,500 - // Output: 200 (compaction summary) - // Total: ~21,700 + // After compaction, the retry only sees agent-visible messages: + // Input: system (6000) + summary (~100) + continuation (~100) + user message (~100) = ~6300 + // Output: 200 (mock detects "summarized" in continuation as compaction) + // Total: ~6500 assert!( - (21000..=22000).contains(&final_input), - "Final input should reflect compaction input (~21,500). Got: {}", + (6000..=6600).contains(&final_input), + "Final input should reflect retry with agent-visible messages (~6300). Got: {}", final_input ); assert_eq!( final_output, Some(200), - "Final output should be compaction output (200). Got: {:?}", + "Final output should be 200 (mock detects continuation as compaction). Got: {:?}", final_output ); @@ -715,13 +715,13 @@ async fn test_context_limit_recovery_compaction() -> Result<()> { // Accumulated tokens should include all operations: // - Initial: 1000 - // - Compaction: ~21,600 input (system + long messages) + 200 output = 21,800 - // - Reply: ~6,300 input + 100 output = 6,400 - // Total: 1000 + 21,800 + 6,400 = 29,200 + // - Compaction: ~6400 input (mock uses system_prompt.len()/4) + 200 output = ~6600 + // - Reply: ~6500 input + 200 output = ~6700 + // Total: 1000 + 6600 + 6700 = ~14300 let accumulated = updated_session.accumulated_total_tokens.unwrap(); assert!( - (28000..=30000).contains(&accumulated), - "Accumulated should be ~29,200 (initial + compaction + reply). Got: {}", + (13000..=16000).contains(&accumulated), + "Accumulated should be ~14300 (initial + compaction + reply). Got: {}", accumulated );