[gmaclennan]

Stacked on #1353, which it builds on; the diff here is the last five commits.

Leaving the Exchange screen while an upload to the archive is still running cuts the websocket straight away, and the archive is left holding a partial original until the phone comes back and the upload resumes. The screen calls disconnectServers() on blur, and that closed every server websocket at once whatever was in flight, including the auth and config namespaces. digidem/comapeo-mobile#2213 first tried to make the screen decide when to disconnect, but the screen only has derived UI state and the decision belongs with the thing that knows whether sync has finished.

With this change disconnectServers() means "disconnect from servers once sync with them has finished". What counts as finished depends on the level the peer is at: while data sync is wanted, every non-blocked namespace must be open with nothing to transfer either way; otherwise the presync namespaces (auth, config, blob index) must be. A namespace the server has not opened yet is not finished, because the peer has been set to a level that includes it. The bound is the same as finishing after stop(): if the peer's want and wanted have not changed for 15 s it is closed anyway, and the timer restarts on any change, so a slow server that is still making progress is never cut. A connection that has no peer yet, because the handshake has not completed, is closed straight away since nothing has started. connectServers() cancels a pending disconnect and keeps a connection that is still open, so blurring and refocusing the screen keeps the socket. A server that we are finishing with after stop() is held for the data namespaces by #1353 and then closed here, which is the Exchange screen flow: the user presses stop and leaves, and the upload completes before the socket goes.

While data sync is still enabled, "finished with the server" can keep extending if the archive receives new data from other devices. That is real transfer rather than a stall, but it means a blur during a busy period holds the socket for longer. A full stop (app backgrounded) does not touch server websockets, as before.

The implementation keeps the protomux alongside each websocket so the connection can be matched to its PeerSyncController, and sweeps pending disconnects at the end of every #updateState(). The finished check takes the sync level that #updateState() has just set for the peer, exposed by a new getter on the controller, rather than what the controller has already opened, because the sweep runs before the controllers react to the same state event. The first version decided from remote status alone and treated a namespace that read stopped as finished; an independent review showed that closes the socket at the first intermediate synced state when the disconnect lands before the server's namespaces have all opened, after presync with data sync wanted or after auth with data sync off, and the rest never uploads. The window is from connect until the next namespaces open, a second or more. A pending status that records "enabled on our side, not yet open" would make this a property of the state rather than of the level, and is proposed separately in #1362. A third commit guards the socket close handler so a reconnect that replaced the socket for the same url is not dropped from the connection map. Nothing new is added to the sync state; the server's entry in remoteDeviceSyncState disappears when its socket closes, and data.isStopping covers the stop case.

Tests add a pausable websocket relay for the manager's makeWebsocket option so a server connection can be held mid-transfer against the in-process test server. Ten cases: an upload finishes before the socket closes; connectServers() cancels the disconnect; a stalled server is dropped after the idle timeout; stop then disconnect finishes the transfer first; config syncs to the server before disconnecting when data sync is off; a disconnect during presync still syncs the data that start() asked for; a disconnect while auth is still syncing finishes the config first; start() while a disconnect is pending syncs the data too; autostop while pending; and a full stop while pending. The three window tests fail on the first version of the check. The stall test uses fake timers but settles in real time after each advance and waits only briefly for the close, so a 5 s or a 25 s timeout fails it; the earlier version passed with both. The existing server tests pass unchanged.

Related changes: digidem/comapeo-mobile#2213 now only swaps the screen's direct api calls for the core-react mutation hooks and goes back to an unconditional disconnect on blur, so the behaviour arrives with the next core release. digidem/comapeo-cloud#78 closes a server-side socket that has gone silent, #1359 proposes closing the older connection when a device reconnects, and #1362 proposes the pending status.

[gmaclennan]

Two more commits after an independent review of the first version.

The review showed the finished check was wrong in a window I had taken to be a few milliseconds: a namespace the server has not opened yet reads stopped, which counted as finished, so a disconnect that landed before the namespaces had all opened closed the socket at the first intermediate synced state. With data sync wanted that was after presync, with the data never uploaded; with data sync off it was after auth, with the config never uploaded. Both reproduced every time, and the window runs from connect until the next namespaces open. 1d08c84 decides from the level #updateState() has just set for the peer instead, so a namespace at that level that is not open is not finished; the idle timer still bounds a server that never opens one. #1362 proposes a pending status so this can live in the state rather than in the level.

The review also showed the stall test did not pin the 15 s bound: it passed with the timeout set to 5 s and to 25 s. dffd5c3 adds the two reproductions and three more cases (start(), autostop and a full stop while a disconnect is pending), and restructures the stall test so that both of those values fail it. The same structure is used by the stall test in #1353; I will fix that there.