From 37c19c17bf7d54217485e07ca4deed6e1e20358a Mon Sep 17 00:00:00 2001 From: MartinYe1234 <52641447+MartinYe1234@users.noreply.github.com> Date: Tue, 19 May 2026 19:40:57 -0700 Subject: [PATCH] Replace Rules UI with Skills creation UI (#56924) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the legacy Rules creation UI with a focused "New Skill" creation window when the `SkillsFeatureFlag` is on. The new `skills_library` crate provides a single-skill form with: - Name, Description, and Body fields (placeholders double as labels) - Live validation matching the SKILL.md spec (name: 1–64 lowercase + digits + hyphens, no leading/trailing hyphen; description: non-empty, ≤1024 chars) - A scope dropdown listing every local worktree in the originating workspace plus "Global" - An Optional Parameters card with the `disable-model-invocation` checkbox - Save writes `/.agents/skills//SKILL.md` via `serde_yaml_ng` (YAML-safe escaping), refuses to overwrite existing skills, shows a success toast in the originating workspace, then closes the window The "Rules" entry in the agent panel triple-dot menu is renamed to "Skills" when the flag is on. The existing `cmd-alt-l` keyboard shortcut still works: `AgentPanel::deploy_rules_library` reroutes to `deploy_skills_library` when the flag is on, so any persisted keymaps and the menu's automatic shortcut lookup keep functioning without changes to the default keymap files. The old `rules_library` crate is intentionally left in place for this PR — once the flag rolls out and the few remaining `OpenRulesLibrary { prompt_to_select }` call sites (in thread_view link handlers and @-mention crease) are migrated to an "open existing skill" flow, a follow-up can delete the crate entirely. 15 unit tests cover the validation rules, frontmatter formatting (including YAML-special-character round-tripping), and disk write behavior (creates directory, refuses overwrite). Closes AI-247 Release Notes: - Added a new Skills creation UI that replaces the old Rules library when the Skills feature flag is enabled. --------- Co-authored-by: Danilo Leal --- Cargo.lock | 25 + Cargo.toml | 2 + assets/keymaps/default-linux.json | 16 + assets/keymaps/default-macos.json | 18 + assets/keymaps/default-windows.json | 18 + assets/keymaps/vim.json | 20 +- crates/agent_skills/agent_skills.rs | 167 ++- crates/agent_ui/Cargo.toml | 1 + crates/agent_ui/src/agent_panel.rs | 188 +-- crates/agent_ui/src/agent_ui.rs | 4 + crates/settings_ui/src/pages/skills_setup.rs | 6 +- crates/skill_creator/Cargo.toml | 36 + crates/skill_creator/LICENSE-GPL | 1 + crates/skill_creator/src/skill_creator.rs | 1073 ++++++++++++++++++ crates/zed/src/zed.rs | 1 + crates/zed_actions/src/lib.rs | 2 + 16 files changed, 1466 insertions(+), 112 deletions(-) create mode 100644 crates/skill_creator/Cargo.toml create mode 120000 crates/skill_creator/LICENSE-GPL create mode 100644 crates/skill_creator/src/skill_creator.rs diff --git a/Cargo.lock b/Cargo.lock index 7d1d803623..877ae7160f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -438,6 +438,7 @@ dependencies = [ "serde_json", "serde_json_lenient", "settings", + "skill_creator", "streaming_diff", "task", "telemetry", @@ -16532,6 +16533,30 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "56199f7ddabf13fe5074ce809e7d3f42b42ae711800501b5b16ea82ad029c39d" +[[package]] +name = "skill_creator" +version = "0.1.0" +dependencies = [ + "agent_skills", + "anyhow", + "editor", + "fs", + "gpui", + "language", + "menu", + "platform_title_bar", + "release_channel", + "serde_json", + "serde_yaml_ng", + "settings", + "theme_settings", + "ui", + "ui_input", + "util", + "workspace", + "worktree", +] + [[package]] name = "skrifa" version = "0.37.0" diff --git a/Cargo.toml b/Cargo.toml index 7e113da2f7..619cc83a08 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -172,6 +172,7 @@ members = [ "crates/rope", "crates/rpc", "crates/rules_library", + "crates/skill_creator", "crates/scheduler", "crates/schema_generator", "crates/search", @@ -432,6 +433,7 @@ rodio = { git = "https://github.com/RustAudio/rodio", rev = "e50e726ddd0292f6ef9 rope = { path = "crates/rope" } rpc = { path = "crates/rpc" } rules_library = { path = "crates/rules_library" } +skill_creator = { path = "crates/skill_creator" } scheduler = { path = "crates/scheduler" } search = { path = "crates/search" } session = { path = "crates/session" } diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index 0988611644..349c980e8b 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -1558,4 +1558,20 @@ "shift-tab": "git_graph::FocusPreviousTabStop", }, }, + { + "context": "SkillCreator", + "bindings": { + "ctrl-w": "workspace::CloseWindow", + "tab": "skill_creator::FocusNextField", + "shift-tab": "skill_creator::FocusPreviousField", + }, + }, + { + "context": "SkillCreator > Editor", + "bindings": { + "ctrl-w": "workspace::CloseWindow", + "tab": "skill_creator::FocusNextField", + "shift-tab": "skill_creator::FocusPreviousField", + }, + }, ] diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index 4c9aa4c0bc..23fd201b0d 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -1651,4 +1651,22 @@ "shift-tab": "git_graph::FocusPreviousTabStop", }, }, + { + "context": "SkillCreator", + "use_key_equivalents": true, + "bindings": { + "cmd-w": "workspace::CloseWindow", + "tab": "skill_creator::FocusNextField", + "shift-tab": "skill_creator::FocusPreviousField", + }, + }, + { + "context": "SkillCreator > Editor", + "use_key_equivalents": true, + "bindings": { + "cmd-w": "workspace::CloseWindow", + "tab": "skill_creator::FocusNextField", + "shift-tab": "skill_creator::FocusPreviousField", + }, + }, ] diff --git a/assets/keymaps/default-windows.json b/assets/keymaps/default-windows.json index ea11c7f423..3eece808cc 100644 --- a/assets/keymaps/default-windows.json +++ b/assets/keymaps/default-windows.json @@ -1577,4 +1577,22 @@ "shift-tab": "git_graph::FocusPreviousTabStop", }, }, + { + "context": "SkillCreator", + "use_key_equivalents": true, + "bindings": { + "ctrl-w": "workspace::CloseWindow", + "tab": "skill_creator::FocusNextField", + "shift-tab": "skill_creator::FocusPreviousField", + }, + }, + { + "context": "SkillCreator > Editor", + "use_key_equivalents": true, + "bindings": { + "ctrl-w": "workspace::CloseWindow", + "tab": "skill_creator::FocusNextField", + "shift-tab": "skill_creator::FocusPreviousField", + }, + }, ] diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index 396c6e4085..bcb114acfa 100644 --- a/assets/keymaps/vim.json +++ b/assets/keymaps/vim.json @@ -943,7 +943,7 @@ "space w j": "workspace::ActivatePaneDown", "space w k": "workspace::ActivatePaneUp", "space w l": "workspace::ActivatePaneRight", - "space w q": "pane::CloseActiveItem", + "space w q": "pane::CloseActiveItem", }, }, { @@ -1056,8 +1056,8 @@ "ctrl-d": "git_graph::ScrollDown", "ctrl-u": "git_graph::ScrollUp", "shift-g": "menu::SelectLast", - "g g": "menu::SelectFirst" - } + "g g": "menu::SelectFirst", + }, }, { "context": "GitPanel && ChangesList && !GitBranchSelector", @@ -1205,4 +1205,18 @@ "enter": "editor::Newline", }, }, + { + "context": "SkillCreator", + "bindings": { + "tab": "skill_creator::FocusNextField", + "shift-tab": "skill_creator::FocusPreviousField", + }, + }, + { + "context": "SkillCreator > Editor", + "bindings": { + "tab": "skill_creator::FocusNextField", + "shift-tab": "skill_creator::FocusPreviousField", + }, + }, ] diff --git a/crates/agent_skills/agent_skills.rs b/crates/agent_skills/agent_skills.rs index 2b37d88c64..0c240998f7 100644 --- a/crates/agent_skills/agent_skills.rs +++ b/crates/agent_skills/agent_skills.rs @@ -1,5 +1,5 @@ use anyhow::{Context as _, Result}; -use const_format::concatcp; +use const_format::{concatcp, formatcp}; use fs::Fs; use futures::StreamExt; use gpui::{Global, SharedString}; @@ -251,8 +251,8 @@ pub fn parse_skill_frontmatter( let (metadata, _body) = extract_frontmatter(content)?; - validate_name(&metadata.name)?; - validate_description(&metadata.description)?; + validate_name(&metadata.name).map_err(anyhow::Error::msg)?; + validate_description(&metadata.description).map_err(anyhow::Error::msg)?; let directory_path = skill_file_path .parent() @@ -346,6 +346,14 @@ fn extract_frontmatter(content: &str) -> Result<(SkillMetadata, &str)> { /// by [`validate_name`]. pub const MAX_SKILL_NAME_LEN: usize = 64; +/// Maximum length (in bytes) for a valid skill description. Mirrors the +/// upper bound enforced by [`validate_description`]. +/// +/// Byte-based rather than char-based because that's what `.len()` returns +/// and what every caller currently measures; the UI also surfaces this +/// limit as a byte count so the editor's counter matches the validator. +pub const MAX_SKILL_DESCRIPTION_LEN: usize = 1024; + /// Convert an arbitrary human-readable string into a valid skill name, or /// return `None` if no valid name can be produced (e.g. the input contains /// no ASCII alphanumeric characters at all). @@ -410,34 +418,54 @@ pub fn slugify_skill_name(input: &str) -> Option { if slug.is_empty() { None } else { Some(slug) } } -fn validate_name(name: &str) -> Result<()> { +/// Validate a skill name against the rules enforced by both the loader +/// and the create-skill UI. +/// +/// Rules: +/// * non-empty +/// * at most [`MAX_SKILL_NAME_LEN`] bytes +/// * ASCII lowercase letters, digits, and hyphens only +/// * must not start or end with a hyphen — [`slugify_skill_name`] +/// already guarantees this for its output, so requiring it in the +/// validator keeps hand-written `SKILL.md` files consistent with +/// slugifier output +/// +/// Error messages are returned as `&'static str` (interpolated at +/// compile time via `formatcp!`) so that UI surfaces can store them in +/// `Option<&'static str>` fields without allocating, and loader callers +/// can convert them to `anyhow::Error` via `anyhow::Error::msg`. +pub fn validate_name(name: &str) -> Result<(), &'static str> { if name.is_empty() { - anyhow::bail!("Skill name cannot be empty"); + return Err("Skill name cannot be empty"); } - if name.len() > MAX_SKILL_NAME_LEN { - anyhow::bail!("Skill name must be at most {MAX_SKILL_NAME_LEN} characters"); + return Err(formatcp!( + "Skill name must be at most {MAX_SKILL_NAME_LEN} characters" + )); + } + if name.starts_with('-') || name.ends_with('-') { + return Err("Skill name must not start or end with a hyphen"); } - if !name .chars() .all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || c == '-') { - anyhow::bail!("Skill name must contain only lowercase letters, numbers, and hyphens"); + return Err("Skill name must contain only lowercase letters, numbers, and hyphens"); } - Ok(()) } -fn validate_description(description: &str) -> Result<()> { - if description.is_empty() { - anyhow::bail!("Skill description cannot be empty"); +/// Validate a skill description against the rules enforced by both the +/// loader and the create-skill UI. +pub fn validate_description(description: &str) -> Result<(), &'static str> { + if description.trim().is_empty() { + return Err("Skill description cannot be empty"); } - - if description.len() > 1024 { - anyhow::bail!("Skill description must be at most 1024 characters"); + if description.len() > MAX_SKILL_DESCRIPTION_LEN { + return Err(formatcp!( + "Skill description must be at most {MAX_SKILL_DESCRIPTION_LEN} bytes" + )); } - Ok(()) } @@ -672,8 +700,8 @@ pub fn builtin_skills() -> Vec { /// gets a synthetic `` path since it doesn't live on disk. fn parse_builtin_skill(name: &str, content: &'static str) -> Result { let (metadata, body) = extract_frontmatter(content)?; - validate_name(&metadata.name)?; - validate_description(&metadata.description)?; + validate_name(&metadata.name).map_err(anyhow::Error::msg)?; + validate_description(&metadata.description).map_err(anyhow::Error::msg)?; let synthetic_dir = PathBuf::from(format!("/{}", name)); let synthetic_path = synthetic_dir.join(SKILL_FILE_NAME); @@ -1004,12 +1032,8 @@ Content. SkillSource::Global, ); assert!(result.is_err()); - assert!( - result - .unwrap_err() - .to_string() - .contains("at most 64 characters") - ); + let expected = format!("at most {MAX_SKILL_NAME_LEN} characters"); + assert!(result.unwrap_err().to_string().contains(&expected)); } #[test] @@ -1285,12 +1309,8 @@ Content. SkillSource::Global, ); assert!(result.is_err()); - assert!( - result - .unwrap_err() - .to_string() - .contains("at most 1024 characters") - ); + let expected = format!("at most {MAX_SKILL_DESCRIPTION_LEN} bytes"); + assert!(result.unwrap_err().to_string().contains(&expected)); } #[test] @@ -1891,4 +1911,89 @@ description: A skill with no body content "project/.AGENTS/SKILLS/foo" ))); } + + #[test] + fn validate_name_accepts_valid_names() { + assert!(validate_name("draft-pr").is_ok()); + assert!(validate_name("a").is_ok()); + assert!(validate_name("skill1").is_ok()); + assert!(validate_name(&"a".repeat(MAX_SKILL_NAME_LEN)).is_ok()); + } + + #[test] + fn validate_name_rejects_empty() { + assert!(validate_name("").is_err()); + } + + #[test] + fn validate_name_rejects_uppercase() { + assert!(validate_name("Draft-PR").is_err()); + } + + #[test] + fn validate_name_rejects_leading_and_trailing_hyphens() { + assert!(validate_name("-draft").is_err()); + assert!(validate_name("draft-").is_err()); + } + + #[test] + fn validate_name_rejects_invalid_chars() { + assert!(validate_name("draft_pr").is_err()); + assert!(validate_name("draft pr").is_err()); + assert!(validate_name("draft.pr").is_err()); + } + + #[test] + fn validate_name_rejects_too_long() { + assert!(validate_name(&"a".repeat(MAX_SKILL_NAME_LEN + 1)).is_err()); + } + + #[test] + fn validate_description_accepts_valid() { + assert!(validate_description("A useful skill").is_ok()); + } + + #[test] + fn validate_description_rejects_empty_and_whitespace_only() { + assert!(validate_description("").is_err()); + assert!(validate_description(" ").is_err()); + assert!(validate_description("\t\n ").is_err()); + } + + #[test] + fn validate_description_rejects_too_long() { + assert!(validate_description(&"a".repeat(MAX_SKILL_DESCRIPTION_LEN + 1)).is_err()); + } + + #[test] + fn validate_description_length_is_measured_in_bytes() { + // "é" is 2 bytes in UTF-8. A string of MAX/2 + 1 "é" characters has + // only ~MAX/2 + 1 chars but exceeds MAX bytes, so it must be + // rejected by a byte-based validator (and accepted by a char-based + // one). This regression-tests the byte semantics that the loader + // and UI both rely on. + let chars = MAX_SKILL_DESCRIPTION_LEN / 2 + 1; + let description = "é".repeat(chars); + assert!(description.chars().count() <= MAX_SKILL_DESCRIPTION_LEN); + assert!(description.len() > MAX_SKILL_DESCRIPTION_LEN); + assert!(validate_description(&description).is_err()); + } + + #[test] + fn slugify_output_always_passes_validate_name() { + for input in [ + "foo", + "Foo Bar", + "rock & roll", + "---weird---", + "a".repeat(200).as_str(), + ] { + if let Some(slug) = slugify_skill_name(input) { + assert!( + validate_name(&slug).is_ok(), + "slug {slug:?} from {input:?} failed validate_name" + ); + } + } + } } diff --git a/crates/agent_ui/Cargo.toml b/crates/agent_ui/Cargo.toml index 91b20aca27..91c0f21a51 100644 --- a/crates/agent_ui/Cargo.toml +++ b/crates/agent_ui/Cargo.toml @@ -89,6 +89,7 @@ remote.workspace = true remote_connection.workspace = true rope.workspace = true rules_library.workspace = true +skill_creator.workspace = true schemars.workspace = true serde.workspace = true serde_json.workspace = true diff --git a/crates/agent_ui/src/agent_panel.rs b/crates/agent_ui/src/agent_panel.rs index 2e91decb17..c7a962b277 100644 --- a/crates/agent_ui/src/agent_panel.rs +++ b/crates/agent_ui/src/agent_panel.rs @@ -27,7 +27,7 @@ use zed_actions::{ ResetAgentZoom, ResetOnboarding, ResolveConflictedFilesWithAgent, ResolveConflictsWithAgent, ReviewBranchDiff, }, - assistant::{FocusAgent, OpenRulesLibrary, Toggle, ToggleFocus}, + assistant::{FocusAgent, OpenRulesLibrary, OpenSkillCreator, Toggle, ToggleFocus}, }; use crate::ExpandMessageEditor; @@ -38,7 +38,7 @@ use crate::terminal_thread_metadata_store::{TerminalThreadMetadata, TerminalThre use crate::thread_metadata_store::{ThreadId, ThreadMetadataStore, ThreadMetadataStoreEvent}; use crate::{ AddContextServer, AgentDiffPane, ConversationView, CopyThreadToClipboard, Follow, - InlineAssistant, LoadThreadFromClipboard, NewThread, OpenActiveThreadAsMarkdown, OpenAgentDiff, + LoadThreadFromClipboard, NewThread, OpenActiveThreadAsMarkdown, OpenAgentDiff, ResetTrialEndUpsell, ResetTrialUpsell, ShowAllSidebarThreadMetadata, ShowThreadMetadata, ToggleNewThreadMenu, ToggleOptionsMenu, agent_configuration::{AgentConfiguration, AssistantConfigurationEvent}, @@ -66,16 +66,16 @@ use fs::Fs; use gpui::{ Action, Anchor, Animation, AnimationExt, AnyElement, App, AsyncWindowContext, ClipboardItem, Entity, EventEmitter, ExternalPaths, FocusHandle, Focusable, KeyContext, Pixels, - PlatformDisplay, Subscription, Task, TaskExt, UpdateGlobal, WeakEntity, WindowHandle, - prelude::*, pulsating_between, + PlatformDisplay, Subscription, Task, TaskExt, WeakEntity, WindowHandle, prelude::*, + pulsating_between, }; use language::LanguageRegistry; use language_model::LanguageModelRegistry; use project::{Project, ProjectPath, Worktree}; -use prompt_store::{PromptStore, UserPromptId}; -use rules_library::{RulesLibrary, open_rules_library}; +use prompt_store::PromptStore; use settings::TerminalDockPosition; use settings::{NotifyWhenAgentWaiting, Settings, update_settings_file}; +use skill_creator::open_skill_creator; use terminal::{Event as TerminalEvent, terminal_settings::TerminalSettings}; use terminal_view::{TerminalView, terminal_panel::TerminalPanel}; use theme_settings::ThemeSettings; @@ -300,6 +300,14 @@ pub fn init(cx: &mut App) { }); } }) + .register_action(|workspace, action: &OpenSkillCreator, window, cx| { + if let Some(panel) = workspace.panel::(cx) { + workspace.focus_panel::(window, cx); + panel.update(cx, |panel, cx| { + panel.deploy_skill_creator(action, window, cx) + }); + } + }) .register_action(|workspace, _: &Follow, window, cx| { workspace.follow(CollaboratorId::Agent, window, cx); }) @@ -2980,16 +2988,27 @@ impl AgentPanel { fn deploy_rules_library( &mut self, - action: &OpenRulesLibrary, + _action: &OpenRulesLibrary, + window: &mut Window, + cx: &mut Context, + ) { + // The legacy Rules action is rerouted to the skill creator so the + // existing keyboard shortcut (still bound to `OpenRulesLibrary` in + // the default keymaps) and any persisted user keymap entries keep + // working. + self.deploy_skill_creator(&OpenSkillCreator, window, cx); + } + + fn deploy_skill_creator( + &mut self, + _action: &OpenSkillCreator, _window: &mut Window, cx: &mut Context, ) { - open_rules_library( + open_skill_creator( + Some(self.workspace.clone()), self.language_registry.clone(), - Box::new(PromptLibraryInlineAssist::new(self.workspace.clone())), - action - .prompt_to_select - .map(|uuid| UserPromptId(uuid).into()), + self.fs.clone(), cx, ) .detach_and_log_err(cx); @@ -4670,6 +4689,16 @@ impl AgentPanel { cx: &mut Context, ) -> impl IntoElement { let focus_handle = self.focus_handle(cx); + // Resolve menu shortcuts at the thread root; the active editor can + // shadow panel-level commands such as OpenRulesLibrary. + let menu_action_context = match &self.base_view { + BaseView::AgentThread { conversation_view } => conversation_view + .read(cx) + .active_thread() + .map(|thread| thread.read(cx).focus_handle.clone()) + .unwrap_or_else(|| focus_handle.clone()), + _ => focus_handle.clone(), + }; let showing_terminal = matches!(self.visible_surface(), VisibleSurface::Terminal(_)); let conversation_view = match &self.base_view { @@ -4698,16 +4727,13 @@ impl AgentPanel { .trigger_with_tooltip( IconButton::new("agent-options-menu", IconName::Ellipsis) .icon_size(IconSize::Small), - { - let focus_handle = focus_handle.clone(); - move |_window, cx| { - Tooltip::for_action_in( - "Toggle Agent Menu", - &ToggleOptionsMenu, - &focus_handle, - cx, - ) - } + move |_window, cx| { + Tooltip::for_action_in( + "Toggle Agent Menu", + &ToggleOptionsMenu, + &focus_handle, + cx, + ) }, ) .anchor(Anchor::TopRight) @@ -4715,7 +4741,7 @@ impl AgentPanel { .menu({ move |window, cx| { Some(ContextMenu::build(window, cx, |mut menu, _window, _| { - menu = menu.context(focus_handle.clone()); + menu = menu.context(menu_action_context.clone()); if can_regenerate_thread_title { menu = menu.header("Current Thread"); @@ -4750,6 +4776,14 @@ impl AgentPanel { .action("Add Custom Server…", Box::new(AddContextServer)) .separator(); + menu = menu.entry( + "Skills", + Some(Box::new(OpenRulesLibrary::default())), + |window, cx| { + window.dispatch_action(Box::new(OpenSkillCreator), cx); + }, + ); + menu = menu.action("Profiles", Box::new(ManageProfiles::default())); } @@ -5575,6 +5609,7 @@ impl Render for AgentPanel { })) .on_action(cx.listener(Self::open_active_thread_as_markdown)) .on_action(cx.listener(Self::deploy_rules_library)) + .on_action(cx.listener(Self::deploy_skill_creator)) .on_action(cx.listener(Self::go_back)) .on_action(cx.listener(Self::toggle_options_menu)) .on_action(cx.listener(Self::increase_font_size)) @@ -5626,57 +5661,6 @@ impl Render for AgentPanel { } } -struct PromptLibraryInlineAssist { - workspace: WeakEntity, -} - -impl PromptLibraryInlineAssist { - pub fn new(workspace: WeakEntity) -> Self { - Self { workspace } - } -} - -impl rules_library::InlineAssistDelegate for PromptLibraryInlineAssist { - fn assist( - &self, - prompt_editor: &Entity, - initial_prompt: Option, - window: &mut Window, - cx: &mut Context, - ) { - InlineAssistant::update_global(cx, |assistant, cx| { - let Some(workspace) = self.workspace.upgrade() else { - return; - }; - let Some(panel) = workspace.read(cx).panel::(cx) else { - return; - }; - let project = workspace.read(cx).project().downgrade(); - let panel = panel.read(cx); - let thread_store = panel.thread_store().clone(); - assistant.assist( - prompt_editor, - self.workspace.clone(), - project, - thread_store, - None, - initial_prompt, - window, - cx, - ); - }) - } - - fn focus_agent_panel( - &self, - workspace: &mut Workspace, - window: &mut Window, - cx: &mut Context, - ) -> bool { - workspace.focus_panel::(window, cx).is_some() - } -} - struct OnboardingUpsell; impl Dismissable for OnboardingUpsell { @@ -5958,7 +5942,7 @@ mod tests { use anyhow::{Result, anyhow}; use feature_flags::FeatureFlagAppExt; use fs::FakeFs; - use gpui::{App, TestAppContext, VisualTestContext}; + use gpui::{App, TestAppContext, UpdateGlobal, VisualTestContext}; use parking_lot::Mutex; use project::{Project, WorktreePaths}; use std::any::Any; @@ -8030,6 +8014,60 @@ mod tests { }); } + #[gpui::test] + async fn test_skills_menu_entry_shows_rules_shortcut(cx: &mut TestAppContext) { + init_test(cx); + cx.update(|cx| { + let default_key_bindings = settings::KeymapFile::load_asset_allow_partial_failure( + "keymaps/default-macos.json", + cx, + ) + .unwrap(); + cx.bind_keys(default_key_bindings); + agent::ThreadStore::init_global(cx); + language_model::LanguageModelRegistry::test(cx); + }); + + let fs = FakeFs::new(cx.executor()); + cx.update(|cx| ::set_global(fs.clone(), cx)); + fs.insert_tree("/project", json!({ "file.txt": "" })).await; + let project = Project::test(fs.clone(), [Path::new("/project")], cx).await; + + let multi_workspace = + cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + let workspace = multi_workspace + .read_with(cx, |multi_workspace, _cx| { + multi_workspace.workspace().clone() + }) + .unwrap(); + let mut cx = VisualTestContext::from_window(multi_workspace.into(), cx); + + let panel = workspace.update_in(&mut cx, |workspace, window, cx| { + let panel = cx.new(|cx| AgentPanel::new(workspace, None, window, cx)); + workspace.add_panel(panel.clone(), window, cx); + panel + }); + open_thread_with_connection(&panel, StubAgentConnection::new(), &mut cx); + workspace.update_in(&mut cx, |workspace, window, cx| { + workspace.focus_panel::(window, cx); + }); + cx.run_until_parked(); + + panel.update_in(&mut cx, |panel, window, cx| { + panel.toggle_options_menu(&ToggleOptionsMenu, window, cx); + }); + cx.run_until_parked(); + + assert!( + cx.debug_bounds("MENU_ITEM-Skills").is_some(), + "Skills menu item should be visible" + ); + assert!( + cx.debug_bounds("KEY_BINDING-l").is_some(), + "Skills menu item should show the OpenRulesLibrary shortcut" + ); + } + #[gpui::test] async fn test_terminal_close_event_closes_without_sidebar(cx: &mut TestAppContext) { let (panel, mut cx) = setup_panel(cx).await; diff --git a/crates/agent_ui/src/agent_ui.rs b/crates/agent_ui/src/agent_ui.rs index ef7ff079b5..bb3fb26ac6 100644 --- a/crates/agent_ui/src/agent_ui.rs +++ b/crates/agent_ui/src/agent_ui.rs @@ -506,6 +506,7 @@ pub fn init( ) { agent::ThreadStore::init_global(cx); rules_library::init(cx); + skill_creator::init(cx); if !is_eval { // Initializing the language model from the user settings messes with the eval, so we only initialize them when // we're not running inside of the eval. @@ -650,6 +651,7 @@ fn update_command_palette_filter(cx: &mut App) { ]; let open_rules_library_action = [TypeId::of::()]; + let open_skill_creator_action = [TypeId::of::()]; if disable_ai { filter.hide_namespace("agent"); @@ -707,8 +709,10 @@ fn update_command_palette_filter(cx: &mut App) { // rest of that namespace's actions. if !disable_ai { filter.hide_action_types(&open_rules_library_action); + filter.show_action_types(open_skill_creator_action.iter()); } else { filter.show_action_types(open_rules_library_action.iter()); + filter.hide_action_types(&open_skill_creator_action); } }); } diff --git a/crates/settings_ui/src/pages/skills_setup.rs b/crates/settings_ui/src/pages/skills_setup.rs index e3ea3bb548..bce8b7e7f5 100644 --- a/crates/settings_ui/src/pages/skills_setup.rs +++ b/crates/settings_ui/src/pages/skills_setup.rs @@ -56,7 +56,7 @@ pub(crate) fn render_skills_setup_page( .gap_2() .child(Label::new(message).color(Color::Muted)) .child( - Button::new("open-agent-panel", "Create with Agent") + Button::new("open-skill-creator", "Create a Skill") .tab_index(0_isize) .style(ButtonStyle::Outlined) .end_icon( @@ -71,10 +71,10 @@ pub(crate) fn render_skills_setup_page( original_window .update(cx, |_workspace, original_window, cx| { original_window.dispatch_action( - zed_actions::assistant::ToggleFocus.boxed_clone(), + zed_actions::assistant::OpenSkillCreator + .boxed_clone(), cx, ); - original_window.activate_window(); }) .log_err(); window.remove_window(); diff --git a/crates/skill_creator/Cargo.toml b/crates/skill_creator/Cargo.toml new file mode 100644 index 0000000000..2ffd7bb85d --- /dev/null +++ b/crates/skill_creator/Cargo.toml @@ -0,0 +1,36 @@ +[package] +name = "skill_creator" +version = "0.1.0" +edition.workspace = true +publish.workspace = true +license = "GPL-3.0-or-later" + +[lints] +workspace = true + +[lib] +path = "src/skill_creator.rs" + +[dependencies] +agent_skills.workspace = true +anyhow.workspace = true +editor.workspace = true +fs.workspace = true +gpui.workspace = true +language.workspace = true +menu.workspace = true +platform_title_bar.workspace = true +release_channel.workspace = true +serde_yaml_ng.workspace = true +settings.workspace = true +theme_settings.workspace = true +ui.workspace = true +ui_input.workspace = true +util.workspace = true +workspace.workspace = true +worktree.workspace = true + +[dev-dependencies] +fs = { workspace = true, features = ["test-support"] } +gpui = { workspace = true, features = ["test-support"] } +serde_json.workspace = true diff --git a/crates/skill_creator/LICENSE-GPL b/crates/skill_creator/LICENSE-GPL new file mode 120000 index 0000000000..89e542f750 --- /dev/null +++ b/crates/skill_creator/LICENSE-GPL @@ -0,0 +1 @@ +../../LICENSE-GPL \ No newline at end of file diff --git a/crates/skill_creator/src/skill_creator.rs b/crates/skill_creator/src/skill_creator.rs new file mode 100644 index 0000000000..8a1ee24437 --- /dev/null +++ b/crates/skill_creator/src/skill_creator.rs @@ -0,0 +1,1073 @@ +use agent_skills::{ + AGENTS_DIR_NAME, GLOBAL_SKILLS_DIR_DISPLAY, SKILL_FILE_NAME, SKILLS_DIR_NAME, SkillMetadata, + global_skills_dir, validate_description, validate_name, +}; +use anyhow::{Context as _, Result}; +use editor::{CurrentLineHighlight, Editor, EditorElement, EditorEvent, EditorStyle}; +use fs::Fs; +use gpui::{ + App, Bounds, DEFAULT_ADDITIONAL_WINDOW_SIZE, Entity, FocusHandle, Focusable, Subscription, + Task, TextStyle, Tiling, TitlebarOptions, WeakEntity, WindowBounds, WindowHandle, + WindowOptions, actions, point, +}; +use language::{Buffer, LanguageRegistry, language_settings::SoftWrap}; +use platform_title_bar::PlatformTitleBar; +use release_channel::ReleaseChannel; +use settings::{ActionSequence, Settings}; +use std::path::PathBuf; +use std::sync::Arc; +use theme_settings::ThemeSettings; +use ui::{ + ContextMenu, Divider, DropdownMenu, DropdownStyle, Headline, HeadlineSize, SwitchField, + prelude::*, +}; +use ui_input::{ErasedEditorEvent, InputField}; +use util::ResultExt; +use workspace::{ + Toast, Workspace, WorkspaceSettings, client_side_decorations, notifications::NotificationId, +}; +use worktree::WorktreeId; + +actions!( + skill_creator, + [SaveSkill, Cancel, FocusNextField, FocusPreviousField,] +); + +const NAME_FIELD_TAB_INDEX: isize = 1; +const DESCRIPTION_FIELD_TAB_INDEX: isize = 2; +const SCOPE_FIELD_TAB_INDEX: isize = 3; +const DISABLE_MODEL_INVOCATION_TAB_INDEX: isize = 4; +const BODY_FIELD_TAB_INDEX: isize = 5; + +pub fn init(_cx: &mut App) {} + +#[derive(Clone, Debug)] +enum ScopeChoice { + Global, + Project { + worktree_id: WorktreeId, + root_name: SharedString, + abs_path: Arc, + }, +} + +impl ScopeChoice { + fn label(&self) -> SharedString { + match self { + ScopeChoice::Global => "Global".into(), + ScopeChoice::Project { root_name, .. } => root_name.clone(), + } + } + + fn key(&self) -> SharedString { + match self { + ScopeChoice::Global => "global".into(), + ScopeChoice::Project { worktree_id, .. } => { + SharedString::from(format!("project-{}", worktree_id.to_usize())) + } + } + } + + /// Absolute path of the `.agents/skills` directory this scope writes to. + fn skills_dir(&self) -> PathBuf { + match self { + ScopeChoice::Global => global_skills_dir(), + ScopeChoice::Project { abs_path, .. } => { + abs_path.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME) + } + } + } +} + +/// Collect the user-visible worktrees from the originating workspace and +/// turn them into project-scope choices. Returns an empty `Vec` if the +/// workspace can't be read (e.g. it was already dropped). +fn project_scopes_from_workspace( + workspace: &Option>, + cx: &App, +) -> Vec { + let Some(workspace) = workspace.as_ref().and_then(|w| w.upgrade()) else { + return Vec::new(); + }; + let workspace = workspace.read(cx); + let project = workspace.project().read(cx); + project + .visible_worktrees(cx) + .filter_map(|worktree| { + let worktree = worktree.read(cx); + if !worktree.is_local() { + return None; + } + Some(ScopeChoice::Project { + worktree_id: worktree.id(), + root_name: SharedString::from(worktree.root_name_str().to_string()), + abs_path: worktree.abs_path(), + }) + }) + .collect() +} + +/// Open the skills library window. If one is already open, brings it to the +/// foreground. +pub fn open_skill_creator( + workspace: Option>, + language_registry: Arc, + fs: Arc, + cx: &mut App, +) -> Task>> { + cx.spawn(async move |cx| { + let existing = cx.update(|cx| { + let handle = cx + .windows() + .into_iter() + .find_map(|window| window.downcast::()); + if let Some(handle) = handle { + handle + .update(cx, |_, window, _| window.activate_window()) + .ok(); + Some(handle) + } else { + None + } + }); + if let Some(window) = existing { + return Ok(window); + } + + cx.update(|cx| { + let app_id = ReleaseChannel::global(cx).app_id(); + let bounds = Bounds::centered(None, DEFAULT_ADDITIONAL_WINDOW_SIZE, cx); + let window_decorations = match std::env::var("ZED_WINDOW_DECORATIONS") { + Ok(val) if val == "server" => gpui::WindowDecorations::Server, + Ok(val) if val == "client" => gpui::WindowDecorations::Client, + _ => match WorkspaceSettings::get_global(cx).window_decorations { + settings::WindowDecorations::Server => gpui::WindowDecorations::Server, + settings::WindowDecorations::Client => gpui::WindowDecorations::Client, + }, + }; + cx.open_window( + WindowOptions { + titlebar: Some(TitlebarOptions { + title: Some("New Skill".into()), + appears_transparent: true, + traffic_light_position: Some(point(px(12.0), px(12.0))), + }), + app_id: Some(app_id.to_owned()), + window_bounds: Some(WindowBounds::Windowed(bounds)), + window_background: cx.theme().window_background_appearance(), + window_decorations: Some(window_decorations), + window_min_size: Some(DEFAULT_ADDITIONAL_WINDOW_SIZE), + kind: gpui::WindowKind::Floating, + ..Default::default() + }, + |window, cx| { + cx.new(|cx| SkillCreator::new(workspace, language_registry, fs, window, cx)) + }, + ) + }) + }) +} + +pub struct SkillCreator { + focus_handle: FocusHandle, + title_bar: Option>, + workspace: Option>, + fs: Arc, + name_editor: Entity, + description_editor: Entity, + body_editor: Entity, + description_length: usize, + scopes: Vec, + selected_scope_key: SharedString, + disable_model_invocation: bool, + name_error: Option<&'static str>, + description_error: Option<&'static str>, + body_error: Option<&'static str>, + save_error: Option, + saving: bool, + // Held so that dropping the entity (e.g. the window closing) cancels + // an in-flight save. Detaching the task instead would let + // `write_skill_to_disk` complete after the UI is gone, silently + // creating a SKILL.md on disk with no toast and no error feedback. + save_task: Option>, + _subscriptions: Vec, +} + +impl SkillCreator { + fn new( + workspace: Option>, + language_registry: Arc, + fs: Arc, + window: &mut Window, + cx: &mut Context, + ) -> Self { + let focus_handle = cx.focus_handle(); + let project_scopes = project_scopes_from_workspace(&workspace, cx); + + // Default to first project scope (project-level) when available; + // otherwise fall back to Global. + let mut scopes: Vec = Vec::with_capacity(project_scopes.len() + 1); + scopes.push(ScopeChoice::Global); + scopes.extend(project_scopes); + let selected_scope_key = scopes + .iter() + .find(|scope| matches!(scope, ScopeChoice::Project { .. })) + .map(|scope| scope.key()) + .unwrap_or_else(|| ScopeChoice::Global.key()); + + let name_editor = cx.new(|cx| { + InputField::new(window, cx, "my-new-skill") + .label("Name") + .tab_index(NAME_FIELD_TAB_INDEX) + .tab_stop(true) + }); + // Focus the name field on open. Without this, no element inside + // the window has focus, so dispatching the `Cancel` action from + // the Cancel button (which walks the focused element's dispatch + // path looking for `on_action` handlers) silently does nothing + // until the user manually clicks into one of the editors. The + // name editor is also the natural first field to type into. + window.focus(&name_editor.focus_handle(cx), cx); + + let description_editor = cx.new(|cx| { + InputField::new( + window, + cx, + "e.g., Fill the PR description following this template.", + ) + .label("Description") + .tab_index(DESCRIPTION_FIELD_TAB_INDEX) + .tab_stop(true) + }); + + let body_editor = cx.new(|cx| { + let buffer = cx.new(|cx| { + let buffer = Buffer::local(String::new(), cx); + buffer.set_language_registry(language_registry.clone()); + buffer + }); + let mut editor = Editor::for_buffer(buffer, None, window, cx); + editor.set_placeholder_text("Add skill content…", window, cx); + editor.set_soft_wrap_mode(SoftWrap::EditorWidth, cx); + editor.set_show_gutter(false, cx); + editor.set_show_wrap_guides(false, cx); + editor.set_show_indent_guides(false, cx); + editor.set_use_modal_editing(true); + editor.set_current_line_highlight(Some(CurrentLineHighlight::None)); + editor + }); + + // Attach Markdown language to the body editor asynchronously, since + // `language_for_name` returns a Task. + cx.spawn_in(window, { + let body_editor = body_editor.downgrade(); + let language_registry = language_registry.clone(); + async move |_this, cx| { + let markdown = language_registry.language_for_name("Markdown").await.ok(); + if let Some(markdown) = markdown { + body_editor + .update(cx, |editor, cx| { + editor.buffer().update(cx, |multi_buffer, cx| { + if let Some(buffer) = multi_buffer.as_singleton() { + buffer.update(cx, |buffer, cx| { + buffer.set_language(Some(markdown), cx) + }); + } + }); + }) + .ok(); + } + } + }) + .detach(); + + let name_input_editor = name_editor.read(cx).editor().clone(); + let description_input_editor = description_editor.read(cx).editor().clone(); + let weak = cx.weak_entity(); + let name_subscription = name_input_editor.subscribe( + Box::new(move |event, window, cx| { + weak.update(cx, |this, cx| { + this.handle_name_input_event(&event, window, cx); + }) + .ok(); + }), + window, + cx, + ); + let weak = cx.weak_entity(); + let description_subscription = description_input_editor.subscribe( + Box::new(move |event, window, cx| { + weak.update(cx, |this, cx| { + this.handle_description_input_event(&event, window, cx); + }) + .ok(); + }), + window, + cx, + ); + + let subscriptions = vec![ + name_subscription, + description_subscription, + cx.subscribe_in(&body_editor, window, Self::handle_body_editor_event), + ]; + + Self { + focus_handle, + title_bar: if !cfg!(target_os = "macos") { + Some(cx.new(|cx| PlatformTitleBar::new("skill-creator-title-bar", cx))) + } else { + None + }, + workspace, + fs, + name_editor, + description_editor, + body_editor, + description_length: 0, + scopes, + selected_scope_key, + disable_model_invocation: false, + name_error: None, + description_error: None, + body_error: None, + save_error: None, + saving: false, + save_task: None, + _subscriptions: subscriptions, + } + } + + fn handle_name_input_event( + &mut self, + event: &ErasedEditorEvent, + _window: &mut Window, + cx: &mut Context, + ) { + if matches!(event, ErasedEditorEvent::BufferEdited) { + self.recompute_name_error(cx); + self.save_error = None; + cx.notify(); + } + } + + fn handle_description_input_event( + &mut self, + event: &ErasedEditorEvent, + _window: &mut Window, + cx: &mut Context, + ) { + if matches!(event, ErasedEditorEvent::BufferEdited) { + self.recompute_description_error(cx); + self.save_error = None; + cx.notify(); + } + } + + fn handle_body_editor_event( + &mut self, + _: &Entity, + event: &EditorEvent, + _window: &mut Window, + cx: &mut Context, + ) { + if matches!(event, EditorEvent::BufferEdited) { + self.recompute_body_error(cx); + self.save_error = None; + cx.notify(); + } + } + + fn current_name(&self, cx: &App) -> String { + self.name_editor.read(cx).text(cx) + } + + fn current_description(&self, cx: &App) -> String { + self.description_editor.read(cx).text(cx) + } + + fn current_body(&self, cx: &App) -> String { + self.body_editor.read(cx).text(cx) + } + + fn recompute_name_error(&mut self, cx: &App) { + let name = self.current_name(cx); + self.name_error = validate_name(&name).err(); + } + + fn recompute_description_error(&mut self, cx: &App) { + let description = self.current_description(cx); + self.description_length = description.len(); + self.description_error = validate_description(&description).err(); + } + + fn recompute_body_error(&mut self, cx: &App) { + let body = self.current_body(cx); + self.body_error = if body.trim().is_empty() { + Some("Body is required.") + } else { + None + }; + } + + fn is_valid(&self, cx: &App) -> bool { + validate_name(&self.current_name(cx)).is_ok() + && validate_description(&self.current_description(cx)).is_ok() + && !self.current_body(cx).trim().is_empty() + && self.selected_scope().is_some() + } + + fn selected_scope(&self) -> Option<&ScopeChoice> { + self.scopes + .iter() + .find(|scope| scope.key() == self.selected_scope_key) + } + + fn save_skill(&mut self, _: &SaveSkill, window: &mut Window, cx: &mut Context) { + // Surface any field-level errors before attempting to save. + self.recompute_name_error(cx); + self.recompute_description_error(cx); + self.recompute_body_error(cx); + + if !self.is_valid(cx) || self.saving { + cx.notify(); + return; + } + + let Some(scope) = self.selected_scope().cloned() else { + self.save_error = Some("Select a scope to save this skill to.".into()); + cx.notify(); + return; + }; + + let name = self.current_name(cx); + let description = self.current_description(cx); + let body = self.current_body(cx); + let disable_model_invocation = self.disable_model_invocation; + let fs = self.fs.clone(); + let workspace = self.workspace.clone(); + let scope_label = scope.label(); + + self.saving = true; + self.save_error = None; + cx.notify(); + + let task = cx.spawn_in(window, async move |this, cx| { + let result = write_skill_to_disk( + fs.as_ref(), + &scope.skills_dir(), + &name, + &description, + &body, + disable_model_invocation, + ) + .await; + + this.update_in(cx, |this, window, cx| { + this.saving = false; + this.save_task = None; + match result { + Ok(path) => { + if let Some(workspace) = workspace.as_ref().and_then(|w| w.upgrade()) { + workspace.update(cx, |workspace, cx| { + workspace.show_toast( + Toast::new( + NotificationId::unique::(), + format!( + "Saved skill \"{name}\" to {scope_label} ({})", + path.display() + ), + ), + cx, + ); + }); + } + window.remove_window(); + } + Err(err) => { + this.save_error = Some(SharedString::from(err.to_string())); + cx.notify(); + } + } + }) + .log_err(); + }); + self.save_task = Some(task); + } + + fn cancel(&mut self, _: &Cancel, window: &mut Window, _cx: &mut Context) { + // Block dismissal while a save is in flight. Otherwise the + // detached I/O could complete after the window is gone, leaving + // a SKILL.md on disk with no success or error feedback. The + // user can still force-close the window via the platform + // chrome, in which case dropping `self.save_task` cancels the + // pending write. + if self.saving { + return; + } + window.remove_window(); + } + + fn select_scope(&mut self, key: SharedString, cx: &mut Context) { + if self.scopes.iter().any(|scope| scope.key() == key) { + self.selected_scope_key = key; + self.save_error = None; + cx.notify(); + } + } + + fn toggle_disable_model_invocation(&mut self, cx: &mut Context) { + self.disable_model_invocation = !self.disable_model_invocation; + cx.notify(); + } + + fn render_scope_field(&self, window: &mut Window, cx: &mut Context) -> impl IntoElement { + let scopes = self.scopes.clone(); + let selected = self.selected_scope().cloned(); + let selected_label: SharedString = match selected.as_ref() { + Some(ScopeChoice::Global) => "Global".into(), + Some(ScopeChoice::Project { root_name, .. }) => { + SharedString::from(format!("{root_name} (project)")) + } + None => "Select a scope\u{2026}".into(), + }; + let sep = std::path::MAIN_SEPARATOR; + let scope_hint: SharedString = match selected.as_ref() { + Some(ScopeChoice::Global) => SharedString::from(format!( + "Available across every Zed project. \ + Saved to {GLOBAL_SKILLS_DIR_DISPLAY}{sep}\u{2039}name\u{203A}{sep}{SKILL_FILE_NAME}." + )), + Some(ScopeChoice::Project { root_name, .. }) => SharedString::from(format!( + "Only available when this project is open. \ + Saved to {root_name}{sep}{AGENTS_DIR_NAME}{sep}{SKILLS_DIR_NAME}{sep}\u{2039}name\u{203A}{sep}{SKILL_FILE_NAME}." + )), + None => "Choose where this skill should live.".into(), + }; + + let selected_label = h_flex() + .min_w_0() + .w_full() + .child(Label::new(selected_label).truncate()) + .into_any_element(); + + let weak = cx.weak_entity(); + + let menu = ContextMenu::build(window, cx, move |mut menu, _window, _cx| { + for scope in &scopes { + let key = scope.key(); + let weak = weak.clone(); + let entry_label: SharedString = match scope { + ScopeChoice::Global => "Global".into(), + ScopeChoice::Project { root_name, .. } => { + SharedString::from(format!("{root_name} (project)")) + } + }; + menu = menu.entry(entry_label, None, move |_window, cx| { + weak.update(cx, |this, cx| { + this.select_scope(key.clone(), cx); + }) + .log_err(); + }); + } + menu + }); + + h_flex() + .min_w_0() + .w_full() + .gap_6() + .justify_between() + .child( + v_flex() + .flex_1() + .min_w_0() + .child(Label::new("Scope")) + .child(Label::new(scope_hint).color(Color::Muted)), + ) + .child( + div().w_1_3().min_w_0().child( + DropdownMenu::new_with_element("skill-scope-dropdown", selected_label, menu) + .tab_index(SCOPE_FIELD_TAB_INDEX) + .style(DropdownStyle::Outlined) + .trigger_size(ButtonSize::Medium) + .full_width(true), + ), + ) + } + + fn render_optional_params(&self, cx: &mut Context) -> impl IntoElement { + let toggle_state: ToggleState = self.disable_model_invocation.into(); + + SwitchField::new( + "disable-model-invocation", + Some("Disable model invocation"), + Some( + "Hide this skill from the model's catalog. It can still be invoked via slash command." + .into(), + ), + toggle_state, + cx.listener(|this, _state: &ToggleState, _window, cx| { + this.toggle_disable_model_invocation(cx); + }), + ) + .tab_index(DISABLE_MODEL_INVOCATION_TAB_INDEX).into_any_element() + } + + fn render_body_field(&self, window: &mut Window, cx: &mut Context) -> impl IntoElement { + let settings = ThemeSettings::get_global(cx); + let theme = cx.theme().clone(); + + let has_error = self.body_error.is_some(); + + let focus_handle = self + .body_editor + .focus_handle(cx) + .tab_index(BODY_FIELD_TAB_INDEX) + .tab_stop(true); + + let border_color = if has_error { + theme.status().error_border + } else if focus_handle.contains_focused(window, cx) { + theme.colors().border_focused + } else { + theme.colors().border + }; + + div() + .w_full() + .flex_1() + .min_h(px(160.)) + .p_2p5() + .rounded_md() + .border_1() + .border_color(border_color) + .bg(theme.colors().editor_background) + .track_focus(&focus_handle) + .overflow_hidden() + .child(EditorElement::new( + &self.body_editor, + EditorStyle { + local_player: theme.players().local(), + text: TextStyle { + color: theme.colors().text, + font_family: settings.buffer_font.family.clone(), + font_features: settings.buffer_font.features.clone(), + font_size: rems(0.875).into(), + font_weight: settings.buffer_font.weight, + line_height: relative(settings.buffer_line_height.value()), + ..Default::default() + }, + syntax: theme.syntax().clone(), + inlay_hints_style: editor::make_inlay_hints_style(cx), + edit_prediction_styles: editor::make_suggestion_styles(cx), + ..EditorStyle::default() + }, + )) + } + + fn render_action_bar(&self, cx: &mut Context) -> impl IntoElement { + let valid = self.is_valid(cx); + let saving = self.saving; + let main_action = if saving { "Saving…" } else { "Save Skill" }; + + h_flex() + .w_full() + .map(|this| { + if self.save_error.is_some() { + this.justify_between() + } else { + this.justify_end() + } + }) + .gap_2() + .children( + self.save_error + .clone() + .map(|err| Label::new(err).size(LabelSize::Small).color(Color::Error)), + ) + .child( + h_flex() + .gap_1() + .child( + Button::new("cancel-skill", "Cancel") + .disabled(saving) + .on_click(|_, window, cx| { + window.dispatch_action(Box::new(Cancel), cx); + }), + ) + .child( + Button::new("save-skill", main_action) + .style(ButtonStyle::Filled) + .layer(ui::ElevationIndex::ModalSurface) + .disabled(!valid || saving) + .loading(saving) + .on_click(|_, window, cx| { + window.dispatch_action(Box::new(SaveSkill), cx); + }), + ), + ) + } + + fn render_header(&self, cx: &mut Context) -> impl IntoElement { + let theme = cx.theme().clone(); + let needs_traffic_light_clearance = cfg!(target_os = "macos"); + + h_flex() + .w_full() + .h_10() + .px_4() + .when(needs_traffic_light_clearance, |this| this.pl(px(84.))) + .border_b_1() + .border_color(theme.colors().border) + .child(Headline::new("Skill Creator").size(HeadlineSize::XSmall)) + } + + fn focus_next_field( + &mut self, + _: &FocusNextField, + window: &mut Window, + cx: &mut Context, + ) { + window.focus_next(cx); + } + + fn focus_previous_field( + &mut self, + _: &FocusPreviousField, + window: &mut Window, + cx: &mut Context, + ) { + window.focus_prev(cx); + } + + // When focus is on a non-editor tab stop (dropdown button, switch), + // Tab dispatches the global `menu::SelectNext` rather than our + // custom `FocusNextField`. Catching it here keeps the cycle moving. + fn on_menu_next(&mut self, _: &menu::SelectNext, window: &mut Window, cx: &mut Context) { + window.focus_next(cx); + } + + fn on_menu_prev( + &mut self, + _: &menu::SelectPrevious, + window: &mut Window, + cx: &mut Context, + ) { + window.focus_prev(cx); + } +} + +impl Focusable for SkillCreator { + fn focus_handle(&self, _cx: &App) -> FocusHandle { + self.focus_handle.clone() + } +} + +impl Render for SkillCreator { + fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { + let ui_font = theme_settings::setup_ui_font(window, cx); + let theme = cx.theme().clone(); + + client_side_decorations( + v_flex() + .id("skill-creator") + .key_context("SkillCreator") + .track_focus(&self.focus_handle) + .on_action( + |action_sequence: &ActionSequence, window: &mut Window, cx: &mut App| { + for action in &action_sequence.0 { + window.dispatch_action(action.boxed_clone(), cx); + } + }, + ) + .on_action(cx.listener(Self::save_skill)) + .on_action(cx.listener(Self::cancel)) + .on_action(cx.listener(Self::focus_next_field)) + .on_action(cx.listener(Self::focus_previous_field)) + .on_action(cx.listener(Self::on_menu_next)) + .on_action(cx.listener(Self::on_menu_prev)) + .size_full() + .overflow_hidden() + .font(ui_font) + .text_color(theme.colors().text) + .bg(theme.colors().panel_background) + .children(self.title_bar.clone()) + .child(self.render_header(cx)) + .child( + v_flex() + .id("skill-creator-form") + .tab_index(0) + .tab_group() + .tab_stop(false) + .flex_1() + .min_h_0() + .gap_4() + .p_4() + .child( + v_flex() + .gap_2() + .child(Label::new("Font-matter")) + .child(self.name_editor.clone()) + .child(self.description_editor.clone()), + ) + .child(self.render_optional_params(cx)) + .child(Divider::horizontal()) + .child(self.render_scope_field(window, cx)) + .child(Divider::horizontal()) + .child( + v_flex() + .flex_1() + .gap_2() + .child(Label::new("Skill Content")) + .child(self.render_body_field(window, cx)), + ), + ) + .child( + h_flex() + .w_full() + .p_2p5() + .border_t_1() + .border_color(theme.colors().border_variant) + .bg(theme.colors().panel_background) + .child(self.render_action_bar(cx)), + ), + window, + cx, + Tiling::default(), + ) + } +} + +/// Serialize the SKILL.md file to disk at `//SKILL.md`. +/// +/// Refuses to overwrite an existing directory at `/`. The +/// caller surfaces the resulting error to the user, who picks a different +/// name. +async fn write_skill_to_disk( + fs: &dyn Fs, + skills_dir: &std::path::Path, + name: &str, + description: &str, + body: &str, + disable_model_invocation: bool, +) -> Result { + let skill_dir = skills_dir.join(name); + match fs.metadata(&skill_dir).await { + Ok(Some(metadata)) if metadata.is_dir => { + anyhow::bail!( + "A skill named \"{name}\" already exists at {}. Pick a different name.", + skill_dir.display() + ); + } + Ok(Some(_)) => { + // Something exists at this path, but it isn't a directory — e.g. + // a stray file the user (or another tool) left there. Without + // this branch we'd fall through to `create_dir`, which on the + // real fs returns a generic "File exists" IO error that gives + // the user no idea what's wrong or how to recover. + anyhow::bail!( + "A file (not a skill directory) already exists at {}. \ + Delete it or pick a different skill name.", + skill_dir.display() + ); + } + Ok(None) => {} + Err(err) => { + return Err(err).with_context(|| { + format!( + "failed to check whether {} already exists", + skill_dir.display() + ) + }); + } + } + + let content = format_skill_file(name, description, body, disable_model_invocation)?; + + fs.create_dir(&skill_dir) + .await + .with_context(|| format!("failed to create skill directory {}", skill_dir.display()))?; + let skill_file_path = skill_dir.join(SKILL_FILE_NAME); + fs.write(&skill_file_path, content.as_bytes()) + .await + .with_context(|| format!("failed to write {}", skill_file_path.display()))?; + + Ok(skill_file_path) +} + +fn format_skill_file( + name: &str, + description: &str, + body: &str, + disable_model_invocation: bool, +) -> Result { + let metadata = SkillMetadata { + name: name.to_string(), + description: description.to_string(), + disable_model_invocation, + }; + let frontmatter = serde_yaml_ng::to_string(&metadata) + .context("failed to serialize skill frontmatter as YAML")?; + + let mut content = String::with_capacity(frontmatter.len() + body.len() + 16); + content.push_str("---\n"); + content.push_str(&frontmatter); + content.push_str("---\n"); + let trimmed_body = body.trim(); + if !trimmed_body.is_empty() { + content.push('\n'); + content.push_str(trimmed_body); + content.push('\n'); + } + Ok(content) +} + +#[cfg(test)] +mod tests { + use super::*; + use agent_skills::{SkillSource, parse_skill_frontmatter}; + use fs::FakeFs; + use std::path::Path; + + // Name and description validation rules are unit-tested in + // `agent_skills`, which owns `validate_name` / `validate_description` + // / `MAX_SKILL_DESCRIPTION_LEN`. The tests below cover this crate's + // own surface area: SKILL.md formatting and disk-writing. + + #[test] + fn format_skill_file_round_trips_through_parser() { + let content = + format_skill_file("draft-pr", "Push a draft PR", "Do the thing.", false).unwrap(); + let skill = parse_skill_frontmatter( + Path::new("/skills/draft-pr/SKILL.md"), + &content, + SkillSource::Global, + ) + .expect("generated frontmatter must round-trip through parse_skill_frontmatter"); + assert_eq!(skill.name, "draft-pr"); + assert_eq!(skill.description, "Push a draft PR"); + assert!(!skill.disable_model_invocation); + } + + #[test] + fn format_skill_file_writes_disable_model_invocation_true() { + let content = format_skill_file("my-skill", "description", "body", true).unwrap(); + assert!(content.contains("disable-model-invocation: true")); + } + + #[test] + fn format_skill_file_omits_body_when_empty() { + let content = format_skill_file("my-skill", "description", " ", false).unwrap(); + // The trailing closing-delimiter newline is the last byte. + assert!(content.ends_with("---\n")); + } + + #[test] + fn format_skill_file_escapes_yaml_specials_in_description() { + // serde_yaml_ng must quote/escape descriptions that contain YAML + // specials so the file round-trips. If we ever swap formatters, + // this test will catch a regression. + let tricky = "contains: a colon, # a hash, and a \"quote\""; + let content = format_skill_file("weird-skill", tricky, "body", false).unwrap(); + let skill = parse_skill_frontmatter( + Path::new("/skills/weird-skill/SKILL.md"), + &content, + SkillSource::Global, + ) + .expect("YAML-special characters must round-trip"); + assert_eq!(skill.description, tricky); + } + + #[gpui::test] + async fn write_skill_to_disk_creates_directory_and_file(cx: &mut gpui::TestAppContext) { + let fs = FakeFs::new(cx.executor()); + fs.insert_tree("/skills", serde_json::json!({})).await; + + let path = write_skill_to_disk( + fs.as_ref(), + Path::new("/skills"), + "draft-pr", + "Push a draft PR", + "Body of the skill.", + false, + ) + .await + .expect("write should succeed"); + + assert_eq!(path, Path::new("/skills/draft-pr/SKILL.md")); + let content = fs.load(&path).await.expect("file should exist"); + let skill = parse_skill_frontmatter(&path, &content, SkillSource::Global) + .expect("written file should be parseable"); + assert_eq!(skill.name, "draft-pr"); + assert_eq!(skill.description, "Push a draft PR"); + } + + #[gpui::test] + async fn write_skill_to_disk_refuses_to_overwrite(cx: &mut gpui::TestAppContext) { + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + "/skills", + serde_json::json!({ + "draft-pr": { + "SKILL.md": "---\nname: draft-pr\ndescription: existing\n---\nbody\n" + } + }), + ) + .await; + + let err = write_skill_to_disk( + fs.as_ref(), + Path::new("/skills"), + "draft-pr", + "Push a draft PR", + "Body of the skill.", + false, + ) + .await + .expect_err("writing over an existing skill must fail"); + assert!( + err.to_string().contains("already exists"), + "error message should mention the conflict, got: {err}" + ); + } + + #[gpui::test] + async fn write_skill_to_disk_rejects_non_directory_at_skill_path( + cx: &mut gpui::TestAppContext, + ) { + let fs = FakeFs::new(cx.executor()); + // A *file* (not a directory) sitting at `/skills/draft-pr`. With the + // old `is_dir` check this slipped through and we ended up surfacing + // the underlying "File exists" OS error. + fs.insert_tree( + "/skills", + serde_json::json!({ "draft-pr": "i am a stray file" }), + ) + .await; + + let err = write_skill_to_disk( + fs.as_ref(), + Path::new("/skills"), + "draft-pr", + "Push a draft PR", + "Body of the skill.", + false, + ) + .await + .expect_err("writing where a file already lives must fail"); + let message = err.to_string(); + assert!( + message.contains("not a skill directory"), + "error should explain the conflict is a non-directory, got: {message}" + ); + // Path separator differs between platforms (`/` on Unix, `\` on + // Windows), so reconstruct the expected `Display` form rather than + // hard-coding a separator. + let expected_path = Path::new("/skills").join("draft-pr"); + let expected_path = expected_path.display().to_string(); + assert!( + message.contains(&expected_path), + "error should include the conflicting path {expected_path:?}, got: {message}" + ); + } +} diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index fedf9d8cf2..17c910ab02 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -5253,6 +5253,7 @@ mod tests { "search", "settings_editor", "settings_profile_selector", + "skill_creator", "snippets", "stash_picker", "svg", diff --git a/crates/zed_actions/src/lib.rs b/crates/zed_actions/src/lib.rs index da51f3ccc7..8390cda86c 100644 --- a/crates/zed_actions/src/lib.rs +++ b/crates/zed_actions/src/lib.rs @@ -570,6 +570,8 @@ pub mod assistant { #[action(deprecated_aliases = ["assistant::ToggleFocus"])] ToggleFocus, FocusAgent, + /// Opens the skill creator window for creating a new skill. + OpenSkillCreator, ] );