From d3e33ce32cde5d01c2cb88d1ea619b3d563abe89 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 8 May 2026 18:38:08 -0700 Subject: [PATCH] feat(runs): own sandbox lifecycle Create run-owned sandbox lifecycle operations so terminal runs stop by default, resumes attach and start persisted sandboxes, and run deletion deletes or hands off provider resources according to preserve settings. --- apps/fabro-web/app/routes/run-settings.tsx | 3 + apps/fabro-web/app/routes/workflow-detail.tsx | 1 + docs/public/api-reference/fabro-api.yaml | 33 +++- lib/crates/fabro-api/build.rs | 18 +++ lib/crates/fabro-config/src/defaults.toml | 1 + lib/crates/fabro-config/src/layers/run.rs | 16 +- lib/crates/fabro-config/src/resolve/run.rs | 3 + .../fabro-config/src/tests/resolve_run.rs | 17 +++ lib/crates/fabro-sandbox/src/daytona/mod.rs | 82 +++++++++- lib/crates/fabro-sandbox/src/docker.rs | 126 +++++++++++++-- lib/crates/fabro-sandbox/src/lib.rs | 1 + lib/crates/fabro-sandbox/src/local.rs | 4 + lib/crates/fabro-sandbox/src/reconnect.rs | 35 ++++- lib/crates/fabro-sandbox/src/sandbox.rs | 108 +++++++++++++ lib/crates/fabro-sandbox/src/test_support.rs | 39 +++++ lib/crates/fabro-sandbox/src/worktree.rs | 26 ++++ lib/crates/fabro-server/src/demo/mod.rs | 15 +- lib/crates/fabro-server/src/run_files.rs | 74 +++------ lib/crates/fabro-server/src/server.rs | 131 ++++++++++++++-- .../fabro-server/src/server/handler/runs.rs | 5 +- .../src/server/handler/sandbox.rs | 26 +++- lib/crates/fabro-server/src/server/tests.rs | 144 ++++++++++++++++++ lib/crates/fabro-types/src/run_event/infra.rs | 57 +++++++ lib/crates/fabro-types/src/run_event/mod.rs | 36 +++++ lib/crates/fabro-types/src/settings/run.rs | 35 +++-- lib/crates/fabro-types/src/status.rs | 41 ++++- lib/crates/fabro-workflow/Cargo.toml | 1 + .../fabro-workflow/src/event/convert.rs | 82 ++++++++++ lib/crates/fabro-workflow/src/event/names.rs | 9 ++ .../fabro-workflow/src/operations/start.rs | 9 +- .../fabro-workflow/src/pipeline/finalize.rs | 90 ++++++++++- .../fabro-workflow/src/pipeline/initialize.rs | 93 ++++++++--- .../fabro-workflow/src/pipeline/types.rs | 1 + .../src/.openapi-generator/FILES | 2 + .../fabro-api-client/src/api/runs-api.ts | 14 +- .../src/models/delete-run-response.ts | 27 ++++ .../src/models/delete-run-sandbox.ts | 20 +++ .../fabro-api-client/src/models/index.ts | 2 + .../src/models/run-sandbox-settings.ts | 1 + 39 files changed, 1265 insertions(+), 163 deletions(-) create mode 100644 lib/packages/fabro-api-client/src/models/delete-run-response.ts create mode 100644 lib/packages/fabro-api-client/src/models/delete-run-sandbox.ts diff --git a/apps/fabro-web/app/routes/run-settings.tsx b/apps/fabro-web/app/routes/run-settings.tsx index 77bc744f4..42796d5e5 100644 --- a/apps/fabro-web/app/routes/run-settings.tsx +++ b/apps/fabro-web/app/routes/run-settings.tsx @@ -163,6 +163,9 @@ function SandboxPanel({ snapshot }: { snapshot: Snapshot }) { + + + diff --git a/apps/fabro-web/app/routes/workflow-detail.tsx b/apps/fabro-web/app/routes/workflow-detail.tsx index ce9ac37d4..4f36d5d19 100644 --- a/apps/fabro-web/app/routes/workflow-detail.tsx +++ b/apps/fabro-web/app/routes/workflow-detail.tsx @@ -66,6 +66,7 @@ function sampleSettings({ sandbox: { provider: "daytona", preserve: false, + stop_on_terminal: true, devcontainer: true, env: {}, local: { worktree_mode: "dirty" }, diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index 71933b9d3..1751fa1b2 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -922,11 +922,17 @@ paths: operationId: deleteRun tags: [Runs] summary: Delete Run - description: Deletes durable store state for a run. This does not remove any local run directory. Active runs require `force=true`. + description: Deletes durable store state, local run scratch data, and the run-owned sandbox unless sandbox preservation is enabled. Active runs require `force=true`. parameters: - $ref: "#/components/parameters/RunId" - $ref: "#/components/parameters/ForceRunDelete" responses: + "200": + description: Run deleted and sandbox preservation details returned + content: + application/json: + schema: + $ref: "#/components/schemas/DeleteRunResponse" "204": description: Run deleted or already absent "404": @@ -5105,6 +5111,27 @@ components: `server.web.url` is non-empty; absent otherwise. example: "http://localhost:3000/runs/01JNQVR7M0EJ5GKAT2SC4ERS1Z" + DeleteRunResponse: + description: Returned when a run is deleted but its sandbox is intentionally preserved. + type: object + required: [deleted, sandbox_preserved, sandbox] + properties: + deleted: + type: boolean + sandbox_preserved: + type: boolean + sandbox: + $ref: "#/components/schemas/DeleteRunSandbox" + + DeleteRunSandbox: + type: object + required: [provider, identifier] + properties: + provider: + type: string + identifier: + type: string + ApiQuestionOption: description: A selectable option for a multiple-choice or multi-select question. type: object @@ -8083,12 +8110,14 @@ components: RunSandboxSettings: type: object - required: [provider, preserve, devcontainer, env, local, docker, daytona] + required: [provider, preserve, stop_on_terminal, devcontainer, env, local, docker, daytona] properties: provider: type: string preserve: type: boolean + stop_on_terminal: + type: boolean devcontainer: type: boolean env: diff --git a/lib/crates/fabro-api/build.rs b/lib/crates/fabro-api/build.rs index d094f67bf..bb29e8d8b 100644 --- a/lib/crates/fabro-api/build.rs +++ b/lib/crates/fabro-api/build.rs @@ -134,6 +134,23 @@ fn patch_codegen_request_body_media_types(value: &mut serde_json::Value) { } } +/// Progenitor currently panics when an operation advertises a typed success +/// response alongside a no-content success response. Keep the source spec +/// accurate for docs and other generators, but preserve the existing Rust +/// client shape for `DELETE /runs/{id}`: success with no body. +fn patch_codegen_delete_run_responses(value: &mut serde_json::Value) { + let Some(responses) = value + .get_mut("paths") + .and_then(|paths| paths.get_mut("/api/v1/runs/{id}")) + .and_then(|path| path.get_mut("delete")) + .and_then(|operation| operation.get_mut("responses")) + .and_then(serde_json::Value::as_object_mut) + else { + return; + }; + responses.remove("200"); +} + fn spec_path_from_manifest_dir(manifest_dir: &Path) -> PathBuf { manifest_dir .ancestors() @@ -161,6 +178,7 @@ fn main() { spec_value["openapi"] = serde_json::Value::String("3.0.3".to_string()); patch_nullable(&mut spec_value); patch_codegen_request_body_media_types(&mut spec_value); + patch_codegen_delete_run_responses(&mut spec_value); let spec: openapiv3::OpenAPI = serde_json::from_value(spec_value).expect("failed to deserialize OpenAPI spec"); diff --git a/lib/crates/fabro-config/src/defaults.toml b/lib/crates/fabro-config/src/defaults.toml index d673e64a9..2654f0eb2 100644 --- a/lib/crates/fabro-config/src/defaults.toml +++ b/lib/crates/fabro-config/src/defaults.toml @@ -19,6 +19,7 @@ timeout = "5m" [run.sandbox] provider = "docker" preserve = false +stop_on_terminal = true devcontainer = false [run.sandbox.local] diff --git a/lib/crates/fabro-config/src/layers/run.rs b/lib/crates/fabro-config/src/layers/run.rs index b4aaabda1..927c515cb 100644 --- a/lib/crates/fabro-config/src/layers/run.rs +++ b/lib/crates/fabro-config/src/layers/run.rs @@ -256,20 +256,22 @@ pub struct RunCheckpointLayer { #[serde(deny_unknown_fields)] pub struct RunSandboxLayer { #[serde(default, skip_serializing_if = "Option::is_none")] - pub provider: Option, + pub provider: Option, #[serde(default, skip_serializing_if = "Option::is_none")] - pub preserve: Option, + pub preserve: Option, #[serde(default, skip_serializing_if = "Option::is_none")] - pub devcontainer: Option, + pub stop_on_terminal: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub devcontainer: Option, /// Sticky merge-by-key across layers. #[serde(default, skip_serializing_if = "StickyMap::is_empty")] - pub env: StickyMap, + pub env: StickyMap, #[serde(default, skip_serializing_if = "Option::is_none")] - pub local: Option, + pub local: Option, #[serde(default, skip_serializing_if = "Option::is_none")] - pub docker: Option, + pub docker: Option, #[serde(default, skip_serializing_if = "Option::is_none")] - pub daytona: Option, + pub daytona: Option, } #[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] diff --git a/lib/crates/fabro-config/src/resolve/run.rs b/lib/crates/fabro-config/src/resolve/run.rs index c538d70ec..433714e93 100644 --- a/lib/crates/fabro-config/src/resolve/run.rs +++ b/lib/crates/fabro-config/src/resolve/run.rs @@ -179,6 +179,9 @@ fn resolve_sandbox( preserve: sandbox .preserve .expect("defaults.toml should provide run.sandbox.preserve"), + stop_on_terminal: sandbox + .stop_on_terminal + .expect("defaults.toml should provide run.sandbox.stop_on_terminal"), devcontainer: sandbox .devcontainer .expect("defaults.toml should provide run.sandbox.devcontainer"), diff --git a/lib/crates/fabro-config/src/tests/resolve_run.rs b/lib/crates/fabro-config/src/tests/resolve_run.rs index acb887813..d8b3b1e5f 100644 --- a/lib/crates/fabro-config/src/tests/resolve_run.rs +++ b/lib/crates/fabro-config/src/tests/resolve_run.rs @@ -14,6 +14,7 @@ fn resolves_run_defaults_from_empty_settings() { assert!(settings.execution.retros); assert_eq!(settings.prepare.timeout_ms, 300_000); assert_eq!(settings.sandbox.provider, "docker"); + assert!(settings.sandbox.stop_on_terminal); assert_eq!(settings.sandbox.local.worktree_mode, WorktreeMode::Always); let docker = settings .sandbox @@ -27,6 +28,22 @@ fn resolves_run_defaults_from_empty_settings() { assert!(settings.pull_request.is_none()); } +#[test] +fn resolves_explicit_stop_on_terminal_false() { + let settings = WorkflowSettingsBuilder::from_toml( + r" +_version = 1 + +[run.sandbox] +stop_on_terminal = false +", + ) + .expect("sandbox stop_on_terminal setting should resolve") + .run; + + assert!(!settings.sandbox.stop_on_terminal); +} + #[test] fn resolves_minimal_local_provider_without_docker_table() { let settings = WorkflowSettingsBuilder::from_toml( diff --git a/lib/crates/fabro-sandbox/src/daytona/mod.rs b/lib/crates/fabro-sandbox/src/daytona/mod.rs index 8230bcaee..c0ee9a87b 100644 --- a/lib/crates/fabro-sandbox/src/daytona/mod.rs +++ b/lib/crates/fabro-sandbox/src/daytona/mod.rs @@ -425,7 +425,8 @@ impl DaytonaSandbox { name: Some(name), auto_stop_interval: self.config.auto_stop_interval, labels: self.config.labels.clone(), - ephemeral: Some(true), + auto_delete_interval: Some(-1), + ephemeral: Some(false), network_block_all, network_allow_list, ..Default::default() @@ -902,16 +903,62 @@ impl Sandbox for DaytonaSandbox { Ok(()) } - async fn cleanup(&self) -> crate::Result<()> { - self.emit(SandboxEvent::CleanupStarted { + async fn start(&self) -> crate::Result<()> { + self.emit(SandboxEvent::StartStarted { + provider: "daytona".into(), + }); + let start = Instant::now(); + let sandbox = self.sandbox()?; + if let Err(e) = self.client.start(&sandbox.name).await { + let err = crate::Error::context("Failed to start Daytona sandbox", e); + self.emit(SandboxEvent::StartFailed { + provider: "daytona".into(), + error: err.to_string(), + causes: err.causes(), + }); + return Err(err); + } + let duration_ms = u64::try_from(start.elapsed().as_millis()).unwrap_or(u64::MAX); + self.emit(SandboxEvent::StartCompleted { + provider: "daytona".into(), + duration_ms, + }); + Ok(()) + } + + async fn stop(&self) -> crate::Result<()> { + self.emit(SandboxEvent::StopStarted { + provider: "daytona".into(), + }); + let start = Instant::now(); + let sandbox = self.sandbox()?; + if let Err(e) = self.client.stop(&sandbox.name).await { + let err = crate::Error::context("Failed to stop Daytona sandbox", e); + self.emit(SandboxEvent::StopFailed { + provider: "daytona".into(), + error: err.to_string(), + causes: err.causes(), + }); + return Err(err); + } + let duration_ms = u64::try_from(start.elapsed().as_millis()).unwrap_or(u64::MAX); + self.emit(SandboxEvent::StopCompleted { + provider: "daytona".into(), + duration_ms, + }); + Ok(()) + } + + async fn delete(&self) -> crate::Result<()> { + self.emit(SandboxEvent::DeleteStarted { provider: "daytona".into(), }); let start = Instant::now(); if let Some(sandbox) = self.sandbox.get() { - tracing::info!("Destroying Daytona sandbox"); + tracing::info!("Deleting Daytona sandbox"); if let Err(e) = sandbox.delete().await { let err = crate::Error::context("Failed to delete Daytona sandbox", e); - self.emit(SandboxEvent::CleanupFailed { + self.emit(SandboxEvent::DeleteFailed { provider: "daytona".into(), error: err.to_string(), causes: err.causes(), @@ -920,13 +967,17 @@ impl Sandbox for DaytonaSandbox { } } let duration_ms = u64::try_from(start.elapsed().as_millis()).unwrap_or(u64::MAX); - self.emit(SandboxEvent::CleanupCompleted { + self.emit(SandboxEvent::DeleteCompleted { provider: "daytona".into(), duration_ms, }); Ok(()) } + async fn cleanup(&self) -> crate::Result<()> { + self.delete().await + } + fn working_directory(&self) -> &str { WORKING_DIRECTORY } @@ -1969,6 +2020,25 @@ mod tests { assert!(config.labels.is_none()); } + #[tokio::test] + async fn base_params_create_run_owned_non_ephemeral_sandbox() { + let sandbox = DaytonaSandbox::new( + DaytonaConfig::default(), + None, + None, + None, + None, + Some("dtn_test".to_string()), + ) + .await + .expect("sandbox config should be valid"); + + let params = sandbox.base_params(); + + assert_eq!(params.ephemeral, Some(false)); + assert_eq!(params.auto_delete_interval, Some(-1)); + } + #[test] fn command_kind_classifies_known_prefixes() { assert_eq!(command_kind(" git status"), "git"); diff --git a/lib/crates/fabro-sandbox/src/docker.rs b/lib/crates/fabro-sandbox/src/docker.rs index 5e9dcce85..b1e743d9d 100644 --- a/lib/crates/fabro-sandbox/src/docker.rs +++ b/lib/crates/fabro-sandbox/src/docker.rs @@ -130,11 +130,12 @@ impl DockerSandbox { repo_cloned: bool, clone_origin_url: Option, clone_branch: Option, + run_id: Option, ) -> crate::Result { let sandbox = Self::new( DockerSandboxOptions::default(), None, - None, + run_id, clone_origin_url.clone(), clone_branch, )?; @@ -691,8 +692,26 @@ impl DockerSandbox { .map_err(|e| crate::Error::context("Failed to upload file to container", e)) } - fn cleanup_error(&self, error: crate::Error) -> crate::Result<()> { - self.emit(SandboxEvent::CleanupFailed { + fn start_error(&self, error: crate::Error) -> crate::Result<()> { + self.emit(SandboxEvent::StartFailed { + provider: "docker".into(), + error: error.to_string(), + causes: error.causes(), + }); + Err(error) + } + + fn stop_error(&self, error: crate::Error) -> crate::Result<()> { + self.emit(SandboxEvent::StopFailed { + provider: "docker".into(), + error: error.to_string(), + causes: error.causes(), + }); + Err(error) + } + + fn delete_error(&self, error: crate::Error) -> crate::Result<()> { + self.emit(SandboxEvent::DeleteFailed { provider: "docker".into(), error: error.to_string(), causes: error.causes(), @@ -1110,15 +1129,64 @@ impl Sandbox for DockerSandbox { Ok(()) } - async fn cleanup(&self) -> crate::Result<()> { - self.emit(SandboxEvent::CleanupStarted { + async fn start(&self) -> crate::Result<()> { + self.emit(SandboxEvent::StartStarted { + provider: "docker".into(), + }); + let start = Instant::now(); + let container_id = self.container_id()?.to_string(); + let labels = match self.inspect_labels(&container_id).await { + Ok(labels) => labels, + Err(e) => return self.start_error(e), + }; + if let Err(e) = verify_managed_labels(&container_id, &labels, self.run_id.as_ref()) { + return self.start_error(e); + } + + if let Err(e) = self + .docker + .start_container(&container_id, None::>) + .await + { + if !docker_already_stopped(&e) { + return self.start_error(crate::Error::context( + format!( + "Failed to start Docker container '{container_id}' with labels {labels:?}" + ), + e, + )); + } + } + + let (_, stderr, exit_code) = self + .docker_exec(vec!["true".to_string()], None, None) + .await + .map_err(|e| { + crate::Error::context(format!("Docker container '{container_id}' health check"), e) + })?; + if exit_code != 0 { + return self.start_error(crate::Error::message(format!( + "Docker container '{container_id}' health check failed: {stderr}" + ))); + } + + let duration_ms = u64::try_from(start.elapsed().as_millis()).unwrap_or(u64::MAX); + self.emit(SandboxEvent::StartCompleted { + provider: "docker".into(), + duration_ms, + }); + Ok(()) + } + + async fn stop(&self) -> crate::Result<()> { + self.emit(SandboxEvent::StopStarted { provider: "docker".into(), }); let start = Instant::now(); let Some(container_id) = self.container_id.get().cloned() else { let duration_ms = u64::try_from(start.elapsed().as_millis()).unwrap_or(u64::MAX); - self.emit(SandboxEvent::CleanupCompleted { + self.emit(SandboxEvent::StopCompleted { provider: "docker".into(), duration_ms, }); @@ -1127,10 +1195,10 @@ impl Sandbox for DockerSandbox { let labels = match self.inspect_labels(&container_id).await { Ok(labels) => labels, - Err(e) => return self.cleanup_error(e), + Err(e) => return self.stop_error(e), }; if let Err(e) = verify_managed_labels(&container_id, &labels, self.run_id.as_ref()) { - return self.cleanup_error(e); + return self.stop_error(e); } let stop_opts = StopContainerOptions { t: 1 }; @@ -1140,7 +1208,7 @@ impl Sandbox for DockerSandbox { .await { if !docker_not_found(&e) && !docker_already_stopped(&e) { - return self.cleanup_error(crate::Error::context( + return self.stop_error(crate::Error::context( format!( "Failed to stop Docker container '{container_id}' with labels {labels:?}" ), @@ -1149,6 +1217,38 @@ impl Sandbox for DockerSandbox { } } + let duration_ms = u64::try_from(start.elapsed().as_millis()).unwrap_or(u64::MAX); + self.emit(SandboxEvent::StopCompleted { + provider: "docker".into(), + duration_ms, + }); + + Ok(()) + } + + async fn delete(&self) -> crate::Result<()> { + self.emit(SandboxEvent::DeleteStarted { + provider: "docker".into(), + }); + let start = Instant::now(); + + let Some(container_id) = self.container_id.get().cloned() else { + let duration_ms = u64::try_from(start.elapsed().as_millis()).unwrap_or(u64::MAX); + self.emit(SandboxEvent::DeleteCompleted { + provider: "docker".into(), + duration_ms, + }); + return Ok(()); + }; + + let labels = match self.inspect_labels(&container_id).await { + Ok(labels) => labels, + Err(e) => return self.delete_error(e), + }; + if let Err(e) = verify_managed_labels(&container_id, &labels, self.run_id.as_ref()) { + return self.delete_error(e); + } + let remove_opts = RemoveContainerOptions { force: true, ..Default::default() @@ -1159,7 +1259,7 @@ impl Sandbox for DockerSandbox { .await { if !docker_not_found(&e) { - return self.cleanup_error(crate::Error::context( + return self.delete_error(crate::Error::context( format!( "Failed to remove Docker container '{container_id}' with labels {labels:?}" ), @@ -1169,7 +1269,7 @@ impl Sandbox for DockerSandbox { } let duration_ms = u64::try_from(start.elapsed().as_millis()).unwrap_or(u64::MAX); - self.emit(SandboxEvent::CleanupCompleted { + self.emit(SandboxEvent::DeleteCompleted { provider: "docker".into(), duration_ms, }); @@ -1177,6 +1277,10 @@ impl Sandbox for DockerSandbox { Ok(()) } + async fn cleanup(&self) -> crate::Result<()> { + self.delete().await + } + async fn exec_command( &self, command: &str, diff --git a/lib/crates/fabro-sandbox/src/lib.rs b/lib/crates/fabro-sandbox/src/lib.rs index 44b44c038..a17296239 100644 --- a/lib/crates/fabro-sandbox/src/lib.rs +++ b/lib/crates/fabro-sandbox/src/lib.rs @@ -35,6 +35,7 @@ pub use docker::{DockerSandbox, DockerSandboxOptions}; pub use error::{Error, Result, default_redacted_output_tail, display_for_log}; pub use local::LocalSandbox; pub use read_guard::ReadBeforeWriteSandbox; +pub use reconnect::{reconnect, reconnect_for_run, reconnect_for_run_with_callback}; pub use sandbox::{ CommandOutputCallback, DEFAULT_EXEC_OUTPUT_TAIL_BYTES, DirEntry, ExecResult, ExecStreamingResult, GitRunInfo, GitSetupIntent, GrepOptions, Sandbox, SandboxEvent, diff --git a/lib/crates/fabro-sandbox/src/local.rs b/lib/crates/fabro-sandbox/src/local.rs index a8b49e215..7deb20a38 100644 --- a/lib/crates/fabro-sandbox/src/local.rs +++ b/lib/crates/fabro-sandbox/src/local.rs @@ -620,6 +620,10 @@ impl Sandbox for LocalSandbox { Ok(()) } + async fn delete(&self) -> crate::Result<()> { + Ok(()) + } + fn working_directory(&self) -> &str { self.working_directory.to_str().unwrap_or(".") } diff --git a/lib/crates/fabro-sandbox/src/reconnect.rs b/lib/crates/fabro-sandbox/src/reconnect.rs index 288682d1e..7f90d11a9 100644 --- a/lib/crates/fabro-sandbox/src/reconnect.rs +++ b/lib/crates/fabro-sandbox/src/reconnect.rs @@ -5,7 +5,9 @@ use std::path::PathBuf; reason = "Feature-gated branches consume these imports when optional backends are enabled." )] use anyhow::{Context, Result, bail}; +use fabro_types::RunId; +use crate::SandboxEventCallback; #[cfg(feature = "daytona")] use crate::daytona::DaytonaSandbox; #[cfg(feature = "docker")] @@ -25,10 +27,30 @@ use crate::sandbox_record::SandboxRecord; pub async fn reconnect( record: &SandboxRecord, daytona_api_key: Option, +) -> Result> { + reconnect_for_run(record, daytona_api_key, None).await +} + +pub async fn reconnect_for_run( + record: &SandboxRecord, + daytona_api_key: Option, + run_id: Option, +) -> Result> { + reconnect_for_run_with_callback(record, daytona_api_key, run_id, None).await +} + +pub async fn reconnect_for_run_with_callback( + record: &SandboxRecord, + daytona_api_key: Option, + run_id: Option, + event_callback: Option, ) -> Result> { match record.provider.as_str() { "local" => { - let sandbox = LocalSandbox::new(PathBuf::from(&record.working_directory)); + let mut sandbox = LocalSandbox::new(PathBuf::from(&record.working_directory)); + if let Some(callback) = event_callback { + sandbox.set_event_callback(callback); + } Ok(Box::new(sandbox)) } #[cfg(feature = "docker")] @@ -40,14 +62,18 @@ pub async fn reconnect( let repo_cloned = record .repo_cloned .context("Docker sandbox record missing repo_cloned metadata")?; - let sandbox = DockerSandbox::reconnect( + let mut sandbox = DockerSandbox::reconnect( identifier, repo_cloned, record.clone_origin_url.clone(), record.clone_branch.clone(), + run_id, ) .await .context("Failed to reconnect Docker sandbox")?; + if let Some(callback) = event_callback { + sandbox.set_event_callback(callback); + } Ok(Box::new(sandbox)) } #[cfg(feature = "daytona")] @@ -60,7 +86,7 @@ pub async fn reconnect( .repo_cloned .context("Daytona sandbox record missing repo_cloned metadata")?; - let sandbox = DaytonaSandbox::reconnect( + let mut sandbox = DaytonaSandbox::reconnect( name, daytona_api_key, repo_cloned, @@ -69,6 +95,9 @@ pub async fn reconnect( ) .await .map_err(anyhow::Error::new)?; + if let Some(callback) = event_callback { + sandbox.set_event_callback(callback); + } Ok(Box::new(sandbox)) } other => bail!("Unknown sandbox provider: {other}"), diff --git a/lib/crates/fabro-sandbox/src/sandbox.rs b/lib/crates/fabro-sandbox/src/sandbox.rs index bfd31d91b..ef7ae3b90 100644 --- a/lib/crates/fabro-sandbox/src/sandbox.rs +++ b/lib/crates/fabro-sandbox/src/sandbox.rs @@ -145,6 +145,18 @@ macro_rules! delegate_sandbox { self.$field.initialize().await } + async fn start(&self) -> $crate::Result<()> { + self.$field.start().await + } + + async fn stop(&self) -> $crate::Result<()> { + self.$field.stop().await + } + + async fn delete(&self) -> $crate::Result<()> { + self.$field.delete().await + } + async fn cleanup(&self) -> $crate::Result<()> { self.$field.cleanup().await } @@ -263,6 +275,45 @@ pub enum SandboxEvent { #[serde(default, skip_serializing_if = "Vec::is_empty")] causes: Vec, }, + StartStarted { + provider: String, + }, + StartCompleted { + provider: String, + duration_ms: u64, + }, + StartFailed { + provider: String, + error: String, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + causes: Vec, + }, + StopStarted { + provider: String, + }, + StopCompleted { + provider: String, + duration_ms: u64, + }, + StopFailed { + provider: String, + error: String, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + causes: Vec, + }, + DeleteStarted { + provider: String, + }, + DeleteCompleted { + provider: String, + duration_ms: u64, + }, + DeleteFailed { + provider: String, + error: String, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + causes: Vec, + }, // -- Snapshot lifecycle -- SnapshotPulling { @@ -337,6 +388,54 @@ impl SandboxEvent { } => { warn!(provider, error, causes = ?causes, "Sandbox cleanup failed"); } + Self::StartStarted { provider } => { + info!(provider, "Sandbox start started"); + } + Self::StartCompleted { + provider, + duration_ms, + } => { + info!(provider, duration_ms, "Sandbox start completed"); + } + Self::StartFailed { + provider, + error, + causes, + } => { + warn!(provider, error, causes = ?causes, "Sandbox start failed"); + } + Self::StopStarted { provider } => { + info!(provider, "Sandbox stop started"); + } + Self::StopCompleted { + provider, + duration_ms, + } => { + info!(provider, duration_ms, "Sandbox stop completed"); + } + Self::StopFailed { + provider, + error, + causes, + } => { + warn!(provider, error, causes = ?causes, "Sandbox stop failed"); + } + Self::DeleteStarted { provider } => { + info!(provider, "Sandbox delete started"); + } + Self::DeleteCompleted { + provider, + duration_ms, + } => { + info!(provider, duration_ms, "Sandbox delete completed"); + } + Self::DeleteFailed { + provider, + error, + causes, + } => { + warn!(provider, error, causes = ?causes, "Sandbox delete failed"); + } Self::SnapshotPulling { name } => { debug!(name, "Snapshot pulling"); } @@ -675,6 +774,15 @@ pub trait Sandbox: Send + Sync { remote_path: &str, ) -> crate::Result<()>; async fn initialize(&self) -> crate::Result<()>; + async fn start(&self) -> crate::Result<()> { + Ok(()) + } + async fn stop(&self) -> crate::Result<()> { + Ok(()) + } + async fn delete(&self) -> crate::Result<()> { + self.cleanup().await + } async fn cleanup(&self) -> crate::Result<()>; fn working_directory(&self) -> &str; fn platform(&self) -> &str; diff --git a/lib/crates/fabro-sandbox/src/test_support.rs b/lib/crates/fabro-sandbox/src/test_support.rs index 2566930fd..0c7af3dd9 100644 --- a/lib/crates/fabro-sandbox/src/test_support.rs +++ b/lib/crates/fabro-sandbox/src/test_support.rs @@ -32,6 +32,9 @@ pub struct MockSandbox { pub captured_working_dirs: Mutex>>, /// Captures the `env_vars` argument from `exec_command` calls. pub captured_env_vars: Mutex>>, + pub start_calls: Mutex, + pub stop_calls: Mutex, + pub delete_calls: Mutex, pub event_callback: Option, } @@ -44,6 +47,21 @@ impl MockSandbox { ..Default::default() } } + + pub fn start_count(&self) -> u32 { + *self.start_calls.lock().expect("start_calls lock poisoned") + } + + pub fn stop_count(&self) -> u32 { + *self.stop_calls.lock().expect("stop_calls lock poisoned") + } + + pub fn delete_count(&self) -> u32 { + *self + .delete_calls + .lock() + .expect("delete_calls lock poisoned") + } } impl MockSandbox { @@ -78,6 +96,9 @@ impl Default for MockSandbox { captured_commands: Mutex::new(Vec::new()), captured_working_dirs: Mutex::new(Vec::new()), captured_env_vars: Mutex::new(None), + start_calls: Mutex::new(0), + stop_calls: Mutex::new(0), + delete_calls: Mutex::new(0), event_callback: None, } } @@ -227,6 +248,24 @@ impl Sandbox for MockSandbox { Ok(()) } + async fn start(&self) -> crate::Result<()> { + *self.start_calls.lock().expect("start_calls lock poisoned") += 1; + Ok(()) + } + + async fn stop(&self) -> crate::Result<()> { + *self.stop_calls.lock().expect("stop_calls lock poisoned") += 1; + Ok(()) + } + + async fn delete(&self) -> crate::Result<()> { + *self + .delete_calls + .lock() + .expect("delete_calls lock poisoned") += 1; + Ok(()) + } + async fn cleanup(&self) -> crate::Result<()> { self.emit(SandboxEvent::CleanupStarted { provider: "mock".into(), diff --git a/lib/crates/fabro-sandbox/src/worktree.rs b/lib/crates/fabro-sandbox/src/worktree.rs index d7941dde0..87a31db31 100644 --- a/lib/crates/fabro-sandbox/src/worktree.rs +++ b/lib/crates/fabro-sandbox/src/worktree.rs @@ -210,6 +210,18 @@ impl Sandbox for WorktreeSandbox { Ok(()) } + async fn start(&self) -> crate::Result<()> { + self.inner.start().await + } + + async fn stop(&self) -> crate::Result<()> { + self.inner.stop().await + } + + async fn delete(&self) -> crate::Result<()> { + self.inner.delete().await + } + fn working_directory(&self) -> &str { &self.config.worktree_path } @@ -690,6 +702,20 @@ mod tests { ); } + #[tokio::test] + async fn lifecycle_operations_forward_to_inner_sandbox() { + let (inner, mock) = make_mock(); + let wt = WorktreeSandbox::new(inner, make_config("/tmp/wt")); + + wt.start().await.unwrap(); + wt.stop().await.unwrap(); + wt.delete().await.unwrap(); + + assert_eq!(mock.start_count(), 1); + assert_eq!(mock.stop_count(), 1); + assert_eq!(mock.delete_count(), 1); + } + // ----------------------------------------------------------------------- // Bug: initialize() is not idempotent — double call destroys worktree // ----------------------------------------------------------------------- diff --git a/lib/crates/fabro-server/src/demo/mod.rs b/lib/crates/fabro-server/src/demo/mod.rs index eba3e32d3..4ed186b53 100644 --- a/lib/crates/fabro-server/src/demo/mod.rs +++ b/lib/crates/fabro-server/src/demo/mod.rs @@ -1652,13 +1652,14 @@ mod runs { timeout_ms: 120_000, }, sandbox: RunSandboxSettings { - provider: "daytona".into(), - preserve: false, - devcontainer: false, - env: HashMap::new(), - local: LocalSandboxSettings::default(), - docker: None, - daytona: Some(DaytonaSettings { + provider: "daytona".into(), + preserve: false, + stop_on_terminal: true, + devcontainer: false, + env: HashMap::new(), + local: LocalSandboxSettings::default(), + docker: None, + daytona: Some(DaytonaSettings { auto_stop_interval: Some(60), labels: HashMap::from([( "project".to_string(), diff --git a/lib/crates/fabro-server/src/run_files.rs b/lib/crates/fabro-server/src/run_files.rs index 22eed6cd5..1db9aafe0 100644 --- a/lib/crates/fabro-server/src/run_files.rs +++ b/lib/crates/fabro-server/src/run_files.rs @@ -30,7 +30,7 @@ use fabro_api::types::{ DiffFile, DiffStats, FileDiff, FileDiffChangeKind, FileDiffTruncationReason, PaginatedRunFileList, RunFilesMeta, RunFilesMetaDegradedReason, RunFilesMetaToSha, }; -use fabro_sandbox::reconnect::reconnect; +use fabro_sandbox::reconnect::reconnect_for_run; use fabro_static::EnvVars; use fabro_types::RunId; use fabro_workflow::sandbox_git::{ @@ -177,9 +177,9 @@ where /// serves the full run diff). /// 2. Load the run projection. 404 covers both missing run and missing access — /// IDOR-safe. -/// 3. Try to reconnect the sandbox; on success, build a structured diff. -/// 4. On reconnect failure or garbage-collected base, fall through to a -/// degraded response built from `RunProjection.final_patch`. +/// 3. Reconnect and start the sandbox, then build a structured diff. +/// 4. On garbage-collected base commits, fall through to a degraded response +/// built from `RunProjection.final_patch`. /// /// All logging emits a single `tracing::info!` with an allowlisted field /// set enforced by [`RunFilesMetrics::emit`] — no paths, contents, or raw @@ -244,8 +244,8 @@ fn validate_one_sha(value: Option<&str>, param_name: &str) -> std::result::Resul /// Materialize the response for `GET /runs/{id}/files`. Prefers the live /// sandbox path; falls through to a `final_patch`-based degraded response -/// when the sandbox is unreachable or gone; falls through to an empty -/// envelope when neither is available. +/// when the base objects are gone; falls through to an empty envelope when +/// neither is available. async fn materialize_sandbox_path(state: &Arc, run_id: &RunId) -> ListRunFilesResult { let start = Instant::now(); @@ -256,15 +256,7 @@ async fn materialize_sandbox_path(state: &Arc, run_id: &RunId) -> List return Ok(empty_envelope()); }; - // Try to reconnect; on failure fall through to the final-patch fallback. - let Some(sandbox) = try_reconnect_run_sandbox(state, &projection).await? else { - return Ok(build_fallback_response( - &projection, - reason_for_fallback(&projection), - run_id, - start, - )); - }; + let sandbox = reconnect_run_sandbox(state, run_id, &projection).await?; // Resolve HEAD (sha + commit time) in one round-trip. let (to_sha, to_sha_committed_at) = resolve_head_sha_and_time(sandbox.as_ref()).await?; @@ -376,28 +368,6 @@ async fn materialize_sandbox_path(state: &Arc, run_id: &RunId) -> List }) } -/// Choose a degraded reason given the current projection. Docker-provider -/// runs aren't supported by the deployed server; completed runs are "gone"; -/// everything else is a transient "unreachable" (sandbox may come back). -fn reason_for_fallback(projection: &fabro_store::RunProjection) -> RunFilesMetaDegradedReason { - let provider = projection - .sandbox - .as_ref() - .map(|s| s.provider.to_ascii_lowercase()); - if matches!(provider.as_deref(), Some("docker")) { - return RunFilesMetaDegradedReason::ProviderUnsupported; - } - let is_terminal = projection - .status - .as_ref() - .is_some_and(|status| status.is_terminal()); - if is_terminal { - RunFilesMetaDegradedReason::SandboxGone - } else { - RunFilesMetaDegradedReason::SandboxUnreachable - } -} - /// Build the degraded response from the stored `final_patch`. /// When `final_patch` is `None`, returns the empty envelope (UI maps this to /// R4(c)). Keeps the same `FileDiff[]` shape as live responses, but leaves @@ -768,24 +738,24 @@ async fn load_projection( .map_err(|err| ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string())) } -/// Reconnect semantics tailored to the Files endpoint: -/// - `Ok(Some(sandbox))`: reconnected, caller proceeds on the sandbox path. -/// - `Ok(None)`: no sandbox record, reconnect failed, or the provider isn't -/// supported by this build — caller falls through to the degraded fallback -/// instead of returning 409. -/// - `Err(ApiError)`: unrecoverable error loading run state. -async fn try_reconnect_run_sandbox( +async fn reconnect_run_sandbox( state: &Arc, + run_id: &RunId, projection: &fabro_store::RunProjection, -) -> std::result::Result>, ApiError> { - let Some(record) = projection.sandbox.clone() else { - return Ok(None); - }; +) -> std::result::Result, ApiError> { + let record = projection + .sandbox + .clone() + .ok_or_else(|| ApiError::new(StatusCode::CONFLICT, "Run has no active sandbox."))?; let daytona_api_key = state.vault_or_env_pub(EnvVars::DAYTONA_API_KEY); - match reconnect(&record, daytona_api_key).await { - Ok(sandbox) => Ok(Some(sandbox)), - Err(_) => Ok(None), - } + let sandbox = reconnect_for_run(&record, daytona_api_key, Some(*run_id)) + .await + .map_err(|err| ApiError::new(StatusCode::CONFLICT, err.to_string()))?; + sandbox + .start() + .await + .map_err(|err| ApiError::new(StatusCode::CONFLICT, err.display_with_causes()))?; + Ok(sandbox) } /// Resolve HEAD's SHA and its commit time in a single sandbox round-trip. diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 18e63dc3a..b5c898221 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -58,7 +58,7 @@ use fabro_llm::types::{ use fabro_model::{BilledTokenCounts, Catalog, ModelTestMode, Provider}; use fabro_redact::redact_jsonl_line; use fabro_sandbox::daytona::{self, DaytonaSandbox}; -use fabro_sandbox::reconnect::reconnect; +use fabro_sandbox::reconnect::reconnect_for_run; use fabro_sandbox::{Sandbox, SandboxProvider}; use fabro_slack::client::{PostedMessage as SlackPostedMessage, SlackClient}; use fabro_slack::config::resolve_credentials as resolve_slack_credentials; @@ -96,6 +96,7 @@ use fabro_workflow::run_lookup::{ }; use fabro_workflow::run_status::{FailureReason, RunStatus, SuccessReason}; use fabro_workflow::{Error as WorkflowError, operations, pull_request}; +use serde::Serialize; use sha2::{Digest, Sha256}; use tempfile::NamedTempFile; use tokio::fs; @@ -1590,22 +1591,40 @@ pub(crate) fn build_app_state(config: AppStateConfig) -> anyhow::Result, id: RunId, force: bool, -) -> Result<(), Response> { +) -> Result { if !force { reject_active_delete_without_force(state.as_ref(), &id).await?; } - let managed_run = if let Ok(mut runs) = state.runs.lock() { + let mut managed_run = if let Ok(mut runs) = state.runs.lock() { runs.remove(&id) } else { None }; - if let Some(mut managed_run) = managed_run { + if let Some(managed_run) = managed_run.as_mut() { if let Some(token) = &managed_run.cancel_token { token.cancel(); } @@ -1636,6 +1655,11 @@ async fn delete_run_internal( delete_grace, ) .await; + } + + let delete_outcome = delete_run_sandbox_resource(state, id, force).await?; + + if let Some(mut managed_run) = managed_run { if let Some(run_dir) = managed_run.run_dir.take() { remove_run_dir(&run_dir).map_err(|err| { ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()).into_response() @@ -1659,7 +1683,82 @@ async fn delete_run_internal( .map_err(|err| { ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()).into_response() })?; - Ok(()) + Ok(delete_outcome) +} + +async fn delete_run_sandbox_resource( + state: &Arc, + id: RunId, + force: bool, +) -> Result { + let Ok(run_store) = state.store.open_run(&id).await else { + return Ok(DeleteRunOutcome::NoContent); + }; + let projection = run_store.state().await.map_err(|err| { + ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()).into_response() + })?; + let delete_started = matches!(projection.status, Some(RunStatus::Removing)); + let can_mark_removing = projection + .status + .is_some_and(|status| status.can_transition_to(RunStatus::Removing)); + if !delete_started && can_mark_removing { + workflow_event::append_event(&run_store, &id, &workflow_event::Event::RunRemoving) + .await + .map_err(|err| { + ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()).into_response() + })?; + } + + let preserve = projection + .spec() + .is_some_and(|spec| spec.settings.run.sandbox.preserve); + let Some(record) = projection.sandbox else { + return Ok(DeleteRunOutcome::NoContent); + }; + if preserve { + let identifier = record + .identifier + .clone() + .unwrap_or_else(|| record.working_directory.clone()); + return Ok(DeleteRunOutcome::Preserved(DeleteRunResponse { + deleted: true, + sandbox_preserved: true, + sandbox: DeleteRunSandboxResponse { + provider: record.provider, + identifier, + }, + })); + } + + let daytona_api_key = state.vault_or_env(EnvVars::DAYTONA_API_KEY); + let sandbox = match reconnect_for_run(&record, daytona_api_key, Some(id)).await { + Ok(sandbox) => sandbox, + Err(err) if force || delete_started => { + tracing::warn!( + run_id = %id, + error = %render_with_causes(&err.to_string(), &collect_causes(err.as_ref())), + "Skipping sandbox provider delete during run deletion" + ); + return Ok(DeleteRunOutcome::NoContent); + } + Err(err) => { + let detail = render_with_causes(&err.to_string(), &collect_causes(err.as_ref())); + return Err(ApiError::new(StatusCode::CONFLICT, detail).into_response()); + } + }; + if let Err(err) = sandbox.delete().await { + if force || delete_started { + tracing::warn!( + run_id = %id, + error = %err.display_with_causes(), + "Skipping failed sandbox provider delete during run deletion" + ); + return Ok(DeleteRunOutcome::NoContent); + } + return Err(ApiError::new(StatusCode::CONFLICT, err.display_with_causes()).into_response()); + } + + Ok(DeleteRunOutcome::NoContent) } async fn reject_active_delete_without_force( @@ -1691,11 +1790,23 @@ async fn reject_active_delete_without_force( } match state.store.runs().find(run_id).await { - Ok(Some(summary)) if summary.status.is_active() => Err(ApiError::new( - StatusCode::CONFLICT, - active_run_delete_message(*run_id, summary.status), - ) - .into_response()), + Ok(Some(summary)) + if matches!( + summary.status, + RunStatus::Submitted + | RunStatus::Queued + | RunStatus::Starting + | RunStatus::Running + | RunStatus::Blocked { .. } + | RunStatus::Paused { .. } + ) => + { + Err(ApiError::new( + StatusCode::CONFLICT, + active_run_delete_message(*run_id, summary.status), + ) + .into_response()) + } Ok(_) => Ok(()), Err(err) => { Err(ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()).into_response()) diff --git a/lib/crates/fabro-server/src/server/handler/runs.rs b/lib/crates/fabro-server/src/server/handler/runs.rs index 1bec57597..432710ccf 100644 --- a/lib/crates/fabro-server/src/server/handler/runs.rs +++ b/lib/crates/fabro-server/src/server/handler/runs.rs @@ -392,7 +392,10 @@ async fn delete_run( }; match delete_run_internal(&state, id, query.force).await { - Ok(()) => StatusCode::NO_CONTENT.into_response(), + Ok(super::super::DeleteRunOutcome::NoContent) => StatusCode::NO_CONTENT.into_response(), + Ok(super::super::DeleteRunOutcome::Preserved(response)) => { + (StatusCode::OK, Json(response)).into_response() + } Err(response) => response, } } diff --git a/lib/crates/fabro-server/src/server/handler/sandbox.rs b/lib/crates/fabro-server/src/server/handler/sandbox.rs index 2761139d6..4493b96c8 100644 --- a/lib/crates/fabro-server/src/server/handler/sandbox.rs +++ b/lib/crates/fabro-server/src/server/handler/sandbox.rs @@ -5,7 +5,7 @@ use super::super::{ PreviewUrlRequest, PreviewUrlResponse, Query, RequiredUser, Response, Router, RunId, Sandbox, SandboxFileEntry, SandboxFileListResponse, SandboxProvider, SshAccessRequest, SshAccessResponse, State, StatusCode, collect_causes, fs, get, octet_stream_response, - parse_run_id_path, post, reconnect, reject_if_archived, render_with_causes, + parse_run_id_path, post, reconnect_for_run, reject_if_archived, render_with_causes, }; pub(super) fn routes() -> Router> { @@ -212,10 +212,16 @@ async fn reconnect_run_sandbox( ) -> Result, Response> { let record = load_run_sandbox_record(state, run_id).await?; let daytona_api_key = state.vault_or_env(EnvVars::DAYTONA_API_KEY); - reconnect(&record, daytona_api_key).await.map_err(|err| { - let detail = render_with_causes(&err.to_string(), &collect_causes(err.as_ref())); - ApiError::new(StatusCode::CONFLICT, detail).into_response() - }) + let sandbox = reconnect_for_run(&record, daytona_api_key, Some(*run_id)) + .await + .map_err(|err| { + let detail = render_with_causes(&err.to_string(), &collect_causes(err.as_ref())); + ApiError::new(StatusCode::CONFLICT, detail).into_response() + })?; + sandbox.start().await.map_err(|err| { + ApiError::new(StatusCode::CONFLICT, err.display_with_causes()).into_response() + })?; + Ok(sandbox) } async fn reconnect_daytona_sandbox( @@ -245,7 +251,7 @@ async fn reconnect_daytona_sandbox( .into_response()); }; let daytona_api_key = state.vault_or_env(EnvVars::DAYTONA_API_KEY); - DaytonaSandbox::reconnect( + let sandbox = DaytonaSandbox::reconnect( name, daytona_api_key, repo_cloned, @@ -253,7 +259,13 @@ async fn reconnect_daytona_sandbox( record.clone_branch.clone(), ) .await - .map_err(|err| ApiError::new(StatusCode::CONFLICT, err.display_with_causes()).into_response()) + .map_err(|err| { + ApiError::new(StatusCode::CONFLICT, err.display_with_causes()).into_response() + })?; + sandbox.start().await.map_err(|err| { + ApiError::new(StatusCode::CONFLICT, err.display_with_causes()).into_response() + })?; + Ok(sandbox) } async fn load_run_sandbox_record( diff --git a/lib/crates/fabro-server/src/server/tests.rs b/lib/crates/fabro-server/src/server/tests.rs index 2693c406b..6b7292ba8 100644 --- a/lib/crates/fabro-server/src/server/tests.rs +++ b/lib/crates/fabro-server/src/server/tests.rs @@ -7248,6 +7248,150 @@ async fn delete_run_removes_durable_run() { assert_status!(response, StatusCode::NOT_FOUND).await; } +#[tokio::test] +async fn delete_run_with_preserved_sandbox_returns_handoff() { + let state = test_app_state(); + let app = crate::test_support::build_test_router(Arc::clone(&state)); + let run_id = RunId::new(); + let mut settings = fabro_types::WorkflowSettings::default(); + settings.run.sandbox.preserve = true; + let graph = Graph::new("test"); + + create_durable_run_with_events(&state, run_id, &[ + workflow_event::Event::RunCreated { + run_id, + settings: serde_json::to_value(settings).unwrap(), + graph: serde_json::to_value(graph).unwrap(), + workflow_source: None, + workflow_config: None, + labels: std::collections::BTreeMap::default(), + run_dir: "/tmp/fabro-run".to_string(), + source_directory: Some("/tmp/fabro-run".to_string()), + workflow_slug: Some("test".to_string()), + db_prefix: None, + provenance: None, + manifest_blob: None, + git: None, + fork_source_ref: None, + in_place: false, + web_url: None, + }, + workflow_event::Event::RunSubmitted { + definition_blob: None, + }, + workflow_event::Event::SandboxInitialized { + provider: "local".to_string(), + working_directory: "/tmp/fabro-preserved-sandbox".to_string(), + identifier: Some("sandbox-preserve-1".to_string()), + repo_cloned: None, + clone_origin_url: None, + clone_branch: None, + }, + ]) + .await; + + let req = Request::builder() + .method("DELETE") + .uri(api(&format!("/runs/{run_id}?force=true"))) + .body(Body::empty()) + .unwrap(); + let response = app.clone().oneshot(req).await.unwrap(); + let body = response_json!(response, StatusCode::OK).await; + assert_eq!(body["deleted"].as_bool(), Some(true)); + assert_eq!(body["sandbox_preserved"].as_bool(), Some(true)); + assert_eq!(body["sandbox"]["provider"].as_str(), Some("local")); + assert_eq!( + body["sandbox"]["identifier"].as_str(), + Some("sandbox-preserve-1") + ); + + let req = Request::builder() + .method("GET") + .uri(api(&format!("/runs/{run_id}"))) + .body(Body::empty()) + .unwrap(); + let response = app.oneshot(req).await.unwrap(); + assert_status!(response, StatusCode::NOT_FOUND).await; +} + +#[tokio::test] +async fn delete_run_retry_after_missing_provider_resource_removes_metadata() { + let state = test_app_state(); + let app = crate::test_support::build_test_router(Arc::clone(&state)); + let run_id = RunId::new(); + let graph = Graph::new("test"); + + create_durable_run_with_events(&state, run_id, &[ + workflow_event::Event::RunCreated { + run_id, + settings: serde_json::to_value(fabro_types::WorkflowSettings::default()).unwrap(), + graph: serde_json::to_value(graph).unwrap(), + workflow_source: None, + workflow_config: None, + labels: std::collections::BTreeMap::default(), + run_dir: "/tmp/fabro-run".to_string(), + source_directory: Some("/tmp/fabro-run".to_string()), + workflow_slug: Some("test".to_string()), + db_prefix: None, + provenance: None, + manifest_blob: None, + git: None, + fork_source_ref: None, + in_place: false, + web_url: None, + }, + workflow_event::Event::RunSubmitted { + definition_blob: None, + }, + workflow_event::Event::RunStarting, + workflow_event::Event::RunRunning, + workflow_event::Event::SandboxInitialized { + provider: "missing-provider".to_string(), + working_directory: "/tmp/fabro-missing-sandbox".to_string(), + identifier: Some("missing-sandbox".to_string()), + repo_cloned: None, + clone_origin_url: None, + clone_branch: None, + }, + workflow_event::Event::WorkflowRunCompleted { + duration_ms: 1, + artifact_count: 0, + status: "succeeded".to_string(), + reason: SuccessReason::Completed, + total_usd_micros: None, + final_git_commit_sha: None, + final_patch: None, + diff_summary: None, + billing: None, + }, + ]) + .await; + + let req = Request::builder() + .method("DELETE") + .uri(api(&format!("/runs/{run_id}"))) + .body(Body::empty()) + .unwrap(); + let response = app.clone().oneshot(req).await.unwrap(); + response_json!(response, StatusCode::CONFLICT).await; + + let req = Request::builder() + .method("DELETE") + .uri(api(&format!("/runs/{run_id}"))) + .body(Body::empty()) + .unwrap(); + let response = app.clone().oneshot(req).await.unwrap(); + assert_status!(response, StatusCode::NO_CONTENT).await; + + let req = Request::builder() + .method("GET") + .uri(api(&format!("/runs/{run_id}"))) + .body(Body::empty()) + .unwrap(); + let response = app.oneshot(req).await.unwrap(); + assert_status!(response, StatusCode::NOT_FOUND).await; +} + #[tokio::test] async fn delete_active_run_requires_force() { let state = test_app_state(); diff --git a/lib/crates/fabro-types/src/run_event/infra.rs b/lib/crates/fabro-types/src/run_event/infra.rs index 77319920f..0c54b43f5 100644 --- a/lib/crates/fabro-types/src/run_event/infra.rs +++ b/lib/crates/fabro-types/src/run_event/infra.rs @@ -227,6 +227,63 @@ pub struct SandboxCleanupFailedProps { pub causes: Vec, } +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct SandboxStartStartedProps { + pub provider: String, +} + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct SandboxStartCompletedProps { + pub provider: String, + pub duration_ms: u64, +} + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct SandboxStartFailedProps { + pub provider: String, + pub error: String, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub causes: Vec, +} + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct SandboxStopStartedProps { + pub provider: String, +} + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct SandboxStopCompletedProps { + pub provider: String, + pub duration_ms: u64, +} + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct SandboxStopFailedProps { + pub provider: String, + pub error: String, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub causes: Vec, +} + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct SandboxDeleteStartedProps { + pub provider: String, +} + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct SandboxDeleteCompletedProps { + pub provider: String, + pub duration_ms: u64, +} + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct SandboxDeleteFailedProps { + pub provider: String, + pub error: String, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub causes: Vec, +} + #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct SnapshotNameProps { pub name: String, diff --git a/lib/crates/fabro-types/src/run_event/mod.rs b/lib/crates/fabro-types/src/run_event/mod.rs index 91bac42d7..9a2eecf24 100644 --- a/lib/crates/fabro-types/src/run_event/mod.rs +++ b/lib/crates/fabro-types/src/run_event/mod.rs @@ -218,6 +218,24 @@ pub enum EventBody { SandboxCleanupCompleted(SandboxCleanupCompletedProps), #[serde(rename = "sandbox.cleanup.failed")] SandboxCleanupFailed(SandboxCleanupFailedProps), + #[serde(rename = "sandbox.start.started")] + SandboxStartStarted(SandboxStartStartedProps), + #[serde(rename = "sandbox.start.completed")] + SandboxStartCompleted(SandboxStartCompletedProps), + #[serde(rename = "sandbox.start.failed")] + SandboxStartFailed(SandboxStartFailedProps), + #[serde(rename = "sandbox.stop.started")] + SandboxStopStarted(SandboxStopStartedProps), + #[serde(rename = "sandbox.stop.completed")] + SandboxStopCompleted(SandboxStopCompletedProps), + #[serde(rename = "sandbox.stop.failed")] + SandboxStopFailed(SandboxStopFailedProps), + #[serde(rename = "sandbox.delete.started")] + SandboxDeleteStarted(SandboxDeleteStartedProps), + #[serde(rename = "sandbox.delete.completed")] + SandboxDeleteCompleted(SandboxDeleteCompletedProps), + #[serde(rename = "sandbox.delete.failed")] + SandboxDeleteFailed(SandboxDeleteFailedProps), #[serde(rename = "sandbox.snapshot.pulling")] SnapshotPulling(SnapshotNameProps), #[serde(rename = "sandbox.snapshot.creating")] @@ -434,6 +452,15 @@ impl EventBody { Self::SandboxCleanupStarted(_) => "sandbox.cleanup.started", Self::SandboxCleanupCompleted(_) => "sandbox.cleanup.completed", Self::SandboxCleanupFailed(_) => "sandbox.cleanup.failed", + Self::SandboxStartStarted(_) => "sandbox.start.started", + Self::SandboxStartCompleted(_) => "sandbox.start.completed", + Self::SandboxStartFailed(_) => "sandbox.start.failed", + Self::SandboxStopStarted(_) => "sandbox.stop.started", + Self::SandboxStopCompleted(_) => "sandbox.stop.completed", + Self::SandboxStopFailed(_) => "sandbox.stop.failed", + Self::SandboxDeleteStarted(_) => "sandbox.delete.started", + Self::SandboxDeleteCompleted(_) => "sandbox.delete.completed", + Self::SandboxDeleteFailed(_) => "sandbox.delete.failed", Self::SnapshotPulling(_) => "sandbox.snapshot.pulling", Self::SnapshotCreating(_) => "sandbox.snapshot.creating", Self::SnapshotReady(_) => "sandbox.snapshot.ready", @@ -575,6 +602,15 @@ fn is_known_event_name(event: &str) -> bool { | "sandbox.cleanup.started" | "sandbox.cleanup.completed" | "sandbox.cleanup.failed" + | "sandbox.start.started" + | "sandbox.start.completed" + | "sandbox.start.failed" + | "sandbox.stop.started" + | "sandbox.stop.completed" + | "sandbox.stop.failed" + | "sandbox.delete.started" + | "sandbox.delete.completed" + | "sandbox.delete.failed" | "sandbox.snapshot.pulling" | "sandbox.snapshot.creating" | "sandbox.snapshot.ready" diff --git a/lib/crates/fabro-types/src/settings/run.rs b/lib/crates/fabro-types/src/settings/run.rs index ba57c28d4..f61123785 100644 --- a/lib/crates/fabro-types/src/settings/run.rs +++ b/lib/crates/fabro-types/src/settings/run.rs @@ -197,25 +197,32 @@ pub struct RunCheckpointSettings { #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct RunSandboxSettings { - pub provider: String, - pub preserve: bool, - pub devcontainer: bool, - pub env: HashMap, - pub local: LocalSandboxSettings, - pub docker: Option, - pub daytona: Option, + pub provider: String, + pub preserve: bool, + #[serde(default = "default_stop_on_terminal")] + pub stop_on_terminal: bool, + pub devcontainer: bool, + pub env: HashMap, + pub local: LocalSandboxSettings, + pub docker: Option, + pub daytona: Option, +} + +fn default_stop_on_terminal() -> bool { + true } impl Default for RunSandboxSettings { fn default() -> Self { Self { - provider: "local".to_string(), - preserve: false, - devcontainer: false, - env: HashMap::new(), - local: LocalSandboxSettings::default(), - docker: None, - daytona: None, + provider: "local".to_string(), + preserve: false, + stop_on_terminal: true, + devcontainer: false, + env: HashMap::new(), + local: LocalSandboxSettings::default(), + docker: None, + daytona: None, } } } diff --git a/lib/crates/fabro-types/src/status.rs b/lib/crates/fabro-types/src/status.rs index 63cdf729e..398be2770 100644 --- a/lib/crates/fabro-types/src/status.rs +++ b/lib/crates/fabro-types/src/status.rs @@ -31,9 +31,10 @@ impl RunStatus { } /// Whether the run's status is frozen and cannot transition outbound - /// (except via the `* -> Dead` escape hatch). `Archived` is intentionally - /// NOT immutable — it can transition back to its prior terminal status - /// via `unarchive`. + /// through normal lifecycle events. Deletion and the `* -> Dead` escape + /// hatch are allowed separately. `Archived` is intentionally NOT + /// immutable — it can transition back to its prior terminal status via + /// `unarchive`. pub fn is_immutable(self) -> bool { matches!( self, @@ -75,6 +76,9 @@ impl RunStatus { if matches!(to, Self::Dead) { return true; } + if matches!(to, Self::Removing) { + return !matches!(self, Self::Removing); + } if matches!((self, to), (Self::Failed { .. }, Self::Submitted)) { return true; } @@ -450,6 +454,37 @@ mod tests { assert!(!RunStatus::Paused { prior_block: None }.can_transition_to(archived)); } + #[test] + fn run_statuses_can_transition_to_removing_for_deletion() { + let removing = RunStatus::Removing; + for status in [ + RunStatus::Submitted, + RunStatus::Queued, + RunStatus::Starting, + RunStatus::Running, + RunStatus::Blocked { + blocked_reason: BlockedReason::HumanInputRequired, + }, + RunStatus::Paused { prior_block: None }, + RunStatus::Succeeded { + reason: SuccessReason::Completed, + }, + RunStatus::Failed { + reason: FailureReason::Cancelled, + }, + RunStatus::Dead, + RunStatus::Archived { + prior: TerminalStatus::Dead, + }, + ] { + assert!( + status.can_transition_to(removing), + "{status} should transition to removing" + ); + } + assert!(!removing.can_transition_to(removing)); + } + #[test] fn archived_to_archived_is_rejected() { let archived = RunStatus::Archived { diff --git a/lib/crates/fabro-workflow/Cargo.toml b/lib/crates/fabro-workflow/Cargo.toml index 8455a4871..01f4e7b10 100644 --- a/lib/crates/fabro-workflow/Cargo.toml +++ b/lib/crates/fabro-workflow/Cargo.toml @@ -71,6 +71,7 @@ toml.workspace = true fabro-vault = { path = "../fabro-vault" } [dev-dependencies] base64.workspace = true +fabro-sandbox = { path = "../fabro-sandbox", features = ["daytona", "test-support"] } fabro-mcp = { path = "../fabro-mcp" } tokio = { workspace = true, features = ["test-util", "macros"] } object_store.workspace = true diff --git a/lib/crates/fabro-workflow/src/event/convert.rs b/lib/crates/fabro-workflow/src/event/convert.rs index 86348dbfb..18ddd3860 100644 --- a/lib/crates/fabro-workflow/src/event/convert.rs +++ b/lib/crates/fabro-workflow/src/event/convert.rs @@ -782,6 +782,69 @@ fn event_body_from_event(event: &Event) -> EventBody { error: error.clone(), causes: causes.clone(), }), + SandboxEvent::StartStarted { provider } => { + EventBody::SandboxStartStarted(fabro_types::SandboxStartStartedProps { + provider: provider.clone(), + }) + } + SandboxEvent::StartCompleted { + provider, + duration_ms, + } => EventBody::SandboxStartCompleted(fabro_types::SandboxStartCompletedProps { + provider: provider.clone(), + duration_ms: *duration_ms, + }), + SandboxEvent::StartFailed { + provider, + error, + causes, + } => EventBody::SandboxStartFailed(fabro_types::SandboxStartFailedProps { + provider: provider.clone(), + error: error.clone(), + causes: causes.clone(), + }), + SandboxEvent::StopStarted { provider } => { + EventBody::SandboxStopStarted(fabro_types::SandboxStopStartedProps { + provider: provider.clone(), + }) + } + SandboxEvent::StopCompleted { + provider, + duration_ms, + } => EventBody::SandboxStopCompleted(fabro_types::SandboxStopCompletedProps { + provider: provider.clone(), + duration_ms: *duration_ms, + }), + SandboxEvent::StopFailed { + provider, + error, + causes, + } => EventBody::SandboxStopFailed(fabro_types::SandboxStopFailedProps { + provider: provider.clone(), + error: error.clone(), + causes: causes.clone(), + }), + SandboxEvent::DeleteStarted { provider } => { + EventBody::SandboxDeleteStarted(fabro_types::SandboxDeleteStartedProps { + provider: provider.clone(), + }) + } + SandboxEvent::DeleteCompleted { + provider, + duration_ms, + } => EventBody::SandboxDeleteCompleted(fabro_types::SandboxDeleteCompletedProps { + provider: provider.clone(), + duration_ms: *duration_ms, + }), + SandboxEvent::DeleteFailed { + provider, + error, + causes, + } => EventBody::SandboxDeleteFailed(fabro_types::SandboxDeleteFailedProps { + provider: provider.clone(), + error: error.clone(), + causes: causes.clone(), + }), SandboxEvent::SnapshotPulling { name } => { EventBody::SnapshotPulling(fabro_types::SnapshotNameProps { name: name.clone() }) } @@ -1431,6 +1494,25 @@ mod tests { assert_eq!(properties["duration_ms"], 2500); } + #[test] + fn run_event_sandbox_stop_and_delete_use_distinct_event_names() { + let stopped = to_run_event(&fixtures::RUN_5, &Event::Sandbox { + event: SandboxEvent::StopCompleted { + provider: "docker".to_string(), + duration_ms: 10, + }, + }); + let deleted = to_run_event(&fixtures::RUN_5, &Event::Sandbox { + event: SandboxEvent::DeleteCompleted { + provider: "docker".to_string(), + duration_ms: 20, + }, + }); + + assert_eq!(stopped.event_name(), "sandbox.stop.completed"); + assert_eq!(deleted.event_name(), "sandbox.delete.completed"); + } + #[test] fn run_event_sandbox_failure_serializes_causes() { let stored = to_run_event(&fixtures::RUN_5, &Event::Sandbox { diff --git a/lib/crates/fabro-workflow/src/event/names.rs b/lib/crates/fabro-workflow/src/event/names.rs index 69ae5ef63..7696aaa7a 100644 --- a/lib/crates/fabro-workflow/src/event/names.rs +++ b/lib/crates/fabro-workflow/src/event/names.rs @@ -93,6 +93,15 @@ pub fn event_name(event: &Event) -> &'static str { SandboxEvent::CleanupStarted { .. } => "sandbox.cleanup.started", SandboxEvent::CleanupCompleted { .. } => "sandbox.cleanup.completed", SandboxEvent::CleanupFailed { .. } => "sandbox.cleanup.failed", + SandboxEvent::StartStarted { .. } => "sandbox.start.started", + SandboxEvent::StartCompleted { .. } => "sandbox.start.completed", + SandboxEvent::StartFailed { .. } => "sandbox.start.failed", + SandboxEvent::StopStarted { .. } => "sandbox.stop.started", + SandboxEvent::StopCompleted { .. } => "sandbox.stop.completed", + SandboxEvent::StopFailed { .. } => "sandbox.stop.failed", + SandboxEvent::DeleteStarted { .. } => "sandbox.delete.started", + SandboxEvent::DeleteCompleted { .. } => "sandbox.delete.completed", + SandboxEvent::DeleteFailed { .. } => "sandbox.delete.failed", SandboxEvent::SnapshotPulling { .. } => "sandbox.snapshot.pulling", SandboxEvent::SnapshotCreating { .. } => "sandbox.snapshot.creating", SandboxEvent::SnapshotReady { .. } => "sandbox.snapshot.ready", diff --git a/lib/crates/fabro-workflow/src/operations/start.rs b/lib/crates/fabro-workflow/src/operations/start.rs index e6b3337b4..5541e83db 100644 --- a/lib/crates/fabro-workflow/src/operations/start.rs +++ b/lib/crates/fabro-workflow/src/operations/start.rs @@ -76,6 +76,7 @@ struct RunSession { registry_override: Option>, retro_enabled: bool, preserve_sandbox: bool, + stop_on_terminal: bool, pr_config: Option, pr_github_app: Option, pr_origin_url: Option, @@ -451,6 +452,7 @@ impl RunSession { registry_override: services.registry_override, retro_enabled: resolved.execution.retros && project_config::is_retro_enabled(), preserve_sandbox: resolved.sandbox.preserve, + stop_on_terminal: resolved.sandbox.stop_on_terminal, pr_config, pr_github_app: services.github_app, pr_origin_url: record.repo_origin_url().map(str::to_string), @@ -692,7 +694,6 @@ impl RunSession { persisted: Persisted, checkpoint: Option, ) -> Result { - let preserve_sandbox = self.preserve_sandbox; let on_node = self.on_node.clone(); let record = persisted.run_spec(); @@ -770,13 +771,14 @@ impl RunSession { initialized.on_node = on_node; let sandbox_for_cleanup = Arc::clone(&initialized.engine.run.sandbox); + let stop_on_terminal = self.stop_on_terminal; let cleanup_guard = scopeguard::guard((), move |()| { - if preserve_sandbox { + if !stop_on_terminal { return; } if let Ok(handle) = Handle::try_current() { handle.spawn(async move { - let _ = sandbox_for_cleanup.cleanup().await; + let _ = sandbox_for_cleanup.stop().await; }); } }); @@ -817,6 +819,7 @@ impl RunSession { run_id: retroed.run_options.run_id, workflow_name: retroed.graph.name.clone(), preserve_sandbox: self.preserve_sandbox, + stop_on_terminal: self.stop_on_terminal, last_git_sha: last_git_sha.lock().unwrap().clone(), }; let pr_opts = PullRequestOptions { diff --git a/lib/crates/fabro-workflow/src/pipeline/finalize.rs b/lib/crates/fabro-workflow/src/pipeline/finalize.rs index 3189ee21f..4afcfb917 100644 --- a/lib/crates/fabro-workflow/src/pipeline/finalize.rs +++ b/lib/crates/fabro-workflow/src/pipeline/finalize.rs @@ -499,11 +499,11 @@ pub(crate) fn build_terminal_event( } } -async fn cleanup_sandbox( +async fn stop_sandbox_on_terminal( services: &RunServices, run_id: &fabro_types::RunId, workflow_name: &str, - preserve: bool, + stop_on_terminal: bool, ) -> fabro_sandbox::Result<()> { let hook_ctx = HookContext::new( HookEvent::SandboxCleanup, @@ -511,8 +511,8 @@ async fn cleanup_sandbox( workflow_name.to_string(), ); let _ = services.run_hooks(&hook_ctx).await; - if !preserve { - services.sandbox.cleanup().await?; + if stop_on_terminal { + services.sandbox.stop().await?; } Ok(()) } @@ -602,20 +602,20 @@ pub async fn finalize(retroed: Retroed, options: &FinalizeOptions) -> Result = if let Some(plan) = worktree_plan.as_ref() { + let mut sandbox_initialized = true; + let sandbox: Arc = if attach_existing { + let run_state = options + .run_store + .state() + .await + .map_err(|err| Error::engine(err.to_string()))?; + let record = run_state.sandbox.ok_or_else(|| { + Error::Precondition("cannot resume run: sandbox record is missing".to_string()) + })?; + let daytona_api_key = match &options.vault { + Some(vault) => vault + .read() + .await + .get(EnvVars::DAYTONA_API_KEY) + .map(str::to_string), + None => None, + }; + let sandbox = reconnect_for_run_with_callback( + &record, + daytona_api_key, + Some(options.run_id), + Some(Arc::clone(&sandbox_event_callback)), + ) + .await + .map_err(|err| Error::engine_with_anyhow("Failed to reconnect sandbox for resume", &err))?; + sandbox_initialized = false; + Arc::new(ReadBeforeWriteSandbox::new(Arc::from(sandbox))) + } else if let Some(plan) = worktree_plan.as_ref() { let inner = options .sandbox .build(Some(Arc::clone(&sandbox_event_callback))) @@ -552,18 +586,27 @@ pub async fn initialize( } options.run_options.git = None; } - let cleanup_guard = scopeguard::guard(Arc::clone(&sandbox), |sandbox| { - if let Ok(handle) = Handle::try_current() { - handle.spawn(async move { - let _ = sandbox.cleanup().await; - }); - } + let cleanup_guard = (!attach_existing).then(|| { + scopeguard::guard(Arc::clone(&sandbox), |sandbox| { + if let Ok(handle) = Handle::try_current() { + handle.spawn(async move { + let _ = sandbox.delete().await; + }); + } + }) }); - sandbox - .initialize() - .await - .map_err(|e| Error::engine_with_source("Failed to initialize sandbox", &e))?; + if attach_existing { + sandbox + .start() + .await + .map_err(|e| Error::engine_with_source("Failed to start sandbox", &e))?; + } else { + sandbox + .initialize() + .await + .map_err(|e| Error::engine_with_source("Failed to initialize sandbox", &e))?; + } let hook_ctx = HookContext::new( HookEvent::SandboxReady, @@ -582,15 +625,17 @@ pub async fn initialize( return Err(Error::engine(msg)); } - let sandbox_record = options.sandbox.to_sandbox_record(&*sandbox); - options.emitter.emit(&Event::SandboxInitialized { - working_directory: sandbox_record.working_directory.clone(), - provider: sandbox_record.provider.clone(), - identifier: sandbox_record.identifier.clone(), - repo_cloned: sandbox_record.repo_cloned, - clone_origin_url: sandbox_record.clone_origin_url.clone(), - clone_branch: sandbox_record.clone_branch.clone(), - }); + if sandbox_initialized { + let sandbox_record = options.sandbox.to_sandbox_record(&*sandbox); + options.emitter.emit(&Event::SandboxInitialized { + working_directory: sandbox_record.working_directory.clone(), + provider: sandbox_record.provider.clone(), + identifier: sandbox_record.identifier.clone(), + repo_cloned: sandbox_record.repo_cloned, + clone_origin_url: sandbox_record.clone_origin_url.clone(), + clone_branch: sandbox_record.clone_branch.clone(), + }); + } let (base_env, github_token) = build_sandbox_env( &options.sandbox_env, @@ -783,7 +828,9 @@ pub async fn initialize( workflow_bundle: options.workflow_bundle.clone(), }); - scopeguard::ScopeGuard::into_inner(cleanup_guard); + if let Some(cleanup_guard) = cleanup_guard { + scopeguard::ScopeGuard::into_inner(cleanup_guard); + } Ok(Initialized { graph, diff --git a/lib/crates/fabro-workflow/src/pipeline/types.rs b/lib/crates/fabro-workflow/src/pipeline/types.rs index 0913d1843..b0113a9a3 100644 --- a/lib/crates/fabro-workflow/src/pipeline/types.rs +++ b/lib/crates/fabro-workflow/src/pipeline/types.rs @@ -343,6 +343,7 @@ pub struct FinalizeOptions { pub run_id: RunId, pub workflow_name: String, pub preserve_sandbox: bool, + pub stop_on_terminal: bool, pub last_git_sha: Option, } diff --git a/lib/packages/fabro-api-client/src/.openapi-generator/FILES b/lib/packages/fabro-api-client/src/.openapi-generator/FILES index 655643687..ebf9b9f6f 100644 --- a/lib/packages/fabro-api-client/src/.openapi-generator/FILES +++ b/lib/packages/fabro-api-client/src/.openapi-generator/FILES @@ -62,6 +62,8 @@ models/daytona-network-layer-one-of.ts models/daytona-network-layer.ts models/daytona-settings.ts models/daytona-snapshot-settings.ts +models/delete-run-response.ts +models/delete-run-sandbox.ts models/delete-secret-request.ts models/demo-toggle-request.ts models/demo-toggle-response.ts diff --git a/lib/packages/fabro-api-client/src/api/runs-api.ts b/lib/packages/fabro-api-client/src/api/runs-api.ts index 2be64a53d..02116a0be 100644 --- a/lib/packages/fabro-api-client/src/api/runs-api.ts +++ b/lib/packages/fabro-api-client/src/api/runs-api.ts @@ -26,6 +26,8 @@ import type { CloseRunPullRequestResponse } from '../models'; // @ts-ignore import type { CreateRunPullRequestRequest } from '../models'; // @ts-ignore +import type { DeleteRunResponse } from '../models'; +// @ts-ignore import type { ErrorResponse } from '../models'; // @ts-ignore import type { ForkRequest } from '../models'; @@ -275,7 +277,7 @@ export const RunsApiAxiosParamCreator = function (configuration?: Configuration) }; }, /** - * Deletes durable store state for a run. This does not remove any local run directory. Active runs require `force=true`. + * Deletes durable store state, local run scratch data, and the run-owned sandbox unless sandbox preservation is enabled. Active runs require `force=true`. * @summary Delete Run * @param {string} id Unique run identifier (ULID). * @param {boolean} [force] Whether to force deletion of an active run. Defaults to `false`. @@ -1162,14 +1164,14 @@ export const RunsApiFp = function(configuration?: Configuration) { return (axios, basePath) => createRequestFunction(localVarAxiosArgs, globalAxios, BASE_PATH, configuration)(axios, localVarOperationServerBasePath || basePath); }, /** - * Deletes durable store state for a run. This does not remove any local run directory. Active runs require `force=true`. + * Deletes durable store state, local run scratch data, and the run-owned sandbox unless sandbox preservation is enabled. Active runs require `force=true`. * @summary Delete Run * @param {string} id Unique run identifier (ULID). * @param {boolean} [force] Whether to force deletion of an active run. Defaults to `false`. * @param {*} [options] Override http request option. * @throws {RequiredError} */ - async deleteRun(id: string, force?: boolean, options?: RawAxiosRequestConfig): Promise<(axios?: AxiosInstance, basePath?: string) => AxiosPromise> { + async deleteRun(id: string, force?: boolean, options?: RawAxiosRequestConfig): Promise<(axios?: AxiosInstance, basePath?: string) => AxiosPromise> { const localVarAxiosArgs = await localVarAxiosParamCreator.deleteRun(id, force, options); const localVarOperationServerIndex = configuration?.serverIndex ?? 0; const localVarOperationServerBasePath = operationServerMap['RunsApi.deleteRun']?.[localVarOperationServerIndex]?.url; @@ -1479,14 +1481,14 @@ export const RunsApiFactory = function (configuration?: Configuration, basePath? return localVarFp.createRunPullRequest(id, createRunPullRequestRequest, options).then((request) => request(axios, basePath)); }, /** - * Deletes durable store state for a run. This does not remove any local run directory. Active runs require `force=true`. + * Deletes durable store state, local run scratch data, and the run-owned sandbox unless sandbox preservation is enabled. Active runs require `force=true`. * @summary Delete Run * @param {string} id Unique run identifier (ULID). * @param {boolean} [force] Whether to force deletion of an active run. Defaults to `false`. * @param {*} [options] Override http request option. * @throws {RequiredError} */ - deleteRun(id: string, force?: boolean, options?: RawAxiosRequestConfig): AxiosPromise { + deleteRun(id: string, force?: boolean, options?: RawAxiosRequestConfig): AxiosPromise { return localVarFp.deleteRun(id, force, options).then((request) => request(axios, basePath)); }, /** @@ -1742,7 +1744,7 @@ export class RunsApi extends BaseAPI { } /** - * Deletes durable store state for a run. This does not remove any local run directory. Active runs require `force=true`. + * Deletes durable store state, local run scratch data, and the run-owned sandbox unless sandbox preservation is enabled. Active runs require `force=true`. * @summary Delete Run * @param {string} id Unique run identifier (ULID). * @param {boolean} [force] Whether to force deletion of an active run. Defaults to `false`. diff --git a/lib/packages/fabro-api-client/src/models/delete-run-response.ts b/lib/packages/fabro-api-client/src/models/delete-run-response.ts new file mode 100644 index 000000000..696f161ca --- /dev/null +++ b/lib/packages/fabro-api-client/src/models/delete-run-response.ts @@ -0,0 +1,27 @@ +/* tslint:disable */ +/* eslint-disable */ +/** + * Fabro Run API + * HTTP API for managing Fabro workflow run executions. + * + * The version of the OpenAPI document: 0.1.0 + * + * + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). + * https://openapi-generator.tech + * Do not edit the class manually. + */ + + +// May contain unused imports in some cases +// @ts-ignore +import type { DeleteRunSandbox } from './delete-run-sandbox'; + +/** + * Returned when a run is deleted but its sandbox is intentionally preserved. + */ +export interface DeleteRunResponse { + 'deleted': boolean; + 'sandbox_preserved': boolean; + 'sandbox': DeleteRunSandbox; +} diff --git a/lib/packages/fabro-api-client/src/models/delete-run-sandbox.ts b/lib/packages/fabro-api-client/src/models/delete-run-sandbox.ts new file mode 100644 index 000000000..44a3d093b --- /dev/null +++ b/lib/packages/fabro-api-client/src/models/delete-run-sandbox.ts @@ -0,0 +1,20 @@ +/* tslint:disable */ +/* eslint-disable */ +/** + * Fabro Run API + * HTTP API for managing Fabro workflow run executions. + * + * The version of the OpenAPI document: 0.1.0 + * + * + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). + * https://openapi-generator.tech + * Do not edit the class manually. + */ + + + +export interface DeleteRunSandbox { + 'provider': string; + 'identifier': string; +} diff --git a/lib/packages/fabro-api-client/src/models/index.ts b/lib/packages/fabro-api-client/src/models/index.ts index e83c07657..af3ab1e12 100644 --- a/lib/packages/fabro-api-client/src/models/index.ts +++ b/lib/packages/fabro-api-client/src/models/index.ts @@ -40,6 +40,8 @@ export * from './daytona-network-layer-one-of'; export * from './daytona-network-layer-one-of-allow-list'; export * from './daytona-settings'; export * from './daytona-snapshot-settings'; +export * from './delete-run-response'; +export * from './delete-run-sandbox'; export * from './delete-secret-request'; export * from './demo-toggle-request'; export * from './demo-toggle-response'; diff --git a/lib/packages/fabro-api-client/src/models/run-sandbox-settings.ts b/lib/packages/fabro-api-client/src/models/run-sandbox-settings.ts index 749170056..ddfede4ee 100644 --- a/lib/packages/fabro-api-client/src/models/run-sandbox-settings.ts +++ b/lib/packages/fabro-api-client/src/models/run-sandbox-settings.ts @@ -26,6 +26,7 @@ import type { LocalSandboxSettings } from './local-sandbox-settings'; export interface RunSandboxSettings { 'provider': string; 'preserve': boolean; + 'stop_on_terminal': boolean; 'devcontainer': boolean; 'env': { [key: string]: string; }; 'local': LocalSandboxSettings;