From faeeeef59be7fe72a7975ee5e3d7dd597f97a733 Mon Sep 17 00:00:00 2001 From: Hunter Bown Date: Wed, 10 Jun 2026 16:25:03 -0700 Subject: [PATCH] =?UTF-8?q?fix(subagent):=20un-hardcode=20DeepSeek=20from?= =?UTF-8?q?=20model=20validation=20=E2=80=94=20accept=20any=20provider=20i?= =?UTF-8?q?d=20(#3018)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two changes to let non-DeepSeek providers use their own model IDs: 1. config.rs: add requested_model_for_provider() — DeepSeek providers use strict normalize_model_name(); all others accept any non-empty string via normalize_custom_model_id(). 2. subagent/mod.rs: normalize_requested_subagent_model() now takes an ApiProvider parameter and delegates to requested_model_for_provider. Error messages name the active provider and list accepted model IDs from model_completion_names_for_provider() instead of hardcoding "Expected a DeepSeek model id". parse_optional_subagent_model() keeps basic trimming-only validation; provider-aware checks are deferred to the spawn path where the runtime is available. --- crates/tui/src/config.rs | 13 ++++++++ crates/tui/src/tools/subagent/mod.rs | 45 ++++++++++++++++++++++++---- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/crates/tui/src/config.rs b/crates/tui/src/config.rs index d8f17de4..37035a38 100644 --- a/crates/tui/src/config.rs +++ b/crates/tui/src/config.rs @@ -575,6 +575,19 @@ pub(crate) fn normalize_custom_model_id(model: &str) -> Option { } } +/// Validate a user-requested model id against the active provider (#3018). +/// +/// DeepSeek providers use the strict `normalize_model_name` gate (official +/// API only accepts DeepSeek IDs). All other providers pass any non-empty, +/// non-control-character string through — the provider API is the authority. +#[must_use] +pub fn requested_model_for_provider(provider: ApiProvider, model: &str) -> Option { + match provider { + ApiProvider::Deepseek | ApiProvider::DeepseekCN => normalize_model_name(model), + _ => normalize_custom_model_id(model), + } +} + fn canonical_official_deepseek_model_id(model: &str) -> Option<&'static str> { match model.trim().to_ascii_lowercase().as_str() { "deepseek-v4-pro" diff --git a/crates/tui/src/tools/subagent/mod.rs b/crates/tui/src/tools/subagent/mod.rs index 61265544..a0af6aa5 100644 --- a/crates/tui/src/tools/subagent/mod.rs +++ b/crates/tui/src/tools/subagent/mod.rs @@ -5048,18 +5048,41 @@ fn with_default_fork_context(mut input: Value, default: bool) -> Value { pub(crate) fn normalize_requested_subagent_model( value: &str, field: &str, + provider: crate::config::ApiProvider, ) -> Result { let trimmed = value.trim(); if trimmed.is_empty() { return Err(ToolError::invalid_input(format!("{field} cannot be blank"))); } - crate::config::normalize_model_name(trimmed).ok_or_else(|| { + // #3018: Use provider-aware validation so non-DeepSeek providers can + // accept their own model IDs instead of failing with "Expected a + // DeepSeek model id". + crate::config::requested_model_for_provider(provider, trimmed).ok_or_else(|| { + let valid_names = crate::config::model_completion_names_for_provider(provider); + let valid_hint = if valid_names.is_empty() { + String::new() + } else { + format!(" (accepted: {})", valid_names.join(", ")) + }; ToolError::invalid_input(format!( - "Invalid {field} '{trimmed}'. Expected a DeepSeek model id such as deepseek-v4-pro or deepseek-v4-flash" + "Invalid {field} '{trimmed}' for provider {}{valid_hint}", + provider_name_for_error(provider) )) }) } +fn provider_name_for_error(provider: crate::config::ApiProvider) -> &'static str { + match provider { + crate::config::ApiProvider::Deepseek | crate::config::ApiProvider::DeepseekCN => { + "DeepSeek" + } + crate::config::ApiProvider::Openai | crate::config::ApiProvider::OpenaiCodex => "OpenAI", + crate::config::ApiProvider::Moonshot => "Moonshot", + crate::config::ApiProvider::Ollama => "Ollama", + _ => "this provider", + } +} + pub(crate) fn configured_model_for_role_or_type( runtime: &SubAgentRuntime, role: Option<&str>, @@ -5074,8 +5097,12 @@ pub(crate) fn configured_model_for_role_or_type( for key in keys { if let Some(model) = runtime.role_models.get(&key) { - return normalize_requested_subagent_model(model, &format!("subagents.{key}.model")) - .map(Some); + return normalize_requested_subagent_model( + model, + &format!("subagents.{key}.model"), + runtime.client.api_provider(), + ) + .map(Some); } } Ok(None) @@ -5264,7 +5291,15 @@ fn message_response_text(blocks: &[ContentBlock]) -> String { fn parse_optional_subagent_model(input: &Value, key: &str) -> Result, ToolError> { match input.get(key) { None | Some(Value::Null) => Ok(None), - Some(Value::String(value)) => normalize_requested_subagent_model(value, key).map(Some), + Some(Value::String(value)) => { + let trimmed = value.trim(); + if trimmed.is_empty() { + return Err(ToolError::invalid_input(format!("{key} cannot be blank"))); + } + // #3018: Basic parsing only — provider-aware validation is deferred + // to the spawn path where the runtime's ApiProvider is available. + Ok(Some(trimmed.to_string())) + } Some(_) => Err(ToolError::invalid_input(format!("{key} must be a string"))), } }