From 8ebb247daee92e694820d8a9dde8fc5f1ff7b01e Mon Sep 17 00:00:00 2001 From: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> Date: Sun, 9 Aug 2026 12:24:43 +0100 Subject: [PATCH 1/8] Force the Nextflow server's deferred workspace scan before finding references (#1832) The Nextflow language server does not scan the workspace when it says it does. `LanguageService.initialize` -- which the "Initializing" progress notification we wait for wraps -- only marks the workspace as unscanned and clears the AST cache. The scan itself happens in `LanguageService.update0`, behind a 1s debounce, and only on a round that finds no pending file change; a round that does find one re-defers it via `updateLater`. The first `didOpen` of a session therefore pushes the scan out by at least one further round, so a references request issued right after it sees an AST cache holding nothing but the file it just opened and answers with an empty list. Force the scan rather than waiting for it. `completion` is one of the two requests that call `updateNow` before consulting their provider, and `DebouncingExecutor.executeNow` cancels the pending debounce and runs the update synchronously on the request thread, so the response is not sent until that round has finished. Two rounds suffice whatever the workspace size: the first drains the pending file changes -- which is what re-defers the scan -- and the second finds nothing pending, so it compiles the whole workspace before replying. Polling the workspace symbol index instead would be unsound: `LanguageService.symbol` neither awaits the update nor holds the monitor that `update` synchronises on. On a generated 1050-file workspace, polling it made the server raise `java.util.ConcurrentModificationException` from inside the scan, non-deterministically (6, 3 and 0 times across three runs). The existing cross-file reference test did not catch the empty results because the `language_server` fixture is module-scoped: earlier tests in the class had already opened main.nf and forced the server to compile it, so the assertion passed for a reason that does not hold in a real session. Run alone, it failed. The added test starts its own server and opens nothing but the defining file. Verified on the 1050-file workspace, where the symbol has exactly 100 references: without the fix 0 are found, with it all 100, in 2.5s. Co-Authored-By: Claude Opus 5 --- .../nextflow_language_server.py | 35 +++++++++++++++++-- test/solidlsp/nextflow/test_nextflow_basic.py | 20 +++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/solidlsp/language_servers/nextflow_language_server.py b/src/solidlsp/language_servers/nextflow_language_server.py index d39cc834..0b3fcf17 100644 --- a/src/solidlsp/language_servers/nextflow_language_server.py +++ b/src/solidlsp/language_servers/nextflow_language_server.py @@ -88,6 +88,10 @@ class NextflowLanguageServer(SolidLanguageServer): self._active_progress_tokens: set[str] = set() self._progress_lock = threading.Lock() + # Whether the server's deferred workspace scan has been observed to complete + # (see _flush_deferred_workspace_scan). + self._workspace_scan_flushed = False + def _create_dependency_provider(self) -> LanguageServerDependencyProvider: return self.DependencyProvider(self._custom_settings, self._ls_resources_dir) @@ -297,6 +301,32 @@ class NextflowLanguageServer(SolidLanguageServer): else: log.warning("Nextflow LS did not signal scan completion within %.0fs; proceeding anyway", _WORKSPACE_SCAN_TIMEOUT) + def _flush_deferred_workspace_scan(self, relative_file_path: str) -> None: + """ + Forces the workspace scan, which the server defers past the first request of a session. + + The scan runs in ``LanguageService.update0`` behind a 1s debounce, and only on a round with no + pending file change; the session's first ``didOpen`` re-defers it, leaving references to be + answered from an AST cache holding just that one file. + + ``completion`` calls ``updateNow``, which runs the update synchronously before replying, so two + such requests force the scan whatever the workspace size: the first drains the pending change, + the second finds none and scans. Their results are discarded. + """ + if self._workspace_scan_flushed: + return + + params: lsp_types.CompletionParams = { + "textDocument": {"uri": self._resolve_file_uri(relative_file_path)}, + "position": {"line": 0, "character": 0}, + } + for _ in range(2): + try: + self.server.send.completion(params) + except Exception as e: + log.debug("Completion request used to flush the Nextflow workspace scan failed: %s", e) + self._workspace_scan_flushed = True + @override def _send_references_request(self, relative_file_path: str, line: int, column: int) -> list[lsp_types.Location] | None: """ @@ -309,13 +339,14 @@ class NextflowLanguageServer(SolidLanguageServer): come back empty. Sending a ``documentSymbol`` request for the same file first is a cheap way to block until the update has been applied. """ + self._flush_deferred_workspace_scan(relative_file_path) self.server.send.document_symbol({"textDocument": {"uri": self._resolve_file_uri(relative_file_path)}}) return super()._send_references_request(relative_file_path, line, column) @override def _get_wait_time_for_cross_file_referencing(self) -> float: - """Small safety buffer; the actual synchronisation happens in _send_references_request.""" - return 1.0 + """No blind wait is needed: _send_references_request synchronises with the server explicitly.""" + return 0.0 @override def _document_symbols_cache_fingerprint(self) -> Hashable: diff --git a/test/solidlsp/nextflow/test_nextflow_basic.py b/test/solidlsp/nextflow/test_nextflow_basic.py index 4f7243d1..d60bd2cd 100644 --- a/test/solidlsp/nextflow/test_nextflow_basic.py +++ b/test/solidlsp/nextflow/test_nextflow_basic.py @@ -7,6 +7,7 @@ import pytest from solidlsp import SolidLanguageServer from solidlsp.ls_config import LanguageServerId from solidlsp.ls_utils import SymbolUtils +from test.conftest import start_default_ls_context pytestmark = pytest.mark.skipif(shutil.which("java") is None, reason="Nextflow language server requires a Java runtime") @@ -74,6 +75,25 @@ class TestNextflowLanguageServer: # main.nf, 0-indexed line 12: greetings = GREET(names) assert ("main.nf", 12) in ref_lines, f"invocation of GREET in main.nf not found. Found: {ref_lines}" + def test_find_references_across_files_on_a_fresh_server(self) -> None: + """Cross-file references must not depend on another request having opened the referencing file. + + The module-scoped ``language_server`` fixture is shared, so by the time + ``test_find_references_across_files`` runs, earlier tests have already opened main.nf and thereby + forced the server to compile it. This test starts its own server and touches nothing but the + defining file, which is what a real session's first request looks like. + """ + module_path = os.path.join("modules", "greet", "main.nf") + with start_default_ls_context(LanguageServerId.NEXTFLOW) as language_server: + symbol = self._get_symbol(language_server, module_path, "GREET") + start = symbol["selectionRange"]["start"] + + refs = language_server.request_references(module_path, start["line"], start["character"]) + + ref_lines = self._reference_lines(refs) + # main.nf, 0-indexed line 12: greetings = GREET(names) + assert ("main.nf", 12) in ref_lines, f"invocation of GREET in main.nf not found. Found: {ref_lines}" + @staticmethod def _reference_lines(references: list[dict]) -> set[tuple[str, int]]: """(relative path, 0-indexed start line) per reference; paths use the platform separator.""" From c687e751af9386f281939156139f6ee19b18e1bf Mon Sep 17 00:00:00 2001 From: Amir Fathi Date: Sun, 9 Aug 2026 05:26:18 -0600 Subject: [PATCH 2/8] fix(gitignore): escape directory names before interpolating into pattern position (#1809) * fix(gitignore): escape directory names before interpolating into pattern position GitignoreParser rescopes patterns from nested .gitignore files by prefixing the containing directory's relative path, but interpolates that path into pattern position without escaping it. A directory whose name contains a gitignore metacharacter (*, ?, [, ], !, #) is then read as glob syntax instead of a literal name, so a directory like a stray "***" venv can turn a scoped pattern into one matching most or all of the project. Fixes #1806 * fix(gitignore): build patterns with '/' only, never os.sep/os.path.join The escaping added in the prior commit inserts literal backslash escape characters before pattern metacharacters. On Windows, os.sep is also backslash, so os.path.join(rel_dir_pattern, line) mixed path-separator backslashes with escape backslashes, and the pre-existing trailing `.replace(os.sep, "/")` normalization step then converted ALL of them indiscriminately -- turning an escaped 'a\[1\]' into 'a/[1/]', which no longer matches the literal directory name. Verified with ntpath (Windows path semantics) on this Linux session: simulating a Windows relpath through the old os.sep/os.path.join/replace chain produces 'a/[1/]/mod.py', which pathspec does not match against 'a[1]/mod.py'; the new '/'-only construction produces 'a\[1\]/mod.py', which matches. Fixed by normalizing rel_dir to '/' immediately and joining every pattern component with a literal '/', removing the now-unneeded (and on Windows actively harmful) blanket separator replace. Also marks the two directory-creation regression tests that use '*'/'?' literally in a directory name as Windows-skipped (those characters are illegal in Windows filenames, so the directories cannot be created there -- a platform limitation of the test, not the fix), and adds a pure-function test for _escape_gitignore_path_component so the '*'/'?' escaping logic still has cross-platform coverage. Found via this PR's own Windows CI run (jobs 91787560091/91787560092/91787560183 on run 30843966565), which failed test_gitignore_dir_name_with_metachars_anchored_pattern with an assertion failure (not the OSError the other two tests hit), before any maintainer had looked at the PR. --- CHANGELOG.md | 5 ++ src/serena/util/file_system.py | 41 +++++++++++---- test/serena/util/test_file_system.py | 79 +++++++++++++++++++++++++++- 3 files changed, 115 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f87f31d..3525c2de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ Status of the `main` branch. Changes prior to the next official version change w * General: - Fix: Race conditions in ProjectServer when used by multiple clients in parallel + - Fix: `GitignoreParser` interpolated a directory's name unescaped into gitignore pattern position; + a directory named with pattern metacharacters (e.g. a stray `***`) could turn a scoped pattern + into one matching far more than intended, silently excluding most or all of the project from + indexing #1806 + - Fix: the README, the Language Support docs page and the project template omitted several already-supported language servers - Fix: a tool call exceeding the timeout blocked the task executor indefinitely; the executor now recovers without user-induced cancellation - Add Grok Build support (context `grok`, setup CLI, hooks) diff --git a/src/serena/util/file_system.py b/src/serena/util/file_system.py index 06cedc8c..fcde0e09 100644 --- a/src/serena/util/file_system.py +++ b/src/serena/util/file_system.py @@ -1,5 +1,6 @@ import logging import os +import re from collections.abc import Callable, Iterator from dataclasses import dataclass, field from pathlib import Path @@ -11,6 +12,18 @@ from sensai.util.logging import LogTime log = logging.getLogger(__name__) +# Characters meaningful to pathspec's gitignore grammar: glob wildcards, bracket expressions, +# the escape character itself, and '!'/'#' which change a whole pattern's meaning when they +# are its first character. Backslash-escaping them makes a literal name safe to interpolate. +_GITIGNORE_PATTERN_SPECIAL_CHARS_RE = re.compile(r"([\\*?\[\]!#])") + + +def _escape_gitignore_path_component(component: str) -> str: + """Escape gitignore/pathspec pattern metacharacters in a single path component (no + separators) so it is matched as a literal name rather than as glob syntax. + """ + return _GITIGNORE_PATTERN_SPECIAL_CHARS_RE.sub(r"\\\1", component) + class ScanResult(NamedTuple): """Result of scanning a directory.""" @@ -223,11 +236,21 @@ class GitignoreParser: """ patterns = [] - # Get the relative path from repo root to the gitignore directory - rel_dir = os.path.relpath(gitignore_dir, self.repo_root) + # Get the relative path from repo root to the gitignore directory. Normalize to + # forward slashes immediately: os.path.relpath returns native separators, but + # gitignore/pathspec patterns are always POSIX-style, and on Windows os.sep is + # backslash -- the same character pathspec uses as its escape character. Building + # the pattern with any raw os.sep would make a later blanket backslash->slash + # normalization indistinguishable from the escape backslashes below. + rel_dir = os.path.relpath(gitignore_dir, self.repo_root).replace(os.sep, "/") if rel_dir == ".": rel_dir = "" + # rel_dir is a filesystem path, but the code below interpolates it into pattern + # position; escape each of its components so a directory name containing pattern + # metacharacters (e.g. "***") is matched literally instead of as glob syntax. + rel_dir_pattern = "/".join(_escape_gitignore_path_component(part) for part in rel_dir.split("/")) if rel_dir else rel_dir + for line in content.splitlines(): # Strip trailing whitespace (but preserve leading whitespace for now) line = line.rstrip() @@ -256,20 +279,23 @@ class GitignoreParser: if is_anchored: line = line[1:] - # Adjust pattern based on gitignore file location + # Adjust pattern based on gitignore file location. Joined with a literal "/", + # never os.path.join/os.sep: gitignore patterns are always POSIX-style, and on + # Windows os.sep is backslash, indistinguishable from the escape backslashes + # rel_dir_pattern may already contain. if rel_dir: if is_anchored: # Anchored patterns are relative to the gitignore directory - adjusted_pattern = os.path.join(rel_dir, line) + adjusted_pattern = f"{rel_dir_pattern}/{line}" else: # Non-anchored patterns can match anywhere below the gitignore directory # We need to preserve this behavior if line.startswith("**/"): # Even if pattern starts with **, it should still be scoped to the subdirectory - adjusted_pattern = os.path.join(rel_dir, line) + adjusted_pattern = f"{rel_dir_pattern}/{line}" else: # Add the directory prefix but also allow matching in subdirectories - adjusted_pattern = os.path.join(rel_dir, "**", line) + adjusted_pattern = f"{rel_dir_pattern}/**/{line}" else: if is_anchored: # Anchored patterns in root should only match at root level @@ -283,9 +309,6 @@ class GitignoreParser: if is_negation: adjusted_pattern = "!" + adjusted_pattern - # Normalize path separators to forward slashes (gitignore uses forward slashes) - adjusted_pattern = adjusted_pattern.replace(os.sep, "/") - patterns.append(adjusted_pattern) return patterns diff --git a/test/serena/util/test_file_system.py b/test/serena/util/test_file_system.py index ca8a0872..1e1272b9 100644 --- a/test/serena/util/test_file_system.py +++ b/test/serena/util/test_file_system.py @@ -1,11 +1,13 @@ import os import shutil +import sys import tempfile from pathlib import Path +import pytest from pathspec import PathSpec -from serena.util.file_system import GitignoreParser, GitignoreSpec, match_path +from serena.util.file_system import GitignoreParser, GitignoreSpec, _escape_gitignore_path_component, match_path class TestGitignoreParser: @@ -715,6 +717,81 @@ src/*.o # foo.txt in other/ should NOT be ignored (outside foo/ subtree) assert not parser.should_ignore("other/foo.txt"), "other/foo.txt should NOT be ignored by foo/.gitignore" + @pytest.mark.skipif(sys.platform == "win32", reason="'*' is illegal in Windows filenames; this directory name cannot exist there") + def test_gitignore_dir_name_with_glob_metachars_is_not_a_wildcard(self): + """A directory named with glob metacharacters (e.g. a stray '***' venv) must be + matched literally, not interpreted as a pattern (issue #1806). + """ + test_dir = self.repo_path / "test_metachar_dirname" + test_dir.mkdir() + (test_dir / "pkg").mkdir() + (test_dir / "***").mkdir() + + (test_dir / "pkg" / "mod.py").touch() + (test_dir / "***" / "junk.txt").touch() + + gitignore = test_dir / "***" / ".gitignore" + gitignore.write_text("*\n") + + parser = GitignoreParser(str(test_dir)) + + # pkg/mod.py is outside the "***" directory and must not be affected by its gitignore + assert not parser.should_ignore("pkg/mod.py"), "pkg/mod.py should not be ignored by ***/.gitignore" + + # junk.txt inside "***" is still ignored by its own gitignore's "*" pattern + assert parser.should_ignore("***/junk.txt"), "***/junk.txt should be ignored by its own gitignore" + + def test_gitignore_dir_name_with_metachars_anchored_pattern(self): + """Same as above, for the anchored-pattern join site.""" + test_dir = self.repo_path / "test_metachar_dirname_anchored" + test_dir.mkdir() + (test_dir / "pkg").mkdir() + (test_dir / "a[1]").mkdir() + + (test_dir / "pkg" / "mod.py").touch() + (test_dir / "a[1]" / "mod.py").touch() + + gitignore = test_dir / "a[1]" / ".gitignore" + gitignore.write_text("/mod.py\n") + + parser = GitignoreParser(str(test_dir)) + + assert not parser.should_ignore("pkg/mod.py"), "pkg/mod.py should not be ignored by a[1]/.gitignore" + assert parser.should_ignore("a[1]/mod.py"), "a[1]/mod.py should be ignored by its own /mod.py pattern" + + @pytest.mark.skipif(sys.platform == "win32", reason="'?' is illegal in Windows filenames; this directory name cannot exist there") + def test_gitignore_dir_name_with_metachars_implicit_double_star_pattern(self): + """A non-anchored pattern with no leading '**/' is joined as (rel_dir, "**", line): + the third join site. An unescaped '?' in the directory name would leak the pattern + into a sibling directory whose name merely matches the wildcard (issue #1806). + """ + test_dir = self.repo_path / "test_metachar_dirname_implicit_doublestar" + test_dir.mkdir() + (test_dir / "q?").mkdir() + (test_dir / "qA").mkdir() + (test_dir / "qA" / "sub").mkdir() + + (test_dir / "qA" / "sub" / "mod.py").touch() + + gitignore = test_dir / "q?" / ".gitignore" + gitignore.write_text("mod.py\n") + + parser = GitignoreParser(str(test_dir)) + + # "q?/.gitignore" must not reach into the sibling "qA/" directory just because "?" + # would match the "A" in "qA" if left as a wildcard. + assert not parser.should_ignore("qA/sub/mod.py"), "qA/sub/mod.py should not be ignored by q?/.gitignore" + + def test_escape_gitignore_path_component_escapes_all_metachars(self): + """Pure-function coverage for the '*' and '?' escaping cases the two directory-creation + tests above cannot exercise on Windows (those characters are illegal in Windows + filenames, so this runs on every platform instead of touching the filesystem). + """ + assert _escape_gitignore_path_component("***") == "\\*\\*\\*" + assert _escape_gitignore_path_component("q?") == "q\\?" + assert _escape_gitignore_path_component("a[1]") == "a\\[1\\]" + assert _escape_gitignore_path_component("plain") == "plain" + class TestGitignoreParserPermissionError: """Test PermissionError handling in GitignoreParser.""" From 1b459899cae0ecdb5edded35f1f134669b933d33 Mon Sep 17 00:00:00 2001 From: Pavel Hajek Date: Sun, 9 Aug 2026 13:31:30 +0200 Subject: [PATCH 3/8] Add Wolfram Language support via official LSPServer paclet (#1108) Adds support for Wolfram Language (.wl, .wls files) using the official WolframResearch LSPServer paclet, which communicates via stdio. Requires Wolfram Mathematica 13.0+ or Wolfram Engine 12.1+. The WolframKernel is located via WOLFRAM_PATH, the system PATH, common install locations, or ls_path in ls_specific_settings. Includes language server implementation, test repo, test suite (skipped gracefully when WolframKernel is unavailable), and documentation updates. Co-Authored-By: Claude Opus 4.6 Co-Authored-By: Claude Fable 5 Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- CHANGELOG.md | 4 + README.md | 2 +- docs/01-about/020_programming-languages.md | 4 +- pyproject.toml | 1 + src/serena/resources/project.template.yml | 22 +-- .../wolfram_language_server.py | 183 ++++++++++++++++++ src/solidlsp/ls_config.py | 11 ++ test/conftest.py | 9 + .../repos/wolfram/test_repo/lib/helper.wl | 3 + .../resources/repos/wolfram/test_repo/main.wl | 17 ++ test/solidlsp/wolfram/__init__.py | 0 test/solidlsp/wolfram/test_wolfram_basic.py | 66 +++++++ 12 files changed, 309 insertions(+), 13 deletions(-) create mode 100644 src/solidlsp/language_servers/wolfram_language_server.py create mode 100644 test/resources/repos/wolfram/test_repo/lib/helper.wl create mode 100644 test/resources/repos/wolfram/test_repo/main.wl create mode 100644 test/solidlsp/wolfram/__init__.py create mode 100644 test/solidlsp/wolfram/test_wolfram_basic.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 3525c2de..022448dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,10 @@ Status of the `main` branch. Changes prior to the next official version change w `intellicode_version`, `intellicode_xmx` and `intellicode_xms` settings remain accepted but are ignored #1821 - Kotlin: update the managed Kotlin LSP from `261.13587.0` to `262.9593.0`, including support for the new platform-specific archive layout and Windows ARM64 builds + - Add support for Wolfram Language via the official [WolframResearch LSPServer](https://github.com/WolframResearch/LSPServer) paclet. + Requires Wolfram Mathematica 13.0+ or Wolfram Engine 12.1+. Set `WOLFRAM_PATH` environment variable or configure + `ls_path` in `ls_specific_settings`. Supports .wl and .wls files with diagnostics, document symbols, + within-file references, hover documentation, and formatting. - Nix/nixd: support custom `ls_path` launchers and external JSON settings through `config_path` #1737 - Fix: Nix/nixd diagnostics now use published diagnostics instead of the unsupported `textDocument/diagnostic` request, which terminated nixd #1802 diff --git a/README.md b/README.md index bd58d919..6e16380f 100644 --- a/README.md +++ b/README.md @@ -110,7 +110,7 @@ Serena incorporates a powerful abstraction layer for the integration of language The underlying language servers are typically open-source projects or at least freely available for use. When using Serena's language server backend, we provide **support for over 40 programming languages**, including -Ada / SPARK, AL, Angular, Ansible, Bash, BSL, C#, C/C++, Clojure, Crystal, CUE, Dart, Deno, Elixir, Elm, Erlang, Fortran, F#, GDScript, Gleam, GLSL, Go, Groovy, Haskell, Haxe, HLSL, HTML, Java, JavaScript, JSON, Julia, Kotlin, LaTeX, Lean 4, Lua, Luau, Markdown, MATLAB, mSL, Nextflow, Nix, OCaml, Pascal, Perl, PHP, PowerShell, Python, QML, R, Rego, Ruby, Rust, Scala, SCSS / Sass / CSS, Solidity, Svelte, Swift, SystemVerilog, Terraform, TOML, TypeScript, Vue, WGSL, YAML, and Zig. +Ada / SPARK, AL, Angular, Ansible, Bash, BSL, C#, C/C++, Clojure, Crystal, CUE, Dart, Deno, Elixir, Elm, Erlang, Fortran, F#, GDScript, Gleam, GLSL, Go, Groovy, Haskell, Haxe, HLSL, HTML, Java, JavaScript, JSON, Julia, Kotlin, LaTeX, Lean 4, Lua, Luau, Markdown, MATLAB, mSL, Nextflow, Nix, OCaml, Pascal, Perl, PHP, PowerShell, Python, QML, R, Rego, Ruby, Rust, Scala, SCSS / Sass / CSS, Solidity, Svelte, Swift, SystemVerilog, Terraform, TOML, TypeScript, Vue, WGSL, Wolfram Language, YAML, and Zig. ### The Serena JetBrains Plugin diff --git a/docs/01-about/020_programming-languages.md b/docs/01-about/020_programming-languages.md index 62d581bb..331c1581 100644 --- a/docs/01-about/020_programming-languages.md +++ b/docs/01-about/020_programming-languages.md @@ -177,8 +177,10 @@ Some languages require additional installations or setup steps, as noted. globals, which the plain TypeScript language server does not; overlaps `typescript` on file extensions, so it is not auto-detected and must be set as the language explicitly — do not also enable `typescript` for the same files) -* **Vue** +* **Vue** (3.x with TypeScript; requires Node.js v18+ and npm; supports .vue Single File Components with monorepo detection) +* **Wolfram Language** + (requires Wolfram Mathematica 13.0+ or Wolfram Engine 12.1+; uses the official [WolframResearch LSPServer](https://github.com/WolframResearch/LSPServer) paclet; supports .wl and .wls files; references are within-file only) * **YAML** * **JSON** (experimental; must be explicitly added to the languages list; requires Node.js and npm) diff --git a/pyproject.toml b/pyproject.toml index 9944d840..96ed8e12 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -381,6 +381,7 @@ markers = [ "ada: language server running for Ada / SPARK (uses AdaCore Ada Language Server)", "qml: language server running for QML (uses Qt's qmlls)", "gleam: language server running for Gleam (uses the Gleam compiler's bundled `gleam lsp`)", + "wolfram: language server running for Wolfram Language (requires Mathematica 13.0+ or Wolfram Engine 12.1+)", ] [tool.codespell] diff --git a/src/serena/resources/project.template.yml b/src/serena/resources/project.template.yml index 5b1d0293..5008aa08 100644 --- a/src/serena/resources/project.template.yml +++ b/src/serena/resources/project.template.yml @@ -6,17 +6,17 @@ project_name: "project_name" # bsl clojure cpp cpp_ccls crystal # csharp csharp_omnisharp cue dart deno # elixir elm erlang fortran fsharp -# gdscript go groovy haskell haxe -# hlsl html java json julia -# kotlin latex lean4 lua luau -# markdown matlab msl nextflow nix -# ocaml pascal perl php php_phpactor -# php_phpantom powershell python python_basedpyright python_jedi -# python_pyrefly python_ty qml r rego -# ruby ruby_solargraph rust scala scss -# solidity svelte swift systemverilog terraform -# toml typescript typescript_vts vue yaml -# zig +# gdscript gleam go groovy haskell +# haxe hlsl html java json +# julia kotlin latex lean4 lua +# luau markdown matlab msl nextflow +# nix ocaml pascal perl php +# php_phpactor php_phpantom powershell python python_basedpyright +# python_jedi python_pyrefly python_ty qml r +# rego ruby ruby_solargraph rust scala +# scss solidity svelte swift systemverilog +# terraform toml typescript typescript_vts vue +# wolfram yaml zig # (This list may be outdated; generated with scripts/print_language_list.py; # For the current list, see values of the LanguageServerId enum here: # https://github.com/oraios/serena/blob/main/src/solidlsp/ls_config.py) diff --git a/src/solidlsp/language_servers/wolfram_language_server.py b/src/solidlsp/language_servers/wolfram_language_server.py new file mode 100644 index 00000000..a3c3790d --- /dev/null +++ b/src/solidlsp/language_servers/wolfram_language_server.py @@ -0,0 +1,183 @@ +"""Wolfram Language server integration using the official WolframResearch LSPServer paclet.""" + +import glob +import logging +import os +import platform +import shutil + +from solidlsp.ls import LanguageServerDependencyProvider, LanguageServerDependencyProviderSinglePath, SolidLanguageServer +from solidlsp.ls_config import LanguageServerConfig +from solidlsp.settings import SolidLSPSettings + +log = logging.getLogger(__name__) + +WOLFRAM_PATH_ENV_VAR = "WOLFRAM_PATH" + + +class WolframLanguageServer(SolidLanguageServer): + """ + Wolfram Language server using the official WolframResearch LSPServer paclet + (https://github.com/WolframResearch/LSPServer), which is bundled with Wolfram + installations and communicates via stdio. + + Requires Wolfram Mathematica 13.0+ or Wolfram Engine 12.1+. + The WolframKernel executable is discovered via the ``ls_path`` entry in + ``ls_specific_settings.wolfram``, the ``WOLFRAM_PATH`` environment variable + (pointing to the executable or the installation directory), the system PATH, + or common installation locations. + """ + + def __init__(self, config: LanguageServerConfig, repository_root_path: str, solidlsp_settings: SolidLSPSettings): + super().__init__(config, repository_root_path, None, "wolfram", solidlsp_settings) + + def _create_dependency_provider(self) -> LanguageServerDependencyProvider: + return self.DependencyProvider(self._custom_settings, self._ls_resources_dir) + + class DependencyProvider(LanguageServerDependencyProviderSinglePath): + def _get_or_install_core_dependency(self) -> str: + """ + Discover the WolframKernel executable. + + :return: path to the WolframKernel executable + :raises FileNotFoundError: if WolframKernel cannot be found + """ + return _find_wolfram_kernel() + + def _create_launch_command(self, core_path: str) -> list[str]: + return [core_path, "-noprompt", "-noinit", "-run", 'Needs["LSPServer`"];LSPServer`StartServer[]'] + + def _create_base_initialize_params(self) -> dict: + return { + "capabilities": { + "workspace": {"workspaceFolders": True}, + "textDocument": { + "definition": {"dynamicRegistration": True}, + "references": {"dynamicRegistration": True}, + "documentSymbol": { + "dynamicRegistration": True, + "hierarchicalDocumentSymbolSupport": True, + }, + "hover": {"dynamicRegistration": True, "contentFormat": ["markdown", "plaintext"]}, + "formatting": {"dynamicRegistration": True}, + "publishDiagnostics": {"relatedInformation": True}, + }, + }, + } + + def _start_server(self) -> None: + def do_nothing(params: dict) -> None: + return + + def window_log_message(msg: dict) -> None: + log.info(f"Wolfram LSP: window/logMessage: {msg}") + + self.server.on_request("client/registerCapability", do_nothing) + self.server.on_notification("window/logMessage", window_log_message) + self.server.on_notification("$/progress", do_nothing) + self.server.on_notification("textDocument/publishDiagnostics", do_nothing) + + log.info("Starting Wolfram LSPServer process") + self.server.start() + + initialize_params = self._create_initialize_params() + init_response = self.server.send.initialize(initialize_params) + + log.info(f"Wolfram LSP capabilities: {list(init_response.get('capabilities', {}).keys())}") + + self.server.notify.initialized({}) + log.info("Wolfram LSPServer initialized and ready.") + + +def _find_wolfram_kernel() -> str: + """Find the WolframKernel executable via WOLFRAM_PATH, the system PATH, or common install locations.""" + # 1. WOLFRAM_PATH environment variable (executable or installation directory) + env_path = os.environ.get(WOLFRAM_PATH_ENV_VAR) + if env_path: + if os.path.isfile(env_path) and os.access(env_path, os.X_OK): + log.info(f"Using WolframKernel from {WOLFRAM_PATH_ENV_VAR}: {env_path}") + return env_path + kernel_in_dir = _find_kernel_in_install_dir(env_path) + if kernel_in_dir: + log.info(f"Using WolframKernel from {WOLFRAM_PATH_ENV_VAR} directory: {kernel_in_dir}") + return kernel_in_dir + + # 2. System PATH + kernel_path = shutil.which("WolframKernel") + if kernel_path: + log.info(f"Using WolframKernel from PATH: {kernel_path}") + return kernel_path + + # 3. Common installation locations + system = platform.system() + search_locations: list[str] = [] + + if system == "Darwin": + search_locations = [ + "/Applications/Mathematica.app/Contents/MacOS/WolframKernel", + "/Applications/Wolfram.app/Contents/MacOS/WolframKernel", + "/Applications/Wolfram Engine.app/Contents/MacOS/WolframKernel", + ] + for pattern in [ + "/Applications/Mathematica*.app/Contents/MacOS/WolframKernel", + "/Applications/Wolfram*.app/Contents/MacOS/WolframKernel", + ]: + search_locations.extend(sorted(glob.glob(pattern), reverse=True)) + + elif system == "Linux": + search_locations = [ + "/usr/local/bin/WolframKernel", + "/usr/bin/WolframKernel", + ] + for pattern in [ + "/usr/local/Wolfram/Mathematica/*/Executables/WolframKernel", + "/usr/local/Wolfram/WolframEngine/*/Executables/WolframKernel", + "/opt/Wolfram/Mathematica/*/Executables/WolframKernel", + ]: + search_locations.extend(sorted(glob.glob(pattern), reverse=True)) + + elif system == "Windows": + for pattern in [ + "C:\\Program Files\\Wolfram Research\\Mathematica\\*\\WolframKernel.exe", + "C:\\Program Files\\Wolfram Research\\Wolfram Engine\\*\\WolframKernel.exe", + ]: + search_locations.extend(sorted(glob.glob(pattern), reverse=True)) + + for location in search_locations: + if os.path.isfile(location) and os.access(location, os.X_OK): + log.info(f"Found WolframKernel at: {location}") + return location + + raise FileNotFoundError( + "WolframKernel not found. Please either:\n" + f"1. Set the {WOLFRAM_PATH_ENV_VAR} environment variable to your Wolfram installation\n" + "2. Add WolframKernel to your system PATH\n" + "3. Configure ls_path in ls_specific_settings.wolfram\n" + "4. Install Wolfram Mathematica (13.0+) or Wolfram Engine (12.1+) from https://www.wolfram.com/" + ) + + +def _find_kernel_in_install_dir(install_dir: str) -> str | None: + """Try to locate WolframKernel within a Wolfram installation directory.""" + system = platform.system() + + if system == "Darwin": + candidates = [ + os.path.join(install_dir, "Contents", "MacOS", "WolframKernel"), + os.path.join(install_dir, "MacOS", "WolframKernel"), + ] + elif system == "Windows": + candidates = [ + os.path.join(install_dir, "WolframKernel.exe"), + ] + else: + candidates = [ + os.path.join(install_dir, "Executables", "WolframKernel"), + os.path.join(install_dir, "WolframKernel"), + ] + + for candidate in candidates: + if os.path.isfile(candidate) and os.access(candidate, os.X_OK): + return candidate + + return None diff --git a/src/solidlsp/ls_config.py b/src/solidlsp/ls_config.py index d4f05de0..67d77b02 100644 --- a/src/solidlsp/ls_config.py +++ b/src/solidlsp/ls_config.py @@ -196,6 +196,11 @@ class LanguageServerId(str, Enum): it reports no symbols for them, so they are not treated as source files here). Automatically downloads the language server JAR; requires Java 17+ (JAVA_HOME or 'java' on PATH). """ + WOLFRAM = "wolfram" + """Wolfram Language server using the official WolframResearch LSPServer paclet. + Requires Wolfram Mathematica 13.0+ or Wolfram Engine 12.1+. + Set WOLFRAM_PATH environment variable or configure ls_path in ls_specific_settings. + """ # Experimental or deprecated Language Servers TYPESCRIPT_VTS = "typescript_vts" """Use the typescript language server through the natively bundled vscode extension via https://github.com/yioneko/vtsls""" @@ -598,6 +603,8 @@ class LanguageServerId(str, Enum): # only scripts: the language server does have a service for .config files, but it provides # no symbols for them, so treating them as source files would only pollute the symbol index return FilenameMatcher(".nf") + case self.WOLFRAM: + return FilenameMatcher(".wl", ".wls") case self.HTML: return FilenameMatcher(".html", ".htm") case self.SCSS: @@ -901,6 +908,10 @@ class LanguageServerId(str, Enum): from solidlsp.language_servers.nextflow_language_server import NextflowLanguageServer return NextflowLanguageServer + case self.WOLFRAM: + from solidlsp.language_servers.wolfram_language_server import WolframLanguageServer + + return WolframLanguageServer case self.HTML: from solidlsp.language_servers.vscode_html_language_server import VsCodeHtmlLanguageServer diff --git a/test/conftest.py b/test/conftest.py index a44be041..c06076b6 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -492,6 +492,15 @@ def _determine_disabled_language_servers() -> list[LanguageServerId]: if is_ci: result.append(LanguageServerId.KOTLIN) # IntelliJ-based Kotlin LSP crashes on JVM restart under CI memory limits + # Disable Wolfram tests if WolframKernel is not available (checked with the same + # discovery logic used by the language server itself) + from solidlsp.language_servers.wolfram_language_server import _find_wolfram_kernel + + try: + _find_wolfram_kernel() + except FileNotFoundError: + result.append(LanguageServerId.WOLFRAM) + return result diff --git a/test/resources/repos/wolfram/test_repo/lib/helper.wl b/test/resources/repos/wolfram/test_repo/lib/helper.wl new file mode 100644 index 00000000..8502c91e --- /dev/null +++ b/test/resources/repos/wolfram/test_repo/lib/helper.wl @@ -0,0 +1,3 @@ +sayHello[name_String] := StringJoin["Hello, ", name, "!"] + +formatResult[value_] := StringJoin["Result: ", ToString[value]] diff --git a/test/resources/repos/wolfram/test_repo/main.wl b/test/resources/repos/wolfram/test_repo/main.wl new file mode 100644 index 00000000..8b3f2826 --- /dev/null +++ b/test/resources/repos/wolfram/test_repo/main.wl @@ -0,0 +1,17 @@ +Get["lib/helper.wl"] + +calculateSum[a_, b_] := a + b + +processData[data_List] := Module[{result}, + result = Total[data]; + formatResult[result] +] + +main[] := Module[{result, greeting}, + result = calculateSum[5, 3]; + greeting = sayHello["World"]; + Print[greeting]; + Print[result] +] + +main[] diff --git a/test/solidlsp/wolfram/__init__.py b/test/solidlsp/wolfram/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/test/solidlsp/wolfram/test_wolfram_basic.py b/test/solidlsp/wolfram/test_wolfram_basic.py new file mode 100644 index 00000000..44b20385 --- /dev/null +++ b/test/solidlsp/wolfram/test_wolfram_basic.py @@ -0,0 +1,66 @@ +import pytest + +from solidlsp.ls import SolidLanguageServer +from solidlsp.ls_config import LanguageServerId +from test.conftest import language_server_tests_enabled + +pytestmark = [ + pytest.mark.wolfram, + pytest.mark.skipif( + not language_server_tests_enabled(LanguageServerId.WOLFRAM), reason="Wolfram tests disabled (WolframKernel not available)" + ), +] + +# Note: the LSPServer paclet computes textDocument/references per document, i.e. only +# references within the file containing the queried position are returned; cross-file +# references are not supported by the language server. + + +class TestWolframLanguageServer: + @pytest.mark.parametrize("language_server", [LanguageServerId.WOLFRAM], indirect=True) + def test_wolfram_symbols(self, language_server: SolidLanguageServer): + """ + Test if we can find the top-level symbols in the main.wl file. + """ + all_symbols, _ = language_server.request_document_symbols("main.wl").get_all_symbols_and_roots() + symbol_names = {s["name"] for s in all_symbols} + assert "calculateSum" in symbol_names + assert "main" in symbol_names + + @pytest.mark.parametrize("language_server", [LanguageServerId.WOLFRAM], indirect=True) + def test_wolfram_symbols_in_subdirectory(self, language_server: SolidLanguageServer): + """ + Test if we can find the top-level symbols in a file in a subdirectory. + """ + all_symbols, _ = language_server.request_document_symbols("lib/helper.wl").get_all_symbols_and_roots() + symbol_names = {s["name"] for s in all_symbols} + assert "sayHello" in symbol_names + assert "formatResult" in symbol_names + + @pytest.mark.parametrize("language_server", [LanguageServerId.WOLFRAM], indirect=True) + def test_wolfram_within_file_references(self, language_server: SolidLanguageServer): + """ + Test finding references to a function within the same file. + """ + # 'calculateSum' is defined on line 2 of main.wl and called on line 10 (0-based) + references = language_server.request_references("main.wl", line=2, column=0) + + reference_lines = {(ref["relativePath"], ref["range"]["start"]["line"]) for ref in references} + assert ("main.wl", 2) in reference_lines, f"Expected the definition site among references, got {reference_lines}" + assert ("main.wl", 10) in reference_lines, f"Expected the call site among references, got {reference_lines}" + + @pytest.mark.parametrize("language_server", [LanguageServerId.WOLFRAM], indirect=True) + def test_wolfram_no_cross_file_references(self, language_server: SolidLanguageServer): + """ + Test documenting that references are computed per document by the LSPServer paclet. + + 'sayHello' is defined in lib/helper.wl and called in main.wl, but querying references + from its definition must only return locations within lib/helper.wl. If this test + starts failing, the language server gained cross-file reference support and the + documentation (docs page, CHANGELOG, module comment above) should be updated. + """ + references = language_server.request_references("lib/helper.wl", line=0, column=0) + + assert references, "Expected at least one within-file reference in lib/helper.wl fixture" + reference_paths = {ref["relativePath"] for ref in references} + assert reference_paths == {"lib/helper.wl"}, f"Expected references only within lib/helper.wl, got {reference_paths}" From c3f5aab59eddab795f55c6b74ee1019584634a21 Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Sun, 9 Aug 2026 13:48:03 +0200 Subject: [PATCH 4/8] Improve project health-check : Disable symbol groupers, remove flawed pattern search test Resolves #1826 --- CHANGELOG.md | 8 +++++--- src/serena/cli.py | 36 ++++++++++-------------------------- 2 files changed, 15 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 022448dc..a24fc9bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,9 +28,11 @@ Status of the `main` branch. Changes prior to the next official version change w * CLI: - Fix: `start-mcp-server` help text for `--project-from-cwd` falsely promised a fallback to the CWD, which was removed in v1.0.0 #1773 - - Fix: `project health-check` always exited with code 0, even when the check failed, so callers - (CI, scripts) could not act on its verdict; it now exits with code 1 on failure. A `find_symbol` - result without any matches is now reported as a failure rather than as a warning. + - Improve `project health-check`: + - Fix: Process always exited with code 0, even when the check failed, so callers + (CI, scripts) could not act on its verdict; it now exits with code 1 on failure. A `find_symbol` + result without any matches is now reported as a failure rather than as a warning. + - Disable symbol groupers, remove flawed pattern search test * Tools: - `find_symbol`, `jet_brains_find_symbol`: Change tool description to improve tool search results in clients that load tools dynamically diff --git a/src/serena/cli.py b/src/serena/cli.py index f6471733..b2b04ac3 100644 --- a/src/serena/cli.py +++ b/src/serena/cli.py @@ -921,14 +921,14 @@ class ProjectCommands(AutoRegisteringGroup): @click.argument("project", type=click.Path(exists=True, file_okay=False, dir_okay=True), default=os.getcwd()) def health_check(project: str) -> None: """ - Perform a comprehensive health check of the project's tools and language server. + Perform a basic health check which checks whether language server tools are functional for the project. :param project: path to the project directory, defaults to the current working directory. """ # NOTE: completely written by Claude Code, only functionality was reviewed, not implementation from serena.agent import SerenaAgent from serena.project import Project - from serena.tools import FindReferencingSymbolsTool, FindSymbolTool, GetSymbolsOverviewTool, SearchForPatternTool + from serena.tools import FindReferencingSymbolsTool, FindSymbolTool, GetSymbolsOverviewTool logging.configure(level=logging.INFO) project_path = os.path.abspath(project) @@ -977,7 +977,6 @@ class ProjectCommands(AutoRegisteringGroup): overview_tool = agent.get_tool(GetSymbolsOverviewTool) find_symbol_tool = agent.get_tool(FindSymbolTool) find_refs_tool = agent.get_tool(FindReferencingSymbolsTool) - search_pattern_tool = agent.get_tool(SearchForPatternTool) # Test 1: Get symbols overview log.info("Testing GetSymbolsOverviewTool on file: %s", target_file) @@ -1006,42 +1005,27 @@ class ProjectCommands(AutoRegisteringGroup): # Test 2: FindSymbolTool log.info("Testing FindSymbolTool for symbol: %s", symbol_name) - find_symbol_result = agent.execute_task( - lambda: find_symbol_tool.apply(symbol_name, relative_path=target_file, include_body=True) - ) + with find_symbol_tool.symbol_dict_grouper.disabled_context(): + find_symbol_result = agent.execute_task( + lambda: find_symbol_tool.apply(symbol_name, relative_path=target_file, include_body=True) + ) find_symbol_data = json.loads(find_symbol_result) log.info("FindSymbolTool found %d matches for symbol %s", len(find_symbol_data), symbol_name) # Test 3: FindReferencingSymbolsTool log.info("Testing FindReferencingSymbolsTool for symbol: %s", symbol_name) try: - find_refs_result = agent.execute_task(lambda: find_refs_tool.apply(symbol_name, relative_path=target_file)) - find_refs_data = json.loads(find_refs_result) - log.info("FindReferencingSymbolsTool found %d references for symbol %s", len(find_refs_data), symbol_name) + with find_refs_tool.symbol_dict_grouper.disabled_context(): + find_refs_result = agent.execute_task(lambda: find_refs_tool.apply(symbol_name, relative_path=target_file)) + find_refs_data = json.loads(find_refs_result) + log.info("FindReferencingSymbolsTool found %d references for symbol %s", len(find_refs_data), symbol_name) except Exception as e: log.warning("FindReferencingSymbolsTool failed for symbol %s: %s", symbol_name, str(e)) - find_refs_data = [] - - # Test 4: SearchForPatternTool to verify references - log.info("Testing SearchForPatternTool for pattern: %s", symbol_name) - try: - search_result = agent.execute_task( - lambda: search_pattern_tool.apply(substring_pattern=symbol_name, restrict_search_to_code_files=True) - ) - search_data = json.loads(search_result) - pattern_matches = sum(len(matches) for matches in search_data.values()) - log.info("SearchForPatternTool found %d pattern matches for %s", pattern_matches, symbol_name) - except Exception as e: - log.warning("SearchForPatternTool failed for pattern %s: %s", symbol_name, str(e)) - pattern_matches = 0 # Verify tools worked as expected if not find_symbol_data: raise ProjectCommands._HealthCheckFailure("FindSymbolTool returned no results") - if len(find_refs_data) == 0 and pattern_matches == 0: - log.warning("Both FindReferencingSymbolsTool and SearchForPatternTool found no matches - this might indicate an issue") - log.info("Health check completed successfully") # expected failures and unexpected exceptions are reported alike: both mean the project's From ec18c48202a7d3298a94fd69f88db85b9dd902f8 Mon Sep 17 00:00:00 2001 From: Amir Fathi Date: Sat, 8 Aug 2026 00:57:30 +0000 Subject: [PATCH 5/8] Fix: LSP process-group cleanup no longer requires process-table enumeration An independently-started LSP process is already its own POSIX process group leader (start_new_session=True), so cleanup can signal that known group directly via os.killpg instead of walking the tree with psutil.Process.children(recursive=True), which requires system-wide process-table enumeration that can be denied even for processes we started and own (#1818). --- CHANGELOG.md | 3 + src/solidlsp/ls_process.py | 12 +- src/solidlsp/util/subprocess_util.py | 46 +++- test/solidlsp/test_process_group_cleanup.py | 227 ++++++++++++++++++++ 4 files changed, 284 insertions(+), 4 deletions(-) create mode 100644 test/solidlsp/test_process_group_cleanup.py diff --git a/CHANGELOG.md b/CHANGELOG.md index a24fc9bc..b8139e9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ Status of the `main` branch. Changes prior to the next official version change w a directory named with pattern metacharacters (e.g. a stray `***`) could turn a scoped pattern into one matching far more than intended, silently excluding most or all of the project from indexing #1806 + - Fix: cleanup of an independently-started LSP process and its children required enumerating the + system process table (`psutil`), which can be denied even for processes Serena owns in a + sandboxed environment; cleanup now signals the known process group directly (#1818) - Fix: the README, the Language Support docs page and the project template omitted several already-supported language servers - Fix: a tool call exceeding the timeout blocked the task executor indefinitely; the executor now recovers without user-induced cancellation diff --git a/src/solidlsp/ls_process.py b/src/solidlsp/ls_process.py index c4c1d9e7..8e8cca2d 100644 --- a/src/solidlsp/ls_process.py +++ b/src/solidlsp/ls_process.py @@ -1,6 +1,7 @@ import asyncio import json import logging +import os import socket import subprocess import threading @@ -510,6 +511,9 @@ class StdioLanguageServer(LanguageServerInterface): self.process_launch_info = process_launch_info self.process: subprocess.Popen[bytes] | None = None self.start_independent_lsp_process = start_independent_lsp_process + self._process_group_id: int | None = None + """the POSIX process group ID owned by ``self.process``, captured at launch time (see + ``_start``); ``None`` on Windows or when the process was not started in its own session""" self._stdin_lock = threading.Lock() @@ -523,6 +527,8 @@ class StdioLanguageServer(LanguageServerInterface): self.process_launch_info, start_new_session=self.start_independent_lsp_process ) self.process = process + # start_new_session=True makes the new process its own group leader, so its PGID is its PID. + self._process_group_id = process.pid if (self.start_independent_lsp_process and os.name == "posix") else None # Check if process terminated immediately if process.returncode is not None: @@ -559,10 +565,14 @@ class StdioLanguageServer(LanguageServerInterface): # Ignore errors here, we are proceeding to terminate anyway. # terminate the process subprocess_util.terminate_process_tree_with_kill_fallback( - self.process, terminate_timeout=timeout, process_name=f"LS[{self.ls_id.value}]" + self.process, + terminate_timeout=timeout, + process_name=f"LS[{self.ls_id.value}]", + process_group_id=self._process_group_id, ) finally: self.process = None + self._process_group_id = None @staticmethod def _safely_close_pipe(pipe: IO[AnyStr] | None) -> None: diff --git a/src/solidlsp/util/subprocess_util.py b/src/solidlsp/util/subprocess_util.py index e1e48598..b6912715 100644 --- a/src/solidlsp/util/subprocess_util.py +++ b/src/solidlsp/util/subprocess_util.py @@ -246,7 +246,33 @@ def _signal_process_tree(process: subprocess.Popen[bytes], terminate: bool = Tru signal_process(process) -def terminate_process_tree_with_kill_fallback(process: subprocess.Popen, terminate_timeout: float, process_name: str = "Process") -> None: +def _signal_process_group(pgid: int, terminate: bool = True) -> None: + """ + Sends a signal to every process in the given POSIX process group by group ID, without + enumerating the system process table. Requires the caller to know the group already exists + and is owned by us (see ``terminate_process_tree_with_kill_fallback``'s ``process_group_id``). + + :param pgid: the process group ID to signal + :param terminate: if True, signal terminate (SIGTERM), otherwise signal kill (SIGKILL) + """ + sig = signal.SIGTERM if terminate else signal.SIGKILL + try: + os.killpg(pgid, sig) + except ProcessLookupError: + # the group is already gone; nothing left to signal + pass + except PermissionError as e: + log.warning(f"Permission denied signaling process group {pgid} with {sig.name}: {e}") + except Exception as e: + log.warning(f"Unexpected error signaling process group {pgid} with {sig.name}: {e}") + + +def terminate_process_tree_with_kill_fallback( + process: subprocess.Popen, + terminate_timeout: float, + process_name: str = "Process", + process_group_id: int | None = None, +) -> None: """ Attempts to terminate the given process and its children by signaling them to terminate, and if that fails (i.e. they don't exit within the given timeout), forcefully kills them. @@ -256,9 +282,23 @@ def terminate_process_tree_with_kill_fallback(process: subprocess.Popen, termina :param process: the process to terminate :param terminate_timeout: the time to wait for the process to terminate gracefully before killing it :param process_name: the name of the process (used for logging purposes); should start with capital letter + :param process_group_id: if given, the POSIX process group ID that ``process`` leads (i.e. it was + launched with ``start_new_session=True``, so its PGID equals its PID). When set, cleanup + signals the whole group directly via ``os.killpg`` instead of walking the process tree with + ``psutil``, which requires system-wide process-table enumeration (``sysctl(KERN_PROC_ALL)`` on + macOS) that can be denied even for processes we started and own. Only pass this for a process + that was started in its own session: signaling the group of a process that shares ours would + also signal us. """ log.debug(f"Terminating process {process.pid}, current status: {process.poll()}") - _signal_process_tree(process, terminate=True) + + def signal_tree(terminate: bool) -> None: + if process_group_id is not None: + _signal_process_group(process_group_id, terminate=terminate) + else: + _signal_process_tree(process, terminate=terminate) + + signal_tree(terminate=True) try: log.debug(f"Waiting for process {process.pid} to terminate...") exit_code = process.wait(timeout=terminate_timeout) @@ -266,7 +306,7 @@ def terminate_process_tree_with_kill_fallback(process: subprocess.Popen, termina except subprocess.TimeoutExpired: # If termination failed, forcefully kill the process log.warning(f"{process_name} (pid={process.pid}) termination timed out, killing process forcefully...") - _signal_process_tree(process, terminate=False) + signal_tree(terminate=False) try: exit_code = process.wait(timeout=2.0) log.info(f"{process_name} killed successfully with exit code {exit_code}.") diff --git a/test/solidlsp/test_process_group_cleanup.py b/test/solidlsp/test_process_group_cleanup.py new file mode 100644 index 00000000..0a37ccc0 --- /dev/null +++ b/test/solidlsp/test_process_group_cleanup.py @@ -0,0 +1,227 @@ +"""Regression tests for issue #1818: LSP process-group cleanup must not require enumerating +the system process table (``psutil.Process.children(recursive=True)``), which can be denied +even for processes we started and own (``Operation not permitted`` from +``sysctl(KERN_PROC_ALL)`` in a sandboxed macOS environment). + +``StdioLanguageServer`` already starts every LSP process in its own session +(``start_independent_lsp_process`` defaults to True, see ``ls_config.py``), which makes the +process its own POSIX process group leader, with a PGID equal to its PID at launch; +``subprocess_util.terminate_process_tree_with_kill_fallback`` accepts that PGID as +``process_group_id`` and, when given, signals the whole group directly via ``os.killpg`` +instead of walking the tree with ``psutil``. No language markers: these exercise +``subprocess_util`` directly with plain Python helper processes and run in catch-all. +""" + +from __future__ import annotations + +import os +import platform +import signal +import subprocess +import sys +import textwrap +import time + +import psutil +import pytest + +from solidlsp.util.subprocess_util import _signal_process_group, terminate_process_tree_with_kill_fallback + +pytestmark = pytest.mark.skipif(platform.system() == "Windows", reason="process groups / os.killpg are POSIX-specific") + + +def _group_is_gone(pgid: int) -> bool: + try: + os.killpg(pgid, 0) + return False + except ProcessLookupError: + return True + + +def _wait_until(predicate, timeout: float = 5.0, interval: float = 0.1) -> bool: + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + if predicate(): + return True + time.sleep(interval) + return predicate() + + +def _process_alive(pid: int) -> bool: + try: + os.kill(pid, 0) + return True + except ProcessLookupError: + return False + + +class _DenyingProcess: + """Stand-in for ``psutil.Process`` that always raises ``AccessDenied``, used to simulate + a sandboxed environment denying process-table enumeration without needing one. A real class + (not a plain function) so it substitutes cleanly for ``psutil.Process`` in the + ``subprocess.Popen | psutil.Process`` type union that subprocess_util.py evaluates eagerly. + """ + + def __new__(cls, pid: int) -> "_DenyingProcess": + raise psutil.AccessDenied(pid) + + +def _spawn_ready(src: str) -> subprocess.Popen: + """Starts ``src`` in its own session and waits for it to print READY, mirroring + test_pdeathsig.py's driver pattern (deterministic sync instead of a blind sleep). + """ + proc = subprocess.Popen([sys.executable, "-c", src], start_new_session=True, stdout=subprocess.PIPE, text=True) + ready_line = proc.stdout.readline() + assert ready_line.strip() == "READY", f"helper process failed to start: {ready_line!r}" + return proc + + +class TestSignalProcessGroup: + def test_nonexistent_group_is_treated_as_already_clean(self) -> None: + bogus_pgid = 2**30 # not a real PGID; must be handled like an already-gone group + _signal_process_group(bogus_pgid, terminate=True) # must not raise + + def test_permission_error_is_caught_and_logged(self, monkeypatch: pytest.MonkeyPatch) -> None: + def raise_eperm(pgid: int, sig: int) -> None: + raise PermissionError("simulated sandbox denial") + + monkeypatch.setattr(os, "killpg", raise_eperm) + _signal_process_group(12345, terminate=True) # must not raise + + +class TestTerminateProcessTreeWithKillFallback: + def test_terminates_child_and_grandchild_via_group_id(self) -> None: + """A focused POSIX cleanup test: start a child and grandchild in one new session, + invoke the cleanup utility by PGID only, and verify both are gone, without ever + calling psutil. + """ + src = textwrap.dedent( + """ + import subprocess, sys, time + subprocess.Popen([sys.executable, "-c", "import time; time.sleep(300)"]) + print("READY", flush=True) + time.sleep(300) + """ + ) + proc = _spawn_ready(src) + pgid = proc.pid + try: + assert not _group_is_gone(pgid), "process group should be alive before cleanup" + terminate_process_tree_with_kill_fallback(proc, terminate_timeout=5.0, process_group_id=pgid) + assert _wait_until(lambda: _group_is_gone(pgid)), f"process group {pgid} survived cleanup" + finally: + if proc.poll() is None: + proc.kill() + proc.wait(timeout=2.0) + + def test_falls_back_to_kill_when_group_ignores_sigterm(self) -> None: + src = textwrap.dedent( + """ + import signal, time + signal.signal(signal.SIGTERM, signal.SIG_IGN) + print("READY", flush=True) + time.sleep(300) + """ + ) + proc = _spawn_ready(src) + pgid = proc.pid + try: + terminate_process_tree_with_kill_fallback(proc, terminate_timeout=1.0, process_group_id=pgid) + assert _wait_until(lambda: _group_is_gone(pgid)), f"process group {pgid} survived SIGKILL fallback" + finally: + if proc.poll() is None: + proc.kill() + proc.wait(timeout=2.0) + + def test_already_exited_group_does_not_raise(self) -> None: + proc = subprocess.Popen([sys.executable, "-c", "pass"], start_new_session=True) + pgid = proc.pid + proc.wait(timeout=5.0) + assert _wait_until(lambda: _group_is_gone(pgid)), "group should be released once the leader is reaped" + terminate_process_tree_with_kill_fallback(proc, terminate_timeout=1.0, process_group_id=pgid) + + def test_process_group_id_none_never_calls_killpg(self, monkeypatch: pytest.MonkeyPatch) -> None: + """start_independent_lsp_process=False means the process shares our own process + group, so cleanup must never call killpg: doing so could signal Serena itself. + """ + + def fail_if_called(pgid: int, sig: int) -> None: + raise AssertionError("os.killpg must not be called when process_group_id is None") + + monkeypatch.setattr(os, "killpg", fail_if_called) + proc = subprocess.Popen([sys.executable, "-c", "import time; time.sleep(300)"]) + try: + terminate_process_tree_with_kill_fallback(proc, terminate_timeout=5.0, process_group_id=None) + proc.wait(timeout=5.0) + finally: + if proc.poll() is None: + proc.kill() + proc.wait(timeout=2.0) + + +class TestPsutilDenialConsequences: + """Demonstrates the actual production consequence when process-table enumeration is denied, + without depending on macOS: ``psutil.AccessDenied`` is the same exception class regardless of + which syscall the platform used to deny it. Without a known ``process_group_id``, + ``_signal_process_tree`` falls back to signaling only the ``Popen`` object it was given (see + its ``except (psutil.NoSuchProcess, psutil.AccessDenied, Exception): pass`` branch), so a + child the leader spawned itself leaks. Passing the group id (this fix) avoids psutil + entirely and reaps it regardless. + """ + + @staticmethod + def _spawn_leader_with_child() -> tuple[subprocess.Popen, int]: + src = textwrap.dedent( + """ + import subprocess, sys, time + child = subprocess.Popen([sys.executable, "-c", "import time; time.sleep(300)"]) + print(child.pid, flush=True) + print("READY", flush=True) + time.sleep(300) + """ + ) + proc = subprocess.Popen([sys.executable, "-c", src], start_new_session=True, stdout=subprocess.PIPE, text=True) + child_pid = int(proc.stdout.readline().strip()) + ready_line = proc.stdout.readline() + assert ready_line.strip() == "READY", f"helper process failed to start: {ready_line!r}" + return proc, child_pid + + def test_psutil_denial_without_group_id_leaks_the_leaders_child(self, monkeypatch: pytest.MonkeyPatch) -> None: + proc, child_pid = self._spawn_leader_with_child() + + monkeypatch.setattr("solidlsp.util.subprocess_util.psutil.Process", _DenyingProcess) + try: + # The call shape every site used before this fix: no process_group_id. + terminate_process_tree_with_kill_fallback(proc, terminate_timeout=2.0, process_name="leader") + assert _wait_until(lambda: proc.poll() is not None), "leader itself should still die (direct signal, not enumerated)" + time.sleep(0.3) + assert _process_alive(child_pid), ( + "expected the leader's own child to leak when psutil is denied and no process_group_id is given " + "(this is the #1818 defect: process-table denial silently drops descendants)" + ) + finally: + for pid in (child_pid, proc.pid): + try: + os.kill(pid, signal.SIGKILL) + except ProcessLookupError: + pass + if proc.poll() is None: + proc.wait(timeout=2.0) + + def test_process_group_id_survives_psutil_denial(self, monkeypatch: pytest.MonkeyPatch) -> None: + proc, child_pid = self._spawn_leader_with_child() + pgid = proc.pid + + monkeypatch.setattr("solidlsp.util.subprocess_util.psutil.Process", _DenyingProcess) + try: + terminate_process_tree_with_kill_fallback(proc, terminate_timeout=5.0, process_group_id=pgid) + assert _wait_until(lambda: _group_is_gone(pgid)), f"process group {pgid} survived cleanup despite psutil denial" + assert not _process_alive(child_pid), "leader's child leaked even though the group id path avoids psutil entirely" + finally: + for pid in (child_pid, proc.pid): + try: + os.kill(pid, signal.SIGKILL) + except ProcessLookupError: + pass + if proc.poll() is None: + proc.wait(timeout=2.0) From bdc9a2de2dae42637ab4ee165322d679919ce0aa Mon Sep 17 00:00:00 2001 From: xor <127287135+xormania@users.noreply.github.com> Date: Sun, 9 Aug 2026 10:00:55 -0400 Subject: [PATCH 6/8] CI: cache npm dependencies for web test fixtures (#1698) --- .github/workflows/pytest.yml | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index ee126e5c..b83544fd 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -27,9 +27,11 @@ # # CACHING # Per-batch language-server cache keyed by os+batch (language-servers--), -# plus dedicated caches for the uv venv (keyed on uv.lock), swiftly, ZLS, FPC and -# the Go build/bin. Most niche toolchains restore from cache, so that batch is fast -# despite its size. +# plus dedicated caches for the uv venv (keyed on uv.lock), swiftly, ZLS, FPC, the +# Go build/bin, and npm for the web fixtures (Angular node_modules keyed on its +# lockfile; the npm download cache via setup-node, which also warms the svelte +# fixture's test-time `npm ci`). Most niche toolchains restore from cache, so that +# batch is fast despite its size. # # TESTING / SKIPS # All language skip logic is centralized in test/conftest.py @@ -110,6 +112,13 @@ jobs: uses: actions/setup-node@v7 with: node-version: '24.x' + # npm download cache, catch-all only (empty string disables caching in other + # batches -- they run no npm installs). Warms the svelte fixture's test-time + # `npm ci` and the global/LS npm installs; keyed on the committed lockfiles. + cache: ${{ matrix.batch == 'catch-all' && 'npm' || '' }} + cache-dependency-path: | + test/resources/repos/angular/test_repo/package-lock.json + test/resources/repos/svelte/test_repo/package-lock.json - name: Ensure cached directory exist before calling cache-related actions shell: bash run: | @@ -719,8 +728,17 @@ jobs: echo "PERL_MB_OPT=--install_base \"$HOME/perl5\"" >> $GITHUB_ENV echo "PERL_MM_OPT=INSTALL_BASE=$HOME/perl5" >> $GITHUB_ENV echo "$HOME/perl5/bin" >> $GITHUB_PATH - - name: Install npm deps for Angular test repo + # The Angular fixture's node_modules is cached directly (exact lockfile match, like + # the uv venv cache) so the npm install below is skipped entirely on a hit. + - name: Cache Angular test repo node_modules if: matrix.batch == 'catch-all' + id: cache-angular-npm + uses: actions/cache@v3 + with: + path: test/resources/repos/angular/test_repo/node_modules + key: angular-npm-${{ runner.os }}-lock-${{ hashFiles('test/resources/repos/angular/test_repo/package-lock.json') }} + - name: Install npm deps for Angular test repo + if: matrix.batch == 'catch-all' && steps.cache-angular-npm.outputs.cache-hit != 'true' shell: bash run: | if [ -d test/resources/repos/angular/test_repo ]; then From c9f0ab80f117505121999eb2229902e5db6dc096 Mon Sep 17 00:00:00 2001 From: xor <127287135+xormania@users.noreply.github.com> Date: Sun, 9 Aug 2026 10:07:20 -0400 Subject: [PATCH 7/8] SolidLSP: admit Struct, Interface and Constant symbols as reference containers (#1831) find_referencing_symbols degraded to file-level attribution for any reference located inside a Go struct body, interface body or const group: request_containing_symbol found no admissible container, so request_referencing_symbols logged "Could not find containing symbol ... Returning file symbol instead" and fell back to the file symbol, losing the answer the tool exists to give. Root cause: the candidate-container filter admitted only Python's container kinds {Method, Function, Class} (plus Variable via the one-liner-exempt path). The language server is not at fault: hierarchicalDocumentSymbolSupport is declared and the DocumentSymbol tree is complete, but Go structs arrive as SymbolKind.Struct, interfaces as Interface and const-group members as Constant - none of which were admitted, so every candidate was rejected. Fix, mirroring the existing structure of the filter: - Struct and Interface join the multi-line container whitelist, - Constant joins Variable on the one-liner-exempt path (const-group members are one-liners by nature, exactly like the module-level variables that path already serves). The Python suite shows no attribution change (test/solidlsp/python/test_symbol_retrieval.py: 80 passed). Languages whose containers use the added kinds (e.g. Rust and C structs, TypeScript interfaces) get the same admission; verified for Go and Python only. Regression coverage: new Go fixture containment_sample.go with a const-iota group, a struct field and an interface method all referencing a named type; four tests assert containing-symbol and referencing-symbol attribution, and all four fail against the pre-change ls.py (verified). Full go-marker suite: 21 passed, 1 skipped. --- CHANGELOG.md | 4 ++ src/solidlsp/ls.py | 23 +++++-- .../repos/go/test_repo/containment_sample.go | 21 +++++++ test/solidlsp/go/test_go_basic.py | 60 ++++++++++++++++++- 4 files changed, 101 insertions(+), 7 deletions(-) create mode 100644 test/resources/repos/go/test_repo/containment_sample.go diff --git a/CHANGELOG.md b/CHANGELOG.md index a24fc9bc..21f5744f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -93,6 +93,10 @@ Status of the `main` branch. Changes prior to the next official version change w `Deno.*` globals, which the plain TypeScript language server does not. Overlaps TypeScript on file extensions, so it is not auto-detected and must be selected explicitly; requires the `deno` CLI on PATH + - Fix: `find_referencing_symbols` reported file-level containers for references located inside Go + struct bodies, interface bodies and `const` groups, because containing-symbol detection admitted + only Python's container kinds; `Struct`, `Interface` and `Constant` symbols are now admitted as + containers * JetBrains: - `jet_brains_find_symbol`: Disallow wildcard-only search, delegating to overview tool if request is for file diff --git a/src/solidlsp/ls.py b/src/solidlsp/ls.py index d0706a72..fb809e63 100644 --- a/src/solidlsp/ls.py +++ b/src/solidlsp/ls.py @@ -2533,8 +2533,9 @@ class SolidLanguageServer(ABC): ) -> ls_types.UnifiedSymbolInformation | None: """ Finds the first symbol containing the position for the given file. - For Python, container symbols are considered to be those with kinds corresponding to - functions, methods, or classes (typically: Function (12), Method (6), Class (5)). + Container symbols are those with kinds corresponding to functions, methods, classes, structs + and interfaces, as well as variables and constants (which may be one-liners, e.g. members + of a Go ``const`` group). The method operates as follows: - Request the document symbols for the file. @@ -2546,7 +2547,7 @@ class SolidLanguageServer(ABC): among those above the given line. - If no container candidates are found, return None. - :param relative_file_path: The relative path to the Python file. + :param relative_file_path: The relative path to the file. :param line: The 0-indexed line number. :param column: The 0-indexed column (also called character). If not passed, the lookup will be based only on the line. @@ -2589,8 +2590,15 @@ class SolidLanguageServer(ABC): location["relativePath"] = relative_file_path location["uri"] = Path(absolute_file_path).as_uri() - # Allowed container kinds, currently only for Python - container_symbol_kinds = {ls_types.SymbolKind.Method, ls_types.SymbolKind.Function, ls_types.SymbolKind.Class} + # allowed container kinds; Struct and Interface cover languages (e.g. Go, Rust, C) whose + # containers are not reported as classes + container_symbol_kinds = { + ls_types.SymbolKind.Method, + ls_types.SymbolKind.Function, + ls_types.SymbolKind.Class, + ls_types.SymbolKind.Struct, + ls_types.SymbolKind.Interface, + } def is_position_in_range(line: int, range_d: ls_types.Range) -> bool: start = range_d["start"] @@ -2613,7 +2621,10 @@ class SolidLanguageServer(ABC): for s in document_symbols.iter_symbols() if s["kind"] in container_symbol_kinds and s["location"]["range"]["start"]["line"] != s["location"]["range"]["end"]["line"] ] - var_containers = [s for s in document_symbols.iter_symbols() if s["kind"] == ls_types.SymbolKind.Variable] + # variables and constants are admitted even as one-liners (e.g. members of a Go const group) + var_containers = [ + s for s in document_symbols.iter_symbols() if s["kind"] in {ls_types.SymbolKind.Variable, ls_types.SymbolKind.Constant} + ] candidate_containers.extend(var_containers) if not candidate_containers: diff --git a/test/resources/repos/go/test_repo/containment_sample.go b/test/resources/repos/go/test_repo/containment_sample.go new file mode 100644 index 00000000..c8ccc861 --- /dev/null +++ b/test/resources/repos/go/test_repo/containment_sample.go @@ -0,0 +1,21 @@ +package main + +type Severity int + +const ( + SevLow Severity = iota + SevHigh +) + +type Alert struct { + Level Severity + Msg string +} + +func (a Alert) IsHigh() bool { + return a.Level == SevHigh +} + +type Notifier interface { + Notify(level Severity) error +} diff --git a/test/solidlsp/go/test_go_basic.py b/test/solidlsp/go/test_go_basic.py index d623eb87..3f8f9b04 100644 --- a/test/solidlsp/go/test_go_basic.py +++ b/test/solidlsp/go/test_go_basic.py @@ -4,11 +4,13 @@ from pathlib import Path import pytest from serena.symbol import LanguageServerSymbol +from serena.util.text_utils import find_text_coordinates from solidlsp import SolidLanguageServer from solidlsp.ls_config import LanguageServerId +from solidlsp.ls_types import SymbolKind from solidlsp.ls_utils import SymbolUtils from test.conftest import find_identifier_position, get_repo_path, ls_has_verified_implementation_support -from test.solidlsp.conftest import format_symbol_for_assert, has_malformed_name, request_all_symbols +from test.solidlsp.conftest import format_symbol_for_assert, has_malformed_name, read_repo_file, request_all_symbols @pytest.mark.go @@ -88,6 +90,62 @@ class TestGoLanguageServer: assert body.startswith(name), f"Expected grouped var {name} body to start with the identifier, got {body[:24]!r}" assert not body.startswith("var"), f"Grouped var {name} body must not include the 'var' keyword" + @pytest.mark.parametrize("language_server", [LanguageServerId.GO], indirect=True) + def test_request_containing_symbol_const_group_member(self, language_server: SolidLanguageServer) -> None: + """A reference inside a ``const`` group must be attributed to the enclosing constant.""" + file_path = os.path.join("containment_sample.go") + file_content = read_repo_file(language_server, file_path) + coords = find_text_coordinates(file_content, r"SevLow (Severity) = iota") + assert coords is not None, "Could not find the Severity reference in the const group" + containing_symbol = language_server.request_containing_symbol(file_path, coords.line, coords.col) + assert containing_symbol is not None, "Expected a containing symbol for a reference inside a const group" + assert containing_symbol["name"] == "SevLow" + assert containing_symbol["kind"] == SymbolKind.Constant + + @pytest.mark.parametrize("language_server", [LanguageServerId.GO], indirect=True) + def test_request_containing_symbol_struct_field(self, language_server: SolidLanguageServer) -> None: + """A reference in a struct field declaration must be attributed to the enclosing struct.""" + file_path = os.path.join("containment_sample.go") + file_content = read_repo_file(language_server, file_path) + coords = find_text_coordinates(file_content, r"Level (Severity)") + assert coords is not None, "Could not find the Severity reference in the struct field" + containing_symbol = language_server.request_containing_symbol(file_path, coords.line, coords.col) + assert containing_symbol is not None, "Expected a containing symbol for a reference inside a struct body" + assert containing_symbol["name"] == "Alert" + assert containing_symbol["kind"] == SymbolKind.Struct + + @pytest.mark.parametrize("language_server", [LanguageServerId.GO], indirect=True) + def test_request_containing_symbol_interface_method(self, language_server: SolidLanguageServer) -> None: + """A reference in an interface method signature must be attributed to the enclosing interface.""" + file_path = os.path.join("containment_sample.go") + file_content = read_repo_file(language_server, file_path) + coords = find_text_coordinates(file_content, r"Notify\(level (Severity)\)") + assert coords is not None, "Could not find the Severity reference in the interface method" + containing_symbol = language_server.request_containing_symbol(file_path, coords.line, coords.col) + assert containing_symbol is not None, "Expected a containing symbol for a reference inside an interface body" + assert containing_symbol["name"] == "Notifier" + assert containing_symbol["kind"] == SymbolKind.Interface + + @pytest.mark.parametrize("language_server", [LanguageServerId.GO], indirect=True) + def test_request_referencing_symbols_attributes_containers(self, language_server: SolidLanguageServer) -> None: + """References to a type used in a const group, a struct field and an interface method must + be attributed to the constant, the struct and the interface respectively, not to the file. + """ + file_path = os.path.join("containment_sample.go") + all_symbols, _ = language_server.request_document_symbols(file_path).get_all_symbols_and_roots() + severity_symbol = next((sym for sym in all_symbols if sym.get("name") == "Severity"), None) + assert severity_symbol is not None, "Could not find the 'Severity' type symbol in containment_sample.go" + sel_start = severity_symbol["selectionRange"]["start"] + ref_symbols = [ + ref.symbol for ref in language_server.request_referencing_symbols(file_path, sel_start["line"], sel_start["character"]) + ] + assert ref_symbols, "Expected references to the Severity type" + ref_names = {ref["name"] for ref in ref_symbols} + assert "SevLow" in ref_names + assert "Alert" in ref_names + assert "Notifier" in ref_names + assert all(ref["kind"] != SymbolKind.File for ref in ref_symbols), f"File-level fallback attribution in {ref_names}" + if ls_has_verified_implementation_support(LanguageServerId.GO): @pytest.mark.parametrize("language_server", [LanguageServerId.GO], indirect=True) From bd86cf840adeae5ab8df52ba9ff1556f49dc8e97 Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Sun, 9 Aug 2026 17:01:32 +0200 Subject: [PATCH 8/8] Introduce ManagedSubprocess to encapsulate subprocess lifecycle management The knowledge that a process launched with start_new_session=True is its own group leader (and thus that its PGID equals its PID) was previously held by StdioLanguageServer, i.e. the consumer had to know how the launcher works in order to terminate the process correctly. ManagedSubprocess now owns the process group ID alongside the Popen instance, deriving it at construction time, and provides the termination logic; the launcher returns instances of it. StdioLanguageServer merely holds one and accesses the process through its interface. LanguageServerSubprocessLauncher now returns ManagedSubprocess instances and is renamed to ManagedSubprocessLauncher accordingly. --- src/solidlsp/ls_process.py | 63 ++++++++----------- src/solidlsp/util/subprocess_util.py | 94 ++++++++++++++++++++++++---- 2 files changed, 106 insertions(+), 51 deletions(-) diff --git a/src/solidlsp/ls_process.py b/src/solidlsp/ls_process.py index 8e8cca2d..e231d967 100644 --- a/src/solidlsp/ls_process.py +++ b/src/solidlsp/ls_process.py @@ -1,9 +1,7 @@ import asyncio import json import logging -import os import socket -import subprocess import threading import time from abc import ABC, abstractmethod @@ -33,6 +31,7 @@ from solidlsp.lsp_protocol_handler.server import ( make_response, ) from solidlsp.util import subprocess_util +from solidlsp.util.subprocess_util import ManagedSubprocess log = logging.getLogger(__name__) @@ -508,27 +507,21 @@ class StdioLanguageServer(LanguageServerInterface): """ super().__init__(ls_id, determine_log_level, logger, request_timeout) - self.process_launch_info = process_launch_info - self.process: subprocess.Popen[bytes] | None = None - self.start_independent_lsp_process = start_independent_lsp_process - self._process_group_id: int | None = None - """the POSIX process group ID owned by ``self.process``, captured at launch time (see - ``_start``); ``None`` on Windows or when the process was not started in its own session""" - + self._process_launch_info = process_launch_info + self._process: ManagedSubprocess[bytes] | None = None + self._start_independent_lsp_process = start_independent_lsp_process self._stdin_lock = threading.Lock() def is_running(self) -> bool: - return self.process is not None and self.process.returncode is None + return self._process is not None and self._process.returncode is None def _start(self) -> None: - log.info("Starting language server process via command: %s", self.process_launch_info.cmd) + log.info("Starting language server process via command: %s", self._process_launch_info.cmd) - process = subprocess_util.LanguageServerSubprocessLauncher.get_instance().launch( - self.process_launch_info, start_new_session=self.start_independent_lsp_process + process = subprocess_util.ManagedSubprocessLauncher.get_instance().launch( + self._process_launch_info, name=f"LS[{self.ls_id.value}]", start_new_session=self._start_independent_lsp_process ) - self.process = process - # start_new_session=True makes the new process its own group leader, so its PGID is its PID. - self._process_group_id = process.pid if (self.start_independent_lsp_process and os.name == "posix") else None + self._process = process # Check if process terminated immediately if process.returncode is not None: @@ -551,7 +544,7 @@ class StdioLanguageServer(LanguageServerInterface): ).start() def _stop(self, timeout: float) -> None: - if self.process is None: + if self._process is None: log.debug("Server process is None, cannot shutdown.") return @@ -559,20 +552,14 @@ class StdioLanguageServer(LanguageServerInterface): # send LSP shutdown and close stdin to signal no more input try: self._send_shutdown_in_thread() - self._safely_close_pipe(self.process.stdin) + self._safely_close_pipe(self._process.stdin) except Exception as e: log.debug(f"Exception during graceful shutdown: {e}") # Ignore errors here, we are proceeding to terminate anyway. # terminate the process - subprocess_util.terminate_process_tree_with_kill_fallback( - self.process, - terminate_timeout=timeout, - process_name=f"LS[{self.ls_id.value}]", - process_group_id=self._process_group_id, - ) + self._process.terminate(timeout=timeout) finally: - self.process = None - self._process_group_id = None + self._process = None @staticmethod def _safely_close_pipe(pipe: IO[AnyStr] | None) -> None: @@ -583,7 +570,7 @@ class StdioLanguageServer(LanguageServerInterface): except Exception: pass - def _read_bytes_from_process(self, process, stream, num_bytes) -> bytes: + def _read_bytes_from_process(self, process: ManagedSubprocess[bytes], stream: IO[bytes], num_bytes: int) -> bytes: """Read exactly num_bytes from process stdout""" data = b"" while len(data) < num_bytes: @@ -607,10 +594,10 @@ class StdioLanguageServer(LanguageServerInterface): """ exception: Exception | None = None try: - while self.process and self.process.stdout: - if self.process.poll() is not None: # process has terminated + while self._process and self._process.stdout: + if self._process.poll() is not None: # process has terminated break - line = self.process.stdout.readline() + line = self._process.stdout.readline() if not line: continue try: @@ -620,10 +607,10 @@ class StdioLanguageServer(LanguageServerInterface): if num_bytes is None: continue while line and line.strip(): - line = self.process.stdout.readline() + line = self._process.stdout.readline() if not line: continue - body = self._read_bytes_from_process(self.process, self.process.stdout, num_bytes) + body = self._read_bytes_from_process(self._process, self._process.stdout, num_bytes) self._handle_body(body) except LanguageServerTerminatedException as e: @@ -646,11 +633,11 @@ class StdioLanguageServer(LanguageServerInterface): Continuously read from the language server process stderr and log the messages """ try: - while self.process and self.process.stderr: - if self.process.poll() is not None: + while self._process and self._process.stderr: + if self._process.poll() is not None: # process has terminated break - line = self.process.stderr.readline() + line = self._process.stderr.readline() if not line: continue line_str = line.decode(ENCODING, errors="replace") @@ -664,7 +651,7 @@ class StdioLanguageServer(LanguageServerInterface): log.info("Language server stderr reader thread has terminated") def _send_payload(self, payload: StringDict) -> None: - if not self.process or not self.process.stdin: + if not self._process or not self._process.stdin: return self._trace("solidlsp", "ls", payload) msg = create_message(payload) @@ -672,8 +659,8 @@ class StdioLanguageServer(LanguageServerInterface): # Use lock to prevent concurrent writes to stdin that cause buffer corruption with self._stdin_lock: try: - self.process.stdin.writelines(msg) - self.process.stdin.flush() + self._process.stdin.writelines(msg) + self._process.stdin.flush() except (BrokenPipeError, ConnectionResetError, OSError) as e: # Log the error but don't raise to prevent cascading failures log.error(f"Failed to write to stdin: {e}") diff --git a/src/solidlsp/util/subprocess_util.py b/src/solidlsp/util/subprocess_util.py index b6912715..8763f745 100644 --- a/src/solidlsp/util/subprocess_util.py +++ b/src/solidlsp/util/subprocess_util.py @@ -6,10 +6,11 @@ import signal import subprocess import threading from collections.abc import Callable -from typing import TYPE_CHECKING, Any, cast +from typing import IO, TYPE_CHECKING, Any, Generic, TypeVar, cast import oslex import psutil +from sensai.util.string import ToStringMixin if TYPE_CHECKING: import ctypes @@ -17,6 +18,7 @@ if TYPE_CHECKING: from solidlsp.lsp_protocol_handler.server import ProcessLaunchInfo log = logging.getLogger(__name__) +TStream = TypeVar("TStream", bound=str | bytes) def subprocess_kwargs() -> dict: @@ -70,9 +72,73 @@ def convert_shell_cmd(cmd: str | list[str]) -> str: return oslex.join(cmd) if isinstance(cmd, list) else cmd -class LanguageServerSubprocessLauncher: +class ManagedSubprocess(Generic[TStream], ToStringMixin): """ - Launcher for language server subprocesses, which are started for stdio-based communication. + Represents a subprocess.Popen instance with additional lifecycle management, including the ability to terminate the process + and its children gracefully, with a fallback to forceful termination if necessary. + """ + + def __init__(self, popen: subprocess.Popen[TStream], name: str, start_new_session: bool) -> None: + """ + :param popen: the subprocess.Popen instance representing the launched process + :param start_new_session: whether the process was launched in its own session, i.e. whether it is the + leader of its own process group + """ + self._popen = popen + self._name = name + + # a process launched with start_new_session=True is its own group leader, so its PGID is its PID + self._process_group_id = popen.pid if (start_new_session and os.name == "posix") else None + + def _tostring_includes(self) -> list[str]: + return ["_name"] + + @property + def stdin(self) -> "IO[TStream] | None": + return self._popen.stdin + + @property + def stdout(self) -> "IO[TStream] | None": + return self._popen.stdout + + @property + def stderr(self) -> "IO[TStream] | None": + return self._popen.stderr + + @property + def returncode(self) -> int | None: + """ + :return: the exit code of the process as of the most recent status check (see :meth:`poll`), + or None if the process was not yet known to have terminated + """ + return self._popen.returncode + + def poll(self) -> int | None: + """ + Checks whether the process has terminated, updating :attr:`returncode` accordingly. + + :return: the exit code of the process or None if it is still running + """ + return self._popen.poll() + + def terminate(self, timeout: float) -> None: + """ + Terminates the process and its children, forcefully killing them if they do not exit in time. + + :param timeout: the time to wait for the process to terminate gracefully before killing it + :param process_name: the name of the process (used for logging purposes); should start with a capital letter + """ + terminate_process_tree_with_kill_fallback( + self._popen, + terminate_timeout=timeout, + process_name=self._name, + process_group_id=self._process_group_id, + ) + + +class ManagedSubprocessLauncher: + """ + Launcher for managed subprocesses (see :class:`ManagedSubprocess`), which are started for stdio-based communication. It is home to the concern of launching a subprocess with well-defined lifecycle properties, ensuring, in particular, that a launched subprocess cannot outlive this process (insofar as the platform allows) -- even if the subprocess is started in its own session (see @@ -85,16 +151,16 @@ class LanguageServerSubprocessLauncher: _PR_SET_PDEATHSIG = 1 """the PR_SET_PDEATHSIG option value for prctl(2)""" - _instance: "LanguageServerSubprocessLauncher | None" = None + _instance: "ManagedSubprocessLauncher | None" = None _instance_lock = threading.Lock() def __init__(self) -> None: self._libc = self._load_libc() - self._spawner: "LanguageServerSubprocessLauncher._PDeathSigSpawner | None" = None + self._spawner: "ManagedSubprocessLauncher._PDeathSigSpawner | None" = None self._spawner_lock = threading.Lock() @classmethod - def get_instance(cls) -> "LanguageServerSubprocessLauncher": + def get_instance(cls) -> "ManagedSubprocessLauncher": if cls._instance is None: with cls._instance_lock: if cls._instance is None: @@ -115,7 +181,7 @@ class LanguageServerSubprocessLauncher: return ctypes.CDLL(None) except OSError as e: log.warning( - "Could not load libc (%s); language server processes will not be protected against " + "Could not load libc (%s); subprocesses will not be protected against " "orphaning if this process is killed without a chance to shut down cleanly", e, ) @@ -130,11 +196,12 @@ class LanguageServerSubprocessLauncher: if self._libc is not None: self._libc.prctl(self._PR_SET_PDEATHSIG, signal.SIGTERM) - def launch(self, process_launch_info: "ProcessLaunchInfo", start_new_session: bool) -> subprocess.Popen[bytes]: + def launch(self, process_launch_info: "ProcessLaunchInfo", name: str, start_new_session: bool) -> ManagedSubprocess[bytes]: """ - Launches a language server process from ``process_launch_info``. + Launches a subprocess from ``process_launch_info``. :param process_launch_info: the command, environment and working directory to launch with + :param name: the name of the process (used for logging purposes); should start with a capital letter :param start_new_session: whether to start the process in its own session (own process group, detached from ours) """ @@ -158,8 +225,8 @@ class LanguageServerSubprocessLauncher: if use_pdeathsig: kwargs["preexec_fn"] = self._set_pdeathsig_on_parent_exit - def do_popen() -> subprocess.Popen[bytes]: - return cast( + def do_popen() -> ManagedSubprocess[bytes]: + popen = cast( "subprocess.Popen[bytes]", subprocess.Popen( cmd, @@ -172,6 +239,7 @@ class LanguageServerSubprocessLauncher: **kwargs, ), ) + return ManagedSubprocess(popen, name, start_new_session) # perform the actual Popen call, funneling the fork() through the dedicated spawner thread # when pdeathsig applies @@ -192,7 +260,7 @@ class LanguageServerSubprocessLauncher: """ def __init__(self) -> None: - self._queue: queue.Queue[tuple[Callable[[], subprocess.Popen], "queue.Queue"]] = queue.Queue() + self._queue: queue.Queue[tuple[Callable[[], ManagedSubprocess[bytes]], "queue.Queue"]] = queue.Queue() self._thread = threading.Thread(target=self._run, name="solidlsp-pdeathsig-spawner", daemon=True) self._thread.start() @@ -204,7 +272,7 @@ class LanguageServerSubprocessLauncher: except BaseException as e: result_queue.put((e, None)) - def spawn(self, func: Callable[[], subprocess.Popen]) -> subprocess.Popen: + def spawn(self, func: Callable[[], ManagedSubprocess[bytes]]) -> ManagedSubprocess[bytes]: result_queue: queue.Queue = queue.Queue(maxsize=1) self._queue.put((func, result_queue)) error, process = result_queue.get()