diff --git a/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1760/.openspec.yaml b/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1760/.openspec.yaml new file mode 100644 index 000000000..d160e09cf --- /dev/null +++ b/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1760/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-08-21 diff --git a/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1760/design.md b/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1760/design.md new file mode 100644 index 000000000..f99431586 --- /dev/null +++ b/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1760/design.md @@ -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. diff --git a/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1760/proposal.md b/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1760/proposal.md new file mode 100644 index 000000000..4754b776b --- /dev/null +++ b/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1760/proposal.md @@ -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. diff --git a/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1760/tasks.md b/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1760/tasks.md new file mode 100644 index 000000000..f137531ec --- /dev/null +++ b/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1760/tasks.md @@ -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=`) 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` diff --git a/openspec/config.yaml b/openspec/config.yaml new file mode 100644 index 000000000..c4d34acea --- /dev/null +++ b/openspec/config.yaml @@ -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 diff --git a/src/cli/cli.c b/src/cli/cli.c index d981af266..ddd21e18d 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -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); diff --git a/tests/test_cli.c b/tests/test_cli.c index 6295479f2..d6d0fd294 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -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) { @@ -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);