diff --git a/CHANGELOG.md b/CHANGELOG.md index d4d618a6..964c7087 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,17 @@ ## Unreleased +- Make the 5-minute cloud-sync poller's + `_broadcast_sync_changes` actually refresh the kanban. + It was sending `resource: "tasks"` while the frontend's + `RESOURCE_TO_QUERY` map only routes `kanban` to the + tasks React Query, so cloud-side task edits picked up + by the periodic sync (rather than the WS hot path) + silently never reached the board. Same fix drops the + `pulled+deleted == 0` gate that suppressed legitimate + refreshes whenever cursor races or push-lock + contention returned zero counts. Closes #150. + - Surface the new task date fields in the kanban UI. Snoozed tasks (`scheduled` in the future) drop off the board until the day arrives, then return with a small diff --git a/kaisho/cron/scheduler.py b/kaisho/cron/scheduler.py index 4dc11207..6816c420 100644 --- a/kaisho/cron/scheduler.py +++ b/kaisho/cron/scheduler.py @@ -391,20 +391,31 @@ def restart_cloud_ws() -> None: def _broadcast_sync_changes(result: dict) -> None: """Notify the desktop frontend after a sync cycle. - Broadcasts a refresh event for each resource so - the UI updates without a manual reload. Over- - broadcasts all resources because the result dict - aggregates counts — this is safe (just extra - React Query refetches). + Broadcasts a refresh event for each resource so the UI + updates without a manual reload. Over-broadcasts all + resources because the ``result`` dict only carries + aggregate counts — sending all four is safe (a few + extra React Query refetches at most). + + Resource names must match the keys in + ``frontend/src/hooks/useWebSocket.ts``'s + ``RESOURCE_TO_QUERY`` — the tasks query is routed via + the ``kanban`` key there, so ``tasks`` (the obvious + name) would be a silent no-op. + + No ``pulled+deleted == 0`` gate: a zero-count result is + not proof that nothing changed — cursor races, + push-lock contention, and partial-success cycles can + all produce zero counts even when remote state has + moved. The cost of always broadcasting is a couple of + extra refetches when the result really is empty, which + is cheaper than the user staring at stale data after a + cycle the gate quietly swallowed. """ - pulled = result.get("pulled_up", 0) - deleted = result.get("pulled_del", 0) - if pulled + deleted == 0: - return from ..api.ws.manager import broadcast_sync try: for resource in ( - "clocks", "inbox", "tasks", "notes", + "clocks", "inbox", "kanban", "notes", ): broadcast_sync({ "resource": resource, diff --git a/tests/test_cloud_ws_broadcast.py b/tests/test_cloud_ws_broadcast.py index d62f2c5f..42c2e6c2 100644 --- a/tests/test_cloud_ws_broadcast.py +++ b/tests/test_cloud_ws_broadcast.py @@ -132,6 +132,47 @@ def fake_run_cloud_sync(): assert {m["resource"] for m in captured} == {"clocks"} +def test_broadcast_sync_changes_uses_kanban_not_tasks( + monkeypatch, +): + """The poller's ``_broadcast_sync_changes`` must + broadcast ``resource: "kanban"`` for the tasks query. + The frontend's RESOURCE_TO_QUERY only routes the + ``kanban`` key to the tasks React Query, so the + previous ``tasks`` payload was a silent no-op.""" + captured = _capture_broadcasts(monkeypatch) + scheduler._broadcast_sync_changes( + {"pulled_up": 1, "pulled_del": 0}, + ) + resources = {m["resource"] for m in captured} + assert "kanban" in resources, ( + "broadcast must use the 'kanban' resource key so " + "the frontend's RESOURCE_TO_QUERY actually " + "invalidates the tasks query" + ) + assert "tasks" not in resources + + +def test_broadcast_sync_changes_fires_when_counts_zero( + monkeypatch, +): + """The old ``pulled+deleted == 0`` gate suppressed + legitimate refreshes when the sync cycle returned + zero counts (cursor races, push-lock contention, + partial-success cycles). The gate is gone: the + function must broadcast every cycle, trusting that an + occasional empty refetch is cheaper than the user + staring at stale data.""" + captured = _capture_broadcasts(monkeypatch) + scheduler._broadcast_sync_changes( + {"pulled_up": 0, "pulled_del": 0}, + ) + resources = {m["resource"] for m in captured} + assert resources == { + "clocks", "inbox", "kanban", "notes", + } + + def test_failed_sync_does_not_broadcast(monkeypatch): """If the sync raises, the pending set must stay intact so the next attempt still has the resource recorded —