Skip to content
Merged
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
2 changes: 1 addition & 1 deletion ScreenScribe/Sources/Onboarding/PermissionStepView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ struct PermissionStepView: View {
Button(action: {
Task {
isCheckingPermission = true
await permissionManager.requestPermissionAndStartMonitoring()
await permissionManager.requestPermissionAndStartMonitoring(userInitiated: true)
isCheckingPermission = false
}
}) {
Expand Down
88 changes: 64 additions & 24 deletions ScreenScribe/Sources/Services/ScreenCapturePermissionManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ final class ScreenCapturePermissionManager: ObservableObject {
/// Published property for reactive UI updates
@Published private(set) var hasPermission: Bool = false

/// UserDefaults keys
private let verifiedPermissionKey = "hasVerifiedScreenRecordingPermission"
private let hasRequestedPermissionKey = "hasRequestedScreenRecordingPermission"
private let lastPromptDateKey = "lastScreenRecordingPermissionPromptDate"

/// UserDefaults instance for persistence
private let defaults = UserDefaults.standard

/// Timer for periodic permission checking
private var permissionCheckTimer: Timer?

Expand All @@ -23,10 +31,18 @@ final class ScreenCapturePermissionManager: ObservableObject {
/// Timestamp of the last time we showed the system permission dialog
private var lastPopupTime: Date?

/// On macOS Sequoia/Tahoe and newer, permission checks can be flaky
private var isModernMacOS: Bool {
ProcessInfo.processInfo.operatingSystemVersion.majorVersion >= 15
}

private init() {
Comment on lines +35 to 39
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

The isModernMacOS property uses majorVersion >= 15 to detect macOS Sequoia/Tahoe and newer. However, this is a computed property that's called multiple times throughout the code. Consider making this a stored property initialized once in init() for better performance, or add a comment explaining why it needs to be computed (in case the OS version could change at runtime, which is unlikely).

Suggested change
private var isModernMacOS: Bool {
ProcessInfo.processInfo.operatingSystemVersion.majorVersion >= 15
}
private init() {
private let isModernMacOS: Bool
private init() {
self.isModernMacOS = ProcessInfo.processInfo.operatingSystemVersion.majorVersion >= 15

Copilot uses AI. Check for mistakes.
// Only use safe, read-only check on init
// CGPreflightScreenCaptureAccess does NOT trigger any dialog
hasPermission = CGPreflightScreenCaptureAccess()
if hasPermission {
defaults.set(true, forKey: verifiedPermissionKey)
}
// Note: On macOS Sequoia, this may return false even when permission is granted
// The verification via ScreenCaptureKit will happen when explicitly requested
// via requestPermissionAndStartMonitoring()
Expand All @@ -52,14 +68,14 @@ final class ScreenCapturePermissionManager: ObservableObject {
hasPermission = true
stopPolling()
// Track successful permission verification
UserDefaults.standard.set(true, forKey: "hasVerifiedScreenRecordingPermission")
defaults.set(true, forKey: verifiedPermissionKey)
Logger.log(.info, "ScreenCaptureKit permission verified on attempt \(attempt)")
return .granted
} catch let error as NSError {
// Error code -3801 (SCStreamErrorUserDeclined) indicates user explicitly denied permission
// Error code -3802 (SCStreamErrorFailedToStart) can also indicate permission issues
// On macOS Sequoia/Tahoe, these can also occur transiently during cold boot
let isDefinitiveDenial = error.code == -3801
// On macOS Sequoia/Tahoe+, -3801 can still be transient, so only trust it on older macOS
let isDefinitiveDenial = error.code == -3801 && !isModernMacOS
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

On modern macOS (15+), error code -3801 is never treated as a definitive denial due to the condition !isModernMacOS. This means if a user explicitly revokes a previously granted permission in System Settings, verifyPermissionViaScreenCaptureKit returns .transientError instead of .denied, and the verifiedPermissionKey is never cleared. With hadPermissionBefore remaining true, the app will suppress automatic permission dialogs and poll silently indefinitely. The user would need to manually click a "Recheck Permission" button to trigger a new prompt. Consider adding logic to detect when permission has been revoked (e.g., checking for consistent -3801 errors over multiple polling iterations) and clearing the verified flag in that case, or documenting this as expected behavior.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Handle real -3801 denials on modern macOS

In verifyPermissionViaScreenCaptureKit, -3801 is no longer treated as a definitive denial on macOS 15+ (isDefinitiveDenial at line 78), but the downstream flow still routes prior-granted users through the transient path that only starts polling. When a user actually revokes Screen Recording permission after previously granting it, requestPermissionAndStartMonitoring (called automatically from App.proceedWithNormalStartup()) now keeps polling and never re-requests access, so the app can remain stuck in limited mode without an automatic recovery prompt.

Useful? React with 👍 / 👎.


Logger.log(.info, "ScreenCaptureKit attempt \(attempt)/\(maxAttempts) failed: \(error.localizedDescription) (domain: \(error.domain), code: \(error.code), definitiveDenial: \(isDefinitiveDenial))")

Expand Down Expand Up @@ -88,12 +104,13 @@ final class ScreenCapturePermissionManager: ObservableObject {
}

/// Request permission and start monitoring for changes
func requestPermissionAndStartMonitoring() async {
Logger.log(.info, "Starting permission request and monitoring...")
func requestPermissionAndStartMonitoring(userInitiated: Bool = false) async {
Logger.log(.info, "Starting permission request and monitoring (userInitiated: \(userInitiated))...")

// Check if user previously had permission verified successfully
// This is more reliable than just checking onboarding completion
let hadPermissionBefore = UserDefaults.standard.bool(forKey: "hasVerifiedScreenRecordingPermission")
let hadPermissionBefore = defaults.bool(forKey: verifiedPermissionKey)
let hasRequestedBefore = defaults.bool(forKey: hasRequestedPermissionKey)

// First, check via ScreenCaptureKit with retries (more reliable on macOS Sequoia/Tahoe)
// CGPreflightScreenCaptureAccess can return false even when permission is granted
Expand All @@ -114,47 +131,62 @@ final class ScreenCapturePermissionManager: ObservableObject {
if CGPreflightScreenCaptureAccess() {
Logger.log(.info, "Permission detected via CGPreflightScreenCaptureAccess after ScreenCaptureKit failed")
hasPermission = true
UserDefaults.standard.set(true, forKey: "hasVerifiedScreenRecordingPermission")
defaults.set(true, forKey: verifiedPermissionKey)
stopPolling()
return
}

let shouldSuppressAutomaticDialog = isModernMacOS
&& !userInitiated
&& (hadPermissionBefore || hasRequestedBefore)

// Decision logic for showing the system permission dialog:
// - If we got a definitive denial: user explicitly declined/revoked, show dialog
// - If we got transient errors AND user had permission before: likely cold boot issue, poll silently
// - If we got transient errors AND user never had permission: new user, show dialog
// - On modern macOS, avoid repeated automatic dialogs after initial prompt/verification
// - Allow explicit user-initiated checks to request again when needed
// - Poll silently when permission checks are likely transient

switch verificationResult {
case .granted:
// Already handled above
break

case .denied:
// User explicitly declined or revoked permission
// Clear the "had permission" flag and show dialog
Logger.log(.info, "Permission was explicitly denied/revoked, showing system dialog")
UserDefaults.standard.set(false, forKey: "hasVerifiedScreenRecordingPermission")
showSystemDialogIfCooldownExpired()
// User explicitly declined or revoked permission.
// Clear the verified flag, then either prompt or suppress based on platform/trigger.
defaults.set(false, forKey: verifiedPermissionKey)

if shouldSuppressAutomaticDialog {
Logger.log(.info, "Suppressing automatic system dialog on modern macOS after prior prompt/verification. Polling silently.")
startPolling()
} else {
Logger.log(.info, "Permission was denied/revoked, showing system dialog")
showSystemDialogIfCooldownExpired(userInitiated: userInitiated)
}

case .transientError:
if hadPermissionBefore {
// User had permission before, this is likely a macOS Sequoia/Tahoe cold-boot timing issue
// Don't spam them with dialogs — just poll silently until the system catches up
Logger.log(.info, "Permission was verified before; transient error likely due to cold boot. Polling silently.")
if hadPermissionBefore || shouldSuppressAutomaticDialog {
// On modern macOS, permission checks can fail transiently even after prior prompt/verification.
// Poll silently until the system catches up instead of repeatedly showing dialogs.
Logger.log(.info, "Transient permission check failure; polling silently instead of showing another automatic dialog.")
startPolling()
} else {
// New user who never had permission, show the dialog
Logger.log(.info, "New user, showing system dialog")
showSystemDialogIfCooldownExpired()
showSystemDialogIfCooldownExpired(userInitiated: userInitiated)
}
}
}

/// Show the system permission dialog if cooldown has expired
private func showSystemDialogIfCooldownExpired() {
private func showSystemDialogIfCooldownExpired(userInitiated: Bool) {
let now = Date()
let shouldShowPopup: Bool
let persistedPopupTime = defaults.object(forKey: lastPromptDateKey) as? Date
let latestPopupTime = [lastPopupTime, persistedPopupTime].compactMap { $0 }.max()

if let lastTime = lastPopupTime {
if userInitiated {
shouldShowPopup = true
} else if let lastTime = latestPopupTime {
let timeSinceLastPopup = now.timeIntervalSince(lastTime)
shouldShowPopup = timeSinceLastPopup >= popupCooldownInterval
Comment on lines 198 to 199
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

The cooldown logic compares the current time with persisted dates using timeIntervalSince. If the system clock is adjusted backward or the persisted date is somehow in the future, timeIntervalSince could return a negative value, causing timeSinceLastPopup >= popupCooldownInterval to be false even when it should be true. This could result in the permission dialog being suppressed indefinitely until the user manually triggers a recheck. Consider adding a check to handle negative time intervals or dates in the future by treating them as if the cooldown has expired.

Copilot uses AI. Check for mistakes.

Expand All @@ -171,6 +203,13 @@ final class ScreenCapturePermissionManager: ObservableObject {
let result = CGRequestScreenCaptureAccess()
hasPermission = result
lastPopupTime = now
defaults.set(true, forKey: hasRequestedPermissionKey)
defaults.set(now, forKey: lastPromptDateKey)

if result {
defaults.set(true, forKey: verifiedPermissionKey)
stopPolling()
}
}

// If not granted, start polling
Expand All @@ -184,8 +223,9 @@ final class ScreenCapturePermissionManager: ObservableObject {
// Only use the safe read-only API
if CGPreflightScreenCaptureAccess() {
hasPermission = true
stopPolling()
// Track successful verification for future cold boot handling
UserDefaults.standard.set(true, forKey: "hasVerifiedScreenRecordingPermission")
defaults.set(true, forKey: verifiedPermissionKey)
return true
}
// Return the cached value (may have been updated by prior ScreenCaptureKit verification)
Expand Down Expand Up @@ -219,7 +259,7 @@ final class ScreenCapturePermissionManager: ObservableObject {
if !hasPermission {
hasPermission = true
stopPolling()
UserDefaults.standard.set(true, forKey: "hasVerifiedScreenRecordingPermission")
defaults.set(true, forKey: verifiedPermissionKey)
Logger.log(.info, "Screen capture permission granted (via CGPreflightScreenCaptureAccess)")
}
return
Expand Down