feat(mcp): add CBM_IN_PROCESS to serve stdio MCP without the daemon - #2072
pmaxvsbobo wants to merge 6 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 |
bc945a4 to
838fb70
Compare
|
@DeusData I think this is needed for making codebase-memory-mcp more popular on enterprise env, can u pls prioritize reviewing it? |
|
Thank you for clarifying the read-only and sandbox use cases. Those answers address the use-case questions already raised. We need more time to decide the in-process/read-only execution contract before making an integration decision; there is no additional questionnaire or speculative rebase requested from you at this stage. |
|
Thank you, @pmaxvsbobo. We've made the decision: Two things before we review it for merge:
Thanks again for spelling out the use cases. They made the decision easy. |
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>
The switch was on for any non-empty value other than "0", so CBM_IN_PROCESS=false (or off, or no) silently made the session read-only and daemon-free. Match CBM_DIAGNOSTICS, the house convention for boolean environment switches (src/foundation/diagnostics.c): only "1" or "true" turn it on. tests/test_in_process_mcp.sh (next commit) runs its daemon-backed positive control with CBM_IN_PROCESS=false, so a regression here fails that check. Signed-off-by: pmaxvsbobo <pmaxvsbobo@gmail.com>
Requested by the maintainer on DeusData#2072 as the condition for the mode: an in-process client must never write shared cache state -- no daemon coordination files, no cohort or lifetime locks, and no writes to another process's index -- so it cannot disturb a daemon running alongside it. tests/test_in_process_mcp.sh (wired as Step 5g of scripts/test.sh, on the private runtime from scripts/test-runtime.sh) drives the real binary: 1. Fresh state, no daemon: every mutating tool is attempted, and the runtime, cache and HOME directories must still be empty afterwards. 2. A live permanent daemon owning two real indexes: an in-process session reads them with every tool it serves (each must succeed) and attempts every mutation. Every entry under those directories (type, mode, size, mtime, inode, sha256) must be identical to a quiescent baseline, and the live session may hold no socket and no descriptor under CBM_RUNTIME_DIR -- a pre-existing lock file shows no trace of being locked, so only the descriptor check can prove no lock was taken. 3. The same after a daemon-side write left the index in WAL mode: only SQLite's reader -shm and an empty -wal may appear; the database must stay byte-identical. A positive control started with CBM_IN_PROCESS=false must FIND the socket and the runtime lock, so the descriptor check is known to be able to fail. This test FAILS at this commit, by design: FAIL: CBM_IN_PROCESS created shared state from a fresh start: ['cache/_config.db'] Since 21ae021, handle_index_repository loads the index policy before the rejecting executor can refuse, and with no server config that creates ${CBM_CACHE_DIR}/_config.db. The next commit fixes it. Replaces scripts/test_mcp_in_process.py, which nothing ran. Signed-off-by: pmaxvsbobo <pmaxvsbobo@gmail.com>
A refused in-process index_repository wrote shared state. Since 21ae021 the handler calls load_index_policy() before the index executor is consulted; an in-process server has no config, so that calls cbm_config_open(), which creates ${CBM_CACHE_DIR}/_config.db -- or opens the daemon's copy read-write -- before the rejecting executor runs. The refusing guards cannot prevent this: they sit inside the handlers, after work like that. Clamp the in-process tool profile to the analysis allowlist (scout stays scout), so dispatch_tool() refuses every mutating tool before its handler runs, and tools/list no longer offers index_repository, delete_project, manage_adr or ingest_traces. The refusing mutation guard, the rejecting executor and background_tasks=false stay as the second line: a write that gets past the allowlist still fails closed at the lease. Verified on macOS 27.0 arm64 (python 3.9, git 2.54), with `bash tests/test_in_process_mcp.sh`: - this commit: PASS (~32 s) - parent commit: FAIL ['cache/_config.db'] - mutant that also joins the version cohort from the in-process branch: FAIL (leg 1: runtime cohort locks created; leg 2 alone: the descriptor check finds cbm-version-cohort-lifetime-v1.lock) - mutant that utimes() each project .db at in-process startup: FAIL (leg 2; and leg 3 alone, so the WAL allowance hides no index write) Not run: Linux (CI's first run); Windows (the test skips, as test_watcher_disabled.sh does). Signed-off-by: pmaxvsbobo <pmaxvsbobo@gmail.com>
Requested by the maintainer on DeusData#2072: a short entry saying what the mode is for, that it opts out of the shared daemon, and when not to use it. Adds "Running without the daemon" to docs/CONFIGURATION.md and points the env-table row and the README daemon section at it. It also states what the mode still needs: a writable CBM_CACHE_DIR (every CBM process hardens it to 0700 at startup, so a read-only mount is not supported), scratch space in ${CBM_CACHE_DIR}/logs and /tmp, and -- on a WAL-mode index -- SQLite's reader -shm/-wal, the files any reader creates. Signed-off-by: pmaxvsbobo <pmaxvsbobo@gmail.com>
8199efa to
1a37f0d
Compare
|
Written with Claude Code; I've reviewed it and I'm accountable for it. Thanks, @DeusData. Both requests are done, and working on the first one turned up a real bug. On the current The bug. After merging Commits (on top of
The test is
A positive control run with Evidence (macOS 27.0 arm64, Python 3.9, git 2.54):
The test hasn't been run on Linux; this CI run is its first. On Windows it's skipped, as Branch. I rebased the PR onto Read-only mounts aren't covered, and I'd like your call. Every CBM process, this mode included, runs The errno PR (fast-failing |
DeusData
left a comment
There was a problem hiding this comment.
Thank you, @pmaxvsbobo. This is a far stronger answer than we asked for. Writing the test first and catching the _config.db creation on current main is exactly why we asked for a test rather than an argument. Clamping the tool set so the refusal happens before any handler runs is the right fix, and the positive control plus the mutants you ran show the test actually binds. The docs section is clear about what the mode gives up.
Two things before merge:
- Read-only mounts: please do it here. You're right that
main_build_identity()chmodsCBM_CACHE_DIRbefore the in-process branch, which fails withEROFS. We'd like your verify-only path in this PR: in in-process mode, refuse unless the cache directory is already owner-only, and never chmod it. Please add a test for a read-only cache directory as well, since read-only mounts and sandboxes are the reason this mode exists. - The settle loop in the test.
quiescent_snapshot()waits for two identical snapshots 0.5 s apart. That's a settle window, so the verdict still depends on timing: a daemon write landing just after the window would fail the leg for reasons unrelated to in-process mode. We ask tests to wait for a state they can observe instead. Could the baseline be taken once the daemon is idle by construction, for example with the watcher off via config (astests/test_watcher_disabled.shdoes) and the indexes already published whencli index_repositoryreturns? If you saw a specific late daemon write that made the settle loop necessary, tell us what it was and we'll wait on that exact signal instead. (Small nit: ifdaemon stopalready blocks until the daemon exits, the stop poll at the end can go.)
Thanks again. Your testing turned up a bug we would otherwise have shipped.
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)