From cf644be09d0dfcf1fa168f1e7343f7cc70a97a23 Mon Sep 17 00:00:00 2001
From: ogt <ogt@awoooi.local>
Date: Tue, 14 Jul 2026 18:34:44 +0800
Subject: [PATCH] fix(agent): kill timed out ansible process groups

---
 .../awooop_ansible_check_mode_service.py      | 17 ++++-
 .../tests/test_awooop_truth_chain_service.py  | 62 ++++++++++++++++++-
 2 files changed, 75 insertions(+), 4 deletions(-)

diff --git a/apps/api/src/services/awooop_ansible_check_mode_service.py b/apps/api/src/services/awooop_ansible_check_mode_service.py
index c38a2fdad..776949b45 100644
--- a/apps/api/src/services/awooop_ansible_check_mode_service.py
+++ b/apps/api/src/services/awooop_ansible_check_mode_service.py
@@ -12,6 +12,7 @@ import json
 import os
 import re
 import shutil
+import signal
 import time
 from collections.abc import Mapping
 from dataclasses import dataclass, replace
@@ -632,10 +633,22 @@ async def _run_ansible_command(spec: AnsibleCommandSpec, *, timeout_seconds: int
         *spec.command,
         cwd=str(spec.cwd),
         env=spec.env,
+        start_new_session=True,
         stdin=asyncio.subprocess.DEVNULL,
         stdout=asyncio.subprocess.PIPE,
         stderr=asyncio.subprocess.PIPE,
     )
+
+    def kill_process_group() -> None:
+        if process.returncode is not None:
+            return
+        try:
+            os.killpg(process.pid, signal.SIGKILL)
+        except ProcessLookupError:
+            return
+        except PermissionError:
+            process.kill()
+
     timed_out = False
     try:
         stdout_bytes, stderr_bytes = await asyncio.wait_for(
@@ -644,10 +657,10 @@ async def _run_ansible_command(spec: AnsibleCommandSpec, *, timeout_seconds: int
         )
     except TimeoutError:
         timed_out = True
-        process.kill()
+        kill_process_group()
         stdout_bytes, stderr_bytes = await process.communicate()
     except asyncio.CancelledError:
-        process.kill()
+        kill_process_group()
         await process.communicate()
         raise
     duration_ms = int((time.monotonic() - started) * 1000)
diff --git a/apps/api/tests/test_awooop_truth_chain_service.py b/apps/api/tests/test_awooop_truth_chain_service.py
index da7eeed76..dd154f1c0 100644
--- a/apps/api/tests/test_awooop_truth_chain_service.py
+++ b/apps/api/tests/test_awooop_truth_chain_service.py
@@ -1,7 +1,9 @@
 from __future__ import annotations
 
+import asyncio
 import inspect
 import os
+import signal
 from datetime import UTC, datetime, timedelta
 from pathlib import Path
 from types import SimpleNamespace
@@ -3158,15 +3160,71 @@ async def test_controlled_apply_duplicate_never_runs_subprocess(
     assert claim.input_payload["existing_apply_status"] == "success"
 
 
-def test_ansible_subprocess_is_terminated_when_worker_task_is_cancelled() -> None:
+def test_ansible_subprocess_group_is_terminated_on_timeout_or_cancel() -> None:
     source = inspect.getsource(_run_ansible_command)
 
+    assert "start_new_session=True" in source
     assert "stdin=asyncio.subprocess.DEVNULL" in source
+    assert "os.killpg(process.pid, signal.SIGKILL)" in source
+    assert "except TimeoutError" in source
     assert "except asyncio.CancelledError" in source
-    assert "process.kill()" in source
     assert "await process.communicate()" in source
 
 
+@pytest.mark.asyncio
+async def test_ansible_subprocess_timeout_kills_isolated_process_group(
+    monkeypatch,
+    tmp_path: Path,
+) -> None:
+    class FakeProcess:
+        pid = 4242
+        returncode = None
+
+        def __init__(self) -> None:
+            self.communicate_calls = 0
+
+        async def communicate(self) -> tuple[bytes, bytes]:
+            self.communicate_calls += 1
+            if self.returncode is None:
+                await asyncio.sleep(60)
+            return b"", b""
+
+        def kill(self) -> None:
+            raise AssertionError("isolated process group must be killed")
+
+    fake_process = FakeProcess()
+    create_kwargs: dict[str, object] = {}
+    killpg_calls: list[tuple[int, signal.Signals]] = []
+
+    async def fake_create_subprocess_exec(*_args, **kwargs):
+        create_kwargs.update(kwargs)
+        return fake_process
+
+    def fake_killpg(pid: int, requested_signal: signal.Signals) -> None:
+        killpg_calls.append((pid, requested_signal))
+        fake_process.returncode = -int(requested_signal)
+
+    monkeypatch.setattr(
+        "src.services.awooop_ansible_check_mode_service.asyncio.create_subprocess_exec",
+        fake_create_subprocess_exec,
+    )
+    monkeypatch.setattr(
+        "src.services.awooop_ansible_check_mode_service.os.killpg",
+        fake_killpg,
+    )
+
+    result = await _run_ansible_command(
+        SimpleNamespace(command=["ansible-playbook"], cwd=tmp_path, env={}),
+        timeout_seconds=0.001,
+    )
+
+    assert create_kwargs["start_new_session"] is True
+    assert killpg_calls == [(4242, signal.SIGKILL)]
+    assert fake_process.communicate_calls == 2
+    assert result.returncode == 124
+    assert result.timed_out is True
+
+
 def test_controlled_apply_shutdown_marks_pending_row_before_cancelling() -> None:
     source = inspect.getsource(run_controlled_apply_for_claim)