feat(acp): add model selection support for session/new and session/set_model#7112
feat(acp): add model selection support for session/new and session/set_model#7112codefromthecrypt merged 1 commit intomainfrom
Conversation
…t_model Return available models in session/new and session/load responses so ACP clients like Zed can show a model picker. Handle session/set_model to switch models mid-session. sacp v10.1.0 lacks typed support for session/set_model, so we route via .otherwise() with UntypedMessage and deserialize into schema types. Signed-off-by: Adrian Cole <adrian@tetrate.io>
fa7899c to
6ad1ab4
Compare
There was a problem hiding this comment.
Pull request overview
Adds ACP model selection support to goose acp by surfacing provider model lists in session/new / session/load responses and handling session/set_model to switch models mid-session (with integration tests and fixtures to validate behavior).
Changes:
- Include
SessionModelState(models) insession/newandsession/loadresponses using provider model discovery. - Add (temporary) untyped JSON handling for
session/set_modelto switch the provider/model during a session. - Add integration tests + OpenAI
/v1/modelsfixture data to verify model listing and model switching behavior.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/goose/src/providers/openai.rs | Adds gpt-5-nano to the known model/context-limit list. |
| crates/goose/src/providers/errors.rs | Makes ProviderError clonable (used by new tests/mocks). |
| crates/goose-test-support/src/session.rs | Introduces TEST_MODEL constant for tests/fixtures. |
| crates/goose-test-support/src/lib.rs | Re-exports TEST_MODEL. |
| crates/goose-acp/tests/test_data/openai_models.json | Adds fixture response for OpenAI model listing. |
| crates/goose-acp/tests/server_test.rs | Registers new integration tests for model list and set_model. |
| crates/goose-acp/tests/fixtures/server.rs | Captures NewSessionResponse.models and adds an untyped session/set_model sender for tests. |
| crates/goose-acp/tests/fixtures/mod.rs | Mocks /v1/models and wires config to use TEST_MODEL; extends Session test trait. |
| crates/goose-acp/tests/common_tests/mod.rs | Adds run_model_list + run_set_model common test helpers. |
| crates/goose-acp/src/server.rs | Implements model state building, attaches models to responses, and adds session/set_model handling. |
| crates/goose-acp/Cargo.toml | Enables schema feature (unstable_session_model) via direct dependency. |
| Cargo.lock | Records the new dependency for goose-acp. |
| model_id: &str, | ||
| ) -> Result<SetSessionModelResponse, sacp::Error> { | ||
| let model_config = goose::model::ModelConfig::new(model_id).map_err(|e| { | ||
| sacp::Error::internal_error().data(format!("Invalid model config: {}", e)) |
There was a problem hiding this comment.
ModelConfig::new(model_id) failing is a client input error, but this is currently returned as internal_error; returning invalid_params (and similarly mapping “unknown session” to invalid_params) will give ACP clients correct feedback when they select an unsupported model or bad session id.
| sacp::Error::internal_error().data(format!("Invalid model config: {}", e)) | |
| sacp::Error::invalid_params().data(format!("Invalid model config: {}", e)) |
|
Aside: Copilot reviews are getting worse lately only 2/5 comments valid and another pr 2/4 |
michaelneale
left a comment
There was a problem hiding this comment.
yeah copilot was so believable but yeah, nice one.
…tensions-deeplinks * 'main' of github.com:block/goose: [docs] update authors.yaml file (#7114) Implement manpage generation for goose-cli (#6980) docs: tool output optimization (#7109) Fix duplicated output in Code Mode by filtering content by audience (#7117) Enable tom (Top Of Mind) platform extension by default (#7111) chore: added notification for canary build failure (#7106) fix: fix windows bundle random failure and optimise canary build (#7105) feat(acp): add model selection support for session/new and session/set_model (#7112) fix: isolate claude-code sessions via stream-json session_id (#7108) ci: enable agentic provider live tests (claude-code, codex, gemini-cli) (#7088) docs: codex subscription support (#7104) chore: add a new scenario (#7107) fix: Goose Desktop missing Calendar and Reminders entitlements (#7100) Fix 'Edit In Place' and 'Fork Session' features (#6970) Fix: Only send command content to command injection classifier (excluding part of tool call dict) (#7082) Docs: require auth optional for custom providers (#7098) fix: improve text-muted contrast for better readability (#7095) Always sync bundled extensions (#7057)
* origin/main: feat: add AGENT=goose environment variable for cross-tool compatibility (#7017) fix: strip empty extensions array when deeplink also (#7096) [docs] update authors.yaml file (#7114) Implement manpage generation for goose-cli (#6980) docs: tool output optimization (#7109) Fix duplicated output in Code Mode by filtering content by audience (#7117) Enable tom (Top Of Mind) platform extension by default (#7111) chore: added notification for canary build failure (#7106) fix: fix windows bundle random failure and optimise canary build (#7105) feat(acp): add model selection support for session/new and session/set_model (#7112) fix: isolate claude-code sessions via stream-json session_id (#7108) ci: enable agentic provider live tests (claude-code, codex, gemini-cli) (#7088) docs: codex subscription support (#7104) chore: add a new scenario (#7107) fix: Goose Desktop missing Calendar and Reminders entitlements (#7100) Fix 'Edit In Place' and 'Fork Session' features (#6970) Fix: Only send command content to command injection classifier (excluding part of tool call dict) (#7082) # Conflicts: # crates/goose/src/agents/extension.rs
* origin/main: (30 commits) docs: GCP Vertex AI org policy filtering & update OnboardingProviderSetup component (#7125) feat: replace subagent and skills with unified summon extension (#6964) feat: add AGENT=goose environment variable for cross-tool compatibility (#7017) fix: strip empty extensions array when deeplink also (#7096) [docs] update authors.yaml file (#7114) Implement manpage generation for goose-cli (#6980) docs: tool output optimization (#7109) Fix duplicated output in Code Mode by filtering content by audience (#7117) Enable tom (Top Of Mind) platform extension by default (#7111) chore: added notification for canary build failure (#7106) fix: fix windows bundle random failure and optimise canary build (#7105) feat(acp): add model selection support for session/new and session/set_model (#7112) fix: isolate claude-code sessions via stream-json session_id (#7108) ci: enable agentic provider live tests (claude-code, codex, gemini-cli) (#7088) docs: codex subscription support (#7104) chore: add a new scenario (#7107) fix: Goose Desktop missing Calendar and Reminders entitlements (#7100) Fix 'Edit In Place' and 'Fork Session' features (#6970) Fix: Only send command content to command injection classifier (excluding part of tool call dict) (#7082) Docs: require auth optional for custom providers (#7098) ...
…t_model (block#7112) Signed-off-by: Adrian Cole <adrian@tetrate.io>
…t_model (block#7112) Signed-off-by: Adrian Cole <adrian@tetrate.io>
…t_model (block#7112) Signed-off-by: Adrian Cole <adrian@tetrate.io>
Summary
Add model selection support to the ACP server (
goose acp). ACP clients like Zed show a model picker when the agent returnsmodelsin thesession/newresponse, and switch models mid-session viasession/set_model. This adds both so clients can see and switch between the configured provider's available models.Clients that don't support model selection continue to work unchanged -- the
modelsfield is optional andsession/set_modelis never sent by those clients.The sacp crate is at the latest version, v10.1.0, but is out-of-date and even main lacks model support. HACK comments apply until sorted.
Type of Change
AI Assistance
Testing
Manual: Zed 0.222.4 + goose (this branch):
cargo build --release -p goose-cliIntegration tests (
cargo test -p goose-acp --test server_test -- --nocapture):test_model_list-- verifiessession/newresponse includesSessionModelStatewith current model and full available model listtest_set_model-- switches too4-mini, verifies the model change reaches the OpenAI API via exchange pattern matchingRelated Issues
fetch_recommended_modelson the provider trait; this PR uses it to populate the ACP responseScreenshots/Demos