fix(lsp,dap): resolve program names via PATHEXT before spawning

CHANGELOG.md

@@ -4,6 +4,14 @@ All notable changes to Nexis. Format loosely follows [Keep a Changelog](https://
 
 ## [Unreleased]
 
+### Fixed
+- **npm-installed language servers were found by the probe and then failed to spawn on Windows.** Installing `vscode-langservers-extracted` gave Nexis a working CSS/HTML/JSON server that it still refused to start, and the missing-tools notice told the story backwards: pressing refresh cleared the entry, then opening a `.css` file put it straight back. The two sides were asking different questions. `tool_probe` walks `PATHEXT` deliberately — 1.25.0 added that precisely because these servers land as `.cmd` shims — but the spawn in `lsp/session.rs` handed the bare name to `std::process::Command`, and Rust's Windows program resolution appends **only** `.exe`; it never consults `PATHEXT`. So `vscode-css-language-server` resolved for the probe and did not exist for the spawn.
+  - `tool_probe`'s PATH walk now returns the resolved path instead of a bool (`tools::resolve_on_host`), and both the LSP client and the DAP client spawn that path rather than the bare name. One walk answers both questions, so the two cannot drift apart again. When resolution finds nothing the bare name is still passed through, leaving the failure path — and its error message — exactly as it was.
+  - **PATHEXT spellings are now tried before the extensionless one**, and that ordering is the fix, not a detail. npm's shim writer emits three files per bin: `foo.cmd`, `foo.ps1`, and an extensionless `foo` that is a *bash* script for MSYS/Git Bash. Preferring the bare spelling — which the old suffix list did — would have resolved to a path `CreateProcessW` cannot execute, turning a lookup miss into a spawn failure. Trying PATHEXT first selects the `.cmd`, which `Command` routes through `cmd.exe` with its own hardened quoting.
+  - The DAP client is covered for the same reason rather than pre-emptively: the adapter command is free text in the debugger panel, so anyone naming an npm-installed adapter (`js-debug-adapter`) hits the identical gap.
+  - **A `.cmd` server is not the process Nexis holds a handle to**, so stopping it needed a tree kill. Rust's std spawns a batch program as `cmd.exe /d /c "<shim> ..."` — the handle is the wrapper and the server is a grandchild, so `Child::kill` (a bare `TerminateProcess`) left the server alive holding both pipe ends: the reader thread never saw EOF and every restart leaked another orphan. LSP and DAP sessions now hold a Windows Job Object with `KILL_ON_JOB_CLOSE`, the same guard ConPTY children have always had — the primitive moved from `pty/job.rs` to `modules/job.rs` and is now shared rather than duplicated. Both sessions also `wait()` after killing, so the child is reaped instead of lingering as a zombie until the app exits.
+  - **Resolution now answers the question the spawn asks, not an approximation of it.** Three ways it could return a path that would not run: a set-but-empty `PATHEXT` reads back as `Some("")` rather than absent, so the `.COM;.EXE;.BAT;.CMD` default never fired and the suffix list collapsed to the extensionless spelling — silently disabling `.cmd` lookup altogether; a program named as a *path* (`.\node_modules\.bin\js-debug-adapter`, which the debugger panel invites) skipped the suffix walk entirely and matched npm's extensionless bash script sitting next to the shim; and on Unix `mode & 0o111 != 0` accepted an exec bit belonging to somebody else, so a root-owned `0700` copy early on `PATH` would shadow the `0755` one later, where `execvp` skips the `EACCES` match and keeps walking. The suffix parsing is now a pure function tested on every platform rather than only on a Windows runner, and Unix executability is `access(X_OK)`.
+
 ## [1.25.0] — 2026-08-19
 
 ### Added

docs/vault/Home.md

@@ -49,6 +49,6 @@ This vault is the **navigational knowledge base** for the Nexis codebase. It ans
 
 ## Other sections
 
-- `decisions/` — lightweight ADRs: why something is the way it is, alternatives rejected (empty so far — use `templates/decision.md`)
+- `decisions/` — lightweight ADRs: why something is the way it is, alternatives rejected. [[expansion-packs]], [[nexis-ml-artifact-pinning]], [[program-resolution-before-spawn]] (new ones from `templates/decision.md`)
 - `runbooks/` — how to do rare-but-recurring tasks (release, debugging a class of bug, forcing cache refreshes)
 - `templates/` — copy these when creating a new note

docs/vault/decisions/program-resolution-before-spawn.md

@@ -0,0 +1,42 @@
+---
+type: decision
+description: LSP and DAP resolve a program name to a concrete path via the same PATH walk tool_probe uses, then spawn that path — because Rust's Command ignores PATHEXT on Windows
+---
+
+# External programs are resolved to a path before spawning, not handed to `Command` as a bare name
+
+**Date:** 2026-08
+**Status:** active
+
+## Context
+
+Nexis asks two different questions about the same external tool, and they were answered by two different pieces of code:
+
+- **"Is it installed?"** — `tool_probe` (`src-tauri/src/modules/tools.rs`), behind the missing-tools pill's refresh button.
+- **"Run it."** — `LspSession::start` / `DapSession::start`, which called `proc::command(<bare name>)`.
+
+`tool_probe` walks `PATHEXT` on Windows on purpose: every server from `vscode-langservers-extracted` installs as a `.cmd` shim, not an `.exe`. `std::process::Command` does **not** walk `PATHEXT` — its Windows resolution appends only `.exe`. So `vscode-css-language-server` was simultaneously "installed" (probe) and "not found" (spawn).
+
+The user-visible shape of this was a notice that lied in both directions: refresh cleared the entry, and the next `.css` file put it back, forever.
+
+## Decision
+
+The PATH walk returns the **resolved path**, not a bool. `tools::resolve_on_host(name) -> Option<PathBuf>` is the single implementation; `resolves_on_host` is now `resolve_on_host(..).is_some()`. Both spawn sites resolve first and pass the resulting path to `proc::command`, falling back to the bare name when resolution finds nothing so the failure path and its error message are unchanged.
+
+On Windows the suffix list tries **PATHEXT entries before the extensionless spelling**. npm writes three files per bin — `foo.cmd`, `foo.ps1`, and an extensionless `foo` that is a bash script for MSYS/Git Bash. Preferring the bare spelling resolves to a path `CreateProcessW` cannot execute, which is strictly worse than not resolving at all.
+
+## Alternatives rejected
+
+- **Append `.cmd` on Windows at the call site** — hard-codes one shim flavour, ignores the user's actual `PATHEXT`, and puts platform knowledge in every spawn site instead of one.
+- **Spawn through `cmd /c`** — Rust's std already detects `.bat`/`.cmd` by extension and routes them through `cmd.exe` with the hardened quoting added for CVE-2024-24576. Doing it by hand re-opens the quoting hole that fix closed.
+- **Teach `proc::command` to resolve** — tempting, but `proc::command` is also used for `wsl.exe`, `git`, `nvidia-smi` and absolute interpreter paths, where a PATH walk is either wasted or actively wrong. Resolution belongs to the callers that take a *user- or config-supplied* program name.
+
+## Consequences
+
+- Probe and spawn can no longer disagree: they are the same walk. A future divergence would require someone to reintroduce a second lookup.
+- Any **new** spawn site whose program name comes from config or user input should call `resolve_on_host` first. Sites that spawn a fixed system binary (`wsl.exe`, `git`) or an already-absolute path do not need it.
+- This is host-side resolution only. WSL tools resolve inside the distro via `command -v` through `wsl_exec_capture` — see pitfall #20 in CLAUDE.md for why the two sides must not answer for each other.
+- Resolving means the returned path is *spawned*, so it has to be runnable by **this** user, not merely marked executable for someone: Unix executability is `access(X_OK)`, matching what `execvp` asks before it skips a match and keeps walking `PATH`.
+- Spawning a resolved `.cmd` makes the direct child `cmd.exe`, not the server. Both session types therefore hold a `KILL_ON_JOB_CLOSE` Job Object (`modules/job.rs`, shared with the ConPTY children it was written for) — `Child::kill` alone terminates the wrapper and orphans the server.
+- The `PATHEXT` parsing is a pure function (`pathext_suffixes`) rather than an env read inside the `#[cfg(windows)]` branch.
+- The Windows branch is `#[cfg(windows)]`, so CI on Linux does not exercise it. The `windows_tries_pathext_before_the_extensionless_name` test asserts the ordering invariant on every platform by checking the suffix list's shape.

src-tauri/src/modules/dap/session.rs

@@ -25,6 +25,16 @@ use crate::modules::proc;
 type PendingMap = Arc<Mutex<HashMap<u32, mpsc::SyncSender<Result<Value, String>>>>>;
 
 pub struct DapSession {
+    /// Windows only: kills the whole process tree when the session drops.
+    ///
+    /// Load-bearing since programs are resolved before spawning. A server that
+    /// resolves to a `.cmd` shim is run as `cmd.exe /d /c "<shim> ..."`, so
+    /// `_child` is the wrapper and the server itself is a grandchild —
+    /// `kill()` would terminate the wrapper and leave the server alive holding
+    /// both pipe ends, so the reader thread never sees EOF and every restart
+    /// leaks another orphan. Declared before `_child` so the Job closes first.
+    #[cfg(windows)]
+    _job: Option<crate::modules::job::ProcessJob>,
     _child: Child,
     stdin: Arc<Mutex<Box<dyn Write + Send>>>,
     pending: PendingMap,
@@ -42,7 +52,13 @@ impl DapSession {
         session_id: u32,
         app: AppHandle,
     ) -> Result<Self, String> {
-        let mut cmd = proc::command(adapter_cmd);
+        // Same PATHEXT resolution the LSP client does, and for the same
+        // reason: the adapter command is free text in the debugger panel, so
+        // a user who types an npm-installed adapter (`js-debug-adapter`) hits
+        // the `.cmd`-shim gap that `Command` alone cannot bridge on Windows.
+        let program = crate::modules::tools::resolve_on_host(adapter_cmd)
+            .unwrap_or_else(|| std::path::PathBuf::from(adapter_cmd));
+        let mut cmd = proc::command(&program);
         cmd.args(adapter_args)
             .stdin(Stdio::piped())
             .stdout(Stdio::piped())
@@ -52,6 +68,18 @@ impl DapSession {
             .spawn()
             .map_err(|e| format!("dap: failed to start '{adapter_cmd}': {e}"))?;
 
+        // Tree-kill guard, set up before anything can fail out of this
+        // function. See the `_job` field for why `kill()` alone is not enough
+        // once a `.cmd` shim is in play.
+        #[cfg(windows)]
+        let job = match crate::modules::job::ProcessJob::create_for(child.id()) {
+            Ok(j) => Some(j),
+            Err(e) => {
+                log::warn!("dap job-object setup failed for pid={}: {e}", child.id());
+                None
+            }
+        };
+
         let stdin = child.stdin.take().ok_or("dap: no stdin")?;
         let stdout = child.stdout.take().ok_or("dap: no stdout")?;
 
@@ -66,6 +94,8 @@ impl DapSession {
         }
 
         Ok(Self {
+            #[cfg(windows)]
+            _job: job,
             _child: child,
             stdin,
             pending,
@@ -232,6 +262,8 @@ impl Drop for DapSession {
     fn drop(&mut self) {
         let _ = self.disconnect();
         let _ = self._child.kill();
+        // Reap, so the killed adapter does not linger as a zombie.
+        let _ = self._child.wait();
     }
 }
 

src-tauri/src/modules/pty/job.rs → src-tauri/src/modules/job.rs

@@ -4,9 +4,17 @@
 // ║  2026                                ║
 // ╚══════════════════════════════════════╝
 
-//! Windows Job Object with KILL_ON_JOB_CLOSE for ConPTY children.
+//! Windows Job Object with KILL_ON_JOB_CLOSE for spawned children.
 //! Dropping the handle kills the whole tree — only reliable orphan guard
 //! on Windows.
+//!
+//! Two callers need it, for the same reason from different directions. A
+//! ConPTY child is a shell, so anything it started is a grandchild. And a
+//! program resolved to a `.cmd` shim is not run directly at all: Rust's std
+//! detects the extension and spawns `cmd.exe /d /c "<shim> ..."`, so the
+//! handle the caller holds is the wrapper and the real process — an
+//! npm-installed language server or debug adapter — is a grandchild again.
+//! `Child::kill` is `TerminateProcess`, which does not walk the tree.
 
 // Panic-lint gate: no `.unwrap()`/`.expect()` in production code here.
 // Tests may still panic (allow-*-in-tests in clippy.toml). CI's
@@ -25,14 +33,14 @@ use windows_sys::Win32::System::JobObjects::{
 };
 use windows_sys::Win32::System::Threading::{OpenProcess, PROCESS_SET_QUOTA, PROCESS_TERMINATE};
 
-pub struct PtyJob {
+pub struct ProcessJob {
     handle: HANDLE,
 }
 
-unsafe impl Send for PtyJob {}
-unsafe impl Sync for PtyJob {}
+unsafe impl Send for ProcessJob {}
+unsafe impl Sync for ProcessJob {}
 
-impl PtyJob {
+impl ProcessJob {
     pub fn create_for(pid: u32) -> io::Result<Self> {
         unsafe {
             let job = CreateJobObjectW(std::ptr::null(), std::ptr::null());
@@ -74,7 +82,7 @@ impl PtyJob {
     }
 }
 
-impl Drop for PtyJob {
+impl Drop for ProcessJob {
     fn drop(&mut self) {
         if !self.handle.is_null() && self.handle != INVALID_HANDLE_VALUE {
             unsafe { CloseHandle(self.handle) };

src-tauri/src/modules/lsp/session.rs

@@ -25,6 +25,16 @@ use crate::modules::proc;
 type PendingMap = Arc<Mutex<HashMap<u32, mpsc::SyncSender<Result<Value, String>>>>>;
 
 pub struct LspSession {
+    /// Windows only: kills the whole process tree when the session drops.
+    ///
+    /// Load-bearing since programs are resolved before spawning. A server that
+    /// resolves to a `.cmd` shim is run as `cmd.exe /d /c "<shim> ..."`, so
+    /// `_child` is the wrapper and the server itself is a grandchild —
+    /// `kill()` would terminate the wrapper and leave the server alive holding
+    /// both pipe ends, so the reader thread never sees EOF and every restart
+    /// leaks another orphan. Declared before `_child` so the Job closes first.
+    #[cfg(windows)]
+    _job: Option<crate::modules::job::ProcessJob>,
     /// Kept alive so stdin/stdout pipes stay open.
     _child: Child,
     stdin: Arc<Mutex<Box<dyn Write + Send>>>,
@@ -44,7 +54,17 @@ impl LspSession {
         initialization_options: Option<Value>,
         app: AppHandle,
     ) -> Result<Self, String> {
-        let mut cmd = proc::command(server_cmd);
+        // Resolve before spawning. `Command` on Windows only ever appends
+        // `.exe` to a bare name, but every npm-installed server here lands as
+        // a `.cmd` shim — so `vscode-css-language-server` fails to spawn even
+        // though `tool_probe` (which does walk PATHEXT) reports it present.
+        // That split made the missing-tools pill lie in both directions: it
+        // cleared on refresh, then came straight back on the next open.
+        // Falling back to the bare name keeps the failure path unchanged when
+        // resolution finds nothing, so the error still comes from the spawn.
+        let program = crate::modules::tools::resolve_on_host(server_cmd)
+            .unwrap_or_else(|| std::path::PathBuf::from(server_cmd));
+        let mut cmd = proc::command(&program);
         cmd.args(server_args)
             .stdin(Stdio::piped())
             .stdout(Stdio::piped())
@@ -54,6 +74,18 @@ impl LspSession {
             .spawn()
             .map_err(|e| format!("lsp: failed to start '{server_cmd}': {e}"))?;
 
+        // Tree-kill guard, set up before anything can fail out of this
+        // function. See the `_job` field for why `kill()` alone is not enough
+        // once a `.cmd` shim is in play.
+        #[cfg(windows)]
+        let job = match crate::modules::job::ProcessJob::create_for(child.id()) {
+            Ok(j) => Some(j),
+            Err(e) => {
+                log::warn!("lsp job-object setup failed for pid={}: {e}", child.id());
+                None
+            }
+        };
+
         let stdin = child.stdin.take().ok_or("lsp: no stdin pipe")?;
         let stdout = child.stdout.take().ok_or("lsp: no stdout pipe")?;
 
@@ -70,6 +102,8 @@ impl LspSession {
         }
 
         let session = Self {
+            #[cfg(windows)]
+            _job: job,
             _child: child,
             stdin,
             pending,
@@ -222,9 +256,12 @@ impl LspSession {
 
 impl Drop for LspSession {
     fn drop(&mut self) {
-        // Best-effort graceful shutdown then kill.
+        // Best-effort graceful shutdown then kill. `wait` is not optional:
+        // without it the killed child stays a zombie until the app exits, and
+        // on Windows it is what releases the wrapper before the Job closes.
         let _ = self.notify("exit".to_string(), None);
         let _ = self._child.kill();
+        let _ = self._child.wait();
     }
 }
 

src-tauri/src/modules/mod.rs

@@ -13,6 +13,8 @@ pub mod fs;
 pub mod fswatch;
 pub mod git;
 pub mod http_share;
+#[cfg(windows)]
+pub mod job;
 pub mod lsp;
 pub mod ml;
 pub mod net;

src-tauri/src/modules/pty/mod.rs

@@ -5,8 +5,6 @@
 // ╚══════════════════════════════════════╝
 
 pub(crate) mod da_filter;
-#[cfg(windows)]
-mod job;
 mod session;
 pub(crate) mod shell_init;
 mod watchdog;

src-tauri/src/modules/pty/session.rs

@@ -47,7 +47,7 @@ pub struct Session {
     //   4. `master` — last; ClosePseudoConsole on Windows. By now the child
     //      is dead and conhost has nothing left to drain.
     #[cfg(windows)]
-    _job: Option<super::job::PtyJob>,
+    _job: Option<crate::modules::job::ProcessJob>,
     pub killer: Mutex<Box<dyn ChildKiller + Send + Sync>>,
     /// FIFO input queue drained by the dedicated writer thread. `pty_write`
     /// enqueues here (never blocks); the thread does the actual pipe write,
@@ -193,7 +193,7 @@ pub fn spawn(
 
     #[cfg(windows)]
     let job = match child.process_id() {
-        Some(pid) => match super::job::PtyJob::create_for(pid) {
+        Some(pid) => match crate::modules::job::ProcessJob::create_for(pid) {
             Ok(j) => Some(j),
             Err(e) => {
                 log::warn!("pty job-object setup failed for pid={pid}: {e}");