fix(tabs): ask before closing a tab that holds unsaved work

CHANGELOG.md

@@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - The PHP serialized viewer's tree filter now behaves like the JSON one. Both ignore accents, so `cafe` finds `café`. (#2204)
 - Picking a database in the connections strip returns you to the tab you last used in it, the way picking a connection already returns you to that connection's tab. A database with nothing open just moves the object browser, as before. (#2217)
 - Two tabs showing objects with the same name from different databases now carry the database in their names, so two tabs called `orders` read as `app.orders` and `staging.orders`. A name no other tab uses stays short. Hovering a tab, or reading it with VoiceOver, always names its database. (#2217)
+- Save in the "Do you want to save changes?" prompt now closes what you asked to close once the save lands. Closing a group of tabs, or closing a connection, used to save and then leave everything open.
 
 ### Fixed
 
@@ -52,6 +53,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - A tree filter that finds nothing now says so instead of showing an empty list, and says when the value was too large to load in full. (#2204)
 - Filtering a tree now searches the whole of a long string value instead of only the shortened form shown in the row. (#2204)
 - Copy Value on a JSON object or array now copies that part of the document instead of a summary like `{3 keys}`, and long strings copy in full. (#2204)
+- Closing a tab holding unsaved work asks before it goes, instead of discarding the work in silence. This covers unsaved cell edits in a table tab, a `.sql` file that differs from what is on disk, staged structure and Create Table changes, and staged user and role changes. Cell edits were the worst case: nothing brought them back, not even Reopen Closed Tab. Tabs holding only typed query text still close without asking, because that text comes back.
+- A tab with unsaved cell edits now shows the unsaved dot, so a tab is never marked clean and then asks to be saved.
+- Closing a tab used to leave its unsaved cell edits loaded behind it. The connection went on reporting unsaved changes with no tabs to show for it, and saving from there could write those edits to whichever database the sidebar had moved to.
+- A `.sql` file opened from a linked favorite can be opened again after its tab is closed. Opening it used to just bring the window forward and do nothing.
 
 ## [0.66.0] - 2026-08-19
 

TablePro/Core/Services/Infrastructure/ConnectionCloseAction.swift

@@ -49,7 +49,10 @@ internal enum ConnectionCloseAction {
             window: presentingWindow
         ) {
         case .save:
-            coordinator?.commandActions?.saveChanges()
+            /// Save closes too, once the save has actually landed. It used to start the save and
+            /// stop there, so the connection the user asked to close stayed open.
+            guard await coordinator?.commandActions?.saveSelectedTabWork() == true else { break }
+            WindowManager.shared.closeWindow(for: connectionId)
         case .dontSave:
             WindowManager.shared.closeWindow(for: connectionId)
         case .cancel:

TablePro/Views/Main/Child/MainEditorContentView.swift

@@ -264,7 +264,7 @@ struct MainEditorContentView: View {
     private func usersRolesContent(tab: QueryTab) -> some View {
         Group {
             if let vm = usersRolesViewModels[tab.id] {
-                UsersRolesTabView(viewModel: vm, coordinator: coordinator)
+                UsersRolesTabView(viewModel: vm, coordinator: coordinator, tabID: tab.id)
             } else {
                 ProgressView(String(localized: "Loading users and roles..."))
                     .frame(maxWidth: .infinity, maxHeight: .infinity)
@@ -502,7 +502,7 @@ struct MainEditorContentView: View {
                 guard tabId == tabManager.selectedTabId,
                       let index = tabManager.tabs.firstIndex(where: { $0.id == tabId }),
                       let window = coordinator.contentWindow else { return }
-                let showsIndicator = tabManager.tabs[index].showsUnsavedIndicator
+                let showsIndicator = coordinator.showsUnsavedIndicator(for: tabManager.tabs[index])
                 Task { @MainActor in
                     window.isDocumentEdited = showsIndicator
                 }

TablePro/Views/Main/Extensions/MainContentCoordinator+Protection.swift

@@ -13,29 +13,70 @@ extension MainContentCoordinator {
         rightPanelState?.editState.hasEdits ?? false
     }
 
-    /// Work that is only recoverable by saving it. A scratch query tab is deliberately absent:
-    /// its text is persisted with the tab and comes back on relaunch, so it needs no alert.
-    /// Closing a tab also files it in `RecentlyClosedTabStore`; quitting does not, which is why
-    /// the tab-state write must never be skipped or cleared on an empty in-memory tab list.
+    /// Only the selected tab's editors are mounted, so only the selected tab has live state on the
+    /// coordinator to read.
+    func isSelectedTab(_ tab: QueryTab) -> Bool {
+        tabManager.selectedTabId == tab.id
+    }
+
+    /// Work in one tab that only saving can recover.
+    ///
+    /// Where that work lives depends on whether the tab is selected, because a tab's editors are
+    /// mounted only while it is. The selected tab's live edits sit on the coordinator, in
+    /// `changeManager` and `toolbarState`; a background tab carries whatever was snapshotted into
+    /// `pendingChanges` the last time it was switched away from. Reading the snapshot for the
+    /// selected tab answers "no" for the gesture users make most, editing a cell and closing the
+    /// tab they are looking at, because nothing has switched away to write the snapshot yet.
+    ///
+    /// Connection-scoped work is deliberately absent. A staged TRUNCATE or an unsaved sidebar edit
+    /// belongs to the connection rather than to any one tab, so letting it gate a tab's close would
+    /// pose a question neither Save nor Don't Save could answer for the tab being closed.
+    ///
+    /// A scratch query tab is absent too, and that one is a decision rather than an omission: its
+    /// text is persisted with the tab and filed in `RecentlyClosedTabStore` on close, so it comes
+    /// back. Grid edits do not. `TabChangeSnapshot` is not `Codable` and `PersistedTab` carries no
+    /// change fields, so closing a table tab is the one gesture that destroys them for good.
     func hasUnsavedWork(in tab: QueryTab?) -> Bool {
         guard let tab else { return false }
-        if tab.tabType == .usersRoles {
-            return usersRolesActions?.hasChanges() ?? false
+        if tab.content.isFileDirty { return true }
+        guard isSelectedTab(tab) else { return backgroundUnsavedWork(in: tab) }
+        return liveUnsavedWork(in: tab)
+    }
+
+    /// A deselected tab left its editors behind, so the answer is whatever they wrote down before
+    /// they went. Users and roles keeps its own record because its view model outlives the view
+    /// while `usersRolesActions` does not, so the staged principals survive a deselect even though
+    /// nothing on the coordinator can still reach them.
+    private func backgroundUnsavedWork(in tab: QueryTab) -> Bool {
+        if tab.tabType == .usersRoles { return tabsWithStagedPrincipals.contains(tab.id) }
+        return tab.pendingChanges.hasChanges
+    }
+
+    private func liveUnsavedWork(in tab: QueryTab) -> Bool {
+        switch tab.tabType {
+        case .usersRoles:
+            return usersRolesActions?.hasChanges() ?? tabsWithStagedPrincipals.contains(tab.id)
+        case .createTable:
+            return toolbarState.hasCreateTablePending
+        default:
+            return changeManager.hasChanges || toolbarState.hasStructureChanges
         }
-        return tab.content.isFileDirty || tab.pendingChanges.hasChanges
     }
 
-    func hasUnsavedWorkInSelectedTab() -> Bool {
-        changeManager.hasChanges
-            || hasPendingDestructiveTableOps
-            || hasSidebarEdits
-            || hasUnsavedWork(in: tabManager.selectedTab)
+    /// The dot on the tab and in the window's close button. Deliberately broader than
+    /// `hasUnsavedWork(in:)`: a scratch query tab shows the dot because its text is unsaved, yet
+    /// closing it asks nothing because the text comes back. It is never narrower, so anything that
+    /// would raise the save prompt is marked before the user reaches for the close button.
+    func showsUnsavedIndicator(for tab: QueryTab) -> Bool {
+        tab.showsUnsavedIndicator || hasUnsavedWork(in: tab)
     }
 
     func hasAnyUnsavedWork() -> Bool {
         changeManager.hasChanges
             || hasPendingDestructiveTableOps
             || hasSidebarEdits
+            || toolbarState.hasStructureChanges
+            || toolbarState.hasCreateTablePending
             || tabManager.tabs.contains { hasUnsavedWork(in: $0) }
     }
 

TablePro/Views/Main/Extensions/MainContentCoordinator+TabClosing.swift

@@ -13,14 +13,39 @@ extension MainContentCoordinator {
     ///
     /// Closing tabs never closes the window: the window hosts every open connection now, so the
     /// connection is simply left on its empty state.
+    ///
+    /// Consent to close is not consent to lose work. Callers that can reach a tab holding unsaved
+    /// work ask first, through `MainContentCommandActions.closeTabAwaiting(id:)`.
     func closeTabsByUser(ids: [UUID]) {
         dataTabDelegate?.tableViewCoordinator?.flushPendingColumnLayoutPersistence()
         for id in ids {
             guard let tab = tabManager.tabs.first(where: { $0.id == id }) else { continue }
             RecentlyClosedTabStore.shared.push(tab: tab, connection: connection)
+            releaseResources(of: tab)
             tabManager.closeTab(id: id)
         }
         guard tabManager.tabs.isEmpty else { return }
         persistence.clearForUserClosedAllTabs()
     }
+
+    /// A closed tab has to take its coordinator-side state with it, and this is the only place that
+    /// can: `handleTabChange` snapshots a tab by looking it up in `tabManager.tabs`, and by the time
+    /// it runs the tab is already gone. So it must happen before `tabManager.closeTab(id:)`, which
+    /// is also what makes the selected-tab test meaningful, since that call reassigns the selection.
+    ///
+    /// Left behind, the change manager keeps the closed tab's edits and the table name they were
+    /// written against. Nothing clears it, so the connection goes on reporting unsaved work with no
+    /// tabs to show for it, and the next Save resolves its scope through `browseScope` and runs
+    /// those statements against whatever database the sidebar has since moved to.
+    private func releaseResources(of tab: QueryTab) {
+        if let url = tab.content.sourceFileURL {
+            WindowLifecycleMonitor.shared.unregisterSourceFile(url)
+        }
+        tabsWithStagedPrincipals.remove(tab.id)
+        guard isSelectedTab(tab) else { return }
+        changeManager.clearChangesAndUndoHistory()
+        toolbarState.hasStructureChanges = false
+        toolbarState.hasCreateTablePending = false
+        toolbarState.hasPrincipalChanges = false
+    }
 }

TablePro/Views/Main/Extensions/MainContentView+Setup.swift

@@ -245,7 +245,7 @@ extension MainContentView {
             connectionId: connection.id
         )
         viewWindow?.representedURL = selectedTab?.content.sourceFileURL
-        viewWindow?.isDocumentEdited = selectedTab?.showsUnsavedIndicator ?? false
+        viewWindow?.isDocumentEdited = selectedTab.map(coordinator.showsUnsavedIndicator) ?? false
     }
 
     /// Configure the hosting NSWindow — called by WindowAccessor when the window is available.
@@ -270,7 +270,7 @@ extension MainContentView {
 
         // Native proxy icon (Cmd+click shows path in Finder) and dirty dot
         window.representedURL = tabManager.selectedTab?.content.sourceFileURL
-        window.isDocumentEdited = tabManager.selectedTab?.showsUnsavedIndicator ?? false
+        window.isDocumentEdited = tabManager.selectedTab.map(coordinator.showsUnsavedIndicator) ?? false
 
         commandActions?.window = window
 

TablePro/Views/Main/MainContentCommandActions+BulkClose.swift

@@ -78,7 +78,13 @@ extension MainContentCommandActions {
 
     /// A partial close leaves the window open, so it cannot lean on the window's own prompt.
     /// Unsaved work is tracked for the connection rather than per tab, so the question is asked
-    /// once for the batch.
+    /// once for the batch rather than once per tab, which is also what keeps the sheets from
+    /// queueing: `NSWindow.beginSheet` queues a second sheet behind the first rather than
+    /// presenting it, so N prompts would be answered one at a time with no way to see why.
+    ///
+    /// Save goes on to close. This used to save and then return false, which left the batch
+    /// standing after a successful save and made Save mean "cancel" on this path while it meant
+    /// "close" on the window path.
     func confirmDiscardingUnsavedWork() async -> Bool {
         guard hasUnsavedWorkInConnection else { return true }
 
@@ -87,8 +93,7 @@ extension MainContentCommandActions {
             window: closeAnchorWindow
         ) {
         case .save:
-            saveChanges()
-            return false
+            return await saveSelectedTabWork()
         case .dontSave:
             return true
         case .cancel:

TablePro/Views/Main/MainContentCommandActions.swift

@@ -496,7 +496,61 @@ final class MainContentCommandActions {
     /// in right after connecting. The window hosts every open connection now, so closing it here
     /// would take the other connections' tabs and their unsaved edits with it.
     func closeTab(id: UUID) {
-        coordinator?.closeTabsByUser(ids: [id])
+        Task { await closeTabAwaiting(id: id) }
+    }
+
+    /// A tab holding work only a save can recover asks before it goes, which is what the window
+    /// close and the batch closes already do and what the HIG requires of an app that does not
+    /// autosave: "present a save dialog when people choose to close the document, quit your app,
+    /// log out, or restart".
+    ///
+    /// Save proceeds with the close, per `NSDocument.canCloseDocumentWithDelegate`: "shouldClose
+    /// will be YES if ... the user chose to discard modifications, or chose to save and the saving
+    /// was successful". `saveSelectedTabWork` returns false for the one case where saving cannot
+    /// finish on its own, staged principals, whose review sheet is now up and owns the decision.
+    func closeTabAwaiting(id: UUID) async {
+        guard let coordinator,
+              let tab = coordinator.tabManager.tabs.first(where: { $0.id == id }) else { return }
+        guard coordinator.hasUnsavedWork(in: tab) else {
+            coordinator.closeTabsByUser(ids: [id])
+            return
+        }
+        guard coordinator.tabClosesInFlight.insert(id).inserted else { return }
+        defer { coordinator.tabClosesInFlight.remove(id) }
+
+        let previousSelection = coordinator.tabManager.selectedTabId
+        revealTab(id)
+
+        switch await AlertHelper.confirmSaveChanges(
+            message: String(localized: "Your changes will be lost if you don't save them."),
+            window: closeAnchorWindow
+        ) {
+        case .save:
+            guard await saveSelectedTabWork() else { return }
+            coordinator.closeTabsByUser(ids: [id])
+        case .dontSave:
+            coordinator.closeTabsByUser(ids: [id])
+        case .cancel:
+            restoreSelection(previousSelection)
+        }
+    }
+
+    /// Shown, then asked. The save and discard machinery reads the selected tab, so the tab being
+    /// closed has to be the selected one before the question is put; naming work the user cannot
+    /// see would also ask them to decide about something they have no way to look at first.
+    private func revealTab(_ id: UUID) {
+        guard let coordinator, coordinator.tabManager.selectedTabId != id else { return }
+        coordinator.tabManager.selectedTabId = id
+    }
+
+    /// Cancel puts everything back, including a selection that only moved so the sheet had
+    /// somewhere honest to point.
+    private func restoreSelection(_ id: UUID?) {
+        guard let coordinator,
+              let id,
+              coordinator.tabManager.selectedTabId != id,
+              coordinator.tabManager.tabs.contains(where: { $0.id == id }) else { return }
+        coordinator.tabManager.selectedTabId = id
     }
 
     /// Cmd+W closes the tab in front. Pressed again with no tabs left it closes the connection,
@@ -613,15 +667,15 @@ final class MainContentCommandActions {
         coordinator.toolbarState.isTableTab = false
     }
 
-    private func saveAndClose(asBatchSurvivor: Bool?) async -> Bool {
-        guard let coordinator = coordinator else {
-            finish(asBatchSurvivor: asBatchSurvivor)
-            return true
-        }
+    /// The save half of a close, shared by the tab close, the window close and the batch close so
+    /// the three cannot drift on what Save means. Returns whether the caller may go on to close.
+    ///
+    /// False comes back for exactly one case: user and role changes can only be applied after the
+    /// SQL is reviewed, so Save opens the review sheet and stands the close down. Falling through
+    /// there would close over the sheet and destroy every staged change.
+    func saveSelectedTabWork() async -> Bool {
+        guard let coordinator = 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
-        // destroy every staged change.
         if isUsersRolesTab, coordinator.usersRolesActions?.hasChanges() == true {
             coordinator.usersRolesActions?.reviewAndApply()
             return false
@@ -630,7 +684,6 @@ final class MainContentCommandActions {
         // Structure view saves via direct coordinator call
         if coordinator.tabManager.selectedTab?.display.resultsViewMode == .structure {
             coordinator.structureActions?.saveChanges?()
-            finish(asBatchSurvivor: asBatchSurvivor)
             return true
         }
 
@@ -639,30 +692,33 @@ final class MainContentCommandActions {
             || !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
         }
 
+        return true
+    }
+
+    private func saveAndClose(asBatchSurvivor: Bool?) async -> Bool {
+        guard coordinator != nil else {
+            finish(asBatchSurvivor: asBatchSurvivor)
+            return true
+        }
+        guard await saveSelectedTabWork() else { return false }
         finish(asBatchSurvivor: asBatchSurvivor)
         return true
     }