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

[elibosley]

Summary

• switch the state watcher back to watching canonical *.ini files directly instead of using directory-level .ini.new normalization
• configure an explicit atomic replacement window for state file watchers so .ini.new -> .ini replacements collapse into canonical file change handling
• keep the existing polling behavior for disks.ini and shares.ini, and update the state watcher tests to cover the canonical-file watcher shape

Testing

• pnpm --filter ./api test src/__test__/store/watch/state-watch.test.ts
• pnpm --filter ./api test src/__test__/store/watch/state-watch.integration.test.ts
• deployed api to devgen via pnpm --filter ./api unraid:deploy devgen
• verified on devgen that an atomic var.ini.new -> var.ini replacement emitted Loading state file for var after change in /var/log/graphql-api.log

Summary by CodeRabbit

• Tests

  • Enhanced state file watching tests to verify per-file monitoring with atomic replacement detection and conditional polling behavior.

• Refactor

  • Improved state file watching mechanism for more reliable detection of file changes and atomic file replacements across critical system state files.

[coderabbitai]

Walkthrough

The state file watcher has been refactored to monitor individual canonical state files (e.g., var.ini, disks.ini) rather than the entire directory. Each file watch includes atomic replacement detection with a 200ms window; polling is selectively enabled only for disks and shares keys. Tests were updated to validate per-file watcher registrations.

Changes

Cohort / File(s)	Summary
Test Updates (Unit) api/src/__test__/store/watch/state-watch.test.ts	Updated watcher assertions to verify individual file watches per StateFileKey (e.g., /usr/local/emhttp/state/var.ini) instead of single directory watch. Added validation for atomic: 200 and ignoreInitial: true options. Changed event handling from add to change events on canonical file paths. Adjusted routing logic to locate watchers by file path.
Test Updates (Integration) api/src/__test__/store/watch/state-watch.integration.test.ts	Added baseline var.ini and disks.ini file creation in temporary state directory. Refactored "var" reload test to model atomic replacement (write .new then rename to canonical). Adjusted "disks" negative test to assert that .new files don't trigger reactions until canonical file replacement.
Implementation api/src/store/watch/state-watch.ts	Removed filename normalization logic (.new suffix handling). Replaced directory-level watch with per-key chokidar watches using new chokidarOptionsForStateKey() function. Added atomic: 200 for replacement detection. Implemented conditional polling (only disks and shares use polling at 10s interval). Updated getStateFileKeyFromPath() to derive key directly from filename via parse(path).name.

Sequence Diagram(s)

sequenceDiagram
    participant FileSystem as File System
    participant Chokidar as Chokidar (per-file)
    participant StateManager as StateManager
    participant Dispatcher as Dispatcher
    
    Note over FileSystem,Dispatcher: Old Approach: Directory Watch
    FileSystem->>Chokidar: emit 'add' or 'change' for var.ini.new
    Chokidar->>StateManager: event detected
    StateManager->>StateManager: normalize filename, check if relevant
    alt relevant file detected
        StateManager->>Dispatcher: dispatch loadSingleStateFile()
    end
    
    Note over FileSystem,Dispatcher: New Approach: Per-File Atomic Watch
    FileSystem->>FileSystem: write var.ini.new
    FileSystem->>FileSystem: atomic rename var.ini.new → var.ini
    Chokidar->>StateManager: 'change' event on var.ini (within 200ms window)
    StateManager->>Dispatcher: dispatch loadSingleStateFile(StateFileKey.var)
    Dispatcher->>Dispatcher: process state update

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 Hark! Each file now gets its own keen watcher's eye,
No more the tangled directory dance, nor .new files awry,
With atomic replacements in two-hundred milliseconds fleet,
And selective polling for disks and shares—oh how neat!
The state now flows with purpose clear and true, ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: switching from directory-level watching to atomic canonical file watching for .ini files.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/state-watcher-atomic-file-paths

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 51.73%. Comparing base (7265105) to head (a6fa908).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1953      +/-   ##
==========================================
- Coverage   51.75%   51.73%   -0.02%     
==========================================
  Files        1026     1026              
  Lines       70750    70718      -32     
  Branches     7897     7881      -16     
==========================================
- Hits        36615    36586      -29     
+ Misses      34012    34009       -3     
  Partials      123      123              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

This plugin has been deployed to Cloudflare R2 and is available for testing.
Download it at this URL:

https://preview.dl.unraid.net/unraid-api/tag/PR1953/dynamix.unraid.net.plg

[coderabbitai]

🧹 Nitpick comments (1)

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

103-110: Consider increasing the timing margin for CI stability.

The 250ms wait before asserting that no dispatch occurred is shorter than the ATOMIC_REPLACEMENT_WINDOW_MS (200ms). While this should be sufficient, CI environments with I/O contention could introduce timing variability.

🔧 Consider a small timing buffer

         await writeFile(join(statesDirectory, 'disks.ini.new'), '[disk1]\nname=disk1\n');
-        await new Promise((resolve) => setTimeout(resolve, 250));
+        await new Promise((resolve) => setTimeout(resolve, 300));
 
         expect(store.dispatch).not.toHaveBeenCalled();

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@api/src/__test__/store/watch/state-watch.integration.test.ts` around lines
103 - 110, The 250ms sleep in the test "does not react to disks.ini.new before
the canonical file is replaced" is too close to ATOMIC_REPLACEMENT_WINDOW_MS and
can flake on CI; update the test to wait using the actual
ATOMIC_REPLACEMENT_WINDOW_MS plus a small buffer (e.g.,
ATOMIC_REPLACEMENT_WINDOW_MS + 100) instead of a hardcoded 250, importing
ATOMIC_REPLACEMENT_WINDOW_MS from the module that defines it (the watcher code)
and replace the new Promise setTimeout call in that test with a wait based on
that constant so CI timing variability is tolerated.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@api/src/__test__/store/watch/state-watch.integration.test.ts`:
- Around line 103-110: The 250ms sleep in the test "does not react to
disks.ini.new before the canonical file is replaced" is too close to
ATOMIC_REPLACEMENT_WINDOW_MS and can flake on CI; update the test to wait using
the actual ATOMIC_REPLACEMENT_WINDOW_MS plus a small buffer (e.g.,
ATOMIC_REPLACEMENT_WINDOW_MS + 100) instead of a hardcoded 250, importing
ATOMIC_REPLACEMENT_WINDOW_MS from the module that defines it (the watcher code)
and replace the new Promise setTimeout call in that test with a wait based on
that constant so CI timing variability is tolerated.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 6c4bdfd5-9022-4ca5-9bb1-fa0407ff87f0

📥 Commits

Reviewing files that changed from the base of the PR and between 7265105 and a6fa908.

📒 Files selected for processing (3)

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