From cb84e7d94e77dcfe022c653dd007d9861e72debd Mon Sep 17 00:00:00 2001
From: teknium1 <127238744+teknium1@users.noreply.github.com>
Date: Mon, 14 Sep 2026 20:54:29 -0700
Subject: [PATCH] fix(codex): import kill_process_tree so the SIGTERM-timeout
 path actually kills

close() escalated to kill_process_tree() but never imported it; the NameError
was swallowed by contextlib.suppress, so on the TimeoutExpired path neither the
kill nor the post-kill wait ran and a root codex ignoring SIGTERM leaked (a
regression vs the previous self._proc.kill()). Import it from agent.deadline
and add a test forcing the timeout path that asserts the tree kill and the
follow-up wait both run. Also drop the upstream product reference from the
test docstring (credit stays in the PR body) and pass encoding= to the PID
file reads flagged by the Windows footgun scanner.
---
 agent/transports/codex_app_server.py          |  1 +
 .../test_codex_app_server_runtime.py          | 37 +++++++++++++++----
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/agent/transports/codex_app_server.py b/agent/transports/codex_app_server.py
index 7d7d1c38e4..1681605aa9 100644
--- a/agent/transports/codex_app_server.py
+++ b/agent/transports/codex_app_server.py
@@ -17,6 +17,7 @@ import threading
 from dataclasses import dataclass
 from typing import Any, Optional
 
+from agent.deadline import kill_process_tree
 from tools.environments.local import hermes_subprocess_env
 
 MIN_CODEX_VERSION = (0, 125, 0)
diff --git a/tests/agent/transports/test_codex_app_server_runtime.py b/tests/agent/transports/test_codex_app_server_runtime.py
index 92a675ca25..b3cae82c3a 100644
--- a/tests/agent/transports/test_codex_app_server_runtime.py
+++ b/tests/agent/transports/test_codex_app_server_runtime.py
@@ -121,11 +121,10 @@ class TestCodexAppServerClose:
     def test_close_reaps_independent_descendant_process_group(self, tmp_path):
         """A Codex-owned MCP child that calls setsid must not survive close().
 
-        Regression ported from openclaw/openclaw#126285: killing only the
-        app-server root lets independently grouped stdio MCP descendants live
-        past client retirement. The fake codex binary below spawns a long-lived
-        child in its own session, records its PID, and exits promptly on root
-        SIGTERM; close() must still reap the child.
+        Killing only the app-server root lets independently grouped stdio MCP
+        descendants live past client retirement. The fake codex binary below
+        spawns a long-lived child in its own session, records its PID, and exits
+        promptly on root SIGTERM; close() must still reap the child.
         """
         import os
         import stat
@@ -173,7 +172,7 @@ while True:
             while not child_pid_file.exists() and time.time() < deadline:
                 time.sleep(0.05)
             assert child_pid_file.exists(), "fake codex did not report child PID"
-            child_pid = int(child_pid_file.read_text())
+            child_pid = int(child_pid_file.read_text(encoding="utf-8"))
             assert psutil.pid_exists(child_pid)
 
             client.close(timeout=1.0)
@@ -189,10 +188,34 @@ while True:
             client.close(timeout=0.1)
             if child_pid_file.exists():
                 try:
-                    os.kill(int(child_pid_file.read_text()), 9)
+                    os.kill(int(child_pid_file.read_text(encoding="utf-8")), 9)
                 except ProcessLookupError:
                     pass
 
+    def test_close_escalates_to_tree_kill_when_root_ignores_sigterm(self, monkeypatch):
+        """When the root outlives the graceful wait, close() must kill the tree AND
+        run the post-kill wait so a codex ignoring SIGTERM cannot leak."""
+        import subprocess
+        from unittest import mock
+
+        from agent.transports import codex_app_server as mod
+
+        proc = mock.MagicMock()
+        proc.pid = 4242
+        proc.stdin = None
+        proc.wait.side_effect = [subprocess.TimeoutExpired(cmd="codex", timeout=0.01), 0]
+        killed: list[int] = []
+        monkeypatch.setattr(mod, "kill_process_tree", lambda pid, **kw: killed.append(pid) or True)
+        monkeypatch.setattr(mod, "_snapshot_descendants", lambda pid: [])
+
+        client = mod.CodexAppServerClient.__new__(mod.CodexAppServerClient)
+        client._proc = proc
+        client._closed = False
+        client.close(timeout=0.01)
+
+        assert killed == [4242]
+        assert proc.wait.call_args_list == [mock.call(timeout=0.01), mock.call(timeout=1.0)]
+
 
 class TestSpawnEnvIsolation:
     """The codex spawn must NOT rewrite HOME — codex's shell tool spawns