fix(acp): don't fail session creation when model listing is unavailable#7484
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where ACP session creation fails silently when using proxy-based providers that don't implement the OpenAI-compatible /models endpoint. The fix changes build_model_state to gracefully degrade by logging a warning and returning an empty model list instead of propagating the error, allowing sessions to start normally with model selection unavailable.
Changes:
- Modified
build_model_stateto handlefetch_recommended_modelserrors gracefully instead of propagating them - Updated function signature to return
SessionModelStatedirectly instead ofResult<SessionModelState, sacp::Error> - Updated test case from "fetch error propagates" to "fetch error falls back to current model only" with correct assertions
f09689a to
0374d72
Compare
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>
0374d72 to
91f354e
Compare
|
For sanity, built the custom binary and verified the log is emitted as expected and session is starting: |
|
@katzdave - thanks for the quick review! |
Nope sorry let's merge! |
Summary
Fixes #7427
build_model_statetreats model listing as a hard gate on session creation — if the GET to the models endpoint fails,on_new_sessionreturns an error and the session never starts. This breaks proxy-based and deployment-specific providers that only implement the chat completions endpoint (no/modelsroute).Now
build_model_statecatches the error, logs a warning, and returns an empty model list. The session starts normally; model selection in the UI just won't be available. This matches providers like Azure and xAI that already return empty model lists through the defaultfetch_supported_modelsimpl.Type of Change
AI Assistance
Testing
test_build_model_state::fetch_error_falls_back_to_current_model_only— previously assertedErr(_), now asserts the function returns the current model with an empty available-models listcargo fmt,cargo clippy -D warningscleanRelated Issues
Follow-up to #7112 (model selection support). That PR added
build_model_statebut made it fatal, so any provider without a models endpoint can't start sessions.