Uh oh!
There was an error while loading. Please reload this page.
fix(metadata): MetadataPlugin.watch defaults to false, matching its documented contract (#9770) - #9812
Merged
Conversation
…ocumented contract (#9770) `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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM
…adata-plugin-watch-default
Contributor
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 1 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 11 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
os-elon
marked this pull request as ready for review
August 19, 2026 01:37
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#9770
MetadataPluginOptions.watchdocuments its own default directly above the field(
packages/metadata/src/plugin.ts):The constructor implemented the opposite, and it did so in two places, so both
entry shapes resolved
true:this.options = { watch: true, ...options }trueundefinedthis.options.watch ?? truetrueBoth non-test construction sites in this repo —
packages/runtime/src/standalone-stack.tsand
packages/cli/src/commands/serve.ts— passwatch: falseexplicitly, each citing anEMFILE hazard, which is exactly why the drift was invisible: no in-repo configuration
exercised the default, so no test and no gate could see it.
MetadataPluginis a publicexport of
@objectstack/metadataand@objectstack/metadata/node, so the consumers whodid reach the default were external ones, reaching it by doing the documented-safe
thing and not naming the key at all. What they got was a recursive chokidar poll over the
entire project root — the pre-fix reverse-verification leg below prints the live watcher
object,
usePolling: true, interval: 1000, withnode_modulesexcluded only bychokidar's default
ignored.The change
watch: options.watch ?? false(this.options.watch ?? false), as adjudicatedThis is a default flip, not a capability removal: an explicit
watch: truestillattaches the scanner and its watcher. The sealed-runtime carve-out is untouched —
bootstrap: 'artifact-only'still forces watching off even against an explicitwatch: true.One deviation from the adjudicated spelling, and why
The ruling prescribed
{ watch: true, ...options }→{ watch: false, ...options }. ThisPR instead spells the same default as
{ ...options, watch: options.watch ?? false }. Theruling's substance is unchanged — the default is
false— but the prescribed spellingwould have left the two entry shapes diverging again, one layer further down:
watch: undefinedverbatim, sothis.options.watchwould stayundefinedfor that entry shape;start()-timeFileSystemRepositoryattachment readsdisableWatch: this.options.watch === falseandlogs
watch: this.options.watch !== false— identity checks, not nullish ones.So under the literal spelling, an omitted key would disable both watchers while an explicit
undefinedwould disable the source-file scanner but still attach the repository watcher,and the log line would claim watching was on. Normalizing once in the constructor makes
both entry shapes resolve identically at every downstream read. The
?? falsefallback is kept exactly as adjudicated, now as a defensive spelling.
Pins
Six pins in
packages/metadata/src/plugin.test.ts, all asserting on the observable —whether a watcher object exists on the manager — rather than on the resolved options value
alone, so an options field reading
falsewhile a watcher is still constructed cannot passthem:
watchkey omitted — no watcherwatch: undefinedexplicitly — no watcher, and the resolved value is normalizedfalsebootstrap: 'lazy'with the key omitted — no watcherwatch: true— watcher is attachedbootstrap: 'artifact-only'against an explicitwatch: true— no watcherVerification
Full package suite, on the merge commit:
pnpm --filter @objectstack/metadata test—31 files, 609 tests passed.
Reverse verification, both legs run against the built dependency closure, each restored and
proven byte-identical afterwards (
git diff --quiet→ YES). The subject resolves through arelative same-package import, so vitest transforms the mutated source directly; no
diststaleness can mask either leg, and both legs demonstrably changed the outcome:
plugin.tsreverted to its pre-fix version: 4 failed | 13 passed. Thefour failures are exactly the four default pins; the control and the artifact-only pin
stayed green. The failure output prints the attached chokidar watcher with
usePolling: true, interval: 1000.bootstrapMode === 'artifact-only'limb deleted from the fixed file:2 failed | 15 passed, the two artifact-only pins. That is how the carve-out was
verified still short-circuiting: it is load-bearing, and removing it is detected.
Gates
origin/mainwas merged first (it had moved to4c260cda5, carrying #9657's change tocheck-durability-degradation-log-level). Every reading below is post-merge, taken on167beb665with a clean tree, re-derived from the real diff withnode scripts/pm/dispatch-gates.mjsrather than from the dispatch's lead:check:changeset-gate-self-testscheck:cross-package-test-inputscheck:durability-log-levelcheck:objectui-changesetcheck:stack-collection-mapscheck:engine-double-contractcheck:where-matchercheck:query-options-erasurecheck:type-check-coveragecheck:type-check-debt --re-measurecheck-adr-0087-registration/check-changeset-no-major/check-empty-changesetdocs-audit/check-affected-docscheck:nul-bytescheck:type-check-debt --re-measurewas run with the workspace closure built(
turbo run build --filter='./packages/*' --filter='./packages/*/*', 70 tasks successful),so it is a real measurement, not a refusal. No baseline was raised.
@objectstack/metadatadeclares notypecheckscript — the invocation fails loudly withERR_PNPM_RECURSIVE_RUN_NO_SCRIPTrather than passing vacuously — so its type coverage isaccounted for through the ledger the re-measure above exercises.
Consequence for #9701
#9701 was closed
completedon the verdict that its watcher exposure was acceptablebecause no shipped configuration reaches the watcher. This default was the single thing
that made it reachable, and only for external consumers. No evidence was found that the
truedefault was deliberate — the doc comment, both call sites, andREADME.md's ownbootstrap: 'lazy'example (captioned "no FS scan", with the key omitted) all point theother way, and no test asserted a
truedefault. With the flip, that verdict standspermanently; #9701 does not need reopening.
Changeset: patch on
@objectstack/metadata.Generated by Claude Code
Generated by Claude Code