fix: consolidate tool permission logic for consistent display and exe…

crates/chat-cli/src/cli/agent/mod.rs

@@ -776,32 +776,14 @@ impl Agents {
 
     /// Returns a label to describe the permission status for a given tool.
     pub fn display_label(&self, tool_name: &str, origin: &ToolOrigin) -> String {
-        use crate::util::pattern_matching::matches_any_pattern;
+        use crate::util::tool_permission_checker::is_tool_in_allowlist;
 
         let tool_trusted = self.get_active().is_some_and(|a| {
-            if matches!(origin, &ToolOrigin::Native) {
-                return matches_any_pattern(&a.allowed_tools, tool_name);
-            }
-
-            a.allowed_tools.iter().any(|name| {
-                name.strip_prefix("@").is_some_and(|remainder| {
-                    remainder
-                        .split_once(MCP_SERVER_TOOL_DELIMITER)
-                        .is_some_and(|(_left, right)| right == tool_name)
-                        || remainder == <ToolOrigin as Borrow<str>>::borrow(origin)
-                }) || {
-                    if let Some(server_name) = name.strip_prefix("@").and_then(|s| s.split('/').next()) {
-                        if server_name == <ToolOrigin as Borrow<str>>::borrow(origin) {
-                            let tool_pattern = format!("@{}/{}", server_name, tool_name);
-                            matches_any_pattern(&a.allowed_tools, &tool_pattern)
-                        } else {
-                            false
-                        }
-                    } else {
-                        false
-                    }
-                }
-            })
+            let server_name = match origin {
+                ToolOrigin::Native => None,
+                _ => Some(<ToolOrigin as Borrow<str>>::borrow(origin)),
+            };
+            is_tool_in_allowlist(&a.allowed_tools, tool_name, server_name)
         });
 
         if tool_trusted || self.trust_all_tools {

crates/chat-cli/src/cli/chat/tools/custom_tool.rs

@@ -28,7 +28,6 @@ use crate::mcp_client::{
 };
 use crate::os::Os;
 use crate::util::MCP_SERVER_TOOL_DELIMITER;
-use crate::util::pattern_matching::matches_any_pattern;
 
 #[derive(Clone, Serialize, Deserialize, Debug, Eq, PartialEq, JsonSchema)]
 #[serde(rename_all = "camelCase")]
@@ -175,18 +174,12 @@ impl CustomTool {
     }
 
     pub fn eval_perm(&self, _os: &Os, agent: &Agent) -> PermissionEvalResult {
-        let server_name = &self.server_name;
+        use crate::util::tool_permission_checker::is_tool_in_allowlist;
 
-        let server_pattern = format!("@{server_name}");
-        if agent.allowed_tools.contains(&server_pattern) {
-            return PermissionEvalResult::Allow;
-        }
-
-        let tool_pattern = self.namespaced_tool_name();
-        if matches_any_pattern(&agent.allowed_tools, &tool_pattern) {
-            return PermissionEvalResult::Allow;
+        if is_tool_in_allowlist(&agent.allowed_tools, &self.name, Some(&self.server_name)) {
+            PermissionEvalResult::Allow
+        } else {
+            PermissionEvalResult::Ask
         }
-
-        PermissionEvalResult::Ask
     }
 }

crates/chat-cli/src/cli/chat/tools/execute/mod.rs

@@ -23,7 +23,7 @@ use crate::cli::chat::tools::{
 };
 use crate::cli::chat::util::truncate_safe;
 use crate::os::Os;
-use crate::util::pattern_matching::matches_any_pattern;
+use crate::util::tool_permission_checker::is_tool_in_allowlist;
 
 // Platform-specific modules
 #[cfg(windows)]
@@ -205,7 +205,7 @@ impl ExecuteCommand {
 
         let Self { command, .. } = self;
         let tool_name = if cfg!(windows) { "execute_cmd" } else { "execute_bash" };
-        let is_in_allowlist = matches_any_pattern(&agent.allowed_tools, tool_name);
+        let is_in_allowlist = is_tool_in_allowlist(&agent.allowed_tools, tool_name, None);
         match agent.tools_settings.get(tool_name) {
             Some(settings) => {
                 let Settings {

crates/chat-cli/src/cli/chat/tools/fs_read.rs

@@ -46,7 +46,7 @@ use crate::cli::chat::{
 };
 use crate::os::Os;
 use crate::util::directories;
-use crate::util::pattern_matching::matches_any_pattern;
+use crate::util::tool_permission_checker::is_tool_in_allowlist;
 
 #[derive(Debug, Clone, Deserialize)]
 pub struct FsRead {
@@ -113,7 +113,7 @@ impl FsRead {
             allow_read_only: bool,
         }
 
-        let is_in_allowlist = matches_any_pattern(&agent.allowed_tools, "fs_read");
+        let is_in_allowlist = is_tool_in_allowlist(&agent.allowed_tools, "fs_read", None);
         let settings = agent
             .tools_settings
             .get("fs_read")

crates/chat-cli/src/cli/chat/tools/fs_write.rs

@@ -45,7 +45,7 @@ use crate::cli::agent::{
 use crate::cli::chat::line_tracker::FileLineTracker;
 use crate::os::Os;
 use crate::util::directories;
-use crate::util::pattern_matching::matches_any_pattern;
+use crate::util::tool_permission_checker::is_tool_in_allowlist;
 
 static SYNTAX_SET: LazyLock<SyntaxSet> = LazyLock::new(SyntaxSet::load_defaults_newlines);
 static THEME_SET: LazyLock<ThemeSet> = LazyLock::new(ThemeSet::load_defaults);
@@ -470,7 +470,7 @@ impl FsWrite {
             denied_paths: Vec<String>,
         }
 
-        let is_in_allowlist = matches_any_pattern(&agent.allowed_tools, "fs_write");
+        let is_in_allowlist = is_tool_in_allowlist(&agent.allowed_tools, "fs_write", None);
         match agent.tools_settings.get("fs_write") {
             Some(settings) => {
                 let Settings {

crates/chat-cli/src/cli/chat/tools/knowledge.rs

@@ -20,7 +20,7 @@ use crate::cli::agent::{
 use crate::database::settings::Setting;
 use crate::os::Os;
 use crate::util::knowledge_store::KnowledgeStore;
-use crate::util::pattern_matching::matches_any_pattern;
+use crate::util::tool_permission_checker::is_tool_in_allowlist;
 
 /// The Knowledge tool allows storing and retrieving information across chat sessions.
 /// It provides semantic search capabilities for files, directories, and text content.
@@ -497,7 +497,7 @@ impl Knowledge {
         _ = self;
         _ = os;
 
-        if matches_any_pattern(&agent.allowed_tools, "knowledge") {
+        if is_tool_in_allowlist(&agent.allowed_tools, "knowledge", None) {
             PermissionEvalResult::Allow
         } else {
             PermissionEvalResult::Ask

crates/chat-cli/src/cli/chat/tools/use_aws.rs

@@ -29,7 +29,7 @@ use crate::cli::agent::{
     PermissionEvalResult,
 };
 use crate::os::Os;
-use crate::util::pattern_matching::matches_any_pattern;
+use crate::util::tool_permission_checker::is_tool_in_allowlist;
 
 const READONLY_OPS: [&str; 6] = ["get", "describe", "list", "ls", "search", "batch_get"];
 
@@ -187,7 +187,7 @@ impl UseAws {
         }
 
         let Self { service_name, .. } = self;
-        let is_in_allowlist = matches_any_pattern(&agent.allowed_tools, "use_aws");
+        let is_in_allowlist = is_tool_in_allowlist(&agent.allowed_tools, "use_aws", None);
         match agent.tools_settings.get("use_aws") {
             Some(settings) => {
                 let settings = match serde_json::from_value::<Settings>(settings.clone()) {

crates/chat-cli/src/util/mod.rs

@@ -7,6 +7,7 @@ pub mod spinner;
 pub mod system_info;
 #[cfg(test)]
 pub mod test;
+pub mod tool_permission_checker;
 pub mod ui;
 
 use std::fmt::Display;

crates/chat-cli/src/util/tool_permission_checker.rs

@@ -0,0 +1,82 @@
+use std::collections::HashSet;
+
+use tracing::debug;
+
+use crate::util::MCP_SERVER_TOOL_DELIMITER;
+use crate::util::pattern_matching::matches_any_pattern;
+
+/// Checks if a tool is allowed based on the agent's allowed_tools configuration.
+/// This function handles both native tools and MCP tools with wildcard pattern support.
+pub fn is_tool_in_allowlist(allowed_tools: &HashSet<String>, tool_name: &str, server_name: Option<&str>) -> bool {
+    let filter_patterns = |predicate: fn(&str) -> bool| -> HashSet<String> {
+        allowed_tools
+            .iter()
+            .filter(|pattern| predicate(pattern))
+            .cloned()
+            .collect()
+    };
+
+    match server_name {
+        // Native tool
+        None => {
+            let patterns = filter_patterns(|p| !p.starts_with('@'));
+            debug!("Native patterns: {:?}", patterns);
+            let result = matches_any_pattern(&patterns, tool_name);
+            debug!("Native tool '{}' permission check result: {}", tool_name, result);
+            result
+        },
+        // MCP tool
+        Some(server) => {
+            let patterns = filter_patterns(|p| p.starts_with('@'));
+            debug!("MCP patterns: {:?}", patterns);
+
+            // Check server-level permission first: @server_name
+            let server_pattern = format!("@{}", server);
+            debug!("Checking server-level pattern: '{}'", server_pattern);
+            if matches_any_pattern(&patterns, &server_pattern) {
+                debug!("Server-level permission granted for '{}'", server_pattern);
+                return true;
+            }
+
+            // Check tool-specific permission: @server_name/tool_name
+            let tool_pattern = format!("@{}{}{}", server, MCP_SERVER_TOOL_DELIMITER, tool_name);
+            debug!("Checking tool-specific pattern: '{}'", tool_pattern);
+            let result = matches_any_pattern(&patterns, &tool_pattern);
+            debug!("Tool-specific permission result for '{}': {}", tool_pattern, result);
+            result
+        },
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use std::collections::HashSet;
+
+    use super::*;
+
+    #[test]
+    fn test_native_vs_mcp_separation() {
+        let mut allowed = HashSet::new();
+        allowed.insert("fs_*".to_string());
+        allowed.insert("@git".to_string());
+
+        // Native patterns only apply to native tools
+        assert!(is_tool_in_allowlist(&allowed, "fs_read", None));
+        assert!(!is_tool_in_allowlist(&allowed, "fs_read", Some("server")));
+
+        // MCP patterns only apply to MCP tools
+        assert!(is_tool_in_allowlist(&allowed, "status", Some("git")));
+        assert!(!is_tool_in_allowlist(&allowed, "git", None));
+    }
+
+    #[test]
+    fn test_mcp_wildcard_patterns() {
+        let mut allowed = HashSet::new();
+        allowed.insert("@*quip*".to_string());
+        allowed.insert("@git/read_*".to_string());
+
+        assert!(is_tool_in_allowlist(&allowed, "tool", Some("quip-server")));
+        assert!(is_tool_in_allowlist(&allowed, "read_file", Some("git")));
+        assert!(!is_tool_in_allowlist(&allowed, "write_file", Some("git")));
+    }
+}