History

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::<SkillsFeatureFlag>()` 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. ---------

Richard Feldman committed 5 months ago

Commit 2e700591 parent 952119d9 files · +43 −123Browse files

Unified · Split

9 files
  1. Cargo.lock +0 −2
  2. crates/agent/src/agent.rs +16 −47
  3. crates/agent_ui/src/agent_panel.rs +1 −11
  4. crates/agent_ui/src/agent_ui.rs +18 −32
  5. crates/feature_flags/src/flags.rs +0 −12
  6. crates/prompt_store/Cargo.toml +1 −1
  7. crates/prompt_store/src/rules_to_skills_migration.rs +6 −15
  8. crates/title_bar/Cargo.toml +1 −1
  9. crates/title_bar/src/title_bar.rs +0 −2
Cargo.lock+0−2
@@ -13688,7 +13688,6 @@ dependencies = [ · 13687 unmodified lines
1368813688 "chrono",
1368913689 "collections",
1369013690 "db",
13691− "feature_flags",
1369213691 "fs",
1369313692 "futures 0.3.32",
1369413693 "fuzzy",
@@ -18251,7 +18250,6 @@ dependencies = [ · 4556 unmodified lines
1825118250 "client",
1825218251 "cloud_api_types",
1825318252 "db",
18254− "feature_flags",
1825518253 "fs",
1825618254 "git_ui",
1825718255 "gpui",
crates/agent/src/agent.rs+16−47
@@ -38,7 +38,7 @@ use agent_skills::{ · 37 unmodified lines
3838 use anyhow::{Context as _, Result, anyhow};
3939 use chrono::{DateTime, Utc};
4040 use collections::{HashMap, HashSet, IndexMap};
41−use feature_flags::{FeatureFlagAppExt as _, SkillsFeatureFlag};
41+
4242 use fs::Fs;
4343 use futures::channel::{mpsc, oneshot};
4444 use futures::future::Shared;
@@ -392,11 +392,11 @@ impl NativeAgent { · 347 unmodified lines
392392 /// Kicks off a one-time scan of the global skills directory if one
393393 /// isn't already in progress and a watch isn't already active.
394394 ///
395− /// Idempotent and cheap: returns immediately if the user lacks the
396− /// skills feature flag, or if a scan or watch is already running.
397− /// The expected callers are user-interaction events from the agent
398− /// panel (input focus, slash autocomplete, conversation submit);
399− /// firing this from any of them is equivalent and safe to repeat.
395+ /// Idempotent and cheap: returns immediately if a scan or watch is
396+ /// already running. The expected callers are user-interaction events
397+ /// from the agent panel (input focus, slash autocomplete, conversation
398+ /// submit); firing this from any of them is equivalent and safe to
399+ /// repeat.
400400 ///
401401 /// The scan itself runs detached on the foreground executor. If
402402 /// `~/.agents/skills/` exists it transitions state to
@@ -405,9 +405,6 @@ impl NativeAgent { · 2 unmodified lines
405405 /// next trigger retries (covering the case where the user creates
406406 /// the directory after the first scan).
407407 pub fn ensure_skills_scan_started(&mut self, cx: &mut Context<Self>) {
408− if !cx.has_flag::<SkillsFeatureFlag>() {
409− return;
410− }
411408 if !matches!(self.skills_state, SkillsState::Idle) {
412409 return;
413410 }
@@ -598,12 +595,10 @@ impl NativeAgent { · 184 unmodified lines
598595 // after the thread is constructed are still visible to the
599596 // model — without this, the catalog and tool would drift out
600597 // of sync until the session was reopened.
601− if cx.has_flag::<SkillsFeatureFlag>() {
602− thread.add_tool(SkillTool::new(
603− skills_resolver_for_project(weak.clone(), project_id),
604− self.fs.clone(),
605− ));
606− }
598+ thread.add_tool(SkillTool::new(
599+ skills_resolver_for_project(weak.clone(), project_id),
600+ self.fs.clone(),
601+ ));
607602 });
608603
609604 let subscriptions = vec![
@@ -822,12 +817,8 @@ impl NativeAgent { · 212 unmodified lines
822817 })
823818 .collect::<Vec<_>>();
824819
825− // Skills are gated behind the "skills" feature flag. Without it we
826− // skip all on-disk lookups so users see no behavior change.
827− let skills_enabled = cx.has_flag::<SkillsFeatureFlag>();
828−
829820 // Load global skills
830− let global_skills_task = if skills_enabled {
821+ let global_skills_task = {
831822 let global_skills_dir = global_skills_dir();
832823 let global_skills_fs = fs.clone();
833824 cx.background_spawn(async move {
@@ -838,8 +829,6 @@ impl NativeAgent { · 4 unmodified lines
838829 )
839830 .await
840831 })
841− } else {
842− Task::ready(Vec::new())
843832 };
844833
845834 // Load project-local skills, but only from worktrees the user has
@@ -852,7 +841,7 @@ impl NativeAgent { · 6 unmodified lines
852841 // worktrees pick up their skills without restarting.
853842 let trusted_worktrees = TrustedWorktrees::try_get_global(cx);
854843 let worktree_store = project.read(cx).worktree_store();
855− let project_skills_task = if skills_enabled {
844+ let project_skills_task = {
856845 let project_skills_futures: Vec<
857846 futures::future::BoxFuture<'static, Vec<Result<Skill, SkillLoadError>>>,
858847 > = worktrees
@@ -897,8 +886,6 @@ impl NativeAgent { · 38 unmodified lines
897886 })
898887 .collect();
899888 cx.background_spawn(async move { future::join_all(project_skills_futures).await })
900− } else {
901− Task::ready(Vec::new())
902889 };
903890 let default_user_rules_task = if let Some(prompt_store) = prompt_store.as_ref() {
904891 prompt_store.read_with(cx, |prompt_store, cx| {
@@ -1107,7 +1094,7 @@ impl NativeAgent { · 202 unmodified lines
11071094 &mut self,
11081095 project: Entity<Project>,
11091096 event: &project::Event,
1110− cx: &mut Context<Self>,
1097+ _cx: &mut Context<Self>,
11111098 ) {
11121099 let project_id = project.entity_id();
11131100 let Some(state) = self.projects.get_mut(&project_id) else {
@@ -1118,16 +1105,14 @@ impl NativeAgent { · 4 unmodified lines
11181105 state.project_context_needs_refresh.send(()).ok();
11191106 }
11201107 project::Event::WorktreeUpdatedEntries(_, items) => {
1121− let skills_enabled = cx.has_flag::<SkillsFeatureFlag>();
11221108 if items.iter().any(|(path, _, _)| {
11231109 let path_ref = path.as_ref();
11241110 RULES_FILE_REL_PATHS
11251111 .iter()
11261112 .any(|rules_path| path_ref == rules_path.as_ref())
1127− || (skills_enabled
1128− && SKILLS_PREFIX
1129− .as_ref()
1130− .is_some_and(|prefix| path_ref.starts_with(prefix)))
1113+ || SKILLS_PREFIX
1114+ .as_ref()
1115+ .is_some_and(|prefix| path_ref.starts_with(prefix))
11311116 }) {
11321117 state.project_context_needs_refresh.send(()).ok();
11331118 }
@@ -3583,9 +3568,6 @@ mod internal_tests { · 2449 unmodified lines
35833568 #[gpui::test]
35843569 async fn test_global_skills_load_and_reload(cx: &mut TestAppContext) {
35853570 init_test(cx);
3586− cx.update(|cx| {
3587− cx.update_flags(true, vec!["skills".to_string()]);
3588− });
35893571 let fs = FakeFs::new(cx.executor());
35903572 let skills_dir = global_skills_dir();
35913573 let initial_skill_dir = skills_dir.join("my-skill");
@@ -3651,9 +3633,6 @@ mod internal_tests { · 59 unmodified lines
36513633 #[gpui::test]
36523634 async fn test_global_skills_dir_created_after_startup(cx: &mut TestAppContext) {
36533635 init_test(cx);
3654− cx.update(|cx| {
3655− cx.update_flags(true, vec!["skills".to_string()]);
3656− });
36573636 let fs = FakeFs::new(cx.executor());
36583637 let skills_dir = global_skills_dir();
36593638
@@ -3735,9 +3714,6 @@ mod internal_tests { · 75 unmodified lines
37353714 #[gpui::test]
37363715 async fn test_skills_added_after_session_visible_to_skill_tool(cx: &mut TestAppContext) {
37373716 init_test(cx);
3738− cx.update(|cx| {
3739− cx.update_flags(true, vec!["skills".to_string()]);
3740− });
37413717 let fs = FakeFs::new(cx.executor());
37423718 let skills_dir = global_skills_dir();
37433719
@@ -3879,9 +3855,6 @@ mod internal_tests { · 135 unmodified lines
38793855 #[gpui::test]
38803856 async fn test_subagent_skills_lookup_matches_parent(cx: &mut TestAppContext) {
38813857 init_test(cx);
3882− cx.update(|cx| {
3883− cx.update_flags(true, vec!["skills".to_string()]);
3884− });
38853858 let fs = FakeFs::new(cx.executor());
38863859 let skills_dir = global_skills_dir();
38873860 let skill_dir = skills_dir.join("shared-skill");
@@ -3982,9 +3955,6 @@ mod internal_tests { · 94 unmodified lines
39823955 #[gpui::test]
39833956 async fn test_skills_appear_as_available_skills(cx: &mut TestAppContext) {
39843957 init_test(cx);
3985− cx.update(|cx| {
3986− cx.update_flags(true, vec!["skills".to_string()]);
3987− });
39883958 let fs = FakeFs::new(cx.executor());
39893959 let skills_dir = global_skills_dir();
39903960
@@ -4087,7 +4057,6 @@ mod internal_tests { · 96 unmodified lines
40874057
40884058 init_test(cx);
40894059 cx.update(|cx| {
4090− cx.update_flags(true, vec!["skills".to_string()]);
40914060 // The trust global isn't created by `init_test`. We need it
40924061 // for `Project::test_with_worktree_trust` to actually wire up
40934062 // trust tracking and for our subscription in
crates/agent_ui/src/agent_panel.rs+1−11
@@ -61,7 +61,7 @@ use collections::HashMap; · 60 unmodified lines
6161 use editor::{Editor, MultiBuffer};
6262 use extension::ExtensionEvents;
6363 use extension_host::ExtensionStore;
64−use feature_flags::{FeatureFlagAppExt as _, SkillsFeatureFlag};
64+
6565 use fs::Fs;
6666 use gpui::{
6767 Action, Anchor, Animation, AnimationExt, AnyElement, App, AsyncWindowContext, ClipboardItem,
@@ -4714,12 +4714,6 @@ impl AgentPanel { · 4646 unmodified lines
47144714 .with_handle(self.agent_panel_menu_handle.clone())
47154715 .menu({
47164716 move |window, cx| {
4717− // When the Skills feature flag is on, hide the legacy Rules menu entry.
4718− // The flag is read from a global store populated asynchronously, and
4719− // this menu builder runs on every open, so the latest resolved value is
4720− // reflected when the user clicks the ellipsis.
4721− let skills_enabled = cx.has_flag::<SkillsFeatureFlag>();
4722−
47234717 Some(ContextMenu::build(window, cx, |mut menu, _window, _| {
47244718 menu = menu.context(focus_handle.clone());
47254719
@@ -4756,10 +4750,6 @@ impl AgentPanel { · 30 unmodified lines
47564750 .action("Add Custom Server…", Box::new(AddContextServer))
47574751 .separator();
47584752
4759− if !skills_enabled {
4760− menu = menu.action("Rules", Box::new(OpenRulesLibrary::default()));
4761− }
4762−
47634753 menu = menu.action("Profiles", Box::new(ManageProfiles::default()));
47644754 }
47654755
crates/agent_ui/src/agent_ui.rs+18−32
@@ -43,7 +43,7 @@ use ::ui::IconName; · 42 unmodified lines
4343 use agent_client_protocol::schema as acp;
4444 use agent_settings::{AgentProfileId, AgentSettings};
4545 use command_palette_hooks::CommandPaletteFilter;
46−use feature_flags::{FeatureFlagAppExt as _, SkillsFeatureFlag};
46+use feature_flags::FeatureFlagAppExt as _;
4747 use fs::Fs;
4848 use gpui::{
4949 Action, App, Context, Entity, ImageSource, Resource, SharedString, SharedUri, Window, actions,
@@ -560,21 +560,7 @@ pub fn init( · 510 unmodified lines
560560 _: &zed_actions::agent::OpenRulesToSkillsMigrationInfo,
561561 window: &mut Window,
562562 cx: &mut Context<Workspace>| {
563− // The banner is the only intended entry point and is
564− // gated on the skills flag, but dispatch from the
565− // command palette or a keybind is still possible — only
566− // open the explainer if the flag is enabled so it never
567− // surfaces outside its intended audience.
568− //
569− // Race note: `has_flag` returns false before server
570− // flags are received, so a dispatch during that brief
571− // window is a no-op even for users who genuinely have
572− // the flag. The banner itself has the same race — it
573− // stays hidden until flags arrive — so a user who can
574− // see the banner has, by definition, already passed it.
575− if cx.has_flag::<SkillsFeatureFlag>() {
576− crate::ui::RulesToSkillsModal::toggle(workspace, window, cx);
577− }
563+ crate::ui::RulesToSkillsModal::toggle(workspace, window, cx);
578564 },
579565 );
580566 })
@@ -608,10 +594,16 @@ pub fn init( · 27 unmodified lines
608594 })
609595 .detach();
610596
611− // Once the `skills` feature flag has resolved, kick off the one-time
612− // migration of non-Default Rules to global Skills. Idempotent and
613− // self-gated on the flag, so it's safe to call on every flag-ready
614− // notification (and a no-op for users without the flag).
597+ // Kick off the one-time migration of non-Default Rules to global
598+ // Skills, deferred until server feature flags arrive.
599+ //
600+ // The migration itself is idempotent and no longer gated on a flag,
601+ // but the deferral via `on_flags_ready` still matters: the migration
602+ // writes to the on-disk `GlobalKeyValueStore`, which dispatches work
603+ // on the `sqlezWorker` background thread. In `gpui::test` contexts,
604+ // server flags are never received, so this callback never fires —
605+ // which keeps that sqlite worker activity from racing with the
606+ // `TestScheduler` and tripping its non-determinism panic.
615607 {
616608 let fs = fs.clone();
617609 cx.on_flags_ready(move |_, cx| {
@@ -653,12 +645,6 @@ fn update_command_palette_filter(cx: &mut App) { · 35 unmodified lines
653645 .edit_predictions
654646 .provider;
655647
656− // The Skills feature flag is loaded asynchronously, so this value may
657− // be `false` before flags resolve. `update_command_palette_filter`
658− // gets re-run from `cx.on_flags_ready` (see `init`), which means the
659− // filter is reapplied with the correct value once flags arrive.
660− let skills_enabled = cx.has_flag::<SkillsFeatureFlag>();
661−
662648 CommandPaletteFilter::update_global(cx, |filter, _| {
663649 use editor::actions::{
664650 AcceptEditPrediction, AcceptNextLineEditPrediction, AcceptNextWordEditPrediction,
@@ -726,12 +712,12 @@ fn update_command_palette_filter(cx: &mut App) { · 61 unmodified lines
726712 filter.show_namespace("multi_workspace");
727713 }
728714
729− // Hide `assistant: open rules library` when Skills are enabled —
730− // Rules are surfaced through the Skills UI in that case. Applied
731− // after the disable-ai / agent-enabled branches so it overrides
732− // the `show_namespace("assistant")` call above without affecting
733− // the rest of that namespace's actions.
734− if !disable_ai && skills_enabled {
715+ // Hide `assistant: open rules library` — Rules are surfaced
716+ // through the Skills UI now. Applied after the disable-ai /
717+ // agent-enabled branches so it overrides the
718+ // `show_namespace("assistant")` call above without affecting the
719+ // rest of that namespace's actions.
720+ if !disable_ai {
735721 filter.hide_action_types(&open_rules_library_action);
736722 } else {
737723 filter.show_action_types(open_rules_library_action.iter());
crates/feature_flags/src/flags.rs+0−12
@@ -123,15 +123,3 @@ impl FeatureFlag for AutoWatchFeatureFlag { · 122 unmodified lines
123123 type Value = PresenceFlag;
124124 }
125125 register_feature_flag!(AutoWatchFeatureFlag);
126−
127−pub struct SkillsFeatureFlag;
128−
129−impl FeatureFlag for SkillsFeatureFlag {
130− const NAME: &'static str = "skills";
131− type Value = PresenceFlag;
132−
133− fn enabled_for_staff() -> bool {
134− true
135− }
136−}
137−register_feature_flag!(SkillsFeatureFlag);
crates/prompt_store/Cargo.toml+1−1
@@ -18,7 +18,7 @@ assets.workspace = true · 17 unmodified lines
1818 chrono.workspace = true
1919 collections.workspace = true
2020 db.workspace = true
21−feature_flags.workspace = true
21+
2222 fs.workspace = true
2323 futures.workspace = true
2424 fuzzy.workspace = true
crates/prompt_store/src/rules_to_skills_migration.rs+6−15
@@ -24,14 +24,10 @@ · 23 unmodified lines
2424 //! (still using Zed's shipped default content) are skipped so we don't
2525 //! pollute AGENTS.md with text the user never wrote.
2626 //!
27−//! Both migrations are gated by:
28−//!
29−//! * the `skills` feature flag — users without it never have their Rules
30−//! touched in any way;
31−//! * a single global "migration already ran" flag persisted in
32−//! [`GlobalKeyValueStore`] — keyed by [`MIGRATION_DONE_KEY`], so a
33−//! shared home directory only gets populated once per machine even
34−//! across release channels.
27+//! Both migrations are gated by a single global "migration already ran"
28+//! flag persisted in [`GlobalKeyValueStore`] — keyed by
29+//! [`MIGRATION_DONE_KEY`], so a shared home directory only gets
30+//! populated once per machine even across release channels.
3531 //!
3632 //! The migration is intentionally non-destructive: rule rows in the LMDB
3733 //! database are left in place after the migration. That way users can
@@ -45,7 +41,6 @@ use std::sync::atomic::{AtomicBool, Ordering}; · 7 unmodified lines
4541 use agent_skills::{SKILL_FILE_NAME, global_skills_dir, slugify_skill_name};
4642 use anyhow::{Context as _, Result};
4743 use db::kvp::GlobalKeyValueStore;
48−use feature_flags::{FeatureFlagAppExt as _, SkillsFeatureFlag};
4944 use fs::Fs;
5045 use gpui::{App, AsyncApp, Entity, TaskExt as _};
5146 use serde::{Deserialize, Serialize};
@@ -151,13 +146,9 @@ static MIGRATION_TASK_SPAWNED: AtomicBool = AtomicBool::new(false); · 99 unmodified lines
151146 /// Migrate non-Default user rules to global Skills, if not already done.
152147 ///
153148 /// Safe to call on every startup — short-circuits immediately when the
154−/// migration has already run, when another invocation in this process
155−/// has already started it, or when the user doesn't have the `skills`
156−/// feature flag enabled.
149+/// migration has already run or when another invocation in this process
150+/// has already started it.
157151 pub fn migrate_rules_to_skills_if_needed(fs: Arc<dyn Fs>, cx: &mut App) {
158− if !cx.has_flag::<SkillsFeatureFlag>() {
159− return;
160− }
161152 if migration_done() {
162153 return;
163154 }
crates/title_bar/Cargo.toml+1−1
@@ -40,7 +40,7 @@ chrono.workspace = true · 39 unmodified lines
4040 client.workspace = true
4141 cloud_api_types.workspace = true
4242 db.workspace = true
43−feature_flags.workspace = true
43+
4444 git_ui.workspace = true
4545 gpui = { workspace = true, features = ["screen-capture"] }
4646 icons.workspace = true
crates/title_bar/src/title_bar.rs+0−2
@@ -25,7 +25,6 @@ use auto_update::AutoUpdateStatus; · 24 unmodified lines
2525 use call::ActiveCall;
2626 use client::{Client, UserStore, zed_urls};
2727 use cloud_api_types::Plan;
28−use feature_flags::{FeatureFlagAppExt as _, SkillsFeatureFlag};
2928
3029 use gpui::{
3130 Action, Anchor, Animation, AnimationExt, AnyElement, App, Context, Element, Entity, Focusable,
@@ -469,7 +468,6 @@ impl TitleBar { · 437 unmodified lines
469468 zed_actions::agent::OpenRulesToSkillsMigrationInfo.boxed_clone(),
470469 cx,
471470 )
472− .visible_when(|cx| cx.has_flag::<SkillsFeatureFlag>())
473471 }));
474472
475473 let mut this = Self {

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t