mirror of
https://github.com/zed-industries/zed.git
synced 2026-05-31 19:05:00 +07:00
Allow path tools to operate on global agent skills (#57760)
Extends the same `~/.agents/skills` special case that `create_directory`, `edit_file`, and `write_file` already use to the path tools, so the agent can copy or move skills into and out of the global skills folder and delete individual skills or skill resources beneath it. - `copy_path` now allows source and/or destination to be a descendant of `~/.agents/skills`, going through `fs::copy_recursive` directly when one side is outside the project. - `move_path` now allows source and/or destination to be a descendant of `~/.agents/skills`, going through `fs.rename` directly when one side is outside the project. Moving the `~/.agents/skills` root itself is rejected. - `delete_path` now allows deleting any file or directory beneath `~/.agents/skills`, going through `fs.remove_dir` / `fs.remove_file` directly. Deleting the `~/.agents/skills` root itself is rejected. - All three tools still always prompt for approval on agent-skill paths, even when default tool permissions are set to allow. - Added shared helpers in `tool_permissions.rs` for resolving global skill descendants and rejecting operations on the skills root where needed. - Added tests covering copying and moving skills in both directions, deleting a global skill directory/file, and rejecting deletion of the skills root. Release Notes: - Agent can now copy or move skills into and out of `~/.agents/skills` and delete individual skills, with an explicit confirmation prompt for each operation
This commit is contained in:
parent
a55e3e99f5
commit
d2cbf930f7
4 changed files with 623 additions and 2 deletions
|
|
@ -1,5 +1,6 @@
|
|||
use super::tool_permissions::{
|
||||
authorize_symlink_escapes, canonicalize_worktree_roots, collect_symlink_escapes,
|
||||
resolve_creatable_global_skill_descendant_path, resolve_global_skill_descendant_path,
|
||||
sensitive_settings_kind,
|
||||
};
|
||||
use crate::{
|
||||
|
|
@ -23,6 +24,7 @@ use util::markdown::MarkdownInlineCode;
|
|||
///
|
||||
/// This tool should be used when it's desirable to create a copy of a file or directory without modifying the original.
|
||||
/// It's much more efficient than doing this by separately reading and then writing the file or directory's contents, so this tool should be preferred over that approach whenever copying is the goal.
|
||||
/// The only supported paths outside the project are descendants of `~/.agents/skills`, for global agent skills.
|
||||
#[derive(Debug, Serialize, Deserialize, JsonSchema)]
|
||||
pub struct CopyPathToolInput {
|
||||
/// The source path of the file or directory to copy.
|
||||
|
|
@ -100,6 +102,15 @@ impl AgentTool for CopyPathTool {
|
|||
let fs = project.read_with(cx, |project, _cx| project.fs().clone());
|
||||
let canonical_roots = canonicalize_worktree_roots(&project, &fs, cx).await;
|
||||
|
||||
let global_source_path =
|
||||
resolve_global_skill_descendant_path(Path::new(&input.source_path), fs.as_ref())
|
||||
.await;
|
||||
let global_destination_path = resolve_creatable_global_skill_descendant_path(
|
||||
Path::new(&input.destination_path),
|
||||
fs.as_ref(),
|
||||
)
|
||||
.await;
|
||||
|
||||
let symlink_escapes: Vec<(&str, std::path::PathBuf)> =
|
||||
project.read_with(cx, |project, cx| {
|
||||
collect_symlink_escapes(
|
||||
|
|
@ -160,6 +171,63 @@ impl AgentTool for CopyPathTool {
|
|||
authorize.await.map_err(|e| e.to_string())?;
|
||||
}
|
||||
|
||||
if global_source_path.is_some() || global_destination_path.is_some() {
|
||||
let source_path = if let Some(global_source_path) = global_source_path {
|
||||
global_source_path
|
||||
} else {
|
||||
project.read_with(cx, |project, cx| {
|
||||
let project_path = project.find_project_path(&input.source_path, cx).ok_or_else(|| {
|
||||
format!("Source path {} was not found in the project.", input.source_path)
|
||||
})?;
|
||||
project.entry_for_path(&project_path, cx).ok_or_else(|| {
|
||||
format!("Source path {} was not found in the project.", input.source_path)
|
||||
})?;
|
||||
project.absolute_path(&project_path, cx).ok_or_else(|| {
|
||||
format!("Source path {} could not be resolved.", input.source_path)
|
||||
})
|
||||
})?
|
||||
};
|
||||
|
||||
let destination_path = if let Some(global_destination_path) = global_destination_path
|
||||
{
|
||||
global_destination_path
|
||||
} else {
|
||||
project.read_with(cx, |project, cx| {
|
||||
let project_path = project.find_project_path(&input.destination_path, cx).ok_or_else(|| {
|
||||
format!(
|
||||
"Destination path {} was outside the project.",
|
||||
input.destination_path
|
||||
)
|
||||
})?;
|
||||
project.absolute_path(&project_path, cx).ok_or_else(|| {
|
||||
format!(
|
||||
"Destination path {} could not be resolved.",
|
||||
input.destination_path
|
||||
)
|
||||
})
|
||||
})?
|
||||
};
|
||||
|
||||
futures::select! {
|
||||
result = fs::copy_recursive(
|
||||
fs.as_ref(),
|
||||
&source_path,
|
||||
&destination_path,
|
||||
fs::CopyOptions::default(),
|
||||
).fuse() => {
|
||||
result.map_err(|e| format!("Copying {} to {}: {e}", input.source_path, input.destination_path))?;
|
||||
}
|
||||
_ = event_stream.cancelled_by_user().fuse() => {
|
||||
return Err("Copy cancelled by user".to_string());
|
||||
}
|
||||
}
|
||||
|
||||
return Ok(format!(
|
||||
"Copied {} to {}",
|
||||
input.source_path, input.destination_path
|
||||
));
|
||||
}
|
||||
|
||||
let copy_task = project.update(cx, |project, cx| {
|
||||
match project
|
||||
.find_project_path(&input.source_path, cx)
|
||||
|
|
@ -222,6 +290,124 @@ mod tests {
|
|||
});
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_copy_path_global_skill_directory_to_project(cx: &mut TestAppContext) {
|
||||
init_test(cx);
|
||||
|
||||
let fs = FakeFs::new(cx.executor());
|
||||
fs.insert_tree(path!("/root/project"), json!({})).await;
|
||||
let skill_dir = agent_skills::global_skills_dir().join("my-skill");
|
||||
fs.insert_tree(&skill_dir, json!({ "SKILL.md": "content" }))
|
||||
.await;
|
||||
let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
|
||||
cx.executor().run_until_parked();
|
||||
|
||||
let tool = Arc::new(CopyPathTool::new(project));
|
||||
let input_path = PathBuf::from("~")
|
||||
.join(".agents")
|
||||
.join("skills")
|
||||
.join("my-skill")
|
||||
.to_string_lossy()
|
||||
.into_owned();
|
||||
|
||||
let (event_stream, mut event_rx) = ToolCallEventStream::test();
|
||||
let task = cx.update(|cx| {
|
||||
tool.run(
|
||||
ToolInput::resolved(CopyPathToolInput {
|
||||
source_path: input_path,
|
||||
destination_path: path!("/root/project/my-skill").to_string(),
|
||||
}),
|
||||
event_stream,
|
||||
cx,
|
||||
)
|
||||
});
|
||||
|
||||
let auth = event_rx.expect_authorization().await;
|
||||
let title = auth.tool_call.fields.title.as_deref().unwrap_or("");
|
||||
assert!(
|
||||
title.contains("agent skills"),
|
||||
"Authorization title should mention agent skills, got: {title}",
|
||||
);
|
||||
auth.response
|
||||
.send(acp_thread::SelectedPermissionOutcome::new(
|
||||
acp::PermissionOptionId::new("allow"),
|
||||
acp::PermissionOptionKind::AllowOnce,
|
||||
))
|
||||
.expect("authorization response should send");
|
||||
|
||||
let result = task.await;
|
||||
assert!(result.is_ok(), "should copy after approval: {result:?}");
|
||||
assert!(fs.is_dir(&skill_dir).await);
|
||||
assert_eq!(
|
||||
fs.load(path!("/root/project/my-skill/SKILL.md").as_ref())
|
||||
.await
|
||||
.unwrap(),
|
||||
"content"
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_copy_path_project_directory_to_global_skill_directory(cx: &mut TestAppContext) {
|
||||
init_test(cx);
|
||||
|
||||
let fs = FakeFs::new(cx.executor());
|
||||
fs.insert_tree(
|
||||
path!("/root/project"),
|
||||
json!({ "exported-skill": { "SKILL.md": "content" } }),
|
||||
)
|
||||
.await;
|
||||
let skills_dir = agent_skills::global_skills_dir();
|
||||
fs.create_dir(&skills_dir).await.unwrap();
|
||||
let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
|
||||
cx.executor().run_until_parked();
|
||||
|
||||
let tool = Arc::new(CopyPathTool::new(project));
|
||||
let destination_path = PathBuf::from("~")
|
||||
.join(".agents")
|
||||
.join("skills")
|
||||
.join("exported-skill")
|
||||
.to_string_lossy()
|
||||
.into_owned();
|
||||
|
||||
let (event_stream, mut event_rx) = ToolCallEventStream::test();
|
||||
let task = cx.update(|cx| {
|
||||
tool.run(
|
||||
ToolInput::resolved(CopyPathToolInput {
|
||||
source_path: path!("/root/project/exported-skill").to_string(),
|
||||
destination_path,
|
||||
}),
|
||||
event_stream,
|
||||
cx,
|
||||
)
|
||||
});
|
||||
|
||||
let auth = event_rx.expect_authorization().await;
|
||||
let title = auth.tool_call.fields.title.as_deref().unwrap_or("");
|
||||
assert!(
|
||||
title.contains("agent skills"),
|
||||
"Authorization title should mention agent skills, got: {title}",
|
||||
);
|
||||
auth.response
|
||||
.send(acp_thread::SelectedPermissionOutcome::new(
|
||||
acp::PermissionOptionId::new("allow"),
|
||||
acp::PermissionOptionKind::AllowOnce,
|
||||
))
|
||||
.expect("authorization response should send");
|
||||
|
||||
let result = task.await;
|
||||
assert!(result.is_ok(), "should copy after approval: {result:?}");
|
||||
assert!(
|
||||
fs.is_dir(path!("/root/project/exported-skill").as_ref())
|
||||
.await
|
||||
);
|
||||
assert_eq!(
|
||||
fs.load(skills_dir.join("exported-skill").join("SKILL.md").as_ref())
|
||||
.await
|
||||
.unwrap(),
|
||||
"content"
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_copy_path_symlink_escape_source_requests_authorization(cx: &mut TestAppContext) {
|
||||
init_test(cx);
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
use super::tool_permissions::{
|
||||
authorize_symlink_access, canonicalize_worktree_roots, detect_symlink_escape,
|
||||
sensitive_settings_kind,
|
||||
resolve_global_skill_descendant_path, resolves_to_global_skills_dir, sensitive_settings_kind,
|
||||
};
|
||||
use crate::{
|
||||
AgentTool, ToolCallEventStream, ToolInput, ToolPermissionDecision,
|
||||
|
|
@ -20,6 +20,8 @@ use std::sync::Arc;
|
|||
use util::markdown::MarkdownInlineCode;
|
||||
|
||||
/// Deletes the file or directory (and the directory's contents, recursively) at the specified path in the project, and returns confirmation of the deletion.
|
||||
///
|
||||
/// The only supported paths outside the project are descendants of `~/.agents/skills`, for global agent skills.
|
||||
#[derive(Debug, Serialize, Deserialize, JsonSchema)]
|
||||
pub struct DeletePathToolInput {
|
||||
/// The path of the file or directory to delete.
|
||||
|
|
@ -95,6 +97,16 @@ impl AgentTool for DeletePathTool {
|
|||
let fs = project.read_with(cx, |project, _cx| project.fs().clone());
|
||||
let canonical_roots = canonicalize_worktree_roots(&project, &fs, cx).await;
|
||||
|
||||
if resolves_to_global_skills_dir(Path::new(&path), fs.as_ref()).await {
|
||||
return Err(
|
||||
"Cannot delete the global agent skills directory itself. Delete a skill directory or file beneath it instead."
|
||||
.to_string(),
|
||||
);
|
||||
}
|
||||
|
||||
let global_skill_path =
|
||||
resolve_global_skill_descendant_path(Path::new(&path), fs.as_ref()).await;
|
||||
|
||||
let symlink_escape_target = project.read_with(cx, |project, cx| {
|
||||
detect_symlink_escape(project, &path, &canonical_roots, cx)
|
||||
.map(|(_, target)| target)
|
||||
|
|
@ -147,6 +159,38 @@ impl AgentTool for DeletePathTool {
|
|||
authorize.await.map_err(|e| e.to_string())?;
|
||||
}
|
||||
|
||||
if let Some(global_skill_path) = global_skill_path {
|
||||
let metadata = fs
|
||||
.metadata(&global_skill_path)
|
||||
.await
|
||||
.map_err(|e| format!("Deleting {path}: {e}"))?
|
||||
.ok_or_else(|| format!("Deleting {path}: path not found"))?;
|
||||
|
||||
futures::select! {
|
||||
result = async {
|
||||
if metadata.is_dir {
|
||||
fs.remove_dir(
|
||||
&global_skill_path,
|
||||
fs::RemoveOptions {
|
||||
recursive: true,
|
||||
..fs::RemoveOptions::default()
|
||||
},
|
||||
)
|
||||
.await
|
||||
} else {
|
||||
fs.remove_file(&global_skill_path, fs::RemoveOptions::default()).await
|
||||
}
|
||||
}.fuse() => {
|
||||
result.map_err(|e| format!("Deleting {path}: {e}"))?;
|
||||
}
|
||||
_ = event_stream.cancelled_by_user().fuse() => {
|
||||
return Err("Delete cancelled by user".to_string());
|
||||
}
|
||||
}
|
||||
|
||||
return Ok(format!("Deleted {path}"));
|
||||
}
|
||||
|
||||
let (project_path, worktree_snapshot) = project.read_with(cx, |project, cx| {
|
||||
let project_path = project.find_project_path(&path, cx).ok_or_else(|| {
|
||||
format!("Couldn't delete {path} because that path isn't in this project.")
|
||||
|
|
@ -248,6 +292,145 @@ mod tests {
|
|||
});
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_delete_path_global_skill_directory(cx: &mut TestAppContext) {
|
||||
init_test(cx);
|
||||
|
||||
let fs = FakeFs::new(cx.executor());
|
||||
fs.insert_tree(path!("/root/project"), json!({})).await;
|
||||
let skills_dir = agent_skills::global_skills_dir();
|
||||
let skill_dir = skills_dir.join("my-skill");
|
||||
fs.insert_tree(&skill_dir, json!({ "SKILL.md": "content" }))
|
||||
.await;
|
||||
let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
|
||||
cx.executor().run_until_parked();
|
||||
|
||||
let action_log = cx.new(|_| ActionLog::new(project.clone()));
|
||||
let tool = Arc::new(DeletePathTool::new(project, action_log));
|
||||
let input_path = PathBuf::from("~")
|
||||
.join(".agents")
|
||||
.join("skills")
|
||||
.join("my-skill")
|
||||
.to_string_lossy()
|
||||
.into_owned();
|
||||
|
||||
let (event_stream, mut event_rx) = ToolCallEventStream::test();
|
||||
let task = cx.update(|cx| {
|
||||
tool.run(
|
||||
ToolInput::resolved(DeletePathToolInput { path: input_path }),
|
||||
event_stream,
|
||||
cx,
|
||||
)
|
||||
});
|
||||
|
||||
let auth = event_rx.expect_authorization().await;
|
||||
let title = auth.tool_call.fields.title.as_deref().unwrap_or("");
|
||||
assert!(
|
||||
title.contains("agent skills"),
|
||||
"Authorization title should mention agent skills, got: {title}",
|
||||
);
|
||||
auth.response
|
||||
.send(acp_thread::SelectedPermissionOutcome::new(
|
||||
acp::PermissionOptionId::new("allow"),
|
||||
acp::PermissionOptionKind::AllowOnce,
|
||||
))
|
||||
.expect("authorization response should send");
|
||||
|
||||
let result = task.await;
|
||||
assert!(result.is_ok(), "should delete after approval: {result:?}");
|
||||
assert!(fs.is_dir(&skills_dir).await);
|
||||
assert!(!fs.is_dir(&skill_dir).await);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_delete_path_global_skill_file(cx: &mut TestAppContext) {
|
||||
init_test(cx);
|
||||
|
||||
let fs = FakeFs::new(cx.executor());
|
||||
fs.insert_tree(path!("/root/project"), json!({})).await;
|
||||
let skill_file = agent_skills::global_skills_dir()
|
||||
.join("my-skill")
|
||||
.join("references")
|
||||
.join("notes.md");
|
||||
fs.create_dir(skill_file.parent().unwrap()).await.unwrap();
|
||||
fs.insert_file(&skill_file, b"notes".to_vec()).await;
|
||||
let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
|
||||
cx.executor().run_until_parked();
|
||||
|
||||
let action_log = cx.new(|_| ActionLog::new(project.clone()));
|
||||
let tool = Arc::new(DeletePathTool::new(project, action_log));
|
||||
let input_path = PathBuf::from("~")
|
||||
.join(".agents")
|
||||
.join("skills")
|
||||
.join("my-skill")
|
||||
.join("references")
|
||||
.join("notes.md")
|
||||
.to_string_lossy()
|
||||
.into_owned();
|
||||
|
||||
let (event_stream, mut event_rx) = ToolCallEventStream::test();
|
||||
let task = cx.update(|cx| {
|
||||
tool.run(
|
||||
ToolInput::resolved(DeletePathToolInput { path: input_path }),
|
||||
event_stream,
|
||||
cx,
|
||||
)
|
||||
});
|
||||
|
||||
let auth = event_rx.expect_authorization().await;
|
||||
auth.response
|
||||
.send(acp_thread::SelectedPermissionOutcome::new(
|
||||
acp::PermissionOptionId::new("allow"),
|
||||
acp::PermissionOptionKind::AllowOnce,
|
||||
))
|
||||
.expect("authorization response should send");
|
||||
|
||||
let result = task.await;
|
||||
assert!(result.is_ok(), "should delete after approval: {result:?}");
|
||||
assert!(!fs.is_file(&skill_file).await);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_delete_path_rejects_global_skills_root(cx: &mut TestAppContext) {
|
||||
init_test(cx);
|
||||
|
||||
let fs = FakeFs::new(cx.executor());
|
||||
fs.insert_tree(path!("/root/project"), json!({})).await;
|
||||
let skills_dir = agent_skills::global_skills_dir();
|
||||
fs.create_dir(&skills_dir).await.unwrap();
|
||||
let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
|
||||
cx.executor().run_until_parked();
|
||||
|
||||
let action_log = cx.new(|_| ActionLog::new(project.clone()));
|
||||
let tool = Arc::new(DeletePathTool::new(project, action_log));
|
||||
let input_path = PathBuf::from("~")
|
||||
.join(".agents")
|
||||
.join("skills")
|
||||
.to_string_lossy()
|
||||
.into_owned();
|
||||
|
||||
let (event_stream, mut event_rx) = ToolCallEventStream::test();
|
||||
let result = cx
|
||||
.update(|cx| {
|
||||
tool.run(
|
||||
ToolInput::resolved(DeletePathToolInput { path: input_path }),
|
||||
event_stream,
|
||||
cx,
|
||||
)
|
||||
})
|
||||
.await;
|
||||
|
||||
assert!(result.is_err(), "should reject deleting skills root");
|
||||
assert!(fs.is_dir(&skills_dir).await);
|
||||
assert!(
|
||||
!matches!(
|
||||
event_rx.try_recv(),
|
||||
Ok(Ok(crate::ThreadEvent::ToolCallAuthorization(_)))
|
||||
),
|
||||
"Deleting the skills root should fail before requesting authorization",
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_delete_path_symlink_escape_requests_authorization(cx: &mut TestAppContext) {
|
||||
init_test(cx);
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
use super::tool_permissions::{
|
||||
authorize_symlink_escapes, canonicalize_worktree_roots, collect_symlink_escapes,
|
||||
sensitive_settings_kind,
|
||||
resolve_creatable_global_skill_descendant_path, resolve_global_skill_descendant_path,
|
||||
resolves_to_global_skills_dir, sensitive_settings_kind,
|
||||
};
|
||||
use crate::{
|
||||
AgentTool, ToolCallEventStream, ToolInput, ToolPermissionDecision,
|
||||
|
|
@ -22,6 +23,7 @@ use util::markdown::MarkdownInlineCode;
|
|||
/// If the source and destination directories are the same, but the filename is different, this performs a rename. Otherwise, it performs a move.
|
||||
///
|
||||
/// This tool should be used when it's desirable to move or rename a file or directory without changing its contents at all.
|
||||
/// The only supported paths outside the project are descendants of `~/.agents/skills`, for global agent skills.
|
||||
#[derive(Debug, Serialize, Deserialize, JsonSchema)]
|
||||
pub struct MovePathToolInput {
|
||||
/// The source path of the file or directory to move/rename.
|
||||
|
|
@ -116,6 +118,28 @@ impl AgentTool for MovePathTool {
|
|||
let fs = project.read_with(cx, |project, _cx| project.fs().clone());
|
||||
let canonical_roots = canonicalize_worktree_roots(&project, &fs, cx).await;
|
||||
|
||||
if resolves_to_global_skills_dir(Path::new(&input.source_path), fs.as_ref()).await
|
||||
|| resolves_to_global_skills_dir(
|
||||
Path::new(&input.destination_path),
|
||||
fs.as_ref(),
|
||||
)
|
||||
.await
|
||||
{
|
||||
return Err(
|
||||
"Cannot move the global agent skills directory itself. Move a skill directory or file beneath it instead."
|
||||
.to_string(),
|
||||
);
|
||||
}
|
||||
|
||||
let global_source_path =
|
||||
resolve_global_skill_descendant_path(Path::new(&input.source_path), fs.as_ref())
|
||||
.await;
|
||||
let global_destination_path = resolve_creatable_global_skill_descendant_path(
|
||||
Path::new(&input.destination_path),
|
||||
fs.as_ref(),
|
||||
)
|
||||
.await;
|
||||
|
||||
let symlink_escapes: Vec<(&str, std::path::PathBuf)> =
|
||||
project.read_with(cx, |project, cx| {
|
||||
collect_symlink_escapes(
|
||||
|
|
@ -176,6 +200,65 @@ impl AgentTool for MovePathTool {
|
|||
authorize.await.map_err(|e| e.to_string())?;
|
||||
}
|
||||
|
||||
if global_source_path.is_some() || global_destination_path.is_some() {
|
||||
let source_path = if let Some(global_source_path) = global_source_path {
|
||||
global_source_path
|
||||
} else {
|
||||
project.read_with(cx, |project, cx| {
|
||||
let project_path = project.find_project_path(&input.source_path, cx).ok_or_else(|| {
|
||||
format!("Source path {} was not found in the project.", input.source_path)
|
||||
})?;
|
||||
project.entry_for_path(&project_path, cx).ok_or_else(|| {
|
||||
format!("Source path {} was not found in the project.", input.source_path)
|
||||
})?;
|
||||
project.absolute_path(&project_path, cx).ok_or_else(|| {
|
||||
format!("Source path {} could not be resolved.", input.source_path)
|
||||
})
|
||||
})?
|
||||
};
|
||||
|
||||
let destination_path = if let Some(global_destination_path) = global_destination_path
|
||||
{
|
||||
global_destination_path
|
||||
} else {
|
||||
project.read_with(cx, |project, cx| {
|
||||
let project_path = project.find_project_path(&input.destination_path, cx).ok_or_else(|| {
|
||||
format!(
|
||||
"Destination path {} was outside the project.",
|
||||
input.destination_path
|
||||
)
|
||||
})?;
|
||||
project.absolute_path(&project_path, cx).ok_or_else(|| {
|
||||
format!(
|
||||
"Destination path {} could not be resolved.",
|
||||
input.destination_path
|
||||
)
|
||||
})
|
||||
})?
|
||||
};
|
||||
|
||||
futures::select! {
|
||||
result = fs.rename(
|
||||
&source_path,
|
||||
&destination_path,
|
||||
fs::RenameOptions {
|
||||
create_parents: true,
|
||||
..fs::RenameOptions::default()
|
||||
},
|
||||
).fuse() => {
|
||||
result.map_err(|e| format!("Moving {} to {}: {e}", input.source_path, input.destination_path))?;
|
||||
}
|
||||
_ = event_stream.cancelled_by_user().fuse() => {
|
||||
return Err("Move cancelled by user".to_string());
|
||||
}
|
||||
}
|
||||
|
||||
return Ok(format!(
|
||||
"Moved {} to {}",
|
||||
input.source_path, input.destination_path
|
||||
));
|
||||
}
|
||||
|
||||
let rename_task = project.update(cx, |project, cx| {
|
||||
match project
|
||||
.find_project_path(&input.source_path, cx)
|
||||
|
|
@ -232,6 +315,125 @@ mod tests {
|
|||
});
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_move_path_global_skill_directory_to_project(cx: &mut TestAppContext) {
|
||||
init_test(cx);
|
||||
|
||||
let fs = FakeFs::new(cx.executor());
|
||||
fs.insert_tree(path!("/root/project"), json!({})).await;
|
||||
let skill_dir = agent_skills::global_skills_dir().join("my-skill");
|
||||
fs.insert_tree(&skill_dir, json!({ "SKILL.md": "content" }))
|
||||
.await;
|
||||
let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
|
||||
cx.executor().run_until_parked();
|
||||
|
||||
let tool = Arc::new(MovePathTool::new(project));
|
||||
let input_path = PathBuf::from("~")
|
||||
.join(".agents")
|
||||
.join("skills")
|
||||
.join("my-skill")
|
||||
.to_string_lossy()
|
||||
.into_owned();
|
||||
let destination_path = path!("/root/project/my-skill").to_string();
|
||||
|
||||
let (event_stream, mut event_rx) = ToolCallEventStream::test();
|
||||
let task = cx.update(|cx| {
|
||||
tool.run(
|
||||
ToolInput::resolved(MovePathToolInput {
|
||||
source_path: input_path,
|
||||
destination_path,
|
||||
}),
|
||||
event_stream,
|
||||
cx,
|
||||
)
|
||||
});
|
||||
|
||||
let auth = event_rx.expect_authorization().await;
|
||||
let title = auth.tool_call.fields.title.as_deref().unwrap_or("");
|
||||
assert!(
|
||||
title.contains("agent skills"),
|
||||
"Authorization title should mention agent skills, got: {title}",
|
||||
);
|
||||
auth.response
|
||||
.send(acp_thread::SelectedPermissionOutcome::new(
|
||||
acp::PermissionOptionId::new("allow"),
|
||||
acp::PermissionOptionKind::AllowOnce,
|
||||
))
|
||||
.expect("authorization response should send");
|
||||
|
||||
let result = task.await;
|
||||
assert!(result.is_ok(), "should move after approval: {result:?}");
|
||||
assert!(!fs.is_dir(&skill_dir).await);
|
||||
assert_eq!(
|
||||
fs.load(path!("/root/project/my-skill/SKILL.md").as_ref())
|
||||
.await
|
||||
.unwrap(),
|
||||
"content"
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_move_path_project_directory_to_global_skill_directory(cx: &mut TestAppContext) {
|
||||
init_test(cx);
|
||||
|
||||
let fs = FakeFs::new(cx.executor());
|
||||
fs.insert_tree(
|
||||
path!("/root/project"),
|
||||
json!({ "exported-skill": { "SKILL.md": "content" } }),
|
||||
)
|
||||
.await;
|
||||
let skills_dir = agent_skills::global_skills_dir();
|
||||
fs.create_dir(&skills_dir).await.unwrap();
|
||||
let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
|
||||
cx.executor().run_until_parked();
|
||||
|
||||
let tool = Arc::new(MovePathTool::new(project));
|
||||
let destination_path = PathBuf::from("~")
|
||||
.join(".agents")
|
||||
.join("skills")
|
||||
.join("exported-skill")
|
||||
.to_string_lossy()
|
||||
.into_owned();
|
||||
|
||||
let (event_stream, mut event_rx) = ToolCallEventStream::test();
|
||||
let task = cx.update(|cx| {
|
||||
tool.run(
|
||||
ToolInput::resolved(MovePathToolInput {
|
||||
source_path: path!("/root/project/exported-skill").to_string(),
|
||||
destination_path,
|
||||
}),
|
||||
event_stream,
|
||||
cx,
|
||||
)
|
||||
});
|
||||
|
||||
let auth = event_rx.expect_authorization().await;
|
||||
let title = auth.tool_call.fields.title.as_deref().unwrap_or("");
|
||||
assert!(
|
||||
title.contains("agent skills"),
|
||||
"Authorization title should mention agent skills, got: {title}",
|
||||
);
|
||||
auth.response
|
||||
.send(acp_thread::SelectedPermissionOutcome::new(
|
||||
acp::PermissionOptionId::new("allow"),
|
||||
acp::PermissionOptionKind::AllowOnce,
|
||||
))
|
||||
.expect("authorization response should send");
|
||||
|
||||
let result = task.await;
|
||||
assert!(result.is_ok(), "should move after approval: {result:?}");
|
||||
assert!(
|
||||
!fs.is_dir(path!("/root/project/exported-skill").as_ref())
|
||||
.await
|
||||
);
|
||||
assert_eq!(
|
||||
fs.load(skills_dir.join("exported-skill").join("SKILL.md").as_ref())
|
||||
.await
|
||||
.unwrap(),
|
||||
"content"
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_move_path_symlink_escape_source_requests_authorization(cx: &mut TestAppContext) {
|
||||
init_test(cx);
|
||||
|
|
|
|||
|
|
@ -199,6 +199,56 @@ pub async fn resolve_creatable_global_skill_path(path: &Path, fs: &dyn Fs) -> Op
|
|||
}
|
||||
}
|
||||
|
||||
fn is_strict_descendant(path: &Path, ancestor: &Path) -> bool {
|
||||
path != ancestor && path.starts_with(ancestor)
|
||||
}
|
||||
|
||||
/// Returns whether `path` resolves to the global agent skills directory itself.
|
||||
///
|
||||
/// This is used by destructive tools to reject operations targeting the root
|
||||
/// `~/.agents/skills` directory while still allowing operations on individual
|
||||
/// skills or resources beneath it.
|
||||
pub async fn resolves_to_global_skills_dir(path: &Path, fs: &dyn Fs) -> bool {
|
||||
let Some(normalized_path) = resolve_lexical_global_skill_path(path) else {
|
||||
return false;
|
||||
};
|
||||
let Some(canonical_path) = canonicalize_with_ancestors(&normalized_path, fs).await else {
|
||||
return false;
|
||||
};
|
||||
let Some(canonical_skills_dir) = canonical_global_skills_dir(fs).await else {
|
||||
return false;
|
||||
};
|
||||
|
||||
canonical_path == canonical_skills_dir
|
||||
}
|
||||
|
||||
/// Filters a previously-resolved global skills path so that callers which
|
||||
/// must never act on `~/.agents/skills` itself (move, delete) only see paths
|
||||
/// that point strictly below the skills root.
|
||||
async fn restrict_to_skill_descendant(
|
||||
canonical_path: Option<PathBuf>,
|
||||
fs: &dyn Fs,
|
||||
) -> Option<PathBuf> {
|
||||
let canonical_path = canonical_path?;
|
||||
let canonical_skills_dir = canonical_global_skills_dir(fs).await?;
|
||||
is_strict_descendant(&canonical_path, &canonical_skills_dir).then_some(canonical_path)
|
||||
}
|
||||
|
||||
/// Like [`resolve_global_skill_path`], but only succeeds for paths strictly
|
||||
/// below `~/.agents/skills`, not the skills directory itself.
|
||||
pub async fn resolve_global_skill_descendant_path(path: &Path, fs: &dyn Fs) -> Option<PathBuf> {
|
||||
restrict_to_skill_descendant(resolve_global_skill_path(path, fs).await, fs).await
|
||||
}
|
||||
|
||||
/// Like [`resolve_creatable_global_skill_path`], but only succeeds for paths
|
||||
/// strictly below `~/.agents/skills`, not the skills directory itself.
|
||||
pub async fn resolve_creatable_global_skill_descendant_path(
|
||||
path: &Path,
|
||||
fs: &dyn Fs,
|
||||
) -> Option<PathBuf> {
|
||||
restrict_to_skill_descendant(resolve_creatable_global_skill_path(path, fs).await, fs).await
|
||||
}
|
||||
|
||||
/// Returns the kind of sensitive settings or agent skills location this path targets, if any:
|
||||
/// either inside a `.zed/` local-settings directory, inside `.agents/skills/`, or inside
|
||||
/// the global config dir.
|
||||
|
|
|
|||
Loading…
Reference in a new issue