fix(plugins): wait for the network before the plugin update pass

TablePro/Core/Plugins/PluginManager+AutoUpdate.swift

@@ -41,8 +41,6 @@ extension PluginManager {
     func scheduleRegistryUpdatePass(after delay: Duration = .zero) {
         guard SoftwareUpdater.isUpdatingEnabled else { return }
         registryUpdateTask?.cancel()
-        // A connection the initial load releases runs before this task starts, and must wait for its repairs.
-        isRepairingRejectedPlugins = delay == .zero
         registryUpdateTask = Task { [weak self] in
             if delay > .zero {
                 try? await Task.sleep(for: delay)
@@ -74,9 +72,15 @@ extension PluginManager {
         registryClient: RegistryClient = .shared,
         appBuild: Int = Bundle.main.buildNumber
     ) async -> RegistryUpdatePassResult {
+        let targets = registryUpdateTargets(appBuild: appBuild)
+        guard !targets.isEmpty else { return .finished }
+        guard let manifest = await registryClient.fetchWaitingForConnectivity() else {
+            Self.logger.warning("Plugin update pass deferred: the plugin manifest is unavailable")
+            return .retry
+        }
         isRepairingRejectedPlugins = true
         defer { isRepairingRejectedPlugins = false }
-        return await applyRegistryUpdates(to: registryUpdateTargets(appBuild: appBuild), registryClient: registryClient)
+        return await applyRegistryUpdates(to: targets, manifest: manifest)
     }
 
     func waitForRejectedPluginRepairs() async {
@@ -86,16 +90,8 @@ extension PluginManager {
 
     private func applyRegistryUpdates(
         to targets: [RegistryUpdateTarget],
-        registryClient: RegistryClient
+        manifest: RegistryManifest
     ) async -> RegistryUpdatePassResult {
-        guard !targets.isEmpty else { return .finished }
-
-        await registryClient.fetch(policy: .mustBeCurrent)
-        guard registryClient.isManifestCurrent, let manifest = registryClient.manifest else {
-            Self.logger.warning("Plugin update pass deferred: the plugin manifest is unavailable")
-            return .retry
-        }
-
         let pending = Self.pendingRegistryUpdates(
             installed: targets,
             manifest: manifest,
@@ -320,7 +316,7 @@ extension PluginManager {
         }
         if driverPlugin(for: type) == nil, hasOutdatedRejectedPlugin(forTypeId: typeId) {
             Self.logger.info("Plugin '\(typeId)' is installed but outdated, updating it before connect")
-            await ensurePluginReady(forTypeId: typeId)
+            await ensurePluginReady(forTypeId: typeId, registryClient: registryClient)
         }
         guard driverPlugin(for: type) == nil else { return }
         if PluginMetadataRegistry.shared.snapshot(for: type) == nil {
@@ -345,12 +341,19 @@ extension PluginManager {
         }
     }
 
-    func ensurePluginReady(forTypeId typeId: String) async {
-        guard SoftwareUpdater.isUpdatingEnabled, hasOutdatedRejectedPlugin(forTypeId: typeId) else { return }
+    func ensurePluginReady(forTypeId typeId: String, registryClient: RegistryClient = .shared) async {
+        guard SoftwareUpdater.isUpdatingEnabled else { return }
         await waitForRejectedPluginRepairs()
+        guard !registryUpdateTargets(forTypeId: typeId, appBuild: Bundle.main.buildNumber).isEmpty else { return }
+        let fetched = await registryClient.fetch(policy: .mustBeCurrent)
+        // The update pass may have got its manifest meanwhile and be installing this plugin.
+        await waitForRejectedPluginRepairs()
+        guard let manifest = fetched else {
+            Self.logger.warning("Plugin '\(typeId)' not updated before connect: the plugin manifest is unavailable")
+            return
+        }
         let targets = registryUpdateTargets(forTypeId: typeId, appBuild: Bundle.main.buildNumber)
-        guard !targets.isEmpty else { return }
-        _ = await applyRegistryUpdates(to: targets, registryClient: .shared)
+        _ = await applyRegistryUpdates(to: targets, manifest: manifest)
         AppEvents.shared.pluginsRejected.send(rejectedPlugins)
     }
 

TablePro/Core/Plugins/PluginManager.swift

@@ -112,7 +112,7 @@ final class PluginManager: ObservableObject {
     internal var registryUpdateAttempts = 0
     internal var installsInFlight: Set<String> = []
 
-    // Set from a pass's start until its rejected plugins are done, so a connection never waits on a staging download.
+    // Set while a pass installs rejected plugins, so a connection waits for those but not for the network or a staging download.
     internal var isRepairingRejectedPlugins = false {
         didSet {
             guard !isRepairingRejectedPlugins else { return }

TablePro/Core/Plugins/Registry/RegistryClient.swift

@@ -19,13 +19,14 @@ internal final class RegistryClient: ObservableObject {
     internal private(set) var isManifestCurrent = false
 
     internal let session: URLSession
+    internal let updatePassSession: URLSession
     internal let manifestURL: URL
 
     private var inFlight: InFlightFetch?
 
     private struct InFlightFetch {
         let policy: RegistryFetchPolicy
-        let task: Task<Void, Never>
+        let task: Task<RegistryManifest?, Never>
     }
 
     internal init(
@@ -34,6 +35,7 @@ internal final class RegistryClient: ObservableObject {
     ) {
         self.manifestURL = manifestURL
         self.session = session
+        updatePassSession = Self.makeUpdatePassSession(from: session)
         manifest = Self.cachedManifest(for: manifestURL, in: session.configuration.urlCache)
     }
 
@@ -52,6 +54,14 @@ internal final class RegistryClient: ObservableObject {
         return URLSession(configuration: config)
     }
 
+    // Shares the interactive session's cache; offline it waits for the network until the resource timeout.
+    nonisolated internal static func makeUpdatePassSession(from session: URLSession) -> URLSession {
+        let config = session.configuration
+        config.waitsForConnectivity = true
+        config.timeoutIntervalForResource = 7 * 24 * 60 * 60
+        return URLSession(configuration: config)
+    }
+
     nonisolated private static func cachedManifest(for url: URL, in cache: URLCache?) -> RegistryManifest? {
         guard let cached = cache?.cachedResponse(for: URLRequest(url: url)) else { return nil }
         do {
@@ -62,14 +72,24 @@ internal final class RegistryClient: ObservableObject {
         }
     }
 
-    internal func fetch(policy: RegistryFetchPolicy) async {
+    // The result is this fetch's own; isManifestCurrent also reflects the update pass's fetch.
+    @discardableResult
+    internal func fetch(policy: RegistryFetchPolicy) async -> RegistryManifest? {
         while let current = inFlight {
-            await current.task.value
-            if policy == .ifStale || current.policy == .mustBeCurrent { return }
+            let result = await current.task.value
+            if policy == .ifStale || current.policy == .mustBeCurrent { return result }
+        }
+        let task = Task {
+            defer { self.inFlight = nil }
+            return await self.load(self.makeRequest(policy: policy), from: self.session)
         }
-        let task = Task { await self.load(policy) }
         inFlight = InFlightFetch(policy: policy, task: task)
-        await task.value
+        return await task.value
+    }
+
+    // Never coalesced with fetch(policy:), so an interactive caller never waits for the network with the pass.
+    internal func fetchWaitingForConnectivity() async -> RegistryManifest? {
+        await load(makeRequest(policy: .mustBeCurrent), from: updatePassSession)
     }
 
     internal func makeRequest(policy: RegistryFetchPolicy) -> URLRequest {
@@ -83,23 +103,24 @@ internal final class RegistryClient: ObservableObject {
         return request
     }
 
-    private func load(_ policy: RegistryFetchPolicy) async {
-        defer { inFlight = nil }
+    private func load(_ request: URLRequest, from session: URLSession) async -> RegistryManifest? {
         do {
-            let (data, response) = try await session.data(for: makeRequest(policy: policy))
+            let (data, response) = try await session.data(for: request)
             let status = (response as? HTTPURLResponse)?.statusCode ?? 0
             guard (200...299).contains(status) else {
                 Self.logger.error("Plugin manifest fetch returned HTTP \(status)")
                 isManifestCurrent = false
-                return
+                return nil
             }
             let decoded = try JSONDecoder().decode(RegistryManifest.self, from: data)
             manifest = decoded
             isManifestCurrent = true
             Self.logger.info("Fetched the plugin manifest with \(decoded.plugins.count) plugin(s)")
+            return decoded
         } catch {
             isManifestCurrent = false
             Self.logger.error("Plugin manifest fetch failed: \(error.publicLogShape, privacy: .public)")
+            return nil
         }
     }
 }

TableProTests/Core/Plugins/PluginRegistryUpdatePassTests.swift

@@ -8,11 +8,17 @@ private final class ManifestStubProtocol: URLProtocol, @unchecked Sendable {
     private static let lock = NSLock()
     nonisolated(unsafe) private static var bodies: [URL: Data] = [:]
     nonisolated(unsafe) private static var requests: [URL: Int] = [:]
+    nonisolated(unsafe) private static var unansweredRequests: [URL: Int] = [:]
 
     static func serve(_ body: Data, at url: URL) {
         lock.withLock { bodies[url] = body }
     }
 
+    // An unanswered request stays open until its task is cancelled, like one waiting for the network.
+    static func leaveUnanswered(next count: Int, at url: URL) {
+        lock.withLock { unansweredRequests[url] = count }
+    }
+
     static func requestCount(for url: URL) -> Int {
         lock.withLock { requests[url, default: 0] }
     }
@@ -31,10 +37,15 @@ private final class ManifestStubProtocol: URLProtocol, @unchecked Sendable {
             client?.urlProtocol(self, didFailWithError: URLError(.badURL))
             return
         }
-        let body = Self.lock.withLock {
+        let body: Data? = Self.lock.withLock {
             Self.requests[url, default: 0] += 1
+            guard Self.unansweredRequests[url, default: 0] == 0 else {
+                Self.unansweredRequests[url, default: 0] -= 1
+                return nil
+            }
             return Self.bodies[url] ?? Data()
         }
+        guard let body else { return }
         client?.urlProtocol(self, didReceive: response, cacheStoragePolicy: .notAllowed)
         client?.urlProtocol(self, didLoad: body)
         client?.urlProtocolDidFinishLoading(self)
@@ -438,6 +449,42 @@ struct PluginRegistryUpdatePassTests {
         #expect(manager.stagedPluginIds.isEmpty)
     }
 
+    @Test("The pass fetches on a session that waits for the network; interactive fetches keep the 30 s one")
+    func passSessionWaitsForConnectivity() throws {
+        let manifestURL = try #require(URL(string: "https://registry.invalid/\(UUID().uuidString)/plugins/v3.json"))
+        let client = RegistryClient(manifestURL: manifestURL, session: RegistryClient.makeDefaultSession())
+        let interactive = client.session.configuration
+        let pass = client.updatePassSession.configuration
+
+        #expect(pass.waitsForConnectivity)
+        #expect(pass.timeoutIntervalForResource == 7 * 24 * 60 * 60)
+        #expect(interactive.timeoutIntervalForResource == 30)
+        #expect(pass.urlCache === interactive.urlCache)
+    }
+
+    @Test("While the pass waits for the manifest, a connection neither waits for the pass nor joins its fetch")
+    func connectionSkipsThePassWaitingForTheNetwork() async throws {
+        let fixture = try makeFixture()
+        defer { try? FileManager.default.removeItem(at: fixture.root) }
+        let manager = fixture.manager
+        manager.rejectedPlugins = [makeRejected(bundleId: "com.example.driver")]
+        let (client, manifestURL) = try makeRegistryClient(servingBuild: 20)
+        ManifestStubProtocol.leaveUnanswered(next: 1, at: manifestURL)
+
+        let pass = Task { await manager.runRegistryUpdatePass(registryClient: client, appBuild: 20) }
+        try await waitUntil { ManifestStubProtocol.requestCount(for: manifestURL) == 1 }
+
+        #expect(!manager.isRepairingRejectedPlugins)
+        await manager.waitForRejectedPluginRepairs()
+        await client.fetch(policy: .mustBeCurrent)
+        #expect(client.isManifestCurrent)
+        #expect(ManifestStubProtocol.requestCount(for: manifestURL) == 2)
+
+        pass.cancel()
+        let result = await pass.value
+        #expect(result == .retry)
+    }
+
     @Test("Retries wait 30 s, 300 s, then 600 s, and stop after the fifth attempt")
     func retryBackoff() {
         let delays = (1...5).map { PluginManager.registryUpdateRetryDelay(afterAttempt: $0) }

TableProTests/Core/Plugins/RegistryClientFreshnessTests.swift

@@ -144,12 +144,13 @@ struct RegistryClientFreshnessTests {
         ])
         let (client, _) = try makeEnvironment()
 
-        async let first: Void = client.fetch(policy: .ifStale)
-        async let second: Void = client.fetch(policy: .ifStale)
-        _ = await (first, second)
+        async let first = client.fetch(policy: .ifStale)
+        async let second = client.fetch(policy: .ifStale)
+        let (firstResult, secondResult) = await (first, second)
 
         #expect(MockRegistryProtocol.requestCount == 1)
         #expect(client.isManifestCurrent)
+        #expect(firstResult != nil && secondResult != nil)
     }
 
     @Test("mustBeCurrent fetches again after an ifStale fetch and replaces the manifest")
@@ -173,8 +174,9 @@ struct RegistryClientFreshnessTests {
         MockRegistryProtocol.reset(responses: [(500, Data())])
         let (client, _) = try makeEnvironment(cachedManifest: manifestData(includingTestPlugin: true))
 
-        await client.fetch(policy: .mustBeCurrent)
+        let result = await client.fetch(policy: .mustBeCurrent)
 
+        #expect(result == nil)
         #expect(client.manifest?.plugins.count == 1)
         #expect(!client.isManifestCurrent)
     }