[Minervaowl7]

Fixes #2796

Problem

When the shared daemon dies under a live MCP frontend, the worker's exchange fails and the current path calls _Exit(EXIT_FAILURE) — "a failed daemon transport cannot wake a thread blocked in stdio portably". The agent sees server EOF, and every pooled connection to that daemon breaks at once. On Windows this triggers whenever a client reaps its spawned endpoint tree: DETACHED_PROCESS detaches the console, not the process tree, so the shared daemon dies with the endpoint that spawned it and takes every attached frontend with it (two incidents with full client/server logs in #2796).

Fix

Recover on the worker, where the request is already in hand — the stdin reader is never touched:

• New cbm_daemon_frontend_daemon_lost_fn callback on cbm_daemon_frontend_mcp_run (NULL keeps the legacy fatal behavior; all existing call sites pass NULL). The callback must return a fresh, open client distinct from every client returned earlier in the run; the current and already-retired clients are rejected, and the frontend takes ownership of a successful replacement.
• Only CBM_DAEMON_RUNTIME_APPLICATION_TRANSPORT_ERROR retries. BUSY, UNAVAILABLE, REJECTED, HANDLER_ERROR and CANCELLED are daemon answers and keep their existing meaning — in particular, a cancelled request is never re-issued as a fresh one. Bounded by FRONTEND_DAEMON_RECONNECT_MAX = 3.
• A replayed request may already have executed in the dead daemon (a transport failure cannot distinguish "never sent" from "reply lost"), so the interrupted call has at-least-once semantics; frontend.h states this explicitly.
• Lifetime: the replaced client is close_begin'd at swap time but allocated until the run tail. The stdin reader and maintenance monitor can hold a pointer read before the swap, so close_finish runs after the worker is joined and the monitor is stopped. All non-worker client reads are snapshotted under the state mutex.
• Cancellation survives the reconnect: the published token is invalidated while the callback runs, a matching cancel is remembered in cancel_pending, and the retry delivers it against the freshly minted token. The retry reserves and publishes that token under one lock, and CANCEL_ERROR no longer tears down the reader — the in-flight exchange observes the same failure and the worker owns recovery. A maintenance stop that lands in the reconnect window takes the monitor's bounded grace instead of exiting immediately.
• Liveness: the EOF drain and the join watchdog share the new frontend_liveness policy module. Each keeps its own ordinary window (15 s drain, 5 s watchdog); a replacement attempt extends both to one 120 s grace anchored at the attempt's real start, so a hung supplier cannot spend 2×120 s across the two waiters. A late re-sample grants the grace once (idempotent), completed-item progress never extends it, and _Static_asserts require the grace to cover both windows. The attempt timestamp and active flag are published before the attempt counter (release/acquire), so a waiter that observes a new attempt cannot pair it with the previous attempt's timestamp.
• main() supplies the callback. Initial startup and the reconnect supplier now share main_establish_mcp_session() (bootstrap + upgrade retry + session context); session_live switches only bootstrap failure reporting, so the reconnect path cannot drift from startup. The callback returns NULL immediately once g_shutdown is set (SIGTERM/SIGINT), so shutdown never cold-starts a daemon. It logs the reconnect through cbm_log_warn and reports failures on stderr only — main_report_client_bootstrap_failure() would inject an unsolicited id:null JSON-RPC frame into the live session's stdout; endpoint failures include the validation detail. The POSIX fork child calls the shared cbm_posix_close_fds_from() (close_range → closefrom → bounded loop, with the 436 syscall fallback) and receives a cap from cbm_posix_fd_close_cap() read in the parent, so the child stays async-signal-safe while the callback runs on a worker thread. UI flags are not re-applied (the daemon persists its UI configuration).

Architecture follow-up included here

The reconnect fix made three existing seams visible; this PR deepens them because they are all on the new code path:

• src/daemon/frontend_liveness.{c,h} — one deterministic no-progress policy for both waiters. Previously the two loops disagreed: the drain granted a cold-spawn grace while the 5 s join watchdog killed the same worker.
• foundation/subprocess.{c,h} — cbm_posix_close_fds_from() and cbm_posix_fd_close_cap() are now the single fork-child descriptor policy shared by subprocess spawn and daemon bootstrap. The duplicate copy in bootstrap.c (which had already drifted: no test seam, a different cap source) is deleted.
• src/main.c — one endpoint-to-session path (main_establish_mcp_session) and one endpoint-failure reporter shared by startup and reconnect.

Deferred deliberately: a frontend_session value type owning the client pointer, token, cancel_pending and retired clients is the right next refactor, but it moves ~200 lines and belongs in a follow-up where the diff is the refactor rather than the bug fix. A maintenance activation still fail-stops on the monitor's own shorter grace rather than waiting 120 s for a reconnect; that trade-off is documented in frontend.h.

Verification

• New regression tests (fork-isolated harness, in-process services):
  • daemon_frontend_reconnects_after_transport_failure — the first service is stopped, the supplier returns a live replacement, and the request is answered exactly once by it.
  • daemon_frontend_retries_across_two_replacements — the first replacement's daemon is also dead; the second callback supplies a live client (two swaps, two retirements, one answer).
  • daemon_frontend_does_not_retry_daemon_cancellation — a CANCELLED answer reaches the client and the supplier is never called; fails if the retry gate is widened.
  • daemon_frontend_fail_stops_without_a_replacement — a NULL supplier fail-stops, with a marker file proving the callback ran before _Exit.
  • daemon_frontend_rejects_returning_the_dead_client — returning dead_client is rejected without a double close.
  • daemon_frontend_liveness_treats_reconnect_as_progress — drives the policy with a synthetic clock: ordinary expiry, item progress, grace without a counter change, a late waiter sharing the same attempt budget, a new attempt while the previous one was observed, a timestamp newer than the sample, idempotent late re-sampling, and UINT64_MAX saturation.
  • subprocess_fd_close_cap_is_bounded_for_fork_children — the shared cap never disables the sweep or exceeds the clamp.
• scripts/test.sh --suites daemon_frontend — 20 passed (sanitizer build). daemon_frontend,daemon_bootstrap,subprocess — 104 passed, 2 skipped.
• End-to-end on WSL (gcc 13.3), real daemon, kill -9 mid-session: the interrupted request is served by a re-spawned daemon and stderr carries one level=warn msg=mcp.frontend.daemon_lost_reconnect; pre-fix the same scenario exits 1 and the request is lost.
• scripts/build.sh production build succeeds.
• clang-format --dry-run --Werror clean on every changed source; test_daemon_frontend.c has pre-existing 18.1.3 drift outside the added block.

Files

src/daemon/frontend.c, src/daemon/frontend.h, src/daemon/frontend_liveness.c, src/daemon/frontend_liveness.h, src/main.c, src/daemon/bootstrap.c, src/foundation/subprocess.c, src/foundation/subprocess.h, Makefile.cbm, tests/test_daemon_frontend.c, tests/test_subprocess.c. No public API or dependency changes.

Residual gaps (for review, not blockers)

• No process-level test that a supplier blocked past the 5 s watchdog window survives: the policy is unit-tested directly, and the watchdog wiring passes the same atomics, but the end-to-end path is not reproduced deterministically.
• A maintenance activation can still end the process inside a replacement spawn on the monitor's own grace. A detached spawn child is safe to orphan; making the update window reconnect-aware would need a new monitor callback.

[DeusData]

Hi @Minervaowl7, welcome, and thank you for your first contribution here. This is careful work: you found exactly the right place for recovery (the worker already holds the request, so the stdin reader never needs to be woken), you kept the legacy behavior behind a NULL callback, you bounded the retries, and your commit message explains the reasoning clearly. Your report in #2796, with timelines from both the client pool and the daemon log, is one of the most useful lifecycle reports we have received.

One thing you could not have known from the issue: an older PR, #1784, implements the same recovery path (replace the daemon client after a transport loss and retry on the worker). It has been through several rounds of maintainer review since August and is further along. Both PRs change the signature of cbm_daemon_frontend_mcp_run and touch the same four files, so they cannot both land as they are. We would rather tell you that now than let your PR wait without an answer.

Because the two designs differ, here is what the review of #1784 settled, in case it helps you and for the record on this PR:

1. Retry only on a real transport loss. Here the reconnect runs on every non-OK status, including CANCELLED. When an agent cancels a running request, the daemon answers CANCELLED, and this branch would then close the healthy session, reconnect and run the cancelled request again. REJECTED and HANDLER_ERROR are answers from a live daemon too, so retrying them only repeats the same answer after a reconnect.

2. Re-send only a request that never reached the daemon. If the daemon dies while it is executing a tool call, sending it again can run the call a second time (a re-index, an ADR write, a project delete). A request that itself brings the daemon down would be replayed against each replacement daemon, which every other attached session shares. fix(mcp): recover unsent requests after daemon loss #1784 records whether the request frame was actually sent and retries once, only when it was not.

3. Client ownership during the swap. The callback closes the old client before the frontend has committed the swap. If the swap is then refused because teardown has started, or the callback fails while the frontend is stopping, state->client still points at the closed client and teardown closes it a second time. The stdin reader (cancellation) and the maintenance monitor also read state->client outside the mutex. fix(mcp): recover unsent requests after daemon loss #1784 detaches the old client under the mutex first and routes cancellations under the mutex.

4. Two smaller items: frontend.c goes from 12 to 13 raw allocator call sites (the free before the retry), and the memory-core ratchet in our lint job fails on any growth. There is also no automated test for the new path yet; every existing call site passes NULL, so the suite stays green without exercising it.

Two places where your help would make a real difference:

• Windows. fix(mcp): recover unsent requests after daemon loss #1784's tests are fork-based and POSIX-only, so named pipes are not covered end to end. If you could run your ZCode lab repro, and the variant that kills only endpoint A, against the fix(mcp): recover unsent requests after daemon loss #1784 branch on Windows and post the result there, that is exactly the evidence we are missing.
• The open question on Windows: killing one endpoint's process takes down the shared daemon and every attached connection ("Connection closed") #2796. Whether killing only endpoint A takes the daemon down, and whether ZCode uses a process-tree or job kill, decides whether we also need the Windows-side change your issue suggests (keeping the shared daemon out of the client's job). Neither PR addresses that trigger.

We will keep this PR open while #1784 goes through its decision and come back here once that is done. Thank you again, both for the fix and for explaining it so clearly.

[github-actions]

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

• It will not be closed for inactivity. No stale bot touches pull requests here.
• It may still sit a while before a human reads it. That is on us, not on you.
• Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

• Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
• Get CI green, or say which failures you believe are pre-existing.
• Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
• Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

[Minervaowl7]

Revised after auditing the first revision (force-pushed to 5647405):

• Retry gate narrowed to TRANSPORT_ERROR only. The first revision retried any non-OK exchange, which would have re-issued CANCELLED requests and torn down a healthy daemon on HANDLER_ERROR/BUSY/REJECTED.
• Client lifetime fixed. The replaced client was freed inside the callback, while the stdin reader and the maintenance monitor can still hold a pointer read before the swap (use-after-free). It is now close_begin'd at swap time and close_finish'd in the run tail, after the worker is joined and the monitor is stopped.
• Two regressions added: transport-failure reconnect (exactly one callback, request served by the replacement) and CANCELLED-no-retry (fails when the retry gate is widened). scripts/test.sh --suites daemon_frontend -> 16 passed.

The real-daemon kill -9 repro is still GREEN with one daemon_lost_reconnect warning.

[Minervaowl7]

Reviewed this change with six OCR passes (high/medium effort) and fixed every critical/high/medium finding. Final commit: a13b4a3.

Fixed across passes

• A failed reconnect wrote an unsolicited id:null JSON-RPC error to stdout (the agent's live response stream) → stderr only.
• The retry gate re-issued CANCELLED requests and tore down a healthy daemon on HANDLER_ERROR/BUSY → only TRANSPORT_ERROR retries.
• The replaced client was freed inside the callback while the stdin reader and maintenance monitor could still hold its pointer → close_begin at swap, close_finish in the run tail after join/monitor stop; non-worker reads snapshotted under the state mutex.
• A cancellation arriving during the reconnect was dropped → cancel_pending + delayed delivery with the new token; CANCEL_ERROR no longer tears down the reader; a maintenance stop in that window takes the monitor's grace.
• The EOF drain could be defeated by a slow or hung spawn → per-attempt atomic generation counter and a bounded 120 s grace.
• A supplier returning the current or an already-retired client could be double-freed → rejected before the swap.
• The Linux spawn path forked from a worker thread and still called sysconf() → close_range/closefrom with a getrlimit fallback, keeping the child async-signal-safe.
• The reconnect ignored g_shutdown, and the global client handle stayed dangling for the whole run → both fixed.

Verification after the final pass

• scripts/test.sh --suites daemon_frontend — 19 passed (sanitizer build).
• scripts/build.sh — production build OK.
• Real daemon kill -9 mid-session — request served by a re-spawned daemon, one daemon_lost_reconnect warning.
• clang-format --dry-run -Werror clean on the changed sources (the test file's pre-existing drift is untouched).

Known residual gaps (documented, not correctness blockers)

• No automated test for the cancel_pending/DEFERRED path (needs deterministic timing) or for reconnect-budget exhaustion; both are reviewed and the budget is a simple bound.
• At-least-once replay of an interrupted call is an accepted trade-off (stated in frontend.h).
• The 120 s EOF-drain grace is a hard cap; raising CBM_STARTUP_TIMEOUT_MS beyond it can cut a batch-client reconnect.

[Minervaowl7]

Architecture pass on the reconnect fix

Ran an improve-codebase-architecture pass over this PR, then two OpenCodeReview rounds over the new diff. The reconnect design is unchanged; the pass concentrated three policies that the fix exposed and fixed two real defects found by the second review round.

Deepened

• src/daemon/frontend_liveness.{c,h} — the EOF drain and the join watchdog now share one deterministic no-progress policy. Each keeps its ordinary window (15 s drain, 5 s watchdog); a replacement attempt extends both to one 120 s grace anchored at the attempt's real start. Before this, the two loops had drifted: the drain granted the spawn grace while the 5 s watchdog could kill the same worker mid-spawn.
• foundation/subprocess.{c,h} — cbm_posix_close_fds_from() and cbm_posix_fd_close_cap() are the single fork-child descriptor policy for both subprocess spawn and daemon bootstrap. The duplicate copy in bootstrap.c (already missing the ENOSYS test seam and using a different cap source) is gone; the cap is read in the parent before fork().
• main_establish_mcp_session() — initial startup and the reconnect supplier share one endpoint → bootstrap + upgrade retry → session-context path. session_live switches only bootstrap failure reporting, so the reconnect path cannot drift from startup.

Defects found and fixed by the second review round

• The drain and the watchdog each anchored the 120 s grace at their own first poll, so one hung supplier could spend ~240 s. The attempt start is now published in frontend_state_t.reconnect_started_ms and both waiters use it; a late waiter spends the same budget.
• The attempt counter was incremented before the timestamp/active flag were published, so on a weak-memory platform a waiter could pair the new counter with the previous attempt's timestamp and compress the new grace. The timestamp and flag are now published first (release), then the counter (release), and waiters read it with acquire.
• reconnect_late() is idempotent and returns whether it granted a grace; the caller no longer reads the policy struct's internal field.
• The exported fd-sweep clamps max_fd to INT_MAX internally, so an out-of-contract caller cannot make the int loop wrap in the fork child.
• The join watchdog also checks frontend_worker_is_done() before fail-stop, covering the window between the worker marking itself done and the main thread publishing complete.

Tests added

• daemon_frontend_liveness_treats_reconnect_as_progress — synthetic clock: ordinary expiry, item progress, grace without a counter change, a late waiter sharing the same attempt budget, a new attempt while the previous one was observed, a timestamp newer than the sample, idempotent late re-sampling, and UINT64_MAX saturation.
• subprocess_fd_close_cap_is_bounded_for_fork_children — the shared cap never disables the sweep or exceeds the clamp.

Verification

• scripts/test.sh --suites daemon_frontend — 20 passed (sanitizer build).
• daemon_frontend,daemon_bootstrap,subprocess — 104 passed, 2 skipped.
• scripts/build.sh — production build succeeds.
• Real daemon kill -9 mid-session — re-spawned daemon serves the interrupted request; one daemon_lost_reconnect warning; VERDICT: GREEN.
• clang-format --dry-run --Werror clean on every changed source; the pre-existing drift in test_daemon_frontend.c remains untouched.

Residual gaps (not blockers)

• No process-level test that a supplier blocked past the 5 s watchdog window survives. The policy is unit-tested directly and the watchdog wiring passes the same atomics, but the end-to-end path is not reproduced deterministically.
• A maintenance activation still fail-stops on the monitor's own shorter grace inside a replacement spawn (a detached spawn child is safe to orphan). Making the update window reconnect-aware would need a new monitor callback.
• A frontend_session value type owning the client pointer, token, cancel_pending and retired clients is the right next refactor; it moves ~200 lines and belongs in a follow-up where the diff is the refactor, not this bug fix.

[Minervaowl7]

Thanks for the detailed review — and for the pointer to #1784. I've read it, and I'm not going to push for both to land. If #1784 is the one you want, I'll close this; I'd rather the fix land than my version of it.

A quick update since your comment: it was based on the first revision (5647405). The branch is now at 9b280d4, and three of your four points are addressed:

• The retry gate is now TRANSPORT_ERROR only. CANCELLED, REJECTED, HANDLER_ERROR, BUSY and UNAVAILABLE come back unchanged, and a test fails if the gate is widened.
• The callback no longer closes the dead client. The frontend commits the swap under the mutex, close_begins the old client, keeps it in retired[] until the run tail, and close_finishes it after the worker is joined and the monitor stopped. Cancellation and the maintenance monitor snapshot (client, token) under the same mutex. A refused swap closes the replacement, not the current client, so there's no double close.
• frontend.c still has 12 raw allocator sites (same as baseline; lint-memory-core.py passes), and the reconnect path now has tests: five fork-isolated cases, the liveness policy, and the fd cap. daemon_frontend is 20 passed; the three affected suites are 104 passed / 2 skipped.

Point 2 is the one that's still a real difference, and you're right that #1784's rule is safer. Without runtime support I can't tell "never sent" from "reply lost", so this branch replays with at-least-once semantics — stated in frontend.h and in the PR. For destructive tools that's a genuine trade-off, not something I can hand-wave away.

On #2796: you're right that the current lab repro kills the daemon directly. The incident logs only show the victim endpoint vanishing without a graceful path; I haven't yet isolated "kill only endpoint A" or confirmed whether ZCode uses a process-tree/job kill. I can run both variants against #1784 on Windows and post the daemon/B outcome there. Two caveats: my lab currently has the keeper workaround holding the daemon, so I'd need to stop it for a clean run; and I don't have a native Windows build of #1784 yet (this branch was tested in WSL), so a Windows artifact or the preferred build command would help.

Heads-up: the branch also has an architecture pass on top (shared drain/watchdog liveness, one fork-child fd path, one startup/reconnect session path) — details are in the comment above.

Thanks again for the clear prior-art pointer and for keeping this open while #1784 is decided.