fix: keep recalled history agent-visible (#11490)
This commit is contained in:
@@ -7,8 +7,8 @@ use anyhow::Result;
|
||||
use async_trait::async_trait;
|
||||
use indoc::indoc;
|
||||
use rmcp::model::{
|
||||
CallToolResult, ContentBlock, Implementation, InitializeResult, JsonObject, ListToolsResult,
|
||||
ServerCapabilities, Tool, ToolAnnotations,
|
||||
Annotations, CallToolResult, ContentBlock, Implementation, InitializeResult, JsonObject,
|
||||
ListToolsResult, Role, ServerCapabilities, TextContent, Tool, ToolAnnotations,
|
||||
};
|
||||
use schemars::{schema_for, JsonSchema};
|
||||
use serde::{Deserialize, Serialize};
|
||||
@@ -40,6 +40,13 @@ pub struct ChatRecallClient {
|
||||
context: PlatformExtensionContext,
|
||||
}
|
||||
|
||||
fn agent_only_history(text: String) -> ContentBlock {
|
||||
ContentBlock::Text(
|
||||
TextContent::new(text)
|
||||
.with_annotations(Annotations::default().with_audience(vec![Role::Assistant])),
|
||||
)
|
||||
}
|
||||
|
||||
fn format_agent_visible_excerpt(conversation: &Conversation) -> Option<(usize, String)> {
|
||||
let messages: Vec<_> = conversation
|
||||
.agent_visible_messages()
|
||||
@@ -156,7 +163,7 @@ impl ChatRecallClient {
|
||||
|
||||
output.push_str(&excerpt);
|
||||
|
||||
Ok(vec![ContentBlock::text(output)])
|
||||
Ok(vec![agent_only_history(output)])
|
||||
}
|
||||
Err(e) => Err(format!("Failed to load session: {}", e)),
|
||||
}
|
||||
@@ -241,7 +248,12 @@ impl ChatRecallClient {
|
||||
}
|
||||
output
|
||||
};
|
||||
Ok(vec![ContentBlock::text(formatted_results)])
|
||||
let content = if results.total_matches == 0 {
|
||||
ContentBlock::text(formatted_results)
|
||||
} else {
|
||||
agent_only_history(formatted_results)
|
||||
};
|
||||
Ok(vec![content])
|
||||
}
|
||||
Err(e) => Err(format!("Chat recall failed: {}", e)),
|
||||
}
|
||||
@@ -325,8 +337,10 @@ impl McpClientTrait for ChatRecallClient {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::config::GooseMode;
|
||||
use crate::conversation::message::{Message, MessageContent, MessageMetadata};
|
||||
use rmcp::model::{Annotations, Role, TextContent};
|
||||
use crate::session::SessionManager;
|
||||
use std::sync::Arc;
|
||||
|
||||
fn annotated_text(text: &str, audience: Vec<Role>) -> MessageContent {
|
||||
MessageContent::Text(
|
||||
@@ -334,6 +348,28 @@ mod tests {
|
||||
)
|
||||
}
|
||||
|
||||
fn projected_tool_text(content: Vec<ContentBlock>, audience: Role) -> String {
|
||||
let message =
|
||||
Message::user().with_tool_response("chatrecall", Ok(CallToolResult::success(content)));
|
||||
let projected = match audience {
|
||||
Role::Assistant => message.agent_visible_content(),
|
||||
Role::User => message.user_visible_content(),
|
||||
};
|
||||
let Some(MessageContent::ToolResponse(response)) = projected.content.first() else {
|
||||
return String::new();
|
||||
};
|
||||
let Ok(result) = &response.tool_result else {
|
||||
return String::new();
|
||||
};
|
||||
|
||||
result
|
||||
.content
|
||||
.iter()
|
||||
.filter_map(|content| content.as_text().map(|text| text.text.clone()))
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n")
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn loaded_excerpt_projects_audience_before_selecting_endpoints() {
|
||||
let conversation = Conversation::new_unvalidated([
|
||||
@@ -370,4 +406,76 @@ mod tests {
|
||||
assert!(canonical_user_text.contains("user-only first secret"));
|
||||
assert!(canonical_user_text.contains("user-only last secret"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn history_results_remain_agent_visible_without_becoming_user_visible() {
|
||||
let temp_dir = tempfile::tempdir().unwrap();
|
||||
let session_manager = Arc::new(SessionManager::new(temp_dir.path().to_path_buf()));
|
||||
let current_session = session_manager
|
||||
.create_session(
|
||||
temp_dir.path().to_path_buf(),
|
||||
"current".to_string(),
|
||||
SessionType::User,
|
||||
GooseMode::default(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
let target_session = session_manager
|
||||
.create_session(
|
||||
temp_dir.path().to_path_buf(),
|
||||
"target".to_string(),
|
||||
SessionType::User,
|
||||
GooseMode::default(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
session_manager
|
||||
.add_message(
|
||||
&target_session.id,
|
||||
&Message::assistant()
|
||||
.with_text("agent-only secret marker")
|
||||
.with_metadata(MessageMetadata::agent_only()),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let client = ChatRecallClient::new(PlatformExtensionContext {
|
||||
extension_manager: None,
|
||||
session_manager,
|
||||
scheduler: None,
|
||||
session: Some(Arc::new(current_session.clone())),
|
||||
use_login_shell_path: false,
|
||||
})
|
||||
.unwrap();
|
||||
let load_output = client
|
||||
.handle_chatrecall(
|
||||
¤t_session.id,
|
||||
Some(
|
||||
serde_json::json!({ "session_id": target_session.id })
|
||||
.as_object()
|
||||
.unwrap()
|
||||
.clone(),
|
||||
),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
let search_output = client
|
||||
.handle_chatrecall(
|
||||
¤t_session.id,
|
||||
Some(
|
||||
serde_json::json!({ "query": "agent-only secret marker" })
|
||||
.as_object()
|
||||
.unwrap()
|
||||
.clone(),
|
||||
),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
for output in [load_output, search_output] {
|
||||
assert!(projected_tool_text(output.clone(), Role::Assistant)
|
||||
.contains("agent-only secret marker"));
|
||||
assert!(!projected_tool_text(output, Role::User).contains("agent-only secret marker"));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -982,7 +982,9 @@ impl Operation<Session, GooseEffect> for ToolExecutionOperation<'_> {
|
||||
effects.push(self.extension_state_effect(session).await?);
|
||||
}
|
||||
|
||||
let response = emit.message(response).await;
|
||||
let response = response.with_generated_id_if_missing();
|
||||
emit.emit(AgentEvent::Message(response.user_visible_content()))
|
||||
.await;
|
||||
effects.push(response.into());
|
||||
applied(effects)
|
||||
}
|
||||
|
||||
@@ -9,6 +9,7 @@ use super::pipeline::MessageKind::{Agent, Confirmation, ToolCall, ToolResponse};
|
||||
use super::pipeline::MAX_TURNS;
|
||||
use super::test_pipeline;
|
||||
use crate::agents::tool_execution::{CHAT_MODE_TOOL_SKIPPED_RESPONSE, DECLINED_RESPONSE};
|
||||
use crate::agents::AgentEvent;
|
||||
use crate::config::permission::PermissionLevel;
|
||||
use crate::config::GooseMode;
|
||||
use crate::conversation::message::{Message, MessageContent};
|
||||
@@ -268,6 +269,21 @@ async fn execution_recovers_from_timeout_cancellation_and_filtered_output() -> R
|
||||
api.on("result: 3").reply("shown once");
|
||||
let result = pipeline.run(["show the result once"]).await?;
|
||||
result.assert_message(-1, Agent, "shown once");
|
||||
let emitted_result_count = result
|
||||
.events
|
||||
.iter()
|
||||
.filter_map(|event| match event {
|
||||
AgentEvent::Message(message) => Some(message),
|
||||
_ => None,
|
||||
})
|
||||
.flat_map(|message| &message.content)
|
||||
.filter_map(MessageContent::as_tool_response)
|
||||
.filter_map(|response| response.tool_result.as_ref().ok())
|
||||
.flat_map(|result| &result.content)
|
||||
.filter_map(|content| content.as_text())
|
||||
.filter(|text| text.text == "result: 3")
|
||||
.count();
|
||||
assert_eq!(emitted_result_count, 1);
|
||||
assert_eq!(
|
||||
api.calls().last().unwrap().input_occurrences("result: 3"),
|
||||
1
|
||||
|
||||
Reference in New Issue
Block a user