Skip to content

Commit 0374d72

Browse files
idosavionclaude
andcommitted
fix(acp): don't fail session creation when model listing is unavailable
Proxy-based providers that only implement the chat completions POST endpoint (and not a GET /models endpoint) caused session creation to fail because build_model_state propagated the fetch error via `?`. Model listing is optional UI metadata for the model-switcher. It should not be a hard gate on session creation. This aligns with providers like Azure and xAI that already return empty model lists successfully. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ido Savion <ido@diversion.dev>
1 parent c8b70b4 commit 0374d72

File tree

1 file changed

+20
-19
lines changed

1 file changed

+20
-19
lines changed

crates/goose-acp/src/server.rs

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -276,20 +276,21 @@ async fn add_extensions(agent: &Agent, extensions: Vec<ExtensionConfig>) {
276276
}
277277
}
278278

279-
async fn build_model_state(
280-
provider: &dyn Provider,
281-
current_model: &str,
282-
) -> Result<SessionModelState, sacp::Error> {
283-
let models = provider.fetch_recommended_models().await.map_err(|e| {
284-
sacp::Error::internal_error().data(format!("Failed to fetch models: {}", e))
285-
})?;
286-
Ok(SessionModelState::new(
279+
async fn build_model_state(provider: &dyn Provider, current_model: &str) -> SessionModelState {
280+
let models = match provider.fetch_recommended_models().await {
281+
Ok(models) => models,
282+
Err(e) => {
283+
warn!(error = %e, "failed to fetch models, model selection will be unavailable");
284+
vec![]
285+
}
286+
};
287+
SessionModelState::new(
287288
ModelId::new(current_model),
288289
models
289290
.iter()
290291
.map(|name| ModelInfo::new(ModelId::new(&**name), &**name))
291292
.collect(),
292-
))
293+
)
293294
}
294295

295296
impl GooseAcpAgent {
@@ -728,7 +729,7 @@ impl GooseAcpAgent {
728729
);
729730

730731
let model_state =
731-
build_model_state(&*provider, &provider.get_model_config().model_name).await?;
732+
build_model_state(&*provider, &provider.get_model_config().model_name).await;
732733

733734
Ok(NewSessionResponse::new(SessionId::new(goose_session.id)).models(model_state))
734735
}
@@ -853,7 +854,7 @@ impl GooseAcpAgent {
853854
);
854855

855856
let model_state =
856-
build_model_state(&*provider, &provider.get_model_config().model_name).await?;
857+
build_model_state(&*provider, &provider.get_model_config().model_name).await;
857858

858859
Ok(LoadSessionResponse::new().models(model_state))
859860
}
@@ -1521,37 +1522,37 @@ print(\"hello, world\")
15211522

15221523
#[test_case(
15231524
"model-a", Ok(vec!["model-a".into(), "model-b".into()])
1524-
=> Ok(SessionModelState::new(
1525+
=> SessionModelState::new(
15251526
ModelId::new("model-a"),
15261527
vec![ModelInfo::new(ModelId::new("model-a"), "model-a"),
15271528
ModelInfo::new(ModelId::new("model-b"), "model-b")],
1528-
))
1529+
)
15291530
; "returns current and available models"
15301531
)]
15311532
#[test_case(
15321533
"model-a", Ok(vec![])
1533-
=> Ok(SessionModelState::new(ModelId::new("model-a"), vec![]))
1534+
=> SessionModelState::new(ModelId::new("model-a"), vec![])
15341535
; "empty model list"
15351536
)]
15361537
#[test_case(
15371538
"model-a", Err(ProviderError::ExecutionError("fail".into()))
1538-
=> matches Err(_)
1539-
; "fetch error propagates"
1539+
=> SessionModelState::new(ModelId::new("model-a"), vec![])
1540+
; "fetch error falls back to current model only"
15401541
)]
15411542
#[test_case(
15421543
"switched-model", Ok(vec!["model-a".into(), "switched-model".into()])
1543-
=> Ok(SessionModelState::new(
1544+
=> SessionModelState::new(
15441545
ModelId::new("switched-model"),
15451546
vec![ModelInfo::new(ModelId::new("model-a"), "model-a"),
15461547
ModelInfo::new(ModelId::new("switched-model"), "switched-model")],
1547-
))
1548+
)
15481549
; "current model reflects switched model"
15491550
)]
15501551
#[tokio::test]
15511552
async fn test_build_model_state(
15521553
current_model: &str,
15531554
models: Result<Vec<String>, ProviderError>,
1554-
) -> Result<SessionModelState, sacp::Error> {
1555+
) -> SessionModelState {
15551556
let provider = MockModelProvider { models };
15561557
build_model_state(&provider, current_model).await
15571558
}

0 commit comments

Comments
 (0)