From 93185cb93c581fc0a7ab72f2c8d1783424469992 Mon Sep 17 00:00:00 2001 From: alexpolo1 Date: Fri, 28 Aug 2026 06:40:19 +0200 Subject: [PATCH] fix(control): make hermes 'start' idempotent and report pause state in status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 'start' was a plain alias for 'pause' — both branches called the same toggle: if data == 'start' or data == 'pause': start_or_pause_bot(controllers) So a caller retrying a timed-out 'start' PAUSED the bot. On 2026-08-28 a restart routine sent it three times and the bot sat frozen for 4h50m of an overnight run, stopping at the next state change because trigger_or_stop blocks while _pausing is set. The verification that should have caught it failed too: 'status' returned controllers.game.is_running, which tracks the game controller and not Bot._pausing, so a paused bot answered running=True. Both fixed: - 'start' is idempotent — starts a stopped bot, resumes a paused one, and is a no-op on a healthy one. 'pause'/'toggle' remain the toggle. - 'status' reports "running=X paused=Y". The handler was extracted from an inline closure into handle_hermes_command() so this is testable behaviourally rather than by asserting on source text. Verified by falsification: restoring the original semantics makes the suite fail with "repeated 'start' paused a healthy bot" and "status hides the pause state: 'running=True'"; the fix makes all five pass. Co-Authored-By: Claude Opus 5 --- src/main.py | 41 ++++++++++++++--- test/test_hermes_control.py | 92 +++++++++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+), 7 deletions(-) create mode 100644 test/test_hermes_control.py diff --git a/src/main.py b/src/main.py index 0b447df..0a136f5 100644 --- a/src/main.py +++ b/src/main.py @@ -60,6 +60,37 @@ def start_or_pause_bot(controllers: Controllers): screen.start_detecting_window() controllers.game.start() +def handle_hermes_command(data: str, controllers: Controllers): + """Handle one hermes control-socket command. Returns a reply string or None. + + 'start' used to be a plain alias for 'pause' — the same toggle. A caller + retrying a timed-out 'start' therefore PAUSED the bot. On 2026-08-28 that + left it frozen for 4h50m, and `status` still answered running=True because + that field reports the game controller, not Bot._pausing. + + 'start' is now idempotent: it starts a stopped bot, resumes a paused one, + and does nothing to a healthy one. Use 'pause'/'toggle' for the toggle. + """ + bot = getattr(controllers.game, "bot", None) + paused = bool(getattr(bot, "_pausing", False)) + + if data == "start": + if not controllers.game.is_running: + start_or_pause_bot(controllers) + elif paused: + start_or_pause_bot(controllers) # resume + return None # already running and not paused: no-op + if data in ("pause", "toggle"): + start_or_pause_bot(controllers) + return None + if data == "stop": + on_exit(controllers) + return None + if data == "status": + return f"running={controllers.game.is_running} paused={paused}" + return None + + def start_or_stop_graphic_debugger(controllers: Controllers): if controllers.debugger.is_running: controllers.debugger.stop() @@ -312,13 +343,9 @@ def main(): try: conn, _ = _hermes_socket.accept() data = conn.recv(1024).decode().strip().lower() - if data == 'start' or data == 'pause': - start_or_pause_bot(controllers) - elif data == 'stop': - on_exit(controllers) - elif data == 'status': - status = f"running={controllers.game.is_running}" - conn.sendall(status.encode()) + _reply = handle_hermes_command(data, controllers) + if _reply is not None: + conn.sendall(_reply.encode()) conn.close() except Exception: pass diff --git a/test/test_hermes_control.py b/test/test_hermes_control.py new file mode 100644 index 0000000..bc6fd11 --- /dev/null +++ b/test/test_hermes_control.py @@ -0,0 +1,92 @@ +"""The control socket must not be able to pause the bot by accident. + +2026-08-28: `start` was a plain alias for `pause` — both called the same +toggle. A restart routine that retried a timed-out `start` sent it three times +and left the bot PAUSED. It sat frozen for 4h50m of an overnight run. + +The verification that was supposed to catch that also failed: `status` reported +`controllers.game.is_running`, which tracks the game controller, not +Bot._pausing. A paused bot answered running=True. +""" +import types + +import pytest + + +class _FakeBot: + def __init__(self, pausing=False): + self._pausing = pausing + + +class _FakeGame: + def __init__(self, running=False, pausing=False): + self.is_running = running + self.bot = _FakeBot(pausing) if running else None + self.toggles = 0 + self.starts = 0 + + +class _FakeControllers: + def __init__(self, running=False, pausing=False): + self.game = _FakeGame(running, pausing) + self.debugger = types.SimpleNamespace(is_running=False, stop=lambda: None, start=lambda: None) + + +@pytest.fixture +def patched(monkeypatch): + import main + + def fake_toggle(controllers): + g = controllers.game + if g.is_running: + g.toggles += 1 + g.bot._pausing = not g.bot._pausing + else: + g.starts += 1 + g.is_running = True + g.bot = _FakeBot(False) + + monkeypatch.setattr(main, "start_or_pause_bot", fake_toggle) + return main + + +def test_repeated_start_never_pauses_a_running_bot(patched): + """THE regression. Three starts must leave the bot running and unpaused.""" + c = _FakeControllers(running=True, pausing=False) + for _ in range(3): + patched.handle_hermes_command("start", c) + assert c.game.is_running is True + assert c.game.bot._pausing is False, "repeated 'start' paused a healthy bot" + assert c.game.toggles == 0, "'start' toggled a bot that was already running" + + +def test_start_starts_a_stopped_bot(patched): + c = _FakeControllers(running=False) + patched.handle_hermes_command("start", c) + assert c.game.is_running is True + assert c.game.starts == 1 + + +def test_start_resumes_a_paused_bot(patched): + c = _FakeControllers(running=True, pausing=True) + patched.handle_hermes_command("start", c) + assert c.game.bot._pausing is False, "'start' did not resume a paused bot" + + +def test_pause_still_toggles(patched): + c = _FakeControllers(running=True, pausing=False) + patched.handle_hermes_command("pause", c) + assert c.game.bot._pausing is True + patched.handle_hermes_command("pause", c) + assert c.game.bot._pausing is False + + +def test_status_reports_pause_state(patched): + """A paused bot answering running=True is what hid the outage.""" + c = _FakeControllers(running=True, pausing=True) + reply = patched.handle_hermes_command("status", c) + assert "paused=True" in reply, f"status hides the pause state: {reply!r}" + assert "running=True" in reply + + c2 = _FakeControllers(running=True, pausing=False) + assert "paused=False" in patched.handle_hermes_command("status", c2)