gui: deny execution of hold&ask events immediately if unavailable

[russellhancox]

Prior to this change, if a hold & ask execution triggered a Santa dialog but biometry is not available (and the EnableStandalonePasswordFallback key is not set to true) the execution would remain held until the dialog was dismissed. With this change, the execution will be denied immediately.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a guarded reply invocation in SNTBinaryMessageWindowView: introduces a repliedToCallback state and a centralized callReplyCallback(_:), routes all UI and response code paths through it to ensure replyCallback is invoked at most once and only when present.

Changes

Cohort / File(s)	Summary
Centralized reply handling Source/gui/SNTBinaryMessageWindowView.swift	Added repliedToCallback state and callReplyCallback(_:) helper; removed forced unwraps and direct replyCallback calls; all callback paths now use the guarded helper to ensure single invocation.
UI action & response updates Source/gui/SNTBinaryMessageWindowView.swift	Replaced direct callback calls in openButton, standAloneButton, dismissButton, and final response branches with callReplyCallback(...); adjusted DispatchQueue usage for standAloneButton when event is nil.
holdAndAsk TouchID error path Source/gui/SNTBinaryMessageWindowView.swift	When TouchID is unavailable in holdAndAsk flow, error display is wrapped in a Group and a .task now calls callReplyCallback(false) immediately.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: denying hold&ask executions immediately when biometry is unavailable, which matches the core objective of the PR.
Description check	✅ Passed	The description directly explains the behavior change, detailing what happened before and what happens now with the unavailability of biometric authentication.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

Source/gui/SNTBinaryMessageWindowView.swift (1)

290-300: Avoid repeated replyCallback invocations from SwiftUI body.

SwiftUI recomputes body frequently; calling replyCallback?(false) here can fire multiple times and can also race with user clicks on Dismiss/Open, resulting in duplicate replies. Gate the auto-deny to a single execution and consider closing/disable UI after auto-deny.

🔧 Proposed fix (single-shot auto-deny)

@@
   `@State` public var preventFutureNotifications = false
   `@State` public var preventFutureNotificationPeriod: TimeInterval = NotificationSilencePeriods[0]
+  `@State` private var didAutoDeny = false
@@
       if event?.holdAndAsk ?? false {
         let (canAuthz, err) = CanAuthorizeWithTouchID()
         if !canAuthz {
-          if let errMsg = err {
-            Text(errMsg.localizedDescription).foregroundColor(.red)
-          }
-
-          // If this is a holdAndAsk event but TouchID is not available,
-          // call the reply block immediately with false.
-          let _ = replyCallback?(false)
+          Group {
+            if let errMsg = err {
+              Text(errMsg.localizedDescription).foregroundColor(.red)
+            }
+          }
+          .task {
+            guard !didAutoDeny else { return }
+            didAutoDeny = true
+            replyCallback?(false)
+            window?.close() // Optional: prevents further user actions after reply
+          }
         }
       }

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

Source/gui/SNTBinaryMessageWindowView.swift (1)

371-385: Inconsistent callback invocation bypasses the guard.

The guard clause at lines 373-375 directly invokes replyCallback instead of using callReplyCallback. This bypasses the duplicate-call protection and doesn't set repliedToCallback, which could lead to the callback being invoked twice if another code path runs afterward.

🐛 Proposed fix

  func standAloneButton() {
    guard let e = self.event else {
-     if let cb = self.replyCallback {
-       cb(false)
-     }
+     callReplyCallback(false)
      return
    }

    SNTAuthorizationHelper.authorizeExecution(for: e) { success in
      callReplyCallback(success)
      DispatchQueue.main.sync {
        window?.close()
      }
    }
  }

🤖 Fix all issues with AI agents

In `@Source/gui/SNTBinaryMessageWindowView.swift`:
- Around line 257-263: callReplyCallback currently ignores its response
parameter and always invokes replyCallback(false); change it to pass the actual
response value and keep the early-return guard and repliedToCallback update.
Specifically, in function callReplyCallback(_ response: Bool) replace the
invocation of replyCallback with replyCallback(response) and ensure
repliedToCallback is set to true after invoking replyCallback; reference
symbols: callReplyCallback, replyCallback, repliedToCallback, response.

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

Source/gui/SNTBinaryMessageWindowView.swift (1)

372-383: Ensure reply callback and @State mutation run on the main thread.

callReplyCallback mutates @State repliedToCallback (line 253, 262). SNTAuthorizationHelper.authorizeExecution internally uses LAContext.evaluatePolicy, which calls its completion handler off the main thread. This causes callReplyCallback to mutate @State from a background thread, triggering "Publishing changes from background threads" warnings. Additionally, DispatchQueue.main.sync (line 382) called from an off-main completion risks deadlock. The nil-event guard (lines 371-375) also bypasses main dispatch, creating inconsistent behavior.

Use DispatchQueue.main.async to dispatch once to main and call callReplyCallback and window close there. Apply this pattern consistently to both the nil-event guard and the authorization completion paths.

Suggested fix

func standAloneButton() {
  guard let e = self.event else {
-    if let cb = self.replyCallback {
-      cb(false)
-    }
+    DispatchQueue.main.async {
+      callReplyCallback(false)
+      window?.close()
+    }
    return
  }

  SNTAuthorizationHelper.authorizeExecution(for: e) { success in
-    callReplyCallback(success)
-    DispatchQueue.main.sync {
-      window?.close()
-    }
+    DispatchQueue.main.async {
+      callReplyCallback(success)
+      window?.close()
+    }
  }
}

🧹 Nitpick comments (1)

Source/gui/SNTBinaryMessageWindowView.swift (1)

253-263: Prevent re‑entrancy by flipping the guard before invoking the callback.

If replyCallback re-enters (directly or indirectly), the flag is still false and the callback can fire twice. Set repliedToCallback = true before invoking the callback.

♻️ Proposed fix

func callReplyCallback(_ response: Bool) {
  guard !repliedToCallback else { return }
-  if let cb = replyCallback {
-    cb(response)
-  }
-  repliedToCallback = true
+  repliedToCallback = true
+  replyCallback?(response)
}

[russellhancox]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@Source/gui/SNTBinaryMessageWindowView.swift`:
- Around line 371-379: The guard path in standAloneButton currently invokes
replyCallback directly (cb(false)) which bypasses the double-call guard; change
this to use the existing callReplyCallback(...) helper so repliedToCallback is
set and duplicate invocations are prevented. In standAloneButton, when event is
nil and replyCallback exists, dispatch to main and call callReplyCallback(false)
(or the equivalent helper) instead of calling cb(false) directly; ensure you
reference replyCallback, callReplyCallback, and repliedToCallback so the helper
marks repliedToCallback = true and nulls out replyCallback consistently.