fix(diagnostics): TSLS push capability, reason-aware fallback, PATH-independent LSP discovery

.github/workflows/ci.yml

@@ -20,15 +20,40 @@ jobs:
         with:
           python-version: ${{ matrix.python-version }}
 
-      - name: Install tree-sitter + ast-grep test deps
+      - name: Set up Node (for real LSP servers)
+        uses: actions/setup-node@v4
+        with:
+          node-version: "20"
+
+      - name: Install tree-sitter + ast-grep + LSP test deps
         run: |
           python -m pip install --upgrade pip
-          pip install tree-sitter tree-sitter-python tree-sitter-javascript tree-sitter-typescript tree-sitter-rust tree-sitter-go tree-sitter-java ast-grep-py pytest
+          pip install tree-sitter tree-sitter-python tree-sitter-javascript tree-sitter-typescript tree-sitter-rust tree-sitter-go tree-sitter-java ast-grep-py pytest pytest-timeout
+          # Real LSP servers so the live diagnostics tests actually run on CI
+          # (typescript-language-server + tsserver for TS, pyright for Python).
+          npm install -g typescript@5.8.3 typescript-language-server@5.1.3 pyright
+          LSP_BIN_DIR="$(dirname "$(command -v typescript-language-server)")"
+          test -x "$LSP_BIN_DIR/typescript-language-server"
+          test -x "$LSP_BIN_DIR/node"
+          echo "CODE_INTEL_LSP_BIN_DIRS=$LSP_BIN_DIR" >> "$GITHUB_ENV"
+          npm list -g --depth=0 || true
+          npx pyright --version || true
+          typescript-language-server --version || true
 
       - name: Compile check
         run: |
           python -m py_compile code_intel.py lsp_bridge.py __init__.py scripts/health_check.py
 
+      - name: Probe TSLS initialize handshake (diagnose silent no-reply)
+        run: |
+          node scripts/tsls_probe.js || true
+          echo "probe done"
+
       - name: Run tests
         run: |
-          python -m pytest -q
+          python -m pytest -q --timeout=120 --timeout-method=thread --maxfail=6 -p faulthandler --junitxml=pytest-result.xml
+      - name: Dump test result summary (always, for debugging hangs)
+        if: always()
+        run: |
+          tail -c 4000 pytest-result.xml || true
+          grep -o 'testcase name="[^"]*"' pytest-result.xml | tail -40 || true

.gitignore

@@ -9,3 +9,5 @@ patch_*.py
 # Self-referential symlink guard — the repo root may be symlinked into a
 # profile plugins dir; never let that link be committed or scanned recursively.
 code-intel-plugin
+# Generated diagnostics test fixtures (rewritten by ci_diag_fixtures.ensure_fixtures)
+tests/.fixtures/

README.md

@@ -155,7 +155,7 @@ pip install tree-sitter tree-sitter-languages ast-grep-py
 > go install golang.org/x/tools/gopls@latest
 > ```
 >
-> The plugin auto-discovers servers via PATH, monorepo `node_modules/.bin`, and `npx` fallback. No additional configuration needed.
+> The plugin auto-discovers servers via PATH, monorepo `node_modules/.bin`, and `npx` fallback. For service or CI launchers with a reduced PATH, set `CODE_INTEL_LSP_BIN_DIRS` to a path-separated list of verified LSP executable directories; this explicit setting is searched before generic fallback directories.
 
 ### Monorepo Support
 
@@ -425,3 +425,29 @@ Contributions welcome! This is a community plugin — PRs for new languages, bet
 - [pyright](https://github.com/microsoft/pyright) — Python LSP server (fallback)
 - [typescript-language-server](https://github.com/typescript-language-server/typescript-language-server) — TypeScript/JavaScript LSP server
 - [tsserver](https://github.com/microsoft/TypeScript) — TypeScript language service (used by typescript-language-server)
+
+## Durability & Revision Pinning
+
+The nightly maintenance gate needs a **source-controlled known-good revision**
+of this plugin. The pin lives in the scripts repo
+(`~/.hermes/scripts/code_intel_plugin_manifest.json`, fields: `pin_revision`,
+`branch`, `remote`, `minimum_compatible`).
+
+`~/.hermes/scripts/code_intel_plugin_restore.py` verifies the live checkout
+against that pin. It is read-only by default and never touches foreign WIP:
+
+| Live checkout state | Action |
+|---|---|
+| clean, matches pin | OK |
+| clean, newer healthy revision (descendant of pin or listed in `minimum_compatible`) | OK (no downgrade) |
+| dirty | pin-revision content verified independently (`git show pin:path`); WIP kept untouched |
+| checkout absent | restore into a disposable fixture dir from the pushed remote; live location never modified |
+| conflicting/drifted | fail visibly with remediation hints; WIP never destroyed |
+
+The health checker resolves the authoritative loaded plugin source for its
+diagnostics fixtures: the profile plugin symlink target when it resolves to the
+live repo, else the pinned revision rendered into a disposable fixture dir.
+A symlink alone is never sufficient — actual bytes/behavior are validated.
+
+Manual: `code_intel_plugin_restore.py [--json] [--restore]` (`--restore`
+creates a disposable recovery fixture, nothing else).

lsp_bridge.py

@@ -57,10 +57,11 @@
 
 # Maximum time (seconds) to wait for the server to start and respond to
 # the ``initialize`` handshake.
-# Reduced from 60 → 15: if a server can't init in 15s (warm or cold),
-# it's likely blocked on something (stderr pipe, plugin init, etc.).
-# A 60s timeout just makes Hermes stall for a full minute.
-_LSP_INIT_TIMEOUT = 15
+# 30s: CI runners cold-start tsserver/pyright from network caches; 15s made
+# the *first* initialize time out, which then deadlocked in shutdown()
+# (see RLock note at _init_lock) and stalled the whole suite. 30s covers a
+# cold server; locally warm servers init in <1s.
+_LSP_INIT_TIMEOUT = 30
 
 # How long to keep an idle server alive before shutting it down.
 _LSP_IDLE_TIMEOUT = 300  # 5 minutes
@@ -318,9 +319,62 @@ def _find_workspace_folders(root: str) -> List[str]:
     return folders
 
 
+_LSP_EXTRA_BIN_DIRS = (
+    "/opt/homebrew/bin",       # Apple Silicon Homebrew (npm -g LSP servers)
+    "/opt/homebrew/opt/node/bin",
+    "/usr/local/bin",          # Intel Homebrew + manual installs
+    "/usr/bin",
+    str(Path.home() / ".local" / "bin"),
+)
+# Explicit extra bins for service/CI launchers. The installer records the
+# verified directory after resolving each global LSP executable. A child can
+# then start under PATH=/usr/bin:/bin without relying on a parent shell PATH.
+_LSP_ENV_BIN_DIRS = tuple(
+    d for d in os.environ.get("CODE_INTEL_LSP_BIN_DIRS", "").split(os.pathsep)
+    if d
+)
+_LSP_EXTRA_BIN_DIRS = tuple(_LSP_EXTRA_BIN_DIRS) + _LSP_ENV_BIN_DIRS
+
+
+def _lsp_bin_dirs() -> Tuple[str, ...]:
+    """Return static plus current-process configured LSP bin directories."""
+    dynamic = tuple(
+        d for d in os.environ.get("CODE_INTEL_LSP_BIN_DIRS", "").split(os.pathsep)
+        if d
+    )
+    return tuple(dict.fromkeys(_LSP_EXTRA_BIN_DIRS + dynamic))
+
+
 def _resolve_command(cmd: str) -> Optional[str]:
-    """Return the full path for *cmd* if it exists on ``$PATH``, else ``None``."""
-    return shutil.which(cmd)
+    """Return the full path for *cmd* if it exists, else ``None``.
+
+    ``shutil.which`` only searches ``$PATH``. Scheduled/daemon launch contexts
+    (launchd, Paseo workers, nightly maintenance) frequently run with a reduced
+    PATH that omits ``/opt/homebrew/bin``, which silently hides globally
+    installed LSP servers (typescript-language-server, pyright-langserver).
+    Fall back to the well-known install dirs so server discovery is
+    PATH-independent.
+    """
+    found = shutil.which(cmd)
+    if found:
+        return found
+    if os.sep in cmd or "/" in cmd:
+        return None  # explicit path given — do not second-guess
+    # Explicit configuration wins over generic fallback dirs. This makes
+    # service/CI installs deterministic when multiple global npm prefixes exist.
+    configured = tuple(
+        d for d in os.environ.get("CODE_INTEL_LSP_BIN_DIRS", "").split(os.pathsep)
+        if d
+    )
+    for d in configured + tuple(d for d in _lsp_bin_dirs() if d not in configured):
+        try:
+            candidate = Path(d) / cmd
+            if candidate.is_file() and os.access(candidate, os.X_OK):
+                logger.info("LSP server %s not on PATH, resolved via fallback dir: %s", cmd, candidate)
+                return str(candidate)
+        except OSError:
+            continue
+    return None
 
 
 # ---------------------------------------------------------------------------
@@ -346,11 +400,16 @@ class LSPBridge:
     _alive: bool = field(default=False, init=False, repr=False)
     _last_activity: float = field(default=0.0, init=False, repr=False)
     _initialized: bool = field(default=False, init=False, repr=False)
-    _init_lock: threading.Lock = field(default_factory=threading.Lock, init=False, repr=False)
+    _init_lock: threading.RLock = field(default_factory=threading.RLock, init=False, repr=False)
     _diagnostics_cache: Dict[str, List[dict]] = field(default_factory=dict, init=False, repr=False)
     _open_documents: set = field(default_factory=set, init=False, repr=False)  # Track open docs to avoid duplicate didOpen
     _reconcile_close_uris: Dict[str, float] = field(default_factory=dict, init=False, repr=False)
     _doc_versions: Dict[str, int] = field(default_factory=dict, init=False, repr=False)  # textDocument version per URI for didChange
+    # mtime at which each file was last analysed (validates cached diagnostics)
+    _diag_mtime: Dict[str, Optional[float]] = field(default_factory=dict, init=False, repr=False)
+    # Server capabilities from the initialize response (used to decide whether
+    # pull diagnostics (textDocument/diagnostic) can possibly work at all).
+    init_capabilities: dict = field(default_factory=dict, init=False, repr=False)
 
     # -- lifecycle -----------------------------------------------------------
 
@@ -366,6 +425,14 @@ def _build_env(self) -> Dict[str, str]:
         # TypeScript: ensure TSServer can resolve types from workspace
         if self.language_id in ("typescript", "typescriptreact", "javascript", "javascriptreact"):
             env["TSS_LOG"] = "-"  # Log to stderr (captured, not lost)
+        # Reduced-PATH launch contexts (launchd, schedulers): LSP wrappers like
+        # typescript-language-server carry a `#!/usr/bin/env node` shebang and
+        # cannot exec node unless its dir is on the child PATH. Add the same
+        # fallback dirs used by _resolve_command, preserving any existing PATH.
+        existing_path = env.get("PATH", "")
+        extra = [d for d in _LSP_EXTRA_BIN_DIRS if d not in existing_path.split(os.pathsep)]
+        if extra:
+            env["PATH"] = os.pathsep.join([existing_path] + extra) if existing_path else os.pathsep.join(extra)
         return env
 
     def _get_initialization_options(self) -> Dict[str, Any]:
@@ -452,6 +519,14 @@ def _start_and_init(self) -> bool:
                         "hover": {"dynamicRegistration": False, "contentFormat": ["plaintext", "markdown"]},
                         "typeDefinition": {"dynamicRegistration": False},
                         "rename": {"dynamicRegistration": False, "prepareSupport": True},
+                        # typescript-language-server only publishes diagnostics
+                        # when client explicitly advertises this LSP capability.
+                        "publishDiagnostics": {
+                            "relatedInformation": True,
+                            "versionSupport": True,
+                            "codeDescriptionSupport": True,
+                            "dataSupport": True,
+                        },
                     },
                     "workspace": {
                         "symbol": {"dynamicRegistration": False},
@@ -477,6 +552,7 @@ def _start_and_init(self) -> bool:
             # Send initialized notification
             self._send_notification("initialized", {})
             self._initialized = True
+            self.init_capabilities = init_result.get("capabilities", {}) or {}
             server_info = init_result.get("serverInfo", {})
             logger.info("LSP server initialized: %s (%s %s) in %.1fs",
                 self.command,
@@ -1274,10 +1350,22 @@ def collect_diagnostics(
         if not self.ensure_initialized():
             return None
         uri = f"file://{file_path}"
+        on_disk_mtime = _safe_mtime(file_path)
+        with self._lock:
+            cached = self._diagnostics_cache.get(file_path)
+            already_open = uri in self._open_documents
+            last_mtime = self._diag_mtime.get(file_path)
+        # Fast path: document already open and the on-disk file is unchanged
+        # since the last analysis -> the cached result is still authoritative.
+        # TSLS (server-side) dedupes repeated empty results and would NOT
+        # re-publish for an unchanged clean file, so a blind cache-pop + wait
+        # here would wrongly return None ("no push") for a perfectly healthy
+        # server. Only skip the fast path when there is nothing cached yet.
+        if already_open and cached is not None and last_mtime == on_disk_mtime:
+            return cached
         # Drop any stale push so we can detect the *fresh* one for this call.
         with self._lock:
             self._diagnostics_cache.pop(file_path, None)
-            already_open = uri in self._open_documents
         if already_open:
             # didOpen would be a no-op on an open document, so the server would
             # not re-analyse. A didChange (full sync, bumped version) forces a
@@ -1312,7 +1400,10 @@ def collect_diagnostics(
                     break
             time.sleep(0.05)
         with self._lock:
-            return self._diagnostics_cache.get(file_path)
+            result = self._diagnostics_cache.get(file_path)
+            if result is not None:
+                self._diag_mtime[file_path] = on_disk_mtime
+            return result
 
     def get_server_info(self) -> dict:
         """Return basic health info about this bridge."""
@@ -1512,6 +1603,14 @@ def get_lsp_manager() -> LSPManager:
 # ---------------------------------------------------------------------------
 
 
+def _safe_mtime(file_path: str) -> Optional[float]:
+    """Best-effort file mtime; ``None`` when unreadable (still safe to compare)."""
+    try:
+        return Path(file_path).stat().st_mtime
+    except OSError:
+        return None
+
+
 def _detect_language_for_lsp(file_path: str) -> Optional[str]:
     """Detect language suitable for LSP resolution."""
     ext = Path(file_path).suffix.lower()
@@ -1789,6 +1888,17 @@ def code_diagnostics_tool(
                 return _json.dumps(summary, indent=2)
 
     # No push arrived — try pull diagnostics (LSP 3.17+, servers that support it)
+    # Note: pyright answers textDocument/diagnostic but does NOT advertise a
+    # diagnosticProvider capability, so treat pyright-family servers as pull-
+    # capable even without the advertised capability.
+    pull_supported = False
+    if bridge is not None:
+        try:
+            pull_supported = bool(
+                (bridge.init_capabilities or {}).get("diagnosticProvider")
+            ) or "pyright" in (bridge.command or "")
+        except Exception:
+            pull_supported = False
     if bridge and bridge.ensure_initialized():
         try:
             resp = bridge._send_request("textDocument/diagnostic", {
@@ -1814,7 +1924,17 @@ def code_diagnostics_tool(
 
     # Fallback: AST heuristic (no LSP server, or server reported nothing in time)
     logger.debug("code_diagnostics: using AST fallback")
-    return _ast_fallback_diagnostics(str(target), lang)
+    if lang is None:
+        fallback_reason = "unsupported_language"
+    elif bridge is None:
+        fallback_reason = "lsp_server_unavailable"
+    elif not bridge.ensure_initialized():
+        fallback_reason = "lsp_init_failed"
+    elif not pull_supported:
+        fallback_reason = "no_diagnostics_support"
+    else:
+        fallback_reason = "no_diagnostics_response"
+    return _ast_fallback_diagnostics(str(target), lang, fallback_reason=fallback_reason)
 
 
 def code_callers_tool(
@@ -2208,7 +2328,11 @@ def _ast_fallback_references(
         })
 
 
-def _ast_fallback_diagnostics(file_path: str, lang: Optional[str]) -> str:
+def _ast_fallback_diagnostics(
+    file_path: str,
+    lang: Optional[str],
+    fallback_reason: str = "lsp_unavailable",
+) -> str:
     """Lightweight AST-based heuristic for common issues: unused imports, undefined names."""
     import json as _json
     content = ""
@@ -2287,7 +2411,8 @@ def _ast_fallback_diagnostics(file_path: str, lang: Optional[str]) -> str:
     return _json.dumps({
         "path": file_path,
         "method": "ast_heuristic",
-        "warning": "LSP server unavailable. Using lightweight AST heuristic.",
+        "warning": f"LSP diagnostics unavailable ({fallback_reason}). Using lightweight AST heuristic.",
+        "fallback_reason": fallback_reason,
         "diagnostic_count": len(diagnostics),
         "errors": len([d for d in diagnostics if d.get("severity", 1) == 1]),
         "warnings": len([d for d in diagnostics if d.get("severity", 2) == 2]),

scripts/tsls_probe.js

@@ -0,0 +1,18 @@
+// TSLS initialize-handshake probe: prints exactly what the server answers
+// (or "NO RESPONSE"), bounded, non-interactive. Used by CI to diagnose why
+// typescript-language-server --stdio never answers initialize on runners.
+const { spawn } = require("child_process");
+const p = spawn("typescript-language-server", ["--stdio"], { cwd: process.cwd() });
+let got = false;
+p.stdout.on("data", d => { got = true; console.log("TSLS-OUT:", d.toString().slice(0, 400)); });
+p.stderr.on("data", d => console.log("TSLS-ERR:", d.toString().slice(0, 400)));
+p.on("exit", (c, s) => console.log("TSLS-EXIT:", c, s));
+const init = JSON.stringify({jsonrpc:"2.0",id:1,method:"initialize",params:{processId:null,rootUri:"file://" + process.cwd(),capabilities:{}}});
+const msg = "Content-Length: " + Buffer.byteLength(init) + "\r\n\r\n" + init;
+p.stdin.write(msg);
+setTimeout(() => {
+  if (!got) { console.log("TSLS: NO RESPONSE in 8s"); p.kill("SIGKILL"); process.exit(1); }
+  console.log("TSLS: replied OK");
+  p.kill("SIGKILL");
+  process.exit(0);
+}, 8000);

tests/ci_diag_fixtures.py

@@ -0,0 +1,31 @@
+#!/usr/bin/env python3
+"""code_intel diagnostics fixtures: TS + Python, clean + intentional error.
+
+Used by code_intel_health_check.py and tests. Keep errors INTENTIONAL.
+"""
+import sys, os
+
+FIXTURES = {
+    "clean.ts": "const cleanValue: number = 1;\nconsole.log(cleanValue);\n",
+    # Intentional TS errors: type mismatch (2322) + undefined name (2304)
+    "error.ts": 'const badValue: number = "oops";\nconsole.log(notDefinedName);\n',
+    "clean.py": "def clean_fn() -> int:\n    return 1\n",
+    # Intentional Python error: undefined name
+    "error.py": "def bad_fn() -> int:\n    return not_defined_name\n",
+}
+
+def ensure_fixtures(base_dir: str) -> dict:
+    """Write fixtures under base_dir; return {relname: abs_path}."""
+    os.makedirs(base_dir, exist_ok=True)
+    out = {}
+    for name, content in FIXTURES.items():
+        p = os.path.join(base_dir, name)
+        with open(p, "w") as f:
+            f.write(content)
+        out[name] = p
+    return out
+
+if __name__ == "__main__":
+    base = sys.argv[1] if len(sys.argv) > 1 else os.path.expanduser(
+        "~/.hermes/scripts/.code_intel_fixtures")
+    print(ensure_fixtures(base))