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