ssh remoting: Fix wrong working directory for SSH terminals (#19672)

Thorsten Ball created 1 year ago

Before this change, we would save the working directory *on the client*
of each shell that was running in a terminal.

While it's technically right, it's wrong in all of these cases where
`working_directory` was used:

- in inline assistant
- when resolving file paths in the terminal output
- when serializing the current working dir and deserializing it on
restart

Release Notes:

- Fixed terminals opened on remote hosts failing to deserialize with an
error message after restarting Zed.

Change summary

crates/project/src/terminals.rs           |  1 
crates/terminal/src/terminal.rs           | 28 +++++++++++++++++++-----
crates/terminal_view/src/terminal_view.rs |  2 
3 files changed, 24 insertions(+), 7 deletions(-)

Detailed changes

crates/project/src/terminals.rs

@@ -240,6 +240,7 @@ impl Project {
             settings.cursor_shape.unwrap_or_default(),
             settings.alternate_scroll,
             settings.max_scroll_history_lines,
+            ssh_details.is_some(),
             window,
             completion_tx,
             cx,

crates/terminal/src/terminal.rs

@@ -330,6 +330,7 @@ impl TerminalBuilder {
         cursor_shape: CursorShape,
         alternate_scroll: AlternateScroll,
         max_scroll_history_lines: Option<usize>,
+        is_ssh_terminal: bool,
         window: AnyWindowHandle,
         completion_tx: Sender<()>,
         cx: &AppContext,
@@ -469,6 +470,7 @@ impl TerminalBuilder {
             url_regex: RegexSearch::new(URL_REGEX).unwrap(),
             word_regex: RegexSearch::new(WORD_REGEX).unwrap(),
             vi_mode_enabled: false,
+            is_ssh_terminal,
         };
 
         Ok(TerminalBuilder {
@@ -626,6 +628,7 @@ pub struct Terminal {
     word_regex: RegexSearch,
     task: Option<TaskState>,
     vi_mode_enabled: bool,
+    is_ssh_terminal: bool,
 }
 
 pub struct TaskState {
@@ -734,10 +737,6 @@ impl Terminal {
         self.selection_phase == SelectionPhase::Selecting
     }
 
-    pub fn get_cwd(&self) -> Option<PathBuf> {
-        self.pty_info.current.as_ref().map(|info| info.cwd.clone())
-    }
-
     ///Takes events from Alacritty and translates them to behavior on this view
     fn process_terminal_event(
         &mut self,
@@ -951,7 +950,7 @@ impl Terminal {
                             } else {
                                 MaybeNavigationTarget::PathLike(PathLikeTarget {
                                     maybe_path: maybe_url_or_path,
-                                    terminal_dir: self.get_cwd(),
+                                    terminal_dir: self.working_directory(),
                                 })
                             };
                             cx.emit(Event::Open(target));
@@ -1006,7 +1005,7 @@ impl Terminal {
         } else {
             MaybeNavigationTarget::PathLike(PathLikeTarget {
                 maybe_path: word,
-                terminal_dir: self.get_cwd(),
+                terminal_dir: self.working_directory(),
             })
         };
         cx.emit(Event::NewNavigationTarget(Some(navigation_target)));
@@ -1636,6 +1635,23 @@ impl Terminal {
     }
 
     pub fn working_directory(&self) -> Option<PathBuf> {
+        if self.is_ssh_terminal {
+            // We can't yet reliably detect the working directory of a shell on the
+            // SSH host. Until we can do that, it doesn't make sense to display
+            // the working directory on the client and persist that.
+            None
+        } else {
+            self.client_side_working_directory()
+        }
+    }
+
+    /// Returns the working directory of the process that's connected to the PTY.
+    /// That means it returns the working directory of the local shell or program
+    /// that's running inside the terminal.
+    ///
+    /// This does *not* return the working directory of the shell that runs on the
+    /// remote host, in case Zed is connected to a remote host.
+    fn client_side_working_directory(&self) -> Option<PathBuf> {
         self.pty_info
             .current
             .as_ref()

crates/terminal_view/src/terminal_view.rs

@@ -1192,7 +1192,7 @@ impl SerializableItem for TerminalView {
             return None;
         }
 
-        if let Some((cwd, workspace_id)) = terminal.get_cwd().zip(self.workspace_id) {
+        if let Some((cwd, workspace_id)) = terminal.working_directory().zip(self.workspace_id) {
             Some(cx.background_executor().spawn(async move {
                 TERMINAL_DB
                     .save_working_directory(item_id, workspace_id, cwd)