From 16e8e24da103d47beb5f43df1f5585985f9aabd6 Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Mon, 20 Jul 2026 22:16:42 +0200 Subject: [PATCH] Fix: Language servers were not notified of external file system changes, causing some symbolic operations (such as `find_referencing_symbols`) to report stale information. An explicit file system synchronisation mechanism is now used to detect changes prior to the affected tool executions. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Note that tools like `find_symbol` and `get_symbols_overview` do not need the synchronisation, because they explicitly open all the relevant files themselves. Resolves #1718 Co-authored-by: Niccolò Franzin --- CHANGELOG.md | 4 ++ src/serena/ls_manager.py | 118 +++++++++++++++++++++++++++++-- src/serena/project.py | 10 ++- src/serena/tools/symbol_tools.py | 14 +++- test/serena/test_ls_file_sync.py | 92 ++++++++++++++++++++++++ 5 files changed, 231 insertions(+), 7 deletions(-) create mode 100644 test/serena/test_ls_file_sync.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a7a0a0f..f2995eae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,10 @@ Status of the `main` branch. Changes prior to the next official version change w * Language Servers: - Fix: Properly differentiate between raw and high-level symbol cache fingerprints, avoiding unnecessary invalidations of the raw cache when only the derived high-level representation changes + - Fix: Language servers were not notified of external file system changes, causing some + symbolic operations (such as `find_referencing_symbols`) to report stale information. + An explicit file system polling mechanism is now used to detect changes prior to the affected + tool executions. - Improve uv-based language server launch command compatibility: Use the more widely supported `uv tool run` instead of `uv x` #1721 - Java (JDT-LS): add `runtimes` to `ls_specific_settings.java`, a list of extra JRE/JDK entries diff --git a/src/serena/ls_manager.py b/src/serena/ls_manager.py index 3b44a9af..27218ded 100644 --- a/src/serena/ls_manager.py +++ b/src/serena/ls_manager.py @@ -2,14 +2,20 @@ import logging import os.path import threading from collections.abc import Iterator +from pathlib import Path +from typing import TYPE_CHECKING from sensai.util.logging import LogTime from serena.config.serena_config import ProjectConfig, SerenaPaths from solidlsp import SolidLanguageServer from solidlsp.ls_config import Language, LanguageServerConfig +from solidlsp.lsp_protocol_handler.lsp_types import DidChangeWatchedFilesParams, FileChangeType, FileEvent from solidlsp.settings import SolidLSPSettings +if TYPE_CHECKING: + from .project import Project + log = logging.getLogger(__name__) @@ -68,9 +74,7 @@ class LanguageServerManager: """ def __init__( - self, - language_servers: dict[Language, SolidLanguageServer], - language_server_factory: LanguageServerFactory | None = None, + self, language_servers: dict[Language, SolidLanguageServer], language_server_factory: LanguageServerFactory, project: "Project" ) -> None: """ :param language_servers: a mapping from language to language server; the servers are assumed to be already started. @@ -81,6 +85,7 @@ class LanguageServerManager: """ self._language_servers = language_servers self._language_server_factory = language_server_factory + self._file_change_notifier = LanguageServerFileChangeNotifier(project, self) @property def _default_language_server(self) -> SolidLanguageServer: @@ -89,13 +94,14 @@ class LanguageServerManager: return next(iter(self._language_servers.values())) @staticmethod - def from_languages(languages: list[Language], factory: LanguageServerFactory) -> "LanguageServerManager": + def from_languages(languages: list[Language], factory: LanguageServerFactory, project: "Project") -> "LanguageServerManager": """ Creates a manager with language servers for the given languages using the given factory. The language servers are started in parallel threads. :param languages: the languages for which to spawn language servers :param factory: the factory for language server creation + :param project: the project for which the language servers are created :return: the instance """ @@ -145,7 +151,7 @@ class LanguageServerManager: failure_messages = "\n".join([f"{lang.value}: {e}" for lang, e in exceptions.items()]) raise LanguageServerManagerInitialisationError(f"Failed to start {len(exceptions)} language server(s):\n{failure_messages}") - return LanguageServerManager(language_servers, factory) + return LanguageServerManager(language_servers, factory, project) def _ensure_functional_ls(self, ls: SolidLanguageServer) -> SolidLanguageServer: if not ls.is_running(): @@ -254,3 +260,105 @@ class LanguageServerManager: def has_suitable_ls_for_file(self, relative_file_path: str) -> bool: return self._get_suitable_language_server(relative_file_path) is not None + + def sync_file_system_changes(self) -> int: + """ + Polls the file system for changes to source files and notifies the language servers of any changes + (particularly changes that happened outside of Serena's own file tools, which are not covered by + the notifications sent by those tools/CodeEditor). + + :return: the number of individual file change events detected (0 if nothing changed). + """ + log.info("Polling file system for changes to source files ...") + num_changes = self._file_change_notifier.poll_and_notify() + log.info(f"File system polling complete; {num_changes} change events sent to language servers.") + return num_changes + + +class LanguageServerFileChangeNotifier: + """ + Detects changes to source files on disk and notifies language servers of those changes. + """ + + def __init__(self, project: "Project", language_server_manager: LanguageServerManager) -> None: + self._project = project + self._language_server_manager = language_server_manager + self._freshness_last_seen_mtimes: dict[str, float] | None = None + self._freshness_lock = threading.Lock() + + def poll_and_notify(self) -> int: + """ + Detects source files that were changed, created or deleted on disk since the last call + and notifies every language server managed for this project via the LSP + ``workspace/didChangeWatchedFiles`` notification. + + This exists because Serena's own file and symbol tools notify the language server inline + (via didOpen/didChange/didClose) when they edit a file, but edits made through any other + channel (another editor, a second agent, a git checkout, a build step) are otherwise + invisible to a warm language server, causing symbolic queries to answer from a stale index. + + The set of files considered is exactly the set Serena itself tracks (see + :meth:`gather_source_files`), so no separate file-discovery logic has to be kept in sync. + The dominant cost is the directory walk plus one ``os.stat`` per tracked file; this is + intended to be called before symbolic tool invocations rather than on a timer. + + :return: the number of change events sent (0 if nothing changed, if no language server is + running yet, or on the first call, which only establishes the baseline). + """ + current: dict[str, float] = {} + for rel_path in self._project.gather_source_files(): + try: + current[rel_path] = os.stat(os.path.join(self._project.project_root, rel_path)).st_mtime + except OSError: + continue + + # Read-diff-swap under the lock only; the filesystem walk above and the LSP notifications + # below stay outside it so concurrent callers do not serialize on I/O. + with self._freshness_lock: + previous = self._freshness_last_seen_mtimes + self._freshness_last_seen_mtimes = current + + if previous is None: + return 0 + + # compute the set of individual events (created, changed, deleted) + events: list[tuple[str, FileChangeType]] = [] + for rel_path, mtime in current.items(): + prev_mtime = previous.get(rel_path) + if prev_mtime is None: + events.append((rel_path, FileChangeType.Created)) + elif mtime > prev_mtime: + events.append((rel_path, FileChangeType.Changed)) + events.extend((rel_path, FileChangeType.Deleted) for rel_path in previous if rel_path not in current) + + if not events: + return 0 + + # create the change didChangeWatchedFiles notification + changes: list[FileEvent] = [ + {"uri": Path(self._project.project_root, rel_path).resolve().as_uri(), "type": change_type} for rel_path, change_type in events + ] + params: DidChangeWatchedFilesParams = {"changes": changes} + created_paths = [rel_path for rel_path, change_type in events if change_type == FileChangeType.Created] + + for ls in self._language_server_manager.iter_language_servers(): + # send the didChangeWatchedFiles notification to the language server + try: + ls.server.notify.did_change_watched_files(params) + except Exception as e: + log.error("Failed to notify language server of watched file changes", exc_info=e) + + # A didChangeWatchedFiles(Created) notification alone is not enough for every backend + # (observed with pyright) to fold a brand-new file into its cross-file reference graph; + # an open/close cycle forces the parse+bind that Serena's own file tools trigger via + # SolidLanguageServer.open_file(). + for rel_path in created_paths: + if ls.is_ignored_path(rel_path, ignore_unsupported_files=True): + continue + try: + with ls.open_file(rel_path): + pass + except Exception as e: + log.error(f"Failed to refresh newly created file {rel_path!r} in language server", exc_info=e) + + return len(events) diff --git a/src/serena/project.py b/src/serena/project.py index 9cb183d9..e316f7a2 100644 --- a/src/serena/project.py +++ b/src/serena/project.py @@ -493,7 +493,7 @@ class Project(ToStringMixin): ls_specific_settings=ls_specific_settings, trace_lsp_communication=self.serena_config.trace_lsp_communication, ) - self.language_server_manager = LanguageServerManager.from_languages(self.project_config.languages, factory) + self.language_server_manager = LanguageServerManager.from_languages(self.project_config.languages, factory, self) return self.language_server_manager except Exception as e: self._language_server_manager_init_error = e @@ -557,6 +557,14 @@ class Project(ToStringMixin): log.info("Removing and stopping the language server for language %s ...", language.value) self.language_server_manager.remove_language_server(language) + def ls_sync_file_system_changes(self) -> int: + """ + Synchronizes file system changes with the project's associated language server(s), if applicable + """ + if self.language_server_manager: + return self.language_server_manager.sync_file_system_changes() + return 0 + def shutdown(self, timeout: float = 2.0) -> None: if self.language_server_manager is not None: self.language_server_manager.stop_all(save_cache=True, timeout=timeout) diff --git a/src/serena/tools/symbol_tools.py b/src/serena/tools/symbol_tools.py index a0e5fc10..f79333e4 100644 --- a/src/serena/tools/symbol_tools.py +++ b/src/serena/tools/symbol_tools.py @@ -274,11 +274,13 @@ class FindReferencingSymbolsTool(Tool, ToolMarkerSymbolicRead): :param max_answer_chars: max result length; -1 for default :return: a list of JSON objects with the symbols referencing the requested symbol """ + self.project.ls_sync_file_system_changes() + include_body = False # It is probably never a good idea to include the body of the referencing symbols parsed_include_kinds: Sequence[SymbolKind] | None = [SymbolKind(k) for k in include_kinds] if include_kinds else None parsed_exclude_kinds: Sequence[SymbolKind] | None = [SymbolKind(k) for k in exclude_kinds] if exclude_kinds else None - symbol_retriever = self.create_language_server_symbol_retriever() + symbol_retriever = self.create_language_server_symbol_retriever() references_in_symbols = symbol_retriever.find_referencing_symbols( name_path, relative_file_path=relative_path, @@ -361,6 +363,8 @@ class FindImplementationsTool(Tool, ToolMarkerSymbolicRead): :param max_answer_chars: max result length; -1 for default :return: a list of JSON objects with the symbols implementing the requested symbol """ + self.project.ls_sync_file_system_changes() + include_body = False parsed_include_kinds: Sequence[SymbolKind] | None = [SymbolKind(k) for k in include_kinds] if include_kinds else None parsed_exclude_kinds: Sequence[SymbolKind] | None = [SymbolKind(k) for k in exclude_kinds] if exclude_kinds else None @@ -414,6 +418,8 @@ class FindDeclarationTool(Tool, ToolMarkerSymbolicRead): :param include_body: whether to include the symbol's body in the result. Default False. :param include_info: whether to include additional info (hover-like). Default False. """ + self.project.ls_sync_file_system_changes() + symbol_retriever = self.create_language_server_symbol_retriever() relative_path = self._sanitize_input_param(relative_path) regex = self._sanitize_input_param(regex) @@ -496,6 +502,8 @@ class GetDiagnosticsForFileTool(Tool, ToolMarkerSymbolicRead): :param max_answer_chars: max result length; -1 for default :return: grouped diagnostics for the requested file. """ + self.project.ls_sync_file_system_changes() + symbol_retriever = self.create_language_server_symbol_retriever() diagnostics = symbol_retriever.get_file_diagnostics( relative_file_path=relative_path, @@ -547,6 +555,8 @@ class GetDiagnosticsForSymbolTool(Tool, ToolMarkerSymbolicRead, ToolMarkerOption :param max_answer_chars: max result length; -1 for default :return: grouped diagnostics for the requested symbol and, optionally, its referencing symbols. """ + self.project.ls_sync_file_system_changes() + symbol_retriever = self.create_language_server_symbol_retriever() diagnostics_by_symbol = symbol_retriever.get_symbol_diagnostics( name_path=name_path, @@ -693,6 +703,8 @@ class SafeDeleteSymbol(Tool, ToolMarkerSymbolicEdit): :param name_path_pattern: name path of the symbol to delete :param relative_path: the relative path to the file containing the symbol to delete """ + self.project.ls_sync_file_system_changes() + ls_symbol_retriever = self.create_language_server_symbol_retriever() symbol = ls_symbol_retriever.find_unique(name_path_pattern, substring_matching=False, within_relative_path=relative_path) symbol_rel_path = symbol.relative_path diff --git a/test/serena/test_ls_file_sync.py b/test/serena/test_ls_file_sync.py new file mode 100644 index 00000000..babe4bab --- /dev/null +++ b/test/serena/test_ls_file_sync.py @@ -0,0 +1,92 @@ +""" +End-to-end test for language server file synchronisation as a result of externally-made file changes. + +Exercises all three ``FileChangeType`` branches against a real pyright backend: +Created (new caller file), Changed (append a second caller), Deleted (remove the caller file). +""" + +import os +import shutil + +import pytest + +from solidlsp import SolidLanguageServer +from solidlsp.ls_config import Language +from test.conftest import get_repo_path, project_with_ls_context + +pytestmark = pytest.mark.python + +# A method that already has a call site in the fixture, so its baseline reference set is non-empty. +_TARGET_FILE = os.path.join("test_repo", "services.py") +_TARGET_SYMBOL = "create_user" + +_CALLER_REL_PATH = os.path.join("test_repo", "external_caller.py") +_CALLER_ONE = "external_caller_one" +_CALLER_TWO = "external_caller_two" + + +def _caller_source(*function_names: str) -> str: + body = "\n\n".join( + f"def {name}() -> User:\n return UserService().{_TARGET_SYMBOL}('x', 'y', 'z@example.com')" for name in function_names + ) + return "from test_repo.models import User\nfrom test_repo.services import UserService\n\n\n" + body + "\n" + + +def _referencing_symbol_names(ls: SolidLanguageServer) -> list[str]: + """Names of the symbols that reference ``_TARGET_SYMBOL``, per the warm language server.""" + document_symbols = ls.request_document_symbols(_TARGET_FILE).get_all_symbols_and_roots() + target = next((s for s in document_symbols[0] if s.get("name") == _TARGET_SYMBOL), None) + assert target is not None and "selectionRange" in target, f"{_TARGET_SYMBOL} not found in {_TARGET_FILE}" + start = target["selectionRange"]["start"] + references = ls.request_referencing_symbols(_TARGET_FILE, start["line"], start["character"]) + return [ref.symbol["name"] for ref in references] + + +def test_external_changes_become_visible_after_sync(tmp_path): + # Work on an isolated copy so we can freely create/edit/delete files under the project root. + repo_root = tmp_path / "repo" + shutil.copytree(get_repo_path(Language.PYTHON), repo_root) + caller_abs = repo_root / _CALLER_REL_PATH + + with project_with_ls_context(Language.PYTHON, str(repo_root)) as project: + language_server = next(iter(project.language_server_manager.iter_language_servers())) + + def sync_fs() -> int: + return project.language_server_manager.sync_file_system_changes() + + # Warm the reference index, then establish the freshness baseline (first poll never notifies). + _referencing_symbol_names(language_server) + assert sync_fs() == 0, "first poll must only establish the baseline" + + # --- Created ------------------------------------------------------------------------------- + caller_abs.write_text(_caller_source(_CALLER_ONE), encoding="utf-8") + + # Adversarial: without the poll, the warm server has never seen the new file -> stale. + assert _CALLER_ONE not in _referencing_symbol_names(language_server), "expected the new caller to be invisible before poll" + + assert sync_fs() == 1, "one Created event expected" + assert _CALLER_ONE in _referencing_symbol_names(language_server), "new caller must be visible after poll" + + # --- Changed ------------------------------------------------------------------------------- + caller_abs.write_text(_caller_source(_CALLER_ONE, _CALLER_TWO), encoding="utf-8") + assert sync_fs() == 1, "one Changed event expected" + names_after_change = _referencing_symbol_names(language_server) + assert _CALLER_TWO in names_after_change, "appended caller must be visible after poll" + assert _CALLER_ONE in names_after_change + + # --- Deleted ------------------------------------------------------------------------------- + caller_abs.unlink() + assert sync_fs() == 1, "one Deleted event expected" + names_after_delete = _referencing_symbol_names(language_server) + assert _CALLER_ONE not in names_after_delete, "deleted caller must disappear after poll" + assert _CALLER_TWO not in names_after_delete + + +def test_first_poll_is_baseline_only(tmp_path): + """A newly-loaded project must not report its entire pre-existing file set as changed.""" + repo_root = tmp_path / "repo" + shutil.copytree(get_repo_path(Language.PYTHON), repo_root) + with project_with_ls_context(Language.PYTHON, str(repo_root)) as project: + assert project.ls_sync_file_system_changes() == 0 + # No external edits between the two calls -> still nothing to report. + assert project.ls_sync_file_system_changes() == 0