diff --git a/emrg/server/__main__.py b/emrg/server/__main__.py index 2731f446..735eb900 100644 --- a/emrg/server/__main__.py +++ b/emrg/server/__main__.py @@ -199,10 +199,12 @@ def main() -> None: config = load_config() # The agent register's move from its pre-rename name runs here and only here # (#2521): this is the one process that owns the file, and it runs before the - # server object below asks `emrg.config.agents_file()` for the path. It is - # placed after `load_config()` so a startup whose configuration is broken - # fails without touching the host's register at all, and it sits in no path - # derivation — asking where the register lives must never move it. + # server object below asks `emrg.config.register_for_reading()` which file to + # open, so a host whose move succeeds is already at the new name before anything + # reads or writes it (#2549). It is placed after `load_config()` so a startup + # whose configuration is broken fails without touching the host's register at + # all, and it sits in no path derivation — asking where the register lives must + # never move it. migrate_legacy_registry() # The task table's agent key moved from `project` to `agent` (issue #2531), # and it moves here for the same reason: this is a process that owns the diff --git a/emrg/server/daemon.py b/emrg/server/daemon.py index 8d54ef10..85aec53f 100644 --- a/emrg/server/daemon.py +++ b/emrg/server/daemon.py @@ -39,12 +39,12 @@ from emrg.config import ( LlmConfig, SandboxConfig, - agents_file, config_dir, logs_dir, find_model_entry, load_sandbox_config, load_update_config, + register_for_reading, resolve_model_vision, ) from emrg.connect import EMRGD_PORT, cleanup_server, is_server_running_sync @@ -869,7 +869,18 @@ def __init__(self, llm_config: LlmConfig) -> None: # (session_id → (text, injected_ms)) so unchanged context is not # re-sent (context_refresh_interval_ms gating). self._context_snapshots: dict[str, tuple[str, float]] = {} - self._agents_log = agents_file(runtime_dir) + # The register this daemon reads and writes is the one that *is there*, not the + # name it belongs at: `register_for_reading` answers the file + # `migrate_legacy_registry` actually left on disk (#2545), and a register still + # carrying its pre-rename name is a state that migration documents and tolerates + # — a read-only home, a permission boundary, a caller that got there first (#2521). + # Asking the canonical name here put both halves on a file that does not exist, + # and `AgentRegistry._read` answers a missing file with `[]`, so the first session + # opened wrote a *second* register holding only its own row; because readers + # prefer the new name once it exists, every entry left at the old one then + # vanished from view without an error (#2549 — the shape #2548 removed from the + # scheduler's self-heal, at the call site that runs on every session open). + self._agents_log = register_for_reading(runtime_dir) self._rants_log = runtime_dir / "rants.jsonl" # ── Phase 2 broadcast model (protocol-contract §2.6) ── diff --git a/tests/conftest.py b/tests/conftest.py index 8b25856b..8f3d53b3 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -153,15 +153,16 @@ def _redirect_the_agent_register(monkeypatch, tmp_path): Autouse for the same reason as the guard above — the test that would leak is the one written three commits from now. - The seam is ``agents_file``, which is how the daemon asks where the register lives. - A test that wants to inspect or steer the register sets ``server._agents_log`` - afterwards, and that assignment wins: the registry is rebuilt from it. + The seam is ``register_for_reading``, which is how the daemon asks where the register + lives — the file that is there, not the name it belongs at (#2549). A test that wants + to inspect or steer the register sets ``server._agents_log`` afterwards, and that + assignment wins: the registry is rebuilt from it. """ import emrg.server.daemon as daemon_mod monkeypatch.setattr( daemon_mod, - "agents_file", + "register_for_reading", lambda base=None: tmp_path / "agent-register" / "agents.yml", ) diff --git a/tests/test_daemon.py b/tests/test_daemon.py index fd7cefe9..86bac219 100644 --- a/tests/test_daemon.py +++ b/tests/test_daemon.py @@ -172,6 +172,102 @@ def _make_server() -> EmrgServer: return server +def test_the_register_written_is_the_one_that_exists(tmp_path, monkeypatch): + """The daemon writes the register that is *there*, not the name it belongs at. + + A register still carrying its pre-rename name is a state + `migrate_legacy_registry` documents and tolerates: a read-only home, a permission + boundary, or a caller that got there first each leave `projects.yml` exactly where + it is, and the daemon still starts. `AgentRegistry._read` answers a *missing* file + with `[]`, so a daemon asking the canonical name registered the first session + opened into a brand-new register holding that one row — and because readers prefer + the new name once it exists, every entry left at the old one disappeared from view + with nothing raised and nothing logged (#2549; the shape #2548 removed from the + scheduler's self-heal). + + Both seams are pinned here so the two names disagree in exactly the way a stuck + migration leaves them, and the assertion names which one the write must reach. + """ + legacy = tmp_path / "projects.yml" + legacy.write_text( + yaml.safe_dump([{"name": "other", "path": str(tmp_path / "gone")}]), + encoding="utf-8", + ) + # The register belongs at `agents.yml`; it is actually at `projects.yml`. + # + # `raising=False` is load-bearing rather than tolerance: the fixed module no longer + # imports `agents_file`, so this pin exists *for the mutant*. A canonical-name call + # re-introduced by a regression resolves through this module global, which is what + # makes the arm fail on the assertion below — the one that names the defect — instead + # of writing the host's register, where the suite's config guard would refuse with a + # different message and a reader would debug the guard. The assertion right after it + # keeps that true mechanically: delete the pin and this test says so instead of + # passing for the wrong reason. + monkeypatch.setattr( + daemon_mod, "agents_file", lambda base=None: tmp_path / "agents.yml", + raising=False, + ) + monkeypatch.setattr(daemon_mod, "register_for_reading", lambda base=None: legacy) + assert daemon_mod.agents_file() == tmp_path / "agents.yml", ( + "the canonical-name pin is what the red direction resolves through; without it " + "a regression would reach the host's register instead of failing here" + ) + + server = EmrgServer(LlmConfig(base_url="http://localhost", api_key="test")) + work = tmp_path / "work" + work.mkdir() + row = server._register_agent(work) + + assert row["path"] == str(work) + assert not (tmp_path / "agents.yml").exists(), ( + "a second register was created: the write addressed a file that does not exist" + ) + assert [e["name"] for e in yaml.safe_load(legacy.read_text(encoding="utf-8"))] == [ + "other", "work", + ], "the row joined the register every reader opens, and no entry was lost" + + +def test_a_fresh_install_registers_into_the_canonical_name(tmp_path, monkeypatch): + """With no register at either name, the daemon still creates `agents.yml`. + + The other half of the claim above, and the half that would regress silently: a host + that has never registered anything has neither file, `register_for_reading` answers + the name the register belongs at, and `AgentRegistry.for_dir` creates it there. The + fix changes *which derivation* answers the path, not where a new register goes, so + this pins the equivalence for the state every new installation starts in. + + Both derivations are pinned to one temporary path, so the two names agree here by + construction — which is why this test is the behaviour half and the disagreeing-name + test above is the one that discriminates the mutant. What this one would catch is a + fix that redirected a new register to the legacy name, or stopped creating one. + """ + monkeypatch.setattr( + daemon_mod, "agents_file", lambda base=None: tmp_path / "agents.yml", + raising=False, + ) + monkeypatch.setattr( + daemon_mod, "register_for_reading", lambda base=None: tmp_path / "agents.yml", + ) + assert daemon_mod.agents_file() == tmp_path / "agents.yml", ( + "both derivations are pinned here so a mutant cannot reach a host's register" + ) + + server = EmrgServer(LlmConfig(base_url="http://localhost", api_key="test")) + work = tmp_path / "work" + work.mkdir() + row = server._register_agent(work) + + register = tmp_path / "agents.yml" + assert register.exists(), "a fresh install must still end up with a register" + assert row["path"] == str(work) + assert [e["path"] for e in yaml.safe_load(register.read_text(encoding="utf-8"))] == [ + str(work), + ] + assert not (tmp_path / "projects.yml").exists(), ( + "the legacy name is never the destination of a new register" + ) + + def test_context_section_no_files(tmp_path): """No context files found → returns empty list.""" server = _make_server()