diff --git a/graph.example.yaml b/graph.example.yaml index ece3cda..f1ea9ae 100644 --- a/graph.example.yaml +++ b/graph.example.yaml @@ -68,9 +68,6 @@ enabled_skills: inject_skill_instructions: true # Inject a hint pointing the model at `skill__list`. Defaults to true; suppressed # automatically when no skills are available. skill_instructions: null # Custom text for the skill hint (optional; uses the built-in default if omitted). -# Note: `enabled_macros` is NOT a graph setting. Graph nodes never dispatch REPL -# commands, so the field is silently ignored in graph configs; scope macros via -# the global config, roles, agents, or sessions instead. conversation_starters: # Suggested prompts surfaced in the UI - "Research the current state of WebAssembly outside the browser" diff --git a/src/config/macro_policy.rs b/src/config/macro_policy.rs index 8dba677..6636778 100644 --- a/src/config/macro_policy.rs +++ b/src/config/macro_policy.rs @@ -11,11 +11,8 @@ use std::fmt; use std::fs::{read_dir, read_to_string}; use std::path::PathBuf; -/// Names that cannot be used as macro names because they are reserved for -/// `.macro enable ` / `.macro disable `. pub const RESERVED_MACRO_NAMES: [&str; 2] = ["enable", "disable"]; -/// Where a macro definition file was discovered. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum MacroSource { Workspace, @@ -52,25 +49,13 @@ impl fmt::Display for MacroAllowlistLevel { } } -/// The effective state of a macro within the active context. #[derive(Debug, Clone, PartialEq, Eq)] pub enum MacroState { - /// Visible and invocable. Enabled, - /// Excluded by the GLOBAL-level `enabled_macros` list (config file or - /// runtime toggle — the two are indistinguishable by design). Can be - /// re-enabled at runtime with `.macro enable `. DisabledRuntime, - /// Excluded by a role/agent/session `enabled_macros` allowlist. Not - /// enableable from the REPL; the owning config is the source of truth. - /// `level` is never `Global` — a global exclusion is `DisabledRuntime`. Locked { level: MacroAllowlistLevel }, - /// Named by the effective allowlist, but no such macro is installed. Missing, - /// The macro name collides with a built-in REPL command; invocable only - /// via `.macro `, never as a top-level `.name` command. ShadowedBuiltin, - /// The definition file failed to parse, or the name is reserved. Invalid { reason: String }, } @@ -80,34 +65,24 @@ impl MacroState { } } -/// A macro definition file found on disk, before allowlist resolution. #[derive(Debug, Clone)] pub struct DiscoveredMacro { pub name: String, pub source: MacroSource, - /// The parsed definition, or the parse failure reason. pub definition: Result, - /// True for a GLOBAL entry whose name is shadowed by a workspace entry. pub shadowed_by_workspace: bool, } -/// One row of the resolved macro set. Missing allowlist entries produce rows -/// with `source: None`. #[derive(Debug, Clone)] pub struct ResolvedMacro { pub name: String, pub source: Option, pub description: Option, pub isolated: Option, - /// True for a global entry shadowed by a workspace entry of the same - /// name; the row is kept for display but is never the invocation target. pub shadowed_by_workspace: bool, pub state: MacroState, } -/// The visible macro set for the active context, resolved lazily on demand -/// from the discovered definition files and the effective `enabled_macros` -/// allowlist. #[derive(Debug)] pub struct MacroPolicy { pub macros: Vec, @@ -181,6 +156,7 @@ impl MacroPolicy { }); } } + macros.extend(missing); } @@ -193,9 +169,6 @@ impl MacroPolicy { Self { macros } } - /// The invocation target for `name`: the workspace entry when one shadows - /// a global entry, otherwise the single discovered entry. Missing rows - /// are never returned. pub fn find(&self, name: &str) -> Option<&ResolvedMacro> { self.macros .iter() @@ -221,6 +194,7 @@ fn resolve_state( "Ignoring macro '{}': the name is reserved for '.macro {}'", discovered.name, discovered.name ); + return MacroState::Invalid { reason: format!("'{}' is a reserved macro name", discovered.name), }; @@ -248,14 +222,12 @@ fn resolve_state( MacroState::Enabled } -/// Rescans the workspace and global macro directories. Workspace entries -/// shadow global entries of the same name; the shadowed global entry is kept -/// and flagged so both remain listable. 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) } @@ -270,14 +242,17 @@ fn discover_macros_in(dirs: &[(MacroSource, PathBuf)]) -> Vec { }; 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() @@ -286,9 +261,11 @@ fn discover_macros_in(dirs: &[(MacroSource, PathBuf)]) -> Vec { else { continue; }; + if name.is_empty() { continue; } + let definition = read_to_string(entry.path()) .map_err(|err| err.to_string()) .and_then(|content| { @@ -372,6 +349,7 @@ mod tests { #[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); } @@ -380,6 +358,7 @@ mod tests { 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); } @@ -388,6 +367,7 @@ mod tests { 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); } @@ -395,7 +375,9 @@ mod tests { #[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"), @@ -408,7 +390,9 @@ mod tests { #[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 { @@ -420,7 +404,9 @@ mod tests { #[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 { @@ -432,7 +418,9 @@ mod tests { #[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 { @@ -444,7 +432,9 @@ mod tests { #[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 { @@ -456,7 +446,9 @@ mod tests { #[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 { @@ -469,6 +461,7 @@ mod tests { fn session_wins_over_agent() { let session = list(&["a"]); let agent = list(&["b"]); + let policy = resolve( globals(&["a", "b"]), Some(&session), @@ -476,6 +469,7 @@ mod tests { None, None, ); + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); assert_eq!( state_of(&policy, "b"), @@ -489,7 +483,9 @@ mod tests { 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"), @@ -503,7 +499,9 @@ mod tests { 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"), @@ -517,7 +515,9 @@ mod tests { 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 { @@ -529,7 +529,9 @@ mod tests { #[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(); @@ -539,7 +541,9 @@ mod tests { #[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); } @@ -547,6 +551,7 @@ mod tests { #[test] fn no_missing_rows_without_an_allowlist() { let policy = resolve(globals(&["a"]), None, None, None, None); + assert_eq!(policy.macros.len(), 1); } @@ -554,6 +559,7 @@ mod tests { 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); } @@ -561,6 +567,7 @@ mod tests { #[test] fn locked_wins_over_shadowed_builtin() { let l = list(&["a"]); + let policy = MacroPolicy::effective_with( globals(&["help", "a"]), Some(&l), @@ -569,6 +576,7 @@ mod tests { None, &["help"], ); + assert_eq!( state_of(&policy, "help"), &MacroState::Locked { @@ -580,8 +588,10 @@ mod tests { #[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); } @@ -601,7 +611,9 @@ mod tests { #[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 { @@ -614,6 +626,7 @@ mod tests { 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 { @@ -625,6 +638,7 @@ mod tests { #[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 { @@ -639,6 +653,7 @@ mod tests { #[test] fn invalid_wins_over_allowlist_exclusion() { let l = list(&["other"]); + let policy = resolve( vec![disc_invalid("bad", "boom")], Some(&l), @@ -646,6 +661,7 @@ mod tests { None, None, ); + assert_eq!( state_of(&policy, "bad"), &MacroState::Invalid { @@ -663,7 +679,9 @@ mod tests { ..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); @@ -676,13 +694,16 @@ mod tests { #[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"]); } @@ -690,6 +711,7 @@ mod tests { #[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)); diff --git a/src/config/macros.rs b/src/config/macros.rs index 667a020..93d9806 100644 --- a/src/config/macros.rs +++ b/src/config/macros.rs @@ -41,6 +41,7 @@ pub async fn macro_execute( println!(">> {}", multiline_text(&command)); run_repl_command(&mut live, abort_signal.clone(), &command).await?; } + return Ok(()); } @@ -84,9 +85,6 @@ pub async fn macro_execute( Ok(()) } -/// Marks a live context as executing a non-isolated macro for the duration of -/// its steps. Restores the previous flag and mode on drop, so every exit path -/// — including a failing step — leaves the context as it found it. struct MacroModeGuard<'a> { ctx: &'a mut RequestContext, prev_flag: bool, @@ -223,9 +221,11 @@ 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 } } @@ -416,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")); } @@ -438,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"); } @@ -462,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"); } @@ -473,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"); @@ -482,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]..."); } @@ -515,6 +536,7 @@ mod tests { var("target", false, None), var("flags", true, Some("")), ]); + assert_eq!(m.usage("build"), "build [flags]..."); } @@ -522,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"); } @@ -532,6 +555,7 @@ mod tests { ("b".to_string(), "2".to_string()), ]); let result = Macro::interpolate_command("{{a}} + {{b}}", &vars); + assert_eq!(result, "1 + 2"); } @@ -539,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"); } @@ -546,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}}"); } @@ -578,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())); @@ -590,7 +618,9 @@ variables: steps: - ".help" "#; + let m: Macro = serde_yaml::from_str(yaml).unwrap(); + assert!(m.variables.is_empty()); assert_eq!(m.steps.len(), 1); } @@ -601,7 +631,9 @@ steps: steps: - ".help" "#; + let m: Macro = serde_yaml::from_str(yaml).unwrap(); + assert!(m.description.is_none()); assert!(m.isolated); } @@ -617,7 +649,9 @@ 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") @@ -634,8 +668,10 @@ variables: 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); @@ -648,6 +684,7 @@ variables: 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()); @@ -659,8 +696,10 @@ variables: for file in MacroAssets::iter() { let embedded = MacroAssets::get(&file).unwrap(); let content = std::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.description.is_none(), "asset '{}'", file.as_ref()); assert!(m.isolated, "asset '{}'", file.as_ref()); assert!(!m.steps.is_empty(), "asset '{}'", file.as_ref()); diff --git a/src/config/mod.rs b/src/config/mod.rs index f36cc99..ea90e76 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -1140,12 +1140,14 @@ clients: #[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()]) @@ -1155,6 +1157,7 @@ clients: #[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()]) @@ -1164,12 +1167,14 @@ clients: #[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)); @@ -1179,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)); @@ -1187,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())); @@ -1196,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)); @@ -1206,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)); @@ -1215,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)); @@ -1225,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())); @@ -1235,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/request_context.rs b/src/config/request_context.rs index 4762e9c..be8daf2 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -124,8 +124,6 @@ fn complete_skills_with_descriptions(names: Vec) -> Vec<(String, Option< .collect() } -/// Keys offered by `.set ` completion. `reasoning_effort` is appended at -/// completion time only when the current model supports reasoning levels. const SET_COMPLETION_KEYS: [&str; 26] = [ "auto_continue", "continuation_prompt", @@ -155,10 +153,6 @@ const SET_COMPLETION_KEYS: [&str; 26] = [ "raw_markdown", ]; -/// The new global-level `enabled_macros` list after toggling `name`, or -/// `None` when the toggle is a no-op (already in the requested state). -/// Disabling with no current list (all macros visible) materializes the list -/// as every active macro name minus `name`. fn toggled_enabled_macros( current: Option<&[String]>, all_active: &[String], @@ -198,7 +192,6 @@ fn toggled_enabled_macros( } } -/// The `.list macros` state column for a resolved row. fn macro_state_display( row: &ResolvedMacro, lock_owner: impl Fn(MacroAllowlistLevel) -> String, @@ -213,8 +206,6 @@ fn macro_state_display( } } -/// The `.list macros` source column: where the definition file lives, or `-` -/// for allowlist entries with no installed file. fn macro_source_display(source: Option) -> String { match source { Some(source) => source.to_string(), @@ -233,9 +224,6 @@ pub struct RequestContext { pub app: Arc, pub macro_flag: bool, - /// Companion to `macro_flag`: true while a non-isolated macro is running - /// its steps directly on this live context. Isolated macros execute on a - /// forked context and leave this false. pub macro_non_isolated: bool, pub info_flag: bool, pub working_mode: WorkingMode, @@ -2488,16 +2476,10 @@ impl RequestContext { Ok(()) } - /// Whether a non-isolated macro is currently running its steps on this - /// context. Macro invocations are rejected in this mode: the nested - /// macro's steps would interleave with the outer macro's on the live - /// session. pub fn in_non_isolated_macro(&self) -> bool { self.macro_flag && self.macro_non_isolated } - /// The resolved macro set for the active context (workspace + global - /// discovery, effective `enabled_macros` allowlist, built-in shadowing). pub fn macro_policy(&self) -> MacroPolicy { MacroPolicy::effective( &self.app.config, @@ -2509,8 +2491,6 @@ impl RequestContext { ) } - /// A human-readable name for the config level whose `enabled_macros` - /// allowlist restricts a macro, e.g. `agent:oracle` or `role:coder`. pub fn macro_lock_owner(&self, level: MacroAllowlistLevel) -> String { let name = match level { MacroAllowlistLevel::Session => self.session.as_ref().map(|s| s.name()), @@ -2524,9 +2504,6 @@ impl RequestContext { } } - /// Enables or disables a macro by editing the in-memory global-level - /// `enabled_macros` list. Errors when a role/agent/session allowlist is - /// active, since a global-level write would be silently shadowed. pub fn macro_toggle(&mut self, name: &str, enable: bool) -> Result<()> { let restricting_level = if self .session @@ -2577,6 +2554,7 @@ impl RequestContext { .collect(); let action = if enable { "enabled" } else { "disabled" }; + match toggled_enabled_macros( self.app.config.enabled_macros.as_deref(), &all_active, @@ -2589,12 +2567,10 @@ impl RequestContext { } None => println!("Macro '{name}' is already {action}"), } + Ok(()) } - /// The macros offered by top-level `.` completion: enabled rows - /// only. Macros shadowed by a built-in command never appear here; they - /// stay reachable via `.macro `. pub fn visible_macro_completions(&self) -> Vec<(String, Option)> { self.macro_policy() .macros @@ -2622,6 +2598,7 @@ impl RequestContext { "name", "source", "isolated", "state", "description" ); println!("{header}"); + for row in &policy.macros { let source = macro_source_display(row.source); let isolated = match row.isolated { @@ -4032,7 +4009,7 @@ 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 + // 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(|| { diff --git a/src/config/role.rs b/src/config/role.rs index 213f848..db82655 100644 --- a/src/config/role.rs +++ b/src/config/role.rs @@ -558,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"); } @@ -566,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)); @@ -576,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()]) @@ -586,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()]) @@ -596,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); @@ -606,18 +614,21 @@ mod tests { #[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()]) @@ -627,6 +638,7 @@ mod tests { #[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()]) @@ -636,25 +648,30 @@ mod tests { #[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()); } @@ -662,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()); } @@ -669,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); @@ -684,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)); } @@ -699,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")); } @@ -713,6 +739,7 @@ Input 1 ### OUTPUT: Output 1 "#; + assert_eq!( parse_structure_prompt(prompt), ("System message", vec![("Input 1", "Output 1")]) @@ -727,6 +754,7 @@ Input 1 ### OUTPUT: Output 1 "#; + assert_eq!( parse_structure_prompt(prompt), ("", vec![("Input 1", "Output 1")]) @@ -740,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 728b100..1e3b042 100644 --- a/src/config/session.rs +++ b/src/config/session.rs @@ -942,6 +942,7 @@ 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); @@ -951,6 +952,7 @@ mod tests { #[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); } @@ -958,6 +960,7 @@ mod tests { 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![])); } @@ -966,6 +969,7 @@ mod tests { let session: Session = serde_yaml::from_str("model: provider:test\nenabled_macros: \"\"\nmessages: []") .unwrap(); + assert_eq!(session.enabled_macros, Some(vec![])); } @@ -974,6 +978,7 @@ mod tests { 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()]) @@ -984,6 +989,7 @@ mod tests { 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")); } @@ -1001,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"); @@ -1040,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)); } @@ -1130,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 8e13538..18bdf5d 100644 --- a/src/graph/types.rs +++ b/src/graph/types.rs @@ -591,7 +591,9 @@ 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"); } diff --git a/src/repl/mod.rs b/src/repl/mod.rs index 21bd991..77e9233 100644 --- a/src/repl/mod.rs +++ b/src/repl/mod.rs @@ -1086,7 +1086,7 @@ pub async fn run_repl_command( 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}""# + r#"Macro '{name}' is disabled. Enable it with ".macro enable {name}""# ), Some(MacroState::Locked { level }) => bail!( "Macro '{name}' is restricted by {} enabled_macros", @@ -1567,9 +1567,6 @@ fn unknown_command() -> Result<()> { bail!(r#"Unknown command. Type ".help" for additional help."#); } -/// The name of every built-in REPL command (first word, without the leading -/// dot), sorted and deduplicated. Macros with one of these names are shadowed -/// by the built-in and stay reachable only via `.macro `. pub fn builtin_command_names() -> Vec<&'static str> { let mut names: Vec<&'static str> = REPL_COMMANDS .iter()