fix(apodex): kill the whole command session when run_shell times out

apodex/sandbox.py

@@ -41,9 +41,11 @@
 from __future__ import annotations
 
 import asyncio
+import contextlib
 import logging
 import os
 import shlex
+import signal
 import sys
 from dataclasses import dataclass
 from pathlib import Path
@@ -258,8 +260,20 @@ async def run_shell(
         cwd=cwd,
         stdout=asyncio.subprocess.PIPE,
         stderr=asyncio.subprocess.PIPE,
+        start_new_session=True,
     )
-    out, err = await asyncio.wait_for(proc.communicate(), timeout=timeout)
+    try:
+        out, err = await asyncio.wait_for(proc.communicate(), timeout=timeout)
+    finally:
+        if proc.returncode is None:
+            # Timed out or cancelled: kill the whole session, not just the
+            # shell, or its children keep running and holding the pipes. Same
+            # contract as ``_CurrentCommands.run`` in plugins.tools._sandbox,
+            # including the bounded wait for a setsid escapee killpg misses.
+            with contextlib.suppress(OSError):
+                os.killpg(proc.pid, signal.SIGKILL)
+            with contextlib.suppress(TimeoutError):
+                await asyncio.wait_for(proc.wait(), timeout=5)
     return (
         proc.returncode or 0,
         out.decode("utf-8", "replace"),

apodex/tests/test_native.py

@@ -2,8 +2,11 @@
 
 import asyncio
 import os
+import time
 from pathlib import Path
 
+import pytest
+
 from apodex import cli, docker, sandbox
 from apodex.native import prepare_native_runtime
 from apodex.sandbox import BWRAP, CONTAINER, NATIVE, Strategy, resolve_strategy
@@ -199,6 +202,22 @@ def kill(self):
     assert second.binds == ((str(second_workspace.resolve()),) * 2 + (False,),)
 
 
+def test_run_shell_kills_the_whole_command_on_timeout(tmp_path) -> None:
+    """A timed-out command must not keep writing to the workspace.
+
+    The subshell is a grandchild holding the output pipes, so killing only the
+    shell would still leave it alive to write the marker.
+    """
+    with pytest.raises(TimeoutError):
+        asyncio.run(sandbox.run_shell(
+            "(sleep 2; touch marker) & wait", str(tmp_path), 1,
+            Strategy(NATIVE, "test"),
+        ))
+    time.sleep(2)
+
+    assert not (tmp_path / "marker").exists()
+
+
 def test_macos_falls_back_to_native_when_docker_is_unavailable(
     tmp_path, monkeypatch, capsys,
 ) -> None: