fix: address CodeRabbit review on entity live reload

Stop obsolete entity file watchers on config reload, guard callbacks
against removed paths, and ignore stale getEntities responses in the
Entities page when reload events overlap.

Co-authored-by: Cursor <cursoragent@cursor.com>

frontend/resources/vue/views/EntitiesView.vue

@@ -29,6 +29,7 @@ import Section from 'picocrank/vue/components/Section.vue'
 import EntityDefinitionSection from '../components/EntityDefinitionSection.vue'
 const definitionsLoaded = ref(false)
 const entityDefinitions = ref([])
+let entityFetchGeneration = 0
 
 const totalInstances = computed(() =>
   entityDefinitions.value.reduce(
@@ -38,15 +39,20 @@ const totalInstances = computed(() =>
 )
 
 async function fetchEntities () {
+  const fetchGeneration = ++entityFetchGeneration
   try {
     const ret = await window.client.getEntities()
+    if (fetchGeneration !== entityFetchGeneration) return
     entityDefinitions.value = ret.entityDefinitions ?? []
   } catch (err) {
+    if (fetchGeneration !== entityFetchGeneration) return
     console.error('Failed to fetch entities:', err)
     window.showBigError('fetch-entities', 'getting entities', err, false)
     entityDefinitions.value = []
   } finally {
-    definitionsLoaded.value = true
+    if (fetchGeneration === entityFetchGeneration) {
+      definitionsLoaded.value = true
+    }
   }
 }
 

service/internal/entities/entities.go

@@ -19,10 +19,15 @@ var (
 	EntityChangedSender chan bool
 	listeners           []func()
 
-	watchedMu    sync.Mutex
-	watchedPaths = map[string]struct{}{}
+	watchedMu       sync.Mutex
+	watchedBindings = map[string]entityWatchBinding{}
 )
 
+type entityWatchBinding struct {
+	entityName string
+	sourceFile string
+}
+
 type Entity struct {
 	Data      any
 	UniqueKey string
@@ -38,13 +43,10 @@ func SetupEntityFileWatchers(cfg *config.Config) {
 }
 
 // SyncEntityFileWatchers ensures each configured entity file has a watcher and reloads
-// entity data. Safe to call on config reload; already-watched paths are not watched twice.
+// entity data. Safe to call on config reload; obsolete watchers are stopped.
 func SyncEntityFileWatchers(cfg *config.Config) {
 	baseDir := ResolveEntitiesBaseDir(cfg.GetDir())
-	for i := range cfg.Entities { // #337 - iterate by key, not by value
-		ef := cfg.Entities[i]
-		syncEntityFileWatcher(baseDir, ef)
-	}
+	reconcileEntityWatchers(desiredEntityWatchers(baseDir, cfg.Entities))
 }
 
 // ResolveEntitiesBaseDir returns the directory used to resolve relative entity file paths.
@@ -82,23 +84,71 @@ func resolveEntityFilePath(baseDir string, file string) string {
 	return p
 }
 
-func syncEntityFileWatcher(baseDir string, ef *config.EntityFile) {
-	p := resolveEntityFilePath(baseDir, ef.File)
+func desiredEntityWatchers(baseDir string, entityFiles []*config.EntityFile) map[string]entityWatchBinding {
+	desired := make(map[string]entityWatchBinding, len(entityFiles))
+	for i := range entityFiles { // #337 - iterate by key, not by value
+		ef := entityFiles[i]
+		path := resolveEntityFilePath(baseDir, ef.File)
+		desired[path] = entityWatchBinding{
+			entityName: ef.Name,
+			sourceFile: ef.SourceFile,
+		}
+	}
+	return desired
+}
+
+func reconcileEntityWatchers(desired map[string]entityWatchBinding) {
+	stopObsoleteEntityWatchers(desired)
+	for path, binding := range desired {
+		ensureEntityFileWatcher(path, binding)
+	}
+}
 
+func stopObsoleteEntityWatchers(desired map[string]entityWatchBinding) {
 	watchedMu.Lock()
-	_, alreadyWatched := watchedPaths[p]
-	if !alreadyWatched {
-		watchedPaths[p] = struct{}{}
+	defer watchedMu.Unlock()
+
+	for path, binding := range watchedBindings {
+		wanted, ok := desired[path]
+		if ok && wanted == binding {
+			continue
+		}
+		filehelper.StopFileWatch(path)
+		delete(watchedBindings, path)
 	}
+}
+
+func ensureEntityFileWatcher(path string, binding entityWatchBinding) {
+	watchedMu.Lock()
+	existing, watching := watchedBindings[path]
+	if watching && existing == binding {
+		watchedMu.Unlock()
+		loadEntityFile(path, binding.entityName)
+		return
+	}
+	watchedBindings[path] = binding
 	watchedMu.Unlock()
 
-	if !alreadyWatched {
-		go filehelper.WatchFileWrite(p, func(_ string) { loadEntityFile(p, ef.Name) }, filehelper.WatchMeta{
-			ConfigFile: ef.SourceFile,
-		})
+	filehelper.WatchFileWrite(path, makeEntityFileWatchCallback(path), filehelper.WatchMeta{
+		ConfigFile: binding.sourceFile,
+	})
+	loadEntityFile(path, binding.entityName)
+}
+
+func makeEntityFileWatchCallback(path string) func(string) {
+	return func(_ string) {
+		entityFileWatchCallback(path)
 	}
+}
 
-	loadEntityFile(p, ef.Name)
+func entityFileWatchCallback(path string) {
+	watchedMu.Lock()
+	binding, ok := watchedBindings[path]
+	watchedMu.Unlock()
+	if !ok {
+		return
+	}
+	loadEntityFile(path, binding.entityName)
 }
 
 func loadEntityFile(filename string, entityname string) {

service/internal/entities/entities_test.go

@@ -85,6 +85,76 @@ func TestSyncEntityFileWatchers_doesNotDuplicateWatchers(t *testing.T) {
 	assert.Equal(t, 1, watchedPathCountForTests())
 }
 
+func TestSyncEntityFileWatchers_removedPathDoesNotReload(t *testing.T) {
+	ResetEntityWatchersForTests()
+	t.Cleanup(func() {
+		ResetEntityWatchersForTests()
+		ClearEntitiesOfType("vehicle")
+	})
+
+	dir := t.TempDir()
+	vehiclePath := filepath.Join(dir, "vehicles.yaml")
+	require.NoError(t, os.WriteFile(vehiclePath, []byte("- title: car1\n"), 0o600))
+
+	cfg := config.DefaultConfig()
+	cfg.SetDir(dir)
+	cfg.Entities = []*config.EntityFile{
+		{Name: "vehicle", File: "vehicles.yaml"},
+	}
+
+	SyncEntityFileWatchers(cfg)
+	require.Equal(t, 1, watchedPathCountForTests())
+	require.Len(t, GetEntityInstancesOrdered("vehicle"), 1)
+
+	cfg.Entities = nil
+	SyncEntityFileWatchers(cfg)
+	assert.Equal(t, 0, watchedPathCountForTests())
+
+	ClearEntitiesOfType("vehicle")
+	require.NoError(t, os.WriteFile(vehiclePath, []byte("- title: car2\n"), 0o600))
+	entityFileWatchCallback(vehiclePath)
+	assert.Empty(t, GetEntityInstancesOrdered("vehicle"))
+}
+
+func TestSyncEntityFileWatchers_replacedPathUsesNewBinding(t *testing.T) {
+	ResetEntityWatchersForTests()
+	t.Cleanup(func() {
+		ResetEntityWatchersForTests()
+		ClearEntitiesOfType("vehicle")
+	})
+
+	dir := t.TempDir()
+	vehiclePath := filepath.Join(dir, "vehicles.yaml")
+	carsPath := filepath.Join(dir, "cars.yaml")
+	require.NoError(t, os.WriteFile(vehiclePath, []byte("- title: car1\n"), 0o600))
+	require.NoError(t, os.WriteFile(carsPath, []byte("- title: car2\n"), 0o600))
+
+	cfg := config.DefaultConfig()
+	cfg.SetDir(dir)
+	cfg.Entities = []*config.EntityFile{
+		{Name: "vehicle", File: "vehicles.yaml"},
+	}
+
+	SyncEntityFileWatchers(cfg)
+	require.Equal(t, 1, watchedPathCountForTests())
+	require.Equal(t, "car1", GetEntityInstancesOrdered("vehicle")[0].Title)
+
+	cfg.Entities = []*config.EntityFile{
+		{Name: "vehicle", File: "cars.yaml"},
+	}
+	SyncEntityFileWatchers(cfg)
+	require.Equal(t, 1, watchedPathCountForTests())
+	require.Equal(t, "car2", GetEntityInstancesOrdered("vehicle")[0].Title)
+
+	require.NoError(t, os.WriteFile(vehiclePath, []byte("- title: stale\n"), 0o600))
+	entityFileWatchCallback(vehiclePath)
+	assert.Equal(t, "car2", GetEntityInstancesOrdered("vehicle")[0].Title, "obsolete path should not reload")
+
+	require.NoError(t, os.WriteFile(carsPath, []byte("- title: car3\n"), 0o600))
+	entityFileWatchCallback(carsPath)
+	assert.Equal(t, "car3", GetEntityInstancesOrdered("vehicle")[0].Title)
+}
+
 func TestSyncEntityFileWatchers_picksUpNewEntityType(t *testing.T) {
 	ResetEntityWatchersForTests()
 	t.Cleanup(func() {

service/internal/entities/load_state.go

@@ -1,6 +1,10 @@
 package entities
 
-import "sync"
+import (
+	"sync"
+
+	"github.com/OliveTin/OliveTin/internal/filehelper"
+)
 
 var (
 	loadAttemptedMu sync.RWMutex
@@ -36,11 +40,14 @@ func ResetEntityLoadAttempts() {
 func ResetEntityWatchersForTests() {
 	watchedMu.Lock()
 	defer watchedMu.Unlock()
-	watchedPaths = map[string]struct{}{}
+	for path := range watchedBindings {
+		filehelper.StopFileWatch(path)
+	}
+	watchedBindings = map[string]entityWatchBinding{}
 }
 
 func watchedPathCountForTests() int {
 	watchedMu.Lock()
 	defer watchedMu.Unlock()
-	return len(watchedPaths)
+	return len(watchedBindings)
 }

service/internal/filehelper/file_change_notify.go

@@ -15,6 +15,9 @@ var (
 	debounceWriteLog map[string]*FsNotifyLogEntry
 
 	debounceWriteLogMutex = sync.Mutex{}
+
+	fileWatchMu    sync.Mutex
+	fileWatchStops = map[string]chan struct{}{}
 )
 
 func init() {
@@ -54,7 +57,7 @@ func WatchDirectoryCreate(fullpath string, callback func(filename string), meta
 		callback:        callback,
 		interestedEvent: fsnotify.Create,
 		meta:            meta,
-	})
+	}, nil)
 }
 
 func WatchDirectoryWrite(fullpath string, callback func(filename string), meta WatchMeta) {
@@ -64,23 +67,57 @@ func WatchDirectoryWrite(fullpath string, callback func(filename string), meta W
 		callback:        callback,
 		interestedEvent: fsnotify.Write,
 		meta:            meta,
-	})
+	}, nil)
 }
 
 func WatchFileWrite(fullpath string, callback func(filename string), meta WatchMeta) {
 	filename := filepath.Base(fullpath)
 	filedir := filepath.Dir(fullpath)
+	watchKey := filepath.Join(filedir, filename)
 
-	watchPath(&watchContext{
+	done := registerFileWatch(watchKey)
+	go watchPath(&watchContext{
 		filedir:         filedir,
 		filename:        filename,
 		callback:        callback,
 		interestedEvent: fsnotify.Write,
 		meta:            meta,
-	})
+	}, done)
 }
 
-func watchPath(ctx *watchContext) {
+// StopFileWatch stops a file write watcher started by WatchFileWrite.
+func StopFileWatch(fullpath string) {
+	watchKey, err := filepath.Abs(fullpath)
+	if err != nil {
+		watchKey = fullpath
+	}
+
+	fileWatchMu.Lock()
+	done, ok := fileWatchStops[watchKey]
+	if ok {
+		delete(fileWatchStops, watchKey)
+	}
+	fileWatchMu.Unlock()
+
+	if ok {
+		close(done)
+	}
+}
+
+func registerFileWatch(watchKey string) chan struct{} {
+	absKey, err := filepath.Abs(watchKey)
+	if err == nil {
+		watchKey = absKey
+	}
+
+	done := make(chan struct{})
+	fileWatchMu.Lock()
+	fileWatchStops[watchKey] = done
+	fileWatchMu.Unlock()
+	return done
+}
+
+func watchPath(ctx *watchContext, done <-chan struct{}) {
 	watcher, err := fsnotify.NewWatcher()
 	if err != nil {
 		reportWatcherFailure(ctx, err)
@@ -94,7 +131,7 @@ func watchPath(ctx *watchContext) {
 		return
 	}
 
-	for processEvent(ctx, watcher) {
+	for processEvent(ctx, watcher, done) {
 	}
 }
 
@@ -127,8 +164,26 @@ func reportWatcherFailure(ctx *watchContext, err error) {
 
 // processEvent waits for one watcher event. It returns false when the watcher
 // channels are closed so the caller can stop looping.
-func processEvent(ctx *watchContext, watcher *fsnotify.Watcher) bool {
+func processEvent(ctx *watchContext, watcher *fsnotify.Watcher, done <-chan struct{}) bool {
+	if done == nil {
+		return waitForWatcherEvent(ctx, watcher)
+	}
+	return waitForWatcherEventOrCancel(ctx, watcher, done)
+}
+
+func waitForWatcherEvent(ctx *watchContext, watcher *fsnotify.Watcher) bool {
+	select {
+	case event, ok := <-watcher.Events:
+		return handleWatcherEvent(ctx, event, ok)
+	case err, ok := <-watcher.Errors:
+		return handleWatcherError(ctx, err, ok)
+	}
+}
+
+func waitForWatcherEventOrCancel(ctx *watchContext, watcher *fsnotify.Watcher, done <-chan struct{}) bool {
 	select {
+	case <-done:
+		return false
 	case event, ok := <-watcher.Events:
 		return handleWatcherEvent(ctx, event, ok)
 	case err, ok := <-watcher.Errors: