fix(server): consistent sandbox access after the main process exits

crates/openshell-cli/src/run.rs

@@ -2013,8 +2013,9 @@ pub async fn sandbox_exec_grpc(
         .sandbox
         .ok_or_else(|| miette::miette!("sandbox not found"))?;
 
-    // Verify the sandbox is ready before issuing the exec.
-    if SandboxPhase::try_from(sandbox.phase()) != Ok(SandboxPhase::Ready) {
+    // Reject phases where the relay can never be open. The gateway decides
+    // whether a Completed or Error sandbox still has a reachable supervisor.
+    if !relay_phase_may_be_reachable(sandbox.phase()) {
         return Err(miette::miette!(
             "sandbox '{}' is not ready (phase: {}); wait for it to reach Ready state",
             name,
@@ -2168,7 +2169,7 @@ pub async fn service_forward_tcp(
     let (bind_addr, bind_port) = parse_tcp_forward_spec(local, target_port)?;
     let mut client = grpc_client(server, tls).await?;
 
-    let mut replica = fetch_ready_sandbox_for_forward(&mut client, name, workspace).await?;
+    let mut replica = fetch_reachable_sandbox_for_forward(&mut client, name, workspace).await?;
 
     let listener = tokio::net::TcpListener::bind((bind_addr.as_str(), bind_port))
         .await
@@ -2202,7 +2203,7 @@ pub async fn service_forward_tcp(
             }
 
             _ = health_check.tick() => {
-                replica = fetch_ready_sandbox_for_forward(&mut client, name, workspace).await?;
+                replica = fetch_reachable_sandbox_for_forward(&mut client, name, workspace).await?;
             }
 
             accepted = listener.accept() => {
@@ -2268,8 +2269,18 @@ fn forward_requires_session_token(status: &Status) -> bool {
         && status.message().contains("authorization_token is required")
 }
 
-/// Confirm the sandbox is still ready, returning the replica that owns it.
-async fn fetch_ready_sandbox_for_forward(
+/// Whether the gateway may still relay into a sandbox in this phase. A
+/// Completed or Error sandbox keeps its access plane while its supervisor
+/// stays connected; only the gateway can tell whether it still is.
+fn relay_phase_may_be_reachable(phase: i32) -> bool {
+    matches!(
+        SandboxPhase::try_from(phase),
+        Ok(SandboxPhase::Ready | SandboxPhase::Completed | SandboxPhase::Error)
+    )
+}
+
+/// Confirm the sandbox is still reachable, returning the replica that owns it.
+async fn fetch_reachable_sandbox_for_forward(
     client: &mut crate::tls::GrpcClient,
     name: &str,
     workspace: &str,
@@ -2298,7 +2309,7 @@ async fn fetch_ready_sandbox_for_forward(
         .sandbox
         .ok_or_else(|| miette::miette!("sandbox '{name}' not found"))?;
 
-    if SandboxPhase::try_from(sandbox.phase()) != Ok(SandboxPhase::Ready) {
+    if !relay_phase_may_be_reachable(sandbox.phase()) {
         return Err(miette::miette!(
             "sandbox '{}' is no longer ready (phase: {}); stopping service forward",
             name,

crates/openshell-cli/tests/sandbox_exec_streaming_integration.rs

@@ -46,6 +46,7 @@ struct Calls {
 #[derive(Clone)]
 struct MockGateway {
     scenario: Scenario,
+    phase: proto::SandboxPhase,
     calls: Arc<Mutex<Calls>>,
 }
 
@@ -189,7 +190,7 @@ macro_rules! mock_gateway {
                             ..Default::default()
                         }),
                         status: Some(proto::SandboxStatus {
-                            phase: proto::SandboxPhase::Ready.into(),
+                            phase: self.phase.into(),
                             ..Default::default()
                         }),
                         ..Default::default()
@@ -324,6 +325,10 @@ impl Drop for TestGateway {
 
 impl TestGateway {
     async fn start(scenario: Scenario) -> Self {
+        Self::start_in_phase(scenario, proto::SandboxPhase::Ready).await
+    }
+
+    async fn start_in_phase(scenario: Scenario, phase: proto::SandboxPhase) -> Self {
         let (ca, ca_key) = build_ca();
         let (server_cert, server_key) = build_server_cert(&ca, &ca_key);
         let (client_cert, client_key) = build_client_cert(&ca, &ca_key);
@@ -344,6 +349,7 @@ impl TestGateway {
         let calls = Arc::new(Mutex::new(Calls::default()));
         let service = MockGateway {
             scenario,
+            phase,
             calls: Arc::clone(&calls),
         };
         let task = tokio::spawn(async move {
@@ -476,6 +482,39 @@ async fn default_remote_exit_does_not_wait_for_idle_open_stdin() {
     drop(held_open);
 }
 
+#[tokio::test]
+async fn exec_reaches_gateway_for_completed_and_error_sandboxes() {
+    // The supervisor keeps its access plane after the main process exits, so
+    // the gateway, not the CLI, decides whether the relay is still reachable.
+    for phase in [proto::SandboxPhase::Completed, proto::SandboxPhase::Error] {
+        let gateway = TestGateway::start_in_phase(Scenario::EarlyExit, phase).await;
+        let mut child = gateway.spawn();
+        let held_open = child.stdin.take().unwrap();
+        let output = finish(child).await;
+        drop(held_open);
+        assert!(
+            output.status.success(),
+            "{phase:?}: {}",
+            String::from_utf8_lossy(&output.stderr)
+        );
+        assert_eq!(output.stdout, b"before-exit\n");
+        gateway.assert_one_stream();
+    }
+}
+
+#[tokio::test]
+async fn exec_is_refused_before_starting_for_stopped_sandbox() {
+    let gateway =
+        TestGateway::start_in_phase(Scenario::EarlyExit, proto::SandboxPhase::Stopped).await;
+    let mut child = gateway.spawn();
+    drop(child.stdin.take());
+    let output = finish(child).await;
+    assert!(!output.status.success());
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    assert!(stderr.contains("is not ready (phase: Stopped)"), "{stderr}");
+    assert!(gateway.calls.lock().unwrap().starts.is_empty());
+}
+
 #[tokio::test]
 async fn default_open_pipe_checks_trailer_error_after_exit() {
     let gateway = TestGateway::start(Scenario::ErrorAfterExit).await;

crates/openshell-conformance/src/lib.rs

@@ -27,7 +27,7 @@ pub use scenarios::{
     FILE_TRANSFER_GIT_FILTERING_SCENARIO, FILE_TRANSFER_PATH_SAFETY_SCENARIO,
     FILE_TRANSFER_ROUND_TRIP_SCENARIO, MECHANISTIC_PROPOSAL_SCENARIO,
     NEW_HOSTNAME_PROPOSAL_SCENARIO, POLICY_LOCAL_SCENARIO, SANDBOX_LIFECYCLE_SCENARIO,
-    SMOKE_SCENARIO,
+    SANDBOX_RETENTION_SCENARIO, SMOKE_SCENARIO,
 };
 
 /// A portable conformance scenario exercised by the Cargo test suite.

crates/openshell-conformance/src/scenarios/mod.rs

@@ -15,5 +15,5 @@ pub use file_transfer::{
 pub use policy_behavior::{
     MECHANISTIC_PROPOSAL_SCENARIO, NEW_HOSTNAME_PROPOSAL_SCENARIO, POLICY_LOCAL_SCENARIO,
 };
-pub use sandbox_lifecycle::SANDBOX_LIFECYCLE_SCENARIO;
+pub use sandbox_lifecycle::{SANDBOX_LIFECYCLE_SCENARIO, SANDBOX_RETENTION_SCENARIO};
 pub use smoke::SMOKE_SCENARIO;

crates/openshell-conformance/src/scenarios/sandbox_lifecycle.rs

@@ -20,6 +20,8 @@ struct SandboxState {
     id: String,
     name: String,
     phase: String,
+    #[serde(default)]
+    exit_code: Option<i32>,
 }
 
 #[derive(Debug, Deserialize)]
@@ -41,6 +43,124 @@ fn run_sandbox_lifecycle(runner: &mut OpenShellRunner) -> ScenarioFuture<'_> {
     })
 }
 
+/// Certify what happens to a sandbox after its main process exits.
+///
+/// An ephemeral sandbox is deleted whether the main process succeeds or
+/// fails. A retained sandbox records the result and stays reachable through
+/// exec, because the supervisor keeps its access plane after the exit.
+pub const SANDBOX_RETENTION_SCENARIO: Scenario = Scenario {
+    name: "sandbox-retention",
+    run: run_sandbox_retention,
+};
+
+fn run_sandbox_retention(runner: &mut OpenShellRunner) -> ScenarioFuture<'_> {
+    Box::pin(async move {
+        for exit_code in [0, 3] {
+            ephemeral_is_deleted_after_exit(runner, exit_code).await?;
+            retained_accepts_exec_after_exit(runner, exit_code).await?;
+        }
+        Ok(())
+    })
+}
+
+async fn ephemeral_is_deleted_after_exit(
+    runner: &mut OpenShellRunner,
+    exit_code: i32,
+) -> Result<(), String> {
+    let sandbox_name = format!("ct-{}-e{exit_code}", runner.id());
+    let step = format!("ephemeral-exit-{exit_code}");
+    runner.track_sandbox(&sandbox_name);
+    let create = runner
+        .step(format!("{step}/create"))
+        .description(format!(
+            "ephemeral sandbox '{sandbox_name}' runs its main process and returns exit code {exit_code}"
+        ))
+        .with_timeout(CREATE_TIMEOUT)
+        .run(&[
+            "sandbox",
+            "create",
+            "--name",
+            &sandbox_name,
+            "--no-keep",
+            "--no-tty",
+            "--",
+            "sh",
+            "-c",
+            &format!("echo ephemeral-done; exit {exit_code}"),
+        ])
+        .await
+        .map_err(|error| error.to_string())?;
+    if create.exit_code() != Some(exit_code) {
+        return Err(create.failure_diagnostic(&format!("create exits with code {exit_code}")));
+    }
+    if !create.stdout().contains("ephemeral-done") {
+        return Err(create.failure_diagnostic("main process stdout is streamed to the caller"));
+    }
+
+    wait_for_name_absence(runner, &sandbox_name, &format!("{step}/deleted")).await?;
+    runner.forget_sandbox(&sandbox_name);
+    Ok(())
+}
+
+async fn retained_accepts_exec_after_exit(
+    runner: &mut OpenShellRunner,
+    exit_code: i32,
+) -> Result<(), String> {
+    let sandbox_name = format!("ct-{}-r{exit_code}", runner.id());
+    let step = format!("retained-exit-{exit_code}");
+    let result_path = "/sandbox/.openshell-retained-result";
+    let result = format!("openshell-retained-{}-{exit_code}", runner.id());
+    runner.track_sandbox(&sandbox_name);
+    let create = runner
+        .step(format!("{step}/create"))
+        .description(format!("retained sandbox '{sandbox_name}' is created"))
+        .with_timeout(CREATE_TIMEOUT)
+        .run(&[
+            "sandbox",
+            "create",
+            "--name",
+            &sandbox_name,
+            "--detach",
+            "--no-tty",
+            "--",
+            "sh",
+            "-c",
+            &format!("printf '%s\\n' '{result}' > '{result_path}' && sync; exit {exit_code}"),
+        ])
+        .await
+        .map_err(|error| error.to_string())?;
+    create.require_success()?;
+
+    let expected_phase = if exit_code == 0 { "Completed" } else { "Error" };
+    let state = wait_for_phase(
+        runner,
+        &sandbox_name,
+        expected_phase,
+        &format!("{step}/{}", expected_phase.to_lowercase()),
+    )
+    .await?;
+    if state.exit_code != Some(exit_code) {
+        return Err(format!(
+            "sandbox '{sandbox_name}' recorded exit code {:?}; expected {exit_code}",
+            state.exit_code
+        ));
+    }
+
+    exec_expect_exact(
+        runner,
+        &sandbox_name,
+        &format!("{step}/exec-after-exit"),
+        &["cat", result_path],
+        &format!("{result}\n"),
+    )
+    .await?;
+
+    run_lifecycle_command(runner, "delete", &sandbox_name, &format!("{step}/delete")).await?;
+    wait_for_absence(runner, &state.id, &sandbox_name, &format!("{step}/deleted")).await?;
+    runner.forget_sandbox(&sandbox_name);
+    Ok(())
+}
+
 async fn stop_start_preserves_workspace(runner: &mut OpenShellRunner) -> Result<(), String> {
     let sandbox_name = format!("ct-{}-ss", runner.id());
     let sentinel = format!("openshell-stop-start-{}", runner.id());
@@ -56,7 +176,7 @@ async fn stop_start_preserves_workspace(runner: &mut OpenShellRunner) -> Result<
     exec_expect_exact(
         runner,
         &sandbox_name,
-        "write-sentinel",
+        "stop-start/write-sentinel",
         &[
             "sh",
             "-lc",
@@ -99,15 +219,15 @@ async fn stop_start_preserves_workspace(runner: &mut OpenShellRunner) -> Result<
     exec_expect_exact(
         runner,
         &sandbox_name,
-        "read-sentinel",
+        "stop-start/read-sentinel",
         &["cat", sentinel_path],
         &format!("{sentinel}\n"),
     )
     .await?;
     exec_expect_exact(
         runner,
         &sandbox_name,
-        "read-main-run-count",
+        "stop-start/read-main-run-count",
         &["cat", run_count_path],
         "2\n",
     )
@@ -190,7 +310,7 @@ async fn exec_expect_exact(
     let mut args = vec!["sandbox", "exec", "--name", sandbox_name, "--no-tty", "--"];
     args.extend_from_slice(command);
     let result = runner
-        .step(format!("stop-start/{step}"))
+        .step(step)
         .description(format!("sandbox '{sandbox_name}' exec {step} succeeds"))
         .with_timeout(COMMAND_TIMEOUT)
         .run(&args)
@@ -283,6 +403,51 @@ async fn wait_for_absence(
         .map_err(|error| error.to_string())
 }
 
+/// Wait until `sandbox get` reports that the name no longer exists. Used
+/// when the gateway deletes the sandbox on its own, so the caller may never
+/// observe its ID.
+async fn wait_for_name_absence(
+    runner: &mut OpenShellRunner,
+    sandbox_name: &str,
+    step: &str,
+) -> Result<(), String> {
+    let sandbox_name = sandbox_name.to_string();
+    let step = step.to_string();
+    let poll_step = step.clone();
+    runner
+        .poll_until(
+            &poll_step,
+            TRANSITION_TIMEOUT,
+            TRANSITION_INTERVAL,
+            async move |runner| {
+                let result = runner
+                    .step(format!("{step}/get"))
+                    .description(format!("sandbox '{sandbox_name}' no longer exists"))
+                    .with_timeout(COMMAND_TIMEOUT)
+                    .run(&["sandbox", "get", &sandbox_name, "--output", "json"])
+                    .await;
+                match result {
+                    Ok(result) if result.success() => match result.json::<SandboxState>() {
+                        Ok(state) => Poll::Pending(format!(
+                            "sandbox '{sandbox_name}' still exists in phase {:?}",
+                            state.phase
+                        )),
+                        Err(error) => Poll::Failed(error.to_string()),
+                    },
+                    Ok(result) if result.stderr().to_lowercase().contains("not found") => {
+                        Poll::Ready(())
+                    }
+                    Ok(result) => Poll::Pending(result.failure_diagnostic(&format!(
+                        "sandbox get reports that '{sandbox_name}' was not found"
+                    ))),
+                    Err(error) => Poll::Pending(error.to_string()),
+                }
+            },
+        )
+        .await
+        .map_err(|error| error.to_string())
+}
+
 async fn sandbox_is_listed(
     runner: &OpenShellRunner,
     sandbox_id: &str,