From 4eebf3fdbcb04625a8bb6491be49896c0d423a9f Mon Sep 17 00:00:00 2001 From: MartinYe1234 <52641447+MartinYe1234@users.noreply.github.com> Date: Mon, 8 Jun 2026 07:57:47 -0700 Subject: [PATCH] settings_ui: Move skills management into settings UI (#58701) Closes AI-344 Closes AI-342 Moves agent skills management out of the agent panel and into the settings UI. The skill creator (previously its own crate) now lives in `settings_ui` as a settings page, alongside the skills setup page. The agent panel's ellipsis menu now links to the settings page instead of hosting skill management directly. Release Notes: - Improved agent skills management by moving it into the settings UI --------- Co-authored-by: Danilo Leal --- Cargo.lock | 32 +- Cargo.toml | 2 - assets/keymaps/default-linux.json | 2 +- assets/keymaps/default-macos.json | 2 +- assets/keymaps/default-windows.json | 2 +- crates/agent_skills/agent_skills.rs | 10 +- crates/agent_ui/Cargo.toml | 1 - crates/agent_ui/src/agent_panel.rs | 177 ++--- crates/agent_ui/src/agent_ui.rs | 36 +- .../src/conversation_view/thread_view.rs | 3 +- crates/settings_ui/Cargo.toml | 3 + crates/settings_ui/src/page_data.rs | 2 +- crates/settings_ui/src/pages.rs | 5 + .../src/pages}/skill_creator.rs | 730 ++++++------------ crates/settings_ui/src/pages/skills_setup.rs | 31 +- crates/settings_ui/src/settings_ui.rs | 420 +++++++++- crates/skill_creator/Cargo.toml | 39 - crates/skill_creator/LICENSE-GPL | 1 - crates/ui_input/src/input_field.rs | 19 + crates/zed/src/main.rs | 19 +- crates/zed_actions/Cargo.toml | 1 - crates/zed_actions/src/lib.rs | 16 +- docs/.doc-examples/complex-feature.md | 13 +- docs/src/ai/skills.md | 13 +- 24 files changed, 793 insertions(+), 786 deletions(-) rename crates/{skill_creator/src => settings_ui/src/pages}/skill_creator.rs (70%) delete mode 100644 crates/skill_creator/Cargo.toml delete mode 120000 crates/skill_creator/LICENSE-GPL diff --git a/Cargo.lock b/Cargo.lock index 12818a49939..6728dadcf8c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -519,7 +519,6 @@ dependencies = [ "serde_json", "serde_json_lenient", "settings", - "skill_creator", "streaming_diff", "task", "telemetry", @@ -16551,6 +16550,7 @@ dependencies = [ "fuzzy", "gpui", "heck 0.5.0", + "http_client", "itertools 0.14.0", "language", "log", @@ -16567,6 +16567,7 @@ dependencies = [ "search", "serde", "serde_json", + "serde_yaml_ng", "settings", "shell_command_parser", "strum 0.27.2", @@ -16575,6 +16576,7 @@ dependencies = [ "theme_settings", "title_bar", "ui", + "ui_input", "util", "workspace", "zed_actions", @@ -16849,33 +16851,6 @@ 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", - "futures 0.3.32", - "gpui", - "http_client", - "language", - "menu", - "notifications", - "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" @@ -23090,7 +23065,6 @@ dependencies = [ "schemars 1.0.4", "serde", "util", - "uuid", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 6e88facf82b..f3925a6223d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -170,7 +170,6 @@ members = [ "crates/rope", "crates/rpc", "crates/sandbox", - "crates/skill_creator", "crates/scheduler", "crates/schema_generator", "crates/search", @@ -428,7 +427,6 @@ reqwest_client = { path = "crates/reqwest_client" } rodio = { git = "https://github.com/RustAudio/rodio", rev = "e50e726ddd0292f6ef9de0dda6b90af4ed1fb66a", features = ["wav", "playback", "wav_output", "recording"] } rope = { path = "crates/rope" } rpc = { path = "crates/rpc" } -skill_creator = { path = "crates/skill_creator" } scheduler = { path = "crates/scheduler" } sandbox = { path = "crates/sandbox" } search = { path = "crates/search" } diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index 29988d2c292..e74abe77628 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -261,7 +261,7 @@ "ctrl-n": "agent::NewThread", "ctrl--": "pane::GoBack", "ctrl-alt-p": "agent::ManageProfiles", - "ctrl-alt-l": "agent::OpenRulesLibrary", + "ctrl-alt-l": "agent::ManageSkills", "ctrl-i": "agent::ToggleProfileSelector", "shift-tab": "agent::CycleModeSelector", "ctrl-alt-/": "agent::ToggleModelSelector", diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index a14cc04fe2f..9e0f2bb8376 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -306,7 +306,7 @@ "bindings": { "cmd-n": "agent::NewThread", "ctrl--": "pane::GoBack", - "cmd-alt-l": "agent::OpenRulesLibrary", + "cmd-alt-l": "agent::ManageSkills", "cmd-alt-p": "agent::ManageProfiles", "cmd-i": "agent::ToggleProfileSelector", "shift-tab": "agent::CycleModeSelector", diff --git a/assets/keymaps/default-windows.json b/assets/keymaps/default-windows.json index 3d926a67693..734506aa7bc 100644 --- a/assets/keymaps/default-windows.json +++ b/assets/keymaps/default-windows.json @@ -261,7 +261,7 @@ "bindings": { "ctrl-n": "agent::NewThread", "ctrl--": "pane::GoBack", - "shift-alt-l": "agent::OpenRulesLibrary", + "shift-alt-l": "agent::ManageSkills", "shift-alt-p": "agent::ManageProfiles", "ctrl-i": "agent::ToggleProfileSelector", "shift-tab": "agent::CycleModeSelector", diff --git a/crates/agent_skills/agent_skills.rs b/crates/agent_skills/agent_skills.rs index a4d20ea5f78..731a1cb23c7 100644 --- a/crates/agent_skills/agent_skills.rs +++ b/crates/agent_skills/agent_skills.rs @@ -2,9 +2,10 @@ use anyhow::{Context as _, Result}; use const_format::{concatcp, formatcp}; use fs::Fs; use futures::StreamExt; -use gpui::{Global, SharedString}; +use gpui::{App, Global, SharedString}; use serde::{Deserialize, Serialize}; use std::path::{Path, PathBuf}; +use std::rc::Rc; use std::sync::Arc; use url::Url; use util::paths::component_matches_ignore_ascii_case; @@ -195,6 +196,11 @@ pub struct ProjectSkillGroup { impl Global for SkillIndex {} +/// Rescan skill agent skill directories when skills are created or modified via UI +pub struct SkillsUpdatedHook(pub Rc); + +impl Global for SkillsUpdatedHook {} + /// Just the frontmatter, used for parsing #[derive(Debug, Clone, Serialize, Deserialize)] pub struct SkillMetadata { @@ -204,7 +210,7 @@ pub struct SkillMetadata { pub disable_model_invocation: bool, } -/// Minimal skill info for system prompt (not full content). +/// Minimal skill info for system prompt. /// /// `Serialize` is required for handlebars rendering of the system prompt /// template (see `ProjectContext` in `prompt_store`). `PartialEq, Eq` lets diff --git a/crates/agent_ui/Cargo.toml b/crates/agent_ui/Cargo.toml index 62ff6ac2140..0d7c076a1a5 100644 --- a/crates/agent_ui/Cargo.toml +++ b/crates/agent_ui/Cargo.toml @@ -90,7 +90,6 @@ release_channel.workspace = true remote.workspace = true remote_connection.workspace = true rope.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 882fba20922..e376cf17bdb 100644 --- a/crates/agent_ui/src/agent_panel.rs +++ b/crates/agent_ui/src/agent_panel.rs @@ -30,8 +30,8 @@ use zed_actions::{ ResolveConflictsWithAgent, ReviewBranchDiff, }, assistant::{ - CreateSkillFromUrl, FocusAgent, OpenGlobalAgentsMdRules, OpenProjectAgentsMdRules, - OpenRulesLibrary, OpenSkillCreator, Toggle, ToggleFocus, + FocusAgent, ManageSkills, OpenGlobalAgentsMdRules, OpenProjectAgentsMdRules, Toggle, + ToggleFocus, }, }; @@ -85,7 +85,7 @@ use notifications::status_toast::StatusToast; use project::{Project, ProjectPath, Worktree}; use settings::TerminalDockPosition; use settings::{NotifyWhenAgentWaiting, Settings, update_settings_file}; -use skill_creator::{SkillCreatorOpenMode, is_supported_skill_url, open_skill_creator}; + use terminal::{Event as TerminalEvent, terminal_settings::TerminalSettings}; use terminal_view::{TerminalView, terminal_panel::TerminalPanel}; use text::OffsetRangeExt; @@ -422,12 +422,10 @@ pub fn init(cx: &mut App) { }); } }) - .register_action(|workspace, action: &OpenRulesLibrary, window, cx| { + .register_action(|workspace, action: &ManageSkills, window, cx| { if let Some(panel) = workspace.panel::(cx) { workspace.focus_panel::(window, cx); - panel.update(cx, |panel, cx| { - panel.deploy_rules_library(action, window, cx) - }); + panel.update(cx, |panel, cx| panel.manage_skills(action, window, cx)); } }) .register_action(|workspace, _: &OpenGlobalAgentsMdRules, window, cx| { @@ -436,22 +434,6 @@ pub fn init(cx: &mut App) { .register_action(|workspace, _: &OpenProjectAgentsMdRules, window, cx| { open_project_rules(workspace, window, cx); }) - .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, action: &CreateSkillFromUrl, window, cx| { - if let Some(panel) = workspace.panel::(cx) { - workspace.focus_panel::(window, cx); - panel.update(cx, |panel, cx| { - panel.deploy_skill_creator_from_url(action, window, cx) - }); - } - }) .register_action(|workspace, _: &Follow, window, cx| { workspace.follow(CollaboratorId::Agent, window, cx); }) @@ -3405,90 +3387,46 @@ impl AgentPanel { self.set_base_view(thread.into(), focus, window, cx); } - fn deploy_rules_library( + fn manage_skills( &mut self, - _action: &OpenRulesLibrary, + _action: &ManageSkills, 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, - ) { - self.open_skill_creator(SkillCreatorOpenMode::Form, cx); - } - - fn deploy_skill_creator_from_url( - &mut self, - _action: &CreateSkillFromUrl, - _window: &mut Window, - cx: &mut Context, - ) { - let initial_url = cx - .read_from_clipboard() - .and_then(|clipboard| clipboard.text()) - .map(|text| text.trim().to_string()) - .filter(|text| is_supported_skill_url(text)); - - self.open_skill_creator(SkillCreatorOpenMode::Url { initial_url }, cx); - } - - /// Open the skill creator pre-filled with a skill received from a - /// `zed://skill` share link, so the user can review it and choose a scope - /// before installing. - pub fn install_shared_skill(&mut self, content: String, cx: &mut Context) { - self.open_skill_creator(SkillCreatorOpenMode::Install { content }, cx); - } - - fn open_skill_creator(&mut self, open_mode: SkillCreatorOpenMode, cx: &mut Context) { - let this = cx.weak_entity(); - let on_saved: Rc = Rc::new(move |cx: &mut App| { - this.update(cx, |this, cx| { - if !this.has_open_project(cx) { - return; - } - - this.ensure_native_agent_connection(cx); - let Some(connect_task) = this.connection_store.update(cx, |store, cx| { - store - .entry(&Agent::NativeAgent) - .map(|entry| entry.read(cx).wait_for_connection()) - }) else { - return; - }; - let project = this.project.clone(); - cx.spawn(async move |_this, cx| -> Result<()> { - let connected = connect_task.await?; - if let Some(native_connection) = connected - .connection - .downcast::() - { - cx.update(|cx| native_connection.refresh_skills_for_project(project, cx)); - } - Ok(()) - }) - .detach_and_log_err(cx); - }) - .log_err(); - }); - - open_skill_creator( - Some(self.workspace.clone()), - self.language_registry.clone(), - self.fs.clone(), - open_mode, - Some(on_saved), + window.dispatch_action( + Box::new(zed_actions::OpenSettingsAt { + path: zed_actions::AGENT_SKILLS_SETTINGS_PATH.to_string(), + target: None, + }), cx, - ) + ); + } + + /// Refresh the native agent's view of available skills + pub fn refresh_skills(&mut self, cx: &mut Context) { + if !self.has_open_project(cx) { + return; + } + + self.ensure_native_agent_connection(cx); + let Some(connect_task) = self.connection_store.update(cx, |store, cx| { + store + .entry(&Agent::NativeAgent) + .map(|entry| entry.read(cx).wait_for_connection()) + }) else { + return; + }; + let project = self.project.clone(); + cx.spawn(async move |_this, cx| -> Result<()> { + let connected = connect_task.await?; + if let Some(native_connection) = connected + .connection + .downcast::() + { + cx.update(|cx| native_connection.refresh_skills_for_project(project, cx)); + } + Ok(()) + }) .detach_and_log_err(cx); } @@ -5595,7 +5533,7 @@ impl AgentPanel { ) -> 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. + // shadow panel-level commands such as ManageSkills. let menu_action_context = match &self.base_view { BaseView::AgentThread { conversation_view } => conversation_view .read(cx) @@ -5692,28 +5630,10 @@ impl AgentPanel { }), ) .separator() - .header("Skills") - .entry( - "Create Skill…", - Some(Box::new(OpenRulesLibrary::default())), - |window, cx| { - window.dispatch_action(Box::new(OpenSkillCreator), cx); - }, - ) - .entry("Manage Skills…", None, |window, cx| { - window.dispatch_action( - Box::new(zed_actions::OpenSettingsAt { - path: "agent.skills".to_string(), - target: None, - }), - cx, - ); - }) - .separator(); + .header("Context") + .action("Skills", Box::new(ManageSkills)); if project_agents_md_path.is_some() || global_agents_md_loaded { - menu = menu.header("Rules"); - if global_agents_md_loaded { let workspace = workspace.clone(); @@ -6606,8 +6526,7 @@ impl Render for AgentPanel { this.open_configuration(window, cx); })) .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::manage_skills)) .on_action(cx.listener(Self::go_back)) .on_action(cx.listener(Self::toggle_options_menu)) .on_action(cx.listener(Self::increase_font_size)) @@ -9313,7 +9232,7 @@ mod tests { } #[gpui::test] - async fn test_skills_menu_entry_shows_rules_shortcut(cx: &mut TestAppContext) { + async fn test_skills_menu_entry_shows_manage_skills_shortcut(cx: &mut TestAppContext) { init_test(cx); cx.update(|cx| { let default_key_bindings = settings::KeymapFile::load_asset_allow_partial_failure( @@ -9357,12 +9276,12 @@ mod tests { cx.run_until_parked(); assert!( - cx.debug_bounds("MENU_ITEM-Create Skill…").is_some(), - "Create Skill… menu item should be visible" + cx.debug_bounds("MENU_ITEM-Skills").is_some(), + "Skills menu item should be visible" ); assert!( cx.debug_bounds("KEY_BINDING-l").is_some(), - "Create Skill… menu item should show the OpenRulesLibrary shortcut" + "Skills menu item should show the ManageSkills shortcut" ); } diff --git a/crates/agent_ui/src/agent_ui.rs b/crates/agent_ui/src/agent_ui.rs index a7c0170652e..46fefb9e00d 100644 --- a/crates/agent_ui/src/agent_ui.rs +++ b/crates/agent_ui/src/agent_ui.rs @@ -556,7 +556,26 @@ pub fn init( ) { agent::ThreadStore::init_global(cx); prompt_store::init(cx); - skill_creator::init(cx); + + cx.set_global(agent_skills::SkillsUpdatedHook(std::rc::Rc::new(|cx| { + let workspaces: Vec<_> = workspace::AppState::global(cx) + .workspace_store + .read(cx) + .workspaces() + .cloned() + .collect(); + + for workspace in workspaces { + workspace + .update(cx, |workspace, cx| { + if let Some(panel) = workspace.panel::(cx) { + panel.update(cx, |panel, cx| panel.refresh_skills(cx)); + } + }) + .ok(); + } + }))); + 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. @@ -763,7 +782,7 @@ fn update_command_palette_filter(cx: &mut App) { TypeId::of::(), ]; - let open_rules_library_action = [TypeId::of::()]; + let manage_skills_action = [TypeId::of::()]; let skill_creator_actions = [ TypeId::of::(), TypeId::of::(), @@ -818,16 +837,15 @@ fn update_command_palette_filter(cx: &mut App) { filter.show_namespace("multi_workspace"); } - // Hide `assistant: open rules library` — Rules are surfaced - // through the Skills UI now. Applied after the disable-ai / - // agent-enabled branches so it overrides the - // `show_namespace("assistant")` call above without affecting the - // rest of that namespace's actions. + // Hide `agent: manage skills` — skills are surfaced through the + // settings UI now. Applied after the disable-ai / agent-enabled + // branches so it overrides the `show_namespace("assistant")` call + // above without affecting the rest of that namespace's actions. if !disable_ai { - filter.hide_action_types(&open_rules_library_action); + filter.hide_action_types(&manage_skills_action); filter.show_action_types(skill_creator_actions.iter()); } else { - filter.show_action_types(open_rules_library_action.iter()); + filter.show_action_types(manage_skills_action.iter()); filter.hide_action_types(&skill_creator_actions); } }); diff --git a/crates/agent_ui/src/conversation_view/thread_view.rs b/crates/agent_ui/src/conversation_view/thread_view.rs index 539bb322031..82ee5a3b2b9 100644 --- a/crates/agent_ui/src/conversation_view/thread_view.rs +++ b/crates/agent_ui/src/conversation_view/thread_view.rs @@ -9879,7 +9879,8 @@ impl ThreadView { .on_click(move |_, window, cx| { window.dispatch_action( Box::new(zed_actions::OpenSettingsAt { - path: "agent.skills".to_string(), + path: zed_actions::AGENT_SKILLS_SETTINGS_PATH + .to_string(), target: target.clone(), }), cx, diff --git a/crates/settings_ui/Cargo.toml b/crates/settings_ui/Cargo.toml index eed98c17191..cdd1b71f033 100644 --- a/crates/settings_ui/Cargo.toml +++ b/crates/settings_ui/Cargo.toml @@ -36,6 +36,7 @@ futures.workspace = true fuzzy.workspace = true gpui.workspace = true heck.workspace = true +http_client.workspace = true itertools.workspace = true language.workspace = true log.workspace = true @@ -51,6 +52,7 @@ schemars.workspace = true search.workspace = true serde.workspace = true serde_json.workspace = true +serde_yaml_ng.workspace = true settings.workspace = true shell_command_parser.workspace = true strum.workspace = true @@ -58,6 +60,7 @@ telemetry.workspace = true theme.workspace = true theme_settings.workspace = true ui.workspace = true +ui_input.workspace = true util.workspace = true workspace.workspace = true zed_actions.workspace = true diff --git a/crates/settings_ui/src/page_data.rs b/crates/settings_ui/src/page_data.rs index fc1369e5fca..e8fc1f7248f 100644 --- a/crates/settings_ui/src/page_data.rs +++ b/crates/settings_ui/src/page_data.rs @@ -7825,7 +7825,7 @@ fn ai_page(cx: &App) -> SettingsPage { SettingsPageItem::SubPageLink(SubPageLink { title: "Skills".into(), r#type: Default::default(), - json_path: Some("agent.skills"), + json_path: Some(zed_actions::AGENT_SKILLS_SETTINGS_PATH), description: Some("View and manage agent skills installed globally or in project worktrees.".into()), in_json: false, files: USER | PROJECT, diff --git a/crates/settings_ui/src/pages.rs b/crates/settings_ui/src/pages.rs index 9ed2444b75d..7e5020779a0 100644 --- a/crates/settings_ui/src/pages.rs +++ b/crates/settings_ui/src/pages.rs @@ -2,6 +2,7 @@ mod audio_input_output_setup; mod audio_test_window; mod edit_prediction_provider_setup; mod feature_flags; +mod skill_creator; mod skills_setup; mod tool_permissions_setup; @@ -11,6 +12,10 @@ pub(crate) use audio_input_output_setup::{ pub(crate) use audio_test_window::open_audio_test_window; pub(crate) use edit_prediction_provider_setup::render_edit_prediction_setup_page; pub(crate) use feature_flags::render_feature_flags_page; +pub use skill_creator::SkillCreatorOpenMode; +pub(crate) use skill_creator::{ + SkillCreatorEvent, SkillCreatorPage, render_skill_creator_page, skill_url_from_clipboard, +}; #[cfg(test)] pub(crate) use skills_setup::displayed_skills; pub(crate) use skills_setup::render_skills_setup_page; diff --git a/crates/skill_creator/src/skill_creator.rs b/crates/settings_ui/src/pages/skill_creator.rs similarity index 70% rename from crates/skill_creator/src/skill_creator.rs rename to crates/settings_ui/src/pages/skill_creator.rs index 947302df9d9..35f9cc6d27b 100644 --- a/crates/skill_creator/src/skill_creator.rs +++ b/crates/settings_ui/src/pages/skill_creator.rs @@ -1,6 +1,6 @@ use agent_skills::{ - AGENTS_DIR_NAME, GLOBAL_SKILLS_DIR_DISPLAY, MAX_SKILL_DESCRIPTION_LEN, MAX_SKILL_FILE_SIZE, - SKILL_FILE_NAME, SKILLS_DIR_NAME, SkillMetadata, global_skills_dir, parse_skill_file_content, + AGENTS_DIR_NAME, MAX_SKILL_DESCRIPTION_LEN, MAX_SKILL_FILE_SIZE, SKILL_FILE_NAME, + SKILLS_DIR_NAME, SkillMetadata, SkillsUpdatedHook, global_skills_dir, parse_skill_file_content, slugify_skill_name, validate_description, validate_name, }; use anyhow::{Context as _, Result, anyhow}; @@ -8,28 +8,22 @@ use editor::{CurrentLineHighlight, Editor, EditorElement, EditorEvent, EditorSty use fs::Fs; use futures::AsyncReadExt; use gpui::{ - App, Bounds, Entity, FocusHandle, Focusable, ScrollHandle, Subscription, Task, TextStyle, - Tiling, TitlebarOptions, WeakEntity, WindowBounds, WindowHandle, WindowOptions, actions, point, + App, Entity, EventEmitter, FocusHandle, Focusable, ScrollHandle, Subscription, Task, TextStyle, + WeakEntity, WindowHandle, actions, }; use http_client::{AsyncBody, HttpClient, HttpRequestExt, Request, StatusCode, Url}; -use language::{Buffer, LanguageRegistry, language_settings::SoftWrap}; -use notifications::status_toast::StatusToast; -use platform_title_bar::PlatformTitleBar; -use release_channel::ReleaseChannel; +use language::{Buffer, language_settings::SoftWrap}; use settings::{ActionSequence, Settings}; use std::path::PathBuf; -use std::rc::Rc; use std::sync::Arc; use std::time::Duration; use theme_settings::ThemeSettings; -use ui::{ - Banner, ContextMenu, Divider, DropdownMenu, DropdownStyle, Headline, HeadlineSize, SwitchField, - WithScrollbar, prelude::*, -}; +use ui::{Banner, Divider, SwitchField, WithScrollbar, prelude::*}; use ui_input::{ErasedEditorEvent, InputField}; use util::ResultExt; -use workspace::{Workspace, WorkspaceSettings, client_side_decorations}; -use worktree::WorktreeId; +use workspace::MultiWorkspace; + +use crate::{SettingsUiFile, SettingsWindow, all_projects}; actions!( skill_creator, @@ -40,15 +34,11 @@ const URL_FIELD_TAB_INDEX: isize = 1; const NAME_FIELD_TAB_INDEX: isize = 2; const DESCRIPTION_FIELD_TAB_INDEX: isize = 3; const DISABLE_MODEL_INVOCATION_TAB_INDEX: isize = 4; -const SCOPE_FIELD_TAB_INDEX: isize = 5; -const BODY_FIELD_TAB_INDEX: isize = 6; -const CANCEL_BUTTON_TAB_INDEX: isize = 7; -const SAVE_BUTTON_TAB_INDEX: isize = 8; -const URL_IMPORT_DEBOUNCE: Duration = Duration::from_millis(100); +const BODY_FIELD_TAB_INDEX: isize = 5; +const SAVE_BUTTON_TAB_INDEX: isize = 6; +const URL_IMPORT_DEBOUNCE: Duration = Duration::from_millis(300); const URL_IMPORT_ERROR_BODY_MAX_LEN: usize = 2048; -pub fn init(_cx: &mut App) {} - #[derive(Clone, Debug, Default)] pub enum SkillCreatorOpenMode { #[default] @@ -56,15 +46,16 @@ pub enum SkillCreatorOpenMode { Url { initial_url: Option, }, - /// Review and install a skill whose full `SKILL.md` contents are - /// supplied inline, e.g. from a `zed://skill` share link. The form is - /// pre-filled with the parsed skill so the recipient can review it and - /// pick a scope before saving. Install { content: String, }, } +pub(crate) enum SkillCreatorEvent { + Dismissed, + Saved, +} + #[derive(Clone, Debug)] enum UrlImportStatus { Idle, @@ -80,26 +71,16 @@ struct ImportedSkill { disable_model_invocation: bool, } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq)] enum ScopeChoice { Global, Project { - worktree_id: WorktreeId, root_name: SharedString, abs_path: Arc, }, } impl ScopeChoice { - 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 { @@ -111,135 +92,56 @@ impl ScopeChoice { } } -/// 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>, +fn scope_for_settings_file( + current_file: &SettingsUiFile, + original_window: Option<&WindowHandle>, cx: &App, -) -> Vec { - let Some(workspace) = workspace.as_ref().and_then(|w| w.upgrade()) else { - return Vec::new(); - }; - let workspace = workspace.read(cx); - let root_paths = workspace.root_paths(cx); - workspace - .visible_worktrees(cx) - .zip(root_paths) - .map(|(worktree, abs_path)| { - let worktree = worktree.read(cx); - ScopeChoice::Project { - worktree_id: worktree.id(), - root_name: SharedString::from(worktree.root_name_str().to_string()), - abs_path, +) -> ScopeChoice { + if let SettingsUiFile::Project((worktree_id, _)) = current_file { + for project in all_projects(original_window, cx) { + if let Some(worktree) = project.read(cx).worktree_for_id(*worktree_id, cx) { + let worktree = worktree.read(cx); + return ScopeChoice::Project { + 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, - open_mode: SkillCreatorOpenMode, - on_saved: Option>, - cx: &mut App, -) -> Task>> { - cx.spawn(async move |cx| { - let open_mode_for_existing = open_mode.clone(); - let on_saved_for_existing = on_saved.clone(); - let existing = cx.update(|cx| { - let handle = cx - .windows() - .into_iter() - .find_map(|window| window.downcast::()); - if let Some(handle) = handle { - handle - .update(cx, |this, window, cx| { - window.activate_window(); - this.on_saved = on_saved_for_existing.clone(); - this.apply_open_mode(open_mode_for_existing.clone(), window, cx); - }) - .ok(); - Some(handle) - } else { - None - } - }); - if let Some(window) = existing { - return Ok(window); } - - let window_size = gpui::size(px(900.), px(1050.)); - // Allow the window to be resized noticeably smaller than the - // default so that the form scrolls inside the available space. - let window_min_size = gpui::size(px(500.), px(420.)); - - cx.update(|cx| { - let app_id = ReleaseChannel::global(cx).app_id(); - let http_client = cx.http_client(); - let bounds = Bounds::centered(None, 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(window_min_size), - kind: gpui::WindowKind::Floating, - ..Default::default() - }, - |window, cx| { - let skill_creator = cx.new(|cx| { - SkillCreator::new( - workspace, - language_registry, - fs, - http_client, - on_saved, - window, - cx, - ) - }); - skill_creator.update(cx, |this, cx| { - this.apply_open_mode(open_mode, window, cx); - }); - skill_creator - }, - ) - }) - }) + } + ScopeChoice::Global } -pub struct SkillCreator { +pub(crate) fn skill_url_from_clipboard(cx: &App) -> Option { + cx.read_from_clipboard() + .and_then(|clipboard| clipboard.text()) + .map(|text| text.trim().to_string()) + .filter(|text| is_supported_skill_url(text)) +} + +/// Renders the skill creator sub-page pushed by +/// [`SettingsWindow::open_skill_creator_sub_page`]. +pub(crate) fn render_skill_creator_page( + settings_window: &SettingsWindow, + _scroll_handle: &ScrollHandle, + _window: &mut Window, + _cx: &mut Context, +) -> AnyElement { + let Some(page) = settings_window.skill_creator_page() else { + return gpui::Empty.into_any_element(); + }; + page.into_any_element() +} + +pub struct SkillCreatorPage { focus_handle: FocusHandle, - title_bar: Option>, - workspace: Option>, fs: Arc, http_client: Arc, - on_saved: Option>, url_editor: Entity, name_editor: Entity, description_editor: Entity, body_editor: Entity, description_length: usize, - scopes: Vec, - selected_scope_key: SharedString, + settings_window: WeakEntity, disable_model_invocation: bool, name_error: Option<&'static str>, description_error: Option<&'static str>, @@ -247,44 +149,27 @@ pub struct SkillCreator { save_error: Option, url_import_status: UrlImportStatus, 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>, - // Held so replacing it or switching back to the form cancels a pending debounced import. url_import_debounce_task: Option>, - // Held so replacing it or switching back to the form cancels an in-flight import. url_import_task: Option>, scroll_handle: ScrollHandle, - cancel_button_focus_handle: FocusHandle, - save_button_focus_handle: FocusHandle, _subscriptions: Vec, } -impl SkillCreator { - fn new( - workspace: Option>, - language_registry: Arc, - fs: Arc, - http_client: Arc, - on_saved: Option>, +impl EventEmitter for SkillCreatorPage {} + +impl SkillCreatorPage { + pub(crate) fn new( + settings_window: WeakEntity, window: &mut Window, cx: &mut Context, ) -> Self { - let focus_handle = cx.focus_handle(); - let project_scopes = project_scopes_from_workspace(&workspace, cx); + let app_state = workspace::AppState::global(cx); + let fs = app_state.fs.clone(); + let language_registry = app_state.languages.clone(); + let http_client = cx.http_client(); - // 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 focus_handle = cx.focus_handle(); let url_editor = cx.new(|cx| { InputField::new( @@ -302,12 +187,7 @@ impl SkillCreator { .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. + // Focus the name field on open. window.focus(&name_editor.focus_handle(cx), cx); let description_editor = cx.new(|cx| { @@ -338,8 +218,6 @@ impl SkillCreator { 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(); @@ -408,22 +286,14 @@ impl SkillCreator { 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, http_client, - on_saved, url_editor, name_editor, description_editor, body_editor, description_length: 0, - scopes, - selected_scope_key, + settings_window, disable_model_invocation: false, name_error: None, description_error: None, @@ -435,12 +305,14 @@ impl SkillCreator { url_import_debounce_task: None, url_import_task: None, scroll_handle: ScrollHandle::new(), - cancel_button_focus_handle: cx.focus_handle(), - save_button_focus_handle: cx.focus_handle(), _subscriptions: subscriptions, } } + pub(crate) fn name_editor_focus_handle(&self, cx: &App) -> FocusHandle { + self.name_editor.focus_handle(cx) + } + fn handle_url_input_event( &mut self, event: &ErasedEditorEvent, @@ -520,15 +392,21 @@ impl SkillCreator { self.url_editor.read(cx).text(cx) } - fn recompute_name_error(&mut self, cx: &App) { + fn recompute_name_error(&mut self, cx: &mut Context) { let name = self.current_name(cx); - self.name_error = validate_name(&name).err(); + let error = validate_name(&name).err(); + self.name_error = error; + self.name_editor + .update(cx, |field, cx| field.set_error(error, cx)); } - fn recompute_description_error(&mut self, cx: &App) { + fn recompute_description_error(&mut self, cx: &mut Context) { let description = self.current_description(cx); self.description_length = description.len(); - self.description_error = validate_description(&description).err(); + let error = validate_description(&description).err(); + self.description_error = error; + self.description_editor + .update(cx, |field, cx| field.set_error(error, cx)); } fn recompute_body_error(&mut self, cx: &App) { @@ -544,16 +422,9 @@ impl SkillCreator { 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 apply_open_mode( + pub(crate) fn apply_open_mode( &mut self, open_mode: SkillCreatorOpenMode, window: &mut Window, @@ -705,9 +576,6 @@ impl SkillCreator { cx.notify(); } - /// Populate the form fields from a parsed skill (shared by URL import and - /// share-link install). Deferred so the programmatic `set_text` calls run - /// before focus moves to the name field. fn apply_imported_skill( &mut self, imported: ImportedSkill, @@ -749,7 +617,6 @@ impl SkillCreator { } 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); @@ -759,22 +626,23 @@ impl SkillCreator { 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; - }; - + // Resolve the scope at save time so the skill is written to whichever + // settings file is selected at the moment the user clicks Save. + let scope = self + .settings_window + .read_with(cx, |settings_window, cx| { + scope_for_settings_file( + &settings_window.current_file, + settings_window.original_window.as_ref(), + cx, + ) + }) + .unwrap_or(ScopeChoice::Global); 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_description: SharedString = match &scope { - ScopeChoice::Global => "your global skills".into(), - ScopeChoice::Project { root_name, .. } => root_name.clone(), - }; self.saving = true; self.save_error = None; @@ -791,30 +659,18 @@ impl SkillCreator { ) .await; - this.update_in(cx, |this, window, cx| { + this.update_in(cx, |this, _window, cx| { this.saving = false; this.save_task = None; match result { Ok(_) => { - if let Some(on_saved) = &this.on_saved { - on_saved(cx); + // Rescan skill directories so new skills show up in Settings page right away + if let Some(hook) = cx.try_global::() { + let hook = hook.0.clone(); + hook(cx); } - if let Some(workspace) = workspace.as_ref().and_then(|w| w.upgrade()) { - workspace.update(cx, |workspace, cx| { - let message = - format!("Saved skill \"{name}\" to {scope_description}"); - let status_toast = StatusToast::new(message, cx, |this, _cx| { - this.icon( - Icon::new(IconName::Check) - .size(IconSize::Small) - .color(Color::Success), - ) - .dismiss_button(true) - }); - workspace.toggle_status_toast(status_toast, cx); - }); - } - window.remove_window(); + + cx.emit(SkillCreatorEvent::Saved); } Err(err) => { this.save_error = Some(SharedString::from(err.to_string())); @@ -827,25 +683,12 @@ impl SkillCreator { 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. + fn cancel(&mut self, _: &Cancel, _window: &mut Window, cx: &mut Context) { + // Block dismissal while a save is in flight 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(); - } + cx.emit(SkillCreatorEvent::Dismissed); } fn toggle_disable_model_invocation(&mut self, cx: &mut Context) { @@ -866,7 +709,8 @@ impl SkillCreator { .child(self.url_editor.clone()) .child(match &self.url_import_status { UrlImportStatus::Idle => Label::new( - "Paste a GitHub .md URL. Zed will fetch it and fill out the skill form.", + "Paste a GitHub .md URL to fetch it and fill out the form. \ + For private files, Zed retries using GITHUB_TOKEN, if set.", ) .size(LabelSize::Small) .color(Color::Muted) @@ -891,11 +735,6 @@ impl SkillCreator { } fn render_form_fields(&self, window: &mut Window, cx: &mut Context) -> impl IntoElement { - // `flex_grow` lets the form fields absorb extra vertical space when - // the window is tall; `flex_shrink_0` keeps them at their natural - // (content + body min-height) size when the window is short, which - // causes the surrounding scroll container to start scrolling rather - // than squeezing the body editor below its minimum height. v_flex() .id("skill-creator-form-fields") .flex_grow_1() @@ -910,85 +749,16 @@ impl SkillCreator { ) .child(self.render_optional_params(cx)) .child(Divider::horizontal()) - .child(self.render_scope_field(window, cx)) - .child(Divider::horizontal()) .child( v_flex() .flex_grow_1() .flex_shrink_0() .gap_2() .child(Label::new("Skill Content")) - .child(self.render_body_field(window, cx)), - ) - } - - 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() - .child(Label::new(selected_label)) - .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( - 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(false), + .child(self.render_body_field(window, cx)) + .when_some(self.body_error, |this, error| { + this.child(Label::new(error).size(LabelSize::Small).color(Color::Error)) + }), ) } @@ -1063,31 +833,16 @@ impl SkillCreator { )) } - fn render_footer(&self, window: &Window, cx: &mut Context) -> impl IntoElement { - let valid = self.is_valid(cx); + fn render_footer(&self, _window: &Window, cx: &mut Context) -> impl IntoElement { let saving = self.saving; let main_action = if saving { "Saving…" } else { "Save Skill" }; - // Draw a faint outline around whichever button currently holds - // keyboard focus, so tabbing to Cancel/Save is clearly visible. The - // ring border is always present (transparent when unfocused) so - // focusing a button never shifts the surrounding layout. - let focus_ring = |focus_handle: &FocusHandle| { - let focused = focus_handle.is_focused(window) && window.last_input_was_keyboard(); - let border_color = if focused { - cx.theme().colors().border_focused - } else { - cx.theme().colors().border_transparent - }; - div().rounded_sm().border_1().border_color(border_color) - }; - v_flex() .w_full() - .p_2p5() + .py_2p5() + .px_8() .border_t_1() - .border_color(cx.theme().colors().border_variant) - .bg(cx.theme().colors().panel_background) + .border_color(cx.theme().colors().border_variant.opacity(0.4)) .when(self.save_error.is_some(), |this| { this.gap_2().child( Banner::new() @@ -1096,61 +851,24 @@ impl SkillCreator { ) }) .child( - h_flex() - .w_full() - .gap_1() - .justify_end() - .child( - focus_ring(&self.cancel_button_focus_handle).child( - Button::new("cancel-skill", "Cancel") - .track_focus( - &self - .cancel_button_focus_handle - .clone() - .tab_index(CANCEL_BUTTON_TAB_INDEX) - .tab_stop(true), - ) - .disabled(saving) - .on_click(|_, window, cx| { - window.dispatch_action(Box::new(Cancel), cx); - }), - ), - ) - .child( - focus_ring(&self.save_button_focus_handle).child( - Button::new("save-skill", main_action) - .track_focus( - &self - .save_button_focus_handle - .clone() - .tab_index(SAVE_BUTTON_TAB_INDEX) - .tab_stop(true), - ) - .style(ButtonStyle::Filled) - .layer(ui::ElevationIndex::ModalSurface) - .disabled(!valid || saving) - .loading(saving) - .on_click(|_, window, cx| { - window.dispatch_action(Box::new(SaveSkill), cx); - }), - ), - ), + h_flex().w_full().gap_1().justify_end().child( + Button::new("save-skill", main_action) + .size(ButtonSize::Medium) + .style(ButtonStyle::Outlined) + .loading(saving) + .tab_index(SAVE_BUTTON_TAB_INDEX) + // Call `save_skill` directly instead of dispatching the + // `SaveSkill` action: action dispatch follows the focused + // element's path, so a dispatched action is silently + // dropped whenever focus is outside the creator (e.g. + // right after switching the settings file/scope). + .on_click(cx.listener(|this, _, window, cx| { + this.save_skill(&SaveSkill, window, cx); + })), + ), ) } - fn render_header(&self, _window: &Window, cx: &mut Context) -> impl IntoElement { - let needs_traffic_light_clearance = cfg!(target_os = "macos"); - - h_flex() - .w_full() - .h_11() - .px_4() - .when(needs_traffic_light_clearance, |this| this.pl(px(84.))) - .border_b_1() - .border_color(cx.theme().colors().border) - .child(Headline::new("Skill Creator").size(HeadlineSize::XSmall)) - } - fn focus_next_field( &mut self, _: &FocusNextField, @@ -1169,9 +887,6 @@ impl SkillCreator { 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); } @@ -1186,69 +901,57 @@ impl SkillCreator { } } -impl Focusable for SkillCreator { +impl Focusable for SkillCreatorPage { fn focus_handle(&self, _cx: &App) -> FocusHandle { self.focus_handle.clone() } } -impl Render for SkillCreator { +impl Render for SkillCreatorPage { 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(window, cx)) - .child( - div() - .flex_1() - .min_h_0() - .w_full() - .vertical_scrollbar_for(&self.scroll_handle, window, cx) - .child( - v_flex() - .id("skill-creator-form") - .tab_index(0) - .tab_group() - .tab_stop(false) - .size_full() - .overflow_y_scroll() - .track_scroll(&self.scroll_handle) - .gap_4() - .p_4() - .child(self.render_url_import()) - .child(Divider::horizontal()) - .child(self.render_form_fields(window, cx)), - ), - ) - .child(self.render_footer(window, cx)), - window, - cx, - Tiling::default(), - ) + 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() + .child( + div() + .flex_1() + .min_h_0() + .w_full() + .vertical_scrollbar_for(&self.scroll_handle, window, cx) + .child( + v_flex() + .id("skill-creator-form") + .tab_index(0) + .tab_group() + .tab_stop(false) + .size_full() + .overflow_y_scroll() + .track_scroll(&self.scroll_handle) + .gap_4() + .px_8() + .py_4() + .child(self.render_url_import()) + .child(Divider::horizontal().flex_shrink_0().flex_grow_1()) + .child(self.render_form_fields(window, cx)), + ), + ) + .child(self.render_footer(window, cx)) } } @@ -1298,8 +1001,19 @@ async fn fetch_skill_url( raw_url: &str, github_token: Option<&str>, ) -> Result<(StatusCode, Vec)> { + // When sending the GitHub token, don't follow redirects: whether an + // `Authorization` header survives a cross-origin redirect depends on the + // underlying `HttpClient` implementation, and a redirect away from + // raw.githubusercontent.com must never carry the user's token with it. + // Authenticated raw.githubusercontent.com responses are served directly, + // so a redirect on that path is unexpected anyway. + let redirect_policy = if github_token.is_some() { + http_client::RedirectPolicy::NoFollow + } else { + http_client::RedirectPolicy::FollowAll + }; let request = Request::get(raw_url) - .follow_redirects(http_client::RedirectPolicy::FollowAll) + .follow_redirects(redirect_policy) .when_some(github_token, |builder, token| { builder.header("Authorization", format!("Bearer {token}")) }) @@ -1311,6 +1025,12 @@ async fn fetch_skill_url( .with_context(|| format!("failed to fetch {raw_url}"))?; let status = response.status(); + if github_token.is_some() && status.is_redirection() { + anyhow::bail!( + "GitHub returned an unexpected redirect ({}) for the authenticated request to {raw_url}", + status.as_u16() + ); + } let mut body = Vec::new(); response .body_mut() @@ -1342,7 +1062,7 @@ fn github_fetch_error(status: StatusCode, body: &[u8]) -> anyhow::Error { anyhow!(message) } -pub fn is_supported_skill_url(input: &str) -> bool { +pub(crate) fn is_supported_skill_url(input: &str) -> bool { github_raw_url(input).is_ok() } @@ -1570,6 +1290,7 @@ mod tests { struct TestHttpClient { responses: Mutex>, authorization_headers: Mutex>>, + redirect_policies: Mutex>, } impl TestHttpClient { @@ -1592,6 +1313,7 @@ mod tests { .collect(), ), authorization_headers: Mutex::new(Vec::new()), + redirect_policies: Mutex::new(Vec::new()), }) } @@ -1601,6 +1323,13 @@ mod tests { .expect("authorization header mutex should not be poisoned") .clone() } + + fn redirect_policies(&self) -> Vec { + self.redirect_policies + .lock() + .expect("redirect policy mutex should not be poisoned") + .clone() + } } impl HttpClient for TestHttpClient { @@ -1633,6 +1362,20 @@ mod tests { } } + let redirect_policy = req + .extensions() + .get::() + .cloned() + .unwrap_or_default(); + match self.redirect_policies.lock() { + Ok(mut redirect_policies) => redirect_policies.push(redirect_policy), + Err(_) => { + return Box::pin(async { + Err(anyhow::anyhow!("test redirect policy mutex was poisoned")) + }); + } + } + let response = match self.responses.lock() { Ok(mut responses) => responses.pop_front(), Err(_) => { @@ -1680,8 +1423,8 @@ mod tests { // 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. + // / `MAX_SKILL_DESCRIPTION_LEN`. The tests below cover the skill + // creator's own surface area: SKILL.md formatting and disk-writing. #[test] fn format_skill_file_round_trips_through_parser() { @@ -1819,6 +1562,39 @@ mod tests { client.authorization_headers(), vec![None, Some("Bearer secret-token".to_string())] ); + assert_eq!( + client.redirect_policies(), + vec![ + http_client::RedirectPolicy::FollowAll, + http_client::RedirectPolicy::NoFollow, + ], + "the authenticated retry must not follow redirects, so the token \ + can never be forwarded to another host" + ); + } + + #[gpui::test] + async fn fetch_imported_skill_rejects_redirect_on_authenticated_request( + _cx: &mut gpui::TestAppContext, + ) { + let client = TestHttpClient::new_sequence(vec![ + (404, AsyncBody::from("Not Found")), + (302, AsyncBody::from("")), + ]); + + let error = fetch_imported_skill_from_url_with_github_token( + client.clone(), + "https://github.com/owner/repo/blob/main/skill.md".to_string(), + Some("secret-token".to_string()), + ) + .await + .expect_err("a redirect on the authenticated request should be an error"); + let message = error.to_string(); + + assert!( + message.contains("unexpected redirect (302)"), + "error should report the redirect, got: {message}" + ); } #[gpui::test] @@ -1980,9 +1756,7 @@ mod tests { 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. + // Path separator differs between platforms let expected_path = Path::new("/skills").join("draft-pr"); let expected_path = expected_path.display().to_string(); assert!( diff --git a/crates/settings_ui/src/pages/skills_setup.rs b/crates/settings_ui/src/pages/skills_setup.rs index 75bbe5e317a..fbae3912c96 100644 --- a/crates/settings_ui/src/pages/skills_setup.rs +++ b/crates/settings_ui/src/pages/skills_setup.rs @@ -1,12 +1,13 @@ use agent_skills::{MAX_SKILL_DESCRIPTION_LEN, Skill, SkillIndex, encode_skill_share_link}; use fs::RemoveOptions; -use gpui::{Action as _, App, ClipboardItem, ScrollHandle, SharedString, prelude::*}; +use gpui::{App, ClipboardItem, ScrollHandle, SharedString, prelude::*}; use ui::{Divider, Tooltip, prelude::*}; use util::ResultExt as _; use std::borrow::Cow; +use crate::pages::SkillCreatorOpenMode; use crate::{SettingsUiFile, SettingsWindow}; /// Skills shown on the Skills page for the currently selected settings file: @@ -63,36 +64,26 @@ pub(crate) fn render_skills_setup_page( _ => "No skills available for this context.", }; - let original_window = settings_window.original_window; - this.px_8().items_center().justify_center().child( v_flex() .items_center() .gap_2() .child(Label::new(message).color(Color::Muted)) .child( - Button::new("open-skill-creator", "Create a Skill") + Button::new("open-skill-creator-empty", "Create a Skill") .tab_index(0_isize) .style(ButtonStyle::Outlined) - .end_icon( - Icon::new(IconName::ArrowUpRight) + .start_icon( + Icon::new(IconName::Plus) .size(IconSize::Small) .color(Color::Muted), ) - .on_click(cx.listener(move |_this, _event, window, cx| { - let Some(original_window) = original_window else { - return; - }; - original_window - .update(cx, |_workspace, original_window, cx| { - original_window.dispatch_action( - zed_actions::assistant::OpenSkillCreator - .boxed_clone(), - cx, - ); - }) - .log_err(); - window.remove_window(); + .on_click(cx.listener(move |this, _event, window, cx| { + this.open_skill_creator_sub_page( + SkillCreatorOpenMode::Form, + window, + cx, + ); })), ), ) diff --git a/crates/settings_ui/src/settings_ui.rs b/crates/settings_ui/src/settings_ui.rs index 6d5b484f5fd..b6cb1944d44 100644 --- a/crates/settings_ui/src/settings_ui.rs +++ b/crates/settings_ui/src/settings_ui.rs @@ -48,7 +48,10 @@ use workspace::{ AppState, MultiWorkspace, OpenOptions, OpenVisible, Workspace, WorkspaceSettings, client_side_decorations, }; -use zed_actions::{OpenProjectSettings, OpenSettings, OpenSettingsAt, OpenSettingsAtTarget}; +use zed_actions::{ + AGENT_SKILLS_SETTINGS_PATH, OpenProjectSettings, OpenSettings, OpenSettingsAt, + OpenSettingsAtTarget, +}; use crate::components::{ EnumVariantDropdown, NumberField, NumberFieldMode, NumberFieldType, SettingsInputField, @@ -420,6 +423,13 @@ pub fn init(cx: &mut App) { cx.on_action(|_: &OpenSettings, cx| { open_settings_editor(None, None, None, cx); }); + cx.on_action(|_: &zed_actions::assistant::OpenSkillCreator, cx| { + open_skill_creator(pages::SkillCreatorOpenMode::Form, None, cx); + }); + cx.on_action(|_: &zed_actions::assistant::CreateSkillFromUrl, cx| { + let initial_url = pages::skill_url_from_clipboard(cx); + open_skill_creator(pages::SkillCreatorOpenMode::Url { initial_url }, None, cx); + }); cx.observe_new(|workspace: &mut workspace::Workspace, _, _| { workspace @@ -449,7 +459,24 @@ pub fn init(cx: &mut App) { .then_some(tree.read(cx).id()) }); open_settings_editor(None, target_worktree_id, window_handle, cx); - }); + }) + .register_action( + |_, _: &zed_actions::assistant::OpenSkillCreator, window, cx| { + let window_handle = window.window_handle().downcast::(); + open_skill_creator(pages::SkillCreatorOpenMode::Form, window_handle, cx); + }, + ) + .register_action( + |_, _: &zed_actions::assistant::CreateSkillFromUrl, window, cx| { + let window_handle = window.window_handle().downcast::(); + let initial_url = pages::skill_url_from_clipboard(cx); + open_skill_creator( + pages::SkillCreatorOpenMode::Url { initial_url }, + window_handle, + cx, + ); + }, + ); }) .detach(); } @@ -635,8 +662,6 @@ fn open_settings_editor_at_target( workspace_handle: Option>, cx: &mut App, ) { - telemetry::event!("Settings Viewed"); - fn select_target_file( target_file: SettingsFileTarget, settings_window: &mut SettingsWindow, @@ -699,6 +724,36 @@ fn open_settings_editor_at_target( cx.notify(); } + let path = path.map(ToOwned::to_owned); + open_settings_editor_with(workspace_handle, cx, move |settings_window, window, cx| { + if let Some(target_file) = target_file { + select_target_file(target_file, settings_window, window, cx); + } + if let Some(path) = path { + open_path(&path, settings_window, window, cx); + } else if target_file.is_some() { + cx.notify(); + } + }); +} + +pub fn open_skill_creator( + open_mode: pages::SkillCreatorOpenMode, + workspace_handle: Option>, + cx: &mut App, +) { + open_settings_editor_with(workspace_handle, cx, |settings_window, window, cx| { + settings_window.navigate_to_skill_creator(open_mode, window, cx); + }); +} + +fn open_settings_editor_with( + workspace_handle: Option>, + cx: &mut App, + callback: impl FnOnce(&mut SettingsWindow, &mut Window, &mut Context) + 'static, +) { + telemetry::event!("Settings Viewed"); + let existing_window = cx .windows() .into_iter() @@ -710,21 +765,13 @@ fn open_settings_editor_at_target( settings_window.original_window = workspace_handle; window.activate_window(); - if let Some(target_file) = target_file { - select_target_file(target_file, settings_window, window, cx); - } - if let Some(path) = path { - open_path(path, settings_window, window, cx); - } else if target_file.is_some() { - cx.notify(); - } + callback(settings_window, window, cx); }) .ok(); return; } // We have to defer this to get the workspace off the stack. - let path = path.map(ToOwned::to_owned); cx.defer(move |cx| { let current_rem_size: f32 = theme_settings::ThemeSettings::get_global(cx) .ui_font_size(cx) @@ -773,12 +820,7 @@ fn open_settings_editor_at_target( let settings_window = cx.new(|cx| SettingsWindow::new(workspace_handle, window, cx)); settings_window.update(cx, |settings_window, cx| { - if let Some(target_file) = target_file { - select_target_file(target_file, settings_window, window, cx); - } - if let Some(path) = path { - open_path(&path, settings_window, window, cx); - } + callback(settings_window, window, cx); }); settings_window @@ -845,6 +887,7 @@ pub struct SettingsWindow { /// Directory path of the skill whose share link was most recently copied, /// used to show a transient "copied" checkmark on its share button. pub(crate) last_copied_skill_directory_path: Option, + skill_creator_page: Option<(Entity, Subscription)>, } struct SearchDocument { @@ -1472,6 +1515,7 @@ impl PartialEq for SettingItem { #[derive(Clone, PartialEq, Default)] enum SubPageType { Language, + SkillCreator, #[default] Other, } @@ -1819,6 +1863,7 @@ impl SettingsWindow { list_state, last_copied_link_path: None, last_copied_skill_directory_path: None, + skill_creator_page: None, }; this.fetch_files(window, cx); @@ -3513,6 +3558,8 @@ impl SettingsWindow { let page_content; if let Some(current_sub_page) = self.sub_page_stack.last() { + let is_skills_page = + current_sub_page.link.json_path == Some(AGENT_SKILLS_SETTINGS_PATH); page_header = h_flex() .w_full() .min_w_0() @@ -3532,23 +3579,39 @@ impl SettingsWindow { ) .child(self.render_sub_page_breadcrumbs(window, cx)), ) - .when(current_sub_page.link.in_json, |this| { - this.child( - div().flex_shrink_0().child( - Button::new("open-in-settings-file", "Edit in settings.json") - .tab_index(0_isize) - .style(ButtonStyle::OutlinedGhost) - .tooltip(Tooltip::for_action_title_in( - "Edit in settings.json", - &OpenCurrentFile, - &self.focus_handle, - )) - .on_click(cx.listener(|this, _, window, cx| { - this.open_current_settings_file(window, cx); - })), - ), - ) - }) + .child( + div() + .flex_shrink_0() + .when(current_sub_page.link.in_json, |this| { + this.child( + Button::new("open-in-settings-file", "Edit in settings.json") + .tab_index(0_isize) + .style(ButtonStyle::OutlinedGhost) + .tooltip(Tooltip::for_action_title_in( + "Edit in settings.json", + &OpenCurrentFile, + &self.focus_handle, + )) + .on_click(cx.listener(|this, _, window, cx| { + this.open_current_settings_file(window, cx); + })), + ) + }) + .when(is_skills_page, |this| { + this.child( + Button::new("open-skill-creator", "Create Skill") + .tab_index(0_isize) + .style(ButtonStyle::OutlinedGhost) + .on_click(cx.listener(|this, _, window, cx| { + this.open_skill_creator_sub_page( + pages::SkillCreatorOpenMode::Form, + window, + cx, + ); + })), + ) + }), + ) .into_any_element(); let active_page_render_fn = ¤t_sub_page.link.render; @@ -3973,6 +4036,101 @@ impl SettingsWindow { self.push_sub_page(sub_page_link, section_header.into(), window, cx); } + pub(crate) fn skill_creator_page(&self) -> Option> { + self.skill_creator_page + .as_ref() + .map(|(page, _)| page.clone()) + } + + /// If the creator is already the active sub-page, the open mode is applied + /// to the existing form instead + pub fn open_skill_creator_sub_page( + &mut self, + open_mode: pages::SkillCreatorOpenMode, + window: &mut Window, + cx: &mut Context, + ) { + let creator_is_active_sub_page = self + .sub_page_stack + .last() + .is_some_and(|sub_page| sub_page.link.r#type == SubPageType::SkillCreator); + + if creator_is_active_sub_page && let Some((page, _)) = &self.skill_creator_page { + let page = page.clone(); + page.update(cx, |page, cx| page.apply_open_mode(open_mode, window, cx)); + return; + } + + let settings_window = cx.weak_entity(); + let page = cx.new(|cx| pages::SkillCreatorPage::new(settings_window, window, cx)); + + let subscription = + cx.subscribe_in( + &page, + window, + |this, _page, event: &pages::SkillCreatorEvent, window, cx| match event { + pages::SkillCreatorEvent::Dismissed | pages::SkillCreatorEvent::Saved => { + if this.sub_page_stack.last().is_some_and(|sub_page| { + sub_page.link.r#type == SubPageType::SkillCreator + }) { + this.pop_sub_page(window, cx); + } + } + }, + ); + + self.skill_creator_page = Some((page.clone(), subscription)); + + let sub_page_link = SubPageLink { + title: "Create Skill".into(), + r#type: SubPageType::SkillCreator, + description: None, + json_path: None, + in_json: false, + files: USER | PROJECT, + render: pages::render_skill_creator_page, + }; + + self.push_sub_page(sub_page_link, "Agent".into(), window, cx); + + let creating_from_url = !matches!(open_mode, pages::SkillCreatorOpenMode::Url { .. }); + page.update(cx, |page, cx| { + page.apply_open_mode(open_mode, window, cx); + }); + if creating_from_url { + let name_editor_focus_handle = page.read(cx).name_editor_focus_handle(cx); + window.focus(&name_editor_focus_handle, cx); + } + } + + pub fn navigate_to_skill_creator( + &mut self, + open_mode: pages::SkillCreatorOpenMode, + window: &mut Window, + cx: &mut Context, + ) { + self.sub_page_stack.clear(); + let skills_page_index = self.pages.iter().position(|page| { + page.items.iter().any(|item| { + matches!( + item, + SettingsPageItem::SubPageLink(link) + if link.json_path == Some(AGENT_SKILLS_SETTINGS_PATH) + ) + }) + }); + if let Some(page_index) = skills_page_index + && let Some(navbar_entry_index) = self + .navbar_entries + .iter() + .position(|entry| entry.page_index == page_index && entry.is_root) + { + self.open_navbar_entry_page(navbar_entry_index); + } + self.navigate_to_sub_page(AGENT_SKILLS_SETTINGS_PATH, window, cx); + self.open_skill_creator_sub_page(open_mode, window, cx); + } + /// Navigate to a sub-page by its json_path. /// Returns true if the sub-page was found and pushed, false otherwise. pub fn navigate_to_sub_page( @@ -4046,7 +4204,11 @@ impl SettingsWindow { fn pop_sub_page(&mut self, window: &mut Window, cx: &mut Context) { self.regex_validation_error = None; - self.sub_page_stack.pop(); + if let Some(popped) = self.sub_page_stack.pop() + && popped.link.r#type == SubPageType::SkillCreator + { + self.skill_creator_page = None; + } self.content_focus_handle.focus_handle(cx).focus(window, cx); cx.notify(); } @@ -4202,7 +4364,7 @@ impl Render for SettingsWindow { } } -fn all_projects( +pub(crate) fn all_projects( window: Option<&WindowHandle>, cx: &App, ) -> impl Iterator> { @@ -4857,6 +5019,7 @@ pub mod test { regex_validation_error: None, last_copied_link_path: None, last_copied_skill_directory_path: None, + skill_creator_page: None, } } } @@ -4985,6 +5148,7 @@ pub mod test { regex_validation_error: None, last_copied_link_path: None, last_copied_skill_directory_path: None, + skill_creator_page: None, }; settings_window.build_filter_table(); @@ -5739,7 +5903,7 @@ pub mod test { assert_eq!(settings_window.current_file, SettingsUiFile::User); assert!( - settings_window.navigate_to_sub_page("agent.skills", window, cx), + settings_window.navigate_to_sub_page(AGENT_SKILLS_SETTINGS_PATH, window, cx), "Skills sub-page should exist" ); assert_eq!(displayed_skill_names(settings_window, cx), ["global-skill"]); @@ -5778,6 +5942,184 @@ pub mod test { assert_eq!(displayed_skill_names(settings_window, cx), ["global-skill"]); }); } + + #[gpui::test] + async fn test_open_skill_creator_navigates_to_sub_page(cx: &mut gpui::TestAppContext) { + use project::Project; + + cx.update(|cx| { + register_settings(cx); + }); + + let app_state = cx.update(|cx| { + let app_state = AppState::test(cx); + AppState::set_global(app_state.clone(), cx); + app_state + }); + + app_state + .fs + .as_fake() + .insert_tree("/project", serde_json::json!({ "main.rs": "fn main() {}" })) + .await; + + let project = cx.update(|cx| { + Project::local( + app_state.client.clone(), + app_state.node_runtime.clone(), + app_state.user_store.clone(), + app_state.languages.clone(), + app_state.fs.clone(), + None, + project::LocalProjectFlags::default(), + cx, + ) + }); + project + .update(cx, |project, cx| { + project.find_or_create_worktree("/project", true, cx) + }) + .await + .expect("Failed to create worktree"); + + let (_multi_workspace, cx) = cx.add_window_view(|window, cx| { + let workspace = cx.new(|cx| { + Workspace::new( + Default::default(), + project.clone(), + app_state.clone(), + window, + cx, + ) + }); + MultiWorkspace::new(workspace, window, cx) + }); + let workspace_handle = cx.window_handle().downcast::().unwrap(); + + cx.run_until_parked(); + + let (settings_window, cx) = cx + .add_window_view(|window, cx| SettingsWindow::new(Some(workspace_handle), window, cx)); + + cx.run_until_parked(); + + settings_window.update_in(cx, |settings_window, window, cx| { + settings_window.navigate_to_skill_creator( + pages::SkillCreatorOpenMode::Form, + window, + cx, + ); + }); + + cx.run_until_parked(); + + settings_window.read_with(cx, |settings_window, _| { + let titles: Vec<_> = settings_window + .sub_page_stack + .iter() + .map(|sub_page| sub_page.link.title.to_string()) + .collect(); + assert_eq!( + titles, + ["Skills", "Create Skill"], + "skill creator should be pushed on top of the skills page" + ); + assert!( + settings_window.skill_creator_page().is_some(), + "skill creator page state should exist" + ); + }); + } + + #[gpui::test] + async fn test_open_skill_creator_action_opens_settings_window_at_sub_page( + cx: &mut gpui::TestAppContext, + ) { + use project::Project; + + cx.update(|cx| { + register_settings(cx); + release_channel::init("0.0.0".parse().unwrap(), cx); + crate::init(cx); + }); + + let app_state = cx.update(|cx| { + let app_state = AppState::test(cx); + AppState::set_global(app_state.clone(), cx); + app_state + }); + + app_state + .fs + .as_fake() + .insert_tree("/project", serde_json::json!({ "main.rs": "fn main() {}" })) + .await; + + let project = cx.update(|cx| { + Project::local( + app_state.client.clone(), + app_state.node_runtime.clone(), + app_state.user_store.clone(), + app_state.languages.clone(), + app_state.fs.clone(), + None, + project::LocalProjectFlags::default(), + cx, + ) + }); + project + .update(cx, |project, cx| { + project.find_or_create_worktree("/project", true, cx) + }) + .await + .expect("Failed to create worktree"); + + let (multi_workspace, cx) = cx.add_window_view(|window, cx| { + let workspace = cx.new(|cx| { + Workspace::new( + Default::default(), + project.clone(), + app_state.clone(), + window, + cx, + ) + }); + MultiWorkspace::new(workspace, window, cx) + }); + + cx.run_until_parked(); + + // Dispatch the action the way the command palette does: on the + // workspace window. + multi_workspace.update_in(cx, |_multi_workspace, window, cx| { + window.dispatch_action(Box::new(zed_actions::assistant::OpenSkillCreator), cx); + }); + + cx.run_until_parked(); + + let settings_window = cx + .update(|_, cx| { + cx.windows() + .into_iter() + .find_map(|window| window.downcast::()) + }) + .expect("dispatching agent::OpenSkillCreator should open the settings window"); + + settings_window + .read_with(cx, |settings_window, _| { + let titles: Vec<_> = settings_window + .sub_page_stack + .iter() + .map(|sub_page| sub_page.link.title.to_string()) + .collect(); + assert_eq!( + titles, + ["Skills", "Create Skill"], + "skill creator should be pushed on top of the skills page" + ); + }) + .unwrap(); + } } #[cfg(test)] diff --git a/crates/skill_creator/Cargo.toml b/crates/skill_creator/Cargo.toml deleted file mode 100644 index 8512c4d50b2..00000000000 --- a/crates/skill_creator/Cargo.toml +++ /dev/null @@ -1,39 +0,0 @@ -[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 -futures.workspace = true -gpui.workspace = true -http_client.workspace = true -language.workspace = true -menu.workspace = true -notifications.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 deleted file mode 120000 index 89e542f750c..00000000000 --- a/crates/skill_creator/LICENSE-GPL +++ /dev/null @@ -1 +0,0 @@ -../../LICENSE-GPL \ No newline at end of file diff --git a/crates/ui_input/src/input_field.rs b/crates/ui_input/src/input_field.rs index b384fd388ed..e1c51384758 100644 --- a/crates/ui_input/src/input_field.rs +++ b/crates/ui_input/src/input_field.rs @@ -41,6 +41,9 @@ pub struct InputField { tab_stop: bool, /// Whether the field content is masked (for sensitive fields like passwords or API keys). masked: Option, + /// An optional validation error. When set, the field's border turns red + /// and the message is shown as hint subtext below the field. + error: Option, } impl Focusable for InputField { @@ -67,6 +70,7 @@ impl InputField { tab_index: None, tab_stop: true, masked: None, + error: None, } } @@ -106,6 +110,14 @@ impl InputField { self } + /// Sets a validation error message, turning the field's border red and + /// showing the message as hint subtext below the field. Pass `None` to + /// clear the error. + pub fn set_error(&mut self, error: Option>, cx: &mut Context) { + self.error = error.map(Into::into); + cx.notify(); + } + pub fn is_empty(&self, cx: &App) -> bool { self.editor().text(cx).trim().is_empty() } @@ -149,6 +161,9 @@ impl Render for InputField { let focus_handle = self.editor.focus_handle(cx); + let has_error = self.error.is_some(); + let error_border = cx.theme().status().error_border; + let configured_handle = if let Some(tab_index) = self.tab_index { focus_handle.tab_index(tab_index).tab_stop(self.tab_stop) } else if !self.tab_stop { @@ -186,6 +201,7 @@ impl Render for InputField { editor.focus_handle(cx).contains_focused(window, cx), |this| this.border_color(theme_color.border_focused), ) + .when(has_error, |this| this.border_color(error_border)) .when_some(self.start_icon, |this, icon| { this.gap_1() .child(Icon::new(icon).size(IconSize::Small).color(Color::Muted)) @@ -216,6 +232,9 @@ impl Render for InputField { ) }), ) + .when_some(self.error.clone(), |this, error| { + this.child(Label::new(error).size(LabelSize::Small).color(Color::Error)) + }) } } diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 100b01f1a9b..01d2c7b08fe 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -1150,19 +1150,12 @@ fn handle_open_request(request: OpenRequest, app_state: Arc, cx: &mut let multi_workspace = workspace::get_any_active_multi_workspace(app_state, cx.clone()).await?; - multi_workspace.update(cx, |multi_workspace, window, cx| { - multi_workspace.workspace().update(cx, |workspace, cx| { - if let Some(panel) = workspace.focus_panel::(window, cx) { - panel.update(cx, |panel, cx| { - panel.install_shared_skill(content, cx); - }); - } else { - log::warn!( - "zed://skill received but the AgentPanel is not registered \ - (is `disable_ai` enabled?)" - ); - } - }); + multi_workspace.update(cx, |_multi_workspace, _window, cx| { + settings_ui::open_skill_creator( + settings_ui::pages::SkillCreatorOpenMode::Install { content }, + Some(multi_workspace), + cx, + ); }) }) .detach_and_log_err(cx); diff --git a/crates/zed_actions/Cargo.toml b/crates/zed_actions/Cargo.toml index 9da1f323a27..1e368745412 100644 --- a/crates/zed_actions/Cargo.toml +++ b/crates/zed_actions/Cargo.toml @@ -13,4 +13,3 @@ gpui.workspace = true schemars.workspace = true serde.workspace = true util.workspace = true -uuid.workspace = true diff --git a/crates/zed_actions/src/lib.rs b/crates/zed_actions/src/lib.rs index b20caaa5321..8ff04471dd0 100644 --- a/crates/zed_actions/src/lib.rs +++ b/crates/zed_actions/src/lib.rs @@ -149,6 +149,9 @@ pub struct OpenSettingsAt { pub target: Option, } +/// `OpenSettingsAt` path of the agent skills page in the settings UI. +pub const AGENT_SKILLS_SETTINGS_PATH: &str = "agent.skills"; + #[derive(PartialEq, Clone, Debug, Deserialize, JsonSchema)] #[serde(rename_all = "snake_case")] pub enum OpenSettingsAtTarget { @@ -577,7 +580,6 @@ pub mod assistant { use gpui::{Action, actions}; use schemars::JsonSchema; use serde::Deserialize; - use uuid::Uuid; actions!( agent, @@ -597,18 +599,12 @@ pub mod assistant { /// Opens the project AGENTS.md rules file. #[action(name = "OpenProjectAGENTS.mdRules")] OpenProjectAgentsMdRules, + /// Opens the skills manager in the settings window. + #[action(deprecated_aliases = ["agent::OpenRulesLibrary", "assistant::OpenRulesLibrary", "assistant::DeployPromptLibrary"])] + ManageSkills, ] ); - /// Opens the rules library for managing agent rules and prompts. - #[derive(PartialEq, Clone, Default, Debug, Deserialize, JsonSchema, Action)] - #[action(namespace = agent, deprecated_aliases = ["assistant::OpenRulesLibrary", "assistant::DeployPromptLibrary"])] - #[serde(deny_unknown_fields)] - pub struct OpenRulesLibrary { - #[serde(skip)] - pub prompt_to_select: Option, - } - /// Deploys the assistant interface with the specified configuration. #[derive(Clone, Default, Deserialize, PartialEq, JsonSchema, Action)] #[action(namespace = assistant)] diff --git a/docs/.doc-examples/complex-feature.md b/docs/.doc-examples/complex-feature.md index dd44d9a0312..0fffe67c7e4 100644 --- a/docs/.doc-examples/complex-feature.md +++ b/docs/.doc-examples/complex-feature.md @@ -242,10 +242,17 @@ See [Feature-specific models](./ai/agent-settings.md#feature-specific-models) fo } ``` -To customize the format of generated commit messages, run {#action agent::OpenRulesLibrary} and select the "Commit message" rule on the left side. -From there, you can modify the prompt to match your desired format. +To add custom instructions that apply only to commit message generation, use the `commit_message_instructions` field in your agent settings: -Any specific instructions for commit messages added to [Rules files](./ai/rules.md) are also picked up by the model tasked with writing your commit message. +```json [settings] +{ + "agent": { + "commit_message_instructions": "Use the Conventional Commits format: (): ." + } +} +``` + +These instructions are sent to the model in addition to any instruction files, such as `.rules` or `AGENTS.md`. To add instructions that apply to both commit messages and the agent more broadly, use the global `AGENTS.md` file located `~/.config/zed/AGENTS.md` on macOS and Linux, `%APPDATA%\Zed\AGENTS.md` on Windows. ## Git Integrations {#git-integrations} diff --git a/docs/src/ai/skills.md b/docs/src/ai/skills.md index 8e25a113dfb..db9f4d6a24c 100644 --- a/docs/src/ai/skills.md +++ b/docs/src/ai/skills.md @@ -15,7 +15,7 @@ A skill is a folder containing a `SKILL.md` file with metadata and instructions. Zed includes a built-in `create-skill` skill — invoke it with `/create-skill` and the agent walks you through the process. -You can also open the Skill Creator from the Agent Panel using {#kb agent::OpenRulesLibrary}, or by clicking `...` and selecting **Skills**. Outside the panel, use the {#action agent::OpenSkillCreator} action from the command palette. It opens a window where you fill in the skill's name, description, scope (global or project-local), body, and optionally toggle `disable-model-invocation`. +You can also open the Skills Manager from the Agent Panel using {#kb agent::ManageSkills}, or by clicking `...` and selecting **Skills**. Outside the panel, use the {#action agent::OpenSkillCreator} action from the command palette, or click **Create Skill** on the **AI > Skills** settings page. The creator opens as a page in the settings window where you fill in the skill's name, description, body, and optionally toggle `disable-model-invocation`. The skill is saved to the scope of the settings file selected in the settings window — the **User** tab creates a global skill, while a **Project** tab creates a project-local skill — and the form shows exactly where the file will be written. Lastly, it's also possible to add a skill through importing it from an existing GitHub Markdown file. Open the command palette and look for the {#action agent::CreateSkillFromUrl} action. If your clipboard contains a supported GitHub `.md` URL, Zed pre-fills and fetches it automatically. @@ -35,7 +35,7 @@ To install a skill, copy the skill's folder into `~/.agents/skills/` for global Open the Settings Editor (`Cmd+,` on macOS, `Ctrl+,` on Linux/Windows) and navigate to **AI > Skills**, or go directly to [agent.skills](zed://settings/agent.skills). -The **User** tab shows your global skills. The **Project** tab shows skills for the current project. +The **User** tab shows your global skills, and each **Project** tab shows the skills for that project. For each skill you can: @@ -43,13 +43,16 @@ For each skill you can: - **Open** — opens the skill's `SKILL.md` file in the editor - **Delete** — removes the skill folder from disk -If no skills are installed, the page shows a **Create a Skill** button that opens the Skill Creator. +In the skills page, you'll see a **Create Skill** button that opens the settings window, providing the ability to create a skill directly through the UI. ## Sharing Skills {#sharing-skills} -You can hand a skill to a teammate without hosting it anywhere. In the Skills settings page, click the **link** icon on a skill row to copy a `zed://skill?data=…` link to your clipboard. The link is self-contained: it embeds the full `SKILL.md` contents (base64url-encoded), so the recipient doesn't need access to your project or any registry. +You can hand a skill to a teammate without hosting it anywhere. In the Skills settings page, click the **link** icon on a skill row to copy a `zed://skill?data=…` link to your clipboard. +The link is self-contained: it embeds the full `SKILL.md` contents (base64url-encoded), so the recipient doesn't need access to your project or any registry. -When someone opens that link (for example by pasting it into their browser or clicking it in a chat), Zed launches the Skill Creator pre-filled with the shared skill. The recipient can review the name, description, and full body, choose a scope (global or project-local), and click **Save** to install it. Nothing is written to disk until they explicitly save, so a shared link can never silently install instructions into someone's agent. +When someone opens that link (for example by pasting it into their browser or clicking it in a chat), Zed opens the "Create Skill" page in the settings window, pre-filled with the shared skill. +The recipient can review the name, description, and full body, choose a scope by selecting the **User** tab (global) or a **Project** tab, and click **Save** to install it. +Nothing is written to disk until they explicitly save, so a shared link can never silently install instructions into someone's agent. ## Using Skills {#using-skills}