fix(cli): scope the activation guard to the install-target HOME so a sandbox install can't drain the host daemon - #2115
Open
DeusData wants to merge 2 commits into
Open
fix(cli): scope the activation guard to the install-target HOME so a sandbox install can't drain the host daemon#2115DeusData wants to merge 2 commits into
DeusData wants to merge 2 commits into
Conversation
…dment)
The rendezvous directory the CLI's activation guard talks to is per OS
account (service.h), not per HOME/CBM_CACHE_DIR: every daemon owned by
one user meets at the same endpoint, and only the cache fingerprint
carried in the cohort identity separates one HOME/CBM_CACHE_DIR
namespace from another. `cli_activation_production_reserve` used
`cbm_version_cohort_reserve_for_mutation`, which treats any lifetime-
lock holder at that shared endpoint as the daemon this activation must
drain, with no cache-fingerprint awareness at all. An `install` run
against a second HOME (a sandbox, a second profile, `--skip-binary`
into an alternate CBM_CACHE_DIR) therefore reached the LIVE host
daemon at the shared endpoint and drained it — disconnecting every one
of the host's real MCP clients — even though nothing the sandboxed
install touched belonged to the host's cache namespace.
The fix adds `cli_activation_resolve_scope`, a non-blocking probe via
the already cache-fingerprint-aware `cbm_version_cohort_acquire`, run
before the blind drain path. It distinguishes three cases from the
resolved target of THIS activation:
- nothing to quiesce at all: a `--skip-binary` install that also
resets no index publishes nothing, so no session anywhere needs to
stop (`quiesce_required` on `cli_activation_guard_scoped`, plumbed
from `cbm_cmd_install`'s own binary/index-reset decision);
- the active cohort's cache fingerprint doesn't match this
activation's target cache: a foreign namespace, left untouched;
- same cache fingerprint (or no active cohort): this activation
really does replace that daemon, so the existing drain/quiesce
path runs unchanged, including the barrier wait when another
activation already holds maintenance.
Verified against a real forked daemon (a live runtime service with one
committed client) standing in for the host: an install into a foreign
HOME/CBM_CACHE_DIR (`--skip-binary` and a full binary install) leaves
that daemon serving with its client still attached, while an install
that targets the host's own namespace still drains it and the
activation audit still names the client it disconnected.
New/updated tests in tests/test_cli.c (cli suite):
- cli_install_skip_binary_into_foreign_home_never_drains_host_cohort
- cli_install_binary_into_foreign_home_never_drains_host_cohort
- cli_install_into_host_namespace_still_drains_host_cohort (regression)
- cli_install_skip_binary_unchanged_in_host_namespace_quiesces_nothing
- cli_activation_quiesce_does_not_wait_on_bootstrap_startup updated to
carry a real cache fingerprint for its participant, now that scope
is fingerprint-gated
src/mcp/index_supervisor.{c,h}: CBM_CLI_ENABLE_TEST_API-gated setter
so the fixture's forked daemon can carry the same build fingerprint
the guard captures from the supervisor.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
…2115) The four namespace-scope tests fork a real host daemon and, in the parent, waited on it with two UNBOUNDED calls: read() for the child's one-byte readiness signal in cli_scope_fixture_start, and waitpid(host, 0) in cli_scope_fixture_finish. If the forked child stalls before it can signal — a fork-time sanitizer allocator stall is the classic cause, the same hazard the earlier posix_spawn work addressed — the parent blocks forever and the WHOLE cli suite hangs to the CI wall-clock kill (the observed rc=124 at 900 s on the ubuntu-24.04-arm gcc shard, which passed on every other #2115 shard). Attribution: the production drain path is already bounded (DRAIN 15 s, CONTROL 2 s) and a foreign-namespace install takes the non-blocking scope probe and skips draining entirely, so this is not a production deadlock — it is a test-fixture determinism gap, amplified by ASan on a loaded arm runner. Fix, test-only: - cli_scope_wait_ready(): poll the ready pipe to a generous, bounded deadline (90 s, comfortably past the child's own 45 s cohort-admission deadline plus service start), so a wedged child yields a clean ASSERT_TRUE(ready) failure instead of an unbounded read(). - cli_scope_reap_host(): reap with WNOHANG to a bound (60 s), then SIGKILL and reap, so a wedged child is force-killed rather than hanging finish(); a killed child leaves host_exit == -1, which fails the caller's assertion cleanly. This also prevents a stuck child from holding cohort locks that would poison later tests. - cli_scope_host_serving(): raise the status probe deadline from 5 s to 15 s so a slow-but-alive daemon on a loaded runner is not misread as "drained" (the flaky ASSERT(host_serving) failure this fixture showed); it still fails cleanly when the daemon is genuinely gone or reports stopping. None of these decides a healthy test — the child signals in well under a second on a healthy runner; the bounds only convert an otherwise unbounded hang into a deterministic pass/fail. The scoping contract still binds: with cli_activation_resolve_scope forced to always return ACTIVE, the three "must-not-drain" tests go RED at ASSERT(host_serving) and the host-namespace "must-drain" test stays GREEN. cli suite: 313 passed locally (macOS, twice); 30-round pre-fix and 8-round post-fix stress of the four tests were clean; a fault-injected never-ready child fails all four cleanly in ~37 s with no suite hang. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
DeusData
added
bug
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
An
installinto a different HOME (a sandbox, a second profile, orinstall --skip-binary) must not disturb the live host daemon. Today the activation guard keys the "sessions to quiesce" decision on the build cohort / rendezvous endpoint rather than on the install target, so installing into another HOME drained the running host daemon's cohort and disconnected every one of its MCP clients (observed:daemon_active_clients:17 … detail:"cohort drained"from a sandbox--skip-binaryinstall).This scopes the guard to the resolved runtime/rendezvous namespace of this activation's target HOME: an install whose target namespace has no daemon quiesces nothing (logged
clients=0), an install into a different namespace never touches the host daemon, and--skip-binary(which replaces no binary) does not drain. The host-namespace install keeps today's behaviour.Local verification (macOS): build clean;
cli,daemon_application,daemon_runtimesuites pass; RED-on-revert on the new test (an install into namespace B leaves namespace A's daemon and its committed client untouched). Windows is covered by CI.🤖 Generated with Claude Code