From 5c5504508cea606e508116920507b3cd7e227720 Mon Sep 17 00:00:00 2001 From: beastoin Date: Fri, 10 Apr 2026 09:41:02 +0000 Subject: [PATCH 01/45] fix(desktop): reduce chat message polling from 15s to 120s (#6500) The 15s chat poll interval was the single biggest API traffic contributor at 240 req/user/hour. Increasing to 120s cuts chat polling traffic by 87%. Co-Authored-By: Claude Opus 4.6 --- desktop/Desktop/Sources/Providers/ChatProvider.swift | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/desktop/Desktop/Sources/Providers/ChatProvider.swift b/desktop/Desktop/Sources/Providers/ChatProvider.swift index 032d98a0c6..e15f104fea 100644 --- a/desktop/Desktop/Sources/Providers/ChatProvider.swift +++ b/desktop/Desktop/Sources/Providers/ChatProvider.swift @@ -539,10 +539,9 @@ A screenshot may be attached — use it silently only if relevant. Never mention private var sessionGroupingObserver: AnyCancellable? // MARK: - Cross-Platform Message Polling - /// Polls for new messages from other platforms (mobile) every 15 seconds. - /// Similar to TasksStore's 30-second polling pattern. + /// Polls for new messages from other platforms (mobile) every 120 seconds. private var messagePollTimer: AnyCancellable? - private static let messagePollInterval: TimeInterval = 15.0 + private static let messagePollInterval: TimeInterval = 120.0 // MARK: - Streaming Buffer /// Accumulates text deltas during streaming and flushes them to the published @@ -638,7 +637,7 @@ A screenshot may be attached — use it silently only if relevant. Never mention } } - // Poll for new messages from other platforms (mobile) every 15 seconds + // Poll for new messages from other platforms (mobile) every 120 seconds messagePollTimer = Timer.publish(every: Self.messagePollInterval, on: .main, in: .common) .autoconnect() .sink { [weak self] _ in From 30af865c9152da1c9524e93ee93b42545dd0215a Mon Sep 17 00:00:00 2001 From: beastoin Date: Fri, 10 Apr 2026 09:41:11 +0000 Subject: [PATCH 02/45] fix(desktop): reduce task polling from 30s to 120s (#6500) Tasks already have an isActive page-visibility guard, so polling only fires when the tasks page is visible. Increasing interval from 30s to 120s further reduces unnecessary API traffic. Co-Authored-By: Claude Opus 4.6 --- desktop/Desktop/Sources/Stores/TasksStore.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/desktop/Desktop/Sources/Stores/TasksStore.swift b/desktop/Desktop/Sources/Stores/TasksStore.swift index 1a2c7bba42..41d8f81deb 100644 --- a/desktop/Desktop/Sources/Stores/TasksStore.swift +++ b/desktop/Desktop/Sources/Stores/TasksStore.swift @@ -161,8 +161,8 @@ class TasksStore: ObservableObject { // MARK: - Initialization private init() { - // Auto-refresh tasks every 30 seconds - Timer.publish(every: 30.0, on: .main, in: .common) + // Auto-refresh tasks every 120 seconds + Timer.publish(every: 120.0, on: .main, in: .common) .autoconnect() .sink { [weak self] _ in Task { await self?.refreshTasksIfNeeded() } From 899b8a87e368a725bc94f8a2502a63b70c625423 Mon Sep 17 00:00:00 2001 From: beastoin Date: Fri, 10 Apr 2026 09:41:16 +0000 Subject: [PATCH 03/45] fix(desktop): reduce memories polling from 30s to 120s (#6500) Memories already have an isActive page-visibility guard. Increasing the polling interval from 30s to 120s reduces background API traffic without affecting user experience. Co-Authored-By: Claude Opus 4.6 --- desktop/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/desktop/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift b/desktop/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift index db3e10b27b..7ea1086529 100644 --- a/desktop/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift +++ b/desktop/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift @@ -206,8 +206,8 @@ class MemoriesViewModel: ObservableObject { // MARK: - Initialization init() { - // Auto-refresh memories every 30 seconds - Timer.publish(every: 30.0, on: .main, in: .common) + // Auto-refresh memories every 120 seconds + Timer.publish(every: 120.0, on: .main, in: .common) .autoconnect() .sink { [weak self] _ in Task { await self?.refreshMemoriesIfNeeded() } From e611e855b2f1edb1b016b8358eaa004d9615a87e Mon Sep 17 00:00:00 2001 From: beastoin Date: Fri, 10 Apr 2026 09:41:22 +0000 Subject: [PATCH 04/45] fix(desktop): reduce conversation polling to 120s with activation cooldown (#6500) - Increase periodic conversation refresh from 30s to 120s - Add 60s cooldown on didBecomeActive to prevent cmd-tab spam - Skip getConversationsCount on periodic refreshes (halves timer traffic) - Conversations still refresh immediately on first app activation Co-Authored-By: Claude Opus 4.6 --- .../Desktop/Sources/MainWindow/DesktopHomeView.swift | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/desktop/Desktop/Sources/MainWindow/DesktopHomeView.swift b/desktop/Desktop/Sources/MainWindow/DesktopHomeView.swift index 3158011da0..b9b190525d 100644 --- a/desktop/Desktop/Sources/MainWindow/DesktopHomeView.swift +++ b/desktop/Desktop/Sources/MainWindow/DesktopHomeView.swift @@ -32,6 +32,7 @@ struct DesktopHomeView: View { @State private var showTryAskingPopup = false @State private var previousIndexBeforeSettings: Int = 0 @State private var logoPulse = false + @State private var lastActivationRefresh = Date.distantPast // Pre-loaded hero logo to avoid NSImage init crashes during SwiftUI body evaluation private static let heroLogoImage: NSImage? = { @@ -196,9 +197,13 @@ struct DesktopHomeView: View { await AgentVMService.shared.ensureProvisioned() } // Refresh conversations when app becomes active (e.g. switching back from another app) + // Cooldown: skip if last activation refresh was less than 60 seconds ago .onReceive( NotificationCenter.default.publisher(for: NSApplication.didBecomeActiveNotification) ) { _ in + let now = Date() + guard now.timeIntervalSince(lastActivationRefresh) >= 60 else { return } + lastActivationRefresh = now Task { await appState.refreshConversations() } // Auto-start monitoring when returning to app if screen analysis is enabled // but monitoring is not running. Handles the case where the user granted @@ -234,9 +239,9 @@ struct DesktopHomeView: View { } } } - // Periodic refresh every 30s to pick up conversations from other devices (e.g. Omi Glass) - .onReceive(Timer.publish(every: 30, on: .main, in: .common).autoconnect()) { _ in - Task { await appState.refreshConversations() } + // Periodic refresh every 120s to pick up conversations from other devices (e.g. Omi Glass) + .onReceive(Timer.publish(every: 120, on: .main, in: .common).autoconnect()) { _ in + Task { await appState.refreshConversations(skipCount: true) } } // On sign-out: reset @AppStorage-backed onboarding flag and stop transcription. // hasCompletedOnboarding must be set here (in a View) because @AppStorage From d834ea3b20101301731bbd8548e1b31716373d4c Mon Sep 17 00:00:00 2001 From: beastoin Date: Fri, 10 Apr 2026 09:41:28 +0000 Subject: [PATCH 05/45] fix(desktop): add skipCount parameter to refreshConversations (#6500) Allow callers to skip the separate getConversationsCount API call during periodic background refreshes. This halves the traffic from the conversation refresh timer without affecting user-triggered refreshes. Co-Authored-By: Claude Opus 4.6 --- desktop/Desktop/Sources/AppState.swift | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/desktop/Desktop/Sources/AppState.swift b/desktop/Desktop/Sources/AppState.swift index e6bb165214..22e6b2935c 100644 --- a/desktop/Desktop/Sources/AppState.swift +++ b/desktop/Desktop/Sources/AppState.swift @@ -1997,7 +1997,7 @@ class AppState: ObservableObject { /// Refresh conversations silently (for auto-refresh timer and app-activate). /// Fetches from API only, merges in-place, and only triggers @Published if data actually changed. - func refreshConversations() async { + func refreshConversations(skipCount: Bool = false) async { // Skip if user is signed out (tokens are cleared) guard AuthState.shared.isSignedIn else { return } // Skip if in auth backoff period (recent 401 errors) @@ -2057,14 +2057,16 @@ class AppState: ObservableObject { } } - // Update total count - do { - let count = try await APIClient.shared.getConversationsCount(includeDiscarded: false) - if totalConversationsCount != count { - totalConversationsCount = count + // Update total count (skipped during periodic background refreshes to halve traffic) + if !skipCount { + do { + let count = try await APIClient.shared.getConversationsCount(includeDiscarded: false) + if totalConversationsCount != count { + totalConversationsCount = count + } + } catch { + // Keep existing count } - } catch { - // Keep existing count } } From 6a31ed709a8a7c77c54714481cdd3a2b35bab419 Mon Sep 17 00:00:00 2001 From: beastoin Date: Fri, 10 Apr 2026 09:47:59 +0000 Subject: [PATCH 06/45] fix(desktop): add activation refresh for chat messages (#6500) Chat had no didBecomeActive refresh path, so with the 120s poll interval messages from mobile could be invisible for up to 2 minutes. Adding an activation observer ensures messages sync immediately when the user returns to the app, matching the conversation refresh behavior. Co-Authored-By: Claude Opus 4.6 --- desktop/Desktop/Sources/Providers/ChatProvider.swift | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/desktop/Desktop/Sources/Providers/ChatProvider.swift b/desktop/Desktop/Sources/Providers/ChatProvider.swift index e15f104fea..99b7dd6671 100644 --- a/desktop/Desktop/Sources/Providers/ChatProvider.swift +++ b/desktop/Desktop/Sources/Providers/ChatProvider.swift @@ -537,6 +537,7 @@ A screenshot may be attached — use it silently only if relevant. Never mention private var multiChatObserver: AnyCancellable? private var playwrightExtensionObserver: AnyCancellable? private var sessionGroupingObserver: AnyCancellable? + private var activationObserver: AnyCancellable? // MARK: - Cross-Platform Message Polling /// Polls for new messages from other platforms (mobile) every 120 seconds. @@ -646,6 +647,14 @@ A screenshot may be attached — use it silently only if relevant. Never mention } } + // Refresh messages when app becomes active (compensates for longer poll interval) + activationObserver = NotificationCenter.default.publisher(for: NSApplication.didBecomeActiveNotification) + .sink { [weak self] _ in + Task { @MainActor in + await self?.pollForNewMessages() + } + } + // Observe changes to Playwright extension mode setting — restart bridge to pick up new env vars playwrightExtensionObserver = UserDefaults.standard.publisher(for: \.playwrightUseExtension) .dropFirst() From 261c84c5f6cc83df00f8ae7be19e7c061159c3fa Mon Sep 17 00:00:00 2001 From: beastoin Date: Fri, 10 Apr 2026 09:55:52 +0000 Subject: [PATCH 07/45] fix(desktop): centralize polling intervals in PollingConfig (#6500) Extract all polling interval constants into a single PollingConfig enum. This makes intervals testable and provides a single source of truth for all auto-refresh timers across the desktop app. Co-Authored-By: Claude Opus 4.6 --- desktop/Desktop/Sources/PollingConfig.swift | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 desktop/Desktop/Sources/PollingConfig.swift diff --git a/desktop/Desktop/Sources/PollingConfig.swift b/desktop/Desktop/Sources/PollingConfig.swift new file mode 100644 index 0000000000..d1e1d0c2ed --- /dev/null +++ b/desktop/Desktop/Sources/PollingConfig.swift @@ -0,0 +1,20 @@ +import Foundation + +/// Centralized polling interval constants for all desktop auto-refresh timers. +/// Changing values here updates every timer that references them. +enum PollingConfig { + /// Chat message polling interval (seconds). Syncs messages from other platforms. + static let chatPollInterval: TimeInterval = 120.0 + + /// Tasks auto-refresh interval (seconds). Guarded by page visibility. + static let tasksPollInterval: TimeInterval = 120.0 + + /// Memories auto-refresh interval (seconds). Guarded by page visibility. + static let memoriesPollInterval: TimeInterval = 120.0 + + /// Conversations auto-refresh interval (seconds). + static let conversationsPollInterval: TimeInterval = 120.0 + + /// Minimum time between app-activation conversation refreshes (seconds). + static let activationCooldown: TimeInterval = 60.0 +} From 64ff419bd528788b6d4dee1dca6f0994bdb61b53 Mon Sep 17 00:00:00 2001 From: beastoin Date: Fri, 10 Apr 2026 09:55:57 +0000 Subject: [PATCH 08/45] fix(desktop): wire ChatProvider to PollingConfig (#6500) Use PollingConfig.chatPollInterval instead of inline constant. Co-Authored-By: Claude Opus 4.6 --- desktop/Desktop/Sources/Providers/ChatProvider.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/desktop/Desktop/Sources/Providers/ChatProvider.swift b/desktop/Desktop/Sources/Providers/ChatProvider.swift index 99b7dd6671..cd9697aad5 100644 --- a/desktop/Desktop/Sources/Providers/ChatProvider.swift +++ b/desktop/Desktop/Sources/Providers/ChatProvider.swift @@ -542,7 +542,7 @@ A screenshot may be attached — use it silently only if relevant. Never mention // MARK: - Cross-Platform Message Polling /// Polls for new messages from other platforms (mobile) every 120 seconds. private var messagePollTimer: AnyCancellable? - private static let messagePollInterval: TimeInterval = 120.0 + private static let messagePollInterval: TimeInterval = PollingConfig.chatPollInterval // MARK: - Streaming Buffer /// Accumulates text deltas during streaming and flushes them to the published From 73ebc8a6f158079d19b37a23020f0ecba1521f38 Mon Sep 17 00:00:00 2001 From: beastoin Date: Fri, 10 Apr 2026 09:56:02 +0000 Subject: [PATCH 09/45] fix(desktop): wire TasksStore to PollingConfig (#6500) Use PollingConfig.tasksPollInterval instead of inline constant. Co-Authored-By: Claude Opus 4.6 --- desktop/Desktop/Sources/Stores/TasksStore.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/desktop/Desktop/Sources/Stores/TasksStore.swift b/desktop/Desktop/Sources/Stores/TasksStore.swift index 41d8f81deb..8a1da47345 100644 --- a/desktop/Desktop/Sources/Stores/TasksStore.swift +++ b/desktop/Desktop/Sources/Stores/TasksStore.swift @@ -161,8 +161,8 @@ class TasksStore: ObservableObject { // MARK: - Initialization private init() { - // Auto-refresh tasks every 120 seconds - Timer.publish(every: 120.0, on: .main, in: .common) + // Auto-refresh tasks periodically + Timer.publish(every: PollingConfig.tasksPollInterval, on: .main, in: .common) .autoconnect() .sink { [weak self] _ in Task { await self?.refreshTasksIfNeeded() } From c30f4bf3f99e0e4878a5dabdcf644313eff15139 Mon Sep 17 00:00:00 2001 From: beastoin Date: Fri, 10 Apr 2026 09:56:07 +0000 Subject: [PATCH 10/45] fix(desktop): wire MemoriesPage to PollingConfig (#6500) Use PollingConfig.memoriesPollInterval instead of inline constant. Co-Authored-By: Claude Opus 4.6 --- desktop/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/desktop/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift b/desktop/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift index 7ea1086529..f439ac48fc 100644 --- a/desktop/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift +++ b/desktop/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift @@ -206,8 +206,8 @@ class MemoriesViewModel: ObservableObject { // MARK: - Initialization init() { - // Auto-refresh memories every 120 seconds - Timer.publish(every: 120.0, on: .main, in: .common) + // Auto-refresh memories periodically + Timer.publish(every: PollingConfig.memoriesPollInterval, on: .main, in: .common) .autoconnect() .sink { [weak self] _ in Task { await self?.refreshMemoriesIfNeeded() } From 2b4051a7979c6cec38234140d24d5b88f786240d Mon Sep 17 00:00:00 2001 From: beastoin Date: Fri, 10 Apr 2026 09:56:13 +0000 Subject: [PATCH 11/45] fix(desktop): wire DesktopHomeView to PollingConfig (#6500) Use PollingConfig.conversationsPollInterval and activationCooldown instead of inline constants. Co-Authored-By: Claude Opus 4.6 --- desktop/Desktop/Sources/MainWindow/DesktopHomeView.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/desktop/Desktop/Sources/MainWindow/DesktopHomeView.swift b/desktop/Desktop/Sources/MainWindow/DesktopHomeView.swift index b9b190525d..d93b96efd8 100644 --- a/desktop/Desktop/Sources/MainWindow/DesktopHomeView.swift +++ b/desktop/Desktop/Sources/MainWindow/DesktopHomeView.swift @@ -202,7 +202,7 @@ struct DesktopHomeView: View { NotificationCenter.default.publisher(for: NSApplication.didBecomeActiveNotification) ) { _ in let now = Date() - guard now.timeIntervalSince(lastActivationRefresh) >= 60 else { return } + guard now.timeIntervalSince(lastActivationRefresh) >= PollingConfig.activationCooldown else { return } lastActivationRefresh = now Task { await appState.refreshConversations() } // Auto-start monitoring when returning to app if screen analysis is enabled @@ -239,8 +239,8 @@ struct DesktopHomeView: View { } } } - // Periodic refresh every 120s to pick up conversations from other devices (e.g. Omi Glass) - .onReceive(Timer.publish(every: 120, on: .main, in: .common).autoconnect()) { _ in + // Periodic refresh to pick up conversations from other devices (e.g. Omi Glass) + .onReceive(Timer.publish(every: PollingConfig.conversationsPollInterval, on: .main, in: .common).autoconnect()) { _ in Task { await appState.refreshConversations(skipCount: true) } } // On sign-out: reset @AppStorage-backed onboarding flag and stop transcription. From e64dd98058e7d1fa8d89ff346bf7751f84899582 Mon Sep 17 00:00:00 2001 From: beastoin Date: Fri, 10 Apr 2026 09:56:19 +0000 Subject: [PATCH 12/45] test(desktop): add polling frequency and cooldown unit tests (#6500) Tests verify: - All polling intervals are 120s (chat, tasks, memories, conversations) - Activation cooldown is 60s - Cooldown boundary behavior (first activation, within cooldown, at boundary, after cooldown) Co-Authored-By: Claude Opus 4.6 --- .../Desktop/Tests/PollingFrequencyTests.swift | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 desktop/Desktop/Tests/PollingFrequencyTests.swift diff --git a/desktop/Desktop/Tests/PollingFrequencyTests.swift b/desktop/Desktop/Tests/PollingFrequencyTests.swift new file mode 100644 index 0000000000..777ab98c93 --- /dev/null +++ b/desktop/Desktop/Tests/PollingFrequencyTests.swift @@ -0,0 +1,65 @@ +import XCTest +@testable import Omi_Computer + +/// Tests for polling frequency reduction (#6500). +/// Verifies that polling intervals are at their target values (120s) +/// and that the activation cooldown logic works correctly. +final class PollingFrequencyTests: XCTestCase { + + // MARK: - Polling Interval Constants + + func testChatPollIntervalIs120Seconds() { + // ChatProvider.messagePollInterval is private, so verify via the config constant + XCTAssertEqual(PollingConfig.chatPollInterval, 120.0, "Chat poll interval should be 120s") + } + + func testTasksPollIntervalIs120Seconds() { + XCTAssertEqual(PollingConfig.tasksPollInterval, 120.0, "Tasks poll interval should be 120s") + } + + func testMemoriesPollIntervalIs120Seconds() { + XCTAssertEqual(PollingConfig.memoriesPollInterval, 120.0, "Memories poll interval should be 120s") + } + + func testConversationsPollIntervalIs120Seconds() { + XCTAssertEqual(PollingConfig.conversationsPollInterval, 120.0, "Conversations poll interval should be 120s") + } + + // MARK: - Activation Cooldown + + func testActivationCooldownIs60Seconds() { + XCTAssertEqual(PollingConfig.activationCooldown, 60.0, "Activation cooldown should be 60s") + } + + func testFirstActivationAlwaysAllowed() { + // distantPast means no previous activation — should always be allowed + let lastRefresh = Date.distantPast + let now = Date() + let elapsed = now.timeIntervalSince(lastRefresh) + XCTAssertGreaterThanOrEqual(elapsed, PollingConfig.activationCooldown) + } + + func testActivationWithinCooldownIsBlocked() { + let lastRefresh = Date() + // 30 seconds later — within cooldown + let now = lastRefresh.addingTimeInterval(30) + let elapsed = now.timeIntervalSince(lastRefresh) + XCTAssertLessThan(elapsed, PollingConfig.activationCooldown) + } + + func testActivationAtCooldownBoundaryIsAllowed() { + let lastRefresh = Date() + // Exactly 60 seconds later — at boundary + let now = lastRefresh.addingTimeInterval(60) + let elapsed = now.timeIntervalSince(lastRefresh) + XCTAssertGreaterThanOrEqual(elapsed, PollingConfig.activationCooldown) + } + + func testActivationAfterCooldownIsAllowed() { + let lastRefresh = Date() + // 90 seconds later — past cooldown + let now = lastRefresh.addingTimeInterval(90) + let elapsed = now.timeIntervalSince(lastRefresh) + XCTAssertGreaterThanOrEqual(elapsed, PollingConfig.activationCooldown) + } +} From 70d14834de3322501cbe9051d4279976fee92432 Mon Sep 17 00:00:00 2001 From: beastoin Date: Fri, 10 Apr 2026 10:00:24 +0000 Subject: [PATCH 13/45] fix(desktop): scope activation cooldown to conversation refresh only (#6500) The cooldown guard was blocking the entire didBecomeActive handler, including the screen-analysis recovery path. Now the cooldown only gates refreshConversations() while screen-recording permission checks and monitoring restarts still run on every activation. Co-Authored-By: Claude Opus 4.6 --- desktop/Desktop/Sources/MainWindow/DesktopHomeView.swift | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/desktop/Desktop/Sources/MainWindow/DesktopHomeView.swift b/desktop/Desktop/Sources/MainWindow/DesktopHomeView.swift index d93b96efd8..27c066cb02 100644 --- a/desktop/Desktop/Sources/MainWindow/DesktopHomeView.swift +++ b/desktop/Desktop/Sources/MainWindow/DesktopHomeView.swift @@ -197,14 +197,15 @@ struct DesktopHomeView: View { await AgentVMService.shared.ensureProvisioned() } // Refresh conversations when app becomes active (e.g. switching back from another app) - // Cooldown: skip if last activation refresh was less than 60 seconds ago .onReceive( NotificationCenter.default.publisher(for: NSApplication.didBecomeActiveNotification) ) { _ in + // Cooldown: only refresh conversations if last activation was 60+ seconds ago let now = Date() - guard now.timeIntervalSince(lastActivationRefresh) >= PollingConfig.activationCooldown else { return } - lastActivationRefresh = now - Task { await appState.refreshConversations() } + if now.timeIntervalSince(lastActivationRefresh) >= PollingConfig.activationCooldown { + lastActivationRefresh = now + Task { await appState.refreshConversations() } + } // Auto-start monitoring when returning to app if screen analysis is enabled // but monitoring is not running. Handles the case where the user granted // screen recording permission in System Settings and switched back. From 60f14b809f9ac8d8961147ad5c12da75ecb35373 Mon Sep 17 00:00:00 2001 From: beastoin Date: Tue, 14 Apr 2026 12:07:41 +0000 Subject: [PATCH 14/45] refactor(desktop): remove polling intervals from PollingConfig (#6500) All periodic polling timers eliminated. PollingConfig now only holds the activation cooldown constant. Co-Authored-By: Claude Opus 4.6 --- desktop/Desktop/Sources/PollingConfig.swift | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/desktop/Desktop/Sources/PollingConfig.swift b/desktop/Desktop/Sources/PollingConfig.swift index d1e1d0c2ed..24d35330cf 100644 --- a/desktop/Desktop/Sources/PollingConfig.swift +++ b/desktop/Desktop/Sources/PollingConfig.swift @@ -1,20 +1,10 @@ import Foundation -/// Centralized polling interval constants for all desktop auto-refresh timers. -/// Changing values here updates every timer that references them. +/// Centralized configuration for event-driven data refresh. +/// All periodic polling timers have been removed — data refreshes on +/// app activation (didBecomeActiveNotification) and manual Cmd+R. enum PollingConfig { - /// Chat message polling interval (seconds). Syncs messages from other platforms. - static let chatPollInterval: TimeInterval = 120.0 - - /// Tasks auto-refresh interval (seconds). Guarded by page visibility. - static let tasksPollInterval: TimeInterval = 120.0 - - /// Memories auto-refresh interval (seconds). Guarded by page visibility. - static let memoriesPollInterval: TimeInterval = 120.0 - - /// Conversations auto-refresh interval (seconds). - static let conversationsPollInterval: TimeInterval = 120.0 - /// Minimum time between app-activation conversation refreshes (seconds). + /// Prevents cmd-tab spam from flooding the API. static let activationCooldown: TimeInterval = 60.0 } From a565e1ddd4f2a0e33e792bbddbffe1d4b8aa9342 Mon Sep 17 00:00:00 2001 From: beastoin Date: Tue, 14 Apr 2026 12:07:45 +0000 Subject: [PATCH 15/45] feat(desktop): add refreshAllData notification for Cmd+R (#6500) New notification name that all data providers observe to refresh on demand, replacing periodic polling timers. Co-Authored-By: Claude Opus 4.6 --- desktop/Desktop/Sources/AppState.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/desktop/Desktop/Sources/AppState.swift b/desktop/Desktop/Sources/AppState.swift index 22e6b2935c..2bcf162975 100644 --- a/desktop/Desktop/Sources/AppState.swift +++ b/desktop/Desktop/Sources/AppState.swift @@ -3067,6 +3067,8 @@ extension Notification.Name { static let navigateToTasks = Notification.Name("navigateToTasks") /// Posted by keyboard shortcuts to navigate sidebar. userInfo: ["rawValue": Int] static let navigateToSidebarItem = Notification.Name("navigateToSidebarItem") + /// Posted by Cmd+R to refresh all data (conversations, chat, tasks, memories) + static let refreshAllData = Notification.Name("refreshAllData") /// Posted by the local desktop automation bridge to request semantic navigation. static let desktopAutomationNavigateRequested = Notification.Name( "desktopAutomationNavigateRequested") From 1a48cbef3009af6abc86313456d9d4b4bd5bb1c9 Mon Sep 17 00:00:00 2001 From: beastoin Date: Tue, 14 Apr 2026 12:07:49 +0000 Subject: [PATCH 16/45] feat(desktop): add Cmd+R keyboard shortcut for manual refresh (#6500) Global shortcut posts refreshAllData notification, triggering all data providers to fetch fresh data on demand. Co-Authored-By: Claude Opus 4.6 --- desktop/Desktop/Sources/OmiApp.swift | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/desktop/Desktop/Sources/OmiApp.swift b/desktop/Desktop/Sources/OmiApp.swift index e20eaad665..5fda969184 100644 --- a/desktop/Desktop/Sources/OmiApp.swift +++ b/desktop/Desktop/Sources/OmiApp.swift @@ -191,6 +191,13 @@ struct OMIApp: App { } .keyboardShortcut(",", modifiers: .command) } + + CommandGroup(after: .toolbar) { + Button("Refresh") { + NotificationCenter.default.post(name: .refreshAllData, object: nil) + } + .keyboardShortcut("r", modifiers: .command) + } } // Note: Menu bar is now handled by NSStatusBar in AppDelegate.setupMenuBar() From 5f0351b60e3bd64ae900766673198ea21091756c Mon Sep 17 00:00:00 2001 From: beastoin Date: Tue, 14 Apr 2026 12:07:55 +0000 Subject: [PATCH 17/45] fix(desktop): remove chat polling timer, add Cmd+R observer (#6500) Replace 120s periodic poll with event-driven refresh: app activation observer (already existed) + Cmd+R manual refresh. Eliminates 720 unnecessary API calls per user per day. Co-Authored-By: Claude Opus 4.6 --- .../Desktop/Sources/Providers/ChatProvider.swift | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/desktop/Desktop/Sources/Providers/ChatProvider.swift b/desktop/Desktop/Sources/Providers/ChatProvider.swift index cd9697aad5..334fdf2ad0 100644 --- a/desktop/Desktop/Sources/Providers/ChatProvider.swift +++ b/desktop/Desktop/Sources/Providers/ChatProvider.swift @@ -539,10 +539,7 @@ A screenshot may be attached — use it silently only if relevant. Never mention private var sessionGroupingObserver: AnyCancellable? private var activationObserver: AnyCancellable? - // MARK: - Cross-Platform Message Polling - /// Polls for new messages from other platforms (mobile) every 120 seconds. - private var messagePollTimer: AnyCancellable? - private static let messagePollInterval: TimeInterval = PollingConfig.chatPollInterval + private var refreshAllObserver: AnyCancellable? // MARK: - Streaming Buffer /// Accumulates text deltas during streaming and flushes them to the published @@ -638,17 +635,16 @@ A screenshot may be attached — use it silently only if relevant. Never mention } } - // Poll for new messages from other platforms (mobile) every 120 seconds - messagePollTimer = Timer.publish(every: Self.messagePollInterval, on: .main, in: .common) - .autoconnect() + // Refresh messages when app becomes active + activationObserver = NotificationCenter.default.publisher(for: NSApplication.didBecomeActiveNotification) .sink { [weak self] _ in Task { @MainActor in await self?.pollForNewMessages() } } - // Refresh messages when app becomes active (compensates for longer poll interval) - activationObserver = NotificationCenter.default.publisher(for: NSApplication.didBecomeActiveNotification) + // Cmd+R: refresh messages on demand + refreshAllObserver = NotificationCenter.default.publisher(for: .refreshAllData) .sink { [weak self] _ in Task { @MainActor in await self?.pollForNewMessages() From 6b6a7917c799d909a52a5e522a5417938914d5eb Mon Sep 17 00:00:00 2001 From: beastoin Date: Tue, 14 Apr 2026 12:07:59 +0000 Subject: [PATCH 18/45] fix(desktop): replace conversation polling timer with Cmd+R observer (#6500) Remove periodic 120s conversation refresh timer. Conversations now refresh on app activation (with 60s cooldown) and Cmd+R only. Co-Authored-By: Claude Opus 4.6 --- desktop/Desktop/Sources/MainWindow/DesktopHomeView.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/desktop/Desktop/Sources/MainWindow/DesktopHomeView.swift b/desktop/Desktop/Sources/MainWindow/DesktopHomeView.swift index 27c066cb02..5521d711cf 100644 --- a/desktop/Desktop/Sources/MainWindow/DesktopHomeView.swift +++ b/desktop/Desktop/Sources/MainWindow/DesktopHomeView.swift @@ -240,9 +240,9 @@ struct DesktopHomeView: View { } } } - // Periodic refresh to pick up conversations from other devices (e.g. Omi Glass) - .onReceive(Timer.publish(every: PollingConfig.conversationsPollInterval, on: .main, in: .common).autoconnect()) { _ in - Task { await appState.refreshConversations(skipCount: true) } + // Cmd+R: refresh all data (conversations, chat, tasks, memories) + .onReceive(NotificationCenter.default.publisher(for: .refreshAllData)) { _ in + Task { await appState.refreshConversations() } } // On sign-out: reset @AppStorage-backed onboarding flag and stop transcription. // hasCompletedOnboarding must be set here (in a View) because @AppStorage From 469eaff05d9bf0dd37526352c1de875affcc8142 Mon Sep 17 00:00:00 2001 From: beastoin Date: Tue, 14 Apr 2026 12:08:03 +0000 Subject: [PATCH 19/45] fix(desktop): replace task polling timer with activation + Cmd+R (#6500) Remove periodic 120s task refresh timer. Tasks now refresh on app activation, page visibility, and Cmd+R. Co-Authored-By: Claude Opus 4.6 --- desktop/Desktop/Sources/Stores/TasksStore.swift | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/desktop/Desktop/Sources/Stores/TasksStore.swift b/desktop/Desktop/Sources/Stores/TasksStore.swift index 8a1da47345..7ddab45cd4 100644 --- a/desktop/Desktop/Sources/Stores/TasksStore.swift +++ b/desktop/Desktop/Sources/Stores/TasksStore.swift @@ -161,9 +161,15 @@ class TasksStore: ObservableObject { // MARK: - Initialization private init() { - // Auto-refresh tasks periodically - Timer.publish(every: PollingConfig.tasksPollInterval, on: .main, in: .common) - .autoconnect() + // Refresh tasks when app becomes active + NotificationCenter.default.publisher(for: NSApplication.didBecomeActiveNotification) + .sink { [weak self] _ in + Task { await self?.refreshTasksIfNeeded() } + } + .store(in: &cancellables) + + // Cmd+R: refresh tasks on demand + NotificationCenter.default.publisher(for: .refreshAllData) .sink { [weak self] _ in Task { await self?.refreshTasksIfNeeded() } } From c47d15a3f16e4ed4ed3e477af7bb11fbe7044978 Mon Sep 17 00:00:00 2001 From: beastoin Date: Tue, 14 Apr 2026 12:08:07 +0000 Subject: [PATCH 20/45] fix(desktop): replace memories polling timer with activation + Cmd+R (#6500) Remove periodic 120s memories refresh timer. Memories now refresh on app activation, page visibility, and Cmd+R. Co-Authored-By: Claude Opus 4.6 --- .../Sources/MainWindow/Pages/MemoriesPage.swift | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/desktop/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift b/desktop/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift index f439ac48fc..21a362d75d 100644 --- a/desktop/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift +++ b/desktop/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift @@ -206,9 +206,15 @@ class MemoriesViewModel: ObservableObject { // MARK: - Initialization init() { - // Auto-refresh memories periodically - Timer.publish(every: PollingConfig.memoriesPollInterval, on: .main, in: .common) - .autoconnect() + // Refresh memories when app becomes active + NotificationCenter.default.publisher(for: NSApplication.didBecomeActiveNotification) + .sink { [weak self] _ in + Task { await self?.refreshMemoriesIfNeeded() } + } + .store(in: &cancellables) + + // Cmd+R: refresh memories on demand + NotificationCenter.default.publisher(for: .refreshAllData) .sink { [weak self] _ in Task { await self?.refreshMemoriesIfNeeded() } } From c4e802c9f3d05c0ac2f05e7233da2816cd42e971 Mon Sep 17 00:00:00 2001 From: beastoin Date: Tue, 14 Apr 2026 12:08:13 +0000 Subject: [PATCH 21/45] test(desktop): update tests for event-driven refresh architecture (#6500) Remove poll interval constant tests (constants no longer exist). Add test for refreshAllData notification name. Keep cooldown tests. Co-Authored-By: Claude Opus 4.6 --- .../Desktop/Tests/PollingFrequencyTests.swift | 35 +++++++++---------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/desktop/Desktop/Tests/PollingFrequencyTests.swift b/desktop/Desktop/Tests/PollingFrequencyTests.swift index 777ab98c93..2738343098 100644 --- a/desktop/Desktop/Tests/PollingFrequencyTests.swift +++ b/desktop/Desktop/Tests/PollingFrequencyTests.swift @@ -1,28 +1,17 @@ import XCTest @testable import Omi_Computer -/// Tests for polling frequency reduction (#6500). -/// Verifies that polling intervals are at their target values (120s) -/// and that the activation cooldown logic works correctly. +/// Tests for event-driven refresh architecture (#6500). +/// Verifies that all periodic polling timers have been removed and +/// that activation cooldown logic works correctly. final class PollingFrequencyTests: XCTestCase { - // MARK: - Polling Interval Constants + // MARK: - No Polling Timers - func testChatPollIntervalIs120Seconds() { - // ChatProvider.messagePollInterval is private, so verify via the config constant - XCTAssertEqual(PollingConfig.chatPollInterval, 120.0, "Chat poll interval should be 120s") - } - - func testTasksPollIntervalIs120Seconds() { - XCTAssertEqual(PollingConfig.tasksPollInterval, 120.0, "Tasks poll interval should be 120s") - } - - func testMemoriesPollIntervalIs120Seconds() { - XCTAssertEqual(PollingConfig.memoriesPollInterval, 120.0, "Memories poll interval should be 120s") - } - - func testConversationsPollIntervalIs120Seconds() { - XCTAssertEqual(PollingConfig.conversationsPollInterval, 120.0, "Conversations poll interval should be 120s") + func testPollingConfigHasNoPollIntervals() { + // PollingConfig should only contain activationCooldown — no poll intervals. + // If someone adds a poll interval constant, this test must be updated. + XCTAssertEqual(PollingConfig.activationCooldown, 60.0, "Activation cooldown should be 60s") } // MARK: - Activation Cooldown @@ -62,4 +51,12 @@ final class PollingFrequencyTests: XCTestCase { let elapsed = now.timeIntervalSince(lastRefresh) XCTAssertGreaterThanOrEqual(elapsed, PollingConfig.activationCooldown) } + + // MARK: - Refresh All Notification + + func testRefreshAllDataNotificationNameExists() { + // Verify the notification name is defined (Cmd+R triggers this) + let name = Notification.Name.refreshAllData + XCTAssertEqual(name.rawValue, "refreshAllData") + } } From 7e3ec82f26ee6ea5754b60769c6bc64e650dfe40 Mon Sep 17 00:00:00 2001 From: beastoin Date: Tue, 14 Apr 2026 12:16:07 +0000 Subject: [PATCH 22/45] fix(desktop): remove CrispManager polling timer, use activation + Cmd+R (#6500) Replace 120s Timer.scheduledTimer with didBecomeActiveNotification and refreshAllData observers. Eliminates the last periodic API polling timer in the desktop app. Co-Authored-By: Claude Opus 4.6 --- .../Sources/MainWindow/CrispManager.swift | 39 +++++++++++-------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/desktop/Desktop/Sources/MainWindow/CrispManager.swift b/desktop/Desktop/Sources/MainWindow/CrispManager.swift index 53149f8738..deb1ae24a0 100644 --- a/desktop/Desktop/Sources/MainWindow/CrispManager.swift +++ b/desktop/Desktop/Sources/MainWindow/CrispManager.swift @@ -1,6 +1,7 @@ +import AppKit import Foundation -/// Polls the backend Crisp API endpoint for unread operator messages, +/// Fetches Crisp operator messages on app activation and Cmd+R, /// fires macOS notifications, and tracks unread count for the sidebar badge. @MainActor class CrispManager: ObservableObject { @@ -38,15 +39,15 @@ class CrispManager: ObservableObject { /// Track message texts we've already sent notifications for (to avoid duplicates) private var notifiedMessages = Set() - /// Polling timer - private var pollTimer: Timer? - - /// Whether polling has started + /// Whether start() has been called private var isStarted = false + private var activationObserver: NSObjectProtocol? + private var refreshAllObserver: NSObjectProtocol? + private init() {} - /// Call once after sign-in to start polling for Crisp messages + /// Call once after sign-in to fetch Crisp messages and listen for activation/Cmd+R func start() { guard !isStarted else { return } isStarted = true @@ -58,15 +59,19 @@ class CrispManager: ObservableObject { lastSeenTimestamp = UInt64(Date().timeIntervalSince1970) } - // Poll immediately, then every 2 minutes + // Fetch immediately on start pollForMessages() - pollTimer = Timer.scheduledTimer(withTimeInterval: 120, repeats: true) { [weak self] _ in - Task { @MainActor in - self?.pollForMessages() - } - } - log("CrispManager: started polling for operator messages") + // Refresh on app activation and Cmd+R (no periodic timer) + activationObserver = NotificationCenter.default.addObserver( + forName: NSApplication.didBecomeActiveNotification, object: nil, queue: .main + ) { [weak self] _ in Task { @MainActor in self?.pollForMessages() } } + + refreshAllObserver = NotificationCenter.default.addObserver( + forName: .refreshAllData, object: nil, queue: .main + ) { [weak self] _ in Task { @MainActor in self?.pollForMessages() } } + + log("CrispManager: started (event-driven, no polling timer)") } /// Mark messages as read (called when user opens Help tab) @@ -75,10 +80,12 @@ class CrispManager: ObservableObject { lastSeenTimestamp = latestOperatorTimestamp } - /// Stop polling (called on sign-out) + /// Stop observing (called on sign-out) func stop() { - pollTimer?.invalidate() - pollTimer = nil + if let obs = activationObserver { NotificationCenter.default.removeObserver(obs) } + if let obs = refreshAllObserver { NotificationCenter.default.removeObserver(obs) } + activationObserver = nil + refreshAllObserver = nil isStarted = false unreadCount = 0 // Clear persisted timestamps so next sign-in starts fresh From 1dbe9319721172f8bf9d1a75642b0c03549e6f32 Mon Sep 17 00:00:00 2001 From: beastoin Date: Tue, 14 Apr 2026 12:16:13 +0000 Subject: [PATCH 23/45] fix(desktop): add in-flight guard to chat message sync (#6500) Prevent overlapping fetches when activation and Cmd+R fire back-to-back. The isPolling flag ensures only one fetch runs at a time, avoiding duplicate message insertion. Co-Authored-By: Claude Opus 4.6 --- desktop/Desktop/Sources/Providers/ChatProvider.swift | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/desktop/Desktop/Sources/Providers/ChatProvider.swift b/desktop/Desktop/Sources/Providers/ChatProvider.swift index 334fdf2ad0..4e0adaf5e1 100644 --- a/desktop/Desktop/Sources/Providers/ChatProvider.swift +++ b/desktop/Desktop/Sources/Providers/ChatProvider.swift @@ -1924,11 +1924,16 @@ A screenshot may be attached — use it silently only if relevant. Never mention isLoading = false } - // MARK: - Cross-Platform Message Polling + // MARK: - Cross-Platform Message Sync - /// Poll for new messages from other platforms (e.g. mobile). + /// Whether a poll is currently in-flight (prevents overlapping fetches) + private var isPolling = false + + /// Fetch new messages from other platforms (e.g. mobile). /// Merges new messages into the existing array without disrupting the UI. private func pollForNewMessages() async { + // Prevent overlapping fetches from activation + Cmd+R firing together + guard !isPolling else { return } // Skip if user is signed out (tokens are cleared) guard AuthState.shared.isSignedIn else { return } // Skip if in auth backoff period (recent 401 errors) @@ -1942,6 +1947,9 @@ A screenshot may be attached — use it silently only if relevant. Never mention // Skip if there's an active streaming message guard !messages.contains(where: { $0.isStreaming }) else { return } + isPolling = true + defer { isPolling = false } + do { let persistedMessages: [ChatMessageDB] From d1b833581d2a8537dc96d792a09220133bf30750 Mon Sep 17 00:00:00 2001 From: beastoin Date: Tue, 14 Apr 2026 12:28:52 +0000 Subject: [PATCH 24/45] test(desktop): improve polling frequency test coverage per tester feedback - Replace reimplemented Date arithmetic with production-equivalent comparisons - Add rapid activation throttling test (10 activations 1s apart) - Add cooldown reset-after-expiry sequence test - Add notification deliverability test - Add CrispManager lifecycle tests (start idempotency, stop cleanup, markAsRead) Co-Authored-By: Claude Opus 4.6 --- .../Desktop/Tests/PollingFrequencyTests.swift | 119 +++++++++++++++--- 1 file changed, 102 insertions(+), 17 deletions(-) diff --git a/desktop/Desktop/Tests/PollingFrequencyTests.swift b/desktop/Desktop/Tests/PollingFrequencyTests.swift index 2738343098..17c0c26898 100644 --- a/desktop/Desktop/Tests/PollingFrequencyTests.swift +++ b/desktop/Desktop/Tests/PollingFrequencyTests.swift @@ -20,43 +20,128 @@ final class PollingFrequencyTests: XCTestCase { XCTAssertEqual(PollingConfig.activationCooldown, 60.0, "Activation cooldown should be 60s") } + /// Uses the same comparison as production code in DesktopHomeView: + /// `now.timeIntervalSince(lastActivationRefresh) >= PollingConfig.activationCooldown` func testFirstActivationAlwaysAllowed() { - // distantPast means no previous activation — should always be allowed let lastRefresh = Date.distantPast let now = Date() - let elapsed = now.timeIntervalSince(lastRefresh) - XCTAssertGreaterThanOrEqual(elapsed, PollingConfig.activationCooldown) + // Mirror production: elapsed >= cooldown → refresh allowed + XCTAssertTrue( + now.timeIntervalSince(lastRefresh) >= PollingConfig.activationCooldown, + "First activation (distantPast) must always pass cooldown check" + ) } func testActivationWithinCooldownIsBlocked() { let lastRefresh = Date() - // 30 seconds later — within cooldown - let now = lastRefresh.addingTimeInterval(30) - let elapsed = now.timeIntervalSince(lastRefresh) - XCTAssertLessThan(elapsed, PollingConfig.activationCooldown) + let now = lastRefresh.addingTimeInterval(PollingConfig.activationCooldown - 30) + XCTAssertFalse( + now.timeIntervalSince(lastRefresh) >= PollingConfig.activationCooldown, + "Activation 30s before cooldown expires must be blocked" + ) } - func testActivationAtCooldownBoundaryIsAllowed() { + func testActivationAtExactCooldownBoundaryIsAllowed() { let lastRefresh = Date() - // Exactly 60 seconds later — at boundary - let now = lastRefresh.addingTimeInterval(60) - let elapsed = now.timeIntervalSince(lastRefresh) - XCTAssertGreaterThanOrEqual(elapsed, PollingConfig.activationCooldown) + let now = lastRefresh.addingTimeInterval(PollingConfig.activationCooldown) + // Production uses >= so exactly at boundary is allowed + XCTAssertTrue( + now.timeIntervalSince(lastRefresh) >= PollingConfig.activationCooldown, + "Activation at exactly cooldown boundary must be allowed (>= comparison)" + ) } func testActivationAfterCooldownIsAllowed() { let lastRefresh = Date() - // 90 seconds later — past cooldown - let now = lastRefresh.addingTimeInterval(90) - let elapsed = now.timeIntervalSince(lastRefresh) - XCTAssertGreaterThanOrEqual(elapsed, PollingConfig.activationCooldown) + let now = lastRefresh.addingTimeInterval(PollingConfig.activationCooldown + 30) + XCTAssertTrue( + now.timeIntervalSince(lastRefresh) >= PollingConfig.activationCooldown, + "Activation 30s past cooldown must be allowed" + ) + } + + func testRapidActivationsAreThrottled() { + // Simulate rapid cmd-tab: 10 activations 1 second apart + let start = Date() + var lastAllowed = Date.distantPast + var allowedCount = 0 + for i in 0..<10 { + let activation = start.addingTimeInterval(Double(i)) + if activation.timeIntervalSince(lastAllowed) >= PollingConfig.activationCooldown { + allowedCount += 1 + lastAllowed = activation + } + } + // Only the first activation should pass (subsequent ones are within 60s) + XCTAssertEqual(allowedCount, 1, "Rapid activations (1s apart) should only allow 1 refresh") + } + + func testCooldownResetsAfterExpiry() { + // First activation allowed, second blocked, third allowed after cooldown + let start = Date() + var lastAllowed = Date.distantPast + var results: [Bool] = [] + + let activations = [ + start, // t=0s: should be allowed + start.addingTimeInterval(30), // t=30s: within cooldown + start.addingTimeInterval(PollingConfig.activationCooldown + 1) // t=61s: past cooldown + ] + + for activation in activations { + let allowed = activation.timeIntervalSince(lastAllowed) >= PollingConfig.activationCooldown + results.append(allowed) + if allowed { lastAllowed = activation } + } + + XCTAssertEqual(results, [true, false, true], "Expected: allowed, blocked, allowed after cooldown") } // MARK: - Refresh All Notification func testRefreshAllDataNotificationNameExists() { - // Verify the notification name is defined (Cmd+R triggers this) let name = Notification.Name.refreshAllData XCTAssertEqual(name.rawValue, "refreshAllData") } + + func testRefreshAllDataNotificationIsReceivable() { + let expectation = XCTestExpectation(description: "Notification received") + let observer = NotificationCenter.default.addObserver( + forName: .refreshAllData, object: nil, queue: .main + ) { _ in expectation.fulfill() } + + NotificationCenter.default.post(name: .refreshAllData, object: nil) + wait(for: [expectation], timeout: 1.0) + NotificationCenter.default.removeObserver(observer) + } + + // MARK: - CrispManager Lifecycle + + @MainActor + func testCrispManagerStartIsIdempotent() { + let manager = CrispManager.shared + // start() has a guard — calling twice should not double-register observers + manager.start() + manager.start() // Second call is a no-op + manager.stop() // Clean up + XCTAssertEqual(manager.unreadCount, 0, "unreadCount should be 0 after stop") + } + + @MainActor + func testCrispManagerStopClearsState() { + let manager = CrispManager.shared + manager.start() + manager.stop() + XCTAssertEqual(manager.unreadCount, 0, "unreadCount should be 0 after stop") + XCTAssertFalse(manager.isViewingHelp, "isViewingHelp should be false after stop") + } + + @MainActor + func testCrispManagerMarkAsReadResetsCount() { + let manager = CrispManager.shared + manager.start() + manager.markAsRead() + XCTAssertEqual(manager.unreadCount, 0, "unreadCount should be 0 after markAsRead") + manager.stop() + } } From 4fbf7c836413de7294dd34613209592dc70c4f7a Mon Sep 17 00:00:00 2001 From: beastoin Date: Tue, 14 Apr 2026 15:22:35 +0000 Subject: [PATCH 25/45] refactor(desktop): extract activation cooldown predicate to PollingConfig (#6500) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per reviewer feedback: - Add PollingConfig.shouldAllowActivationRefresh(now:lastRefresh:) as the single source of truth for the >=activationCooldown check. - DesktopHomeView now calls the helper instead of inlining the comparison, so a >= → > regression in production is caught by the unit tests. - Remove race-prone CrispManager singleton lifecycle tests that asserted on state that was already zero before the call. - Add backward-clock-skew test and tighten the boundary test. Co-Authored-By: Claude Opus 4.6 --- .../Sources/MainWindow/DesktopHomeView.swift | 2 +- desktop/Desktop/Sources/PollingConfig.swift | 8 ++ .../Desktop/Tests/PollingFrequencyTests.swift | 107 +++++++----------- 3 files changed, 52 insertions(+), 65 deletions(-) diff --git a/desktop/Desktop/Sources/MainWindow/DesktopHomeView.swift b/desktop/Desktop/Sources/MainWindow/DesktopHomeView.swift index 5521d711cf..2a09ec7584 100644 --- a/desktop/Desktop/Sources/MainWindow/DesktopHomeView.swift +++ b/desktop/Desktop/Sources/MainWindow/DesktopHomeView.swift @@ -202,7 +202,7 @@ struct DesktopHomeView: View { ) { _ in // Cooldown: only refresh conversations if last activation was 60+ seconds ago let now = Date() - if now.timeIntervalSince(lastActivationRefresh) >= PollingConfig.activationCooldown { + if PollingConfig.shouldAllowActivationRefresh(now: now, lastRefresh: lastActivationRefresh) { lastActivationRefresh = now Task { await appState.refreshConversations() } } diff --git a/desktop/Desktop/Sources/PollingConfig.swift b/desktop/Desktop/Sources/PollingConfig.swift index 24d35330cf..63903f8d6d 100644 --- a/desktop/Desktop/Sources/PollingConfig.swift +++ b/desktop/Desktop/Sources/PollingConfig.swift @@ -7,4 +7,12 @@ enum PollingConfig { /// Minimum time between app-activation conversation refreshes (seconds). /// Prevents cmd-tab spam from flooding the API. static let activationCooldown: TimeInterval = 60.0 + + /// Returns `true` when enough time has passed since `lastRefresh` to allow + /// another activation-triggered refresh. Used by DesktopHomeView to throttle + /// didBecomeActiveNotification bursts. Shared between production and tests + /// so a regression (e.g. `>=` → `>`) is caught by the unit tests. + static func shouldAllowActivationRefresh(now: Date = Date(), lastRefresh: Date) -> Bool { + now.timeIntervalSince(lastRefresh) >= activationCooldown + } } diff --git a/desktop/Desktop/Tests/PollingFrequencyTests.swift b/desktop/Desktop/Tests/PollingFrequencyTests.swift index 17c0c26898..7f67005537 100644 --- a/desktop/Desktop/Tests/PollingFrequencyTests.swift +++ b/desktop/Desktop/Tests/PollingFrequencyTests.swift @@ -2,51 +2,47 @@ import XCTest @testable import Omi_Computer /// Tests for event-driven refresh architecture (#6500). -/// Verifies that all periodic polling timers have been removed and -/// that activation cooldown logic works correctly. +/// All polling timers are removed — refreshes happen on app activation +/// (didBecomeActiveNotification) and manual Cmd+R (.refreshAllData). +/// +/// Tests call `PollingConfig.shouldAllowActivationRefresh(now:lastRefresh:)`, +/// which is the same function used by DesktopHomeView's activation handler. +/// A regression in that predicate (e.g. `>=` → `>`) is caught here. final class PollingFrequencyTests: XCTestCase { // MARK: - No Polling Timers func testPollingConfigHasNoPollIntervals() { - // PollingConfig should only contain activationCooldown — no poll intervals. - // If someone adds a poll interval constant, this test must be updated. - XCTAssertEqual(PollingConfig.activationCooldown, 60.0, "Activation cooldown should be 60s") + // PollingConfig only exposes activationCooldown + the shared predicate. + // If someone reintroduces a poll interval constant, update this test. + XCTAssertEqual(PollingConfig.activationCooldown, 60.0) } - // MARK: - Activation Cooldown + // MARK: - Activation Cooldown (shared predicate) - func testActivationCooldownIs60Seconds() { - XCTAssertEqual(PollingConfig.activationCooldown, 60.0, "Activation cooldown should be 60s") - } - - /// Uses the same comparison as production code in DesktopHomeView: - /// `now.timeIntervalSince(lastActivationRefresh) >= PollingConfig.activationCooldown` func testFirstActivationAlwaysAllowed() { - let lastRefresh = Date.distantPast let now = Date() - // Mirror production: elapsed >= cooldown → refresh allowed XCTAssertTrue( - now.timeIntervalSince(lastRefresh) >= PollingConfig.activationCooldown, - "First activation (distantPast) must always pass cooldown check" + PollingConfig.shouldAllowActivationRefresh(now: now, lastRefresh: .distantPast), + "First activation (distantPast) must always be allowed" ) } func testActivationWithinCooldownIsBlocked() { let lastRefresh = Date() - let now = lastRefresh.addingTimeInterval(PollingConfig.activationCooldown - 30) + let now = lastRefresh.addingTimeInterval(PollingConfig.activationCooldown - 0.001) XCTAssertFalse( - now.timeIntervalSince(lastRefresh) >= PollingConfig.activationCooldown, - "Activation 30s before cooldown expires must be blocked" + PollingConfig.shouldAllowActivationRefresh(now: now, lastRefresh: lastRefresh), + "Activation just under cooldown must be blocked" ) } func testActivationAtExactCooldownBoundaryIsAllowed() { let lastRefresh = Date() let now = lastRefresh.addingTimeInterval(PollingConfig.activationCooldown) - // Production uses >= so exactly at boundary is allowed + // Production uses >= — boundary must be inclusive. Guards against >= → > regressions. XCTAssertTrue( - now.timeIntervalSince(lastRefresh) >= PollingConfig.activationCooldown, + PollingConfig.shouldAllowActivationRefresh(now: now, lastRefresh: lastRefresh), "Activation at exactly cooldown boundary must be allowed (>= comparison)" ) } @@ -55,53 +51,66 @@ final class PollingFrequencyTests: XCTestCase { let lastRefresh = Date() let now = lastRefresh.addingTimeInterval(PollingConfig.activationCooldown + 30) XCTAssertTrue( - now.timeIntervalSince(lastRefresh) >= PollingConfig.activationCooldown, + PollingConfig.shouldAllowActivationRefresh(now: now, lastRefresh: lastRefresh), "Activation 30s past cooldown must be allowed" ) } + func testBackwardClockSkewIsAllowed() { + // If system clock jumps backward, elapsed is negative < cooldown. + // Predicate returns false (blocked). This documents current behavior — + // a user with clock skew just won't get an auto-refresh that cycle. + let lastRefresh = Date() + let now = lastRefresh.addingTimeInterval(-10) + XCTAssertFalse( + PollingConfig.shouldAllowActivationRefresh(now: now, lastRefresh: lastRefresh), + "Negative elapsed (backward clock skew) must be treated as within cooldown" + ) + } + func testRapidActivationsAreThrottled() { - // Simulate rapid cmd-tab: 10 activations 1 second apart + // Simulate cmd-tab spam: 10 activations 1 second apart. + // Driven by the same predicate as production. let start = Date() var lastAllowed = Date.distantPast var allowedCount = 0 for i in 0..<10 { let activation = start.addingTimeInterval(Double(i)) - if activation.timeIntervalSince(lastAllowed) >= PollingConfig.activationCooldown { + if PollingConfig.shouldAllowActivationRefresh(now: activation, lastRefresh: lastAllowed) { allowedCount += 1 lastAllowed = activation } } - // Only the first activation should pass (subsequent ones are within 60s) XCTAssertEqual(allowedCount, 1, "Rapid activations (1s apart) should only allow 1 refresh") } func testCooldownResetsAfterExpiry() { - // First activation allowed, second blocked, third allowed after cooldown + // Sequence [allowed, blocked, allowed] — the third activation is past cooldown. let start = Date() var lastAllowed = Date.distantPast var results: [Bool] = [] let activations = [ - start, // t=0s: should be allowed - start.addingTimeInterval(30), // t=30s: within cooldown - start.addingTimeInterval(PollingConfig.activationCooldown + 1) // t=61s: past cooldown + start, // t=0: allowed (first) + start.addingTimeInterval(30), // t=30: blocked + start.addingTimeInterval(PollingConfig.activationCooldown + 1) // t=61: allowed ] for activation in activations { - let allowed = activation.timeIntervalSince(lastAllowed) >= PollingConfig.activationCooldown + let allowed = PollingConfig.shouldAllowActivationRefresh( + now: activation, lastRefresh: lastAllowed + ) results.append(allowed) if allowed { lastAllowed = activation } } - XCTAssertEqual(results, [true, false, true], "Expected: allowed, blocked, allowed after cooldown") + XCTAssertEqual(results, [true, false, true]) } // MARK: - Refresh All Notification func testRefreshAllDataNotificationNameExists() { - let name = Notification.Name.refreshAllData - XCTAssertEqual(name.rawValue, "refreshAllData") + XCTAssertEqual(Notification.Name.refreshAllData.rawValue, "refreshAllData") } func testRefreshAllDataNotificationIsReceivable() { @@ -109,39 +118,9 @@ final class PollingFrequencyTests: XCTestCase { let observer = NotificationCenter.default.addObserver( forName: .refreshAllData, object: nil, queue: .main ) { _ in expectation.fulfill() } + defer { NotificationCenter.default.removeObserver(observer) } NotificationCenter.default.post(name: .refreshAllData, object: nil) wait(for: [expectation], timeout: 1.0) - NotificationCenter.default.removeObserver(observer) - } - - // MARK: - CrispManager Lifecycle - - @MainActor - func testCrispManagerStartIsIdempotent() { - let manager = CrispManager.shared - // start() has a guard — calling twice should not double-register observers - manager.start() - manager.start() // Second call is a no-op - manager.stop() // Clean up - XCTAssertEqual(manager.unreadCount, 0, "unreadCount should be 0 after stop") - } - - @MainActor - func testCrispManagerStopClearsState() { - let manager = CrispManager.shared - manager.start() - manager.stop() - XCTAssertEqual(manager.unreadCount, 0, "unreadCount should be 0 after stop") - XCTAssertFalse(manager.isViewingHelp, "isViewingHelp should be false after stop") - } - - @MainActor - func testCrispManagerMarkAsReadResetsCount() { - let manager = CrispManager.shared - manager.start() - manager.markAsRead() - XCTAssertEqual(manager.unreadCount, 0, "unreadCount should be 0 after markAsRead") - manager.stop() } } From 0c9eeed859c3391fd1cbdfd9698e0d5f886363bb Mon Sep 17 00:00:00 2001 From: beastoin Date: Wed, 15 Apr 2026 04:03:15 +0000 Subject: [PATCH 26/45] refactor(desktop): remove unused skipCount from refreshConversations (#6500) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dead code after polling timer removal — all 7 call sites use the default. Simplifies the API to match the event-driven phase-1 scope. Co-Authored-By: Claude Opus 4.6 --- desktop/Desktop/Sources/AppState.swift | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/desktop/Desktop/Sources/AppState.swift b/desktop/Desktop/Sources/AppState.swift index 2bcf162975..5823d7b697 100644 --- a/desktop/Desktop/Sources/AppState.swift +++ b/desktop/Desktop/Sources/AppState.swift @@ -1995,9 +1995,9 @@ class AppState: ObservableObject { NotificationCenter.default.post(name: .conversationsPageDidLoad, object: nil) } - /// Refresh conversations silently (for auto-refresh timer and app-activate). + /// Refresh conversations silently (for app-activation and Cmd+R event-driven refreshes). /// Fetches from API only, merges in-place, and only triggers @Published if data actually changed. - func refreshConversations(skipCount: Bool = false) async { + func refreshConversations() async { // Skip if user is signed out (tokens are cleared) guard AuthState.shared.isSignedIn else { return } // Skip if in auth backoff period (recent 401 errors) @@ -2057,16 +2057,13 @@ class AppState: ObservableObject { } } - // Update total count (skipped during periodic background refreshes to halve traffic) - if !skipCount { - do { - let count = try await APIClient.shared.getConversationsCount(includeDiscarded: false) - if totalConversationsCount != count { - totalConversationsCount = count - } - } catch { - // Keep existing count + do { + let count = try await APIClient.shared.getConversationsCount(includeDiscarded: false) + if totalConversationsCount != count { + totalConversationsCount = count } + } catch { + // Keep existing count } } From 8cea4e1c8129f5e2b3d7932089511d55a32a1f72 Mon Sep 17 00:00:00 2001 From: beastoin Date: Wed, 15 Apr 2026 04:03:20 +0000 Subject: [PATCH 27/45] feat(desktop): add ReentrancyGate helper for in-flight guards (#6500) Extracts the single-entry guard pattern into a named, testable type. Used by ChatProvider.pollForNewMessages to prevent overlapping fetches when didBecomeActive + Cmd+R fire back-to-back. Co-Authored-By: Claude Opus 4.6 --- desktop/Desktop/Sources/ReentrancyGate.swift | 32 ++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 desktop/Desktop/Sources/ReentrancyGate.swift diff --git a/desktop/Desktop/Sources/ReentrancyGate.swift b/desktop/Desktop/Sources/ReentrancyGate.swift new file mode 100644 index 0000000000..6f932e9e2e --- /dev/null +++ b/desktop/Desktop/Sources/ReentrancyGate.swift @@ -0,0 +1,32 @@ +import Foundation + +/// Single-entry reentrancy gate for preventing overlapping async operations. +/// +/// Use when two independent triggers (e.g. `didBecomeActive` + Cmd+R) can fire +/// back-to-back and would otherwise cause duplicate fetches/inserts. Call +/// `tryEnter()` at the start of the critical section — if it returns `false`, +/// another caller is already in-flight and the current caller should bail out. +/// Always pair `tryEnter()` with `exit()` via `defer` so the gate reopens even +/// on thrown errors or early returns. +/// +/// Tested in `ReentrancyGateTests`. +@MainActor +final class ReentrancyGate { + private var isInFlight = false + + /// Attempts to enter the critical section. + /// - Returns: `true` if the caller acquired the gate (must call `exit()` when done), + /// `false` if another operation is already in-flight (caller should skip its work). + func tryEnter() -> Bool { + guard !isInFlight else { return false } + isInFlight = true + return true + } + + /// Releases the gate. Safe to call even if `tryEnter()` returned `false` + /// (no-op in that case — but callers should only `exit()` when their matching + /// `tryEnter()` returned `true`). + func exit() { + isInFlight = false + } +} From 9ee4165f5b0e867fdf44b4672af1f3a9f7390663 Mon Sep 17 00:00:00 2001 From: beastoin Date: Wed, 15 Apr 2026 04:03:21 +0000 Subject: [PATCH 28/45] refactor(desktop): use ReentrancyGate for pollForNewMessages (#6500) Replaces the ad-hoc isPolling bool + guard/defer pattern with ReentrancyGate for clearer semantics and unit-testability. Co-Authored-By: Claude Opus 4.6 --- desktop/Desktop/Sources/Providers/ChatProvider.swift | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/desktop/Desktop/Sources/Providers/ChatProvider.swift b/desktop/Desktop/Sources/Providers/ChatProvider.swift index 4e0adaf5e1..002717996e 100644 --- a/desktop/Desktop/Sources/Providers/ChatProvider.swift +++ b/desktop/Desktop/Sources/Providers/ChatProvider.swift @@ -1926,14 +1926,15 @@ A screenshot may be attached — use it silently only if relevant. Never mention // MARK: - Cross-Platform Message Sync - /// Whether a poll is currently in-flight (prevents overlapping fetches) - private var isPolling = false + /// Prevents overlapping fetches when activation + Cmd+R fire back-to-back. + private let pollGate = ReentrancyGate() /// Fetch new messages from other platforms (e.g. mobile). /// Merges new messages into the existing array without disrupting the UI. private func pollForNewMessages() async { // Prevent overlapping fetches from activation + Cmd+R firing together - guard !isPolling else { return } + guard pollGate.tryEnter() else { return } + defer { pollGate.exit() } // Skip if user is signed out (tokens are cleared) guard AuthState.shared.isSignedIn else { return } // Skip if in auth backoff period (recent 401 errors) @@ -1947,9 +1948,6 @@ A screenshot may be attached — use it silently only if relevant. Never mention // Skip if there's an active streaming message guard !messages.contains(where: { $0.isStreaming }) else { return } - isPolling = true - defer { isPolling = false } - do { let persistedMessages: [ChatMessageDB] From 335d843d633500dc2736f9d9d81e4ec3a91d765b Mon Sep 17 00:00:00 2001 From: beastoin Date: Wed, 15 Apr 2026 04:03:25 +0000 Subject: [PATCH 29/45] refactor(desktop): expose CrispManager lifecycle fields for tests (#6500) Drops private on isStarted, activationObserver, refreshAllObserver, and the UserDefaults-backed timestamps so CrispManagerLifecycleTests can assert start() idempotency, stop() observer cleanup, and markAsRead() timestamp advancement. Co-Authored-By: Claude Opus 4.6 --- .../Sources/MainWindow/CrispManager.swift | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/desktop/Desktop/Sources/MainWindow/CrispManager.swift b/desktop/Desktop/Sources/MainWindow/CrispManager.swift index deb1ae24a0..f1556dd1da 100644 --- a/desktop/Desktop/Sources/MainWindow/CrispManager.swift +++ b/desktop/Desktop/Sources/MainWindow/CrispManager.swift @@ -24,14 +24,16 @@ class CrispManager: ObservableObject { /// Timestamp of the most recent operator message we've already notified about. /// Persisted to UserDefaults so unread messages survive app restarts. /// Stored as Double because UserDefaults can't round-trip UInt64. - private var lastSeenTimestamp: UInt64 { + /// Non-`private` so `CrispManagerLifecycleTests` can assert `markAsRead()` advances it. + var lastSeenTimestamp: UInt64 { get { UInt64(UserDefaults.standard.double(forKey: "crisp_lastSeenTimestamp")) } set { UserDefaults.standard.set(Double(newValue), forKey: "crisp_lastSeenTimestamp") } } /// Track the latest operator message timestamp from any poll. /// Persisted to UserDefaults so we don't re-notify after restart. - private var latestOperatorTimestamp: UInt64 { + /// Non-`private` so `CrispManagerLifecycleTests` can seed it before `markAsRead()`. + var latestOperatorTimestamp: UInt64 { get { UInt64(UserDefaults.standard.double(forKey: "crisp_latestOperatorTimestamp")) } set { UserDefaults.standard.set(Double(newValue), forKey: "crisp_latestOperatorTimestamp") } } @@ -39,11 +41,13 @@ class CrispManager: ObservableObject { /// Track message texts we've already sent notifications for (to avoid duplicates) private var notifiedMessages = Set() - /// Whether start() has been called - private var isStarted = false + /// Whether start() has been called. Non-`private` so lifecycle tests can + /// assert idempotency after calling `start()` twice. + var isStarted = false - private var activationObserver: NSObjectProtocol? - private var refreshAllObserver: NSObjectProtocol? + /// Non-`private` so lifecycle tests can assert `stop()` clears both observers. + var activationObserver: NSObjectProtocol? + var refreshAllObserver: NSObjectProtocol? private init() {} From 8c56cb0ffdc0f616407559e108f1e877a9a0d0bb Mon Sep 17 00:00:00 2001 From: beastoin Date: Wed, 15 Apr 2026 04:03:27 +0000 Subject: [PATCH 30/45] test(desktop): add ReentrancyGate unit tests (#6500) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Covers: first enter, overlap blocking, enter-after-exit, repeated cycles, 3-way overlap → single entry, and spurious-exit safety. Addresses CP8 tester feedback that the isPolling in-flight guard had no regression coverage. Co-Authored-By: Claude Opus 4.6 --- .../Desktop/Tests/ReentrancyGateTests.swift | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 desktop/Desktop/Tests/ReentrancyGateTests.swift diff --git a/desktop/Desktop/Tests/ReentrancyGateTests.swift b/desktop/Desktop/Tests/ReentrancyGateTests.swift new file mode 100644 index 0000000000..17bf526d27 --- /dev/null +++ b/desktop/Desktop/Tests/ReentrancyGateTests.swift @@ -0,0 +1,79 @@ +import XCTest +@testable import Omi_Computer + +/// Tests for `ReentrancyGate`, the single-entry gate that prevents overlapping +/// `ChatProvider.pollForNewMessages()` fetches when `didBecomeActive` and +/// `.refreshAllData` fire back-to-back. +@MainActor +final class ReentrancyGateTests: XCTestCase { + + func testFirstEnterSucceeds() { + let gate = ReentrancyGate() + XCTAssertTrue(gate.tryEnter(), "First enter on a fresh gate must succeed") + } + + func testSecondEnterWithoutExitIsBlocked() { + let gate = ReentrancyGate() + XCTAssertTrue(gate.tryEnter()) + XCTAssertFalse( + gate.tryEnter(), + "Second enter without matching exit must be blocked (in-flight)" + ) + } + + func testEnterAfterExitSucceeds() { + let gate = ReentrancyGate() + XCTAssertTrue(gate.tryEnter()) + gate.exit() + XCTAssertTrue( + gate.tryEnter(), + "Enter after exit must succeed — gate should be reopened" + ) + } + + func testRepeatedEnterExitCyclesAllSucceed() { + // Simulates activation + Cmd+R firing sequentially (not overlapping) — + // every cycle should complete cleanly. + let gate = ReentrancyGate() + for cycle in 0..<5 { + XCTAssertTrue( + gate.tryEnter(), + "Cycle \(cycle): enter should succeed after prior exit" + ) + gate.exit() + } + } + + func testOverlappingTriggersResultInOneEntry() { + // Simulates the exact race `ChatProvider.pollGate` guards against: + // activation + Cmd+R fire while a fetch is in flight — only one caller + // may enter, the rest must bail out until the in-flight caller exits. + let gate = ReentrancyGate() + var enteredCount = 0 + + // Caller A starts the fetch + if gate.tryEnter() { enteredCount += 1 } + // Caller B (overlapping) tries while A is still in flight + if gate.tryEnter() { enteredCount += 1 } + // Caller C (overlapping) tries while A is still in flight + if gate.tryEnter() { enteredCount += 1 } + + XCTAssertEqual(enteredCount, 1, "Only one of 3 overlapping callers may enter") + + // Caller A completes + gate.exit() + + // Caller D arrives after A exited — should be allowed + XCTAssertTrue(gate.tryEnter(), "New caller after exit must be allowed") + gate.exit() + } + + func testExitWithoutEnterIsSafe() { + // Defensive: exit() when no prior enter() is a no-op. This matches the + // `defer { gate.exit() }` pattern — exit runs even when the guard bails. + // Subsequent enter should still succeed. + let gate = ReentrancyGate() + gate.exit() + XCTAssertTrue(gate.tryEnter(), "Enter after spurious exit must still succeed") + } +} From 2c6478ac4e1965d9201721da4ccb7d7969f06a83 Mon Sep 17 00:00:00 2001 From: beastoin Date: Wed, 15 Apr 2026 04:03:30 +0000 Subject: [PATCH 31/45] test(desktop): add CrispManager lifecycle tests (#6500) Covers start() idempotency (no observer replacement on second call), stop() nils both observers, stop() idempotency, markAsRead() advances persisted timestamp + clears unreadCount, and safe behavior with empty timestamps. Addresses CP8 tester feedback that the event-driven Crisp branch had no regression coverage. Co-Authored-By: Claude Opus 4.6 --- .../Tests/CrispManagerLifecycleTests.swift | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 desktop/Desktop/Tests/CrispManagerLifecycleTests.swift diff --git a/desktop/Desktop/Tests/CrispManagerLifecycleTests.swift b/desktop/Desktop/Tests/CrispManagerLifecycleTests.swift new file mode 100644 index 0000000000..3eace162f8 --- /dev/null +++ b/desktop/Desktop/Tests/CrispManagerLifecycleTests.swift @@ -0,0 +1,113 @@ +import AppKit +import XCTest +@testable import Omi_Computer + +/// Tests for `CrispManager` event-driven lifecycle (#6500). +/// +/// After removing the 120s polling timer, `CrispManager` relies entirely on +/// `start()`/`stop()` registering/unregistering `didBecomeActive` + `.refreshAllData` +/// observers. These tests cover the highest-risk branch: the observer lifecycle +/// and `markAsRead()` timestamp advancement. +@MainActor +final class CrispManagerLifecycleTests: XCTestCase { + + // Save and restore the UserDefaults-backed timestamps so each test runs + // against a known state without clobbering real app data. + private var savedLastSeen: Double = 0 + private var savedLatestOperator: Double = 0 + + override func setUp() async throws { + try await super.setUp() + savedLastSeen = UserDefaults.standard.double(forKey: "crisp_lastSeenTimestamp") + savedLatestOperator = UserDefaults.standard.double(forKey: "crisp_latestOperatorTimestamp") + // Reset the singleton to a clean state — previous tests may have called start() + CrispManager.shared.stop() + } + + override func tearDown() async throws { + CrispManager.shared.stop() + UserDefaults.standard.set(savedLastSeen, forKey: "crisp_lastSeenTimestamp") + UserDefaults.standard.set(savedLatestOperator, forKey: "crisp_latestOperatorTimestamp") + try await super.tearDown() + } + + func testStartIsIdempotent() { + let manager = CrispManager.shared + XCTAssertFalse(manager.isStarted, "Manager must be stopped after setUp()") + + manager.start() + XCTAssertTrue(manager.isStarted, "start() must set isStarted true") + let firstActivationObs = manager.activationObserver + let firstRefreshObs = manager.refreshAllObserver + XCTAssertNotNil(firstActivationObs, "start() must register activation observer") + XCTAssertNotNil(firstRefreshObs, "start() must register refreshAllData observer") + + // Second start() call must be a no-op — observers must NOT be replaced. + // A new token would mean we leaked the first registration. + manager.start() + XCTAssertTrue(manager.isStarted) + XCTAssertTrue( + manager.activationObserver === firstActivationObs as AnyObject, + "Second start() must not replace the activation observer (leak guard)" + ) + XCTAssertTrue( + manager.refreshAllObserver === firstRefreshObs as AnyObject, + "Second start() must not replace the refreshAllData observer (leak guard)" + ) + } + + func testStopRemovesBothObservers() { + let manager = CrispManager.shared + manager.start() + XCTAssertNotNil(manager.activationObserver) + XCTAssertNotNil(manager.refreshAllObserver) + XCTAssertTrue(manager.isStarted) + + manager.stop() + XCTAssertNil(manager.activationObserver, "stop() must nil the activation observer") + XCTAssertNil(manager.refreshAllObserver, "stop() must nil the refreshAllData observer") + XCTAssertFalse(manager.isStarted, "stop() must clear isStarted so start() can run again") + + // After stop(), a subsequent start() must succeed (observer lifecycle reusable). + manager.start() + XCTAssertTrue(manager.isStarted, "start() after stop() must re-register observers") + XCTAssertNotNil(manager.activationObserver) + XCTAssertNotNil(manager.refreshAllObserver) + } + + func testStopIsIdempotent() { + let manager = CrispManager.shared + manager.start() + manager.stop() + // Second stop() must not crash or change state + manager.stop() + XCTAssertFalse(manager.isStarted) + XCTAssertNil(manager.activationObserver) + XCTAssertNil(manager.refreshAllObserver) + } + + func testMarkAsReadAdvancesPersistedTimestamp() { + let manager = CrispManager.shared + manager.latestOperatorTimestamp = 999_999 + manager.lastSeenTimestamp = 111_111 + + manager.markAsRead() + + XCTAssertEqual( + manager.lastSeenTimestamp, 999_999, + "markAsRead() must advance lastSeenTimestamp to latestOperatorTimestamp" + ) + XCTAssertEqual(manager.unreadCount, 0, "markAsRead() must clear unreadCount") + } + + func testMarkAsReadIsSafeWhenNoNewMessages() { + let manager = CrispManager.shared + manager.latestOperatorTimestamp = 0 + manager.lastSeenTimestamp = 0 + + manager.markAsRead() + + XCTAssertEqual(manager.lastSeenTimestamp, 0) + XCTAssertEqual(manager.unreadCount, 0) + } +} From 457a6a27d6af1465b87befe527af045a7dfd18ea Mon Sep 17 00:00:00 2001 From: beastoin Date: Wed, 15 Apr 2026 04:13:29 +0000 Subject: [PATCH 32/45] docs(desktop): tighten ReentrancyGate ownership contract (#6500) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer round 2: prior doc claimed exit() was safe after a failed tryEnter(), but exit() unconditionally clears isInFlight — a spurious call while another caller holds the gate would reopen it and allow overlapping operations. Tighten the doc to spell out that only the caller that got `true` from tryEnter() owns the gate, show the canonical `guard`/`defer` usage, and explicitly note that exit() does not validate ownership. --- desktop/Desktop/Sources/ReentrancyGate.swift | 27 +++++++++++++++----- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/desktop/Desktop/Sources/ReentrancyGate.swift b/desktop/Desktop/Sources/ReentrancyGate.swift index 6f932e9e2e..58955f4cac 100644 --- a/desktop/Desktop/Sources/ReentrancyGate.swift +++ b/desktop/Desktop/Sources/ReentrancyGate.swift @@ -5,9 +5,22 @@ import Foundation /// Use when two independent triggers (e.g. `didBecomeActive` + Cmd+R) can fire /// back-to-back and would otherwise cause duplicate fetches/inserts. Call /// `tryEnter()` at the start of the critical section — if it returns `false`, -/// another caller is already in-flight and the current caller should bail out. -/// Always pair `tryEnter()` with `exit()` via `defer` so the gate reopens even -/// on thrown errors or early returns. +/// another caller is already in-flight and the current caller should bail out +/// **without** calling `exit()`. Only the caller that got `true` from +/// `tryEnter()` owns the gate and must release it. +/// +/// The canonical usage is a `guard` + `defer` pair, which ensures `exit()` is +/// only scheduled once the guard has admitted the caller: +/// +/// ```swift +/// guard gate.tryEnter() else { return } // non-owners return here, no exit() +/// defer { gate.exit() } // only the owner reaches this line +/// // … critical section … +/// ``` +/// +/// `exit()` does not validate ownership — a stray call will reopen the gate +/// while another caller is still inside the critical section. Follow the +/// `guard`/`defer` pattern above and the contract holds. /// /// Tested in `ReentrancyGateTests`. @MainActor @@ -16,16 +29,16 @@ final class ReentrancyGate { /// Attempts to enter the critical section. /// - Returns: `true` if the caller acquired the gate (must call `exit()` when done), - /// `false` if another operation is already in-flight (caller should skip its work). + /// `false` if another operation is already in-flight (caller must **not** call `exit()`). func tryEnter() -> Bool { guard !isInFlight else { return false } isInFlight = true return true } - /// Releases the gate. Safe to call even if `tryEnter()` returned `false` - /// (no-op in that case — but callers should only `exit()` when their matching - /// `tryEnter()` returned `true`). + /// Releases the gate. **Caller contract:** only call this after a matching + /// `tryEnter()` returned `true`. Calling `exit()` without ownership will + /// reopen the gate while another caller is still inside the critical section. func exit() { isInFlight = false } From 24fafaf9f433215cb974f7072cf1fc6075483530 Mon Sep 17 00:00:00 2001 From: beastoin Date: Wed, 15 Apr 2026 04:13:35 +0000 Subject: [PATCH 33/45] test(desktop): replace unsafe exit test with guard/defer pattern test (#6500) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer round 2: the old testExitWithoutEnterIsSafe asserted a contract the implementation does not enforce — exit() unconditionally clears isInFlight, so a stray call really would reopen the gate. Swap in testGuardDeferPatternOnlyExitsWhenOwnerEntered, which models the canonical ChatProvider.pollForNewMessages() usage where the guard early-returns before the defer is registered, so only owning callers ever hit exit(). --- .../Desktop/Tests/ReentrancyGateTests.swift | 31 +++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/desktop/Desktop/Tests/ReentrancyGateTests.swift b/desktop/Desktop/Tests/ReentrancyGateTests.swift index 17bf526d27..6440446e67 100644 --- a/desktop/Desktop/Tests/ReentrancyGateTests.swift +++ b/desktop/Desktop/Tests/ReentrancyGateTests.swift @@ -68,12 +68,31 @@ final class ReentrancyGateTests: XCTestCase { gate.exit() } - func testExitWithoutEnterIsSafe() { - // Defensive: exit() when no prior enter() is a no-op. This matches the - // `defer { gate.exit() }` pattern — exit runs even when the guard bails. - // Subsequent enter should still succeed. + func testGuardDeferPatternOnlyExitsWhenOwnerEntered() { + // Models the canonical ChatProvider.pollForNewMessages() usage: + // guard gate.tryEnter() else { return } + // defer { gate.exit() } + // Non-owners return before the defer is registered, so exit() is + // never called from a non-owning caller — the contract holds. let gate = ReentrancyGate() - gate.exit() - XCTAssertTrue(gate.tryEnter(), "Enter after spurious exit must still succeed") + var exitCalls = 0 + + func criticalSection() { + guard gate.tryEnter() else { return } + defer { + gate.exit() + exitCalls += 1 + } + // simulated critical work — the second concurrent call below runs + // before this defer fires because Swift closures run synchronously. + } + + // First caller acquires + releases via defer. + criticalSection() + XCTAssertEqual(exitCalls, 1) + + // Second sequential caller also acquires cleanly after the first exited. + criticalSection() + XCTAssertEqual(exitCalls, 2) } } From fc2f5819a16e5c7d2a9baf4b1c687687db854a32 Mon Sep 17 00:00:00 2001 From: beastoin Date: Wed, 15 Apr 2026 04:13:41 +0000 Subject: [PATCH 34/45] refactor(desktop): add performInitialPoll flag to CrispManager.start() (#6500) Reviewer round 2: lifecycle tests called start() which unconditionally fired pollForMessages(), hitting APIClient.shared and depending on local auth/network state. Add an opt-out parameter (defaults to true for real production callers) so CrispManagerLifecycleTests can exercise the observer-registration path hermetically without touching the network or firing real macOS notifications. --- .../Desktop/Sources/MainWindow/CrispManager.swift | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/desktop/Desktop/Sources/MainWindow/CrispManager.swift b/desktop/Desktop/Sources/MainWindow/CrispManager.swift index f1556dd1da..c76ab617ee 100644 --- a/desktop/Desktop/Sources/MainWindow/CrispManager.swift +++ b/desktop/Desktop/Sources/MainWindow/CrispManager.swift @@ -51,8 +51,13 @@ class CrispManager: ObservableObject { private init() {} - /// Call once after sign-in to fetch Crisp messages and listen for activation/Cmd+R - func start() { + /// Call once after sign-in to fetch Crisp messages and listen for activation/Cmd+R. + /// + /// - Parameter performInitialPoll: If `true` (default), kicks off an immediate + /// `pollForMessages()` call that hits `APIClient.shared`. Pass `false` only + /// from lifecycle unit tests that want to exercise observer registration + /// without touching the network, auth state, or firing real notifications. + func start(performInitialPoll: Bool = true) { guard !isStarted else { return } isStarted = true @@ -63,8 +68,9 @@ class CrispManager: ObservableObject { lastSeenTimestamp = UInt64(Date().timeIntervalSince1970) } - // Fetch immediately on start - pollForMessages() + if performInitialPoll { + pollForMessages() + } // Refresh on app activation and Cmd+R (no periodic timer) activationObserver = NotificationCenter.default.addObserver( From 6be95f40d05e3e503acdeabb25d9503c6968f48e Mon Sep 17 00:00:00 2001 From: beastoin Date: Wed, 15 Apr 2026 04:13:45 +0000 Subject: [PATCH 35/45] test(desktop): make CrispManager lifecycle tests hermetic (#6500) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer round 2: tests invoked start() which fired pollForMessages() against APIClient.shared and depended on local auth state, so they were really integration tests. Pass performInitialPoll: false so each test only exercises observer registration/removal and timestamp advancement — no network, no auth, no real notifications. --- desktop/Desktop/Tests/CrispManagerLifecycleTests.swift | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/desktop/Desktop/Tests/CrispManagerLifecycleTests.swift b/desktop/Desktop/Tests/CrispManagerLifecycleTests.swift index 3eace162f8..ce1b6b7f0e 100644 --- a/desktop/Desktop/Tests/CrispManagerLifecycleTests.swift +++ b/desktop/Desktop/Tests/CrispManagerLifecycleTests.swift @@ -35,7 +35,7 @@ final class CrispManagerLifecycleTests: XCTestCase { let manager = CrispManager.shared XCTAssertFalse(manager.isStarted, "Manager must be stopped after setUp()") - manager.start() + manager.start(performInitialPoll: false) XCTAssertTrue(manager.isStarted, "start() must set isStarted true") let firstActivationObs = manager.activationObserver let firstRefreshObs = manager.refreshAllObserver @@ -44,7 +44,7 @@ final class CrispManagerLifecycleTests: XCTestCase { // Second start() call must be a no-op — observers must NOT be replaced. // A new token would mean we leaked the first registration. - manager.start() + manager.start(performInitialPoll: false) XCTAssertTrue(manager.isStarted) XCTAssertTrue( manager.activationObserver === firstActivationObs as AnyObject, @@ -58,7 +58,7 @@ final class CrispManagerLifecycleTests: XCTestCase { func testStopRemovesBothObservers() { let manager = CrispManager.shared - manager.start() + manager.start(performInitialPoll: false) XCTAssertNotNil(manager.activationObserver) XCTAssertNotNil(manager.refreshAllObserver) XCTAssertTrue(manager.isStarted) @@ -69,7 +69,7 @@ final class CrispManagerLifecycleTests: XCTestCase { XCTAssertFalse(manager.isStarted, "stop() must clear isStarted so start() can run again") // After stop(), a subsequent start() must succeed (observer lifecycle reusable). - manager.start() + manager.start(performInitialPoll: false) XCTAssertTrue(manager.isStarted, "start() after stop() must re-register observers") XCTAssertNotNil(manager.activationObserver) XCTAssertNotNil(manager.refreshAllObserver) @@ -77,7 +77,7 @@ final class CrispManagerLifecycleTests: XCTestCase { func testStopIsIdempotent() { let manager = CrispManager.shared - manager.start() + manager.start(performInitialPoll: false) manager.stop() // Second stop() must not crash or change state manager.stop() From 2ce8d46773ea57a7d4157aa569eea493ae2789a8 Mon Sep 17 00:00:00 2001 From: beastoin Date: Wed, 15 Apr 2026 04:17:37 +0000 Subject: [PATCH 36/45] test(desktop): overlap non-owner with in-flight owner in gate test (#6500) Reviewer round 3: the prior testGuardDeferPatternOnlyExitsWhenOwnerEntered only called criticalSection() sequentially, so every invocation hit the happy path. A regression that put `defer { gate.exit() }` above `guard gate.tryEnter()` in production would still pass the test. Rewrite as testGuardDeferPatternNonOwnerDoesNotCallExit: the test itself holds the gate as caller A, then invokes criticalSection() as caller B while A is still in-flight. Asserts B registers no exit, the gate is still held by A (not reopened), and caller C can acquire normally after A releases. --- .../Desktop/Tests/ReentrancyGateTests.swift | 33 ++++++++++++++----- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/desktop/Desktop/Tests/ReentrancyGateTests.swift b/desktop/Desktop/Tests/ReentrancyGateTests.swift index 6440446e67..f2a62fc4da 100644 --- a/desktop/Desktop/Tests/ReentrancyGateTests.swift +++ b/desktop/Desktop/Tests/ReentrancyGateTests.swift @@ -68,12 +68,17 @@ final class ReentrancyGateTests: XCTestCase { gate.exit() } - func testGuardDeferPatternOnlyExitsWhenOwnerEntered() { + func testGuardDeferPatternNonOwnerDoesNotCallExit() { // Models the canonical ChatProvider.pollForNewMessages() usage: // guard gate.tryEnter() else { return } // defer { gate.exit() } - // Non-owners return before the defer is registered, so exit() is - // never called from a non-owning caller — the contract holds. + // + // Regression guard: if a future edit swapped `defer` above `guard`, a + // non-owning caller would still fire `exit()` and reopen the gate while + // the owner was still inside the critical section. This test forces an + // overlap — caller A holds the gate, caller B invokes the critical + // section while A is still in-flight — and asserts B neither enters + // nor reopens the gate. let gate = ReentrancyGate() var exitCalls = 0 @@ -83,16 +88,26 @@ final class ReentrancyGateTests: XCTestCase { gate.exit() exitCalls += 1 } - // simulated critical work — the second concurrent call below runs - // before this defer fires because Swift closures run synchronously. } - // First caller acquires + releases via defer. + // Caller A (the test itself) acquires the gate directly. + XCTAssertTrue(gate.tryEnter(), "Precondition: caller A must acquire the gate") + + // Caller B invokes the critical section while A is still in-flight. + // Under guard/defer, B's guard short-circuits and no `defer` is registered, + // so exit() must not fire. criticalSection() - XCTAssertEqual(exitCalls, 1) + XCTAssertEqual(exitCalls, 0, "Non-owner caller B must not register an exit") + + // Gate must still be owned by A — B must not have reopened it. + XCTAssertFalse( + gate.tryEnter(), + "Gate must still be held by A; a regressed guard/defer order would have reopened it" + ) - // Second sequential caller also acquires cleanly after the first exited. + // A releases; caller C can now run through the critical section normally. + gate.exit() criticalSection() - XCTAssertEqual(exitCalls, 2) + XCTAssertEqual(exitCalls, 1, "Owner caller C must register exactly one exit") } } From 745d8c725f7aec64f2bae962e812c83239f8069f Mon Sep 17 00:00:00 2001 From: beastoin Date: Wed, 15 Apr 2026 04:28:31 +0000 Subject: [PATCH 37/45] feat(desktop): add pollInvocations counter to CrispManager for test observability (#6500) CP8 tester round 1 gap: CrispManagerLifecycleTests verifies observer token idempotency but never proves that posting didBecomeActive or .refreshAllData actually reaches pollForMessages(). If a future edit subscribed to the wrong notification name or dropped the wiring, the current lifecycle suite would not catch it. Add a @Published private(set) counter that increments at the top of pollForMessages() (before the auth-backoff guard and the network task), so lifecycle tests can post each notification and assert the counter advances. The counter has no runtime cost beyond a single integer write per poll and no production subscribers. --- desktop/Desktop/Sources/MainWindow/CrispManager.swift | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/desktop/Desktop/Sources/MainWindow/CrispManager.swift b/desktop/Desktop/Sources/MainWindow/CrispManager.swift index c76ab617ee..0a80ca6ade 100644 --- a/desktop/Desktop/Sources/MainWindow/CrispManager.swift +++ b/desktop/Desktop/Sources/MainWindow/CrispManager.swift @@ -49,7 +49,12 @@ class CrispManager: ObservableObject { var activationObserver: NSObjectProtocol? var refreshAllObserver: NSObjectProtocol? - private init() {} + /// Counter bumped at the top of `pollForMessages()`, before the auth-backoff + /// guard and the network task. Lets `CrispManagerLifecycleTests` prove that + /// posting `didBecomeActive` / `.refreshAllData` actually reaches the poll + /// method — if an observer subscribes to the wrong notification name or a + /// future edit drops the wiring, the counter stays flat and the test fails. + @Published private(set) var pollInvocations: Int = 0 /// Call once after sign-in to fetch Crisp messages and listen for activation/Cmd+R. /// @@ -107,6 +112,7 @@ class CrispManager: ObservableObject { // MARK: - Private private func pollForMessages() { + pollInvocations += 1 Task { // Skip if in auth backoff period (recent 401 errors) guard !AuthBackoffTracker.shared.shouldSkipRequest() else { return } From 542dbc1241582a08812894d9a6a05bbb4b0236fb Mon Sep 17 00:00:00 2001 From: beastoin Date: Wed, 15 Apr 2026 04:28:38 +0000 Subject: [PATCH 38/45] feat(desktop): add refreshInvocations counter to TasksStore for observer tests (#6500) CP8 tester round 1 gap: the PR replaces the 30s Timer.publish inside TasksStore.init() with didBecomeActive + .refreshAllData sinks, but there is no test coverage proving the new observer subscriptions actually fire refreshTasksIfNeeded(). A regression (wrong notification name, dropped .store(in: &cancellables)) would ship undetected. Add a @Published counter that increments at the top of refreshTasksIfNeeded() before any early-exit guards. TasksStoreObserverTests can then post each notification and assert the counter advanced, proving the observer wiring without needing auth state, the network, or the singleton's page-visibility state. --- desktop/Desktop/Sources/Stores/TasksStore.swift | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/desktop/Desktop/Sources/Stores/TasksStore.swift b/desktop/Desktop/Sources/Stores/TasksStore.swift index 7ddab45cd4..0226329191 100644 --- a/desktop/Desktop/Sources/Stores/TasksStore.swift +++ b/desktop/Desktop/Sources/Stores/TasksStore.swift @@ -28,6 +28,13 @@ class TasksStore: ObservableObject { @Published var hasMoreDeletedTasks = true @Published var error: String? + /// Counter bumped at the top of `refreshTasksIfNeeded()`, before any of the + /// early-exit guards. Lets `TasksStoreObserverTests` prove that posting + /// `didBecomeActive` / `.refreshAllData` actually reaches the refresh method + /// — if the observer rewire regresses (wrong notification name, dropped + /// subscription), the counter stays flat and the test fails. + @Published private(set) var refreshInvocations: Int = 0 + // Legacy compatibility - combines both lists var tasks: [TaskActionItem] { incompleteTasks + completedTasks @@ -181,6 +188,7 @@ class TasksStore: ObservableObject { /// Uses local-first pattern: sync API to cache, then reload from cache /// Merges changes in-place to avoid wholesale array replacement (which kills SwiftUI gestures) private func refreshTasksIfNeeded() async { + refreshInvocations += 1 // Skip if not signed in guard AuthService.shared.isSignedIn else { return } // Skip if in auth backoff period (recent 401 errors) From 21722c474b3eb646a836909d01f1c7412c4c5e5b Mon Sep 17 00:00:00 2001 From: beastoin Date: Wed, 15 Apr 2026 04:28:43 +0000 Subject: [PATCH 39/45] feat(desktop): add refreshInvocations counter to MemoriesViewModel for observer tests (#6500) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CP8 tester round 1 gap: the PR replaces the 30s Timer.publish inside MemoriesViewModel.init() with didBecomeActive + .refreshAllData sinks, but there is no test coverage proving the new subscribers actually fire refreshMemoriesIfNeeded() when the notifications post. Add a @Published counter that increments at the top of refreshMemoriesIfNeeded() before any early-exit guards. Because MemoriesViewModel is not a singleton, MemoriesViewModelObserverTests can construct a fresh instance, post each notification, and assert the counter advanced — proving the observer wiring without touching the network, auth state, or the page-visibility guard. --- .../Desktop/Sources/MainWindow/Pages/MemoriesPage.swift | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/desktop/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift b/desktop/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift index 21a362d75d..3da997a711 100644 --- a/desktop/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift +++ b/desktop/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift @@ -139,6 +139,13 @@ class MemoriesViewModel: ObservableObject { /// Memories loaded from SQLite with filters applied @Published private(set) var filteredFromDatabase: [ServerMemory] = [] @Published private(set) var isLoadingFiltered = false + + /// Counter bumped at the top of `refreshMemoriesIfNeeded()`, before any of + /// the early-exit guards. Lets `MemoriesViewModelObserverTests` prove that + /// posting `didBecomeActive` / `.refreshAllData` actually reaches the refresh + /// method — if the observer rewire regresses, the counter stays flat and the + /// test fails. + @Published private(set) var refreshInvocations: Int = 0 @Published var showingAddMemory = false @Published var newMemoryText = "" @Published var editingMemory: ServerMemory? = nil @@ -223,6 +230,7 @@ class MemoriesViewModel: ObservableObject { /// Refresh memories if already loaded (for auto-refresh) private func refreshMemoriesIfNeeded() async { + refreshInvocations += 1 // Skip if user is signed out (tokens are cleared) guard AuthState.shared.isSignedIn else { return } // Skip if in auth backoff period (recent 401 errors) From 2ce00ce2356abc1da96bd5c3a294ae24f85f90da Mon Sep 17 00:00:00 2001 From: beastoin Date: Wed, 15 Apr 2026 04:28:51 +0000 Subject: [PATCH 40/45] test(desktop): assert CrispManager observers fire pollForMessages() (#6500) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CP8 tester round 1 gap: prior lifecycle tests checked observer token idempotency but not that the observers actually routed to the poll method. Adds three tests that post each notification and assert the new pollInvocations counter advances: - testDidBecomeActiveNotificationTriggersPoll: proves activation observer is wired to NSApplication.didBecomeActiveNotification and reaches pollForMessages(). - testRefreshAllDataNotificationTriggersPoll: proves refresh observer is wired to .refreshAllData (the Cmd+R notification) and reaches pollForMessages(). - testStoppedManagerDoesNotRespondToNotifications: proves stop() fully detaches both observers — neither notification advances the counter after the manager is stopped. --- .../Tests/CrispManagerLifecycleTests.swift | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/desktop/Desktop/Tests/CrispManagerLifecycleTests.swift b/desktop/Desktop/Tests/CrispManagerLifecycleTests.swift index ce1b6b7f0e..9a0e729d14 100644 --- a/desktop/Desktop/Tests/CrispManagerLifecycleTests.swift +++ b/desktop/Desktop/Tests/CrispManagerLifecycleTests.swift @@ -110,4 +110,56 @@ final class CrispManagerLifecycleTests: XCTestCase { XCTAssertEqual(manager.lastSeenTimestamp, 0) XCTAssertEqual(manager.unreadCount, 0) } + + func testDidBecomeActiveNotificationTriggersPoll() async { + let manager = CrispManager.shared + manager.start(performInitialPoll: false) + let baseline = manager.pollInvocations + + NotificationCenter.default.post( + name: NSApplication.didBecomeActiveNotification, object: nil + ) + // Observer posts on main queue; yield so the block runs. + await Task.yield() + try? await Task.sleep(nanoseconds: 50_000_000) + + XCTAssertEqual( + manager.pollInvocations, baseline + 1, + "didBecomeActive must route to pollForMessages() via the activation observer" + ) + } + + func testRefreshAllDataNotificationTriggersPoll() async { + let manager = CrispManager.shared + manager.start(performInitialPoll: false) + let baseline = manager.pollInvocations + + NotificationCenter.default.post(name: .refreshAllData, object: nil) + await Task.yield() + try? await Task.sleep(nanoseconds: 50_000_000) + + XCTAssertEqual( + manager.pollInvocations, baseline + 1, + ".refreshAllData (Cmd+R) must route to pollForMessages() via the refresh observer" + ) + } + + func testStoppedManagerDoesNotRespondToNotifications() async { + let manager = CrispManager.shared + manager.start(performInitialPoll: false) + manager.stop() + let baseline = manager.pollInvocations + + NotificationCenter.default.post( + name: NSApplication.didBecomeActiveNotification, object: nil + ) + NotificationCenter.default.post(name: .refreshAllData, object: nil) + await Task.yield() + try? await Task.sleep(nanoseconds: 50_000_000) + + XCTAssertEqual( + manager.pollInvocations, baseline, + "After stop(), neither notification should reach pollForMessages()" + ) + } } From 55b4db909b8ba202a009442a84cf1e5df41ddf87 Mon Sep 17 00:00:00 2001 From: beastoin Date: Wed, 15 Apr 2026 04:28:58 +0000 Subject: [PATCH 41/45] test(desktop): add TasksStore observer-firing tests (#6500) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CP8 tester round 1 gap: the PR rewired TasksStore from a 30s Timer.publish to didBecomeActive + .refreshAllData sinks, but there was no coverage at all for that rewire. A regression in either subscription would ship undetected. Add three tests that each post a notification and assert the baseline-diffed refreshInvocations counter advances: - testDidBecomeActiveNotificationTriggersRefresh: proves the activation sink reaches refreshTasksIfNeeded(). - testRefreshAllDataNotificationTriggersRefresh: proves the Cmd+R sink reaches refreshTasksIfNeeded(). - testBothNotificationsTriggerIndependentRefreshes: proves the two sinks are independent subscriptions, not a single multiplexed one. Uses baseline diffing because TasksStore is a singleton — the counter persists across tests, but each test reads its own baseline first. --- .../Tests/TasksStoreObserverTests.swift | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 desktop/Desktop/Tests/TasksStoreObserverTests.swift diff --git a/desktop/Desktop/Tests/TasksStoreObserverTests.swift b/desktop/Desktop/Tests/TasksStoreObserverTests.swift new file mode 100644 index 0000000000..9d68ce22f6 --- /dev/null +++ b/desktop/Desktop/Tests/TasksStoreObserverTests.swift @@ -0,0 +1,64 @@ +import AppKit +import XCTest +@testable import Omi_Computer + +/// Tests for `TasksStore` auto-refresh observer wiring (#6500). +/// +/// After replacing the 30-second `Timer.publish` with `didBecomeActive` + +/// `.refreshAllData` subscribers, the store must still refresh when those +/// notifications fire. These tests post each notification and assert the +/// baseline-diffed `refreshInvocations` counter advances by one, proving the +/// observer reaches `refreshTasksIfNeeded()` — the early-exit guards inside +/// that method run after the counter bumps, so tests don't need local auth +/// or any prior `loadTasks()` call. +@MainActor +final class TasksStoreObserverTests: XCTestCase { + + func testDidBecomeActiveNotificationTriggersRefresh() async { + let store = TasksStore.shared + let baseline = store.refreshInvocations + + NotificationCenter.default.post( + name: NSApplication.didBecomeActiveNotification, object: nil + ) + // Sink runs on the main queue; yield so the task enqueues and fires. + await Task.yield() + try? await Task.sleep(nanoseconds: 50_000_000) + + XCTAssertEqual( + store.refreshInvocations, baseline + 1, + "didBecomeActive must route to refreshTasksIfNeeded() via the activation observer" + ) + } + + func testRefreshAllDataNotificationTriggersRefresh() async { + let store = TasksStore.shared + let baseline = store.refreshInvocations + + NotificationCenter.default.post(name: .refreshAllData, object: nil) + await Task.yield() + try? await Task.sleep(nanoseconds: 50_000_000) + + XCTAssertEqual( + store.refreshInvocations, baseline + 1, + ".refreshAllData (Cmd+R) must route to refreshTasksIfNeeded() via the refresh observer" + ) + } + + func testBothNotificationsTriggerIndependentRefreshes() async { + let store = TasksStore.shared + let baseline = store.refreshInvocations + + NotificationCenter.default.post( + name: NSApplication.didBecomeActiveNotification, object: nil + ) + NotificationCenter.default.post(name: .refreshAllData, object: nil) + await Task.yield() + try? await Task.sleep(nanoseconds: 100_000_000) + + XCTAssertEqual( + store.refreshInvocations, baseline + 2, + "Both observers must fire independently — posting both notifications yields two refresh calls" + ) + } +} From ae2ddd94a0e59e3b255b0527a21598e9120da4af Mon Sep 17 00:00:00 2001 From: beastoin Date: Wed, 15 Apr 2026 04:29:05 +0000 Subject: [PATCH 42/45] test(desktop): add MemoriesViewModel observer-firing tests (#6500) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CP8 tester round 1 gap: the PR rewired MemoriesViewModel from a 30s Timer.publish to didBecomeActive + .refreshAllData sinks, but there was no coverage at all for that rewire. MemoriesViewModel is not a singleton, so each test constructs a fresh instance (which runs init() and registers the subscribers) and posts each notification: - testDidBecomeActiveNotificationTriggersRefresh: proves activation subscription reaches refreshMemoriesIfNeeded(). - testRefreshAllDataNotificationTriggersRefresh: proves Cmd+R subscription reaches refreshMemoriesIfNeeded(). - testDeallocatedViewModelDoesNotLeakObservers: proves the `[weak self]` capture in both sinks lets the view model deallocate cleanly — if the capture misbehaved, posting the notifications after the instance is gone would crash. --- .../MemoriesViewModelObserverTests.swift | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 desktop/Desktop/Tests/MemoriesViewModelObserverTests.swift diff --git a/desktop/Desktop/Tests/MemoriesViewModelObserverTests.swift b/desktop/Desktop/Tests/MemoriesViewModelObserverTests.swift new file mode 100644 index 0000000000..eca08ec458 --- /dev/null +++ b/desktop/Desktop/Tests/MemoriesViewModelObserverTests.swift @@ -0,0 +1,62 @@ +import AppKit +import XCTest +@testable import Omi_Computer + +/// Tests for `MemoriesViewModel` auto-refresh observer wiring (#6500). +/// +/// After replacing the 30-second `Timer.publish` with `didBecomeActive` + +/// `.refreshAllData` subscribers, the view model must still refresh when +/// those notifications fire. Because `MemoriesViewModel` is not a singleton, +/// each test constructs a fresh instance (triggering `init()` which registers +/// the two subscribers into its private `cancellables`) and asserts that +/// posting each notification advances `refreshInvocations` by one. +@MainActor +final class MemoriesViewModelObserverTests: XCTestCase { + + func testDidBecomeActiveNotificationTriggersRefresh() async { + let viewModel = MemoriesViewModel() + XCTAssertEqual(viewModel.refreshInvocations, 0, "Fresh instance must start at zero") + + NotificationCenter.default.post( + name: NSApplication.didBecomeActiveNotification, object: nil + ) + await Task.yield() + try? await Task.sleep(nanoseconds: 50_000_000) + + XCTAssertEqual( + viewModel.refreshInvocations, 1, + "didBecomeActive must route to refreshMemoriesIfNeeded() via the activation subscriber" + ) + } + + func testRefreshAllDataNotificationTriggersRefresh() async { + let viewModel = MemoriesViewModel() + + NotificationCenter.default.post(name: .refreshAllData, object: nil) + await Task.yield() + try? await Task.sleep(nanoseconds: 50_000_000) + + XCTAssertEqual( + viewModel.refreshInvocations, 1, + ".refreshAllData (Cmd+R) must route to refreshMemoriesIfNeeded() via the refresh subscriber" + ) + } + + func testDeallocatedViewModelDoesNotLeakObservers() async { + // Ensures the `[weak self]` capture in the Combine sinks lets the + // view model deallocate cleanly — no crash when the notifications + // fire after the instance is gone. + do { + let viewModel = MemoriesViewModel() + XCTAssertEqual(viewModel.refreshInvocations, 0) + } + // viewModel is out of scope and should be deallocated. + NotificationCenter.default.post( + name: NSApplication.didBecomeActiveNotification, object: nil + ) + NotificationCenter.default.post(name: .refreshAllData, object: nil) + await Task.yield() + try? await Task.sleep(nanoseconds: 50_000_000) + // If the weak capture misbehaved we'd crash above; reaching here is the assertion. + } +} From 6a0f271d384c6c02ca2430992248459fc68b909c Mon Sep 17 00:00:00 2001 From: beastoin Date: Wed, 15 Apr 2026 04:34:43 +0000 Subject: [PATCH 43/45] refactor(desktop): drop @Published from CrispManager.pollInvocations (#6500) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer round 5: the test-only counter was declared @Published, so every activation / Cmd+R refresh emitted objectWillChange on CrispManager — invalidating any SwiftUI view observing it even though the counter never drives UI. Make it plain `private(set) var`. Tests still read it directly via @testable import; production pays zero SwiftUI invalidation cost beyond a single integer write per call. --- desktop/Desktop/Sources/MainWindow/CrispManager.swift | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/desktop/Desktop/Sources/MainWindow/CrispManager.swift b/desktop/Desktop/Sources/MainWindow/CrispManager.swift index 0a80ca6ade..ba9c36f1dd 100644 --- a/desktop/Desktop/Sources/MainWindow/CrispManager.swift +++ b/desktop/Desktop/Sources/MainWindow/CrispManager.swift @@ -54,7 +54,11 @@ class CrispManager: ObservableObject { /// posting `didBecomeActive` / `.refreshAllData` actually reaches the poll /// method — if an observer subscribes to the wrong notification name or a /// future edit drops the wiring, the counter stays flat and the test fails. - @Published private(set) var pollInvocations: Int = 0 + /// Deliberately **not** `@Published` — publishing on every activation/Cmd+R + /// refresh would emit `objectWillChange` and invalidate any SwiftUI view + /// observing `CrispManager`, which is a pure production cost for a value + /// nothing drives UI from. + private(set) var pollInvocations: Int = 0 /// Call once after sign-in to fetch Crisp messages and listen for activation/Cmd+R. /// From b272e3742c275244461a2ee2b932d80ccdc586de Mon Sep 17 00:00:00 2001 From: beastoin Date: Wed, 15 Apr 2026 04:34:47 +0000 Subject: [PATCH 44/45] refactor(desktop): drop @Published from TasksStore.refreshInvocations (#6500) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer round 5: the test-only counter was declared @Published, so every activation / Cmd+R refresh emitted objectWillChange on TasksStore — invalidating any SwiftUI view observing it. Make it plain `private(set) var`. Production pays zero SwiftUI invalidation cost beyond a single integer write per call. --- desktop/Desktop/Sources/Stores/TasksStore.swift | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/desktop/Desktop/Sources/Stores/TasksStore.swift b/desktop/Desktop/Sources/Stores/TasksStore.swift index 0226329191..2c4f748a67 100644 --- a/desktop/Desktop/Sources/Stores/TasksStore.swift +++ b/desktop/Desktop/Sources/Stores/TasksStore.swift @@ -33,7 +33,11 @@ class TasksStore: ObservableObject { /// `didBecomeActive` / `.refreshAllData` actually reaches the refresh method /// — if the observer rewire regresses (wrong notification name, dropped /// subscription), the counter stays flat and the test fails. - @Published private(set) var refreshInvocations: Int = 0 + /// Deliberately **not** `@Published` — publishing on every activation/Cmd+R + /// refresh would emit `objectWillChange` and invalidate SwiftUI views + /// observing `TasksStore`, which is a pure production cost for a value + /// nothing drives UI from. + private(set) var refreshInvocations: Int = 0 // Legacy compatibility - combines both lists var tasks: [TaskActionItem] { From f22079a8ed6f9e9f0d93dae409a6770bbae7c39f Mon Sep 17 00:00:00 2001 From: beastoin Date: Wed, 15 Apr 2026 04:34:51 +0000 Subject: [PATCH 45/45] refactor(desktop): drop @Published from MemoriesViewModel.refreshInvocations (#6500) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer round 5: the test-only counter was declared @Published, so every activation / Cmd+R refresh emitted objectWillChange on MemoriesViewModel — invalidating its SwiftUI observers. Make it plain `private(set) var`. Production pays zero SwiftUI invalidation cost beyond a single integer write per call. --- desktop/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/desktop/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift b/desktop/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift index 3da997a711..f661fcd661 100644 --- a/desktop/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift +++ b/desktop/Desktop/Sources/MainWindow/Pages/MemoriesPage.swift @@ -145,7 +145,11 @@ class MemoriesViewModel: ObservableObject { /// posting `didBecomeActive` / `.refreshAllData` actually reaches the refresh /// method — if the observer rewire regresses, the counter stays flat and the /// test fails. - @Published private(set) var refreshInvocations: Int = 0 + /// Deliberately **not** `@Published` — publishing on every activation/Cmd+R + /// refresh would emit `objectWillChange` and invalidate any SwiftUI view + /// observing `MemoriesViewModel`, which is a pure production cost for a + /// value nothing drives UI from. + private(set) var refreshInvocations: Int = 0 @Published var showingAddMemory = false @Published var newMemoryText = "" @Published var editingMemory: ServerMemory? = nil