From 581b4116d4d90ea0d5665782bc4dc777cc41eb77 Mon Sep 17 00:00:00 2001 From: Ramon Bartl Date: Tue, 9 Jun 2026 22:15:19 +0200 Subject: [PATCH] Make the periodic poller's _broadcast_sync_changes actually refresh the kanban by renaming the tasks resource to kanban and dropping the pulled-counts gate Closes #150. Two latent bugs in the periodic cloud-sync refresh path that the WS hot path (#149) does not touch: - The broadcast emitted resource: 'tasks', but the frontend's RESOURCE_TO_QUERY in frontend/src/hooks/useWebSocket.ts only routes the 'kanban' key to the tasks React Query. So cloud-side task edits picked up by the 5-minute poller never refreshed the board -- the user had to switch views or restart for them to appear. One-character fix in the resource tuple. - A 'pulled_up + pulled_del == 0' early return swallowed the broadcast on cycles where the sync claimed zero pulls. But zero counts happen for reasons other than 'nothing changed': cursor races (sync cursor advanced ahead of the user-visible edit on a prior cycle), push-lock contention (active profile sync skipped because a local mutation was mid-push), partial-success cycles. Dropping the gate means a handful of extra refetches per cycle in the genuinely-empty case, which is cheaper than the user staring at stale data. Regression tests pin both: - test_broadcast_sync_changes_uses_kanban_not_tasks asserts kanban is in the broadcast and tasks is not. - test_broadcast_sync_changes_fires_when_counts_zero asserts the function broadcasts even when the result dict has pulled_up=0 and pulled_del=0. --- CHANGELOG.md | 11 +++++++++ kaisho/cron/scheduler.py | 31 ++++++++++++++++-------- tests/test_cloud_ws_broadcast.py | 41 ++++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 10 deletions(-) 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 —