From d4e5f81333d9da9b807280d7fc69a61204bb8cbf Mon Sep 17 00:00:00 2001 From: Ryan Lee Date: Mon, 17 Aug 2026 21:31:23 +0000 Subject: [PATCH 1/6] fix(watch): load JSONC IDE excludes so the 4096 budget can recover Strict serde_json treated real VS Code settings as empty, so tracked vendor trees stayed in the Linux watch census and tripped TooManyFolders at the capped 4097 count. Parse comments and trailing commas, record load status, and tell the truth in the degraded-watch warning. --- Cargo.lock | 10 ++ Cargo.toml | 1 + crates/gitcomet-state/Cargo.toml | 1 + crates/gitcomet-state/src/msg.rs | 2 +- crates/gitcomet-state/src/msg/message.rs | 24 ++- crates/gitcomet-state/src/store/reducer.rs | 68 ++++++-- .../gitcomet-state/src/store/repo_monitor.rs | 143 +++++++++++++--- .../src/store/watcher_excludes.rs | 152 +++++++++++++++--- 8 files changed, 344 insertions(+), 57 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4e3a01b24..82a45a2b3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2302,6 +2302,7 @@ version = "0.2.0" dependencies = [ "gitcomet-core", "gix", + "jsonc-parser", "notify", "rustc-hash 2.1.3", "serde", @@ -4261,6 +4262,15 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "jsonc-parser" +version = "0.32.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "840785e1a8b4fdb27be18440a35a81af64820dc185c3973ff8b012d4c6282512" +dependencies = [ + "serde", +] + [[package]] name = "khronos-egl" version = "6.0.0" diff --git a/Cargo.toml b/Cargo.toml index 27acada6c..da59da24c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -65,6 +65,7 @@ semver = "1.0.28" schemars = "1.2.2" serde = { version = "1.0.229", features = ["derive"] } serde_json = "1.0.151" +jsonc-parser = { version = "0.32.1", default-features = false, features = ["serde"] } signal-hook = "0.4.4" smallvec = "1.15.2" smol = "2.0.2" diff --git a/crates/gitcomet-state/Cargo.toml b/crates/gitcomet-state/Cargo.toml index 6f0b1bd5d..091f6cdd2 100644 --- a/crates/gitcomet-state/Cargo.toml +++ b/crates/gitcomet-state/Cargo.toml @@ -21,6 +21,7 @@ notify = { workspace = true } smol = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } +jsonc-parser = { workspace = true } rustc-hash = { workspace = true } smallvec = { workspace = true } tempfile = { workspace = true } diff --git a/crates/gitcomet-state/src/msg.rs b/crates/gitcomet-state/src/msg.rs index 35d8d9865..9c0def32d 100644 --- a/crates/gitcomet-state/src/msg.rs +++ b/crates/gitcomet-state/src/msg.rs @@ -11,7 +11,7 @@ pub use effect::Effect; pub use message::{ CommitSelectMode, ConflictAutosolveMode, ConflictAutosolveStats, ConflictBulkChoice, ConflictBulkScope, ConflictRegionChoice, ConflictRegionResolutionUpdate, InternalMsg, Msg, - RepoActionKind, RepoWatchDegradedReason, + RepoActionKind, RepoWatchDegradedReason, WatcherExcludeLoadStatus, }; pub use repo_command_kind::RepoCommandKind; pub use repo_external_change::RepoExternalChange; diff --git a/crates/gitcomet-state/src/msg/message.rs b/crates/gitcomet-state/src/msg/message.rs index 0be821996..5009031df 100644 --- a/crates/gitcomet-state/src/msg/message.rs +++ b/crates/gitcomet-state/src/msg/message.rs @@ -128,12 +128,32 @@ impl ConflictAutosolveStats { } } +/// How `/.vscode/settings.json` `files.watcherExclude` loaded for the +/// active monitor. `Copy` so it can ride on [`RepoWatchDegradedReason`]. +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +pub enum WatcherExcludeLoadStatus { + /// The Respect-IDE-excludes setting is off; the file was not read. + #[default] + Disabled, + /// The settings file is absent. + Missing, + /// The file existed but could not be read or parsed as JSONC. + Unreadable, + /// The file parsed. True-entry count lives on the rule set, not here. + Parsed, +} + /// Why the file-system watcher is in a degraded state (carried by [`Msg::RepoWatchDegraded`]). #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum RepoWatchDegradedReason { /// The worktree has more non-ignored folders than the watch budget, so its source folders are - /// not watched live at all. Carries the folder count. - TooManyFolders { dir_count: usize }, + /// not watched live at all. `dir_count` is the probe length; `capped` means the walk stopped + /// early so the count is not an exact census. + TooManyFolders { + dir_count: usize, + capped: bool, + load_status: WatcherExcludeLoadStatus, + }, /// Some per-directory watches could not be added (the kernel inotify limit was reached), so part /// of the worktree is not watched live. Carries the number of folders left unwatched. WatchLimitReached { unwatched_dirs: usize }, diff --git a/crates/gitcomet-state/src/store/reducer.rs b/crates/gitcomet-state/src/store/reducer.rs index 4f706f517..04ef7484d 100644 --- a/crates/gitcomet-state/src/store/reducer.rs +++ b/crates/gitcomet-state/src/store/reducer.rs @@ -900,12 +900,29 @@ fn reduce_inner( } Msg::RepoWatchDegraded { repo_id: _, reason } => { let message = match reason { - crate::msg::RepoWatchDegradedReason::TooManyFolders { dir_count } => format!( - "This repository has {dir_count} folders — live file watching is disabled to \ - stay within system limits. Changes refresh when the window regains focus. Add \ - build/output dirs to .gitignore or raise fs.inotify.max_user_watches to \ - re-enable." - ), + crate::msg::RepoWatchDegradedReason::TooManyFolders { + dir_count, + capped, + load_status, + } => { + let count = if capped { + format!("more than {}", dir_count.saturating_sub(1)) + } else { + dir_count.to_string() + }; + let parse_note = match load_status { + crate::msg::WatcherExcludeLoadStatus::Unreadable => { + " .vscode/settings.json could not be parsed, so files.watcherExclude was not applied." + } + _ => "", + }; + format!( + "This repository has {count} folders — live file watching is disabled to \ + stay within system limits. Changes refresh when the window regains focus.{parse_note} \ + Add build/output dirs to .gitignore or to .vscode/settings.json files.watcherExclude, \ + or raise fs.inotify.max_user_watches to re-enable." + ) + } crate::msg::RepoWatchDegradedReason::WatchLimitReached { unwatched_dirs } => { format!( "Live file watching is partial: {unwatched_dirs} folders could not be watched \ @@ -2398,7 +2415,11 @@ mod nav_history_tests { &mut state, Msg::RepoWatchDegraded { repo_id: RepoId(1), - reason: crate::msg::RepoWatchDegradedReason::TooManyFolders { dir_count: 9000 }, + reason: crate::msg::RepoWatchDegradedReason::TooManyFolders { + dir_count: 9000, + capped: false, + load_status: crate::msg::WatcherExcludeLoadStatus::Parsed, + }, }, ); assert_eq!(state.notifications.len(), 1); @@ -2409,8 +2430,35 @@ mod nav_history_tests { "warning should mention the folder count: {}", note.message ); + assert!( + note.message.contains("files.watcherExclude"), + "warning should name the IDE exclude remedy: {}", + note.message + ); + + dispatch( + &mut state, + Msg::RepoWatchDegraded { + repo_id: RepoId(1), + reason: crate::msg::RepoWatchDegradedReason::TooManyFolders { + dir_count: 4097, + capped: true, + load_status: crate::msg::WatcherExcludeLoadStatus::Unreadable, + }, + }, + ); + let note = &state.notifications[1]; + assert!( + note.message.contains("more than 4096"), + "capped warning must not treat the probe as an exact census: {}", + note.message + ); + assert!( + note.message.contains("could not be parsed"), + "unreadable settings must be named: {}", + note.message + ); - // A partial watch failure surfaces a (distinct) warning too — not just the stderr log. dispatch( &mut state, Msg::RepoWatchDegraded { @@ -2420,8 +2468,8 @@ mod nav_history_tests { }, }, ); - assert_eq!(state.notifications.len(), 2); - let note = &state.notifications[1]; + assert_eq!(state.notifications.len(), 3); + let note = &state.notifications[2]; assert_eq!(note.kind, crate::model::AppNotificationKind::Warning); assert!( note.message.contains("42"), diff --git a/crates/gitcomet-state/src/store/repo_monitor.rs b/crates/gitcomet-state/src/store/repo_monitor.rs index f0f1073e5..75258230d 100644 --- a/crates/gitcomet-state/src/store/repo_monitor.rs +++ b/crates/gitcomet-state/src/store/repo_monitor.rs @@ -1,5 +1,5 @@ use crate::model::RepoId; -use crate::msg::{Msg, RepoExternalChange, RepoWatchDegradedReason}; +use crate::msg::{Msg, RepoExternalChange, RepoWatchDegradedReason, WatcherExcludeLoadStatus}; use gix::index::entry::Mode as GitIndexMode; use notify::event::{AccessKind, AccessMode, EventKindMask}; use notify::{Config as NotifyConfig, RecommendedWatcher, RecursiveMode, Watcher}; @@ -771,11 +771,18 @@ fn build_workdir_watcher( /// The user-facing degraded-watch reason for an outcome, or `None` when watching is healthy /// (fully watched, or the root watch failed and the watcher is being discarded). -fn watch_degraded_reason(outcome: WatchSetupOutcome) -> Option { +fn watch_degraded_reason( + outcome: WatchSetupOutcome, + load_status: WatcherExcludeLoadStatus, +) -> Option { match outcome { #[cfg(any(target_os = "linux", test))] - WatchSetupOutcome::WorktreeSubdirsSkipped { dir_count } => { - Some(RepoWatchDegradedReason::TooManyFolders { dir_count }) + WatchSetupOutcome::WorktreeSubdirsSkipped { dir_count, capped } => { + Some(RepoWatchDegradedReason::TooManyFolders { + dir_count, + capped, + load_status, + }) } WatchSetupOutcome::Watching { failed_dirs } if failed_dirs > 0 => { Some(RepoWatchDegradedReason::WatchLimitReached { @@ -792,8 +799,9 @@ fn watch_degraded_reason(outcome: WatchSetupOutcome) -> Option Option { - let reason = watch_degraded_reason(outcome); + let reason = watch_degraded_reason(outcome, load_status); let should_warn = reason.is_some() && !*previously_degraded; *previously_degraded = reason.is_some(); if should_warn { reason } else { None } @@ -806,8 +814,9 @@ fn note_watch_outcome( repo_id: RepoId, previously_degraded: &mut bool, outcome: WatchSetupOutcome, + load_status: WatcherExcludeLoadStatus, ) { - if let Some(reason) = watch_degraded_transition(previously_degraded, outcome) { + if let Some(reason) = watch_degraded_transition(previously_degraded, outcome, load_status) { msg_tx.send_repo_monitor_or_log( Msg::RepoWatchDegraded { repo_id, reason }, "repo monitor watch degraded", @@ -940,7 +949,13 @@ fn repo_monitor_thread( // repository correct. let mut watch_degraded = false; let mut last_recovery_attempt: Option = None; - note_watch_outcome(&msg_tx, repo_id, &mut watch_degraded, watch_outcome); + note_watch_outcome( + &msg_tx, + repo_id, + &mut watch_degraded, + watch_outcome, + watcher_excludes.load_status(), + ); let debounce = Duration::from_millis(250); let max_delay = Duration::from_secs(2); @@ -1115,7 +1130,13 @@ fn repo_monitor_thread( ) { watcher = new_watcher; watch_outcome = new_outcome; - note_watch_outcome(&msg_tx, repo_id, &mut watch_degraded, watch_outcome); + note_watch_outcome( + &msg_tx, + repo_id, + &mut watch_degraded, + watch_outcome, + watcher_excludes.load_status(), + ); rules_rebuild_pending = false; } } @@ -1144,7 +1165,13 @@ fn repo_monitor_thread( ) { watcher = new_watcher; watch_outcome = new_outcome; - note_watch_outcome(&msg_tx, repo_id, &mut watch_degraded, watch_outcome); + note_watch_outcome( + &msg_tx, + repo_id, + &mut watch_degraded, + watch_outcome, + watcher_excludes.load_status(), + ); } } } @@ -1339,7 +1366,7 @@ enum WatchSetupOutcome { /// root); the source tree is left to the `.git` watch + focus-triggered full refresh. Carries /// the subdirectory count for the user-facing warning. #[cfg(any(target_os = "linux", test))] - WorktreeSubdirsSkipped { dir_count: usize }, + WorktreeSubdirsSkipped { dir_count: usize, capped: bool }, /// The workdir root watch failed; the watcher is unusable. RootWatchFailed, } @@ -1421,26 +1448,34 @@ fn setup_workdir_watch_with_limit( max_dirs, ); let subdir_count = dirs.len().saturating_sub(1); + let capped = dirs.len() > max_dirs.saturating_add(1); if subdir_count > max_dirs { // Too many folders to watch within the kernel limit: do not watch any source folders. The // `.git` watch keeps git operations live, and focus reload re-reads the whole worktree. repo_load_trace::trace!( - "monitor_setup_watches_skipped repo_id={:?} workdir={} subdirs={} max={}", + "monitor_setup_watches_skipped repo_id={:?} workdir={} subdirs={} max={} capped={} exclude_status={:?} exclude_rules={}", repo_id, workdir.display(), subdir_count, - max_dirs + max_dirs, + capped, + watcher_excludes.load_status(), + watcher_excludes.parsed_rule_count(), ); eprintln!( "gitcomet-state: repo monitor is not watching the {subdir_count} worktree folders of \ repo_id={repo_id:?} (workdir={}) because that exceeds the watch budget ({max_dirs}); \ live file watching is disabled and changes refresh when the window regains focus. Add \ - build/output dirs to .gitignore or raise fs.inotify.max_user_watches to re-enable.", + build/output dirs to .gitignore or to .vscode/settings.json files.watcherExclude, or \ + raise fs.inotify.max_user_watches to re-enable. exclude_status={:?} exclude_rules={}", workdir.display(), + watcher_excludes.load_status(), + watcher_excludes.parsed_rule_count(), ); return WatchSetupOutcome::WorktreeSubdirsSkipped { dir_count: subdir_count, + capped, }; } @@ -2848,44 +2883,55 @@ mod tests { #[test] fn watch_degraded_transition_fires_once_per_degraded_episode() { let mut degraded = false; - // Entering the skipped state warns, carrying the folder count. + let parsed = WatcherExcludeLoadStatus::Parsed; assert_eq!( watch_degraded_transition( &mut degraded, - WatchSetupOutcome::WorktreeSubdirsSkipped { dir_count: 9000 } + WatchSetupOutcome::WorktreeSubdirsSkipped { + dir_count: 9000, + capped: false, + }, + parsed, ), - Some(RepoWatchDegradedReason::TooManyFolders { dir_count: 9000 }) + Some(RepoWatchDegradedReason::TooManyFolders { + dir_count: 9000, + capped: false, + load_status: parsed, + }) ); - // Staying degraded (e.g. a .gitignore rebuild that is still over budget) does not re-warn. assert_eq!( watch_degraded_transition( &mut degraded, - WatchSetupOutcome::WorktreeSubdirsSkipped { dir_count: 9001 } + WatchSetupOutcome::WorktreeSubdirsSkipped { + dir_count: 9001, + capped: false, + }, + parsed, ), None ); - // Recovering to full watching clears the flag without warning. assert_eq!( watch_degraded_transition( &mut degraded, - WatchSetupOutcome::Watching { failed_dirs: 0 } + WatchSetupOutcome::Watching { failed_dirs: 0 }, + parsed, ), None ); assert!(!degraded); - // A partial watch failure is also a degraded transition and warns with the unwatched count. assert_eq!( watch_degraded_transition( &mut degraded, - WatchSetupOutcome::Watching { failed_dirs: 7 } + WatchSetupOutcome::Watching { failed_dirs: 7 }, + parsed, ), Some(RepoWatchDegradedReason::WatchLimitReached { unwatched_dirs: 7 }) ); - // Still partially failing on a rebuild does not re-warn. assert_eq!( watch_degraded_transition( &mut degraded, - WatchSetupOutcome::Watching { failed_dirs: 3 } + WatchSetupOutcome::Watching { failed_dirs: 3 }, + parsed, ), None ); @@ -3530,6 +3576,8 @@ mod tests { let git_dir = resolve_git_dir(&workdir); let mut gitignore = GitignoreRules::load(&workdir); let mut excludes = WatcherExcludes::load(&workdir, true); + assert_eq!(excludes.load_status(), WatcherExcludeLoadStatus::Parsed); + assert_eq!(excludes.parsed_rule_count(), 1); let dirs = collect_watchable_dirs( &workdir, @@ -3551,8 +3599,8 @@ mod tests { "non-excluded dirs must stay watched" ); - // Disabled rule set: everything is watched again (T2.1/T2.6 parity). let mut disabled = WatcherExcludes::load(&workdir, false); + assert_eq!(disabled.load_status(), WatcherExcludeLoadStatus::Disabled); let dirs = collect_watchable_dirs( &workdir, &workdir, @@ -3564,6 +3612,51 @@ mod tests { assert!(dirs.contains(&workdir.join("node_modules").join("pkg"))); } + #[test] + fn jsonc_repos_exclude_drops_tracked_vendor_tree() { + let dir = unique_temp_dir("gitcomet-monitor-jsonc-repos"); + let workdir = dir.path().join("repo"); + init_repo_for_ignore_tests(&workdir); + fs::create_dir_all(workdir.join("repos").join("pkg")).expect("create repos"); + fs::write(workdir.join("repos").join("tracked.txt"), "x").expect("write tracked"); + fs::create_dir_all(workdir.join("src")).expect("create src"); + run_git(&workdir, &["add", "repos/tracked.txt"]); + run_git(&workdir, &["commit", "-m", "track vendor"]); + fs::create_dir_all(workdir.join(".vscode")).expect("create .vscode"); + fs::write( + WatcherExcludes::config_path(&workdir), + r#"{ + // IDE excludes + "files.watcherExclude": { + "repos/": true, + }, + }"#, + ) + .expect("write settings.json"); + let git_dir = resolve_git_dir(&workdir); + let mut gitignore = GitignoreRules::load(&workdir); + let mut excludes = WatcherExcludes::load(&workdir, true); + assert_eq!(excludes.load_status(), WatcherExcludeLoadStatus::Parsed); + assert_eq!(excludes.parsed_rule_count(), 1); + + let dirs = collect_watchable_dirs( + &workdir, + &workdir, + git_dir.as_deref(), + &mut gitignore, + &mut excludes, + ); + assert!( + dirs.contains(&workdir.join("src")), + "sibling source dir must stay watchable" + ); + assert!( + !dirs.contains(&workdir.join("repos")), + "JSONC-excluded tracked vendor dir must drop from the census" + ); + assert!(!dirs.contains(&workdir.join("repos").join("pkg"))); + } + #[test] fn ide_watcher_excludes_suppress_classify_events() { let dir = unique_temp_dir("gitcomet-monitor-ide-excludes"); diff --git a/crates/gitcomet-state/src/store/watcher_excludes.rs b/crates/gitcomet-state/src/store/watcher_excludes.rs index e2d44492f..b22102fbc 100644 --- a/crates/gitcomet-state/src/store/watcher_excludes.rs +++ b/crates/gitcomet-state/src/store/watcher_excludes.rs @@ -22,8 +22,9 @@ use std::path::{Path, PathBuf}; -use super::repo_load_trace; +use crate::msg::WatcherExcludeLoadStatus; +use super::repo_load_trace; /// Directory and file of the VS Code settings inside the worktree. const VSCODE_SETTINGS_DIR: &str = ".vscode"; const VSCODE_SETTINGS_FILE: &str = "settings.json"; @@ -190,10 +191,23 @@ fn pattern_excludes( /// The watcher-exclude rule set for one repository worktree. /// /// Immutable after load; the monitor reloads it when the config file changes. -#[derive(Debug, Default)] +#[derive(Debug)] pub(crate) struct WatcherExcludes { enabled: bool, patterns: Vec, + load_status: WatcherExcludeLoadStatus, + parsed_rule_count: usize, +} + +impl Default for WatcherExcludes { + fn default() -> Self { + Self { + enabled: false, + patterns: Vec::new(), + load_status: WatcherExcludeLoadStatus::Disabled, + parsed_rule_count: 0, + } + } } impl WatcherExcludes { @@ -202,12 +216,17 @@ impl WatcherExcludes { /// `enabled` gates the whole mechanism: a disabled rule set is empty and /// never reads the config file. pub(crate) fn load(workdir: &Path, enabled: bool) -> Self { - let patterns = if enabled { - parse_vscode_watcher_exclude(workdir).unwrap_or_default() - } else { - Vec::new() - }; - Self { enabled, patterns } + if !enabled { + return Self::default(); + } + let (patterns, load_status, parsed_rule_count) = + parse_vscode_watcher_exclude_with_status(workdir); + Self { + enabled: true, + patterns, + load_status, + parsed_rule_count, + } } /// Path of the config file this rule set reads from. @@ -221,6 +240,14 @@ impl WatcherExcludes { self.enabled } + pub(crate) fn load_status(&self) -> WatcherExcludeLoadStatus { + self.load_status + } + + pub(crate) fn parsed_rule_count(&self) -> usize { + self.parsed_rule_count + } + /// Returns `true` when `rel` (worktree-relative) falls under an exclude /// rule. `is_dir_hint` disambiguates directory-only patterns /// (`Some(false)` = definitely a file). @@ -247,10 +274,12 @@ impl WatcherExcludes { /// Parses `files.watcherExclude` from `/.vscode/settings.json`. /// -/// Returns `None` on any structural problem (missing file, invalid JSON, wrong -/// types) — the caller treats that as an empty rule set; diagnosability comes -/// from the trace lines (plan T1.1). -fn parse_vscode_watcher_exclude(workdir: &Path) -> Option> { +/// Missing / unreadable / unparseable files yield an empty rule set plus the +/// matching [`WatcherExcludeLoadStatus`]. Diagnosability comes from the trace +/// lines (origin plan T1.1). +fn parse_vscode_watcher_exclude_with_status( + workdir: &Path, +) -> (Vec, WatcherExcludeLoadStatus, usize) { let path = WatcherExcludes::config_path(workdir); let text = match std::fs::read_to_string(&path) { Ok(text) => text, @@ -259,26 +288,43 @@ fn parse_vscode_watcher_exclude(workdir: &Path) -> Option> { "watcher_excludes: {} not found — treating as empty", path.display() ); - return Some(Vec::new()); + return (Vec::new(), WatcherExcludeLoadStatus::Missing, 0); } Err(error) => { repo_load_trace::trace!( "watcher_excludes: could not read {}: {error} — treating as empty", path.display() ); - return Some(Vec::new()); + return (Vec::new(), WatcherExcludeLoadStatus::Unreadable, 0); } }; - let value: serde_json::Value = match serde_json::from_str(&text) { + + let parse_options = jsonc_parser::ParseOptions { + allow_comments: true, + allow_trailing_commas: true, + allow_loose_object_property_names: false, + allow_missing_commas: false, + allow_single_quoted_strings: false, + allow_hexadecimal_numbers: false, + allow_unary_plus_numbers: false, + }; + let value: serde_json::Value = match jsonc_parser::parse_to_serde_value(&text, &parse_options) { Ok(value) => value, Err(error) => { repo_load_trace::trace!( "watcher_excludes: could not parse {}: {error} — treating as empty", path.display() ); - return Some(Vec::new()); + return (Vec::new(), WatcherExcludeLoadStatus::Unreadable, 0); } }; + if !value.is_object() { + repo_load_trace::trace!( + "watcher_excludes: {} is not a JSON object — treating as empty", + path.display() + ); + return (Vec::new(), WatcherExcludeLoadStatus::Unreadable, 0); + } let Some(excludes) = value .get("files.watcherExclude") .and_then(serde_json::Value::as_object) @@ -287,7 +333,7 @@ fn parse_vscode_watcher_exclude(workdir: &Path) -> Option> { "watcher_excludes: {} has no `files.watcherExclude` object — treating as empty", path.display() ); - return Some(Vec::new()); + return (Vec::new(), WatcherExcludeLoadStatus::Parsed, 0); }; let mut patterns = Vec::new(); @@ -333,7 +379,12 @@ fn parse_vscode_watcher_exclude(workdir: &Path) -> Option> { dir_only, }); } - Some(patterns) + let parsed_rule_count = patterns.len(); + ( + patterns, + WatcherExcludeLoadStatus::Parsed, + parsed_rule_count, + ) } /// Whether the original pattern contains a slash (after stripping the trailing @@ -391,14 +442,16 @@ mod tests { let excludes = WatcherExcludes::load(dir.path(), true); assert!(!excludes.is_excluded(rel("src"), Some(true))); assert!(!excludes.is_excluded(rel("node_modules"), Some(true))); + assert_eq!(excludes.load_status(), WatcherExcludeLoadStatus::Missing); + assert_eq!(excludes.parsed_rule_count(), 0); } - #[test] fn empty_settings_yield_empty_rules() { let dir = temp_workdir(); write_settings(dir.path(), ""); let excludes = WatcherExcludes::load(dir.path(), true); assert!(!excludes.is_excluded(rel("node_modules"), Some(true))); + assert_eq!(excludes.load_status(), WatcherExcludeLoadStatus::Unreadable); } #[test] @@ -407,6 +460,63 @@ mod tests { write_settings(dir.path(), "{ not json"); let excludes = WatcherExcludes::load(dir.path(), true); assert!(!excludes.is_excluded(rel("node_modules"), Some(true))); + assert_eq!(excludes.load_status(), WatcherExcludeLoadStatus::Unreadable); + assert_eq!(excludes.parsed_rule_count(), 0); + } + + #[test] + fn jsonc_comments_and_trailing_comma_load_repos_exclude() { + let dir = temp_workdir(); + write_settings( + dir.path(), + r#"{ + // workspace watcher excludes + "files.watcherExclude": { + "repos/": true, /* vendor trees */ + }, + }"#, + ); + let excludes = WatcherExcludes::load(dir.path(), true); + assert_eq!(excludes.load_status(), WatcherExcludeLoadStatus::Parsed); + assert_eq!(excludes.parsed_rule_count(), 1); + assert!(excludes.is_excluded(rel("repos"), Some(true))); + assert!(excludes.is_excluded(rel("repos/x"), Some(true))); + assert!(!excludes.is_excluded(rel("src"), Some(true))); + } + #[test] + fn jsonc_string_containing_slashes_is_not_a_comment() { + let dir = temp_workdir(); + write_settings( + dir.path(), + r#"{ + "homepage": "http://example.com//docs", + "files.watcherExclude": { + "repos/": true + } + }"#, + ); + let excludes = WatcherExcludes::load(dir.path(), true); + assert_eq!(excludes.load_status(), WatcherExcludeLoadStatus::Parsed); + assert_eq!(excludes.parsed_rule_count(), 1); + assert!(excludes.is_excluded(rel("repos"), Some(true))); + } + + #[test] + fn parsed_file_without_watcher_exclude_is_empty_parsed() { + let dir = temp_workdir(); + write_settings(dir.path(), r#"{"editor.tabSize": 4}"#); + let excludes = WatcherExcludes::load(dir.path(), true); + assert_eq!(excludes.load_status(), WatcherExcludeLoadStatus::Parsed); + assert_eq!(excludes.parsed_rule_count(), 0); + assert!(!excludes.is_excluded(rel("repos"), Some(true))); + } + + #[test] + fn default_rule_set_is_disabled() { + let excludes = WatcherExcludes::default(); + assert_eq!(excludes.load_status(), WatcherExcludeLoadStatus::Disabled); + assert!(!excludes.enabled()); + assert!(!excludes.is_excluded(rel("repos"), Some(true))); } #[test] @@ -415,6 +525,8 @@ mod tests { write_settings(dir.path(), r#"{"files.watcherExclude": 42}"#); let excludes = WatcherExcludes::load(dir.path(), true); assert!(!excludes.is_excluded(rel("node_modules"), Some(true))); + assert_eq!(excludes.load_status(), WatcherExcludeLoadStatus::Parsed); + assert_eq!(excludes.parsed_rule_count(), 0); } #[test] @@ -428,6 +540,8 @@ mod tests { assert!(excludes.is_excluded(rel("node_modules"), Some(true))); assert!(!excludes.is_excluded(rel("dist"), Some(true))); assert!(!excludes.is_excluded(rel("vendor"), Some(true))); + assert_eq!(excludes.load_status(), WatcherExcludeLoadStatus::Parsed); + assert_eq!(excludes.parsed_rule_count(), 1); } #[test] From f4dee4b25453e76fe88e7e2431725c8ab96631ea Mon Sep 17 00:00:00 2001 From: Ryan Lee Date: Mon, 17 Aug 2026 21:58:21 +0000 Subject: [PATCH 2/6] docs(review): record residual review findings --- .../fix-ide-excludes-jsonc-watch-budget.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 docs/residual-review-findings/fix-ide-excludes-jsonc-watch-budget.md diff --git a/docs/residual-review-findings/fix-ide-excludes-jsonc-watch-budget.md b/docs/residual-review-findings/fix-ide-excludes-jsonc-watch-budget.md new file mode 100644 index 000000000..0f217e473 --- /dev/null +++ b/docs/residual-review-findings/fix-ide-excludes-jsonc-watch-budget.md @@ -0,0 +1,16 @@ +# Residual Review Findings + +Source: ce-code-review run `20260817-213836-f22a5a7f` on branch `fix/ide-excludes-jsonc-watch-budget` (head `d4e5f813`). + +## Residual Review Findings + +- P2 `crates/gitcomet-state/src/store/repo_monitor.rs:3617` — Plan T2.2 missing: JSONC exclude under injected budget — [systemfsoftware/GitComet#58](https://github.com/systemfsoftware/GitComet/issues/58) + +No apply-step findings were eligible (the only actionable item is confidence 75 without cross-persona agreement). + +## Source run context + +- Plan: `docs/plans/ide-excludes-jsonc-watch-budget.md` +- Review artifact: `/tmp/compound-engineering-0/ce-code-review/20260817-213836-f22a5a7f` +- Reviewers completed: correctness, testing, maintainability, reliability, learnings +- Adversarial: timeout (cross-model peer not started) From a7d909b9cde012cc94a4f7580dd1544a7d71db67 Mon Sep 17 00:00:00 2001 From: Ryan Lee Date: Mon, 17 Aug 2026 21:59:01 +0000 Subject: [PATCH 3/6] refactor(watch): derive exclude rule count from the pattern list Drop the stored sibling count so it cannot drift from patterns.len(). --- crates/gitcomet-state/src/store/reducer.rs | 4 ++- .../gitcomet-state/src/store/repo_monitor.rs | 4 +-- .../src/store/watcher_excludes.rs | 35 ++++++------------- 3 files changed, 16 insertions(+), 27 deletions(-) diff --git a/crates/gitcomet-state/src/store/reducer.rs b/crates/gitcomet-state/src/store/reducer.rs index 04ef7484d..0590c855d 100644 --- a/crates/gitcomet-state/src/store/reducer.rs +++ b/crates/gitcomet-state/src/store/reducer.rs @@ -914,7 +914,9 @@ fn reduce_inner( crate::msg::WatcherExcludeLoadStatus::Unreadable => { " .vscode/settings.json could not be parsed, so files.watcherExclude was not applied." } - _ => "", + crate::msg::WatcherExcludeLoadStatus::Disabled + | crate::msg::WatcherExcludeLoadStatus::Missing + | crate::msg::WatcherExcludeLoadStatus::Parsed => "", }; format!( "This repository has {count} folders — live file watching is disabled to \ diff --git a/crates/gitcomet-state/src/store/repo_monitor.rs b/crates/gitcomet-state/src/store/repo_monitor.rs index 75258230d..ae9fe4166 100644 --- a/crates/gitcomet-state/src/store/repo_monitor.rs +++ b/crates/gitcomet-state/src/store/repo_monitor.rs @@ -1363,8 +1363,8 @@ enum WatchSetupOutcome { /// worktree is only partially watched. Watching { failed_dirs: usize }, /// Too many non-ignored worktree directories: no source folders are watched (only the workdir - /// root); the source tree is left to the `.git` watch + focus-triggered full refresh. Carries - /// the subdirectory count for the user-facing warning. + /// root); the source tree is left to the `.git` watch + focus-triggered full refresh. `dir_count` + /// is the probe length; `capped` is true when the walk stopped early so that count is not exact. #[cfg(any(target_os = "linux", test))] WorktreeSubdirsSkipped { dir_count: usize, capped: bool }, /// The workdir root watch failed; the watcher is unusable. diff --git a/crates/gitcomet-state/src/store/watcher_excludes.rs b/crates/gitcomet-state/src/store/watcher_excludes.rs index b22102fbc..7839b2e84 100644 --- a/crates/gitcomet-state/src/store/watcher_excludes.rs +++ b/crates/gitcomet-state/src/store/watcher_excludes.rs @@ -196,7 +196,6 @@ pub(crate) struct WatcherExcludes { enabled: bool, patterns: Vec, load_status: WatcherExcludeLoadStatus, - parsed_rule_count: usize, } impl Default for WatcherExcludes { @@ -205,7 +204,6 @@ impl Default for WatcherExcludes { enabled: false, patterns: Vec::new(), load_status: WatcherExcludeLoadStatus::Disabled, - parsed_rule_count: 0, } } } @@ -219,13 +217,11 @@ impl WatcherExcludes { if !enabled { return Self::default(); } - let (patterns, load_status, parsed_rule_count) = - parse_vscode_watcher_exclude_with_status(workdir); + let (patterns, load_status) = parse_vscode_watcher_exclude_with_status(workdir); Self { enabled: true, patterns, load_status, - parsed_rule_count, } } @@ -245,7 +241,7 @@ impl WatcherExcludes { } pub(crate) fn parsed_rule_count(&self) -> usize { - self.parsed_rule_count + self.patterns.len() } /// Returns `true` when `rel` (worktree-relative) falls under an exclude @@ -279,7 +275,7 @@ impl WatcherExcludes { /// lines (origin plan T1.1). fn parse_vscode_watcher_exclude_with_status( workdir: &Path, -) -> (Vec, WatcherExcludeLoadStatus, usize) { +) -> (Vec, WatcherExcludeLoadStatus) { let path = WatcherExcludes::config_path(workdir); let text = match std::fs::read_to_string(&path) { Ok(text) => text, @@ -288,14 +284,14 @@ fn parse_vscode_watcher_exclude_with_status( "watcher_excludes: {} not found — treating as empty", path.display() ); - return (Vec::new(), WatcherExcludeLoadStatus::Missing, 0); + return (Vec::new(), WatcherExcludeLoadStatus::Missing); } Err(error) => { repo_load_trace::trace!( "watcher_excludes: could not read {}: {error} — treating as empty", path.display() ); - return (Vec::new(), WatcherExcludeLoadStatus::Unreadable, 0); + return (Vec::new(), WatcherExcludeLoadStatus::Unreadable); } }; @@ -315,7 +311,7 @@ fn parse_vscode_watcher_exclude_with_status( "watcher_excludes: could not parse {}: {error} — treating as empty", path.display() ); - return (Vec::new(), WatcherExcludeLoadStatus::Unreadable, 0); + return (Vec::new(), WatcherExcludeLoadStatus::Unreadable); } }; if !value.is_object() { @@ -323,7 +319,7 @@ fn parse_vscode_watcher_exclude_with_status( "watcher_excludes: {} is not a JSON object — treating as empty", path.display() ); - return (Vec::new(), WatcherExcludeLoadStatus::Unreadable, 0); + return (Vec::new(), WatcherExcludeLoadStatus::Unreadable); } let Some(excludes) = value .get("files.watcherExclude") @@ -333,18 +329,14 @@ fn parse_vscode_watcher_exclude_with_status( "watcher_excludes: {} has no `files.watcherExclude` object — treating as empty", path.display() ); - return (Vec::new(), WatcherExcludeLoadStatus::Parsed, 0); + return (Vec::new(), WatcherExcludeLoadStatus::Parsed); }; let mut patterns = Vec::new(); for (glob, include) in excludes { if include.as_bool() != Some(true) { - // `false` is an explicit non-exclude; non-booleans are ignored. continue; } - // A trailing `/` marks a directory-only pattern; strip it first. The - // anchoring decision uses the ORIGINAL glob: a trailing slash implies a - // slash, so an anchored (root-relative) pattern. let (glob, dir_only) = match glob.strip_suffix('/') { Some(trimmed) => (trimmed, true), None => (glob.as_str(), false), @@ -364,8 +356,6 @@ fn parse_vscode_watcher_exclude_with_status( .iter() .all(|segment| segment.as_ref().is_some_and(|c| c.text.is_empty())) { - // `"/"` (or a run of slashes) compiles to empty literal - // segments, which never match a real path component. repo_load_trace::trace!( "watcher_excludes: pattern {:?} in {} is empty and is ignored", glob, @@ -379,12 +369,7 @@ fn parse_vscode_watcher_exclude_with_status( dir_only, }); } - let parsed_rule_count = patterns.len(); - ( - patterns, - WatcherExcludeLoadStatus::Parsed, - parsed_rule_count, - ) + (patterns, WatcherExcludeLoadStatus::Parsed) } /// Whether the original pattern contains a slash (after stripping the trailing @@ -445,6 +430,7 @@ mod tests { assert_eq!(excludes.load_status(), WatcherExcludeLoadStatus::Missing); assert_eq!(excludes.parsed_rule_count(), 0); } + #[test] fn empty_settings_yield_empty_rules() { let dir = temp_workdir(); @@ -483,6 +469,7 @@ mod tests { assert!(excludes.is_excluded(rel("repos/x"), Some(true))); assert!(!excludes.is_excluded(rel("src"), Some(true))); } + #[test] fn jsonc_string_containing_slashes_is_not_a_comment() { let dir = temp_workdir(); From 8c357a3a34243728a9e0436e2e561a9d8f0d96e3 Mon Sep 17 00:00:00 2001 From: Ryan Lee Date: Mon, 17 Aug 2026 21:59:03 +0000 Subject: [PATCH 4/6] docs(plan): record JSONC watch-budget fix plan --- docs/plans/ide-excludes-jsonc-watch-budget.md | 154 ++++++++++++++++++ 1 file changed, 154 insertions(+) create mode 100644 docs/plans/ide-excludes-jsonc-watch-budget.md diff --git a/docs/plans/ide-excludes-jsonc-watch-budget.md b/docs/plans/ide-excludes-jsonc-watch-budget.md new file mode 100644 index 000000000..1e1d00ce9 --- /dev/null +++ b/docs/plans/ide-excludes-jsonc-watch-budget.md @@ -0,0 +1,154 @@ +--- +title: IDE Excludes Still Trip Watch Budget - Plan +type: fix +date: 2026-08-17 +artifact_contract: ce-unified-plan/v1 +artifact_readiness: implementation-ready +product_contract_source: ce-plan-bootstrap +execution: code +origin: docs/plans/respect-ide-watch-excludes.md +--- + +# IDE Excludes Still Trip Watch Budget - Plan + +## Goal Capsule + +- **Objective:** A worktree whose `.vscode/settings.json` excludes the high-cardinality trees (the motivating case is a tracked `repos/` vendor tree) stays under the Linux watch budget, so live watching stays on. +- **Authority:** The user report is the symptom. `docs/plans/respect-ide-watch-excludes.md` owns the exclude source and matcher. This plan only makes that source load for real IDE files and makes a still-over-budget warning tell the truth. +- **Execution profile:** Lightweight bugfix in `gitcomet-state`. Test-first on the JSONC load path. +- **Stop conditions:** R1–R5 hold and the Verification Contract gates pass. Do not raise `MAX_WORKTREE_WATCH_DIRS`, do not change host `fs.inotify.max_user_watches`, do not add user-level or non-VS-Code sources. + +--- + +## Product Contract + +### Summary + +GitComet already skips IDE-excluded directories in the Linux watch census. Real VS Code / Cursor workspace files are JSONC. The loader uses strict `serde_json`, so comments or a trailing comma empty the rule set. Tracked vendor trees then stay in the census, the walk stops at budget + 1, and the UI reports 4097 folders. + +### Problem Frame + +`crates/gitcomet-state/src/store/repo_monitor.rs` sets `MAX_WORKTREE_WATCH_DIRS` to 4096. `collect_watchable_dirs_capped` stops once the result is longer than `max_subdirs + 1`, so an over-budget repo always reports 4097 folders even when the real tree is much larger. + +`GitignoreMatcher::path_is_tracked` treats a directory as not-ignored when any index entry lives under it. A committed `repos/` or similar vendor tree is therefore never skipped by gitignore. Only `files.watcherExclude` can drop it. That is why the prior plan exists. + +`WatcherExcludes` reads only `/.vscode/settings.json`. `parse_vscode_watcher_exclude` calls `serde_json::from_str`. On error it returns an empty rule set and a `repo_load_trace` line. VS Code writes JSONC (line comments, block comments, trailing commas). The prior residual already names this as a documented limitation. The user-facing warning still says "Add build/output dirs to .gitignore" and never says the exclude file failed to load. + +### Requirements + +**Load** + +- R1. `WatcherExcludes::load(workdir, true)` accepts a `.vscode/settings.json` that is valid VS Code JSONC: `//` comments, `/* */` comments, and trailing commas. True `files.watcherExclude` entries from that file become exclude rules. +- R2. A file that is not JSON and not JSONC still yields an empty rule set. The load records a failed-parse status distinct from "file missing" and from "parsed, zero true entries". + +**Census** + +- R3. A tracked directory that matches a loaded exclude rule is absent from `collect_watchable_dirs_capped`, the same as today's strict-JSON path. + +**Warning** + +- R4. `RepoWatchDegradedReason::TooManyFolders` copy names `.vscode/settings.json` `files.watcherExclude` as a remedy, not only `.gitignore`. +- R5. When the settings file existed and failed to parse, that warning says the file could not be parsed. When the walk stopped at the cap, the count is worded as more than the budget, not as an exact folder total. + +### Acceptance Examples + +- AE1. `.vscode/settings.json` is JSONC with a comment, a trailing comma, and `"repos/": true`. `repos/` is tracked. `collect_watchable_dirs` does not contain `repos` or anything under it. (`Covers R1, R3`) +- AE2. The same file with `{ not json` yields zero rules and a failed-parse status. (`Covers R2`) +- AE3. Over-budget after a failed parse: the warning names the folder budget and says settings.json could not be parsed. (`Covers R4, R5`) +- AE4. Over-budget after a successful load: the warning names watcherExclude and does not claim the capped probe count is the exact tree size. (`Covers R4, R5`) + +### Scope Boundaries + +- **In** — JSONC load of the existing workspace file; load-status on the rule set; warning copy; tests that a JSONC `repos/` exclude drops a tracked tree from the census. +- **Deferred** — user-level VS Code / Cursor settings; `.code-workspace`; `files.exclude` / `search.exclude`; VS Code default watcherExclude; other IDEs. +- **Not this change** — `MAX_WORKTREE_WATCH_DIRS`; host inotify sysctl; gitignore tracked-path carve-out; matcher glob semantics. + +### Key Decisions + +- **Workspace file stays the only source.** Same source as the origin plan KTD1. User-level settings are a new source, not a load bug. Governs R1. +- **JSONC is the file format, not a best-effort strip.** Comments inside strings must survive. Governs R1, R2. + +--- + +## Planning Contract + +### Key Technical Decisions + +- KTD1. **Parse JSONC, then read `files.watcherExclude` as today.** Keep the existing glob matcher. Change only the text-to-`serde_json::Value` step in `parse_vscode_watcher_exclude`. +- KTD2. **Use a workspace-pinned JSONC crate that accepts comments and trailing commas.** Do not hand-roll a comment stripper. Do not switch to JSON5 (unquoted keys and other extras are not VS Code JSONC). Pin it under `[workspace.dependencies]` and take it from `gitcomet-state` the same way `serde_json` is taken. If two crates both cover JSONC, pick the smaller one that `serde_json::Value` can consume. Restrict parser options to comments and trailing commas; do not enable JSON5 extras. +- KTD3. **Record load status on `WatcherExcludes` as a `Copy` enum with no payloads:** `Disabled`, `Missing`, `Unreadable`, `Parsed`. Carry the true-entry count as a sibling `parsed_rule_count: usize` on the struct (0 when not `Parsed`). `WatcherExcludes::load(workdir, enabled) -> Self` stays the constructor. Add `pub(crate) fn load_status(&self) -> LoadStatus`. `WatcherExcludes::default()` is `Disabled` with count 0, so existing `default()` test fixtures keep compiling. Split parse into a private `parse_vscode_watcher_exclude_with_status(workdir) -> (Vec, LoadStatus, usize)` so the four early returns each set the matching variant. `is_excluded` is unchanged. +- KTD4. **Widen `TooManyFolders` and keep `RepoWatchDegradedReason` `Copy`.** Shape: `{ dir_count: usize, capped: bool, load_status: LoadStatus }`. `WatchSetupOutcome::WorktreeSubdirsSkipped` carries the same `{ dir_count, capped }` pair; set `capped = true` when the walk exited because `result.len() > max_subdirs + 1`. The reducer says "more than {budget} folders" when `capped`, else the literal count. R4 still names watcherExclude when status is `Disabled` (the toggle is the on-ramp). Recovery reload at `attempt_degraded_watch_recovery` assigns `WatcherExcludes::load(...)` so status refreshes with the rules. Update `repo_watch_degraded_pushes_warning_notification`. +- KTD5. **Do not raise the 4096 budget.** If JSONC load still leaves a repo over budget, the honest warning is the product. Raising the cap spends inotify watches the budget exists to protect. + +### Assumptions + +- The motivating repo's excludes live in workspace `.vscode/settings.json`, not only in user settings. That matches the origin plan's named file. +- A JSONC-capable crate can be added without touching UI crates. + +### Sequencing + +U1 (parse + status) then U2 (warning + census tests that need status). + +--- + +## Implementation Units + +### U1. JSONC load and status + +- **Goal:** Real VS Code settings load. Failed parse is distinguishable from missing or empty. +- **Requirements:** R1, R2 +- **Files:** `crates/gitcomet-state/src/store/watcher_excludes.rs`; `crates/gitcomet-state/Cargo.toml`; root `Cargo.toml` / `Cargo.lock` for the new workspace pin. +- **Approach:** Replace `serde_json::from_str` with the KTD2 parser. Keep the rest of `parse_vscode_watcher_exclude` (true-only entries, unsupported-syntax skip, empty-pattern skip). Implement KTD3 (`LoadStatus`, `load_status()`, `default()` = Disabled). Existing strict-JSON tests stay green. Add JSONC fixtures. +- **Test Scenarios** + - T1.1 JSONC with `//`, `/* */`, and a trailing comma loads `"repos/": true` and excludes `repos` and `repos/x`. Status is `Parsed`, count 1. + - T1.2 A string value containing `//` is not treated as a comment. + - T1.3 `{ not json` → zero rules, status = `Unreadable`. + - T1.4 Missing file → zero rules, status = `Missing`. + - T1.5 Valid JSON with no `files.watcherExclude` object → zero rules, status = `Parsed`, count 0. + - T1.6 Pre-existing strict-JSON tests in `watcher_excludes.rs` (`missing_file_yields_empty_rules`, `invalid_json_yields_empty_rules`, `only_true_entries_are_excludes`, and the other origin-plan matcher rows) still pass. +- **Verification:** `cargo test -p gitcomet-state watcher_excludes` + +### U2. Census proof and warning copy + +- **Goal:** A JSONC `repos/` exclude drops a tracked vendor tree from the watch set. The over-budget warning names the real remedy and does not lie about the count. +- **Requirements:** R3, R4, R5 +- **Files:** `crates/gitcomet-state/src/store/repo_monitor.rs`; `crates/gitcomet-state/src/msg/message.rs`; `crates/gitcomet-state/src/store/reducer.rs`; the existing `repo_watch_degraded_pushes_warning_notification` test. +- **Approach:** Thread `load_status()` and `capped` into `WorktreeSubdirsSkipped` / `TooManyFolders` per KTD4. When the walk hits the cap, the warning says more than `max_dirs` folders, not an exact total. Mention `files.watcherExclude`. If status is `Unreadable`, say settings.json could not be parsed. Keep `MAX_WORKTREE_WATCH_DIRS` at 4096. +- **Test Scenarios** + - T2.1 JSONC settings with `"repos/": true`, tracked files under `repos/`, plus a sibling `src/`: `collect_watchable_dirs` contains `src` and does not contain `repos`. Assert `load_status() == Parsed` and `parsed_rule_count == 1`. + - T2.2 Same fixture under a small injected budget: if only `repos/` pushed it over, setup is `Watching`, not `WorktreeSubdirsSkipped`. + - T2.3 Failed-parse settings plus an over-budget tree: warning text includes parse failure and watcherExclude. Assert `load_status() == Unreadable`. + - T2.4 Successful load, walk hits the cap: warning contains "more than" the budget and watcherExclude, and does not treat the probe length as an exact census. `capped` is true. + - T2.5 Existing `ide_excludes_apply_even_to_tracked_files` and `repo_watch_degraded_pushes_warning_notification` still pass after the reason shape change. + - T2.6 `WatcherExcludes::default()` reports `Disabled` and excludes nothing. +- **Verification:** `cargo test -p gitcomet-state repo_monitor repo_watch_degraded`; `cargo test -p gitcomet-state` + +--- + +## Verification Contract + +| Gate | Command | Applies to | Exit signal | +|---|---|---|---| +| Matcher + load | `cargo test -p gitcomet-state watcher_excludes` | U1 | All pass, including T1.1–T1.6 | +| Monitor + warning | `cargo test -p gitcomet-state repo_monitor repo_watch_degraded` | U2 | All pass, including T2.1–T2.6 | +| Crate | `cargo test -p gitcomet-state` | U1, U2 | All pass | +| Clippy | `cargo clippy -p gitcomet-state -- -D warnings` | U1, U2 | Clean | +| Compile | `cargo check -p gitcomet-state` | U1, U2 | Succeeds | + +No UI or browser surface. The settings toggle is unchanged. + +--- + +## Definition of Done + +**Global.** R1–R5 hold. Verification Contract gates green. Diff is this fix and its tests. No host sysctl change. No budget constant change. + +**Per-unit.** Each unit's tests fail on the bug they name: T1.1 fails if JSONC still yields an empty set; T2.1 fails if a tracked excluded tree stays in the census; T2.4 fails if the warning reports the capped probe as an exact total. + +**Cleanup.** No probe crates, no leftover parse-debug prints beyond the existing `repo_load_trace` / setup `eprintln` lines. + +--- + +## Appendix + +Software-wiki queries before this write (lex + vec + hyde, intent: JSONC `settings.json` parse; intent: inotify watch budget / too-many-folders) returned no settled answer. Top hits were plugin discovery and OpenCode context composition. The load bug is grounded in this repo: `serde_json::from_str` at `crates/gitcomet-state/src/store/watcher_excludes.rs`, budget 4096 at `crates/gitcomet-state/src/store/repo_monitor.rs`, tracked-path carve-out in `GitignoreMatcher::path_is_tracked`, residual note in `docs/residual-pi/respect-ide-watch-excludes.md`. From aa92734f6442439f40fe2cb318f2943c98889d9b Mon Sep 17 00:00:00 2001 From: Ryan Lee Date: Mon, 17 Aug 2026 22:00:47 +0000 Subject: [PATCH 5/6] docs(solutions): capture JSONC silent-empty watcher exclude bug --- .../vscode-settings-jsonc-silent-empty.md | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 docs/solutions/logic-errors/vscode-settings-jsonc-silent-empty.md diff --git a/docs/solutions/logic-errors/vscode-settings-jsonc-silent-empty.md b/docs/solutions/logic-errors/vscode-settings-jsonc-silent-empty.md new file mode 100644 index 000000000..f5534bf2e --- /dev/null +++ b/docs/solutions/logic-errors/vscode-settings-jsonc-silent-empty.md @@ -0,0 +1,58 @@ +--- +title: "VS Code settings JSONC parsed as empty watcher excludes" +date: 2026-08-17 +category: logic-errors +module: gitcomet-state watcher +problem_type: logic_error +component: background_job +symptoms: + - "Live watching stays disabled after IDE watcherExclude was added" + - "TooManyFolders warning still reports the capped folder budget" +root_cause: wrong_api +resolution_type: code_fix +severity: high +tags: [file-watcher, jsonc, watcher-exclude, vscode] +--- + +# VS Code settings JSONC parsed as empty watcher excludes + +## Problem + +Respecting IDE watcher excludes does not drop a tracked vendor tree from the Linux watch census when the workspace settings file is real VS Code JSONC. Live watching stays off and the UI still reports the capped folder budget. + +## Symptoms + +- The degraded-watch warning still fires after `files.watcherExclude` was added. +- The reported folder count is the budget plus one, not the real tree size. +- Git-ignore still counts committed vendor directories because they are tracked. + +## What Didn't Work + +- Adding `files.watcherExclude` in workspace settings. The matcher already honors true entries. The file never loaded. +- Strict JSON parse with a silent empty fallback. VS Code writes comments and trailing commas. Parse fails. The rule set is empty. The census is unchanged. +- Raising the watch budget or host inotify limits. That spends watches the budget exists to protect. + +## Solution + +Parse the workspace settings file as JSONC: line comments, block comments, and trailing commas. Restrict extras that are not VS Code JSONC (unquoted keys, single-quoted strings, hex numbers). Keep the existing glob matcher. + +Record load status as a Copy enum: Disabled, Missing, Unreadable, Parsed. When the file existed and did not parse, the degraded-watch warning says so. When the walk stopped at the cap, the count is worded as more than the budget. + +A comment-looking sequence inside a JSON string must stay a string. A fixture with a homepage URL containing `//` still loads the exclude map. + +## Why This Works + +The failure mode is fail-closed-to-watch: any parse problem yields zero excludes, so the census includes every tracked directory. Tracked vendor trees cannot be skipped by git-ignore. Only a loaded exclude rule can drop them. + +JSONC parse makes the file the user actually wrote produce rules. Load status makes a remaining over-budget case diagnosable instead of looking like excludes were ignored. + +## Prevention + +- Never treat an IDE settings file as strict JSON when the editor writes JSONC. +- Do not map every load failure to the same empty set without a status the warning can name. +- Pin a JSONC fixture with comments, a trailing comma, and a `//` inside a string. + +## Related Issues + +- Related design pattern: watcher exclude matcher invariants (dir-only gating, bare globstar, coalesced rebuilds) +- Residual test: injected-budget Watching outcome after a JSONC vendor-tree exclude (GitHub issue 58) From c93d1fe93468a97b15ae9ad04857afab31499868 Mon Sep 17 00:00:00 2001 From: Ryan Lee Date: Mon, 17 Aug 2026 22:55:17 +0000 Subject: [PATCH 6/6] fix(watch): strip UTF-8 BOM before parsing IDE settings jsonc-parser does not treat U+FEFF as whitespace, so a BOM-prefixed settings.json stayed Unreadable and dropped every watcherExclude. --- .../src/store/watcher_excludes.rs | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/gitcomet-state/src/store/watcher_excludes.rs b/crates/gitcomet-state/src/store/watcher_excludes.rs index 7839b2e84..a7f74d943 100644 --- a/crates/gitcomet-state/src/store/watcher_excludes.rs +++ b/crates/gitcomet-state/src/store/watcher_excludes.rs @@ -294,6 +294,10 @@ fn parse_vscode_watcher_exclude_with_status( return (Vec::new(), WatcherExcludeLoadStatus::Unreadable); } }; + // jsonc-parser does not treat U+FEFF as whitespace. A UTF-8 BOM is a + // leading format character, so it must be stripped or the file is + // Unreadable and every exclude is lost. + let text = text.strip_prefix('\u{feff}').unwrap_or(&text); let parse_options = jsonc_parser::ParseOptions { allow_comments: true, @@ -304,7 +308,7 @@ fn parse_vscode_watcher_exclude_with_status( allow_hexadecimal_numbers: false, allow_unary_plus_numbers: false, }; - let value: serde_json::Value = match jsonc_parser::parse_to_serde_value(&text, &parse_options) { + let value: serde_json::Value = match jsonc_parser::parse_to_serde_value(text, &parse_options) { Ok(value) => value, Err(error) => { repo_load_trace::trace!( @@ -470,6 +474,20 @@ mod tests { assert!(!excludes.is_excluded(rel("src"), Some(true))); } + #[test] + fn utf8_bom_prefixed_jsonc_still_loads_excludes() { + let dir = temp_workdir(); + write_settings( + dir.path(), + "\u{feff}{\n // workspace watcher excludes\n \"files.watcherExclude\": {\n \"repos/\": true,\n },\n }", + ); + let excludes = WatcherExcludes::load(dir.path(), true); + assert_eq!(excludes.load_status(), WatcherExcludeLoadStatus::Parsed); + assert_eq!(excludes.parsed_rule_count(), 1); + assert!(excludes.is_excluded(rel("repos"), Some(true))); + } + + #[test] fn jsonc_string_containing_slashes_is_not_a_comment() { let dir = temp_workdir();