Skip to content

feat(tabs): move an editor tab into a window of its own - #2572

Merged
datlechin merged 1 commit into
mainfrom
fix/editor-tab-detach-to-window
Aug 29, 2026
Merged

feat(tabs): move an editor tab into a window of its own#2572
datlechin merged 1 commit into
mainfrom
fix/editor-tab-detach-to-window

Conversation

@datlechin

Copy link
Copy Markdown
Member

Draft. Do not merge yet. Two Codex rounds have taken this from 15 findings (8 P1) to 8 (3 P1), and the three that remain are data-loss or broken-state class. They are listed in full below. TableProUITests also cannot run on this machine, so none of it has been exercised end to end.

Follow-up to #2571, which gave the tab strip's pointer input a single AppKit owner and left canTearOff answering false. This makes it answer for real.

Fixes the second half of #2438.

What a connection hosted by two windows needs

Detaching a tab means one connection is visible in two windows. That state existed before 0.65.0 and the machinery is still here, so most of this is teaching the parts that assumed one window to ask for all of them:

  • WindowManager.workspaces(for:) and coordinators(for:) return every hosting window. workspace(for:) stays as the first of them, and its two callers are corrected.
  • MainContentCoordinator.allTabs(for:) unions across hosting windows and dedupes by tab id. It previously answered from the first window found, so a new query was numbered after tabs it could not see.
  • aggregatedTabs(for:) dedupes, because the saved set is the union and writing a partial list over a full one is how tabs that were never closed get erased.
  • TabPersistenceCoordinator is now one instance per connection, not one per coordinator. hasObservedTabs is the gate that stops a partial write, and a second instance starts with it closed: the detached window's saves would all have been withheld, and whichever instance the periodic save elected decided whether anything reached disk at all.
  • clearForUserClosedAllTabs refuses while any window still holds tabs.
  • ConnectionCloseAction detects and saves unsaved work in every window, not the first.
  • WorkspaceCloseAction gathers a container's tabs from every window, closes each through the coordinator that owns it, and runs the closing lifecycle on every workspace.

The move

WindowManager.openTabInNewWindow(connectionId:tabId:) resolves the workspace that actually holds the tab, builds a SessionState carrying just that tab, hands the tab's live TabSession to the new registry so the rows it already loaded survive, registers it as pending, and opens a standalone window with .openContent (.restoreOrDefault would fill the new window with the whole saved set). The tab leaves the source manager only once the window exists.

EditorTabDetachPolicy is the gate: more than one tab, nothing unsaved, nothing in flight, connection up. The command sits on the AppKit contextual menu, the SwiftUI menu and the accessibility actions, so the keyboard and VoiceOver reach it too.

A detached window shows no tab strip, because showsTabStrip hides at one tab the way Safari hides its tab bar. That is deliberate, and it is why the UI tests identify the detached window by title.

Still open, and why this is a draft

  • [P1] The detached window's close bypasses the save gate. I had closeFrontmostTab return false so AppKit closes the window rather than emptying it. Codex found that this routes around closeTabAwaiting, discarding live edits with no Save/Cancel and skipping Recently Closed Tabs, and CLAUDE.md requires Cmd+W to stay on the closeTab() route. I reverted it, so the current behaviour is the lesser bug: closing a detached window's last tab leaves an empty window, and closing again takes the connection down in both. Needs an async route through the normal close primitive.
  • [P1] WorkspaceCloseAction still classifies victims with one coordinator. Closing is routed to the owning coordinator now, but confirm(victims, coordinator:) still asks the first one, so a foreign victim with live grid or DDL edits reads as clean and is closed without a prompt.
  • [P1] A moved table tab is not initialized in its destination coordinator. The rows transfer and the grid renders, but the destination was built with payload: nil and selectedTabId never changes after mount, so toolbarState.isTableTab stays false and changeManager keeps empty table, column and primary-key metadata: Find and Filter are disabled and later edits cannot save correctly.
  • [P2] The busy gate is too narrow. isExecuting is false during Fetch All and exact row counts; the tab would arrive permanently busy. Needs TabExecutionRegistry.isBusy plus pagination state.
  • [P2] The connected gate reads the driver handle, which CLAUDE.md says is not evidence of health. Should read ConnectionSession.reportedStatus or liveness.
  • [P2] host(for:) still returns the first owner, so a Create Table started in the detached window can land in the original one.
  • [P2] canMoveToNewWindow / moveToNewWindow are ambiguous for a split connection.
  • [P3] A moved query tab loses its caret and selection (cursorPositions live on the source coordinator).

Verified

  • build PASS.
  • test PASS, 64 cases: the new EditorTabDetachPolicyTests plus every EditorTab* suite and TabDiskStateDecodingTests, which owns the persisted-tab decoding this change touches.
  • swiftlint lint --strict clean over the whole worktree. It also fixes two violations that reached main with refactor(tabs): give the editor tab strip's pointer input a single AppKit owner #2571: a let _ = discard and two ACL-above-parent declarations.
  • Both docs scripts clean.
  • Reviewed by Codex twice. Round one raised 15 findings, 8 of them P1; round two raised 8, 3 of them P1. Everything closed between the rounds is in the list above; everything still open is in "Still open".

TableProUITests did not run. Every case fails at launch with "The sample database never finished opening", on this branch and on a worktree at clean main alike, and the app launched by hand copies the sample then sits idle at 0% CPU with no window. EditorTabDetachUITests is written and committed; it has never executed.

Given three open P1s and no end-to-end run, my recommendation is that this stays a draft until the UI harness works and the three are closed.

https://claude.ai/code/session_01L7uaHbJBPV1LaWL5QXzxyp

@mintlify

mintlify Bot commented Aug 29, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
TablePro 🟢 Ready View Preview Aug 29, 2026, 11:33 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@datlechin
datlechin merged commit d139f77 into main Aug 29, 2026
9 checks passed
@datlechin
datlechin deleted the fix/editor-tab-detach-to-window branch August 29, 2026 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant