From 81ad1cae9ff61b031967ef9af8a36665c292f76c Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Mon, 6 Jul 2026 14:35:43 +0200 Subject: [PATCH 1/4] refactor(compute): extract create_sandbox_record and update_sandbox_record helpers Split apply_sandbox_update_locked into named helpers to make the two distinct paths explicit: create_sandbox_record for first-observation events and update_sandbox_record for subsequent driver snapshots on existing sandboxes. The dispatcher now uses a match on the existing record rather than an early-return guard. No behavior change. Signed-off-by: Evan Lezar --- crates/openshell-server/src/compute/mod.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index cf6b17c7e9..88b72273c1 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -1686,13 +1686,23 @@ impl ComputeRuntime { return Ok(()); } - // Single-attempt CAS: on conflict, the next watch event will naturally retry + self.update_sandbox_record(incoming, existing_record.resource_version) + .await + } + + // Subsequent driver snapshot for an existing sandbox: apply a single-attempt CAS update. + // On conflict the next watch event will naturally retry. + async fn update_sandbox_record( + &self, + incoming: DriverSandbox, + expected_resource_version: u64, + ) -> Result<(), String> { let session_connected = self.supervisor_sessions.has_session(&incoming.id); let sandbox = self .store .update_message_cas::( &incoming.id, - existing_record.resource_version, + expected_resource_version, |sandbox| apply_driver_snapshot(sandbox, &incoming, session_connected), ) .await From 21a061d6d8daf0d55eeedeacb0fb5674415ce169 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Mon, 6 Jul 2026 16:21:39 +0200 Subject: [PATCH 2/4] refactor(compute): make sandbox readiness gateway-owned across all drivers Introduce compute_phase_components and apply_readiness_conditions to centralise the gateway's phase composition logic. The public SandboxPhase is now determined by combining the backend phase reported by the driver with supervisor session presence, independent of the driver implementation. Remove SupervisorReadiness from the driver contract. Running containers always report BackendReady; the gateway owns the Ready decision. Rename the dispatcher match to three arms so that status-less events for existing sandboxes are a documented no-op rather than a silent pass-through. Drop backend_ready_no_session and the SupervisorNotConnected condition. The BackendReady driver condition plus the Provisioning phase already communicates that the backend is up but the supervisor has not connected. The redundant condition added noise without new information. Closes #1951 Signed-off-by: Evan Lezar --- architecture/compute-runtimes.md | 52 +++ crates/openshell-driver-docker/src/lib.rs | 64 +-- crates/openshell-driver-docker/src/tests.rs | 42 +- crates/openshell-server/src/compute/mod.rs | 385 ++++++++++++++++-- .../src/supervisor_session.rs | 6 - 5 files changed, 431 insertions(+), 118 deletions(-) diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index f211f906d6..2c438a5ff0 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -16,12 +16,64 @@ Each runtime receives a sandbox spec from the gateway and is responsible for: - Reporting lifecycle and platform events back to the gateway. - Cleaning up runtime-owned resources. +Drivers report **backend state only**. A driver snapshot with `Ready=True` means +the underlying compute resource (container, pod, VM) is healthy and running — +nothing more. Drivers must not gate on supervisor session state or hold +references to gateway-internal types. The gateway owns the public +`SandboxPhase::Ready` decision. This applies equally to extension drivers +implementing `ComputeDriver` out of tree. + Drivers own runtime-specific platform event interpretation. When an event should drive client provisioning UI, the driver attaches the shared `openshell.progress.*` metadata defined in `openshell-core` instead of requiring clients to parse Kubernetes reasons, VM cache states, or other driver-local reason strings. +## Sandbox Readiness Composition + +The gateway composes driver backend state with supervisor session presence to +produce the public `SandboxPhase`. This composition is gateway-owned and applied +uniformly across all drivers: + +``` +backend_phase = derive_phase(driver_status) + +public_phase = + if backend_phase in {Error, Deleting}: → pass through (terminal precedence) + if backend_phase == Ready && session connected: → Ready + if backend_phase == Ready && no session: → Provisioning + if backend_phase in {Provisioning, Unknown} && session: → Ready + if backend_phase in {Provisioning, Unknown} && no session: → Provisioning +``` + +When `public_phase == Ready` the sandbox is usable through the gateway — both the +backend resource is healthy and a supervisor session is registered. A sandbox whose +backend reports ready but has no supervisor session yet holds `Provisioning`; the +driver's `BackendReady=True` condition is visible in the sandbox status for operators +who need to distinguish that state from a sandbox still provisioning its compute resource. + +**Session precedence over lagging driver snapshots:** A supervisor session can only be +established by a running workload. When `set_supervisor_session_state` promotes the +store record to `Ready` on session connect, a driver watch event may still arrive +shortly after carrying a stale `Provisioning` or `Unknown` backend phase. The +composition rule treats a connected session as the stronger signal and keeps `Ready` +in that case, preventing a lagging snapshot from undoing the session-driven promotion. + +**Known HA limitation:** Supervisor sessions are process-local while the public +sandbox phase is shared. A replica that reconciles a driver snapshot without owning +the active supervisor session can demote the shared phase to `Provisioning`. The +session-owning replica may not receive another connection event to restore `Ready`, +so a usable sandbox can remain unavailable through the public phase gate. Reliable +HA readiness requires persisted or leased supervisor presence plus routing to the +session-owning replica. That work is deferred to GitHub issue #1868. Until then, +deployments that require reliable readiness composition must run a single gateway +replica. + +**Extension point:** The readiness decision is a safety invariant, not an +operator-configurable hook. The driver contract is the correct extension point for +custom backend readiness semantics. RFC-0010 lifecycle hooks may observe readiness +transitions via `post_commit`; they do not override the composition rule. + The capability RPC reports driver identity, version, and the default sandbox image used by the gateway. GPU availability stays driver-local and is validated when a sandbox create request asks for GPU resources. diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index 2f89c2229e..28b2588756 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -79,17 +79,6 @@ const HOST_OPENSHELL_INTERNAL: &str = "host.openshell.internal"; const HOST_DOCKER_INTERNAL: &str = "host.docker.internal"; const DOCKER_NETWORK_DRIVER: &str = "bridge"; -/// Queried by the Docker driver to decide when a sandbox's supervisor -/// relay is live. Implementations return `true` once a sandbox has an -/// active `ConnectSupervisor` session registered. -/// -/// The driver cannot observe the supervisor's SSH socket directly (it -/// lives inside the container), so it leans on this signal to flip the -/// Ready condition from `DependenciesNotReady` to `True`. -pub trait SupervisorReadiness: Send + Sync + 'static { - fn is_supervisor_connected(&self, sandbox_id: &str) -> bool; -} - /// Gateway-local configuration for the Docker compute driver. #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] #[serde(default, deny_unknown_fields)] @@ -212,7 +201,6 @@ pub struct DockerComputeDriver { config: DockerDriverRuntimeConfig, events: broadcast::Sender, pending: Arc>>, - supervisor_readiness: Arc, gpu_selector: Arc, } @@ -309,11 +297,7 @@ type WatchStream = Pin> + Send + 'static>>; impl DockerComputeDriver { - pub async fn new( - config: &Config, - docker_config: &DockerComputeConfig, - supervisor_readiness: Arc, - ) -> CoreResult { + pub async fn new(config: &Config, docker_config: &DockerComputeConfig) -> CoreResult { let socket_path = docker_config .socket_path .clone() @@ -395,7 +379,6 @@ impl DockerComputeDriver { }, events: broadcast::channel(WATCH_BUFFER).0, pending: Arc::new(Mutex::new(HashMap::new())), - supervisor_readiness, gpu_selector: Arc::new(CdiGpuDefaultSelector::new( cdi_gpu_inventory, allow_all_default_gpu, @@ -606,9 +589,9 @@ impl DockerComputeDriver { let container = self .find_managed_container_summary(sandbox_id, sandbox_name) .await?; - if let Some(sandbox) = container.and_then(|summary| { - sandbox_from_container_summary(&summary, self.supervisor_readiness.as_ref()) - }) { + if let Some(sandbox) = + container.and_then(|summary| sandbox_from_container_summary(&summary)) + { return Ok(Some(sandbox)); } @@ -619,9 +602,7 @@ impl DockerComputeDriver { let containers = self.list_managed_container_summaries().await?; let container_sandboxes = containers .iter() - .filter_map(|summary| { - sandbox_from_container_summary(summary, self.supervisor_readiness.as_ref()) - }) + .filter_map(sandbox_from_container_summary) .collect::>(); let mut by_id = self.pending_snapshot_map().await; for sandbox in container_sandboxes { @@ -1123,8 +1104,7 @@ impl DockerComputeDriver { if let Some(summary) = self .find_managed_container_summary(sandbox_id, sandbox_name) .await? - && let Some(sandbox) = - sandbox_from_container_summary(&summary, self.supervisor_readiness.as_ref()) + && let Some(sandbox) = sandbox_from_container_summary(&summary) { self.publish_sandbox_snapshot(sandbox); } @@ -2738,10 +2718,7 @@ fn parse_memory_limit(value: &str) -> Result, Status> { Ok(Some((amount * multiplier).round() as i64)) } -fn sandbox_from_container_summary( - summary: &ContainerSummary, - readiness: &dyn SupervisorReadiness, -) -> Option { +fn sandbox_from_container_summary(summary: &ContainerSummary) -> Option { let labels = summary.labels.as_ref()?; let id = labels.get(LABEL_SANDBOX_ID)?.clone(); let name = labels.get(LABEL_SANDBOX_NAME)?.clone(); @@ -2754,17 +2731,12 @@ fn sandbox_from_container_summary( .cloned() .unwrap_or_default(); - let supervisor_connected = readiness.is_supervisor_connected(&id); Some(DriverSandbox { id, name: name.clone(), namespace, spec: None, - status: Some(driver_status_from_summary( - summary, - &name, - supervisor_connected, - )), + status: Some(driver_status_from_summary(summary, &name)), workspace, }) } @@ -2772,10 +2744,9 @@ fn sandbox_from_container_summary( fn driver_status_from_summary( summary: &ContainerSummary, sandbox_name: &str, - supervisor_connected: bool, ) -> DriverSandboxStatus { let state = summary.state.unwrap_or(ContainerSummaryStateEnum::EMPTY); - let (ready, reason, message, deleting) = container_ready_condition(state, supervisor_connected); + let (ready, reason, message, deleting) = container_ready_condition(state); DriverSandboxStatus { sandbox_name: summary_container_name(summary).unwrap_or_else(|| sandbox_name.to_string()), @@ -2795,25 +2766,10 @@ fn driver_status_from_summary( fn container_ready_condition( state: ContainerSummaryStateEnum, - supervisor_connected: bool, ) -> (&'static str, &'static str, &'static str, bool) { match state { ContainerSummaryStateEnum::RUNNING => { - if supervisor_connected { - ( - "True", - "SupervisorConnected", - "Supervisor relay is live", - false, - ) - } else { - ( - "False", - "DependenciesNotReady", - "Container is running; waiting for supervisor relay", - false, - ) - } + ("True", "BackendReady", "Container is running", false) } ContainerSummaryStateEnum::CREATED => ("False", "Starting", "Container created", false), ContainerSummaryStateEnum::RESTARTING => ( diff --git a/crates/openshell-driver-docker/src/tests.rs b/crates/openshell-driver-docker/src/tests.rs index 67036b2192..7f3d049d60 100644 --- a/crates/openshell-driver-docker/src/tests.rs +++ b/crates/openshell-driver-docker/src/tests.rs @@ -141,14 +141,6 @@ fn inspected_volume(driver: &str, options: HashMap) -> bollard:: } } -struct DisconnectedSupervisorReadiness; - -impl SupervisorReadiness for DisconnectedSupervisorReadiness { - fn is_supervisor_connected(&self, _sandbox_id: &str) -> bool { - false - } -} - fn test_driver_with_config(config: DockerDriverRuntimeConfig) -> DockerComputeDriver { let allow_all_default_gpu = config.allow_all_default_gpu; DockerComputeDriver { @@ -159,7 +151,6 @@ fn test_driver_with_config(config: DockerDriverRuntimeConfig) -> DockerComputeDr config, events: broadcast::channel(WATCH_BUFFER).0, pending: Arc::new(tokio::sync::Mutex::new(HashMap::new())), - supervisor_readiness: Arc::new(DisconnectedSupervisorReadiness), gpu_selector: Arc::new(CdiGpuDefaultSelector::new( CdiGpuInventory::default(), allow_all_default_gpu, @@ -1728,34 +1719,19 @@ fn driver_status_keeps_running_sandboxes_provisioning_with_stable_message() { ..running.clone() }; - let running_status = driver_status_from_summary(&running, "demo", false); - let running_later_status = driver_status_from_summary(&running_later, "demo", false); - assert_eq!(running_status.conditions[0].status, "False"); - assert_eq!(running_status.conditions[0].reason, "DependenciesNotReady"); - assert_eq!( - running_status.conditions[0].message, - "Container is running; waiting for supervisor relay" - ); + // A running container always emits Ready=True with BackendReady. The gateway + // composes this with supervisor-session presence to decide public SandboxPhase. + let running_status = driver_status_from_summary(&running, "demo"); + let running_later_status = driver_status_from_summary(&running_later, "demo"); + assert_eq!(running_status.conditions[0].status, "True"); + assert_eq!(running_status.conditions[0].reason, "BackendReady"); + assert_eq!(running_status.conditions[0].message, "Container is running"); assert_eq!(running_status.conditions, running_later_status.conditions); - let exited_status = driver_status_from_summary(&exited, "demo", false); + let exited_status = driver_status_from_summary(&exited, "demo"); assert_eq!(exited_status.conditions[0].status, "False"); assert_eq!(exited_status.conditions[0].reason, "ContainerExited"); assert_eq!(exited_status.conditions[0].message, "Container exited"); - - // With a live supervisor session, a RUNNING container flips Ready=True - // so ExecSandbox and other "sandbox must be ready" gates can proceed. - let running_connected = driver_status_from_summary(&running, "demo", true); - assert_eq!(running_connected.conditions[0].status, "True"); - assert_eq!( - running_connected.conditions[0].reason, - "SupervisorConnected" - ); - - // Supervisor readiness is ignored for non-RUNNING states -- an exited - // container must not report Ready=True. - let exited_connected = driver_status_from_summary(&exited, "demo", true); - assert_eq!(exited_connected.conditions[0].status, "False"); } #[test] @@ -1773,7 +1749,7 @@ fn driver_status_marks_restarting_sandboxes_as_error() { ..Default::default() }; - let status = driver_status_from_summary(&restarting, "demo", false); + let status = driver_status_from_summary(&restarting, "demo"); assert_eq!(status.conditions[0].status, "False"); assert_eq!(status.conditions[0].reason, "ContainerRestarting"); assert_eq!( diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index 88b72273c1..493d3105a1 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -467,7 +467,7 @@ impl ComputeRuntime { supervisor_sessions: Arc, ) -> Result { let driver = Arc::new( - DockerComputeDriver::new(&config, &docker_config, supervisor_sessions.clone()) + DockerComputeDriver::new(&config, &docker_config) .await .map_err(|err| ComputeError::Message(err.to_string()))?, ); @@ -2446,27 +2446,36 @@ fn public_status_from_driver( fn apply_driver_snapshot(sandbox: &mut Sandbox, incoming: &DriverSandbox, session_connected: bool) { let old_phase = SandboxPhase::try_from(sandbox.phase()).unwrap_or(SandboxPhase::Unknown); - let mut phase = incoming - .status - .as_ref() - .map_or(old_phase, |status| derive_phase(Some(status))); let sandbox_name = &incoming.name; - let supervisor_promoted = - session_connected && matches!(phase, SandboxPhase::Provisioning | SandboxPhase::Unknown); - if supervisor_promoted { - phase = SandboxPhase::Ready; - } let cpv = sandbox.current_policy_version(); - let mut status = incoming - .status - .as_ref() - .map(|status| public_status_from_driver(status, phase, cpv)) - .or_else(|| sandbox.status.clone()); - rewrite_user_facing_conditions(&mut status, sandbox.spec.as_ref()); - if supervisor_promoted { - ensure_supervisor_ready_status(&mut status, sandbox_name); - } + let (phase, mut status) = incoming.status.as_ref().map_or_else( + || { + let mut phase = old_phase; + let supervisor_promoted = session_connected + && matches!(phase, SandboxPhase::Provisioning | SandboxPhase::Unknown); + if supervisor_promoted { + phase = SandboxPhase::Ready; + } + + let mut status = sandbox.status.clone(); + rewrite_user_facing_conditions(&mut status, sandbox.spec.as_ref()); + if supervisor_promoted { + ensure_supervisor_ready_status(&mut status, sandbox_name); + } + (phase, status) + }, + |incoming_status| { + let composed = ComposedPhase::new(incoming_status, session_connected); + let mut status = Some(public_status_from_driver( + incoming_status, + composed.phase, + cpv, + )); + composed.apply_readiness_conditions(&mut status, sandbox_name, sandbox.spec.as_ref()); + (composed.phase, status) + }, + ); if let Some(status) = status.as_mut() && status.sandbox_name.is_empty() @@ -2525,6 +2534,48 @@ fn ensure_supervisor_ready_status(status: &mut Option, sandbox_na ); } +/// Compose the public `SandboxPhase` from backend driver state and supervisor session presence. +/// +/// The readiness decision is a gateway-owned safety invariant: `SandboxPhase::Ready` means +/// "usable through this gateway." The driver contract is the extension point for custom backend +/// readiness semantics. RFC-0010 lifecycle hooks observe this decision via `post_commit`; they +/// do not modify it. +struct ComposedPhase { + phase: SandboxPhase, + session_connected: bool, +} + +impl ComposedPhase { + fn new(incoming_status: &DriverSandboxStatus, session_connected: bool) -> Self { + let backend_phase = derive_phase(Some(incoming_status)); + // A live supervisor session is a stronger readiness signal than the backend phase. + // set_supervisor_session_state may have already promoted the store record to Ready + // before this driver snapshot arrived. Keep Ready rather than letting a lagging + // backend phase overwrite it. + let phase = match backend_phase { + SandboxPhase::Error | SandboxPhase::Deleting => backend_phase, + _ if session_connected => SandboxPhase::Ready, + _ => SandboxPhase::Provisioning, + }; + Self { + phase, + session_connected, + } + } + + fn apply_readiness_conditions( + &self, + status: &mut Option, + sandbox_name: &str, + spec: Option<&SandboxSpec>, + ) { + rewrite_user_facing_conditions(status, spec); + if self.session_connected && self.phase == SandboxPhase::Ready { + ensure_supervisor_ready_status(status, sandbox_name); + } + } +} + fn ensure_supervisor_not_ready_status(status: &mut Option, sandbox_name: &str) { upsert_ready_condition( status, @@ -3466,8 +3517,8 @@ mod tests { conditions: vec![DriverCondition { r#type: "Ready".to_string(), status: "True".to_string(), - reason: "DependenciesReady".to_string(), - message: "Sandbox is ready".to_string(), + reason: "BackendReady".to_string(), + message: "Container is running".to_string(), last_transition_time: String::new(), }], deleting: false, @@ -3921,8 +3972,8 @@ mod tests { conditions: vec![DriverCondition { r#type: "Ready".to_string(), status: "True".to_string(), - reason: "DependenciesReady".to_string(), - message: "Pod is Ready".to_string(), + reason: "BackendReady".to_string(), + message: "Container is running".to_string(), last_transition_time: String::new(), }], deleting: false, @@ -4049,8 +4100,14 @@ mod tests { ); assert_eq!( SandboxPhase::try_from(stored.phase()).unwrap(), - SandboxPhase::Ready + SandboxPhase::Provisioning ); + let ready_condition = stored + .status + .as_ref() + .and_then(|s| s.conditions.iter().find(|c| c.r#type == "Ready")) + .unwrap(); + assert_eq!(ready_condition.reason, "BackendReady"); } #[tokio::test] @@ -4813,7 +4870,7 @@ mod tests { .unwrap(); assert_eq!( SandboxPhase::try_from(stored.phase()).unwrap(), - SandboxPhase::Ready + SandboxPhase::Provisioning ); assert_sandbox_owned_records(&runtime, &sandbox, &session, true).await; assert_eq!( @@ -5220,6 +5277,282 @@ mod tests { assert_eq!(ready.message, "Supervisor session disconnected"); } + // --- Composition rule tests --- + + fn make_ready_driver_status() -> DriverSandboxStatus { + DriverSandboxStatus { + sandbox_name: "test".to_string(), + instance_id: "test-pod".to_string(), + agent_fd: String::new(), + sandbox_fd: String::new(), + conditions: vec![DriverCondition { + r#type: "Ready".to_string(), + status: "True".to_string(), + reason: "BackendReady".to_string(), + message: "Container is running".to_string(), + last_transition_time: String::new(), + }], + deleting: false, + } + } + + fn make_deleting_driver_status() -> DriverSandboxStatus { + DriverSandboxStatus { + sandbox_name: "test".to_string(), + instance_id: "test-pod".to_string(), + agent_fd: String::new(), + sandbox_fd: String::new(), + conditions: vec![DriverCondition { + r#type: "Ready".to_string(), + status: "False".to_string(), + reason: "Deleting".to_string(), + message: "Container is being removed".to_string(), + last_transition_time: String::new(), + }], + deleting: true, + } + } + + fn ready_condition(sandbox: &Sandbox) -> Option<&SandboxCondition> { + sandbox + .status + .as_ref() + .and_then(|s| s.conditions.iter().find(|c| c.r#type == "Ready")) + } + + #[tokio::test] + async fn backend_ready_without_supervisor_stays_provisioning() { + let runtime = test_runtime(Arc::new(TestDriver::default())).await; + let sandbox = sandbox_record("sb-1", "sandbox-a", SandboxPhase::Provisioning); + runtime.store.put_message(&sandbox).await.unwrap(); + + runtime + .apply_sandbox_update(DriverSandbox { + id: "sb-1".to_string(), + name: "sandbox-a".to_string(), + namespace: "default".to_string(), + workspace: String::new(), + spec: None, + status: Some(make_ready_driver_status()), + }) + .await + .unwrap(); + + let stored = runtime + .store + .get_message::("sb-1") + .await + .unwrap() + .unwrap(); + assert_eq!( + SandboxPhase::try_from(stored.phase()).unwrap(), + SandboxPhase::Provisioning + ); + let cond = ready_condition(&stored).unwrap(); + assert_eq!(cond.status, "True"); + assert_eq!(cond.reason, "BackendReady"); + } + + #[tokio::test] + async fn backend_ready_with_supervisor_becomes_ready() { + let runtime = test_runtime(Arc::new(TestDriver::default())).await; + let sandbox = sandbox_record("sb-1", "sandbox-a", SandboxPhase::Provisioning); + runtime.store.put_message(&sandbox).await.unwrap(); + register_test_supervisor_session(&runtime, "sb-1"); + + runtime + .apply_sandbox_update(DriverSandbox { + id: "sb-1".to_string(), + name: "sandbox-a".to_string(), + namespace: "default".to_string(), + workspace: String::new(), + spec: None, + status: Some(make_ready_driver_status()), + }) + .await + .unwrap(); + + let stored = runtime + .store + .get_message::("sb-1") + .await + .unwrap() + .unwrap(); + assert_eq!( + SandboxPhase::try_from(stored.phase()).unwrap(), + SandboxPhase::Ready + ); + let cond = ready_condition(&stored).unwrap(); + assert_eq!(cond.status, "True"); + assert_eq!(cond.reason, "DependenciesReady"); + } + + #[tokio::test] + async fn backend_not_ready_with_supervisor_becomes_ready() { + // VM path: supervisor connects before backend reports Ready. + let runtime = test_runtime(Arc::new(TestDriver::default())).await; + let sandbox = sandbox_record("sb-1", "sandbox-a", SandboxPhase::Provisioning); + runtime.store.put_message(&sandbox).await.unwrap(); + register_test_supervisor_session(&runtime, "sb-1"); + + runtime + .apply_sandbox_update(DriverSandbox { + id: "sb-1".to_string(), + name: "sandbox-a".to_string(), + namespace: "default".to_string(), + workspace: String::new(), + spec: None, + status: Some(make_driver_status(make_driver_condition( + "Starting", + "VM is starting", + ))), + }) + .await + .unwrap(); + + let stored = runtime + .store + .get_message::("sb-1") + .await + .unwrap() + .unwrap(); + assert_eq!( + SandboxPhase::try_from(stored.phase()).unwrap(), + SandboxPhase::Ready + ); + } + + #[tokio::test] + async fn terminal_failure_ignores_supervisor_session() { + let runtime = test_runtime(Arc::new(TestDriver::default())).await; + let sandbox = sandbox_record("sb-1", "sandbox-a", SandboxPhase::Provisioning); + runtime.store.put_message(&sandbox).await.unwrap(); + register_test_supervisor_session(&runtime, "sb-1"); + + runtime + .apply_sandbox_update(DriverSandbox { + id: "sb-1".to_string(), + name: "sandbox-a".to_string(), + namespace: "default".to_string(), + workspace: "default".to_string(), + spec: None, + status: Some(make_driver_status(make_driver_condition( + "ImagePullBackOff", + "Failed to pull image", + ))), + }) + .await + .unwrap(); + + let stored = runtime + .store + .get_message::("sb-1") + .await + .unwrap() + .unwrap(); + assert_eq!( + SandboxPhase::try_from(stored.phase()).unwrap(), + SandboxPhase::Error + ); + } + + #[tokio::test] + async fn later_driver_ready_without_session_does_not_repromote() { + // Re-promotion bug fix: backend-ready snapshot after session disconnect must not + // re-promote the sandbox to Ready. + let runtime = test_runtime(Arc::new(TestDriver::default())).await; + let sandbox = sandbox_record("sb-1", "sandbox-a", SandboxPhase::Provisioning); + runtime.store.put_message(&sandbox).await.unwrap(); + + // Promote to Ready via supervisor session connect. + register_test_supervisor_session(&runtime, "sb-1"); + runtime.supervisor_session_connected("sb-1").await.unwrap(); + let stored = runtime + .store + .get_message::("sb-1") + .await + .unwrap() + .unwrap(); + assert_eq!( + SandboxPhase::try_from(stored.phase()).unwrap(), + SandboxPhase::Ready + ); + + // Session drops. + runtime.supervisor_sessions.cleanup_sandbox("sb-1"); + runtime + .supervisor_session_disconnected("sb-1") + .await + .unwrap(); + let stored = runtime + .store + .get_message::("sb-1") + .await + .unwrap() + .unwrap(); + assert_eq!( + SandboxPhase::try_from(stored.phase()).unwrap(), + SandboxPhase::Provisioning + ); + + // Backend-ready snapshot arrives with no active session — must not re-promote. + runtime + .apply_sandbox_update(DriverSandbox { + id: "sb-1".to_string(), + name: "sandbox-a".to_string(), + namespace: "default".to_string(), + workspace: "default".to_string(), + spec: None, + status: Some(make_ready_driver_status()), + }) + .await + .unwrap(); + + let stored = runtime + .store + .get_message::("sb-1") + .await + .unwrap() + .unwrap(); + assert_eq!( + SandboxPhase::try_from(stored.phase()).unwrap(), + SandboxPhase::Provisioning + ); + let cond = ready_condition(&stored).unwrap(); + assert_eq!(cond.reason, "BackendReady"); + } + + #[tokio::test] + async fn deleting_ignores_supervisor_session() { + let runtime = test_runtime(Arc::new(TestDriver::default())).await; + let sandbox = sandbox_record("sb-1", "sandbox-a", SandboxPhase::Provisioning); + runtime.store.put_message(&sandbox).await.unwrap(); + register_test_supervisor_session(&runtime, "sb-1"); + + runtime + .apply_sandbox_update(DriverSandbox { + id: "sb-1".to_string(), + name: "sandbox-a".to_string(), + namespace: "default".to_string(), + workspace: "default".to_string(), + spec: None, + status: Some(make_deleting_driver_status()), + }) + .await + .unwrap(); + + let stored = runtime + .store + .get_message::("sb-1") + .await + .unwrap() + .unwrap(); + assert_eq!( + SandboxPhase::try_from(stored.phase()).unwrap(), + SandboxPhase::Deleting + ); + } + #[tokio::test] async fn reconcile_store_with_backend_applies_driver_snapshot() { let runtime = test_runtime(Arc::new(TestDriver { @@ -5279,6 +5612,7 @@ mod tests { }; runtime.store.put_message(&sandbox).await.unwrap(); runtime.sandbox_index.update_from_sandbox(&sandbox); + register_test_supervisor_session(&runtime, "sb-1"); runtime .reconcile_store_with_backend(Duration::ZERO) @@ -5377,6 +5711,7 @@ mod tests { let sandbox = sandbox_record("sb-1", "sandbox-a", SandboxPhase::Provisioning); runtime.store.put_message(&sandbox).await.unwrap(); runtime.sandbox_index.update_from_sandbox(&sandbox); + register_test_supervisor_session(&runtime, "sb-1"); runtime .reconcile_store_with_backend(Duration::ZERO) diff --git a/crates/openshell-server/src/supervisor_session.rs b/crates/openshell-server/src/supervisor_session.rs index b3dbaa569a..94f783fa0d 100644 --- a/crates/openshell-server/src/supervisor_session.rs +++ b/crates/openshell-server/src/supervisor_session.rs @@ -64,12 +64,6 @@ struct LiveSession { /// target-open failure reported by the supervisor. type RelayStreamSender = oneshot::Sender>; -impl openshell_driver_docker::SupervisorReadiness for SupervisorSessionRegistry { - fn is_supervisor_connected(&self, sandbox_id: &str) -> bool { - Self::is_connected(self, sandbox_id) - } -} - /// Registry of active supervisor sessions and pending relay channels. #[derive(Default)] pub struct SupervisorSessionRegistry { From 0e2ffd1f5cecb1c61762b3027e246574d2fce292 Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Fri, 24 Jul 2026 18:09:57 -0700 Subject: [PATCH 3/4] fix(compute): expose disconnected supervisor readiness Signed-off-by: Drew Newberry --- architecture/compute-runtimes.md | 8 ++-- crates/openshell-server/src/compute/mod.rs | 40 ++++++++++++++++--- .../src/supervisor_session.rs | 8 ---- 3 files changed, 40 insertions(+), 16 deletions(-) diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index 2c438a5ff0..07188f0d06 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -48,9 +48,11 @@ public_phase = When `public_phase == Ready` the sandbox is usable through the gateway — both the backend resource is healthy and a supervisor session is registered. A sandbox whose -backend reports ready but has no supervisor session yet holds `Provisioning`; the -driver's `BackendReady=True` condition is visible in the sandbox status for operators -who need to distinguish that state from a sandbox still provisioning its compute resource. +backend reports ready but has no supervisor session yet holds `Provisioning` with a +`Ready=False`, `SupervisorNotConnected` condition and the message +`Backend ready; waiting for supervisor session`. This distinguishes it from a sandbox +whose compute resource is still provisioning without exposing contradictory public +readiness signals. **Session precedence over lagging driver snapshots:** A supervisor session can only be established by a running workload. When `set_supervisor_session_state` promotes the diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index 493d3105a1..7c66fdacd2 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -2543,6 +2543,7 @@ fn ensure_supervisor_ready_status(status: &mut Option, sandbox_na struct ComposedPhase { phase: SandboxPhase, session_connected: bool, + backend_ready_without_session: bool, } impl ComposedPhase { @@ -2560,6 +2561,8 @@ impl ComposedPhase { Self { phase, session_connected, + backend_ready_without_session: backend_phase == SandboxPhase::Ready + && !session_connected, } } @@ -2570,12 +2573,28 @@ impl ComposedPhase { spec: Option<&SandboxSpec>, ) { rewrite_user_facing_conditions(status, spec); - if self.session_connected && self.phase == SandboxPhase::Ready { + if self.backend_ready_without_session { + ensure_supervisor_not_connected_status(status, sandbox_name); + } else if self.session_connected && self.phase == SandboxPhase::Ready { ensure_supervisor_ready_status(status, sandbox_name); } } } +fn ensure_supervisor_not_connected_status(status: &mut Option, sandbox_name: &str) { + upsert_ready_condition( + status, + sandbox_name, + SandboxCondition { + r#type: "Ready".to_string(), + status: "False".to_string(), + reason: "SupervisorNotConnected".to_string(), + message: "Backend ready; waiting for supervisor session".to_string(), + last_transition_time: String::new(), + }, + ); +} + fn ensure_supervisor_not_ready_status(status: &mut Option, sandbox_name: &str) { upsert_ready_condition( status, @@ -2700,6 +2719,7 @@ fn is_terminal_failure_reason(reason: &str) -> bool { let transient_reasons = [ "reconcilererror", "dependenciesnotready", + "supervisornotconnected", "starting", "containerstarting", "containercreated", @@ -3620,6 +3640,10 @@ mod tests { "Pod exists with phase: Pending; Service Exists", ), ("dependenciesnotready", "lowercase also works"), + ( + "SupervisorNotConnected", + "Backend ready; waiting for supervisor session", + ), ("Starting", "VM is starting"), ( "ContainerCreated", @@ -4107,7 +4131,8 @@ mod tests { .as_ref() .and_then(|s| s.conditions.iter().find(|c| c.r#type == "Ready")) .unwrap(); - assert_eq!(ready_condition.reason, "BackendReady"); + assert_eq!(ready_condition.status, "False"); + assert_eq!(ready_condition.reason, "SupervisorNotConnected"); } #[tokio::test] @@ -5349,8 +5374,12 @@ mod tests { SandboxPhase::Provisioning ); let cond = ready_condition(&stored).unwrap(); - assert_eq!(cond.status, "True"); - assert_eq!(cond.reason, "BackendReady"); + assert_eq!(cond.status, "False"); + assert_eq!(cond.reason, "SupervisorNotConnected"); + assert_eq!( + cond.message, + "Backend ready; waiting for supervisor session" + ); } #[tokio::test] @@ -5519,7 +5548,8 @@ mod tests { SandboxPhase::Provisioning ); let cond = ready_condition(&stored).unwrap(); - assert_eq!(cond.reason, "BackendReady"); + assert_eq!(cond.status, "False"); + assert_eq!(cond.reason, "SupervisorNotConnected"); } #[tokio::test] diff --git a/crates/openshell-server/src/supervisor_session.rs b/crates/openshell-server/src/supervisor_session.rs index 94f783fa0d..e6b8085151 100644 --- a/crates/openshell-server/src/supervisor_session.rs +++ b/crates/openshell-server/src/supervisor_session.rs @@ -136,14 +136,6 @@ impl SupervisorSessionRegistry { } } - /// Report whether a live supervisor session is registered for a sandbox. - /// - /// Used by compute drivers that need to surface "supervisor relay ready" - /// through the Ready condition without polling the sandbox runtime. - pub fn is_connected(&self, sandbox_id: &str) -> bool { - self.sessions.lock().unwrap().contains_key(sandbox_id) - } - /// Remove the session for a sandbox. fn remove(&self, sandbox_id: &str) { self.sessions.lock().unwrap().remove(sandbox_id); From 74396b84e087a449250e198bfd6e4cdf723927f4 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Tue, 28 Jul 2026 17:44:24 +0200 Subject: [PATCH 4/4] docs(sandboxes): clarify supervisor readiness lifecycle Signed-off-by: Evan Lezar --- docs/sandboxes/manage-sandboxes.mdx | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/docs/sandboxes/manage-sandboxes.mdx b/docs/sandboxes/manage-sandboxes.mdx index f3f36ecc9e..921efe0208 100644 --- a/docs/sandboxes/manage-sandboxes.mdx +++ b/docs/sandboxes/manage-sandboxes.mdx @@ -439,12 +439,19 @@ openshell sandbox delete my-sandbox Every sandbox moves through a defined set of phases: -| Phase | Description | -| ------------ | -------------------------------------------------------------------------------------------------------------------------------------- | -| Provisioning | The runtime is setting up the sandbox environment, injecting credentials, and applying your policy. | -| Ready | The sandbox is running. The agent process is active and all isolation layers are enforced. You can connect, sync files, and view logs. | -| Error | Something went wrong during provisioning or execution. Check logs with `openshell logs` for details. | -| Deleting | The sandbox is being torn down. The system releases resources and purges credentials. | +| Phase | Description | +| ------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Provisioning | The runtime is setting up the sandbox environment, or the gateway is waiting for the sandbox supervisor to establish its authenticated control session. | +| Ready | The sandbox is running and its supervisor control session is connected. You can connect, execute commands, sync files, and view logs. | +| Error | Something went wrong during provisioning or execution. Check logs with `openshell logs` for details. | +| Deleting | The sandbox is being torn down. The system releases resources and purges credentials. | + +The compute backend can become ready before the sandbox supervisor connects to +the gateway. During this interval, the sandbox remains in `Provisioning` and +reports a `Ready=False` condition with the reason `SupervisorNotConnected`. +After a gateway restart, an existing sandbox can return to `Provisioning` +temporarily while its supervisor reconnects. Wait for the phase to return to +`Ready` before you connect to the sandbox or execute commands. ## Sandbox Compute Drivers