Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion desktop/CHANGELOG.json
Original file line number Diff line number Diff line change
@@ -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": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
101 changes: 95 additions & 6 deletions desktop/Desktop/Sources/ScreenCaptureService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,31 @@ 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?
let resolvedAt: Date
}
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() {}

Expand Down Expand Up @@ -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,
Expand All @@ -432,19 +458,82 @@ 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)
Comment on lines +492 to 499
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 isInNilWindowFallbackStreak not reset on the expired-cache / no-cache path

When the resolver returns a nil-windowID result and the cache is either expired or absent, execution falls through to return resolved / return (nil,nil,nil) without ever resetting isInNilWindowFallbackStreak. If a prior call left the flag true (e.g. a successful fallback streak while the cache was still fresh), and the cache then expires while the helper app is still frontmost, the very first log for the next fresh-cache fallback streak will be suppressed — the rate-limiter already thinks it's mid-streak.

This is a cosmetic logging issue only (not a correctness bug), but it's worth resetting the flag when no fallback is actually taken so the streak tracks only active fallback windows:

// No usable cache and resolver returned a nil-windowID result —
// return raw result and clear any stale streak state.
if let resolved {
    axStateLock.withLock { isInNilWindowFallbackStreak = false }
    return resolved
}
axStateLock.withLock { isInNilWindowFallbackStreak = false }
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 }
}

private static func resolveActiveWindowInfoWithTimeout() async -> (
appName: String?, windowTitle: String?, windowID: CGWindowID?
)? {
Expand Down
167 changes: 167 additions & 0 deletions desktop/Desktop/Tests/ScreenCaptureResolverCacheTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
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")
}
}