From 7e9637351581cae60c24e1eef883c721c6277807 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 2 Jun 2026 08:23:05 +0000 Subject: [PATCH] watch: non-recursive config file watching with dynamic include tracking --- tools/watch/api.go | 111 +++++++++++++++------- tools/watch/api_test.go | 200 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 279 insertions(+), 32 deletions(-) diff --git a/tools/watch/api.go b/tools/watch/api.go index e08632fd7..0a1acc229 100644 --- a/tools/watch/api.go +++ b/tools/watch/api.go @@ -21,27 +21,16 @@ import ( var _ = fmt.Print -// watch_dir starts fswatcher in a background goroutine and pipes events to a custom channel. -func watch_dirs(ctx context.Context, paths []string, debounce time.Duration, eventChan chan<- fswatcher.WatchEvent) error { - opts := []fswatcher.WatcherOpt{ - fswatcher.WithCooldown(debounce), +// get_parent_dirs returns a deduplicated list of the immediate parent directory for each path. +// Unlike get_unique_directories it does not filter out subdirectories, so every unique +// parent is returned even when some are descendants of others. This is the correct +// set of directories to pass to a non-recursive (top-level) file-system watcher. +func get_parent_dirs(paths []string) []string { + dirSet := utils.NewSet[string](len(paths)) + for _, p := range paths { + dirSet.Add(filepath.Dir(p)) } - for _, path := range paths { - if unix.Access(path, unix.R_OK|unix.X_OK) == nil { - opts = append(opts, fswatcher.WithPath(path)) - } - } - w, err := fswatcher.New(opts...) - if err != nil { - return err - } - go w.Watch(ctx) - go func() { - for event := range w.Events() { - eventChan <- event - } - }() - return nil + return dirSet.AsSlice() } // returns the closest unique parent directories for a list of paths. @@ -122,33 +111,91 @@ func get_set_of_config_files(config_paths []string) *utils.Set[string] { return result } -// watch_for_config_changes watches the directories derived from config_paths and calls action -// whenever a watched config file (including includes and auto color scheme files) changes. +// watch_for_config_changes watches the parent directories of every conf file (main configs, +// includes, and auto color-scheme files) and calls action whenever one of those files changes. +// Watching is non-recursive (top-level only): only the immediate parent directories are added. +// When a conf file change is detected the full set of conf files is re-scanned so that newly +// added or removed include directives are reflected in the watched-directory set. // It runs until ctx is cancelled. func watch_for_config_changes(ctx context.Context, action func() error, debounce_time time.Duration, config_paths []string) error { event_chan := make(chan fswatcher.WatchEvent) + all_paths := get_set_of_config_files(config_paths) - dirs_to_watch := get_unique_directories(all_paths.AsSlice()) - if len(dirs_to_watch) == 0 { + + // desired_dirs is the full set of parent directories we want to watch + // (one per conf file, including files that may not yet exist). + desired_dirs := utils.NewSet[string]() + for _, p := range all_paths.AsSlice() { + desired_dirs.Add(filepath.Dir(p)) + } + if desired_dirs.Len() == 0 { return fmt.Errorf("No directories to watch provided") } - filtered_action := func(ev fswatcher.WatchEvent) error { - all_paths := get_set_of_config_files(config_paths) - if all_paths.Has(resolve_path(ev.Path)) { - return action() + // Create the watcher with top-level (non-recursive) depth. + opts := []fswatcher.WatcherOpt{fswatcher.WithCooldown(debounce_time)} + watched_dirs := utils.NewSet[string]() + for _, dir := range desired_dirs.AsSlice() { + if unix.Access(dir, unix.R_OK|unix.X_OK) == nil { + opts = append(opts, fswatcher.WithPath(dir, fswatcher.WithDepth(fswatcher.WatchTopLevel))) + watched_dirs.Add(dir) } - return nil + } + if watched_dirs.Len() == 0 { + return fmt.Errorf("No directories to watch provided") } - if err := watch_dirs(ctx, dirs_to_watch, debounce_time, event_chan); err != nil { + w, err := fswatcher.New(opts...) + if err != nil { return err } + go w.Watch(ctx) + go func() { + for event := range w.Events() { + event_chan <- event + } + }() + + // sync_watched_dirs reconciles watched_dirs with desired_dirs: any desired directory + // that now exists is added to the watcher, and any watched directory that is no longer + // desired is dropped. + sync_watched_dirs := func() { + desired_dirs.ForEach(func(d string) { + if !watched_dirs.Has(d) && unix.Access(d, unix.R_OK|unix.X_OK) == nil { + if err := w.AddPath(d, fswatcher.WithDepth(fswatcher.WatchTopLevel)); err == nil { + watched_dirs.Add(d) + } + } + }) + watched_dirs.ForEach(func(d string) { + if !desired_dirs.Has(d) { + _ = w.DropPath(d) + watched_dirs.Discard(d) + } + }) + } + for { select { case event := <-event_chan: - if err := filtered_action(event); err != nil { - fmt.Fprintf(os.Stderr, "failed to signal kitty in event: %s with error: %s\n", event, err) + // On every event try to activate any desired directories that may have been + // created since the last check (e.g. a new include directory was mkdir'd). + sync_watched_dirs() + + new_all_paths := get_set_of_config_files(config_paths) + if new_all_paths.Has(resolve_path(event.Path)) { + // A conf file changed: rebuild desired_dirs from the new include set and + // sync the watcher so new include directories are watched and stale ones dropped. + new_desired := utils.NewSet[string]() + for _, p := range new_all_paths.AsSlice() { + new_desired.Add(filepath.Dir(p)) + } + desired_dirs = new_desired + sync_watched_dirs() + + if err := action(); err != nil { + fmt.Fprintf(os.Stderr, "failed to signal kitty in event: %s with error: %s\n", event, err) + } } case <-ctx.Done(): return nil diff --git a/tools/watch/api_test.go b/tools/watch/api_test.go index d28cf289b..4f10aeef9 100644 --- a/tools/watch/api_test.go +++ b/tools/watch/api_test.go @@ -22,6 +22,68 @@ func write_file(t *testing.T, path string, data string) { } } +func TestGetParentDirs(t *testing.T) { + type tc struct { + name string + input []string + expect []string + } + cases := []tc{ + { + name: "nil input", + input: nil, + expect: nil, + }, + { + name: "single file", + input: []string{"/a/b/file.conf"}, + expect: []string{"/a/b"}, + }, + { + name: "files in same directory", + input: []string{"/a/b/file1.conf", "/a/b/file2.conf"}, + expect: []string{"/a/b"}, + }, + { + name: "sibling directories", + input: []string{"/a/b/file.conf", "/a/c/file.conf"}, + expect: []string{"/a/b", "/a/c"}, + }, + { + name: "parent and child directory both returned", + // Unlike get_unique_directories, subdirectories are NOT filtered out. + input: []string{"/a/file.conf", "/a/b/file.conf"}, + expect: []string{"/a", "/a/b"}, + }, + { + name: "deeply nested all returned", + input: []string{ + "/a/file.conf", + "/a/b/file.conf", + "/a/b/c/file.conf", + }, + expect: []string{"/a", "/a/b", "/a/b/c"}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + result := get_parent_dirs(tc.input) + sort.Strings(result) + sort.Strings(tc.expect) + if tc.expect == nil { + if len(result) != 0 { + t.Fatalf("expected empty result, got %v", result) + } + return + } + if diff := cmp.Diff(tc.expect, result); diff != "" { + t.Fatalf("get_parent_dirs mismatch (-want +got):\n%s", diff) + } + }) + } +} + func TestGetUniqueDirectories(t *testing.T) { // Empty input if result := get_unique_directories(nil); result != nil { @@ -283,3 +345,141 @@ func TestWatchForConfigChangesDebounce(t *testing.T) { t.Fatal("watch_for_config_changes did not exit after context cancel") } } + +// TestWatchForConfigChangesIncludeAdded verifies that when the main config gains a new +// include directive the parent directory of the new include file is added to the watcher, +// and subsequent changes to that include file trigger the action. +func TestWatchForConfigChangesIncludeAdded(t *testing.T) { + tdir := resolve_path(t.TempDir()) + extradir := filepath.Join(tdir, "extra") + if err := os.Mkdir(extradir, 0o700); err != nil { + t.Fatal(err) + } + + main_conf := filepath.Join(tdir, "kitty.conf") + extra_conf := filepath.Join(extradir, "custom.conf") + + // Start with no includes so extradir is NOT initially watched. + write_file(t, main_conf, "font_size 12\n") + // Create the include file before modifying kitty.conf so the re-scan can find it. + write_file(t, extra_conf, "background black\n") + + var action_count atomic.Int32 + action := func() error { + action_count.Add(1) + return nil + } + + const debounce = 50 * time.Millisecond + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + done := make(chan error, 1) + go func() { + done <- watch_for_config_changes(ctx, action, debounce, []string{main_conf}) + }() + + // Give the watcher time to start. + time.Sleep(200 * time.Millisecond) + + // Step 1: add the include directive to kitty.conf. + before_add := action_count.Load() + write_file(t, main_conf, "font_size 12\ninclude extra/custom.conf\n") + count := wait_for_count(&action_count, before_add+1, 2*time.Second) + if count <= before_add { + t.Fatalf("Expected action after kitty.conf gained include directive, count=%d", count) + } + + // Give the watcher a moment to register the new directory. + time.Sleep(200 * time.Millisecond) + + // Step 2: modify the newly included file; it should now be watched. + before_extra := action_count.Load() + write_file(t, extra_conf, "background white\n") + count = wait_for_count(&action_count, before_extra+1, 2*time.Second) + if count <= before_extra { + t.Fatalf("Expected action after modifying newly included file, count=%d", count) + } + + cancel() + select { + case err := <-done: + if err != nil { + t.Fatalf("watch_for_config_changes returned error: %v", err) + } + case <-time.After(2 * time.Second): + t.Fatal("watch_for_config_changes did not exit after context cancel") + } +} + +// TestWatchForConfigChangesIncludeRemoved verifies that when the main config loses an +// include directive the parent directory of the removed include file is dropped from the +// watcher, and subsequent changes to that file no longer trigger the action. +func TestWatchForConfigChangesIncludeRemoved(t *testing.T) { + tdir := resolve_path(t.TempDir()) + subdir := filepath.Join(tdir, "sub") + if err := os.Mkdir(subdir, 0o700); err != nil { + t.Fatal(err) + } + + main_conf := filepath.Join(tdir, "kitty.conf") + sub_conf := filepath.Join(subdir, "extra.conf") + + // Start with an include so subdir IS initially watched. + write_file(t, main_conf, "font_size 12\ninclude sub/extra.conf\n") + write_file(t, sub_conf, "background black\n") + + var action_count atomic.Int32 + action := func() error { + action_count.Add(1) + return nil + } + + const debounce = 50 * time.Millisecond + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + done := make(chan error, 1) + go func() { + done <- watch_for_config_changes(ctx, action, debounce, []string{main_conf}) + }() + + // Give the watcher time to start. + time.Sleep(200 * time.Millisecond) + + // Confirm sub_conf is currently watched by verifying a change triggers action. + before_verify := action_count.Load() + write_file(t, sub_conf, "background blue\n") + if wait_for_count(&action_count, before_verify+1, 2*time.Second) <= before_verify { + t.Fatal("sub_conf changes should trigger action before include is removed") + } + + // Step 1: remove the include directive from kitty.conf. + before_remove := action_count.Load() + write_file(t, main_conf, "font_size 12\n") + if wait_for_count(&action_count, before_remove+1, 2*time.Second) <= before_remove { + t.Fatal("Expected action after kitty.conf lost include directive") + } + + // Give the watcher a moment to drop the old directory. + time.Sleep(200 * time.Millisecond) + + // Step 2: modify the no-longer-included file; the watch should have been removed. + before_after := action_count.Load() + write_file(t, sub_conf, "background green\n") + time.Sleep(debounce + 200*time.Millisecond) + after := action_count.Load() + if after != before_after { + t.Fatalf("Expected NO action after modifying removed-include file, count went from %d to %d", before_after, after) + } + + cancel() + select { + case err := <-done: + if err != nil { + t.Fatalf("watch_for_config_changes returned error: %v", err) + } + case <-time.After(2 * time.Second): + t.Fatal("watch_for_config_changes did not exit after context cancel") + } +}