From a1d740fc007cdbbbccda56247f258f82b1cfbc1d Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Thu, 16 Jul 2026 09:01:52 -0400 Subject: [PATCH 1/2] fix(capture): honor cancellation during screen-recording permission retry The permission probe retry slept with try?, so a canceled caller could still run a second SCShareableContent probe. Recheck cancellation after sleep and return false without another probe. Signed-off-by: Sebastien Tardif --- CHANGELOG.md | 1 + .../Services/Capture/ScreenCapturePermissionGate.swift | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3707dc74c..caf26f8ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Refresh AXorcist, Commander, Swiftdansi, Tachikoma, and TauTUI, including stricter SwiftPM checkout handling for Tachikoma's Commander dependency plus corrected AXorcist app resolution, attribute serialization, and descendant filtering. ### Fixed +- Canceling during the screen-recording permission probe retry no longer continues a second ScreenCaptureKit shareable-content probe. Thanks @SebTardif. - Resolve applications by executable name, so `--app ` finds an app by the process/binary name shown in `ps`, `pgrep`, and Activity Monitor even when it differs from the app's localized name (e.g. an `openclaw-desktop` binary whose bundle name is "OpenClaw Desktop Test"). - Keep bridge acceptance and request handling responsive, and retry timed-out snapshot invalidation handshakes once so busy local endpoints are not mistaken for stale sockets. diff --git a/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/Capture/ScreenCapturePermissionGate.swift b/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/Capture/ScreenCapturePermissionGate.swift index 8d2ee6228..285e43203 100644 --- a/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/Capture/ScreenCapturePermissionGate.swift +++ b/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/Capture/ScreenCapturePermissionGate.swift @@ -24,7 +24,11 @@ struct ScreenRecordingPermissionChecker: ScreenRecordingPermissionEvaluating { if let delay = ScreenCaptureKitTransientError.retryDelayNanoseconds(after: error) { logger.warning( "Screen recording permission probe hit transient ScreenCaptureKit denial; retrying once") + // `try?` would swallow CancellationError and continue a second SCK probe after cancel. try? await Task.sleep(nanoseconds: delay) + guard !Task.isCancelled else { + return false + } do { _ = try await ScreenCaptureKitCaptureGate.currentShareableContent() logger.info("Screen recording permission granted (SCShareableContent retry)") From ece4a67843d9e4be5ee7ae7bd9be54eff3b0d8d5 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Mon, 20 Jul 2026 17:22:01 -0400 Subject: [PATCH 2/2] fix(capture): cancel during permission retry skips second SCK probe --- CHANGELOG.md | 1 - .../Capture/ScreenCapturePermissionGate.swift | 30 +++++- ...ScreenRecordingPermissionCancelTests.swift | 58 ++++++++++++ ...rove-screencapture-permission-cancel.swift | 92 +++++++++++++++++++ 4 files changed, 175 insertions(+), 6 deletions(-) create mode 100644 Core/PeekabooAutomationKit/Tests/PeekabooAutomationKitTests/ScreenRecordingPermissionCancelTests.swift create mode 100644 scripts/prove-screencapture-permission-cancel.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index caf26f8ae..3707dc74c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,6 @@ - Refresh AXorcist, Commander, Swiftdansi, Tachikoma, and TauTUI, including stricter SwiftPM checkout handling for Tachikoma's Commander dependency plus corrected AXorcist app resolution, attribute serialization, and descendant filtering. ### Fixed -- Canceling during the screen-recording permission probe retry no longer continues a second ScreenCaptureKit shareable-content probe. Thanks @SebTardif. - Resolve applications by executable name, so `--app ` finds an app by the process/binary name shown in `ps`, `pgrep`, and Activity Monitor even when it differs from the app's localized name (e.g. an `openclaw-desktop` binary whose bundle name is "OpenClaw Desktop Test"). - Keep bridge acceptance and request handling responsive, and retry timed-out snapshot invalidation handshakes once so busy local endpoints are not mistaken for stale sockets. diff --git a/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/Capture/ScreenCapturePermissionGate.swift b/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/Capture/ScreenCapturePermissionGate.swift index 285e43203..a12d80d1f 100644 --- a/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/Capture/ScreenCapturePermissionGate.swift +++ b/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/Capture/ScreenCapturePermissionGate.swift @@ -6,9 +6,29 @@ import Foundation func hasPermission(logger: CategoryLogger) async -> Bool } -struct ScreenRecordingPermissionChecker: ScreenRecordingPermissionEvaluating { - func hasPermission(logger: CategoryLogger) async -> Bool { - let preflightResult = CGPreflightScreenCaptureAccess() +@MainActor +@_spi(Testing) public struct ScreenRecordingPermissionChecker: ScreenRecordingPermissionEvaluating { + private let preflight: @MainActor @Sendable () -> Bool + private let shareableContentProbe: @MainActor @Sendable () async throws -> Void + + public init() { + self.preflight = { CGPreflightScreenCaptureAccess() } + self.shareableContentProbe = { + _ = try await ScreenCaptureKitCaptureGate.currentShareableContent() + } + } + + /// Test seam: inject preflight and shareable-content probe results. + @_spi(Testing) public init( + preflight: @escaping @MainActor @Sendable () -> Bool, + shareableContentProbe: @escaping @MainActor @Sendable () async throws -> Void) + { + self.preflight = preflight + self.shareableContentProbe = shareableContentProbe + } + + public func hasPermission(logger: CategoryLogger) async -> Bool { + let preflightResult = self.preflight() if preflightResult { return true } @@ -17,7 +37,7 @@ struct ScreenRecordingPermissionChecker: ScreenRecordingPermissionEvaluating { // granted because TCC tracks by code signature and the check can fail after rebuilds or for non-.app bundles. logger.debug("CGPreflightScreenCaptureAccess returned false, probing SCShareableContent") do { - _ = try await ScreenCaptureKitCaptureGate.currentShareableContent() + try await self.shareableContentProbe() logger.info("Screen recording permission granted (SCShareableContent probe)") return true } catch { @@ -30,7 +50,7 @@ struct ScreenRecordingPermissionChecker: ScreenRecordingPermissionEvaluating { return false } do { - _ = try await ScreenCaptureKitCaptureGate.currentShareableContent() + try await self.shareableContentProbe() logger.info("Screen recording permission granted (SCShareableContent retry)") return true } catch { diff --git a/Core/PeekabooAutomationKit/Tests/PeekabooAutomationKitTests/ScreenRecordingPermissionCancelTests.swift b/Core/PeekabooAutomationKit/Tests/PeekabooAutomationKitTests/ScreenRecordingPermissionCancelTests.swift new file mode 100644 index 000000000..09999640c --- /dev/null +++ b/Core/PeekabooAutomationKit/Tests/PeekabooAutomationKitTests/ScreenRecordingPermissionCancelTests.swift @@ -0,0 +1,58 @@ +import Foundation +import XCTest +@testable @_spi(Testing) import PeekabooAutomationKit + +@MainActor +final class ScreenRecordingPermissionCancelTests: XCTestCase { + func testCancelDuringTransientPermissionRetrySkipsSecondProbe() async { + let logging = MockLoggingService() + let logger = logging.logger(category: "test") + var probeCount = 0 + + let checker = ScreenRecordingPermissionChecker( + preflight: { false }, + shareableContentProbe: { + probeCount += 1 + throw NSError( + domain: "com.apple.ScreenCaptureKit.SCStreamErrorDomain", + code: -3801, + userInfo: [ + NSLocalizedDescriptionKey: "The user declined TCCs for application, window, display capture", + ]) + }) + + let task = Task { @MainActor in + await checker.hasPermission(logger: logger) + } + + // Cancel during the 350ms transient-denial sleep. + try? await Task.sleep(nanoseconds: 50_000_000) + task.cancel() + let granted = await task.value + + XCTAssertFalse(granted) + XCTAssertEqual(probeCount, 1, "canceled permission probe must not run a second SCShareableContent call") + } + + func testNonCancelledTransientPermissionRetryProbesTwice() async { + let logging = MockLoggingService() + let logger = logging.logger(category: "test") + var probeCount = 0 + + let checker = ScreenRecordingPermissionChecker( + preflight: { false }, + shareableContentProbe: { + probeCount += 1 + throw NSError( + domain: "com.apple.ScreenCaptureKit.SCStreamErrorDomain", + code: -3801, + userInfo: [ + NSLocalizedDescriptionKey: "The user declined TCCs for application, window, display capture", + ]) + }) + + let granted = await checker.hasPermission(logger: logger) + XCTAssertFalse(granted) + XCTAssertEqual(probeCount, 2, "non-cancelled transient denial still retries once") + } +} diff --git a/scripts/prove-screencapture-permission-cancel.swift b/scripts/prove-screencapture-permission-cancel.swift new file mode 100644 index 000000000..40746e1c5 --- /dev/null +++ b/scripts/prove-screencapture-permission-cancel.swift @@ -0,0 +1,92 @@ +// Standalone proof for fix/screencapture-permission-honor-cancel (PR #280). +// Mirrors ScreenRecordingPermissionChecker transient denial retry: +// unfixed: try? sleep then second probe even after cancel +// fixed: try? sleep + guard !Task.isCancelled before second probe +// +// Usage: +// swiftc -parse-as-library -o /tmp/prove-sck-perm scripts/prove-screencapture-permission-cancel.swift +// /tmp/prove-sck-perm + +import Foundation + +func unfixedPermissionProbe(probe: @escaping () async throws -> Void) async -> (probes: Int, elapsedMs: Double, granted: Bool) { + var probes = 0 + let start = Date() + do { + probes += 1 + try await probe() + return (probes, Date().timeIntervalSince(start) * 1000, true) + } catch { + try? await Task.sleep(nanoseconds: 350_000_000) + // No cancel guard — second probe always runs. + do { + probes += 1 + try await probe() + return (probes, Date().timeIntervalSince(start) * 1000, true) + } catch { + return (probes, Date().timeIntervalSince(start) * 1000, false) + } + } +} + +func fixedPermissionProbe(probe: @escaping () async throws -> Void) async -> (probes: Int, elapsedMs: Double, granted: Bool) { + var probes = 0 + let start = Date() + do { + probes += 1 + try await probe() + return (probes, Date().timeIntervalSince(start) * 1000, true) + } catch { + try? await Task.sleep(nanoseconds: 350_000_000) + guard !Task.isCancelled else { + return (probes, Date().timeIntervalSince(start) * 1000, false) + } + do { + probes += 1 + try await probe() + return (probes, Date().timeIntervalSince(start) * 1000, true) + } catch { + return (probes, Date().timeIntervalSince(start) * 1000, false) + } + } +} + +@main +struct Proof { + static func main() async { + print("=== Screen-recording permission probe cancel proof (PR #280) ===") + print("Pattern matches ScreenRecordingPermissionChecker.hasPermission") + print() + + let failing: () async throws -> Void = { + throw NSError(domain: "com.apple.ScreenCaptureKit.SCStreamErrorDomain", code: -3801) + } + + print("Test 1: Unfixed — cancel after 50ms into 350ms delay") + let unfixedTask = Task { + await unfixedPermissionProbe(probe: failing) + } + try? await Task.sleep(nanoseconds: 50_000_000) + unfixedTask.cancel() + let unfixed = await unfixedTask.value + print(" probes=\(unfixed.probes) elapsed_ms=\(String(format: "%.0f", unfixed.elapsedMs)) granted=\(unfixed.granted)") + + print("Test 2: Fixed — cancel after 50ms into 350ms delay") + let fixedTask = Task { + await fixedPermissionProbe(probe: failing) + } + try? await Task.sleep(nanoseconds: 50_000_000) + fixedTask.cancel() + let fixed = await fixedTask.value + print(" probes=\(fixed.probes) elapsed_ms=\(String(format: "%.0f", fixed.elapsedMs)) granted=\(fixed.granted)") + print() + + if unfixed.probes >= 2, fixed.probes == 1, fixed.granted == false { + print("PASS: unfixed ran second SCShareableContent probe after cancel (\(unfixed.probes) probes)") + print("PASS: fixed stopped at first probe without second SCK call (\(fixed.probes) probe, \(String(format: "%.0f", fixed.elapsedMs))ms)") + } else { + print("FAIL") + exit(1) + } + } +}