fix(core): register signal handler before startup work and handshake

CHANGELOG.md

@@ -57,6 +57,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **`run_stdio` registered its `SIGTERM`/`SIGINT` handler too late to catch a signal sent early in startup** — `serve_with` previously ran config validation, workspace-root heuristics, and `spawn_lsp_servers_background` (which spawns LSP child processes concurrently) before `run_stdio` ever registered a signal handler, and `run_stdio` itself only did so *after* `mcp_server.serve(..)`'s MCP `initialize` handshake resolved — `rmcp`'s `serve(..)` awaits the client's first message internally, so a signal arriving at any point up to and including that wait fell through to the OS's default disposition (immediate termination, skipping `Translator::shutdown_servers` and risking an orphaned LSP child process mid-spawn; the exact failure mode #270 was filed to prevent). New `ShutdownSignal` type is now constructed once, as the first statement in `serve_with`, before any of that startup work, and moved by value into whichever transport runs; `run_stdio` races it against the handshake itself, then reuses the same instance in its existing post-handshake `select!`. Also fixed a related gap introduced while designing the reused handle: `SIGINT` was previously re-registered via `tokio::signal::ctrl_c()` on every wait (a fresh one-shot listener each time), which silently lost a signal delivered while a different `select!` branch was being polled — `ShutdownSignal` now holds a persistent listener per signal kind (`SIGTERM` and `SIGINT` on Unix via `tokio::signal::unix::signal`, `Ctrl-C` on Windows via the persistent `tokio::signal::windows::ctrl_c()` stream) for its entire lifetime instead. (#318)
 - **`MCPLS_LOG_JSON`/`MCPLS_TRUST_PROJECT_CONFIG` rejected common boolean env-var spellings** — both flags are declared as plain `bool` fields with clap's `env` attribute, whose derived value parser is `str::parse::<bool>()`, accepting only the exact lowercase literals `"true"`/`"false"`. Any other common convention (`1`/`0`, `yes`/`no`, `Y`/`N`, `on`/`off`, uppercase `TRUE`/`FALSE`) caused clap to print a parse error and exit nonzero before `logging::init` or anything else ran — no LSP servers spawned, no MCP server started, nothing logged. A new `parse_bool_flag` value parser, applied to both fields via `#[arg(value_parser = ...)]`, now accepts `1`/`0`, `true`/`false`, `yes`/`no`, `y`/`n`, and `on`/`off`, case-insensitively; the `--log-json`/`--trust-project-config` CLI flags themselves are unaffected (still bare, no-value flags). Any other value is still a clear parse error at startup. (#295)
 - **mcpls did not exit on `SIGTERM`/`SIGINT` while a stdio MCP client kept its stdin write end open** — `rmcp::transport::stdio()` is backed by `tokio::io::stdin()`, which internally parks an uncancellable `spawn_blocking` thread in a raw `read()` syscall; `#[tokio::main]`'s generated wrapper blocks in `Runtime::drop` waiting for that thread once `main`'s body returns, even though all real shutdown work (LSP server teardown, log flush) had already completed by then. `main` now calls `std::process::exit` as its final step instead of returning normally, terminating immediately once `run().await` resolves and bypassing the blocking-pool wait. `main`'s signature changed from `-> std::process::ExitCode` (added in #279) to `()`; not a breaking change for library embedders — a binary's `main` signature is not part of `mcpls-core`'s public API, and the resulting exit codes (0/1) are unchanged. Scoped to `crates/mcpls-cli/src/main.rs` only — `mcpls-core`'s `serve`/`serve_with`/`shutdown`/`run_stdio` are unaffected and keep normal `Result`-returning semantics for library embedders; their doc comments now carry a "Shutdown" note (and updated examples) warning embedders of the same `process::exit` requirement under the stdio transport. Also fixed: `await_lsp_init_handle`'s timeout branch called `JoinHandle::abort()` without a subsequent await, so on a `SIGTERM` arriving mid-`spawn_batch` (before any server registers), `process::exit` could now run before the runtime dropped the aborted task's locals — including not-yet-registered LSP `Child` handles relying on `kill_on_drop` — orphaning those processes (the failure mode #270 guards against). `abort()` is now followed by a bounded (1s) re-await of the same handle so that drop happens before `main` exits. (#308)
 - **`workspace.position_encodings` was parsed but never consumed** — the configured preference order was previously dead: `LspServer::spawn`'s `initialize` handshake always offered a hardcoded `["utf-8", "utf-16"]` regardless of what `mcpls.toml` set. It is now sent as `capabilities.general.positionEncodings` in the configured order (see the accompanying breaking changes under Changed above). Note this only changes which encodings mcpls *offers*; the encoding a server actually negotiates is still not consumed downstream of the handshake (tracked separately in #290). (#287)

crates/mcpls-core/src/lib.rs

@@ -63,7 +63,7 @@ pub use transport::HttpConfig;
 pub use transport::Transport;
 #[cfg(feature = "transport-http")]
 use transport::run_http;
-use transport::run_stdio;
+use transport::{ShutdownSignal, run_stdio};
 
 /// Whether `uri` falls within one of `workspace_roots`.
 ///
@@ -467,6 +467,17 @@ pub async fn serve(config: ServerConfig) -> Result<(), Error> {
 pub async fn serve_with(config: ServerConfig, transport: Transport) -> Result<(), Error> {
     info!("Starting MCPLS server...");
 
+    // Registered before any other startup work -- including
+    // `spawn_lsp_servers_background` below, which spawns LSP child processes
+    // concurrently on another worker thread -- so a `SIGTERM`/`SIGINT`
+    // arriving during config validation, workspace-root heuristics, or LSP
+    // spawning is caught rather than hitting the OS's default disposition
+    // (immediate termination, orphaning any LSP child mid-spawn; see #270)
+    // and skipping the `shutdown()` cleanup below entirely. See
+    // `ShutdownSignal`'s docs for why this must be a single instance carried
+    // through by value rather than re-registered later.
+    let shutdown_signal = ShutdownSignal::new();
+
     // `ServerConfig::load`/`load_from` already validate the TOML-loading
     // path; this covers the other one -- a caller building `ServerConfig`
     // programmatically (e.g. a library embedder) previously hit no
@@ -615,10 +626,10 @@ pub async fn serve_with(config: ServerConfig, transport: Transport) -> Result<()
     let result = match transport {
         Transport::Stdio => {
             info!("Listening for MCP requests on stdio...");
-            run_stdio(mcp_server, &peer_cell).await
+            run_stdio(mcp_server, &peer_cell, shutdown_signal).await
         }
         #[cfg(feature = "transport-http")]
-        Transport::Http(cfg) => run_http(mcp_server, cfg).await,
+        Transport::Http(cfg) => run_http(mcp_server, cfg, shutdown_signal).await,
     };
 
     shutdown(&cancel_tx, &translator, lsp_init_handle).await;