fix(control): make hermes 'start' idempotent and report pause state in status
'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 <noreply@anthropic.com>
This commit is contained in:
41
src/main.py
41
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
|
||||
|
||||
92
test/test_hermes_control.py
Normal file
92
test/test_hermes_control.py
Normal file
@@ -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)
|
||||
Reference in New Issue
Block a user