From 981253f9904c783853775ccf8020a04529a94012 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 11 Sep 2026 15:05:06 -0600 Subject: [PATCH] Collapse the run sandbox's lifecycle surface RunSandbox grew its lifecycle methods one adopter at a time and ended up with several names for each step. Reconnecting from a run record had four entry points (reconnect, reconnect_for_run, reconnect_for_run_with_events, reconnect_driver_for_run) that all forwarded to the last one. Bringing a sandbox back had two (start and activate) over the same make_ready, and releasing it had two (delete and cleanup) over the same release. Two more methods had no callers at all: set_autostop_interval, which nothing set after the driver took over lifecycle timers, and resume_setup_commands, which resume stopped using when checkout moved to the git facet. There is now one of each. reconnect_for_run takes the record, the provider access, an optional run id, and an optional event context; callers that need none pass None. activate is the single "make usable" step: a running sandbox only learns its platform when it has not yet, a stopped or paused one is started and its Bash verified, and resume calls it like every access-time caller. delete is the single release; for a designated host directory it frees the handle and leaves the directory in place, as cleanup did. The tests and server call sites follow the renames; behavior is unchanged except that resuming an already running sandbox no longer re-runs the Bash probe. Co-Authored-By: Claude Fable 5.1 --- lib/apps/fabro-server/src/run_files.rs | 2 +- lib/apps/fabro-server/src/run_manifest.rs | 4 +- lib/apps/fabro-server/src/server.rs | 2 +- .../src/server/handler/sandbox.rs | 4 +- .../src/server/handler/sessions.rs | 2 +- .../fabro-agent/tests/it/docker_shell.rs | 2 +- lib/components/fabro-sandbox/src/daytona.rs | 2 +- lib/components/fabro-sandbox/src/details.rs | 2 +- .../fabro-sandbox/src/driver_sandbox.rs | 57 ++++--------------- lib/components/fabro-sandbox/src/lib.rs | 5 +- lib/components/fabro-sandbox/src/reconnect.rs | 32 ++--------- .../tests/daytona_streaming_live.rs | 10 ++-- .../fabro-sandbox/tests/docker_streaming.rs | 12 ++-- .../fabro-sandbox/tests/driver_bench.rs | 2 +- .../fabro-workflow/src/handler/llm/acp.rs | 5 +- .../fabro-workflow/src/pipeline/initialize.rs | 8 +-- .../fabro-workflow/tests/it/cp_integration.rs | 14 ++--- .../tests/it/daytona_integration.rs | 42 +++++++------- .../fabro-workflow/tests/it/integration.rs | 2 +- 19 files changed, 74 insertions(+), 135 deletions(-) diff --git a/lib/apps/fabro-server/src/run_files.rs b/lib/apps/fabro-server/src/run_files.rs index d9680931e..2cd7f9d96 100644 --- a/lib/apps/fabro-server/src/run_files.rs +++ b/lib/apps/fabro-server/src/run_files.rs @@ -1181,7 +1181,7 @@ async fn reconnect_run_sandbox( .provider_access() .await .map_err(|err| ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()))?; - let sandbox = reconnect_for_run(&record, &access, Some(*run_id)) + let sandbox = reconnect_for_run(&record, &access, Some(*run_id), None) .await .map_err(|err| ApiError::new(StatusCode::CONFLICT, err.to_string()))?; sandbox diff --git a/lib/apps/fabro-server/src/run_manifest.rs b/lib/apps/fabro-server/src/run_manifest.rs index 581346a64..2fb76d177 100644 --- a/lib/apps/fabro-server/src/run_manifest.rs +++ b/lib/apps/fabro-server/src/run_manifest.rs @@ -1000,7 +1000,7 @@ async fn run_sandbox_check( warn: true, }); } - if let Err(err) = sandbox.cleanup().await { + if let Err(err) = sandbox.delete().await { checks.push(CheckResult { name: "Sandbox".into(), status: CheckStatus::Error, @@ -1020,7 +1020,7 @@ async fn run_sandbox_check( true } Err(err) => { - let cleanup_error = sandbox.cleanup().await.err(); + let cleanup_error = sandbox.delete().await.err(); checks.push(CheckResult { name: "Sandbox".into(), status: CheckStatus::Error, diff --git a/lib/apps/fabro-server/src/server.rs b/lib/apps/fabro-server/src/server.rs index a363135f0..4921a81bb 100644 --- a/lib/apps/fabro-server/src/server.rs +++ b/lib/apps/fabro-server/src/server.rs @@ -2775,7 +2775,7 @@ async fn delete_run_sandbox_resource( .provider_access() .await .map_err(|err| ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()))?; - let sandbox = match reconnect_for_run(&record, &access, Some(id)).await { + let sandbox = match reconnect_for_run(&record, &access, Some(id), None).await { Ok(sandbox) => sandbox, Err(err) if force || delete_started => { tracing::warn!( diff --git a/lib/apps/fabro-server/src/server/handler/sandbox.rs b/lib/apps/fabro-server/src/server/handler/sandbox.rs index 6e12be966..c01b9ae61 100644 --- a/lib/apps/fabro-server/src/server/handler/sandbox.rs +++ b/lib/apps/fabro-server/src/server/handler/sandbox.rs @@ -5,7 +5,7 @@ use std::time::Duration; use axum::extract::ws::{Message as WsMessage, WebSocket, WebSocketUpgrade}; use fabro_sandbox::{ - FileKind, ProviderAccess, PtySize, RunSandbox, open_terminal_for_run, reconnect_driver_for_run, + FileKind, ProviderAccess, PtySize, RunSandbox, open_terminal_for_run, reconnect_for_run, }; use fabro_types::{RunSandboxInstance, SandboxProviderKind}; use futures_util::FutureExt; @@ -735,7 +735,7 @@ async fn reconnect_run_sandbox_instance( record: &RunSandboxInstance, ) -> Result { let access = load_provider_access(state).await?; - let sandbox = reconnect_driver_for_run(record, &access, Some(*run_id), None) + let sandbox = reconnect_for_run(record, &access, Some(*run_id), None) .await .map_err(|err| { let detail = render_with_causes(&err.to_string(), &collect_causes(err.as_ref())); diff --git a/lib/apps/fabro-server/src/server/handler/sessions.rs b/lib/apps/fabro-server/src/server/handler/sessions.rs index 09e78b6af..3c57cf1ae 100644 --- a/lib/apps/fabro-server/src/server/handler/sessions.rs +++ b/lib/apps/fabro-server/src/server/handler/sessions.rs @@ -721,7 +721,7 @@ async fn build_agent_session( .provider_access() .await .map_err(|err| AskFabroBuildError::Agent(anyhow::Error::new(err)))?; - let sandbox = reconnect_for_run(sandbox_instance, &access, Some(run_id)) + let sandbox = reconnect_for_run(sandbox_instance, &access, Some(run_id), None) .await .map_err(AskFabroBuildError::SandboxUnavailable)?; sandbox diff --git a/lib/components/fabro-agent/tests/it/docker_shell.rs b/lib/components/fabro-agent/tests/it/docker_shell.rs index 7eb56508d..c92a91948 100644 --- a/lib/components/fabro-agent/tests/it/docker_shell.rs +++ b/lib/components/fabro-agent/tests/it/docker_shell.rs @@ -60,7 +60,7 @@ async fn shell_reports_real_docker_process_outcome() { ) .await; sandbox - .cleanup() + .delete() .await .expect("docker cleanup should succeed"); diff --git a/lib/components/fabro-sandbox/src/daytona.rs b/lib/components/fabro-sandbox/src/daytona.rs index aba6f31a6..952892a28 100644 --- a/lib/components/fabro-sandbox/src/daytona.rs +++ b/lib/components/fabro-sandbox/src/daytona.rs @@ -435,6 +435,6 @@ mod wire_gate { ); }; checks.await; - sandbox.cleanup().await.expect("cleanup"); + sandbox.delete().await.expect("cleanup"); } } diff --git a/lib/components/fabro-sandbox/src/details.rs b/lib/components/fabro-sandbox/src/details.rs index 28c0a40e4..da6682b2e 100644 --- a/lib/components/fabro-sandbox/src/details.rs +++ b/lib/components/fabro-sandbox/src/details.rs @@ -11,7 +11,7 @@ pub async fn sandbox_details( access: &ProviderAccess, run_id: Option, ) -> Result { - let sandbox = reconnect::reconnect_driver_for_run(record, access, run_id, None).await?; + let sandbox = reconnect::reconnect_for_run(record, access, run_id, None).await?; let status = sandbox.handle()?.describe().await.map_err(|err| { anyhow::anyhow!( "Failed to describe {} sandbox '{}': {err}", diff --git a/lib/components/fabro-sandbox/src/driver_sandbox.rs b/lib/components/fabro-sandbox/src/driver_sandbox.rs index 6cca51848..699826e5c 100644 --- a/lib/components/fabro-sandbox/src/driver_sandbox.rs +++ b/lib/components/fabro-sandbox/src/driver_sandbox.rs @@ -19,11 +19,10 @@ use std::time::{Duration, Instant}; use fabro_github::GitHubCredentials; use fabro_github::token_source::{InstallationTokenSource, TokenSnapshot}; use fabro_types::SandboxProviderKind; -use fabro_util::shell; use fabro_util::workspace_glob::WorkspaceGlob; use sandbox_driver::{ DirEntry, EventContext, ExecControls, ExecResult, ExecSpec, ExecStreamingResult, FileKind, - GitRetryPolicy, GrepMatch, GrepOptions, LifecycleTimers, PtyOptions, PtySession, PtySize, + GitRetryPolicy, GrepMatch, GrepOptions, PtyOptions, PtySession, PtySize, Sandbox as DriverHandle, SandboxProvider as DriverProvider, SandboxSource, SandboxSpec as DriverSpec, SandboxState, Search as _, StdioProcess, WaitOptions, WalkOptions, }; @@ -480,7 +479,7 @@ impl RunSandbox { } /// Bring the sandbox to `Running` with a verified Bash, and learn its - /// platform. Shared by initialize and start. + /// platform. Shared by initialize and activate. async fn make_ready(&self) -> crate::Result<()> { sandbox_driver::activate(self.handle()?.as_ref(), &WaitOptions::default()).await?; self.learn_platform().await @@ -883,32 +882,27 @@ impl RunSandbox { .and_then(|status| status.web_url) } - /// Idempotent access-time check: a running sandbox is left alone; a - /// stopped or paused one is brought back and its Bash verified. + /// Brings the sandbox back into use, idempotently: a running sandbox is + /// left alone and only its platform is learned when unknown; a stopped + /// or paused one is started and its Bash verified. Resume and every + /// access-time caller share this one entry point. pub async fn activate(&self) -> crate::Result<()> { let status = self.handle()?.describe().await?; if status.state == SandboxState::Running { - return Ok(()); + return self.learn_platform().await; } self.make_ready().await } - pub async fn start(&self) -> crate::Result<()> { - self.make_ready().await - } - pub async fn stop(&self) -> crate::Result<()> { self.handle()?.stop().await.map_err(crate::Error::from) } - pub async fn delete(&self) -> crate::Result<()> { - self.release().await - } - /// Releases the sandbox. For a designated host directory this frees the /// handle and leaves the directory in place; for an isolated provider it - /// removes the sandbox. - pub async fn cleanup(&self) -> crate::Result<()> { + /// removes the sandbox. A pending sandbox that was never created has + /// nothing to release. + pub async fn delete(&self) -> crate::Result<()> { self.release().await } @@ -964,22 +958,6 @@ impl RunSandbox { self.workspace.as_ref().and_then(RepoWorkspace::record) } - pub async fn set_autostop_interval(&self, minutes: i32) -> crate::Result<()> { - let mut timers = LifecycleTimers::default(); - timers.auto_stop_after_idle = u64::try_from(minutes) - .ok() - .filter(|minutes| *minutes > 0) - .map(Duration::from_mins); - match self.handle()?.set_timers(&timers).await { - // A provider without timers has nothing to stop automatically. - Ok(()) | Err(sandbox_driver::Error::Unsupported { .. }) => Ok(()), - Err(error) => Err(crate::Error::context( - "Failed to set sandbox auto-stop", - error, - )), - } - } - pub async fn setup_git(&self, intent: &GitSetupIntent) -> crate::Result> { if !self.repo_cloned() { return Ok(None); @@ -987,17 +965,6 @@ impl RunSandbox { sandbox::setup_git(self, intent).await.map(Some) } - pub fn resume_setup_commands(&self, run_branch: &str) -> Vec { - if !self.repo_cloned() { - return Vec::new(); - } - vec![format!( - "git fetch origin {} && git checkout {}", - shell::shell_quote(run_branch), - shell::shell_quote(run_branch) - )] - } - pub async fn git_push_ref( &self, refspec: &str, @@ -1388,7 +1355,7 @@ mod tests { sandbox.stop().await.unwrap(); sandbox.activate().await.unwrap(); - sandbox.cleanup().await.unwrap(); + sandbox.delete().await.unwrap(); assert!( dir.path().is_dir(), "designated directories survive cleanup" @@ -1440,7 +1407,7 @@ mod tests { Path::new(sandbox.working_directory()), workspace.canonicalize().unwrap() ); - sandbox.cleanup().await.unwrap(); + sandbox.delete().await.unwrap(); assert!(workspace.is_dir()); } diff --git a/lib/components/fabro-sandbox/src/lib.rs b/lib/components/fabro-sandbox/src/lib.rs index c971578bc..c935c02fe 100644 --- a/lib/components/fabro-sandbox/src/lib.rs +++ b/lib/components/fabro-sandbox/src/lib.rs @@ -50,10 +50,7 @@ pub use git_policy::{ }; pub use provider::{SandboxInventory, SandboxLookupError}; pub use provider_sandbox::{attach_provider_sandbox, provider_sandbox}; -pub use reconnect::{ - open_terminal_for_run, reconnect, reconnect_driver_for_run, reconnect_for_run, - reconnect_for_run_with_events, -}; +pub use reconnect::{open_terminal_for_run, reconnect_for_run}; pub use sandbox::{ DEFAULT_EXEC_OUTPUT_TAIL_BYTES, GitRunInfo, GitSetupIntent, PushAttempt, PushError, PushReport, SandboxFile, SandboxWorkspaceLayout, redacted_output_tail, setup_git, diff --git a/lib/components/fabro-sandbox/src/reconnect.rs b/lib/components/fabro-sandbox/src/reconnect.rs index a1b1c0175..cd5ef9f56 100644 --- a/lib/components/fabro-sandbox/src/reconnect.rs +++ b/lib/components/fabro-sandbox/src/reconnect.rs @@ -9,38 +9,16 @@ use crate::driver::ProviderAccess; use crate::driver_sandbox::RunSandbox; use crate::provider_sandbox; -/// Reconnect to a sandbox from a saved record. +/// Reconnect to a run's sandbox from its saved record. /// /// `access` carries the provider settings and vault credentials the record's -/// provider needs; the process environment is never consulted. -pub async fn reconnect(record: &RunSandboxInstance, access: &ProviderAccess) -> Result { - reconnect_for_run(record, access, None).await -} - +/// provider needs; the process environment is never consulted. `run_id` +/// narrows the ownership scope to the run when known, and the driver reports +/// the sandbox's lifecycle from here on through `events`. pub async fn reconnect_for_run( record: &RunSandboxInstance, access: &ProviderAccess, run_id: Option, -) -> Result { - reconnect_for_run_with_events(record, access, run_id, None).await -} - -pub async fn reconnect_for_run_with_events( - record: &RunSandboxInstance, - access: &ProviderAccess, - run_id: Option, - events: Option, -) -> Result { - reconnect_driver_for_run(record, access, run_id, events).await -} - -/// Reconnects as the driver-backed sandbox type, for callers that need a -/// driver facet fabro's [`Sandbox`](crate::Sandbox) trait does not carry -/// (VNC, signed previews, leased SSH). -pub async fn reconnect_driver_for_run( - record: &RunSandboxInstance, - access: &ProviderAccess, - run_id: Option, events: Option, ) -> Result { let runtime = &record.runtime; @@ -84,7 +62,7 @@ pub async fn open_terminal_for_run( run_id: Option, size: PtySize, ) -> crate::Result> { - let sandbox = reconnect_driver_for_run(record, access, run_id, None) + let sandbox = reconnect_for_run(record, access, run_id, None) .await .map_err(|err| crate::Error::context_anyhow("Failed to reconnect sandbox", err))?; sandbox.activate().await?; diff --git a/lib/components/fabro-sandbox/tests/daytona_streaming_live.rs b/lib/components/fabro-sandbox/tests/daytona_streaming_live.rs index 47fe1b109..2d05dce0f 100644 --- a/lib/components/fabro-sandbox/tests/daytona_streaming_live.rs +++ b/lib/components/fabro-sandbox/tests/daytona_streaming_live.rs @@ -43,7 +43,7 @@ mod daytona_streaming_live { sandbox.initialize().await?; let smoke_result = run_smoke(Arc::clone(&sandbox)).await; - let cleanup_result = sandbox.cleanup().await.context("clean up Daytona sandbox"); + let cleanup_result = sandbox.delete().await.context("clean up Daytona sandbox"); smoke_result?; cleanup_result?; @@ -144,7 +144,7 @@ mod daytona_streaming_live { } .await; - let cleanup_result = sandbox.cleanup().await.context("clean up Daytona sandbox"); + let cleanup_result = sandbox.delete().await.context("clean up Daytona sandbox"); checks?; cleanup_result?; @@ -182,7 +182,7 @@ mod daytona_streaming_live { .await .context("describe sandbox")? .labels; - let cleanup_result = sandbox.cleanup().await.context("clean up Daytona sandbox"); + let cleanup_result = sandbox.delete().await.context("clean up Daytona sandbox"); ensure_eq( &labels.get("sh.fabro.managed").map(String::as_str), @@ -246,7 +246,7 @@ mod daytona_streaming_live { None, ) .await?; - let cleanup_result = sandbox.cleanup().await.context("clean up Daytona sandbox"); + let cleanup_result = sandbox.delete().await.context("clean up Daytona sandbox"); ensure!( result.success(), @@ -292,7 +292,7 @@ mod daytona_streaming_live { sandbox.initialize().await?; let glob_result = run_glob_checks(&sandbox).await; - let cleanup_result = sandbox.cleanup().await.context("clean up Daytona sandbox"); + let cleanup_result = sandbox.delete().await.context("clean up Daytona sandbox"); glob_result?; cleanup_result?; diff --git a/lib/components/fabro-sandbox/tests/docker_streaming.rs b/lib/components/fabro-sandbox/tests/docker_streaming.rs index 37e196fb1..99b7d729c 100644 --- a/lib/components/fabro-sandbox/tests/docker_streaming.rs +++ b/lib/components/fabro-sandbox/tests/docker_streaming.rs @@ -92,7 +92,7 @@ async fn streaming_timeout_terminates_docker_exec_before_returning() { .await .expect("process probe should run"); sandbox - .cleanup() + .delete() .await .expect("docker cleanup should succeed"); @@ -144,7 +144,7 @@ async fn streaming_command_receives_exact_stdin_and_eof() { .expect("injection probe should run"); sandbox - .cleanup() + .delete() .await .expect("docker cleanup should succeed"); @@ -207,7 +207,7 @@ async fn cloned_docker_sandbox_uses_repos_checkout_and_workspace_symlink() { .await .expect("layout verification command should run"); sandbox - .cleanup() + .delete() .await .expect("docker cleanup should succeed"); @@ -289,7 +289,7 @@ async fn docker_runs_clean_bash_through_both_command_paths() { .expect("streaming command should run"); sandbox - .cleanup() + .delete() .await .expect("docker cleanup should succeed"); @@ -363,7 +363,7 @@ async fn docker_glob_matches_patterns_containing_a_path_separator() { let recursive = sandbox.glob("**/SKILL.md", Some("skills")).await; sandbox - .cleanup() + .delete() .await .expect("docker cleanup should succeed"); @@ -450,7 +450,7 @@ async fn docker_runtime_directory_is_private_and_outside_workspace() { let readback = sandbox.read_file_text(&blob_path).await; sandbox - .cleanup() + .delete() .await .expect("docker cleanup should succeed"); diff --git a/lib/components/fabro-sandbox/tests/driver_bench.rs b/lib/components/fabro-sandbox/tests/driver_bench.rs index 4dab31c44..dfb2771c5 100644 --- a/lib/components/fabro-sandbox/tests/driver_bench.rs +++ b/lib/components/fabro-sandbox/tests/driver_bench.rs @@ -374,7 +374,7 @@ async fn agent_tool_call_latency_through_the_driver() { fabro_docker.initialize().await.expect("fabro docker init"); unpack_fabro(&fabro_docker, &repo).await; rows.extend(bench_fabro("fabro Docker (driver-backed)", &fabro_docker, &repo).await); - fabro_docker.cleanup().await.expect("fabro docker cleanup"); + fabro_docker.delete().await.expect("fabro docker cleanup"); let docker_provider = Arc::new(DockerProvider::connect().await.expect("docker connect")); let container = docker_provider diff --git a/lib/components/fabro-workflow/src/handler/llm/acp.rs b/lib/components/fabro-workflow/src/handler/llm/acp.rs index a3ddb6503..f52194597 100644 --- a/lib/components/fabro-workflow/src/handler/llm/acp.rs +++ b/lib/components/fabro-workflow/src/handler/llm/acp.rs @@ -77,9 +77,8 @@ fn parse_refresh_enabled(raw: Option<&str>) -> bool { ) } -/// Parse the refresh-ahead loop interval. `None` disables the loop (explicit -/// `0`, mirroring the codebase's `set_autostop_interval` "0 to disable" -/// convention). Unset/empty or an unparsable value falls back to the default. +/// Parse the refresh-ahead loop interval. `None` disables the loop (an +/// explicit `0`). Unset/empty or an unparsable value falls back to the default. fn parse_refresh_interval(raw: Option<&str>) -> Option { match raw.map(str::trim) { None | Some("") => Some(REFRESH_INTERVAL_DEFAULT), diff --git a/lib/components/fabro-workflow/src/pipeline/initialize.rs b/lib/components/fabro-workflow/src/pipeline/initialize.rs index 055557c8f..b38940756 100644 --- a/lib/components/fabro-workflow/src/pipeline/initialize.rs +++ b/lib/components/fabro-workflow/src/pipeline/initialize.rs @@ -12,7 +12,7 @@ use fabro_llm::credentials::{CredentialProvider, readiness}; use fabro_llm::lithos_catalog::Catalog; use fabro_sandbox::{ DaytonaCredentials, ExecResultExt, GitSetupIntent, ProviderAccess, SandboxSpec, - reconnect_for_run_with_events, + reconnect_for_run, }; use fabro_static::EnvVars; use fabro_types::RunSandboxKind; @@ -433,7 +433,7 @@ pub async fn initialize( DaytonaCredentials::from_api_key(api_key.to_string(), process_env_var) }), }; - let sandbox = reconnect_for_run_with_events( + let sandbox = reconnect_for_run( &instance, &access, Some(options.run_options.run_id), @@ -460,10 +460,8 @@ pub async fn initialize( }); if attach_existing { - // Resume needs the full provider health check. `activate()` is the - // lighter access-time operation used after a run is already active. sandbox - .start() + .activate() .await .map_err(|e| Error::engine_with_source("Failed to start sandbox", e))?; } else { diff --git a/lib/components/fabro-workflow/tests/it/cp_integration.rs b/lib/components/fabro-workflow/tests/it/cp_integration.rs index ee879855c..df21ae5a3 100644 --- a/lib/components/fabro-workflow/tests/it/cp_integration.rs +++ b/lib/components/fabro-workflow/tests/it/cp_integration.rs @@ -14,7 +14,7 @@ reason = "This integration test stages sandbox fixtures with sync std::fs." )] -use fabro_sandbox::reconnect::reconnect; +use fabro_sandbox::reconnect::reconnect_for_run; use fabro_sandbox::{CloneRequest, ProviderAccess, provider_sandbox}; use fabro_types::{RunSandboxInstance, RunSandboxRuntime, SandboxProviderKind}; use sandbox_driver::{SandboxSource, SandboxSpec}; @@ -50,7 +50,7 @@ async fn local_cp_upload_download_round_trip() { let scratch = tempfile::tempdir().unwrap(); let record = local_record(sandbox_dir.path()); - let sandbox = reconnect(&record, &ProviderAccess::default()) + let sandbox = reconnect_for_run(&record, &ProviderAccess::default(), None, None) .await .expect("reconnect local"); @@ -83,7 +83,7 @@ async fn local_cp_binary_round_trip() { let scratch = tempfile::tempdir().unwrap(); let record = local_record(sandbox_dir.path()); - let sandbox = reconnect(&record, &ProviderAccess::default()) + let sandbox = reconnect_for_run(&record, &ProviderAccess::default(), None, None) .await .expect("reconnect local"); @@ -112,7 +112,7 @@ async fn local_cp_creates_parent_dirs() { let scratch = tempfile::tempdir().unwrap(); let record = local_record(sandbox_dir.path()); - let sandbox = reconnect(&record, &ProviderAccess::default()) + let sandbox = reconnect_for_run(&record, &ProviderAccess::default(), None, None) .await .expect("reconnect local"); @@ -237,7 +237,7 @@ async fn docker_cp_upload_download_round_trip() { let scratch = tempfile::tempdir().unwrap(); let record = docker_record(&container.id); - let sandbox = reconnect(&record, &ProviderAccess::default()) + let sandbox = reconnect_for_run(&record, &ProviderAccess::default(), None, None) .await .expect("reconnect docker"); @@ -268,7 +268,7 @@ async fn docker_cp_binary_round_trip() { let scratch = tempfile::tempdir().unwrap(); let record = docker_record(&container.id); - let sandbox = reconnect(&record, &ProviderAccess::default()) + let sandbox = reconnect_for_run(&record, &ProviderAccess::default(), None, None) .await .expect("reconnect docker"); @@ -297,7 +297,7 @@ async fn docker_cp_creates_parent_dirs() { let scratch = tempfile::tempdir().unwrap(); let record = docker_record(&container.id); - let sandbox = reconnect(&record, &ProviderAccess::default()) + let sandbox = reconnect_for_run(&record, &ProviderAccess::default(), None, None) .await .expect("reconnect docker"); diff --git a/lib/components/fabro-workflow/tests/it/daytona_integration.rs b/lib/components/fabro-workflow/tests/it/daytona_integration.rs index e6fd9a815..73fa4e3d1 100644 --- a/lib/components/fabro-workflow/tests/it/daytona_integration.rs +++ b/lib/components/fabro-workflow/tests/it/daytona_integration.rs @@ -278,7 +278,7 @@ async fn daytona_exec_command() { assert_eq!(result.exit_code, Some(0)); assert!(result.stdout_lossy().contains("hello")); - env.cleanup().await.unwrap(); + env.delete().await.unwrap(); } #[fabro_macros::e2e_test(live("DAYTONA_API_KEY"), live("GITHUB_APP_PRIVATE_KEY"))] @@ -294,7 +294,7 @@ async fn daytona_exec_command_with_pipe() { assert_eq!(result.exit_code, Some(0)); assert!(result.stdout_lossy().trim().contains('2')); - env.cleanup().await.unwrap(); + env.delete().await.unwrap(); } #[fabro_macros::e2e_test(live("DAYTONA_API_KEY"), live("GITHUB_APP_PRIVATE_KEY"))] @@ -325,7 +325,7 @@ async fn daytona_exec_command_cancelled() { )); assert_eq!(result.stderr_lossy(), "Command cancelled"); - env.cleanup().await.unwrap(); + env.delete().await.unwrap(); } #[fabro_macros::e2e_test(live("DAYTONA_API_KEY"), live("GITHUB_APP_PRIVATE_KEY"))] @@ -358,7 +358,7 @@ async fn daytona_exec_command_local_timeout() { assert_eq!(result.termination, fabro_sandbox::Termination::TimedOut); assert_eq!(result.stderr_lossy(), "Command timed out locally"); - env.cleanup().await.unwrap(); + env.delete().await.unwrap(); } #[fabro_macros::e2e_test(live("DAYTONA_API_KEY"), live("GITHUB_APP_PRIVATE_KEY"))] @@ -383,7 +383,7 @@ async fn daytona_file_round_trip() { env.delete_file(test_path).await.unwrap(); assert!(!env.file_exists(test_path).await.unwrap()); - env.cleanup().await.unwrap(); + env.delete().await.unwrap(); } #[fabro_macros::e2e_test(live("DAYTONA_API_KEY"), live("GITHUB_APP_PRIVATE_KEY"))] @@ -408,7 +408,7 @@ async fn daytona_full_lifecycle() { assert!(!entries.is_empty()); // Cleanup (deletes sandbox) - env.cleanup().await.unwrap(); + env.delete().await.unwrap(); } #[fabro_macros::e2e_test(live("DAYTONA_API_KEY"), live("GITHUB_APP_PRIVATE_KEY"))] @@ -446,7 +446,7 @@ async fn daytona_snapshot_sandbox() { assert_eq!(result.exit_code, Some(0)); assert!(result.stdout_lossy().contains("ripgrep")); - env.cleanup().await.unwrap(); + env.delete().await.unwrap(); } #[fabro_macros::e2e_test(live("DAYTONA_API_KEY"), live("GITHUB_APP_PRIVATE_KEY"))] @@ -498,7 +498,7 @@ async fn daytona_artifact_sync_uploads_and_rewrites_pointer() { remote_content.len() ); - env.cleanup().await.unwrap(); + env.delete().await.unwrap(); } // --------------------------------------------------------------------------- @@ -612,7 +612,7 @@ async fn daytona_pipeline_artifact_offload_and_sync() { "offloaded value should round-trip through the run store" ); - env.cleanup().await.unwrap(); + env.delete().await.unwrap(); } // --------------------------------------------------------------------------- @@ -823,7 +823,7 @@ async fn daytona_git_checkpoint_remote_emits_events() { "checkpoint should have git_commit_sha" ); - env.cleanup().await.unwrap(); + env.delete().await.unwrap(); } // --------------------------------------------------------------------------- @@ -971,7 +971,7 @@ async fn daytona_git_checkpoint_with_shadow_branch() { "sandbox commit should have Fabro-Run trailer, got:\n{commit_msg}" ); - env.cleanup().await.unwrap(); + env.delete().await.unwrap(); } // --------------------------------------------------------------------------- @@ -1104,7 +1104,7 @@ async fn daytona_asset_collection() { "artifact scratch cache should not be created" ); - env.cleanup().await.unwrap(); + env.delete().await.unwrap(); } #[fabro_macros::e2e_test(live("DAYTONA_API_KEY"), live("GITHUB_APP_PRIVATE_KEY"))] @@ -1123,7 +1123,7 @@ async fn daytona_ssh_access() { "ssh_command should contain 'ssh': {ssh_command}", ); - env.cleanup().await.unwrap(); + env.delete().await.unwrap(); } #[fabro_macros::e2e_test(live("DAYTONA_API_KEY"), live("GITHUB_APP_PRIVATE_KEY"))] @@ -1204,7 +1204,7 @@ async fn daytona_clone_private_repo_with_github_app_iat() { result.stdout_lossy().trim() ); - env.cleanup().await.unwrap(); + env.delete().await.unwrap(); } /// E2E: Verify that repos in an installed org get credentials (needed for @@ -1390,7 +1390,7 @@ async fn daytona_git_push_run_branch_to_origin() { } } - env.cleanup().await.unwrap(); + env.delete().await.unwrap(); } /// Diagnose toolbox proxy staleness after idle time. @@ -1528,7 +1528,7 @@ async fn daytona_toolbox_idle_diagnostic() { } eprintln!("\n=== PASS: all idle durations survived ==="); - env.cleanup().await.unwrap(); + env.delete().await.unwrap(); } /// E2E test for `fabro cp` against a live Daytona sandbox. @@ -1537,7 +1537,7 @@ async fn daytona_toolbox_idle_diagnostic() { /// uploads a file, downloads it back, and verifies the round-trip. #[fabro_macros::e2e_test(live("DAYTONA_API_KEY"), live("GITHUB_APP_PRIVATE_KEY"))] async fn daytona_cp_upload_download_round_trip() { - use fabro_sandbox::reconnect::reconnect; + use fabro_sandbox::reconnect::reconnect_for_run; use fabro_types::RunSandboxInstance; // 1. Create and initialize a real Daytona sandbox @@ -1574,7 +1574,7 @@ async fn daytona_cp_upload_download_round_trip() { daytona: Some(live_daytona_credentials()), ..ProviderAccess::default() }; - let reconnected = reconnect(&record, &access) + let reconnected = reconnect_for_run(&record, &access, None, None) .await .expect("reconnect should succeed"); @@ -1632,7 +1632,7 @@ async fn daytona_cp_upload_download_round_trip() { ); // 9. Cleanup - env.cleanup().await.unwrap(); + env.delete().await.unwrap(); } #[fabro_macros::e2e_test(live("DAYTONA_API_KEY"))] @@ -1772,7 +1772,7 @@ async fn daytona_computer_use_browser_screenshot() { ); // 7. Cleanup - env.cleanup().await.unwrap(); + env.delete().await.unwrap(); } #[fabro_macros::e2e_test(live("DAYTONA_API_KEY"))] @@ -2005,5 +2005,5 @@ async fn daytona_playwright_mcp_sandbox_transport() { } // 8. Cleanup - sandbox.cleanup().await.unwrap(); + sandbox.delete().await.unwrap(); } diff --git a/lib/components/fabro-workflow/tests/it/integration.rs b/lib/components/fabro-workflow/tests/it/integration.rs index 4d58fd7f1..f8cd0ac10 100644 --- a/lib/components/fabro-workflow/tests/it/integration.rs +++ b/lib/components/fabro-workflow/tests/it/integration.rs @@ -13727,7 +13727,7 @@ async fn asset_collection_docker_sandbox() { "artifact scratch cache should not be created" ); - sandbox.cleanup().await.unwrap(); + sandbox.delete().await.unwrap(); } #[tokio::test]