fix(state-watch): watch canonical ini files atomically

api/src/__test__/store/watch/state-watch.integration.test.ts

@@ -1,4 +1,4 @@
-import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises';
+import { mkdir, mkdtemp, rename, rm, writeFile } from 'node:fs/promises';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
 
@@ -64,6 +64,8 @@ describe('StateManager integration', () => {
         tempRoot = await mkdtemp(join(tmpdir(), 'state-watch-'));
         statesDirectory = join(tempRoot, 'state');
         await mkdir(statesDirectory);
+        await writeFile(join(statesDirectory, 'var.ini'), 'NAME="Before"\n');
+        await writeFile(join(statesDirectory, 'disks.ini'), '[disk1]\nname=disk1\n');
         testContext.states = statesDirectory;
 
         const { StateManager } = await import('@app/store/watch/state-watch.js');
@@ -84,20 +86,21 @@ describe('StateManager integration', () => {
         await rm(tempRoot, { recursive: true, force: true });
     });
 
-    it('reloads var state when emhttp writes var.ini.new into the state directory', async () => {
+    it('reloads var state when emhttp atomically replaces var.ini', async () => {
         const { store } = await import('@app/store/index.js');
         const { loadSingleStateFile } = await import('@app/store/modules/emhttp.js');
         const { loadRegistrationKey } = await import('@app/store/modules/registration.js');
 
         await writeFile(join(statesDirectory, 'var.ini.new'), 'NAME="Gamer5"\n');
+        await rename(join(statesDirectory, 'var.ini.new'), join(statesDirectory, 'var.ini'));
 
         await vi.waitFor(() => {
             expect(store.dispatch).toHaveBeenNthCalledWith(1, loadSingleStateFile(StateFileKey.var));
             expect(store.dispatch).toHaveBeenNthCalledWith(2, loadRegistrationKey());
         });
     });
 
-    it('does not route disks.ini.new through the directory watcher', async () => {
+    it('does not react to disks.ini.new before the canonical file is replaced', async () => {
         const { store } = await import('@app/store/index.js');
 
         await writeFile(join(statesDirectory, 'disks.ini.new'), '[disk1]\nname=disk1\n');

api/src/__test__/store/watch/state-watch.test.ts

@@ -73,34 +73,45 @@ describe('StateManager', () => {
         StateManager.instance = null;
     });
 
-    it('watches the emhttp state directory and keeps polling scoped to replacement-prone files', async () => {
+    it('watches each canonical emhttp state file and keeps polling scoped to replacement-prone files', async () => {
         const { StateManager } = await import('@app/store/watch/state-watch.js');
 
         await StateManager.getInstance().ready;
 
-        expect(chokidarWatch).toHaveBeenCalledTimes(3);
+        expect(chokidarWatch).toHaveBeenCalledTimes(Object.values(StateFileKey).length);
         expect(chokidarWatch).toHaveBeenNthCalledWith(
             1,
-            '/usr/local/emhttp/state',
+            '/usr/local/emhttp/state/var.ini',
             expect.objectContaining({
+                atomic: 200,
                 ignoreInitial: true,
                 usePolling: false,
-                ignored: expect.any(Function),
             })
         );
         expect(chokidarWatch).toHaveBeenNthCalledWith(
             2,
-            '/usr/local/emhttp/state/disks.ini',
+            '/usr/local/emhttp/state/devs.ini',
+            expect.objectContaining({
+                atomic: 200,
+                ignoreInitial: true,
+                usePolling: false,
+            })
+        );
+        expect(chokidarWatch).toHaveBeenNthCalledWith(
+            5,
+            '/usr/local/emhttp/state/shares.ini',
             expect.objectContaining({
+                atomic: 200,
                 ignoreInitial: true,
                 usePolling: true,
                 interval: 10_000,
             })
         );
         expect(chokidarWatch).toHaveBeenNthCalledWith(
-            3,
-            '/usr/local/emhttp/state/shares.ini',
+            6,
+            '/usr/local/emhttp/state/disks.ini',
             expect.objectContaining({
+                atomic: 200,
                 ignoreInitial: true,
                 usePolling: true,
                 interval: 10_000,
@@ -127,7 +138,7 @@ describe('StateManager', () => {
         expect(dispatchedStateLoads).toEqual(Object.values(StateFileKey));
     });
 
-    it('routes non-polled state files through the standard directory watcher', async () => {
+    it('routes non-polled state files through their canonical file watchers', async () => {
         const { StateManager } = await import('@app/store/watch/state-watch.js');
         const { store } = await import('@app/store/index.js');
         const { loadSingleStateFile } = await import('@app/store/modules/emhttp.js');
@@ -136,7 +147,7 @@ describe('StateManager', () => {
         vi.mocked(store.dispatch).mockClear();
 
         const standardWatcher = watchRegistrations.find(
-            (registration) => registration.options.usePolling === false
+            (registration) => registration.path === '/usr/local/emhttp/state/devs.ini'
         );
         const changeHandler = standardWatcher?.handlers.change;
         expect(changeHandler).toBeDefined();
@@ -146,29 +157,6 @@ describe('StateManager', () => {
         expect(store.dispatch).toHaveBeenCalledWith(loadSingleStateFile(StateFileKey.devs));
     });
 
-    it('ignores non-state files while still allowing non-polled state files through the directory watcher', async () => {
-        const { StateManager } = await import('@app/store/watch/state-watch.js');
-
-        await StateManager.getInstance().ready;
-
-        const standardWatcher = watchRegistrations.find(
-            (registration) => registration.options.usePolling === false
-        );
-        const ignored = standardWatcher?.options.ignored;
-
-        expect(ignored).toBeTypeOf('function');
-        expect((ignored as (path: string) => boolean)('/usr/local/emhttp/state')).toBe(false);
-        expect((ignored as (path: string) => boolean)('/usr/local/emhttp/state/README.txt')).toBe(true);
-        expect((ignored as (path: string) => boolean)('/usr/local/emhttp/state/devs.ini')).toBe(false);
-        expect((ignored as (path: string) => boolean)('/usr/local/emhttp/state/var.ini.new')).toBe(
-            false
-        );
-        expect((ignored as (path: string) => boolean)('/usr/local/emhttp/state/disks.ini')).toBe(true);
-        expect((ignored as (path: string) => boolean)('/usr/local/emhttp/state/disks.ini.new')).toBe(
-            true
-        );
-    });
-
     it('reloads registration key when var.ini is replaced after boot', async () => {
         const { StateManager } = await import('@app/store/watch/state-watch.js');
         const { store } = await import('@app/store/index.js');
@@ -178,40 +166,19 @@ describe('StateManager', () => {
         await StateManager.getInstance().ready;
         vi.mocked(store.dispatch).mockClear();
 
-        const standardWatcher = watchRegistrations.find(
-            (registration) => registration.options.usePolling === false
+        const watcher = watchRegistrations.find(
+            (registration) => registration.path === '/usr/local/emhttp/state/var.ini'
         );
-        const addHandler = standardWatcher?.handlers.add;
-        expect(addHandler).toBeDefined();
-
-        await addHandler?.('/usr/local/emhttp/state/var.ini');
-
-        expect(store.dispatch).toHaveBeenNthCalledWith(1, loadSingleStateFile(StateFileKey.var));
-        expect(store.dispatch).toHaveBeenNthCalledWith(2, loadRegistrationKey());
-    });
-
-    it('reloads registration key when var.ini.new is observed after boot', async () => {
-        const { StateManager } = await import('@app/store/watch/state-watch.js');
-        const { store } = await import('@app/store/index.js');
-        const { loadSingleStateFile } = await import('@app/store/modules/emhttp.js');
-        const { loadRegistrationKey } = await import('@app/store/modules/registration.js');
-
-        await StateManager.getInstance().ready;
-        vi.mocked(store.dispatch).mockClear();
-
-        const standardWatcher = watchRegistrations.find(
-            (registration) => registration.options.usePolling === false
-        );
-        const addHandler = standardWatcher?.handlers.add;
-        expect(addHandler).toBeDefined();
+        const changeHandler = watcher?.handlers.change;
+        expect(changeHandler).toBeDefined();
 
-        await addHandler?.('/usr/local/emhttp/state/var.ini.new');
+        await changeHandler?.('/usr/local/emhttp/state/var.ini');
 
         expect(store.dispatch).toHaveBeenNthCalledWith(1, loadSingleStateFile(StateFileKey.var));
         expect(store.dispatch).toHaveBeenNthCalledWith(2, loadRegistrationKey());
     });
 
-    it('routes polled state files through the polling directory watcher', async () => {
+    it('routes polled state files through their canonical file watchers', async () => {
         const { StateManager } = await import('@app/store/watch/state-watch.js');
         const { store } = await import('@app/store/index.js');
         const { loadSingleStateFile } = await import('@app/store/modules/emhttp.js');

api/src/store/watch/state-watch.ts

@@ -12,45 +12,19 @@ import { StateFileKey } from '@app/store/types.js';
 
 const POLLED_STATE_KEYS = [StateFileKey.disks, StateFileKey.shares] as const;
 const POLLED_STATE_KEY_SET = new Set<StateFileKey>(POLLED_STATE_KEYS);
-const STATE_FILE_NAMES = new Set(Object.values(StateFileKey).map((key) => `${key}.ini`));
+const ATOMIC_REPLACEMENT_WINDOW_MS = 200;
 
-const getNormalizedStateFileName = (path: string): string | null => {
-    const parsed = parse(path);
-    if (STATE_FILE_NAMES.has(parsed.base)) {
-        return parsed.base;
-    }
-
-    if (!parsed.base.endsWith('.new')) {
-        return null;
-    }
-
-    const originalStateFileName = parsed.base.slice(0, -'.new'.length);
-    return STATE_FILE_NAMES.has(originalStateFileName) ? originalStateFileName : null;
-};
-
-const shouldIgnoreStatePath = (path: string): boolean => {
-    const normalizedStateFileName = getNormalizedStateFileName(path);
-
-    if (!normalizedStateFileName) {
-        return true;
-    }
-
-    const stateFileKey = StateFileKey[normalizedStateFileName.slice(0, -'.ini'.length)];
-    return POLLED_STATE_KEY_SET.has(stateFileKey);
-};
-
-const chokidarOptionsForStateDirectory = (statesPath: string): ChokidarOptions => ({
+const chokidarOptionsForStateKey = (key: StateFileKey): ChokidarOptions => ({
+    atomic: ATOMIC_REPLACEMENT_WINDOW_MS,
     ignoreInitial: true,
-    ignored: (path, stats) => {
-        if (path === statesPath) {
-            return false;
-        }
-        if (stats?.isDirectory()) {
-            return false;
-        }
-        return shouldIgnoreStatePath(path);
-    },
-    usePolling: CHOKIDAR_USEPOLLING,
+    ...(POLLED_STATE_KEY_SET.has(key)
+        ? {
+              usePolling: true,
+              interval: 10_000,
+          }
+        : {
+              usePolling: CHOKIDAR_USEPOLLING,
+          }),
 });
 
 export class StateManager {
@@ -80,12 +54,8 @@ export class StateManager {
     }
 
     private getStateFileKeyFromPath(path: string): StateFileKey | undefined {
-        const normalizedStateFileName = getNormalizedStateFileName(path);
-        if (!normalizedStateFileName) {
-            return undefined;
-        }
-
-        return StateFileKey[normalizedStateFileName.slice(0, -'.ini'.length)];
+        const parsed = parse(path);
+        return StateFileKey[parsed.name];
     }
 
     private async reloadStateFile(stateFile: StateFileKey, reason: 'add' | 'change' | 'startup-sync') {
@@ -124,20 +94,10 @@ export class StateManager {
     private readonly setupChokidarWatchForState = async () => {
         const { states } = getters.paths();
 
-        emhttpLogger.debug('Setting up watch for path: %s', states);
-        const directoryWatch = watch(states, chokidarOptionsForStateDirectory(states));
-        directoryWatch.on('add', async (path) => this.handleStateFileUpdate(path, 'add'));
-        directoryWatch.on('change', async (path) => this.handleStateFileUpdate(path, 'change'));
-        this.fileWatchers.push(directoryWatch);
-
-        for (const key of POLLED_STATE_KEYS) {
+        for (const key of Object.values(StateFileKey)) {
             const pathToWatch = join(states, `${key}.ini`);
             emhttpLogger.debug('Setting up watch for path: %s', pathToWatch);
-            const stateWatch = watch(pathToWatch, {
-                ignoreInitial: true,
-                usePolling: true,
-                interval: 10_000,
-            });
+            const stateWatch = watch(pathToWatch, chokidarOptionsForStateKey(key));
             stateWatch.on('add', async (path) => this.handleStateFileUpdate(path, 'add'));
             stateWatch.on('change', async (path) => this.handleStateFileUpdate(path, 'change'));
             this.fileWatchers.push(stateWatch);