diff --git a/desktop/CHANGELOG.json b/desktop/CHANGELOG.json index 5b3ba60596..39af593b77 100644 --- a/desktop/CHANGELOG.json +++ b/desktop/CHANGELOG.json @@ -1,6 +1,7 @@ { "unreleased": [ - "Fixed screen recording randomly stopping with a false Permission Required notification after closing a tab or dismissing a popup" + "Fixed screen recording randomly stopping with a false Permission Required notification after closing a tab or dismissing a popup", + "Fixed screen recording silently dropping when the frontmost app is a helper with no captureable window (LogiPluginService, Dock, etc.) \u2014 resolver now falls back to the last known good captureable window" ], "releases": [ { diff --git a/desktop/Desktop/Sources/ProactiveAssistants/ProactiveAssistantsPlugin.swift b/desktop/Desktop/Sources/ProactiveAssistants/ProactiveAssistantsPlugin.swift index 26eb80a422..9aabac946b 100644 --- a/desktop/Desktop/Sources/ProactiveAssistants/ProactiveAssistantsPlugin.swift +++ b/desktop/Desktop/Sources/ProactiveAssistants/ProactiveAssistantsPlugin.swift @@ -620,7 +620,10 @@ public class ProactiveAssistantsPlugin: NSObject { return } - // Get current window info (use real app name, not cached) + // Get current window info. Normally this is the current frontmost app, but if + // the frontmost is a helper with no captureable window (LogiPluginService, Dock, + // UserNotificationCenter, etc.) the resolver falls back to the last known good + // captureable window for up to ~2s — see ScreenCaptureService.getActiveWindowInfoAsync. let (realAppName, windowTitle, windowID) = await WindowMonitor.getActiveWindowInfoAsync() // Check if the current app is excluded from Rewind capture diff --git a/desktop/Desktop/Sources/ScreenCaptureService.swift b/desktop/Desktop/Sources/ScreenCaptureService.swift index 48a0a80909..d25dc45734 100644 --- a/desktop/Desktop/Sources/ScreenCaptureService.swift +++ b/desktop/Desktop/Sources/ScreenCaptureService.swift @@ -25,9 +25,14 @@ final class ScreenCaptureService: Sendable { /// Must be accessed only while holding axStateLock. nonisolated(unsafe) private static var axSystemwideDisabled = false - /// Cache the last successfully resolved active window to avoid losing capture - /// when the resolver times out or transiently fails. - private struct ActiveWindowSnapshot { + /// Cache the last successfully resolved active window (with a non-nil windowID). + /// Used as a "last captureable context" fallback for up to activeWindowCacheTTL when + /// the resolver times out, transiently fails, or returns a nil-windowID result because + /// the frontmost app is a helper with no captureable window (LogiPluginService, Dock, + /// UserNotificationCenter, etc.) or CGWindowListCopyWindowInfo returns nothing. + /// Holding the cache prevents capture from dropping during brief transitions through + /// such apps, at the cost of reporting the previous app's name/title for up to TTL. + internal struct ActiveWindowSnapshot: Equatable { let appName: String? let windowTitle: String? let windowID: CGWindowID? @@ -35,6 +40,16 @@ final class ScreenCaptureService: Sendable { } nonisolated(unsafe) private static var lastActiveWindowSnapshot: ActiveWindowSnapshot? nonisolated(unsafe) private static var isActiveWindowResolutionInFlight = false + /// Tracks whether we are currently inside a streak of nil-window fallbacks so the + /// fallback log fires once per streak instead of once per capture frame. Reset on + /// any successful non-nil resolution. + nonisolated(unsafe) private static var isInNilWindowFallbackStreak = false + + /// Test-only seam: when set, replaces `resolveActiveWindowInfoWithTimeout()` so that + /// unit tests can drive `getActiveWindowInfoAsync()` with deterministic results + /// without touching NSWorkspace or CGWindowListCopyWindowInfo. + nonisolated(unsafe) internal static var _resolverOverrideForTests: + (@Sendable () async -> (appName: String?, windowTitle: String?, windowID: CGWindowID?)?)? init() {} @@ -422,8 +437,19 @@ final class ScreenCaptureService: Sendable { } } - let resolved = await resolveActiveWindowInfoWithTimeout() - if let resolved { + let resolved: (appName: String?, windowTitle: String?, windowID: CGWindowID?)? + if let override = _resolverOverrideForTests { + resolved = await override() + } else { + resolved = await resolveActiveWindowInfoWithTimeout() + } + + // Only cache snapshots with a non-nil windowID. A (appName, nil, nil) result means + // the current frontmost app is a helper with no captureable window (LogiPluginService, + // Dock, UserNotificationCenter, etc.) or window enumeration transiently returned + // nothing; overwriting the cache with that would poison it for the full TTL and + // silently break capture until the user focused a new "real" window. + if let resolved, resolved.windowID != nil { let snapshot = ActiveWindowSnapshot( appName: resolved.appName, windowTitle: resolved.windowTitle, @@ -432,19 +458,104 @@ final class ScreenCaptureService: Sendable { ) axStateLock.withLock { lastActiveWindowSnapshot = snapshot + isInNilWindowFallbackStreak = false } return resolved } + // Resolver returned nil or a nil-windowID result. Prefer the last known good + // captureable context for up to activeWindowCacheTTL to preserve recording across + // brief helper-app transitions and window enumeration hiccups. if let cached = getCachedActiveWindowSnapshot() { - log("ScreenCaptureService: Active window lookup timed out, using cached window info") + let shouldLog = axStateLock.withLock { () -> Bool in + if isInNilWindowFallbackStreak { + return false + } + isInNilWindowFallbackStreak = true + return true + } + if shouldLog { + if resolved == nil { + log( + "ScreenCaptureService: Active window lookup timed out, using cached window info" + ) + } else { + log( + "ScreenCaptureService: Frontmost app has no captureable window; " + + "using last known good window (\(cached.appName ?? "?"))" + ) + } + } return (cached.appName, cached.windowTitle, cached.windowID) } + if let resolved { + // Nil-windowID resolution with no usable cache — return the raw result so + // callers can apply their own exclusion/metadata handling. + return resolved + } + log("ScreenCaptureService: Active window lookup timed out with no cached fallback") return (nil, nil, nil) } + // MARK: - Test-only helpers + + /// Clears `lastActiveWindowSnapshot`, the in-flight guard, and the fallback-log streak + /// flag under the cache lock. Test-only. + internal static func _resetActiveWindowCacheForTests() { + axStateLock.withLock { + lastActiveWindowSnapshot = nil + isActiveWindowResolutionInFlight = false + isInNilWindowFallbackStreak = false + } + } + + /// Seeds `lastActiveWindowSnapshot` with a custom `resolvedAt` so tests can + /// exercise fresh, expired, or missing cache cases deterministically. Test-only. + internal static func _seedActiveWindowCacheForTests( + appName: String?, + windowTitle: String?, + windowID: CGWindowID?, + resolvedAt: Date + ) { + axStateLock.withLock { + lastActiveWindowSnapshot = ActiveWindowSnapshot( + appName: appName, + windowTitle: windowTitle, + windowID: windowID, + resolvedAt: resolvedAt + ) + } + } + + /// Returns a copy of the current cached snapshot (ignoring TTL) for test assertions. + internal static func _peekActiveWindowCacheForTests() -> ActiveWindowSnapshot? { + axStateLock.withLock { lastActiveWindowSnapshot } + } + + /// Returns the current value of `isInNilWindowFallbackStreak` for test assertions. + internal static func _peekNilWindowFallbackStreakForTests() -> Bool { + axStateLock.withLock { isInNilWindowFallbackStreak } + } + + /// Returns the current value of `isActiveWindowResolutionInFlight` for test assertions. + internal static func _peekIsResolutionInFlightForTests() -> Bool { + axStateLock.withLock { isActiveWindowResolutionInFlight } + } + + /// Forces `isActiveWindowResolutionInFlight = true` so tests can assert it is + /// cleared by `_resetActiveWindowCacheForTests()`. + internal static func _forceResolutionInFlightForTests(_ value: Bool) { + axStateLock.withLock { isActiveWindowResolutionInFlight = value } + } + + /// Forces `isInNilWindowFallbackStreak = true` so tests can assert it is + /// cleared by `_resetActiveWindowCacheForTests()`. + internal static func _forceNilWindowFallbackStreakForTests(_ value: Bool) { + axStateLock.withLock { isInNilWindowFallbackStreak = value } + } + private static func resolveActiveWindowInfoWithTimeout() async -> ( appName: String?, windowTitle: String?, windowID: CGWindowID? )? { diff --git a/desktop/Desktop/Tests/ScreenCaptureResolverCacheTests.swift b/desktop/Desktop/Tests/ScreenCaptureResolverCacheTests.swift new file mode 100644 index 0000000000..e87b1c0440 --- /dev/null +++ b/desktop/Desktop/Tests/ScreenCaptureResolverCacheTests.swift @@ -0,0 +1,239 @@ +import CoreGraphics +import XCTest + +@testable import Omi_Computer + +/// Tests for the "last known good captureable window" fallback policy in +/// `ScreenCaptureService.getActiveWindowInfoAsync()`. Issue #6640: a helper app +/// without a captureable window (LogiPluginService, Dock, UserNotificationCenter) +/// used to poison the cache with a nil-windowID snapshot, silently breaking +/// screen capture for up to `activeWindowCacheTTL` seconds per transition. +final class ScreenCaptureResolverCacheTests: XCTestCase { + + override func setUp() { + super.setUp() + ScreenCaptureService._resetActiveWindowCacheForTests() + ScreenCaptureService._resolverOverrideForTests = nil + } + + override func tearDown() { + ScreenCaptureService._resetActiveWindowCacheForTests() + ScreenCaptureService._resolverOverrideForTests = nil + super.tearDown() + } + + // MARK: - Happy path + + func testSuccessfulResolutionOverwritesCache() async { + ScreenCaptureService._resolverOverrideForTests = { + (appName: "Safari", windowTitle: "GitHub", windowID: CGWindowID(42)) + } + + let result = await ScreenCaptureService.getActiveWindowInfoAsync() + + XCTAssertEqual(result.appName, "Safari") + XCTAssertEqual(result.windowTitle, "GitHub") + XCTAssertEqual(result.windowID, CGWindowID(42)) + + let cached = ScreenCaptureService._peekActiveWindowCacheForTests() + XCTAssertEqual(cached?.appName, "Safari") + XCTAssertEqual(cached?.windowID, CGWindowID(42)) + } + + // MARK: - The bug: nil-window poisoning + + func testNilWindowIDResolutionDoesNotPoisonFreshCache() async { + // Seed a fresh good snapshot (as if capture was working moments ago). + ScreenCaptureService._seedActiveWindowCacheForTests( + appName: "Safari", + windowTitle: "GitHub", + windowID: CGWindowID(42), + resolvedAt: Date() + ) + let seededSnapshot = ScreenCaptureService._peekActiveWindowCacheForTests() + + // Simulate frontmost switching to a helper app with no captureable window. + ScreenCaptureService._resolverOverrideForTests = { + (appName: "LogiPluginService", windowTitle: nil, windowID: nil) + } + + let result = await ScreenCaptureService.getActiveWindowInfoAsync() + + // Caller sees the last known good window, not nil. + XCTAssertEqual(result.windowID, CGWindowID(42)) + XCTAssertEqual(result.appName, "Safari") + XCTAssertEqual(result.windowTitle, "GitHub") + + // Cache is unchanged: still the Safari snapshot, same timestamp. + let cached = ScreenCaptureService._peekActiveWindowCacheForTests() + XCTAssertEqual(cached, seededSnapshot) + } + + func testNilWindowIDResolutionWithNoCacheReturnsResolvedAsIs() async { + ScreenCaptureService._resolverOverrideForTests = { + (appName: "LogiPluginService", windowTitle: nil, windowID: nil) + } + + let result = await ScreenCaptureService.getActiveWindowInfoAsync() + + XCTAssertEqual(result.appName, "LogiPluginService") + XCTAssertNil(result.windowID) + XCTAssertNil(result.windowTitle) + XCTAssertNil( + ScreenCaptureService._peekActiveWindowCacheForTests(), + "nil-windowID result must not be written to the cache" + ) + } + + func testExpiredCacheIsNotReusedForNilWindowResolution() async { + // Seed an expired snapshot (>activeWindowCacheTTL in the past). The service + // uses 2s TTL, so 10s ago is safely expired. + ScreenCaptureService._seedActiveWindowCacheForTests( + appName: "Safari", + windowTitle: "GitHub", + windowID: CGWindowID(42), + resolvedAt: Date().addingTimeInterval(-10) + ) + + ScreenCaptureService._resolverOverrideForTests = { + (appName: "LogiPluginService", windowTitle: nil, windowID: nil) + } + + let result = await ScreenCaptureService.getActiveWindowInfoAsync() + + // Expired cache must not be used; resolver's raw nil-window result is returned. + XCTAssertEqual(result.appName, "LogiPluginService") + XCTAssertNil(result.windowID) + } + + // MARK: - Timeout path + + func testTimeoutPathWithFreshCacheStillFallsBack() async { + ScreenCaptureService._seedActiveWindowCacheForTests( + appName: "Safari", + windowTitle: "GitHub", + windowID: CGWindowID(42), + resolvedAt: Date() + ) + + // Simulate the resolver hitting the timeout race (returning nil). + ScreenCaptureService._resolverOverrideForTests = { nil } + + let result = await ScreenCaptureService.getActiveWindowInfoAsync() + + XCTAssertEqual(result.windowID, CGWindowID(42)) + XCTAssertEqual(result.appName, "Safari") + } + + func testTimeoutPathWithNoCacheReturnsAllNil() async { + ScreenCaptureService._resolverOverrideForTests = { nil } + + let result = await ScreenCaptureService.getActiveWindowInfoAsync() + + XCTAssertNil(result.appName) + XCTAssertNil(result.windowTitle) + XCTAssertNil(result.windowID) + } + + // MARK: - Recovery + + func testSuccessfulResolutionAfterFallbackOverwritesCache() async { + ScreenCaptureService._seedActiveWindowCacheForTests( + appName: "Safari", + windowTitle: "GitHub", + windowID: CGWindowID(42), + resolvedAt: Date() + ) + + // First: helper app transition -> fallback to Safari. + ScreenCaptureService._resolverOverrideForTests = { + (appName: "LogiPluginService", windowTitle: nil, windowID: nil) + } + _ = await ScreenCaptureService.getActiveWindowInfoAsync() + + // Then: user refocuses a real window. + ScreenCaptureService._resolverOverrideForTests = { + (appName: "Xcode", windowTitle: "Project.swift", windowID: CGWindowID(99)) + } + let result = await ScreenCaptureService.getActiveWindowInfoAsync() + + XCTAssertEqual(result.windowID, CGWindowID(99)) + XCTAssertEqual(result.appName, "Xcode") + + let cached = ScreenCaptureService._peekActiveWindowCacheForTests() + XCTAssertEqual(cached?.windowID, CGWindowID(99)) + XCTAssertEqual(cached?.appName, "Xcode") + } + + // MARK: - Fallback streak flag (P7) + + func testNilWindowFallbackStreakIsSetOnFirstFallbackAndIdempotent() async { + ScreenCaptureService._seedActiveWindowCacheForTests( + appName: "Safari", + windowTitle: "GitHub", + windowID: CGWindowID(42), + resolvedAt: Date() + ) + ScreenCaptureService._resolverOverrideForTests = { + (appName: "LogiPluginService", windowTitle: nil, windowID: nil) + } + + XCTAssertFalse(ScreenCaptureService._peekNilWindowFallbackStreakForTests()) + + _ = await ScreenCaptureService.getActiveWindowInfoAsync() + XCTAssertTrue( + ScreenCaptureService._peekNilWindowFallbackStreakForTests(), + "first nil-window fallback should enter the streak" + ) + + _ = await ScreenCaptureService.getActiveWindowInfoAsync() + _ = await ScreenCaptureService.getActiveWindowInfoAsync() + XCTAssertTrue( + ScreenCaptureService._peekNilWindowFallbackStreakForTests(), + "consecutive nil-window fallbacks should stay in the streak (idempotent)" + ) + } + + func testSuccessfulResolutionClearsNilWindowFallbackStreak() async { + ScreenCaptureService._seedActiveWindowCacheForTests( + appName: "Safari", + windowTitle: "GitHub", + windowID: CGWindowID(42), + resolvedAt: Date() + ) + ScreenCaptureService._forceNilWindowFallbackStreakForTests(true) + + ScreenCaptureService._resolverOverrideForTests = { + (appName: "Xcode", windowTitle: "Project.swift", windowID: CGWindowID(99)) + } + _ = await ScreenCaptureService.getActiveWindowInfoAsync() + + XCTAssertFalse( + ScreenCaptureService._peekNilWindowFallbackStreakForTests(), + "successful non-nil resolution must clear the streak" + ) + } + + // MARK: - Test-only reset (P8) + + func testResetActiveWindowCacheClearsAllState() { + ScreenCaptureService._seedActiveWindowCacheForTests( + appName: "Safari", + windowTitle: "GitHub", + windowID: CGWindowID(42), + resolvedAt: Date() + ) + ScreenCaptureService._forceResolutionInFlightForTests(true) + ScreenCaptureService._forceNilWindowFallbackStreakForTests(true) + + XCTAssertNotNil(ScreenCaptureService._peekActiveWindowCacheForTests()) + XCTAssertTrue(ScreenCaptureService._peekIsResolutionInFlightForTests()) + XCTAssertTrue(ScreenCaptureService._peekNilWindowFallbackStreakForTests()) + + ScreenCaptureService._resetActiveWindowCacheForTests() + + XCTAssertNil(ScreenCaptureService._peekActiveWindowCacheForTests()) + XCTAssertFalse(ScreenCaptureService._peekIsResolutionInFlightForTests()) + XCTAssertFalse(ScreenCaptureService._peekNilWindowFallbackStreakForTests()) + } +}