feat: add /clear command, fix use_aws parameters bug, add tilde expansion, minor fixes (#677)

Author: brandonskiser

crates/q_cli/src/cli/chat/conversation_state.rs

@@ -94,6 +94,12 @@ impl ConversationState {
         }
     }
 
+    /// Clears the conversation history.
+    pub fn clear(&mut self) {
+        self.next_message = None;
+        self.history.clear();
+    }
+
     pub async fn append_new_user_message(&mut self, input: String) {
         debug_assert!(self.next_message.is_none(), "next_message should not exist");
         if let Some(next_message) = self.next_message.as_ref() {

crates/q_cli/src/cli/chat/mod.rs

@@ -23,6 +23,7 @@ use rustyline::{
 use winnow::stream::AsChar;
 
 const MODIFIERS: &[&str] = &["@history", "@git", "@env"];
+const COMMANDS: &[&str] = &["/clear"];
 
 pub struct ChatCompleater {}
 
@@ -50,6 +51,12 @@ impl Completer for ChatCompleater {
                     .filter(|p| p.starts_with(word))
                     .map(|s| (*s).to_owned())
                     .collect()
+            } else if word.starts_with('/') {
+                COMMANDS
+                    .iter()
+                    .filter(|p| p.starts_with(word))
+                    .map(|s| (*s).to_owned())
+                    .collect()
             } else {
                 Vec::new()
             },

crates/q_cli/src/cli/chat/prompt.rs

@@ -2,7 +2,6 @@ use std::collections::VecDeque;
 use std::fs::Metadata;
 use std::io::Write;
 use std::os::unix::fs::PermissionsExt;
-use std::path::PathBuf;
 
 use crossterm::queue;
 use crossterm::style::{
@@ -22,6 +21,7 @@ use super::{
     InvokeOutput,
     OutputKind,
     format_path,
+    sanitize_path_tool_arg,
     stylize_output_if_able,
 };
 
@@ -46,10 +46,8 @@ impl FsRead {
     }
 
     pub async fn invoke(&self, ctx: &Context, updates: &mut impl Write) -> Result<InvokeOutput> {
-        // Required for testing scenarios: since the path is passed directly as a command argument,
-        // we need to pass it through the Context first.
-        let path = ctx.fs().chroot_path_str(&self.path);
-        let is_file = ctx.fs().symlink_metadata(&self.path).await?.is_file();
+        let path = sanitize_path_tool_arg(ctx, &self.path);
+        let is_file = ctx.fs().symlink_metadata(&path).await?.is_file();
 
         if is_file {
             if let Some((start, Some(end))) = self.read_range()? {
@@ -114,7 +112,7 @@ impl FsRead {
             let max_depth = self.read_range()?.map_or(0, |(d, _)| d);
             let mut result = Vec::new();
             let mut dir_queue = VecDeque::new();
-            dir_queue.push_back((PathBuf::from(path), 0));
+            dir_queue.push_back((path, 0));
             while let Some((path, depth)) = dir_queue.pop_front() {
                 if depth > max_depth {
                     break;
@@ -239,11 +237,12 @@ impl FsRead {
     }
 
     pub async fn validate(&mut self, ctx: &Context) -> Result<()> {
-        if !PathBuf::from(&self.path).exists() {
+        let path = sanitize_path_tool_arg(ctx, &self.path);
+        if !path.exists() {
             bail!("'{}' does not exist", self.path);
         }
 
-        let is_file = ctx.fs().symlink_metadata(&self.path).await?.is_file();
+        let is_file = ctx.fs().symlink_metadata(&path).await?.is_file();
         self.ty = Some(is_file);
 
         Ok(())

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

@@ -1,6 +1,5 @@
 use std::borrow::Cow;
 use std::io::Write;
-use std::path::PathBuf;
 
 use crossterm::queue;
 use crossterm::style::{
@@ -18,6 +17,7 @@ use serde::Deserialize;
 use super::{
     InvokeOutput,
     format_path,
+    sanitize_path_tool_arg,
     stylize_output_if_able,
 };
 
@@ -46,8 +46,9 @@ impl FsWrite {
         let cwd = ctx.env().current_dir()?;
         match self {
             FsWrite::Create { path, file_text } => {
-                let relative_path = format_path(cwd, fs.chroot_path(path));
-                let invoke_description = if fs.exists(path) {
+                let path = sanitize_path_tool_arg(ctx, path);
+                let relative_path = format_path(cwd, fs.chroot_path(&path));
+                let invoke_description = if fs.exists(&path) {
                     "Replacing the current file contents at"
                 } else {
                     "Creating a new file at"
@@ -63,12 +64,13 @@ impl FsWrite {
                 Ok(Default::default())
             },
             FsWrite::StrReplace { path, old_str, new_str } => {
+                let path = sanitize_path_tool_arg(ctx, path);
                 let file = fs.read_to_string(&path).await?;
                 let matches = file.match_indices(old_str).collect::<Vec<_>>();
                 queue!(
                     updates,
                     style::SetForegroundColor(Color::Green),
-                    style::Print(format!("Updating {}", format_path(cwd, path))),
+                    style::Print(format!("Updating {}", format_path(cwd, &path))),
                     style::ResetColor,
                     style::Print("\n"),
                 )?;
@@ -160,13 +162,13 @@ impl FsWrite {
                 insert_line,
                 new_str,
             } => {
-                let path = format_path(cwd, path);
-                let file = stylize_output_if_able(ctx, &path, new_str, Some(*insert_line), Some("+"));
+                let relative_path = format_path(cwd, path);
+                let file = stylize_output_if_able(ctx, &relative_path, new_str, Some(*insert_line), Some("+"));
                 queue!(
                     updates,
                     style::Print("Path: "),
                     style::SetForegroundColor(Color::Green),
-                    style::Print(path),
+                    style::Print(relative_path),
                     style::ResetColor,
                     style::Print("\n\nContents:\n"),
                     style::Print(file),
@@ -175,20 +177,20 @@ impl FsWrite {
                 Ok(())
             },
             FsWrite::StrReplace { path, old_str, new_str } => {
-                let path = format_path(cwd, path);
-                let file = ctx.fs().read_to_string_sync(&path)?;
+                let relative_path = format_path(cwd, path);
+                let file = ctx.fs().read_to_string_sync(&relative_path)?;
                 // TODO: we should pass some additional lines as context before and after the file.
                 let (start_line, _) = match line_number_at(&file, old_str) {
                     Some((start_line, end_line)) => (Some(start_line), Some(end_line)),
                     _ => (None, None),
                 };
-                let old_str = stylize_output_if_able(ctx, &path, old_str, start_line, Some("-"));
-                let new_str = stylize_output_if_able(ctx, &path, new_str, start_line, Some("+"));
+                let old_str = stylize_output_if_able(ctx, &relative_path, old_str, start_line, Some("-"));
+                let new_str = stylize_output_if_able(ctx, &relative_path, new_str, start_line, Some("+"));
                 queue!(
                     updates,
                     style::Print("Path: "),
                     style::SetForegroundColor(Color::Green),
-                    style::Print(path),
+                    style::Print(relative_path),
                     style::ResetColor,
                     style::Print("\n\n"),
                     style::Print("Replacing:\n"),
@@ -204,15 +206,16 @@ impl FsWrite {
         }
     }
 
-    pub async fn validate(&mut self, _ctx: &Context) -> Result<()> {
+    pub async fn validate(&mut self, ctx: &Context) -> Result<()> {
         match self {
             FsWrite::Create { path, .. } => {
                 if path.is_empty() {
                     bail!("Path must not be empty")
                 };
             },
             FsWrite::StrReplace { path, .. } | FsWrite::Insert { path, .. } => {
-                if !PathBuf::from(&path).exists() {
+                let path = sanitize_path_tool_arg(ctx, path);
+                if !path.exists() {
                     bail!("The provided path must exist in order to replace or insert contents into it")
                 }
             },

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

@@ -67,6 +67,16 @@ impl Tool {
         }
     }
 
+    /// Whether or not the tool should prompt the user for consent before [Self::invoke] is called.
+    pub fn requires_consent(&self, _ctx: &Context) -> bool {
+        match self {
+            Tool::FsRead(_) => false,
+            Tool::FsWrite(_) => true,
+            Tool::ExecuteBash(_) => true,
+            Tool::UseAws(_) => false,
+        }
+    }
+
     /// Invokes the tool asynchronously
     pub async fn invoke(&self, context: &Context, updates: &mut impl Write) -> Result<InvokeOutput> {
         match self {
@@ -185,9 +195,26 @@ pub fn serde_value_to_document(value: serde_json::Value) -> Document {
     }
 }
 
+/// Performs tilde expansion and other required sanitization modifications for handling tool use
+/// path arguments.
+///
+/// Required since path arguments are defined by the model.
 #[allow(dead_code)]
-fn path_expand(_ctx: &Context, _path: impl AsRef<Path>) -> PathBuf {
-    PathBuf::new()
+fn sanitize_path_tool_arg(ctx: &Context, path: impl AsRef<Path>) -> PathBuf {
+    let mut res = PathBuf::new();
+    // Expand `~` only if it is the first part.
+    let mut path = path.as_ref().components();
+    match path.next() {
+        Some(p) if p.as_os_str() == "~" => {
+            res.push(ctx.env().home().unwrap_or_default());
+        },
+        Some(p) => res.push(p),
+        None => return res,
+    }
+    res.push(path);
+    // For testing scenarios, we need to make sure paths are appropriately handled in chroot test
+    // file systems since they are passed directly from the model.
+    ctx.fs().chroot_path(res)
 }
 
 /// Converts `path` to a relative path according to the current working directory `cwd`.
@@ -341,6 +368,8 @@ fn stylized_file(
 
 #[cfg(test)]
 mod tests {
+    use fig_os_shim::EnvProvider;
+
     use super::*;
 
     #[test]
@@ -352,4 +381,28 @@ mod tests {
         assert_eq!(terminal_width(100), 3);
         assert_eq!(terminal_width(999), 3);
     }
+
+    #[tokio::test]
+    async fn test_tilde_path_expansion() {
+        let ctx = Context::builder().with_test_home().await.unwrap().build_fake();
+
+        let actual = sanitize_path_tool_arg(&ctx, "~");
+        assert_eq!(
+            actual,
+            ctx.fs().chroot_path(ctx.env().home().unwrap()),
+            "tilde should expand"
+        );
+        let actual = sanitize_path_tool_arg(&ctx, "~/hello");
+        assert_eq!(
+            actual,
+            ctx.fs().chroot_path(ctx.env().home().unwrap().join("hello")),
+            "tilde should expand"
+        );
+        let actual = sanitize_path_tool_arg(&ctx, "/~");
+        assert_eq!(
+            actual,
+            ctx.fs().chroot_path("/~"),
+            "tilde should not expand when not the first component"
+        );
+    }
 }

crates/q_cli/src/cli/chat/tools/mod.rs

@@ -66,6 +66,8 @@ impl UseAws {
         command.arg(&self.service_name).arg(&self.operation_name);
         if let Some(parameters) = &self.parameters {
             for (param_name, val) in parameters {
+                // Model might sometimes give parameters capitalized.
+                let param_name = param_name.to_lowercase();
                 if param_name.starts_with("--") {
                     command.arg(param_name).arg(val);
                 } else {