Skip to content

Commit ca0db35

Browse files
committed
Merge remote-tracking branch 'upstream/develop' into feat/gossip-echo
2 parents 0a28eab + 639c69a commit ca0db35

8 files changed

Lines changed: 200 additions & 18 deletions

File tree

.github/CODEOWNERS

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,19 @@
1+
# Every owner listed here must have write access to the repository —
2+
# GitHub silently ignores a rule naming a user who does not, which
3+
# leaves the matching paths with no reviewer at all.
4+
15
# Default owners for everything
2-
* @milla-jovovich @bensig @igorls
6+
* @milla-jovovich @igorls
37

48
# Core library
5-
mempalace/ @milla-jovovich @bensig
9+
mempalace/ @milla-jovovich @igorls
610

711
# CI and workflows
8-
.github/ @bensig
12+
.github/ @igorls
913

1014
# Plugins and integrations
11-
.claude-plugin/ @bensig
12-
.codex-plugin/ @bensig
13-
integrations/ @bensig
15+
.antigravity-plugin/ @igorls
16+
.claude-plugin/ @igorls
17+
.codex-plugin/ @igorls
18+
.cursor-plugin/ @igorls
19+
integrations/ @igorls

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
88

99
## [Unreleased]
1010

11+
### Bug Fixes
12+
13+
- **`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)
14+
- **`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)
15+
- **`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)
16+
1117
---
1218

1319
## [3.7.1] — 2026-08-12

mempalace/miner.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ def _read_text_no_follow(filepath: Path, root: Path) -> Optional[tuple[str, floa
8484
# A reader that breaks a write lease gets EAGAIN when it passes
8585
# O_NONBLOCK, where a blocking open waits out lease-break-time
8686
# and succeeds. The kernel grants leases on regular files only
87-
# (F_SETLEASE on a pipe gives ENXIO), so re-check the type and
87+
# (F_SETLEASE on a pipe fails EINVAL), so re-check the type and
8888
# then read it the way this code did before the flag existed;
8989
# dropping it would silently lose a file that used to be mined.
9090
if exc.errno != errno.EAGAIN or not stat.S_ISREG(os.lstat(filepath).st_mode):

mempalace/project_scanner.py

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -188,13 +188,15 @@ def _parse_pom(path: Path) -> Optional[str]:
188188

189189

190190
def _parse_gradle_root_project_name(path: Path) -> Optional[str]:
191-
# ``_parse_gradle`` reaches this with a SIBLING path it constructs itself
192-
# (``build.gradle`` next to ``settings.gradle``), which the manifest walk
193-
# never vetted. Opening a FIFO for reading blocks in the kernel until a
194-
# writer appears; ``is_file()`` stats instead and never blocks.
195-
if not path.is_file():
196-
return None
197191
try:
192+
# Reached two ways: with the walk-vetted manifest path, and from
193+
# ``_parse_gradle`` with a ``settings.gradle`` sibling it builds next
194+
# to a vetted ``build.gradle`` — that one no walk ever saw. Opening a
195+
# FIFO for reading blocks in the kernel until a writer appears;
196+
# ``is_file()`` stats instead. It goes inside this ``try``, which
197+
# already absorbs the PermissionError an unsearchable parent raises.
198+
if not path.is_file():
199+
return None
198200
text = path.read_text(encoding="utf-8", errors="replace")
199201
except OSError:
200202
return None
@@ -431,7 +433,10 @@ def _collect_manifest_names(repo_root: Path) -> list[tuple[str, str, Path]]:
431433
# Every parser below opens the path. A FIFO named
432434
# ``package.json`` would park that open in the kernel until a
433435
# writer appears; ``is_file()`` stats instead and never blocks.
434-
if not manifest_path.is_file():
436+
# ``os.path.isfile`` rather than ``Path.is_file``: each parser
437+
# swallows OSError itself, so the gate must swallow it too — an
438+
# unsearchable directory used to yield "no name", not a traceback.
439+
if not os.path.isfile(manifest_path):
435440
continue
436441
name = parser(manifest_path)
437442
if name:

mempalace/split_mega_files.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,18 @@ def split_file(filepath, output_dir, dry_run=False):
224224
if dry_run:
225225
print(f" [{i + 1}/{len(boundaries) - 1}] {name} ({len(chunk)} lines)")
226226
else:
227+
# The gate in ``main`` covers the files the glob listed; this name
228+
# is built here, so nothing has vetted it. Opening a pre-existing
229+
# FIFO for writing blocks in the kernel until a reader appears.
230+
# ``lexists`` rather than ``exists``: the latter follows the link
231+
# and so answers False for a DANGLING symlink, and writing through
232+
# one creates the target instead — a chunk landing wherever the
233+
# link points, outside the output directory entirely.
234+
# ``os.path`` rather than ``Path``: neither call can raise, so a
235+
# write that fails still fails at ``write_text`` as it always did.
236+
if os.path.lexists(out_path) and not os.path.isfile(out_path):
237+
print(f" SKIP: {name} (not a regular file)")
238+
continue
227239
out_path.write_text("".join(chunk), encoding="utf-8")
228240
print(f" + {name} ({len(chunk)} lines)")
229241

mempalace/sweeper.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,14 @@ def sweep_directory(dir_path: str, palace_path: str) -> dict:
356356
try:
357357
regular = stat.S_ISREG(f.stat().st_mode)
358358
except OSError as exc:
359-
print(f" SKIP: {f.name} (stat error: {exc.strerror or exc})", file=sys.stderr)
359+
# A stat that FAILS is a real error, not a benign type. A dangling
360+
# symlink, a symlink loop and a file unlinked between rglob and
361+
# here all land here, and every one of them used to reach ``open``
362+
# and be booked below. Keep booking them, or ``sweep`` reports
363+
# success on a transcript it could not read.
364+
logger.error("sweeper: stat failed on %s: %s", f, exc)
365+
print(f" WARNING: stat failed on {f}: {exc}", file=sys.stderr)
366+
failures.append({"file": str(f), "error": str(exc)})
360367
continue
361368
if not regular:
362369
print(f" SKIP: {f.name} (not a regular file)", file=sys.stderr)

tests/test_non_regular_file_guards.py

Lines changed: 140 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,11 @@
4040
from mempalace.llm_refine import collect_corpus_text
4141
from mempalace.miner import _read_text_no_follow, load_config, mine, scan_project
4242
from mempalace.normalize import _read_transcript_file
43-
from mempalace.project_scanner import _collect_manifest_names
43+
from mempalace.project_scanner import _collect_manifest_names, _parse_gradle
4444
from mempalace.repair import _copy_file_no_follow, _open_regular_file_no_follow
4545
from mempalace.room_detector_local import detect_rooms_local
4646
from mempalace.split_mega_files import main as split_main
47+
from mempalace.split_mega_files import split_file
4748
from mempalace.sweeper import parse_claude_jsonl, sweep_directory
4849

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

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

5971

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

358370

371+
@posix_only
372+
def test_split_file_skips_a_fifo_at_its_own_output_name(tmp_path, capsys):
373+
"""The walk gate covers the source; the output name is built here.
374+
375+
``split_file`` synthesises each per-session filename from the transcript
376+
and writes it into the source directory, so nothing has vetted that path.
377+
A pre-existing FIFO sitting at one of those names turned the write into a
378+
blocking open — the same hang, in the one path the discovery gate cannot
379+
reach.
380+
"""
381+
session = "Claude Code v1.0\n" + "content line\n" * 14 + "\n" * 5
382+
source = write_regular(tmp_path, "real.txt", session * 2)
383+
planned = split_file(str(source), None, dry_run=True)
384+
assert len(planned) >= 2, "fixture must produce at least two output files"
385+
blocked = Path(planned[0])
386+
os.mkfifo(blocked)
387+
388+
with hard_timeout(TIMEOUT_SECONDS, "split_file writing over a FIFO output"):
389+
written = split_file(str(source), None, dry_run=False)
390+
391+
out = capsys.readouterr().out
392+
assert f"SKIP: {blocked.name} (not a regular file)" in out
393+
assert blocked not in written
394+
# The pipe must cost only its own chunk: every other session still lands.
395+
assert len(written) == len(planned) - 1
396+
assert all(path.is_file() for path in written)
397+
398+
399+
@posix_only
400+
def test_split_file_skips_a_dangling_symlink_at_its_own_output_name(tmp_path, capsys):
401+
"""A broken link at an output name must not redirect the write.
402+
403+
``os.path.exists`` follows the link and answers False for a dangling one,
404+
so the type gate would wave it through — and ``write_text`` then CREATES
405+
the target, landing a chunk wherever the link points instead of in the
406+
output directory. The gate has to ask about the link itself.
407+
"""
408+
session = "Claude Code v1.0\n" + "content line\n" * 14 + "\n" * 5
409+
source = write_regular(tmp_path, "real.txt", session * 2)
410+
planned = split_file(str(source), None, dry_run=True)
411+
assert len(planned) >= 2, "fixture must produce at least two output files"
412+
blocked = Path(planned[0])
413+
outside = tmp_path / "outside" / "victim.txt"
414+
outside.parent.mkdir()
415+
os.symlink(outside, blocked)
416+
assert not outside.exists(), "the link must dangle before the run"
417+
418+
with hard_timeout(TIMEOUT_SECONDS, "split_file writing over a dangling symlink"):
419+
written = split_file(str(source), None, dry_run=False)
420+
421+
out = capsys.readouterr().out
422+
assert f"SKIP: {blocked.name} (not a regular file)" in out
423+
assert blocked not in written
424+
assert not outside.exists(), "a chunk was written through the link, outside the output dir"
425+
assert len(written) == len(planned) - 1
426+
427+
428+
@posix_only
429+
@needs_unprivileged_posix
430+
def test_collect_manifest_names_survives_an_unreadable_directory(tmp_path):
431+
"""The type gate must not turn a skipped manifest into a crash.
432+
433+
``os.walk`` lists the children of a directory with ``r`` but no ``x``,
434+
and stating one of them raises ``PermissionError``. Each parser already
435+
swallowed that through its own ``except OSError``, so the gate in front
436+
of them has to swallow it too — otherwise ``mempalace init`` gains a
437+
traceback where it used to report no manifest name.
438+
"""
439+
repo = tmp_path / "repo"
440+
repo.mkdir()
441+
(repo / "package.json").write_text('{"name": "inner"}', encoding="utf-8")
442+
os.chmod(repo, 0o444)
443+
try:
444+
with hard_timeout(TIMEOUT_SECONDS, "_collect_manifest_names over an unreadable dir"):
445+
found = _collect_manifest_names(repo)
446+
finally:
447+
os.chmod(repo, 0o755)
448+
assert found == []
449+
450+
451+
@posix_only
452+
@needs_unprivileged_posix
453+
def test_parse_gradle_survives_an_unreadable_directory(tmp_path):
454+
"""The sibling ``settings.gradle`` is stat'd inside the parser's own try."""
455+
repo = tmp_path / "repo"
456+
repo.mkdir()
457+
build = repo / "build.gradle"
458+
build.write_text("plugins { id 'java' }\n", encoding="utf-8")
459+
os.chmod(repo, 0o444)
460+
try:
461+
with hard_timeout(TIMEOUT_SECONDS, "_parse_gradle over an unreadable dir"):
462+
name = _parse_gradle(build)
463+
finally:
464+
os.chmod(repo, 0o755)
465+
# Falls back to the directory name, exactly as it did before the gate.
466+
assert name == "repo"
467+
468+
359469
@posix_only
360470
def test_format_miner_extract_text_does_not_block_on_fifo(tmp_path):
361471
"""``mine --mode extract`` was already immune — its zero-size gate fires
@@ -584,6 +694,34 @@ def test_sweep_directory_skips_a_fifo_without_booking_a_failure(tmp_path, capsys
584694
assert "SKIP: piped.jsonl (not a regular file)" in capsys.readouterr().err
585695

586696

697+
@posix_only
698+
def test_sweep_directory_still_books_a_stat_failure_as_a_failure(tmp_path, capsys):
699+
"""A pipe is nothing to sweep; a stat that FAILS is a real error.
700+
701+
The type gate has to tell those apart. A dangling symlink, a symlink loop
702+
and a file unlinked between ``rglob`` and the gate all raise from
703+
``stat`` — and every one of them used to reach ``open`` inside ``sweep``
704+
and be booked. Swallowing them would flip ``mempalace sweep`` from exit 2
705+
to exit 0 on a transcript it could not read.
706+
"""
707+
convos = tmp_path / "convos"
708+
convos.mkdir()
709+
write_regular(
710+
convos,
711+
"real.jsonl",
712+
'{"type": "user", "sessionId": "s1", "uuid": "u1", '
713+
'"timestamp": "2026-01-01T00:00:00Z", '
714+
'"message": {"role": "user", "content": "hello"}}\n',
715+
)
716+
os.symlink(convos / "gone.jsonl", convos / "dangling.jsonl")
717+
with hard_timeout(TIMEOUT_SECONDS, "sweep_directory over a dangling symlink"):
718+
result = sweep_directory(str(convos), str(tmp_path / "palace"))
719+
# ``cli.cmd_sweep`` turns a non-empty ``failures`` into ``sys.exit(2)``.
720+
assert [Path(entry["file"]).name for entry in result["failures"]] == ["dangling.jsonl"]
721+
assert result["files_succeeded"] == 1
722+
assert "stat failed" in capsys.readouterr().err
723+
724+
587725
# ─────────────────────────────────────────────────────────────────────────
588726
# O_NONBLOCK must not drop a regular file the blocking open would have read
589727
# ─────────────────────────────────────────────────────────────────────────
@@ -645,6 +783,7 @@ def _fake_open(path, flags, *args, **kwargs):
645783

646784

647785
@posix_only
786+
@needs_unprivileged_posix
648787
def test_gather_origin_samples_survives_an_unreadable_directory(tmp_path):
649788
"""The type gate must not turn a skipped file into a crash.
650789

website/reference/contributing.md

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,23 @@ PRs welcome. MemPalace is open source and we welcome contributions of all sizes
55
## Getting Started
66

77
```bash
8-
git clone https://github.com/MemPalace/mempalace.git
8+
# Fork the repo on GitHub first, then clone your fork
9+
git clone https://github.com/<your-username>/mempalace.git
910
cd mempalace
11+
git remote add upstream https://github.com/MemPalace/mempalace.git
1012

1113
# Recommended: uv (https://docs.astral.sh/uv/) manages the venv for you
1214
uv sync --extra dev
1315

1416
# Or with pip in your own venv:
1517
# pip install -e ".[dev]"
18+
19+
# Activate pre-commit hooks (one-time, per clone)
20+
pre-commit install
1621
```
1722

23+
The `pre-commit install` step matters: the repo pins ruff to the exact version CI uses, so without it you can commit code that passes your local lint but fails CI on push.
24+
1825
## Running Tests
1926

2027
```bash
@@ -46,7 +53,7 @@ See [Benchmarks](/reference/benchmarks) for data download instructions.
4653
- `fix: handle empty transcript files`
4754
- `docs: update MCP tool descriptions`
4855
- `bench: add LoCoMo turn-level metrics`
49-
6. Push to your fork and open a PR against `main`
56+
6. Push to your fork and open a PR against `develop`
5057

5158
## Code Style
5259

0 commit comments

Comments
 (0)