diff --git a/crates/goose-cli/src/commands/configure.rs b/crates/goose-cli/src/commands/configure.rs index ff25e66676..256f8b9639 100644 --- a/crates/goose-cli/src/commands/configure.rs +++ b/crates/goose-cli/src/commands/configure.rs @@ -2108,6 +2108,7 @@ fn add_provider() -> anyhow::Result<()> { requires_auth, catalog_provider_id: None, base_path, + preserves_thinking: None, })?; cliclack::outro(format!("Custom provider added: {}", display_name))?; diff --git a/crates/goose-sdk/src/custom_requests.rs b/crates/goose-sdk/src/custom_requests.rs index e9626d4589..4a8b52accf 100644 --- a/crates/goose-sdk/src/custom_requests.rs +++ b/crates/goose-sdk/src/custom_requests.rs @@ -704,6 +704,7 @@ pub struct CustomProviderConfigDto { #[serde(default, skip_serializing_if = "Option::is_none")] pub api_key_env: Option, pub api_key_set: bool, + pub preserves_thinking: bool, } #[derive(Debug, Default, Clone, Serialize, Deserialize, JsonSchema)] @@ -725,6 +726,8 @@ pub struct CustomProviderUpsertDto { pub catalog_provider_id: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub base_path: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub preserves_thinking: Option, } /// Create a custom provider backed by Goose's declarative provider store. diff --git a/crates/goose-server/src/routes/config_management.rs b/crates/goose-server/src/routes/config_management.rs index 1f93f5e298..e5203fe419 100644 --- a/crates/goose-server/src/routes/config_management.rs +++ b/crates/goose-server/src/routes/config_management.rs @@ -102,6 +102,8 @@ pub struct UpdateCustomProviderRequest { pub catalog_provider_id: Option, #[serde(default)] pub base_path: Option, + #[serde(default)] + pub preserves_thinking: Option, } fn default_requires_auth() -> bool { @@ -595,6 +597,7 @@ pub async fn create_custom_provider( requires_auth: request.requires_auth, catalog_provider_id: request.catalog_provider_id, base_path: request.base_path, + preserves_thinking: request.preserves_thinking, }, )?; @@ -687,6 +690,7 @@ pub async fn update_custom_provider( requires_auth: request.requires_auth, catalog_provider_id: request.catalog_provider_id, base_path: request.base_path, + preserves_thinking: request.preserves_thinking, }, )?; diff --git a/crates/goose/acp-schema.json b/crates/goose/acp-schema.json index fcace1e3c5..66edaf0c21 100644 --- a/crates/goose/acp-schema.json +++ b/crates/goose/acp-schema.json @@ -920,6 +920,12 @@ "string", "null" ] + }, + "preservesThinking": { + "type": [ + "boolean", + "null" + ] } }, "required": [ @@ -1111,6 +1117,9 @@ }, "apiKeySet": { "type": "boolean" + }, + "preservesThinking": { + "type": "boolean" } }, "required": [ @@ -1119,7 +1128,8 @@ "displayName", "apiUrl", "requiresAuth", - "apiKeySet" + "apiKeySet", + "preservesThinking" ] }, "CustomProviderUpdateRequest": { @@ -1177,6 +1187,12 @@ "string", "null" ] + }, + "preservesThinking": { + "type": [ + "boolean", + "null" + ] } }, "required": [ diff --git a/crates/goose/src/acp/server/providers.rs b/crates/goose/src/acp/server/providers.rs index e902626ac8..c4729fcbcc 100644 --- a/crates/goose/src/acp/server/providers.rs +++ b/crates/goose/src/acp/server/providers.rs @@ -390,6 +390,7 @@ fn custom_provider_config_to_dto( base_path: config.base_path.clone(), api_key_env, api_key_set, + preserves_thinking: config.preserves_thinking, } } @@ -513,6 +514,7 @@ impl GooseAcpAgent { requires_auth: provider.requires_auth, catalog_provider_id: provider.catalog_provider_id, base_path: provider.base_path, + preserves_thinking: provider.preserves_thinking, }, ) .internal_err_ctx("Failed to create custom provider")?; @@ -581,6 +583,7 @@ impl GooseAcpAgent { requires_auth: provider.requires_auth, catalog_provider_id: provider.catalog_provider_id, base_path: provider.base_path, + preserves_thinking: provider.preserves_thinking, }, ) .internal_err_ctx("Failed to update custom provider")?; diff --git a/crates/goose/src/config/declarative_providers.rs b/crates/goose/src/config/declarative_providers.rs index 32f765cfd9..e46bec3537 100644 --- a/crates/goose/src/config/declarative_providers.rs +++ b/crates/goose/src/config/declarative_providers.rs @@ -30,7 +30,7 @@ pub fn custom_providers_dir() -> std::path::PathBuf { Paths::config_dir().join("custom_providers") } -#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, ToSchema)] #[serde(rename_all = "lowercase")] pub enum ProviderEngine { OpenAI, @@ -102,12 +102,18 @@ pub struct DeclarativeProviderConfig { pub setup_steps: Vec, #[serde(default, deserialize_with = "deserialize_non_empty_string")] pub fast_model: Option, + #[serde(default)] + pub preserves_thinking: bool, } fn default_requires_auth() -> bool { true } +fn should_preserve_thinking_by_default(engine: &ProviderEngine) -> bool { + matches!(engine, ProviderEngine::OpenAI) +} + impl DeclarativeProviderConfig { pub fn id(&self) -> &str { &self.name @@ -244,6 +250,7 @@ pub struct CreateCustomProviderParams { pub requires_auth: bool, pub catalog_provider_id: Option, pub base_path: Option, + pub preserves_thinking: Option, } #[derive(Debug, Clone)] @@ -259,6 +266,7 @@ pub struct UpdateCustomProviderParams { pub requires_auth: bool, pub catalog_provider_id: Option, pub base_path: Option, + pub preserves_thinking: Option, } pub fn create_custom_provider( @@ -287,9 +295,14 @@ pub fn create_custom_provider( .map(|name| ModelInfo::new(name, 128000)) .collect(); + let engine = ProviderEngine::from_str(¶ms.engine)?; + let preserves_thinking = params + .preserves_thinking + .unwrap_or_else(|| should_preserve_thinking_by_default(&engine)); + let provider_config = DeclarativeProviderConfig { name: id.clone(), - engine: ProviderEngine::from_str(¶ms.engine)?, + engine, display_name: params.display_name.clone(), description: Some(format!("Custom {} provider", params.display_name)), api_key_env, @@ -307,6 +320,7 @@ pub fn create_custom_provider( model_doc_link: None, setup_steps: vec![], fast_model: None, + preserves_thinking, }; let custom_providers_dir = custom_providers_dir(); @@ -353,9 +367,18 @@ pub fn update_custom_provider(params: UpdateCustomProviderParams) -> Result<()> .map(|name| ModelInfo::new(name, 128000)) .collect(); + let engine = ProviderEngine::from_str(¶ms.engine)?; + let preserves_thinking = match params.preserves_thinking { + Some(value) => value, + None if existing_config.engine != engine => { + should_preserve_thinking_by_default(&engine) + } + None => existing_config.preserves_thinking, + }; + let updated_config = DeclarativeProviderConfig { name: params.id.clone(), - engine: ProviderEngine::from_str(¶ms.engine)?, + engine, display_name: params.display_name, description: existing_config.description, api_key_env, @@ -377,6 +400,7 @@ pub fn update_custom_provider(params: UpdateCustomProviderParams) -> Result<()> model_doc_link: existing_config.model_doc_link, setup_steps: existing_config.setup_steps, fast_model: existing_config.fast_model.clone(), + preserves_thinking, }; let file_path = custom_provider_file_path(&updated_config.name)?; @@ -408,7 +432,7 @@ pub fn load_provider(id: &str) -> Result { if custom_file_path.exists() { let content = std::fs::read_to_string(&custom_file_path)?; - let config: DeclarativeProviderConfig = serde_json::from_str(&content)?; + let config = deserialize_provider_config(&content)?; return Ok(LoadedProvider { config, is_editable: true, @@ -450,12 +474,24 @@ pub fn load_custom_providers(dir: &Path) -> Result Result { + let raw: serde_json::Value = serde_json::from_str(content)?; + let preserves_thinking_was_set = raw.get("preserves_thinking").is_some(); + let mut config: DeclarativeProviderConfig = serde_json::from_value(raw)?; + + if !preserves_thinking_was_set { + config.preserves_thinking = should_preserve_thinking_by_default(&config.engine); + } + + Ok(config) +} + fn load_fixed_providers() -> Result> { let mut res = Vec::new(); for file in FIXED_PROVIDERS.files() { @@ -467,7 +503,7 @@ fn load_fixed_providers() -> Result> { .contents_utf8() .ok_or_else(|| anyhow::anyhow!("Failed to read file as UTF-8: {:?}", file.path()))?; - match serde_json::from_str(content) { + match deserialize_provider_config(content) { Ok(config) => res.push(config), Err(e) => { tracing::warn!( @@ -661,12 +697,116 @@ mod tests { #[test] fn test_existing_json_files_still_deserialize_without_new_fields() { let json = include_str!("../providers/declarative/groq.json"); - let config: DeclarativeProviderConfig = - serde_json::from_str(json).expect("groq.json should parse without env_vars"); + let config = + deserialize_provider_config(json).expect("groq.json should parse without env_vars"); assert!(config.env_vars.is_none()); assert!(config.dynamic_models.is_none()); assert!(config.model_doc_link.is_none()); assert!(config.setup_steps.is_empty()); + assert!(config.preserves_thinking); + } + + #[test] + fn test_custom_openai_provider_missing_preserves_thinking_defaults_true() { + let json = r#"{ + "name": "custom_reasoning", + "engine": "openai", + "display_name": "Custom Reasoning", + "description": null, + "api_key_env": "", + "base_url": "https://example.com/v1", + "models": [{"name": "reasoning-model", "context_limit": 128000}], + "headers": null, + "timeout_seconds": null, + "supports_streaming": true, + "requires_auth": false + }"#; + + let config = deserialize_provider_config(json).expect("custom provider json should parse"); + + assert!(matches!(config.engine, ProviderEngine::OpenAI)); + assert!(config.preserves_thinking); + } + + #[test] + fn test_custom_provider_explicit_preserves_thinking_false_is_kept() { + let json = r#"{ + "name": "custom_strict", + "engine": "openai", + "display_name": "Custom Strict", + "description": null, + "api_key_env": "", + "base_url": "https://example.com/v1", + "models": [{"name": "strict-model", "context_limit": 128000}], + "headers": null, + "timeout_seconds": null, + "supports_streaming": true, + "requires_auth": false, + "preserves_thinking": false + }"#; + + let config = deserialize_provider_config(json).expect("custom provider json should parse"); + + assert!(matches!(config.engine, ProviderEngine::OpenAI)); + assert!(!config.preserves_thinking); + } + + #[test] + fn test_zai_json_deserializes() { + let json = include_str!("../providers/declarative/zai.json"); + let config: DeclarativeProviderConfig = + serde_json::from_str(json).expect("zai.json should parse"); + assert_eq!(config.name, "zai"); + assert_eq!(config.display_name, "Z.AI"); + assert!(matches!(config.engine, ProviderEngine::Anthropic)); + assert_eq!(config.api_key_env, "ZHIPU_API_KEY"); + assert_eq!(config.base_url, "${ZAI_BASE_URL}"); + assert_eq!(config.catalog_provider_id, Some("zai".to_string())); + assert_eq!(config.fast_model, Some("glm-4.5-air".to_string())); + assert!(config.preserves_thinking); + assert_eq!(config.supports_streaming, Some(true)); + assert_eq!(config.models[0].name, "glm-5.1"); + + let env_vars = config.env_vars.as_ref().expect("env_vars should be set"); + assert_eq!(env_vars.len(), 1); + assert_eq!(env_vars[0].name, "ZAI_BASE_URL"); + assert_eq!( + env_vars[0].default, + Some("https://api.z.ai/api/anthropic".to_string()) + ); + } + + #[test] + fn test_openai_reasoning_provider_json_preserves_thinking() { + for (name, json) in [ + ( + "custom_deepseek", + include_str!("../providers/declarative/deepseek.json"), + ), + ( + "moonshot", + include_str!("../providers/declarative/moonshot.json"), + ), + ( + "novita", + include_str!("../providers/declarative/novita.json"), + ), + ( + "nvidia", + include_str!("../providers/declarative/nvidia.json"), + ), + ( + "custom_tensorix", + include_str!("../providers/declarative/tensorix.json"), + ), + ("zhipu", include_str!("../providers/declarative/zhipu.json")), + ] { + let config: DeclarativeProviderConfig = + serde_json::from_str(json).expect("provider json should parse"); + assert_eq!(config.name, name); + assert!(matches!(config.engine, ProviderEngine::OpenAI)); + assert!(config.preserves_thinking); + } } #[test] @@ -767,6 +907,7 @@ mod tests { requires_auth: false, catalog_provider_id: None, base_path: None, + preserves_thinking: None, }) .unwrap(); @@ -784,6 +925,22 @@ mod tests { assert!(load_provider("custom_..\\secret").is_err()); } + #[test] + fn test_opencode_go_json_deserializes() { + let json = include_str!("../providers/declarative/opencode_go.json"); + let config: DeclarativeProviderConfig = + serde_json::from_str(json).expect("opencode_go.json should parse"); + assert_eq!(config.name, "opencode_go"); + assert_eq!(config.display_name, "OpenCode Go"); + assert!(matches!(config.engine, ProviderEngine::OpenAI)); + assert_eq!(config.api_key_env, "OPENCODE_API_KEY"); + assert_eq!(config.base_url, "https://opencode.ai/zen/go/v1"); + assert_eq!(config.catalog_provider_id, Some("opencode-go".to_string())); + assert_eq!(config.dynamic_models, Some(true)); + assert!(config.preserves_thinking); + assert_eq!(config.models[0].name, "kimi-k2.6"); + } + #[test] fn test_expand_env_vars_replaces_placeholder() { let _guard = env_lock::lock_env([("TEST_EXPAND_HOST", Some("https://example.com/api"))]); diff --git a/crates/goose/src/providers/anthropic.rs b/crates/goose/src/providers/anthropic.rs index 511242c625..3a99064552 100644 --- a/crates/goose/src/providers/anthropic.rs +++ b/crates/goose/src/providers/anthropic.rs @@ -12,7 +12,8 @@ use super::api_client::{ApiClient, AuthMethod}; use super::base::{ConfigKey, MessageStream, ModelInfo, Provider, ProviderDef, ProviderMetadata}; use super::errors::ProviderError; use super::formats::anthropic::{ - create_request, response_to_streaming_message, thinking_type, ThinkingType, + create_request_with_options, response_to_streaming_message, thinking_type, + AnthropicFormatOptions, ThinkingType, }; use super::inventory::{config_secret_value, serialize_string_map, InventoryIdentityInput}; use super::openai_compatible::handle_status; @@ -59,6 +60,8 @@ pub struct AnthropicProvider { custom_models: Option>, dynamic_models: Option, skip_canonical_filtering: bool, + #[serde(skip)] + format_options: AnthropicFormatOptions, } impl AnthropicProvider { @@ -87,6 +90,7 @@ impl AnthropicProvider { custom_models: None, dynamic_models: None, skip_canonical_filtering: false, + format_options: AnthropicFormatOptions::default(), }) } @@ -124,6 +128,8 @@ impl AnthropicProvider { key: api_key, }; + let format_options = Self::format_options_for_provider(config.preserves_thinking); + let mut api_client = ApiClient::new(config.base_url, auth)? .with_header("anthropic-version", ANTHROPIC_API_VERSION)?; @@ -160,9 +166,17 @@ impl AnthropicProvider { custom_models, dynamic_models: config.dynamic_models, skip_canonical_filtering: config.skip_canonical_filtering, + format_options, }) } + fn format_options_for_provider(preserves_thinking: bool) -> AnthropicFormatOptions { + AnthropicFormatOptions { + preserve_unsigned_thinking: preserves_thinking, + preserve_thinking_context: preserves_thinking, + } + } + fn get_conditional_headers(&self) -> Vec<(&str, &str)> { let mut headers = Vec::new(); @@ -326,7 +340,13 @@ impl Provider for AnthropicProvider { messages: &[Message], tools: &[Tool], ) -> Result { - let mut payload = create_request(model_config, system, messages, tools)?; + let mut payload = create_request_with_options( + model_config, + system, + messages, + tools, + self.format_options, + )?; payload .as_object_mut() .unwrap() @@ -365,7 +385,6 @@ impl Provider for AnthropicProvider { })) } } - #[cfg(test)] mod tests { use super::*; @@ -394,6 +413,7 @@ mod tests { custom_models, dynamic_models, skip_canonical_filtering: false, + format_options: AnthropicFormatOptions::default(), } } @@ -421,6 +441,7 @@ mod tests { model_doc_link: None, setup_steps: vec![], fast_model: None, + preserves_thinking: false, } } diff --git a/crates/goose/src/providers/canonical/name_builder.rs b/crates/goose/src/providers/canonical/name_builder.rs index d5617437fe..10464304c0 100644 --- a/crates/goose/src/providers/canonical/name_builder.rs +++ b/crates/goose/src/providers/canonical/name_builder.rs @@ -49,6 +49,7 @@ pub fn map_provider_name(provider: &str) -> &str { "gemini_oauth" => "google", "zhipu" => "zhipuai", "novita" => "novita-ai", + "opencode_go" => "opencode-go", _ => provider, } } @@ -320,6 +321,10 @@ mod tests { map_to_canonical_model("openai", "gpt-4-turbo-2024-04-09", r), Some("openai/gpt-4-turbo".to_string()) ); + assert_eq!( + map_to_canonical_model("opencode_go", "kimi-k2.6", r), + Some("opencode-go/kimi-k2.6".to_string()) + ); // === OpenRouter === assert_eq!( diff --git a/crates/goose/src/providers/catalog.rs b/crates/goose/src/providers/catalog.rs index 1cfdd662bb..e1dfe6d0f5 100644 --- a/crates/goose/src/providers/catalog.rs +++ b/crates/goose/src/providers/catalog.rs @@ -529,6 +529,23 @@ const SETUP_METADATA: &[CuratedSetupMetadata] = &[ secret_field_default: Some(API_KEY_FIELD), field_overrides: &[], }, + CuratedSetupMetadata { + provider_id: "zai", + category: ProviderSetupCategory::Model, + setup_method: ProviderSetupMethod::SingleApiKey, + group: ProviderSetupGroup::Additional, + display_name: None, + description: Some("GLM models via Z.AI"), + docs_url: Some("https://docs.z.ai/devpack/tool/goose"), + aliases: &["z.ai", "zhipu"], + native_connect_query: None, + binary_name: None, + setup_capabilities: setup_capabilities(false, false, false), + show_only_when_installed: false, + synthetic: false, + secret_field_default: Some(API_KEY_FIELD), + field_overrides: &[], + }, CuratedSetupMetadata { provider_id: "xai", category: ProviderSetupCategory::Model, @@ -1045,16 +1062,35 @@ pub fn get_provider_template(provider_id: &str) -> Option { #[cfg(test)] mod tests { use super::*; + use crate::providers::base::ProviderType; #[tokio::test] async fn test_zai_provider() { - let openai_providers = get_providers_by_format(ProviderFormat::OpenAI).await; - let zai = openai_providers.iter().find(|p| p.id == "zai"); - assert!(zai.is_some(), "z.ai should be in catalog"); + let zai = crate::providers::get_from_registry("zai") + .await + .expect("z.ai should be registered as a declarative provider"); + assert_eq!(zai.provider_type(), ProviderType::Declarative); - let zai = zai.unwrap(); - println!("Z.AI: {} models", zai.model_count); - assert!(zai.model_count > 0, "z.ai should have models"); + let metadata = zai.metadata(); + assert_eq!(metadata.display_name, "Z.AI"); + assert!( + !metadata.known_models.is_empty(), + "z.ai should have known models" + ); + assert!( + metadata + .config_keys + .iter() + .any(|key| key.name == "ZHIPU_API_KEY"), + "z.ai should expose its API key config" + ); + + let setup_entries = get_setup_catalog_entries().await; + let setup_entry = setup_entries + .iter() + .find(|entry| entry.provider_id == "zai") + .expect("z.ai should be in the setup catalog"); + assert_eq!(setup_entry.setup_method, ProviderSetupMethod::SingleApiKey); let template = get_provider_template("zai"); assert!(template.is_some(), "z.ai should have a template"); diff --git a/crates/goose/src/providers/declarative/deepseek.json b/crates/goose/src/providers/declarative/deepseek.json index 04347d1e6f..cb80ab891b 100644 --- a/crates/goose/src/providers/declarative/deepseek.json +++ b/crates/goose/src/providers/declarative/deepseek.json @@ -25,5 +25,6 @@ ], "headers": null, "timeout_seconds": null, + "preserves_thinking": true, "supports_streaming": true -} \ No newline at end of file +} diff --git a/crates/goose/src/providers/declarative/moonshot.json b/crates/goose/src/providers/declarative/moonshot.json index bd64902291..d21c6faee0 100644 --- a/crates/goose/src/providers/declarative/moonshot.json +++ b/crates/goose/src/providers/declarative/moonshot.json @@ -13,5 +13,6 @@ {"name": "moonshot-v1-8k", "context_limit": 8192}, {"name": "moonshot-v1-32k", "context_limit": 32768} ], + "preserves_thinking": true, "supports_streaming": true } diff --git a/crates/goose/src/providers/declarative/novita.json b/crates/goose/src/providers/declarative/novita.json index 7dbd5d6ff4..9fc1bfad9b 100644 --- a/crates/goose/src/providers/declarative/novita.json +++ b/crates/goose/src/providers/declarative/novita.json @@ -33,5 +33,6 @@ "max_tokens": 131072 } ], + "preserves_thinking": true, "supports_streaming": true } diff --git a/crates/goose/src/providers/declarative/nvidia.json b/crates/goose/src/providers/declarative/nvidia.json index 787422835b..23b8cf0dcd 100644 --- a/crates/goose/src/providers/declarative/nvidia.json +++ b/crates/goose/src/providers/declarative/nvidia.json @@ -13,6 +13,7 @@ "context_limit": 131072 } ], + "preserves_thinking": true, "supports_streaming": true, "model_doc_link": "https://build.nvidia.com/models", "setup_steps": [ diff --git a/crates/goose/src/providers/declarative/opencode_go.json b/crates/goose/src/providers/declarative/opencode_go.json new file mode 100644 index 0000000000..f509d2fbb9 --- /dev/null +++ b/crates/goose/src/providers/declarative/opencode_go.json @@ -0,0 +1,29 @@ +{ + "name": "opencode_go", + "engine": "openai", + "display_name": "OpenCode Go", + "description": "OpenCode Go models via OpenAI-compatible API.", + "api_key_env": "OPENCODE_API_KEY", + "base_url": "https://opencode.ai/zen/go/v1", + "catalog_provider_id": "opencode-go", + "dynamic_models": true, + "model_doc_link": "https://opencode.ai/docs/zen", + "preserves_thinking": true, + "models": [ + {"name": "kimi-k2.6", "context_limit": 262144}, + {"name": "kimi-k2.5", "context_limit": 262144}, + {"name": "deepseek-v4-flash", "context_limit": 1000000}, + {"name": "deepseek-v4-pro", "context_limit": 1000000}, + {"name": "glm-5.1", "context_limit": 204800}, + {"name": "glm-5", "context_limit": 204800}, + {"name": "mimo-v2.5-pro", "context_limit": 1048576}, + {"name": "mimo-v2.5", "context_limit": 262144}, + {"name": "mimo-v2-pro", "context_limit": 1048576}, + {"name": "mimo-v2-omni", "context_limit": 262144}, + {"name": "minimax-m2.7", "context_limit": 204800}, + {"name": "minimax-m2.5", "context_limit": 204800}, + {"name": "qwen3.6-plus", "context_limit": 262144}, + {"name": "qwen3.5-plus", "context_limit": 262144} + ], + "supports_streaming": true +} diff --git a/crates/goose/src/providers/declarative/tensorix.json b/crates/goose/src/providers/declarative/tensorix.json index 7f03eb3652..3f9fb6f98f 100644 --- a/crates/goose/src/providers/declarative/tensorix.json +++ b/crates/goose/src/providers/declarative/tensorix.json @@ -49,5 +49,6 @@ ], "headers": null, "timeout_seconds": null, + "preserves_thinking": true, "supports_streaming": true } diff --git a/crates/goose/src/providers/declarative/zai.json b/crates/goose/src/providers/declarative/zai.json new file mode 100644 index 0000000000..3a699b8124 --- /dev/null +++ b/crates/goose/src/providers/declarative/zai.json @@ -0,0 +1,34 @@ +{ + "name": "zai", + "engine": "anthropic", + "display_name": "Z.AI", + "description": "Z.AI GLM models via Anthropic-compatible API.", + "api_key_env": "ZHIPU_API_KEY", + "base_url": "${ZAI_BASE_URL}", + "env_vars": [ + { + "name": "ZAI_BASE_URL", + "required": false, + "secret": false, + "default": "https://api.z.ai/api/anthropic", + "description": "Z.AI Anthropic-compatible API base URL." + } + ], + "catalog_provider_id": "zai", + "model_doc_link": "https://docs.z.ai/devpack/tool/goose", + "fast_model": "glm-4.5-air", + "preserves_thinking": true, + "models": [ + {"name": "glm-5.1", "context_limit": 200000}, + {"name": "glm-5", "context_limit": 204800}, + {"name": "glm-5-turbo", "context_limit": 200000}, + {"name": "glm-4.7", "context_limit": 204800}, + {"name": "glm-4.7-flash", "context_limit": 200000}, + {"name": "glm-4.7-flashx", "context_limit": 200000}, + {"name": "glm-4.6", "context_limit": 204800}, + {"name": "glm-4.5", "context_limit": 131072}, + {"name": "glm-4.5-air", "context_limit": 131072}, + {"name": "glm-4.5-flash", "context_limit": 131072} + ], + "supports_streaming": true +} diff --git a/crates/goose/src/providers/declarative/zhipu.json b/crates/goose/src/providers/declarative/zhipu.json index bc22bc76ee..93d30ab498 100644 --- a/crates/goose/src/providers/declarative/zhipu.json +++ b/crates/goose/src/providers/declarative/zhipu.json @@ -24,5 +24,6 @@ {"name": "glm-4.7-flash", "context_limit": 200000}, {"name": "glm-5", "context_limit": 204800} ], + "preserves_thinking": true, "supports_streaming": true } diff --git a/crates/goose/src/providers/formats/anthropic.rs b/crates/goose/src/providers/formats/anthropic.rs index 5a7f3c6281..72450770bb 100644 --- a/crates/goose/src/providers/formats/anthropic.rs +++ b/crates/goose/src/providers/formats/anthropic.rs @@ -39,6 +39,35 @@ macro_rules! string_enum { string_enum!(ThinkingType { Adaptive => "adaptive", Enabled => "enabled", Disabled => "disabled" }); string_enum!(ThinkingEffort { Low => "low", Medium => "medium", High => "high", Max => "max" }); +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct AnthropicFormatOptions { + pub preserve_unsigned_thinking: bool, + pub preserve_thinking_context: bool, +} + +impl AnthropicFormatOptions { + fn for_model(self, model_config: &ModelConfig) -> Self { + let preserve_thinking_context = model_config + .get_config_param::( + "preserve_thinking_context", + "ANTHROPIC_PRESERVE_THINKING_CONTEXT", + ) + .unwrap_or(self.preserve_thinking_context); + let preserve_unsigned_thinking = model_config + .get_config_param::( + "preserve_unsigned_thinking", + "ANTHROPIC_PRESERVE_UNSIGNED_THINKING", + ) + .unwrap_or(self.preserve_unsigned_thinking) + || preserve_thinking_context; + + Self { + preserve_unsigned_thinking, + preserve_thinking_context, + } + } +} + pub fn supports_adaptive_thinking(model_name: &str) -> bool { let lower = model_name.to_lowercase(); lower.contains("claude-opus-4-6") || lower.contains("claude-sonnet-4-6") @@ -109,6 +138,13 @@ const EVENT_CONTENT_BLOCK_STOP: &str = "content_block_stop"; /// Convert internal Message format to Anthropic's API message specification pub fn format_messages(messages: &[Message]) -> Vec { + format_messages_with_options(messages, AnthropicFormatOptions::default()) +} + +fn format_messages_with_options( + messages: &[Message], + options: AnthropicFormatOptions, +) -> Vec { let mut anthropic_messages = Vec::new(); for message in messages { @@ -195,6 +231,11 @@ pub fn format_messages(messages: &[Message]) -> Vec { THINKING_TYPE: thinking.thinking, SIGNATURE_FIELD: thinking.signature })); + } else if options.preserve_unsigned_thinking && !thinking.thinking.is_empty() { + content.push(json!({ + TYPE_FIELD: THINKING_TYPE, + THINKING_TYPE: thinking.thinking + })); } } MessageContent::RedactedThinking(redacted) => { @@ -354,7 +395,7 @@ pub fn response_to_message(response: &Value) -> Result { let signature = block .get(SIGNATURE_FIELD) .and_then(|s| s.as_str()) - .ok_or_else(|| anyhow!("Missing thinking signature"))?; + .unwrap_or_default(); message = message.with_thinking(thinking, signature); } Some(REDACTED_THINKING_TYPE) => { @@ -478,7 +519,34 @@ pub fn thinking_effort(model_config: &ModelConfig) -> ThinkingEffort { } } -fn apply_thinking_config(payload: &mut Value, model_config: &ModelConfig, max_tokens: i32) { +fn thinking_budget_tokens(model_config: &ModelConfig) -> i32 { + let request_param = model_config + .request_params + .as_ref() + .and_then(|params| params.get("budget_tokens")) + .and_then(|v| serde_json::from_value(v.clone()).ok()); + + request_param + .or_else(|| { + crate::config::Config::global() + .get_param::("ANTHROPIC_THINKING_BUDGET") + .ok() + }) + .or_else(|| { + crate::config::Config::global() + .get_param::("CLAUDE_THINKING_BUDGET") + .ok() + }) + .unwrap_or(16000) + .max(1024) +} + +fn apply_thinking_config( + payload: &mut Value, + model_config: &ModelConfig, + max_tokens: i32, + options: AnthropicFormatOptions, +) { let obj = payload.as_object_mut().unwrap(); match thinking_type(model_config) { ThinkingType::Adaptive => { @@ -487,10 +555,7 @@ fn apply_thinking_config(payload: &mut Value, model_config: &ModelConfig, max_to obj.insert("output_config".to_string(), json!({"effort": effort})); } ThinkingType::Enabled => { - let budget_tokens = model_config - .get_config_param::("budget_tokens", "CLAUDE_THINKING_BUDGET") - .unwrap_or(16000) - .max(1024); + let budget_tokens = thinking_budget_tokens(model_config); obj.insert("max_tokens".to_string(), json!(max_tokens + budget_tokens)); obj.insert( @@ -503,6 +568,24 @@ fn apply_thinking_config(payload: &mut Value, model_config: &ModelConfig, max_to } ThinkingType::Disabled => {} } + + if options.preserve_thinking_context { + if !obj.contains_key("thinking") { + let budget_tokens = thinking_budget_tokens(model_config); + obj.insert("max_tokens".to_string(), json!(max_tokens + budget_tokens)); + obj.insert( + "thinking".to_string(), + json!({ + "type": "enabled", + "budget_tokens": budget_tokens + }), + ); + } + + if let Some(thinking) = obj.get_mut("thinking").and_then(|t| t.as_object_mut()) { + thinking.insert("clear_thinking".to_string(), json!(false)); + } + } } /// Create a complete request payload for Anthropic's API @@ -512,7 +595,24 @@ pub fn create_request( messages: &[Message], tools: &[Tool], ) -> Result { - let anthropic_messages = format_messages(messages); + create_request_with_options( + model_config, + system, + messages, + tools, + AnthropicFormatOptions::default(), + ) +} + +pub fn create_request_with_options( + model_config: &ModelConfig, + system: &str, + messages: &[Message], + tools: &[Tool], + options: AnthropicFormatOptions, +) -> Result { + let options = options.for_model(model_config); + let anthropic_messages = format_messages_with_options(messages, options); let tool_specs = format_tools(tools); let system_spec = format_system(system); @@ -548,7 +648,7 @@ pub fn create_request( .insert("temperature".to_string(), json!(temp)); } - apply_thinking_config(&mut payload, model_config, max_tokens); + apply_thinking_config(&mut payload, model_config, max_tokens, options); Ok(payload) } @@ -655,8 +755,16 @@ where } Some(THINKING_TYPE) => { thinking = Some(ThinkingState { - text: String::new(), - signature: String::new(), + text: content_block + .get(THINKING_TYPE) + .and_then(|t| t.as_str()) + .unwrap_or_default() + .to_string(), + signature: content_block + .get(SIGNATURE_FIELD) + .and_then(|s| s.as_str()) + .unwrap_or_default() + .to_string(), }); } Some(REDACTED_THINKING_TYPE) => { @@ -894,6 +1002,36 @@ mod tests { Ok(()) } + #[test] + fn test_parse_unsigned_thinking_response() -> Result<()> { + let response = json!({ + "id": "msg_123", + "type": "message", + "role": "assistant", + "content": [{ + "type": "thinking", + "thinking": "internal reasoning" + }], + "model": "glm-4.7", + "stop_reason": "end_turn", + "usage": { + "input_tokens": 12, + "output_tokens": 15 + } + }); + + let message = response_to_message(&response)?; + + if let MessageContent::Thinking(thinking) = &message.content[0] { + assert_eq!(thinking.thinking, "internal reasoning"); + assert_eq!(thinking.signature, ""); + } else { + panic!("Expected Thinking content"); + } + + Ok(()) + } + #[test] fn test_message_to_anthropic_spec() { let messages = vec![ @@ -929,6 +1067,29 @@ mod tests { assert_eq!(spec[0]["content"][0]["text"], "Hi there"); } + #[test] + fn test_message_to_anthropic_spec_preserves_unsigned_thinking_when_enabled() { + let messages = vec![ + Message::assistant().with_content(MessageContent::thinking("internal", "")), + Message::assistant().with_text("Hi there"), + ]; + + let spec = format_messages_with_options( + &messages, + AnthropicFormatOptions { + preserve_unsigned_thinking: true, + preserve_thinking_context: false, + }, + ); + + assert_eq!(spec.len(), 2); + assert_eq!(spec[0]["role"], "assistant"); + assert_eq!(spec[0]["content"][0]["type"], "thinking"); + assert_eq!(spec[0]["content"][0]["thinking"], "internal"); + assert!(spec[0]["content"][0].get("signature").is_none()); + assert_eq!(spec[1]["content"][0]["text"], "Hi there"); + } + #[test] fn test_tools_to_anthropic_spec() { let tools = vec![ @@ -1044,6 +1205,7 @@ mod tests { ("CLAUDE_THINKING_TYPE", None::<&str>), ("CLAUDE_THINKING_EFFORT", None::<&str>), ("CLAUDE_THINKING_ENABLED", None::<&str>), + ("ANTHROPIC_THINKING_BUDGET", None::<&str>), ("CLAUDE_THINKING_BUDGET", None::<&str>), ]); @@ -1070,6 +1232,7 @@ mod tests { let _guard = env_lock::lock_env([ ("CLAUDE_THINKING_TYPE", None::<&str>), ("CLAUDE_THINKING_ENABLED", None::<&str>), + ("ANTHROPIC_PRESERVE_THINKING_CONTEXT", None::<&str>), ]); let config = cfg("claude-sonnet-4-20250514"); @@ -1082,6 +1245,78 @@ mod tests { Ok(()) } + #[test] + fn test_create_request_preserves_thinking_context_for_compatible_models() -> Result<()> { + let _guard = env_lock::lock_env([ + ("CLAUDE_THINKING_TYPE", None::<&str>), + ("CLAUDE_THINKING_ENABLED", None::<&str>), + ("ANTHROPIC_THINKING_BUDGET", None::<&str>), + ("CLAUDE_THINKING_BUDGET", None::<&str>), + ("ANTHROPIC_PRESERVE_THINKING_CONTEXT", None::<&str>), + ("ANTHROPIC_PRESERVE_UNSIGNED_THINKING", None::<&str>), + ]); + + let mut config = cfg("glm-4.7"); + config.max_tokens = Some(4096); + let messages = vec![ + Message::assistant().with_content(MessageContent::thinking("internal", "")), + Message::user().with_text("Continue"), + ]; + + let payload = create_request_with_options( + &config, + "system", + &messages, + &[], + AnthropicFormatOptions { + preserve_unsigned_thinking: true, + preserve_thinking_context: true, + }, + )?; + + assert_eq!(payload["thinking"]["type"], "enabled"); + assert_eq!(payload["thinking"]["budget_tokens"], 16000); + assert_eq!(payload["thinking"]["clear_thinking"], false); + assert_eq!(payload["max_tokens"], 4096 + 16000); + assert_eq!(payload["messages"][0]["content"][0]["type"], "thinking"); + assert_eq!(payload["messages"][0]["content"][0]["thinking"], "internal"); + assert!(payload["messages"][0]["content"][0] + .get("signature") + .is_none()); + + Ok(()) + } + + #[test] + fn test_create_request_model_params_enable_preserved_thinking_context() -> Result<()> { + let _guard = env_lock::lock_env([ + ("CLAUDE_THINKING_TYPE", None::<&str>), + ("CLAUDE_THINKING_ENABLED", None::<&str>), + ("ANTHROPIC_THINKING_BUDGET", None::<&str>), + ("CLAUDE_THINKING_BUDGET", None::<&str>), + ("ANTHROPIC_PRESERVE_THINKING_CONTEXT", None::<&str>), + ("ANTHROPIC_PRESERVE_UNSIGNED_THINKING", None::<&str>), + ]); + + let mut params = std::collections::HashMap::new(); + params.insert("preserve_thinking_context".to_string(), json!(true)); + + let mut config = cfg("glm-4.7"); + config.request_params = Some(params); + let messages = vec![ + Message::assistant().with_content(MessageContent::thinking("internal", "")), + Message::user().with_text("Continue"), + ]; + + let payload = create_request(&config, "system", &messages, &[])?; + + assert_eq!(payload["thinking"]["clear_thinking"], false); + assert_eq!(payload["messages"][0]["content"][0]["type"], "thinking"); + assert_eq!(payload["messages"][0]["content"][0]["thinking"], "internal"); + + Ok(()) + } + #[test] fn test_tool_error_handling_maintains_pairing() { use crate::conversation::message::Message; @@ -1353,6 +1588,26 @@ mod tests { assert_eq!(parts.text, vec!["Here is the answer."]); } + #[tokio::test] + async fn test_streaming_thinking_from_start_block_without_signature() { + let events = concat!( + r#"data: {"type":"message_start","message":{"id":"msg_1","role":"assistant","content":[],"model":"glm-4.7","usage":{"input_tokens":10,"output_tokens":0}}}"#, + "\n", + r#"data: {"type":"content_block_start","index":0,"content_block":{"type":"thinking","thinking":"Initial reasoning "}}"#, + "\n", + r#"data: {"type":"content_block_delta","index":0,"delta":{"type":"thinking_delta","thinking":"continues."}}"#, + "\n", + r#"data: {"type":"content_block_stop","index":0}"#, + "\n", + r#"data: {"type":"message_stop"}"#, + ); + + let parts = collect_stream(events).await; + assert_eq!(parts.thinking.len(), 1); + assert_eq!(parts.thinking[0].0, "Initial reasoning continues."); + assert_eq!(parts.thinking[0].1, ""); + } + #[tokio::test] async fn test_streaming_redacted_thinking() { let events = concat!( diff --git a/crates/goose/src/providers/formats/openai.rs b/crates/goose/src/providers/formats/openai.rs index 24de91eb61..66e9acaba5 100644 --- a/crates/goose/src/providers/formats/openai.rs +++ b/crates/goose/src/providers/formats/openai.rs @@ -39,6 +39,32 @@ where Ok(Option::::deserialize(deserializer)?.unwrap_or_default()) } +fn is_reserved_request_param_key(key: &str) -> bool { + matches!(key, "messages" | "model" | "stream" | "stream_options") +} + +#[derive(Debug, Clone, Copy, Default)] +pub struct OpenAiFormatOptions { + pub preserve_thinking_context: bool, +} + +fn merge_reasoning_text(prefix: &str, suffix: &str) -> String { + if prefix.is_empty() { + return suffix.to_string(); + } + if suffix.is_empty() { + return prefix.to_string(); + } + if suffix.starts_with(prefix) { + return suffix.to_string(); + } + if prefix.ends_with(suffix) { + return prefix.to_string(); + } + + format!("{prefix}{suffix}") +} + #[derive(Serialize, Deserialize, Debug, Default)] struct DeltaToolCallFunction { name: Option, @@ -138,8 +164,28 @@ fn extract_content_and_signature( } pub fn format_messages(messages: &[Message], image_format: &ImageFormat) -> Vec { + format_messages_with_options( + messages, + image_format, + OpenAiFormatOptions { + preserve_thinking_context: true, + }, + ) +} + +pub fn format_messages_with_options( + messages: &[Message], + image_format: &ImageFormat, + options: OpenAiFormatOptions, +) -> Vec { let mut messages_spec = Vec::new(); + let mut pending_assistant_reasoning = String::new(); + for message in messages { + if options.preserve_thinking_context && message.role != Role::Assistant { + pending_assistant_reasoning.clear(); + } + let mut converted = json!({ "role": message.role }); @@ -346,14 +392,29 @@ pub fn format_messages(messages: &[Message], image_format: &ImageFormat) -> Vec< converted["content"] = json!(null); } - // Include reasoning_content only when non-empty. - // Kimi rejects empty reasoning_content (""), so we must omit it entirely - // when there's no reasoning to send. - if !reasoning_text.is_empty() { + let has_message_payload = + converted.get("content").is_some() || converted.get("tool_calls").is_some(); + + if options.preserve_thinking_context && message.role == Role::Assistant { + if !has_message_payload && output.is_empty() && !reasoning_text.is_empty() { + pending_assistant_reasoning.push_str(&reasoning_text); + continue; + } + + if !pending_assistant_reasoning.is_empty() { + reasoning_text = + merge_reasoning_text(&pending_assistant_reasoning, &reasoning_text); + pending_assistant_reasoning.clear(); + } + } + + // Include reasoning_content only when non-empty. Kimi rejects empty + // reasoning_content (""), so we must omit it entirely. + if options.preserve_thinking_context && !reasoning_text.is_empty() { converted["reasoning_content"] = json!(reasoning_text); } - if converted.get("content").is_some() || converted.get("tool_calls").is_some() { + if has_message_payload { output.insert(0, converted); } @@ -834,6 +895,7 @@ where let mut accumulated_reasoning_content = String::new(); let mut think_filter = ThinkFilter::new(); let mut saw_structured_reasoning = false; + let mut yielded_reasoning_content_len = 0usize; let mut last_signature: Option = None; // Buffer inline ... content until we know whether structured // reasoning will arrive. Emitting it immediately and then receiving @@ -984,10 +1046,17 @@ where } let mut contents = Vec::new(); - if !accumulated_reasoning_content.is_empty() { - contents.push(MessageContent::thinking(&accumulated_reasoning_content, "")); - accumulated_reasoning_content.clear(); + if yielded_reasoning_content_len < accumulated_reasoning_content.len() { + if let Some(unyielded_reasoning) = + accumulated_reasoning_content.get(yielded_reasoning_content_len..) + { + if !unyielded_reasoning.is_empty() { + contents.push(MessageContent::thinking(unyielded_reasoning, "")); + } + } } + accumulated_reasoning_content.clear(); + yielded_reasoning_content_len = 0; let mut sorted_indices: Vec<_> = tool_call_data.keys().cloned().collect(); sorted_indices.sort(); @@ -1055,6 +1124,7 @@ where if let Some(reasoning) = chunk.choices[0].delta.reasoning_text() { let signature = last_signature.as_deref().unwrap_or(""); content.push(MessageContent::thinking(reasoning, signature)); + yielded_reasoning_content_len = accumulated_reasoning_content.len(); } let (text_content, thought_signature) = extract_content_and_signature(chunk.choices[0].delta.content.as_ref()); @@ -1140,6 +1210,28 @@ pub fn create_request( tools: &[Tool], image_format: &ImageFormat, for_streaming: bool, +) -> anyhow::Result { + create_request_with_options( + model_config, + system, + messages, + tools, + image_format, + for_streaming, + OpenAiFormatOptions { + preserve_thinking_context: true, + }, + ) +} + +pub fn create_request_with_options( + model_config: &ModelConfig, + system: &str, + messages: &[Message], + tools: &[Tool], + image_format: &ImageFormat, + for_streaming: bool, + format_options: OpenAiFormatOptions, ) -> anyhow::Result { if model_config.model_name.starts_with("o1-mini") { return Err(anyhow!( @@ -1155,7 +1247,7 @@ pub fn create_request( "content": system }); - let messages_spec = format_messages(messages, image_format); + let messages_spec = format_messages_with_options(messages, image_format, format_options); let mut tools_spec = format_tools(tools)?; validate_tool_schemas(&mut tools_spec); @@ -1207,7 +1299,9 @@ pub fn create_request( if let Some(params) = &model_config.request_params { if let Some(obj) = payload.as_object_mut() { for (key, value) in params { - obj.insert(key.clone(), value.clone()); + if !is_reserved_request_param_key(key) { + obj.insert(key.clone(), value.clone()); + } } } } @@ -2009,6 +2103,65 @@ mod tests { Ok(()) } + #[test] + fn test_request_params_preserve_reserved_fields() -> anyhow::Result<()> { + let model_config = ModelConfig { + model_name: "glm-4.7".to_string(), + context_limit: Some(204800), + temperature: None, + max_tokens: Some(4096), + toolshim: false, + toolshim_model: None, + fast_model_config: None, + request_params: Some(std::collections::HashMap::from([ + ( + "thinking".to_string(), + json!({ + "type": "enabled", + "clear_thinking": false + }), + ), + ("stream".to_string(), json!(false)), + ( + "stream_options".to_string(), + json!({"include_usage": false}), + ), + ("model".to_string(), json!("wrong-model")), + ("messages".to_string(), json!([])), + ("max_tokens".to_string(), json!(1)), + ("temperature".to_string(), json!(2.0)), + ("provider_custom".to_string(), json!("allowed")), + ])), + reasoning: None, + }; + + let request = create_request( + &model_config, + "system", + &[], + &[], + &ImageFormat::OpenAi, + true, + )?; + + assert_eq!( + request["thinking"], + json!({ + "type": "enabled", + "clear_thinking": false + }) + ); + assert_eq!(request["stream"], true); + assert_eq!(request["stream_options"], json!({"include_usage": true})); + assert_eq!(request["model"], "glm-4.7"); + assert_eq!(request["messages"][0]["role"], "system"); + assert_eq!(request["max_tokens"], 1); + assert_eq!(request["temperature"], 2.0); + assert_eq!(request["provider_custom"], "allowed"); + + Ok(()) + } + #[test] fn test_create_request_o1_default() -> anyhow::Result<()> { // Without an explicit effort suffix the API picks its own default; @@ -2621,7 +2774,13 @@ data: [DONE]"#; .with_arguments(rmcp::object!({"param": "value"}))), ); - let spec = format_messages(&[message], &ImageFormat::OpenAi); + let spec = format_messages_with_options( + &[message], + &ImageFormat::OpenAi, + OpenAiFormatOptions { + preserve_thinking_context: true, + }, + ); assert_eq!(spec.len(), 1); assert_eq!(spec[0]["role"], "assistant"); @@ -2643,6 +2802,196 @@ data: [DONE]"#; Ok(()) } + #[test] + fn test_format_messages_preserves_reasoning_content_for_legacy_compat() -> anyhow::Result<()> { + let message = Message::assistant() + .with_content(MessageContent::thinking( + "Thinking through the problem...", + "", + )) + .with_text("The result is 42"); + + let spec = format_messages(&[message], &ImageFormat::OpenAi); + + assert_eq!(spec.len(), 1); + assert_eq!(spec[0]["content"], "The result is 42"); + assert_eq!( + spec[0]["reasoning_content"], + "Thinking through the problem..." + ); + + Ok(()) + } + + #[test] + fn test_format_messages_with_options_can_omit_reasoning_content() -> anyhow::Result<()> { + let message = Message::assistant() + .with_content(MessageContent::thinking( + "Thinking through the problem...", + "", + )) + .with_text("The result is 42"); + + let spec = format_messages_with_options( + &[message], + &ImageFormat::OpenAi, + OpenAiFormatOptions { + preserve_thinking_context: false, + }, + ); + + assert_eq!(spec.len(), 1); + assert_eq!(spec[0]["content"], "The result is 42"); + assert!(spec[0].get("reasoning_content").is_none()); + + Ok(()) + } + + #[test] + fn test_create_request_preserves_reasoning_content_for_legacy_compat() -> anyhow::Result<()> { + let model_config = ModelConfig { + model_name: "deepseek-reasoner".to_string(), + context_limit: Some(128000), + temperature: None, + max_tokens: Some(1024), + toolshim: false, + toolshim_model: None, + fast_model_config: None, + request_params: None, + reasoning: None, + }; + let message = Message::assistant() + .with_content(MessageContent::thinking("preserve this", "")) + .with_tool_request( + "tool1", + Ok(rmcp::model::CallToolRequestParams::new("test_tool") + .with_arguments(rmcp::object!({}))), + ); + + let request = create_request( + &model_config, + "system", + &[message], + &[], + &ImageFormat::OpenAi, + true, + )?; + + assert_eq!(request["messages"][1]["reasoning_content"], "preserve this"); + + Ok(()) + } + + #[test] + fn test_format_messages_carries_thinking_only_chunks_to_tool_call() -> anyhow::Result<()> { + let messages = vec![ + Message::assistant().with_content(MessageContent::thinking("think ", "")), + Message::assistant().with_content(MessageContent::thinking("once", "")), + Message::assistant().with_tool_request( + "tool1", + Ok(CallToolRequestParams::new("test_tool") + .with_arguments(object!({"param": "value"}))), + ), + ]; + + let spec = format_messages_with_options( + &messages, + &ImageFormat::OpenAi, + OpenAiFormatOptions { + preserve_thinking_context: true, + }, + ); + + assert_eq!(spec.len(), 1); + assert_eq!(spec[0]["role"], "assistant"); + assert_eq!(spec[0]["reasoning_content"], "think once"); + assert_eq!(spec[0]["content"], json!(null)); + assert_eq!(spec[0]["tool_calls"][0]["function"]["name"], "test_tool"); + + Ok(()) + } + + #[test] + fn test_format_messages_does_not_duplicate_pending_thinking() -> anyhow::Result<()> { + let messages = vec![ + Message::assistant().with_content(MessageContent::thinking("think once", "")), + Message::assistant() + .with_content(MessageContent::thinking("think once", "")) + .with_tool_request( + "tool1", + Ok(CallToolRequestParams::new("test_tool") + .with_arguments(object!({"param": "value"}))), + ), + ]; + + let spec = format_messages_with_options( + &messages, + &ImageFormat::OpenAi, + OpenAiFormatOptions { + preserve_thinking_context: true, + }, + ); + + assert_eq!(spec.len(), 1); + assert_eq!(spec[0]["reasoning_content"], "think once"); + assert_eq!(spec[0]["tool_calls"][0]["function"]["name"], "test_tool"); + + Ok(()) + } + + #[test] + fn test_format_messages_merges_pending_thinking_with_tool_call_suffix() -> anyhow::Result<()> { + let messages = vec![ + Message::assistant().with_content(MessageContent::thinking("think ", "")), + Message::assistant() + .with_content(MessageContent::thinking("once", "")) + .with_tool_request( + "tool1", + Ok(CallToolRequestParams::new("test_tool") + .with_arguments(object!({"param": "value"}))), + ), + ]; + + let spec = format_messages_with_options( + &messages, + &ImageFormat::OpenAi, + OpenAiFormatOptions { + preserve_thinking_context: true, + }, + ); + + assert_eq!(spec.len(), 1); + assert_eq!(spec[0]["reasoning_content"], "think once"); + assert_eq!(spec[0]["tool_calls"][0]["function"]["name"], "test_tool"); + + Ok(()) + } + + #[test] + fn test_format_messages_does_not_carry_thinking_across_user_message() -> anyhow::Result<()> { + let messages = vec![ + Message::assistant().with_content(MessageContent::thinking("stale", "")), + Message::user().with_text("new turn"), + Message::assistant() + .with_tool_request("tool1", Ok(CallToolRequestParams::new("test_tool"))), + ]; + + let spec = format_messages_with_options( + &messages, + &ImageFormat::OpenAi, + OpenAiFormatOptions { + preserve_thinking_context: true, + }, + ); + + assert_eq!(spec.len(), 2); + assert_eq!(spec[0]["role"], "user"); + assert_eq!(spec[1]["role"], "assistant"); + assert!(spec[1].get("reasoning_content").is_none()); + + Ok(()) + } + #[test_case( "data: {\"error\":{\"message\":\"Internal server error\",\"type\":\"server_error\",\"code\":500}}\ndata: [DONE]", "Internal server error"; @@ -2831,6 +3180,117 @@ data: [DONE]"#; Ok(()) } + #[tokio::test] + async fn test_streaming_tool_call_does_not_duplicate_yielded_reasoning() -> anyhow::Result<()> { + let response_lines = concat!( + "data: {\"id\":\"x\",\"object\":\"chat.completion.chunk\",\"model\":\"m\",\"choices\":[{\"index\":0,\"delta\":{\"reasoning_content\":\"think \"},\"finish_reason\":null}]}\n", + "data: {\"id\":\"x\",\"object\":\"chat.completion.chunk\",\"model\":\"m\",\"choices\":[{\"index\":0,\"delta\":{\"reasoning_content\":\"once\"},\"finish_reason\":null}]}\n", + "data: {\"id\":\"x\",\"object\":\"chat.completion.chunk\",\"model\":\"m\",\"choices\":[{\"index\":0,\"delta\":{\"tool_calls\":[{\"index\":0,\"id\":\"call_1\",\"type\":\"function\",\"function\":{\"name\":\"test_tool\",\"arguments\":\"\"}}]},\"finish_reason\":null}]}\n", + "data: {\"id\":\"x\",\"object\":\"chat.completion.chunk\",\"model\":\"m\",\"choices\":[{\"index\":0,\"delta\":{\"tool_calls\":[{\"index\":0,\"function\":{\"arguments\":\"{}\"}}]},\"finish_reason\":\"tool_calls\"}]}\n", + "data: [DONE]" + ); + let lines: Vec = response_lines.lines().map(|s| s.to_string()).collect(); + let response_stream = tokio_stream::iter(lines.into_iter().map(Ok)); + let mut messages = std::pin::pin!(response_to_streaming_message(response_stream)); + + let mut thinking = String::new(); + let mut tool_calls = 0; + let mut history = Vec::new(); + while let Some(result) = messages.next().await { + let (message, _usage) = result?; + if let Some(msg) = message { + for content in &msg.content { + match content { + MessageContent::Thinking(t) => thinking.push_str(&t.thinking), + MessageContent::ToolRequest(_) => tool_calls += 1, + _ => {} + } + } + history.push(msg); + } + } + + assert_eq!(thinking, "think once"); + assert_eq!(tool_calls, 1); + + let spec = format_messages_with_options( + &history, + &ImageFormat::OpenAi, + OpenAiFormatOptions { + preserve_thinking_context: true, + }, + ); + assert_eq!(spec.len(), 1); + assert_eq!(spec[0]["reasoning_content"], "think once"); + assert_eq!(spec[0]["tool_calls"][0]["function"]["name"], "test_tool"); + + Ok(()) + } + + #[tokio::test] + async fn test_streaming_tool_call_merges_yielded_reasoning_with_suffix() -> anyhow::Result<()> { + let response_lines = concat!( + "data: {\"id\":\"x\",\"object\":\"chat.completion.chunk\",\"model\":\"m\",\"choices\":[{\"index\":0,\"delta\":{\"reasoning_content\":\"think \"},\"finish_reason\":null}]}\n", + "data: {\"id\":\"x\",\"object\":\"chat.completion.chunk\",\"model\":\"m\",\"choices\":[{\"index\":0,\"delta\":{\"reasoning_content\":\"once\",\"tool_calls\":[{\"index\":0,\"id\":\"call_1\",\"type\":\"function\",\"function\":{\"name\":\"test_tool\",\"arguments\":\"\"}}]},\"finish_reason\":null}]}\n", + "data: {\"id\":\"x\",\"object\":\"chat.completion.chunk\",\"model\":\"m\",\"choices\":[{\"index\":0,\"delta\":{\"tool_calls\":[{\"index\":0,\"function\":{\"arguments\":\"{}\"}}]},\"finish_reason\":\"tool_calls\"}]}\n", + "data: [DONE]" + ); + let lines: Vec = response_lines.lines().map(|s| s.to_string()).collect(); + let response_stream = tokio_stream::iter(lines.into_iter().map(Ok)); + let mut messages = std::pin::pin!(response_to_streaming_message(response_stream)); + + let mut history = Vec::new(); + while let Some(result) = messages.next().await { + let (message, _usage) = result?; + if let Some(msg) = message { + history.push(msg); + } + } + + let spec = format_messages_with_options( + &history, + &ImageFormat::OpenAi, + OpenAiFormatOptions { + preserve_thinking_context: true, + }, + ); + assert_eq!(spec.len(), 1); + assert_eq!(spec[0]["reasoning_content"], "think once"); + assert_eq!(spec[0]["tool_calls"][0]["function"]["name"], "test_tool"); + + Ok(()) + } + + #[tokio::test] + async fn test_streaming_tool_call_preserves_unyielded_reasoning() -> anyhow::Result<()> { + let response_lines = concat!( + "data: {\"id\":\"x\",\"object\":\"chat.completion.chunk\",\"model\":\"m\",\"choices\":[{\"index\":0,\"delta\":{\"reasoning_content\":\"tool thought\",\"tool_calls\":[{\"index\":0,\"id\":\"call_1\",\"type\":\"function\",\"function\":{\"name\":\"test_tool\",\"arguments\":\"{}\"}}]},\"finish_reason\":\"tool_calls\"}]}\n", + "data: [DONE]" + ); + let lines: Vec = response_lines.lines().map(|s| s.to_string()).collect(); + let response_stream = tokio_stream::iter(lines.into_iter().map(Ok)); + let mut messages = std::pin::pin!(response_to_streaming_message(response_stream)); + + let mut thinking = String::new(); + let mut tool_calls = 0; + while let Some(result) = messages.next().await { + let (message, _usage) = result?; + if let Some(msg) = message { + for content in &msg.content { + match content { + MessageContent::Thinking(t) => thinking.push_str(&t.thinking), + MessageContent::ToolRequest(_) => tool_calls += 1, + _ => {} + } + } + } + } + + assert_eq!(thinking, "tool thought"); + assert_eq!(tool_calls, 1); + Ok(()) + } + // Streaming counterpart: both fields in one delta must parse and yield // thinking content, not fail with "duplicate field `reasoning_content`". #[tokio::test] diff --git a/crates/goose/src/providers/openai.rs b/crates/goose/src/providers/openai.rs index 379b9594ff..fe12aaa8e4 100644 --- a/crates/goose/src/providers/openai.rs +++ b/crates/goose/src/providers/openai.rs @@ -4,7 +4,9 @@ use super::base::{ }; use super::embedding::{EmbeddingCapable, EmbeddingRequest, EmbeddingResponse}; use super::errors::ProviderError; -use super::formats::openai::{create_request, get_usage, response_to_message}; +use super::formats::openai::{ + create_request_with_options, get_usage, response_to_message, OpenAiFormatOptions, +}; use super::formats::openai_responses::{ create_responses_request, get_responses_usage, responses_api_to_message, ResponsesApiResponse, }; @@ -120,6 +122,7 @@ pub struct OpenAiProvider { custom_models: Option>, dynamic_models: Option, skip_canonical_filtering: bool, + preserve_thinking_context: bool, } impl OpenAiProvider { @@ -273,6 +276,7 @@ impl OpenAiProvider { custom_models: None, dynamic_models: None, skip_canonical_filtering: false, + preserve_thinking_context: !is_openai, }) } @@ -290,6 +294,7 @@ impl OpenAiProvider { custom_models: None, dynamic_models: None, skip_canonical_filtering: false, + preserve_thinking_context: false, } } @@ -394,6 +399,7 @@ impl OpenAiProvider { custom_models, dynamic_models: config.dynamic_models, skip_canonical_filtering: config.skip_canonical_filtering, + preserve_thinking_context: config.preserves_thinking, }) } @@ -766,13 +772,16 @@ impl Provider for OpenAiProvider { Ok(super::base::stream_from_single_message(message, usage)) } } else { - let payload = create_request( + let payload = create_request_with_options( model_config, system, messages, tools, &ImageFormat::OpenAi, self.supports_streaming, + OpenAiFormatOptions { + preserve_thinking_context: self.preserve_thinking_context, + }, )?; let payload = self.sanitize_request_for_compat(payload); let mut log = RequestLog::start(model_config, &payload)?; @@ -907,6 +916,7 @@ mod tests { custom_models: None, dynamic_models: None, skip_canonical_filtering: false, + preserve_thinking_context: false, } } @@ -1150,6 +1160,7 @@ mod tests { custom_models, dynamic_models, skip_canonical_filtering: false, + preserve_thinking_context: false, } } @@ -1177,6 +1188,7 @@ mod tests { model_doc_link: None, setup_steps: vec![], fast_model: None, + preserves_thinking: false, } } diff --git a/crates/goose/tests/acp_custom_provider_methods_test.rs b/crates/goose/tests/acp_custom_provider_methods_test.rs index 70ac300832..47c5ebe66d 100644 --- a/crates/goose/tests/acp_custom_provider_methods_test.rs +++ b/crates/goose/tests/acp_custom_provider_methods_test.rs @@ -68,8 +68,8 @@ fn acp_catalog_and_custom_provider_methods_use_core_provider_store() { assert!( catalog_providers .iter() - .any(|provider| provider.get("providerId") == Some(&serde_json::json!("zai"))), - "OpenAI-compatible catalog should include z.ai" + .any(|provider| provider.get("providerId") == Some(&serde_json::json!("opencode"))), + "OpenAI-compatible catalog should include OpenCode Zen" ); let setup_catalog = send_custom( @@ -268,6 +268,7 @@ fn acp_catalog_and_custom_provider_methods_use_core_provider_store() { assert_eq!(saved_provider.name, provider_id); assert_eq!(saved_provider.display_name, "Stark ACP Provider"); assert_eq!(saved_provider.base_url, "https://stark.example/v1"); + assert!(saved_provider.preserves_thinking); assert_eq!( saved_provider .models @@ -314,6 +315,7 @@ fn acp_catalog_and_custom_provider_methods_use_core_provider_store() { "basePath": "v1/chat/completions", "apiKeyEnv": "CUSTOM_STARK_ACP_PROVIDER_API_KEY", "apiKeySet": true, + "preservesThinking": true, })) ); @@ -346,7 +348,8 @@ fn acp_catalog_and_custom_provider_methods_use_core_provider_store() { "supportsStreaming": false, "headers": {}, "requiresAuth": true, - "catalogProviderId": "zai" + "catalogProviderId": "zai", + "preservesThinking": false }), ) .await @@ -377,6 +380,7 @@ fn acp_catalog_and_custom_provider_methods_use_core_provider_store() { ); assert_eq!(updated_provider.base_path, None); assert_eq!(updated_provider.headers, None); + assert!(!updated_provider.preserves_thinking); assert_eq!( updated_provider .models @@ -417,6 +421,7 @@ fn acp_catalog_and_custom_provider_methods_use_core_provider_store() { .expect("no-auth provider should remain core-compatible"); assert!(!no_auth_provider.requires_auth); assert_eq!(no_auth_provider.api_key_env, ""); + assert!(!no_auth_provider.preserves_thinking); assert!( matches!( Config::global().get_secret::("CUSTOM_STARK_ACP_PROVIDER_API_KEY"), diff --git a/ui/desktop/openapi.json b/ui/desktop/openapi.json index 9823f8334e..e0b73df4ca 100644 --- a/ui/desktop/openapi.json +++ b/ui/desktop/openapi.json @@ -4718,6 +4718,9 @@ "name": { "type": "string" }, + "preserves_thinking": { + "type": "boolean" + }, "requires_auth": { "type": "boolean" }, @@ -8941,6 +8944,10 @@ "type": "string" } }, + "preserves_thinking": { + "type": "boolean", + "nullable": true + }, "requires_auth": { "type": "boolean" }, diff --git a/ui/desktop/src/api/types.gen.ts b/ui/desktop/src/api/types.gen.ts index 691d9e674c..da88661b8f 100644 --- a/ui/desktop/src/api/types.gen.ts +++ b/ui/desktop/src/api/types.gen.ts @@ -231,6 +231,7 @@ export type DeclarativeProviderConfig = { model_doc_link?: string | null; models: Array; name: string; + preserves_thinking?: boolean; requires_auth?: boolean; setup_steps?: Array; skip_canonical_filtering?: boolean; @@ -1629,6 +1630,7 @@ export type UpdateCustomProviderRequest = { [key: string]: string; } | null; models: Array; + preserves_thinking?: boolean | null; requires_auth?: boolean; supports_streaming?: boolean | null; }; diff --git a/ui/goose2/src/features/providers/api/customProviders.test.ts b/ui/goose2/src/features/providers/api/customProviders.test.ts index 992713b75c..fbe34c0a68 100644 --- a/ui/goose2/src/features/providers/api/customProviders.test.ts +++ b/ui/goose2/src/features/providers/api/customProviders.test.ts @@ -106,6 +106,7 @@ describe("custom provider API", () => { headers: input.headers ?? {}, apiKeyEnv: "ACME_AI_API_KEY", apiKeySet: true, + preservesThinking: true, }, editable: true, status: { providerId: "acme_ai", isConfigured: true }, diff --git a/ui/goose2/src/features/providers/hooks/useCustomProviders.test.tsx b/ui/goose2/src/features/providers/hooks/useCustomProviders.test.tsx index 01a5f49f4a..755635fb22 100644 --- a/ui/goose2/src/features/providers/hooks/useCustomProviders.test.tsx +++ b/ui/goose2/src/features/providers/hooks/useCustomProviders.test.tsx @@ -84,6 +84,7 @@ describe("useCustomProviders", () => { headers: {}, requiresAuth: true, apiKeySet: true, + preservesThinking: true, }, editable: true, status: { providerId: "acme_ai", isConfigured: true }, diff --git a/ui/goose2/src/features/providers/lib/customProviderDraft.test.ts b/ui/goose2/src/features/providers/lib/customProviderDraft.test.ts index 6c642c4bb6..11c32fc4c0 100644 --- a/ui/goose2/src/features/providers/lib/customProviderDraft.test.ts +++ b/ui/goose2/src/features/providers/lib/customProviderDraft.test.ts @@ -118,6 +118,7 @@ describe("custom provider helper functions", () => { basePath: "/v1", apiKeyEnv: "ACME_AI_API_KEY", apiKeySet: true, + preservesThinking: true, }, editable: true, status: { providerId: "acme_ai", isConfigured: true }, diff --git a/ui/sdk/src/generated/types.gen.ts b/ui/sdk/src/generated/types.gen.ts index 8b880d3090..6b5639510b 100644 --- a/ui/sdk/src/generated/types.gen.ts +++ b/ui/sdk/src/generated/types.gen.ts @@ -401,6 +401,7 @@ export type CustomProviderCreateRequest = { requiresAuth: boolean; catalogProviderId?: string | null; basePath?: string | null; + preservesThinking?: boolean | null; }; export type CustomProviderCreateResponse = { @@ -463,6 +464,7 @@ export type CustomProviderConfigDto = { basePath?: string | null; apiKeyEnv?: string | null; apiKeySet: boolean; + preservesThinking: boolean; }; /** @@ -482,6 +484,7 @@ export type CustomProviderUpdateRequest = { requiresAuth: boolean; catalogProviderId?: string | null; basePath?: string | null; + preservesThinking?: boolean | null; }; export type CustomProviderUpdateResponse = { diff --git a/ui/sdk/src/generated/zod.gen.ts b/ui/sdk/src/generated/zod.gen.ts index 6d72c52acb..8b0951df18 100644 --- a/ui/sdk/src/generated/zod.gen.ts +++ b/ui/sdk/src/generated/zod.gen.ts @@ -368,6 +368,10 @@ export const zCustomProviderCreateRequest = z.object({ basePath: z.union([ z.string(), z.null() + ]).optional(), + preservesThinking: z.union([ + z.boolean(), + z.null() ]).optional() }); @@ -433,7 +437,8 @@ export const zCustomProviderConfigDto = z.object({ z.string(), z.null() ]).optional(), - apiKeySet: z.boolean() + apiKeySet: z.boolean(), + preservesThinking: z.boolean() }); export const zCustomProviderReadResponse = z.object({ @@ -468,6 +473,10 @@ export const zCustomProviderUpdateRequest = z.object({ basePath: z.union([ z.string(), z.null() + ]).optional(), + preservesThinking: z.union([ + z.boolean(), + z.null() ]).optional() });