-
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
Merged
Merged
Changes from 18 commits
Commits
Show all changes
39 commits
Select commit
Hold shift + click to select a range
c22d3f8
Platform Extension MOIM (Minus One Info Message)
tlongwell-block c821645
update prompts
tlongwell-block fd6e6c4
remove todo read tool
tlongwell-block e9c48e2
cleanup comments. moim logic in separate file
tlongwell-block 5c28a48
comment
tlongwell-block 250b3c8
comment
tlongwell-block beb11cd
changes per review. Remove option to configure moim. Filter moim from…
tlongwell-block 0acdc97
cargo fmt
tlongwell-block 9ec9fe8
dont need serial in goose cli anymore
tlongwell-block 7813fb5
move moim to very end of history as a user message
tlongwell-block 7a444c0
clean up comments
tlongwell-block d1499c5
Merge main into moim2
tlongwell-block 599f61c
filter moim for scenario
tlongwell-block f85335d
lint
tlongwell-block ba2004e
Merge remote-tracking branch 'origin/main' into moim2
tlongwell-block 811da83
Merge remote-tracking branch 'origin/main' into moim2
tlongwell-block f099955
dont add moim immediately after tool calls
tlongwell-block 780e0ad
moim works better when it is behind the latest message
tlongwell-block b286cbd
simplify moim by simply prepending to last user role message
tlongwell-block d2f075f
Merge main into moim2 branch
tlongwell-block 07c0ce2
tests
tlongwell-block 528282a
Refactor MOIM injection to use Conversation type, remove redundant co…
tlongwell-block fae6e77
remove datetime from snapshots
tlongwell-block 0604895
moim config
tlongwell-block b7b00ae
scenario tests dont work with a dynamic timestamp
tlongwell-block 94f83e2
test minimization
tlongwell-block 4853b30
simplify injection logic
tlongwell-block 5f228ab
tests
tlongwell-block 5607fd5
Merge remote-tracking branch 'origin/main' into moim2
tlongwell-block b153859
moim disable function tested by scenario tests. dont need a dedicated…
tlongwell-block 3cdc5be
cleanup
tlongwell-block 7871fa0
comment cleanup
tlongwell-block 2eb9c81
Merge remote-tracking branch 'origin/main' into moim2
tlongwell-block 2774671
run fix_conversation before returning moim-enriched conversation. Sim…
tlongwell-block df80a59
thread local moim skip in scenario tests. No global/serial changes.
tlongwell-block a7344a1
simplify tests and insertion logic when no assistant message
tlongwell-block a954dc6
whitespace
tlongwell-block 282775e
fmt
tlongwell-block 29fc6f5
autonomous todo directive
tlongwell-block File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1107,6 +1107,32 @@ impl ExtensionManager { | |
| .get(&name.into()) | ||
| .map(|ext| ext.get_client()) | ||
| } | ||
|
|
||
| /// Collect and aggregate MOIM content from all platform extensions. | ||
tlongwell-block marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| pub async fn collect_moim(&self) -> Option<String> { | ||
| use chrono::Local; | ||
tlongwell-block marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| let timestamp = 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() { | ||
| // Only platform extensions can provide MOIM | ||
| 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)] | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,241 @@ | ||
| use crate::agents::extension_manager::ExtensionManager; | ||
| use crate::agents::SessionConfig; | ||
| use crate::conversation::message::{Message, MessageContent}; | ||
| use uuid::Uuid; | ||
|
|
||
| /// Inject MOIM (Minus One Info Message) into conversation. | ||
| /// | ||
| /// MOIM provides ephemeral context that's included in LLM calls | ||
| /// but never persisted to conversation history. | ||
tlongwell-block marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| pub async fn inject_moim( | ||
tlongwell-block marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| messages: &[Message], | ||
| extension_manager: &ExtensionManager, | ||
| _session: &Option<SessionConfig>, | ||
tlongwell-block marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| ) -> Vec<Message> { | ||
| let moim_content = match extension_manager.collect_moim().await { | ||
| Some(content) if !content.trim().is_empty() => content, | ||
| _ => { | ||
| tracing::debug!("No MOIM content available"); | ||
| return messages.to_vec(); | ||
| } | ||
| }; | ||
|
|
||
| tracing::debug!("Injecting MOIM: {} chars", moim_content.len()); | ||
|
|
||
| let moim_message = Message::user() | ||
| .with_text(moim_content) | ||
| .with_id(format!("moim_{}", Uuid::new_v4())) | ||
| .agent_only(); | ||
|
|
||
| let mut messages_with_moim = messages.to_vec(); | ||
|
|
||
| if messages_with_moim.is_empty() { | ||
| messages_with_moim.push(moim_message); | ||
| } else { | ||
| let insert_pos = find_moim_insertion_point(&messages_with_moim); | ||
| messages_with_moim.insert(insert_pos, moim_message); | ||
| } | ||
|
|
||
| messages_with_moim | ||
| } | ||
|
|
||
| /// Find a safe insertion point for MOIM that won't break tool call/response pairs. | ||
| fn find_moim_insertion_point(messages: &[Message]) -> usize { | ||
| if messages.is_empty() { | ||
| return 0; | ||
| } | ||
|
|
||
| let last_pos = messages.len() - 1; | ||
|
|
||
| // Don't break tool call/response pairs | ||
| if last_pos > 0 { | ||
| let prev_msg = &messages[last_pos - 1]; | ||
| let curr_msg = &messages[last_pos]; | ||
|
|
||
| let prev_has_tool_calls = prev_msg | ||
| .content | ||
| .iter() | ||
| .any(|c| matches!(c, MessageContent::ToolRequest(_))); | ||
|
|
||
| let curr_has_tool_responses = curr_msg | ||
| .content | ||
| .iter() | ||
| .any(|c| matches!(c, MessageContent::ToolResponse(_))); | ||
|
|
||
| if prev_has_tool_calls && curr_has_tool_responses { | ||
| tracing::debug!("MOIM: Adjusting position to avoid breaking tool pair"); | ||
| return last_pos.saturating_sub(1); | ||
| } | ||
| } | ||
tlongwell-block marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| last_pos | ||
| } | ||
tlongwell-block marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
| use crate::conversation::message::{ToolRequest, ToolResponse}; | ||
| use rmcp::model::{CallToolRequestParam, Content}; | ||
| use rmcp::object; | ||
|
|
||
| #[test] | ||
| fn test_find_insertion_point_edge_cases() { | ||
| // Test empty messages | ||
| let messages = vec![]; | ||
| assert_eq!(find_moim_insertion_point(&messages), 0); | ||
|
|
||
| // Test single message - should return 0 (insert at beginning) | ||
| let messages = vec![Message::user().with_text("Hello")]; | ||
| assert_eq!(find_moim_insertion_point(&messages), 0); | ||
|
|
||
| // Test multiple messages - should return last position | ||
| let messages = vec![ | ||
| Message::user().with_text("Hello"), | ||
| Message::assistant().with_text("Hi"), | ||
| Message::user().with_text("How are you?"), | ||
| ]; | ||
| assert_eq!(find_moim_insertion_point(&messages), 2); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_find_insertion_point_tool_pair_detection() { | ||
| // Helper to create tool request and response | ||
| let tool_request = ToolRequest { | ||
| id: "test_tool_1".to_string(), | ||
| tool_call: Ok(CallToolRequestParam { | ||
| name: "test_tool".into(), | ||
| arguments: Some(object!({"key": "value"})), | ||
| }), | ||
| }; | ||
|
|
||
| let tool_response = ToolResponse { | ||
| id: "test_tool_1".to_string(), | ||
| tool_result: Ok(vec![Content::text("Tool executed successfully")]), | ||
| }; | ||
|
|
||
| // Test: Tool call/response pair at the end - should back up | ||
| let messages = vec![ | ||
| Message::user().with_text("Please use a tool"), | ||
| Message::assistant() | ||
| .with_text("I'll use the tool now.") | ||
| .with_content(MessageContent::ToolRequest(tool_request.clone())), | ||
| Message::user().with_content(MessageContent::ToolResponse(tool_response.clone())), | ||
| ]; | ||
| assert_eq!(find_moim_insertion_point(&messages), 1); // Should back up to position 1 | ||
|
|
||
| // Test: Tool pair in the middle with more messages after | ||
| let messages_with_more = vec![ | ||
| Message::user().with_text("First request"), | ||
| Message::assistant().with_content(MessageContent::ToolRequest(tool_request.clone())), | ||
| Message::user().with_content(MessageContent::ToolResponse(tool_response)), | ||
| Message::assistant().with_text("Tool completed, here's the result"), | ||
| ]; | ||
| assert_eq!(find_moim_insertion_point(&messages_with_more), 3); // Should use last position | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_find_insertion_point_non_tool_pairs() { | ||
| let tool_request = ToolRequest { | ||
| id: "test_tool_1".to_string(), | ||
| tool_call: Ok(CallToolRequestParam { | ||
| name: "test_tool".into(), | ||
| arguments: Some(object!({"key": "value"})), | ||
| }), | ||
| }; | ||
|
|
||
| let tool_response = ToolResponse { | ||
| id: "test_tool_1".to_string(), | ||
| tool_result: Ok(vec![Content::text("Tool executed")]), | ||
| }; | ||
|
|
||
| // Test: Tool request without matching response | ||
| let messages = vec![ | ||
| Message::user().with_text("Please use a tool"), | ||
| Message::assistant().with_content(MessageContent::ToolRequest(tool_request)), | ||
| Message::assistant().with_text("Actually, let me reconsider."), | ||
| ]; | ||
| assert_eq!(find_moim_insertion_point(&messages), 2); // No pair, use last position | ||
|
|
||
| // Test: Tool response without preceding request | ||
| let messages = vec![ | ||
| Message::user().with_text("Here's a tool response from earlier"), | ||
| Message::assistant().with_text("Okay, I see that."), | ||
| Message::user().with_content(MessageContent::ToolResponse(tool_response)), | ||
| ]; | ||
| assert_eq!(find_moim_insertion_point(&messages), 2); // No pair, use last position | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn test_moim_injection_basic() { | ||
| let extension_manager = ExtensionManager::new(); | ||
|
|
||
| // Test empty conversation | ||
| let messages = vec![]; | ||
| let result = inject_moim(&messages, &extension_manager, &None).await; | ||
| assert_eq!(result.len(), 1); | ||
| assert!(result[0].id.as_ref().unwrap().starts_with("moim_")); | ||
|
|
||
| // Verify MOIM content and metadata | ||
| let content = result[0].content.first().and_then(|c| c.as_text()).unwrap(); | ||
| assert!(content.contains("<info-msg>")); | ||
| assert!(content.contains("Datetime:")); | ||
| assert!(!result[0].is_user_visible()); | ||
| assert!(result[0].is_agent_visible()); | ||
|
|
||
| // Test with existing messages | ||
| let messages = vec![ | ||
| Message::user().with_text("Hello"), | ||
| Message::assistant().with_text("Hi there"), | ||
| ]; | ||
| let result = inject_moim(&messages, &extension_manager, &None).await; | ||
|
|
||
| assert_eq!(result.len(), 3); | ||
| // MOIM should be at position 1 (before the last message) | ||
| assert!(result[1].id.as_ref().unwrap().starts_with("moim_")); | ||
| assert!(!result[1].is_user_visible()); | ||
| assert!(result[1].is_agent_visible()); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn test_moim_injection_preserves_tool_pair() { | ||
| let tool_request = ToolRequest { | ||
| id: "test_tool_1".to_string(), | ||
| tool_call: Ok(CallToolRequestParam { | ||
| name: "test_tool".into(), | ||
| arguments: Some(object!({"key": "value"})), | ||
| }), | ||
| }; | ||
|
|
||
| let tool_response = ToolResponse { | ||
| id: "test_tool_1".to_string(), | ||
| tool_result: Ok(vec![Content::text("Tool executed successfully")]), | ||
| }; | ||
|
|
||
| let messages = vec![ | ||
| Message::user().with_text("Please use a tool"), | ||
| Message::assistant() | ||
| .with_text("I'll use the tool now.") | ||
| .with_content(MessageContent::ToolRequest(tool_request)), | ||
| Message::user().with_content(MessageContent::ToolResponse(tool_response)), | ||
| ]; | ||
|
|
||
| let extension_manager = ExtensionManager::new(); | ||
| let result = inject_moim(&messages, &extension_manager, &None).await; | ||
|
|
||
| // Should have 4 messages total (3 original + 1 MOIM) | ||
| assert_eq!(result.len(), 4); | ||
|
|
||
| // MOIM should be at position 1 (before the tool request/response pair) | ||
| assert!(result[1].id.as_ref().unwrap().starts_with("moim_")); | ||
|
|
||
| // Verify the tool pair is still together (positions 2 and 3) | ||
| assert!(result[2] | ||
| .content | ||
| .iter() | ||
| .any(|c| matches!(c, MessageContent::ToolRequest(_)))); | ||
| assert!(result[3] | ||
| .content | ||
| .iter() | ||
| .any(|c| matches!(c, MessageContent::ToolResponse(_)))); | ||
| } | ||
tlongwell-block marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.