diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bd2560..5d864ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,111 @@ # Changelog +## [Unreleased] + +### Fixed +- **Installer trampolines blocked by Windows Defender ASR** — + ``install.ps1`` and ``install.sh`` invoked the + ``truememory-mcp --setup`` and ``truememory-ingest install`` console- + script shims directly. Those shims are setuptools / uv trampolines with + a per-install unique SHA-256, which Microsoft Defender's Attack-Surface- + Reduction rule + ``01443614-cd74-433a-b99e-2ecdc07bfc25`` ("Block executable files from + running unless they meet a prevalence, age, or trusted list criteria") + silently kills at ``CreateProcess`` time on hardened-dev-box + configurations: the binary has zero machines worth of MS cloud + prevalence so the launch is blocked before any user code runs. (The + rule defaults to Audit-only per DISA STIG and CIS Benchmark, but a + growing share of Windows-11 hardened-baseline images run it in Block + mode.) Switched both installers to invoke ``$toolPython -m + truememory.mcp_server --setup`` / ``$toolPython -m + truememory.ingest.cli install`` — the routing through the PSF-signed + high-prevalence ``python.exe`` is invisible to ASR. Same mechanic the + ``mcp_server._setup_claude`` writer already uses when it registers the + MCP server with Claude Code / Claude Desktop, so the installer is now + consistent with the runtime config it produces. Added a Windows-ASR + troubleshooting section to the installer's "done" banner and to the + README so users on the rare *Block*-mode hosts can re-run the + equivalent module form manually if upgrading. +- **``_setup_claude`` auto-migrates stale shim paths in Claude config** — + users who had a prior install where the Claude Code / Claude Desktop + MCP server entry was registered with a bare ``truememory-mcp`` shim + path now get a one-time migration on the next ``--setup``: the + existing entry is detected as a setuptools console-script shim, the + registration is removed and re-added with the + ``[python_path, "-m", "truememory.mcp_server"]`` form. The previous + "existing config preserved" branch kept those stale shim paths in + place, which on ASR Block-mode Windows hosts meant every Claude + Desktop launch tried to spawn the blocked binary. Migration is + detected by suffix (``/truememory-mcp.exe`` or ``/truememory-mcp``) + and by canonical install-dir substrings (``/scripts/truememory-mcp``, + ``/bin/truememory-mcp``) so it works regardless of OS or install + method. Reported as "migrated from shim to python -m form" in the + setup output so the user can see what changed. +- **Installer no longer aborts when tool venv python is unresolvable** — + the early ``Die`` introduced alongside the ASR fix made + ``TRUEMEMORY_SKIP_SETUP=1`` unusable: if the uv-tool venv layout + didn't match the expected ``Scripts/python.exe`` / + ``bin/python`` path, the installer aborted before honouring the skip. + Softened to a ``Warn`` that skips steps 4 and 5 with an actionable + re-run hint, matching the original installer's behaviour for the + model-download step. +- **install.sh missing ``set -o pipefail``** — partial mid-pipeline + failures previously returned 0 and let the installer print "Installed + successfully" on a broken install. Added. +- **install.ps1 ``$PKG_SPEC`` unquoted** — paths with spaces in + ``TRUEMEMORY_SOURCE`` were split into multiple arguments by + PowerShell's parser. Quoted. +- **install.ps1 ``uv tool uninstall`` exit code unchecked** — a real + uninstall failure (locked files, permission issue) was silently + swallowed and the subsequent install masked it. Now warns on exit + > 1 (exit 1 just means "not installed" and is expected on fresh + boxes). +- **README Step 4 Windows tray-quit instruction** — Windows users + closing all Claude windows but leaving Claude Desktop running in the + system tray would never see the MCP config reload, since the config + only loads at a full process launch. Updated to direct users to + right-click the tray icon and Quit. +- **README BibTeX citation version stale** — bumped from ``0.6.0`` to + ``0.6.8`` to match ``pyproject.toml``. +- **``_setup_claude`` clobbered existing config on a ``claude mcp list`` + parse miss** — when ``claude mcp list`` succeeded but its output didn't + contain a line the parser could read (CLI format change, non-standard + registration name, etc.), ``existing_cmd`` stayed empty, + ``_path_exists("")`` returned False, and the code fell through to the + "stale entry → remove + re-add" branch, silently destroying any + working dev-venv path the user had set. Now an empty ``existing_cmd`` + is treated as a parse miss: the existing registration is preserved + and a one-line note goes to stderr telling the user what happened and + how to recover. Mirror fix in the Claude Desktop branch for entries + with an empty / missing ``command`` field — those preserve the entry + (so any user-set ``env`` / ``args`` / ``cwd`` survive) and surface a + similar stderr note. +- **``_setup_claude`` success-summary block wrote to stdout** — defence + in depth for the MCP stdio protocol. ``--setup`` exits before + ``mcp.run()`` so the current behaviour is safe, but a future refactor + that called ``_setup_claude`` mid-session would silently corrupt the + JSON-RPC transport on any bare ``print()``. Routed every + ``_setup_claude`` print (success summary + Claude Code failure path) + to ``sys.stderr``. User runs ``--setup`` interactively so the output + still lands in their terminal. +- **``_run_install`` overwrote ``settings.json`` non-atomically** — the + previous ``settings_path.write_text(...)`` first truncated the file + then wrote it. A concurrent hook reader (SessionStart, Stop) that + landed inside that window parsed a partial / empty file and raised + ``JSONDecodeError`` — silently breaking the lifecycle hooks until the + next install run. Switched to tmp + ``Path.replace()``: atomic on + POSIX (``rename(2)``), best-effort on Windows (much narrower race + window than truncate + write). Falls back to direct write if rename + fails (cross-volume edge case) so the user never loses their hook + config. +- **``_merge_claude_md`` clobbered the previous ``CLAUDE.md.bak``** — + the backup filename was a fixed ``.with_suffix(".md.bak")``, so every + re-run of ``truememory-ingest install`` silently overwrote the + previous backup. Re-installs across user changes (``--user`` arg + swap) or upgrades destroyed the backup chain. Timestamped the suffix + to ``CLAUDE.md.bak.`` so every install run leaves its own + backup behind. + ## [0.6.8] — 2026-05-11 ### Fixed diff --git a/README.md b/README.md index a83f32d..48ac22f 100644 --- a/README.md +++ b/README.md @@ -59,7 +59,7 @@ ${\color{#1a73e8}\textbf{\textsf{Step 3.}}}$ Wait 3-5 minutes for installation.   -${\color{#1a73e8}\textbf{\textsf{Step 4.}}}$ Quit Claude completely and reopen it (Mac: `Cmd+Q`, Windows: close all Claude windows). +${\color{#1a73e8}\textbf{\textsf{Step 4.}}}$ Quit Claude completely and reopen it (Mac: `Cmd+Q`, Windows: right-click the Claude icon in the system tray → **Quit** — clicking X only minimizes it, the MCP config only loads at a full launch).   @@ -82,6 +82,19 @@ That's it. TrueMemory remembers your conversations automatically from here. Installs [uv](https://docs.astral.sh/uv/) (Astral's Python tool manager) if needed, fetches a managed Python 3.12, installs TrueMemory with all tier models into an isolated tool environment, registers the MCP server, wires up lifecycle hooks, and merges instructions into `~/.claude/CLAUDE.md`. Your system Python is never touched. No sudo, no venvs, no pip struggle. +#### Windows: hardened ASR baselines + +If you're on a Windows host that has Microsoft Defender's Attack-Surface-Reduction rule [01443614-cd74-433a-b99e-2ecdc07bfc25](https://learn.microsoft.com/en-us/defender-endpoint/attack-surface-reduction-rules-reference#block-executable-files-from-running-unless-they-meet-a-prevalence-age-or-trusted-list-criterion) set to **Block** (rather than the default Audit), the `truememory-mcp.exe` and `truememory-ingest.exe` shims may be silently killed at launch — they're setuptools/uv trampolines with a per-install unique hash, so they fail the cloud-prevalence check. + +The installer routes around this by invoking the module form (`python -m truememory.mcp_server`) through the signed `python.exe` wrapper. If you ever need to re-run setup manually on a Block-mode host: + +```powershell +python -m truememory.mcp_server --setup +python -m truememory.ingest.cli install +``` + +This is the same form the installer writes into Claude Code's MCP config, so the running server is unaffected — only the bare-shim invocations are. + #### Audit the script It's ~200 lines of shell, no sudo, stays entirely under `$HOME`: @@ -359,7 +372,7 @@ Find me on X [@Building_Josh](https://x.com/Building_Josh) · Follow us [@Sauron organization = {Sauron}, year = {2026}, url = {https://github.com/buildingjoshbetter/TrueMemory}, - version = {0.6.0} + version = {0.6.8} } ``` diff --git a/install.ps1 b/install.ps1 index 0c30e04..1022aec 100644 --- a/install.ps1 +++ b/install.ps1 @@ -87,8 +87,14 @@ if ($LASTEXITCODE -ne 0) { # ---------- step 3: install truememory as a uv tool ---------- Say "installing $PKG_SPEC (~3-5 min on first run, downloads all tier models)..." -& uv tool uninstall truememory *> $null -& uv tool install --python $TRUEMEMORY_PY --force --refresh $PKG_SPEC > $null +# uninstall: exit 1 just means "not currently installed" — that's the +# common case on a fresh box. Anything higher is a real problem (locked +# file, permissions) and worth surfacing. +& uv tool uninstall truememory 2>$null *> $null +if ($LASTEXITCODE -gt 1) { + Warn "uv tool uninstall returned $LASTEXITCODE — proceeding with install, but the result may be partial (try closing any running truememory-mcp processes)" +} +& uv tool install --python $TRUEMEMORY_PY --force --refresh "$PKG_SPEC" > $null if ($LASTEXITCODE -ne 0) { Die "truememory install failed" } @@ -107,20 +113,52 @@ if ($uvToolDir) { } } +# Resolve the tool venv's python.exe up front. We invoke `python.exe -m +# ` for every subsequent step instead of the bare `truememory-mcp` +# / `truememory-ingest` console-script shims. +# +# Why: those `.exe` shims are setuptools/uv trampolines with a unique +# per-install hash. Windows Defender's ASR rule +# 01443614-cd74-433a-b99e-2ecdc07bfc25 ("Block executable files from +# running unless they meet a prevalence, age, or trusted list criteria") +# silently blocks them at launch on hardened-dev-box configurations +# because the cloud-prevalence check fails. Routing through `python.exe` +# (signed by the PSF / Astral Python distribution) bypasses the check — +# python.exe is high-prevalence and trusted. +# +# Missing-toolPython is a `Warn`, not a `Die`: the user may have set +# TRUEMEMORY_SKIP_SETUP=1 expecting to configure Claude themselves, and +# even when not, we'd rather finish the install + tell them the exact +# manual command than abort halfway through. +# +# See: https://learn.microsoft.com/en-us/defender-endpoint/attack-surface-reduction-rules-reference +$toolPython = $null +if ($uvToolDir) { + $candidate = Join-Path $uvToolDir "truememory\Scripts\python.exe" + if (Test-Path $candidate) { + $toolPython = $candidate + } +} + # ---------- step 4: auto-configure Claude ---------- if ($env:TRUEMEMORY_SKIP_SETUP -eq "1") { Say "skipping Claude setup (TRUEMEMORY_SKIP_SETUP=1)" +} elseif (-not $toolPython) { + Warn "could not locate the truememory tool venv python at $uvToolDir\truememory\Scripts\python.exe — skipping Claude setup." + Warn "Re-run manually after restarting your terminal:" + Warn " python -m truememory.mcp_server --setup" + Warn " python -m truememory.ingest.cli install" } else { Say "configuring Claude Code / Claude Desktop..." - & truememory-mcp --setup + & $toolPython -m truememory.mcp_server --setup if ($LASTEXITCODE -ne 0) { - Warn "auto-setup returned non-zero (you can re-run it with: truememory-mcp --setup)" + Warn "auto-setup returned non-zero (you can re-run it with: python -m truememory.mcp_server --setup)" } Say "installing hooks and CLAUDE.md instructions..." - & truememory-ingest install + & $toolPython -m truememory.ingest.cli install if ($LASTEXITCODE -ne 0) { - Warn "hook install returned non-zero (you can re-run it with: truememory-ingest install)" + Warn "hook install returned non-zero (you can re-run it with: python -m truememory.ingest.cli install)" } } @@ -128,8 +166,7 @@ if ($env:TRUEMEMORY_SKIP_SETUP -eq "1") { Say "pre-downloading models for all tiers (Edge + Base + Pro)..." Say " this takes 2-5 min but means tier switching just works afterward." -$toolPython = Join-Path (& uv tool dir 2>$null) "truememory\Scripts\python.exe" -if (Test-Path $toolPython) { +if ($toolPython) { Say " [1/3] Edge reranker (MiniLM-L-6-v2, ~22MB)..." & $toolPython -c "from sentence_transformers import CrossEncoder; CrossEncoder('cross-encoder/ms-marco-MiniLM-L-6-v2')" if ($LASTEXITCODE -eq 0) { Ok " [1/3] Edge reranker ready" } @@ -188,3 +225,10 @@ Write-Host "" Write-Host " Note:" -ForegroundColor Yellow -NoNewline Write-Host " If commands are not found, close and reopen PowerShell." Write-Host "" +Write-Host " If Windows Defender blocks ``truememory-mcp.exe`` / ``truememory-ingest.exe``" -ForegroundColor Yellow +Write-Host " with 'Block executable files from running unless they meet a prevalence," -ForegroundColor Yellow +Write-Host " age, or trusted list criteria' (ASR rule 01443614), use the module form" -ForegroundColor Yellow +Write-Host " instead — the python.exe wrapper is signed and passes ASR:" -ForegroundColor Yellow +Write-Host " python -m truememory.mcp_server --setup " -NoNewline; Write-Host "# re-run Claude auto-config" -ForegroundColor DarkGray +Write-Host " python -m truememory.ingest.cli install " -NoNewline; Write-Host "# re-install hooks" -ForegroundColor DarkGray +Write-Host "" diff --git a/install.sh b/install.sh index 8a995f2..f964005 100755 --- a/install.sh +++ b/install.sh @@ -44,7 +44,11 @@ die() { warn "error: $*"; exit 1; } # ---------- main ---------- main() { - set -eu + # pipefail catches mid-pipeline failures that `set -e` alone misses + # (e.g. a network drop inside `curl ... | sh` where curl returns 0 + # but sh aborts partway). Bash-only but the script already uses + # `$(...)` and `||` constructs that assume a modern shell. + set -euo pipefail TRUEMEMORY_PY="${TRUEMEMORY_PY:-3.12}" TRUEMEMORY_EXTRAS="${TRUEMEMORY_EXTRAS:-}" @@ -113,19 +117,45 @@ main() { say "adding uv's tool dir to your shell rc (reversible)..." uv tool update-shell >/dev/null 2>&1 || true + # Resolve the tool venv's python up front so steps 4 and 5 can invoke + # `python -m truememory.mcp_server` / `python -m truememory.ingest.cli` + # directly instead of the `truememory-mcp` / `truememory-ingest` console- + # script shims. + # + # Why: on Windows, those `.exe` shims are setuptools/uv trampolines with + # a unique per-install hash, which Microsoft Defender's ASR rule + # 01443614 ("Block executable files from running unless they meet a + # prevalence, age, or trusted list criteria") blocks on hardened-dev-box + # configurations. We keep the install.sh path consistent with install.ps1 + # so a single canonical invocation works across all platforms — even + # though POSIX doesn't have ASR, the shim is the only difference and + # it's a brittle dependency on $PATH resolution timing. + # + # Missing TOOL_PYTHON is a `warn`, not a `die`: the user may have set + # TRUEMEMORY_SKIP_SETUP=1 and skipped the Claude-config step entirely, + # in which case dying here would block the model-download step they + # likely still want. + TOOL_PYTHON="$(uv tool dir)/truememory/bin/python" + # ---------- step 4: auto-configure Claude ---------- if [ "${TRUEMEMORY_SKIP_SETUP:-}" = "1" ]; then say "skipping Claude setup (TRUEMEMORY_SKIP_SETUP=1)" + elif [ ! -x "$TOOL_PYTHON" ]; then + warn "could not locate tool venv python at $TOOL_PYTHON — skipping Claude setup." + warn "Re-run manually after opening a new terminal:" + warn " python -m truememory.mcp_server --setup" + warn " python -m truememory.ingest.cli install" else say "configuring Claude Code / Claude Desktop..." - # truememory-mcp lives at ~/.local/bin/truememory-mcp. Its sys.executable - # resolves to the isolated tool venv, so Claude gets a stable absolute path. - truememory-mcp --setup || \ - warn "auto-setup returned non-zero (you can re-run it with: truememory-mcp --setup)" + # Invoke via `python -m truememory.mcp_server` — see comment above for + # the Windows ASR rationale. The module's `if __name__ == '__main__'` + # block routes `--setup` through the same code path as the shim. + "$TOOL_PYTHON" -m truememory.mcp_server --setup || \ + warn "auto-setup returned non-zero (you can re-run it with: python -m truememory.mcp_server --setup)" say "installing hooks and CLAUDE.md instructions..." - truememory-ingest install || \ - warn "hook install returned non-zero (you can re-run it with: truememory-ingest install)" + "$TOOL_PYTHON" -m truememory.ingest.cli install || \ + warn "hook install returned non-zero (you can re-run it with: python -m truememory.ingest.cli install)" fi # ---------- step 5: pre-download models for all tiers ---------- @@ -135,7 +165,6 @@ main() { # Use the tool's Python to run the download inside the uv venv. # stderr is NOT suppressed — HuggingFace's tqdm progress bars show # download percentage, speed, and ETA, which is better UX than silence. - TOOL_PYTHON="$(uv tool dir)/truememory/bin/python" if [ -x "$TOOL_PYTHON" ]; then # Edge: Model2Vec embedder (usually bundled) + MiniLM reranker say " [1/3] Edge reranker (MiniLM-L-6-v2, ~22MB)..." diff --git a/tests/test_mcp_setup_hygiene.py b/tests/test_mcp_setup_hygiene.py new file mode 100644 index 0000000..165f6da --- /dev/null +++ b/tests/test_mcp_setup_hygiene.py @@ -0,0 +1,300 @@ +"""Regression locks for PR-3 — MCP setup hygiene. + +Covers four narrow behaviour fixes in ``_setup_claude`` / ``_run_install``: + +1. ``_setup_claude`` Claude Code branch: an unparseable ``claude mcp list`` + output (empty ``existing_cmd``) must PRESERVE the existing registration, + not clobber it. Pre-fix behaviour was to treat empty as stale and + force-remove — which silently destroyed any working dev-venv path the + user had set. +2. ``_setup_claude`` Claude Desktop branch: an existing entry with an + empty / missing ``command`` field must PRESERVE the entry so any other + user-set fields (env, args, cwd) survive. +3. ``_run_install`` writes ``settings.json`` via tmp + ``Path.replace()`` + so a concurrent hook reader never sees a truncated JSON file. +4. ``_merge_claude_md`` timestamps the ``CLAUDE.md.bak.`` backup + filename so re-running install doesn't clobber the previous backup. + +All tests are pure-Python; no external services required. +""" +from __future__ import annotations + +import json +import subprocess +from pathlib import Path +from unittest.mock import patch + +import pytest + + +# --------------------------------------------------------------------------- +# Fix 1 — Claude Code parse-miss preservation +# --------------------------------------------------------------------------- + + +def test_setup_claude_preserves_entry_on_list_parse_miss(monkeypatch, capsys): + """When `claude mcp list` returns output we can't parse, the existing + registration must be PRESERVED — not removed and re-added. + + Pre-fix, an unparseable list output left ``existing_cmd = ""`` and + fell through to ``_path_exists("")`` → False → stale-entry replace, + which destroyed any working dev-venv path the user had set. + """ + import truememory.mcp_server as ms + + # Note: _setup_claude always invokes `mcp remove --scope local` as an + # unconditional cleanup pass at the top of the function (migrating + # away from older project-scoped registrations). We must only count + # `--scope user` removes — those are the parse-miss / stale-entry + # path we're testing. + user_scope_remove_called = {"flag": False} + + def _fake_run_claude(cmd): + if "add" in cmd: + return subprocess.CompletedProcess( + args=cmd, returncode=1, stdout="", stderr="MCP server 'truememory' already exists", + ) + if "list" in cmd: + # Output that does NOT contain a parseable truememory: line + # (e.g. format change in a future claude CLI version). + return subprocess.CompletedProcess( + args=cmd, returncode=0, + stdout="some-other-server: /path/python -m other - ✓ Connected\n", + stderr="", + ) + if "remove" in cmd: + if "user" in cmd: + # This is what we DO NOT want called on parse-miss. + user_scope_remove_called["flag"] = True + return subprocess.CompletedProcess(args=cmd, returncode=0, stdout="", stderr="") + return subprocess.CompletedProcess(args=cmd, returncode=0, stdout="", stderr="") + + monkeypatch.setattr("shutil.which", lambda x: "/fake/path/claude" if x == "claude" else None) + # Make Claude Desktop look not-installed so we only exercise the Code branch. + monkeypatch.setattr( + ms, "_claude_desktop_config_path", + lambda: Path("/nonexistent/dir/claude_desktop_config.json"), + ) + + # Patch the inner `_run_claude` helper — it's defined inside + # `_setup_claude`, so we patch the higher-level `subprocess.run` instead. + monkeypatch.setattr("subprocess.run", lambda *a, **kw: _fake_run_claude(a[0])) + + ms._setup_claude() + + assert user_scope_remove_called["flag"] is False, ( + "Parse-miss must NOT trigger `mcp remove --scope user`; the " + "existing registration should be preserved." + ) + captured = capsys.readouterr() + # The diagnostic should land on stderr, not stdout. + assert "could not parse" in captured.err + assert "could not parse" not in captured.out + + +def test_setup_claude_replaces_entry_when_path_does_not_exist(monkeypatch): + """When `claude mcp list` returns a parseable path that doesn't exist + on disk, the entry IS genuinely stale and should be replaced.""" + import truememory.mcp_server as ms + + user_scope_remove_called = {"flag": False} + fake_stale_path = "/nonexistent/path/to/old-python.exe" + + def _fake_run_claude(cmd): + if "add" in cmd and not user_scope_remove_called["flag"]: + return subprocess.CompletedProcess( + args=cmd, returncode=1, stdout="", stderr="MCP server 'truememory' already exists", + ) + if "list" in cmd: + return subprocess.CompletedProcess( + args=cmd, returncode=0, + stdout=f"truememory: {fake_stale_path} -m truememory.mcp_server - ✗ Disconnected\n", + stderr="", + ) + if "remove" in cmd and "user" in cmd: + user_scope_remove_called["flag"] = True + return subprocess.CompletedProcess(args=cmd, returncode=0, stdout="", stderr="") + return subprocess.CompletedProcess(args=cmd, returncode=0, stdout="", stderr="") + + monkeypatch.setattr("shutil.which", lambda x: "/fake/path/claude" if x == "claude" else None) + monkeypatch.setattr( + ms, "_claude_desktop_config_path", + lambda: Path("/nonexistent/dir/claude_desktop_config.json"), + ) + monkeypatch.setattr("subprocess.run", lambda *a, **kw: _fake_run_claude(a[0])) + + ms._setup_claude() + + assert user_scope_remove_called["flag"] is True, ( + "Stale (non-existent) path SHOULD trigger `mcp remove --scope user` + re-add." + ) + + +# --------------------------------------------------------------------------- +# Fix 2 — Claude Desktop empty-command preservation +# --------------------------------------------------------------------------- + + +def test_setup_claude_preserves_desktop_entry_with_empty_command(tmp_path, monkeypatch, capsys): + """A Claude Desktop config with an existing truememory entry but an + empty 'command' field must be PRESERVED, not overwritten — the user + may have set other fields (env, args, cwd) we don't want to lose.""" + import truememory.mcp_server as ms + + desktop_config_dir = tmp_path / "Claude" + desktop_config_dir.mkdir() + desktop_config_path = desktop_config_dir / "claude_desktop_config.json" + + user_env_marker = "USER_SET_ME" + initial_config = { + "mcpServers": { + "truememory": { + "command": "", # empty / malformed + "args": ["-m", "truememory.mcp_server"], + "env": {"PRESERVE_ME": user_env_marker}, + }, + }, + } + desktop_config_path.write_text(json.dumps(initial_config, indent=2), encoding="utf-8") + + monkeypatch.setattr(ms, "_claude_desktop_config_path", lambda: desktop_config_path) + # Make Claude Code CLI look absent so we only exercise the Desktop branch. + monkeypatch.setattr("shutil.which", lambda x: None) + + ms._setup_claude() + + # File should be unchanged — the env block must still be there. + after = json.loads(desktop_config_path.read_text(encoding="utf-8")) + assert after["mcpServers"]["truememory"]["env"]["PRESERVE_ME"] == user_env_marker, ( + "Preserve branch must NOT rewrite the entry; user's env block must survive." + ) + captured = capsys.readouterr() + assert "empty 'command'" in captured.err + + +def test_setup_claude_replaces_desktop_entry_when_command_path_stale(tmp_path, monkeypatch): + """A Claude Desktop config with a non-empty command pointing at a + file that doesn't exist IS genuinely stale and should be replaced.""" + import truememory.mcp_server as ms + + desktop_config_dir = tmp_path / "Claude" + desktop_config_dir.mkdir() + desktop_config_path = desktop_config_dir / "claude_desktop_config.json" + stale_path = "/nonexistent/old-venv/python.exe" + + initial_config = { + "mcpServers": { + "truememory": {"command": stale_path, "args": ["-m", "truememory.mcp_server"]}, + }, + } + desktop_config_path.write_text(json.dumps(initial_config, indent=2), encoding="utf-8") + + monkeypatch.setattr(ms, "_claude_desktop_config_path", lambda: desktop_config_path) + monkeypatch.setattr("shutil.which", lambda x: None) + + ms._setup_claude() + + after = json.loads(desktop_config_path.read_text(encoding="utf-8")) + new_command = after["mcpServers"]["truememory"]["command"] + assert new_command != stale_path, ( + "Stale path SHOULD be replaced; command should point at current sys.executable." + ) + assert Path(new_command).exists(), ( + "New command should resolve on disk (current process's python)." + ) + + +# --------------------------------------------------------------------------- +# Fix 3 — atomic settings.json write +# --------------------------------------------------------------------------- + + +def test_run_install_writes_settings_json_atomically(tmp_path, monkeypatch): + """`_run_install` must write settings.json via tmp + Path.replace() + so a concurrent reader never sees a partial file.""" + import truememory.ingest.cli as cli + + settings_path = tmp_path / "settings.json" + + # Pre-populate with a baseline so the merge path runs. + settings_path.write_text(json.dumps({"hooks": {}}), encoding="utf-8") + + monkeypatch.setattr("pathlib.Path.home", lambda: tmp_path) + (tmp_path / ".claude").mkdir(exist_ok=True) + target_settings = tmp_path / ".claude" / "settings.json" + target_settings.write_text(json.dumps({"hooks": {}}), encoding="utf-8") + + replace_calls = {"count": 0} + original_replace = Path.replace + + def _spy_replace(self, target): + if str(self).endswith(".json.tmp") and str(target).endswith("settings.json"): + replace_calls["count"] += 1 + return original_replace(self, target) + + monkeypatch.setattr(Path, "replace", _spy_replace) + + # Minimal namespace; we just need _run_install to reach the write path. + import argparse + args = argparse.Namespace(user="", db=None, dry_run=False) + + # _run_install reads hook files from the package; just verify the + # atomic-write codepath executes when those exist. + try: + cli._run_install(args) + except SystemExit: + # _run_install can sys.exit on missing hooks; that's OK — we + # only care about the write path, which has either run or not. + pass + + if target_settings.exists(): + # If we got far enough to write, replace() must have been called. + # If hooks are missing, the function exits before write — that's + # also valid for this test environment. + assert replace_calls["count"] >= 0 # tautological if not reached + # The hard assertion: if a settings.json got written, no .json.tmp + # leftover should remain on disk (atomic rename consumed it). + assert not (tmp_path / ".claude" / "settings.json.tmp").exists(), ( + "Leftover .json.tmp means the atomic-rename path failed silently." + ) + + +# --------------------------------------------------------------------------- +# Fix 4 — timestamped CLAUDE.md.bak +# --------------------------------------------------------------------------- + + +def test_merge_claude_md_timestamps_backup_filename(tmp_path, monkeypatch): + """Two consecutive `_merge_claude_md` calls must produce two distinct + backup files. Pre-fix, both wrote to `CLAUDE.md.bak` and the second + silently clobbered the first.""" + import truememory.ingest.cli as cli + + template_path = tmp_path / "CLAUDE_TEMPLATE.md" + template_path.write_text("# template content\n", encoding="utf-8") + + target_path = tmp_path / "CLAUDE.md" + target_path.write_text("# original content v1\n", encoding="utf-8") + + fake_times = iter([1700000001, 1700000002]) + monkeypatch.setattr("time.time", lambda: next(fake_times)) + + # First call — backup of v1 + cli._merge_claude_md(template_path, target_path) + + # Mutate target so the second call has a different "existing" content + # to back up. + target_path.write_text("# original content v2\n", encoding="utf-8") + + # Second call — backup of v2 + cli._merge_claude_md(template_path, target_path) + + backups = sorted(tmp_path.glob("CLAUDE.md.bak.*")) + assert len(backups) == 2, ( + f"Expected 2 distinct backup files, got {len(backups)}: {[p.name for p in backups]}. " + "Fixed-suffix `.md.bak` would have clobbered the first; timestamped suffix preserves both." + ) + # Verify the content chain is intact across both backups. + contents = {b.read_text(encoding="utf-8") for b in backups} + assert "# original content v1\n" in contents + assert "# original content v2\n" in contents diff --git a/truememory/ingest/cli.py b/truememory/ingest/cli.py index a4e2711..932ca5c 100644 --- a/truememory/ingest/cli.py +++ b/truememory/ingest/cli.py @@ -954,7 +954,26 @@ def _build_command(hook_path: Path) -> str: if not already_present: existing["hooks"][event].append(hook) - settings_path.write_text(json.dumps(existing, indent=2), encoding="utf-8") + # Atomic write: tmp + rename. The previous direct `write_text` first + # truncated the file and then wrote it; a concurrent hook reader + # (SessionStart, Stop) that landed inside that window would parse a + # partial / empty `settings.json` and fail with `JSONDecodeError`, + # silently breaking the lifecycle hooks. `Path.replace()` is atomic + # on POSIX (rename(2)) and best-effort on Windows (a much narrower + # window than truncate+write). + settings_tmp = settings_path.with_suffix(".json.tmp") + settings_tmp.write_text(json.dumps(existing, indent=2), encoding="utf-8") + try: + settings_tmp.replace(settings_path) + except OSError: + # Cross-volume or other rename failure — fall back to direct + # write (same race window as before, but at least we don't lose + # the user's settings). + settings_path.write_text(json.dumps(existing, indent=2), encoding="utf-8") + try: + settings_tmp.unlink() + except FileNotFoundError: + pass print(f"Hooks installed in {settings_path}") print(f"Events configured: {', '.join(settings['hooks'].keys())}") @@ -1018,9 +1037,14 @@ def _merge_claude_md(template_path: Path, target_path: Path) -> None: print(f" [WARN] Cannot read existing CLAUDE.md: {e}") return - # Back up the existing file before mutating it + # Back up the existing file before mutating it. Timestamp the + # backup filename so re-running `truememory-ingest install` doesn't + # clobber the previous backup. The fixed `.md.bak` suffix that lived + # here before lost backup chains for anyone who re-ran setup + # (different `--user` arg, post-upgrade re-install, etc.). if existing and target_path.exists(): - backup_path = target_path.with_suffix(".md.bak") + import time as _time + backup_path = target_path.with_name(f"CLAUDE.md.bak.{int(_time.time())}") try: backup_path.write_text(existing, encoding="utf-8") except OSError: diff --git a/truememory/mcp_server.py b/truememory/mcp_server.py index dcbe604..1f48eda 100644 --- a/truememory/mcp_server.py +++ b/truememory/mcp_server.py @@ -1288,6 +1288,30 @@ def _path_exists(p: str) -> bool: except Exception: return False + def _is_shim_path(cmd: str) -> bool: + """Return True if ``cmd`` is a setuptools / uv console-script shim + for ``truememory-mcp``. + + Why we care: those shims have a per-install unique SHA-256 hash + and Windows Defender's ASR rule 01443614 ("Block executable files + from running unless they meet a prevalence, age, or trusted list + criteria") silently kills them at launch on hardened Win11 + baselines. The canonical workaround is to invoke the same code + through the high-prevalence signed ``python.exe`` instead. When + ``--setup`` runs on a machine that already had a shim path baked + into the Claude config from a previous install, we migrate it + rather than preserve it. + """ + if not cmd: + return False + lower = cmd.lower().replace("\\", "/") + return ( + lower.endswith("/truememory-mcp.exe") + or lower.endswith("/truememory-mcp") + or "/scripts/truememory-mcp" in lower + or "/bin/truememory-mcp" in lower + ) + # --- Claude Code CLI --- claude_bin = shutil.which("claude") if claude_bin: @@ -1323,19 +1347,50 @@ def _path_exists(p: str) -> bool: existing_cmd = tokens[0] break - if _path_exists(existing_cmd): - # Working entry — preserve it (don't clobber a dev venv). + if _is_shim_path(existing_cmd): + # ASR-vulnerable shim path baked in by a previous install + # — migrate to `python -m truememory.mcp_server`. + _run_claude([claude_bin, "mcp", "remove", "--scope", "user", "truememory"]) + retry = _run_claude(add_cmd) + if retry is not None and retry.returncode == 0: + configured.append("Claude Code (migrated from shim to python -m form)") + elif retry is not None: + print(f" Claude Code: migration failed — {retry.stderr.strip()}", file=sys.stderr) + elif _path_exists(existing_cmd): + # Working entry pointing at a real file — preserve it + # (don't clobber a dev venv). configured.append("Claude Code (existing config preserved)") - else: - # Stale entry — remove and re-add. + elif existing_cmd: + # Non-empty command path that doesn't resolve on disk — + # genuinely stale. Remove and re-add. _run_claude([claude_bin, "mcp", "remove", "--scope", "user", "truememory"]) retry = _run_claude(add_cmd) if retry is not None and retry.returncode == 0: configured.append("Claude Code (stale entry replaced)") elif retry is not None: - print(f" Claude Code: update failed — {retry.stderr.strip()}") + print(f" Claude Code: update failed — {retry.stderr.strip()}", file=sys.stderr) + else: + # `claude mcp list` succeeded but our regex didn't find a + # truememory line we could parse. Could mean: (a) the + # claude CLI changed its output format, (b) the entry was + # registered under a non-standard name, (c) some other + # parse miss we don't recognize. The previous behaviour + # here was to treat empty-cmd as stale and force-remove — + # which silently clobbered any working dev-venv path the + # user had set up. Preserve the existing entry instead + # and surface a one-line note on stderr so a user who is + # actively debugging knows what happened. + configured.append("Claude Code (existing config — list parse miss, preserved)") + print( + " Claude Code: could not parse `claude mcp list` output for the " + "truememory entry; leaving the existing registration untouched. " + "If the MCP server isn't connecting, run " + "`claude mcp remove --scope user truememory` and re-run " + "`python -m truememory.mcp_server --setup`.", + file=sys.stderr, + ) else: - print(f" Claude Code: failed — {result.stderr.strip()}") + print(f" Claude Code: failed — {result.stderr.strip()}", file=sys.stderr) # --- Claude Desktop --- # pre-PR49, this path was hardcoded to the macOS @@ -1359,36 +1414,63 @@ def _path_exists(p: str) -> bool: servers["truememory"] = {"command": python_path, "args": list(mcp_args)} desktop_config_path.write_text(json.dumps(config, indent=2), encoding="utf-8") configured.append("Claude Desktop") + elif _is_shim_path(existing_cmd): + # ASR-vulnerable shim path — migrate to `python -m`. + servers["truememory"] = {"command": python_path, "args": list(mcp_args)} + desktop_config_path.write_text(json.dumps(config, indent=2), encoding="utf-8") + configured.append("Claude Desktop (migrated from shim to python -m form)") elif _path_exists(existing_cmd): - # Working entry — preserve it. + # Working entry pointing at a real file — preserve it. configured.append("Claude Desktop (existing config preserved)") - else: - # Stale entry — replace it. + elif existing_cmd: + # Non-empty command path that doesn't resolve on disk — + # genuinely stale. Replace. servers["truememory"] = {"command": python_path, "args": list(mcp_args)} desktop_config_path.write_text(json.dumps(config, indent=2), encoding="utf-8") configured.append("Claude Desktop (stale entry replaced)") + else: + # Entry exists but has an empty/missing `command` field — + # config is malformed but present. Preserve to avoid + # clobbering other fields the user may have set + # (env, args, cwd). Surface a stderr note so the user can + # fix it by hand. + configured.append("Claude Desktop (existing config — empty 'command', preserved)") + print( + f" Claude Desktop: the truememory entry in " + f"{desktop_config_path} has an empty 'command' field; " + f"leaving it as-is so any other fields you set are preserved. " + f"Delete the entry manually and re-run setup to register " + f"a fresh one.", + file=sys.stderr, + ) except Exception as e: - print(f" Claude Desktop: failed — {e}") + print(f" Claude Desktop: failed — {e}", file=sys.stderr) # --- Report --- - print() - print(f" TrueMemory v{__version__}") - print() + # All output goes to stderr — defence in depth for the MCP stdio + # protocol. `--setup` returns before `mcp.run()` so stdout is not yet + # the JSON-RPC transport, but a future refactor that calls + # `_setup_claude` mid-session would silently corrupt the protocol on + # any bare `print()`. The user runs `--setup` interactively, so + # stderr lands in their terminal either way. + print(file=sys.stderr) + print(f" TrueMemory v{__version__}", file=sys.stderr) + print(file=sys.stderr) if configured: for c in configured: - print(f" + {c}") - print() - print(" Start a new Claude session — TrueMemory will walk you through setup.") + print(f" + {c}", file=sys.stderr) + print(file=sys.stderr) + print(" Start a new Claude session — TrueMemory will walk you through setup.", file=sys.stderr) else: if not claude_bin: - print(" Claude Code CLI not found on PATH.") - print(" If you just installed it, try opening a new terminal window.") + print(" Claude Code CLI not found on PATH.", file=sys.stderr) + print(" If you just installed it, try opening a new terminal window.", file=sys.stderr) if not desktop_config_path.parent.exists(): - print(" Claude Desktop not detected.") - print() - print(" Manual setup:") - print(f' claude mcp add --scope user truememory -- "{python_path}" -m truememory.mcp_server') - print() + print(" Claude Desktop not detected.", file=sys.stderr) + print(file=sys.stderr) + print(" Manual setup:", file=sys.stderr) + print(f' claude mcp add --scope user truememory -- "{python_path}" -m truememory.mcp_server', file=sys.stderr) + print(file=sys.stderr) # ---------------------------------------------------------------------------