Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions .changeset/metadata-plugin-watch-default-false.md
Original file line numberDiff line numberDiff line change
@@ -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.
72 changes: 72 additions & 0 deletions packages/metadata/src/plugin.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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` /
Expand Down
13 changes: 10 additions & 3 deletions packages/metadata/src/plugin.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -245,9 +245,16 @@ export class MetadataPlugin implements Plugin {
private lastParsedMetadata?: Record<string, unknown[]>;

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();
Expand All@@ -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,
Expand Down
Loading