merges main

Author: dingfeli

.github/CODEOWNERS

@@ -8,7 +8,7 @@ authors = ["Amazon Q CLI Team ([email protected])", "Chay Nabors (nabochay@amazon
 edition = "2024"
 homepage = "https://aws.amazon.com/q/"
 publish = false
-version = "1.15.0"
+version = "1.16.3"
 license = "MIT OR Apache-2.0"
 
 [workspace.dependencies]
@@ -129,7 +129,7 @@ winnow = "=0.6.2"
 winreg = "0.55.0"
 schemars = "1.0.4"
 jsonschema = "0.30.0"
-rmcp = { version = "0.6.3", features = ["client", "transport-sse-client-reqwest", "reqwest", "transport-streamable-http-client-reqwest", "transport-child-process", "tower", "auth"] }
+rmcp = { version = "0.7.0", features = ["client", "transport-sse-client-reqwest", "reqwest", "transport-streamable-http-client-reqwest", "transport-child-process", "tower", "auth"] }
 
 [workspace.lints.rust]
 future_incompatible = "warn"

Cargo.lock

@@ -14,15 +14,17 @@ pub use builder_id::{
 pub use consts::START_URL;
 use thiserror::Error;
 
+use crate::aws_common::SdkErrorDisplay;
+
 #[derive(Debug, Error)]
 pub enum AuthError {
     #[error(transparent)]
     Ssooidc(Box<aws_sdk_ssooidc::Error>),
-    #[error(transparent)]
+    #[error("{}", SdkErrorDisplay(.0))]
     SdkRegisterClient(Box<SdkError<RegisterClientError>>),
-    #[error(transparent)]
+    #[error("{}", SdkErrorDisplay(.0))]
     SdkCreateToken(Box<SdkError<CreateTokenError>>),
-    #[error(transparent)]
+    #[error("{}", SdkErrorDisplay(.0))]
     SdkStartDeviceAuthorization(Box<SdkError<StartDeviceAuthorizationError>>),
     #[error(transparent)]
     Io(#[from] std::io::Error),

Cargo.toml

@@ -1,4 +1,3 @@
-use std::collections::HashMap;
 use std::fmt::Display;
 
 use schemars::JsonSchema;
@@ -11,23 +10,26 @@ const DEFAULT_TIMEOUT_MS: u64 = 30_000;
 const DEFAULT_MAX_OUTPUT_SIZE: usize = 1024 * 10;
 const DEFAULT_CACHE_TTL_SECONDS: u64 = 0;
 
-#[derive(Debug, Clone, Serialize, Deserialize, Eq, PartialEq, JsonSchema)]
-pub struct Hooks(HashMap<HookTrigger, Hook>);
-
 #[derive(Debug, Clone, Copy, Serialize, Deserialize, Eq, PartialEq, JsonSchema, Hash)]
 #[serde(rename_all = "camelCase")]
 pub enum HookTrigger {
     /// Triggered during agent spawn
     AgentSpawn,
     /// Triggered per user message submission
     UserPromptSubmit,
+    /// Triggered before tool execution
+    PreToolUse,
+    /// Triggered after tool execution
+    PostToolUse,
 }
 
 impl Display for HookTrigger {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
             HookTrigger::AgentSpawn => write!(f, "agentSpawn"),
             HookTrigger::UserPromptSubmit => write!(f, "userPromptSubmit"),
+            HookTrigger::PreToolUse => write!(f, "preToolUse"),
+            HookTrigger::PostToolUse => write!(f, "postToolUse"),
         }
     }
 }
@@ -61,6 +63,11 @@ pub struct Hook {
     #[serde(default = "Hook::default_cache_ttl_seconds")]
     pub cache_ttl_seconds: u64,
 
+    /// Optional glob matcher for hook
+    /// Currently used for matching tool name of PreToolUse and PostToolUse hook
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub matcher: Option<String>,
+
     #[schemars(skip)]
     #[serde(default, skip_serializing)]
     pub source: Source,
@@ -73,6 +80,7 @@ impl Hook {
             timeout_ms: Self::default_timeout_ms(),
             max_output_size: Self::default_max_output_size(),
             cache_ttl_seconds: Self::default_cache_ttl_seconds(),
+            matcher: None,
             source,
         }
     }

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

@@ -80,6 +80,7 @@ impl From<LegacyHook> for Option<Hook> {
             timeout_ms: value.timeout_ms,
             max_output_size: value.max_output_size,
             cache_ttl_seconds: value.cache_ttl_seconds,
+            matcher: None,
             source: Default::default(),
         })
     }

crates/chat-cli/src/cli/agent/hook.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,
+                ToolOrigin::McpServer(_) => 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 {
@@ -818,9 +800,9 @@ impl Agents {
             "fs_read" => "trust working directory".dark_grey(),
             "fs_write" => "not trusted".dark_grey(),
             #[cfg(not(windows))]
-            "execute_bash" => "trust read-only commands".dark_grey(),
+            "execute_bash" => "not trusted".dark_grey(),
             #[cfg(windows)]
-            "execute_cmd" => "trust read-only commands".dark_grey(),
+            "execute_cmd" => "not trusted".dark_grey(),
             "use_aws" => "trust read-only commands".dark_grey(),
             "report_issue" => "trusted".dark_green().bold(),
             "introspect" => "trusted".dark_green().bold(),
@@ -959,6 +941,7 @@ mod tests {
     use serde_json::json;
 
     use super::*;
+    use crate::cli::agent::hook::Source;
     const INPUT: &str = r#"
             {
               "name": "some_agent",
@@ -968,21 +951,21 @@ mod tests {
                 "fetch": { "command": "fetch3.1", "args": [] },
                 "git": { "command": "git-mcp", "args": [] }
               },
-              "tools": [                                    
+              "tools": [
                 "@git"
               ],
               "toolAliases": {
                   "@gits/some_tool": "some_tool2"
               },
-              "allowedTools": [                           
-                "fs_read",                               
+              "allowedTools": [
+                "fs_read",
                 "@fetch",
                 "@gits/git_status"
               ],
-              "resources": [                        
+              "resources": [
                 "file://~/my-genai-prompts/unittest.md"
               ],
-              "toolsSettings": {                     
+              "toolsSettings": {
                 "fs_write": { "allowedPaths": ["~/**"] },
                 "@git/git_status": { "git_user": "$GIT_USER" }
               }
@@ -1188,8 +1171,8 @@ mod tests {
         let execute_name = if cfg!(windows) { "execute_cmd" } else { "execute_bash" };
         let execute_bash_label = agents.display_label(execute_name, &ToolOrigin::Native);
         assert!(
-            execute_bash_label.contains("read-only"),
-            "execute_bash should show read-only by default, instead found: {}",
+            execute_bash_label.contains("not trusted"),
+            "execute_bash should not be trusted by default, instead found: {}",
             execute_bash_label
         );
     }
@@ -1353,4 +1336,70 @@ mod tests {
 
         assert_eq!(agents.get_active().and_then(|a| a.model.as_ref()), None);
     }
+
+    #[test]
+    fn test_agent_with_hooks() {
+        let agent_json = json!({
+            "name": "test-agent",
+            "hooks": {
+                "agentSpawn": [
+                    {
+                        "command": "git status"
+                    }
+                ],
+                "preToolUse": [
+                    {
+                        "matcher": "fs_write",
+                        "command": "validate-tool.sh"
+                    },
+                    {
+                        "matcher": "fs_read",
+                        "command": "enforce-tdd.sh"
+                    }
+                ],
+                "postToolUse": [
+                    {
+                        "matcher": "fs_write",
+                        "command": "format-python.sh"
+                    }
+                ]
+            }
+        });
+
+        let agent: Agent = serde_json::from_value(agent_json).expect("Failed to deserialize agent");
+
+        // Verify agent name
+        assert_eq!(agent.name, "test-agent");
+
+        // Verify agentSpawn hook
+        assert!(agent.hooks.contains_key(&HookTrigger::AgentSpawn));
+        let agent_spawn_hooks = &agent.hooks[&HookTrigger::AgentSpawn];
+        assert_eq!(agent_spawn_hooks.len(), 1);
+        assert_eq!(agent_spawn_hooks[0].command, "git status");
+        assert_eq!(agent_spawn_hooks[0].matcher, None);
+
+        // Verify preToolUse hooks
+        assert!(agent.hooks.contains_key(&HookTrigger::PreToolUse));
+        let pre_tool_hooks = &agent.hooks[&HookTrigger::PreToolUse];
+        assert_eq!(pre_tool_hooks.len(), 2);
+
+        assert_eq!(pre_tool_hooks[0].command, "validate-tool.sh");
+        assert_eq!(pre_tool_hooks[0].matcher, Some("fs_write".to_string()));
+
+        assert_eq!(pre_tool_hooks[1].command, "enforce-tdd.sh");
+        assert_eq!(pre_tool_hooks[1].matcher, Some("fs_read".to_string()));
+
+        // Verify postToolUse hooks
+        assert!(agent.hooks.contains_key(&HookTrigger::PostToolUse));
+
+        // Verify default values are set correctly
+        for hooks in agent.hooks.values() {
+            for hook in hooks {
+                assert_eq!(hook.timeout_ms, 30_000);
+                assert_eq!(hook.max_output_size, 10_240);
+                assert_eq!(hook.cache_ttl_seconds, 0);
+                assert_eq!(hook.source, Source::Agent);
+            }
+        }
+    }
 }