too many watch files

lib/watch.js

@@ -34,6 +34,42 @@ const filePathUnixified = filePath =>
     .replace(new RegExp(`^${dir.APP}/`), '')
     .replace(new RegExp(`^${dir.LEGACY_APP}/`), '');
 const moduleAssetRegex = new RegExp('^modules/\\w+/public/assets');
+
+// Directories that must never be watched. chokidar v4+ dropped fsevents, so on
+// macOS each watched directory costs one file descriptor (kqueue). Pruning these
+// keeps pos-cli well under the OS limit and avoids EMFILE on large projects.
+// None of these are ever synced, so ignoring them is safe.
+const WATCH_IGNORED_DIRS = new Set(['node_modules', '.git']);
+
+// chokidar v4+ removed glob support: a string matcher is compared literally, so
+// the old `ignored: ['**/.DS_Store']` silently stopped excluding anything. The
+// supported form is a function `(path, stats?) => boolean` testing the full path.
+const watchIgnored = filePath => {
+  const normalizedPath = filePath.replace(/\\/g, '/');
+  if (path.basename(normalizedPath) === '.DS_Store') return true;
+  return normalizedPath.split('/').some(segment => WATCH_IGNORED_DIRS.has(segment));
+};
+
+// Without this handler an EMFILE (too many open files) — emitted by chokidar as
+// an 'error' event — becomes an uncaught error and crashes pos-cli. Keep the
+// process alive and tell the user how to recover.
+const handleWatcherError = async error => {
+  const code = error && error.code;
+  if (code === 'EMFILE' || code === 'ENFILE' || code === 'ENOSPC') {
+    await logger.Error(
+      `[Sync] The OS file-watch limit was reached (${code}).\n` +
+        `pos-cli watches one file descriptor per directory, and very large projects can exceed the default limit.\n` +
+        `Raise it (e.g. "ulimit -n 10240" on macOS/Linux) or exclude large directories via .posignore, then restart sync.`,
+      { exit: false, notify: false }
+    );
+  } else {
+    await logger.Error(`[Sync] File watcher error: ${(error && error.message) || error}`, {
+      exit: false,
+      notify: false
+    });
+  }
+};
+
 let queue;
 let directUploadData;
 let manifestFilesToAdd = [];
@@ -239,10 +275,9 @@ const start = async (env, directAssetsUpload, liveReload) => {
         stabilityThreshold: 500,
         pollInterval: 100
       },
-      ignored: [
-        '**/.DS_Store'
-      ]
+      ignored: watchIgnored
     })
+    .on('error', handleWatcherError)
     .on('ready', () => logger.Info(`[Sync] Synchronizing changes to: ${program.url}`))
     .on('change', fp => shouldBeSynced(fp, ignoreList) && enqueuePush(fp))
     .on('add', fp => shouldBeSynced(fp, ignoreList) && enqueuePush(fp))
@@ -303,4 +338,4 @@ const sendFile = async (gateway, filePath) => {
   }
 };
 
-export { start, setupGracefulShutdown, sendFile, pushFile, deleteFile };
+export { start, setupGracefulShutdown, sendFile, pushFile, deleteFile, watchIgnored, handleWatcherError };

test/unit/watch.test.js

@@ -34,9 +34,21 @@ vi.mock('#lib/ServerError.js', () => ({
 }));
 
 // Stub modules used only inside start() / sendAsset(), not pushFile/deleteFile.
-vi.mock('chokidar', () => ({
-  default: { watch: vi.fn().mockReturnValue({ on: vi.fn().mockReturnThis() }) }
-}));
+// chokidar.watch() returns a real EventEmitter so the .on('error', ...) wiring in
+// start() is genuinely exercised — emitting 'error' on a real emitter with no
+// listener would throw (this is exactly how EMFILE crashed pos-cli before the fix).
+vi.mock('chokidar', async () => {
+  const { EventEmitter } = await import('node:events');
+  return {
+    default: {
+      watch: vi.fn(() => {
+        const watcher = new EventEmitter();
+        watcher.close = vi.fn().mockResolvedValue(undefined);
+        return watcher;
+      })
+    }
+  };
+});
 vi.mock('livereload', () => ({ default: { createServer: vi.fn() } }));
 vi.mock('async', () => ({ default: { queue: vi.fn() } }));
 vi.mock('#lib/proxy.js', () => ({ default: vi.fn() }));
@@ -58,7 +70,7 @@ import fs from 'fs';
 import logger from '#lib/logger.js';
 import ServerError from '#lib/ServerError.js';
 import Gateway from '#lib/proxy.js';
-import { pushFile, deleteFile, start } from '#lib/watch.js';
+import { pushFile, deleteFile, start, watchIgnored, handleWatcherError } from '#lib/watch.js';
 
 // --- test helpers ---------------------------------------------------------
 
@@ -323,4 +335,92 @@ describe('start', () => {
     expect(ServerError.handler).toHaveBeenCalledWith(networkErr);
     expect(exitSpy).toHaveBeenCalledWith(1);
   });
+
+  // Regression test for the EMFILE crash. chokidar v4+ dropped fsevents, so on
+  // macOS each watched directory consumes a file descriptor; large projects hit
+  // the OS limit and chokidar emits an 'error' event. Before the fix, start()
+  // registered no 'error' listener, so Node threw on the unhandled event and
+  // crashed pos-cli with a raw stack trace.
+  test('registers an error handler so an EMFILE watcher error does not crash the process', async () => {
+    const { watcher } = await start(env, false, false);
+
+    const emfile = Object.assign(new Error('EMFILE: too many open files, watch'), {
+      errno: -24,
+      syscall: 'watch',
+      code: 'EMFILE',
+      filename: null
+    });
+
+    // On a real EventEmitter, emitting 'error' with no listener throws synchronously.
+    // The fix wires .on('error', ...), so this must NOT throw and must report listeners.
+    let hadListeners;
+    expect(() => {
+      hadListeners = watcher.emit('error', emfile);
+    }).not.toThrow();
+    expect(hadListeners).toBe(true);
+
+    // Let the async handler run.
+    await Promise.resolve();
+
+    expect(logger.Error).toHaveBeenCalledWith(
+      expect.stringContaining('OS file-watch limit was reached (EMFILE)'),
+      { exit: false, notify: false }
+    );
+    // Must not exit the process — sync should stay alive.
+    expect(exitSpy).not.toHaveBeenCalled();
+  });
+});
+
+// --- watchIgnored tests ---------------------------------------------------
+
+describe('watchIgnored', () => {
+  test('ignores .DS_Store files anywhere in the tree', () => {
+    expect(watchIgnored('app/.DS_Store')).toBe(true);
+    expect(watchIgnored('modules/foo/public/.DS_Store')).toBe(true);
+  });
+
+  test('ignores node_modules and .git directories (huge, never synced — main EMFILE driver)', () => {
+    expect(watchIgnored('modules/foo/node_modules/dep/index.js')).toBe(true);
+    expect(watchIgnored('app/.git/config')).toBe(true);
+    expect(watchIgnored('node_modules')).toBe(true);
+  });
+
+  test('handles Windows-style backslash separators', () => {
+    expect(watchIgnored('modules\\foo\\node_modules\\dep\\index.js')).toBe(true);
+    expect(watchIgnored('app\\.DS_Store')).toBe(true);
+  });
+
+  test('does not ignore real source files', () => {
+    expect(watchIgnored('app/views/pages/index.liquid')).toBe(false);
+    expect(watchIgnored('modules/foo/public/views/partials/header.liquid')).toBe(false);
+    // A file whose name merely contains "node_modules" is not in such a directory.
+    expect(watchIgnored('app/views/node_modules_guide.liquid')).toBe(false);
+  });
+});
+
+// --- handleWatcherError tests ---------------------------------------------
+
+describe('handleWatcherError', () => {
+  beforeEach(() => vi.clearAllMocks());
+
+  test.each(['EMFILE', 'ENFILE', 'ENOSPC'])(
+    'logs an actionable, non-exiting message for resource-exhaustion error %s',
+    async code => {
+      await handleWatcherError(Object.assign(new Error('boom'), { code }));
+
+      expect(logger.Error).toHaveBeenCalledWith(
+        expect.stringContaining(`OS file-watch limit was reached (${code})`),
+        { exit: false, notify: false }
+      );
+    }
+  );
+
+  test('logs a generic non-exiting message for other watcher errors', async () => {
+    await handleWatcherError(Object.assign(new Error('weird failure'), { code: 'EOTHER' }));
+
+    expect(logger.Error).toHaveBeenCalledWith(
+      '[Sync] File watcher error: weird failure',
+      { exit: false, notify: false }
+    );
+  });
 });