fix: restore MCP tool auto-approval behavior when alwaysAllowMcp is enabled #9205
+301
−3
Conversation
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
…nabled - Fixed regression introduced in #9157 where MCP tools required individual alwaysAllow property - When alwaysAllowMcp is true, all MCP tools are now auto-approved by default - Tools can still opt-out by explicitly setting alwaysAllow=false - Added comprehensive tests for MCP auto-approval scenarios Fixes #9204
dosubot
bot
added
the
size:L
Contributor
Author
Review complete. Found 1 minor issue that should be addressed.
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| import type { McpServer, McpTool } from "../../shared/mcp" | ||
|
|
||
| import { isWriteToolAction, isReadOnlyToolAction } from "./tools" | ||
| import { isMcpToolAlwaysAllowed } from "./mcp" |
Contributor
Author
There was a problem hiding this comment.
The isMcpToolAlwaysAllowed import is no longer used after refactoring the MCP auto-approval logic. This unused import should be removed to keep the code clean.
Fix it with Roo Code or mention @roomote and request a fix.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR attempts to address Issue #9204. Feedback and guidance are welcome.
Problem
When using an MCP tool action with auto-approve set (
alwaysAllowMcp: true), Roo was still asking for approval. This was a regression introduced in version 3.31.1 after the auto-approval logic was moved from the webview to the Task class in PR #9157.Root Cause
The regression was caused by a change in the auto-approval logic that required BOTH:
alwaysAllowMcpsetting to be truealwaysAllowproperty set to trueThis was different from the previous behavior where setting
alwaysAllowMcpwould auto-approve ALL MCP tools by default.Solution
Modified the auto-approval logic in
checkAutoApprovalto:alwaysAllowMcpis true (restoring previous behavior)alwaysAllow: falseChanges
src/core/auto-approval/index.tsto fix the MCP auto-approval logicsrc/core/auto-approval/__tests__/checkAutoApproval.spec.tsTesting
alwaysAllow: falseFixes #9204
Important
Restores MCP tool auto-approval behavior when
alwaysAllowMcpis true, with individual tool opt-out support, and adds comprehensive test coverage.alwaysAllowMcpis true incheckAutoApprovalinindex.ts.alwaysAllow: false.checkAutoApproval.spec.tsfor various MCP auto-approval scenarios.checkAutoApproval.This description was created by
for b8f00cc. You can customize this summary. It will automatically update as commits are pushed.