Skip to content

Commit 2a9752c

Browse files
os-elonclaude
andauthored
fix(metadata): MetadataPlugin.watch defaults to false, matching its documented contract (#9770) (#9812)
`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. Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4e52147 commit 2a9752c

3 files changed

Lines changed: 119 additions & 3 deletions

File tree

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
---
2+
"@objectstack/metadata": patch
3+
---
4+
5+
fix(metadata): `MetadataPlugin`'s `watch` option defaults to `false`, as its own doc comment documents (#9770)
6+
7+
`MetadataPluginOptions.watch` documents its default as ``Default: `false` (post PR-10e —
8+
was previously `true`)``, directly above the field. The constructor implemented the
9+
opposite, and it did so in **two** places, so both entry shapes resolved `true`:
10+
11+
- the options literal `{ watch: true, ...options }` — covering a caller who **omits** the key;
12+
- the fallback `this.options.watch ?? true` — covering a caller who passes an explicit
13+
`undefined`.
14+
15+
Both non-test construction sites in this repo pass `watch: false` explicitly and are
16+
unaffected either way, which is exactly why the drift was invisible to every test and
17+
gate: no in-repo configuration exercised the default. `MetadataPlugin` is a public export
18+
(`@objectstack/metadata`, `@objectstack/metadata/node`), so the consumers who did reach it
19+
were **external** ones — and they reached it by doing the documented-safe thing and not
20+
naming the key at all. What they got was the configuration both internal call sites go out
21+
of their way to refuse, citing an **EMFILE** hazard at both: a recursive chokidar poll
22+
(`usePolling: true, interval: 1000`) over the entire project root, with `node_modules`
23+
excluded only by chokidar's default `ignored`.
24+
25+
The default now resolves `false`. The flag is normalized once in the constructor
26+
(`watch: options.watch ?? false`) rather than spelled `{ watch: false, ...options }`,
27+
because a spread preserves an explicitly-passed `undefined` verbatim and not every read of
28+
the flag routes through a nullish fallback — the `start()`-time `FileSystemRepository`
29+
`disableWatch` keys on `=== false`. Coercing once makes an omitted key and an explicit
30+
`undefined` resolve identically at every downstream read, instead of trading one
31+
two-spelling divergence for another.
32+
33+
This is a default flip, **not** a capability removal: an explicit `watch: true` still
34+
attaches the scanner and its watcher, and the sealed-runtime carve-out
35+
(`bootstrap: 'artifact-only'` forces watching off even against an explicit `watch: true`)
36+
is untouched. Pins cover all four shapes, asserting on the **observable** — whether a
37+
watcher object exists on the manager — rather than on the resolved options value alone.

packages/metadata/src/plugin.test.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,78 @@ describe('MetadataPlugin — bootstrap × watch coupling (D2)', () => {
5858
});
5959
});
6060

61+
// `MetadataPluginOptions.watch` documents its own default as `false` — the posture both
62+
// non-test construction sites (`runtime/standalone-stack.ts`, `cli/commands/serve.ts`)
63+
// assert explicitly, citing an EMFILE hazard: `watch: true` recursively polls the whole
64+
// project root. The constructor used to implement the opposite, and it did so in TWO
65+
// places — an object literal covering the omitted key, and a `?? true` fallback covering
66+
// an explicit `undefined` — so both entry shapes are pinned here, on the OBSERVABLE
67+
// (whether a watcher object exists) rather than on the resolved options value alone.
68+
describe('MetadataPlugin — watch defaults to false, matching its documented contract', () => {
69+
it('attaches NO filesystem watcher when the caller passes no options at all', () => {
70+
const plugin = new MetadataPlugin();
71+
const mgr = (plugin as any).manager as NodeMetadataManager;
72+
expect((mgr as any).watcher).toBeUndefined();
73+
expect((plugin as any).options.watch).toBe(false);
74+
});
75+
76+
it('attaches NO filesystem watcher when the `watch` key is omitted', () => {
77+
const plugin = new MetadataPlugin({
78+
config: { bootstrap: 'eager' },
79+
});
80+
const mgr = (plugin as any).manager as NodeMetadataManager;
81+
expect((mgr as any).watcher).toBeUndefined();
82+
expect((plugin as any).options.watch).toBe(false);
83+
});
84+
85+
it('attaches NO filesystem watcher when `watch` is explicitly undefined', () => {
86+
const plugin = new MetadataPlugin({
87+
watch: undefined,
88+
config: { bootstrap: 'eager' },
89+
});
90+
const mgr = (plugin as any).manager as NodeMetadataManager;
91+
expect((mgr as any).watcher).toBeUndefined();
92+
// Normalized, not merely absent: every read of this flag — including the
93+
// `start()`-time FileSystemRepository `disableWatch`, which keys on `=== false`
94+
// rather than on a nullish fallback — must see the same resolved default.
95+
expect((plugin as any).options.watch).toBe(false);
96+
});
97+
98+
it('lazy bootstrap also attaches NO filesystem watcher by default', () => {
99+
const plugin = new MetadataPlugin({
100+
config: { bootstrap: 'lazy' },
101+
});
102+
const mgr = (plugin as any).manager as NodeMetadataManager;
103+
expect((mgr as any).watcher).toBeUndefined();
104+
});
105+
106+
// CONTROL — this is a default flip, NOT a removal of the capability. Co-located with
107+
// the pins above deliberately: a regression that disabled watching outright would
108+
// leave every `toBeUndefined()` above green.
109+
it('still attaches a filesystem watcher when `watch: true` is explicit', () => {
110+
const plugin = new MetadataPlugin({
111+
watch: true,
112+
config: { bootstrap: 'eager' },
113+
});
114+
const mgr = (plugin as any).manager as NodeMetadataManager;
115+
expect((mgr as any).watcher).toBeDefined();
116+
expect((plugin as any).options.watch).toBe(true);
117+
return mgr.stopWatching();
118+
});
119+
120+
// The sealed-runtime carve-out is independent of the default and must stay intact:
121+
// `artifact-only` forces watching off even against an explicit `watch: true`.
122+
it('artifact-only bootstrap still short-circuits an explicit `watch: true`', () => {
123+
const plugin = new MetadataPlugin({
124+
watch: true,
125+
config: { bootstrap: 'artifact-only' },
126+
});
127+
const mgr = (plugin as any).manager as NodeMetadataManager;
128+
expect((mgr as any).watcher).toBeUndefined();
129+
expect((plugin as any).options.watch).toBe(true);
130+
});
131+
});
132+
61133
// ─────────────────────────────────────────────────────────────────────────
62134
// PR-10e regression: artifact view items have no top-level `name`. Their
63135
// identity is the target object (encoded in `list.data.object` /

packages/metadata/src/plugin.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -245,9 +245,16 @@ export class MetadataPlugin implements Plugin {
245245
private lastParsedMetadata?: Record<string, unknown[]>;
246246

247247
constructor(options: MetadataPluginOptions = {}) {
248+
// Documented default: `watch: false` (see {@link MetadataPluginOptions.watch}).
249+
// Normalized here rather than spelled `{ watch: false, ...options }` because a
250+
// spread preserves an explicitly-passed `watch: undefined` verbatim, and not
251+
// every read of this flag routes through the nullish fallback below — the
252+
// `start()`-time FileSystemRepository `disableWatch` keys on `=== false`. Coercing
253+
// once here makes the omitted key and an explicit `undefined` resolve identically
254+
// at EVERY downstream read.
248255
this.options = {
249-
watch: true,
250-
...options
256+
...options,
257+
watch: options.watch ?? false
251258
};
252259

253260
const rootDir = this.options.rootDir || process.cwd();
@@ -260,7 +267,7 @@ export class MetadataPlugin implements Plugin {
260267
// not as a side effect of any priming pass.
261268
const bootstrapMode = this.options.config?.bootstrap ?? 'eager';
262269
const effectiveWatch =
263-
bootstrapMode === 'artifact-only' ? false : (this.options.watch ?? true);
270+
bootstrapMode === 'artifact-only' ? false : (this.options.watch ?? false);
264271

265272
this.manager = new NodeMetadataManager({
266273
rootDir,

0 commit comments

Comments
 (0)