diff --git a/src/cli/completer.rs b/src/cli/completer.rs index 68b4735..2596e4f 100644 --- a/src/cli/completer.rs +++ b/src/cli/completer.rs @@ -1,7 +1,9 @@ use crate::client::{ModelType, list_models}; use crate::config::paths; -use crate::config::{AppConfig, Config, list_agents_with_descriptions, list_sessions}; -use crate::utils::list_file_names; +use crate::config::{ + AppConfig, Config, labeled_session_names, list_agents_with_descriptions, sanitize_display_text, + session_scope_dirs, +}; use crate::vault::Vault; use clap_complete::{CompletionCandidate, Shell, generate}; use clap_complete_nushell::Nushell; @@ -115,7 +117,11 @@ pub(super) fn bundle_completer(current: &OsStr) -> Vec { } fn extract_agent_from_args() -> Option { - let args: Vec = env::args().collect(); + extract_agent_from_args_in(env::args()) +} + +fn extract_agent_from_args_in>(args: I) -> Option { + let args: Vec = args.into_iter().collect(); let mut i = 0; while i < args.len() { let arg = &args[i]; @@ -134,19 +140,21 @@ fn extract_agent_from_args() -> Option { } pub(super) fn session_completer(current: &OsStr) -> Vec { - let cur = current.to_string_lossy(); + session_candidates( + ¤t.to_string_lossy(), + extract_agent_from_args().as_deref(), + ) +} - let sessions = if let Some(agent_name) = extract_agent_from_args() { - let sessions_dir = paths::agent_data_dir(&agent_name).join("sessions"); - list_file_names(sessions_dir, ".yaml") - } else { - list_sessions() - }; - - sessions +fn session_candidates(cur: &str, agent: Option<&str>) -> Vec { + let (workspace_dir, global_dir) = session_scope_dirs(agent); + labeled_session_names(&workspace_dir, &global_dir) .into_iter() - .filter(|s| s.starts_with(&*cur)) - .map(CompletionCandidate::new) + .filter(|(name, _)| name.starts_with(cur)) + .map(|(name, scope)| { + CompletionCandidate::new(sanitize_display_text(&name)) + .help(Some(scope.to_string().into())) + }) .collect() } @@ -194,3 +202,112 @@ pub(super) fn secrets_completer(current: &OsStr) -> Vec { Err(_) => vec![], } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::default_sessions_dir; + use crate::testing::{EnvVarGuard, TestConfigDirGuard}; + use serial_test::serial; + use std::path::Path; + + fn seed(dir: &Path, name: &str) { + fs::create_dir_all(dir).unwrap(); + fs::write(dir.join(format!("{name}.yaml")), "model: x\n").unwrap(); + } + + fn labeled(candidates: Vec) -> Vec<(String, Option)> { + candidates + .into_iter() + .map(|c| { + ( + c.get_value().to_string_lossy().into_owned(), + c.get_help().map(|h| h.to_string()), + ) + }) + .collect() + } + + #[test] + #[serial] + fn session_completer_labels_scope_and_prefers_workspace() { + let _guard = TestConfigDirGuard::new("cli-session-completer"); + seed(&paths::workspace_sessions_dir(), "both"); + seed(&default_sessions_dir(), "both"); + seed(&default_sessions_dir(), "only-global"); + seed(&paths::workspace_sessions_dir(), "only-ws"); + + assert_eq!( + labeled(session_completer(OsStr::new(""))), + vec![ + ("both".to_string(), Some("workspace".to_string())), + ("only-ws".to_string(), Some("workspace".to_string())), + ("only-global".to_string(), Some("global".to_string())), + ] + ); + assert_eq!( + labeled(session_completer(OsStr::new("on"))), + vec![ + ("only-ws".to_string(), Some("workspace".to_string())), + ("only-global".to_string(), Some("global".to_string())), + ] + ); + } + + #[test] + #[serial] + fn session_candidates_use_the_agent_scope_dirs() { + let _guard = TestConfigDirGuard::new("cli-agent-session-completer"); + let _agent_data_dir = EnvVarGuard::unset("A_DATA_DIR"); + seed(&paths::workspace_agent_sessions_dir("a"), "both"); + seed(&paths::agent_sessions_dir("a"), "both"); + seed(&paths::agent_sessions_dir("a"), "agent-global"); + seed(&default_sessions_dir(), "plain-global"); + + assert_eq!( + labeled(session_candidates("", Some("a"))), + vec![ + ("both".to_string(), Some("workspace".to_string())), + ("agent-global".to_string(), Some("global".to_string())), + ] + ); + assert_eq!( + labeled(session_candidates("", None)), + vec![("plain-global".to_string(), Some("global".to_string()))] + ); + } + + #[test] + fn extract_agent_from_args_in_reads_both_flag_forms() { + fn args(s: &str) -> Vec { + s.split_whitespace().map(str::to_string).collect() + } + assert_eq!( + extract_agent_from_args_in(args("coyote --agent a --session x")), + Some("a".to_string()) + ); + assert_eq!( + extract_agent_from_args_in(args("coyote -a b")), + Some("b".to_string()) + ); + assert_eq!( + extract_agent_from_args_in(args("coyote --agent=c")), + Some("c".to_string()) + ); + assert_eq!(extract_agent_from_args_in(args("coyote --agent")), None); + assert_eq!(extract_agent_from_args_in(args("coyote --session x")), None); + } + + #[test] + #[serial] + fn config_dir_guard_clears_and_restores_the_sessions_dir_override() { + let key = crate::utils::get_env_name("sessions_dir"); + let junk = std::path::PathBuf::from("/nonexistent/junk-sessions"); + let _outer = EnvVarGuard::set(&key, &junk); + { + let guard = TestConfigDirGuard::new("cli-sessions-dir-guard"); + assert!(default_sessions_dir().starts_with(&guard.path)); + } + assert_eq!(default_sessions_dir(), junk); + } +} diff --git a/src/config/mod.rs b/src/config/mod.rs index 5f3511d..e588e46 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -58,7 +58,7 @@ pub use self::request_context::{ pub use self::role::{ CODE_ROLE, CREATE_TITLE_ROLE, EXPLAIN_SHELL_ROLE, Role, RoleLike, SHELL_ROLE, }; -pub use self::session::Session; +pub use self::session::{Session, SessionScope, labeled_session_names, session_scope_dirs}; #[allow(unused_imports)] pub use self::skill::Skill; #[allow(unused_imports)] @@ -183,7 +183,7 @@ const MCP_FILE_NAME: &str = "mcp.json"; const HIDDEN_MCP_FILE_NAME: &str = ".mcp.json"; const MEMORY_DIR_NAME: &str = "memory"; const MEMORY_INDEX_FILE_NAME: &str = "MEMORY.md"; -const WORKSPACE_COYOTE_DIR_NAME: &str = ".coyote"; +pub(crate) const WORKSPACE_COYOTE_DIR_NAME: &str = ".coyote"; const SBX_KIT_DIR_NAME: &str = "sbx-kit"; const SBX_KIT_HASH_FILE: &str = "kit.sha256"; const SBX_MIXIN_FILE_NAME: &str = "sbx-mixin.yaml"; @@ -583,10 +583,6 @@ pub fn default_sessions_dir() -> PathBuf { } } -pub fn list_sessions() -> Vec { - list_file_names(default_sessions_dir(), ".yaml") -} - /// Refreshes the models override. With a URL, mirrors a full models.yaml from /// it; without one, rebuilds the catalog from the live models.dev + OpenRouter /// APIs merged with the embedded quirks. diff --git a/src/config/paths.rs b/src/config/paths.rs index 8b9e125..8392e77 100644 --- a/src/config/paths.rs +++ b/src/config/paths.rs @@ -5,7 +5,7 @@ use super::{ GLOBAL_TOOLS_UTILS_DIR_NAME, HIDDEN_MCP_FILE_NAME, HOOKS_DIR_NAME, INSTALLED_BUNDLES_FILE_NAME, MACROS_DIR_NAME, MCP_FILE_NAME, MEMORY_DIR_NAME, MEMORY_INDEX_FILE_NAME, ModelsOverride, RAGS_DIR_NAME, ROLES_DIR_NAME, SBX_KIT_DIR_NAME, SBX_KIT_HASH_FILE, SBX_MIXIN_FILE_NAME, - SBX_MIXIN_KITS_DIR_NAME, SKILLS_DIR_NAME, WORKSPACE_COYOTE_DIR_NAME, + SBX_MIXIN_KITS_DIR_NAME, SESSIONS_DIR_NAME, SKILLS_DIR_NAME, WORKSPACE_COYOTE_DIR_NAME, }; use crate::client::ProviderModels; use crate::config::REPL_HISTORY_DIR_NAME; @@ -229,6 +229,17 @@ pub fn workspace_macros_dir() -> PathBuf { workspace_config_dir().join(MACROS_DIR_NAME) } +pub fn workspace_sessions_dir() -> PathBuf { + workspace_config_dir().join(SESSIONS_DIR_NAME) +} + +pub fn workspace_agent_sessions_dir(name: &str) -> PathBuf { + workspace_config_dir() + .join(AGENTS_DIR_NAME) + .join(name) + .join(SESSIONS_DIR_NAME) +} + pub fn workspace_mcp_config_file() -> Option { workspace_mcp_config_file_in(&env::current_dir().unwrap_or_default()) } @@ -332,6 +343,10 @@ pub fn agent_data_dir(name: &str) -> PathBuf { } } +pub fn agent_sessions_dir(name: &str) -> PathBuf { + agent_data_dir(name).join(SESSIONS_DIR_NAME) +} + pub fn agent_graph_file(agent_name: &str) -> PathBuf { agent_data_dir(agent_name).join(AGENT_GRAPH_FILE_NAME) } @@ -854,6 +869,21 @@ mod tests { ); }); } + + #[test] + #[serial] + fn workspace_session_dirs_live_under_workspace_config_dir() { + with_workspace_dir(|_, ws_dir| { + assert_eq!(workspace_sessions_dir(), ws_dir.join(SESSIONS_DIR_NAME)); + assert_eq!( + workspace_agent_sessions_dir("helper"), + ws_dir + .join(AGENTS_DIR_NAME) + .join("helper") + .join(SESSIONS_DIR_NAME) + ); + }); + } } mod user_mcp_resolution { diff --git a/src/config/request_context.rs b/src/config/request_context.rs index a83b84e..5f7dea6 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -1,18 +1,23 @@ use super::bundles::installed_bundle_names; use super::mcp_tool_policy::{McpToolPolicy, SkillMcpLayer, ToolFilter, expand_mcp_server_alias}; use super::rag_cache::{RagCache, RagKey}; -use super::session::{INTERRUPTED_RESPONSE_TEXT, Session}; +use super::session::{ + INTERRUPTED_RESPONSE_TEXT, Session, SessionScope, labeled_session_names, merged_session_names, + session_scope_dirs, +}; use super::skill::{SKILL_SCAFFOLD, Skill}; use super::skill_policy::SkillPolicy; use super::skill_registry::SkillRegistry; use super::todo::TodoList; -use super::tool_scope::{McpPromptCompletion, McpRuntime, ToolScope, format_prompt_arguments}; +use super::tool_scope::{ + McpPromptCompletion, McpRuntime, ToolScope, format_prompt_arguments, sanitize_display_text, +}; use super::{ AGENTS_DIR_NAME, Agent, AgentVariables, AppConfig, AppState, AssetCategory, CREATE_TITLE_ROLE, Input, InstallFilter, LEFT_PROMPT, LastMessage, MESSAGES_FILE_NAME, MacroAllowlistLevel, MacroPolicy, MacroSource, MacroState, RESERVED_MACRO_NAMES, RIGHT_PROMPT, ResolvedMacro, Role, - RoleLike, SESSIONS_DIR_NAME, SUMMARIZATION_PROMPT, SUMMARY_CONTEXT_PROMPT, StateFlags, - TEMP_ROLE_NAME, TEMP_SESSION_NAME, WorkingMode, bundles, ensure_parent_exists, + RoleLike, SUMMARIZATION_PROMPT, SUMMARY_CONTEXT_PROMPT, StateFlags, TEMP_ROLE_NAME, + TEMP_SESSION_NAME, WORKSPACE_COYOTE_DIR_NAME, WorkingMode, bundles, ensure_parent_exists, list_agents_with_descriptions, memory, paths, }; use super::{MessageContentToolCalls, prompts}; @@ -768,22 +773,42 @@ impl RequestContext { } pub fn sessions_dir(&self) -> PathBuf { - match &self.agent { - None => match env::var(get_env_name("sessions_dir")) { - Ok(value) => PathBuf::from(value), - Err(_) => paths::local_dir(SESSIONS_DIR_NAME), - }, - Some(agent) => paths::agent_data_dir(agent.name()).join(SESSIONS_DIR_NAME), + self.sessions_dir_for(SessionScope::Global) + } + + pub fn sessions_dir_for(&self, scope: SessionScope) -> PathBuf { + let (workspace, global) = session_scope_dirs(self.agent.as_ref().map(|a| a.name())); + match scope { + SessionScope::Workspace => workspace, + SessionScope::Global => global, } } pub fn session_file(&self, name: &str) -> PathBuf { + self.session_file_for(name, SessionScope::Global) + } + + pub fn session_file_for(&self, name: &str, scope: SessionScope) -> PathBuf { + let dir = self.sessions_dir_for(scope); match name.split_once("/") { - Some((dir, name)) => self.sessions_dir().join(dir).join(format!("{name}.yaml")), - None => self.sessions_dir().join(format!("{name}.yaml")), + Some((subdir, name)) => dir.join(subdir).join(format!("{name}.yaml")), + None => dir.join(format!("{name}.yaml")), } } + /// Workspace shadows global; a name found in neither resolves to the + /// global path so new sessions default to the global scope. + pub fn resolve_session_file(&self, name: &str) -> (PathBuf, SessionScope) { + let workspace = self.session_file_for(name, SessionScope::Workspace); + if workspace.exists() { + return (workspace, SessionScope::Workspace); + } + ( + self.session_file_for(name, SessionScope::Global), + SessionScope::Global, + ) + } + pub fn rag_file(&self, name: &str) -> PathBuf { match &self.agent { Some(agent) => paths::agent_rag_file(agent.name(), name), @@ -1023,11 +1048,37 @@ impl RequestContext { } pub fn list_sessions(&self) -> Vec { - list_file_names(self.sessions_dir(), ".yaml") + merged_session_names( + &self.sessions_dir_for(SessionScope::Workspace), + &self.sessions_dir_for(SessionScope::Global), + ) } - pub fn list_autoname_sessions(&self) -> Vec { - list_file_names(self.sessions_dir().join("_"), ".yaml") + fn labeled_sessions(&self) -> Vec<(String, Option)> { + labeled_session_names( + &self.sessions_dir_for(SessionScope::Workspace), + &self.sessions_dir_for(SessionScope::Global), + ) + .into_iter() + .map(|(name, scope)| (sanitize_display_text(&name), Some(scope.to_string()))) + .collect() + } + + fn labeled_autoname_sessions(&self) -> Vec<(String, Option)> { + let mut values = labeled_session_names( + &self.sessions_dir_for(SessionScope::Workspace).join("_"), + &self.sessions_dir_for(SessionScope::Global).join("_"), + ); + values.sort_unstable_by(|a, b| b.0.cmp(&a.0)); + values + .into_iter() + .map(|(name, scope)| { + ( + format!("_/{}", sanitize_display_text(&name)), + Some(scope.to_string()), + ) + }) + .collect() } pub fn is_compressing_session(&self) -> bool { @@ -1099,7 +1150,7 @@ impl RequestContext { ); } if let Some(mut session) = self.session.take() { - let sessions_dir = self.sessions_dir(); + let sessions_dir = self.sessions_dir_for(session.scope()); session.exit(&sessions_dir, self.working_mode.is_repl())?; self.discontinuous_last_message(); // Todo state is session-scoped: it was mirrored into the session @@ -1111,27 +1162,39 @@ impl RequestContext { Ok(()) } - pub fn save_session(&mut self, name: Option<&str>) -> Result<()> { - let session_name = match &self.session { - Some(session) => match name { - Some(v) => v.to_string(), - None => session - .autoname() - .unwrap_or_else(|| session.name()) - .to_string(), - }, - None => bail!("No session"), + pub fn save_session(&mut self, name: Option<&str>, scope: Option) -> Result<()> { + let Some(session) = self.session.as_ref() else { + bail!("No session"); }; - let session_path = self.session_file(&session_name); - if let Some(session) = self.session.as_mut() { - session.save(&session_name, &session_path, self.working_mode.is_repl())?; + let session_name = match name { + Some(v) => v.to_string(), + None => session + .autoname() + .unwrap_or_else(|| session.name()) + .to_string(), + }; + let scope = scope.unwrap_or_else(|| session.scope()); + if scope == SessionScope::Workspace && session_name == TEMP_SESSION_NAME { + bail!("Name the session first: .save session --{scope}"); + } + let session_path = self.session_file_for(&session_name, scope); + let is_repl = self.working_mode.is_repl(); + let session = self.session.as_mut().unwrap(); + session.save(&session_name, &session_path, is_repl)?; + session.set_scope(scope); + let (resolved_path, resolved_scope) = self.resolve_session_file(&session_name); + if is_repl && resolved_path != session_path { + println!( + "{}", + shadow_warning(scope, resolved_scope, &sanitize_display_text(&session_name)) + ); } Ok(()) } pub fn fork_session(&mut self, fork_name: Option<&str>) -> Result<()> { - let current_name = match &self.session { - Some(s) => s.name().to_string(), + let (current_name, scope) = match &self.session { + Some(s) => (s.name().to_string(), s.scope()), None => bail!("No active session to fork"), }; @@ -1139,20 +1202,22 @@ impl RequestContext { Some(name) => name.to_string(), None => { let base = fork_base_name(¤t_name); - let sessions_dir = self.sessions_dir(); (1_u32..) .map(|n| format!("{base}-fork-{n}")) - .find(|name| !sessions_dir.join(format!("{name}.yaml")).exists()) + .find(|name| !self.resolve_session_file(name).0.exists()) .unwrap() } }; - let fork_path = self.session_file(&fork_name); - if fork_path.exists() { + if fork_name == TEMP_SESSION_NAME { + bail!("'{TEMP_SESSION_NAME}' is a reserved session name"); + } + if self.resolve_session_file(&fork_name).0.exists() { bail!("Session '{}' already exists", fork_name); } + let fork_path = self.session_file_for(&fork_name, scope); - self.save_session(None)?; + self.save_session(None, None)?; let session = self.session.as_ref().unwrap(); let mut fork = session.clone(); @@ -2180,6 +2245,10 @@ impl RequestContext { ("roles_dir", display_path(&paths::roles_dir())), ("skills_dir", display_path(&paths::skills_dir())), ("sessions_dir", display_path(&self.sessions_dir())), + ( + "workspace_sessions_dir", + display_path(&self.sessions_dir_for(SessionScope::Workspace)), + ), ("memory_dir", display_path(&paths::global_memory_dir())), ("rags_dir", display_path(&paths::rags_dir())), ("macros_dir", display_path(&paths::macros_dir())), @@ -2900,12 +2969,12 @@ impl RequestContext { } pub fn edit_session(&mut self, app: &AppConfig) -> Result<()> { - let name = match &self.session { - Some(session) => session.name().to_string(), + let (name, scope) = match &self.session { + Some(session) => (session.name().to_string(), session.scope()), None => bail!("No session"), }; - let session_path = self.session_file(&name); - self.save_session(Some(&name))?; + let session_path = self.session_file_for(&name, scope); + self.save_session(Some(&name), None)?; let editor = app.editor()?; edit_file(&editor, &session_path).with_context(|| { format!( @@ -2914,7 +2983,13 @@ impl RequestContext { editor ) })?; - self.session = Some(Session::load_from_ctx(self, app, &name, &session_path)?); + self.session = Some(Session::load_from_ctx( + self, + app, + &name, + &session_path, + scope, + )?); self.discontinuous_last_message(); Ok(()) } @@ -3100,7 +3175,24 @@ impl RequestContext { pub fn list_assets(&self, kind: &str) -> Result<()> { match kind { "roles" => print_asset_names("roles", &paths::list_roles(true)), - "sessions" => print_asset_names("sessions", &self.list_sessions()), + "sessions" => { + let rows = self.session_list_rows(); + if rows.is_empty() { + println!("No sessions found."); + return Ok(()); + } + + let mut table = asset_table(&["name", "scope", "state"]); + for (name, scope, shadowed) in &rows { + let scope = scope.to_string(); + let state = if *shadowed { "shadowed" } else { "-" }; + table.add_row(vec![name.as_str(), &scope, state]); + } + + println!("Sessions:"); + println!("{table}"); + Ok(()) + } "rags" => print_asset_names("RAGs", &paths::list_rags()), "macros" => { let policy = self.macro_policy(); @@ -3324,9 +3416,11 @@ impl RequestContext { } pub fn delete(&self, kind: &str) -> Result<()> { + if kind == "session" { + return self.delete_sessions(); + } let (dir, file_ext) = match kind { "role" => (paths::roles_dir(), Some(".md")), - "session" => (self.sessions_dir(), Some(".yaml")), "rag" => (paths::rags_dir(), Some(".yaml")), "macro" => (paths::macros_dir(), Some(".yaml")), "skill" => (paths::skills_dir(), None), @@ -3408,6 +3502,82 @@ impl RequestContext { Ok(()) } + fn delete_sessions(&self) -> Result<()> { + let entries = self.session_delete_entries(); + if entries.is_empty() { + bail!("No session to delete") + } + let labels: Vec = entries + .iter() + .map(|(name, scope)| session_delete_label(name, *scope)) + .collect(); + + let selected = MultiSelect::new("Select session to delete:", labels) + .with_validator(|list: &[ListOption<&String>]| { + if list.is_empty() { + Ok(Validation::Invalid( + "At least one item must be selected".into(), + )) + } else { + Ok(Validation::Valid) + } + }) + .raw_prompt()?; + + for option in selected { + let (name, scope) = &entries[option.index]; + self.delete_session_at(name, *scope)?; + } + println!("✓ Successfully deleted session."); + Ok(()) + } + + /// Every session in both scopes, workspace first and NOT deduped: a + /// shadowed global twin must stay deletable. + fn session_delete_entries(&self) -> Vec<(String, SessionScope)> { + [SessionScope::Workspace, SessionScope::Global] + .into_iter() + .flat_map(|scope| { + list_file_names(self.sessions_dir_for(scope), ".yaml") + .into_iter() + .map(move |name| (name, scope)) + }) + .collect() + } + + /// Display rows for `.list sessions`: every file in both scopes, workspace + /// first, with a global twin flagged as shadowed. Names are sanitized for + /// terminal output. + fn session_list_rows(&self) -> Vec<(String, SessionScope, bool)> { + let entries = self.session_delete_entries(); + let workspace_names: HashSet<&str> = entries + .iter() + .filter(|(_, scope)| *scope == SessionScope::Workspace) + .map(|(name, _)| name.as_str()) + .collect(); + entries + .iter() + .map(|(name, scope)| { + let shadowed = + *scope == SessionScope::Global && workspace_names.contains(name.as_str()); + (sanitize_display_text(name), *scope, shadowed) + }) + .collect() + } + + fn delete_session_at(&self, name: &str, scope: SessionScope) -> Result<()> { + let path = self.session_file_for(name, scope); + let shown = sanitize_display_text(name); + remove_file(&path).with_context(|| { + format!( + "Failed to delete {scope} session '{shown}' at '{}'", + path.display() + ) + })?; + println!("Deleted {scope} session '{shown}'"); + Ok(()) + } + pub fn rag_sources(&self) -> Result { match self.rag.as_ref() { Some(rag) => match rag.get_last_sources() { @@ -3885,15 +4055,9 @@ impl RequestContext { .collect(), ".session" => { if args[0].starts_with("_/") { - super::map_completion_values( - self.list_autoname_sessions() - .iter() - .rev() - .map(|v| format!("_/{v}")) - .collect::>(), - ) + self.labeled_autoname_sessions() } else { - super::map_completion_values(self.list_sessions()) + self.labeled_sessions() } } ".rag" => super::map_completion_values(paths::list_rags()), @@ -4337,13 +4501,26 @@ impl RequestContext { .collect(); } else if cmd == ".agent" { if args.len() == 2 { - let dir = paths::agent_data_dir(args[0]).join(SESSIONS_DIR_NAME); - values = list_file_names(dir, ".yaml") + let (workspace_dir, global_dir) = session_scope_dirs(Some(args[0])); + values = labeled_session_names(&workspace_dir, &global_dir) .into_iter() - .map(|v| (v, None)) + .map(|(name, scope)| (sanitize_display_text(&name), Some(scope.to_string()))) .collect(); } values.extend(super::complete_agent_variables(args[0])); + } else if cmd == ".save" && args.first() == Some(&"session") { + if !args.contains(&"--workspace") && !args.contains(&"--global") { + values.push(( + "--workspace".to_string(), + Some(format!( + "Save the session under {WORKSPACE_COYOTE_DIR_NAME}/ in the current workspace" + )), + )); + values.push(( + "--global".to_string(), + Some("Save the session under the global config dir".to_string()), + )); + } } else if args.len() >= 2 { values = self.macro_variable_completions(cmd, &args[..args.len() - 1]); }; @@ -4679,12 +4856,18 @@ impl RequestContext { created_new_session = true; } Some(name) => { - let session_path = self.session_file(name); + let (session_path, scope) = self.resolve_session_file(name); if !session_path.exists() { session = Some(Session::new_from_ctx(self, app, name)?); created_new_session = true; } else { - session = Some(Session::load_from_ctx(self, app, name, &session_path)?); + session = Some(Session::load_from_ctx( + self, + app, + name, + &session_path, + scope, + )?); } } } @@ -5614,6 +5797,16 @@ fn fork_base_name(name: &str) -> &str { name } +fn shadow_warning(scope: SessionScope, resolved_scope: SessionScope, name: &str) -> String { + format!( + "⚠ Saved to {scope}, but a {resolved_scope} session '{name}' shadows it. '.session {name}' will load the {resolved_scope} copy." + ) +} + +fn session_delete_label(name: &str, scope: SessionScope) -> String { + format!("{} ({scope})", sanitize_display_text(name)) +} + /// Assumed summarizer window when neither the compression model nor the /// session model declares `max_input_tokens`. const SUMMARIZATION_WINDOW_FALLBACK_TOKENS: usize = 200_000; @@ -5686,12 +5879,12 @@ mod tests { use super::super::mcp_factory::McpFactory; use super::*; use crate::client::ModelData; - use crate::config::AppState; use crate::config::agent::AgentConfig; use crate::config::bundles::BundleStore; use crate::config::conflict::InstallMode; use crate::config::mcp_tool_policy::LayerSource; use crate::config::tool_scope::test_fixtures::{FixtureServer, fixture_runtime}; + use crate::config::{AppState, WORKSPACE_COYOTE_DIR_NAME}; use crate::function::jobs::RingBuf; use crate::function::{ToolCall, skill}; use crate::hooks::{HookDef, HooksMap, test_sink}; @@ -5756,45 +5949,74 @@ mod tests { let _ = list_all_models(&config); } + /// Pins both the config dir and the workspace config dir to absolute + /// temp paths so no `.coyote/` under the real CWD leaks into the test, + /// and clears the sessions-dir overrides (`COYOTE_SESSIONS_DIR`, plus + /// `HELPER_DATA_DIR` for the `helper` fixture agent) so Global-scope + /// writes cannot escape into the developer's real sessions dir. struct TestConfigDirGuard { - key: String, - previous: Option, + keys: [(String, Option); 4], path: PathBuf, + workspace_root: PathBuf, } impl TestConfigDirGuard { fn new() -> Self { - let key = get_env_name("config_dir"); - let previous = env::var_os(&key); let unique = SystemTime::now() .duration_since(UNIX_EPOCH) .unwrap() .as_nanos(); let path = env::temp_dir().join(format!("coyote-request-context-tests-{unique}")); create_dir_all(&path).unwrap(); - unsafe { - env::set_var(&key, &path); - } + let workspace_root = + env::temp_dir().join(format!("coyote-request-context-ws-{unique}")); + let workspace_path = workspace_root.join(WORKSPACE_COYOTE_DIR_NAME); + create_dir_all(&workspace_path).unwrap(); + let keys = [ + (get_env_name("config_dir"), Some(path.as_os_str())), + ( + get_env_name("workspace_config_dir"), + Some(workspace_path.as_os_str()), + ), + (get_env_name("sessions_dir"), None), + ( + format!("{}_DATA_DIR", utils::normalize_env_name("helper")), + None, + ), + ] + .map(|(key, value)| { + let previous = env::var_os(&key); + unsafe { + match value { + Some(value) => env::set_var(&key, value), + None => env::remove_var(&key), + } + } + (key, previous) + }); Self { - key, - previous, + keys, path, + workspace_root, } } } impl Drop for TestConfigDirGuard { fn drop(&mut self) { - if let Some(previous) = &self.previous { - unsafe { - env::set_var(&self.key, previous); - } - } else { - unsafe { - env::remove_var(&self.key); + for (key, previous) in &self.keys { + if let Some(previous) = previous { + unsafe { + env::set_var(key, previous); + } + } else { + unsafe { + env::remove_var(key); + } } } let _ = remove_dir_all(&self.path); + let _ = remove_dir_all(&self.workspace_root); } } @@ -8690,6 +8912,954 @@ mod tests { assert_eq!(ctx.session.as_ref().unwrap().name(), "my-session"); } + fn write_session_file(ctx: &RequestContext, name: &str, scope: SessionScope, body: &str) { + let path = ctx.session_file_for(name, scope); + ensure_parent_exists(&path).unwrap(); + write(&path, format!("model: test-seeded:test-chat\n{body}")).unwrap(); + } + + #[test] + #[serial] + fn use_session_prefers_workspace_file_over_global() { + let _config = TestConfigDirGuard::new(); + let mut ctx = create_test_ctx(); + write_session_file( + &ctx, + "shared", + SessionScope::Global, + "messages: []\ntodo_list:\n goal: global goal\n", + ); + write_session_file( + &ctx, + "shared", + SessionScope::Workspace, + "messages: []\ntodo_list:\n goal: workspace goal\n", + ); + let app = ctx.app.config.clone(); + + run_async(ctx.use_session(&app, Some("shared"), utils::create_abort_signal())).unwrap(); + + let session = ctx.session.as_ref().unwrap(); + assert_eq!(session.scope(), SessionScope::Workspace); + assert_eq!(session.todo_list().goal, "workspace goal"); + } + + #[test] + #[serial] + fn use_session_new_name_defaults_to_global_scope() { + let _config = TestConfigDirGuard::new(); + let mut ctx = create_test_ctx(); + let app = ctx.app.config.clone(); + + run_async(ctx.use_session(&app, Some("fresh"), utils::create_abort_signal())).unwrap(); + + assert_eq!(ctx.session.as_ref().unwrap().scope(), SessionScope::Global); + ctx.save_session(None, None).unwrap(); + assert!(ctx.session_file_for("fresh", SessionScope::Global).exists()); + assert!( + !ctx.session_file_for("fresh", SessionScope::Workspace) + .exists() + ); + } + + #[test] + #[serial] + fn save_session_workspace_flag_rehomes_session() { + let _config = TestConfigDirGuard::new(); + let mut ctx = create_test_ctx(); + write_session_file(&ctx, "notes", SessionScope::Global, "messages: []\n"); + let app = ctx.app.config.clone(); + run_async(ctx.use_session(&app, Some("notes"), utils::create_abort_signal())).unwrap(); + assert_eq!(ctx.session.as_ref().unwrap().scope(), SessionScope::Global); + + ctx.save_session(None, Some(SessionScope::Workspace)) + .unwrap(); + + let workspace_file = ctx.session_file_for("notes", SessionScope::Workspace); + assert!(workspace_file.exists()); + assert!(ctx.session_file_for("notes", SessionScope::Global).exists()); + assert_eq!( + ctx.session.as_ref().unwrap().scope(), + SessionScope::Workspace + ); + + remove_file(&workspace_file).unwrap(); + ctx.save_session(None, None).unwrap(); + assert!(workspace_file.exists()); + } + + #[test] + #[serial] + fn fork_session_preserves_workspace_scope() { + let _config = TestConfigDirGuard::new(); + let mut ctx = create_test_ctx(); + write_session_file(&ctx, "base", SessionScope::Workspace, "messages: []\n"); + let app = ctx.app.config.clone(); + run_async(ctx.use_session(&app, Some("base"), utils::create_abort_signal())).unwrap(); + + ctx.fork_session(None).unwrap(); + + let session = ctx.session.as_ref().unwrap(); + assert_eq!(session.name(), "base-fork-1"); + assert_eq!(session.scope(), SessionScope::Workspace); + assert!( + ctx.session_file_for("base-fork-1", SessionScope::Workspace) + .exists() + ); + assert!( + !ctx.session_file_for("base-fork-1", SessionScope::Global) + .exists() + ); + } + + #[test] + #[serial] + fn exit_session_autosaves_to_session_scope() { + let _config = TestConfigDirGuard::new(); + let mut ctx = create_test_ctx(); + write_session_file(&ctx, "ws-exit", SessionScope::Workspace, "messages: []\n"); + let app = ctx.app.config.clone(); + run_async(ctx.use_session(&app, Some("ws-exit"), utils::create_abort_signal())).unwrap(); + ctx.set_save_session_this_time().unwrap(); + ctx.init_todo_list("persist me"); + + ctx.exit_session().unwrap(); + + let saved = + read_to_string(ctx.session_file_for("ws-exit", SessionScope::Workspace)).unwrap(); + assert!(saved.contains("persist me")); + assert!( + !ctx.session_file_for("ws-exit", SessionScope::Global) + .exists() + ); + } + + #[test] + #[serial] + fn list_sessions_merges_scopes_workspace_first() { + let _config = TestConfigDirGuard::new(); + let ctx = create_test_ctx(); + write_session_file(&ctx, "both", SessionScope::Global, "messages: []\n"); + write_session_file(&ctx, "both", SessionScope::Workspace, "messages: []\n"); + write_session_file(&ctx, "only-global", SessionScope::Global, "messages: []\n"); + write_session_file(&ctx, "only-ws", SessionScope::Workspace, "messages: []\n"); + + assert_eq!(ctx.list_sessions(), vec!["both", "only-global", "only-ws"]); + assert_eq!( + ctx.resolve_session_file("both"), + ( + ctx.session_file_for("both", SessionScope::Workspace), + SessionScope::Workspace + ) + ); + assert_eq!( + ctx.resolve_session_file("only-global"), + ( + ctx.session_file_for("only-global", SessionScope::Global), + SessionScope::Global + ) + ); + } + + #[test] + #[serial] + fn repl_complete_session_labels_workspace_and_global() { + let _config = TestConfigDirGuard::new(); + let ctx = create_test_ctx(); + write_session_file(&ctx, "both", SessionScope::Global, "messages: []\n"); + write_session_file(&ctx, "both", SessionScope::Workspace, "messages: []\n"); + write_session_file(&ctx, "only-global", SessionScope::Global, "messages: []\n"); + + let mut values = ctx.repl_complete(".session", &[""], ""); + values.sort_unstable(); + + assert_eq!( + values, + vec![ + ("both".to_string(), Some("workspace".to_string())), + ("only-global".to_string(), Some("global".to_string())), + ] + ); + } + + #[test] + fn repl_complete_save_session_offers_scope_flags() { + let ctx = create_test_ctx(); + + let names = |args: &[&str]| -> Vec { + let mut names: Vec = ctx + .repl_complete(".save", args, "") + .into_iter() + .map(|(name, _)| name) + .collect(); + names.sort_unstable(); + names + }; + + assert_eq!(names(&["session", ""]), vec!["--global", "--workspace"]); + assert_eq!(names(&["session", "notes", "--g"]), vec!["--global"]); + assert!(names(&["session", "--workspace", ""]).is_empty()); + assert!(names(&["session", "notes", "--global", ""]).is_empty()); + + for (name, description) in ctx.repl_complete(".save", &["session", ""], "") { + assert!( + description.as_deref().is_some_and(|d| !d.is_empty()), + "{name} has no description" + ); + } + } + + #[test] + #[serial] + fn save_session_failed_write_leaves_scope_and_path_untouched() { + let _config = TestConfigDirGuard::new(); + let mut ctx = create_test_ctx(); + write_session_file(&ctx, "notes", SessionScope::Global, "messages: []\n"); + let app = ctx.app.config.clone(); + run_async(ctx.use_session(&app, Some("notes"), utils::create_abort_signal())).unwrap(); + let before = ctx.session.as_ref().unwrap().export().unwrap(); + write( + ctx.sessions_dir_for(SessionScope::Workspace), + "not a directory", + ) + .unwrap(); + + assert!( + ctx.save_session(None, Some(SessionScope::Workspace)) + .is_err() + ); + + let session = ctx.session.as_ref().unwrap(); + assert_eq!(session.scope(), SessionScope::Global); + assert_eq!(session.export().unwrap(), before); + } + + #[test] + #[serial] + fn save_session_global_copy_stays_shadowed_by_workspace_twin() { + let _config = TestConfigDirGuard::new(); + let mut ctx = create_test_ctx(); + let app = ctx.app.config.clone(); + run_async(ctx.use_session(&app, Some("notes"), utils::create_abort_signal())).unwrap(); + write_session_file(&ctx, "notes", SessionScope::Workspace, "messages: []\n"); + + ctx.save_session(None, None).unwrap(); + + let session_path = ctx.session_file_for("notes", SessionScope::Global); + assert!(session_path.exists()); + assert_eq!(ctx.session.as_ref().unwrap().scope(), SessionScope::Global); + let (resolved_path, resolved_scope) = ctx.resolve_session_file("notes"); + assert_ne!(resolved_path, session_path); + assert_eq!(resolved_scope, SessionScope::Workspace); + } + + #[test] + #[serial] + fn save_session_scoped_temp_session_demands_a_name() { + let _config = TestConfigDirGuard::new(); + let mut ctx = create_test_ctx(); + let app = ctx.app.config.clone(); + run_async(ctx.use_session(&app, None, utils::create_abort_signal())).unwrap(); + + let err = ctx + .save_session(None, Some(SessionScope::Workspace)) + .unwrap_err(); + + assert_eq!( + err.to_string(), + "Name the session first: .save session --workspace" + ); + assert!( + !ctx.session_file_for(TEMP_SESSION_NAME, SessionScope::Workspace) + .exists() + ); + // A Workspace-scoped temp session hits the same guard with no flag. + ctx.session + .as_mut() + .unwrap() + .set_scope(SessionScope::Workspace); + let err = ctx.save_session(None, None).unwrap_err(); + assert_eq!( + err.to_string(), + "Name the session first: .save session --workspace" + ); + assert!( + !ctx.session_file_for(TEMP_SESSION_NAME, SessionScope::Workspace) + .exists() + ); + ctx.save_session(Some("named"), Some(SessionScope::Workspace)) + .unwrap(); + assert!( + ctx.session_file_for("named", SessionScope::Workspace) + .exists() + ); + } + + #[test] + #[serial] + fn fork_session_rejects_name_taken_in_other_scope() { + let _config = TestConfigDirGuard::new(); + let mut ctx = create_test_ctx(); + write_session_file(&ctx, "base", SessionScope::Global, "messages: []\n"); + write_session_file(&ctx, "taken", SessionScope::Workspace, "messages: []\n"); + write_session_file( + &ctx, + "base-fork-1", + SessionScope::Workspace, + "messages: []\n", + ); + let app = ctx.app.config.clone(); + run_async(ctx.use_session(&app, Some("base"), utils::create_abort_signal())).unwrap(); + + let err = ctx.fork_session(Some("taken")).unwrap_err(); + assert_eq!(err.to_string(), "Session 'taken' already exists"); + assert!(!ctx.session_file_for("taken", SessionScope::Global).exists()); + + ctx.fork_session(None).unwrap(); + let session = ctx.session.as_ref().unwrap(); + assert_eq!(session.name(), "base-fork-2"); + assert_eq!(session.scope(), SessionScope::Global); + } + + #[test] + #[serial] + fn fork_session_rejects_the_reserved_temp_name() { + let _config = TestConfigDirGuard::new(); + let mut ctx = create_test_ctx(); + write_session_file(&ctx, "base", SessionScope::Workspace, "messages: []\n"); + let app = ctx.app.config.clone(); + run_async(ctx.use_session(&app, Some("base"), utils::create_abort_signal())).unwrap(); + + let err = ctx.fork_session(Some(TEMP_SESSION_NAME)).unwrap_err(); + assert_eq!( + err.to_string(), + format!("'{TEMP_SESSION_NAME}' is a reserved session name") + ); + assert!( + !ctx.session_file_for(TEMP_SESSION_NAME, SessionScope::Workspace) + .exists() + ); + assert!( + !ctx.session_file_for(TEMP_SESSION_NAME, SessionScope::Global) + .exists() + ); + assert_eq!(ctx.session.as_ref().unwrap().name(), "base"); + } + + #[test] + #[serial] + fn use_session_autoname_prefers_workspace() { + let _config = TestConfigDirGuard::new(); + let mut ctx = create_test_ctx(); + write_session_file( + &ctx, + "_/20260101T000000-x", + SessionScope::Global, + "messages: []\n", + ); + write_session_file( + &ctx, + "_/20260101T000000-x", + SessionScope::Workspace, + "messages: []\n", + ); + let app = ctx.app.config.clone(); + + run_async(ctx.use_session( + &app, + Some("_/20260101T000000-x"), + utils::create_abort_signal(), + )) + .unwrap(); + + let session = ctx.session.as_ref().unwrap(); + assert_eq!(session.scope(), SessionScope::Workspace); + assert_eq!(session.autoname(), Some("x")); + } + + #[test] + #[serial] + fn use_session_temp_branch_stays_global_when_workspace_temp_exists() { + let _config = TestConfigDirGuard::new(); + let mut ctx = create_test_ctx(); + write_session_file( + &ctx, + TEMP_SESSION_NAME, + SessionScope::Workspace, + "messages: []\ntodo_list:\n goal: workspace goal\n", + ); + let app = ctx.app.config.clone(); + + run_async(ctx.use_session(&app, None, utils::create_abort_signal())).unwrap(); + + let session = ctx.session.as_ref().unwrap(); + assert_eq!(session.name(), TEMP_SESSION_NAME); + assert_eq!(session.scope(), SessionScope::Global); + assert!(session.todo_list().goal.is_empty()); + assert!( + ctx.session_file_for(TEMP_SESSION_NAME, SessionScope::Workspace) + .exists() + ); + } + + #[test] + #[serial] + fn repl_complete_agent_sessions_labels_workspace_and_global() { + let _config = TestConfigDirGuard::new(); + let ctx = create_test_ctx(); + for (dir, name) in [ + (paths::workspace_agent_sessions_dir("helper"), "both"), + (paths::agent_sessions_dir("helper"), "both"), + (paths::agent_sessions_dir("helper"), "only-global"), + ] { + create_dir_all(&dir).unwrap(); + write(dir.join(format!("{name}.yaml")), "model: x\n").unwrap(); + } + + let mut values = ctx.repl_complete(".agent", &["helper", ""], ""); + values.sort_unstable(); + + assert_eq!( + values, + vec![ + ("both".to_string(), Some("workspace".to_string())), + ("only-global".to_string(), Some("global".to_string())), + ] + ); + } + + #[test] + #[serial] + fn repl_complete_autoname_sessions_labels_and_orders_newest_first() { + let _config = TestConfigDirGuard::new(); + let ctx = create_test_ctx(); + write_session_file( + &ctx, + "_/20260101T000000-old", + SessionScope::Global, + "messages: []\n", + ); + write_session_file( + &ctx, + "_/20260102T000000-mid", + SessionScope::Global, + "messages: []\n", + ); + write_session_file( + &ctx, + "_/20260102T000000-mid", + SessionScope::Workspace, + "messages: []\n", + ); + write_session_file( + &ctx, + "_/20260103T000000-new", + SessionScope::Workspace, + "messages: []\n", + ); + + let expected = vec![ + ( + "_/20260103T000000-new".to_string(), + Some("workspace".to_string()), + ), + ( + "_/20260102T000000-mid".to_string(), + Some("workspace".to_string()), + ), + ( + "_/20260101T000000-old".to_string(), + Some("global".to_string()), + ), + ]; + assert_eq!(ctx.labeled_autoname_sessions(), expected); + + // The fuzzy filter re-sorts by score, so only membership is checked here. + let mut completed = ctx.repl_complete(".session", &["_/"], ""); + completed.sort_unstable(); + let mut expected_sorted = expected; + expected_sorted.sort_unstable(); + assert_eq!(completed, expected_sorted); + } + + #[test] + #[serial] + fn sysinfo_reports_both_sessions_dirs() { + let _config = TestConfigDirGuard::new(); + let ctx = create_test_ctx(); + let app = ctx.app.config.clone(); + + let info = ctx.sysinfo(&app).unwrap(); + + assert!(info.contains("sessions_dir")); + assert!(info.contains("workspace_sessions_dir")); + } + + // --- usage-probe (spec-first) coverage for the workspace-sessions surface --- + + #[test] + #[serial] + fn usage_probe_sessions_dir_for_is_agent_aware_in_both_scopes() { + let _config = TestConfigDirGuard::new(); + let mut ctx = create_test_ctx(); + ctx.agent = Some(Agent::test_new(AgentConfig { + name: "helper".to_string(), + ..AgentConfig::default() + })); + + assert_eq!( + ctx.sessions_dir_for(SessionScope::Workspace), + paths::workspace_agent_sessions_dir("helper") + ); + assert_eq!( + ctx.sessions_dir_for(SessionScope::Global), + paths::agent_sessions_dir("helper") + ); + // `sessions_dir()` / `session_file()` still mean Global. + assert_eq!( + ctx.sessions_dir(), + ctx.sessions_dir_for(SessionScope::Global) + ); + assert_eq!( + ctx.session_file("notes"), + ctx.session_file_for("notes", SessionScope::Global) + ); + // Autoname subdir layout is preserved in the workspace scope. + assert_eq!( + ctx.session_file_for("_/x", SessionScope::Workspace), + paths::workspace_agent_sessions_dir("helper") + .join("_") + .join("x.yaml") + ); + assert!( + ctx.sessions_dir_for(SessionScope::Workspace) + .starts_with(paths::workspace_config_dir()), + "agent workspace sessions must live under the workspace .coyote dir" + ); + } + + #[test] + #[serial] + fn usage_probe_agent_session_loads_and_autosaves_in_workspace_scope() { + let _config = TestConfigDirGuard::new(); + let mut ctx = create_test_ctx(); + ctx.agent = Some(Agent::test_new(AgentConfig { + name: "helper".to_string(), + ..AgentConfig::default() + })); + write_session_file( + &ctx, + "agent-notes", + SessionScope::Workspace, + "messages: []\n", + ); + let app = ctx.app.config.clone(); + + run_async(ctx.use_session(&app, Some("agent-notes"), utils::create_abort_signal())) + .unwrap(); + assert_eq!( + ctx.session.as_ref().unwrap().scope(), + SessionScope::Workspace + ); + ctx.set_save_session_this_time().unwrap(); + ctx.init_todo_list("agent persist"); + + ctx.exit_agent_session().unwrap(); + + let workspace_file = paths::workspace_agent_sessions_dir("helper").join("agent-notes.yaml"); + assert!( + read_to_string(&workspace_file) + .unwrap() + .contains("agent persist") + ); + assert!( + !paths::agent_sessions_dir("helper") + .join("agent-notes.yaml") + .exists() + ); + assert!( + !paths::workspace_sessions_dir() + .join("agent-notes.yaml") + .exists() + ); + } + + #[test] + #[serial] + fn usage_probe_save_session_global_flag_rehomes_workspace_session_back() { + let _config = TestConfigDirGuard::new(); + let mut ctx = create_test_ctx(); + write_session_file(&ctx, "notes", SessionScope::Workspace, "messages: []\n"); + let app = ctx.app.config.clone(); + run_async(ctx.use_session(&app, Some("notes"), utils::create_abort_signal())).unwrap(); + assert_eq!( + ctx.session.as_ref().unwrap().scope(), + SessionScope::Workspace + ); + + ctx.save_session(None, Some(SessionScope::Global)).unwrap(); + + let global_file = ctx.session_file_for("notes", SessionScope::Global); + let workspace_file = ctx.session_file_for("notes", SessionScope::Workspace); + assert!(global_file.exists()); + assert!( + workspace_file.exists(), + "re-homing must not delete the old file" + ); + assert_eq!(ctx.session.as_ref().unwrap().scope(), SessionScope::Global); + + // Subsequent flagless saves follow the new home. + remove_file(&global_file).unwrap(); + ctx.save_session(None, None).unwrap(); + assert!(global_file.exists()); + + // The workspace twin still shadows the global copy on load. + assert_eq!( + ctx.resolve_session_file("notes"), + (workspace_file, SessionScope::Workspace) + ); + } + + #[test] + #[serial] + fn usage_probe_save_session_rename_without_flag_keeps_workspace_scope() { + let _config = TestConfigDirGuard::new(); + let mut ctx = create_test_ctx(); + write_session_file(&ctx, "draft", SessionScope::Workspace, "messages: []\n"); + let app = ctx.app.config.clone(); + run_async(ctx.use_session(&app, Some("draft"), utils::create_abort_signal())).unwrap(); + + ctx.save_session(Some("final"), None).unwrap(); + + assert!( + ctx.session_file_for("final", SessionScope::Workspace) + .exists() + ); + assert!(!ctx.session_file_for("final", SessionScope::Global).exists()); + assert_eq!( + ctx.session.as_ref().unwrap().scope(), + SessionScope::Workspace + ); + + // Saving with the same scope it already has is a no-op re-home. + ctx.save_session(None, Some(SessionScope::Workspace)) + .unwrap(); + assert_eq!( + ctx.session.as_ref().unwrap().scope(), + SessionScope::Workspace + ); + assert!(!ctx.session_file_for("final", SessionScope::Global).exists()); + } + + #[test] + #[serial] + fn usage_probe_no_workspace_dir_behaves_like_before_and_creates_none() { + let _config = TestConfigDirGuard::new(); + let ctx_probe = create_test_ctx(); + write_session_file(&ctx_probe, "solo", SessionScope::Global, "messages: []\n"); + // A user with no `.coyote/` at all. + remove_dir_all(paths::workspace_config_dir()).unwrap(); + assert!(!paths::workspace_config_dir().exists()); + + let mut ctx = create_test_ctx(); + assert_eq!(ctx.list_sessions(), vec!["solo"]); + assert_eq!( + ctx.resolve_session_file("solo"), + ( + ctx.session_file_for("solo", SessionScope::Global), + SessionScope::Global + ) + ); + assert_eq!( + ctx.repl_complete(".session", &[""], ""), + vec![("solo".to_string(), Some("global".to_string()))] + ); + + // Load existing, save, exit: all global, no workspace dir materialises. + let app = ctx.app.config.clone(); + run_async(ctx.use_session(&app, Some("solo"), utils::create_abort_signal())).unwrap(); + assert_eq!(ctx.session.as_ref().unwrap().scope(), SessionScope::Global); + ctx.save_session(None, None).unwrap(); + ctx.set_save_session_this_time().unwrap(); + ctx.exit_session().unwrap(); + + // Brand-new named session + temp session likewise. + run_async(ctx.use_session(&app, Some("fresh"), utils::create_abort_signal())).unwrap(); + assert_eq!(ctx.session.as_ref().unwrap().scope(), SessionScope::Global); + ctx.save_session(None, None).unwrap(); + ctx.fork_session(None).unwrap(); + ctx.exit_session().unwrap(); + run_async(ctx.use_session(&app, None, utils::create_abort_signal())).unwrap(); + ctx.exit_session().unwrap(); + + assert!(ctx.session_file_for("fresh", SessionScope::Global).exists()); + assert!( + ctx.session_file_for("fresh-fork-1", SessionScope::Global) + .exists() + ); + assert!( + !paths::workspace_config_dir().exists(), + "no session operation may create the workspace .coyote dir" + ); + assert_eq!(ctx.list_sessions(), vec!["fresh", "fresh-fork-1", "solo"]); + } + + #[test] + #[serial] + fn usage_probe_session_info_reports_scope() { + let _config = TestConfigDirGuard::new(); + let mut ctx = create_test_ctx(); + write_session_file(&ctx, "ws-info", SessionScope::Workspace, "messages: []\n"); + let app = ctx.app.config.clone(); + run_async(ctx.use_session(&app, Some("ws-info"), utils::create_abort_signal())).unwrap(); + + let info = ctx.session_info(&app).unwrap(); + assert!( + info.contains("scope"), + "session info missing scope key: {info}" + ); + assert!( + info.contains("workspace"), + "session info missing scope value: {info}" + ); + ctx.exit_session().unwrap(); + + run_async(ctx.use_session(&app, Some("new-global"), utils::create_abort_signal())).unwrap(); + let info = ctx.session_info(&app).unwrap(); + assert!( + info.contains("scope"), + "session info missing scope key: {info}" + ); + assert!( + info.contains("global"), + "session info missing scope value: {info}" + ); + } + + #[test] + #[serial] + fn usage_probe_workspace_temp_twin_is_ignored_and_untouched_on_exit() { + let _config = TestConfigDirGuard::new(); + let mut ctx = create_test_ctx(); + write_session_file( + &ctx, + TEMP_SESSION_NAME, + SessionScope::Workspace, + "messages: []\ntodo_list:\n goal: keep me\n", + ); + let app = ctx.app.config.clone(); + + run_async(ctx.use_session(&app, None, utils::create_abort_signal())).unwrap(); + ctx.set_save_session_this_time().unwrap(); + ctx.init_todo_list("temp global"); + ctx.exit_session().unwrap(); + + let workspace_temp = ctx.session_file_for(TEMP_SESSION_NAME, SessionScope::Workspace); + assert!(read_to_string(&workspace_temp).unwrap().contains("keep me")); + // A saved temp session is autonamed under `_/` in its own (global) scope. + let autonamed = ctx.labeled_autoname_sessions(); + assert_eq!( + autonamed.len(), + 1, + "expected one autonamed session: {autonamed:?}" + ); + assert_eq!(autonamed[0].1.as_deref(), Some("global")); + let saved = ctx.session_file_for(&autonamed[0].0, SessionScope::Global); + assert!(read_to_string(&saved).unwrap().contains("temp global")); + assert!( + !ctx.sessions_dir_for(SessionScope::Workspace) + .join("_") + .exists() + ); + // The workspace twin still doesn't hijack the next temp session. + run_async(ctx.use_session(&app, None, utils::create_abort_signal())).unwrap(); + assert_eq!(ctx.session.as_ref().unwrap().scope(), SessionScope::Global); + assert!(ctx.session.as_ref().unwrap().todo_list().goal.is_empty()); + } + + // Usage probe (spec-first, criteria 3+4): the reserved `temp` name can + // never land in Workspace scope — not via the bare flag, and not by + // spelling the name out either. The rejection is a no-op: no workspace + // `temp.yaml`, and the live session keeps its Global scope/path so a + // later re-home under a real name still works. + #[test] + #[serial] + fn usage_probe_explicit_temp_name_into_workspace_is_rejected_without_side_effects() { + let _config = TestConfigDirGuard::new(); + let mut ctx = create_test_ctx(); + let app = ctx.app.config.clone(); + run_async(ctx.use_session(&app, None, utils::create_abort_signal())).unwrap(); + let before = ctx.session.as_ref().unwrap().export().unwrap(); + + let err = ctx + .save_session(Some(TEMP_SESSION_NAME), Some(SessionScope::Workspace)) + .unwrap_err(); + assert!( + err.to_string().contains("--workspace") && err.to_string().contains(""), + "teaching error should point at the flag grammar: {err}" + ); + assert!( + !ctx.session_file_for(TEMP_SESSION_NAME, SessionScope::Workspace) + .exists() + ); + assert!( + !ctx.sessions_dir_for(SessionScope::Workspace).exists(), + "a rejected save must not even create the workspace sessions dir" + ); + let session = ctx.session.as_ref().unwrap(); + assert_eq!(session.name(), TEMP_SESSION_NAME); + assert_eq!(session.scope(), SessionScope::Global); + assert_eq!( + session.export().unwrap(), + before, + "rejected save must leave name/scope/path untouched" + ); + + // The same session can still be re-homed under a real name. + ctx.save_session(Some("real"), Some(SessionScope::Workspace)) + .unwrap(); + let session = ctx.session.as_ref().unwrap(); + assert_eq!(session.scope(), SessionScope::Workspace); + assert!( + ctx.session_file_for("real", SessionScope::Workspace) + .exists() + ); + assert!(!ctx.session_file_for("real", SessionScope::Global).exists()); + } + + #[test] + #[serial] + fn config_dir_guard_clears_and_restores_the_sessions_dir_override() { + let key = get_env_name("sessions_dir"); + let junk = PathBuf::from("/nonexistent/junk-sessions"); + let _outer = crate::testing::EnvVarGuard::set(&key, &junk); + let _outer_agent = crate::testing::EnvVarGuard::set("HELPER_DATA_DIR", &junk); + { + let guard = TestConfigDirGuard::new(); + assert!(crate::config::default_sessions_dir().starts_with(&guard.path)); + assert!( + paths::agent_sessions_dir("helper").starts_with(&guard.path), + "guard must clear the fixture agent's data-dir override" + ); + } + assert_eq!(crate::config::default_sessions_dir(), junk); + assert_eq!(paths::agent_data_dir("helper"), junk); + } + + #[test] + #[serial] + fn session_delete_entries_list_both_twins_and_delete_one_scope_at_a_time() { + let _config = TestConfigDirGuard::new(); + let ctx = create_test_ctx(); + write_session_file(&ctx, "both", SessionScope::Global, "messages: []\n"); + write_session_file(&ctx, "both", SessionScope::Workspace, "messages: []\n"); + write_session_file(&ctx, "only-global", SessionScope::Global, "messages: []\n"); + + let entries = ctx.session_delete_entries(); + assert_eq!( + entries, + vec![ + ("both".to_string(), SessionScope::Workspace), + ("both".to_string(), SessionScope::Global), + ("only-global".to_string(), SessionScope::Global), + ] + ); + let labels: Vec = entries + .iter() + .map(|(name, scope)| session_delete_label(name, *scope)) + .collect(); + assert_eq!(labels[..2], ["both (workspace)", "both (global)"]); + + ctx.delete_session_at("both", SessionScope::Workspace) + .unwrap(); + assert!( + !ctx.session_file_for("both", SessionScope::Workspace) + .exists() + ); + assert!(ctx.session_file_for("both", SessionScope::Global).exists()); + } + + #[test] + #[serial] + fn session_list_rows_show_both_twins_and_mark_the_shadowed_global() { + let _config = TestConfigDirGuard::new(); + let ctx = create_test_ctx(); + write_session_file(&ctx, "both", SessionScope::Global, "messages: []\n"); + write_session_file(&ctx, "both", SessionScope::Workspace, "messages: []\n"); + write_session_file(&ctx, "only-ws", SessionScope::Workspace, "messages: []\n"); + write_session_file(&ctx, "only-global", SessionScope::Global, "messages: []\n"); + + assert_eq!( + ctx.session_list_rows(), + vec![ + ("both".to_string(), SessionScope::Workspace, false), + ("only-ws".to_string(), SessionScope::Workspace, false), + ("both".to_string(), SessionScope::Global, true), + ("only-global".to_string(), SessionScope::Global, false), + ] + ); + } + + #[test] + #[serial] + fn session_list_rows_strip_terminal_escapes() { + let _config = TestConfigDirGuard::new(); + let ctx = create_test_ctx(); + write_session_file( + &ctx, + "evil\x1b[31mred", + SessionScope::Global, + "messages: []\n", + ); + + assert_eq!( + ctx.session_list_rows(), + vec![("evilred".to_string(), SessionScope::Global, false)] + ); + } + + #[test] + fn session_labels_strip_terminal_escapes() { + let label = session_delete_label("evil\x1b[31mred", SessionScope::Global); + assert!(!label.contains('\x1b')); + assert_eq!(label, "evilred (global)"); + } + + #[test] + #[serial] + fn repl_complete_session_strips_terminal_escapes_from_names() { + let _config = TestConfigDirGuard::new(); + let ctx = create_test_ctx(); + write_session_file( + &ctx, + "evil\x1b[31mred", + SessionScope::Workspace, + "messages: []\n", + ); + + let values = ctx.repl_complete(".session", &[""], ""); + assert_eq!( + values, + vec![("evilred".to_string(), Some("workspace".to_string()))] + ); + } + + #[test] + fn shadow_warning_names_both_scopes_and_the_load_command() { + assert_eq!( + shadow_warning(SessionScope::Global, SessionScope::Workspace, "notes"), + "⚠ Saved to global, but a workspace session 'notes' shadows it. '.session notes' will load the workspace copy." + ); + } + + #[test] + fn shadow_warning_call_site_strips_terminal_escapes_from_names() { + let shown = sanitize_display_text("evil\x1b[31mred"); + let warning = shadow_warning(SessionScope::Global, SessionScope::Workspace, &shown); + assert!(!warning.contains('\x1b'), "got: {warning:?}"); + assert!(warning.contains("'evilred'"), "got: {warning:?}"); + } + #[test] #[serial] fn exit_session_roundtrip() { diff --git a/src/config/session.rs b/src/config/session.rs index 784104a..f344db6 100644 --- a/src/config/session.rs +++ b/src/config/session.rs @@ -12,8 +12,10 @@ use inquire::{Confirm, Text, validator::Validation}; use serde::{Deserialize, Serialize}; use serde_json::json; use std::collections::HashMap; -use std::fs::{read_to_string, write}; -use std::path::Path; +use std::fmt; +use std::fs::{OpenOptions, read_to_string, remove_file, rename}; +use std::io::{self, Write as _}; +use std::path::{Path, PathBuf}; use std::sync::LazyLock; static RE_AUTONAME_PREFIX: LazyLock = LazyLock::new(|| Regex::new(r"\d{8}T\d{6}-").unwrap()); @@ -29,6 +31,62 @@ fn cost_is_zero(v: &f64) -> bool { *v == 0.0 } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum SessionScope { + #[default] + Global, + Workspace, +} + +impl fmt::Display for SessionScope { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + SessionScope::Global => write!(f, "global"), + SessionScope::Workspace => write!(f, "workspace"), + } + } +} + +pub fn session_scope_dirs(agent: Option<&str>) -> (PathBuf, PathBuf) { + match agent { + Some(agent) => ( + paths::workspace_agent_sessions_dir(agent), + paths::agent_sessions_dir(agent), + ), + None => (paths::workspace_sessions_dir(), default_sessions_dir()), + } +} + +/// Session names across both scope dirs, workspace first, deduped by name so +/// a workspace session shadows a global one with the same name. +pub fn labeled_session_names( + workspace_dir: &Path, + global_dir: &Path, +) -> Vec<(String, SessionScope)> { + let mut values = Vec::new(); + let mut seen = HashSet::new(); + for (dir, scope) in [ + (workspace_dir, SessionScope::Workspace), + (global_dir, SessionScope::Global), + ] { + for name in list_file_names(dir, ".yaml") { + if seen.insert(name.clone()) { + values.push((name, scope)); + } + } + } + values +} + +pub fn merged_session_names(workspace_dir: &Path, global_dir: &Path) -> Vec { + let mut names: Vec = labeled_session_names(workspace_dir, global_dir) + .into_iter() + .map(|(name, _)| name) + .collect(); + names.sort_unstable(); + names +} + #[derive(Debug, Clone, Default, Deserialize, Serialize)] pub struct Session { #[serde(rename(serialize = "model", deserialize = "model"))] @@ -120,6 +178,8 @@ pub struct Session { #[serde(skip)] path: Option, #[serde(skip)] + scope: SessionScope, + #[serde(skip)] dirty: bool, #[serde(skip)] save_session_this_time: bool, @@ -173,6 +233,7 @@ impl Session { app: &AppConfig, name: &str, path: &Path, + scope: SessionScope, ) -> Result { let content = read_to_string(path) .with_context(|| format!("Failed to load session {} at {}", name, path.display()))?; @@ -180,6 +241,7 @@ impl Session { serde_yaml::from_str(&content).with_context(|| format!("Invalid session {name}"))?; session.model = Model::retrieve_model(app, &session.model_id, ModelType::Chat)?; + session.scope = scope; if let Some(autoname) = name.strip_prefix("_/") { session.name = TEMP_SESSION_NAME.to_string(); @@ -250,6 +312,14 @@ impl Session { self.name = name; } + pub fn scope(&self) -> SessionScope { + self.scope + } + + pub fn set_scope(&mut self, scope: SessionScope) { + self.scope = scope; + } + pub fn clear_autoname(&mut self) { self.autoname = None; } @@ -313,6 +383,7 @@ impl Session { pub fn export(&self) -> Result { let mut data = json!({ "path": self.path, + "scope": self.scope.to_string(), "model": self.model().id(), }); if let Some(temperature) = self.temperature() { @@ -403,6 +474,7 @@ impl Session { if let Some(path) = &self.path { items.push(("path", path.to_string())); } + items.push(("scope", self.scope.to_string())); if let Some(autoname) = self.autoname() { items.push(("autoname", autoname.to_string())); @@ -886,17 +958,52 @@ impl Session { pub fn save(&mut self, session_name: &str, session_path: &Path, is_repl: bool) -> Result<()> { ensure_parent_exists(session_path)?; - self.path = Some(session_path.display().to_string()); - let content = serde_yaml::to_string(&self) .with_context(|| format!("Failed to serde session '{}'", self.name))?; - write(session_path, content).with_context(|| { + // Write through a sibling temp file and rename it over the target so a + // symlink planted at the session path is replaced, never followed. The + // temp file is opened with create_new so a symlink planted at the temp + // path is not followed either; a stale entry there is removed and the + // open retried once. + let mut tmp_name = session_path.as_os_str().to_os_string(); + tmp_name.push(format!(".{}.tmp", process::id())); + let tmp_path = PathBuf::from(tmp_name); + let write_context = || { format!( "Failed to write session '{}' to '{}'", self.name, session_path.display() ) - })?; + }; + let open_tmp = || { + OpenOptions::new() + .write(true) + .create_new(true) + .open(&tmp_path) + }; + let mut tmp_file = match open_tmp() { + Ok(file) => file, + Err(error) if error.kind() == io::ErrorKind::AlreadyExists => { + remove_file(&tmp_path).with_context(write_context)?; + open_tmp().with_context(write_context)? + } + Err(error) => return Err(error).with_context(write_context), + }; + if let Err(error) = tmp_file + .write_all(content.as_bytes()) + .and_then(|_| tmp_file.flush()) + { + drop(tmp_file); + let _ = remove_file(&tmp_path); + return Err(error).with_context(write_context); + } + drop(tmp_file); + if let Err(error) = rename(&tmp_path, session_path) { + let _ = remove_file(&tmp_path); + return Err(error).with_context(write_context); + } + + self.path = Some(session_path.display().to_string()); if is_repl { println!("✓ Saved the session to '{}'.", session_path.display()); @@ -1317,6 +1424,16 @@ mod tests { ); } + #[test] + fn session_scope_is_not_serialized() { + let mut session = Session::default(); + session.set_scope(SessionScope::Workspace); + + let yaml = serde_yaml::to_string(&session).unwrap(); + + assert!(!yaml.contains("scope")); + } + #[test] fn session_deserialization_ignores_injected_role_hooks() { // A session file is attacker-writable data: a `role_hooks:` key @@ -1356,8 +1473,10 @@ mod tests { fn session_export_includes_real_usage_only_when_reported() { let empty = Session::default().export().unwrap(); assert!(!empty.contains("real_input_tokens")); + assert!(empty.contains("scope: global")); let mut session = Session::default(); + session.set_scope(SessionScope::Workspace); session.accumulate_token_usage(&TokenUsage { input_tokens: Some(5), output_tokens: Some(7), @@ -1365,12 +1484,97 @@ mod tests { cache_read_input_tokens: Some(3), }); let exported = session.export().unwrap(); + assert!(exported.contains("scope: workspace")); assert!(exported.contains("real_input_tokens: 5")); assert!(exported.contains("real_output_tokens: 7")); assert!(exported.contains("cache_read_tokens: 3")); assert!(!exported.contains("cache_creation_tokens")); } + fn rendered_lines(session: &Session) -> Vec { + let options = crate::render::RenderOptions { + raw_markdown: true, + ..Default::default() + }; + let mut render = MarkdownRender::init(options).unwrap(); + session + .render(&mut render, &None) + .unwrap() + .lines() + .map(str::to_string) + .collect() + } + + #[test] + fn session_render_reports_scope_line() { + let mut session = Session::default(); + assert!( + rendered_lines(&session) + .iter() + .any(|line| line.starts_with("scope") && line.ends_with("global")) + ); + + session.set_scope(SessionScope::Workspace); + assert!( + rendered_lines(&session) + .iter() + .any(|line| line.starts_with("scope") && line.ends_with("workspace")) + ); + } + + #[cfg(unix)] + #[test] + fn session_save_replaces_a_symlink_instead_of_following_it() { + let unique = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos(); + let dir = std::env::temp_dir().join(format!("coyote-session-save-symlink-{unique}")); + let sessions_dir = dir.join("sessions"); + std::fs::create_dir_all(&sessions_dir).unwrap(); + let target = dir.join("target.txt"); + std::fs::write(&target, "untouched").unwrap(); + let link = sessions_dir.join("link.yaml"); + std::os::unix::fs::symlink(&target, &link).unwrap(); + + let mut session = Session::default(); + session.save("link", &link, false).unwrap(); + + assert_eq!(read_to_string(&target).unwrap(), "untouched"); + assert!(link.symlink_metadata().unwrap().is_file()); + assert!(read_to_string(&link).unwrap().contains("model:")); + assert_eq!(session.name(), "link"); + std::fs::remove_dir_all(&dir).unwrap(); + } + + #[cfg(unix)] + #[test] + fn session_save_does_not_follow_a_symlink_planted_at_the_temp_path() { + let unique = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos(); + let dir = std::env::temp_dir().join(format!("coyote-session-save-tmp-symlink-{unique}")); + let sessions_dir = dir.join("sessions"); + std::fs::create_dir_all(&sessions_dir).unwrap(); + let target = dir.join("target.txt"); + std::fs::write(&target, "untouched").unwrap(); + let destination = sessions_dir.join("notes.yaml"); + let mut tmp_name = destination.as_os_str().to_os_string(); + tmp_name.push(format!(".{}.tmp", process::id())); + let tmp_path = PathBuf::from(tmp_name); + std::os::unix::fs::symlink(&target, &tmp_path).unwrap(); + + let mut session = Session::default(); + session.save("notes", &destination, false).unwrap(); + + assert_eq!(read_to_string(&target).unwrap(), "untouched"); + assert!(destination.symlink_metadata().unwrap().is_file()); + assert!(read_to_string(&destination).unwrap().contains("model:")); + assert!(!tmp_path.exists(), "temp file must not be left behind"); + std::fs::remove_dir_all(&dir).unwrap(); + } + #[test] fn session_todo_state_survives_yaml_round_trip() { let mut session = Session::default(); diff --git a/src/repl/mod.rs b/src/repl/mod.rs index 2807d58..2cd2ac9 100644 --- a/src/repl/mod.rs +++ b/src/repl/mod.rs @@ -13,7 +13,8 @@ use crate::client::{ }; use crate::config::{ AgentVariables, AppConfig, AssertState, Input, LastMessage, MacroState, RequestContext, - StateFlags, flatten_prompt_messages, macro_execute, resolve_prompt_args, sanitize_display_text, + SessionScope, StateFlags, flatten_prompt_messages, macro_execute, resolve_prompt_args, + sanitize_display_text, }; use crate::config::{AssetCategory, paths}; use crate::function::agents::{GuardrailAction, check_pending_tasks_guardrail}; @@ -1088,11 +1089,24 @@ pub async fn run_repl_command( Some(("role", name)) => { ctx.save_role(name)?; } - Some(("session", name)) => { - ctx.save_session(name)?; - } + Some(("session", rest)) => match parse_repl_save_session(rest) { + ReplSaveSessionDispatch::Run { name, scope } => { + ctx.save_session(name.as_deref(), scope)?; + } + ReplSaveSessionDispatch::Help => println!("{}", repl_save_session_help()), + ReplSaveSessionDispatch::Conflict => { + println!("--workspace and --global are mutually exclusive") + } + ReplSaveSessionDispatch::Usage => { + println!( + "Usage: .save session [name] [--workspace|--global] (see `.save session --help`)" + ) + } + }, _ => { - println!(r#"Usage: .save [name]"#) + println!( + r#"Usage: .save role [name] | .save session [name] [--workspace|--global]"# + ) } }, ".edit" => { @@ -1934,6 +1948,61 @@ fn parse_repl_uninstall(args: Option<&str>) -> ReplUninstallDispatch { } } +fn repl_save_session_help() -> String { + r#"Save the current session to a file. + +Usage: + .save session [name] [--workspace|--global] + +Flags: + --workspace Save under .coyote/ in the current workspace + --global Save under the global config dir + +Without a flag the session is re-saved wherever it currently lives; a brand-new +session lives in the global scope. Re-homing a session never deletes the old file."# + .to_string() +} + +#[derive(Debug, PartialEq)] +enum ReplSaveSessionDispatch { + Run { + name: Option, + scope: Option, + }, + Help, + Conflict, + Usage, +} + +fn parse_repl_save_session(args: Option<&str>) -> ReplSaveSessionDispatch { + let tokens: Vec<&str> = args.unwrap_or("").split_whitespace().collect(); + if tokens + .iter() + .any(|token| *token == "--help" || *token == "-h") + { + return ReplSaveSessionDispatch::Help; + } + let mut scope = None; + let mut names = Vec::new(); + for token in tokens { + let requested = match token { + "--workspace" => SessionScope::Workspace, + "--global" => SessionScope::Global, + other if other.starts_with('-') => return ReplSaveSessionDispatch::Usage, + other => { + names.push(other); + continue; + } + }; + if scope.is_some_and(|current| current != requested) { + return ReplSaveSessionDispatch::Conflict; + } + scope = Some(requested); + } + let name = (!names.is_empty()).then(|| names.join(" ")); + ReplSaveSessionDispatch::Run { name, scope } +} + pub fn builtin_command_names() -> Vec<&'static str> { let mut names: Vec<&'static str> = REPL_COMMANDS .iter() @@ -3207,6 +3276,144 @@ mod tests { assert_eq!(parse_repl_uninstall(None), ReplUninstallDispatch::Usage); } + #[test] + fn parse_repl_save_session_routes_name_and_scope_in_either_order() { + assert_eq!( + parse_repl_save_session(None), + ReplSaveSessionDispatch::Run { + name: None, + scope: None + } + ); + assert_eq!( + parse_repl_save_session(Some("notes")), + ReplSaveSessionDispatch::Run { + name: Some("notes".to_string()), + scope: None + } + ); + assert_eq!( + parse_repl_save_session(Some("notes --workspace")), + ReplSaveSessionDispatch::Run { + name: Some("notes".to_string()), + scope: Some(SessionScope::Workspace) + } + ); + assert_eq!( + parse_repl_save_session(Some("--global notes")), + ReplSaveSessionDispatch::Run { + name: Some("notes".to_string()), + scope: Some(SessionScope::Global) + } + ); + assert_eq!( + parse_repl_save_session(Some("--workspace")), + ReplSaveSessionDispatch::Run { + name: None, + scope: Some(SessionScope::Workspace) + } + ); + } + + #[test] + fn parse_repl_save_session_rejects_conflicts_and_unknown_flags_but_joins_spaced_names() { + assert_eq!( + parse_repl_save_session(Some("--workspace --global")), + ReplSaveSessionDispatch::Conflict + ); + assert_eq!( + parse_repl_save_session(Some("notes --force")), + ReplSaveSessionDispatch::Usage + ); + assert_eq!( + parse_repl_save_session(Some("one two")), + ReplSaveSessionDispatch::Run { + name: Some("one two".to_string()), + scope: None + } + ); + assert_eq!( + parse_repl_save_session(Some("--help")), + ReplSaveSessionDispatch::Help + ); + assert_eq!( + parse_repl_save_session(Some("notes -h")), + ReplSaveSessionDispatch::Help + ); + assert_eq!( + parse_repl_save_session(Some("--workspace --global -h")), + ReplSaveSessionDispatch::Help + ); + assert_eq!( + parse_repl_save_session(Some("--global notes --global")), + ReplSaveSessionDispatch::Run { + name: Some("notes".to_string()), + scope: Some(SessionScope::Global) + } + ); + } + + #[test] + fn repl_save_session_help_text_covers_the_full_surface() { + let help = repl_save_session_help(); + for needle in [ + "--workspace", + "--global", + "[name]", + "never deletes the old file", + ] { + assert!(help.contains(needle), "save session help missing {needle}"); + } + } + + // Usage probe (spec-first, criterion 5): `-h/--help` wins from ANY + // position — including when it is preceded by a token that would + // otherwise route to Usage (unknown `-` flag) or Conflict (both scope + // flags) — while, absent help, an unknown `-` token still routes to + // Usage even when a valid scope flag is present, and a repeated + // identical flag stays idempotent for `--workspace` too. + #[test] + fn usage_probe_save_session_help_prescan_beats_usage_and_conflict() { + for input in [ + "--bogus -h", + "-h --bogus", + "notes --bogus --help", + "--workspace --global notes -h", + "--help --workspace --global", + "one -h two", + ] { + assert_eq!( + parse_repl_save_session(Some(input)), + ReplSaveSessionDispatch::Help, + "help must win for {input:?}" + ); + } + + assert_eq!( + parse_repl_save_session(Some("--workspace --bogus notes")), + ReplSaveSessionDispatch::Usage + ); + assert_eq!( + parse_repl_save_session(Some("-x")), + ReplSaveSessionDispatch::Usage + ); + assert_eq!( + parse_repl_save_session(Some("--workspace notes --workspace")), + ReplSaveSessionDispatch::Run { + name: Some("notes".to_string()), + scope: Some(SessionScope::Workspace) + } + ); + // Non-flag tokens join into ONE name even when a flag sits between them. + assert_eq!( + parse_repl_save_session(Some("one --global two")), + ReplSaveSessionDispatch::Run { + name: Some("one two".to_string()), + scope: Some(SessionScope::Global) + } + ); + } + #[test] fn repl_install_and_uninstall_help_text_cover_the_full_surface() { let install = repl_install_help(); diff --git a/src/testing.rs b/src/testing.rs index 00957d6..4078250 100644 --- a/src/testing.rs +++ b/src/testing.rs @@ -1,7 +1,10 @@ //! Test-only helpers shared across modules' unit tests. +use crate::config::WORKSPACE_COYOTE_DIR_NAME; +use crate::utils; use log::{Level, LevelFilter, Log, Metadata, Record}; use std::ffi::{OsStr, OsString}; +use std::path::PathBuf; use std::sync::{Mutex, Once, OnceLock}; struct TestLogCollector; @@ -147,12 +150,19 @@ impl Drop for EnvVarGuard { } } -/// Points the config dir at a fresh temp directory for the guard's lifetime -/// and removes it on drop, including on panic. Tests using it must serialize -/// (`#[serial]`) — the config-dir env var is process-global. +/// Points the config dir and the workspace config dir at fresh temp +/// directories for the guard's lifetime and removes them on drop, including +/// on panic, so the developer's real `./.coyote/` never leaks into a test. +/// The sessions-dir override is cleared too, so Global-scope session writes +/// land under the temp config dir rather than the developer's real one. +/// Tests using it must serialize (`#[serial]`) — the env vars are +/// process-global. pub(crate) struct TestConfigDirGuard { _env: EnvVarGuard, - pub(crate) path: std::path::PathBuf, + _workspace_env: EnvVarGuard, + _sessions_env: EnvVarGuard, + pub(crate) path: PathBuf, + workspace_path: PathBuf, } impl TestConfigDirGuard { @@ -163,9 +173,19 @@ impl TestConfigDirGuard { .as_nanos(); let path = std::env::temp_dir().join(format!("coyote-{label}-{unique}")); std::fs::create_dir_all(&path).unwrap(); + let workspace_path = std::env::temp_dir() + .join(format!("coyote-{label}-ws-{unique}")) + .join(WORKSPACE_COYOTE_DIR_NAME); + std::fs::create_dir_all(&workspace_path).unwrap(); Self { - _env: EnvVarGuard::set(crate::utils::get_env_name("config_dir"), &path), + _env: EnvVarGuard::set(utils::get_env_name("config_dir"), &path), + _workspace_env: EnvVarGuard::set( + utils::get_env_name("workspace_config_dir"), + &workspace_path, + ), + _sessions_env: EnvVarGuard::unset(utils::get_env_name("sessions_dir")), path, + workspace_path, } } } @@ -173,5 +193,8 @@ impl TestConfigDirGuard { impl Drop for TestConfigDirGuard { fn drop(&mut self) { let _ = std::fs::remove_dir_all(&self.path); + if let Some(root) = self.workspace_path.parent() { + let _ = std::fs::remove_dir_all(root); + } } } diff --git a/tests/inspection_flags.rs b/tests/inspection_flags.rs index bdfb24e..89580a6 100644 --- a/tests/inspection_flags.rs +++ b/tests/inspection_flags.rs @@ -15,11 +15,11 @@ //! a real ~/.coyote_password or a sandbox shell — cannot mask //! virgin-environment regressions. -use std::env; use std::fs; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use std::time::{SystemTime, UNIX_EPOCH}; +use std::{env, process}; fn fresh_config_dir(label: &str) -> PathBuf { let unique = SystemTime::now() @@ -438,3 +438,109 @@ fn sync_models_stays_on_bootstrap_path() { "--sync-models: expected the builtins bootstrap to populate the config dir" ); } + +fn run_list_sessions( + cwd: &Path, + config_dir: &Path, + global_sessions: &Path, + home_dir: &Path, +) -> process::Output { + Command::new(env!("CARGO_BIN_EXE_coyote")) + .arg("--list-sessions") + .env("COYOTE_CONFIG_DIR", config_dir) + .env("COYOTE_SESSIONS_DIR", global_sessions) + .env("HOME", home_dir) + .env("USERPROFILE", home_dir) + .env_remove("IS_SANDBOX") + .env_remove("COYOTE_PROVIDER") + .env_remove("COYOTE_PLATFORM") + .env_remove("COYOTE_WORKSPACE_CONFIG_DIR") + .current_dir(cwd) + .stdin(Stdio::null()) + .output() + .unwrap() +} + +fn seed_session(dir: &Path, name: &str) { + fs::create_dir_all(dir).unwrap(); + fs::write(dir.join(format!("{name}.yaml")), "messages: []\n").unwrap(); +} + +#[test] +fn usage_probe_list_sessions_merges_cwd_workspace_scope_with_global() { + let config_dir = fresh_config_dir("list-sessions-ws"); + let home_dir = fresh_config_dir("list-sessions-ws-home"); + let global_sessions = fresh_config_dir("list-sessions-ws-global"); + let workspace = fresh_config_dir("list-sessions-ws-cwd"); + let bare_cwd = fresh_config_dir("list-sessions-ws-bare"); + let workspace_sessions = workspace.join(".coyote").join("sessions"); + + seed_session(&global_sessions, "both"); + seed_session(&global_sessions, "glob-only"); + seed_session(&workspace_sessions, "both"); + seed_session(&workspace_sessions, "ws-only"); + + let in_workspace = run_list_sessions(&workspace, &config_dir, &global_sessions, &home_dir); + let in_bare = run_list_sessions(&bare_cwd, &config_dir, &global_sessions, &home_dir); + + let config_leftover: Vec<_> = fs::read_dir(&config_dir) + .unwrap() + .map(|entry| entry.unwrap().file_name()) + .collect(); + let bare_created_workspace = bare_cwd.join(".coyote").exists(); + let workspace_entries: Vec<_> = fs::read_dir(&workspace_sessions) + .unwrap() + .map(|entry| entry.unwrap().file_name().to_string_lossy().into_owned()) + .collect(); + for dir in [ + &config_dir, + &home_dir, + &global_sessions, + &workspace, + &bare_cwd, + ] { + let _ = fs::remove_dir_all(dir); + } + + for (label, output) in [("workspace", &in_workspace), ("bare", &in_bare)] { + assert!( + output.status.success(), + "--list-sessions ({label}): expected exit 0, got {:?}\nstdout: {}\nstderr: {}", + output.status, + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + } + + let lines = |output: &process::Output| -> Vec { + String::from_utf8_lossy(&output.stdout) + .lines() + .map(str::to_owned) + .collect() + }; + assert_eq!( + lines(&in_workspace), + vec!["both", "glob-only", "ws-only"], + "workspace CWD: merged, deduplicated, sorted" + ); + assert_eq!( + lines(&in_bare), + vec!["both", "glob-only"], + "CWD without .coyote/: behaviour unchanged (global listing only)" + ); + assert!( + !bare_created_workspace, + "--list-sessions must never create .coyote/ in the CWD" + ); + assert!( + config_leftover.is_empty(), + "--list-sessions must write nothing into the config dir: {config_leftover:?}" + ); + let mut workspace_entries = workspace_entries; + workspace_entries.sort_unstable(); + assert_eq!( + workspace_entries, + vec!["both.yaml", "ws-only.yaml"], + "listing is read-only for the workspace sessions dir" + ); +}