diff --git a/src/gateway/config-reload.test.ts b/src/gateway/config-reload.test.ts index 5cc7dafa9582..77a4ee11ada5 100644 --- a/src/gateway/config-reload.test.ts +++ b/src/gateway/config-reload.test.ts @@ -1590,6 +1590,93 @@ describe("startGatewayConfigReloader", () => { }); }); +describe("startGatewayConfigReloader watcher error recovery", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + function startReloaderWithWatchers(watchers: ReturnType[]) { + const watchSpy = vi.spyOn(chokidar, "watch"); + for (const watcher of watchers) { + watchSpy.mockReturnValueOnce(watcher as unknown as never); + } + const log = { info: vi.fn(), warn: vi.fn(), error: vi.fn() }; + const reloader = startGatewayConfigReloader({ + initialConfig: { gateway: { reload: { debounceMs: 0 } } }, + readSnapshot: vi.fn(async () => makeSnapshot()), + initialPluginInstallRecords: {}, + readPluginInstallRecords: async () => ({}), + onHotReload: vi.fn(async () => {}), + onRestart: vi.fn(), + log, + watchPath: "/tmp/openclaw.json", + }); + return { watchSpy, log, reloader }; + } + + it("re-creates the watcher with backoff after a transient error", async () => { + const first = createWatcherMock(); + const second = createWatcherMock(); + const { watchSpy, log, reloader } = startReloaderWithWatchers([first, second]); + + expect(watchSpy).toHaveBeenCalledTimes(1); + + first.emit("error"); + expect(reloader.hotReloadStatus()).toBe("active"); + expect(log.warn).toHaveBeenCalledWith( + expect.stringContaining("re-creating watcher (attempt 1/3 in 500ms)"), + ); + expect(first.close).toHaveBeenCalledTimes(1); + + // Watcher is only re-created once the backoff timer fires. + expect(watchSpy).toHaveBeenCalledTimes(1); + await vi.advanceTimersByTimeAsync(500); + expect(watchSpy).toHaveBeenCalledTimes(2); + expect(reloader.hotReloadStatus()).toBe("active"); + expect(log.error).not.toHaveBeenCalled(); + + await reloader.stop(); + }); + + it("disables hot-reload and logs at error level after the retry budget is exhausted", async () => { + const watchers = [ + createWatcherMock(), + createWatcherMock(), + createWatcherMock(), + createWatcherMock(), + ]; + const { watchSpy, log, reloader } = startReloaderWithWatchers(watchers); + + // Three errors consume the retry budget; the fourth error escalates. + watchers[0]?.emit("error"); + await vi.advanceTimersByTimeAsync(500); + watchers[1]?.emit("error"); + await vi.advanceTimersByTimeAsync(2000); + watchers[2]?.emit("error"); + await vi.advanceTimersByTimeAsync(5000); + expect(watchSpy).toHaveBeenCalledTimes(4); + expect(reloader.hotReloadStatus()).toBe("active"); + + watchers[3]?.emit("error"); + expect(reloader.hotReloadStatus()).toBe("disabled"); + expect(log.error).toHaveBeenCalledWith( + expect.stringContaining( + "config hot-reload disabled: watcher failed after 3 re-create attempts", + ), + ); + // No further watcher is created once disabled. + await vi.advanceTimersByTimeAsync(10000); + expect(watchSpy).toHaveBeenCalledTimes(4); + + await reloader.stop(); + }); +}); + describe("shouldInvalidateSkillsSnapshotForPaths", () => { it.each([ "skills", diff --git a/src/gateway/config-reload.ts b/src/gateway/config-reload.ts index 26d235166eeb..5ecaaabcd5bc 100644 --- a/src/gateway/config-reload.ts +++ b/src/gateway/config-reload.ts @@ -33,6 +33,13 @@ export type { ChannelKind, GatewayReloadPlan } from "./config-reload-plan.js"; const MISSING_CONFIG_RETRY_DELAY_MS = 150; const MISSING_CONFIG_MAX_RETRIES = 2; +// Watcher 'error' events (for example EMFILE/ENOSPC inotify exhaustion) close +// the chokidar watcher. Re-create it with bounded backoff so a transient fault +// does not permanently kill config hot-reload, but escalate to error + a +// persistent disabled status once the retry budget is exhausted. +const WATCHER_RECREATE_MAX_RETRIES = 3; +const WATCHER_RECREATE_BACKOFF_MS = [500, 2000, 5000] as const; + /** * Paths under `skills.*` always change the snapshot that sessions cache in * sessions.json. Any prefix match here (for example `skills.allowBundled`, @@ -71,8 +78,14 @@ function isNoopReloadPlan(plan: GatewayReloadPlan): boolean { ); } +// Hot-reload stays "active" while a watcher is live. It flips to "disabled" only +// after watcher re-creation fails past the retry budget, so operators/callers +// can detect silent degradation instead of assuming reloads still fire. +export type GatewayHotReloadStatus = "active" | "disabled"; + type GatewayConfigReloader = { stop: () => Promise; + hotReloadStatus: () => GatewayHotReloadStatus; }; type PluginInstallRecords = Record; @@ -371,12 +384,6 @@ export function startGatewayConfigReloader(opts: { } }; - const watcher = chokidar.watch(opts.watchPath, { - ignoreInitial: true, - awaitWriteFinish: { stabilityThreshold: 200, pollInterval: 50 }, - usePolling: Boolean(process.env.VITEST), - }); - const scheduleFromWatcher = () => { schedule(); }; @@ -396,18 +403,59 @@ export function startGatewayConfigReloader(opts: { scheduleAfter(0); }) ?? (() => {}); - watcher.on("add", scheduleFromWatcher); - watcher.on("change", scheduleFromWatcher); - watcher.on("unlink", scheduleFromWatcher); - let watcherClosed = false; - watcher.on("error", (err) => { - if (watcherClosed) { + let watcher: ReturnType | null = null; + let watcherRecreateRetries = 0; + let watcherRecreateTimer: ReturnType | null = null; + let hotReloadStatus: GatewayHotReloadStatus = "active"; + + const createWatcher = () => { + if (stopped) { return; } - watcherClosed = true; - opts.log.warn(`config watcher error: ${String(err)}`); - void watcher.close().catch(() => {}); - }); + const next = chokidar.watch(opts.watchPath, { + ignoreInitial: true, + awaitWriteFinish: { stabilityThreshold: 200, pollInterval: 50 }, + usePolling: Boolean(process.env.VITEST), + }); + next.on("add", scheduleFromWatcher); + next.on("change", scheduleFromWatcher); + next.on("unlink", scheduleFromWatcher); + next.on("error", (err) => { + handleWatcherError(next, err); + }); + watcher = next; + hotReloadStatus = "active"; + }; + + const handleWatcherError = (source: typeof watcher, err: unknown) => { + // Ignore stale errors from a watcher we already replaced or stopped. + if (stopped || source !== watcher) { + return; + } + watcher = null; + void source?.close().catch(() => {}); + if (watcherRecreateRetries >= WATCHER_RECREATE_MAX_RETRIES) { + hotReloadStatus = "disabled"; + opts.log.error( + `config hot-reload disabled: watcher failed after ${WATCHER_RECREATE_MAX_RETRIES} re-create attempts: ${String(err)}`, + ); + return; + } + const backoff = + WATCHER_RECREATE_BACKOFF_MS[watcherRecreateRetries] ?? + WATCHER_RECREATE_BACKOFF_MS[WATCHER_RECREATE_BACKOFF_MS.length - 1] ?? + 0; + watcherRecreateRetries += 1; + opts.log.warn( + `config watcher error; re-creating watcher (attempt ${watcherRecreateRetries}/${WATCHER_RECREATE_MAX_RETRIES} in ${backoff}ms): ${String(err)}`, + ); + watcherRecreateTimer = setTimeout(() => { + watcherRecreateTimer = null; + createWatcher(); + }, backoff); + }; + + createWatcher(); return { stop: async () => { @@ -416,9 +464,15 @@ export function startGatewayConfigReloader(opts: { clearTimeout(debounceTimer); } debounceTimer = null; - watcherClosed = true; + if (watcherRecreateTimer) { + clearTimeout(watcherRecreateTimer); + watcherRecreateTimer = null; + } unsubscribeFromWrites(); - await watcher.close().catch(() => {}); + const active = watcher; + watcher = null; + await active?.close().catch(() => {}); }, + hotReloadStatus: () => hotReloadStatus, }; }