fix(core): relax active skill tool restrictions by default (#72)

* fix(core): relax active skill tool restrictions by default

* docs: note active skill restriction default change

---------

Co-authored-by: Jinxuan Zhang <aspdelus@JinxuandeMac-mini.local>

Author: Aspdelus

CHANGELOG.md

@@ -5,6 +5,16 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [4.1.0] - Unreleased
+
+### Changed
+
+- Active skill `allowed-tools` no longer globally deny ordinary session tool calls
+  by default. Tool calls continue through permission policy, hooks, HITL, and AHP;
+  `SessionOptions::with_active_skill_tool_restrictions(true)` (Node:
+  `enforceActiveSkillToolRestrictions`) restores the legacy global restriction
+  behavior. Skill-local execution still enforces each skill's `allowed-tools`.
+
 ## [4.0.0] - 2026-06-21
 
 Milestone release: **filesystem-first agents**. A single directory

core/src/agent.rs

@@ -92,6 +92,12 @@ pub(crate) struct AgentConfig {
     pub hook_engine: Option<Arc<dyn HookExecutor>>,
     /// Optional skill registry for tool permission enforcement
     pub skill_registry: Option<Arc<crate::skills::SkillRegistry>>,
+    /// When true, active skill `allowed-tools` restrict ordinary session tool calls.
+    ///
+    /// The default is false: active skills may inject instructions, but ordinary
+    /// tool calls continue to the host permission/AHP/HITL approval chain.
+    /// Skill invocations still enable this for their child execution context.
+    pub enforce_active_skill_tool_restrictions: bool,
     /// Max consecutive malformed-tool-args errors before aborting (default: 2).
     ///
     /// When the LLM returns tool arguments with `__parse_error`, the error is
@@ -181,6 +187,10 @@ impl std::fmt::Debug for AgentConfig {
                 "skill_registry",
                 &self.skill_registry.as_ref().map(|r| r.len()),
             )
+            .field(
+                "enforce_active_skill_tool_restrictions",
+                &self.enforce_active_skill_tool_restrictions,
+            )
             .field("max_parse_retries", &self.max_parse_retries)
             .field("tool_timeout_ms", &self.tool_timeout_ms)
             .field("max_parallel_tasks", &self.max_parallel_tasks)
@@ -221,6 +231,7 @@ impl Default for AgentConfig {
             goal_tracking: false,
             hook_engine: None,
             skill_registry: Some(Arc::new(crate::skills::SkillRegistry::with_builtins())),
+            enforce_active_skill_tool_restrictions: false,
             max_parse_retries: 2,
             tool_timeout_ms: None,
             max_parallel_tasks: DEFAULT_MAX_PARALLEL_TASKS,
@@ -695,6 +706,7 @@ pub struct ToolCommand {
     tool_args: Value,
     tool_context: ToolContext,
     skill_registry: Option<Arc<crate::skills::SkillRegistry>>,
+    enforce_active_skill_tool_restrictions: bool,
 }
 
 impl ToolCommand {
@@ -705,13 +717,15 @@ impl ToolCommand {
         tool_args: Value,
         tool_context: ToolContext,
         skill_registry: Option<Arc<crate::skills::SkillRegistry>>,
+        enforce_active_skill_tool_restrictions: bool,
     ) -> Self {
         Self {
             tool_executor,
             tool_name,
             tool_args,
             tool_context,
             skill_registry,
+            enforce_active_skill_tool_restrictions,
         }
     }
 }
@@ -720,25 +734,27 @@ impl ToolCommand {
 impl SessionCommand for ToolCommand {
     async fn execute(&self) -> Result<Value> {
         // Check skill-based tool permissions
-        if let Some(registry) = &self.skill_registry {
-            // If there are instruction skills with tool restrictions, check permissions
-            let restricting_skills = registry.global_tool_restricting_skills();
-
-            if !restricting_skills.is_empty() {
-                let mut allowed = false;
-
-                for skill in &restricting_skills {
-                    if skill.is_tool_allowed(&self.tool_name) {
-                        allowed = true;
-                        break;
+        if self.enforce_active_skill_tool_restrictions {
+            if let Some(registry) = &self.skill_registry {
+                // If there are instruction skills with tool restrictions, check permissions
+                let restricting_skills = registry.global_tool_restricting_skills();
+
+                if !restricting_skills.is_empty() {
+                    let mut allowed = false;
+
+                    for skill in &restricting_skills {
+                        if skill.is_tool_allowed(&self.tool_name) {
+                            allowed = true;
+                            break;
+                        }
                     }
-                }
 
-                if !allowed {
-                    return Err(anyhow::anyhow!(
+                    if !allowed {
+                        return Err(anyhow::anyhow!(
                         "Tool '{}' is not allowed by any active skill. Active skills restrict tools to their allowed-tools lists.",
                         self.tool_name
                     ));
+                    }
                 }
             }
         }

core/src/agent/extra_agent_tests.rs

@@ -827,6 +827,7 @@ async fn test_tool_command_command_type() {
         tool_name: "read".to_string(),
         tool_args: serde_json::json!({"file": "test.rs"}),
         skill_registry: None,
+        enforce_active_skill_tool_restrictions: false,
         tool_context: test_tool_context(),
     };
     assert_eq!(cmd.command_type(), "read");
@@ -841,11 +842,77 @@ async fn test_tool_command_payload() {
         tool_name: "read".to_string(),
         tool_args: args.clone(),
         skill_registry: None,
+        enforce_active_skill_tool_restrictions: false,
         tool_context: test_tool_context(),
     };
     assert_eq!(cmd.payload(), args);
 }
 
+#[tokio::test]
+async fn test_tool_command_ignores_active_skill_restrictions_by_default() {
+    let registry = crate::skills::SkillRegistry::new();
+    registry.register_unchecked(Arc::new(crate::skills::Skill {
+        name: "read-only".to_string(),
+        description: String::new(),
+        allowed_tools: Some("read(*)".to_string()),
+        disable_model_invocation: false,
+        kind: crate::skills::SkillKind::Instruction,
+        content: String::new(),
+        tags: Vec::new(),
+        version: None,
+    }));
+
+    let cmd = ToolCommand {
+        tool_executor: Arc::new(ToolExecutor::new("/tmp".to_string())),
+        tool_name: "not_a_tool".to_string(),
+        tool_args: serde_json::json!({}),
+        skill_registry: Some(Arc::new(registry)),
+        enforce_active_skill_tool_restrictions: false,
+        tool_context: test_tool_context(),
+    };
+
+    let output = cmd.execute().await.unwrap();
+    let output = output["output"].as_str().unwrap_or_default();
+    assert!(
+        !output.contains("not allowed by any active skill"),
+        "default mode must let the command reach the tool executor, got: {output}"
+    );
+    assert!(
+        output.contains("Unknown tool"),
+        "default mode should reach the tool executor, got: {output}"
+    );
+}
+
+#[tokio::test]
+async fn test_tool_command_enforces_active_skill_restrictions_in_legacy_mode() {
+    let registry = crate::skills::SkillRegistry::new();
+    registry.register_unchecked(Arc::new(crate::skills::Skill {
+        name: "read-only".to_string(),
+        description: String::new(),
+        allowed_tools: Some("read(*)".to_string()),
+        disable_model_invocation: false,
+        kind: crate::skills::SkillKind::Instruction,
+        content: String::new(),
+        tags: Vec::new(),
+        version: None,
+    }));
+
+    let cmd = ToolCommand {
+        tool_executor: Arc::new(ToolExecutor::new("/tmp".to_string())),
+        tool_name: "not_a_tool".to_string(),
+        tool_args: serde_json::json!({}),
+        skill_registry: Some(Arc::new(registry)),
+        enforce_active_skill_tool_restrictions: true,
+        tool_context: test_tool_context(),
+    };
+
+    let err = cmd.execute().await.unwrap_err().to_string();
+    assert!(
+        err.contains("not allowed by any active skill"),
+        "legacy mode must deny before tool execution, got: {err}"
+    );
+}
+
 // ========================================================================
 // AgentLoop with queue builder tests
 // ========================================================================

core/src/agent/tests.rs

@@ -2032,11 +2032,13 @@ fn parallel_write_batch_only_fast_paths_when_gate_would_execute() {
     fn loop_with(
         checker: Option<Arc<dyn PermissionChecker>>,
         skills: Option<Arc<SkillRegistry>>,
+        enforce_active_skill_tool_restrictions: bool,
     ) -> AgentLoop {
         // `skill_registry` overrides the default builtins where needed.
         let config = AgentConfig {
             permission_checker: checker,
             skill_registry: skills,
+            enforce_active_skill_tool_restrictions,
             ..Default::default()
         };
         AgentLoop::new(
@@ -2052,32 +2054,37 @@ fn parallel_write_batch_only_fast_paths_when_gate_would_execute() {
 
     // Explicit Allow + no restricting skills → fast path is taken.
     assert!(
-        loop_with(allow(), None).can_run_parallel_write_batch(&calls),
+        loop_with(allow(), None, false).can_run_parallel_write_batch(&calls),
         "explicit Allow with no restrictions → parallel write batch is allowed"
     );
 
     // No permission checker → gate resolves to Ask (a Deny without a confirmation
     // manager), so the fast path must be refused.
     assert!(
-        !loop_with(None, None).can_run_parallel_write_batch(&calls),
+        !loop_with(None, None, false).can_run_parallel_write_batch(&calls),
         "missing checker resolves to Ask/Deny → fast path refused"
     );
 
     // Explicit Deny → refused.
     assert!(
-        !loop_with(Some(Arc::new(Static(PermissionDecision::Deny))), None)
-            .can_run_parallel_write_batch(&calls),
+        !loop_with(
+            Some(Arc::new(Static(PermissionDecision::Deny))),
+            None,
+            false
+        )
+        .can_run_parallel_write_batch(&calls),
         "permission Deny → fast path refused"
     );
 
     // Ask → refused (sequential path would need a human round-trip).
     assert!(
-        !loop_with(Some(Arc::new(Static(PermissionDecision::Ask))), None)
+        !loop_with(Some(Arc::new(Static(PermissionDecision::Ask))), None, false)
             .can_run_parallel_write_batch(&calls),
         "permission Ask → fast path refused"
     );
 
-    // Allow, but an active skill restriction forbids write_file → refused.
+    // By default, active skill restrictions do not block ordinary session
+    // tools before the permission/AHP/HITL chain.
     let restricted = SkillRegistry::new();
     restricted.register_unchecked(Arc::new(Skill {
         name: "read-only".to_string(),
@@ -2089,15 +2096,27 @@ fn parallel_write_batch_only_fast_paths_when_gate_would_execute() {
         tags: Vec::new(),
         version: None,
     }));
+    let restricted = Arc::new(restricted);
     assert!(
-        !loop_with(allow(), Some(Arc::new(restricted))).can_run_parallel_write_batch(&calls),
-        "active skill restriction disallowing write_file → fast path refused"
+        loop_with(allow(), Some(Arc::clone(&restricted)), false)
+            .can_run_parallel_write_batch(&calls),
+        "default active skill restriction mode → fast path follows permission allow"
+    );
+
+    // Compatibility mode preserves the legacy refusal.
+    assert!(
+        !loop_with(allow(), Some(restricted), true).can_run_parallel_write_batch(&calls),
+        "legacy active skill restriction mode → fast path refused"
     );
 
     // Default builtins do not restrict → still fast-paths with Allow.
     assert!(
-        loop_with(allow(), Some(Arc::new(SkillRegistry::with_builtins())))
-            .can_run_parallel_write_batch(&calls),
+        loop_with(
+            allow(),
+            Some(Arc::new(SkillRegistry::with_builtins())),
+            false
+        )
+        .can_run_parallel_write_batch(&calls),
         "non-restricting builtins → fast path still allowed"
     );
 }

core/src/agent/tool_execution_runtime.rs

@@ -127,6 +127,7 @@ impl AgentLoop {
                 args.clone(),
                 ctx.clone(),
                 self.config.skill_registry.clone(),
+                self.config.enforce_active_skill_tool_restrictions,
             );
             let rx = queue.submit_by_tool(name, Box::new(command)).await;
             match rx.await {

core/src/agent_api.rs

@@ -163,6 +163,12 @@ pub struct SessionOptions {
     pub skill_dirs: Vec<PathBuf>,
     /// Optional skill registry for instruction injection
     pub skill_registry: Option<Arc<crate::skills::SkillRegistry>>,
+    /// Whether active skill `allowed-tools` restrict ordinary session tool calls.
+    ///
+    /// Defaults to false so ordinary tools continue through permission policy,
+    /// hooks, HITL, and AHP. Set true to restore the legacy global active-skill
+    /// restriction behavior.
+    pub enforce_active_skill_tool_restrictions: Option<bool>,
     /// Optional memory store for long-term memory persistence
     pub memory_store: Option<Arc<dyn MemoryStore>>,
     /// Deferred file memory directory — constructed async in `build_session()`

core/src/agent_api/session_builder.rs

@@ -155,6 +155,9 @@ pub(super) fn build_agent_session(
         planning_mode: opts.planning_mode,
         goal_tracking: opts.goal_tracking,
         skill_registry: Some(Arc::clone(&effective_registry)),
+        enforce_active_skill_tool_restrictions: opts
+            .enforce_active_skill_tool_restrictions
+            .unwrap_or(base.enforce_active_skill_tool_restrictions),
         max_parse_retries: opts.max_parse_retries.unwrap_or(base.max_parse_retries),
         tool_timeout_ms: opts.tool_timeout_ms.or(base.tool_timeout_ms),
         circuit_breaker_threshold: opts

core/src/agent_api/session_options.rs

@@ -35,6 +35,10 @@ impl std::fmt::Debug for SessionOptions {
                     .as_ref()
                     .map(|r| format!("{} skills", r.len())),
             )
+            .field(
+                "enforce_active_skill_tool_restrictions",
+                &self.enforce_active_skill_tool_restrictions,
+            )
             .field("memory_store", &self.memory_store.is_some())
             .field("session_store", &self.session_store.is_some())
             .field("session_id", &self.session_id)
@@ -210,6 +214,15 @@ impl SessionOptions {
         self
     }
 
+    /// Enable or disable legacy global active-skill `allowed-tools` restrictions.
+    ///
+    /// The default is disabled: active skills do not block ordinary session
+    /// tools before the host permission/AHP/HITL approval chain runs.
+    pub fn with_active_skill_tool_restrictions(mut self, enabled: bool) -> Self {
+        self.enforce_active_skill_tool_restrictions = Some(enabled);
+        self
+    }
+
     /// Add skill directories to scan for skill files (*.md).
     /// Merged with any global `skill_dirs` from [`CodeConfig`] at session build time.
     pub fn with_skill_dirs(mut self, dirs: impl IntoIterator<Item = impl Into<PathBuf>>) -> Self {

core/src/agent_api/session_persistence.rs

@@ -170,6 +170,10 @@ pub(super) fn apply_persisted_runtime_options(
             opts = opts.with_permission_policy(policy);
         }
     }
+    if opts.enforce_active_skill_tool_restrictions.is_none() {
+        opts.enforce_active_skill_tool_restrictions =
+            Some(data.config.enforce_active_skill_tool_restrictions);
+    }
     if opts.max_parallel_tasks.is_none() {
         opts.max_parallel_tasks = data.config.max_parallel_tasks;
     }
@@ -271,6 +275,9 @@ async fn build_session_data_snapshot(input: SessionDataSnapshotInput<'_>) -> Ses
             queue_config: input.config.queue_config.clone(),
             confirmation_policy,
             permission_policy: input.config.permission_policy.clone(),
+            enforce_active_skill_tool_restrictions: input
+                .config
+                .enforce_active_skill_tool_restrictions,
             max_parallel_tasks: Some(input.config.max_parallel_tasks),
             auto_delegation: Some(input.config.auto_delegation.clone()),
             parent_id: None,