From cc78763762e4bd9f8e75075c710bcb05edf62b6d Mon Sep 17 00:00:00 2001 From: Yingjie He Date: Thu, 27 Feb 2025 10:04:17 -0800 Subject: [PATCH] feat: allow user to turn off smart approve (#1407) --- crates/goose-cli/src/commands/configure.rs | 15 ++++------ crates/goose/src/agents/truncate.rs | 6 +++- crates/goose/src/config/experiments.rs | 34 +++++++++------------- 3 files changed, 24 insertions(+), 31 deletions(-) diff --git a/crates/goose-cli/src/commands/configure.rs b/crates/goose-cli/src/commands/configure.rs index f1b639d5..9c49e54d 100644 --- a/crates/goose-cli/src/commands/configure.rs +++ b/crates/goose-cli/src/commands/configure.rs @@ -622,24 +622,19 @@ pub fn remove_extension_dialog() -> Result<(), Box> { } pub fn configure_settings_dialog() -> Result<(), Box> { - let mut setting_select_builder = cliclack::select("What setting would you like to configure?") + let setting_type = cliclack::select("What setting would you like to configure?") .item("goose_mode", "Goose Mode", "Configure Goose mode") .item( "tool_output", "Tool Output", "Show more or less tool output", - ); - - // Conditionally add the "Toggle Experiment" option - if ExperimentManager::is_enabled("EXPERIMENT_CONFIG")? { - setting_select_builder = setting_select_builder.item( + ) + .item( "experiment", "Toggle Experiment", "Enable or disable an experiment feature", - ); - } - - let setting_type = setting_select_builder.interact()?; + ) + .interact()?; match setting_type { "goose_mode" => { diff --git a/crates/goose/src/agents/truncate.rs b/crates/goose/src/agents/truncate.rs index 9f8ebc6c..8bf581d6 100644 --- a/crates/goose/src/agents/truncate.rs +++ b/crates/goose/src/agents/truncate.rs @@ -11,6 +11,7 @@ use super::Agent; use crate::agents::capabilities::Capabilities; use crate::agents::extension::{ExtensionConfig, ExtensionResult}; use crate::config::Config; +use crate::config::ExperimentManager; use crate::message::{Message, ToolRequest}; use crate::providers::base::Provider; use crate::providers::base::ProviderUsage; @@ -244,8 +245,11 @@ impl Agent for TruncateAgent { let mode = goose_mode.clone(); match mode.as_str() { "approve" => { + let mut read_only_tools = Vec::new(); // Process each tool request sequentially with confirmation - let read_only_tools = detect_read_only_tools(&capabilities, tool_requests.clone()).await; + if ExperimentManager::is_enabled("GOOSE_SMART_APPROVE")? { + read_only_tools = detect_read_only_tools(&capabilities, tool_requests.clone()).await; + } for request in &tool_requests { if let Ok(tool_call) = request.tool_call.clone() { // Skip confirmation if the tool_call.name is in the read_only_tools list diff --git a/crates/goose/src/config/experiments.rs b/crates/goose/src/config/experiments.rs index 158ed3a5..c695fde4 100644 --- a/crates/goose/src/config/experiments.rs +++ b/crates/goose/src/config/experiments.rs @@ -5,7 +5,7 @@ use std::collections::HashMap; /// It is the ground truth for init experiments. The experiment names in users' experiment list but not /// in the list will be remove from user list; The experiment names in the ground-truth list but not /// in users' experiment list will be added to user list with default value false; -const ALL_EXPERIMENTS: &[(&str, bool)] = &[("EXPERIMENT_CONFIG", false)]; +const ALL_EXPERIMENTS: &[(&str, bool)] = &[("GOOSE_SMART_APPROVE", true)]; /// Experiment configuration management pub struct ExperimentManager; @@ -19,14 +19,7 @@ impl ExperimentManager { pub fn get_all() -> Result> { let config = Config::global(); let mut experiments: HashMap = config.get("experiments").unwrap_or_default(); - - // Synchronize the user's experiments with the ground truth (`ALL_EXPERIMENTS`) - for &(key, default_value) in ALL_EXPERIMENTS { - experiments.entry(key.to_string()).or_insert(default_value); - } - - // Remove experiments not in `ALL_EXPERIMENTS` - experiments.retain(|key, _| ALL_EXPERIMENTS.iter().any(|(k, _)| k == key)); + Self::refresh_experiments(&mut experiments); Ok(experiments.into_iter().collect()) } @@ -34,28 +27,29 @@ impl ExperimentManager { /// Enable or disable an experiment pub fn set_enabled(name: &str, enabled: bool) -> Result<()> { let config = Config::global(); - - // Load existing experiments or initialize a new map let mut experiments: HashMap = config.get("experiments").unwrap_or_else(|_| HashMap::new()); - - // Update the status of the experiment + Self::refresh_experiments(&mut experiments); experiments.insert(name.to_string(), enabled); - // Save the updated experiments map config.set("experiments", serde_json::to_value(experiments)?)?; Ok(()) } /// Check if an experiment is enabled pub fn is_enabled(name: &str) -> Result { - let config = Config::global(); + let experiments = Self::get_all()?; + let experiments_map: HashMap = experiments.into_iter().collect(); + Ok(*experiments_map.get(name).unwrap_or(&false)) + } - // Load existing experiments or initialize a new map - let experiments: HashMap = - config.get("experiments").unwrap_or_else(|_| HashMap::new()); + fn refresh_experiments(experiments: &mut HashMap) { + // Add missing experiments from `ALL_EXPERIMENTS` + for &(key, default_value) in ALL_EXPERIMENTS { + experiments.entry(key.to_string()).or_insert(default_value); + } - // Return whether the experiment is enabled, defaulting to false - Ok(*experiments.get(name).unwrap_or(&false)) + // Remove experiments not present in `ALL_EXPERIMENTS` + experiments.retain(|key, _| ALL_EXPERIMENTS.iter().any(|(k, _)| k == key)); } }