fix(cli): refuse mm uninstall --force on Windows when writer is alive#756
Merged
fix(cli): refuse mm uninstall --force on Windows when writer is alive#756
mm uninstall --force on Windows when writer is alive#756Conversation
…ve (closes #730) POSIX `--force` semantics rely on unlink-while-open — `shutil.rmtree` succeeds even with a live SQLite writer because the directory entry is gone while the inode lives until the fd closes. Windows refuses to unlink files held by an open handle (WinError 32), so the previous behavior crashed mid-`_delete_inventory` and left the state directory half-wiped. Pin the contract to option (a) from the issue: refuse cleanly with a Windows-specific message instead of attempting a partial wipe. The trailing `pass --force to override` advice in the existing not-force refusal block is also dropped on Windows (it would just refuse again), and `lsof` / `ps aux` hints are replaced with `handle.exe` / Task Manager / Get-Process so the guidance is actionable on each platform. Replace the Windows-skipped `test_force_overrides_db_lock` with a POSIX/Windows pair. The Windows variant seeds a sentinel non-DB file (config.json + memories/) and asserts they survive — proving the refusal fired before `_delete_inventory` ran, not after a partial wipe. Q2 from the issue (transactional wipe via staging dir + atomic rename) is orthogonal and tracked as a follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <[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.
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.
Closes #730.
Summary
--forcesemantics rely on unlink-while-open —shutil.rmtreesucceeds with a live SQLite writer because the directory entry vanishes while the inode lives until the fd closes.WinError 32), so the previous behavior crashed mid-_delete_inventoryand left the state directory half-wiped.2, mirroring the existing refusal path.pass --force to overrideadvice in the existing not-force refusal block is also dropped on Windows (it would just refuse again), andlsof/ps auxhints are replaced withhandle.exe/ Task Manager /Get-Processso the guidance is actionable on each platform.test_force_overrides_db_lockis replaced with a POSIX/Windows pair. The Windows variant seeds a sentinel non-DB file (config.json+memories/) and asserts they survive — proving the refusal fired before_delete_inventoryran, not after a partial wipe.Out of scope (orthogonal follow-up)
Q2 from the issue — transactional wipe (stage to a sibling dir + atomic rename on success) is intentionally NOT in this PR. It helps even on POSIX (mid-rmtree perm/FS errors leave half-gone state today), but bundling it would muddle the contract decision. Will file a separate issue/PR.
Test plan
uv run ruff check+ruff format --check(clean)uv run mypy packages/memtomem/src/memtomem/cli/uninstall_cmd.py(clean)uv run pytest packages/memtomem/tests/test_uninstall_cmd.py -m "not ollama"— 28 passed, 1 skipped (the Windows-only test, as expected on macOS)test_force_refuses_on_windows_when_writer_aliveruns and passes;test_force_overrides_db_lock_posixis skipped;test_refuses_when_writer_holds_lockpasses with the platform-conditionallsof/handle.exeassertion.🤖 Generated with Claude Code