diff --git a/CHANGELOG.md b/CHANGELOG.md index 3707dc74c..009dfc500 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 screen capture during a transient ScreenCaptureKit denial sleep no longer proceeds into a full capture retry. 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/ScreenCaptureEngineSupport.swift b/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/Capture/ScreenCaptureEngineSupport.swift index d92a0690d..8bfeb1052 100644 --- a/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/Capture/ScreenCaptureEngineSupport.swift +++ b/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/Capture/ScreenCaptureEngineSupport.swift @@ -179,7 +179,9 @@ struct NullScreenCaptureMetricsObserver: ScreenCaptureMetricsObserving { "\(api.description) capture hit transient ScreenCaptureKit denial; retrying once", metadata: ["error": String(describing: error)], correlationId: correlationId) - try? await Task.sleep(nanoseconds: delay) + // `try?` would swallow CancellationError and burn a full capture retry after cancel. + try await Task.sleep(nanoseconds: delay) + try Task.checkCancellation() do { let start = Date() let result = try await attempt(api) @@ -254,7 +256,9 @@ struct NullScreenCaptureMetricsObserver: ScreenCaptureMetricsObserving { "\(api.description) capture hit transient ScreenCaptureKit denial; retrying once", metadata: ["error": String(describing: error)], correlationId: correlationId) - try? await Task.sleep(nanoseconds: delay) + // `try?` would swallow CancellationError and burn a full capture retry after cancel. + try await Task.sleep(nanoseconds: delay) + try Task.checkCancellation() do { let start = Date() let result = try await attempt(api) diff --git a/Core/PeekabooCore/Tests/PeekabooTests/ScreenCaptureFallbackRunnerTests.swift b/Core/PeekabooCore/Tests/PeekabooTests/ScreenCaptureFallbackRunnerTests.swift index 3f5993f7c..777854c21 100644 --- a/Core/PeekabooCore/Tests/PeekabooTests/ScreenCaptureFallbackRunnerTests.swift +++ b/Core/PeekabooCore/Tests/PeekabooTests/ScreenCaptureFallbackRunnerTests.swift @@ -150,4 +150,79 @@ struct ScreenCaptureFallbackRunnerTests { #expect(value == "ok_modern") #expect(calls == [.modern, .modern]) } + + /// Transient ScreenCaptureKit denial triggers a 350ms sleep. Cancelling during that sleep + /// must not invoke a second capture attempt (the pre-fix `try?` path swallowed cancel). + @MainActor + @Test + func `cancel during transient denial sleep skips second generic attempt`() async throws { + let logger = LoggingService(subsystem: "test.logger").logger(category: "test") + let runner = ScreenCaptureFallbackRunner(apis: [.modern]) + var attempts = 0 + + let task = Task { @MainActor in + try await runner.run( + operationName: "test", + logger: logger, + correlationId: "cancel-run") + { _ in + attempts += 1 + throw NSError( + domain: "com.apple.ScreenCaptureKit.SCStreamErrorDomain", + code: -3801, + userInfo: [ + NSLocalizedDescriptionKey: "The user declined TCCs for application, window, display capture", + ]) + } + } + + // Cancel while the 350ms transient-denial sleep is in progress. + try await Task.sleep(nanoseconds: 50_000_000) + task.cancel() + + do { + _ = try await task.value + Issue.record("expected cancellation without a second attempt") + } catch { + // CancellationError or first-attempt failure without retry is fine. + } + + #expect(attempts == 1) + } + + @MainActor + @Test + func `cancel during transient denial sleep skips second capture attempt`() async throws { + let logger = LoggingService(subsystem: "test.logger").logger(category: "test") + let runner = ScreenCaptureFallbackRunner(apis: [.modern]) + var attempts = 0 + + let task = Task { @MainActor in + try await runner.runCapture( + operationName: "captureScreen", + logger: logger, + correlationId: "cancel-capture") + { _ in + attempts += 1 + throw NSError( + domain: "com.apple.ScreenCaptureKit.SCStreamErrorDomain", + code: -3801, + userInfo: [ + NSLocalizedDescriptionKey: "The user declined TCCs for application, window, display capture", + ]) + } + } + + try await Task.sleep(nanoseconds: 50_000_000) + task.cancel() + + do { + _ = try await task.value + Issue.record("expected cancellation or failure without second attempt") + } catch { + // Expected: CancellationError or first-attempt error without retry. + } + + #expect(attempts == 1) + } } diff --git a/scripts/prove-screencapture-engine-cancel.swift b/scripts/prove-screencapture-engine-cancel.swift new file mode 100644 index 000000000..a97ebf0a5 --- /dev/null +++ b/scripts/prove-screencapture-engine-cancel.swift @@ -0,0 +1,103 @@ +// Standalone proof for fix/screencapture-engine-honor-cancel (PR #279). +// Mirrors ScreenCaptureFallbackRunner transient-denial sleep: +// unfixed: try? await Task.sleep — cancel is swallowed, second attempt runs +// fixed: try await Task.sleep + checkCancellation — second attempt skipped +// +// Usage: +// swiftc -parse-as-library -o /tmp/prove-sck-engine scripts/prove-screencapture-engine-cancel.swift +// /tmp/prove-sck-engine + +import Foundation + +enum TransientDenial: Error { + case declined +} + +/// Unfixed: try? swallows CancellationError during the 350ms denial sleep. +func unfixedRetry(attempt: @escaping () async throws -> Int) async -> (attempts: Int, elapsedMs: Double, outcome: String) { + var attempts = 0 + let start = Date() + do { + attempts += 1 + _ = try await attempt() + return (attempts, Date().timeIntervalSince(start) * 1000, "success") + } catch { + // Simulated ScreenCaptureKitTransientError.retryDelayNanoseconds == 350ms + try? await Task.sleep(nanoseconds: 350_000_000) + // No cancellation check — second attempt always runs after sleep returns. + do { + attempts += 1 + _ = try await attempt() + return (attempts, Date().timeIntervalSince(start) * 1000, "retry-success") + } catch { + return (attempts, Date().timeIntervalSince(start) * 1000, "retry-failed") + } + } +} + +/// Fixed: throwing sleep + checkCancellation. +func fixedRetry(attempt: @escaping () async throws -> Int) async -> (attempts: Int, elapsedMs: Double, outcome: String) { + var attempts = 0 + let start = Date() + do { + attempts += 1 + _ = try await attempt() + return (attempts, Date().timeIntervalSince(start) * 1000, "success") + } catch { + do { + try await Task.sleep(nanoseconds: 350_000_000) + try Task.checkCancellation() + attempts += 1 + _ = try await attempt() + return (attempts, Date().timeIntervalSince(start) * 1000, "retry-success") + } catch is CancellationError { + return (attempts, Date().timeIntervalSince(start) * 1000, "cancelled") + } catch { + return (attempts, Date().timeIntervalSince(start) * 1000, "retry-failed") + } + } +} + +@main +struct Proof { + static func main() async { + print("=== ScreenCaptureKit engine transient-denial cancel proof (PR #279) ===") + print("Pattern matches ScreenCaptureFallbackRunner.run / runCapture") + print() + + let failing: () async throws -> Int = { + throw TransientDenial.declined + } + + print("Test 1: Unfixed (try? sleep) — cancel after 50ms into 350ms delay") + let unfixedTask = Task { + await unfixedRetry(attempt: failing) + } + try? await Task.sleep(nanoseconds: 50_000_000) + unfixedTask.cancel() + let unfixed = await unfixedTask.value + print(" attempts=\(unfixed.attempts) elapsed_ms=\(String(format: "%.0f", unfixed.elapsedMs)) outcome=\(unfixed.outcome)") + + print("Test 2: Fixed (try sleep + checkCancellation) — cancel after 50ms into 350ms delay") + let fixedTask = Task { + await fixedRetry(attempt: failing) + } + try? await Task.sleep(nanoseconds: 50_000_000) + fixedTask.cancel() + let fixed = await fixedTask.value + print(" attempts=\(fixed.attempts) elapsed_ms=\(String(format: "%.0f", fixed.elapsedMs)) outcome=\(fixed.outcome)") + print() + + let unfixedBurnedRetry = unfixed.attempts >= 2 + let fixedStopped = fixed.attempts == 1 && fixed.outcome == "cancelled" + if unfixedBurnedRetry, fixedStopped { + print("PASS: unfixed burned a second capture attempt after cancel (\(unfixed.attempts) attempts)") + print("PASS: fixed cancelled during denial sleep without second attempt (\(fixed.attempts) attempt, \(String(format: "%.0f", fixed.elapsedMs))ms)") + } else { + print("FAIL: unexpected results") + print(" unfixed attempts=\(unfixed.attempts) outcome=\(unfixed.outcome)") + print(" fixed attempts=\(fixed.attempts) outcome=\(fixed.outcome)") + exit(1) + } + } +}