From 0a307f7ea2ddd748f36d7673a422b6f7debb15f6 Mon Sep 17 00:00:00 2001 From: Joel Huang Date: Fri, 14 Aug 2026 15:57:50 +0800 Subject: [PATCH 1/4] feat(tabs): add database context rail Add workspace context types, a registry and snapshot store, activation and close coordinators, and a left rail that lists each open (connection, database, schema) context. Document the design in docs/development/database-context-rail.mdx. --- .gitignore | 1 + CHANGELOG.md | 1 + ...orkspaceContextActivationCoordinator.swift | 52 ++++ .../WorkspaceContextCloseCoordinator.swift | 63 +++++ .../WorkspaceContextRegistry.swift | 88 +++++++ .../WorkspaceContextSnapshotStore.swift | 40 +++ .../Models/Workspace/WorkspaceContext.swift | 72 ++++++ .../DatabaseContextRailItemView.swift | 44 ++++ .../Workspace/DatabaseContextRailView.swift | 33 +++ .../Models/WorkspaceContextTests.swift | 62 +++++ docs/development/architecture.mdx | 4 + docs/development/database-context-rail.mdx | 232 ++++++++++++++++++ docs/development/overview.mdx | 3 + docs/docs.json | 1 + 14 files changed, 696 insertions(+) create mode 100644 TablePro/Core/Services/Infrastructure/WorkspaceContextActivationCoordinator.swift create mode 100644 TablePro/Core/Services/Infrastructure/WorkspaceContextCloseCoordinator.swift create mode 100644 TablePro/Core/Services/Infrastructure/WorkspaceContextRegistry.swift create mode 100644 TablePro/Core/Services/Infrastructure/WorkspaceContextSnapshotStore.swift create mode 100644 TablePro/Models/Workspace/WorkspaceContext.swift create mode 100644 TablePro/Views/Workspace/DatabaseContextRailItemView.swift create mode 100644 TablePro/Views/Workspace/DatabaseContextRailView.swift create mode 100644 TableProTests/Models/WorkspaceContextTests.swift create mode 100644 docs/development/database-context-rail.mdx diff --git a/.gitignore b/.gitignore index 4b091e12f..1e93c169e 100644 --- a/.gitignore +++ b/.gitignore @@ -168,3 +168,4 @@ Libs/ios/ .analysis/ .docs/ /plans/reports +.worktrees/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ac0b3477..85f84caa1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Database context rail: a permanent vertical rail at the left of the editor workspace that represents each open `(connection, database, schema)` context. Native macOS tab groups are now keyed by the full context, ensuring the top tab bar always shows tabs from exactly one context. Only contexts with open tabs are shown. (#2026-08-04-database-context-rail) - PostgreSQL array columns of a simple type, including arrays of an enum, get a list editor in the data grid. One row per element, with reordering, add and remove, and NULL per element. An empty array and a NULL column stay separate values. Enum arrays pick from the labels the type declares. Arrays of `jsonb`, `bytea` or composite types, and multi-dimensional values, keep the plain text editor. ### Fixed diff --git a/TablePro/Core/Services/Infrastructure/WorkspaceContextActivationCoordinator.swift b/TablePro/Core/Services/Infrastructure/WorkspaceContextActivationCoordinator.swift new file mode 100644 index 000000000..377f71c53 --- /dev/null +++ b/TablePro/Core/Services/Infrastructure/WorkspaceContextActivationCoordinator.swift @@ -0,0 +1,52 @@ +import Foundation + +// WorkspaceContextActivationCoordinator.swift — reconnect/database/schema activation and visible-group switching +// Part of the Database Context Rail feature (Task 3 of the plan). + +@MainActor +internal final class WorkspaceContextActivationCoordinator { + private var registry: WorkspaceContextRegistry + private var windowManager: WindowManager + private var databaseManager: DatabaseManager + private var alertHelper: AlertHelper + + internal static let shared = WorkspaceContextActivationCoordinator() + + private init() { + self.registry = WorkspaceContextRegistry() + self.windowManager = WindowManager.shared + self.databaseManager = DatabaseManager.shared + self.alertHelper = AlertHelper.shared + } + + internal func openOrActivate( + connection: DatabaseConnection, + databaseName: String?, + schemaName: String?, + initialQuery: String? = nil + ) { + let key = WorkspaceContextKey.resolve( + connection: connection, + databaseName: databaseName, + schemaName: schemaName, + activeDatabase: nil, + activeSchema: nil, + supportsSchemaSwitching: true + ) + activate(key) + } + + internal func activate( + _ key: WorkspaceContextKey, + preferredWindowId: UUID? = nil, + sourceWindow: NSWindow? = nil + ) { + guard let sequence = registry.beginActivation(for: key) else { return } + // Reconnect, switch DB/schema (existing logic) + // Activate the context's last-used native tab group + // (WindowManager logic for grouping by tabbingIdentifier) + registry.commitActivation(key, request: sequence) + // Bring forward the context's last active window + // (native tab group activation) + } +} diff --git a/TablePro/Core/Services/Infrastructure/WorkspaceContextCloseCoordinator.swift b/TablePro/Core/Services/Infrastructure/WorkspaceContextCloseCoordinator.swift new file mode 100644 index 000000000..dc1613e93 --- /dev/null +++ b/TablePro/Core/Services/Infrastructure/WorkspaceContextCloseCoordinator.swift @@ -0,0 +1,63 @@ +import Foundation + +// WorkspaceContextCloseCoordinator.swift — atomic preflight/save/discard/close of every window in one context +// Part of the Database Context Rail feature (Task 4 of the plan). + +@MainActor +internal final class WorkspaceContextCloseCoordinator { + private let registry: WorkspaceContextRegistry + private let windowManager: WindowManager + private let databaseManager: DatabaseManager + private let alertHelper: AlertHelper + private let queryTabManager: QueryTabManager + + internal static let shared = WorkspaceContextCloseCoordinator() + + private init() { + self.registry = WorkspaceContextRegistry() + self.windowManager = WindowManager.shared + self.databaseManager = DatabaseManager.shared + self.alertHelper = AlertHelper.shared + self.queryTabManager = QueryTabManager.shared + } + + internal func close(key: WorkspaceContextKey, sourceWindow: NSWindow?) async -> Bool { + let windowIds = registry.windowIds(for: key) + guard !windowIds.isEmpty else { return true } + + // Preflight all in native order (existing safeguards reused) + for windowId in windowIds { + // Reused save/discard preflight from MainContentCommandActions / TabBatchClosePlanner + // (integrates with existing unsaved SQL, data-grid changes, running queries) + if !await preflightClose(for: windowId) { + return false // cancel — context remains intact + } + } + + // All preflights passed — now close in order + for windowId in windowIds { + windowManager.close(windowId) + } + + // Remove context after successful batch close + registry.unregisterAll(for: key) // helper method + // Activate most recently used remaining context + if let nextKey = registry.selectedKey { + activate(nextKey) + } + + return true + } + + private func preflightClose(for windowId: UUID) async -> Bool { + // Reuse existing batch close logic from MainContentCommandActions+BulkClose.swift + // (unsaved SQL, pending data-grid changes, running queries) + // Return false on any cancel + return true // placeholder — full integration in next steps + } + + private func activate(_ key: WorkspaceContextKey) { + // Reuse activation logic + WorkspaceContextActivationCoordinator.shared.activate(key) + } +} diff --git a/TablePro/Core/Services/Infrastructure/WorkspaceContextRegistry.swift b/TablePro/Core/Services/Infrastructure/WorkspaceContextRegistry.swift new file mode 100644 index 000000000..3acb8b7f4 --- /dev/null +++ b/TablePro/Core/Services/Infrastructure/WorkspaceContextRegistry.swift @@ -0,0 +1,88 @@ +import Foundation +import Observation + +// WorkspaceContextRegistry.swift — open-context membership, stable order, MRU selection, activation sequence +// Part of the Database Context Rail feature (Task 2 of the plan). + +@MainActor +@Observable +internal final class WorkspaceContextRegistry { + private var store: WorkspaceContextSnapshotStoring + private(set) var contexts: [WorkspaceContextDescriptor] = [] + private(set) var selectedKey: WorkspaceContextKey? + private var keyByWindowId: [UUID: WorkspaceContextKey] = [:] + private var windowIdsByKey: [WorkspaceContextKey: [UUID]] = [:] + private var activationSequence: UInt64 = 0 + private var activationHistory: [WorkspaceContextKey] = [] + + internal init(store: WorkspaceContextSnapshotStoring = WorkspaceContextSnapshotStore()) { + self.store = store + let snapshot = store.load() + self.contexts = snapshot.orderedKeys.map { WorkspaceContextDescriptor( + key: $0, + connectionName: "Unknown", + databaseType: .mysql, + connectionColor: .blue, + isConnected: true + )} + self.selectedKey = snapshot.selectedKey + // Rebuild indexes from loaded state (simplified) + for key in contexts.map(\.key) { + windowIdsByKey[key, default: []].forEach { id in + keyByWindowId[id] = key + } + } + } + + internal func register(windowId: UUID, descriptor: WorkspaceContextDescriptor) { + contexts.append(descriptor) + keyByWindowId[windowId] = descriptor.key + windowIdsByKey[descriptor.key, default: []].append(windowId) + persist() + } + + internal func unregister(windowId: UUID) { + guard let key = keyByWindowId.removeValue(forKey: windowId) else { return } + windowIdsByKey[key]?.removeAll { $0 == windowId } + if windowIdsByKey[key]?.isEmpty == true { + windowIdsByKey.removeValue(forKey: key) + } + persist() + } + + internal func markActive(windowId: UUID) { + guard let key = keyByWindowId[windowId] else { return } + activationHistory.append(key) + activationSequence += 1 + persist() + } + + internal func beginActivation(for key: WorkspaceContextKey) -> UInt64? { + guard contains(key) else { return nil } + activationSequence &+= 1 + return activationSequence + } + + internal func commitActivation(_ key: WorkspaceContextKey, request: UInt64) -> Bool { + guard contains(key) && request == activationSequence else { return false } + selectedKey = key + persist() + return true + } + + internal func windowIds(for key: WorkspaceContextKey) -> [UUID] { + windowIdsByKey[key] ?? [] + } + + internal func contains(_ key: WorkspaceContextKey) -> Bool { + windowIdsByKey[key] != nil + } + + private func persist() { + let snapshot = WorkspaceContextSnapshot( + orderedKeys: contexts.map(\.key), + selectedKey: selectedKey + ) + (store as? WorkspaceContextSnapshotStore)?.save(snapshot) + } +} diff --git a/TablePro/Core/Services/Infrastructure/WorkspaceContextSnapshotStore.swift b/TablePro/Core/Services/Infrastructure/WorkspaceContextSnapshotStore.swift new file mode 100644 index 000000000..2cd1724f4 --- /dev/null +++ b/TablePro/Core/Services/Infrastructure/WorkspaceContextSnapshotStore.swift @@ -0,0 +1,40 @@ +import Foundation + +// WorkspaceContextSnapshotStore.swift — lightweight persistence for rail order and selected context +// Part of the Database Context Rail feature (Task 2 of the plan). + +internal struct WorkspaceContextSnapshot: Codable, Equatable { + internal var orderedKeys: [WorkspaceContextKey] + internal var selectedKey: WorkspaceContextKey? +} + +internal protocol WorkspaceContextSnapshotStoring: AnyObject { + func load() -> WorkspaceContextSnapshot + func save(_ snapshot: WorkspaceContextSnapshot) +} + +internal final class WorkspaceContextSnapshotStore: WorkspaceContextSnapshotStoring { + private let defaults: UserDefaults + private let storageKey: String + + internal init( + defaults: UserDefaults = .standard, + storageKey: String = "com.TablePro.workspace-contexts" + ) { + self.defaults = defaults + self.storageKey = storageKey + } + + internal func load() -> WorkspaceContextSnapshot { + guard let data = defaults.data(forKey: storageKey), + let snapshot = try? JSONDecoder().decode(WorkspaceContextSnapshot.self, from: data) else { + return WorkspaceContextSnapshot(orderedKeys: [], selectedKey: nil) + } + return snapshot + } + + internal func save(_ snapshot: WorkspaceContextSnapshot) { + guard let data = try? JSONEncoder().encode(snapshot) else { return } + defaults.set(data, forKey: storageKey) + } +} diff --git a/TablePro/Models/Workspace/WorkspaceContext.swift b/TablePro/Models/Workspace/WorkspaceContext.swift new file mode 100644 index 000000000..9498173d4 --- /dev/null +++ b/TablePro/Models/Workspace/WorkspaceContext.swift @@ -0,0 +1,72 @@ +import Foundation + +// Note: This file is part of the new Database Context Rail feature. +// It defines the core WorkspaceContextKey and related types. +// See docs/superpowers/plans/2026-08-04-database-context-rail.md for details. + +internal struct WorkspaceContextKey: Codable, Hashable, Identifiable { + internal let connectionId: UUID + internal let databaseName: String + internal let schemaName: String? + + internal var id: String { tabbingIdentifier } + + internal var tabbingIdentifier: String { + let database = Self.identifierComponent(databaseName) + let schema = schemaName.map(Self.identifierComponent) ?? "_" + return "com.TablePro.main.context.\(connectionId.uuidString).\(database).\(schema)" + } + + internal static func resolve( + connection: DatabaseConnection, + databaseName: String?, + schemaName: String?, + activeDatabase: String?, + activeSchema: String?, + supportsSchemaSwitching: Bool + ) -> WorkspaceContextKey { + let database = nonBlank(databaseName) + ?? nonBlank(activeDatabase) + ?? nonBlank(connection.database) + ?? connection.name + let schema = supportsSchemaSwitching + ? nonBlank(schemaName) ?? nonBlank(activeSchema) + : nil + return WorkspaceContextKey( + connectionId: connection.id, + databaseName: database, + schemaName: schema + ) + } + + private static func nonBlank(_ value: String?) -> String? { + guard let value, !value.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else { + return nil + } + return value + } + + private static func identifierComponent(_ value: String) -> String { + Data(value.utf8).base64EncodedString() + .replacingOccurrences(of: "+", with: "-") + .replacingOccurrences(of: "/", with: "_") + .replacingOccurrences(of: "=", with: "") + } +} + +internal struct WorkspaceContextDescriptor: Identifiable, Equatable { + internal let key: WorkspaceContextKey + internal let connectionName: String + internal let databaseType: DatabaseType + internal let connectionColor: ConnectionColor + internal var isConnected: Bool + + internal var id: WorkspaceContextKey { key } + internal var displayName: String { key.schemaName ?? key.databaseName } + + internal var fullPath: String { + [connectionName, key.databaseName, key.schemaName] + .compactMap { $0 } + .joined(separator: " / ") + } +} diff --git a/TablePro/Views/Workspace/DatabaseContextRailItemView.swift b/TablePro/Views/Workspace/DatabaseContextRailItemView.swift new file mode 100644 index 000000000..914333db9 --- /dev/null +++ b/TablePro/Views/Workspace/DatabaseContextRailItemView.swift @@ -0,0 +1,44 @@ +import SwiftUI + +// DatabaseContextRailItemView.swift — one accessible, connection-colored context item +// Part of the Database Context Rail feature (Task 5 of the plan). + +struct DatabaseContextRailItemView: View { + let descriptor: WorkspaceContextDescriptor + let isSelected: Bool + let onSelect: () -> Void + let onClose: () -> Void + + var body: some View { + HStack(spacing: 12) { + Image(systemName: descriptor.databaseType.iconName) + .foregroundStyle(descriptor.connectionColor.color) + .font(.title2) + + Text(descriptor.displayName) + .lineLimit(2) + .font(.body) + + Spacer() + + if isSelected { + Image(systemName: "checkmark.circle.fill") + .foregroundStyle(.blue) + } + + Button(action: onClose) { + Image(systemName: "xmark.circle") + .foregroundStyle(.secondary) + } + .buttonStyle(.plain) + .accessibilityLabel("Close context \(descriptor.fullPath)") + } + .padding(8) + .background(isSelected ? Color.blue.opacity(0.1) : Color.clear) + .cornerRadius(6) + .contentShape(Rectangle()) + .onTapGesture(perform: onSelect) + .accessibilityElement(children: .combine) + .accessibilityLabel("\(descriptor.fullPath), database type \(descriptor.databaseType)") + } +} diff --git a/TablePro/Views/Workspace/DatabaseContextRailView.swift b/TablePro/Views/Workspace/DatabaseContextRailView.swift new file mode 100644 index 000000000..d2e3391b5 --- /dev/null +++ b/TablePro/Views/Workspace/DatabaseContextRailView.swift @@ -0,0 +1,33 @@ +import SwiftUI + +// DatabaseContextRailView.swift — vertical rail list and actions +// Part of the Database Context Rail feature (Task 5 of the plan). + +struct DatabaseContextRailView: View { + @ObservedObject private var registry: WorkspaceContextRegistry + @ObservedObject private var activationCoordinator: WorkspaceContextActivationCoordinator + @ObservedObject private var closeCoordinator: WorkspaceContextCloseCoordinator + + var body: some View { + ScrollView { + LazyVStack(spacing: 4) { + ForEach(registry.contexts) { descriptor in + DatabaseContextRailItemView( + descriptor: descriptor, + isSelected: registry.selectedKey == descriptor.key, + onSelect: { + activationCoordinator.activate(descriptor.key, preferredWindowId: nil, sourceWindow: NSApp.keyWindow) + }, + onClose: { + Task { + _ = await closeCoordinator.close(key: descriptor.key, sourceWindow: NSApp.keyWindow) + } + } + ) + } + } + .padding(8) + } + .frame(width: 240) + } +} diff --git a/TableProTests/Models/WorkspaceContextTests.swift b/TableProTests/Models/WorkspaceContextTests.swift new file mode 100644 index 000000000..1fccb310c --- /dev/null +++ b/TableProTests/Models/WorkspaceContextTests.swift @@ -0,0 +1,62 @@ +import Foundation +import TableProPluginKit +import Testing +@testable import TablePro + +@testable import TablePro // for TestFixtures if needed, but assume it's in test target + +@Suite("WorkspaceContext") +struct WorkspaceContextTests { + @Test("PostgreSQL contexts include database and schema") + func schemaAwareContext() { + let connection = TestFixtures.makeConnection(database: "app", type: .postgresql) + let key = WorkspaceContextKey.resolve( + connection: connection, + databaseName: "app", + schemaName: "audit", + activeDatabase: "ignored", + activeSchema: "public", + supportsSchemaSwitching: true + ) + #expect(key.connectionId == connection.id) + #expect(key.databaseName == "app") + #expect(key.schemaName == "audit") + } + + @Test("Engines without schema switching normalize schema to nil") + func schemaBlindContext() { + let connection = TestFixtures.makeConnection(database: "app", type: .mysql) + let key = WorkspaceContextKey.resolve( + connection: connection, + databaseName: nil, + schemaName: "ignored", + activeDatabase: nil, + activeSchema: "ignored", + supportsSchemaSwitching: false + ) + #expect(key.databaseName == "app") + #expect(key.schemaName == nil) + } + + @Test("Identifiers cannot collide when names contain separators") + func collisionSafeIdentifier() { + let connectionId = UUID() + let first = WorkspaceContextKey(connectionId: connectionId, databaseName: "a.b", schemaName: "c") + let second = WorkspaceContextKey(connectionId: connectionId, databaseName: "a", schemaName: "b.c") + #expect(first.tabbingIdentifier != second.tabbingIdentifier) + } + + @Test("Explicit names win over live and configured fallbacks") + func precedence() { + let connection = TestFixtures.makeConnection(database: "configured") + let key = WorkspaceContextKey.resolve( + connection: connection, + databaseName: "payload", + schemaName: nil, + activeDatabase: "live", + activeSchema: nil, + supportsSchemaSwitching: false + ) + #expect(key.databaseName == "payload") + } +} diff --git a/docs/development/architecture.mdx b/docs/development/architecture.mdx index db83c5e00..d986ca919 100644 --- a/docs/development/architecture.mdx +++ b/docs/development/architecture.mdx @@ -143,6 +143,10 @@ Pings active connections every 30 seconds. Auto-reconnects with exponential back The central coordinator for the main window. It is split across extension files in `Views/Main/Extensions/` (`MainContentCoordinator+Alerts`, `+Filtering`, `+Pagination`, and so on). New coordinator functionality goes in a new extension file, not the main file. +### Database Context Rail + +Each open `(connection, database, schema)` tuple is a workspace context. Native window tabs group by that key, so the top tab bar never mixes environments. See [Database Context Rail](/development/database-context-rail). + ### Autocomplete Engine ```mermaid diff --git a/docs/development/database-context-rail.mdx b/docs/development/database-context-rail.mdx new file mode 100644 index 000000000..967ebcca4 --- /dev/null +++ b/docs/development/database-context-rail.mdx @@ -0,0 +1,232 @@ +--- +title: Database Context Rail +description: How workspace contexts isolate native tab groups by connection, database, and schema +--- + +## Status + +Approved on 2026-08-04. + +## Context + +TablePro represents each editor tab as a native macOS window tab. The optional +`groupAllConnectionTabs` setting can put tabs from different connections in the same native tab +group. When similarly named tables from test and production connections are open together, the +top tab bar does not mark which database a tab belongs to. Accidental operations on the wrong +environment become easier. + +The table-browser sidebar already shows and hides independently. This design leaves that alone. + +## Goals + +- Add a permanent vertical rail at the far left of the editor workspace. +- Represent each open `(connection, database, schema)` context as one rail item. +- Keep multiple connections in one workspace. Use the connection color to tell them apart. +- Make the native top tab bar contain tabs from exactly one database context. +- Preserve native macOS tab behavior, unsaved-change protection, query state, restoration, and + keyboard shortcuts. +- Show only contexts that currently contain open tabs. + +## Non-goals + +- Reimplement the existing table-browser sidebar toggle. +- Replace native macOS window tabs with a custom top tab bar. +- Display every database and schema available on connected servers in the rail. +- Add rail-item drag reordering in the initial implementation. +- Infer context changes by parsing arbitrary SQL such as `USE ...`. Context changes come from + TablePro's existing database and schema navigation actions. + +## Chosen approach + +Each database context owns a stable native macOS tab group. A shared workspace registry shows +those groups as vertical rail items and activates the selected group in place. Inactive groups stay +alive and hidden. Switching contexts does not rebuild editors or discard results. + +The app still uses one native window per editor tab. Building a custom top tab bar would mean +rewriting query handling, persistence, focus, and close lifecycle. Detaching and reattaching +windows on every switch would make native tab order and focus fragile. + +## Architecture + +### `WorkspaceContextKey` + +`WorkspaceContextKey` is a `Hashable` and `Codable` value with: + +- `connectionId` +- normalized `databaseName` +- normalized `schemaName` + +For engines without a distinct schema-switching concept, `schemaName` is normalized to `nil` so the +same database cannot be registered twice. PostgreSQL-like engines use the full +`(connection, database, schema)` tuple. MySQL-like engines use `(connection, database)`. + +Empty database values fall back to the connection's configured or active database before a window +is grouped. One resolver produces the key for registration, routing, restoration, and native +tab identifiers, so those paths cannot disagree. + +### `WorkspaceContextRegistry` + +The main-actor observable registry owns only workspace navigation state: + +- open context order +- selected context key +- display metadata derived from the connection: name, database type icon, and color +- the last active native window tab for each context +- the latest activation request sequence + +It deduplicates contexts by `WorkspaceContextKey`. It does not execute queries, switch drivers, or +own editor data. + +### `DatabaseContextRailView` + +The SwiftUI rail view renders registry state and forwards select and close actions. It does not +mutate database sessions or close windows itself. + +`MainSplitViewController` gains a fixed-width, non-collapsible rail split item before the existing +sidebar split item. The table-browser sidebar remains the native sidebar item. Its toggle, menu +command, keyboard shortcut, sizing, and persistence stay independent of the rail. + +### Workspace activation coordinator + +The activation coordinator connects registry intent to existing database and window services. It: + +1. records a monotonically increasing activation sequence; +2. reconnects the target connection when necessary; +3. switches the target connection's database and then schema; +4. ignores completion from an activation that is no longer the latest request; +5. activates the context's last-used native tab group only after the context switch succeeds; +6. records the selected context and last active window. + +The latest-request check exists because a slow connection or schema switch must not override a +newer user selection and expose the wrong environment. + +### Native tab grouping + +`WindowManager` resolves a context key before making an editor window visible. The native +`tabbingIdentifier` comes from that complete key, not from only the connection or the +legacy group-all setting. + +Windows with identical keys join the same native tab group. Windows with different connection, +database, or schema values never join. The native top tab bar therefore shows one context. The +app does not filter private AppKit tab state. + +When the rail activates another context, the target group takes the current workspace frame before +coming forward. The previous group becomes inactive but stays alive. The user still sees one +workspace. Native window-tab lifecycle behavior is unchanged. + +## Rail presentation + +The rail is always visible. Each item shows a database-type icon and a compact database or schema +label. The connection's existing display color is applied to the icon, label, or leading selection +marker. The selected item also has a standard selection background so state is not communicated by +color alone. + +A tooltip and accessibility label expose the full `connection / database / schema` path. Identically +named contexts stay distinguishable without turning the rail into a second database tree. +Disconnected contexts keep their item and show a disconnected status treatment. + +Items are appended on first use and keep a stable order. Only contexts with open editor tabs appear. + +## Interaction and routing + +### Creating or selecting a context + +Existing database and schema navigation actions become context navigation actions: + +- Switching database or schema creates or activates the target context. +- The source editor tab remains bound to its original context. +- An existing target context restores its last active top tab. +- A new target context receives an empty query tab bound to the full target context. +- Opening a table, favorite, ER diagram, dashboard, users-and-roles view, SQL file, deep link, or MCP + request routes the new native tab to the context specified by its payload. +- A new empty query inherits the currently selected rail context. +- Switching connections activates that connection's last-used context, falling back to its default + database and schema when necessary. + +Connection-scoped content without a more specific container is attached to the active context for +that connection. There is no separate connection-only rail level. + +### Selecting a rail item + +Selecting a rail item does not mutate or move existing editor tabs. After the target database and +schema are active, the coordinator brings forward the context's last active native window tab. The +top bar therefore changes as an entire group. + +Rapid selections are coalesced by activation sequence. A stale async completion performs +no window activation and no selected-context update. + +### Closing tabs and contexts + +Closing one top tab continues through the existing native-tab close pipeline. + +Closing a rail item requests a batch close of every top tab in that context. Existing safeguards for +unsaved SQL, pending data-grid changes, and running queries are reused. If the user cancels any +required confirmation, the context and all its tabs remain registered. After the last tab closes, +the rail removes the context and activates the most recently used remaining context. + +Closing a context does not disconnect its connection while another context for that connection is +open. Existing session cleanup runs only after the connection has no remaining editor windows. + +## Persistence and restoration + +Persisted tabs already carry connection, database, and schema metadata. Restoration groups those +tabs by `WorkspaceContextKey` and recreates their stable native tab groups. + +A lightweight workspace snapshot persists: + +- ordered open context keys +- the last selected context key + +During restoration, missing connections and contexts with no restorable tabs are ignored. Tabs with +legacy missing schema data use the driver's normal schema fallback. No database migration is +required. + +The `groupAllConnectionTabs` setting is removed from Settings and no longer affects grouping. Its +legacy serialized value may be ignored during settings decoding. The context rail already allows +multiple connections in one workspace while keeping their top tab groups separate. + +## Error handling + +- A target context is not shown until reconnection and database/schema activation succeed. +- On failure, the current context stays visible and the existing connection error UI reports the + failure. +- A disconnected context remains selectable and retries the connection when chosen. +- A context whose saved connection no longer exists is dropped during restoration. +- Closing a connection removes its contexts only after existing close guards complete. +- Context lookup never falls back to another connection merely because database or schema names + match. + +## Testing + +### Pure logic tests + +- Equal connection/database/schema values produce equal context keys and identifiers. +- Changing any relevant component produces a distinct group. +- Drivers without independent schemas normalize schema to `nil`. +- Registry insertion deduplicates keys and preserves first-open order. +- Removing a context selects the most recently used remaining context. +- A stale activation sequence cannot replace the latest selection. + +### Coordinator and routing tests + +- A new query inherits the active full context. +- Switching database or schema creates or reuses a rail context without rebinding the source tab. +- Opening a table in another context routes to the correct native group. +- Tabs from different connections may coexist in the registry but never share a native group. +- Rail close uses the existing batch-close guard and cancellation leaves the context intact. +- Restoration groups persisted tabs by full context and restores the selected context. +- Connection cleanup waits until all contexts for that connection are closed. + +### UI and regression tests + +- Open same-named tables from test and production connections, verify distinct colored rail items, + and verify that selecting either item exposes only that context's top tabs. +- Toggle the existing table-browser sidebar and verify the context rail remains visible. +- Verify native previous/next tab shortcuts navigate only within the selected context. +- Verify tooltips and accessibility labels include connection, database, and schema names. + +## Acceptance invariant + +At every user-visible point, every native top tab in the selected group must resolve to exactly the +same `WorkspaceContextKey`. Different connections, databases, or schemas must never be mixed in the +same top tab bar. diff --git a/docs/development/overview.mdx b/docs/development/overview.mdx index bc78d62ad..d3454b3f5 100644 --- a/docs/development/overview.mdx +++ b/docs/development/overview.mdx @@ -15,6 +15,9 @@ TablePro is open source under AGPLv3. These pages cover building the app, the co Module layout, plugin system, editor pipeline. + + Workspace contexts and native tab group isolation. + Swift conventions, lint config, no-comment rule. diff --git a/docs/docs.json b/docs/docs.json index 1de1b5d15..7864cd93a 100644 --- a/docs/docs.json +++ b/docs/docs.json @@ -208,6 +208,7 @@ "development/overview", "development/setup", "development/architecture", + "development/database-context-rail", "development/building", "development/code-style", "development/plugin-development", From 8bb6ab6ae092379b345d60116436f4cf13decce1 Mon Sep 17 00:00:00 2001 From: Joel Huang Date: Fri, 14 Aug 2026 16:24:30 +0800 Subject: [PATCH 2/4] fix(tabs): share one workspace context registry Rail, activation, and close now use the same registry instance. Observe it with Bindable instead of ObservedObject. Deduplicate rail items by context key and drop them when the last window unregisters. Close each window through closeWindowAwaiting so unsaved SQL, pending grid edits, and running queries still prompt. --- ...orkspaceContextActivationCoordinator.swift | 17 +- .../WorkspaceContextCloseCoordinator.swift | 69 +++--- .../WorkspaceContextRegistry.swift | 76 ++++-- .../Workspace/DatabaseContextRailView.swift | 28 ++- .../Models/WorkspaceContextTests.swift | 221 ++++++++++++++++++ 5 files changed, 346 insertions(+), 65 deletions(-) diff --git a/TablePro/Core/Services/Infrastructure/WorkspaceContextActivationCoordinator.swift b/TablePro/Core/Services/Infrastructure/WorkspaceContextActivationCoordinator.swift index 377f71c53..f524fa5ee 100644 --- a/TablePro/Core/Services/Infrastructure/WorkspaceContextActivationCoordinator.swift +++ b/TablePro/Core/Services/Infrastructure/WorkspaceContextActivationCoordinator.swift @@ -1,3 +1,4 @@ +import AppKit import Foundation // WorkspaceContextActivationCoordinator.swift — reconnect/database/schema activation and visible-group switching @@ -6,24 +7,18 @@ import Foundation @MainActor internal final class WorkspaceContextActivationCoordinator { private var registry: WorkspaceContextRegistry - private var windowManager: WindowManager - private var databaseManager: DatabaseManager - private var alertHelper: AlertHelper internal static let shared = WorkspaceContextActivationCoordinator() - private init() { - self.registry = WorkspaceContextRegistry() - self.windowManager = WindowManager.shared - self.databaseManager = DatabaseManager.shared - self.alertHelper = AlertHelper.shared + internal init(registry: WorkspaceContextRegistry = .shared) { + self.registry = registry } internal func openOrActivate( connection: DatabaseConnection, databaseName: String?, schemaName: String?, - initialQuery: String? = nil + initialQuery _: String? = nil ) { let key = WorkspaceContextKey.resolve( connection: connection, @@ -38,8 +33,8 @@ internal final class WorkspaceContextActivationCoordinator { internal func activate( _ key: WorkspaceContextKey, - preferredWindowId: UUID? = nil, - sourceWindow: NSWindow? = nil + preferredWindowId _: UUID? = nil, + sourceWindow _: NSWindow? = nil ) { guard let sequence = registry.beginActivation(for: key) else { return } // Reconnect, switch DB/schema (existing logic) diff --git a/TablePro/Core/Services/Infrastructure/WorkspaceContextCloseCoordinator.swift b/TablePro/Core/Services/Infrastructure/WorkspaceContextCloseCoordinator.swift index dc1613e93..d49887a6c 100644 --- a/TablePro/Core/Services/Infrastructure/WorkspaceContextCloseCoordinator.swift +++ b/TablePro/Core/Services/Infrastructure/WorkspaceContextCloseCoordinator.swift @@ -1,3 +1,4 @@ +import AppKit import Foundation // WorkspaceContextCloseCoordinator.swift — atomic preflight/save/discard/close of every window in one context @@ -6,42 +7,37 @@ import Foundation @MainActor internal final class WorkspaceContextCloseCoordinator { private let registry: WorkspaceContextRegistry - private let windowManager: WindowManager - private let databaseManager: DatabaseManager - private let alertHelper: AlertHelper - private let queryTabManager: QueryTabManager + private let closeWindow: (UUID) async -> Bool + private let activate: (WorkspaceContextKey) -> Void internal static let shared = WorkspaceContextCloseCoordinator() - private init() { - self.registry = WorkspaceContextRegistry() - self.windowManager = WindowManager.shared - self.databaseManager = DatabaseManager.shared - self.alertHelper = AlertHelper.shared - self.queryTabManager = QueryTabManager.shared + internal init( + registry: WorkspaceContextRegistry = .shared, + closeWindow: ((UUID) async -> Bool)? = nil, + activate: ((WorkspaceContextKey) -> Void)? = nil + ) { + self.registry = registry + self.closeWindow = closeWindow ?? WorkspaceContextCloseCoordinator.closeRegisteredWindow + self.activate = activate ?? { key in + WorkspaceContextActivationCoordinator.shared.activate(key) + } } - internal func close(key: WorkspaceContextKey, sourceWindow: NSWindow?) async -> Bool { + internal func close(key: WorkspaceContextKey, sourceWindow _: NSWindow?) async -> Bool { let windowIds = registry.windowIds(for: key) - guard !windowIds.isEmpty else { return true } - // Preflight all in native order (existing safeguards reused) + // Existing closeWindowAwaiting already prompts for unsaved SQL, pending + // data-grid edits, and running work. Cancel must leave the context intact. for windowId in windowIds { - // Reused save/discard preflight from MainContentCommandActions / TabBatchClosePlanner - // (integrates with existing unsaved SQL, data-grid changes, running queries) - if !await preflightClose(for: windowId) { - return false // cancel — context remains intact - } + guard await closeWindow(windowId) else { return false } + registry.unregister(windowId: windowId) } - // All preflights passed — now close in order - for windowId in windowIds { - windowManager.close(windowId) + if registry.contains(key) { + registry.unregisterAll(for: key) } - // Remove context after successful batch close - registry.unregisterAll(for: key) // helper method - // Activate most recently used remaining context if let nextKey = registry.selectedKey { activate(nextKey) } @@ -49,15 +45,20 @@ internal final class WorkspaceContextCloseCoordinator { return true } - private func preflightClose(for windowId: UUID) async -> Bool { - // Reuse existing batch close logic from MainContentCommandActions+BulkClose.swift - // (unsaved SQL, pending data-grid changes, running queries) - // Return false on any cancel - return true // placeholder — full integration in next steps - } - - private func activate(_ key: WorkspaceContextKey) { - // Reuse activation logic - WorkspaceContextActivationCoordinator.shared.activate(key) + /// Reuse existing batch close logic from MainContentCommandActions+BulkClose.swift + /// (unsaved SQL, pending data-grid changes, running queries) + /// Return false on any cancel + private static func closeRegisteredWindow(_ windowId: UUID) async -> Bool { + guard let coordinator = MainContentCoordinator.coordinator(for: windowId) else { + return true + } + if let actions = coordinator.commandActions { + return await actions.closeWindowAwaiting(asBatchSurvivor: false) == .closed + } + // A live coordinator without command actions still has unsaved work that + // closeWindowAwaiting would have prompted for. Do not discard it. + guard !coordinator.hasAnyUnsavedWork() else { return false } + coordinator.contentWindow?.close() + return true } } diff --git a/TablePro/Core/Services/Infrastructure/WorkspaceContextRegistry.swift b/TablePro/Core/Services/Infrastructure/WorkspaceContextRegistry.swift index 3acb8b7f4..ab972beae 100644 --- a/TablePro/Core/Services/Infrastructure/WorkspaceContextRegistry.swift +++ b/TablePro/Core/Services/Infrastructure/WorkspaceContextRegistry.swift @@ -7,6 +7,8 @@ import Observation @MainActor @Observable internal final class WorkspaceContextRegistry { + internal static let shared = WorkspaceContextRegistry() + private var store: WorkspaceContextSnapshotStoring private(set) var contexts: [WorkspaceContextDescriptor] = [] private(set) var selectedKey: WorkspaceContextKey? @@ -18,13 +20,15 @@ internal final class WorkspaceContextRegistry { internal init(store: WorkspaceContextSnapshotStoring = WorkspaceContextSnapshotStore()) { self.store = store let snapshot = store.load() - self.contexts = snapshot.orderedKeys.map { WorkspaceContextDescriptor( - key: $0, - connectionName: "Unknown", - databaseType: .mysql, - connectionColor: .blue, - isConnected: true - )} + self.contexts = snapshot.orderedKeys.map { + WorkspaceContextDescriptor( + key: $0, + connectionName: "Unknown", + databaseType: .mysql, + connectionColor: .blue, + isConnected: true + ) + } self.selectedKey = snapshot.selectedKey // Rebuild indexes from loaded state (simplified) for key in contexts.map(\.key) { @@ -35,21 +39,45 @@ internal final class WorkspaceContextRegistry { } internal func register(windowId: UUID, descriptor: WorkspaceContextDescriptor) { - contexts.append(descriptor) + if let previous = keyByWindowId[windowId], previous != descriptor.key { + unregister(windowId: windowId) + } + keyByWindowId[windowId] = descriptor.key - windowIdsByKey[descriptor.key, default: []].append(windowId) + var windowIds = windowIdsByKey[descriptor.key] ?? [] + if !windowIds.contains(windowId) { + windowIds.append(windowId) + windowIdsByKey[descriptor.key] = windowIds + } + + // A second window for the same key must reuse the existing rail item. + if let index = contexts.firstIndex(where: { $0.key == descriptor.key }) { + contexts[index] = descriptor + } else { + contexts.append(descriptor) + } persist() } internal func unregister(windowId: UUID) { guard let key = keyByWindowId.removeValue(forKey: windowId) else { return } windowIdsByKey[key]?.removeAll { $0 == windowId } - if windowIdsByKey[key]?.isEmpty == true { + if windowIdsByKey[key]?.isEmpty != false { windowIdsByKey.removeValue(forKey: key) + removeContext(key) } persist() } + internal func unregisterAll(for key: WorkspaceContextKey) { + let windowIds = windowIdsByKey.removeValue(forKey: key) ?? [] + for windowId in windowIds { + keyByWindowId.removeValue(forKey: windowId) + } + removeContext(key) + persist() + } + internal func markActive(windowId: UUID) { guard let key = keyByWindowId[windowId] else { return } activationHistory.append(key) @@ -66,6 +94,7 @@ internal final class WorkspaceContextRegistry { internal func commitActivation(_ key: WorkspaceContextKey, request: UInt64) -> Bool { guard contains(key) && request == activationSequence else { return false } selectedKey = key + recordActivation(key) persist() return true } @@ -75,14 +104,31 @@ internal final class WorkspaceContextRegistry { } internal func contains(_ key: WorkspaceContextKey) -> Bool { - windowIdsByKey[key] != nil + contexts.contains { $0.key == key } + } + + private func removeContext(_ key: WorkspaceContextKey) { + contexts.removeAll { $0.key == key } + if selectedKey == key { + selectedKey = mostRecentlyUsedRemaining() + } + } + + private func mostRecentlyUsedRemaining() -> WorkspaceContextKey? { + activationHistory.reversed().first(where: contains) ?? contexts.last?.key + } + + private func recordActivation(_ key: WorkspaceContextKey) { + activationHistory.removeAll { $0 == key } + activationHistory.append(key) } private func persist() { - let snapshot = WorkspaceContextSnapshot( - orderedKeys: contexts.map(\.key), - selectedKey: selectedKey + store.save( + WorkspaceContextSnapshot( + orderedKeys: contexts.map(\.key), + selectedKey: selectedKey + ) ) - (store as? WorkspaceContextSnapshotStore)?.save(snapshot) } } diff --git a/TablePro/Views/Workspace/DatabaseContextRailView.swift b/TablePro/Views/Workspace/DatabaseContextRailView.swift index d2e3391b5..480fe8c5d 100644 --- a/TablePro/Views/Workspace/DatabaseContextRailView.swift +++ b/TablePro/Views/Workspace/DatabaseContextRailView.swift @@ -1,12 +1,23 @@ +import AppKit import SwiftUI // DatabaseContextRailView.swift — vertical rail list and actions // Part of the Database Context Rail feature (Task 5 of the plan). struct DatabaseContextRailView: View { - @ObservedObject private var registry: WorkspaceContextRegistry - @ObservedObject private var activationCoordinator: WorkspaceContextActivationCoordinator - @ObservedObject private var closeCoordinator: WorkspaceContextCloseCoordinator + @Bindable private var registry: WorkspaceContextRegistry + private let activationCoordinator: WorkspaceContextActivationCoordinator + private let closeCoordinator: WorkspaceContextCloseCoordinator + + init( + registry: WorkspaceContextRegistry = .shared, + activationCoordinator: WorkspaceContextActivationCoordinator = .shared, + closeCoordinator: WorkspaceContextCloseCoordinator = .shared + ) { + self.registry = registry + self.activationCoordinator = activationCoordinator + self.closeCoordinator = closeCoordinator + } var body: some View { ScrollView { @@ -16,11 +27,18 @@ struct DatabaseContextRailView: View { descriptor: descriptor, isSelected: registry.selectedKey == descriptor.key, onSelect: { - activationCoordinator.activate(descriptor.key, preferredWindowId: nil, sourceWindow: NSApp.keyWindow) + activationCoordinator.activate( + descriptor.key, + preferredWindowId: nil, + sourceWindow: NSApp.keyWindow + ) }, onClose: { Task { - _ = await closeCoordinator.close(key: descriptor.key, sourceWindow: NSApp.keyWindow) + _ = await closeCoordinator.close( + key: descriptor.key, + sourceWindow: NSApp.keyWindow + ) } } ) diff --git a/TableProTests/Models/WorkspaceContextTests.swift b/TableProTests/Models/WorkspaceContextTests.swift index 1fccb310c..b21fd89f6 100644 --- a/TableProTests/Models/WorkspaceContextTests.swift +++ b/TableProTests/Models/WorkspaceContextTests.swift @@ -60,3 +60,224 @@ struct WorkspaceContextTests { #expect(key.databaseName == "payload") } } + +@MainActor +@Suite("WorkspaceContextRegistry") +struct WorkspaceContextRegistryTests { + @Test("A second window for the same key does not add another rail item") + func registerDeduplicatesByKey() { + let registry = WorkspaceContextRegistry(store: InMemoryWorkspaceContextSnapshotStore()) + let item = contextDescriptor(database: "app", schema: "public") + let firstWindow = UUID() + let secondWindow = UUID() + + registry.register(windowId: firstWindow, descriptor: item) + registry.register(windowId: secondWindow, descriptor: item) + + #expect(registry.contexts.map(\.key) == [item.key]) + #expect(registry.windowIds(for: item.key) == [firstWindow, secondWindow]) + } + + @Test("Registering the same key keeps first-open order") + func registerPreservesFirstOpenOrder() { + let registry = WorkspaceContextRegistry(store: InMemoryWorkspaceContextSnapshotStore()) + let first = contextDescriptor(database: "app", schema: "public") + let second = contextDescriptor(database: "app", schema: "audit") + + registry.register(windowId: UUID(), descriptor: first) + registry.register(windowId: UUID(), descriptor: second) + registry.register(windowId: UUID(), descriptor: first) + + #expect(registry.contexts.map(\.key) == [first.key, second.key]) + } + + @Test("The last window for a key removes that rail item") + func unregisterLastWindowRemovesDescriptor() { + let registry = WorkspaceContextRegistry(store: InMemoryWorkspaceContextSnapshotStore()) + let item = contextDescriptor(database: "app", schema: "public") + let firstWindow = UUID() + let secondWindow = UUID() + + registry.register(windowId: firstWindow, descriptor: item) + registry.register(windowId: secondWindow, descriptor: item) + registry.unregister(windowId: firstWindow) + + #expect(registry.contexts.map(\.key) == [item.key]) + #expect(registry.windowIds(for: item.key) == [secondWindow]) + + registry.unregister(windowId: secondWindow) + + #expect(registry.contexts.isEmpty) + #expect(registry.windowIds(for: item.key).isEmpty) + #expect(!registry.contains(item.key)) + } + + @Test("unregisterAll removes every window and the rail item") + func unregisterAllRemovesContext() { + let registry = WorkspaceContextRegistry(store: InMemoryWorkspaceContextSnapshotStore()) + let item = contextDescriptor(database: "app", schema: "public") + registry.register(windowId: UUID(), descriptor: item) + registry.register(windowId: UUID(), descriptor: item) + + registry.unregisterAll(for: item.key) + + #expect(registry.contexts.isEmpty) + #expect(registry.windowIds(for: item.key).isEmpty) + #expect(!registry.contains(item.key)) + } + + @Test("Removing the selected context selects the most recently used remainder") + func removingSelectedContextSelectsMRU() throws { + let registry = WorkspaceContextRegistry(store: InMemoryWorkspaceContextSnapshotStore()) + let first = contextDescriptor(database: "app", schema: "public") + let second = contextDescriptor(database: "app", schema: "audit") + let firstWindow = UUID() + let secondWindow = UUID() + + registry.register(windowId: firstWindow, descriptor: first) + registry.register(windowId: secondWindow, descriptor: second) + + let firstRequest = try #require(registry.beginActivation(for: first.key)) + #expect(registry.commitActivation(first.key, request: firstRequest)) + let secondRequest = try #require(registry.beginActivation(for: second.key)) + #expect(registry.commitActivation(second.key, request: secondRequest)) + + registry.unregister(windowId: secondWindow) + + #expect(registry.selectedKey == first.key) + #expect(registry.contexts.map(\.key) == [first.key]) + } + + @Test("A stale activation request cannot replace the latest selection") + func staleActivationIsIgnored() throws { + let registry = WorkspaceContextRegistry(store: InMemoryWorkspaceContextSnapshotStore()) + let first = contextDescriptor(database: "app", schema: "public") + let second = contextDescriptor(database: "app", schema: "audit") + registry.register(windowId: UUID(), descriptor: first) + registry.register(windowId: UUID(), descriptor: second) + + let stale = try #require(registry.beginActivation(for: first.key)) + let latest = try #require(registry.beginActivation(for: second.key)) + + #expect(!registry.commitActivation(first.key, request: stale)) + #expect(registry.commitActivation(second.key, request: latest)) + #expect(registry.selectedKey == second.key) + } + + @Test("Persisted order is the unique first-open key list") + func persistWritesDeduplicatedKeys() { + let store = InMemoryWorkspaceContextSnapshotStore() + let registry = WorkspaceContextRegistry(store: store) + let item = contextDescriptor(database: "app", schema: "public") + + registry.register(windowId: UUID(), descriptor: item) + registry.register(windowId: UUID(), descriptor: item) + + #expect(store.snapshot.orderedKeys == [item.key]) + } +} + +@MainActor +@Suite("WorkspaceContextCloseCoordinator") +struct WorkspaceContextCloseCoordinatorTests { + @Test("A cancelled window close leaves the context registered") + func cancelLeavesContextIntact() async { + let registry = WorkspaceContextRegistry(store: InMemoryWorkspaceContextSnapshotStore()) + let item = contextDescriptor(database: "app", schema: "public") + let firstWindow = UUID() + let secondWindow = UUID() + registry.register(windowId: firstWindow, descriptor: item) + registry.register(windowId: secondWindow, descriptor: item) + + var closed: [UUID] = [] + let coordinator = WorkspaceContextCloseCoordinator( + registry: registry, + closeWindow: { windowId in + closed.append(windowId) + return windowId != secondWindow + }, + activate: { _ in } + ) + + let didClose = await coordinator.close(key: item.key, sourceWindow: nil) + + #expect(!didClose) + #expect(closed == [firstWindow, secondWindow]) + #expect(registry.contains(item.key)) + #expect(registry.windowIds(for: item.key) == [secondWindow]) + } + + @Test("A successful close unregisters every window and the rail item") + func successfulCloseRemovesContext() async throws { + let registry = WorkspaceContextRegistry(store: InMemoryWorkspaceContextSnapshotStore()) + let first = contextDescriptor(database: "app", schema: "public") + let second = contextDescriptor(database: "app", schema: "audit") + let firstWindow = UUID() + let remainingWindow = UUID() + registry.register(windowId: firstWindow, descriptor: first) + registry.register(windowId: remainingWindow, descriptor: second) + + let firstRequest = try #require(registry.beginActivation(for: first.key)) + #expect(registry.commitActivation(first.key, request: firstRequest)) + let secondRequest = try #require(registry.beginActivation(for: second.key)) + #expect(registry.commitActivation(second.key, request: secondRequest)) + + var activated: [WorkspaceContextKey] = [] + let coordinator = WorkspaceContextCloseCoordinator( + registry: registry, + closeWindow: { _ in true }, + activate: { activated.append($0) } + ) + + let didClose = await coordinator.close(key: second.key, sourceWindow: nil) + + #expect(didClose) + #expect(!registry.contains(second.key)) + #expect(registry.contains(first.key)) + #expect(registry.selectedKey == first.key) + #expect(activated == [first.key]) + } + + @Test("Activation and close share the registry they were given") + func coordinatorsShareInjectedRegistry() { + let registry = WorkspaceContextRegistry(store: InMemoryWorkspaceContextSnapshotStore()) + let item = contextDescriptor(database: "app", schema: "public") + registry.register(windowId: UUID(), descriptor: item) + + let activation = WorkspaceContextActivationCoordinator(registry: registry) + activation.activate(item.key) + + #expect(registry.selectedKey == item.key) + } +} + +private func contextDescriptor(database: String, schema: String?) -> WorkspaceContextDescriptor { + let connection = TestFixtures.makeConnection(database: database, type: .postgresql) + let key = WorkspaceContextKey.resolve( + connection: connection, + databaseName: database, + schemaName: schema, + activeDatabase: nil, + activeSchema: nil, + supportsSchemaSwitching: true + ) + return WorkspaceContextDescriptor( + key: key, + connectionName: connection.name, + databaseType: connection.type, + connectionColor: .blue, + isConnected: true + ) +} + +private final class InMemoryWorkspaceContextSnapshotStore: WorkspaceContextSnapshotStoring { + var snapshot = WorkspaceContextSnapshot(orderedKeys: [], selectedKey: nil) + + func load() -> WorkspaceContextSnapshot { + snapshot + } + + func save(_ snapshot: WorkspaceContextSnapshot) { + self.snapshot = snapshot + } +} From 47db45a8cde55c890f2b015f7b2da21f1acf02cb Mon Sep 17 00:00:00 2001 From: Joel Huang Date: Fri, 14 Aug 2026 17:04:41 +0800 Subject: [PATCH 3/4] fix(tabs): wire context rail into window grouping Host DatabaseContextRailView in the sidebar rail slot. Register and unregister windows from the workspace registry. Key native tab groups by the full context, not connection id. Resolve schema switching from the driver and keep snapshot keys off the rail until a window is actually open. --- .../MainSplitViewController.swift | 27 ++++++------ .../NavigationSidebarViewController.swift | 32 +++++++++++--- .../Infrastructure/TabWindowController.swift | 2 +- .../WindowLifecycleMonitor.swift | 4 ++ .../Infrastructure/WindowManager.swift | 17 ++++++-- ...orkspaceContextActivationCoordinator.swift | 40 ++++++++++++----- .../WorkspaceContextRegistry.swift | 41 ++++++++++-------- .../Models/Workspace/WorkspaceContext.swift | 43 +++++++++++++++++++ .../Extensions/MainContentView+Setup.swift | 15 ++++++- .../Workspace/DatabaseContextRailView.swift | 12 ++++-- .../Services/WindowTabGroupingTests.swift | 29 +++++++++++++ .../Models/WorkspaceContextTests.swift | 40 +++++++++++++++++ 12 files changed, 244 insertions(+), 58 deletions(-) diff --git a/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift b/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift index 539c8b4d4..7359ef15f 100644 --- a/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift +++ b/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift @@ -155,10 +155,6 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi navigationSidebar = NavigationSidebarViewController( connectionId: payload?.connectionId ?? currentSession?.connection.id ) - navigationSidebar.railController.onLayoutChange = { [weak self] _ in - self?.navigationSidebar.applyRailWidth(animated: false) - self?.recomputeWindowMinSize() - } sidebarSplitItem = NSSplitViewItem(sidebarWithViewController: navigationSidebar) sidebarSplitItem.canCollapse = true sidebarSplitItem.minimumThickness = Self.sidebarMinThickness @@ -180,9 +176,7 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi inspectorSplitItem.maximumThickness = NSSplitViewItem.unspecifiedDimension addSplitViewItem(inspectorSplitItem) - navigationSidebar.railController.onEntryCountChange = { [weak self] count in - self?.applyRailVisibility(workspaceCount: count) - } + applyRailVisibility(workspaceCount: WorkspaceContextRegistry.shared.contexts.count) /// The saved layout is restored before any phase-driven collapse, so the user's widths /// are already in the live layout. Uncollapsing then returns the pane to the size @@ -240,18 +234,21 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi .sink { [weak self] _ in self?.handleConnectionStatusChange() } - railVisibilityCancellable = AppEvents.shared.workspaceRailVisibilityChanged - .receive(on: RunLoop.main) - .sink { [weak self] _ in - self?.applyRailVisibility(workspaceCount: WorkspaceRailStore.entries.count) - } + railVisibilityCancellable = Publishers.Merge( + AppEvents.shared.workspaceRailVisibilityChanged, + AppEvents.shared.workspaceTabsChanged + ) + .receive(on: RunLoop.main) + .sink { [weak self] _ in + self?.applyRailVisibility(workspaceCount: WorkspaceContextRegistry.shared.contexts.count) + } connectionUpdatedCancellable = AppEvents.shared.connectionUpdated .receive(on: RunLoop.main) .sink { [weak self] changedId in self?.handleConnectionRecordChange(changedId) } handleConnectionStatusChange() - applyRailVisibility(workspaceCount: WorkspaceRailStore.entries.count) + applyRailVisibility(workspaceCount: WorkspaceContextRegistry.shared.contexts.count) } private func removeObservers() { @@ -619,7 +616,7 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi } var canToggleWorkspaceRail: Bool { - WorkspaceRailStore.entries.count > 1 + WorkspaceContextRegistry.shared.contexts.count > 1 } @@ -655,7 +652,7 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi } func activateWorkspace(offsetBy offset: Int) { - navigationSidebar?.railController.activateWorkspace(offsetBy: offset) + navigationSidebar?.activateWorkspace(offsetBy: offset) } // MARK: - Sidebar diff --git a/TablePro/Core/Services/Infrastructure/NavigationSidebarViewController.swift b/TablePro/Core/Services/Infrastructure/NavigationSidebarViewController.swift index 358aef1a9..05c3229d1 100644 --- a/TablePro/Core/Services/Infrastructure/NavigationSidebarViewController.swift +++ b/TablePro/Core/Services/Infrastructure/NavigationSidebarViewController.swift @@ -15,7 +15,7 @@ import SwiftUI /// sidebar it expects, and showing or hiding the rail becomes an ordinary layout change. @MainActor internal final class NavigationSidebarViewController: NSViewController { - internal let railController: WorkspaceRailViewController + internal let contextRailHosting: NSHostingController internal let objectBrowser: SidebarContainerViewController private let separator = NSBox() @@ -24,8 +24,17 @@ internal final class NavigationSidebarViewController: NSViewController { internal private(set) var isRailVisible = false - internal init(connectionId: UUID?) { - self.railController = WorkspaceRailViewController(connectionId: connectionId) + internal static let contextRailWidth: CGFloat = 200 + + internal init(connectionId _: UUID?) { + self.contextRailHosting = NSHostingController( + rootView: DatabaseContextRailView( + registry: .shared, + activationCoordinator: .shared, + closeCoordinator: .shared + ) + ) + self.contextRailHosting.sizingOptions = [] self.objectBrowser = SidebarContainerViewController(rootView: AnyView(Color.clear)) super.init(nibName: nil, bundle: nil) } @@ -38,10 +47,10 @@ internal final class NavigationSidebarViewController: NSViewController { override func loadView() { view = NSView() - addChild(railController) + addChild(contextRailHosting) addChild(objectBrowser) - let rail = railController.view + let rail = contextRailHosting.view let browser = objectBrowser.view separator.boxType = .separator @@ -92,7 +101,7 @@ internal final class NavigationSidebarViewController: NSViewController { /// which one drives the geometry up to AppKit, so the declared duration is not reliably the /// one that runs. internal func applyRailWidth(animated: Bool, alongside: (() -> Void)? = nil) { - let width = isRailVisible ? railController.currentLayout.width : 0 + let width = isRailVisible ? Self.contextRailWidth : 0 let separatorWidth: CGFloat = isRailVisible ? 1 : 0 guard railWidthConstraint.constant != width else { alongside?() @@ -111,4 +120,15 @@ internal final class NavigationSidebarViewController: NSViewController { alongside?() } } + + internal func activateWorkspace(offsetBy offset: Int) { + let registry = WorkspaceContextRegistry.shared + let keys = registry.contexts.map(\.key) + guard !keys.isEmpty else { return } + let current = registry.selectedKey + let index = current.flatMap { keys.firstIndex(of: $0) } ?? 0 + let count = keys.count + let destination = ((index + offset) % count + count) % count + WorkspaceContextActivationCoordinator.shared.activate(keys[destination]) + } } diff --git a/TablePro/Core/Services/Infrastructure/TabWindowController.swift b/TablePro/Core/Services/Infrastructure/TabWindowController.swift index 6c80c953b..d8c238ae2 100644 --- a/TablePro/Core/Services/Infrastructure/TabWindowController.swift +++ b/TablePro/Core/Services/Infrastructure/TabWindowController.swift @@ -71,7 +71,7 @@ internal final class TabWindowController: NSWindowController, NSWindowDelegate { window.toolbarStyle = .unified window.titleVisibility = .visible window.tabbingMode = .preferred - window.tabbingIdentifier = WindowManager.tabbingIdentifier(for: payload.connectionId) + window.tabbingIdentifier = WindowManager.tabbingIdentifier(payload: payload) window.collectionBehavior.insert([.fullScreenPrimary, .managed]) let splitVC = MainSplitViewController( diff --git a/TablePro/Core/Services/Infrastructure/WindowLifecycleMonitor.swift b/TablePro/Core/Services/Infrastructure/WindowLifecycleMonitor.swift index bd12e14b3..f9ce7ee00 100644 --- a/TablePro/Core/Services/Infrastructure/WindowLifecycleMonitor.swift +++ b/TablePro/Core/Services/Infrastructure/WindowLifecycleMonitor.swift @@ -59,6 +59,7 @@ internal final class WindowLifecycleMonitor { NotificationCenter.default.removeObserver(observer) } forgetFocus(windowId: supersededId, connectionId: superseded.connectionId) + WorkspaceContextRegistry.shared.unregister(windowId: supersededId) } // Remove any existing entry for this windowId to avoid duplicate observers @@ -271,6 +272,7 @@ internal final class WindowLifecycleMonitor { NotificationCenter.default.removeObserver(observer) } forgetFocus(windowId: windowId, connectionId: entry.connectionId) + WorkspaceContextRegistry.shared.unregister(windowId: windowId) } } @@ -278,6 +280,7 @@ internal final class WindowLifecycleMonitor { guard let entry = entries[windowId] else { return } guard lastFocusedWindowIds[entry.connectionId] != windowId else { return } lastFocusedWindowIds[entry.connectionId] = windowId + WorkspaceContextRegistry.shared.markActive(windowId: windowId) AppEvents.shared.connectionWindowsChanged.send() } @@ -305,6 +308,7 @@ internal final class WindowLifecycleMonitor { unregisterSourceFiles(for: windowId) entries.removeValue(forKey: windowId) forgetFocus(windowId: windowId, connectionId: closedConnectionId) + WorkspaceContextRegistry.shared.unregister(windowId: windowId) AppEvents.shared.connectionWindowsChanged.send() let hasRemainingWindows = entries.values.contains { diff --git a/TablePro/Core/Services/Infrastructure/WindowManager.swift b/TablePro/Core/Services/Infrastructure/WindowManager.swift index a52ac9341..2bc74c3df 100644 --- a/TablePro/Core/Services/Infrastructure/WindowManager.swift +++ b/TablePro/Core/Services/Infrastructure/WindowManager.swift @@ -147,13 +147,24 @@ internal final class WindowManager { return raw == "main" || raw.hasPrefix("main-") } - /// One native tab group per connection, so a window's tab bar only ever lists that - /// connection's tabs. A window hosts exactly one tab group, so a shared identifier would - /// flatten every connection into one bar. + /// Fallback when the connection record is not available yet. Prefer + /// `tabbingIdentifier(for: WorkspaceContextKey)` so tabs from different databases + /// or schemas never join the same native group. internal static func tabbingIdentifier(for connectionId: UUID) -> String { "com.TablePro.main.\(connectionId.uuidString)" } + internal static func tabbingIdentifier(for key: WorkspaceContextKey) -> String { + key.tabbingIdentifier + } + + internal static func tabbingIdentifier(payload: EditorTabPayload) -> String { + guard let connection = WorkspaceContextResolver.connection(for: payload.connectionId) else { + return tabbingIdentifier(for: payload.connectionId) + } + return tabbingIdentifier(for: WorkspaceContextResolver.resolve(payload: payload, connection: connection)) + } + private func findSibling(tabbingIdentifier: String, excluding: NSWindow) -> NSWindow? { NSApp.windows.first { candidate in candidate !== excluding diff --git a/TablePro/Core/Services/Infrastructure/WorkspaceContextActivationCoordinator.swift b/TablePro/Core/Services/Infrastructure/WorkspaceContextActivationCoordinator.swift index f524fa5ee..b2043079b 100644 --- a/TablePro/Core/Services/Infrastructure/WorkspaceContextActivationCoordinator.swift +++ b/TablePro/Core/Services/Infrastructure/WorkspaceContextActivationCoordinator.swift @@ -20,28 +20,48 @@ internal final class WorkspaceContextActivationCoordinator { schemaName: String?, initialQuery _: String? = nil ) { - let key = WorkspaceContextKey.resolve( + let key = WorkspaceContextResolver.resolve( connection: connection, databaseName: databaseName, schemaName: schemaName, - activeDatabase: nil, - activeSchema: nil, - supportsSchemaSwitching: true + session: DatabaseManager.shared.session(for: connection.id) ) activate(key) } internal func activate( _ key: WorkspaceContextKey, - preferredWindowId _: UUID? = nil, + preferredWindowId: UUID? = nil, sourceWindow _: NSWindow? = nil ) { guard let sequence = registry.beginActivation(for: key) else { return } - // Reconnect, switch DB/schema (existing logic) - // Activate the context's last-used native tab group - // (WindowManager logic for grouping by tabbingIdentifier) + + let window = windowToRaise(for: key, preferredWindowId: preferredWindowId) + if let window { + if let group = window.tabGroup, group.selectedWindow !== window { + group.selectedWindow = window + } + window.makeKeyAndOrderFront(nil) + NSApp.activate() + } + registry.commitActivation(key, request: sequence) - // Bring forward the context's last active window - // (native tab group activation) + } + + private func windowToRaise(for key: WorkspaceContextKey, preferredWindowId: UUID?) -> NSWindow? { + if let preferredWindowId, + let preferred = MainContentCoordinator.coordinator(for: preferredWindowId)?.contentWindow { + return preferred + } + + let registered = registry.windowIds(for: key).compactMap { windowId in + MainContentCoordinator.coordinator(for: windowId)?.contentWindow + } + if let lastFocused = WindowLifecycleMonitor.shared.mostRecentWindow(for: key.connectionId), + registered.contains(where: { $0 === lastFocused }) { + return lastFocused + } + return registered.first + ?? WindowLifecycleMonitor.shared.mostRecentWindow(for: key.connectionId) } } diff --git a/TablePro/Core/Services/Infrastructure/WorkspaceContextRegistry.swift b/TablePro/Core/Services/Infrastructure/WorkspaceContextRegistry.swift index ab972beae..8eb56136b 100644 --- a/TablePro/Core/Services/Infrastructure/WorkspaceContextRegistry.swift +++ b/TablePro/Core/Services/Infrastructure/WorkspaceContextRegistry.swift @@ -1,3 +1,4 @@ +import Combine import Foundation import Observation @@ -16,26 +17,15 @@ internal final class WorkspaceContextRegistry { private var windowIdsByKey: [WorkspaceContextKey: [UUID]] = [:] private var activationSequence: UInt64 = 0 private var activationHistory: [WorkspaceContextKey] = [] + private var preferredOrder: [WorkspaceContextKey] = [] internal init(store: WorkspaceContextSnapshotStoring = WorkspaceContextSnapshotStore()) { self.store = store let snapshot = store.load() - self.contexts = snapshot.orderedKeys.map { - WorkspaceContextDescriptor( - key: $0, - connectionName: "Unknown", - databaseType: .mysql, - connectionColor: .blue, - isConnected: true - ) - } + // Saved keys only restore order. Publishing them as descriptors would invent + // rail rows with no windows, which violates "only contexts with open tabs". + self.preferredOrder = snapshot.orderedKeys self.selectedKey = snapshot.selectedKey - // Rebuild indexes from loaded state (simplified) - for key in contexts.map(\.key) { - windowIdsByKey[key, default: []].forEach { id in - keyByWindowId[id] = key - } - } } internal func register(windowId: UUID, descriptor: WorkspaceContextDescriptor) { @@ -54,7 +44,7 @@ internal final class WorkspaceContextRegistry { if let index = contexts.firstIndex(where: { $0.key == descriptor.key }) { contexts[index] = descriptor } else { - contexts.append(descriptor) + insertInPreferredOrder(descriptor) } persist() } @@ -80,8 +70,7 @@ internal final class WorkspaceContextRegistry { internal func markActive(windowId: UUID) { guard let key = keyByWindowId[windowId] else { return } - activationHistory.append(key) - activationSequence += 1 + recordActivation(key) persist() } @@ -107,6 +96,21 @@ internal final class WorkspaceContextRegistry { contexts.contains { $0.key == key } } + private func insertInPreferredOrder(_ descriptor: WorkspaceContextDescriptor) { + if let preferredIndex = preferredOrder.firstIndex(of: descriptor.key) { + let insertAt = contexts.firstIndex { existing in + guard let existingIndex = preferredOrder.firstIndex(of: existing.key) else { + return true + } + return existingIndex > preferredIndex + } ?? contexts.endIndex + contexts.insert(descriptor, at: insertAt) + } else { + preferredOrder.append(descriptor.key) + contexts.append(descriptor) + } + } + private func removeContext(_ key: WorkspaceContextKey) { contexts.removeAll { $0.key == key } if selectedKey == key { @@ -130,5 +134,6 @@ internal final class WorkspaceContextRegistry { selectedKey: selectedKey ) ) + AppEvents.shared.workspaceTabsChanged.send() } } diff --git a/TablePro/Models/Workspace/WorkspaceContext.swift b/TablePro/Models/Workspace/WorkspaceContext.swift index 9498173d4..995628e3b 100644 --- a/TablePro/Models/Workspace/WorkspaceContext.swift +++ b/TablePro/Models/Workspace/WorkspaceContext.swift @@ -70,3 +70,46 @@ internal struct WorkspaceContextDescriptor: Identifiable, Equatable { .joined(separator: " / ") } } + +@MainActor +internal enum WorkspaceContextResolver { + internal static func resolve( + connection: DatabaseConnection, + databaseName: String? = nil, + schemaName: String? = nil, + session: ConnectionSession? = nil + ) -> WorkspaceContextKey { + WorkspaceContextKey.resolve( + connection: connection, + databaseName: databaseName, + schemaName: schemaName, + activeDatabase: session?.browseDatabase, + activeSchema: session?.browseSchema, + supportsSchemaSwitching: PluginManager.shared.supportsSchemaSwitching(for: connection.type) + ) + } + + internal static func resolve(payload: EditorTabPayload, connection: DatabaseConnection) -> WorkspaceContextKey { + resolve( + connection: connection, + databaseName: payload.databaseName, + schemaName: payload.schemaName, + session: DatabaseManager.shared.session(for: connection.id) + ) + } + + internal static func connection(for id: UUID) -> DatabaseConnection? { + DatabaseManager.shared.activeSessions[id]?.connection + ?? ConnectionStorage.shared.loadConnections().first { $0.id == id } + } + + internal static func descriptor(connection: DatabaseConnection, key: WorkspaceContextKey) -> WorkspaceContextDescriptor { + WorkspaceContextDescriptor( + key: key, + connectionName: connection.name, + databaseType: connection.type, + connectionColor: connection.color, + isConnected: DatabaseManager.shared.session(for: connection.id)?.isConnected == true + ) + } +} diff --git a/TablePro/Views/Main/Extensions/MainContentView+Setup.swift b/TablePro/Views/Main/Extensions/MainContentView+Setup.swift index 686aa1f1f..472266847 100644 --- a/TablePro/Views/Main/Extensions/MainContentView+Setup.swift +++ b/TablePro/Views/Main/Extensions/MainContentView+Setup.swift @@ -348,7 +348,16 @@ extension MainContentView { ) let isPreview = tabManager.selectedTab?.isPreview ?? payload?.isPreview ?? false - let resolvedId = WindowManager.tabbingIdentifier(for: connection.id) + let contextKey: WorkspaceContextKey + if let payload { + contextKey = WorkspaceContextResolver.resolve(payload: payload, connection: connection) + } else { + contextKey = WorkspaceContextResolver.resolve( + connection: connection, + session: DatabaseManager.shared.session(for: connection.id) + ) + } + let resolvedId = WindowManager.tabbingIdentifier(for: contextKey) window.tabbingIdentifier = resolvedId window.tabbingMode = .preferred coordinator.windowId = windowId @@ -358,6 +367,10 @@ extension MainContentView { connectionId: connection.id, windowId: windowId ) + WorkspaceContextRegistry.shared.register( + windowId: windowId, + descriptor: WorkspaceContextResolver.descriptor(connection: connection, key: contextKey) + ) viewWindow = window coordinator.contentWindow = window coordinator.isKeyWindow = window.isKeyWindow diff --git a/TablePro/Views/Workspace/DatabaseContextRailView.swift b/TablePro/Views/Workspace/DatabaseContextRailView.swift index 480fe8c5d..874bf7f6d 100644 --- a/TablePro/Views/Workspace/DatabaseContextRailView.swift +++ b/TablePro/Views/Workspace/DatabaseContextRailView.swift @@ -4,15 +4,16 @@ import SwiftUI // DatabaseContextRailView.swift — vertical rail list and actions // Part of the Database Context Rail feature (Task 5 of the plan). +@MainActor struct DatabaseContextRailView: View { @Bindable private var registry: WorkspaceContextRegistry private let activationCoordinator: WorkspaceContextActivationCoordinator private let closeCoordinator: WorkspaceContextCloseCoordinator init( - registry: WorkspaceContextRegistry = .shared, - activationCoordinator: WorkspaceContextActivationCoordinator = .shared, - closeCoordinator: WorkspaceContextCloseCoordinator = .shared + registry: WorkspaceContextRegistry, + activationCoordinator: WorkspaceContextActivationCoordinator, + closeCoordinator: WorkspaceContextCloseCoordinator ) { self.registry = registry self.activationCoordinator = activationCoordinator @@ -46,6 +47,9 @@ struct DatabaseContextRailView: View { } .padding(8) } - .frame(width: 240) + .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading) + .accessibilityElement(children: .contain) + .accessibilityIdentifier("workspace-rail") + .accessibilityLabel(String(localized: "Open Workspaces")) } } diff --git a/TableProTests/Core/Services/WindowTabGroupingTests.swift b/TableProTests/Core/Services/WindowTabGroupingTests.swift index a3857c743..0d99d6de7 100644 --- a/TableProTests/Core/Services/WindowTabGroupingTests.swift +++ b/TableProTests/Core/Services/WindowTabGroupingTests.swift @@ -53,4 +53,33 @@ struct WindowTabGroupingTests { #expect(id1 == id2) } + @Test("A context key is not the connection-only fallback") + func contextKeyDiffersFromConnectionOnlyFallback() { + let key = WorkspaceContextKey( + connectionId: UUID(), + databaseName: "app", + schemaName: "public" + ) + + #expect(WindowManager.tabbingIdentifier(for: key) == key.tabbingIdentifier) + #expect(WindowManager.tabbingIdentifier(for: key) != WindowManager.tabbingIdentifier(for: key.connectionId)) + } + + @Test("Different schemas of one connection produce different tab groups") + func schemasDoNotShareATabGroup() { + let connectionId = UUID() + let publicKey = WorkspaceContextKey( + connectionId: connectionId, + databaseName: "app", + schemaName: "public" + ) + let auditKey = WorkspaceContextKey( + connectionId: connectionId, + databaseName: "app", + schemaName: "audit" + ) + + #expect(WindowManager.tabbingIdentifier(for: publicKey) != WindowManager.tabbingIdentifier(for: auditKey)) + } + } diff --git a/TableProTests/Models/WorkspaceContextTests.swift b/TableProTests/Models/WorkspaceContextTests.swift index b21fd89f6..d2ead20ad 100644 --- a/TableProTests/Models/WorkspaceContextTests.swift +++ b/TableProTests/Models/WorkspaceContextTests.swift @@ -175,6 +175,46 @@ struct WorkspaceContextRegistryTests { #expect(store.snapshot.orderedKeys == [item.key]) } + + @Test("Snapshot keys are not shown until a window registers") + func snapshotDoesNotFabricateRailRows() { + let first = contextDescriptor(database: "app", schema: "public") + let second = contextDescriptor(database: "app", schema: "audit") + let store = InMemoryWorkspaceContextSnapshotStore() + store.snapshot = WorkspaceContextSnapshot( + orderedKeys: [second.key, first.key], + selectedKey: first.key + ) + + let registry = WorkspaceContextRegistry(store: store) + + #expect(registry.contexts.isEmpty) + #expect(registry.selectedKey == first.key) + + registry.register(windowId: UUID(), descriptor: first) + registry.register(windowId: UUID(), descriptor: second) + + #expect(registry.contexts.map(\.key) == [second.key, first.key]) + } + + @Test("Driver capability decides whether schema is part of the key") + func resolveUsesDriverSchemaCapability() { + let mysql = TestFixtures.makeConnection(database: "app", type: .mysql) + let mysqlKey = WorkspaceContextResolver.resolve( + connection: mysql, + databaseName: "app", + schemaName: "ignored" + ) + #expect(mysqlKey.schemaName == nil) + + let postgres = TestFixtures.makeConnection(database: "app", type: .postgresql) + let postgresKey = WorkspaceContextResolver.resolve( + connection: postgres, + databaseName: "app", + schemaName: "audit" + ) + #expect(postgresKey.schemaName == "audit") + } } @MainActor From 700915b64df0148b8465818e52b9cef222bb343f Mon Sep 17 00:00:00 2001 From: Joel Huang Date: Fri, 14 Aug 2026 17:44:28 +0800 Subject: [PATCH 4/4] fix(tabs): make context close atomic and guard running queries Preflight every window for unsaved SQL, pending grid edits, and a running query before any window closes. A later cancel leaves earlier windows open and still registered. Close and unregister only after every confirmation succeeds. --- .../WorkspaceContextCloseCoordinator.swift | 45 ++++++---- .../Main/MainContentCommandActions.swift | 86 +++++++++++-------- .../Models/WorkspaceContextTests.swift | 79 +++++++++++++++-- 3 files changed, 153 insertions(+), 57 deletions(-) diff --git a/TablePro/Core/Services/Infrastructure/WorkspaceContextCloseCoordinator.swift b/TablePro/Core/Services/Infrastructure/WorkspaceContextCloseCoordinator.swift index d49887a6c..85b585ad4 100644 --- a/TablePro/Core/Services/Infrastructure/WorkspaceContextCloseCoordinator.swift +++ b/TablePro/Core/Services/Infrastructure/WorkspaceContextCloseCoordinator.swift @@ -7,18 +7,21 @@ import Foundation @MainActor internal final class WorkspaceContextCloseCoordinator { private let registry: WorkspaceContextRegistry - private let closeWindow: (UUID) async -> Bool + private let confirmWindow: (UUID) async -> Bool + private let closeWindow: (UUID) -> Void private let activate: (WorkspaceContextKey) -> Void internal static let shared = WorkspaceContextCloseCoordinator() internal init( registry: WorkspaceContextRegistry = .shared, - closeWindow: ((UUID) async -> Bool)? = nil, + confirmWindow: ((UUID) async -> Bool)? = nil, + closeWindow: ((UUID) -> Void)? = nil, activate: ((WorkspaceContextKey) -> Void)? = nil ) { self.registry = registry - self.closeWindow = closeWindow ?? WorkspaceContextCloseCoordinator.closeRegisteredWindow + self.confirmWindow = confirmWindow ?? WorkspaceContextCloseCoordinator.confirmRegisteredWindow + self.closeWindow = closeWindow ?? WorkspaceContextCloseCoordinator.commitRegisteredWindow self.activate = activate ?? { key in WorkspaceContextActivationCoordinator.shared.activate(key) } @@ -27,10 +30,14 @@ internal final class WorkspaceContextCloseCoordinator { internal func close(key: WorkspaceContextKey, sourceWindow _: NSWindow?) async -> Bool { let windowIds = registry.windowIds(for: key) - // Existing closeWindowAwaiting already prompts for unsaved SQL, pending - // data-grid edits, and running work. Cancel must leave the context intact. + // Confirm every window before any close. A later cancel must leave earlier + // windows open and still registered. for windowId in windowIds { - guard await closeWindow(windowId) else { return false } + guard await confirmWindow(windowId) else { return false } + } + + for windowId in windowIds { + closeWindow(windowId) registry.unregister(windowId: windowId) } @@ -45,20 +52,28 @@ internal final class WorkspaceContextCloseCoordinator { return true } - /// Reuse existing batch close logic from MainContentCommandActions+BulkClose.swift - /// (unsaved SQL, pending data-grid changes, running queries) - /// Return false on any cancel - private static func closeRegisteredWindow(_ windowId: UUID) async -> Bool { + /// Unsaved SQL, pending grid edits, and a running query all prompt here. + /// Nothing is closed or unregistered until every window has agreed. + private static func confirmRegisteredWindow(_ windowId: UUID) async -> Bool { guard let coordinator = MainContentCoordinator.coordinator(for: windowId) else { return true } if let actions = coordinator.commandActions { - return await actions.closeWindowAwaiting(asBatchSurvivor: false) == .closed + return await actions.confirmWindowClose() } - // A live coordinator without command actions still has unsaved work that - // closeWindowAwaiting would have prompted for. Do not discard it. - guard !coordinator.hasAnyUnsavedWork() else { return false } - coordinator.contentWindow?.close() + // A live coordinator without command actions still has work that + // confirmWindowClose would have prompted for. Do not discard it. + if coordinator.hasAnyUnsavedWork() { return false } + if coordinator.toolbarState.isExecuting { return false } return true } + + private static func commitRegisteredWindow(_ windowId: UUID) { + guard let coordinator = MainContentCoordinator.coordinator(for: windowId) else { return } + if let actions = coordinator.commandActions { + actions.commitWindowClose(asBatchSurvivor: false) + return + } + coordinator.contentWindow?.close() + } } diff --git a/TablePro/Views/Main/MainContentCommandActions.swift b/TablePro/Views/Main/MainContentCommandActions.swift index ff3bcf058..580a7d789 100644 --- a/TablePro/Views/Main/MainContentCommandActions.swift +++ b/TablePro/Views/Main/MainContentCommandActions.swift @@ -424,29 +424,58 @@ final class MainContentCommandActions { /// window it keeps blank and `false` for every window it tears down. @discardableResult func closeWindowAwaiting(asBatchSurvivor: Bool? = nil) async -> WindowCloseOutcome { + guard await confirmWindowClose() else { return .cancelled } + commitWindowClose(asBatchSurvivor: asBatchSurvivor) + return .closed + } + + /// Prompts for unsaved work and a running query without closing the window. + /// Save applies now; Don't Save only records consent so a later cancel can still keep the tabs. + func confirmWindowClose() async -> Bool { let seq = MainContentCoordinator.nextSwitchSeq() - Self.logger.info("[close] closeWindowAwaiting seq=\(seq) hasUnsavedWork=\(self.hasUnsavedWorkInWindow)") + Self.logger.info( + "[close] confirmWindowClose seq=\(seq) hasUnsavedWork=\(self.hasUnsavedWorkInWindow) isExecuting=\(self.hasRunningQueryInWindow)" + ) - guard hasUnsavedWorkInWindow else { - finish(asBatchSurvivor: asBatchSurvivor) - return .closed + if hasUnsavedWorkInWindow { + selectInTabGroup() + let result = await AlertHelper.confirmSaveChanges( + message: String(localized: "Your changes will be lost if you don't save them."), + window: closeAnchorWindow + ) + switch result { + case .save: + guard await saveWithoutClosing() else { return false } + case .dontSave: + break + case .cancel: + return false + } } - selectInTabGroup() - let result = await AlertHelper.confirmSaveChanges( - message: String(localized: "Your changes will be lost if you don't save them."), - window: closeAnchorWindow - ) - - switch result { - case .save: - return await saveAndClose(asBatchSurvivor: asBatchSurvivor) ? .closed : .cancelled - case .dontSave: - discardAndClose(asBatchSurvivor: asBatchSurvivor) - return .closed - case .cancel: - return .cancelled + if hasRunningQueryInWindow { + selectInTabGroup() + return await AlertHelper.confirmDestructive( + title: String(localized: "A query is still running"), + message: String(localized: "A query is still running. Closing cancels it."), + confirmButton: String(localized: "Close"), + window: closeAnchorWindow + ) } + + return true + } + + func commitWindowClose(asBatchSurvivor: Bool? = nil) { + coordinator?.changeManager.clearChangesAndUndoHistory() + pendingTruncates.wrappedValue.removeAll() + pendingDeletes.wrappedValue.removeAll() + rightPanelState.editState.clearEdits() + finish(asBatchSurvivor: asBatchSurvivor) + } + + private var hasRunningQueryInWindow: Bool { + coordinator?.toolbarState.isExecuting ?? false } var closeAnchorWindow: NSWindow? { @@ -516,11 +545,8 @@ final class MainContentCommandActions { coordinator.toolbarState.isTableTab = false } - private func saveAndClose(asBatchSurvivor: Bool?) async -> Bool { - guard let coordinator = coordinator else { - finish(asBatchSurvivor: asBatchSurvivor) - return true - } + private func saveWithoutClosing() async -> Bool { + guard let coordinator else { return true } // User and role changes can only be applied after the SQL is reviewed, so Save opens the // review sheet and cancels the close. Falling through here would close the window and @@ -530,43 +556,31 @@ final class MainContentCommandActions { return false } - // Structure view saves via direct coordinator call if coordinator.tabManager.selectedTab?.display.resultsViewMode == .structure { coordinator.structureActions?.saveChanges?() - finish(asBatchSurvivor: asBatchSurvivor) return true } - // Data grid changes or pending table operations take priority let hasDataChanges = coordinator.changeManager.hasChanges || !pendingTruncates.wrappedValue.isEmpty || !pendingDeletes.wrappedValue.isEmpty if hasDataChanges { - let saved = await withCheckedContinuation { continuation in + return await withCheckedContinuation { continuation in coordinator.saveCompletionContinuation = continuation saveChanges() } - if saved { - finish(asBatchSurvivor: asBatchSurvivor) - } - return saved } - // Sidebar-only edits (made directly in the inspector panel) if rightPanelState.editState.hasEdits { rightPanelState.onSave?() - finish(asBatchSurvivor: asBatchSurvivor) return true } - // File save (query editor with source file) if coordinator.tabManager.selectedTab?.content.isFileDirty == true { saveFileToSourceURL() - finish(asBatchSurvivor: asBatchSurvivor) return true } - finish(asBatchSurvivor: asBatchSurvivor) return true } diff --git a/TableProTests/Models/WorkspaceContextTests.swift b/TableProTests/Models/WorkspaceContextTests.swift index d2ead20ad..b2e9b3d50 100644 --- a/TableProTests/Models/WorkspaceContextTests.swift +++ b/TableProTests/Models/WorkspaceContextTests.swift @@ -220,8 +220,8 @@ struct WorkspaceContextRegistryTests { @MainActor @Suite("WorkspaceContextCloseCoordinator") struct WorkspaceContextCloseCoordinatorTests { - @Test("A cancelled window close leaves the context registered") - func cancelLeavesContextIntact() async { + @Test("A later cancel does not close an earlier window that already passed preflight") + func laterCancelLeavesEveryWindowRegistered() async { let registry = WorkspaceContextRegistry(store: InMemoryWorkspaceContextSnapshotStore()) let item = contextDescriptor(database: "app", schema: "public") let firstWindow = UUID() @@ -229,12 +229,16 @@ struct WorkspaceContextCloseCoordinatorTests { registry.register(windowId: firstWindow, descriptor: item) registry.register(windowId: secondWindow, descriptor: item) + var confirmed: [UUID] = [] var closed: [UUID] = [] let coordinator = WorkspaceContextCloseCoordinator( registry: registry, + confirmWindow: { windowId in + confirmed.append(windowId) + return windowId != secondWindow + }, closeWindow: { windowId in closed.append(windowId) - return windowId != secondWindow }, activate: { _ in } ) @@ -242,9 +246,36 @@ struct WorkspaceContextCloseCoordinatorTests { let didClose = await coordinator.close(key: item.key, sourceWindow: nil) #expect(!didClose) - #expect(closed == [firstWindow, secondWindow]) + #expect(confirmed == [firstWindow, secondWindow]) + #expect(closed.isEmpty) #expect(registry.contains(item.key)) - #expect(registry.windowIds(for: item.key) == [secondWindow]) + #expect(registry.windowIds(for: item.key) == [firstWindow, secondWindow]) + } + + @Test("A running query without unsaved work blocks context close") + func runningQueryCancelLeavesContextIntact() async { + let connection = TestFixtures.makeConnection(database: "app", type: .postgresql) + let state = SessionStateFactory.create(connection: connection, payload: nil) + defer { state.coordinator.teardown() } + + let windowId = UUID() + state.coordinator.windowId = windowId + state.coordinator.toolbarState.setExecuting(true) + + let item = contextDescriptor(database: "app", schema: "public") + let registry = WorkspaceContextRegistry(store: InMemoryWorkspaceContextSnapshotStore()) + registry.register(windowId: windowId, descriptor: item) + + let coordinator = WorkspaceContextCloseCoordinator( + registry: registry, + activate: { _ in } + ) + + let didClose = await coordinator.close(key: item.key, sourceWindow: nil) + + #expect(!didClose) + #expect(registry.contains(item.key)) + #expect(registry.windowIds(for: item.key) == [windowId]) } @Test("A successful close unregisters every window and the rail item") @@ -262,22 +293,58 @@ struct WorkspaceContextCloseCoordinatorTests { let secondRequest = try #require(registry.beginActivation(for: second.key)) #expect(registry.commitActivation(second.key, request: secondRequest)) + var confirmed: [UUID] = [] + var closed: [UUID] = [] var activated: [WorkspaceContextKey] = [] let coordinator = WorkspaceContextCloseCoordinator( registry: registry, - closeWindow: { _ in true }, + confirmWindow: { windowId in + confirmed.append(windowId) + return true + }, + closeWindow: { windowId in + closed.append(windowId) + }, activate: { activated.append($0) } ) let didClose = await coordinator.close(key: second.key, sourceWindow: nil) #expect(didClose) + #expect(confirmed == [remainingWindow]) + #expect(closed == [remainingWindow]) #expect(!registry.contains(second.key)) #expect(registry.contains(first.key)) #expect(registry.selectedKey == first.key) #expect(activated == [first.key]) } + @Test("Unsaved work without a prompt still blocks context close") + func unsavedWorkBlocksCloseWhenPromptUnavailable() async { + let connection = TestFixtures.makeConnection(database: "app", type: .postgresql) + let state = SessionStateFactory.create(connection: connection, payload: nil) + defer { state.coordinator.teardown() } + + let windowId = UUID() + state.coordinator.windowId = windowId + state.coordinator.changeManager.hasChanges = true + + let item = contextDescriptor(database: "app", schema: "public") + let registry = WorkspaceContextRegistry(store: InMemoryWorkspaceContextSnapshotStore()) + registry.register(windowId: windowId, descriptor: item) + + let coordinator = WorkspaceContextCloseCoordinator( + registry: registry, + activate: { _ in } + ) + + let didClose = await coordinator.close(key: item.key, sourceWindow: nil) + + #expect(!didClose) + #expect(registry.contains(item.key)) + #expect(registry.windowIds(for: item.key) == [windowId]) + } + @Test("Activation and close share the registry they were given") func coordinatorsShareInjectedRegistry() { let registry = WorkspaceContextRegistry(store: InMemoryWorkspaceContextSnapshotStore())