fix(mcp): recover unsent requests after daemon loss

[Pxx500]

this pull request was generated entirely by AI
i can't guarantee its quality, but i've tried to make it as good as possible
i'm happy to revise it if it doesn't meet the repository's standards

What does this PR do?

recovers the stdio MCP frontend when its shared daemon disappears before an application frame is sent
the runtime now reports whether the frame crossed the local transport, which lets the frontend restore its session and retry exactly once only when replay is safe
regression coverage includes daemon replacement, cancellation during reconnect, and shutdown ownership during reconnect

related to #1182

this PR focuses on restoring request handling after daemon loss, it restores context and UI settings but leaves initialize-driven auto-indexing and background activation for a separate follow-up

the recovery tests use fork-based process isolation to exercise the frontend and runtime code shared by POSIX and Windows, they don't cover Windows named-pipe behavior end to end, that remains a test coverage gap

Checklist

• Every commit is signed off (git commit -s) (required, CI rejects unsigned commits; DCO, see CONTRIBUTING.md)
• Tests pass locally (make -f Makefile.cbm test)
• Lint passes (make -f Makefile.cbm lint-ci)
• New behavior is covered by a test (reproduce-first for bug fixes)

[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.

[Pxx500]

it looks like the three failed checks came from a race condition, could you please rerun the failed jobs?

[DeusData]

Thank you for the detailed investigation and for being transparent about the implementation. This changes request sending, reconnect, replay, cancellation, and shutdown ownership, so we are reviewing the correctness and safety boundaries carefully. We will come back with a maintainer decision, including how to handle the failed jobs, once that review is complete. Our review queue is currently full, so this may take a little time. Thank you for your patience and for offering to revise the change.

[Pxx500]

as far as the usability is concerned I have not experienced any problems/crashes since implementing this locally over the last 5 days

[DeusData]

Thanks for the rebase yesterday — the substance holds up under a hostile read, and that is the part that matters most for a recovery path. Replay safety is sound: request_sent_out is captured at runtime.c:3088 right after the frame send and before the cancel-frame reassignment, send_frame reports true only once header and payload are fully written, the Windows wait_pending treats a write completed before CancelIoEx as sent and a cancelled partial write as poisoned, the retry happens exactly once (frontend.c:596 is not re-entered), and a cancel during recovery is read under the mutex before the token reserve. So a request is replayed only when no complete frame can have reached the daemon, and index_repository / manage_adr / delete_project cannot be duplicated. With a local test adaptation (below), daemon_frontend passes 16/16 four times over, the eight daemon/runtime/IPC/bootstrap suites 230/230, mcp + cli 639/0, and reverting the runtime change makes exactly your three new tests fail — they bind.

Four things before it can merge, the first two blocking:

1. The head does not compile the test-runner (same on CI run 33971609329: every test leg red, lint/smoke green). tests/test_daemon_frontend.c:214-217 — frontend_idle_run is main's seams-only idle-observer test from Every idle MCP client burns ~0.7 core since 0.9.1-rc.1 (0.9.0: 0%) — N concurrent sessions cost N cores, Windows #1764 and still calls the four-argument cbm_daemon_frontend_mcp_run; your signature has five. The adaptation is mechanical — add a const cbm_daemon_frontend_session_config_t *session to frontend_idle_run_t, keep the build fingerprint and identity in frontend_idle_fixture_t, and build the session in daemon_frontend_idle_uses_one_maintenance_observer:

cbm_daemon_frontend_session_config_t session = {
    .bootstrap = { .role = CBM_DAEMON_PROCESS_MCP_CLIENT,
                   .endpoint = fixture.maintenance.endpoint,
                   .identity = &fixture.identity,
                   .executable_path = "unused",
                   .connect_timeout_ms = 30000, .startup_timeout_ms = 30000 },
    .session_root = fixture.maintenance.parent,
    .tool_profile = CBM_MCP_TOOL_PROFILE_ALL,
};

(with fixture.identity / fixture.build_fingerprint stored on the fixture instead of stack locals, and .session = &session on the run struct). That is the whole delta; I verified it builds and the suite is green.

2. FRONTEND_RECOVERY_CANCEL orders "cancel routed before release" with cbm_usleep(100000) (~test_daemon_frontend.c:507-511). Under CI load the routing can take longer than 100 ms, the retry path runs, and response_exact fails — a test whose verdict depends on timing is a lottery for us, not a test. Please make the handoff deterministic the way frontend_test_worker_idle_cycles does: a test-seam counter of routed cancellations the test waits on, never a sleep.

3. Minor: recovery calls cbm_daemon_bootstrap_execute from the worker, and on failure bootstrap_production_diagnostic writes to stderr (bootstrap.c:1023-1040), which contradicts frontend.c's own "never log from the worker" rule. Failure path only, right before _Exit, so a note is enough — either route it through the frontend's reporting or say why the exception is acceptable.

4. Scope questions, your call as long as they are answered in the PR text: recovery replays SET_CONTEXT and the UI configuration but not the initialize-driven session state (detect_session / maybe_auto_index, pending_background_initialize), so after a daemon replacement the new session gets no auto-index or background activation — in scope here, or a follow-up you'd file? And Windows/Codex: MCP tool calls close transport while CLI/stdio init work; update -y still prompts for binary variant #1182 is a Windows report while the three new tests are #ifndef _WIN32 (fork-based); a Windows-exercisable variant, or the reason the POSIX tests cover the shared path, would close that gap.

Keep the sign-offs on the rebased commits and this merges on green.

[Pxx500]

looks like another race condition in the Windows cold-start test, one of six concurrent clients failed to create its CLI coordination endpoint, could you please rerun the failed job?

[Pxx500]

@DeusData (ping for visibility)

[Pxx500]

all green 👍 @DeusData

[DeusData]

Thank you for your patience — twelve days after "all green" is longer than this deserved, and the delay is ours. Here is where it stands, concretely.

All four review items are addressed, and I have checked each against the code rather than the description:

1. The test-runner compiles: frontend_idle_run carries the session config, as suggested.
2. The cancel handoff is now deterministic — FRONTEND_RECOVERY_CANCEL waits on cbm_daemon_frontend_test_routed_cancellations() moving past its starting value before releasing the first request, and the cbm_usleep(100000) is gone. That is exactly the shape I asked for.
3. The worker's bootstrap call now carries a comment explaining why the stderr diagnostic on the failure path is the accepted exception to "never log from the worker".
4. The PR text answers both scope questions plainly: initialize-driven auto-index / background activation is left for a follow-up, and the fork-based tests cover the code shared by POSIX and Windows but not named pipes end to end. Stating a coverage gap openly is worth more than hiding it.

All three commits are signed off; the two Merge branch 'main' commits carry no sign-off, which is correct — DCO does not apply to merge commits.

One thing I missed on 5 September, and it is on me, not you. FRONTEND_RECOVERY_CLEAN_EOF orders "shutdown overlaps recovery" by sleeping FRONTEND_RECOVERY_EOF_RELEASE_MS = 16 s so as to land past the frontend's 15 s FRONTEND_EOF_DRAIN_MS. That is the same class of problem as item 2 — two unsynchronised clocks with a one-second margin, plus sixteen seconds of wall time on every lane — and it was already there in the version I reviewed, so I am not going to turn it into a new condition after you did everything that was asked. I will make that handoff observable through a test seam (the way you did for the cancel case) in a follow-up of our own, with credit to you for the test it builds on. You do not need to do anything for it.

What happens next. main has moved 158 commits since your last merge, so before anything lands I build and run the daemon suites on the merge result — your branch merges cleanly by text, but this week taught us twice that a clean textual merge can still be a wrong one. Separately, main itself currently has two reds that are not yours (a linter ratchet and a worker-policy script, both fixed by #2257); your CI will show them until that lands. Once #2257 is in and the merge result is verified, this goes to the maintainer's merge decision — it is a recovery path in the daemon, so it gets a deliberate yes rather than an automatic one.

You labelled this PR as AI-generated in its first line and then stood behind every claim in it, including five days of running it yourself. That is precisely how we would like such contributions to be made.

[DeusData]

An update, and one small thing that is new since the review — caused by a rule that landed on main after you wrote this, not by anything you did wrong.

I brought your branch up to date with main this morning, so CI ran against today's tree. Everything that ran is green; lint / lint is red, and because the test stage only starts once lint passes, the matrix did not run. The reason:

memory-core linter FAILED: raw allocator use grew.
  src/daemon/frontend.c: grew by 1 (12 -> 13); latest sites: 821:free

scripts/lint-memory-core.py (on main since 19 September) is a ratchet: each file has a recorded count of raw malloc/free/strdup sites and that count may only go down. Your recovery path adds one — the free(response); before the retry in frontend_worker (line 609 on your branch). The call is correct: response is raw heap memory from cbm_daemon_application_client_mcp_tagged, so it must be released with free, not routed through the memory core. The ratchet just wants the file's total to stay at 12.

The cheapest honest fix: the same buffer is already freed at the bottom of the loop (line 635). Put the release behind one tiny helper and call it from both places —

static void frontend_response_reset(char **response, size_t *response_length) {
    free(*response);
    *response = NULL;
    *response_length = 0;
}

— which replaces two raw sites with one (13 − 2 + 1 = 12, exactly the recorded count), and also removes the three-line reset you currently spell out before the retry. Please do not raise the number in scripts/memory-core-baseline.txt; it only ever tightens (if a change ever brings the file below 12 the linter asks you to lower it, and then you should).

Once that is pushed, lint goes green and the full matrix runs on your branch for the first time against current main. Everything else from my last comment stands: all four review items verified, the 16-second sleep in the clean-EOF test is ours to replace in a follow-up, and after a green run plus a local build of the merge result this goes to the merge decision.

Thank you for bearing with a moving target.

[Pxx500]

@DeusData this PR has been in the works for longer than anticipated and I'm forgetting about it, can we finally get it merged (or if you have any more small reviews just fix them yourself and merge)?

[DeusData]

Thank you, @Pxx500, and thank you for sticking with this through a moving target. The helper is exactly what the ratchet needed: both release sites now go through frontend_response_reset(), frontend.c is back at its recorded count, and the memory-core linter passes on your branch and on its merge with current main. Every ask we've made on this PR is now addressed.

What's left is on our side, as promised: let the full matrix finish on this head, build and run the daemon suites on the merge result locally, and then the maintainer's merge decision. It's a recovery path in the daemon, so it gets a deliberate yes rather than an automatic one. The clean-EOF test's 16-second handoff stays our follow-up, with credit to you. You don't need to do anything more unless that verification turns something up, and if it does we'll say so here right away.