Skip to content

fix(connections): keep the connections strip and its commands when a connection is down - #2551

Merged
datlechin merged 1 commit into
mainfrom
fix/connection-window-chrome-and-status
Aug 27, 2026
Merged

fix(connections): keep the connections strip and its commands when a connection is down#2551
datlechin merged 1 commit into
mainfrom
fix/connection-window-chrome-and-status

Conversation

@datlechin

Copy link
Copy Markdown
Member

A window whose selected connection is not connected hides the connections strip and disables every menu route to the other connections it hosts. Those connections stay open, with unsaved editor state in them, and the only ways left are Cmd+W, File > Close Connection, or another window's strip.

Found while investigating #2545.

Why the strip goes

ConnectionWindowPaneResolver.hidesChrome is right that an object browser with no session in it is an empty column, and hideWindowChrome spells that as collapsing the sidebar split item. The connections strip lives in that same item, because AppKit grants full-height sidebar layout to exactly one leading sidebar and NavigationSidebarViewController's own comment records that two items was tried and rejected. So a rule about the object browser takes the window's connection switcher with it.

The strip is not per connection. It lists every connection the window hosts, and switching through it needs no session and no coordinator.

What else was dead

Three separate things, each with the same shape: a window-level capability read off the selected connection.

MainContentCommandActions.showPreviousWorkspace, showNextWorkspace, toggleWorkspaceRail, canToggleWorkspaceRail and isWorkspaceRailEnabled all bounced straight back to coordinator?.splitViewController, so the round trip through the coordinator added nothing but a nil failure mode. menuValidationContext returned an all-false context whenever commandActions was nil, which disabled Show Next Connection and Show Previous Connection along with it, and the actions were no-ops anyway.

TabRouter.openConnection fronts the window and then returns at guard activeSessions[id]?.driver == nil. For a connection that is already up, that is every time, so Manage Connections re-fronted a window still showing a different connection and stopped there.

The fix

ConnectionWindowPaneResolver.sidebarChromeMode(for:hasRail:) is a new pure function returning revealed, railOnly or hidden. hidesChrome keeps its answer and its reasoning for the object browser and the inspector.

In railOnly the sidebar is narrowed rather than collapsed: minimumThickness and maximumThickness both go to railAllowance, inside the autosaveName = nil span that already exists, because a clamp writes its width into the autosave record exactly as a collapse writes the collapsed flag. canCollapse goes to false for the span, since a divider double-click sees no menu validation. applySidebarMinimumThickness is inert while clamped, because seven other call sites reach recomputeWindowMinSize and any of them writing the minimum back would leave a minimum above the maximum. The rail's own width is a setting, so onLayoutChange reapplies the clamp.

ChromePaneLayout is captured on the way out of revealed and never again, so a railOnly to hidden step cannot overwrite the user's width with the clamp.

isSidebarCollapsed now answers "the object browser is off screen", which is what its five readers were asking. That keeps the toolbar's segment unlit and the menu title correct, and setSidebarTab, focusSidebarSearch and presentDatabaseFilter are refused while narrowed.

The sidebar segment is disabled when there is no object browser to toggle. The View menu's switching commands act on the registry directly. TabRouter selects the native tab, then the hosted connection.

SidebarContainerViewController's search-field insets drop to .defaultHigh: they are a margin, and at width 0 a required pair is unsatisfiable.

Measured

AppKit is not documented on this, so it was probed with swiftc against the macOS 14 target (Xcode 27.0 / 27A5237l):

  • Clamping minimumThickness == maximumThickness == 52 and later releasing returns the item to its user width, 420, with or without an autosave name.
  • With the name set the clamp writes 52 into the record, exactly as a collapse writes its flag. Hence the existing autosaveName = nil span.
  • A setPosition during the clamped span discards the width: 420 becomes 280. Nothing here calls it.
  • At width 52 the item keeps behavior = 1, allowsFullHeightLayout = true, its NSGlassEffectView and safeAreaTop = 66, so sidebar material and full-height layout survive the clamp.

Not done

Switch Connection in the toolbar stays disabled without a coordinator. Its popover is presented by the coordinator's ToolbarSwitcherPresenter, so enabling it would give a live-looking button that does nothing. The connections strip and the View menu are the routes that survive, and both work now.

Rail visibility still comes from the app-wide WorkspaceRailStore.entries.count. Narrowing it to this window's own workspace count is a different quantity, not a smaller one: two windows each hosting one connection would lose their strips. That is a real question and it is left alone here.

Tests

ConnectionWindowChromeTests is new and builds a real MainSplitViewController in an NSWindow with two workspaces, the harness #2545 established. ConnectionWindowPaneResolverTests gains the pure cases.

Covered: the strip survives an unavailable selected connection; a lone connection still hides its sidebar outright; the strip arriving and leaving while the connection is down moves the mode both ways; a narrowed sidebar refuses setSidebarTab and refuses user collapse; the clamp tracks the rail allowance; reveal restores the user's sidebar across both hidden modes; and the View menu's switching validates with no coordinator.

Making sidebarChromeMode ignore hasRail fails 5 of them. 110 cases pass across the chrome, resolver, three toolbar-validation, pane-synchronization, workspace-registry, phase-machine, detail-width and split-pane suites. macOS Debug build passes; SwiftLint reports 0 violations on the 11 changed files.

No TableProUITests coverage: no suite opens two connections into one window today, and nothing drives a connection down deterministically. That gap is why the new suite drives the controller directly instead.

Review

A Codex review of the branch found five issues, all fixed here: an existing toolbar-validation test broken by the new context field; Switch Connection enabled without a working action, which is why it stays disabled above; makeKeyAndOrderFront not selecting a background native tab; canCollapse left true in railOnly; and the clamp not tracking a rail row-size change.

Before / After

No screenshots. The state needs a window hosting two connections with the selected one down, which is what the new suite constructs; a still frame of a narrowed sidebar does not show that the strip is reachable, which is the whole claim.

@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 afad18d into main Aug 27, 2026
8 checks passed
@datlechin
datlechin deleted the fix/connection-window-chrome-and-status branch August 27, 2026 06:17
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