From 35568cfb95606962defe45460149ede905699cc4 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 19 Aug 2026 00:23:12 +0000 Subject: [PATCH] fix(metadata): MetadataPlugin.watch defaults to false, matching its documented contract (#9770) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `MetadataPluginOptions.watch` documents `Default: false (post PR-10e — was previously true)` directly above the field, but the constructor implemented the opposite in two places: the options literal `{ watch: true, ...options }` (the omitted-key entry shape) and the `this.options.watch ?? true` fallback (the explicit-`undefined` entry shape). Both resolved `true`, so an external consumer constructing the public export without naming the key got the recursive project-root polling watcher that both in-repo call sites explicitly refuse, citing an EMFILE hazard at each. The flag is now normalized once in the constructor (`watch: options.watch ?? false`) instead of `{ watch: false, ...options }`: a spread preserves an explicitly-passed `undefined` verbatim, and not every read routes through a nullish fallback — the start()-time FileSystemRepository `disableWatch` keys on `=== false`. Coercing once makes both entry shapes resolve identically at every downstream read. The `?? false` fallback is kept as the adjudicated defensive spelling. This is a default flip, not a capability removal: an explicit `watch: true` still attaches the watcher, and the `bootstrap: 'artifact-only'` carve-out still forces watching off against an explicit `watch: true`. Pins assert on the observable (whether a watcher object exists on the manager), not on the resolved options value alone. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM --- .../metadata-plugin-watch-default-false.md | 37 ++++++++++ packages/metadata/src/plugin.test.ts | 72 +++++++++++++++++++ packages/metadata/src/plugin.ts | 13 +++- 3 files changed, 119 insertions(+), 3 deletions(-) create mode 100644 .changeset/metadata-plugin-watch-default-false.md diff --git a/.changeset/metadata-plugin-watch-default-false.md b/.changeset/metadata-plugin-watch-default-false.md new file mode 100644 index 0000000000..2d4d8cb104 --- /dev/null +++ b/.changeset/metadata-plugin-watch-default-false.md @@ -0,0 +1,37 @@ +--- +"@objectstack/metadata": patch +--- + +fix(metadata): `MetadataPlugin`'s `watch` option defaults to `false`, as its own doc comment documents (#9770) + +`MetadataPluginOptions.watch` documents its default as ``Default: `false` (post PR-10e — +was previously `true`)``, directly above the field. The constructor implemented the +opposite, and it did so in **two** places, so both entry shapes resolved `true`: + +- the options literal `{ watch: true, ...options }` — covering a caller who **omits** the key; +- the fallback `this.options.watch ?? true` — covering a caller who passes an explicit + `undefined`. + +Both non-test construction sites in this repo pass `watch: false` explicitly and are +unaffected either way, which is exactly why the drift was invisible to every test and +gate: no in-repo configuration exercised the default. `MetadataPlugin` is a public export +(`@objectstack/metadata`, `@objectstack/metadata/node`), so the consumers who did reach it +were **external** ones — and they reached it by doing the documented-safe thing and not +naming the key at all. What they got was the configuration both internal call sites go out +of their way to refuse, citing an **EMFILE** hazard at both: a recursive chokidar poll +(`usePolling: true, interval: 1000`) over the entire project root, with `node_modules` +excluded only by chokidar's default `ignored`. + +The default now resolves `false`. The flag is normalized once in the constructor +(`watch: options.watch ?? false`) rather than spelled `{ watch: false, ...options }`, +because a spread preserves an explicitly-passed `undefined` verbatim and not every read of +the flag routes through a nullish fallback — the `start()`-time `FileSystemRepository` +`disableWatch` keys on `=== false`. Coercing once makes an omitted key and an explicit +`undefined` resolve identically at every downstream read, instead of trading one +two-spelling divergence for another. + +This is a default flip, **not** a capability removal: an explicit `watch: true` still +attaches the scanner and its watcher, and the sealed-runtime carve-out +(`bootstrap: 'artifact-only'` forces watching off even against an explicit `watch: true`) +is untouched. Pins cover all four shapes, asserting on the **observable** — whether a +watcher object exists on the manager — rather than on the resolved options value alone. diff --git a/packages/metadata/src/plugin.test.ts b/packages/metadata/src/plugin.test.ts index 0a80958975..c0268b8a6d 100644 --- a/packages/metadata/src/plugin.test.ts +++ b/packages/metadata/src/plugin.test.ts @@ -58,6 +58,78 @@ describe('MetadataPlugin — bootstrap × watch coupling (D2)', () => { }); }); +// `MetadataPluginOptions.watch` documents its own default as `false` — the posture both +// non-test construction sites (`runtime/standalone-stack.ts`, `cli/commands/serve.ts`) +// assert explicitly, citing an EMFILE hazard: `watch: true` recursively polls the whole +// project root. The constructor used to implement the opposite, and it did so in TWO +// places — an object literal covering the omitted key, and a `?? true` fallback covering +// an explicit `undefined` — so both entry shapes are pinned here, on the OBSERVABLE +// (whether a watcher object exists) rather than on the resolved options value alone. +describe('MetadataPlugin — watch defaults to false, matching its documented contract', () => { + it('attaches NO filesystem watcher when the caller passes no options at all', () => { + const plugin = new MetadataPlugin(); + const mgr = (plugin as any).manager as NodeMetadataManager; + expect((mgr as any).watcher).toBeUndefined(); + expect((plugin as any).options.watch).toBe(false); + }); + + it('attaches NO filesystem watcher when the `watch` key is omitted', () => { + const plugin = new MetadataPlugin({ + config: { bootstrap: 'eager' }, + }); + const mgr = (plugin as any).manager as NodeMetadataManager; + expect((mgr as any).watcher).toBeUndefined(); + expect((plugin as any).options.watch).toBe(false); + }); + + it('attaches NO filesystem watcher when `watch` is explicitly undefined', () => { + const plugin = new MetadataPlugin({ + watch: undefined, + config: { bootstrap: 'eager' }, + }); + const mgr = (plugin as any).manager as NodeMetadataManager; + expect((mgr as any).watcher).toBeUndefined(); + // Normalized, not merely absent: every read of this flag — including the + // `start()`-time FileSystemRepository `disableWatch`, which keys on `=== false` + // rather than on a nullish fallback — must see the same resolved default. + expect((plugin as any).options.watch).toBe(false); + }); + + it('lazy bootstrap also attaches NO filesystem watcher by default', () => { + const plugin = new MetadataPlugin({ + config: { bootstrap: 'lazy' }, + }); + const mgr = (plugin as any).manager as NodeMetadataManager; + expect((mgr as any).watcher).toBeUndefined(); + }); + + // CONTROL — this is a default flip, NOT a removal of the capability. Co-located with + // the pins above deliberately: a regression that disabled watching outright would + // leave every `toBeUndefined()` above green. + it('still attaches a filesystem watcher when `watch: true` is explicit', () => { + const plugin = new MetadataPlugin({ + watch: true, + config: { bootstrap: 'eager' }, + }); + const mgr = (plugin as any).manager as NodeMetadataManager; + expect((mgr as any).watcher).toBeDefined(); + expect((plugin as any).options.watch).toBe(true); + return mgr.stopWatching(); + }); + + // The sealed-runtime carve-out is independent of the default and must stay intact: + // `artifact-only` forces watching off even against an explicit `watch: true`. + it('artifact-only bootstrap still short-circuits an explicit `watch: true`', () => { + const plugin = new MetadataPlugin({ + watch: true, + config: { bootstrap: 'artifact-only' }, + }); + const mgr = (plugin as any).manager as NodeMetadataManager; + expect((mgr as any).watcher).toBeUndefined(); + expect((plugin as any).options.watch).toBe(true); + }); +}); + // ───────────────────────────────────────────────────────────────────────── // PR-10e regression: artifact view items have no top-level `name`. Their // identity is the target object (encoded in `list.data.object` / diff --git a/packages/metadata/src/plugin.ts b/packages/metadata/src/plugin.ts index cc5d562c44..2dc31f9e78 100644 --- a/packages/metadata/src/plugin.ts +++ b/packages/metadata/src/plugin.ts @@ -245,9 +245,16 @@ export class MetadataPlugin implements Plugin { private lastParsedMetadata?: Record; constructor(options: MetadataPluginOptions = {}) { + // Documented default: `watch: false` (see {@link MetadataPluginOptions.watch}). + // Normalized here rather than spelled `{ watch: false, ...options }` because a + // spread preserves an explicitly-passed `watch: undefined` verbatim, and not + // every read of this flag routes through the nullish fallback below — the + // `start()`-time FileSystemRepository `disableWatch` keys on `=== false`. Coercing + // once here makes the omitted key and an explicit `undefined` resolve identically + // at EVERY downstream read. this.options = { - watch: true, - ...options + ...options, + watch: options.watch ?? false }; const rootDir = this.options.rootDir || process.cwd(); @@ -260,7 +267,7 @@ export class MetadataPlugin implements Plugin { // not as a side effect of any priming pass. const bootstrapMode = this.options.config?.bootstrap ?? 'eager'; const effectiveWatch = - bootstrapMode === 'artifact-only' ? false : (this.options.watch ?? true); + bootstrapMode === 'artifact-only' ? false : (this.options.watch ?? false); this.manager = new NodeMetadataManager({ rootDir,