Skip to content

Commit d4bb43d

Browse files
committed
feat(cli): refuse db reset while basic-memory mcp processes run
Closes #765. On POSIX, Path.unlink() removes the directory entry but leaves the inode alive as long as any process holds the file open. A `bm reset` that races against a live `basic-memory mcp` therefore "succeeds" — but the still-running MCP keeps reading the now-invisible memory.db inode and returns phantom search rows back to the user. Windows naturally raises PermissionError on unlink so the bug is POSIX-only. The fix is a pre-flight scan via psutil for any non-self `basic-memory mcp` invocations. If any are found, abort with the PIDs and a platform-appropriate `pgrep`/`Get-CimInstance` hint so the user can clean up themselves — we deliberately do not auto-kill anyone else's processes. `--force` is the documented escape hatch for scripted/CI workflows that have already verified no MCP clients are attached. Signed-off-by: phernandez <paul@basicmachines.co>
1 parent c4956ca commit d4bb43d

5 files changed

Lines changed: 276 additions & 3 deletions

File tree

pyproject.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ dependencies = [
4848
"sqlite-vec>=0.1.6",
4949
"openai>=1.100.2",
5050
"logfire>=4.19.0",
51+
"psutil>=5.9.0",
5152
]
5253

5354
[project.urls]

src/basic_memory/cli/commands/db.py

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
"""Database management commands."""
22

3+
import os
34
from dataclasses import dataclass
45
from pathlib import Path
56

7+
import psutil
68
import typer
79
from loguru import logger
810
from rich.console import Console
@@ -21,6 +23,80 @@
2123
console = Console()
2224

2325

26+
def _find_live_mcp_processes() -> list[tuple[int, str]]:
27+
"""Return (pid, joined_cmdline) for live `basic-memory mcp` processes.
28+
29+
Why this exists (issue #765):
30+
On POSIX, `Path.unlink()` removes the directory entry but the inode
31+
survives as long as any process holds the file open. A `bm reset`
32+
run while Claude Desktop (or another MCP client) is alive will
33+
therefore "succeed" — but the still-running MCP keeps reading the
34+
old, now-invisible memory.db inode and returns phantom rows. On
35+
Windows the OS naturally raises PermissionError on `unlink()`, so
36+
the bug is POSIX-specific. We detect proactively to give the same
37+
error experience on every platform before doing damage.
38+
39+
The current process is excluded so this can be called from inside a
40+
`bm reset` invocation. NoSuchProcess / AccessDenied are swallowed
41+
because process tables race with the scan and we don't want a
42+
transient permission error to mask a real zombie.
43+
"""
44+
me = os.getpid()
45+
matches: list[tuple[int, str]] = []
46+
for proc in psutil.process_iter(["pid", "cmdline"]):
47+
try:
48+
pid = proc.info.get("pid")
49+
if pid is None or pid == me:
50+
continue
51+
cmdline = proc.info.get("cmdline") or []
52+
if not cmdline:
53+
continue
54+
# Match the full `basic-memory mcp` invocation, not just any
55+
# process whose argv mentions either word individually.
56+
joined = " ".join(cmdline)
57+
if "basic-memory" in joined and "mcp" in cmdline:
58+
matches.append((pid, joined))
59+
except (psutil.NoSuchProcess, psutil.AccessDenied):
60+
continue
61+
return matches
62+
63+
64+
def _abort_if_mcp_processes_alive() -> None:
65+
"""Refuse `bm reset` while basic-memory MCP processes are still running.
66+
67+
See _find_live_mcp_processes for the underlying POSIX-vs-Windows
68+
rationale. Prints a per-PID list and platform-appropriate cleanup
69+
instructions, then exits non-zero so destructive work never starts.
70+
"""
71+
zombies = _find_live_mcp_processes()
72+
if not zombies:
73+
return
74+
75+
console.print(
76+
"[red]Refusing to reset:[/red] basic-memory MCP processes are still running."
77+
)
78+
console.print(
79+
"[yellow]On macOS/Linux these would keep reading the deleted memory.db inode "
80+
"and return phantom search results (see #765).[/yellow]"
81+
)
82+
for pid, cmd in zombies:
83+
console.print(f" PID {pid}: {cmd}")
84+
console.print("\n[bold]How to clean up:[/bold]")
85+
console.print(" 1. Quit Claude Desktop and any other MCP clients.")
86+
if os.name == "nt":
87+
console.print(
88+
" 2. Verify nothing remains: "
89+
"[green]Get-CimInstance Win32_Process | "
90+
"Where-Object {$_.CommandLine -like '*basic-memory*mcp*'}[/green]"
91+
)
92+
else:
93+
console.print(
94+
" 2. Verify nothing remains: [green]pgrep -fa 'basic-memory mcp'[/green]"
95+
)
96+
console.print(" 3. Re-run [green]bm reset[/green].")
97+
raise typer.Exit(1)
98+
99+
24100
@dataclass(slots=True)
25101
class EmbeddingProgress:
26102
"""Typed CLI progress payload for embedding backfills."""
@@ -86,6 +162,16 @@ async def _reindex_projects(app_config):
86162
@app.command()
87163
def reset(
88164
reindex: bool = typer.Option(False, "--reindex", help="Rebuild db index from filesystem"),
165+
force: bool = typer.Option(
166+
False,
167+
"--force",
168+
help=(
169+
"Skip the pre-flight check that refuses to reset while "
170+
"basic-memory MCP processes are running. Use only in "
171+
"automated workflows where you've already ensured no MCP "
172+
"clients are attached to the database."
173+
),
174+
),
89175
): # pragma: no cover
90176
"""Reset database (drop all tables and recreate)."""
91177
console.print(
@@ -94,6 +180,14 @@ def reset(
94180
"Use [green]bm reset --reindex[/green] to automatically rebuild the index afterward."
95181
)
96182
if typer.confirm("Reset the database index?"):
183+
# Pre-flight: refuse to proceed if MCP processes still hold the DB
184+
# file open. POSIX would silently let us unlink the inode while
185+
# they keep reading it; Windows would error here anyway. See
186+
# _find_live_mcp_processes for the full story. --force is the
187+
# documented escape hatch for scripted/CI runs.
188+
if not force:
189+
_abort_if_mcp_processes_alive()
190+
97191
logger.info("Resetting database...")
98192
config_manager = ConfigManager()
99193
app_config = config_manager.config

tests/cli/test_db_reset_exit.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,14 @@ def _isolated_env(tmp_path: Path) -> dict[str, str]:
2626

2727
@skip_on_windows
2828
def test_bm_reset_exits_cleanly(tmp_path: Path):
29-
"""bm reset should finish and exit cleanly with non-interactive confirmation."""
29+
"""bm reset should finish and exit cleanly with non-interactive confirmation.
30+
31+
Uses --force to skip the live-MCP pre-flight (#765); we're verifying
32+
process exit semantics here, not the pre-flight, which has dedicated
33+
coverage in test_db_reset_zombie_check.py.
34+
"""
3035
result = subprocess.run(
31-
["uv", "run", "bm", "reset"],
36+
["uv", "run", "bm", "reset", "--force"],
3237
input="y\n",
3338
capture_output=True,
3439
text=True,
@@ -44,7 +49,7 @@ def test_bm_reset_exits_cleanly(tmp_path: Path):
4449
def test_bm_reset_reindex_exits_cleanly(tmp_path: Path):
4550
"""bm reset --reindex should finish and exit cleanly with non-interactive confirmation."""
4651
result = subprocess.run(
47-
["uv", "run", "bm", "reset", "--reindex"],
52+
["uv", "run", "bm", "reset", "--reindex", "--force"],
4853
input="y\n",
4954
capture_output=True,
5055
text=True,
Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
"""Regression tests for the live-MCP-process pre-flight in `bm reset` (#765)."""
2+
3+
from __future__ import annotations
4+
5+
import os
6+
7+
import psutil
8+
import pytest
9+
import typer
10+
11+
from basic_memory.cli.commands import db as db_cmd
12+
13+
14+
class _FakeProc:
15+
"""Minimal stand-in for psutil.Process; only exposes .info."""
16+
17+
def __init__(self, pid: int, cmdline: list[str] | None):
18+
self.info = {"pid": pid, "cmdline": cmdline}
19+
20+
21+
def _patch_iter(monkeypatch: pytest.MonkeyPatch, procs: list[_FakeProc]) -> None:
22+
monkeypatch.setattr(
23+
psutil,
24+
"process_iter",
25+
lambda attrs=None: iter(procs),
26+
)
27+
28+
29+
class TestFindLiveMcpProcesses:
30+
def test_returns_empty_when_no_mcp_processes(self, monkeypatch):
31+
_patch_iter(
32+
monkeypatch,
33+
[
34+
_FakeProc(pid=11, cmdline=["python", "-m", "http.server"]),
35+
_FakeProc(pid=22, cmdline=["bm", "sync"]),
36+
],
37+
)
38+
assert db_cmd._find_live_mcp_processes() == []
39+
40+
def test_matches_basic_memory_mcp_invocations(self, monkeypatch):
41+
_patch_iter(
42+
monkeypatch,
43+
[
44+
_FakeProc(pid=101, cmdline=["/usr/bin/python", "basic-memory", "mcp"]),
45+
_FakeProc(pid=202, cmdline=["bm", "mcp"]), # alt entrypoint, no match
46+
_FakeProc(
47+
pid=303,
48+
cmdline=["python", "-m", "basic_memory.cli.main", "basic-memory", "mcp"],
49+
),
50+
],
51+
)
52+
result = db_cmd._find_live_mcp_processes()
53+
# 101 and 303 match (cmdline contains both "basic-memory" and exact "mcp" arg).
54+
# 202 is intentionally excluded because the joined cmdline has no "basic-memory".
55+
pids = sorted(pid for pid, _ in result)
56+
assert pids == [101, 303]
57+
58+
def test_skips_current_process(self, monkeypatch):
59+
me = os.getpid()
60+
_patch_iter(
61+
monkeypatch,
62+
[
63+
_FakeProc(pid=me, cmdline=["python", "basic-memory", "mcp"]),
64+
],
65+
)
66+
# Self-match is suppressed so the helper can be called from inside
67+
# `bm reset` without flagging the running process.
68+
assert db_cmd._find_live_mcp_processes() == []
69+
70+
def test_skips_processes_with_no_cmdline(self, monkeypatch):
71+
_patch_iter(
72+
monkeypatch,
73+
[
74+
_FakeProc(pid=1, cmdline=None), # kernel-style process
75+
_FakeProc(pid=2, cmdline=[]),
76+
],
77+
)
78+
assert db_cmd._find_live_mcp_processes() == []
79+
80+
def test_swallows_per_process_errors(self, monkeypatch):
81+
"""A NoSuchProcess race during iteration must not abort the scan."""
82+
83+
class _Raising:
84+
@property
85+
def info(self):
86+
raise psutil.NoSuchProcess(pid=999)
87+
88+
_patch_iter(
89+
monkeypatch,
90+
[
91+
_Raising(),
92+
_FakeProc(pid=42, cmdline=["python", "basic-memory", "mcp"]),
93+
],
94+
)
95+
result = db_cmd._find_live_mcp_processes()
96+
assert [pid for pid, _ in result] == [42]
97+
98+
99+
class TestAbortIfMcpProcessesAlive:
100+
def test_no_op_when_no_processes(self, monkeypatch):
101+
monkeypatch.setattr(db_cmd, "_find_live_mcp_processes", lambda: [])
102+
# Must not raise — destructive work should proceed.
103+
db_cmd._abort_if_mcp_processes_alive()
104+
105+
def test_exits_with_pids_when_processes_alive(self, monkeypatch, capsys):
106+
monkeypatch.setattr(
107+
db_cmd,
108+
"_find_live_mcp_processes",
109+
lambda: [(123, "python basic-memory mcp"), (456, "uv run bm mcp wrapper")],
110+
)
111+
with pytest.raises(typer.Exit) as exc_info:
112+
db_cmd._abort_if_mcp_processes_alive()
113+
assert exc_info.value.exit_code == 1
114+
115+
captured = capsys.readouterr()
116+
# PIDs surface so the user can target the cleanup themselves.
117+
assert "123" in captured.out
118+
assert "456" in captured.out
119+
assert "MCP processes" in captured.out
120+
121+
def test_prints_platform_specific_cleanup_hint_posix(self, monkeypatch, capsys):
122+
monkeypatch.setattr(os, "name", "posix")
123+
monkeypatch.setattr(
124+
db_cmd,
125+
"_find_live_mcp_processes",
126+
lambda: [(7, "python basic-memory mcp")],
127+
)
128+
with pytest.raises(typer.Exit):
129+
db_cmd._abort_if_mcp_processes_alive()
130+
out = capsys.readouterr().out
131+
assert "pgrep -fa 'basic-memory mcp'" in out
132+
133+
def test_prints_platform_specific_cleanup_hint_windows(self, monkeypatch, capsys):
134+
monkeypatch.setattr(os, "name", "nt")
135+
monkeypatch.setattr(
136+
db_cmd,
137+
"_find_live_mcp_processes",
138+
lambda: [(7, "python basic-memory mcp")],
139+
)
140+
with pytest.raises(typer.Exit):
141+
db_cmd._abort_if_mcp_processes_alive()
142+
out = capsys.readouterr().out
143+
assert "Get-CimInstance" in out

uv.lock

Lines changed: 30 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)