Hide Always Allow option for agent skills edits (#58463)
Edits targeting the agent skills directory (project `.agents/skills/` and global `~/.agents/skills/`) always prompt regardless of permission settings, because they route through `authorize_always_prompt` which never consults the persisted `always_allow` rules. The permission prompt still rendered an "Always allow" button, though. Clicking it wrote an `always_allow` rule into settings that was then silently ignored on the next skills edit, so users were re-prompted every time despite having chosen "always allow" — exactly the confusion reported in AI-346. This keeps the always-prompt behavior intact and just hides the misleading "Always allow" / "Always deny" options for skills, leaving once-only Allow/Deny. Applies to all mutating skills tools: `edit_file`, `write_file`, `copy_path`, `move_path`, `delete_path`, and `create_directory`. Implementation: - New `ToolPermissionScope::AgentSkills`, set via `ToolPermissionContext::for_agent_skills()`. - `build_permission_options` returns flat once-only Allow/Deny for that scope (mirrors the existing `SymlinkTarget` handling). - Wired into both authorization entry points (`authorize_file_edit` and `authorize_with_sensitive_settings`). - Tests assert no `AllowAlways` option is offered for each affected tool. Settings-file prompts (`.zed/settings.json`, global config) are intentionally left unchanged. Closes AI-346 Release Notes: - Removed the misleading "Always allow" button from agent permission prompts for edits to the agent skills directory, since those edits always require confirmation.
This commit is contained in:
1 parent
4f844b5684
commit
da7945aeae
8 files changed
+92
-4
No files matched your search
@@ -852,6 +852,7 @@ pub struct ToolPermissionContext {
|
||||
pub enum ToolPermissionScope {
|
||||
ToolInput,
|
||||
SymlinkTarget,
|
||||
AgentSkills,
|
||||
}
|
||||
|
||||
impl ToolPermissionContext {
|
||||
@@ -871,6 +872,11 @@ impl ToolPermissionContext {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn for_agent_skills(mut self) -> Self {
|
||||
self.scope = ToolPermissionScope::AgentSkills;
|
||||
self
|
||||
}
|
||||
|
||||
/// Builds the permission options for this tool context.
|
||||
///
|
||||
/// This is the canonical source for permission option generation.
|
||||
@@ -916,6 +922,22 @@ impl ToolPermissionContext {
|
||||
]);
|
||||
}
|
||||
|
||||
// Skills always prompt, so offer only once-only allow/deny.
|
||||
if self.scope == ToolPermissionScope::AgentSkills {
|
||||
return acp_thread::PermissionOptions::Flat(vec![
|
||||
acp::PermissionOption::new(
|
||||
acp::PermissionOptionId::new("allow"),
|
||||
"Allow",
|
||||
acp::PermissionOptionKind::AllowOnce,
|
||||
),
|
||||
acp::PermissionOption::new(
|
||||
acp::PermissionOptionId::new("deny"),
|
||||
"Deny",
|
||||
acp::PermissionOptionKind::RejectOnce,
|
||||
),
|
||||
]);
|
||||
}
|
||||
|
||||
// Check if the user's shell supports POSIX-like command chaining.
|
||||
// See the doc comment above for the full explanation of why this is needed.
|
||||
let shell_supports_always_allow = if tool_name == TerminalTool::NAME {
|
||||
|
||||
@@ -328,6 +328,13 @@ mod tests {
|
||||
title.contains("agent skills"),
|
||||
"Authorization title should mention agent skills, got: {title}",
|
||||
);
|
||||
assert!(
|
||||
auth.options
|
||||
.first_option_of_kind(acp::PermissionOptionKind::AllowAlways)
|
||||
.is_none(),
|
||||
"agent skills prompt must not offer an \"Always allow\" option: {:?}",
|
||||
auth.options,
|
||||
);
|
||||
auth.response
|
||||
.send(acp_thread::SelectedPermissionOutcome::new(
|
||||
acp::PermissionOptionId::new("allow"),
|
||||
@@ -387,6 +394,13 @@ mod tests {
|
||||
title.contains("agent skills"),
|
||||
"Authorization title should mention agent skills, got: {title}",
|
||||
);
|
||||
assert!(
|
||||
auth.options
|
||||
.first_option_of_kind(acp::PermissionOptionKind::AllowAlways)
|
||||
.is_none(),
|
||||
"agent skills prompt must not offer an \"Always allow\" option: {:?}",
|
||||
auth.options,
|
||||
);
|
||||
auth.response
|
||||
.send(acp_thread::SelectedPermissionOutcome::new(
|
||||
acp::PermissionOptionId::new("allow"),
|
||||
|
||||
@@ -243,6 +243,13 @@ mod tests {
|
||||
title.contains("agent skills"),
|
||||
"Authorization title should mention agent skills, got: {title}",
|
||||
);
|
||||
assert!(
|
||||
auth.options
|
||||
.first_option_of_kind(acp::PermissionOptionKind::AllowAlways)
|
||||
.is_none(),
|
||||
"agent skills prompt must not offer an \"Always allow\" option: {:?}",
|
||||
auth.options,
|
||||
);
|
||||
auth.response
|
||||
.send(acp_thread::SelectedPermissionOutcome::new(
|
||||
acp::PermissionOptionId::new("allow"),
|
||||
|
||||
@@ -329,6 +329,13 @@ mod tests {
|
||||
title.contains("agent skills"),
|
||||
"Authorization title should mention agent skills, got: {title}",
|
||||
);
|
||||
assert!(
|
||||
auth.options
|
||||
.first_option_of_kind(acp::PermissionOptionKind::AllowAlways)
|
||||
.is_none(),
|
||||
"agent skills prompt must not offer an \"Always allow\" option: {:?}",
|
||||
auth.options,
|
||||
);
|
||||
auth.response
|
||||
.send(acp_thread::SelectedPermissionOutcome::new(
|
||||
acp::PermissionOptionId::new("allow"),
|
||||
|
||||
@@ -1320,6 +1320,20 @@ mod tests {
|
||||
event.tool_call.fields.title,
|
||||
Some("Edit `root/.agents/skills/my-skill/SKILL.md` (agent skills)".into())
|
||||
);
|
||||
// Skills always prompt, so no "Always allow" option is offered.
|
||||
assert!(
|
||||
event
|
||||
.options
|
||||
.first_option_of_kind(acp::PermissionOptionKind::AllowAlways)
|
||||
.is_none(),
|
||||
"agent skills prompt must not offer an \"Always allow\" option: {:?}",
|
||||
event.options,
|
||||
);
|
||||
assert!(
|
||||
matches!(event.options, acp_thread::PermissionOptions::Flat(_)),
|
||||
"agent skills prompt should use flat allow/deny options: {:?}",
|
||||
event.options,
|
||||
);
|
||||
|
||||
// 5.6: The global .agents/skills directory is sensitive — still prompts
|
||||
let global_skill_path = agent_skills::global_skills_dir()
|
||||
|
||||
@@ -354,6 +354,13 @@ mod tests {
|
||||
title.contains("agent skills"),
|
||||
"Authorization title should mention agent skills, got: {title}",
|
||||
);
|
||||
assert!(
|
||||
auth.options
|
||||
.first_option_of_kind(acp::PermissionOptionKind::AllowAlways)
|
||||
.is_none(),
|
||||
"agent skills prompt must not offer an \"Always allow\" option: {:?}",
|
||||
auth.options,
|
||||
);
|
||||
auth.response
|
||||
.send(acp_thread::SelectedPermissionOutcome::new(
|
||||
acp::PermissionOptionId::new("allow"),
|
||||
@@ -413,6 +420,13 @@ mod tests {
|
||||
title.contains("agent skills"),
|
||||
"Authorization title should mention agent skills, got: {title}",
|
||||
);
|
||||
assert!(
|
||||
auth.options
|
||||
.first_option_of_kind(acp::PermissionOptionKind::AllowAlways)
|
||||
.is_none(),
|
||||
"agent skills prompt must not offer an \"Always allow\" option: {:?}",
|
||||
auth.options,
|
||||
);
|
||||
auth.response
|
||||
.send(acp_thread::SelectedPermissionOutcome::new(
|
||||
acp::PermissionOptionId::new("allow"),
|
||||
|
||||
@@ -509,9 +509,11 @@ pub fn authorize_with_sensitive_settings(
|
||||
Some(SensitiveSettingsKind::Global) => {
|
||||
event_stream.authorize_always_prompt(format!("{title} (settings)"), context, cx)
|
||||
}
|
||||
Some(SensitiveSettingsKind::AgentSkills) => {
|
||||
event_stream.authorize_always_prompt(format!("{title} (agent skills)"), context, cx)
|
||||
}
|
||||
Some(SensitiveSettingsKind::AgentSkills) => event_stream.authorize_always_prompt(
|
||||
format!("{title} (agent skills)"),
|
||||
context.for_agent_skills(),
|
||||
cx,
|
||||
),
|
||||
None => event_stream.authorize(title, context, cx),
|
||||
}
|
||||
}
|
||||
@@ -761,7 +763,8 @@ pub fn authorize_file_edit(
|
||||
let context = ToolPermissionContext::new(
|
||||
&tool_name,
|
||||
vec![path_owned.to_string_lossy().to_string()],
|
||||
);
|
||||
)
|
||||
.for_agent_skills();
|
||||
event_stream.authorize_always_prompt(
|
||||
format!("{title} (agent skills)"),
|
||||
context,
|
||||
|
||||
@@ -373,6 +373,13 @@ mod tests {
|
||||
title.contains("agent skills"),
|
||||
"Authorization title should mention agent skills, got: {title}",
|
||||
);
|
||||
assert!(
|
||||
auth.options
|
||||
.first_option_of_kind(acp::PermissionOptionKind::AllowAlways)
|
||||
.is_none(),
|
||||
"agent skills prompt must not offer an \"Always allow\" option: {:?}",
|
||||
auth.options,
|
||||
);
|
||||
auth.response
|
||||
.send(acp_thread::SelectedPermissionOutcome::new(
|
||||
acp::PermissionOptionId::new("allow"),
|
||||
|
||||
Reference in new issue
Block a user