chore(mcp): hyperd_running false positive when health port unreachable in daemon mode#145
Merged
StefanSteiner merged 4 commits intoJun 16, 2026
Conversation
…e in daemon mode 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 tableau#138
…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.
…review) Reworks the two prior fixes after adversarial review surfaced a latent false-positive and a tableau#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 tableau#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 tableau#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.
Two cheap follow-ups from the final review of the tableau#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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #138. Two related daemon-mode status-correctness bugs, with fixes refined after adversarial review.
Fix 1:
hyperd_runningwas unreliable in daemon modeRoot cause:
is_running()delegated entirely todaemon::discovery::discover(), which readsdaemon.jsonand PINGs the health port. If the health port was unreachable (stale discovery file, port-scan-adopted daemon, firewall),discover()returnedNoneandhyperd_runningwas reported asfalse— even while the engine was actively serving queries over its live libpq connection.Fix (issue's "option 2"): In daemon mode,
is_running()now probes the cached libpqdaemon_endpointdirectly 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:hyperdon a new port, leaving the cached endpoint stale — the probe then correctly reportsfalse(an earlier draft that returnedtruewhenever an endpoint was cached would have reported a dead endpoint as alive).Falls back to
discover()only when no endpoint has been cached yet.Fix 2:
statusreportedengine_busy: trueindefinitely for observer-first clientsRoot cause: The engine was initialized lazily, only on the first data-plane tool call. Clients like Windsurf/Devin that call
statusas their first tool never triggered initialization, sostatusreturnedengine_busy: trueforever.Fix (root-cause, not symptom): Initialize the engine eagerly at server startup via a new
warm_up_engine()(called frommain.rsbeforeserve()), and keepstatusa pure, non-blocking observer (try_lock; degraded response if the engine is missing or the lock is held). This honors issue #118 (statusnever blocks behind a slow data-plane op) and fixes the whole class of "observer-first" tools, not juststatus.warm_up_engine()logs and swallows errors so the server still starts ifhyperdis briefly unreachable; the first data-plane call retries init viawith_engine().Review notes
Both fixes were reworked after an adversarial review caught:
is_running()(cached endpoint treated as authoritative even after a daemon-sidehyperdrestart), andstatus()(which had called the blockingensure_engine()).Known minor (not addressed, by design):
status_degraded()still deriveshyperd_runningfromdiscover()because it deliberately avoids the engine lock and therefore can't reach the cached endpoint. The two paths can momentarily disagree only while another tool holds the engine lock;discover()is the correct independent signal in that case.Test plan
cargo fmt+cargo clippy --all-targetscleanhyperdb-mcplib unit tests passcargo build --releasesucceedsstatusreturnshyperd_running: true+ full payload withengine_busy: falseas the first tool call from a fresh session