fix(kubernetes): prevent stop-start relay race (#3064)

Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>

Author: johntmyers

crates/openshell-driver-kubernetes/README.md

@@ -79,9 +79,9 @@ its pod. The driver sets `spec.operatingMode: Suspended` for `v1beta1` or
 `spec.replicas: 0` for `v1alpha1`. Start sets `Running` or one replica for the
 same resource, so the replacement pod mounts the existing claim. Delete is the
 only lifecycle operation that removes the Sandbox resource and its owned
-storage. The driver confirms the stop from the published `Suspended`
-condition when available. Legacy `v1alpha1` controllers omit a zero replica
-count from status, so the driver confirms that their backing pod is gone.
+storage. The driver confirms the stop from both the published `Suspended`
+condition and deletion of the backing pod. Legacy `v1alpha1` controllers omit
+a usable stopped condition, so pod deletion alone confirms their stop.
 
 The workspace PVC size defaults to `workspace_default_storage_size`. Set
 `workspace_storage_class` to pin the PVC to a specific `StorageClass`; an empty

crates/openshell-driver-kubernetes/src/driver.rs

@@ -1623,8 +1623,7 @@ impl KubernetesComputeDriver {
         let (agent_sandbox_api, kube_name, pod_name, namespace, stop_timeout) = self
             .patch_sandbox_operating_state(sandbox_id, false)
             .await?;
-        let legacy_pod_api = (agent_sandbox_api.resource.version == SANDBOX_VERSION_V1ALPHA1)
-            .then(|| Api::<Pod>::namespaced(self.client.clone(), &namespace));
+        let pod_api = Api::<Pod>::namespaced(self.client.clone(), &namespace);
 
         let deadline = tokio::time::Instant::now() + stop_timeout;
         let mut poll_interval = STOP_INITIAL_POLL_INTERVAL;
@@ -1649,17 +1648,18 @@ impl KubernetesComputeDriver {
                 ))
             })?
             .map_err(KubernetesDriverError::from_kube)?;
-            if kubernetes_sandbox_has_stopped_condition(&object) {
-                return Ok(());
-            }
             if let Some(error) = kubernetes_sandbox_stop_failure(&object) {
                 return Err(KubernetesDriverError::Message(error));
             }
-            if let Some(pod_api) = legacy_pod_api.as_ref()
-                && kubernetes_sandbox_pod_is_gone(pod_api, &pod_name, deadline)
-                    .await
-                    .map_err(KubernetesDriverError::Message)?
-            {
+            let pod_is_gone = kubernetes_sandbox_pod_is_gone(&pod_api, &pod_name, deadline)
+                .await
+                .map_err(KubernetesDriverError::Message)?;
+            let stop_is_complete = kubernetes_sandbox_stop_is_complete(
+                &agent_sandbox_api.resource.version,
+                &object,
+                pod_is_gone,
+            );
+            if stop_is_complete {
                 return Ok(());
             }
             let now = tokio::time::Instant::now();
@@ -4560,6 +4560,19 @@ fn kubernetes_sandbox_has_stopped_condition(obj: &DynamicObject) -> bool {
         })
 }
 
+fn kubernetes_sandbox_stop_is_complete(
+    api_version: &str,
+    obj: &DynamicObject,
+    pod_is_gone: bool,
+) -> bool {
+    if api_version == SANDBOX_VERSION_V1ALPHA1 {
+        // v1alpha1 omits a usable stopped condition.
+        pod_is_gone
+    } else {
+        kubernetes_sandbox_has_stopped_condition(obj) && pod_is_gone
+    }
+}
+
 fn kubernetes_sandbox_stop_failure(obj: &DynamicObject) -> Option<String> {
     obj.data
         .get("status")?
@@ -5392,6 +5405,43 @@ mod tests {
         assert!(kubernetes_sandbox_has_stopped_condition(&sandbox));
     }
 
+    #[test]
+    fn beta_stop_requires_suspended_condition_and_deleted_pod() {
+        let resource = ApiResource::from_gvk(&GroupVersionKind::gvk(
+            SANDBOX_GROUP,
+            SANDBOX_VERSION_V1BETA1,
+            SANDBOX_KIND,
+        ));
+        let mut sandbox = DynamicObject::new("sandbox", &resource);
+
+        assert!(!kubernetes_sandbox_stop_is_complete(
+            SANDBOX_VERSION_V1BETA1,
+            &sandbox,
+            true,
+        ));
+
+        sandbox.data = serde_json::json!({
+            "status": {
+                "conditions": [{"type": "Suspended", "status": "True"}]
+            }
+        });
+        assert!(!kubernetes_sandbox_stop_is_complete(
+            SANDBOX_VERSION_V1BETA1,
+            &sandbox,
+            false,
+        ));
+        assert!(kubernetes_sandbox_stop_is_complete(
+            SANDBOX_VERSION_V1BETA1,
+            &sandbox,
+            true,
+        ));
+        assert!(kubernetes_sandbox_stop_is_complete(
+            SANDBOX_VERSION_V1ALPHA1,
+            &DynamicObject::new("sandbox", &resource),
+            true,
+        ));
+    }
+
     #[test]
     fn stop_failure_only_rejects_terminal_suspension_condition() {
         let resource = ApiResource::from_gvk(&GroupVersionKind::gvk(

crates/openshell-server/src/compute/mod.rs

@@ -3183,6 +3183,17 @@ impl ComputeRuntime {
         else {
             return Ok(());
         };
+        let phase = SandboxPhase::try_from(sandbox.phase()).unwrap_or(SandboxPhase::Unknown);
+        if matches!(
+            phase,
+            SandboxPhase::Deleting | SandboxPhase::Stopping | SandboxPhase::Stopped
+        ) {
+            // Lifecycle shutdown intentionally discards the canonical process
+            // result. Finalization must still acknowledge that discarded
+            // result so the supervisor can exit before the compute backend's
+            // termination grace period expires.
+            return Ok(());
+        }
         let Some(status) = sandbox.status.as_ref() else {
             return Err("main-process exit has not been reported".to_string());
         };
@@ -5600,6 +5611,10 @@ mod tests {
                 .main_process_exited(id, "instance-1", 143)
                 .await
                 .unwrap();
+            runtime
+                .finalize_main_process_exit(id, "instance-1")
+                .await
+                .expect("intentional shutdown finalization should be acknowledged");
 
             let stored = runtime
                 .store

docs/reference/sandbox-compute-drivers.mdx

@@ -436,7 +436,9 @@ The Kubernetes driver creates namespaced `agents.x-k8s.io` `Sandbox` resources f
 Stop patches the existing resource rather than deleting it. For `v1beta1`,
 the driver sets `spec.operatingMode` to `Suspended` or `Running`. For
 `v1alpha1`, it sets `spec.replicas` to `0` or `1`. The Sandbox resource and its
-workspace PVC keep their identity across both operations.
+workspace PVC keep their identity across both operations. Stop returns only
+after the controller reports suspension and deletes the old pod, so an
+immediate start cannot race the prior pod's termination.
 
 If Agent Sandbox is upgraded in place, restart the OpenShell gateway after the controller and CRD rollout completes so the gateway can detect the served API versions again.