-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: auto-approve commands based on risk level #2670
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
|
|
The pull request introduces a significant number of changes across various components, primarily focused on adding risk level parameters and metadata. While these changes are related to the new feature of risk level handling, the size of the pull request is quite large, with 22 files changed and over 1300 lines added. To improve reviewability and maintainability, consider splitting the pull request into smaller, more focused pull requests. Here are some suggestions on how to split the changes:
By splitting the changes into these categories, it will be easier for reviewers to focus on specific areas and ensure thorough review and testing. Thank you for your hard work on this feature! |
|
translations coming to a pull request near you ... |
|
languages are already |
|
It would be nice to add this Markdown table at This was a really cool & practical example of using symbolic math to solve common CS problems we have to deal with. Thanks!
|
great idea!
I really enjoyed predicate logic it and set notation in some of my early computer science discrete math courses, but it's funny that I have only really used them for any practical purpose as part of artificial intelligence instructions (oh yeah, and whiteboard notes!). |
d484b04 to
c622c2f
Compare
a4d5737 to
bfcfe75
Compare
|
this is ready for review |
|
This pull request is quite large, with 34 files changed and over 2000 lines added. It seems to introduce a comprehensive set of features related to command risk levels and risk analysis. To improve the review process, consider splitting the changes into smaller, more focused pull requests if possible. For example:
This will make it easier to review and test each part independently. Thank you! |
a4342f5 to
f48427d
Compare
0b80781 to
4bae6db
Compare
Use consistent translation '只读' for readOnly across settings to improve clarity Signed-off-by: Eric Wheeler <[email protected]>
Clarify risk level definitions: - Specify inverse operations exclude unconfirmed data sources - Add content preservation check for reversible changes - Simplify complex changes as complement of other risk levels - Streamline destructive changes definition around content loss Signed-off-by: Eric Wheeler <[email protected]>
… proof Redefine the 'reversibleChanges' risk level using rigorous mathematical notation: - Replace the informal definition with a formal set-theoretic approach - Add operational semantics for command sequences with termination judgments - Express reversibility as an injective function property - Introduce domain and image sets for command functions This change establishes that command reversibility is equivalent to function injectivity, providing a clear mathematical criterion for determining whether operations can be safely undone. Signed-off-by: Eric Wheeler <[email protected]>
Add risk level and analysis display to CommandExecution component: - Show risk level with codicon icons and colors - Display risk analysis with proper styling - Add translations for risk level descriptions - Pass metadata from ChatRow to CommandExecution Signed-off-by: Eric Wheeler <[email protected]>
Use consistent state access pattern for commandRiskLevel setting: - Remove direct contextProxy.getGlobalState access - Destructure commandRiskLevel from getState like other settings - Pass commandRiskLevel prop to AutoApproveSettings component This aligns with the documented settings persistence chain in cline_docs/settings.md, ensuring proper state hydration and display. Signed-off-by: Eric Wheeler <[email protected]>
This addresses ambiguity in the risk_analysis parameter description by requiring proof of key relationships, ensuring clarity for contributors. It also tightens the readOnly criteria to explicitly exclude all non-read operations and enforce formal constraints, preventing inconsistent implementations. Signed-off-by: Eric Wheeler <[email protected]>
Define inverse_is_knowable to require inverse operations be derivable solely from command sequence, result state, and known inverse commands (C, s′, I). Key changes: - Add local/remote distinction to state space S - Clarify c⁻¹(R) excludes external knowledge - Make readOnly condition more precise with s′ = s₀ - Replace abstract inverse existence with knowable inverse - Add comprehensive command examples by risk level - Define destructiveChanges to catch indirect data loss Improves accuracy of risk assessment by ensuring inverse operations: 1. Do not rely on external context or backups 2. Are inferable from command and result alone 3. Use only standardized inverse commands Signed-off-by: Eric Wheeler <[email protected]>
gh create is not reversible because creating issues can only be closed not deleted extended the set cross product to include api,web,cloud,virt
This change ensures that risk information is always displayed in the CommandExecution component, even during partial streaming of command execution blocks. Previously, the metadata object was only created if commandRisk was truthy, which prevented risk information from being displayed during streaming. Signed-off-by: Eric Wheeler <[email protected]>
Fixed the import path for CommandRiskLevel in ChatView.tsx to use @roo-code/types instead of a non-existent path, resolving the build error. Signed-off-by: Eric Wheeler <[email protected]>
…disabled Adds a new 'disabled' option to command risk levels that: - Completely excludes risk analysis from system instructions when disabled - Bypasses risk and risk_analysis parameter validation for execute_command - Simplifies command execution in trusted environments - Sets 'disabled' as the default for better initial user experience The implementation: - Adds 'disabled' enum value to commandRiskLevel - Changes default from 'none' to 'disabled' across the codebase - Adds conditional logic to skip validation when disabled - Updates UI components with new option and description - Removes duplicate utility functions from webview - Passes commandRiskLevel to system prompt generation
Signed-off-by: Eric Wheeler <[email protected]>
13928ff to
219062f
Compare
|
Reddit stats, people really want this PR to merge: https://www.reddit.com/r/RooCode/comments/1lmzm6l/autoapprove_commands_based_on_risk_level_pending/ |
219062f to
b622a26
Compare
daniel-lxs
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.
This is a solid implementation and functions as expected when tested. However, the addition of ~2,000 tokens to the system prompt for risk analysis is concerning. If the context window fills up, there's a risk the model will ignore these instructions, especially with weaker models, leading to potentially incorrect risk assertions.
I believe that allowing people to whitelist and eventually blacklist command and command prefixes is a pretty simple but effective approach.
Awesome, thanks for testing. I've been using it almost every day for months and it works great, even in very large contexts. |
Hi @daniel-lxs, what do you think about making this an experimental feature? This would give users a chance to test it, and since it is experimental it can always be pulled out if you find that it is not beneficial to the project. |
|
This is definitely a clever idea, but it seems hard to get right - I don't really trust the LLMs to make the right call on risk level without human oversight. My energy is more around using the LLMs to generate good suggested prefixes for allowlisting commands inline (and maybe eventually blocklisting), but still with the human making the final call on which commands execute in a deterministic way. I'd rather use the pixels in the command execution area to support this interaction. Thank you for your efforts on this one, but I am going to close it for now. |
I understand your concern, which is why I thought an experimental flag could be appropriate. I've been using this every day since April and it works great with Sonnet 3.x,4 and Gemini 2.5. I'm not pressuring you, this is your project. I hope you give it some more thought and consider letting other users test it to see how it how it performs. |

Context
This is similar to Cline's competing "auto-approve safe commands" feature—but approval in this PR is much smarter: command risk analysis is performed based on formal predicate logic.
For example, when set to read-only:
Roo will auto-execute only commands that have formally matched the predicate logic for read only (see below):
Implementation
Risk levels are formally defined through predicate logic and the model determines the risk level after producing in the command and risk analysis, so risk level determination is retrospective and accurate. When "disabled" (default), all system instructions for related risk analysis are excluded, so there is no change to Roo's default system prompt size.
Risk level total ordering: readOnly < reversibleChanges < complexChanges < serviceInterruptingChanges < destructiveChanges S = F ∪ P ∪ N ∪ H ∪ M ∪ Z ∪ E F = storage: files, databases, git repos, disks, … P = processes, services, VMs, containers, … N = network configs, … H = hardware, physical controls, … M = memory states, locks, resources, … Z = security: permissions, keys, tokens, … E = temporal state: tasks, cron, timers, … s₀ ∈ S: initial state before C s′ ∈ S′: final state after C atoms(x): minimal addressable units content(x,s): accessible atoms in s a ∉ content(x,s) if deleted, truncated, overwritten, corrupted, encrypted with lost key C = {c₁,c₂,…,cₙ}: command sequence T = Tr ∪ Tm: intended targets Tr ⊆ S: read-only targets Tm = {x | x ∈ S ∨ (x ∉ S ∧ x ∈ S′)}: modifiable/creatable targets R = {x | (x ∈ s₀ ∧ c(x) ≠ x) ∨ (x ∉ s₀ ∧ x ∈ S′)}: modified/created elements c⁻¹(R): inverse operations r(C)=readOnly ⟺ Tm = ∅ ∧ R = ∅ ∧ (Tr ≠ ∅ ∨ Tr = ∅) ∧ S′ = S ∧ (∀x ∈ S: content(x,s₀) = content(x,s′)) r(C)=reversibleChanges ⟺ Tm ≠ ∅ ∧ ∀s₀ ∈ S: ∃c⁻¹: (c⁻¹(c(s₀))=s₀) ∧ (|c⁻¹|=1) ∧ R = Tm r(C)=complexChanges ⟺ ¬destructiveChanges ∧ Tm ≠ ∅ ∧ ∃s′=c(s₀): (R ⊆ Tm) ∧ (∀x ∈ R: (∃p ⊂ atoms(x): p ∈ content(x,s′)) ∧ (∃q ⊂ atoms(x): q ∉ content(x,s′) ∧ ¬∃c⁻¹: c⁻¹(c(q))=q)) r(C)=serviceInterruptingChanges ⟺ ∃p ∈ Tm ∩ P, ∃t₁ ∈ ℝ⁺: A(p,0) ∧ ¬A(p,t₁) ∧ ((∃t₂ > t₁: A(p,t₂)) ∨ p ∉ s′) where A(x,t): availability at time t r(C)=destructiveChanges ⟺ (∃x ∈ s₀: x ∉ s′) ∨ (R ⊈ Tm) ∨ (∃x ∈ s₀: content(x,s₀) ≠ ∅ ∧ (∀a ∈ atoms(x): a ∉ content(x,s′))) For compound commands: r(C) = max{r(c₁), r(c₂), …, r(cₙ)} ### Usage: <execute_command> <command>Your command here</command> <risk_analysis>[reasoning]</risk_analysis> <risk>Risk level here</risk> <cwd>Working directory path (optional)</cwd> </execute_command>Screenshots
This PR provides greater command safety and user feedback before running commands:
and allows users to select the auto-approve risk level they are comfortable with:
How to Test
Get in Touch
Discord: KJ7LNW
Important
Introduces auto-approve feature for commands based on risk levels, updating settings, UI, and logic across multiple files.
global-settings.tsandterminal.ts.readOnly,reversibleChanges,complexChanges,serviceInterruptingChanges, anddestructiveChanges.terminal.ts.commandRiskLevelsetting inglobal-settings.tsandClineProvider.ts.SettingsView.tsxandAutoApproveSettings.tsxto include risk level selection.ChatView.tsxandCommandExecution.tsxto display risk levels and analyses.isRiskAnalysisEnabled,isValidRiskLevel, andisRiskAllowedinterminal.ts.executeCommandToolinexecuteCommandTool.tsto handle risk levels.This description was created by
for c6b43fb77746c38c2622188b20d865b4c01c070e. You can customize this summary. It will automatically update as commits are pushed.