macos: continue sysext downgrade when stale deactivation is already missing

macos/Runner/VPN/SystemExtensionManager.swift

@@ -5,21 +5,34 @@ import SystemExtensions
 
 /// Decides whether a failed system extension request should trigger a
 /// stale-registry recovery attempt. macOS occasionally surfaces
-/// `extensionNotFound` on an activation request when its registry holds a
-/// reference to a previously-installed extension that no longer matches the
-/// app on disk — e.g. after the .app bundle was replaced under an existing
-/// install. Submitting a deactivation request first clears that state and
-/// gives the subsequent activation a clean slot. The decision is gated to a
-/// single attempt per session so a persistent failure doesn't loop.
+/// `extensionNotFound` when its registry holds a reference to a
+/// previously-installed extension that no longer matches the app on disk —
+/// e.g. after the .app bundle was replaced under an existing install.
+/// Activation failures recover by deactivating first; replacement
+/// deactivation failures recover by continuing to the activation that was
+/// already planned. Activation retry is gated to a single attempt per session
+/// so a persistent failure doesn't loop.
 internal enum StaleRegistryRecovery {
+  private static func isExtensionNotFound(_ error: NSError) -> Bool {
+    error.domain == OSSystemExtensionErrorDomain
+      && error.code == OSSystemExtensionError.extensionNotFound.rawValue
+  }
+
   static func shouldRecover(
     from error: NSError,
     activationFailed: Bool,
     alreadyAttempted: Bool
   ) -> Bool {
     guard !alreadyAttempted, activationFailed else { return false }
-    return error.domain == OSSystemExtensionErrorDomain
-      && error.code == OSSystemExtensionError.extensionNotFound.rawValue
+    return isExtensionNotFound(error)
+  }
+
+  static func shouldContinueAfterMissingDeactivation(
+    from error: NSError,
+    activateAfterDeactivation: Bool
+  ) -> Bool {
+    guard activateAfterDeactivation else { return false }
+    return isExtensionNotFound(error)
   }
 }
 
@@ -45,10 +58,10 @@ class SystemExtensionManager: NSObject, OSSystemExtensionRequestDelegate {
 
   @Published private(set) var status: ExtensionStatus = .notInstalled
 
-  /// Set when we've issued a stale-registry recovery (deactivate-then-activate
-  /// in response to `extensionNotFound`). Reset on the next successful
-  /// completion. Single-shot per session to prevent an infinite retry loop if
-  /// the same error reproduces after the recovery deactivation.
+  /// Set when we've taken a stale-registry recovery path for
+  /// `extensionNotFound`. Reset on the next successful activation. Single-shot
+  /// per session to prevent an infinite retry loop if the same error reproduces
+  /// after recovery.
   private var didAttemptStaleRegistryRecovery = false
 
   override init() {
@@ -182,6 +195,19 @@ class SystemExtensionManager: NSObject, OSSystemExtensionRequestDelegate {
       return
     }
 
+    if StaleRegistryRecovery.shouldContinueAfterMissingDeactivation(
+      from: nsError,
+      activateAfterDeactivation: context?.activatesAfterDeactivation ?? false)
+    {
+      didAttemptStaleRegistryRecovery = true
+      appLogger.info(
+        "Replacement deactivation failed with extensionNotFound — continuing with bundled activation (context=\(context?.logDescription ?? "unknown"))"
+      )
+      submitActivationRequest(
+        reason: "activating bundled extension after missing stale extension during replacement")
+      return
+    }
+
     appLogger.error(
       "System extension request failed: context=\(context?.logDescription ?? "unknown") error=\(nsError.localizedDescription)"
     )
@@ -374,6 +400,11 @@ private enum RequestContext: Equatable {
     return false
   }
 
+  var activatesAfterDeactivation: Bool {
+    if case .deactivateThenActivate(_, true) = self { return true }
+    return false
+  }
+
   var logDescription: String {
     switch self {
     case .inspectStatus:

macos/RunnerTests/RunnerTests.swift

@@ -458,11 +458,9 @@ final class RunnerTests: XCTestCase {
     }
   }
 
-  // Recovery is scoped to activation requests. If a properties-query or
-  // deactivation fails with extensionNotFound, we should not attempt to
-  // recover via a second deactivation — those request types don't suffer
-  // from the same stale-activation-slot bug, and retrying would mask real
-  // errors.
+  // Activation retry is scoped to activation requests. If a properties-query
+  // or deactivation fails with extensionNotFound, shouldRecover must not
+  // submit another deactivation request.
   func testStaleRegistryRecoveryDoesNotFireForNonActivationContexts() {
     let error = NSError(
       domain: OSSystemExtensionErrorDomain,
@@ -477,6 +475,45 @@ final class RunnerTests: XCTestCase {
     )
   }
 
+  func testStaleRegistryRecoveryContinuesAfterMissingReplacementDeactivation() {
+    let error = NSError(
+      domain: OSSystemExtensionErrorDomain,
+      code: OSSystemExtensionError.extensionNotFound.rawValue
+    )
+    XCTAssertTrue(
+      StaleRegistryRecovery.shouldContinueAfterMissingDeactivation(
+        from: error,
+        activateAfterDeactivation: true
+      )
+    )
+  }
+
+  func testStaleRegistryRecoveryDoesNotContinueAfterManualDeactivation() {
+    let error = NSError(
+      domain: OSSystemExtensionErrorDomain,
+      code: OSSystemExtensionError.extensionNotFound.rawValue
+    )
+    XCTAssertFalse(
+      StaleRegistryRecovery.shouldContinueAfterMissingDeactivation(
+        from: error,
+        activateAfterDeactivation: false
+      )
+    )
+  }
+
+  func testStaleRegistryRecoveryDoesNotContinueAfterOtherDeactivationErrors() {
+    let error = NSError(
+      domain: OSSystemExtensionErrorDomain,
+      code: OSSystemExtensionError.validationFailed.rawValue
+    )
+    XCTAssertFalse(
+      StaleRegistryRecovery.shouldContinueAfterMissingDeactivation(
+        from: error,
+        activateAfterDeactivation: true
+      )
+    )
+  }
+
   // Non-OSSystemExtensionError domain errors (e.g. networking, file IO)
   // bubbling up here would be unexpected, but if they do we should not
   // attempt recovery — the deactivate-then-activate path only makes sense