fix(mcp,runtime): idle-session timeout for stdio bridges; refuse duplicate daemon boot - #2230
fix(mcp,runtime): idle-session timeout for stdio bridges; refuse duplicate daemon boot#2230ohdearquant wants to merge 9 commits into
Conversation
A second `khived` boot attempt against a store already served by a live, responsive daemon exited Ok(()) after only an info-level log — indistinguishable from ordinary success. Two detached daemons could therefore coexist against one store indefinitely, each holding its own WAL connection and read marks. Boot now refuses loudly (non-zero exit) and names the incumbent's pid, while the existing stale-socket recovery path (dead pid, unreachable socket) is unchanged.
A stdio bridge whose client stopped talking but kept the pipe open ran forever: nothing distinguished it from a live session, so it held its reader-pool admission and DB connection indefinitely. `CancelOnEofTransport` now optionally treats "no request within the timeout" the same as a real EOF, tearing the session down through the existing graceful-close path that already releases pooled resources on disconnect. Configurable via KHIVE_BRIDGE_IDLE_TIMEOUT_SECS (default 3600s, 0 disables); a client that returns later simply respawns the bridge.
… daemon identity Wire silence is not session idleness: a handler admitted before the quiet window can still be running, and cancelling it lost a valid response. The timeout now fires only when no request is in flight. Duplicate-daemon detection likewise no longer treats any accepting socket as an incumbent; it validates the peer speaks the protocol before refusing, so a foreign listener still takes the stale-socket recovery path.
The idle-timeout in_flight counter previously decremented as soon as a response was handed to the inner transport's send(), before the framed write actually resolved. A slow or backpressured reader could leave that write pending after the counter had already dropped to zero, so the next idle check could tear the session down mid-delivery. Also stop discarding a partially received request when the idle check defers: the receive future is now created once and reused across every deferral instead of being recreated each time, so a second request written in fragments across an idle window is no longer lost.
…robe The duplicate-daemon protocol probe accepted any bytes that deserialized as a DaemonResponseFrame, including a config_mismatch or legacy fallback response — neither of which confirms the peer is the same live khived this process would defer to. It now requires the unambiguous probe-ack sentinel plus a matching protocol version and config_id, mirroring the identity check the client-side recovery probe already uses. Also move the Unix connect inside the probe's bounded timeout alongside the write/read, so a slow connect can no longer hold startup open past the advertised deadline.
…aemon probe A peer that admits a stdio-bridge request, then stops reading its response while keeping the pipe open, previously pinned the session forever: the idle-close deferral correctly protects an in-flight response, but nothing bounded how long that deferral could run. Worse, rmcp's underlying transport serializes writes through a lock held across the pending write's await, so even cancelling the session's root token could not by itself unstick a later close() on the same transport. CancelOnEofTransport::send now bounds a response/error write with a new, independently configurable response-delivery deadline (KHIVE_BRIDGE_RESPONSE_DEADLINE_SECS, default 300s). On timeout the write future is dropped -- releasing the lock the same way any future cancellation does -- the in-flight counter is decremented, and the session is cancelled directly rather than waiting for the next idle tick. Separately, the duplicate-daemon probe's acceptance predicate ignored two optional response fields (metrics, request_id), so a well-formed metrics-only response could be misread as a probe acknowledgement. The predicate now requires the exact probe-branch shape. Regression coverage: a peer that never reads its response is reaped within the response-delivery deadline; the duplicate-daemon probe rejects an otherwise-matching metrics-only response.
… duplicate-daemon probe The client-side identity probe accepted a response carrying a metrics snapshot or an echoed request_id as a valid probe acknowledgement, even though the server-side probe classifier already required both fields to be absent. Align the client-side predicate with the server-side one so both sides of the protocol agree on what "alive" means, and add a regression covering the metrics-snapshot case. Also split the existing metrics-only regression into two independently reddening cases (one per optional field) and assert that the fake listener's accept task completes successfully, so the oracle can no longer pass without the response actually being exchanged.
The response-delivery deadline used to treat 0 as "disable the bound", mirroring KHIVE_BRIDGE_IDLE_TIMEOUT_SECS=0. Unlike the idle timeout, an unbounded response-delivery deadline restores the exact defect this deadline exists to close: a peer that admits a request and stops reading pins the bridge's response write forever while the server advertises a bounded lifecycle. 0 is now a startup error naming the variable, the rejected value, and the accepted range (1..=u64::MAX), instead of a supported opt-out. KHIVE_BRIDGE_IDLE_TIMEOUT_SECS keeps its existing 0-disables semantics unchanged.
|
The regression test at The stated premise needs a binary that both predates
Every bump above
Control for the ancestry predicate: A version- The test itself is fine and should stay. It guards the What is inaccurate is the stated reason. As written, a reader is told the probe classifier Suggested reword of |
|
Closing in favour of #2281, which carries the daemon half of this branch cut fresh from current This branch mixed two independent changes — the duplicate-daemon boot refusal and a stdio-bridge idle-session timeout — across nine commits, and went conflicted against What moved to #2281: the duplicate-daemon refusal in What did not move: the stdio-bridge idle-session timeout, the in-flight request accounting, the response-delivery deadline, and the One note for whoever picks the bridge half up: the commit subjects on this branch do not partition the work. |
|
Correction to my previous comment, which was wrong on one point. I wrote that the stdio-bridge work here "is not abandoned — it is simply not carried by #2281 and needs its own branch from current Verified at
So both halves of this PR are resolved: the duplicate-daemon refusal moved to #2281, and the bridge half is superseded by already-merged work. Nothing here is stranded and no follow-up branch is needed. How I got it wrong: I checked that the bridge commits were absent from #2281 and reported them as unlanded, without checking whether they were present on |
2 participants
Closes #1921, closes #1874; addresses #1836 via the same idle-timeout seam (an idle bridge now releases its pooled resources instead of holding a live WAL connection for its lifetime).
CancelOnEofTransportgains an optional idle timeout (envKHIVE_BRIDGE_IDLE_TIMEOUT_SECS, default 3600s, 0 disables): a quiet window past the timeout is treated exactly like client EOF, driving the existing graceful-close path that already tears down the session and its pool handle. An admitted request with a response still being written — running long, or delivered slowly to a backpressured reader — defers that close instead of being cancelled out from under it; a request is only counted as "still awaiting its response" until its write has actually finished, not merely been handed off. Cancellation-safety of racing away the in-flight receive verified against the transport's buffering (state lives inself, not the polled-away future). Daemon mode is unaffected (it never reachesserve_stdio).KHIVE_BRIDGE_RESPONSE_DEADLINE_SECS(default 300s; accepted range is 1 second and up —0is rejected at startup, naming the variable, the rejected value, and the accepted range) caps how long a single response write may stay pending before it is abandoned and the session closed anyway. Without it, a peer that admits a request and then simply stops reading its response — while leaving the connection open — would keep that write pending forever and the idle timeout would defer indefinitely, never reaping the session.kkernel mcp --daemonagainst a store already served by a live, responsive daemon now fails loudly: error names the incumbent pid and socket path, process exits non-zero. The stale-socket recovery path (dead pid or unreachable socket) is unchanged. The liveness probe requires an exact protocol-acknowledgement shape (not just any well-formed response) so it can't mistake an unrelated response — for example a metrics snapshot — from the same daemon for the acknowledgement it's looking for.Tests: idle timeout cancels without EOF; intermittent traffic is not reaped; an admitted long-running request is not cancelled by wire idleness; a backpressured response write defers idle-close; a response write that never resolves is still reaped within the response-delivery deadline; a
0response-delivery deadline is rejected at startup rather than accepted as an unbounded opt-out; duplicate boot refuses while the first daemon is live, including against a metrics-only response (verified to fail without the fix). Full khive-runtime suite green; khive-mcp/kkernel checks clean.