fix(swe-smith): stop command descendants on timeout

docs/75-example-coding-agent.md

@@ -66,6 +66,12 @@ This command installs the OpenAI client into each required SWE-smith base image
 
 ## Training
 
+Shell commands run inside the Linux repository containers. `SMITH_CMD_TIMEOUT`
+(default: 120 seconds) limits each agent command; `SMITH_EVAL_TIMEOUT` (default:
+600 seconds) limits the final pytest run. On timeout, the harness terminates the
+command's process group, including child workers, before continuing and reports
+exit code 124.
+
 The distributed launcher has three roles and must be started in this order:
 
 ```text

examples/swe_smith/agents/smith_agent.py

@@ -4,13 +4,15 @@
 
 from __future__ import annotations
 
+import contextlib
 import http.client
 import json
 import logging
 import os
 import random
 import re
 import shutil
+import signal
 import subprocess
 import sys
 import time
@@ -340,17 +342,27 @@ def _agent_env() -> dict[str, str]:
 
 def _run(command: str, timeout: int) -> tuple[str, int]:
     try:
-        proc = subprocess.run(
+        with subprocess.Popen(
             ["bash", "-c", command],
             cwd=TESTBED,
-            capture_output=True,
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE,
             text=True,
-            timeout=timeout,
             env=_agent_env(),
-        )
-        return proc.stdout + proc.stderr, proc.returncode
-    except subprocess.TimeoutExpired:
-        return f"[timed out after {timeout}s]", 124
+            start_new_session=True,
+        ) as proc:
+            try:
+                stdout, stderr = proc.communicate(timeout=timeout)
+            except subprocess.TimeoutExpired:
+                # Kill shell descendants before another action or evaluation
+                # starts, including workers still holding the output pipes.
+                with contextlib.suppress(ProcessLookupError):
+                    os.killpg(proc.pid, signal.SIGKILL)
+                # Timeout observations discard partial output. Reap the shell
+                # without decoding it; the context manager closes the pipes.
+                proc.wait()
+                return f"[timed out after {timeout}s]", 124
+            return stdout + stderr, proc.returncode
     except Exception as exc:
         return f"[failed to start: {exc}]", 1
 

tests/examples/test_swe_smith_command_timeout.py

@@ -0,0 +1,95 @@
+# Copyright (c) Microsoft. All rights reserved.
+
+"""Timed-out shell actions must not keep running during later agent turns."""
+
+from __future__ import annotations
+
+import os
+import shlex
+import signal
+import sys
+import time
+
+import pytest
+
+from examples.swe_smith.agents import smith_agent
+
+pytestmark = pytest.mark.skipif(os.name != "posix", reason="The SWE-smith harness runs in POSIX containers")
+
+
+@pytest.mark.parametrize("shell_exits_first", [False, True], ids=["foreground-command", "inherited-output-pipes"])
+def test_timeout_stops_child_and_grandchild(monkeypatch, tmp_path, shell_exits_first):
+    script = tmp_path / "command.py"
+    script.write_text(
+        "import os, pathlib, subprocess, sys, time\n"
+        "root = pathlib.Path(__file__).parent\n"
+        "role = sys.argv[1]\n"
+        "(root / (role + '.ready')).write_text(str(os.getpid()))\n"
+        "child = subprocess.Popen([sys.executable, __file__, 'grandchild']) if role == 'child' else None\n"
+        "while not (root / 'release').exists():\n"
+        "    time.sleep(0.01)\n"
+        "(root / (role + '.late')).write_text('changed after timeout')\n"
+        "if child is not None:\n"
+        "    child.wait()\n"
+    )
+    monkeypatch.setattr(smith_agent, "TESTBED", str(tmp_path))
+    command = f"{shlex.quote(sys.executable)} {shlex.quote(str(script))} child"
+    # Keep a foreground shell alive, or let it exit while descendants hold the
+    # captured pipes open. Both cases must terminate the entire command group.
+    command += " &" if shell_exits_first else " && echo done"
+
+    try:
+        start = time.monotonic()
+        output, returncode = smith_agent._run(command, timeout=1)
+        elapsed = time.monotonic() - start
+        assert (output, returncode) == ("[timed out after 1s]", 124)
+        assert elapsed < 4
+        assert (tmp_path / "child.ready").exists()
+        assert (tmp_path / "grandchild.ready").exists()
+
+        # A surviving descendant can now mutate the worktree in the next turn.
+        (tmp_path / "release").touch()
+        deadline = time.monotonic() + 1
+        while time.monotonic() < deadline and not list(tmp_path.glob("*.late")):
+            time.sleep(0.01)
+        assert list(tmp_path.glob("*.late")) == []
+    finally:
+        # Let the original implementation's surviving processes exit as well,
+        # so running the regression before the fix leaves no waiting children.
+        (tmp_path / "release").touch()
+
+
+def test_timeout_tolerates_process_group_exit_race(monkeypatch, tmp_path):
+    monkeypatch.setattr(smith_agent, "TESTBED", str(tmp_path))
+    killpg = os.killpg
+
+    def already_exited(process_group: int, sig: int) -> None:
+        assert sig == signal.SIGKILL
+        killpg(process_group, sig)
+        raise ProcessLookupError("group exited before signal delivery")
+
+    monkeypatch.setattr(os, "killpg", already_exited)
+    command = f"{shlex.quote(sys.executable)} -c 'import time; time.sleep(30)'"
+
+    assert smith_agent._run(command, timeout=1) == ("[timed out after 1s]", 124)
+
+
+def test_normal_command_preserves_output_and_exit_status(monkeypatch, tmp_path):
+    monkeypatch.setattr(smith_agent, "TESTBED", str(tmp_path))
+
+    assert smith_agent._run("printf stdout; printf stderr >&2; exit 7", timeout=5) == ("stdoutstderr", 7)
+
+
+def test_timeout_discards_partial_output_without_decoding_it(monkeypatch, tmp_path):
+    monkeypatch.setattr(smith_agent, "TESTBED", str(tmp_path))
+
+    assert smith_agent._run("printf '\\377'; sleep 30", timeout=1) == ("[timed out after 1s]", 124)
+
+
+def test_command_start_failure_is_reported(monkeypatch, tmp_path):
+    monkeypatch.setattr(smith_agent, "TESTBED", str(tmp_path / "missing"))
+
+    output, returncode = smith_agent._run("true", timeout=5)
+
+    assert returncode == 1
+    assert output.startswith("[failed to start:")