From 7421aad5695cb3b2d0c984ebe84d4fc25c4cb07c Mon Sep 17 00:00:00 2001 From: mike-inkeep Date: Fri, 24 Jul 2026 16:44:32 -0400 Subject: [PATCH] fix(server): chokidar fallback watches subfolders (#760) (#2904) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(server): chokidar fallback watches subfolders (#760) The file watcher prefers @parcel/watcher and falls back to chokidar whenever that native module can't load (every packaged desktop build today). The fallback's `ignored` predicate misclassified any subdirectory chokidar probed WITHOUT file stats — routing it through the file-only `isExcluded`, which default-excludes any non-`.md`/ non-asset name — so chokidar pruned every content subfolder and silently stopped watching it. External edits to docs in subfolders never reached the server; the graph, backlinks, and dead-link views stayed stale until a restart re-read from disk. Extract the predicate as `isChokidarPathIgnored` and lstat a stats-less path to resolve dir-vs-file before deciding (lstat matches followSymlinks:false); a path that can't be stat'd is admitted, not pruned, so a mid-scan delete still propagates. Add a `forceBackend` test seam to `startWatcher` (mirrors head-watcher) and cover the regression: a subfolder edit, a doc created in a brand-new subfolder after watch start, node_modules staying pruned, plus a predicate stats matrix. * fix(server): await chokidar `ready` so the watcher is live before startWatcher resolves The chokidar backend's live-subfolder tests were flaky on Linux CI — even the root-level control edit was missed. `chokidar.watch()` returns before its initial scan + per-directory inotify registration finish, so an edit made immediately after `startWatcher` resolved landed in the setup window and never fired (macOS FSEvents arms fast enough to hide it locally). Await chokidar's `ready` event before returning, bounded by a 10s cap so a pathological scan can't wedge boot. Also space the new-subfolder test's mkdir and write so the create isn't lost to the inotify addDir→subwatch race. * test(server): sentinel-anchor the node_modules-pruned negative assertion Address PR review: the excluded-subtree test replaced its fixed 600ms sleep with a sentinel edit to a watched file. Waiting for the sentinel to dispatch proves the watcher was live through the window, so the absent node_modules event is a real negative rather than a vacuous pass if the watcher never started. * test(server): anchor the new-subfolder sub-watch on the folder-create event The fixed 200ms sleep guessed at how long chokidar takes to arm a sub-watch on a freshly created directory — too short under load, wasted wall time when not. Waiting for the folder-create DiskEvent is the deterministic signal that the addDir was processed. --------- GitOrigin-RevId: 8b6d1d9bc0fbc9f84ca93d82192ee1dd421dcb73 --- .../chokidar-fallback-subfolder-watching.md | 5 + .../file-watcher-chokidar-fallback.test.ts | 187 ++++++++++++++++++ packages/server/src/file-watcher.ts | 98 +++++++-- 3 files changed, 275 insertions(+), 15 deletions(-) create mode 100644 .changeset/chokidar-fallback-subfolder-watching.md create mode 100644 packages/server/src/file-watcher-chokidar-fallback.test.ts diff --git a/.changeset/chokidar-fallback-subfolder-watching.md b/.changeset/chokidar-fallback-subfolder-watching.md new file mode 100644 index 00000000..a2091874 --- /dev/null +++ b/.changeset/chokidar-fallback-subfolder-watching.md @@ -0,0 +1,5 @@ +--- +"@inkeep/open-knowledge": patch +--- + +Fixed external edits to files in subfolders not being picked up when the server runs on its chokidar file-watching fallback (inkeep/open-knowledge#760). The server prefers `@parcel/watcher` but falls back to chokidar whenever that native module can't load. The fallback's ignore predicate misclassified any subdirectory that chokidar probed without file stats — routing it through the file-only exclusion rule, which rejects anything that isn't a Markdown doc or linkable asset — so chokidar pruned every content subfolder and silently stopped watching it. The result: editing a note in a subfolder with an external editor (or a `git pull` landing changes there) never reached the server, and the graph, backlinks, and dead-link views stayed stale until the next restart re-read everything from disk. The predicate now resolves whether a stats-less path is a directory before deciding, so subfolders are watched again; a path that can't be stat'd (for example a file deleted mid-scan) is admitted rather than pruned, so its delete still propagates. Excluded trees such as `node_modules/` and `.git/` stay pruned as before. diff --git a/packages/server/src/file-watcher-chokidar-fallback.test.ts b/packages/server/src/file-watcher-chokidar-fallback.test.ts new file mode 100644 index 00000000..05037891 --- /dev/null +++ b/packages/server/src/file-watcher-chokidar-fallback.test.ts @@ -0,0 +1,187 @@ +/** + * Regression: the chokidar fallback (used whenever @parcel/watcher can't be + * loaded — every packaged desktop build today) must watch SUBDIRECTORIES. + * + * The `ignored` predicate used to route a stats-less directory through the + * file-only `isExcluded`, which default-excludes any non-`.md`/non-asset name, + * so chokidar pruned every content subfolder and no external edit under one + * ever reached the server (graph / backlinks / dead-links stayed stale until a + * restart rebuilt from disk). inkeep/open-knowledge#760. + */ +import { lstatSync, mkdirSync, statSync, writeFileSync } from 'node:fs'; +import { mkdtemp, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { resolve } from 'node:path'; +import { afterEach, beforeEach, describe, expect, test } from 'vitest'; +import { createContentFilter } from './content-filter.ts'; +import { + type DiskEvent, + isChokidarPathIgnored, + lastKnownHash, + startWatcher, + writeTracker, +} from './file-watcher.ts'; + +describe('isChokidarPathIgnored — stats matrix', () => { + let tmpDir: string; + let contentDir: string; + + beforeEach(async () => { + tmpDir = await mkdtemp(resolve(tmpdir(), 'ok-chokidar-ignored-')); + contentDir = resolve(tmpDir, 'content'); + mkdirSync(resolve(contentDir, 'sub'), { recursive: true }); + mkdirSync(resolve(contentDir, 'node_modules', 'pkg'), { recursive: true }); + writeFileSync(resolve(contentDir, 'sub', 'note.md'), '# Note\n'); + }); + + afterEach(async () => { + await rm(tmpDir, { recursive: true, force: true }); + }); + + test('the content dir root is never ignored', () => { + const filter = createContentFilter({ projectDir: tmpDir, contentDir }); + expect(isChokidarPathIgnored(contentDir, filter, contentDir, statSync(contentDir))).toBe(false); + }); + + test('a content subdirectory is NOT ignored — with stats and (the bug) without', () => { + const filter = createContentFilter({ projectDir: tmpDir, contentDir }); + const subDir = resolve(contentDir, 'sub'); + // With directory stats: routed through isDirExcluded → descendable. + expect(isChokidarPathIgnored(contentDir, filter, subDir, statSync(subDir))).toBe(false); + // WITHOUT stats (chokidar's subwatch gate): pre-fix this fell through to + // the file-only isExcluded and returned true, pruning the subtree. + expect(isChokidarPathIgnored(contentDir, filter, subDir, undefined)).toBe(false); + }); + + test('a markdown file is not ignored; a stats-less non-content file routes through isExcluded', () => { + const filter = createContentFilter({ projectDir: tmpDir, contentDir }); + const mdFile = resolve(contentDir, 'sub', 'note.md'); + expect(isChokidarPathIgnored(contentDir, filter, mdFile, lstatSync(mdFile))).toBe(false); + // A real, extension-less file: `isExcluded` excludes it (not a doc/asset) + // while `isDirExcluded` would admit it, so a `true` here proves the + // stats-less path lstat'd to the FILE branch, not the directory branch. + writeFileSync(resolve(contentDir, 'sub', 'Makefile'), 'x'); + const plain = resolve(contentDir, 'sub', 'Makefile'); + expect(filter.isExcluded('sub/Makefile')).toBe(true); + expect(filter.isDirExcluded('sub/Makefile')).toBe(false); + expect(isChokidarPathIgnored(contentDir, filter, plain, undefined)).toBe(true); + }); + + test('excluded directories stay pruned even without stats (node_modules)', () => { + const filter = createContentFilter({ projectDir: tmpDir, contentDir }); + const nm = resolve(contentDir, 'node_modules'); + expect(isChokidarPathIgnored(contentDir, filter, nm, undefined)).toBe(true); + }); + + test('a nonexistent path with no stats is admitted (never prune on uncertainty)', () => { + const filter = createContentFilter({ projectDir: tmpDir, contentDir }); + const gone = resolve(contentDir, 'sub', 'was-just-deleted.md'); + expect(isChokidarPathIgnored(contentDir, filter, gone, undefined)).toBe(false); + }); +}); + +describe('chokidar backend — live subfolder watching (forceBackend)', () => { + let tmpDir: string; + let contentDir: string; + + beforeEach(async () => { + tmpDir = await mkdtemp(resolve(tmpdir(), 'ok-chokidar-live-')); + contentDir = resolve(tmpDir, 'content'); + mkdirSync(resolve(contentDir, 'sub'), { recursive: true }); + writeFileSync(resolve(contentDir, 'root.md'), '# Root\n'); + writeFileSync(resolve(contentDir, 'sub', 'note.md'), '# Note\n\n[Root](./root)\n'); + lastKnownHash.clear(); + writeTracker.clear(); + }); + + afterEach(async () => { + await rm(tmpDir, { recursive: true, force: true }); + }); + + async function until(predicate: () => boolean, timeoutMs = 4000): Promise { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (predicate()) return true; + await new Promise((r) => setTimeout(r, 40)); + } + return predicate(); + } + + test('edits to a pre-existing subfolder doc dispatch a DiskEvent', async () => { + const filter = createContentFilter({ projectDir: tmpDir, contentDir }); + const events: DiskEvent[] = []; + const handle = await startWatcher(contentDir, async (e) => void events.push(e), filter, { + forceBackend: 'chokidar', + }); + try { + // Sanity: a root-level edit is detected (this worked pre-fix too). + writeFileSync(resolve(contentDir, 'root.md'), '# Root edited\n'); + expect( + await until(() => events.some((e) => e.kind === 'update' && e.docName === 'root')), + ).toBe(true); + + // The regression: a subfolder edit must also dispatch. + writeFileSync(resolve(contentDir, 'sub', 'note.md'), '# Note edited\n\n[Gone](./gone)\n'); + expect( + await until(() => events.some((e) => e.kind === 'update' && e.docName === 'sub/note')), + ).toBe(true); + } finally { + await handle.unsubscribe(); + } + }); + + test('a doc created in a NEW subfolder after watch start dispatches a DiskEvent', async () => { + const filter = createContentFilter({ projectDir: tmpDir, contentDir }); + const events: DiskEvent[] = []; + const handle = await startWatcher(contentDir, async (e) => void events.push(e), filter, { + forceBackend: 'chokidar', + }); + try { + mkdirSync(resolve(contentDir, 'fresh')); + // The folder-create dispatch proves chokidar's addDir was processed, so + // the sub-watch is armed before we write into the directory. A fixed + // sleep would race a loaded runner and idle on a fast one. + expect( + await until(() => + events.some((e) => e.kind === 'folder-create' && e.relativePath === 'fresh'), + ), + ).toBe(true); + writeFileSync(resolve(contentDir, 'fresh', 'child.md'), '# Child\n'); + expect( + await until(() => + events.some( + (e) => (e.kind === 'create' || e.kind === 'update') && e.docName === 'fresh/child', + ), + ), + ).toBe(true); + } finally { + await handle.unsubscribe(); + } + }); + + test('excluded subtrees stay pruned — node_modules edits do not dispatch', async () => { + const filter = createContentFilter({ projectDir: tmpDir, contentDir }); + const events: DiskEvent[] = []; + const handle = await startWatcher(contentDir, async (e) => void events.push(e), filter, { + forceBackend: 'chokidar', + }); + try { + // Write into the excluded subtree, then edit a watched file as a + // sentinel. Waiting for the sentinel to dispatch proves the watcher was + // live through the window — so the absent node_modules event is a real + // negative, not a vacuous "nothing fired yet" (node_modules is never + // watched, so its write can't race ahead of the sentinel). + mkdirSync(resolve(contentDir, 'node_modules', 'dep'), { recursive: true }); + writeFileSync(resolve(contentDir, 'node_modules', 'dep', 'readme.md'), '# Dep\n'); + writeFileSync(resolve(contentDir, 'root.md'), '# Root sentinel\n'); + expect( + await until(() => events.some((e) => e.kind === 'update' && e.docName === 'root')), + ).toBe(true); + expect( + events.some((e) => 'docName' in e && String(e.docName).startsWith('node_modules')), + ).toBe(false); + } finally { + await handle.unsubscribe(); + } + }); +}); diff --git a/packages/server/src/file-watcher.ts b/packages/server/src/file-watcher.ts index 575c048c..cdb1b130 100644 --- a/packages/server/src/file-watcher.ts +++ b/packages/server/src/file-watcher.ts @@ -1779,6 +1779,49 @@ async function startParcelWatcher( // ─── Backend: chokidar ────────────────────────────────────────────────────── +/** + * chokidar `ignored` predicate — decides both whether to emit an event for a + * path AND (load-bearing) whether to descend into a directory. + * + * chokidar invokes this WITHOUT `stats` at the subwatch gate that decides + * whether a freshly-discovered directory gets its own watcher (readdirp entries + * carry lstat stats, but that gate does not). With no stats we cannot tell a + * directory from a file, and routing a real subdirectory through the file-only + * `isExcluded` — which default-excludes any non-`.md`/non-asset name — prunes + * the whole subtree, so the fallback silently stops watching every content + * subfolder. `lstatSync` (not `statSync`, to match `followSymlinks: false`) + * resolves the type; any stat error admits the path (return `false`) so a + * just-deleted file still emits its delete and a transiently-missing path is + * never pruned. + */ +export function isChokidarPathIgnored( + contentDir: string, + contentFilter: ContentFilter, + filePath: string, + stats?: Stats, +): boolean { + const rel = toPosix(relative(contentDir, filePath)); + if (rel === '' || rel === '.') return false; + let isDirectory: boolean; + if (stats) { + isDirectory = stats.isDirectory(); + } else { + try { + isDirectory = lstatSync(filePath).isDirectory(); + } catch { + return false; + } + } + return isDirectory ? contentFilter.isDirExcluded(rel) : contentFilter.isExcluded(rel); +} + +/** + * Upper bound on how long `startWatcher` waits for chokidar's `ready` event + * before returning anyway. `ready` normally fires in well under a second; the + * cap only guards against a pathological initial scan wedging server boot. + */ +const CHOKIDAR_READY_TIMEOUT_MS = 10_000; + async function startChokidarWatcher( contentDir: string, contentFilter: ContentFilter | undefined, @@ -1800,12 +1843,8 @@ async function startChokidarWatcher( // contentDir from sourcing events for an arbitrary location on disk. followSymlinks: false, ignored: contentFilter - ? (filePath: string, stats?: import('node:fs').Stats) => { - const rel = toPosix(relative(contentDir, filePath)); - if (rel === '' || rel === '.') return false; - if (stats?.isDirectory()) return contentFilter.isDirExcluded(rel); - return contentFilter.isExcluded(rel); - } + ? (filePath: string, stats?: Stats) => + isChokidarPathIgnored(contentDir, contentFilter, filePath, stats) : undefined, }); @@ -1845,6 +1884,24 @@ async function startChokidarWatcher( watcher.on('addDir', (path) => queueEvent('create', path)); watcher.on('unlinkDir', (path) => queueEvent('delete', path)); + // chokidar's `watch()` returns before its initial scan and per-directory + // watch registration finish; unlike @parcel/watcher's `subscribe()`, the + // returned watcher is not yet observing. Awaiting `ready` makes `startWatcher` + // resolve only once external edits are actually detectable — otherwise an + // edit landing in the setup window (routine on slower inotify hosts) is + // silently missed. Bounded so a pathological scan never wedges boot: past the + // cap we proceed anyway (the watcher keeps arming in the background). + await new Promise((resolveReady) => { + let settled = false; + const settle = () => { + if (settled) return; + settled = true; + resolveReady(); + }; + watcher.once('ready', settle); + setTimeout(settle, CHOKIDAR_READY_TIMEOUT_MS).unref(); + }); + return { unsubscribe: () => { if (batchTimer) { @@ -1876,6 +1933,7 @@ export async function startWatcher( contentDirRaw: string, onDiskEvent: (event: DiskEvent) => Promise, contentFilter?: ContentFilter, + opts: { forceBackend?: 'parcel' | 'chokidar' } = {}, ): Promise { let contentDir: string; try { @@ -1922,19 +1980,29 @@ export async function startWatcher( let subscription: AsyncSubscription; let backend: WatcherBackend; try { - const parcelSub = await startParcelWatcher( - contentDir, - contentFilter, - fileIndex, - folderIndex, - onDiskEvent, - aliasMap, - bumpFileIndexGeneration, - ); + // `forceBackend` is a test seam (mirrors head-watcher): 'chokidar' skips + // the parcel attempt so the fallback can be exercised on a host where the + // native module IS resolvable; 'parcel' throws instead of degrading so a + // test can't silently pass on the wrong backend. + const parcelSub = + opts.forceBackend === 'chokidar' + ? null + : await startParcelWatcher( + contentDir, + contentFilter, + fileIndex, + folderIndex, + onDiskEvent, + aliasMap, + bumpFileIndexGeneration, + ); if (parcelSub) { subscription = parcelSub; backend = 'parcel'; } else { + if (opts.forceBackend === 'parcel') { + throw new Error('@parcel/watcher unavailable (forced backend)'); + } subscription = await startChokidarWatcher( contentDir, contentFilter,