fix(supervisor): stop recovering a lost boundary after SIGTERM

crates/openshell-driver-docker/README.md

@@ -128,8 +128,8 @@ New generations require matching supervisor and sandbox runtime binaries.
 
 ## Stop, Start, and Delete
 
-Stop terminates the supervisor companion and stops the workload container
-without removing it. Docker retains the workload writable layer and attached
+Stop terminates the supervisor companion, allowing it 10 seconds to stop the
+workload, and stops the workload container without removing it. Docker retains the workload writable layer and attached
 volumes. Start stages a fresh sandbox bootstrap bundle, restarts that workload,
 and creates a new supervisor companion. A durably stopped sandbox stays stopped
 across gateway restarts.

crates/openshell-driver-docker/src/lib.rs

@@ -95,6 +95,9 @@ const WATCH_POLL_MAX_BACKOFF: Duration = Duration::from_secs(30);
 // request a bounded opportunity to mark the control exit intentional before
 // publishing a fail-closed runtime error.
 const SUPERVISOR_INTENTIONAL_SHUTDOWN_GRACE: Duration = Duration::from_secs(1);
+// Longer than the supervisor's own bound on stopping its workload, so it
+// finishes stopping before Docker kills it.
+const SUPERVISOR_STOP_TIMEOUT_SECS: i32 = 10;
 const SUPERVISOR_HEALTH_INTERVAL_NS: i64 = 30_000_000_000;
 // Docker probes at this interval only before the first healthy result and
 // within the start period.
@@ -5607,7 +5610,7 @@ async fn spawn_docker_control_process(
             _ = &mut shutdown_requested => {
                 let _ = monitored_docker.stop_container(
                     &monitored_supervisor_id,
-                    Some(StopContainerOptionsBuilder::default().t(5).build()),
+                    Some(StopContainerOptionsBuilder::default().t(SUPERVISOR_STOP_TIMEOUT_SECS).build()),
                 ).await;
                 let _ = remove_supervisor().await;
             }

crates/openshell-sandbox-backend/src/runtime.rs

@@ -958,6 +958,12 @@ impl RunningBoundary {
         expect_response(response, "boundary_terminated")
     }
 
+    /// Stop waiting for a lost boundary connection to recover. The owner is
+    /// stopping, and the boundary may be stopping with it.
+    pub fn begin_stop(&self) {
+        self.process.client.stopping.store(true, Ordering::Release);
+    }
+
     /// Whether the negotiated backend advertised suspend/resume support.
     pub fn supports_suspend(&self) -> bool {
         self.process
@@ -1601,6 +1607,9 @@ struct BoundaryClient {
     recovery_timeout_secs: Option<u32>,
     /// True once a connection of this client was confirmed.
     confirmed: tokio::sync::watch::Sender<bool>,
+    /// Set when the owner stops the boundary; a lost connection is then
+    /// reported instead of recovered.
+    stopping: AtomicBool,
 }
 
 #[derive(Clone)]
@@ -1800,6 +1809,7 @@ impl BoundaryClient {
             requires_takeover: false,
             recovery_timeout_secs: None,
             confirmed: tokio::sync::watch::Sender::new(false),
+            stopping: AtomicBool::new(false),
         }
     }
 
@@ -2208,6 +2218,11 @@ impl BoundaryClient {
             }
             cached.transport.abort();
         }
+        if self.stopping.load(Ordering::Acquire) {
+            return Err(BackendError::Unavailable(
+                "boundary connection lost while stopping".to_string(),
+            ));
+        }
 
         *self.mediation.lock().await = None;
         let attach = self
@@ -3970,6 +3985,72 @@ mod tests {
         assert_eq!(requests.load(Ordering::Acquire), 5);
     }
 
+    #[tokio::test]
+    async fn stopping_client_reports_a_lost_boundary_without_recovery() {
+        let certificate = test_certificate();
+        let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
+        let address = listener.local_addr().unwrap();
+        let server_config = certificate.server_config.clone();
+        let server = tokio::spawn(async move {
+            let (stream, _) = listener.accept().await.unwrap();
+            let stream = tokio_rustls::TlsAcceptor::from(server_config)
+                .accept(stream)
+                .await
+                .unwrap();
+            let service = TestGrpcBoundary {
+                wait_for_half_close: false,
+                expected_token: "a".repeat(32),
+                requests: Arc::default(),
+                mediation_failures: Arc::new(std::sync::atomic::AtomicUsize::new(0)),
+                mediation_ready: false,
+                provider_environment_generation: 0,
+                confirmation: test_confirmation(),
+                attach_snapshots: Arc::default(),
+                received: Arc::default(),
+            };
+            tonic::transport::Server::builder()
+                .add_service(IsolationBackendServer::new(TestIsolationBackend::default()))
+                .add_service(SandboxProtocolServer::new(service))
+                .serve_with_incoming(tokio_stream::iter([Ok::<_, std::io::Error>(TestTlsIo(
+                    Box::new(stream),
+                ))]))
+                .await
+                .unwrap();
+        });
+        let client = BoundaryClient::new(
+            tls_runtime_descriptor(address, certificate.client_tls),
+            test_bearer(&"a".repeat(32)),
+        );
+        let attach = Request::Attach {
+            supervisor_instance_id: client.supervisor_instance_id,
+            policy: Box::new(SandboxPolicyWire::from(sandbox().policy)),
+            supervisor_takeover: None,
+            supervisor_recovery_timeout_secs: None,
+        };
+        client.call_idempotent(attach).await.unwrap();
+        client.call_idempotent(Request::Confirm).await.unwrap();
+
+        // The boundary stops with its owner: no listener remains to reattach.
+        server.abort();
+        let _ = server.await;
+        client.stopping.store(true, Ordering::Release);
+        client
+            .grpc_channel
+            .lock()
+            .await
+            .as_ref()
+            .expect("cached channel")
+            .transport
+            .abort();
+        let result = tokio::time::timeout(
+            Duration::from_secs(5),
+            client.call_idempotent(Request::TerminateBoundary),
+        )
+        .await
+        .expect("a stopping client must not wait for the boundary to recover");
+        assert!(matches!(result, Err(BackendError::Unavailable(_))));
+    }
+
     #[test]
     fn wait_recovery_window_begins_at_transport_failure() {
         let wait_started = tokio::time::Instant::now();

crates/openshell-supervisor/README.md

@@ -57,7 +57,10 @@ loads the launch configuration from its arguments and environment and prepares
 push and the OCSF JSON settings with its logging layers, so renewal by either
 gateway client keeps both current. `run_sandbox_with_backend` sets the
 process-wide OCSF context, binds the readiness socket and port to instance
-readiness, and stops the instance on SIGTERM or SIGINT.
+readiness, and stops the instance on SIGTERM or SIGINT. Stopping gives the
+workload 5 seconds to exit after TERM and finishes, including the exit report
+and trace flush, within 8 seconds. A boundary connection lost while the
+process stops is not recovered; the workload is reported as terminated.
 
 `sandbox_instance_host::SandboxInstanceHost` runs several sandbox instances in
 one process. Each instance gets its own Tokio runtime; every thread of that

crates/openshell-supervisor/src/lib.rs

@@ -504,6 +504,17 @@ use provider_readiness::{EnvironmentIdentity, Tracker as ProviderReadinessTracke
 use tokio::sync::mpsc::UnboundedSender;
 use tokio::time::timeout;
 
+/// Time the workload gets to exit after TERM before it is killed.
+const WORKLOAD_TERM_GRACE: Duration = Duration::from_secs(5);
+
+/// Bound on stopping an instance, including its exit report and trace flush.
+/// Below the 10 second grace the Docker driver gives the supervisor.
+const SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(8);
+
+/// Exit code reported when the boundary is lost before it reports the
+/// stopped workload's status: terminated by TERM.
+const STOPPED_EXIT_CODE: i32 = 128 + 15;
+
 /// Whether a dedicated supervisor's environment grants the SSH socket to its
 /// group rather than its owner.
 #[must_use]
@@ -836,6 +847,7 @@ pub async fn run_sandbox_with_backend(
         bindings,
         SandboxInstanceControls {
             shutdown: Box::pin(wait_for_control_shutdown_signal()),
+            shutdown_ends_process: true,
             suspend,
             readiness,
         },
@@ -999,6 +1011,9 @@ pub struct SandboxInstanceControls {
     /// Completes when the owner asks the instance to stop its workload. It is
     /// first polled once the workload has started.
     pub shutdown: Pin<Box<dyn Future<Output = ()> + Send>>,
+    /// True when `shutdown` also ends this process, so its boundary may be
+    /// stopping too and a lost connection is not worth recovering.
+    pub shutdown_ends_process: bool,
     /// Requests to suspend only this instance. A failed suspension can be
     /// retried by sending another request. Closing the stream disables it.
     pub suspend: Pin<Box<dyn futures::Stream<Item = ()> + Send>>,
@@ -1110,6 +1125,7 @@ pub async fn run_sandbox_instance(
     } = bindings;
     let SandboxInstanceControls {
         shutdown: shutdown_requested,
+        shutdown_ends_process,
         mut suspend,
         readiness,
     } = controls;
@@ -1781,6 +1797,7 @@ pub async fn run_sandbox_instance(
             SandboxInstanceReadiness::start(readiness, boundary_access.session_readiness());
         let instance_id = boundary_access.instance_id().to_string();
         let mut shutdown_requested = shutdown_requested;
+        let mut shutdown_deadline = None;
         // `None` means the boundary was suspended and its reference persisted.
         let lifecycle = loop {
             let wait_agent = agent.clone();
@@ -1800,23 +1817,42 @@ pub async fn run_sandbox_instance(
                     ));
                 }
                 () = &mut shutdown_requested => {
-                    let _ = agent
-                        .signal(openshell_isolation_interface::contract::BoundarySignal::Term)
-                        .await;
-                    let status = if let Ok(result) = timeout(Duration::from_secs(5), agent.wait()).await {
-                        result
-                    } else {
-                        let _ = agent.terminate().await;
-                        agent.wait().await
+                    let deadline = tokio::time::Instant::now() + SHUTDOWN_TIMEOUT;
+                    shutdown_deadline = Some(deadline);
+                    if shutdown_ends_process {
+                        running.begin_stop();
                     }
-                    .map_err(|error| miette::miette!(error.to_string()))?;
-                    running
-                        .terminate()
-                        .await
-                        .map_err(|error| miette::miette!(
-                            "sandbox did not acknowledge terminal state: {error}"
-                        ))?;
-                    break Some((boundary_exit_code(status), false));
+                    let stop = async {
+                        let _ = agent
+                            .signal(openshell_isolation_interface::contract::BoundarySignal::Term)
+                            .await;
+                        let status = if let Ok(result) = timeout(WORKLOAD_TERM_GRACE, agent.wait()).await {
+                            result
+                        } else {
+                            let _ = agent.terminate().await;
+                            agent.wait().await
+                        }?;
+                        running.terminate().await?;
+                        Ok(status)
+                    };
+                    let exit_code = match tokio::time::timeout_at(deadline, stop).await {
+                        Ok(Ok(status)) => boundary_exit_code(status),
+                        Ok(Err(
+                            openshell_isolation_interface::contract::BackendError::Unavailable(error)
+                            | openshell_isolation_interface::contract::BackendError::Terminated(error),
+                        )) => {
+                            warn!(%error, "Isolation boundary lost while stopping; finishing shutdown");
+                            STOPPED_EXIT_CODE
+                        }
+                        Ok(Err(error)) => {
+                            return Err(miette::miette!("stop sandbox workload: {error}"));
+                        }
+                        Err(_) => {
+                            warn!("Isolation boundary did not stop in time; finishing shutdown");
+                            STOPPED_EXIT_CODE
+                        }
+                    };
+                    break Some((exit_code, false));
                 }
                 // The agent wait is cancelled while suspending so a fenced
                 // generation's stream failure cannot race the suspension.
@@ -1853,19 +1889,27 @@ pub async fn run_sandbox_instance(
         if !retain_access {
             instance_readiness.release();
         }
-        boundary_access
-            .publish_main_exit(exit_code, await_main_process_attachment)
-            .await;
+        let publish = boundary_access.publish_main_exit(exit_code, await_main_process_attachment);
+        if let Some(deadline) = shutdown_deadline {
+            let _ = tokio::time::timeout_at(deadline, publish).await;
+        } else {
+            publish.await;
+        }
+        let flush_timeout = shutdown_deadline.map_or(otlp_relay::FLUSH_TIMEOUT, |deadline| {
+            deadline
+                .saturating_duration_since(tokio::time::Instant::now())
+                .min(otlp_relay::FLUSH_TIMEOUT)
+        });
         // Flush relayed agent traces before the exit is reported so the last
         // spans of a short run are not lost. Bounded; never fails. When the
         // access plane is retained for exec sessions, the relay keeps serving
         // them and shuts down at final teardown instead.
         if retain_access {
             if let Some(relay) = otlp_relay.as_ref() {
-                relay.flush(otlp_relay::FLUSH_TIMEOUT).await;
+                relay.flush(flush_timeout).await;
             }
         } else if let Some(relay) = otlp_relay.take() {
-            relay.shutdown(otlp_relay::FLUSH_TIMEOUT).await;
+            relay.shutdown(flush_timeout).await;
         }
         // `shutdown_requested` has already completed when shutdown won the
         // lifecycle select above and must not be polled again.

crates/openshell-supervisor/src/sandbox_instance_host.rs

@@ -689,6 +689,7 @@ fn run_hosted_sandbox_instance(
                     });
                     let instance = body(SandboxInstanceControls {
                         shutdown,
+                        shutdown_ends_process: false,
                         suspend: Box::pin(tokio_stream::wrappers::ReceiverStream::new(suspend_requested)),
                         readiness,
                     });

tests/suites/conformance/src/lifecycle.rs

@@ -52,11 +52,7 @@ const DELETE_DEADLINE: Duration = Duration::from_secs(10);
 /// Many workloads do not exit immediately on TERM, for example while they run
 /// cleanup handlers. Deleting such a running sandbox should still complete
 /// promptly, rather than waiting out the runtime's termination grace period.
-//
-// Unignore this when the supervisor half of #5025 lands: on Podman the
-// supervisor container still waits out its stop timeout.
 #[testkit::test]
-#[ignore = "https://github.com/NVIDIA/OpenShell/issues/5025"]
 async fn delete_is_prompt_when_workload_is_slow_to_handle_term(
     cx: &mut Context,
 ) -> testkit::Result {