fix(desktop): use tree-kill to ensure subprocesses are killed when closing ports

[Kitenite]

Summary

• When killing a process listening on a port, only the main process was being signaled via process.kill(), leaving child processes (like dev server workers) running
• Add treeKillWithEscalation utility that uses tree-kill to kill the entire process tree, with SIGTERM → SIGKILL escalation after 2 seconds
• Update port-manager.ts, manager.ts kill/cleanup methods to use tree-kill
• Properly handle EPERM (process exists but lacks permission) vs ESRCH (process doesn't exist)

Test plan

• Existing tests pass with updated mocks
• Manual test: Start a dev server (e.g., npm run dev), click "Close port" in the ports list, verify child processes are killed
• Manual test: Kill a terminal session, verify all child processes are terminated

Summary by CodeRabbit

• Bug Fixes
  • Improved process termination to reliably clean up entire process trees rather than just parent processes, reducing orphaned processes.
  • Enhanced terminal and port cleanup with automatic escalation for stubborn processes, ensuring reliable termination with better error reporting.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The pull request introduces asynchronous process termination with a new tree-kill escalation mechanism. It creates a treeKillWithEscalation module for graceful process tree termination with SIGTERM-to-SIGKILL escalation, refactors the TRPC ports router kill mutation to async, and updates terminal and port managers to use the new mechanism instead of direct signal handling.

Changes

Cohort / File(s)	Summary
Process termination escalation apps/desktop/src/main/lib/tree-kill-with-escalation.ts	New module implementing asynchronous process tree termination with configurable SIGTERM-to-SIGKILL escalation (default 2s timeout), periodic process aliveness polling, and error handling for missing processes.
TRPC ports router apps/desktop/src/lib/trpc/routers/ports/ports.ts	Converted kill mutation from synchronous to asynchronous, now returns Promise<{ success: boolean; error?: string }> instead of the synchronous variant.
Terminal management apps/desktop/src/main/lib/terminal/manager.ts	Refactored process termination to use treeKillWithEscalation for the process tree; renamed internal handlers for clarity (exitHandler → onExit, cleanup → finish); replaced manual timeout/escalation logic with unified escalation promise; added error logging for failed escalations.
Terminal manager tests apps/desktop/src/main/lib/terminal/manager.test.ts	Updated test mocks to intercept treeKillWithEscalation and wire it into the test environment; adjusted Pty mock to isolate kill from termination semantics; revised assertions to verify treeKillWithEscalation invocation with correct PIDs instead of direct Pty.kill calls.
Port management apps/desktop/src/main/lib/terminal/port-manager.ts	Converted killPort to return Promise<{ success: boolean; error?: string }> and switched from direct process.kill to treeKillWithEscalation for process tree termination while preserving terminal shell process.

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant treeKillWithEscalation
    participant System as System (process.kill)
    participant Monitor as Monitor (polling)
    
    Caller->>treeKillWithEscalation: treeKillWithEscalation({ pid, signal: "SIGTERM" })
    treeKillWithEscalation->>System: process.kill(pid, SIGTERM)
    treeKillWithEscalation->>Monitor: Start polling for process aliveness
    
    Monitor->>System: process.kill(pid, 0) - check aliveness
    System-->>Monitor: Process exists
    Monitor->>Monitor: Wait & retry (fixed interval)
    
    Monitor->>System: process.kill(pid, 0) - escalation timeout reached
    System-->>Monitor: Process still exists
    
    treeKillWithEscalation->>System: process.kill(pid, SIGKILL)
    Note over System: SIGKILL forces termination
    
    Monitor->>System: process.kill(pid, 0) - verify termination
    System-->>Monitor: ESRCH (process gone)
    
    treeKillWithEscalation-->>Caller: { success: true }

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

• feat(desktop): add ability to close ports from ports list #797: Directly related — both PRs modify the same TRPC ports router kill mutation, with this PR converting it to async.
• fix(desktop): remove auto-restart of terminal daemon in dev mode #897: Related — both PRs modify terminal process termination to use tree-kill with escalation logic.
• fix(desktop): kill all terminal child processes on close #892: Related — both PRs replace direct process.kill calls with tree-based escalation for terminating entire process trees.

Poem

🐰 Hops excitedly 🐇
A tree of processes falls with grace,
SIGTERM leads the gentle race,
When leaves don't drop, we escalate,
SIGKILL seals their final fate,
Async whispers, promises kept true! 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: using tree-kill to ensure subprocesses are killed when closing ports, which is directly reflected in the changeset.
Description check	✅ Passed	The description includes a clear summary of changes and test plan, but lacks specific details for all required template sections like Related Issues and Type of Change.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

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.}

[github-actions]

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

• ⚠️ Neon database branch
• ⚠️ Electric Fly.io app

Thank you for your contribution! 🎉