Merge branch 'main' into snapshot_revise

Author: evanliu048

Cargo.lock

@@ -8,7 +8,7 @@ authors = ["Amazon Q CLI Team (q-cli@amazon.com)", "Chay Nabors (nabochay@amazon
 edition = "2024"
 homepage = "https://aws.amazon.com/q/"
 publish = false
-version = "1.16.0"
+version = "1.16.2"
 license = "MIT OR Apache-2.0"
 
 [workspace.dependencies]

Cargo.toml

@@ -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),

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

@@ -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/cli/agent/hook.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/legacy/hooks.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);
+            }
+        }
+    }
 }

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

@@ -52,6 +52,12 @@ impl ClearArgs {
             if let Some(cm) = session.conversation.context_manager.as_mut() {
                 cm.hook_executor.cache.clear();
             }
+
+            // Reset pending tool state to prevent orphaned tool approval prompts
+            session.tool_uses.clear();
+            session.pending_tool_index = None;
+            session.tool_turn_start_time = None;
+
             execute!(
                 session.stderr,
                 style::SetForegroundColor(Color::Green),

crates/chat-cli/src/cli/chat/cli/clear.rs

@@ -84,13 +84,8 @@ impl EditorArgs {
     }
 }
 
-/// Opens the user's preferred editor to compose a prompt
-pub fn open_editor(initial_text: Option<String>) -> Result<String, ChatError> {
-    // Create a temporary file with a unique name
-    let temp_dir = std::env::temp_dir();
-    let file_name = format!("q_prompt_{}.md", Uuid::new_v4());
-    let temp_file_path = temp_dir.join(file_name);
-
+/// Launch the user's preferred editor with the given file path
+fn launch_editor(file_path: &std::path::Path) -> Result<(), ChatError> {
     // Get the editor from environment variable or use a default
     let editor_cmd = std::env::var("EDITOR").unwrap_or_else(|_| "vi".to_string());
 
@@ -104,11 +99,6 @@ pub fn open_editor(initial_text: Option<String>) -> Result<String, ChatError> {
 
     let editor_bin = parts.remove(0);
 
-    // Write initial content to the file if provided
-    let initial_content = initial_text.unwrap_or_default();
-    std::fs::write(&temp_file_path, &initial_content)
-        .map_err(|e| ChatError::Custom(format!("Failed to create temporary file: {}", e).into()))?;
-
     // Open the editor with the parsed command and arguments
     let mut cmd = std::process::Command::new(editor_bin);
     // Add any arguments that were part of the EDITOR variable
@@ -117,14 +107,37 @@ pub fn open_editor(initial_text: Option<String>) -> Result<String, ChatError> {
     }
     // Add the file path as the last argument
     let status = cmd
-        .arg(&temp_file_path)
+        .arg(file_path)
         .status()
         .map_err(|e| ChatError::Custom(format!("Failed to open editor: {}", e).into()))?;
 
     if !status.success() {
         return Err(ChatError::Custom("Editor exited with non-zero status".into()));
     }
 
+    Ok(())
+}
+
+/// Opens the user's preferred editor to edit an existing file
+pub fn open_editor_file(file_path: &std::path::Path) -> Result<(), ChatError> {
+    launch_editor(file_path)
+}
+
+/// Opens the user's preferred editor to compose a prompt
+pub fn open_editor(initial_text: Option<String>) -> Result<String, ChatError> {
+    // Create a temporary file with a unique name
+    let temp_dir = std::env::temp_dir();
+    let file_name = format!("q_prompt_{}.md", Uuid::new_v4());
+    let temp_file_path = temp_dir.join(file_name);
+
+    // Write initial content to the file if provided
+    let initial_content = initial_text.unwrap_or_default();
+    std::fs::write(&temp_file_path, &initial_content)
+        .map_err(|e| ChatError::Custom(format!("Failed to create temporary file: {}", e).into()))?;
+
+    // Launch the editor
+    launch_editor(&temp_file_path)?;
+
     // Read the content back
     let content = std::fs::read_to_string(&temp_file_path)
         .map_err(|e| ChatError::Custom(format!("Failed to read temporary file: {}", e).into()))?;

crates/chat-cli/src/cli/chat/cli/editor.rs

@@ -59,6 +59,11 @@ static AVAILABLE_EXPERIMENTS: &[Experiment] = &[
         ),
         setting_key: Setting::EnabledCheckpoint,
     },
+    Experiment {
+        name: "Context Usage Indicator",
+        description: "Shows context usage percentage in the prompt (e.g., [rust-agent] 6% >)",
+        setting_key: Setting::EnabledContextUsageIndicator,
+    },
 ];
 
 #[derive(Debug, PartialEq, Args)]