Notify about broken task file contents (#27185)

Closes https://github.com/zed-industries/zed/issues/23783


https://github.com/user-attachments/assets/df019f68-a76b-4953-967a-a35ed21206ab

Release Notes:

- Added notifications when invalid tasks.json/debug.json is saved

crates/paths/src/paths.rs

@@ -342,6 +342,14 @@ pub fn local_vscode_tasks_file_relative_path() -> &'static Path {
     Path::new(".vscode/tasks.json")
 }
 
+pub fn debug_task_file_name() -> &'static str {
+    "debug.json"
+}
+
+pub fn task_file_name() -> &'static str {
+    "tasks.json"
+}
+
 /// Returns the relative path to a `launch.json` file within a project.
 pub fn local_debug_file_relative_path() -> &'static Path {
     Path::new(".zed/debug.json")

crates/project/src/project.rs

@@ -865,7 +865,6 @@ impl Project {
 
             let task_store = cx.new(|cx| {
                 TaskStore::local(
-                    fs.clone(),
                     buffer_store.downgrade(),
                     worktree_store.clone(),
                     toolchain_store.read(cx).as_language_toolchain_store(),
@@ -997,7 +996,6 @@ impl Project {
                 .new(|cx| ToolchainStore::remote(SSH_PROJECT_ID, ssh.read(cx).proto_client(), cx));
             let task_store = cx.new(|cx| {
                 TaskStore::remote(
-                    fs.clone(),
                     buffer_store.downgrade(),
                     worktree_store.clone(),
                     toolchain_store.read(cx).as_language_toolchain_store(),
@@ -1008,7 +1006,12 @@ impl Project {
             });
 
             let settings_observer = cx.new(|cx| {
-                SettingsObserver::new_remote(worktree_store.clone(), task_store.clone(), cx)
+                SettingsObserver::new_remote(
+                    fs.clone(),
+                    worktree_store.clone(),
+                    task_store.clone(),
+                    cx,
+                )
             });
             cx.subscribe(&settings_observer, Self::on_settings_observer_event)
                 .detach();
@@ -1244,7 +1247,6 @@ impl Project {
         let task_store = cx.new(|cx| {
             if run_tasks {
                 TaskStore::remote(
-                    fs.clone(),
                     buffer_store.downgrade(),
                     worktree_store.clone(),
                     Arc::new(EmptyToolchainStore),
@@ -1258,7 +1260,7 @@ impl Project {
         })?;
 
         let settings_observer = cx.new(|cx| {
-            SettingsObserver::new_remote(worktree_store.clone(), task_store.clone(), cx)
+            SettingsObserver::new_remote(fs.clone(), worktree_store.clone(), task_store.clone(), cx)
         })?;
 
         let git_store = cx.new(|cx| {
@@ -2720,15 +2722,27 @@ impl Project {
         match event {
             SettingsObserverEvent::LocalSettingsUpdated(result) => match result {
                 Err(InvalidSettingsError::LocalSettings { message, path }) => {
-                    let message =
+                    let message = format!("Failed to set local settings in {path:?}:\n{message}");
-                        format!("Failed to set local settings in {:?}:\n{}", path, message);
                     cx.emit(Event::Toast {
-                        notification_id: "local-settings".into(),
+                        notification_id: format!("local-settings-{path:?}").into(),
                         message,
                     });
                 }
-                Ok(_) => cx.emit(Event::HideToast {
+                Ok(path) => cx.emit(Event::HideToast {
-                    notification_id: "local-settings".into(),
+                    notification_id: format!("local-settings-{path:?}").into(),
+                }),
+                Err(_) => {}
+            },
+            SettingsObserverEvent::LocalTasksUpdated(result) => match result {
+                Err(InvalidSettingsError::Tasks { message, path }) => {
+                    let message = format!("Failed to set local tasks in {path:?}:\n{message}");
+                    cx.emit(Event::Toast {
+                        notification_id: format!("local-tasks-{path:?}").into(),
+                        message,
+                    });
+                }
+                Ok(path) => cx.emit(Event::HideToast {
+                    notification_id: format!("local-tasks-{path:?}").into(),
                 }),
                 Err(_) => {}
             },

crates/project/src/project_settings.rs

@@ -2,7 +2,8 @@ use anyhow::Context as _;
 use collections::HashMap;
 use dap::adapters::DebugAdapterName;
 use fs::Fs;
-use gpui::{App, AsyncApp, BorrowAppContext, Context, Entity, EventEmitter};
+use futures::StreamExt as _;
+use gpui::{App, AsyncApp, BorrowAppContext, Context, Entity, EventEmitter, Task};
 use lsp::LanguageServerName;
 use paths::{
     local_debug_file_relative_path, local_settings_file_relative_path,
@@ -15,10 +16,14 @@ use rpc::{
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 use settings::{
-    parse_json_with_comments, InvalidSettingsError, LocalSettingsKind, Settings, SettingsLocation,
+    parse_json_with_comments, watch_config_file, InvalidSettingsError, LocalSettingsKind, Settings,
-    SettingsSources, SettingsStore, TaskKind,
+    SettingsLocation, SettingsSources, SettingsStore, TaskKind,
+};
+use std::{
+    path::{Path, PathBuf},
+    sync::Arc,
+    time::Duration,
 };
-use std::{path::Path, sync::Arc, time::Duration};
 use task::{TaskTemplates, VsCodeTaskFile};
 use util::ResultExt;
 use worktree::{PathChange, UpdatedEntriesSet, Worktree, WorktreeId};
@@ -315,7 +320,8 @@ pub enum SettingsObserverMode {
 
 #[derive(Clone, Debug, PartialEq)]
 pub enum SettingsObserverEvent {
-    LocalSettingsUpdated(Result<(), InvalidSettingsError>),
+    LocalSettingsUpdated(Result<PathBuf, InvalidSettingsError>),
+    LocalTasksUpdated(Result<PathBuf, InvalidSettingsError>),
 }
 
 impl EventEmitter<SettingsObserverEvent> for SettingsObserver {}
@@ -326,6 +332,7 @@ pub struct SettingsObserver {
     worktree_store: Entity<WorktreeStore>,
     project_id: u64,
     task_store: Entity<TaskStore>,
+    _global_task_config_watchers: (Task<()>, Task<()>),
 }
 
 /// SettingsObserver observers changes to .zed/{settings, task}.json files in local worktrees
@@ -350,16 +357,31 @@ impl SettingsObserver {
         Self {
             worktree_store,
             task_store,
-            mode: SettingsObserverMode::Local(fs),
+            mode: SettingsObserverMode::Local(fs.clone()),
             downstream_client: None,
             project_id: 0,
+            _global_task_config_watchers: (
+                Self::subscribe_to_global_task_file_changes(
+                    fs.clone(),
+                    TaskKind::Script,
+                    paths::tasks_file().clone(),
+                    cx,
+                ),
+                Self::subscribe_to_global_task_file_changes(
+                    fs,
+                    TaskKind::Debug,
+                    paths::debug_tasks_file().clone(),
+                    cx,
+                ),
+            ),
         }
     }
 
     pub fn new_remote(
+        fs: Arc<dyn Fs>,
         worktree_store: Entity<WorktreeStore>,
         task_store: Entity<TaskStore>,
-        _: &mut Context<Self>,
+        cx: &mut Context<Self>,
     ) -> Self {
         Self {
             worktree_store,
@@ -367,6 +389,20 @@ impl SettingsObserver {
             mode: SettingsObserverMode::Remote,
             downstream_client: None,
             project_id: 0,
+            _global_task_config_watchers: (
+                Self::subscribe_to_global_task_file_changes(
+                    fs.clone(),
+                    TaskKind::Script,
+                    paths::tasks_file().clone(),
+                    cx,
+                ),
+                Self::subscribe_to_global_task_file_changes(
+                    fs.clone(),
+                    TaskKind::Debug,
+                    paths::debug_tasks_file().clone(),
+                    cx,
+                ),
+            ),
         }
     }
 
@@ -622,11 +658,7 @@ impl SettingsObserver {
 
                         match result {
                             Err(InvalidSettingsError::LocalSettings { path, message }) => {
-                                log::error!(
+                                log::error!("Failed to set local settings in {path:?}: {message}");
-                                    "Failed to set local settings in {:?}: {:?}",
-                                    path,
-                                    message
-                                );
                                 cx.emit(SettingsObserverEvent::LocalSettingsUpdated(Err(
                                     InvalidSettingsError::LocalSettings { path, message },
                                 )));
@@ -634,14 +666,16 @@ impl SettingsObserver {
                             Err(e) => {
                                 log::error!("Failed to set local settings: {e}");
                             }
-                            Ok(_) => {
+                            Ok(()) => {
-                                cx.emit(SettingsObserverEvent::LocalSettingsUpdated(Ok(())));
+                                cx.emit(SettingsObserverEvent::LocalSettingsUpdated(Ok(
+                                    directory.join(local_settings_file_relative_path())
+                                )));
                             }
                         }
                     }),
-                LocalSettingsKind::Tasks(task_kind) => task_store.update(cx, |task_store, cx| {
+                LocalSettingsKind::Tasks(task_kind) => {
-                    task_store
+                    let result = task_store.update(cx, |task_store, cx| {
-                        .update_user_tasks(
+                        task_store.update_user_tasks(
                             TaskSettingsLocation::Worktree(SettingsLocation {
                                 worktree_id,
                                 path: directory.as_ref(),
@@ -650,8 +684,25 @@ impl SettingsObserver {
                             task_kind,
                             cx,
                         )
-                        .log_err();
+                    });
-                }),
+
+                    match result {
+                        Err(InvalidSettingsError::Tasks { path, message }) => {
+                            log::error!("Failed to set local tasks in {path:?}: {message:?}");
+                            cx.emit(SettingsObserverEvent::LocalTasksUpdated(Err(
+                                InvalidSettingsError::Tasks { path, message },
+                            )));
+                        }
+                        Err(e) => {
+                            log::error!("Failed to set local tasks: {e}");
+                        }
+                        Ok(()) => {
+                            cx.emit(SettingsObserverEvent::LocalTasksUpdated(Ok(
+                                task_kind.config_in_dir(&directory)
+                            )));
+                        }
+                    }
+                }
             };
 
             if let Some(downstream_client) = &self.downstream_client {
@@ -667,6 +718,65 @@ impl SettingsObserver {
             }
         }
     }
+
+    fn subscribe_to_global_task_file_changes(
+        fs: Arc<dyn Fs>,
+        task_kind: TaskKind,
+        file_path: PathBuf,
+        cx: &mut Context<'_, Self>,
+    ) -> Task<()> {
+        let mut user_tasks_file_rx =
+            watch_config_file(&cx.background_executor(), fs, file_path.clone());
+        let user_tasks_content = cx.background_executor().block(user_tasks_file_rx.next());
+        let weak_entry = cx.weak_entity();
+        cx.spawn(async move |settings_observer, cx| {
+            let Ok(task_store) = settings_observer.update(cx, |settings_observer, _| {
+                settings_observer.task_store.clone()
+            }) else {
+                return;
+            };
+            if let Some(user_tasks_content) = user_tasks_content {
+                let Ok(()) = task_store.update(cx, |task_store, cx| {
+                    task_store
+                        .update_user_tasks(
+                            TaskSettingsLocation::Global(&file_path),
+                            Some(&user_tasks_content),
+                            task_kind,
+                            cx,
+                        )
+                        .log_err();
+                }) else {
+                    return;
+                };
+            }
+            while let Some(user_tasks_content) = user_tasks_file_rx.next().await {
+                let Ok(result) = task_store.update(cx, |task_store, cx| {
+                    task_store.update_user_tasks(
+                        TaskSettingsLocation::Global(&file_path),
+                        Some(&user_tasks_content),
+                        task_kind,
+                        cx,
+                    )
+                }) else {
+                    break;
+                };
+
+                weak_entry
+                    .update(cx, |_, cx| match result {
+                        Ok(()) => cx.emit(SettingsObserverEvent::LocalTasksUpdated(Ok(
+                            file_path.clone()
+                        ))),
+                        Err(err) => cx.emit(SettingsObserverEvent::LocalTasksUpdated(Err(
+                            InvalidSettingsError::Tasks {
+                                path: file_path.clone(),
+                                message: err.to_string(),
+                            },
+                        ))),
+                    })
+                    .ok();
+            }
+        })
+    }
 }
 
 pub fn local_settings_kind_from_proto(kind: proto::LocalSettingsKind) -> LocalSettingsKind {

crates/project/src/task_inventory.rs

@@ -8,12 +8,12 @@ use std::{
     sync::Arc,
 };
 
-use anyhow::{Context as _, Result};
+use anyhow::Result;
 use collections::{HashMap, HashSet, VecDeque};
 use gpui::{App, AppContext as _, Entity, SharedString, Task};
 use itertools::Itertools;
 use language::{ContextProvider, File, Language, LanguageToolchainStore, Location};
-use settings::{parse_json_with_comments, TaskKind};
+use settings::{parse_json_with_comments, InvalidSettingsError, TaskKind};
 use task::{
     DebugTaskDefinition, ResolvedTask, TaskContext, TaskId, TaskTemplate, TaskTemplates,
     TaskVariables, VariableName,
@@ -378,10 +378,23 @@ impl Inventory {
         location: TaskSettingsLocation<'_>,
         raw_tasks_json: Option<&str>,
         task_kind: TaskKind,
-    ) -> anyhow::Result<()> {
+    ) -> Result<(), InvalidSettingsError> {
-        let raw_tasks =
+        let raw_tasks = match parse_json_with_comments::<Vec<serde_json::Value>>(
-            parse_json_with_comments::<Vec<serde_json::Value>>(raw_tasks_json.unwrap_or("[]"))
+            raw_tasks_json.unwrap_or("[]"),
-                .context("parsing tasks file content as a JSON array")?;
+        ) {
+            Ok(tasks) => tasks,
+            Err(e) => {
+                return Err(InvalidSettingsError::Tasks {
+                    path: match location {
+                        TaskSettingsLocation::Global(path) => path.to_owned(),
+                        TaskSettingsLocation::Worktree(settings_location) => {
+                            task_kind.config_in_dir(settings_location.path)
+                        }
+                    },
+                    message: format!("Failed to parse tasks file content as a JSON array: {e}"),
+                })
+            }
+        };
         let new_templates = raw_tasks
             .into_iter()
             .filter_map(|raw_template| match &task_kind {

crates/project/src/task_store.rs

@@ -5,15 +5,13 @@ use std::{
 
 use anyhow::Context as _;
 use collections::HashMap;
-use fs::Fs;
-use futures::StreamExt as _;
 use gpui::{App, AsyncApp, Context, Entity, EventEmitter, Task, WeakEntity};
 use language::{
     proto::{deserialize_anchor, serialize_anchor},
     ContextProvider as _, LanguageToolchainStore, Location,
 };
 use rpc::{proto, AnyProtoClient, TypedEnvelope};
-use settings::{watch_config_file, SettingsLocation, TaskKind};
+use settings::{InvalidSettingsError, SettingsLocation, TaskKind};
 use task::{TaskContext, TaskVariables, VariableName};
 use text::{BufferId, OffsetRangeExt};
 use util::ResultExt;
@@ -35,7 +33,6 @@ pub struct StoreState {
     buffer_store: WeakEntity<BufferStore>,
     worktree_store: Entity<WorktreeStore>,
     toolchain_store: Arc<dyn LanguageToolchainStore>,
-    _global_task_config_watchers: (Task<()>, Task<()>),
 }
 
 enum StoreMode {
@@ -161,7 +158,6 @@ impl TaskStore {
     }
 
     pub fn local(
-        fs: Arc<dyn Fs>,
         buffer_store: WeakEntity<BufferStore>,
         worktree_store: Entity<WorktreeStore>,
         toolchain_store: Arc<dyn LanguageToolchainStore>,
@@ -177,25 +173,10 @@ impl TaskStore {
             buffer_store,
             toolchain_store,
             worktree_store,
-            _global_task_config_watchers: (
-                Self::subscribe_to_global_task_file_changes(
-                    fs.clone(),
-                    TaskKind::Script,
-                    paths::tasks_file().clone(),
-                    cx,
-                ),
-                Self::subscribe_to_global_task_file_changes(
-                    fs.clone(),
-                    TaskKind::Debug,
-                    paths::debug_tasks_file().clone(),
-                    cx,
-                ),
-            ),
         })
     }
 
     pub fn remote(
-        fs: Arc<dyn Fs>,
         buffer_store: WeakEntity<BufferStore>,
         worktree_store: Entity<WorktreeStore>,
         toolchain_store: Arc<dyn LanguageToolchainStore>,
@@ -212,20 +193,6 @@ impl TaskStore {
             buffer_store,
             toolchain_store,
             worktree_store,
-            _global_task_config_watchers: (
-                Self::subscribe_to_global_task_file_changes(
-                    fs.clone(),
-                    TaskKind::Script,
-                    paths::tasks_file().clone(),
-                    cx,
-                ),
-                Self::subscribe_to_global_task_file_changes(
-                    fs.clone(),
-                    TaskKind::Debug,
-                    paths::debug_tasks_file().clone(),
-                    cx,
-                ),
-            ),
         })
     }
 
@@ -299,7 +266,7 @@ impl TaskStore {
         raw_tasks_json: Option<&str>,
         task_type: TaskKind,
         cx: &mut Context<'_, Self>,
-    ) -> anyhow::Result<()> {
+    ) -> Result<(), InvalidSettingsError> {
         let task_inventory = match self {
             TaskStore::Functional(state) => &state.task_inventory,
             TaskStore::Noop => return Ok(()),
@@ -312,53 +279,6 @@ impl TaskStore {
             inventory.update_file_based_tasks(location, raw_tasks_json, task_type)
         })
     }
-
-    fn subscribe_to_global_task_file_changes(
-        fs: Arc<dyn Fs>,
-        task_kind: TaskKind,
-        file_path: PathBuf,
-        cx: &mut Context<'_, Self>,
-    ) -> Task<()> {
-        let mut user_tasks_file_rx =
-            watch_config_file(&cx.background_executor(), fs, file_path.clone());
-        let user_tasks_content = cx.background_executor().block(user_tasks_file_rx.next());
-        cx.spawn(async move |task_store, cx| {
-            if let Some(user_tasks_content) = user_tasks_content {
-                let Ok(_) = task_store.update(cx, |task_store, cx| {
-                    task_store
-                        .update_user_tasks(
-                            TaskSettingsLocation::Global(&file_path),
-                            Some(&user_tasks_content),
-                            task_kind,
-                            cx,
-                        )
-                        .log_err();
-                }) else {
-                    return;
-                };
-            }
-            while let Some(user_tasks_content) = user_tasks_file_rx.next().await {
-                let Ok(()) = task_store.update(cx, |task_store, cx| {
-                    let result = task_store.update_user_tasks(
-                        TaskSettingsLocation::Global(&file_path),
-                        Some(&user_tasks_content),
-                        task_kind,
-                        cx,
-                    );
-                    if let Err(err) = &result {
-                        log::error!("Failed to load user {:?} tasks: {err}", task_kind);
-                        cx.emit(crate::Event::Toast {
-                            notification_id: format!("load-user-{:?}-tasks", task_kind).into(),
-                            message: format!("Invalid global {:?} tasks file\n{err}", task_kind),
-                        });
-                    }
-                    cx.refresh_windows();
-                }) else {
-                    break; // App dropped
-                };
-            }
-        })
-    }
 }
 
 fn local_task_context_for_location(

crates/remote_server/src/headless_project.rs

@@ -140,7 +140,6 @@ impl HeadlessProject {
 
         let task_store = cx.new(|cx| {
             let mut task_store = TaskStore::local(
-                fs.clone(),
                 buffer_store.downgrade(),
                 worktree_store.clone(),
                 toolchain_store.read(cx).as_language_toolchain_store(),

crates/settings/src/settings_store.rs

@@ -5,7 +5,9 @@ use fs::Fs;
 use futures::{channel::mpsc, future::LocalBoxFuture, FutureExt, StreamExt};
 use gpui::{App, AsyncApp, BorrowAppContext, Global, Task, UpdateGlobal};
 
-use paths::{local_settings_file_relative_path, EDITORCONFIG_NAME};
+use paths::{
+    debug_task_file_name, local_settings_file_relative_path, task_file_name, EDITORCONFIG_NAME,
+};
 use schemars::{gen::SchemaGenerator, schema::RootSchema, JsonSchema};
 use serde::{de::DeserializeOwned, Deserialize, Serialize};
 use smallvec::SmallVec;
@@ -250,6 +252,16 @@ trait AnySettingValue: 'static + Send + Sync {
 
 struct DeserializedSetting(Box<dyn Any>);
 
+impl TaskKind {
+    /// Returns a file path of a task configuration file of this kind within the given directory.
+    pub fn config_in_dir(&self, dir: &Path) -> PathBuf {
+        dir.join(match self {
+            Self::Debug => debug_task_file_name(),
+            Self::Script => task_file_name(),
+        })
+    }
+}
+
 impl SettingsStore {
     pub fn new(cx: &App) -> Self {
         let (setting_file_updates_tx, mut setting_file_updates_rx) = mpsc::unbounded();
@@ -610,10 +622,11 @@ impl SettingsStore {
                 .map(|content| content.trim())
                 .filter(|content| !content.is_empty()),
         ) {
-            (LocalSettingsKind::Tasks(_), _) => {
+            (LocalSettingsKind::Tasks(task_kind), _) => {
                 return Err(InvalidSettingsError::Tasks {
                     message: "Attempted to submit tasks into the settings store".to_string(),
-                })
+                    path: task_kind.config_in_dir(&directory_path),
+                });
             }
             (LocalSettingsKind::Settings, None) => {
                 zed_settings_changed = self
@@ -1011,7 +1024,7 @@ pub enum InvalidSettingsError {
     ServerSettings { message: String },
     DefaultSettings { message: String },
     Editorconfig { path: PathBuf, message: String },
-    Tasks { message: String },
+    Tasks { path: PathBuf, message: String },
 }
 
 impl std::fmt::Display for InvalidSettingsError {
@@ -1021,7 +1034,7 @@ impl std::fmt::Display for InvalidSettingsError {
             | InvalidSettingsError::UserSettings { message }
             | InvalidSettingsError::ServerSettings { message }
             | InvalidSettingsError::DefaultSettings { message }
-            | InvalidSettingsError::Tasks { message }
+            | InvalidSettingsError::Tasks { message, .. }
             | InvalidSettingsError::Editorconfig { message, .. } => {
                 write!(f, "{message}")
             }