diff --git a/README.md b/README.md index 4143a81..b9b00df 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,10 @@ Coming from [AIChat](https://github.com/sigoden/aichat)? Follow the [migration g * [Create Custom Bash Tools](https://github.com/Dark-Alex-17/coyote/wiki/Custom-Bash-Tools) * [Bash Prompt Utilities](https://github.com/Dark-Alex-17/coyote/wiki/Bash-Prompt-Helpers) * [First-Class MCP Server Support](https://github.com/Dark-Alex-17/coyote/wiki/MCP-Servers): Easily connect and interact with MCP servers for advanced functionality. -* [Macros](https://github.com/Dark-Alex-17/coyote/wiki/Macros): Automate repetitive tasks and workflows with Coyote "scripts" (macros). +* [Macros](https://github.com/Dark-Alex-17/coyote/wiki/Macros): Automate repetitive tasks and workflows with Coyote "scripts" (macros). Macros are Coyote's custom commands: invoke any macro directly by name (e.g. `.review main`), with tab-completion, right alongside the built-in REPL commands. + * Give a macro a `description` (shown in `.list macros` and completions) and set `isolated: false` to run its steps on the live session, exactly as if you typed them. Note that non-isolated steps are recorded in the session, and mutating steps (`.role`, `.model`, ...) persist after the macro ends — by design. Steps are fail-fast: an error aborts the remaining steps, but completed steps' effects remain. A non-isolated macro step cannot invoke another macro, and a `.exit` step never exits the REPL. + * Commit project-specific macros to `.coyote/macros/` in your repo — they shadow same-named global macros (opt out with `--no-workspace-macros`). + * Scope which macros are invocable with `enabled_macros` in the global config, a role, an agent, or a session (most specific wins; an empty list disables all macros), and toggle at runtime with `.macro enable|disable `. * [RAG](https://github.com/Dark-Alex-17/coyote/wiki/RAG): Retrieval-Augmented Generation for enhanced information retrieval and generation. * [Sessions](https://github.com/Dark-Alex-17/coyote/wiki/Sessions): Manage and persist conversational contexts and settings across multiple interactions. * [Memory](https://github.com/Dark-Alex-17/coyote/wiki/Memory): Persistent file-based memory that survives across sessions. Bootstrap with `coyote --init-memory [global|workspace]`. diff --git a/assets/macros/generate-commit-message.yaml b/assets/macros/generate-commit-message.yaml index 364cc26..3cc887c 100644 --- a/assets/macros/generate-commit-message.yaml +++ b/assets/macros/generate-commit-message.yaml @@ -1,2 +1,3 @@ +description: Generate a git commit message from the current diff steps: - - .file `git diff` -- generate a git commit message \ No newline at end of file + - .file `git diff` -- generate a git commit message diff --git a/config.agent.example.yaml b/config.agent.example.yaml index d7c6d5a..c4562df 100644 --- a/config.agent.example.yaml +++ b/config.agent.example.yaml @@ -60,6 +60,8 @@ enabled_skills: # Optional list of skills available when this a inject_skill_instructions: true # Inject a short hint pointing the model at `skill__list` when skills are enabled # (default: true). Suppressed automatically when no skills are available. skill_instructions: null # Custom text for the skill hint (optional; uses built-in default if null) +enabled_macros: # Optional list of macros invocable when this agent is active in the REPL. + - generate-commit-message # An empty list disables all macros. Omit to inherit the role/global default. memory: null # Per-agent memory override (default: inherit). Set to `false` to disable memory # for this agent regardless of workspace/global presence. See the Memory wiki page. diff --git a/config.example.yaml b/config.example.yaml index c93a509..247333a 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -169,6 +169,21 @@ inject_skill_instructions: true # Inject a short hint pointing the model at `s # effective enabled skill set is non-empty (default: true). skill_instructions: null # Custom text used for the skill hint when injected. If null, uses built-in default. +# ---- Macros ---- +# Macros are Coyote's custom commands: named sequences of REPL commands and prompts, invoked directly by name +# (a macro file named `review.yaml` runs as `.review [args]`; built-in commands always win a name collision). +# Workspace-local macros in `.coyote/macros/` shadow same-named global macros (skip them with --no-workspace-macros). +# See the [Macros documentation](https://github.com/Dark-Alex-17/coyote/wiki/Macros) for more details. +enabled_macros: null # Which macros are invocable by default (no role/agent/session active). null = all visible. + # An empty list means NO macros are invocable. Accepts either a YAML list or a + # comma-separated string. Roles, agents, and sessions may define their own + # `enabled_macros`; the most specific active one wins (session > agent > role > global). + # Example (list form): + # enabled_macros: + # - generate-commit-message + # Example (comma-separated form): + # enabled_macros: generate-commit-message,review + # ---- Auto-Continue (Todo System) ---- # The auto-continue system provides built-in task tracking for improved reliability. # When enabled, the model can create todo lists and the system will automatically diff --git a/config.macro.example.yaml b/config.macro.example.yaml index ba83d07..341e8b4 100644 --- a/config.macro.example.yaml +++ b/config.macro.example.yaml @@ -1,3 +1,13 @@ +description: Demonstrates every macro field # Optional; shown in `.list macros` and in `.` tab-completion. +isolated: true # Optional; 'true' by default. When true, steps run in a forked, + # throwaway context: the exchange and any `.role`/`.model` switches + # vanish when the macro ends. When false, steps run on the LIVE + # session exactly as if you typed them: prompts are recorded, and + # mutating steps (e.g. `.role`, `.model`) PERSIST after the macro + # finishes -- by design. Steps are fail-fast in both modes: an error + # aborts the remaining steps, but completed steps' effects remain. + # A non-isolated macro step cannot invoke another macro, and a + # `.exit` step never exits the REPL. variables: # A list of positional variables that the macro uses - name: positional_1 # The name of the positional variable. default: null # Since no default value is provided, this argument is required; 'null' by default diff --git a/config.role.example.md b/config.role.example.md index ff66eaa..3fabd63 100644 --- a/config.role.example.md +++ b/config.role.example.md @@ -24,6 +24,9 @@ enabled_skills: # Skills available when this role is activ inject_skill_instructions: true # Inject a short hint pointing the model at `skill__list` when skills are enabled # (default: true). Suppressed automatically when no skills are available. skill_instructions: null # Custom text for the skill hint (optional; uses built-in default if null) +enabled_macros: # Macros invocable when this role is active. Accepts a YAML list (preferred) + - generate-commit-message # or a comma-separated string (e.g. `enabled_macros: generate-commit-message,review`). + # An empty list disables all macros. Omit to inherit the global default. memory: null # Per-role memory override (default: inherit). Set to `false` to disable memory # when this role is active. See the Memory wiki page. diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 9f86023..9c4394d 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -96,6 +96,9 @@ pub struct Cli { /// Disable loading workspace MCP servers from .coyote/mcp.json, .coyote/.mcp.json, or .mcp.json #[arg(long)] pub no_workspace_mcp: bool, + /// Disable loading workspace macros from .coyote/macros + #[arg(long)] + pub no_workspace_macros: bool, /// Disable memory for this invocation #[arg(long)] pub no_memory: bool, diff --git a/src/config/agent.rs b/src/config/agent.rs index 95467ce..dce814c 100644 --- a/src/config/agent.rs +++ b/src/config/agent.rs @@ -400,6 +400,10 @@ impl Agent { self.config.enabled_skills.as_deref() } + pub fn enabled_macros(&self) -> Option<&[String]> { + self.config.enabled_macros.as_deref() + } + pub fn memory(&self) -> Option { self.config.memory } @@ -744,6 +748,8 @@ pub struct AgentConfig { #[serde(skip_serializing_if = "Option::is_none")] pub enabled_skills: Option>, #[serde(skip_serializing_if = "Option::is_none")] + pub enabled_macros: Option>, + #[serde(skip_serializing_if = "Option::is_none")] pub continuation_prompt: Option, #[serde(default)] pub instructions: String, @@ -1225,6 +1231,30 @@ variables: assert!(config.top_p.is_none()); } + #[test] + fn agent_config_enabled_macros_absent_is_none() { + let yaml = "name: minimal\ninstructions: hi\n"; + let config: AgentConfig = serde_yaml::from_str(yaml).unwrap(); + + assert_eq!(config.enabled_macros, None); + } + + #[test] + fn agent_config_enabled_macros_empty_list_is_some_empty() { + let yaml = "name: minimal\ninstructions: hi\nenabled_macros: []\n"; + let config: AgentConfig = serde_yaml::from_str(yaml).unwrap(); + + assert_eq!(config.enabled_macros, Some(vec![])); + } + + #[test] + fn agent_config_enabled_macros_list() { + let yaml = "name: minimal\ninstructions: hi\nenabled_macros:\n - a\n"; + let config: AgentConfig = serde_yaml::from_str(yaml).unwrap(); + + assert_eq!(config.enabled_macros, Some(vec!["a".to_string()])); + } + #[test] fn agent_config_with_model() { let yaml = diff --git a/src/config/app_config.rs b/src/config/app_config.rs index be85041..5dd6b2d 100644 --- a/src/config/app_config.rs +++ b/src/config/app_config.rs @@ -43,6 +43,8 @@ pub struct AppConfig { #[serde(default, deserialize_with = "super::deserialize_csv_or_vec")] pub enabled_skills: Option>, pub visible_skills: Option>, + #[serde(default, deserialize_with = "super::deserialize_csv_or_vec")] + pub enabled_macros: Option>, pub mcp_server_support: bool, pub mapping_mcp_servers: IndexMap, @@ -96,6 +98,7 @@ pub struct AppConfig { pub user_agent: Option, pub save_shell_history: bool, pub no_workspace_mcp: bool, + pub no_workspace_macros: bool, pub sync_models_url: Option, pub clients: Vec, @@ -127,6 +130,7 @@ impl Default for AppConfig { skills_enabled: true, enabled_skills: None, visible_skills: None, + enabled_macros: None, mcp_server_support: true, mapping_mcp_servers: Default::default(), @@ -178,6 +182,7 @@ impl Default for AppConfig { user_agent: None, save_shell_history: true, no_workspace_mcp: false, + no_workspace_macros: false, sync_models_url: None, clients: vec![], @@ -211,6 +216,7 @@ impl AppConfig { skills_enabled: config.skills_enabled, enabled_skills: config.enabled_skills, visible_skills: config.visible_skills, + enabled_macros: config.enabled_macros, mcp_server_support: config.mcp_server_support, mapping_mcp_servers: config.mapping_mcp_servers, @@ -262,6 +268,7 @@ impl AppConfig { user_agent: config.user_agent, save_shell_history: config.save_shell_history, no_workspace_mcp: false, + no_workspace_macros: false, sync_models_url: config.sync_models_url, clients: config.clients, @@ -533,6 +540,10 @@ impl AppConfig { self.enabled_skills = v.map(|raw| super::csv_to_vec(&raw)); } + if let Some(v) = super::read_env_value::(&get_env_name("enabled_macros")) { + self.enabled_macros = v.map(|raw| super::csv_to_vec(&raw)); + } + if let Some(Some(v)) = super::read_env_bool(&get_env_name("mcp_server_support")) { self.mcp_server_support = v; } @@ -769,6 +780,70 @@ mod tests { ); } + #[test] + #[serial_test::serial] + fn from_config_copies_enabled_macros() { + let cfg = Config { + model_id: "provider:test".to_string(), + enabled_macros: Some(vec!["a".to_string()]), + ..Config::default() + }; + + let app = AppConfig::from_config(cfg).unwrap(); + + assert_eq!(app.enabled_macros, Some(vec!["a".to_string()])); + } + + #[test] + #[serial_test::serial] + fn from_config_preserves_explicit_empty_enabled_macros() { + let cfg = Config { + model_id: "provider:test".to_string(), + enabled_macros: Some(vec![]), + ..Config::default() + }; + + let app = AppConfig::from_config(cfg).unwrap(); + + assert_eq!(app.enabled_macros, Some(vec![])); + } + + #[test] + #[serial_test::serial] + fn load_envs_overrides_enabled_macros() { + let env_name = get_env_name("enabled_macros"); + let prev = std::env::var_os(&env_name); + + let mut app = AppConfig::default(); + + unsafe { std::env::set_var(&env_name, "a,b") }; + app.load_envs(); + assert_eq!( + app.enabled_macros, + Some(vec!["a".to_string(), "b".to_string()]) + ); + + unsafe { std::env::set_var(&env_name, "") }; + app.load_envs(); + assert_eq!(app.enabled_macros, Some(vec![])); + + unsafe { std::env::set_var(&env_name, "null") }; + app.load_envs(); + assert_eq!(app.enabled_macros, None); + + unsafe { std::env::remove_var(&env_name) }; + app.enabled_macros = Some(vec!["keep".to_string()]); + app.load_envs(); + assert_eq!(app.enabled_macros, Some(vec!["keep".to_string()])); + + unsafe { + match prev { + Some(v) => std::env::set_var(&env_name, v), + None => std::env::remove_var(&env_name), + } + } + } + #[test] fn editor_returns_configured_value() { let configured = cached_editor() diff --git a/src/config/macro_policy.rs b/src/config/macro_policy.rs new file mode 100644 index 0000000..4c08ed7 --- /dev/null +++ b/src/config/macro_policy.rs @@ -0,0 +1,983 @@ +use super::agent::Agent; +use super::app_config::AppConfig; +use super::macros::Macro; +use super::paths; +use super::role::Role; +use super::session::Session; + +use log::warn; +use std::collections::HashSet; +use std::fmt; +use std::fs::{read_dir, read_to_string}; +use std::path::PathBuf; + +pub const RESERVED_MACRO_NAMES: [&str; 2] = ["enable", "disable"]; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum MacroSource { + Workspace, + Global, +} + +impl fmt::Display for MacroSource { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + MacroSource::Workspace => write!(f, "workspace"), + MacroSource::Global => write!(f, "global"), + } + } +} + +/// The configuration level whose `enabled_macros` allowlist won the +/// first-`Some`-wins precedence chain (session > agent > role > global). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum MacroAllowlistLevel { + Session, + Agent, + Role, + Global, +} + +impl fmt::Display for MacroAllowlistLevel { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + MacroAllowlistLevel::Session => write!(f, "session"), + MacroAllowlistLevel::Agent => write!(f, "agent"), + MacroAllowlistLevel::Role => write!(f, "role"), + MacroAllowlistLevel::Global => write!(f, "global"), + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum MacroState { + Enabled, + DisabledRuntime, + Locked { level: MacroAllowlistLevel }, + Missing, + ShadowedBuiltin, + Invalid { reason: String }, +} + +impl MacroState { + pub fn is_invocable(&self) -> bool { + matches!(self, MacroState::Enabled | MacroState::ShadowedBuiltin) + } +} + +#[derive(Debug, Clone)] +pub struct DiscoveredMacro { + pub name: String, + pub source: MacroSource, + pub definition: Result, + pub shadowed_by_workspace: bool, +} + +#[derive(Debug, Clone)] +pub struct ResolvedMacro { + pub name: String, + pub source: Option, + pub description: Option, + pub isolated: Option, + pub shadowed_by_workspace: bool, + pub state: MacroState, +} + +#[derive(Debug)] +pub struct MacroPolicy { + pub macros: Vec, +} + +impl MacroPolicy { + pub fn effective( + global: &AppConfig, + role: Option<&Role>, + agent: Option<&Agent>, + session: Option<&Session>, + builtin_commands: &[&str], + no_workspace_macros: bool, + ) -> Self { + Self::effective_with( + discover_macros(no_workspace_macros), + session.and_then(|s| s.enabled_macros()), + agent.and_then(|a| a.enabled_macros()), + role.and_then(|r| r.enabled_macros()), + global.enabled_macros.as_deref(), + builtin_commands, + ) + } + + fn effective_with( + discovered: Vec, + session_list: Option<&[String]>, + agent_list: Option<&[String]>, + role_list: Option<&[String]>, + global_list: Option<&[String]>, + builtin_commands: &[&str], + ) -> Self { + let allowlist = session_list + .map(|list| (MacroAllowlistLevel::Session, list)) + .or_else(|| agent_list.map(|list| (MacroAllowlistLevel::Agent, list))) + .or_else(|| role_list.map(|list| (MacroAllowlistLevel::Role, list))) + .or_else(|| global_list.map(|list| (MacroAllowlistLevel::Global, list))); + + let mut macros: Vec = discovered + .into_iter() + .map(|discovered_macro| { + let state = resolve_state(&discovered_macro, allowlist, builtin_commands); + let (description, isolated) = match &discovered_macro.definition { + Ok(value) => (value.description.clone(), Some(value.isolated)), + Err(_) => (None, None), + }; + ResolvedMacro { + name: discovered_macro.name, + source: Some(discovered_macro.source), + description, + isolated, + shadowed_by_workspace: discovered_macro.shadowed_by_workspace, + state, + } + }) + .collect(); + + if let Some((_, list)) = allowlist { + let known: HashSet<&str> = macros.iter().map(|m| m.name.as_str()).collect(); + let mut missing: Vec = vec![]; + for name in list { + if !known.contains(name.as_str()) && !missing.iter().any(|m| &m.name == name) { + warn!("enabled_macros references macro '{name}' which is not installed"); + missing.push(ResolvedMacro { + name: name.clone(), + source: None, + description: None, + isolated: None, + shadowed_by_workspace: false, + state: MacroState::Missing, + }); + } + } + + macros.extend(missing); + } + + macros.sort_by(|a, b| { + a.name + .cmp(&b.name) + .then_with(|| source_rank(a.source).cmp(&source_rank(b.source))) + }); + + Self { macros } + } + + pub fn find(&self, name: &str) -> Option<&ResolvedMacro> { + self.macros + .iter() + .find(|m| m.name == name && m.source.is_some() && !m.shadowed_by_workspace) + } +} + +fn source_rank(source: Option) -> u8 { + match source { + Some(MacroSource::Workspace) => 0, + Some(MacroSource::Global) => 1, + None => 2, + } +} + +fn resolve_state( + discovered: &DiscoveredMacro, + allowlist: Option<(MacroAllowlistLevel, &[String])>, + builtin_commands: &[&str], +) -> MacroState { + if RESERVED_MACRO_NAMES.contains(&discovered.name.as_str()) { + warn!( + "Ignoring macro '{}': the name is reserved for '.macro {}'", + discovered.name, discovered.name + ); + + return MacroState::Invalid { + reason: format!("'{}' is a reserved macro name", discovered.name), + }; + } + + if let Err(reason) = &discovered.definition { + return MacroState::Invalid { + reason: reason.clone(), + }; + } + + if let Some((level, list)) = allowlist + && !list.iter().any(|name| name == &discovered.name) + { + return match level { + MacroAllowlistLevel::Global => MacroState::DisabledRuntime, + level => MacroState::Locked { level }, + }; + } + + if builtin_commands.contains(&discovered.name.as_str()) { + return MacroState::ShadowedBuiltin; + } + + MacroState::Enabled +} + +pub fn discover_macros(no_workspace_macros: bool) -> Vec { + let mut dirs = vec![]; + if !no_workspace_macros { + dirs.push((MacroSource::Workspace, paths::workspace_macros_dir())); + } + + dirs.push((MacroSource::Global, paths::macros_dir())); + discover_macros_in(&dirs) +} + +fn discover_macros_in(dirs: &[(MacroSource, PathBuf)]) -> Vec { + let mut seen: HashSet = HashSet::new(); + let mut output = vec![]; + + for (source, dir) in dirs { + let Ok(rd) = read_dir(dir) else { + continue; + }; + let mut entries: Vec<_> = rd.flatten().collect(); + entries.sort_by_key(|entry| entry.file_name()); + + for entry in entries { + let is_file = entry + .file_type() + .map(|file_type| file_type.is_file()) + .unwrap_or(false); + + if !is_file { + continue; + } + + let Some(name) = entry + .file_name() + .to_str() + .and_then(|v| v.strip_suffix(".yaml")) + .map(str::to_string) + else { + continue; + }; + + if name.is_empty() { + continue; + } + + let definition = read_to_string(entry.path()) + .map_err(|err| err.to_string()) + .and_then(|content| { + serde_yaml::from_str::(&content).map_err(|err| err.to_string()) + }); + let shadowed_by_workspace = !seen.insert(name.clone()); + output.push(DiscoveredMacro { + name, + source: *source, + definition, + shadowed_by_workspace, + }); + } + } + + output +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::utils::get_env_name; + use serial_test::serial; + use std::path::Path; + use std::sync::atomic::{AtomicUsize, Ordering}; + use std::{env, fs, process}; + + fn valid_macro() -> Macro { + Macro { + description: Some("a test macro".to_string()), + isolated: true, + variables: vec![], + steps: vec![".help".to_string()], + } + } + + fn disc(name: &str, source: MacroSource) -> DiscoveredMacro { + DiscoveredMacro { + name: name.to_string(), + source, + definition: Ok(valid_macro()), + shadowed_by_workspace: false, + } + } + + fn disc_invalid(name: &str, reason: &str) -> DiscoveredMacro { + DiscoveredMacro { + name: name.to_string(), + source: MacroSource::Global, + definition: Err(reason.to_string()), + shadowed_by_workspace: false, + } + } + + fn globals(names: &[&str]) -> Vec { + names.iter().map(|n| disc(n, MacroSource::Global)).collect() + } + + fn list(names: &[&str]) -> Vec { + names.iter().map(|s| s.to_string()).collect() + } + + fn resolve( + discovered: Vec, + session: Option<&[String]>, + agent: Option<&[String]>, + role: Option<&[String]>, + global: Option<&[String]>, + ) -> MacroPolicy { + MacroPolicy::effective_with(discovered, session, agent, role, global, &[]) + } + + fn state_of<'a>(policy: &'a MacroPolicy, name: &str) -> &'a MacroState { + &policy + .macros + .iter() + .find(|m| m.name == name) + .unwrap_or_else(|| panic!("no row for macro '{name}'")) + .state + } + + #[test] + fn all_none_enables_everything() { + let policy = resolve(globals(&["a", "b"]), None, None, None, None); + + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); + assert_eq!(state_of(&policy, "b"), &MacroState::Enabled); + } + + #[test] + fn global_empty_list_disables_all_as_runtime() { + let l = list(&[]); + let policy = resolve(globals(&["a", "b"]), None, None, None, Some(&l)); + + assert_eq!(state_of(&policy, "a"), &MacroState::DisabledRuntime); + assert_eq!(state_of(&policy, "b"), &MacroState::DisabledRuntime); + } + + #[test] + fn global_populated_partitions_enabled_and_disabled_runtime() { + let l = list(&["a"]); + let policy = resolve(globals(&["a", "b"]), None, None, None, Some(&l)); + + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); + assert_eq!(state_of(&policy, "b"), &MacroState::DisabledRuntime); + } + + #[test] + fn role_populated_locks_excluded_at_role_level() { + let l = list(&["a"]); + + let policy = resolve(globals(&["a", "b"]), None, None, Some(&l), None); + + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); + assert_eq!( + state_of(&policy, "b"), + &MacroState::Locked { + level: MacroAllowlistLevel::Role + } + ); + } + + #[test] + fn agent_populated_locks_excluded_at_agent_level() { + let l = list(&["a"]); + + let policy = resolve(globals(&["a", "b"]), None, Some(&l), None, None); + + assert_eq!( + state_of(&policy, "b"), + &MacroState::Locked { + level: MacroAllowlistLevel::Agent + } + ); + } + + #[test] + fn session_populated_locks_excluded_at_session_level() { + let l = list(&["a"]); + + let policy = resolve(globals(&["a", "b"]), Some(&l), None, None, None); + + assert_eq!( + state_of(&policy, "b"), + &MacroState::Locked { + level: MacroAllowlistLevel::Session + } + ); + } + + #[test] + fn role_empty_list_locks_everything_at_role_level() { + let l = list(&[]); + + let policy = resolve(globals(&["a"]), None, None, Some(&l), None); + + assert_eq!( + state_of(&policy, "a"), + &MacroState::Locked { + level: MacroAllowlistLevel::Role + } + ); + } + + #[test] + fn agent_empty_list_locks_everything_at_agent_level() { + let l = list(&[]); + + let policy = resolve(globals(&["a"]), None, Some(&l), None, None); + + assert_eq!( + state_of(&policy, "a"), + &MacroState::Locked { + level: MacroAllowlistLevel::Agent + } + ); + } + + #[test] + fn session_empty_list_locks_everything_at_session_level() { + let l = list(&[]); + + let policy = resolve(globals(&["a"]), Some(&l), None, None, None); + + assert_eq!( + state_of(&policy, "a"), + &MacroState::Locked { + level: MacroAllowlistLevel::Session + } + ); + } + + #[test] + fn session_wins_over_agent() { + let session = list(&["a"]); + let agent = list(&["b"]); + + let policy = resolve( + globals(&["a", "b"]), + Some(&session), + Some(&agent), + None, + None, + ); + + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); + assert_eq!( + state_of(&policy, "b"), + &MacroState::Locked { + level: MacroAllowlistLevel::Session + } + ); + } + + #[test] + fn agent_wins_over_role() { + let agent = list(&["a"]); + let role = list(&["b"]); + + let policy = resolve(globals(&["a", "b"]), None, Some(&agent), Some(&role), None); + + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); + assert_eq!( + state_of(&policy, "b"), + &MacroState::Locked { + level: MacroAllowlistLevel::Agent + } + ); + } + + #[test] + fn role_wins_over_global() { + let role = list(&["a"]); + let global = list(&["b"]); + + let policy = resolve(globals(&["a", "b"]), None, None, Some(&role), Some(&global)); + + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); + assert_eq!( + state_of(&policy, "b"), + &MacroState::Locked { + level: MacroAllowlistLevel::Role + } + ); + } + + #[test] + fn empty_list_at_session_beats_populated_global() { + let session = list(&[]); + let global = list(&["a"]); + + let policy = resolve(globals(&["a"]), Some(&session), None, None, Some(&global)); + + assert_eq!( + state_of(&policy, "a"), + &MacroState::Locked { + level: MacroAllowlistLevel::Session + } + ); + } + + #[test] + fn unknown_allowlist_name_yields_missing_row_without_error() { + let l = list(&["a", "ghost"]); + + let policy = resolve(globals(&["a"]), None, None, None, Some(&l)); + + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); + assert_eq!(state_of(&policy, "ghost"), &MacroState::Missing); + let ghost = policy.macros.iter().find(|m| m.name == "ghost").unwrap(); + assert_eq!(ghost.source, None); + } + + #[test] + fn missing_row_deduplicated_for_repeated_allowlist_names() { + let l = list(&["ghost", "ghost"]); + + let policy = resolve(vec![], None, None, None, Some(&l)); + + assert_eq!(policy.macros.len(), 1); + assert_eq!(state_of(&policy, "ghost"), &MacroState::Missing); + } + + #[test] + fn no_missing_rows_without_an_allowlist() { + let policy = resolve(globals(&["a"]), None, None, None, None); + + assert_eq!(policy.macros.len(), 1); + } + + #[test] + fn builtin_name_collision_is_shadowed() { + let policy = + MacroPolicy::effective_with(globals(&["help", "a"]), None, None, None, None, &["help"]); + + assert_eq!(state_of(&policy, "help"), &MacroState::ShadowedBuiltin); + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); + } + + #[test] + fn locked_wins_over_shadowed_builtin() { + let l = list(&["a"]); + + let policy = MacroPolicy::effective_with( + globals(&["help", "a"]), + Some(&l), + None, + None, + None, + &["help"], + ); + + assert_eq!( + state_of(&policy, "help"), + &MacroState::Locked { + level: MacroAllowlistLevel::Session + } + ); + } + + #[test] + fn allowlisted_builtin_collision_stays_shadowed() { + let l = list(&["help"]); + + let policy = + MacroPolicy::effective_with(globals(&["help"]), None, None, None, Some(&l), &["help"]); + + assert_eq!(state_of(&policy, "help"), &MacroState::ShadowedBuiltin); + } + + #[test] + fn reserved_names_are_invalid() { + let policy = resolve(globals(&["enable", "disable"]), None, None, None, None); + for name in RESERVED_MACRO_NAMES { + assert_eq!( + state_of(&policy, name), + &MacroState::Invalid { + reason: format!("'{name}' is a reserved macro name") + } + ); + } + } + + #[test] + fn reserved_name_invalid_even_when_allowlisted() { + let l = list(&["enable"]); + + let policy = resolve(globals(&["enable"]), Some(&l), None, None, None); + + assert_eq!( + state_of(&policy, "enable"), + &MacroState::Invalid { + reason: "'enable' is a reserved macro name".to_string() + } + ); + } + + #[test] + fn reserved_name_invalid_wins_over_builtin_collision() { + let policy = + MacroPolicy::effective_with(globals(&["enable"]), None, None, None, None, &["enable"]); + + assert_eq!( + state_of(&policy, "enable"), + &MacroState::Invalid { + reason: "'enable' is a reserved macro name".to_string() + } + ); + } + + #[test] + fn parse_failure_is_invalid() { + let policy = resolve(vec![disc_invalid("bad", "boom")], None, None, None, None); + + assert_eq!( + state_of(&policy, "bad"), + &MacroState::Invalid { + reason: "boom".to_string() + } + ); + let bad = policy.macros.iter().find(|m| m.name == "bad").unwrap(); + assert_eq!(bad.description, None); + assert_eq!(bad.isolated, None); + } + + #[test] + fn invalid_wins_over_allowlist_exclusion() { + let l = list(&["other"]); + + let policy = resolve( + vec![disc_invalid("bad", "boom")], + Some(&l), + None, + None, + None, + ); + + assert_eq!( + state_of(&policy, "bad"), + &MacroState::Invalid { + reason: "boom".to_string() + } + ); + } + + #[test] + fn workspace_shadowing_keeps_both_rows_and_find_returns_workspace() { + let discovered = vec![ + disc("a", MacroSource::Workspace), + DiscoveredMacro { + shadowed_by_workspace: true, + ..disc("a", MacroSource::Global) + }, + ]; + + let policy = resolve(discovered, None, None, None, None); + + assert_eq!(policy.macros.len(), 2); + assert_eq!(policy.macros[0].source, Some(MacroSource::Workspace)); + assert!(!policy.macros[0].shadowed_by_workspace); + assert_eq!(policy.macros[1].source, Some(MacroSource::Global)); + assert!(policy.macros[1].shadowed_by_workspace); + let found = policy.find("a").unwrap(); + assert_eq!(found.source, Some(MacroSource::Workspace)); + } + + #[test] + fn find_skips_missing_rows() { + let l = list(&["ghost"]); + + let policy = resolve(vec![], None, None, None, Some(&l)); + + assert!(policy.find("ghost").is_none()); + } + + #[test] + fn rows_are_sorted_by_name() { + let policy = resolve(globals(&["c", "a", "b"]), None, None, None, None); + + let names: Vec<&str> = policy.macros.iter().map(|m| m.name.as_str()).collect(); + assert_eq!(names, vec!["a", "b", "c"]); + } + + #[test] + fn resolved_rows_carry_description_and_isolated() { + let policy = resolve(globals(&["a"]), None, None, None, None); + + let row = policy.macros.first().unwrap(); + assert_eq!(row.description.as_deref(), Some("a test macro")); + assert_eq!(row.isolated, Some(true)); + } + + #[test] + fn is_invocable_only_for_enabled_and_shadowed() { + assert!(MacroState::Enabled.is_invocable()); + assert!(MacroState::ShadowedBuiltin.is_invocable()); + assert!(!MacroState::DisabledRuntime.is_invocable()); + assert!( + !MacroState::Locked { + level: MacroAllowlistLevel::Role + } + .is_invocable() + ); + assert!(!MacroState::Missing.is_invocable()); + assert!( + !MacroState::Invalid { + reason: "x".to_string() + } + .is_invocable() + ); + } + + #[test] + fn level_and_source_display() { + assert_eq!(MacroAllowlistLevel::Session.to_string(), "session"); + assert_eq!(MacroAllowlistLevel::Agent.to_string(), "agent"); + assert_eq!(MacroAllowlistLevel::Role.to_string(), "role"); + assert_eq!(MacroAllowlistLevel::Global.to_string(), "global"); + assert_eq!(MacroSource::Workspace.to_string(), "workspace"); + assert_eq!(MacroSource::Global.to_string(), "global"); + } + + fn with_macro_dirs(f: F) { + static COUNTER: AtomicUsize = AtomicUsize::new(0); + let unique = format!( + "{}-{}", + process::id(), + COUNTER.fetch_add(1, Ordering::Relaxed) + ); + let root = env::temp_dir().join(format!("coyote-macro-policy-test-{unique}")); + let workspace = root.join("workspace-macros"); + let global = root.join("global-macros"); + fs::create_dir_all(&workspace).unwrap(); + fs::create_dir_all(&global).unwrap(); + f(&workspace, &global); + let _ = fs::remove_dir_all(&root); + } + + fn write_macro(dir: &Path, name: &str, content: &str) { + fs::write(dir.join(format!("{name}.yaml")), content).unwrap(); + } + + const VALID_YAML: &str = "steps:\n - \".help\"\n"; + + #[test] + fn discovery_scans_workspace_then_global_with_shadowing() { + with_macro_dirs(|workspace, global| { + write_macro(workspace, "both", VALID_YAML); + write_macro(workspace, "ws-only", VALID_YAML); + write_macro(global, "both", VALID_YAML); + write_macro(global, "global-only", VALID_YAML); + + let discovered = discover_macros_in(&[ + (MacroSource::Workspace, workspace.to_path_buf()), + (MacroSource::Global, global.to_path_buf()), + ]); + + assert_eq!(discovered.len(), 4); + let both_ws = discovered + .iter() + .find(|d| d.name == "both" && d.source == MacroSource::Workspace) + .unwrap(); + assert!(!both_ws.shadowed_by_workspace); + let both_global = discovered + .iter() + .find(|d| d.name == "both" && d.source == MacroSource::Global) + .unwrap(); + assert!(both_global.shadowed_by_workspace); + let global_only = discovered.iter().find(|d| d.name == "global-only").unwrap(); + assert!(!global_only.shadowed_by_workspace); + }); + } + + #[test] + fn discovery_ignores_non_yaml_files_and_directories() { + with_macro_dirs(|_, global| { + write_macro(global, "good", VALID_YAML); + fs::write(global.join("notes.txt"), "not a macro").unwrap(); + fs::write(global.join(".yaml"), VALID_YAML).unwrap(); + fs::create_dir_all(global.join("subdir.yaml")).unwrap(); + + let discovered = discover_macros_in(&[(MacroSource::Global, global.to_path_buf())]); + + assert_eq!(discovered.len(), 1); + assert_eq!(discovered[0].name, "good"); + }); + } + + #[test] + fn discovery_records_parse_failures() { + with_macro_dirs(|_, global| { + write_macro(global, "broken", "steps: {not valid"); + + let discovered = discover_macros_in(&[(MacroSource::Global, global.to_path_buf())]); + + assert_eq!(discovered.len(), 1); + assert!(discovered[0].definition.is_err()); + }); + } + + #[test] + fn discovery_of_nonexistent_dirs_is_empty() { + let discovered = discover_macros_in(&[( + MacroSource::Global, + PathBuf::from("/nonexistent/coyote-macro-policy-test"), + )]); + assert!(discovered.is_empty()); + } + + fn with_macro_dir_envs(workspace: &Path, global: &Path, f: F) { + let ws_env = get_env_name("workspace_config_dir"); + let global_env = get_env_name("macros_dir"); + let prev_ws = env::var_os(&ws_env); + let prev_global = env::var_os(&global_env); + unsafe { + env::set_var(&ws_env, workspace); + env::set_var(&global_env, global); + } + f(); + unsafe { + match prev_ws { + Some(v) => env::set_var(&ws_env, v), + None => env::remove_var(&ws_env), + } + match prev_global { + Some(v) => env::set_var(&global_env, v), + None => env::remove_var(&global_env), + } + } + } + + #[test] + #[serial] + fn discover_macros_honors_no_workspace_macros() { + with_macro_dirs(|workspace_root, global| { + let macros_subdir = workspace_root.join("macros"); + fs::create_dir_all(¯os_subdir).unwrap(); + write_macro(¯os_subdir, "ws-macro", VALID_YAML); + write_macro(global, "global-macro", VALID_YAML); + + with_macro_dir_envs(workspace_root, global, || { + let with_workspace = discover_macros(false); + let names: Vec<&str> = with_workspace.iter().map(|d| d.name.as_str()).collect(); + assert!(names.contains(&"ws-macro")); + assert!(names.contains(&"global-macro")); + + let without_workspace = discover_macros(true); + let names: Vec<&str> = without_workspace.iter().map(|d| d.name.as_str()).collect(); + assert!(!names.contains(&"ws-macro")); + assert!(names.contains(&"global-macro")); + }); + }); + } + + #[test] + #[serial] + fn effective_honors_no_workspace_macros() { + with_macro_dirs(|workspace_root, global| { + let macros_subdir = workspace_root.join("macros"); + fs::create_dir_all(¯os_subdir).unwrap(); + write_macro(¯os_subdir, "shared", VALID_YAML); + write_macro(¯os_subdir, "ws-only", VALID_YAML); + write_macro(global, "shared", VALID_YAML); + write_macro(global, "global-only", VALID_YAML); + + with_macro_dir_envs(workspace_root, global, || { + let config = AppConfig::default(); + + let policy = MacroPolicy::effective(&config, None, None, None, &[], false); + assert_eq!( + policy.find("shared").unwrap().source, + Some(MacroSource::Workspace) + ); + assert!(policy.find("ws-only").is_some()); + assert!(policy.find("global-only").is_some()); + + let policy = MacroPolicy::effective(&config, None, None, None, &[], true); + assert_eq!( + policy.find("shared").unwrap().source, + Some(MacroSource::Global) + ); + assert!(policy.find("ws-only").is_none()); + assert!(policy.find("global-only").is_some()); + }); + }); + } + + #[test] + #[serial] + fn effective_resolves_role_session_and_global_levels() { + with_macro_dirs(|workspace_root, global_dir| { + write_macro(global_dir, "a", VALID_YAML); + write_macro(global_dir, "b", VALID_YAML); + + with_macro_dir_envs(workspace_root, global_dir, || { + let global = AppConfig { + enabled_macros: Some(vec!["a".to_string()]), + ..AppConfig::default() + }; + + let policy = MacroPolicy::effective(&global, None, None, None, &[], false); + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); + assert_eq!(state_of(&policy, "b"), &MacroState::DisabledRuntime); + + let role = Role::new("test", "---\nenabled_macros: b\n---\nbody"); + let policy = MacroPolicy::effective(&global, Some(&role), None, None, &[], false); + assert_eq!( + state_of(&policy, "a"), + &MacroState::Locked { + level: MacroAllowlistLevel::Role + } + ); + assert_eq!(state_of(&policy, "b"), &MacroState::Enabled); + + let session: Session = serde_yaml::from_str( + "model: provider:test\nenabled_macros: \"a\"\nmessages: []", + ) + .unwrap(); + let policy = + MacroPolicy::effective(&global, Some(&role), None, Some(&session), &[], false); + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); + assert_eq!( + state_of(&policy, "b"), + &MacroState::Locked { + level: MacroAllowlistLevel::Session + } + ); + }); + }); + } + + #[test] + #[serial] + fn effective_pins_empty_string_role_allowlist_as_explicit_zero() { + with_macro_dirs(|workspace_root, global_dir| { + write_macro(global_dir, "a", VALID_YAML); + + with_macro_dir_envs(workspace_root, global_dir, || { + let global = AppConfig { + enabled_macros: Some(vec!["a".to_string()]), + ..AppConfig::default() + }; + let role = Role::new("test", "---\nenabled_macros: \"\"\n---\nbody"); + + let policy = MacroPolicy::effective(&global, Some(&role), None, None, &[], false); + assert_eq!( + state_of(&policy, "a"), + &MacroState::Locked { + level: MacroAllowlistLevel::Role + } + ); + }); + }); + } +} diff --git a/src/config/macros.rs b/src/config/macros.rs index 179e52f..7a045e5 100644 --- a/src/config/macros.rs +++ b/src/config/macros.rs @@ -2,12 +2,13 @@ use crate::config::paths; use crate::config::{RequestContext, RoleLike, ensure_parent_exists}; use crate::repl::{run_repl_command, split_args_text}; use crate::utils::{AbortSignal, multiline_text}; -use anyhow::{Context, Result, anyhow}; +use anyhow::{Context, Result, anyhow, bail}; use indexmap::IndexMap; use rust_embed::Embed; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; use std::fs::{File, read_to_string}; use std::io::Write; +use std::ops::{Deref, DerefMut}; use std::sync::Arc; #[derive(Embed)] @@ -21,7 +22,10 @@ pub async fn macro_execute( args: Option<&str>, abort_signal: AbortSignal, ) -> Result<()> { - let macro_value = Macro::load(name)?; + if ctx.in_non_isolated_macro() { + bail!("nested macros not allowed in non-isolated mode"); + } + let macro_value = Macro::load(name, ctx.app.config.no_workspace_macros)?; let (mut new_args, text) = split_args_text(args.unwrap_or_default(), cfg!(windows)); if !text.is_empty() { new_args.push(text.to_string()); @@ -29,6 +33,18 @@ pub async fn macro_execute( let variables = macro_value .resolve_variables(&new_args) .map_err(|err| anyhow!("{err}. Usage: {}", macro_value.usage(name)))?; + + if !macro_value.isolated { + let mut live = MacroModeGuard::new(ctx); + for step in ¯o_value.steps { + let command = Macro::interpolate_command(step, &variables); + println!(">> {}", multiline_text(&command)); + run_repl_command(&mut live, abort_signal.clone(), &command).await?; + } + + return Ok(()); + } + let role = ctx.extract_role(ctx.app.config.as_ref())?; let mut app_config = (*ctx.app.config).clone(); app_config.temperature = role.temperature(); @@ -69,16 +85,66 @@ pub async fn macro_execute( Ok(()) } -#[derive(Debug, Clone, Deserialize)] +struct MacroModeGuard<'a> { + ctx: &'a mut RequestContext, + prev_flag: bool, + prev_non_isolated: bool, +} + +impl<'a> MacroModeGuard<'a> { + fn new(ctx: &'a mut RequestContext) -> Self { + let prev_flag = ctx.macro_flag; + let prev_non_isolated = ctx.macro_non_isolated; + ctx.macro_flag = true; + ctx.macro_non_isolated = true; + Self { + ctx, + prev_flag, + prev_non_isolated, + } + } +} + +impl Deref for MacroModeGuard<'_> { + type Target = RequestContext; + + fn deref(&self) -> &Self::Target { + self.ctx + } +} + +impl DerefMut for MacroModeGuard<'_> { + fn deref_mut(&mut self) -> &mut Self::Target { + self.ctx + } +} + +impl Drop for MacroModeGuard<'_> { + fn drop(&mut self) { + self.ctx.macro_flag = self.prev_flag; + self.ctx.macro_non_isolated = self.prev_non_isolated; + } +} + +#[derive(Debug, Clone, Deserialize, Serialize)] pub struct Macro { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub description: Option, + #[serde(default = "default_true")] + pub isolated: bool, #[serde(default)] pub variables: Vec, pub steps: Vec, } impl Macro { - pub fn load(name: &str) -> Result { - let path = paths::macro_file(name); + pub fn load(name: &str, no_workspace_macros: bool) -> Result { + let workspace_path = paths::workspace_macros_dir().join(format!("{name}.yaml")); + let path = if !no_workspace_macros && workspace_path.exists() { + workspace_path + } else { + paths::macro_file(name) + }; let err = || format!("Failed to load macro '{name}' at '{}'", path.display()); let content = read_to_string(&path).with_context(err)?; let value: Macro = serde_yaml::from_str(&content).with_context(err)?; @@ -155,24 +221,180 @@ impl Macro { pub fn interpolate_command(command: &str, variables: &IndexMap) -> String { let mut output = command.to_string(); + for (key, value) in variables { output = output.replace(&format!("{{{{{key}}}}}"), value); } + output } } -#[derive(Debug, Clone, Deserialize)] +#[derive(Debug, Clone, Deserialize, Serialize)] pub struct MacroVariable { pub name: String, #[serde(default)] pub rest: bool, + #[serde(skip_serializing_if = "Option::is_none")] pub default: Option, } +fn default_true() -> bool { + true +} + #[cfg(test)] mod tests { use super::*; + use crate::config::{AppState, Session, WorkingMode}; + use crate::utils::{create_abort_signal, get_env_name}; + use serial_test::serial; + use std::fs::{create_dir_all, remove_dir_all, write}; + use std::future::Future; + use std::path::PathBuf; + use std::time::{SystemTime, UNIX_EPOCH}; + use std::{env, str}; + + struct TestConfigDirGuard { + key: String, + previous: Option, + path: 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-macros-tests-{unique}")); + create_dir_all(&path).unwrap(); + unsafe { + env::set_var(&key, &path); + } + Self { + key, + previous, + path, + } + } + } + + 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); + } + } + let _ = remove_dir_all(&self.path); + } + } + + fn test_ctx() -> RequestContext { + RequestContext::new(Arc::new(AppState::test_default()), WorkingMode::Cmd) + } + + fn write_macro_file(name: &str, content: &str) { + let path = paths::macros_dir().join(format!("{name}.yaml")); + ensure_parent_exists(&path).unwrap(); + write(&path, content).unwrap(); + } + + /// Sets up a temp workspace macros dir and a temp global macros dir, each + /// containing a `shared` macro whose `description` names its source, and + /// points the workspace/global dir env overrides at them for `f`. + fn with_macro_load_envs(f: F) { + let unique = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos(); + let root = env::temp_dir().join(format!("coyote-macro-load-tests-{unique}")); + let workspace_root = root.join("workspace"); + let workspace_macros = workspace_root.join("macros"); + let global = root.join("global"); + create_dir_all(&workspace_macros).unwrap(); + create_dir_all(&global).unwrap(); + write( + workspace_macros.join("shared.yaml"), + "description: workspace\nsteps:\n - \".help\"\n", + ) + .unwrap(); + write( + global.join("shared.yaml"), + "description: global\nsteps:\n - \".help\"\n", + ) + .unwrap(); + + let ws_env = get_env_name("workspace_config_dir"); + let global_env = get_env_name("macros_dir"); + let prev_ws = env::var_os(&ws_env); + let prev_global = env::var_os(&global_env); + unsafe { + env::set_var(&ws_env, &workspace_root); + env::set_var(&global_env, &global); + } + f(); + unsafe { + match prev_ws { + Some(v) => env::set_var(&ws_env, v), + None => env::remove_var(&ws_env), + } + match prev_global { + Some(v) => env::set_var(&global_env, v), + None => env::remove_var(&global_env), + } + } + let _ = remove_dir_all(&root); + } + + #[test] + #[serial] + fn load_prefers_workspace_over_global_by_default() { + with_macro_load_envs(|| { + let loaded = Macro::load("shared", false).unwrap(); + assert_eq!(loaded.description.as_deref(), Some("workspace")); + }); + } + + #[test] + #[serial] + fn load_skips_workspace_when_no_workspace_macros() { + with_macro_load_envs(|| { + let loaded = Macro::load("shared", true).unwrap(); + assert_eq!(loaded.description.as_deref(), Some("global")); + }); + } + + /// Drives a macro-execution future to completion on a thread with extra + /// stack headroom: nested `run_repl_command` poll frames are deep in + /// debug builds and overflow the 2 MiB default test-thread stack. + fn run_async(f: F) -> F::Output + where + F: Future + Send, + F::Output: Send, + { + std::thread::scope(|scope| { + std::thread::Builder::new() + .stack_size(8 * 1024 * 1024) + .spawn_scoped(scope, || { + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .unwrap() + .block_on(f) + }) + .unwrap() + .join() + .unwrap() + }) + } fn var(name: &str, rest: bool, default: Option<&str>) -> MacroVariable { MacroVariable { @@ -184,6 +406,8 @@ mod tests { fn macro_with_vars(vars: Vec) -> Macro { Macro { + description: None, + isolated: true, variables: vars, steps: vec![], } @@ -192,21 +416,27 @@ mod tests { #[test] fn resolve_no_variables() { let m = macro_with_vars(vec![]); + let result = m.resolve_variables(&[]).unwrap(); + assert!(result.is_empty()); } #[test] fn resolve_required_variable_provided() { let m = macro_with_vars(vec![var("name", false, None)]); + let result = m.resolve_variables(&["Alice".into()]).unwrap(); + assert_eq!(result["name"], "Alice"); } #[test] fn resolve_required_variable_missing_errors() { let m = macro_with_vars(vec![var("name", false, None)]); + let result = m.resolve_variables(&[]); + assert!(result.is_err()); assert!(result.unwrap_err().to_string().contains("name")); } @@ -214,23 +444,29 @@ mod tests { #[test] fn resolve_default_variable_uses_default() { let m = macro_with_vars(vec![var("color", false, Some("blue"))]); + let result = m.resolve_variables(&[]).unwrap(); + assert_eq!(result["color"], "blue"); } #[test] fn resolve_default_variable_overridden() { let m = macro_with_vars(vec![var("color", false, Some("blue"))]); + let result = m.resolve_variables(&["red".into()]).unwrap(); + assert_eq!(result["color"], "red"); } #[test] fn resolve_rest_variable_captures_all_remaining() { let m = macro_with_vars(vec![var("first", false, None), var("rest", true, None)]); + let result = m .resolve_variables(&["a".into(), "b".into(), "c".into()]) .unwrap(); + assert_eq!(result["first"], "a"); assert_eq!(result["rest"], "b c"); } @@ -238,7 +474,9 @@ mod tests { #[test] fn resolve_rest_variable_with_default() { let m = macro_with_vars(vec![var("args", true, Some("default text"))]); + let result = m.resolve_variables(&[]).unwrap(); + assert_eq!(result["args"], "default text"); } @@ -249,7 +487,9 @@ mod tests { var("b", false, None), var("c", false, Some("default_c")), ]); + let result = m.resolve_variables(&["x".into(), "y".into()]).unwrap(); + assert_eq!(result["a"], "x"); assert_eq!(result["b"], "y"); assert_eq!(result["c"], "default_c"); @@ -258,30 +498,35 @@ mod tests { #[test] fn usage_no_variables() { let m = macro_with_vars(vec![]); + assert_eq!(m.usage("my-macro"), "my-macro"); } #[test] fn usage_required_variable() { let m = macro_with_vars(vec![var("name", false, None)]); + assert_eq!(m.usage("greet"), "greet "); } #[test] fn usage_optional_variable() { let m = macro_with_vars(vec![var("color", false, Some("blue"))]); + assert_eq!(m.usage("paint"), "paint [color]"); } #[test] fn usage_rest_variable() { let m = macro_with_vars(vec![var("args", true, None)]); + assert_eq!(m.usage("run"), "run ..."); } #[test] fn usage_rest_with_default() { let m = macro_with_vars(vec![var("args", true, Some("default"))]); + assert_eq!(m.usage("run"), "run [args]..."); } @@ -291,6 +536,7 @@ mod tests { var("target", false, None), var("flags", true, Some("")), ]); + assert_eq!(m.usage("build"), "build [flags]..."); } @@ -298,6 +544,7 @@ mod tests { fn interpolate_replaces_variables() { let vars = IndexMap::from([("name".to_string(), "world".to_string())]); let result = Macro::interpolate_command("hello {{name}}", &vars); + assert_eq!(result, "hello world"); } @@ -308,6 +555,7 @@ mod tests { ("b".to_string(), "2".to_string()), ]); let result = Macro::interpolate_command("{{a}} + {{b}}", &vars); + assert_eq!(result, "1 + 2"); } @@ -315,6 +563,7 @@ mod tests { fn interpolate_no_variables_passthrough() { let vars = IndexMap::new(); let result = Macro::interpolate_command("no vars here", &vars); + assert_eq!(result, "no vars here"); } @@ -322,6 +571,7 @@ mod tests { fn interpolate_variable_not_found_left_as_is() { let vars = IndexMap::new(); let result = Macro::interpolate_command("hello {{missing}}", &vars); + assert_eq!(result, "hello {{missing}}"); } @@ -354,7 +604,9 @@ variables: rest: true default: "none" "#; + let m: Macro = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(m.variables[0].default, Some("fast".to_string())); assert!(m.variables[1].rest); assert_eq!(m.variables[1].default, Some("none".to_string())); @@ -366,8 +618,294 @@ variables: steps: - ".help" "#; + let m: Macro = serde_yaml::from_str(yaml).unwrap(); + assert!(m.variables.is_empty()); assert_eq!(m.steps.len(), 1); } + + #[test] + fn deserialize_macro_without_new_fields_uses_defaults() { + let yaml = r#" +steps: + - ".help" +"#; + + let m: Macro = serde_yaml::from_str(yaml).unwrap(); + + assert!(m.description.is_none()); + assert!(m.isolated); + } + + #[test] + fn deserialize_macro_with_description_and_isolated() { + let yaml = r#" +description: "Review WIP against a base branch" +isolated: false +steps: + - "Review the diff against {{base}}" +variables: + - name: base + default: main +"#; + + let m: Macro = serde_yaml::from_str(yaml).unwrap(); + + assert_eq!( + m.description.as_deref(), + Some("Review WIP against a base branch") + ); + assert!(!m.isolated); + assert_eq!(m.variables.len(), 1); + } + + #[test] + fn round_trip_preserves_new_fields() { + let original = Macro { + description: Some("does a thing".to_string()), + isolated: false, + variables: vec![var("target", false, Some("all"))], + steps: vec!["build {{target}}".to_string()], + }; + + let yaml = serde_yaml::to_string(&original).unwrap(); + let back: Macro = serde_yaml::from_str(&yaml).unwrap(); + + assert_eq!(back.description.as_deref(), Some("does a thing")); + assert!(!back.isolated); + assert_eq!(back.variables.len(), 1); + assert_eq!(back.variables[0].name, "target"); + assert_eq!(back.variables[0].default.as_deref(), Some("all")); + assert_eq!(back.steps, original.steps); + } + + #[test] + fn round_trip_defaults_survive() { + let original = macro_with_vars(vec![]); + let yaml = serde_yaml::to_string(&original).unwrap(); + + assert!(!yaml.contains("description")); + let back: Macro = serde_yaml::from_str(&yaml).unwrap(); + assert!(back.description.is_none()); + assert!(back.isolated); + } + + #[test] + fn embedded_macro_assets_deserialize_with_defaults() { + for file in MacroAssets::iter() { + let embedded = MacroAssets::get(&file).unwrap(); + let content = str::from_utf8(&embedded.data).unwrap(); + + let m: Macro = serde_yaml::from_str(content) + .unwrap_or_else(|e| panic!("asset '{}' failed to deserialize: {e}", file.as_ref())); + + assert!(m.isolated, "asset '{}'", file.as_ref()); + assert!(!m.steps.is_empty(), "asset '{}'", file.as_ref()); + } + } + + #[test] + #[serial] + fn non_isolated_steps_run_on_live_ctx_and_mutations_persist() { + let _guard = TestConfigDirGuard::new(); + write_macro_file( + "live-macro", + "isolated: false\nsteps:\n - \".set temperature 0.42\"\n", + ); + let mut ctx = test_ctx(); + ctx.session = Some(Session::default()); + + run_async(macro_execute( + &mut ctx, + "live-macro", + None, + create_abort_signal(), + )) + .unwrap(); + + assert!(ctx.session.is_some(), "live session must survive the macro"); + assert_eq!( + ctx.session.as_ref().unwrap().temperature(), + Some(0.42), + "the step must mutate the live context's session, not a fork" + ); + assert!(!ctx.macro_flag, "flag must be restored after success"); + assert!( + !ctx.macro_non_isolated, + "mode must be restored after success" + ); + } + + #[test] + #[serial] + fn non_isolated_step_failure_aborts_and_restores_flag_and_mode() { + let _guard = TestConfigDirGuard::new(); + write_macro_file( + "fail-macro", + "isolated: false\nsteps:\n - \".set temperature 0.9\"\n - \".update\"\n - \".set temperature 0.1\"\n", + ); + let mut ctx = test_ctx(); + ctx.session = Some(Session::default()); + + let result = run_async(macro_execute( + &mut ctx, + "fail-macro", + None, + create_abort_signal(), + )); + + assert!(result.is_err(), "a failing step must abort the macro"); + assert_eq!( + ctx.session.as_ref().unwrap().temperature(), + Some(0.9), + "completed steps' mutations persist; steps after the failure never run" + ); + assert!(!ctx.macro_flag, "flag must be restored on the error path"); + assert!( + !ctx.macro_non_isolated, + "mode must be restored on the error path" + ); + } + + #[test] + fn nested_macro_rejected_when_non_isolated_mode_active() { + let mut ctx = test_ctx(); + ctx.macro_flag = true; + ctx.macro_non_isolated = true; + + let result = run_async(macro_execute( + &mut ctx, + "anything", + None, + create_abort_signal(), + )); + + let err = result.unwrap_err().to_string(); + assert!( + err.contains("nested macros not allowed in non-isolated mode"), + "{err}" + ); + } + + #[test] + #[serial] + fn non_isolated_macro_step_invoking_macro_is_rejected() { + let _guard = TestConfigDirGuard::new(); + write_macro_file( + "outer-macro", + "isolated: false\nsteps:\n - \".inner-macro\"\n", + ); + write_macro_file( + "inner-macro", + "isolated: false\nsteps:\n - \".set temperature 0.5\"\n", + ); + let mut ctx = test_ctx(); + ctx.session = Some(Session::default()); + + let result = run_async(macro_execute( + &mut ctx, + "outer-macro", + None, + create_abort_signal(), + )); + + let err = result.unwrap_err().to_string(); + assert!( + err.contains("nested macros not allowed in non-isolated mode"), + "{err}" + ); + assert_eq!( + ctx.session.as_ref().unwrap().temperature(), + None, + "the nested macro's steps must not run" + ); + assert!(!ctx.macro_flag); + assert!(!ctx.macro_non_isolated); + } + + #[test] + #[serial] + fn isolated_macro_step_runs_non_isolated_macro_inline_on_fork() { + let _guard = TestConfigDirGuard::new(); + write_macro_file("iso-outer-macro", "steps:\n - \".inner-macro\"\n"); + write_macro_file( + "inner-macro", + "isolated: false\nsteps:\n - \".set temperature 0.33\"\n", + ); + let mut ctx = test_ctx(); + ctx.session = Some(Session::default()); + + run_async(macro_execute( + &mut ctx, + "iso-outer-macro", + None, + create_abort_signal(), + )) + .unwrap(); + + assert_eq!( + ctx.session.as_ref().unwrap().temperature(), + None, + "the inline run happens on the fork, never on the live context" + ); + assert!(!ctx.macro_flag); + assert!(!ctx.macro_non_isolated); + } + + #[test] + #[serial] + fn isolated_macro_still_forks_and_leaves_live_ctx_untouched() { + let _guard = TestConfigDirGuard::new(); + write_macro_file("iso-macro", "steps:\n - \".set temperature 0.77\"\n"); + let mut ctx = test_ctx(); + ctx.session = Some(Session::default()); + let app_before = Arc::clone(&ctx.app.config); + + run_async(macro_execute( + &mut ctx, + "iso-macro", + None, + create_abort_signal(), + )) + .unwrap(); + + assert!(ctx.session.is_some()); + assert_eq!( + ctx.session.as_ref().unwrap().temperature(), + None, + "an isolated macro's mutations must stay on the fork" + ); + assert!( + Arc::ptr_eq(&ctx.app.config, &app_before), + "isolated execution must not swap the live app config" + ); + assert!(!ctx.macro_flag); + } + + #[test] + #[serial] + fn guard_restores_prior_flag_values_after_inline_run() { + let _guard = TestConfigDirGuard::new(); + write_macro_file( + "inner-macro", + "isolated: false\nsteps:\n - \".set temperature 0.11\"\n", + ); + let mut ctx = test_ctx(); + ctx.macro_flag = true; + + run_async(macro_execute( + &mut ctx, + "inner-macro", + None, + create_abort_signal(), + )) + .unwrap(); + + assert!( + ctx.macro_flag, + "a pre-existing flag must be restored, not cleared" + ); + assert!(!ctx.macro_non_isolated); + } } diff --git a/src/config/mod.rs b/src/config/mod.rs index e92f5b6..ea90e76 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -4,6 +4,7 @@ mod app_state; mod input; mod install_remote; pub(crate) mod instructions; +mod macro_policy; mod macros; mod mcp_factory; pub(crate) mod memory; @@ -30,6 +31,9 @@ pub use self::app_config::AppConfig; pub use self::app_state::AppState; pub use self::input::Input; pub use self::install_remote::{install_remote, install_remote_from_repl_args}; +pub use self::macro_policy::{ + MacroAllowlistLevel, MacroPolicy, MacroSource, MacroState, RESERVED_MACRO_NAMES, ResolvedMacro, +}; #[allow(unused_imports)] pub use self::request_context::{RenderMode, RequestContext, should_inject_skill_instructions}; pub use self::role::{ @@ -221,6 +225,8 @@ pub struct Config { #[serde(default, deserialize_with = "deserialize_csv_or_vec")] pub enabled_skills: Option>, pub visible_skills: Option>, + #[serde(default, deserialize_with = "deserialize_csv_or_vec")] + pub enabled_macros: Option>, pub mcp_server_support: bool, pub mapping_mcp_servers: IndexMap, @@ -303,6 +309,7 @@ impl Default for Config { skills_enabled: true, enabled_skills: None, visible_skills: None, + enabled_macros: None, mcp_server_support: true, mapping_mcp_servers: Default::default(), @@ -1124,9 +1131,50 @@ clients: assert!(cfg.enabled_mcp_servers.is_none()); } + #[test] + fn config_enabled_macros_absent_is_none() { + let cfg: Config = serde_yaml::from_str("model: provider:test").unwrap(); + assert_eq!(cfg.enabled_macros, None); + } + + #[test] + fn config_enabled_macros_empty_string_is_some_empty() { + let cfg: Config = serde_yaml::from_str("enabled_macros: \"\"").unwrap(); + + assert_eq!(cfg.enabled_macros, Some(vec![])); + } + + #[test] + fn config_enabled_macros_csv_string() { + let cfg: Config = serde_yaml::from_str("enabled_macros: \"a, b\"").unwrap(); + + assert_eq!( + cfg.enabled_macros, + Some(vec!["a".to_string(), "b".to_string()]) + ); + } + + #[test] + fn config_enabled_macros_list() { + let cfg: Config = serde_yaml::from_str("enabled_macros:\n - a\n - b").unwrap(); + + assert_eq!( + cfg.enabled_macros, + Some(vec!["a".to_string(), "b".to_string()]) + ); + } + + #[test] + fn config_enabled_macros_null_is_none() { + let cfg: Config = serde_yaml::from_str("enabled_macros: null").unwrap(); + + assert_eq!(cfg.enabled_macros, None); + } + #[test] fn assert_state_pass_always_true() { let pass = AssertState::pass(); + assert!(pass.assert(StateFlags::empty())); assert!(pass.assert(StateFlags::ROLE)); assert!(pass.assert(StateFlags::SESSION | StateFlags::AGENT)); @@ -1136,6 +1184,7 @@ clients: #[test] fn assert_state_bare_only_empty() { let bare = AssertState::bare(); + assert!(bare.assert(StateFlags::empty())); assert!(!bare.assert(StateFlags::ROLE)); assert!(!bare.assert(StateFlags::SESSION)); @@ -1144,6 +1193,7 @@ clients: #[test] fn assert_state_true_requires_flag_present() { let state = AssertState::True(StateFlags::ROLE); + assert!(state.assert(StateFlags::ROLE)); assert!(state.assert(StateFlags::ROLE | StateFlags::SESSION)); assert!(!state.assert(StateFlags::empty())); @@ -1153,6 +1203,7 @@ clients: #[test] fn assert_state_true_with_multiple_flags_any_match() { let state = AssertState::True(StateFlags::SESSION_EMPTY | StateFlags::SESSION); + assert!(state.assert(StateFlags::SESSION_EMPTY)); assert!(state.assert(StateFlags::SESSION)); assert!(state.assert(StateFlags::SESSION | StateFlags::ROLE)); @@ -1163,6 +1214,7 @@ clients: #[test] fn assert_state_false_requires_flag_absent() { let state = AssertState::False(StateFlags::AGENT); + assert!(state.assert(StateFlags::empty())); assert!(state.assert(StateFlags::ROLE)); assert!(!state.assert(StateFlags::AGENT)); @@ -1172,6 +1224,7 @@ clients: #[test] fn assert_state_false_with_multiple_flags() { let state = AssertState::False(StateFlags::SESSION | StateFlags::AGENT); + assert!(state.assert(StateFlags::empty())); assert!(state.assert(StateFlags::ROLE)); assert!(!state.assert(StateFlags::SESSION)); @@ -1182,6 +1235,7 @@ clients: #[test] fn assert_state_truefalse_requires_true_present_and_false_absent() { let state = AssertState::TrueFalse(StateFlags::ROLE, StateFlags::SESSION); + assert!(state.assert(StateFlags::ROLE)); assert!(state.assert(StateFlags::ROLE | StateFlags::RAG)); assert!(!state.assert(StateFlags::empty())); @@ -1192,6 +1246,7 @@ clients: #[test] fn assert_state_equal_exact_match() { let state = AssertState::Equal(StateFlags::ROLE | StateFlags::SESSION); + assert!(state.assert(StateFlags::ROLE | StateFlags::SESSION)); assert!(!state.assert(StateFlags::ROLE)); assert!(!state.assert(StateFlags::SESSION)); diff --git a/src/config/paths.rs b/src/config/paths.rs index 5beeeb7..6a0ecfe 100644 --- a/src/config/paths.rs +++ b/src/config/paths.rs @@ -214,6 +214,10 @@ pub fn workspace_skill_file(name: &str) -> PathBuf { workspace_skills_dir().join(name).join("SKILL.md") } +pub fn workspace_macros_dir() -> PathBuf { + workspace_config_dir().join(MACROS_DIR_NAME) +} + pub fn workspace_mcp_config_file() -> Option { workspace_mcp_config_file_in(&env::current_dir().unwrap_or_default()) } @@ -470,11 +474,6 @@ pub fn list_macros() -> Vec { list_file_names(macros_dir(), ".yaml") } -pub fn has_macro(name: &str) -> bool { - let names = list_macros(); - names.contains(&name.to_string()) -} - pub fn list_skills() -> Vec { let mut names = Vec::new(); let mut seen = HashSet::new(); diff --git a/src/config/request_context.rs b/src/config/request_context.rs index 606cc48..24597b6 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -7,7 +7,8 @@ use super::todo::TodoList; use super::tool_scope::{McpRuntime, ToolScope}; use super::{ AGENTS_DIR_NAME, Agent, AgentVariables, AppConfig, AppState, AssetCategory, CREATE_TITLE_ROLE, - Input, InstallFilter, LEFT_PROMPT, LastMessage, MESSAGES_FILE_NAME, RIGHT_PROMPT, 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, ensure_parent_exists, list_agents_with_descriptions, memory, paths, @@ -32,6 +33,7 @@ use crate::utils::{ AbortSignal, abortable_run_with_spinner, edit_file, fuzzy_filter, get_env_name, list_file_names, now, render_prompt, temp_file, }; +use comfy_table::{ContentArrangement, Table, presets::UTF8_FULL}; use super::instructions; use super::memory::{ @@ -110,6 +112,14 @@ fn print_asset_names(kind: &str, names: &[String]) -> Result<()> { Ok(()) } +fn asset_table(header: &[&str]) -> Table { + let mut table = Table::new(); + table.load_preset(UTF8_FULL); + table.set_content_arrangement(ContentArrangement::Dynamic); + table.set_header(header.to_vec()); + table +} + fn complete_skills_with_descriptions(names: Vec) -> Vec<(String, Option)> { names .into_iter() @@ -123,6 +133,95 @@ fn complete_skills_with_descriptions(names: Vec) -> Vec<(String, Option< .collect() } +const SET_COMPLETION_KEYS: [&str; 26] = [ + "auto_continue", + "continuation_prompt", + "temperature", + "top_p", + "enabled_macros", + "enabled_skills", + "enabled_tools", + "enabled_mcp_servers", + "inject_todo_instructions", + "inject_skill_instructions", + "skill_instructions", + "max_auto_continues", + "memory", + "save_session", + "compression_threshold", + "rag_reranker_model", + "rag_top_k", + "max_output_tokens", + "dry_run", + "function_calling_support", + "mcp_server_support", + "skills_enabled", + "stream", + "save", + "highlight", + "raw_markdown", +]; + +fn toggled_enabled_macros( + current: Option<&[String]>, + all_active: &[String], + name: &str, + enable: bool, +) -> Option> { + match (current, enable) { + (None, true) => None, + (Some(list), true) => { + if list.iter().any(|v| v == name) { + None + } else { + let mut list = list.to_vec(); + list.push(name.to_string()); + Some(list) + } + } + (None, false) => Some( + all_active + .iter() + .filter(|v| v.as_str() != name) + .cloned() + .collect(), + ), + (Some(list), false) => { + if list.iter().any(|v| v == name) { + Some( + list.iter() + .filter(|v| v.as_str() != name) + .cloned() + .collect(), + ) + } else { + None + } + } + } +} + +fn macro_state_display( + row: &ResolvedMacro, + lock_owner: impl Fn(MacroAllowlistLevel) -> String, +) -> String { + match &row.state { + MacroState::Enabled => "enabled".to_string(), + MacroState::DisabledRuntime => "disabled (runtime)".to_string(), + MacroState::Locked { level } => format!("locked ({} enabled_macros)", lock_owner(*level)), + MacroState::Missing => "missing".to_string(), + MacroState::ShadowedBuiltin => "shadowed (built-in)".to_string(), + MacroState::Invalid { reason } => format!("invalid ({reason})"), + } +} + +fn macro_source_display(source: Option) -> String { + match source { + Some(source) => source.to_string(), + None => "-".to_string(), + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] pub enum RenderMode { #[default] @@ -134,6 +233,7 @@ pub struct RequestContext { pub app: Arc, pub macro_flag: bool, + pub macro_non_isolated: bool, pub info_flag: bool, pub working_mode: WorkingMode, @@ -171,6 +271,7 @@ impl RequestContext { Self { app, macro_flag: false, + macro_non_isolated: false, info_flag: false, working_mode, model: Default::default(), @@ -225,6 +326,7 @@ impl RequestContext { Ok(Self { app, macro_flag: false, + macro_non_isolated: false, info_flag, working_mode, model, @@ -274,6 +376,7 @@ impl RequestContext { Self { app: Arc::clone(&self.app), macro_flag: self.macro_flag, + macro_non_isolated: self.macro_non_isolated, info_flag: self.info_flag, working_mode: self.working_mode, model: self.model.clone(), @@ -313,6 +416,7 @@ impl RequestContext { Self { app, macro_flag: parent.macro_flag, + macro_non_isolated: parent.macro_non_isolated, info_flag: parent.info_flag, working_mode: WorkingMode::Cmd, model: parent.model.clone(), @@ -1577,6 +1681,10 @@ impl RequestContext { "enabled_skills", super::format_option_value(&role.enabled_skills().map(|v| v.join(","))), ), + ( + "enabled_macros", + super::format_option_value(&role.enabled_macros().map(|v| v.join(","))), + ), ( "max_output_tokens", role.model() @@ -2357,6 +2465,9 @@ impl RequestContext { } pub fn new_macro(&self, app: &AppConfig, name: &str) -> Result<()> { + if RESERVED_MACRO_NAMES.contains(&name) { + bail!("'{name}' is a reserved macro name"); + } if self.macro_flag { bail!("No macro"); } @@ -2374,12 +2485,147 @@ impl RequestContext { Ok(()) } + pub fn in_non_isolated_macro(&self) -> bool { + self.macro_flag && self.macro_non_isolated + } + + pub fn macro_policy(&self) -> MacroPolicy { + MacroPolicy::effective( + &self.app.config, + self.role.as_ref(), + self.agent.as_ref(), + self.session.as_ref(), + &crate::repl::builtin_command_names(), + self.app.config.no_workspace_macros, + ) + } + + pub fn macro_lock_owner(&self, level: MacroAllowlistLevel) -> String { + let name = match level { + MacroAllowlistLevel::Session => self.session.as_ref().map(|s| s.name()), + MacroAllowlistLevel::Agent => self.agent.as_ref().map(|a| a.name()), + MacroAllowlistLevel::Role => self.role.as_ref().map(|r| r.name()), + MacroAllowlistLevel::Global => return "global config".to_string(), + }; + match name { + Some(name) => format!("{level}:{name}"), + None => level.to_string(), + } + } + + pub fn macro_toggle(&mut self, name: &str, enable: bool) -> Result<()> { + let restricting_level = if self + .session + .as_ref() + .and_then(|s| s.enabled_macros()) + .is_some() + { + Some(MacroAllowlistLevel::Session) + } else if self + .agent + .as_ref() + .and_then(|a| a.enabled_macros()) + .is_some() + { + Some(MacroAllowlistLevel::Agent) + } else if self + .role + .as_ref() + .and_then(|r| r.enabled_macros()) + .is_some() + { + Some(MacroAllowlistLevel::Role) + } else { + None + }; + if let Some(level) = restricting_level { + bail!( + "Macro toggles are restricted by {} enabled_macros; edit enabled_macros there", + self.macro_lock_owner(level) + ); + } + + let policy = self.macro_policy(); + match policy.find(name).map(|row| &row.state) { + None => bail!("Unknown macro '{name}'"), + Some(MacroState::Invalid { reason }) => bail!("Macro '{name}' is invalid: {reason}"), + Some(_) => {} + } + let all_active: Vec = policy + .macros + .iter() + .filter(|row| { + row.source.is_some() + && !row.shadowed_by_workspace + && !matches!(row.state, MacroState::Missing | MacroState::Invalid { .. }) + }) + .map(|row| row.name.clone()) + .collect(); + + let action = if enable { "enabled" } else { "disabled" }; + + match toggled_enabled_macros( + self.app.config.enabled_macros.as_deref(), + &all_active, + name, + enable, + ) { + Some(list) => { + self.update_app_config(|app| app.enabled_macros = Some(list)); + println!("Macro '{name}' {action}"); + } + None => println!("Macro '{name}' is already {action}"), + } + + Ok(()) + } + + pub fn visible_macro_completions(&self) -> Vec<(String, Option)> { + self.macro_policy() + .macros + .into_iter() + .filter(|row| row.state == MacroState::Enabled && !row.shadowed_by_workspace) + .map(|row| (row.name, row.description)) + .collect() + } + 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()), "rags" => print_asset_names("RAGs", &paths::list_rags()), - "macros" => print_asset_names("macros", &paths::list_macros()), + "macros" => { + let policy = self.macro_policy(); + if policy.macros.is_empty() { + println!("No macros found."); + return Ok(()); + } + + let mut table = + asset_table(&["name", "source", "isolated", "state", "description"]); + + for row in &policy.macros { + let source = macro_source_display(row.source); + let isolated = match row.isolated { + Some(true) => "yes", + Some(false) => "no", + None => "-", + }; + let state = macro_state_display(row, |level| self.macro_lock_owner(level)); + let description = row.description.as_deref().unwrap_or_default(); + table.add_row(vec![ + row.name.as_str(), + &source, + isolated, + &state, + description, + ]); + } + + println!("Macros:"); + println!("{table}"); + Ok(()) + } "agents" => { let entries = list_agents_with_descriptions(); if entries.is_empty() { @@ -2387,15 +2633,13 @@ impl RequestContext { return Ok(()); } - println!("Agents:"); + let mut table = asset_table(&["name", "description"]); for (name, description) in entries { - if description.is_empty() { - println!(" • {name}"); - } else { - println!(" • {name} — {description}"); - } + table.add_row(vec![name, description]); } + println!("Agents:"); + println!("{table}"); Ok(()) } "skills" => { @@ -2441,16 +2685,18 @@ impl RequestContext { return Ok(()); } - println!("Skills:"); + let mut table = asset_table(&["loaded", "name", "description"]); for (name, description, loaded) in entries { let marker = if loaded { "✓".green().bold().to_string() } else { "✗".red().bold().to_string() }; - println!(" {marker} {name} — {description}"); + table.add_row(vec![marker, name, description]); } + println!("Skills:"); + println!("{table}"); Ok(()) } "tools" => { @@ -2728,6 +2974,23 @@ impl RequestContext { } self.update_app_config(|app| app.enabled_skills = parsed.clone()); } + "enabled_macros" => { + let raw: Option = super::parse_value(value)?; + let parsed: Option> = raw.map(|s| super::csv_to_vec(&s)); + if let Some(names) = parsed.as_ref() { + let policy = self.macro_policy(); + for name in names { + if !policy + .macros + .iter() + .any(|m| m.source.is_some() && &m.name == name) + { + bail!("macro '{name}' is not installed"); + } + } + } + self.update_app_config(|app| app.enabled_macros = parsed.clone()); + } "skills_enabled" => { let value: Option = super::parse_value(value)?; if let Some(session) = self.session.as_mut() { @@ -3001,7 +3264,28 @@ impl RequestContext { values.push("remote".to_string()); super::map_completion_values(values) } - ".macro" => super::map_completion_values(paths::list_macros()), + ".macro" => { + let policy = self.macro_policy(); + let mut values: Vec<(String, Option)> = policy + .macros + .iter() + .filter(|row| { + row.source.is_some() + && !row.shadowed_by_workspace + && row.state.is_invocable() + }) + .map(|row| (row.name.clone(), row.description.clone())) + .collect(); + values.push(( + "enable ".to_string(), + Some("Re-enable a runtime-disabled macro".to_string()), + )); + values.push(( + "disable ".to_string(), + Some("Disable a macro for the rest of this process".to_string()), + )); + values + } ".reasoning" => { let levels = self.current_model().reasoning_levels(); levels.iter().map(|v| (v.clone(), None)).collect() @@ -3016,32 +3300,7 @@ impl RequestContext { None => vec![], }, ".set" => { - let mut values = vec![ - "auto_continue", - "continuation_prompt", - "temperature", - "top_p", - "enabled_tools", - "enabled_mcp_servers", - "inject_todo_instructions", - "inject_skill_instructions", - "skill_instructions", - "max_auto_continues", - "memory", - "save_session", - "compression_threshold", - "rag_reranker_model", - "rag_top_k", - "max_output_tokens", - "dry_run", - "function_calling_support", - "mcp_server_support", - "skills_enabled", - "stream", - "save", - "highlight", - "raw_markdown", - ]; + let mut values = SET_COMPLETION_KEYS.to_vec(); if !self.current_model().reasoning_levels().is_empty() { values.push("reasoning_effort"); } @@ -3185,6 +3444,25 @@ impl RequestContext { .collect() }; values = super::map_completion_values(candidates); + } else if cmd == ".macro" + && (args.first() == Some(&"enable") || args.first() == Some(&"disable")) + && args.len() == 2 + { + let enable = args.first() == Some(&"enable"); + values = self + .macro_policy() + .macros + .into_iter() + .filter(|row| row.source.is_some() && !row.shadowed_by_workspace) + .filter(|row| { + if enable { + row.state == MacroState::DisabledRuntime + } else { + row.state.is_invocable() + } + }) + .map(|row| (row.name, row.description)) + .collect(); } else if (cmd == ".edit" && args.first() == Some(&"skill") && args.len() == 2) || (cmd == ".skill" && args.first() == Some(&"load") && args.len() == 2) { @@ -3740,8 +4018,11 @@ impl RequestContext { // Graph agents manage their own state; never engage a session, // not even an inherited app-level `agent_session` default. + // Isolated macros suppress an inherited default too: their forked + // context has no session to return to. A non-isolated macro's `.agent` + // step engages it exactly as if the user had typed the command. let session_name = session_name.map(|v| v.to_string()).or_else(|| { - if self.macro_flag || is_graph_agent { + if (self.macro_flag && !self.macro_non_isolated) || is_graph_agent { None } else { agent.agent_session().map(|v| v.to_string()) @@ -6290,6 +6571,73 @@ mod tests { ); } + #[test] + #[serial] + fn use_agent_suppresses_inherited_session_in_isolated_macro() { + let _guard = TestConfigDirGuard::new(); + let mut ctx = create_test_ctx(); + ctx.macro_flag = true; + ctx.update_app_config(|app| app.agent_session = Some("inherited".to_string())); + + let app = ctx.app.config.clone(); + let agent_name = format!( + "test_agent_{}", + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos() + ); + let agent_dir = paths::agent_data_dir(&agent_name); + create_dir_all(&agent_dir).unwrap(); + write( + agent_dir.join("config.yaml"), + format!("name: {agent_name}\ninstructions: hi\n"), + ) + .unwrap(); + + let abort = utils::create_abort_signal(); + run_async(ctx.use_agent(&app, &agent_name, None, abort)).unwrap(); + + assert!( + ctx.session.is_none(), + "an isolated macro must keep suppressing the agent's default session" + ); + } + + #[test] + #[serial] + fn use_agent_engages_inherited_session_in_non_isolated_macro() { + let _guard = TestConfigDirGuard::new(); + let mut ctx = create_test_ctx(); + ctx.macro_flag = true; + ctx.macro_non_isolated = true; + ctx.update_app_config(|app| app.agent_session = Some("inherited".to_string())); + + let app = ctx.app.config.clone(); + let agent_name = format!( + "test_agent_{}", + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos() + ); + let agent_dir = paths::agent_data_dir(&agent_name); + create_dir_all(&agent_dir).unwrap(); + write( + agent_dir.join("config.yaml"), + format!("name: {agent_name}\ninstructions: hi\n"), + ) + .unwrap(); + + let abort = utils::create_abort_signal(); + run_async(ctx.use_agent(&app, &agent_name, None, abort)).unwrap(); + + assert!( + ctx.session.is_some(), + "a non-isolated macro's agent step must engage the default session as if typed" + ); + } + fn first_file(dir: &Path) -> Option { for entry in read_dir(dir).ok()?.flatten() { let path = entry.path(); @@ -6463,4 +6811,115 @@ mod tests { "install_mcp_config must add new bundled servers" ); } + + fn strings(names: &[&str]) -> Vec { + names.iter().map(|s| s.to_string()).collect() + } + + #[test] + fn toggled_enabled_macros_covers_all_transitions() { + let all_active = strings(&["a", "b", "c"]); + type ToggleCase = (Option>, &'static str, bool, Option>); + let cases: Vec = vec![ + (None, "a", true, None), + (Some(strings(&["a"])), "a", true, None), + (Some(strings(&["a"])), "b", true, Some(strings(&["a", "b"]))), + (None, "b", false, Some(strings(&["a", "c"]))), + ( + Some(strings(&["a", "b"])), + "b", + false, + Some(strings(&["a"])), + ), + (Some(strings(&["a"])), "b", false, None), + ]; + for (current, name, enable, expected) in cases { + let result = toggled_enabled_macros(current.as_deref(), &all_active, name, enable); + assert_eq!( + result, expected, + "current={current:?} name={name} enable={enable}" + ); + } + } + + fn resolved(state: MacroState) -> ResolvedMacro { + ResolvedMacro { + name: "m".to_string(), + source: Some(MacroSource::Global), + description: None, + isolated: None, + shadowed_by_workspace: false, + state, + } + } + + #[test] + fn macro_state_display_covers_all_states() { + let owner = |level: MacroAllowlistLevel| format!("{level}:test"); + let cases = vec![ + (MacroState::Enabled, "enabled"), + (MacroState::DisabledRuntime, "disabled (runtime)"), + ( + MacroState::Locked { + level: MacroAllowlistLevel::Agent, + }, + "locked (agent:test enabled_macros)", + ), + (MacroState::Missing, "missing"), + (MacroState::ShadowedBuiltin, "shadowed (built-in)"), + ( + MacroState::Invalid { + reason: "boom".to_string(), + }, + "invalid (boom)", + ), + ]; + for (state, expected) in cases { + assert_eq!(macro_state_display(&resolved(state), owner), expected); + } + } + + #[test] + fn macro_source_display_names_source_or_dash() { + assert_eq!( + macro_source_display(Some(MacroSource::Workspace)), + "workspace" + ); + assert_eq!(macro_source_display(Some(MacroSource::Global)), "global"); + assert_eq!(macro_source_display(None), "-"); + } + + #[test] + fn set_completion_keys_include_enabled_skills_and_macros() { + assert!(SET_COMPLETION_KEYS.contains(&"enabled_skills")); + assert!(SET_COMPLETION_KEYS.contains(&"enabled_macros")); + } + + #[test] + fn new_macro_rejects_reserved_names() { + let ctx = create_test_ctx(); + let app = ctx.app.config.clone(); + for name in RESERVED_MACRO_NAMES { + let err = ctx.new_macro(&app, name).unwrap_err(); + assert_eq!( + err.to_string(), + format!("'{name}' is a reserved macro name") + ); + } + } + + #[test] + fn macro_lock_owner_names_the_owning_config() { + let mut ctx = create_test_ctx(); + assert_eq!(ctx.macro_lock_owner(MacroAllowlistLevel::Role), "role"); + ctx.role = Some(Role::new("coder", "prompt")); + assert_eq!( + ctx.macro_lock_owner(MacroAllowlistLevel::Role), + "role:coder" + ); + assert_eq!( + ctx.macro_lock_owner(MacroAllowlistLevel::Global), + "global config" + ); + } } diff --git a/src/config/role.rs b/src/config/role.rs index cfb6104..db82655 100644 --- a/src/config/role.rs +++ b/src/config/role.rs @@ -75,6 +75,12 @@ pub struct Role { deserialize_with = "super::deserialize_csv_or_vec" )] enabled_skills: Option>, + #[serde( + default, + skip_serializing_if = "Option::is_none", + deserialize_with = "super::deserialize_csv_or_vec" + )] + enabled_macros: Option>, #[serde(skip_serializing_if = "Option::is_none")] auto_continue: Option, #[serde(skip_serializing_if = "Option::is_none")] @@ -129,6 +135,7 @@ impl Role { } "skills_enabled" => role.skills_enabled = value.as_bool(), "enabled_skills" => role.enabled_skills = parse_string_or_array(value), + "enabled_macros" => role.enabled_macros = parse_string_or_array(value), "auto_continue" => role.auto_continue = value.as_bool(), "max_auto_continues" => { role.max_auto_continues = value.as_u64().map(|v| v as usize) @@ -196,6 +203,10 @@ impl Role { let inline = serde_json::to_string(enabled_skills).unwrap_or_else(|_| "[]".to_string()); metadata.push(format!("enabled_skills: {inline}")); } + if let Some(enabled_macros) = &self.enabled_macros { + let inline = serde_json::to_string(enabled_macros).unwrap_or_else(|_| "[]".to_string()); + metadata.push(format!("enabled_macros: {inline}")); + } if let Some(auto_continue) = self.auto_continue { metadata.push(format!("auto_continue: {auto_continue}")); } @@ -357,6 +368,10 @@ impl Role { self.enabled_skills.as_deref() } + pub fn enabled_macros(&self) -> Option<&[String]> { + self.enabled_macros.as_deref() + } + pub fn append_to_prompt(&mut self, text: &str) { self.prompt.push_str(text); } @@ -543,6 +558,7 @@ mod tests { #[test] fn role_new_parses_prompt() { let role = Role::new("test", "You are a helpful assistant"); + assert_eq!(role.name(), "test"); assert_eq!(role.prompt(), "You are a helpful assistant"); } @@ -551,7 +567,9 @@ mod tests { fn role_new_parses_metadata() { let content = "---\nmodel: openai:gpt-4\ntemperature: 0.7\ntop_p: 0.9\n---\nYou are helpful"; + let role = Role::new("test", content); + assert_eq!(role.model_id(), Some("openai:gpt-4")); assert_eq!(role.temperature(), Some(0.7)); assert_eq!(role.top_p(), Some(0.9)); @@ -561,7 +579,9 @@ mod tests { #[test] fn role_new_parses_enabled_tools() { let content = "---\nenabled_tools: tool1,tool2\n---\nPrompt"; + let role = Role::new("test", content); + assert_eq!( role.enabled_tools(), Some(vec!["tool1".to_string(), "tool2".to_string()]) @@ -571,7 +591,9 @@ mod tests { #[test] fn role_new_parses_enabled_mcp_servers() { let content = "---\nenabled_mcp_servers: github,jira\n---\nPrompt"; + let role = Role::new("test", content); + assert_eq!( role.enabled_mcp_servers(), Some(vec!["github".to_string(), "jira".to_string()]) @@ -581,6 +603,7 @@ mod tests { #[test] fn role_new_no_metadata_has_none_fields() { let role = Role::new("test", "Just a prompt"); + assert_eq!(role.model_id(), None); assert_eq!(role.temperature(), None); assert_eq!(role.top_p(), None); @@ -588,9 +611,67 @@ mod tests { assert_eq!(role.enabled_mcp_servers(), None); } + #[test] + fn role_new_enabled_macros_absent_is_none() { + let role = Role::new("test", "---\ntemperature: 0.5\n---\nPrompt"); + + assert_eq!(role.enabled_macros, None); + } + + #[test] + fn role_new_enabled_macros_empty_string_is_some_empty() { + let role = Role::new("test", "---\nenabled_macros: \"\"\n---\nPrompt"); + + assert_eq!(role.enabled_macros, Some(vec![])); + } + + #[test] + fn role_new_enabled_macros_csv_string() { + let role = Role::new("test", "---\nenabled_macros: a, b\n---\nPrompt"); + + assert_eq!( + role.enabled_macros, + Some(vec!["a".to_string(), "b".to_string()]) + ); + } + + #[test] + fn role_new_enabled_macros_list() { + let role = Role::new("test", "---\nenabled_macros: [a, b]\n---\nPrompt"); + + assert_eq!( + role.enabled_macros, + Some(vec!["a".to_string(), "b".to_string()]) + ); + } + + #[test] + fn role_new_enabled_macros_null_is_none() { + let role = Role::new("test", "---\nenabled_macros: null\n---\nPrompt"); + + assert_eq!(role.enabled_macros, None); + } + + #[test] + fn role_export_includes_enabled_macros() { + let role = Role::new("test", "---\nenabled_macros: [a]\n---\nPrompt"); + + let exported = role.export(); + + assert!(exported.contains("enabled_macros: [\"a\"]")); + } + + #[test] + fn role_export_omits_enabled_macros_when_none() { + let role = Role::new("test", "Just a prompt"); + + assert!(!role.export().contains("enabled_macros")); + } + #[test] fn role_builtin_shell_loads() { let role = Role::builtin("shell").unwrap(); + assert_eq!(role.name(), "shell"); assert!(!role.prompt().is_empty()); } @@ -598,6 +679,7 @@ mod tests { #[test] fn role_builtin_code_loads() { let role = Role::builtin("code").unwrap(); + assert_eq!(role.name(), "code"); assert!(!role.prompt().is_empty()); } @@ -605,12 +687,14 @@ mod tests { #[test] fn role_builtin_nonexistent_errors() { let result = Role::builtin("nonexistent_role_xyz"); + assert!(result.is_err()); } #[test] fn role_default_has_empty_fields() { let role = Role::default(); + assert_eq!(role.name(), ""); assert_eq!(role.prompt(), ""); assert_eq!(role.model_id(), None); @@ -620,14 +704,18 @@ mod tests { fn role_set_model_updates_model() { let mut role = Role::new("test", "prompt"); let model = Model::default(); + role.set_model(model.clone()); + assert_eq!(role.model().id(), model.id()); } #[test] fn role_set_temperature_works() { let mut role = Role::new("test", "prompt"); + role.set_temperature(Some(0.5)); + assert_eq!(role.temperature(), Some(0.5)); } @@ -635,7 +723,9 @@ mod tests { fn role_export_includes_metadata() { let content = "---\ntemperature: 0.8\n---\nMy prompt"; let role = Role::new("test", content); + let exported = role.export(); + assert!(exported.contains("temperature")); assert!(exported.contains("My prompt")); } @@ -649,6 +739,7 @@ Input 1 ### OUTPUT: Output 1 "#; + assert_eq!( parse_structure_prompt(prompt), ("System message", vec![("Input 1", "Output 1")]) @@ -663,6 +754,7 @@ Input 1 ### OUTPUT: Output 1 "#; + assert_eq!( parse_structure_prompt(prompt), ("", vec![("Input 1", "Output 1")]) @@ -676,6 +768,7 @@ System message ### INPUT: Input 1 "#; + assert_eq!(parse_structure_prompt(prompt), (prompt, vec![])); } } diff --git a/src/config/session.rs b/src/config/session.rs index 0b593b7..1e3b042 100644 --- a/src/config/session.rs +++ b/src/config/session.rs @@ -46,6 +46,12 @@ pub struct Session { deserialize_with = "super::deserialize_csv_or_vec" )] enabled_skills: Option>, + #[serde( + default, + skip_serializing_if = "Option::is_none", + deserialize_with = "super::deserialize_csv_or_vec" + )] + enabled_macros: Option>, #[serde(skip_serializing_if = "Option::is_none")] save_session: Option, #[serde(skip_serializing_if = "Option::is_none")] @@ -107,6 +113,10 @@ impl Session { self.enabled_skills.as_deref() } + pub fn enabled_macros(&self) -> Option<&[String]> { + self.enabled_macros.as_deref() + } + pub fn set_skills_enabled(&mut self, value: Option) { if self.skills_enabled != value { self.skills_enabled = value; @@ -236,6 +246,9 @@ impl Session { if let Some(enabled_skills) = self.enabled_skills() { data["enabled_skills"] = json!(enabled_skills); } + if let Some(enabled_macros) = self.enabled_macros() { + data["enabled_macros"] = json!(enabled_macros); + } if let Some(save_session) = self.save_session() { data["save_session"] = save_session.into(); } @@ -315,6 +328,10 @@ impl Session { items.push(("enabled_skills", enabled_skills.join(","))); } + if let Some(enabled_macros) = self.enabled_macros() { + items.push(("enabled_macros", enabled_macros.join(","))); + } + if let Some(save_session) = self.save_session() { items.push(("save_session", save_session.to_string())); } @@ -925,12 +942,57 @@ mod tests { #[test] fn session_default_is_empty() { let session = Session::default(); + assert!(session.is_empty()); assert_eq!(session.name(), ""); assert_eq!(session.role_name(), None); assert!(!session.dirty()); } + #[test] + fn session_enabled_macros_absent_is_none() { + let session: Session = serde_yaml::from_str("model: provider:test\nmessages: []").unwrap(); + + assert_eq!(session.enabled_macros, None); + } + + #[test] + fn session_enabled_macros_empty_list_is_some_empty() { + let session: Session = + serde_yaml::from_str("model: provider:test\nenabled_macros: []\nmessages: []").unwrap(); + + assert_eq!(session.enabled_macros, Some(vec![])); + } + + #[test] + fn session_enabled_macros_empty_string_is_some_empty() { + let session: Session = + serde_yaml::from_str("model: provider:test\nenabled_macros: \"\"\nmessages: []") + .unwrap(); + + assert_eq!(session.enabled_macros, Some(vec![])); + } + + #[test] + fn session_enabled_macros_csv_string() { + let session: Session = + serde_yaml::from_str("model: provider:test\nenabled_macros: \"a,b\"\nmessages: []") + .unwrap(); + + assert_eq!( + session.enabled_macros, + Some(vec!["a".to_string(), "b".to_string()]) + ); + } + + #[test] + fn session_serialize_omits_enabled_macros_when_none() { + let session = Session::default(); + let yaml = serde_yaml::to_string(&session).unwrap(); + + assert!(!yaml.contains("enabled_macros")); + } + #[test] fn session_new_from_ctx_captures_save_session() { let app_config = Arc::new(AppConfig::default()); @@ -945,6 +1007,7 @@ mod tests { functions: Functions::default(), }); let ctx = RequestContext::new(app_state, WorkingMode::Cmd); + let session = Session::new_from_ctx(&ctx, &app_config, "test-session").unwrap(); assert_eq!(session.name(), "test-session"); @@ -984,25 +1047,30 @@ mod tests { #[test] fn session_guard_empty_passes_when_empty() { let session = Session::default(); + assert!(session.guard_empty().is_ok()); } #[test] fn session_needs_compression_threshold() { let session = Session::default(); + assert!(!session.needs_compression(4000)); } #[test] fn session_needs_compression_returns_false_when_compressing() { let mut session = Session::default(); + session.set_compressing(true); + assert!(!session.needs_compression(0)); } #[test] fn session_needs_compression_returns_false_when_threshold_zero() { let session = Session::default(); + assert!(!session.needs_compression(0)); } @@ -1074,13 +1142,16 @@ mod tests { #[test] fn session_need_autoname_default_false() { let session = Session::default(); + assert!(!session.need_autoname()); } #[test] fn session_set_autonaming_doesnt_panic_without_autoname() { let mut session = Session::default(); + session.set_autonaming(true); + assert!(!session.need_autoname()); } diff --git a/src/graph/types.rs b/src/graph/types.rs index fcd3292..18bdf5d 100644 --- a/src/graph/types.rs +++ b/src/graph/types.rs @@ -588,6 +588,16 @@ nodes: )); } + #[test] + fn graph_silently_ignores_enabled_macros_key() { + let yaml = "name: g\nenabled_macros: [\"x\"]\nstart: x\nnodes:\n x:\n id: x\n type: end\n output: ok\n"; + + let graph: Graph = serde_yaml::from_str(yaml).unwrap(); + + assert_eq!(graph.name, "g"); + assert_eq!(graph.start, "x"); + } + #[test] fn graph_settings_have_sensible_defaults() { let yaml = "name: g\nstart: x\nnodes:\n x:\n id: x\n type: end\n output: ok\n"; diff --git a/src/main.rs b/src/main.rs index 937d255..68be6c7 100644 --- a/src/main.rs +++ b/src/main.rs @@ -222,6 +222,9 @@ async fn main() -> Result<()> { if cli.no_workspace_mcp { app_config.no_workspace_mcp = true; } + if cli.no_workspace_macros { + app_config.no_workspace_macros = true; + } let app_config: Arc = Arc::new(app_config); let app_state: Arc = Arc::new( AppState::init( diff --git a/src/repl/completer.rs b/src/repl/completer.rs index 880c789..440edbb 100644 --- a/src/repl/completer.rs +++ b/src/repl/completer.rs @@ -74,7 +74,25 @@ impl Completer for ReplCompleter { format!("{name} ") }; create_suggestion(&name, description, span) - })) + })); + + let macros: Vec<(String, Option)> = ctx + .visible_macro_completions() + .into_iter() + .map(|(name, description)| (format!(".{name}"), description)) + .filter(|(name, _)| { + command_filter.len() == 1 + || name.starts_with(command_filter.get(..2).unwrap_or(&command_filter)) + }) + .collect(); + let macros = fuzzy_filter(macros, |(name, _)| name.as_str(), &command_filter); + suggestions.extend(macros.iter().map(|(name, description)| { + create_suggestion( + &format!("{name} "), + description.as_deref().unwrap_or_default(), + span, + ) + })); } suggestions } diff --git a/src/repl/mod.rs b/src/repl/mod.rs index 47e95c0..6f7bbf2 100644 --- a/src/repl/mod.rs +++ b/src/repl/mod.rs @@ -12,8 +12,8 @@ use crate::client::{ oauth, }; use crate::config::{ - AgentVariables, AppConfig, AssertState, Input, LastMessage, RequestContext, StateFlags, - macro_execute, + AgentVariables, AppConfig, AssertState, Input, LastMessage, MacroState, RequestContext, + StateFlags, macro_execute, }; use crate::config::{AssetCategory, paths}; use crate::function::supervisor::{GuardrailAction, check_pending_agents_guardrail}; @@ -1073,15 +1073,44 @@ pub async fn run_repl_command( } }, ".macro" => match split_first_arg(args) { - Some((name, extra)) => { - let app = Arc::clone(&ctx.app.config); - if !paths::has_macro(name) && extra.is_none() { - ctx.new_macro(app.as_ref(), name)?; - } else { - macro_execute(ctx, name, extra, abort_signal.clone()).await?; + Some((sub @ ("enable" | "disable"), rest)) => { + match rest.and_then(|v| v.split_whitespace().next()) { + Some(name) => ctx.macro_toggle(name, sub == "enable")?, + None => println!("Usage: .macro {sub} "), } } - None => println!("Usage: .macro ..."), + Some((name, extra)) => { + let policy = ctx.macro_policy(); + match policy.find(name).map(|row| &row.state) { + Some(state) if state.is_invocable() => { + macro_execute(ctx, name, extra, abort_signal.clone()).await?; + } + Some(MacroState::DisabledRuntime) => bail!( + r#"Macro '{name}' is disabled. Re-enable it with ".macro enable {name}""# + ), + Some(MacroState::Locked { level }) => bail!( + "Macro '{name}' is restricted by {} enabled_macros", + ctx.macro_lock_owner(*level) + ), + Some(MacroState::Invalid { reason }) => { + bail!("Macro '{name}' is invalid: {reason}") + } + Some(_) | None => { + if extra.is_none() { + let app = Arc::clone(&ctx.app.config); + ctx.new_macro(app.as_ref(), name)?; + } else { + macro_execute(ctx, name, extra, abort_signal.clone()).await?; + } + } + } + } + None => println!( + r#"Usage: + .macro [text]... # Execute a macro + .macro enable # Re-enable a runtime-disabled macro + .macro disable # Disable a macro for the rest of this process"# + ), }, ".file" => match args { Some(args) => { @@ -1294,7 +1323,26 @@ pub async fn run_repl_command( println!("Usage: .vault [name]") } }, - _ => unknown_command()?, + _ => { + let name = cmd.strip_prefix('.').unwrap_or(cmd); + let policy = ctx.macro_policy(); + match policy.find(name).map(|row| &row.state) { + Some(MacroState::Enabled) => { + macro_execute(ctx, name, args, abort_signal.clone()).await?; + } + Some(MacroState::DisabledRuntime) => bail!( + r#"Macro '{name}' is disabled. Re-enable it with ".macro enable {name}""# + ), + Some(MacroState::Locked { level }) => bail!( + "Macro '{name}' is restricted by {} enabled_macros", + ctx.macro_lock_owner(*level) + ), + Some(MacroState::Invalid { reason }) => { + bail!("Macro '{name}' is invalid: {reason}") + } + _ => unknown_command()?, + } + } }, None => { if let Some(cmd) = try_extract_shell_command(line) { @@ -1522,6 +1570,17 @@ fn unknown_command() -> Result<()> { bail!(r#"Unknown command. Type ".help" for additional help."#); } +pub fn builtin_command_names() -> Vec<&'static str> { + let mut names: Vec<&'static str> = REPL_COMMANDS + .iter() + .filter_map(|cmd| cmd.name.split_whitespace().next()) + .filter_map(|name| name.strip_prefix('.')) + .collect(); + names.sort_unstable(); + names.dedup(); + names +} + fn dump_repl_help() { let head = REPL_COMMANDS .iter() @@ -1532,6 +1591,10 @@ fn dump_repl_help() { r###"{head} {:<24} Run an arbitrary shell command (stdout/stderr stream to your terminal; Ctrl+C interrupts) +Custom commands (macros): macros are coyote's custom commands. An enabled +macro runs top-level as . [args...], equivalent to ".macro ". +List them with ".list macros"; toggle them with ".macro enable|disable ". + Type ::: to start multi-line editing, type ::: to finish it. Press Ctrl+O to open an editor for editing the input buffer. Press Ctrl+C to cancel the response, Ctrl+D to exit the REPL."###, @@ -1732,6 +1795,22 @@ mod tests { assert_eq!(REPL_COMMANDS.len(), 60); } + #[test] + fn builtin_command_names_are_sorted_deduped_first_words_without_dots() { + let names = builtin_command_names(); + assert!(!names.is_empty()); + for name in &names { + assert!(!name.starts_with('.'), "'{name}' should not keep the dot"); + assert!(!name.contains(' '), "'{name}' should be a single word"); + } + assert!( + names.windows(2).all(|w| w[0] < w[1]), + "names should be sorted and deduplicated: {names:?}" + ); + assert!(names.contains(&"help")); + assert!(names.contains(&"macro")); + } + #[test] fn repl_commands_all_start_with_dot() { for cmd in REPL_COMMANDS.iter() {