fix(claude-code): defensive coding improvements for model switching#7131
fix(claude-code): defensive coding improvements for model switching#7131codefromthecrypt merged 1 commit intomainfrom
Conversation
Replace unwrap() with proper error propagation on serde serialization, match request_id in control_response handling, reap child process with kill().await, and use safe indexing in tests. Signed-off-by: Adrian Cole <adrian@tetrate.io>
| } | ||
| if let Ok(parsed) = serde_json::from_str::<Value>(trimmed) { | ||
| if parsed.get("type").and_then(|t| t.as_str()) == Some("control_response") { | ||
| // Skip responses that don't match our request_id |
There was a problem hiding this comment.
seems non slop comment by why do we need to skip? just curious
There was a problem hiding this comment.
yeah this is responding to copilot comment, lemme dig deeper it might have been slop
| } | ||
|
|
||
| let _ = child.start_kill(); | ||
| let _ = child.kill().await; |
There was a problem hiding this comment.
makes sense, I wonder if we should name this to claude_code_cli just in case oauth comes back one day? (as would be different code?)
There was a problem hiding this comment.
Pull request overview
This PR applies defensive coding improvements to the claude-code provider following the model switching feature added in #7120. The changes focus on replacing panic-prone operations with proper error handling and adding request ID validation.
Changes:
- Replaced direct array indexing with
.first()to prevent panics on empty vectors - Changed
unwrap()to proper error handling for JSON serialization operations - Added request ID filtering to skip mismatched control responses
- Changed synchronous
start_kill()to asynckill().awaitin appropriate context
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| crates/goose/tests/providers.rs | Removed redundant comments and changed array indexing to .first() for defensive null checking |
| crates/goose/src/providers/claude_code.rs | Added proper error handling for JSON serialization, request ID validation, and improved process cleanup |
* 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) ...
…lock#7131) Signed-off-by: Adrian Cole <adrian@tetrate.io>
…lock#7131) Signed-off-by: Adrian Cole <adrian@tetrate.io>
…lock#7131) Signed-off-by: Adrian Cole <adrian@tetrate.io>
Summary
A series of polishing mentioned in #7120 for Claude Code model list support.
Type of Change
AI Assistance
Testing
re-ran the integration test
Related Issues
Relates to #7120