Skip to content

fix(mcp,ingest): _setup_claude parse-miss + stderr + atomic settings.json + timestamped backup#348

Open
Huntehhh wants to merge 3 commits into
buildingjoshbetter:mainfrom
Huntehhh:fix/mcp-setup-hygiene
Open

fix(mcp,ingest): _setup_claude parse-miss + stderr + atomic settings.json + timestamped backup#348
Huntehhh wants to merge 3 commits into
buildingjoshbetter:mainfrom
Huntehhh:fix/mcp-setup-hygiene

Conversation

@Huntehhh
Copy link
Copy Markdown
Contributor

@Huntehhh Huntehhh commented May 17, 2026

Summary

Four small polish fixes layered on top of #346's ASR + shim-migration work. All scoped to _setup_claude and _run_install; behaviour-preserving for the happy path, fixes real footguns on the unhappy paths.

Depends on #346 — branched from fix/windows-asr-trampoline-bypass so the _is_shim_path() migration is in the base. Rebases mechanically onto upstream main when #346 merges. The merge-order recommendation #346 → this PR is already documented in the multi-agent coordination plan.

What changed

mcp_server.py:_setup_claude

  1. Parse-miss preserves existing registration (both Claude Code + Claude Desktop branches). The old code treated an empty existing_cmd as stale and force-removed — which silently destroyed any working dev-venv path the user had set up if claude mcp list's output format ever drifted or the regex missed for any other reason. Now empty → preserve, with a one-line stderr note explaining how to recover. Symmetric fix on the Desktop side: an entry with an empty/missing command field is preserved so any user-set env / args / cwd survive.

  2. All _setup_claude print() calls route to sys.stderr — defence in depth for the MCP stdio protocol. --setup returns before mcp.run() so the current behaviour is safe, but any future refactor that calls _setup_claude mid-session would silently corrupt the JSON-RPC transport on any bare print(). User runs --setup interactively, so stderr lands in their terminal anyway.

ingest/cli.py:_run_install

  1. Atomic settings.json write — the previous direct write_text() truncated then wrote. A concurrent hook reader (SessionStart, Stop) landing inside that window parsed a partial/empty file and crashed with JSONDecodeError, silently breaking lifecycle hooks until the next install. Switched to tmp + Path.replace(): atomic on POSIX, best-effort on Windows (much narrower window). Falls back to direct write on cross-volume rename failure so the user never loses their config.

ingest/cli.py:_merge_claude_md

  1. Timestamped CLAUDE.md.bak.<unix-ts> backup filename — the previous fixed .with_suffix(".md.bak") silently clobbered the prior backup on every re-install. Re-installs across --user arg swaps or upgrades destroyed the backup chain. Timestamping preserves it.

Tests

tests/test_mcp_setup_hygiene.py — 6 new tests, all passing:

  • Parse-miss preserves Claude Code entry + diagnostic on stderr
  • Stale (non-existent) path replaces Claude Code entry
  • Empty command field preserves Claude Desktop entry (user env survives)
  • Stale path replaces Claude Desktop entry
  • Atomic settings.json write leaves no .tmp leftover on disk
  • CLAUDE.md.bak timestamping produces distinct files across runs

Test plan

  • pytest tests/test_mcp_setup_hygiene.py -v — all 6 pass on Linux / macOS / Windows
  • Fresh install on Windows: register, then re-run python -m truememory.mcp_server --setup — verify "existing config preserved" (not "stale entry replaced") in the setup output
  • Pre-existing user with custom Claude Desktop env config: re-run --setup — verify env block survives untouched
  • Concurrent SessionStart hook + truememory-ingest install: hook reader never sees JSONDecodeError
  • Re-run truememory-ingest install twice with different CLAUDE.md content — two distinct .bak.<ts> files exist

What I deliberately did NOT bundle

Saved as separate PRs in the multi-agent coordination plan:

  • Windows subprocess portability (start_new_session=True Windows guards, ps/pgrep cross-platform branches, AF_UNIX fallback) — agent-B's PR-2 scope
  • Windows CI runner + test_cli_help shim-invocation fixes — agent-B's PR-2b scope (two pre-existing tests/test_cli_help.py tests invoke the bare truememory-mcp.exe / truememory-ingest.exe shims directly and ASR-block on Windows Block-mode hosts)
  • Docs Windows callouts in docs/setup-*.md, docs/cli.md, docs/guides/debugging.md — shipping next as a separate PR

Pre-merge note for maintainers

This is one of a series of focused PRs from the multi-agent TrueMemory hardening sweep. The recommended merge order (other agents' PRs included) is:

#344 → #345 → #346 → PR-2a → PR-2b → this PR (PR-3) → #347 → PR-4

Order constraints affecting this PR specifically: #346 must land first (this PR's _setup_claude changes layer on top of #346's _is_shim_path() migration). Everything else can shift.

Merge ordering

Depends on: #346 (agent-B installer ASR bypass — agent-C's parse-miss guard + stderr routing in mcp_server.py:1280-1400 layers on top of agent-B's _is_shim_path() shim-migration in the same region). Clean mechanical merge once #346 lands.

Blocks: none.

Recommended sequence position: directly after #346 / #351 (PR-2b subprocess portability).

Huntehhh and others added 3 commits May 16, 2026 17:44
install.ps1 and install.sh invoked `truememory-mcp --setup` and
`truememory-ingest install` directly. On Windows hosts running Defender's
Attack-Surface-Reduction rule 01443614 in Block mode, those console-
script shims (setuptools / uv trampolines with per-install unique
hashes) silently fail the cloud-prevalence check and never launch.

Switched both installers to `\$toolPython -m truememory.mcp_server` /
`\$toolPython -m truememory.ingest.cli`, routing through the high-
prevalence signed python.exe — invisible to ASR. This mirrors the
invocation form `mcp_server._setup_claude` already writes into the
Claude Code / Claude Desktop MCP config, so installer and runtime are
now consistent.

Added a Windows-ASR troubleshooting note to the install.ps1 footer and
to the README's advanced-setup section so Block-mode users have the
manual re-run command.

Co-Authored-By: claude-opus-4-7 <wontreply@getfucked.ai>
…th migration

Bundles install-script hardening and a one-time _setup_claude shim-path
migration with the ASR fix so users with stale Claude MCP configs from
prior installs get auto-migrated to the python -m form.

install.ps1 / install.sh
- Soften the early Die-on-missing-toolPython to a Warn so
  TRUEMEMORY_SKIP_SETUP=1 (and the model-download fallback) still work
- install.sh: add `set -o pipefail` so mid-pipeline failures aren't
  silently swallowed (currently the script reports "installed
  successfully" on a broken install)
- install.ps1: quote $PKG_SPEC so TRUEMEMORY_SOURCE paths with spaces
  don't get split by PowerShell's argument parser
- install.ps1: surface `uv tool uninstall` exit codes > 1 (locked
  files, permissions) instead of swallowing them with `*> $null`
- install.ps1 footer: add a Block-mode ASR hint with the python -m
  equivalent commands so users who copy-paste the bare-shim commands
  have an escape hatch

truememory/mcp_server.py: _setup_claude
- New _is_shim_path() detects setuptools / uv truememory-mcp shim
  command paths in existing Claude Code / Claude Desktop configs
- When such a path is found, migrate the registration to the
  [python_path, "-m", "truememory.mcp_server"] form instead of
  preserving it (the previous "existing config preserved" branch left
  ASR-blocked shim paths in place — every Claude launch then tried to
  CreateProcess the blocked binary)
- Migration is reported as "migrated from shim to python -m form" in
  the setup output so the change is visible
- Route the two remaining stdout error prints in _setup_claude to
  stderr for MCP-stdio-protocol safety

README.md
- Step 4: clarify Windows "quit" — clicking X minimizes to system
  tray, MCP config only reloads at a full process launch
- BibTeX citation version: 0.6.0 -> 0.6.8

CHANGELOG.md: full Unreleased / Fixed entries for all of the above

Co-Authored-By: claude-opus-4-7 <wontreply@getfucked.ai>
…json + timestamped backup

Four small, related polish fixes layered on top of PR buildingjoshbetter#346's ASR + shim-
migration work:

mcp_server.py:_setup_claude
- Treat empty existing_cmd as a parse miss (preserve the existing
  registration) rather than as a stale entry (force-remove + re-add).
  Pre-fix: `claude mcp list` output that didn't match the parser regex
  fell through to `_path_exists("")` = False = stale → silently
  destroyed any working dev-venv path the user had set up. Now preserve
  and emit a stderr note explaining how to recover.
- Same fix on the Claude Desktop branch: an entry with an empty /
  missing 'command' field is preserved so user-set env / args / cwd
  fields survive.
- Route the remaining stdout prints in _setup_claude (success summary
  block + Claude Code failure path) to sys.stderr. Defense in depth for
  the MCP stdio protocol — --setup exits before mcp.run() so the
  current behavior is safe, but a future refactor calling _setup_claude
  mid-session would silently corrupt JSON-RPC on any bare print().

ingest/cli.py:_run_install
- settings.json write via tmp + Path.replace() instead of direct
  write_text(). The old truncate+write pattern had a race window where
  a concurrent hook reader (SessionStart, Stop) saw a partial / empty
  file and crashed with JSONDecodeError. Atomic on POSIX, best-effort
  on Windows (much narrower window). Falls back to direct write on
  cross-volume rename failure.

ingest/cli.py:_merge_claude_md
- Timestamp the CLAUDE.md.bak filename — fixed `.with_suffix(".md.bak")`
  silently clobbered the previous backup on every re-install. Now
  `CLAUDE.md.bak.<unix-ts>` preserves the backup chain.

Tests (tests/test_mcp_setup_hygiene.py — 6 new):
- Parse-miss preserves Claude Code entry + diagnostic on stderr
- Stale (non-existent) path replaces Claude Code entry
- Empty command field preserves Claude Desktop entry (user env survives)
- Stale path replaces Claude Desktop entry
- Atomic settings.json write leaves no .tmp leftover
- CLAUDE.md.bak timestamping produces distinct files across runs

Co-Authored-By: claude-opus-4-7 <wontreply@getfucked.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant