Skip to content

Commit 41aa979

Browse files
os-muskclaude
andauthored
fix(metadata): read a view container's own object in the artifact/HMR loader (#14401)
`MetadataPlugin._parseAndRegisterArtifact` derives which object an aggregated `defineView` container binds to at two sites, and both walked exactly two levels — `list.data.object` then `form.data.object` — never consulting `ViewSchema.object`, the field whose own `.describe()` names its readers ("read by `getViewsByObject()` / `GET /meta/view?object=`"). A container that declares the binding once at the top and omits `data` from its view arms was therefore dropped by the first site's `if (!viewObject) continue` BEFORE any registration, so `list('view')` never returned it and neither the #13913 read backstop nor `GET /meta/view?object=` could see it. Both sites now call `deriveViewContainerObject` — this package's single spelling of the derivation, landed with #13913 and documented there as the one place the drift is repaired rather than a third private copy to fall behind. It carries the order #13407 settled at the runtime door: the container's own `object` first, then the unchanged `list.data.object` -> `form.data.object` -> row-name fallback. Card relation is declared once in the PR body (this branch squashes). Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8eeca27 commit 41aa979

3 files changed

Lines changed: 275 additions & 7 deletions

File tree

.changeset/lucky-pots-attend.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
'@objectstack/metadata': patch
3+
---
4+
5+
Artifact/HMR loader: read a view container's own top-level `object` field
6+
7+
`MetadataPlugin`'s artifact/HMR registrar derived which object a `defineView`
8+
container binds to by walking `list.data.object` then `form.data.object`, and
9+
never consulted the container's own top-level `object` — the field
10+
`ViewSchema.object` documents as "how a stack-level `views: [...]` entry says
11+
which object its views belong to; read by `getViewsByObject()` /
12+
`GET /meta/view?object=`".
13+
14+
A package-shipped `defineView({ object: 'crm_lead', list: { columns: [...] } })`
15+
therefore registered nothing at all through this path: the container was dropped
16+
before registration, so no expanded `crm_lead.<key>` ViewItems were produced and
17+
`getViewsByObject('crm_lead')` / `GET /meta/view?object=crm_lead` answered empty
18+
for it. Both derivation sites now call the package's single spelling of that
19+
derivation (`deriveViewContainerObject`), which consults the container's own
20+
`object` first and keeps the existing `list.data.object``form.data.object`
21+
row-name fallback unchanged for every container written before that field was
22+
read here. This is the same order #13407 settled at the runtime door.
Lines changed: 224 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,224 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #13912 — the artifact/HMR registrar never read a view container's OWN
5+
* top-level `object` field.
6+
*
7+
* ---------------------------------------------------------------------------
8+
* The defect, and why it survived #13407 and #13913
9+
* ---------------------------------------------------------------------------
10+
* `MetadataPlugin._parseAndRegisterArtifact` derives "which object does this
11+
* container bind to" at two sites, and both walked exactly two levels —
12+
* `list.data.object` then `form.data.object` — never consulting
13+
* `ViewSchema.object`, the field whose own `.describe()` names its readers:
14+
* "how a stack-level `views: [...]` entry says which object its views belong
15+
* to; read by `getViewsByObject()` / `GET /meta/view?object=`".
16+
*
17+
* #13407 repaired the RUNTIME door (`packages/metadata-protocol`, the
18+
* `PUT /meta/view` path) and #13913 repaired the READ backstop
19+
* (`MetadataManager.getViewsByObject`, which expands a container it finds in
20+
* its own store). Neither reaches this registrar, and the reason the backstop
21+
* does not cover it is structural rather than incidental: on the failing shape
22+
* the first site's `if (!viewObject) continue` dropped the container BEFORE any
23+
* registration, so `list('view')` never returned it and `getViewsByObject()`
24+
* had nothing to expand. The gap therefore survived both fixes exactly as the
25+
* card reports.
26+
*
27+
* ---------------------------------------------------------------------------
28+
* What is driven, and why it is the real door
29+
* ---------------------------------------------------------------------------
30+
* Every case goes through `_parseAndRegisterArtifact` — the same entry the boot
31+
* artifact load and the HMR reload share — with a bare `ObjectStackDefinition`,
32+
* so each fixture also passes the door's STRICT parse. That is load-bearing
33+
* here: `ObjectListViewSchema.data` requires `object` when `data` is present at
34+
* all, so the reachable failing shape is a container that declares the binding
35+
* once at the top and omits `data` from its view arms (measured — a `list` with
36+
* `data: { provider: 'object' }` and no `object` is refused by the door before
37+
* this code is reached, and a pin written on that shape would be testing the
38+
* schema, not this registrar).
39+
*
40+
* ---------------------------------------------------------------------------
41+
* Controls
42+
* ---------------------------------------------------------------------------
43+
* The two `CONTROL:` cases are green in BOTH directions, measured under the
44+
* ablation recorded in the PR body: this change must not move what the door
45+
* already registered, so a control going red would be reporting a regression
46+
* rather than this fix. Neither may therefore depend on the top-level `object`
47+
* being read.
48+
*/
49+
50+
import { describe, it, expect, vi } from 'vitest';
51+
import { MetadataPlugin } from './plugin.js';
52+
53+
const logger = vi.hoisted(() => ({
54+
info: vi.fn(),
55+
warn: vi.fn(),
56+
error: vi.fn(),
57+
debug: vi.fn(),
58+
}));
59+
60+
vi.mock('@objectstack/core', async (orig) => ({
61+
...((await orig()) as object),
62+
createLogger: () => logger,
63+
}));
64+
65+
const MANIFEST = { id: 'crm', name: 'CRM', version: '1.0.0', type: 'app' };
66+
67+
/**
68+
* The card's shape: the binding lives ONLY in the container's own top-level
69+
* `object`. No view arm carries `data` at all — which is both the natural way
70+
* to author a container whose object is declared once at the top, and the only
71+
* arrangement the door's strict parse admits with no `data.object` anywhere.
72+
*/
73+
const objectOnlyContainer = {
74+
object: 'crm_lead',
75+
list: { label: 'All Leads', type: 'grid', columns: [{ field: 'name' }, { field: 'company' }] },
76+
listViews: { hot: { label: 'Hot Leads', type: 'grid', columns: [{ field: 'name' }] } },
77+
formViews: { edit: { type: 'simple', sections: [{ label: 'Info', fields: [{ field: 'name' }] }] } },
78+
};
79+
80+
/** What `objectOnlyContainer` expands to, sorted — the whole expected answer. */
81+
const EXPANDED = ['crm_lead.default', 'crm_lead.edit', 'crm_lead.hot'];
82+
83+
/** The pre-existing shape: the binding lives only in `list.data.object`. */
84+
const listDataObjectContainer = {
85+
list: {
86+
label: 'All Accounts',
87+
type: 'grid',
88+
columns: [{ field: 'name' }],
89+
data: { provider: 'object', object: 'crm_account' },
90+
},
91+
formViews: { edit: { type: 'simple', sections: [{ label: 'Info', fields: [{ field: 'name' }] }] } },
92+
};
93+
94+
function fakeCtx() {
95+
return {
96+
logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() },
97+
registerService: vi.fn(),
98+
getService: vi.fn(() => undefined),
99+
trigger: vi.fn(),
100+
} as any;
101+
}
102+
103+
function newPlugin(): any {
104+
return new MetadataPlugin({ watch: false, config: { bootstrap: 'lazy' } });
105+
}
106+
107+
/** Load `views` through the real artifact door; returns the plugin. */
108+
async function loadViews(views: unknown[]): Promise<{ plugin: any; total: number }> {
109+
const plugin = newPlugin();
110+
// Fresh deep copy per load — the door mutates items in place (`applyProtection`).
111+
const definition = JSON.parse(JSON.stringify({ manifest: MANIFEST, views }));
112+
const total = await plugin._parseAndRegisterArtifact(fakeCtx(), definition, 'fixture-13912');
113+
return { plugin, total };
114+
}
115+
116+
const names = (items: unknown[]): string[] =>
117+
(items as { name: string }[]).map((i) => i.name).sort();
118+
119+
describe('#13912 artifact door — a container binds by its own top-level `object`', () => {
120+
it('the fixture carries the card shape (premise guard)', () => {
121+
// If a later edit gives any view arm a `data.object`, the cases below
122+
// stop testing this defect and start passing through the old chain.
123+
const raw = JSON.stringify(objectOnlyContainer);
124+
expect(raw).toContain('"object":"crm_lead"');
125+
expect(raw).not.toContain('"data"');
126+
expect(objectOnlyContainer).not.toHaveProperty('name');
127+
});
128+
129+
it('registers the container under the object its own `object` field names', async () => {
130+
const { plugin } = await loadViews([objectOnlyContainer]);
131+
132+
// Pre-fix the two-deep chain found nothing here and the container was
133+
// dropped by `if (!viewObject) continue` — never registered at all.
134+
expect(await plugin.manager.get('view', 'crm_lead')).toBeDefined();
135+
});
136+
137+
it('expands it into the independent `<object>.<key>` ViewItems', async () => {
138+
const { plugin, total } = await loadViews([objectOnlyContainer]);
139+
140+
for (const name of EXPANDED) {
141+
const item = (await plugin.manager.get('view', name)) as any;
142+
expect(item, `expanded view '${name}' should register`).toBeDefined();
143+
expect(item.object).toBe('crm_lead');
144+
expect(item.viewKind === 'list' || item.viewKind === 'form').toBe(true);
145+
}
146+
// One container + three expanded items.
147+
expect(total).toBe(1 + EXPANDED.length);
148+
});
149+
150+
it('makes `getViewsByObject()` answer for that object, with the expansion and never the container', async () => {
151+
const { plugin } = await loadViews([objectOnlyContainer]);
152+
153+
const views = (await plugin.manager.getViewsByObject('crm_lead')) as Record<string, unknown>[];
154+
155+
// The card's symptom, in one assertion: this used to be `[]`.
156+
expect(views.length).toBeGreaterThan(0);
157+
expect(names(views)).toEqual(EXPANDED);
158+
// #7163 — the container itself is never an answer.
159+
expect(names(views)).not.toContain('crm_lead');
160+
expect(views.some((v) => v.list !== undefined || v.listViews !== undefined)).toBe(false);
161+
});
162+
163+
it('prefers the container own `object` over a disagreeing `list.data.object`', async () => {
164+
// The ordering #13407 settled at the runtime door, made observable: the
165+
// authorial top-level field decides the registration key. No container
166+
// shipped in this repo sets both, so nothing existing moves — this pins
167+
// the order itself so the four derivation sites cannot drift apart again.
168+
const { plugin } = await loadViews([
169+
{
170+
object: 'crm_lead',
171+
list: {
172+
label: 'All',
173+
type: 'grid',
174+
columns: [{ field: 'name' }],
175+
data: { provider: 'object', object: 'crm_account' },
176+
},
177+
},
178+
]);
179+
180+
expect(await plugin.manager.get('view', 'crm_lead')).toBeDefined();
181+
expect(await plugin.manager.get('view', 'crm_lead.default')).toBeDefined();
182+
expect(await plugin.manager.get('view', 'crm_account')).toBeUndefined();
183+
expect(await plugin.manager.get('view', 'crm_account.default')).toBeUndefined();
184+
});
185+
186+
it('reads the same field on the fall-through registrar (the second derivation site)', async () => {
187+
// A `view` entry that is NOT an aggregated container (no `list`/`form`/
188+
// `listViews`/`formViews`) and carries no `name` reaches the second
189+
// site. It, too, used to be dropped despite declaring its binding — the
190+
// door discarding an item's own declared identity, which is exactly the
191+
// shape `ViewSchema.name`'s comment says the door must not repeat.
192+
const { plugin, total } = await loadViews([{ object: 'crm_lead', label: 'Lead views' }]);
193+
194+
expect(total).toBe(1);
195+
expect(await plugin.manager.get('view', 'crm_lead')).toBeDefined();
196+
});
197+
198+
// ------------------------------------------------------------------
199+
// Controls — green in BOTH directions, and measured so.
200+
// ------------------------------------------------------------------
201+
202+
it('CONTROL: a container bound through `list.data.object` still registers and expands', async () => {
203+
const { plugin, total } = await loadViews([listDataObjectContainer]);
204+
205+
expect(await plugin.manager.get('view', 'crm_account')).toBeDefined();
206+
expect(names(await plugin.manager.getViewsByObject('crm_account'))).toEqual([
207+
'crm_account.default',
208+
'crm_account.edit',
209+
]);
210+
expect(total).toBe(3);
211+
});
212+
213+
it('CONTROL: a container with no derivable binding at all is still skipped', async () => {
214+
// No top-level `object`, no `data.object`, no `name` — the derivation
215+
// returns undefined and nothing is registered. The fix widens WHERE the
216+
// binding may be declared; it does not invent one.
217+
const { plugin, total } = await loadViews([
218+
{ list: { label: 'Orphan', type: 'grid', columns: [{ field: 'name' }] } },
219+
]);
220+
221+
expect(total).toBe(0);
222+
expect(await plugin.manager.list('view')).toEqual([]);
223+
});
224+
});

packages/metadata/src/plugin.ts

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,18 @@ const ARTIFACT_FIELD_TO_TYPE: Record<string, string> = {
161161
// callers below and for `view-expand.test.ts`.
162162
export { isAggregatedViewContainer, expandViewContainer } from '@objectstack/spec';
163163
import { isAggregatedViewContainer, expandViewContainer } from '@objectstack/spec';
164+
// [#13912] WHICH object a container binds to is a separate question from HOW it
165+
// expands, and it is the one that had drifted here: this file walked two levels
166+
// (`list.data.object` then `form.data.object`) and never read the container's
167+
// own top-level `object` — the field `ViewSchema.object` documents as the
168+
// authorial signal "read by getViewsByObject() / GET /meta/view?object=". The
169+
// one spelling of that derivation for this package already exists:
170+
// `view-container-expansion.ts`'s `deriveViewContainerObject`, whose header
171+
// declares it to be the single place the drift is repaired rather than a third
172+
// private copy to fall behind. Both sites below call it instead of restating a
173+
// chain of their own; it carries the same order #13407 settled at the runtime
174+
// door (`expandRuntimeViewContainer` in `packages/metadata-protocol`).
175+
import { deriveViewContainerObject } from './view-container-expansion.js';
164176
import type { IHttpServer } from '@objectstack/spec/contracts';
165177

166178

@@ -930,9 +942,13 @@ export class MetadataPlugin implements Plugin {
930942
// reads. Already-independent ViewItems carry a top-level `name`
931943
// and fall through to the normal path below.
932944
if (metaType === 'view' && isAggregatedViewContainer(item)) {
933-
const viewObject =
934-
(item as any)?.list?.data?.object
935-
?? (item as any)?.form?.data?.object;
945+
// [#13912] `object` FIRST — a package-shipped
946+
// `defineView({ object: 'crm_lead', list: { columns } })`
947+
// sets the binding here and nowhere else, and the old
948+
// two-deep chain skipped that container entirely (`continue`
949+
// below), so it never reached the registry at all and
950+
// `getViewsByObject()` had nothing to expand.
951+
const viewObject = deriveViewContainerObject(item);
936952
if (!viewObject) continue;
937953
applyProtection(item as any, {
938954
packageId: manifestPackageId,
@@ -957,7 +973,9 @@ export class MetadataPlugin implements Plugin {
957973
}
958974
// Most metadata items carry a top-level `name`. The `View`
959975
// container (UI namespace) is an exception: it has no own
960-
// `name` — its identity is the target object, encoded under
976+
// `name` — its identity is the target object, declared in the
977+
// container's own top-level `object` field and, for containers
978+
// written before that field was read here, under
961979
// `list.data.object` (or `form.data.object`). Mirror the
962980
// resolution used by `ObjectQL.SchemaRegistry` so that
963981
// artifact-loaded views land in `MetadataManager` under the
@@ -967,12 +985,16 @@ export class MetadataPlugin implements Plugin {
967985
// boot-only SchemaRegistry cache and is never refreshed on
968986
// file edits — leaving MetadataService empty for view/* and
969987
// forcing all reads to return the stale boot copy.
988+
//
989+
// [#13912] `deriveViewContainerObject` is that resolution, in
990+
// the package's one spelling. Its last fallback — the row's own
991+
// `name` — cannot fire on this branch (we are here only because
992+
// `item.name` is falsy), so what it adds here is exactly the
993+
// leading `object` term this path was missing.
970994
let name = (item as any)?.name;
971995
if (!name) {
972996
if (metaType === 'view') {
973-
name =
974-
(item as any)?.list?.data?.object
975-
?? (item as any)?.form?.data?.object;
997+
name = deriveViewContainerObject(item);
976998
}
977999
}
9781000
if (!name) continue;

0 commit comments

Comments
 (0)