-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Add mode permissions for new_task, switch_mode and ask_followup_questions #3171
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
|
| "command": "Komutları Çalıştır", | ||
| "mcp": "MCP Kullan" | ||
| "mcp": "MCP Kullan", | ||
| "subtask": "Alt görevler oluştur", |
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.
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
| "read" | "edit" | "browser" | "command" | "mcp" | "subtask" | "switch" | "followup", | ||
| { | ||
| fileRegex?: string | undefined | ||
| slugRegex?: string | undefined |
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, still think an array of slugs would be easier to manage.
|
|
||
| // 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should either remove the colon from the end of the translations or from here - it's showing up as a double colon currently.
mrubens
left a comment
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.
I think we also need to update the system prompt to remove the tools if they're not allowed in the mode, and probably also to tell the model which modes it can switch to / start a new task with.
| "mcp": "Use MCP", | ||
| "subtask": "Create Subtasks", | ||
| "switch": "Switch Modes", | ||
| "followup": "Ask Follow-up Questions" |
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.
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 |
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. |
we need to decide if we want to just extend this directly to all available tools (which i would prefer). also need to decide on blacklisting vs whitelisting |
|
Hey @cannuri, Sure! I think the best idea is to create an issue for this since that's what we will be checking first from now on. |
@daniel-lxs yep makes sense 👍 |
|
Closing temporarily |
|
Any idea if this, or at least the feature to disable ask_followup_question, will be released? I have some usecases where Roo automatically run and, though clearly instructed not to use ask_followup_question, some mode still uses it causing the whole workflow to be stalled. I usually run Roo in isolated docker env and not look at it for hours. A lot of time, I come back seeing Roo haven't started anything due to stuck at asking a non-relevant question |

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.