fix(modules): auto-restart crashed watchers and fix toggle for dead processes

Three bugs fixed:

1. **Module toggle required two clicks for crashed modules**: `toggle()` checked
   `self.started` instead of `self.is_alive()`, so clicking a crashed module
   first called `stop()` (just cleaning state), then required a second click
   to actually start it.

2. **Crashed modules never auto-restarted**: `check_module_status()` ran once
   at startup, then incorrectly scheduled `rebuild_modules_menu` instead of
   rescheduling itself. After the first 2-second check, module crashes were
   never detected again.

3. **Dialog restart button missing testing arg**: `restart_button.clicked.connect(module.start)`
   didn't pass the `testing` parameter.

Changes:
- `Module.toggle()` now uses `is_alive()` to decide stop vs start
- `check_module_status()` reschedules itself every 5s (was one-shot)
- Crashed modules are auto-restarted up to 3 times with tray notifications
- After max restarts, shows dialog with manual restart option
- Manual toggle/restart resets the auto-restart counter
- Added unit tests for toggle behavior with crashed processes

Refs: ActivityWatch/aw-watcher-window#101, #103

Author: TimeToBuildBob

aw_qt/manager.py

@@ -200,9 +200,12 @@ def stop(self) -> None:
         self.started = False
 
     def toggle(self, testing: bool) -> None:
-        if self.started:
+        if self.is_alive():
             self.stop()
         else:
+            if self.started:
+                # Process died unexpectedly, clean up state
+                self.stop()
             self.start(testing)
 
     def is_alive(self) -> bool:

aw_qt/trayicon.py

@@ -73,6 +73,8 @@ def open_dir(d: str) -> None:
 
 
 class TrayIcon(QSystemTrayIcon):
+    MAX_AUTO_RESTARTS = 3
+
     def __init__(
         self,
         manager: Manager,
@@ -86,6 +88,7 @@ def __init__(
 
         self.manager = manager
         self.testing = testing
+        self._restart_counts: Dict[str, int] = {}
 
         self.root_url = f"http://localhost:{5666 if self.testing else 5600}"
         self.activated.connect(self.on_activated)
@@ -137,11 +140,24 @@ def _build_rootmenu(self) -> None:
         def show_module_failed_dialog(module: Module) -> None:
             box = QMessageBox(self._parent)
             box.setIcon(QMessageBox.Icon.Warning)
-            box.setText(f"Module {module.name} quit unexpectedly")
+            box.setText(
+                f"Module {module.name} quit unexpectedly"
+                + (
+                    f" after {self.MAX_AUTO_RESTARTS} auto-restart attempts"
+                    if self._restart_counts.get(module.name, 0)
+                    >= self.MAX_AUTO_RESTARTS
+                    else ""
+                )
+            )
             box.setDetailedText(module.read_log(self.testing))
 
             restart_button = QPushButton("Restart", box)
-            restart_button.clicked.connect(module.start)
+
+            def on_manual_restart() -> None:
+                self._restart_counts.pop(module.name, None)
+                module.start(self.testing)
+
+            restart_button.clicked.connect(on_manual_restart)
             box.addButton(restart_button, QMessageBox.ButtonRole.AcceptRole)
             box.setStandardButtons(QMessageBox.StandardButton.Cancel)
 
@@ -153,31 +169,53 @@ def rebuild_modules_menu() -> None:
                     module: Module = action.data()
                     alive = module.is_alive()
                     action.setChecked(alive)
-                    # print(module.text(), alive)
 
-            # TODO: Do it in a better way, singleShot isn't pretty...
             QtCore.QTimer.singleShot(2000, rebuild_modules_menu)
 
         QtCore.QTimer.singleShot(2000, rebuild_modules_menu)
 
         def check_module_status() -> None:
             unexpected_exits = self.manager.get_unexpected_stops()
-            if unexpected_exits:
-                for module in unexpected_exits:
+            for module in unexpected_exits:
+                count = self._restart_counts.get(module.name, 0)
+                if count < self.MAX_AUTO_RESTARTS:
+                    logger.info(
+                        f"Auto-restarting crashed module {module.name} "
+                        f"(attempt {count + 1}/{self.MAX_AUTO_RESTARTS})"
+                    )
+                    module.stop()  # Clean up state
+                    module.start(self.testing)
+                    self._restart_counts[module.name] = count + 1
+                    self.showMessage(
+                        "ActivityWatch",
+                        f"Module {module.name} crashed and was auto-restarted",
+                        QSystemTrayIcon.MessageIcon.Warning,
+                        5000,
+                    )
+                else:
+                    logger.warning(
+                        f"Module {module.name} exceeded max auto-restarts "
+                        f"({self.MAX_AUTO_RESTARTS})"
+                    )
                     show_module_failed_dialog(module)
                     module.stop()
 
-            # TODO: Do it in a better way, singleShot isn't pretty...
-            QtCore.QTimer.singleShot(2000, rebuild_modules_menu)
+            QtCore.QTimer.singleShot(5000, check_module_status)
 
-        QtCore.QTimer.singleShot(2000, check_module_status)
+        QtCore.QTimer.singleShot(5000, check_module_status)
 
     def _build_modulemenu(self, moduleMenu: QMenu) -> None:
         moduleMenu.clear()
 
         def add_module_menuitem(module: Module) -> None:
             title = module.name
-            ac = moduleMenu.addAction(title, lambda: module.toggle(self.testing))
+
+            def on_toggle(m: Module = module) -> None:
+                m.toggle(self.testing)
+                # Reset auto-restart count on manual toggle
+                self._restart_counts.pop(m.name, None)
+
+            ac = moduleMenu.addAction(title, on_toggle)
 
             ac.setData(module)
             ac.setCheckable(True)

tests/test_manager.py

@@ -0,0 +1,121 @@
+"""Unit tests for the Module manager."""
+
+import subprocess
+from pathlib import Path
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+from aw_qt.manager import Module
+
+
+@pytest.fixture
+def module():
+    """Create a test module with a mock path."""
+    return Module("aw-test-module", Path("/usr/bin/true"), "system")
+
+
+class TestModuleToggle:
+    """Tests for Module.toggle() behavior, especially with crashed processes."""
+
+    def test_toggle_starts_stopped_module(self, module):
+        """Toggle should start a module that was never started."""
+        assert not module.started
+        assert not module.is_alive()
+
+        with patch.object(module, "start") as mock_start:
+            module.toggle(testing=True)
+            mock_start.assert_called_once_with(True)
+
+    def test_toggle_stops_running_module(self, module):
+        """Toggle should stop a module that is running."""
+        # Simulate a running process
+        mock_proc = MagicMock(spec=subprocess.Popen)
+        mock_proc.returncode = None  # None means still running
+
+        # After terminate+wait, process should report as dead
+        def fake_wait():
+            mock_proc.returncode = -15  # SIGTERM
+
+        mock_proc.wait.side_effect = fake_wait
+        module._process = mock_proc
+        module.started = True
+
+        module.toggle(testing=True)
+        assert not module.started
+        assert module._process is None
+
+    def test_toggle_restarts_crashed_module(self, module):
+        """Toggle should restart a module that crashed (started=True, process dead).
+
+        This is the key fix: previously, toggle checked `self.started` instead of
+        `self.is_alive()`, so the first click would only call stop() (cleaning up
+        state) without starting, requiring a second click to actually start.
+        """
+        # Simulate a crashed process: started=True but process exited
+        mock_proc = MagicMock(spec=subprocess.Popen)
+        mock_proc.returncode = 1  # Non-zero = exited
+        module._process = mock_proc
+        module.started = True
+
+        assert not module.is_alive()  # Process is dead
+        assert module.started  # But flag says started
+
+        with patch.object(module, "start") as mock_start:
+            module.toggle(testing=True)
+            # Should have cleaned up and started in one toggle
+            mock_start.assert_called_once_with(True)
+            assert not module.started  # stop() was called to clean up
+
+
+class TestModuleIsAlive:
+    """Tests for Module.is_alive() behavior."""
+
+    def test_is_alive_no_process(self, module):
+        assert not module.is_alive()
+
+    def test_is_alive_running(self, module):
+        mock_proc = MagicMock(spec=subprocess.Popen)
+        mock_proc.returncode = None
+        module._process = mock_proc
+        assert module.is_alive()
+
+    def test_is_alive_exited(self, module):
+        mock_proc = MagicMock(spec=subprocess.Popen)
+        mock_proc.returncode = 0
+        module._process = mock_proc
+        assert not module.is_alive()
+
+
+class TestGetUnexpectedStops:
+    """Tests for Manager.get_unexpected_stops()."""
+
+    def test_detects_crashed_module(self):
+        from aw_qt.manager import Manager
+
+        with patch.object(Manager, "discover_modules"):
+            mgr = Manager(testing=True)
+
+        mod = Module("aw-test", Path("/usr/bin/true"), "system")
+        mock_proc = MagicMock(spec=subprocess.Popen)
+        mock_proc.returncode = 1
+        mod._process = mock_proc
+        mod.started = True
+        mgr.modules = [mod]
+
+        unexpected = mgr.get_unexpected_stops()
+        assert len(unexpected) == 1
+        assert unexpected[0].name == "aw-test"
+
+    def test_ignores_intentionally_stopped(self):
+        from aw_qt.manager import Manager
+
+        with patch.object(Manager, "discover_modules"):
+            mgr = Manager(testing=True)
+
+        mod = Module("aw-test", Path("/usr/bin/true"), "system")
+        mod.started = False  # Intentionally stopped
+        mgr.modules = [mod]
+
+        unexpected = mgr.get_unexpected_stops()
+        assert len(unexpected) == 0