diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index eb224685ed..dec7a51986 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -2014,15 +2014,6 @@ 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) { - return Err(miette::miette!( - "sandbox '{}' is not ready (phase: {}); wait for it to reach Ready state", - name, - phase_name(sandbox.phase()) - )); - } - // Resolve TTY mode: explicit --tty / --no-tty wins, otherwise auto-detect. let stdin_is_terminal = std::io::stdin().is_terminal(); let tty = tty_override.unwrap_or_else(|| stdin_is_terminal && std::io::stdout().is_terminal()); @@ -2115,7 +2106,7 @@ pub async fn sandbox_exec_grpc( } (response, _) => response, } - .into_diagnostic()? + .map_err(|status| exec_start_error(status, &sandbox))? .into_inner(); // Stream output to terminal in real-time. @@ -2169,7 +2160,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_sandbox_for_forward(&mut client, name, workspace).await?; let listener = tokio::net::TcpListener::bind((bind_addr.as_str(), bind_port)) .await @@ -2203,7 +2194,7 @@ pub async fn service_forward_tcp( } _ = health_check.tick() => { - replica = fetch_ready_sandbox_for_forward(&mut client, name, workspace).await?; + replica = fetch_sandbox_for_forward(&mut client, name, workspace).await?; } accepted = listener.accept() => { @@ -2269,8 +2260,13 @@ 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( +/// Confirm the sandbox can still serve the forward, returning the replica that +/// owns it. +/// +/// The gateway decides whether each connection may reach the sandbox; this +/// only stops a forward whose sandbox is gone or is stopping, stopped, or +/// being deleted, since no connection can succeed until a new command acts. +async fn fetch_sandbox_for_forward( client: &mut crate::tls::GrpcClient, name: &str, workspace: &str, @@ -2299,10 +2295,9 @@ 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 forward_ended_by_phase(sandbox.phase()) { return Err(miette::miette!( - "sandbox '{}' is no longer ready (phase: {}); stopping service forward", - name, + "sandbox '{name}' is no longer available (phase: {}); stopping service forward", phase_name(sandbox.phase()) )); } @@ -2310,6 +2305,15 @@ async fn fetch_ready_sandbox_for_forward( Ok(replica) } +/// Phases in which no forwarded connection can succeed until another command +/// starts or recreates the sandbox. +fn forward_ended_by_phase(phase: i32) -> bool { + matches!( + SandboxPhase::try_from(phase), + Ok(SandboxPhase::Stopping | SandboxPhase::Stopped | SandboxPhase::Deleting) + ) +} + #[derive(Debug)] struct ForwardTcpConnectionError { message: String, @@ -2576,6 +2580,28 @@ impl Drop for TaskGuard { } } +/// The gateway decides whether a sandbox can be used. When it refuses, name the +/// phase this command saw and what would make the sandbox usable again. +fn exec_start_error(status: Status, sandbox: &Sandbox) -> miette::Report { + if status.code() == Code::FailedPrecondition && status.message() == "sandbox is not ready" { + let name = sandbox.object_name(); + let hint = match SandboxPhase::try_from(sandbox.phase()) { + Ok(SandboxPhase::Stopping | SandboxPhase::Stopped) => { + format!("start it with `openshell sandbox start {name}`") + } + Ok(SandboxPhase::Completed | SandboxPhase::Error) => { + "its main process exited and its supervisor is no longer connected".to_string() + } + _ => "wait for it to reach Ready state".to_string(), + }; + return miette::miette!( + "sandbox '{name}' is not ready (phase: {}); {hint}", + phase_name(sandbox.phase()) + ); + } + miette::Report::from_err(status) +} + #[allow(clippy::too_many_arguments)] async fn sandbox_exec_streaming_grpc( mut client: crate::tls::GrpcClient, @@ -2638,7 +2664,7 @@ async fn sandbox_exec_streaming_grpc( } response => response, } - .into_diagnostic()? + .map_err(|status| exec_start_error(status, sandbox))? .into_inner(); // Raw mode is only appropriate for an interactive terminal, not a pipe. @@ -7124,14 +7150,14 @@ fn format_endpoint(endpoint: &openshell_core::proto::NetworkEndpoint) -> String mod tests { use super::{ ForwardTcpConnectionError, PolicyGetView, ProvisioningStep, build_sandbox_resource_limits, - format_endpoint, format_log_line, forward_requires_session_token, git_sync_files, - has_main_process_result, parse_cli_setting_value, parse_credential_expiry_cli_value, - parse_driver_config_json, parse_env_from_pairs, parse_secret_material_env_pairs, - policy_revision_list_json, policy_revision_to_json, proto_execution_timeout, - provisioning_timeout_message, ready_false_condition_message, relay_local_socket, - resolve_from, rootfs_tar_sources_supported_for_gateway, sandbox_should_persist, - sandbox_upload_plan, service_endpoint_to_json, service_status_error, - service_url_for_gateway, workspace_member_to_json, + format_endpoint, format_log_line, forward_ended_by_phase, forward_requires_session_token, + git_sync_files, has_main_process_result, parse_cli_setting_value, + parse_credential_expiry_cli_value, parse_driver_config_json, parse_env_from_pairs, + parse_secret_material_env_pairs, policy_revision_list_json, policy_revision_to_json, + proto_execution_timeout, provisioning_timeout_message, ready_false_condition_message, + relay_local_socket, resolve_from, rootfs_tar_sources_supported_for_gateway, + sandbox_should_persist, sandbox_upload_plan, service_endpoint_to_json, + service_status_error, service_url_for_gateway, workspace_member_to_json, }; use openshell_core::proto::TcpForwardFrame; @@ -9498,4 +9524,27 @@ mod tests { "authorization_token is required for ForwardTcp" ))); } + + #[test] + fn forward_leaves_reachability_after_exit_to_the_gateway() { + use openshell_core::proto::SandboxPhase; + + for phase in [ + SandboxPhase::Stopping, + SandboxPhase::Stopped, + SandboxPhase::Deleting, + ] { + assert!(forward_ended_by_phase(phase as i32), "{phase:?}"); + } + for phase in [ + SandboxPhase::Provisioning, + SandboxPhase::Starting, + SandboxPhase::Ready, + SandboxPhase::Completed, + SandboxPhase::Error, + SandboxPhase::Suspended, + ] { + assert!(!forward_ended_by_phase(phase as i32), "{phase:?}"); + } + } } diff --git a/crates/openshell-cli/tests/sandbox_exec_streaming_integration.rs b/crates/openshell-cli/tests/sandbox_exec_streaming_integration.rs index 31e67baf0f..522c5ce18b 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>, } @@ -66,6 +67,20 @@ fn exit(code: i32) -> proto::ExecSandboxEvent { } impl MockGateway { + /// The gateway's access rule: only it decides whether a sandbox is usable. + fn require_reachable(&self) -> Result<(), Status> { + if matches!( + self.phase, + proto::SandboxPhase::Ready + | proto::SandboxPhase::Completed + | proto::SandboxPhase::Error + ) { + Ok(()) + } else { + Err(Status::failed_precondition("sandbox is not ready")) + } + } + async fn exchange( self, mut input: tonic::Streaming, @@ -189,7 +204,7 @@ macro_rules! mock_gateway { ..Default::default() }), status: Some(proto::SandboxStatus { - phase: proto::SandboxPhase::Ready.into(), + phase: self.phase.into(), ..Default::default() }), ..Default::default() @@ -200,12 +215,14 @@ macro_rules! mock_gateway { type ExecSandboxStream = EventStream; async fn exec_sandbox(&self, _: Request) -> Result, Status> { + self.require_reachable()?; self.calls.lock().unwrap().unary += 1; Err(Status::unimplemented("test requires streaming exec")) } type ExecSandboxInteractiveStream = EventStream; async fn exec_sandbox_interactive(&self, request: Request>) -> Result, Status> { + self.require_reachable()?; let (sender, receiver) = mpsc::channel(1); let service = self.clone(); tokio::spawn(async move { @@ -326,6 +343,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); @@ -346,6 +367,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 { @@ -478,6 +500,47 @@ 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_names_the_phase_when_the_gateway_refuses() { + 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()); + // The error renderer wraps long messages, so compare the words alone. + let stderr = String::from_utf8_lossy(&output.stderr) + .split_whitespace() + .filter(|word| *word != "│") + .collect::>() + .join(" "); + assert!( + stderr.contains("is not ready (phase: Stopped); start it with `openshell sandbox start"), + "{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/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index a6768040e6..bf25a46a2c 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -6963,7 +6963,7 @@ fn terminal_stop_exit_code(sandbox: &Sandbox) -> Option { sandbox.status.as_ref()?.exit_code } -fn is_failed_main_process_result(sandbox: &Sandbox) -> bool { +pub fn is_failed_main_process_result(sandbox: &Sandbox) -> bool { sandbox.phase() == SandboxPhase::Error as i32 && sandbox.status.as_ref().is_some_and(|status| { status.exit_code.is_some() 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 86ff12b170..f0f77d70ef 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)]; @@ -2742,20 +2735,10 @@ pub(super) async fn handle_exec_sandbox( completion.ensure_target(sandbox.object_id())?; } - require_ready_sandbox(&sandbox)?; - - // Open a relay channel through the supervisor session. Use a 15s - // session-wait timeout, enough to cover a transient supervisor reconnect - // while still failing quickly during normal operation. - let (channel_id, relay_rx) = crate::supervisor_session::open_routed_relay_with_target( - state, - sandbox.object_id(), - relay_open::Target::Ssh(SshRelayTarget {}), - String::new(), - std::time::Duration::from_secs(15), - ) - .await - .map_err(|e| Status::unavailable(format!("supervisor relay failed: {e}")))?; + let (channel_id, relay_rx) = crate::sandbox_access::acquire(state, &sandbox) + .await? + .open_relay(relay_open::Target::Ssh(SshRelayTarget {}), String::new()) + .await?; let command_str = build_remote_exec_command(&req) .map_err(|e| Status::invalid_argument(format!("command construction failed: {e}")))?; @@ -2874,23 +2857,14 @@ pub(super) async fn handle_forward_tcp( .await?; // 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")); - } + // its transport. The access rules keep a Completed or Error sandbox + // reachable while its supervisor is connected, so fast commands attach + // without a readiness race. + let access = crate::sandbox_access::acquire(state, &sandbox).await?; let connection_guard = acquire_forward_connection_guard(state, &principal, &init, &sandbox).await?; - let (channel_id, relay_rx) = crate::supervisor_session::open_routed_relay_with_target( - state, - sandbox.object_id(), - target, - init.service_id.clone(), - std::time::Duration::from_secs(15), - ) - .await - .map_err(|e| Status::unavailable(format!("supervisor relay failed: {e}")))?; + let (channel_id, relay_rx) = access.open_relay(target, init.service_id.clone()).await?; let (tx, rx) = mpsc::channel::>(256); let sandbox_id = sandbox.object_id().to_string(); @@ -3321,17 +3295,10 @@ pub(super) async fn handle_exec_sandbox_interactive_start( completion.ensure_target(sandbox.object_id())?; } - require_ready_sandbox(&sandbox)?; - - let (channel_id, relay_rx) = crate::supervisor_session::open_routed_relay_with_target( - state, - sandbox.object_id(), - relay_open::Target::Ssh(SshRelayTarget {}), - String::new(), - std::time::Duration::from_secs(15), - ) - .await - .map_err(|e| Status::unavailable(format!("supervisor relay failed: {e}")))?; + let (channel_id, relay_rx) = crate::sandbox_access::acquire(state, &sandbox) + .await? + .open_relay(relay_open::Target::Ssh(SshRelayTarget {}), String::new()) + .await?; let command_str = build_remote_exec_command(req) .map_err(|e| Status::invalid_argument(format!("command construction failed: {e}")))?; @@ -3389,13 +3356,6 @@ 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())) -} - pub(super) async fn handle_create_ssh_session( state: &Arc, request: Request, @@ -3412,9 +3372,9 @@ 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")); - } + // The session's transport opens its relay through ForwardTcp, which + // checks access again; checking here refuses tokens that could never work. + crate::sandbox_access::acquire(state, &sandbox).await?; let token = uuid::Uuid::new_v4().to_string(); let now_ms = current_time_ms(); @@ -9488,10 +9448,17 @@ mod tests { .supervisor_sessions .finalize_main_process_exit("sandbox-work") ); - assert!(sandbox_relay_reachable(&state, &sandbox)); + assert!( + crate::sandbox_access::acquire(&state, &sandbox) + .await + .is_ok() + ); assert!(state.supervisor_sessions.disconnect("sandbox-work")); - assert!(!sandbox_relay_reachable(&state, &sandbox)); + assert!(matches!( + crate::sandbox_access::acquire(&state, &sandbox).await, + Err(crate::sandbox_access::AccessError::NotReachable) + )); } #[tokio::test] diff --git a/crates/openshell-server/src/lib.rs b/crates/openshell-server/src/lib.rs index 02c1ba16b3..25afec34cd 100644 --- a/crates/openshell-server/src/lib.rs +++ b/crates/openshell-server/src/lib.rs @@ -44,6 +44,7 @@ mod readiness; mod reflection; mod refresh_commit; mod refresh_store; +mod sandbox_access; mod sandbox_index; mod sandbox_watch; mod service_routing; diff --git a/crates/openshell-server/src/sandbox_access.rs b/crates/openshell-server/src/sandbox_access.rs new file mode 100644 index 0000000000..85230ebc2f --- /dev/null +++ b/crates/openshell-server/src/sandbox_access.rs @@ -0,0 +1,257 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! The one gate in front of every way into a sandbox. +//! +//! Exec, SSH sessions (and the file transfer that rides on them), port +//! forwarding, and exposed services all reach a sandbox through a relay its +//! supervisor opens. They must agree on when a sandbox can be used, so each of +//! them calls [`acquire`] and opens its relays through the returned +//! [`SandboxAccess`]. Anything that has to happen before a sandbox can be used +//! belongs in [`acquire`], so every entry point picks it up together. +//! +//! The rules: +//! +//! - A `Ready` sandbox is usable. Opening a relay waits for its supervisor and +//! routes to whichever replica holds it. +//! - A `Completed` or `Error` sandbox is usable while its supervisor is still +//! connected to any replica. A retained sandbox keeps its access plane after +//! its main process exits. +//! - Exposed services, which do not authenticate callers, reach an `Error` +//! sandbox only when its main process failed. An infrastructure failure +//! stays reachable for debugging through the authenticated paths only. +//! - Every other phase is refused. + +use std::fmt; +use std::sync::Arc; +use std::time::Duration; + +use openshell_core::ObjectId; +use openshell_core::proto::{Sandbox, SandboxPhase, relay_open}; +use tokio::io::DuplexStream; +use tokio::sync::oneshot; +use tonic::Status; + +use crate::ServerState; + +/// How long opening a relay waits for the supervisor session: enough to cover +/// a transient supervisor reconnect while still failing quickly. +const RELAY_SESSION_WAIT: Duration = Duration::from_secs(15); + +/// Why a sandbox could not be used. +#[derive(Debug)] +pub enum AccessError { + /// The sandbox is in a state that does not allow access. + NotReachable, + /// The gateway could not determine whether the sandbox is reachable. + Lookup(Status), +} + +impl fmt::Display for AccessError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::NotReachable => f.write_str("sandbox is not ready"), + Self::Lookup(status) => write!(f, "sandbox access check failed: {status}"), + } + } +} + +impl From for Status { + fn from(err: AccessError) -> Self { + match err { + AccessError::NotReachable => Self::failed_precondition(err.to_string()), + AccessError::Lookup(status) => status, + } + } +} + +/// A sandbox that passed the access rules. Relays into it are opened here. +pub struct SandboxAccess { + state: Arc, + sandbox_id: String, +} + +/// Apply the access rules to `sandbox`. +pub async fn acquire( + state: &Arc, + sandbox: &Sandbox, +) -> Result { + let reachable = match SandboxPhase::try_from(sandbox.phase()).ok() { + Some(SandboxPhase::Ready) => true, + Some(SandboxPhase::Completed | SandboxPhase::Error) => { + crate::supervisor_session::supervisor_connected(state, sandbox.object_id()) + .await + .map_err(AccessError::Lookup)? + } + _ => false, + }; + if !reachable { + return Err(AccessError::NotReachable); + } + Ok(SandboxAccess { + state: state.clone(), + sandbox_id: sandbox.object_id().to_string(), + }) +} + +/// Apply the access rules for an exposed service, which serves callers the +/// gateway does not authenticate. +pub async fn acquire_for_exposed_service( + state: &Arc, + sandbox: &Sandbox, +) -> Result { + if sandbox.phase() == SandboxPhase::Error as i32 + && !crate::compute::is_failed_main_process_result(sandbox) + { + return Err(AccessError::NotReachable); + } + acquire(state, sandbox).await +} + +impl SandboxAccess { + /// Ask the supervisor to open a relay to `target`, returning the channel + /// id and a receiver that resolves once the supervisor claims it. + pub async fn open_relay( + &self, + target: relay_open::Target, + service_id: String, + ) -> Result<(String, oneshot::Receiver>), Status> { + crate::supervisor_session::open_routed_relay_with_target( + &self.state, + &self.sandbox_id, + target, + service_id, + RELAY_SESSION_WAIT, + ) + .await + .map_err(|err| Status::unavailable(format!("supervisor relay failed: {err}"))) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use tokio::sync::mpsc; + + fn sandbox_in(id: &str, phase: SandboxPhase) -> Sandbox { + let mut sandbox = Sandbox { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: id.to_string(), + name: id.to_string(), + workspace: "default".to_string(), + ..Default::default() + }), + ..Default::default() + }; + sandbox.set_phase(phase as i32); + sandbox + } + + #[tokio::test] + async fn access_follows_phase_and_supervisor_session() { + let state = crate::grpc::test_support::test_server_state().await; + let reachable = |phase: SandboxPhase| { + let state = state.clone(); + async move { + match acquire(&state, &sandbox_in("access", phase)).await { + Ok(_) => true, + Err(AccessError::NotReachable) => false, + Err(err) => panic!("unexpected access error: {err:?}"), + } + } + }; + + // Without a supervisor, only Ready is usable; relay opening waits for + // its supervisor. + assert!(reachable(SandboxPhase::Ready).await); + for phase in [ + SandboxPhase::Provisioning, + SandboxPhase::Completed, + SandboxPhase::Error, + SandboxPhase::Stopped, + SandboxPhase::Deleting, + ] { + assert!(!reachable(phase).await, "{phase:?}"); + } + + // A connected supervisor keeps Completed and Error sandboxes usable, + // but never revives one that is not running. + let (tx, _rx) = mpsc::channel(1); + let (shutdown_tx, _shutdown_rx) = oneshot::channel(); + state.supervisor_sessions.register( + "access".to_string(), + "session".to_string(), + tx, + shutdown_tx, + ); + for phase in [ + SandboxPhase::Ready, + SandboxPhase::Completed, + SandboxPhase::Error, + ] { + assert!(reachable(phase).await, "{phase:?}"); + } + for phase in [ + SandboxPhase::Provisioning, + SandboxPhase::Stopped, + SandboxPhase::Deleting, + ] { + assert!(!reachable(phase).await, "{phase:?}"); + } + } + + fn error_sandbox(id: &str, reason: &str, exit_code: Option) -> Sandbox { + let mut sandbox = sandbox_in(id, SandboxPhase::Error); + sandbox.status = Some(openshell_core::proto::SandboxStatus { + phase: SandboxPhase::Error.into(), + exit_code, + conditions: vec![openshell_core::proto::SandboxCondition { + r#type: "Ready".into(), + status: "False".into(), + reason: reason.into(), + ..Default::default() + }], + ..Default::default() + }); + sandbox + } + + #[tokio::test] + async fn exposed_services_skip_infrastructure_errors() { + let state = crate::grpc::test_support::test_server_state().await; + let (tx, _rx) = mpsc::channel(1); + let (shutdown_tx, _shutdown_rx) = oneshot::channel(); + state.supervisor_sessions.register( + "access".to_string(), + "session".to_string(), + tx, + shutdown_tx, + ); + let main_failed = error_sandbox("access", "MainProcessFailed", Some(3)); + let timed_out = error_sandbox("access", "ProvisioningTimedOut", None); + + assert!( + acquire_for_exposed_service(&state, &main_failed) + .await + .is_ok() + ); + assert!(matches!( + acquire_for_exposed_service(&state, &timed_out).await, + Err(AccessError::NotReachable) + )); + // Authenticated paths keep the debugging shell. + assert!(acquire(&state, &timed_out).await.is_ok()); + assert!( + acquire_for_exposed_service(&state, &sandbox_in("access", SandboxPhase::Completed)) + .await + .is_ok() + ); + } + + #[test] + fn not_reachable_keeps_the_existing_status() { + let status = Status::from(AccessError::NotReachable); + assert_eq!(status.code(), tonic::Code::FailedPrecondition); + assert_eq!(status.message(), "sandbox is not ready"); + } +} diff --git a/crates/openshell-server/src/service_routing.rs b/crates/openshell-server/src/service_routing.rs index ece2add1e0..b20843b35d 100644 --- a/crates/openshell-server/src/service_routing.rs +++ b/crates/openshell-server/src/service_routing.rs @@ -11,7 +11,7 @@ use http::{HeaderMap, HeaderValue, Method, Request, Response, StatusCode, header use hyper_util::rt::TokioIo; use openshell_core::config::ServiceRoutingConfig; use openshell_core::proto::{ - Sandbox, SandboxPhase, ServiceAuthorizationMode, ServiceEndpoint, TcpRelayTarget, relay_open, + Sandbox, ServiceAuthorizationMode, ServiceEndpoint, TcpRelayTarget, relay_open, }; use openshell_core::{ObjectId, ObjectWorkspace}; use openshell_ocsf::{ @@ -28,6 +28,7 @@ use tracing::warn; use crate::ServerState; use crate::persistence::{ObjectListQuery, ObjectType, Store}; +use crate::sandbox_access::{self, AccessError, SandboxAccess}; const ENDPOINT_OBJECT_TYPE: &str = "service_endpoint"; const ROUTING_RULE_NAME: &str = "sandbox_service_routing"; @@ -727,18 +728,29 @@ async fn proxy_to_endpoint( return Err(route_err); } }; - if SandboxPhase::try_from(sandbox.phase()).ok() != Some(SandboxPhase::Ready) { - let err = ServiceRouteError::sandbox_not_ready(); - emit_service_http_failure( - &state, - &req, - &sandbox_name, - &service_name, - Some(&endpoint), - &err, - ); - return Err(err); - } + // Every request passes the access gate, including ones served from a + // pooled upstream connection. + let access = match sandbox_access::acquire_for_exposed_service(&state, &sandbox).await { + Ok(access) => access, + Err(err) => { + let route_err = match err { + AccessError::NotReachable => ServiceRouteError::sandbox_not_ready(), + AccessError::Lookup(err) => { + warn!(error = %err, sandbox_id = %endpoint.sandbox_id, "sandbox service routing: failed to check sandbox access"); + ServiceRouteError::internal_error() + } + }; + emit_service_http_failure( + &state, + &req, + &sandbox_name, + &service_name, + Some(&endpoint), + &route_err, + ); + return Err(route_err); + } + }; let Ok(target_port) = u16::try_from(endpoint.target_port) else { let err = ServiceRouteError::endpoint_unavailable(); emit_service_http_failure( @@ -791,7 +803,7 @@ async fn proxy_to_endpoint( let reused = pooled.is_some(); let mut sender = match pooled { Some(sender) => sender, - None => open_upstream(&state, &sandbox, &endpoint, target_port, websocket_upgrade).await?, + None => open_upstream(&access, &endpoint, target_port, websocket_upgrade).await?, }; let upstream = build_upstream_request(req, target_port, websocket_upgrade, authorization_mode)?; @@ -817,8 +829,7 @@ async fn proxy_to_endpoint( emit_service_relay_failure(&endpoint, target_port, route_err.reason); return Err(route_err); }; - sender = - open_upstream(&state, &sandbox, &endpoint, target_port, websocket_upgrade).await?; + sender = open_upstream(&access, &endpoint, target_port, websocket_upgrade).await?; sender.send_request(retry_request).await.map_err(|err| { warn!(error = %err, "sandbox service routing: upstream HTTP retry failed"); let route_err = ServiceRouteError::service_unreachable(); @@ -866,23 +877,20 @@ async fn proxy_to_endpoint( } async fn open_upstream( - state: &Arc, - sandbox: &Sandbox, + access: &SandboxAccess, endpoint: &ServiceEndpoint, target_port: u16, websocket_upgrade: bool, ) -> Result { - let (_channel_id, relay_rx) = crate::supervisor_session::open_routed_relay_with_target( - state, - sandbox.object_id(), - relay_open::Target::Tcp(TcpRelayTarget { - host: RELAY_TARGET_HOST.to_string(), - port: u32::from(target_port), - }), - endpoint.object_id().to_string(), - Duration::from_secs(15), - ) - .await + let (_channel_id, relay_rx) = access + .open_relay( + relay_open::Target::Tcp(TcpRelayTarget { + host: RELAY_TARGET_HOST.to_string(), + port: u32::from(target_port), + }), + endpoint.object_id().to_string(), + ) + .await .map_err(|err| { warn!(error = %err, sandbox_id = %endpoint.sandbox_id, "sandbox service routing: supervisor relay unavailable"); let route_err = ServiceRouteError::service_unreachable(); @@ -2271,6 +2279,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 9520fc106a..fcbbcda704 100644 --- a/crates/openshell-server/src/supervisor_session.rs +++ b/crates/openshell-server/src/supervisor_session.rs @@ -1613,7 +1613,11 @@ pub(crate) async fn forward_provider_status_query_to_owner( }) } -pub async fn open_routed_relay_with_target( +/// Open a relay into a sandbox wherever its supervisor is connected. +/// +/// Entry points open relays through `sandbox_access::SandboxAccess`, which +/// applies the access rules first. +pub(crate) async fn open_routed_relay_with_target( state: &Arc, sandbox_id: &str, target: relay_open::Target, @@ -1825,6 +1829,33 @@ fn owner_is_fresh(owner: &crate::supervisor_owner::OwnerRecord) -> bool { owner.is_fresh(OWNER_TTL) } +/// Whether this sandbox's supervisor is connected to any replica. +/// +/// A peer replica counts only through a fresh owner record. An owner record +/// naming this replica without a local session is a leftover from a lost +/// session. +pub async fn supervisor_connected( + state: &Arc, + sandbox_id: &str, +) -> Result { + if state.supervisor_sessions.has_session(sandbox_id) { + return Ok(true); + } + if state.store.is_single_replica() { + return Ok(false); + } + let owner_index = SupervisorOwnerIndex::new(state.store.clone(), OWNER_TTL); + let owner = resolve_owner(state, &owner_index, sandbox_id).await?; + Ok(peer_holds_session(&state.replica_id, owner.as_ref())) +} + +fn peer_holds_session( + self_replica_id: &str, + owner: Option<&crate::supervisor_owner::OwnerRecord>, +) -> bool { + owner.is_some_and(|owner| owner_is_fresh(owner) && owner.owner_replica_id != self_replica_id) +} + /// 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() { @@ -3719,6 +3750,59 @@ mod tests { .expect("complete chart peer TLS materials should configure tonic"); } + #[tokio::test] + async fn supervisor_connected_follows_the_local_session() { + let state = crate::grpc::test_support::test_server_state().await; + let connected = |sandbox_id: &'static str| { + let state = state.clone(); + async move { supervisor_connected(&state, sandbox_id).await.unwrap() } + }; + + assert!(!connected("relay-none").await); + + 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!(connected("relay-local").await); + assert!(state.supervisor_sessions.disconnect("relay-local")); + assert!(!connected("relay-local").await); + + // A single replica holds every session itself, so an owner record + // naming a peer is a leftover and is not read. + assert!(state.store.is_single_replica()); + SupervisorOwnerIndex::new(state.store.clone(), OWNER_TTL) + .publish( + "relay-peer", + "session", + "supervisor", + 1, + "peer-replica", + "https://peer", + ) + .await + .unwrap(); + assert!(!connected("relay-peer").await); + } + + #[test] + fn peer_holds_session_only_through_a_fresh_peer_owner_record() { + assert!(peer_holds_session("gw-0", Some(&owner_record("gw-1")))); + assert!(!peer_holds_session("gw-0", None)); + + let mut stale = owner_record("gw-1"); + stale.updated_at_ms = 0; + assert!(!peer_holds_session("gw-0", Some(&stale))); + + // An owner record naming this replica without a local session is a + // leftover from a lost session, not a live supervisor. + assert!(!peer_holds_session("gw-0", Some(&owner_record("gw-0")))); + } + 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 2ac6a4f69d..16af889067 100644 --- a/docs/how-it-works/sandboxes/overview.mdx +++ b/docs/how-it-works/sandboxes/overview.mdx @@ -1216,6 +1216,35 @@ 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. + +An `Error` caused by an infrastructure failure, such as +`ImagePreparationTimedOut`, usually has no supervisor left, so these +operations fail with `sandbox is not ready`. If the supervisor survived, exec, +file transfer, and port forwarding still work for debugging, but exposed +services stay unavailable. + ## 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/sdk/typescript/src/client.test.ts b/sdk/typescript/src/client.test.ts index 86fdfa3972..a32ecfbb79 100644 --- a/sdk/typescript/src/client.test.ts +++ b/sdk/typescript/src/client.test.ts @@ -2236,18 +2236,34 @@ describe('forward', () => { expect(revokedToken).toBe('fwd-tok'); }); - it('rejects when the sandbox is not ready', async () => { + it('rejects when the sandbox is stopped', async () => { const sandbox = client({ getSandbox: () => ({ sandbox: { metadata: { id: 'sb-id', name: 'sb' }, - status: { phase: SandboxPhase.PROVISIONING }, + status: { phase: SandboxPhase.STOPPED }, }, }), }); await expect(sandbox.forward('sb', { targetPort: 9000 })).rejects.toMatchObject({ code: 'connect' }); }); + it('leaves a completed sandbox to the gateway', async () => { + // A retained sandbox keeps its access plane after the main process exits; + // the gateway checks each forwarded connection. + const sandbox = client({ + getSandbox: () => ({ + sandbox: { + metadata: { id: 'sb-id', name: 'sb' }, + status: { phase: SandboxPhase.COMPLETED, exitCode: 0 }, + }, + }), + }); + const handle = await sandbox.forward('sb', { targetPort: 9000 }); + await handle.close(); + await handle.closed; + }); + // Backpressure (fix #6): the sandbox->local relay must stop pulling gRPC // frames when socket.write() returns false and resume after 'drain', so a // slow local reader cannot make Node buffer sandbox output without bound. diff --git a/sdk/typescript/src/client.ts b/sdk/typescript/src/client.ts index 205796f1bd..65ccdbbf66 100644 --- a/sdk/typescript/src/client.ts +++ b/sdk/typescript/src/client.ts @@ -1731,7 +1731,9 @@ export class SandboxClient { } // Bind a local TCP listener that tunnels each accepted connection into the - // sandbox. Mirrors the CLI service forward: READY check, then per socket mint + // sandbox. Mirrors the CLI service forward: refuse a sandbox that is stopping, + // stopped, or being deleted and leave every other phase to the gateway, which + // checks each connection. Then per socket mint // a short-lived SSH session token, open a forwardTcp bidi whose first frame is // the `init` (TCP target + token), relay bytes both ways in ~64 KiB chunks, // and revoke the token on close. Process-lifetime only. @@ -1749,8 +1751,8 @@ export class SandboxClient { ...(opts.signal ? { signal: opts.signal } : {}), }); const ref = found.ref; - if (ref.phase !== 'ready') { - throw new SdkError('connect', `sandbox '${name}' is not ready (phase: ${ref.phase})`); + if (ref.phase === 'stopping' || ref.phase === 'stopped' || ref.phase === 'deleting') { + throw new SdkError('connect', `sandbox '${name}' is not available (phase: ${ref.phase})`); } sandboxId = ref.id; owner = found.owner; diff --git a/skills/openshell-cli/SKILL.md b/skills/openshell-cli/SKILL.md index 351d7e3b84..bbf0b5bd52 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/README.md b/tests/suites/conformance/README.md index fe5c95b5f8..7f2d3846b1 100644 --- a/tests/suites/conformance/README.md +++ b/tests/suites/conformance/README.md @@ -10,7 +10,7 @@ nextest run the tests as the library's unit tests. The crate also builds the |---|---| | `smoke` | Create a sandbox, run `echo` in it, delete it | | `lifecycle` | SDK: long-running and run-to-completion sandboxes, the default command, restart of a stopped sandbox, deleting a stopped sandbox | -| `cli` | The same lifecycle through the `openshell` CLI: status, create, get, list, exec, stop, start, delete | +| `cli` | The same lifecycle through the `openshell` CLI: status, create, get, list, exec, stop, start, delete; `--no-keep` sandboxes are deleted after the main process exits, and retained ones still accept exec | | `persistent-workspace` | Files in `/sandbox` survive stop and start | | `policy-advisor` | `policy.local` accepts a proposed rule; a denied address or hostname yields one draft rule | | `file-transfer` | `sandbox upload` and `download`: round trips, `.gitignore` filtering, workspace-escape and option-looking names | diff --git a/tests/suites/conformance/src/cli.rs b/tests/suites/conformance/src/cli.rs index d26eed9bb5..54ddbf92e5 100644 --- a/tests/suites/conformance/src/cli.rs +++ b/tests/suites/conformance/src/cli.rs @@ -111,6 +111,116 @@ async fn cli_stop_start_delete(cx: &mut Context) -> testkit::Result { cx.expect_absent(&sandbox).await } +/// An ephemeral (`--no-keep`) sandbox streams its main process output, returns +/// the main process's exit code, and is deleted afterwards, whether the main +/// process succeeds or fails. +#[testkit::test] +async fn cli_ephemeral_sandbox_is_deleted_after_exit(cx: &mut Context) -> testkit::Result { + for exit_code in [0, 3] { + let name = cx.name(&format!("e{exit_code}")); + cx.track(&name); + let create = cx + .cli([ + "sandbox", + "create", + "--name", + &name, + "--no-keep", + "--no-tty", + "--", + "sh", + "-c", + &format!("echo ephemeral-done; exit {exit_code}"), + ]) + .await?; + testkit::ensure!( + create.exit_code == exit_code, + "sandbox create exited {}; expected {exit_code}; stderr: {}", + create.exit_code, + create.stderr + ); + testkit::ensure!( + create.stdout.contains("ephemeral-done"), + "sandbox create stdout {:?} lacks the main process output", + create.stdout + ); + + wait_unlisted(cx, &name).await?; + let get = cx.cli(["sandbox", "get", &name]).await?.failure()?; + testkit::ensure!( + get.stderr.to_lowercase().contains("not found"), + "sandbox get {name} failed without reporting it missing: {}", + get.stderr + ); + } + Ok(()) +} + +/// A retained sandbox records Completed or Error with the main process's exit +/// code, and still accepts `sandbox exec` after the main process has exited. +#[testkit::test] +async fn cli_retained_sandbox_accepts_exec_after_exit(cx: &mut Context) -> testkit::Result { + let result_path = "/sandbox/.openshell-retained-result"; + for exit_code in [0, 3] { + let name = cx.name(&format!("r{exit_code}")); + let result = format!("openshell-retained-{}-{exit_code}", cx.run_id()); + cx.track(&name); + cx.cli([ + "sandbox", + "create", + "--name", + &name, + "--detach", + "--no-tty", + "--", + "sh", + "-c", + &format!("printf '%s\\n' '{result}' > '{result_path}' && sync; exit {exit_code}"), + ]) + .await? + .success()?; + let sandbox = cx.adopt(&name).await?; + + let finished = sandbox.wait_finished().await?; + let expected = if exit_code == 0 { + testkit::sdk::SandboxPhase::Completed + } else { + testkit::sdk::SandboxPhase::Error + }; + testkit::ensure!( + finished.phase == expected && finished.exit_code == Some(exit_code), + "sandbox {name} finished in phase {:?} with exit code {:?}; expected {expected:?} with {exit_code}", + finished.phase, + finished.exit_code + ); + + let exec = cx + .cli([ + "sandbox", + "exec", + "--name", + &name, + "--no-tty", + "--", + "cat", + result_path, + ]) + .await? + .success()?; + testkit::ensure!( + exec.stdout == format!("{result}\n"), + "sandbox exec stdout was {:?}; expected {:?}", + exec.stdout, + format!("{result}\n") + ); + + cx.cli(["sandbox", "delete", &name]).await?.success()?; + wait_unlisted(cx, &name).await?; + cx.expect_absent(&sandbox).await?; + } + Ok(()) +} + /// Wait until `sandbox list` no longer shows the sandbox. async fn wait_unlisted(cx: &Context, name: &str) -> testkit::Result { testkit::poll_until(