fix(services): clean up timed out command groups

bin/base-demo-services

@@ -359,10 +359,19 @@ def terminate_process_group(
     process_group_id: int,
     process: subprocess.Popen[Any] | None = None,
 ) -> bool:
+    def group_is_running() -> bool:
+        try:
+            os.killpg(process_group_id, 0)
+        except ProcessLookupError:
+            return False
+        except PermissionError:
+            return True
+        return True
+
     def stopped() -> bool:
-        if process is not None:
-            return process.poll() is not None
-        return not process_is_running(pid)
+        if process is not None and process.poll() is None:
+            return False
+        return not group_is_running()
 
     try:
         os.killpg(process_group_id, signal.SIGTERM)
@@ -385,6 +394,36 @@ def terminate_process_group(
     return stopped()
 
 
+def run_command_check(command: list[str], root: Path, label: str) -> CheckResult:
+    process: subprocess.Popen[bytes] | None = None
+    try:
+        process = subprocess.Popen(
+            command,
+            cwd=root,
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE,
+            start_new_session=True,
+        )
+        try:
+            process.communicate(timeout=3.0)
+        except subprocess.TimeoutExpired as exc:
+            stopped = terminate_process_group(process.pid, process.pid, process)
+            try:
+                process.communicate(timeout=2.0)
+            except subprocess.TimeoutExpired:
+                stopped = False
+            detail = f"{label} ({exc})"
+            if not stopped:
+                detail = f"{detail}; process group cleanup failed"
+            return CheckResult(False, label, detail)
+    except OSError as exc:
+        return CheckResult(False, label, f"{label} ({exc})")
+
+    if process is None:
+        return CheckResult(False, label, f"{label} (process did not start)")
+    return CheckResult(process.returncode == 0, label, label)
+
+
 def readiness_timeout(service: dict[str, Any]) -> float:
     lifecycle = service.get("lifecycle") or {}
     configured = os.environ.get("BASE_DEMO_SERVICES_START_TIMEOUT")
@@ -743,18 +782,7 @@ def check_service(
         if not isinstance(command, list) or not all(isinstance(item, str) for item in command):
             return CheckResult(False, "command:<invalid command>", "command:<invalid command>")
         label = f"command:{shlex.join(command)}"
-        try:
-            result = subprocess.run(
-                command,
-                cwd=root,
-                check=False,
-                capture_output=True,
-                text=False,
-                timeout=3.0,
-            )
-        except (OSError, subprocess.TimeoutExpired) as exc:
-            return CheckResult(False, label, f"{label} ({exc})")
-        return CheckResult(result.returncode == 0, label, label)
+        return run_command_check(command, root, label)
 
     detail = f"unsupported:{check_type}"
     return CheckResult(False, detail, detail)

tests/services_test.bats

@@ -1020,3 +1020,80 @@ EOF
   wait "$UNRELATED_PID" 2>/dev/null || true
   UNRELATED_PID=""
 }
+
+@test "services command timeouts terminate owned descendants" {
+  local catalog="$TEST_TMPDIR/catalog.json"
+  local command="$TEST_TMPDIR/timeout-command.py"
+  local detached_marker="$TEST_TMPDIR/detached-marker"
+  local inherited_marker="$TEST_TMPDIR/inherited-marker"
+
+  cat > "$command" <<'PY'
+#!/usr/bin/env python3
+import pathlib
+import subprocess
+import sys
+import time
+
+marker, mode = sys.argv[1:]
+child = [
+    sys.executable,
+    "-c",
+    "import pathlib,sys,time; time.sleep(4); pathlib.Path(sys.argv[1]).touch()",
+    marker,
+]
+kwargs = {}
+if mode == "detached":
+    kwargs = {"stdout": subprocess.DEVNULL, "stderr": subprocess.DEVNULL}
+subprocess.Popen(child, **kwargs)
+time.sleep(10)
+PY
+  chmod +x "$command"
+
+  python3 - "$catalog" "$command" "$detached_marker" "$inherited_marker" <<'PY'
+import json
+import sys
+
+catalog, command, detached_marker, inherited_marker = sys.argv[1:]
+with open(catalog, "w", encoding="utf-8") as handle:
+    json.dump(
+        {
+            "services": [
+                {
+                    "name": "timeout-detached",
+                    "kind": "service",
+                    "runtime": "test",
+                    "required": True,
+                    "check": {"type": "command", "command": [command, detached_marker, "detached"]},
+                },
+                {
+                    "name": "timeout-inherited",
+                    "kind": "service",
+                    "runtime": "test",
+                    "required": False,
+                    "check": {"type": "command", "command": [command, inherited_marker, "inherited"]},
+                },
+                {
+                    "name": "later-service",
+                    "kind": "service",
+                    "runtime": "test",
+                    "required": False,
+                    "check": {"type": "none"},
+                },
+            ]
+        },
+        handle,
+    )
+PY
+
+  run "$TEST_ROOT/bin/base-demo-services" --catalog "$catalog" check
+
+  [ "$status" -eq 1 ]
+  [[ "$output" == *"timeout-detached fail command:"* ]]
+  [[ "$output" == *"timeout-inherited skip optional command:"* ]]
+  [[ "$output" == *"later-service ok"* ]]
+  [[ "$output" != *"process group cleanup failed"* ]]
+
+  sleep 5
+  [ ! -e "$detached_marker" ]
+  [ ! -e "$inherited_marker" ]
+}