-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Platform Extension MOIM (Minus One Info Message) #5027
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 34 commits
c22d3f8
c821645
fd6e6c4
e9c48e2
5c28a48
250b3c8
beb11cd
0acdc97
9ec9fe8
7813fb5
7a444c0
d1499c5
599f61c
f85335d
ba2004e
811da83
f099955
780e0ad
b286cbd
d2f075f
07c0ce2
528282a
fae6e77
0604895
b7b00ae
94f83e2
4853b30
5f228ab
5607fd5
b153859
3cdc5be
7871fa0
2eb9c81
2774671
df80a59
a7344a1
a954dc6
282775e
29fc6f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -946,10 +946,15 @@ impl Agent { | |
| } | ||
| } | ||
|
|
||
| let conversation_with_moim = super::moim::inject_moim( | ||
| conversation.clone(), | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this replaces conversation, so I don't think we need to clone here |
||
| &self.extension_manager, | ||
| ).await; | ||
|
|
||
| let mut stream = Self::stream_response_from_provider( | ||
| self.provider().await?, | ||
| &system_prompt, | ||
| conversation.messages(), | ||
| conversation_with_moim.messages(), | ||
| &tools, | ||
| &toolshim_tools, | ||
| ).await?; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1163,6 +1163,28 @@ impl ExtensionManager { | |
| .get(&name.into()) | ||
| .map(|ext| ext.get_client()) | ||
| } | ||
|
|
||
| pub async fn collect_moim(&self) -> Option<String> { | ||
| let timestamp = chrono::Local::now().format("%Y-%m-%d %H:%M:%S").to_string(); | ||
| let mut content = format!("<info-msg>\nDatetime: {}\n", timestamp); | ||
tlongwell-block marked this conversation as resolved.
Show resolved
Hide resolved
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a list with join would be more memory effiicient |
||
|
|
||
| let extensions = self.extensions.lock().await; | ||
| for (name, extension) in extensions.iter() { | ||
| if let ExtensionConfig::Platform { .. } = &extension.config { | ||
| let client = extension.get_client(); | ||
| let client_guard = client.lock().await; | ||
| if let Some(moim_content) = client_guard.get_moim().await { | ||
| tracing::debug!("MOIM content from {}: {} chars", name, moim_content.len()); | ||
| content.push('\n'); | ||
| content.push_str(&moim_content); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| content.push_str("\n</info-msg>"); | ||
|
|
||
| Some(content) | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,147 @@ | ||
| use crate::agents::extension_manager::ExtensionManager; | ||
| use crate::conversation::message::Message; | ||
| use crate::conversation::{fix_conversation, Conversation}; | ||
| use rmcp::model::Role; | ||
|
|
||
| pub async fn inject_moim( | ||
tlongwell-block marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| conversation: Conversation, | ||
| extension_manager: &ExtensionManager, | ||
| ) -> Conversation { | ||
| let config = crate::config::Config::global(); | ||
| if !config.get_param("GOOSE_MOIM_ENABLED").unwrap_or(true) { | ||
| return conversation; | ||
| } | ||
|
|
||
| if let Some(moim) = extension_manager.collect_moim().await { | ||
| let mut messages = conversation.messages().clone(); | ||
| let idx = messages | ||
| .iter() | ||
| .rposition(|m| m.role == Role::Assistant) | ||
| .unwrap_or(messages.len()); | ||
| messages.insert(idx, Message::user().with_text(moim)); | ||
|
|
||
| let (fixed, _issues) = fix_conversation(Conversation::new_unvalidated(messages)); | ||
| return fixed; | ||
| } | ||
tlongwell-block marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| conversation | ||
tlongwell-block marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
tlongwell-block marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
| use rmcp::model::CallToolRequestParam; | ||
|
|
||
| #[tokio::test] | ||
| async fn test_moim_injection_before_assistant() { | ||
| let em = ExtensionManager::new_without_provider(); | ||
|
|
||
| let conv = Conversation::new_unvalidated(vec![ | ||
| Message::user().with_text("Hello"), | ||
| Message::assistant().with_text("Hi"), | ||
| Message::user().with_text("Bye"), | ||
| Message::assistant().with_text("Goodbye"), | ||
| ]); | ||
| let result = inject_moim(conv, &em).await; | ||
| let msgs = result.messages(); | ||
|
|
||
| // MOIM gets inserted before last assistant, then merged with "Bye", | ||
| // but then the trailing assistant gets removed by fix_conversation | ||
| assert_eq!(msgs.len(), 3); | ||
| assert_eq!(msgs[0].content[0].as_text().unwrap(), "Hello"); | ||
| assert_eq!(msgs[1].content[0].as_text().unwrap(), "Hi"); | ||
|
|
||
| // The third message should be the merged "Bye" + MOIM | ||
| let merged_content = msgs[2] | ||
| .content | ||
| .iter() | ||
| .filter_map(|c| c.as_text()) | ||
| .collect::<Vec<_>>() | ||
| .join(""); | ||
| assert!(merged_content.contains("Bye")); | ||
| assert!(merged_content.contains("<info-msg>")); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn test_moim_injection_no_assistant() { | ||
| let em = ExtensionManager::new_without_provider(); | ||
|
|
||
| let conv = Conversation::new_unvalidated(vec![Message::user().with_text("Hello")]); | ||
| let result = inject_moim(conv, &em).await; | ||
|
|
||
| // MOIM gets merged with the existing user message | ||
| assert_eq!(result.messages().len(), 1); | ||
|
|
||
| let merged_content = result.messages()[0] | ||
| .content | ||
| .iter() | ||
| .filter_map(|c| c.as_text()) | ||
| .collect::<Vec<_>>() | ||
| .join(""); | ||
| assert!(merged_content.contains("Hello")); | ||
| assert!(merged_content.contains("<info-msg>")); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn test_moim_with_tool_calls() { | ||
| let em = ExtensionManager::new_without_provider(); | ||
|
|
||
| let conv = Conversation::new_unvalidated(vec![ | ||
| Message::user().with_text("Search for something"), | ||
| Message::assistant() | ||
| .with_text("I'll search for you") | ||
| .with_tool_request( | ||
| "search_1", | ||
| Ok(CallToolRequestParam { | ||
| name: "search".into(), | ||
| arguments: None, | ||
| }), | ||
| ), | ||
| Message::user().with_tool_response("search_1", Ok(vec![])), | ||
| Message::assistant().with_text("Found results"), | ||
| ]); | ||
|
|
||
| let result = inject_moim(conv, &em).await; | ||
| let msgs = result.messages(); | ||
|
|
||
| // MOIM gets inserted as separate message, trailing assistant removed | ||
| assert_eq!(msgs.len(), 4); | ||
|
|
||
| let tool_request_idx = msgs | ||
| .iter() | ||
| .position(|m| { | ||
| m.content.iter().any(|c| { | ||
| matches!( | ||
| c, | ||
| crate::conversation::message::MessageContent::ToolRequest(_) | ||
| ) | ||
| }) | ||
| }) | ||
| .unwrap(); | ||
|
|
||
| let tool_response_idx = msgs | ||
| .iter() | ||
| .position(|m| { | ||
| m.content.iter().any(|c| { | ||
| matches!( | ||
| c, | ||
| crate::conversation::message::MessageContent::ToolResponse(_) | ||
| ) | ||
| }) | ||
| }) | ||
| .unwrap(); | ||
|
|
||
| // MOIM should be in a separate message after tool response | ||
| let moim_msg = &msgs[3]; | ||
| let has_moim = moim_msg | ||
| .content | ||
| .iter() | ||
| .any(|c| c.as_text().map_or(false, |t| t.contains("<info-msg>"))); | ||
|
|
||
| assert!(has_moim, "MOIM should be in the last message"); | ||
| assert_eq!( | ||
| tool_response_idx, | ||
| tool_request_idx + 1, | ||
| "Tool response should immediately follow tool request" | ||
| ); | ||
| } | ||
tlongwell-block marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.