Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-08-21
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
## Context

See proposal.md for motivation. Verified current state (against source):

- `cli_activation_production_reserve` (src/cli/cli.c:527) holds cohort EX locks + the startup lock, then calls `cbm_daemon_ipc_generation_probe_under_startup_lock` (cli.c:571-572). `generation != 0` ⇒ return BUSY (0) or error (cli.c:573-578).
- The probe's endpoint-probe branch (ipc.c) is deliberately fail-closed: a present-but-ECONNREFUSED socket counts as generation 1 (to protect saturated BSD listeners whose reserved backlog yields ECONNREFUSED while alive).
- All locks in the path are kernel-released on process death (flock LOCK_NB / fcntl F_SETLK / F_GETLK probe) — a dead process cannot keep a reservation. The only surviving state is the socket publication: `.sock` + `.anc` (+ identity commits), never unlinked by a kill -9.
- `cbm_daemon_ipc_stale_generation_cleanup` (ipc.c:2464, POSIX) already exists: it validates the startup lock matches, acquires a temporary lifetime reservation EX (refuses if a live daemon holds one), and unlinks only inode-matched provably-current stale identities. Used by daemon-start paths and `cbm_version_cohort_daemon_presence` (version_cohort.c:850-877).
- `cbm_daemon_ipc_lifetime_reservation_probe` (ipc.h:128): 0 = authoritatively absent, 1 = held, -1 = cannot validate.

## Goals / Non-Goals

**Goals:**
- A CLI activation reserve reaching `generation == 1` with a provably-absent lifetime reservation recovers: run the existing stale cleanup and re-probe before returning BUSY.
- Reuse the established repair; no new primitives (no new lock kinds, no new probe semantics).
- Live daemon safety by construction: cleanup itself re-checks + re-refuses; the CLI re-probe closes the window.

**Non-Goals:**
- Changing `cbm_daemon_ipc_endpoint_probe`'s ECONNREFUSED→active semantics (deliberate + tested; used by other security-sensitive callers).
- Windows behavior (rendezvous-record model; guarded out).
- Client-visible CLI surface changes (no flags, no new messages).
- Faster retry loop for the eager shutdown probe against stale sockets (a separate latency improvement, noted in the oracle report).

## Decisions

**D1 — Insertion point: `cli_activation_production_reserve`, right after the generation probe.**
Keeping it in the CLI layer (not in ipc.c) preserves the probe's semantics for other callers and keeps the recovery scoped to the activation decision. The startup lock is already held at this point (cli.c:566), which is exactly what the cleanup requires.

**D2 — Guarded by `#ifndef _WIN32` + three conditions.**
The recovery runs only when:
1. `generation == 1` (some publication claims an active generation), AND
2. `cbm_daemon_ipc_lifetime_reservation_probe(...) == 0` (no live daemon holds the lifetime reservation — every live listener does, from listen to close), AND
3. `cbm_daemon_ipc_stale_generation_cleanup(...) == 1` (cleanup itself validated + removed the stale identity; 0 = refused, -1 = invalid).

Then the generation probe runs once more (ENOENT ⇒ 0). Any other combination falls through to today's verdict.

**D3 — The re-probe is the anti-TOCTOU close.**
Only the re-probed 0 grants a clean path; if the re-probe still says 1, the original BUSY decision stands. No new code path can mutate anything: the existing `if (generation != 0)` branch is unchanged.

**D4 — Single guarded block, no new helper function.**
The block is 8 lines and needs the surrounding context (lease/lock lifecycle) — a helper would need the same arguments.

## Risks / Trade-offs

- [Recovery could race a daemon starting between probe and cleanup] → A boot must hold the startup lock; the CLI holds it throughout. Daemon already booted ⇒ lifetime reservation held ⇒ condition 2 excludes it. Double-sealed.
- [Saturated-listener regression] → That daemon holds its lifetime reservation ⇒ cleanup returns 0 ⇒ BUSY as today (test 1875 pattern).
- [Cleanup partially refuses (unknown identity preserved)] → returns 0 ⇒ conservative BUSY; the reporter keeps today's message. Acceptable; the reporter in #1760 has a *provably* stale pair.
- [Windows] → excluded by `#ifndef _WIN32`; no change.
- [Cost when clean] → one lifetime probe (F_GETLK) on the busy path where generation==1 only; no cost on the common clean path (generation==0 skips).

## Migration Plan

No data/config migration. Behavior change is additive recovery; rollback = revert.

## Open Questions

None.
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
## Why

After CBM daemon processes are hard-killed (`pkill`/`kill -9`), the rendezvous directory under the runtime dir keeps the published Unix socket identity (`.sock`/`.anc` pair, no `.lock`/`.pid`). Every subsequent `install`/`update` then fails permanently with "active CBM sessions ... could not be stopped safely" even though no process, lock, or socket endpoint is alive. The socket publication survives the hard kill (it is an artifact of bind/linkat, not a held file lock), and the activation guard's generation probe treats an `ECONNREFUSED` socket as an active generation (fail-closed, deliberate for saturated BSD listeners). The stale socket is only repaired on daemon start paths — which the CLI never reaches because it refuses before mutating anything. Deadlock by residue.

## What Changes

- During a CLI activation reserve (`cli_activation_production_reserve`, the path behind `install`/`update`), when the generation probe reports an active generation (1) **but** the lifetime reservation probe reports none (0) and the startup lock is held (it is), the stale socket identity is provably residual publication of a dead process. The CLI then runs the existing `cbm_daemon_ipc_stale_generation_cleanup` repair (same one daemon-start paths and `cbm_version_cohort_daemon_presence` use) and re-probes generation before deciding.
- `stale_generation_cleanup` refuses (returns 0) when a lifetime reservation is genuinely held or the identity is mismatched/unknown — so a live daemon (including a saturated BSD listener) is never disturbed; the guard keeps today's BUSY verdict in every such case.
- POSIX-only. Windows keeps its existing rendezvous-record based semantics unchanged.
- No public CLI/API shape changes: same commands, same ordering, same messages — a previously impossible recovery path only.

## Capabilities

### New Capabilities
- `cli-activation-stale-rendezvous-recovery`: CLI activation's recovery of residual rendezvous state left by hard-killed daemon processes before declaring BUSY.

### Modified Capabilities
- None (no existing specs).

## Impact

- `src/cli/cli.c``cli_activation_production_reserve`: probe → (generation 1 + lifetime 0) → cleanup → re-probe before returning BUSY.
- `tests/test_cli.c` and/or `tests/test_daemon_ipc.c` — regression tests: stale socket produced by bind+listen+close (no unlink) under an isolated runtime parent is recovered by the activation guard; a LIVE listener (kept open) still blocks (BUSY).
- Reuses existing `cbm_daemon_ipc_stale_generation_cleanup`; no new lock, no new IPC surface.
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
## 1. Activation recovery fix

- [ ] 1.1 In `cli_activation_production_reserve` (src/cli/cli.c), after the generation probe, add the `#ifndef _WIN32` recovery block: when `generation == 1 && cbm_daemon_ipc_lifetime_reservation_probe(endpoint) == 0 && cbm_daemon_ipc_stale_generation_cleanup(endpoint, startup_lock) == 1`, re-run the generation probe; keep the existing `generation != 0` decision branch unchanged
- [ ] 1.2 Verify: `make -f Makefile.cbm test-focused TEST_SUITES=cli` compiles and passes (existing activation tests unchanged)

## 2. Regression tests

- [ ] 2.1 Add `TEST(cli_activation_recovers_stale_rendezvous_publication_issue1760)` in tests/test_cli.c: isolate runtime parent via test seam, fork a child that binds a Unix socket at the endpoint address, listens, signals ready and `_exit(0)` without unlink (hard-kill residue), then run the real activation guard (POSIX `install` path with `--force --skip-config --yes --dir=<tmp>`) and assert rc == 0 and the stale socket is gone
- [ ] 2.2 Add a second assertion in the same test (or a sibling): with a LIVE listener still bound and held (child stays alive), the guard returns BUSY and the socket remains published — the recovery must not disturb a live daemon
- [ ] 2.3 Verify: `make -f Makefile.cbm test-focused TEST_SUITES=cli` passes with the new tests

## 3. Final validation

- [ ] 3.1 Run `make -f Makefile.cbm test` (full suite) — passes
- [ ] 3.2 `OPENSPEC_NO_UPDATE_CHECK=1 openspec validate oss-codebase-memory-mcp-issue-1760 --json` — clean
- [ ] 3.3 Archive the change: `OPENSPEC_NO_UPDATE_CHECK=1 openspec archive oss-codebase-memory-mcp-issue-1760 --yes`
32 changes: 32 additions & 0 deletions openspec/config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
schema: spec-driven

# Project context (optional)
# This is shown to AI when creating artifacts.
# Add your tech stack, conventions, style guides, domain knowledge, etc.
# Example:
# context: |
# Tech stack: TypeScript, React, Node.js
# We use conventional commits
# Domain: e-commerce platform

# Per-artifact rules (optional)
# Add custom rules for specific artifacts.
# Example:
# rules:
# proposal:
# - Keep proposals under 500 words
# - Always include a "Non-goals" section
# tasks:
# - Break tasks into chunks of max 2 hours

# Per-operation guidance (optional)
# Add advisory guidance for how apply and archive work should be conducted.
# This is separate from artifact rules above.
# Example:
# operations:
# apply:
# guidance:
# - Keep test summaries concise
# archive:
# guidance:
# - Summarize the archive outcome before finishing
16 changes: 16 additions & 0 deletions src/cli/cli.c
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,22 @@ static int cli_activation_production_reserve(void *opaque, cbm_cli_activation_lo
}
int generation = cbm_daemon_ipc_generation_probe_under_startup_lock(context->endpoint,
context->startup_lock);
#ifndef _WIN32
/* #1760: generation==1 together with an absent lifetime reservation cannot
* be a live daemon — every listener retains its lifetime reservation for
* the whole of its life (ipc.c listen path), and the startup lock we hold
* excludes any boot in flight. That combination is residual socket
* publication from a hard-killed process (the endpoint probe is
* intentionally fail-closed on ECONNREFUSED to protect saturated BSD
* listeners). Repair it exactly like cbm_version_cohort_daemon_presence()
* does, then re-probe: stale_generation_cleanup refuses (returns 0) when a
* reservation is genuinely held, so this can never damage a live daemon. */
if (generation == 1 && cbm_daemon_ipc_lifetime_reservation_probe(context->endpoint) == 0 &&
cbm_daemon_ipc_stale_generation_cleanup(context->endpoint, context->startup_lock) == 1) {
generation = cbm_daemon_ipc_generation_probe_under_startup_lock(context->endpoint,
context->startup_lock);
}
#endif
if (generation != 0) {
cli_activation_startup_lock_release_complete(context);
cli_activation_release_cleanup_lease(context, &lease);
Expand Down
89 changes: 89 additions & 0 deletions tests/test_cli.c
Original file line number Diff line number Diff line change
Expand Up @@ -1014,6 +1014,94 @@ TEST(cli_activation_quiesce_does_not_wait_on_bootstrap_startup) {
ASSERT_TRUE(event_order);
PASS();
}

/* #1760: a hard-killed daemon leaves its socket/anchor/identity publication
* behind while every lock is kernel-released. The activation guard used to
* treat that residue as an active generation forever ("could not be stopped
* safely"); it must now repair the provably-stale publication (no lifetime
* reservation, startup lock held) and proceed with the install. */
TEST(cli_activation_recovers_stale_rendezvous_publication_issue1760) {
char tmpdir[256];
snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-activation-stale-XXXXXX");
if (!cbm_mkdtemp(tmpdir)) {
FAIL("cbm_mkdtemp failed");
}
char runtime_parent[512];
snprintf(runtime_parent, sizeof(runtime_parent), "%s/runtime", tmpdir);
if (test_mkdirp(runtime_parent) != 0) {
test_rmdir_r(tmpdir);
FAIL("runtime parent setup failed");
}
int ready_pipe[2] = {-1, -1};
if (pipe(ready_pipe) != 0) {
test_rmdir_r(tmpdir);
FAIL("pipe failed");
}
pid_t child = fork();
if (child == 0) {
close(ready_pipe[0]);
cbm_daemon_ipc_endpoint_t *endpoint = cbm_daemon_bootstrap_endpoint_new(runtime_parent);
cbm_daemon_ipc_listener_t *listener = endpoint ? cbm_daemon_ipc_listen(endpoint) : NULL;
uint8_t result = listener ? 'R' : 'E';
(void)write(ready_pipe[1], &result, 1);
(void)close(ready_pipe[1]);
/* Deliberately bypass listener_close + unlink: the kernel releases the
* descriptors and file locks while the current-generation socket
* identity stays behind — exactly the hard-kill residue of #1760. */
_exit(listener ? 0 : 1);
}
(void)close(ready_pipe[1]);
uint8_t ready = 0;
bool child_ready = child > 0 && read(ready_pipe[0], &ready, 1) == 1 && ready == 'R';
(void)close(ready_pipe[0]);
int child_status = 0;
bool child_ok = child > 0 && waitpid(child, &child_status, 0) == child &&
WIFEXITED(child_status) && WEXITSTATUS(child_status) == 0;

char *old_home = NULL;
char *old_cache = NULL;
cli_activation_save_env(&old_home, &old_cache);
cbm_setenv("HOME", tmpdir, 1);
cbm_set_auto_answer_for_test(0);
char cache_dir[512];
char install_dir[512];
char activation_log[640];
snprintf(cache_dir, sizeof(cache_dir), "%s/cache", tmpdir);
snprintf(install_dir, sizeof(install_dir), "%s/custom/bin", tmpdir);
snprintf(activation_log, sizeof(activation_log), "%s/logs/activation-events.ndjson", cache_dir);
cbm_setenv("CBM_CACHE_DIR", cache_dir, 1);
cbm_cli_set_activation_runtime_parent_for_test(runtime_parent);
char dir_arg[640];
snprintf(dir_arg, sizeof(dir_arg), "--dir=%s", install_dir);
char *install_argv[] = {"--force", "--skip-config", "--yes", dir_arg};
int install_rc = child_ready && child_ok ? cli_test_cmd_install(4, install_argv) : -1;
cbm_cli_set_activation_runtime_parent_for_test(NULL);

/* The repair identity must be gone: the endpoint is keyed the same way the
* CLI and the daemon derive it, so a fresh endpoint yields the same path. */
cbm_daemon_ipc_endpoint_t *probe = cbm_daemon_bootstrap_endpoint_new(runtime_parent);
const char *socket_address = probe ? cbm_daemon_ipc_endpoint_address(probe) : NULL;
struct stat socket_state;
errno = 0;
bool socket_gone =
socket_address && lstat(socket_address, &socket_state) != 0 && errno == ENOENT;
if (probe) {
cbm_daemon_ipc_endpoint_free(probe);
}

const char *events = read_test_file(activation_log);
const char *completed = events ? strstr(events, "\"phase\":\"completed\"") : NULL;

cli_activation_restore_env(old_home, old_cache);
test_rmdir_r(tmpdir);

ASSERT_TRUE(child_ready);
ASSERT_TRUE(child_ok);
ASSERT_EQ(install_rc, 0);
ASSERT_TRUE(socket_gone);
ASSERT_NOT_NULL(completed);
PASS();
}
#endif

TEST(cli_install_force_quiesces_active_cohort_before_replacing_binary) {
Expand Down Expand Up @@ -12967,6 +13055,7 @@ SUITE(cli) {
#ifndef _WIN32
RUN_TEST(cli_activation_cleanup_failure_fail_stops_before_lease_release);
RUN_TEST(cli_activation_quiesce_does_not_wait_on_bootstrap_startup);
RUN_TEST(cli_activation_recovers_stale_rendezvous_publication_issue1760);
#endif
RUN_TEST(cli_install_force_quiesces_active_cohort_before_replacing_binary);
RUN_TEST(cli_install_dir_and_skip_config_stage_first_install_safely);
Expand Down
Loading