feat(mcp): add CBM_IN_PROCESS to serve stdio MCP without the daemon - #2072
pmaxvsbobo wants to merge 4 commits into
Conversation
0664442 to
bae6977
Compare
|
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. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
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. |
|
Thanks for this, and for measuring it (31 s to 4.6 s is a real pain point, and the analysis of why the failure is misreported is right: the connect result carries no errno, so an
No decision yet on the mode itself; the maintainer wants to think about it with your answers in hand. Thank you again for the careful write-up. |
|
Thanks — you're right about the lease, and it was worse than "no lease is acquired". Fixed in What
|
|
Answers to your questions: I can't tell you how common this is beyond my own org. But the pattern — sandbox the agent's 2. Why AF_UNIX can't be allowed. It's ruled out, by the team that owns the profile. Two specifics that matter:
There is an escape hatch — the vendor lets a user exempt a named binary from the sandbox entirely — but I don't think that's the better answer. It trades a scoped capability for removing containment altogether from a process that indexes source code, and it's a per-machine manual step the project can't rely on. 3. What I need inside the sandbox — reads only. This is the useful answer, and it's why your suggestion is simply better than what I sent. Querying an index built elsewhere is enough. Indexing already happens outside the sandbox, from an ordinary terminal where the daemon works normally and owns the watcher and the lease. The sandboxed session only needs 4. Would the errno fix alone unblock me? Not on its own — an accurate instant error still leaves the tools unusable, so I'd still be reaching for a terminal. But it's worth far more than its size, and I'd rather it landed first regardless. It cost me most of a day: the 30 s wait lands just past the 30 s MCP handshake budget my host imposes, so all I saw was "connection timed out after 30000ms" — the actual error never arrived. And when it does arrive it actively misleads: the daemon was healthy throughout, and a running daemon is what makes the wait last the full 30 s, since holding the cohort lock is what makes presence read COORDINATED and the I'll send that as its own PR: an On CI: DCO is fixed and both commits are signed off. The |
On hosts whose sandbox denies socket syscalls, an MCP client can never complete the daemon handshake. A macOS seatbelt profile that permits filesystem access but applies `(deny network*)` makes both bind() and connect() fail with EPERM, AF_UNIX included, so there is no reachable rendezvous and no alternative transport. The failure is slow and misreported rather than immediate. Daemon presence is inferred from a POSIX file lock, which such a profile still permits, so a live daemon reads as COORDINATED. connect() then returns EPERM, but cbm_daemon_runtime_connect_result_t (daemon/runtime.h) carries no errno, so cbm_daemon_bootstrap_classify_failed_connect() cannot distinguish "denied" from "still starting" and returns RESERVED, which is waitable. The bootstrap loop retries every BOOTSTRAP_RETRY_NS until MAIN_MCP_STARTUP_TIMEOUT_MS expires -- roughly 30,000 blind retries of an instantaneous EPERM -- and only then reports failure. Hosts that impose their own 30s MCP handshake budget see a timeout just before the error arrives, which hides the cause. A complete socket-free stdio server already existed but was unreachable from main(): cbm_mcp_server_run() (mcp.h) has no socket, sockaddr_un or IPC reference anywhere in src/mcp/mcp.c, and its only caller was tests/test_mcp.c. This routes MCP clients to it behind an opt-in CBM_IN_PROCESS gate, so default behaviour is byte-for-byte unchanged. The store is resolved with cbm_mcp_server_new(NULL) exactly as daemon/application.c and ui/http_server.c already do, so an in-process session reads the same CBM_CACHE_DIR indexes a daemon builds. Background tasks stay at the standalone default documented in mcp.h: with no config store attached, maybe_auto_index() resolves auto_index=false and returns without doing synchronous work on the initialize path. Trade-off, documented in both README.md and docs/CONFIGURATION.md: no cross-session coordination -- no shared watchers, no shared indexing jobs, no UI, no exact-build admission barrier. Measured on macOS with the sandbox active (sandbox_check() == 1): without CBM_IN_PROCESS: no response, rc=1 after 31.1s with CBM_IN_PROCESS: initialize result in 4.6s scripts/test_mcp_in_process.py asserts the handshake completes AND that no rendezvous appears under CBM_RUNTIME_DIR, since "it answered" alone would still pass on an ordinary host if the session silently fell back to the daemon. Verified to fail against an unpatched binary. Signed-off-by: pmaxvsbobo <pmaxvsbobo@gmail.com>
… taking them unguarded
Review caught a real hazard, and it was worse than "no lease is acquired":
mcp_project_mutation_begin() is
return !srv->mutation_begin || srv->mutation_begin(...)
so an UNSET guard fails OPEN and the mutation proceeds. The first version of
this branch set no mutation guard, no try guard and no index executor, and left
background_tasks at its default true — so an in-process index_repository wrote
the shared cache under CBM_CACHE_DIR with no cross-session lease at all, racing
any daemon session mutating the same project.
Every other construction site in the tree already gets this right:
daemon/application.c installs the guard, the try guard, a config store and an
index executor; ui/http_server.c installs a refusing index executor plus both
guards; and main.c's own local-CLI path, in the same function this patch edits,
installs both guards. This mirrors them.
Without a daemon there is nothing to acquire a lease FROM, so the honest answer
is that an in-process session cannot coordinate a write and must refuse it.
Indexing stays with the daemon, outside the sandbox — which is the actual use
case anyway: the sandboxed session only needs to read an index built elsewhere.
- background_tasks off, so maybe_auto_index() cannot index on initialize
- a mutation guard that refuses, and the try guard deliberately left NULL:
with mutation_begin set and mutation_try_begin NULL,
mcp_project_mutation_try_begin() also returns false, so opportunistic
writes during a read are refused too
- an index executor that rejects with a message naming the reason, rather
than letting the guard report it as "blocked by an active index" when
nothing is blocking
Test extended to cover it, and the assertion was verified to fail against a
build with the three calls removed. That build did not merely skip the guard —
it returned {"status":"indexed"}, isError:false, having written the cache.
So the test now asserts both that the call is refused AND that no .db appears
under CBM_CACHE_DIR: an error message alone could be emitted after a partial
write, and only the second check distinguishes a refusing guard from an absent
one.
Signed-off-by: pmaxvsbobo <pmaxvsbobo@gmail.com>
bc945a4 to
838fb70
Compare
What does this PR do?
Adds an opt-in
CBM_IN_PROCESSenvironment variable that serves an MCP sessionin-process over stdio, without starting or connecting to the coordination daemon.
Why it's needed: on hosts whose sandbox denies socket syscalls, the daemon
handshake can never complete. A macOS seatbelt profile that permits filesystem
access but applies
(deny network*)makes bothbind()andconnect()failwith
EPERM—AF_UNIXincluded — so there is no reachable rendezvous and noalternative transport. Today that surfaces as a 30s hang and a misleading
"CBM daemon is active or starting" error;
The change is small because the server already existed:
cbm_mcp_server_run()is a complete socket-free stdio loop (zero socket/IPC references in
src/mcp/mcp.c) whose only caller wastests/test_mcp.c. This routes MCPclients to it behind a gate, so default behaviour is unchanged.
Measured under an active sandbox: 31.1s failure → 4.6s
initializeresult.Trade-off (documented in
README.mdanddocs/CONFIGURATION.md): no daemonmeans no cross-session coordination — no shared watchers, no shared indexing
jobs, no UI, no exact-build admission barrier.
Checklist
git commit -s) — required, CI rejectsunsigned commits (DCO, see CONTRIBUTING.md)
make -f Makefile.cbm test)make -f Makefile.cbm lint-ci)