diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 78a794aa30..d1ad940ab0 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -71,6 +71,8 @@ use std::process::Command; use std::time::{Duration, Instant}; use tonic::{Code, Status}; +const PROVISIONAL_CONTAINER_EXIT_RECONCILIATION_TIMEOUT: Duration = Duration::from_secs(5); + // Re-export SSH functions for backward compatibility pub use crate::ssh::{Editor, print_ssh_config}; pub use crate::ssh::{ @@ -253,6 +255,19 @@ fn has_main_process_result(sandbox: &Sandbox) -> bool { }) } +fn is_provisional_container_exit(sandbox: &Sandbox) -> bool { + let phase = SandboxPhase::try_from(sandbox.phase()).unwrap_or(SandboxPhase::Unknown); + phase == SandboxPhase::Error + && sandbox.status.as_ref().is_some_and(|status| { + status.exit_code.is_none() + && status.conditions.iter().any(|condition| { + condition.r#type == "Ready" + && condition.status.eq_ignore_ascii_case("false") + && condition.reason == "ContainerExited" + }) + }) +} + fn build_sandbox_resource_limits( cpu: Option<&str>, memory: Option<&str>, @@ -763,6 +778,12 @@ pub async fn sandbox_create( .unwrap_or(300), ); let mut provisioning_idle_deadline = Instant::now() + provision_timeout; + // The compute driver can publish ContainerExited while the supervisor's + // authoritative canonical-process result is waiting for the same gateway + // state lock. Keep watching briefly so the provisional error cannot race + // ephemeral cleanup, but retain a deadline for containers that exit before + // the supervisor can report a result. + let mut provisional_container_exit_deadline: Option = None; // Track whether we saw the gateway become ready (from log messages). let mut saw_gateway_ready = false; @@ -771,8 +792,15 @@ pub async fn sandbox_create( // longer than the default timeout pulling and preparing large images, // but only recognized progress events extend the idle deadline. Logs // and generic status churn must not keep a stuck sandbox alive forever. - let remaining = provisioning_idle_deadline.saturating_duration_since(Instant::now()); + let now = Instant::now(); + let mut remaining = provisioning_idle_deadline.saturating_duration_since(now); + if let Some(deadline) = provisional_container_exit_deadline { + remaining = remaining.min(deadline.saturating_duration_since(now)); + } if remaining.is_zero() { + if provisional_container_exit_deadline.is_some() { + break; + } let timeout_message = provisioning_timeout_message( provision_timeout.as_secs(), resource_requirements.as_ref(), @@ -792,6 +820,7 @@ pub async fn sandbox_create( let item = match maybe_item { Ok(Some(item)) => item, Ok(None) => break, // stream ended + Err(_elapsed) if provisional_container_exit_deadline.is_some() => break, Err(_elapsed) => { // Timeout fired — the stream was idle for too long. let timeout_message = provisioning_timeout_message( @@ -848,6 +877,12 @@ pub async fn sandbox_create( format!("{}: {}", condition.reason, condition.message); } } + if is_provisional_container_exit(&s) { + provisional_container_exit_deadline.get_or_insert_with(|| { + Instant::now() + PROVISIONAL_CONTAINER_EXIT_RECONCILIATION_TIMEOUT + }); + continue; + } break; } diff --git a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs index 7be771c442..246b7769de 100644 --- a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs +++ b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs @@ -39,7 +39,7 @@ use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::time::{Duration, Instant}; use tempfile::TempDir; use tokio::net::TcpListener; -use tokio::sync::{Mutex, mpsc}; +use tokio::sync::{Mutex, Notify, mpsc}; use tokio_stream::wrappers::TcpListenerStream; use tonic::transport::{Certificate as TlsCertificate, Identity, Server, ServerTlsConfig}; use tonic::{Response, Status}; @@ -54,6 +54,9 @@ struct SandboxState { vm_slow_progress_before_ready: Arc, vm_log_churn_before_ready: Arc, terminal_before_relay: Arc, + terminal_after_provisional_container_exit: Arc, + provisional_container_exit_without_result: Arc, + provisional_container_exit_sent: Arc, ssh_session_failures_remaining: Arc, ssh_session_requests: Arc, global_settings: Arc>>, @@ -561,6 +564,16 @@ impl OpenShell for TestOpenShell { .load(Ordering::SeqCst); let vm_log_churn_before_ready = self.state.vm_log_churn_before_ready.load(Ordering::SeqCst); let terminal_before_relay = self.state.terminal_before_relay.load(Ordering::SeqCst); + let terminal_after_provisional_container_exit = self + .state + .terminal_after_provisional_container_exit + .load(Ordering::SeqCst); + let provisional_container_exit_without_result = self + .state + .provisional_container_exit_without_result + .load(Ordering::SeqCst); + let provisional_container_exit_sent = + Arc::clone(&self.state.provisional_container_exit_sent); tokio::spawn(async move { let mut provisioning = Sandbox { @@ -604,11 +617,44 @@ impl OpenShell for TestOpenShell { }); completed.set_phase(SandboxPhase::Completed as i32); + let mut provisional_container_exit = error.clone(); + if let Some(ready) = provisional_container_exit + .status + .as_mut() + .and_then(|status| status.conditions.first_mut()) + { + ready.reason = "ContainerExited".to_string(); + ready.message = "Sandbox container exited".to_string(); + } + let _ = tx .send(Ok(SandboxStreamEvent { payload: Some(sandbox_stream_event::Payload::Sandbox(provisioning)), })) .await; + if terminal_after_provisional_container_exit + || provisional_container_exit_without_result + { + let _ = tx + .send(Ok(SandboxStreamEvent { + payload: Some(sandbox_stream_event::Payload::Sandbox( + provisional_container_exit, + )), + })) + .await; + provisional_container_exit_sent.notify_waiters(); + if provisional_container_exit_without_result { + std::future::pending::<()>().await; + return; + } + tokio::task::yield_now().await; + let _ = tx + .send(Ok(SandboxStreamEvent { + payload: Some(sandbox_stream_event::Payload::Sandbox(completed)), + })) + .await; + return; + } if vm_error_after_started { let _ = tx .send(Ok(SandboxStreamEvent { @@ -2184,6 +2230,109 @@ async fn sandbox_create_retries_terminal_attachment_until_relay_registers() { ); } +#[tokio::test] +async fn sandbox_create_waits_for_main_result_after_provisional_container_exit() { + let server = run_server().await; + server + .openshell + .state + .terminal_after_provisional_container_exit + .store(true, Ordering::SeqCst); + let fake_ssh_dir = tempfile::tempdir().unwrap(); + let xdg_dir = tempfile::tempdir().unwrap(); + let _env = test_env(&fake_ssh_dir, &xdg_dir); + let tls = test_tls(&server); + install_fake_ssh(&fake_ssh_dir); + + let exit_code = run::sandbox_create( + &server.endpoint, + "openshell", + run::SandboxCreateConfig { + name: Some("fast-ephemeral-command"), + keep: false, + command: &["echo".into(), "OK".into()], + ..test_config() + }, + "default", + &tls, + ) + .await + .expect("a provisional container exit must yield to the canonical main-process result"); + + assert_eq!(exit_code, 0); + assert_eq!( + deleted_names(&server).await, + vec![vec!["fast-ephemeral-command".to_string()]] + ); +} + +#[tokio::test] +async fn sandbox_create_bounds_provisional_container_exit_reconciliation() { + let server = run_server().await; + server + .openshell + .state + .provisional_container_exit_without_result + .store(true, Ordering::SeqCst); + let fake_ssh_dir = tempfile::tempdir().unwrap(); + let xdg_dir = tempfile::tempdir().unwrap(); + let _env = test_env(&fake_ssh_dir, &xdg_dir); + let tls = test_tls(&server); + install_fake_ssh(&fake_ssh_dir); + + let provisional_container_exit_sent = server + .openshell + .state + .provisional_container_exit_sent + .notified(); + tokio::pin!(provisional_container_exit_sent); + let command = ["echo".into(), "OK".into()]; + let create = run::sandbox_create( + &server.endpoint, + "openshell", + run::SandboxCreateConfig { + name: Some("missing-main-result"), + command: &command, + ..test_config() + }, + "default", + &tls, + ); + tokio::pin!(create); + + tokio::select! { + () = &mut provisional_container_exit_sent => {} + result = &mut create => panic!("sandbox create returned before the provisional exit was observed: {result:?}"), + } + let reconciliation_started = Instant::now(); + let result = tokio::time::timeout(Duration::from_secs(10), &mut create) + .await + .expect("provisional container exit reconciliation must remain bounded"); + let reconciliation_elapsed = reconciliation_started.elapsed(); + + let err = result + .expect_err("a missing canonical main-process result must retain the container exit error"); + + let rendered = err.to_string(); + assert!( + rendered.contains("sandbox entered error phase while provisioning"), + "unexpected error: {rendered}" + ); + assert!( + rendered.contains("ContainerExited: Sandbox container exited"), + "unexpected error: {rendered}" + ); + assert!( + !rendered.contains("timed out"), + "unexpected error: {rendered}" + ); + assert!( + reconciliation_elapsed >= Duration::from_secs(5), + "provisional container exit returned before reconciliation: {reconciliation_elapsed:?}" + ); + assert!(deleted_names(&server).await.is_empty()); +} + #[tokio::test] async fn sandbox_create_deletes_command_sessions_with_no_keep() { let server = run_server().await;