feat(cli): add mm upgrade for kill-then-reinstall hygiene (#443)#530
Merged
feat(cli): add mm upgrade for kill-then-reinstall hygiene (#443)#530
mm upgrade for kill-then-reinstall hygiene (#443)#530Conversation
`uv tool install --reinstall memtomem` only swaps the on-disk bytes; any `memtomem-server` already imported by an MCP client keeps running the previous version until it exits. That split-brain is what produced the v0.1.25 → v0.1.26 stale `.server.pid` repro behind issue #443. `mm upgrade` wraps the reinstall with the missing process-level hygiene: probe the server pid lock, send SIGTERM, escalate to SIGKILL after `--grace` seconds, unlink the stale pid file, then run `uv tool install --refresh --reinstall memtomem`. `--version` pins a release, `--dry-run` previews the plan, `--json` emits a structured result. On Windows the kill stage is skipped automatically (POSIX advisory flock + signals are unavailable) and the user is told to stop the server manually if a split-brain shows up. The kill-then-reinstall ordering is the whole reason the command exists, so there is no `--skip-pkill` opt-out — advanced users who manage lifecycle elsewhere can keep calling `uv tool install` directly. Liveness helpers move from `uninstall_cmd.py` to a new `cli/_liveness.py` so both commands share one probe; the existing uninstall test suite (32 cases) stays green to confirm behavior is unchanged. Co-Authored-By: Claude <[email protected]>
…eview) Address review feedback on PR #530: - **Extras preserved across reinstall** (review issue 1, blocking). ``uv tool install --reinstall memtomem`` silently drops extras, so ``mm upgrade`` was regressing every ``memtomem[all]`` install down to BM25-only. ``_detect_installed_extras()`` now reads ``<uv tool dir>/memtomem/uv-receipt.toml`` and re-passes the same ``extras`` list on the reinstall command line. Override with ``--extras onnx,web`` or suppress with ``--extras none``. - **Re-probe before pid-file unlink** (review issue 3). After the SIGKILL settle window an MCP client could respawn a fresh server at the same pid path; we'd then delete its lockfile. ``_stop_server`` now calls ``probe_pid_file`` immediately before unlink and skips + warns when a live writer is detected. - **Drop ``while/else`` for SIGKILL escalation** (review issue 4) — flatten to a separate ``if`` so the control flow reads linearly. - **Cancel exits 0 with consistent JSON shape** (review issue 5). Voluntary ``click.confirm`` decline now returns 0 (was 1) and emits ``{ok:true, cancelled:true}`` under ``--json``. - **Stale comment fix** (review issue 2) — ``_ServerState`` is no longer aliased in ``uninstall_cmd.py`` so the comment now lists only the symbols actually re-imported. Adds 6 tests covering extras auto-detect / flag override / ``none`` / version+extras combo, the unlink-skip-on-respawn race, and the cancel exit-zero JSON shape. Co-Authored-By: Claude <[email protected]>
…review) Final review nit (issue 6). ``mm upgrade --version >=0.1.30`` would compose to ``memtomem==>=0.1.30`` and uv rejects it with a noisy parser error. Validate against a bare PEP 440 release pattern up front and emit a friendly ``BadParameter`` instead. Pre/post/dev/local versions (``0.1.30rc1``, ``0.1.30.post1``, ``0.1.30+local``) stay allowed. Co-Authored-By: Claude <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mm upgradecommand: probe live server → SIGTERM (escalate to SIGKILL after--grace) → unlink stale pid file →uv tool install --refresh --reinstall memtomem. Closes cli: addmm upgradeto stop running server before reinstall (hygiene wrapper) #443.uninstall_cmd.pyinto a newcli/_liveness.pyso both commands share one probe; existing uninstall behavior is unchanged (32 tests still pass).--skip-pkillopt-out — kill-then-reinstall is the whole reason the command exists. Windows auto-skips the kill stage (no fcntl/signals) and warns.Why
uv tool install --reinstall memtomemonly swaps the on-disk bytes. Anymemtomem-serveralready imported by an MCP client (Claude Code keeps the server alive across prompts) keeps running the previous version until it exits, producing a split-brain where pre-upgrade teardown bugs leak alongside the freshly written disk code. That's exactly the v0.1.25 → v0.1.26 stale.server.pidrepro that motivated issue #443.Surface
--versionpins a release; default is latest on the configured index.--grace(default 5s) is the SIGTERM → SIGKILL window.--dry-runprints the plan without killing or installing.--jsonemits{ok, killed, removed, reinstalled, version}(or{ok:false, error,…}).Test plan
pytest packages/memtomem/tests/test_upgrade_cmd.py— 9 new tests (no-running-server / SIGTERM / SIGKILL escalation / Windows-skips-kill / version pin / uv failure / dry-run / json shape / non-TTY-no-yes)pytest packages/memtomem/tests/test_uninstall_cmd.py— all 32 existing tests still pass after liveness helper extractionpytest packages/memtomem/tests/ -m "not ollama"— 3010 passruff check && ruff format --checkclean repo-wideuv run mm upgrade --help/--dry-runsmoke (detects real running server, prints plan)mm web, runmm upgrade --dry-run, thenmm upgrade --yes --grace 3and verify the pid file is gone and a fresh server spawns clean)🤖 Generated with Claude Code