From d97b82df653f6484c15a7f110b52086c25cc030c Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sat, 28 Mar 2026 16:34:27 -0400 Subject: [PATCH] Centralize sandbox variant handling --- lib/crates/fabro-api/src/server.rs | 5 +- .../fabro-cli/src/commands/preflight.rs | 106 ++---- lib/crates/fabro-sandbox/src/exe/mod.rs | 4 + lib/crates/fabro-sandbox/src/lib.rs | 5 + lib/crates/fabro-sandbox/src/sandbox.rs | 10 + .../fabro-sandbox/src/sandbox_provider.rs | 8 +- lib/crates/fabro-sandbox/src/sandbox_spec.rs | 281 ++++++++++++++ lib/crates/fabro-sandbox/src/ssh/mod.rs | 4 + lib/crates/fabro-sandbox/src/ssh_common.rs | 14 + lib/crates/fabro-sandbox/src/worktree.rs | 4 + .../fabro-workflows/src/operations/mod.rs | 3 +- .../fabro-workflows/src/operations/start.rs | 48 +-- .../src/pipeline/execute/tests.rs | 3 +- .../src/pipeline/initialize.rs | 360 ++++-------------- .../fabro-workflows/src/pipeline/mod.rs | 2 +- .../fabro-workflows/src/pipeline/types.rs | 36 +- 16 files changed, 463 insertions(+), 430 deletions(-) create mode 100644 lib/crates/fabro-sandbox/src/sandbox_spec.rs diff --git a/lib/crates/fabro-api/src/server.rs b/lib/crates/fabro-api/src/server.rs index e1dfbb9c8..a9ce0e287 100644 --- a/lib/crates/fabro-api/src/server.rs +++ b/lib/crates/fabro-api/src/server.rs @@ -40,12 +40,11 @@ use crate::sessions as sessions_mod; use crate::sessions::{SessionStore, new_session_store}; use fabro_interview::{Answer, Interviewer, QuestionType, WebInterviewer}; use fabro_retro::RetroExt; +use fabro_sandbox::SandboxSpec; use fabro_workflows::context::Context; use fabro_workflows::event::{EventEmitter, WorkflowRunEvent}; use fabro_workflows::operations::{self, CreateRunInput, WorkflowInput}; -use fabro_workflows::pipeline::{ - self, InitOptions, LlmSpec, Persisted, SandboxEnvSpec, SandboxSpec, -}; +use fabro_workflows::pipeline::{self, InitOptions, LlmSpec, Persisted, SandboxEnvSpec}; use fabro_workflows::records::{Checkpoint, CheckpointExt}; use fabro_workflows::run_options::LifecycleOptions; use fabro_workflows::run_options::RunOptions; diff --git a/lib/crates/fabro-cli/src/commands/preflight.rs b/lib/crates/fabro-cli/src/commands/preflight.rs index bbacadbaa..d3e3c9b88 100644 --- a/lib/crates/fabro-cli/src/commands/preflight.rs +++ b/lib/crates/fabro-cli/src/commands/preflight.rs @@ -2,7 +2,6 @@ use std::path::Path; use std::sync::Arc; use anyhow::bail; -use fabro_agent::{DockerSandbox, DockerSandboxConfig, LocalSandbox, Sandbox}; use fabro_config::cli::load_cli_config; use fabro_config::project::{ ResolveSettingsInput, resolve_settings, resolve_workflow_path, resolve_working_directory, @@ -11,9 +10,11 @@ use fabro_config::{FabroConfig, FabroSettings}; use fabro_graphviz::graph::{Graph, is_llm_handler_type}; use fabro_llm::client::Client as LlmClient; use fabro_model::{Catalog, Provider}; -use fabro_sandbox::SandboxProvider; -use fabro_sandbox::daytona::{DaytonaConfig, DaytonaSandbox, detect_repo_info}; -use fabro_sandbox::ssh::{GitCloneParams as SshGitCloneParams, SshConfig, SshSandbox}; +use fabro_sandbox::daytona::{DaytonaConfig, detect_repo_info}; +use fabro_sandbox::ssh::SshConfig; +use fabro_sandbox::{ + DockerSandboxConfig, Sandbox, SandboxProvider, SandboxSpec, detect_clone_params, +}; use fabro_util::terminal::Styles; use fabro_workflows::git::{GitSyncStatus, sync_status}; use fabro_workflows::operations::{ValidateInput, WorkflowInput, validate}; @@ -150,37 +151,12 @@ fn resolve_exe_config(settings: &FabroSettings) -> Option Option { - let (detected_url, branch) = match detect_repo_info(cwd) { - Ok(info) => info, - Err(err) => { - tracing::warn!("No git repo detected for exe.dev clone: {err}"); - return None; - } - }; - let url = fabro_github::ssh_url_to_https(&detected_url); - Some(fabro_sandbox::exe::GitCloneParams { url, branch }) -} - fn resolve_ssh_config(settings: &FabroSettings) -> Option { settings .sandbox_settings() .and_then(|sandbox| sandbox.ssh.clone()) } -fn resolve_ssh_clone_params(cwd: &Path) -> Option { - let (detected_url, branch) = match detect_repo_info(cwd) { - Ok(info) => info, - Err(err) => { - tracing::warn!("No git repo detected for SSH clone: {err}"); - return None; - } - }; - let url = fabro_github::ssh_url_to_https(&detected_url); - Some(SshGitCloneParams { url, branch }) -} - async fn mint_github_token( creds: &fabro_github::GitHubAppCredentials, origin_url: &str, @@ -279,53 +255,55 @@ async fn run_preflight( let ssh_config = resolve_ssh_config(settings); let sandbox_result: Result, String> = match sandbox_provider { - SandboxProvider::Docker => { - let config = DockerSandboxConfig { + SandboxProvider::Local => SandboxSpec::Local { + working_directory: working_directory.to_path_buf(), + } + .build(None) + .await + .map_err(|e| e.to_string()), + SandboxProvider::Docker => SandboxSpec::Docker { + config: DockerSandboxConfig { host_working_directory: working_directory.to_string_lossy().to_string(), ..DockerSandboxConfig::default() - }; - DockerSandbox::new(config) - .map(|env| Arc::new(env) as Arc) - .map_err(|e| format!("Docker sandbox creation failed: {e}")) + }, } - SandboxProvider::Daytona => { - let config = daytona_config.unwrap_or_default(); - match DaytonaSandbox::new(config, github_app.clone(), None, None).await { - Ok(env) => Ok(Arc::new(env) as Arc), - Err(e) => Err(format!("Daytona sandbox creation failed: {e}")), - } + .build(None) + .await + .map_err(|e| e.to_string()), + SandboxProvider::Daytona => SandboxSpec::Daytona { + config: daytona_config.unwrap_or_default(), + github_app: github_app.clone(), + run_id: None, + clone_branch: None, } + .build(None) + .await + .map_err(|e| format!("Daytona sandbox creation failed: {e}")), #[cfg(feature = "exedev")] - SandboxProvider::Exe => { - match fabro_sandbox::exe::OpensshRunner::connect_raw("exe.dev").await { - Ok(mgmt_ssh) => { - let config = exe_config.unwrap_or_default(); - let clone_params = resolve_exe_clone_params(working_directory); - let env = fabro_sandbox::exe::ExeSandbox::new( - Box::new(mgmt_ssh), - config, - clone_params, - None, - None, - ); - Ok(Arc::new(env) as Arc) - } - Err(e) => Err(format!("exe.dev SSH connection failed: {e}")), - } + SandboxProvider::Exe => SandboxSpec::Exe { + config: exe_config.unwrap_or_default(), + clone_params: detect_clone_params(working_directory), + run_id: None, + github_app: None, + mgmt_destination: "exe.dev".to_string(), } + .build(None) + .await + .map_err(|e| format!("exe sandbox creation failed: {e}")), #[cfg(not(feature = "exedev"))] SandboxProvider::Exe => Err("exe sandbox requires the exedev feature".to_string()), SandboxProvider::Ssh => match ssh_config { - Some(config) => { - let clone_params = resolve_ssh_clone_params(working_directory); - let env = SshSandbox::new(config, clone_params, None, None); - Ok(Arc::new(env) as Arc) + Some(config) => SandboxSpec::Ssh { + config, + clone_params: detect_clone_params(working_directory), + run_id: None, + github_app: None, } + .build(None) + .await + .map_err(|e| e.to_string()), None => Err("SSH sandbox requires [sandbox.ssh] config".to_string()), }, - SandboxProvider::Local => { - Ok(Arc::new(LocalSandbox::new(working_directory.to_path_buf())) as Arc) - } }; let sandbox_ok = match sandbox_result { diff --git a/lib/crates/fabro-sandbox/src/exe/mod.rs b/lib/crates/fabro-sandbox/src/exe/mod.rs index 6abd6c8c5..51dd5631b 100644 --- a/lib/crates/fabro-sandbox/src/exe/mod.rs +++ b/lib/crates/fabro-sandbox/src/exe/mod.rs @@ -722,6 +722,10 @@ impl Sandbox for ExeSandbox { self.ssh_command().map(Some) } + fn data_host(&self) -> Option<&str> { + self.data_host.get().map(String::as_str) + } + fn origin_url(&self) -> Option<&str> { self.origin_url.get().map(String::as_str) } diff --git a/lib/crates/fabro-sandbox/src/lib.rs b/lib/crates/fabro-sandbox/src/lib.rs index 1e83d9d34..e6bef219b 100644 --- a/lib/crates/fabro-sandbox/src/lib.rs +++ b/lib/crates/fabro-sandbox/src/lib.rs @@ -1,4 +1,5 @@ pub mod sandbox; +pub mod sandbox_spec; pub mod read_guard; @@ -38,6 +39,7 @@ pub use sandbox::{ DirEntry, ExecResult, GitRunInfo, GrepOptions, Sandbox, SandboxEvent, SandboxEventCallback, format_lines_numbered, git_push_via_exec, setup_git_via_exec, shell_quote, }; +pub use sandbox_spec::{SandboxSpec, WorkdirStrategy}; pub use read_guard::ReadBeforeWriteSandbox; @@ -52,3 +54,6 @@ pub use local::LocalSandbox; pub use docker::{DockerSandbox, DockerSandboxConfig}; pub use sandbox_record::{SandboxRecord, SandboxRecordExt}; + +#[cfg(all(feature = "ssh", feature = "daytona"))] +pub use ssh_common::detect_clone_params; diff --git a/lib/crates/fabro-sandbox/src/sandbox.rs b/lib/crates/fabro-sandbox/src/sandbox.rs index 915a116ae..fb8aa9299 100644 --- a/lib/crates/fabro-sandbox/src/sandbox.rs +++ b/lib/crates/fabro-sandbox/src/sandbox.rs @@ -136,6 +136,10 @@ macro_rules! delegate_sandbox { self.$field.host_git_dir() } + fn data_host(&self) -> Option<&str> { + self.$field.data_host() + } + fn parallel_worktree_path( &self, run_dir: &std::path::Path, @@ -457,6 +461,12 @@ pub trait Sandbox: Send + Sync { None } + /// The remote host for reconnection (e.g. SSH destination, exe.dev data plane). + /// Default is None; Exe and Ssh override. + fn data_host(&self) -> Option<&str> { + None + } + /// Compute the filesystem path for a parallel branch worktree. fn parallel_worktree_path( &self, diff --git a/lib/crates/fabro-sandbox/src/sandbox_provider.rs b/lib/crates/fabro-sandbox/src/sandbox_provider.rs index 6e3ae6169..215a9a167 100644 --- a/lib/crates/fabro-sandbox/src/sandbox_provider.rs +++ b/lib/crates/fabro-sandbox/src/sandbox_provider.rs @@ -17,7 +17,13 @@ pub enum SandboxProvider { Ssh, } -impl SandboxProvider {} +impl SandboxProvider { + /// True only for Local. Used by dry-run to force local execution. + /// NOT the same as "runs on the host" (Docker is host-adjacent but not dry-run compatible). + pub fn is_local(&self) -> bool { + matches!(self, Self::Local) + } +} impl fmt::Display for SandboxProvider { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { diff --git a/lib/crates/fabro-sandbox/src/sandbox_spec.rs b/lib/crates/fabro-sandbox/src/sandbox_spec.rs new file mode 100644 index 000000000..f47776abf --- /dev/null +++ b/lib/crates/fabro-sandbox/src/sandbox_spec.rs @@ -0,0 +1,281 @@ +use std::path::PathBuf; +use std::sync::Arc; + +use anyhow::anyhow; +use fabro_types::settings::WorktreeMode; + +use crate::sandbox_record::SandboxRecord; +use crate::{Sandbox, SandboxEventCallback}; + +#[cfg(feature = "daytona")] +use crate::daytona::{DaytonaConfig, DaytonaSandbox, DaytonaSnapshotConfig}; +#[cfg(feature = "docker")] +use crate::docker::{DockerSandbox, DockerSandboxConfig}; +#[cfg(feature = "exe")] +use crate::exe::{ExeConfig, ExeSandbox, GitCloneParams as ExeGitCloneParams, OpensshRunner}; +use crate::local::LocalSandbox; +#[cfg(feature = "ssh")] +use crate::ssh::{GitCloneParams as SshGitCloneParams, SshConfig, SshSandbox}; + +#[cfg(any(feature = "daytona", feature = "exe", feature = "ssh"))] +use fabro_github::GitHubAppCredentials; + +/// Options for sandbox initialization and construction. +pub enum SandboxSpec { + Local { + working_directory: PathBuf, + }, + #[cfg(feature = "docker")] + Docker { + config: DockerSandboxConfig, + }, + #[cfg(feature = "daytona")] + Daytona { + config: DaytonaConfig, + github_app: Option, + run_id: Option, + clone_branch: Option, + }, + #[cfg(feature = "exe")] + Exe { + config: ExeConfig, + clone_params: Option, + run_id: Option, + github_app: Option, + mgmt_destination: String, + }, + #[cfg(feature = "ssh")] + Ssh { + config: SshConfig, + clone_params: Option, + run_id: Option, + github_app: Option, + }, +} + +#[derive(Clone, Copy, PartialEq, Eq)] +pub enum WorkdirStrategy { + LocalDirectory, + LocalWorktree, + Cloud, +} + +impl SandboxSpec { + pub fn provider_name(&self) -> &'static str { + match self { + Self::Local { .. } => "local", + #[cfg(feature = "docker")] + Self::Docker { .. } => "docker", + #[cfg(feature = "daytona")] + Self::Daytona { .. } => "daytona", + #[cfg(feature = "exe")] + Self::Exe { .. } => "exe", + #[cfg(feature = "ssh")] + Self::Ssh { .. } => "ssh", + } + } + + /// Host-accessible repo path for git status / worktree decisions. + /// Only Local and Docker have one. + pub fn host_repo_path(&self) -> Option { + match self { + Self::Local { working_directory } => Some(working_directory.clone()), + #[cfg(feature = "docker")] + Self::Docker { config } => Some(PathBuf::from(&config.host_working_directory)), + #[allow(unreachable_patterns)] + _ => None, + } + } + + /// Build a SandboxRecord for persistence. + pub fn to_sandbox_record(&self, sandbox: &dyn Sandbox) -> SandboxRecord { + let working_directory = sandbox.working_directory().to_string(); + let identifier = { + let info = sandbox.sandbox_info(); + if info.is_empty() { None } else { Some(info) } + }; + + match self { + #[cfg(feature = "docker")] + Self::Docker { config } => SandboxRecord { + provider: self.provider_name().to_string(), + working_directory: working_directory.clone(), + identifier, + host_working_directory: Some(config.host_working_directory.clone()), + container_mount_point: Some(working_directory), + data_host: None, + }, + #[cfg(feature = "ssh")] + Self::Ssh { .. } => SandboxRecord { + provider: self.provider_name().to_string(), + working_directory, + identifier, + host_working_directory: None, + container_mount_point: None, + data_host: sandbox.data_host().map(ToOwned::to_owned), + }, + #[cfg(feature = "exe")] + Self::Exe { .. } => SandboxRecord { + provider: self.provider_name().to_string(), + working_directory, + identifier, + host_working_directory: None, + container_mount_point: None, + data_host: sandbox.data_host().map(ToOwned::to_owned), + }, + _ => SandboxRecord { + provider: self.provider_name().to_string(), + working_directory, + identifier, + host_working_directory: None, + container_mount_point: None, + data_host: None, + }, + } + } + + /// Apply devcontainer snapshot config. Only Daytona uses this. + #[cfg(feature = "daytona")] + pub fn apply_devcontainer_snapshot(&mut self, snapshot: DaytonaSnapshotConfig) { + if let Self::Daytona { config, .. } = self { + config.snapshot = Some(snapshot); + } + } + + pub fn workdir_strategy( + &self, + worktree_mode: WorktreeMode, + git_is_clean: bool, + checkpoint_present: bool, + ) -> WorkdirStrategy { + if checkpoint_present { + return match self { + Self::Local { .. } => WorkdirStrategy::LocalDirectory, + #[cfg(feature = "docker")] + Self::Docker { .. } => WorkdirStrategy::LocalDirectory, + #[allow(unreachable_patterns)] + _ => WorkdirStrategy::Cloud, + }; + } + + match self { + Self::Local { .. } => match worktree_mode { + WorktreeMode::Always => WorkdirStrategy::LocalWorktree, + WorktreeMode::Clean => { + if git_is_clean { + WorkdirStrategy::LocalWorktree + } else { + WorkdirStrategy::LocalDirectory + } + } + WorktreeMode::Dirty => { + if git_is_clean { + WorkdirStrategy::LocalDirectory + } else { + WorkdirStrategy::LocalWorktree + } + } + WorktreeMode::Never => WorkdirStrategy::LocalDirectory, + }, + #[cfg(feature = "docker")] + Self::Docker { .. } => WorkdirStrategy::LocalDirectory, + #[allow(unreachable_patterns)] + _ => WorkdirStrategy::Cloud, + } + } + + pub async fn build( + &self, + event_callback: Option, + ) -> Result, anyhow::Error> { + match self { + Self::Local { working_directory } => { + let mut sandbox = LocalSandbox::new(working_directory.clone()); + if let Some(callback) = event_callback { + sandbox.set_event_callback(callback); + } + Ok(Arc::new(sandbox)) + } + #[cfg(feature = "docker")] + Self::Docker { config } => { + let mut sandbox = DockerSandbox::new(DockerSandboxConfig { + image: config.image.clone(), + host_working_directory: config.host_working_directory.clone(), + container_mount_point: config.container_mount_point.clone(), + network_mode: config.network_mode.clone(), + extra_mounts: config.extra_mounts.clone(), + memory_limit: config.memory_limit, + cpu_quota: config.cpu_quota, + auto_pull: config.auto_pull, + env_vars: config.env_vars.clone(), + }) + .map_err(|e| anyhow!("Failed to create Docker sandbox: {e}"))?; + if let Some(callback) = event_callback { + sandbox.set_event_callback(callback); + } + Ok(Arc::new(sandbox)) + } + #[cfg(feature = "daytona")] + Self::Daytona { + config, + github_app, + run_id, + clone_branch, + } => { + let mut sandbox = DaytonaSandbox::new( + config.clone(), + github_app.clone(), + run_id.clone(), + clone_branch.clone(), + ) + .await + .map_err(|e| anyhow!(e))?; + if let Some(callback) = event_callback { + sandbox.set_event_callback(callback); + } + Ok(Arc::new(sandbox)) + } + #[cfg(feature = "exe")] + Self::Exe { + config, + clone_params, + run_id, + github_app, + mgmt_destination, + } => { + let mgmt_ssh = OpensshRunner::connect_raw(mgmt_destination) + .await + .map_err(|e| anyhow!("Failed to connect to {mgmt_destination}: {e}"))?; + let mut sandbox = ExeSandbox::new( + Box::new(mgmt_ssh), + config.clone(), + clone_params.clone(), + run_id.clone(), + github_app.clone(), + ); + if let Some(callback) = event_callback { + sandbox.set_event_callback(callback); + } + Ok(Arc::new(sandbox)) + } + #[cfg(feature = "ssh")] + Self::Ssh { + config, + clone_params, + run_id, + github_app, + } => { + let mut sandbox = SshSandbox::new( + config.clone(), + clone_params.clone(), + run_id.clone(), + github_app.clone(), + ); + if let Some(callback) = event_callback { + sandbox.set_event_callback(callback); + } + Ok(Arc::new(sandbox)) + } + } + } +} diff --git a/lib/crates/fabro-sandbox/src/ssh/mod.rs b/lib/crates/fabro-sandbox/src/ssh/mod.rs index 425d5fa17..4d8dd74ca 100644 --- a/lib/crates/fabro-sandbox/src/ssh/mod.rs +++ b/lib/crates/fabro-sandbox/src/ssh/mod.rs @@ -593,6 +593,10 @@ impl Sandbox for SshSandbox { Ok(Some(self.ssh_command())) } + fn data_host(&self) -> Option<&str> { + Some(&self.config.destination) + } + fn origin_url(&self) -> Option<&str> { self.origin_url.get().map(String::as_str) } diff --git a/lib/crates/fabro-sandbox/src/ssh_common.rs b/lib/crates/fabro-sandbox/src/ssh_common.rs index 511b059ec..2605ab156 100644 --- a/lib/crates/fabro-sandbox/src/ssh_common.rs +++ b/lib/crates/fabro-sandbox/src/ssh_common.rs @@ -1,5 +1,6 @@ //! Shared types and utilities for SSH-based sandbox implementations (exe, ssh). +use std::path::Path; use std::time::Instant; use async_trait::async_trait; @@ -41,6 +42,19 @@ pub struct GitCloneParams { pub branch: Option, } +#[cfg(feature = "daytona")] +pub fn detect_clone_params(cwd: &Path) -> Option { + let (detected_url, branch) = match crate::daytona::detect_repo_info(cwd) { + Ok(info) => info, + Err(err) => { + tracing::warn!("No git repo detected for sandbox clone: {err}"); + return None; + } + }; + let url = fabro_github::ssh_url_to_https(&detected_url); + Some(GitCloneParams { url, branch }) +} + /// Wrap a shell command in base64 encoding to avoid escaping issues. pub(crate) fn wrap_bash_command(command: &str) -> String { let encoded = STANDARD.encode(command); diff --git a/lib/crates/fabro-sandbox/src/worktree.rs b/lib/crates/fabro-sandbox/src/worktree.rs index a1185354e..25619cad4 100644 --- a/lib/crates/fabro-sandbox/src/worktree.rs +++ b/lib/crates/fabro-sandbox/src/worktree.rs @@ -293,6 +293,10 @@ impl Sandbox for WorktreeSandbox { Some(&self.config.worktree_path) } + fn data_host(&self) -> Option<&str> { + self.inner.data_host() + } + async fn setup_git_for_run(&self, run_id: &str) -> Result, String> { self.inner.setup_git_for_run(run_id).await } diff --git a/lib/crates/fabro-workflows/src/operations/mod.rs b/lib/crates/fabro-workflows/src/operations/mod.rs index 5a69ba328..7217d39f6 100644 --- a/lib/crates/fabro-workflows/src/operations/mod.rs +++ b/lib/crates/fabro-workflows/src/operations/mod.rs @@ -8,8 +8,9 @@ mod start; mod test_support; mod validate; -pub use crate::pipeline::{DevcontainerSpec, LlmSpec, SandboxEnvSpec, SandboxSpec}; +pub use crate::pipeline::{DevcontainerSpec, LlmSpec, SandboxEnvSpec}; pub use create::{CreateRunInput, CreatedRun, create}; +pub use fabro_sandbox::SandboxSpec; pub use fork::{ForkRunInput, fork}; pub use resume::resume; pub use rewind::{ diff --git a/lib/crates/fabro-workflows/src/operations/start.rs b/lib/crates/fabro-workflows/src/operations/start.rs index efadd4d9c..4675d39bf 100644 --- a/lib/crates/fabro-workflows/src/operations/start.rs +++ b/lib/crates/fabro-workflows/src/operations/start.rs @@ -10,7 +10,7 @@ use fabro_config::sandbox::WorktreeMode; use fabro_config::{project as project_config, run as run_config, sandbox as sandbox_config}; use fabro_interview::{AutoApproveInterviewer, Interviewer}; use fabro_model::{Catalog, FallbackTarget, Provider}; -use fabro_sandbox::SandboxProvider; +use fabro_sandbox::{SandboxProvider, SandboxSpec, detect_clone_params}; use serde::Serialize; use crate::context::Context; @@ -23,8 +23,8 @@ use crate::handler::HandlerRegistry; use crate::outcome::{Outcome, StageStatus}; use crate::pipeline::{ self, DevcontainerSpec, FinalizeOptions, Finalized, InitOptions, LlmSpec, Persisted, - PullRequestOptions, RetroOptions, SandboxEnvSpec, SandboxSpec, build_conclusion, - classify_engine_result, persist_terminal_outcome, + PullRequestOptions, RetroOptions, SandboxEnvSpec, build_conclusion, classify_engine_result, + persist_terminal_outcome, }; use crate::records::{Checkpoint, Conclusion, ConclusionExt, RunRecord, RunRecordExt}; use crate::run_options::{GitCheckpointOptions, LifecycleOptions, RunOptions}; @@ -33,7 +33,7 @@ use fabro_config::run::PullRequestSettings; use fabro_retro::retro::Retro; use fabro_sandbox::daytona::DaytonaConfig; use fabro_sandbox::daytona::detect_repo_info; -use fabro_sandbox::ssh::{GitCloneParams as SshGitCloneParams, SshConfig}; +use fabro_sandbox::ssh::SshConfig; use tokio::runtime::Handle; struct RunSession { @@ -174,12 +174,11 @@ impl RunSession { .unwrap_or((None, None)); let sandbox_provider = resolve_sandbox_provider(&settings)?; - let sandbox_provider = - if settings.dry_run_enabled() && !matches!(sandbox_provider, SandboxProvider::Local) { - SandboxProvider::Local - } else { - sandbox_provider - }; + let sandbox_provider = if settings.dry_run_enabled() && !sandbox_provider.is_local() { + SandboxProvider::Local + } else { + sandbox_provider + }; let model = settings .llm .as_ref() @@ -225,7 +224,7 @@ impl RunSession { #[cfg(feature = "exedev")] SandboxProvider::Exe => SandboxSpec::Exe { config: resolve_exe_config(&settings).unwrap_or_default(), - clone_params: resolve_exe_clone_params(&working_directory), + clone_params: detect_clone_params(&working_directory), run_id: Some(record.run_id.clone()), github_app: services.github_app.clone(), mgmt_destination: "exe.dev".to_string(), @@ -242,7 +241,7 @@ impl RunSession { "--sandbox ssh requires [sandbox.ssh] config".to_string(), ) })?, - clone_params: resolve_ssh_clone_params(&working_directory), + clone_params: detect_clone_params(&working_directory), run_id: Some(record.run_id.clone()), github_app: services.github_app.clone(), }, @@ -346,37 +345,12 @@ fn resolve_exe_config(settings: &FabroSettings) -> Option Option { - let (detected_url, branch) = match fabro_sandbox::daytona::detect_repo_info(cwd) { - Ok(info) => info, - Err(err) => { - tracing::warn!("No git repo detected for exe.dev clone: {err}"); - return None; - } - }; - let url = fabro_github::ssh_url_to_https(&detected_url); - Some(fabro_sandbox::exe::GitCloneParams { url, branch }) -} - fn resolve_ssh_config(settings: &FabroSettings) -> Option { settings .sandbox_settings() .and_then(|sandbox| sandbox.ssh.clone()) } -fn resolve_ssh_clone_params(cwd: &Path) -> Option { - let (detected_url, branch) = match detect_repo_info(cwd) { - Ok(info) => info, - Err(err) => { - tracing::warn!("No git repo detected for SSH clone: {err}"); - return None; - } - }; - let url = fabro_github::ssh_url_to_https(&detected_url); - Some(SshGitCloneParams { url, branch }) -} - fn resolve_fallback_chain( provider: Provider, model: &str, diff --git a/lib/crates/fabro-workflows/src/pipeline/execute/tests.rs b/lib/crates/fabro-workflows/src/pipeline/execute/tests.rs index 61ebb04a0..887fc8c8c 100644 --- a/lib/crates/fabro-workflows/src/pipeline/execute/tests.rs +++ b/lib/crates/fabro-workflows/src/pipeline/execute/tests.rs @@ -11,6 +11,7 @@ use fabro_config::FabroSettings; use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node}; use fabro_hooks::HookConfig; use fabro_interview::AutoApproveInterviewer; +use fabro_sandbox::SandboxSpec; use super::*; use crate::context::{self, Context}; @@ -20,7 +21,7 @@ use crate::handler::start::StartHandler; use crate::handler::{Handler as HandlerTrait, HandlerRegistry}; use crate::outcome::{Outcome, OutcomeExt, StageStatus}; use crate::pipeline::initialize; -use crate::pipeline::types::{InitOptions, LlmSpec, Persisted, SandboxEnvSpec, SandboxSpec}; +use crate::pipeline::types::{InitOptions, LlmSpec, Persisted, SandboxEnvSpec}; use crate::records::{Checkpoint, CheckpointExt, RunRecord, StartRecordExt}; use crate::run_options::{GitCheckpointOptions, LifecycleOptions, RunOptions}; use crate::test_support::run_graph; diff --git a/lib/crates/fabro-workflows/src/pipeline/initialize.rs b/lib/crates/fabro-workflows/src/pipeline/initialize.rs index e02d4017b..6ea1c039a 100644 --- a/lib/crates/fabro-workflows/src/pipeline/initialize.rs +++ b/lib/crates/fabro-workflows/src/pipeline/initialize.rs @@ -4,11 +4,10 @@ use std::sync::Arc; use std::time::Instant; use fabro_agent::Sandbox; -use fabro_config::sandbox::WorktreeMode; use fabro_hooks::{HookContext, HookDecision, HookEvent, HookRunner}; use fabro_llm::client::Client; use fabro_sandbox::{ - DockerSandbox, LocalSandbox, ReadBeforeWriteSandbox, SandboxRecord, SandboxRecordExt, + ReadBeforeWriteSandbox, SandboxEventCallback, SandboxRecordExt, WorkdirStrategy, WorktreeConfig, WorktreeSandbox, }; use shlex::try_quote; @@ -19,28 +18,13 @@ use crate::event::{EventEmitter, RunNoticeLevel, WorkflowRunEvent}; use crate::git::{self, GitSyncStatus, MetadataStore}; use crate::handler::llm::{AgentApiBackend, AgentCliBackend, BackendRouter}; use crate::handler::{HandlerRegistry, default_registry}; -use crate::run_options::{GitCheckpointOptions, RunOptions}; -use fabro_sandbox::daytona::DaytonaSandbox; -use fabro_sandbox::docker::DockerSandboxConfig; -use fabro_sandbox::ssh::SshSandbox; +use crate::run_options::GitCheckpointOptions; use tokio::process::Command as TokioCommand; use tokio::runtime::Handle; use tokio::task::spawn_blocking; use tokio::time::timeout as tokio_timeout; -use super::types::{InitOptions, Initialized, LlmSpec, Persisted, SandboxEnvSpec, SandboxSpec}; - -struct SandboxBuildResult { - sandbox: Arc, - worktree_created: bool, -} - -#[derive(Clone, Copy, PartialEq, Eq)] -enum WorkdirStrategy { - LocalDirectory, - LocalWorktree, - Cloud, -} +use super::types::{InitOptions, Initialized, LlmSpec, Persisted, SandboxEnvSpec}; struct WorktreePlan { branch_name: String, @@ -73,64 +57,6 @@ fn emit_run_notice( }); } -fn sandbox_provider_name(spec: &SandboxSpec) -> &'static str { - match spec { - SandboxSpec::Local { .. } => "local", - SandboxSpec::Docker { .. } => "docker", - SandboxSpec::Daytona { .. } => "daytona", - #[cfg(feature = "exedev")] - SandboxSpec::Exe { .. } => "exe", - SandboxSpec::Ssh { .. } => "ssh", - } -} - -fn host_repo_path_for_planning(run_options: &RunOptions, spec: &SandboxSpec) -> Option { - run_options.host_repo_path.clone().or_else(|| match spec { - SandboxSpec::Local { working_directory } => Some(working_directory.clone()), - SandboxSpec::Docker { config } => Some(PathBuf::from(&config.host_working_directory)), - _ => None, - }) -} - -fn resolve_workdir_strategy( - spec: &SandboxSpec, - worktree_mode: WorktreeMode, - git_status: GitSyncStatus, - checkpoint_present: bool, -) -> WorkdirStrategy { - if checkpoint_present { - return match spec { - SandboxSpec::Local { .. } | SandboxSpec::Docker { .. } => { - WorkdirStrategy::LocalDirectory - } - _ => WorkdirStrategy::Cloud, - }; - } - - match spec { - SandboxSpec::Local { .. } => match worktree_mode { - WorktreeMode::Always => WorkdirStrategy::LocalWorktree, - WorktreeMode::Clean => { - if git_status.is_clean() { - WorkdirStrategy::LocalWorktree - } else { - WorkdirStrategy::LocalDirectory - } - } - WorktreeMode::Dirty => { - if git_status.is_clean() { - WorkdirStrategy::LocalDirectory - } else { - WorkdirStrategy::LocalWorktree - } - } - WorktreeMode::Never => WorkdirStrategy::LocalDirectory, - }, - SandboxSpec::Docker { .. } => WorkdirStrategy::LocalDirectory, - _ => WorkdirStrategy::Cloud, - } -} - async fn resolve_worktree_plan( options: &mut InitOptions, ) -> Result, FabroError> { @@ -139,16 +65,19 @@ async fn resolve_worktree_plan( return Ok(None); }; - let host_repo_path = host_repo_path_for_planning(&options.run_options, &options.sandbox); + let host_repo_path = options + .run_options + .host_repo_path + .clone() + .or_else(|| options.sandbox.host_repo_path()); let git_status = host_repo_path .as_ref() .map_or(GitSyncStatus::Dirty, |path| { git::sync_status(path, "origin", options.run_options.base_branch.as_deref()) }); - let strategy = resolve_workdir_strategy( - &options.sandbox, + let strategy = options.sandbox.workdir_strategy( worktree_mode, - git_status, + git_status.is_clean(), options.checkpoint.is_some(), ); @@ -256,154 +185,6 @@ async fn resolve_worktree_plan( } } -fn local_sandbox_with_callback( - working_directory: PathBuf, - emitter: Arc, -) -> Arc { - let mut sandbox = LocalSandbox::new(working_directory); - sandbox.set_event_callback(Arc::new(move |event| { - emitter.emit(&WorkflowRunEvent::Sandbox { event }); - })); - Arc::new(sandbox) -} - -async fn build_sandbox( - spec: &SandboxSpec, - worktree_plan: Option<&WorktreePlan>, - emitter: Arc, -) -> Result { - let mut worktree_created = false; - let sandbox: Arc = match spec { - SandboxSpec::Local { working_directory } => { - if let Some(plan) = worktree_plan { - let inner = - local_sandbox_with_callback(working_directory.clone(), Arc::clone(&emitter)); - let mut worktree = WorktreeSandbox::new( - inner, - WorktreeConfig { - branch_name: plan.branch_name.clone(), - base_sha: plan.base_sha.clone(), - worktree_path: plan.worktree_path.to_string_lossy().into_owned(), - skip_branch_creation: false, - }, - ); - worktree.set_event_callback(Arc::clone(&emitter).worktree_callback()); - match worktree.initialize().await { - Ok(()) => { - worktree_created = true; - Arc::new(ReadBeforeWriteSandbox::new(Arc::new(worktree))) - } - Err(e) => { - emit_run_notice( - &emitter, - RunNoticeLevel::Warn, - "worktree_setup_failed", - format!("Git worktree setup failed ({e}), running without worktree."), - ); - Arc::new(ReadBeforeWriteSandbox::new(local_sandbox_with_callback( - working_directory.clone(), - Arc::clone(&emitter), - ))) - } - } - } else { - Arc::new(ReadBeforeWriteSandbox::new(local_sandbox_with_callback( - working_directory.clone(), - Arc::clone(&emitter), - ))) - } - } - SandboxSpec::Docker { config } => { - let mut sandbox = DockerSandbox::new(DockerSandboxConfig { - image: config.image.clone(), - host_working_directory: config.host_working_directory.clone(), - container_mount_point: config.container_mount_point.clone(), - network_mode: config.network_mode.clone(), - extra_mounts: config.extra_mounts.clone(), - memory_limit: config.memory_limit, - cpu_quota: config.cpu_quota, - auto_pull: config.auto_pull, - env_vars: config.env_vars.clone(), - }) - .map_err(|e| FabroError::engine(format!("Failed to create Docker sandbox: {e}")))?; - let emitter_cb = Arc::clone(&emitter); - sandbox.set_event_callback(Arc::new(move |event| { - emitter_cb.emit(&WorkflowRunEvent::Sandbox { event }); - })); - Arc::new(ReadBeforeWriteSandbox::new(Arc::new(sandbox))) - } - SandboxSpec::Daytona { - config, - github_app, - run_id, - clone_branch, - } => { - let mut sandbox = DaytonaSandbox::new( - config.clone(), - github_app.clone(), - run_id.clone(), - clone_branch.clone(), - ) - .await - .map_err(FabroError::engine)?; - let emitter_cb = Arc::clone(&emitter); - sandbox.set_event_callback(Arc::new(move |event| { - emitter_cb.emit(&WorkflowRunEvent::Sandbox { event }); - })); - Arc::new(ReadBeforeWriteSandbox::new(Arc::new(sandbox))) - } - #[cfg(feature = "exedev")] - SandboxSpec::Exe { - config, - clone_params, - run_id, - github_app, - mgmt_destination, - } => { - let mgmt_ssh = fabro_sandbox::exe::OpensshRunner::connect_raw(mgmt_destination) - .await - .map_err(|e| { - FabroError::engine(format!("Failed to connect to {mgmt_destination}: {e}")) - })?; - let mut sandbox = fabro_sandbox::exe::ExeSandbox::new( - Box::new(mgmt_ssh), - config.clone(), - clone_params.clone(), - run_id.clone(), - github_app.clone(), - ); - let emitter_cb = Arc::clone(&emitter); - sandbox.set_event_callback(Arc::new(move |event| { - emitter_cb.emit(&WorkflowRunEvent::Sandbox { event }); - })); - Arc::new(ReadBeforeWriteSandbox::new(Arc::new(sandbox))) - } - SandboxSpec::Ssh { - config, - clone_params, - run_id, - github_app, - } => { - let mut sandbox = SshSandbox::new( - config.clone(), - clone_params.clone(), - run_id.clone(), - github_app.clone(), - ); - let emitter_cb = Arc::clone(&emitter); - sandbox.set_event_callback(Arc::new(move |event| { - emitter_cb.emit(&WorkflowRunEvent::Sandbox { event }); - })); - Arc::new(ReadBeforeWriteSandbox::new(Arc::new(sandbox))) - } - }; - - Ok(SandboxBuildResult { - sandbox, - worktree_created, - }) -} - async fn mint_github_token( creds: &fabro_github::GitHubAppCredentials, origin_url: &str, @@ -531,12 +312,9 @@ async fn resolve_devcontainer(options: &mut InitOptions) -> Result<(), FabroErro workspace_folder: config.workspace_folder.clone(), }); - if let SandboxSpec::Daytona { - config: daytona, .. - } = &mut options.sandbox - { - daytona.snapshot = Some(devcontainer_to_snapshot_config(&config)); - } + options + .sandbox + .apply_devcontainer_snapshot(devcontainer_to_snapshot_config(&config)); let timeout = std::time::Duration::from_millis(300_000); for command in &config.initialize_commands { @@ -602,48 +380,6 @@ async fn resolve_devcontainer(options: &mut InitOptions) -> Result<(), FabroErro Ok(()) } - -fn write_sandbox_record( - run_dir: &Path, - spec: &SandboxSpec, - sandbox: &Arc, -) -> Result<(), anyhow::Error> { - let working_directory = sandbox.working_directory().to_string(); - let identifier = { - let info = sandbox.sandbox_info(); - if info.is_empty() { None } else { Some(info) } - }; - - let record = match spec { - SandboxSpec::Docker { config } => SandboxRecord { - provider: sandbox_provider_name(spec).to_string(), - working_directory: working_directory.clone(), - identifier, - host_working_directory: Some(config.host_working_directory.clone()), - container_mount_point: Some(working_directory), - data_host: None, - }, - SandboxSpec::Ssh { config, .. } => SandboxRecord { - provider: sandbox_provider_name(spec).to_string(), - working_directory, - identifier, - host_working_directory: None, - container_mount_point: None, - data_host: Some(config.destination.clone()), - }, - _ => SandboxRecord { - provider: sandbox_provider_name(spec).to_string(), - working_directory, - identifier, - host_working_directory: None, - container_mount_point: None, - data_host: None, - }, - }; - - record.save(&run_dir.join("sandbox.json")) -} - /// INITIALIZE phase: prepare the sandbox, env, and handlers for execution. pub async fn initialize( persisted: Persisted, @@ -670,17 +406,62 @@ pub async fn initialize( }); } - let sandbox_result = build_sandbox( - &options.sandbox, - worktree_plan.as_ref(), - Arc::clone(&options.emitter), - ) - .await?; - if worktree_plan.is_some() && !sandbox_result.worktree_created { + let sandbox_event_callback: SandboxEventCallback = { + let emitter = Arc::clone(&options.emitter); + Arc::new(move |event| { + emitter.emit(&WorkflowRunEvent::Sandbox { event }); + }) + }; + let mut worktree_created = false; + let sandbox: Arc = if let Some(plan) = worktree_plan.as_ref() { + let inner = options + .sandbox + .build(Some(Arc::clone(&sandbox_event_callback))) + .await + .map_err(|e| FabroError::engine(e.to_string()))?; + let mut worktree = WorktreeSandbox::new( + inner, + WorktreeConfig { + branch_name: plan.branch_name.clone(), + base_sha: plan.base_sha.clone(), + worktree_path: plan.worktree_path.to_string_lossy().into_owned(), + skip_branch_creation: false, + }, + ); + worktree.set_event_callback(Arc::clone(&options.emitter).worktree_callback()); + match worktree.initialize().await { + Ok(()) => { + worktree_created = true; + Arc::new(ReadBeforeWriteSandbox::new(Arc::new(worktree))) + } + Err(e) => { + emit_run_notice( + &options.emitter, + RunNoticeLevel::Warn, + "worktree_setup_failed", + format!("Git worktree setup failed ({e}), running without worktree."), + ); + Arc::new(ReadBeforeWriteSandbox::new( + options + .sandbox + .build(Some(Arc::clone(&sandbox_event_callback))) + .await + .map_err(|e| FabroError::engine(e.to_string()))?, + )) + } + } + } else { + Arc::new(ReadBeforeWriteSandbox::new( + options + .sandbox + .build(Some(Arc::clone(&sandbox_event_callback))) + .await + .map_err(|e| FabroError::engine(e.to_string()))?, + )) + }; + if worktree_plan.is_some() && !worktree_created { options.run_options.git = None; } - - let sandbox = sandbox_result.sandbox; let cleanup_guard = scopeguard::guard(Arc::clone(&sandbox), |sandbox| { if let Ok(handle) = Handle::try_current() { handle.spawn(async move { @@ -714,7 +495,11 @@ pub async fn initialize( options.emitter.emit(&WorkflowRunEvent::SandboxInitialized { working_directory: sandbox.working_directory().to_string(), }); - if let Err(e) = write_sandbox_record(&run_dir, &options.sandbox, &sandbox) { + if let Err(e) = options + .sandbox + .to_sandbox_record(&*sandbox) + .save(&run_dir.join("sandbox.json")) + { tracing::warn!(error = %e, "Failed to save sandbox record"); } @@ -868,6 +653,7 @@ mod tests { use fabro_config::FabroSettings; use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node}; use fabro_interview::AutoApproveInterviewer; + use fabro_sandbox::SandboxSpec; use super::*; use crate::pipeline::types::InitOptions; diff --git a/lib/crates/fabro-workflows/src/pipeline/mod.rs b/lib/crates/fabro-workflows/src/pipeline/mod.rs index 0fb99092d..1880cdd4e 100644 --- a/lib/crates/fabro-workflows/src/pipeline/mod.rs +++ b/lib/crates/fabro-workflows/src/pipeline/mod.rs @@ -25,6 +25,6 @@ pub use transform::transform; pub use types::{ Concluded, DevcontainerSpec, Executed, FinalizeOptions, Finalized, InitOptions, Initialized, LlmSpec, Parsed, Persisted, PullRequestOptions, RetroOptions, Retroed, SandboxEnvSpec, - SandboxSpec, TransformOptions, Transformed, Validated, + TransformOptions, Transformed, Validated, }; pub use validate::validate; diff --git a/lib/crates/fabro-workflows/src/pipeline/types.rs b/lib/crates/fabro-workflows/src/pipeline/types.rs index 09fba0cda..0099a7aa2 100644 --- a/lib/crates/fabro-workflows/src/pipeline/types.rs +++ b/lib/crates/fabro-workflows/src/pipeline/types.rs @@ -10,11 +10,7 @@ use fabro_interview::Interviewer; use fabro_llm::Provider; use fabro_mcp::config::McpServerConfig; use fabro_model::FallbackTarget; -use fabro_sandbox::daytona::DaytonaConfig; -use fabro_sandbox::docker::DockerSandboxConfig; -#[cfg(feature = "exedev")] -use fabro_sandbox::exe::{ExeConfig, GitCloneParams as ExeGitCloneParams}; -use fabro_sandbox::ssh::{GitCloneParams as SshGitCloneParams, SshConfig}; +use fabro_sandbox::SandboxSpec; use fabro_validate::Diagnostic; use crate::context::Context; @@ -202,36 +198,6 @@ impl Persisted { } } -/// Options for the INITIALIZE phase. -pub enum SandboxSpec { - Local { - working_directory: PathBuf, - }, - Docker { - config: DockerSandboxConfig, - }, - Daytona { - config: DaytonaConfig, - github_app: Option, - run_id: Option, - clone_branch: Option, - }, - #[cfg(feature = "exedev")] - Exe { - config: ExeConfig, - clone_params: Option, - run_id: Option, - github_app: Option, - mgmt_destination: String, - }, - Ssh { - config: SshConfig, - clone_params: Option, - run_id: Option, - github_app: Option, - }, -} - #[derive(Clone)] pub struct LlmSpec { pub model: String,