diff --git a/internal/indexer/poller_test.go b/internal/indexer/poller_test.go index 28a555aa8..c91e1b568 100644 --- a/internal/indexer/poller_test.go +++ b/internal/indexer/poller_test.go @@ -209,8 +209,10 @@ func TestPoller_DetectsGitHeadMoveMissedByFsnotify(t *testing.T) { // TestPoller_RespectsWatcherDisableKnob verifies the poller honours // the per-repo watcher-disable knob: when WatchConfig.Enabled is -// false, Start must not create a poller — the disabled repo gets no -// fallback either. +// false, Start must not create the alongside-fsnotify poller. This +// is not the whole story on a slow mount, where the degraded-path +// pollers still start unconditionally regardless of Enabled — see +// TestWatcher_ShippedDefaultStillWatches and the slow-mount tests. func TestPoller_RespectsWatcherDisableKnob(t *testing.T) { dir := t.TempDir() writeTestFile(t, filepath.Join(dir, "main.go"), "package main\n\nfunc Main() {}\n") diff --git a/internal/indexer/slow_mount_linux.go b/internal/indexer/slow_mount_linux.go index 738cccd1c..5795fc2f6 100644 --- a/internal/indexer/slow_mount_linux.go +++ b/internal/indexer/slow_mount_linux.go @@ -4,42 +4,40 @@ package indexer import ( "os" - "strings" "syscall" ) // slowWatchMount reports whether path lives on a filesystem where native -// fsnotify is unreliable or prohibitively slow — notably a Windows drive -// surfaced into WSL2 via 9p/drvfs (inotify events arrive late or never), or -// an SMB/CIFS share. On such a mount the watcher disables fsnotify and -// relies on the adaptive poller + git hooks. GORTEX_FORCE_FSNOTIFY=1 forces -// native fsnotify on regardless. +// fsnotify is unreliable or prohibitively slow — a Windows drive surfaced +// into WSL2 via 9p/drvfs (inotify events arrive late or never), an SMB/CIFS +// share (whether mounted directly or via WSL2), or an NFS mount (the kernel +// inotify backend does not reliably notice changes made by another NFS +// client, and even same-client notifications can arrive late enough to miss +// the watcher's readiness window entirely — see confirmWatchActive's 5s +// timeout). On such a mount the watcher disables fsnotify and relies on the +// adaptive poller + git hooks, which are mount-agnostic. +// GORTEX_FORCE_FSNOTIFY=1 forces native fsnotify on regardless. func slowWatchMount(path string) bool { if path == "" || os.Getenv("GORTEX_FORCE_FSNOTIFY") == "1" { return false } - if !runningUnderWSL() { - return false - } var st syscall.Statfs_t if err := syscall.Statfs(path, &st); err != nil { return false } - switch int64(st.Type) { + return isSlowMountFSType(int64(st.Type)) +} + +// isSlowMountFSType is the magic-number check slowWatchMount applies to a +// statfs result. Factored out so it can be unit-tested directly against +// known-bad magic numbers without needing a live WSL2, SMB, or NFS mount in +// the test environment. +func isSlowMountFSType(fsType int64) bool { + switch fsType { case 0x01021997, // V9FS_MAGIC — 9p, WSL2's drvfs transport for Windows drives - 0xFF534D42: // CIFS_MAGIC — SMB/CIFS share + 0xFF534D42, // CIFS_MAGIC — SMB/CIFS share + 0x6969: // NFS_SUPER_MAGIC — NFS v3/v4 return true } return false } - -// runningUnderWSL reports whether the process is inside the Windows -// Subsystem for Linux, probed from /proc/version's microsoft/WSL marker. -func runningUnderWSL() bool { - b, err := os.ReadFile("/proc/version") - if err != nil { - return false - } - v := strings.ToLower(string(b)) - return strings.Contains(v, "microsoft") || strings.Contains(v, "wsl") -} diff --git a/internal/indexer/slow_mount_test.go b/internal/indexer/slow_mount_test.go index d871a9e7b..49dcab9b5 100644 --- a/internal/indexer/slow_mount_test.go +++ b/internal/indexer/slow_mount_test.go @@ -1,15 +1,35 @@ +//go:build linux + package indexer import "testing" -// TestSlowWatchMount_NormalMountNotDegraded guards the safe default: a -// normal local filesystem (the test temp dir) must never be flagged as a -// slow mount, so fsnotify is only disabled on a genuine WSL2 9p/SMB mount. -func TestSlowWatchMount_NormalMountNotDegraded(t *testing.T) { - if slowWatchMount(t.TempDir()) { - t.Error("a normal local mount must not be flagged slow (fsnotify must stay enabled)") +// TestIsSlowMountFSType pins the magic-number check slowWatchMount applies +// to a statfs result, independent of the host actually having a WSL2, SMB, +// or NFS mount available to probe. NFS (0x6969, NFS_SUPER_MAGIC) previously +// went undetected on any non-WSL2 host: slowWatchMount's own filesystem-type +// check was gated behind a WSL-only probe, so a native Linux host with a +// repo on an NFS mount fell through every safety net — the fsnotify backend +// then reliably failed its 5s readiness wait, and because that failure path +// returns before the adaptive poller is ever constructed, the repo ended up +// with neither fsnotify nor the poller: no update mechanism at all until a +// manual untrack+track. +func TestIsSlowMountFSType(t *testing.T) { + cases := []struct { + name string + typ int64 + want bool + }{ + {"ext4/xfs/local (typical, unlisted)", 0xEF53, false}, + {"V9FS (WSL2 9p/drvfs)", 0x01021997, true}, + {"CIFS/SMB", 0xFF534D42, true}, + {"NFS_SUPER_MAGIC", 0x6969, true}, } - if slowWatchMount("") { - t.Error("an empty path must not be flagged slow") + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if got := isSlowMountFSType(c.typ); got != c.want { + t.Errorf("isSlowMountFSType(%#x) = %v, want %v", c.typ, got, c.want) + } + }) } } diff --git a/internal/indexer/slow_watch_mount_test.go b/internal/indexer/slow_watch_mount_test.go new file mode 100644 index 000000000..407105b45 --- /dev/null +++ b/internal/indexer/slow_watch_mount_test.go @@ -0,0 +1,31 @@ +package indexer + +import ( + "runtime" + "testing" +) + +// TestSlowWatchMount_NormalMountNotDegraded guards the safe default: a +// normal local filesystem must never be flagged as a slow mount, so +// fsnotify is only disabled on a genuine slow-mount type. Runs on every +// platform, unlike TestIsSlowMountFSType (Linux-only, in +// slow_mount_test.go) which pins the statfs magic-number check itself +// against known constants and is the sturdier of the two checks. +// +// On Linux, slowWatchMount does a live statfs of the given path, so +// probing t.TempDir() here depends on TMPDIR actually sitting on local +// disk — not guaranteed on every CI runner. TestIsSlowMountFSType +// already covers the Linux logic against a known-local magic number +// (0xEF53), so skip the live probe there and keep it only on platforms +// where slowWatchMount is a static stub (always false, can't flake). +func TestSlowWatchMount_NormalMountNotDegraded(t *testing.T) { + if slowWatchMount("") { + t.Error("an empty path must not be flagged slow") + } + if runtime.GOOS == "linux" { + t.Skip("Linux magic-number logic is pinned by TestIsSlowMountFSType against known constants, not a live TMPDIR probe") + } + if slowWatchMount(t.TempDir()) { + t.Error("a normal local mount must not be flagged slow (fsnotify must stay enabled)") + } +} diff --git a/internal/indexer/watcher.go b/internal/indexer/watcher.go index 6303383e9..182518a0d 100644 --- a/internal/indexer/watcher.go +++ b/internal/indexer/watcher.go @@ -419,24 +419,23 @@ func (w *Watcher) Start(paths []string) (retErr error) { } // WSL2 / slow-mount degradation: on a 9p/drvfs mount (a Windows drive - // under WSL2, an SMB share) native fsnotify delivers events late or not - // at all, and confirmWatchActive would hang ~5s per path before timing - // out. Skip the fsnotify backend entirely and rely on the adaptive - // poller + git hooks, which are mount-agnostic. The downstream code - // already tolerates a nil fsw. GORTEX_FORCE_FSNOTIFY=1 overrides. - if w.config.Enabled { - probe := paths[0] - if abs, err := filepath.Abs(probe); err == nil { - probe = abs - } - if slowWatchMount(probe) { - w.degradedNoFsnotify = true - w.logger.Warn("watcher: slow mount detected — disabling native fsnotify, using adaptive poller fallback", - zap.String("path", probe)) - w.poller = newPoller(w, w.indexer, w.logger) - w.poller.Start() - return nil - } + // under WSL2, an SMB share) or an NFS mount, native fsnotify delivers + // events late or not at all, and confirmWatchActive would hang ~5s per + // path before timing out. Skip the fsnotify backend entirely and rely + // on the adaptive poller + git hooks, which are mount-agnostic. The + // downstream code already tolerates a nil fsw. GORTEX_FORCE_FSNOTIFY=1 + // overrides. + probe := paths[0] + if abs, err := filepath.Abs(probe); err == nil { + probe = abs + } + if slowWatchMount(probe) { + w.degradedNoFsnotify = true + w.logger.Warn("watcher: slow mount detected — disabling native fsnotify, using adaptive poller fallback", + zap.String("path", probe)) + w.poller = newPoller(w, w.indexer, w.logger) + w.poller.Start() + return nil } ready := make(chan struct{}) // Own the events/dropped channels so the library never closes them on @@ -543,10 +542,8 @@ func (w *Watcher) Start(paths []string) (retErr error) { w.fsw.Close() w.fsw = nil } - if w.config.Enabled { - w.poller = newPoller(w, w.indexer, w.logger) - w.poller.Start() - } + w.poller = newPoller(w, w.indexer, w.logger) + w.poller.Start() return nil } return err @@ -608,10 +605,13 @@ func (w *Watcher) Start(paths []string) (retErr error) { } } - // Launch the adaptive-interval poller alongside the fsnotify - // backend. It is a fallback for the changes fsnotify misses, so - // it shares the watcher's lifecycle. Gated on WatchConfig.Enabled - // — a repo that opted out of watching gets no fallback either. + // Launch the adaptive-interval poller alongside the fsnotify backend. + // It is a fallback for the changes fsnotify misses, so it shares the + // watcher's lifecycle. Enabled is the opt-in only here, where + // fsnotify is LIVE — there the poller is a belt-and-braces extra for + // what fsnotify misses, and a repo may decline it. The degraded + // paths above start it unconditionally: fsnotify is dead there, so + // declining it means the repo silently goes stale. if w.config.Enabled { w.poller = newPoller(w, w.indexer, w.logger) w.poller.Start() diff --git a/internal/indexer/watcher_test.go b/internal/indexer/watcher_test.go index 2d2a4f79b..16f3bc934 100644 --- a/internal/indexer/watcher_test.go +++ b/internal/indexer/watcher_test.go @@ -58,6 +58,54 @@ func writeTestFile(t *testing.T, path, content string) { require.NoError(t, os.WriteFile(path, []byte(content), 0o644)) } +// TestWatcher_ShippedDefaultStillWatches drives config.Default().Watch — +// Enabled: false, exactly what every repo under `gortex daemon` gets with +// no override — through Start() and asserts a file change still reaches +// the graph. Every other watcher test in this package hardcodes +// Enabled: true, so none of them exercise the daemon's actual default; +// that gap once let Enabled: false silently disable fsnotify itself +// (not just the adaptive poller) without any test catching it. +func TestWatcher_ShippedDefaultStillWatches(t *testing.T) { + dir := t.TempDir() + writeTestFile(t, filepath.Join(dir, "main.go"), `package main + +func Original() {} +`) + + g := graph.New() + reg := parser.NewRegistry() + reg.Register(languages.NewGoExtractor()) + cfg := config.Default() + cfg.Index.Workers = 1 + + idx := New(g, reg, cfg.Index, zap.NewNop()) + _, err := idx.Index(dir) + require.NoError(t, err) + + wcfg := cfg.Watch + require.False(t, wcfg.Enabled, "config.Default().Watch must ship Enabled: false") + wcfg.Paths = []string{dir} + wcfg.DebounceMs = 50 // short debounce for tests + + w, err := NewWatcher(idx, wcfg, zap.NewNop()) + require.NoError(t, err) + require.NoError(t, w.Start([]string{dir})) + t.Cleanup(func() { _ = w.Stop() }) + + assert.Nil(t, w.poller, + "the shipped default disables the adaptive poller only, not fsnotify") + + writeTestFile(t, filepath.Join(dir, "main.go"), `package main + +func Modified() {} +`) + + ev := waitForEvent(t, w, 2*time.Second) + assert.Equal(t, ChangeModified, ev.Kind) + assert.NotEmpty(t, idx.graph.FindNodesByName("Modified"), + "a file change under the shipped watch default must reach the graph via fsnotify") +} + func waitForEvent(t *testing.T, w *Watcher, timeout time.Duration) GraphChangeEvent { t.Helper() select {