diff --git a/lib/components/fabro-sandbox/src/test_support.rs b/lib/components/fabro-sandbox/src/test_support.rs index 05c84b16e..56d611627 100644 --- a/lib/components/fabro-sandbox/src/test_support.rs +++ b/lib/components/fabro-sandbox/src/test_support.rs @@ -29,6 +29,10 @@ use crate::driver_sandbox::RunSandbox; use crate::managed_labels::{MANAGED_LABEL, MANAGED_LABEL_VALUE}; use crate::sandbox::SandboxFile; +mod deleted_on_drop; + +pub use deleted_on_drop::DeletedOnDrop; + /// The id a run record carries for a local sandbox at `working_directory`, /// as the Host provider derives it from the canonical path. A record a test /// writes by hand reconnects the way one fabro wrote would. The directory diff --git a/lib/components/fabro-sandbox/src/test_support/deleted_on_drop.rs b/lib/components/fabro-sandbox/src/test_support/deleted_on_drop.rs new file mode 100644 index 000000000..396ed957c --- /dev/null +++ b/lib/components/fabro-sandbox/src/test_support/deleted_on_drop.rs @@ -0,0 +1,281 @@ +//! A sandbox a test deletes even when it fails. +//! +//! A live test that creates a provider sandbox and deletes it on its last +//! line leaks a running (and billed) sandbox whenever it panics or fails an +//! assertion before that line. [`DeletedOnDrop`] owns the sandbox for the +//! test: the happy path still calls `delete` explicitly, and any other exit +//! deletes it from `Drop`. +//! +//! `Drop` is synchronous and may run while the test's runtime is unwinding +//! a panic, so the cleanup never uses that runtime: it spawns a thread with +//! a small runtime of its own and blocks until the delete finishes or a +//! bounded timeout passes. A live provider's handle cannot be driven from +//! that thread either, because its pooled HTTP connections are tasks on the +//! test's runtime, which nobody polls while it unwinds. The guard therefore +//! connects the provider afresh through the [`ProviderAccess`] the test +//! built the sandbox with and deletes the sandbox by id over that new +//! connection. + +use std::fmt; +use std::ops::Deref; +use std::sync::Arc; +use std::time::Duration; + +use fabro_types::SandboxProviderKind; +use tokio::runtime::Builder as RuntimeBuilder; +use tokio::time; + +use crate::driver::ProviderAccess; +use crate::driver_sandbox::RunSandbox; +use crate::error::display_for_log; +use crate::provider_sandbox; + +/// How long a drop-time delete may take before the guard gives up and +/// reports the sandbox as possibly leaked. Daytona's driver bounds each +/// delete call at 10s and may wait out a state change once; a reconnect +/// adds a few seconds of its own. +const DROP_DELETE_TIMEOUT: Duration = Duration::from_secs(90); + +/// A run sandbox that is deleted when the guard drops, unless the test +/// deleted it explicitly through [`DeletedOnDrop::delete`]. +/// +/// Derefs to the [`RunSandbox`] so a test reads the same as before; code +/// that needs a shared handle takes one from [`DeletedOnDrop::shared`]. +pub struct DeletedOnDrop { + sandbox: Arc, + /// Access for a fresh provider connection at drop time. `None` deletes + /// through the handle the sandbox already holds. + access: Option, + deleted: bool, +} + +impl DeletedOnDrop { + /// Guards a sandbox built through `access`, as every live provider test + /// builds one. A drop-time delete reconnects the provider through + /// `access` and deletes the sandbox by id. + pub fn new(sandbox: impl Into>, access: &ProviderAccess) -> Self { + Self { + sandbox: sandbox.into(), + access: Some(access.clone()), + deleted: false, + } + } + + /// Guards a sandbox whose own handle can finish a delete from any + /// thread: the scripted double, whose delete needs no live connection. + /// Not for a live provider, whose handle is bound to the test's runtime + /// (see the module docs). + pub fn through_handle(sandbox: impl Into>) -> Self { + Self { + sandbox: sandbox.into(), + access: None, + deleted: false, + } + } + + /// A shared handle to the sandbox for code that takes an `Arc`, such as + /// a workflow runner or an agent environment. The guard keeps its own + /// and still deletes the sandbox when it drops. + #[must_use] + pub fn shared(&self) -> Arc { + Arc::clone(&self.sandbox) + } + + /// Deletes the sandbox now, returning the driver's result. After a + /// successful delete the drop does nothing; after a failed one it tries + /// once more so a transient failure still leaves nothing behind. + pub async fn delete(mut self) -> crate::Result<()> { + let result = self.sandbox.delete().await; + self.deleted = result.is_ok(); + result + } +} + +impl Deref for DeletedOnDrop { + type Target = RunSandbox; + + fn deref(&self) -> &RunSandbox { + &self.sandbox + } +} + +impl fmt::Debug for DeletedOnDrop { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("DeletedOnDrop") + .field("kind", self.sandbox.kind()) + .field("id", &self.sandbox.sandbox_info()) + .field("deleted", &self.deleted) + .finish_non_exhaustive() + } +} + +impl Drop for DeletedOnDrop { + #[expect( + clippy::print_stderr, + reason = "The guard runs during a failing test; its report has to reach the captured test output." + )] + fn drop(&mut self) { + if self.deleted { + return; + } + let id = self.sandbox.sandbox_info(); + if id.is_empty() { + // Never created on the provider: nothing to delete. + return; + } + let kind = self.sandbox.kind().clone(); + eprintln!("DeletedOnDrop: deleting the {kind} sandbox {id} the test left behind"); + let outcome = delete_on_own_thread( + kind.clone(), + id.clone(), + Arc::clone(&self.sandbox), + self.access.clone(), + ); + match outcome { + Ok(()) => eprintln!("DeletedOnDrop: deleted the {kind} sandbox {id}"), + Err(error) => { + eprintln!("DeletedOnDrop: the {kind} sandbox {id} may be leaked: {error}"); + } + } + } +} + +/// Runs the delete to completion on a dedicated thread with its own +/// runtime, bounded by [`DROP_DELETE_TIMEOUT`]. +#[expect( + clippy::disallowed_methods, + reason = "Drop is synchronous and the test's runtime may be unwinding; the delete needs a thread and runtime of its own." +)] +fn delete_on_own_thread( + kind: SandboxProviderKind, + id: String, + sandbox: Arc, + access: Option, +) -> Result<(), String> { + let thread = std::thread::Builder::new() + .name("sandbox-delete-on-drop".to_string()) + .spawn(move || -> Result<(), String> { + let runtime = RuntimeBuilder::new_current_thread() + .enable_all() + .build() + .map_err(|error| format!("could not build a runtime for the delete: {error}"))?; + runtime.block_on(async { + time::timeout( + DROP_DELETE_TIMEOUT, + delete_afresh(&kind, &id, &sandbox, access.as_ref()), + ) + .await + .map_err(|_| { + format!( + "the delete did not finish within {}s", + DROP_DELETE_TIMEOUT.as_secs() + ) + })? + }) + }) + .map_err(|error| format!("could not spawn the delete thread: {error}"))?; + thread + .join() + .map_err(|_| "the delete thread panicked".to_string())? +} + +/// Deletes sandbox `id` over a fresh provider connection when `access` is +/// given, through the sandbox's own handle otherwise. +async fn delete_afresh( + kind: &SandboxProviderKind, + id: &str, + sandbox: &RunSandbox, + access: Option<&ProviderAccess>, +) -> Result<(), String> { + let Some(access) = access else { + return sandbox + .delete() + .await + .map_err(|error| display_for_log(&error)); + }; + let fresh = provider_sandbox::attach_provider_sandbox( + kind.clone(), + access, + id, + false, + sandbox.working_directory().to_string(), + None, + None, + None, + ) + .await + .map_err(|error| format!("could not reconnect: {}", display_for_log(&error)))?; + fresh + .delete() + .await + .map_err(|error| display_for_log(&error)) +} + +#[cfg(test)] +mod tests { + use std::panic::AssertUnwindSafe; + + use super::*; + use crate::test_support::MockSandbox; + + #[tokio::test] + async fn deletes_once_when_dropped_without_an_explicit_delete() { + let mock = MockSandbox::default(); + let guard = DeletedOnDrop::through_handle(mock.sandbox()); + assert_eq!( + guard.working_directory(), + "/work", + "reads through to the sandbox" + ); + assert_eq!(mock.driver().delete_count(), 0); + + drop(guard); + + assert_eq!(mock.driver().delete_count(), 1); + } + + #[tokio::test] + async fn an_explicit_delete_runs_once() { + let mock = MockSandbox::default(); + let guard = DeletedOnDrop::through_handle(mock.sandbox()); + let shared = guard.shared(); + + guard.delete().await.unwrap(); + + assert_eq!(mock.driver().delete_count(), 1); + drop(shared); + assert_eq!( + mock.driver().delete_count(), + 1, + "a shared handle does not delete" + ); + } + + #[test] + fn a_panic_before_the_delete_still_deletes_once() { + let mock = MockSandbox::default(); + let sandbox = mock.sandbox(); + + let outcome = std::panic::catch_unwind(AssertUnwindSafe(|| { + let _guard = DeletedOnDrop::through_handle(sandbox); + panic!("the test failed before its delete"); + })); + + assert!(outcome.is_err(), "the panic still propagates"); + assert_eq!(mock.driver().delete_count(), 1); + } + + #[tokio::test] + async fn a_panic_inside_a_runtime_still_deletes_once() { + let mock = MockSandbox::default(); + let sandbox = mock.sandbox(); + + let outcome = std::panic::catch_unwind(AssertUnwindSafe(|| { + let _guard = DeletedOnDrop::through_handle(sandbox); + panic!("the test failed before its delete"); + })); + + assert!(outcome.is_err(), "the panic still propagates"); + assert_eq!(mock.driver().delete_count(), 1); + } +} diff --git a/lib/components/fabro-workflow/tests/it/daytona_integration.rs b/lib/components/fabro-workflow/tests/it/daytona_integration.rs index ed9906ceb..7339dd6e6 100644 --- a/lib/components/fabro-workflow/tests/it/daytona_integration.rs +++ b/lib/components/fabro-workflow/tests/it/daytona_integration.rs @@ -23,6 +23,7 @@ use std::path::Path; use std::sync::Arc; use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node}; +use fabro_sandbox::test_support::DeletedOnDrop; use fabro_sandbox::{ CloneRequest, DaytonaCredentials, ProviderAccess, RunSandbox, SandboxProviderKind, provider_sandbox, @@ -197,7 +198,7 @@ fn live_daytona_credentials() -> DaytonaCredentials { DaytonaCredentials::from_api_key(api_key, |name| std::env::var(name).ok()) } -async fn create_env() -> RunSandbox { +async fn create_env() -> DeletedOnDrop { let creds = load_github_app_credentials(); create_env_with_github_app(Some(creds)).await } @@ -212,17 +213,19 @@ fn test_artifact_store(run_dir: &Path) -> ArtifactStore { async fn create_env_with_github_app( github_app: Option, -) -> RunSandbox { - provider_sandbox( +) -> DeletedOnDrop { + let access = daytona_access(live_daytona_credentials()); + let sandbox = provider_sandbox( SandboxProviderKind::DAYTONA, - &daytona_access(live_daytona_credentials()), + &access, SandboxSpec::new(SandboxSource::HostDirectory), &CloneRequest::default(), github_app.as_ref(), None, ) .await - .expect("Failed to create Daytona client — is DAYTONA_API_KEY set?") + .expect("Failed to create Daytona client — is DAYTONA_API_KEY set?"); + DeletedOnDrop::new(sandbox, &access) } fn load_github_app_credentials() -> fabro_github::GitHubCredentials { @@ -425,9 +428,10 @@ async fn daytona_snapshot_sandbox() { .timers(timers); let creds = load_github_app_credentials(); + let access = daytona_access(live_daytona_credentials()); let env = provider_sandbox( SandboxProviderKind::DAYTONA, - &daytona_access(live_daytona_credentials()), + &access, spec, &CloneRequest::default(), Some(&creds), @@ -435,6 +439,7 @@ async fn daytona_snapshot_sandbox() { ) .await .expect("Failed to create Daytona client — is DAYTONA_API_KEY set?"); + let env = DeletedOnDrop::new(env, &access); env.initialize().await.unwrap(); // Verify rg is available (installed by snapshot) @@ -532,7 +537,6 @@ impl Handler for LargeOutputHandler { async fn daytona_pipeline_artifact_offload_and_sync() { let env = create_env().await; env.initialize().await.unwrap(); - let env: Arc = Arc::new(env); // Pipeline: start -> big_output -> exit let mut graph = Graph::new("DaytonaArtifactPipeline"); @@ -570,7 +574,7 @@ async fn daytona_pipeline_artifact_offload_and_sync() { registry.register("start", Box::new(StartHandler)); registry.register("exit", Box::new(ExitHandler)); - let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), env.clone()); + let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), env.shared()); let run_options = RunOptions { settings: WorkflowSettings::default(), run_dir: dir.path().to_path_buf(), @@ -687,7 +691,6 @@ async fn setup_daytona_git(sandbox: &RunSandbox) -> (RunId, String, String) { async fn daytona_git_checkpoint_remote_emits_events() { let env = create_env().await; env.initialize().await.unwrap(); - let env: Arc = Arc::new(env); // Install git if not available (the default ubuntu:22.04 image may not have it) let git_check = env @@ -759,7 +762,7 @@ async fn daytona_git_checkpoint_remote_emits_events() { registry.register("start", Box::new(StartHandler)); registry.register("exit", Box::new(ExitHandler)); - let engine = WorkflowRunner::new(registry, Arc::new(emitter), env.clone()); + let engine = WorkflowRunner::new(registry, Arc::new(emitter), env.shared()); let run_options = RunOptions { settings: WorkflowSettings::default(), run_dir: dir.path().to_path_buf(), @@ -836,7 +839,6 @@ async fn daytona_git_checkpoint_remote_emits_events() { async fn daytona_git_checkpoint_without_metadata_branch() { let env = create_env().await; env.initialize().await.unwrap(); - let env: Arc = Arc::new(env); // Install git if not available let git_check = env @@ -901,7 +903,7 @@ async fn daytona_git_checkpoint_without_metadata_branch() { registry.register("start", Box::new(StartHandler)); registry.register("exit", Box::new(ExitHandler)); - let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), env.clone()); + let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), env.shared()); let run_options = RunOptions { settings: WorkflowSettings::default(), run_dir: dir.path().to_path_buf(), @@ -997,7 +999,6 @@ impl Handler for AssetCreatorHandler { async fn daytona_asset_collection() { let env = create_env().await; env.initialize().await.unwrap(); - let env: Arc = Arc::new(env); let dir = tempfile::tempdir().unwrap(); @@ -1005,7 +1006,7 @@ async fn daytona_asset_collection() { registry.register("start", Box::new(StartHandler)); registry.register("exit", Box::new(ExitHandler)); - let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), env.clone()); + let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), env.shared()); let mut graph = Graph::new("DaytonaAssetTest"); graph.attrs.insert( @@ -1256,7 +1257,6 @@ async fn daytona_git_push_run_branch_to_origin() { let creds = load_github_app_credentials(); let env = create_env_with_github_app(Some(creds)).await; env.initialize().await.unwrap(); - let env: Arc = Arc::new(env); // Install git if not available let git_check = env @@ -1319,7 +1319,7 @@ async fn daytona_git_push_run_branch_to_origin() { registry.register("start", Box::new(StartHandler)); registry.register("exit", Box::new(ExitHandler)); - let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), env.clone()); + let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), env.shared()); let run_options = RunOptions { settings: WorkflowSettings::default(), run_dir: dir.path().to_path_buf(), @@ -1623,9 +1623,10 @@ async fn daytona_cp_upload_download_round_trip() { #[fabro_macros::e2e_test(live("DAYTONA_API_KEY"))] async fn daytona_computer_use_browser_screenshot() { + let access = daytona_access(live_daytona_credentials()); let env = provider_sandbox( SandboxProviderKind::DAYTONA, - &daytona_access(live_daytona_credentials()), + &access, SandboxSpec::new(SandboxSource::HostDirectory), &CloneRequest::none(), None, @@ -1633,6 +1634,7 @@ async fn daytona_computer_use_browser_screenshot() { ) .await .expect("DAYTONA_API_KEY must be set"); + let env = DeletedOnDrop::new(env, &access); env.initialize().await.unwrap(); // 1. Start the computer use desktop environment (Xvfb, xfce4, etc.) through the @@ -1764,9 +1766,10 @@ async fn daytona_computer_use_browser_screenshot() { #[fabro_macros::e2e_test(live("DAYTONA_API_KEY"))] async fn daytona_playwright_mcp_sandbox_transport() { // Create sandbox from daytona-medium (has Node.js + Chromium) + let access = daytona_access(live_daytona_credentials()); let sandbox = provider_sandbox( SandboxProviderKind::DAYTONA, - &daytona_access(live_daytona_credentials()), + &access, SandboxSpec::new(SandboxSource::HostDirectory), &CloneRequest::none(), None, @@ -1774,6 +1777,7 @@ async fn daytona_playwright_mcp_sandbox_transport() { ) .await .expect("DAYTONA_API_KEY must be set"); + let sandbox = DeletedOnDrop::new(sandbox, &access); sandbox.initialize().await.unwrap(); // 1. Install Playwright MCP server and its browser @@ -1858,13 +1862,13 @@ async fn daytona_playwright_mcp_sandbox_transport() { ), ]), ); - let sandbox = Arc::new(sandbox); let routes = sandbox + .shared() .port_routes() .expect("Daytona forwards ports through preview URLs"); let mut agent = pebble_coding_agent::CodingAgent::builder( client, - Arc::clone(&sandbox) as Arc, + sandbox.shared() as Arc, ) .model("test/model") .permission_level(pebble_coding_agent::events::PermissionLevel::Full)