recent_projects: Fix remote reconnect when server is not running (#49834) (cherry-pick to preview) (#49871)

zed-zippy[bot] and Filipe Azevedo created

Cherry-pick of #49834 to preview

----
Closes https://github.com/zed-industries/zed/issues/49363

When an existing remote workspace's connection is dead (e.g. the server
died and reconnect failed, leaving the client in `ServerNotRunning`
state), `remote_connection()` returns `None`. Previously this caused an
error dialog, blocking reconnection — the user had to manually switch to
another project and back to recover.

Now the code falls through to establish a fresh connection instead,
matching the previous behavior where clicking "Reconnect" would just
work.

Release Notes:

- Fixed remote reconnect failing with an error when the server is not
running, now establishes a fresh connection instead.

Co-authored-by: Filipe Azevedo <filipe@azevedo.io>

Change summary

crates/recent_projects/src/remote_connections.rs | 245 ++++++++++++++---
crates/remote/src/remote_client.rs               |  23 +
crates/remote/src/transport/mock.rs              |  16 +
3 files changed, 238 insertions(+), 46 deletions(-)

Detailed changes

crates/recent_projects/src/remote_connections.rs 🔗

@@ -144,57 +144,62 @@ pub async fn open_remote_project(
     .await;
 
     if let Some((existing_window, existing_workspace)) = existing {
-        let remote_connection = cx
-            .update(|cx| {
-                existing_workspace
-                    .read(cx)
-                    .project()
-                    .read(cx)
-                    .remote_client()
-                    .and_then(|client| client.read(cx).remote_connection())
-            })
-            .ok_or_else(|| anyhow::anyhow!("no remote connection for existing remote workspace"))?;
-
-        let (resolved_paths, paths_with_positions) =
-            determine_paths_with_positions(&remote_connection, paths).await;
+        let remote_connection = cx.update(|cx| {
+            existing_workspace
+                .read(cx)
+                .project()
+                .read(cx)
+                .remote_client()
+                .and_then(|client| client.read(cx).remote_connection())
+        });
 
-        let open_results = existing_window
-            .update(cx, |multi_workspace, window, cx| {
-                window.activate_window();
-                multi_workspace.activate(existing_workspace.clone(), cx);
-                existing_workspace.update(cx, |workspace, cx| {
-                    workspace.open_paths(
-                        resolved_paths,
-                        OpenOptions {
-                            visible: Some(open_visible),
-                            ..Default::default()
-                        },
-                        None,
-                        window,
-                        cx,
-                    )
-                })
-            })?
-            .await;
+        if let Some(remote_connection) = remote_connection {
+            let (resolved_paths, paths_with_positions) =
+                determine_paths_with_positions(&remote_connection, paths).await;
+
+            let open_results = existing_window
+                .update(cx, |multi_workspace, window, cx| {
+                    window.activate_window();
+                    multi_workspace.activate(existing_workspace.clone(), cx);
+                    existing_workspace.update(cx, |workspace, cx| {
+                        workspace.open_paths(
+                            resolved_paths,
+                            OpenOptions {
+                                visible: Some(open_visible),
+                                ..Default::default()
+                            },
+                            None,
+                            window,
+                            cx,
+                        )
+                    })
+                })?
+                .await;
 
-        _ = existing_window.update(cx, |multi_workspace, _, cx| {
-            let workspace = multi_workspace.workspace().clone();
-            workspace.update(cx, |workspace, cx| {
-                for item in open_results.iter().flatten() {
-                    if let Err(e) = item {
-                        workspace.show_error(&e, cx);
+            _ = existing_window.update(cx, |multi_workspace, _, cx| {
+                let workspace = multi_workspace.workspace().clone();
+                workspace.update(cx, |workspace, cx| {
+                    for item in open_results.iter().flatten() {
+                        if let Err(e) = item {
+                            workspace.show_error(&e, cx);
+                        }
                     }
-                }
+                });
             });
-        });
 
-        let items = open_results
-            .into_iter()
-            .map(|r| r.and_then(|r| r.ok()))
-            .collect::<Vec<_>>();
-        navigate_to_positions(&existing_window, items, &paths_with_positions, cx);
+            let items = open_results
+                .into_iter()
+                .map(|r| r.and_then(|r| r.ok()))
+                .collect::<Vec<_>>();
+            navigate_to_positions(&existing_window, items, &paths_with_positions, cx);
 
-        return Ok(());
+            return Ok(());
+        }
+        // If the remote connection is dead (e.g. server not running after failed reconnect),
+        // fall through to establish a fresh connection instead of showing an error.
+        log::info!(
+            "existing remote workspace found but connection is dead, starting fresh connection"
+        );
     }
 
     let (window, initial_workspace) = if let Some(window) = open_options.replace_window {
@@ -722,6 +727,156 @@ mod tests {
         );
     }
 
+    #[gpui::test]
+    async fn test_reconnect_when_server_not_running(
+        cx: &mut TestAppContext,
+        server_cx: &mut TestAppContext,
+    ) {
+        let app_state = init_test(cx);
+        let executor = cx.executor();
+
+        cx.update(|cx| {
+            release_channel::init(semver::Version::new(0, 0, 0), cx);
+        });
+        server_cx.update(|cx| {
+            release_channel::init(semver::Version::new(0, 0, 0), cx);
+        });
+
+        let (opts, server_session, connect_guard) = RemoteClient::fake_server(cx, server_cx);
+
+        let remote_fs = FakeFs::new(server_cx.executor());
+        remote_fs
+            .insert_tree(
+                path!("/project"),
+                json!({
+                    "src": {
+                        "main.rs": "fn main() {}",
+                    },
+                }),
+            )
+            .await;
+
+        server_cx.update(HeadlessProject::init);
+        let http_client = Arc::new(BlockedHttpClient);
+        let node_runtime = NodeRuntime::unavailable();
+        let languages = Arc::new(language::LanguageRegistry::new(server_cx.executor()));
+        let proxy = Arc::new(ExtensionHostProxy::new());
+
+        let _headless = server_cx.new(|cx| {
+            HeadlessProject::new(
+                HeadlessAppState {
+                    session: server_session,
+                    fs: remote_fs.clone(),
+                    http_client: http_client.clone(),
+                    node_runtime: node_runtime.clone(),
+                    languages: languages.clone(),
+                    extension_host_proxy: proxy.clone(),
+                },
+                false,
+                cx,
+            )
+        });
+
+        drop(connect_guard);
+
+        // Open the remote project normally.
+        let paths = vec![PathBuf::from(path!("/project"))];
+        let mut async_cx = cx.to_async();
+        open_remote_project(
+            opts.clone(),
+            paths.clone(),
+            app_state.clone(),
+            workspace::OpenOptions::default(),
+            &mut async_cx,
+        )
+        .await
+        .expect("initial open should succeed");
+
+        executor.run_until_parked();
+
+        assert_eq!(cx.update(|cx| cx.windows().len()), 1);
+        let window = cx.update(|cx| cx.windows()[0].downcast::<MultiWorkspace>().unwrap());
+
+        // Force the remote client into ServerNotRunning state (simulates the
+        // scenario where the remote server died and reconnection failed).
+        window
+            .update(cx, |multi_workspace, _, cx| {
+                let workspace = multi_workspace.workspace().clone();
+                workspace.update(cx, |workspace, cx| {
+                    let client = workspace
+                        .project()
+                        .read(cx)
+                        .remote_client()
+                        .expect("should have remote client");
+                    client.update(cx, |client, cx| {
+                        client.force_server_not_running(cx);
+                    });
+                });
+            })
+            .unwrap();
+
+        executor.run_until_parked();
+
+        // Register a new mock server under the same options so the reconnect
+        // path can establish a fresh connection.
+        let (server_session_2, connect_guard_2) =
+            RemoteClient::fake_server_with_opts(&opts, cx, server_cx);
+
+        let _headless_2 = server_cx.new(|cx| {
+            HeadlessProject::new(
+                HeadlessAppState {
+                    session: server_session_2,
+                    fs: remote_fs.clone(),
+                    http_client,
+                    node_runtime,
+                    languages,
+                    extension_host_proxy: proxy,
+                },
+                false,
+                cx,
+            )
+        });
+
+        drop(connect_guard_2);
+
+        // Simulate clicking "Reconnect": calls open_remote_project with
+        // replace_window pointing to the existing window.
+        let result = open_remote_project(
+            opts,
+            paths,
+            app_state,
+            workspace::OpenOptions {
+                replace_window: Some(window),
+                ..Default::default()
+            },
+            &mut async_cx,
+        )
+        .await;
+
+        executor.run_until_parked();
+
+        assert!(
+            result.is_ok(),
+            "reconnect should succeed but got: {:?}",
+            result.err()
+        );
+
+        // Should still be a single window with a working remote project.
+        assert_eq!(cx.update(|cx| cx.windows().len()), 1);
+
+        window
+            .update(cx, |multi_workspace, _, cx| {
+                let workspace = multi_workspace.workspace().clone();
+                workspace.update(cx, |workspace, cx| {
+                    assert!(
+                        workspace.project().read(cx).is_remote(),
+                        "project should be remote after reconnect"
+                    );
+                });
+            })
+            .unwrap();
+    }
+
     fn init_test(cx: &mut TestAppContext) -> Arc<AppState> {
         cx.update(|cx| {
             let state = AppState::test(cx);

crates/remote/src/remote_client.rs 🔗

@@ -1055,6 +1055,11 @@ impl RemoteClient {
         }
     }
 
+    #[cfg(any(test, feature = "test-support"))]
+    pub fn force_server_not_running(&mut self, cx: &mut Context<Self>) {
+        self.set_state(State::ServerNotRunning, cx);
+    }
+
     #[cfg(any(test, feature = "test-support"))]
     pub fn simulate_disconnect(&self, client_cx: &mut App) -> Task<()> {
         let opts = self.connection_options();
@@ -1100,6 +1105,24 @@ impl RemoteClient {
         (opts.into(), server_client, connect_guard)
     }
 
+    /// Registers a new mock server for existing connection options.
+    ///
+    /// Use this to simulate reconnection: after forcing a disconnect, register
+    /// a new server so the next `connect()` call succeeds.
+    #[cfg(any(test, feature = "test-support"))]
+    pub fn fake_server_with_opts(
+        opts: &RemoteConnectionOptions,
+        client_cx: &mut gpui::TestAppContext,
+        server_cx: &mut gpui::TestAppContext,
+    ) -> (AnyProtoClient, ConnectGuard) {
+        use crate::transport::mock::MockConnection;
+        let mock_opts = match opts {
+            RemoteConnectionOptions::Mock(mock_opts) => mock_opts.clone(),
+            _ => panic!("fake_server_with_opts requires Mock connection options"),
+        };
+        MockConnection::new_with_opts(mock_opts, client_cx, server_cx)
+    }
+
     /// Creates a `RemoteClient` connected to a mock server.
     ///
     /// Call `fake_server` first to get the connection options, set up the

crates/remote/src/transport/mock.rs 🔗

@@ -145,7 +145,21 @@ impl MockConnection {
         static NEXT_ID: AtomicU64 = AtomicU64::new(0);
         let id = NEXT_ID.fetch_add(1, Ordering::SeqCst);
         let opts = MockConnectionOptions { id };
+        let (server_client, connect_guard) =

+            Self::new_with_opts(opts.clone(), client_cx, server_cx);

+        (opts, server_client, connect_guard)

+    }

 
+    /// Creates a mock connection pair for existing `MockConnectionOptions`.

+    ///

+    /// This is useful when simulating reconnection: after a connection is torn

+    /// down, register a new mock server under the same options so the next

+    /// `ConnectionPool::connect` call finds it.

+    pub(crate) fn new_with_opts(

+        opts: MockConnectionOptions,

+        client_cx: &mut TestAppContext,

+        server_cx: &mut TestAppContext,

+    ) -> (AnyProtoClient, ConnectGuard) {

         let (outgoing_tx, _) = mpsc::unbounded::<Envelope>();
         let (_, incoming_rx) = mpsc::unbounded::<Envelope>();
         let server_client = server_cx
@@ -165,7 +179,7 @@ impl MockConnection {
                 .insert(opts.id, (rx, connection));
         });
 
-        (opts, server_client.into(), tx)

+        (server_client.into(), tx)

     }
 }