diff --git a/openhands-tools/openhands/tools/terminal/terminal/windows_terminal.py b/openhands-tools/openhands/tools/terminal/terminal/windows_terminal.py index fa310f180b..1ae39820c1 100644 --- a/openhands-tools/openhands/tools/terminal/terminal/windows_terminal.py +++ b/openhands-tools/openhands/tools/terminal/terminal/windows_terminal.py @@ -32,6 +32,7 @@ _SETUP_DELAY_SECONDS = 0.5 _SETUP_POLL_INTERVAL_SECONDS = 0.05 _MAX_SETUP_WAIT_SECONDS = 2.0 +_INTERRUPT_GRACE_SECONDS = 0.5 _WINDOWS_SPECIALS: dict[str, str] = { "ENTER": "\n", @@ -172,6 +173,7 @@ def close(self) -> None: return self._stop_reader.set() + self._terminate_child_processes() if self.process is not None: try: @@ -327,6 +329,62 @@ def clear_screen(self) -> None: time.sleep(_SCREEN_CLEAR_DELAY_SECONDS) self._command_running_event.clear() + def _terminate_child_processes(self) -> bool: + """Terminate descendants of the persistent PowerShell process.""" + if ( + platform.system() != "Windows" + or self.process is None + or self.process.poll() is not None + ): + return False + + script = f""" +$root = {self.process.pid} +$childrenByParent = @{{}} +Get-CimInstance Win32_Process | ForEach-Object {{ + $parentId = [int]$_.ParentProcessId + if (-not $childrenByParent.ContainsKey($parentId)) {{ + $childrenByParent[$parentId] = New-Object System.Collections.Generic.List[int] + }} + $childrenByParent[$parentId].Add([int]$_.ProcessId) +}} +$toStop = New-Object System.Collections.Generic.List[int] +function Add-Descendants([int]$processId) {{ + if (-not $childrenByParent.ContainsKey($processId)) {{ return }} + foreach ($childId in $childrenByParent[$processId]) {{ + if ($childId -eq $PID) {{ continue }} + $toStop.Add($childId) + Add-Descendants $childId + }} +}} +Add-Descendants $root +for ($i = $toStop.Count - 1; $i -ge 0; $i--) {{ + Stop-Process -Id $toStop[$i] -Force -ErrorAction SilentlyContinue +}} +if ($toStop.Count -gt 0) {{ exit 0 }} else {{ exit 1 }} +""" + startupinfo = None + startupinfo_cls = getattr(subprocess, "STARTUPINFO", None) + if startupinfo_cls is not None: + startupinfo = startupinfo_cls() + startupinfo.dwFlags |= getattr(subprocess, "STARTF_USESHOWWINDOW", 0) + creationflags = getattr(subprocess, "CREATE_NO_WINDOW", 0) + + try: + result = subprocess.run( + [self.shell_path, "-NoLogo", "-NoProfile", "-Command", script], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + check=False, + timeout=5.0, + startupinfo=startupinfo, + creationflags=creationflags, + ) + return result.returncode == 0 + except (subprocess.TimeoutExpired, OSError) as exc: + logger.debug("Failed to terminate PowerShell child processes: %s", exc) + return False + def interrupt(self) -> bool: """Interrupt the active command if the process is still alive.""" if self.process is None or self.process.poll() is not None: @@ -341,15 +399,21 @@ def interrupt(self) -> bool: except Exception as exc: logger.debug("Failed to send CTRL_BREAK_EVENT: %s", exc) - if not sent_ctrl_break: + if sent_ctrl_break: + time.sleep(_INTERRUPT_GRACE_SECONDS) + + terminated_children = self._terminate_child_processes() + sent_ctrl_c_input = False + if not sent_ctrl_break and not terminated_children: try: self._write_to_stdin(_WINDOWS_SPECIALS["C-C"]) + sent_ctrl_c_input = True except RuntimeError as exc: logger.debug("Failed to write Ctrl+C to PowerShell stdin: %s", exc) return False self._command_running_event.clear() - return True + return sent_ctrl_break or terminated_children or sent_ctrl_c_input def is_running(self) -> bool: """Return whether a command is still running in the PowerShell session.""" diff --git a/tests/tools/browser_use/test_browser_toolset.py b/tests/tools/browser_use/test_browser_toolset.py index ef8fa28fe9..6c568076b7 100644 --- a/tests/tools/browser_use/test_browser_toolset.py +++ b/tests/tools/browser_use/test_browser_toolset.py @@ -31,7 +31,8 @@ def _mock_browser_executor_init(): def fake_init(self, **_kwargs): self.full_output_save_dir = None self._initialized = False - self._cleanup_initiated = False + # Toolset tests never allocate browser resources; keep close() a no-op. + self._cleanup_initiated = True self._action_timeout_seconds = 30.0 self._async_executor = MagicMock() self._async_executor.close = MagicMock() diff --git a/tests/tools/terminal/test_windows_ctrl_c.py b/tests/tools/terminal/test_windows_ctrl_c.py new file mode 100644 index 0000000000..0908215c66 --- /dev/null +++ b/tests/tools/terminal/test_windows_ctrl_c.py @@ -0,0 +1,106 @@ +"""Windows-specific terminal interrupt behavior tests.""" + +import platform +import subprocess + +import pytest + +from openhands.tools.terminal.definition import TerminalAction +from openhands.tools.terminal.terminal import create_terminal_session +from openhands.tools.terminal.terminal.terminal_session import TerminalCommandStatus + + +pytestmark = pytest.mark.skipif( + platform.system() != "Windows", + reason="Windows CTRL_BREAK/PowerShell process behavior only applies on Windows", +) + + +def _powershell_process_exists(pid: int) -> bool: + result = subprocess.run( + [ + "powershell.exe", + "-NoLogo", + "-NoProfile", + "-Command", + ( + f"if (Get-Process -Id {pid} -ErrorAction SilentlyContinue) " + "{ exit 0 } else { exit 1 }" + ), + ], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + check=False, + ) + return result.returncode == 0 + + +def _stop_powershell_process(pid: int) -> None: + subprocess.run( + [ + "powershell.exe", + "-NoLogo", + "-NoProfile", + "-Command", + f"Stop-Process -Id {pid} -Force -ErrorAction SilentlyContinue", + ], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + check=False, + ) + + +@pytest.mark.timeout(20) +def test_windows_ctrl_c_interrupt_kills_child_process_tree(tmp_path) -> None: + """Ctrl-C after a timeout should stop the process that kept the command alive. + + This captures the behavior promised by the timeout prompt. The current + PowerShell backend sends CTRL_BREAK to the persistent PowerShell process, but + does not ensure child processes launched by the command are terminated. + """ + pid_path = tmp_path / "child.pid" + script_path = tmp_path / "wait_on_child.ps1" + script_path.write_text( + "\n".join( + [ + f"$pidPath = '{pid_path.as_posix()}'", + "$child = Start-Process -FilePath powershell.exe " + "-ArgumentList '-NoLogo','-NoProfile','-Command'," + "'Start-Sleep -Seconds 120' -PassThru", + "Set-Content -LiteralPath $pidPath -Value $child.Id", + "Wait-Process -Id $child.Id", + ] + ), + encoding="utf-8", + ) + + session = create_terminal_session( + work_dir=str(tmp_path), + terminal_type="powershell", + no_change_timeout_seconds=1, + ) + child_pid: int | None = None + child_was_still_running = False + try: + session.initialize() + + obs = session.execute(TerminalAction(command=f"& '{script_path.as_posix()}'")) + + assert obs.metadata.exit_code == -1 + assert session.prev_status == TerminalCommandStatus.NO_CHANGE_TIMEOUT + assert pid_path.exists() + child_pid = int(pid_path.read_text(encoding="utf-8").strip()) + assert _powershell_process_exists(child_pid) + + session.execute(TerminalAction(command="C-c", is_input=True, timeout=3)) + + child_was_still_running = _powershell_process_exists(child_pid) + finally: + if child_pid is not None: + _stop_powershell_process(child_pid) + session.close() + + assert not child_was_still_running, ( + "Windows Ctrl-C reported through the terminal did not terminate the " + "child process that kept the timed-out command alive." + )