fix(watcher): detect NFS mounts and make WatchConfig.Enabled actually disable watching

[timkjr]

This fixes two related bugs that together explain why repos on NFS/SMB mounts silently stop being watched, with no error surfaced anywhere obvious.

1. slowWatchMount never recognized NFS

The slow-mount gate only checked for WSL2's 9p/drvfs magic number and (within a WSL context) CIFS. A native Linux host with a repo on a plain NFS mount fell through every safety net — NFS_SUPER_MAGIC (0x6969) was never checked. Native fsnotify was then attempted regardless, reliably failing confirmWatchActive's 5s readiness window, leaving the repo with neither fsnotify nor the poller fallback until a manual untrack+track.

2. WatchConfig.Enabled didn't actually disable watching

config.Default() ships Watch.Enabled: false — checked into gortex's own .gortex.yaml too. But Watcher.Start only gated the safe branches (the slow-mount skip-to-poller path from fix #1, and launching the poller as a fallback) behind Enabled. It never gated the actual attempt to start native fsnotify, which ran unconditionally regardless of the flag. So with the shipped default, every repo blindly attempted raw fsnotify, racing the same 5s timeout with no safety net and no fallback — meaning fix #1 was effectively dead code unless a user manually set watch.enabled: true, which is undocumented anywhere. This PR adds an early return when Enabled is false, matching what the surrounding comments already claimed the flag did.

Known related gap, not fixed here: slow_mount_other.go (the non-Linux build) still returns false unconditionally, so a macOS host with a repo on an NFS mount gets no slow-mount protection either — the same class of bug this PR closes for Linux. I don't have a Mac to test against, so I've left that alone rather than guess at the right statfs-equivalent check; happy to take a pass at it if a maintainer can confirm the right approach (or point at existing platform-detection code I should mirror).

Verified: go test -race ./... and golangci-lint run both clean. Manually confirmed on a live NFS-mounted multi-repo daemon — all repos went from erratic per-boot watcher-attach failures to 100% success (fsnotify where safe, poller fallback on slow mounts).

Separately flagging for maintainer discussion, not changed here: is Enabled: false the right default? A code-intelligence tool whose core value is live indexing arguably shouldn't ship watching off by default with no documentation pointing at the flag.

[zzet]

Thanks for this — the investigation behind it is genuinely good, and you found two real bugs, not one. The NFS detection half is correct and I want it. But the WatchConfig.Enabled half has to change before this can merge: as written it disables live indexing for every repo the daemon tracks, under the shipped default.

Blocker: the Enabled early return turns watching off everywhere

The premise that Enabled gates watching isn't quite right, and the daemon is the case that proves it. cmd/gortex/daemon_state.go:987 builds the per-repo watch configs straight from repo config, with no override:

watchCfgs[prefix] = state.configManager.GetRepoConfig(prefix).Watch

The only place in the tree that sets Watch.Enabled = true is cmd/gortex/mcp.go:507, the gortex mcp --watch path. Since config.Default() ships enabled: false, the new early return fires for every repo under gortex daemon: no fsnotify, no poller, no live indexing at all.

I verified this rather than reading it. A test that drives config.Default().Watch through Start() fails on this branch and passes on main:

this branch:  file change never reached the graph — repo has neither fsnotify nor poller
main:         change detected, live indexing works

It also silently defeats the staleness alarm. The early return yields nil, so MultiWatcher.Start records started = true, and WatchedRepos() then reports live == configured — which means the warning at daemon_state.go:1013, added for exactly this symptom, never fires:

// Reporting the configured count here made an install where every watcher had
// failed to start look fully watched, so the one signal that the graph was
// going stale never fired.

The daemon would log daemon: watching repos=N configured=N and publish watched_repos: N while nothing is watching anything.

The underlying reason for the misread is fair: Enabled has never meant "watch at all" — it gates the adaptive poller only, and fsnotify has always run regardless. TestPoller_RespectsWatcherDisableKnob (internal/indexer/poller_test.go:214) is the contract. The flag is badly named, which is the actual defect you walked into.

What this PR gets right — please keep both

1. NFS detection. slowWatchMount being gated behind runningUnderWSL() meant NFS_SUPER_MAGIC (0x6969) and native-Linux CIFS were never checked. Correct diagnosis, correct magic numbers, and factoring out isSlowMountFSType for direct testing is the right call.
2. Ungating the degraded-path pollers. Both the slow-mount branch and the confirmWatchActive readiness-failure branch built the fallback poller only if w.config.Enabled — so under the default, a repo whose fsnotify died got neither mechanism. That is a second genuine dead-repo bug and your removal of those two gates is exactly right. It's the part of your report I'd most want fixed.

Suggested fix (verified)

Drop the early return, keep both degraded-path pollers ungated as you have them, and re-gate only the final alongside-healthy-fsnotify poller:

// Enabled is the opt-in only 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()
}

With that, TestPoller_RespectsWatcherDisableKnob and a new daemon-default regression test both pass, and the full indexer suite is green (1573 passed).

Why the suite didn't catch it

Your go test -race ./... result is accurate — all 1649 indexer + server tests pass on this branch. The gap is that every watcher test hardcodes Enabled: true (see setupWatcher), so the daemon's actual default is never exercised. Please add a test that drives config.Default().Watch through Start() and asserts a file change reaches the graph; that's the guard that would have caught this.

Please split out the tool-promotion commits

Four of the eight commits and seven of the twelve files here are the deferred-MCP-tool promotion work (SetToolPromoter / getToolOrPromote), which the description doesn't mention. It's substantively sound — EnsureToolPromoted already exists, Promote is mutex-guarded, and hidden tools are never deferred so the hide gate isn't bypassed — but it's unrelated to the watcher fix and needs to be reviewed on its own. Two things to carry into that PR:

• Concurrency: lazyToolRegistry.Promote marks promoted[name] under the lock but calls promoteFn outside it, so a concurrent second first-call for the same tool gets false back and skips the retry lookup — a spurious "not registered". The established idiom at cmd/gortex/daemon_mcp.go:155 uses that boolean for bookkeeping only and proceeds regardless; worth matching.
• Session surface: daemon_mcp.go deliberately checks IsToolEnabledForSession before touching the process-global lazy registry. The new newLocalToolExecutor path in server_router.go calls EnsureToolPromoted directly, skipping that guard.

On your two open questions

Is Enabled: false the right default? Given it actually gates the poller, yes — fsnotify on, poller off, and the poller auto-engaging when fsnotify is dead once your fix #2 lands. What should change is the name, not the default. A follow-up renaming it to something like FallbackPoller (with the old key accepted for a release) would be very welcome.

slow_mount_other.go on macOS: leaving it alone was the right instinct, and no, don't mirror the Linux approach — Darwin's statfs carries f_fstypename as a string, so the check there is a name comparison against nfs / smbfs / webdav rather than a magic number. Happy to take that as a separate PR from you if you want it; it doesn't need to block this one.

One thing I checked and am not asking you to change: int64(st.Type) would truncate CIFS_MAGIC on a 32-bit host, but only amd64 and arm64 ship, and both have Statfs_t.Type int64. Not a problem.

Thanks again for chasing this down to the actual mechanism — the report quality made it fast to verify.

[timkjr]

Thanks for the detailed review — you were right on all counts, and it exposed a real gap in the test suite that I've closed.

The blocker (Enabled disabling everything) — fixed exactly as suggested. Dropped the !Enabled early return entirely. Both degraded-path pollers (slow-mount fallback, inotify/FD-exhaustion fallback) now start unconditionally, as they did before my regression. Only the final poller — the one running alongside healthy fsnotify — stays gated on w.config.Enabled:

if w.config.Enabled {
    w.poller = newPoller(w, w.indexer, w.logger)
    w.poller.Start()
}

New test — added TestWatcher_ShippedDefaultStillWatches, which drives the real config.Default().Watch (asserting Enabled == false rather than hardcoding it, so this can't drift silently) through Start(), confirms the poller is nil, and asserts a file change still reaches the graph — proving it arrives via fsnotify, not the poller. TestPoller_RespectsWatcherDisableKnob and the full internal/indexer suite pass under -race.

Split the tool-promotion commits out — done. This branch is now rebased to 4 commits against current main, touching only internal/indexer/{watcher.go, slow_mount_linux.go, slow_mount_test.go, slow_watch_mount_test.go}. The deferred-MCP-tool-promotion work belongs with #649, which is already open and further along (it's since picked up a session-identity fix this branch's copy didn't have) — I'll bring your two flagged bugs (the Promote concurrency issue and the newLocalToolExecutor session-policy bypass) over there rather than duplicating/regressing that work here.

On the two open questions — agreed on both. Enabled: false is the right default; I'll open the FallbackPoller rename as a follow-up rather than bundling it here. And leaving slow_mount_other.go alone was intentional — happy to take the Darwin f_fstypename follow-up as a separate PR if useful, just let me know.

[timkjr]

Following up on the two bugs from your split-out ask — both are already fixed, no new PR needed.

lazyToolRegistry.Promote's concurrency issue and newLocalToolExecutor's session-surface bypass turned out to be exactly round-1 findings #1 and #3 from your own review on #649 (that branch carried a stale pre-review snapshot of the same code). #649 merged with both fixed — the lock now wraps mark-and-register in one critical section (TestPromote_ConcurrentCallersNeverFalse404 pins it), and the dispatch path calls EnsureToolPromotedForSession instead of the bare EnsureToolPromoted, so a hidden-session tool is never promoted on its behalf (TestLocalExecutor_HiddenSessionDeniedWithoutPromotion/...EvenWhenAlreadyLive). Verified both under -race on merged main.

For the record, since it's adjacent: #649's round 2 flagged the bigger issue (remote-proxied calls skipping the origin session's policy/workspace-boundary enforcement) as a "potential security issue," and the fix that landed only closes the local half by design — the remote/federation half is tracked separately in #696, still open. Not something this PR needs to touch, just flagging so it doesn't look dropped.