-
Notifications
You must be signed in to change notification settings - Fork 2.3k
enhancement: copy MCP permission #6456
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
Merged
Merged
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
dinhlongviolin1
approved these changes
Sep 15, 2025
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.
Caution
Changes requested ❌
Reviewed everything up to a26445e in 1 minute and 55 seconds. Click for details.
- Reviewed
325lines of code in21files - Skipped
0files when reviewing. - Skipped posting
6draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. web-app/src/containers/dialogs/ToolApproval.tsx:55
- Draft comment:
Good enhancement: appending the permission scope clarification below the tool name improves clarity. Verify that punctuation and spacing are consistent. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. web-app/src/containers/dialogs/ToolApproval.tsx:101
- Draft comment:
Nice addition of the 'capitalize' class on the allow button to improve UI consistency. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. web-app/src/locales/en/tool-approval.json:3
- Draft comment:
Possible unintended 'hello' text in the description; consider removing it if it was left by mistake. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to consider removing a text from the description, which is not allowed as per the rules. The rules explicitly state not to ask the PR author to update the PR description.
4. web-app/src/locales/en/mcp-servers.json:37
- Draft comment:
The updated allowPermissionsDesc now clearly states that the setting applies globally to all conversations, including new chats. The revised text is clear and appropriate. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
5. web-app/src/locales/en/tools.json:10
- Draft comment:
The addition of the new 'permissionScope' key helps clarify that permissions granted apply only to the current conversation. This consistency across locales is beneficial. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
6. web-app/src/locales/zh-CN/tools.json:7
- Draft comment:
The translation for "alwaysAllow" was changed from "始终允许" to "在线程中允许". Please double-check if "在线程中允许" is intentional, as it might be a typographical error. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is a UI translation change. The new translation actually seems more accurate since line 10 explicitly states that permissions only apply to the current conversation. The old "always allow" was potentially misleading. We should trust that the PR author (likely a Chinese speaker) made this change intentionally to improve accuracy. I might be wrong about the nuances of Chinese translation. Maybe there's a better way to express this concept in Chinese. While translation nuances can be complex, our rules explicitly state not to comment on UI changes and to assume the author made UI changes correctly. This is clearly a UI text change. Delete the comment. This is a UI translation change that we should assume was made correctly by the author, and the new translation actually appears to better match the stated permission scope.
Workflow ID: wflow_t5nhdgce9N4Whvai
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Minh141120
approved these changes
Sep 15, 2025
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.
Describe Your Changes
This pull request refines the tool approval dialog and updates localization strings across multiple languages to clarify the scope of tool permissions and improve consistency. The main focus is on making it clear that permissions granted for tools apply only to the current conversation/thread, not globally, and on improving the user interface for tool approval actions.
Tool Approval Dialog Improvements:
ToolApproval.tsxto display a new description about the permission scope, and improved button styling and labeling for clarity. [1] [2]Localization and Permission Scope Clarifications:
permissionScopestring intools.jsonfor all supported languages to clarify that granted permissions only apply to the current conversation/thread. [1] [2] [3] [4] [5] [6]Global Permission Setting Clarifications:
allowPermissionsDescstring inmcp-servers.jsonfor all supported languages to clarify that enabling global tool permissions applies to all conversations, including new chats. [1] [2] [3] [4] [5] [6]Minor UI/UX Adjustments:
These changes ensure users have a clearer understanding of how tool permissions are applied and improve the overall experience when approving tool actions.
Fixes Issues
Self Checklist
Important
Refines tool approval dialog and updates localization strings to clarify tool permission scope and improve consistency across languages.
ToolApproval.tsxto include a new description about permission scope and improved button styling.permissionScopestring intools.jsonfor all supported languages to specify permissions apply only to the current conversation.allowPermissionsDescinmcp-servers.jsonto clarify global tool permissions apply to all conversations, including new chats.This description was created by
for a26445e. You can customize this summary. It will automatically update as commits are pushed.