diff --git a/.agents/skills/openshell-cli/SKILL.md b/.agents/skills/openshell-cli/SKILL.md index 8c8ba6f546..9ee73cda2b 100644 --- a/.agents/skills/openshell-cli/SKILL.md +++ b/.agents/skills/openshell-cli/SKILL.md @@ -74,6 +74,12 @@ This creates a sandbox whose canonical main process is `/bin/bash -l` and attaches your terminal to that retained process. Add `--detach` to return after the sandbox becomes ready without attaching. +An explicit trailing command is foreground even when stdin or stdout is not a +terminal. The CLI streams its stdout and stderr and returns its exact exit +status. Exit code 0 leaves a retained sandbox in `Completed`; nonzero leaves it +in `Error` with `MainProcessFailed`. Use `--no-keep` to delete either result +after output drains, or `--detach` for a long-running service. + When supplying `--name`, use a portable DNS-1123 label: at most 63 lowercase alphanumeric or `-` characters, beginning and ending with an alphanumeric character. The Kubernetes driver rejects uppercase letters, underscores, dots, and other names that cannot become Kubernetes resource labels. **Shortcut for known tools**: When the trailing command is a recognized tool, the CLI auto-creates the required provider from local credentials: @@ -236,7 +242,7 @@ Key flags: - `--approval-mode manual|auto`: Control handling of agent-authored policy proposals; `manual` is the default - `--upload [:]`: Upload local files into the container working directory or an explicit destination - `--no-git-ignore`: Disable `.gitignore` filtering for uploads -- `--no-keep`: Delete the sandbox after the initial command or shell exits +- `--no-keep`: Delete the sandbox after main output and the exit result drain - `--detach`: Start the canonical main process without attaching - `--forward [BIND_ADDRESS:]PORT`: Forward a local port and keep the sandbox alive - `--editor vscode|cursor`: Open a remote editor after creation and keep the sandbox alive @@ -351,7 +357,9 @@ 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. Delete remains the operation that removes retained state. +stopped or completed. Starting a retained `Completed` or +`Error/MainProcessFailed` sandbox launches a fresh canonical-main instance. +Delete remains the operation that removes retained state. --- diff --git a/.agents/skills/openshell-cli/cli-reference.md b/.agents/skills/openshell-cli/cli-reference.md index 61beab95f1..759dbd8838 100644 --- a/.agents/skills/openshell-cli/cli-reference.md +++ b/.agents/skills/openshell-cli/cli-reference.md @@ -207,13 +207,16 @@ identity provider. Requires an authenticated gateway connection. Create a sandbox through the selected gateway and launch its canonical main process. By default, the CLI attaches to that retained process after the sandbox becomes ready. A trailing command defines the canonical main process; -without one, the default is `/bin/bash -l` with a PTY. +without one, the default is `/bin/bash -l` with a PTY. Explicit commands remain +foreground in non-interactive automation: stdout and stderr stream to the +caller and the CLI returns the command's exact status. Exit 0 leaves +`Completed`; nonzero leaves `Error/MainProcessFailed`. | Flag | Description | |------|-------------| | `--name ` | Sandbox name (auto-generated if omitted) | | `--from ` | Community name, Dockerfile path, directory, or image reference (BYOC) | -| `--no-keep` | Delete the sandbox after the initial command or shell exits | +| `--no-keep` | Delete the sandbox after main output and the result drain | | `--detach` | Start the canonical main process without attaching | | `--editor vscode|cursor` | Launch a remote editor and keep the sandbox alive | | `--gpu [COUNT]` | Request the driver's default GPU selection or a specific count | @@ -269,8 +272,9 @@ and waits for the `Stopped` phase. ### `openshell sandbox start [name]` -Start a stopped sandbox and wait for `Ready`. The name defaults to the -last-used sandbox. +Start a stopped, failed, or completed sandbox and wait for `Ready`. This +launches a fresh canonical-main instance. The name defaults to the last-used +sandbox. ### `openshell sandbox exec [OPTIONS] -- COMMAND...` diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index 2a36073486..30dacc140c 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -137,6 +137,12 @@ The gateway persists lifecycle intent before mutating compute: Ready -> Stopping -> Stopped -> Starting -> Ready ``` +A canonical main process that exits successfully follows `Ready -> Completed`. +A nonzero or signal-normalized result follows `Ready -> Error` with a +`MainProcessFailed` condition. Both retained results may be started explicitly, +which creates a fresh main-process instance. Drivers must not automatically +restart a completed or failed canonical process. + `StopSandbox` and `StartSandbox` are idempotent driver operations. Stop retains the driver resource and its persistent workspace boundary while making exec, SSH, forwarding, and exposed services unavailable. Start reactivates the diff --git a/architecture/gateway.md b/architecture/gateway.md index 32bca6a1f6..8d01ef297c 100644 --- a/architecture/gateway.md +++ b/architecture/gateway.md @@ -16,8 +16,10 @@ workloads. - Coordinate supervisor relay sessions for connect, exec, file sync, and service forwarding. - Persist the canonical main-process instance ID and normalized exit code on - sandbox status. Any main process exit transitions the sandbox to `Error`, - including exit code zero. + sandbox status. Exit code zero transitions the sandbox to `Completed`; + nonzero results transition it to `Error/MainProcessFailed`. Infrastructure + failures also use `Error`, with a distinct reason and no fabricated command + result. The gateway does not enforce agent network policy at request time. That happens inside each sandbox, where the supervisor and proxy can observe local process @@ -26,7 +28,9 @@ identity. The live supervisor session is the readiness authority for its main-process instance. The supervisor reports its normalized result through the sandbox-authenticated `ReportMainProcessExit` RPC, and the gateway rejects -results from stale instance IDs. +results from stale instance IDs. The process supervisor keeps the main SSH +session alive until an attached foreground client receives the terminal result +or a bounded detached timeout expires, then reports the result and closes. ## Protocol and Auth diff --git a/architecture/sandbox.md b/architecture/sandbox.md index 6274f4b2bf..e8adea7daf 100644 --- a/architecture/sandbox.md +++ b/architecture/sandbox.md @@ -446,7 +446,9 @@ engine with a gateway policy revision. re-evaluate. - If the supervisor relay drops, the sandbox can keep running, but connect and exec operations fail until the supervisor registers again. -- If the canonical main process exits, including with code 0, the supervisor - reports its normalized exit code before shutdown. The gateway persists the - code on sandbox status, records `MainProcessExited`, and makes the sandbox - terminal `Error`; runtime restart policies must not replace the process. +- If the canonical main process exits, the supervisor drains its retained main + output and reports the normalized result before shutdown. Exit code 0 records + `Completed/MainProcessCompleted`; nonzero and signal-normalized exits record + `Error/MainProcessFailed`. Infrastructure failures also use `Error`, with a + distinct condition reason and no fabricated canonical-process result. Runtime + restart policies must not replace the canonical process. diff --git a/crates/openshell-cli/src/commands/common.rs b/crates/openshell-cli/src/commands/common.rs index 75dc260ba7..2af950cd01 100644 --- a/crates/openshell-cli/src/commands/common.rs +++ b/crates/openshell-cli/src/commands/common.rs @@ -65,6 +65,7 @@ pub fn phase_name(phase: i32) -> &'static str { Ok(SandboxPhase::Stopping) => "Stopping", Ok(SandboxPhase::Stopped) => "Stopped", Ok(SandboxPhase::Starting) => "Starting", + Ok(SandboxPhase::Completed) => "Completed", Ok(SandboxPhase::Unknown) | Err(_) => "Unknown", } } diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index de057b5f82..672d1e38d7 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -3063,7 +3063,7 @@ async fn run_async() -> Result<()> { let endpoint = &ctx.endpoint; let mut tls = tls.with_gateway_name(&ctx.name); apply_auth(&mut tls, &ctx.name); - Box::pin(run::sandbox_create( + let exit_code = Box::pin(run::sandbox_create( endpoint, &ctx.name, run::SandboxCreateConfig { @@ -3092,6 +3092,9 @@ async fn run_async() -> Result<()> { &tls, )) .await?; + if exit_code != 0 { + std::process::exit(exit_code); + } } SandboxCommands::Upload { name, @@ -3217,7 +3220,12 @@ async fn run_async() -> Result<()> { ) .await?; } else { - run::sandbox_connect(endpoint, &name, &tls, &cli.workspace).await?; + let exit_code = + run::sandbox_connect(endpoint, &name, &tls, &cli.workspace) + .await?; + if exit_code != 0 { + std::process::exit(exit_code); + } } let _ = save_last_sandbox(&ctx.name, &cli.workspace, &name); } diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index fd0e585068..8e75b6ec0b 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -337,19 +337,21 @@ async fn finalize_sandbox_create_session( server: &str, sandbox_name: &str, persist: bool, - session_result: Result<()>, + session_result: Result, workspace: &str, tls: &TlsOptions, gateway: &str, -) -> Result<()> { +) -> Result { if persist { return session_result; } let names = [sandbox_name.to_string()]; if let Err(err) = sandbox_delete(server, &names, false, workspace, tls, gateway).await { - if session_result.is_ok() { - return Err(err); + if let Ok(exit_code) = session_result.as_ref() { + return Err(miette::miette!( + "sandbox command exited with status {exit_code}, but ephemeral cleanup failed: {err}" + )); } eprintln!("Failed to delete sandbox {sandbox_name}: {err}"); } @@ -420,7 +422,7 @@ pub async fn sandbox_create( config: SandboxCreateConfig<'_>, workspace: &str, tls: &TlsOptions, -) -> Result<()> { +) -> Result { let SandboxCreateConfig { name, from, @@ -454,6 +456,11 @@ pub async fn sandbox_create( "--upload cannot be combined with a trailing main command yet because uploads complete after the canonical process starts" )); } + if output != "table" && !command.is_empty() && !detach { + return Err(miette::miette!( + "structured output cannot be combined with an attached trailing command; use table output to stream the command or add --detach" + )); + } // Check port availability *before* creating the sandbox so we don't // leave an orphaned sandbox behind when the forward would fail. @@ -536,6 +543,15 @@ pub async fn sandbox_create( } else { command.to_vec() }; + let persist = sandbox_should_persist(keep, forward.as_ref()); + let annotations = if persist { + HashMap::new() + } else { + HashMap::from([( + "openshell.nvidia.com/retention".to_string(), + "ephemeral".to_string(), + )]) + }; let request = CreateSandboxRequest { spec: Some(SandboxSpec { resource_requirements, @@ -549,7 +565,7 @@ pub async fn sandbox_create( }), name: name.unwrap_or_default().to_string(), labels, - annotations: HashMap::new(), + annotations, workspace: workspace.to_string(), }; @@ -569,7 +585,6 @@ pub async fn sandbox_create( .ok_or_else(|| miette::miette!("sandbox missing from response"))?; let interactive = std::io::stdout().is_terminal(); - let persist = sandbox_should_persist(keep, forward.as_ref()); let sandbox_name = if sandbox.object_name().is_empty() { "unknown".to_string() } else { @@ -746,8 +761,23 @@ pub async fn sandbox_create( saw_non_ready = true; } - // Capture error reason from conditions only when phase is Error - // to avoid showing stale transient error reasons + let has_main_process_result = s + .status + .as_ref() + .is_some_and(|status| status.exit_code.is_some()); + if matches!( + phase, + SandboxPhase::Completed | SandboxPhase::Error | SandboxPhase::Stopped + ) && has_main_process_result + { + if let Some(d) = display.as_interactive_mut() { + d.clear(); + } + break; + } + + // Capture infrastructure error reasons only after excluding a + // canonical-command result, which must attach and drain output. if phase == SandboxPhase::Error && let Some(status) = &s.status { @@ -826,7 +856,14 @@ pub async fn sandbox_create( // If we exited the loop without hitting the Ready break, finish the display. let final_phase = SandboxPhase::try_from(last_phase).unwrap_or(SandboxPhase::Unknown); - if final_phase != SandboxPhase::Ready + let final_has_main_process_result = last_sandbox + .status + .as_ref() + .is_some_and(|status| status.exit_code.is_some()); + if !(matches!( + final_phase, + SandboxPhase::Ready | SandboxPhase::Completed | SandboxPhase::Stopped + ) || final_phase == SandboxPhase::Error && final_has_main_process_result) && let Some(d) = display.as_interactive_mut() { if final_phase == SandboxPhase::Error { @@ -939,7 +976,7 @@ pub async fn sandbox_create( if structured_output { crate::output::print_output_single(output, &last_sandbox, sandbox_to_json)?; - return Ok(()); + return Ok(0); } if let Some(editor) = editor { @@ -953,18 +990,18 @@ pub async fn sandbox_create( workspace, ) .await?; - return Ok(()); + return Ok(0); } - // Persistent non-interactive creates detach implicitly. An - // explicitly ephemeral (`--no-keep`) create must still attach so - // it can observe the canonical process and delete the sandbox when - // that session ends. + // An explicit trailing command is foreground regardless of TTY + // detection. Only --detach opts out. Scratch shells retain the + // non-interactive implicit-detach behavior. if detach || (persist + && command.is_empty() && (!std::io::stdin().is_terminal() || !std::io::stdout().is_terminal())) { - return Ok(()); + return Ok(0); } let connect_result = if persist { @@ -990,6 +1027,32 @@ pub async fn sandbox_create( ) .await } + SandboxPhase::Completed | SandboxPhase::Stopped | SandboxPhase::Error + if final_has_main_process_result => + { + drop(stream); + drop(client); + if detach { + return Ok(0); + } + let connect_result = crate::ssh::sandbox_connect_without_exec( + &effective_server, + &sandbox_name, + &effective_tls, + workspace, + ) + .await; + finalize_sandbox_create_session( + &effective_server, + &sandbox_name, + persist, + connect_result, + workspace, + &effective_tls, + gateway_name, + ) + .await + } SandboxPhase::Error => { drop(stream); drop(client); @@ -1324,6 +1387,9 @@ pub async fn sandbox_get( println!(" {} {}", "Id:".dimmed(), id); println!(" {} {}", "Name:".dimmed(), name); println!(" {} {}", "Phase:".dimmed(), phase_name(sandbox.phase())); + if let Some(exit_code) = sandbox.status.as_ref().and_then(|status| status.exit_code) { + println!(" {} {}", "Exit Code:".dimmed(), exit_code); + } println!( " {} {}", "Resource version:".dimmed(), @@ -2057,8 +2123,16 @@ pub async fn sandbox_list( for sandbox in sandboxes { let phase = phase_name(sandbox.phase()); let phase_colored = match SandboxPhase::try_from(sandbox.phase()) { - Ok(SandboxPhase::Ready) => phase.green().to_string(), + Ok(SandboxPhase::Ready | SandboxPhase::Completed) => phase.green().to_string(), Ok(SandboxPhase::Error) => phase.red().to_string(), + Ok(SandboxPhase::Stopped) + if sandbox + .status + .as_ref() + .is_some_and(|status| status.exit_code.is_some()) => + { + phase.red().to_string() + } Ok(SandboxPhase::Provisioning) => phase.yellow().to_string(), Ok(SandboxPhase::Deleting) => phase.dimmed().to_string(), _ => phase.to_string(), @@ -2102,6 +2176,7 @@ fn sandbox_to_json(sandbox: &Sandbox) -> serde_json::Value { "created_at": format_epoch_ms(meta.map_or(0, |m| m.created_at_ms)), "phase": phase_name(sandbox.phase()), "current_policy_version": sandbox.current_policy_version(), + "exit_code": sandbox.status.as_ref().and_then(|status| status.exit_code), }) } @@ -2391,13 +2466,21 @@ pub async fn sandbox_delete( } } - let response = client + let response = match client .delete_sandbox(DeleteSandboxRequest { name: name.clone(), workspace: workspace.to_string(), }) .await - .into_diagnostic()?; + { + Ok(response) => response, + Err(status) if status.code() == Code::NotFound => { + clear_last_sandbox_if_matches(gateway, workspace, name); + println!("{} Sandbox {name} already deleted", "✓".green().bold()); + continue; + } + Err(status) => return Err(status).into_diagnostic(), + }; let deleted = response.into_inner().deleted; if deleted { @@ -2476,9 +2559,9 @@ async fn wait_for_lifecycle_phase( return Ok(sandbox); } if current == SandboxPhase::Error { - return Err(miette!( - "sandbox entered Error while waiting for {target:?}" - )); + let detail = ready_false_condition_message(sandbox.status.as_ref()) + .unwrap_or_else(|| "sandbox entered Error".to_string()); + return Err(miette!("{detail} while waiting for {target:?}")); } let timeout = Duration::from_secs( diff --git a/crates/openshell-cli/src/ssh.rs b/crates/openshell-cli/src/ssh.rs index 4768dc27f2..04c2d01312 100644 --- a/crates/openshell-cli/src/ssh.rs +++ b/crates/openshell-cli/src/ssh.rs @@ -229,7 +229,7 @@ fn reset_transient_tty_signals(command: &mut Command) { } } -fn exec_or_wait(mut command: Command, replace_process: bool) -> Result<()> { +fn exec_or_wait(mut command: Command, replace_process: bool) -> Result { if replace_process && std::io::stdin().is_terminal() { #[cfg(unix)] { @@ -248,11 +248,7 @@ fn exec_or_wait(mut command: Command, replace_process: bool) -> Result<()> { let status = command.status().into_diagnostic()?; - if !status.success() { - return Err(miette::miette!("ssh exited with status {status}")); - } - - Ok(()) + Ok(status.code().unwrap_or(1)) } async fn sandbox_connect_with_mode( @@ -261,7 +257,7 @@ async fn sandbox_connect_with_mode( tls: &TlsOptions, replace_process: bool, workspace: &str, -) -> Result<()> { +) -> Result { let session = ssh_session_config(server, name, tls, workspace).await?; let mut command = ssh_base_command(&session.proxy_command); @@ -280,11 +276,11 @@ async fn sandbox_connect_with_mode( .stdout(Stdio::inherit()) .stderr(Stdio::inherit()); - tokio::task::spawn_blocking(move || exec_or_wait(command, replace_process)) + let exit_code = tokio::task::spawn_blocking(move || exec_or_wait(command, replace_process)) .await .into_diagnostic()??; - Ok(()) + Ok(exit_code) } /// Connect to a sandbox via SSH. @@ -293,7 +289,7 @@ pub async fn sandbox_connect( name: &str, tls: &TlsOptions, workspace: &str, -) -> Result<()> { +) -> Result { sandbox_connect_with_mode(server, name, tls, true, workspace).await } @@ -302,7 +298,7 @@ pub(crate) async fn sandbox_connect_without_exec( name: &str, tls: &TlsOptions, workspace: &str, -) -> Result<()> { +) -> Result { sandbox_connect_with_mode(server, name, tls, false, workspace).await } diff --git a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs index c7ac19ce93..e69da4d02e 100644 --- a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs +++ b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs @@ -1651,6 +1651,14 @@ async fn sandbox_create_deletes_command_sessions_with_no_keep() { deleted_names(&server).await, vec![vec!["ephemeral-command".to_string()]] ); + let requests = create_requests(&server).await; + assert_eq!( + requests[0] + .annotations + .get("openshell.nvidia.com/retention") + .map(String::as_str), + Some("ephemeral") + ); assert_eq!( load_last_sandbox("openshell", "default"), None, @@ -1658,6 +1666,37 @@ async fn sandbox_create_deletes_command_sessions_with_no_keep() { ); } +#[tokio::test] +async fn sandbox_create_returns_exact_main_status_after_no_keep_cleanup() { + let server = run_server().await; + let fake_ssh_dir = tempfile::tempdir().unwrap(); + let xdg_dir = tempfile::tempdir().unwrap(); + let _env = test_env(&fake_ssh_dir, &xdg_dir); + let tls = test_tls(&server); + install_executable_script(&fake_ssh_dir, "ssh", "#!/bin/sh\nexit 7\n"); + + let exit_code = run::sandbox_create( + &server.endpoint, + "openshell", + run::SandboxCreateConfig { + name: Some("ephemeral-failure"), + keep: false, + command: &["sh".into(), "-c".into(), "exit 7".into()], + ..test_config() + }, + "default", + &tls, + ) + .await + .expect("a main-process failure is a command result, not a cleanup error"); + + assert_eq!(exit_code, 7); + assert_eq!( + deleted_names(&server).await, + vec![vec!["ephemeral-failure".to_string()]] + ); +} + #[tokio::test] async fn sandbox_create_deletes_shell_sessions_with_no_keep() { let server = run_server().await; diff --git a/crates/openshell-sdk/src/client.rs b/crates/openshell-sdk/src/client.rs index f95b7ee111..06368ea34d 100644 --- a/crates/openshell-sdk/src/client.rs +++ b/crates/openshell-sdk/src/client.rs @@ -246,15 +246,19 @@ impl OpenShellClient { } /// Poll [`OpenShellClient::get_sandbox`] until the sandbox reaches - /// [`SandboxPhase::Ready`] or the `timeout` elapses. + /// [`SandboxPhase::Ready`] or successful [`SandboxPhase::Completed`], or + /// until the `timeout` elapses. /// /// Returns the terminal sandbox snapshot on success. Returns an /// [`SdkError::Connect`] when the timeout expires, or whatever error /// the gateway returns if the sandbox transitions into - /// [`SandboxPhase::Error`]. + /// [`SandboxPhase::Stopped`] or [`SandboxPhase::Error`]. pub async fn wait_ready(&self, name: &str, timeout: Duration) -> Result { self.wait_for(name, timeout, |phase| match phase { - SandboxPhase::Ready => Some(Ok(())), + SandboxPhase::Ready | SandboxPhase::Completed => Some(Ok(())), + SandboxPhase::Stopped => Some(Err(SdkError::connect(format!( + "sandbox '{name}' main process failed" + )))), SandboxPhase::Error => Some(Err(SdkError::connect(format!( "sandbox '{name}' entered error phase" )))), @@ -644,15 +648,22 @@ impl WorkspaceScopedClient { sandbox_from_response(response.sandbox) } - /// Poll until the sandbox reaches [`SandboxPhase::Ready`] or the timeout - /// elapses. + /// Poll until the sandbox reaches [`SandboxPhase::Ready`] or successful + /// [`SandboxPhase::Completed`], or the timeout elapses. pub async fn wait_ready(&self, name: &str, timeout: Duration) -> Result { let deadline = Instant::now() + timeout; let mut delay = Duration::from_millis(250); loop { let snapshot = self.get_sandbox(name).await?; match snapshot.phase { - SandboxPhase::Ready => return Ok(snapshot), + SandboxPhase::Ready | SandboxPhase::Completed => return Ok(snapshot), + SandboxPhase::Stopped => { + let detail = snapshot.exit_code.map_or_else( + || "stopped before becoming ready".to_string(), + |code| format!("main process failed with status {code}"), + ); + return Err(SdkError::connect(format!("sandbox '{name}' {detail}"))); + } SandboxPhase::Error => { return Err(SdkError::connect(format!( "sandbox '{name}' entered error phase" diff --git a/crates/openshell-sdk/src/types.rs b/crates/openshell-sdk/src/types.rs index 3715c7fdd6..f7f1547115 100644 --- a/crates/openshell-sdk/src/types.rs +++ b/crates/openshell-sdk/src/types.rs @@ -65,6 +65,7 @@ pub enum SandboxPhase { Stopping, Stopped, Starting, + Completed, } impl From for SandboxPhase { @@ -79,6 +80,7 @@ impl From for SandboxPhase { proto::SandboxPhase::Stopping => Self::Stopping, proto::SandboxPhase::Stopped => Self::Stopped, proto::SandboxPhase::Starting => Self::Starting, + proto::SandboxPhase::Completed => Self::Completed, } } } diff --git a/crates/openshell-sdk/tests/client_mock.rs b/crates/openshell-sdk/tests/client_mock.rs index c5467b809d..1b49582467 100644 --- a/crates/openshell-sdk/tests/client_mock.rs +++ b/crates/openshell-sdk/tests/client_mock.rs @@ -901,6 +901,41 @@ async fn wait_ready_transitions_through_phases() { assert!(state.get_calls.load(Ordering::SeqCst) >= 3); } +#[tokio::test] +async fn wait_ready_accepts_successful_completion() { + let state = Arc::new(MockState { + phase_sequence: vec![ + proto::SandboxPhase::Provisioning, + proto::SandboxPhase::Completed, + ], + ..Default::default() + }); + let endpoint = start_mock(state).await; + let client = connect(&endpoint).await; + + let sandbox = client + .wait_ready("short-job", std::time::Duration::from_secs(5)) + .await + .unwrap(); + assert_eq!(sandbox.phase, SandboxPhase::Completed); +} + +#[tokio::test] +async fn wait_ready_surfaces_stopped_phase_without_timing_out() { + let state = Arc::new(MockState { + phase_sequence: vec![proto::SandboxPhase::Stopped], + ..Default::default() + }); + let endpoint = start_mock(state).await; + let client = connect(&endpoint).await; + + let err = client + .wait_ready("failed-job", std::time::Duration::from_secs(5)) + .await + .unwrap_err(); + assert_eq!(err.code(), "connect"); +} + #[tokio::test] async fn wait_ready_surfaces_error_phase() { let state = Arc::new(MockState { diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index 30a1303bd5..7f113a8504 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -1024,7 +1024,9 @@ impl ComputeRuntime { } let phase = SandboxPhase::try_from(current.phase()).unwrap_or(SandboxPhase::Unknown); - if phase == SandboxPhase::Stopped { + if matches!(phase, SandboxPhase::Stopped | SandboxPhase::Completed) + || is_failed_main_process_result(¤t) + { self.cleanup_stopped_sandbox_sessions(¤t) .await .map_err(Status::internal)?; @@ -1182,9 +1184,13 @@ impl ComputeRuntime { if phase == SandboxPhase::Ready { return Ok(current); } - if !matches!(phase, SandboxPhase::Stopped | SandboxPhase::Starting) { + if !matches!( + phase, + SandboxPhase::Stopped | SandboxPhase::Completed | SandboxPhase::Starting + ) && !is_failed_main_process_result(¤t) + { return Err(Status::failed_precondition(format!( - "sandbox must be Stopped to start (current phase: {phase:?})" + "sandbox must be Stopped, Completed, or a failed main-process Error to start (current phase: {phase:?})" ))); } @@ -2280,11 +2286,16 @@ impl ComputeRuntime { }; let phase = SandboxPhase::try_from(sandbox.phase()).unwrap_or(SandboxPhase::Unknown); match phase { - SandboxPhase::Stopped => { + SandboxPhase::Stopped | SandboxPhase::Completed => { if let Err(err) = self.cleanup_stopped_sandbox_sessions(&sandbox).await { warn!(sandbox_id = %sandbox.object_id(), error = %err, "Failed to complete recovered sandbox session cleanup"); } } + SandboxPhase::Error if is_failed_main_process_result(&sandbox) => { + if let Err(err) = self.cleanup_stopped_sandbox_sessions(&sandbox).await { + warn!(sandbox_id = %sandbox.object_id(), error = %err, "Failed to complete recovered failed-main session cleanup"); + } + } SandboxPhase::Stopping => { let sandbox_id = sandbox.object_id().to_string(); let sandbox_name = sandbox.object_name().to_string(); @@ -2835,6 +2846,7 @@ impl ComputeRuntime { | SandboxPhase::Error | SandboxPhase::Stopping | SandboxPhase::Stopped + | SandboxPhase::Completed ) { return Ok(()); } @@ -2886,15 +2898,15 @@ impl ComputeRuntime { Ok(()) } - /// Persist a terminal canonical-process result. Exit code zero is still a - /// sandbox error because the canonical process defines sandbox health. + /// Persist a terminal canonical-process result. Successful completion is + /// distinct from a nonzero command result and from infrastructure error. pub async fn main_process_exited( &self, sandbox_id: &str, instance_id: &str, exit_code: i32, ) -> Result<(), String> { - let _guard = self.sync_lock.lock().await; + let guard = self.sync_lock.lock().await; let Some(existing) = self .store .get_message::(sandbox_id) @@ -2906,7 +2918,10 @@ impl ComputeRuntime { let phase = SandboxPhase::try_from(existing.phase()).unwrap_or(SandboxPhase::Unknown); if matches!( phase, - SandboxPhase::Deleting | SandboxPhase::Stopping | SandboxPhase::Stopped + SandboxPhase::Deleting + | SandboxPhase::Stopping + | SandboxPhase::Stopped + | SandboxPhase::Completed ) { return Ok(()); } @@ -2949,6 +2964,14 @@ impl ComputeRuntime { } } let expected_resource_version = sandbox_resource_version(&existing); + let ephemeral = existing.metadata.as_ref().is_some_and(|metadata| { + metadata + .annotations + .get("openshell.nvidia.com/retention") + .is_some_and(|value| value == "ephemeral") + }); + let workspace = existing.object_workspace().to_string(); + let name = existing.object_name().to_string(); let sandbox = self .store .update_message_cas::(sandbox_id, expected_resource_version, |sandbox| { @@ -2958,6 +2981,19 @@ impl ComputeRuntime { .map_err(|error| error.to_string())?; self.sandbox_index.update_from_sandbox(&sandbox); self.sandbox_watch_bus.notify(sandbox_id); + drop(guard); + if ephemeral { + let runtime = self.clone(); + tokio::spawn(async move { + if let Err(error) = runtime.delete_sandbox(&workspace, &name).await { + tracing::warn!( + sandbox_name = %name, + error = %error, + "Failed to delete completed ephemeral sandbox" + ); + } + }); + } Ok(()) } @@ -3228,6 +3264,11 @@ impl ComputeRuntime { let sandbox = decode_sandbox_record(¤t_record)?; let phase = SandboxPhase::try_from(sandbox.phase()).unwrap_or(SandboxPhase::Unknown); + if phase == SandboxPhase::Completed || is_failed_main_process_result(&sandbox) { + // A terminal canonical process may legitimately have removed its + // transient compute object. Keep the durable command result. + return Ok(()); + } if matches!( phase, SandboxPhase::Stopping | SandboxPhase::Stopped | SandboxPhase::Starting @@ -3317,24 +3358,53 @@ impl ComputeRuntime { fn apply_main_process_exit(sandbox: &mut Sandbox, instance_id: &str, exit_code: i32) { let sandbox_name = sandbox.object_name().to_string(); + let preserve_infrastructure_error = sandbox.phase() == SandboxPhase::Error as i32; let status = sandbox.status.get_or_insert_with(|| SandboxStatus { sandbox_name: sandbox_name.clone(), ..Default::default() }); status.main_process_instance_id = instance_id.to_string(); status.exit_code = Some(exit_code); + if preserve_infrastructure_error { + return; + } + let (phase, reason, message) = if exit_code == 0 { + ( + SandboxPhase::Completed, + "MainProcessCompleted", + "Canonical main process completed successfully".to_string(), + ) + } else { + ( + SandboxPhase::Error, + "MainProcessFailed", + format!("Canonical main process exited with status {exit_code}"), + ) + }; upsert_ready_condition( &mut sandbox.status, &sandbox_name, SandboxCondition { r#type: "Ready".to_string(), status: "False".to_string(), - reason: "MainProcessExited".to_string(), - message: "Canonical main process exited".to_string(), + reason: reason.to_string(), + message, last_transition_time: String::new(), }, ); - sandbox.set_phase(SandboxPhase::Error as i32); + sandbox.set_phase(phase as i32); +} + +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() + && status.conditions.iter().any(|condition| { + condition.r#type == "Ready" + && condition.status.eq_ignore_ascii_case("false") + && condition.reason == "MainProcessFailed" + }) + }) } /// Connect to an unmanaged remote compute driver that is already listening on @@ -3711,10 +3781,10 @@ fn apply_driver_snapshot(sandbox: &mut Sandbox, incoming: &DriverSandbox, sessio let old_phase = SandboxPhase::try_from(sandbox.phase()).unwrap_or(SandboxPhase::Unknown); let sandbox_name = &incoming.name; - // Error is terminal until an explicit future lifecycle operation changes - // desired state. In particular, a still-running backend snapshot must not - // revive a sandbox whose canonical process has exited. - if old_phase == SandboxPhase::Error { + // Infrastructure errors and successful main-process completions are + // sticky until an explicit lifecycle operation changes desired state. A + // late backend snapshot must not revive either result. + if matches!(old_phase, SandboxPhase::Error | SandboxPhase::Completed) { if let Some(metadata) = sandbox.metadata.as_mut() { metadata.name.clone_from(sandbox_name); } @@ -3761,6 +3831,7 @@ fn apply_driver_snapshot(sandbox: &mut Sandbox, incoming: &DriverSandbox, sessio } SandboxPhase::Stopping if phase != SandboxPhase::Error => SandboxPhase::Stopping, SandboxPhase::Stopped => SandboxPhase::Stopped, + SandboxPhase::Completed => SandboxPhase::Completed, SandboxPhase::Starting if !matches!(phase, SandboxPhase::Ready | SandboxPhase::Error) => { SandboxPhase::Starting } @@ -4971,13 +5042,13 @@ mod tests { } #[test] - fn main_process_exit_zero_is_terminal_error() { + fn main_process_exit_zero_is_completed() { let mut sandbox = sandbox_record("sb-1", "sandbox-a", SandboxPhase::Ready); apply_main_process_exit(&mut sandbox, "instance-1", 0); assert_eq!( SandboxPhase::try_from(sandbox.phase()), - Ok(SandboxPhase::Error) + Ok(SandboxPhase::Completed) ); let status = sandbox.status.as_ref().unwrap(); assert_eq!(status.exit_code, Some(0)); @@ -4985,7 +5056,26 @@ mod tests { assert!(status.conditions.iter().any(|condition| { condition.r#type == "Ready" && condition.status == "False" - && condition.reason == "MainProcessExited" + && condition.reason == "MainProcessCompleted" + })); + } + + #[test] + fn main_process_nonzero_exit_is_error() { + let mut sandbox = sandbox_record("sb-1", "sandbox-a", SandboxPhase::Ready); + apply_main_process_exit(&mut sandbox, "instance-1", 7); + + assert_eq!( + SandboxPhase::try_from(sandbox.phase()), + Ok(SandboxPhase::Error) + ); + let status = sandbox.status.as_ref().unwrap(); + assert_eq!(status.exit_code, Some(7)); + assert_eq!(status.main_process_instance_id, "instance-1"); + assert!(status.conditions.iter().any(|condition| { + condition.r#type == "Ready" + && condition.status == "False" + && condition.reason == "MainProcessFailed" })); } @@ -6016,6 +6106,68 @@ mod tests { assert_eq!(driver.start_calls(), 2, "ready start is idempotent"); } + #[tokio::test] + async fn completed_sandbox_can_start_a_fresh_main_instance() { + let driver = ControlledDriver::new(); + let runtime = test_runtime(driver.clone()).await; + let mut sandbox = + sandbox_record("sb-completed", "sandbox-completed", SandboxPhase::Completed); + sandbox.status = Some(SandboxStatus { + phase: SandboxPhase::Completed as i32, + main_process_instance_id: "instance-old".to_string(), + exit_code: Some(0), + ..Default::default() + }); + runtime.store.put_message(&sandbox).await.unwrap(); + + let starting = runtime + .start_sandbox("default", sandbox.object_name()) + .await + .unwrap(); + + assert_eq!(starting.phase(), SandboxPhase::Starting as i32); + let status = starting.status.unwrap(); + assert_eq!(status.main_process_instance_id, "instance-old"); + assert_eq!(status.exit_code, None); + assert_eq!(driver.start_calls(), 1); + } + + #[tokio::test] + async fn failed_main_process_error_can_start_a_fresh_instance() { + let driver = ControlledDriver::new(); + let runtime = test_runtime(driver.clone()).await; + let mut sandbox = sandbox_record("sb-failed", "sandbox-failed", SandboxPhase::Ready); + apply_main_process_exit(&mut sandbox, "instance-old", 130); + runtime.store.put_message(&sandbox).await.unwrap(); + + let starting = runtime + .start_sandbox("default", sandbox.object_name()) + .await + .unwrap(); + + assert_eq!(starting.phase(), SandboxPhase::Starting as i32); + let status = starting.status.unwrap(); + assert_eq!(status.main_process_instance_id, "instance-old"); + assert_eq!(status.exit_code, None); + assert_eq!(driver.start_calls(), 1); + } + + #[tokio::test] + async fn infrastructure_error_cannot_be_started_as_a_command_result() { + let driver = ControlledDriver::new(); + let runtime = test_runtime(driver.clone()).await; + let sandbox = sandbox_record("sb-error", "sandbox-error", SandboxPhase::Error); + runtime.store.put_message(&sandbox).await.unwrap(); + + let error = runtime + .start_sandbox("default", sandbox.object_name()) + .await + .unwrap_err(); + + assert_eq!(error.code(), Code::FailedPrecondition); + assert_eq!(driver.start_calls(), 0); + } + #[tokio::test] async fn retained_stopping_transition_retries_driver_operation() { let driver = ControlledDriver::new(); @@ -7707,6 +7859,38 @@ mod tests { assert_eq!(status.exit_code, None); } + #[tokio::test] + async fn late_driver_exit_preserves_completed_main_result() { + let runtime = test_runtime(Arc::new(TestDriver::default())).await; + let mut sandbox = sandbox_record("sb-1", "sandbox-a", SandboxPhase::Completed); + sandbox.status = Some(SandboxStatus { + sandbox_name: "sandbox-a".to_string(), + phase: SandboxPhase::Completed as i32, + main_process_instance_id: "instance-1".to_string(), + exit_code: Some(0), + ..Default::default() + }); + runtime.store.put_message(&sandbox).await.unwrap(); + let mut exited = ready_driver_sandbox("sb-1", "sandbox-a"); + exited.status = Some(make_driver_status(make_driver_condition( + "ContainerExited", + "container exited after the canonical process completed", + ))); + + runtime.apply_sandbox_update(exited).await.unwrap(); + + let stored = runtime + .store + .get_message::("sb-1") + .await + .unwrap() + .unwrap(); + assert_eq!(stored.phase(), SandboxPhase::Completed as i32); + let status = stored.status.unwrap(); + assert_eq!(status.main_process_instance_id, "instance-1"); + assert_eq!(status.exit_code, Some(0)); + } + #[tokio::test] async fn apply_sandbox_update_without_status_preserves_existing_status() { let runtime = test_runtime(Arc::new(TestDriver::default())).await; diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index 89f8c942ea..abdc8880c1 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -1032,7 +1032,7 @@ pub(super) async fn handle_watch_sandbox( if stop_on_terminal { let phase = SandboxPhase::try_from(sandbox.phase()) .unwrap_or(SandboxPhase::Unknown); - if phase == SandboxPhase::Ready { + if is_watch_terminal(phase) { return; } } @@ -1106,7 +1106,7 @@ pub(super) async fn handle_watch_sandbox( } if stop_on_terminal { let phase = SandboxPhase::try_from(sandbox.phase()).unwrap_or(SandboxPhase::Unknown); - if phase == SandboxPhase::Ready { + if is_watch_terminal(phase) { return; } } @@ -1179,6 +1179,13 @@ pub(super) async fn handle_watch_sandbox( Ok(Response::new(WatchSandboxStream::new(rx, producer))) } +fn is_watch_terminal(phase: SandboxPhase) -> bool { + matches!( + phase, + SandboxPhase::Ready | SandboxPhase::Completed | SandboxPhase::Stopped | SandboxPhase::Error + ) +} + // --------------------------------------------------------------------------- // Exec handler // --------------------------------------------------------------------------- @@ -2458,6 +2465,30 @@ mod tests { // ---- shell_escape ---- + #[test] + fn watch_terminal_phases_include_command_results_and_errors() { + for phase in [ + SandboxPhase::Ready, + SandboxPhase::Completed, + SandboxPhase::Stopped, + SandboxPhase::Error, + ] { + assert!(is_watch_terminal(phase), "{phase:?} should stop the watch"); + } + for phase in [ + SandboxPhase::Provisioning, + SandboxPhase::Starting, + SandboxPhase::Stopping, + SandboxPhase::Deleting, + SandboxPhase::Unknown, + ] { + assert!( + !is_watch_terminal(phase), + "{phase:?} should keep the watch open" + ); + } + } + #[test] fn telemetry_compute_driver_uses_resolved_driver_kind() { assert_eq!( diff --git a/crates/openshell-supervisor-process/src/main_session.rs b/crates/openshell-supervisor-process/src/main_session.rs index fc5c9693e6..682de6227a 100644 --- a/crates/openshell-supervisor-process/src/main_session.rs +++ b/crates/openshell-supervisor-process/src/main_session.rs @@ -54,6 +54,10 @@ struct OutputLogState { struct OutputLog { state: Mutex, version: watch::Sender, + terminal_delivered: std::sync::atomic::AtomicBool, + terminal_delivered_notify: Notify, + terminal_reported: std::sync::atomic::AtomicBool, + terminal_reported_notify: Notify, } impl OutputLog { @@ -66,6 +70,10 @@ impl OutputLog { next_sequence: 0, }), version, + terminal_delivered: std::sync::atomic::AtomicBool::new(false), + terminal_delivered_notify: Notify::new(), + terminal_reported: std::sync::atomic::AtomicBool::new(false), + terminal_reported_notify: Notify::new(), }) } @@ -170,6 +178,7 @@ pub struct MainSession { pty_master: Option>, readers_remaining: AtomicUsize, readers_done: Notify, + finished: std::sync::atomic::AtomicBool, } impl MainSession { @@ -186,6 +195,7 @@ impl MainSession { pty_master: None, readers_remaining: AtomicUsize::new(0), readers_done: Notify::new(), + finished: std::sync::atomic::AtomicBool::new(false), }) } @@ -230,6 +240,7 @@ impl MainSession { pty_master, readers_remaining: AtomicUsize::new(if terminal { 1 } else { 2 }), readers_done: Notify::new(), + finished: std::sync::atomic::AtomicBool::new(false), }); Self::start_io(&session, io, input_rx); session @@ -345,6 +356,7 @@ impl MainSession { if self.readers_remaining.load(Ordering::Acquire) != 0 { let _ = tokio::time::timeout(std::time::Duration::from_secs(2), notified).await; } + self.finished.store(true, Ordering::Release); self.publish(MainOutput::Exit(exit_code)); } @@ -352,6 +364,41 @@ impl MainSession { self.output.subscribe() } + /// Wait until an attached main-session consumer receives the terminal + /// exit event. Callers use a bounded timeout for detached commands. + pub async fn wait_for_terminal_delivery(&self) { + let notified = self.output.terminal_delivered_notify.notified(); + if self.output.terminal_delivered.load(Ordering::Acquire) { + return; + } + notified.await; + } + + /// Record that an SSH main attachment drained output through the terminal + /// event and is ready for the durable lifecycle report. + pub fn mark_terminal_delivered(&self) { + self.output + .terminal_delivered + .store(true, Ordering::Release); + self.output.terminal_delivered_notify.notify_waiters(); + } + + /// Wait until the gateway durably acknowledges the main-process result. + pub async fn wait_for_terminal_reported(&self) { + let notified = self.output.terminal_reported_notify.notified(); + if self.output.terminal_reported.load(Ordering::Acquire) { + return; + } + notified.await; + } + + /// Release attached clients to receive their SSH exit status after the + /// durable sandbox phase and exit code have been recorded. + pub fn mark_terminal_reported(&self) { + self.output.terminal_reported.store(true, Ordering::Release); + self.output.terminal_reported_notify.notify_waiters(); + } + pub fn acquire_input(&self) -> Result<(u64, tokio::sync::mpsc::Sender>), &'static str> { let mut owner = self.input_owner.lock().expect("main input lock poisoned"); if owner.is_some() { @@ -394,6 +441,11 @@ impl MainSession { pub const fn terminal(&self) -> bool { self.terminal } + + #[must_use] + pub fn finished(&self) -> bool { + self.finished.load(Ordering::Acquire) + } } fn set_nonblocking(file: &std::fs::File) -> Result<(), nix::errno::Errno> { @@ -439,6 +491,55 @@ mod tests { )); } + #[tokio::test] + async fn terminal_delivery_requires_attachment_acknowledgement() { + let session = MainSession::inert(); + session.finish(0).await; + assert!(session.finished()); + + assert!( + tokio::time::timeout( + std::time::Duration::from_millis(10), + session.wait_for_terminal_delivery(), + ) + .await + .is_err(), + "publishing Exit alone must not count as delivery" + ); + + session.mark_terminal_delivered(); + tokio::time::timeout( + std::time::Duration::from_secs(1), + session.wait_for_terminal_delivery(), + ) + .await + .expect("delivery acknowledgement should wake waiter"); + } + + #[tokio::test] + async fn terminal_report_acknowledgement_is_independent_from_delivery() { + let session = MainSession::inert(); + session.mark_terminal_delivered(); + + assert!( + tokio::time::timeout( + std::time::Duration::from_millis(10), + session.wait_for_terminal_reported(), + ) + .await + .is_err(), + "draining output must not imply durable gateway persistence" + ); + + session.mark_terminal_reported(); + tokio::time::timeout( + std::time::Duration::from_secs(1), + session.wait_for_terminal_reported(), + ) + .await + .expect("durable report acknowledgement should wake waiter"); + } + #[tokio::test] async fn exit_is_replayed_once_to_late_subscribers() { let session = MainSession::inert(); diff --git a/crates/openshell-supervisor-process/src/run.rs b/crates/openshell-supervisor-process/src/run.rs index 34b0001110..8e0a48fe19 100644 --- a/crates/openshell-supervisor-process/src/run.rs +++ b/crates/openshell-supervisor-process/src/run.rs @@ -39,6 +39,8 @@ use crate::process::{ ResolvedWorkspace, }; +const TERMINAL_DRAIN_TIMEOUT: Duration = Duration::from_secs(30); + pub type SidecarExitReport = ( String, i32, @@ -312,9 +314,9 @@ pub async fn run_process( } }); - // Wait for the SSH server to bind its socket before spawning the - // entrypoint process. This prevents exec requests from racing against - // SSH server startup when Kubernetes marks the pod Ready. + // Wait for the SSH server to bind before advertising its relay. The + // main process is already supervised; MainSession retains any output + // produced while this endpoint is being prepared. match timeout(Duration::from_secs(10), ssh_ready_rx).await { Ok(Ok(Ok(()))) => { ocsf_emit!( @@ -344,37 +346,35 @@ pub async fn run_process( let supervisor_terminating = Arc::new(AtomicBool::new(false)); // A canonical process may have completed while the SSH socket was being - // prepared. Never open a readiness-bearing supervisor session for a child - // that is already terminal. + // prepared. Its relay must still register so a foreground create can + // attach and replay retained output before runtime shutdown. let early_exit = handle.try_wait().into_diagnostic()?; // Spawn the persistent supervisor session if we have a gateway endpoint // and sandbox identity. The session provides relay channels for SSH // connect and ExecSandbox through the gateway. - let supervisor_session_task = if early_exit.is_none() - && let (Some(endpoint), Some(id), Some(socket)) = + let (supervisor_session_task, supervisor_session_ready) = + if let (Some(endpoint), Some(id), Some(socket)) = (openshell_endpoint, sandbox_id, ssh_socket_path.as_ref()) - { - let task = crate::supervisor_session::spawn( - endpoint.to_string(), - id.to_string(), - socket.clone(), - ssh_netns_fd, - None, - Arc::clone(&supervisor_terminating), - main_instance_id.clone(), - ); - info!("supervisor session task spawned"); - Some(task) - } else { - None - }; + { + let (task, ready) = crate::supervisor_session::spawn_with_ready( + endpoint.to_string(), + id.to_string(), + socket.clone(), + ssh_netns_fd, + None, + Arc::clone(&supervisor_terminating), + main_instance_id.clone(), + ); + info!("supervisor session task spawned"); + (Some(task), Some(ready)) + } else { + (None, None) + }; // Store the entrypoint PID so the proxy can resolve TCP peer identity entrypoint_pid.store(handle.pid(), Ordering::Release); - if early_exit.is_none() - && let Some(tx) = entrypoint_started_tx - { + if let Some(tx) = entrypoint_started_tx { let _ = tx.send((handle.pid(), main_instance_id.clone())); } ocsf_emit!( @@ -397,8 +397,8 @@ pub async fn run_process( .await? }; - let rendered_code = match outcome { - ProcessWaitOutcome::Exited(status) => status.code(), + let (rendered_code, drain_terminal) = match outcome { + ProcessWaitOutcome::Exited(status) => (status.code(), true), ProcessWaitOutcome::TimedOut => { ocsf_emit!( ProcessActivityBuilder::new(ocsf_ctx()) @@ -410,7 +410,7 @@ pub async fn run_process( .message("Process timed out, killing") .build() ); - 124 + (124, false) } ProcessWaitOutcome::ShutdownSignal { signal, status } => { info!( @@ -418,10 +418,9 @@ pub async fn run_process( exit_code = status.code(), "Entrypoint exited after supervisor shutdown signal" ); - status.code() + (status.code(), false) } }; - supervisor_terminating.store(true, Ordering::Release); main_session.finish(rendered_code).await; ocsf_emit!( @@ -436,8 +435,21 @@ pub async fn run_process( .build() ); - if let Some(task) = supervisor_session_task { - task.abort(); + if drain_terminal && let Some(ready) = supervisor_session_ready { + timeout(TERMINAL_DRAIN_TIMEOUT, ready) + .await + .map_err(|_| miette::miette!("supervisor session registration timed out"))? + .map_err(|_| miette::miette!("supervisor session ended before registration"))?; + } + if drain_terminal && (supervisor_session_task.is_some() || sidecar_exit_tx.is_some()) { + // Keep the public phase Ready until a foreground attachment has + // drained the result. Explicitly detached commands use the bounded + // fallback and then publish their terminal result. + let _ = timeout( + TERMINAL_DRAIN_TIMEOUT, + main_session.wait_for_terminal_delivery(), + ) + .await; } if let Some(tx) = sidecar_exit_tx { let (ack_tx, ack_rx) = tokio::sync::oneshot::channel(); @@ -452,6 +464,12 @@ pub async fn run_process( report_main_process_exit_until_ack(endpoint, id, &main_instance_id, rendered_code).await; info!(instance_id = %main_instance_id, "main-process exit acknowledged"); } + main_session.mark_terminal_reported(); + + supervisor_terminating.store(true, Ordering::Release); + if let Some(task) = supervisor_session_task { + task.abort(); + } Ok(rendered_code) } @@ -505,7 +523,6 @@ async fn wait_for_process_exit_or_shutdown( tokio::pin!(deadline); tokio::select! { result = &mut wait => { - terminating.store(true, Ordering::Release); Ok(ProcessWaitOutcome::Exited(result.into_diagnostic()?)) } () = &mut deadline => { @@ -523,7 +540,6 @@ async fn wait_for_process_exit_or_shutdown( } else { tokio::select! { result = &mut wait => { - terminating.store(true, Ordering::Release); Ok(ProcessWaitOutcome::Exited(result.into_diagnostic()?)) } signal = wait_for_supervisor_shutdown_signal() => { diff --git a/crates/openshell-supervisor-process/src/ssh.rs b/crates/openshell-supervisor-process/src/ssh.rs index 41546f6473..25c6531682 100644 --- a/crates/openshell-supervisor-process/src/ssh.rs +++ b/crates/openshell-supervisor-process/src/ssh.rs @@ -413,6 +413,12 @@ impl russh::server::Handler for SshHandler { reply: ChannelOpenHandle, _session: &mut Session, ) -> Result<(), Self::Error> { + if self.main_session.finished() { + reply + .reject(ChannelOpenFailure::AdministrativelyProhibited) + .await; + return Ok(()); + } // Validate port range before truncating u32 -> u16. The SSH protocol // uses u32 for ports, but valid TCP ports are 0-65535. Without this // check, port 65537 truncates to port 1 (privileged). @@ -548,6 +554,10 @@ impl russh::server::Handler for SshHandler { channel: ChannelId, session: &mut Session, ) -> Result<(), Self::Error> { + if self.main_session.finished() { + session.channel_failure(channel)?; + return Ok(()); + } session.channel_success(channel)?; // Only allocate a PTY when the client explicitly requested one via // pty_request. VS Code Remote-SSH sends shell_request *without* a @@ -565,6 +575,10 @@ impl russh::server::Handler for SshHandler { data: &[u8], session: &mut Session, ) -> Result<(), Self::Error> { + if self.main_session.finished() { + session.channel_failure(channel)?; + return Ok(()); + } session.channel_success(channel)?; let command = String::from_utf8_lossy(data).trim().to_string(); if command.is_empty() { @@ -610,6 +624,7 @@ impl russh::server::Handler for SshHandler { state.main_detach_prefix_pending = false; state.input_sender = input; let mut output = self.main_session.subscribe(); + let terminal_delivery = Arc::clone(&self.main_session); let handle = session.handle(); session.channel_success(channel)?; if let Some(error) = input_warning { @@ -625,11 +640,14 @@ impl russh::server::Handler for SshHandler { loop { match output.recv().await { Ok(event) => { - let exited = matches!(event, MainOutput::Exit(_)); - send_main_output(&handle, channel, event).await; - if exited { + if let MainOutput::Exit(code) = event { + terminal_delivery.mark_terminal_delivered(); + terminal_delivery.wait_for_terminal_reported().await; + let _ = send_main_output(&handle, channel, MainOutput::Exit(code)) + .await; break; } + let _ = send_main_output(&handle, channel, event).await; } Err(error) => { let _ = handle @@ -652,7 +670,7 @@ impl russh::server::Handler for SshHandler { if let Some(state) = self.channels.get_mut(&channel) { state.main_output_task = Some(output_task.abort_handle()); } - } else if name == "sftp" { + } else if name == "sftp" && !self.main_session.finished() { session.channel_success(channel)?; // sftp-server speaks the SFTP binary protocol over stdin/stdout, // which is exactly what spawn_pipe_exec wires up. This enables @@ -810,20 +828,18 @@ impl russh::server::Handler for SshHandler { } } -async fn send_main_output(handle: &Handle, channel: ChannelId, event: MainOutput) { +async fn send_main_output(handle: &Handle, channel: ChannelId, event: MainOutput) -> bool { match event { - MainOutput::Stdout(data) => { - let _ = handle.data(channel, data).await; - } - MainOutput::Stderr(data) => { - let _ = handle.extended_data(channel, 1, data).await; - } + MainOutput::Stdout(data) => handle.data(channel, data).await.is_ok(), + MainOutput::Stderr(data) => handle.extended_data(channel, 1, data).await.is_ok(), MainOutput::Exit(code) => { - let _ = handle.eof(channel).await; - let _ = handle + let eof_sent = handle.eof(channel).await.is_ok(); + let status_sent = handle .exit_status_request(channel, code.max(0).unsigned_abs()) - .await; - let _ = handle.close(channel).await; + .await + .is_ok(); + let close_sent = handle.close(channel).await.is_ok(); + eof_sent && status_sent && close_sent } } } diff --git a/crates/openshell-supervisor-process/src/supervisor_session.rs b/crates/openshell-supervisor-process/src/supervisor_session.rs index e8a140e483..4192537ce2 100644 --- a/crates/openshell-supervisor-process/src/supervisor_session.rs +++ b/crates/openshell-supervisor-process/src/supervisor_session.rs @@ -283,7 +283,7 @@ pub fn spawn( terminating: Arc, instance_id: String, ) -> tokio::task::JoinHandle<()> { - tokio::spawn(run_session_loop( + spawn_with_ready( endpoint, sandbox_id, ssh_socket_path, @@ -291,10 +291,41 @@ pub fn spawn( expected_ssh_peer_pid, terminating, instance_id, - )) + ) + .0 } -async fn run_session_loop( +/// Spawn the supervisor session with an acceptance notification. +/// +/// The notification orders a fast main-process exit after relay registration +/// so terminal output remains attachable. +pub fn spawn_with_ready( + endpoint: String, + sandbox_id: String, + ssh_socket_path: std::path::PathBuf, + netns_fd: Option, + expected_ssh_peer_pid: Option, + terminating: Arc, + instance_id: String, +) -> ( + tokio::task::JoinHandle<()>, + tokio::sync::oneshot::Receiver<()>, +) { + let (ready_tx, ready_rx) = tokio::sync::oneshot::channel(); + let config = SessionConfig { + endpoint, + sandbox_id, + ssh_socket_path, + netns_fd, + expected_ssh_peer_pid, + terminating, + instance_id, + }; + let task = tokio::spawn(run_session_loop(config, Some(ready_tx))); + (task, ready_rx) +} + +struct SessionConfig { endpoint: String, sandbox_id: String, ssh_socket_path: std::path::PathBuf, @@ -302,6 +333,11 @@ async fn run_session_loop( expected_ssh_peer_pid: Option, terminating: Arc, instance_id: String, +} + +async fn run_session_loop( + config: SessionConfig, + mut ready_tx: Option>, ) { let mut backoff = INITIAL_BACKOFF; let mut attempt: u64 = 0; @@ -309,27 +345,20 @@ async fn run_session_loop( loop { attempt += 1; - match run_single_session( - &endpoint, - &sandbox_id, - &ssh_socket_path, - netns_fd, - expected_ssh_peer_pid, - Arc::clone(&terminating), - &instance_id, - ) - .await - { + match run_single_session(&config, &mut ready_tx).await { Ok(()) => { - let event = - session_closed_event(openshell_ocsf::ctx::ctx(), &endpoint, &sandbox_id); + let event = session_closed_event( + openshell_ocsf::ctx::ctx(), + &config.endpoint, + &config.sandbox_id, + ); ocsf_emit!(event); break; } Err(e) => { let event = session_failed_event( openshell_ocsf::ctx::ctx(), - &endpoint, + &config.endpoint, attempt, &e.to_string(), ); @@ -342,19 +371,14 @@ async fn run_session_loop( } async fn run_single_session( - endpoint: &str, - sandbox_id: &str, - ssh_socket_path: &std::path::Path, - netns_fd: Option, - expected_ssh_peer_pid: Option, - terminating: Arc, - instance_id: &str, + config: &SessionConfig, + ready_tx: &mut Option>, ) -> Result<(), Box> { // Connect to the gateway. The same `Channel` is used for both the // long-lived control stream and all data-plane `RelayStream` calls, so // every relay rides the same TCP+TLS+HTTP/2 connection — no new TLS // handshake per relay. - let channel = grpc_client::connect_channel_pub(endpoint) + let channel = grpc_client::connect_channel_pub(&config.endpoint) .await .map_err(|e| format!("connect failed: {e}"))?; let mut client = OpenShellClient::new(channel.clone()); @@ -366,8 +390,8 @@ async fn run_single_session( // Send hello as the first message. tx.send(SupervisorMessage { payload: Some(supervisor_message::Payload::Hello(SupervisorHello { - sandbox_id: sandbox_id.to_string(), - instance_id: instance_id.to_string(), + sandbox_id: config.sandbox_id.clone(), + instance_id: config.instance_id.clone(), })), }) .await @@ -397,11 +421,14 @@ async fn run_single_session( let heartbeat_secs = accepted.heartbeat_interval_secs.max(5); let event = session_established_event( openshell_ocsf::ctx::ctx(), - endpoint, + &config.endpoint, &accepted.session_id, heartbeat_secs, ); ocsf_emit!(event); + if let Some(ready_tx) = ready_tx.take() { + let _ = ready_tx.send(()); + } // Main loop: receive gateway messages + send heartbeats. let mut heartbeat_interval = @@ -414,19 +441,19 @@ async fn run_single_session( let msg = match map_session_stream_message( msg, "gateway closed stream", - &terminating, + &config.terminating, )? { SessionStreamMessage::Message(msg) => msg, SessionStreamMessage::ExpectedShutdownClose => return Ok(()), }; let context = GatewayMessageContext { - sandbox_id, - ssh_socket_path, - netns_fd, - expected_ssh_peer_pid, + sandbox_id: &config.sandbox_id, + ssh_socket_path: &config.ssh_socket_path, + netns_fd: config.netns_fd, + expected_ssh_peer_pid: config.expected_ssh_peer_pid, channel: &channel, tx: &tx, - terminating: &terminating, + terminating: &config.terminating, }; handle_gateway_message( &msg, diff --git a/crates/openshell-tui/src/lib.rs b/crates/openshell-tui/src/lib.rs index 1f610015b4..3c9a5b825d 100644 --- a/crates/openshell-tui/src/lib.rs +++ b/crates/openshell-tui/src/lib.rs @@ -2690,6 +2690,7 @@ fn phase_label(phase: i32) -> String { x if x == SandboxPhase::Stopping as i32 => "Stopping", x if x == SandboxPhase::Stopped as i32 => "Stopped", x if x == SandboxPhase::Starting as i32 => "Starting", + x if x == SandboxPhase::Completed as i32 => "Completed", _ => "Unknown", } .to_string() @@ -2728,6 +2729,7 @@ mod phase_label_tests { assert_eq!(phase_label(SandboxPhase::Stopping as i32), "Stopping"); assert_eq!(phase_label(SandboxPhase::Stopped as i32), "Stopped"); assert_eq!(phase_label(SandboxPhase::Starting as i32), "Starting"); + assert_eq!(phase_label(SandboxPhase::Completed as i32), "Completed"); } } diff --git a/crates/openshell-tui/src/ui/sandbox_detail.rs b/crates/openshell-tui/src/ui/sandbox_detail.rs index 434f369d39..4ff78617f0 100644 --- a/crates/openshell-tui/src/ui/sandbox_detail.rs +++ b/crates/openshell-tui/src/ui/sandbox_detail.rs @@ -22,7 +22,7 @@ pub fn draw(frame: &mut Frame<'_>, app: &App, area: Rect) { let age = app.sandbox_ages.get(idx).map_or("-", String::as_str); let phase_style = match phase { - "Ready" => t.status_ok, + "Ready" | "Completed" => t.status_ok, "Provisioning" | "Stopping" | "Starting" => t.status_warn, "Error" => t.status_err, _ => t.muted, @@ -30,6 +30,7 @@ pub fn draw(frame: &mut Frame<'_>, app: &App, area: Rect) { let status_indicator = match phase { "Ready" => "●", + "Completed" => "✓", "Provisioning" | "Stopping" | "Starting" => "◐", "Error" | "Stopped" => "○", _ => "…", diff --git a/crates/openshell-tui/src/ui/sandboxes.rs b/crates/openshell-tui/src/ui/sandboxes.rs index d927537189..b1d3edc1fe 100644 --- a/crates/openshell-tui/src/ui/sandboxes.rs +++ b/crates/openshell-tui/src/ui/sandboxes.rs @@ -40,7 +40,7 @@ pub fn draw(frame: &mut Frame<'_>, app: &App, area: Rect, focused: bool) { let draft_count = app.sandbox_draft_counts.get(i).copied().unwrap_or(0); let phase_style = match phase { - "Ready" => t.status_ok, + "Ready" | "Completed" => t.status_ok, "Provisioning" | "Stopping" | "Starting" => t.status_warn, "Error" => t.status_err, _ => t.muted, diff --git a/docs/reference/sandbox-compute-drivers.mdx b/docs/reference/sandbox-compute-drivers.mdx index 656ae43bb6..3d0b01e56a 100644 --- a/docs/reference/sandbox-compute-drivers.mdx +++ b/docs/reference/sandbox-compute-drivers.mdx @@ -29,7 +29,9 @@ The gateway forwards one exact, persisted main-process specification to every driver. Drivers serialize that specification in `OPENSHELL_MAIN_PROCESS_SPEC`; they do not install an idle `sleep` workload or reconstruct argv with shell parsing. Runtime restart policies are disabled so -an exited canonical process remains a terminal sandbox error. +an exited canonical process remains a terminal sandbox result. Exit code zero +produces `Completed`; a nonzero or signal-normalized exit produces `Error` +with the exact exit code. Driver and supervisor failures remain `Error`. ## Configure a Compute Driver diff --git a/docs/sandboxes/manage-sandboxes.mdx b/docs/sandboxes/manage-sandboxes.mdx index ead6533294..b1c975f984 100644 --- a/docs/sandboxes/manage-sandboxes.mdx +++ b/docs/sandboxes/manage-sandboxes.mdx @@ -21,7 +21,9 @@ openshell sandbox create -- claude ``` The trailing command is the sandbox's canonical main process. OpenShell starts -it once and attaches your terminal to it. With no trailing command, OpenShell +it once, streams its output, and returns its exit status. Exit code 0 leaves a +retained sandbox in `Completed`; a nonzero exit leaves it in `Error` with a +`MainProcessFailed` condition. With no trailing command, OpenShell starts `/bin/bash -l` in a retained pseudo-terminal. Add `--detach` to create the sandbox without attaching: @@ -29,6 +31,13 @@ the sandbox without attaching: openshell sandbox create --name worker --detach -- ./worker ``` +Use `--no-keep` for an ephemeral command. OpenShell drains stdout and stderr, +captures the command result, and deletes the sandbox after the command exits: + +```shell +openshell sandbox create --no-keep -- sh -c 'echo done; exit 0' +``` + `--upload` cannot yet be combined with a trailing main command because uploads finish after the canonical process starts. Create a scratch sandbox, upload the files, then launch the workload with `sandbox exec`, or build the files into the @@ -478,8 +487,10 @@ commands, transfer files, forward ports, or reach exposed services. Policies, provider attachments, settings, service definitions, and persistent workspace data remain associated with the sandbox. -Stop and start are idempotent. Delete a stopped sandbox normally when you -no longer need its retained state. +Stop and start are idempotent. You can also start a retained `Completed` or +`Error/MainProcessFailed` sandbox to launch a fresh instance of its canonical +command. Delete an inactive sandbox normally when you no longer need its +retained state. ## Delete Sandboxes @@ -498,9 +509,10 @@ Every sandbox moves through a defined set of phases: | Provisioning | The runtime is setting up the sandbox environment, or the gateway is waiting for the sandbox supervisor to establish its authenticated control session. | | Ready | The sandbox is running and its supervisor control session is connected. You can connect, execute commands, sync files, and view logs. | | Stopping | The gateway accepted a stop request and is stopping compute while retaining persistent state. | -| Stopped | Compute is stopped and access is unavailable. The sandbox record and driver-owned persistent workspace remain. | +| Stopped | Compute was stopped explicitly and access is unavailable. | | Starting | Compute is starting. The sandbox becomes usable only after a fresh supervisor session connects. | -| Error | Provisioning failed or the canonical main process exited unexpectedly. Main-process exit is terminal even with exit code 0. Check logs with `openshell logs`. | +| Completed | The canonical main process exited with code 0. Its normalized result is available in `status.exit_code`. | +| Error | The canonical main process failed, or sandbox infrastructure failed. Inspect the condition reason and `status.exit_code`. | | Deleting | The sandbox is being torn down. The system releases resources and purges credentials. | The compute backend can become ready before the sandbox supervisor connects to @@ -510,10 +522,11 @@ After a gateway restart, an existing sandbox can return to `Provisioning` temporarily while its supervisor reconnects. Wait for the phase to return to `Ready` before you connect to the sandbox or execute commands. -The gateway records a canonical main-process exit as `Ready=False` with reason -`MainProcessExited`. It also sets `status.exit_code`; signal exits use the -standard `128 + signal` convention. Compute runtimes do not automatically -restart that process. +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. Compute runtimes do +not automatically restart that process. ## Sandbox Compute Drivers diff --git a/e2e/mcp-conformance.sh b/e2e/mcp-conformance.sh index c1b46fe53a..1bbe5951ff 100755 --- a/e2e/mcp-conformance.sh +++ b/e2e/mcp-conformance.sh @@ -338,6 +338,7 @@ create_client_sandbox() { --from "${CLIENT_IMAGE}" \ --policy "${policy_file}" \ --no-tty \ + --detach \ -- sleep infinity; then rm -f "${policy_file}" return 1 diff --git a/e2e/rust/tests/oidc_pkce.rs b/e2e/rust/tests/oidc_pkce.rs index e6f6067a34..f8edc3d7b2 100644 --- a/e2e/rust/tests/oidc_pkce.rs +++ b/e2e/rust/tests/oidc_pkce.rs @@ -28,8 +28,6 @@ use url::Url; static SANDBOX_LIFECYCLE_LOCK: Mutex<()> = Mutex::const_new(()); -const DURABLE_MAIN_SCRIPT: &str = r#"echo "$1"; exec sleep infinity"#; - #[derive(Clone, Copy)] struct IdentityScenario { gateway_name: &'static str, @@ -915,10 +913,7 @@ async fn workspace_user_cannot_create_sandbox_in_another_workspace() { "oidc-xcreate-denied", "--no-tty", "--", - "sh", - "-c", - DURABLE_MAIN_SCRIPT, - "_", + "echo", "denied", ], ) @@ -1388,12 +1383,8 @@ async fn assert_can_create_sandbox(session: &LoginSession, workspace: &str, sand "--name", sandbox_name, "--no-tty", - "--detach", "--", - "sh", - "-c", - DURABLE_MAIN_SCRIPT, - "_", + "echo", &marker, ], ) @@ -1413,6 +1404,10 @@ async fn assert_can_create_sandbox(session: &LoginSession, workspace: &str, sand let list_output = combined_output(&list); let cleanup = run_workspace_cli(session, workspace, &["sandbox", "delete", sandbox_name]).await; + assert!( + create_output.contains(&marker), + "sandbox command output should contain {marker}:\n{create_output}" + ); assert!( list.status.success() && list_output.contains(sandbox_name), "created sandbox {sandbox_name} should appear in the sandbox list:\n{list_output}" @@ -1435,12 +1430,8 @@ async fn assert_can_delete_sandbox(session: &LoginSession, workspace: &str, sand "--name", sandbox_name, "--no-tty", - "--detach", "--", - "sh", - "-c", - DURABLE_MAIN_SCRIPT, - "_", + "echo", &marker, ], ) diff --git a/e2e/rust/tests/sandbox_lifecycle.rs b/e2e/rust/tests/sandbox_lifecycle.rs index 0b0f4e0e66..c920fd42bb 100644 --- a/e2e/rust/tests/sandbox_lifecycle.rs +++ b/e2e/rust/tests/sandbox_lifecycle.rs @@ -220,7 +220,7 @@ async fn sandbox_can_be_deleted_while_stopped() { } #[tokio::test] -async fn canonical_main_exit_transitions_persistent_sandbox_to_error() { +async fn canonical_main_exit_zero_completes_persistent_sandbox() { let mut cmd = openshell_tty_cmd(&["sandbox", "create", "--", "echo", "OK"]); cmd.stdout(Stdio::piped()).stderr(Stdio::piped()); @@ -229,9 +229,10 @@ async fn canonical_main_exit_transitions_persistent_sandbox_to_error() { let stderr = String::from_utf8_lossy(&output.stderr).to_string(); let combined = normalize_output(&format!("{stdout}{stderr}")); + assert!(output.status.success(), "create failed:\n{combined}"); assert!( - !output.status.success(), - "main-process exit must fail create" + combined.contains("OK"), + "main output was not streamed:\n{combined}" ); let sandbox_name = extract_sandbox_name(&combined).expect("sandbox name should be present in output"); @@ -260,13 +261,65 @@ async fn canonical_main_exit_transitions_persistent_sandbox_to_error() { "sandbox get failed:\n{details}" ); assert!( - details.contains("Phase: Error"), + details.contains("Phase: Completed"), "expected terminal sandbox phase:\n{details}" ); delete_sandbox(&sandbox_name).await; } +#[tokio::test] +async fn canonical_main_nonzero_exit_preserves_status() { + let mut cmd = openshell_tty_cmd(&[ + "sandbox", + "create", + "--", + "sh", + "-c", + "echo failed-main; exit 7", + ]); + cmd.stdout(Stdio::piped()).stderr(Stdio::piped()); + + let output = cmd.output().await.expect("spawn openshell sandbox create"); + let combined = normalize_output(&format!( + "{}{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + )); + assert_eq!( + output.status.code(), + Some(7), + "unexpected result:\n{combined}" + ); + assert!( + combined.contains("failed-main"), + "main output was not streamed:\n{combined}" + ); + let sandbox_name = + extract_sandbox_name(&combined).expect("sandbox name should be present in output"); + + let mut get_cmd = openshell_cmd(); + get_cmd + .args(["sandbox", "get", &sandbox_name]) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + let get_output = get_cmd.output().await.expect("spawn openshell sandbox get"); + let details = normalize_output(&format!( + "{}{}", + String::from_utf8_lossy(&get_output.stdout), + String::from_utf8_lossy(&get_output.stderr), + )); + assert!( + details.contains("Phase: Error"), + "unexpected phase:\n{details}" + ); + assert!( + details.contains("Exit Code: 7"), + "missing exit code:\n{details}" + ); + delete_sandbox(&sandbox_name).await; +} + #[tokio::test] async fn canonical_tty_main_uses_sandbox_environment() { let script = r#"printf 'canonical_env home=%s user=%s term=%s\n' "$HOME" "$USER" "$TERM"; while true; do sleep 1; done"#; @@ -431,9 +484,10 @@ async fn sandbox_create_with_no_keep_cleans_up_after_tty_command() { let stderr = String::from_utf8_lossy(&output.stderr).to_string(); let combined = normalize_output(&format!("{stdout}{stderr}")); + assert!(output.status.success(), "create failed:\n{combined}"); assert!( - !output.status.success(), - "main-process exit must fail create" + combined.contains("OK"), + "main output was not streamed:\n{combined}" ); let sandbox_name = extract_sandbox_name(&combined).expect("sandbox name should be present in output"); diff --git a/e2e/rust/tests/workspace_namespace_managed.rs b/e2e/rust/tests/workspace_namespace_managed.rs index b6d0555f3a..eae13e2cf6 100644 --- a/e2e/rust/tests/workspace_namespace_managed.rs +++ b/e2e/rust/tests/workspace_namespace_managed.rs @@ -148,16 +148,16 @@ async fn managed_creates_namespace_with_labels() { &ws, "--name", "mgd-sb", - "--detach", "--", - "sh", - "-c", - DURABLE_MAIN_SCRIPT, - "_", + "echo", "managed-ok", ]) .await; assert!(ok, "sandbox create failed: {out}"); + assert!( + out.contains("managed-ok"), + "sandbox output missing expected string: {out}" + ); // Verify the managed namespace was created. let (ok, out) = kubectl(&["get", "namespace", &ns]).await; @@ -281,10 +281,7 @@ async fn managed_namespace_survives_with_remaining_sandboxes() { "--name", "sb-a", "--", - "sh", - "-c", - DURABLE_MAIN_SCRIPT, - "_", + "echo", "a", ]) .await; @@ -298,10 +295,7 @@ async fn managed_namespace_survives_with_remaining_sandboxes() { "--name", "sb-b", "--", - "sh", - "-c", - DURABLE_MAIN_SCRIPT, - "_", + "echo", "b", ]) .await; @@ -363,10 +357,7 @@ async fn managed_isolates_workspaces_into_separate_namespaces() { "--name", "sb-iso-a", "--", - "sh", - "-c", - DURABLE_MAIN_SCRIPT, - "_", + "echo", "a", ]) .await; @@ -380,10 +371,7 @@ async fn managed_isolates_workspaces_into_separate_namespaces() { "--name", "sb-iso-b", "--", - "sh", - "-c", - DURABLE_MAIN_SCRIPT, - "_", + "echo", "b", ]) .await; @@ -457,16 +445,16 @@ async fn managed_workspace_delete_removes_namespace() { &ws, "--name", "del-sb", - "--detach", "--", - "sh", - "-c", - DURABLE_MAIN_SCRIPT, - "_", + "echo", "del-ok", ]) .await; assert!(ok, "sandbox create failed: {out}"); + assert!( + out.contains("del-ok"), + "sandbox output missing expected string: {out}" + ); let (ok, _) = kubectl(&["get", "namespace", &ns]).await; assert!( @@ -529,16 +517,16 @@ async fn managed_tls_secret_copied_to_namespace() { &ws, "--name", "tls-sb", - "--detach", "--", - "sh", - "-c", - DURABLE_MAIN_SCRIPT, - "_", + "echo", "tls-ok", ]) .await; assert!(ok, "sandbox create failed: {out}"); + assert!( + out.contains("tls-ok"), + "sandbox output missing expected string: {out}" + ); let (ok, out) = kubectl(&["get", "secret", "openshell-client-tls", "-n", &ns]).await; assert!( @@ -596,10 +584,7 @@ async fn managed_rejects_namespace_owned_by_different_gateway() { "--name", "conflict-sb", "--", - "sh", - "-c", - DURABLE_MAIN_SCRIPT, - "_", + "echo", "nope", ]) .await; @@ -629,10 +614,7 @@ async fn managed_full_lifecycle_with_multiple_sandboxes() { "--name", "lc-a", "--", - "sh", - "-c", - DURABLE_MAIN_SCRIPT, - "_", + "echo", "a", ]) .await; @@ -646,10 +628,7 @@ async fn managed_full_lifecycle_with_multiple_sandboxes() { "--name", "lc-b", "--", - "sh", - "-c", - DURABLE_MAIN_SCRIPT, - "_", + "echo", "b", ]) .await; @@ -727,6 +706,7 @@ async fn managed_stop_waits_for_workspace_pod_to_disappear() { &ws, "--name", sandbox, + "--detach", "--", "sh", "-c", @@ -780,10 +760,7 @@ async fn managed_rejects_invalid_dns1123_sandbox_name() { "--name", "my_bad_name", "--", - "sh", - "-c", - DURABLE_MAIN_SCRIPT, - "_", + "echo", "nope", ]) .await; @@ -804,10 +781,7 @@ async fn managed_rejects_invalid_dns1123_sandbox_name() { "--name", "MyBadName", "--", - "sh", - "-c", - DURABLE_MAIN_SCRIPT, - "_", + "echo", "nope", ]) .await; @@ -825,10 +799,7 @@ async fn managed_rejects_invalid_dns1123_sandbox_name() { "--name", "trailing-", "--", - "sh", - "-c", - DURABLE_MAIN_SCRIPT, - "_", + "echo", "nope", ]) .await; diff --git a/e2e/rust/tests/workspace_namespace_operator.rs b/e2e/rust/tests/workspace_namespace_operator.rs index 315587bacb..a874c1188b 100644 --- a/e2e/rust/tests/workspace_namespace_operator.rs +++ b/e2e/rust/tests/workspace_namespace_operator.rs @@ -18,7 +18,6 @@ use openshell_e2e::harness::output::strip_ansi; const OPERATOR_LABEL: &str = "openshell.ai/e2e-operator-workspace=true"; const SA_NAME: &str = "openshell-sandbox"; -const DURABLE_MAIN_SCRIPT: &str = r#"echo "$1"; exec sleep infinity"#; fn kube_context() -> String { std::env::var("OPENSHELL_E2E_KUBE_CONTEXT_ACTIVE") @@ -163,7 +162,7 @@ async fn operator_sandbox_in_labeled_namespace() { // Poll until the gateway's namespace watcher discovers the labeled namespace // and sandbox creation succeeds (up to 30s). let deadline = tokio::time::Instant::now() + Duration::from_secs(30); - loop { + let sandbox_out = loop { let (ok, out) = run_cli(&[ "sandbox", "create", @@ -171,23 +170,23 @@ async fn operator_sandbox_in_labeled_namespace() { &ns, "--name", "op-sb", - "--detach", "--", - "sh", - "-c", - DURABLE_MAIN_SCRIPT, - "_", + "echo", "operator-ok", ]) .await; if ok { - break; + break out; } if tokio::time::Instant::now() >= deadline { panic!("sandbox create did not succeed within 30s: {out}"); } tokio::time::sleep(Duration::from_secs(2)).await; - } + }; + assert!( + sandbox_out.contains("operator-ok"), + "sandbox output missing expected string: {sandbox_out}" + ); // Verify the sandbox CR lives in the pre-provisioned namespace. let (ok, out) = kubectl(&["get", "sandbox.agents.x-k8s.io", "-n", &ns, "-o", "name"]).await; @@ -255,10 +254,7 @@ async fn operator_rejects_unlabeled_namespace() { "--name", "should-fail", "--", - "sh", - "-c", - DURABLE_MAIN_SCRIPT, - "_", + "echo", "nope", ]) .await; @@ -294,10 +290,7 @@ async fn operator_rejects_nonexistent_namespace() { "--name", "should-fail", "--", - "sh", - "-c", - DURABLE_MAIN_SCRIPT, - "_", + "echo", "nope", ]) .await; @@ -334,10 +327,7 @@ async fn operator_workspace_delete_preserves_namespace() { "--name", "opdel-sb", "--", - "sh", - "-c", - DURABLE_MAIN_SCRIPT, - "_", + "echo", "opdel-ok", ]) .await; @@ -399,10 +389,7 @@ async fn operator_label_removal_blocks_sandbox_creation() { "--name", "lbl-sb1", "--", - "sh", - "-c", - DURABLE_MAIN_SCRIPT, - "_", + "echo", "lbl-ok", ]) .await; @@ -439,10 +426,7 @@ async fn operator_label_removal_blocks_sandbox_creation() { "--name", "lbl-sb2", "--", - "sh", - "-c", - DURABLE_MAIN_SCRIPT, - "_", + "echo", "should-fail", ]) .await; diff --git a/proto/openshell.proto b/proto/openshell.proto index 2dc70eec01..852437a78d 100644 --- a/proto/openshell.proto +++ b/proto/openshell.proto @@ -898,7 +898,8 @@ message SandboxStatus { // The gateway uses this to reject stale exit reports after a restart. string main_process_instance_id = 8; // Normalized main process result. Signal exits use 128 + signal number. - // Presence indicates that the main process exited and the sandbox is in Error. + // Presence indicates that the canonical main process exited. Exit code 0 + // produces Completed; nonzero and signal-normalized exits produce Error. optional int32 exit_code = 9; } @@ -930,6 +931,8 @@ enum SandboxPhase { SANDBOX_PHASE_STOPPING = 6; SANDBOX_PHASE_STOPPED = 7; SANDBOX_PHASE_STARTING = 8; + // The canonical main process exited successfully and its result is final. + SANDBOX_PHASE_COMPLETED = 9; } // Public platform event exposed on the sandbox watch stream. @@ -1346,7 +1349,8 @@ message WatchSandboxRequest { // Replay the last N platform events (best-effort) before following. uint32 event_tail = 6; - // Stop streaming once the sandbox reaches a terminal phase (READY or ERROR). + // Stop streaming once the sandbox reaches READY or a terminal result phase + // (COMPLETED, STOPPED, or ERROR). bool stop_on_terminal = 7; // Only include log lines with timestamp >= this value (milliseconds since epoch). diff --git a/python/openshell/sandbox.py b/python/openshell/sandbox.py index b66ed936b6..651d552a07 100644 --- a/python/openshell/sandbox.py +++ b/python/openshell/sandbox.py @@ -623,6 +623,18 @@ def _wait_for_phase( sandbox = self.get(sandbox_name, workspace=workspace) if sandbox.status.phase == target_phase: return sandbox + if ( + target_phase == openshell_pb2.SANDBOX_PHASE_READY + and sandbox.status.phase == openshell_pb2.SANDBOX_PHASE_COMPLETED + ): + return sandbox + if ( + target_phase == openshell_pb2.SANDBOX_PHASE_READY + and sandbox.status.phase == openshell_pb2.SANDBOX_PHASE_STOPPED + ): + raise SandboxError( + f"sandbox {sandbox_name} stopped before becoming ready" + ) if sandbox.status.phase == openshell_pb2.SANDBOX_PHASE_ERROR: raise SandboxError(f"sandbox {sandbox_name} entered error phase") time.sleep(1) diff --git a/python/openshell/sandbox_test.py b/python/openshell/sandbox_test.py index a059192460..eb6e1e2706 100644 --- a/python/openshell/sandbox_test.py +++ b/python/openshell/sandbox_test.py @@ -1692,6 +1692,41 @@ def test_stop_and_start_forward_workspace_and_return_phase() -> None: assert starting.phase == openshell_pb2.SANDBOX_PHASE_STARTING +@pytest.mark.parametrize( + ("phase", "should_succeed"), + [ + (openshell_pb2.SANDBOX_PHASE_COMPLETED, True), + (openshell_pb2.SANDBOX_PHASE_ERROR, False), + ], +) +def test_wait_ready_handles_terminal_main_process_results( + phase: openshell_pb2.SandboxPhase, should_succeed: bool +) -> None: + class TerminalStub(_FakeSandboxStub): + def GetSandbox( + self, + request: openshell_pb2.GetSandboxRequest, + timeout: float | None = None, + ) -> Any: + _ = timeout + return SimpleNamespace( + sandbox=_make_sandbox_proto( + "sandbox-1", + request.name, + phase=phase, + workspace=request.workspace, + ) + ) + + client = _client_with_fake_stub(TerminalStub()) + if should_succeed: + result = client.wait_ready("job-1", workspace="default", timeout_seconds=0.1) + assert result.phase == openshell_pb2.SANDBOX_PHASE_COMPLETED + else: + with pytest.raises(SandboxError, match="entered error phase"): + client.wait_ready("job-1", workspace="default", timeout_seconds=0.1) + + def test_create_without_args_sends_empty_metadata() -> None: stub = _FakeSandboxStub() client = _client_with_fake_stub(stub) diff --git a/sdk/go/openshell/v1/fake/sandbox.go b/sdk/go/openshell/v1/fake/sandbox.go index da32adf4c5..6fa0c316a9 100644 --- a/sdk/go/openshell/v1/fake/sandbox.go +++ b/sdk/go/openshell/v1/fake/sandbox.go @@ -504,8 +504,8 @@ func (c *fakeSandboxClient) WaitReady(ctx context.Context, workspace, name strin // Watch registers a watcher for sandbox events. If name is non-empty, only // events for that sandbox are delivered. When StopOnTerminal is set, the -// watcher auto-closes after delivering a terminal phase event (SandboxReady -// or SandboxError). +// watcher auto-closes after delivering a terminal phase event (SandboxReady, +// SandboxCompleted, SandboxStopped, or SandboxError). func (c *fakeSandboxClient) Watch(ctx context.Context, _, name string, opts ...v1.WatchOptions) (types.WatchInterface[*types.Sandbox], error) { if c.closedFunc() { return nil, &types.StatusError{Code: types.ErrorUnavailable, Message: "client is closed"} @@ -547,7 +547,7 @@ func (c *fakeSandboxClient) Watch(ctx context.Context, _, name string, opts ...v return } if ev.Object != nil && - (ev.Object.Status.Phase == types.SandboxReady || ev.Object.Status.Phase == types.SandboxError) { + (ev.Object.Status.Phase == types.SandboxReady || ev.Object.Status.Phase == types.SandboxCompleted || ev.Object.Status.Phase == types.SandboxStopped || ev.Object.Status.Phase == types.SandboxError) { inner.Stop() return } diff --git a/sdk/go/openshell/v1/internal/converter/sandbox.go b/sdk/go/openshell/v1/internal/converter/sandbox.go index 5d26a1a7e2..6b61053295 100644 --- a/sdk/go/openshell/v1/internal/converter/sandbox.go +++ b/sdk/go/openshell/v1/internal/converter/sandbox.go @@ -125,6 +125,8 @@ func SandboxPhaseFromProto(phase pb.SandboxPhase) types.SandboxPhase { return types.SandboxStopped case pb.SandboxPhase_SANDBOX_PHASE_STARTING: return types.SandboxStarting + case pb.SandboxPhase_SANDBOX_PHASE_COMPLETED: + return types.SandboxCompleted default: return types.SandboxUnknown } @@ -149,6 +151,8 @@ func SandboxPhaseToProto(phase types.SandboxPhase) pb.SandboxPhase { return pb.SandboxPhase_SANDBOX_PHASE_STOPPED case types.SandboxStarting: return pb.SandboxPhase_SANDBOX_PHASE_STARTING + case types.SandboxCompleted: + return pb.SandboxPhase_SANDBOX_PHASE_COMPLETED default: return pb.SandboxPhase_SANDBOX_PHASE_UNKNOWN } diff --git a/sdk/go/openshell/v1/internal/converter/sandbox_test.go b/sdk/go/openshell/v1/internal/converter/sandbox_test.go index 8293a784c0..9f68013845 100644 --- a/sdk/go/openshell/v1/internal/converter/sandbox_test.go +++ b/sdk/go/openshell/v1/internal/converter/sandbox_test.go @@ -194,6 +194,7 @@ func TestSandboxPhaseFromProto(t *testing.T) { {pb.SandboxPhase_SANDBOX_PHASE_UNKNOWN, v1.SandboxUnknown}, {pb.SandboxPhase_SANDBOX_PHASE_STOPPING, v1.SandboxStopping}, {pb.SandboxPhase_SANDBOX_PHASE_STOPPED, v1.SandboxStopped}, + {pb.SandboxPhase_SANDBOX_PHASE_COMPLETED, v1.SandboxCompleted}, {pb.SandboxPhase_SANDBOX_PHASE_STARTING, v1.SandboxStarting}, {pb.SandboxPhase_SANDBOX_PHASE_UNSPECIFIED, v1.SandboxUnknown}, {pb.SandboxPhase(999), v1.SandboxUnknown}, @@ -216,6 +217,7 @@ func TestSandboxPhaseToProto(t *testing.T) { {v1.SandboxUnknown, pb.SandboxPhase_SANDBOX_PHASE_UNKNOWN}, {v1.SandboxStopping, pb.SandboxPhase_SANDBOX_PHASE_STOPPING}, {v1.SandboxStopped, pb.SandboxPhase_SANDBOX_PHASE_STOPPED}, + {v1.SandboxCompleted, pb.SandboxPhase_SANDBOX_PHASE_COMPLETED}, {v1.SandboxStarting, pb.SandboxPhase_SANDBOX_PHASE_STARTING}, {v1.SandboxPhase("bogus"), pb.SandboxPhase_SANDBOX_PHASE_UNKNOWN}, } diff --git a/sdk/go/openshell/v1/sandbox_client.go b/sdk/go/openshell/v1/sandbox_client.go index 94d6047a01..f31cd40af9 100644 --- a/sdk/go/openshell/v1/sandbox_client.go +++ b/sdk/go/openshell/v1/sandbox_client.go @@ -214,6 +214,13 @@ func checkTerminalPhase(sb *Sandbox, name string, target SandboxPhase) (*Sandbox return sb, nil } switch sb.Status.Phase { + case SandboxCompleted: + if target == SandboxReady { + return sb, nil + } + return nil, &StatusError{Code: ErrorInternal, Message: fmt.Sprintf("sandbox %q completed before reaching %s", name, target)} + case SandboxStopped: + return nil, &StatusError{Code: ErrorInternal, Message: fmt.Sprintf("sandbox %q stopped before reaching %s", name, target)} case SandboxError: return nil, &StatusError{Code: ErrorInternal, Message: fmt.Sprintf("sandbox %q is in error state", name)} case SandboxDeleting: @@ -278,7 +285,7 @@ func (s *sandboxClient) Watch(ctx context.Context, workspace, name string, opts case <-w.done: return } - if watchOpts.StopOnTerminal && (sandbox.Status.Phase == SandboxReady || sandbox.Status.Phase == SandboxError) { + if watchOpts.StopOnTerminal && (sandbox.Status.Phase == SandboxReady || sandbox.Status.Phase == SandboxCompleted || sandbox.Status.Phase == SandboxStopped || sandbox.Status.Phase == SandboxError) { w.Stop() return } diff --git a/sdk/go/openshell/v1/sandbox_client_test.go b/sdk/go/openshell/v1/sandbox_client_test.go index c3725afad6..91e80db148 100644 --- a/sdk/go/openshell/v1/sandbox_client_test.go +++ b/sdk/go/openshell/v1/sandbox_client_test.go @@ -633,6 +633,36 @@ func TestSandboxWaitReady_SandboxFailed(t *testing.T) { require.Error(t, err) } +func TestSandboxWaitReady_SandboxCompleted(t *testing.T) { + mock := newMockSandboxServer() + mock.sandboxes["complete-sb"] = &pb.Sandbox{ + Metadata: &dm.ObjectMeta{Name: "complete-sb"}, + Status: &pb.SandboxStatus{Phase: pb.SandboxPhase_SANDBOX_PHASE_COMPLETED}, + } + client, cleanup := setupSandboxTest(t, mock) + defer cleanup() + + result, err := client.WaitReady(context.Background(), "default", "complete-sb") + + require.NoError(t, err) + assert.Equal(t, SandboxCompleted, result.Status.Phase) +} + +func TestSandboxWaitReady_SandboxStopped(t *testing.T) { + mock := newMockSandboxServer() + mock.sandboxes["stopped-sb"] = &pb.Sandbox{ + Metadata: &dm.ObjectMeta{Name: "stopped-sb"}, + Status: &pb.SandboxStatus{Phase: pb.SandboxPhase_SANDBOX_PHASE_STOPPED}, + } + client, cleanup := setupSandboxTest(t, mock) + defer cleanup() + + _, err := client.WaitReady(context.Background(), "default", "stopped-sb") + + require.Error(t, err) + assert.Contains(t, err.Error(), "stopped") +} + func TestSandboxWaitReady_SandboxDeleting(t *testing.T) { mock := newMockSandboxServer() mock.sandboxes["deleting-sb"] = &pb.Sandbox{ diff --git a/sdk/go/openshell/v1/types.go b/sdk/go/openshell/v1/types.go index dea7872a04..6226aa0707 100644 --- a/sdk/go/openshell/v1/types.go +++ b/sdk/go/openshell/v1/types.go @@ -20,6 +20,7 @@ const ( SandboxStopping = types.SandboxStopping SandboxStopped = types.SandboxStopped SandboxStarting = types.SandboxStarting + SandboxCompleted = types.SandboxCompleted ) // EventType classifies watch events. diff --git a/sdk/go/openshell/v1/types/types.go b/sdk/go/openshell/v1/types/types.go index 4e3b830805..5b32ec3685 100644 --- a/sdk/go/openshell/v1/types/types.go +++ b/sdk/go/openshell/v1/types/types.go @@ -18,6 +18,7 @@ const ( SandboxStopping SandboxPhase = "Stopping" SandboxStopped SandboxPhase = "Stopped" SandboxStarting SandboxPhase = "Starting" + SandboxCompleted SandboxPhase = "Completed" ) // EventType classifies watch events. diff --git a/sdk/go/proto/openshellv1/openshell.pb.go b/sdk/go/proto/openshellv1/openshell.pb.go index 38e54f419e..0ff29a7302 100644 --- a/sdk/go/proto/openshellv1/openshell.pb.go +++ b/sdk/go/proto/openshellv1/openshell.pb.go @@ -44,6 +44,8 @@ const ( SandboxPhase_SANDBOX_PHASE_STOPPING SandboxPhase = 6 SandboxPhase_SANDBOX_PHASE_STOPPED SandboxPhase = 7 SandboxPhase_SANDBOX_PHASE_STARTING SandboxPhase = 8 + // The canonical main process exited successfully and its result is final. + SandboxPhase_SANDBOX_PHASE_COMPLETED SandboxPhase = 9 ) // Enum value maps for SandboxPhase. @@ -58,6 +60,7 @@ var ( 6: "SANDBOX_PHASE_STOPPING", 7: "SANDBOX_PHASE_STOPPED", 8: "SANDBOX_PHASE_STARTING", + 9: "SANDBOX_PHASE_COMPLETED", } SandboxPhase_value = map[string]int32{ "SANDBOX_PHASE_UNSPECIFIED": 0, @@ -69,6 +72,7 @@ var ( "SANDBOX_PHASE_STOPPING": 6, "SANDBOX_PHASE_STOPPED": 7, "SANDBOX_PHASE_STARTING": 8, + "SANDBOX_PHASE_COMPLETED": 9, } ) @@ -1449,7 +1453,8 @@ type SandboxStatus struct { // The gateway uses this to reject stale exit reports after a restart. MainProcessInstanceId string `protobuf:"bytes,8,opt,name=main_process_instance_id,json=mainProcessInstanceId,proto3" json:"main_process_instance_id,omitempty"` // Normalized main process result. Signal exits use 128 + signal number. - // Presence indicates that the main process exited and the sandbox is in Error. + // Presence indicates that the canonical main process exited. Exit code 0 + // produces Completed; nonzero and signal-normalized exits produce Error. ExitCode *int32 `protobuf:"varint,9,opt,name=exit_code,json=exitCode,proto3,oneof" json:"exit_code,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache @@ -4173,7 +4178,8 @@ type WatchSandboxRequest struct { LogTailLines uint32 `protobuf:"varint,5,opt,name=log_tail_lines,json=logTailLines,proto3" json:"log_tail_lines,omitempty"` // Replay the last N platform events (best-effort) before following. EventTail uint32 `protobuf:"varint,6,opt,name=event_tail,json=eventTail,proto3" json:"event_tail,omitempty"` - // Stop streaming once the sandbox reaches a terminal phase (READY or ERROR). + // Stop streaming once the sandbox reaches READY or a terminal result phase + // (COMPLETED, STOPPED, or ERROR). StopOnTerminal bool `protobuf:"varint,7,opt,name=stop_on_terminal,json=stopOnTerminal,proto3" json:"stop_on_terminal,omitempty"` // Only include log lines with timestamp >= this value (milliseconds since epoch). // 0 means no time filter. Applies to both tail replay and live streaming. @@ -14337,7 +14343,7 @@ const file_openshell_proto_rawDesc = "" + "\x1aExtensionServiceCredential\x12!\n" + "\fservice_name\x18\x01 \x01(\tR\vserviceName\x12\x1a\n" + "\x05token\x18\x02 \x01(\tB\x04\x88\xb5\x18\x01R\x05token\x12\"\n" + - "\rexpires_at_ms\x18\x03 \x01(\x03R\vexpiresAtMs*\x89\x02\n" + + "\rexpires_at_ms\x18\x03 \x01(\x03R\vexpiresAtMs*\xa6\x02\n" + "\fSandboxPhase\x12\x1d\n" + "\x19SANDBOX_PHASE_UNSPECIFIED\x10\x00\x12\x1e\n" + "\x1aSANDBOX_PHASE_PROVISIONING\x10\x01\x12\x17\n" + @@ -14347,7 +14353,8 @@ const file_openshell_proto_rawDesc = "" + "\x15SANDBOX_PHASE_UNKNOWN\x10\x05\x12\x1a\n" + "\x16SANDBOX_PHASE_STOPPING\x10\x06\x12\x19\n" + "\x15SANDBOX_PHASE_STOPPED\x10\a\x12\x1a\n" + - "\x16SANDBOX_PHASE_STARTING\x10\b*\xc3\x03\n" + + "\x16SANDBOX_PHASE_STARTING\x10\b\x12\x1b\n" + + "\x17SANDBOX_PHASE_COMPLETED\x10\t*\xc3\x03\n" + "!ProviderCredentialRefreshStrategy\x124\n" + "0PROVIDER_CREDENTIAL_REFRESH_STRATEGY_UNSPECIFIED\x10\x00\x12/\n" + "+PROVIDER_CREDENTIAL_REFRESH_STRATEGY_STATIC\x10\x01\x121\n" + diff --git a/sdk/typescript/src/client.test.ts b/sdk/typescript/src/client.test.ts index 1c08e423ca..29a7438c69 100644 --- a/sdk/typescript/src/client.test.ts +++ b/sdk/typescript/src/client.test.ts @@ -291,6 +291,30 @@ describe('create', () => { }); describe('waits', () => { + it('waitReady accepts successful main-process completion', async () => { + const sandbox = client({ + getSandbox: () => ({ + sandbox: { + metadata: { id: 'sb-id', name: 'sb' }, + status: { phase: SandboxPhase.COMPLETED, exitCode: 0 }, + }, + }), + }); + await expect(sandbox.waitReady('sb', 1)).resolves.toMatchObject({ phase: 'completed', exitCode: 0 }); + }); + + it('waitReady rejects stopped main-process results without waiting for timeout', async () => { + const sandbox = client({ + getSandbox: () => ({ + sandbox: { + metadata: { id: 'sb-id', name: 'sb' }, + status: { phase: SandboxPhase.STOPPED, exitCode: 7 }, + }, + }), + }); + await expect(sandbox.waitReady('sb', 30)).rejects.toMatchObject({ code: 'connect' }); + }); + it('waitReady rejects rather than hanging when get() never resolves', async () => { const sandbox = client({ // Only settles when the per-poll deadline signal aborts the call. diff --git a/sdk/typescript/src/client.ts b/sdk/typescript/src/client.ts index b6db97943f..3f3247ca2a 100644 --- a/sdk/typescript/src/client.ts +++ b/sdk/typescript/src/client.ts @@ -59,7 +59,8 @@ export type SandboxPhaseName = | 'unknown' | 'stopping' | 'stopped' - | 'starting'; + | 'starting' + | 'completed'; /** Lowercase mirror of the generated `ServiceStatus` enum. Hand-maintained. */ export type HealthStatus = 'unspecified' | 'healthy' | 'degraded' | 'unhealthy'; @@ -293,6 +294,7 @@ export const PHASE_NAMES: Record = { [SandboxPhase.STOPPING]: 'stopping', [SandboxPhase.STOPPED]: 'stopped', [SandboxPhase.STARTING]: 'starting', + [SandboxPhase.COMPLETED]: 'completed', }; export const STATUS_NAMES: Record = { [ServiceStatus.UNSPECIFIED]: 'unspecified', @@ -632,7 +634,8 @@ export class SandboxClient { } catch (e) { throw mapWaitError(e, name, deadline, signal); } - if (ref.phase === 'ready') return ref; + if (ref.phase === 'ready' || ref.phase === 'completed') return ref; + if (ref.phase === 'stopped') throw new SdkError('connect', `sandbox '${name}' stopped before becoming ready`); if (ref.phase === 'error') throw new SdkError('connect', `sandbox '${name}' entered error phase`); if (Date.now() >= deadline) throw new SdkError('connect', `timed out waiting for sandbox '${name}'`); await waitSleep(delay, deadline, signal);