From ffcb66ba43e4f0a3d9f09c75b330469f62b53317 Mon Sep 17 00:00:00 2001 From: Amir Fathi Date: Wed, 12 Aug 2026 13:10:45 +0000 Subject: [PATCH 1/6] Fix: surface a mid-indexing tsserver crash instead of returning {} typescript-language-server sends a $/progress "end" for the in-flight token as part of tearing its connection down after tsserver aborts (e.g. a V8 heap OOM). _wait_for_indexing_start_or_completion could not distinguish that from a real completion, so find_referencing_symbols logged "TypeScript cross-file indexing complete" and returned {} with isError: false, indistinguishable from a symbol genuinely having no references. The crash is already observed independently via window/logMessage (type Error, "[tsserver] Exited"), but the handler only logged it. Wire that signal into a _crash_message flag and raise TypeScriptServerCrashedError from every path that would otherwise report indexing as complete (wait_for_indexing and both early-return branches of _wait_for_indexing_start_or_completion), so a crash propagates as a tool error instead of a silent empty result. Fixes #1814 --- CHANGELOG.md | 5 + .../typescript_language_server.py | 49 +++++++- .../test_typescript_timeout_policy.py | 108 +++++++++++++++++- 3 files changed, 160 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d3211e0..0a61aded 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,11 @@ Status of the `main` branch. Changes prior to the next official version change w indicate it was incomplete. Serena now declares work-done progress support and waits for the work Metals reports, bounded by the new `indexing_timeout`, `indexing_start_grace` and `indexing_quiet_period` settings + - Fix: a `tsserver` crash mid-indexing (e.g. a V8 heap OOM) sent the same `$/progress` "end" + event as a normal completion, so `find_referencing_symbols` and other cross-file queries + silently returned an empty result instead of surfacing the crash. The crash is now detected + independently via the `window/logMessage` notification tsserver already sends, and the + affected wait now raises instead of reporting success (#1814) * Dependencies: - Remove the redundant `dotenv` dependency; the `dotenv` module is provided by `python-dotenv` diff --git a/src/solidlsp/language_servers/typescript_language_server.py b/src/solidlsp/language_servers/typescript_language_server.py index 063b2524..c02cf13c 100644 --- a/src/solidlsp/language_servers/typescript_language_server.py +++ b/src/solidlsp/language_servers/typescript_language_server.py @@ -4,6 +4,7 @@ Provides TypeScript specific instantiation of the LanguageServer class. Contains import logging import os +import re import shutil import threading import time @@ -15,7 +16,9 @@ from sensai.util.logging import LogTime from solidlsp import ls_types from solidlsp.ls import LanguageServerDependencyProvider, LanguageServerDependencyProviderSinglePath, SolidLanguageServer from solidlsp.ls_config import LanguageServerConfig +from solidlsp.ls_exceptions import SolidLSPException from solidlsp.ls_utils import PlatformId, PlatformUtils +from solidlsp.lsp_protocol_handler.lsp_types import MessageType from solidlsp.settings import SolidLSPSettings from .common import RuntimeDependency, RuntimeDependencyCollection, build_npm_install_command @@ -65,6 +68,21 @@ def prefer_non_node_modules_definition(definitions: list[ls_types.Location]) -> return definitions[0] +class TypeScriptServerCrashedError(SolidLSPException): + """Raised when tsserver reported its own abnormal exit via window/logMessage. + + typescript-language-server sends a $/progress "end" event for the in-flight + token as part of tearing its connection down after tsserver dies, which is + otherwise indistinguishable from a normal indexing completion. + """ + + +# Matches typescript-language-server's window/logMessage notification for an abnormal +# tsserver exit, e.g. "[lspserver] [tsclient] [tsserver] Exited. Code: null. Signal: SIGABRT". +# A clean shutdown does not produce this message. +_TSSERVER_EXITED_PATTERN = re.compile(r"\[tsserver\]\s+Exited\b", re.IGNORECASE) + + class TypeScriptLanguageServer(SolidLanguageServer): """ Provides TypeScript specific instantiation of the LanguageServer class. Contains various configurations and settings specific to TypeScript. @@ -114,14 +132,36 @@ class TypeScriptLanguageServer(SolidLanguageServer): self._active_progress_tokens: set[str] = set() self._indexing_complete = threading.Event() self._indexing_complete.set() # Initially set (no active work) + # set from window/logMessage when tsserver reports its own abnormal exit; + # a crash mid-indexing still drains _active_progress_tokens via a $/progress + # "end" event, so that alone cannot distinguish a crash from real completion + self._crash_message: str | None = None + + def _raise_if_crashed(self) -> None: + if self._crash_message is not None: + raise TypeScriptServerCrashedError(self._crash_message) + + @staticmethod + def _tsserver_exit_message(msg: dict) -> str | None: + """:return: the log text if ``msg`` is tsserver reporting its own abnormal exit, else None.""" + if msg.get("type") != MessageType.Error: + return None + message_text = str(msg.get("message", "")) + if _TSSERVER_EXITED_PATTERN.search(message_text): + return message_text + return None def wait_for_indexing(self, timeout: float) -> bool: """Block until all $/progress tokens complete. :param timeout: Maximum seconds to wait. :return: True if indexing completed, False on timeout. + :raises TypeScriptServerCrashedError: if tsserver reported an abnormal exit. """ - return self._indexing_complete.wait(timeout=timeout) + result = self._indexing_complete.wait(timeout=timeout) + if result: + self._raise_if_crashed() + return result def _wait_for_indexing_start_or_completion(self, timeout: float, start_grace: float | None = None) -> bool: """Wait until TypeScript indexing has started and drained, or provably never started. @@ -129,6 +169,7 @@ class TypeScriptLanguageServer(SolidLanguageServer): :param timeout: Maximum seconds to wait once active indexing progress is observed. :param start_grace: Maximum seconds to wait for progress to begin after opening files. :return: True if indexing completed or no progress began within the grace period, False on timeout. + :raises TypeScriptServerCrashedError: if tsserver reported an abnormal exit. """ grace = self.INDEXING_START_GRACE if start_grace is None else start_grace @@ -139,6 +180,7 @@ class TypeScriptLanguageServer(SolidLanguageServer): if self._active_progress_tokens: break if self._indexing_complete.is_set(): + self._raise_if_crashed() return True time.sleep(0.05) @@ -147,6 +189,7 @@ class TypeScriptLanguageServer(SolidLanguageServer): has_active_progress = bool(self._active_progress_tokens) if not has_active_progress: self._indexing_complete.set() + self._raise_if_crashed() return True # wait for active progress to drain @@ -382,6 +425,10 @@ class TypeScriptLanguageServer(SolidLanguageServer): def window_log_message(msg: dict) -> None: log.info(f"LSP: window/logMessage: {msg}") + crash_text = self._tsserver_exit_message(msg) + if crash_text is not None: + log.warning(f"tsserver reported an abnormal exit: {crash_text}") + self._crash_message = f"tsserver exited abnormally: {crash_text}" def handle_typescript_version(params: dict) -> None: """ diff --git a/test/solidlsp/test_typescript_timeout_policy.py b/test/solidlsp/test_typescript_timeout_policy.py index ad2274b0..f71867ca 100644 --- a/test/solidlsp/test_typescript_timeout_policy.py +++ b/test/solidlsp/test_typescript_timeout_policy.py @@ -17,7 +17,7 @@ from solidlsp.language_servers.svelte_language_server import ( SvelteLanguageServer, SvelteTypeScriptServer, ) -from solidlsp.language_servers.typescript_language_server import TypeScriptLanguageServer +from solidlsp.language_servers.typescript_language_server import TypeScriptLanguageServer, TypeScriptServerCrashedError from solidlsp.settings import SolidLSPSettings @@ -31,6 +31,7 @@ def _bare_ts_server(cls: type[TypeScriptLanguageServer], custom_settings: dict | server._active_progress_tokens = set() server._indexing_complete = threading.Event() server._indexing_complete.set() # mirrors __init__: initially no active work + server._crash_message = None # mirrors __init__: no crash observed yet server._custom_settings = SolidLSPSettings.CustomLSSettings(custom_settings) return server @@ -149,6 +150,111 @@ class TestWaitForIndexingStartOrCompletion: timer.cancel() +class TestTsserverCrashDetection: + """oraios/serena#1814: typescript-language-server sends a $/progress "end" for the + in-flight token as part of tearing its connection down after tsserver dies, which + _wait_for_indexing_start_or_completion previously could not distinguish from a real + completion: find_referencing_symbols returned {} with isError: false instead of + surfacing the crash. window/logMessage does carry the crash independently; these + tests drive the exact begin/crash-log/end sequence from the reporter's log and pin + that every wait path now raises instead of reporting silent success. + """ + + def test_tsserver_exit_message_matches_sigabrt_line(self) -> None: + msg = {"type": 1, "message": "[lspserver] [tsclient] [tsserver] Exited. Code: null. Signal: SIGABRT"} + assert TypeScriptLanguageServer._tsserver_exit_message(msg) == msg["message"] + + def test_tsserver_exit_message_ignores_non_error_type(self) -> None: + # type 3 (Info) is what a clean shutdown/log line would carry, not type 1 (Error) + msg = {"type": 3, "message": "[lspserver] [tsclient] [tsserver] Exited. Code: null. Signal: SIGABRT"} + assert TypeScriptLanguageServer._tsserver_exit_message(msg) is None + + def test_tsserver_exit_message_ignores_unrelated_error(self) -> None: + msg = {"type": 1, "message": "Some other tsserver error unrelated to process exit"} + assert TypeScriptLanguageServer._tsserver_exit_message(msg) is None + + def test_raise_if_crashed_is_a_noop_when_no_crash_observed(self) -> None: + server = _bare_ts_server(TypeScriptLanguageServer) + server._raise_if_crashed() # must not raise + + def test_raise_if_crashed_raises_with_recorded_message(self) -> None: + server = _bare_ts_server(TypeScriptLanguageServer) + server._crash_message = "tsserver exited abnormally: [tsserver] Exited. Code: null. Signal: SIGABRT" + + with pytest.raises(TypeScriptServerCrashedError, match="SIGABRT"): + server._raise_if_crashed() + + def test_wait_for_indexing_raises_when_progress_end_follows_a_crash(self) -> None: + """Drives the reporter's exact sequence: $/progress begin, the SIGABRT window/logMessage, + then $/progress end (teardown), the same "end" that previously read as success. + """ + server = _bare_ts_server(TypeScriptLanguageServer) + server.expect_indexing() + server._active_progress_tokens.add("646a4e19") + + # window/logMessage handler's effect: record the crash independently of progress state + server._crash_message = "tsserver exited abnormally: [tsserver] Exited. Code: null. Signal: SIGABRT" + # $/progress end (teardown): drains the token and sets indexing_complete, exactly as a + # real completion would + server._active_progress_tokens.discard("646a4e19") + server._indexing_complete.set() + + with pytest.raises(TypeScriptServerCrashedError, match="SIGABRT"): + server.wait_for_indexing(timeout=1.0) + + def test_wait_for_indexing_returns_true_on_clean_completion(self) -> None: + """Same $/progress begin/end shape as the crash test, without a crash message: must still + return True, not regress the ordinary success path. + """ + server = _bare_ts_server(TypeScriptLanguageServer) + server.expect_indexing() + server._active_progress_tokens.add("646a4e19") + server._active_progress_tokens.discard("646a4e19") + server._indexing_complete.set() + + assert server.wait_for_indexing(timeout=1.0) is True + + def test_wait_for_indexing_start_or_completion_raises_via_early_is_set_branch(self) -> None: + """Covers the early "if self._indexing_complete.is_set(): return True" branch, which is + reached (not the tail wait_for_indexing() call) when begin+crash+end all land before the + first poll iteration observes an active token. + """ + server = _bare_ts_server(TypeScriptLanguageServer) + server.expect_indexing() + server._crash_message = "tsserver exited abnormally: [tsserver] Exited. Code: null. Signal: SIGABRT" + server._indexing_complete.set() # progress already drained by the time we start waiting + + with pytest.raises(TypeScriptServerCrashedError, match="SIGABRT"): + server._wait_for_indexing_start_or_completion(timeout=1.0, start_grace=1.0) + + def test_wait_for_indexing_start_or_completion_raises_via_absent_progress_branch(self) -> None: + """Covers the "treat absent progress as ready" branch: no active token observed within the + start grace, but a crash was already recorded. + """ + server = _bare_ts_server(TypeScriptLanguageServer) + server.expect_indexing() + server._crash_message = "tsserver exited abnormally: [tsserver] Exited. Code: null. Signal: SIGABRT" + + with pytest.raises(TypeScriptServerCrashedError, match="SIGABRT"): + server._wait_for_indexing_start_or_completion(timeout=1.0, start_grace=0.05) + + def test_wait_for_cross_file_references_propagates_the_crash(self) -> None: + """The actual call path find_referencing_symbols drives (ls.py's SymbolLocationRequest.execute + calls this with no surrounding try/except) must raise, not silently log "indexing complete" + and let the caller receive an empty result. + """ + server = _bare_ts_server(TypeScriptLanguageServer) + server._has_waited_for_cross_file_references = False + server.expect_indexing() + server._crash_message = "tsserver exited abnormally: [tsserver] Exited. Code: null. Signal: SIGABRT" + + with pytest.raises(TypeScriptServerCrashedError, match="SIGABRT"): + server._wait_for_cross_file_references_if_needed() + + # the crash must be reported, not swallowed into a false "we've already waited" state + assert server._has_waited_for_cross_file_references is False + + class TestWaitForCrossFileReferencesUsesConfiguredGrace: """The actual find-references call path (not just the helper in isolation) must honor indexing_start_grace: this is the mechanism behind oraios/serena#1586, where a large From 19f1c33bbf8d79f43ebc1909f1848072216e63e3 Mon Sep 17 00:00:00 2001 From: xor <127287135+xormania@users.noreply.github.com> Date: Sun, 16 Aug 2026 10:57:13 -0400 Subject: [PATCH 2/6] Format --- src/interprompt/prompt_factory.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/interprompt/prompt_factory.py b/src/interprompt/prompt_factory.py index 599be189..5a8fcfbd 100644 --- a/src/interprompt/prompt_factory.py +++ b/src/interprompt/prompt_factory.py @@ -40,8 +40,9 @@ class PromptFactoryBase: return self._prompt_collection.get_prompt_list(prompt_name, self.lang_code) -def autogenerate_prompt_factory_module(prompts_dir: str, target_module_path: str, interprompt_library_package: str = "interprompt", - class_name: str = "PromptFactory") -> None: +def autogenerate_prompt_factory_module( + prompts_dir: str, target_module_path: str, interprompt_library_package: str = "interprompt", class_name: str = "PromptFactory" +) -> None: """ Auto-generates a prompt factory module for the given prompt directory. The generated `PromptFactory` class is meant to be the central entry class for retrieving and rendering prompt templates and prompt From d4dbcecaae24b4b4994b558b92083dfe960c4fe5 Mon Sep 17 00:00:00 2001 From: aayushbaluni <73417844+aayushbaluni@users.noreply.github.com> Date: Mon, 17 Aug 2026 11:39:28 +0530 Subject: [PATCH 3/6] fix(clojure): declare didChangeWatchedFiles client capability Serena notifies language servers about files changed outside its own edit tools (a git checkout, another editor, a build step) by sending workspace/didChangeWatchedFiles from LanguageServerManager.poll_and_notify, whose docstring names this exact case: such edits are 'otherwise invisible to a warm language server, causing symbolic queries to answer from a stale index'. ClojureLSP never declared the corresponding client capability, so clojure-lsp was not told those notifications are sent and need not treat them as a reason to invalidate its analysis. DefaultInitializeParamsBuilder does not supply a default capabilities block, so nothing filled this in on the server's behalf. Relates to #1593, where find_symbol returned a body from a comment mentioning the symbol rather than its definition, at a position the symbol had previously occupied. --- CHANGELOG.md | 4 ++ src/solidlsp/language_servers/clojure_lsp.py | 6 +++ .../clojure/test_clojure_initialize_params.py | 42 +++++++++++++++++++ 3 files changed, 52 insertions(+) create mode 100644 test/solidlsp/clojure/test_clojure_initialize_params.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 161709e7..bba1abc6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ Status of the `main` branch. Changes prior to the next official version change w * Language Servers: - Fix: Dart's `$/analyzerStatus` notifications were logged as unhandled-method warnings during analysis (#1855) + - Fix: clojure-lsp was not told that Serena sends `workspace/didChangeWatchedFiles`, so changes made + outside Serena's own edit tools (a git checkout, another editor, a build step) need not invalidate + its analysis; symbol queries could then answer from a stale index, e.g. `find_symbol` returning a + body from the position the symbol used to occupy (#1593) - Fix: Scala cross-file queries waited a fixed 5s after the first file was opened, which on a cold Metals is long before its build import, indexing and compilation have finished; the first `find_referencing_symbols` of a session could return a fraction of the references with nothing to diff --git a/src/solidlsp/language_servers/clojure_lsp.py b/src/solidlsp/language_servers/clojure_lsp.py index a8cb9d83..41851a03 100644 --- a/src/solidlsp/language_servers/clojure_lsp.py +++ b/src/solidlsp/language_servers/clojure_lsp.py @@ -325,6 +325,12 @@ class ClojureLSP(SolidLanguageServer): "workspace": { "applyEdit": True, "workspaceEdit": {"documentChanges": True}, + # Serena notifies language servers about files changed outside its own + # edit tools (git checkout, another editor, a build step) via + # workspace/didChangeWatchedFiles; see LanguageServerManager.poll_and_notify. + # Without declaring the capability, clojure-lsp is not told the client + # sends those notifications and may keep answering from its stale analysis. + "didChangeWatchedFiles": {"dynamicRegistration": True}, "symbol": {"symbolKind": {"valueSet": list(range(1, 27))}}, "workspaceFolders": True, }, diff --git a/test/solidlsp/clojure/test_clojure_initialize_params.py b/test/solidlsp/clojure/test_clojure_initialize_params.py new file mode 100644 index 00000000..54538309 --- /dev/null +++ b/test/solidlsp/clojure/test_clojure_initialize_params.py @@ -0,0 +1,42 @@ +import pytest + +from solidlsp.language_servers.clojure_lsp import ClojureLSP + +pytestmark = pytest.mark.clojure + + +def _make_server(monkeypatch: pytest.MonkeyPatch) -> ClojureLSP: + """Build a ClojureLSP without running __init__ (no clojure-lsp binary needed).""" + server = object.__new__(ClojureLSP) + monkeypatch.setattr(ClojureLSP, "_resolve_source_paths", lambda _self: None) + return server + + +def test_declares_did_change_watched_files_capability(monkeypatch: pytest.MonkeyPatch) -> None: + """clojure-lsp must announce that the client sends workspace/didChangeWatchedFiles. + + Serena notifies language servers about files changed outside its own edit tools + (git checkout, another editor, a build step) via that notification + (LanguageServerManager.poll_and_notify). A server that was never told the client + supports it may keep answering symbol queries from its own stale analysis, which + surfaces as find_symbol returning a body from the wrong location. + """ + params = _make_server(monkeypatch)._create_base_initialize_params() + + workspace = params["capabilities"]["workspace"] + assert "didChangeWatchedFiles" in workspace, ( + "clojure-lsp does not declare the didChangeWatchedFiles client capability, so external file changes may not invalidate its analysis" + ) + assert workspace["didChangeWatchedFiles"]["dynamicRegistration"] is True + + +def test_base_initialize_params_keep_existing_workspace_capabilities( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The added capability must not displace the ones already relied upon.""" + workspace = _make_server(monkeypatch)._create_base_initialize_params()["capabilities"]["workspace"] + + assert workspace["applyEdit"] is True + assert workspace["workspaceEdit"] == {"documentChanges": True} + assert workspace["workspaceFolders"] is True + assert workspace["symbol"]["symbolKind"]["valueSet"] == list(range(1, 27)) From 43ae0211d7f3bba4101cd0552707fa21d37f4c84 Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Sun, 30 Aug 2026 17:37:14 +0200 Subject: [PATCH 4/6] Fix: `read_only` restriction in project definition was not applied to base tool set when in single-project context Fixes #1938 --- CHANGELOG.md | 1 + src/serena/agent.py | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30a50473..e60a96fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ Status of the `main` branch. Changes prior to the next official version change w * General: - Fix: Parallel agents auto-registering projects could overwrite each other's changes to the global project list in `serena_config.yml` + - Fix: `read_only` restriction in project definition was not applied to base tool set when in single-project context (#1938) * Language Servers: - Fix: Dart's `$/analyzerStatus` notifications were logged as unhandled-method warnings during analysis (#1855) diff --git a/src/serena/agent.py b/src/serena/agent.py index 175f338b..4a2d843f 100644 --- a/src/serena/agent.py +++ b/src/serena/agent.py @@ -794,6 +794,7 @@ class SerenaAgent: # of tools that will be exposed to the client. # Furthermore, we disable tools that are only relevant for project activation. # So if the project exists, we apply all the aforementioned exclusions. + apply_read_only = False if is_single_project: assert project is not None log.info( @@ -807,9 +808,12 @@ class SerenaAgent: ) ) tool_inclusion_definitions.append(project.project_config) + apply_read_only = project.project_config.read_only # compute the resulting tool set base_toolset = ToolSet.default().apply(*tool_inclusion_definitions) + if apply_read_only: + base_toolset = base_toolset.without_editing_tools() log.info(f"Number of exposed tools: {len(base_toolset)}") return base_toolset From dc59a8931c5ca90deb18bed2a5392ea2fc37add8 Mon Sep 17 00:00:00 2001 From: Amir Fathi Date: Wed, 2 Sep 2026 04:53:41 -0600 Subject: [PATCH 5/6] Fix: Exceptions raised during LanguageServerManager.start did not stop the language server subprocess if it was already started start() now catches an exception from _start_server(), stops the process if it is already running, and re-raises. Fixes #1949 --- CHANGELOG.md | 2 + src/serena/ls_manager.py | 3 + src/solidlsp/ls.py | 12 ++- test/serena/test_ls_manager.py | 103 +++++++++++++++++++++++++ test/solidlsp/test_ls_start_cleanup.py | 55 +++++++++++++ 5 files changed, 174 insertions(+), 1 deletion(-) create mode 100644 test/serena/test_ls_manager.py create mode 100644 test/solidlsp/test_ls_start_cleanup.py diff --git a/CHANGELOG.md b/CHANGELOG.md index e60a96fb..7d8dfa43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ Status of the `main` branch. Changes prior to the next official version change w - Fix: `read_only` restriction in project definition was not applied to base tool set when in single-project context (#1938) * Language Servers: + - Fix: Exceptions raised during `LanguageServerManager.start` did not stop the language server subprocess if it was + already started (#1949) - Fix: Dart's `$/analyzerStatus` notifications were logged as unhandled-method warnings during analysis (#1855) - Fix: clojure-lsp was not told that Serena sends `workspace/didChangeWatchedFiles`, so changes made outside Serena's own edit tools (a git checkout, another editor, a build step) need not invalidate diff --git a/src/serena/ls_manager.py b/src/serena/ls_manager.py index 2dd3aaf4..513e3651 100644 --- a/src/serena/ls_manager.py +++ b/src/serena/ls_manager.py @@ -144,6 +144,9 @@ class LanguageServerManager: language_servers[thread.ls_id] = thread.language_server # If any server failed to start up, raise an exception and stop all started language servers. + # A server whose own thread raised has already stopped its own process, since + # SolidLanguageServer.start() cleans up after itself on failure; only the servers that + # started successfully (and are therefore absent from `exceptions`) still need stopping. # We intentionally fail fast here. The user's intention is to work with all the specified languages, # so if any of them is not available, it is better to make symbolic tool calls fail, bringing the issue to the # user's attention instead of silently continuing with a subset of the language servers and potentially diff --git a/src/solidlsp/ls.py b/src/solidlsp/ls.py index cba90723..13497121 100644 --- a/src/solidlsp/ls.py +++ b/src/solidlsp/ls.py @@ -3179,7 +3179,17 @@ class SolidLanguageServer(ABC): """ log.info(f"Starting language server {self.language_server.ls_id} for {self.language_server.repository_root_path}") self.server_started = True - self._start_server() + try: + self._start_server() + except Exception: + # `_start_server()` may raise after already spawning the underlying process (a + # capability assertion or an initialize() timeout firing post-spawn). Stop it here + # so a failed start never leaves an orphaned process behind, regardless of caller. + if self.is_running(): + self.stop() + else: + self.server_started = False + raise return self def stop(self, shutdown_timeout: float = 2.0) -> None: diff --git a/test/serena/test_ls_manager.py b/test/serena/test_ls_manager.py new file mode 100644 index 00000000..ae6994b6 --- /dev/null +++ b/test/serena/test_ls_manager.py @@ -0,0 +1,103 @@ +import subprocess +import sys +import time + +import psutil +import pytest + +from serena.ls_manager import LanguageServerManager, LanguageServerManagerInitialisationError +from solidlsp.ls_config import LanguageServerId + + +def _pid_alive(pid: int) -> bool: + # Not `os.kill(pid, 0)`: that is a no-op liveness probe on POSIX, but on Windows + # `os.kill` has no signal-0 special case and falls through to `TerminateProcess(handle, + # 0)`, so the "check" can itself kill (or, if the pid was already recycled, terminate an + # unrelated process) instead of only reading process-table state. + return psutil.pid_exists(pid) + + +class _FakeLanguageServer: + """Duck-typed stand-in for SolidLanguageServer: `from_languages` only calls + `.start()`/`.is_running()`/`.stop()` on it, never isinstance-checks the object. + """ + + def __init__(self, ls_id: LanguageServerId, should_fail: bool): + self.ls_id = ls_id + self.should_fail = should_fail + self.proc: subprocess.Popen | None = None + self._running = False + + def start(self) -> "_FakeLanguageServer": + # Mirrors SolidLanguageServer.start(): the OS subprocess is spawned as part of + # start(), a capability/initialize() check can still raise after that process is + # already running, and start() stops the process it just spawned before re-raising + # (see SolidLanguageServer.start). + self.proc = subprocess.Popen([sys.executable, "-c", "import time; time.sleep(100)"]) + if self.should_fail: + self._running = True + self.stop() + raise RuntimeError(f"simulated: capability assertion failed after initialize() ({self.ls_id.value})") + self._running = True + return self + + def is_running(self) -> bool: + return self._running + + def stop(self, shutdown_timeout: float = 2.0) -> None: + if self.proc is not None and self.proc.poll() is None: + self.proc.terminate() + try: + self.proc.wait(timeout=shutdown_timeout) + except subprocess.TimeoutExpired: + self.proc.kill() + self._running = False + + +class _FakeLanguageServerFactory: + def __init__(self, fail_ids: set[LanguageServerId]): + self.fail_ids = fail_ids + self.created: dict[LanguageServerId, _FakeLanguageServer] = {} + + def create_language_server(self, ls_id: LanguageServerId) -> _FakeLanguageServer: + ls = _FakeLanguageServer(ls_id, should_fail=ls_id in self.fail_ids) + self.created[ls_id] = ls + return ls + + +@pytest.fixture +def _cleanup_leftover_pids(): + """Belt-and-braces: kill any spawned test subprocess still alive after the test body, + so a regression in the fix under test cannot leak a real OS process past this test. + """ + pids: list[int] = [] + yield pids + for pid in pids: + try: + psutil.Process(pid).kill() + except psutil.NoSuchProcess: + pass + + +def test_from_languages_stops_process_of_server_that_raises_after_spawning(_cleanup_leftover_pids): + """End-to-end: a language server whose `start()` spawns its OS subprocess and then raises + (e.g. a capability assertion or an initialize() timeout firing after the process is + already up) must not leak that process, and a sibling that started successfully must be + stopped too since `from_languages` fails the whole batch. The failing server cleans up its + own process (SolidLanguageServer.start()); `from_languages` is responsible only for + stopping the siblings that succeeded. + """ + ok_id = LanguageServerId("python") + failing_id = LanguageServerId("rust") + factory = _FakeLanguageServerFactory(fail_ids={failing_id}) + + with pytest.raises(LanguageServerManagerInitialisationError): + LanguageServerManager.from_languages([ok_id, failing_id], factory, project=None) + + time.sleep(0.3) + pids = {ls_id: ls.proc.pid for ls_id, ls in factory.created.items() if ls.proc is not None} + _cleanup_leftover_pids.extend(pids.values()) + + assert set(pids) == {ok_id, failing_id} + assert not _pid_alive(pids[ok_id]), "the successfully-started server's process should be stopped" + assert not _pid_alive(pids[failing_id]), "the process spawned by the server that raised post-spawn must not leak" diff --git a/test/solidlsp/test_ls_start_cleanup.py b/test/solidlsp/test_ls_start_cleanup.py new file mode 100644 index 00000000..7e574311 --- /dev/null +++ b/test/solidlsp/test_ls_start_cleanup.py @@ -0,0 +1,55 @@ +from unittest.mock import MagicMock + +import pytest + +from solidlsp.ls import SolidLanguageServer + + +class _RaisingLanguageServer(SolidLanguageServer): + """Bypasses SolidLanguageServer.__init__ (like DummyLanguageServer in + test_rename_didopen.py) so _start_server can be made to raise after the underlying + process is already considered running, without spinning up a real language server. + """ + + def _start_server(self) -> None: + raise RuntimeError("simulated: capability assertion failed after initialize()") + + def _create_base_initialize_params(self) -> dict: + return {} + + +def _make_server(is_running: bool) -> _RaisingLanguageServer: + server = object.__new__(_RaisingLanguageServer) + server.ls_id = "python" + server.repository_root_path = "/tmp/project" + server.server_started = False + server.server = MagicMock() + server.server.is_running.return_value = is_running + return server + + +def test_start_stops_process_when_start_server_raises_after_spawning(): + """_start_server() spawning the OS process and then raising (e.g. a capability + assertion or an initialize() timeout firing post-spawn) must not leak that process: + start() itself stops it before re-raising. + """ + server = _make_server(is_running=True) + + with pytest.raises(RuntimeError, match="capability assertion"): + server.start() + + server.server.stop.assert_called_once() + assert server.server_started is False + + +def test_start_does_not_call_stop_when_start_server_raises_before_spawning(): + """When _start_server() raises before any process was ever spawned (e.g. the language + server binary is missing), there is nothing to stop. + """ + server = _make_server(is_running=False) + + with pytest.raises(RuntimeError, match="capability assertion"): + server.start() + + server.server.stop.assert_not_called() + assert server.server_started is False From e473dd64b5c01a01b9d93a456d6f5e64f6e43c8a Mon Sep 17 00:00:00 2001 From: mameikagou Date: Wed, 26 Aug 2026 04:00:19 +0800 Subject: [PATCH 6/6] docs: fix issue template user guide link --- .../issue--bug--performance-problem--question-.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/ISSUE_TEMPLATE/issue--bug--performance-problem--question-.md b/.github/ISSUE_TEMPLATE/issue--bug--performance-problem--question-.md index a438edfe..e756e3db 100644 --- a/.github/ISSUE_TEMPLATE/issue--bug--performance-problem--question-.md +++ b/.github/ISSUE_TEMPLATE/issue--bug--performance-problem--question-.md @@ -10,7 +10,7 @@ assignees: '' Preconditions: - [ ] I have made sure it's an actual issue, not a question (use [GitHub Discussions](https://github.com/oraios/serena/discussions) instead). -- [ ] I have consulted the [user guide](https://oraios.github.io/serena/02-usage/) and verified that the issue cannot be resolved by adjusting configuration/following recommended workflows. +- [ ] I have consulted the [user guide](https://oraios.github.io/serena/02-usage/000_intro.html) and verified that the issue cannot be resolved by adjusting configuration/following recommended workflows. - [ ] I have looked for similar issues and discussions, including closed ones. Issue details: