diff --git a/.changeset/quiet-moons-search.md b/.changeset/quiet-moons-search.md new file mode 100644 index 0000000000..1cf1616e6b --- /dev/null +++ b/.changeset/quiet-moons-search.md @@ -0,0 +1,18 @@ +--- +'@objectstack/metadata-fs': patch +--- + +fix(metadata-fs): the `FileSystemRepository` watcher now sees external edits in the production layout + +`MetadataPlugin` attaches the repository at `/.objectstack/metadata`, and the +watcher's `ignored` matcher was a bare dotfile regex. chokidar applies that matcher to the +watched root path itself, not only to entries found underneath it, so the `.objectstack` +segment of the root matched and the entire watch was inert — `getWatched()` returned `{}` +and no event ever fired. Hand edits, a `git checkout` that brings metadata JSON in, and any +other out-of-process writer under `.objectstack/metadata/` were invisible until the next +`start()`, even though `MetadataManager.setRepository()` is wired to those events and uses +them to invalidate the registry and the `list()` cache. + +The matcher is now evaluated against the path *relative* to the watch root, so dot segments +belonging to the root itself are never considered while dotfiles under the root — including +the repository's own `.objectstack/` bookkeeping subtree — stay ignored as before. diff --git a/packages/metadata-fs/src/repository.ts b/packages/metadata-fs/src/repository.ts index 8361757bb6..7426a3fa49 100644 --- a/packages/metadata-fs/src/repository.ts +++ b/packages/metadata-fs/src/repository.ts @@ -434,8 +434,12 @@ export class FileSystemRepository implements MetadataRepository { } private startWatcher(): void { - const w = chokidar.watch(this.layout.root, { - ignored: [/(^|[\\/])\../], // skip dotfiles incl. .objectstack + const root = this.layout.root; + const w = chokidar.watch(root, { + // Skip dotfiles under the root — including the repository's own + // `.objectstack/` bookkeeping subtree — matched on the path RELATIVE + // to the watch root (#7150). See `isIgnoredWatchPath`. + ignored: (p: string) => isIgnoredWatchPath(root, p), ignoreInitial: true, depth: 2, awaitWriteFinish: { stabilityThreshold: 50, pollInterval: 20 }, @@ -507,6 +511,43 @@ export class FileSystemRepository implements MetadataRepository { // ── Utilities ───────────────────────────────────────────────────────── +/** + * Watcher ignore matcher — "everything under the root, except the + * repository's own bookkeeping" (#7150). + * + * chokidar hands its matcher **absolute** paths, and applies it to the + * watched root itself as well as to entries discovered underneath it. The + * previous matcher was a bare dotfile regex (`/(^|[\\/])\../`), which + * therefore matched the `.objectstack` segment of the root path the plugin + * actually uses (`/.objectstack/metadata`, `REPO_SUBDIR` in + * `packages/metadata/src/plugin.ts`) and ignored the whole watch. Measured on + * chokidar 5 with this repository's own options, two identical trees + * differing only in whether the root sits under a dot-directory: + * + * plain root getWatched: ['', 'view'] events: add+change + * dot-rooted getWatched: [] events: none + * + * So the intent is kept and only the *frame of reference* is fixed: judge the + * path relative to the root, so dot segments belonging to the root itself are + * never considered. + * + * Why not drop the matcher entirely and lean on `parseItemPath`, which already + * rejects `.objectstack`? Measured: `parseItemPath` rejects that ONE name, so + * a dot-directory at the type level leaks — `/.cache/x.json` parses as + * type `.cache`, and `/view/.scratch.json` as an item named `.scratch`. + * Both would be published as `MetadataEvent`s while `scanHeads` skips every + * dot entry on boot, leaving the boot scan and the watcher disagreeing about + * what the repository contains. Dropping it also puts `.objectstack/.log/` in + * the poll set, so every one of the repository's own log appends wakes + * `handleFsChange` only to be discarded. + */ +function isIgnoredWatchPath(root: string, absPath: string): boolean { + const rel = path.relative(root, absPath); + // The watched root itself, and anything outside it, are not ours to judge. + if (rel === '' || rel.startsWith('..')) return false; + return rel.split(/[\\/]/).some((segment) => segment.startsWith('.')); +} + async function readJson(file: string): Promise { try { const text = await fs.readFile(file, 'utf8'); diff --git a/packages/metadata-fs/test/fs-behavior.test.ts b/packages/metadata-fs/test/fs-behavior.test.ts index f14995b4dc..8f07844f6f 100644 --- a/packages/metadata-fs/test/fs-behavior.test.ts +++ b/packages/metadata-fs/test/fs-behavior.test.ts @@ -15,6 +15,30 @@ const baseRef = (name: string): MetaRef => ({ const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); +/** + * Deadline for a **positive** watcher assertion — the longest a case waits for + * an event it expects to arrive. Raced against the event promise, so a healthy + * run still finishes in roughly the poll interval; the number is only ever + * paid on the way to a failure. + * + * Sized for the merge queue, not for a quiet laptop. The queue runs the FULL + * suite (PR-side CI runs only the affected subset), and this repository's + * watcher rides `usePolling: 1000ms` plus an `awaitWriteFinish` stability + * window — wall-clock timers that stretch under a saturated runner while the + * assertions themselves are unaffected. The `chokidar: external file change` + * case below ejected PR #7208 from the queue on its old 3s deadline while + * being green on the identical SHA in PR CI; `watch-dot-root.test.ts` ejected + * it a second time on an 8s one. Both are widened to this value. + */ +const EVENT_WAIT_MS = 20_000; + +/** + * Per-case ceiling for the watcher cases. Must clear a full `EVENT_WAIT_MS` + * plus setup, or the case dies on the vitest timeout before reaching its own + * deadline — which reports as a timeout instead of as the missing event. + */ +const WATCHER_CASE_TIMEOUT_MS = 45_000; + describe('FileSystemRepository — on-disk semantics', () => { let root: string; let repo: FileSystemRepository; @@ -98,14 +122,14 @@ describe('FileSystemRepository — on-disk semantics', () => { const file = path.join(root, 'view', 'externally_edited.json'); await fs.writeFile(file, JSON.stringify({ label: 'externally edited' }, null, 2)); - await Promise.race([collectorDone, sleep(3000)]); + await Promise.race([collectorDone, sleep(EVENT_WAIT_MS)]); await iter.return?.(undefined); expect(collected).toHaveLength(1); expect(collected[0]!.op).toBe('update'); expect(collected[0]!.source).toBe('fs'); expect(collected[0]!.actor).toBe('fs'); - }, 10000); + }, WATCHER_CASE_TIMEOUT_MS); it('chokidar: own writes do not re-trigger events (self-write suppression)', async () => { repo = new FileSystemRepository({ root, org: 'system' }); diff --git a/packages/metadata-fs/test/no-root-on-attach.test.ts b/packages/metadata-fs/test/no-root-on-attach.test.ts index cf05e3e3e4..1a70549a5c 100644 --- a/packages/metadata-fs/test/no-root-on-attach.test.ts +++ b/packages/metadata-fs/test/no-root-on-attach.test.ts @@ -31,6 +31,30 @@ import { FileSystemRepository } from '../src/index.js'; const ref = (name: string): MetaRef => ({ org: 'system', type: 'view', name }); const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); +/** + * Deadline for the **positive** watcher-arming assertion at the bottom of this + * file, raced against the event promise — a healthy run still finishes in + * roughly the poll interval, so the number is only paid on the way to a + * failure. + * + * Same value, and same reason, as `fs-behavior.test.ts` and + * `watch-dot-root.test.ts`: the merge queue runs the FULL suite while this + * watcher rides `usePolling: 1000ms` plus an `awaitWriteFinish` stability + * window, and those are wall-clock timers that stretch under a saturated + * runner. PR #7208 was ejected twice on exactly that mechanism, once from each + * of those two files. This case is the third instance of the identical + * event-wait shape in the same package and shard, so it is widened with them + * rather than left as the next one to eject. + */ +const EVENT_WAIT_MS = 20_000; + +/** + * Per-case ceiling. Must clear a full `EVENT_WAIT_MS` plus setup, or the case + * dies on the vitest timeout before reaching its own deadline — reporting as a + * timeout instead of as the missing event. + */ +const WATCHER_CASE_TIMEOUT_MS = 45_000; + describe('FileSystemRepository — attaching creates nothing on disk (#7000)', () => { /** Stands in for the project directory: nothing below it exists yet. */ let base: string; @@ -166,12 +190,12 @@ describe('FileSystemRepository — attaching creates nothing on disk (#7000)', ( JSON.stringify({ label: 'externally edited' }, null, 2), ); - await Promise.race([collector, sleep(8000)]); + await Promise.race([collector, sleep(EVENT_WAIT_MS)]); await iter.return?.(undefined); expect(collected).toHaveLength(1); expect(collected[0]!.op).toBe('update'); expect(collected[0]!.source).toBe('fs'); expect(collected[0]!.actor).toBe('fs'); - }, 20_000); + }, WATCHER_CASE_TIMEOUT_MS); }); diff --git a/packages/metadata-fs/test/watch-dot-root.test.ts b/packages/metadata-fs/test/watch-dot-root.test.ts new file mode 100644 index 0000000000..354ac104b8 --- /dev/null +++ b/packages/metadata-fs/test/watch-dot-root.test.ts @@ -0,0 +1,188 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #7150 — the watcher must see changes under a **dot-rooted** watch root. + * + * `MetadataPlugin` attaches the repository at `REPO_SUBDIR = + * '.objectstack/metadata'` under the project root, so in the layout the + * product actually ships the watched root path contains a dot segment. + * chokidar applies its `ignored` matcher to the watched root itself, not only + * to entries found underneath it, so the old bare dotfile regex + * (`/(^|[\\/])\../`) matched that segment and the whole watch was inert: + * `getWatched()` returned `{}` and no event ever fired. The consumer side is + * live code — `MetadataManager.setRepository()` subscribes to `repo.watch({})` + * and invalidates the registry and `list()` cache per event — so the source + * could never fire for the layout that is in use. + * + * The two cases below are the two halves of the watcher's promise, which the + * fix has to satisfy together: **see everything under the root** (case 1) and + * **ignore the repository's own bookkeeping** (case 2). A fix that only + * widened the matcher would pass case 1 and fail case 2. + * + * Sibling pin: `no-root-on-attach.test.ts` deliberately uses a root that is + * NOT under a dot-directory, because it measures watcher *arming* (#7000). + * Do not repurpose it — it has to keep measuring what it measures. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import fs from 'node:fs/promises'; +import path from 'node:path'; +import os from 'node:os'; +import type { MetaRef, MetadataEvent } from '@objectstack/metadata-core'; +import { FileSystemRepository } from '../src/index.js'; + +const ref = (name: string): MetaRef => ({ org: 'system', type: 'view', name }); +const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); + +/** + * Deadline for a **positive** watcher assertion — the longest a case waits for + * an event it expects to arrive. Each wait races this against the event + * promise, so a healthy run still finishes in roughly the poll interval; the + * number is only ever paid on the way to a failure. + * + * Sized for the merge queue, not for a quiet laptop. The queue runs the FULL + * suite (PR-side CI runs only the affected subset), and this watcher rides + * `usePolling: 1000ms` plus an `awaitWriteFinish` stability window — both are + * wall-clock timers that stretch when the runner is saturated, while the + * assertions themselves are unaffected. PR #7208 was ejected from the queue + * twice on exactly that: first `fs-behavior.test.ts > chokidar: external file + * change emits an update event` (3s deadline), then this file's positive case + * (8s deadline) — both green on the identical SHA in PR CI and locally. + */ +const EVENT_WAIT_MS = 20_000; + +/** + * Quiet window for the **negative** assertion below. ⛔ Never shorten this: a + * too-short quiet window cannot fail, it can only produce a FALSE PASS on an + * empty-array assertion. Its liveness control uses `EVENT_WAIT_MS` like every + * other positive wait — the control is a positive assertion and flakes under + * load the same way. + */ +const QUIET_WINDOW_MS = 4_000; + +/** + * Per-case ceiling. Has to clear the quiet window plus a full `EVENT_WAIT_MS` + * plus repository setup, or the case dies on the vitest timeout before its own + * deadline is reached — which reports as a timeout rather than as the missing + * event, and re-introduces the flake the deadlines above are widening away. + */ +const CASE_TIMEOUT_MS = 60_000; + +describe('FileSystemRepository watcher — dot-rooted watch root (#7150)', () => { + /** Stands in for the project directory. */ + let base: string; + /** Stands in for `/.objectstack/metadata` — the production layout. */ + let root: string; + let repo: FileSystemRepository | undefined; + + beforeEach(async () => { + base = await fs.mkdtemp(path.join(os.tmpdir(), 'objectstack-fs7150-')); + root = path.join(base, '.objectstack', 'metadata'); + }); + + afterEach(async () => { + if (repo) await repo.close().catch(() => undefined); + repo = undefined; + await fs.rm(base, { recursive: true, force: true }); + }); + + /** + * Drain `repo.watch()` into an array for the life of the case. `since: 999` + * skips log replay, so everything collected here came from the watcher. + */ + function collectEvents(r: FileSystemRepository): { + events: MetadataEvent[]; + first: Promise; + stop: () => Promise; + } { + const iter = r.watch({ org: 'system' }, 999)[Symbol.asyncIterator](); + const events: MetadataEvent[] = []; + let resolveFirst!: () => void; + const first = new Promise((res) => { resolveFirst = res; }); + let stopped = false; + void (async () => { + while (!stopped) { + const next = await iter.next(); + if (next.done) return; + events.push(next.value as MetadataEvent); + resolveFirst(); + } + })(); + return { + events, + first, + stop: async () => { stopped = true; await iter.return?.(undefined); }, + }; + } + + it('sees an external edit when the root is under a dot-directory', async () => { + repo = new FileSystemRepository({ root, org: 'system' }); // watcher ENABLED + await repo.start(); + + // The first write materializes the root and arms the watcher (#7000). + await repo.put(ref('case_grid'), { label: 'original' }, { + parentVersion: null, + actor: 'tester', + }); + + const sink = collectEvents(repo); + // Past the 200ms self-write suppression window of the put above. + await sleep(400); + + // An out-of-process writer: a hand edit, or a `git checkout` bringing + // metadata JSON in. Before the fix this produced nothing, ever. + await fs.writeFile( + path.join(root, 'view', 'case_grid.json'), + JSON.stringify({ label: 'externally edited' }, null, 2), + ); + + await Promise.race([sink.first, sleep(EVENT_WAIT_MS)]); + await sink.stop(); + + expect(sink.events).toHaveLength(1); + expect(sink.events[0]!.op).toBe('update'); + expect(sink.events[0]!.ref.name).toBe('case_grid'); + expect(sink.events[0]!.source).toBe('fs'); + expect(sink.events[0]!.actor).toBe('fs'); + }, CASE_TIMEOUT_MS); + + it('still ignores dot entries UNDER the root, including its own bookkeeping', async () => { + repo = new FileSystemRepository({ root, org: 'system' }); // watcher ENABLED + await repo.start(); + await repo.put(ref('seed'), { label: 'seed' }, { parentVersion: null, actor: 'tester' }); + + const sink = collectEvents(repo); + await sleep(400); + + // Noise that must stay invisible. `.cache/x.json` and `view/.scratch.json` + // are the measured leaks of "drop the matcher and let `parseItemPath` + // decide": that guard rejects the single name `.objectstack`, so these two + // parse as type `.cache` and as an item named `.scratch` respectively — + // while `scanHeads` skips every dot entry on boot. `.objectstack/.log/` + // is the repository's own change log. + await fs.mkdir(path.join(root, '.cache'), { recursive: true }); + await fs.writeFile(path.join(root, '.cache', 'x.json'), '{"junk":true}'); + await fs.writeFile(path.join(root, 'view', '.scratch.json'), '{"junk":true}'); + await fs.appendFile( + path.join(root, '.objectstack', '.log', 'main.jsonl'), + '{"not":"an event"}\n', + ); + + // Give the 1s poll several turns to deliver anything it was going to. + await sleep(QUIET_WINDOW_MS); + expect(sink.events).toEqual([]); + + // Control: the watcher is genuinely alive, so the emptiness above is the + // interesting kind and not a watcher that never armed. + await fs.writeFile( + path.join(root, 'view', 'seed.json'), + JSON.stringify({ label: 'really edited' }, null, 2), + ); + await Promise.race([sink.first, sleep(EVENT_WAIT_MS)]); + await sink.stop(); + + expect(sink.events).toHaveLength(1); + expect(sink.events[0]!.ref.name).toBe('seed'); + expect(sink.events[0]!.op).toBe('update'); + }, CASE_TIMEOUT_MS); +}); diff --git a/packages/metadata/src/metadata-repository-fs-dot-root.test.ts b/packages/metadata/src/metadata-repository-fs-dot-root.test.ts new file mode 100644 index 0000000000..0bbb2c1e7b --- /dev/null +++ b/packages/metadata/src/metadata-repository-fs-dot-root.test.ts @@ -0,0 +1,104 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #7150, consumer half — an external edit under the **production** metadata + * layout must reach `MetadataManager` and drop its caches. + * + * `MetadataPlugin` attaches a `FileSystemRepository` rooted at + * `/.objectstack/metadata` (`REPO_SUBDIR`), and `setRepository()` + * subscribes to `repo.watch({})`, re-emits each event through + * `notifyWatchers`, and invalidates the registry entry plus the `list()` + * cache for the affected type. That whole chain was dead in this layout: the + * repository's chokidar `ignored` matcher was a bare dotfile regex, chokidar + * applies it to the watched root path itself, and the root has a + * `.objectstack` segment — so the source end never fired. + * + * `metadata-fs`'s own `watch-dot-root.test.ts` pins the repository end. This + * case is the end-to-end one: a real `FileSystemRepository` on a real + * dot-rooted temp dir, a real out-of-process write, and the assertion made at + * the consumer — the seam that a reader of `metadata-fs` alone cannot see is + * load-bearing. + */ + +import { describe, it, expect } from 'vitest'; +import fs from 'node:fs/promises'; +import path from 'node:path'; +import os from 'node:os'; +import type { MetaRef } from '@objectstack/metadata-core'; +import type { MetadataWatchEvent } from '@objectstack/spec/system'; +import { FileSystemRepository } from '@objectstack/metadata-fs'; +import { MetadataManager } from './metadata-manager.js'; + +const ref = (name: string): MetaRef => ({ org: 'system', type: 'view' as never, name }); +const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); + +/** + * Deadline for the positive assertion below — the longest this case waits for + * an event it expects to arrive. The loop exits as soon as the event lands, so + * a healthy run costs about one poll interval and the number is only paid on + * the way to a failure. + * + * Sized for the merge queue, not for a quiet laptop, and matching the + * `EVENT_WAIT_MS` in `metadata-fs`'s three watcher files. The queue runs the + * FULL suite (PR-side CI runs only the affected subset) while the repository + * underneath rides `usePolling: 1000ms` plus an `awaitWriteFinish` stability + * window — wall-clock timers that stretch under a saturated runner without + * changing anything the assertions look at. PR #7208 was ejected from the + * queue twice on that mechanism, from `metadata-fs`'s side of this same + * watcher; this consumer-side wait is the same shape and is widened with them. + */ +const EVENT_WAIT_MS = 25_000; + +/** + * Per-case ceiling. Must clear a full `EVENT_WAIT_MS` plus repository and + * manager setup, or the case dies on the vitest timeout before reaching its + * own deadline — reporting as a timeout instead of as the missing event. + */ +const CASE_TIMEOUT_MS = 60_000; + +describe('MetadataManager ← FileSystemRepository under `.objectstack/metadata` (#7150)', () => { + it('an out-of-process write reaches subscribe() in the dot-rooted layout', async () => { + const base = await fs.mkdtemp(path.join(os.tmpdir(), 'objectstack-md7150-')); + // Exactly what `MetadataPlugin` builds: `path.join(rootDir, REPO_SUBDIR)`. + const root = path.join(base, '.objectstack', 'metadata'); + + const repo = new FileSystemRepository({ root, org: 'system' }); // watcher ENABLED + const mgr = new MetadataManager({ formats: ['json'] }); + try { + await repo.start(); + // The first write materializes the root and arms the watcher (#7000). + await repo.put(ref('case_grid'), { name: 'case_grid', label: 'Cases' }, { + parentVersion: null, + actor: 'tester', + }); + + const seen: MetadataWatchEvent[] = []; + mgr.setRepository(repo); + mgr.subscribe('view', (evt) => { seen.push(evt); }); + + // Past the repository's 200ms self-write suppression window. + await sleep(400); + const before = seen.length; + + // The out-of-process writer: a hand edit, or a `git checkout` bringing + // metadata JSON in. This is what was invisible until the next start(). + await fs.writeFile( + path.join(root, 'view', 'case_grid.json'), + JSON.stringify({ name: 'case_grid', label: 'Cases (edited on disk)' }, null, 2), + ); + + const deadline = Date.now() + EVENT_WAIT_MS; + while (seen.length === before && Date.now() < deadline) await sleep(100); + + const fresh = seen.slice(before); + expect(fresh).toHaveLength(1); + expect(fresh[0]!.type).toBe('changed'); + expect(fresh[0]!.metadataType).toBe('view'); + expect(fresh[0]!.name).toBe('case_grid'); + } finally { + await mgr.dispose().catch(() => undefined); + await repo.close().catch(() => undefined); + await fs.rm(base, { recursive: true, force: true }); + } + }, CASE_TIMEOUT_MS); +});