diff --git a/lib/apps/fabro-server/src/run_manifest.rs b/lib/apps/fabro-server/src/run_manifest.rs index be188f515..0811e565d 100644 --- a/lib/apps/fabro-server/src/run_manifest.rs +++ b/lib/apps/fabro-server/src/run_manifest.rs @@ -19,8 +19,8 @@ use fabro_llm::lithos_catalog::Catalog; use fabro_llm::probe::{self, ModelTestStatus}; use fabro_sandbox::redact::redact_auth_url; use fabro_sandbox::{ - ProviderAccess, ProviderSandboxSpec, RunSandbox, SandboxSpec, - local_working_directory_from_environment, options_from_environment, unresolved_env, + CloneRequest, ProviderAccess, ProviderSandboxSpec, RunSandbox, SandboxSpec, + sandbox_spec_for_environment, }; use fabro_static::EnvVars; use fabro_types::settings::ModelRef; @@ -918,30 +918,35 @@ fn preflight_sandbox_spec( let clone_branch = prepared.git.as_ref().map(|git| git.branch.clone()); if sandbox_provider.bundled() == Some(BundledProvider::Local) { - let working_directory = local_working_directory_from_environment( - &resolved_run.environment, - Some(&prepared.source_directory), - )?; + let working_directory = resolved_run + .environment + .local_working_directory(Some(&prepared.source_directory)) + .map_err(|err| { + fabro_sandbox::Error::context( + "Failed to resolve local environment working directory", + err, + ) + })?; return Ok(SandboxSpec::Local { working_directory }); } // No vault is available on this path, so a `{{ secrets.* }}` value keeps - // its source form. - let mut options = options_from_environment( + // its source form. Preflight never clones. + let spec = sandbox_spec_for_environment( &resolved_run.environment, - &resolved_run.clone, - unresolved_env(&resolved_run.environment), + resolved_run.environment.unresolved_env(), )?; - options.skip_clone = true; + let clone = CloneRequest { + origin_url: clone_origin_url, + branch: clone_branch, + ..CloneRequest::none() + }; Ok(SandboxSpec::Provider(Box::new(ProviderSandboxSpec { kind: sandbox_provider.clone(), access: access.clone(), - options, + spec, + clone, github_app, run_id: None, - clone_origin_url, - clone_branch, - clone_tag: None, - clone_commit_sha: None, }))) } @@ -2218,12 +2223,12 @@ provider = "local" match spec { Ok(SandboxSpec::Provider(spec)) => { assert_eq!(spec.kind, SandboxProviderKind::DOCKER); - assert!(spec.options.skip_clone); + assert!(spec.clone.skip); assert_eq!( - spec.clone_origin_url.as_deref(), + spec.clone.origin_url.as_deref(), Some("https://github.com/acme/widgets") ); - assert_eq!(spec.clone_branch.as_deref(), Some("main")); + assert_eq!(spec.clone.branch.as_deref(), Some("main")); } _ => panic!("expected Docker preflight sandbox spec"), } diff --git a/lib/components/fabro-agent/src/lib.rs b/lib/components/fabro-agent/src/lib.rs index f3ea13a7b..8a03ced4f 100644 --- a/lib/components/fabro-agent/src/lib.rs +++ b/lib/components/fabro-agent/src/lib.rs @@ -38,7 +38,7 @@ pub use config::{ pub use error::{CompactionError, Error, InterruptReason, Result}; pub use event::Emitter; pub use fabro_mcp::config::McpServerSettings; -pub use fabro_sandbox::{ProviderAccess, SandboxOptions, SandboxProviderKind, provider_sandbox}; +pub use fabro_sandbox::{CloneRequest, ProviderAccess, SandboxProviderKind, provider_sandbox}; pub use fabro_types::SteeringMessage; pub use history::History; pub use local_sandbox::local_sandbox; @@ -55,11 +55,11 @@ pub use question_tools::{ OPENAI_REQUEST_USER_INPUT_TOOL, register_question_tools, }; pub use sandbox::{ - CaptureStats, DirEntry, ExecControls, ExecResult, ExecResultExt, ExecSpec, ExecStreamingResult, - FileKind, GrepMatch, GrepOptions, OutputSink, OutputStream, RefreshOutcome, - RemoteCredentialAction, RunSandbox, SandboxFile, StderrTail, StdioProcess, StdioProcessHandle, - Termination, TokenProvenance, TokenSnapshot, WalkOptions, command_termination, - format_lines_numbered, program_exit_code, shell_quote, + CaptureStats, DirEntry, DriverSpec, ExecControls, ExecResult, ExecResultExt, ExecSpec, + ExecStreamingResult, FileKind, GrepMatch, GrepOptions, OutputSink, OutputStream, + RefreshOutcome, RemoteCredentialAction, RunSandbox, SandboxFile, SandboxSource, StderrTail, + StdioProcess, StdioProcessHandle, Termination, TokenProvenance, TokenSnapshot, WalkOptions, + command_termination, format_lines_numbered, program_exit_code, shell_quote, }; pub use session::{ CompletionCoordinator, Session, SessionControlHandle, SessionInputTiming, diff --git a/lib/components/fabro-agent/src/sandbox.rs b/lib/components/fabro-agent/src/sandbox.rs index 9706536e1..462659125 100644 --- a/lib/components/fabro-agent/src/sandbox.rs +++ b/lib/components/fabro-agent/src/sandbox.rs @@ -1,8 +1,8 @@ // Re-export the sandbox types the agent works with from fabro-sandbox. pub use fabro_sandbox::{ - CaptureStats, DirEntry, ExecControls, ExecResult, ExecResultExt, ExecSpec, ExecStreamingResult, - FileKind, GrepMatch, GrepOptions, OutputSink, OutputStream, RefreshOutcome, - RemoteCredentialAction, RunSandbox, SandboxFile, StderrTail, StdioProcess, StdioProcessHandle, - Termination, TokenProvenance, TokenSnapshot, WalkOptions, command_termination, - format_lines_numbered, program_exit_code, shell_quote, + CaptureStats, DirEntry, DriverSpec, ExecControls, ExecResult, ExecResultExt, ExecSpec, + ExecStreamingResult, FileKind, GrepMatch, GrepOptions, OutputSink, OutputStream, + RefreshOutcome, RemoteCredentialAction, RunSandbox, SandboxFile, SandboxSource, StderrTail, + StdioProcess, StdioProcessHandle, Termination, TokenProvenance, TokenSnapshot, WalkOptions, + command_termination, format_lines_numbered, program_exit_code, shell_quote, }; diff --git a/lib/components/fabro-agent/tests/it/docker_shell.rs b/lib/components/fabro-agent/tests/it/docker_shell.rs index 17d2d691d..7eb56508d 100644 --- a/lib/components/fabro-agent/tests/it/docker_shell.rs +++ b/lib/components/fabro-agent/tests/it/docker_shell.rs @@ -8,7 +8,10 @@ use fabro_agent::event::SessionBoundEmitter; use fabro_agent::tool_registry::ToolContext; use fabro_agent::tools::make_shell_tool; use fabro_agent::types::AgentEvent; -use fabro_agent::{Emitter, ProviderAccess, SandboxOptions, SandboxProviderKind, provider_sandbox}; +use fabro_agent::{ + CloneRequest, DriverSpec, Emitter, ProviderAccess, SandboxProviderKind, SandboxSource, + provider_sandbox, +}; use fabro_types::CommandTermination; use tokio::sync::broadcast; use tokio_util::sync::CancellationToken; @@ -19,15 +22,10 @@ async fn shell_reports_real_docker_process_outcome() { let Ok(sandbox) = provider_sandbox( SandboxProviderKind::DOCKER, &ProviderAccess::default(), - SandboxOptions { - image: Some("buildpack-deps:noble".to_string()), - skip_clone: true, - ..SandboxOptions::default() - }, - None, - None, - None, - None, + DriverSpec::new(SandboxSource::Image { + reference: "buildpack-deps:noble".to_string(), + }), + &CloneRequest::none(), None, None, ) diff --git a/lib/components/fabro-sandbox/src/daytona.rs b/lib/components/fabro-sandbox/src/daytona.rs index 6076a5cd6..8a2dcd39d 100644 --- a/lib/components/fabro-sandbox/src/daytona.rs +++ b/lib/components/fabro-sandbox/src/daytona.rs @@ -16,7 +16,7 @@ use async_trait::async_trait; use fabro_types::settings::server::ServerSandboxProviderSettings; use fabro_types::{RunId, SandboxProviderKind}; use sandbox_driver::{ - EventContext, HealthStatus, LifecycleTimers, Resources, SandboxProvider, SandboxSource, + EventContext, HealthStatus, Resources, SandboxProvider, SandboxSource, SandboxSpec as DriverSpec, SnapshotId, SnapshotSource, SnapshotSpec, }; use tokio::time; @@ -24,7 +24,6 @@ use tokio::time; pub use crate::driver::DaytonaCredentials; use crate::driver::{ProviderConnectOptions, connect_provider}; use crate::driver_sandbox::{CreatePlan, PreparedCreate, WorkspaceLayout}; -use crate::options::SandboxOptions; pub(crate) const WORKING_DIRECTORY: &str = "/home/daytona/workspace"; pub(crate) const REPOS_ROOT: &str = "/home/daytona/repos"; @@ -69,25 +68,30 @@ pub enum SnapshotInput<'a> { Dockerfile(&'a str), } -/// The snapshot `options` ask for, or `None` when the environment names no +/// The snapshot `spec` asks for, or `None` when the environment names no /// image or Dockerfile and the sandbox comes from Daytona's default. -pub fn snapshot_inputs(options: &SandboxOptions) -> Option> { - let source = match (&options.image, &options.dockerfile) { - (Some(image), _) => SnapshotInput::Image(image), - (None, Some(dockerfile)) => SnapshotInput::Dockerfile(dockerfile), - (None, None) => return None, +pub fn snapshot_inputs(spec: &DriverSpec) -> Option> { + let source = match &spec.source { + SandboxSource::Image { reference } => SnapshotInput::Image(reference), + SandboxSource::Dockerfile { content } => SnapshotInput::Dockerfile(content), + _ => return None, }; Some(SnapshotInputs { source, - cpu: options.cpu.and_then(|cpu| i32::try_from(cpu).ok()), - memory_gb: options.memory_bytes.map(bytes_to_gb), - disk_gb: options.disk_bytes.map(bytes_to_gb), + cpu: spec + .resources + .cpu_cores + .and_then(|cpu| i32::try_from(cpu).ok()), + memory_gb: spec.resources.memory_mb.map(gigabytes), + disk_gb: spec.resources.disk_mb.map(gigabytes), }) } -/// Whole decimal gigabytes, the unit Daytona sizes snapshots in. -fn bytes_to_gb(bytes: u64) -> i32 { - i32::try_from(bytes / 1_000_000_000).unwrap_or(i32::MAX) +/// Whole gibibytes, rounded up and never zero: the unit Daytona sizes +/// snapshots in, computed as the driver's Daytona provider does so the +/// snapshot's name and its provisioned size agree. +fn gigabytes(mb: u64) -> i32 { + i32::try_from(mb.div_ceil(1024)).unwrap_or(i32::MAX).max(1) } pub mod snapshot_identity { @@ -300,12 +304,12 @@ pub(crate) fn layout() -> WorkspaceLayout { } } -/// Daytona's additions to the base spec: the snapshot the sandbox is created -/// from, the fixed working directory, the run's Daytona name, and the -/// lifecycle timers. +/// Daytona's additions to the environment's spec: the snapshot the sandbox +/// is created from, the fixed working directory, the run's Daytona name, +/// and the lifecycle timers. The snapshot carries the resources; Daytona +/// refuses them on a sandbox created from one. pub(crate) fn overlay( spec: DriverSpec, - options: &SandboxOptions, run_id: Option<&RunId>, snapshot: &SnapshotId, ) -> DriverSpec { @@ -314,10 +318,11 @@ pub(crate) fn overlay( id: snapshot.clone(), }; spec.name = run_id.map(|run_id| format!("fabro-{run_id}")); - let mut timers = LifecycleTimers::default(); + spec.resources = Resources::default(); + let mut timers = spec.timers; // An explicit zero disables auto-stop; the driver encodes // `Duration::ZERO` as that wire value. - timers.auto_stop_after_idle = Some(options.auto_stop.unwrap_or(DEFAULT_AUTO_STOP)); + timers.auto_stop_after_idle = Some(timers.auto_stop_after_idle.unwrap_or(DEFAULT_AUTO_STOP)); // Run sandboxes are never deleted on stop: the run record may need // them again on resume, and `fabro system prune` reclaims them. timers.auto_delete_after_stop = Some(Duration::ZERO); @@ -377,25 +382,21 @@ pub(crate) struct DaytonaCreatePlan { provider: Arc, api_key: String, base: DriverSpec, - options: SandboxOptions, run_id: Option, } -/// The create plan for a run on Daytona: `base` is the spec the -/// environment's options built, which the plan completes with the snapshot -/// once it exists. +/// The create plan for a run on Daytona: `base` is the environment's spec, +/// which the plan completes with the snapshot once it exists. pub(crate) fn create_plan( provider: Arc, api_key: String, base: DriverSpec, - options: SandboxOptions, run_id: Option, ) -> DaytonaCreatePlan { DaytonaCreatePlan { provider, api_key, base, - options, run_id, } } @@ -403,7 +404,7 @@ pub(crate) fn create_plan( #[async_trait] impl CreatePlan for DaytonaCreatePlan { async fn prepare(&self, events: Option) -> crate::Result { - let (snapshot_id, snapshot_name) = match snapshot_inputs(&self.options) { + let (snapshot_id, snapshot_name) = match snapshot_inputs(&self.base) { // The driver finds, activates, builds, or waits for the snapshot // as needed, and reports that work through `events`. Some(inputs) => { @@ -415,12 +416,7 @@ impl CreatePlan for DaytonaCreatePlan { ), }; Ok(PreparedCreate { - spec: overlay( - self.base.clone(), - &self.options, - self.run_id.as_ref(), - &snapshot_id, - ), + spec: overlay(self.base.clone(), self.run_id.as_ref(), &snapshot_id), snapshot: Some(snapshot_name), }) } @@ -428,12 +424,9 @@ impl CreatePlan for DaytonaCreatePlan { #[cfg(test)] mod tests { - use std::collections::BTreeMap; - - use sandbox_driver::NetworkPolicy; + use sandbox_driver::{LifecycleTimers, NetworkPolicy}; use super::*; - use crate::options::base_spec; fn run_id() -> RunId { "01HY0000000000000000000000".parse().unwrap() @@ -450,17 +443,20 @@ mod tests { #[test] fn snapshot_inputs_come_from_the_image_or_dockerfile_in_whole_gigabytes() { - assert!(snapshot_inputs(&SandboxOptions::default()).is_none()); + assert!(snapshot_inputs(&DriverSpec::new(SandboxSource::HostDirectory)).is_none()); - let options = SandboxOptions { - image: Some("ubuntu:24.04".to_string()), - cpu: Some(2), - memory_bytes: Some(4_000_000_000), - disk_bytes: Some(10_500_000_000), - ..SandboxOptions::default() - }; + // 4 GB and 10.5 GB of memory and disk, as the environment mapping + // sizes them in mebibytes. + let mut resources = Resources::default(); + resources.cpu_cores = Some(2); + resources.memory_mb = Some(3815); + resources.disk_mb = Some(10_014); + let spec = DriverSpec::new(SandboxSource::Image { + reference: "ubuntu:24.04".to_string(), + }) + .resources(resources); assert_eq!( - snapshot_inputs(&options), + snapshot_inputs(&spec), Some(SnapshotInputs { source: SnapshotInput::Image("ubuntu:24.04"), cpu: Some(2), @@ -469,32 +465,30 @@ mod tests { }) ); - let options = SandboxOptions { - dockerfile: Some("FROM ubuntu".to_string()), - ..SandboxOptions::default() - }; + let spec = DriverSpec::new(SandboxSource::Dockerfile { + content: "FROM ubuntu".to_string(), + }); assert_eq!( - snapshot_inputs(&options).map(|inputs| inputs.source), + snapshot_inputs(&spec).map(|inputs| inputs.source), Some(SnapshotInput::Dockerfile("FROM ubuntu")) ); + assert_eq!(gigabytes(1), 1, "a snapshot is never sized at zero"); + assert_eq!(gigabytes(1024), 1); + assert_eq!(gigabytes(1025), 2); } #[test] fn overlay_names_the_run_and_carries_fabro_labels_and_timers() { - let options = SandboxOptions { - labels: BTreeMap::from([("team".to_string(), "platform".to_string())]), - network: NetworkPolicy::CidrAllowList { + let mut resources = Resources::default(); + resources.cpu_cores = Some(2); + let base = DriverSpec::new(SandboxSource::HostDirectory) + .label("team", "platform") + .network(NetworkPolicy::CidrAllowList { cidrs: vec!["10.0.0.0/8".to_string()], - }, - ..SandboxOptions::default() - }; + }) + .resources(resources); let snapshot = SnapshotId::try_new("snap-1").unwrap(); - let spec = overlay( - base_spec(&options, Some(&run_id())), - &options, - Some(&run_id()), - &snapshot, - ); + let spec = overlay(base, Some(&run_id()), &snapshot); assert!(matches!(&spec.source, SandboxSource::Snapshot { id } if id == &snapshot)); assert_eq!( @@ -519,18 +513,23 @@ mod tests { "an unset auto-stop gets fabro's explicit default, never Daytona's 15 minutes" ); assert_eq!(spec.timers.auto_delete_after_stop, Some(Duration::ZERO)); + assert_eq!( + spec.resources, + Resources::default(), + "the snapshot carries the resources; Daytona refuses them on the sandbox" + ); assert!(!spec.ephemeral); } #[test] fn overlay_passes_explicit_auto_stop_through_and_zero_disables() { let snapshot = SnapshotId::try_new(DEFAULT_SNAPSHOT).unwrap(); - let options = SandboxOptions { - auto_stop: Some(Duration::from_mins(45)), - network: NetworkPolicy::Block, - ..SandboxOptions::default() - }; - let explicit = overlay(base_spec(&options, None), &options, None, &snapshot); + let mut timers = LifecycleTimers::default(); + timers.auto_stop_after_idle = Some(Duration::from_mins(45)); + let base = DriverSpec::new(SandboxSource::HostDirectory) + .network(NetworkPolicy::Block) + .timers(timers); + let explicit = overlay(base, None, &snapshot); assert_eq!( explicit.timers.auto_stop_after_idle, Some(Duration::from_mins(45)) @@ -538,11 +537,13 @@ mod tests { assert!(matches!(explicit.network, NetworkPolicy::Block)); assert!(explicit.name.is_none()); - let options = SandboxOptions { - auto_stop: Some(Duration::ZERO), - ..SandboxOptions::default() - }; - let disabled = overlay(base_spec(&options, None), &options, None, &snapshot); + let mut timers = LifecycleTimers::default(); + timers.auto_stop_after_idle = Some(Duration::ZERO); + let disabled = overlay( + DriverSpec::new(SandboxSource::HostDirectory).timers(timers), + None, + &snapshot, + ); assert_eq!(disabled.timers.auto_stop_after_idle, Some(Duration::ZERO)); } @@ -728,7 +729,7 @@ mod wire_gate { use super::*; use crate::driver_sandbox::{LayoutSource, RepoWorkspace, RunSandbox}; - use crate::options::base_spec; + use crate::environment::CloneRequest; #[expect( clippy::disallowed_methods, @@ -765,18 +766,20 @@ mod wire_gate { let workspace = RepoWorkspace::plan( LayoutSource::Fixed(layout()), - false, - Some("https://github.com/brynary/rack-test"), - None, - None, - None, - Some(100), + &CloneRequest { + origin_url: Some("https://github.com/brynary/rack-test".to_string()), + depth: Some(100), + ..CloneRequest::default() + }, None, ) .expect("clone plan"); let snapshot = SnapshotId::try_new(DEFAULT_SNAPSHOT).expect("snapshot id"); - let options = SandboxOptions::default(); - let spec = overlay(base_spec(&options, None), &options, None, &snapshot); + let spec = overlay( + DriverSpec::new(SandboxSource::HostDirectory), + None, + &snapshot, + ); let sandbox = RunSandbox::pending(SandboxProviderKind::DAYTONA, remote, spec, workspace); sandbox .initialize() diff --git a/lib/components/fabro-sandbox/src/docker.rs b/lib/components/fabro-sandbox/src/docker.rs index 87cb66f57..138cd582c 100644 --- a/lib/components/fabro-sandbox/src/docker.rs +++ b/lib/components/fabro-sandbox/src/docker.rs @@ -8,12 +8,11 @@ //! [`REPOS_ROOT`] and is linked into the workspace, so the run works in //! `/workspace/`. -use sandbox_driver::{HealthStatus, SandboxSource, SandboxSpec as DriverSpec}; +use sandbox_driver::{HealthStatus, LifecycleTimers, SandboxSource, SandboxSpec as DriverSpec}; use sandbox_driver_docker_config::DockerProviderConfig; use crate::driver::ProviderAccess; use crate::driver_sandbox::WorkspaceLayout; -use crate::options::SandboxOptions; use crate::provider_sandbox; pub const WORKING_DIRECTORY: &str = "/workspace"; @@ -30,28 +29,28 @@ pub(crate) fn layout() -> WorkspaceLayout { } /// The image a Docker sandbox runs: the environment's, or the default. -pub(crate) fn effective_image(options: &SandboxOptions) -> String { - options - .image - .clone() - .unwrap_or_else(|| DEFAULT_IMAGE.to_string()) +pub(crate) fn effective_image(spec: &DriverSpec) -> String { + match &spec.source { + SandboxSource::Image { reference } => reference.clone(), + _ => DEFAULT_IMAGE.to_string(), + } } -/// Docker's additions to the base spec, and the image it will run. -pub(crate) fn overlay(spec: DriverSpec, options: &SandboxOptions) -> (DriverSpec, String) { - let image = effective_image(options); +/// Docker's additions to the environment's spec: the image it will run, +/// the fixed working directory, and a pull for a missing image. Docker has +/// no lifecycle timers, so the environment's auto-stop does not apply. +pub(crate) fn overlay(spec: DriverSpec) -> DriverSpec { + let image = effective_image(&spec); let mut spec = spec; - spec.source = SandboxSource::Image { - reference: image.clone(), - }; - let spec = spec.working_directory(WORKING_DIRECTORY).provider_config( + spec.source = SandboxSource::Image { reference: image }; + spec.timers = LifecycleTimers::default(); + spec.working_directory(WORKING_DIRECTORY).provider_config( DockerProviderConfig { auto_pull: true, ..DockerProviderConfig::default() } .into_value(), - ); - (spec, image) + ) } /// Whether the Docker daemon answers. Used by `fabro doctor`. @@ -76,57 +75,49 @@ pub async fn check_docker_daemon() -> crate::Result<()> { #[cfg(test)] mod tests { - use std::collections::BTreeMap; + use std::time::Duration; - use fabro_types::RunId; use sandbox_driver::NetworkPolicy; use super::*; - use crate::options::base_spec; #[test] fn overlay_fixes_the_workspace_and_pulls_the_named_image() { - let run_id: RunId = "01HY0000000000000000000000".parse().unwrap(); - let options = SandboxOptions { - image: Some("ghcr.io/acme/dev:1".to_string()), - env: BTreeMap::from([("FOO".to_string(), "bar".to_string())]), - memory_bytes: Some(4_000_000_000), - cpu: Some(2), - network: NetworkPolicy::Block, - ..SandboxOptions::default() - }; - let (spec, image) = overlay(base_spec(&options, Some(&run_id)), &options); - - assert_eq!(image, "ghcr.io/acme/dev:1"); + let mut requested = LifecycleTimers::default(); + requested.auto_stop_after_idle = Some(Duration::from_mins(45)); + let spec = overlay( + DriverSpec::new(SandboxSource::Image { + reference: "ubuntu:24.04".to_string(), + }) + .network(NetworkPolicy::Block) + .timers(requested), + ); assert!(matches!( &spec.source, - SandboxSource::Image { reference } if reference == "ghcr.io/acme/dev:1" + SandboxSource::Image { reference } if reference == "ubuntu:24.04" )); - assert_eq!( - spec.name.as_deref(), - Some("fabro-run-01HY0000000000000000000000") - ); assert_eq!(spec.working_directory.as_deref(), Some(WORKING_DIRECTORY)); - assert!( - !spec.labels.contains_key("sh.fabro.managed"), - "ownership labels come from the scope the provider is connected through" - ); - assert_eq!(spec.env.get("FOO").map(String::as_str), Some("bar")); - assert_eq!(spec.resources.cpu_cores, Some(2)); - assert_eq!(spec.resources.memory_mb, Some(3815)); assert!(matches!(spec.network, NetworkPolicy::Block)); - assert_eq!(spec.provider_config["auto_pull"], true); + assert_eq!( + spec.timers, + LifecycleTimers::default(), + "docker has no timers to honor the environment's auto-stop with" + ); + let config: DockerProviderConfig = + serde_json::from_value(spec.provider_config).expect("docker provider config"); + assert!(config.auto_pull); } #[test] fn overlay_supplies_the_default_image_when_the_environment_names_none() { - let options = SandboxOptions::default(); - let (spec, image) = overlay(base_spec(&options, None), &options); - assert_eq!(image, DEFAULT_IMAGE); + let spec = overlay(DriverSpec::new(SandboxSource::HostDirectory)); assert!(matches!( &spec.source, SandboxSource::Image { reference } if reference == DEFAULT_IMAGE )); - assert!(spec.name.is_none()); + assert_eq!( + effective_image(&DriverSpec::new(SandboxSource::HostDirectory)), + DEFAULT_IMAGE + ); } } diff --git a/lib/components/fabro-sandbox/src/driver_sandbox.rs b/lib/components/fabro-sandbox/src/driver_sandbox.rs index 8be873974..4dc99ad02 100644 --- a/lib/components/fabro-sandbox/src/driver_sandbox.rs +++ b/lib/components/fabro-sandbox/src/driver_sandbox.rs @@ -35,6 +35,7 @@ use tokio_util::sync::CancellationToken; use crate::clone::{self, GitHubClone}; use crate::clone_source::{self, CloneDecision, EmptyWorkspaceReason}; +use crate::environment::CloneRequest; use crate::push_credentials::{self, PushCredentialState}; use crate::{GitRunInfo, GitSetupIntent, RefreshOutcome, RetryPlan}; @@ -131,31 +132,22 @@ pub(crate) struct RepoWorkspace { impl RepoWorkspace { /// Decide the clone for a new sandbox. Fails before any provider call /// when the selectors are inconsistent (a pin without a branch, a - /// non-GitHub origin without `skip_clone`). - #[expect( - clippy::too_many_arguments, - reason = "the clone selectors are validated together by decide_clone" - )] + /// non-GitHub origin without `skip`). pub(crate) fn plan( layout: LayoutSource, - skip_clone: bool, - clone_origin_url: Option<&str>, - clone_branch: Option<&str>, - clone_tag: Option<&str>, - clone_commit_sha: Option<&str>, - clone_depth: Option, + clone: &CloneRequest, github_app: Option<&GitHubCredentials>, ) -> crate::Result { let decision = clone_source::decide_clone( - skip_clone, - clone_origin_url, - clone_branch, - clone_tag, - clone_commit_sha, + clone.skip, + clone.origin_url.as_deref(), + clone.branch.as_deref(), + clone.tag.as_deref(), + clone.commit_sha.as_deref(), )?; let credentials = PushCredentialState::new(push_credentials::build_token_source( github_app, - clone_origin_url, + clone.origin_url.as_deref(), )?); let plan = match decision { CloneDecision::EmptyWorkspace { reason } => WorkspacePlan::Empty(reason), @@ -169,7 +161,7 @@ impl RepoWorkspace { branch, tag, commit_sha, - depth: clone_depth, + depth: clone.depth, }), }; Ok(Self { diff --git a/lib/components/fabro-sandbox/src/environment.rs b/lib/components/fabro-sandbox/src/environment.rs new file mode 100644 index 000000000..76461fbce --- /dev/null +++ b/lib/components/fabro-sandbox/src/environment.rs @@ -0,0 +1,314 @@ +//! What an environment asks of a sandbox, mapped once onto the driver's spec. +//! +//! The environment names an image or Dockerfile, resources, a network +//! policy, labels, variables, and a lifecycle. Every provider starts from +//! the same driver [`SandboxSpec`] built here; a bundled provider adds only +//! what its backend needs on top (the Docker working directory and default +//! image, the Daytona snapshot and timers) in its own overlay, and the +//! ownership scope adds fabro's labels. The clone policy travels beside the +//! spec as a [`CloneRequest`]: cloning is fabro's work once the sandbox +//! exists, not the provider's. + +use std::collections::BTreeMap; + +use fabro_types::RunId; +use fabro_types::settings::run::{ + DockerfileSource, EnvironmentNetworkMode, RunCloneSettings, RunEnvironmentSettings, +}; +use sandbox_driver::{ + Capabilities, LifecycleTimers, NetworkPolicy, Resources, SandboxSource, SandboxSpec, +}; + +/// What to clone into a provider sandbox, if anything. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct CloneRequest { + pub origin_url: Option, + /// The branch the checkout works on. + pub branch: Option, + /// A tag to pin the checkout to; the branch still names the checkout. + pub tag: Option, + /// An exact commit to pin the checkout to, authoritative over `tag`. + pub commit_sha: Option, + /// Maximum Git history depth fetched; `None` fetches full history. + pub depth: Option, + /// Create an empty workspace instead of cloning, even when an origin + /// is present. + pub skip: bool, +} + +impl CloneRequest { + /// No clone: the run starts in an empty workspace. + #[must_use] + pub fn none() -> Self { + Self { + skip: true, + ..Self::default() + } + } + + /// The environment's clone policy: whether to clone and how deep. The + /// origin and the selectors come from the run's target. + #[must_use] + pub fn from_settings(clone: &RunCloneSettings) -> Self { + Self { + depth: clone + .depth_limit() + .and_then(|depth| u32::try_from(depth).ok()), + skip: !clone.enabled, + ..Self::default() + } + } +} + +/// The driver spec every provider starts from: the environment's source +/// (an image, a Dockerfile, or a managed directory when it names neither), +/// its labels, variables, resources, network policy, and auto-stop. `env` +/// is the environment's variables, resolved by the caller: the worker +/// resolves secrets through the vault, while preflight carries them in +/// source form. +/// +/// A Dockerfile given as a path must have been resolved to inline content +/// earlier; none of the providers can read a path. +pub fn sandbox_spec_for_environment( + settings: &RunEnvironmentSettings, + env: BTreeMap, +) -> crate::Result { + // fabro-config rejects environments that set both image.docker and + // image.dockerfile. If both still arrive here, the image wins. + let source = match (&settings.image.docker, &settings.image.dockerfile) { + (Some(reference), _) => SandboxSource::Image { + reference: reference.clone(), + }, + (None, Some(DockerfileSource::Inline(content))) => SandboxSource::Dockerfile { + content: content.clone(), + }, + (None, Some(DockerfileSource::Path { path })) => { + return Err(crate::Error::message(format!( + "environment `{}` names a Dockerfile path ({path}) that should have been \ + resolved to inline content before sandbox creation", + settings.id + ))); + } + // A provider without images (a host-style plugin) manages a + // workspace directory of its own. + (None, None) => SandboxSource::HostDirectory, + }; + let network = match settings.network.mode { + EnvironmentNetworkMode::Block => NetworkPolicy::Block, + EnvironmentNetworkMode::AllowAll => NetworkPolicy::AllowAll, + EnvironmentNetworkMode::CidrAllowList => NetworkPolicy::CidrAllowList { + cidrs: settings.network.allow.clone(), + }, + }; + let mut spec = SandboxSpec::new(source).network(network); + // The environment's labels; fabro's ownership labels are stamped by the + // ownership scope the provider is connected through. + for (key, value) in &settings.labels { + spec = spec.label(key, value); + } + for (key, value) in env { + spec = spec.env_var(key, value); + } + let mut resources = Resources::default(); + resources.cpu_cores = settings + .resources + .cpu + .and_then(|cpu| u32::try_from(cpu).ok()); + resources.memory_mb = settings + .resources + .memory + .map(|size| mebibytes(size.as_bytes())); + resources.disk_mb = settings + .resources + .disk + .map(|size| mebibytes(size.as_bytes())); + let mut timers = LifecycleTimers::default(); + timers.auto_stop_after_idle = settings + .lifecycle + .auto_stop + .map(|duration| duration.as_std()); + Ok(spec.resources(resources).timers(timers)) +} + +/// Whole mebibytes, rounded up: the unit the driver sizes resources in. +fn mebibytes(bytes: u64) -> u64 { + bytes.div_ceil(1024 * 1024) +} + +/// The provider-side name of a run's sandbox. +pub(crate) fn run_name(run_id: &RunId) -> String { + format!("fabro-run-{run_id}") +} + +/// The environment's default `allow_all` means "unrestricted", which a +/// provider without network controls already is; asking such a provider +/// for it explicitly would be rejected. An explicit restriction is still +/// requested, and refused by the provider when it cannot honor it. +pub(crate) fn supported_network( + requested: NetworkPolicy, + capabilities: &Capabilities, +) -> NetworkPolicy { + match requested { + NetworkPolicy::AllowAll if !capabilities.network.allow_all => { + NetworkPolicy::ProviderDefault + } + other => other, + } +} + +/// The environment's auto-stop is a request a backend without timers +/// cannot take; such a provider gets no timers rather than a rejected spec. +pub(crate) fn supported_timers( + requested: LifecycleTimers, + capabilities: &Capabilities, +) -> LifecycleTimers { + if capabilities.lifecycle.timers { + requested + } else { + LifecycleTimers::default() + } +} + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + use std::time::Duration; + + use fabro_types::SandboxProviderKind; + use fabro_types::settings::run::{ + EnvironmentImageSettings, EnvironmentLifecycleSettings, EnvironmentNetworkSettings, + EnvironmentResourcesSettings, + }; + use fabro_types::settings::{Duration as SettingsDuration, Size}; + + use super::*; + + fn environment(kind: &str) -> RunEnvironmentSettings { + RunEnvironmentSettings { + id: kind.to_string(), + provider: SandboxProviderKind::try_new(kind).unwrap(), + cwd: None, + image: EnvironmentImageSettings::default(), + resources: EnvironmentResourcesSettings::default(), + network: EnvironmentNetworkSettings::default(), + lifecycle: EnvironmentLifecycleSettings::default(), + labels: HashMap::from([("team".to_string(), "platform".to_string())]), + env: HashMap::new(), + } + } + + #[test] + fn an_environment_without_an_image_asks_for_a_managed_directory() { + let spec = sandbox_spec_for_environment( + &environment("host"), + BTreeMap::from([("FOO".to_string(), "bar".to_string())]), + ) + .unwrap(); + assert!(matches!(spec.source, SandboxSource::HostDirectory)); + assert!(spec.working_directory.is_none()); + assert!( + spec.name.is_none(), + "the run names the sandbox, not the environment" + ); + assert_eq!(spec.env.get("FOO").map(String::as_str), Some("bar")); + assert_eq!( + spec.labels.get("team").map(String::as_str), + Some("platform") + ); + assert!( + !spec.labels.contains_key("sh.fabro.managed"), + "ownership labels come from the scope, not the environment" + ); + assert!(matches!(spec.network, NetworkPolicy::AllowAll)); + assert_eq!(spec.resources, Resources::default()); + assert_eq!(spec.timers, LifecycleTimers::default()); + } + + #[test] + fn an_environment_with_an_image_maps_resources_network_and_lifecycle() { + let mut settings = environment("e2b"); + settings.image.docker = Some("ubuntu:24.04".to_string()); + settings.resources.cpu = Some(2); + settings.resources.memory = Some(Size::from_bytes(4_000_000_000)); + settings.network.mode = EnvironmentNetworkMode::Block; + settings.lifecycle.auto_stop = Some(SettingsDuration::from_std(Duration::from_mins(45))); + + let spec = sandbox_spec_for_environment(&settings, BTreeMap::new()).unwrap(); + assert!(matches!( + &spec.source, + SandboxSource::Image { reference } if reference == "ubuntu:24.04" + )); + assert_eq!(spec.resources.cpu_cores, Some(2)); + assert_eq!(spec.resources.memory_mb, Some(3815)); + assert!(matches!(spec.network, NetworkPolicy::Block)); + assert_eq!( + spec.timers.auto_stop_after_idle, + Some(Duration::from_mins(45)) + ); + } + + #[test] + fn the_clone_request_carries_the_environments_policy() { + let clone = CloneRequest::from_settings(&RunCloneSettings::default()); + assert_eq!(clone.depth, Some(100)); + assert!(!clone.skip); + + let clone = CloneRequest::from_settings(&RunCloneSettings { + enabled: false, + depth: 0, + }); + assert_eq!(clone.depth, None); + assert!(clone.skip); + assert!(CloneRequest::none().skip); + } + + #[test] + fn an_inline_dockerfile_becomes_the_source_and_a_path_is_rejected() { + let mut settings = environment("daytona"); + settings.image.dockerfile = Some(DockerfileSource::Inline("FROM ubuntu".to_string())); + let spec = sandbox_spec_for_environment(&settings, BTreeMap::new()).unwrap(); + assert!(matches!( + spec.source, + SandboxSource::Dockerfile { content } if content == "FROM ubuntu" + )); + + settings.image.dockerfile = Some(DockerfileSource::Path { + path: "Dockerfile".to_string(), + }); + let error = sandbox_spec_for_environment(&settings, BTreeMap::new()).unwrap_err(); + assert!(error.to_string().contains("Dockerfile path"), "{error}"); + } + + #[test] + fn allow_all_falls_back_to_the_provider_default_without_network_control() { + let none = Capabilities::minimal(sandbox_driver::Isolation::None); + assert!(matches!( + supported_network(NetworkPolicy::AllowAll, &none), + NetworkPolicy::ProviderDefault + )); + assert!(matches!( + supported_network(NetworkPolicy::Block, &none), + NetworkPolicy::Block + )); + let mut full = Capabilities::minimal(sandbox_driver::Isolation::Container); + full.network.allow_all = true; + assert!(matches!( + supported_network(NetworkPolicy::AllowAll, &full), + NetworkPolicy::AllowAll + )); + } + + #[test] + fn timers_are_dropped_for_a_provider_without_them() { + let mut requested = LifecycleTimers::default(); + requested.auto_stop_after_idle = Some(Duration::from_mins(45)); + let none = Capabilities::minimal(sandbox_driver::Isolation::None); + assert_eq!( + supported_timers(requested, &none), + LifecycleTimers::default() + ); + let mut with_timers = Capabilities::minimal(sandbox_driver::Isolation::Container); + with_timers.lifecycle.timers = true; + assert_eq!(supported_timers(requested, &with_timers), requested); + } +} diff --git a/lib/components/fabro-sandbox/src/lib.rs b/lib/components/fabro-sandbox/src/lib.rs index e5524c132..268b3c94c 100644 --- a/lib/components/fabro-sandbox/src/lib.rs +++ b/lib/components/fabro-sandbox/src/lib.rs @@ -1,5 +1,5 @@ +pub mod environment; pub mod error; -pub mod options; pub mod provider; pub mod sandbox; pub mod sandbox_spec; @@ -36,6 +36,7 @@ pub use details::sandbox_details; pub use docker::check_docker_daemon; pub use driver::{DaytonaCredentials, ProviderAccess}; pub use driver_sandbox::{RunSandbox, local_sandbox}; +pub use environment::{CloneRequest, sandbox_spec_for_environment}; pub use error::{Error, Result, default_redacted_output_tail, display_for_log}; pub use exec::{ DEFAULT_RETAINED_OUTPUT_BYTES, DEFAULT_STOP_GRACE, ExecResultExt, ExplicitEnvPolicy, @@ -48,10 +49,6 @@ pub use fabro_types::{RunSandboxInstance, SandboxProviderKind}; pub use git_retry::{ CredentialContext, GitRetryReason, RetryPlan, classify_failure, retry_git_operation, }; -pub use options::{ - SandboxOptions, local_working_directory_from_environment, options_from_environment, - unresolved_env, -}; pub use provider::{SandboxInventory, SandboxLookupError}; pub use provider_sandbox::{attach_provider_sandbox, provider_sandbox}; pub use push_credentials::RefreshErrorKind; @@ -65,12 +62,13 @@ pub use sandbox::{ format_lines_numbered, redacted_output_tail, setup_git, shell_quote, }; /// Driver types a run sandbox speaks: what a command is and how it ended, -/// what the file and search operations return, and the network policy a -/// [`SandboxOptions`] asks for. Re-exported so consumers need no direct -/// driver dependency. +/// what the file and search operations return, and what an environment +/// asks of a sandbox. Re-exported so consumers need no direct driver +/// dependency. pub use sandbox_driver::{ CaptureStats, DirEntry, ExecControls, ExecFailure, ExecResult, ExecSpec, ExecStreamingResult, - FileKind, GrepMatch, GrepOptions, NetworkPolicy, OutputSink, OutputStream, PtySession, PtySize, - StderrTail, StdioProcess, StdioProcessHandle, Termination, TransportError, WalkOptions, + FileKind, GrepMatch, GrepOptions, LifecycleTimers, NetworkPolicy, OutputSink, OutputStream, + PtySession, PtySize, Resources, SandboxSource, SandboxSpec as DriverSpec, StderrTail, + StdioProcess, StdioProcessHandle, Termination, TransportError, WalkOptions, }; pub use sandbox_spec::{ProviderSandboxSpec, SandboxSpec}; diff --git a/lib/components/fabro-sandbox/src/options.rs b/lib/components/fabro-sandbox/src/options.rs deleted file mode 100644 index f192e534a..000000000 --- a/lib/components/fabro-sandbox/src/options.rs +++ /dev/null @@ -1,391 +0,0 @@ -//! What an environment asks of a sandbox, mapped once for every provider. -//! -//! The environment names an image or Dockerfile, resources, a network -//! policy, labels, variables, a lifecycle, and a clone policy. Every -//! provider consumes the same [`SandboxOptions`]: the driver spec is built -//! from them in one place, and a bundled provider adds only what its -//! backend needs on top (the Docker working directory, the Daytona -//! snapshot and timers) in its own overlay. - -use std::collections::BTreeMap; -use std::path::{Path, PathBuf}; -use std::time::Duration; - -use fabro_types::RunId; -use fabro_types::settings::run::{ - DockerfileSource, EnvironmentNetworkMode, RunCloneSettings, RunEnvironmentSettings, -}; -use sandbox_driver::{ - Capabilities, NetworkPolicy, Resources, SandboxSource, SandboxSpec as DriverSpec, -}; - -/// What an environment asks of a sandbox, provider-neutral. -#[derive(Clone, Debug, Default)] -pub struct SandboxOptions { - /// Image reference, when the environment names one. - pub image: Option, - /// Inline Dockerfile, when the environment names one instead of an - /// image. - pub dockerfile: Option, - /// Environment variables for the sandbox, resolved. - pub env: BTreeMap, - pub network: NetworkPolicy, - pub cpu: Option, - pub memory_bytes: Option, - pub disk_bytes: Option, - /// Labels from the environment; fabro's managed labels are added. - pub labels: BTreeMap, - /// Idle time before the provider stops the sandbox, when the - /// environment sets one. - pub auto_stop: Option, - /// Maximum Git history depth fetched during clone; `None` fetches full - /// history. - pub clone_depth: Option, - /// Create an empty workspace instead of cloning even when an origin - /// exists. - pub skip_clone: bool, -} - -impl SandboxOptions { - /// Memory in whole mebibytes, rounded up. - pub fn memory_mb(&self) -> Option { - self.memory_bytes.map(|bytes| bytes.div_ceil(1024 * 1024)) - } - - /// Disk in whole mebibytes, rounded up. - pub fn disk_mb(&self) -> Option { - self.disk_bytes.map(|bytes| bytes.div_ceil(1024 * 1024)) - } -} - -/// Maps resolved environment settings onto sandbox options. `env` is the -/// environment's variables, resolved by the caller: the worker resolves -/// secrets through the vault, while preflight carries them in source form. -/// -/// A Dockerfile given as a path must have been resolved to inline content -/// earlier; none of the providers can read a path. -pub fn options_from_environment( - settings: &RunEnvironmentSettings, - clone: &RunCloneSettings, - env: BTreeMap, -) -> crate::Result { - // fabro-config rejects environments that set both image.docker and - // image.dockerfile. If both still arrive here, the image wins. - let dockerfile = match (&settings.image.docker, &settings.image.dockerfile) { - (Some(_), _) | (None, None) => None, - (None, Some(DockerfileSource::Inline(content))) => Some(content.clone()), - (None, Some(DockerfileSource::Path { path })) => { - return Err(crate::Error::message(format!( - "environment `{}` names a Dockerfile path ({path}) that should have been \ - resolved to inline content before sandbox creation", - settings.id - ))); - } - }; - Ok(SandboxOptions { - image: settings.image.docker.clone(), - dockerfile, - env, - network: match settings.network.mode { - EnvironmentNetworkMode::Block => NetworkPolicy::Block, - EnvironmentNetworkMode::AllowAll => NetworkPolicy::AllowAll, - EnvironmentNetworkMode::CidrAllowList => NetworkPolicy::CidrAllowList { - cidrs: settings.network.allow.clone(), - }, - }, - cpu: settings - .resources - .cpu - .and_then(|cpu| u32::try_from(cpu).ok()), - memory_bytes: settings.resources.memory.map(|size| size.as_bytes()), - disk_bytes: settings.resources.disk.map(|size| size.as_bytes()), - labels: settings - .labels - .iter() - .map(|(key, value)| (key.clone(), value.clone())) - .collect(), - auto_stop: settings - .lifecycle - .auto_stop - .map(|duration| duration.as_std()), - clone_depth: clone - .depth_limit() - .and_then(|depth| u32::try_from(depth).ok()), - skip_clone: !clone.enabled, - }) -} - -/// The environment's variables in source form, for a path with no vault -/// (server preflight): a `{{ secrets.* }}` value keeps its token, and -/// nothing else is left to resolve because `{{ vars.* }}` is substituted at -/// run creation. -pub fn unresolved_env(settings: &RunEnvironmentSettings) -> BTreeMap { - #[expect( - clippy::disallowed_methods, - reason = "preflight has no vault, so an unresolved secret token is carried in source form" - )] - settings - .env - .iter() - .map(|(key, value)| (key.clone(), value.as_source())) - .collect() -} - -pub fn local_working_directory_from_environment( - settings: &RunEnvironmentSettings, - source_directory: Option<&Path>, -) -> crate::Result { - if let Some(cwd) = settings.cwd.as_deref() { - return Ok(PathBuf::from(cwd)); - } - - let Some(source_directory) = source_directory else { - return Err(crate::Error::message( - "local environment requires a server-side working directory; configure `environment.cwd = \"/absolute/path\"` on the selected local environment", - )); - }; - - if source_directory.is_dir() { - return Ok(source_directory.to_path_buf()); - } - - Err(crate::Error::message(format!( - "local environment source_directory does not exist or is not a directory on this server: {}. Configure `environment.cwd = \"/absolute/path\"` on the selected local environment for remote client/server deployments.", - source_directory.display() - ))) -} - -/// The driver spec every provider starts from: the environment's source -/// (an image, a Dockerfile, or a managed directory when it names -/// neither), the run's name, the environment's labels, variables, -/// resources, and network policy. A bundled provider's overlay adjusts -/// what its backend needs, and the ownership scope adds fabro's labels. -pub(crate) fn base_spec(options: &SandboxOptions, run_id: Option<&RunId>) -> DriverSpec { - let source = match (&options.image, &options.dockerfile) { - (Some(reference), _) => SandboxSource::Image { - reference: reference.clone(), - }, - (None, Some(content)) => SandboxSource::Dockerfile { - content: content.clone(), - }, - // A provider without images (a host-style plugin) manages a - // workspace directory of its own. - (None, None) => SandboxSource::HostDirectory, - }; - let mut spec = DriverSpec::new(source).network(options.network.clone()); - if let Some(run_id) = run_id { - spec = spec.name(run_name(run_id)); - } - // The environment's labels; fabro's ownership labels are stamped by the - // ownership scope the provider is connected through. - for (key, value) in &options.labels { - spec = spec.label(key, value); - } - for (key, value) in &options.env { - spec = spec.env_var(key, value); - } - let mut resources = Resources::default(); - resources.cpu_cores = options.cpu; - resources.memory_mb = options.memory_mb(); - resources.disk_mb = options.disk_mb(); - spec.resources(resources) -} - -/// The provider-side name of a run's sandbox. -pub(crate) fn run_name(run_id: &RunId) -> String { - format!("fabro-run-{run_id}") -} - -/// The environment's default `allow_all` means "unrestricted", which a -/// provider without network controls already is; asking such a provider -/// for it explicitly would be rejected. An explicit restriction is still -/// requested, and refused by the provider when it cannot honor it. -pub(crate) fn supported_network( - requested: NetworkPolicy, - capabilities: &Capabilities, -) -> NetworkPolicy { - match requested { - NetworkPolicy::AllowAll if !capabilities.network.allow_all => { - NetworkPolicy::ProviderDefault - } - other => other, - } -} - -#[cfg(test)] -mod tests { - use std::collections::HashMap; - - use fabro_types::SandboxProviderKind; - use fabro_types::settings::run::{ - EnvironmentImageSettings, EnvironmentLifecycleSettings, EnvironmentNetworkSettings, - EnvironmentResourcesSettings, - }; - use fabro_types::settings::{Duration as SettingsDuration, Size}; - - use super::*; - - fn environment(kind: &str) -> RunEnvironmentSettings { - RunEnvironmentSettings { - id: kind.to_string(), - provider: SandboxProviderKind::try_new(kind).unwrap(), - cwd: None, - image: EnvironmentImageSettings::default(), - resources: EnvironmentResourcesSettings::default(), - network: EnvironmentNetworkSettings::default(), - lifecycle: EnvironmentLifecycleSettings::default(), - labels: HashMap::from([("team".to_string(), "platform".to_string())]), - env: HashMap::new(), - } - } - - fn run_id() -> RunId { - "01HY0000000000000000000000".parse().unwrap() - } - - #[test] - fn options_without_an_image_ask_for_a_managed_directory() { - let options = options_from_environment( - &environment("host"), - &RunCloneSettings::default(), - BTreeMap::from([("FOO".to_string(), "bar".to_string())]), - ) - .unwrap(); - assert!(options.image.is_none()); - assert!(options.dockerfile.is_none()); - assert_eq!(options.clone_depth, Some(100)); - assert!(!options.skip_clone); - assert!(options.auto_stop.is_none()); - - let spec = base_spec(&options, Some(&run_id())); - assert!(matches!(spec.source, SandboxSource::HostDirectory)); - assert!(spec.working_directory.is_none()); - assert_eq!( - spec.name.as_deref(), - Some("fabro-run-01HY0000000000000000000000") - ); - assert_eq!(spec.env.get("FOO").map(String::as_str), Some("bar")); - assert_eq!( - spec.labels.get("team").map(String::as_str), - Some("platform") - ); - assert!( - !spec.labels.contains_key("sh.fabro.managed"), - "ownership labels come from the scope, not the environment" - ); - assert!(matches!(spec.network, NetworkPolicy::AllowAll)); - } - - #[test] - fn options_with_an_image_map_resources_network_and_lifecycle() { - let mut settings = environment("e2b"); - settings.image.docker = Some("ubuntu:24.04".to_string()); - settings.resources.cpu = Some(2); - settings.resources.memory = Some(Size::from_bytes(4_000_000_000)); - settings.network.mode = EnvironmentNetworkMode::Block; - settings.lifecycle.auto_stop = Some(SettingsDuration::from_std(Duration::from_mins(45))); - let clone = RunCloneSettings { - enabled: false, - depth: 0, - }; - let options = options_from_environment(&settings, &clone, BTreeMap::new()).unwrap(); - assert_eq!(options.image.as_deref(), Some("ubuntu:24.04")); - assert!(options.skip_clone); - assert_eq!(options.clone_depth, None); - assert_eq!(options.memory_bytes, Some(4_000_000_000)); - assert_eq!(options.memory_mb(), Some(3815)); - assert_eq!(options.auto_stop, Some(Duration::from_mins(45))); - - let spec = base_spec(&options, None); - assert!(matches!( - &spec.source, - SandboxSource::Image { reference } if reference == "ubuntu:24.04" - )); - assert_eq!(spec.resources.cpu_cores, Some(2)); - assert_eq!(spec.resources.memory_mb, Some(3815)); - assert!(matches!(spec.network, NetworkPolicy::Block)); - assert!(spec.name.is_none()); - } - - #[test] - fn an_inline_dockerfile_becomes_the_source_and_a_path_is_rejected() { - let mut settings = environment("daytona"); - settings.image.dockerfile = Some(DockerfileSource::Inline("FROM ubuntu".to_string())); - let options = - options_from_environment(&settings, &RunCloneSettings::default(), BTreeMap::new()) - .unwrap(); - assert_eq!(options.dockerfile.as_deref(), Some("FROM ubuntu")); - assert!(matches!( - base_spec(&options, None).source, - SandboxSource::Dockerfile { content } if content == "FROM ubuntu" - )); - - settings.image.dockerfile = Some(DockerfileSource::Path { - path: "Dockerfile".to_string(), - }); - let error = - options_from_environment(&settings, &RunCloneSettings::default(), BTreeMap::new()) - .unwrap_err(); - assert!(error.to_string().contains("Dockerfile path"), "{error}"); - } - - #[test] - fn allow_all_falls_back_to_the_provider_default_without_network_control() { - let none = Capabilities::minimal(sandbox_driver::Isolation::None); - assert!(matches!( - supported_network(NetworkPolicy::AllowAll, &none), - NetworkPolicy::ProviderDefault - )); - assert!(matches!( - supported_network(NetworkPolicy::Block, &none), - NetworkPolicy::Block - )); - let mut full = Capabilities::minimal(sandbox_driver::Isolation::Container); - full.network.allow_all = true; - assert!(matches!( - supported_network(NetworkPolicy::AllowAll, &full), - NetworkPolicy::AllowAll - )); - } - - #[test] - fn local_working_directory_prefers_environment_cwd() { - let mut settings = environment("local"); - settings.cwd = Some("/srv/fabro/workspaces/team-a".to_string()); - let missing_source = Path::new("/path/that/should/not/exist"); - - let resolved = local_working_directory_from_environment(&settings, Some(missing_source)) - .expect("configured cwd should be accepted"); - - assert_eq!(resolved, PathBuf::from("/srv/fabro/workspaces/team-a")); - assert!(!missing_source.exists()); - } - - #[test] - fn local_working_directory_uses_existing_source_directory_without_cwd() { - let settings = environment("local"); - let dir = tempfile::tempdir().unwrap(); - - let resolved = local_working_directory_from_environment(&settings, Some(dir.path())) - .expect("existing source directory should be accepted"); - - assert_eq!(resolved, dir.path()); - } - - #[test] - fn local_working_directory_rejects_missing_source_directory_without_cwd() { - let settings = environment("local"); - let dir = tempfile::tempdir().unwrap(); - let missing = dir.path().join("client-only"); - - let err = local_working_directory_from_environment(&settings, Some(&missing)) - .expect_err("missing source directory without cwd should fail"); - - let message = err.to_string(); - assert!( - message.contains("environment.cwd") && message.contains("does not exist"), - "unexpected error: {message}" - ); - assert!(!missing.exists()); - } -} diff --git a/lib/components/fabro-sandbox/src/provider_sandbox.rs b/lib/components/fabro-sandbox/src/provider_sandbox.rs index 65e02add6..aa5069d79 100644 --- a/lib/components/fabro-sandbox/src/provider_sandbox.rs +++ b/lib/components/fabro-sandbox/src/provider_sandbox.rs @@ -1,60 +1,49 @@ //! Run sandboxes on any provider fabro can name: a bundled kind in process //! or a sandbox-driver plugin executable. //! -//! One path builds them all. The environment's [`SandboxOptions`] become -//! the driver spec once, the provider is connected through the single +//! One path builds them all. The environment's spec arrives built (see +//! [`crate::environment`]), the provider is connected through the single //! construction function, and a bundled provider adds only what its //! backend needs on top: Docker its fixed working directory and default //! image, Daytona the snapshot it creates sandboxes from and its lifecycle -//! timers. A plugin gets the spec as is, laid out inside the working -//! directory the provider chooses. +//! timers. A plugin gets the spec as is, trimmed to what it can honor, laid +//! out inside the working directory the provider chooses. use std::sync::Arc; use fabro_github::GitHubCredentials; use fabro_types::{BundledProvider, RunId, SandboxProviderKind}; -use sandbox_driver::{EventContext, OwnedProvider, SandboxId, SandboxProvider}; +use sandbox_driver::{ + EventContext, OwnedProvider, SandboxId, SandboxProvider, SandboxSource, + SandboxSpec as DriverSpec, +}; use crate::driver::{ProviderAccess, connect_provider}; use crate::driver_sandbox::{LayoutSource, RepoWorkspace, RunSandbox}; -use crate::options::{self, SandboxOptions}; +use crate::environment::{self, CloneRequest}; use crate::{daytona, docker, managed_labels}; /// A sandbox for a run on `kind`. The sandbox is created by `initialize`; /// construction validates the clone request and connects the provider, so -/// a bad spec, a missing credential, or a missing plugin executable fails -/// before any backend call. -#[expect( - clippy::too_many_arguments, - reason = "mirrors SandboxSpec::Provider; clone inputs are validated together" -)] +/// a bad request, a missing credential, or a missing plugin executable +/// fails before any backend call. pub async fn provider_sandbox( kind: SandboxProviderKind, access: &ProviderAccess, - options: SandboxOptions, + spec: DriverSpec, + clone: &CloneRequest, github_app: Option<&GitHubCredentials>, run_id: Option, - clone_origin_url: Option, - clone_branch: Option, - clone_tag: Option, - clone_commit_sha: Option, ) -> crate::Result { - let workspace = RepoWorkspace::plan( - layout_source(&kind), - options.skip_clone, - clone_origin_url.as_deref(), - clone_branch.as_deref(), - clone_tag.as_deref(), - clone_commit_sha.as_deref(), - options.clone_depth, - github_app, - )?; + let workspace = RepoWorkspace::plan(layout_source(&kind), clone, github_app)?; let provider = connect(&kind, access, run_id.as_ref()).await?; - let base = options::base_spec(&options, run_id.as_ref()); + let mut spec = spec; + if let Some(run_id) = &run_id { + spec = spec.name(environment::run_name(run_id)); + } Ok(match kind.bundled() { Some(BundledProvider::Docker) => { - let (spec, _image) = docker::overlay(base, &options); - RunSandbox::pending(kind, provider, spec, workspace) + RunSandbox::pending(kind, provider, docker::overlay(spec), workspace) } Some(BundledProvider::Daytona) => { let credentials = access @@ -64,8 +53,7 @@ pub async fn provider_sandbox( let plan = daytona::create_plan( Arc::clone(&provider), credentials.api_key.clone(), - base, - options, + spec, run_id, ); RunSandbox::pending_with_plan(kind, provider, Box::new(plan), workspace) @@ -76,8 +64,9 @@ pub async fn provider_sandbox( )); } None => { - let mut spec = base; - spec.network = options::supported_network(spec.network, provider.capabilities()); + let capabilities = provider.capabilities(); + spec.network = environment::supported_network(spec.network, capabilities); + spec.timers = environment::supported_timers(spec.timers, capabilities); RunSandbox::pending(kind, provider, spec, workspace) } }) @@ -128,13 +117,11 @@ pub async fn attach_provider_sandbox( /// The image the run record names for a sandbox on `kind`: the /// environment's, or Docker's default when the environment names none. -pub(crate) fn recorded_image( - kind: &SandboxProviderKind, - options: &SandboxOptions, -) -> Option { - match kind.bundled() { - Some(BundledProvider::Docker) => Some(docker::effective_image(options)), - _ => options.image.clone(), +pub(crate) fn recorded_image(kind: &SandboxProviderKind, spec: &DriverSpec) -> Option { + match (kind.bundled(), &spec.source) { + (Some(BundledProvider::Docker), _) => Some(docker::effective_image(spec)), + (_, SandboxSource::Image { reference }) => Some(reference.clone()), + _ => None, } } diff --git a/lib/components/fabro-sandbox/src/sandbox_spec.rs b/lib/components/fabro-sandbox/src/sandbox_spec.rs index 0efe6611e..8345e1bb3 100644 --- a/lib/components/fabro-sandbox/src/sandbox_spec.rs +++ b/lib/components/fabro-sandbox/src/sandbox_spec.rs @@ -4,11 +4,11 @@ use std::sync::Arc; use anyhow::Context as _; use fabro_github::GitHubCredentials; use fabro_types::{RunId, RunSandboxInstance, RunSandboxRuntime, SandboxProviderKind}; -use sandbox_driver::EventContext; +use sandbox_driver::{EventContext, SandboxSpec as DriverSpec}; use crate::driver::ProviderAccess; use crate::driver_sandbox::{LayoutSource, RunSandbox, local_sandbox_with_events}; -use crate::options::SandboxOptions; +use crate::environment::CloneRequest; use crate::{clone_source, provider_sandbox}; /// Options for sandbox initialization and construction. @@ -26,16 +26,15 @@ pub enum SandboxSpec { /// the repository is cloned into it. #[derive(Clone, Debug)] pub struct ProviderSandboxSpec { - pub kind: SandboxProviderKind, + pub kind: SandboxProviderKind, /// The provider settings and vault credentials the kind needs. - pub access: ProviderAccess, - pub options: SandboxOptions, - pub github_app: Option, - pub run_id: Option, - pub clone_origin_url: Option, - pub clone_branch: Option, - pub clone_tag: Option, - pub clone_commit_sha: Option, + pub access: ProviderAccess, + /// The environment's request, as the driver spec every provider + /// starts from. + pub spec: DriverSpec, + pub clone: CloneRequest, + pub github_app: Option, + pub run_id: Option, } impl SandboxSpec { @@ -56,7 +55,7 @@ impl SandboxSpec { pub fn image(&self) -> Option { match self { Self::Local { .. } => None, - Self::Provider(spec) => provider_sandbox::recorded_image(&spec.kind, &spec.options), + Self::Provider(spec) => provider_sandbox::recorded_image(&spec.kind, &spec.spec), } } @@ -79,16 +78,11 @@ impl SandboxSpec { match self { Self::Provider(spec) => { let ProviderSandboxSpec { - kind, - options, - clone_origin_url, - clone_branch, - .. + kind, spec, clone, .. } = spec.as_ref(); - let repo_cloned = clone_source::repo_cloned_for_record( - options.skip_clone, - clone_origin_url.as_deref(), - ); + let clone_origin_url = &clone.origin_url; + let repo_cloned = + clone_source::repo_cloned_for_record(clone.skip, clone_origin_url.as_deref()); // A fixed layout is known before the sandbox exists; a // provider-chosen one only from the sandbox. let layout = match provider_sandbox::layout_source(kind) { @@ -114,7 +108,7 @@ impl SandboxSpec { }; RunSandboxInstance { provider: kind.clone(), - image: provider_sandbox::recorded_image(kind, options), + image: provider_sandbox::recorded_image(kind, spec), snapshot: sandbox.snapshot_info(), runtime: RunSandboxRuntime { id, @@ -123,7 +117,7 @@ impl SandboxSpec { clone_origin_url: clone_source::clean_clone_origin_for_record( clone_origin_url.as_deref(), ), - clone_branch: clone_branch.clone(), + clone_branch: clone.branch.clone(), workspace_root: layout.as_ref().map(|layout| layout.workspace_root.clone()), repos_root: layout.as_ref().map(|layout| layout.repos_root.clone()), primary_repo_path: layout @@ -172,24 +166,18 @@ impl SandboxSpec { let ProviderSandboxSpec { kind, access, - options, + spec, + clone, github_app, run_id, - clone_origin_url, - clone_branch, - clone_tag, - clone_commit_sha, } = spec.as_ref(); let mut sandbox = provider_sandbox::provider_sandbox( kind.clone(), access, - options.clone(), + spec.clone(), + clone, github_app.as_ref(), *run_id, - clone_origin_url.clone(), - clone_branch.clone(), - clone_tag.clone(), - clone_commit_sha.clone(), ) .await .with_context(|| format!("Failed to create {kind} sandbox"))?; @@ -217,10 +205,22 @@ fn runtime_layout_metadata( #[cfg(test)] mod tests { use fabro_types::RunId; + use sandbox_driver::SandboxSource; use sandbox_driver_testing::ScriptedSandbox; use super::*; + fn provider_spec(clone: CloneRequest) -> ProviderSandboxSpec { + ProviderSandboxSpec { + kind: SandboxProviderKind::DOCKER, + access: ProviderAccess::default(), + spec: DriverSpec::new(SandboxSource::HostDirectory), + clone, + github_app: None, + run_id: None, + } + } + fn sandbox_at(working_dir: &str) -> RunSandbox { RunSandbox::new( SandboxProviderKind::DOCKER, @@ -233,17 +233,11 @@ mod tests { #[test] fn docker_run_sandbox_persists_layout_metadata_for_cloned_repo() { - let spec = SandboxSpec::Provider(Box::new(ProviderSandboxSpec { - kind: SandboxProviderKind::DOCKER, - access: ProviderAccess::default(), - options: SandboxOptions::default(), - github_app: None, - run_id: None, - clone_origin_url: Some("git@github.com:brynary/rack-test.git".to_string()), - clone_branch: Some("main".to_string()), - clone_tag: None, - clone_commit_sha: None, - })); + let spec = SandboxSpec::Provider(Box::new(provider_spec(CloneRequest { + origin_url: Some("git@github.com:brynary/rack-test.git".to_string()), + branch: Some("main".to_string()), + ..CloneRequest::default() + }))); let sandbox = sandbox_at("/workspace/rack-test"); let run_id: RunId = "01HY0000000000000000000000".parse().unwrap(); @@ -272,17 +266,12 @@ mod tests { #[tokio::test] async fn invalid_exact_checkout_spec_fails_before_provider_connection() { - let spec = SandboxSpec::Provider(Box::new(ProviderSandboxSpec { - kind: SandboxProviderKind::DOCKER, - access: ProviderAccess::default(), - options: SandboxOptions::default(), - github_app: None, - run_id: None, - clone_origin_url: Some("https://github.com/acme/widgets".to_string()), - clone_branch: Some("main".to_string()), - clone_tag: None, - clone_commit_sha: Some("not-a-sha".to_string()), - })); + let spec = SandboxSpec::Provider(Box::new(provider_spec(CloneRequest { + origin_url: Some("https://github.com/acme/widgets".to_string()), + branch: Some("main".to_string()), + commit_sha: Some("not-a-sha".to_string()), + ..CloneRequest::default() + }))); let error = spec .build(None) @@ -300,20 +289,10 @@ mod tests { #[test] fn docker_run_sandbox_omits_primary_repo_metadata_for_empty_workspace() { - let spec = SandboxSpec::Provider(Box::new(ProviderSandboxSpec { - kind: SandboxProviderKind::DOCKER, - access: ProviderAccess::default(), - options: SandboxOptions { - skip_clone: true, - ..SandboxOptions::default() - }, - github_app: None, - run_id: None, - clone_origin_url: Some("https://gitlab.com/acme/widgets".to_string()), - clone_branch: None, - clone_tag: None, - clone_commit_sha: None, - })); + let spec = SandboxSpec::Provider(Box::new(provider_spec(CloneRequest { + origin_url: Some("https://gitlab.com/acme/widgets".to_string()), + ..CloneRequest::none() + }))); let sandbox = sandbox_at("/workspace"); let run_id: RunId = "01HY0000000000000000000000".parse().unwrap(); diff --git a/lib/components/fabro-sandbox/tests/daytona_streaming_live.rs b/lib/components/fabro-sandbox/tests/daytona_streaming_live.rs index d5dbfd1c5..2d43e1d83 100644 --- a/lib/components/fabro-sandbox/tests/daytona_streaming_live.rs +++ b/lib/components/fabro-sandbox/tests/daytona_streaming_live.rs @@ -4,11 +4,12 @@ mod daytona_streaming_live { use anyhow::{Context, Result, ensure}; use fabro_sandbox::{ - DaytonaCredentials, ExecControls, ExecSpec, ExecStreamingResult, OutputSink, OutputStream, - ProviderAccess, RunSandbox, SandboxOptions, SandboxProviderKind, Termination, + CloneRequest, DaytonaCredentials, ExecControls, ExecSpec, ExecStreamingResult, OutputSink, + OutputStream, ProviderAccess, RunSandbox, SandboxProviderKind, Termination, provider_sandbox, }; use fabro_static::EnvVars; + use sandbox_driver::{SandboxSource, SandboxSpec}; use tokio::sync::Mutex; use tokio::time::{Instant, sleep}; use tokio_util::sync::CancellationToken; @@ -31,14 +32,8 @@ mod daytona_streaming_live { provider_sandbox( SandboxProviderKind::DAYTONA, &daytona_access(live_credentials()?), - SandboxOptions { - skip_clone: true, - ..Default::default() - }, - None, - None, - None, - None, + SandboxSpec::new(SandboxSource::HostDirectory), + &CloneRequest::none(), None, None, ) @@ -71,14 +66,8 @@ mod daytona_streaming_live { let sandbox = provider_sandbox( SandboxProviderKind::DAYTONA, &daytona_access(live_credentials()?), - SandboxOptions { - skip_clone: true, - ..Default::default() - }, - None, - None, - None, - None, + SandboxSpec::new(SandboxSource::HostDirectory), + &CloneRequest::none(), None, None, ) @@ -177,20 +166,11 @@ mod daytona_streaming_live { let sandbox = provider_sandbox( SandboxProviderKind::DAYTONA, &daytona_access(live_credentials()?), - SandboxOptions { - skip_clone: true, - labels: std::collections::BTreeMap::from([( - "team".to_string(), - "platform".to_string(), - )]), - ..Default::default() - }, + SandboxSpec::new(SandboxSource::HostDirectory) + .label("team".to_string(), "platform".to_string()), + &CloneRequest::none(), None, Some(run_id), - None, - None, - None, - None, ) .await?; @@ -235,16 +215,13 @@ mod daytona_streaming_live { let sandbox = provider_sandbox( SandboxProviderKind::DAYTONA, &daytona_access(live_credentials()?), - SandboxOptions { - skip_clone: false, - ..Default::default() + SandboxSpec::new(SandboxSource::HostDirectory), + &CloneRequest { + origin_url: Some("https://github.com/brynary/rack-test".to_string()), + ..CloneRequest::default() }, None, None, - Some("https://github.com/brynary/rack-test".to_string()), - None, - None, - None, ) .await?; @@ -305,14 +282,8 @@ mod daytona_streaming_live { let sandbox = provider_sandbox( SandboxProviderKind::DAYTONA, &daytona_access(live_credentials()?), - SandboxOptions { - skip_clone: true, - ..Default::default() - }, - None, - None, - None, - None, + SandboxSpec::new(SandboxSource::HostDirectory), + &CloneRequest::none(), None, None, ) diff --git a/lib/components/fabro-sandbox/tests/docker_streaming.rs b/lib/components/fabro-sandbox/tests/docker_streaming.rs index b99597c7e..37e196fb1 100644 --- a/lib/components/fabro-sandbox/tests/docker_streaming.rs +++ b/lib/components/fabro-sandbox/tests/docker_streaming.rs @@ -1,13 +1,13 @@ //! Docker sandbox behaviour through the sandbox-driver Docker provider. -use std::collections::BTreeMap; use std::sync::Arc; use std::time::Duration; use fabro_sandbox::{ - ExecControls, ExecSpec, OutputSink, ProviderAccess, SandboxOptions, SandboxProviderKind, + CloneRequest, ExecControls, ExecSpec, OutputSink, ProviderAccess, SandboxProviderKind, Termination, provider_sandbox, }; +use sandbox_driver::{SandboxSource, SandboxSpec}; use tokio::process::Command; use tokio::sync::Mutex; @@ -44,15 +44,10 @@ async fn streaming_timeout_terminates_docker_exec_before_returning() { let sandbox = provider_sandbox( SandboxProviderKind::DOCKER, &ProviderAccess::default(), - SandboxOptions { - image: Some(image.to_string()), - skip_clone: true, - ..SandboxOptions::default() - }, - None, - None, - None, - None, + SandboxSpec::new(SandboxSource::Image { + reference: image.to_string(), + }), + &CloneRequest::none(), None, None, ) @@ -119,15 +114,10 @@ async fn streaming_command_receives_exact_stdin_and_eof() { let sandbox = provider_sandbox( SandboxProviderKind::DOCKER, &ProviderAccess::default(), - SandboxOptions { - image: Some(image.to_string()), - skip_clone: true, - ..SandboxOptions::default() - }, - None, - None, - None, - None, + SandboxSpec::new(SandboxSource::Image { + reference: image.to_string(), + }), + &CloneRequest::none(), None, None, ) @@ -182,17 +172,15 @@ async fn cloned_docker_sandbox_uses_repos_checkout_and_workspace_symlink() { let sandbox = provider_sandbox( SandboxProviderKind::DOCKER, &ProviderAccess::default(), - SandboxOptions { - image: Some(image.to_string()), - skip_clone: false, - ..SandboxOptions::default() + SandboxSpec::new(SandboxSource::Image { + reference: image.to_string(), + }), + &CloneRequest { + origin_url: Some("https://github.com/brynary/rack-test".to_string()), + ..CloneRequest::default() }, None, None, - Some("https://github.com/brynary/rack-test".to_string()), - None, - None, - None, ) .await .expect("docker sandbox should construct"); @@ -248,16 +236,11 @@ async fn docker_runs_clean_bash_through_both_command_paths() { let sandbox = provider_sandbox( SandboxProviderKind::DOCKER, &ProviderAccess::default(), - SandboxOptions { - image: Some(image.to_string()), - env: BTreeMap::from([("BASH_ENV".to_string(), "/tmp/fabro-bash-env".to_string())]), - skip_clone: true, - ..SandboxOptions::default() - }, - None, - None, - None, - None, + SandboxSpec::new(SandboxSource::Image { + reference: image.to_string(), + }) + .env_var("BASH_ENV".to_string(), "/tmp/fabro-bash-env".to_string()), + &CloneRequest::none(), None, None, ) @@ -345,15 +328,10 @@ async fn docker_glob_matches_patterns_containing_a_path_separator() { let sandbox = provider_sandbox( SandboxProviderKind::DOCKER, &ProviderAccess::default(), - SandboxOptions { - image: Some(image.to_string()), - skip_clone: true, - ..SandboxOptions::default() - }, - None, - None, - None, - None, + SandboxSpec::new(SandboxSource::Image { + reference: image.to_string(), + }), + &CloneRequest::none(), None, None, ) @@ -430,15 +408,10 @@ async fn docker_runtime_directory_is_private_and_outside_workspace() { let sandbox = provider_sandbox( SandboxProviderKind::DOCKER, &ProviderAccess::default(), - SandboxOptions { - image: Some(image.to_string()), - skip_clone: true, - ..SandboxOptions::default() - }, - None, - None, - None, - None, + SandboxSpec::new(SandboxSource::Image { + reference: image.to_string(), + }), + &CloneRequest::none(), None, None, ) diff --git a/lib/components/fabro-sandbox/tests/driver_bench.rs b/lib/components/fabro-sandbox/tests/driver_bench.rs index 961db5b7d..4dab31c44 100644 --- a/lib/components/fabro-sandbox/tests/driver_bench.rs +++ b/lib/components/fabro-sandbox/tests/driver_bench.rs @@ -36,8 +36,7 @@ use std::sync::Arc; use std::time::{Duration, Instant}; use fabro_sandbox::{ - ProviderAccess, RunSandbox, SandboxOptions, SandboxProviderKind, local_sandbox, - provider_sandbox, + CloneRequest, ProviderAccess, RunSandbox, SandboxProviderKind, local_sandbox, provider_sandbox, }; use sandbox_driver::{ ExecSpec, GrepOptions, Sandbox as DriverHandle, SandboxProvider, SandboxSource, SandboxSpec, @@ -363,15 +362,10 @@ async fn agent_tool_call_latency_through_the_driver() { let fabro_docker = provider_sandbox( SandboxProviderKind::DOCKER, &ProviderAccess::default(), - SandboxOptions { - image: Some(IMAGE.to_owned()), - skip_clone: true, - ..SandboxOptions::default() - }, - None, - None, - None, - None, + SandboxSpec::new(SandboxSource::Image { + reference: IMAGE.to_owned(), + }), + &CloneRequest::none(), None, None, ) diff --git a/lib/components/fabro-workflow/src/operations/start.rs b/lib/components/fabro-workflow/src/operations/start.rs index 0d2050d61..657538022 100644 --- a/lib/components/fabro-workflow/src/operations/start.rs +++ b/lib/components/fabro-workflow/src/operations/start.rs @@ -10,8 +10,8 @@ use fabro_llm::credentials::readiness; use fabro_llm::lithos_catalog::Catalog; use fabro_mcp::config::McpServerSettings; use fabro_sandbox::{ - DaytonaCredentials, ProviderAccess, ProviderSandboxSpec, SandboxOptions, SandboxSpec, - local_working_directory_from_environment, options_from_environment, + CloneRequest, DaytonaCredentials, ProviderAccess, ProviderSandboxSpec, SandboxSpec, + sandbox_spec_for_environment, }; use fabro_static::EnvVars; #[cfg(test)] @@ -521,16 +521,15 @@ impl RunSession { working_directory: folder_working_directory_from_record(record, path).await?, }, None => { - let working_directory = local_working_directory_from_environment( - &resolved.environment, - record.source_directory.as_deref().map(Path::new), - ) - .map_err(|err| { - Error::engine_with_source( - "Failed to resolve local environment working directory", - err, - ) - })?; + let working_directory = resolved + .environment + .local_working_directory(record.source_directory.as_deref().map(Path::new)) + .map_err(|err| { + Error::engine_with_source( + "Failed to resolve local environment working directory", + err, + ) + })?; SandboxSpec::Local { working_directory } } }, @@ -542,18 +541,20 @@ impl RunSession { providers: services.sandbox_providers.clone(), daytona, }; - let mut options = resolve_sandbox_options(resolved, secret_lookup)?; - options.skip_clone |= clone_source.skip_clone; + let spec = resolve_sandbox_spec(resolved, secret_lookup)?; + let mut clone = CloneRequest::from_settings(&resolved.clone); + clone.skip |= clone_source.skip_clone; + clone.origin_url = clone_source.origin_url; + clone.branch = clone_source.branch; + clone.tag = clone_source.tag; + clone.commit_sha = clone_source.commit_sha; SandboxSpec::Provider(Box::new(ProviderSandboxSpec { kind: sandbox_provider.clone(), access, - options, + spec, + clone, github_app: services.github_app.clone(), run_id: Some(record.run_id), - clone_origin_url: clone_source.origin_url, - clone_branch: clone_source.branch, - clone_tag: clone_source.tag, - clone_commit_sha: clone_source.commit_sha, })) } }; @@ -802,20 +803,20 @@ fn resolve_sandbox_provider(settings: &ResolvedRunSettings) -> SandboxProviderKi settings.environment.provider.clone() } -/// The environment's sandbox options with its variables resolved through -/// the vault. -fn resolve_sandbox_options( +/// The environment's sandbox spec with its variables resolved through the +/// vault. +fn resolve_sandbox_spec( settings: &ResolvedRunSettings, secrets_lookup: impl FnMut(&str) -> Option, -) -> Result { +) -> Result { let env = settings .environment .resolve_env(secrets_lookup) .map_err(|err| Error::engine_with_source("failed to resolve environment variables", err))? .into_iter() .collect(); - options_from_environment(&settings.environment, &settings.clone, env) - .map_err(|err| Error::engine_with_source("failed to resolve sandbox options", err)) + sandbox_spec_for_environment(&settings.environment, env) + .map_err(|err| Error::engine_with_source("failed to resolve sandbox spec", err)) } fn resolve_start_llm( @@ -1525,9 +1526,9 @@ mod tests { ..RunLayer::default() }); - let options = resolve_sandbox_options(&settings.run, |_| None).unwrap(); - assert!(options.skip_clone); - assert_eq!(options.clone_depth, Some(1)); + let clone = CloneRequest::from_settings(&settings.run.clone); + assert!(clone.skip); + assert_eq!(clone.depth, Some(1)); } #[test] @@ -1540,16 +1541,16 @@ mod tests { ..RunLayer::default() }); - let options = resolve_sandbox_options(&settings.run, |_| None).unwrap(); - assert_eq!(options.clone_depth, None); + let clone = CloneRequest::from_settings(&settings.run.clone); + assert_eq!(clone.depth, None); } #[test] fn clone_providers_default_to_depth_100() { let settings = settings_from_run_layer(RunLayer::default()); - let options = resolve_sandbox_options(&settings.run, |_| None).unwrap(); - assert_eq!(options.clone_depth, Some(100)); + let clone = CloneRequest::from_settings(&settings.run.clone); + assert_eq!(clone.depth, Some(100)); } #[test] @@ -1877,19 +1878,12 @@ mod tests { let SandboxSpec::Provider(spec) = sandbox else { panic!("none target should retain the selected Docker provider"); }; - let ProviderSandboxSpec { - kind, - options, - clone_origin_url, - clone_branch, - clone_commit_sha, - .. - } = *spec; + let ProviderSandboxSpec { kind, clone, .. } = *spec; assert_eq!(kind, SandboxProviderKind::DOCKER); - assert!(options.skip_clone); - assert_eq!(clone_origin_url, None); - assert_eq!(clone_branch, None); - assert_eq!(clone_commit_sha, None); + assert!(clone.skip); + assert_eq!(clone.origin_url, None); + assert_eq!(clone.branch, None); + assert_eq!(clone.commit_sha, None); assert_eq!(sandbox_env.origin_url, None); assert_eq!(pr_origin_url, None); } @@ -1949,18 +1943,15 @@ mod tests { let ProviderSandboxSpec { kind, access, - options, - clone_origin_url, - clone_branch, - clone_commit_sha, + clone, .. } = *spec; assert_eq!(kind, SandboxProviderKind::DAYTONA); assert!(access.daytona.is_some(), "the vault key reaches the spec"); - assert!(options.skip_clone); - assert_eq!(clone_origin_url, None); - assert_eq!(clone_branch, None); - assert_eq!(clone_commit_sha, None); + assert!(clone.skip); + assert_eq!(clone.origin_url, None); + assert_eq!(clone.branch, None); + assert_eq!(clone.commit_sha, None); assert_eq!(sandbox_env.origin_url, None); assert_eq!(pr_origin_url, None); } @@ -2338,17 +2329,21 @@ mod tests { ..RunLayer::default() }); - let options = resolve_sandbox_options(&settings.run, |_| None).unwrap(); + let spec = resolve_sandbox_spec(&settings.run, |_| None).unwrap(); - assert_eq!(options.image.as_deref(), Some("ubuntu:24.04")); - assert_eq!(options.cpu, Some(4)); - assert_eq!(options.memory_bytes, Some(2_000_000_000)); assert!(matches!( - options.network, - fabro_sandbox::NetworkPolicy::Block + &spec.source, + fabro_sandbox::SandboxSource::Image { reference } if reference == "ubuntu:24.04" )); + assert_eq!(spec.resources.cpu_cores, Some(4)); assert_eq!( - options.env, + spec.resources.memory_mb, + Some(1908), + "2 GB rounds up to whole mebibytes" + ); + assert!(matches!(spec.network, fabro_sandbox::NetworkPolicy::Block)); + assert_eq!( + spec.env, std::collections::BTreeMap::from([("NODE_ENV".to_string(), "test".to_string())]) ); } diff --git a/lib/components/fabro-workflow/tests/it/cp_integration.rs b/lib/components/fabro-workflow/tests/it/cp_integration.rs index f78200a21..ee879855c 100644 --- a/lib/components/fabro-workflow/tests/it/cp_integration.rs +++ b/lib/components/fabro-workflow/tests/it/cp_integration.rs @@ -15,8 +15,9 @@ )] use fabro_sandbox::reconnect::reconnect; -use fabro_sandbox::{ProviderAccess, SandboxOptions, provider_sandbox}; +use fabro_sandbox::{CloneRequest, ProviderAccess, provider_sandbox}; use fabro_types::{RunSandboxInstance, RunSandboxRuntime, SandboxProviderKind}; +use sandbox_driver::{SandboxSource, SandboxSpec}; const DOCKER_CP_IMAGE: &str = "buildpack-deps:noble"; @@ -187,15 +188,10 @@ async fn docker_cp_container() -> DockerCpContainer { let sandbox = provider_sandbox( SandboxProviderKind::DOCKER, &ProviderAccess::default(), - SandboxOptions { - image: Some(DOCKER_CP_IMAGE.to_string()), - skip_clone: true, - ..SandboxOptions::default() - }, - None, - None, - None, - None, + SandboxSpec::new(SandboxSource::Image { + reference: DOCKER_CP_IMAGE.to_string(), + }), + &CloneRequest::none(), None, None, ) diff --git a/lib/components/fabro-workflow/tests/it/daytona_integration.rs b/lib/components/fabro-workflow/tests/it/daytona_integration.rs index b559e5b15..1207f2948 100644 --- a/lib/components/fabro-workflow/tests/it/daytona_integration.rs +++ b/lib/components/fabro-workflow/tests/it/daytona_integration.rs @@ -25,7 +25,7 @@ use std::sync::Arc; use fabro_agent::RunSandbox; use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node}; use fabro_sandbox::{ - DaytonaCredentials, ProviderAccess, SandboxOptions, SandboxProviderKind, provider_sandbox, + CloneRequest, DaytonaCredentials, ProviderAccess, SandboxProviderKind, provider_sandbox, }; use fabro_static::EnvVars; use fabro_store::{ArtifactKey, ArtifactStore}; @@ -44,6 +44,7 @@ use fabro_workflow::run_options::{GitCheckpointOptions, RunOptions}; use fabro_workflow::runtime_store::RunStoreHandle; use fabro_workflow::test_support::{WorkflowRunner, test_store_dir}; use object_store::local::LocalFileSystem; +use sandbox_driver::{LifecycleTimers, Resources, SandboxSource, SandboxSpec}; use tokio_util::sync::CancellationToken; use ulid::Ulid; @@ -224,13 +225,10 @@ async fn create_env_with_github_app( provider_sandbox( SandboxProviderKind::DAYTONA, &daytona_access(live_daytona_credentials()), - SandboxOptions::default(), + SandboxSpec::new(SandboxSource::HostDirectory), + &CloneRequest::default(), github_app.as_ref(), None, - None, - None, - None, - None, ) .await .expect("Failed to create Daytona client — is DAYTONA_API_KEY set?") @@ -423,28 +421,26 @@ async fn daytona_full_lifecycle() { #[fabro_macros::e2e_test(live("DAYTONA_API_KEY"), live("GITHUB_APP_PRIVATE_KEY"))] async fn daytona_snapshot_sandbox() { - let options = SandboxOptions { - auto_stop: Some(std::time::Duration::from_hours(1)), - dockerfile: Some( - "FROM ubuntu:22.04\nRUN apt-get update && apt-get install -y ripgrep".to_string(), - ), - cpu: Some(2), - memory_bytes: Some(4_000_000_000), - disk_bytes: Some(10_000_000_000), - ..SandboxOptions::default() - }; + let mut resources = Resources::default(); + resources.cpu_cores = Some(2); + resources.memory_mb = Some(4096); + resources.disk_mb = Some(10_240); + let mut timers = LifecycleTimers::default(); + timers.auto_stop_after_idle = Some(std::time::Duration::from_hours(1)); + let spec = SandboxSpec::new(SandboxSource::Dockerfile { + content: "FROM ubuntu:22.04\nRUN apt-get update && apt-get install -y ripgrep".to_string(), + }) + .resources(resources) + .timers(timers); let creds = load_github_app_credentials(); let env = provider_sandbox( SandboxProviderKind::DAYTONA, &daytona_access(live_daytona_credentials()), - options, + spec, + &CloneRequest::default(), Some(&creds), None, - None, - None, - None, - None, ) .await .expect("Failed to create Daytona client — is DAYTONA_API_KEY set?"); @@ -1652,18 +1648,11 @@ async fn daytona_cp_upload_download_round_trip() { #[fabro_macros::e2e_test(live("DAYTONA_API_KEY"))] async fn daytona_computer_use_browser_screenshot() { - let options = SandboxOptions { - skip_clone: true, - ..SandboxOptions::default() - }; let env = provider_sandbox( SandboxProviderKind::DAYTONA, &daytona_access(live_daytona_credentials()), - options, - None, - None, - None, - None, + SandboxSpec::new(SandboxSource::HostDirectory), + &CloneRequest::none(), None, None, ) @@ -1800,18 +1789,11 @@ 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 options = SandboxOptions { - skip_clone: true, - ..SandboxOptions::default() - }; let sandbox = provider_sandbox( SandboxProviderKind::DAYTONA, &daytona_access(live_daytona_credentials()), - options, - None, - None, - None, - None, + SandboxSpec::new(SandboxSource::HostDirectory), + &CloneRequest::none(), None, None, ) diff --git a/lib/components/fabro-workflow/tests/it/integration.rs b/lib/components/fabro-workflow/tests/it/integration.rs index 2230aeacc..4d58fd7f1 100644 --- a/lib/components/fabro-workflow/tests/it/integration.rs +++ b/lib/components/fabro-workflow/tests/it/integration.rs @@ -13625,19 +13625,12 @@ async fn asset_collection_local_sandbox_on_failure() { async fn asset_collection_docker_sandbox() { let run_dir = tempfile::tempdir().unwrap(); - let options = fabro_agent::SandboxOptions { - skip_clone: true, - ..Default::default() - }; let sandbox: Arc = Arc::new( fabro_agent::provider_sandbox( fabro_agent::SandboxProviderKind::DOCKER, &fabro_agent::ProviderAccess::default(), - options, - None, - None, - None, - None, + sandbox_driver::SandboxSpec::new(sandbox_driver::SandboxSource::HostDirectory), + &fabro_agent::CloneRequest::none(), None, None, ) diff --git a/lib/foundation/fabro-types/src/settings/run.rs b/lib/foundation/fabro-types/src/settings/run.rs index e9327a147..26ceafa40 100644 --- a/lib/foundation/fabro-types/src/settings/run.rs +++ b/lib/foundation/fabro-types/src/settings/run.rs @@ -7,7 +7,7 @@ //! behavior, and artifact collection. use std::collections::{BTreeMap, BTreeSet, HashMap}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::time::Duration as StdDuration; use fabro_util::shell; @@ -1228,6 +1228,19 @@ impl Default for EnvironmentSettings { } } +/// Why a `local` run has no directory to work in. +#[derive(Debug, thiserror::Error)] +pub enum LocalWorkingDirectoryError { + #[error( + "local environment requires a server-side working directory; configure `environment.cwd = \"/absolute/path\"` on the selected local environment" + )] + MissingCwd, + #[error( + "local environment source_directory does not exist or is not a directory on this server: {0}. Configure `environment.cwd = \"/absolute/path\"` on the selected local environment for remote client/server deployments." + )] + MissingSourceDirectory(PathBuf), +} + #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct RunEnvironmentSettings { pub id: String, @@ -1258,6 +1271,42 @@ impl RunEnvironmentSettings { } } + /// The environment's variables in source form, for a path with no vault + /// (server preflight): a `{{ secrets.* }}` value keeps its token, and + /// nothing else is left to resolve because `{{ vars.* }}` is substituted + /// at run creation. + #[must_use] + pub fn unresolved_env(&self) -> BTreeMap { + #[expect( + clippy::disallowed_methods, + reason = "preflight has no vault, so an unresolved secret token is carried in source form" + )] + self.env + .iter() + .map(|(key, value)| (key.clone(), value.as_source())) + .collect() + } + + /// The directory a `local` run works in: the environment's `cwd`, or + /// the run's source directory when it exists on this host. + pub fn local_working_directory( + &self, + source_directory: Option<&Path>, + ) -> Result { + if let Some(cwd) = self.cwd.as_deref() { + return Ok(PathBuf::from(cwd)); + } + let Some(source_directory) = source_directory else { + return Err(LocalWorkingDirectoryError::MissingCwd); + }; + if source_directory.is_dir() { + return Ok(source_directory.to_path_buf()); + } + Err(LocalWorkingDirectoryError::MissingSourceDirectory( + source_directory.to_path_buf(), + )) + } + /// Resolve every environment value's `{{ secrets.* }}` tokens via /// `secrets_lookup`. `{{ vars.* }}` is already substituted server-side at /// run creation, so anything still unresolved here fails closed.