fix(cli): scope the activation guard to the install-target HOME so a sandbox install can't drain the host daemon - #2115
Merged
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
DeusData
enabled auto-merge
September 12, 2026 10:31
sha256_transform held its 64-word message schedule as a 256-byte local. That is large enough for ASan's use-after-return fake stack, so an instrumented build heap-allocated it through __asan_stack_malloc on every 64-byte block -- millions of allocations to hash one large file. Under the sanitized CI settings (detect_stack_use_after_return=1) fingerprinting a freshly installed binary therefore took minutes, and the cli suite blew its 900s wall-clock budget and was killed as hung on the Linux arm64 leg. The suite passes on macOS, where the fake stack is not engaged the same way, which is why this only ever surfaced on one leg. Move the schedule into cbm_sha256_ctx so it is allocated once per hash instead of once per block. The computed values and the resulting digest are identical; no sanitizer option is relaxed and use-after-return detection stays enabled everywhere. Verified in the Linux arm64 sanitized container under the exact CI ASAN_OPTIONS (detect_stack_use_after_return=1:strict_string_checks=1: detect_stack_use_after_scope=1): cli 316 passed in 356s, previously killed at the 900s budget; daemon_version 10 passed, covering the build-fingerprint digests that would break first on a wrong hash. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The macos-15-intel leg killed the cli suite at its 900s wall clock while reporting 7537 passed, 0 failed. Two separate things were wrong. The suite was misclassified. cli spends 497s of the 900s default on macos-14, the FASTEST macOS runner, while macos-15-intel in the same matrix is 2.4-3.6x slower on comparable suites (daemon_runtime 842s vs 349s, stack_overflow_b 277s vs 76s). 497s at that ratio cannot fit. daemon_runtime, at 842s on that same runner, passes only because it was already in SLOW_SUITES. cli belongs there too -- it is deterministic and simply large, which is the only thing that list is allowed to mean. The drain test was also waiting on a clock instead of on the system. Profiling the suite per test (315 tests, 300s wall / 166s cpu) put cli_install_into_host_namespace_still_drains_host_cohort at 42s wall for 19s of cpu -- 23s idle, the largest single idle block, where the next worst was 11s. The existing cli_install_force_quiesces_active_cohort_before_replacing_binary drains a cohort too and idles 0.5s, which is what pointed at the difference. The cost was the host_serving probe. Its generous budget exists for the POSITIVE question -- is this daemon still up? -- where a slow reply on a loaded runner must not be misread as drained; that fixed a real flake and is kept. Asked in the negative it inverts: no reply is coming, so the full 15s is spent establishing silence and ASSERT_FALSE is decided by the timeout expiring rather than by the daemon's behaviour. The drain is already proven positively in that test -- install returns 0 only after the activation completed, and the host child exits CLI_SCOPE_HOST_DRAINED -- so the probe only has to confirm it. Also shortens the drained child's teardown budget, which retried service_free / lease_release / manager_free against 10s on a path where the activation had already torn the service down. Behaviour at the deadline is unchanged; it is reached sooner when nothing is wedged. Measured at only -3s on its own, kept because it is correct and free. Local, same machine, ASan+UBSan: the drain test 42.0s -> 25.8s (-38%), the suite 300s -> 279s, 315 passed before and after -- no test removed, no assertion weakened. PR #2188, which runs the same leg without these two tests, passed macos-15-intel in 37m34s, so the suite was not already over budget on main. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
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