From 974fa5519f8a409661af5155620ee019ff4067fa Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Wed, 25 Mar 2026 12:12:32 -0400 Subject: [PATCH] refactor(sandbox): move SandboxRecord and sandbox_reconnect to fabro-sandbox MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These are purely sandbox concerns — they serialize/deserialize sandbox connection info and reconstruct sandbox instances. Moving them to fabro-sandbox improves cohesion and removes workflow-layer coupling from sandbox lifecycle logic. Co-Authored-By: Claude Opus 4.6 (1M context) --- Cargo.lock | 1 + lib/crates/fabro-cli/src/commands/cp.rs | 4 +-- lib/crates/fabro-cli/src/commands/diff.rs | 4 +-- lib/crates/fabro-cli/src/commands/inspect.rs | 2 +- lib/crates/fabro-cli/src/commands/preview.rs | 2 +- lib/crates/fabro-cli/src/commands/resume.rs | 2 +- lib/crates/fabro-cli/src/commands/run.rs | 2 +- lib/crates/fabro-cli/src/commands/runs.rs | 4 +-- lib/crates/fabro-cli/src/commands/shared.rs | 2 +- lib/crates/fabro-cli/src/commands/ssh.rs | 2 +- lib/crates/fabro-sandbox/Cargo.toml | 1 + lib/crates/fabro-sandbox/src/lib.rs | 6 ++++ .../src/reconnect.rs} | 32 ++++++++++--------- .../src/sandbox_record.rs} | 11 ++++--- lib/crates/fabro-workflows/src/lib.rs | 1 - lib/crates/fabro-workflows/src/records/mod.rs | 2 -- .../fabro-workflows/tests/cp_integration.rs | 4 +-- .../tests/daytona_integration.rs | 4 +-- 18 files changed, 48 insertions(+), 38 deletions(-) rename lib/crates/{fabro-workflows/src/sandbox_reconnect.rs => fabro-sandbox/src/reconnect.rs} (70%) rename lib/crates/{fabro-workflows/src/records/sandbox.rs => fabro-sandbox/src/sandbox_record.rs} (92%) diff --git a/Cargo.lock b/Cargo.lock index d3ff3e508..de7e21654 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1608,6 +1608,7 @@ dependencies = [ name = "fabro-sandbox" version = "0.176.2" dependencies = [ + "anyhow", "async-trait", "base64", "bollard", diff --git a/lib/crates/fabro-cli/src/commands/cp.rs b/lib/crates/fabro-cli/src/commands/cp.rs index cd18f8160..48e66c06d 100644 --- a/lib/crates/fabro-cli/src/commands/cp.rs +++ b/lib/crates/fabro-cli/src/commands/cp.rs @@ -105,12 +105,12 @@ async fn load_sandbox( let run_dir = fabro_workflows::run_lookup::resolve_run(base, run_prefix)?.path; let sandbox_json = run_dir.join("sandbox.json"); debug!(path = %sandbox_json.display(), "Loading sandbox record"); - let record = fabro_workflows::records::SandboxRecord::load(&sandbox_json).context( + let record = fabro_sandbox::SandboxRecord::load(&sandbox_json).context( "Failed to load sandbox.json — was this run started with a recent version of arc?", )?; info!(run_id = %run_prefix, provider = %record.provider, "Connecting to sandbox"); - fabro_workflows::sandbox_reconnect::reconnect(&record).await + fabro_sandbox::reconnect::reconnect(&record).await } async fn download_recursive( diff --git a/lib/crates/fabro-cli/src/commands/diff.rs b/lib/crates/fabro-cli/src/commands/diff.rs index e595872a0..4cba7a0d1 100644 --- a/lib/crates/fabro-cli/src/commands/diff.rs +++ b/lib/crates/fabro-cli/src/commands/diff.rs @@ -71,12 +71,12 @@ async fn resolve_diff(run_dir: &Path, args: &DiffArgs) -> Result { debug!("No final.patch found; attempting live diff from sandbox"); let sandbox_json = run_dir.join("sandbox.json"); - let record = fabro_workflows::records::SandboxRecord::load(&sandbox_json).context( + let record = fabro_sandbox::SandboxRecord::load(&sandbox_json).context( "Failed to load sandbox.json — was this run started with a recent version of arc?", )?; info!(provider = %record.provider, "Reconnecting to sandbox for live diff"); - let sandbox = fabro_workflows::sandbox_reconnect::reconnect(&record).await?; + let sandbox = fabro_sandbox::reconnect::reconnect(&record).await?; let cmd = build_live_diff_cmd(base_sha, args.stat, args.shortstat); debug!(cmd, "Running git diff in sandbox"); diff --git a/lib/crates/fabro-cli/src/commands/inspect.rs b/lib/crates/fabro-cli/src/commands/inspect.rs index 238d3ab9d..af1494e7e 100644 --- a/lib/crates/fabro-cli/src/commands/inspect.rs +++ b/lib/crates/fabro-cli/src/commands/inspect.rs @@ -48,7 +48,7 @@ fn inspect_run_dir( let checkpoint = fabro_workflows::records::Checkpoint::load(&run_dir.join("checkpoint.json")) .ok() .and_then(|v| serde_json::to_value(v).ok()); - let sandbox = fabro_workflows::records::SandboxRecord::load(&run_dir.join("sandbox.json")) + let sandbox = fabro_sandbox::SandboxRecord::load(&run_dir.join("sandbox.json")) .ok() .and_then(|v| serde_json::to_value(v).ok()); diff --git a/lib/crates/fabro-cli/src/commands/preview.rs b/lib/crates/fabro-cli/src/commands/preview.rs index 95747f3b6..9288adde8 100644 --- a/lib/crates/fabro-cli/src/commands/preview.rs +++ b/lib/crates/fabro-cli/src/commands/preview.rs @@ -31,7 +31,7 @@ pub async fn run(args: PreviewArgs) -> Result<()> { let base = fabro_workflows::run_lookup::default_runs_base(); let run_dir = fabro_workflows::run_lookup::resolve_run(&base, &args.run)?.path; let sandbox_json = run_dir.join("sandbox.json"); - let record = fabro_workflows::records::SandboxRecord::load(&sandbox_json).context( + let record = fabro_sandbox::SandboxRecord::load(&sandbox_json).context( "Failed to load sandbox.json — was this run started with a recent version of arc?", )?; diff --git a/lib/crates/fabro-cli/src/commands/resume.rs b/lib/crates/fabro-cli/src/commands/resume.rs index c9e21f44b..0be1768a8 100644 --- a/lib/crates/fabro-cli/src/commands/resume.rs +++ b/lib/crates/fabro-cli/src/commands/resume.rs @@ -1040,7 +1040,7 @@ async fn run_resumed( }; let is_docker = provider == SandboxProvider::Docker; - let record = fabro_workflows::records::SandboxRecord { + let record = fabro_sandbox::SandboxRecord { provider: provider.to_string(), working_directory: working_directory.clone(), identifier: sandbox_info_opt, diff --git a/lib/crates/fabro-cli/src/commands/run.rs b/lib/crates/fabro-cli/src/commands/run.rs index 79508351a..380a56e81 100644 --- a/lib/crates/fabro-cli/src/commands/run.rs +++ b/lib/crates/fabro-cli/src/commands/run.rs @@ -1406,7 +1406,7 @@ async fn run_command_impl( }); let is_docker = provider == SandboxProvider::Docker; - let record = fabro_workflows::records::SandboxRecord { + let record = fabro_sandbox::SandboxRecord { provider: provider.to_string(), working_directory: working_directory.clone(), identifier: sandbox_info_opt, diff --git a/lib/crates/fabro-cli/src/commands/runs.rs b/lib/crates/fabro-cli/src/commands/runs.rs index c8ef6bb8b..a5febe22b 100644 --- a/lib/crates/fabro-cli/src/commands/runs.rs +++ b/lib/crates/fabro-cli/src/commands/runs.rs @@ -539,9 +539,9 @@ async fn remove_from(args: &RunsRemoveArgs, base: &Path) -> Result<()> { ); let sandbox_path = run.path.join("sandbox.json"); - if let Ok(record) = fabro_workflows::records::SandboxRecord::load(&sandbox_path) { + if let Ok(record) = fabro_sandbox::SandboxRecord::load(&sandbox_path) { if record.provider != "local" { - match fabro_workflows::sandbox_reconnect::reconnect(&record).await { + match fabro_sandbox::reconnect::reconnect(&record).await { Ok(sandbox) => { if let Err(err) = sandbox.cleanup().await { warn!(run_id = %run.run_id, error = %err, "sandbox cleanup failed"); diff --git a/lib/crates/fabro-cli/src/commands/shared.rs b/lib/crates/fabro-cli/src/commands/shared.rs index 2fda35951..ed92a731f 100644 --- a/lib/crates/fabro-cli/src/commands/shared.rs +++ b/lib/crates/fabro-cli/src/commands/shared.rs @@ -85,7 +85,7 @@ pub fn split_run_path(s: &str) -> Option<(&str, &str)> { } pub fn validate_daytona_provider( - record: &fabro_workflows::records::SandboxRecord, + record: &fabro_sandbox::SandboxRecord, feature: &str, ) -> Result<()> { if record.provider != "daytona" { diff --git a/lib/crates/fabro-cli/src/commands/ssh.rs b/lib/crates/fabro-cli/src/commands/ssh.rs index 88763e210..f9ed907da 100644 --- a/lib/crates/fabro-cli/src/commands/ssh.rs +++ b/lib/crates/fabro-cli/src/commands/ssh.rs @@ -20,7 +20,7 @@ pub async fn run(args: SshArgs) -> Result<()> { let base = fabro_workflows::run_lookup::default_runs_base(); let run_dir = fabro_workflows::run_lookup::resolve_run(&base, &args.run)?.path; let sandbox_json = run_dir.join("sandbox.json"); - let record = fabro_workflows::records::SandboxRecord::load(&sandbox_json).context( + let record = fabro_sandbox::SandboxRecord::load(&sandbox_json).context( "Failed to load sandbox.json — was this run started with a recent version of arc?", )?; diff --git a/lib/crates/fabro-sandbox/Cargo.toml b/lib/crates/fabro-sandbox/Cargo.toml index 92727fbbd..7199e2d9d 100644 --- a/lib/crates/fabro-sandbox/Cargo.toml +++ b/lib/crates/fabro-sandbox/Cargo.toml @@ -19,6 +19,7 @@ test-support = [] doctest = false [dependencies] +anyhow.workspace = true async-trait.workspace = true tokio.workspace = true tokio-util.workspace = true diff --git a/lib/crates/fabro-sandbox/src/lib.rs b/lib/crates/fabro-sandbox/src/lib.rs index b0e414e20..65d4c7636 100644 --- a/lib/crates/fabro-sandbox/src/lib.rs +++ b/lib/crates/fabro-sandbox/src/lib.rs @@ -2,8 +2,12 @@ pub mod sandbox; pub mod read_guard; +pub mod reconnect; + pub mod sandbox_provider; +pub mod sandbox_record; + pub mod worktree; #[cfg(feature = "ssh")] @@ -46,3 +50,5 @@ pub use local::LocalSandbox; #[cfg(feature = "docker")] pub use docker::{DockerSandbox, DockerSandboxConfig}; + +pub use sandbox_record::SandboxRecord; diff --git a/lib/crates/fabro-workflows/src/sandbox_reconnect.rs b/lib/crates/fabro-sandbox/src/reconnect.rs similarity index 70% rename from lib/crates/fabro-workflows/src/sandbox_reconnect.rs rename to lib/crates/fabro-sandbox/src/reconnect.rs index e94b139b9..8eae4a7a9 100644 --- a/lib/crates/fabro-workflows/src/sandbox_reconnect.rs +++ b/lib/crates/fabro-sandbox/src/reconnect.rs @@ -2,19 +2,19 @@ use std::path::PathBuf; use anyhow::{bail, Context, Result}; -use crate::records::SandboxRecord; +use crate::sandbox_record::SandboxRecord; /// Reconnect to a sandbox from a saved record. /// /// Returns a sandbox that can perform file operations. -pub async fn reconnect(record: &SandboxRecord) -> Result> { +pub async fn reconnect(record: &SandboxRecord) -> Result> { match record.provider.as_str() { + #[cfg(feature = "local")] "local" => { - let sandbox = fabro_agent::local_sandbox::LocalSandbox::new(PathBuf::from( - &record.working_directory, - )); + let sandbox = crate::local::LocalSandbox::new(PathBuf::from(&record.working_directory)); Ok(Box::new(sandbox)) } + #[cfg(feature = "docker")] "docker" => { let host_dir = record .host_working_directory @@ -25,61 +25,63 @@ pub async fn reconnect(record: &SandboxRecord) -> Result { let name = record .identifier .as_deref() .context("Daytona sandbox record missing identifier (sandbox name)")?; - let sandbox = fabro_sandbox::daytona::DaytonaSandbox::reconnect(name) + let sandbox = crate::daytona::DaytonaSandbox::reconnect(name) .await .map_err(|e| anyhow::anyhow!("{e}"))?; Ok(Box::new(sandbox)) } - #[cfg(feature = "exedev")] + #[cfg(feature = "exe")] "exe" => { let data_host = record .data_host .as_deref() .context("Exe sandbox record missing data_host")?; - let data_ssh = fabro_sandbox::exe::OpensshRunner::connect(data_host) + let data_ssh = crate::exe::OpensshRunner::connect(data_host) .await .map_err(|e| { anyhow::anyhow!("Failed to connect to exe sandbox '{data_host}': {e}") })?; - let sandbox = fabro_sandbox::exe::ExeSandbox::from_existing(Box::new(data_ssh)); + let sandbox = crate::exe::ExeSandbox::from_existing(Box::new(data_ssh)); Ok(Box::new(sandbox)) } + #[cfg(feature = "ssh")] "ssh" => { let destination = record .data_host .as_deref() .context("SSH sandbox record missing data_host (destination)")?; - let ssh = fabro_sandbox::ssh::OpensshRunner::connect(destination, None) + let ssh = crate::ssh::OpensshRunner::connect(destination, None) .await .map_err(|e| { anyhow::anyhow!("Failed to connect to SSH sandbox '{destination}': {e}") })?; - let config = fabro_sandbox::ssh::SshConfig { + let config = crate::ssh::SshConfig { destination: destination.to_string(), working_directory: record.working_directory.clone(), config_file: None, preview_url_base: None, }; - let sandbox = fabro_sandbox::ssh::SshSandbox::from_existing(Box::new(ssh), config); + let sandbox = crate::ssh::SshSandbox::from_existing(Box::new(ssh), config); Ok(Box::new(sandbox)) } other => bail!("Unknown sandbox provider: {other}"), diff --git a/lib/crates/fabro-workflows/src/records/sandbox.rs b/lib/crates/fabro-sandbox/src/sandbox_record.rs similarity index 92% rename from lib/crates/fabro-workflows/src/records/sandbox.rs rename to lib/crates/fabro-sandbox/src/sandbox_record.rs index 167cf79f8..3b0c1ce13 100644 --- a/lib/crates/fabro-workflows/src/records/sandbox.rs +++ b/lib/crates/fabro-sandbox/src/sandbox_record.rs @@ -1,9 +1,8 @@ use std::path::Path; +use anyhow::{Context, Result}; use serde::{Deserialize, Serialize}; -use crate::error::Result; - #[derive(Debug, Clone, Serialize, Deserialize)] pub struct SandboxRecord { /// Provider type: "local", "docker", "daytona", "exe" @@ -26,11 +25,15 @@ pub struct SandboxRecord { impl SandboxRecord { pub fn save(&self, path: &Path) -> Result<()> { - crate::save_json(self, path, "sandbox_record") + let json = serde_json::to_string_pretty(self).context("sandbox_record serialize failed")?; + std::fs::write(path, json)?; + Ok(()) } pub fn load(path: &Path) -> Result { - crate::load_json(path, "sandbox_record") + let data = std::fs::read_to_string(path) + .with_context(|| format!("failed to read {}", path.display()))?; + serde_json::from_str(&data).context("sandbox_record deserialize failed") } } diff --git a/lib/crates/fabro-workflows/src/lib.rs b/lib/crates/fabro-workflows/src/lib.rs index 92029118a..06ccc88ab 100644 --- a/lib/crates/fabro-workflows/src/lib.rs +++ b/lib/crates/fabro-workflows/src/lib.rs @@ -113,7 +113,6 @@ pub mod run_lookup; pub mod run_settings; pub mod run_status; pub mod sandbox_git; -pub mod sandbox_reconnect; #[doc(hidden)] pub mod test_support; pub mod transforms; diff --git a/lib/crates/fabro-workflows/src/records/mod.rs b/lib/crates/fabro-workflows/src/records/mod.rs index 532cfa20d..43f2d1dfd 100644 --- a/lib/crates/fabro-workflows/src/records/mod.rs +++ b/lib/crates/fabro-workflows/src/records/mod.rs @@ -1,11 +1,9 @@ mod checkpoint; mod conclusion; mod run; -mod sandbox; mod start; pub use checkpoint::Checkpoint; pub use conclusion::{Conclusion, StageSummary}; pub use run::RunRecord; -pub use sandbox::SandboxRecord; pub use start::StartRecord; diff --git a/lib/crates/fabro-workflows/tests/cp_integration.rs b/lib/crates/fabro-workflows/tests/cp_integration.rs index c2ad205ca..c311e24b5 100644 --- a/lib/crates/fabro-workflows/tests/cp_integration.rs +++ b/lib/crates/fabro-workflows/tests/cp_integration.rs @@ -4,8 +4,8 @@ //! Docker tests require a Docker daemon and are marked `#[ignore]`. //! Run Docker tests with: `cargo test --package arc-workflows --test cp_integration -- --ignored` -use fabro_workflows::records::SandboxRecord; -use fabro_workflows::sandbox_reconnect::reconnect; +use fabro_sandbox::reconnect::reconnect; +use fabro_sandbox::SandboxRecord; // --------------------------------------------------------------------------- // Local sandbox diff --git a/lib/crates/fabro-workflows/tests/daytona_integration.rs b/lib/crates/fabro-workflows/tests/daytona_integration.rs index d57b92a46..7fde13a4d 100644 --- a/lib/crates/fabro-workflows/tests/daytona_integration.rs +++ b/lib/crates/fabro-workflows/tests/daytona_integration.rs @@ -1740,8 +1740,8 @@ async fn daytona_toolbox_idle_diagnostic() { #[tokio::test] #[ignore] async fn daytona_cp_upload_download_round_trip() { - use fabro_workflows::records::SandboxRecord; - use fabro_workflows::sandbox_reconnect::reconnect; + use fabro_sandbox::reconnect::reconnect; + use fabro_sandbox::SandboxRecord; // 1. Create and initialize a real Daytona sandbox let env = create_env().await;