Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),

## [Unreleased]

### Bug Fixes

- **`sweep` books a failed `stat` as a failure again.** The non-regular-file gate added in 3.7.1 reads `stat.S_ISREG(f.stat().st_mode)` inside a `try`, and its `except OSError` printed `SKIP` and moved on. A dangling symlink, a symlink loop and a file unlinked between `rglob` and the gate all raise there, and before the gate existed every one of them reached `sweep()` and was booked in `failures` — so `sweep` went from reporting a transcript it could not read to reporting success. A failed probe is now an error, not a benign file type: it is logged, printed as `WARNING`, and appended to `failures`, while a probe that succeeds and says "not regular" still skips silently. (#2221)
- **`mempalace init` no longer tracebacks on a directory it cannot enter.** `_parse_gradle`'s `is_file()` gate sat in front of the `try` that the parser's own `except OSError` provides, so a manifest under a directory with `r` but no `x` raised `PermissionError` out of a call that used to answer "no manifest name". The gate moved inside that `try`, and `_collect_manifest_names` stats through `os.path.isfile`, which reports rather than raises. (#2221)
- **`split` no longer blocks on a FIFO at its own output name, nor writes through a broken link.** The type gate in `main` covers the files the glob listed; `split_file` builds its output names itself, so a pre-existing named pipe at one of them wedged `write_text` in the kernel waiting for a reader. The check asks about the link itself rather than its target, because a dangling symlink at an output name reads as "nothing there" and `write_text` would create the target — landing a chunk outside the output directory. Output names that are anything but a regular file are now skipped with a `SKIP` line. (#2221)

---

## [3.7.1] — 2026-08-12
Expand Down
2 changes: 1 addition & 1 deletion mempalace/miner.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ def _read_text_no_follow(filepath: Path, root: Path) -> Optional[tuple[str, floa
# A reader that breaks a write lease gets EAGAIN when it passes
# O_NONBLOCK, where a blocking open waits out lease-break-time
# and succeeds. The kernel grants leases on regular files only
# (F_SETLEASE on a pipe gives ENXIO), so re-check the type and
# (F_SETLEASE on a pipe fails EINVAL), so re-check the type and
# then read it the way this code did before the flag existed;
# dropping it would silently lose a file that used to be mined.
if exc.errno != errno.EAGAIN or not stat.S_ISREG(os.lstat(filepath).st_mode):
Expand Down
19 changes: 12 additions & 7 deletions mempalace/project_scanner.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,13 +188,15 @@ def _parse_pom(path: Path) -> Optional[str]:


def _parse_gradle_root_project_name(path: Path) -> Optional[str]:
# ``_parse_gradle`` reaches this with a SIBLING path it constructs itself
# (``build.gradle`` next to ``settings.gradle``), which the manifest walk
# never vetted. Opening a FIFO for reading blocks in the kernel until a
# writer appears; ``is_file()`` stats instead and never blocks.
if not path.is_file():
return None
try:
# Reached two ways: with the walk-vetted manifest path, and from
# ``_parse_gradle`` with a ``settings.gradle`` sibling it builds next
# to a vetted ``build.gradle`` — that one no walk ever saw. Opening a
# FIFO for reading blocks in the kernel until a writer appears;
# ``is_file()`` stats instead. It goes inside this ``try``, which
# already absorbs the PermissionError an unsearchable parent raises.
if not path.is_file():
return None
text = path.read_text(encoding="utf-8", errors="replace")
except OSError:
return None
Expand Down Expand Up @@ -431,7 +433,10 @@ def _collect_manifest_names(repo_root: Path) -> list[tuple[str, str, Path]]:
# Every parser below opens the path. A FIFO named
# ``package.json`` would park that open in the kernel until a
# writer appears; ``is_file()`` stats instead and never blocks.
if not manifest_path.is_file():
# ``os.path.isfile`` rather than ``Path.is_file``: each parser
# swallows OSError itself, so the gate must swallow it too — an
# unsearchable directory used to yield "no name", not a traceback.
if not os.path.isfile(manifest_path):
continue
name = parser(manifest_path)
if name:
Expand Down
12 changes: 12 additions & 0 deletions mempalace/split_mega_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,18 @@ def split_file(filepath, output_dir, dry_run=False):
if dry_run:
print(f" [{i + 1}/{len(boundaries) - 1}] {name} ({len(chunk)} lines)")
else:
# The gate in ``main`` covers the files the glob listed; this name
# is built here, so nothing has vetted it. Opening a pre-existing
# FIFO for writing blocks in the kernel until a reader appears.
# ``lexists`` rather than ``exists``: the latter follows the link
# and so answers False for a DANGLING symlink, and writing through
# one creates the target instead — a chunk landing wherever the
# link points, outside the output directory entirely.
# ``os.path`` rather than ``Path``: neither call can raise, so a
# write that fails still fails at ``write_text`` as it always did.
if os.path.lexists(out_path) and not os.path.isfile(out_path):
print(f" SKIP: {name} (not a regular file)")
continue
out_path.write_text("".join(chunk), encoding="utf-8")
print(f" + {name} ({len(chunk)} lines)")

Expand Down
9 changes: 8 additions & 1 deletion mempalace/sweeper.py
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,14 @@ def sweep_directory(dir_path: str, palace_path: str) -> dict:
try:
regular = stat.S_ISREG(f.stat().st_mode)
except OSError as exc:
print(f" SKIP: {f.name} (stat error: {exc.strerror or exc})", file=sys.stderr)
# A stat that FAILS is a real error, not a benign type. A dangling
# symlink, a symlink loop and a file unlinked between rglob and
# here all land here, and every one of them used to reach ``open``
# and be booked below. Keep booking them, or ``sweep`` reports
# success on a transcript it could not read.
logger.error("sweeper: stat failed on %s: %s", f, exc)
print(f" WARNING: stat failed on {f}: {exc}", file=sys.stderr)
failures.append({"file": str(f), "error": str(exc)})
continue
if not regular:
print(f" SKIP: {f.name} (not a regular file)", file=sys.stderr)
Expand Down
141 changes: 140 additions & 1 deletion tests/test_non_regular_file_guards.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,11 @@
from mempalace.llm_refine import collect_corpus_text
from mempalace.miner import _read_text_no_follow, load_config, mine, scan_project
from mempalace.normalize import _read_transcript_file
from mempalace.project_scanner import _collect_manifest_names
from mempalace.project_scanner import _collect_manifest_names, _parse_gradle
from mempalace.repair import _copy_file_no_follow, _open_regular_file_no_follow
from mempalace.room_detector_local import detect_rooms_local
from mempalace.split_mega_files import main as split_main
from mempalace.split_mega_files import split_file
from mempalace.sweeper import parse_claude_jsonl, sweep_directory

# ``os.mkfifo`` and ``SIGALRM`` are both POSIX-only. Windows has no FIFO in
Expand All @@ -54,6 +55,17 @@
reason="requires POSIX FIFOs and SIGALRM",
)

# Root holds CAP_DAC_OVERRIDE and walks straight into a directory with no
# ``x`` bit, so the file each test walls off stays readable and the assertion
# below breaks: the state these tests need cannot be built as root, they do
# not merely pass vacuously there. ``tests/test_backups.py`` gates the same
# way and additionally excludes Windows, which it has to because it carries
# no ``posix_only``; every use here already sits under ``posix_only``.
needs_unprivileged_posix = pytest.mark.skipif(
hasattr(os, "geteuid") and os.geteuid() == 0,
reason="directory permission bits do not gate root",
)

TIMEOUT_SECONDS = 10.0


Expand Down Expand Up @@ -356,6 +368,104 @@ def test_split_mega_files_skips_fifo(tmp_path, capsys, monkeypatch):
assert "real.txt" in out


@posix_only
def test_split_file_skips_a_fifo_at_its_own_output_name(tmp_path, capsys):
"""The walk gate covers the source; the output name is built here.

``split_file`` synthesises each per-session filename from the transcript
and writes it into the source directory, so nothing has vetted that path.
A pre-existing FIFO sitting at one of those names turned the write into a
blocking open — the same hang, in the one path the discovery gate cannot
reach.
"""
session = "Claude Code v1.0\n" + "content line\n" * 14 + "\n" * 5
source = write_regular(tmp_path, "real.txt", session * 2)
planned = split_file(str(source), None, dry_run=True)
assert len(planned) >= 2, "fixture must produce at least two output files"
blocked = Path(planned[0])
os.mkfifo(blocked)

with hard_timeout(TIMEOUT_SECONDS, "split_file writing over a FIFO output"):
written = split_file(str(source), None, dry_run=False)

out = capsys.readouterr().out
assert f"SKIP: {blocked.name} (not a regular file)" in out
assert blocked not in written
# The pipe must cost only its own chunk: every other session still lands.
assert len(written) == len(planned) - 1
assert all(path.is_file() for path in written)


@posix_only
def test_split_file_skips_a_dangling_symlink_at_its_own_output_name(tmp_path, capsys):
"""A broken link at an output name must not redirect the write.

``os.path.exists`` follows the link and answers False for a dangling one,
so the type gate would wave it through — and ``write_text`` then CREATES
the target, landing a chunk wherever the link points instead of in the
output directory. The gate has to ask about the link itself.
"""
session = "Claude Code v1.0\n" + "content line\n" * 14 + "\n" * 5
source = write_regular(tmp_path, "real.txt", session * 2)
planned = split_file(str(source), None, dry_run=True)
assert len(planned) >= 2, "fixture must produce at least two output files"
blocked = Path(planned[0])
outside = tmp_path / "outside" / "victim.txt"
outside.parent.mkdir()
os.symlink(outside, blocked)
assert not outside.exists(), "the link must dangle before the run"

with hard_timeout(TIMEOUT_SECONDS, "split_file writing over a dangling symlink"):
written = split_file(str(source), None, dry_run=False)

out = capsys.readouterr().out
assert f"SKIP: {blocked.name} (not a regular file)" in out
assert blocked not in written
assert not outside.exists(), "a chunk was written through the link, outside the output dir"
assert len(written) == len(planned) - 1


@posix_only
@needs_unprivileged_posix
def test_collect_manifest_names_survives_an_unreadable_directory(tmp_path):
"""The type gate must not turn a skipped manifest into a crash.

``os.walk`` lists the children of a directory with ``r`` but no ``x``,
and stating one of them raises ``PermissionError``. Each parser already
swallowed that through its own ``except OSError``, so the gate in front
of them has to swallow it too — otherwise ``mempalace init`` gains a
traceback where it used to report no manifest name.
"""
repo = tmp_path / "repo"
repo.mkdir()
(repo / "package.json").write_text('{"name": "inner"}', encoding="utf-8")
os.chmod(repo, 0o444)
try:
with hard_timeout(TIMEOUT_SECONDS, "_collect_manifest_names over an unreadable dir"):
found = _collect_manifest_names(repo)
finally:
os.chmod(repo, 0o755)
assert found == []


@posix_only
@needs_unprivileged_posix
def test_parse_gradle_survives_an_unreadable_directory(tmp_path):
"""The sibling ``settings.gradle`` is stat'd inside the parser's own try."""
repo = tmp_path / "repo"
repo.mkdir()
build = repo / "build.gradle"
build.write_text("plugins { id 'java' }\n", encoding="utf-8")
os.chmod(repo, 0o444)
try:
with hard_timeout(TIMEOUT_SECONDS, "_parse_gradle over an unreadable dir"):
name = _parse_gradle(build)
finally:
os.chmod(repo, 0o755)
# Falls back to the directory name, exactly as it did before the gate.
assert name == "repo"


@posix_only
def test_format_miner_extract_text_does_not_block_on_fifo(tmp_path):
"""``mine --mode extract`` was already immune — its zero-size gate fires
Expand Down Expand Up @@ -584,6 +694,34 @@ def test_sweep_directory_skips_a_fifo_without_booking_a_failure(tmp_path, capsys
assert "SKIP: piped.jsonl (not a regular file)" in capsys.readouterr().err


@posix_only
def test_sweep_directory_still_books_a_stat_failure_as_a_failure(tmp_path, capsys):
"""A pipe is nothing to sweep; a stat that FAILS is a real error.

The type gate has to tell those apart. A dangling symlink, a symlink loop
and a file unlinked between ``rglob`` and the gate all raise from
``stat`` — and every one of them used to reach ``open`` inside ``sweep``
and be booked. Swallowing them would flip ``mempalace sweep`` from exit 2
to exit 0 on a transcript it could not read.
"""
convos = tmp_path / "convos"
convos.mkdir()
write_regular(
convos,
"real.jsonl",
'{"type": "user", "sessionId": "s1", "uuid": "u1", '
'"timestamp": "2026-01-01T00:00:00Z", '
'"message": {"role": "user", "content": "hello"}}\n',
)
os.symlink(convos / "gone.jsonl", convos / "dangling.jsonl")
with hard_timeout(TIMEOUT_SECONDS, "sweep_directory over a dangling symlink"):
result = sweep_directory(str(convos), str(tmp_path / "palace"))
# ``cli.cmd_sweep`` turns a non-empty ``failures`` into ``sys.exit(2)``.
assert [Path(entry["file"]).name for entry in result["failures"]] == ["dangling.jsonl"]
assert result["files_succeeded"] == 1
assert "stat failed" in capsys.readouterr().err


# ─────────────────────────────────────────────────────────────────────────
# O_NONBLOCK must not drop a regular file the blocking open would have read
# ─────────────────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -645,6 +783,7 @@ def _fake_open(path, flags, *args, **kwargs):


@posix_only
@needs_unprivileged_posix
def test_gather_origin_samples_survives_an_unreadable_directory(tmp_path):
"""The type gate must not turn a skipped file into a crash.

Expand Down