feat: human-readable permission labels, deny rule feedback, and multi-dir search improvements#2637
Conversation
…-dir search tests - Add buildHumanReadableRuleLabel() to convert raw permission rules into natural-language descriptions for the 'Always Allow' UI options - Add PermissionManager.findMatchingDenyRule() to surface which deny rule caused a tool to be blocked, improving error messages in coreToolScheduler - Update ToolConfirmationMessage to use friendly labels with i18n support - Add comprehensive tests for new permission features and multi-directory search in glob, grep, and ripGrep tools - Fix integration test for tool-control allowedTools configuration
📋 Review SummaryThis PR introduces significant UX improvements to the permission system by replacing raw rule syntax with human-readable labels in confirmation dialogs, and adds matching deny rule information to error messages. It also enhances multi-directory search functionality for glob/grep/ripGrep tools with comprehensive test coverage. The implementation is well-structured, thoroughly tested (all 291 tests pass), and maintains backward compatibility. 🔍 General Feedback
🎯 Specific Feedback🟢 Medium
🔵 Low
✅ Highlights
|
TLDR
Improve the permission system UX by replacing raw rule syntax (e.g.
Read(//Users/alice/**)) with human-readable labels (e.g. "read files in /Users/alice/") in the "Always Allow" confirmation dialog, and surface matching deny rules in error messages when tools are blocked. Also adds comprehensive tests for multi-directory search in glob/grep/ripGrep tools (introduced in the previous commit).Screenshots / Video Demo
Before:
After:
Deny rule feedback (new):
Dive Deeper
Human-readable permission labels (
rule-parser.ts)buildHumanReadableRuleLabel()that converts rule arrays frombuildPermissionRules()into natural-language descriptionscleanPathSpecifier()helper to strip glob suffixes (/**,/*) and the//absolute-path prefix for cleaner displayDISPLAY_NAME_TO_VERB(e.g. Read → "read files", Bash → "run commands", WebFetch → "fetch from")Deny rule feedback (
permission-manager.ts,coreToolScheduler.ts)PermissionManager.findMatchingDenyRule()to locate the first deny rule matching a given contextcoreToolSchedulerto include the matching deny rule in error messages, helping users understand why a tool was blockedUI & i18n (
ToolConfirmationMessage.tsx, locale files)[Rule(specifier)]bracket labels withbuildHumanReadableRuleLabel()output'Always allow {{action}} in this project'and'Always allow {{action}} for this user'across all 6 localesMulti-directory search tests (glob, grep, ripGrep)
.qwenignorefrom each workspace directory (ripGrep)Reviewer Test Plan
qwenin a project, trigger a tool that requires confirmation (e.g. file edit outside workspace). Verify the "Always allow" options show human-readable labels instead of raw rule syntax.Bash(rm *)in settings), then ask the agent to runrm— verify the error message includes "Matching deny rule: ...".npm run test -- packages/core/src/permissions/permission-manager.test.ts packages/core/src/tools/glob.test.ts packages/core/src/tools/grep.test.ts packages/core/src/tools/ripGrep.test.ts— all 291 tests should pass.Testing Matrix
Linked issues / bugs
N/A — general permission UX improvement and test coverage for multi-directory search.