diff --git a/emrg/server/daemon.py b/emrg/server/daemon.py index 55fa680f..59e948d4 100644 --- a/emrg/server/daemon.py +++ b/emrg/server/daemon.py @@ -409,6 +409,27 @@ def _non_regular_kind(path) -> str | None: return None +def _refuse_non_regular_file(path) -> None: + """Raise, for a subject that cannot be read to an end, the ``OSError`` callers already answer. + + The predicate above answers *what* the subject is; this states *what happens next* at a + site whose own ``except OSError`` already answers a file it cannot read — the refusal is + an ``OSError``, so it lands in that handler and the reading the caller gets is the one + its contract already promised for an unreadable file. Written once because the sites + that need it are one shape: no new branch at any of them, only the answer that + currently never arrives (`#2062`: a FIFO blocks at the open and raises nothing). + + A missing subject does **not** raise: :func:`_non_regular_kind` answers ``None`` when + its ``stat`` fails, because absence is a case every caller here already has a branch + for, and a writer among them must keep reaching it. + + :param path: the file about to be opened. + """ + kind = _non_regular_kind(path) + if kind is not None: + raise OSError(f"{path} is {kind}, not a regular file") + + def _unreadable_index_notice(path, exc: BaseException, *, where: str) -> str: """The sentence both carriers of a memory index use when the file cannot be read. @@ -1074,7 +1095,19 @@ def _rebuild_sessions_index(self) -> None: try: project_paths: list[str] = [] - if self._projects_log.exists(): + kind = _non_regular_kind(self._projects_log) + if kind is not None: + # Not opened, and **said so** rather than caught and dropped: the reader + # below answers a registry it cannot read with no paths, and the + # difference between "the file is not there" and "the file cannot be + # read to an end" is the whole point of asking the kind (`#2062`). A FIFO + # here would hold this startup task — and the event loop with it — for + # ever, before any client can connect. + logger.debug( + "sessions index backfill: %s is %s, not a regular file; not opened", + self._projects_log, kind, + ) + elif self._projects_log.exists(): try: data = yaml.safe_load(self._projects_log.read_text(encoding="utf-8")) if isinstance(data, list): @@ -1904,6 +1937,20 @@ def _touch_project(self, cwd: str) -> None: # Read existing entries (normalize by realpath to avoid duplicates) projects: dict[str, dict] = {} + kind = _non_regular_kind(self._projects_log) + if kind is not None: + # Skipped, and skipped **before** the read rather than inside the `except` + # below: this method reads the registry and then rewrites it, so falling + # through to the "rebuilding" branch would answer a host file it could not + # read with an **empty** registry — deleting every project the host + # registered, in one write, to report a problem it never mentioned. + # Registration is bookkeeping, so the honest reading is "not this time": + # the file keeps what it had and the warning names the kind. + logger.warning( + "_touch_project: %s is %s, not a regular file — leaving the project " + "registry unread and unrewritten", self._projects_log, kind, + ) + return if self._projects_log.exists(): try: data = yaml.safe_load(self._projects_log.read_text(encoding="utf-8")) @@ -2477,13 +2524,11 @@ def _cap_memory_index(self, path) -> str: # only reader. Both scopes are measured in # `tests/test_memory_index_thresholds.py`. limit = INDEX_SIZE_WARN - kind = _non_regular_kind(path) - if kind is not None: - # Raised, not answered: this is a pure function whose one production caller - # (`_index_for_prompt`) already turns a read it cannot do into the notice - # the prompt carries, so the condition reaches the agent with its carrier - # named — the same shape as an index that is not valid UTF-8. - raise OSError(f"{path} is {kind}, not a regular file") + # Raised, not answered: this is a pure function whose one production caller + # (`_index_for_prompt`) already turns a read it cannot do into the notice + # the prompt carries, so the condition reaches the agent with its carrier + # named — the same shape as an index that is not valid UTF-8. + _refuse_non_regular_file(path) text = path.read_text(encoding="utf-8") if len(text) <= limit: return text @@ -2643,11 +2688,9 @@ def _index_for_frame(self, path) -> str: try: if not path.exists(): return "" - kind = _non_regular_kind(path) - if kind is not None: - # Same condition, same answer as an index that cannot be decoded: the - # frame keeps its rows and the `index` field carries the notice. - raise OSError(f"{path} is {kind}, not a regular file") + # Same condition, same answer as an index that cannot be decoded: the + # frame keeps its rows and the `index` field carries the notice. + _refuse_non_regular_file(path) return path.read_text(encoding="utf-8") except (OSError, UnicodeDecodeError) as exc: logger.debug("memory index could not be read: %s", path, exc_info=True) @@ -6059,6 +6102,11 @@ async def _handle_list_projects( projects: list[dict] = [] try: if self._projects_log.exists(): + # Every client asks this on connect, so an unguarded open here is the + # daemon's event loop stopping on the first frame a GUI or TUI sends — + # the refusal lands in the `except` below, which is already this + # method's answer for a registry it cannot read (`#2062`). + _refuse_non_regular_file(self._projects_log) data = yaml.safe_load(self._projects_log.read_text(encoding="utf-8")) if isinstance(data, list): entries = [p for p in data if isinstance(p, dict)] @@ -6118,6 +6166,10 @@ async def _handle_remove_project(self, name: str, ws) -> None: }) return try: + # The same refusal as `_handle_list_projects`, and this caller already has + # the branch for it: an unreadable registry is answered with `removed: + # false` and the reason, so a FIFO cannot hold the event loop (`#2062`). + _refuse_non_regular_file(self._projects_log) data = yaml.safe_load(self._projects_log.read_text(encoding="utf-8")) except (yaml.YAMLError, OSError): logger.exception("remove_project: failed to read %s", self._projects_log) diff --git a/tests/test_daemon.py b/tests/test_daemon.py index aa547187..4abd8dcf 100644 --- a/tests/test_daemon.py +++ b/tests/test_daemon.py @@ -30,6 +30,7 @@ from emrg.server.scheduler import TaskHandler, TaskScheduler from emrg.tools import base as base_mod from tests.task_handler_factory import make_handler +from tests.bounded_read import DEADLINE_SECONDS, within from emrg.session import Session, _validate_tool_messages @@ -4820,6 +4821,173 @@ def test_the_rant_panel_survives_a_legacy_array_row_and_a_bare_scalar(tmp_path): assert frames[0]["rants"][0]["project"] == "emrg", frames[0] +# ── projects.yml: the registry every client reads first ───────────── +# +# These are the daemon's four readers of one file, `~/.emrg/projects.yml`, and each of +# them runs on the daemon's **event loop**: the startup backfill, the per-message project +# touch, `list_projects` — which every client sends on connect, so it is the first frame +# a GUI or TUI ever gets an answer to — and `remove_project`. A FIFO at that path +# therefore does not fail a request, it stops the daemon answering anyone again, which is +# the condition issue #2062 named for the three file tools and #2073 for the daemon's own +# readers. Measured 2026-10-11 (`cyc20261011-173122`) on master `60541e39`: each of the +# four calls below **did not return** under a FIFO, while the pipe-free controls ran. +# +# Two shapes, and the difference is deliberate: a **reader** answers the refusal through +# the `except OSError` it already has for a registry it cannot read (so no new branch is +# added), while `_touch_project` — which reads the registry and then **rewrites** it — +# skips the registration instead of falling through to its "rebuilding" branch, because +# that branch would answer a host file it could not read with an *empty* registry and +# delete every project the host had registered. +# +# Every arm is bounded by `within`, so a removed guard is a **5 s failure** rather than a +# wedged pytest run (the `no_verdict` trap `tests/bounded_read.py` states). The FIFO arms +# are skipped where `mkfifo` does not exist, so each site also carries a pipe-free arm in +# the same shape — a **directory** — which the Windows leg runs: an unguarded read answers +# a directory with `IsADirectoryError`'s own message, so asserting the refusal *names the +# kind* is what makes these discriminate rather than merely pass. + + +@_needs_mkfifo +def test_a_named_pipe_registry_does_not_stop_the_project_listing(tmp_path, monkeypatch): + """The first frame every client sends: answered, not blocked on the open.""" + server = _make_server() + fifo = tmp_path / "projects.yml" + os.mkfifo(fifo) + monkeypatch.setattr(server, "_projects_log", fifo) + writer = _FakeWriter() + + within(DEADLINE_SECONDS, lambda: asyncio.run(server._handle_list_projects(writer))) + + replies = _decode_frames(writer) + assert len(replies) == 1 and replies[0]["type"] == "projects_list", replies + assert replies[0]["projects"] == [], replies + + +def test_a_directory_registry_is_refused_by_its_kind_in_the_listing( + tmp_path, monkeypatch, caplog): + """The pipe-free half: the refusal names the kind, which is what discriminates.""" + server = _make_server() + registry = tmp_path / "projects.yml" + registry.mkdir() + monkeypatch.setattr(server, "_projects_log", registry) + writer = _FakeWriter() + + with caplog.at_level(logging.ERROR): + asyncio.run(server._handle_list_projects(writer)) + + assert _decode_frames(writer)[0]["projects"] == [] + assert "not a regular file" in caplog.text, ( + "the read was refused by its kind — an unguarded one reports the raw " + f"directory error instead: {caplog.text}") + + +@_needs_mkfifo +def test_a_named_pipe_registry_is_neither_read_nor_rewritten_by_touch_project( + tmp_path, monkeypatch): + """The read-then-write site: skipped, and the registry is left as it was.""" + import stat + + server = _make_server() + fifo = tmp_path / "projects.yml" + os.mkfifo(fifo) + monkeypatch.setattr(server, "_projects_log", fifo) + + within(DEADLINE_SECONDS, lambda: server._touch_project(str(tmp_path))) + + assert stat.S_ISFIFO(os.stat(fifo).st_mode), ( + "the registry was rewritten — a read that answered 'no entries' would have " + "deleted every project the host registered") + + +def test_a_directory_registry_warns_touch_project_and_keeps_it( + tmp_path, monkeypatch, caplog): + """The pipe-free half: the warning names the kind, and no write replaces it.""" + server = _make_server() + registry = tmp_path / "projects.yml" + registry.mkdir() + monkeypatch.setattr(server, "_projects_log", registry) + + with caplog.at_level(logging.WARNING): + server._touch_project(str(tmp_path)) + + assert registry.is_dir(), "the registry was replaced by the write this guard prevents" + assert "not a regular file" in caplog.text, ( + f"the read was refused by its kind, and the warning says so: {caplog.text}") + + +@_needs_mkfifo +def test_a_named_pipe_registry_does_not_stop_the_index_backfill(tmp_path, monkeypatch): + """Startup: the backfill runs with no project paths rather than never returning.""" + from emrg import sessions_index as sessions_index_mod + + server = _make_server() + fifo = tmp_path / "projects.yml" + os.mkfifo(fifo) + monkeypatch.setattr(server, "_projects_log", fifo) + seen: dict[str, list[str]] = {} + monkeypatch.setattr( + sessions_index_mod, "rebuild_sessions_index", + lambda cfg, paths: (seen.update(paths=list(paths)), 0)[1], + ) + + within(DEADLINE_SECONDS, server._rebuild_sessions_index) + + assert seen.get("paths") == [], ( + "the backfill read a registry it could not read instead of skipping it") + + +def test_a_directory_registry_is_refused_by_its_kind_in_the_backfill( + tmp_path, monkeypatch, caplog): + """The pipe-free half, read through the log: the skip names the kind.""" + from emrg import sessions_index as sessions_index_mod + + server = _make_server() + registry = tmp_path / "projects.yml" + registry.mkdir() + monkeypatch.setattr(server, "_projects_log", registry) + monkeypatch.setattr( + sessions_index_mod, "rebuild_sessions_index", lambda cfg, paths: 0) + + with caplog.at_level(logging.DEBUG): + server._rebuild_sessions_index() + + assert "not a regular file" in caplog.text, caplog.text + + +@_needs_mkfifo +def test_a_named_pipe_registry_does_not_stop_remove_project(tmp_path): + """The removal answers `removed: false` with its reason, as for a corrupt file.""" + server = _make_server() + fifo = tmp_path / "projects.yml" + os.mkfifo(fifo) + server._projects_log = fifo + writer = _FakeWriter() + + within(DEADLINE_SECONDS, + lambda: asyncio.run(server._handle_remove_project("alpha", writer))) + + replies = _decode_frames(writer) + assert replies and replies[0]["type"] == "project_removed", replies + assert replies[0]["removed"] is False, replies + assert "failed to read" in replies[0].get("error", ""), replies + + +def test_a_directory_registry_is_refused_by_its_kind_in_remove_project( + tmp_path, monkeypatch, caplog): + """The pipe-free half: the same refusal, observed through the log.""" + server = _make_server() + registry = tmp_path / "projects.yml" + registry.mkdir() + monkeypatch.setattr(server, "_projects_log", registry) + writer = _FakeWriter() + + with caplog.at_level(logging.ERROR): + asyncio.run(server._handle_remove_project("alpha", writer)) + + assert _decode_frames(writer)[0]["removed"] is False + assert "not a regular file" in caplog.text, caplog.text + + # ── the daemon's own `~/.emrg/logs` files ──────────────────────────────────────────── #