Fall back to old key when loading agent settings (#30001)

This PR updates #29943 to fall back to loading agent panel settings from
the old `assistant` key if the `agent` key is not present. Edits to
these settings will also target `assistant` in this situation instead of
`agent` as before.

Release Notes:

- Agent Beta: Fixed a regression that caused the agent panel not to
load, or buttons in the agent panel not to work.

Cargo.lock

@@ -608,6 +608,7 @@ dependencies = [
  "paths",
  "schemars",
  "serde",
+ "serde_json",
  "serde_json_lenient",
  "settings",
  "workspace-hack",

crates/assistant_settings/Cargo.toml

@@ -33,4 +33,5 @@ fs.workspace = true
 gpui = { workspace = true, features = ["test-support"] }
 paths.workspace = true
 serde_json_lenient.workspace = true
+serde_json.workspace = true
 settings = { workspace = true, features = ["test-support"] }

crates/assistant_settings/src/assistant_settings.rs

@@ -695,6 +695,8 @@ pub struct LegacyAssistantSettingsContent {
 impl Settings for AssistantSettings {
     const KEY: Option<&'static str> = Some("agent");
 
+    const FALLBACK_KEY: Option<&'static str> = Some("assistant");
+
     const PRESERVED_KEYS: Option<&'static [&'static str]> = Some(&["version"]);
 
     type FileContent = AssistantSettingsContent;
@@ -826,6 +828,7 @@ fn merge<T>(target: &mut T, value: Option<T>) {
 mod tests {
     use fs::Fs;
     use gpui::{ReadGlobal, TestAppContext};
+    use settings::SettingsStore;
 
     use super::*;
 
@@ -902,4 +905,67 @@ mod tests {
 
         assert!(!assistant_settings.agent.is_version_outdated());
     }
+
+    #[gpui::test]
+    async fn test_load_settings_from_old_key(cx: &mut TestAppContext) {
+        let fs = fs::FakeFs::new(cx.executor().clone());
+        fs.create_dir(paths::settings_file().parent().unwrap())
+            .await
+            .unwrap();
+
+        cx.update(|cx| {
+            let mut test_settings = settings::SettingsStore::test(cx);
+            let user_settings_content = r#"{
+            "assistant": {
+                "enabled": true,
+                "version": "2",
+                "default_model": {
+                  "provider": "zed.dev",
+                  "model": "gpt-99"
+                },
+            }}"#;
+            test_settings
+                .set_user_settings(user_settings_content, cx)
+                .unwrap();
+            cx.set_global(test_settings);
+            AssistantSettings::register(cx);
+        });
+
+        cx.run_until_parked();
+
+        let assistant_settings = cx.update(|cx| AssistantSettings::get_global(cx).clone());
+        assert!(assistant_settings.enabled);
+        assert!(!assistant_settings.using_outdated_settings_version);
+        assert_eq!(assistant_settings.default_model.model, "gpt-99");
+
+        cx.update_global::<SettingsStore, _>(|settings_store, cx| {
+            settings_store.update_user_settings::<AssistantSettings>(cx, |settings| {
+                *settings = AssistantSettingsContent {
+                    inner: Some(AssistantSettingsContentInner::for_v2(
+                        AssistantSettingsContentV2 {
+                            enabled: Some(false),
+                            default_model: Some(LanguageModelSelection {
+                                provider: "xai".to_owned(),
+                                model: "grok".to_owned(),
+                            }),
+                            ..Default::default()
+                        },
+                    )),
+                };
+            });
+        });
+
+        cx.run_until_parked();
+
+        let settings = cx.update(|cx| SettingsStore::global(cx).raw_user_settings().clone());
+
+        #[derive(Debug, Deserialize)]
+        struct AssistantSettingsTest {
+            assistant: AssistantSettingsContent,
+            agent: Option<serde_json_lenient::Value>,
+        }
+
+        let assistant_settings: AssistantSettingsTest = serde_json::from_value(settings).unwrap();
+        assert!(assistant_settings.agent.is_none());
+    }
 }

crates/settings/src/settings_store.rs

@@ -37,6 +37,8 @@ pub trait Settings: 'static + Send + Sync {
     /// from the root object.
     const KEY: Option<&'static str>;
 
+    const FALLBACK_KEY: Option<&'static str> = None;
+
     /// The name of the keys in the [`FileContent`](Self::FileContent) that should
     /// always be written to a settings file, even if their value matches the default
     /// value.
@@ -231,7 +233,13 @@ struct SettingValue<T> {
 trait AnySettingValue: 'static + Send + Sync {
     fn key(&self) -> Option<&'static str>;
     fn setting_type_name(&self) -> &'static str;
-    fn deserialize_setting(&self, json: &Value) -> Result<DeserializedSetting>;
+    fn deserialize_setting(&self, json: &Value) -> Result<DeserializedSetting> {
+        self.deserialize_setting_with_key(json).1
+    }
+    fn deserialize_setting_with_key(
+        &self,
+        json: &Value,
+    ) -> (Option<&'static str>, Result<DeserializedSetting>);
     fn load_setting(
         &self,
         sources: SettingsSources<DeserializedSetting>,
@@ -537,7 +545,8 @@ impl SettingsStore {
             .get(&setting_type_id)
             .unwrap_or_else(|| panic!("unregistered setting type {}", type_name::<T>()));
         let raw_settings = parse_json_with_comments::<Value>(text).unwrap_or_default();
-        let old_content = match setting.deserialize_setting(&raw_settings) {
+        let (key, deserialized_setting) = setting.deserialize_setting_with_key(&raw_settings);
+        let old_content = match deserialized_setting {
             Ok(content) => content.0.downcast::<T::FileContent>().unwrap(),
             Err(_) => Box::<<T as Settings>::FileContent>::default(),
         };
@@ -548,7 +557,7 @@ impl SettingsStore {
         let new_value = serde_json::to_value(new_content).unwrap();
 
         let mut key_path = Vec::new();
-        if let Some(key) = T::KEY {
+        if let Some(key) = key {
             key_path.push(key);
         }
 
@@ -1153,17 +1162,27 @@ impl<T: Settings> AnySettingValue for SettingValue<T> {
         )?))
     }
 
-    fn deserialize_setting(&self, mut json: &Value) -> Result<DeserializedSetting> {
-        if let Some(key) = T::KEY {
-            if let Some(value) = json.get(key) {
+    fn deserialize_setting_with_key(
+        &self,
+        mut json: &Value,
+    ) -> (Option<&'static str>, Result<DeserializedSetting>) {
+        let mut key = None;
+        if let Some(k) = T::KEY {
+            if let Some(value) = json.get(k) {
                 json = value;
+                key = Some(k);
+            } else if let Some((k, value)) = T::FALLBACK_KEY.and_then(|k| Some((k, json.get(k)?))) {
+                json = value;
+                key = Some(k);
             } else {
                 let value = T::FileContent::default();
-                return Ok(DeserializedSetting(Box::new(value)));
+                return (T::KEY, Ok(DeserializedSetting(Box::new(value))));
             }
         }
-        let value = T::FileContent::deserialize(json)?;
-        Ok(DeserializedSetting(Box::new(value)))
+        let value = T::FileContent::deserialize(json)
+            .map(|value| DeserializedSetting(Box::new(value)))
+            .map_err(anyhow::Error::from);
+        (key, value)
     }
 
     fn value_for_path(&self, path: Option<SettingsLocation>) -> &dyn Any {
@@ -1211,7 +1230,8 @@ impl<T: Settings> AnySettingValue for SettingValue<T> {
         text: &mut String,
         edits: &mut Vec<(Range<usize>, String)>,
     ) {
-        let old_content = match self.deserialize_setting(raw_settings) {
+        let (key, deserialized_setting) = self.deserialize_setting_with_key(raw_settings);
+        let old_content = match deserialized_setting {
             Ok(content) => content.0.downcast::<T::FileContent>().unwrap(),
             Err(_) => Box::<<T as Settings>::FileContent>::default(),
         };
@@ -1222,7 +1242,7 @@ impl<T: Settings> AnySettingValue for SettingValue<T> {
         let new_value = serde_json::to_value(new_content).unwrap();
 
         let mut key_path = Vec::new();
-        if let Some(key) = T::KEY {
+        if let Some(key) = key {
             key_path.push(key);
         }