From 8c5bed3e341d76c93335492207a5946e244d1eeb Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Wed, 3 Jun 2026 14:54:42 -0600 Subject: [PATCH] fix: effective_policy unconditionally overwrote skill values for role-like structs --- src/config/request_context.rs | 10 +++++++++- src/config/skill_registry.rs | 36 +++++++++++++++++++++++------------ src/graph/llm.rs | 12 ++++++++++-- 3 files changed, 43 insertions(+), 15 deletions(-) diff --git a/src/config/request_context.rs b/src/config/request_context.rs index 98f76c6..4d6af34 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -627,7 +627,15 @@ impl RequestContext { } } - self.skill_registry.effective_role(&role) + match SkillPolicy::effective( + app, + self.role.as_ref(), + self.agent.as_ref(), + self.session.as_ref(), + ) { + Ok(policy) => self.skill_registry.effective_role(&role, &policy), + Err(_) => role, + } } pub fn auto_continue_config(&self) -> AutoContinueConfig { diff --git a/src/config/skill_registry.rs b/src/config/skill_registry.rs index 71703a5..2a58df8 100644 --- a/src/config/skill_registry.rs +++ b/src/config/skill_registry.rs @@ -1,5 +1,6 @@ use super::role::{Role, RoleLike}; use super::skill::Skill; +use super::skill_policy::SkillPolicy; use anyhow::{Result, bail}; use indexmap::IndexMap; @@ -58,8 +59,8 @@ impl SkillRegistry { self.loaded.retain(|_, skill| !skill.auto_unload()); } - pub fn effective_role(&self, base: &Role) -> Role { - if self.loaded.is_empty() { + pub fn effective_role(&self, base: &Role, policy: &SkillPolicy) -> Role { + if !policy.skills_enabled || self.loaded.is_empty() { return base.clone(); } @@ -78,7 +79,10 @@ impl SkillRegistry { .map(|v| v.into_iter().collect()) .unwrap_or_default(); - for (_, skill) in &self.loaded { + for (name, skill) in &self.loaded { + if !policy.allows(name) { + continue; + } if let Some(skill_tools) = skill.enabled_tools() { tools.extend(skill_tools.iter().cloned()); } @@ -113,6 +117,14 @@ impl SkillRegistry { fn insert_for_test(&mut self, skill: Skill) { self.loaded.insert(skill.name().to_string(), skill); } + + fn effective_role_for_test(&self, base: &Role) -> Role { + let policy = SkillPolicy { + skills_enabled: true, + enabled: self.loaded.keys().cloned().collect(), + }; + self.effective_role(base, &policy) + } } #[cfg(test)] @@ -133,7 +145,7 @@ mod tests { let base = Role::new("test", "You are a helper"); let registry = SkillRegistry::default(); - let effective = registry.effective_role(&base); + let effective = registry.effective_role_for_test(&base); assert_eq!(effective.prompt(), base.prompt()); } @@ -144,7 +156,7 @@ mod tests { registry.insert_for_test(make_skill("git-master", "description: D", "Git knowledge")); let base = Role::new("test", "You are a helper"); - let effective = registry.effective_role(&base); + let effective = registry.effective_role_for_test(&base); assert_eq!(effective.prompt(), "You are a helper\n\nGit knowledge"); } @@ -156,7 +168,7 @@ mod tests { registry.insert_for_test(make_skill("b", "", "Beta body")); let base = Role::new("test", "Base"); - let effective = registry.effective_role(&base); + let effective = registry.effective_role_for_test(&base); assert_eq!(effective.prompt(), "Base\n\nAlpha body\n\nBeta body"); } @@ -168,7 +180,7 @@ mod tests { registry.insert_for_test(make_skill("b", "", "Beta")); let base = Role::new("test", ""); - let effective = registry.effective_role(&base); + let effective = registry.effective_role_for_test(&base); assert_eq!(effective.prompt(), "Alpha\n\nBeta"); } @@ -183,7 +195,7 @@ mod tests { )); let base = Role::new("test", "Process: __INPUT__"); - let effective = registry.effective_role(&base); + let effective = registry.effective_role_for_test(&base); assert_eq!(effective.prompt(), "Process: __INPUT__"); let tools = effective.enabled_tools().expect("tools set by skill"); @@ -196,7 +208,7 @@ mod tests { registry.insert_for_test(make_skill("knowledge", "enabled_tools: fs", "")); let base = Role::new("test", "Base"); - let effective = registry.effective_role(&base); + let effective = registry.effective_role_for_test(&base); assert_eq!(effective.prompt(), "Base"); } @@ -218,7 +230,7 @@ mod tests { let mut base = Role::new("test", "body"); base.set_enabled_tools(Some(vec!["web_search".to_string()])); - let effective = registry.effective_role(&base); + let effective = registry.effective_role_for_test(&base); let tools_vec = effective.enabled_tools().unwrap(); let tools: BTreeSet<&str> = tools_vec.iter().map(|s| s.as_str()).collect(); @@ -235,7 +247,7 @@ mod tests { registry.insert_for_test(make_skill("knowledge", "", "Pure knowledge")); let base = Role::new("test", "Base"); - let effective = registry.effective_role(&base); + let effective = registry.effective_role_for_test(&base); assert!(effective.enabled_tools().is_none()); assert!(effective.enabled_mcp_servers().is_none()); @@ -248,7 +260,7 @@ mod tests { let mut base = Role::new("test", "Base"); base.set_enabled_tools(Some(Vec::new())); - let effective = registry.effective_role(&base); + let effective = registry.effective_role_for_test(&base); assert_eq!(effective.enabled_tools().as_deref(), Some([].as_slice())); } diff --git a/src/graph/llm.rs b/src/graph/llm.rs index d8e31b4..6aa9e6e 100644 --- a/src/graph/llm.rs +++ b/src/graph/llm.rs @@ -2,7 +2,7 @@ use super::state::StateManager; use super::structured; use super::types::LlmNode; use crate::client::{Model, ModelType, call_chat_completions}; -use crate::config::{Input, RequestContext, Role, RoleLike}; +use crate::config::{Input, RequestContext, Role, RoleLike, SkillPolicy}; use crate::utils::create_abort_signal; use anyhow::{Context, Error, Result, anyhow, bail}; use serde_json::Value; @@ -115,7 +115,15 @@ async fn run( let saved_agent_skill_state = swap_in_node_skill_policy(node, parent_ctx); - let composed_role = parent_ctx.skill_registry.effective_role(&role); + let composed_role = match SkillPolicy::effective( + &parent_ctx.app.config, + parent_ctx.role.as_ref(), + parent_ctx.agent.as_ref(), + parent_ctx.session.as_ref(), + ) { + Ok(policy) => parent_ctx.skill_registry.effective_role(&role, &policy), + Err(_) => role, + }; let saved_role = parent_ctx.role.clone(); parent_ctx.role = Some(composed_role);