Skip to content

fix(tabs): give every window of a connection its own tabs back on reconnect - #2067

Merged
datlechin merged 2 commits into
mainfrom
feat/reconnect-tab-restore
Aug 10, 2026
Merged

fix(tabs): give every window of a connection its own tabs back on reconnect#2067
datlechin merged 2 commits into
mainfrom
feat/reconnect-tab-restore

Conversation

@datlechin

Copy link
Copy Markdown
Member

Follow-up to #2056, which shipped Disconnect with a documented caveat: a connection with several windows open came back with every window empty. This removes the caveat, and in doing so makes the window position saved with every tab actually mean something.

Why every window came back empty

Restore was "one window fans out into N": the window that claimed it kept persisted tab 0 and opened a brand-new window for every other tab. To stop two windows both doing that, handleRestoreOrDefault stood down whenever hasOtherWindows was true. On a reconnect that guard is true for all of them, because the windows never closed, so every window stood down and nobody restored.

The data needed to do better was already on disk and already ignored. aggregatedTabs stamps each tab with windowGroupIndex on every save, but QueryTab has no such field, QueryTab.init(from:) drops it, and restoreFromDisk returns a flat array. Nothing had ever read it back. So restore also flattened a window that held two tabs into two windows, and filled a window that was empty with the next window's tab, at launch as well as on reconnect.

The rule

Each window asks where it sits in its own native tab group and claims the saved group at that position. Nothing arbitrates. That matters because the N windows all react to one session-return broadcast, each subscribed with .receive(on: RunLoop.main), and their SwiftUI .task bodies contain awaits and interleave in no defined order. Any design where one window has to act before another would be racing. Here each window's answer depends only on its own position and on immutable file content, so order stops mattering.

Only the window at position 0 opens windows, and only for groups whose position is past the last open window. That is a structural property rather than a protocol: exactly one window computes position 0, so two windows cannot both reopen the same saved group. WindowGroupAssignmentTests.onlyLeftmostWindowFansOut is the test that pins it.

With one window the rule reduces to what launch already did: position 0 keeps group 0 and everything past the only window is reopened. Launch and reconnect are now the same code path.

Save and restore had to agree, and did not

Both halves describe a window by its position, so they have to count the same windows. They did not. aggregatedTabs numbered coordinators, ordered by window position but skipping any window without one, while the restore side would number positions in tabbedWindows, which counts every window. A window with no live session has no coordinator and still occupies a tab, so with three windows whose middle one had lost its session, save wrote positions 0 and 1 while restore read 0 and 2: the third window's tabs would land in the second window and the third would get nothing.

WindowTabGroupOrder is now the single definition both use, and aggregatedTabs stamps the window's real position in the tab group rather than its rank among coordinators. This was the reason to share the primitive; tidiness was not.

Files saved before tabs recorded their window

Those carry no grouping at all. Reading every tab as position 0 would pile them into one window, and since there is no in-window tab bar, all but one would be invisible: tabs the user could no longer reach. Instead normalizedGroupIndices gives each tab its array position when no tab records one, so an old file restores exactly the way it always has, through the same single code path rather than a second algorithm kept alongside.

Focus

A window being created by this restore pass can be brought to the front, because nothing has focus yet: that is launch, and RestoreWindowPlan still decides which group wins, now per group rather than per tab, since a group can hold several tabs.

Once siblings already exist, restore never moves focus. The user is looking at one of these windows and a reconnect must not pull the key window out from under them. Each window decides its own eager load from its own isKeyWindow instead, which also stops a reconnect firing one query per window: only the visible one loads now, the rest defer and load when the user switches to them, through the windowDidBecomeKey path that already exists.

Other changes

  • RestoreWindowPlan.resolveFrontTabId becomes resolveFrontGroup. Keeping a tab-based front decision while groups can hold several tabs would have been a latent inconsistency. Its four tests are updated in the same commit.
  • WindowLifecycleMonitor.hasOtherWindows is deleted; the guard it existed for is gone. WindowLifecycleMonitorRegistrationTests asserted the supersede fix from feat(connections): disconnect a connection without closing its window #2056 through it, so those assertions now go through windows(for:), which states the invariant more directly: one entry per physical window.
  • The window resolved for the index comes from coordinator.splitViewController?.view.window, not viewWindow. The split view controller is wired up in adoptSession before this view is built, while viewWindow arrives from configureWindow and can still be nil at .task time.

Known limitation

Closing a window while the connection is disconnected renumbers the positions of the windows that remain, so their tabs shift by one and the last group opens as a new window. Every tab still comes back and nothing is lost, but they can be distributed differently than they were. Position is the only window identity persisted; fixing it properly means giving windows a stable identity on disk, which is its own change.

Testing

Build succeeds. swiftlint lint --strict clean across 1301 files. 108 tests pass across 9 suites, including the pre-existing persistence, restore-plan, window-monitor and multi-window suites.

Newly covered, none of which had any coverage before:

  • WindowGroupAssignmentTests: a window claims the group at its position; only the leftmost reports orphans; a group that still has a window is never reopened; orphans come back in saved order; a window with nothing saved at its position stays empty; a window that held several tabs gets all of them; the selection is reported only by the group holding it; and both legacy-file cases.
  • WindowTabGroupOrderTests: position within a group, the fallback for a window absent from its group, and a window with no tab group.
  • TabPersistenceCoordinatorTests: one added case proving window positions survive the round trip.

Not automated: the real multi-window check needs several NSWindows joined with addTabbedWindow, which depends on window-server state and is not exercised anywhere in this repo's tests today. Manual pass for review: open a connection, open three native tabs on it, Disconnect, Reconnect, and confirm each window shows the tab it had, none are blank, and focus does not jump.

@mintlify

mintlify Bot commented Aug 10, 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 10, 2026, 5:34 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.

Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
@datlechin
datlechin merged commit 34edbe2 into main Aug 10, 2026
4 checks passed
@datlechin
datlechin deleted the feat/reconnect-tab-restore branch August 10, 2026 05:42
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