emrg: the skills registry asks the kind before opening the catalog and the state file

emrg/skills/registry.py

@@ -27,6 +27,11 @@
 
 from emrg.config import config_dir
 
+# The "is this path a regular file" rule is the tool layer's, reached at a path. Imported
+# rather than restated, for the reason `emrg/memory.py` gives at the same import: the
+# boundary is one fact, and a second spelling of it is a second boundary that can drift.
+from emrg.tools.base import non_regular_kind
+
 logger = logging.getLogger(__name__)
 
 # Canonical baseline — must match emrg/skills/skill-catalog.md
@@ -159,9 +164,35 @@ def _strip_quotes(value: str) -> str:
 
 
 def load_catalog_skills() -> list[dict]:
-    """Read the catalog's installable-skill list (empty when missing)."""
+    """Read the catalog's installable-skill list (empty when missing).
+
+    The subject's **kind** is asked before it is opened, because opening it is what decides
+    whether this call returns at all: `read_text` on a FIFO blocks at the *open* until a
+    writer appears, so the `except OSError` below cannot bound the consequence — nothing is
+    raised and the call simply never returns. This runs on the daemon's event loop (the
+    `skills_available` and `skills_install` handlers), so the block is the whole server.
+
+    Measured 2026-10-11 (`cyc20261011-100220`): a `mkfifo` at `~/.emrg/skills/skill-catalog.md`
+    made this function **not return in 8 s**, where the same fixture with a regular file
+    returned in 0.00 s.
+
+    The path is the directory this instance installs skills into, and `load_skills` already
+    kind-checks the `*.md` files beside it (#2084) — this file sits in the same directory and
+    was the one left unguarded.
+
+    Asked **instead of** the `path.exists()` this function used to lean on, not in addition
+    to it: `exists()` is true for a FIFO, which is exactly the reading that let the block
+    through, so it is removed rather than kept as a second opinion.
+    """
     path = catalog_path()
-    if not path.exists():
+    try:
+        kind = non_regular_kind(path)
+    except OSError:
+        # A missing (or unstattable) path: the same empty answer this function has always
+        # given for one.
+        return []
+    if kind:
+        logger.debug("cannot read %s — it is %s, not a regular file", path, kind)
         return []
     try:
         text = path.read_text(encoding="utf-8")
@@ -182,9 +213,27 @@ def find_catalog_skill(name: str) -> dict | None:
 # ── install/update state (.state.json) ───────────────────────────────
 
 def read_state() -> dict[str, dict]:
-    """Read skill state: {name: {version, installed_at, managed, ...}}."""
+    """Read skill state: {name: {version, installed_at, managed, ...}}.
+
+    Asks the subject's **kind** before opening it, for the reason
+    :func:`load_catalog_skills` states at length: `.state.json` lives in the same directory
+    (`~/.emrg/skills/`), `read_text` on a FIFO blocks at the open and raises nothing, and
+    both this function and that one run on the daemon's event loop. Measured 2026-10-11
+    (`cyc20261011-100220`): a `mkfifo` at `~/.emrg/skills/.state.json` made this **not
+    return in 8 s** where a regular file returned in 0.00 s.
+
+    The `path.exists()` this used to lean on is gone rather than kept: `exists()` is true
+    for a FIFO, and a caller that answers "does it exist" when it means "can I open it" is
+    how the block got through.
+    """
     path = state_path()
-    if not path.exists():
+    try:
+        kind = non_regular_kind(path)
+    except OSError:
+        # Missing or unstattable: the empty mapping this function already answers with.
+        return {}
+    if kind:
+        logger.debug("cannot parse %s — it is %s, not a regular file", path, kind)
         return {}
     try:
         data = json.loads(path.read_text(encoding="utf-8"))

tests/test_skills_registry.py

@@ -16,6 +16,7 @@
 
 import pytest
 
+from tests.bounded_read import DEADLINE_SECONDS, within
 import emrg.skills.installer as installer
 import emrg.skills.registry as registry
 from emrg.skills.loader import _parse_frontmatter, _parse_skill_file, load_skills
@@ -409,3 +410,58 @@ def test_no_state_noop(self, tmp_home, with_cli):
         ensure_catalog_file()
         result = _run(installer.update_managed_skills(runner=FakeRunner(VALID_SKILL_MD), http_get=FakeHttp("v0.1.8")))
         assert result == {"checked": 0, "updated": [], "skipped": [], "errors": []}
+
+
+# ── the catalog and the state file are read by kind, not by opening ──
+#
+# `exists()` is true for a FIFO, which is why it is not a kind: a reader that tested it and
+# then opened its subject had nothing bounding the open, and `read_text` on a FIFO waits at
+# the *open* for a writer and raises nothing — so no handler runs and the call never returns.
+# Both readers below run on the daemon's event loop (`skills_available`, `skills_install`),
+# so the block is the whole server. Measured on master `94dcdae7` (2026-10-11,
+# `cyc20261011-100220`): each FIFO made its reader **not return in 8 s**, where a regular
+# file returned in 0.00 s.
+#
+# The reads are bounded by `tests.bounded_read.within`, the one deadline this suite keeps
+# (issue #2091): the bound is what makes a regression *fail* instead of wedging the run.
+
+
+class TestTheRegistryAsksTheKindBeforeOpening:
+    """A FIFO at either path is skipped, not opened (issue #2093)."""
+
+    @pytest.mark.skipif(not hasattr(os, "mkfifo"), reason="this platform has no FIFOs")
+    def test_a_fifo_catalog_is_skipped_rather_than_opened(self, tmp_home):
+        """Returns the empty answer it gives a missing catalog — and it returns at all."""
+        path = registry.catalog_path()
+        path.parent.mkdir(parents=True, exist_ok=True)
+        os.mkfifo(path)
+        assert within(DEADLINE_SECONDS, load_catalog_skills) == []
+
+    def test_the_control_catalog_without_the_fifo(self, tmp_home):
+        """Same fixture, a regular file: the catalog reads, so the kind is the axis."""
+        ensure_catalog_file()
+        assert [e["name"] for e in load_catalog_skills()] == ["browser-harness"]
+
+    @pytest.mark.skipif(not hasattr(os, "mkfifo"), reason="this platform has no FIFOs")
+    def test_a_fifo_state_file_is_skipped_rather_than_opened(self, tmp_home):
+        path = registry.state_path()
+        path.parent.mkdir(parents=True, exist_ok=True)
+        os.mkfifo(path)
+        assert within(DEADLINE_SECONDS, read_state) == {}
+        # the reader's other caller goes through the same open
+        assert within(DEADLINE_SECONDS, lambda: skill_is_managed("browser-harness")) is False
+
+    def test_the_control_state_file_without_the_fifo(self, tmp_home):
+        write_state({"browser-harness": {"managed": True}})
+        assert read_state() == {"browser-harness": {"managed": True}}
+        assert skill_is_managed("browser-harness") is True
+
+    def test_a_missing_subject_is_still_the_empty_answer(self, tmp_home):
+        """What the kind check must not narrow: absent is empty, not an exception.
+
+        `non_regular_kind` stats its subject and `stat` raises on a missing path, so this
+        contract is the one the fix had to keep while dropping `path.exists()`.
+        """
+        assert load_catalog_skills() == []
+        assert read_state() == {}
+        assert skill_is_managed("anything") is False