Merge branch 'main' into bb/gui
This commit is contained in:
@@ -0,0 +1 @@
|
||||
"""Pytest helpers for LSP-related tests."""
|
||||
@@ -0,0 +1,159 @@
|
||||
#!/usr/bin/env python3
|
||||
"""A minimal in-process LSP server used by tests.
|
||||
|
||||
Speaks just enough LSP to drive :class:`agent.lsp.client.LSPClient`
|
||||
through a full lifecycle: ``initialize``, ``initialized``,
|
||||
``textDocument/didOpen``, ``textDocument/didChange``, then a
|
||||
``textDocument/publishDiagnostics`` notification followed by
|
||||
``shutdown`` + ``exit``.
|
||||
|
||||
Behaviour (all behaviours selectable via env var ``MOCK_LSP_SCRIPT``):
|
||||
|
||||
- ``"clean"`` — initialize, accept didOpen/didChange, push empty
|
||||
diagnostics on every open/change, exit cleanly on shutdown.
|
||||
- ``"errors"`` — same as ``clean`` but the published diagnostics
|
||||
carry one severity-1 entry pointing at line 0:0.
|
||||
- ``"crash"`` — exit immediately after responding to ``initialize``
|
||||
(simulates a crashing server).
|
||||
- ``"slow"`` — same as ``clean`` but sleeps 1s before responding to
|
||||
``initialize`` (lets us test timeout behaviour).
|
||||
|
||||
The script writes JSON-RPC framed messages to stdout and reads from
|
||||
stdin. No third-party dependencies — uses only stdlib so it runs
|
||||
under whatever Python the test process picks up.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
import sys
|
||||
import time
|
||||
|
||||
|
||||
def read_message():
|
||||
"""Read one Content-Length framed JSON-RPC message from stdin."""
|
||||
headers = {}
|
||||
while True:
|
||||
line = sys.stdin.buffer.readline()
|
||||
if not line:
|
||||
return None
|
||||
line = line.rstrip(b"\r\n")
|
||||
if not line:
|
||||
break
|
||||
k, _, v = line.decode("ascii").partition(":")
|
||||
headers[k.strip().lower()] = v.strip()
|
||||
n = int(headers["content-length"])
|
||||
body = sys.stdin.buffer.read(n)
|
||||
return json.loads(body.decode("utf-8"))
|
||||
|
||||
|
||||
def write_message(obj):
|
||||
body = json.dumps(obj, separators=(",", ":")).encode("utf-8")
|
||||
sys.stdout.buffer.write(f"Content-Length: {len(body)}\r\n\r\n".encode("ascii"))
|
||||
sys.stdout.buffer.write(body)
|
||||
sys.stdout.buffer.flush()
|
||||
|
||||
|
||||
def main():
|
||||
script = os.environ.get("MOCK_LSP_SCRIPT", "clean")
|
||||
|
||||
while True:
|
||||
msg = read_message()
|
||||
if msg is None:
|
||||
return 0
|
||||
|
||||
if "id" in msg and msg.get("method") == "initialize":
|
||||
if script == "slow":
|
||||
time.sleep(1.0)
|
||||
write_message(
|
||||
{
|
||||
"jsonrpc": "2.0",
|
||||
"id": msg["id"],
|
||||
"result": {
|
||||
"capabilities": {
|
||||
"textDocumentSync": 1, # Full
|
||||
"diagnosticProvider": {"interFileDependencies": False, "workspaceDiagnostics": False},
|
||||
},
|
||||
"serverInfo": {"name": "mock-lsp", "version": "0.1"},
|
||||
},
|
||||
}
|
||||
)
|
||||
if script == "crash":
|
||||
return 0
|
||||
continue
|
||||
|
||||
if msg.get("method") == "initialized":
|
||||
continue
|
||||
|
||||
if msg.get("method") == "workspace/didChangeConfiguration":
|
||||
continue
|
||||
|
||||
if msg.get("method") == "workspace/didChangeWatchedFiles":
|
||||
continue
|
||||
|
||||
if msg.get("method") in ("textDocument/didOpen", "textDocument/didChange"):
|
||||
params = msg.get("params") or {}
|
||||
td = params.get("textDocument") or {}
|
||||
uri = td.get("uri", "")
|
||||
version = td.get("version", 0)
|
||||
diagnostics = []
|
||||
if script == "errors":
|
||||
diagnostics = [
|
||||
{
|
||||
"range": {
|
||||
"start": {"line": 0, "character": 0},
|
||||
"end": {"line": 0, "character": 5},
|
||||
},
|
||||
"severity": 1,
|
||||
"code": "MOCK001",
|
||||
"source": "mock-lsp",
|
||||
"message": "synthetic error from mock-lsp",
|
||||
}
|
||||
]
|
||||
write_message(
|
||||
{
|
||||
"jsonrpc": "2.0",
|
||||
"method": "textDocument/publishDiagnostics",
|
||||
"params": {
|
||||
"uri": uri,
|
||||
"version": version,
|
||||
"diagnostics": diagnostics,
|
||||
},
|
||||
}
|
||||
)
|
||||
continue
|
||||
|
||||
if msg.get("method") == "textDocument/diagnostic":
|
||||
# Pull endpoint — return empty.
|
||||
write_message(
|
||||
{
|
||||
"jsonrpc": "2.0",
|
||||
"id": msg["id"],
|
||||
"result": {"kind": "full", "items": []},
|
||||
}
|
||||
)
|
||||
continue
|
||||
|
||||
if msg.get("method") == "textDocument/didSave":
|
||||
continue
|
||||
|
||||
if msg.get("method") == "shutdown":
|
||||
write_message({"jsonrpc": "2.0", "id": msg["id"], "result": None})
|
||||
continue
|
||||
|
||||
if msg.get("method") == "exit":
|
||||
return 0
|
||||
|
||||
# Unknown request: respond with method-not-found.
|
||||
if "id" in msg:
|
||||
write_message(
|
||||
{
|
||||
"jsonrpc": "2.0",
|
||||
"id": msg["id"],
|
||||
"error": {"code": -32601, "message": f"method not found: {msg.get('method')}"},
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
@@ -0,0 +1,108 @@
|
||||
"""Integration test: LSP layer is skipped on non-local backends.
|
||||
|
||||
The host-side LSP server can't see files inside a Docker/Modal/SSH
|
||||
sandbox. When the agent's terminal env isn't ``LocalEnvironment``,
|
||||
the file_operations layer must skip both ``snapshot_baseline`` and
|
||||
``get_diagnostics_sync`` calls — falling back to the in-process
|
||||
syntax check exactly as if LSP were disabled.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import sys
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from agent.lsp import eventlog
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _reset():
|
||||
eventlog.reset_announce_caches()
|
||||
|
||||
|
||||
def test_local_only_helper_returns_true_for_local_env():
|
||||
from tools.environments.local import LocalEnvironment
|
||||
from tools.file_operations import ShellFileOperations
|
||||
|
||||
fops = ShellFileOperations(LocalEnvironment(cwd="/tmp"))
|
||||
assert fops._lsp_local_only() is True
|
||||
|
||||
|
||||
def test_local_only_helper_returns_false_for_non_local_env():
|
||||
"""A mocked non-local env (Docker/Modal/SSH stand-in) returns False."""
|
||||
from tools.file_operations import ShellFileOperations
|
||||
|
||||
# Build something that's NOT a LocalEnvironment. We use a bare
|
||||
# MagicMock — isinstance() against LocalEnvironment is False.
|
||||
fake_env = MagicMock()
|
||||
fake_env.execute = MagicMock(return_value=MagicMock(exit_code=0, stdout=""))
|
||||
fake_env.cwd = "/sandbox"
|
||||
fops = ShellFileOperations(fake_env)
|
||||
assert fops._lsp_local_only() is False
|
||||
|
||||
|
||||
def test_snapshot_baseline_skipped_for_non_local(monkeypatch):
|
||||
"""Verify the LSP service's snapshot_baseline is NOT called when
|
||||
the backend isn't local."""
|
||||
from tools.file_operations import ShellFileOperations
|
||||
|
||||
fake_env = MagicMock()
|
||||
fake_env.execute = MagicMock(return_value=MagicMock(exit_code=0, stdout=""))
|
||||
fake_env.cwd = "/sandbox"
|
||||
fops = ShellFileOperations(fake_env)
|
||||
|
||||
snapshot_called = []
|
||||
|
||||
class FakeService:
|
||||
def snapshot_baseline(self, path):
|
||||
snapshot_called.append(path)
|
||||
|
||||
monkeypatch.setattr("agent.lsp.get_service", lambda: FakeService())
|
||||
|
||||
fops._snapshot_lsp_baseline("/sandbox/x.py")
|
||||
assert snapshot_called == [], "snapshot must be skipped for non-local backends"
|
||||
|
||||
|
||||
def test_maybe_lsp_diagnostics_returns_empty_for_non_local(monkeypatch):
|
||||
from tools.file_operations import ShellFileOperations
|
||||
|
||||
fake_env = MagicMock()
|
||||
fake_env.execute = MagicMock(return_value=MagicMock(exit_code=0, stdout=""))
|
||||
fake_env.cwd = "/sandbox"
|
||||
fops = ShellFileOperations(fake_env)
|
||||
|
||||
called = []
|
||||
|
||||
class FakeService:
|
||||
def enabled_for(self, path):
|
||||
called.append(("enabled_for", path))
|
||||
return True
|
||||
def get_diagnostics_sync(self, path, **kw):
|
||||
called.append(("get_diagnostics_sync", path))
|
||||
return [{"severity": 1, "message": "should not see this"}]
|
||||
|
||||
monkeypatch.setattr("agent.lsp.get_service", lambda: FakeService())
|
||||
|
||||
result = fops._maybe_lsp_diagnostics("/sandbox/x.py")
|
||||
assert result == ""
|
||||
assert called == [], "service must not be queried for non-local backends"
|
||||
|
||||
|
||||
def test_snapshot_baseline_called_for_local_env(tmp_path, monkeypatch):
|
||||
from tools.environments.local import LocalEnvironment
|
||||
from tools.file_operations import ShellFileOperations
|
||||
|
||||
fops = ShellFileOperations(LocalEnvironment(cwd=str(tmp_path)))
|
||||
|
||||
snapshot_called = []
|
||||
|
||||
class FakeService:
|
||||
def snapshot_baseline(self, path):
|
||||
snapshot_called.append(path)
|
||||
|
||||
monkeypatch.setattr("agent.lsp.get_service", lambda: FakeService())
|
||||
|
||||
fops._snapshot_lsp_baseline(str(tmp_path / "x.py"))
|
||||
assert snapshot_called == [str(tmp_path / "x.py")]
|
||||
@@ -0,0 +1,213 @@
|
||||
"""Tests for the broken-set short-circuit added to handle outer-timeout failures.
|
||||
|
||||
When ``snapshot_baseline`` or ``get_diagnostics_sync`` time out from the
|
||||
service layer (because a language server hangs during initialize, or
|
||||
the binary is wedged), the inner spawn task is cancelled — but the
|
||||
inner exception handler that adds to ``_broken`` never runs. Without
|
||||
the service-layer fallback added in this module, every subsequent
|
||||
edit re-pays the full timeout cost until the process exits.
|
||||
|
||||
This module verifies:
|
||||
- ``_mark_broken_for_file`` adds the right key
|
||||
- ``enabled_for`` short-circuits on broken keys
|
||||
- a missing binary is broken-set'd after one snapshot attempt
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from agent.lsp.manager import LSPService
|
||||
from agent.lsp.servers import SERVERS, ServerContext, ServerDef, SpawnSpec
|
||||
from agent.lsp.workspace import clear_cache
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _clear_workspace_cache():
|
||||
clear_cache()
|
||||
yield
|
||||
clear_cache()
|
||||
|
||||
|
||||
def _make_git_workspace(tmp_path: Path) -> Path:
|
||||
"""Build a minimal git repo with a pyproject so pyright's root resolver fires."""
|
||||
repo = tmp_path / "repo"
|
||||
repo.mkdir()
|
||||
(repo / ".git").mkdir()
|
||||
(repo / "pyproject.toml").write_text("[project]\nname='t'\n")
|
||||
return repo
|
||||
|
||||
|
||||
def test_mark_broken_for_file_adds_correct_key(tmp_path, monkeypatch):
|
||||
"""``_mark_broken_for_file`` keys the broken-set on
|
||||
(server_id, per_server_root) so subsequent ``enabled_for`` calls
|
||||
for files in the same project skip immediately."""
|
||||
repo = _make_git_workspace(tmp_path)
|
||||
monkeypatch.chdir(str(repo))
|
||||
src = repo / "x.py"
|
||||
src.write_text("")
|
||||
|
||||
svc = LSPService(
|
||||
enabled=True,
|
||||
wait_mode="document",
|
||||
wait_timeout=2.0,
|
||||
install_strategy="manual",
|
||||
)
|
||||
try:
|
||||
svc._mark_broken_for_file(str(src), RuntimeError("simulated"))
|
||||
# The pyright server resolves to the repo root via pyproject.toml.
|
||||
assert ("pyright", str(repo)) in svc._broken
|
||||
finally:
|
||||
svc.shutdown()
|
||||
|
||||
|
||||
def test_enabled_for_returns_false_after_broken(tmp_path, monkeypatch):
|
||||
"""Once a (server_id, root) pair is in the broken-set,
|
||||
``enabled_for`` returns False so the file_operations layer skips
|
||||
the LSP path entirely."""
|
||||
repo = _make_git_workspace(tmp_path)
|
||||
monkeypatch.chdir(str(repo))
|
||||
src = repo / "x.py"
|
||||
src.write_text("")
|
||||
|
||||
svc = LSPService(
|
||||
enabled=True,
|
||||
wait_mode="document",
|
||||
wait_timeout=2.0,
|
||||
install_strategy="manual",
|
||||
)
|
||||
try:
|
||||
# Initially enabled.
|
||||
assert svc.enabled_for(str(src)) is True
|
||||
# Mark broken.
|
||||
svc._mark_broken_for_file(str(src), RuntimeError("simulated"))
|
||||
# Now disabled — the broken-set short-circuits.
|
||||
assert svc.enabled_for(str(src)) is False
|
||||
finally:
|
||||
svc.shutdown()
|
||||
|
||||
|
||||
def test_enabled_for_other_file_in_same_project_also_skipped(tmp_path, monkeypatch):
|
||||
"""The broken key is (server_id, root), so ALL files routed through
|
||||
the same server in the same project are skipped — not just the one
|
||||
that triggered the failure."""
|
||||
repo = _make_git_workspace(tmp_path)
|
||||
monkeypatch.chdir(str(repo))
|
||||
a = repo / "a.py"
|
||||
a.write_text("")
|
||||
b = repo / "b.py"
|
||||
b.write_text("")
|
||||
|
||||
svc = LSPService(
|
||||
enabled=True,
|
||||
wait_mode="document",
|
||||
wait_timeout=2.0,
|
||||
install_strategy="manual",
|
||||
)
|
||||
try:
|
||||
svc._mark_broken_for_file(str(a), RuntimeError("simulated"))
|
||||
# Both files in the same project skip pyright now.
|
||||
assert svc.enabled_for(str(a)) is False
|
||||
assert svc.enabled_for(str(b)) is False
|
||||
finally:
|
||||
svc.shutdown()
|
||||
|
||||
|
||||
def test_unrelated_project_not_affected_by_broken(tmp_path, monkeypatch):
|
||||
"""Marking pyright broken for project A must NOT affect project B."""
|
||||
repo_a = _make_git_workspace(tmp_path)
|
||||
repo_b = tmp_path / "repo-b"
|
||||
repo_b.mkdir()
|
||||
(repo_b / ".git").mkdir()
|
||||
(repo_b / "pyproject.toml").write_text("[project]\nname='b'\n")
|
||||
a_src = repo_a / "x.py"
|
||||
a_src.write_text("")
|
||||
b_src = repo_b / "x.py"
|
||||
b_src.write_text("")
|
||||
|
||||
monkeypatch.chdir(str(repo_a))
|
||||
svc = LSPService(
|
||||
enabled=True,
|
||||
wait_mode="document",
|
||||
wait_timeout=2.0,
|
||||
install_strategy="manual",
|
||||
)
|
||||
try:
|
||||
svc._mark_broken_for_file(str(a_src), RuntimeError("simulated"))
|
||||
# Project A skipped.
|
||||
assert svc.enabled_for(str(a_src)) is False
|
||||
# Project B still enabled — the broken key is per-project.
|
||||
monkeypatch.chdir(str(repo_b))
|
||||
assert svc.enabled_for(str(b_src)) is True
|
||||
finally:
|
||||
svc.shutdown()
|
||||
|
||||
|
||||
def test_mark_broken_handles_missing_server_silently(tmp_path):
|
||||
"""If the file extension doesn't match any registered server,
|
||||
``_mark_broken_for_file`` no-ops — nothing to mark."""
|
||||
svc = LSPService(
|
||||
enabled=True,
|
||||
wait_mode="document",
|
||||
wait_timeout=2.0,
|
||||
install_strategy="manual",
|
||||
)
|
||||
try:
|
||||
# No registered server for .xyz; must not raise.
|
||||
svc._mark_broken_for_file(str(tmp_path / "weird.xyz"), RuntimeError("x"))
|
||||
assert len(svc._broken) == 0
|
||||
finally:
|
||||
svc.shutdown()
|
||||
|
||||
|
||||
def test_mark_broken_handles_no_workspace_silently(tmp_path):
|
||||
"""File outside any git worktree → no workspace → no key to add."""
|
||||
src = tmp_path / "orphan.py"
|
||||
src.write_text("")
|
||||
svc = LSPService(
|
||||
enabled=True,
|
||||
wait_mode="document",
|
||||
wait_timeout=2.0,
|
||||
install_strategy="manual",
|
||||
)
|
||||
try:
|
||||
svc._mark_broken_for_file(str(src), RuntimeError("x"))
|
||||
assert len(svc._broken) == 0
|
||||
finally:
|
||||
svc.shutdown()
|
||||
|
||||
|
||||
def test_snapshot_failure_marks_broken_via_outer_timeout(tmp_path, monkeypatch):
|
||||
"""End-to-end: ``snapshot_baseline``'s outer ``_loop.run`` timeout
|
||||
triggers ``_mark_broken_for_file``, so a second call to
|
||||
``enabled_for`` returns False."""
|
||||
repo = _make_git_workspace(tmp_path)
|
||||
monkeypatch.chdir(str(repo))
|
||||
src = repo / "x.py"
|
||||
src.write_text("")
|
||||
|
||||
svc = LSPService(
|
||||
enabled=True,
|
||||
wait_mode="document",
|
||||
wait_timeout=2.0,
|
||||
install_strategy="manual",
|
||||
)
|
||||
try:
|
||||
# Force the inner snapshot coroutine to raise.
|
||||
async def boom(_path):
|
||||
raise RuntimeError("outer-timeout simulated")
|
||||
|
||||
with patch.object(svc, "_snapshot_async", boom):
|
||||
assert svc.enabled_for(str(src)) is True
|
||||
svc.snapshot_baseline(str(src))
|
||||
|
||||
# After the failure, the file's pair is in the broken-set and
|
||||
# ``enabled_for`` skips it.
|
||||
assert ("pyright", str(repo)) in svc._broken
|
||||
assert svc.enabled_for(str(src)) is False
|
||||
finally:
|
||||
svc.shutdown()
|
||||
@@ -0,0 +1,143 @@
|
||||
"""End-to-end client tests against the in-process mock LSP server.
|
||||
|
||||
Spins up :file:`_mock_lsp_server.py` as an actual subprocess, drives
|
||||
it through real LSP traffic, and asserts diagnostic flow. This is
|
||||
the closest thing we have to integration coverage without requiring
|
||||
pyright/gopls/etc. to be installed in CI.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import os
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from agent.lsp.client import LSPClient
|
||||
|
||||
|
||||
MOCK_SERVER = str(Path(__file__).parent / "_mock_lsp_server.py")
|
||||
|
||||
|
||||
def _client(workspace: Path, script: str = "clean") -> LSPClient:
|
||||
env = {"MOCK_LSP_SCRIPT": script, "PYTHONPATH": os.environ.get("PYTHONPATH", "")}
|
||||
return LSPClient(
|
||||
server_id=f"mock-{script}",
|
||||
workspace_root=str(workspace),
|
||||
command=[sys.executable, MOCK_SERVER],
|
||||
env=env,
|
||||
cwd=str(workspace),
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_client_lifecycle_clean(tmp_path: Path):
|
||||
"""Full lifecycle: spawn, initialize, open, get clean diagnostics, shutdown."""
|
||||
f = tmp_path / "x.py"
|
||||
f.write_text("print('hi')\n")
|
||||
|
||||
client = _client(tmp_path, "clean")
|
||||
await client.start()
|
||||
try:
|
||||
assert client.is_running
|
||||
version = await client.open_file(str(f), language_id="python")
|
||||
assert version == 0
|
||||
await client.wait_for_diagnostics(str(f), version, mode="document")
|
||||
diags = client.diagnostics_for(str(f))
|
||||
assert diags == []
|
||||
finally:
|
||||
await client.shutdown()
|
||||
assert not client.is_running
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_client_receives_published_errors(tmp_path: Path):
|
||||
f = tmp_path / "x.py"
|
||||
f.write_text("print('hi')\n")
|
||||
|
||||
client = _client(tmp_path, "errors")
|
||||
await client.start()
|
||||
try:
|
||||
version = await client.open_file(str(f), language_id="python")
|
||||
await client.wait_for_diagnostics(str(f), version, mode="document")
|
||||
diags = client.diagnostics_for(str(f))
|
||||
assert len(diags) == 1
|
||||
d = diags[0]
|
||||
assert d["severity"] == 1
|
||||
assert d["code"] == "MOCK001"
|
||||
assert d["source"] == "mock-lsp"
|
||||
assert "synthetic error" in d["message"]
|
||||
finally:
|
||||
await client.shutdown()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_client_didchange_bumps_version(tmp_path: Path):
|
||||
f = tmp_path / "x.py"
|
||||
f.write_text("print('hi')\n")
|
||||
|
||||
client = _client(tmp_path, "errors")
|
||||
await client.start()
|
||||
try:
|
||||
v0 = await client.open_file(str(f), language_id="python")
|
||||
f.write_text("print('hi 2')\n")
|
||||
v1 = await client.open_file(str(f), language_id="python") # re-open path = didChange
|
||||
assert v1 == v0 + 1
|
||||
await client.wait_for_diagnostics(str(f), v1, mode="document")
|
||||
# Mock pushed a diagnostic for both events; merged view has one
|
||||
# entry (push store keyed by file path).
|
||||
diags = client.diagnostics_for(str(f))
|
||||
assert len(diags) == 1
|
||||
finally:
|
||||
await client.shutdown()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_client_handles_crashing_server(tmp_path: Path):
|
||||
"""When the server exits right after initialize, subsequent requests
|
||||
fail gracefully (not hang)."""
|
||||
f = tmp_path / "x.py"
|
||||
f.write_text("")
|
||||
|
||||
client = _client(tmp_path, "crash")
|
||||
await client.start() # should succeed (mock answers initialize before crashing)
|
||||
# Give the OS a moment to deliver the EOF.
|
||||
await asyncio.sleep(0.2)
|
||||
# The reader loop should detect EOF and mark pending requests as failed.
|
||||
try:
|
||||
await asyncio.wait_for(
|
||||
client.open_file(str(f), language_id="python"), timeout=2.0
|
||||
)
|
||||
except Exception:
|
||||
pass # any exception is acceptable; the contract is "doesn't hang"
|
||||
await client.shutdown()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_client_shutdown_idempotent(tmp_path: Path):
|
||||
"""Calling shutdown twice must be safe."""
|
||||
f = tmp_path / "x.py"
|
||||
f.write_text("")
|
||||
client = _client(tmp_path, "clean")
|
||||
await client.start()
|
||||
await client.shutdown()
|
||||
await client.shutdown() # must not raise
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_client_diagnostics_are_deduped(tmp_path: Path):
|
||||
"""Repeated identical pushes must not produce duplicate diagnostics."""
|
||||
f = tmp_path / "x.py"
|
||||
f.write_text("")
|
||||
client = _client(tmp_path, "errors")
|
||||
await client.start()
|
||||
try:
|
||||
for _ in range(3):
|
||||
v = await client.open_file(str(f), language_id="python")
|
||||
await client.wait_for_diagnostics(str(f), v, mode="document")
|
||||
diags = client.diagnostics_for(str(f))
|
||||
# Push store overwrites on every notification — should have 1.
|
||||
assert len(diags) == 1
|
||||
finally:
|
||||
await client.shutdown()
|
||||
@@ -0,0 +1,146 @@
|
||||
"""Tests for the ``lsp_diagnostics`` field on WriteResult / PatchResult.
|
||||
|
||||
The field exists so the agent can read syntax errors (``lint``) and
|
||||
semantic errors (``lsp_diagnostics``) as separate signals rather than
|
||||
having LSP output prepended to the lint string.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import sys
|
||||
import tempfile
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from tools.environments.local import LocalEnvironment
|
||||
from tools.file_operations import (
|
||||
PatchResult,
|
||||
ShellFileOperations,
|
||||
WriteResult,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Dataclass shape
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_writeresult_lsp_diagnostics_optional():
|
||||
r = WriteResult()
|
||||
assert r.lsp_diagnostics is None
|
||||
|
||||
|
||||
def test_writeresult_to_dict_omits_field_when_none():
|
||||
r = WriteResult(bytes_written=10)
|
||||
assert "lsp_diagnostics" not in r.to_dict()
|
||||
|
||||
|
||||
def test_writeresult_to_dict_includes_field_when_set():
|
||||
r = WriteResult(bytes_written=10, lsp_diagnostics="<diagnostics>...</diagnostics>")
|
||||
d = r.to_dict()
|
||||
assert d["lsp_diagnostics"] == "<diagnostics>...</diagnostics>"
|
||||
|
||||
|
||||
def test_patchresult_to_dict_includes_field_when_set():
|
||||
r = PatchResult(success=True, lsp_diagnostics="ERROR [1:1] thing")
|
||||
d = r.to_dict()
|
||||
assert d["lsp_diagnostics"] == "ERROR [1:1] thing"
|
||||
|
||||
|
||||
def test_patchresult_to_dict_omits_field_when_none():
|
||||
r = PatchResult(success=True)
|
||||
assert "lsp_diagnostics" not in r.to_dict()
|
||||
|
||||
|
||||
def test_patchresult_to_dict_omits_field_when_empty_string():
|
||||
"""Empty string counts as falsy — agent shouldn't see an empty field."""
|
||||
r = PatchResult(success=True, lsp_diagnostics="")
|
||||
assert "lsp_diagnostics" not in r.to_dict()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Channel separation: lint and lsp_diagnostics stay independent
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_lint_and_lsp_diagnostics_are_separate_channels():
|
||||
"""A WriteResult can carry BOTH a syntax-error lint AND an LSP
|
||||
diagnostic block. They belong in separate fields."""
|
||||
r = WriteResult(
|
||||
bytes_written=42,
|
||||
lint={"status": "error", "output": "SyntaxError: ..."},
|
||||
lsp_diagnostics="<diagnostics>ERROR [1:5] type mismatch</diagnostics>",
|
||||
)
|
||||
d = r.to_dict()
|
||||
assert "lint" in d
|
||||
assert "lsp_diagnostics" in d
|
||||
assert d["lint"]["output"] == "SyntaxError: ..."
|
||||
assert "type mismatch" in d["lsp_diagnostics"]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# write_file populates the field via _maybe_lsp_diagnostics
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_write_file_populates_lsp_diagnostics_when_layer_returns_block(tmp_path):
|
||||
"""When the LSP layer returns a non-empty block, write_file puts it
|
||||
into the ``lsp_diagnostics`` field — NOT into ``lint.output``."""
|
||||
fops = ShellFileOperations(LocalEnvironment(cwd=str(tmp_path)))
|
||||
target = tmp_path / "x.py"
|
||||
|
||||
block = "<diagnostics file=\"x.py\">\nERROR [1:1] problem\n</diagnostics>"
|
||||
|
||||
with patch.object(fops, "_maybe_lsp_diagnostics", return_value=block):
|
||||
res = fops.write_file(str(target), "x = 1\n")
|
||||
|
||||
assert res.lsp_diagnostics == block
|
||||
# Lint is the syntax check, which is clean for "x = 1" — must NOT
|
||||
# have the LSP block folded into it.
|
||||
assert res.lint == {"status": "ok", "output": ""}
|
||||
|
||||
|
||||
def test_write_file_lsp_diagnostics_none_when_layer_returns_empty(tmp_path):
|
||||
fops = ShellFileOperations(LocalEnvironment(cwd=str(tmp_path)))
|
||||
target = tmp_path / "x.py"
|
||||
|
||||
with patch.object(fops, "_maybe_lsp_diagnostics", return_value=""):
|
||||
res = fops.write_file(str(target), "x = 1\n")
|
||||
|
||||
assert res.lsp_diagnostics is None
|
||||
|
||||
|
||||
def test_write_file_skips_lsp_when_syntax_failed(tmp_path):
|
||||
"""If the syntax check finds errors, the LSP layer should not be
|
||||
consulted (a file that won't parse won't yield meaningful semantic
|
||||
diagnostics)."""
|
||||
fops = ShellFileOperations(LocalEnvironment(cwd=str(tmp_path)))
|
||||
target = tmp_path / "broken.py"
|
||||
|
||||
with patch.object(fops, "_maybe_lsp_diagnostics") as mock_lsp:
|
||||
res = fops.write_file(str(target), "def x(:\n") # syntax error
|
||||
assert mock_lsp.call_count == 0
|
||||
assert res.lsp_diagnostics is None
|
||||
assert res.lint["status"] == "error"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# patch_replace propagates the field from the inner write_file
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_patch_replace_propagates_lsp_diagnostics(tmp_path):
|
||||
"""patch_replace's internal write_file populates lsp_diagnostics —
|
||||
the outer PatchResult must carry it forward."""
|
||||
fops = ShellFileOperations(LocalEnvironment(cwd=str(tmp_path)))
|
||||
target = tmp_path / "x.py"
|
||||
target.write_text("x = 1\n")
|
||||
|
||||
block = "<diagnostics>ERROR [1:5] semantic issue</diagnostics>"
|
||||
|
||||
with patch.object(fops, "_maybe_lsp_diagnostics", return_value=block):
|
||||
res = fops.patch_replace(str(target), "x = 1", "x = 2")
|
||||
|
||||
assert res.success is True
|
||||
assert res.lsp_diagnostics == block
|
||||
@@ -0,0 +1,199 @@
|
||||
"""Tests for the structured logging dedup model.
|
||||
|
||||
The contract: a 1000-write session in one project should emit exactly
|
||||
ONE INFO line ("active for <root>") at the default INFO threshold.
|
||||
Steady-state events stay at DEBUG; first-time-seen events surface
|
||||
once at INFO/WARNING.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
|
||||
import pytest
|
||||
|
||||
from agent.lsp import eventlog
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _reset():
|
||||
eventlog.reset_announce_caches()
|
||||
yield
|
||||
eventlog.reset_announce_caches()
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def caplog_lsp(caplog):
|
||||
caplog.set_level(logging.DEBUG, logger="hermes.lint.lsp")
|
||||
return caplog
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Steady-state silence (DEBUG)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_clean_emits_at_debug(caplog_lsp):
|
||||
for _ in range(10):
|
||||
eventlog.log_clean("pyright", "/proj/x.py")
|
||||
info_records = [r for r in caplog_lsp.records if r.levelno >= logging.INFO]
|
||||
debug_records = [r for r in caplog_lsp.records if r.levelno == logging.DEBUG]
|
||||
assert info_records == []
|
||||
assert len(debug_records) == 10
|
||||
|
||||
|
||||
def test_disabled_emits_at_debug(caplog_lsp):
|
||||
eventlog.log_disabled("pyright", "/x.py", "feature off")
|
||||
eventlog.log_disabled("pyright", "/x.py", "ext not mapped")
|
||||
assert all(r.levelno == logging.DEBUG for r in caplog_lsp.records)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# State transitions: INFO once, DEBUG thereafter
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_active_for_fires_once_per_root(caplog_lsp):
|
||||
for _ in range(50):
|
||||
eventlog.log_active("pyright", "/proj")
|
||||
info_records = [
|
||||
r for r in caplog_lsp.records
|
||||
if r.levelno == logging.INFO and "active for" in r.getMessage()
|
||||
]
|
||||
assert len(info_records) == 1
|
||||
|
||||
|
||||
def test_active_for_fires_per_distinct_root(caplog_lsp):
|
||||
eventlog.log_active("pyright", "/proj-a")
|
||||
eventlog.log_active("pyright", "/proj-b")
|
||||
info = [r for r in caplog_lsp.records if r.levelno == logging.INFO]
|
||||
assert len(info) == 2
|
||||
|
||||
|
||||
def test_active_for_separate_per_server(caplog_lsp):
|
||||
eventlog.log_active("pyright", "/proj")
|
||||
eventlog.log_active("typescript", "/proj")
|
||||
info = [r for r in caplog_lsp.records if r.levelno == logging.INFO]
|
||||
assert len(info) == 2
|
||||
|
||||
|
||||
def test_no_project_root_fires_once_per_path(caplog_lsp):
|
||||
for _ in range(5):
|
||||
eventlog.log_no_project_root("pyright", "/orphan.py")
|
||||
info = [r for r in caplog_lsp.records if r.levelno == logging.INFO]
|
||||
assert len(info) == 1
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Diagnostics events fire INFO every time
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_diagnostics_always_info(caplog_lsp):
|
||||
for i in range(5):
|
||||
eventlog.log_diagnostics("pyright", f"/x{i}.py", 1)
|
||||
info = [r for r in caplog_lsp.records if r.levelno == logging.INFO]
|
||||
assert len(info) == 5
|
||||
assert all("diags" in r.getMessage() for r in info)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Action-required: WARNING once, DEBUG thereafter (or per call for novel events)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_server_unavailable_warns_once_per_binary(caplog_lsp):
|
||||
for _ in range(20):
|
||||
eventlog.log_server_unavailable("pyright", "pyright-langserver")
|
||||
warns = [r for r in caplog_lsp.records if r.levelno == logging.WARNING]
|
||||
assert len(warns) == 1
|
||||
assert "pyright-langserver" in warns[0].getMessage()
|
||||
|
||||
|
||||
def test_server_unavailable_separate_per_binary(caplog_lsp):
|
||||
eventlog.log_server_unavailable("pyright", "pyright-langserver")
|
||||
eventlog.log_server_unavailable("typescript", "typescript-language-server")
|
||||
warns = [r for r in caplog_lsp.records if r.levelno == logging.WARNING]
|
||||
assert len(warns) == 2
|
||||
|
||||
|
||||
def test_no_server_configured_warns_once(caplog_lsp):
|
||||
for _ in range(10):
|
||||
eventlog.log_no_server_configured("pyright")
|
||||
warns = [r for r in caplog_lsp.records if r.levelno == logging.WARNING]
|
||||
assert len(warns) == 1
|
||||
|
||||
|
||||
def test_timeout_warns_every_call(caplog_lsp):
|
||||
for _ in range(3):
|
||||
eventlog.log_timeout("pyright", "/x.py")
|
||||
warns = [r for r in caplog_lsp.records if r.levelno == logging.WARNING]
|
||||
assert len(warns) == 3
|
||||
|
||||
|
||||
def test_server_error_warns_every_call(caplog_lsp):
|
||||
for _ in range(3):
|
||||
eventlog.log_server_error("pyright", "/x.py", RuntimeError("boom"))
|
||||
warns = [r for r in caplog_lsp.records if r.levelno == logging.WARNING]
|
||||
assert len(warns) == 3
|
||||
|
||||
|
||||
def test_spawn_failed_warns(caplog_lsp):
|
||||
eventlog.log_spawn_failed("pyright", "/proj", FileNotFoundError("nope"))
|
||||
warns = [r for r in caplog_lsp.records if r.levelno == logging.WARNING]
|
||||
assert len(warns) == 1
|
||||
assert "spawn/initialize failed" in warns[0].getMessage()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Format: log lines all carry the lsp[<server_id>] prefix for grep
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_log_lines_use_lsp_prefix(caplog_lsp):
|
||||
eventlog.log_clean("pyright", "/x.py")
|
||||
eventlog.log_active("pyright", "/proj")
|
||||
eventlog.log_diagnostics("typescript", "/y.ts", 2)
|
||||
for r in caplog_lsp.records:
|
||||
assert r.getMessage().startswith("lsp[")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Steady-state contract: 1000 clean writes → 1 INFO at most
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_thousand_clean_writes_emit_one_info(caplog_lsp):
|
||||
"""A long session writes lots of files cleanly; agent.log should
|
||||
show ONE 'active for' INFO and zero other INFO lines."""
|
||||
eventlog.log_active("pyright", "/proj")
|
||||
for _ in range(1000):
|
||||
eventlog.log_clean("pyright", "/proj/x.py")
|
||||
info_records = [r for r in caplog_lsp.records if r.levelno == logging.INFO]
|
||||
assert len(info_records) == 1
|
||||
assert "active for" in info_records[0].getMessage()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Path shortening
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_short_path_uses_relative_when_inside_cwd(tmp_path, monkeypatch):
|
||||
monkeypatch.chdir(tmp_path)
|
||||
sub = tmp_path / "x.py"
|
||||
sub.write_text("")
|
||||
out = eventlog._short_path(str(sub))
|
||||
assert out == "x.py"
|
||||
|
||||
|
||||
def test_short_path_keeps_absolute_when_outside(tmp_path, monkeypatch):
|
||||
monkeypatch.chdir(tmp_path / "a") if (tmp_path / "a").exists() else None
|
||||
monkeypatch.chdir(tmp_path)
|
||||
other = "/var/log/foo.txt"
|
||||
out = eventlog._short_path(other)
|
||||
# Outside cwd: keeps absolute (no leading "../")
|
||||
assert out == "/var/log/foo.txt" or not out.startswith("..")
|
||||
|
||||
|
||||
def test_short_path_handles_empty_string():
|
||||
assert eventlog._short_path("") == ""
|
||||
@@ -0,0 +1,279 @@
|
||||
"""Tests for follow-up fixes to the LSP integration (PR after #24168).
|
||||
|
||||
Covers:
|
||||
|
||||
1. ``typescript-language-server`` install recipe pulls in ``typescript``
|
||||
alongside the server, so the npm install command targets both.
|
||||
2. ``hermes lsp status`` surfaces a ``Backend warnings`` section when
|
||||
bash-language-server is installed but ``shellcheck`` is missing.
|
||||
3. ``_check_lint`` returns ``skipped`` (not ``error``) when the linter
|
||||
command exists on PATH but couldn't actually run — e.g. ``npx tsc``
|
||||
without the typescript SDK installed. This is what unblocks the
|
||||
LSP semantic tier on TypeScript files when the user doesn't also
|
||||
have a project-level ``tsc``.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import io
|
||||
from contextlib import redirect_stdout
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from agent.lsp.install import INSTALL_RECIPES
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fix 1: typescript install recipe carries the typescript SDK
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_typescript_recipe_includes_typescript_sdk():
|
||||
recipe = INSTALL_RECIPES["typescript-language-server"]
|
||||
extras = recipe.get("extra_pkgs") or []
|
||||
assert "typescript" in extras, (
|
||||
"typescript-language-server requires the `typescript` SDK as a "
|
||||
"sibling install — without it `initialize` fails with "
|
||||
"'Could not find a valid TypeScript installation'."
|
||||
)
|
||||
|
||||
|
||||
def test_install_npm_passes_extras_to_npm_command(tmp_path, monkeypatch):
|
||||
"""Verify the npm subprocess is invoked with both pkg AND extras."""
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
|
||||
captured = {}
|
||||
|
||||
def fake_run(cmd, **kwargs):
|
||||
captured["cmd"] = cmd
|
||||
# Pretend npm succeeded but binary doesn't exist — install code
|
||||
# will return None, which is fine for this test.
|
||||
return MagicMock(returncode=0, stderr="")
|
||||
|
||||
from agent.lsp import install as install_mod
|
||||
|
||||
monkeypatch.setattr(install_mod.subprocess, "run", fake_run)
|
||||
monkeypatch.setattr(install_mod.shutil, "which", lambda c: "/usr/bin/npm" if c == "npm" else None)
|
||||
|
||||
install_mod._install_npm("typescript-language-server", "typescript-language-server",
|
||||
extra_pkgs=["typescript"])
|
||||
|
||||
cmd = captured["cmd"]
|
||||
assert "typescript-language-server" in cmd
|
||||
assert "typescript" in cmd
|
||||
# Both must come AFTER the npm flags, in install-target position
|
||||
install_idx = cmd.index("install")
|
||||
assert cmd.index("typescript-language-server") > install_idx
|
||||
assert cmd.index("typescript") > install_idx
|
||||
|
||||
|
||||
def test_install_npm_works_without_extras(tmp_path, monkeypatch):
|
||||
"""Backwards compat: pyright-style recipes (no extras) still install."""
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
|
||||
captured = {}
|
||||
|
||||
def fake_run(cmd, **kwargs):
|
||||
captured["cmd"] = cmd
|
||||
return MagicMock(returncode=0, stderr="")
|
||||
|
||||
from agent.lsp import install as install_mod
|
||||
|
||||
monkeypatch.setattr(install_mod.subprocess, "run", fake_run)
|
||||
monkeypatch.setattr(install_mod.shutil, "which", lambda c: "/usr/bin/npm" if c == "npm" else None)
|
||||
|
||||
install_mod._install_npm("pyright", "pyright-langserver")
|
||||
|
||||
cmd = captured["cmd"]
|
||||
assert "pyright" in cmd
|
||||
# Should not blow up when extra_pkgs is omitted/None
|
||||
install_targets = [c for c in cmd if not c.startswith("-") and c not in (
|
||||
"install", "--prefix", str(install_mod.hermes_lsp_bin_dir().parent),
|
||||
"/usr/bin/npm",
|
||||
)]
|
||||
assert install_targets == ["pyright"]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fix 2: ``hermes lsp status`` surfaces shellcheck-missing for bash
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_backend_warnings_quiet_when_bash_not_installed(tmp_path, monkeypatch):
|
||||
"""No bash → no warning."""
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
from agent.lsp import cli as lsp_cli
|
||||
|
||||
with patch("shutil.which", return_value=None):
|
||||
notes = lsp_cli._backend_warnings()
|
||||
assert notes == []
|
||||
|
||||
|
||||
def test_backend_warnings_quiet_when_bash_and_shellcheck_both_present(tmp_path, monkeypatch):
|
||||
"""Both installed → no warning."""
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
from agent.lsp import cli as lsp_cli
|
||||
|
||||
def which(name):
|
||||
return f"/usr/bin/{name}" # both found
|
||||
|
||||
with patch("shutil.which", side_effect=which):
|
||||
notes = lsp_cli._backend_warnings()
|
||||
assert notes == []
|
||||
|
||||
|
||||
def test_backend_warnings_fires_when_bash_installed_but_shellcheck_missing(tmp_path, monkeypatch):
|
||||
"""The exact scenario from the bug report."""
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
from agent.lsp import cli as lsp_cli
|
||||
|
||||
def which(name):
|
||||
if name == "bash-language-server":
|
||||
return "/fake/bin/bash-language-server"
|
||||
return None # shellcheck missing
|
||||
|
||||
with patch("shutil.which", side_effect=which):
|
||||
notes = lsp_cli._backend_warnings()
|
||||
assert len(notes) == 1
|
||||
assert "shellcheck" in notes[0].lower()
|
||||
assert "bash-language-server" in notes[0].lower()
|
||||
|
||||
|
||||
def test_status_output_includes_backend_warnings_section(tmp_path, monkeypatch):
|
||||
"""End-to-end: status command output includes the warning section."""
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
|
||||
# Pretend bash-language-server is installed but shellcheck is missing
|
||||
def which(name):
|
||||
if name == "bash-language-server":
|
||||
return "/fake/bin/bash-language-server"
|
||||
return None
|
||||
|
||||
from agent.lsp import cli as lsp_cli
|
||||
|
||||
buf = io.StringIO()
|
||||
with patch("shutil.which", side_effect=which), redirect_stdout(buf):
|
||||
lsp_cli._cmd_status(emit_json=False)
|
||||
|
||||
output = buf.getvalue()
|
||||
assert "Backend warnings" in output
|
||||
assert "shellcheck" in output
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fix 3: tier-1 lint treats unusable linters as ``skipped``, not ``error``
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_npx_tsc_missing_treated_as_skipped():
|
||||
"""The original bug: ``npx tsc`` errors when tsc isn't installed.
|
||||
|
||||
Without this fix, the lint result is ``error``, which means the LSP
|
||||
semantic tier (gated on ``success or skipped``) is skipped — the user
|
||||
gets a useless tooling-error message instead of real diagnostics.
|
||||
"""
|
||||
from tools.file_operations import _looks_like_linter_unusable
|
||||
|
||||
npx_failure_output = (
|
||||
" \n"
|
||||
" This is not the tsc command you are looking for \n"
|
||||
" \n"
|
||||
"\n"
|
||||
"To get access to the TypeScript compiler, tsc, from the command line either:\n"
|
||||
"- Use npm install typescript to first add TypeScript to your project before using npx\n"
|
||||
)
|
||||
|
||||
assert _looks_like_linter_unusable("npx", npx_failure_output) is True
|
||||
|
||||
|
||||
def test_real_lint_error_not_classified_as_unusable():
|
||||
"""A genuine TypeScript type error must NOT be misclassified."""
|
||||
from tools.file_operations import _looks_like_linter_unusable
|
||||
|
||||
real_error = (
|
||||
"bad.ts:5:1 - error TS2322: Type 'number' is not assignable to type 'string'.\n"
|
||||
"5 const x: string = greet(42);\n"
|
||||
" ~~~~~~~~~~~~~~~\n"
|
||||
)
|
||||
|
||||
assert _looks_like_linter_unusable("npx", real_error) is False
|
||||
|
||||
|
||||
def test_unknown_base_cmd_returns_false():
|
||||
"""Unfamiliar linters fall through and use the normal error path."""
|
||||
from tools.file_operations import _looks_like_linter_unusable
|
||||
|
||||
assert _looks_like_linter_unusable("eslint", "any output") is False
|
||||
assert _looks_like_linter_unusable("", "anything") is False
|
||||
|
||||
|
||||
def test_check_lint_returns_skipped_when_npx_tsc_unusable(tmp_path):
|
||||
"""Integration: _check_lint sees npx exit non-zero with the npx banner
|
||||
and returns a ``skipped`` LintResult so LSP can still run."""
|
||||
from tools.environments.local import LocalEnvironment
|
||||
from tools.file_operations import ShellFileOperations
|
||||
|
||||
ts_file = tmp_path / "bad.ts"
|
||||
ts_file.write_text("const x: string = 42;\n")
|
||||
|
||||
env = LocalEnvironment()
|
||||
fops = ShellFileOperations(env)
|
||||
|
||||
# Patch _exec to simulate ``npx tsc`` failing because tsc is missing.
|
||||
npx_banner = (
|
||||
" \n"
|
||||
" This is not the tsc command you are looking for \n"
|
||||
)
|
||||
|
||||
def fake_exec(cmd, **kwargs):
|
||||
result = MagicMock()
|
||||
result.exit_code = 1
|
||||
result.stdout = npx_banner
|
||||
return result
|
||||
|
||||
with patch.object(fops, "_exec", side_effect=fake_exec), \
|
||||
patch.object(fops, "_has_command", return_value=True):
|
||||
lint = fops._check_lint(str(ts_file))
|
||||
|
||||
assert lint.skipped is True, (
|
||||
f"expected skipped (so LSP runs); got success={lint.success}, "
|
||||
f"output={lint.output!r}"
|
||||
)
|
||||
assert "not usable" in (lint.message or "")
|
||||
|
||||
|
||||
def test_check_lint_returns_error_for_real_ts_type_errors(tmp_path):
|
||||
"""Sanity: real TypeScript errors still go through the error path."""
|
||||
from tools.environments.local import LocalEnvironment
|
||||
from tools.file_operations import ShellFileOperations
|
||||
|
||||
ts_file = tmp_path / "bad.ts"
|
||||
ts_file.write_text("const x: string = 42;\n")
|
||||
|
||||
env = LocalEnvironment()
|
||||
fops = ShellFileOperations(env)
|
||||
|
||||
real_tsc_error = (
|
||||
"bad.ts:1:7 - error TS2322: Type 'number' is not assignable to type 'string'.\n"
|
||||
"1 const x: string = 42;\n"
|
||||
" ~\n"
|
||||
"Found 1 error.\n"
|
||||
)
|
||||
|
||||
def fake_exec(cmd, **kwargs):
|
||||
result = MagicMock()
|
||||
result.exit_code = 1
|
||||
result.stdout = real_tsc_error
|
||||
return result
|
||||
|
||||
with patch.object(fops, "_exec", side_effect=fake_exec), \
|
||||
patch.object(fops, "_has_command", return_value=True):
|
||||
lint = fops._check_lint(str(ts_file))
|
||||
|
||||
assert lint.skipped is False
|
||||
assert lint.success is False
|
||||
assert "TS2322" in lint.output
|
||||
|
||||
|
||||
if __name__ == "__main__": # pragma: no cover
|
||||
pytest.main([__file__, "-v"])
|
||||
@@ -0,0 +1,144 @@
|
||||
"""Tests for service-singleton lifecycle: atexit handler, idempotent shutdown.
|
||||
|
||||
These cover the exit-cleanup behavior added to plug the language-server
|
||||
process leak — without the atexit hook, ``hermes chat`` exits while
|
||||
pyright/gopls/etc. are still alive on the host.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import atexit
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from agent import lsp as lsp_module
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _reset_singleton():
|
||||
"""Force a clean module state before each test.
|
||||
|
||||
Tests in this file share process-global state (the lazy
|
||||
singleton + atexit registration flag); reset both before and
|
||||
after every test so order doesn't matter.
|
||||
"""
|
||||
lsp_module._service = None
|
||||
lsp_module._atexit_registered = False
|
||||
yield
|
||||
lsp_module._service = None
|
||||
lsp_module._atexit_registered = False
|
||||
|
||||
|
||||
def test_get_service_registers_atexit_handler_once(monkeypatch):
|
||||
"""First call to ``get_service`` must register an atexit handler;
|
||||
subsequent calls must NOT register another one (Python's ``atexit``
|
||||
runs every registered callable, so a duplicate would shutdown
|
||||
twice — harmless but wasteful)."""
|
||||
fake_svc = MagicMock()
|
||||
fake_svc.is_active.return_value = True
|
||||
monkeypatch.setattr(
|
||||
lsp_module.LSPService, "create_from_config", classmethod(lambda cls: fake_svc)
|
||||
)
|
||||
|
||||
registrations = []
|
||||
|
||||
def fake_register(fn):
|
||||
registrations.append(fn)
|
||||
|
||||
monkeypatch.setattr(atexit, "register", fake_register)
|
||||
|
||||
a = lsp_module.get_service()
|
||||
b = lsp_module.get_service()
|
||||
c = lsp_module.get_service()
|
||||
|
||||
assert a is fake_svc
|
||||
assert b is fake_svc
|
||||
assert c is fake_svc
|
||||
assert len(registrations) == 1
|
||||
# The registered callable must be our internal shutdown wrapper.
|
||||
assert registrations[0] is lsp_module._atexit_shutdown
|
||||
|
||||
|
||||
def test_atexit_shutdown_calls_shutdown_service(monkeypatch):
|
||||
"""The atexit-registered wrapper invokes ``shutdown_service`` and
|
||||
swallows any exception — by the time atexit fires, the user has
|
||||
already seen the response and a noisy traceback would be clutter."""
|
||||
called = []
|
||||
monkeypatch.setattr(
|
||||
lsp_module, "shutdown_service", lambda: called.append("shutdown")
|
||||
)
|
||||
lsp_module._atexit_shutdown()
|
||||
assert called == ["shutdown"]
|
||||
|
||||
|
||||
def test_atexit_shutdown_swallows_exceptions(monkeypatch):
|
||||
def boom():
|
||||
raise RuntimeError("server already dead")
|
||||
|
||||
monkeypatch.setattr(lsp_module, "shutdown_service", boom)
|
||||
# Must not raise.
|
||||
lsp_module._atexit_shutdown()
|
||||
|
||||
|
||||
def test_shutdown_service_idempotent(monkeypatch):
|
||||
"""Calling shutdown twice must be safe — first call cleans up,
|
||||
second call no-ops (nothing to shut down)."""
|
||||
fake_svc = MagicMock()
|
||||
fake_svc.is_active.return_value = True
|
||||
fake_svc.shutdown = MagicMock()
|
||||
monkeypatch.setattr(
|
||||
lsp_module.LSPService, "create_from_config", classmethod(lambda cls: fake_svc)
|
||||
)
|
||||
monkeypatch.setattr(atexit, "register", lambda fn: None)
|
||||
|
||||
lsp_module.get_service()
|
||||
lsp_module.shutdown_service()
|
||||
lsp_module.shutdown_service() # must not raise
|
||||
|
||||
assert fake_svc.shutdown.call_count == 1
|
||||
|
||||
|
||||
def test_shutdown_service_no_op_when_never_started():
|
||||
"""Calling shutdown without ever creating the service is safe."""
|
||||
lsp_module.shutdown_service() # must not raise
|
||||
|
||||
|
||||
def test_shutdown_service_swallows_exception(monkeypatch):
|
||||
"""An exception during ``svc.shutdown()`` must not propagate —
|
||||
the caller (often atexit) has nothing useful to do with it."""
|
||||
fake_svc = MagicMock()
|
||||
fake_svc.is_active.return_value = True
|
||||
fake_svc.shutdown = MagicMock(side_effect=RuntimeError("kill -9 already"))
|
||||
monkeypatch.setattr(
|
||||
lsp_module.LSPService, "create_from_config", classmethod(lambda cls: fake_svc)
|
||||
)
|
||||
monkeypatch.setattr(atexit, "register", lambda fn: None)
|
||||
|
||||
lsp_module.get_service()
|
||||
lsp_module.shutdown_service() # must not raise
|
||||
|
||||
|
||||
def test_get_service_returns_none_for_inactive_service(monkeypatch):
|
||||
"""A service whose ``is_active()`` returns False is treated as
|
||||
not running — callers see ``None`` and fall back."""
|
||||
fake_svc = MagicMock()
|
||||
fake_svc.is_active.return_value = False
|
||||
monkeypatch.setattr(
|
||||
lsp_module.LSPService, "create_from_config", classmethod(lambda cls: fake_svc)
|
||||
)
|
||||
monkeypatch.setattr(atexit, "register", lambda fn: None)
|
||||
|
||||
assert lsp_module.get_service() is None
|
||||
# Subsequent call returns None too — but the inactive instance is
|
||||
# cached so we don't re-build it on every check.
|
||||
assert lsp_module.get_service() is None
|
||||
|
||||
|
||||
def test_get_service_returns_none_when_create_fails(monkeypatch):
|
||||
"""Service factory returning ``None`` (no config, etc.) propagates."""
|
||||
monkeypatch.setattr(
|
||||
lsp_module.LSPService, "create_from_config", classmethod(lambda cls: None)
|
||||
)
|
||||
monkeypatch.setattr(atexit, "register", lambda fn: None)
|
||||
|
||||
assert lsp_module.get_service() is None
|
||||
@@ -0,0 +1,197 @@
|
||||
"""Tests for the LSP protocol framing layer.
|
||||
|
||||
The framer is small but load-bearing — Content-Length parsing is the
|
||||
single most common reason for hand-rolled LSP clients to silently
|
||||
deadlock. These tests exercise:
|
||||
|
||||
- exact wire format of outgoing messages (encode_message)
|
||||
- partial-read tolerance + EOF handling (read_message)
|
||||
- envelope helpers (request, response, notification, error)
|
||||
- message classification
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import json
|
||||
import pytest
|
||||
|
||||
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,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# encode_message
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_encode_message_uses_compact_separators_and_utf8():
|
||||
msg = {"jsonrpc": "2.0", "id": 1, "method": "x", "params": {"k": "ä"}}
|
||||
out = encode_message(msg)
|
||||
# Header is plain ASCII Content-Length CRLF CRLF
|
||||
header_end = out.index(b"\r\n\r\n") + 4
|
||||
header = out[:header_end].decode("ascii")
|
||||
body = out[header_end:]
|
||||
assert "Content-Length:" in header
|
||||
declared = int(header.split("Content-Length:")[1].split("\r\n")[0].strip())
|
||||
# Declared length must equal actual body bytes.
|
||||
assert declared == len(body)
|
||||
# Body parses as JSON and round-trips.
|
||||
parsed = json.loads(body.decode("utf-8"))
|
||||
assert parsed == msg
|
||||
# Body uses compact separators (no spaces between kv).
|
||||
assert b'"id":1' in body
|
||||
|
||||
|
||||
def test_encode_message_handles_unicode_in_strings():
|
||||
msg = {"jsonrpc": "2.0", "method": "log", "params": {"text": "🚀 ünıcödé"}}
|
||||
out = encode_message(msg)
|
||||
header_end = out.index(b"\r\n\r\n") + 4
|
||||
declared = int(out[: out.index(b"\r\n")].split(b": ")[1])
|
||||
assert declared == len(out[header_end:])
|
||||
assert json.loads(out[header_end:].decode("utf-8")) == msg
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# read_message
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
async def _stream_from_bytes(data: bytes) -> asyncio.StreamReader:
|
||||
"""Build an asyncio.StreamReader pre-populated with ``data``."""
|
||||
reader = asyncio.StreamReader()
|
||||
reader.feed_data(data)
|
||||
reader.feed_eof()
|
||||
return reader
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_read_message_round_trip():
|
||||
msg = {"jsonrpc": "2.0", "method": "ping"}
|
||||
reader = await _stream_from_bytes(encode_message(msg))
|
||||
parsed = await read_message(reader)
|
||||
assert parsed == msg
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_read_message_clean_eof_returns_none():
|
||||
reader = await _stream_from_bytes(b"")
|
||||
assert await read_message(reader) is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_read_message_truncated_body_raises():
|
||||
msg = encode_message({"jsonrpc": "2.0", "method": "x"})
|
||||
truncated = msg[: -3] # cut the body
|
||||
reader = await _stream_from_bytes(truncated)
|
||||
with pytest.raises(LSPProtocolError):
|
||||
await read_message(reader)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_read_message_missing_content_length_raises():
|
||||
bad = b"X-Other: 5\r\n\r\n12345"
|
||||
reader = await _stream_from_bytes(bad)
|
||||
with pytest.raises(LSPProtocolError):
|
||||
await read_message(reader)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_read_message_two_messages_back_to_back():
|
||||
a = encode_message({"jsonrpc": "2.0", "method": "a"})
|
||||
b = encode_message({"jsonrpc": "2.0", "method": "b"})
|
||||
reader = await _stream_from_bytes(a + b)
|
||||
assert (await read_message(reader))["method"] == "a"
|
||||
assert (await read_message(reader))["method"] == "b"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_read_message_rejects_runaway_header():
|
||||
"""A pathological server that streams headers without ever emitting
|
||||
the CRLF-CRLF terminator must not loop forever — the 8 KiB cap kicks
|
||||
in and surfaces a protocol error."""
|
||||
flood = (b"X-Junk: " + b"A" * 200 + b"\r\n") * 60 # ~12 KiB worth
|
||||
reader = await _stream_from_bytes(flood)
|
||||
with pytest.raises(LSPProtocolError) as exc:
|
||||
await read_message(reader)
|
||||
assert "8 KiB" in str(exc.value)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# envelope helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_make_request_includes_id_and_method():
|
||||
msg = make_request(7, "ping", {"v": 1})
|
||||
assert msg == {"jsonrpc": "2.0", "id": 7, "method": "ping", "params": {"v": 1}}
|
||||
|
||||
|
||||
def test_make_request_omits_params_when_none():
|
||||
msg = make_request(7, "ping", None)
|
||||
assert "params" not in msg
|
||||
|
||||
|
||||
def test_make_notification_omits_id():
|
||||
msg = make_notification("log", {"line": "hi"})
|
||||
assert "id" not in msg
|
||||
assert msg["method"] == "log"
|
||||
|
||||
|
||||
def test_make_response_carries_result():
|
||||
msg = make_response(7, {"ok": True})
|
||||
assert msg["id"] == 7 and msg["result"] == {"ok": True}
|
||||
|
||||
|
||||
def test_make_error_response_shape():
|
||||
msg = make_error_response(7, ERROR_CONTENT_MODIFIED, "stale", {"hint": "retry"})
|
||||
assert msg["error"]["code"] == ERROR_CONTENT_MODIFIED
|
||||
assert msg["error"]["message"] == "stale"
|
||||
assert msg["error"]["data"] == {"hint": "retry"}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# classify_message
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_classify_message_request():
|
||||
msg = {"jsonrpc": "2.0", "id": 1, "method": "x"}
|
||||
assert classify_message(msg) == ("request", 1)
|
||||
|
||||
|
||||
def test_classify_message_response():
|
||||
msg = {"jsonrpc": "2.0", "id": 1, "result": None}
|
||||
assert classify_message(msg) == ("response", 1)
|
||||
|
||||
|
||||
def test_classify_message_notification():
|
||||
msg = {"jsonrpc": "2.0", "method": "log"}
|
||||
assert classify_message(msg) == ("notification", "log")
|
||||
|
||||
|
||||
def test_classify_message_invalid():
|
||||
assert classify_message({"id": 1})[0] == "invalid"
|
||||
assert classify_message({"jsonrpc": "1.0", "method": "x"})[0] == "invalid"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# LSPRequestError
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_lsp_request_error_carries_code_and_data():
|
||||
e = LSPRequestError(ERROR_METHOD_NOT_FOUND, "no", {"x": 1})
|
||||
assert e.code == ERROR_METHOD_NOT_FOUND
|
||||
assert e.message == "no"
|
||||
assert e.data == {"x": 1}
|
||||
@@ -0,0 +1,94 @@
|
||||
"""Tests for the diagnostic reporter (formatting layer)."""
|
||||
from __future__ import annotations
|
||||
|
||||
from agent.lsp.reporter import (
|
||||
DEFAULT_SEVERITIES,
|
||||
MAX_PER_FILE,
|
||||
format_diagnostic,
|
||||
report_for_file,
|
||||
truncate,
|
||||
)
|
||||
|
||||
|
||||
def _diag(line=0, col=0, sev=1, code="E001", source="ls", msg="oops"):
|
||||
return {
|
||||
"range": {
|
||||
"start": {"line": line, "character": col},
|
||||
"end": {"line": line, "character": col + 1},
|
||||
},
|
||||
"severity": sev,
|
||||
"code": code,
|
||||
"source": source,
|
||||
"message": msg,
|
||||
}
|
||||
|
||||
|
||||
def test_format_diagnostic_uses_one_indexed_position():
|
||||
line = format_diagnostic(_diag(line=4, col=2))
|
||||
assert "[5:3]" in line # +1 on both
|
||||
|
||||
|
||||
def test_format_diagnostic_includes_severity_label():
|
||||
assert format_diagnostic(_diag(sev=1)).startswith("ERROR")
|
||||
assert format_diagnostic(_diag(sev=2)).startswith("WARN")
|
||||
assert format_diagnostic(_diag(sev=3)).startswith("INFO")
|
||||
assert format_diagnostic(_diag(sev=4)).startswith("HINT")
|
||||
|
||||
|
||||
def test_format_diagnostic_includes_code_and_source():
|
||||
line = format_diagnostic(_diag(code="X42", source="src"))
|
||||
assert "[X42]" in line
|
||||
assert "(src)" in line
|
||||
|
||||
|
||||
def test_format_diagnostic_omits_missing_optional_fields():
|
||||
line = format_diagnostic(
|
||||
{
|
||||
"range": {
|
||||
"start": {"line": 0, "character": 0},
|
||||
"end": {"line": 0, "character": 0},
|
||||
},
|
||||
"severity": 1,
|
||||
"message": "bare",
|
||||
}
|
||||
)
|
||||
assert "[" not in line.split("]", 1)[1] # no extra brackets after the position
|
||||
assert "(" not in line
|
||||
|
||||
|
||||
def test_report_for_file_returns_empty_when_only_warnings():
|
||||
"""Default severity filter is ERROR-only."""
|
||||
report = report_for_file("/x.py", [_diag(sev=2)])
|
||||
assert report == ""
|
||||
|
||||
|
||||
def test_report_for_file_emits_block_with_errors():
|
||||
diag = _diag(msg="real error")
|
||||
report = report_for_file("/x.py", [diag])
|
||||
assert "<diagnostics file=\"/x.py\">" in report
|
||||
assert "real error" in report
|
||||
assert "</diagnostics>" in report
|
||||
|
||||
|
||||
def test_report_for_file_caps_at_max_per_file():
|
||||
diags = [_diag(line=i) for i in range(MAX_PER_FILE + 5)]
|
||||
report = report_for_file("/x.py", diags)
|
||||
assert "and 5 more" in report
|
||||
|
||||
|
||||
def test_report_for_file_respects_custom_severities():
|
||||
diag = _diag(sev=2, msg="warn")
|
||||
report = report_for_file("/x.py", [diag], severities=frozenset({1, 2}))
|
||||
assert "warn" in report
|
||||
|
||||
|
||||
def test_truncate_below_limit_unchanged():
|
||||
s = "abc" * 100
|
||||
assert truncate(s, limit=4000) == s
|
||||
|
||||
|
||||
def test_truncate_above_limit_appends_marker():
|
||||
s = "x" * 10000
|
||||
out = truncate(s, limit=200)
|
||||
assert out.endswith("[truncated]")
|
||||
assert len(out) <= 200
|
||||
@@ -0,0 +1,149 @@
|
||||
"""Tests for the synchronous LSPService wrapper.
|
||||
|
||||
Drives the service through ``snapshot_baseline`` →
|
||||
``get_diagnostics_sync`` against the mock LSP server, exercising the
|
||||
delta filter that ``tools/file_operations._check_lint_delta`` relies
|
||||
on.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from agent.lsp.manager import LSPService
|
||||
from agent.lsp.servers import (
|
||||
SERVERS,
|
||||
ServerContext,
|
||||
ServerDef,
|
||||
SpawnSpec,
|
||||
find_server_for_file,
|
||||
)
|
||||
|
||||
|
||||
MOCK_SERVER = str(Path(__file__).parent / "_mock_lsp_server.py")
|
||||
|
||||
|
||||
def _install_mock_server(monkeypatch, script: str = "errors", server_id: str = "pyright"):
|
||||
"""Replace one registered server with a wrapper that spawns the mock.
|
||||
|
||||
We reuse ``pyright`` so .py files route to it. This keeps the
|
||||
test free of any LSP toolchain dependency.
|
||||
"""
|
||||
target_index = next(i for i, s in enumerate(SERVERS) if s.server_id == server_id)
|
||||
original = SERVERS[target_index]
|
||||
|
||||
def _spawn(root: str, ctx: ServerContext) -> SpawnSpec:
|
||||
env = {"MOCK_LSP_SCRIPT": script}
|
||||
return SpawnSpec(
|
||||
command=[sys.executable, MOCK_SERVER],
|
||||
workspace_root=root,
|
||||
cwd=root,
|
||||
env=env,
|
||||
initialization_options={},
|
||||
)
|
||||
|
||||
replacement = ServerDef(
|
||||
server_id=server_id,
|
||||
extensions=original.extensions,
|
||||
resolve_root=lambda fp, ws: ws, # always use workspace root
|
||||
build_spawn=_spawn,
|
||||
seed_first_push=False,
|
||||
description="mock " + server_id,
|
||||
)
|
||||
# Patch the SERVERS list element directly + restore on teardown.
|
||||
SERVERS[target_index] = replacement
|
||||
|
||||
yield
|
||||
|
||||
SERVERS[target_index] = original
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mock_pyright(monkeypatch, tmp_path):
|
||||
"""Install the mock as ``pyright`` and create a fake git workspace."""
|
||||
repo = tmp_path / "repo"
|
||||
repo.mkdir()
|
||||
(repo / ".git").mkdir()
|
||||
(repo / "pyproject.toml").write_text("") # so pyright's root resolver finds it
|
||||
monkeypatch.chdir(str(repo))
|
||||
gen = _install_mock_server(monkeypatch, "errors", "pyright")
|
||||
next(gen)
|
||||
yield repo
|
||||
try:
|
||||
next(gen)
|
||||
except StopIteration:
|
||||
pass
|
||||
|
||||
|
||||
def test_service_returns_empty_when_disabled(tmp_path):
|
||||
svc = LSPService(
|
||||
enabled=False,
|
||||
wait_mode="document",
|
||||
wait_timeout=2.0,
|
||||
install_strategy="auto",
|
||||
)
|
||||
assert not svc.is_active()
|
||||
f = tmp_path / "x.py"
|
||||
f.write_text("")
|
||||
assert svc.get_diagnostics_sync(str(f)) == []
|
||||
svc.shutdown()
|
||||
|
||||
|
||||
def test_service_skips_files_outside_workspace(tmp_path):
|
||||
"""Files outside any git worktree must not trigger LSP."""
|
||||
svc = LSPService(
|
||||
enabled=True,
|
||||
wait_mode="document",
|
||||
wait_timeout=2.0,
|
||||
install_strategy="manual",
|
||||
)
|
||||
f = tmp_path / "x.py"
|
||||
f.write_text("")
|
||||
# No .git anywhere — service should report not enabled for this file.
|
||||
assert not svc.enabled_for(str(f))
|
||||
svc.shutdown()
|
||||
|
||||
|
||||
def test_service_e2e_delta_filter(mock_pyright):
|
||||
"""End-to-end: snapshot baseline → wait → delta returned."""
|
||||
repo = mock_pyright
|
||||
f = repo / "x.py"
|
||||
f.write_text("print('hi')\n")
|
||||
|
||||
svc = LSPService(
|
||||
enabled=True,
|
||||
wait_mode="document",
|
||||
wait_timeout=3.0,
|
||||
install_strategy="manual",
|
||||
)
|
||||
try:
|
||||
assert svc.enabled_for(str(f))
|
||||
# Baseline first — server pushes 1 error.
|
||||
svc.snapshot_baseline(str(f))
|
||||
# Re-poll: same error is in baseline, so delta is empty.
|
||||
new_diags = svc.get_diagnostics_sync(str(f))
|
||||
assert new_diags == []
|
||||
finally:
|
||||
svc.shutdown()
|
||||
|
||||
|
||||
def test_service_status_includes_clients(mock_pyright):
|
||||
repo = mock_pyright
|
||||
f = repo / "x.py"
|
||||
f.write_text("")
|
||||
svc = LSPService(
|
||||
enabled=True,
|
||||
wait_mode="document",
|
||||
wait_timeout=3.0,
|
||||
install_strategy="manual",
|
||||
)
|
||||
try:
|
||||
svc.get_diagnostics_sync(str(f))
|
||||
info = svc.get_status()
|
||||
assert info["enabled"] is True
|
||||
assert any(c["server_id"] == "pyright" for c in info["clients"])
|
||||
finally:
|
||||
svc.shutdown()
|
||||
@@ -0,0 +1,139 @@
|
||||
"""Tests for workspace + project-root resolution."""
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from agent.lsp.workspace import (
|
||||
clear_cache,
|
||||
find_git_worktree,
|
||||
is_inside_workspace,
|
||||
nearest_root,
|
||||
normalize_path,
|
||||
resolve_workspace_for_file,
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _clear():
|
||||
clear_cache()
|
||||
yield
|
||||
clear_cache()
|
||||
|
||||
|
||||
def test_find_git_worktree_returns_none_outside_repo(tmp_path: Path):
|
||||
sub = tmp_path / "sub"
|
||||
sub.mkdir()
|
||||
assert find_git_worktree(str(sub)) is None
|
||||
|
||||
|
||||
def test_find_git_worktree_finds_dotgit(tmp_path: Path):
|
||||
repo = tmp_path / "repo"
|
||||
repo.mkdir()
|
||||
(repo / ".git").mkdir()
|
||||
sub = repo / "src" / "deep"
|
||||
sub.mkdir(parents=True)
|
||||
assert find_git_worktree(str(sub)) == str(repo)
|
||||
|
||||
|
||||
def test_find_git_worktree_handles_dotgit_file(tmp_path: Path):
|
||||
"""``.git`` can also be a file (gitfile pointing into a worktree)."""
|
||||
repo = tmp_path / "repo"
|
||||
repo.mkdir()
|
||||
(repo / ".git").write_text("gitdir: /elsewhere\n")
|
||||
assert find_git_worktree(str(repo)) == str(repo)
|
||||
|
||||
|
||||
def test_is_inside_workspace_true_for_subpath(tmp_path: Path):
|
||||
root = tmp_path / "p"
|
||||
root.mkdir()
|
||||
sub = root / "x" / "y.py"
|
||||
sub.parent.mkdir(parents=True)
|
||||
sub.write_text("")
|
||||
assert is_inside_workspace(str(sub), str(root))
|
||||
|
||||
|
||||
def test_is_inside_workspace_false_for_unrelated(tmp_path: Path):
|
||||
a = tmp_path / "a"
|
||||
b = tmp_path / "b"
|
||||
a.mkdir()
|
||||
b.mkdir()
|
||||
f = b / "x.py"
|
||||
f.write_text("")
|
||||
assert not is_inside_workspace(str(f), str(a))
|
||||
|
||||
|
||||
def test_nearest_root_finds_first_marker(tmp_path: Path):
|
||||
root = tmp_path / "p"
|
||||
deep = root / "src" / "pkg"
|
||||
deep.mkdir(parents=True)
|
||||
(root / "pyproject.toml").write_text("")
|
||||
found = nearest_root(str(deep / "mod.py"), ["pyproject.toml"])
|
||||
assert found == str(root)
|
||||
|
||||
|
||||
def test_nearest_root_excludes_take_priority(tmp_path: Path):
|
||||
"""If an exclude marker matches first, return None."""
|
||||
root = tmp_path / "p"
|
||||
sub = root / "deno-app"
|
||||
sub.mkdir(parents=True)
|
||||
(sub / "deno.json").write_text("{}")
|
||||
(root / "package.json").write_text("{}") # would match if not for exclude
|
||||
found = nearest_root(
|
||||
str(sub / "main.ts"),
|
||||
["package.json"],
|
||||
excludes=["deno.json"],
|
||||
)
|
||||
assert found is None
|
||||
|
||||
|
||||
def test_nearest_root_returns_none_when_no_marker(tmp_path: Path):
|
||||
f = tmp_path / "x.py"
|
||||
f.write_text("")
|
||||
assert nearest_root(str(f), ["pyproject.toml"]) is None
|
||||
|
||||
|
||||
def test_resolve_workspace_for_file_uses_cwd_first(tmp_path: Path, monkeypatch):
|
||||
repo = tmp_path / "repo"
|
||||
(repo / ".git").mkdir(parents=True)
|
||||
file_path = repo / "x.py"
|
||||
file_path.write_text("")
|
||||
# cwd is inside the repo
|
||||
monkeypatch.chdir(str(repo))
|
||||
root, gated = resolve_workspace_for_file(str(file_path))
|
||||
assert root == str(repo)
|
||||
assert gated is True
|
||||
|
||||
|
||||
def test_resolve_workspace_for_file_no_repo_returns_none(tmp_path: Path, monkeypatch):
|
||||
monkeypatch.chdir(str(tmp_path))
|
||||
f = tmp_path / "x.py"
|
||||
f.write_text("")
|
||||
root, gated = resolve_workspace_for_file(str(f))
|
||||
assert root is None
|
||||
assert gated is False
|
||||
|
||||
|
||||
def test_resolve_workspace_falls_back_to_file_location(tmp_path: Path, monkeypatch):
|
||||
"""When cwd isn't a git repo but the file is inside one, we still
|
||||
discover the workspace from the file's path."""
|
||||
not_a_repo = tmp_path / "loose"
|
||||
not_a_repo.mkdir()
|
||||
monkeypatch.chdir(str(not_a_repo))
|
||||
|
||||
repo = tmp_path / "actual-repo"
|
||||
(repo / ".git").mkdir(parents=True)
|
||||
f = repo / "x.py"
|
||||
f.write_text("")
|
||||
|
||||
root, gated = resolve_workspace_for_file(str(f))
|
||||
assert root == str(repo)
|
||||
assert gated is True
|
||||
|
||||
|
||||
def test_normalize_path_expands_tilde(monkeypatch):
|
||||
monkeypatch.setenv("HOME", "/home/user")
|
||||
p = normalize_path("~/x.py")
|
||||
assert p == os.path.abspath("/home/user/x.py")
|
||||
@@ -660,6 +660,7 @@ class TestAuxiliaryPoolAwareness:
|
||||
with (
|
||||
patch("agent.auxiliary_client.load_pool", return_value=_Pool()),
|
||||
patch("agent.auxiliary_client.OpenAI") as mock_openai,
|
||||
patch("hermes_cli.models.get_nous_recommended_aux_model", return_value=None),
|
||||
):
|
||||
from agent.auxiliary_client import _try_nous
|
||||
|
||||
|
||||
@@ -473,6 +473,240 @@ class TestCodexOAuthContextLength:
|
||||
assert ctx == 1_000_000, "Non-codex 1M cache entries must be respected"
|
||||
|
||||
|
||||
# =========================================================================
|
||||
# Nous Portal context-window resolution (provider="nous")
|
||||
# =========================================================================
|
||||
|
||||
class TestNousPortalContextResolution:
|
||||
"""Nous Portal /v1/models is authoritative for what Nous infra enforces
|
||||
and may diverge from the OpenRouter catalog.
|
||||
|
||||
Invariants this class pins down:
|
||||
1. Portal value wins over the OR fallback.
|
||||
2. Portal-derived values are persisted to disk.
|
||||
3. OR-fallback values are NEVER persisted — otherwise a single portal
|
||||
blip would freeze the wrong value in via step-1 cache short-circuit.
|
||||
4. Pre-fix persistent-cache entries (seeded from the OR catalog) are
|
||||
bypassed at step 1 and overwritten once the portal responds.
|
||||
5. Pre-fix persistent-cache entries SURVIVE on disk when the portal
|
||||
is unreachable — no opportunistic invalidation that loses the only
|
||||
value we have.
|
||||
"""
|
||||
|
||||
def setup_method(self):
|
||||
import agent.model_metadata as mm
|
||||
mm._endpoint_model_metadata_cache.clear()
|
||||
mm._endpoint_model_metadata_cache_time.clear()
|
||||
|
||||
@patch("agent.model_metadata.fetch_endpoint_model_metadata")
|
||||
@patch("agent.model_metadata.fetch_model_metadata")
|
||||
def test_portal_value_wins_over_openrouter_catalog(
|
||||
self, mock_or, mock_portal, tmp_path, monkeypatch
|
||||
):
|
||||
"""The motivating case: OR catalog says 1M for qwen3.6-plus, but
|
||||
the Nous portal correctly enforces 262144. Portal must win."""
|
||||
import agent.model_metadata as mm
|
||||
cache_file = tmp_path / "context_length_cache.yaml"
|
||||
monkeypatch.setattr(mm, "_get_context_cache_path", lambda: cache_file)
|
||||
|
||||
mock_portal.return_value = {
|
||||
"qwen3.6-plus": {"context_length": 262_144},
|
||||
}
|
||||
mock_or.return_value = {
|
||||
"qwen/qwen3.6-plus": {"context_length": 1_000_000},
|
||||
}
|
||||
|
||||
ctx = mm.get_model_context_length(
|
||||
model="qwen3.6-plus",
|
||||
base_url="https://inference-api.nousresearch.com/v1",
|
||||
api_key="fake-token",
|
||||
provider="nous",
|
||||
)
|
||||
assert ctx == 262_144, (
|
||||
f"Portal must override OR catalog; got {ctx} (OR leak?)"
|
||||
)
|
||||
|
||||
@patch("agent.model_metadata.fetch_endpoint_model_metadata")
|
||||
@patch("agent.model_metadata.fetch_model_metadata")
|
||||
def test_portal_value_is_persisted_to_disk(
|
||||
self, mock_or, mock_portal, tmp_path, monkeypatch
|
||||
):
|
||||
"""Portal-derived value should land in the persistent cache so
|
||||
cross-process callers (e.g. child agents) see the same value."""
|
||||
import agent.model_metadata as mm
|
||||
cache_file = tmp_path / "context_length_cache.yaml"
|
||||
monkeypatch.setattr(mm, "_get_context_cache_path", lambda: cache_file)
|
||||
|
||||
mock_portal.return_value = {
|
||||
"qwen3.6-plus": {"context_length": 262_144},
|
||||
}
|
||||
mock_or.return_value = {}
|
||||
|
||||
base_url = "https://inference-api.nousresearch.com/v1"
|
||||
ctx = mm.get_model_context_length(
|
||||
model="qwen3.6-plus",
|
||||
base_url=base_url,
|
||||
api_key="fake",
|
||||
provider="nous",
|
||||
)
|
||||
assert ctx == 262_144
|
||||
persisted = yaml.safe_load(cache_file.read_text()).get("context_lengths", {})
|
||||
assert persisted.get(f"qwen3.6-plus@{base_url}") == 262_144, (
|
||||
"Portal-derived value should be persisted to disk"
|
||||
)
|
||||
|
||||
@patch("agent.model_metadata.fetch_endpoint_model_metadata")
|
||||
@patch("agent.model_metadata.fetch_model_metadata")
|
||||
def test_openrouter_fallback_is_not_persisted(
|
||||
self, mock_or, mock_portal, tmp_path, monkeypatch
|
||||
):
|
||||
"""When the portal can't resolve a model (network blip, auth glitch,
|
||||
model not yet listed) we fall back to the OR catalog so the agent
|
||||
keeps working — but we must NOT write the OR value to disk. Once
|
||||
cached on disk, step-1 short-circuits forever and the user is stuck
|
||||
with the wrong number until they manually clear the cache."""
|
||||
import agent.model_metadata as mm
|
||||
cache_file = tmp_path / "context_length_cache.yaml"
|
||||
monkeypatch.setattr(mm, "_get_context_cache_path", lambda: cache_file)
|
||||
|
||||
mock_portal.return_value = {} # portal unreachable / model unknown
|
||||
mock_or.return_value = {
|
||||
"qwen/qwen3.6-plus": {"context_length": 1_000_000},
|
||||
}
|
||||
|
||||
base_url = "https://inference-api.nousresearch.com/v1"
|
||||
ctx = mm.get_model_context_length(
|
||||
model="qwen3.6-plus",
|
||||
base_url=base_url,
|
||||
api_key="fake",
|
||||
provider="nous",
|
||||
)
|
||||
assert ctx == 1_000_000, "OR fallback should still serve the request"
|
||||
assert not cache_file.exists() or not yaml.safe_load(
|
||||
cache_file.read_text()
|
||||
).get("context_lengths", {}), (
|
||||
"OR-fallback values must NOT be persisted — a single portal blip "
|
||||
"would otherwise freeze the wrong value in via step-1 cache hit"
|
||||
)
|
||||
|
||||
@patch("agent.model_metadata.fetch_endpoint_model_metadata")
|
||||
@patch("agent.model_metadata.fetch_model_metadata")
|
||||
def test_stale_cache_is_bypassed_and_overwritten_by_portal(
|
||||
self, mock_or, mock_portal, tmp_path, monkeypatch
|
||||
):
|
||||
"""Users upgrading from pre-fix builds have ``qwen3.6-plus@…nous… =
|
||||
1000000`` (OR-derived) sitting in their cache file. Step 1 must
|
||||
NOT short-circuit on that entry — step 5b reconciles against the
|
||||
portal and overwrites the persistent value with 262144."""
|
||||
import agent.model_metadata as mm
|
||||
cache_file = tmp_path / "context_length_cache.yaml"
|
||||
monkeypatch.setattr(mm, "_get_context_cache_path", lambda: cache_file)
|
||||
|
||||
base_url = "https://inference-api.nousresearch.com/v1"
|
||||
stale_key = f"qwen3.6-plus@{base_url}"
|
||||
other_key = "other-model@https://api.openai.com/v1"
|
||||
cache_file.write_text(yaml.dump({"context_lengths": {
|
||||
stale_key: 1_000_000, # pre-fix OR-derived value
|
||||
other_key: 128_000, # unrelated, must survive
|
||||
}}))
|
||||
|
||||
mock_portal.return_value = {
|
||||
"qwen3.6-plus": {"context_length": 262_144},
|
||||
}
|
||||
mock_or.return_value = {}
|
||||
|
||||
ctx = mm.get_model_context_length(
|
||||
model="qwen3.6-plus",
|
||||
base_url=base_url,
|
||||
api_key="fake",
|
||||
provider="nous",
|
||||
)
|
||||
assert ctx == 262_144, (
|
||||
f"Stale OR-derived cache entry should not have leaked through; got {ctx}"
|
||||
)
|
||||
|
||||
remaining = yaml.safe_load(cache_file.read_text()).get("context_lengths", {})
|
||||
assert remaining.get(stale_key) == 262_144, (
|
||||
"Portal value should have overwritten the stale entry on disk"
|
||||
)
|
||||
assert remaining.get(other_key) == 128_000, (
|
||||
"Unrelated cache entries must not be touched"
|
||||
)
|
||||
|
||||
@patch("agent.model_metadata.fetch_endpoint_model_metadata")
|
||||
@patch("agent.model_metadata.fetch_model_metadata")
|
||||
def test_stale_cache_survives_when_portal_unreachable(
|
||||
self, mock_or, mock_portal, tmp_path, monkeypatch
|
||||
):
|
||||
"""When the portal is unreachable AND we have a (potentially stale)
|
||||
on-disk cache entry, the entry must survive untouched — we don't
|
||||
want a transient outage to delete the only value we have. The
|
||||
request itself still gets served via OR fallback for this call."""
|
||||
import agent.model_metadata as mm
|
||||
cache_file = tmp_path / "context_length_cache.yaml"
|
||||
monkeypatch.setattr(mm, "_get_context_cache_path", lambda: cache_file)
|
||||
|
||||
base_url = "https://inference-api.nousresearch.com/v1"
|
||||
existing_key = f"qwen3.6-plus@{base_url}"
|
||||
cache_file.write_text(yaml.dump({"context_lengths": {
|
||||
existing_key: 1_000_000,
|
||||
}}))
|
||||
|
||||
mock_portal.return_value = {} # portal unreachable
|
||||
mock_or.return_value = {
|
||||
"qwen/qwen3.6-plus": {"context_length": 1_000_000},
|
||||
}
|
||||
|
||||
mm.get_model_context_length(
|
||||
model="qwen3.6-plus",
|
||||
base_url=base_url,
|
||||
api_key="fake",
|
||||
provider="nous",
|
||||
)
|
||||
|
||||
remaining = yaml.safe_load(cache_file.read_text()).get("context_lengths", {})
|
||||
assert remaining.get(existing_key) == 1_000_000, (
|
||||
"Persistent cache entry must survive a transient portal outage"
|
||||
)
|
||||
|
||||
@patch("agent.model_metadata.fetch_endpoint_model_metadata")
|
||||
@patch("agent.model_metadata.fetch_model_metadata")
|
||||
def test_bypass_keyed_on_url_not_provider_string(
|
||||
self, mock_or, mock_portal, tmp_path, monkeypatch
|
||||
):
|
||||
"""Some call sites pass ``provider=""`` or ``provider="openrouter"``
|
||||
when the user is really on Nous Portal (e.g. cred-pool fallback).
|
||||
The Nous-URL bypass must trigger off the URL host, not the provider
|
||||
string, so the portal-first resolver still runs in that case."""
|
||||
import agent.model_metadata as mm
|
||||
cache_file = tmp_path / "context_length_cache.yaml"
|
||||
monkeypatch.setattr(mm, "_get_context_cache_path", lambda: cache_file)
|
||||
|
||||
base_url = "https://inference-api.nousresearch.com/v1"
|
||||
cache_file.write_text(yaml.dump({"context_lengths": {
|
||||
f"qwen3.6-plus@{base_url}": 1_000_000, # stale
|
||||
}}))
|
||||
|
||||
mock_portal.return_value = {
|
||||
"qwen3.6-plus": {"context_length": 262_144},
|
||||
}
|
||||
mock_or.return_value = {}
|
||||
|
||||
for provider_arg in ("", "openrouter", "custom"):
|
||||
mm._endpoint_model_metadata_cache.clear()
|
||||
mm._endpoint_model_metadata_cache_time.clear()
|
||||
ctx = mm.get_model_context_length(
|
||||
model="qwen3.6-plus",
|
||||
base_url=base_url,
|
||||
api_key="fake",
|
||||
provider=provider_arg,
|
||||
)
|
||||
assert ctx == 262_144, (
|
||||
f"URL-based Nous detection must fire for provider={provider_arg!r}; "
|
||||
f"got {ctx}"
|
||||
)
|
||||
|
||||
|
||||
# =========================================================================
|
||||
# get_model_context_length — resolution order
|
||||
# =========================================================================
|
||||
|
||||
@@ -190,3 +190,37 @@ def test_custom_endpoint_models_api_pricing_is_supported(monkeypatch):
|
||||
|
||||
assert float(entry.input_cost_per_million) == 0.5
|
||||
assert float(entry.output_cost_per_million) == 2.0
|
||||
|
||||
|
||||
def test_deepseek_v4_pro_pricing_entry_exists():
|
||||
"""Regression test: deepseek-v4-pro must have a pricing entry.
|
||||
|
||||
Before this fix, deepseek-v4-pro sessions showed as unknown cost
|
||||
in hermes insights because the _OFFICIAL_DOCS_PRICING table had no
|
||||
entry for that model. See #24218.
|
||||
"""
|
||||
entry = get_pricing_entry(
|
||||
"deepseek-v4-pro",
|
||||
provider="deepseek",
|
||||
)
|
||||
|
||||
assert entry is not None
|
||||
assert entry.input_cost_per_million is not None
|
||||
assert entry.output_cost_per_million is not None
|
||||
assert float(entry.input_cost_per_million) == 1.74
|
||||
assert float(entry.output_cost_per_million) == 3.48
|
||||
assert float(entry.cache_read_cost_per_million) == 0.0145
|
||||
|
||||
|
||||
def test_deepseek_v4_pro_estimate_usage_cost():
|
||||
"""Ensure deepseek-v4-pro sessions get a dollar estimate, not unknown."""
|
||||
result = estimate_usage_cost(
|
||||
"deepseek-v4-pro",
|
||||
CanonicalUsage(input_tokens=1000000, output_tokens=500000),
|
||||
provider="deepseek",
|
||||
)
|
||||
|
||||
assert result.status == "estimated"
|
||||
assert result.amount_usd is not None
|
||||
# 1M input × $1.74/M + 500K output × $3.48/M = $1.74 + $1.74 = $3.48
|
||||
assert float(result.amount_usd) == 3.48
|
||||
|
||||
Reference in New Issue
Block a user