fix(watcher): debounce fsnotify events + atomic index/links updates

vault/vault.go

@@ -9,6 +9,7 @@ import (
 	"path/filepath"
 	"strings"
 	"sync"
+	"time"
 
 	"github.com/fsnotify/fsnotify"
 	"github.com/google/uuid"
@@ -66,6 +67,14 @@ type Client struct {
 	searchIndex   *SearchIndex            // inverted index for full-text search
 	mu            sync.RWMutex            // protects all maps above
 	watcher       *fsnotify.Watcher       // file system watcher
+
+	// Debounce state for fsnotify events. Absorbs the back-to-back
+	// Remove+Create sequences produced by atomic temp+rename saves
+	// (internal write paths, iCloud sync, external editors). See
+	// watcher_debounce.go for the coalescer and atomic helpers.
+	pendingMu     sync.Mutex
+	pendingEvents map[string]*pendingEvent
+	debounceDelay time.Duration // 0 → defaultDebounceDelay (100ms)
 }
 
 // blockLookup stores a block and its page for UUID-based retrieval.
@@ -308,7 +317,10 @@ func (c *Client) watchLoop() {
 	}
 }
 
-// handleEvent processes a single fsnotify event.
+// handleEvent processes a single fsnotify event by scheduling a debounced
+// resolution. The actual resolve reads disk state after debounceDelay so it
+// absorbs the back-to-back Remove+Create sequences produced by atomic
+// temp+rename. See watcher_debounce.go for the coalescer and atomic helpers.
 func (c *Client) handleEvent(event fsnotify.Event) {
 	// Skip non-.md files.
 	if !strings.HasSuffix(event.Name, ".md") {
@@ -330,43 +342,26 @@ func (c *Client) handleEvent(event fsnotify.Event) {
 		return
 	}
 
+	// Coalesce every Create/Write/Remove/Rename per path. The final
+	// resolve (after debounceDelay) reads real disk state: present →
+	// reindex, absent → remove. Race-free against atomic rename.
 	switch {
-	case event.Op&fsnotify.Create == fsnotify.Create, event.Op&fsnotify.Write == fsnotify.Write:
-		// File created or modified: re-index it.
-		content, err := os.ReadFile(event.Name)
-		if err != nil {
-			log.Printf("graphthulhu: failed to read %s: %v\n", event.Name, err)
-			return
-		}
-		info, err := os.Stat(event.Name)
-		if err != nil {
-			log.Printf("graphthulhu: failed to stat %s: %v\n", event.Name, err)
-			return
-		}
-		c.indexFile(filepath.ToSlash(relPath), string(content), info)
-		c.BuildBacklinks()
-		log.Printf("graphthulhu: reindexed %s\n", relPath)
-
-	case event.Op&fsnotify.Remove == fsnotify.Remove:
-		// File deleted: remove from index.
-		name := strings.TrimSuffix(filepath.ToSlash(relPath), ".md")
-		lowerName := strings.ToLower(name)
-		c.removePageFromIndex(lowerName)
-		c.BuildBacklinks()
-		log.Printf("graphthulhu: removed %s from index\n", relPath)
-
-	case event.Op&fsnotify.Rename == fsnotify.Rename:
-		// File renamed: treat as remove (new name will trigger Create event).
-		name := strings.TrimSuffix(filepath.ToSlash(relPath), ".md")
-		lowerName := strings.ToLower(name)
-		c.removePageFromIndex(lowerName)
-		c.BuildBacklinks()
-		log.Printf("graphthulhu: removed %s from index (rename)\n", relPath)
+	case event.Op&(fsnotify.Create|fsnotify.Write|fsnotify.Remove|fsnotify.Rename) != 0:
+		c.scheduleEventResolution(event.Name, relPath)
 	}
 }
 
-// Close stops the file watcher.
+// Close stops the file watcher and any pending debounce timers.
+// Without this, timers scheduled before shutdown would still fire and
+// mutate c.pages / log spam after the server has stopped.
 func (c *Client) Close() error {
+	c.pendingMu.Lock()
+	for _, pe := range c.pendingEvents {
+		pe.timer.Stop()
+	}
+	c.pendingEvents = nil
+	c.pendingMu.Unlock()
+
 	if c.watcher != nil {
 		return c.watcher.Close()
 	}

vault/vault_test.go

@@ -656,8 +656,8 @@ func TestWatchFileCreate(t *testing.T) {
 		t.Fatalf("WriteFile: %v", err)
 	}
 
-	// Give the watcher time to process.
-	time.Sleep(100 * time.Millisecond)
+	// Give the watcher time to process (fsnotify event + 100ms debounce + resolve).
+	time.Sleep(250 * time.Millisecond)
 
 	// Should be indexed.
 	page, err := c.GetPage(ctx, "watched-create")
@@ -695,8 +695,8 @@ func TestWatchFileModify(t *testing.T) {
 		t.Fatalf("WriteFile modify: %v", err)
 	}
 
-	// Give the watcher time to process.
-	time.Sleep(100 * time.Millisecond)
+	// Give the watcher time to process (fsnotify event + 100ms debounce + resolve).
+	time.Sleep(250 * time.Millisecond)
 
 	// Should have updated content.
 	blocks, err := c.GetPageBlocksTree(ctx, "watched-modify")
@@ -743,8 +743,8 @@ func TestWatchFileDelete(t *testing.T) {
 		t.Fatalf("Remove: %v", err)
 	}
 
-	// Give the watcher time to process.
-	time.Sleep(100 * time.Millisecond)
+	// Give the watcher time to process (fsnotify event + 100ms debounce + resolve).
+	time.Sleep(250 * time.Millisecond)
 
 	// Should be gone from index.
 	page, err := c.GetPage(ctx, "watched-delete")

vault/watcher_debounce.go

@@ -0,0 +1,123 @@
+package vault
+
+import (
+	"log"
+	"os"
+	"path/filepath"
+	"strings"
+	"time"
+)
+
+// defaultDebounceDelay is how long we wait before resolving an fsnotify
+// event. 100ms absorbs the macOS / iCloud atomic temp+rename pattern
+// (Remove followed by Create within ~10ms) without noticeably delaying
+// real deletes or legitimate writes.
+const defaultDebounceDelay = 100 * time.Millisecond
+
+// pendingEvent pairs a debounce timer with the path it resolves.
+type pendingEvent struct {
+	absPath string
+	relPath string
+	timer   *time.Timer
+}
+
+// scheduleEventResolution coalesces fsnotify events for the same path.
+// A new event for an in-flight path resets the existing timer so we wait
+// debounceDelay again before resolving. The resolver reads disk state
+// and applies remove or reindex accordingly — race-free against atomic
+// temp+rename, since the final state always reflects what is on disk at
+// resolve time, no matter which Remove/Create sequence we received.
+func (c *Client) scheduleEventResolution(absPath, relPath string) {
+	c.pendingMu.Lock()
+	defer c.pendingMu.Unlock()
+
+	if c.pendingEvents == nil {
+		c.pendingEvents = make(map[string]*pendingEvent)
+	}
+
+	delay := c.debounceDelay
+	if delay == 0 {
+		delay = defaultDebounceDelay
+	}
+
+	if existing, ok := c.pendingEvents[absPath]; ok {
+		existing.timer.Stop()
+	}
+
+	pe := &pendingEvent{absPath: absPath, relPath: relPath}
+	pe.timer = time.AfterFunc(delay, func() {
+		c.resolveFileState(absPath, relPath)
+		c.pendingMu.Lock()
+		// Match by pointer identity: a concurrent schedule may have
+		// installed a newer pendingEvent between timer fire and lock
+		// acquisition. Don't delete that newer entry.
+		if cur, ok := c.pendingEvents[absPath]; ok && cur == pe {
+			delete(c.pendingEvents, absPath)
+		}
+		c.pendingMu.Unlock()
+	})
+	c.pendingEvents[absPath] = pe
+}
+
+// resolveFileState reads the current disk state and applies remove or
+// reindex. Called by the debounce timer. Race-free against fsnotify: the
+// final state always reflects what is on disk at the time of the call,
+// no matter which event sequence was delivered.
+func (c *Client) resolveFileState(absPath, relPath string) {
+	info, err := os.Stat(absPath)
+	if err != nil {
+		// File absent (real delete or unresolved rename). Remove for good.
+		name := strings.TrimSuffix(filepath.ToSlash(relPath), ".md")
+		lowerName := strings.ToLower(name)
+		c.removePageWithLinks(lowerName)
+		log.Printf("graphthulhu: removed %s from index\n", relPath)
+		return
+	}
+	content, err := os.ReadFile(absPath)
+	if err != nil {
+		log.Printf("graphthulhu: failed to read %s: %v\n", absPath, err)
+		return
+	}
+	c.indexFileWithLinks(filepath.ToSlash(relPath), string(content), info)
+	log.Printf("graphthulhu: reindexed %s\n", relPath)
+}
+
+// indexFileWithLinks parses, indexes a file, AND rebuilds backlinks under
+// a single lock. A reader cannot observe a state where the page exists in
+// c.pages but backlinks don't yet reflect its new content. Replaces the
+// previous indexFile + BuildBacklinks sequence (two separate lock
+// acquisitions with an observable intermediate window).
+func (c *Client) indexFileWithLinks(relPath, content string, info os.FileInfo) {
+	page := c.parseFile(relPath, content, info)
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	c.applyPageIndex(page)
+	c.rebuildLinksLocked()
+}
+
+// removePageWithLinks removes a page AND rebuilds backlinks under a
+// single lock, so the removal is atomic to readers.
+func (c *Client) removePageWithLinks(lowerName string) {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	c.removePageFromIndexLocked(lowerName)
+	c.rebuildLinksLocked()
+}
+
+// flushPendingEventsForTest synchronously resolves all pending events.
+// Used only by tests to avoid waiting for the real debounceDelay.
+// Production code has no reason to call this.
+func (c *Client) flushPendingEventsForTest() {
+	c.pendingMu.Lock()
+	pending := make([]*pendingEvent, 0, len(c.pendingEvents))
+	for _, pe := range c.pendingEvents {
+		pe.timer.Stop()
+		pending = append(pending, pe)
+	}
+	c.pendingEvents = nil
+	c.pendingMu.Unlock()
+
+	for _, pe := range pending {
+		c.resolveFileState(pe.absPath, pe.relPath)
+	}
+}