refactor(agent/lsp): dedupe status lookups in cli, compact docstrings and reader dispatch
This commit is contained in:
+39
-54
@@ -9,18 +9,23 @@ import sys
|
||||
|
||||
_STATUS_MARKERS = {"installed": "✓", "missing": "·", "manual-only": "?"}
|
||||
|
||||
# (name, parser kwargs, optional (flag, add_argument kwargs)) — order defines the help listing.
|
||||
# (name, help, optional (flag, add_argument kwargs), handler(args)) — order defines the help listing.
|
||||
_SUBCOMMANDS = [
|
||||
("status", {"help": "Show LSP service status"},
|
||||
("--json", {"action": "store_true", "help": "Emit machine-readable JSON"})),
|
||||
("list", {"help": "List supported language servers"},
|
||||
("--installed-only", {"action": "store_true", "help": "Only show servers whose binary is currently available"})),
|
||||
("install", {"help": "Install a server binary"}, ("server", {"help": "Server id (e.g. pyright, gopls)"})),
|
||||
("install-all", {"help": "Install every server with a known auto-install recipe"},
|
||||
("--include-manual", {"action": "store_true", "help": "Even attempt servers marked manual-install (best effort)"})),
|
||||
("restart", {"help": "Tear down running LSP clients (next edit re-spawns)"}, None),
|
||||
("which", {"help": "Print binary path for a server"}, ("server", {"help": "Server id"})),
|
||||
("status", "Show LSP service status",
|
||||
("--json", {"action": "store_true", "help": "Emit machine-readable JSON"}),
|
||||
lambda a: _cmd_status(getattr(a, "json", False))),
|
||||
("list", "List supported language servers",
|
||||
("--installed-only", {"action": "store_true", "help": "Only show servers whose binary is currently available"}),
|
||||
lambda a: _cmd_list(getattr(a, "installed_only", False))),
|
||||
("install", "Install a server binary", ("server", {"help": "Server id (e.g. pyright, gopls)"}),
|
||||
lambda a: _cmd_install(a.server)),
|
||||
("install-all", "Install every server with a known auto-install recipe",
|
||||
("--include-manual", {"action": "store_true", "help": "Even attempt servers marked manual-install (best effort)"}),
|
||||
lambda a: _cmd_install_all(getattr(a, "include_manual", False))),
|
||||
("restart", "Tear down running LSP clients (next edit re-spawns)", None, lambda a: _cmd_restart()),
|
||||
("which", "Print binary path for a server", ("server", {"help": "Server id"}), lambda a: _cmd_which(a.server)),
|
||||
]
|
||||
_COMMANDS = {name: handler for name, _, _, handler in _SUBCOMMANDS}
|
||||
|
||||
|
||||
def register_subparser(subparsers: argparse._SubParsersAction) -> None:
|
||||
@@ -31,23 +36,13 @@ def register_subparser(subparsers: argparse._SubParsersAction) -> None:
|
||||
description="Manage the LSP layer that powers post-write semantic diagnostics in write_file/patch.",
|
||||
)
|
||||
sub = parser.add_subparsers(dest="lsp_command")
|
||||
for name, kwargs, arg in _SUBCOMMANDS:
|
||||
p = sub.add_parser(name, **kwargs)
|
||||
for name, help_text, arg, _handler in _SUBCOMMANDS:
|
||||
p = sub.add_parser(name, help=help_text)
|
||||
if arg is not None:
|
||||
p.add_argument(arg[0], **arg[1])
|
||||
parser.set_defaults(func=run_lsp_command)
|
||||
|
||||
|
||||
_COMMANDS = {
|
||||
"status": lambda a: _cmd_status(getattr(a, "json", False)),
|
||||
"list": lambda a: _cmd_list(getattr(a, "installed_only", False)),
|
||||
"install": lambda a: _cmd_install(a.server),
|
||||
"install-all": lambda a: _cmd_install_all(getattr(a, "include_manual", False)),
|
||||
"restart": lambda a: _cmd_restart(),
|
||||
"which": lambda a: _cmd_which(a.server),
|
||||
}
|
||||
|
||||
|
||||
def run_lsp_command(args: argparse.Namespace) -> int:
|
||||
"""Top-level dispatcher for ``hermes lsp <subcommand>``."""
|
||||
sub = getattr(args, "lsp_command", None) or "status"
|
||||
@@ -61,21 +56,20 @@ def run_lsp_command(args: argparse.Namespace) -> int:
|
||||
return 130
|
||||
|
||||
|
||||
def _status_for(server_id: str) -> str:
|
||||
from agent.lsp.install import detect_status
|
||||
return detect_status(_recipe_pkg_for(server_id))
|
||||
|
||||
|
||||
def _cmd_status(emit_json: bool) -> int:
|
||||
from agent.lsp import get_service
|
||||
from agent.lsp.servers import SERVERS
|
||||
from agent.lsp.install import detect_status
|
||||
|
||||
svc = get_service()
|
||||
info = svc.get_status() if svc is not None else {"enabled": False}
|
||||
|
||||
if emit_json:
|
||||
import json
|
||||
registry = [
|
||||
{"server_id": s.server_id, "extensions": list(s.extensions), "description": s.description,
|
||||
"binary_status": detect_status(_recipe_pkg_for(s.server_id))}
|
||||
for s in SERVERS
|
||||
]
|
||||
registry = [{"server_id": s.server_id, "extensions": list(s.extensions), "description": s.description,
|
||||
"binary_status": _status_for(s.server_id)} for s in SERVERS]
|
||||
sys.stdout.write(json.dumps({"service": info, "registry": registry}, indent=2) + "\n")
|
||||
return 0
|
||||
|
||||
@@ -91,17 +85,14 @@ def _cmd_status(emit_json: bool) -> int:
|
||||
if broken:
|
||||
out.append(f" broken pairs: {len(broken)}")
|
||||
out += [f" - {b}" for b in broken]
|
||||
disabled = info.get("disabled_servers") or []
|
||||
if disabled:
|
||||
if disabled := info.get("disabled_servers"):
|
||||
out.append(f" disabled in cfg: {', '.join(disabled)}")
|
||||
|
||||
# Sidecar gaps the registry table can't show (bash-language-server -> shellcheck).
|
||||
backend_warnings = _backend_warnings()
|
||||
if backend_warnings:
|
||||
if backend_warnings := _backend_warnings():
|
||||
out += ["", "Backend warnings", "================"] + [f" ! {line}" for line in backend_warnings]
|
||||
out += ["", "Registered Servers", "=================="]
|
||||
for s in SERVERS:
|
||||
status = detect_status(_recipe_pkg_for(s.server_id))
|
||||
status = _status_for(s.server_id)
|
||||
ext_summary = ", ".join(list(s.extensions)[:5])
|
||||
if len(s.extensions) > 5:
|
||||
ext_summary += f", … (+{len(s.extensions) - 5})"
|
||||
@@ -114,45 +105,41 @@ def _cmd_status(emit_json: bool) -> int:
|
||||
|
||||
def _cmd_list(installed_only: bool) -> int:
|
||||
from agent.lsp.servers import SERVERS
|
||||
from agent.lsp.install import detect_status
|
||||
|
||||
for s in SERVERS:
|
||||
status = detect_status(_recipe_pkg_for(s.server_id))
|
||||
status = _status_for(s.server_id)
|
||||
if not (installed_only and status != "installed"):
|
||||
sys.stdout.write(f"{s.server_id:24s} [{status:11s}] {','.join(s.extensions)}\n")
|
||||
return 0
|
||||
|
||||
|
||||
def _cmd_install(server_id: str) -> int:
|
||||
from agent.lsp.install import try_install, INSTALL_RECIPES, detect_status
|
||||
from agent.lsp.install import try_install, INSTALL_RECIPES
|
||||
pkg = _recipe_pkg_for(server_id)
|
||||
if detect_status(pkg) == "installed":
|
||||
if _status_for(server_id) == "installed":
|
||||
sys.stdout.write(f"{server_id} already installed\n")
|
||||
return 0
|
||||
sys.stdout.write(f"installing {server_id} (pkg={pkg}) ...\n")
|
||||
sys.stdout.flush()
|
||||
bin_path = try_install(pkg, "auto")
|
||||
if bin_path is None:
|
||||
if (INSTALL_RECIPES.get(pkg) or {}).get("strategy") == "manual":
|
||||
sys.stderr.write(f"{server_id}: this server requires a manual install. See documentation.\n")
|
||||
else:
|
||||
sys.stderr.write(f"{server_id}: install failed (see logs).\n")
|
||||
return 1
|
||||
sys.stdout.write(f"installed: {bin_path}\n")
|
||||
return 0
|
||||
if bin_path is not None:
|
||||
sys.stdout.write(f"installed: {bin_path}\n")
|
||||
return 0
|
||||
manual = (INSTALL_RECIPES.get(pkg) or {}).get("strategy") == "manual"
|
||||
sys.stderr.write(f"{server_id}: this server requires a manual install. See documentation.\n" if manual
|
||||
else f"{server_id}: install failed (see logs).\n")
|
||||
return 1
|
||||
|
||||
|
||||
def _cmd_install_all(include_manual: bool) -> int:
|
||||
from agent.lsp.servers import SERVERS
|
||||
from agent.lsp.install import try_install, INSTALL_RECIPES, detect_status
|
||||
|
||||
from agent.lsp.install import try_install, INSTALL_RECIPES
|
||||
rc = 0
|
||||
for s in SERVERS:
|
||||
pkg = _recipe_pkg_for(s.server_id)
|
||||
recipe = INSTALL_RECIPES.get(pkg)
|
||||
if recipe is None or (recipe.get("strategy") == "manual" and not include_manual):
|
||||
continue
|
||||
if detect_status(pkg) == "installed":
|
||||
if _status_for(s.server_id) == "installed":
|
||||
sys.stdout.write(f" {s.server_id:24s} already installed\n")
|
||||
continue
|
||||
sys.stdout.write(f" installing {s.server_id} (pkg={pkg}) ... ")
|
||||
@@ -165,7 +152,6 @@ def _cmd_install_all(include_manual: bool) -> int:
|
||||
|
||||
def _cmd_restart() -> int:
|
||||
from agent.lsp import shutdown_service
|
||||
|
||||
shutdown_service()
|
||||
sys.stdout.write("LSP service shut down. Next edit will respawn clients.\n")
|
||||
return 0
|
||||
@@ -173,7 +159,6 @@ def _cmd_restart() -> int:
|
||||
|
||||
def _cmd_which(server_id: str) -> int:
|
||||
from agent.lsp.install import INSTALL_RECIPES, _existing_binary
|
||||
|
||||
resolved = _existing_binary((INSTALL_RECIPES.get(server_id) or {}).get("bin", server_id))
|
||||
if resolved:
|
||||
sys.stdout.write(resolved + "\n")
|
||||
|
||||
+47
-75
@@ -21,17 +21,8 @@ from urllib.parse import quote, unquote
|
||||
from hermes_cli._subprocess_compat import windows_hide_flags
|
||||
|
||||
from agent.lsp.protocol import (
|
||||
ERROR_CONTENT_MODIFIED,
|
||||
ERROR_METHOD_NOT_FOUND,
|
||||
LSPProtocolError,
|
||||
LSPRequestError,
|
||||
classify_message,
|
||||
encode_message,
|
||||
make_error_response,
|
||||
make_notification,
|
||||
make_request,
|
||||
make_response,
|
||||
read_message,
|
||||
ERROR_CONTENT_MODIFIED, ERROR_METHOD_NOT_FOUND, LSPProtocolError, LSPRequestError, classify_message,
|
||||
encode_message, make_error_response, make_notification, make_request, make_response, read_message,
|
||||
)
|
||||
|
||||
logger = logging.getLogger("agent.lsp.client")
|
||||
@@ -102,14 +93,10 @@ def _end_position(text: str) -> Dict[str, int]:
|
||||
|
||||
@dataclass
|
||||
class _DocState:
|
||||
"""Per-document state.
|
||||
|
||||
``version`` is the LSP document version last sent (didOpen=0, +1 per didChange)
|
||||
and doubles as the freshness token: ``push_version`` / ``pull_version`` tag stored
|
||||
results, fresh iff tag >= version; -1 means "no data yet". Servers that echo a
|
||||
version in publishDiagnostics get exact tagging; others are credited with the
|
||||
current version at receipt.
|
||||
"""
|
||||
"""Per-document state. ``version`` is the LSP document version last sent (didOpen=0, +1 per
|
||||
didChange) and doubles as the freshness token: ``push_version`` / ``pull_version`` tag stored
|
||||
results, fresh iff tag >= version; -1 means "no data yet". Servers that echo a version in
|
||||
publishDiagnostics get exact tagging; others are credited with the current version at receipt."""
|
||||
version: int = 0
|
||||
text: str = ""
|
||||
push: List[Dict[str, Any]] = field(default_factory=list)
|
||||
@@ -151,9 +138,8 @@ class LSPClient:
|
||||
self._next_id: int = 0
|
||||
self._pending: Dict[int, asyncio.Future] = {}
|
||||
|
||||
# Server → client requests; anything else gets method-not-found. Capability
|
||||
# (un)registration and diagnostic refresh are acknowledged but not acted on:
|
||||
# we re-pull on every touch anyway.
|
||||
# Server → client requests; anything else gets method-not-found. Capability (un)registration
|
||||
# and diagnostic refresh are acknowledged but not acted on: we re-pull on every touch anyway.
|
||||
self._request_handlers: Dict[str, Callable[[Any], Awaitable[Any]]] = {
|
||||
"window/workDoneProgress/create": self._handle_null,
|
||||
"workspace/configuration": self._handle_workspace_configuration,
|
||||
@@ -171,8 +157,8 @@ class LSPClient:
|
||||
self._state: str = "stopped"
|
||||
self._sync_kind: int = 1 # 1=Full, 2=Incremental
|
||||
self._stopping: bool = False
|
||||
# Waiters snapshot ``_push_counter`` and treat any increase as "recheck
|
||||
# the predicate" — avoids the asyncio.Event sticky-state trap.
|
||||
# Waiters snapshot ``_push_counter`` and treat any increase as "recheck the
|
||||
# predicate" — avoids the asyncio.Event sticky-state trap.
|
||||
self._push_event = asyncio.Event()
|
||||
self._push_counter = 0
|
||||
|
||||
@@ -216,11 +202,10 @@ class LSPClient:
|
||||
if sys.platform == "win32" and cmd[0].lower().endswith((".cmd", ".bat")):
|
||||
cmd = ["cmd.exe", "/c", *cmd] # CreateProcess can't run .cmd/.bat shims directly
|
||||
try:
|
||||
# start_new_session=True gives the server its own process group;
|
||||
# otherwise it inherits the gateway's pgid and mcp_tool's orphan
|
||||
# sweeper can killpg() the TUI parent along with it.
|
||||
# windows_hide_flags() suppresses the console window a .cmd shim
|
||||
# would flash from a console-less host (CREATE_NO_WINDOW; 0 on POSIX).
|
||||
# start_new_session=True gives the server its own process group; otherwise it inherits
|
||||
# the gateway's pgid and mcp_tool's orphan sweeper can killpg() the TUI parent with it.
|
||||
# windows_hide_flags() suppresses the console window a .cmd shim would flash from a
|
||||
# console-less host (CREATE_NO_WINDOW; 0 on POSIX).
|
||||
self._proc = await asyncio.create_subprocess_exec(
|
||||
cmd[0], *cmd[1:],
|
||||
stdin=asyncio.subprocess.PIPE, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE,
|
||||
@@ -243,20 +228,23 @@ class LSPClient:
|
||||
except (asyncio.CancelledError, OSError):
|
||||
pass
|
||||
|
||||
def _dispatch(self, msg: dict) -> None:
|
||||
kind, key = classify_message(msg)
|
||||
if kind == "response":
|
||||
self._dispatch_response(key, msg)
|
||||
elif kind == "request":
|
||||
asyncio.create_task(self._dispatch_request(key, msg))
|
||||
elif kind == "notification":
|
||||
self._dispatch_notification(key, msg)
|
||||
else:
|
||||
logger.warning("[%s] dropping invalid message: %r", self.server_id, msg)
|
||||
|
||||
async def _reader_loop(self) -> None:
|
||||
if self._proc is None or self._proc.stdout is None:
|
||||
return
|
||||
try:
|
||||
while (msg := await read_message(self._proc.stdout)) is not None:
|
||||
kind, key = classify_message(msg)
|
||||
if kind == "response":
|
||||
self._dispatch_response(key, msg)
|
||||
elif kind == "request":
|
||||
asyncio.create_task(self._dispatch_request(key, msg))
|
||||
elif kind == "notification":
|
||||
self._dispatch_notification(key, msg)
|
||||
else:
|
||||
logger.warning("[%s] dropping invalid message: %r", self.server_id, msg)
|
||||
self._dispatch(msg)
|
||||
logger.debug("[%s] server closed stdout cleanly", self.server_id)
|
||||
except LSPProtocolError as e:
|
||||
logger.warning("[%s] protocol error in reader loop: %s", self.server_id, e)
|
||||
@@ -288,9 +276,7 @@ class LSPClient:
|
||||
sync = sync.get("change")
|
||||
self._sync_kind = sync if isinstance(sync, int) else 1 # default to Full
|
||||
await self._send_notification("initialized", {})
|
||||
if self._init_options:
|
||||
# Some servers (vtsls, eslint) only pick config up via
|
||||
# didChangeConfiguration even when it was in initializationOptions.
|
||||
if self._init_options: # vtsls/eslint only pick config up via didChangeConfiguration
|
||||
await self._send_notification("workspace/didChangeConfiguration", {"settings": self._init_options})
|
||||
|
||||
async def shutdown(self) -> None:
|
||||
@@ -312,20 +298,15 @@ class LSPClient:
|
||||
self._state = "stopped"
|
||||
await self._cleanup_process()
|
||||
|
||||
@staticmethod
|
||||
async def _cancel_task(task: Optional[asyncio.Task]) -> None:
|
||||
if task is not None and not task.done():
|
||||
task.cancel()
|
||||
await asyncio.gather(task, return_exceptions=True)
|
||||
|
||||
async def _cleanup_process(self) -> None:
|
||||
async with self._cleanup_lock:
|
||||
reader_task, self._reader_task = self._reader_task, None
|
||||
stderr_task, self._stderr_task = self._stderr_task, None
|
||||
tasks = [self._reader_task, self._stderr_task]
|
||||
self._reader_task = self._stderr_task = None
|
||||
proc, self._proc = self._proc, None
|
||||
if reader_task is not asyncio.current_task():
|
||||
await self._cancel_task(reader_task)
|
||||
await self._cancel_task(stderr_task)
|
||||
live = [t for t in tasks if t is not None and not t.done() and t is not asyncio.current_task()]
|
||||
for t in live:
|
||||
t.cancel()
|
||||
await asyncio.gather(*live, return_exceptions=True)
|
||||
if proc is None or proc.returncode is not None:
|
||||
return
|
||||
try:
|
||||
@@ -490,8 +471,7 @@ class LSPClient:
|
||||
"workspace/didChangeWatchedFiles", {"changes": [{"uri": uri, "type": 1 if doc is None else 2}]}
|
||||
)
|
||||
if doc is None:
|
||||
# Fresh state: anything a pre-open push stashed under this path
|
||||
# (relatedDocuments spillover) is discarded.
|
||||
# Fresh state: anything a pre-open push stashed under this path (relatedDocuments spillover) is discarded.
|
||||
self._docs[abs_path] = _DocState(version=0, text=text)
|
||||
await self._send_notification(
|
||||
"textDocument/didOpen",
|
||||
@@ -520,11 +500,9 @@ class LSPClient:
|
||||
# ---- diagnostics: pull + wait ----
|
||||
|
||||
async def _pull_document_diagnostics(self, path: str) -> None:
|
||||
"""Send ``textDocument/diagnostic`` for one file into the pull store.
|
||||
|
||||
Results are tagged with the version captured at send time, so a didChange racing
|
||||
past the request makes them stale automatically. Silently no-ops on errors.
|
||||
"""
|
||||
"""Send ``textDocument/diagnostic`` for one file into the pull store. Results are tagged with the
|
||||
version captured at send time, so a didChange racing past the request makes them stale
|
||||
automatically. Silently no-ops on errors (server may not support pull)."""
|
||||
abs_path = os.path.abspath(path)
|
||||
doc = self._docs.get(abs_path)
|
||||
sent_version = doc.version if doc else -1
|
||||
@@ -554,10 +532,9 @@ class LSPClient:
|
||||
timeout: Optional[float] = None) -> bool:
|
||||
"""Wait for fresh diagnostics for ``path`` at ``version``; True iff fresh data arrived in budget.
|
||||
|
||||
``mode`` is ``"document"`` (5s) or ``"full"`` (10s); ``timeout`` overrides the
|
||||
budget (how ``lsp.wait_timeout`` reaches the loop). Callers must treat False
|
||||
as "no data", NOT "no errors" — the stores may still hold stale entries.
|
||||
Never throws for servers lacking pull support; the push side still works.
|
||||
``mode`` is ``"document"`` (5s) or ``"full"`` (10s); ``timeout`` overrides the budget (how
|
||||
``lsp.wait_timeout`` reaches the loop). Callers must treat False as "no data", NOT "no errors" —
|
||||
the stores may still hold stale entries. Never throws for servers lacking pull support.
|
||||
"""
|
||||
if not (timeout is not None and timeout > 0):
|
||||
timeout = DIAGNOSTICS_FULL_WAIT if mode == "full" else DIAGNOSTICS_DOCUMENT_WAIT
|
||||
@@ -588,9 +565,9 @@ class LSPClient:
|
||||
self._push_event.clear()
|
||||
try:
|
||||
await asyncio.wait_for(self._push_event.wait(), timeout=timeout)
|
||||
return True
|
||||
except asyncio.TimeoutError:
|
||||
return False
|
||||
return True
|
||||
|
||||
async def _wait_for_fresh_push(self, path: str, version: int, timeout: float) -> None:
|
||||
"""Wait until a fresh publishDiagnostics arrives for ``path`` at ``version``+."""
|
||||
@@ -619,11 +596,9 @@ class LSPClient:
|
||||
await self._await_push(min(remaining, 0.5))
|
||||
|
||||
def diagnostics_for(self, path: str, *, fresh_only: bool = False) -> List[Dict[str, Any]]:
|
||||
"""Merged + deduped push/pull diagnostics for one file.
|
||||
|
||||
With ``fresh_only=True`` a store only contributes once its version tag has caught
|
||||
up to the document's — report paths must use this so "stale" and "clean" aren't conflated.
|
||||
"""
|
||||
"""Merged + deduped push/pull diagnostics for one file. With ``fresh_only=True`` a store only
|
||||
contributes once its version tag has caught up to the document's — report paths must use this
|
||||
so "stale" and "clean" aren't conflated."""
|
||||
doc = self._docs.get(os.path.abspath(path))
|
||||
if doc is None:
|
||||
return []
|
||||
@@ -643,12 +618,9 @@ def _dedupe(*lists: List[Dict[str, Any]]) -> List[Dict[str, Any]]:
|
||||
|
||||
|
||||
def _diagnostic_key(d: Dict[str, Any]) -> str:
|
||||
"""Content-equality key: severity + code + source + message + range.
|
||||
|
||||
Shared with the manager's cross-edit delta filter (as ``_diag_key``) so both layers
|
||||
agree on identity. Range is included so an identical error at a second site still
|
||||
surfaces as new; the manager line-shifts its baseline into post-edit coordinates first.
|
||||
"""
|
||||
"""Content-equality key: severity + code + source + message + range. Shared with the manager's
|
||||
cross-edit delta filter (``_diag_key``) so both layers agree on identity. Range is included so an
|
||||
identical error at a second site still surfaces as new (the manager line-shifts its baseline first)."""
|
||||
rng = d.get("range") or {}
|
||||
start = rng.get("start") or {}
|
||||
end = rng.get("end") or {}
|
||||
|
||||
+7
-10
@@ -1,15 +1,12 @@
|
||||
"""Structured logging with steady-state silence for the LSP layer.
|
||||
|
||||
LSP fires on every write_file/patch, so the level model keeps ``agent.log``
|
||||
greppable (``rg 'lsp\\['``) without noise: DEBUG for steady-state events with
|
||||
no novel signal (clean, skipped, repeat "no project root" / "server
|
||||
unavailable"); INFO for once-per-session transitions (first ``active for
|
||||
<root>``, first ``no project root`` per file) and every diagnostic event;
|
||||
WARNING for action-required failures (first ``server unavailable`` per
|
||||
(server_id, binary), every timeout / unexpected error). Dedup uses
|
||||
module-level sets bounded by the distinct pairs touched in one process — a
|
||||
bounded LRU was rejected because evicting an entry would re-fire the line
|
||||
we explicitly want suppressed.
|
||||
LSP fires on every write_file/patch, so the level model keeps ``agent.log`` greppable
|
||||
(``rg 'lsp\\['``) without noise: DEBUG for steady-state events with no novel signal (clean,
|
||||
skipped, repeat "no project root" / "server unavailable"); INFO for once-per-session
|
||||
transitions (first ``active for <root>``, first ``no project root`` per file) and every
|
||||
diagnostic event; WARNING for action-required failures (first ``server unavailable`` per
|
||||
(server_id, binary), every timeout / unexpected error). Dedup uses module-level sets bounded
|
||||
by the distinct pairs touched in one process — a bounded LRU would re-fire suppressed lines.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
|
||||
+7
-11
@@ -83,15 +83,12 @@ def hermes_lsp_bin_dir() -> Path:
|
||||
|
||||
def _native_binary_candidates(base: Path) -> list[Path]:
|
||||
"""Return platform-native executable candidates for a staged binary (``base`` plus Windows wrappers)."""
|
||||
candidates = [base]
|
||||
if _is_windows():
|
||||
seen = {str(base).lower()}
|
||||
for suffix in _WINDOWS_WRAPPER_SUFFIXES:
|
||||
candidate = Path(str(base) + suffix)
|
||||
if str(candidate).lower() not in seen:
|
||||
candidates.append(candidate)
|
||||
seen.add(str(candidate).lower())
|
||||
return candidates
|
||||
if not _is_windows():
|
||||
return [base]
|
||||
cands: Dict[str, Path] = {}
|
||||
for c in (base, *(Path(str(base) + s) for s in _WINDOWS_WRAPPER_SUFFIXES)):
|
||||
cands.setdefault(str(c).lower(), c)
|
||||
return list(cands.values())
|
||||
|
||||
|
||||
def _first_existing(*bases: Path) -> Optional[Path]:
|
||||
@@ -132,8 +129,7 @@ def _do_install(pkg: str) -> Optional[str]:
|
||||
return shutil.which(pkg) # not in our registry — best-effort: just probe PATH
|
||||
strategy = recipe.get("strategy", "manual")
|
||||
bin_name = recipe.get("bin", pkg)
|
||||
existing = _existing_binary(bin_name)
|
||||
if existing:
|
||||
if existing := _existing_binary(bin_name):
|
||||
return existing
|
||||
if strategy == "manual":
|
||||
logger.debug("[install] %s requires manual install (recipe=%s)", pkg, recipe)
|
||||
|
||||
+27
-39
@@ -1,13 +1,11 @@
|
||||
"""Service-level orchestration for LSP clients.
|
||||
|
||||
:class:`LSPService` bridges the synchronous file_operations layer and the async
|
||||
:class:`agent.lsp.client.LSPClient`: one asyncio loop in a background thread
|
||||
(``get_diagnostics_sync`` opens + waits + drains in one blocking call), one lazily
|
||||
spawned client per ``(server_id, workspace_root)``, a **broken-set** of pairs that
|
||||
failed to spawn/initialize (never retried for the life of the service), and a
|
||||
**delta baseline** per file (``snapshot_baseline()`` runs BEFORE a write; the next
|
||||
``get_diagnostics_sync()`` returns only diagnostics not in it). Off unless config
|
||||
enables it — file_operations falls back to in-process syntax checks.
|
||||
:class:`agent.lsp.client.LSPClient`: one asyncio loop in a background thread, one lazily
|
||||
spawned client per ``(server_id, workspace_root)``, a **broken-set** of pairs that failed
|
||||
to spawn/initialize (never retried for the life of the service), and a **delta baseline**
|
||||
per file (``snapshot_baseline()`` runs BEFORE a write; the next ``get_diagnostics_sync()``
|
||||
returns only diagnostics not in it). Off unless config enables it.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
@@ -66,8 +64,7 @@ class _BackgroundLoop:
|
||||
if asyncio.iscoroutine(coro):
|
||||
coro.close()
|
||||
raise RuntimeError("background loop not started")
|
||||
fut = safe_schedule_threadsafe(coro, self._loop)
|
||||
if fut is None:
|
||||
if (fut := safe_schedule_threadsafe(coro, self._loop)) is None:
|
||||
raise RuntimeError("background loop not running")
|
||||
try:
|
||||
return fut.result(timeout=timeout)
|
||||
@@ -77,15 +74,15 @@ class _BackgroundLoop:
|
||||
|
||||
def stop(self) -> None:
|
||||
loop, self._loop = self._loop, None
|
||||
thread, self._thread = self._thread, None
|
||||
if loop is None:
|
||||
return
|
||||
try:
|
||||
loop.call_soon_threadsafe(loop.stop)
|
||||
except RuntimeError:
|
||||
pass
|
||||
if self._thread is not None:
|
||||
self._thread.join(timeout=2.0)
|
||||
self._thread = None
|
||||
if thread is not None:
|
||||
thread.join(timeout=2.0)
|
||||
|
||||
|
||||
class LSPService:
|
||||
@@ -135,17 +132,15 @@ class LSPService:
|
||||
except Exception as e: # noqa: BLE001
|
||||
logger.debug("LSP config load failed: %s", e)
|
||||
return None
|
||||
lsp_cfg = (cfg.get("lsp") or {}) if isinstance(cfg, dict) else {}
|
||||
if not isinstance(lsp_cfg, dict):
|
||||
lsp_cfg = {}
|
||||
lsp_cfg = cfg.get("lsp") if isinstance(cfg, dict) else None
|
||||
lsp_cfg = lsp_cfg if isinstance(lsp_cfg, dict) else {}
|
||||
try:
|
||||
idle_timeout = float(lsp_cfg.get("idle_timeout", DEFAULT_IDLE_TIMEOUT))
|
||||
except (TypeError, ValueError):
|
||||
idle_timeout = DEFAULT_IDLE_TIMEOUT
|
||||
if 0 < idle_timeout < MIN_IDLE_TIMEOUT:
|
||||
# Below the per-op wait budget the reaper could kill a client mid-flight and
|
||||
# the outer timeout would then mark the pair broken for the process lifetime.
|
||||
# Clamp (0 still disables).
|
||||
# Below the per-op wait budget the reaper could kill a client mid-flight and the outer
|
||||
# timeout would then mark the pair broken for the process lifetime. Clamp (0 still disables).
|
||||
idle_timeout = MIN_IDLE_TIMEOUT
|
||||
servers_cfg = lsp_cfg.get("servers") or {}
|
||||
servers = {n: c for n, c in servers_cfg.items() if isinstance(c, dict)} if isinstance(servers_cfg, dict) else {}
|
||||
@@ -195,10 +190,7 @@ class LSPService:
|
||||
|
||||
def snapshot_baseline(self, file_path: str) -> None:
|
||||
"""Snapshot current diagnostics for ``file_path`` as the delta baseline (call BEFORE a write).
|
||||
|
||||
Best-effort: failures are swallowed so a flaky server can't break a
|
||||
write, but outer timeouts mark the pair broken so later edits skip it.
|
||||
"""
|
||||
Best-effort: failures are swallowed so a flaky server can't break a write, but they mark the pair broken."""
|
||||
if not self.enabled_for(file_path):
|
||||
return
|
||||
try:
|
||||
@@ -217,11 +209,10 @@ class LSPService:
|
||||
) -> _Diags:
|
||||
"""Synchronously open ``file_path``, wait for diagnostics, return them. Never raises.
|
||||
|
||||
With ``delta`` (default) the result excludes the :meth:`snapshot_baseline`;
|
||||
``line_shift`` (from :func:`agent.lsp.range_shift.build_line_shift`) remaps
|
||||
that baseline into post-edit coordinates first, so pre-existing diagnostics
|
||||
that merely moved don't look introduced by this edit. ``[]`` when LSP is
|
||||
disabled, nothing matches, or the server can't be spawned.
|
||||
With ``delta`` (default) the result excludes the :meth:`snapshot_baseline`; ``line_shift`` (from
|
||||
:func:`agent.lsp.range_shift.build_line_shift`) remaps that baseline into post-edit coordinates
|
||||
first, so pre-existing diagnostics that merely moved don't look introduced by this edit.
|
||||
``[]`` when LSP is disabled, nothing matches, or the server can't be spawned.
|
||||
"""
|
||||
if not self.enabled_for(file_path):
|
||||
return []
|
||||
@@ -239,9 +230,9 @@ class LSPService:
|
||||
self._mark_broken_for_file(file_path, e)
|
||||
return []
|
||||
if diags is None:
|
||||
# Server alive but no verdict on the post-edit content in budget (common for
|
||||
# tsserver on big projects). Report "no data" rather than stale stores — that
|
||||
# would be the ghost-diagnostics bug. Not marked broken: slow is not dead.
|
||||
# Server alive but no verdict on the post-edit content in budget (common for tsserver on big
|
||||
# projects). Report "no data" rather than stale stores — that would be the ghost-diagnostics
|
||||
# bug. Not marked broken: slow is not dead.
|
||||
eventlog.log_timeout(server_id, file_path, kind="fresh diagnostics")
|
||||
return []
|
||||
if delta:
|
||||
@@ -274,11 +265,9 @@ class LSPService:
|
||||
|
||||
def _mark_broken_for_file(self, file_path: str, exc: BaseException) -> None:
|
||||
"""Mark the file's ``(server_id, root)`` pair broken after an outer timeout/error.
|
||||
|
||||
The outer ``_loop.run`` timeout cancels the in-flight spawn before ``_get_or_spawn``
|
||||
could record the failure; without this every later write would re-pay the full
|
||||
timeout. Also kills any half-initialized client and logs the failure once.
|
||||
"""
|
||||
The outer ``_loop.run`` timeout cancels the in-flight spawn before ``_get_or_spawn`` could record
|
||||
the failure; without this every later write would re-pay the full timeout. Also kills any
|
||||
half-initialized client and logs the failure once."""
|
||||
srv = find_server_for_file(file_path)
|
||||
key = self._broken_key(srv, file_path) if srv is not None else None
|
||||
if key is None:
|
||||
@@ -453,8 +442,7 @@ class LSPService:
|
||||
except asyncio.CancelledError:
|
||||
raise
|
||||
except Exception as e: # noqa: BLE001
|
||||
# A transient sweep error must not kill the reaper, or the
|
||||
# unbounded-accumulation leak it exists to fix comes back.
|
||||
# A transient sweep error must not kill the reaper, or the accumulation leak it fixes comes back.
|
||||
logger.debug("LSP idle reaper sweep error: %s", e)
|
||||
|
||||
async def _reap_idle_once(self) -> None:
|
||||
@@ -469,8 +457,8 @@ class LSPService:
|
||||
await asyncio.gather(*(client.shutdown() for client in clients), return_exceptions=True)
|
||||
|
||||
async def _shutdown_async(self) -> None:
|
||||
reaper, self._idle_reaper_task = self._idle_reaper_task, None
|
||||
if reaper is not None:
|
||||
if (reaper := self._idle_reaper_task) is not None:
|
||||
self._idle_reaper_task = None
|
||||
reaper.cancel()
|
||||
await asyncio.gather(reaper, return_exceptions=True)
|
||||
with self._state_lock:
|
||||
|
||||
@@ -71,8 +71,7 @@ async def _read_headers(reader: asyncio.StreamReader) -> Optional[dict]:
|
||||
async def read_message(reader: asyncio.StreamReader) -> Optional[dict]:
|
||||
"""Read one framed message.
|
||||
|
||||
Returns ``None`` on clean EOF between messages (typical shutdown);
|
||||
raises :class:`LSPProtocolError` on malformed framing.
|
||||
``None`` on clean EOF between messages (typical shutdown); :class:`LSPProtocolError` on malformed framing.
|
||||
"""
|
||||
headers = await _read_headers(reader)
|
||||
if headers is None:
|
||||
@@ -121,10 +120,8 @@ def make_error_response(req_id: Any, code: int, message: str, data: Any = None)
|
||||
|
||||
|
||||
def classify_message(msg: dict) -> Tuple[str, Any]:
|
||||
"""Return ``(kind, key)``: kind ∈ request/response/notification/invalid.
|
||||
|
||||
Key is the id for request/response, the method for notifications, ``None`` for invalid.
|
||||
"""
|
||||
"""Return ``(kind, key)``: kind ∈ request/response/notification/invalid; key is the id (request/response),
|
||||
the method (notification) or ``None`` (invalid)."""
|
||||
if not isinstance(msg, dict) or msg.get("jsonrpc") != "2.0":
|
||||
return "invalid", None
|
||||
if "id" in msg:
|
||||
|
||||
@@ -26,10 +26,9 @@ MAX_SOURCE_CHARS = 80
|
||||
def _sanitize_field(value: Any, *, limit: int) -> str:
|
||||
"""Make a language-server field safe to embed in a tool-result block.
|
||||
|
||||
``message``/``code``/``source`` come from a server that just parsed
|
||||
user-controlled code, so a hostile repo can smuggle instruction-shaped
|
||||
text through identifier names. We collapse CR/LF, drop control chars,
|
||||
cap the length, and HTML-escape ``< > &`` so the text can't close
|
||||
``message``/``code``/``source`` come from a server that just parsed user-controlled code, so a
|
||||
hostile repo can smuggle instruction-shaped text through identifier names. We collapse CR/LF,
|
||||
drop control chars, cap the length, and HTML-escape ``< > &`` so the text can't close
|
||||
``<diagnostics>`` early. ``None``/empty → ``""`` so callers can omit the part.
|
||||
"""
|
||||
if value is None:
|
||||
|
||||
@@ -89,7 +89,6 @@ class ServerContext:
|
||||
|
||||
# ---- helpers ----
|
||||
|
||||
|
||||
def _file_ext_or_basename(path: str) -> str:
|
||||
"""Lower-cased extension, or the full basename for extensionless files (``Dockerfile``)."""
|
||||
base = os.path.basename(path)
|
||||
@@ -148,7 +147,6 @@ def _simple_spawn(server_id: str, which: Sequence[str], args: Sequence[str] = ()
|
||||
|
||||
# ---- bespoke spawn builders ----
|
||||
|
||||
|
||||
def _spawn_pyright(root: str, ctx: ServerContext) -> Optional[SpawnSpec]:
|
||||
bin_path = _find_binary(ctx, "pyright", ("pyright-langserver", "pyright"), "pyright")
|
||||
if bin_path is None:
|
||||
@@ -173,6 +171,7 @@ _warned_once: set = set()
|
||||
|
||||
|
||||
def _warn_once(key: str, message: str) -> None:
|
||||
"""Log ``message`` at WARNING the first time ``key`` is seen in this process."""
|
||||
if key not in _warned_once:
|
||||
_warned_once.add(key)
|
||||
logger.warning(message)
|
||||
@@ -191,12 +190,9 @@ def _spawn_bash_ls(root: str, ctx: ServerContext) -> Optional[SpawnSpec]:
|
||||
|
||||
|
||||
def _find_pses_bundle(ctx: ServerContext) -> Optional[str]:
|
||||
"""Locate the PowerShellEditorServices bundle dir (release zip, manual install).
|
||||
|
||||
Resolution order: ``lsp.servers.powershell.command[0]`` when a directory,
|
||||
``init_overrides["powershell"]["bundlePath"]``, ``PSES_BUNDLE_PATH`` env,
|
||||
then ``<HERMES_HOME>/lsp/PowerShellEditorServices``.
|
||||
"""
|
||||
"""Locate the PowerShellEditorServices bundle dir (release zip, manual install). Resolution order:
|
||||
``lsp.servers.powershell.command[0]`` when a directory, ``init_overrides["powershell"]["bundlePath"]``,
|
||||
``PSES_BUNDLE_PATH`` env, then ``<HERMES_HOME>/lsp/PowerShellEditorServices``."""
|
||||
from hermes_constants import get_hermes_home
|
||||
override = ctx.binary_overrides.get("powershell")
|
||||
init = ctx.init_overrides.get("powershell", {})
|
||||
|
||||
Reference in New Issue
Block a user