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-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/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/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/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 { diff --git a/docs/how-it-works/sandboxes/overview.mdx b/docs/how-it-works/sandboxes/overview.mdx index 430e90c92f..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` @@ -1087,6 +1109,33 @@ show the attempt number and next deadline. Inspect them with 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 84e6d65a2c..26bd4e10c7 100644 --- a/skills/openshell-cli/SKILL.md +++ b/skills/openshell-cli/SKILL.md @@ -522,7 +522,11 @@ 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`), 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 the operation that removes retained state. 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}"); + } +}