From 35526c86301e5cc6543d8d379e6eba9b5fe99d2c Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Mon, 27 Jul 2026 23:19:33 -0700 Subject: [PATCH 1/5] fleet|fix: Probe the directory before announcing an active watch Fleet's startup line names the watch mode actually in effect on every platform. On Linux, a missing events directory left it announcing an active recursive watch that had never started, and no later message corrected it. The degraded-mode line names the directory that could not be watched and the underlying reason. --- packages/fleet/src/store/watcher.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/fleet/src/store/watcher.ts b/packages/fleet/src/store/watcher.ts index 2dc93b22..5e929036 100644 --- a/packages/fleet/src/store/watcher.ts +++ b/packages/fleet/src/store/watcher.ts @@ -2,7 +2,7 @@ // an always-on rescan interval as the correctness backstop. Watching is best-effort — recursive support varies by // platform, and a watch can fail mid-run — so degradation to rescan-only is announced, never silent, and never fatal. -import { type FSWatcher, watch } from 'node:fs'; +import { type FSWatcher, statSync, watch } from 'node:fs'; /** A running watcher; `stop` releases the watch handle and every timer. */ export interface Watcher { @@ -35,6 +35,9 @@ export function startWatcher(input: { } try { + // `fs.watch` reports a missing target inconsistently across platforms, returning an inert watcher on Linux, so its + // not throwing is no evidence that the watch is live. + statSync(input.dir); watcher = watch(input.dir, { recursive: true }, handleWatchEvent); watcher.on('error', (error) => { // A mid-run watch error — the tree removed, an OS watch limit — must not crash the server; the rescan @@ -45,7 +48,7 @@ export function startWatcher(input: { }); input.log(`recursive watch active on ${input.dir}; rescan backstop every ${input.rescanMs}ms`); } catch (error) { - input.log(`fs.watch unavailable (${readMessage(error)}); rescan-only every ${input.rescanMs}ms`); + input.log(`cannot watch ${input.dir} (${readMessage(error)}); rescan-only every ${input.rescanMs}ms`); } const rescanTimer = setInterval(() => input.onDirty(), input.rescanMs); From 6678beb8d3109150df393462b3adb4978f01d182 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Mon, 27 Jul 2026 23:21:22 -0700 Subject: [PATCH 2/5] fleet|refactor: Resolve the watch through an injectable seam The watcher's announcement and degradation behavior is exercisable without an OS-level watch. Tests supply their own watch starter, and the module falls back to Node's recursive `fs.watch` when none is given. Coverage now includes a watch that fails to start, and pins that a missing directory is detected before any watch is attempted. --- .../fleet/src/store/__tests__/watcher.test.ts | 64 ++++++++++++++++++- packages/fleet/src/store/watcher.ts | 24 +++++-- 2 files changed, 82 insertions(+), 6 deletions(-) diff --git a/packages/fleet/src/store/__tests__/watcher.test.ts b/packages/fleet/src/store/__tests__/watcher.test.ts index 5e4aeaf2..8c72077b 100644 --- a/packages/fleet/src/store/__tests__/watcher.test.ts +++ b/packages/fleet/src/store/__tests__/watcher.test.ts @@ -4,13 +4,53 @@ import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { startWatcher, type Watcher } from '../watcher.ts'; +import { startWatcher, type Watcher, type WatchStarter } from '../watcher.ts'; + +interface WatcherOverrides { + debounceMs?: number; + dir?: string; + rescanMs?: number; + startWatch?: WatchStarter; +} let dir: string; let watchers: Watcher[]; +/** A `WatchStarter` that records how it was driven, so tests exercise the watch contract without an OS watch. */ +function createFakeWatch(): { + emitError: (error: Error) => void; + fireEvent: () => void; + isClosed: () => boolean; + isStarted: () => boolean; + startWatch: WatchStarter; +} { + let closed = false; + let onError: ((error: Error) => void) | undefined; + let onEvent: (() => void) | undefined; + let started = false; + + return { + emitError: (error) => onError?.(error), + fireEvent: () => onEvent?.(), + isClosed: () => closed, + isStarted: () => started, + startWatch: (_dir, event) => { + started = true; + onEvent = event; + return { + close: () => { + closed = true; + }, + on: (_event, listener) => { + onError = listener; + }, + }; + }, + }; +} + /** Starts a watcher on a short rescan interval, registering it for cleanup. */ -function startTestWatcher(overrides: { dir?: string } = {}): { +function startTestWatcher(overrides: WatcherOverrides = {}): { log: ReturnType; onDirty: ReturnType; } { @@ -56,6 +96,26 @@ describe('startWatcher', () => { }); }); + it('when the directory does not exist, does not attempt a watch', () => { + const fake = createFakeWatch(); + + startTestWatcher({ dir: join(dir, 'missing'), startWatch: fake.startWatch }); + + expect(fake.isStarted()).toBe(false); + }); + + it('when the watch fails to start, names the target and the reason', () => { + const { log } = startTestWatcher({ + startWatch: () => { + throw new Error('EPERM: operation not permitted'); + }, + }); + + expect(log).toHaveBeenCalledExactlyOnceWith( + expect.stringContaining(`cannot watch ${dir} (EPERM: operation not permitted); rescan-only`), + ); + }); + it('stops firing after stop()', async () => { const { onDirty } = startTestWatcher(); await vi.waitFor(() => { diff --git a/packages/fleet/src/store/watcher.ts b/packages/fleet/src/store/watcher.ts index 5e929036..bddbf2c5 100644 --- a/packages/fleet/src/store/watcher.ts +++ b/packages/fleet/src/store/watcher.ts @@ -2,17 +2,26 @@ // an always-on rescan interval as the correctness backstop. Watching is best-effort — recursive support varies by // platform, and a watch can fail mid-run — so degradation to rescan-only is announced, never silent, and never fatal. -import { type FSWatcher, statSync, watch } from 'node:fs'; +import { statSync, watch } from 'node:fs'; /** A running watcher; `stop` releases the watch handle and every timer. */ export interface Watcher { stop(): void; } +/** The subset of a watch handle this module uses: stopping it, and learning that it failed. */ +export interface WatchHandle { + close(): void; + on(event: 'error', listener: (error: Error) => void): unknown; +} + +/** Starts a recursive watch on `dir`, invoking `onEvent` for every filesystem event. */ +export type WatchStarter = (dir: string, onEvent: () => void) => WatchHandle; + /** * Starts watching `dir`, invoking `onDirty` on debounced watch events and on every rescan tick. `log` receives one * startup line naming the active mode — recursive watch or rescan-only, with the reason — and a line on any later - * downgrade. + * downgrade. `startWatch` is injectable for tests and defaults to {@link startRecursiveWatch}. */ export function startWatcher(input: { debounceMs: number; @@ -20,9 +29,11 @@ export function startWatcher(input: { log: (message: string) => void; onDirty: () => void; rescanMs: number; + startWatch?: WatchStarter; }): Watcher { + const startWatch = input.startWatch ?? startRecursiveWatch; let debounce: ReturnType | undefined; - let watcher: FSWatcher | undefined; + let watcher: WatchHandle | undefined; function handleWatchEvent(): void { if (debounce !== undefined) { @@ -38,7 +49,7 @@ export function startWatcher(input: { // `fs.watch` reports a missing target inconsistently across platforms, returning an inert watcher on Linux, so its // not throwing is no evidence that the watch is live. statSync(input.dir); - watcher = watch(input.dir, { recursive: true }, handleWatchEvent); + watcher = startWatch(input.dir, handleWatchEvent); watcher.on('error', (error) => { // A mid-run watch error — the tree removed, an OS watch limit — must not crash the server; the rescan // interval keeps state current. @@ -71,4 +82,9 @@ function readMessage(error: unknown): string { return error instanceof Error ? error.message : String(error); } +/** Starts Node's recursive watch on `dir`. */ +function startRecursiveWatch(dir: string, onEvent: () => void): WatchHandle { + return watch(dir, { recursive: true }, onEvent); +} + // endregion | Helpers From e171ca54f12c774fa8cca3ce2eadbbfcd265f662 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Mon, 27 Jul 2026 23:22:07 -0700 Subject: [PATCH 3/5] fleet|tests: Cover the debounce and watch-error paths The watcher suite covers a burst of watch events collapsing into a single dirty signal, and an error from the watch downgrading to rescan-only and releasing the handle. Only the test asserting the default watch mode starts a real OS-level watch; the rest drive the module through an injected starter. --- .../fleet/src/store/__tests__/watcher.test.ts | 29 +++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/packages/fleet/src/store/__tests__/watcher.test.ts b/packages/fleet/src/store/__tests__/watcher.test.ts index 8c72077b..5c6825fd 100644 --- a/packages/fleet/src/store/__tests__/watcher.test.ts +++ b/packages/fleet/src/store/__tests__/watcher.test.ts @@ -80,13 +80,38 @@ describe('startWatcher', () => { }); it('fires onDirty on every rescan tick', async () => { - const { onDirty } = startTestWatcher(); + const { onDirty } = startTestWatcher({ startWatch: createFakeWatch().startWatch }); await vi.waitFor(() => { expect(onDirty.mock.calls.length).toBeGreaterThanOrEqual(2); }); }); + it('collapses a burst of watch events into one onDirty', async () => { + const fake = createFakeWatch(); + // The rescan interval fires the same callback, so it is pushed past this test's lifetime to leave the count + // attributable to the burst alone. + const { onDirty } = startTestWatcher({ rescanMs: 60_000, startWatch: fake.startWatch }); + + fake.fireEvent(); + fake.fireEvent(); + fake.fireEvent(); + await new Promise((resolve) => setTimeout(resolve, 30)); + + expect(onDirty).toHaveBeenCalledOnce(); + }); + + it('when the watch emits an error, announces the downgrade and closes the handle', () => { + const fake = createFakeWatch(); + const { log } = startTestWatcher({ startWatch: fake.startWatch }); + log.mockClear(); + + fake.emitError(new Error('EMFILE: too many open files')); + + expect(log).toHaveBeenCalledExactlyOnceWith(expect.stringContaining('downgrading to rescan-only')); + expect(fake.isClosed()).toBe(true); + }); + it('when the directory does not exist, announces rescan-only mode and still rescans', async () => { const { log, onDirty } = startTestWatcher({ dir: join(dir, 'missing') }); @@ -117,7 +142,7 @@ describe('startWatcher', () => { }); it('stops firing after stop()', async () => { - const { onDirty } = startTestWatcher(); + const { onDirty } = startTestWatcher({ startWatch: createFakeWatch().startWatch }); await vi.waitFor(() => { expect(onDirty).toHaveBeenCalled(); }); From 4cc7fa3dfbc799054db9ab7d72dfffaed00981e3 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Mon, 27 Jul 2026 23:43:27 -0700 Subject: [PATCH 4/5] fleet|tests: Scope test resources to the test that creates them The watcher suite's temp directory and running watchers are created and released per test, by helpers that register their own teardown. Module-level mutable state, `beforeEach`, and `afterEach` leave the file, and with them the three lint warnings it carried. --- .../fleet/src/store/__tests__/watcher.test.ts | 134 +++++++++--------- 1 file changed, 67 insertions(+), 67 deletions(-) diff --git a/packages/fleet/src/store/__tests__/watcher.test.ts b/packages/fleet/src/store/__tests__/watcher.test.ts index 5c6825fd..12811014 100644 --- a/packages/fleet/src/store/__tests__/watcher.test.ts +++ b/packages/fleet/src/store/__tests__/watcher.test.ts @@ -2,9 +2,9 @@ import { mkdtempSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { describe, expect, it, onTestFinished, vi } from 'vitest'; -import { startWatcher, type Watcher, type WatchStarter } from '../watcher.ts'; +import { startWatcher, type WatchStarter } from '../watcher.ts'; interface WatcherOverrides { debounceMs?: number; @@ -13,65 +13,6 @@ interface WatcherOverrides { startWatch?: WatchStarter; } -let dir: string; -let watchers: Watcher[]; - -/** A `WatchStarter` that records how it was driven, so tests exercise the watch contract without an OS watch. */ -function createFakeWatch(): { - emitError: (error: Error) => void; - fireEvent: () => void; - isClosed: () => boolean; - isStarted: () => boolean; - startWatch: WatchStarter; -} { - let closed = false; - let onError: ((error: Error) => void) | undefined; - let onEvent: (() => void) | undefined; - let started = false; - - return { - emitError: (error) => onError?.(error), - fireEvent: () => onEvent?.(), - isClosed: () => closed, - isStarted: () => started, - startWatch: (_dir, event) => { - started = true; - onEvent = event; - return { - close: () => { - closed = true; - }, - on: (_event, listener) => { - onError = listener; - }, - }; - }, - }; -} - -/** Starts a watcher on a short rescan interval, registering it for cleanup. */ -function startTestWatcher(overrides: WatcherOverrides = {}): { - log: ReturnType; - onDirty: ReturnType; -} { - const log = vi.fn(); - const onDirty = vi.fn(); - watchers.push(startWatcher({ debounceMs: 5, dir, log, onDirty, rescanMs: 10, ...overrides })); - return { log, onDirty }; -} - -beforeEach(() => { - dir = mkdtempSync(join(tmpdir(), 'watcher-')); - watchers = []; -}); - -afterEach(() => { - for (const watcher of watchers) { - watcher.stop(); - } - rmSync(dir, { recursive: true, force: true }); -}); - describe('startWatcher', () => { it('logs the recursive-watch mode when the directory is watchable', () => { const { log } = startTestWatcher(); @@ -113,7 +54,7 @@ describe('startWatcher', () => { }); it('when the directory does not exist, announces rescan-only mode and still rescans', async () => { - const { log, onDirty } = startTestWatcher({ dir: join(dir, 'missing') }); + const { log, onDirty } = startTestWatcher({ dir: join(createTempDir(), 'missing') }); expect(log).toHaveBeenCalledExactlyOnceWith(expect.stringContaining('rescan-only')); await vi.waitFor(() => { @@ -124,13 +65,16 @@ describe('startWatcher', () => { it('when the directory does not exist, does not attempt a watch', () => { const fake = createFakeWatch(); - startTestWatcher({ dir: join(dir, 'missing'), startWatch: fake.startWatch }); + startTestWatcher({ dir: join(createTempDir(), 'missing'), startWatch: fake.startWatch }); expect(fake.isStarted()).toBe(false); }); it('when the watch fails to start, names the target and the reason', () => { + const dir = createTempDir(); + const { log } = startTestWatcher({ + dir, startWatch: () => { throw new Error('EPERM: operation not permitted'); }, @@ -142,17 +86,73 @@ describe('startWatcher', () => { }); it('stops firing after stop()', async () => { - const { onDirty } = startTestWatcher({ startWatch: createFakeWatch().startWatch }); + const { onDirty, stop } = startTestWatcher({ startWatch: createFakeWatch().startWatch }); await vi.waitFor(() => { expect(onDirty).toHaveBeenCalled(); }); - for (const watcher of watchers.splice(0)) { - watcher.stop(); - } + stop(); onDirty.mockClear(); await new Promise((resolve) => setTimeout(resolve, 50)); expect(onDirty).not.toHaveBeenCalled(); }); }); + +// region | Helpers + +/** A `WatchStarter` that records how it was driven, so tests exercise the watch contract without an OS watch. */ +function createFakeWatch(): { + emitError: (error: Error) => void; + fireEvent: () => void; + isClosed: () => boolean; + isStarted: () => boolean; + startWatch: WatchStarter; +} { + let closed = false; + let onError: ((error: Error) => void) | undefined; + let onEvent: (() => void) | undefined; + let started = false; + + return { + emitError: (error) => onError?.(error), + fireEvent: () => onEvent?.(), + isClosed: () => closed, + isStarted: () => started, + startWatch: (_dir, event) => { + started = true; + onEvent = event; + return { + close: () => { + closed = true; + }, + on: (_event, listener) => { + onError = listener; + }, + }; + }, + }; +} + +/** Creates a temp directory for the current test, removed when the test finishes. */ +function createTempDir(): string { + const dir = mkdtempSync(join(tmpdir(), 'watcher-')); + onTestFinished(() => rmSync(dir, { recursive: true, force: true })); + return dir; +} + +/** Starts a watcher on a short rescan interval, stopped when the current test finishes. */ +function startTestWatcher(overrides: WatcherOverrides = {}): { + log: ReturnType; + onDirty: ReturnType; + stop: () => void; +} { + const dir = overrides.dir ?? createTempDir(); + const log = vi.fn(); + const onDirty = vi.fn(); + const watcher = startWatcher({ debounceMs: 5, dir, log, onDirty, rescanMs: 10, ...overrides }); + onTestFinished(() => watcher.stop()); + return { log, onDirty, stop: () => watcher.stop() }; +} + +// endregion | Helpers From 3865565b7ebd7bde320f0cace89a30f1f0c795c0 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Tue, 28 Jul 2026 00:03:50 -0700 Subject: [PATCH 5/5] fleet|tests: Poll for the debounced call instead of sleeping past it The burst test waits for the dirty signal itself rather than for a fixed interval long enough to contain it, so a garbage-collection pause or a loaded runner cannot fail it with zero calls. --- packages/fleet/src/store/__tests__/watcher.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/fleet/src/store/__tests__/watcher.test.ts b/packages/fleet/src/store/__tests__/watcher.test.ts index 12811014..feebbd06 100644 --- a/packages/fleet/src/store/__tests__/watcher.test.ts +++ b/packages/fleet/src/store/__tests__/watcher.test.ts @@ -37,7 +37,7 @@ describe('startWatcher', () => { fake.fireEvent(); fake.fireEvent(); fake.fireEvent(); - await new Promise((resolve) => setTimeout(resolve, 30)); + await vi.waitFor(() => expect(onDirty).toHaveBeenCalled()); expect(onDirty).toHaveBeenCalledOnce(); });