From 79a095d7c665f270cb15ab9cf29cc81652934136 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 8 May 2026 22:41:42 -0400 Subject: [PATCH] fix(runs): restore terminal sandbox regressions Emit local sandbox stop events so CLI follow and event-history tests can observe terminal cleanup under run-owned sandbox lifecycle. Fall back to stored file diffs when completed runs no longer have an active sandbox. --- lib/crates/fabro-cli/tests/it/cmd/attach.rs | 3 +- lib/crates/fabro-cli/tests/it/cmd/events.rs | 12 +++--- lib/crates/fabro-cli/tests/it/cmd/run.rs | 6 +-- lib/crates/fabro-cli/tests/it/cmd/support.rs | 6 +-- lib/crates/fabro-sandbox/src/local.rs | 42 ++++++++++++++++++++ lib/crates/fabro-sandbox/src/reconnect.rs | 8 ++++ lib/crates/fabro-server/src/run_files.rs | 13 +++++- 7 files changed, 76 insertions(+), 14 deletions(-) diff --git a/lib/crates/fabro-cli/tests/it/cmd/attach.rs b/lib/crates/fabro-cli/tests/it/cmd/attach.rs index b4fc5a53a..339d81d1e 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/attach.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/attach.rs @@ -988,7 +988,8 @@ fn attach_json_errors_without_prompting_for_human_input() { "worktree_mode": "always" }, "preserve": false, - "provider": "local" + "provider": "local", + "stop_on_terminal": true }, "scm": { "github": null, diff --git a/lib/crates/fabro-cli/tests/it/cmd/events.rs b/lib/crates/fabro-cli/tests/it/cmd/events.rs index 7e664d325..b195d2359 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/events.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/events.rs @@ -97,7 +97,7 @@ fn events_completed_run_outputs_raw_ndjson() { "stage.started", "stage.completed", "run.completed", - "sandbox.cleanup.completed", + "sandbox.stop.completed", ]); } @@ -131,8 +131,8 @@ fn events_completed_run_reads_store_without_progress_jsonl() { success: true exit_code: 0 ----- stdout ----- - {"actor":{"kind":"worker","run_id":"[ULID]"},"event":"sandbox.cleanup.started","id":"[EVENT_ID]","properties":{"provider":"local"},"run_id":"[ULID]","ts":"[TIMESTAMP]"} - {"actor":{"kind":"worker","run_id":"[ULID]"},"event":"sandbox.cleanup.completed","id":"[EVENT_ID]","properties":{"duration_ms": [DURATION_MS],"provider":"local"},"run_id":"[ULID]","ts":"[TIMESTAMP]"} + {"actor":{"kind":"worker","run_id":"[ULID]"},"event":"sandbox.stop.started","id":"[EVENT_ID]","properties":{"provider":"local"},"run_id":"[ULID]","ts":"[TIMESTAMP]"} + {"actor":{"kind":"worker","run_id":"[ULID]"},"event":"sandbox.stop.completed","id":"[EVENT_ID]","properties":{"duration_ms": [DURATION_MS],"provider":"local"},"run_id":"[ULID]","ts":"[TIMESTAMP]"} ----- stderr ----- "#); } @@ -165,8 +165,8 @@ fn events_tail_limits_output() { success: true exit_code: 0 ----- stdout ----- - {"actor":{"kind":"worker","run_id":"[ULID]"},"event":"sandbox.cleanup.started","id":"[EVENT_ID]","properties":{"provider":"local"},"run_id":"[ULID]","ts":"[TIMESTAMP]"} - {"actor":{"kind":"worker","run_id":"[ULID]"},"event":"sandbox.cleanup.completed","id":"[EVENT_ID]","properties":{"duration_ms": [DURATION_MS],"provider":"local"},"run_id":"[ULID]","ts":"[TIMESTAMP]"} + {"actor":{"kind":"worker","run_id":"[ULID]"},"event":"sandbox.stop.started","id":"[EVENT_ID]","properties":{"provider":"local"},"run_id":"[ULID]","ts":"[TIMESTAMP]"} + {"actor":{"kind":"worker","run_id":"[ULID]"},"event":"sandbox.stop.completed","id":"[EVENT_ID]","properties":{"duration_ms": [DURATION_MS],"provider":"local"},"run_id":"[ULID]","ts":"[TIMESTAMP]"} ----- stderr ----- "#); } @@ -245,6 +245,6 @@ fn events_follow_detached_run_streams_until_completion() { "stage.started", "stage.completed", "run.completed", - "sandbox.cleanup.completed", + "sandbox.stop.completed", ]); } diff --git a/lib/crates/fabro-cli/tests/it/cmd/run.rs b/lib/crates/fabro-cli/tests/it/cmd/run.rs index 21abe81ed..581889f08 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/run.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/run.rs @@ -831,7 +831,7 @@ fn dry_run_persists_event_history_in_store() { .success(); let run_dir = context.find_run_dir(&run_id); - wait_for_event_names(&run_dir, &["run.completed", "sandbox.cleanup.completed"]); + wait_for_event_names(&run_dir, &["run.completed", "sandbox.stop.completed"]); let output = context .command() .args(["events", &run_id]) @@ -872,7 +872,7 @@ fn dry_run_persists_event_history_in_store() { ); assert_eq!( progress.last().and_then(|event| event["event"].as_str()), - Some("sandbox.cleanup.completed") + Some("sandbox.stop.completed") ); let tail_output = context @@ -898,7 +898,7 @@ fn dry_run_persists_event_history_in_store() { "kind": "worker", "run_id": "[ULID]" }, - "event": "sandbox.cleanup.completed", + "event": "sandbox.stop.completed", "id": "[EVENT_ID]", "properties": { "duration_ms": "[DURATION_MS]", diff --git a/lib/crates/fabro-cli/tests/it/cmd/support.rs b/lib/crates/fabro-cli/tests/it/cmd/support.rs index 6cf1977e4..1932f27cc 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/support.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/support.rs @@ -220,7 +220,7 @@ fn run_completed_dry_run(context: &TestContext, workflow: &Path) -> RunSetup { }; wait_for_event_names(&run_setup.run_dir, &[ "run.completed", - "sandbox.cleanup.completed", + "sandbox.stop.completed", ]); run_setup } @@ -1084,7 +1084,7 @@ async fn append_seeded_simple_completion_events( base_url, &run.run_id, None, - "sandbox.cleanup.started", + "sandbox.stop.started", serde_json::json!({ "provider": "local", }), @@ -1095,7 +1095,7 @@ async fn append_seeded_simple_completion_events( base_url, &run.run_id, None, - "sandbox.cleanup.completed", + "sandbox.stop.completed", serde_json::json!({ "provider": "local", "duration_ms": 1, diff --git a/lib/crates/fabro-sandbox/src/local.rs b/lib/crates/fabro-sandbox/src/local.rs index 7deb20a38..d7709bace 100644 --- a/lib/crates/fabro-sandbox/src/local.rs +++ b/lib/crates/fabro-sandbox/src/local.rs @@ -620,6 +620,19 @@ impl Sandbox for LocalSandbox { Ok(()) } + async fn stop(&self) -> crate::Result<()> { + self.emit(SandboxEvent::StopStarted { + provider: "local".into(), + }); + let start = Instant::now(); + let duration_ms = u64::try_from(start.elapsed().as_millis()).unwrap_or(u64::MAX); + self.emit(SandboxEvent::StopCompleted { + provider: "local".into(), + duration_ms, + }); + Ok(()) + } + async fn delete(&self) -> crate::Result<()> { Ok(()) } @@ -1050,6 +1063,35 @@ mod tests { std::fs::remove_dir_all(&dir).unwrap(); } + #[tokio::test] + async fn stop_emits_events() { + use std::sync::{Arc, Mutex}; + + use crate::SandboxEvent; + + let dir = temp_dir(); + let events: Arc>> = Arc::new(Mutex::new(Vec::new())); + let events_clone = Arc::clone(&events); + + let mut env = LocalSandbox::new(dir.clone()); + env.set_event_callback(Arc::new(move |e| { + events_clone.lock().unwrap().push(e); + })); + + env.stop().await.unwrap(); + + let captured = events.lock().unwrap(); + assert_eq!(captured.len(), 2); + assert!( + matches!(&captured[0], SandboxEvent::StopStarted { provider } if provider == "local") + ); + assert!( + matches!(&captured[1], SandboxEvent::StopCompleted { provider, .. } if provider == "local") + ); + + std::fs::remove_dir_all(&dir).unwrap(); + } + #[tokio::test] async fn grep_finds_matches() { let dir = temp_dir(); diff --git a/lib/crates/fabro-sandbox/src/reconnect.rs b/lib/crates/fabro-sandbox/src/reconnect.rs index 7f90d11a9..961ab06cd 100644 --- a/lib/crates/fabro-sandbox/src/reconnect.rs +++ b/lib/crates/fabro-sandbox/src/reconnect.rs @@ -31,6 +31,10 @@ pub async fn reconnect( reconnect_for_run(record, daytona_api_key, None).await } +#[allow( + unused_variables, + reason = "Feature-gated sandbox backends leave parameters unused on partial builds." +)] pub async fn reconnect_for_run( record: &SandboxRecord, daytona_api_key: Option, @@ -39,6 +43,10 @@ pub async fn reconnect_for_run( reconnect_for_run_with_callback(record, daytona_api_key, run_id, None).await } +#[allow( + unused_variables, + reason = "Feature-gated sandbox backends leave parameters unused on partial builds." +)] pub async fn reconnect_for_run_with_callback( record: &SandboxRecord, daytona_api_key: Option, diff --git a/lib/crates/fabro-server/src/run_files.rs b/lib/crates/fabro-server/src/run_files.rs index 1db9aafe0..cecc79670 100644 --- a/lib/crates/fabro-server/src/run_files.rs +++ b/lib/crates/fabro-server/src/run_files.rs @@ -256,7 +256,18 @@ async fn materialize_sandbox_path(state: &Arc, run_id: &RunId) -> List return Ok(empty_envelope()); }; - let sandbox = reconnect_run_sandbox(state, run_id, &projection).await?; + let sandbox = match reconnect_run_sandbox(state, run_id, &projection).await { + Ok(sandbox) => sandbox, + Err(err) if err.status() == StatusCode::CONFLICT => { + return Ok(build_fallback_response( + &projection, + RunFilesMetaDegradedReason::SandboxGone, + run_id, + start, + )); + } + Err(err) => return Err(err), + }; // 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?;