diff --git a/agent/lsp/client.py b/agent/lsp/client.py index 36c2721027..872665f689 100644 --- a/agent/lsp/client.py +++ b/agent/lsp/client.py @@ -103,6 +103,11 @@ def file_uri(path: str) -> str: return "file://" + quote(abs_path, safe="/:") +def _folder(root: str) -> Dict[str, str]: + """Build an LSP ``WorkspaceFolder`` for ``root``.""" + return {"name": os.path.basename(root.rstrip(os.sep)) or root, "uri": file_uri(root)} + + def uri_to_path(uri: str) -> str: """Inverse of :func:`file_uri`.""" if not uri.startswith("file://"): @@ -197,6 +202,10 @@ class LSPClient: ) -> None: self.server_id = server_id self.workspace_root = workspace_root + # Roots this server is serving. Single-root servers only ever + # hold ``workspace_root``; multi-root servers (pyright) grow this + # via :meth:`add_workspace_folder` instead of a second process. + self.workspace_folders: List[str] = [workspace_root] self._command = list(command) self._env = env self._cwd = cwd or workspace_root @@ -402,9 +411,7 @@ class LSPClient: "rootUri": file_uri(self.workspace_root), "rootPath": self.workspace_root, "processId": os.getpid(), - "workspaceFolders": [ - {"name": "workspace", "uri": file_uri(self.workspace_root)} - ], + "workspaceFolders": [_folder(r) for r in self.workspace_folders], "initializationOptions": self._init_options, "capabilities": { "window": {"workDoneProgress": True}, @@ -701,7 +708,21 @@ class LSPClient: return None async def _handle_workspace_folders(self, params: Any) -> Any: - return [{"name": "workspace", "uri": file_uri(self.workspace_root)}] + return [_folder(r) for r in self.workspace_folders] + + async def add_workspace_folder(self, root: str) -> None: + """Attach another root to a running multi-root server. + + Idempotent; the folder is recorded before the notification is + sent so concurrent callers for the same root only announce once. + """ + if root in self.workspace_folders: + return + self.workspace_folders.append(root) + await self._send_notification( + "workspace/didChangeWorkspaceFolders", + {"event": {"added": [_folder(root)], "removed": []}}, + ) async def _handle_diagnostic_refresh(self, params: Any) -> Any: # We don't honour refresh — we re-pull on every touchFile. diff --git a/agent/lsp/manager.py b/agent/lsp/manager.py index 7ba1b914f7..7dbb5c38bb 100644 --- a/agent/lsp/manager.py +++ b/agent/lsp/manager.py @@ -12,7 +12,10 @@ Design choices: - One client per ``(server_id, workspace_root)`` key. Lazy spawn: the first request for a key spawns the client; subsequent requests - re-use it. + re-use it. Servers flagged ``multi_root`` (pyright) get ONE client + per ``server_id``; further roots — typically sibling git worktrees — + are attached to the running process via + ``workspace/didChangeWorkspaceFolders`` instead of a new spawn. - A **broken-set** records ``(server_id, workspace_root)`` pairs that failed to spawn or initialize. These are never retried for the @@ -448,9 +451,10 @@ class LSPService: # cancelled future never reached the broken-set add inside # ``_get_or_spawn`` so the client may still be hanging in # ``_clients`` with a half-initialized state. + ckey = _client_key(srv, per_server_root) with self._state_lock: - client = self._clients.pop(key, None) - self._last_used.pop(key, None) + client = self._clients.pop(ckey, None) + self._last_used.pop(ckey, None) if client is not None: try: # Fire-and-forget shutdown — give it a second to cleanup, @@ -527,7 +531,7 @@ class LSPService: if not (ws and gated and srv): return [] with self._state_lock: - client = self._clients.get((srv.server_id, ws)) + client = self._clients.get(_client_key(srv, ws)) if client is None: return [] return list(client.diagnostics_for(file_path, fresh_only=True)) @@ -550,21 +554,26 @@ class LSPService: ) return None # exclude marker hit, server gated off - key = (srv.server_id, per_server_root) - if key in self._broken: + if (srv.server_id, per_server_root) in self._broken: return None + key = _client_key(srv, per_server_root) with self._state_lock: client = self._clients.get(key) if client is not None and client.is_running: self._last_used[key] = time.time() eventlog.log_active(srv.server_id, per_server_root) - return client + else: + client = None spawning = self._spawning.get(key) - if spawning is not None: + if client is None and spawning is not None: try: - return await spawning + client = await spawning except Exception: # noqa: BLE001 return None + if client is not None: + if srv.multi_root: + await client.add_workspace_folder(per_server_root) + return client # Begin spawn loop = asyncio.get_running_loop() @@ -586,7 +595,7 @@ class LSPService: # or install attempt failed). Surface this once via # the structured logger so the user can act on it. eventlog.log_server_unavailable(srv.server_id, srv.server_id) - self._broken.add(key) + self._broken.add((srv.server_id, per_server_root)) spawn_future.set_result(None) return None client = LSPClient( @@ -602,7 +611,7 @@ class LSPService: await client.start() except Exception as e: # noqa: BLE001 eventlog.log_spawn_failed(srv.server_id, per_server_root, e) - self._broken.add(key) + self._broken.add((srv.server_id, per_server_root)) spawn_future.set_result(None) return None with self._state_lock: @@ -626,10 +635,10 @@ class LSPService: the key. All writers and the reaper run on the background loop thread; the lock keeps this consistent with the reader anyway. """ - key = (client.server_id, client.workspace_root) with self._state_lock: - if key in self._clients: - self._last_used[key] = time.time() + for key, c in self._clients.items(): + if c is client: + self._last_used[key] = time.time() async def _idle_reaper_loop(self) -> None: interval = min(60.0, self._idle_timeout) @@ -691,12 +700,13 @@ class LSPService: with self._state_lock: clients = [ { - "server_id": k[0], - "workspace_root": k[1], + "server_id": c.server_id, + "workspace_root": c.workspace_root, + "workspace_folders": list(c.workspace_folders), "state": c.state, "running": c.is_running, } - for k, c in self._clients.items() + for c in self._clients.values() ] broken = list(self._broken) return { @@ -710,6 +720,15 @@ class LSPService: } +def _client_key(srv, root: str) -> Tuple[str, str]: + """Cache key for the client serving ``root``. + + Multi-root servers share one process per ``server_id``; everything + else is keyed per resolved project root. + """ + return (srv.server_id, "" if srv.multi_root else root) + + def _diag_key(d: Dict[str, Any]) -> str: """Content equality key used for cross-edit delta filtering. diff --git a/agent/lsp/servers.py b/agent/lsp/servers.py index fc2a0b2616..d71fcce8db 100644 --- a/agent/lsp/servers.py +++ b/agent/lsp/servers.py @@ -146,6 +146,10 @@ class ServerDef: build_spawn: Callable[[str, "ServerContext"], Optional[SpawnSpec]] seed_first_push: bool = False description: str = "" + # Server handles ``workspace/didChangeWorkspaceFolders``: one process + # serves every project root (git worktrees included) as extra + # workspaceFolders instead of one process per root. + multi_root: bool = False def matches(self, file_path: str) -> bool: """Return True iff this server handles ``file_path``.""" @@ -974,6 +978,7 @@ SERVERS: List[ServerDef] = [ extensions=(".py", ".pyi"), resolve_root=_root_python, build_spawn=_spawn_pyright, + multi_root=True, description="Python — Microsoft pyright", ), ServerDef( diff --git a/tests/agent/lsp/_mock_lsp_server.py b/tests/agent/lsp/_mock_lsp_server.py index d7ce410151..57bfedbe19 100644 --- a/tests/agent/lsp/_mock_lsp_server.py +++ b/tests/agent/lsp/_mock_lsp_server.py @@ -103,6 +103,14 @@ def main(): if msg.get("method") == "workspace/didChangeWatchedFiles": continue + if msg.get("method") == "workspace/didChangeWorkspaceFolders": + # Multi-root tests observe attached folders through this log. + log_path = os.environ.get("MOCK_LSP_FOLDERS_LOG") + if log_path: + with open(log_path, "a", encoding="utf-8") as fh: + fh.write(json.dumps(msg.get("params")) + "\n") + continue + if msg.get("method") in {"textDocument/didOpen", "textDocument/didChange"}: params = msg.get("params") or {} td = params.get("textDocument") or {} diff --git a/tests/agent/lsp/test_multi_root.py b/tests/agent/lsp/test_multi_root.py new file mode 100644 index 0000000000..4f05aaac80 --- /dev/null +++ b/tests/agent/lsp/test_multi_root.py @@ -0,0 +1,125 @@ +"""Multi-root servers share ONE process across project roots. + +A profiled session with subagents editing across ~30 git worktrees ran +30-60 pyright processes. Pyright supports multi-root workspaces, so +the service keys such clients by ``server_id`` alone and attaches each +new root via ``workspace/didChangeWorkspaceFolders``. Single-root +servers keep the one-client-per-root behaviour. +""" +from __future__ import annotations + +import json +import sys +from pathlib import Path + +import pytest + +from agent.lsp.manager import LSPService +from agent.lsp.servers import SERVERS, ServerContext, ServerDef, SpawnSpec +from agent.lsp.workspace import clear_cache + +MOCK_SERVER = str(Path(__file__).parent / "_mock_lsp_server.py") + + +@pytest.fixture(autouse=True) +def _clear_workspace_cache(): + clear_cache() + yield + clear_cache() + + +def _make_repo(tmp_path: Path, name: str) -> Path: + repo = tmp_path / name + repo.mkdir() + (repo / ".git").mkdir() + (repo / "pyproject.toml").write_text("", encoding="utf-8") + (repo / "x.py").write_text("print('hi')\n", encoding="utf-8") + return repo + + +@pytest.fixture +def two_repos(tmp_path): + return _make_repo(tmp_path, "repo-a"), _make_repo(tmp_path, "repo-b") + + +@pytest.fixture +def mock_pyright(monkeypatch, tmp_path): + """Install the mock as ``pyright``; yield (spawn_count, folders_log, set_multi_root).""" + idx = next(i for i, s in enumerate(SERVERS) if s.server_id == "pyright") + original = SERVERS[idx] + spawns = {"value": 0} + folders_log = tmp_path / "folders.jsonl" + + def _spawn(root: str, ctx: ServerContext) -> SpawnSpec: + spawns["value"] += 1 + return SpawnSpec( + command=[sys.executable, MOCK_SERVER], + workspace_root=root, + cwd=root, + env={"MOCK_LSP_SCRIPT": "errors", "MOCK_LSP_FOLDERS_LOG": str(folders_log)}, + ) + + def _install(multi_root: bool) -> None: + SERVERS[idx] = ServerDef( + server_id="pyright", + extensions=original.extensions, + resolve_root=lambda fp, ws: ws, + build_spawn=_spawn, + multi_root=multi_root, + description="mock pyright", + ) + + yield spawns, folders_log, _install + SERVERS[idx] = original + + +def _service() -> LSPService: + return LSPService( + enabled=True, wait_mode="document", wait_timeout=3.0, install_strategy="manual" + ) + + +def test_multi_root_server_shares_one_client_across_roots(two_repos, mock_pyright, monkeypatch): + repo_a, repo_b = two_repos + spawns, folders_log, install = mock_pyright + install(multi_root=True) + svc = _service() + try: + monkeypatch.chdir(str(repo_a)) + diags_a = svc.get_diagnostics_sync(str(repo_a / "x.py")) + monkeypatch.chdir(str(repo_b)) + diags_b = svc.get_diagnostics_sync(str(repo_b / "x.py")) + + # Exactly one process; the second root arrived as a folder change. + assert spawns["value"] == 1 + assert len(svc._clients) == 1 + client = next(iter(svc._clients.values())) + assert client.workspace_folders == [str(repo_a), str(repo_b)] + events = [json.loads(line) for line in folders_log.read_text(encoding="utf-8").splitlines()] + assert [f["uri"] for e in events for f in e["event"]["added"]] == [ + Path(repo_b).as_uri() + ] + # Diagnostics still resolve per file in both folders. + assert len(diags_a) == 1 and len(diags_b) == 1 + status = svc.get_status()["clients"][0] + assert status["workspace_root"] == str(repo_a) + assert status["workspace_folders"] == [str(repo_a), str(repo_b)] + finally: + svc.shutdown() + + +def test_single_root_server_still_spawns_per_root(two_repos, mock_pyright, monkeypatch): + repo_a, repo_b = two_repos + spawns, folders_log, install = mock_pyright + install(multi_root=False) + svc = _service() + try: + monkeypatch.chdir(str(repo_a)) + svc.get_diagnostics_sync(str(repo_a / "x.py")) + monkeypatch.chdir(str(repo_b)) + svc.get_diagnostics_sync(str(repo_b / "x.py")) + assert spawns["value"] == 2 + assert set(svc._clients) == {("pyright", str(repo_a)), ("pyright", str(repo_b))} + assert not folders_log.exists() + finally: + svc.shutdown() diff --git a/website/docs/user-guide/features/lsp.md b/website/docs/user-guide/features/lsp.md index 8f5830f479..c34ac0e9c0 100644 --- a/website/docs/user-guide/features/lsp.md +++ b/website/docs/user-guide/features/lsp.md @@ -237,6 +237,12 @@ respawned automatically on the next relevant file operation. Set `idle_timeout: 0` to disable reaping and hold every server's index warm for the life of the process. +Servers that support multi-root workspaces (currently pyright) run as a +**single process** per Hermes process: the first Python project spawns +it, and every further project root — for example sibling git worktrees +edited by parallel subagents — is attached to that same server as an +additional workspace folder instead of starting another copy. + ## Disabling Set `lsp.enabled: false` in `config.yaml` to disable the entire