Add mode permissions for new_task, switch_mode and ask_followup_questions #3171
Conversation
|
|
I had to change the layout structure from a grid to a flex column to prevent layout issues. |
| "toolNames": { | ||
| "read": "Dosyaları Oku", | ||
| "edit": "Dosyaları Düzenle", | ||
| "browser": "Tarayıcı Kullan", | ||
| "command": "Komutları Çalıştır", | ||
| "mcp": "MCP Kullan" | ||
| "mcp": "MCP Kullan", | ||
| "subtask": "Alt görevler oluştur", |
There was a problem hiding this comment.
For consistency with the other tool name translations (e.g., 'Dosyaları Oku', 'Dosyaları Düzenle', 'Tarayıcı Kullan', 'MCP Kullan'), consider capitalizing the first letters of each word in the newly added strings. For example, change 'Alt görevler oluştur' to 'Alt Görevler Oluştur', 'Modları değiştir' to 'Modları Değiştir', and 'Takip soruları sor' to 'Takip Soruları Sor'. This way, all similar UI labels use the same style.
| "subtask": "Alt görevler oluştur", | |
| "subtask": "Alt Görevler Oluştur", |
src/exports/roo-code.d.ts
Outdated
| { | ||
| fileRegex?: string | undefined | ||
| slugRegex?: string | undefined |
There was a problem hiding this comment.
Let me see if I change my mind after reading the whole diff, but I think an array of slugs might make more sense than a regex here.
There was a problem hiding this comment.
Yeah, still think an array of slugs would be easier to manage.
| const currentMode = getCurrentMode() | ||
| const isCustomMode = findModeBySlug(visualMode, customModes) | ||
| const customMode = isCustomMode | ||
| const isGroupEnabled = isCustomMode | ||
| ? customMode?.groups?.some((g) => getGroupName(g) === group) | ||
| : currentMode?.groups?.some((g) => getGroupName(g) === group) | ||
|
|
||
| // Check if the group should be always available and thus potentially non-editable | ||
| // For now, we allow editing all for custom modes as per test requirements | ||
| const isDisabled = !isCustomMode // || TOOL_GROUPS[group].alwaysAvailable; |
There was a problem hiding this comment.
| const isDisabled = !isCustomMode // || TOOL_GROUPS[group].alwaysAvailable; | |
| const isDisabled = !isCustomMode |
| const matchingNames = getMatchingModeNames(options.slugRegex, modes) | ||
| if (matchingNames.length > 0) { | ||
| // Format for expanded view: Allowed modes: Name1, Name2 | ||
| description = `${t("prompts:tools.allowedModes")}: ${matchingNames.join(", ")}` |
There was a problem hiding this comment.
We should either remove the colon from the end of the translations or from here - it's showing up as a double colon currently.
| "mcp": "Use MCP", | ||
| "subtask": "Create Subtasks", | ||
| "switch": "Switch Modes", | ||
| "followup": "Ask Follow-up Questions" |
There was a problem hiding this comment.
I think "Ask Questions" is a more obvious label here
|
I wonder if we should start by just adding the 3 new checkboxes without the allowed modes and get that solid, and then do the mode restrictions in a follow-up PR? |
Sure, I can take that out. Which option do you prefer? The array or to leave it out for now? |
I think leave it out for now, since I think there’s a little prompt engineering involved to make sure the model knows what it’s allowed to do |
Wouldn't be so hard to add that and I was thinking about it but decided against because file restrictions are also not mentioned in the prompt and also I tried to avoid adding more token. Alright, will remove it. |
* refactor to not pass message for showMcpView * changeset
|
@mrubens please wait with your review until I give you a go |
|
How do you think we should handle backwards compatibility on this? For instance the custom orchestrators that people have written? |
|
Would it make sense to switch from a whitelist to a blacklist for available tools and migrate the custom modes as well? This approach would allow us to infer which tools the user expects to be available while also enabling users to exclude tools they don't want enabled for any mode. The UI would not require any changes. |
I will have a look into that. pretty busy these days, sorry. |
So you mean we should extend it and allow blacklisting any available tool? Actually I would also prefer the ability to disable any tool, even read_file if someone likes to. @mrubens what do you think about that? |
|
@cannuri |
Yeah get u. Therefore u meant blacklisting instead of whitelisting. But we could also auto migrate their .roomodes |
|
Thanks for your patience on this PR. We need to either land the plane on this soon or shift the discussion back to an issue to clearly agree on the scope and details before implementation. Given the time elapsed since the changes were requested, let's determine whether to wrap this up quickly or close it and revisit through our issue-first approach. Let us know how you'd like to proceed. |
Context
This PR introduces a more granular permission system for specific tools (
new_task,switch_mode,ask_followup_question) and adds a mechanism (slugRegex) to restrict which modes can be targeted bynew_taskandswitch_mode.It is now possible to restrict a mode's ability to create subtasks, switch modes or ask followup question. Furthermore you can also define which modes are available for a subtask or for switching by defining
slugRegex(just like definingfileRegex)WHY:
Orchestratorto create a subtask instead of switching modes (as it would do that from time to time which unnecessarily bloats the context window)fileRegexfor file editing.Implementation
This feature was implemented by introducing new permission groups (
subtask,switch,followup) in the schema and shared tool configurations. The corresponding tools (new_task,switch_mode,ask_followup_question) were moved under these groups, removing them from being always available.The core permission checking logic in
isToolAllowedForModewas updated to handle these new groups. For thesubtaskandswitchgroups, an optionalslugRegexproperty was added to the schema (groupOptionsSchema) and the checking logic was enhanced to parse tool parameters (modeormode_slug) and validate them against the provided regex, denying the tool use if the target mode slug doesn't match.The Prompts UI (
PromptsView.tsx) was updated to display checkboxes for the new permissions. Logic was added to interpret theslugRegexand display the names of allowed modes in the UI, formatted differently for collapsed(Names...)and expandedAllowed modes: ...views (just like "Edit Files" is styled). This required accessing the list of all available modes within the component.Localization keys were added to
prompts.jsonfiles for the new permission names and the "Allowed modes:" text. Default permissions were enabled for built-in modes. Comprehensive tests (logic, UI, schema) were added and updated throughout the process, following TDD principles where appropriate. External documentation was also updated.Screenshots
Before
After
How to Test
slugRegex:.roomodesfile for a custom mode.subtaskpermission using the tuple format with aslugRegex, e.g.,[ "subtask", { "slugRegex": "^(code|test)$" } ].(Code, Test)next to it.Allowed modes: Code, Testappears below the "Create Subtasks" checkbox.<new_task><mode>code</mode>...</new_task>. This should succeed.<new_task><mode>ask</mode>...</new_task>. This should fail (the tool use should be rejected by the backend based onisToolAllowedForMode).switchpermission and theswitch_modetool.Get in Touch
Discord: can.nuri
Important
This PR adds a granular permission system for specific tools, updates the UI to support these changes, and includes tests and localization updates.
subtask,switch,followupinschemas/index.ts.isToolAllowedForModeinmodes.tsto handle new groups andslugRegex.PromptsView.tsxto display new permissions with checkboxes and interpretslugRegex.prompts.jsonfiles.slugRegexlogic inmodes.test.tsandPromptsView.test.tsx.toolGroupsintools.tsto include new groups.new_task,switch_mode,ask_followup_questionfromALWAYS_AVAILABLE_TOOLS.This description was created by
for 7f2e046. You can customize this summary. It will automatically update as commits are pushed.