fix: remove Option from model listing return types, propagate errors

[codefromthecrypt]

Summary

Remove Option from fetch_supported_models and fetch_recommended_models return types. An empty vec already means "no models" — the Option wrapper just forced every caller to handle None differently.

Additionally, dynamic providers now propagate errors instead of swallowing them into Ok(None). This ensures misconfigured providers (e.g. bad credentials, unreachable host) surface errors to the desktop and CLI rather than silently returning no models.

Type of Change

• Refactor / Code quality

AI Assistance

• This PR was created or reviewed with AI assistance

Testing

providers test with ollama:

Details

$ OLLAMA_HOST=http://localhost:11434 cargo test -p goose --test providers -- test_ollama_provider --nocapture                                                
Compiling tonic v0.12.3
Compiling goose-mcp v1.23.0 (/Users/codefromthecrypt/oss/goose-goosed/crates/goose-mcp)
Compiling opentelemetry-proto v0.27.0
Compiling opentelemetry-otlp v0.27.0
Compiling goose v1.23.0 (/Users/codefromthecrypt/oss/goose-goosed/crates/goose)
Finished `test` profile [unoptimized + debuginfo] target(s) in 37.60s
Running tests/providers.rs (target/debug/deps/providers-28809165ae79d804)

running 1 test
=== Ollama::model_listing ===
[crates/goose/tests/providers.rs:374:9] &models = [
"all-minilm:33m",
"all-minilm:l6-v2",
"devstral:latest",
"gemma3:latest",
"llama3.2:latest",
"nomic-embed-text:latest",
"qwen2.5-coder:32b",
"qwen2.5:0.5b",
"qwen2.5:latest",
"qwen3-vl:2b",
"qwen3-vl:latest",
"qwen3:0.6b",
"qwen3:1.7b",
"qwen3:4b",
"qwen3:latest",
]
===================
=== Ollama::reponse1 ===
[crates/goose/tests/providers.rs:154:9] &response1 = Message {
id: None,
role: Assistant,
created: 1770519075,
content: [
Text(
Annotated {
raw: RawTextContent {
text: "",
meta: None,
},
annotations: None,
},
),
ToolRequest(
ToolRequest {
id: "call_p694a081",
tool_call: Ok(
CallToolRequestParams {
meta: None,
name: "get_weather",
arguments: Some(
{
"location": String("San Francisco, CA"),
},
),
task: None,
},
),
metadata: None,
tool_meta: None,
},
),
],
metadata: MessageMetadata {
user_visible: true,
agent_visible: true,
},
}
===================
=== Ollama::reponse2 ===
[crates/goose/tests/providers.rs:203:9] &response2 = Message {
id: None,
role: Assistant,
created: 1770519079,
content: [
Text(
Annotated {
raw: RawTextContent {
text: "The weather in San Francisco is currently **Clear** with a temperature of **50°F (10°C)**. Conditions show **0% precipitation**, **84% humidity**, and **2 mph wind**. This forecast is valid for Saturday at 9:00 PM. ☀\u{fe0f}",
meta: None,
},
annotations: None,
},
),
],
metadata: MessageMetadata {
user_visible: true,
agent_visible: true,
},
}
===================
=== Ollama::context_length_exceeded_error ===
[crates/goose/tests/providers.rs:247:9] &result = Ok(
(
Message {
id: None,
role: Assistant,
created: 1770519089,
content: [
Text(
Annotated {
raw: RawTextContent {
text: "no",
meta: None,
},
annotations: None,
},
),
],
metadata: MessageMetadata {
user_visible: true,
agent_visible: true,
},
},
ProviderUsage {
model: "qwen3",
usage: Usage {
input_tokens: Some(
79,
),
output_tokens: Some(
691,
),
total_tokens: Some(
770,
),
},
},
),
)
===================
Test image not found at crates/goose/examples/test_assets/test_image.png, skipping image test
test test_ollama_provider ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 13 filtered out; finished in 19.35s


============== Providers ==============
✅ Ollama
=======================================

Related Issues

Obviates #7065

[Copilot]

Pull request overview

This PR simplifies provider model-listing APIs by removing Option wrappers (empty Vec now consistently means “no models”) and changes multiple dynamic providers to propagate model-listing errors instead of silently returning no results, so misconfiguration is surfaced to the CLI/desktop/server.

Changes:

• Update Provider trait model-listing methods to return Result<Vec<String>, ProviderError> and adjust call sites accordingly.
• Update multiple providers (e.g., OpenRouter, Tetrate, Databricks, LiteLLM) to return errors for request/parse/API failures instead of Ok(None).
• Update server/CLI/tests and supporting utilities to use the new semantics (empty list vs error).

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
crates/goose/src/providers/base.rs	Changes trait defaults and recommended-model filtering to use Vec<String> (empty means none).
crates/goose/src/providers/openrouter.rs	Propagates request/parse/API errors; returns Vec<String> directly.
crates/goose/src/providers/tetrate.rs	Propagates JSON/shape/API errors; removes “warn + None” fallback.
crates/goose/src/providers/databricks.rs	Propagates fetch/parse/status errors; returns Vec<String>.
crates/goose/src/providers/litellm.rs	Propagates fetch errors instead of warning + None.
crates/goose/src/providers/openai_compatible.rs	Returns Vec<String> and errors on unexpected response shape.
crates/goose/src/providers/openai.rs	Returns Vec<String> (no Option).
crates/goose/src/providers/anthropic.rs	Returns Vec<String> and errors on missing data array.
crates/goose/src/providers/google.rs	Returns Vec<String> and errors on missing models array.
crates/goose/src/providers/githubcopilot.rs	Returns Vec<String> and errors on missing data array.
crates/goose/src/providers/ollama.rs	Returns Vec<String> (no Option).
crates/goose/src/providers/venice.rs	Returns Vec<String> (no Option).
crates/goose/src/providers/gcpvertexai.rs	Returns filtered Vec<String> (no Option).
crates/goose/src/providers/lead_worker.rs	Combines lead/worker model lists under new non-Option contract.
crates/goose/src/providers/snowflake.rs	Adds fetch_supported_models returning known models.
crates/goose/src/providers/bedrock.rs	Adds fetch_supported_models returning known models.
crates/goose/src/providers/codex.rs	Returns known models as Vec<String> (no Option).
crates/goose/src/providers/chatgpt_codex.rs	Returns known models as Vec<String> (no Option).
crates/goose/src/providers/claude_code.rs	Adds fetch_supported_models returning known models.
crates/goose/src/providers/gemini_cli.rs	Adds fetch_supported_models returning known models.
crates/goose/src/providers/cursor_agent.rs	Adds fetch_supported_models returning known models.
crates/goose/src/providers/auto_detect.rs	Updates detection logic to treat empty list as “no match”.
crates/goose/src/providers/canonical/build_canonical_models.rs	Adapts canonical model build/check tool to new return type.
crates/goose/src/agents/reply_parts.rs	Adapts enhanced error messaging to new model-listing semantics.
crates/goose-server/src/routes/config_management.rs	Removes declarative-provider static model short-circuit; returns models directly on Ok(Vec).
crates/goose-cli/src/commands/configure.rs	Uses “non-empty list => select” else free-text under new API.
crates/goose/tests/providers.rs	Updates model listing test to assume Vec<String> return.

Comments suppressed due to low confidence (1)

crates/goose-server/src/routes/config_management.rs:387

• Removing the declarative-provider fast path changes this endpoint from returning the static config.models list to returning a 400 when the provider isn’t configured, which can break UI flows that rely on model names being available before credentials/network access; consider restoring a fallback for ProviderType::Declarative/Custom when load_provider succeeds (at least for requires_auth == false or when a static models list is present).

pub async fn get_provider_models(
    Path(name): Path<String>,
) -> Result<Json<Vec<String>>, ErrorResponse> {
    let all = get_providers().await.into_iter().collect::<Vec<_>>();
    let Some((metadata, provider_type)) = all.into_iter().find(|(m, _)| m.name == name) else {
        return Err(ErrorResponse::bad_request(format!(
            "Unknown provider: {}",
            name
        )));
    };
    if !check_provider_configured(&metadata, provider_type) {
        return Err(ErrorResponse::bad_request(format!(
            "Provider '{}' is not configured",
            name
        )));
    }

[michaelneale]

this makes a lot of sense to me - I don't know exactly what flow on effects are and not hand tested, but this has always frustrated me - just need to put one in most of the time.