fix(welcome): stop a shared connection row from acting on the local connection

[datlechin]

Four defects in the welcome-window connection list, found while reviewing #2310 and each verified against the code before being fixed. They are grouped because they all live in the same list and the same view model. None of them are caused by #2310; the swipe actions just made me read this code closely.

A Team Library row could delete your local connection

The worst of the four, because it destroys Keychain data.

Publishing a connection to the Team Library sends sourceConnectionId: connection.id.uuidString, the pull echoes it back, and the row was built with that value as its identity. The row is tagged with it in the same List(selection:) that tags local connection rows with connection.id, so your own published connection appeared under both Connections and Team Library carrying one identity. The context menu resolves its ids against local storage only, so Edit and Delete on the Team Library copy resolved to the local connection. Confirming that delete removed the local connection along with its password, SSH password, key passphrase, SSL passphrase and TOTP secret, while the shared entry stayed on screen untouched.

Shared rows now derive their identity from the payload, using the same SHA-256 derivation linked-folder rows already use, so a shared row can never collide with a local one. primaryAction had the same collision: it iterates local connections and team library connections for the same id, so one Return could start two connects.

Opening a linked or Team Library connection did nothing at all

connectToLinkedConnection built a complete connection from the shared payload and then forwarded only its id, which was resolved against local storage. A linked-folder id is a derived UUID and a colleague's Team Library id is their local UUID, so neither is ever in storage. The lookup threw, and the error handler returns early when no window is open for that id, which is always true here because no window ever opened. The result was a double-click that produced no window, no spinner, no alert, and one info log line.

TabRouter already had the pattern for this: openTable accepts a transientConnection and calls registerPendingSession for a connection that is not in storage. openConnection now does the same, and the linked path routes through it.

A connection that could not be deleted vanished anyway

ConnectionStorage.deleteConnections bails out when the write fails, logging an error, but returned Void. The view model removed the rows regardless, so a failed delete looked like a successful one and the connection came back on the next launch. Both delete methods now report success, matching the @discardableResult -> Bool convention the rest of that file already uses, and the view model reloads from storage instead of trimming the list when the write fails.

Select All and Ctrl+J skipped favorited connections

When there are favorites and no active search, a favorited ungrouped connection is deliberately removed from the tree because it renders in the Favorites section instead. But flatVisibleConnections was derived only from the tree, and that is what Cmd+A and the Ctrl+J / Ctrl+K navigation walk. So starring an ungrouped connection quietly removed it from Select All and made it unreachable by keyboard.

It now includes the Favorites section in display order, deduplicated so a favorited connection inside a group, which is rendered in both places by design, is still visited once.

Verification

• verify.sh build: PASS
• verify.sh test WelcomeViewModelTests ConnectionStorageSyncDeleteTests: PASS, 15 cases
• swiftlint --strict on all five changed files: clean

Four new unit tests cover the keyboard reachability of favorites, the no-duplicates guarantee, the failed-delete path using a read-only directory to force the write to fail, and the stability and distinctness of derived shared-row ids.

The Team Library collision itself is not directly unit tested, because building one requires a Team-tier license and the live sync coordinator, neither of which is injectable here. The test covers the derivation the fix depends on instead. Worth a manual pass by someone with Team access: publish a connection, then Delete the Team Library copy and confirm the local one survives.

[chatgpt-codex-connector]

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