fix(metadata-fs): declare startWatcher()'s chokidar atomic option explicitly

.changeset/metadata-fs-watcher-atomic-declared.md

@@ -0,0 +1,33 @@
+---
+"@objectstack/metadata-fs": patch
+---
+
+fix(metadata-fs): declare `startWatcher()`'s chokidar `atomic` option explicitly (#12696)
+
+`FileSystemRepository.startWatcher()` constructed its chokidar watcher with
+`usePolling: true` but never passed `atomic`, leaving it to inherit chokidar's
+default. That default is unconditionally `true` in the installed version
+(chokidar 5.0.0): the defaults literal assigns `atomic: true` *before* the
+caller's options are spread in, so chokidar's own default-correction
+(`if (opts.atomic === undefined) opts.atomic = !opts.usePolling`) can never
+fire — it only runs when `atomic` is literally `undefined` after the merge,
+which it never is. The comment beside that correction ("Editor atomic write
+normalization enabled by default with fs.watch") reads as "off under
+polling"; the actual resolved behaviour was on regardless.
+
+This change passes `atomic: true` explicitly at the call site, with a comment
+explaining why. **Patch, not a behaviour change**: verified at runtime
+(constructing a watcher the way `startWatcher()` does and reading back
+`watcher.options.atomic`) that the resolved value is identical before and
+after — `true` either way, today. The only thing that changes is that the
+value is now DECLARED rather than inherited from an upstream branch that
+cannot execute, so a future chokidar release that fixes the ordering (making
+the correction real) cannot silently flip this repository's watcher to
+`atomic: false` under polling and change behaviour with no diff to review.
+
+Not addressed here (see #12696): whether `atomic: true` (the 100ms
+unlink-coalescing deferral and the `DOT_RE` editor-temp-file matcher it turns
+on) is actually the right value. No evidence surfaced that either has ever
+affected a run; flipping it to `false` is a deliberate behaviour change to a
+live delivery path that needs its own reverse verification, and is out of
+scope for this card.

packages/metadata-fs/src/repository.ts

@@ -595,6 +595,23 @@ export class FileSystemRepository implements MetadataRepository {
  usePolling: true,
  interval: 1000,
  binaryInterval: 2000,
+ // Declared explicitly, not inherited. chokidar's own default-correction
+ // (`if (opts.atomic === undefined) opts.atomic = !opts.usePolling`) can
+ // only fire when the caller omits `atomic`, but its defaults literal
+ // already assigns `atomic: true` *before* the caller's options are
+ // spread in — so leaving `atomic` unset here does not mean "off under
+ // polling" the way the correction's own comment claims, it silently
+ // resolves to `true` regardless of `usePolling`. That has been this
+ // repository's actual runtime behaviour all along (verified by reading
+ // back the resolved option from a real watcher instance, #12696): every
+ // `unlink` gets chokidar's 100ms editor-atomic-write deferral, and
+ // `DOT_RE` (vim swap files, `~`, sublime tmp) is folded into
+ // `_isIgnored` on top of this repository's own `isIgnoredWatchPath`
+ // (#7150). `atomic: true` here keeps that behaviour byte-for-byte —
+ // this is a declaration, not a change. Flipping it to `false` would
+ // remove both behaviours from a live delivery path and needs its own
+ // reverse verification; see #12696 for the analysis.
+ atomic: true,
  });
  w.on('add', (p) => void this.handleFsChange(p, 'add'));
  w.on('change', (p) => void this.handleFsChange(p, 'change'));

packages/metadata-fs/test/watcher-atomic-declared.test.ts

@@ -0,0 +1,113 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #12696 — `startWatcher()` must pass `atomic` to chokidar EXPLICITLY.
+ *
+ * chokidar 5's defaults literal assigns `atomic: true` BEFORE the caller's
+ * options are spread in (`node_modules/chokidar/index.js`):
+ *
+ * const opts = {
+ * // Defaults
+ * ...
+ * atomic: true, // NOTE: overwritten later (depends on usePolling)
+ * ..._opts,
+ * ...
+ * };
+ * ...
+ * // Editor atomic write normalization enabled by default with fs.watch
+ * if (opts.atomic === undefined)
+ * opts.atomic = !opts.usePolling;
+ *
+ * so the "overwritten later" comment is aspirational: the correction can
+ * only run when the caller omits `atomic` AND that omission left
+ * `opts.atomic === undefined`, but it never does — the defaults literal
+ * already assigned `true`, and the caller's spread has no `atomic` key to
+ * override it with. The branch is dead.
+ *
+ * The consequence for THIS pin: the RESOLVED value chokidar hands back
+ * (`watcher.options.atomic`) is `true` whether `startWatcher()` declares it
+ * or not — a merged-value read cannot tell "declared here" apart from
+ * "inherited a dead branch that happens to agree today". A pin written
+ * against that merged read would stay green across the exact regression it
+ * exists to catch: a future chokidar release that fixes the ordering (making
+ * the correction real) would then silently flip this repository's watcher to
+ * `atomic: false` under `usePolling` — losing the 100ms unlink-coalescing
+ * deferral and the `DOT_RE` editor-temp matcher from a live delivery path —
+ * and nothing here would notice.
+ *
+ * So this pin does not read the merged option. It reads the ACTUAL call
+ * `startWatcher()` makes to `chokidar.watch()`, via a spy that lets the real
+ * call through unchanged (this pins DECLARATION, not delivery — every other
+ * watcher behaviour in this package must keep working identically). That is
+ * a runtime observation of what this repository hands off, not a grep of the
+ * call-site literal, and it is the one thing whose presence a future
+ * chokidar default cannot silently override.
+ *
+ * ⛔ No wall-clock wait anywhere in this file, deliberately (see
+ * `watch-dot-root.test.ts` for the standing prohibition and its history of
+ * merge-queue ejections). None is needed: the root is created by `mkdtemp()`
+ * before `start()` runs, so `start()` arms the watcher SYNCHRONOUSLY inside
+ * its own call (see its comment on #7000/#9339) — `chokidar.watch()` is
+ * invoked, and the spy has recorded the call, by the time `start()`'s
+ * promise resolves. This case never touches the 100ms unlink window itself
+ * (out of scope for #12696 — see the card's "carry forward" section).
+ */
+
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
+import fs from 'node:fs/promises';
+import path from 'node:path';
+import os from 'node:os';
+import chokidar from 'chokidar';
+import { FileSystemRepository } from '../src/index.js';
+
+describe('FileSystemRepository — startWatcher() declares `atomic` explicitly (#12696)', () => {
+ let root: string;
+ let repo: FileSystemRepository | undefined;
+ let watchSpy: ReturnType<typeof vi.spyOn>;
+
+ beforeEach(async () => {
+ root = await fs.mkdtemp(path.join(os.tmpdir(), 'objectstack-fsatomic-'));
+ // Let the real call through — nothing here is faked, so every other
+ // watcher behaviour this case exercises stays exactly as it runs in
+ // production.
+ watchSpy = vi.spyOn(chokidar, 'watch');
+ });
+
+ afterEach(async () => {
+ if (repo) await repo.close().catch(() => undefined);
+ repo = undefined;
+ watchSpy.mockRestore();
+ await fs.rm(root, { recursive: true, force: true });
+ });
+
+ it('passes `atomic: true` as an OWN key of the options object handed to chokidar.watch()', async () => {
+ repo = new FileSystemRepository({ root, org: 'system', disableWatch: false });
+ await repo.start();
+
+ expect(watchSpy).toHaveBeenCalledTimes(1);
+ const [, options] = watchSpy.mock.calls[0] as [string, Record<string, unknown>];
+
+ // The behaviour this pin exists to protect: `atomic` must be an OWN key
+ // of the call-site options object, not merely absent-and-coincidentally
+ // `true` after chokidar's internal merge (see file header).
+ expect(Object.prototype.hasOwnProperty.call(options, 'atomic')).toBe(true);
+ expect(options.atomic).toBe(true);
+ });
+
+ // Positive control (#12696 ablation) — a SEPARATE case so it runs to
+ // completion, and so its verdict, on its own assertions, is independent of
+ // whatever the case above does. `usePolling` is unconditionally declared
+ // today and untouched by this card's fix; it must stay green under the
+ // ablation that removes the explicit `atomic`. If it ever went red too,
+ // the spy/harness would be the suspect, not the `atomic` declaration.
+ it('[control] passes `usePolling: true` as an OWN key of the same options object', async () => {
+ repo = new FileSystemRepository({ root, org: 'system', disableWatch: false });
+ await repo.start();
+
+ expect(watchSpy).toHaveBeenCalledTimes(1);
+ const [, options] = watchSpy.mock.calls[0] as [string, Record<string, unknown>];
+
+ expect(Object.prototype.hasOwnProperty.call(options, 'usePolling')).toBe(true);
+ expect(options.usePolling).toBe(true);
+ });
+});