From 2e70059cd980e42dadcb0677e56031931b4940cc Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 19 May 2026 20:28:09 -0400 Subject: [PATCH] Remove Skills feature flag (#57162) Removes the `skills` feature flag now that Skills are ready to ship to all users. Cleans up all `cx.has_flag::()` gates across the agent, agent_ui, prompt_store, and title_bar crates, drops the now-unused `feature_flags` dependency from `prompt_store` and `title_bar`, and removes the dead `update_flags(true, vec!["skills"])` calls from the agent's skills tests. Closes AI-269 Release Notes: - Enabled Skills for all users. --------- Co-authored-by: MartinYe1234 <52641447+MartinYe1234@users.noreply.github.com> Co-authored-by: Martin Ye --- Cargo.lock | 2 - crates/agent/src/agent.rs | 63 +++++-------------- crates/agent_ui/src/agent_panel.rs | 12 +--- crates/agent_ui/src/agent_ui.rs | 50 ++++++--------- crates/feature_flags/src/flags.rs | 12 ---- crates/prompt_store/Cargo.toml | 2 +- .../src/rules_to_skills_migration.rs | 21 ++----- crates/title_bar/Cargo.toml | 2 +- crates/title_bar/src/title_bar.rs | 2 - 9 files changed, 43 insertions(+), 123 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bc935d54f76..31a6af6fcf9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -13688,7 +13688,6 @@ dependencies = [ "chrono", "collections", "db", - "feature_flags", "fs", "futures 0.3.32", "fuzzy", @@ -18251,7 +18250,6 @@ dependencies = [ "client", "cloud_api_types", "db", - "feature_flags", "fs", "git_ui", "gpui", diff --git a/crates/agent/src/agent.rs b/crates/agent/src/agent.rs index 09a13b38818..f704e997ac0 100644 --- a/crates/agent/src/agent.rs +++ b/crates/agent/src/agent.rs @@ -38,7 +38,7 @@ use agent_skills::{ use anyhow::{Context as _, Result, anyhow}; use chrono::{DateTime, Utc}; use collections::{HashMap, HashSet, IndexMap}; -use feature_flags::{FeatureFlagAppExt as _, SkillsFeatureFlag}; + use fs::Fs; use futures::channel::{mpsc, oneshot}; use futures::future::Shared; @@ -392,11 +392,11 @@ impl NativeAgent { /// Kicks off a one-time scan of the global skills directory if one /// isn't already in progress and a watch isn't already active. /// - /// Idempotent and cheap: returns immediately if the user lacks the - /// skills feature flag, or if a scan or watch is already running. - /// The expected callers are user-interaction events from the agent - /// panel (input focus, slash autocomplete, conversation submit); - /// firing this from any of them is equivalent and safe to repeat. + /// Idempotent and cheap: returns immediately if a scan or watch is + /// already running. The expected callers are user-interaction events + /// from the agent panel (input focus, slash autocomplete, conversation + /// submit); firing this from any of them is equivalent and safe to + /// repeat. /// /// The scan itself runs detached on the foreground executor. If /// `~/.agents/skills/` exists it transitions state to @@ -405,9 +405,6 @@ impl NativeAgent { /// next trigger retries (covering the case where the user creates /// the directory after the first scan). pub fn ensure_skills_scan_started(&mut self, cx: &mut Context) { - if !cx.has_flag::() { - return; - } if !matches!(self.skills_state, SkillsState::Idle) { return; } @@ -598,12 +595,10 @@ impl NativeAgent { // after the thread is constructed are still visible to the // model — without this, the catalog and tool would drift out // of sync until the session was reopened. - if cx.has_flag::() { - thread.add_tool(SkillTool::new( - skills_resolver_for_project(weak.clone(), project_id), - self.fs.clone(), - )); - } + thread.add_tool(SkillTool::new( + skills_resolver_for_project(weak.clone(), project_id), + self.fs.clone(), + )); }); let subscriptions = vec![ @@ -822,12 +817,8 @@ impl NativeAgent { }) .collect::>(); - // Skills are gated behind the "skills" feature flag. Without it we - // skip all on-disk lookups so users see no behavior change. - let skills_enabled = cx.has_flag::(); - // Load global skills - let global_skills_task = if skills_enabled { + let global_skills_task = { let global_skills_dir = global_skills_dir(); let global_skills_fs = fs.clone(); cx.background_spawn(async move { @@ -838,8 +829,6 @@ impl NativeAgent { ) .await }) - } else { - Task::ready(Vec::new()) }; // Load project-local skills, but only from worktrees the user has @@ -852,7 +841,7 @@ impl NativeAgent { // worktrees pick up their skills without restarting. let trusted_worktrees = TrustedWorktrees::try_get_global(cx); let worktree_store = project.read(cx).worktree_store(); - let project_skills_task = if skills_enabled { + let project_skills_task = { let project_skills_futures: Vec< futures::future::BoxFuture<'static, Vec>>, > = worktrees @@ -897,8 +886,6 @@ impl NativeAgent { }) .collect(); cx.background_spawn(async move { future::join_all(project_skills_futures).await }) - } else { - Task::ready(Vec::new()) }; let default_user_rules_task = if let Some(prompt_store) = prompt_store.as_ref() { prompt_store.read_with(cx, |prompt_store, cx| { @@ -1107,7 +1094,7 @@ impl NativeAgent { &mut self, project: Entity, event: &project::Event, - cx: &mut Context, + _cx: &mut Context, ) { let project_id = project.entity_id(); let Some(state) = self.projects.get_mut(&project_id) else { @@ -1118,16 +1105,14 @@ impl NativeAgent { state.project_context_needs_refresh.send(()).ok(); } project::Event::WorktreeUpdatedEntries(_, items) => { - let skills_enabled = cx.has_flag::(); if items.iter().any(|(path, _, _)| { let path_ref = path.as_ref(); RULES_FILE_REL_PATHS .iter() .any(|rules_path| path_ref == rules_path.as_ref()) - || (skills_enabled - && SKILLS_PREFIX - .as_ref() - .is_some_and(|prefix| path_ref.starts_with(prefix))) + || SKILLS_PREFIX + .as_ref() + .is_some_and(|prefix| path_ref.starts_with(prefix)) }) { state.project_context_needs_refresh.send(()).ok(); } @@ -3583,9 +3568,6 @@ mod internal_tests { #[gpui::test] async fn test_global_skills_load_and_reload(cx: &mut TestAppContext) { init_test(cx); - cx.update(|cx| { - cx.update_flags(true, vec!["skills".to_string()]); - }); let fs = FakeFs::new(cx.executor()); let skills_dir = global_skills_dir(); let initial_skill_dir = skills_dir.join("my-skill"); @@ -3651,9 +3633,6 @@ mod internal_tests { #[gpui::test] async fn test_global_skills_dir_created_after_startup(cx: &mut TestAppContext) { init_test(cx); - cx.update(|cx| { - cx.update_flags(true, vec!["skills".to_string()]); - }); let fs = FakeFs::new(cx.executor()); let skills_dir = global_skills_dir(); @@ -3735,9 +3714,6 @@ mod internal_tests { #[gpui::test] async fn test_skills_added_after_session_visible_to_skill_tool(cx: &mut TestAppContext) { init_test(cx); - cx.update(|cx| { - cx.update_flags(true, vec!["skills".to_string()]); - }); let fs = FakeFs::new(cx.executor()); let skills_dir = global_skills_dir(); @@ -3879,9 +3855,6 @@ mod internal_tests { #[gpui::test] async fn test_subagent_skills_lookup_matches_parent(cx: &mut TestAppContext) { init_test(cx); - cx.update(|cx| { - cx.update_flags(true, vec!["skills".to_string()]); - }); let fs = FakeFs::new(cx.executor()); let skills_dir = global_skills_dir(); let skill_dir = skills_dir.join("shared-skill"); @@ -3982,9 +3955,6 @@ mod internal_tests { #[gpui::test] async fn test_skills_appear_as_available_skills(cx: &mut TestAppContext) { init_test(cx); - cx.update(|cx| { - cx.update_flags(true, vec!["skills".to_string()]); - }); let fs = FakeFs::new(cx.executor()); let skills_dir = global_skills_dir(); @@ -4087,7 +4057,6 @@ mod internal_tests { init_test(cx); cx.update(|cx| { - cx.update_flags(true, vec!["skills".to_string()]); // The trust global isn't created by `init_test`. We need it // for `Project::test_with_worktree_trust` to actually wire up // trust tracking and for our subscription in diff --git a/crates/agent_ui/src/agent_panel.rs b/crates/agent_ui/src/agent_panel.rs index 862b007f52c..2e91decb170 100644 --- a/crates/agent_ui/src/agent_panel.rs +++ b/crates/agent_ui/src/agent_panel.rs @@ -61,7 +61,7 @@ use collections::HashMap; use editor::{Editor, MultiBuffer}; use extension::ExtensionEvents; use extension_host::ExtensionStore; -use feature_flags::{FeatureFlagAppExt as _, SkillsFeatureFlag}; + use fs::Fs; use gpui::{ Action, Anchor, Animation, AnimationExt, AnyElement, App, AsyncWindowContext, ClipboardItem, @@ -4714,12 +4714,6 @@ impl AgentPanel { .with_handle(self.agent_panel_menu_handle.clone()) .menu({ move |window, cx| { - // When the Skills feature flag is on, hide the legacy Rules menu entry. - // The flag is read from a global store populated asynchronously, and - // this menu builder runs on every open, so the latest resolved value is - // reflected when the user clicks the ellipsis. - let skills_enabled = cx.has_flag::(); - Some(ContextMenu::build(window, cx, |mut menu, _window, _| { menu = menu.context(focus_handle.clone()); @@ -4756,10 +4750,6 @@ impl AgentPanel { .action("Add Custom Server…", Box::new(AddContextServer)) .separator(); - if !skills_enabled { - menu = menu.action("Rules", Box::new(OpenRulesLibrary::default())); - } - menu = menu.action("Profiles", Box::new(ManageProfiles::default())); } diff --git a/crates/agent_ui/src/agent_ui.rs b/crates/agent_ui/src/agent_ui.rs index f67a1d11aaf..c6ef0af0159 100644 --- a/crates/agent_ui/src/agent_ui.rs +++ b/crates/agent_ui/src/agent_ui.rs @@ -43,7 +43,7 @@ use ::ui::IconName; use agent_client_protocol::schema as acp; use agent_settings::{AgentProfileId, AgentSettings}; use command_palette_hooks::CommandPaletteFilter; -use feature_flags::{FeatureFlagAppExt as _, SkillsFeatureFlag}; +use feature_flags::FeatureFlagAppExt as _; use fs::Fs; use gpui::{ Action, App, Context, Entity, ImageSource, Resource, SharedString, SharedUri, Window, actions, @@ -560,21 +560,7 @@ pub fn init( _: &zed_actions::agent::OpenRulesToSkillsMigrationInfo, window: &mut Window, cx: &mut Context| { - // The banner is the only intended entry point and is - // gated on the skills flag, but dispatch from the - // command palette or a keybind is still possible — only - // open the explainer if the flag is enabled so it never - // surfaces outside its intended audience. - // - // Race note: `has_flag` returns false before server - // flags are received, so a dispatch during that brief - // window is a no-op even for users who genuinely have - // the flag. The banner itself has the same race — it - // stays hidden until flags arrive — so a user who can - // see the banner has, by definition, already passed it. - if cx.has_flag::() { - crate::ui::RulesToSkillsModal::toggle(workspace, window, cx); - } + crate::ui::RulesToSkillsModal::toggle(workspace, window, cx); }, ); }) @@ -608,10 +594,16 @@ pub fn init( }) .detach(); - // Once the `skills` feature flag has resolved, kick off the one-time - // migration of non-Default Rules to global Skills. Idempotent and - // self-gated on the flag, so it's safe to call on every flag-ready - // notification (and a no-op for users without the flag). + // Kick off the one-time migration of non-Default Rules to global + // Skills, deferred until server feature flags arrive. + // + // The migration itself is idempotent and no longer gated on a flag, + // but the deferral via `on_flags_ready` still matters: the migration + // writes to the on-disk `GlobalKeyValueStore`, which dispatches work + // on the `sqlezWorker` background thread. In `gpui::test` contexts, + // server flags are never received, so this callback never fires — + // which keeps that sqlite worker activity from racing with the + // `TestScheduler` and tripping its non-determinism panic. { let fs = fs.clone(); cx.on_flags_ready(move |_, cx| { @@ -653,12 +645,6 @@ fn update_command_palette_filter(cx: &mut App) { .edit_predictions .provider; - // The Skills feature flag is loaded asynchronously, so this value may - // be `false` before flags resolve. `update_command_palette_filter` - // gets re-run from `cx.on_flags_ready` (see `init`), which means the - // filter is reapplied with the correct value once flags arrive. - let skills_enabled = cx.has_flag::(); - CommandPaletteFilter::update_global(cx, |filter, _| { use editor::actions::{ AcceptEditPrediction, AcceptNextLineEditPrediction, AcceptNextWordEditPrediction, @@ -726,12 +712,12 @@ fn update_command_palette_filter(cx: &mut App) { filter.show_namespace("multi_workspace"); } - // Hide `assistant: open rules library` when Skills are enabled — - // Rules are surfaced through the Skills UI in that case. 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 && skills_enabled { + // 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. + if !disable_ai { filter.hide_action_types(&open_rules_library_action); } else { filter.show_action_types(open_rules_library_action.iter()); diff --git a/crates/feature_flags/src/flags.rs b/crates/feature_flags/src/flags.rs index bfae995f9f2..9f3c54394b9 100644 --- a/crates/feature_flags/src/flags.rs +++ b/crates/feature_flags/src/flags.rs @@ -123,15 +123,3 @@ impl FeatureFlag for AutoWatchFeatureFlag { type Value = PresenceFlag; } register_feature_flag!(AutoWatchFeatureFlag); - -pub struct SkillsFeatureFlag; - -impl FeatureFlag for SkillsFeatureFlag { - const NAME: &'static str = "skills"; - type Value = PresenceFlag; - - fn enabled_for_staff() -> bool { - true - } -} -register_feature_flag!(SkillsFeatureFlag); diff --git a/crates/prompt_store/Cargo.toml b/crates/prompt_store/Cargo.toml index 91bcdd251bf..8c1f296f171 100644 --- a/crates/prompt_store/Cargo.toml +++ b/crates/prompt_store/Cargo.toml @@ -18,7 +18,7 @@ assets.workspace = true chrono.workspace = true collections.workspace = true db.workspace = true -feature_flags.workspace = true + fs.workspace = true futures.workspace = true fuzzy.workspace = true diff --git a/crates/prompt_store/src/rules_to_skills_migration.rs b/crates/prompt_store/src/rules_to_skills_migration.rs index 13dba07207d..7fad0ab389b 100644 --- a/crates/prompt_store/src/rules_to_skills_migration.rs +++ b/crates/prompt_store/src/rules_to_skills_migration.rs @@ -24,14 +24,10 @@ //! (still using Zed's shipped default content) are skipped so we don't //! pollute AGENTS.md with text the user never wrote. //! -//! Both migrations are gated by: -//! -//! * the `skills` feature flag — users without it never have their Rules -//! touched in any way; -//! * a single global "migration already ran" flag persisted in -//! [`GlobalKeyValueStore`] — keyed by [`MIGRATION_DONE_KEY`], so a -//! shared home directory only gets populated once per machine even -//! across release channels. +//! Both migrations are gated by a single global "migration already ran" +//! flag persisted in [`GlobalKeyValueStore`] — keyed by +//! [`MIGRATION_DONE_KEY`], so a shared home directory only gets +//! populated once per machine even across release channels. //! //! The migration is intentionally non-destructive: rule rows in the LMDB //! database are left in place after the migration. That way users can @@ -45,7 +41,6 @@ use std::sync::atomic::{AtomicBool, Ordering}; use agent_skills::{SKILL_FILE_NAME, global_skills_dir, slugify_skill_name}; use anyhow::{Context as _, Result}; use db::kvp::GlobalKeyValueStore; -use feature_flags::{FeatureFlagAppExt as _, SkillsFeatureFlag}; use fs::Fs; use gpui::{App, AsyncApp, Entity, TaskExt as _}; use serde::{Deserialize, Serialize}; @@ -151,13 +146,9 @@ static MIGRATION_TASK_SPAWNED: AtomicBool = AtomicBool::new(false); /// Migrate non-Default user rules to global Skills, if not already done. /// /// Safe to call on every startup — short-circuits immediately when the -/// migration has already run, when another invocation in this process -/// has already started it, or when the user doesn't have the `skills` -/// feature flag enabled. +/// migration has already run or when another invocation in this process +/// has already started it. pub fn migrate_rules_to_skills_if_needed(fs: Arc, cx: &mut App) { - if !cx.has_flag::() { - return; - } if migration_done() { return; } diff --git a/crates/title_bar/Cargo.toml b/crates/title_bar/Cargo.toml index 61ffd3f8175..4a762b18eef 100644 --- a/crates/title_bar/Cargo.toml +++ b/crates/title_bar/Cargo.toml @@ -40,7 +40,7 @@ chrono.workspace = true client.workspace = true cloud_api_types.workspace = true db.workspace = true -feature_flags.workspace = true + git_ui.workspace = true gpui = { workspace = true, features = ["screen-capture"] } icons.workspace = true diff --git a/crates/title_bar/src/title_bar.rs b/crates/title_bar/src/title_bar.rs index 3bc12a20748..cb2a6406994 100644 --- a/crates/title_bar/src/title_bar.rs +++ b/crates/title_bar/src/title_bar.rs @@ -25,7 +25,6 @@ use auto_update::AutoUpdateStatus; use call::ActiveCall; use client::{Client, UserStore, zed_urls}; use cloud_api_types::Plan; -use feature_flags::{FeatureFlagAppExt as _, SkillsFeatureFlag}; use gpui::{ Action, Anchor, Animation, AnimationExt, AnyElement, App, Context, Element, Entity, Focusable, @@ -469,7 +468,6 @@ impl TitleBar { zed_actions::agent::OpenRulesToSkillsMigrationInfo.boxed_clone(), cx, ) - .visible_when(|cx| cx.has_flag::()) })); let mut this = Self {