From 3c5da5307c8d70951de29b8c27d8f68b0733f765 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 29 Jul 2026 18:50:09 -0500 Subject: [PATCH] feat(pairing): give pairing its own change signal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The poll this page's pairing block rode was retired hours earlier by f8e07a332, which moved Messaging onto platforms.changed. That signal is the mtime of gateway_state.json — where the gateway persists connect/disconnect health — and a new pairing request moves none of it. So on an event-capable backend a pending row stayed invisible until something unrelated reconnected, and the count badge with it. Adds pairing.changed to the change watcher, signed off the pending/approved ledgers across the global store and every profile's own. _rate_limits.json is deliberately excluded: it moves on every unauthorized DM, including ones that produce no new row, so signalling on it would refetch for nothing. The page now refreshes platforms and pairing on their own signals rather than one combined call, and the legacy visible-tab poll (older backends) covers both. --- apps/desktop/src/app/messaging/index.test.tsx | 35 ++++++++++ apps/desktop/src/app/messaging/index.tsx | 69 ++++++++++++------- .../hooks/use-message-stream/gateway-event.ts | 6 +- apps/desktop/src/store/live-sync.ts | 5 ++ tests/tui_gateway/test_change_watcher.py | 48 +++++++++++++ tui_gateway/server.py | 41 +++++++++++ 6 files changed, 179 insertions(+), 25 deletions(-) diff --git a/apps/desktop/src/app/messaging/index.test.tsx b/apps/desktop/src/app/messaging/index.test.tsx index be451825de4..836c5fc9384 100644 --- a/apps/desktop/src/app/messaging/index.test.tsx +++ b/apps/desktop/src/app/messaging/index.test.tsx @@ -168,4 +168,39 @@ describe('MessagingView pairing', () => { expect((await screen.findAllByText('Microsoft Teams')).length).toBeGreaterThan(0) expect(screen.queryByRole('button', { name: 'Approve' })).toBeNull() }) + + it('refetches pending rows on pairing.changed, not on platforms.changed', async () => { + // The two signals are not interchangeable: platforms.changed tracks + // connect/disconnect health via gateway_state.json, which a new pairing + // request never moves. Riding it would leave someone invisible in the + // pending list until an unrelated reconnect happened to fire. + const { $changeEventsAvailable, $pairingChangeTick, $platformsChangeTick } = await import( + '@/store/live-sync' + ) + + getMessagingPlatforms.mockResolvedValue({ platforms: [platform()] }) + getPairing.mockResolvedValue({ approved: [], pending: [] }) + + await renderMessaging() + await act(async () => { + $changeEventsAvailable.set(true) + }) + getPairing.mockClear() + + // Someone DMs the bot: the store moves, the watcher ticks pairing.changed. + getPairing.mockResolvedValue({ approved: [], pending: [pendingUser] }) + await act(async () => { + $pairingChangeTick.set($pairingChangeTick.get() + 1) + }) + + await waitFor(() => expect(getPairing).toHaveBeenCalled()) + expect(await screen.findByRole('button', { name: 'Approve' })).toBeTruthy() + + // A platform health tick alone must not be what fetches pairing. + getPairing.mockClear() + await act(async () => { + $platformsChangeTick.set($platformsChangeTick.get() + 1) + }) + expect(getPairing).not.toHaveBeenCalled() + }) }) diff --git a/apps/desktop/src/app/messaging/index.tsx b/apps/desktop/src/app/messaging/index.tsx index 737eb8e4d25..a921e8f97d3 100644 --- a/apps/desktop/src/app/messaging/index.tsx +++ b/apps/desktop/src/app/messaging/index.tsx @@ -26,7 +26,7 @@ import { openExternalLink } from '@/lib/external-link' import { ExternalLink, Save, Trash2 } from '@/lib/icons' import { normalize } from '@/lib/text' import { cn } from '@/lib/utils' -import { $changeEventsAvailable, $platformsChangeTick } from '@/store/live-sync' +import { $changeEventsAvailable, $pairingChangeTick, $platformsChangeTick } from '@/store/live-sync' import { notify, notifyError } from '@/store/notifications' import { runGatewayRestart } from '@/store/system-actions' @@ -151,22 +151,11 @@ export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, . } try { - // Pairing rides the same refresh as platform status: both feed this - // page and a lone failure shouldn't blank the other. A backend older - // than the request-id endpoint just yields no rows. - const [result, pairingResult] = await Promise.allSettled([getMessagingPlatforms(), getPairing()]) - - if (result.status === 'fulfilled') { - setPlatforms(result.value.platforms) - } else if (!silent) { - notifyError(result.reason, m.loadFailed) - } - - if (pairingResult.status === 'fulfilled') { - setPairing({ - approved: pairingResult.value.approved ?? [], - pending: pairingResult.value.pending ?? [] - }) + const result = await getMessagingPlatforms() + setPlatforms(result.platforms) + } catch (err) { + if (!silent) { + notifyError(err, m.loadFailed) } } finally { if (!silent) { @@ -177,14 +166,46 @@ export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, . [m] ) - useRefreshHotkey(() => void refreshPlatforms()) + // Pairing has its own signal. platforms.changed tracks connect/disconnect + // health via gateway_state.json, which a new pairing request never moves — + // riding it would leave a pending row invisible until something unrelated + // reconnected. Failures stay silent: an older backend without the endpoint + // should show no rows, not an error banner over a working page. + const refreshPairing = useCallback(async () => { + try { + const result = await getPairing() + setPairing({ approved: result.approved ?? [], pending: result.pending ?? [] }) + } catch { + // Leave the last known rows in place rather than blanking them. + } + }, []) + + const refreshAll = useCallback( + async (silent = false) => { + await Promise.all([refreshPlatforms(silent), refreshPairing()]) + }, + [refreshPairing, refreshPlatforms] + ) + + useRefreshHotkey(() => void refreshAll()) useEffect(() => { - void refreshPlatforms() - }, [refreshPlatforms]) + void refreshAll() + }, [refreshAll]) const changeEventsAvailable = useStore($changeEventsAvailable) const platformsChangeTick = useStore($platformsChangeTick) + const pairingChangeTick = useStore($pairingChangeTick) + + // A new pending request (or a grant from another surface) moves the pairing + // store on disk; the change watcher turns that into pairing.changed. + useEffect(() => { + if (!changeEventsAvailable || pairingChangeTick === 0 || document.hidden) { + return + } + + void refreshPairing() + }, [changeEventsAvailable, pairingChangeTick, refreshPairing]) // Connection status updates without a manual "check" click. platforms.changed // (the gateway persisting connect/disconnect/health to gateway_state.json) @@ -210,7 +231,7 @@ export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, . return } - void refreshPlatforms(true) + void refreshAll(true) } const id = window.setInterval(tick, 6000) @@ -219,7 +240,7 @@ export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, . cancelled = true window.clearInterval(id) } - }, [changeEventsAvailable, refreshPlatforms]) + }, [changeEventsAvailable, refreshAll]) const selected = useMemo(() => { if (!platforms) { @@ -346,7 +367,7 @@ export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, . try { await approvePairing(user.platform, user.request_id) notify({ kind: 'success', title: m.approvedUser(pairingLabel(user)), message: m.approvedHint }) - await refreshPlatforms(true) + await refreshPairing() } catch (err) { setPairing(snapshot) // 429 is the code path's brute-force lockout — a distinct condition the @@ -371,7 +392,7 @@ export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, . try { await revokePairing(user.platform, user.user_id) notify({ kind: 'success', title: m.revokedUser(pairingLabel(user)), message: user.platform }) - await refreshPlatforms(true) + await refreshPairing() } catch (err) { setPairing(snapshot) throw err diff --git a/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts b/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts index f8b232a8831..9ee6a2feb9b 100644 --- a/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts +++ b/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts @@ -26,6 +26,7 @@ import { $gateway } from '@/store/gateway' import { applyGoalStatusText } from '@/store/goals' import { notifyCronChanged, + notifyPairingChanged, notifyPetChanged, notifyPlatformsChanged, notifySessionsChanged, @@ -303,7 +304,8 @@ export function useGatewayEventHandler(deps: GatewayEventDeps) { event.type === 'pet.changed' || event.type === 'cron.changed' || event.type === 'sessions.changed' || - event.type === 'platforms.changed' + event.type === 'platforms.changed' || + event.type === 'pairing.changed' ) { // Change-watcher broadcasts (server._broadcast_watched_changes): the // backend's on-disk signature moved. Route to the live-sync ticks the @@ -319,6 +321,8 @@ export function useGatewayEventHandler(deps: GatewayEventDeps) { notifyCronChanged() } else if (event.type === 'platforms.changed') { notifyPlatformsChanged() + } else if (event.type === 'pairing.changed') { + notifyPairingChanged() } else { notifySessionsChanged() } diff --git a/apps/desktop/src/store/live-sync.ts b/apps/desktop/src/store/live-sync.ts index cab1f5a8a99..61b2a55cf9f 100644 --- a/apps/desktop/src/store/live-sync.ts +++ b/apps/desktop/src/store/live-sync.ts @@ -19,6 +19,7 @@ export const $changeEventsAvailable = atom(false) export const $cronChangeTick = atom(0) export const $sessionsChangeTick = atom(0) export const $platformsChangeTick = atom(0) +export const $pairingChangeTick = atom(0) /** `pet.info.meta`-shaped payload carried on `pet.changed` — lets the pet skip * the heavy sprite refetch when the broadcast already says enabled=false. */ @@ -52,6 +53,10 @@ export function notifyPlatformsChanged(): void { $platformsChangeTick.set($platformsChangeTick.get() + 1) } +export function notifyPairingChanged(): void { + $pairingChangeTick.set($pairingChangeTick.get() + 1) +} + /** Reset on gateway wipe/reconnect — a new backend re-advertises capability on * its own gateway.ready, and stale ticks must not fire refreshes into stores * the wipe just cleared. */ diff --git a/tests/tui_gateway/test_change_watcher.py b/tests/tui_gateway/test_change_watcher.py index b4b6558418e..c0785283496 100644 --- a/tests/tui_gateway/test_change_watcher.py +++ b/tests/tui_gateway/test_change_watcher.py @@ -72,6 +72,54 @@ def test_gateway_state_move_broadcasts_platforms_changed(watcher_home): assert ("platforms.changed", {}) in events +def test_pending_pairing_request_broadcasts_pairing_changed(watcher_home): + """A new pending request must reach the Messaging page on its own signal. + + The messaging gateway writes the pending code from a different process, and + it moves nothing in gateway_state.json — so platforms.changed cannot stand + in for this. Without a dedicated signal the badge stays invisible until an + unrelated connect/disconnect happens to fire. + """ + home, events = watcher_home + store = home / "platforms" / "pairing" + store.mkdir(parents=True) + server._broadcast_watched_changes(now=0.0) + + (store / "telegram-pending.json").write_text('{"abc": {"user_id": "1"}}') + server._broadcast_watched_changes(now=10.0) + + assert ("pairing.changed", {}) in events + assert ("platforms.changed", {}) not in events + + +def test_pairing_signal_follows_a_profile_store(watcher_home): + """Each profile keeps its own whitelist, and the page can be scoped to any.""" + home, events = watcher_home + store = home / "profiles" / "work" / "platforms" / "pairing" + store.mkdir(parents=True) + server._broadcast_watched_changes(now=0.0) + + (store / "telegram-approved.json").write_text('{"u1": {"user_id": "u1"}}') + server._broadcast_watched_changes(now=10.0) + + assert ("pairing.changed", {}) in events + + +def test_rate_limit_churn_does_not_broadcast_pairing_changed(watcher_home): + """_rate_limits.json moves on every unauthorized DM, including ones that + produce no new row — signalling on it would refetch for nothing.""" + home, events = watcher_home + store = home / "platforms" / "pairing" + store.mkdir(parents=True) + (store / "telegram-pending.json").write_text("{}") + server._broadcast_watched_changes(now=0.0) + + (store / "_rate_limits.json").write_text('{"telegram:1": 123}') + server._broadcast_watched_changes(now=10.0) + + assert ("pairing.changed", {}) not in events + + def test_sessions_floor_coalesces_burst_but_keeps_trailing_edge(watcher_home): home, events = watcher_home server._broadcast_watched_changes(now=0.0) diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 5cc0d87742f..b3a0ffa6187 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -3082,6 +3082,46 @@ def _platforms_sig(): return None +def _pairing_sig(): + """Newest mtime across every profile's pairing store. + + An unknown DMer's pending code is written by the messaging gateway — a + DIFFERENT process that never touches this gateway's transports — so the + files are the only shared signal. ``platforms.changed`` cannot stand in + for this: it tracks connect/disconnect/health, and a pairing request + moves nothing in gateway_state.json. + """ + home = _watcher_home() + sig = None + # Global store (legacy `pairing/` and consolidated `platforms/pairing/`) + # plus every named profile's own — the Messaging page can be scoped to any + # of them, and a request landing in a profile store must still tick. + roots = [home / "pairing", home / "platforms" / "pairing"] + try: + for profile_dir in (home / "profiles").iterdir(): + roots.append(profile_dir / "pairing") + roots.append(profile_dir / "platforms" / "pairing") + except OSError: + pass + + for root in roots: + try: + entries = list(root.iterdir()) + except OSError: + continue + for entry in entries: + # Only the pending/approved ledgers — _rate_limits.json moves on + # every unauthorized DM, including ones that produce no new row. + if not entry.name.endswith(("-pending.json", "-approved.json")): + continue + try: + mtime = entry.stat().st_mtime_ns + except OSError: + continue + sig = mtime if sig is None else max(sig, mtime) + return sig + + # Watched change signals: event → (check interval, signature fn, payload fn). # Signatures are stat/dict-lookup cheap, same bar as the skin watcher; the # check interval keeps the pricier probes (pet resolves the active sheet off @@ -3091,6 +3131,7 @@ _CHANGE_WATCHES: dict[str, tuple[float, Any, Any]] = { "cron.changed": (1.0, _cron_sig, lambda: {}), "sessions.changed": (0.5, _sessions_sig, lambda: {}), "platforms.changed": (2.0, _platforms_sig, lambda: {}), + "pairing.changed": (2.0, _pairing_sig, lambda: {}), } # state.db moves on every message append during a streaming turn, and the