feat(providers): honor supports_vision declared on custom provider mo… (#12769)

Co-authored-by: Matt Maguire <admin@m.teachmaths.org>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Jasper Hugo <jasper@jasperhugo.com>

crates/goose-provider-types/src/base.rs

@@ -266,6 +266,9 @@ pub struct ModelInfo {
     /// Whether this model supports reasoning/thinking controls
     #[serde(default)]
     pub reasoning: bool,
+    /// Whether this model accepts image input; when set, overrides canonical detection
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub supports_vision: Option<bool>,
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub thinking_preservation_format: Option<ThinkingPreservationFormat>,
     /// Static params merged into the request body for this model.
@@ -284,6 +287,7 @@ impl ModelInfo {
             currency: None,
             supports_cache_control: None,
             reasoning: false,
+            supports_vision: None,
             thinking_preservation_format: None,
             request_params: None,
         }
@@ -299,6 +303,11 @@ impl ModelInfo {
         self
     }
 
+    pub fn with_vision_support(mut self, supports_vision: bool) -> Self {
+        self.supports_vision = Some(supports_vision);
+        self
+    }
+
     /// Create a new ModelInfo with cost information (per token)
     pub fn with_cost(
         name: impl Into<String>,
@@ -315,6 +324,7 @@ impl ModelInfo {
             currency: Some("$".to_string()),
             supports_cache_control: None,
             reasoning: false,
+            supports_vision: None,
             thinking_preservation_format: None,
             request_params: None,
         }
@@ -365,11 +375,17 @@ pub fn model_info_for_provider_model(provider_name: &str, model_name: &str) -> M
         currency: None,
         supports_cache_control: None,
         reasoning,
+        supports_vision: None,
         thinking_preservation_format: None,
         request_params: None,
     }
 }
 
+/// Model IDs are case-sensitive; declarations apply only to the requested ID.
+pub fn find_declared_model<'a>(models: &'a [ModelInfo], model_name: &str) -> Option<&'a ModelInfo> {
+    models.iter().find(|model| model.name == model_name)
+}
+
 /// Build `ModelInfo` for discovered model names, preferring metadata declared in
 /// provider configuration over the canonical registry.
 ///
@@ -1102,6 +1118,7 @@ mod tests {
             currency: None,
             supports_cache_control: None,
             reasoning: false,
+            supports_vision: None,
             thinking_preservation_format: None,
             request_params: None,
         };
@@ -1117,6 +1134,7 @@ mod tests {
             currency: None,
             supports_cache_control: None,
             reasoning: false,
+            supports_vision: None,
             thinking_preservation_format: None,
             request_params: None,
         };
@@ -1132,6 +1150,7 @@ mod tests {
             currency: None,
             supports_cache_control: None,
             reasoning: false,
+            supports_vision: None,
             thinking_preservation_format: None,
             request_params: None,
         };

crates/goose-provider-types/tests/declared_model.rs

@@ -0,0 +1,30 @@
+use goose_provider_types::base::{find_declared_model, ModelInfo};
+
+#[test]
+fn exact_model_ids_win_over_case_insensitive_collisions() {
+    let models = [
+        ModelInfo::new("Foo").with_vision_support(false),
+        ModelInfo::new("foo").with_vision_support(true),
+    ];
+
+    assert_eq!(
+        find_declared_model(&models, "foo").unwrap().supports_vision,
+        Some(true)
+    );
+    assert_eq!(
+        find_declared_model(&models, "Foo").unwrap().supports_vision,
+        Some(false)
+    );
+    assert!(find_declared_model(&models, "FOO").is_none());
+}
+
+#[test]
+fn undeclared_case_variants_do_not_inherit_model_metadata() {
+    let models = [ModelInfo::new("Foo").with_vision_support(true)];
+
+    assert_eq!(find_declared_model(&models, "Foo").unwrap().name, "Foo");
+    assert!(find_declared_model(&models, "foo").is_none());
+    assert!(find_declared_model(&models, "FOO").is_none());
+    assert!(find_declared_model(&models, "another-model").is_none());
+    assert!(find_declared_model(&[], "Foo").is_none());
+}

crates/goose-providers/src/azure_foundry.rs

@@ -417,6 +417,7 @@ fn model_info_for_deployment(deployment_name: &str, model_name: &str) -> ModelIn
         reasoning: canonical
             .and_then(|model| model.reasoning)
             .unwrap_or_else(|| ModelConfig::new(model_name).is_reasoning_model()),
+        supports_vision: None,
         thinking_preservation_format: None,
         request_params: None,
     }

crates/goose-providers/src/databricks.rs

@@ -520,6 +520,7 @@ impl DatabricksProvider {
             currency: None,
             supports_cache_control: None,
             reasoning,
+            supports_vision: None,
             thinking_preservation_format: None,
             request_params: None,
         }

crates/goose-providers/src/openai.rs

@@ -1,5 +1,8 @@
 use super::api_client::ApiClient;
-use super::base::{known_models_from_registry, ConfigKey, ModelInfo, Provider, ProviderMetadata};
+use super::base::{
+    find_declared_model, known_models_from_registry, ConfigKey, ModelInfo, Provider,
+    ProviderMetadata,
+};
 use super::retry::ProviderRetry;
 use crate::api_client::{AuthMethod, TlsConfig};
 use crate::conversation::message::Message;
@@ -279,6 +282,7 @@ impl OpenAiProvider {
         tools: &[Tool],
     ) -> Result<MessageStream, ProviderError> {
         let mut request_config = model_config.clone();
+        request_config.supports_vision = self.vision_support(model_config);
         if self.native_openai && request_config.reasoning.is_none() {
             let canonical = goose_provider_types::canonical::maybe_get_canonical_model(
                 "openai",
@@ -463,10 +467,13 @@ impl OpenAiProvider {
     }
 
     fn declared_model(&self, model_name: &str) -> Option<&ModelInfo> {
-        self.custom_models
-            .as_ref()?
-            .iter()
-            .find(|m| m.name == model_name)
+        find_declared_model(self.custom_models.as_deref()?, model_name)
+    }
+
+    fn vision_support(&self, model_config: &ModelConfig) -> Option<bool> {
+        self.declared_model(&model_config.model_name)
+            .and_then(|model| model.supports_vision)
+            .or(model_config.supports_vision)
     }
 
     fn sanitize_request_for_compat(
@@ -806,6 +813,7 @@ impl Provider for OpenAiProvider {
             let declared_model = self.declared_model(&model_config.model_name);
             let thinking_preservation_format =
                 declared_model.and_then(|m| m.thinking_preservation_format);
+            let supports_vision = self.vision_support(model_config).unwrap_or_default();
 
             let mut payload = create_request_with_options(
                 model_config,
@@ -817,7 +825,7 @@ impl Provider for OpenAiProvider {
                 OpenAiFormatOptions {
                     preserve_thinking_context: self.preserve_thinking_context
                         || thinking_preservation_format.is_some(),
-                    supports_vision: model_config.supports_vision.unwrap_or_default(),
+                    supports_vision,
                     thinking_preservation_format,
                 },
             )?;

crates/goose-providers/tests/declared_vision.rs

@@ -0,0 +1,127 @@
+use goose_providers::{
+    api_client::{ApiClient, AuthMethod},
+    base::{ModelInfo, Provider},
+    conversation::message::Message,
+    model::ModelConfig,
+    openai::OpenAiProviderBuilder,
+};
+use rmcp::model::{CallToolRequestParams, CallToolResult, ContentBlock};
+use serde_json::{json, Value};
+use wiremock::{
+    matchers::{method, path},
+    Mock, MockServer, ResponseTemplate,
+};
+
+async fn image_request(models: Vec<ModelInfo>, model: ModelConfig, endpoint: &str) -> Value {
+    let server = MockServer::start().await;
+    let response = if endpoint == "responses" {
+        json!({
+            "id": "response-1", "object": "response", "created_at": 0,
+            "status": "completed", "model": model.model_name,
+            "output": [{"type": "message", "role": "assistant", "content": [
+                {"type": "output_text", "text": "Seen"}
+            ]}]
+        })
+    } else {
+        json!({
+            "id": "response-1", "model": model.model_name,
+            "choices": [{"message": {"role": "assistant", "content": "Seen"},
+                         "finish_reason": "stop"}]
+        })
+    };
+    Mock::given(method("POST"))
+        .and(path(format!("/v1/{endpoint}")))
+        .respond_with(ResponseTemplate::new(200).set_body_json(response))
+        .expect(1)
+        .mount(&server)
+        .await;
+
+    let client = ApiClient::new_with_tls(server.uri(), AuthMethod::NoAuth, None)
+        .unwrap()
+        .with_loopback_http_only()
+        .unwrap();
+    let provider = OpenAiProviderBuilder::new(client)
+        .name("custom_vision")
+        .base_path(format!("v1/{endpoint}"))
+        .custom_models(Some(models))
+        .supports_streaming(false)
+        .build();
+    let messages = [
+        Message::user()
+            .with_text("Describe these images")
+            .with_image("dXNlcg==", "image/png"),
+        Message::assistant()
+            .with_tool_request("tool-1", Ok(CallToolRequestParams::new("screenshot"))),
+        Message::user().with_tool_response(
+            "tool-1",
+            Ok(CallToolResult::success(vec![ContentBlock::image(
+                "dG9vbA==",
+                "image/png",
+            )])),
+        ),
+    ];
+    provider
+        .complete(&model, "Describe images", &messages, &[])
+        .await
+        .unwrap();
+    let requests = server.received_requests().await.unwrap();
+    assert_eq!(requests.len(), 1);
+    let body: Value = requests[0].body_json().unwrap();
+    assert_eq!(body["model"], model.model_name);
+    body
+}
+
+fn assert_images(body: &Value, expected: bool) {
+    let request = body.to_string();
+    if expected {
+        assert!(request.contains("data:image/png;base64,dXNlcg=="), "{body}");
+        assert!(request.contains("data:image/png;base64,dG9vbA=="), "{body}");
+    } else {
+        assert!(!request.contains("data:image"), "{body}");
+        assert!(request.contains("model does not support vision"), "{body}");
+    }
+}
+
+#[tokio::test]
+async fn declared_vision_controls_user_and_tool_images_on_both_endpoints() {
+    for endpoint in ["chat/completions", "responses"] {
+        for (declared, configured, expected) in [
+            (Some(true), Some(false), true),
+            (Some(false), Some(true), false),
+            (Some(true), None, true),
+            (Some(false), None, false),
+            (None, Some(true), true),
+            (None, Some(false), false),
+            (None, None, false),
+        ] {
+            let mut info = ModelInfo::new("local-vlm");
+            info.supports_vision = declared;
+            let mut model = ModelConfig::new("local-vlm");
+            model.supports_vision = configured;
+            let body = image_request(vec![info], model, endpoint).await;
+            assert_images(&body, expected);
+        }
+    }
+}
+
+#[tokio::test]
+async fn model_matching_never_borrows_vision_from_a_case_collision() {
+    for endpoint in ["chat/completions", "responses"] {
+        let models = vec![
+            ModelInfo::new("Foo").with_vision_support(false),
+            ModelInfo::new("foo").with_vision_support(true),
+        ];
+        for (name, expected) in [("Foo", false), ("foo", true), ("FOO", false)] {
+            let body = image_request(models.clone(), ModelConfig::new(name), endpoint).await;
+            assert_images(&body, expected);
+        }
+
+        let body = image_request(
+            vec![ModelInfo::new("Foo").with_vision_support(true)],
+            ModelConfig::new("foo").with_vision_support(false),
+            endpoint,
+        )
+        .await;
+        assert_images(&body, false);
+    }
+}

crates/goose/src/config/declarative_providers.rs

@@ -307,6 +307,7 @@ pub fn update_custom_provider(params: UpdateCustomProviderParams) -> Result<()>
                     model.supports_cache_control = model
                         .supports_cache_control
                         .or(existing.supports_cache_control);
+                    model.supports_vision = model.supports_vision.or(existing.supports_vision);
                     model.reasoning |= existing.reasoning;
                     model.thinking_preservation_format = model
                         .thinking_preservation_format
@@ -611,6 +612,7 @@ mod tests {
                 currency: None,
                 supports_cache_control: None,
                 reasoning: false,
+                supports_vision: None,
                 thinking_preservation_format: None,
                 request_params: None,
             }],
@@ -799,7 +801,8 @@ mod tests {
         let temp_root = temp_dir.path().display().to_string();
         let _guard = env_lock::lock_env([("GOOSE_PATH_ROOT", Some(temp_root.as_str()))]);
 
-        let mut model = ModelInfo::with_cost("large-model", 1_048_576, 0.000002, 0.000006);
+        let mut model = ModelInfo::with_cost("large-model", 1_048_576, 0.000002, 0.000006)
+            .with_vision_support(true);
         model.request_params = Some(HashMap::from([(
             "temperature".to_string(),
             serde_json::json!(0.25),
@@ -821,23 +824,35 @@ mod tests {
         })
         .unwrap();
 
-        update_custom_provider(UpdateCustomProviderParams {
-            id: created.name.clone(),
-            engine: "openai".to_string(),
-            display_name: created.display_name.clone(),
-            api_url: created.base_url.clone(),
-            api_key: None,
-            models: vec![ModelInfo::new("large-model").with_context_limit(2_097_152)],
-            supports_streaming: Some(true),
-            headers: None,
-            requires_auth: false,
-            catalog_provider_id: None,
-            base_path: None,
-            toolshim: false,
-            preserves_thinking: None,
-            auth: None,
-        })
-        .unwrap();
+        for (declared, expected) in [
+            (None, true),
+            (Some(false), false),
+            (None, false),
+            (Some(true), true),
+        ] {
+            let mut replacement = ModelInfo::new("large-model").with_context_limit(2_097_152);
+            replacement.supports_vision = declared;
+            update_custom_provider(UpdateCustomProviderParams {
+                id: created.name.clone(),
+                engine: "openai".to_string(),
+                display_name: created.display_name.clone(),
+                api_url: created.base_url.clone(),
+                api_key: None,
+                models: vec![replacement],
+                supports_streaming: Some(true),
+                headers: None,
+                requires_auth: false,
+                catalog_provider_id: None,
+                base_path: None,
+                toolshim: false,
+                preserves_thinking: None,
+                auth: None,
+            })
+            .unwrap();
+
+            let loaded = load_provider(&created.name).unwrap();
+            assert_eq!(loaded.config.models[0].supports_vision, Some(expected));
+        }
 
         let loaded = load_provider(&created.name).unwrap();
         let model = &loaded.config.models[0];

crates/goose/src/providers/provider_registry.rs

@@ -1,5 +1,8 @@
 use super::api_client::TlsConfig;
-use super::base::{ConfigKey, ModelInfo, Provider, ProviderDef, ProviderMetadata, ProviderType};
+use super::base::{
+    find_declared_model, ConfigKey, ModelInfo, Provider, ProviderDef, ProviderMetadata,
+    ProviderType,
+};
 use super::inventory::{InventoryIdentityInput, InventoryRegistration, InventoryResolvers};
 use crate::config::{DeclarativeProviderConfig, ExtensionConfig};
 use anyhow::Result;
@@ -69,6 +72,14 @@ impl ProviderEntry {
         if self.toolshim_enabled(model.toolshim) {
             model = model.with_toolshim(true);
         }
+        // Declared model metadata is authoritative over canonical detection, which
+        // may already have populated supports_vision earlier in materialization.
+        if let Some(supports_vision) =
+            find_declared_model(&self.metadata.known_models, &model.model_name)
+                .and_then(|m| m.supports_vision)
+        {
+            model.supports_vision = Some(supports_vision);
+        }
         crate::model_config::materialize_model_config(&self.metadata.name, model)
     }
 
@@ -456,4 +467,92 @@ mod tests {
         assert!(registry.entries["custom_default"].toolshim_enabled(true));
         assert!(!registry.entries["custom_default"].toolshim_enabled(false));
     }
+
+    #[test]
+    fn canonical_metadata_preserves_explicit_vision_overrides() {
+        let mut registry = ProviderRegistry::new(None);
+        registry.register::<OpenAiProviderDef>(false);
+        let entry = &registry.entries["openai"];
+
+        for (name, canonical_vision) in [("gpt-4o", true), ("gpt-4", false)] {
+            let detected = entry
+                .normalize_model_config(ModelConfig::new(name))
+                .unwrap();
+            assert_eq!(detected.supports_vision, Some(canonical_vision));
+
+            let overridden = entry
+                .normalize_model_config(
+                    ModelConfig::new(name).with_vision_support(!canonical_vision),
+                )
+                .unwrap();
+            assert_eq!(overridden.supports_vision, Some(!canonical_vision));
+        }
+    }
+
+    #[test]
+    fn case_collisions_do_not_change_the_selected_models_vision() {
+        let mut registry = ProviderRegistry::new(None);
+        let mut config = test_config();
+        config.models = vec![
+            ModelInfo::new("Foo").with_vision_support(false),
+            ModelInfo::new("foo").with_vision_support(true),
+            ModelInfo::new("Bar").with_vision_support(true),
+        ];
+        registry.register_with_name::<OpenAiProviderDef, _, _>(
+            &config,
+            ProviderType::Custom,
+            false,
+            |_| unreachable!("constructor is not used by this test"),
+            || Ok(InventoryIdentityInput::new("custom_hf", "custom_hf")),
+        );
+        let entry = &registry.entries["custom_hf"];
+
+        for (name, expected) in [
+            ("Foo", Some(false)),
+            ("foo", Some(true)),
+            ("FOO", None),
+            ("bar", None),
+        ] {
+            let model = entry
+                .normalize_model_config(ModelConfig::new(name))
+                .unwrap();
+            assert_eq!(model.supports_vision, expected);
+        }
+    }
+
+    #[test]
+    fn declared_supports_vision_overrides_detected_value() {
+        let mut registry = ProviderRegistry::new(None);
+        let mut config = test_config();
+        config.models = vec![
+            ModelInfo::new("vision-model").with_vision_support(true),
+            ModelInfo::new("text-model").with_vision_support(false),
+            ModelInfo::new("undeclared-model"),
+        ];
+        registry.register_with_name::<OpenAiProviderDef, _, _>(
+            &config,
+            ProviderType::Custom,
+            false,
+            |_| unreachable!("constructor is not used by this test"),
+            || Ok(InventoryIdentityInput::new("custom_hf", "custom_hf")),
+        );
+        let entry = &registry.entries["custom_hf"];
+
+        let mut detected_without_vision = ModelConfig::new("vision-model");
+        detected_without_vision.supports_vision = Some(false);
+        let mut detected_with_vision = ModelConfig::new("text-model");
+        detected_with_vision.supports_vision = Some(true);
+
+        let vision = entry
+            .normalize_model_config(detected_without_vision)
+            .unwrap();
+        let text = entry.normalize_model_config(detected_with_vision).unwrap();
+        let undeclared = entry
+            .normalize_model_config(ModelConfig::new("undeclared-model"))
+            .unwrap();
+
+        assert_eq!(vision.supports_vision, Some(true));
+        assert_eq!(text.supports_vision, Some(false));
+        assert_eq!(undeclared.supports_vision, None);
+    }
 }

crates/goose/tests/declared_vision_registry.rs

@@ -0,0 +1,95 @@
+use goose::config::declarative_providers::{create_custom_provider, CreateCustomProviderParams};
+use goose::conversation::message::Message;
+use goose::providers::{get_from_registry, refresh_custom_providers};
+use goose_providers::{base::ModelInfo, model::ModelConfig};
+use serde_json::json;
+use wiremock::{
+    matchers::{method, path},
+    Mock, MockServer, ResponseTemplate,
+};
+
+#[tokio::test]
+async fn persisted_vision_declarations_control_registry_requests() {
+    let root = tempfile::tempdir().unwrap();
+    let root_path = root.path().to_str().unwrap();
+    let _guard = env_lock::lock_env([("GOOSE_PATH_ROOT", Some(root_path))]);
+
+    for endpoint in ["chat/completions", "responses"] {
+        for (declared, configured, expected) in [
+            (Some(true), false, true),
+            (Some(false), true, false),
+            (None, true, true),
+            (None, false, false),
+        ] {
+            let server = MockServer::start().await;
+            let response = if endpoint == "responses" {
+                json!({
+                    "id": "response-1", "object": "response", "created_at": 0,
+                    "status": "completed", "model": "local-vlm",
+                    "output": [{"type": "message", "role": "assistant", "content": [
+                        {"type": "output_text", "text": "Seen"}
+                    ]}]
+                })
+            } else {
+                json!({
+                    "id": "response-1", "model": "local-vlm",
+                    "choices": [{"message": {"role": "assistant", "content": "Seen"},
+                                 "finish_reason": "stop"}]
+                })
+            };
+            Mock::given(method("POST"))
+                .and(path(format!("/v1/{endpoint}")))
+                .respond_with(ResponseTemplate::new(200).set_body_json(response))
+                .expect(1)
+                .mount(&server)
+                .await;
+
+            let mut info = ModelInfo::new("local-vlm");
+            info.supports_vision = declared;
+            let saved = create_custom_provider(CreateCustomProviderParams {
+                engine: "openai".to_string(),
+                display_name: "Declared Vision Self Test".to_string(),
+                api_url: server.uri(),
+                api_key: None,
+                models: vec![info],
+                supports_streaming: Some(false),
+                headers: None,
+                requires_auth: false,
+                catalog_provider_id: None,
+                base_path: Some(format!("v1/{endpoint}")),
+                toolshim: false,
+                preserves_thinking: None,
+                auth: None,
+            })
+            .unwrap();
+            refresh_custom_providers().await.unwrap();
+            let entry = get_from_registry(&saved.name).await.unwrap();
+            let model = entry
+                .normalize_model_config(
+                    ModelConfig::new("local-vlm").with_vision_support(configured),
+                )
+                .unwrap();
+            let provider = entry.create(vec![]).await.unwrap();
+            provider
+                .complete(
+                    &model,
+                    "Describe images",
+                    &[Message::user()
+                        .with_text("Describe this image")
+                        .with_image("dXNlcg==", "image/png")],
+                    &[],
+                )
+                .await
+                .unwrap();
+            let requests = server.received_requests().await.unwrap();
+            assert_eq!(requests.len(), 1);
+            let body: serde_json::Value = requests[0].body_json().unwrap();
+            assert_eq!(body["model"], "local-vlm");
+            assert_eq!(
+                body.to_string().contains("data:image/png;base64,dXNlcg=="),
+                expected,
+                "{body}"
+            );
+        }
+    }
+}

goose-self-test.yaml

@@ -10,6 +10,7 @@ activities:
   - Validate shell command execution and error handling
   - Validate Azure AI Foundry provider routing and deployment aliases
   - Validate the configurable Databricks AI Gateway path across all inference routes
+  - Validate declared vision through persisted custom-provider configuration and outbound requests
   - Analyze code structure and parsing capabilities
   - Test extension discovery and management
   - Test load tool for knowledge injection and discovery
@@ -27,7 +28,7 @@ parameters:
     input_type: string
     requirement: optional
     default: "all"
-    description: "Which test phases to run: all, basic, extensions, delegation, reasoning, zai-coding-plan, acp-effort, compaction, advanced"
+    description: "Which test phases to run: all, basic, extensions, delegation, declared-vision, reasoning, zai-coding-plan, acp-effort, compaction, advanced"
 
   - key: test_depth
     input_type: string
@@ -344,6 +345,24 @@ prompt: |
   Log results to: {{ workspace_dir }}/phase3_delegation.md
   {% endif %}
 
+  {% if test_phases == "all" or "declared-vision" in test_phases %}
+  ## Declared Custom-Provider Vision
+
+  When running from a goose source checkout with a Rust toolchain:
+  1. Run `cargo test -p goose --features rustls-tls --test declared_vision_registry`.
+  2. Verify saved custom-provider declarations survive registry reload and control actual
+     image forwarding on both chat completions and Responses. Declared true and false
+     override opposing model settings; an absent declaration preserves those settings.
+  3. Run `cargo test -p goose-providers --features rustls-tls --test declared_vision`.
+  4. Verify actual user and tool-result image requests follow the same precedence and
+     undeclared case variants never inherit another model's declaration.
+  These tests use a local HTTP mock and isolated configuration, requiring no credentials
+  or downloaded vision model. If the source checkout or toolchain is unavailable, mark
+  this phase as SKIPPED.
+
+  Log results to: {{ workspace_dir }}/declared_vision.md
+  {% endif %}
+
   {% if test_phases == "all" or "vision" in test_phases %}
   ## 📷 PHASE 3B: Local Inference Vision Testing