Skip to content

Conversation

@rtfeldman
Copy link
Contributor

This fixes code duplication where visual tests and unit tests were manually constructing permission options instead of using the production code path.

Now tests use ToolPermissionContext::new(...).build_permission_options() which ensures they exercise the actual production logic for generating permission option IDs, labels, and patterns.

Changes

  • Added ToolPermissionContext::new() constructor
  • Added ToolPermissionContext::build_permission_options() method that contains the canonical logic for generating permission options
  • Updated authorize_with_context() to use the new method
  • Updated all visual tests in visual_test_runner.rs to use the production code path
  • Updated all unit tests in thread_view.rs to use the production code path

rtfeldman and others added 29 commits January 6, 2026 22:58
Add granular per-tool permission settings for the Zed agent with regex-based
rules. This PR adds:

- ToolPermissionsFeatureFlag to gate UI changes
- Settings schema types: ToolPermissionsContent, ToolRulesContent,
  ToolRegexRule, ToolPermissionMode
- Runtime types: ToolPermissions, ToolRules, CompiledRegex
- Regex compilation in from_settings() with error logging for invalid patterns
- Default deny rules for dangerous terminal commands and system paths
- Unit tests for settings parsing and regex compilation

This is the first of multiple PRs implementing the tool permissions design.
Settings can now be written and parsed without errors.

Co-authored-by: Claude Opus 4.5
Amp-Thread-ID: https://ampcode.com/threads/T-019b9022-7cc2-739a-aa81-05863e50d0de
Co-authored-by: Amp <[email protected]>
- Store case_sensitive flag in CompiledRegex for debugging
- Include tool name in invalid regex warning messages
- Add comprehensive tests for ToolRules::default() and multiple tools
- Add integration tests that validate default.json tool_permissions
- Add tests verifying dangerous commands are blocked
- Add tests verifying safe commands are allowed
- Add always_allow defaults for safe terminal commands (cargo, git status, ls, etc.)
- Add edit_file deny patterns for sensitive files (.env, .pem, .key)
- Add fetch allow rules for common documentation sites
- Add .git directory to delete_path deny rules
- Fix fork bomb regex pattern escaping

Co-Authored-By: Claude Opus 4.5
Co-Authored-By: Claude Opus 4.5
Co-Authored-By: Claude Opus 4.5
- Add Target State section clarifying end goal
- Add Third-Party Tools section for MCP/custom tools
- Update UI Changes to document hidden 'Always Allow' button
- Add PR 3.5 for third-party tool permissions
- Add example settings for third-party tools

Co-Authored-By: Claude Opus 4.5
- Add tool ID format specification (mcp__<server>__<tool>)
- List files to modify with specific changes
- Add implementation details with code examples
- Add specific unit test requirements
- Clarify verification criteria

Co-Authored-By: Claude Opus 4.5
- Rename 'Third-Party Tools' to 'MCP Tools from Context Servers'
- Clarify why MCP tools don't get pattern buttons (arbitrary schemas)
- List exact 3 buttons for MCP tools
- Fix example settings to use proper mcp__ prefix
- Add note about tool ID namespacing to avoid collisions

Co-Authored-By: Claude Opus 4.5
- Add tool_permissions.rs with decide_permission() function
- Implement deny > confirm > allow precedence hierarchy
- Integrate permission checking into terminal tool
- Add tests for permission evaluation and terminal tool integration

Co-Authored-By: Claude Opus 4.5
Adds permission checking to the remaining 7 tools that require granular
permissions:
- edit_file: Checks path against permission rules
- delete_path: Checks path against permission rules
- move_path: Checks both source and destination paths
- create_directory: Checks path against permission rules
- save_file: Checks all paths, denies if any are blocked
- fetch: Checks URL against permission rules
- web_search: Checks query against permission rules

Each tool follows the pattern: Allow = proceed, Deny = return error,
Confirm = prompt user. The deny > confirm > allow precedence is enforced
by the decide_permission_from_settings() function.

Co-authored-by: Claude Opus 4.5
Amp-Thread-ID: https://ampcode.com/threads/T-019b93c6-044c-739c-8c0f-fb8e426eaa5d
Co-authored-by: Amp <[email protected]>
Co-Authored-By: Claude Opus 4.5
- Add authorize_third_party_tool() method to ToolCallEventStream for MCP tools
- Add mcp_tool_id() helper to generate prefixed tool IDs (mcp:<server>:<tool>)
- Update ContextServerTool::run() to use new authorization method
- Add set_tool_default_mode() helper to settings for updating tool permissions
- Add comprehensive unit tests for MCP tool permissions

MCP tools now show 3 buttons: 'Always allow <name> MCP tool', 'Allow', 'Deny'.
Unlike built-in tools, MCP tools don't support pattern-based permissions since
their input schemas are arbitrary. Tool IDs are namespaced with 'mcp:' prefix
to avoid collisions with built-in tools.

Co-Authored-By: Claude Opus 4.5
- Add ToolPermissionContext struct to carry tool name and input value through authorization flow
- Add authorize_with_context method that generates tool-specific buttons:
  - 'Always allow terminal' sets tools.terminal.default_mode = 'allow'
  - 'Always allow cargo commands' adds pattern to tools.terminal.always_allow
  - Similar buttons for all 8 granular permission tools
- Add pattern extraction functions for terminal commands, file paths, and URLs
- Add methods to AgentSettingsContent for modifying tool permissions:
  - set_tool_default_mode
  - add_tool_allow_pattern
- Update all tools to use authorize_with_context

Co-Authored-By: Claude Opus 4.5
- Remove 'Always Allow' from authorize_with_context options
- Update tests to expect 4 buttons with pattern, 3 without
- Add visual tests for permission button layouts
- Update design doc with third-party tool support plan

Co-Authored-By: Claude Opus 4.5
Co-Authored-By: Claude Opus 4.5
…d_permission_options()

This fixes code duplication where visual tests and unit tests were manually
constructing permission options instead of using the production code path.

Now tests use ToolPermissionContext::new(...).build_permission_options()
which ensures they exercise the actual production logic for generating
permission option IDs, labels, and patterns.
#45423 changed the Blade
renderer to use a white clear color for "opaque" windows. This interacts
poorly with our client-side decorations, causing the shadows around the
window to be drawn over a white background.

Release Notes:

- N/A
…ew is in use (#46283)

The tree view effectively always sorts by status, not by path.

Release Notes:

- Fixed incorrect ordering of paths in the project diff when using the
git panel's tree view.
Closes #ISSUE

Release Notes:

- N/A *or* Added/Fixed/Improved ...
Closes #ISSUE

Organizes the giant list of settings UI items in `page_data.rs` a bit so
that sections are split out in sub functions that (for the most part)
return constant size arrays. Page items is also converted to a
`Box<[...]>` instead of a Vec.

The goal here is to have working breadcrumbs when in the settings UI
file (e.g. `fn general_page() > fn general_section()`), and to escape
the `vec![]` macro so that `rust-analyzer` works more consistently.

This should help both humans and LLMs know where they are in the file
and use the outline to navigate it instead of having to read the whole
thing to have an idea of what is going on.

Release Notes:

- N/A *or* Added/Fixed/Improved ...
## Add variable row height mode to data table infrastructure

This PR introduces support for variable row heights in the data table
component, laying the groundwork for more flexible tabular data
rendering in Zed.

**Context:**  
This is the first in a series of infrastructure-focused PRs split out
from the [original CSV preview draft
PR](#44344). The draft PR
remains open as a reference and will be incrementally decomposed into
smaller, reviewable pieces like this one.

**Details:**  
- Adds a variable row height mode to the data table, enabling future
features that require rows of differing heights (such as CSV preview
and, eventually, database table views).
- No user-facing changes; this is an internal refactor to support
upcoming functionality.

**Thanks:**  
Big thanks to @Anthony-Eid for pairing sessions and guidance on how to
best structure and land these changes incrementally.

---

Release Notes:

- N/A (internal infrastructure change, no user impact)
…ghted (#46302)

Release Notes:

- Fixed deleted portions of hunks in the branch diff and text diff views
not being syntax-highlighted.
@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Jan 8, 2026
@zed-industries-bot
Copy link

zed-industries-bot commented Jan 8, 2026

Warnings
⚠️

This PR is missing release notes.

Please add a "Release Notes" section that describes the change:

Release Notes:

- Added/Fixed/Improved ...

If your change is not user-facing, you can use "N/A" for the entry:

Release Notes:

- N/A

Generated by 🚫 dangerJS against d44c4c1

- Fix cx.update() calls that incorrectly used ? operator (API change)
- Add visual_test_settings() that uses system UI font for UI elements
  and Menlo for code, instead of Courier for everything
- Fix erroneous ? operators on update_settings_file calls
@rtfeldman rtfeldman closed this Jan 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed The user has signed the Contributor License Agreement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants