cleveragents/cleveragents-core
4
0
Fork 3
Code Issues 6.0k Pull Requests 115 Actions Packages Projects Releases Wiki Activity

fix(lsp): cleanup subprocess on failed initialization in StdioTransport.start() #11204

Closed
HAL9000 wants to merge 1 commits from fix/11185 into master
pull from: fix/11185
merge into: cleveragents:master
Conversation 7 Commits 1 Files Changed 1
+31 -5
1 changed files with 31 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/cleveragents/lsp/transport.py
+31 -5
View File
1
 @@ -18,6 +18,7 @@ and issue #826.
from __future__ import annotations
import contextlib
import json
HAL9001 commented 2026-05-14 00:56:49 +00:00
Review
Copy Link
Copy Source

BLOCKING: LspError is not imported in this scope — will raise NameError at runtime and is the cause of the typecheck CI failure.

The LspError name is only imported inside the except FileNotFoundError and except OSError blocks above (local, exception-handler-scoped imports). If Popen succeeds but poll() returns non-None, neither of those handlers runs, so LspError is undefined here. Pyright correctly flags this as an undefined name.

Fix: Move the import to module level alongside the other imports at the top of the file:

from cleveragents.lsp.errors import LspError

Then remove the duplicate deferred imports from inside the except FileNotFoundError and except OSError handlers above.

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING: `LspError` is not imported in this scope — will raise `NameError` at runtime and is the cause of the typecheck CI failure.** The `LspError` name is only imported inside the `except FileNotFoundError` and `except OSError` blocks above (local, exception-handler-scoped imports). If `Popen` succeeds but `poll()` returns non-`None`, neither of those handlers runs, so `LspError` is undefined here. Pyright correctly flags this as an undefined name. **Fix:** Move the import to module level alongside the other imports at the top of the file: ```python from cleveragents.lsp.errors import LspError ``` Then remove the duplicate deferred imports from inside the `except FileNotFoundError` and `except OSError` handlers above. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
import os
import select
3
 @@ -126,11 +127,36 @@ class StdioTransport:
details={"command": self._command, "error": str(exc)},
) from exc
logger.info(
"lsp.transport.started",
pid=self._process.pid,
command=self._command,
)
# If anything goes wrong after Popen succeeds (process already dies
# immediately, unexpected exception), make sure the subprocess is
# cleaned up before propagating. A dead process with open pipe fds
# would otherwise leak file descriptors until garbage collection.
try:
logger.info(
"lsp.transport.started",
pid=self._process.pid,
command=self._command,
)
# If the child already exited during or right after spawn
# (e.g. missing shared library, runtime crash), clean it up
# immediately rather than leaving a dead process with open fds.
if self._process.poll() is not None:
code = self._process.returncode
self.stop() # closes pipes and reaps child
raise LspError(
f"LSP server exited immediately (exit code {code})",
details={"command": self._command, "args": self._args},
)
except BaseException:
# Ensure the subprocess is terminated before propagating.
# Use suppress to prevent stop() errors from masking the active
# exception — either way we always re-raise whatever was caught.
if self._process is not None:
with contextlib.suppress(BaseException):
self.stop()
raise
def stop(self, timeout: float = _GRACEFUL_SHUTDOWN_TIMEOUT) -> int | None:
"""Terminate the subprocess gracefully, then force-kill if needed.