From 2d02b78c1cb657d82dac0fcc78dfa656962e5a2b Mon Sep 17 00:00:00 2001 From: Piotr Mlocek Date: Mon, 26 Oct 2026 16:46:02 -0700 Subject: [PATCH 1/4] fix(server): allow exec on exited sandboxes with a live supervisor After the main process exits, the supervisor keeps its access plane, and SSH and port forwarding already reached Completed and Error sandboxes while it stayed connected. Exec still required Ready in both the CLI and the gateway, so it was refused on the same sandboxes. Exec, SSH sessions, and TCP forwarding now share one gate: a sandbox is reachable when it is Ready, or Completed/Error with a supervisor session on this replica or a fresh owner record naming a peer replica. The previous check only looked at the local session map, so on a multi-replica gateway a request landing on a non-owner replica was refused even though relay opening routes to the owner. The CLI exec pre-check and the service forward recheck accept Ready, Completed, and Error and leave the supervisor decision to the gateway. Signed-off-by: Piotr Mlocek --- crates/openshell-cli/src/run.rs | 25 ++-- .../sandbox_exec_streaming_integration.rs | 41 ++++++- .../grpc/mutation_replay/streaming/tests.rs | 41 +++++++ crates/openshell-server/src/grpc/sandbox.rs | 47 ++++---- .../src/supervisor_session.rs | 109 ++++++++++++++++++ 5 files changed, 233 insertions(+), 30 deletions(-) diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 8aca727b5c..cbafa8c9c3 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -2013,8 +2013,9 @@ pub async fn sandbox_exec_grpc( .sandbox .ok_or_else(|| miette::miette!("sandbox not found"))?; - // Verify the sandbox is ready before issuing the exec. - if SandboxPhase::try_from(sandbox.phase()) != Ok(SandboxPhase::Ready) { + // Reject phases where the relay can never be open. The gateway decides + // whether a Completed or Error sandbox still has a reachable supervisor. + if !relay_phase_may_be_reachable(sandbox.phase()) { return Err(miette::miette!( "sandbox '{}' is not ready (phase: {}); wait for it to reach Ready state", name, @@ -2168,7 +2169,7 @@ pub async fn service_forward_tcp( let (bind_addr, bind_port) = parse_tcp_forward_spec(local, target_port)?; let mut client = grpc_client(server, tls).await?; - let mut replica = fetch_ready_sandbox_for_forward(&mut client, name, workspace).await?; + let mut replica = fetch_reachable_sandbox_for_forward(&mut client, name, workspace).await?; let listener = tokio::net::TcpListener::bind((bind_addr.as_str(), bind_port)) .await @@ -2202,7 +2203,7 @@ pub async fn service_forward_tcp( } _ = health_check.tick() => { - replica = fetch_ready_sandbox_for_forward(&mut client, name, workspace).await?; + replica = fetch_reachable_sandbox_for_forward(&mut client, name, workspace).await?; } accepted = listener.accept() => { @@ -2268,8 +2269,18 @@ fn forward_requires_session_token(status: &Status) -> bool { && status.message().contains("authorization_token is required") } -/// Confirm the sandbox is still ready, returning the replica that owns it. -async fn fetch_ready_sandbox_for_forward( +/// Whether the gateway may still relay into a sandbox in this phase. A +/// Completed or Error sandbox keeps its access plane while its supervisor +/// stays connected; only the gateway can tell whether it still is. +fn relay_phase_may_be_reachable(phase: i32) -> bool { + matches!( + SandboxPhase::try_from(phase), + Ok(SandboxPhase::Ready | SandboxPhase::Completed | SandboxPhase::Error) + ) +} + +/// Confirm the sandbox is still reachable, returning the replica that owns it. +async fn fetch_reachable_sandbox_for_forward( client: &mut crate::tls::GrpcClient, name: &str, workspace: &str, @@ -2298,7 +2309,7 @@ async fn fetch_ready_sandbox_for_forward( .sandbox .ok_or_else(|| miette::miette!("sandbox '{name}' not found"))?; - if SandboxPhase::try_from(sandbox.phase()) != Ok(SandboxPhase::Ready) { + if !relay_phase_may_be_reachable(sandbox.phase()) { return Err(miette::miette!( "sandbox '{}' is no longer ready (phase: {}); stopping service forward", name, diff --git a/crates/openshell-cli/tests/sandbox_exec_streaming_integration.rs b/crates/openshell-cli/tests/sandbox_exec_streaming_integration.rs index 48b84a3373..b1a7446ed4 100644 --- a/crates/openshell-cli/tests/sandbox_exec_streaming_integration.rs +++ b/crates/openshell-cli/tests/sandbox_exec_streaming_integration.rs @@ -46,6 +46,7 @@ struct Calls { #[derive(Clone)] struct MockGateway { scenario: Scenario, + phase: proto::SandboxPhase, calls: Arc>, } @@ -189,7 +190,7 @@ macro_rules! mock_gateway { ..Default::default() }), status: Some(proto::SandboxStatus { - phase: proto::SandboxPhase::Ready.into(), + phase: self.phase.into(), ..Default::default() }), ..Default::default() @@ -324,6 +325,10 @@ impl Drop for TestGateway { impl TestGateway { async fn start(scenario: Scenario) -> Self { + Self::start_in_phase(scenario, proto::SandboxPhase::Ready).await + } + + async fn start_in_phase(scenario: Scenario, phase: proto::SandboxPhase) -> Self { let (ca, ca_key) = build_ca(); let (server_cert, server_key) = build_server_cert(&ca, &ca_key); let (client_cert, client_key) = build_client_cert(&ca, &ca_key); @@ -344,6 +349,7 @@ impl TestGateway { let calls = Arc::new(Mutex::new(Calls::default())); let service = MockGateway { scenario, + phase, calls: Arc::clone(&calls), }; let task = tokio::spawn(async move { @@ -476,6 +482,39 @@ async fn default_remote_exit_does_not_wait_for_idle_open_stdin() { drop(held_open); } +#[tokio::test] +async fn exec_reaches_gateway_for_completed_and_error_sandboxes() { + // The supervisor keeps its access plane after the main process exits, so + // the gateway, not the CLI, decides whether the relay is still reachable. + for phase in [proto::SandboxPhase::Completed, proto::SandboxPhase::Error] { + let gateway = TestGateway::start_in_phase(Scenario::EarlyExit, phase).await; + let mut child = gateway.spawn(); + let held_open = child.stdin.take().unwrap(); + let output = finish(child).await; + drop(held_open); + assert!( + output.status.success(), + "{phase:?}: {}", + String::from_utf8_lossy(&output.stderr) + ); + assert_eq!(output.stdout, b"before-exit\n"); + gateway.assert_one_stream(); + } +} + +#[tokio::test] +async fn exec_is_refused_before_starting_for_stopped_sandbox() { + let gateway = + TestGateway::start_in_phase(Scenario::EarlyExit, proto::SandboxPhase::Stopped).await; + let mut child = gateway.spawn(); + drop(child.stdin.take()); + let output = finish(child).await; + assert!(!output.status.success()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("is not ready (phase: Stopped)"), "{stderr}"); + assert!(gateway.calls.lock().unwrap().starts.is_empty()); +} + #[tokio::test] async fn default_open_pipe_checks_trailer_error_after_exit() { let gateway = TestGateway::start(Scenario::ErrorAfterExit).await; diff --git a/crates/openshell-server/src/grpc/mutation_replay/streaming/tests.rs b/crates/openshell-server/src/grpc/mutation_replay/streaming/tests.rs index 112cf64157..97b05d5a01 100644 --- a/crates/openshell-server/src/grpc/mutation_replay/streaming/tests.rs +++ b/crates/openshell-server/src/grpc/mutation_replay/streaming/tests.rs @@ -167,6 +167,17 @@ async fn receive_relay(state: &ServerState, messages: &mut mpsc::Receiver("default", &req.sandbox) + .await + .unwrap() + .unwrap(); + sandbox.set_phase(phase as i32); + state.store.put_message(&sandbox).await.unwrap(); +} + async fn completion(state: &ServerState) -> Completion { let rows = state .store @@ -210,6 +221,36 @@ fn interactive_fingerprint_uses_start_schema_and_ignores_request_id() { ); } +#[tokio::test] +async fn exec_reaches_terminal_sandbox_only_while_supervisor_is_connected() { + // A supervisor retains its access plane after the main process exits. + // Exec follows the same reachability rule as SSH and port forwarding. + for interactive in [false, true] { + for phase in [SandboxPhase::Completed, SandboxPhase::Error] { + let directory = tempfile::tempdir().unwrap(); + let state = setup("sqlite::memory:", &directory).await; + let req = request(&state).await; + set_phase(&state, &req, phase).await; + let (mut client, server) = wire_client(state.clone(), None, None).await; + + let status = exec_rpc(&mut client, req.clone(), interactive) + .await + .unwrap_err(); + assert_eq!(status.code(), tonic::Code::FailedPrecondition, "{phase:?}"); + assert_eq!(status.message(), "sandbox is not ready"); + + let mut messages = register(&state, &sandbox_id(&state, &req).await); + let mut retry = req; + retry.request_id = uuid::Uuid::new_v4().to_string(); + exec_rpc(&mut client, retry, interactive) + .await + .unwrap_or_else(|status| panic!("{phase:?} interactive={interactive}: {status}")); + receive_relay(&state, &mut messages).await; + server.abort(); + } + } +} + #[tokio::test] async fn concurrent_exec_claims_open_one_relay_and_store_no_secrets() { let directory = tempfile::tempdir().unwrap(); diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index cac481b149..591159fb2f 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -276,13 +276,6 @@ pub(super) async fn resolve_and_authorize_sandbox_name( Ok(sandbox) } -fn require_ready_sandbox(sandbox: &Sandbox) -> Result<(), Status> { - match SandboxPhase::try_from(sandbox.phase()).ok() { - Some(SandboxPhase::Ready) => Ok(()), - _ => Err(Status::failed_precondition("sandbox is not ready")), - } -} - fn generate_routable_name() -> String { let name = petname::petname(2, "-").unwrap_or_else(generate_name); let mut truncated = &name[..name.len().min(MAX_ROUTABLE_NAME_LEN)]; @@ -2544,7 +2537,7 @@ pub(super) async fn handle_exec_sandbox( completion.ensure_target(sandbox.object_id())?; } - require_ready_sandbox(&sandbox)?; + require_relay_reachable(state, &sandbox).await?; // Open a relay channel through the supervisor session. Use a 15s // session-wait timeout, enough to cover a transient supervisor reconnect @@ -2678,9 +2671,7 @@ pub(super) async fn handle_forward_tcp( // The main process may finish between minting the SSH token and opening // its transport. Keep the relay reachable until terminal delivery is // finalized so fast commands can attach without a readiness race. - if !sandbox_relay_reachable(state, &sandbox) { - return Err(Status::failed_precondition("sandbox is not ready")); - } + require_relay_reachable(state, &sandbox).await?; let connection_guard = acquire_forward_connection_guard(state, &principal, &init, &sandbox).await?; @@ -3123,7 +3114,7 @@ pub(super) async fn handle_exec_sandbox_interactive_start( completion.ensure_target(sandbox.object_id())?; } - require_ready_sandbox(&sandbox)?; + require_relay_reachable(state, &sandbox).await?; let (channel_id, relay_rx) = crate::supervisor_session::open_routed_relay_with_target( state, @@ -3191,11 +3182,17 @@ pub(super) async fn handle_exec_sandbox_interactive_start( // SSH session handlers // --------------------------------------------------------------------------- -fn sandbox_relay_reachable(state: &ServerState, sandbox: &Sandbox) -> bool { - let phase = SandboxPhase::try_from(sandbox.phase()).ok(); - matches!(phase, Some(SandboxPhase::Ready)) - || (matches!(phase, Some(SandboxPhase::Completed | SandboxPhase::Error)) - && state.supervisor_sessions.has_session(sandbox.object_id())) +/// Exec, SSH, and port forwarding share one access rule: a Completed or +/// Error sandbox stays reachable while its supervisor retains the access plane. +async fn require_relay_reachable( + state: &Arc, + sandbox: &Sandbox, +) -> Result<(), Status> { + if crate::supervisor_session::sandbox_relay_reachable(state, sandbox).await? { + Ok(()) + } else { + Err(Status::failed_precondition("sandbox is not ready")) + } } pub(super) async fn handle_create_ssh_session( @@ -3214,9 +3211,7 @@ pub(super) async fn handle_create_ssh_session( .await?; let sandbox_id = sandbox.object_id().to_string(); - if !sandbox_relay_reachable(state, &sandbox) { - return Err(Status::failed_precondition("sandbox is not ready")); - } + require_relay_reachable(state, &sandbox).await?; let token = uuid::Uuid::new_v4().to_string(); let now_ms = current_time_ms(); @@ -9273,10 +9268,18 @@ mod tests { .supervisor_sessions .finalize_main_process_exit("sandbox-work") ); - assert!(sandbox_relay_reachable(&state, &sandbox)); + assert!( + crate::supervisor_session::sandbox_relay_reachable(&state, &sandbox) + .await + .unwrap() + ); assert!(state.supervisor_sessions.disconnect("sandbox-work")); - assert!(!sandbox_relay_reachable(&state, &sandbox)); + assert!( + !crate::supervisor_session::sandbox_relay_reachable(&state, &sandbox) + .await + .unwrap() + ); } #[tokio::test] diff --git a/crates/openshell-server/src/supervisor_session.rs b/crates/openshell-server/src/supervisor_session.rs index a0fb2654be..a47027c358 100644 --- a/crates/openshell-server/src/supervisor_session.rs +++ b/crates/openshell-server/src/supervisor_session.rs @@ -1781,6 +1781,34 @@ fn owner_is_fresh(owner: &crate::supervisor_owner::OwnerRecord) -> bool { owner.is_fresh(OWNER_TTL) } +/// Whether the gateway can relay a new connection into this sandbox. +/// +/// A Ready sandbox always qualifies; relay opening waits for and routes to its +/// owner. After the main process exits, the supervisor keeps its access plane, +/// so a Completed or Error sandbox qualifies while its supervisor session is +/// live on this replica or a fresh owner record names a peer replica. +pub async fn sandbox_relay_reachable( + state: &Arc, + sandbox: &Sandbox, +) -> Result { + match SandboxPhase::try_from(sandbox.phase()).ok() { + Some(SandboxPhase::Ready) => Ok(true), + Some(SandboxPhase::Completed | SandboxPhase::Error) => { + let sandbox_id = sandbox.object_id(); + if state.supervisor_sessions.has_session(sandbox_id) { + return Ok(true); + } + let owner_index = SupervisorOwnerIndex::new(state.store.clone(), OWNER_TTL); + Ok(resolve_owner(state, &owner_index, sandbox_id) + .await? + .is_some_and(|owner| { + owner_is_fresh(&owner) && owner.owner_replica_id != state.replica_id + })) + } + _ => Ok(false), + } +} + /// The replica currently holding this sandbox's supervisor session, if known. pub async fn owner_replica_id(state: &Arc, sandbox_id: &str) -> Option { if state.store.is_single_replica() { @@ -3535,6 +3563,87 @@ mod tests { .expect("complete chart peer TLS materials should configure tonic"); } + #[tokio::test] + async fn relay_reachability_after_exit_follows_the_live_session_on_any_replica() { + let state = crate::grpc::test_support::test_server_state().await; + let reachable = |sandbox: Sandbox| { + let state = state.clone(); + async move { sandbox_relay_reachable(&state, &sandbox).await.unwrap() } + }; + let in_phase = |id: &str, phase: SandboxPhase| { + let mut sandbox = sandbox_record(id, id); + sandbox.set_phase(phase as i32); + sandbox + }; + + // Ready routes to the owner during relay opening; never-started or + // stopped sandboxes are refused regardless of sessions. + assert!(reachable(in_phase("relay-ready", SandboxPhase::Ready)).await); + for phase in [ + SandboxPhase::Provisioning, + SandboxPhase::Stopped, + SandboxPhase::Deleting, + ] { + assert!( + !reachable(in_phase("relay-local", phase)).await, + "{phase:?}" + ); + } + + for phase in [SandboxPhase::Completed, SandboxPhase::Error] { + // No session anywhere. + assert!(!reachable(in_phase("relay-none", phase)).await, "{phase:?}"); + + // Session held by this replica. + let (tx, _rx) = mpsc::channel(1); + let (shutdown_tx, _shutdown_rx) = oneshot::channel(); + state.supervisor_sessions.register( + "relay-local".to_string(), + "session".to_string(), + tx, + shutdown_tx, + ); + assert!(reachable(in_phase("relay-local", phase)).await, "{phase:?}"); + assert!(state.supervisor_sessions.disconnect("relay-local")); + assert!( + !reachable(in_phase("relay-local", phase)).await, + "{phase:?}" + ); + } + + // Session held by a peer replica, known only through its owner record. + let owner_index = SupervisorOwnerIndex::new(state.store.clone(), OWNER_TTL); + owner_index + .publish( + "relay-peer", + "session", + "supervisor", + 1, + "peer-replica", + "https://peer", + ) + .await + .unwrap(); + for phase in [SandboxPhase::Completed, SandboxPhase::Error] { + assert!(reachable(in_phase("relay-peer", phase)).await, "{phase:?}"); + } + + // An owner record naming this replica without a local session is a + // leftover from a lost session, not a live supervisor. + owner_index + .publish( + "relay-self", + "session", + "supervisor", + 1, + &state.replica_id, + "https://self", + ) + .await + .unwrap(); + assert!(!reachable(in_phase("relay-self", SandboxPhase::Completed)).await); + } + fn sandbox_record(id: &str, name: &str) -> Sandbox { Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { From 558bfd81ad4934d7f3e8fc638cd08bde2999c8d6 Mon Sep 17 00:00:00 2001 From: Piotr Mlocek Date: Mon, 26 Oct 2026 16:58:19 -0700 Subject: [PATCH 2/4] fix(server): route exposed services to exited sandboxes with a live supervisor Exposed-service HTTP routing still required Ready while port forwarding to the same sandbox kept working after the main process exited. Service routing now uses the shared relay reachability gate, so a retained Completed or Error sandbox serves its exposed services while its supervisor stays connected. Document what stays reachable after a retained sandbox's main process exits, and that --no-keep sandboxes are deleted once it does. Signed-off-by: Piotr Mlocek --- .../openshell-server/src/service_routing.rs | 88 ++++++++++++++++++- docs/how-it-works/sandboxes/overview.mdx | 7 +- skills/openshell-cli/SKILL.md | 5 +- 3 files changed, 96 insertions(+), 4 deletions(-) diff --git a/crates/openshell-server/src/service_routing.rs b/crates/openshell-server/src/service_routing.rs index a84245d2cc..3e8385f53f 100644 --- a/crates/openshell-server/src/service_routing.rs +++ b/crates/openshell-server/src/service_routing.rs @@ -12,7 +12,7 @@ use hyper_util::rt::TokioIo; use openshell_core::ObjectId; use openshell_core::config::ServiceRoutingConfig; use openshell_core::proto::{ - Sandbox, SandboxPhase, ServiceAuthorizationMode, ServiceEndpoint, TcpRelayTarget, relay_open, + Sandbox, ServiceAuthorizationMode, ServiceEndpoint, TcpRelayTarget, relay_open, }; use openshell_ocsf::{ ActionId, ActivityId, ConfigStateChangeBuilder, DispositionId, Endpoint, EventContext, @@ -394,7 +394,25 @@ async fn proxy_to_endpoint( return Err(route_err); } }; - if SandboxPhase::try_from(sandbox.phase()).ok() != Some(SandboxPhase::Ready) { + // Services follow the same access rule as exec, SSH, and port forwarding. + let reachable = match crate::supervisor_session::sandbox_relay_reachable(&state, &sandbox).await + { + Ok(reachable) => reachable, + Err(err) => { + warn!(error = %err, sandbox_id = %endpoint.sandbox_id, "sandbox service routing: failed to resolve supervisor owner"); + let route_err = ServiceRouteError::internal_error(); + emit_service_http_failure( + &state, + &req, + &sandbox_name, + &service_name, + Some(&endpoint), + &route_err, + ); + return Err(route_err); + } + }; + if !reachable { let err = ServiceRouteError::sandbox_not_ready(); emit_service_http_failure( &state, @@ -1611,6 +1629,72 @@ mod tests { assert_eq!(upstream.headers()[header::UPGRADE], "websocket"); } + #[tokio::test] + async fn completed_sandbox_routes_services_only_while_supervisor_is_live() { + use openshell_core::proto::{GatewayMessage, SandboxPhase, gateway_message}; + + let state = crate::grpc::test_support::test_server_state().await; + let mut ep = endpoint(); + ep.metadata.as_mut().unwrap().id = "svc-completed-endpoint".to_string(); + ep.metadata.as_mut().unwrap().name = "svc-completed--web".to_string(); + ep.sandbox_id = "svc-completed".to_string(); + ep.sandbox = "svc-completed".to_string(); + state.store.put_message(&ep).await.unwrap(); + let mut sandbox = Sandbox { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: "svc-completed".to_string(), + name: "svc-completed".to_string(), + workspace: "default".to_string(), + ..Default::default() + }), + ..Default::default() + }; + sandbox.set_phase(SandboxPhase::Completed as i32); + state.store.put_message(&sandbox).await.unwrap(); + let route = |state: Arc| async move { + let req = Request::builder().uri("/").body(Body::empty()).unwrap(); + proxy_to_endpoint( + state, + req, + "default", + "svc-completed".to_string(), + "web".to_string(), + ) + .await + }; + + let Err(err) = route(state.clone()).await else { + panic!("a Completed sandbox without a supervisor must not route"); + }; + assert_eq!(err.reason, ServiceRouteError::sandbox_not_ready().reason); + + // With the supervisor retaining its access plane, the request passes + // the gate and asks the supervisor for a relay. + let (tx, mut messages) = tokio::sync::mpsc::channel::(1); + let (shutdown_tx, _shutdown_rx) = tokio::sync::oneshot::channel(); + state.supervisor_sessions.register( + "svc-completed".to_string(), + "session".to_string(), + tx, + shutdown_tx, + ); + let routed = tokio::spawn(route(state.clone())); + let message = tokio::time::timeout(Duration::from_secs(5), messages.recv()) + .await + .unwrap() + .unwrap(); + let Some(gateway_message::Payload::RelayOpen(open)) = message.payload else { + panic!("expected relay open"); + }; + state + .supervisor_sessions + .fail_pending_relay(&open.channel_id, "test relay ended".into()); + let Err(err) = routed.await.unwrap() else { + panic!("a failed relay must not produce a response"); + }; + assert_eq!(err.reason, ServiceRouteError::service_unreachable().reason); + } + #[tokio::test] async fn load_endpoint_uses_workspace_for_lookup() { let store = crate::persistence::test_store().await; diff --git a/docs/how-it-works/sandboxes/overview.mdx b/docs/how-it-works/sandboxes/overview.mdx index 430e90c92f..736779970c 100644 --- a/docs/how-it-works/sandboxes/overview.mdx +++ b/docs/how-it-works/sandboxes/overview.mdx @@ -1080,7 +1080,12 @@ temporarily while its supervisor reconnects. Wait for the phase to return to The gateway records a successful canonical main-process exit as `Ready=False` with reason `MainProcessCompleted`. Nonzero and signal-normalized results use `MainProcessFailed` and the `Error` phase. It also sets `status.exit_code`; -signal exits use the standard `128 + signal` convention. When restart policy +signal exits use the standard `128 + signal` convention. After the main +process of a retained sandbox exits, you can still connect, execute commands, +transfer files, forward ports, and reach exposed services while the sandbox +supervisor stays connected. An ephemeral (`--no-keep`) sandbox is deleted after its main process +exits, and an `Error` caused by an infrastructure failure usually has no +connected supervisor, so neither accepts new sessions. When restart policy selects replacement, `status.restart_count` and `status.next_restart_time` show the attempt number and next deadline. Inspect them with `openshell sandbox get `. `sandbox stop` cancels a pending replacement; diff --git a/skills/openshell-cli/SKILL.md b/skills/openshell-cli/SKILL.md index 84e6d65a2c..07ef0b6f17 100644 --- a/skills/openshell-cli/SKILL.md +++ b/skills/openshell-cli/SKILL.md @@ -522,7 +522,10 @@ openshell sandbox start [name] Both commands default to the last-used sandbox. Stop stops background forwards and waits for `Stopped`; start waits for `Ready`. Connect, exec, file transfer, forwarding, and exposed services are unavailable while -stopped or completed. Starting a retained `Completed` or +stopped. After the main process of a retained sandbox exits (`Completed`, or +`Error` with `MainProcessFailed`), connect, exec, file transfer, forwarding, and +exposed services keep working while the supervisor stays connected. `--no-keep` sandboxes are +deleted once the main process exits. Starting a retained `Completed` or `Error/MainProcessFailed` sandbox launches a fresh canonical-main instance and invalidates SSH sessions from the previous runtime generation. Delete remains the operation that removes retained state. From 67cf0d0241e1294f1138e2d22f38a7d9a5438a82 Mon Sep 17 00:00:00 2001 From: Piotr Mlocek Date: Mon, 26 Oct 2026 16:59:07 -0700 Subject: [PATCH 3/4] test(conformance): certify sandbox retention after main process exit Add a sandbox-retention scenario that runs for exit codes 0 and 3: an ephemeral (--no-keep) sandbox returns the exit code and is deleted, and a retained sandbox records Completed or Error with the exit code and still accepts exec. Signed-off-by: Piotr Mlocek --- crates/openshell-conformance/src/lib.rs | 2 +- .../src/scenarios/mod.rs | 2 +- .../src/scenarios/sandbox_lifecycle.rs | 173 +++++++++++++++++- .../suites/conformance/cli/tests/retention.rs | 22 +++ 4 files changed, 193 insertions(+), 6 deletions(-) create mode 100644 tests/suites/conformance/cli/tests/retention.rs diff --git a/crates/openshell-conformance/src/lib.rs b/crates/openshell-conformance/src/lib.rs index 4d1fcc1308..89bb5f9e19 100644 --- a/crates/openshell-conformance/src/lib.rs +++ b/crates/openshell-conformance/src/lib.rs @@ -27,7 +27,7 @@ pub use scenarios::{ FILE_TRANSFER_GIT_FILTERING_SCENARIO, FILE_TRANSFER_PATH_SAFETY_SCENARIO, FILE_TRANSFER_ROUND_TRIP_SCENARIO, MECHANISTIC_PROPOSAL_SCENARIO, NEW_HOSTNAME_PROPOSAL_SCENARIO, POLICY_LOCAL_SCENARIO, SANDBOX_LIFECYCLE_SCENARIO, - SMOKE_SCENARIO, + SANDBOX_RETENTION_SCENARIO, SMOKE_SCENARIO, }; /// A portable conformance scenario exercised by the Cargo test suite. diff --git a/crates/openshell-conformance/src/scenarios/mod.rs b/crates/openshell-conformance/src/scenarios/mod.rs index 72a90925f0..615f50f2af 100644 --- a/crates/openshell-conformance/src/scenarios/mod.rs +++ b/crates/openshell-conformance/src/scenarios/mod.rs @@ -15,5 +15,5 @@ pub use file_transfer::{ pub use policy_behavior::{ MECHANISTIC_PROPOSAL_SCENARIO, NEW_HOSTNAME_PROPOSAL_SCENARIO, POLICY_LOCAL_SCENARIO, }; -pub use sandbox_lifecycle::SANDBOX_LIFECYCLE_SCENARIO; +pub use sandbox_lifecycle::{SANDBOX_LIFECYCLE_SCENARIO, SANDBOX_RETENTION_SCENARIO}; pub use smoke::SMOKE_SCENARIO; diff --git a/crates/openshell-conformance/src/scenarios/sandbox_lifecycle.rs b/crates/openshell-conformance/src/scenarios/sandbox_lifecycle.rs index a0b1c0016d..8f09d3f1de 100644 --- a/crates/openshell-conformance/src/scenarios/sandbox_lifecycle.rs +++ b/crates/openshell-conformance/src/scenarios/sandbox_lifecycle.rs @@ -20,6 +20,8 @@ struct SandboxState { id: String, name: String, phase: String, + #[serde(default)] + exit_code: Option, } #[derive(Debug, Deserialize)] @@ -41,6 +43,124 @@ fn run_sandbox_lifecycle(runner: &mut OpenShellRunner) -> ScenarioFuture<'_> { }) } +/// Certify what happens to a sandbox after its main process exits. +/// +/// An ephemeral sandbox is deleted whether the main process succeeds or +/// fails. A retained sandbox records the result and stays reachable through +/// exec, because the supervisor keeps its access plane after the exit. +pub const SANDBOX_RETENTION_SCENARIO: Scenario = Scenario { + name: "sandbox-retention", + run: run_sandbox_retention, +}; + +fn run_sandbox_retention(runner: &mut OpenShellRunner) -> ScenarioFuture<'_> { + Box::pin(async move { + for exit_code in [0, 3] { + ephemeral_is_deleted_after_exit(runner, exit_code).await?; + retained_accepts_exec_after_exit(runner, exit_code).await?; + } + Ok(()) + }) +} + +async fn ephemeral_is_deleted_after_exit( + runner: &mut OpenShellRunner, + exit_code: i32, +) -> Result<(), String> { + let sandbox_name = format!("ct-{}-e{exit_code}", runner.id()); + let step = format!("ephemeral-exit-{exit_code}"); + runner.track_sandbox(&sandbox_name); + let create = runner + .step(format!("{step}/create")) + .description(format!( + "ephemeral sandbox '{sandbox_name}' runs its main process and returns exit code {exit_code}" + )) + .with_timeout(CREATE_TIMEOUT) + .run(&[ + "sandbox", + "create", + "--name", + &sandbox_name, + "--no-keep", + "--no-tty", + "--", + "sh", + "-c", + &format!("echo ephemeral-done; exit {exit_code}"), + ]) + .await + .map_err(|error| error.to_string())?; + if create.exit_code() != Some(exit_code) { + return Err(create.failure_diagnostic(&format!("create exits with code {exit_code}"))); + } + if !create.stdout().contains("ephemeral-done") { + return Err(create.failure_diagnostic("main process stdout is streamed to the caller")); + } + + wait_for_name_absence(runner, &sandbox_name, &format!("{step}/deleted")).await?; + runner.forget_sandbox(&sandbox_name); + Ok(()) +} + +async fn retained_accepts_exec_after_exit( + runner: &mut OpenShellRunner, + exit_code: i32, +) -> Result<(), String> { + let sandbox_name = format!("ct-{}-r{exit_code}", runner.id()); + let step = format!("retained-exit-{exit_code}"); + let result_path = "/sandbox/.openshell-retained-result"; + let result = format!("openshell-retained-{}-{exit_code}", runner.id()); + runner.track_sandbox(&sandbox_name); + let create = runner + .step(format!("{step}/create")) + .description(format!("retained sandbox '{sandbox_name}' is created")) + .with_timeout(CREATE_TIMEOUT) + .run(&[ + "sandbox", + "create", + "--name", + &sandbox_name, + "--detach", + "--no-tty", + "--", + "sh", + "-c", + &format!("printf '%s\\n' '{result}' > '{result_path}' && sync; exit {exit_code}"), + ]) + .await + .map_err(|error| error.to_string())?; + create.require_success()?; + + let expected_phase = if exit_code == 0 { "Completed" } else { "Error" }; + let state = wait_for_phase( + runner, + &sandbox_name, + expected_phase, + &format!("{step}/{}", expected_phase.to_lowercase()), + ) + .await?; + if state.exit_code != Some(exit_code) { + return Err(format!( + "sandbox '{sandbox_name}' recorded exit code {:?}; expected {exit_code}", + state.exit_code + )); + } + + exec_expect_exact( + runner, + &sandbox_name, + &format!("{step}/exec-after-exit"), + &["cat", result_path], + &format!("{result}\n"), + ) + .await?; + + run_lifecycle_command(runner, "delete", &sandbox_name, &format!("{step}/delete")).await?; + wait_for_absence(runner, &state.id, &sandbox_name, &format!("{step}/deleted")).await?; + runner.forget_sandbox(&sandbox_name); + Ok(()) +} + async fn stop_start_preserves_workspace(runner: &mut OpenShellRunner) -> Result<(), String> { let sandbox_name = format!("ct-{}-ss", runner.id()); let sentinel = format!("openshell-stop-start-{}", runner.id()); @@ -56,7 +176,7 @@ async fn stop_start_preserves_workspace(runner: &mut OpenShellRunner) -> Result< exec_expect_exact( runner, &sandbox_name, - "write-sentinel", + "stop-start/write-sentinel", &[ "sh", "-lc", @@ -99,7 +219,7 @@ async fn stop_start_preserves_workspace(runner: &mut OpenShellRunner) -> Result< exec_expect_exact( runner, &sandbox_name, - "read-sentinel", + "stop-start/read-sentinel", &["cat", sentinel_path], &format!("{sentinel}\n"), ) @@ -107,7 +227,7 @@ async fn stop_start_preserves_workspace(runner: &mut OpenShellRunner) -> Result< exec_expect_exact( runner, &sandbox_name, - "read-main-run-count", + "stop-start/read-main-run-count", &["cat", run_count_path], "2\n", ) @@ -190,7 +310,7 @@ async fn exec_expect_exact( let mut args = vec!["sandbox", "exec", "--name", sandbox_name, "--no-tty", "--"]; args.extend_from_slice(command); let result = runner - .step(format!("stop-start/{step}")) + .step(step) .description(format!("sandbox '{sandbox_name}' exec {step} succeeds")) .with_timeout(COMMAND_TIMEOUT) .run(&args) @@ -283,6 +403,51 @@ async fn wait_for_absence( .map_err(|error| error.to_string()) } +/// Wait until `sandbox get` reports that the name no longer exists. Used +/// when the gateway deletes the sandbox on its own, so the caller may never +/// observe its ID. +async fn wait_for_name_absence( + runner: &mut OpenShellRunner, + sandbox_name: &str, + step: &str, +) -> Result<(), String> { + let sandbox_name = sandbox_name.to_string(); + let step = step.to_string(); + let poll_step = step.clone(); + runner + .poll_until( + &poll_step, + TRANSITION_TIMEOUT, + TRANSITION_INTERVAL, + async move |runner| { + let result = runner + .step(format!("{step}/get")) + .description(format!("sandbox '{sandbox_name}' no longer exists")) + .with_timeout(COMMAND_TIMEOUT) + .run(&["sandbox", "get", &sandbox_name, "--output", "json"]) + .await; + match result { + Ok(result) if result.success() => match result.json::() { + Ok(state) => Poll::Pending(format!( + "sandbox '{sandbox_name}' still exists in phase {:?}", + state.phase + )), + Err(error) => Poll::Failed(error.to_string()), + }, + Ok(result) if result.stderr().to_lowercase().contains("not found") => { + Poll::Ready(()) + } + Ok(result) => Poll::Pending(result.failure_diagnostic(&format!( + "sandbox get reports that '{sandbox_name}' was not found" + ))), + Err(error) => Poll::Pending(error.to_string()), + } + }, + ) + .await + .map_err(|error| error.to_string()) +} + async fn sandbox_is_listed( runner: &OpenShellRunner, sandbox_id: &str, diff --git a/tests/suites/conformance/cli/tests/retention.rs b/tests/suites/conformance/cli/tests/retention.rs new file mode 100644 index 0000000000..6e957bd85e --- /dev/null +++ b/tests/suites/conformance/cli/tests/retention.rs @@ -0,0 +1,22 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Driver-agnostic sandbox retention conformance tests. + +use openshell_conformance::{OpenShellRunner, SANDBOX_RETENTION_SCENARIO}; + +/// Exercise ephemeral deletion and retained access after the main process exits. +#[tokio::test] +async fn sandbox_retention() { + let mut runner = OpenShellRunner::from_env(SANDBOX_RETENTION_SCENARIO.name) + .expect("candidate openshell CLI is available"); + + let result = async { + runner.check_gateway_status().await?; + SANDBOX_RETENTION_SCENARIO.run(&mut runner).await + } + .await; + if let Err(error) = runner.finish(result).await { + panic!("sandbox retention conformance scenario failed:\n{error}"); + } +} From 2f98bd9179b83c9cc5594e9abe48623e4e20724e Mon Sep 17 00:00:00 2001 From: Piotr Mlocek Date: Mon, 26 Oct 2026 18:47:32 -0700 Subject: [PATCH 4/4] docs(sandboxes): add lifecycle diagram and post-exit behavior Add a phase diagram to the Sandbox Lifecycle section and describe what happens after the main process exits. A --no-keep sandbox is deleted. A retained sandbox keeps its compute and still accepts exec, file transfer, port forwarding and exposed services while its supervisor is connected. sandbox connect fails because it attaches to the exited main process, so the docs point to sandbox exec --tty for a shell. Signed-off-by: Piotr Mlocek --- docs/how-it-works/sandboxes/overview.mdx | 56 +++++++++++++++++++++--- skills/openshell-cli/SKILL.md | 5 ++- 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/docs/how-it-works/sandboxes/overview.mdx b/docs/how-it-works/sandboxes/overview.mdx index 736779970c..1e85a593c5 100644 --- a/docs/how-it-works/sandboxes/overview.mdx +++ b/docs/how-it-works/sandboxes/overview.mdx @@ -1032,6 +1032,28 @@ after it attempts every requested deletion if any entry failed. Every sandbox moves through a defined set of phases: +```mermaid +stateDiagram-v2 + [*] --> Provisioning: create + Provisioning --> Ready: supervisor connects + Provisioning --> Error: preparation or admission times out + Ready --> Provisioning: supervisor disconnects + Ready --> Completed: main process exits 0 + Ready --> Error: main process exits nonzero + Ready --> Stopping: stop + Error --> Stopping: stop + Stopping --> Stopped + Stopped --> Starting: start + Completed --> Starting: start or restart policy + Error --> Starting: start or restart policy + Starting --> Ready: new supervisor connects + Completed --> Deleting: delete, or --no-keep cleanup + Error --> Deleting: delete, or --no-keep cleanup + Stopped --> Deleting: delete + Ready --> Deleting: delete + Deleting --> [*] +``` + Before workload activation, OpenShell validates the effective policy and matching provider configuration. A rejection keeps the workload unstarted and exposes a `ConfigurationInvalid` condition in `Provisioning`. Use `openshell sandbox get` @@ -1080,18 +1102,40 @@ temporarily while its supervisor reconnects. Wait for the phase to return to The gateway records a successful canonical main-process exit as `Ready=False` with reason `MainProcessCompleted`. Nonzero and signal-normalized results use `MainProcessFailed` and the `Error` phase. It also sets `status.exit_code`; -signal exits use the standard `128 + signal` convention. After the main -process of a retained sandbox exits, you can still connect, execute commands, -transfer files, forward ports, and reach exposed services while the sandbox -supervisor stays connected. An ephemeral (`--no-keep`) sandbox is deleted after its main process -exits, and an `Error` caused by an infrastructure failure usually has no -connected supervisor, so neither accepts new sessions. When restart policy +signal exits use the standard `128 + signal` convention. When restart policy selects replacement, `status.restart_count` and `status.next_restart_time` show the attempt number and next deadline. Inspect them with `openshell sandbox get `. `sandbox stop` cancels a pending replacement; an explicit later start resets the restart count. Compute runtimes do not apply their own restart policy. +### After the main process exits + +Whether a sandbox outlives its main process depends on how you created it: + +- With `--no-keep`, the sandbox's lifecycle is tied to the main process. The + gateway deletes the sandbox after the main process exits, whatever the exit + code, once its output has been delivered. +- By default, the sandbox is retained. It moves to `Completed` or `Error`, and + its compute keeps running: the workload with its filesystem, and the + supervisor. It stays that way until you delete the sandbox or start it again. + +In a retained `Completed` or `Error` sandbox, these keep working while its +supervisor stays connected: + +- `sandbox exec`, including `sandbox exec --tty -- /bin/bash -l` for an + interactive shell +- `sandbox upload` and `sandbox download` +- port forwarding +- exposed services + +`sandbox connect` fails, because it attaches to the main process and that +process has exited. Use `sandbox exec --tty` instead. + +An `Error` caused by an infrastructure failure, such as +`ImagePreparationTimedOut`, usually has no supervisor left, so these +operations fail with `sandbox is not ready`. + ## Sandbox Runtimes The gateway's configured compute driver determines how OpenShell creates each sandbox. The CLI workflow stays the same across drivers: you create, connect to, inspect, and delete sandboxes through the gateway API. diff --git a/skills/openshell-cli/SKILL.md b/skills/openshell-cli/SKILL.md index 07ef0b6f17..26bd4e10c7 100644 --- a/skills/openshell-cli/SKILL.md +++ b/skills/openshell-cli/SKILL.md @@ -523,8 +523,9 @@ Both commands default to the last-used sandbox. Stop stops background forwards and waits for `Stopped`; start waits for `Ready`. Connect, exec, file transfer, forwarding, and exposed services are unavailable while stopped. After the main process of a retained sandbox exits (`Completed`, or -`Error` with `MainProcessFailed`), connect, exec, file transfer, forwarding, and -exposed services keep working while the supervisor stays connected. `--no-keep` sandboxes are +`Error` with `MainProcessFailed`), exec, file transfer, forwarding, and +exposed services keep working while the supervisor stays connected. `connect` +fails because the main process has exited; use `sandbox exec --tty` for a shell. `--no-keep` sandboxes are deleted once the main process exits. Starting a retained `Completed` or `Error/MainProcessFailed` sandbox launches a fresh canonical-main instance and invalidates SSH sessions from the previous runtime generation. Delete remains