From 985cb43c5392ea080bff92f02e57d0e5b87b9bb0 Mon Sep 17 00:00:00 2001 From: Jordan Coin Jackson Date: Sun, 2 Aug 2026 20:11:34 -0400 Subject: [PATCH] fix(watch): Rebuild the dependency graph after a filter change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refreshConfiguredFiles invalidated the dependency graph and left it that way, with a comment deferring the rebuild to "restart". Nothing rebuilds it lazily, so a single edit to .codemap/config.json — or a touch of any .gitignore, which filterControlEvent matches by basename anywhere in the tree — permanently stripped hub and importer intelligence from the running daemon. Every hook that reads daemon state silently degraded from that point on. Rebuild after invalidating, behind the same shouldComputeDependencyGraph guard Start uses so large repos still skip the work. computeDeps takes the graph lock itself, so it is called after the unlock. TestConfiguredFilterChangeInvalidatesDependencyState asserted the graph stayed nil, which encoded the defect rather than the intent. Its real invariant is that state computed under the old filters is discarded, so it now asserts the stale entries are gone; the published-state check is unchanged. A new test covers the rebuild, using a sentinel entry so a rebuilt graph is distinguishable from the startup graph. Follow-up to #95 by @reneleonhardt. Co-Authored-By: Claude Opus 5 (1M context) --- watch/daemon.go | 12 +++++++-- watch/more_test.go | 65 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 73 insertions(+), 4 deletions(-) diff --git a/watch/daemon.go b/watch/daemon.go index 3140042..bdd8198 100644 --- a/watch/daemon.go +++ b/watch/daemon.go @@ -228,12 +228,20 @@ func (d *Daemon) refreshConfiguredFiles(resetIgnoreCache bool) error { } d.graph.mu.Lock() d.graph.ConfiguredFiles = configured - // Filters define dependency membership too. Do not publish the previous - // graph under a new configured-file count; rebuild lazily on restart. + // Filters define dependency membership too, so the previous graph must not + // be published under a new configured-file count. d.graph.FileGraph = nil d.graph.DepCtx = make(map[string]*DepContext) d.graph.HasDeps = false d.graph.mu.Unlock() + + // Invalidation alone would leave the daemon serving no hub or importer + // intelligence until it restarts, so every hook reading daemon state would + // silently degrade after one config edit. Rebuild under the same size guard + // Start uses. computeDeps takes the lock itself, so call it unlocked. + if shouldComputeDependencyGraph(len(configured)) { + d.computeDeps() + } return nil } diff --git a/watch/more_test.go b/watch/more_test.go index d7cf1b4..fbf9a2e 100644 --- a/watch/more_test.go +++ b/watch/more_test.go @@ -302,10 +302,21 @@ func TestConfiguredFilterChangeInvalidatesDependencyState(t *testing.T) { if err := os.WriteFile(configPath, []byte(`{"only":["sql"]}`), 0o644); err != nil { t.Fatal(err) } - waitForWatchCondition(t, 2*time.Second, func() bool { + // The invariant is that state computed under the old filters is discarded, + // not that the graph is left destroyed: refreshConfiguredFiles rebuilds it + // (see TestConfiguredFilterChangeRebuildsDependencyGraph), so assert the + // stale entries are gone rather than that the graph is nil. + waitForWatchCondition(t, 5*time.Second, func() bool { d.graph.mu.RLock() defer d.graph.mu.RUnlock() - return !d.graph.HasDeps && d.graph.FileGraph == nil && len(d.graph.DepCtx) == 0 + if _, stale := d.graph.DepCtx["old.go"]; stale { + return false + } + if d.graph.FileGraph == nil { + return true + } + _, stale := d.graph.FileGraph.Importers["old.go"] + return !stale }) state := ReadState(root) if state == nil || len(state.Hubs) != 0 || len(state.Imports) != 0 || len(state.Importers) != 0 { @@ -446,3 +457,53 @@ func TestDaemonStartTracksWriteEventsAndState(t *testing.T) { t.Fatalf("expected watch state with recent events, got %+v", state) } } + +// TestConfiguredFilterChangeRebuildsDependencyGraph pins that invalidating the +// dependency graph after a filter change is followed by rebuilding it. Dropping +// it and waiting for a restart leaves the daemon serving no hub or importer +// intelligence for the rest of its life, so every hook that reads daemon state +// silently degrades after a single config edit. +func TestConfiguredFilterChangeRebuildsDependencyGraph(t *testing.T) { + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, ".codemap"), 0o755); err != nil { + t.Fatal(err) + } + configPath := filepath.Join(root, ".codemap", "config.json") + if err := os.WriteFile(configPath, []byte(`{"only":["go"]}`), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "main.go"), []byte("package main\n\nfunc main() {}\n"), 0o644); err != nil { + t.Fatal(err) + } + d, err := NewDaemon(root, false) + if err != nil { + t.Fatal(err) + } + if err := d.Start(); err != nil { + t.Fatal(err) + } + defer d.Stop() + + // Plant a sentinel so a rebuilt graph is distinguishable both from the + // startup graph and from one that was merely dropped. + d.graph.mu.Lock() + d.graph.FileGraph = &scanner.FileGraph{Importers: map[string][]string{"stale.go": {"a.go"}}} + d.graph.DepCtx = map[string]*DepContext{"stale.go": {Importers: []string{"a.go"}}} + d.graph.HasDeps = true + d.graph.mu.Unlock() + + // Widen the filters; Go files stay configured, so dependency intelligence + // must come back rather than stay dropped. + if err := os.WriteFile(configPath, []byte(`{"only":["go","md"]}`), 0o644); err != nil { + t.Fatal(err) + } + waitForWatchCondition(t, 5*time.Second, func() bool { + d.graph.mu.RLock() + defer d.graph.mu.RUnlock() + if !d.graph.HasDeps || d.graph.FileGraph == nil { + return false + } + _, stale := d.graph.FileGraph.Importers["stale.go"] + return !stale + }) +}