fix: kill surviving children on stop, taskkill fallback on Windows, bounded log read; fix bad-merge default-model tests

Co-Authored-By: Claude Opus 5.5 <[email protected]>

src/praisonai-agents/praisonaiagents/tools/shell_tools.py

@@ -367,7 +367,7 @@ class ShellTools:
             with open(entry["log_path"], "rb") as f:
                 f.seek(0, os.SEEK_END)
                 f.seek(max(0, f.tell() - max_bytes))
-                data = f.read().decode("utf-8", errors="replace")
+                data = f.read(max_bytes).decode("utf-8", errors="replace")
         except OSError as e:
             data = f"<unable to read output: {e}>"
         lines = data.splitlines()[-tail:] if tail > 0 else []
@@ -409,15 +409,18 @@ class ShellTools:
             import signal
             try:
                 os.killpg(process.pid, signal.SIGTERM)
-                process.wait(timeout=5)
-                return
-            except subprocess.TimeoutExpired:
-                try:
-                    os.killpg(process.pid, signal.SIGKILL)
-                except ProcessLookupError:
-                    pass
             except ProcessLookupError:
                 pass
+            try:
+                process.wait(timeout=5)
+            except subprocess.TimeoutExpired:
+                pass
+            # The parent exiting does not mean its children did: SIGKILL
+            # whatever is left in the group so nothing outlives stop_command.
+            try:
+                os.killpg(process.pid, signal.SIGKILL)
+            except (ProcessLookupError, PermissionError):
+                pass
         else:
             self._kill_process_tree(process)
         try:
@@ -455,7 +458,8 @@ class ShellTools:
         """Kill a child process and its descendants (best-effort).
 
         Uses psutil to reach the whole tree when available, falling back to
-        ``process.kill()`` otherwise. Safe to call if the process already exited.
+        ``taskkill /T`` on Windows or ``process.kill()`` otherwise. Safe to
+        call if the process already exited.
         """
         try:
             import psutil
@@ -469,6 +473,12 @@ class ShellTools:
             parent.kill()
         except ImportError:
             try:
+                if platform.system() == 'Windows':
+                    subprocess.run(
+                        ['taskkill', '/F', '/T', '/PID', str(process.pid)],
+                        stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL,
+                        timeout=10,
+                    )
                 process.kill()
             except Exception:
                 pass

src/praisonai-agents/tests/unit/tools/test_shell_background.py

@@ -94,3 +94,35 @@ def test_exported_from_tools_package():
 
     assert callable(tools.get_command_output)
     assert callable(tools.stop_command)
+
+
+def test_stop_kills_children_that_ignore_sigterm(tmp_path):
+    """Parent exits on SIGTERM; a child ignoring it must still be killed."""
+    import os
+
+    pid_file = tmp_path / "child.pid"
+    child = "import signal, time; signal.signal(signal.SIGTERM, signal.SIG_IGN); time.sleep(60)"
+    script = tmp_path / "parent.py"
+    script.write_text(
+        "import subprocess, sys, time\n"
+        f"c = subprocess.Popen([sys.executable, '-c', {child!r}])\n"
+        f"open({str(pid_file)!r}, 'w').write(str(c.pid))\n"
+        "time.sleep(60)\n"
+    )
+    tools = ShellTools()
+    result = _run(tools, f"{sys.executable} {script}", run_in_background=True)
+    deadline = time.monotonic() + 10
+    while not (pid_file.exists() and pid_file.read_text()) and time.monotonic() < deadline:
+        time.sleep(0.05)
+    child_pid = int(pid_file.read_text())
+
+    assert tools.stop_command(result["process_id"])["status"] == "exited"
+    deadline = time.monotonic() + 5
+    while time.monotonic() < deadline:
+        try:
+            os.kill(child_pid, 0)
+        except ProcessLookupError:
+            break
+        time.sleep(0.05)
+    else:
+        pytest.fail("child process survived stop_command")

src/praisonai/tests/unit/test_auto_generator.py

@@ -38,7 +38,7 @@ _RANKED_OPENAI = "gpt-ranked-test"
 
 def _patch_best_available():
     return patch(
-        "praisonai_code.llm.catalogue.ModelCatalogue.best_available",
+        _BEST_AVAILABLE,
         lambda self, provider: _RANKED_OPENAI if provider == "openai" else None,
     )
 
@@ -52,13 +52,10 @@ class TestAutoGeneratorDefaultModel:
             # Set required API key
             os.environ['OPENAI_API_KEY'] = 'test-key'
             
-            # Pin the fixed representative regardless of the installed
-            # litellm catalogue's capability ranking (see #5749).
-            with patch(_BEST_AVAILABLE, return_value=None):
-                generator = AutoGenerator(
-                    topic="Test topic",
-                    framework="praisonai"
-                )
+            generator = AutoGenerator(
+                topic="Test topic",
+                framework="praisonai"
+            )
             
             assert generator.config_list[0]['model'] == _RANKED_OPENAI
     
@@ -275,10 +272,9 @@ class TestWorkflowAutoGenerator:
         with patch.dict(os.environ, {}, clear=True), _patch_best_available():
             os.environ['OPENAI_API_KEY'] = 'test-key'
             
-            with patch(_BEST_AVAILABLE, return_value=None):
-                generator = WorkflowAutoGenerator(
-                    topic="Test workflow"
-                )
+            generator = WorkflowAutoGenerator(
+                topic="Test workflow"
+            )
             
             assert generator.config_list[0]['model'] == _RANKED_OPENAI