fix(lsp): terminate LSP child process trees on failure

[Olusammytee]

Summary

• replace direct ChildProcess.kill() calls in LSP startup failure paths with Shell.killTree
• ensure Windows uses askkill /t semantics so child LSP processes are also terminated
• prevent orphaned LSP processes from surviving failed startup attempts

Fixes #591

[github-actions]

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

1. Open an issue describing the bug/feature (if one doesn't exist)
2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

[kiloconnect]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Clean, focused change that replaces direct ChildProcess.kill() calls with Shell.killTree() in three LSP startup failure paths. This ensures entire process trees are terminated (including child processes), which is especially important on Windows where taskkill /t semantics are needed.

The void fire-and-forget pattern is consistent with how Shell.killTree is used in event handlers elsewhere in the codebase (bash.ts, prompt.ts). The exited callback using handle.process.exitCode !== null is a standard Node.js check.

Files Reviewed (1 files)

• packages/opencode/src/lsp/index.ts - 0 issues

[Copilot]

Pull request overview

Updates LSP startup failure handling to terminate entire spawned process trees (especially on Windows) so failed LSP startups don’t leave orphaned/zombie language server processes behind.

Changes:

• Replace ChildProcess.kill() calls in LSP client initialization failure paths with Shell.killTree(...).
• Use a process “exited” predicate to avoid attempting to kill already-terminated processes.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The exited predicate only checks exitCode !== null, but child processes can also exit via signal (where signalCode is set and exitCode may remain null). That can cause Shell.killTree to unnecessarily attempt SIGKILL/taskkill after the process already exited. Consider treating the process as exited when either exitCode or signalCode is non-null (or attach an exit listener and use a boolean, like other killTree call sites).

[Copilot]

Shell.killTree tries process.kill(-pid, ...) on non-Windows to signal the process group, but LSP servers in this file are spawned without detached: true (so pid typically isn't a process group id). In practice this means the group kill will likely throw and killTree will fall back to proc.kill(), which won’t reliably terminate child processes on POSIX. If the goal is to kill process trees cross-platform, spawn LSP servers in their own process group (e.g., detached: process.platform !== "win32") so the negative-PID kill path actually works.

[Copilot]

This change introduces new failure-path behavior (killing the spawned LSP process tree) but there isn’t a unit test verifying that Shell.killTree is invoked when LSPClient.create fails / when client is undefined. Since the repo already has LSP-related tests (e.g., test/lsp/client.test.ts), it would be good to add a focused test that stubs Shell.killTree and asserts it’s called in these startup failure paths.