fix(acp): don't fail session creation when model listing is unavailable#7484
Merged
katzdave merged 1 commit intoblock:mainfrom Feb 26, 2026
Merged
Conversation
Contributor
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
idosavion
force-pushed
the
idosavion/goose-model-validation
branch
from
f09689a to
0374d72
Compare
katzdave
approved these changes
Feb 24, 2026
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 <[email protected]> Signed-off-by: Ido Savion <[email protected]>
idosavion
force-pushed
the
idosavion/goose-model-validation
branch
from
0374d72 to
91f354e
Compare
Contributor
Author
|
For sanity, built the custom binary and verified the log is emitted as expected and session is starting: |
Contributor
Author
|
@katzdave - thanks for the quick review! |
Contributor
Nope sorry let's merge! |
This was referenced Feb 26, 2026
tlongwell-block
added a commit
that referenced
this pull request
Feb 27, 2026
…patible * origin/main: (70 commits) feat: allow goose askai bot to search goose codebase (#7508) Revert "Reapply "fix: prevent crashes in long-running Electron sessions"" Reapply "fix: prevent crashes in long-running Electron sessions" Revert "fix: prevent crashes in long-running Electron sessions" fix: replace unwrap() with graceful error in scheduler execute_job (#7436) fix: Dictation API error message shows incorrect limit (#7423) fix(acp): Use ACP schema types for session/list (#7409) fix(desktop): make bundle and updater asset naming configurable (#7337) fix(openai): preserve order in Responses API history (#7500) Use the correct Goose emoji 🪿 instead of Swan in README.md (#7485) feat(ui): implement fullscreen and pip display modes for MCP Apps (#7312) fix: prevent crashes in long-running Electron sessions Disable tool pair summarization (#7481) fix: New Recipe Warning does not close on cancel (#7524) The client is not the source of truth (#7438) feat: support Anthropic adaptive thinking (#7356) copilot instructions: reword no prerelease docs (#7101) fix(acp): don't fail session creation when model listing is unavailable (#7484) feat: simplify developer extension (#7466) feat: add goose-powered release notes generator workflow (#7503) ... # Conflicts: # Cargo.lock
craigwalkeruk
pushed a commit
to craigwalkeruk/custom-goose
that referenced
this pull request
Mar 5, 2026
…le (block#7484) Signed-off-by: Ido Savion <[email protected]> Co-authored-by: Claude Opus 4.6 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.