mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-08-28 05:27:41 +00:00
Allow runnable runs to persist pre-start failures
This commit is contained in:
parent
0abf2297c0
commit
dd79336785
3 changed files with 288 additions and 7 deletions
|
|
@ -4,7 +4,7 @@ use std::os::unix::fs::PermissionsExt;
|
|||
use std::path::{Path, PathBuf};
|
||||
#[cfg(unix)]
|
||||
use std::process::Stdio;
|
||||
use std::sync::atomic::{AtomicBool, Ordering};
|
||||
use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
|
||||
use std::sync::{Arc as StdArc, Mutex as StdMutex};
|
||||
|
||||
use async_zip::base::read::mem::ZipFileReader;
|
||||
|
|
@ -58,7 +58,7 @@ use crate::worker_control::{
|
|||
LocalWorkerControlBus, WorkerControlBus, WorkerControlCursor, WorkerControlReceiver,
|
||||
};
|
||||
use crate::worker_runtime::{
|
||||
LocalWorkerRuntime, StartedWorker, WorkerLaunchSpec, WorkerRef, WorkerRuntime,
|
||||
LocalWorkerRuntime, StartedWorker, WorkerExit, WorkerLaunchSpec, WorkerRef, WorkerRuntime,
|
||||
};
|
||||
|
||||
const MINIMAL_DOT: &str = r#"digraph Test {
|
||||
|
|
@ -2982,6 +2982,60 @@ impl RecordingWorkerRuntime {
|
|||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
enum PreStartWorkerOutcome {
|
||||
LaunchFailure,
|
||||
EarlyExit,
|
||||
}
|
||||
|
||||
struct PreStartWorkerRuntime {
|
||||
outcome: PreStartWorkerOutcome,
|
||||
starts: AtomicUsize,
|
||||
}
|
||||
|
||||
impl PreStartWorkerRuntime {
|
||||
fn new(outcome: PreStartWorkerOutcome) -> Self {
|
||||
Self {
|
||||
outcome,
|
||||
starts: AtomicUsize::new(0),
|
||||
}
|
||||
}
|
||||
|
||||
fn start_count(&self) -> usize {
|
||||
self.starts.load(Ordering::Relaxed)
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl WorkerRuntime for PreStartWorkerRuntime {
|
||||
async fn start(&self, _spec: WorkerLaunchSpec) -> anyhow::Result<StartedWorker> {
|
||||
self.starts.fetch_add(1, Ordering::Relaxed);
|
||||
match self.outcome {
|
||||
PreStartWorkerOutcome::LaunchFailure => {
|
||||
anyhow::bail!("test worker launch failed")
|
||||
}
|
||||
PreStartWorkerOutcome::EarlyExit => Ok(StartedWorker {
|
||||
worker_ref: test_worker_ref(u32::MAX),
|
||||
stderr: Box::pin(tokio::io::empty()),
|
||||
wait: Box::pin(async {
|
||||
Ok(WorkerExit {
|
||||
success: false,
|
||||
detail: "test worker exited before starting".to_string(),
|
||||
})
|
||||
}),
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
async fn request_stop(&self, _worker_ref: &WorkerRef) {}
|
||||
|
||||
async fn force_stop(&self, _worker_ref: &WorkerRef) {}
|
||||
|
||||
async fn is_alive(&self, _worker_ref: &WorkerRef) -> bool {
|
||||
false
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl WorkerRuntime for RecordingWorkerRuntime {
|
||||
async fn start(&self, _spec: WorkerLaunchSpec) -> anyhow::Result<StartedWorker> {
|
||||
|
|
@ -4436,6 +4490,154 @@ async fn create_and_start_run(app: &Router, dot_source: &str) -> String {
|
|||
run_id
|
||||
}
|
||||
|
||||
fn subprocess_pre_start_failure_state(runtime: Arc<PreStartWorkerRuntime>) -> Arc<AppState> {
|
||||
let state = TestAppStateBuilder::new()
|
||||
.vault_entries([(EnvVars::OPENAI_API_KEY, "test-openai-api-key")])
|
||||
.worker_runtime(runtime)
|
||||
.build();
|
||||
let runtime_directory = Storage::new(state.server_storage_dir()).runtime_directory();
|
||||
ServerDaemon::new(
|
||||
std::process::id(),
|
||||
Bind::Tcp("127.0.0.1:32276".parse().expect("test bind should parse")),
|
||||
runtime_directory.log_path(),
|
||||
)
|
||||
.write(&runtime_directory)
|
||||
.expect("test server record should be written");
|
||||
state
|
||||
}
|
||||
|
||||
async fn assert_subprocess_pre_start_failure(
|
||||
outcome: PreStartWorkerOutcome,
|
||||
expected_reason: FailureReason,
|
||||
) {
|
||||
let runtime = Arc::new(PreStartWorkerRuntime::new(outcome));
|
||||
let state = subprocess_pre_start_failure_state(Arc::clone(&runtime));
|
||||
let app = crate::test_support::build_test_router(Arc::clone(&state));
|
||||
let run_id = create_and_start_run(&app, MINIMAL_DOT)
|
||||
.await
|
||||
.parse::<RunId>()
|
||||
.expect("created run id should parse");
|
||||
let run_store = state
|
||||
.stores
|
||||
.runs
|
||||
.open_run_reader(&run_id)
|
||||
.await
|
||||
.expect("created run should remain readable");
|
||||
|
||||
assert_eq!(
|
||||
run_store
|
||||
.state()
|
||||
.await
|
||||
.expect("runnable run state should load")
|
||||
.status,
|
||||
RunStatus::Runnable
|
||||
);
|
||||
|
||||
execute_run(Arc::clone(&state), run_id).await;
|
||||
|
||||
assert_eq!(runtime.start_count(), 1);
|
||||
let events = run_store
|
||||
.list_events()
|
||||
.await
|
||||
.expect("failed run events should remain readable");
|
||||
let lifecycle_events = events
|
||||
.iter()
|
||||
.map(|envelope| envelope.event.event_name())
|
||||
.filter(|name| matches!(*name, "run.runnable" | "run.starting" | "run.failed"))
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(lifecycle_events, vec!["run.runnable", "run.failed"]);
|
||||
let failure_reasons = events
|
||||
.iter()
|
||||
.filter_map(|envelope| match &envelope.event.body {
|
||||
fabro_types::run_event::EventBody::RunFailed(props) => Some(props.failure.reason),
|
||||
_ => None,
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(failure_reasons, vec![expected_reason]);
|
||||
|
||||
let expected_status = RunStatus::Failed {
|
||||
reason: expected_reason,
|
||||
};
|
||||
assert_eq!(
|
||||
run_store
|
||||
.state()
|
||||
.await
|
||||
.expect("failed run state should load")
|
||||
.status,
|
||||
expected_status
|
||||
);
|
||||
assert_eq!(
|
||||
state
|
||||
.runs
|
||||
.lock()
|
||||
.expect("runs lock poisoned")
|
||||
.get(&run_id)
|
||||
.expect("managed run should remain present")
|
||||
.status,
|
||||
expected_status
|
||||
);
|
||||
|
||||
let response = app
|
||||
.clone()
|
||||
.oneshot(
|
||||
Request::builder()
|
||||
.method("GET")
|
||||
.uri(api(&format!("/runs/{run_id}")))
|
||||
.body(Body::empty())
|
||||
.expect("run request should build"),
|
||||
)
|
||||
.await
|
||||
.expect("run request should complete");
|
||||
let body = response_json!(response, StatusCode::OK).await;
|
||||
assert_eq!(run_json_status(&body)["kind"], "failed");
|
||||
assert_eq!(
|
||||
run_json_status(&body)["reason"],
|
||||
expected_reason.to_string()
|
||||
);
|
||||
|
||||
let response = app
|
||||
.oneshot(
|
||||
Request::builder()
|
||||
.method("GET")
|
||||
.uri(api("/runs"))
|
||||
.body(Body::empty())
|
||||
.expect("run list request should build"),
|
||||
)
|
||||
.await
|
||||
.expect("run list request should complete");
|
||||
let body = response_json!(response, StatusCode::OK).await;
|
||||
let run_id_string = run_id.to_string();
|
||||
let listed = body["data"]
|
||||
.as_array()
|
||||
.expect("run list data should be an array")
|
||||
.iter()
|
||||
.find(|run| run_json_id(run) == Some(run_id_string.as_str()))
|
||||
.expect("failed run should remain listed");
|
||||
assert_eq!(run_json_status(listed)["kind"], "failed");
|
||||
assert_eq!(
|
||||
run_json_status(listed)["reason"],
|
||||
expected_reason.to_string()
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn subprocess_pre_start_failure_persists_launch_failure_from_runnable() {
|
||||
assert_subprocess_pre_start_failure(
|
||||
PreStartWorkerOutcome::LaunchFailure,
|
||||
FailureReason::LaunchFailed,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn subprocess_pre_start_failure_persists_early_worker_exit_from_runnable() {
|
||||
assert_subprocess_pre_start_failure(
|
||||
PreStartWorkerOutcome::EarlyExit,
|
||||
FailureReason::Terminated,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
async fn create_durable_run_with_events(
|
||||
state: &Arc<AppState>,
|
||||
run_id: RunId,
|
||||
|
|
|
|||
|
|
@ -740,6 +740,29 @@ mod tests {
|
|||
)
|
||||
}
|
||||
|
||||
fn sandbox_init_failure_payload(label: &str) -> EventPayload {
|
||||
event_payload(
|
||||
label,
|
||||
"2026-03-27T12:00:04Z",
|
||||
"run.failed",
|
||||
&serde_json::json!({
|
||||
"failure": {
|
||||
"reason": "sandbox_init_failed",
|
||||
"detail": {
|
||||
"message": "sandbox initialization failed",
|
||||
"category": "deterministic"
|
||||
}
|
||||
},
|
||||
"timing": {
|
||||
"wall_time_ms": 1,
|
||||
"inference_time_ms": 0,
|
||||
"tool_time_ms": 0,
|
||||
"active_time_ms": 0
|
||||
},
|
||||
}),
|
||||
)
|
||||
}
|
||||
|
||||
async fn append_completed(run: &RunDatabase, label: &str, created_at: DateTime<Utc>) {
|
||||
append_running(run, label, created_at).await;
|
||||
run.append_event(&event_payload(
|
||||
|
|
@ -914,7 +937,7 @@ mod tests {
|
|||
let events_before = run.list_events().await.unwrap();
|
||||
|
||||
let err = run
|
||||
.append_event(&workflow_failure_payload("run-1"))
|
||||
.append_event(&sandbox_init_failure_payload("run-1"))
|
||||
.await
|
||||
.unwrap_err();
|
||||
|
||||
|
|
@ -926,7 +949,7 @@ mod tests {
|
|||
Error::InvalidTransition(fabro_types::InvalidTransition {
|
||||
from: RunStatus::Runnable,
|
||||
to: RunStatus::Failed {
|
||||
reason: FailureReason::WorkflowError,
|
||||
reason: FailureReason::SandboxInitFailed,
|
||||
},
|
||||
})
|
||||
));
|
||||
|
|
@ -947,7 +970,7 @@ mod tests {
|
|||
append_runnable(&run, "run-1", dt("2026-03-27T12:00:00Z")).await;
|
||||
|
||||
let err = run
|
||||
.append_event(&workflow_failure_payload("run-1"))
|
||||
.append_event(&sandbox_init_failure_payload("run-1"))
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(matches!(err, Error::EventRejected { .. }));
|
||||
|
|
|
|||
|
|
@ -157,7 +157,18 @@ impl RunStatus {
|
|||
Self::Submitted
|
||||
)
|
||||
| (Self::Pending { .. }, Self::Runnable)
|
||||
| (Self::Runnable, Self::Starting)
|
||||
// A worker can fail before it appends RunStarting, while the durable run is still
|
||||
// Runnable. Keep this set aligned with the audited pre-Starting failure callers.
|
||||
| (
|
||||
Self::Runnable,
|
||||
Self::Starting
|
||||
| Self::Failed {
|
||||
reason: FailureReason::LaunchFailed
|
||||
| FailureReason::WorkflowError
|
||||
| FailureReason::Terminated
|
||||
| FailureReason::BootstrapFailed,
|
||||
}
|
||||
)
|
||||
| (
|
||||
Self::Submitted | Self::Pending { .. } | Self::Runnable,
|
||||
Self::Failed {
|
||||
|
|
@ -417,7 +428,7 @@ mod tests {
|
|||
assert!(runnable.can_transition_to(RunStatus::Failed {
|
||||
reason: FailureReason::Cancelled,
|
||||
}));
|
||||
assert!(!runnable.can_transition_to(RunStatus::Failed {
|
||||
assert!(runnable.can_transition_to(RunStatus::Failed {
|
||||
reason: FailureReason::Terminated,
|
||||
}));
|
||||
assert!(running.can_transition_to(blocked));
|
||||
|
|
@ -428,6 +439,51 @@ mod tests {
|
|||
}));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn runnable_pre_start_failures_are_audited() {
|
||||
let pre_start_failures = [
|
||||
FailureReason::LaunchFailed,
|
||||
FailureReason::WorkflowError,
|
||||
FailureReason::Terminated,
|
||||
FailureReason::BootstrapFailed,
|
||||
];
|
||||
|
||||
for reason in pre_start_failures {
|
||||
let failed = RunStatus::Failed { reason };
|
||||
assert!(
|
||||
RunStatus::Runnable.can_transition_to(failed),
|
||||
"Runnable should accept {reason} before Starting"
|
||||
);
|
||||
assert!(
|
||||
!RunStatus::Submitted.can_transition_to(failed),
|
||||
"Submitted should reject {reason}"
|
||||
);
|
||||
assert!(
|
||||
!RunStatus::Pending {
|
||||
reason: PendingReason::ApprovalRequired,
|
||||
}
|
||||
.can_transition_to(failed),
|
||||
"Pending should reject {reason}"
|
||||
);
|
||||
}
|
||||
|
||||
assert!(RunStatus::Runnable.can_transition_to(RunStatus::Failed {
|
||||
reason: FailureReason::Cancelled,
|
||||
}));
|
||||
for reason in [
|
||||
FailureReason::SandboxInitFailed,
|
||||
FailureReason::PublishFailed,
|
||||
FailureReason::TransientInfra,
|
||||
FailureReason::BudgetExhausted,
|
||||
FailureReason::ApprovalDenied,
|
||||
] {
|
||||
assert!(
|
||||
!RunStatus::Runnable.can_transition_to(RunStatus::Failed { reason }),
|
||||
"Runnable should reject unrelated failure {reason}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn success_and_failure_reasons_parse_and_round_trip() {
|
||||
let success = SuccessReason::from_str("completed").expect("completed should parse");
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue