fix(metadata): register()/unregister() announce to subscribe() watchers (#3112)#3131
Merged
Conversation
…rs (#3112) `MetadataManager.register()` updated the in-memory registry, persisted to writable loaders and published to the realtime service — but never called `notifyWatchers()`. `unregister()` had the same gap. Watchers only ever fired from the `saveMetaItem` path (indirectly, via the sys_metadata repository watch stream) and the filesystem watcher, so `subscribe()` looked like it covered every write while silently missing all of them. The consumer that pays for this is ObjectQL's SchemaRegistry bridge — the component that decides what is queryable. #3108 worked around it for the `metadata:reloaded` seam by carrying the parsed artifact on the event payload, but the asymmetry remained for every other `register()` call site and every other `subscribe()` consumer. Announce by default, so a new call site is correct without knowing the contract exists. Two concrete fixes fall out: `unregisterPackage()` now tells cached consumers to drop uninstalled objects instead of serving them until restart, and runtime datasource writes reach watchers. Bulk ingest opts out explicitly via the new `MetadataWriteOptions` (`{ notify: false }`) at the five boot/batch sites, each of which either runs before consumers cache anything or announces the whole batch once. ObjectQL's registry bridge MUST stay silent for a second reason: it copies objects OUT of the SchemaRegistry, and announcing would feed them back through a handler that re-registers under `_packageId ?? 'metadata-service'` — overwriting the true package provenance of every object whose body carries no `_packageId`. Additive only: the 3-arg forms keep working, and the api-surface ratchet reports 0 breaking / 1 added. Tests pin both halves of the contract (announce by default, silence only when asked) and the no-flood invariant on the reload path; both were verified to fail against the pre-change behavior. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 3 package(s): 108 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…, not a bug fix unregisterPackage()/bulkUnregister() have no production caller in framework or cloud today, so their announce is correct-but-latent. The one live behavior change is runtime datasource writes reaching the HMR SSE stream. Say so instead of advertising two 'concrete fixes'. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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 #3112.
The trap
MetadataManager.register()updated the in-memory registry, persisted to writable loaders and published to the realtime service — but never callednotifyWatchers().unregister()had the same gap.subscribe()looked like it covered every write, and covered almost none of them.One correction to the issue's framing, verified while investigating: watchers never fired "from the
saveMetaItempath" directly —saveMetaItemnever touches the MetadataManager at all. It writessys_metadata, the repository's in-memory broadcaster emits, andapplyRepoEvent(metadata-manager.ts:1590) translates that into the watch event. Same conclusion, different mechanism.The consumer that pays for this is ObjectQL's SchemaRegistry bridge — the component that decides what is queryable. #3108 worked around it for the
metadata:reloadedseam; the asymmetry remained everywhere else.Decision: option 1, with an explicit opt-out
Announcing is now the default, so a new call site (e.g. a cloud artifact-api mid-run refresh) is correct without knowing this contract exists. The five bulk/boot sites opt out explicitly via the new
MetadataWriteOptions—{ notify: false }— so the silent path is deliberate and visible at the call site instead of a global trap.Naive "just call notifyWatchers()" was not viable, for a reason beyond the flood the issue anticipated:
plugin.ts:367; the bulkbridgeObjectsToMetadataServiceruns at:492. The subscriber is already attached, so the flood is real._packageId ?? 'metadata-service'— silently overwriting the true package provenance of every bridged object whose body carries no_packageId. That site must stay silent on correctness grounds, not just cost.What actually changes at runtime
This is a contract fix, not a bug-fix bundle — being precise after auditing every consumer:
datasource-admin-plugin.ts:254/383) now reach the HMR SSE stream, which subscribes to every registered type. The "future mid-runregister()call site" the issue predicted already exists today, and was silent.unregisterPackage()/bulkUnregister()now announce their deletes. Correct, and regression-tested — but neither has a production caller in framework or cloud today, so nothing was observably broken. Called out so the changeset doesn't oversell it.objecttype sees no new events at runtime — both of itsregister()sites are deliberately silenced.Call-site posture
objectqlboot bridgemetadataartifact ingest (×3)metadata:reloadedmetadataFS boot primingdatasource-adminregister/unregisterunregisterPackage/bulkUnregisterVerification
register-notifies-watchers.test.ts(15 tests) pins both halves: announce by default, silence only on request. Verified non-vacuous — 8 fail against the pre-change manager, and the 6 asserting silence correctly still pass.expected [{ type: 'added', … }] to deeply equal []when the{ notify: false }opt-out is dropped.MetadataWriteOptions); snapshot regenerated. The 3-arg forms keep working — implementors with fewer params stay structurally compatible.Downstream (
cloud) — audited, follow-up filedcloudconsumes framework vialink:to this checkout, so this lands there on merge with no version bump. Audit result: 5 call sites, all boot-time bulk (service-aiagent bridge,ai-studiotools/agents/skills, legacy-agent cleanup), noagent/tool/skillsubscriber, and noIMetadataServiceimplementor — so no correctness break. They should still take{ notify: false }for the same reason this PR's bridge does:notifyWatchers()publishes to the cluster unconditionally whenever pubsub is attached, andapps/objectos-eeruns service-cluster + redis — so each node boot would otherwise emit ~50 needlessmetadata.changedbroadcasts carrying full definition bodies. Sequenced after this PR, sinceMetadataWriteOptionsdoes not exist until it merges.Reviewer notes
notifyWatchers()also does cluster fan-out, so runtimeregister()now broadcasts onmetadata.changed. That is the intended peer-invalidation design (see the field comment at metadata-manager.ts:132). No new secret egress:StoredDatasourcecarriesexternal.credentialsRef(a reference — "cleartext secret is transient, never stored"), andregister()already published the same body to the realtime service.registerInMemory()stays silent by design (GitOps-owned artefacts) — now documented on the method rather than left as folklore.🤖 Generated with Claude Code