fix(menubar): wire bb-vpn:// via AppDelegate + harden EnrollHandler subprocess plumbing

client/menubar/BBVPN/BBVPNApp.swift

@@ -17,20 +17,25 @@
 // receipt and which drops one `inbox/enroll-*.json` for the root
 // daemon to ingest (drop-box is mode 1733 by the .pkg postinstall).
 
+import AppKit
 import SwiftUI
 
 @main
 struct BBVPNApp: App {
+    // Launch Services delivers `bb-vpn://enroll?…` via NSApplication's
+    // `application(_:open:)` delegate hook — the canonical AppKit URL
+    // entry point. We bind it via NSApplicationDelegateAdaptor because
+    // SwiftUI's `.onOpenURL` on `MenuBarExtra` does not reliably fire
+    // for LSUIElement (background) apps on macOS 13/14: end-to-end
+    // testing on macold showed Launch Services brought BBVPN.app to
+    // the foreground (visible in syslog as SetFrontProcess) but the
+    // SwiftUI scene-modifier never received the URL. The previous
+    // NSAppleEventManager-in-init approach had the same problem.
+    // application(_:open:) works because AppKit dispatches it
+    // directly off the run loop regardless of scene/window state.
+    @NSApplicationDelegateAdaptor(AppDelegate.self) private var appDelegate
     @StateObject private var status = StatusModel()
 
-    init() {
-        // Wire Launch Services → EnrollHandler. Done once at process
-        // start so the very first bb-vpn:// click after install lands
-        // even though the user hasn't opened the app yet (MenuBarExtra
-        // apps launch implicitly on first URL receipt).
-        URLEventHandler.shared.register()
-    }
-
     var body: some Scene {
         MenuBarExtra {
             menuContent
@@ -93,3 +98,24 @@ struct BBVPNApp: App {
             .keyboardShortcut("q")
     }
 }
+
+// AppDelegate exists only to provide application(_:open:) — the
+// AppKit hook Launch Services uses for `bb-vpn://` clicks. URLs
+// arrive here as an array because macOS can deliver multiple at
+// once (e.g., the user clicks several enroll links in quick
+// succession before the app is fully booted); each is dispatched
+// to EnrollHandler independently.
+//
+// `application(_:open:)` is also AppKit's catch-all for file:// URLs
+// from `open -a BBVPN <files>` and Finder drag-and-drop — scopes the
+// previous NSAppleEventManager + kInternetEventClass handler did
+// NOT receive. Filter to bb-vpn:// only so a stray file drop doesn't
+// fire N "Not a bb-vpn enroll link" modals serially on the main
+// thread.
+final class AppDelegate: NSObject, NSApplicationDelegate {
+    func application(_ application: NSApplication, open urls: [URL]) {
+        for url in urls where url.scheme?.lowercased() == "bb-vpn" {
+            EnrollHandler.handleEnrollURL(url)
+        }
+    }
+}

client/menubar/BBVPN/EnrollHandler.swift

@@ -47,29 +47,112 @@ enum EnrollHandler {
         let proc = Process()
         proc.executableURL = URL(fileURLWithPath: bbvpnBin)
         proc.arguments = args
+        // Discard stdout to /dev/null. An unread Pipe buffers the
+        // child's writes and blocks on write() once the kernel buffer
+        // fills (~16-64KB on macOS), which would hang the child while
+        // the parent blocks on `exited.wait()` and force the watchdog
+        // to terminate with a misleading "timeout" error.
+        proc.standardOutput = FileHandle.nullDevice
+
+        // Stderr feeds the user-visible NSAlert on failure, so we
+        // can't discard it — but we also can't leave the Pipe unread
+        // (same deadlock as stdout) or read synchronously *after*
+        // `exited.wait()` returns (readToEnd would block forever if
+        // a future grandchild inherits the writer fd, escaping the
+        // watchdog). Drain concurrently via readabilityHandler:
+        // bytes accumulate into stderrData while the child runs, and
+        // we snapshot it after the child exits. On EOF (empty chunk =
+        // child closed its writer), the handler signals stderrEOF; the
+        // main thread clears readabilityHandler after the bounded wait.
         let errPipe = Pipe()
         proc.standardError = errPipe
-        proc.standardOutput = Pipe()  // discard stdout
+        var stderrData = Data()
+        let stderrLock = NSLock()
+        // EOF is signaled by the readabilityHandler on its empty-chunk
+        // delivery (writer-side close = child exit). NOTE_EXIT (which
+        // signals `exited` below) and EVFILT_READ (which fires the
+        // readabilityHandler) are independent kevents on separate GCD
+        // queues — the termination semaphore CAN fire before the
+        // handler has drained the bytes still buffered in the pipe.
+        // Without this extra synchronization, runBBVPN can return an
+        // empty stderr and the user sees "bb-vpn enroll failed (exit
+        // N)" instead of the actual error string.
+        let stderrEOF = DispatchSemaphore(value: 0)
+        // `hasEOF` is read+written under stderrLock so the handler can
+        // safely fire multiple empty-chunk deliveries (which is what
+        // happens until the main thread clears the handler) without
+        // over-signaling the semaphore. Doing the clear ONLY from the
+        // main thread also sidesteps the question of whether
+        // FileHandle's readabilityHandler setter is reentrant against
+        // the IO queue that runs the closure.
+        var hasEOF = false
+        errPipe.fileHandleForReading.readabilityHandler = { handle in
+            let chunk = handle.availableData
+            stderrLock.lock()
+            if chunk.isEmpty {
+                if !hasEOF {
+                    hasEOF = true
+                    stderrLock.unlock()
+                    stderrEOF.signal()
+                    return
+                }
+                stderrLock.unlock()
+                return
+            }
+            stderrData.append(chunk)
+            stderrLock.unlock()
+        }
+
         // Watchdog: handleEnrollURL runs on the main thread (Launch Services
-        // delivers the bb-vpn:// click via NSAppleEventManager → AppleEvent
-        // handler → here). A hung `bb-vpn enroll` (slow disk, future enroll
-        // flow with a network call, etc.) would freeze the menubar UI
-        // indefinitely. Cap at 10s — enroll is local-only (validate UUID,
-        // write inbox/<uuid>.json), so 10s is well over the realistic
-        // worst case and short enough to keep the UI responsive.
+        // delivers the bb-vpn:// click via AppDelegate.application(_:open:)
+        // in BBVPNApp.swift → here). A hung `bb-vpn enroll` (slow disk,
+        // future enroll flow with a network call, etc.) would freeze the
+        // menubar UI indefinitely. Cap at 10s — enroll is local-only
+        // (validate UUID, write inbox/<uuid>.json), so 10s is well over
+        // the realistic worst case and short enough to keep the UI
+        // responsive.
         let exited = DispatchSemaphore(value: 0)
         proc.terminationHandler = { _ in exited.signal() }
         do {
             try proc.run()
         } catch {
+            errPipe.fileHandleForReading.readabilityHandler = nil
             return CLIResult(status: -1, stderr: "spawn \(bbvpnBin): \(error.localizedDescription)")
         }
         if exited.wait(timeout: .now() + 10) == .timedOut {
             proc.terminate()
-            return CLIResult(status: -1, stderr: "bb-vpn enroll timed out after 10s")
+            // proc.terminate() sends SIGTERM. When the child exits in
+            // response, the kernel closes its stderr writer fd, which
+            // unblocks the readabilityHandler with a final empty
+            // chunk. Bounded so a SIGTERM-ignoring child or a
+            // leaked-fd grandchild cannot pin us.
+            _ = stderrEOF.wait(timeout: .now() + 0.1)
+            errPipe.fileHandleForReading.readabilityHandler = nil
+            stderrLock.lock()
+            let captured = String(decoding: stderrData, as: UTF8.self)
+            stderrLock.unlock()
+            // Preserve whatever stderr the child managed to emit
+            // before hanging — that's the diagnostic the watchdog
+            // exists to make visible.
+            let msg = captured.isEmpty
+                ? "bb-vpn enroll timed out after 10s"
+                : "bb-vpn enroll timed out after 10s. Last stderr: \(captured)"
+            return CLIResult(status: -1, stderr: msg)
         }
-        let data = (try? errPipe.fileHandleForReading.readToEnd()) ?? Data()
-        let stderrText = String(data: data, encoding: .utf8) ?? ""
+        // Normal exit: wait up to 100ms for the readabilityHandler to
+        // drain final bytes. Bounded short — if EOF doesn't arrive in
+        // this window, return what's been captured (a future grandchild
+        // inheriting the fd would otherwise pin us indefinitely).
+        _ = stderrEOF.wait(timeout: .now() + 0.1)
+        errPipe.fileHandleForReading.readabilityHandler = nil
+
+        stderrLock.lock()
+        // Lossy UTF-8 decode (invalid sequences → U+FFFD). The strict
+        // form `String(data:encoding:.utf8) ?? ""` drops the entire
+        // string if the drain happens to split a multi-byte sequence
+        // mid-character; lossy decode preserves the diagnostic.
+        let stderrText = String(decoding: stderrData, as: UTF8.self)
+        stderrLock.unlock()
         return CLIResult(status: proc.terminationStatus, stderr: stderrText)
     }
 
@@ -86,27 +169,9 @@ enum EnrollHandler {
     }
 }
 
-// MARK: - URL hook
-//
-// MenuBarExtra-based apps don't get a normal AppDelegate. We wire URL
-// handling via NSAppleEventManager so Launch Services delivers the
-// bb-vpn:// click directly to the running app.
-
-final class URLEventHandler: NSObject {
-    static let shared = URLEventHandler()
-
-    func register() {
-        NSAppleEventManager.shared().setEventHandler(
-            self,
-            andSelector: #selector(handle(_:withReplyEvent:)),
-            forEventClass: AEEventClass(kInternetEventClass),
-            andEventID: AEEventID(kAEGetURL)
-        )
-    }
-
-    @objc func handle(_ event: NSAppleEventDescriptor, withReplyEvent: NSAppleEventDescriptor) {
-        guard let str = event.paramDescriptor(forKeyword: AEKeyword(keyDirectObject))?.stringValue,
-              let url = URL(string: str) else { return }
-        EnrollHandler.handleEnrollURL(url)
-    }
-}
+// URL dispatch (Launch Services → bb-vpn://) is wired via
+// NSApplicationDelegateAdaptor + AppDelegate.application(_:open:)
+// in BBVPNApp.swift. The previous NSAppleEventManager registration
+// in BBVPNApp.init() was removed — it failed to fire for LSUIElement
+// (background) menubar apps on macOS 13/14 even though Launch
+// Services successfully foregrounded BBVPN.app.

client/menubar/Info.plist

@@ -28,7 +28,8 @@
     <true/>
     <!-- Registers bb-vpn:// with Launch Services. Clicking a
          bb-vpn://enroll?uuid=… link in Slack/Mail/Messages routes the
-         URL via NSAppleEventManager → EnrollHandler.handleEnrollURL. -->
+         URL to BBVPN.app via AppDelegate.application(_:open:) (see
+         BBVPNApp.swift) → EnrollHandler.handleEnrollURL. -->
     <key>CFBundleURLTypes</key>
     <array>
         <dict>