fix(workspaces): skip reconnect when already connected

Avoid spawning when a workspace session already exists and add tests for both
connected and unconnected connect paths.

Author: Dimillian

src-tauri/src/shared/workspaces_core/connect.rs

@@ -25,6 +25,12 @@ where
     Fut: Future<Output = Result<Arc<WorkspaceSession>, String>>,
 {
     let (entry, parent_entry) = resolve_entry_and_parent(workspaces, &workspace_id).await?;
+    {
+        let sessions = sessions.lock().await;
+        if sessions.contains_key(&entry.id) {
+            return Ok(());
+        }
+    }
     let (default_bin, codex_args) = {
         let settings = app_settings.lock().await;
         (
@@ -47,3 +53,128 @@ pub(super) async fn kill_session_by_id(
         kill_child_process_tree(&mut child).await;
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    use std::collections::HashMap;
+    use std::process::Stdio;
+    use std::sync::atomic::{AtomicU64, AtomicUsize, Ordering};
+    use std::sync::Arc;
+
+    use tokio::process::Command;
+    use tokio::sync::Mutex;
+
+    use crate::types::{WorkspaceKind, WorkspaceSettings};
+
+    fn make_workspace_entry(id: &str) -> WorkspaceEntry {
+        WorkspaceEntry {
+            id: id.to_string(),
+            name: id.to_string(),
+            path: "/tmp".to_string(),
+            codex_bin: None,
+            kind: WorkspaceKind::Main,
+            parent_id: None,
+            worktree: None,
+            settings: WorkspaceSettings::default(),
+        }
+    }
+
+    fn make_session(entry: WorkspaceEntry) -> Arc<WorkspaceSession> {
+        let mut cmd = if cfg!(windows) {
+            let mut cmd = Command::new("cmd");
+            cmd.args(["/C", "more"]);
+            cmd
+        } else {
+            let mut cmd = Command::new("sh");
+            cmd.args(["-c", "cat"]);
+            cmd
+        };
+
+        cmd.stdin(Stdio::piped())
+            .stdout(Stdio::null())
+            .stderr(Stdio::null());
+
+        let mut child = cmd.spawn().expect("spawn dummy child");
+        let stdin = child.stdin.take().expect("dummy child stdin");
+
+        Arc::new(WorkspaceSession {
+            entry,
+            codex_args: None,
+            child: Mutex::new(child),
+            stdin: Mutex::new(stdin),
+            pending: Mutex::new(HashMap::new()),
+            next_id: AtomicU64::new(0),
+            background_thread_callbacks: Mutex::new(HashMap::new()),
+        })
+    }
+
+    #[test]
+    fn connect_workspace_is_noop_when_already_connected() {
+        tokio::runtime::Runtime::new().unwrap().block_on(async {
+            let entry = make_workspace_entry("ws-1");
+            let workspaces = Mutex::new(HashMap::from([(entry.id.clone(), entry.clone())]));
+            let sessions = Mutex::new(HashMap::from([(
+                entry.id.clone(),
+                make_session(entry.clone()),
+            )]));
+            let app_settings = Mutex::new(AppSettings::default());
+            let spawn_calls = Arc::new(AtomicUsize::new(0));
+            let spawn_calls_ref = spawn_calls.clone();
+
+            connect_workspace_core(
+                entry.id.clone(),
+                &workspaces,
+                &sessions,
+                &app_settings,
+                move |_entry, _default_bin, _codex_args, _codex_home| {
+                    let spawn_calls_ref = spawn_calls_ref.clone();
+                    async move {
+                        spawn_calls_ref.fetch_add(1, Ordering::SeqCst);
+                        Err("should not spawn".to_string())
+                    }
+                },
+            )
+            .await
+            .expect("connect should be noop");
+
+            assert_eq!(spawn_calls.load(Ordering::SeqCst), 0);
+            kill_session_by_id(&sessions, &entry.id).await;
+        });
+    }
+
+    #[test]
+    fn connect_workspace_spawns_when_not_connected() {
+        tokio::runtime::Runtime::new().unwrap().block_on(async {
+            let entry = make_workspace_entry("ws-2");
+            let workspaces = Mutex::new(HashMap::from([(entry.id.clone(), entry.clone())]));
+            let sessions = Mutex::new(HashMap::<String, Arc<WorkspaceSession>>::new());
+            let app_settings = Mutex::new(AppSettings::default());
+            let spawn_calls = Arc::new(AtomicUsize::new(0));
+            let spawn_calls_ref = spawn_calls.clone();
+            let entry_for_spawn = entry.clone();
+
+            connect_workspace_core(
+                entry.id.clone(),
+                &workspaces,
+                &sessions,
+                &app_settings,
+                move |_entry, _default_bin, _codex_args, _codex_home| {
+                    let spawn_calls_ref = spawn_calls_ref.clone();
+                    let entry_for_spawn = entry_for_spawn.clone();
+                    async move {
+                        spawn_calls_ref.fetch_add(1, Ordering::SeqCst);
+                        Ok(make_session(entry_for_spawn))
+                    }
+                },
+            )
+            .await
+            .expect("connect should spawn");
+
+            assert_eq!(spawn_calls.load(Ordering::SeqCst), 1);
+            assert!(sessions.lock().await.contains_key(&entry.id));
+            kill_session_by_id(&sessions, &entry.id).await;
+        });
+    }
+}