fix(welcome): deliver New Connection and the File menu commands without a welcome window (#1975)

[datlechin]

Fixes #1975.

The bug

With a connection open, File > New Connection... and ⌘N did nothing. The menu item stayed enabled and inert, which the HIG explicitly rules out: an item that cannot act must be dimmed.

Root cause

New Connection does not open a window directly. It asks the welcome window to present the database-type chooser, over an unbuffered Combine PassthroughSubject whose only subscriber is WelcomeWindowView:

TableProApp.swift → WindowOpener.openConnectionForm() → WindowOpenerBridge → AppCommands.presentDatabaseTypeChooser.send(payload)

TabRouter.closeWelcomeWindows() genuinely closes the welcome window on every successful connect, from eight call sites. So "no welcome window alive" is not an edge case, it is the steady state as soon as you open one connection. The event reached zero subscribers and was dropped. The reporter's "2 windows" detail is incidental; one is enough.

Why this is a refactor and not a one-line fix

Five sibling File-menu items worked around the same fault by calling WindowOpener.openWelcome() immediately before .send(...). Commit b830044 added that prefix to all five and missed New Connection, so the workaround was already forgotten once and is still unreleased. A sixth hand-added prefix would be the third occurrence of the same footgun, waiting for a seventh command to forget it.

The guarantee had to move off the call site. WelcomeRouter already implements the correct buffered pattern for this: stash a pending value, open the window, and let WelcomeViewModel drain it on mount. That is how routeImport, routeShare, routeError and routePluginInstall already work, with no ordering dependency and no live-subscriber requirement.

What changed

• WelcomeRouter gains a WelcomeRequest slot with route(_:) and consumePendingRequest(), covering all six commands. It is a separate slot from the four existing ones, drained first because it is the only user-invoked one, and last-wins on a single slot rather than a queue: these are mutually exclusive one-shot menu intents.
• The six PassthroughSubjects are deleted from AppCommands, and the five menu items lose their paired openWelcome() call.
• The chooser and URL-import presentation state moves from WelcomeWindowView @State onto WelcomeViewModel. This is load-bearing, not polish: the drain runs in the view model and cannot reach view-local state. It also moves the "is the driver installed" branch out of the view.
• DatabaseTypeChooserPayload becomes Identifiable, which lets the WelcomeChooserState wrapper disappear.
• AppServices gains welcomeRouter so the router is injectable, matching how the tests already inject storage.
• The observation loop now drains at the top of each iteration instead of only after a change. That closes a real gap: the loop's Task arms asynchronously, so a request routed between setUp() and the task arming was previously invisible until some later mutation.

Net effect on app code is a simplification: 154 lines deleted against 374 added, most of the additions being tests.

Behaviour

⌘N brings the welcome window forward with the database chooser on it, which is what the welcome screen's + button already does. That matches Postico, where ⌘N brings the favourites window forward, and TablePlus. The shortcut itself is unchanged, so docs/features/keyboard-shortcuts.mdx needs no edit.

Tests

• WelcomeRouterTests: request survives with no subscriber, consume-once, last-wins, payload round-trip, independence from the other pending slots.
• WelcomeViewModelTests: the cmd+n not working #1975 regression in both directions, welcome window closed and welcome window already open, plus the sibling commands and the drain ordering against a background plugin install.
• WindowOpenerTests: New Connection queues a chooser request with no bridge wired.
• NewConnectionCommandUITests: launch, close the welcome window, invoke New Connection from the menu, assert the chooser appears. This reproduces the exact failing state without needing a live connection.

All 16 unit tests pass. The UI test passes standalone in ~33s. It fails if run in the same xcodebuild invocation as the unit tests, but so does the pre-existing TableProLaunchUITests under the same conditions, on its own first assertion; UI and unit tests are separate commands in CLAUDE.md.

swiftlint lint --strict reports 0 violations across 1211 files.

Flagged, not fixed

• WindowOpener caches openWindow closures captured from per-window SwiftUI environments and never resets isWired. This is latent, not the active cause: AppDelegate.windowWillClose and the dock menu both call openWelcome() successfully from that detached state. Reworking the menu to read @Environment(\.openWindow) in the Commands body is behaviour-neutral today and carries real regression risk.
• consumePendingRouterActions() early-returns after the first non-nil slot, so two slots filled in one batch can starve one. Pre-existing, and contained: the only two that could race are in mutually exclusive branches. Draining all slots per pass risks colliding sheet presentations, so it belongs in its own change.

[chatgpt-codex-connector]

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