fix(sandbox): fence terminal delivery and restarts

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

Author: drew

.agents/skills/openshell-cli/SKILL.md

@@ -358,8 +358,9 @@ Both commands default to the last-used sandbox. Stop stops background
 forwards and waits for `Stopped`; start waits for `Ready`. Connect, exec,
 file transfer, forwarding, and exposed services are unavailable while
 stopped or completed. Starting a retained `Completed` or
-`Error/MainProcessFailed` sandbox launches a fresh canonical-main instance.
-Delete remains the operation that removes retained state.
+`Error/MainProcessFailed` sandbox launches a fresh canonical-main instance and
+invalidates SSH sessions from the previous runtime generation. Delete remains
+the operation that removes retained state.
 
 ---
 

.agents/skills/openshell-cli/cli-reference.md

@@ -211,6 +211,8 @@ 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`.
+Starting either retained terminal result invalidates SSH sessions from the
+previous runtime generation.
 
 | Flag | Description |
 |------|-------------|

architecture/compute-runtimes.md

@@ -141,7 +141,9 @@ 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.
+restart a completed or failed canonical process. Before an explicit restart,
+the gateway disconnects the prior supervisor session and deletes its SSH
+sessions so credentials cannot cross runtime generations.
 
 `StopSandbox` and `StartSandbox` are idempotent driver operations. Stop
 retains the driver resource and its persistent workspace boundary while making

architecture/gateway.md

@@ -29,8 +29,9 @@ 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. 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.
+session alive until an attached foreground client drains output to the terminal
+event or a bounded detached timeout expires. It durably reports the result,
+sends the SSH exit status, and only then releases deferred ephemeral cleanup.
 
 ## Protocol and Auth
 

architecture/sandbox.md

@@ -447,7 +447,8 @@ engine with a gateway policy revision.
 - 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, the supervisor drains its retained main
-  output and reports the normalized result before shutdown. Exit code 0 records
+  output, durably reports the normalized result, sends the SSH exit status, and
+  releases deferred ephemeral cleanup 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

crates/openshell-cli/src/run.rs

@@ -222,6 +222,23 @@ fn sandbox_should_persist(keep: bool, forward: Option<&ForwardSpec>) -> bool {
     keep || forward.is_some()
 }
 
+fn has_main_process_result(sandbox: &Sandbox) -> bool {
+    let Some(status) = sandbox.status.as_ref() else {
+        return false;
+    };
+    if status.exit_code.is_none() {
+        return false;
+    }
+
+    let phase = SandboxPhase::try_from(sandbox.phase()).unwrap_or(SandboxPhase::Unknown);
+    phase != SandboxPhase::Error
+        || status.conditions.iter().any(|condition| {
+            condition.r#type == "Ready"
+                && condition.status.eq_ignore_ascii_case("false")
+                && condition.reason == "MainProcessFailed"
+        })
+}
+
 fn build_sandbox_resource_limits(
     cpu: Option<&str>,
     memory: Option<&str>,
@@ -761,14 +778,11 @@ pub async fn sandbox_create(
                     saw_non_ready = true;
                 }
 
-                let has_main_process_result = s
-                    .status
-                    .as_ref()
-                    .is_some_and(|status| status.exit_code.is_some());
+                let main_process_result = has_main_process_result(&s);
                 if matches!(
                     phase,
                     SandboxPhase::Completed | SandboxPhase::Error | SandboxPhase::Stopped
-                ) && has_main_process_result
+                ) && main_process_result
                 {
                     if let Some(d) = display.as_interactive_mut() {
                         d.clear();
@@ -856,10 +870,7 @@ 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);
-    let final_has_main_process_result = last_sandbox
-        .status
-        .as_ref()
-        .is_some_and(|status| status.exit_code.is_some());
+    let final_has_main_process_result = has_main_process_result(&last_sandbox);
     if !(matches!(
         final_phase,
         SandboxPhase::Ready | SandboxPhase::Completed | SandboxPhase::Stopped
@@ -7206,13 +7217,14 @@ mod tests {
     use super::{
         PolicyGetView, ProvisioningStep, build_sandbox_resource_limits,
         dockerfile_sources_supported_for_gateway, format_endpoint,
-        format_provider_attachment_table, git_sync_files, inferred_provider_type,
-        parse_cli_setting_value, parse_credential_expiry_cli_value, parse_credential_expiry_pairs,
-        parse_credential_pairs, parse_driver_config_json, parse_secret_material_env_pairs,
-        policy_revision_to_json, provider_profile_allows_empty_credentials,
-        provisioning_timeout_message, ready_false_condition_message, refresh_status_header,
-        refresh_status_row, resolve_from, sandbox_should_persist, sandbox_upload_plan,
-        service_expose_status_error, service_url_for_gateway,
+        format_provider_attachment_table, git_sync_files, has_main_process_result,
+        inferred_provider_type, parse_cli_setting_value, parse_credential_expiry_cli_value,
+        parse_credential_expiry_pairs, parse_credential_pairs, parse_driver_config_json,
+        parse_secret_material_env_pairs, policy_revision_to_json,
+        provider_profile_allows_empty_credentials, provisioning_timeout_message,
+        ready_false_condition_message, refresh_status_header, refresh_status_row, resolve_from,
+        sandbox_should_persist, sandbox_upload_plan, service_expose_status_error,
+        service_url_for_gateway,
     };
     use crate::TEST_ENV_LOCK;
     use crate::commands::common::progress_step_from_metadata;
@@ -7751,6 +7763,48 @@ mod tests {
         assert!(sandbox_should_persist(false, Some(&spec)));
     }
 
+    #[test]
+    fn infrastructure_error_with_observed_exit_is_not_a_main_process_result() {
+        let mut sandbox = Sandbox {
+            status: Some(SandboxStatus {
+                exit_code: Some(137),
+                conditions: vec![SandboxCondition {
+                    r#type: "Ready".to_string(),
+                    status: "False".to_string(),
+                    reason: "ComputeResourceMissing".to_string(),
+                    message: "sandbox runtime disappeared".to_string(),
+                    ..Default::default()
+                }],
+                ..Default::default()
+            }),
+            ..Default::default()
+        };
+        sandbox.set_phase(SandboxPhase::Error as i32);
+
+        assert!(!has_main_process_result(&sandbox));
+    }
+
+    #[test]
+    fn main_process_failed_condition_identifies_command_result() {
+        let mut sandbox = Sandbox {
+            status: Some(SandboxStatus {
+                exit_code: Some(7),
+                conditions: vec![SandboxCondition {
+                    r#type: "Ready".to_string(),
+                    status: "False".to_string(),
+                    reason: "MainProcessFailed".to_string(),
+                    message: "canonical main process exited with status 7".to_string(),
+                    ..Default::default()
+                }],
+                ..Default::default()
+            }),
+            ..Default::default()
+        };
+        sandbox.set_phase(SandboxPhase::Error as i32);
+
+        assert!(has_main_process_result(&sandbox));
+    }
+
     #[test]
     fn resolve_from_classifies_existing_dockerfile_path() {
         let temp = tempfile::tempdir().expect("failed to create tempdir");

crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs

@@ -47,6 +47,7 @@ struct SandboxState {
     deleted_names: Arc<Mutex<Vec<Vec<String>>>>,
     create_requests: Arc<Mutex<Vec<CreateSandboxRequest>>>,
     vm_error_after_started: Arc<AtomicBool>,
+    vm_error_with_observed_exit: Arc<AtomicBool>,
     vm_slow_progress_before_ready: Arc<AtomicBool>,
     vm_log_churn_before_ready: Arc<AtomicBool>,
     global_settings: Arc<Mutex<HashMap<String, SettingValue>>>,
@@ -401,6 +402,10 @@ impl OpenShell for TestOpenShell {
         let sandbox_id = request.into_inner().id;
         let (tx, rx) = mpsc::channel(4);
         let vm_error_after_started = self.state.vm_error_after_started.load(Ordering::SeqCst);
+        let vm_error_with_observed_exit = self
+            .state
+            .vm_error_with_observed_exit
+            .load(Ordering::SeqCst);
         let vm_slow_progress_before_ready = self
             .state
             .vm_slow_progress_before_ready
@@ -437,6 +442,9 @@ impl OpenShell for TestOpenShell {
                 ..provisioning.clone()
             };
             error.set_phase(SandboxPhase::Error as i32);
+            if vm_error_with_observed_exit {
+                error.status.as_mut().unwrap().exit_code = Some(137);
+            }
             let mut ready = provisioning.clone();
             ready.set_phase(SandboxPhase::Ready as i32);
 
@@ -1550,6 +1558,44 @@ async fn sandbox_create_returns_vm_error_without_waiting_for_timeout() {
     assert!(!rendered.contains("timed out"));
 }
 
+#[tokio::test]
+async fn sandbox_create_preserves_vm_error_when_exit_code_is_observed() {
+    let server = run_server().await;
+    server
+        .openshell
+        .state
+        .vm_error_after_started
+        .store(true, Ordering::SeqCst);
+    server
+        .openshell
+        .state
+        .vm_error_with_observed_exit
+        .store(true, Ordering::SeqCst);
+    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_fake_ssh(&fake_ssh_dir);
+
+    let err = run::sandbox_create(
+        &server.endpoint,
+        "openshell",
+        run::SandboxCreateConfig {
+            name: Some("vm-error-with-exit"),
+            command: &["echo".into(), "OK".into()],
+            ..test_config()
+        },
+        "default",
+        &tls,
+    )
+    .await
+    .expect_err("an observed process exit must not hide the infrastructure error");
+
+    let rendered = err.to_string();
+    assert!(rendered.contains("sandbox entered error phase while provisioning"));
+    assert!(rendered.contains("ProcessExited: VM process exited with status 0"));
+}
+
 #[tokio::test]
 async fn sandbox_create_keeps_waiting_while_vm_progress_arrives() {
     let server = run_server().await;

crates/openshell-sandbox/src/lib.rs

@@ -866,13 +866,16 @@ pub async fn run_sandbox(
                     openshell_supervisor_process::run::SidecarExitReport,
                 >(1);
                 tokio::spawn(async move {
-                    while let Some((instance_id, exit_code, ack)) = rx.recv().await {
+                    while let Some((instance_id, exit_code, defer_ephemeral_cleanup, ack)) =
+                        rx.recv().await
+                    {
                         let (durable_tx, durable_rx) = tokio::sync::oneshot::channel();
                         *exit_ack.lock().await = Some((instance_id.clone(), durable_tx));
                         let result = match sidecar_control::send_main_process_exited(
                             &writer,
                             instance_id,
                             exit_code,
+                            defer_ephemeral_cleanup,
                         )
                         .await
                         {
@@ -1177,7 +1180,6 @@ fn spawn_sidecar_entrypoint_handler(
         let terminating = Arc::new(AtomicBool::new(false));
         while let Some(started) = entrypoint_rx.recv().await {
             if let Some(exit_code) = started.exit_code {
-                terminating.store(true, Ordering::Release);
                 if let (Some(endpoint), Some(id)) =
                     (openshell_endpoint.as_ref(), sandbox_id.as_ref())
                 {
@@ -1188,6 +1190,7 @@ fn spawn_sidecar_entrypoint_handler(
                             id,
                             &started.instance_id,
                             exit_code,
+                            started.defer_ephemeral_cleanup,
                         )
                         .await
                         {
@@ -1203,6 +1206,10 @@ fn spawn_sidecar_entrypoint_handler(
                         publisher.publish_main_process_exit_ack(started.instance_id.clone());
                     }
                 }
+                if started.defer_ephemeral_cleanup {
+                    continue;
+                }
+                terminating.store(true, Ordering::Release);
                 break;
             }
             entrypoint_pid.store(started.pid, Ordering::Release);