Skip to content

refactor(coordinator): give a window a registry of connection workspaces - #2097

Open
datlechin wants to merge 5 commits into
mainfrom
worktree-single-window-connections
Open

refactor(coordinator): give a window a registry of connection workspaces#2097
datlechin wants to merge 5 commits into
mainfrom
worktree-single-window-connections

Conversation

@datlechin

Copy link
Copy Markdown
Member

Why

A user opened two connections and got two separate windows, each showing the full workspace rail listing every open connection, and asked whether they could be merged into one window.

That is not a stray bug. Since 0.64.0 every connection gets its own window, and WindowManager.tabbingIdentifier(for:) keys the native tab group on the connection id, so two connections can never share a window. Every window still renders the rail listing all of them, so the app names all your connections in N places and makes you window-switch between them. No mature client does both: tools with a persistent connection sidebar have one window, tools with a window per connection put the connection list behind an on-demand switcher.

The tell that this was already known: TabWindowController.frameAutosaveName is one shared slot for every connection, so the windows stack and a rail click "reads as the window changing content rather than a different window being raised". That illusion survives one interaction. Every window writes that key on resize and move, only new windows read it back, and recomputeWindowMinSize() resizes windows on its own.

This is the first of three PRs moving the app to a single-window model. It is scaffolding, with no user-visible change.

What this changes

The connection was bound into the object graph at the wrong layer. MainSplitViewController fixed its connection at init and held the session, phase, and panel state as scalars, because a window served exactly one connection for its whole life.

  • ConnectionWorkspace holds everything a window needs to present one connection: phase, attempt token, session, session state, right-panel state.
  • ConnectionWorkspaceRegistry holds the set of connections a window hosts and which one it shows, with ordering, neighbour selection on removal, and wrap-around cycling.
  • MainSplitViewController reads those fields through the registry instead of storing them. Workspace construction moved out of init into adoptWorkspace(payload:autoConnect:), so a window can call it more than once.
  • Status reconciliation fans out over every hosted connection instead of only the visible one. A background workspace that loses its session still reaches the right phase, so switching to it later cannot show content for a connection that is already gone.
  • WindowManager.connectionIdsRetainingRestoreIntent() collects per workspace rather than per window.

The registry holds one entry throughout this PR, so behaviour is identical to before.

Two decisions worth reviewing

A late connect attempt discards itself. finishAttempt now takes the connection id and returns early when the registry no longer holds that workspace. The missing entry is the generation check, so a connect that outlives a connection the user closed cannot write a phase back and resurrect it. transition(to:for:) repaints only when the target workspace is the one on screen.

Each workspace owns its undo stack. NSWindow.undoManager was the right source while a window meant one connection. One window shared between connections would let Cmd+Z in one roll back an edit made in another, so ConnectionWorkspace carries its own UndoManager. DataChangeManager still routes through the window for now and moves over in PR 2, when a window can actually hold more than one connection.

Testing

ConnectionWorkspaceRegistryTests, 10 cases: insertion order and selection, idempotent insert, phase and attempt-token isolation between workspaces, distinct undo managers, neighbour selection on removal, clearing selection when the last workspace goes, ignoring selection of an unhosted connection, a removed workspace being unreachable, and cycling in both directions.

No UI automation, because this PR has no user-visible behaviour to drive. The UI automation that would have caught the original report lands in PR 3, which is where two connections start sharing a window.

Build succeeds and swiftlint lint --strict is clean.

Not in this PR

The TabWindowController and MainSplitViewController renames the design called for are left out. They touch around 40 files with no behaviour change and would bury this diff. Worth doing as separate cleanup.

Next

  • PR 2: editor tabs stop spawning windows. A custom tab strip on NSTabViewController with tabStyle = .unspecified, newTab() appending to the existing tab manager, and the four window-reconciliation files deleted.
  • PR 3: connections consolidate into one window. The rail switches content in place, the frame hack goes, and window-level tabbing becomes native and shared so Merge All Windows and drag-out keep working.

@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.

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