fix(mcp): survive atomic binary replacement

[DavidMusk93]

Summary

• prefer the saved executable launch path when it still points to an executable
• reject a deleted /proc/self/exe path before spawning index workers
• add a Linux integration regression that forks, atomically replaces the running test binary, and verifies subprocess resolution

Tests

• build/c/test-runner httpd: 44 passed, 1 Windows-only skip
• scripts/run-tests-parallel.sh build/c/test-runner 16: 6386 passed, 1 unrelated baseline failure, 1 Windows-only skip

The baseline failure is tests/test_cli.c:6781 (installer must not follow symlinked agent roots outside the selected home). It is unrelated to the three files in this PR and is fixed by a separate local change, intentionally excluded here.

Runtime verification

A static build carrying this patch was loaded by both host and container MCP instances. A real index_repository call completed with 17,237 nodes, 99,775 edges, skipped_count=0, and parse_partial_count=0.

[Copilot]

Pull request overview

This PR hardens Linux subprocess spawning against atomic replacement of the running binary by preferring a previously saved launch path when it remains executable and refusing to use a deleted /proc/self/exe target before spawning index workers. It also adds a Linux regression test that exercises the “deleted self” scenario by forking, atomically replacing the on-disk executable, and validating that subprocess resolution still uses the saved launch path.

Changes:

• Update cbm_http_server_resolve_binary_path to prefer g_binary_path when it still points to an executable, and to reject non-executable self paths on Linux.
• Add a Linux-only “deleted-self” probe mode to the test runner so a fork/exec’d child can validate resolution behavior after atomic replacement.
• Add a Linux integration regression test covering atomic replacement while the child is running.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
tests/test_main.c	Adds a Linux-only probe entrypoint used by the new integration test to validate binary-path resolution in a fork/exec child.
tests/test_httpd.c	Adds a Linux regression test that forks, atomically replaces the running executable on disk, and asserts resolution prefers the saved launch path.
src/ui/http_server.c	Adjusts binary-path resolution to prefer the saved executable path when valid and to avoid returning a deleted /proc/self/exe target as spawnable.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[DeusData]

Thanks for this, and sorry for the slow acknowledgement. Queued for review.

Two things to clear first: the branch is CONFLICTING against main, and CI shows 2 failing checks. A rebase is worth doing before diagnosing the failures, since some may be drift rather than the change.

Surviving atomic binary replacement is a real scenario — self-update and package-manager upgrades both hit it — so this is a welcome area to harden.

[DeusData]

Reviewed in depth — and I want to be upfront that the news is awkward: the work is good, the bug is real, and a rebase alone will not restore what your fix does. That needs a maintainer decision, which I have raised.

Your diagnosis is correct and I verified it in source. cbm_http_server_resolve_binary_path skips the argv0 checks when called with NULL from the index supervisor, goes straight to resolve_self_executable, and on Linux that is readlink("/proc/self/exe"). After the installer's atomic rename-over, that returns "/path/to/bin (deleted)" — a path that does not exist — and http_server.c:963 hands it back unvalidated, so the worker spawn fails with ENOENT and index_repository breaks from a long-running server. Corroborating detail I liked: g_binary_path is currently write-only on main — set in three places, read nowhere. Your change makes it meaningful again.

The integration test is exactly the right shape. Copy the running binary to a launch path, fork and exec it as a probe, pipe-handshake, atomically rename() a replacement over it, then assert resolution. That is genuine reproduce-first work, and it is binding on Linux: without the source change the probe resolves the dead (deleted) string and exits 44.

What changed underneath you. Your base is 7d6cdb2 — 299 commits behind current main. The daemon rework since then added a build-fingerprint gate: the supervisor passes a content hash of its own running image (src/mcp/index_supervisor.c:603), and the spawned worker refuses on mismatch (index_supervisor.c:216-219, fail-closed by design).

Your fix prefers the saved launch path — which, after replacement, holds the new build. So the old daemon would spawn the new binary, and that gate would reject it. The net user-visible outcome on current main would still be a failure, just a different one: a deliberate fingerprint refusal instead of an ENOENT. That is why this is a semantic rebase rather than a mechanical one.

A fingerprint-compatible variant exists on Linux: spawn the literal /proc/self/exe rather than resolving it to a string. The magic link execs the original inode even after deletion, so parent and worker stay the same build and the gate is satisfied. macOS has no equivalent once the old inode is unlinked, which pushes that platform toward daemon-restart-on-replacement instead.

Which of those we want is a maintainer call — same-build via the magic link, fail-closed until restart, or new-build workers as you have it — so I have put the question upstream rather than deciding it in review. Please hold off reworking until there is an answer; a 299-commit rebase is a lot to spend on semantics that might change.

One thing is worth keeping regardless of that decision: the is_executable_file guard on the self-resolved path. Returning a non-existent path unvalidated is wrong under any of the three designs.

Two smaller notes. There is a latent aliasing bug: cbm_http_server_set_binary_path resolves into g_binary_path while your new code reads it, so a second call with a non-directly-executable path would snprintf a buffer into itself — overlapping copy, undefined behaviour. Resolving into a local, or guarding out != g_binary_path, avoids it. And the test is #if defined(__linux__) with a silent PASS elsewhere, so the title's "survive atomic binary replacement" is Linux-only in substance.

Thank you for this — the reproduction is the hard part and you did it properly.