diff --git a/crates/tui/src/commands/change.rs b/crates/tui/src/commands/change.rs index e424ec9b..220cdb2e 100644 --- a/crates/tui/src/commands/change.rs +++ b/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); diff --git a/crates/tui/src/commands/session.rs b/crates/tui/src/commands/session.rs index ac03487c..098b00eb 100644 --- a/crates/tui/src/commands/session.rs +++ b/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] diff --git a/crates/tui/src/mcp.rs b/crates/tui/src/mcp.rs index 950b3dd9..c07fe5ac 100644 --- a/crates/tui/src/mcp.rs +++ b/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> = 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::>>)); @@ -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 diff --git a/crates/tui/src/shell_dispatcher.rs b/crates/tui/src/shell_dispatcher.rs index b4af5220..a2063c41 100644 --- a/crates/tui/src/shell_dispatcher.rs +++ b/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))] diff --git a/crates/tui/src/task_manager.rs b/crates/tui/src/task_manager.rs index 8f927023..4920917f 100644 --- a/crates/tui/src/task_manager.rs +++ b/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") + ); + } } diff --git a/crates/tui/src/test_support.rs b/crates/tui/src/test_support.rs index 8212a66f..db7b2bdc 100644 --- a/crates/tui/src/test_support.rs +++ b/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, +} + +impl EnvVarGuard { + pub(crate) fn set(key: &'static str, value: impl AsRef) -> 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 { + 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 diff --git a/crates/tui/src/tui/app.rs b/crates/tui/src/tui/app.rs index ffcb6d7b..ca57ff1d 100644 --- a/crates/tui/src/tui/app.rs +++ b/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, - } - - impl EnvVarGuard { - fn set(key: &'static str, value: impl AsRef) -> 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()); diff --git a/crates/tui/src/tui/ui/tests.rs b/crates/tui/src/tui/ui/tests.rs index 5e5c707e..0f2677c4 100644 --- a/crates/tui/src/tui/ui/tests.rs +++ b/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