Nc.swift.review parity

.github/workflows/app_check_core.yml

@@ -22,7 +22,7 @@ jobs:
         os: [macos-15, macos-26]
         include:
           - os: macos-15
-            xcode: Xcode_16.4
+            xcode: Xcode_26.2
           - os: macos-26
             xcode: Xcode_26.2
     runs-on: ${{ matrix.os }}

.github/workflows/spm.yml

@@ -18,12 +18,8 @@ jobs:
     strategy:
       matrix:
         os: [macos-15]
-        xcode: [Xcode_16.4]
+        xcode: [Xcode_26.2]
         platform: [iOS, tvOS, macOS, catalyst]
-        include:
-          - os: macos-14
-            xcode: Xcode_16.2
-            platform: iOS
     runs-on: ${{ matrix.os }}
     steps:
     - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1
@@ -36,3 +32,17 @@ jobs:
       run: xcodebuild -list
     - name: iOS Unit Tests
       run: scripts/third_party/travis/retry.sh scripts/build.sh AppCheck-Package ${{ matrix.platform }} spm
+
+  # SwiftPM emits Swift diagnostics but still exits 0, so warnings scroll past
+  # unnoticed in the job above. `pod lib lint` escalates the same warnings to
+  # fatal, which previously made CocoaPods the only gate able to reject them --
+  # a ~5 minute round-trip, and an accidental dependency rather than a designed
+  # one. This job makes the cheap gate as strict as the expensive one.
+  warnings-as-errors:
+    runs-on: macos-15
+    steps:
+    - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1
+    - name: Xcode
+      run: sudo xcode-select -s /Applications/Xcode_26.2.app/Contents/Developer
+    - name: Build sources and tests with warnings as errors
+      run: swift build --build-tests -Xswiftc -warnings-as-errors

AppCheckCore/Sources/AppAttestProvider/AppCheckCoreAppAttestProvider.swift

@@ -140,25 +140,38 @@ public class AppCheckCoreAppAttestProvider: NSObject, AppCheckCoreProvider {
       }
 
       ongoingGetTokenOperationLimitedUse = limitedUse
-      let newTask = Task {
-        try await createGetTokenSequenceWithBackoff(limitedUse: limitedUse)
+      let newTask = Task { () throws -> AppCheckCoreToken in
+        // Release the ongoing operation from *within* the operation, so the
+        // slot is already cleared by the time any chained waiter resumes. The
+        // Objective-C implementation did this by chaining `.thenOn`/
+        // `.recoverOn` onto the operation itself. Clearing it in the
+        // originating caller instead lets a waiter observe the completed task
+        // still parked in the slot and spin through repeated `.retry`
+        // recursions until the originator happens to run.
+        defer {
+          self.lock.execute {
+            self.ongoingGetTokenOperationTask = nil
+          }
+        }
+        return try await self.createGetTokenSequenceWithBackoff(limitedUse: limitedUse)
       }
       ongoingGetTokenOperationTask = newTask
       return .run(newTask)
     }
 
     switch action {
     case let .retry(ongoingTask):
-      _ = try? await ongoingTask.value
+      // Wait for the in-flight operation, then start a fresh sequence.
+      //
+      // This must NOT swallow the in-flight error. Objective-C chained with
+      // `.thenOn`, which only runs on success, so when the ongoing operation
+      // failed the chaining caller was rejected with that same error instead
+      // of kicking off another full attestation sequence.
+      _ = try await ongoingTask.value
       return try await getToken(limitedUse: limitedUse)
     case let .wait(ongoingTask):
       return try await ongoingTask.value
     case let .run(newTask):
-      defer {
-        lock.execute {
-          ongoingGetTokenOperationTask = nil
-        }
-      }
       return try await newTask.value
     }
   }
@@ -430,13 +443,26 @@ public class AppCheckCoreAppAttestProvider: NSObject, AppCheckCoreProvider {
       return AppCheckCoreAppAttestProviderState(unsupportedWithError: error)
     }
 
-    let appAttestKeyID = try await keyIDStorage.getAppAttestKeyID()
-    guard let keyID = appAttestKeyID else {
+    // 2. Check for stored key ID of the generated App Attest key pair.
+    //
+    // A missing key ID is reported by the storage as a thrown
+    // `appAttestKeyIDNotFound` error rather than as `nil`. Treat any failure to
+    // read the key ID as "no key yet" and fall back to the initial state so a
+    // new key pair is generated, matching the behavior of the Objective-C
+    // implementation (`FBLPromiseAwait` returned `nil` and the error was
+    // deliberately ignored).
+    let appAttestKeyID = try? await keyIDStorage.getAppAttestKeyID()
+    guard let keyID = appAttestKeyID ?? nil else {
       return AppCheckCoreAppAttestProviderState(supportedInitialState: ())
     }
 
-    let attestationArtifact = try await artifactStorage.getArtifact(forKey: keyID)
-    guard let artifact = attestationArtifact else {
+    // 3. Check for a stored attestation artifact received from the backend.
+    //
+    // As above, a failure to read the artifact (e.g. a transient Keychain
+    // error) degrades to re-attesting the existing key rather than failing the
+    // whole token fetch.
+    let attestationArtifact = try? await artifactStorage.getArtifact(forKey: keyID)
+    guard let artifact = attestationArtifact ?? nil else {
       return AppCheckCoreAppAttestProviderState(generatedKeyID: keyID)
     }
 

AppCheckCore/Sources/Core/AppCheckCore.swift

@@ -83,17 +83,28 @@ public class AppCheckCore: NSObject, AppCheckCoreProtocol {
 
   private func periodicTokenRefresh(completion: @escaping AppCheckCoreTokenRefreshCompletion) {
     Task {
+      let refreshResult: AppCheckCoreTokenRefreshResult
       do {
         let token = try await self.token(forcingRefresh: false)
-        let refreshResult = AppCheckCoreTokenRefreshResult(status: .success,
-                                                           expirationDate: token.expirationDate,
-                                                           receivedAtDate: token.receivedAtDate)
-        completion(refreshResult)
+        refreshResult = AppCheckCoreTokenRefreshResult(status: .success,
+                                                       expirationDate: token.expirationDate,
+                                                       receivedAtDate: token.receivedAtDate)
       } catch {
-        let refreshResult = AppCheckCoreTokenRefreshResult(status: .failure,
-                                                           expirationDate: nil,
-                                                           receivedAtDate: nil)
-        completion(refreshResult)
+        refreshResult = AppCheckCoreTokenRefreshResult(status: .failure,
+                                                       expirationDate: nil,
+                                                       receivedAtDate: nil)
+      }
+      // Parity with v11: `-[GACAppCheck periodicTokenRefreshWithCompletion:]`
+      // used bare `.then` / `.catch`, so this ran on the main queue.
+      //
+      // `nonisolated(unsafe)` is an explicit acknowledgement that the result
+      // crosses a queue boundary without compiler-checked isolation. That is
+      // the same boundary v11 crossed via `dispatch_async`, with the same
+      // absence of synchronization, and the value is an immutable holder.
+      nonisolated(unsafe) let result = refreshResult
+      nonisolated(unsafe) let completion = completion
+      DispatchQueue.main.async {
+        completion(result)
       }
     }
   }
@@ -177,11 +188,13 @@ public class AppCheckCore: NSObject, AppCheckCoreProtocol {
     let refreshResult = AppCheckCoreTokenRefreshResult(status: .success,
                                                        expirationDate: token.expirationDate,
                                                        receivedAtDate: token.receivedAtDate)
-    tokenRefresher.updateWithRefreshResult(refreshResult)
 
-    if let tokenDelegate = tokenDelegate {
-      tokenDelegate.tokenDidUpdate(token, serviceName: serviceName)
-    }
+    // Parity with v11: both of these ran inside a bare `FBLPromise.then`, which
+    // dispatches onto `FBLPromise.defaultDispatchQueue` (the main queue). They
+    // are awaited rather than fire-and-forget because the v11 promise only
+    // resolved *after* this block completed, so callers were guaranteed the
+    // delegate had already been notified by the time they received the token.
+    await notifyTokenUpdateOnMainQueue(token, refreshResult: refreshResult)
 
     return token
   }
@@ -191,9 +204,9 @@ public class AppCheckCore: NSObject, AppCheckCoreProtocol {
     Task {
       do {
         let token = try await self.token(forcingRefresh: forcingRefresh)
-        completion(AppCheckCoreTokenResult(token: token))
+        Self.deliverOnMainQueue(AppCheckCoreTokenResult(token: token), to: completion)
       } catch {
-        completion(AppCheckCoreTokenResult(error: error))
+        Self.deliverOnMainQueue(AppCheckCoreTokenResult(error: error), to: completion)
       }
     }
   }
@@ -207,9 +220,72 @@ public class AppCheckCore: NSObject, AppCheckCoreProtocol {
     Task {
       do {
         let token = try await self.limitedUseToken()
-        completion(AppCheckCoreTokenResult(token: token))
+        Self.deliverOnMainQueue(AppCheckCoreTokenResult(token: token), to: completion)
       } catch {
-        completion(AppCheckCoreTokenResult(error: error))
+        Self.deliverOnMainQueue(AppCheckCoreTokenResult(error: error), to: completion)
+      }
+    }
+  }
+
+  // MARK: - Main-queue delivery (v11 parity)
+
+  /// Delivers a completion handler on the main queue.
+  ///
+  /// In v11 every public completion handler was invoked from a bare
+  /// `FBLPromise` `.then` / `.catch`, which dispatches onto
+  /// `FBLPromise.defaultDispatchQueue`. That default is `dispatch_get_main_queue()`
+  /// (set in `+[FBLPromise initialize]`) and is never reassigned by this library
+  /// or its known consumers, so handlers were always delivered on the main
+  /// queue — and always asynchronously, since `FBLPromise` used an
+  /// unconditional `dispatch_group_async` with no same-queue fast path.
+  ///
+  /// `DispatchQueue.main.async` is used rather than `MainActor.run` to reproduce
+  /// that "always async" behavior exactly, including when the caller is already
+  /// on the main thread.
+  ///
+  /// Note this applies only to the completion-handler API. The `async` variants
+  /// resume on the cooperative pool as normal; `async` callers are expected to
+  /// hop to the main actor themselves, and forcing a hop would be a new
+  /// divergence rather than parity.
+  private static func deliverOnMainQueue(_ result: AppCheckCoreTokenResult,
+                                         to completion: @escaping AppCheckCoreTokenHandler) {
+    // See the note in `notifyTokenUpdateOnMainQueue` — the caller's handler and
+    // the result object cross the same queue boundary v11 crossed, and the
+    // handler is arbitrary caller-supplied code that cannot be declared
+    // `Sendable` on their behalf.
+    nonisolated(unsafe) let result = result
+    nonisolated(unsafe) let completion = completion
+    DispatchQueue.main.async {
+      completion(result)
+    }
+  }
+
+  /// Notifies the token refresher and token delegate on the main queue.
+  ///
+  /// Mirrors the v11 bare `.then` in `-[GACAppCheck refreshToken]`, which ran
+  /// both of these on the main queue before resolving the promise. Suspends
+  /// until they have run so that ordering relative to the returned token is
+  /// preserved.
+  private func notifyTokenUpdateOnMainQueue(
+    _ token: AppCheckCoreToken,
+    refreshResult: AppCheckCoreTokenRefreshResult
+  ) async {
+    // `nonisolated(unsafe)` is an explicit acknowledgement that these cross a
+    // queue boundary without compiler-checked isolation. The refresher and
+    // delegate are arbitrary caller-supplied objects, so neither can honestly
+    // be declared `Sendable` here. This is the same hand-off v11 performed via
+    // `dispatch_async` with no synchronization at all, so it is parity rather
+    // than a new hazard.
+    nonisolated(unsafe) let tokenRefresher = self.tokenRefresher
+    nonisolated(unsafe) let tokenDelegate = self.tokenDelegate
+    nonisolated(unsafe) let result = refreshResult
+    let serviceName = self.serviceName
+
+    await withCheckedContinuation { continuation in
+      DispatchQueue.main.async {
+        tokenRefresher.updateWithRefreshResult(result)
+        tokenDelegate?.tokenDidUpdate(token, serviceName: serviceName)
+        continuation.resume()
       }
     }
   }

AppCheckCore/Sources/Core/AppCheckCoreLogger.swift

@@ -25,16 +25,36 @@ public enum AppCheckCoreLogLevel: Int {
 
 @objc(GACAppCheckLogger)
 public class AppCheckCoreLogger: NSObject {
+  private static let logLevelLock = NSLock()
   private static var _logLevel: AppCheckCoreLogLevel = .warning
 
+  /// The current log level.
+  ///
+  /// Access is serialized by a lock to match the `atomic` semantics of the
+  /// Objective-C `GACAppCheckLogger.logLevel` class property, which was backed
+  /// by a `volatile` static.
   @objc public static var logLevel: AppCheckCoreLogLevel {
-    get { return _logLevel }
-    set { _logLevel = newValue }
+    get {
+      logLevelLock.lock()
+      defer { logLevelLock.unlock() }
+      return _logLevel
+    }
+    set {
+      logLevelLock.lock()
+      defer { logLevelLock.unlock() }
+      _logLevel = newValue
+    }
   }
 
   public static func log(code: AppCheckCoreMessageCode, logLevel: AppCheckCoreLogLevel,
                          message: String) {
-    #if !NDEBUG
+    // Don't log anything in non-debug builds.
+    //
+    // Note: this must be `DEBUG`, not `!NDEBUG`. `NDEBUG` is a C preprocessor
+    // macro and is never defined as a Swift compilation condition, so
+    // `#if !NDEBUG` is unconditionally true in Swift and would leak logging
+    // (including the App Check debug token) into Release builds.
+    #if DEBUG
       if logLevel.rawValue >= self.logLevel.rawValue {
         let levelString: String
         switch logLevel {
@@ -46,7 +66,10 @@ public class AppCheckCoreLogger: NSObject {
         @unknown default: levelString = "Unknown"
         }
         let codeString = String(format: "I-GAC%06ld", code.rawValue)
-        print("<\(levelString)> [AppCheckCore][\(codeString)] \(message)")
+        // `NSLog` (rather than `print`) so output reaches the system log and is
+        // visible in Console.app without a debugger attached, matching the
+        // Objective-C implementation.
+        NSLog("<%@> [AppCheckCore][%@] %@", levelString, codeString, message)
       }
     #endif
   }

AppCheckCore/Sources/Core/Errors/AppCheckCoreErrors.swift

@@ -17,6 +17,27 @@ import Foundation
 /// Firebase app check error domain.
 public let AppCheckCoreErrorDomain = "com.google.app_check_core"
 
+/// Objective-C accessor for the App Check error domain.
+///
+/// Swift global constants are not bridged to Objective-C, so the v11
+/// `GACAppCheckErrorDomain` global is no longer visible there. Objective-C
+/// callers should use `GACAppCheckErrors.errorDomain` instead:
+///
+/// ```objc
+/// if ([error.domain isEqualToString:GACAppCheckErrors.errorDomain]) { ... }
+/// ```
+@objc(GACAppCheckErrors)
+public final class AppCheckCoreErrorsObjC: NSObject {
+  /// The App Check error domain. Equivalent to the Swift
+  /// `AppCheckCoreErrorDomain` global.
+  @objc public static var errorDomain: String { return AppCheckCoreErrorDomain }
+
+  @available(*, unavailable)
+  override private init() {
+    super.init()
+  }
+}
+
 @objc(GACAppCheckErrorCode)
 public enum AppCheckCoreErrorCode: Int, Error {
   /// An unknown or non-actionable error.

AppCheckCore/Sources/Core/Storage/AppCheckCoreStorage.swift

@@ -76,18 +76,10 @@ public final class AppCheckCoreStorage: NSObject, AppCheckCoreStorageProtocol {
         let storedToken = AppCheckCoreStoredToken()
         storedToken.update(with: token)
         keychainStorage
-          .setObject(storedToken, forKey: tokenKey, accessGroup: accessGroup) { result, error in
+          .setObject(storedToken, forKey: tokenKey, accessGroup: accessGroup) { _, error in
             if let error = error {
-              let nsError = error as NSError
-              if nsError.domain == "com.gul.keychain.ErrorDomain",
-                 let failureReason = nsError.userInfo[NSLocalizedFailureReasonErrorKey] as? String,
-                 failureReason.contains("-25299") {
-                // Ignore errSecDuplicateItem (-25299) caused by concurrent tests
-                continuation.resume(returning: token)
-              } else {
-                let wrappedError = AppCheckCoreErrorUtil.keychainError(with: error)
-                continuation.resume(throwing: wrappedError)
-              }
+              let wrappedError = AppCheckCoreErrorUtil.keychainError(with: error)
+              continuation.resume(throwing: wrappedError)
             } else {
               continuation.resume(returning: token)
             }