Skip to content

Commit 4276c7c

Browse files
committed
fix: relax canonical model listing for custom providers
My company has a custom proxy that exposes an Anthropic-compatible API. When connecting Goose, it correctly calls the /v1/models endpoint to enumerate available models—but several models like glm-5 and kimi-k2 were mysteriously missing from the dropdown. The models worked fine when manually specified in config.yaml, yet the UI refused to show them. After debugging, it turns out fetch_recommended_models was filtering out any model that couldn't be mapped to a canonical model ID in Goose's bundled registry. The code tried to look up each model name in the registry to get metadata like release dates and tool support—but if the lookup failed, the model was discarded entirely. This is inconsistent with how the runtime actually works. When you configure glm-5 in config.yaml, the ModelConfig::with_canonical_limits function gracefully handles missing canonical metadata—it just skips filling in defaults. The model runs perfectly fine without it. So the listing code was treating the registry as required while the execution code treated it as optional. If you think about it, why should a model be blocked from appearing in a dropdown just because Goose doesn't know its release date? The metadata is nice to have, not a prerequisite for using the model. Fix: - Changed fetch_recommended_models to branch on provider_type() - Built-in OpenAI/Anthropic (with default host): strict filtering (require canonical metadata, text modality, tool support) - Custom/declarative providers: relaxed filtering (allow unknown models, use canonical metadata when available for sorting) - Alternate-host OpenAI/Anthropic (OPENAI_HOST/ANTHROPIC_HOST set): treated as custom - Default provider_type is Custom, only OpenAI/Anthropic override to Builtin when using canonical names and default host - Registry load errors are propagated instead of silently returning empty list - Added regression tests for custom-include vs builtin-exclude behavior Signed-off-by: Pete Gonzalez <4673363+octogonz@users.noreply.github.com>
1 parent e2fc8dd commit 4276c7c

3 files changed

Lines changed: 151 additions & 27 deletions

File tree

crates/goose/src/providers/anthropic.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ pub struct AnthropicProvider {
5555
model: ModelConfig,
5656
supports_streaming: bool,
5757
name: String,
58+
is_custom_host: bool,
5859
}
5960

6061
impl AnthropicProvider {
@@ -80,6 +81,7 @@ impl AnthropicProvider {
8081
model,
8182
supports_streaming: true,
8283
name: ANTHROPIC_PROVIDER_NAME.to_string(),
84+
is_custom_host: host != "https://api.anthropic.com",
8385
})
8486
}
8587

@@ -124,6 +126,7 @@ impl AnthropicProvider {
124126
model,
125127
supports_streaming,
126128
name: config.name.clone(),
129+
is_custom_host: true,
127130
})
128131
}
129132

@@ -189,6 +192,14 @@ impl Provider for AnthropicProvider {
189192
&self.name
190193
}
191194

195+
fn provider_type(&self) -> crate::providers::base::ProviderType {
196+
if self.name == ANTHROPIC_PROVIDER_NAME && !self.is_custom_host {
197+
crate::providers::base::ProviderType::Builtin
198+
} else {
199+
crate::providers::base::ProviderType::Custom
200+
}
201+
}
202+
192203
fn get_model_config(&self) -> ModelConfig {
193204
self.model.clone()
194205
}

crates/goose/src/providers/base.rs

Lines changed: 128 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,11 @@ pub trait Provider: Send + Sync {
456456
/// Get the name of this provider instance
457457
fn get_name(&self) -> &str;
458458

459+
/// Get the provider classification for model listing behavior.
460+
fn provider_type(&self) -> ProviderType {
461+
ProviderType::Custom
462+
}
463+
459464
/// Primary streaming method that all providers must implement.
460465
///
461466
/// Note: Do not add `#[instrument]` here — the call sites (`complete` and
@@ -534,40 +539,77 @@ pub trait Provider: Send + Sync {
534539
Ok(vec![])
535540
}
536541

537-
/// Fetch models filtered by canonical registry and usability
542+
/// Fetch models sorted by release date when available from canonical registry.
543+
/// For built-in providers, models must be in the canonical registry and pass
544+
/// usability checks (text modality, tool support).
545+
/// For custom providers, all models are included; unknown models sort alphabetically.
538546
async fn fetch_recommended_models(&self) -> Result<Vec<String>, ProviderError> {
539547
let all_models = self.fetch_supported_models().await?;
540548

549+
// Try to load the canonical registry for metadata.
550+
// If it fails, propagate the error - we don't want to silently return
551+
// an empty list or all models when we can't properly validate.
541552
let registry = CanonicalModelRegistry::bundled().map_err(|e| {
542553
ProviderError::ExecutionError(format!("Failed to load canonical registry: {}", e))
543554
})?;
544555

545556
let provider_name = self.get_name();
557+
let provider_type = self.provider_type();
558+
let uses_strict_model_filtering = matches!(
559+
provider_type,
560+
ProviderType::Builtin | ProviderType::Preferred
561+
);
562+
let allows_unknown_models = matches!(
563+
provider_type,
564+
ProviderType::Custom | ProviderType::Declarative
565+
);
566+
let toolshim_enabled = self.get_model_config().toolshim;
546567

547-
// Get all text-capable models with their release dates
568+
// Build list of (model_name, release_date) for sorting.
569+
// For built-in providers, filter out models without canonical metadata
570+
// or that don't pass usability checks.
548571
let mut models_with_dates: Vec<(String, Option<String>)> = all_models
549572
.iter()
550573
.filter_map(|model| {
551-
let canonical_id = map_to_canonical_model(provider_name, model, registry)?;
552-
553-
let (provider, model_name) = canonical_id.split_once('/')?;
554-
let canonical_model = registry.get(provider, model_name)?;
574+
let canonical = map_to_canonical_model(provider_name, model, registry).and_then(
575+
|canonical_id| {
576+
let (provider, model_name) = canonical_id.split_once('/')?;
577+
registry.get(provider, model_name)
578+
},
579+
);
580+
581+
match canonical {
582+
Some(cm) => {
583+
// Model has canonical metadata - apply checks
584+
// Check text modality
585+
if !cm
586+
.modalities
587+
.input
588+
.contains(&crate::providers::canonical::Modality::Text)
589+
{
590+
return None;
591+
}
555592

556-
if !canonical_model
557-
.modalities
558-
.input
559-
.contains(&crate::providers::canonical::Modality::Text)
560-
{
561-
return None;
562-
}
593+
// Check tool support
594+
if !cm.tool_call && !toolshim_enabled {
595+
return None;
596+
}
563597

564-
if !canonical_model.tool_call && !self.get_model_config().toolshim {
565-
return None;
598+
Some((model.clone(), cm.release_date.clone()))
599+
}
600+
None => {
601+
// Model not in canonical registry
602+
if uses_strict_model_filtering {
603+
// Built-in/preferred providers: skip unknown models
604+
None
605+
} else if allows_unknown_models {
606+
// Custom/declarative providers: include unknown models
607+
Some((model.clone(), None))
608+
} else {
609+
None
610+
}
611+
}
566612
}
567-
568-
let release_date = canonical_model.release_date.clone();
569-
570-
Some((model.clone(), release_date))
571613
})
572614
.collect();
573615

@@ -579,16 +621,10 @@ pub trait Provider: Send + Sync {
579621
(None, None) => a.0.cmp(&b.0),
580622
});
581623

582-
let recommended_models: Vec<String> = models_with_dates
624+
Ok(models_with_dates
583625
.into_iter()
584626
.map(|(name, _)| name)
585-
.collect();
586-
587-
if recommended_models.is_empty() {
588-
Ok(all_models)
589-
} else {
590-
Ok(recommended_models)
591-
}
627+
.collect())
592628
}
593629

594630
async fn map_to_canonical_model(
@@ -895,6 +931,45 @@ mod tests {
895931
}
896932
}
897933

934+
struct ListingProvider {
935+
name: String,
936+
provider_type: ProviderType,
937+
model_config: ModelConfig,
938+
supported_models: Vec<String>,
939+
}
940+
941+
#[async_trait::async_trait]
942+
impl Provider for ListingProvider {
943+
fn get_name(&self) -> &str {
944+
&self.name
945+
}
946+
947+
fn provider_type(&self) -> ProviderType {
948+
self.provider_type
949+
}
950+
951+
fn get_model_config(&self) -> ModelConfig {
952+
self.model_config.clone()
953+
}
954+
955+
async fn fetch_supported_models(&self) -> Result<Vec<String>, ProviderError> {
956+
Ok(self.supported_models.clone())
957+
}
958+
959+
async fn stream(
960+
&self,
961+
_model_config: &ModelConfig,
962+
_session_id: &str,
963+
_system: &str,
964+
_messages: &[Message],
965+
_tools: &[Tool],
966+
) -> Result<MessageStream, ProviderError> {
967+
Err(ProviderError::ExecutionError(
968+
"stream not implemented for listing tests".to_string(),
969+
))
970+
}
971+
}
972+
898973
fn create_test_stream(
899974
items: Vec<String>,
900975
) -> impl Stream<Item = Result<(Option<Message>, Option<ProviderUsage>), ProviderError>> {
@@ -1078,4 +1153,30 @@ mod tests {
10781153
assert_eq!(info.output_token_cost, Some(0.00001));
10791154
assert_eq!(info.currency, Some("$".to_string()));
10801155
}
1156+
1157+
#[tokio::test]
1158+
async fn test_fetch_recommended_models_includes_unknown_for_custom_provider() {
1159+
let provider = ListingProvider {
1160+
name: "custom-proxy".to_string(),
1161+
provider_type: ProviderType::Custom,
1162+
model_config: ModelConfig::new_or_fail("glm-5"),
1163+
supported_models: vec!["glm-5".to_string()],
1164+
};
1165+
1166+
let recommended = provider.fetch_recommended_models().await.unwrap();
1167+
assert!(recommended.contains(&"glm-5".to_string()));
1168+
}
1169+
1170+
#[tokio::test]
1171+
async fn test_fetch_recommended_models_excludes_unknown_for_builtin_provider() {
1172+
let provider = ListingProvider {
1173+
name: "openai".to_string(),
1174+
provider_type: ProviderType::Builtin,
1175+
model_config: ModelConfig::new_or_fail("gpt-4o"),
1176+
supported_models: vec!["definitely-unknown-model-id".to_string()],
1177+
};
1178+
1179+
let recommended = provider.fetch_recommended_models().await.unwrap();
1180+
assert!(recommended.is_empty());
1181+
}
10811182
}

crates/goose/src/providers/openai.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ pub struct OpenAiProvider {
6565
custom_headers: Option<HashMap<String, String>>,
6666
supports_streaming: bool,
6767
name: String,
68+
is_custom_host: bool,
6869
}
6970

7071
impl OpenAiProvider {
@@ -126,6 +127,7 @@ impl OpenAiProvider {
126127
custom_headers,
127128
supports_streaming: true,
128129
name: OPEN_AI_PROVIDER_NAME.to_string(),
130+
is_custom_host: host != "https://api.openai.com",
129131
})
130132
}
131133

@@ -140,6 +142,7 @@ impl OpenAiProvider {
140142
custom_headers: None,
141143
supports_streaming: true,
142144
name: OPEN_AI_PROVIDER_NAME.to_string(),
145+
is_custom_host: false,
143146
}
144147
}
145148

@@ -208,6 +211,7 @@ impl OpenAiProvider {
208211
custom_headers: config.headers,
209212
supports_streaming: config.supports_streaming.unwrap_or(true),
210213
name: config.name.clone(),
214+
is_custom_host: true,
211215
})
212216
}
213217

@@ -361,6 +365,14 @@ impl Provider for OpenAiProvider {
361365
&self.name
362366
}
363367

368+
fn provider_type(&self) -> crate::providers::base::ProviderType {
369+
if self.name == OPEN_AI_PROVIDER_NAME && !self.is_custom_host {
370+
crate::providers::base::ProviderType::Builtin
371+
} else {
372+
crate::providers::base::ProviderType::Custom
373+
}
374+
}
375+
364376
fn get_model_config(&self) -> ModelConfig {
365377
self.model.clone()
366378
}

0 commit comments

Comments
 (0)