wip

brandonskiser · brandonskiser · commit c7134b7e501d · 2025-10-30T14:47:01.000-07:00
diff --git a/crates/agent/src/agent/agent_config/parse.rs b/crates/agent/src/agent/agent_config/parse.rs
@@ -5,25 +5,17 @@ use std::str::FromStr;
 
 use crate::agent::tools::BuiltInToolName;
 use crate::agent::util::path::canonicalize_path_sys;
-use crate::agent::util::providers::{
-    RealProvider,
-    SystemProvider,
-};
+use crate::agent::util::providers::SystemProvider;
 
 /// Represents a value from the `resources` array in the agent config.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum ResourceKind<'a> {
-    File { original: &'a str, file_path: &'a str },
+    File { original: &'a str, file_path: String },
     FileGlob { original: &'a str, pattern: glob::Pattern },
 }
 
 impl<'a> ResourceKind<'a> {
-    pub fn parse(value: &'a str) -> Result<Self, String> {
-        let sys = RealProvider;
-        Self::parse_impl(value, &sys)
-    }
-
-    fn parse_impl(value: &'a str, sys: &impl SystemProvider) -> Result<Self, String> {
+    pub fn parse(value: &'a str, sys: &impl SystemProvider) -> Result<Self, String> {
         if !value.starts_with("file://") {
             return Err("Only file schemes are currently supported".to_string());
         }
@@ -41,7 +33,8 @@ impl<'a> ResourceKind<'a> {
         } else {
             Ok(Self::File {
                 original: value,
-                file_path,
+                file_path: canonicalize_path_sys(file_path, sys)
+                    .map_err(|err| format!("Failed to canonicalize path for {}: {}", file_path, err))?,
             })
         }
     }
@@ -216,7 +209,7 @@ mod tests {
     #[test]
     fn test_resource_kind_parse_nonfile() {
         assert!(
-            ResourceKind::parse("https://google.com").is_err(),
+            ResourceKind::parse("https://google.com", &TestProvider::new()).is_err(),
             "non-file scheme should be an error"
         );
     }
@@ -226,18 +219,15 @@ mod tests {
         let sys = TestProvider::new();
 
         let resource = "file://project/README.md";
-        assert_eq!(ResourceKind::parse_impl(resource, &sys).unwrap(), ResourceKind::File {
+        assert_eq!(ResourceKind::parse(resource, &sys).unwrap(), ResourceKind::File {
             original: resource,
-            file_path: "project/README.md"
+            file_path: "project/README.md".to_string()
         });
 
         let resource = "file://~/project/**/*.rs";
-        assert_eq!(
-            ResourceKind::parse_impl(resource, &sys).unwrap(),
-            ResourceKind::FileGlob {
-                original: resource,
-                pattern: glob::Pattern::new("/home/testuser/project/**/*.rs").unwrap()
-            }
-        );
+        assert_eq!(ResourceKind::parse(resource, &sys).unwrap(), ResourceKind::FileGlob {
+            original: resource,
+            pattern: glob::Pattern::new("/home/testuser/project/**/*.rs").unwrap()
+        });
     }
 }
diff --git a/crates/agent/src/agent/mod.rs b/crates/agent/src/agent/mod.rs
@@ -1645,18 +1645,20 @@ impl Agent {
             match result {
                 ToolExecutorResult::Completed { id, result } => match result {
                     Ok(res) => {
+                        let mut content_items = Vec::new();
                         for item in &res.items {
                             let content_item = match item {
                                 ToolExecutionOutputItem::Text(s) => ToolResultContentBlock::Text(s.clone()),
                                 ToolExecutionOutputItem::Json(v) => ToolResultContentBlock::Json(v.clone()),
                                 ToolExecutionOutputItem::Image(i) => ToolResultContentBlock::Image(i.clone()),
                             };
-                            content.push(ContentBlock::ToolResult(ToolResultBlock {
-                                tool_use_id: id.tool_use_id().to_string(),
-                                content: vec![content_item],
-                                status: ToolResultStatus::Success,
-                            }));
+                            content_items.push(content_item);
                         }
+                        content.push(ContentBlock::ToolResult(ToolResultBlock {
+                            tool_use_id: id.tool_use_id().to_string(),
+                            content: content_items,
+                            status: ToolResultStatus::Success,
+                        }));
                     },
                     Err(err) => content.push(ContentBlock::ToolResult(ToolResultBlock {
                         tool_use_id: id.tool_use_id().to_string(),
@@ -1916,7 +1918,7 @@ where
 
     let mut return_val = Vec::new();
     for resource in resources {
-        let Ok(kind) = ResourceKind::parse(resource.as_ref()) else {
+        let Ok(kind) = ResourceKind::parse(resource.as_ref(), provider) else {
             continue;
         };
         match kind {
@@ -2165,3 +2167,34 @@ pub enum HookStage {
     /// Hooks after executing tool uses
     PostToolUse { tool_results: Vec<ToolExecutorResult> },
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::util::test::{
+        TestDir,
+        TestProvider,
+    };
+
+    #[tokio::test]
+    async fn test_collect_resources() {
+        let mut test_dir = TestDir::new();
+        let test_provider = TestProvider::new_with_base(test_dir.path());
+
+        let files = [
+            (".amazonq/rules/first.md", "first"),
+            (".amazonq/rules/dir/subdir.md", "subdir"),
+            ("~/home.txt", "home"),
+        ];
+
+        for file in files {
+            test_dir = test_dir.with_file_sys(file, &test_provider).await;
+        }
+
+        let resources = collect_resources(["file://.amazonq/rules/**/*.md", "file://~/home.txt"], &test_provider).await;
+
+        for file in files {
+            assert!(resources.iter().any(|r| r.content == file.1));
+        }
+    }
+}
diff --git a/crates/agent/src/agent/task_executor/mod.rs b/crates/agent/src/agent/task_executor/mod.rs
@@ -706,7 +706,7 @@ mod tests {
             })
             .await;
 
-        run_with_timeout(Duration::from_millis(100), async move {
+        run_with_timeout(Duration::from_millis(1000), async move {
             let mut event_buf = Vec::new();
             loop {
                 executor.recv_next(&mut event_buf).await;
diff --git a/crates/agent/src/agent/tools/fs_write.rs b/crates/agent/src/agent/tools/fs_write.rs
@@ -65,6 +65,10 @@ const FS_WRITE_SCHEMA: &str = r#"
             "description": "Required parameter of `strReplace` command containing the string in `path` to replace.",
             "type": "string"
         },
+        "replaceAll": {
+            "description": "Optional parameter of `strReplace` command. Default is false. When true, all instances of `oldStr` will be replaced with `newStr`.",
+            "type": "boolean"
+        },
         "path": {
             "description": "Path to the file",
             "type": "string"
@@ -204,6 +208,7 @@ pub struct StrReplace {
     path: String,
     old_str: String,
     new_str: String,
+    #[serde(default)]
     replace_all: bool,
 }
 
diff --git a/crates/agent/src/agent/util/test.rs b/crates/agent/src/agent/util/test.rs
@@ -6,6 +6,7 @@ use std::path::{
     PathBuf,
 };
 
+use super::path::canonicalize_path_sys;
 use super::providers::{
     CwdProvider,
     EnvProvider,
@@ -37,10 +38,11 @@ impl TestDir {
     /// Writes the given file under the test directory. Creates parent directories if needed.
     ///
     /// The path given by `file` is *not* canonicalized.
+    #[deprecated]
     pub async fn with_file(self, file: impl TestFile) -> Self {
         let file_path = file.path();
-        if file_path.is_absolute() {
-            panic!("absolute paths are currently not supported");
+        if file_path.is_absolute() && !file_path.starts_with(self.temp_dir.path()) {
+            panic!("path falls outside of the temp dir");
         }
 
         let path = self.temp_dir.path().join(file_path);
@@ -52,6 +54,28 @@ impl TestDir {
         tokio::fs::write(path, file.content()).await.unwrap();
         self
     }
+
+    /// Writes the given file under the test directory. Creates parent directories if needed.
+    ///
+    /// This function panics if the file path is outside of the test directory.
+    pub async fn with_file_sys<P: SystemProvider>(self, file: impl TestFile, provider: &P) -> Self {
+        let file_path = canonicalize_path_sys(file.path().to_string_lossy(), provider).unwrap();
+
+        // Check to ensure that the file path resolves under the test directory.
+        if !file_path.starts_with(&self.temp_dir.path().to_string_lossy().to_string()) {
+            panic!("outside of temp dir");
+        }
+
+        let file_path = PathBuf::from(file_path);
+        if let Some(parent) = file_path.parent() {
+            if !parent.exists() {
+                tokio::fs::create_dir_all(parent).await.unwrap();
+            }
+        }
+
+        tokio::fs::write(file_path, file.content()).await.unwrap();
+        self
+    }
 }
 
 impl Default for TestDir {
@@ -79,6 +103,16 @@ where
     }
 }
 
+impl TestFile for Box<dyn TestFile> {
+    fn path(&self) -> PathBuf {
+        (**self).path()
+    }
+
+    fn content(&self) -> Vec<u8> {
+        (**self).content()
+    }
+}
+
 /// Test helper that implements [EnvProvider], [HomeProvider], and [CwdProvider].
 #[derive(Debug, Clone)]
 pub struct TestProvider {
@@ -161,3 +195,34 @@ impl CwdProvider for TestProvider {
 }
 
 impl SystemProvider for TestProvider {}
+
+#[cfg(test)]
+mod tests {
+    use tokio::fs;
+
+    use super::*;
+
+    #[tokio::test]
+    async fn test_tempdir_files() {
+        let mut test_dir = TestDir::new();
+        let test_provider = TestProvider::new_with_base(test_dir.path());
+
+        let files = [("base", "base"), ("~/tilde", "tilde"), ("$HOME/home", "home")];
+        for file in files {
+            test_dir = test_dir.with_file_sys(file, &test_provider).await;
+        }
+
+        assert_eq!(fs::read_to_string(test_dir.join("base")).await.unwrap(), "base");
+        assert_eq!(fs::read_to_string(test_dir.join("tilde")).await.unwrap(), "tilde");
+        assert_eq!(fs::read_to_string(test_dir.join("home")).await.unwrap(), "home");
+    }
+
+    #[tokio::test]
+    #[should_panic]
+    async fn test_tempdir_write_file_outside() {
+        let test_dir = TestDir::new();
+        let test_provider = TestProvider::new_with_base(test_dir.path());
+
+        let _ = test_dir.with_file_sys(("..", "hello"), &test_provider).await;
+    }
+}
diff --git a/crates/agent/src/cli/run.rs b/crates/agent/src/cli/run.rs
@@ -165,7 +165,6 @@ impl RunArgs {
 
         if self.output_format == Some(OutputFormat::Json) {
             let md = user_turn_metadata.expect("user turn metadata should exist");
-            println!("user turn metadata: {:?}", md);
             let is_error = md.end_reason != LoopEndReason::UserTurnEnd || md.result.as_ref().is_none_or(|v| v.is_err());
             let result = md.result.and_then(|r| r.ok().map(|m| m.text()));
 
diff --git a/crates/agent/tests/common/mod.rs b/crates/agent/tests/common/mod.rs
@@ -19,6 +19,7 @@ use agent::agent_loop::protocol::{
 };
 use agent::agent_loop::types::{
     ContentBlock,
+    Message,
     Role,
     ToolSpec,
 };
@@ -45,6 +46,8 @@ use rand::Rng as _;
 use rand::distr::Alphanumeric;
 use serde::Serialize;
 
+type MockResponseStreams = Vec<Vec<StreamResult>>;
+
 #[derive(Default)]
 pub struct TestCaseBuilder {
     test_name: Option<String>,
@@ -99,7 +102,13 @@ impl TestCaseBuilder {
         }
 
         let mut agent = Agent::new(snapshot, Arc::new(model), McpManager::new().spawn()).await?;
-        let temp_dir = TestDir::new();
+
+        let mut temp_dir = TestDir::new();
+        let test_provider = TestProvider::new_with_base(temp_dir.path());
+        for file in self.files {
+            temp_dir = temp_dir.with_file_sys(file, &test_provider).await;
+        }
+
         agent.set_sys_provider(TestProvider::new_with_base(temp_dir.path()));
 
         let test_name = self.test_name.unwrap_or(format!(
@@ -234,6 +243,10 @@ pub struct SentRequest {
 }
 
 impl SentRequest {
+    pub fn messages(&self) -> &[Message] {
+        &self.original.messages
+    }
+
     pub fn prompt_contains_text(&self, text: impl AsRef<str>) -> bool {
         let text = text.as_ref();
         let prompt = self.original.messages.last().unwrap();
@@ -278,5 +291,3 @@ pub async fn parse_response_streams(content: impl AsRef<str>) -> Result<MockResp
     }
     Ok(stream)
 }
-
-type MockResponseStreams = Vec<Vec<StreamResult>>;
diff --git a/crates/agent/tests/mod.rs b/crates/agent/tests/mod.rs
@@ -16,13 +16,17 @@ async fn test_agent_defaults() {
     const AMAZON_Q_MD_CONTENT: &str = "AmazonQ.md-FILE-CONTENT";
     const AGENTS_MD_CONTENT: &str = "AGENTS.md-FILE-CONTENT";
     const README_MD_CONTENT: &str = "README.md-FILE-CONTENT";
+    const LOCAL_RULE_MD_CONTENT: &str = "local_rule.md-FILE-CONTENT";
+    const SUB_LOCAL_RULE_MD_CONTENT: &str = "sub_local_rule.md-FILE-CONTENT";
 
     let mut test = TestCase::builder()
         .test_name("agent default config behavior")
         .with_agent_config(AgentConfig::default())
         .with_file(("AmazonQ.md", AMAZON_Q_MD_CONTENT))
         .with_file(("AGENTS.md", AGENTS_MD_CONTENT))
         .with_file(("README.md", README_MD_CONTENT))
+        .with_file((".amazonq/rules/local_rule.md", LOCAL_RULE_MD_CONTENT))
+        .with_file((".amazonq/rules/subfolder/sub_local_rule.md", SUB_LOCAL_RULE_MD_CONTENT))
         .with_responses(
             parse_response_streams(include_str!("./mock_responses/builtin_tools.jsonl"))
                 .await
@@ -49,4 +53,21 @@ async fn test_agent_defaults() {
     test.send_prompt("start turn".to_string()).await;
 
     test.wait_until_agent_stop(Duration::from_secs(2)).await;
+
+    for req in test.requests() {
+        let first_msg = req.messages().first().expect("first message should exist").text();
+        let assert_contains = |expected: &str| {
+            assert!(
+                first_msg.contains(expected),
+                "expected to find '{}' inside content: '{}'",
+                expected,
+                first_msg
+            );
+        };
+        assert_contains(AMAZON_Q_MD_CONTENT);
+        assert_contains(AGENTS_MD_CONTENT);
+        assert_contains(README_MD_CONTENT);
+        assert_contains(LOCAL_RULE_MD_CONTENT);
+        assert_contains(SUB_LOCAL_RULE_MD_CONTENT);
+    }
 }
