fix(plugins): install rejected plugins one at a time

TablePro/Core/Plugins/PluginManager+AutoUpdate.swift

@@ -39,7 +39,7 @@ extension PluginManager {
     }
 
     func scheduleRegistryUpdatePass(after delay: Duration = .zero) {
-        guard SoftwareUpdater.isUpdatingEnabled else { return }
+        guard isUpdatingEnabled else { return }
         registryUpdateTask?.cancel()
         registryUpdateTask = Task { [weak self] in
             if delay > .zero {
@@ -78,14 +78,27 @@ extension PluginManager {
             Self.logger.warning("Plugin update pass deferred: the plugin manifest is unavailable")
             return .retry
         }
-        isRepairingRejectedPlugins = true
-        defer { isRepairingRejectedPlugins = false }
-        return await applyRegistryUpdates(to: targets, manifest: manifest)
+        let repaired = await repairRejectedPlugins(targets.filter { $0.rejectedURL != nil }, manifest: manifest)
+        let updated = await applyRegistryUpdates(to: targets.filter { $0.rejectedURL == nil }, manifest: manifest)
+        return repaired == .retry ? .retry : updated
     }
 
     func waitForRejectedPluginRepairs() async {
-        guard isRepairingRejectedPlugins else { return }
-        await withCheckedContinuation { rejectedRepairWaiters.append($0) }
+        while isRepairingRejectedPlugins {
+            await withCheckedContinuation { rejectedRepairWaiters.append($0) }
+        }
+    }
+
+    // One holder at a time: the pass and a connection would otherwise both install the same rejected plugin.
+    private func repairRejectedPlugins(
+        _ targets: [RegistryUpdateTarget],
+        manifest: RegistryManifest
+    ) async -> RegistryUpdatePassResult {
+        guard !targets.isEmpty else { return .finished }
+        await waitForRejectedPluginRepairs()
+        isRepairingRejectedPlugins = true
+        defer { isRepairingRejectedPlugins = false }
+        return await applyRegistryUpdates(to: targets, manifest: manifest)
     }
 
     private func applyRegistryUpdates(
@@ -103,9 +116,6 @@ extension PluginManager {
         for update in pending {
             guard !Task.isCancelled else { return .finished }
             guard isRegistryUpdateTarget(update.target) else { continue }
-            if update.target.rejectedURL == nil {
-                isRepairingRejectedPlugins = false
-            }
             do {
                 try await applyRegistryUpdate(update)
             } catch {
@@ -341,19 +351,18 @@ extension PluginManager {
         }
     }
 
-    func ensurePluginReady(forTypeId typeId: String, registryClient: RegistryClient = .shared) async {
-        guard SoftwareUpdater.isUpdatingEnabled else { return }
+    func ensurePluginReady(forTypeId typeId: String, registryClient: RegistryClient) async {
+        guard 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 {
+        guard let manifest = await registryClient.fetch(policy: .mustBeCurrent) else {
             Self.logger.warning("Plugin '\(typeId)' not updated before connect: the plugin manifest is unavailable")
+            // The pass may have got its manifest meanwhile and be installing this plugin.
+            await waitForRejectedPluginRepairs()
             return
         }
         let targets = registryUpdateTargets(forTypeId: typeId, appBuild: Bundle.main.buildNumber)
-        _ = await applyRegistryUpdates(to: targets, manifest: manifest)
+        _ = await repairRejectedPlugins(targets, manifest: manifest)
         AppEvents.shared.pluginsRejected.send(rejectedPlugins)
     }
 

TablePro/Core/Plugins/PluginManager.swift

@@ -25,6 +25,7 @@ final class PluginManager: ObservableObject {
     private let builtInPluginsURL: URL?
     internal let userPluginsDir: URL
     internal let registryInstaller: any RegistryPluginInstalling
+    internal let isUpdatingEnabled: Bool
 
     @Published internal(set) var plugins: [PluginEntry] = []
 
@@ -112,7 +113,7 @@ final class PluginManager: ObservableObject {
     internal var registryUpdateAttempts = 0
     internal var installsInFlight: Set<String> = []
 
-    // Set while a pass installs rejected plugins, so a connection waits for those but not for the network or a staging download.
+    // Held while the pass or a connection installs rejected plugins, never during a manifest fetch or a staging download.
     internal var isRepairingRejectedPlugins = false {
         didSet {
             guard !isRepairingRejectedPlugins else { return }
@@ -132,12 +133,14 @@ final class PluginManager: ObservableObject {
         builtInPluginsURL: URL? = Bundle.main.builtInPlugInsURL,
         userPluginsDir: URL = PluginManager.defaultUserPluginsDir(),
         registryInstaller: any RegistryPluginInstalling = PluginInstaller.shared,
-        initialLoadClock: any Clock<Duration> = ContinuousClock()
+        initialLoadClock: any Clock<Duration> = ContinuousClock(),
+        isUpdatingEnabled: Bool = SoftwareUpdater.isUpdatingEnabled
     ) {
         self.builtInPluginsURL = builtInPluginsURL
         self.userPluginsDir = userPluginsDir
         self.registryInstaller = registryInstaller
         self.initialLoadClock = initialLoadClock
+        self.isUpdatingEnabled = isUpdatingEnabled
     }
 
     nonisolated static func defaultUserPluginsDir() -> URL {

TableProTests/Core/Plugins/PluginRegistryUpdatePassTests.swift

@@ -141,13 +141,19 @@ struct PluginRegistryUpdatePassTests {
     }
 
     private func makeFixture(
-        installer: any RegistryPluginInstalling = PluginInstaller.shared
+        installer: any RegistryPluginInstalling = PluginInstaller.shared,
+        isUpdatingEnabled: Bool = false
     ) throws -> Fixture {
         let root = FileManager.default.temporaryDirectory
             .appendingPathComponent("PluginRegistryUpdatePassTests-\(UUID().uuidString)", isDirectory: true)
         let pluginsDir = root.appendingPathComponent("Plugins", isDirectory: true)
         try FileManager.default.createDirectory(at: pluginsDir, withIntermediateDirectories: true)
-        let manager = PluginManager(builtInPluginsURL: nil, userPluginsDir: pluginsDir, registryInstaller: installer)
+        let manager = PluginManager(
+            builtInPluginsURL: nil,
+            userPluginsDir: pluginsDir,
+            registryInstaller: installer,
+            isUpdatingEnabled: isUpdatingEnabled
+        )
         return Fixture(manager: manager, pluginsDir: pluginsDir, root: root)
     }
 
@@ -461,29 +467,85 @@ struct PluginRegistryUpdatePassTests {
         #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()
+    @Test("While the pass waits for the manifest, connecting repairs the plugin with a manifest request of its own")
+    func connectionRepairsWhileThePassWaitsForTheNetwork() async throws {
+        let installer = FixtureRegistryInstaller()
+        let fixture = try makeFixture(installer: installer, isUpdatingEnabled: true)
         defer { try? FileManager.default.removeItem(at: fixture.root) }
         let manager = fixture.manager
-        manager.rejectedPlugins = [makeRejected(bundleId: "com.example.driver")]
+        manager.rejectedPlugins = [
+            makeRejected(bundleId: "com.example.driver", providedDatabaseTypeIds: [DatabaseType.mssql.pluginTypeId])
+        ]
         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)
+        try await manager.prepareForConnecting(to: .mssql, registryClient: client)
 
+        #expect(ManifestStubProtocol.requestCount(for: manifestURL) == 2)
+        let installedIds = await installer.installedIds
+        #expect(installedIds == ["com.example.driver"])
         pass.cancel()
         let result = await pass.value
         #expect(result == .retry)
     }
 
+    @Test("A pass whose manifest arrives during a connection's repair waits for it, then skips the plugin it repaired")
+    func passWaitsForAConnectionsRepair() async throws {
+        let installGate = AsyncGate()
+        let installer = FixtureRegistryInstaller(installGate: installGate)
+        let fixture = try makeFixture(installer: installer, isUpdatingEnabled: true)
+        defer { try? FileManager.default.removeItem(at: fixture.root) }
+        let manager = fixture.manager
+        let typeId = DatabaseType.mssql.pluginTypeId
+        let rejected = makeRejected(bundleId: "com.example.driver", providedDatabaseTypeIds: [typeId])
+        manager.rejectedPlugins = [rejected]
+        let (client, _) = try makeRegistryClient(servingBuild: 20)
+
+        let connection = Task { await manager.ensurePluginReady(forTypeId: typeId, registryClient: client) }
+        await installGate.arrived(1)
+        let pass = Task { await manager.runRegistryUpdatePass(registryClient: client, appBuild: 20) }
+        try await waitUntil { manager.rejectedRepairWaiters.count == 1 }
+        // Stands in for the load, which the unsigned fixture cannot pass.
+        manager.removeFromRejected(url: rejected.url)
+        await installGate.open()
+        await connection.value
+
+        let result = await pass.value
+        #expect(result == .finished)
+        let installedIds = await installer.installedIds
+        #expect(installedIds == ["com.example.driver"])
+    }
+
+    @Test("A connection that arrives during the pass's repair waits for it, then skips the plugin it repaired")
+    func connectionWaitsForThePassRepair() async throws {
+        let installGate = AsyncGate()
+        let installer = FixtureRegistryInstaller(installGate: installGate)
+        let fixture = try makeFixture(installer: installer, isUpdatingEnabled: true)
+        defer { try? FileManager.default.removeItem(at: fixture.root) }
+        let manager = fixture.manager
+        let typeId = DatabaseType.mssql.pluginTypeId
+        let rejected = makeRejected(bundleId: "com.example.driver", providedDatabaseTypeIds: [typeId])
+        manager.rejectedPlugins = [rejected]
+        let (client, manifestURL) = try makeRegistryClient(servingBuild: 20)
+
+        let pass = Task { await manager.runRegistryUpdatePass(registryClient: client, appBuild: 20) }
+        await installGate.arrived(1)
+        let connection = Task { await manager.ensurePluginReady(forTypeId: typeId, registryClient: client) }
+        try await waitUntil { manager.rejectedRepairWaiters.count == 1 }
+        // Stands in for the load, which the unsigned fixture cannot pass.
+        manager.removeFromRejected(url: rejected.url)
+        await installGate.open()
+        await connection.value
+
+        #expect(ManifestStubProtocol.requestCount(for: manifestURL) == 1)
+        let installedIds = await installer.installedIds
+        #expect(installedIds == ["com.example.driver"])
+        _ = await pass.value
+    }
+
     @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) }