feat(claude-code): dynamic model listing and mid-session model switching#7120
feat(claude-code): dynamic model listing and mid-session model switching#7120codefromthecrypt merged 1 commit intomainfrom
Conversation
Before, the claude provider hard-coded 'claude-sonnet-4-20250514' instead of using "default", didn't support model switching mid-session, and didn't support listing available models dynamically. This uses the stream-json control protocol to fetch models via initialize and switch models via set_model. Also removes buggy ContextLengthExceeded handling as claude-code responses are unpredictable for oversized context. Signed-off-by: Adrian Cole <adrian@tetrate.io>
There was a problem hiding this comment.
Pull request overview
This PR updates the claude-code provider to rely on Claude CLI model aliases dynamically (instead of a hard-coded dated model), and adds support for switching models during an active provider lifecycle, with accompanying provider integration test updates.
Changes:
- Switch
claude-codedefault model to"default"and fetch supported model aliases dynamically via a CLIinitializecontrol request. - Add mid-run model switching via
set_modelcontrol requests on the persistent CLI subprocess. - Update provider integration tests to cover model switching and adjust context-length testing behavior for
claude-code.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| crates/goose/src/providers/claude_code.rs | Implements dynamic model listing and adds set_model control requests to support model switching on the persistent CLI process. |
| crates/goose/tests/providers.rs | Extends the provider test harness to optionally test model switching and adjusts context-length testing for claude-code. |
| } | ||
|
|
||
| let _ = child.start_kill(); | ||
| Ok(parse_models_from_lines(&lines)) |
There was a problem hiding this comment.
child.start_kill() sends a kill signal but doesn't await wait() to reap the process; in long-running contexts repeated model listings can leave zombies—prefer child.kill().await followed by child.wait().await (or wait_with_output).
| .await?; | ||
|
|
||
| assert!( | ||
| matches!(response.content[0], MessageContent::Text(_)), |
There was a problem hiding this comment.
This test can panic if a provider returns an empty content array; use a non-panicking check (e.g., response.content.first()) before matching on the item.
| matches!(response.content[0], MessageContent::Text(_)), | |
| matches!(response.content.first(), Some(MessageContent::Text(_))), |
| "request_id": request_id, | ||
| "request": {"subtype": "set_model", "model": model} | ||
| }); | ||
| let mut req_str = serde_json::to_string(&req).unwrap(); |
There was a problem hiding this comment.
serde_json::to_string(&req).unwrap() can panic in production; return a ProviderError::RequestFailed (or similar) with the serialization error instead of unwrapping.
| let mut req_str = serde_json::to_string(&req).unwrap(); | |
| let mut req_str = serde_json::to_string(&req).map_err(|e| { | |
| ProviderError::RequestFailed(format!("Failed to serialize set_model request: {e}")) | |
| })?; |
| // Read lines until we get the control_response for our request. | ||
| let mut line = String::new(); | ||
| loop { | ||
| line.clear(); | ||
| match self.reader.read_line(&mut line).await { | ||
| Ok(0) => { | ||
| return Err(ProviderError::RequestFailed( | ||
| "CLI process terminated while waiting for set_model response".to_string(), | ||
| )); | ||
| } | ||
| Ok(_) => { | ||
| let trimmed = line.trim(); | ||
| if trimmed.is_empty() { | ||
| continue; | ||
| } | ||
| if let Ok(parsed) = serde_json::from_str::<Value>(trimmed) { | ||
| if parsed.get("type").and_then(|t| t.as_str()) == Some("control_response") { | ||
| let success = | ||
| parsed.pointer("/response/subtype").and_then(|s| s.as_str()) | ||
| == Some("success"); | ||
| if success { | ||
| self.current_model = model.to_string(); | ||
| self.log_model_update = true; | ||
| return Ok(()); | ||
| } else { | ||
| let err = parsed | ||
| .pointer("/response/error") | ||
| .and_then(|e| e.as_str()) | ||
| .unwrap_or("unknown"); | ||
| return Err(ProviderError::RequestFailed(format!( | ||
| "set_model failed: {err}" | ||
| ))); | ||
| } | ||
| } |
There was a problem hiding this comment.
send_set_model treats the first control_response it sees as the response to the set_model request, but it never verifies request_id matches the one it sent, so an unrelated control_response could incorrectly update current_model and desynchronize the stream; parse and match request_id (and ideally ignore/queue other messages).
| "request_id": "model_list", | ||
| "request": {"subtype": "initialize"} | ||
| }); | ||
| let mut request_str = serde_json::to_string(&request).unwrap(); |
There was a problem hiding this comment.
In fetch_supported_models, serde_json::to_string(&request).unwrap() can panic; convert this to a fallible serialization with proper ProviderError propagation.
| let mut request_str = serde_json::to_string(&request).unwrap(); | |
| let mut request_str = serde_json::to_string(&request).map_err(|e| { | |
| ProviderError::RequestFailed(format!("Failed to serialize initialize request: {e}")) | |
| })?; |
|
finally, a few decent review comments, @copilot! |
|
@codefromthecrypt I've opened a new pull request, #7129, to work on those changes. Once the pull request is ready, I'll request review from you. |
* origin/main: (107 commits) feat: Allow overriding default bat themes using environment variables (#7140) Make the system prompt smaller (#6991) Pre release script (#7145) Spelling (#7137) feat(mcp): upgrade rmcp to 0.15.0 and advertise MCP Apps UI extension capability (#6927) fix: ensure assistant messages with tool_calls include content field (#7076) fix(canonical): handle gcp_vertex_ai model mapping correctly (#6836) Group dependencies in root Cargo.toml (#6948) refactor: updated elevenLabs API module and `remove button` UX (#6781) fix: we were missing content from langfuse traces (#7135) docs: update username in authors.yml (#7132) fix extension selector syncing issues (#7133) fix(acp): per-session Agent for model isolation and load_session restore (#7115) fix(claude-code): defensive coding improvements for model switching (#7131) feat(claude-code): dynamic model listing and mid-session model switching (#7120) Inline worklet source (#7128) [docs] One shot prompting is dead - Blog Post (#7113) fix: correct spelling of Debbie O'Brien's name in authors.yml (#7127) docs: GCP Vertex AI org policy filtering & update OnboardingProviderSetup component (#7125) feat: replace subagent and skills with unified summon extension (#6964) ... # Conflicts: # Cargo.lock # Cargo.toml
* upstream/main: (109 commits) [docs] Skills Marketplace UI Improvements (block#7158) More no-window flags (block#7122) feat: Allow overriding default bat themes using environment variables (block#7140) Make the system prompt smaller (block#6991) Pre release script (block#7145) Spelling (block#7137) feat(mcp): upgrade rmcp to 0.15.0 and advertise MCP Apps UI extension capability (block#6927) fix: ensure assistant messages with tool_calls include content field (block#7076) fix(canonical): handle gcp_vertex_ai model mapping correctly (block#6836) Group dependencies in root Cargo.toml (block#6948) refactor: updated elevenLabs API module and `remove button` UX (block#6781) fix: we were missing content from langfuse traces (block#7135) docs: update username in authors.yml (block#7132) fix extension selector syncing issues (block#7133) fix(acp): per-session Agent for model isolation and load_session restore (block#7115) fix(claude-code): defensive coding improvements for model switching (block#7131) feat(claude-code): dynamic model listing and mid-session model switching (block#7120) Inline worklet source (block#7128) [docs] One shot prompting is dead - Blog Post (block#7113) fix: correct spelling of Debbie O'Brien's name in authors.yml (block#7127) ...
…ing (block#7120) Signed-off-by: Adrian Cole <adrian@tetrate.io>
…ing (block#7120) Signed-off-by: Adrian Cole <adrian@tetrate.io>
…ing (block#7120) Signed-off-by: Adrian Cole <adrian@tetrate.io>
Summary
Before, the claude code provider had a few issues:
claude /modelsuse.This changes the provider to get the dynamic list of available model choices from claude, doing no filtering as they are already filtered.
This also implements model switching and adds a helpful log message and test to validate it works.
This also reverse documents that unlisted models are supported, even if there is no list of names.
Finally, this removed buggy ContextLengthExceeded handling as it is not predictable in claude.
Type of Change
AI Assistance
Testing
New
test_model_switchwhich tests switching from default to sonnet;Here's a run that also shows model list works, via debug logging:
Related Issues