From 6dfeeca45e1feb21a96e71174d19656bcc86b795 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Tue, 4 Aug 2026 15:01:19 -0400 Subject: [PATCH] perf(agent): skip Daytona folder request for edits --- lib/components/fabro-agent/src/tools.rs | 4 +- .../fabro-sandbox/src/daytona/mod.rs | 84 ++++++++++++++++--- lib/components/fabro-sandbox/src/sandbox.rs | 10 +++ .../fabro-sandbox/src/test_support.rs | 14 +++- 4 files changed, 100 insertions(+), 12 deletions(-) diff --git a/lib/components/fabro-agent/src/tools.rs b/lib/components/fabro-agent/src/tools.rs index f892c971c..aad8e4e3d 100644 --- a/lib/components/fabro-agent/src/tools.rs +++ b/lib/components/fabro-agent/src/tools.rs @@ -226,7 +226,7 @@ pub fn make_edit_file_tool() -> RegisteredTool { }; ctx.env - .write_file(file_path, &new_content) + .write_existing_file(file_path, &new_content) .await .map_err(|e| e.display_with_causes())?; Ok(format!("Successfully edited {file_path}")) @@ -1002,6 +1002,7 @@ mod tests { ) .await; assert_eq!(result.unwrap(), "Successfully wrote to /out.txt"); + assert_eq!(env.existing_file_write_count(), 0); let written = env.written_files.lock().unwrap(); assert_eq!(written.len(), 1); assert_eq!(written[0].0, "/out.txt"); @@ -1036,6 +1037,7 @@ mod tests { ) .await; assert_eq!(result.unwrap(), "Successfully edited /f.txt"); + assert_eq!(env.existing_file_write_count(), 1); let written = env.written_files.lock().unwrap(); assert_eq!(written.len(), 1); assert_eq!(written[0].1, "goodbye world"); diff --git a/lib/components/fabro-sandbox/src/daytona/mod.rs b/lib/components/fabro-sandbox/src/daytona/mod.rs index 6371f23d2..72b0d2fff 100644 --- a/lib/components/fabro-sandbox/src/daytona/mod.rs +++ b/lib/components/fabro-sandbox/src/daytona/mod.rs @@ -511,6 +511,19 @@ impl DaytonaSandbox { resolve_path(path, self.working_directory()) } + async fn upload_file_content(&self, resolved_path: &str, content: &str) -> crate::Result<()> { + let sandbox = self.sandbox()?; + let fs_svc = sandbox + .fs() + .await + .map_err(|e| crate::Error::context("Failed to get fs service", e))?; + + fs_svc + .upload_file_bytes(resolved_path, content.as_bytes()) + .await + .map_err(|e| crate::Error::context(format!("Failed to write file {resolved_path}"), e)) + } + /// Verify a Daytona sandbox evaluates commands as non-login Bash. /// /// Runs on a freshly created sandbox before any Fabro-owned setup, and @@ -1613,17 +1626,12 @@ impl Sandbox for DaytonaSandbox { } } - let fs_svc = sandbox - .fs() - .await - .map_err(|e| crate::Error::context("Failed to get fs service", e))?; + self.upload_file_content(&resolved, content).await + } - fs_svc - .upload_file_bytes(&resolved, content.as_bytes()) - .await - .map_err(|e| crate::Error::context(format!("Failed to write file {resolved}"), e))?; - - Ok(()) + async fn write_existing_file(&self, path: &str, content: &str) -> crate::Result<()> { + let resolved = self.resolve_path(path); + self.upload_file_content(&resolved, content).await } async fn delete_file(&self, path: &str) -> crate::Result<()> { @@ -3432,6 +3440,62 @@ mod tests { delete.assert_async().await; } + #[tokio::test] + async fn write_existing_file_skips_parent_directory_creation() { + let server = MockServer::start_async().await; + let server_url = server.base_url(); + let sandbox_response = server + .mock_async(|when, then| { + when.method(GET).path("/sandbox/sandbox-edit"); + then.status(200) + .header("content-type", "application/json") + .json_body(sandbox_body("sandbox-edit", SandboxState::Started)); + }) + .await; + let toolbox_response = server + .mock_async(|when, then| { + when.method(GET) + .path("/sandbox/sandbox-edit/toolbox-proxy-url"); + then.status(200) + .header("content-type", "application/json") + .json_body(serde_json::json!({"url": server_url})); + }) + .await; + let folder = server + .mock_async(|when, then| { + when.method(POST).path("/sandbox-edit/files/folder"); + then.status(200); + }) + .await; + let upload = server + .mock_async(|when, then| { + when.method(POST) + .path("/sandbox-edit/files/upload") + .query_param("path", "/home/daytona/workspace/src/lib.rs") + .body_includes("updated contents"); + then.status(200); + }) + .await; + + let sandbox = mock_daytona_sandbox(&server, "dtn_test", DaytonaConfig::default()).await; + let sdk_sandbox = sandbox + .client + .get("sandbox-edit") + .await + .expect("get mock sandbox"); + assert!(sandbox.sandbox.set(sdk_sandbox).is_ok()); + + sandbox + .write_existing_file("src/lib.rs", "updated contents") + .await + .expect("write existing file"); + + sandbox_response.assert_async().await; + toolbox_response.assert_async().await; + upload.assert_async().await; + folder.assert_calls_async(0).await; + } + /// Recover the inner command a wrapper carries, proving it survives the /// base64 transport byte-for-byte. fn decode_wrapped_command(wrapped: &str) -> String { diff --git a/lib/components/fabro-sandbox/src/sandbox.rs b/lib/components/fabro-sandbox/src/sandbox.rs index 6e6e2b336..31a873300 100644 --- a/lib/components/fabro-sandbox/src/sandbox.rs +++ b/lib/components/fabro-sandbox/src/sandbox.rs @@ -1048,6 +1048,16 @@ pub trait Sandbox: Send + Sync { } async fn write_file(&self, path: &str, content: &str) -> crate::Result<()>; + + /// Write a file that the caller has already confirmed exists. + /// + /// Providers can override this method to skip setup that is only needed + /// when creating a new path. The default preserves the behavior of + /// [`Sandbox::write_file`]. + async fn write_existing_file(&self, path: &str, content: &str) -> crate::Result<()> { + self.write_file(path, content).await + } + async fn delete_file(&self, path: &str) -> crate::Result<()>; async fn file_exists(&self, path: &str) -> crate::Result; async fn list_directory( diff --git a/lib/components/fabro-sandbox/src/test_support.rs b/lib/components/fabro-sandbox/src/test_support.rs index 7364dc8b0..d25d9bbf4 100644 --- a/lib/components/fabro-sandbox/src/test_support.rs +++ b/lib/components/fabro-sandbox/src/test_support.rs @@ -1,6 +1,6 @@ use std::collections::HashMap; use std::sync::Mutex; -use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::time::Duration; use async_trait::async_trait; @@ -29,6 +29,8 @@ pub struct MockSandbox { pub os_version_str: String, /// Captures (path, content) pairs from `write_file` calls. pub written_files: Mutex>, + /// Counts calls to `write_existing_file`. + pub existing_file_writes: AtomicUsize, /// Captures the `timeout_ms` argument from `exec_command` calls. pub captured_timeout: Mutex>, /// Captures the `command` argument from `exec_command` calls (last only). @@ -104,6 +106,10 @@ impl MockSandbox { .expect("delete_calls lock poisoned") } + pub fn existing_file_write_count(&self) -> usize { + self.existing_file_writes.load(Ordering::Relaxed) + } + pub fn set_stdio_process(&self, process: MockStdioProcess) { *self .stdio_process @@ -156,6 +162,7 @@ impl Default for MockSandbox { platform_str: "darwin", os_version_str: "Darwin 24.0.0".into(), written_files: Mutex::new(Vec::new()), + existing_file_writes: AtomicUsize::new(0), captured_timeout: Mutex::new(None), captured_command: Mutex::new(None), captured_commands: Mutex::new(Vec::new()), @@ -250,6 +257,11 @@ impl Sandbox for MockSandbox { Ok(()) } + async fn write_existing_file(&self, path: &str, content: &str) -> crate::Result<()> { + self.existing_file_writes.fetch_add(1, Ordering::Relaxed); + self.write_file(path, content).await + } + async fn delete_file(&self, _path: &str) -> crate::Result<()> { Ok(()) }