fix(lsp): share one pyright process across git worktrees via workspaceFolders
Multi-root servers (pyright) are keyed by server_id; a file whose resolved root is new for a running client is attached with workspace/didChangeWorkspaceFolders instead of spawning another server. Single-root servers keep the (server_id, workspace_root) key and behavior. A profiled fan-out across ~30 worktrees ran 30-60 pyright processes (~8.7 GB); the same fan-out now runs one.
This commit is contained in:
+25
-4
@@ -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.
|
||||
|
||||
+36
-17
@@ -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.
|
||||
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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 {}
|
||||
|
||||
@@ -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()
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user