refactor(meshchat): simplify auto-announce and sync logic by introducing interval_action_due utility function; add tests for auto-announce functionality

meshchatx/meshchat.py

@@ -87,6 +87,7 @@ from meshchatx.src.backend.meshchat_utils import (
     convert_propagation_node_state_to_string,
     has_attachments,
     hex_identifier_to_bytes,
+    interval_action_due,
     message_fields_have_attachments,
     normalize_hex_identifier,
     parse_bool_query_param,
@@ -1604,28 +1605,13 @@ class ReticulumMeshChat:
         gc_counter = 0
 
         while self.running and ctx.running and ctx.session_id == session_id:
-            should_announce = False
-
-            # check if auto announce is enabled
-            if ctx.config.auto_announce_enabled.get():
-                # check if we have announced recently
-                last_announced_at = ctx.config.last_announced_at.get()
-                if last_announced_at is not None:
-                    # determine when next announce should be sent
-                    auto_announce_interval_seconds = (
-                        ctx.config.auto_announce_interval_seconds.get()
-                    )
-                    next_announce_at = (
-                        last_announced_at + auto_announce_interval_seconds
-                    )
-
-                    # we should announce if current time has passed next announce at timestamp
-                    if time.time() > next_announce_at:
-                        should_announce = True
-
-                else:
-                    # last announced at is null, so we have never announced, lets do it now
-                    should_announce = True
+            now = time.time()
+            should_announce = interval_action_due(
+                ctx.config.auto_announce_enabled.get(),
+                ctx.config.last_announced_at.get(),
+                ctx.config.auto_announce_interval_seconds.get(),
+                now,
+            )
 
             # announce
             if should_announce:
@@ -1650,26 +1636,18 @@ class ReticulumMeshChat:
             return
 
         while self.running and ctx.running and ctx.session_id == session_id:
-            should_sync = False
-
-            # check if auto sync is enabled
-            auto_sync_interval_seconds = ctx.config.lxmf_preferred_propagation_node_auto_sync_interval_seconds.get()
-            if auto_sync_interval_seconds > 0:
-                # check if we have synced recently
-                last_synced_at = (
-                    ctx.config.lxmf_preferred_propagation_node_last_synced_at.get()
-                )
-                if last_synced_at is not None:
-                    # determine when next sync should happen
-                    next_sync_at = last_synced_at + auto_sync_interval_seconds
-
-                    # we should sync if current time has passed next sync at timestamp
-                    if time.time() > next_sync_at:
-                        should_sync = True
-
-                else:
-                    # last synced at is null, so we have never synced, lets do it now
-                    should_sync = True
+            auto_sync_interval_seconds = (
+                ctx.config.lxmf_preferred_propagation_node_auto_sync_interval_seconds.get()
+            )
+            last_synced_at = (
+                ctx.config.lxmf_preferred_propagation_node_last_synced_at.get()
+            )
+            should_sync = interval_action_due(
+                auto_sync_interval_seconds is not None and auto_sync_interval_seconds > 0,
+                last_synced_at,
+                auto_sync_interval_seconds,
+                time.time(),
+            )
 
             # sync
             if should_sync:

meshchatx/src/backend/meshchat_utils.py

@@ -232,3 +232,28 @@ def hex_identifier_to_bytes(value: str | None) -> bytes | None:
         return bytes.fromhex(h)
     except ValueError:
         return None
+
+
+def interval_action_due(
+    enabled: bool,
+    last_at: int | None,
+    interval_seconds: int | None,
+    now: float,
+) -> bool:
+    """Return whether a periodic action should run now.
+
+    Used for auto-announce, propagation sync, and similar timers stored in config.
+    If ``last_at`` is ahead of ``now`` (clock skew, restored DB, or bad values),
+    the action is treated as due so scheduling does not stall until wall clock
+    catches a corrupted future timestamp.
+    """
+    if not enabled:
+        return False
+    iv = interval_seconds if interval_seconds is not None else 0
+    if iv <= 0:
+        return False
+    if last_at is None:
+        return True
+    if last_at > now:
+        return True
+    return now > last_at + iv

tests/backend/test_auto_announce_schedule.py

@@ -0,0 +1,98 @@
+"""Regression tests for auto-announce and periodic interval scheduling (config / DB time)."""
+
+import pytest
+
+from meshchatx.src.backend.meshchat_utils import interval_action_due
+
+
+class TestIntervalActionDue:
+    def test_disabled_never_fires(self):
+        assert interval_action_due(False, None, 3600, 1_000_000.0) is False
+        assert interval_action_due(False, 1, 3600, 1_000_000.0) is False
+
+    def test_zero_or_negative_interval_never_fires_even_if_enabled(self):
+        assert interval_action_due(True, 100, 0, 200.0) is False
+        assert interval_action_due(True, 100, -1, 200.0) is False
+        assert interval_action_due(True, None, 0, 200.0) is False
+
+    def test_none_interval_treated_as_zero(self):
+        assert interval_action_due(True, 100, None, 500.0) is False
+
+    def test_first_run_when_last_at_is_none(self):
+        assert interval_action_due(True, None, 900, 1.0) is True
+
+    def test_due_when_past_next_boundary(self):
+        now = 1_000_000.0
+        last = int(now - 1000)
+        assert interval_action_due(True, last, 900, now) is True
+
+    def test_not_due_inside_interval(self):
+        now = 1_000_000.0
+        last = int(now - 100)
+        assert interval_action_due(True, last, 900, now) is False
+
+    def test_future_last_at_treated_as_due_regression(self):
+        """Stale future timestamps (clock skew, restored DB) must not stall the loop."""
+        now = 1_000_000.0
+        last = int(now + 86_400)
+        assert interval_action_due(True, last, 3600, now) is True
+
+    def test_boundary_exclusive(self):
+        now = 1_000_000.0
+        last = int(now - 900)
+        assert interval_action_due(True, last, 900, now) is False
+        assert interval_action_due(True, last, 900, now + 0.001) is True
+
+    def test_propagation_sync_equivalent_enabled_flag(self):
+        interval = 600
+        now = 500_000.0
+        last = int(now - 601)
+        assert (
+            interval_action_due(interval > 0, last, interval, now) is True
+        )
+
+    def test_large_interval_last_at_slightly_in_future_ntp(self):
+        """Small positive skew still triggers one correction announce."""
+        now = 1_000_000.0
+        last = int(now + 2)
+        assert interval_action_due(True, last, 3600, now) is True
+
+
+@pytest.mark.parametrize(
+    ("enabled", "last", "interval", "now", "expected"),
+    [
+        (True, None, 60, 0.0, True),
+        (True, 0, 60, 61.0, True),
+        (True, 0, 60, 60.0, False),
+        (False, None, 60, 999.0, False),
+    ],
+)
+def test_interval_action_due_table(enabled, last, interval, now, expected):
+    assert interval_action_due(enabled, last, interval, now) is expected
+
+
+def test_last_announced_at_config_roundtrip_db(tmp_path):
+    """Config table stores last_announced_at; invalid strings fall back to default."""
+    import os
+
+    from meshchatx.src.backend.config_manager import ConfigManager
+    from meshchatx.src.backend.database import Database
+
+    db_path = tmp_path / "c.db"
+    database = Database(str(db_path))
+    database.initialize()
+    try:
+        cfg = ConfigManager(database)
+        cfg.last_announced_at.set(1_700_000_000)
+        assert cfg.last_announced_at.get() == 1_700_000_000
+
+        cfg2 = ConfigManager(database)
+        assert cfg2.last_announced_at.get() == 1_700_000_000
+
+        database.config.set("last_announced_at", "not-an-int")
+        cfg3 = ConfigManager(database)
+        assert cfg3.last_announced_at.get() is None
+    finally:
+        database.close()
+        if os.path.exists(db_path):
+            os.remove(db_path)

tests/frontend/AppAutoAnnounce.test.js

@@ -0,0 +1,101 @@
+import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
+import App from "../../meshchatx/src/frontend/components/App.vue";
+import ToastUtils from "../../meshchatx/src/frontend/js/ToastUtils";
+import WebSocketConnection from "../../meshchatx/src/frontend/js/WebSocketConnection";
+
+vi.mock("../../meshchatx/src/frontend/js/ToastUtils", () => ({
+    default: {
+        success: vi.fn(),
+        error: vi.fn(),
+    },
+}));
+
+vi.mock("../../meshchatx/src/frontend/js/WebSocketConnection", () => ({
+    default: {
+        send: vi.fn(),
+        connect: vi.fn(),
+        on: vi.fn(),
+        off: vi.fn(),
+        destroy: vi.fn(),
+    },
+}));
+
+describe("App.vue sidebar announce and auto-announce interval", () => {
+    const axiosMock = { get: vi.fn() };
+
+    beforeEach(() => {
+        vi.clearAllMocks();
+        window.api = axiosMock;
+    });
+
+    afterEach(() => {
+        delete window.api;
+    });
+
+    it("sendAnnounce requests announce endpoint and refreshes config", async () => {
+        axiosMock.get.mockImplementation((url) => {
+            if (url === "/api/v1/announce") {
+                return Promise.resolve({ data: {} });
+            }
+            if (url === "/api/v1/config") {
+                return Promise.resolve({
+                    data: {
+                        config: {
+                            last_announced_at: Math.floor(Date.now() / 1000),
+                            auto_announce_interval_seconds: 3600,
+                        },
+                    },
+                });
+            }
+            return Promise.resolve({ data: {} });
+        });
+
+        const ctx = {
+            config: { auto_announce_interval_seconds: 3600 },
+            getConfig: App.methods.getConfig,
+            $t: (k) => k,
+        };
+
+        await App.methods.sendAnnounce.call(ctx);
+
+        expect(axiosMock.get).toHaveBeenCalledWith("/api/v1/announce");
+        expect(axiosMock.get).toHaveBeenCalledWith("/api/v1/config");
+        expect(ToastUtils.success).toHaveBeenCalled();
+        expect(ctx.config.last_announced_at).toBeDefined();
+    });
+
+    it("sendAnnounce surfaces failure toast on announce error", async () => {
+        axiosMock.get.mockImplementation((url) => {
+            if (url === "/api/v1/announce") {
+                return Promise.reject(new Error("network"));
+            }
+            return Promise.resolve({ data: {} });
+        });
+
+        const ctx = {
+            config: {},
+            getConfig: vi.fn(),
+            $t: (k) => k,
+        };
+
+        await App.methods.sendAnnounce.call(ctx);
+
+        expect(ToastUtils.error).toHaveBeenCalled();
+    });
+
+    it("onAnnounceIntervalSecondsChange sends config.set with interval", async () => {
+        const ctx = {
+            config: { auto_announce_interval_seconds: 3600 },
+            updateConfig: App.methods.updateConfig,
+            $t: (k) => k,
+        };
+
+        await App.methods.onAnnounceIntervalSecondsChange.call(ctx);
+
+        expect(WebSocketConnection.send).toHaveBeenCalled();
+        const raw = WebSocketConnection.send.mock.calls[0][0];
+        const parsed = JSON.parse(raw);
+        expect(parsed.type).toBe("config.set");
+        expect(parsed.config.auto_announce_interval_seconds).toBe(3600);
+    });
+});