From 10e665a9b9caf3a6bcfa6e6296002afcdef24299 Mon Sep 17 00:00:00 2001 From: morgmart <98432065+morgmart@users.noreply.github.com> Date: Wed, 10 Jun 2026 02:20:14 -0700 Subject: [PATCH] expose ACP thinking effort config option (#9711) Signed-off-by: morgmart <98432065+morgmart@users.noreply.github.com> Co-authored-by: Lifei Zhou --- crates/goose-sdk-types/src/custom_requests.rs | 1 + crates/goose/acp-schema.json | 1 + crates/goose/src/acp/response_builder.rs | 154 ++++++++++++++- crates/goose/src/acp/server.rs | 112 +++-------- crates/goose/src/acp/server/config.rs | 63 ++++--- crates/goose/src/acp/server/dispatch.rs | 6 + crates/goose/src/agents/agent.rs | 49 +++++ crates/goose/src/model.rs | 178 ++++++++++++++++++ .../goose/tests/acp_custom_requests_test.rs | 36 ++-- crates/goose/tests/acp_server_test.rs | 54 +++++- ui/sdk/src/generated/types.gen.ts | 2 +- ui/sdk/src/generated/zod.gen.ts | 1 + 12 files changed, 527 insertions(+), 130 deletions(-) diff --git a/crates/goose-sdk-types/src/custom_requests.rs b/crates/goose-sdk-types/src/custom_requests.rs index 3ef2df1ce..c69654032 100644 --- a/crates/goose-sdk-types/src/custom_requests.rs +++ b/crates/goose-sdk-types/src/custom_requests.rs @@ -314,6 +314,7 @@ pub struct PreferencesRemoveRequest { pub enum PreferenceKey { #[default] AutoCompactThreshold, + GooseThinkingEffort, VoiceAutoSubmitPhrases, VoiceDictationProvider, VoiceDictationPreferredMic, diff --git a/crates/goose/acp-schema.json b/crates/goose/acp-schema.json index ebad7d1bc..1cdb6a3e9 100644 --- a/crates/goose/acp-schema.json +++ b/crates/goose/acp-schema.json @@ -1915,6 +1915,7 @@ "type": "string", "enum": [ "autoCompactThreshold", + "gooseThinkingEffort", "voiceAutoSubmitPhrases", "voiceDictationProvider", "voiceDictationPreferredMic" diff --git a/crates/goose/src/acp/response_builder.rs b/crates/goose/src/acp/response_builder.rs index d108aa117..dfa7bfb7b 100644 --- a/crates/goose/src/acp/response_builder.rs +++ b/crates/goose/src/acp/response_builder.rs @@ -1,4 +1,5 @@ use crate::config::GooseMode; +use crate::model::ModelConfig; use crate::providers::inventory::{ProviderInventoryEntry, ProviderInventoryService}; use crate::session::Session; use agent_client_protocol::schema::{ @@ -8,6 +9,7 @@ use agent_client_protocol::schema::{ SessionUpdate, UnstructuredCommandInput, }; use agent_client_protocol::{Client, ConnectionTo}; +use goose_providers::thinking::ThinkingEffort; use strum::{EnumMessage, VariantNames}; use super::server::{build_usage_updates, DEFAULT_PROVIDER_ID, DEFAULT_PROVIDER_LABEL}; @@ -146,6 +148,7 @@ pub(super) async fn build_session_setup_config( let config_options = build_config_options( &mode_state, &model_state, + model_config, provider_selection, provider_options, ); @@ -155,6 +158,7 @@ pub(super) async fn build_session_setup_config( pub(super) fn build_config_options( mode_state: &SessionModeState, model_state: &SessionModelState, + model_config: &ModelConfig, provider_selection: &str, provider_options: Vec, ) -> Vec { @@ -171,6 +175,14 @@ pub(super) fn build_config_options( .iter() .map(|m| SessionConfigSelectOption::new(m.model_id.0.clone(), m.name.clone())) .collect(); + let thinking_effort_options = thinking_effort_values(model_config) + .iter() + .map(|effort| { + let effort = effort.to_string(); + SessionConfigSelectOption::new(effort.clone(), effort) + }) + .collect::>(); + let current_thinking_effort = current_thinking_effort_value(model_config); vec![ SessionConfigOption::select( "provider", @@ -192,9 +204,42 @@ pub(super) fn build_config_options( model_options, ) .category(SessionConfigOptionCategory::Model), + SessionConfigOption::select( + "thinking_effort", + "Thinking effort", + current_thinking_effort, + thinking_effort_options, + ) + .description("Controls reasoning effort for models that support extended thinking.") + .category(SessionConfigOptionCategory::ThoughtLevel), ] } +fn thinking_effort_values(model_config: &ModelConfig) -> &'static [ThinkingEffort] { + if model_config.is_reasoning_model() { + &[ + ThinkingEffort::Off, + ThinkingEffort::Low, + ThinkingEffort::Medium, + ThinkingEffort::High, + ThinkingEffort::Max, + ] + } else { + &[ThinkingEffort::Off] + } +} + +fn current_thinking_effort_value(model_config: &ModelConfig) -> String { + if model_config.is_reasoning_model() { + model_config + .thinking_effort() + .map(|effort| effort.to_string()) + .unwrap_or_else(|| "off".to_string()) + } else { + "off".to_string() + } +} + fn available_commands_update(working_dir: &std::path::Path) -> AvailableCommandsUpdate { let commands = crate::slash_commands::slash_command::list_acp_commands(Some(working_dir)) .into_iter() @@ -236,6 +281,7 @@ pub(super) fn send_session_setup_notifications( #[cfg(test)] mod tests { use super::*; + use agent_client_protocol::schema::SessionConfigKind; use test_case::test_case; #[test_case( @@ -363,6 +409,12 @@ mod tests { SessionConfigSelectOption::new("gpt-3.5", "gpt-3.5"), ], ).category(SessionConfigOptionCategory::Model), + SessionConfigOption::select( + "thinking_effort", "Thinking effort", "off", + vec![SessionConfigSelectOption::new("off", "off")], + ) + .description("Controls reasoning effort for models that support extended thinking.") + .category(SessionConfigOptionCategory::ThoughtLevel), ] ; "auto mode with multiple models" )] @@ -389,6 +441,12 @@ mod tests { "model", "Model", "only-model", vec![SessionConfigSelectOption::new("only-model", "only-model")], ).category(SessionConfigOptionCategory::Model), + SessionConfigOption::select( + "thinking_effort", "Thinking effort", "off", + vec![SessionConfigSelectOption::new("off", "off")], + ) + .description("Controls reasoning effort for models that support extended thinking.") + .category(SessionConfigOptionCategory::ThoughtLevel), ] ; "approve mode with single model" )] @@ -398,6 +456,100 @@ mod tests { provider_options: Vec, model_state: SessionModelState, ) -> Vec { - build_config_options(&mode_state, &model_state, provider_name, provider_options) + let model_config = ModelConfig { + model_name: model_state.current_model_id.0.to_string(), + request_params: Some(std::collections::HashMap::from([( + "thinking_effort".to_string(), + serde_json::json!("off"), + )])), + ..Default::default() + }; + build_config_options( + &mode_state, + &model_state, + &model_config, + provider_name, + provider_options, + ) + } + + #[test] + fn test_build_config_options_uses_current_thinking_effort() { + let mode_state = build_mode_state(GooseMode::Auto).unwrap(); + let model_state = SessionModelState::new( + ModelId::new("claude-sonnet-4"), + vec![ModelInfo::new( + ModelId::new("claude-sonnet-4"), + "claude-sonnet-4", + )], + ); + let model_config = ModelConfig { + model_name: "claude-sonnet-4".to_string(), + request_params: Some(std::collections::HashMap::from([( + "thinking_effort".to_string(), + serde_json::json!("high"), + )])), + ..Default::default() + }; + + let options = build_config_options( + &mode_state, + &model_state, + &model_config, + "openai", + vec![SessionConfigSelectOption::new("openai", "openai")], + ); + let option = options + .iter() + .find(|option| option.id.0.as_ref() == "thinking_effort") + .expect("thinking_effort option"); + let select = match &option.kind { + SessionConfigKind::Select(select) => select, + _ => panic!("thinking_effort should be a select option"), + }; + + assert_eq!(select.current_value.0.as_ref(), "high"); + } + + #[test] + fn test_build_config_options_masks_non_reasoning_thinking_effort() { + let mode_state = build_mode_state(GooseMode::Auto).unwrap(); + let model_state = SessionModelState::new( + ModelId::new("gpt-4"), + vec![ModelInfo::new(ModelId::new("gpt-4"), "gpt-4")], + ); + let model_config = ModelConfig { + model_name: "gpt-4".to_string(), + request_params: Some(std::collections::HashMap::from([( + "thinking_effort".to_string(), + serde_json::json!("high"), + )])), + reasoning: Some(false), + ..Default::default() + }; + + let options = build_config_options( + &mode_state, + &model_state, + &model_config, + "openai", + vec![SessionConfigSelectOption::new("openai", "openai")], + ); + let option = options + .iter() + .find(|option| option.id.0.as_ref() == "thinking_effort") + .expect("thinking_effort option"); + let select = match &option.kind { + SessionConfigKind::Select(select) => select, + _ => panic!("thinking_effort should be a select option"), + }; + + assert_eq!(select.current_value.0.as_ref(), "off"); + assert_eq!( + select.options, + agent_client_protocol::schema::SessionConfigSelectOptions::Ungrouped(vec![ + SessionConfigSelectOption::new("off", "off") + ]) + ); } } diff --git a/crates/goose/src/acp/server.rs b/crates/goose/src/acp/server.rs index 783ef7dd7..023ffc231 100644 --- a/crates/goose/src/acp/server.rs +++ b/crates/goose/src/acp/server.rs @@ -908,34 +908,6 @@ fn builtin_to_extension_config(name: &str) -> ExtensionConfig { } } -fn with_preserved_session_request_params( - mut model_config: crate::model::ModelConfig, - current_model_config: Option<&crate::model::ModelConfig>, - request_params: Option>, -) -> crate::model::ModelConfig { - let has_model_effort = model_config - .request_params - .as_ref() - .and_then(|params| params.get("thinking_effort")) - .is_some(); - if !has_model_effort { - if let Some(thinking_effort) = current_model_config - .and_then(|config| config.request_params.as_ref()) - .and_then(|params| params.get("thinking_effort")) - .cloned() - { - model_config = model_config.with_merged_request_params(HashMap::from([( - "thinking_effort".into(), - thinking_effort, - )])); - } - } - if let Some(request_params) = request_params { - model_config = model_config.with_merged_request_params(request_params); - } - model_config -} - fn to_nonnegative_u64(value: Option) -> Option { value.and_then(|v| u64::try_from(v).ok()) } @@ -2634,7 +2606,6 @@ impl GooseAcpAgent { session_id: &str, model_id: &str, ) -> Result { - let config = self.config()?; let agent = self.get_session_agent(session_id, None).await?; let current_provider = agent .provider() @@ -2642,36 +2613,15 @@ impl GooseAcpAgent { .internal_err_ctx("Failed to get provider")?; let provider_name = current_provider.get_name().to_string(); let current_model_config = current_provider.get_model_config(); - let extensions = - EnabledExtensionsState::for_session(&self.session_manager, session_id, config).await; let model_config = crate::model::ModelConfig::new(model_id) .invalid_params_err_ctx("Invalid model config")? .with_canonical_limits(&provider_name); let model_config = - with_preserved_session_request_params(model_config, Some(¤t_model_config), None); - let session = self - .session_manager - .get_session(session_id, false) - .await - .internal_err_ctx("Failed to get session")?; - let provider = self - .create_provider( - &provider_name, - model_config, - extensions, - Some(session.working_dir), - ) - .await - .internal_err_ctx("Failed to create provider")?; + model_config.with_inherited_session_settings_from(Some(¤t_model_config), None); agent - .update_provider(provider, session_id) + .recreate_provider_for_session(session_id, &provider_name, model_config) .await - .internal_err_ctx("Failed to update provider")?; - let mode = agent.goose_mode().await; - agent - .update_goose_mode(mode, session_id) - .await - .internal_err_ctx("Failed to propagate mode")?; + .internal_err_ctx("Failed to recreate provider")?; // model_config is already updated on the session by the agent's update_provider call. Ok(SetSessionModelResponse::new()) } @@ -2691,7 +2641,8 @@ impl GooseAcpAgent { .await .internal_err_ctx("Failed to get provider")?; let provider_name = provider.get_name().to_string(); - let current_model = provider.get_model_config().model_name.clone(); + let current_model_config = provider.get_model_config(); + let current_model = current_model_config.model_name.clone(); let goose_mode = agent.goose_mode().await; let inventory = self .provider_inventory @@ -2708,6 +2659,7 @@ impl GooseAcpAgent { let config_options = build_config_options( &mode_state, &model_state, + ¤t_model_config, session_provider_selection(&session), provider_options, ); @@ -2739,6 +2691,26 @@ impl GooseAcpAgent { Ok(SetSessionModeResponse::new()) } + async fn on_set_thinking_effort( + &self, + session_id: &str, + effort_id: &str, + ) -> Result<(), agent_client_protocol::Error> { + let effort = effort_id + .parse::() + .map_err(|_| { + agent_client_protocol::Error::invalid_params() + .data(format!("Invalid thinking effort: {}", effort_id)) + })?; + let agent = self.get_session_agent(session_id, None).await?; + agent + .update_thinking_effort(session_id, effort) + .await + .internal_err_ctx("Failed to update thinking effort")?; + + Ok(()) + } + async fn update_provider( &self, session_id: &str, @@ -2781,37 +2753,13 @@ impl GooseAcpAgent { .invalid_params_err_ctx("Invalid model config")? .with_canonical_limits(&resolved_provider_name) .with_context_limit(context_limit); - model_config = with_preserved_session_request_params( - model_config, - (!is_changing_provider).then_some(¤t_model_config), - request_params, - ); + model_config = model_config + .with_inherited_session_settings_from(Some(¤t_model_config), request_params); - let extensions = - EnabledExtensionsState::for_session(&self.session_manager, session_id, config).await; - let session = self - .session_manager - .get_session(session_id, false) - .await - .internal_err_ctx("Failed to get session")?; - let new_provider = self - .create_provider( - &resolved_provider_name, - model_config, - extensions, - Some(session.working_dir), - ) - .await - .internal_err_ctx("Failed to create provider")?; agent - .update_provider(new_provider, session_id) + .recreate_provider_for_session(session_id, &resolved_provider_name, model_config) .await - .internal_err_ctx("Failed to update provider")?; - let mode = agent.goose_mode().await; - agent - .update_goose_mode(mode, session_id) - .await - .internal_err_ctx("Failed to propagate mode")?; + .internal_err_ctx("Failed to recreate provider")?; // provider_name is already updated on the session by the agent's update_provider call. Ok(()) diff --git a/crates/goose/src/acp/server/config.rs b/crates/goose/src/acp/server/config.rs index 1657e79e5..654af78c4 100644 --- a/crates/goose/src/acp/server/config.rs +++ b/crates/goose/src/acp/server/config.rs @@ -1,4 +1,5 @@ use super::*; +use goose_providers::thinking::ThinkingEffort; impl GooseAcpAgent { pub(super) async fn on_preferences_read( @@ -37,8 +38,8 @@ impl GooseAcpAgent { for preference in &req.values { let def = preference_def(preference.key)?; - (def.validate)(&preference.value)?; - updates.push((def.config_key.to_string(), preference.value.clone())); + let value = (def.prepare)(&preference.value)?; + updates.push((def.config_key.to_string(), value)); } config.set_param_values(&updates).internal_err()?; @@ -131,29 +132,34 @@ impl GooseAcpAgent { struct PreferenceDef { key: PreferenceKey, config_key: &'static str, - validate: fn(&serde_json::Value) -> Result<(), agent_client_protocol::Error>, + prepare: fn(&serde_json::Value) -> Result, } const PREFERENCE_DEFS: &[PreferenceDef] = &[ PreferenceDef { key: PreferenceKey::AutoCompactThreshold, config_key: "GOOSE_AUTO_COMPACT_THRESHOLD", - validate: validate_auto_compact_threshold, + prepare: prepare_auto_compact_threshold, + }, + PreferenceDef { + key: PreferenceKey::GooseThinkingEffort, + config_key: "GOOSE_THINKING_EFFORT", + prepare: prepare_thinking_effort, }, PreferenceDef { key: PreferenceKey::VoiceAutoSubmitPhrases, config_key: "VOICE_AUTO_SUBMIT_PHRASES", - validate: validate_voice_auto_submit_phrases, + prepare: prepare_voice_auto_submit_phrases, }, PreferenceDef { key: PreferenceKey::VoiceDictationProvider, config_key: "VOICE_DICTATION_PROVIDER", - validate: validate_voice_dictation_provider, + prepare: prepare_voice_dictation_provider, }, PreferenceDef { key: PreferenceKey::VoiceDictationPreferredMic, config_key: "VOICE_DICTATION_PREFERRED_MIC", - validate: validate_voice_dictation_preferred_mic, + prepare: prepare_voice_dictation_preferred_mic, }, ]; @@ -169,35 +175,50 @@ fn preference_def( }) } -fn validate_auto_compact_threshold( +fn prepare_auto_compact_threshold( value: &serde_json::Value, -) -> Result<(), agent_client_protocol::Error> { - let Some(value) = value.as_f64() else { +) -> Result { + let Some(threshold) = value.as_f64() else { return Err(agent_client_protocol::Error::invalid_params() .data("autoCompactThreshold must be a number")); }; - if !value.is_finite() || value <= 0.0 || value > 1.0 { + if !threshold.is_finite() || threshold <= 0.0 || threshold > 1.0 { return Err(agent_client_protocol::Error::invalid_params() .data("autoCompactThreshold must be greater than 0 and at most 1")); } - Ok(()) + Ok(value.clone()) } -fn validate_voice_auto_submit_phrases( +fn prepare_thinking_effort( value: &serde_json::Value, -) -> Result<(), agent_client_protocol::Error> { +) -> Result { + let Some(value) = value.as_str() else { + return Err(agent_client_protocol::Error::invalid_params() + .data("gooseThinkingEffort must be a string")); + }; + let effort = value.parse::().map_err(|err| { + agent_client_protocol::Error::invalid_params() + .data(format!("Invalid gooseThinkingEffort: {err}")) + })?; + + Ok(serde_json::Value::String(effort.to_string())) +} + +fn prepare_voice_auto_submit_phrases( + value: &serde_json::Value, +) -> Result { if !value.is_string() { return Err(agent_client_protocol::Error::invalid_params() .data("voiceAutoSubmitPhrases must be a string")); } - Ok(()) + Ok(value.clone()) } -fn validate_voice_dictation_provider( +fn prepare_voice_dictation_provider( value: &serde_json::Value, -) -> Result<(), agent_client_protocol::Error> { +) -> Result { let Some(value) = value.as_str() else { return Err(agent_client_protocol::Error::invalid_params() .data("voiceDictationProvider must be a string")); @@ -207,12 +228,12 @@ fn validate_voice_dictation_provider( .data("voiceDictationProvider is not supported")); } - Ok(()) + Ok(serde_json::Value::String(value.to_string())) } -fn validate_voice_dictation_preferred_mic( +fn prepare_voice_dictation_preferred_mic( value: &serde_json::Value, -) -> Result<(), agent_client_protocol::Error> { +) -> Result { let Some(value) = value.as_str() else { return Err(agent_client_protocol::Error::invalid_params() .data("voiceDictationPreferredMic must be a string")); @@ -222,7 +243,7 @@ fn validate_voice_dictation_preferred_mic( .data("voiceDictationPreferredMic must be non-empty")); } - Ok(()) + Ok(serde_json::Value::String(value.to_string())) } fn is_supported_voice_dictation_provider(value: &str) -> bool { diff --git a/crates/goose/src/acp/server/dispatch.rs b/crates/goose/src/acp/server/dispatch.rs index b02e03511..2e622f389 100644 --- a/crates/goose/src/acp/server/dispatch.rs +++ b/crates/goose/src/acp/server/dispatch.rs @@ -143,6 +143,12 @@ impl HandleDispatchFrom for GooseAcpHandler { Err(e) => { responder.respond_with_error(e)?; return Ok(()); } } } + "thinking_effort" => { + match agent.on_set_thinking_effort(&session_id.0, &value_id.0).await { + Ok(_) => {} + Err(e) => { responder.respond_with_error(e)?; return Ok(()); } + } + } other => { responder.respond_with_error( agent_client_protocol::Error::invalid_params().data(format!("Unsupported config option: {}", other)) diff --git a/crates/goose/src/agents/agent.rs b/crates/goose/src/agents/agent.rs index d9d381d49..c77b39b1a 100644 --- a/crates/goose/src/agents/agent.rs +++ b/crates/goose/src/agents/agent.rs @@ -54,6 +54,7 @@ use crate::tool_inspection::ToolInspectionManager; use crate::tool_monitor::RepetitionInspector; use crate::utils::is_token_cancelled; use goose_providers::errors::ProviderError; +use goose_providers::thinking::ThinkingEffort; use regex::Regex; use rmcp::model::{ CallToolRequestParams, CallToolResult, Content, ErrorCode, ErrorData, GetPromptResult, Prompt, @@ -2490,6 +2491,54 @@ impl Agent { *self.current_goose_mode.lock().await } + pub async fn recreate_provider_for_session( + &self, + session_id: &str, + provider_name: &str, + model_config: crate::model::ModelConfig, + ) -> Result<()> { + let session = self + .config + .session_manager + .get_session(session_id, false) + .await + .context("Failed to get session")?; + + let extensions = EnabledExtensionsState::extensions_or_default( + Some(&session.extension_data), + Config::global(), + ); + + let provider = crate::providers::create_with_working_dir( + provider_name, + model_config, + extensions, + session.working_dir.clone(), + ) + .await + .map_err(|e| anyhow!("Could not create provider: {}", e))?; + + self.update_provider(provider, session_id).await?; + + let mode = self.goose_mode().await; + self.update_goose_mode(mode, session_id).await + } + + pub async fn update_thinking_effort( + &self, + session_id: &str, + effort: ThinkingEffort, + ) -> Result<()> { + let current_provider = self.provider().await?; + let provider_name = current_provider.get_name().to_string(); + let model_config = current_provider + .get_model_config() + .with_thinking_effort(effort); + + self.recreate_provider_for_session(session_id, &provider_name, model_config) + .await + } + /// Restore the provider from session data or fall back to global config /// This is used when resuming a session to restore the provider state /// Returns true if the session's provider was replaced with a fallback. diff --git a/crates/goose/src/model.rs b/crates/goose/src/model.rs index 739777cad..3c1c08a72 100644 --- a/crates/goose/src/model.rs +++ b/crates/goose/src/model.rs @@ -359,6 +359,47 @@ impl ModelConfig { self } + pub fn with_thinking_effort(mut self, effort: ThinkingEffort) -> Self { + let params = self.request_params.get_or_insert_with(HashMap::new); + params.insert( + "thinking_effort".to_string(), + serde_json::json!(effort.to_string()), + ); + self + } + + pub fn with_inherited_session_settings_from( + mut self, + previous: Option<&ModelConfig>, + request_params: Option>, + ) -> Self { + if let Some(previous) = previous { + let has_thinking_effort = self + .request_params + .as_ref() + .and_then(|params| params.get("thinking_effort")) + .is_some(); + + if !has_thinking_effort { + if let Some(thinking_effort) = previous + .request_params + .as_ref() + .and_then(|params| params.get("thinking_effort")) + .cloned() + { + let params = self.request_params.get_or_insert_with(HashMap::new); + params.insert("thinking_effort".to_string(), thinking_effort); + } + } + } + + if let Some(request_params) = request_params { + self = self.with_merged_request_params(request_params); + } + + self + } + pub fn use_fast_model(&self) -> Self { if let Some(fast_config) = &self.fast_model_config { *fast_config.clone() @@ -665,6 +706,143 @@ mod tests { assert_eq!(config.thinking_effort(), Some(ThinkingEffort::Low)); } + #[test] + fn with_thinking_effort_sets_request_param() { + let config = ModelConfig { + model_name: "test".to_string(), + ..Default::default() + } + .with_thinking_effort(ThinkingEffort::High); + + assert_eq!( + config + .request_params + .as_ref() + .and_then(|params| params.get("thinking_effort")), + Some(&serde_json::json!("high")) + ); + } + + #[test] + fn preserves_explicit_thinking_effort() { + let previous = ModelConfig { + model_name: "previous".to_string(), + request_params: Some(HashMap::from([( + "thinking_effort".to_string(), + serde_json::json!("high"), + )])), + ..Default::default() + }; + let config = ModelConfig { + model_name: "next".to_string(), + ..Default::default() + } + .with_inherited_session_settings_from(Some(&previous), None); + + assert_eq!( + config + .request_params + .as_ref() + .and_then(|params| params.get("thinking_effort")), + Some(&serde_json::json!("high")) + ); + } + + #[test] + fn does_not_override_existing_thinking_effort() { + let previous = ModelConfig { + model_name: "previous".to_string(), + request_params: Some(HashMap::from([( + "thinking_effort".to_string(), + serde_json::json!("high"), + )])), + ..Default::default() + }; + let config = ModelConfig { + model_name: "next".to_string(), + request_params: Some(HashMap::from([( + "thinking_effort".to_string(), + serde_json::json!("low"), + )])), + ..Default::default() + } + .with_inherited_session_settings_from(Some(&previous), None); + + assert_eq!( + config + .request_params + .as_ref() + .and_then(|params| params.get("thinking_effort")), + Some(&serde_json::json!("low")) + ); + } + + #[test] + fn does_not_preserve_unrelated_request_params() { + let previous = ModelConfig { + model_name: "previous".to_string(), + request_params: Some(HashMap::from([( + "provider_specific".to_string(), + serde_json::json!("old"), + )])), + ..Default::default() + }; + let config = ModelConfig { + model_name: "next".to_string(), + ..Default::default() + } + .with_inherited_session_settings_from(Some(&previous), None); + + assert!(config.request_params.is_none()); + } + + #[test] + fn does_not_materialize_env_thinking_effort() { + let _guard = env_lock::lock_env([("GOOSE_THINKING_EFFORT", Some("high"))]); + let previous = ModelConfig { + model_name: "previous".to_string(), + ..Default::default() + }; + let config = ModelConfig { + model_name: "next".to_string(), + ..Default::default() + } + .with_inherited_session_settings_from(Some(&previous), None); + + assert!(config.request_params.is_none()); + } + + #[test] + fn explicit_request_params_override_preserved_session_settings() { + let previous = ModelConfig { + model_name: "previous".to_string(), + request_params: Some(HashMap::from([( + "thinking_effort".to_string(), + serde_json::json!("high"), + )])), + ..Default::default() + }; + let config = ModelConfig { + model_name: "next".to_string(), + ..Default::default() + } + .with_inherited_session_settings_from( + Some(&previous), + Some(HashMap::from([( + "thinking_effort".to_string(), + serde_json::json!("low"), + )])), + ); + + assert_eq!( + config + .request_params + .as_ref() + .and_then(|params| params.get("thinking_effort")), + Some(&serde_json::json!("low")) + ); + } + #[test] fn legacy_claude_thinking_type_fallback() { for value in ["enabled", "adaptive"] { diff --git a/crates/goose/tests/acp_custom_requests_test.rs b/crates/goose/tests/acp_custom_requests_test.rs index cf3388417..c70f83381 100644 --- a/crates/goose/tests/acp_custom_requests_test.rs +++ b/crates/goose/tests/acp_custom_requests_test.rs @@ -78,26 +78,6 @@ impl Provider for MockProvider { } } -fn mock_provider_factory() -> AcpProviderFactory { - Arc::new(|provider_name, model_config, _extensions, _working_dir| { - Box::pin(async move { - let recommended_models = match provider_name.as_str() { - "anthropic" => vec![ - "claude-3-7-sonnet-latest".to_string(), - "claude-3-5-haiku-latest".to_string(), - ], - _ => vec!["gpt-4o".to_string(), "o4-mini".to_string()], - }; - Ok(Arc::new(MockProvider { - name: provider_name, - model_config, - supported_models: recommended_models.clone(), - recommended_models, - }) as Arc) - }) - }) -} - #[test] #[serial] fn test_custom_get_tools() { @@ -366,7 +346,7 @@ fn test_custom_provider_inventory_includes_metadata() { #[serial] fn test_custom_preferences_read_save_remove() { let config_dir = write_acp_global_config( - "GOOSE_MODEL: gpt-4o\nGOOSE_PROVIDER: openai\nGOOSE_AUTO_COMPACT_THRESHOLD: 0.7\nVOICE_AUTO_SUBMIT_PHRASES: send it\n", + "GOOSE_MODEL: gpt-4o\nGOOSE_PROVIDER: openai\nGOOSE_AUTO_COMPACT_THRESHOLD: 0.7\nGOOSE_THINKING_EFFORT: high\nVOICE_AUTO_SUBMIT_PHRASES: send it\n", ); run_test(async move { @@ -383,6 +363,7 @@ fn test_custom_preferences_read_save_remove() { serde_json::json!({ "keys": [ "autoCompactThreshold", + "gooseThinkingEffort", "voiceAutoSubmitPhrases", "voiceDictationPreferredMic" ], @@ -394,6 +375,7 @@ fn test_custom_preferences_read_save_remove() { response.get("values"), Some(&serde_json::json!([ { "key": "autoCompactThreshold", "value": 0.7 }, + { "key": "gooseThinkingEffort", "value": "high" }, { "key": "voiceAutoSubmitPhrases", "value": "send it" }, { "key": "voiceDictationPreferredMic", "value": null }, ])) @@ -404,6 +386,7 @@ fn test_custom_preferences_read_save_remove() { "_goose/unstable/preferences/save", serde_json::json!({ "values": [ + { "key": "gooseThinkingEffort", "value": "disabled" }, { "key": "voiceDictationProvider", "value": "__disabled__" }, { "key": "voiceDictationPreferredMic", "value": "mic-1" } ], @@ -426,7 +409,7 @@ fn test_custom_preferences_read_save_remove() { conn.cx(), "_goose/unstable/preferences/read", serde_json::json!({ - "keys": ["voiceDictationProvider", "voiceDictationPreferredMic"], + "keys": ["gooseThinkingEffort", "voiceDictationProvider", "voiceDictationPreferredMic"], }), ) .await @@ -434,6 +417,7 @@ fn test_custom_preferences_read_save_remove() { assert_eq!( response.get("values"), Some(&serde_json::json!([ + { "key": "gooseThinkingEffort", "value": "off" }, { "key": "voiceDictationProvider", "value": null }, { "key": "voiceDictationPreferredMic", "value": "mic-1" }, ])) @@ -456,6 +440,12 @@ fn test_custom_preferences_save_rejects_invalid_values() { serde_json::json!({ "values": [{ "key": "autoCompactThreshold", "value": 1.1 }], }), + serde_json::json!({ + "values": [{ "key": "gooseThinkingEffort", "value": "bogus" }], + }), + serde_json::json!({ + "values": [{ "key": "gooseThinkingEffort", "value": ["high"] }], + }), serde_json::json!({ "values": [{ "key": "voiceAutoSubmitPhrases", "value": ["send"] }], }), @@ -658,11 +648,11 @@ fn test_raw_config_and_secret_methods_are_removed() { #[test] #[serial] fn test_provider_switching_updates_session_state() { + let _env = env_lock::lock_env([("ANTHROPIC_API_KEY", Some("test-key"))]); write_acp_global_config(DEFAULT_ACP_TEST_CONFIG); run_test(async { let openai = OpenAiFixture::new(vec![], Arc::new(EnforceSessionId::default())).await; let config = TestConnectionConfig { - provider_factory: Some(mock_provider_factory()), current_model: "gpt-4o".to_string(), ..Default::default() }; diff --git a/crates/goose/tests/acp_server_test.rs b/crates/goose/tests/acp_server_test.rs index 22cbfa32d..708b144a7 100644 --- a/crates/goose/tests/acp_server_test.rs +++ b/crates/goose/tests/acp_server_test.rs @@ -1,10 +1,13 @@ #[allow(dead_code)] #[path = "acp_common_tests/mod.rs"] mod common_tests; -use agent_client_protocol::schema::{ListSessionsRequest, ListSessionsResponse}; +use agent_client_protocol::schema::{ + ListSessionsRequest, ListSessionsResponse, SessionConfigKind, SessionConfigOptionCategory, + SessionConfigOptionValue, SetSessionConfigOptionRequest, +}; use agent_client_protocol::ErrorCode; use common_tests::fixtures::server::AcpServerConnection; -use common_tests::fixtures::{run_test, Connection, OpenAiFixture, TestConnectionConfig}; +use common_tests::fixtures::{run_test, Connection, OpenAiFixture, Session, TestConnectionConfig}; #[cfg(feature = "code-mode")] use common_tests::run_prompt_codemode; use common_tests::{ @@ -176,6 +179,53 @@ fn test_config_option_model_set() { run_test(async { run_config_option_model_set::().await }); } +#[test] +fn test_config_option_thinking_effort_set() { + run_test(async { + let openai = OpenAiFixture::new( + vec![], + ::expected_session_id(), + ) + .await; + let mut conn = ::new( + TestConnectionConfig { + current_model: "claude-sonnet-4".to_string(), + ..Default::default() + }, + openai, + ) + .await; + let data = conn.new_session().await.unwrap(); + + let response = conn + .cx() + .send_request(SetSessionConfigOptionRequest::new( + data.session.session_id().clone(), + "thinking_effort".to_string(), + SessionConfigOptionValue::value_id("high".to_string()), + )) + .block_task() + .await + .unwrap(); + + let option = response + .config_options + .iter() + .find(|option| option.id.0.as_ref() == "thinking_effort") + .expect("thinking_effort option"); + assert_eq!( + option.category, + Some(SessionConfigOptionCategory::ThoughtLevel) + ); + let select = match &option.kind { + SessionConfigKind::Select(select) => select, + _ => panic!("thinking_effort should be a select option"), + }; + + assert_eq!(select.current_value.0.as_ref(), "high"); + }); +} + #[test] fn test_delete_session() { run_test(async { run_delete_session::().await }); diff --git a/ui/sdk/src/generated/types.gen.ts b/ui/sdk/src/generated/types.gen.ts index bf7bf3112..3dfe18d0c 100644 --- a/ui/sdk/src/generated/types.gen.ts +++ b/ui/sdk/src/generated/types.gen.ts @@ -794,7 +794,7 @@ export type PreferencesReadRequest_unstable = { keys?: Array; }; -export type PreferenceKey = 'autoCompactThreshold' | 'voiceAutoSubmitPhrases' | 'voiceDictationProvider' | 'voiceDictationPreferredMic'; +export type PreferenceKey = 'autoCompactThreshold' | 'gooseThinkingEffort' | 'voiceAutoSubmitPhrases' | 'voiceDictationProvider' | 'voiceDictationPreferredMic'; export type PreferencesReadResponse_unstable = { values: Array; diff --git a/ui/sdk/src/generated/zod.gen.ts b/ui/sdk/src/generated/zod.gen.ts index a73387f5d..24e0f637b 100644 --- a/ui/sdk/src/generated/zod.gen.ts +++ b/ui/sdk/src/generated/zod.gen.ts @@ -766,6 +766,7 @@ export const zProviderConfigAuthenticateRequest_unstable = z.object({ export const zPreferenceKey = z.enum([ 'autoCompactThreshold', + 'gooseThinkingEffort', 'voiceAutoSubmitPhrases', 'voiceDictationProvider', 'voiceDictationPreferredMic'