fix(server): stop Windows terminal processes when closing - #207
Merged
Conversation
Omit POSIX signals on Windows so node-pty can terminate the process tree. Linux and macOS keep SIGTERM/SIGKILL. Adapted from pingdotgg#10771.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
size:XS
vouch:trusted
Greptile SummarySummary
Confidence Score: 5/5Safe to merge. There are no outstanding findings. Reviews (2): Last reviewed commit: "Merge branch 'main' into fix/windows-ter..." | Re-trigger Greptile |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Closing a Windows terminal can hide the session while child processes keep running, including HTTP servers that still hold their ports. The terminal manager sends SIGTERM/SIGKILL, but node-pty rejects POSIX signals on Windows.
Changes
NodePtyAdapteromits the signal on Windows so node-pty runs its native process-tree termination. Linux and macOS keep their existing signal behavior. The change stays in the server adapter, so every client that closes a terminal gets it.Adapted from pingdotgg#10771. Process ownership, history, and Akeru session fields are unchanged.
Scope
This PR is Windows terminal close only.
Covered here:
Still assigned to this handoff, in later PRs:
Server history bounds already landed in a separate PR.
Verification
vp test run apps/server/src/terminal/NodePtyAdapter.test.ts: 6 passed, including Windows, Linux, and macOS kill semantics. The Windows case fails if a signal is passed through.vp linton the two adapter files: clean.No Windows host in this environment, so native process-tree termination was not exercised on a real Windows machine. Linux/macOS signal behavior is covered by the same adapter tests.
Implemented and verified by Grok 4.6 High in Grok Build via Orca.