From 28bd9616a888f99555ca854bd2dd60864cf8bfc4 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sun, 8 Mar 2026 19:50:38 -0400 Subject: [PATCH] Add e2e tests for ReadBeforeWriteSandbox through execute_and_emit_one_tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tests the full production path: execute_and_emit_one_tool → real tool executor → ReadBeforeWriteSandbox guard, covering write-blocked, read-then-write, grep-then-write, edit-blocked, and new-file-allowed scenarios. Also updates MutableMockSandbox::grep to do substring matching against in-memory files so the grep integration test works end-to-end. Co-Authored-By: Claude Opus 4.6 --- crates/arc-agent/src/test_support.rs | 13 +- crates/arc-agent/src/tool_execution.rs | 202 +++++++++++++++++++++++++ 2 files changed, 213 insertions(+), 2 deletions(-) diff --git a/crates/arc-agent/src/test_support.rs b/crates/arc-agent/src/test_support.rs index cd2d5ff3d..d2b352002 100644 --- a/crates/arc-agent/src/test_support.rs +++ b/crates/arc-agent/src/test_support.rs @@ -308,11 +308,20 @@ impl Sandbox for MutableMockSandbox { async fn grep( &self, - _pattern: &str, + pattern: &str, _path: &str, _options: &GrepOptions, ) -> Result, String> { - Ok(vec![]) + let files = self.files.lock().expect("files lock poisoned"); + let mut results = Vec::new(); + for (path, content) in files.iter() { + for (i, line) in content.lines().enumerate() { + if line.contains(pattern) { + results.push(format!("{}:{}:{}", path, i + 1, line)); + } + } + } + Ok(results) } async fn glob(&self, _pattern: &str, _path: Option<&str>) -> Result, String> { diff --git a/crates/arc-agent/src/tool_execution.rs b/crates/arc-agent/src/tool_execution.rs index a4514aeda..8b2cdb87b 100644 --- a/crates/arc-agent/src/tool_execution.rs +++ b/crates/arc-agent/src/tool_execution.rs @@ -602,4 +602,206 @@ mod tests { let content = result.content.to_string(); assert!(content.contains("echo: hello")); } + + // --- ReadBeforeWriteSandbox e2e tests --- + + fn make_guarded_sandbox(files: HashMap) -> Arc { + Arc::new( + crate::read_before_write_sandbox::ReadBeforeWriteSandbox::new(Arc::new( + crate::test_support::MutableMockSandbox::new(files), + )), + ) + } + + #[tokio::test] + async fn write_to_unread_file_blocked() { + let mut registry = ToolRegistry::new(); + registry.register(crate::tools::make_write_file_tool()); + + let sandbox = make_guarded_sandbox(HashMap::from([("a.ts".into(), "content".into())])); + let tc = make_tool_call( + "write_file", + "call_1", + serde_json::json!({"file_path": "a.ts", "content": "new"}), + ); + let emitter = EventEmitter::new(); + let config = SessionConfig::default(); + + let result = execute_and_emit_one_tool( + &tc, + ®istry, + sandbox, + None, + CancellationToken::new(), + &config, + &emitter, + "test-session", + None, + ) + .await; + + assert!(result.is_error); + assert!(result.content.to_string().contains("has not been read")); + } + + #[tokio::test] + async fn read_then_write_succeeds() { + let mut registry = ToolRegistry::new(); + registry.register(crate::tools::make_read_file_tool()); + registry.register(crate::tools::make_write_file_tool()); + + let sandbox = make_guarded_sandbox(HashMap::from([("a.ts".into(), "content".into())])); + let emitter = EventEmitter::new(); + let config = SessionConfig::default(); + + // First read the file + let read_tc = make_tool_call( + "read_file", + "call_1", + serde_json::json!({"file_path": "a.ts"}), + ); + let read_result = execute_and_emit_one_tool( + &read_tc, + ®istry, + sandbox.clone(), + None, + CancellationToken::new(), + &config, + &emitter, + "test-session", + None, + ) + .await; + assert!(!read_result.is_error); + + // Then write should succeed + let write_tc = make_tool_call( + "write_file", + "call_2", + serde_json::json!({"file_path": "a.ts", "content": "new"}), + ); + let write_result = execute_and_emit_one_tool( + &write_tc, + ®istry, + sandbox, + None, + CancellationToken::new(), + &config, + &emitter, + "test-session", + None, + ) + .await; + + assert!(!write_result.is_error); + } + + #[tokio::test] + async fn grep_then_write_succeeds() { + let mut registry = ToolRegistry::new(); + registry.register(crate::tools::make_grep_tool()); + registry.register(crate::tools::make_write_file_tool()); + + let sandbox = make_guarded_sandbox(HashMap::from([("a.ts".into(), "content".into())])); + let emitter = EventEmitter::new(); + let config = SessionConfig::default(); + + // Grep matching a.ts + let grep_tc = make_tool_call("grep", "call_1", serde_json::json!({"pattern": "content"})); + let grep_result = execute_and_emit_one_tool( + &grep_tc, + ®istry, + sandbox.clone(), + None, + CancellationToken::new(), + &config, + &emitter, + "test-session", + None, + ) + .await; + assert!(!grep_result.is_error); + + // Then write should succeed + let write_tc = make_tool_call( + "write_file", + "call_2", + serde_json::json!({"file_path": "a.ts", "content": "new"}), + ); + let write_result = execute_and_emit_one_tool( + &write_tc, + ®istry, + sandbox, + None, + CancellationToken::new(), + &config, + &emitter, + "test-session", + None, + ) + .await; + + assert!(!write_result.is_error); + } + + #[tokio::test] + async fn edit_unread_file_blocked() { + let mut registry = ToolRegistry::new(); + registry.register(crate::tools::make_edit_file_tool()); + + let sandbox = make_guarded_sandbox(HashMap::from([("a.ts".into(), "content".into())])); + let tc = make_tool_call( + "edit_file", + "call_1", + serde_json::json!({"file_path": "a.ts", "old_string": "content", "new_string": "updated"}), + ); + let emitter = EventEmitter::new(); + let config = SessionConfig::default(); + + let result = execute_and_emit_one_tool( + &tc, + ®istry, + sandbox, + None, + CancellationToken::new(), + &config, + &emitter, + "test-session", + None, + ) + .await; + + assert!(result.is_error); + assert!(result.content.to_string().contains("has not been read")); + } + + #[tokio::test] + async fn write_new_file_succeeds() { + let mut registry = ToolRegistry::new(); + registry.register(crate::tools::make_write_file_tool()); + + let sandbox = make_guarded_sandbox(HashMap::new()); + let tc = make_tool_call( + "write_file", + "call_1", + serde_json::json!({"file_path": "new.ts", "content": "hello"}), + ); + let emitter = EventEmitter::new(); + let config = SessionConfig::default(); + + let result = execute_and_emit_one_tool( + &tc, + ®istry, + sandbox, + None, + CancellationToken::new(), + &config, + &emitter, + "test-session", + None, + ) + .await; + + assert!(!result.is_error); + } }