diff --git a/Agent.md b/Agent.md index 642e18c4..8a6f99d0 100644 --- a/Agent.md +++ b/Agent.md @@ -118,7 +118,7 @@ Community needs voiced in HN agent-UI discussions map directly to EMRG's design: pkill -f "emrg.server"; rm -f ~/.emrg/emrgd.port; python -m emrg ``` -Python: `uv run pytest tests/ -v` (934) — import check: `uv run python -c "from emrg.client.app import run_client"` +Python: `uv run pytest tests/ -v` (938) — import check: `uv run python -c "from emrg.client.app import run_client"` GUI: `cd emrg/gui && npm test` (258: 45 daemon_client + 19 conn-manager + 22 app-commands + 129 renderer smoke + 16 i18n + 7 integration + 3 commands + 8 build-config + 7 gui-state + 2 tool-group) — syntax: `node --check main.js preload.js daemon_client.js renderer/js/*.js` CI: `uv run pytest` (ubuntu + **windows-2025 matrix** — Windows pytest 回归在 PR CI 即失败，v0.2.29 教训 #725) + GUI tests + **actionlint workflow lint** (`rhysd/actionlint@v1.7.12` gate, #444 — workflow 解析错误在 PR CI 即失败，如 `if:` secrets 上下文) Re-trigger: `scripts/re-trigger-ci.sh [branch]` (workflow_dispatch, #527 — 替代空 commit 重触发：Actions outage 会整段丢弃 push 事件，dispatch 走 API 路径不受影响) diff --git a/emrg/client/daemon_manager.py b/emrg/client/daemon_manager.py index d2d8c21b..24524e7d 100644 --- a/emrg/client/daemon_manager.py +++ b/emrg/client/daemon_manager.py @@ -168,25 +168,51 @@ async def check_and_restart_if_stale() -> None: logger.info( "%s, restarting (old pid=%d)", restart_reason, server_pid, ) - # Kill old server: SIGTERM first, SIGKILL if still alive + # Kill old server: SIGTERM first, SIGKILL if still alive. + # ⚠️ (rant 2026-08-18T12:49:09 ②) The old daemon must be TRULY + # dead before the port file is removed and a new daemon spawns. + # Previously cleanup_server() deleted the port file BEFORE the + # wait, so is_running() (a port-file probe) returned False + # instantly and a new daemon spawned while the old one was still + # shutting down → multiple emrg.server instances on different + # ports. Wait on the old PID itself (POSIX os.kill(pid,0) probe), + # then remove the port file only after it is gone. try: os.kill(server_pid, signal.SIGTERM) except (ProcessLookupError, OSError): pass - cleanup_server() - # Wait for old server to die - for _ in range(10): + + def _old_pid_alive() -> bool: + if sys.platform == "win32": + # os.kill(pid, 0) would TerminateProcess on Windows — + # never use it as a liveness probe. Windows SIGTERM is + # an immediate hard kill, so the port probe suffices. + return is_running() + try: + os.kill(server_pid, 0) + return True + except ProcessLookupError: + return False + except OSError: + return True # EPERM → process exists + + for _ in range(50): # up to 10s for graceful shutdown await asyncio.sleep(0.2) - if not is_running(): + if not _old_pid_alive(): break else: # SIGTERM didn't work — force kill logger.warning("old daemon (pid=%d) didn't die, sending SIGKILL", server_pid) try: os.kill(server_pid, signal.SIGKILL) - await asyncio.sleep(0.3) except (ProcessLookupError, OSError): pass + for _ in range(10): # up to 2s for SIGKILL to land + await asyncio.sleep(0.2) + if not _old_pid_alive(): + break + # Old daemon is gone — now safe to remove its port file + cleanup_server() except (ConnectionRefusedError, FileNotFoundError, OSError, json.JSONDecodeError, asyncio.TimeoutError, ConnectionClosed): # G129 (rant 2026-08-09T08:03:46): only genuinely transient connection diff --git a/emrg/server/daemon.py b/emrg/server/daemon.py index 3a06fbe2..7d01d5b9 100644 --- a/emrg/server/daemon.py +++ b/emrg/server/daemon.py @@ -29,7 +29,7 @@ from emrg._win import win32_no_window_kwargs from emrg.config import LlmConfig, config_dir -from emrg.connect import cleanup_server +from emrg.connect import cleanup_server, is_server_running_sync from emrg.server.atomic import atomic_write_bytes, atomic_write_yaml from emrg.server.llm import LlmClient from emrg.server.git_utils import ( @@ -213,6 +213,27 @@ async def serve(self) -> None: # ── PID file: prevent duplicate daemon instances ─── runtime_dir = config_dir() pid_file = runtime_dir / "emrgd.pid" + + # ── Single-instance admission: port-file liveness probe ─── + # (rant 2026-08-18T12:49:09 ③) Multiple resident clients (GUI + TUI, + # possibly different installs) each spawn/restart the daemon on their + # own schedule; stale-restart sequences can leave the pid file missing + # while an old daemon is still alive, so the pid-file check alone lets + # a second instance start (observed: 4 emrg.server processes + # coexisting on different ports). Probe the port file first — if a + # live daemon already answers, do NOT start a duplicate. + try: + if is_server_running_sync(timeout=1.0): + logger.error( + "another emrgd instance is already listening (port file %s) — " + "refusing to start a duplicate (single-instance admission)", + runtime_dir / "emrgd.port", + ) + self._running = False + return + except Exception: + logger.debug("single-instance port probe failed — continuing startup", exc_info=True) + try: # Atomic create — fails if file already exists fd = os.open(pid_file, os.O_CREAT | os.O_EXCL | os.O_WRONLY) diff --git a/tests/test_daemon.py b/tests/test_daemon.py index cec976d7..1cba1c38 100644 --- a/tests/test_daemon.py +++ b/tests/test_daemon.py @@ -1492,3 +1492,51 @@ async def fake_run_once(state=None): assert calls == [], "no force → must return cache without a fresh fetch" reply = json.loads(writer._frames[-1]) assert reply["type"] == "update_check" + + +# ── rant 2026-08-18T12:49:09 ③：单 daemon 准入（端口活性探测）────────── +def test_serve_refuses_duplicate_when_daemon_alive(tmp_path): + """serve() must refuse to start when another emrgd is already listening. + + Multi-client (GUI + TUI, possibly different installs) stale-restart + sequences can leave the pid file missing while an old daemon is still + alive — the port-file liveness probe catches this and exits cleanly + instead of binding a second port (observed: 4 emrg.server processes). + """ + from unittest.mock import AsyncMock, patch + + server = _make_server() + with patch("emrg.server.daemon.config_dir", return_value=tmp_path), \ + patch("emrg.server.daemon.is_server_running_sync", return_value=True), \ + patch("emrg.server.daemon.serve", new_callable=AsyncMock) as mock_serve: + import asyncio + asyncio.run(server.serve()) + + assert server._running is False, "duplicate daemon must not keep running" + mock_serve.assert_not_awaited(), "must not bind a second socket when one daemon is alive" + # no pid file written by the refused instance + assert not (tmp_path / "emrgd.pid").exists(), "refused instance must not claim the pid file" + + +def test_serve_proceeds_when_no_live_daemon(tmp_path): + """Negative path: no live daemon on the port file → the admission probe + must NOT block startup (the flow reaches the pid-file section).""" + from unittest.mock import AsyncMock, patch + + server = _make_server() + with patch("emrg.server.daemon.config_dir", return_value=tmp_path), \ + patch("emrg.server.daemon.is_server_running_sync", return_value=False), \ + patch("emrg.server.daemon.serve", new_callable=AsyncMock, + side_effect=RuntimeError("abort after probe — not reached in this test")): + import asyncio + # Let the probe run but abort at the websockets bind via a side_effect + # on the module-level serve; the pid file write happens between the + # probe and the bind, proving the probe let us through. + try: + asyncio.run(server.serve()) + except RuntimeError as e: + assert "abort after probe" in str(e), f"unexpected abort: {e}" + else: + raise AssertionError("expected the websockets serve abort (probe passed)") + assert (tmp_path / "emrgd.pid").exists(), ( + "no-live-daemon probe must proceed to pid-file write (admission is liveness-based)") diff --git a/tests/test_daemon_manager.py b/tests/test_daemon_manager.py index e265656a..e78b47a4 100644 --- a/tests/test_daemon_manager.py +++ b/tests/test_daemon_manager.py @@ -8,6 +8,7 @@ import asyncio import json +import signal import tempfile from pathlib import Path from unittest.mock import AsyncMock, MagicMock, patch @@ -155,12 +156,78 @@ def test_source_newer_triggers_restart(self, mock_connect, mock_kill, # started_at in the past → source mtime (1e12) > server_start mock_connect.return_value = FakeWS([_ping_pong_frame()]) + def fake_kill(pid, sig): + # liveness probe (sig=0) → old daemon already dead → no wait + if sig == 0: + raise ProcessLookupError(pid) + + mock_kill.side_effect = fake_kill + with patch("emrg.client.daemon_manager.get_server_path", return_value=str(port_file)): asyncio.run(daemon_manager.check_and_restart_if_stale()) # SIGTERM sent to pid 9999 kill_calls = [c.args for c in mock_kill.call_args_list] assert any(9999 in call and call[0] == 9999 for call in kill_calls) + # rant 12:49:09 ②：port file cleanup happens only AFTER old pid confirmed dead + assert mock_cleanup.called + + @patch("emrg.client.daemon_manager._get_config_mtime", return_value=0.0) + @patch("emrg.client.daemon_manager._get_server_source_mtime", return_value=1e12) + @patch("emrg.client.daemon_manager.is_running", return_value=True) + @patch("emrg.client.daemon_manager.cleanup_server") + @patch("emrg.client.daemon_manager.os.kill") + @patch("emrg.client.daemon_manager.connect_to_server", new_callable=AsyncMock) + def test_restart_waits_until_old_pid_dead_before_cleanup( + self, mock_connect, mock_kill, mock_cleanup, mock_running, + mock_src, mock_cfg, tmp_path): + """rant 12:49:09 ② — old daemon takes ~0.4s to die: cleanup_server() + must NOT run while the old pid is still alive (multi-instance guard).""" + port_file = tmp_path / "emrgd.port" + port_file.write_text("12345\ntoken\n") + mock_connect.return_value = FakeWS([_ping_pong_frame()]) + + probe_calls = {"n": 0} + + def fake_kill(pid, sig): + if sig == 0: + probe_calls["n"] += 1 + if probe_calls["n"] < 3: + return # still alive (no exception = process exists) + raise ProcessLookupError(pid) # dies on 3rd probe + + mock_kill.side_effect = fake_kill + + with patch("emrg.client.daemon_manager.get_server_path", + return_value=str(port_file)): + asyncio.run(daemon_manager.check_and_restart_if_stale()) + # waited ≥2 probe rounds (old pid alive → no cleanup yet), then cleanup after death + assert probe_calls["n"] >= 3, f"should probe liveness ≥3 times, got {probe_calls['n']}" + assert mock_cleanup.called + + @patch("emrg.client.daemon_manager._get_config_mtime", return_value=0.0) + @patch("emrg.client.daemon_manager._get_server_source_mtime", return_value=1e12) + @patch("emrg.client.daemon_manager.is_running", return_value=True) + @patch("emrg.client.daemon_manager.cleanup_server") + @patch("emrg.client.daemon_manager.os.kill") + @patch("emrg.client.daemon_manager.connect_to_server", new_callable=AsyncMock) + def test_restart_force_kills_stuck_old_pid( + self, mock_connect, mock_kill, mock_cleanup, mock_running, + mock_src, mock_cfg, tmp_path): + """rant 12:49:09 ② — old daemon never dies on SIGTERM → SIGKILL fallback, + and cleanup still happens after the kill.""" + port_file = tmp_path / "emrgd.port" + port_file.write_text("12345\ntoken\n") + mock_connect.return_value = FakeWS([_ping_pong_frame()]) + mock_kill.side_effect = lambda pid, sig: None # pid stays "alive" forever + + with patch("emrg.client.daemon_manager.get_server_path", + return_value=str(port_file)): + asyncio.run(daemon_manager.check_and_restart_if_stale()) + kill_calls = [c.args for c in mock_kill.call_args_list] + assert any(c[1] == signal.SIGKILL for c in kill_calls), ( + "stuck old pid must be SIGKILLed after the SIGTERM grace window") + assert mock_cleanup.called @patch("emrg.client.daemon_manager._get_config_mtime", return_value=0.0) @patch("emrg.client.daemon_manager._get_server_source_mtime", return_value=0.0)