From e4727e5031bb50a86f75d804f2a40ec6c9cd0b72 Mon Sep 17 00:00:00 2001 From: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> Date: Tue, 23 Sep 2025 02:36:19 +0000 Subject: [PATCH 1/2] test: add test for --- .../tests/tool_inspection_manager_tests.rs | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 crates/goose/tests/tool_inspection_manager_tests.rs diff --git a/crates/goose/tests/tool_inspection_manager_tests.rs b/crates/goose/tests/tool_inspection_manager_tests.rs new file mode 100644 index 000000000000..9524f70ccc1a --- /dev/null +++ b/crates/goose/tests/tool_inspection_manager_tests.rs @@ -0,0 +1,75 @@ +use goose::tool_inspection::{InspectionAction, InspectionResult, ToolInspectionManager, ToolInspector}; +use goose::conversation::message::{Message, ToolRequest}; +use anyhow::{anyhow, Result}; +use async_trait::async_trait; + +struct MockInspectorOk { + name: &'static str, + results: Vec, +} + +struct MockInspectorErr { + name: &'static str, +} + +#[async_trait] +impl ToolInspector for MockInspectorOk { + fn name(&self) -> &'static str { self.name } + fn as_any(&self) -> &dyn std::any::Any { self } + async fn inspect(&self, _tool_requests: &[ToolRequest], _messages: &[Message]) -> Result> { + Ok(self.results.clone()) + } +} + +#[async_trait] +impl ToolInspector for MockInspectorErr { + fn name(&self) -> &'static str { self.name } + fn as_any(&self) -> &dyn std::any::Any { self } + async fn inspect(&self, _tool_requests: &[ToolRequest], _messages: &[Message]) -> Result> { + Err(anyhow!("simulated failure")) + } +} + +#[tokio::test] +async fn test_inspect_tools_aggregates_and_handles_errors() { + // Arrange: create a manager with one successful and one failing inspector + let ok_results = vec![ + InspectionResult { + tool_request_id: "req_1".to_string(), + action: InspectionAction::Allow, + reason: "looks safe".to_string(), + confidence: 0.95, + inspector_name: "ok".to_string(), + finding_id: None, + }, + InspectionResult { + tool_request_id: "req_2".to_string(), + action: InspectionAction::RequireApproval(Some("double check".to_string())), + reason: "needs user confirmation".to_string(), + confidence: 0.7, + inspector_name: "ok".to_string(), + finding_id: Some("FND-123".to_string()), + }, + ]; + + let mut manager = ToolInspectionManager::new(); + manager.add_inspector(Box::new(MockInspectorOk { name: "ok", results: ok_results.clone() })); + manager.add_inspector(Box::new(MockInspectorErr { name: "err" })); + + // No specific input is required for this aggregation behavior + let tool_requests: Vec = vec![]; + let messages: Vec = vec![]; + + // Act + let results = manager.inspect_tools(&tool_requests, &messages).await.expect("inspect_tools should not fail when one inspector errors"); + + // Assert: results from the successful inspector are returned; failing inspector is ignored + assert_eq!(results.len(), 2, "Should aggregate results from successful inspectors only"); + // Also verify inspector_names() order/presence + let names = manager.inspector_names(); + assert_eq!(names, vec!["ok", "err"], "Inspector names should reflect registration order"); + + // Verify that specific actions are preserved + assert!(results.iter().any(|r| matches!(r.action, InspectionAction::Allow))); + assert!(results.iter().any(|r| matches!(r.action, InspectionAction::RequireApproval(_)))); +} From 04957fd7429427cf140c8862ba14967d2b2fa368 Mon Sep 17 00:00:00 2001 From: Tyler Longwell Date: Thu, 25 Sep 2025 11:13:47 -0400 Subject: [PATCH 2/2] cargo fmt --- .../tests/tool_inspection_manager_tests.rs | 64 +++++++++++++++---- 1 file changed, 50 insertions(+), 14 deletions(-) diff --git a/crates/goose/tests/tool_inspection_manager_tests.rs b/crates/goose/tests/tool_inspection_manager_tests.rs index 9524f70ccc1a..6701b1c3684f 100644 --- a/crates/goose/tests/tool_inspection_manager_tests.rs +++ b/crates/goose/tests/tool_inspection_manager_tests.rs @@ -1,7 +1,9 @@ -use goose::tool_inspection::{InspectionAction, InspectionResult, ToolInspectionManager, ToolInspector}; -use goose::conversation::message::{Message, ToolRequest}; use anyhow::{anyhow, Result}; use async_trait::async_trait; +use goose::conversation::message::{Message, ToolRequest}; +use goose::tool_inspection::{ + InspectionAction, InspectionResult, ToolInspectionManager, ToolInspector, +}; struct MockInspectorOk { name: &'static str, @@ -14,18 +16,34 @@ struct MockInspectorErr { #[async_trait] impl ToolInspector for MockInspectorOk { - fn name(&self) -> &'static str { self.name } - fn as_any(&self) -> &dyn std::any::Any { self } - async fn inspect(&self, _tool_requests: &[ToolRequest], _messages: &[Message]) -> Result> { + fn name(&self) -> &'static str { + self.name + } + fn as_any(&self) -> &dyn std::any::Any { + self + } + async fn inspect( + &self, + _tool_requests: &[ToolRequest], + _messages: &[Message], + ) -> Result> { Ok(self.results.clone()) } } #[async_trait] impl ToolInspector for MockInspectorErr { - fn name(&self) -> &'static str { self.name } - fn as_any(&self) -> &dyn std::any::Any { self } - async fn inspect(&self, _tool_requests: &[ToolRequest], _messages: &[Message]) -> Result> { + fn name(&self) -> &'static str { + self.name + } + fn as_any(&self) -> &dyn std::any::Any { + self + } + async fn inspect( + &self, + _tool_requests: &[ToolRequest], + _messages: &[Message], + ) -> Result> { Err(anyhow!("simulated failure")) } } @@ -53,7 +71,10 @@ async fn test_inspect_tools_aggregates_and_handles_errors() { ]; let mut manager = ToolInspectionManager::new(); - manager.add_inspector(Box::new(MockInspectorOk { name: "ok", results: ok_results.clone() })); + manager.add_inspector(Box::new(MockInspectorOk { + name: "ok", + results: ok_results.clone(), + })); manager.add_inspector(Box::new(MockInspectorErr { name: "err" })); // No specific input is required for this aggregation behavior @@ -61,15 +82,30 @@ async fn test_inspect_tools_aggregates_and_handles_errors() { let messages: Vec = vec![]; // Act - let results = manager.inspect_tools(&tool_requests, &messages).await.expect("inspect_tools should not fail when one inspector errors"); + let results = manager + .inspect_tools(&tool_requests, &messages) + .await + .expect("inspect_tools should not fail when one inspector errors"); // Assert: results from the successful inspector are returned; failing inspector is ignored - assert_eq!(results.len(), 2, "Should aggregate results from successful inspectors only"); + assert_eq!( + results.len(), + 2, + "Should aggregate results from successful inspectors only" + ); // Also verify inspector_names() order/presence let names = manager.inspector_names(); - assert_eq!(names, vec!["ok", "err"], "Inspector names should reflect registration order"); + assert_eq!( + names, + vec!["ok", "err"], + "Inspector names should reflect registration order" + ); // Verify that specific actions are preserved - assert!(results.iter().any(|r| matches!(r.action, InspectionAction::Allow))); - assert!(results.iter().any(|r| matches!(r.action, InspectionAction::RequireApproval(_)))); + assert!(results + .iter() + .any(|r| matches!(r.action, InspectionAction::Allow))); + assert!(results + .iter() + .any(|r| matches!(r.action, InspectionAction::RequireApproval(_)))); }