emrg: the memory reader refuses a subject that is not a regular file

emrg/memory.py

@@ -30,6 +30,13 @@
 from pathlib import Path
 from typing import ClassVar, Optional
 
+# The "is this path a regular file" rule is the tool layer's (`special_file_kind`, a
+# whitelist of `S_ISREG`), reached at a path rather than at a mode. Imported rather than
+# restated: the boundary is one fact, and a second spelling of it is a second boundary
+# that can drift. `emrg.tools.base` imports only `emrg.server.tool_types`, so this adds
+# no cycle — measured by importing this module from a fresh interpreter.
+from emrg.tools.base import non_regular_kind
+
 logger = logging.getLogger(__name__)
 
 # ── Helpers ────────────────────────────────────────────────────────
@@ -242,6 +249,24 @@ def is_index_row(line: str) -> bool:
 # accepts any `## <valid type>` heading; this only decides where new rows file.
 INDEX_TYPE_ORDER = ("user", "feedback", "project", "reference", "decision", "task")
 
+
+class NotARegularFile(OSError):
+    """A memory path whose kind rules out reading it, raised instead of opened.
+
+    An **open is not a read**: on a FIFO it blocks until a writer appears, on a socket
+    until a connection does, and a device never ends. ``from_file`` opened its subject
+    without asking, and its callers' ``except (OSError, ValueError)`` cannot bound the
+    consequence — nothing is raised, the call simply never returns, and it runs inside a
+    coroutine the daemon awaits on its event loop, so the block is the whole daemon.
+
+    An ``OSError`` on purpose: that is what every caller of ``from_file`` already handles,
+    and it is what a *directory* already produced here (``IsADirectoryError``), which is
+    the same answer one kind over. Naming the kind in the class rather than only in the
+    message matters because `_scan` renders ``type(exc).__name__`` beside the file, and
+    ``OSError`` there would name nothing.
+    """
+
+
 # ── MemoryFile ─────────────────────────────────────────────────────
 
 
@@ -300,9 +325,32 @@ def filename(self) -> str:
 
     @classmethod
     def from_file(cls, path: Path) -> MemoryFile:
-        """Parse a memory file from disk."""
+        """Parse a memory file from disk.
+
+        The subject's **kind** is asked before it is opened, because opening it is what
+        decides whether this call returns at all (`NotARegularFile`). Measured
+        2026-10-11 on master `63ee3a54`, one fixture (a named pipe beside a regular
+        memory), each call in its own process under a parent's 8 s cap:
+        ``ProjectMemoryStore.list()`` **did not return**, while the same fixture without
+        the pipe returned in 0.00 s. Two client frames walk this reader — `list_memories`
+        and `read_memory` — and the directory is the agent's own workspace, so anything
+        running there can put a pipe in it.
+
+        Every reader in this module comes through here, which is why the guard is one
+        site rather than one per walker: `_scan`, `list`, `_rebuild_index`, `update`,
+        `merge`, `promote_to_project`, `get_by_filename`, `_resolve_filename`.
+        """
         if not path.exists():
             raise FileNotFoundError(f"Memory file not found: {path}")
+        kind = non_regular_kind(path)
+        if kind:
+            raise NotARegularFile(
+                f"{path} is {kind}, not a regular file. This reader opens its subject "
+                f"directly, and opening this one blocks until a peer appears — a FIFO "
+                f"waits for a writer, a socket for a connection, a device never ends — "
+                f"so the call would never return, and neither would the daemon, which "
+                f"runs its readers on its event loop."
+            )
         content = path.read_text(encoding="utf-8")
         return cls.from_text(content, _filename=path.name)
 

emrg/tools/base.py

@@ -7,6 +7,7 @@
 from __future__ import annotations
 
 import math
+import os
 import stat
 from abc import ABC, abstractmethod
 
@@ -171,6 +172,26 @@ def special_file_kind(mode: int) -> str | None:
     return "not a regular file"
 
 
+def non_regular_kind(path: str | os.PathLike[str]) -> str | None:
+    """Name a path's kind when it is not a regular file — :func:`special_file_kind` at a path.
+
+    ``special_file_kind`` reads a ``st_mode`` its caller already has, which is the shape a
+    *tool* wants: it stats its own subject to describe it to the caller. A reader that was
+    handed a **path** has no mode, and two of those exist outside the tool layer — the
+    daemon's own readers (`#2073`) and the memory store's (`from_file`). Both need the same
+    whitelist, so it is stated once and reached two ways rather than restated per layer.
+
+    ``os.stat`` follows a symlink, so a link to a pipe is a pipe: what the caller is about
+    to open is what is judged, not how it was spelled.
+
+    :param path: the path a reader was handed.
+    :returns: the noun phrase for a non-regular subject, or ``None`` when it is a regular
+        file. A missing path raises ``FileNotFoundError`` from the ``stat`` — the answer
+        callers of a missing file already handle.
+    """
+    return special_file_kind(os.stat(path).st_mode)
+
+
 def special_file_refusal(path: object, kind: str) -> str:
     """The one refusal the file tools give a subject that is not a regular file.
 

tests/test_base_tool.py

@@ -1,10 +1,11 @@
 """Tests for the tool base class."""
 
+import os
 import stat
 
 import pytest
 
-from emrg.tools.base import ToolExecutor, special_file_kind
+from emrg.tools.base import ToolExecutor, non_regular_kind, special_file_kind
 
 
 def test_tool_executor_is_abstract():
@@ -72,3 +73,45 @@ def test_special_file_kind_ignores_permission_bits():
     """Only the type bits decide; a read-only or world-writable regular file is regular."""
     for perms in (0o000, 0o400, 0o444, 0o777):
         assert special_file_kind(stat.S_IFREG | perms) is None
+
+
+# ── `non_regular_kind`: the same rule, at a path ──
+#
+# A tool holds its subject's mode already; a *reader* handed a path does not. The memory
+# store's reader (issue #2075) and the daemon's (`#2073`) are both readers, so the
+# whitelist is reached at a path rather than restated per layer. These pin the stat's
+# fields of view — what it follows, and what it raises for a path that is not there.
+
+
+def test_non_regular_kind_is_none_for_a_regular_file(tmp_path):
+    """The ordinary subject: the caller opens it, and `None` is the licence to."""
+    regular = tmp_path / "notes.md"
+    regular.write_text("hello\n", encoding="utf-8")
+    assert non_regular_kind(regular) is None
+
+
+def test_non_regular_kind_names_a_directory(tmp_path):
+    """A mode built from `S_IF*` is not needed here — a real directory carries one."""
+    assert non_regular_kind(tmp_path) == "a directory"
+
+
+def test_non_regular_kind_raises_for_a_path_that_is_not_there(tmp_path):
+    """Missing is the callers' other answer, and it stays an exception rather than a noun."""
+    with pytest.raises(FileNotFoundError):
+        non_regular_kind(tmp_path / "absent.md")
+
+
+@pytest.mark.skipif(not hasattr(os, "mkfifo"), reason="this platform has no FIFOs")
+def test_non_regular_kind_judges_a_symlink_by_what_will_be_opened(tmp_path):
+    """`os.stat` follows the link, so a link to a pipe is a pipe.
+
+    Stated because the decision is the stat's and not the spelling's: opening the link
+    opens the pipe, so a predicate that judged the link itself would license the open it
+    exists to refuse.
+    """
+    fifo = tmp_path / "pipe"
+    os.mkfifo(fifo)
+    link = tmp_path / "link.md"
+    link.symlink_to(fifo)
+    assert non_regular_kind(link) == "a FIFO (named pipe)"
+    assert non_regular_kind(fifo) == "a FIFO (named pipe)"

tests/test_daemon.py

@@ -4310,3 +4310,60 @@ async def stream(messages, tools=None):
         "a genuinely completed round no longer stamps the marker the #1114 alarm "
         "measures — the empty-answer guard must not have swallowed it"
     )
+
+
+# ── a memory subject that is not a regular file ────────────────────
+#
+# The sibling of the section above, one layer down: `list_memories` and `read_memory`
+# walk `<cwd>/.emrg/memory` through `MemoryStore`, whose single reader used to open
+# whatever it was handed. A pipe there is not a slow request — `open` on a FIFO never
+# returns, the walk is synchronous inside the handler, and the handler is awaited on
+# the event loop, so the frame that never gets its answer is the daemon that never
+# serves another (issue #2075). The guard is `emrg.memory.MemoryFile.from_file`.
+#
+# Driving the dispatch rather than the store is the point: the store tolerates one
+# unreadable file by design, and what has to be proved is that the *frame* arrives.
+
+
+@pytest.mark.skipif(not hasattr(os, "mkfifo"), reason="this platform has no FIFOs")
+def test_a_memory_that_is_a_named_pipe_costs_the_row_not_the_listing(tmp_path):
+    """A pipe in the memory directory is skipped; the readable rows still arrive."""
+    server = _make_server()
+    directory = _memory_project(tmp_path, b"# Memory Index\n")
+    os.mkfifo(directory / "pipe.md")
+    (directory / "note.md").write_text(
+        "---\nid: aaaa1111\ntype: reference\nscope: project\nstatus: active\n"
+        "title: A note\n---\n\nbody\n",
+        encoding="utf-8",
+    )
+
+    frames = _drive(server, {"type": "list_memories", "scope": "project", "cwd": str(tmp_path)})
+
+    assert len(frames) == 1, f"expected one frame, got {frames!r}"
+    frame = frames[0]
+    assert frame["type"] == "memories_list"
+    assert [m["id"] for m in frame["memories"]] == ["aaaa1111"], (
+        "the pipe is not a memory, and it must not cost the memory beside it"
+    )
+
+
+@pytest.mark.skipif(not hasattr(os, "mkfifo"), reason="this platform has no FIFOs")
+def test_a_read_memory_through_a_pipe_directory_answers_rather_than_hangs(tmp_path):
+    """`read_memory` walks the same directory to match an id; the answer must come back.
+
+    The reason names the file and its kind, because "Memory not found" would be a claim
+    this walk never measured — the pipe is a file the walk could not finish looking at.
+    """
+    server = _make_server()
+    directory = _memory_project(tmp_path, b"# Memory Index\n")
+    os.mkfifo(directory / "pipe.md")
+
+    frames = _drive(
+        server,
+        {"type": "read_memory", "scope": "project", "memory_id": "aaaa1111", "cwd": str(tmp_path)},
+    )
+
+    assert len(frames) == 1, f"expected one frame, got {frames!r}"
+    assert "error" in frames[0], frames[0]
+    assert "pipe.md" in frames[0]["error"], frames[0]["error"]
+    assert "NotARegularFile" in frames[0]["error"], frames[0]["error"]

tests/test_memory.py

@@ -1,5 +1,6 @@
 """Tests for the EMRG memory module."""
 
+import os
 import tempfile
 from pathlib import Path
 
@@ -10,6 +11,7 @@
     INDEX_TITLE_MAX_CHARS,
     MemoryFile,
     MemoryIndex,
+    NotARegularFile,
     ProjectMemoryStore,
     SessionMemoryStore,
     generate_id,
@@ -769,3 +771,95 @@ def test_the_filename_field_stays_out_of_the_file_format(self, project_store):
         reloaded = MemoryFile.from_file(project_store.directory / mem.filename)
         assert reloaded._filename == mem.filename
         assert reloaded.to_markdown() == text, "a load → save rewrote the file"
+
+
+# ── the subject has to be a regular file ──────────────────────────
+#
+# An open is not a read: on a FIFO a read blocks until a writer appears, on a socket
+# until a connection does, and a device never ends. The three file tools ask
+# `special_file_kind` before opening (#2062) and the daemon's own readers took the same
+# guard (#2073). This is the third layer, and it is the memory module's single reader:
+# fourteen call sites, of which `list_memories` and `read_memory` are the two client
+# frames — and the directory they walk is `<session cwd>/.emrg/memory`, inside the
+# agent's own workspace, so anything running there can put a pipe in it.
+#
+# Each test is written so that removing the guard makes it **hang** rather than pass:
+# the condition is a call that never returns, and no assertion can bound that (the shape
+# `tests/test_read_tool.py` states for the same class). Windows has no `mkfifo`, so the
+# family skips there.
+
+
+def _needs_mkfifo(func):
+    return pytest.mark.skipif(
+        not hasattr(os, "mkfifo"), reason="this platform has no FIFOs"
+    )(func)
+
+
+@_needs_mkfifo
+def test_the_reader_refuses_a_named_pipe_instead_of_blocking(tmp_path):
+    """`from_file` names the kind rather than opening the subject."""
+    fifo = tmp_path / "notes.md"
+    os.mkfifo(fifo)
+
+    with pytest.raises(NotARegularFile) as excinfo:
+        MemoryFile.from_file(fifo)
+
+    assert "a FIFO (named pipe)" in str(excinfo.value), str(excinfo.value)
+    assert "not a regular file" in str(excinfo.value), str(excinfo.value)
+
+
+@_needs_mkfifo
+def test_the_reader_still_reads_a_regular_file_beside_a_pipe(tmp_path):
+    """The control: the refusal is about the kind, not about the directory holding it."""
+    os.mkfifo(tmp_path / "pipe.md")
+    regular = tmp_path / "notes.md"
+    regular.write_text(
+        "---\nid: aaaa1111\ntype: reference\nscope: project\nstatus: active\n"
+        "title: A note\n---\n\nbody\n",
+        encoding="utf-8",
+    )
+
+    assert MemoryFile.from_file(regular).id == "aaaa1111"
+
+
+@_needs_mkfifo
+def test_the_store_lists_the_memories_beside_a_pipe(temp_cwd):
+    """The measured route: `list_memories` walks every `*.md`, so a pipe in the
+    directory is a client frame that never gets its answer.
+
+    Asserted through `ProjectMemoryStore.list()` rather than through `from_file`,
+    because the walk's `except (OSError, ValueError)` is what turns the refusal into a
+    skipped file — a width the reader alone does not decide.
+    """
+    directory = temp_cwd / ".emrg" / "memory"
+    directory.mkdir(parents=True)
+    (directory / "MEMORY.md").write_text("# Memory Index\n", encoding="utf-8")
+    os.mkfifo(directory / "pipe.md")
+    store = ProjectMemoryStore(temp_cwd)
+    store.create("reference", "A note", "body")
+
+    assert [m.title for m in store.list()] == ["A note"], (
+        "the pipe is not a memory and the readable sibling still is"
+    )
+
+
+@_needs_mkfifo
+def test_the_scan_names_the_pipe_as_a_reason_rather_than_never_answering(temp_cwd):
+    """`read_memory`'s route: `_scan` walks the same directory to match an id.
+
+    An id that is not there must still come back as an answer, and the pipe must be
+    reported as the reason the walk could not finish — "not found" would be a claim this
+    walk did not measure.
+    """
+    directory = temp_cwd / ".emrg" / "memory"
+    directory.mkdir(parents=True)
+    os.mkfifo(directory / "pipe.md")
+    store = ProjectMemoryStore(temp_cwd)
+
+    mem, reason = store.get_with_reason("aaaa1111")
+
+    assert mem is None
+    assert "pipe.md" in reason, reason
+    assert "NotARegularFile" in reason, (
+        "the reason must name the kind's verdict — `OSError` there would name nothing"
+    )