Skip to content

fix(cli): make mm uninstall deletion transactional via stage-then-rmtree#758

Merged
memtomem merged 2 commits intomainfrom
fix/757-uninstall-staging
May 3, 2026
Merged

fix(cli): make mm uninstall deletion transactional via stage-then-rmtree#758
memtomem merged 2 commits intomainfrom
fix/757-uninstall-staging

Conversation

@memtomem
Copy link
Copy Markdown
Owner

@memtomem memtomem commented May 3, 2026

Closes #757.

Summary

  • Old _delete_inventory deleted inventory groups in place; a mid-run failure (perm denied, transient FS error, antivirus) left the user with a half-gone state dir (config + memories wiped, DB + runtime files surviving) and no practical recovery path.
  • This PR makes the wipe transactional via a stage-then-rmtree pattern: every to-be-deleted path is moved via os.replace into a sibling <anchor>/.uninstall-staging-<pid>/ dir (atomic on same-FS), and only after every group is staged do we shutil.rmtree the staging dirs to actually delete the data.
  • Mid-stage failures roll back: walk staged moves in reverse and os.replace each back to its original location. If rollback itself fails, the staging dir paths are surfaced so the user can recover manually (the original concern from the issue's "rollback granularity" question).
  • Whole-dir staging for owned subdirs (config.d, memories, uploads) — one rename per subdir instead of N, and removes the post-deletion empty-prune dance the old code needed for those paths.
  • Cross-FS layouts (e.g. a bind mount inside ~/.memtomem/) can't satisfy os.replace's same-FS atomicity guarantee. Detected upfront via st_dev probe and refused with a clear message; late-detection on EXDEV from os.replace covers what the upfront probe misses (stat race, transient errors). Detect-and-refuse rather than copy+delete fallback — the whole point of staging is atomicity.
  • --keep-config / --keep-data still exempt their respective groups from the plan, so kept paths are never moved into staging at all.

Acceptance criteria

  • If any single group's deletion fails, the user's state directory is left in its original (pre-uninstall) state — no half-states (test_rollback_restores_original_state_on_mid_stage_failure).
  • Existing --keep-config / --keep-data / partial-error coverage still passes (28 + 1 skipped before, 31 + 1 skipped after).
  • New test induces a mid-staging failure and asserts the original state dir is intact.
  • New test covers cross-FS detection via monkeypatched EXDEV (real cross-FS layouts are too environment-dependent for CI; same code path either way).
  • New test for the rollback-itself-fails recovery surface so the user gets the staging dir path.

Out of scope

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" — 31 passed, 1 skipped (Windows-only, as expected on macOS)
  • uv run pytest packages/memtomem/tests/test_cli_*.py -m "not ollama" — 63 passed (no sibling regressions)
  • Windows CI lane: existing test_force_refuses_on_windows_when_writer_alive still fires before staging (refusal precedes _delete_inventory); new tests are platform-agnostic (monkeypatch-based).

🤖 Generated with Claude Code

memtomem added 2 commits May 3, 2026 22:21
…mtree (closes #757)

The old `_delete_inventory` deleted inventory groups in place. If any
group's deletion failed mid-run (permission denied, transient FS error,
antivirus interference on Windows, etc.), the user was left with a
half-gone state directory — config and memories already wiped while
the DB and runtime files survived. Re-running uninstall against that
half-cleaned dir was inconsistent and manual recovery impractical.

PR #756 closed the Windows-`--force` slice of this hazard via clean
refusal; this PR closes the orthogonal partial-failure case for all
platforms by making the wipe transactional:

1. Stage every to-be-deleted path into a sibling
   `<anchor>/.uninstall-staging-<pid>/` dir via `os.replace` —
   atomic on the same FS, so each move either completes or doesn't.
2. After every group is staged successfully, `shutil.rmtree` the
   staging dirs to actually delete the data.
3. On any mid-stage failure, roll back: walk staged moves in reverse
   and `os.replace` each back to its original location. If rollback
   itself fails, surface the staging dir paths so the user can recover
   manually.

Whole-dir staging for owned subdirs (`config.d`, `memories`,
`uploads`) cuts N renames down to 1 per subdir and removes the
post-deletion empty-prune dance the old code needed for those paths.

Cross-FS layouts (e.g. a bind mount inside `~/.memtomem/`) can't
satisfy `os.replace`'s same-FS atomicity guarantee. Detect via
upfront `st_dev` probe and refuse with a clear message rather than
falling back to copy+delete — the whole point of staging is
atomicity, and copy+delete would just reintroduce the half-state risk.
The late-detection branch on `EXDEV` from `os.replace` itself
covers anything the upfront probe misses (stat race, transient FS
errors).

`--keep-config` / `--keep-data` still exempt their respective groups
from the plan, so kept paths are never moved into staging at all.
Windows CI lane caught the snapshot dict using `str(Path(...))` for
its keys — that's `'memories/x.md'` on POSIX but `'memories\\x.md'`
on Windows, which broke the hardcoded `assert "memories/x.md" in
before` sanity check on the windows-latest runner.

Switch to `as_posix()` so keys are platform-stable.
@memtomem memtomem merged commit c5ee64a into main May 3, 2026
10 checks passed
@memtomem memtomem deleted the fix/757-uninstall-staging branch May 3, 2026 13:47
@github-actions github-actions Bot locked and limited conversation to collaborators May 3, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

uninstall: transactional wipe via staging dir + atomic rename (Q2 follow-up from #730)

1 participant