From f000409e7016f85ffb19c6790a9c5a042939810b Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Mon, 15 Jun 2026 18:33:23 -0700 Subject: [PATCH 1/4] chore(mcp): hyperd_running false positive when health port unreachable in daemon mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In daemon mode, is_running() delegated entirely to daemon::discovery::discover(), which PINGs the health port. If the health port is unreachable (stale daemon.json, port-scan-adopted daemon, or firewall rule), discover() returns None and hyperd_running is reported as false — even while the engine is actively serving queries over its cached libpq endpoint. Fix: treat a populated daemon_endpoint as authoritative for liveness. The engine only stores that field after a successful Connection::connect(), so its presence is strong evidence that hyperd was reachable. Fall back to discover() only when no cached endpoint exists. Note: chore so additional commits can follow before cutting a new version. Fixes #138 --- hyperdb-mcp/src/engine.rs | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/hyperdb-mcp/src/engine.rs b/hyperdb-mcp/src/engine.rs index deb6ba1..f852ed4 100644 --- a/hyperdb-mcp/src/engine.rs +++ b/hyperdb-mcp/src/engine.rs @@ -427,12 +427,27 @@ impl Engine { } /// Whether the backing `hyperd` process is still alive. - /// In daemon mode, checks the daemon health port. + /// + /// In local mode, delegates to the owned `HyperProcess`. In daemon mode, + /// a cached `daemon_endpoint` is treated as authoritative — the engine + /// only holds that endpoint after a successful connection, so its presence + /// means `hyperd` was reachable when this session started. Discovery + /// (`daemon.json` + health-port PING) is used as a fallback only when no + /// cached endpoint exists (e.g. before the first connection attempt). + /// + /// This avoids a false negative where the health port is unreachable + /// (stale `daemon.json`, port-scan-adopted daemon, firewall rule) while + /// the engine is actively serving queries over its live libpq connection. pub fn is_running(&self) -> bool { if let Some(ref hyper) = self.hyper { hyper.is_running() + } else if self.daemon_endpoint.is_some() { + // Daemon mode with a live cached endpoint: treat it as running. + // The engine only stores this endpoint after a successful connect, + // so its presence is strong evidence the daemon is up. + true } else { - // Daemon mode: check if daemon is still reachable + // Daemon mode but no cached endpoint yet — fall back to discovery. daemon::discovery::discover().is_some() } } From cdab8366f21662cd9c0c0aa1d0ecefdc9cae8838 Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Mon, 15 Jun 2026 19:37:53 -0700 Subject: [PATCH 2/4] chore(mcp): status hangs at engine_busy:true when no data-plane call is made first `status` used try_lock and returned status_degraded() whenever the engine was None, expecting "the first data-plane call will init the engine". But clients like Windsurf that call `status` as their first tool call would never trigger Engine::new() and would see engine_busy:true indefinitely. Fix: when the engine is None after try_lock succeeds, drop the guard and call ensure_engine() (blocking init) so status always returns a full response on the first call, regardless of prior tool usage. --- hyperdb-mcp/src/server.rs | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/hyperdb-mcp/src/server.rs b/hyperdb-mcp/src/server.rs index c2478cb..07c7fdd 100644 --- a/hyperdb-mcp/src/server.rs +++ b/hyperdb-mcp/src/server.rs @@ -2963,11 +2963,24 @@ impl HyperMcpServer { // true` and knows table/disk stats are unavailable this call. return Self::ok_content(self.status_degraded()); }; + if guard.is_none() { + // Engine not yet initialized. Drop the try_lock guard and use + // ensure_engine() (blocking lock) to initialize it now. Without + // this, sessions that never make a data-plane call would never + // trigger initialization and `status` would return engine_busy:true + // indefinitely — a hang visible in clients like Windsurf that call + // `status` before any other tool. + drop(guard); + if let Err(e) = self.ensure_engine() { + return Self::err_content(e); + } + } + // Re-acquire after initialization (or if guard was already Some). + let guard = match self.engine.try_lock() { + Ok(g) => g, + Err(_) => return Self::ok_content(self.status_degraded()), + }; let Some(engine) = guard.as_ref() else { - // Engine not yet initialized (first call after server start, or - // after a ConnectionLost drop). Return the degraded response rather - // than an error — the first data-plane call will init the engine, - // and subsequent `status` calls will get the full response. return Self::ok_content(self.status_degraded()); }; self.ensure_catalog_ready(engine); From d5c6afffd3717dc2d04c4556a356f6b57a038aa5 Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Mon, 15 Jun 2026 20:12:10 -0700 Subject: [PATCH 3/4] fix(mcp): probe endpoint for liveness + eager engine init (addresses review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reworks the two prior fixes after adversarial review surfaced a latent false-positive and a #118 regression risk. is_running() (engine.rs): instead of returning true whenever a cached daemon_endpoint exists, probe that endpoint with a short-timeout TCP connect. This is issue #138's "option 2" and reflects *current* liveness of the libpq endpoint the engine actually queries. It is robust to both the original false negative (health port unreachable while libpq serves) AND a stale-endpoint false positive (daemon restarts hyperd on a new port — the probe then correctly reports false). The unconditional `true` could have reported a dead endpoint as alive. status() (server.rs): revert to a pure, non-blocking observer. The previous approach made status() call the blocking ensure_engine(), which risked reintroducing the #118 hang. Instead, initialize the engine eagerly at server startup via a new warm_up_engine() (called from main.rs before serve()). The "stuck engine_busy:true forever" symptom was a lazy-init-only-on-data-plane design smell; eager init fixes the whole class (any read-only-first tool), not just status. warm_up_engine() logs and swallows errors so startup is resilient if hyperd is briefly unreachable; the first data-plane call retries via with_engine(). Verified: cargo fmt + clippy --all-targets clean, 82 lib tests pass, release build succeeds. --- hyperdb-mcp/src/engine.rs | 50 +++++++++++++++++++++++++------------ hyperdb-mcp/src/main.rs | 5 ++++ hyperdb-mcp/src/server.rs | 52 ++++++++++++++++++++++++--------------- 3 files changed, 71 insertions(+), 36 deletions(-) diff --git a/hyperdb-mcp/src/engine.rs b/hyperdb-mcp/src/engine.rs index f852ed4..a8e9403 100644 --- a/hyperdb-mcp/src/engine.rs +++ b/hyperdb-mcp/src/engine.rs @@ -426,28 +426,27 @@ impl Engine { })) } - /// Whether the backing `hyperd` process is still alive. + /// Whether the backing `hyperd` is currently reachable. /// /// In local mode, delegates to the owned `HyperProcess`. In daemon mode, - /// a cached `daemon_endpoint` is treated as authoritative — the engine - /// only holds that endpoint after a successful connection, so its presence - /// means `hyperd` was reachable when this session started. Discovery - /// (`daemon.json` + health-port PING) is used as a fallback only when no - /// cached endpoint exists (e.g. before the first connection attempt). - /// - /// This avoids a false negative where the health port is unreachable - /// (stale `daemon.json`, port-scan-adopted daemon, firewall rule) while - /// the engine is actively serving queries over its live libpq connection. + /// probes the cached libpq `daemon_endpoint` directly with a short-timeout + /// TCP connect — the same endpoint queries run against. This reflects + /// *current* liveness of the resource the engine actually depends on, and + /// is robust to two failure modes the health-port PING is not: + /// - the health port being unreachable (stale `daemon.json`, + /// port-scan-adopted daemon, firewall) while the libpq endpoint serves; + /// - the daemon restarting `hyperd` on a new port, leaving the cached + /// endpoint stale (the probe then correctly reports `false`). + /// + /// Falls back to discovery (`daemon.json` + health-port PING) only when no + /// endpoint has been cached yet (before the first connection attempt). pub fn is_running(&self) -> bool { if let Some(ref hyper) = self.hyper { hyper.is_running() - } else if self.daemon_endpoint.is_some() { - // Daemon mode with a live cached endpoint: treat it as running. - // The engine only stores this endpoint after a successful connect, - // so its presence is strong evidence the daemon is up. - true + } else if let Some(ref endpoint) = self.daemon_endpoint { + probe_endpoint_alive(endpoint) } else { - // Daemon mode but no cached endpoint yet — fall back to discovery. + // No cached endpoint yet — fall back to discovery. daemon::discovery::discover().is_some() } } @@ -1886,6 +1885,25 @@ impl Drop for Engine { } } +/// Cheap liveness probe for a daemon-mode `hyperd`: attempt a short-timeout +/// TCP connect to `endpoint` (`host:port`). Returns `true` if the connect +/// succeeds (something is listening). A bare connect is sufficient here — we +/// only need to know the port the engine's libpq connection targets is still +/// accepting connections, not to perform a full protocol round-trip. +fn probe_endpoint_alive(endpoint: &str) -> bool { + use std::net::ToSocketAddrs; + const PROBE_TIMEOUT: std::time::Duration = std::time::Duration::from_millis(300); + match endpoint.to_socket_addrs() { + Ok(addrs) => { + let addrs: Vec<_> = addrs.collect(); + addrs + .iter() + .any(|addr| std::net::TcpStream::connect_timeout(addr, PROBE_TIMEOUT).is_ok()) + } + Err(_) => false, + } +} + fn bootstrap_public_schema(connection: &Connection) -> Result<(), McpError> { connection .execute_command("CREATE SCHEMA IF NOT EXISTS public") diff --git a/hyperdb-mcp/src/main.rs b/hyperdb-mcp/src/main.rs index 4c80d1d..07d5eb2 100644 --- a/hyperdb-mcp/src/main.rs +++ b/hyperdb-mcp/src/main.rs @@ -230,6 +230,11 @@ async fn run_mcp_mode(cli: Cli) -> Result<(), Box> { ); let server = HyperMcpServer::with_no_daemon(persistent_str, cli.read_only, cli.no_daemon); + // Eagerly initialize the engine before accepting tool calls so observer + // tools like `status` report full stats on the first call (issue #138). + // Errors are logged and swallowed inside `warm_up_engine` — startup + // proceeds even if hyperd is momentarily unreachable. + server.warm_up_engine(); let service = server.serve(rmcp::transport::io::stdio()).await?; service.waiting().await?; diff --git a/hyperdb-mcp/src/server.rs b/hyperdb-mcp/src/server.rs index 07c7fdd..5e35770 100644 --- a/hyperdb-mcp/src/server.rs +++ b/hyperdb-mcp/src/server.rs @@ -1122,6 +1122,30 @@ impl HyperMcpServer { Ok(guard) } + /// Eagerly initialize the engine at server startup, before any tool call + /// arrives. This makes read-only observer tools like `status` able to + /// report full stats on the very first call without having to trigger + /// initialization themselves — `status` stays a pure, non-blocking + /// observer (honoring issue #118). + /// + /// Errors are logged and swallowed rather than propagated: if `hyperd` is + /// unreachable at startup the server still comes up, and the first + /// data-plane tool call will retry initialization via `with_engine`. + pub fn warm_up_engine(&self) { + match self.ensure_engine() { + Ok(_guard) => { + tracing::info!("engine initialized eagerly at startup"); + } + Err(e) => { + tracing::warn!( + err = %e.message, + "eager engine initialization failed at startup; \ + will retry on first data-plane tool call" + ); + } + } + } + /// Idempotently create and reconcile `_table_catalog` on first call /// per engine. No-op in bare or read-only mode (read-only can't /// mutate; bare callers never wanted the catalog in the first place). @@ -2957,29 +2981,17 @@ impl HyperMcpServer { // operation on the same session (issue #118). If the engine lock is held // by another tool call, return a degraded-but-instant response with the // metadata available without the engine (daemon health, paths, watchers). + // + // `status` is a pure observer: it never initializes the engine. The + // engine is initialized eagerly at server startup (see + // `warm_up_engine`) and lazily by data-plane tools via `with_engine`, + // so by the time a client can call any tool the engine is normally + // already `Some`. If it is still `None` here (eager init failed because + // hyperd was down at startup, or a ConnectionLost just dropped it), we + // report the degraded response honestly rather than blocking to init. let Ok(guard) = self.engine.try_lock() else { - // Engine is locked by another tool call — return a degraded - // response rather than blocking. The caller sees `engine_busy: - // true` and knows table/disk stats are unavailable this call. return Self::ok_content(self.status_degraded()); }; - if guard.is_none() { - // Engine not yet initialized. Drop the try_lock guard and use - // ensure_engine() (blocking lock) to initialize it now. Without - // this, sessions that never make a data-plane call would never - // trigger initialization and `status` would return engine_busy:true - // indefinitely — a hang visible in clients like Windsurf that call - // `status` before any other tool. - drop(guard); - if let Err(e) = self.ensure_engine() { - return Self::err_content(e); - } - } - // Re-acquire after initialization (or if guard was already Some). - let guard = match self.engine.try_lock() { - Ok(g) => g, - Err(_) => return Self::ok_content(self.status_degraded()), - }; let Some(engine) = guard.as_ref() else { return Self::ok_content(self.status_degraded()); }; From c5137b69ddcad7daddcb6af6895c2456e6649b20 Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Mon, 15 Jun 2026 20:17:49 -0700 Subject: [PATCH 4/4] refactor(mcp): drop needless Vec in probe + warm up on blocking thread Two cheap follow-ups from the final review of the #138 fix: - probe_endpoint_alive: iterate the to_socket_addrs() iterator directly with .any() instead of collecting into a Vec first. Short-circuits on the first address that connects; one connect in the common single-address case. - main: run warm_up_engine() inside tokio::task::spawn_blocking. Warm-up does synchronous I/O and may spawn the daemon (up to several seconds); running it on a runtime worker would stall that thread. Nothing else uses the runtime before serve(), so this only delays startup. cargo fmt + clippy --all-targets clean, 82 lib tests pass, release build ok. --- hyperdb-mcp/src/engine.rs | 10 +++++----- hyperdb-mcp/src/main.rs | 11 +++++++++-- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/hyperdb-mcp/src/engine.rs b/hyperdb-mcp/src/engine.rs index a8e9403..a6cd0e4 100644 --- a/hyperdb-mcp/src/engine.rs +++ b/hyperdb-mcp/src/engine.rs @@ -1894,11 +1894,11 @@ fn probe_endpoint_alive(endpoint: &str) -> bool { use std::net::ToSocketAddrs; const PROBE_TIMEOUT: std::time::Duration = std::time::Duration::from_millis(300); match endpoint.to_socket_addrs() { - Ok(addrs) => { - let addrs: Vec<_> = addrs.collect(); - addrs - .iter() - .any(|addr| std::net::TcpStream::connect_timeout(addr, PROBE_TIMEOUT).is_ok()) + // Probe each resolved address, short-circuiting on the first that + // accepts a connection. `daemon_endpoint` is normally a single + // `127.0.0.1:PORT`, so this is one connect in the common case. + Ok(mut addrs) => { + addrs.any(|addr| std::net::TcpStream::connect_timeout(&addr, PROBE_TIMEOUT).is_ok()) } Err(_) => false, } diff --git a/hyperdb-mcp/src/main.rs b/hyperdb-mcp/src/main.rs index 07d5eb2..b2e9426 100644 --- a/hyperdb-mcp/src/main.rs +++ b/hyperdb-mcp/src/main.rs @@ -233,8 +233,15 @@ async fn run_mcp_mode(cli: Cli) -> Result<(), Box> { // Eagerly initialize the engine before accepting tool calls so observer // tools like `status` report full stats on the first call (issue #138). // Errors are logged and swallowed inside `warm_up_engine` — startup - // proceeds even if hyperd is momentarily unreachable. - server.warm_up_engine(); + // proceeds even if hyperd is momentarily unreachable. Run on a blocking + // thread: warm-up does synchronous I/O (and may spawn the daemon) and + // would otherwise stall a runtime worker. Nothing else runs on the + // runtime yet (serve() is below), so this only delays startup. + let server = tokio::task::spawn_blocking(move || { + server.warm_up_engine(); + server + }) + .await?; let service = server.serve(rmcp::transport::io::stdio()).await?; service.waiting().await?;