-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: add config parameter to new_task tool (#6839) #6840
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add optional config parameter to new_task tool description - Update newTaskTool.ts to handle config parameter validation - Modify initClineWithTask to accept and apply config profiles - Add comprehensive tests for new functionality - Maintain backward compatibility for existing usage Fixes #6839
| cline.emit(RooCodeEventName.TaskSpawned, newCline.taskId) | ||
|
|
||
| pushToolResult(`Successfully created new task in ${targetMode.name} mode with message: ${unescapedMessage}`) | ||
| const successMessage = configName |
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.
User-facing success messages are constructed as plain strings. Consider using the translation function (e.g., t()) for these messages to support internationalization.
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
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.
Reviewing my own code feels like debugging in a mirror - everything's backwards but somehow still broken.
| | "experiments" | ||
| > | ||
| >, | ||
| ) { |
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.
The overloaded function signature accepting configNameOrOptions as either a string or options object could be confusing. Consider splitting into two methods or using a more explicit parameter structure for better clarity?
This works but might trip up future maintainers (including myself in 3 months).
| } | ||
| } catch (error) { | ||
| // Log error but continue with default config | ||
| this.log(`Failed to load config '${configName}' for new task: ${error}`) |
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.
When a config fails to load, we log the error but continue silently. Should we notify the user that the requested config couldn't be loaded and we're falling back to the default? This could prevent confusion when a task doesn't use the expected model.
| Parameters: | ||
| - mode: (required) The slug of the mode to start the new task in (e.g., "code", "debug", "architect"). | ||
| - message: (required) The initial user message or instructions for this new task. | ||
| - config: (optional) The slug/name of the API configuration profile to use for this task (e.g., "claude-3-5-sonnet", "gpt-4-debug", "fast-model"). If not specified, uses the default configuration for the mode. |
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.
The tool description could mention that invalid configs fall back to default gracefully. This would set proper expectations for users.
| // Verify initClineWithTask was NOT called for partial message | ||
| expect(mockInitClineWithTask).not.toHaveBeenCalled() | ||
| }) | ||
| }) |
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.
Great test coverage for the main functionality! Consider adding edge case tests for:
- Config loading errors (network failures, corrupted config)
- Config names with special characters
- Race conditions when switching configs rapidly
|
Not planned |
Summary
This PR implements the feature requested in #6839 to add an optional
configparameter to thenew_tasktool, allowing users to specify different API configuration profiles when creating sub-tasks.Changes
Core Implementation
src/core/prompts/tools/new-task.tsto include the new optionalconfigparameter with clear documentation and examplessrc/core/tools/newTaskTool.tsto:src/core/webview/ClineProvider.tsto:initClineWithTaskType System Updates
configtotoolParamNamesinsrc/shared/tools.tsNewTaskToolUseinterface to include the config parameterTesting
Benefits
✅ Enhanced Flexibility: Users can optimize model selection per task
✅ Cost Control: Use appropriate models for different task types
✅ Workflow Efficiency: No need to interrupt workflow to change global settings
✅ Backward Compatibility: Optional parameter doesn't break existing usage
✅ Leverages Existing Infrastructure: Uses the established configuration management system
Testing
npx vitest run core/tools/__tests__/newTaskTool.spec.ts)Example Usage
Fixes #6839
Important
Adds optional
configparameter tonew_tasktool for specifying API configuration profiles, enhancing flexibility and efficiency.configparameter tonew_tasktool innew-task.tsfor specifying API configuration profiles.newTaskTool.tsto handleconfigparameter, validate existence, and apply it during task initialization.ClineProvider.tsto load and apply configuration profiles ininitClineWithTask().configtotoolParamNamesintools.ts.NewTaskToolUseinterface to includeconfigparameter.newTaskTool.spec.tsfor config validation, error handling, and backward compatibility.This description was created by
for e73e20d. You can customize this summary. It will automatically update as commits are pushed.