From e4b648574a6c6f7fe9cf77fb9f1fe05eea09069e Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 20 Oct 2026 21:16:50 +0700 Subject: [PATCH 1/4] fix(connections): reopen every strip entry, leaving canceled and disconnected ones not connected --- .claude/rules/connection-window.md | 2 +- .claude/rules/data-sync-storage.md | 1 + .../Infrastructure/ConnectionWorkspace.swift | 4 - .../Infrastructure/LaunchEnvironment.swift | 17 +- .../MainSplitViewController.swift | 11 +- .../Infrastructure/TabWindowController.swift | 2 + .../Infrastructure/WindowManager.swift | 22 +- .../Storage/LastOpenConnectionsStorage.swift | 76 +++++-- .../Core/Storage/RecoveryConnectionList.swift | 60 +++++- .../Core/Storage/SessionRecoveryTracker.swift | 50 +++-- ...ewControllerPaneSynchronizationTests.swift | 13 ++ .../OffscreenConnectionWindowTests.swift | 5 +- .../LastOpenConnectionsStorageTests.swift | 192 ++++++++++++++++++ .../Storage/RecoveryConnectionListTests.swift | 182 ++++++++++++----- 14 files changed, 500 insertions(+), 137 deletions(-) create mode 100644 apple/TableProTests/Core/Storage/LastOpenConnectionsStorageTests.swift diff --git a/.claude/rules/connection-window.md b/.claude/rules/connection-window.md index e385dd0b59..722e00c323 100644 --- a/.claude/rules/connection-window.md +++ b/.claude/rules/connection-window.md @@ -13,7 +13,7 @@ paths: - **A cancel updates the UI at once and is fenced by the workspace's `attemptToken`**, because the driver may finish late. Closing a window cancels the attempt of every workspace it hosts. - **`transition(to:for:)` is the only phase writer and ends in `syncPanes(of:)`** for the workspace it names, selected or not, so a background connection repaints. Panes rebuild only when their `WorkspacePaneRenderKey` changes, so a repaint request is cheap. - **An installed driver is a handle, not a live connection.** Health comes from `ConnectionSession.liveness`, set only through `markSessionUnreachable(_:startedWith:info:)` and `markSessionLive`, and every UI reads `reportedStatus`, never `status` or `hasDriver`. Never nil the driver to signal an outage: metadata reads, queries and the reconnect still go through it. -- **A failed connect keeps its place in "Reopen Last Session"; a canceled one leaves it** (`retainsRestoreIntent`). +- **"Reopen Last Session" brings back every connection on the strip, and only a connected or failed one dials again** (`retainsRestoreIntent`). A canceled or disconnected one comes back Not Connected. - **Window titles go through `WindowTitleResolver`** and editor tab labels through `EditorTabLabelResolver`. Never write `window.title`; change `tab.title` and call `QueryTabManager.markTabRenamed(_:)`. A title must be right and non-blank from creation, because AppKit draws background tabs that never activate. - **Only the user emptying a connection may clear its saved tabs**: `closeTabsByUser`, or `takeTabForMove` taking its last tab. A lost session empties the tab manager without the user closing anything, so teardown saves (`saveAggregatedSync`) skip an empty list, and a clear before the connection's saved tabs were read (`hasObservedTabs`) is refused. - **A restore merges, never replaces.** A tab can reach a coordinator before its `.task` restore (a reopened or linked tab), so `RestoredTabMerge` keeps it. diff --git a/.claude/rules/data-sync-storage.md b/.claude/rules/data-sync-storage.md index b26b5bb73c..884e78dea8 100644 --- a/.claude/rules/data-sync-storage.md +++ b/.claude/rules/data-sync-storage.md @@ -39,3 +39,4 @@ This path holds user data. A silent wrong answer here looks exactly like an empt | History drawer state | UserDefaults, device-local | `HistoryPanelPreferencesStorage` | | Trusted external links (loopback only) | UserDefaults | `ExternalConnectionTrustStore` | | Table load timings (no identifying data, never uploaded) | JSON lines | `TableLoadHistoryStore` | +| Reopen Last Session list (`LastSession.json`, plus `LastOpenConnections.json` holding only the ids that reconnect, which older builds read) | JSON | `LastOpenConnectionsStorage` | diff --git a/apple/TablePro/Core/Services/Infrastructure/ConnectionWorkspace.swift b/apple/TablePro/Core/Services/Infrastructure/ConnectionWorkspace.swift index 2bede2a98d..c0daa0a738 100644 --- a/apple/TablePro/Core/Services/Infrastructure/ConnectionWorkspace.swift +++ b/apple/TablePro/Core/Services/Infrastructure/ConnectionWorkspace.swift @@ -384,10 +384,6 @@ internal final class ConnectionWorkspace { } } - internal var retainsRestoreIntent: Bool { - ConnectionWindowPhaseMachine.retainsRestoreIntent(phase: phase) - } - /// The panes go down with the rest of it. They retain the SwiftUI tree, which retains the /// coordinator this tears down, and a coordinator only leaves the app-wide registry on deinit. internal func teardown() { diff --git a/apple/TablePro/Core/Services/Infrastructure/LaunchEnvironment.swift b/apple/TablePro/Core/Services/Infrastructure/LaunchEnvironment.swift index 6473df450f..a757d896ba 100644 --- a/apple/TablePro/Core/Services/Infrastructure/LaunchEnvironment.swift +++ b/apple/TablePro/Core/Services/Infrastructure/LaunchEnvironment.swift @@ -168,22 +168,27 @@ internal final class LiveLaunchEnvironment: LaunchEnvironment { ConnectionWindowIdentity.isConnectionWindow($0.identifier?.rawValue) }) else { return } - let connectionIds = LastOpenConnectionsStorage.shared.load() - guard !connectionIds.isEmpty else { return } + let session = LastOpenConnectionsStorage.shared.load() + guard !session.connections.isEmpty else { return } let knownIds = Set(ConnectionStorage.shared.loadConnections().map(\.id)) var frontWindow: NSWindow? - for connectionId in connectionIds where knownIds.contains(connectionId) { + for connection in session.connections where knownIds.contains(connection.id) { WindowManager.shared.openTab( - payload: EditorTabPayload(connectionId: connectionId, intent: .restoreOrDefault), + payload: EditorTabPayload(connectionId: connection.id, intent: .restoreOrDefault), activate: false, - autoConnect: true + autoConnect: connection.reconnects ) if frontWindow == nil { - frontWindow = WindowManager.shared.window(for: connectionId) + frontWindow = WindowManager.shared.window(for: connection.id) } } + // Each adoption selects the newcomer, which could leave the window on a Not Connected entry. + if let shown = session.shownConnectionId, knownIds.contains(shown) { + WindowManager.shared.show(shown, inWindowHosting: shown) + } + guard let frontWindow else { return } WindowOpener.shared.closeWelcome() frontWindow.makeKeyAndOrderFront(nil) diff --git a/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift b/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift index b1de78f887..d5e220c969 100644 --- a/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift +++ b/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift @@ -366,6 +366,7 @@ internal final class MainSplitViewController: NSSplitViewController { /// `NSSplitViewItem` remembers, rather than depending on a second restore. workspaces.onSelectionChange = { [weak self] _ in self?.applySelectedWorkspace() + SessionRecoveryTracker.sync() } /// A connection joining or leaving the window changes what every rail in the app lists, @@ -374,6 +375,8 @@ internal final class MainSplitViewController: NSSplitViewController { workspaces.onMembershipChange = { [weak self] in self?.recentTabSwitcher.cancel() AppEvents.shared.connectionWindowsChanged.send() + // A Not Connected entry joins or leaves without a phase transition to record it. + SessionRecoveryTracker.sync() } splitView.autosaveName = splitAutosaveName @@ -811,14 +814,6 @@ internal final class MainSplitViewController: NSSplitViewController { applyPhase() } - internal var retainsRestoreIntent: Bool { - workspaces.workspaces.contains { $0.retainsRestoreIntent } - } - - internal var connectionIdsRetainingRestoreIntent: [UUID] { - workspaces.workspaces.filter(\.retainsRestoreIntent).map(\.connectionId) - } - // MARK: - Pane Construction /// Rebuilds one connection's panes into its own hosting controllers, whether or not it is the diff --git a/apple/TablePro/Core/Services/Infrastructure/TabWindowController.swift b/apple/TablePro/Core/Services/Infrastructure/TabWindowController.swift index 75af788f41..c465b7fdcb 100644 --- a/apple/TablePro/Core/Services/Infrastructure/TabWindowController.swift +++ b/apple/TablePro/Core/Services/Infrastructure/TabWindowController.swift @@ -219,6 +219,8 @@ internal final class TabWindowController: NSWindowController, NSWindowDelegate { if let splitVC = window.contentViewController as? MainSplitViewController { splitVC.startActivationConnectIfNeeded() splitVC.syncFrontmostTabManager() + // The recovery list records the connection the front window shows. + SessionRecoveryTracker.sync() } guard let coordinator = MainContentCoordinator.coordinator(forWindow: window) else { return } diff --git a/apple/TablePro/Core/Services/Infrastructure/WindowManager.swift b/apple/TablePro/Core/Services/Infrastructure/WindowManager.swift index 34e6c0f9f3..a24d3c3bc0 100644 --- a/apple/TablePro/Core/Services/Infrastructure/WindowManager.swift +++ b/apple/TablePro/Core/Services/Infrastructure/WindowManager.swift @@ -35,6 +35,10 @@ internal final class WindowManager { if let existing = host.workspaces.workspace(for: payload.connectionId) { host.workspaces.select(payload.connectionId) existing.open(payload) + // An open that asks to connect dials one sitting not connected, as picking it in the switcher does. + if autoConnect { + host.reconnectWorkspaceIfIdle(payload.connectionId) + } } else { host.adoptWorkspace(payload: payload, autoConnect: autoConnect) } @@ -156,6 +160,14 @@ internal final class WindowManager { .first } + // Front to back and visible or not: a hidden app or a minimized window still has a selection to restore. + internal func shownConnectionId() -> UUID? { + let ordered = ([NSApp.keyWindow].compactMap { $0 } + NSApp.orderedWindows) + .lazy + .compactMap { $0.contentViewController as? MainSplitViewController } + return (ordered.first ?? hosts().first)?.workspaces.selectedConnectionId + } + /// The window already hosting this connection wins, so opening a table for it never lands in /// a different window that merely happened to be in front. The choice itself is /// `WindowHostSelection`, which is pure and tested. @@ -489,6 +501,8 @@ internal final class WindowManager { self?.release(windowKey: key) } } + // The window's first workspace joined before its registry reported membership changes. + SessionRecoveryTracker.sync() } private func release(windowKey: ObjectIdentifier) { @@ -634,14 +648,6 @@ internal final class WindowManager { } } - internal func connectionIdsRetainingRestoreIntent() -> [UUID] { - var seen = Set() - return controllers.values - .compactMap { $0.window?.contentViewController as? MainSplitViewController } - .flatMap(\.connectionIdsRetainingRestoreIntent) - .filter { seen.insert($0).inserted } - } - /// Closing a connection removes its workspace. The window itself only closes once it has no /// connection left to show, because it is no longer the connection's window. /// A miniaturized window still hosts its connections, so `isVisible` is not the test: closing a diff --git a/apple/TablePro/Core/Storage/LastOpenConnectionsStorage.swift b/apple/TablePro/Core/Storage/LastOpenConnectionsStorage.swift index c8a8764bb9..27b2499100 100644 --- a/apple/TablePro/Core/Storage/LastOpenConnectionsStorage.swift +++ b/apple/TablePro/Core/Storage/LastOpenConnectionsStorage.swift @@ -16,7 +16,11 @@ final class LastOpenConnectionsStorage { nonisolated private static let logger = Logger(subsystem: "com.TablePro", category: "LastOpenConnections") - private let fileURL: URL + // The legacy file keeps the [UUID] format older builds read, holding only connections that + // reconnect, so an older build never dials one the user canceled. + private let legacyURL: URL + private let sessionURL: URL + private var isSealed = false private convenience init() { let appSupport = AppStorageEnvironment.shared.applicationSupportRoot @@ -25,35 +29,79 @@ final class LastOpenConnectionsStorage { init(directory: URL) { try? FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) - fileURL = directory.appendingPathComponent("LastOpenConnections.json") + legacyURL = directory.appendingPathComponent("LastOpenConnections.json") + sessionURL = directory.appendingPathComponent("LastSession.json") } - func save(connectionIds: [UUID]) { - guard !connectionIds.isEmpty else { + // A missing legacy list means an older build removed it, so this never leaves the session alone: + // a first save writes the list before the session, and every later save writes the session first + // so one cut short leaves the session the newer file. The list is written even when empty. + func save(_ session: LastSession) { + guard !isSealed else { return } + guard !session.connections.isEmpty else { clear() return } + let reconnecting = session.reconnectingIds + let hasLegacy = FileManager.default.fileExists(atPath: legacyURL.path) + if !hasLegacy { + write(reconnecting, to: legacyURL) + } + write(session, to: sessionURL) + if hasLegacy { + write(reconnecting, to: legacyURL) + } + } + + // An older build rewrites only the legacy file, so when the two disagree the newer one is current. + func load() -> LastSession { + guard let legacy = read([UUID].self, from: legacyURL) else { return .empty } + let legacySession = LastSession( + connections: legacy.map { LastSession.Connection(id: $0, reconnects: true) }, + shownConnectionId: nil + ) + guard let session = read(LastSession.self, from: sessionURL) else { return legacySession } + guard session.reconnectingIds != legacy else { return session } + return modificationDate(of: legacyURL) > modificationDate(of: sessionURL) ? legacySession : session + } + + func clear() { + remove(legacyURL) + remove(sessionURL) + } + + // The quit save is the last word: AppKit closes the windows after it, and no coordinator may + // exist to have flagged termination, so those closes would otherwise save an empty session. + func seal() { + isSealed = true + } + + private func write(_ value: some Encodable, to url: URL) { do { - let data = try JSONEncoder().encode(connectionIds) - try data.write(to: fileURL, options: .atomic) + let data = try JSONEncoder().encode(value) + try data.write(to: url, options: .atomic) } catch { Self.logger.error("Failed to save last open connections: \(error.publicLogShape, privacy: .public)") } } - func load() -> [UUID] { - guard FileManager.default.fileExists(atPath: fileURL.path) else { return [] } + private func read(_ type: Value.Type, from url: URL) -> Value? { + guard FileManager.default.fileExists(atPath: url.path) else { return nil } do { - let data = try Data(contentsOf: fileURL) - return try JSONDecoder().decode([UUID].self, from: data) + return try JSONDecoder().decode(type, from: Data(contentsOf: url)) } catch { Self.logger.error("Failed to load last open connections: \(error.publicLogShape, privacy: .public)") - return [] + return nil } } - func clear() { - guard FileManager.default.fileExists(atPath: fileURL.path) else { return } - try? FileManager.default.removeItem(at: fileURL) + private func modificationDate(of url: URL) -> Date { + let attributes = try? FileManager.default.attributesOfItem(atPath: url.path) + return attributes?[.modificationDate] as? Date ?? .distantPast + } + + private func remove(_ url: URL) { + guard FileManager.default.fileExists(atPath: url.path) else { return } + try? FileManager.default.removeItem(at: url) } } diff --git a/apple/TablePro/Core/Storage/RecoveryConnectionList.swift b/apple/TablePro/Core/Storage/RecoveryConnectionList.swift index b74af9cc1e..6337f5694d 100644 --- a/apple/TablePro/Core/Storage/RecoveryConnectionList.swift +++ b/apple/TablePro/Core/Storage/RecoveryConnectionList.swift @@ -7,22 +7,60 @@ import Foundation struct RecoveryCandidate { let connectionId: UUID + let openedAt: Date + let phase: ConnectionWindowPhase let isActivated: Bool - let retainsRestoreIntent: Bool +} + +struct LastSession: Codable, Equatable { + struct Connection: Codable, Equatable { + let id: UUID + let reconnects: Bool + } + + var connections: [Connection] + var shownConnectionId: UUID? + + static let empty = LastSession(connections: [], shownConnectionId: nil) + + var reconnectingIds: [UUID] { + connections.filter(\.reconnects).map(\.id) + } } enum RecoveryConnectionList { - static func connectionIds(from candidates: [RecoveryCandidate]) -> [UUID] { - var seen = Set() - return candidates - .filter { $0.isActivated || $0.retainsRestoreIntent } - .map(\.connectionId) - .filter { seen.insert($0).inserted } + // Every strip entry comes back, but a canceled or user-ended one never dials again (#1358). + // Ordered by openedAt, which is how the strip ranks entries nobody dragged. + static func lastSession( + from candidates: [RecoveryCandidate], + endedByUser: (UUID) -> Bool = { _ in false }, + shownConnectionId: UUID? + ) -> LastSession { + var openedAt: [UUID: Date] = [:] + var reconnects: [UUID: Bool] = [:] + for candidate in candidates where candidate.phase != .closing { + let id = candidate.connectionId + openedAt[id] = min(openedAt[id] ?? candidate.openedAt, candidate.openedAt) + let wantsConnection = candidate.isActivated + || ConnectionWindowPhaseMachine.retainsRestoreIntent(phase: candidate.phase) + reconnects[id] = (reconnects[id] ?? false) || wantsConnection + } + + let ordered = openedAt.keys.sorted { lhs, rhs in + let left = openedAt[lhs] ?? .distantFuture + let right = openedAt[rhs] ?? .distantFuture + guard left == right else { return left < right } + return lhs.uuidString < rhs.uuidString + } + let connections = ordered.map { id in + LastSession.Connection(id: id, reconnects: (reconnects[id] ?? false) && !endedByUser(id)) + } + let shown = shownConnectionId.flatMap { openedAt[$0] == nil ? nil : $0 } + return LastSession(connections: connections, shownConnectionId: shown) } - /// A session a client launched that never showed a connection says nothing about the person's - /// last one, so quitting it leaves the recovery list as it found it. - static func shouldSaveOnQuit(_ connectionIds: [UUID], origin: AppSessionOrigin) -> Bool { - !connectionIds.isEmpty || origin == .user + // A client-launched session that never showed a connection says nothing about the user's last one. + static func shouldSaveOnQuit(_ session: LastSession, origin: AppSessionOrigin) -> Bool { + !session.connections.isEmpty || origin == .user } } diff --git a/apple/TablePro/Core/Storage/SessionRecoveryTracker.swift b/apple/TablePro/Core/Storage/SessionRecoveryTracker.swift index 48527b5200..3cf3c3bfb4 100644 --- a/apple/TablePro/Core/Storage/SessionRecoveryTracker.swift +++ b/apple/TablePro/Core/Storage/SessionRecoveryTracker.swift @@ -9,48 +9,44 @@ import Foundation enum SessionRecoveryTracker { private static let storage: LastOpenConnectionsStorage? = AppStorageEnvironment.shared.isUnitTestHost ? nil : .shared - /// Connections eligible for "Reopen Last Session": one the user actually worked in, - /// or one whose window is still holding the intent to reach it. A cancelled attempt - /// and a closing window are both excluded, so neither is replayed on the next launch. - /// A connection that merely failed to reach its server is kept, because a database - /// that was down is not a user who gave up. - static func connectionIds() -> [UUID] { - let activated = MainContentCoordinator.activeCoordinators.values.map { + static func lastSession() -> LastSession { + let activated = Set( + MainContentCoordinator.activeCoordinators.values.filter(\.isActivated).map(\.connectionId) + ) + let candidates = WindowManager.shared.hostedWorkspaces().map { RecoveryCandidate( connectionId: $0.connectionId, - isActivated: $0.isActivated, - retainsRestoreIntent: false + openedAt: $0.openedAt, + phase: $0.phase, + isActivated: activated.contains($0.connectionId) ) } - let pending = WindowManager.shared.connectionIdsRetainingRestoreIntent().map { - RecoveryCandidate(connectionId: $0, isActivated: false, retainsRestoreIntent: true) - } - return RecoveryConnectionList.connectionIds(from: activated + pending) - .filter { !wasEndedByUser($0) } + return RecoveryConnectionList.lastSession( + from: candidates, + endedByUser: wasEndedByUser, + shownConnectionId: WindowManager.shared.shownConnectionId() + ) } - /// A connection the user disconnected is never replayed, whatever its windows still report. A - /// window subscribes to session changes only while it is on screen, so a native tab that AppKit - /// joined but never displayed sits frozen at the phase it was born with and would otherwise vote - /// to reopen a session the user deliberately ended. The flag clears when a fresh connect begins. + // A native tab AppKit joined but never displayed keeps the phase it was born with, so a stale + // `.connected` there must not redial a session the user ended. The flag clears on the next connect. private static func wasEndedByUser(_ connectionId: UUID) -> Bool { guard DatabaseManager.shared.wasDisconnectedByUser(connectionId) else { return false } return DatabaseManager.shared.activeSessions[connectionId] == nil } - /// Rewrite the recovery list from the live window set. Called whenever that set - /// changes so the file stays correct after a crash or a force quit, neither of - /// which runs `applicationWillTerminate`. + // Kept current on every change because a crash or force quit never runs `applicationWillTerminate`. static func sync() { guard !MainContentCoordinator.isAppTerminating, let storage else { return } - storage.save(connectionIds: connectionIds()) + storage.save(lastSession()) } static func saveOnQuit() { - let ids = connectionIds() - guard let storage, - RecoveryConnectionList.shouldSaveOnQuit(ids, origin: AppActivationPolicyController.shared.origin) - else { return } - storage.save(connectionIds: ids) + guard let storage else { return } + let session = lastSession() + if RecoveryConnectionList.shouldSaveOnQuit(session, origin: AppActivationPolicyController.shared.origin) { + storage.save(session) + } + storage.seal() } } diff --git a/apple/TableProTests/Core/Services/Infrastructure/MainSplitViewControllerPaneSynchronizationTests.swift b/apple/TableProTests/Core/Services/Infrastructure/MainSplitViewControllerPaneSynchronizationTests.swift index a64d7b574a..5ec64b472a 100644 --- a/apple/TableProTests/Core/Services/Infrastructure/MainSplitViewControllerPaneSynchronizationTests.swift +++ b/apple/TableProTests/Core/Services/Infrastructure/MainSplitViewControllerPaneSynchronizationTests.swift @@ -532,6 +532,19 @@ struct MainSplitViewControllerPaneSynchronizationTests { #expect(saved?.tabs.first?.query == "SELECT 1") } + @Test("A connection reopened without its connect stays not connected when its window activates") + func reopenedWithoutConnectStaysNotConnected() throws { + let harness = try Harness() + defer { harness.tearDown() } + #expect(harness.controller.workspaces.selectedConnectionId == harness.foreground.connectionId) + + harness.controller.startActivationConnectIfNeeded() + + #expect(harness.foreground.phase == .idle) + #expect(harness.foreground.attemptToken == nil) + #expect(harness.foreground.resolvedPane == .unavailable(.notConnected)) + } + @Test("Reconnecting only when idle shows a connecting connection and leaves its attempt alone") func reconnectIfIdleLeavesAConnectingWorkspaceAlone() throws { let harness = try Harness() diff --git a/apple/TableProTests/Core/Services/Infrastructure/OffscreenConnectionWindowTests.swift b/apple/TableProTests/Core/Services/Infrastructure/OffscreenConnectionWindowTests.swift index 89df1c784e..84ec484b07 100644 --- a/apple/TableProTests/Core/Services/Infrastructure/OffscreenConnectionWindowTests.swift +++ b/apple/TableProTests/Core/Services/Infrastructure/OffscreenConnectionWindowTests.swift @@ -69,9 +69,10 @@ internal struct OffscreenConnectionWindowTests { let fixture = try OffscreenConnectionWindow(size: Self.size, connectedTo: connection) let coordinator = try #require(fixture.workspace.sessionState?.coordinator) - #expect(coordinator.isActivated, "Only an activated coordinator puts its connection on the recovery list") + #expect(coordinator.isActivated, "Only an activated coordinator marks its connection to reconnect") + let recorded = LastOpenConnectionsStorage.shared.load().connections.map(\.id) #expect( - !LastOpenConnectionsStorage.shared.load().contains(connection.id), + !recorded.contains(connection.id), "The fixture's connection reached the user's recovery list" ) fixture.setInspectorOpen(true) diff --git a/apple/TableProTests/Core/Storage/LastOpenConnectionsStorageTests.swift b/apple/TableProTests/Core/Storage/LastOpenConnectionsStorageTests.swift new file mode 100644 index 0000000000..e3c1ab3807 --- /dev/null +++ b/apple/TableProTests/Core/Storage/LastOpenConnectionsStorageTests.swift @@ -0,0 +1,192 @@ +// +// LastOpenConnectionsStorageTests.swift +// TableProTests +// + +import Foundation +@testable import TablePro +import Testing + +@MainActor +struct LastOpenConnectionsStorageTests { + private let directory: URL + private let storage: LastOpenConnectionsStorage + + init() { + directory = FileManager.default.temporaryDirectory + .appendingPathComponent("LastOpenConnectionsStorageTests-\(UUID().uuidString)", isDirectory: true) + storage = LastOpenConnectionsStorage(directory: directory) + } + + private var legacyURL: URL { directory.appendingPathComponent("LastOpenConnections.json") } + private var sessionURL: URL { directory.appendingPathComponent("LastSession.json") } + + private func legacyIds() throws -> [UUID] { + try JSONDecoder().decode([UUID].self, from: Data(contentsOf: legacyURL)) + } + + private func modificationDate(of url: URL) throws -> Date { + let attributes = try FileManager.default.attributesOfItem(atPath: url.path) + return try #require(attributes[.modificationDate] as? Date) + } + + private func touch(_ url: URL, after other: URL) throws { + let later = try modificationDate(of: other).addingTimeInterval(60) + try FileManager.default.setAttributes([.modificationDate: later], ofItemAtPath: url.path) + } + + @Test("A relaunch brings back every strip entry in order, and only the live ones dial") + func relaunchWithCanceledAndDisconnectedConnections() { + defer { try? FileManager.default.removeItem(at: directory) } + let start = Date(timeIntervalSinceReferenceDate: 800_000_000) + let mysql = UUID() + let redis = UUID() + let canceled = UUID() + let disconnected = UUID() + func candidate(_ id: UUID, _ offset: TimeInterval, _ phase: ConnectionWindowPhase) -> RecoveryCandidate { + RecoveryCandidate( + connectionId: id, + openedAt: start.addingTimeInterval(offset), + phase: phase, + isActivated: phase == .connected + ) + } + let candidates = [ + candidate(disconnected, 3, .unavailable(.disconnectedByUser)), + candidate(mysql, 0, .connected), + candidate(canceled, 2, .unavailable(.cancelled)), + candidate(redis, 1, .connected), + ] + + storage.save(RecoveryConnectionList.lastSession(from: candidates, shownConnectionId: redis)) + let relaunched = LastOpenConnectionsStorage(directory: directory).load() + + #expect(relaunched.connections == [ + LastSession.Connection(id: mysql, reconnects: true), + LastSession.Connection(id: redis, reconnects: true), + LastSession.Connection(id: canceled, reconnects: false), + LastSession.Connection(id: disconnected, reconnects: false), + ]) + #expect(relaunched.shownConnectionId == redis) + } + + @Test("The file older builds read holds only the connections that dial") + func legacyFileHoldsOnlyReconnectingIds() throws { + defer { try? FileManager.default.removeItem(at: directory) } + let live = UUID() + let canceled = UUID() + + storage.save(LastSession(connections: [ + LastSession.Connection(id: canceled, reconnects: false), + LastSession.Connection(id: live, reconnects: true), + ])) + + #expect(try legacyIds() == [live]) + } + + @Test("A session with nothing to dial leaves an older build nothing to dial") + func nothingToDialWritesAnEmptyLegacyList() throws { + defer { try? FileManager.default.removeItem(at: directory) } + let canceled = LastSession(connections: [LastSession.Connection(id: UUID(), reconnects: false)]) + storage.save(LastSession(connections: [LastSession.Connection(id: UUID(), reconnects: true)])) + + storage.save(canceled) + + #expect(try legacyIds().isEmpty) + #expect(storage.load() == canceled) + } + + @Test("A list written by an older build reopens every connection it names") + func legacyFileAloneReconnectsEverything() throws { + defer { try? FileManager.default.removeItem(at: directory) } + let first = UUID() + let second = UUID() + try JSONEncoder().encode([first, second]).write(to: legacyURL) + + #expect(storage.load() == LastSession(connections: [ + LastSession.Connection(id: first, reconnects: true), + LastSession.Connection(id: second, reconnects: true), + ])) + } + + @Test("A list an older build wrote after the session wins over it") + func olderBuildsNewerListWins() throws { + defer { try? FileManager.default.removeItem(at: directory) } + let after = UUID() + storage.save(LastSession(connections: [ + LastSession.Connection(id: UUID(), reconnects: true), + LastSession.Connection(id: UUID(), reconnects: false), + ])) + + try JSONEncoder().encode([after]).write(to: legacyURL) + try touch(legacyURL, after: sessionURL) + + #expect(storage.load() == LastSession(connections: [LastSession.Connection(id: after, reconnects: true)])) + } + + @Test("A save cut short before the legacy list keeps the newer session") + func interruptedSaveKeepsTheNewerSession() throws { + defer { try? FileManager.default.removeItem(at: directory) } + let connected = LastSession.Connection(id: UUID(), reconnects: true) + let canceled = LastSession.Connection(id: UUID(), reconnects: false) + storage.save(LastSession(connections: [connected, canceled])) + let newer = LastSession(connections: [connected, canceled, LastSession.Connection(id: UUID(), reconnects: true)]) + + try JSONEncoder().encode(newer).write(to: sessionURL) + try touch(sessionURL, after: legacyURL) + + #expect(storage.load() == newer) + } + + @Test("A first save writes the legacy list before the session, and a later save after it") + func saveOrderLeavesARecoverableFileBehind() throws { + defer { try? FileManager.default.removeItem(at: directory) } + let first = LastSession(connections: [LastSession.Connection(id: UUID(), reconnects: false)]) + let second = LastSession(connections: [LastSession.Connection(id: UUID(), reconnects: true)]) + + storage.save(first) + #expect(try modificationDate(of: legacyURL) <= modificationDate(of: sessionURL)) + + storage.save(second) + #expect(try modificationDate(of: legacyURL) >= modificationDate(of: sessionURL)) + #expect(storage.load() == second) + } + + @Test("A list an older build removed reopens nothing") + func olderBuildsRemovedListReopensNothing() throws { + defer { try? FileManager.default.removeItem(at: directory) } + storage.save(LastSession(connections: [LastSession.Connection(id: UUID(), reconnects: true)])) + + try FileManager.default.removeItem(at: legacyURL) + + #expect(storage.load() == .empty) + } + + @Test("Windows closing after the quit save leave the saved session alone") + func sealedStorageKeepsTheQuitSave() { + defer { try? FileManager.default.removeItem(at: directory) } + let notConnected = LastSession(connections: [ + LastSession.Connection(id: UUID(), reconnects: false), + LastSession.Connection(id: UUID(), reconnects: false), + ]) + storage.save(notConnected) + + storage.seal() + storage.save(.empty) + storage.save(LastSession(connections: [LastSession.Connection(id: UUID(), reconnects: true)])) + + #expect(LastOpenConnectionsStorage(directory: directory).load() == notConnected) + } + + @Test("An empty session clears both files") + func emptySessionClearsBothFiles() { + defer { try? FileManager.default.removeItem(at: directory) } + storage.save(LastSession(connections: [LastSession.Connection(id: UUID(), reconnects: true)])) + + storage.save(.empty) + + #expect(!FileManager.default.fileExists(atPath: legacyURL.path)) + #expect(!FileManager.default.fileExists(atPath: sessionURL.path)) + #expect(storage.load() == .empty) + } +} diff --git a/apple/TableProTests/Core/Storage/RecoveryConnectionListTests.swift b/apple/TableProTests/Core/Storage/RecoveryConnectionListTests.swift index 8319dd2676..7975a4d64c 100644 --- a/apple/TableProTests/Core/Storage/RecoveryConnectionListTests.swift +++ b/apple/TableProTests/Core/Storage/RecoveryConnectionListTests.swift @@ -2,9 +2,8 @@ // RecoveryConnectionListTests.swift // TableProTests // -// Pins #1358: "Reopen Last Session" must not replay a connection whose attempt the -// user cancelled. It must still replay one whose server was merely unreachable, -// which a failed restore used to erase from the list as if the user had given up. +// Every connection on the strip comes back after a relaunch (#3658), but one whose connect the +// user canceled or ended never dials again (#1358). // import Foundation @@ -12,95 +11,166 @@ import Foundation import Testing struct RecoveryConnectionListTests { - @Test("A connected window is restored") - func activatedWindowIsRestored() { - let id = UUID() - - let ids = RecoveryConnectionList.connectionIds( - from: [RecoveryCandidate(connectionId: id, isActivated: true, retainsRestoreIntent: false)] + private static let start = Date(timeIntervalSinceReferenceDate: 800_000_000) + + private static let failure = ConnectionFailureInfo( + message: "Could not connect to the server.", + failureReason: "The server refused the connection.", + recoverySuggestion: "Make sure the database server is running." + ) + + private static func candidate( + _ id: UUID, + openedAt offset: TimeInterval = 0, + phase: ConnectionWindowPhase, + isActivated: Bool = false + ) -> RecoveryCandidate { + RecoveryCandidate( + connectionId: id, + openedAt: start.addingTimeInterval(offset), + phase: phase, + isActivated: isActivated ) - - #expect(ids == [id]) } - @Test("A cancelled connection attempt is not restored") - func cancelledAttemptIsNotRestored() { - let ids = RecoveryConnectionList.connectionIds( - from: [RecoveryCandidate(connectionId: UUID(), isActivated: false, retainsRestoreIntent: false)] + private static func session( + _ candidates: [RecoveryCandidate], + endedByUser: Set = [], + shown: UUID? = nil + ) -> LastSession { + RecoveryConnectionList.lastSession( + from: candidates, + endedByUser: { endedByUser.contains($0) }, + shownConnectionId: shown ) + } - #expect(ids.isEmpty) + @Test("A connected connection reconnects") + func connectedReconnects() { + let id = UUID() + + let session = Self.session([Self.candidate(id, phase: .connected, isActivated: true)]) + + #expect(session.connections == [LastSession.Connection(id: id, reconnects: true)]) } - @Test("A window whose server was unreachable is still restored") - func failedConnectionIsStillRestored() { + @Test("A canceled connect comes back without dialing") + func canceledComesBackNotConnected() { let id = UUID() - let ids = RecoveryConnectionList.connectionIds( - from: [RecoveryCandidate(connectionId: id, isActivated: false, retainsRestoreIntent: true)] - ) + let session = Self.session([Self.candidate(id, phase: .unavailable(.cancelled))]) - #expect(ids == [id]) + #expect(session.connections == [LastSession.Connection(id: id, reconnects: false)]) } - @Test("Cancelling one connection leaves the other connected windows restorable") - func cancelledAttemptDoesNotDropConnectedWindows() { - let connected = UUID() - let cancelled = UUID() + @Test("A connection the user disconnected comes back without dialing") + func disconnectedComesBackNotConnected() { + let id = UUID() - let ids = RecoveryConnectionList.connectionIds(from: [ - RecoveryCandidate(connectionId: connected, isActivated: true, retainsRestoreIntent: false), - RecoveryCandidate(connectionId: cancelled, isActivated: false, retainsRestoreIntent: false), - ]) + let session = Self.session([Self.candidate(id, phase: .unavailable(.disconnectedByUser))]) - #expect(ids == [connected]) + #expect(session.connections == [LastSession.Connection(id: id, reconnects: false)]) } - @Test("A connection with several windows is listed once") - func duplicateConnectionIsListedOnce() { + @Test("A connection that never dialed comes back without dialing") + func idleComesBackNotConnected() { let id = UUID() - let ids = RecoveryConnectionList.connectionIds(from: [ - RecoveryCandidate(connectionId: id, isActivated: true, retainsRestoreIntent: false), - RecoveryCandidate(connectionId: id, isActivated: true, retainsRestoreIntent: false), + let session = Self.session([Self.candidate(id, phase: .idle)]) + + #expect(session.connections == [LastSession.Connection(id: id, reconnects: false)]) + } + + @Test("A connection whose server was unreachable still reconnects") + func failedConnectionReconnects() { + let failed = UUID() + let dropped = UUID() + + let session = Self.session([ + Self.candidate(failed, openedAt: 0, phase: .unavailable(.failed(Self.failure))), + Self.candidate(dropped, openedAt: 1, phase: .unavailable(.disconnected(nil))), ]) - #expect(ids == [id]) + #expect(session.reconnectingIds == [failed, dropped]) } - @Test("An activated window and a still-pending one list the connection once") - func activatedAndPendingListOnce() { + @Test("A connection the user ended never redials, whatever a stale window still reports") + func endedByUserOverridesAStalePhase() { let id = UUID() - let ids = RecoveryConnectionList.connectionIds(from: [ - RecoveryCandidate(connectionId: id, isActivated: true, retainsRestoreIntent: false), - RecoveryCandidate(connectionId: id, isActivated: false, retainsRestoreIntent: true), + let session = Self.session( + [Self.candidate(id, phase: .connected, isActivated: true)], + endedByUser: [id] + ) + + #expect(session.connections == [LastSession.Connection(id: id, reconnects: false)]) + } + + @Test("A connection in a closing window is left out") + func closingWindowIsLeftOut() { + let session = Self.session([Self.candidate(UUID(), phase: .closing, isActivated: true)]) + + #expect(session.connections.isEmpty) + } + + @Test("Connections come back in the order they joined the app") + func orderFollowsOpenedAt() { + let first = UUID() + let second = UUID() + let third = UUID() + + let session = Self.session([ + Self.candidate(third, openedAt: 30, phase: .connected, isActivated: true), + Self.candidate(first, openedAt: 10, phase: .unavailable(.cancelled)), + Self.candidate(second, openedAt: 20, phase: .connecting), ]) - #expect(ids == [id]) + #expect(session.connections.map { $0.id } == [first, second, third]) } - @Test("A connection stays restorable while any of its windows is connected") - func connectionSurvivesWhenOneWindowIsStillConnecting() { - let id = UUID() + @Test("Connections that joined at the same moment keep one order across launches") + func sameOpenedAtOrdersByIdentifier() { + let ids = (0..<4).map { _ in UUID() } + + let session = Self.session(ids.map { Self.candidate($0, phase: .connected, isActivated: true) }) - let ids = RecoveryConnectionList.connectionIds(from: [ - RecoveryCandidate(connectionId: id, isActivated: false, retainsRestoreIntent: false), - RecoveryCandidate(connectionId: id, isActivated: true, retainsRestoreIntent: false), + #expect(session.connections.map { $0.id } == ids.sorted { $0.uuidString < $1.uuidString }) + } + + @Test("A connection hosted by two windows is listed once, at its earliest place") + func connectionInTwoWindowsIsListedOnce() { + let shared = UUID() + let other = UUID() + + let session = Self.session([ + Self.candidate(other, openedAt: 5, phase: .connected, isActivated: true), + Self.candidate(shared, openedAt: 9, phase: .unavailable(.cancelled)), + Self.candidate(shared, openedAt: 1, phase: .connecting), ]) - #expect(ids == [id]) + #expect(session.connections == [ + LastSession.Connection(id: shared, reconnects: true), + LastSession.Connection(id: other, reconnects: true), + ]) } - @Test("No windows means nothing to restore") - func emptyCandidatesRestoreNothing() { - #expect(RecoveryConnectionList.connectionIds(from: []).isEmpty) + @Test("The connection on screen is kept only while it is listed") + func shownConnectionMustBeListed() { + let listed = UUID() + + let kept = Self.session([Self.candidate(listed, phase: .idle)], shown: listed) + let dropped = Self.session([Self.candidate(listed, phase: .idle)], shown: UUID()) + + #expect(kept.shownConnectionId == listed) + #expect(dropped.shownConnectionId == nil) } @Test("Quitting a client-launched session with nothing open keeps the last session") func machineSessionWithNothingOpenKeepsTheList() { - #expect(!RecoveryConnectionList.shouldSaveOnQuit([], origin: .machine)) - #expect(RecoveryConnectionList.shouldSaveOnQuit([UUID()], origin: .machine)) - #expect(RecoveryConnectionList.shouldSaveOnQuit([], origin: .user)) + let one = LastSession(connections: [LastSession.Connection(id: UUID(), reconnects: false)]) + + #expect(!RecoveryConnectionList.shouldSaveOnQuit(.empty, origin: .machine)) + #expect(RecoveryConnectionList.shouldSaveOnQuit(one, origin: .machine)) + #expect(RecoveryConnectionList.shouldSaveOnQuit(.empty, origin: .user)) } } From d7abec2f1e83569cb5823fe66d42116541f7a820 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 20 Oct 2026 21:17:41 +0700 Subject: [PATCH 2/4] docs(changelog): add the #3701 fragments --- apple/changelog.d/3701.fixed.1.md | 1 + apple/changelog.d/3701.fixed.2.md | 1 + apple/changelog.d/3701.fixed.md | 1 + 3 files changed, 3 insertions(+) create mode 100644 apple/changelog.d/3701.fixed.1.md create mode 100644 apple/changelog.d/3701.fixed.2.md create mode 100644 apple/changelog.d/3701.fixed.md diff --git a/apple/changelog.d/3701.fixed.1.md b/apple/changelog.d/3701.fixed.1.md new file mode 100644 index 0000000000..af015d5000 --- /dev/null +++ b/apple/changelog.d/3701.fixed.1.md @@ -0,0 +1 @@ +Reopen Last Session forgot a session whose connections had all failed or were not connected. diff --git a/apple/changelog.d/3701.fixed.2.md b/apple/changelog.d/3701.fixed.2.md new file mode 100644 index 0000000000..1154bf1ed9 --- /dev/null +++ b/apple/changelog.d/3701.fixed.2.md @@ -0,0 +1 @@ +The MCP tools that open a connection or a table timed out on a connection that was open but not connected. diff --git a/apple/changelog.d/3701.fixed.md b/apple/changelog.d/3701.fixed.md new file mode 100644 index 0000000000..4e5fa578a1 --- /dev/null +++ b/apple/changelog.d/3701.fixed.md @@ -0,0 +1 @@ +Reopen Last Session dropped connections that were on the connections strip but not connected. From 1808a396081f24f48bab5cb267dc493393471328 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 20 Oct 2026 21:37:26 +0700 Subject: [PATCH 3/4] fix(connections): leave canceled and disconnected workspaces alone on an MCP open --- .../MainSplitViewController+Connection.swift | 12 +++------ .../Infrastructure/WindowManager.swift | 4 +-- ...ewControllerPaneSynchronizationTests.swift | 25 +++++++++++++++++++ 3 files changed, 31 insertions(+), 10 deletions(-) diff --git a/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+Connection.swift b/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+Connection.swift index b1ae179a59..63478666ea 100644 --- a/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+Connection.swift +++ b/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+Connection.swift @@ -24,14 +24,10 @@ internal extension MainSplitViewController { Logger(subsystem: "com.TablePro", category: "ConnectionWindow") } - /// Every path out of here settles the workspace, because a window that declines to dial and - /// says nothing is a window the resolver can only answer for with a timeout, and a state - /// reached by a timeout is a state nothing transitions into. That used to be the pane - /// resolver's job: it reported nothing for half a second and then fell back to - /// `.notConnected`, which is also what put a blank window on screen for the first half second - /// of every connect. - func startActivationConnectIfNeeded() { - guard autoConnect else { return } + // Every path settles the phase: a workspace that declines to dial silently is one the pane resolver + // could only time out on. `requested` is a caller asking a workspace not opened to dial on its own. + func startActivationConnectIfNeeded(requested: Bool = false) { + guard autoConnect || requested else { return } guard ConnectionWindowPhaseMachine.allowsActivationConnect(phase: phase) else { return } /// A workspace whose record has gone still names a connection through its session, so the /// pane has something to draw and nothing to dial with. diff --git a/apple/TablePro/Core/Services/Infrastructure/WindowManager.swift b/apple/TablePro/Core/Services/Infrastructure/WindowManager.swift index a24d3c3bc0..04b5c64eed 100644 --- a/apple/TablePro/Core/Services/Infrastructure/WindowManager.swift +++ b/apple/TablePro/Core/Services/Infrastructure/WindowManager.swift @@ -35,9 +35,9 @@ internal final class WindowManager { if let existing = host.workspaces.workspace(for: payload.connectionId) { host.workspaces.select(payload.connectionId) existing.open(payload) - // An open that asks to connect dials one sitting not connected, as picking it in the switcher does. + // Dials only where a new window would: never one the user canceled or disconnected. if autoConnect { - host.reconnectWorkspaceIfIdle(payload.connectionId) + host.startActivationConnectIfNeeded(requested: true) } } else { host.adoptWorkspace(payload: payload, autoConnect: autoConnect) diff --git a/apple/TableProTests/Core/Services/Infrastructure/MainSplitViewControllerPaneSynchronizationTests.swift b/apple/TableProTests/Core/Services/Infrastructure/MainSplitViewControllerPaneSynchronizationTests.swift index 5ec64b472a..359a922157 100644 --- a/apple/TableProTests/Core/Services/Infrastructure/MainSplitViewControllerPaneSynchronizationTests.swift +++ b/apple/TableProTests/Core/Services/Infrastructure/MainSplitViewControllerPaneSynchronizationTests.swift @@ -545,6 +545,31 @@ struct MainSplitViewControllerPaneSynchronizationTests { #expect(harness.foreground.resolvedPane == .unavailable(.notConnected)) } + @Test("A connect request leaves a workspace the user canceled alone") + func requestedConnectLeavesACanceledWorkspaceAlone() throws { + let harness = try Harness() + defer { harness.tearDown() } + harness.controller.transition(to: .unavailable(.cancelled), for: harness.foreground.connectionId) + + harness.controller.startActivationConnectIfNeeded(requested: true) + + #expect(harness.foreground.phase == .unavailable(.cancelled)) + #expect(harness.foreground.attemptToken == nil) + } + + @Test("A connect request reaches a reopened workspace that was not opened to dial") + func requestedConnectReachesAnIdleWorkspace() throws { + let harness = try Harness() + defer { harness.tearDown() } + harness.foreground.payloadConnection?.preConnectScript = "true" + + harness.controller.startActivationConnectIfNeeded(requested: true) + + // A script needs consent, so the request settles on Not Connected instead of dialing. + #expect(harness.foreground.phase == .unavailable(.notConnected)) + #expect(harness.foreground.attemptToken == nil) + } + @Test("Reconnecting only when idle shows a connecting connection and leaves its attempt alone") func reconnectIfIdleLeavesAConnectingWorkspaceAlone() throws { let harness = try Harness() From b2a5dfd07804399be30ea3de77f2607d7283daeb Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 20 Oct 2026 21:40:08 +0700 Subject: [PATCH 4/4] docs(changelog): narrow the MCP open fragment --- apple/changelog.d/3701.fixed.2.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apple/changelog.d/3701.fixed.2.md b/apple/changelog.d/3701.fixed.2.md index 1154bf1ed9..6c8147c16b 100644 --- a/apple/changelog.d/3701.fixed.2.md +++ b/apple/changelog.d/3701.fixed.2.md @@ -1 +1 @@ -The MCP tools that open a connection or a table timed out on a connection that was open but not connected. +MCP tools that open a connection or a table timed out on a connection whose window showed it failed or not connected.