mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-08 03:10:26 +00:00
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.
This commit is contained in:
parent
c1b5f15bbd
commit
79a095d7c6
7 changed files with 76 additions and 14 deletions
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
]);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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]",
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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<Mutex<Vec<SandboxEvent>>> = 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();
|
||||
|
|
|
|||
|
|
@ -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<String>,
|
||||
|
|
@ -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<String>,
|
||||
|
|
|
|||
|
|
@ -256,7 +256,18 @@ async fn materialize_sandbox_path(state: &Arc<AppState>, 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?;
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue