fix: address PR review — ordered FIFO queue, cold-launch fallback, restore comments

Author: datlechin

TablePro/AppDelegate+WindowConfig.swift

@@ -267,7 +267,7 @@ extension AppDelegate {
             configuredWindows.insert(windowId)
 
             let pendingConnectionId = MainActor.assumeIsolated {
-                WindowOpener.shared.consumeAnyPendingConnectionId()
+                WindowOpener.shared.consumeOldestPendingConnectionId()
             }
 
             if pendingConnectionId == nil && !isAutoReconnecting {

TablePro/Core/Services/Infrastructure/WindowOpener.swift

@@ -3,7 +3,7 @@
 //  TablePro
 //
 //  Bridges SwiftUI's openWindow environment action to imperative code.
-//  Stored by ContentView on appear so MainContentCommandActions can open native tabs.
+//  Stored on appear by ContentView, WelcomeViewModel, or ConnectionFormView.
 //
 
 import os
@@ -15,40 +15,42 @@ internal final class WindowOpener {
 
     internal static let shared = WindowOpener()
 
-    /// Set by ContentView when it appears. Safe to store — OpenWindowAction is app-scoped, not view-scoped.
+    /// Set on appear by ContentView, WelcomeViewModel, or ConnectionFormView.
+    /// Safe to store — OpenWindowAction is app-scoped, not view-scoped.
     internal var openWindow: OpenWindowAction?
 
-    /// Payloads for windows that have been requested but not yet acknowledged
-    /// by MainContentView.configureWindow. Keyed by payload.id.
-    /// Stores connectionId so windowDidBecomeKey can compute tabbingIdentifier
-    /// synchronously (before SwiftUI renders) to avoid flicker.
-    internal private(set) var pendingPayloads: [UUID: UUID] = [:]  // [payloadId: connectionId]
+    /// Ordered queue of pending payloads — windows requested via openNativeTab
+    /// but not yet acknowledged by MainContentView.configureWindow.
+    /// Ordered so consumeOldestPendingConnectionId returns the correct entry
+    /// when multiple windows open in quick succession (e.g., tab restore).
+    internal private(set) var pendingPayloads: [(id: UUID, connectionId: UUID)] = []
 
     /// Whether any payloads are pending — used for orphan detection in windowDidBecomeKey.
     internal var hasPendingPayloads: Bool { !pendingPayloads.isEmpty }
 
     /// Opens a new native window tab with the given payload.
+    /// Falls back to .openMainWindow notification if openWindow is not yet available
+    /// (cold launch from Dock menu before any SwiftUI view has appeared).
     internal func openNativeTab(_ payload: EditorTabPayload) {
-        pendingPayloads[payload.id] = payload.connectionId
-        guard let openWindow else {
-            Self.logger.warning("openNativeTab called before openWindow was set — payload dropped")
-            pendingPayloads.removeValue(forKey: payload.id)
-            return
+        pendingPayloads.append((id: payload.id, connectionId: payload.connectionId))
+        if let openWindow {
+            openWindow(id: "main", value: payload)
+        } else {
+            Self.logger.info("openWindow not set — falling back to .openMainWindow notification")
+            NotificationCenter.default.post(name: .openMainWindow, object: payload)
         }
-        openWindow(id: "main", value: payload)
     }
 
     /// Called by MainContentView.configureWindow after the window is fully set up.
     internal func acknowledgePayload(_ id: UUID) {
-        pendingPayloads.removeValue(forKey: id)
+        pendingPayloads.removeAll { $0.id == id }
     }
 
     /// Consumes and returns the connectionId for the oldest pending payload.
-    /// Removes the entry so subsequent calls don't return stale data.
-    internal func consumeAnyPendingConnectionId() -> UUID? {
-        guard let first = pendingPayloads.first else { return nil }
-        pendingPayloads.removeValue(forKey: first.key)
-        return first.value
+    /// Removes the entry so subsequent calls return the next payload in order.
+    internal func consumeOldestPendingConnectionId() -> UUID? {
+        guard !pendingPayloads.isEmpty else { return nil }
+        return pendingPayloads.removeFirst().connectionId
     }
 
     /// Returns the tabbingIdentifier for a connection.

TablePro/Views/Main/MainContentView.swift

@@ -472,6 +472,7 @@ struct MainContentView: View {
                     {
                         Task { await coordinator.switchDatabase(to: selectedTab.databaseName) }
                     } else {
+                        // columns is [] on initial load — buildFilteredQuery uses SELECT *
                         if !selectedTab.filterState.appliedFilters.isEmpty,
                             let tableName = selectedTab.tableName,
                             let tabIndex = tabManager.selectedTabIndex
@@ -491,6 +492,7 @@ struct MainContentView: View {
                         coordinator.executeTableTabQueryDirectly()
                     }
                 } else {
+                    // Reactive path: fires via onChange(of: sessionVersion) when connection is ready
                     coordinator.needsLazyLoad = true
                 }
             }

TableProTests/Core/Services/WindowTabGroupingTests.swift

@@ -18,23 +18,26 @@ import Testing
 struct WindowTabGroupingTests {
     // MARK: - WindowOpener pending payload tracking
 
-    @Test("openNativeTab without openWindow action drops payload and removes from pending")
-    func openNativeTabWithoutOpenWindowDropsPayload() {
+    @Test("openNativeTab without openWindow falls back to notification and keeps pending")
+    func openNativeTabWithoutOpenWindowFallsBack() {
         let connectionId = UUID()
         let opener = WindowOpener.shared
 
         opener.openWindow = nil
         let payload = EditorTabPayload(connectionId: connectionId, tabType: .table, tableName: "users")
         opener.openNativeTab(payload)
 
-        #expect(opener.pendingPayloads[payload.id] == nil)
+        // Payload stays pending (notification handler will create the window)
+        #expect(opener.pendingPayloads.contains { $0.id == payload.id })
+        // Clean up
+        opener.acknowledgePayload(payload.id)
     }
 
     @Test("pendingPayloads is empty initially")
     func pendingPayloadsEmptyInitially() {
         let opener = WindowOpener.shared
-        for id in opener.pendingPayloads.keys {
-            opener.acknowledgePayload(id)
+        for entry in opener.pendingPayloads {
+            opener.acknowledgePayload(entry.id)
         }
 
         #expect(opener.pendingPayloads.isEmpty)
@@ -46,7 +49,30 @@ struct WindowTabGroupingTests {
         let payloadId = UUID()
 
         opener.acknowledgePayload(payloadId)
-        #expect(opener.pendingPayloads[payloadId] == nil)
+        #expect(!opener.pendingPayloads.contains { $0.id == payloadId })
+    }
+
+    @Test("consumeOldestPendingConnectionId returns in FIFO order")
+    func consumeOldestReturnsFIFO() {
+        let opener = WindowOpener.shared
+        // Clear any stale state
+        while opener.consumeOldestPendingConnectionId() != nil {}
+
+        let idA = UUID()
+        let idB = UUID()
+        let payloadA = EditorTabPayload(connectionId: idA, tabType: .query)
+        let payloadB = EditorTabPayload(connectionId: idB, tabType: .query)
+
+        opener.openWindow = nil
+        opener.openNativeTab(payloadA)
+        opener.openNativeTab(payloadB)
+
+        let first = opener.consumeOldestPendingConnectionId()
+        let second = opener.consumeOldestPendingConnectionId()
+
+        #expect(first == idA)
+        #expect(second == idB)
+        #expect(opener.consumeOldestPendingConnectionId() == nil)
     }
 
     // MARK: - TabbingIdentifier resolution