Fix code execution PTY reset hangs

Start local PTY shells in a new process session and make close() escalate from SIGTERM to SIGKILL when a foreground command refuses to exit. This keeps code_execution_tool reset from blocking indefinitely on stuck terminal processes.\n\nAdd regression coverage for closing a TERM-resistant shell and document the reset lifecycle contract.
This commit is contained in:
Alessandro 2026-07-09 03:29:52 +02:00
parent 5f14435093
commit f1787b65f5
3 changed files with 51 additions and 15 deletions

View file

@ -14,6 +14,7 @@
## Local Contracts
- Keep session concurrency, timeout, streaming, and reset behavior predictable.
- Terminal reset/close must not hang on foreground commands or shells that ignore SIGTERM.
- Explicitly target local versus SSH execution runtimes.
- Do not hardcode secrets, SSH credentials, or local user paths.

View file

@ -1,10 +1,12 @@
import asyncio, os, sys, platform, errno
import asyncio, os, sys, platform, errno, signal
_IS_WIN = platform.system() == "Windows"
if _IS_WIN:
import winpty # pip install pywinpty # type: ignore
import msvcrt
_CLOSE_TIMEOUT_SECONDS = 2
def _reconfigure_stream_errors(stream) -> None:
reconfigure = getattr(stream, "reconfigure", None)
@ -77,15 +79,16 @@ class TTYSession:
# Terminate the process if it exists
if self._proc:
if getattr(self._proc, "returncode", None) is None:
self._signal_process(signal.SIGTERM)
try:
if getattr(self._proc, "returncode", None) is None:
self._proc.terminate()
except ProcessLookupError:
pass
except Exception:
pass
try:
await self._proc.wait()
await asyncio.wait_for(self._proc.wait(), _CLOSE_TIMEOUT_SECONDS)
except asyncio.TimeoutError:
self._signal_process(signal.SIGKILL)
try:
await asyncio.wait_for(self._proc.wait(), _CLOSE_TIMEOUT_SECONDS)
except Exception:
pass
except Exception:
pass
@ -93,6 +96,28 @@ class TTYSession:
self._proc = None
self._pump_task = None
def _signal_process(self, sig):
if self._proc is None:
return
try:
if _IS_WIN:
if sig == signal.SIGKILL:
self._proc.kill()
else:
self._proc.terminate()
return
os.killpg(self._proc.pid, sig)
except ProcessLookupError:
pass
except Exception:
try:
if sig == signal.SIGKILL:
self._proc.kill()
else:
self._proc.terminate()
except Exception:
pass
def _release_pty_master(self):
"""Release the POSIX PTY master exactly once.
@ -166,11 +191,7 @@ class TTYSession:
# Only attempt to kill if the process is still running
if getattr(self._proc, "returncode", None) is None:
try:
self._proc.kill()
except ProcessLookupError:
# Child already gone treat as successfully killed
pass
self._signal_process(signal.SIGKILL)
self._release_pty_master()
async def read(self, timeout=None):
@ -241,6 +262,7 @@ async def _spawn_posix_pty(cmd, cwd, env, echo):
cwd=cwd,
env=env,
close_fds=True,
start_new_session=True,
)
os.close(slave)

View file

@ -1,10 +1,13 @@
"""Regression tests for issue #1697.
"""Regression tests for code execution shell lifecycle behavior.
Pagers (more/less) must be disabled in the non-interactive shells created by the
code execution tool: without user input they block forever and spin at 100% CPU.
"""
import asyncio
from plugins._code_execution.helpers import shell_local, shell_ssh
from plugins._code_execution.helpers.tty_session import TTYSession
def test_local_env_disables_pagers_and_preserves_existing():
@ -30,3 +33,13 @@ def test_local_env_does_not_mutate_input():
def test_ssh_command_disables_pagers():
assert "GIT_PAGER=cat" in shell_ssh.PAGER_DISABLE_COMMAND
assert "PAGER=cat" in shell_ssh.PAGER_DISABLE_COMMAND
def test_tty_close_kills_term_resistant_process():
async def run():
session = TTYSession("bash -lc 'trap \"\" TERM; sleep 30'")
await session.start()
await asyncio.wait_for(session.close(), timeout=6)
assert session._proc is None
asyncio.run(run())