Merge pull request #2212 from mvalentsev/fix/2207-repair-backup-non-regular

fix(backups): stop a socket in the palace from aborting repair (#2207)
This commit is contained in:
Igor Lins e Silva 2026-08-11 07:06:39 -03:00 committed by GitHub
commit db9c917078
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 1071 additions and 17 deletions

View File

@ -31,6 +31,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
- **Repair and recovery are safer under contention.** `repair --mode from-sqlite` takes the mine-lock before archiving; rebuilds preserve a verified temp collection when the live swap fails; sparse drawers with zero `embedding_metadata` rows are no longer dropped; truncated ID pagination fails loud instead of pretending success. (#2109, #2086, #2087)
- **`repair --mode from-sqlite --dry-run` is a true preview.** It no longer archives or re-embeds; it prints per-collection would-be counts from SQLite ground truth and exits without touching the palace. Unreadable counts fail closed instead of inventing zeros. (#2133, #2095, #1654)
- **`repair --dry-run` is a true preview in the default (legacy) mode too.** That path ignored the flag entirely and ran the real rebuild — deleting any existing `<palace>.backup`, copying the palace over it, and re-filing the drawers collection. It now prints a read-only plan and exits without opening a chromadb client, which is itself a write to `chroma.sqlite3`. The plan names the live-collection delete the rebuild performs, warns when an existing backup would be destroyed, and reports the truncation guard as disabled when `--confirm-truncation-ok` is set. An isolated FTS5 inverted-index error is reported as auto-healable instead of raising the manual-recovery abort a real run never reaches, unreadable counts fail closed with a non-zero exit, and the `--dry-run` help no longer claims to be `--mode max-seq-id` only. (#2144)
- **`repair` and `migrate` survive a socket or a named pipe in the palace directory.** Both take a whole-directory `shutil.copytree` backup before they overwrite the palace, and `copytree` cannot duplicate a Unix domain socket left beside `chroma.sqlite3`, nor a named pipe: it copied everything else and then raised `shutil.Error`, so the command died at the backup step before any rebuild ran, and re-running only repeated it. Neither entry carries palace data, so both are now skipped and named in the output while the backup completes. Device nodes are skipped too, because the copy dereferences them instead of failing on them. Every other copy failure still aborts the command, including a symlink whose target cannot be read: no errno separates a deleted target from one on a volume that is not mounted, so an entry that may have held data is left for the copy to fail on. (#2207)
- **HNSW divergence is preflighted before remaining `col.count()` crash sites** across mine, dedup, migrate, repair, and palace helpers. (#2093)
- **Re-mine and conversation ingest no longer lose or duplicate drawers.** Content-hash dedup prevents duplicate LLM conversation drawers; sweeper drawers are excluded from convo extract-mode purge scope and failed purges abort; search returns round-trippable `drawer_id` values for `get_drawer`. (#2050, #2125, #2089, #2090, #2044, #2080)
- **MCP and daemon lifecycle harden multi-agent use.** Read-only mode refuses config and checkpoint-ack tools that rewrite host state; stdio MCP exits on stdin EOF/broken pipe so orphaned sessions release locks; daemon jobs refused the palace lock are deferred instead of failed permanently. (#2126, #2103, #2101, #2072, #2029, #2014)

View File

@ -1,4 +1,4 @@
"""Retention pruning for timestamped palace backups.
"""Writing and pruning palace backups.
``mempalace migrate`` and ``mempalace repair max-seq-id`` each write a fresh,
timestamped backup every time they run and historically never deleted the old
@ -9,11 +9,222 @@ filling the disk. This module prunes the backup set down to a bounded count
after each new backup is written.
The retention count comes from ``MempalaceConfig.max_backups`` (default 10).
``copy_palace_dir`` is the other half: the whole-directory copy that
``mempalace repair`` in its default mode and ``mempalace migrate`` take before
they overwrite a live palace. The other backup paths copy a single file and do
not use it.
"""
import glob
import os
import shutil
import stat
# Names for the file types a backup copy deliberately leaves behind. A socket
# or a named pipe makes ``shutil.copytree`` record an error for that entry and
# raise at the end. Device nodes do not, but dereferencing one copies the
# DEVICE instead of palace data, and a link to ``/dev/zero`` is copied without
# bound, so they are left out for the opposite reason: the copy would succeed
# at the wrong thing.
_UNCOPYABLE_FILE_TYPES = (
(stat.S_ISSOCK, "socket"),
(stat.S_ISFIFO, "named pipe"),
(stat.S_ISCHR, "character device"),
(stat.S_ISBLK, "block device"),
)
_UNNAMED_FILE_TYPE = "not a regular file or directory"
def _file_type_label(mode):
"""Name the file type in ``mode``, falling back to a generic phrase.
The fallback covers a file type this table has no name for. Nothing on
Linux reaches it, because ``stat`` there reports ``S_IFDOOR``,
``S_IFPORT`` and ``S_IFWHT`` as 0. macOS does define ``S_IFWHT``, a
union-mount whiteout, which is precisely what the fallback exists for:
an entry no copy can carry is skipped with an honest reason rather than
read as copyable.
"""
for is_type, label in _UNCOPYABLE_FILE_TYPES:
if is_type(mode):
return label
return _UNNAMED_FILE_TYPE
def _uncopyable_reason(path, *, follow_symlinks):
"""Name why ``path`` cannot be copied into a backup, or return ``None``.
This is an allowlist, matching how the rest of the package treats
directory entries: only regular files and directories are copyable, and
everything else is named and left out.
Args:
path: The directory entry to classify.
follow_symlinks: Mirrors the copy's own link handling. ``True`` when
the copy dereferences links, so the TARGET's file type decides;
``False`` when it recreates them as links, in which case any
symlink is fine and its target is never read.
Returns:
A short human-readable reason, or ``None`` both when the entry is
copyable and when its type could not be established.
Those two ``None`` cases are deliberately the same answer: only a file
type this could read names a reason. An entry whose type it could not
read is left for the copy to attempt and, if it really is broken, to
fail on.
A failed ``stat`` therefore never becomes a reason to skip. It says the
entry cannot be resolved right now, which is not the same as holding no
data, and no errno separates the two: a symlink into a volume that is
not mounted fails exactly like one whose target was deleted, and on
Windows an unmapped drive letter and an unreachable network share both
arrive as ``ENOENT`` as well.
"""
try:
st = os.lstat(path)
except OSError:
return None
if stat.S_ISLNK(st.st_mode):
if not follow_symlinks:
return None
try:
st = os.stat(path)
except OSError:
return None
if stat.S_ISREG(st.st_mode) or stat.S_ISDIR(st.st_mode):
return None
return _file_type_label(st.st_mode)
def _report_skipped(skipped, src, log):
"""Tell the operator what the copy left out.
Every step that can fail is guarded on its own, because this also runs
after the copy has failed: both callers pass ``print``, so one line the
terminal will not take must cost neither the copy's own outcome nor the
lines after it.
"""
noun = "entry" if len(skipped) == 1 else "entries"
lines = [f" Backup: skipped {len(skipped)} {noun} that cannot be copied:"]
for path, reason in skipped:
try:
name = os.path.relpath(path, src)
except (OSError, ValueError):
# On Windows ``relpath`` rejects paths on different drives; for a
# relative ``src`` it also reaches ``os.getcwd()``, which can fail
# on a deleted directory. Neither costs us the entry's name.
name = path
lines.append(f" {name} ({reason})")
for line in lines:
try:
log(line)
except UnicodeError:
# An stdout that cannot encode this entry's name. Dropping the
# line would leave a header whose count disagrees with the list
# under it, so the name is escaped and retried before it is
# given up on.
try:
log(line.encode("ascii", "backslashreplace").decode("ascii"))
except (OSError, UnicodeError):
continue
except OSError:
# A write that the device refused. A short report is buffered
# whole, so this arrives only once the report is long enough to
# flush partway through, and then the header is already out.
# Anything else is a caller passing something that is not a
# working ``log``, which should surface rather than be swallowed.
continue
def copy_palace_dir(src, dst, *, symlinks=False, log=None):
"""Copy a palace directory to ``dst``, skipping entries no copy can carry.
``shutil.copytree`` finishes the copy but raises ``shutil.Error`` at the
end when a directory entry is not something it knows how to duplicate: a
Unix domain socket, or a named pipe. The caller has to treat that as a
failed backup, so the command died before the rebuild the backup was
guarding (#2207). Such entries are runtime artifacts of whatever process
created them and never hold palace data, so a backup without them is
still a complete backup of the palace.
Args:
src: Palace directory to copy.
dst: Destination path. Must not already exist.
symlinks: Passed to ``shutil.copytree``. ``True`` recreates symlinks
as symlinks, ``False`` copies what they point at.
log: Optional callable (e.g. ``print``) for human-readable progress.
Returns:
The list of ``(path, reason)`` pairs that were skipped: sorted within
each directory, in the order the copy visited the directories. Like
``prune_backups``, this both returns what it did and logs it.
Every other copy failure still raises out of ``shutil.copytree``. A caller
about to overwrite the live palace must still stop when its safety copy
did not come out whole, so this narrows what the copy attempts rather
than swallowing what it reports.
``shutil.copytree`` hands the callback names rather than the ``os.scandir``
entries it already holds, so classifying costs one extra ``os.lstat`` per
directory entry, and it classifies a whole directory before copying any of
it. An entry whose type changes inside that window is handled by the
earlier reading, which cuts both ways: one that became a socket still
aborts the copy, and one that was a socket and became a regular file is
skipped with its contents. Nothing in MemPalace replaces an entry that
way, and the skipped name is printed either way.
"""
skipped = []
# ``shutil.copytree`` classifies the top directory before it creates the
# destination, so a copy that never starts still produces a skip list.
# Reporting one would tell the operator what a nonexistent backup is
# missing, so both ways that happens are excluded below: a destination
# already occupied (this snapshot), and a destination that could not be
# created at all.
dst_existed = os.path.lexists(dst)
def _ignore(directory, names):
ignored = set()
found = []
for name in names:
path = os.path.join(directory, name)
reason = _uncopyable_reason(path, follow_symlinks=not symlinks)
if reason is not None:
ignored.add(name)
found.append((path, reason))
skipped.extend(sorted(found))
return ignored
def _report():
if skipped and log and not dst_existed and os.path.isdir(dst):
_report_skipped(skipped, src, log)
try:
shutil.copytree(src, dst, symlinks=symlinks, ignore=_ignore)
except BaseException:
# Reported for a failed copy too, including an interrupted one: a
# half-written backup is exactly when the operator needs to know what
# was left out of it. But the copy's own failure is what the caller
# has to diagnose, so a report that fails on top of it is dropped
# rather than raised in its place.
try:
_report()
except Exception:
pass
raise
# Not suppressed here: with the backup complete and nothing destructive
# done yet, a ``log`` that cannot be called at all is a caller's bug worth
# hearing about. The terminal failures inside ``_report_skipped`` are
# absorbed on both paths.
_report()
return skipped
def prune_backups(pattern, max_backups, *, log=None):

View File

@ -1563,6 +1563,7 @@ def cmd_repair(args):
import shutil
from .backends.chroma import ChromaBackend
from .backups import copy_palace_dir
from .migrate import confirm_destructive_action, contains_palace_database
from .repair import (
RebuildCollectionError,
@ -1780,7 +1781,7 @@ def cmd_repair(args):
return
shutil.rmtree(backup_path)
print(f" Backing up to {backup_path}...")
shutil.copytree(palace_path, backup_path)
copy_palace_dir(palace_path, backup_path, log=print)
try:
filed = _rebuild_collection_via_temp(

View File

@ -29,7 +29,7 @@ from collections import defaultdict
from contextlib import closing
from datetime import datetime
from .backups import prune_backups
from .backups import copy_palace_dir, prune_backups
from .config import MempalaceConfig
@ -316,7 +316,7 @@ def migrate(palace_path: str, dry_run: bool = False, confirm: bool = False):
timestamp = datetime.now().strftime("%Y%m%d_%H%M%S")
backup_path = f"{palace_path}.pre-migrate.{timestamp}"
print(f"\n Backing up to {backup_path}...")
shutil.copytree(palace_path, backup_path, symlinks=True)
copy_palace_dir(palace_path, backup_path, symlinks=True, log=print)
# Enforce backup retention so repeated migrations cannot fill the disk
# with full-palace copies. The backup we just created is the newest, so

View File

@ -1,16 +1,100 @@
"""Tests for backup retention pruning (mempalace.backups.prune_backups).
"""Tests for writing and pruning palace backups (mempalace.backups).
These guard the fix for unbounded backup growth: ``mempalace migrate`` and
``mempalace repair max-seq-id`` each drop a fresh full-size, timestamped copy
every run, and used to never delete the old ones a palace was found with
hundreds of GB of stale backups beside a few hundred MB of live data.
``prune_backups`` guards the fix for unbounded backup growth: ``mempalace
migrate`` and ``mempalace repair max-seq-id`` each drop a fresh full-size,
timestamped copy every run, and used to never delete the old ones. A palace
was found with hundreds of GB of stale backups beside a few hundred MB of
live data.
``copy_palace_dir`` guards #2207: the whole-directory copy that ``repair``
and ``migrate`` take before overwriting a live palace used to abort the whole
command when the palace held a directory entry ``shutil.copytree`` cannot
duplicate.
"""
import errno
import os
import shutil
import socket
import stat
import pytest
from mempalace.backups import prune_backups
from mempalace.backups import (
_file_type_label,
_uncopyable_reason,
copy_palace_dir,
prune_backups,
)
needs_unix_socket = pytest.mark.skipif(
os.name == "nt" or not hasattr(socket, "AF_UNIX"),
reason="Unix domain socket files are POSIX-only",
)
needs_fifo = pytest.mark.skipif(not hasattr(os, "mkfifo"), reason="named pipes are POSIX-only")
needs_unprivileged_posix = pytest.mark.skipif(
os.name == "nt" or (hasattr(os, "geteuid") and os.geteuid() == 0),
reason="directory permission bits gate neither root nor Windows",
)
needs_char_device = pytest.mark.skipif(
os.name == "nt" or not os.path.exists("/dev/null"),
reason="device nodes are POSIX-only",
)
def _symlink_or_skip(link, target):
"""Create ``link`` pointing at ``target``, or skip if the platform refuses.
Windows without ``SeCreateSymbolicLinkPrivilege`` raises ``OSError``
before any product code runs. Per PR #1555 review (Igor), symlink tests
skip cleanly there rather than fail spuriously. Trying the syscall keeps
the symlink half of this module under test wherever it does work, which a
blanket ``os.name == "nt"`` skip would give up on.
``NotImplementedError`` covers the restricted sandboxes ``test_exporter``
already documents. Otherwise only a permission refusal is turned into a
skip: ``EEXIST`` from a test that left the name behind, ``ENOENT`` from a
missing parent and ``ENOSPC`` are bugs in the test, and swallowing them
would delete this module's symlink coverage from a run that still
reported success.
"""
try:
link.symlink_to(target)
except NotImplementedError as exc:
pytest.skip(f"symlinks are unavailable here: {exc}")
except OSError as exc:
if os.name != "nt" and exc.errno not in (errno.EPERM, errno.EACCES):
raise
pytest.skip(f"symlink creation not permitted for this user: {exc}")
def _make_palace(parent, name="palace"):
"""A palace directory holding the one file every caller cares about."""
path = parent / name
path.mkdir()
(path / "chroma.sqlite3").write_bytes(b"SQLite format 3\x00")
segment = path / "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"
segment.mkdir()
(segment / "data_level0.bin").write_bytes(b"\x00" * 32)
return path
def _bind_socket(directory, name, monkeypatch):
"""Bind a Unix socket inside ``directory`` and return its path.
Bound by relative name from inside the directory: an absolute pytest tmp
path can exceed the ~100-byte ``sun_path`` limit, which is tight on
macOS. ``monkeypatch.chdir`` restores the working directory afterwards.
"""
monkeypatch.chdir(directory)
sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
try:
sock.bind(name)
finally:
# The socket FILE outlives the process that bound it, which is
# exactly the state #2207 was reported in.
sock.close()
return os.path.join(str(directory), name)
def _make_backup_dir(parent, name, mtime):
@ -155,3 +239,621 @@ def test_prune_is_best_effort_on_delete_failure(tmp_path, monkeypatch):
assert str(tmp_path / "b.1") not in removed
assert (tmp_path / "b.1").exists()
assert any("could not remove" in line for line in logs)
# ---------------------------------------------------------------------------
# copy_palace_dir (#2207)
# ---------------------------------------------------------------------------
def test_copy_palace_dir_copies_an_ordinary_palace(tmp_path):
palace = _make_palace(tmp_path)
dest = tmp_path / "palace.backup"
skipped = copy_palace_dir(str(palace), str(dest))
assert skipped == []
assert (dest / "chroma.sqlite3").read_bytes() == b"SQLite format 3\x00"
assert (dest / "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" / "data_level0.bin").is_file()
def test_copy_palace_dir_logs_nothing_when_every_entry_copies(tmp_path):
palace = _make_palace(tmp_path)
logs = []
copy_palace_dir(str(palace), str(tmp_path / "palace.backup"), log=logs.append)
assert logs == []
@needs_unix_socket
def test_copy_palace_dir_skips_a_socket_and_still_copies_the_database(tmp_path, monkeypatch):
"""The reported #2207 case: a leftover daemon socket beside chroma.sqlite3."""
palace = _make_palace(tmp_path)
sock_path = _bind_socket(palace, "mcp.sock", monkeypatch)
dest = tmp_path / "palace.backup"
logs = []
skipped = copy_palace_dir(str(palace), str(dest), log=logs.append)
assert skipped == [(sock_path, "socket")]
assert (dest / "chroma.sqlite3").read_bytes() == b"SQLite format 3\x00"
assert not os.path.lexists(dest / "mcp.sock")
# Compared line for line, and with one entry, so the singular header and
# the socket detail line are both pinned. A substring assertion accepts a
# plural that disagrees with the count and a detail line with anything
# appended to it.
assert logs == [
" Backup: skipped 1 entry that cannot be copied:",
" mcp.sock (socket)",
]
@needs_fifo
def test_copy_palace_dir_skips_a_named_pipe(tmp_path):
palace = _make_palace(tmp_path)
fifo = palace / "mempalace.fifo"
os.mkfifo(fifo)
dest = tmp_path / "palace.backup"
skipped = copy_palace_dir(str(palace), str(dest))
assert skipped == [(str(fifo), "named pipe")]
assert (dest / "chroma.sqlite3").is_file()
def test_copy_palace_dir_still_raises_on_a_symlink_whose_target_is_missing(tmp_path):
"""A link that does not resolve is NOT proof that it carried no data.
A palace segment can be a link onto another volume, and a volume that is
not mounted right now fails ``os.stat`` with the same ``ENOENT`` as a
target that was deleted. On Windows an unmapped drive letter and an
unreachable network share arrive as ``ENOENT`` too. Skipping on that
would drop a whole subtree from the safety copy and let the rebuild run
over the live palace anyway, so the copy has to fail the way it does on
``develop``.
"""
palace = _make_palace(tmp_path)
link = palace / "chroma.sqlite3.prev"
_symlink_or_skip(link, palace / "gone-away.sqlite3")
dest = tmp_path / "palace.backup"
assert _uncopyable_reason(str(link), follow_symlinks=True) is None
with pytest.raises(shutil.Error):
copy_palace_dir(str(palace), str(dest))
assert not os.path.lexists(dest / "chroma.sqlite3.prev")
@needs_unix_socket
def test_copy_palace_dir_skips_a_symlink_pointing_at_a_socket(tmp_path, monkeypatch):
"""The copy dereferences links by default, so the TARGET's type decides."""
palace = _make_palace(tmp_path)
sock_path = _bind_socket(palace, "daemon.sock", monkeypatch)
link = palace / "mcp.sock"
_symlink_or_skip(link, palace / "daemon.sock")
dest = tmp_path / "palace.backup"
skipped = copy_palace_dir(str(palace), str(dest))
# Sorted within the directory: "daemon.sock" before "mcp.sock".
assert skipped == [(sock_path, "socket"), (str(link), "socket")]
assert (dest / "chroma.sqlite3").is_file()
def test_copy_palace_dir_follows_a_symlink_to_a_real_file(tmp_path):
"""Link handling is unchanged from the plain ``copytree`` this replaced.
A resolvable link is dereferenced and its content lands in the backup,
which is what the bare call did. Pinned so the refactor cannot quietly
alter it, not as an argument that dereferencing is the right default.
"""
palace = _make_palace(tmp_path)
(tmp_path / "outside.json").write_text("payload", encoding="utf-8")
_symlink_or_skip(palace / "tunnels.json", tmp_path / "outside.json")
dest = tmp_path / "palace.backup"
skipped = copy_palace_dir(str(palace), str(dest))
assert skipped == []
assert (dest / "tunnels.json").read_text(encoding="utf-8") == "payload"
def test_copy_palace_dir_keeps_a_broken_symlink_when_symlinks_are_preserved(tmp_path):
"""``migrate`` copies with ``symlinks=True``, so links are recreated as
links and their targets are never read."""
palace = _make_palace(tmp_path)
_symlink_or_skip(palace / "chroma.sqlite3.prev", palace / "gone-away.sqlite3")
dest = tmp_path / "palace.pre-migrate.1"
skipped = copy_palace_dir(str(palace), str(dest), symlinks=True)
assert skipped == []
assert (dest / "chroma.sqlite3.prev").is_symlink()
@needs_unix_socket
def test_copy_palace_dir_skips_a_socket_even_when_symlinks_are_preserved(tmp_path, monkeypatch):
palace = _make_palace(tmp_path)
sock_path = _bind_socket(palace, "mcp.sock", monkeypatch)
dest = tmp_path / "palace.pre-migrate.1"
skipped = copy_palace_dir(str(palace), str(dest), symlinks=True)
assert skipped == [(sock_path, "socket")]
assert (dest / "chroma.sqlite3").is_file()
@needs_unix_socket
def test_copy_palace_dir_keeps_a_link_to_a_socket_when_symlinks_are_preserved(
tmp_path, monkeypatch
):
"""The link's own type decides when the copy recreates links as links.
``migrate`` never reads through a link, so a link that happens to point
at a socket is a link like any other and belongs in the backup. Judging
it by its target instead would drop it.
"""
palace = _make_palace(tmp_path)
_bind_socket(palace, "daemon.sock", monkeypatch)
_symlink_or_skip(palace / "mcp.sock", palace / "daemon.sock")
dest = tmp_path / "palace.pre-migrate.1"
skipped = copy_palace_dir(str(palace), str(dest), symlinks=True)
assert [reason for _, reason in skipped] == ["socket"]
assert (dest / "mcp.sock").is_symlink()
@needs_unprivileged_posix
def test_copy_palace_dir_still_raises_on_a_copy_failure_it_cannot_classify(tmp_path):
"""Only provably uncopyable entries are skipped.
Anything else must still abort the caller: the backup is the safety net
for a rebuild that is about to overwrite the live palace, so a copy that
did not come out whole has to be loud.
"""
palace = _make_palace(tmp_path)
locked = palace / "locked-segment"
locked.mkdir()
(locked / "data_level0.bin").write_bytes(b"\x00")
locked.chmod(0o000)
try:
with pytest.raises(shutil.Error):
copy_palace_dir(str(palace), str(tmp_path / "palace.backup"))
finally:
locked.chmod(0o700)
@needs_unix_socket
def test_uncopyable_reason_names_a_socket(tmp_path, monkeypatch):
sock_path = _bind_socket(tmp_path, "mcp.sock", monkeypatch)
assert _uncopyable_reason(sock_path, follow_symlinks=True) == "socket"
assert _uncopyable_reason(sock_path, follow_symlinks=False) == "socket"
def test_uncopyable_reason_passes_regular_files_and_directories(tmp_path):
regular = tmp_path / "chroma.sqlite3"
regular.write_bytes(b"SQLite format 3\x00")
directory = tmp_path / "segment"
directory.mkdir()
for path in (regular, directory):
assert _uncopyable_reason(str(path), follow_symlinks=True) is None
assert _uncopyable_reason(str(path), follow_symlinks=False) is None
@needs_unix_socket
def test_uncopyable_reason_judges_a_symlink_by_how_the_copy_treats_it(tmp_path, monkeypatch):
_bind_socket(tmp_path, "mcp.sock", monkeypatch)
link = tmp_path / "link-to-sock"
_symlink_or_skip(link, tmp_path / "mcp.sock")
# Dereferenced by the copy: the target's type decides.
assert _uncopyable_reason(str(link), follow_symlinks=True) == "socket"
# Recreated as a link by the copy: the target is never read.
assert _uncopyable_reason(str(link), follow_symlinks=False) is None
def test_uncopyable_reason_never_skips_on_a_failed_stat(tmp_path):
"""An entry whose type could not be read is left for the copy to attempt.
Naming a reason here would skip it silently. A failed ``stat`` says the
entry cannot be resolved right now, not that it holds no data, and no
errno separates a deleted target from one on a volume that is not
mounted, so ``None`` is the only answer that cannot lose data.
"""
missing = tmp_path / "not-there"
assert _uncopyable_reason(str(missing), follow_symlinks=True) is None
link = tmp_path / "onto-a-missing-volume"
_symlink_or_skip(link, missing)
assert _uncopyable_reason(str(link), follow_symlinks=True) is None
def test_file_type_label_names_what_it_knows_and_refuses_the_rest():
"""The allowlist needs an answer for a type its table has no name for.
Nothing on Linux reaches the fallback: ``stat.S_IFDOOR`` and its siblings
read as ``0`` there. macOS does define ``S_IFWHT``, so a mode carrying it
would land here, though producing one needs a union mount that no CI
runner has. The label is therefore exercised directly, rather than left
unpinned behind a state no platform here can create.
"""
assert _file_type_label(stat.S_IFSOCK) == "socket"
assert _file_type_label(stat.S_IFIFO) == "named pipe"
assert _file_type_label(stat.S_IFCHR) == "character device"
assert _file_type_label(stat.S_IFBLK) == "block device"
assert _file_type_label(stat.S_IFLNK) == "not a regular file or directory"
@needs_unprivileged_posix
def test_copy_palace_dir_still_raises_when_a_symlink_target_cannot_be_inspected(tmp_path):
"""The target may be real palace data behind a directory we cannot enter.
Skipping it would drop live data from the safety copy and let the rebuild
proceed over the live palace, so the copy has to fail the way it does on
``develop``.
"""
palace = _make_palace(tmp_path)
vault = tmp_path / "vault"
vault.mkdir()
(vault / "shard.sqlite3").write_text("REAL PALACE DATA", encoding="utf-8")
_symlink_or_skip(palace / "shard.sqlite3", vault / "shard.sqlite3")
vault.chmod(0o000)
dest = tmp_path / "palace.backup"
try:
with pytest.raises(shutil.Error):
copy_palace_dir(str(palace), str(dest))
finally:
vault.chmod(0o700)
assert not (dest / "shard.sqlite3").exists()
def test_copy_palace_dir_still_raises_on_a_symlink_cycle(tmp_path):
"""A cycle resolves to nothing, and that is still not a reason to skip."""
palace = _make_palace(tmp_path)
_symlink_or_skip(palace / "loop_a", palace / "loop_b")
_symlink_or_skip(palace / "loop_b", palace / "loop_a")
with pytest.raises(shutil.Error):
copy_palace_dir(str(palace), str(tmp_path / "palace.backup"))
def test_copy_palace_dir_still_raises_on_a_symlink_to_its_own_parent(tmp_path):
"""The directory-cycle form, which bloats the copy rather than failing fast."""
palace = _make_palace(tmp_path)
_symlink_or_skip(palace / "self", palace)
with pytest.raises(shutil.Error):
copy_palace_dir(str(palace), str(tmp_path / "palace.backup"))
@needs_char_device
def test_copy_palace_dir_skips_a_symlink_to_a_character_device(tmp_path):
"""Device nodes do not make the copy raise; they make it copy the wrong thing.
Dereferencing a link to ``/dev/null`` writes an empty regular file, and a
link to ``/dev/zero`` is copied without bound, so both are left out. The
entry is a link rather than a real node because creating one needs root.
"""
palace = _make_palace(tmp_path)
link = palace / "devnull"
_symlink_or_skip(link, "/dev/null")
dest = tmp_path / "palace.backup"
skipped = copy_palace_dir(str(palace), str(dest))
assert skipped == [(str(link), "character device")]
assert not os.path.lexists(dest / "devnull")
assert (dest / "chroma.sqlite3").is_file()
@needs_fifo
@needs_unprivileged_posix
def test_copy_palace_dir_reports_skips_even_when_the_copy_fails(tmp_path):
"""The skip list is what the operator needs most when the copy died.
The copy is made to fail partway through rather than before it starts, so
this really is the half-written-backup case and does not lean on the
order in which ``shutil`` happens to call the ignore callback.
"""
palace = _make_palace(tmp_path)
os.mkfifo(palace / "aaa.fifo")
locked = palace / "locked-segment"
locked.mkdir()
(locked / "data_level0.bin").write_bytes(b"\x00")
locked.chmod(0o000)
dest = tmp_path / "palace.backup"
logs = []
try:
with pytest.raises(shutil.Error):
copy_palace_dir(str(palace), str(dest), log=logs.append)
finally:
locked.chmod(0o700)
assert any("aaa.fifo (named pipe)" in line for line in logs)
assert (dest / "chroma.sqlite3").is_file()
@needs_fifo
@needs_unprivileged_posix
def test_copy_palace_dir_report_never_replaces_the_copys_own_failure(tmp_path):
"""A failing ``log`` must not become the error the caller diagnoses.
Both callers pass ``print``, so the report can fail on its own: a closed
pipe (``mempalace repair | head``) or an stdout that cannot encode a
skipped entry's name. Letting that out of the failure path would hand the
caller that error instead of the reason the backup did not come out whole.
"""
palace = _make_palace(tmp_path)
os.mkfifo(palace / "leftover.fifo")
locked = palace / "locked-segment"
locked.mkdir()
(locked / "data_level0.bin").write_bytes(b"\x00")
locked.chmod(0o000)
dest = tmp_path / "palace.backup"
def log_that_dies(_line):
# Not one of the terminal failures the per-line guard absorbs, so
# this is the report escaping as far as it ever can.
raise RuntimeError("logging is broken")
try:
with pytest.raises(shutil.Error):
copy_palace_dir(str(palace), str(dest), log=log_that_dies)
finally:
locked.chmod(0o700)
@needs_fifo
@needs_unprivileged_posix
def test_copy_palace_dir_says_nothing_when_the_destination_cannot_be_created(tmp_path):
"""The other way the copy never starts: the destination's parent is closed.
``os.path.lexists`` reads False there as well, so the before-and-after
snapshot alone would report a skip list for a backup that does not exist.
"""
palace = _make_palace(tmp_path)
os.mkfifo(palace / "leftover.fifo")
closed = tmp_path / "closed"
closed.mkdir()
closed.chmod(0o500) # readable, not writable
logs = []
try:
with pytest.raises(PermissionError):
copy_palace_dir(str(palace), str(closed / "palace.backup"), log=logs.append)
finally:
closed.chmod(0o700)
assert logs == []
@needs_fifo
def test_copy_palace_dir_surfaces_an_unusable_log_when_the_copy_succeeded(tmp_path):
"""Best-effort covers the terminal, not a caller who passed no logger.
Suppressing this too would hide an integration bug behind a backup that
reports success. The backup is already complete when it surfaces, and
nothing destructive has run yet.
"""
palace = _make_palace(tmp_path)
os.mkfifo(palace / "leftover.fifo")
with pytest.raises(TypeError):
copy_palace_dir(str(palace), str(tmp_path / "palace.backup"), log="not-a-callable")
@needs_fifo
@pytest.mark.parametrize("shape", ["directory", "regular file", "dangling symlink"])
def test_copy_palace_dir_says_nothing_about_a_backup_it_never_started(tmp_path, shape):
"""``copytree`` classifies the top directory before it creates the
destination, so a destination it refuses still yields a skip list. It
describes a backup that does not exist, whatever is sitting in the way."""
palace = _make_palace(tmp_path)
os.mkfifo(palace / "leftover.fifo")
dest = tmp_path / "palace.backup"
if shape == "directory":
dest.mkdir()
elif shape == "regular file":
dest.write_text("in the way", encoding="utf-8")
else:
_symlink_or_skip(dest, tmp_path / "gone-away")
logs = []
with pytest.raises(FileExistsError):
copy_palace_dir(str(palace), str(dest), log=logs.append)
assert logs == []
@needs_fifo
def test_copy_palace_dir_survives_a_failing_report_on_a_good_copy(tmp_path):
"""The report is best-effort, like ``prune_backups``' own cleanup."""
palace = _make_palace(tmp_path)
os.mkfifo(palace / "leftover.fifo")
dest = tmp_path / "palace.backup"
def log_that_dies(_line):
raise BrokenPipeError(32, "Broken pipe")
skipped = copy_palace_dir(str(palace), str(dest), log=log_that_dies)
assert [reason for _, reason in skipped] == ["named pipe"]
assert (dest / "chroma.sqlite3").is_file()
@needs_fifo
def test_copy_palace_dir_reports_skips_as_palace_relative_lines(tmp_path):
"""The whole report is pinned, not just fragments of it.
Substring assertions accept an absolute path, a wrong plural and a lost
indent alike, so the rendering is compared line for line instead.
"""
palace = _make_palace(tmp_path)
os.mkfifo(palace / "aaa.fifo")
os.mkfifo(palace / "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" / "bbb.fifo")
logs = []
copy_palace_dir(str(palace), str(tmp_path / "palace.backup"), log=logs.append)
assert logs == [
" Backup: skipped 2 entries that cannot be copied:",
" aaa.fifo (named pipe)",
f" aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee{os.sep}bbb.fifo (named pipe)",
]
@needs_fifo
def test_copy_palace_dir_report_does_not_swallow_an_interrupt(tmp_path):
"""Best-effort covers failures, not the operator pressing Ctrl-C."""
palace = _make_palace(tmp_path)
os.mkfifo(palace / "leftover.fifo")
def log_that_aborts(_line):
raise KeyboardInterrupt
with pytest.raises(KeyboardInterrupt):
copy_palace_dir(str(palace), str(tmp_path / "palace.backup"), log=log_that_aborts)
@needs_fifo
def test_copy_palace_dir_reports_skips_in_a_stable_order(tmp_path):
"""Within one directory the order is sorted, not whatever ``scandir`` gave.
Across directories it stays the order the copy visited them in, which is
filesystem-dependent; only the within-a-directory half is a promise.
"""
palace = _make_palace(tmp_path)
for name in ("zzz.fifo", "mmm.fifo", "aaa.fifo"):
os.mkfifo(palace / name)
skipped = copy_palace_dir(str(palace), str(tmp_path / "palace.backup"))
names = [os.path.basename(path) for path, _ in skipped]
assert names == ["aaa.fifo", "mmm.fifo", "zzz.fifo"]
@needs_fifo
def test_copy_palace_dir_keeps_directories_in_visit_order(tmp_path):
"""Directories stay in the order the copy walked them, not sorted globally.
The nested entry's full path sorts before the top-level one, so sorting
the whole list at the end would swap them and break the promise the
docstring makes.
"""
palace = _make_palace(tmp_path)
os.mkfifo(palace / "zzz.fifo")
os.mkfifo(palace / "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" / "bbb.fifo")
logs = []
copy_palace_dir(str(palace), str(tmp_path / "palace.backup"), log=logs.append)
assert logs == [
" Backup: skipped 2 entries that cannot be copied:",
" zzz.fifo (named pipe)",
f" aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee{os.sep}bbb.fifo (named pipe)",
]
@needs_fifo
def test_copy_palace_dir_reports_skips_when_the_copy_is_interrupted(tmp_path, monkeypatch):
"""Ctrl-C during a long copy still tells the operator what was left out.
That is why the copy is wrapped in ``except BaseException`` rather than
``except Exception``: the half-written backup is real, and what it lacks
is what the operator has to know before deciding what to do with it.
"""
palace = _make_palace(tmp_path)
os.mkfifo(palace / "leftover.fifo")
logs = []
def interrupt(*_args, **_kwargs):
raise KeyboardInterrupt
monkeypatch.setattr("mempalace.backups.shutil.copystat", interrupt)
with pytest.raises(KeyboardInterrupt):
copy_palace_dir(str(palace), str(tmp_path / "palace.backup"), log=logs.append)
assert logs == [
" Backup: skipped 1 entry that cannot be copied:",
" leftover.fifo (named pipe)",
]
@needs_fifo
def test_copy_palace_dir_names_an_entry_it_cannot_make_relative(tmp_path, monkeypatch):
"""``os.path.relpath`` calls ``os.getcwd()`` and rejects other drives.
Neither is a reason to drop the entry from the report, so the absolute
path stands in for the palace-relative one.
"""
palace = _make_palace(tmp_path)
fifo = palace / "leftover.fifo"
os.mkfifo(fifo)
logs = []
def no_relpath(*_args, **_kwargs):
raise ValueError("path is on mount 'C:', start on mount 'D:'")
monkeypatch.setattr("mempalace.backups.os.path.relpath", no_relpath)
copy_palace_dir(str(palace), str(tmp_path / "palace.backup"), log=logs.append)
assert logs == [
" Backup: skipped 1 entry that cannot be copied:",
f" {fifo} (named pipe)",
]
@needs_fifo
def test_copy_palace_dir_escapes_a_name_the_terminal_cannot_encode(tmp_path):
"""A name stdout cannot encode is escaped, not dropped.
The count in the header comes from the copy, so dropping the line would
leave the operator reading "skipped 3" above a list of two. Retried in
ASCII, the entry is still named.
"""
palace = _make_palace(tmp_path)
for name in ("aaa.fifo", "mü.fifo", "zzz.fifo"):
os.mkfifo(palace / name)
logs = []
def log_that_cannot_encode_one(line):
# Stands in for an stdout whose encoding cannot carry the name.
if not line.isascii():
raise UnicodeEncodeError("ascii", line, 0, 1, "cannot encode")
logs.append(line)
copy_palace_dir(str(palace), str(tmp_path / "palace.backup"), log=log_that_cannot_encode_one)
assert logs == [
" Backup: skipped 3 entries that cannot be copied:",
" aaa.fifo (named pipe)",
" m\\xfc.fifo (named pipe)",
" zzz.fifo (named pipe)",
]
@needs_fifo
def test_copy_palace_dir_report_survives_a_line_that_cannot_be_written(tmp_path):
"""A per-line write failure must not cost the operator the other lines."""
palace = _make_palace(tmp_path)
for name in ("aaa.fifo", "mmm.fifo", "zzz.fifo"):
os.mkfifo(palace / name)
logs = []
def log_that_dies_once(line):
if "mmm.fifo" in line:
raise BrokenPipeError(32, "Broken pipe")
logs.append(line)
copy_palace_dir(str(palace), str(tmp_path / "palace.backup"), log=log_that_dies_once)
assert logs == [
" Backup: skipped 3 entries that cannot be copied:",
" aaa.fifo (named pipe)",
" zzz.fifo (named pipe)",
]

View File

@ -1,8 +1,10 @@
"""Tests for mempalace.cli — the main CLI dispatcher."""
import argparse
import errno
import os
import shlex
import socket
import sqlite3
import subprocess
import sys
@ -1199,6 +1201,79 @@ def test_cmd_repair_success(mock_config_cls, tmp_path, capsys):
mock_new_col.add.assert_not_called()
@pytest.mark.skipif(
os.name == "nt" or not hasattr(socket, "AF_UNIX"),
reason="Unix domain socket files are POSIX-only",
)
@patch("mempalace.cli.MempalaceConfig")
def test_cmd_repair_survives_a_socket_in_the_palace_directory(
mock_config_cls, tmp_path, monkeypatch, capsys
):
"""#2207: a leftover daemon socket aborted repair at the backup step.
``shutil.copytree`` raises on a Unix domain socket, so the command died
with a traceback before ``_rebuild_collection_via_temp`` ever ran, and
re-running only repeated it.
"""
palace_dir = tmp_path / "palace"
palace_dir.mkdir()
sqlite3.connect(str(palace_dir / "chroma.sqlite3")).close()
# Bound by relative name: the absolute tmp path can exceed the sun_path
# limit, which is tight on macOS.
monkeypatch.chdir(palace_dir)
leftover = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
try:
leftover.bind("mcp.sock")
finally:
leftover.close()
# A link out of the palace, to pin that this backup still DEREFERENCES
# links. Preserving them instead would put a bare symlink in the backup,
# and the rebuild that follows can leave it pointing at nothing.
outside = tmp_path / "outside.json"
outside.write_text("tunnel payload", encoding="utf-8")
try:
(palace_dir / "tunnels.json").symlink_to(outside)
except OSError as exc:
# Only a permission refusal is a skip; anything else is a bug here.
if os.name != "nt" and exc.errno not in (errno.EPERM, errno.EACCES):
raise
pytest.skip(f"symlink creation not permitted for this user: {exc}")
mock_config_cls.return_value.palace_path = str(palace_dir)
mock_config_cls.return_value.collection_name = "mempalace_drawers"
args = argparse.Namespace(palace=None, yes=True)
mock_col = MagicMock()
mock_col.count.return_value = 2
mock_col.get.return_value = {
"ids": ["id1", "id2"],
"documents": ["doc1", "doc2"],
"metadatas": [{"wing": "a"}, {"wing": "b"}],
}
mock_temp_col = MagicMock()
mock_temp_col.count.return_value = 2
mock_new_col = MagicMock()
mock_new_col.count.return_value = 2
mock_backend = _mock_backend_for(col=mock_col, new_col=mock_new_col)
mock_backend.create_collection.side_effect = [mock_temp_col, mock_new_col]
with patch("mempalace.backends.chroma.ChromaBackend", return_value=mock_backend):
cmd_repair(args)
out = capsys.readouterr().out
assert "Repair complete" in out
assert " mcp.sock (socket)" in out.splitlines()
backup_dir = tmp_path / "palace.backup"
assert (backup_dir / "chroma.sqlite3").is_file()
assert not (backup_dir / "mcp.sock").exists()
# The socket is skipped, never removed from the live palace.
assert (palace_dir / "mcp.sock").exists()
# Dereferenced, so the backup survives losing what the link pointed at.
backed_up_link = backup_dir / "tunnels.json"
assert not backed_up_link.is_symlink()
outside.unlink()
assert backed_up_link.read_text(encoding="utf-8") == "tunnel payload"
@patch("mempalace.cli.MempalaceConfig")
def test_cmd_repair_uses_configured_collection(mock_config_cls, tmp_path, capsys):
palace_dir = tmp_path / "palace"

View File

@ -2,6 +2,7 @@
import errno
import os
import socket
import sqlite3
from types import SimpleNamespace
from unittest.mock import MagicMock, patch
@ -46,7 +47,7 @@ def test_migrate_aborts_without_confirmation(tmp_path, capsys):
return_value=[{"id": "id1", "document": "doc", "metadata": {"wing": "w", "room": "r"}}],
),
patch("builtins.input", return_value="n"),
patch("mempalace.migrate.shutil.copytree") as mock_copytree,
patch("mempalace.migrate.copy_palace_dir") as mock_backup_copy,
patch("mempalace.migrate.shutil.rmtree") as mock_rmtree,
):
result = migrate(str(palace_dir))
@ -54,7 +55,7 @@ def test_migrate_aborts_without_confirmation(tmp_path, capsys):
out = capsys.readouterr().out
assert result is False
assert "Aborted." in out
mock_copytree.assert_not_called()
mock_backup_copy.assert_not_called()
mock_rmtree.assert_not_called()
@ -304,7 +305,7 @@ def test_migrate_cleans_temp_palace_on_chromadb_failure(tmp_path):
return_value=[{"id": "id1", "document": "doc", "metadata": {"wing": "w", "room": "r"}}],
),
patch("builtins.input", return_value="y"),
patch("mempalace.migrate.shutil.copytree"),
patch("mempalace.migrate.copy_palace_dir"),
patch("mempalace.migrate.tempfile.mkdtemp", side_effect=tracking_mkdtemp),
patch.object(_chroma_mod, "ChromaBackend", return_value=failing_backend),
):
@ -321,10 +322,10 @@ def test_migrate_cleans_temp_palace_on_chromadb_failure(tmp_path):
def test_migrate_prunes_old_pre_migrate_backups(tmp_path, monkeypatch):
"""Repeated migrations must not accumulate full-palace copies forever.
The backup + prune happen right after copytree, before the (mocked)
chromadb step, so even a migration that fails afterward still trims the
backup set. We let copytree run for real so the fresh backup exists on
disk for the prune to evaluate.
The backup + prune happen right after the directory copy, before the
(mocked) chromadb step, so even a migration that fails afterward still
trims the backup set. We let the copy run for real so the fresh backup
exists on disk for the prune to evaluate.
"""
palace_dir = tmp_path / "palace"
palace_dir.mkdir()
@ -367,6 +368,69 @@ def test_migrate_prunes_old_pre_migrate_backups(tmp_path, monkeypatch):
assert "palace.pre-migrate.20260101_000000" not in backups
@pytest.mark.skipif(
os.name == "nt" or not hasattr(socket, "AF_UNIX"),
reason="Unix domain socket files are POSIX-only",
)
def test_migrate_backup_survives_a_socket_in_the_palace_directory(tmp_path, monkeypatch, capsys):
"""#2207 in the second full-palace copy: migrate takes the same backup.
The backend is mocked to fail after the backup, so the expected
RuntimeError proves the copy got past the socket instead of dying on it.
"""
# Retention comes from the user's config file otherwise, which this test
# must not read; the sibling backup tests pin it the same way.
monkeypatch.setenv("MEMPALACE_MAX_BACKUPS", "2")
palace_dir = tmp_path / "palace"
palace_dir.mkdir()
(palace_dir / "chroma.sqlite3").write_text("db")
# Pins that migrate still copies with ``symlinks=True``: links are
# recreated as links, so a link to a file outside the palace does not get
# duplicated by content into every ``.pre-migrate.*`` copy. The target is
# real, so the link resolves and only the copy mode decides the outcome.
(tmp_path / "outside.json").write_text("payload", encoding="utf-8")
try:
(palace_dir / "tunnels.json").symlink_to(tmp_path / "outside.json")
except OSError as exc:
# Only a permission refusal is a skip; anything else is a bug here.
if os.name != "nt" and exc.errno not in (errno.EPERM, errno.EACCES):
raise
pytest.skip(f"symlink creation not permitted for this user: {exc}")
# Bound by relative name: the absolute tmp path can exceed the sun_path
# limit, which is tight on macOS.
monkeypatch.chdir(palace_dir)
leftover = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
try:
leftover.bind("mcp.sock")
finally:
leftover.close()
failing_backend = MagicMock()
failing_backend.get_or_create_collection.side_effect = RuntimeError("chromadb boom")
import mempalace.backends.chroma as _chroma_mod
with (
patch("mempalace.migrate.detect_chromadb_version", return_value="0.5.x"),
patch(
"mempalace.migrate.extract_drawers_from_sqlite",
return_value=[{"id": "id1", "document": "doc", "metadata": {"wing": "w", "room": "r"}}],
),
patch.object(_chroma_mod, "ChromaBackend", return_value=failing_backend),
pytest.raises(RuntimeError, match="chromadb boom"),
):
migrate(str(palace_dir), confirm=True)
out = capsys.readouterr().out
assert " mcp.sock (socket)" in out.splitlines()
backups = list(tmp_path.glob("palace.pre-migrate.*"))
assert len(backups) == 1
assert (backups[0] / "chroma.sqlite3").read_text() == "db"
assert not (backups[0] / "mcp.sock").exists()
assert (palace_dir / "mcp.sock").exists()
assert (backups[0] / "tunnels.json").is_symlink()
def test_migrate_restores_palace_on_swap_failure(tmp_path, capsys):
"""End-to-end coverage for swap-failure rollback.