fix: reliably kill shell process tree on Windows timeout

[muhamedfazalps]

Fixes #8809

On Windows subprocess.run(timeout=N) with shell=True only kills cmd.exe, not the child process. Replaced with Popen+communicate + taskkill /F /T on Windows.

---

https://buymeacoffee.com/muhamedfazalps

Summary by Sourcery

Improve local shell command execution to handle timeouts more robustly across platforms.

Bug Fixes:

• Ensure timed-out shell commands on Windows terminate the entire process tree instead of only the intermediate cmd.exe process.

Enhancements:

• Switch local shell command execution from subprocess.run to Popen with explicit communicate and timeout handling for consistent stdout/stderr capture and exit codes.

[sourcery-ai]

Hey - I've reviewed your changes and they look great!

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[gemini-code-assist]

Code Review

This pull request refactors local command execution in astrbot/core/computer/booters/local.py to use subprocess.Popen and proc.communicate instead of subprocess.run to handle timeouts manually. On Windows, it attempts to terminate the process tree using taskkill. The review feedback points out a redundant import of subprocess and notes that a failure in the taskkill command could mask the original TimeoutExpired exception, preventing proper process cleanup. It suggests wrapping the taskkill call in a try...except block to ensure robust cleanup.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

There are two issues with the current implementation of the timeout handling on Windows:

1. Redundant Import: subprocess is already imported at the top of the file (line 7), so import subprocess as _sp is unnecessary.
2. Exception Masking: If taskkill fails (e.g., if the executable is not found or if the taskkill command itself times out), it will raise an exception. This will mask the original TimeoutExpired exception and prevent proc.wait() from being called, potentially leaking resources. Wrapping the taskkill call in a try...except block ensures that cleanup failures do not interfere with the propagation of the original timeout exception.

            except subprocess.TimeoutExpired:
                if sys.platform == "win32":
                    try:
                        subprocess.run(["taskkill", "/F", "/T", "/PID", str(proc.pid)],
                                       stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, timeout=5)
                    except Exception:
                        pass
                else:
                    proc.kill()
                proc.wait()
                raise