You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
On iPhone and iPad, tapping a connection opened the connecting screen, which then closed itself and returned to the connection list with no error. Tapping the same connection again kept the screen open but hung on "Connecting..." for good: no error, no Retry, only Cancel. Killing the app was the only way out.
Root cause
Two defects, and the second is reachable on its own.
The wedge.ConnectionCoordinator.connect() fenced re-entry with a bare isConnecting Bool cleared only by defer on an await that could never return. ConnectionManager.connect has no cancellation plumbing, and the connect bottoms out in blocking C calls: PQconnectdb, mysql_real_connect, and an SSH handshake that set libssh2_session_set_blocking(sess, 1) and then called libssh2_session_handshake with no timeout at all. Only the TCP leg had a bound. So when the connecting screen went away, SwiftUI cancelled its .task but nothing unwound the connect, and isConnecting stayed true forever.
Because the coordinator was cached, the next tap reused it. connect() hit guard !isConnecting and returned without touching phase, which was still its initial .connecting. The screen rendered the spinner forever. reconnectIfNeeded() could not repair it, because its own guard requires session != nil.
That path does not need the dismissal to happen. The Cancel button on the connecting screen called dismiss() and nothing else, so cancel, then tap again, wedged the connection identically.
The dismissal. The presented ConnectedView wrote the presenting ConnectionListView's @State (coordinatorCache) from its own .task, and the cover's item binding was a Binding rebuilt on every body pass over @SceneStorage and @Observable state. The reporter's A/B isolates that write as the trigger: the second tap takes the cached branch, never calls the closure, and does not dismiss.
I could not confirm the SwiftUI mechanism. Two self-driving Simulator probes reproducing the shape (computed Binding over @SceneStorage plus an @Observable array, presented view writing the presenter's @State dictionary from .task, then again with all seven sibling .sheet modifiers this view really has) both reported coverAppeared=true setterNilCalls=0 coverDisappeared=false. So the simple explanations are refuted. This change removes the write rather than depending on the mechanism.
The fix
Two ownership moves plus a real bound on the blocking calls.
ConnectionCoordinatorStore owns the coordinator per connection, replacing the presenting view's @State dictionary. ConnectedView resolves its own coordinator and never calls back into the presenter.
Attempt token and stored task replace isConnecting. A second connect() joins the attempt already running instead of returning early, and every write to session and phase is gated on the token still being current, so a late attempt discards itself.
cancelConnect() retires the token, clears the task so the next call cannot join a cancelled one, invalidates the manager's attempt, and sets a retryable error phase synchronously with the button press. It never waits on the driver, because Task.cancel() is cooperative and these drivers cannot observe it.
ConnectionManager generation fence.storeSession was unconditional, so a late orphan could overwrite the session a newer attempt had established. Adoption now checks the attempt generation and a losing attempt disconnects its own driver, matching the macOS ConnectionAttemptRegistry rule in CLAUDE.md.
libssh2_session_set_timeout bounds the handshake, auth and teardown. It was declared in the header and called nowhere in the repo.
SSHProvider.createTunnel takes a connectionId. It used to read a single unkeyed pending-id slot set just before the call, which concurrent attempts would race for, resolving another connection's SSH credentials. The side channel is gone.
Cancellable driver connects. PostgreSQL uses PQconnectStart/PQconnectPoll with an app-owned deadline, mirroring the macOS plugin. MySQL runs mysql_real_connect on its own queue behind the existing resume-once gate, and a late call closes the handle it was still using.
Each tunnel carries its own identity.IOSSSHProvider kept one tunnel per connection id, so a losing attempt's cleanup closed whichever tunnel the retry had installed. The store is keyed by tunnel now, and a losing attempt closes only its own.
The coordinator store is per scene. iPad allows several windows, and a coordinator owns the screen's tab, navigation path and connect attempt, so two windows on one connection must not share it.
Editing a connection also retires its coordinator, which fixes a separate defect found during the investigation: a coordinator pinned the values it was built from, so changing a host or port was ignored until relaunch. An explicit save always retires it, because a password lives in the Keychain and no comparison of the stored connection can see it change. Sync- and reorder-driven changes use a narrower rule: only a change to how the app dials drops the live session, so renaming, reordering, grouping and tagging cannot tear a working connection down.
Review
Codex reviewed the working tree and raised ten issues. All ten are addressed in this branch:
The attempt generation was taken after await disconnect, so resume ordering could hand a cancelled call a fresh generation. It is taken before the suspension now and rechecked after it.
mysql_real_connect ran on one static queue shared by every MySQL driver, so one wedged call blocked every other MySQL connect. Each attempt gets its own queue.
The SSH tunnel and coordinator-store findings above.
A cover already open kept a .connected coordinator over a driver that had just been disconnected. The store carries a revision and the screen re-resolves on it.
reconcile returned early when no coordinator was cached, leaving live sessions untracked.
The MySQL deadline surfaced as CancellationError, so it read as a generic cancellation rather than a timeout.
The new PostgreSQL failure strings were raw English literals.
Narrative comments were trimmed to short rationale.
Verification
iOS app builds: ** BUILD SUCCEEDED **.
TableProMobileTests: ** TEST SUCCEEDED **, 0 failures, including 3 new redial cases.
TableProDatabaseTests/ConnectionManagerTests: 13 tests pass, including 3 new ones covering the generation fence and the tunnel id.
SwiftLint: 0 new violations. The seven that remain on ConnectionCoordinator.swift are pre-existing storage_environment_defaults hits on UserDefaults.standard lines this change does not touch, present at the same set of lines on main.
New tests: an attempt invalidated mid-connect discards its own driver; a late attempt cannot overwrite a newer attempt's session; a losing attempt closes its own tunnel and not the winner's; a tunnel is opened for the connection being dialed; and the redial rules that keep a rename or a reorder from dropping a live session.
No UI automation. The mobile target has no UI test target at all (TableProMobile/project.yml declares only TableProMobile, TableProWidgetExtension and TableProMobileTests), and creating one is beyond this fix. The reporter's repro was confirmed interactively instead.
Local build note. Building the iOS app on this machine requires patching the vendored oracle-nio fork, whose @TaskLocal macro expands to @usableFromInlinenonisolated under Xcode 27. That patch was applied to a private copy of the package checkouts and is not part of this change.
Codex ran a second pass against the approach itself and returned no-ship with five findings. Four are fixed in beb4faa95:
The toolbar Connections button bypassed cancellation. It called dismiss() alone, so the connecting screen's other prominent exit left the attempt running and reopening joined the same blocked task. The original wedge was reachable through it. Both exits now retire the attempt.
Cancel after session adoption.ConnectionManager adopts and publishes the session before the coordinator's first fetchTables, so a cancel during bootstrap metadata left an adopted session that a later attempt would queue behind on the same blocked driver. cancelConnect() now calls retireSession(for:), which removes the session from lookup under the lock and tears the driver down afterwards.
A rename dropped a live connection. Wiring the credential signal in the previous round made both edit callbacks invalidate unconditionally, which bypassed the redial rule and contradicted this PR's own claim. ConnectionFormViewModel.credentialsChanged compares the loaded Keychain secrets against the edited ones, and only a real credential change retires the session now. A rename goes through reconcile, which preserves it.
A failed SSH handshake stranded its socket. The bounded handshake freed the libssh2 session but left socketFD open, and no cleanup path could reach it because the factory only closes the tunnel on host-key failure. The new timeout made that path far more reachable, so it closes the socket.
Not fixed, reported instead: the attempt generation is keyed by connection id, and every scene shares one ConnectionManager. Two iPad windows connecting to the same connection can therefore have one window's Cancel reject the other's successful attempt. That follows from one session per connection id, which predates this change (ConnectionManager.connect has always torn down the existing session for the id first). Fixing it properly needs per-scene session leases rather than a shared registry, which is a larger redesign than this fix should carry.
Verification after the round: iOS app builds, TableProMobileTests** TEST SUCCEEDED ** with 0 failures, ConnectionManagerTests 14/14, SwiftLint 0 new violations.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
On iPhone and iPad, tapping a connection opened the connecting screen, which then closed itself and returned to the connection list with no error. Tapping the same connection again kept the screen open but hung on "Connecting..." for good: no error, no Retry, only Cancel. Killing the app was the only way out.
Root cause
Two defects, and the second is reachable on its own.
The wedge.
ConnectionCoordinator.connect()fenced re-entry with a bareisConnectingBool cleared only bydeferon anawaitthat could never return.ConnectionManager.connecthas no cancellation plumbing, and the connect bottoms out in blocking C calls:PQconnectdb,mysql_real_connect, and an SSH handshake that setlibssh2_session_set_blocking(sess, 1)and then calledlibssh2_session_handshakewith no timeout at all. Only the TCP leg had a bound. So when the connecting screen went away, SwiftUI cancelled its.taskbut nothing unwound the connect, andisConnectingstayed true forever.Because the coordinator was cached, the next tap reused it.
connect()hitguard !isConnectingand returned without touchingphase, which was still its initial.connecting. The screen rendered the spinner forever.reconnectIfNeeded()could not repair it, because its own guard requiressession != nil.That path does not need the dismissal to happen. The Cancel button on the connecting screen called
dismiss()and nothing else, so cancel, then tap again, wedged the connection identically.The dismissal. The presented
ConnectedViewwrote the presentingConnectionListView's@State(coordinatorCache) from its own.task, and the cover's item binding was aBindingrebuilt on every body pass over@SceneStorageand@Observablestate. The reporter's A/B isolates that write as the trigger: the second tap takes the cached branch, never calls the closure, and does not dismiss.I could not confirm the SwiftUI mechanism. Two self-driving Simulator probes reproducing the shape (computed
Bindingover@SceneStorageplus an@Observablearray, presented view writing the presenter's@Statedictionary from.task, then again with all seven sibling.sheetmodifiers this view really has) both reportedcoverAppeared=true setterNilCalls=0 coverDisappeared=false. So the simple explanations are refuted. This change removes the write rather than depending on the mechanism.The fix
Two ownership moves plus a real bound on the blocking calls.
ConnectionCoordinatorStoreowns the coordinator per connection, replacing the presenting view's@Statedictionary.ConnectedViewresolves its own coordinator and never calls back into the presenter.Attempt token and stored task replace
isConnecting. A secondconnect()joins the attempt already running instead of returning early, and every write tosessionandphaseis gated on the token still being current, so a late attempt discards itself.cancelConnect()retires the token, clears the task so the next call cannot join a cancelled one, invalidates the manager's attempt, and sets a retryable error phase synchronously with the button press. It never waits on the driver, becauseTask.cancel()is cooperative and these drivers cannot observe it.ConnectionManagergeneration fence.storeSessionwas unconditional, so a late orphan could overwrite the session a newer attempt had established. Adoption now checks the attempt generation and a losing attempt disconnects its own driver, matching the macOSConnectionAttemptRegistryrule in CLAUDE.md.libssh2_session_set_timeoutbounds the handshake, auth and teardown. It was declared in the header and called nowhere in the repo.SSHProvider.createTunneltakes aconnectionId. It used to read a single unkeyed pending-id slot set just before the call, which concurrent attempts would race for, resolving another connection's SSH credentials. The side channel is gone.Cancellable driver connects. PostgreSQL uses
PQconnectStart/PQconnectPollwith an app-owned deadline, mirroring the macOS plugin. MySQL runsmysql_real_connecton its own queue behind the existing resume-once gate, and a late call closes the handle it was still using.Each tunnel carries its own identity.
IOSSSHProviderkept one tunnel per connection id, so a losing attempt's cleanup closed whichever tunnel the retry had installed. The store is keyed by tunnel now, and a losing attempt closes only its own.The coordinator store is per scene. iPad allows several windows, and a coordinator owns the screen's tab, navigation path and connect attempt, so two windows on one connection must not share it.
Editing a connection also retires its coordinator, which fixes a separate defect found during the investigation: a coordinator pinned the values it was built from, so changing a host or port was ignored until relaunch. An explicit save always retires it, because a password lives in the Keychain and no comparison of the stored connection can see it change. Sync- and reorder-driven changes use a narrower rule: only a change to how the app dials drops the live session, so renaming, reordering, grouping and tagging cannot tear a working connection down.
Review
Codex reviewed the working tree and raised ten issues. All ten are addressed in this branch:
await disconnect, so resume ordering could hand a cancelled call a fresh generation. It is taken before the suspension now and rechecked after it.mysql_real_connectran on one static queue shared by every MySQL driver, so one wedged call blocked every other MySQL connect. Each attempt gets its own queue..connectedcoordinator over a driver that had just been disconnected. The store carries a revision and the screen re-resolves on it.reconcilereturned early when no coordinator was cached, leaving live sessions untracked.CancellationError, so it read as a generic cancellation rather than a timeout.Verification
** BUILD SUCCEEDED **.TableProMobileTests:** TEST SUCCEEDED **, 0 failures, including 3 new redial cases.TableProDatabaseTests/ConnectionManagerTests: 13 tests pass, including 3 new ones covering the generation fence and the tunnel id.ConnectionCoordinator.swiftare pre-existingstorage_environment_defaultshits onUserDefaults.standardlines this change does not touch, present at the same set of lines onmain.New tests: an attempt invalidated mid-connect discards its own driver; a late attempt cannot overwrite a newer attempt's session; a losing attempt closes its own tunnel and not the winner's; a tunnel is opened for the connection being dialed; and the redial rules that keep a rename or a reorder from dropping a live session.
No UI automation. The mobile target has no UI test target at all (
TableProMobile/project.ymldeclares onlyTableProMobile,TableProWidgetExtensionandTableProMobileTests), and creating one is beyond this fix. The reporter's repro was confirmed interactively instead.Local build note. Building the iOS app on this machine requires patching the vendored
oracle-niofork, whose@TaskLocalmacro expands to@usableFromInlinenonisolatedunder Xcode 27. That patch was applied to a private copy of the package checkouts and is not part of this change.https://claude.ai/code/session_01XLAgkECUGM1CXAUgKbUegP