fix: 3.7.1 critical patch — hang, re-mine honesty, stale Chroma cache - #2228
Merged
Conversation
…2002) _get_client() detects a peer writer's inode/mtime change and rebuilds the client via ChromaBackend.make_client(), but chromadb caches its System (and the live in-memory HNSW segment) keyed by path. The rebuilt client is handed back the same stale segment, which on its next _persist() overwrites the on-disk index, destroying records other writers had already indexed. Observed in a live multi-writer palace: the persisted index count went backwards (4 to 3). Call the existing _force_chroma_cache_reset() on the staleness path, before make_client(), so chromadb rebuilds the segment from the on-disk state. The call is guarded by the existing inode_changed/mtime_changed check, so it has no effect on first-open. Adds test_get_client_resets_chroma_system_cache_on_reconnect, which asserts the reset runs before make_client on an mtime reconnect (fails without the fix). Refs #1963.
…n inode/mtime reopen _client() reconstructs PersistentClient on an inode/mtime change but did not drop chromadb's process-global SharedSystemClient cache first, so the rebuilt client reused the stale path-keyed System (and its in-memory HNSW segment) and could persist an outdated index over on-disk changes -- the same class as #2002, reached via _client() instead of _get_client. Add SharedSystemClient.clear_system_cache() to the external-change branch of _client(), mirroring mcp_server._force_chroma_cache_reset (#2026) and repair._close_chroma_handles. Backend-level regression test asserts the reset fires on the change reopen, strictly before the reconstruct, and not on first open (chroma-core/chroma#2536, #5843). Fixes #2028.
Two monkeypatch.setattr calls were wrapped across lines that fit within the line length; ruff format --check flagged them. Formatter-only, no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gg6g5efZ1rNbBTHqGz2Tjw
The #2002 staleness reconnect makes _get_client() call _force_chroma_cache_reset(), which clears chromadb's path-keyed SharedSystemClient cache. Two TestDeleteBySource tests grabbed a closets collection handle *before* calling tool_delete_by_source and then asserted on it afterwards, by which point the reset had dropped the Rust binding underneath the handle (AttributeError: 'RustBindingsAPI' object has no attribute 'bindings'). Re-acquire the closets collection after the tool call in both tests. Production callers already re-acquire fresh handles per call, so this is a test-lifetime issue, not a regression in the fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gg6g5efZ1rNbBTHqGz2Tjw
#21 (CRITICAL, data-loss): multi-batch re-mine had no completion marker. A mid-file crash after batch 1 committed but before a later batch left permanently silent partial data -- the surviving drawers shared the file's unchanged on-disk mtime, so file_already_mined() treated the file as fully mined forever. Every chunk now carries chunk_total, and file_already_mined() verifies a matching-mtime group's drawer count reaches chunk_total before reporting True. Drawers with no chunk_total (legacy rows, single-shot add_drawer()) are trusted as before. #22 (HIGH, correctness/TOCTOU): source_mtime was captured via a fresh os.path.getmtime() well after content was read, chunked, and room-detected. A file appended to in that window got its new drawers stamped with an mtime that already matched the (now newer) on-disk state, so the appended tail was silently, permanently skipped on every future mine. _read_text_no_follow now returns (content, mtime) from the same fstat() that validates the file; process_file threads that single value through instead of re-stating. #23 (HIGH, silent-failure): a failed stale-drawer purge was swallowed to a debug log and mining proceeded anyway, silently producing duplicate or orphaned drawers. A purge failure now aborts this file's mine attempt (old drawers' stored mtime is untouched, so the next mine still sees a mismatch and retries) and prints a visible warning, matching every other degraded path in this module. #24 (LOW, data-loss): the old-drawer delete ran unconditionally, but the closet purge+rebuild only ran when drawers_added > 0 -- a file whose chunks all landed below min_chunk_size after boundary-splitting lost its drawers but kept stale closets pointing at now-deleted IDs. purge_file_closets now runs whenever the delete-and-rebuild cycle does, regardless of the new chunk count; only the rebuild itself stays conditional. 169 tests pass across test_miner.py/test_convo_miner*.py/test_palace.py/ test_hallways.py/test_format_miner.py/test_miner_fts5_validation.py, no regressions. Full suite: 1 unrelated pre-existing flake in test_mcp_server.py (module-global peer-writer-lock state leaking across test files in full-suite ordering -- passes standalone and as a full file; the diff here never touches mcp_server.py).
`os.walk` and `glob` list a FIFO, a socket and a device node as ordinary filenames, and MemPalace decides what to read from the suffix. Opening a FIFO for reading parks in the kernel until a writer appears, so a named pipe called `notes.md` in a mined directory wedged `mempalace mine` forever — no output, no error, no progress. `mine --mode convos`, `sweep`, `init`, `compress` and `split` blocked the same way. Two shapes are at fault. Four helpers already refused non-regular files with `fstat` + `S_ISREG`, but the check sat *after* a blocking `os.open`, so it could never run. Adding `O_NONBLOCK` to those opens makes the existing type check reachable: the open returns immediately and the file mode decides, with no errno guesswork. A FIFO that does have a live writer is refused just the same. Linux open(2) states the flag has no effect on regular files; the one exception is a write lease, where a non-blocking open fails EAGAIN instead of waiting out lease-break-time. Leases are granted on regular files only, so that branch re-checks the type and then opens without the flag rather than silently dropping a file that used to be mined. The rest guard with `exists()`, which is true for a pipe, and then open anyway. Those become type checks: a discovery walk drops non-regular entries before any reader sees them, and a fixed-name read decides with `is_file()` instead. `scan_project` and `scan_convos` already stat every candidate for the size limit, so the type check costs no extra syscall. Where the gate replaced an `open` that sat inside a `try`, it goes in the same `try`: `is_file()` raises `PermissionError` on a directory without `x`, which that handler already absorbed. O_NONBLOCK: miner._read_text_no_follow, convo_miner._is_regular_source_file, normalize._read_transcript_file, repair._open_regular_file_no_follow. Type gate: miner.scan_project, miner.load_config, convo_miner.scan_convos, sweeper.parse_claude_jsonl, sweeper.sweep_directory, entity_detector.detect_entities, cli._gather_origin_samples, cli._ensure_mempalace_files_gitignored, cli.cmd_compress, cli.cmd_init, project_scanner._collect_manifest_names, project_scanner._parse_gradle_root_project_name, room_detector_local.detect_rooms_local, llm_refine.collect_corpus_text, split_mega_files.main, hook_shell.count_human_messages.
Stack the post-3.7.0 hang and silent-skip fixes for a fast patch release: - Keep #2223 (non-regular file hang) and #2088 (chunk_total / same-fstat mtime / purge abort / closet purge) as the base. - On multi-batch upsert failure, delete partial drawers and closets for that source before re-raising so the next mine retries (#2122, #2151). - Install SIGTERM/SIGHUP handlers in mcp_server.main so atexit can release the palace writer lease (#2205). - Adapt non-regular-file tests to the (content, mtime) read return type.
) Port project-miner re-mine honesty to conversation ingest so an interrupted transcript mine cannot permanently skip missing exchanges: - stamp chunk_total on every convo drawer in the pass - delete partial drawers for the source/extract_mode on upsert failure - teach prefetch_mined_set the same completeness rule as file_already_mined
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 759b8f13cd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # the next mine. The source lock prevents this cleanup from | ||
| # deleting another miner's work for the same file. (#2122) | ||
| try: | ||
| collection.delete(where={"source_file": source_file}) |
There was a problem hiding this comment.
When a re-mine fails on its second or later upsert batch, the old complete drawers have already been purged, and this new cleanup deletes every successfully written replacement batch and its closets before re-raising. A transient embedding or Chroma error therefore leaves the source with no searchable memory at all, whereas retaining the committed batches would at least preserve partial recall; the replacement should be staged and swapped only after all batches succeed, or the previous complete set must be restored on failure. The mirrored cleanup in convo_miner._file_chunks_locked has the same issue. This conflicts with the repository's append-only and crash-safety invariant.
AGENTS.md reference: AGENTS.md:L22-L23
Useful? React with 👍 / 👎.
…name In-place rebuild tests archive the palace directory after _seed_palace. backend.close() alone left chromadb's path-keyed System holding files open on Windows (WinError 5), so rebuild_from_sqlite aborted before the mocked upsert path and test_rebuild_from_sqlite_raises_on_upsert_failure never raised RebuildPartialError. Clear the shared cache and GC after close.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc1431a661
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # rebuilt client persists an outdated index over the on-disk | ||
| # change. Gated on genuine external change (not first open) so | ||
| # cold opens never pay the global-evict cost. | ||
| _clear_chroma_system_cache() |
There was a problem hiding this comment.
After any write changes chroma.sqlite3, the next backend lookup reaches this branch because _freshness is updated only when _client() opens a client, not by write methods. This clears SharedSystemClient while self._clients[palace_path] and previously returned collection adapters still reference the old live System, then constructs a second System for the same palace; the stale System can subsequently persist its old HNSW state over newer data. Mirror _force_chroma_cache_reset() by closing and evicting the cached palace client before clearing the shared cache, or refresh the snapshot after self-writes so this path is limited to genuine external changes.
AGENTS.md reference: AGENTS.md:L23-L23
Useful? React with 👍 / 👎.
This was referenced Aug 12, 2026
Merged
This was referenced Aug 13, 2026
mvalentsev
added a commit
to mvalentsev/mempalace
that referenced
this pull request
Aug 13, 2026
…mPalace#2221) The MemPalace#2221 fix reached develop through MemPalace#2228 at an earlier revision of the branch, so three guards and their tests did not come with it. One of the three is a regression the gate that did land introduced. sweep_directory: the new gate probes the file type with f.stat() inside a try, and its except OSError printed SKIP and continued. A dangling symlink, a symlink loop and a file unlinked between rglob and the gate all raise there. Before the gate existed each of them reached sweep(), raised, and was appended to failures — so the gate turned "could not read this transcript" into a silent skip and a successful exit. A probe that FAILS is an error, not a benign file type: log it, print WARNING, book it in failures. A probe that succeeds and reports a non-regular file still skips silently. _parse_gradle: the is_file() gate sat in front of the try whose except OSError the parser already had, 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 moves inside that try. _collect_manifest_names stats with os.path.isfile, which reports instead of raising, matching the parsers it guards. split_file: the type gate in main() covers the files the glob listed, but split_file builds its output names itself, so a pre-existing FIFO at one of them wedged write_text in the kernel waiting for a reader. Output names that are anything but a regular file are skipped. That gate asks os.path.lexists, not os.path.exists. exists() follows the link, so a DANGLING symlink at an output name reads as "nothing there" and the write goes through it, creating the target — a chunk landing wherever the link points rather than in the output directory. Measured: the two calls differ on that one case and agree on every other (regular file, symlink to a file, missing name, FIFO, symlink to FIFO, directory). test_gather_origin_samples_survives_an_unreadable_directory broke under root rather than passing vacuously: CAP_DAC_OVERRIDE walks into the 0o444 directory, the walled-off file stays readable, and the count assertion sees two samples instead of one. It now carries the same needs_unprivileged_posix gate as the three new permission tests. miner._read_text_no_follow: comment fix only. F_SETLEASE on a FIFO fails EINVAL, not ENXIO — measured on Linux 6.18 / glibc 2.39. The code branches on EAGAIN and is unaffected.
jjcav84
pushed a commit
to jjcav84/mempalace
that referenced
this pull request
Aug 22, 2026
…mPalace#2221) The MemPalace#2221 fix reached develop through MemPalace#2228 at an earlier revision of the branch, so three guards and their tests did not come with it. One of the three is a regression the gate that did land introduced. sweep_directory: the new gate probes the file type with f.stat() inside a try, and its except OSError printed SKIP and continued. A dangling symlink, a symlink loop and a file unlinked between rglob and the gate all raise there. Before the gate existed each of them reached sweep(), raised, and was appended to failures — so the gate turned "could not read this transcript" into a silent skip and a successful exit. A probe that FAILS is an error, not a benign file type: log it, print WARNING, book it in failures. A probe that succeeds and reports a non-regular file still skips silently. _parse_gradle: the is_file() gate sat in front of the try whose except OSError the parser already had, 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 moves inside that try. _collect_manifest_names stats with os.path.isfile, which reports instead of raising, matching the parsers it guards. split_file: the type gate in main() covers the files the glob listed, but split_file builds its output names itself, so a pre-existing FIFO at one of them wedged write_text in the kernel waiting for a reader. Output names that are anything but a regular file are skipped. That gate asks os.path.lexists, not os.path.exists. exists() follows the link, so a DANGLING symlink at an output name reads as "nothing there" and the write goes through it, creating the target — a chunk landing wherever the link points rather than in the output directory. Measured: the two calls differ on that one case and agree on every other (regular file, symlink to a file, missing name, FIFO, symlink to FIFO, directory). test_gather_origin_samples_survives_an_unreadable_directory broke under root rather than passing vacuously: CAP_DAC_OVERRIDE walks into the 0o444 directory, the walled-off file stays readable, and the count assertion sees two samples instead of one. It now carries the same needs_unprivileged_posix gate as the three new permission tests. miner._read_text_no_follow: comment fix only. F_SETLEASE on a FIFO fails EINVAL, not ENXIO — measured on Linux 6.18 / glibc 2.39. The code branches on EAGAIN and is unaffected.
jjcav84
pushed a commit
to jjcav84/mempalace
that referenced
this pull request
Aug 22, 2026
…mPalace#2221) The MemPalace#2221 fix reached develop through MemPalace#2228 at an earlier revision of the branch, so three guards and their tests did not come with it. One of the three is a regression the gate that did land introduced. sweep_directory: the new gate probes the file type with f.stat() inside a try, and its except OSError printed SKIP and continued. A dangling symlink, a symlink loop and a file unlinked between rglob and the gate all raise there. Before the gate existed each of them reached sweep(), raised, and was appended to failures — so the gate turned "could not read this transcript" into a silent skip and a successful exit. A probe that FAILS is an error, not a benign file type: log it, print WARNING, book it in failures. A probe that succeeds and reports a non-regular file still skips silently. _parse_gradle: the is_file() gate sat in front of the try whose except OSError the parser already had, 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 moves inside that try. _collect_manifest_names stats with os.path.isfile, which reports instead of raising, matching the parsers it guards. split_file: the type gate in main() covers the files the glob listed, but split_file builds its output names itself, so a pre-existing FIFO at one of them wedged write_text in the kernel waiting for a reader. Output names that are anything but a regular file are skipped. That gate asks os.path.lexists, not os.path.exists. exists() follows the link, so a DANGLING symlink at an output name reads as "nothing there" and the write goes through it, creating the target — a chunk landing wherever the link points rather than in the output directory. Measured: the two calls differ on that one case and agree on every other (regular file, symlink to a file, missing name, FIFO, symlink to FIFO, directory). test_gather_origin_samples_survives_an_unreadable_directory broke under root rather than passing vacuously: CAP_DAC_OVERRIDE walks into the 0o444 directory, the walled-off file stays readable, and the count assertion sees two samples instead of one. It now carries the same needs_unprivileged_posix gate as the three new permission tests. miner._read_text_no_follow: comment fix only. F_SETLEASE on a FIFO fails EINVAL, not ENXIO — measured on Linux 6.18 / glibc 2.39. The code branches on EAGAIN and is unaffected.
jjcav84
pushed a commit
to jjcav84/mempalace
that referenced
this pull request
Aug 22, 2026
…mPalace#2221) The MemPalace#2221 fix reached develop through MemPalace#2228 at an earlier revision of the branch, so three guards and their tests did not come with it. One of the three is a regression the gate that did land introduced. sweep_directory: the new gate probes the file type with f.stat() inside a try, and its except OSError printed SKIP and continued. A dangling symlink, a symlink loop and a file unlinked between rglob and the gate all raise there. Before the gate existed each of them reached sweep(), raised, and was appended to failures — so the gate turned "could not read this transcript" into a silent skip and a successful exit. A probe that FAILS is an error, not a benign file type: log it, print WARNING, book it in failures. A probe that succeeds and reports a non-regular file still skips silently. _parse_gradle: the is_file() gate sat in front of the try whose except OSError the parser already had, 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 moves inside that try. _collect_manifest_names stats with os.path.isfile, which reports instead of raising, matching the parsers it guards. split_file: the type gate in main() covers the files the glob listed, but split_file builds its output names itself, so a pre-existing FIFO at one of them wedged write_text in the kernel waiting for a reader. Output names that are anything but a regular file are skipped. That gate asks os.path.lexists, not os.path.exists. exists() follows the link, so a DANGLING symlink at an output name reads as "nothing there" and the write goes through it, creating the target — a chunk landing wherever the link points rather than in the output directory. Measured: the two calls differ on that one case and agree on every other (regular file, symlink to a file, missing name, FIFO, symlink to FIFO, directory). test_gather_origin_samples_survives_an_unreadable_directory broke under root rather than passing vacuously: CAP_DAC_OVERRIDE walks into the 0o444 directory, the walled-off file stays readable, and the count assertion sees two samples instead of one. It now carries the same needs_unprivileged_posix gate as the three new permission tests. miner._read_text_no_follow: comment fix only. F_SETLEASE on a FIFO fails EINVAL, not ENXIO — measured on Linux 6.18 / glibc 2.39. The code branches on EAGAIN and is unaffected.
jjcav84
pushed a commit
to jjcav84/mempalace
that referenced
this pull request
Aug 22, 2026
…mPalace#2221) The MemPalace#2221 fix reached develop through MemPalace#2228 at an earlier revision of the branch, so three guards and their tests did not come with it. One of the three is a regression the gate that did land introduced. sweep_directory: the new gate probes the file type with f.stat() inside a try, and its except OSError printed SKIP and continued. A dangling symlink, a symlink loop and a file unlinked between rglob and the gate all raise there. Before the gate existed each of them reached sweep(), raised, and was appended to failures — so the gate turned "could not read this transcript" into a silent skip and a successful exit. A probe that FAILS is an error, not a benign file type: log it, print WARNING, book it in failures. A probe that succeeds and reports a non-regular file still skips silently. _parse_gradle: the is_file() gate sat in front of the try whose except OSError the parser already had, 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 moves inside that try. _collect_manifest_names stats with os.path.isfile, which reports instead of raising, matching the parsers it guards. split_file: the type gate in main() covers the files the glob listed, but split_file builds its output names itself, so a pre-existing FIFO at one of them wedged write_text in the kernel waiting for a reader. Output names that are anything but a regular file are skipped. That gate asks os.path.lexists, not os.path.exists. exists() follows the link, so a DANGLING symlink at an output name reads as "nothing there" and the write goes through it, creating the target — a chunk landing wherever the link points rather than in the output directory. Measured: the two calls differ on that one case and agree on every other (regular file, symlink to a file, missing name, FIFO, symlink to FIFO, directory). test_gather_origin_samples_survives_an_unreadable_directory broke under root rather than passing vacuously: CAP_DAC_OVERRIDE walks into the 0o444 directory, the walled-off file stays readable, and the count assertion sees two samples instead of one. It now carries the same needs_unprivileged_posix gate as the three new permission tests. miner._read_text_no_follow: comment fix only. F_SETLEASE on a FIFO fails EINVAL, not ENXIO — measured on Linux 6.18 / glibc 2.39. The code branches on EAGAIN and is unaffected.
jjcav84
pushed a commit
to jjcav84/mempalace
that referenced
this pull request
Aug 22, 2026
…mPalace#2221) The MemPalace#2221 fix reached develop through MemPalace#2228 at an earlier revision of the branch, so three guards and their tests did not come with it. One of the three is a regression the gate that did land introduced. sweep_directory: the new gate probes the file type with f.stat() inside a try, and its except OSError printed SKIP and continued. A dangling symlink, a symlink loop and a file unlinked between rglob and the gate all raise there. Before the gate existed each of them reached sweep(), raised, and was appended to failures — so the gate turned "could not read this transcript" into a silent skip and a successful exit. A probe that FAILS is an error, not a benign file type: log it, print WARNING, book it in failures. A probe that succeeds and reports a non-regular file still skips silently. _parse_gradle: the is_file() gate sat in front of the try whose except OSError the parser already had, 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 moves inside that try. _collect_manifest_names stats with os.path.isfile, which reports instead of raising, matching the parsers it guards. split_file: the type gate in main() covers the files the glob listed, but split_file builds its output names itself, so a pre-existing FIFO at one of them wedged write_text in the kernel waiting for a reader. Output names that are anything but a regular file are skipped. That gate asks os.path.lexists, not os.path.exists. exists() follows the link, so a DANGLING symlink at an output name reads as "nothing there" and the write goes through it, creating the target — a chunk landing wherever the link points rather than in the output directory. Measured: the two calls differ on that one case and agree on every other (regular file, symlink to a file, missing name, FIFO, symlink to FIFO, directory). test_gather_origin_samples_survives_an_unreadable_directory broke under root rather than passing vacuously: CAP_DAC_OVERRIDE walks into the 0o444 directory, the walled-off file stays readable, and the count assertion sees two samples instead of one. It now carries the same needs_unprivileged_posix gate as the three new permission tests. miner._read_text_no_follow: comment fix only. F_SETLEASE on a FIFO fails EINVAL, not ENXIO — measured on Linux 6.18 / glibc 2.39. The code branches on EAGAIN and is unaffected.
jjcav84
pushed a commit
to jjcav84/mempalace
that referenced
this pull request
Aug 22, 2026
…mPalace#2221) The MemPalace#2221 fix reached develop through MemPalace#2228 at an earlier revision of the branch, so three guards and their tests did not come with it. One of the three is a regression the gate that did land introduced. sweep_directory: the new gate probes the file type with f.stat() inside a try, and its except OSError printed SKIP and continued. A dangling symlink, a symlink loop and a file unlinked between rglob and the gate all raise there. Before the gate existed each of them reached sweep(), raised, and was appended to failures — so the gate turned "could not read this transcript" into a silent skip and a successful exit. A probe that FAILS is an error, not a benign file type: log it, print WARNING, book it in failures. A probe that succeeds and reports a non-regular file still skips silently. _parse_gradle: the is_file() gate sat in front of the try whose except OSError the parser already had, 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 moves inside that try. _collect_manifest_names stats with os.path.isfile, which reports instead of raising, matching the parsers it guards. split_file: the type gate in main() covers the files the glob listed, but split_file builds its output names itself, so a pre-existing FIFO at one of them wedged write_text in the kernel waiting for a reader. Output names that are anything but a regular file are skipped. That gate asks os.path.lexists, not os.path.exists. exists() follows the link, so a DANGLING symlink at an output name reads as "nothing there" and the write goes through it, creating the target — a chunk landing wherever the link points rather than in the output directory. Measured: the two calls differ on that one case and agree on every other (regular file, symlink to a file, missing name, FIFO, symlink to FIFO, directory). test_gather_origin_samples_survives_an_unreadable_directory broke under root rather than passing vacuously: CAP_DAC_OVERRIDE walks into the 0o444 directory, the walled-off file stays readable, and the count assertion sees two samples instead of one. It now carries the same needs_unprivileged_posix gate as the three new permission tests. miner._read_text_no_follow: comment fix only. F_SETLEASE on a FIFO fails EINVAL, not ENXIO — measured on Linux 6.18 / glibc 2.39. The code branches on EAGAIN and is unaffected.
jjcav84
pushed a commit
to jjcav84/mempalace
that referenced
this pull request
Aug 22, 2026
…mPalace#2221) The MemPalace#2221 fix reached develop through MemPalace#2228 at an earlier revision of the branch, so three guards and their tests did not come with it. One of the three is a regression the gate that did land introduced. sweep_directory: the new gate probes the file type with f.stat() inside a try, and its except OSError printed SKIP and continued. A dangling symlink, a symlink loop and a file unlinked between rglob and the gate all raise there. Before the gate existed each of them reached sweep(), raised, and was appended to failures — so the gate turned "could not read this transcript" into a silent skip and a successful exit. A probe that FAILS is an error, not a benign file type: log it, print WARNING, book it in failures. A probe that succeeds and reports a non-regular file still skips silently. _parse_gradle: the is_file() gate sat in front of the try whose except OSError the parser already had, 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 moves inside that try. _collect_manifest_names stats with os.path.isfile, which reports instead of raising, matching the parsers it guards. split_file: the type gate in main() covers the files the glob listed, but split_file builds its output names itself, so a pre-existing FIFO at one of them wedged write_text in the kernel waiting for a reader. Output names that are anything but a regular file are skipped. That gate asks os.path.lexists, not os.path.exists. exists() follows the link, so a DANGLING symlink at an output name reads as "nothing there" and the write goes through it, creating the target — a chunk landing wherever the link points rather than in the output directory. Measured: the two calls differ on that one case and agree on every other (regular file, symlink to a file, missing name, FIFO, symlink to FIFO, directory). test_gather_origin_samples_survives_an_unreadable_directory broke under root rather than passing vacuously: CAP_DAC_OVERRIDE walks into the 0o444 directory, the walled-off file stays readable, and the count assertion sees two samples instead of one. It now carries the same needs_unprivileged_posix gate as the three new permission tests. miner._read_text_no_follow: comment fix only. F_SETLEASE on a FIFO fails EINVAL, not ENXIO — measured on Linux 6.18 / glibc 2.39. The code branches on EAGAIN and is unaffected.
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Fast post-3.7.0 integrity patch for hang forever, silent incomplete mine, and multi-process HNSW rewind.
chunk_total, same-fstat mtime, purge abort, closet purge, partial-batch cleanupchunk_total, partial cleanup,prefetch_mined_setcompleteness)sys.exitso atexit releases the palace writer leaseCredits
Integrates and stacks contributor work from #2223 (mvalentsev), #2088 (KeilerHirsch), #2151 (zrh805), #2026 (colorpanda82), #2032.
Test plan
tests/test_non_regular_file_guards.py(FIFO / non-regular)chunk_total, incomplete multi-batch, partial upsert cleanupchunk_total, partial cleanup, prefetch omits incomplete groupsSystemExit(0)ruff check/ruff formaton touched filesOut of scope (3.8 / follow-up)
mempalace repairsegfaults (SIGSEGV / exit 139) on a large palace, leaving vector search permanently disabled #2113)