fix: closing a terminal on Windows never kills its processes

[badcuban]

What changed

On Windows, closing a terminal (trash can, or closing its thread) never kills the terminal's processes. runKillEscalation in apps/server/src/terminal/Layers/Manager.ts always passes a signal to PtyProcess.kill (SIGTERM, then SIGKILL after the grace period), and node-pty's WindowsTerminal.kill(signal) throws 'Signals not supported on windows.' whenever a signal is provided. Both attempts are caught and logged as failed to kill terminal process, and nothing is killed.

The result on win32: the shell and everything started in it (dev servers, watchers) keep running detached until the whole app exits, still holding their ports. Since closing also removes the session and deletes its history, the orphans are invisible afterwards; reopening spawns a fresh pty next to the still-running old one.

Repro (Windows): open a thread terminal, run anything that binds a port (npm run dev), close the terminal from the UI, then check the process list or the port. The dev server is still running.

The fix

On win32, call kill() with no signal and skip the SIGTERM/SIGKILL escalation. node-pty's signal-less Windows kill enumerates the console process list and terminates every process attached to the pty's console; that code path exists in node-pty precisely so node servers don't outlive the terminal (its source references microsoft/vscode#26807). POSIX behavior is unchanged.

TerminalProcessSignalError.signal becomes optional because the Windows failure case has no signal to report.

Tests

• New: kills the terminal without a signal on Windows (manager pinned to platform: "win32", asserts exactly one signal-less kill).
• The two existing kill-escalation tests used the host platform, so with this change they would exercise the win32 path when run on a Windows machine; they now pin platform: "linux" and deterministically test the POSIX escalation everywhere.

vp check (0 errors), vp run typecheck, and the terminal Manager suite (43 passed) all pass.

Found and verified in a downstream fork running on Windows 11; the diff here is built against current main.

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes process teardown on Windows for all terminal close and shutdown paths; incorrect behavior could leave orphans or kill too aggressively, but scope is isolated to runKillEscalation with POSIX unchanged.

Overview
Fixes terminal close on Windows leaving shell and child processes (e.g. dev servers) running because node-pty rejects signaled kills.

runKillEscalation in Manager.ts now branches on win32: it calls process.kill() with no signal and skips the SIGTERM → grace → SIGKILL path. POSIX behavior is unchanged. TerminalProcessSignalError.signal is optional so Windows failures without a signal still type-check.

Tests add kills the terminal without a signal on Windows and pin platform: "linux" on the existing escalation/shutdown tests so POSIX escalation is asserted on every host.

^{Reviewed by Cursor Bugbot for commit 3a9b11e. Bugbot is set up for automated code reviews on this repo. Configure here.}

Note

Fix terminal close on Windows to kill processes without a signal

On Windows, process.kill() requires no signal argument to correctly terminate a console process tree. The runKillEscalation util in Manager.ts now branches on platform: on win32 it calls kill() without a signal and skips the SIGTERM/SIGKILL escalation sequence; on other platforms, behavior is unchanged. The signal field on TerminalProcessSignalError is made optional to accommodate the signal-less kill path.

^{Macroscope summarized 3a9b11e.}

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 6631e97f-207c-489f-8e19-c163263d1d02

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[macroscopeapp]

Approvability

Verdict: Approved

This is a well-scoped platform-specific bug fix for Windows terminal process handling, with clear documentation explaining the node-pty limitation and appropriate test coverage added.

^{You can customize Macroscope's approvability policy. Learn more.}