Merge branch 'main' into fix/1871-nextflow-scan-flush-flag

This commit is contained in:
Dr. Dominik Jain authored and GitHub committed 2026-09-02 13:06:49 +02:00
commit f41f33c904
12 files changed
+395 -6

No files matched your search

@@ -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:
+12
View File
@@ -5,18 +5,30 @@ 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: Nextflow's `_flush_deferred_workspace_scan` marked the workspace scan flushed even when both
of its `completion` probes failed, permanently skipping the flush (and silencing retries) for the
rest of the session (#1871)
- 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
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
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`
+3 -2
View File
@@ -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
+4
View File
@@ -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
+3
View File
@@ -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
@@ -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,
},
@@ -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:
"""
+11 -1
View File
@@ -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:
+103
View File
@@ -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"
@@ -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))
+55
View File
@@ -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
+107 -1
View File
@@ -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