fix: tidy up claude cli handling #5594
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -16,9 +16,8 @@ use crate::model::ModelConfig; | |||||||
| use rmcp::model::Tool; | ||||||||
|
|
||||||||
| pub const CLAUDE_CODE_DEFAULT_MODEL: &str = "claude-sonnet-4-20250514"; | ||||||||
| pub const CLAUDE_CODE_KNOWN_MODELS: &[&str] = &["sonnet", "opus"]; | ||||||||
| pub const CLAUDE_CODE_DOC_URL: &str = "https://code.claude.com/docs/en/setup"; | ||||||||
|
|
||||||||
| #[derive(Debug, serde::Serialize)] | ||||||||
| pub struct ClaudeCodeProvider { | ||||||||
|
|
@@ -169,6 +168,34 @@ impl ClaudeCodeProvider { | |||||||
| } | ||||||||
|
|
||||||||
| /// Parse the JSON response from claude CLI | ||||||||
|
There was a problem hiding this comment. The doc comment '/// Parse the JSON response from claude CLI' is incorrect for this function. It should describe applying permission flags to the command, not parsing JSON responses.
Suggested change
Collaborator
There was a problem hiding this comment. thanks, co-pilot! or delete the comment |
||||||||
| fn apply_permission_flags(cmd: &mut Command) -> Result<(), ProviderError> { | ||||||||
| let config = Config::global(); | ||||||||
| match config.get_goose_mode() { | ||||||||
| Ok(GooseMode::Auto) => { | ||||||||
| cmd.arg("--dangerously-skip-permissions"); | ||||||||
| } | ||||||||
| Ok(GooseMode::SmartApprove) => { | ||||||||
| cmd.arg("--permission-mode").arg("acceptEdits"); | ||||||||
| } | ||||||||
| Ok(GooseMode::Approve) => { | ||||||||
| return Err(ProviderError::RequestFailed( | ||||||||
| "\n\n\n### NOTE\n\n\n \ | ||||||||
| Claude Code CLI provider does not support Approve mode.\n \ | ||||||||
| Please use Auto (which will run anything it needs to) or \ | ||||||||
| SmartApprove (most things will run or Chat Mode)\n\n\n" | ||||||||
|
Comment on lines
+184
to
+185
There was a problem hiding this comment. The error message has unclear wording. 'SmartApprove (most things will run or Chat Mode)' is grammatically incorrect. It should be clearer, such as: 'Please use Auto (which will run anything it needs to), SmartApprove (most things will run), or Chat Mode.'
Suggested change
|
||||||||
| .to_string(), | ||||||||
| )); | ||||||||
| } | ||||||||
| Ok(GooseMode::Chat) => { | ||||||||
| // Chat mode doesn't need permission flags | ||||||||
| } | ||||||||
| Err(_) => { | ||||||||
| // Default behavior if mode is not set | ||||||||
| } | ||||||||
| } | ||||||||
| Ok(()) | ||||||||
| } | ||||||||
|
|
||||||||
| fn parse_claude_response( | ||||||||
| &self, | ||||||||
| json_lines: &[String], | ||||||||
|
|
@@ -314,19 +341,21 @@ impl ClaudeCodeProvider { | |||||||
| cmd.arg("--verbose").arg("--output-format").arg("json"); | ||||||||
|
|
||||||||
| // Add permission mode based on GOOSE_MODE setting | ||||||||
| Self::apply_permission_flags(&mut cmd)?; | ||||||||
|
|
||||||||
| cmd.stdout(Stdio::piped()).stderr(Stdio::piped()); | ||||||||
|
|
||||||||
| let mut child = cmd | ||||||||
| .spawn() | ||||||||
| .map_err(|e| ProviderError::RequestFailed(format!( | ||||||||
| "\n\n ## Unable to find Claude Code CLI on the path.\n\n\ | ||||||||
| **Error details:** Failed to spawn command '{}': {}\n\n\ | ||||||||
| **Please ensure:**\n\ | ||||||||
| - Claude Code CLI is installed and logged in\n\ | ||||||||
| - The command is in your PATH, or set `CLAUDE_CODE_COMMAND` in your config\n\n\ | ||||||||
| **For full features, please use the Anthropic provider with an API key if possible.**\n\n\ | ||||||||
| Visit {} for installation instructions.", | ||||||||
|
Collaborator
There was a problem hiding this comment. or change the path using @jamadeo 's new tricks |
||||||||
| self.command, e, CLAUDE_CODE_DOC_URL | ||||||||
| )))?; | ||||||||
|
|
||||||||
| let stdout = child | ||||||||
|
|
@@ -427,8 +456,8 @@ impl Provider for ClaudeCodeProvider { | |||||||
| fn metadata() -> ProviderMetadata { | ||||||||
| ProviderMetadata::new( | ||||||||
| "claude-code", | ||||||||
| "Claude Code CLI", | ||||||||
| "Requires claude CLI installed, no MCPs. Use Anthropic provider for full features.", | ||||||||
| CLAUDE_CODE_DEFAULT_MODEL, | ||||||||
| CLAUDE_CODE_KNOWN_MODELS.to_vec(), | ||||||||
| CLAUDE_CODE_DOC_URL, | ||||||||
|
|
||||||||
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing 'claude-sonnet-4-20250514' from CLAUDE_CODE_KNOWN_MODELS will cause the model argument to not be passed to the CLI when using the default model (since line 337 checks if the model is in KNOWN_MODELS). This breaks the default behavior where users expect the default model to be passed to the command.