Skip to content

fix(connections): synchronize background connection pane state - #2547

Merged
datlechin merged 2 commits into
TableProApp:mainfrom
devy1540:feat/2545-background-pane-sync
Aug 27, 2026
Merged

fix(connections): synchronize background connection pane state#2547
datlechin merged 2 commits into
TableProApp:mainfrom
devy1540:feat/2545-background-pane-sync

Conversation

@devy1540

@devy1540 devy1540 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Purpose

Closes #2545

Fix the case where a workspace that finishes connecting in the background remains stuck on the Preparing the session screen when selected later.

Changes and intent

  • Refresh the persistent panes owned by a background workspace after updating it to the final connection phase.
  • Keep the existing applyPhase() path for the selected workspace.
  • Preserve the normal workspace-switching behavior, which swaps existing hosting controllers without rebuilding editor, grid, scroll, selection, or undo state.
  • Add an AppKit and SwiftUI regression test that mounts two workspaces in a real NSWindow, completes the background connection, selects it, and verifies that MainContentView.onAppear activates the coordinator and installs command actions.
  • Record the fix in the Unreleased Fixed section of CHANGELOG.md.

Success criteria

  • Confirmed that the regression test failed before the production change at coordinator.isActivated and commandActions.
  • Confirmed that the same regression test passes after the fix.
  • Passed 52 adjacent tests across six suites covering connection phases, pane resolution, workspace registries and containers, and editor teardown.
  • Passed all 13,126 TableProTests in 1,517 suites with -testLanguage en -testRegion US.
  • Passed the macOS Debug build.
  • Passed SwiftLint with zero violations on the changed Swift files.
  • Passed audit-refactor-health and git diff --check.
  • Completed independent quality, security, UI contract, challenge, and synthesis reviews with no blocking findings.

A full run under the local Korean system locale exposed 153 existing assertions that compare localized output directly with English strings. The same full test target passed when the test language and region were pinned to English. The focused regression test and adjacent suites also passed independently.

@devy1540
devy1540 force-pushed the feat/2545-background-pane-sync branch from a6befa2 to c3215fc Compare August 27, 2026 03:43
@devy1540 devy1540 changed the title fix(connections): 백그라운드 연결 패널 상태 동기화 fix(connections): synchronize background connection pane state Aug 27, 2026
@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

Copy link
Copy Markdown
Member

Thanks for the report and the diagnosis, both were exact. I pushed a commit on top that keeps your fix's behaviour and moves it to the ownership boundary, plus a second bug the investigation turned up in the same place.

Why more than the one line

refreshPanes(of:) in the unselected branch does fix all five transitions. It works by repainting a second time after the first paint drew the wrong phase, so reconcileStatus still paints before it writes the phase. Two things fall out of leaving that in place.

A workspace can reach the window with no phase change to follow at all. adoptWorkspace builds one, resolves .connected when the session already has a driver, and inserts it; startActivationConnectIfNeeded returns early because allowsActivationConnect(.connected) is false, so no transition ever fires and nothing assigns a rootView. The panes keep the empty view from WorkspacePanes.init, and reconcileStatus returns at its alreadyRendered guard on every later status event, so it never heals. Opening a second connection into a window that is already on screen shows a blank sidebar, grid and inspector. That is reachable through WindowManager.openTab, so through the MCP server, a deeplink, or the connection switcher.

And a session can be replaced under an unchanged phase. A database switch on a reconnect-based engine hits reconcileStatus with nextPhase == workspace.phase == .connected, which transition's phase != next guard skips.

What the commit does

Nothing paints before the phase is final. reconcileStatus adopts or releases and stops there, releaseSession no longer paints, and transition(to:for:) is the single sink: it writes the phase, then syncs the panes of the workspace it names, selected or not, and on an unchanged phase too. Only the window's own chrome stays gated on the phase actually moving.

WorkspacePanes records a WorkspacePaneRenderKey of what its four root views were last built from: the resolved pane, the connection record, and a session revision that ConnectionWorkspace bumps whenever its session is replaced. It holds a revision rather than the session so a released driver is not kept alive by the record. refreshPanes is the only writer.

That key is what lets applySelectedWorkspace sync on every switch, which is what fixes the blank-pane case. Without it that call has to be unconditional, and then every connection switch re-evaluates MainContentView and SidebarView, which is the work the per-connection panes exist to avoid.

Measured, because the fix depends on it

Probes built with swiftc against the macOS 14 target, since none of this is documented:

  • A rootView write on a detached hosting controller is deferred, not lost. The last value written is what mounts when the pane is re-parented, and @State, NSTableView scroll offset and selection, and NSTextView undo all survive it.
  • Every pane builder erases one _ConditionalContent, so the type inside the AnyView is stable and only the branch changes. 30 identical-branch rewrites on a mounted pane cost 0 view creations and 0 teardowns. 8 background workspaces cost 0.0141ms.
  • Two writes in one run-loop turn coalesce to the last one. Split across turns, the intermediate mounts and tears down. So layoutSubtreeIfNeeded must stay out of the repaint path; WorkspacePanes.teardown() needs it only because its panes are never parented again.

CLAUDE.md said the opposite of the first and third points, and WorkspacePanes blamed AnyView for the old cost. Both are corrected in the commit, and the workspace-phase invariant now records this rule.

Tests

Your test is kept and the suite is extended to six cases, each asserting the final phase and the cached render state together: background connect succeeds, connect fails, session lost, reconnect, repeated connected events, and a workspace adopted with a live session.

Reverting only the sync in transition fails 5 of the 6; with it, 6 of 6 pass. 82 cases pass across the nine neighbouring suites (ConnectionWorkspaceRegistry, ConnectionWorkspaceContainers, ConnectionWindowPaneResolver, ConnectionWindowPhaseMachine, MainSplitViewControllerDetailWidth, SplitPaneHoldingPriority, WindowTitleResolver, EditorLifecycleTeardown, and the pane synchronization suite). macOS Debug build passes, SwiftLint reports 0 violations on the changed files.

An adversarial review caught two regressions in my own first draft, both fixed here:

  • Narrowing the adoption guard to content equivalence alone dropped the phase condition, so a recovered tunnel would have left the workspace holding the driver the recovery had just disconnected, and that driver's cached password. isContentViewEquivalent answers whether the views draw the same, not whether it is the same session. The reconnect test now asserts the replacement driver is adopted.
  • Routing the connection-record change through the key skipped the repaint when a connection is deleted while its session is still open, which purges the per-connection registries without changing the record. That path keeps its unconditional repaint, and it is the one place that skips the key.

No screenshots: the bug needs two connections and a background connect completing while another is selected, which is not something a still frame shows. The regression suite is the evidence instead.

@datlechin
datlechin merged commit 11ae3b3 into TableProApp:main Aug 27, 2026
7 checks passed
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.

Background connections keep showing ‘Preparing the session’ after they have connected

2 participants