fix(acp): don't fail session creation when model listing is unavailable (#7484)
Signed-off-by: Ido Savion <ido@diversion.dev> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -286,20 +286,21 @@ async fn add_extensions(agent: &Agent, extensions: Vec<ExtensionConfig>) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn build_model_state(
|
async fn build_model_state(provider: &dyn Provider, current_model: &str) -> SessionModelState {
|
||||||
provider: &dyn Provider,
|
let models = match provider.fetch_recommended_models().await {
|
||||||
current_model: &str,
|
Ok(models) => models,
|
||||||
) -> Result<SessionModelState, sacp::Error> {
|
Err(e) => {
|
||||||
let models = provider.fetch_recommended_models().await.map_err(|e| {
|
warn!(error = %e, "failed to fetch models, model selection will be unavailable");
|
||||||
sacp::Error::internal_error().data(format!("Failed to fetch models: {}", e))
|
vec![]
|
||||||
})?;
|
}
|
||||||
Ok(SessionModelState::new(
|
};
|
||||||
|
SessionModelState::new(
|
||||||
ModelId::new(current_model),
|
ModelId::new(current_model),
|
||||||
models
|
models
|
||||||
.iter()
|
.iter()
|
||||||
.map(|name| ModelInfo::new(ModelId::new(&**name), &**name))
|
.map(|name| ModelInfo::new(ModelId::new(&**name), &**name))
|
||||||
.collect(),
|
.collect(),
|
||||||
))
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
impl GooseAcpAgent {
|
impl GooseAcpAgent {
|
||||||
@@ -738,7 +739,7 @@ impl GooseAcpAgent {
|
|||||||
);
|
);
|
||||||
|
|
||||||
let model_state =
|
let model_state =
|
||||||
build_model_state(&*provider, &provider.get_model_config().model_name).await?;
|
build_model_state(&*provider, &provider.get_model_config().model_name).await;
|
||||||
|
|
||||||
Ok(NewSessionResponse::new(SessionId::new(goose_session.id)).models(model_state))
|
Ok(NewSessionResponse::new(SessionId::new(goose_session.id)).models(model_state))
|
||||||
}
|
}
|
||||||
@@ -863,7 +864,7 @@ impl GooseAcpAgent {
|
|||||||
);
|
);
|
||||||
|
|
||||||
let model_state =
|
let model_state =
|
||||||
build_model_state(&*provider, &provider.get_model_config().model_name).await?;
|
build_model_state(&*provider, &provider.get_model_config().model_name).await;
|
||||||
|
|
||||||
Ok(LoadSessionResponse::new().models(model_state))
|
Ok(LoadSessionResponse::new().models(model_state))
|
||||||
}
|
}
|
||||||
@@ -1528,37 +1529,37 @@ print(\"hello, world\")
|
|||||||
|
|
||||||
#[test_case(
|
#[test_case(
|
||||||
"model-a", Ok(vec!["model-a".into(), "model-b".into()])
|
"model-a", Ok(vec!["model-a".into(), "model-b".into()])
|
||||||
=> Ok(SessionModelState::new(
|
=> SessionModelState::new(
|
||||||
ModelId::new("model-a"),
|
ModelId::new("model-a"),
|
||||||
vec![ModelInfo::new(ModelId::new("model-a"), "model-a"),
|
vec![ModelInfo::new(ModelId::new("model-a"), "model-a"),
|
||||||
ModelInfo::new(ModelId::new("model-b"), "model-b")],
|
ModelInfo::new(ModelId::new("model-b"), "model-b")],
|
||||||
))
|
)
|
||||||
; "returns current and available models"
|
; "returns current and available models"
|
||||||
)]
|
)]
|
||||||
#[test_case(
|
#[test_case(
|
||||||
"model-a", Ok(vec![])
|
"model-a", Ok(vec![])
|
||||||
=> Ok(SessionModelState::new(ModelId::new("model-a"), vec![]))
|
=> SessionModelState::new(ModelId::new("model-a"), vec![])
|
||||||
; "empty model list"
|
; "empty model list"
|
||||||
)]
|
)]
|
||||||
#[test_case(
|
#[test_case(
|
||||||
"model-a", Err(ProviderError::ExecutionError("fail".into()))
|
"model-a", Err(ProviderError::ExecutionError("fail".into()))
|
||||||
=> matches Err(_)
|
=> SessionModelState::new(ModelId::new("model-a"), vec![])
|
||||||
; "fetch error propagates"
|
; "fetch error falls back to current model only"
|
||||||
)]
|
)]
|
||||||
#[test_case(
|
#[test_case(
|
||||||
"switched-model", Ok(vec!["model-a".into(), "switched-model".into()])
|
"switched-model", Ok(vec!["model-a".into(), "switched-model".into()])
|
||||||
=> Ok(SessionModelState::new(
|
=> SessionModelState::new(
|
||||||
ModelId::new("switched-model"),
|
ModelId::new("switched-model"),
|
||||||
vec![ModelInfo::new(ModelId::new("model-a"), "model-a"),
|
vec![ModelInfo::new(ModelId::new("model-a"), "model-a"),
|
||||||
ModelInfo::new(ModelId::new("switched-model"), "switched-model")],
|
ModelInfo::new(ModelId::new("switched-model"), "switched-model")],
|
||||||
))
|
)
|
||||||
; "current model reflects switched model"
|
; "current model reflects switched model"
|
||||||
)]
|
)]
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn test_build_model_state(
|
async fn test_build_model_state(
|
||||||
current_model: &str,
|
current_model: &str,
|
||||||
models: Result<Vec<String>, ProviderError>,
|
models: Result<Vec<String>, ProviderError>,
|
||||||
) -> Result<SessionModelState, sacp::Error> {
|
) -> SessionModelState {
|
||||||
let provider = MockModelProvider { models };
|
let provider = MockModelProvider { models };
|
||||||
build_model_state(&provider, current_model).await
|
build_model_state(&provider, current_model).await
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user