emrg: a register at the pre-rename name is read, not read as empty

emrg/config.py

@@ -247,6 +247,41 @@ def agents_file(base: Path | None = None) -> Path:
     return directory / "agents.yml"
 
 
+def register_for_reading(base: Path | None = None) -> Path:
+    """Returns the register a *reader* should open (#2545).
+
+    :func:`agents_file` names where the register belongs; this answers where it
+    actually is. :func:`migrate_legacy_registry` moves a register still at the
+    pre-rename name, and it is documented to leave the file exactly where it is
+    when the move cannot happen — a read-only home, a permission boundary, a
+    concurrent caller that got there first. A checkout also runs ahead of the
+    daemon that still holds the register at the old name. In both states
+    :func:`agents_file` names a file that does not exist, and a reader asking
+    only that one answers an **empty register** while the real one sits one
+    filename away — silently, because "nothing there" and "it is at the other
+    name" print the same.
+
+    The new name governs whenever it exists, so a legacy file that an older
+    daemon re-created cannot shadow the current register. When neither name
+    exists the new one comes back: the absent file the caller would have opened
+    anyway.
+
+    **Why this is not folded into** :func:`agents_file`: the migration takes
+    that function's answer as its move *target*, so a fallback inside it would
+    hand the migration the legacy path as the target, ``target.exists()`` would
+    be true, and the move would answer ``False`` on every host for ever — the
+    register would never reach the new name at all.
+
+    Reading never moves anything: this stats the two names and opens neither.
+    """
+    directory = config_dir() if base is None else base
+    current = agents_file(directory)
+    if current.exists():
+        return current
+    legacy = directory / _LEGACY_AGENTS_NAME
+    return legacy if legacy.exists() else current
+
+
 def migrate_legacy_registry(base: Path | None = None) -> bool:
     """Move a register still at the pre-rename name onto the new one.
 

emrg/server/scheduler.py

@@ -36,7 +36,7 @@
 from pathlib import Path
 import yaml
 
-from emrg.config import agents_file, config_dir
+from emrg.config import agents_file, config_dir, register_for_reading
 from emrg.connect import connect_to_server
 from emrg.memory import NotARegularFile
 from emrg.tools.base import non_regular_kind
@@ -993,8 +993,13 @@ def _resolve_task_template(task_type: str) -> Path:
 
 
 def _resolve_agent_path(name: str) -> str | None:
-    """Resolve an agent name to its path from agents.yml."""
-    register = agents_file(config_dir())
+    """Resolve an agent name to its path from the register.
+
+    The register is opened through :func:`emrg.config.register_for_reading`, so a
+    host whose file is still at the pre-rename name resolves here instead of
+    answering `None` (#2545).
+    """
+    register = register_for_reading(config_dir())
     if not register.exists():
         return None
     try:
@@ -1010,14 +1015,14 @@ def _resolve_agent_path(name: str) -> str | None:
 
 
 def _load_agent_config(name: str, source_dir: str) -> dict:
-    """Return the full agents.yml entry matching the current agent.
+    """Return the full register entry matching the current agent.
 
     Matches by ``name`` first, then by ``path`` against ``source_dir``.
     Returns an empty dict when no match is found. Custom fields added by
     the user in agents.yml are preserved, so templates can reference
     them via ``{{ agent.<field> }}`` without code changes.
     """
-    register = agents_file(config_dir())
+    register = register_for_reading(config_dir())
     if not register.exists():
         return {}
     try:

emrg/tools/file_policy.py

@@ -47,8 +47,9 @@
 
 #: Daemon state files a sandboxed tool call must never write, at any tier. Each is
 #: a file the host owns and the daemon reads at startup. The agent registry is
-#: not spelled here: its path is derived (:func:`emrg.config.agents_file`), so it
-#: joins this list where names become paths.
+#: not spelled here: its path is derived (:func:`emrg.config.register_for_reading`,
+#: which answers whichever name the register currently holds, #2545), so it joins
+#: this list where names become paths.
 PROTECTED_FILES = (
     "~/.emrg/config.toml",
     "~/.emrg/emrgd.token",
@@ -62,10 +63,10 @@ def protected_paths() -> list[str]:
     # Imported at call time, not module scope: `emrg.config` imports
     # `emrg.tools.base`, and this module is reached from the tool packages that
     # sit beside it (#2521).
-    from emrg.config import agents_file
+    from emrg.config import register_for_reading
 
     out: list[str] = []
-    for p in (*PROTECTED_FILES, str(agents_file())):
+    for p in (*PROTECTED_FILES, str(register_for_reading())):
         try:
             out.append(os.path.realpath(os.path.expanduser(p)))
         except OSError:

emrg/tools/submit_rant_tool.py

@@ -316,10 +316,14 @@ def _execute_submit(self, arguments: dict) -> ToolResult:
         )
 
     def _registered_agent_names(self) -> list[str]:
-        """Short names registered in ~/.emrg/agents.yml (config dir).
+        """Short names registered in the register (config dir).
 
         The daemon registers a directory there on first use; the list is the
-        authoritative source for the rant ``agent`` field. Any read error
+        authoritative source for the rant ``agent`` field. The path is asked of
+        :func:`emrg.config.register_for_reading`, which answers the pre-rename
+        name while that is the file that holds the register (#2545) — a reader
+        that asked only for the new name saw an empty register, and the warning
+        below is gated on a non-empty list, so it never fired. Any read error
         degrades to "no candidates" (validation is advisory, never blocking).
 
         The subject's kind is asked before it is opened: `exists()` is true for a FIFO, and
@@ -331,9 +335,9 @@ def _registered_agent_names(self) -> list[str]:
         try:
             import yaml
 
-            from emrg.config import agents_file
+            from emrg.config import register_for_reading
 
-            p = agents_file(self._rants_log().parent)
+            p = register_for_reading(self._rants_log().parent)
             try:
                 kind = non_regular_kind(p)
             except OSError:

tests/test_agents_registry_migration.py

@@ -15,7 +15,7 @@
 import pytest
 import yaml
 
-from emrg.config import agents_file, migrate_legacy_registry
+from emrg.config import agents_file, migrate_legacy_registry, register_for_reading
 
 #: A register with what the daemon cares about: a `name`/`path` pair, and the
 #: `last_active` stamp the pickers sort by. The second entry carries a custom
@@ -193,3 +193,85 @@ def test_the_old_name_is_spelled_only_where_the_move_is():
     assert not offenders, "the register's path is built outside its one producer:\n" + "\n".join(
         offenders
     )
+
+
+# ── The register read path (#2545) ────────────────────────────────────────────
+#
+# The move above is the *writer's* business. These pin the reader's: while the
+# file is still at the old name — because the move cannot happen on this host,
+# or because a checkout runs ahead of the daemon that performs it — every reader
+# must read the register that is actually there, and must not answer "empty".
+
+
+def test_a_register_at_the_old_name_is_read_by_every_reader(tmp_path, monkeypatch):
+    """The measured defect, one reader per assertion.
+
+    With only the pre-rename file present, each of these answered the empty
+    answer — `[]`, `None`, `{}` — while the register held two entries: silent,
+    because "nothing there" and "it is at the other name" read alike. The
+    candidate list is the one with a consequence: `submit_rant`'s warning about a
+    mis-spelled agent is gated on that list being non-empty, so it never fired.
+    """
+    from emrg.server.scheduler import _load_agent_config, _resolve_agent_path
+    from emrg.tools.file_policy import protected_paths
+    from emrg.tools.submit_rant_tool import SubmitRantTool
+
+    legacy = _legacy(tmp_path)
+    # Each module holds its own bound copy of `config_dir` (`from emrg.config
+    # import config_dir`), so patching the definition does not rebind them — a
+    # seam that patches only the library leaves the caller reading the host's
+    # real register, which is host state in a test.
+    monkeypatch.setattr("emrg.config.config_dir", lambda: tmp_path)
+    monkeypatch.setattr("emrg.server.scheduler.config_dir", lambda: tmp_path)
+
+    assert register_for_reading(tmp_path) == legacy
+
+    assert sorted(SubmitRantTool()._registered_agent_names()) == ["aitokenpool", "emrg"]
+
+    assert _resolve_agent_path("emrg") == "/home/h/evolution/emrg"
+
+    entry = _load_agent_config("emrg", "/home/h/evolution/emrg")
+    assert entry.get("name") == "emrg", entry
+    assert entry.get("path") == "/home/h/evolution/emrg", entry
+
+    assert str(legacy.resolve()) in protected_paths(), (
+        "the register a sandboxed write must not touch is the one that is live"
+    )
+
+
+def test_the_new_name_governs_when_both_are_present(tmp_path):
+    """A legacy file an older daemon re-created cannot shadow the register."""
+    _legacy(tmp_path)
+    current = tmp_path / "agents.yml"
+    current.write_text(yaml.safe_dump([{"name": "only-new"}]), encoding="utf-8")
+
+    assert register_for_reading(tmp_path) == current
+
+
+def test_the_read_path_never_moves_the_register(tmp_path):
+    """Asking a reader where the register is must not perform the migration."""
+    legacy = _legacy(tmp_path)
+
+    assert register_for_reading(tmp_path) == legacy
+
+    assert legacy.exists(), "reading the register moved it"
+    assert not (tmp_path / "agents.yml").exists()
+
+
+def test_the_read_fallback_cannot_shadow_the_migration_target(tmp_path):
+    """The reason the fallback is its own function and not part of `agents_file`.
+
+    `migrate_legacy_registry` takes `agents_file()` as the move *target*. A
+    fallback inside that function would hand it the legacy path as the target,
+    `target.exists()` would be true, and the move would answer "already done" on
+    every host for ever. Read the fallback first, then migrate, and the move
+    still happens.
+    """
+    legacy = _legacy(tmp_path)
+
+    assert register_for_reading(tmp_path) == legacy
+    assert migrate_legacy_registry(tmp_path) is True, "the fallback made the move a no-op"
+
+    assert not legacy.exists()
+    assert agents_file(tmp_path).exists()
+    assert register_for_reading(tmp_path) == agents_file(tmp_path)