fix(metadata): getViewsByObject() expands aggregated view containers instead of answering empty - #14206
Conversation
📓 Docs Drift CheckThis PR changes 1 package(s): 3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 2 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 12 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 76764bb12eb24c3326829918c002402033184cbc && git checkout 76764bb12eb24c3326829918c002402033184cbc
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin c41b42e8db6efdc7091e9c320c0598cd30c7777d 523d6e4affbe94d27e93724ffc6618c9795c3b87 && git checkout -B drift-repro c41b42e8db6efdc7091e9c320c0598cd30c7777d && git merge --no-ff 523d6e4affbe94d27e93724ffc6618c9795c3b87
node scripts/docs-audit/affected-docs.mjs --json c41b42e8db6efdc7091e9c320c0598cd30c7777d
|
Fixes #13913
The defect
MetadataManager.getViewsByObject()readsthis.list('view')— the manager'sown loader-based store (
this.registryplus every registered loader). Thatis a completely different store from the
sys_metadatarows the REST route(
GET /meta/view?object=) reads throughObjectStackProtocolImplementation.getMetaItems.#13407 taught
getMetaItemsto expand a runtime-authored aggregateddefineViewcontainer inline. This exit never callsgetMetaItemsand had noequivalent step, so the very container #13407/#13929 made visible on the wire
still answered empty here — for every internal/SDK caller that uses this
entry point rather than the route.
What was NOT done, and why
.object, exactly asViewSchema.objectdeclares it should. The gap was in the store, not thepredicate.
viewKindrequirement was not relaxed. That is the tempting one-line"fix" and it is a regression: it answers with the container itself as a
view, the behaviour [finding] the nested-plugin seam does not expand an aggregated
viewscontainer — a nested plugin's per-view items never reach the registry, sogetViewsByObject()/GET /meta/view?object=answer with the container alone #7163 ruled wrong. This PR pins against it — seeAblation 2 below, where the pin is measured firing.
packages/specwas not touched. No accept-set moves;ViewSchema.objectalready names
getViewsByObject()as one of its readers, so this restores adeclared contract rather than widening one.
The repair
getViewsByObject()expands aggregated containers inline, per read, andkeeps the filter as-is — what changes is that the filter now also sees each
container's expansion, whose items carry the
viewKind+objectpair it hasalways tested.
Registry-free, mirroring #13407's choice at the other exit and for the same
reason:
MetadataManager's registry is process-wide, so a read must not graftrows into it. Already-present names win, so a container whose expanded ViewItems
were registered by a source registrar (the ObjectQL boot loop, the artifact/HMR
loader) still answers with those registered, fully-enriched items and gains
nothing new.
The cross-package reuse question, measured
The card asked whether
expandRuntimeViewContainercould be shared frompackages/metadata-protocol, or whetherpackages/metadataneeds a localequivalent. Neither, as posed — the reusable substance already sits below
both packages. Three readings:
@objectstack/metadata-protocoldepends on@objectstack/metadata(itspackage.jsondependencies). An import the other way inverts an existing edge and closes a cycle.expandRuntimeViewContaineris a private method onObjectStackProtocolImplementation(protocol.ts), reachable only throughthis, and it calls two further private members.⇒ A shared export from
metadata-protocolis the wrong direction and is nottaken. But this is not duplication either: the canonical expansion
primitives (
isAggregatedViewContainer,expandViewContainer) live in@objectstack/spec, one level below both packages, and this package alreadyimports them (
plugin.ts). They are reused unchanged.What is genuinely local is the ~6-line object-derivation chain — and that is
the part that has silently drifted three ways: the ObjectQL boot loop keys off
the registration name,
plugin.tswalks two levels(
list.data.object→form.data.object), andprotocol.tswalks four since#13407 (
object→list.data.object→form.data.object→name). This PRgives
packages/metadataone spelling of it,deriveViewContainerObject()in the newview-container-expansion.ts, so thedrift has a single place to be repaired rather than a third private copy to fall
behind.
📌 For #13912's implementer: that card's surface (
plugin.ts, 2 sites) stillcarries the pre-#13407 two-deep chain, which is exactly why a container with
a top-level
objectand nolist.data.objectis dropped there. ImportderiveViewContainerObjectfrom./view-container-expansion.js— same package,relative import, no public-surface change — rather than adding a fourth copy.
Clause-② : no. The new module is internal: it is not re-exported from
packages/metadata/src/index.ts, so@objectstack/metadata's published surfaceis unchanged, and no accept set moves.
Tests
New pin:
packages/metadata/src/metadata-manager-views-by-object-container.test.ts(7 cases). Every assertion drives
getViewsByObject()itself — a pin writtenagainst
GET /meta/view?object=would have gone green onmainwithouttouching this bug, since #13929 already repaired that exit. The fixture is the
card's own shape: a top-level
objectand nolist.data.object.Both admission routes are driven: a container registered in the in-memory
registry, and a container arriving from a
DatabaseLoaderover a mocksys_metadatastore.Ablation 1 — revert the repair (direction re-predicted, then measured)
Run on the committed tree; the mutation was confirmed on disk by marker
counts and a
git hash-objectcomparison against the HEAD blob before anyreading was taken, and the restore was proven by the file's hash returning to
the HEAD blob with
git diff HEADempty. No rebuild is involved: the pinimports
./metadata-manager.jsrelatively, so vitest resolves it to thepackage's own source rather than through
dist— independently evidenced by thevery first pin run, which observed the new expansion behaviour with
@objectstack/metadataitself never built.Reverting
getViewsByObjectto its pre-#13913 body: 4 red / 3 green.expected 0 to be greater than 0)objectwhenlist.data.objectis absentThe first ablation corrected the test. The dedupe case was originally one
control asserting both the full answer and the registered item's identity; it
went red, because pre-fix the answer is that registered item alone. A control
that goes red under ablation is not a control, so it was split: the set
assertion became an ordinary case, and the both-directions half is the object-
identity assertion. The prediction above is the re-prediction made after that
split, and it matched exactly.
Ablation 2 — apply the forbidden one-line "fix"
The #7163 case is green under Ablation 1 only vacuously (the pre-fix answer is
empty, so nothing can be a container). It guards a different mutation, so it was
ablated against that one: dropping
v.viewKind &&from the filter — thetempting one-liner — makes the container answer as a view.
Measured: 5 red, including
answers with EXPANDED items and never the container itself (#7163). The guard is known to fire, not assumed to.Gates
Derived at the actual diff with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands(the script takes its own change set from the merge base; no hand-built path
list was fed to it) and run with exit codes captured by redirect before any
pipe.
32 of 35 green. 3 are PREREQUISITE NOT MET — recorded as NOT MEASURED, never as a
pass, on each gate's own instruction; all three need a whole-repo
pnpm build,which is CI's run, and each says in its own words that it is "NOT a pass and NOT
a finding". Verdicts are quoted from the lines the gates print, not from a bare
$?.node scripts/check-test-completeness.mjsturbo run testlog; the family names it with no argument. Its own text: "running the family locally, record this gate as NOT MEASURED".pnpm check:dual-build-cjs-loadspnpm check:type-check-debt--re-measurerefuses without the built closure: "measuring now would not fail, it would silently measure a DIFFERENT WORLD". Its siblingpnpm check:type-check-coverageran green (exit 0).Note
check:engine-double-contract,check:cross-package-test-inputsandcheck:test-source-aliasare all green — the new test file adds no cross-packageread and no aliased import.
The type-check ratchet, measured directly
@objectstack/metadatahas notypecheckscript; it is a shrink-only DEBTledger entry frozen at 89 raw errors. Its dependency closure is built here,
so the number was measured rather than deferred:
And the files were proven to be in the program rather than silently excluded —
tsc --noEmit --listFilesnames all three:view-container-expansion.ts,metadata-manager-views-by-object-container.test.ts,metadata-manager.ts.Repo-wide lint
Run in full, not narrowed:
All three touched files appear in that run at 0/0.
Mergeability
Re-checked at the end against a freshly fetched
origin/main(c41b42e8):git merge-tree --write-tree origin/main HEADreturns a clean tree, exit 0 — noconflicts, so nothing was merged in. The three upstream commits since the base
touch
driver-sql,lint, and one docblock inpackages/spec/src/shared/identifiers.zod.ts;none overlaps this diff behaviourally. The gate family was re-derived after that
fetch and is byte-identical to the family run above.
Adjacent defect found and filed, NOT fixed here
readListUncached()drops every loader-held item whose stored body has notop-level
name— which an aggregated view container has none of, by design. Soa container persisted through
register()is invisible tolist('view')entirely after a restart, before this exit ever sees it. Measured with a positive
control and filed unassigned as #14205. Out of scope: different function,
different failure path, and repairing it changes
list()for every metadata type.Generated by Claude Code