[codex] Fix task migration and session env isolation (#2272)

* Fix task migration and session env isolation

* test: guard session HOME mutation

* test: stabilize tui CI checks

* fix: satisfy current tui clippy warnings

---------

Co-authored-by: Lee-take <210963840+Lee-take@users.noreply.github.com>
Co-authored-by: Hunter B <hmbown@gmail.com>

crates/tui/src/commands/change.rs

@@ -312,6 +312,7 @@ mod tests {
     use super::*;
     use crate::config::Config;
     use crate::localization::Locale;
+    use crate::test_support::{EnvVarGuard, lock_test_env};
     use crate::tui::app::{App, TuiOptions};
     fn make_app(tmpdir: &tempfile::TempDir, locale: Locale, has_api_key: bool) -> App {
         let mut config = Config::default();
@@ -505,6 +506,11 @@ Previous release.\n";
     #[test]
     fn change_in_non_english_without_api_key_uses_explicit_fallback() {
         let tmp = tempfile::TempDir::new().unwrap();
+        let _lock = lock_test_env();
+        let _config_path = EnvVarGuard::set("DEEPSEEK_CONFIG_PATH", tmp.path().join("config.toml"));
+        let _deepseek_key = EnvVarGuard::remove("DEEPSEEK_API_KEY");
+        let _deepseek_provider = EnvVarGuard::remove("DEEPSEEK_PROVIDER");
+        let _codewhale_provider = EnvVarGuard::remove("CODEWHALE_PROVIDER");
         let mut app = make_app(&tmp, Locale::ZhHans, false);
         let result = change(&mut app, None);
         assert!(!result.is_error);

crates/tui/src/commands/session.rs

@@ -441,6 +441,7 @@ fn line_to_string(line: ratatui::text::Line<'static>) -> String {
 mod tests {
     use super::*;
     use crate::config::{Config, DEFAULT_TEXT_MODEL};
+    use crate::test_support::EnvVarGuard;
     use crate::tui::app::{App, ReasoningEffort, TuiOptions, TurnCacheRecord};
     use std::time::Instant;
     use tempfile::TempDir;
@@ -517,11 +518,8 @@ mod tests {
         let _lock = crate::test_support::lock_test_env();
         let home = tmpdir.path().join("home");
         std::fs::create_dir_all(&home).unwrap();
-        let previous_home = std::env::var_os("HOME");
-        // SAFETY: guarded by the process-wide test env mutex above.
-        unsafe {
-            std::env::set_var("HOME", &home);
-        }
+        let home_guard = EnvVarGuard::set("HOME", &home);
+        let previous_home = home_guard.previous();
         let mut app = create_test_app_with_tmpdir(&tmpdir);
         app.current_session_id = Some("parent-session".to_string());
         app.api_messages.push(crate::models::Message {
@@ -551,14 +549,8 @@ mod tests {
             Some("parent-session")
         );
         assert_eq!(child.metadata.forked_from_message_count, Some(1));
-        // SAFETY: guarded by the process-wide test env mutex above.
-        unsafe {
-            if let Some(previous_home) = previous_home {
-                std::env::set_var("HOME", previous_home);
-            } else {
-                std::env::remove_var("HOME");
-            }
-        }
+        drop(home_guard);
+        assert_eq!(std::env::var_os("HOME"), previous_home);
     }
 
     #[test]
@@ -668,14 +660,15 @@ mod tests {
     #[test]
     fn test_save_with_default_path_uses_managed_sessions_dir() {
         let tmpdir = TempDir::new().unwrap();
+        let _lock = crate::test_support::lock_test_env();
         // Set CODEWHALE_HOME so the managed sessions directory lands inside the
         // temp dir rather than the real user home. Pre-create the directory so
         // resolve_state_dir picks it up instead of falling back to legacy.
         let home = tmpdir.path().join("home");
         let sessions_dir = home.join("sessions");
         std::fs::create_dir_all(&sessions_dir).unwrap();
-        // SAFETY: test-only, single-threaded via cargo test
-        unsafe { std::env::set_var("CODEWHALE_HOME", home.to_str().unwrap()) };
+        let codewhale_home = EnvVarGuard::set("CODEWHALE_HOME", &home);
+        let previous_codewhale_home = codewhale_home.previous();
         let mut app = create_test_app_with_tmpdir(&tmpdir);
         let result = save(&mut app, None);
         assert!(result.message.is_some());
@@ -691,11 +684,13 @@ mod tests {
         } else {
             Vec::new()
         };
+        drop(codewhale_home);
         // Session should be saved to the managed dir, not the workspace root.
         assert!(
             !entries.is_empty(),
             "expected session file in {sessions_dir:?}, got none; msg: {msg}"
         );
+        assert_eq!(std::env::var_os("CODEWHALE_HOME"), previous_codewhale_home);
     }
 
     #[test]

crates/tui/src/mcp.rs

@@ -2883,7 +2883,14 @@ mod tests {
     use super::*;
     use std::collections::VecDeque;
     use std::sync::atomic::{AtomicBool, Ordering as AtomicOrdering};
-    use std::sync::{Arc, Mutex};
+    use std::sync::{Arc, Mutex, OnceLock};
+
+    async fn lock_mcp_loopback_tests() -> tokio::sync::MutexGuard<'static, ()> {
+        static LOCK: OnceLock<tokio::sync::Mutex<()>> = OnceLock::new();
+        LOCK.get_or_init(|| tokio::sync::Mutex::new(()))
+            .lock()
+            .await
+    }
 
     #[test]
     fn test_mcp_config_defaults() {
@@ -3803,6 +3810,7 @@ mod tests {
             socket.write_all(response.as_bytes()).await.unwrap();
         }
 
+        let _lock = lock_mcp_loopback_tests().await;
         let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
         let addr = listener.local_addr().unwrap();
         let server = tokio::spawn(async move {
@@ -4081,6 +4089,7 @@ mod tests {
         use tokio::io::{AsyncReadExt, AsyncWriteExt};
         use tokio::net::TcpListener;
 
+        let _lock = lock_mcp_loopback_tests().await;
         let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
         let addr = listener.local_addr().unwrap();
         let post_seen = Arc::new(AtomicBool::new(false));
@@ -4172,6 +4181,7 @@ mod tests {
         use tokio::io::{AsyncReadExt, AsyncWriteExt};
         use tokio::net::TcpListener;
 
+        let _lock = lock_mcp_loopback_tests().await;
         let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
         let addr = listener.local_addr().unwrap();
         let post_seen = Arc::new(AtomicBool::new(false));
@@ -4262,6 +4272,7 @@ mod tests {
         use tokio::io::{AsyncReadExt, AsyncWriteExt};
         use tokio::net::TcpListener;
 
+        let _lock = lock_mcp_loopback_tests().await;
         let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
         let addr = listener.local_addr().unwrap();
         let get_header_seen = Arc::new(AtomicBool::new(false));
@@ -4363,6 +4374,7 @@ mod tests {
         use tokio::io::{AsyncReadExt, AsyncWriteExt};
         use tokio::net::TcpListener;
 
+        let _lock = lock_mcp_loopback_tests().await;
         let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
         let addr = listener.local_addr().unwrap();
         let cancel_token = tokio_util::sync::CancellationToken::new();
@@ -4448,6 +4460,7 @@ mod tests {
         use tokio::io::{AsyncReadExt, AsyncWriteExt};
         use tokio::net::TcpListener;
 
+        let _lock = lock_mcp_loopback_tests().await;
         let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
         let addr = listener.local_addr().unwrap();
         let get_count = Arc::new(AtomicUsize::new(0));
@@ -4586,8 +4599,8 @@ mod tests {
                 env: HashMap::new(),
                 url: Some(format!("http://{addr}/mcp")),
                 transport: None,
-                connect_timeout: Some(2),
-                execute_timeout: Some(2),
+                connect_timeout: Some(10),
+                execute_timeout: Some(10),
                 read_timeout: None,
                 disabled: false,
                 enabled: true,
@@ -4621,6 +4634,7 @@ mod tests {
         use tokio::net::TcpListener;
         use tokio::sync::mpsc;
 
+        let _lock = lock_mcp_loopback_tests().await;
         let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
         let addr = listener.local_addr().unwrap();
 
@@ -4717,6 +4731,7 @@ mod tests {
             (headers, json)
         }
 
+        let _lock = lock_mcp_loopback_tests().await;
         let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
         let addr = listener.local_addr().unwrap();
         let active_sse = Arc::new(Mutex::new(None::<mpsc::UnboundedSender<Option<String>>>));
@@ -4838,8 +4853,8 @@ mod tests {
                 env: HashMap::new(),
                 url: Some(format!("http://{addr}/sse")),
                 transport: Some("sse".to_string()),
-                connect_timeout: Some(2),
-                execute_timeout: Some(2),
+                connect_timeout: Some(10),
+                execute_timeout: Some(10),
                 read_timeout: None,
                 disabled: false,
                 enabled: true,
@@ -4883,6 +4898,7 @@ mod tests {
         use tokio::io::{AsyncReadExt, AsyncWriteExt};
         use tokio::net::TcpListener;
 
+        let _lock = lock_mcp_loopback_tests().await;
         let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
         let addr = listener.local_addr().unwrap();
         let server = tokio::spawn(async move {
@@ -4958,6 +4974,7 @@ mod tests {
         use tokio::io::{AsyncReadExt, AsyncWriteExt};
         use tokio::net::TcpListener;
 
+        let _lock = lock_mcp_loopback_tests().await;
         let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
         let addr = listener.local_addr().unwrap();
         // The test signals success by writing to this flag — the GET handler

crates/tui/src/shell_dispatcher.rs

@@ -315,7 +315,7 @@ impl ShellDispatcher {
             if Self::find_exe("powershell.exe") {
                 return ShellKind::WindowsPowerShell;
             }
-            return ShellKind::Cmd;
+            ShellKind::Cmd
         }
 
         #[cfg(not(windows))]

crates/tui/src/task_manager.rs

@@ -1639,7 +1639,8 @@ fn default_auto_approve() -> bool {
     true
 }
 
-/// Default task persistence location (`~/.deepseek/tasks`).
+/// Default task manager data location (`~/.codewhale/tasks`, or legacy
+/// `~/.deepseek/tasks` when only the legacy directory exists).
 #[must_use]
 pub fn default_tasks_dir() -> PathBuf {
     if let Ok(path) = std::env::var("DEEPSEEK_TASKS_DIR")
@@ -1647,10 +1648,21 @@ pub fn default_tasks_dir() -> PathBuf {
     {
         return PathBuf::from(path);
     }
-    if let Some(home) = dirs::home_dir() {
-        return home.join(".codewhale").join("tasks");
+    dirs::home_dir()
+        .map(|home| default_tasks_dir_for_home(&home))
+        .unwrap_or_else(|| PathBuf::from(".codewhale").join("tasks"))
+}
+
+fn default_tasks_dir_for_home(home: &Path) -> PathBuf {
+    let primary = home.join(".codewhale").join("tasks");
+    if primary.is_dir() {
+        return primary;
     }
-    PathBuf::from(".codewhale").join("tasks")
+    let legacy = home.join(".deepseek").join("tasks");
+    if legacy.is_dir() {
+        return legacy;
+    }
+    primary
 }
 
 /// Wait for a task to reach a terminal status (tests and API helpers).
@@ -1910,4 +1922,62 @@ mod tests {
         }
         Ok(())
     }
+
+    #[test]
+    fn default_tasks_dir_falls_back_to_legacy_deepseek_tasks() {
+        let temp_home = tempfile::tempdir().unwrap();
+        let home = temp_home.path();
+        let legacy_tasks = home.join(".deepseek").join("tasks");
+        std::fs::create_dir_all(&legacy_tasks).unwrap();
+
+        assert_eq!(default_tasks_dir_for_home(home), legacy_tasks);
+    }
+
+    #[test]
+    fn default_tasks_dir_prefers_existing_codewhale_tasks() {
+        let temp_home = tempfile::tempdir().unwrap();
+        let home = temp_home.path();
+        let primary_tasks = home.join(".codewhale").join("tasks");
+        let legacy_tasks = home.join(".deepseek").join("tasks");
+        std::fs::create_dir_all(&primary_tasks).unwrap();
+        std::fs::create_dir_all(&legacy_tasks).unwrap();
+
+        assert_eq!(default_tasks_dir_for_home(home), primary_tasks);
+    }
+
+    #[test]
+    fn default_tasks_dir_falls_back_to_legacy_when_primary_is_file() {
+        let temp_home = tempfile::tempdir().unwrap();
+        let home = temp_home.path();
+        let primary_tasks = home.join(".codewhale").join("tasks");
+        let legacy_tasks = home.join(".deepseek").join("tasks");
+        std::fs::create_dir_all(primary_tasks.parent().unwrap()).unwrap();
+        std::fs::write(&primary_tasks, "not a directory").unwrap();
+        std::fs::create_dir_all(&legacy_tasks).unwrap();
+
+        assert_eq!(default_tasks_dir_for_home(home), legacy_tasks);
+    }
+
+    #[test]
+    fn default_tasks_dir_ignores_legacy_file_for_new_installs() {
+        let temp_home = tempfile::tempdir().unwrap();
+        let home = temp_home.path();
+        let primary_tasks = home.join(".codewhale").join("tasks");
+        let legacy_tasks = home.join(".deepseek").join("tasks");
+        std::fs::create_dir_all(legacy_tasks.parent().unwrap()).unwrap();
+        std::fs::write(&legacy_tasks, "not a directory").unwrap();
+
+        assert_eq!(default_tasks_dir_for_home(home), primary_tasks);
+    }
+
+    #[test]
+    fn default_tasks_dir_uses_codewhale_tasks_for_new_installs() {
+        let temp_home = tempfile::tempdir().unwrap();
+        let home = temp_home.path();
+
+        assert_eq!(
+            default_tasks_dir_for_home(home),
+            home.join(".codewhale").join("tasks")
+        );
+    }
 }

crates/tui/src/test_support.rs

@@ -1,5 +1,6 @@
 //! Shared test-only helpers.
 
+use std::ffi::{OsStr, OsString};
 use std::sync::{Mutex, MutexGuard, OnceLock};
 
 fn env_lock() -> &'static Mutex<()> {
@@ -18,6 +19,49 @@ pub(crate) fn lock_test_env() -> MutexGuard<'static, ()> {
     }
 }
 
+/// Restore one environment variable when dropped.
+///
+/// Callers that mutate process-global environment variables must hold
+/// [`lock_test_env`] until after this guard is dropped.
+pub(crate) struct EnvVarGuard {
+    key: &'static str,
+    previous: Option<OsString>,
+}
+
+impl EnvVarGuard {
+    pub(crate) fn set(key: &'static str, value: impl AsRef<OsStr>) -> Self {
+        let previous = std::env::var_os(key);
+        // SAFETY: callers hold the process-wide test env mutex.
+        unsafe { std::env::set_var(key, value) };
+        Self { key, previous }
+    }
+
+    pub(crate) fn remove(key: &'static str) -> Self {
+        let previous = std::env::var_os(key);
+        // SAFETY: callers hold the process-wide test env mutex.
+        unsafe { std::env::remove_var(key) };
+        Self { key, previous }
+    }
+
+    pub(crate) fn previous(&self) -> Option<OsString> {
+        self.previous.clone()
+    }
+}
+
+impl Drop for EnvVarGuard {
+    fn drop(&mut self) {
+        // SAFETY: callers hold the process-wide test env mutex until after this
+        // guard is dropped.
+        unsafe {
+            if let Some(value) = self.previous.take() {
+                std::env::set_var(self.key, value);
+            } else {
+                std::env::remove_var(self.key);
+            }
+        }
+    }
+}
+
 /// Find the byte position of the first divergence between two strings,
 /// returning a windowed view (`±32 bytes` around the divergence) so failures
 /// in cache-prefix-stability tests show *which* bytes drifted, not just that

crates/tui/src/tui/app.rs

@@ -4884,11 +4884,10 @@ pub enum McpUiAction {
 mod tests {
     use super::*;
     use crate::config::{ApiProvider, Config, ProviderConfig, ProvidersConfig};
-    use crate::test_support::lock_test_env;
+    use crate::test_support::{EnvVarGuard, lock_test_env};
     use crate::tools::plan::{PlanItemArg, StepStatus, UpdatePlanArgs};
     use crate::tools::todo::TodoStatus;
     use crate::tui::clipboard::PastedImage;
-    use std::ffi::OsString;
 
     fn test_options(yolo: bool) -> TuiOptions {
         TuiOptions {
@@ -5019,34 +5018,6 @@ mod tests {
         assert_eq!(app.cursor_position, "abc\n".len()); // unchanged
     }
 
-    struct EnvVarGuard {
-        key: &'static str,
-        previous: Option<OsString>,
-    }
-
-    impl EnvVarGuard {
-        fn set(key: &'static str, value: impl AsRef<std::ffi::OsStr>) -> Self {
-            let previous = std::env::var_os(key);
-            unsafe { std::env::set_var(key, value) };
-            Self { key, previous }
-        }
-
-        fn remove(key: &'static str) -> Self {
-            let previous = std::env::var_os(key);
-            unsafe { std::env::remove_var(key) };
-            Self { key, previous }
-        }
-    }
-
-    impl Drop for EnvVarGuard {
-        fn drop(&mut self) {
-            match self.previous.as_ref() {
-                Some(value) => unsafe { std::env::set_var(self.key, value) },
-                None => unsafe { std::env::remove_var(self.key) },
-            }
-        }
-    }
-
     #[test]
     fn test_trust_mode_follows_yolo_on_startup() {
         let app = App::new(test_options(true), &Config::default());

crates/tui/src/tui/ui/tests.rs

@@ -1360,6 +1360,8 @@ fn create_test_options() -> TuiOptions {
 }
 
 #[tokio::test]
+// This test intentionally pins the process-global spillover root until the
+// async receipt path finishes.
 #[allow(clippy::await_holding_lock)]
 async fn tool_result_api_content_receipts_large_live_output() {
     let _guard = crate::tools::truncate::TEST_SPILLOVER_GUARD